feat(mcp): route project-qualified paths in the POSIX tools - #1421
feat(mcp): route project-qualified paths in the POSIX tools#1421phernandez wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfd26bafa9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6224e1626a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6224e1626a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if "/" in candidate: | ||
| detected = await detect_project_from_identifier_prefix(candidate, config, context=context) |
There was a problem hiding this comment.
When a cloud/factory session advertises a project whose permalink is also an accessible workspace slug, and the next path segment names a project in that workspace (for example, /team is a mount while workspace team contains project docs), this lookup performs workspace-qualified discovery before consulting addressable_projects. Consequently, cat("team/docs/x") or ls("team/docs") routes to project docs in workspace team instead of directory docs in the advertised project team, potentially reading the wrong project's data. Fresh evidence after the prior fix is that session-mount matching remains only in the detected is None fallback, so check the advertised first-segment mount before attempting workspace-qualified parsing.
AGENTS.md reference: AGENTS.md:L337-L343
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c57ccefcf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _session_routes_to_cloud(): | ||
| project_list = await _session_project_list(context=context) | ||
| projects = ( | ||
| AddressableProject(name=item.name, permalink=item.permalink) |
There was a problem hiding this comment.
In a factory/cloud session scoped to a non-default workspace, if that workspace and an accessible default workspace both contain a project with the same permalink, this discards the project's external_id and workspace and turns the advertised mount into a bare name. The later resolve_workspace_project_identifier call resolves duplicate bare names using the cached/default workspace, so on a first call cat("docs/x") can read the default workspace even though ls("/") advertised docs from the session workspace. Preserve an unambiguous workspace-qualified identifier or external ID in the route.
AGENTS.md reference: AGENTS.md:L393-L395
Useful? React with 👍 / 👎.
| if detected is None and "/" in candidate: | ||
| detected = await detect_project_from_identifier_prefix(candidate, config, context=context) |
There was a problem hiding this comment.
When acme is an accessible non-current workspace with project docs and no mount is named acme, ls("acme/docs") reaches this branch, but detect_project_from_identifier_prefix recognizes workspace routes only when _split_workspace_identifier_segments finds a third nonempty path segment. With one current mount this falls through to the default and lists /acme/docs inside the current project; with several mounts it refuses. Treat the two-segment <workspace>/<project> form as that project's root with an empty remainder.
AGENTS.md reference: AGENTS.md:L393-L395
Useful? React with 👍 / 👎.
| projects = ( | ||
| AddressableProject(name=name, permalink=generate_permalink(name)) | ||
| for name in ConfigManager().config.projects |
There was a problem hiding this comment.
ProjectInfoRequest and ProjectService accept project names containing /, and generate_permalink preserves that separator. In a multi-project local config containing Research/2026, the root advertises /research/2026, but the resolver compares only the first segment (research) against the full stored permalink (research/2026), so the advertised mount cannot be entered. Either reject slash-containing project names at the project boundary or route using the complete mount permalink.
AGENTS.md reference: AGENTS.md:L337-L343
Useful? React with 👍 / 👎.
Projects become mount points (#1415): every posix verb accepts <project>/path — exactly the prefixed identifiers tool outputs and stored permalinks produce — resolved by one shared helper the CLI inherits. Explicit project params win only on agreement; disagreement refuses naming both. ls with no project lists active projects as the root directory. In multi-project configs an unqualified path that matches no project refuses with the copyable project list; single project configs keep resolving unqualified paths unchanged. Motivated by measured agent behavior in the #1398 A/B runs: agents faithfully quote prefixed output identifiers, omit the project arg (shell affordances prime cwd thinking), and the stateless default was silently wrong — both surfaces wrote a perfect relation into the wrong project. Known scope gap for cloud/factory-mode surfaces noted in review; follow-up tracked on #1415. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
The mount view and the routing resolver read different sources, so a cloud tenant could advertise /research in ls '/' while routing asked the local config, found nothing, refused nothing, and fell through to the default project. In a team workspace that default is one shared mutable is_default flag, so an unqualified call could silently read or write another member's project. Root cause of why the existing multi-project refusal never fired in the hosted server: BasicMemoryConfig always materializes a placeholder 'main' project, so config.projects is never empty and len(config.projects) > 1 was never a usable signal for a cloud session. Both surfaces now read one addressable_projects() set — config locally, the session's project listing in factory/cloud mode, memoized per MCP request. The refusal counts that same set, so anything ls '/' advertises is addressable, asserted as a property over the advertised list rather than a hand-picked example. Nothing consults is_default. Single-project workspaces still resolve unqualified references, and the local path adds no new call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Workspace-qualified parsing ran before the advertised-project lookup, so
when a project's permalink was also an accessible workspace slug, the
next segment naming a project in that workspace won. With /team
advertised and workspace team holding project docs, cat('team/docs/x')
read workspace team's docs project instead of the docs directory in the
advertised team project — another project's data, from a name ls /
promised.
The advertised list is a promise, so it now claims the first segment
first; workspace-qualified spellings resolve only when no mount matches.
The collision this creates is pinned rather than swallowed: while a
project's permalink equals a workspace slug, that workspace's other
projects lose their qualified path spelling. They stay addressable
through the project parameter, and pairing the two now raises a prefix
conflict instead of quietly agreeing, which is what teaches the escape.
Two side effects worth having: mount-prefixed paths skip workspace
discovery entirely, and ls 'research' and ls 'research/notes' now agree
on the project spelling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp
Signed-off-by: phernandez <paul@basicmachines.co>
1c57cce to
c940782
Compare
1 participant
Why
Implements #1415: projects become mount points in the POSIX tool namespace. Motivated by measured agent behavior in the #1398 A/B runs — tool outputs return project-prefixed identifiers, inputs demanded a separate
projectarg, agents faithfully quote outputs and omit the arg (shell affordances prime cwd-thinking), and the stateless default was silently wrong. In run 5, both surfaces' agents wrote a perfect relation into the wrong project.Stacked on #1416 (
1398-eval-test-fixes).What changed
mcp/project_context.py): first path/identifier segment naming an active project routes there with the remainder — inputs now accept exactly what outputs produce. The CLI verbs inherit it through the shared layer.projectparam + path prefix disagreeing refuses naming both; a workspace-qualified explicit param keeps its workspace (review catch — the agree-branch initially discarded it).ls /lists active projects as the root directory — in-band discovery, the mount-point view.no project 'x' — active projects: …); single-project configs resolve unqualified paths unchanged.Validated by the eval it came from (run
at-b0442a6f5cd7, Sonnet 5)The entire wrong-scope failure class vanished.
curate-connect— which agents had "solved" into the wrong project on every prior run — passed on both surfaces. Posix's metadata-search went 0/3 → 3/3 (agents brute-force it at ~2× rich's token cost, which is the remaining efficiency gap: grep/ls/find have no structured frontmatter predicates). Where the surfaces overlap cleanly, posix stays 17–24% cheaper (manual 23.8k vs 31.3k; curate 74.7k vs 89.9k). Aggregate tokens/completed: rich 72.9k, posix 88.0k (+21%, driven by the metadata brute-forcing and navigational chattiness — 13.8 vs 7.8 mean calls).Remaining failures are genuine task behavior (decoy discrimination on both surfaces, one formatting miss, one incomplete status update, one token-budget stop) — none are scope errors.
Verification
ty— clean;tests/mcp1030 passed;tests/cli933 passed (21 new resolver tests + 16 routing tests; additive-only test diffs)🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp