Skip to content

fix(map): canonicalize and validate workspace roots - #545

Open
Hiro-Chiba wants to merge 2 commits into
ix-infrastructure:mainfrom
Hiro-Chiba:fix/map-root-resolution
Open

fix(map): canonicalize and validate workspace roots#545
Hiro-Chiba wants to merge 2 commits into
ix-infrastructure:mainfrom
Hiro-Chiba:fix/map-root-resolution

Conversation

@Hiro-Chiba

@Hiro-Chiba Hiro-Chiba commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #544

Summary

Select, validate, and canonicalize the workspace root before ix map registers a workspace or acquires its lock.

Type

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Test
  • CI

Changes

  • Reuse the same root-resolution cascade as the other CLI commands: explicit path, nearest registered workspace, named workspace, default workspace, Git root, then cwd.
  • Use execFileSync for Git root discovery instead of a shell command.
  • Reject missing and non-directory paths before bootstrap can persist them.
  • Canonicalize workspace roots at registration and lookup, preventing symlink aliases or Windows path aliases from creating an unfindable or duplicate workspace entry.
  • Canonicalize single-flight lock keys so equivalent paths share one map lock.
  • Emit structured JSON and LLM errors for invalid map paths, with a command-level JSON regression test.
  • Cover named/default workspaces, nested Git roots, symlinks, registration migration, missing paths, files, and lock identity.

Behavior change

Running ix map without a path from a subdirectory now maps the selected workspace or Git root. This also applies to IX_AUTO_MAP=1 callers. ix map . remains the explicit way to map only the current directory.

An existing registry entry stored under a non-canonical alias is normalized instead of duplicated. If its path-derived workspace ID changes, the existing migration path forces a re-ingest and cleans up the previous workspace ID.

Validation

  • npm test (85 files, 1454 passed, 3 skipped; parser smoke passed)
  • npm run typecheck
  • npm run build
  • npm run knip

Checklist

  • Tests pass
  • Smoke tests pass
  • No raw errors introduced
  • CLI output follows Ix format

Release checklist (if merging to main)

  • ix-cli/package.json version bumped
  • After merge: tag pushed (git tag vX.Y.Z && git push origin vX.Y.Z)
  • If backend changes: ix-memory-layer tagged and released first
  • If docker-compose.standalone.yml changed: verified curl | sh install works

@KageBinary

Copy link
Copy Markdown
Collaborator

Review — correct, with two consistency risks worth a decision

Validating the path before bootstrap can persist it is the right shape, and findGitRoot using execFileSync with stdio: ["ignore","pipe","ignore"] is strictly better than the execSync (shell) that resolveWorkspaceRoot still uses for the same question. The lock-key canonicalisation is a real fix — a symlinked root and its target used to take different locks and could map concurrently.

1. ix map and every other command now use different cascades

resolveMapRoot is:

pathArg -> registered workspace -> git root -> cwd

resolveWorkspaceRoot, which text, read, context, doctor and the rest use, is:

--root -> registered workspace -> named workspace (ix config set workspace) -> default workspace -> git root -> cwd

The two middle steps are missing. So for a user who has set a named or default workspace and runs from a directory no workspace covers but a git repo does, ix map now maps the git root while ix text answers for the named workspace. Before this PR ix map used process.cwd() — obviously different, so nobody could mistake them for the same rule. Half-aligning is the state where they look identical and are not.

Reusing resolveWorkspaceRoot and wrapping it in canonicalMapRoot would give one cascade and keep everything else in the PR.

2. realpathSync.native can register a root string that nothing else matches

canonicalMapRoot returns realpathSync.native(resolved), and that value becomes the registered root_path. But selectWorkspaceForCwd matches lexically:

workspaces.filter(w => isPathInside(w.root_path, cwd))

For the no-arg case this is fine — POSIX process.cwd() is already canonical, so the old code was effectively doing the same thing. For ix map <path> it is new: ix map /tmp/ws used to register /tmp/ws and now registers /private/tmp/ws on macOS.

Two consequences:

  • Windows. realpathSync.native expands 8.3 short names, and process.cwd() may report the short form. C:\Users\RUNNER~1\proj registered as C:\Users\runneradmin\proj is not lexically inside itself, so other commands would not find the workspace. CI is green, and real users do not type 8.3 paths, but CI does — we have been bitten by exactly this RUNNER~1 / runneradmin split before.
  • Existing registries. bootstrap.ts matches on w.root_path === rootPath, so a workspace previously registered under a non-canonical path gets a second entry rather than being updated. Harmless (the longest match wins and both point at the same directory) but it leaves duplicates behind, and it is invisible until someone reads ~/.ix/config.json.

Neither is a blocker; both argue for canonicalising root_path in one place — at registration — rather than at one caller.

3. Behaviour change worth putting in the body

ix map from a subdirectory now maps the whole repo instead of that subdirectory. That is the fix, and ix map . still scopes to cwd, so there is an escape hatch. But IX_AUTO_MAP=1 editor hooks fire ix map from wherever the editor is, so a hook that used to ingest one subtree now ingests the git root on every change. Please confirm that is intended before this ships — the cost lands on people who never read the changelog.

Smaller

  • Third hand-rolled json/llm/stderr switch in this stack (see my note on fix(text): enforce workspace-relative search paths #543). One reportError in ui.ts would cover all of them.
  • No test that ix map actually emits invalid_map_path in json or llm — the new tests all exercise the resolver directly. One end-to-end case would pin the wire format the same way the resolver cases pin the logic.
  • lockPathFor now canonicalises too, which is redundant once resolveMapRoot has already done it — harmless, and it does protect the other caller. Worth a one-line comment saying which one it is for.

@Hiro-Chiba

Copy link
Copy Markdown
Contributor Author

Thanks! I aligned the workspace root resolution and updated the PR body and tests. CI is passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] ix map can register duplicate or invalid workspace roots

2 participants