Skip to content

fix(mcp-grant-sync): stop claiming the overlay releases the call - #556

Merged
wenzowski merged 1 commit into
mainfrom
claude/mcp-grants-bundle-xpji8t
Aug 20, 2026
Merged

wenzowski merged 1 commit into
mainfrom
claude/mcp-grants-bundle-xpji8t

Conversation

@wenzowski

@wenzowski wenzowski commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Refs: CLOUD-734
DO-NOT-CLOSE

CLOUD-734 is already Done — released in v0.0.89. This corrects a claim its work
left in the repository, which is false and which a future session would read as
established.

Both headers asserted that the projection is read at the next session start and
therefore puts the committed grant in force. Measured false this session:

fact evidence
the overlay was written at 00:42, in the previous session that session's run log
the container's disk survived (files from 00:42–02:50 all present) mtimes
so the overlay was on disk before this session started at 02:58:57 mtimes
this session's --allowed-tools carries zero entries for that server the live CLI cmdline
the identical committed-but-inert call is refused anyway probed

That is the third surface tried and the third negative — after the generated
config edited mid-session and the overlay written mid-session. Every local
surface reachable from this repository has now been tested. The refusal comes
from the CCR proxy, upstream of the local permission layer, where no settings
file reaches it (anthropics/claude-code#61044, #61015; the capability gap
CLOUD-312 tracks).

No behaviour changes here, and the projection stays. CLOUD-734's acceptance
made the measurement the deliverable either way, and what shipped is still a
correct, tested, gated projection of committed intent onto a rotating identity —
it is also what makes mcp-allow-check's inert-rule predicate closable, since
"this rule names a server that never attached" is otherwise a verdict no edit can
satisfy. What changes is that both headers now say what the projection is for
instead of promising a release it does not deliver.

Touches no crate file, so release-plz cuts no tag for it.

Tests

No new cases — this is comment-only. tests/session-start.bats,
tests/mcp-grant-sync.bats and tests/mcp-allow-check.bats all still pass, and
the edit deliberately avoids every line those suites assert on.

Summary by CodeRabbit

  • Documentation
    • Clarified that startup settings are read before session hooks can modify them.
    • Documented that command-line settings are fixed at startup and cannot be changed by session hooks.
    • Explained that permission changes made before, during, or after a session remain subject to proxy restrictions.
    • Clarified that synchronization writes committed permission intent to the session overlay for validation, rather than granting permissions directly.
    • Documented that overlay updates do not override proxy refusals.

@linear-code

linear-code Bot commented Aug 20, 2026 •

Copy link
Copy Markdown
CLOUD-734 Re-project the committed MCP grants onto the session's generated config at SessionStart, so a rotating server id cannot silently revoke an approval a human already gave

Why

.claude/settings.json records grants by label (mcp__Linear__*, mcp__Claude_Code_Remote__create_session). The host attaches those servers under generated identifiers that rotate — measured this session as mcp__4db58e41-… and mcp__bf7c680d-…, with the Linear one changing mid-session. The committed grant therefore names something that does not exist, and the human intent it encodes is silently unenforced.

Where the policy actually lives — measured, not assumed. The per-session file /tmp/mcp-config-<session-id>.json carries, per server, a tools[] array with a permission_policy on each entry:

$ jq -r '[.mcpServers[].tools[]?.permission_policy] | group_by(.) | map({policy:.[0], n:length})' "$F"
[{"policy":"always_allow","n":53},{"policy":"always_ask","n":25}]

$ jq -r '.mcpServers["bf7c680d-…"].tools[]? | "\(.name) -> \(.permission_policy)"' "$F"
create_session  ->  always_ask
list_sessions   ->  always_ask
get_session     ->  always_ask
…

So the decision is expressed locally, per tool, in a file this repo's hooks can reach. That is the lever.

Measured negative: editing it mid-session does nothing. Flipping create_session to always_allow in a running session and retrying the call returned the same MCP tool call requires approval. The client does not consult the file per call — it reads it once at startup, or the proxy re-enforces, or both. This does not refute the mechanism below, and the distinction is the whole point: that edit landed after the read. A SessionStart hook writes before it.

Why the ordering is plausible rather than hopeful. .claude/hooks/session-start.sh's own header states it runs "before the session (and its project-scoped MCP servers) start", and CLOUD-196 established that ordering by measurement when it closed the Serena startup race. The generated config is consumed when those servers start, so a hook that rewrites it has a window — provided generation precedes the hook, which is what this issue measures first.

What makes the identification robust. The generated file carries no human label for the rotating servers; only github is named. But every server carries its full tool inventory, and tool names do not rotate. So a server is identified by what it exposes, never by what it is called — which is strictly more stable than the label the committed allowlist uses, and is the property whose absence caused the defect.

Scope, and the line this must not cross. This re-projects grants a human already committed. It never widens: a tool with no committed grant keeps always_ask. A hook that flipped all 25 always_ask entries would be a bypass on a trigger, which is the inverse of what this repository is for, and non-negotiable rule 4's sibling concern — a gate that disables gates. The predicate is intent-preserving by construction because its input is the committed file.

Acceptance

  • Ordering is measured and recorded: whether SessionStart runs before or after the session config is generated.
  • A tool named by a committed allow rule carries always_allow in the generated config once the session is live.
  • A tool not named by a committed rule is untouched, asserted by a fixture that would fail if the rewrite widened.
  • If the ordering or the proxy defeats the mechanism, that is recorded as a measured negative and the task closes without shipping a rewriter — the measurement is the deliverable either way.

Refinement — Ready (re-project committed intent onto generated identity; identify a server by its tools, never by its name)

Refinement gate: Definition of Ready & Done. This body carries only specializations.

  • Source of truth (§1). .claude/settings.json's permissions.allow stays the only authority for what is granted; this introduces no second grant surface and can express nothing the committed file does not already say. The generated session config is a projection target, not a source — it is written, never read for intent. Where a label needs resolving to a server, the label→identifying-tool mapping is consumer config in batten.toml (non-negotiable rule 1: consumer facts live in the consumer's own file, never in crates/batten).
  • Computable predicate (§2). For every allow rule of the form mcp__<label>__<tool>, the generated config's entry for that tool carries always_allow; for every tool with no committed rule, its policy is unchanged from what the host wrote. Decided by reading the two files and comparing — a pure function of (settings, generated config), no network. The ordering question is decided by a timestamp the hook stamps against the config's mtime.
  • Effect (§3). write — it mutates a file outside the repository, and that must be declared rather than smuggled in under the read a hook usually is. It writes exactly one path, the session's own generated config, and never the committed settings; the asymmetry is the safety property and belongs in the effect table.
  • Output & exit (§5). Pointer-only per non-negotiable rule 4: a count of tools re-projected and the server-id prefix, never a tool name, never a URL, and never the headers block — that file carries session credentials, so echoing any of it is the worst available failure. Exit 0 when the projection is complete or a no-op; non-zero when a committed rule names a tool no attached server exposes, which is CLOUD-684's inert-rule finding surfacing from the other side.
  • Commit / bump (§6). feat → patch until 0.1.0 (DoR §6: below 0.1.0 release-plz bumps the patch whatever the type says).
  • Test obligation (§7). tests/mcp-grant-sync.bats, over fixture config trees rather than a live session, in the shape mcp-attach-check's suite already uses: (a) a committed rule naming a tool flips exactly that tool; (b) a tool with no committed rule is untouched — the widening guard, and the case that must be shown to fail if the predicate is wrong (CLOUD-418); (c) a rule naming a tool no server exposes exits non-zero; (d) a server is matched by tool inventory, proven by a fixture whose identifier differs from any label; (e) the report contains no tool name, URL or header value. The live-ordering measurement is recorded on this issue, not asserted by a test, since it is a property of the world.
  • Blockers (§8). None. relatedTo CLOUD-684, which gates the inert rule from the settings side while this re-projects it from the session side — the two are complements and neither subsumes the other; relatedTo CLOUD-196, which established the SessionStart-before-MCP ordering this depends on; relatedTo CLOUD-187, which recorded that mid-session contract changes do not bind a running session — the same startup-snapshot property, and the reason this must be a SessionStart hook rather than any later event.

What this does not attempt. Making the call succeed if the CCR proxy re-enforces policy server-side. Upstream (anthropics/claude-code#61044, #61015) reports the refusal arriving as an HTTP error from the endpoint, which if true means no local projection can help and the honest outcome is the measured negative in the acceptance above. That is a capability gap of the kind CLOUD-312 tracks, not something to engineer around here.

Review in Linear

Both headers asserted that the projection is read at the next session start and
therefore puts the committed grant in force. Measured false: the overlay was on
disk before this session started — same container, same disk, verified by mtime —
and the identical committed-but-inert call was refused anyway, as it was for the
generated config edited mid-session and for the overlay written mid-session.

Every local surface reachable from this repository has now been tried. The
refusal comes from the CCR proxy, upstream of the permission layer, which no
settings file reaches.

What the projection is for is stated instead: the committed intent expressed
against the identity that actually attached, which is what mcp-allow-check's
inert-rule predicate reads and what makes that predicate closable at all.

Refs: CLOUD-734
DO-NOT-CLOSE
@coderabbitai

coderabbitai Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d02d2bb5-e842-4114-a891-432691851eb5

📥 Commits

Reviewing files that changed from the base of the PR and between a0c6edb and 3bca59f.

📒 Files selected for processing (2)
  • .claude/hooks/session-start.sh
  • mise-tasks/mcp-grant-sync
🚧 Files skipped from review as they are similar to previous changes (2)
  • mise-tasks/mcp-grant-sync
  • .claude/hooks/session-start.sh

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The change updates comments in the session-start hook and MCP grant synchronization task. The comments distinguish startup configuration from permission enforcement and describe how committed intent supports mcp-allow-check.

Changes

MCP grant synchronization

Layer / File(s) Summary
Synchronization rationale
.claude/hooks/session-start.sh, mise-tasks/mcp-grant-sync
Comments document startup argument immutability, CCR-proxy refusal behavior, overlay limitations, rotating server identities, and the projection’s role in mcp-allow-check.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 3bca5

This change clarifies what the grant projection represents without changing runtime behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • button-inc/batten#547: Introduces the MCP synchronization components whose behavior rationale is clarified here.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main comment-only change: removing the incorrect claim that the overlay releases the MCP call.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/mcp-grants-bundle-xpji8t

Comment @coderabbitai help to get the list of available commands.

@wenzowski
wenzowski marked this pull request as ready for review August 20, 2026 03:57
@wenzowski
wenzowski force-pushed the claude/mcp-grants-bundle-xpji8t branch from b5d7944 to 3bca59f Compare August 20, 2026 03:57
@sonarqubecloud

Copy link
Copy Markdown

@wenzowski

Copy link
Copy Markdown
Contributor Author

/fast-forward

@wenzowski
wenzowski merged commit 3bca59f into main Aug 20, 2026
10 checks passed
@wenzowski
wenzowski deleted the claude/mcp-grants-bundle-xpji8t branch August 20, 2026 04:13
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