Skip to content

fix(cli): narrow the public seat tool list on every adapter - #2994

Merged
lilyshen0722 merged 2 commits into
mainfrom
fix/public-attach-file
Oct 30, 2026
Merged

lilyshen0722 merged 2 commits into
mainfrom
fix/public-attach-file

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

@coderabbitai ignore

Public seats on Claude, Codex, OpenCode and Qwen are no longer offered commonly_attach_file, which brings them in line with Goose's public list. All four adapters read one shared list in cli/src/lib/public-withheld-tools.js, and each removes the tool its own way:

adapter how
Claude mcp__<server>__commonly_attach_file in --disallowedTools (public and workspace trust)
Codex mcp_servers.<name>.disabled_tools=["commonly_attach_file"] on public seats
OpenCode <server>_commonly_attach_file: deny, after the <server>_* allow, so the deny wins whichever matching order applies
Qwen both spellings in --exclude-tools on public seats

Claude workspace seats (own-repo trust) follow the same list. Private seats are unchanged; the exact -c list in the Codex private test confirms this.

Tests: one assertion per adapter in the existing public-spawn tests, plus the Claude workspace test now expecting the same deny.

Written for TASK-287.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N4VWLC9MiWprFWXFNjJSnr

Public Claude, Codex, OpenCode and Qwen seats are no longer offered
commonly_attach_file, matching Goose's public list. One shared list,
applied in each adapter's own vocabulary. A workspace (own-repo) Claude
seat keeps it.

TASK-287

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4VWLC9MiWprFWXFNjJSnr
@lilyshen0722
lilyshen0722 force-pushed the fix/public-attach-file branch from 7c480b5 to 9c8bf28 Compare October 30, 2026 11:13

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tier 2 gate at 2e548cd95773343150a2db7982d95947bd196c8 (cli, TASK-287). This head is 9c8bf28 plus "fix(cli): workspace seats follow the same tool list as public seats". My earlier reading was at 9c8bf28 and the verdict below covers both commits.

What I ran: a scratch copy of the head with cli/node_modules from the #2987 tree and @commonlyai/seats linked to the head's packages/seats.

  • Adapter tests (claude.environment, claude, claude.context-tokens, codex, opencode, qwen) plus environment, default-environment and public-pod-sandbox-gate: 9 suites, 317 passed, 2 skipped (skips already in the suite).
  • One mutant per adapter, each removing that adapter's withheld deny, plus an OpenCode mutant that moves the deny above the _* allow. Each mutant fails the test that owns the exclusion. At 2e548cd, Claude mutants: withheld denies not appended (2 tests fail, including the workspace-seat test); workspace exemption put back (the workspace-seat test fails). Each source file was restored byte-identical after its run.

(a) Does each exclusion remove the tool on a public spawn?

  • Claude: a confined spawn carries mcp__<server>__commonly_attach_file in --disallowedTools. The delta removes the workspace exemption, so public and workspace spawns now share the list. Deny-over-allow for the mcp__commonly__* wildcard is documented Claude Code behaviour and was not run live.
  • Codex: a public spawn emits -c mcp_servers.<name>.disabled_tools=["commonly_attach_file"]. The key is in the config schema of the codex-cli 0.162.1 binary on this machine, next to mcp_servers and enabled_tools. Not run against a live session.
  • OpenCode: the deny key is commonly_commonly_attach_file, placed after commonly_*. The binary is not installed here, so I read OpenCode v1.19.4 source. Config string values become rules in object order (fromConfig). evaluate takes the last matching rule (findLast). disabled() hides a tool from the model when its last matching rule is a * deny. So the deny wins over the wildcard allow, and the tool is removed from the list the model sees, not only refused when called.
  • Qwen: commonly_attach_file and mcp__commonly__commonly_attach_file both go into --exclude-tools, on public spawns only. Not run; qwen is not installed here.

(b) Private seats: unchanged. The Claude list applies only on the confined policy path, and the private Claude spawn never calls it. Codex's disabled_tools is gated on ctx.publicSeat, which is sandbox.trust === 'public' after legacy normalization (internal becomes public). Qwen's list is gated on isPublic. OpenCode's deny is only in publicPermissions. The exact -c list on the private codex spawn still passes unchanged.

(c) Claude workspace seat (own-repo trust): it should not keep attach_file, and the new head is right to remove the exemption. The reason is that trust is an environment declaration, so an exemption is only as correct as the label on each seat. One list for every confined seat avoids depending on that label being right, and an owner who wants a file in a pod can still upload it through the app. This is a recommendation, and I did not find a defect in the uniform version.

Not verified by a live run: the codex key's effect on the live tool list; qwen's matching of both spellings in --exclude-tools; Claude's deny-over-allow.

Non-blocking:

  1. OpenCode name mismatch for server names containing a hyphen. permissionMcpName maps every character outside [A-Za-z0-9_] to _, hyphens included. OpenCode's tool key (toolName → sanitize in packages/opencode/src/mcp/catalog.ts at v1.19.4) keeps -. For a server named commonly-mcp, the deny key commonly_mcp_commonly_attach_file does not match the real key commonly-mcp_commonly_attach_file, so the tool stays in the model's list. Current declarations use commonly, so no present seat is affected. Suggest /[^A-Za-z0-9_-]/g to match OpenCode's sanitize, with a test that uses a hyphenated name. The existing _* allow has the same mismatch.
  2. On a public Codex seat, disabled_tools is written for every stdio server in the environment spec, not only the one named commonly. Harmless for this tool name; noting it so a reader does not assume otherwise.

Written by Rhea, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otto gate (Opus, Tier 2) at 2e548cd. That is 9c8bf28 plus 2e548cd, which withholds the tool on Claude workspace seats too.

Each adapter's public spawn drops commonly_attach_file:

  • Claude: every confined spawn (Seatbelt and bwrap, public trust) goes through buildPublicClaudePolicyArgs. It adds mcp__<server>__commonly_attach_file to --disallowedTools for every declared server, and in Claude Code a disallow beats the mcp__<server>__* allow. Public bwrap passes no trust, so it gets the deny as well.
  • Codex: publicSeat is trust === 'public' after normalizeSandboxTrust, so legacy internal is covered too, and workspace trust is refused for codex. mcp_servers.<name>.disabled_tools is part of the MCP server schema in the codex-cli 0.162.1 binary on this host.
  • OpenCode: the <server>_commonly_attach_file: deny key comes after <server>_*: allow. Under last-match-wins the deny wins. Under longest-pattern-wins the deny key is also longer, so it wins either way. The test pins the key order.
  • Qwen: both spellings go into --exclude-tools, on public spawns only.

Private seats: unchanged. Codex keeps its exact -c list. OpenCode still sets permission: 'allow'. Qwen's exclusions stay behind isPublic. Private Claude never calls the public policy builder.

Tests: the four suites pass, 194 passed and 1 skipped. I tried five mutants and each one failed its own test:

  1. Claude withheld denies off fails the public Seatbelt test.
  2. The workspace exemption restored fails the workspace test (at 2e548cd).
  3. The codex disabled_tools push removed fails the public warm MCP test.
  4. The OpenCode deny moved before the allow fails the public permissions test.
  5. The Qwen spread removed fails the public launch test.

I couldn't run real OpenCode or Qwen binaries on this host. The OpenCode and Qwen results come from the config semantics and the tests.

The workspace question. Withholding it there is right, and 2e548cd does that. A workspace-trust turn only runs when every one of the seat's pods is confirmed private. Otherwise workspaceTurnPlacement runs the turn as public. But commonly_attach_file reads filePath with readFileSync in the MCP server process, so the workspace tier's Read(...) deny rules don't govern it, including ./.git/** and the home-directory denies. The jail is the only bound, and it keeps .git readable for the git tool. The tool can come back for workspace seats once attach_file confines its path to what the seat's Read policy allows. At this head the workspace test asserts the deny, and putting the trust === 'workspace' exemption back fails it.

Non-blocking:

  • docs/agents/COMMONLY_MCP.md still says "public seats"; Claude workspace seats are now included too.
  • Qwen's prefixed spelling is hardcoded to mcp__commonly__. The other three adapters loop over every declared server name. A public Qwen seat with a Commonly server declared under another name keeps the tool under that name. Building the prefixed list from Object.keys(mcp.mcpServers) would match the others.

CI at filing: CLI, MCP and hosted runtime tests is green. The backend and service shards were still running; this PR touches no backend code.

Written by Otto, a Commonly agent — https://commonly.me/v2/pods/6a920d3eb08dee6c42a81bfc

GATE: PASS

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connector lane review at 2e548cd (cli/).

I read 9c8bf28 earlier. The one commit since then, 2e548cd, carries out the lane ruling that a workspace seat loses commonly_attach_file too. That seat's sandbox can still read the repo's .git, and the attach tool reads outside the agent's usual read rules, so it could post .git/config (which can hold a token in a remote URL) into a pod.

  • buildPublicClaudePolicyArgs now adds the withheld denies on every confined trust. The trust === 'workspace' exception is gone, and private seats never reach this builder.
  • The workspace-seat test now expects the deny (adapters.claude.environment.test.mjs:1278). Both gates' mutants show it is the case that owns this rule.
  • CI at this head: 21 pass, 4 skipped, 0 failing.

The live public Codex spawn proof is still owed before TASK-287 closes. It doesn't block this merge.

LANE: PASS

@lilyshen0722
lilyshen0722 added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit 2c53798 Oct 30, 2026
29 of 31 checks passed
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.

1 participant