Skip to content

feat(permissions): follow-ups from the permission-mode review - #2425

Open
aponcedeleonch wants to merge 11 commits into
mainfrom
impl/permission-mode-followups
Open

aponcedeleonch wants to merge 11 commits into
mainfrom
impl/permission-mode-followups

Conversation

@aponcedeleonch

@aponcedeleonch aponcedeleonch commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Stage: Implementation. Implements the follow-ups agreed in the #1935 review (now merged).

What it does

  • Refused-mode recovery reachable from mecatui. When mecatui --resume shows a pending approval that the server refuses because the session's stored mode is no longer admitted, shift+tab now switches to a lower mode. The server's refused_mode_recovery cancels the pending turn without running it, and the UI returns to an idle, usable session with the draft kept. A refused switch keeps the approval open and shows the server's reason. An approve or deny that the server refuses for this reason now names the recovery instead of the generic "could not be acknowledged" message. Gated on the refused_mode_recovery feature.

  • Refused-mode recovery observable to watchers. After the recovery save, SetMode appends and publishes the same shape a live cancellation produces, stamped with the parked run's id: permission.retract for the ask, one cancellation tool.result per unanswered call, then result{cancelled}. Watchers and live subscribers no longer see an ask that stays open forever, and eventsource.Fold agrees with the snapshot. The close-out text comes from the new Session.InterruptCloseOuts, the same text the next run's Interrupt writes.

  • A resumed auto subagent needs an enforcing checker. A persisted child stored in auto and resumed under a yolo parent kept auto but ran with advisory or no checking after a restart that removed the checker. A resume now runs the host's admission rule on the child's re-admitted mode (new agent.WithChildModeAdmission, wired to the same server.ModeAdmission the server applies to stored sessions). A refused resume is a tool error to the parent; the child doesn't run and its history is unchanged.

  • Crash-orphaned running session in a refused mode. A session left running by a crash, under a mode the restarted server no longer admits, had no prompt recovery: StartRun refused before its crash-orphan repair, SetMode was illegal from running, CancelRun had no live run, and the stale sweep only applies after its threshold. SetMode now abandons the dead turn to idle (closing out unanswered tool calls, running nothing) and applies the mode in one save, under runEntryMu and the mutation lease with no registered run, the same proof StartRun's repair uses. A peer holding the lease keeps it refused, and the admission error for a running session names the recovery. Raised by @jhrozek on feat(permissions): one named permission-mode vocabulary and two admission gates #1935.

  • ApprovePlan with an unspecified or unrecognized target denies. The proto documents PLAN/UNSPECIFIED target_mode as a deny, and HTTP already treats an empty or unknown target that way, but gRPC mapped UNSPECIFIED, and any out-of-range enum value, to default and approved the plan. Both now deny. Raised on feat(permissions): one named permission-mode vocabulary and two admission gates #1935.

  • The stored-mode refusal has its own error code. admitStoredMode now refuses with session_mode_not_admitted (still FailedPrecondition / HTTP 412) instead of the generic failed_precondition. mecatui offers the shift+tab recovery only for that code, so another refusal on the verdict path (for example, an ask another client already answered) keeps the generic "changed or could not be acknowledged" message and no mode switch. The TypeScript SDK error-code list gains the code.

  • Lease warnings. Build warns when a store other processes may share resolves no session lease, and a lease backend that reports leasing unsupported now downgrades at WARN instead of INFO. The storage-level gaps (no fencing token or aggregate version on Save) are out of scope.

  • Tests. The unavailable-mode composer tests now execute the returned commands and assert exactly one transport send. TestPermissionModeTokenDerivesNoTrust seeds an AGENTS.md sentinel and checks whether it reaches the model, in addition to the trust narration. The resumed-subagent admission test runs for a no-FS parent too, and the mecatui client's SetMode state mapping is tested over real gRPC.

  • Server CLI reference. user-docs/reference/server-cli.md is regenerated; it had drifted from the --permission-mode help text changed in feat(permissions): one named permission-mode vocabulary and two admission gates #1935.

API compatibility

Assessment: additive. No API major change.

  • The refused-mode recovery and its refused_mode_recovery feature are unreleased (feat(permissions): one named permission-mode vocabulary and two admission gates #1935). This PR only appends existing event types (permission.retract, tool.result, result) with the existing run-id stamping on that feature-advertised path. Existing clients already handle all three for a run. No new event type, no field change. The feature and SetMode proto comments describe the events.
  • A Subagent resume returns an error tool result only when the child's mode is one the server already refuses for stored sessions. No shapes change.
  • SetMode on a stored running session left by a crash in a refused mode goes from InvalidArgument to success, within the same unreleased refused_mode_recovery feature. No shapes change; the feature, proto SetMode comment, and API-surface doc describe it.
  • gRPC ApprovePlan with UNSPECIFIED or an unrecognized target_mode now denies instead of approving. This restores the documented proto contract and matches HTTP, so a conforming client is unaffected. The TypeScript SDK and mecatui always send an explicit, defined target.
  • The stored-mode refusal's ErrorInfo reason changes from failed_precondition to the new stable code session_mode_not_admitted. The refusal is unreleased (not in v0.0.49), so no deployed client branches on the old reason; gRPC code and HTTP status are unchanged. Adding a code is a minor SDK change.
  • mecatui consumes refused_mode_recovery; without it, shift+tab in the approval view does nothing, as before.

Go source: engine adds Session.InterruptCloseOuts and agent.WithChildModeAdmission (both Added, minor); see engine/CHANGELOG.md. Stored data: no schema change.

User-facing documentation: user-docs/mecatui/sessions.md (recover a pending approval), user-docs/mecatui/keybindings.md (shift+tab), user-docs/features/security-and-execution/permissions-and-posture.md, and the ApprovePlan rows in the gRPC and HTTP/SSE references.

Gates

  • task lint: clean except the pre-existing darwin-only gosec finding in environment/microvm (untouched).
  • task docs, task api:check, task generate (no drift): pass.
  • task test:race: pass except tests that also fail on main on the author's macOS machine (socket paths, /private temp symlinks, git worktrees, microVM, kind fixtures). Run with provider keys unset and KUBECONFIG=/dev/null.
  • go run ./cmd/mecademo: tool call, permission ask/approval, and result.
  • Last three commits (error code, unrecognized target, tests): task lint, task docs, task generate, and the SDK typecheck/test/API/docs checks pass. Race tests of internal/adapter/server, internal/app, and cmd/mecatui/{client,ui} pass except eight macOS-only failures that reproduce without these commits. The full task test:race was not rerun for them.

🤖 Generated with Claude Code

@aponcedeleonch

Copy link
Copy Markdown
Member Author

#1935 picked up jhrozek's review fixes, so this needs a rebase on the updated branch. Expect overlap in the mecatui header and /posture code and in internal/app permission-mode files.

Besides the existing commits, this PR should take:

  • Crash-orphan recovery (review thread): when the lease proves a session left Running is orphaned, repair it to Idle without running before stored-mode admission, so SetMode can downgrade after a restart with a lower ceiling. Document it next to the awaiting/authorizing recovery cases.
  • ApprovePlan with UNSPECIFIED approves while the proto says it denies (grpc_approveplan.go, same on main).

aponcedeleonch and others added 11 commits October 26, 2026 12:21
…cted

TestUnavailableModeKeepsComposerUsable accepted any returned command as
proof that enter sent the draft, and TestUnavailableModeDoesNotHoldQueuedPrompt
never ran the drain's command. Both now execute the returned commands and
check the transport: idle sends exactly one Prompt frame carrying the draft,
running stages exactly that draft and sends nothing, and the drain sends one
Prompt frame with the queued follow-up.

Follow-up agreed in the review of #1935.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestPermissionModeTokenDerivesNoTrust only read the trust_project
narration. It now also seeds an AGENTS.md sentinel, runs one turn of a
server-default session through the real Build, and checks whether the
sentinel reaches the provider request. No --permission-mode token feeds
untrusted instructions to the model, --trust-project does, and the
deprecated --posture yolo trust floor still does.

Follow-up agreed in the review of #1935.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Session saves overwrite whole snapshots, and the session lease is the only
thing that keeps two replicas off the same session. A store other
processes may share (a remote driver or a store without its own lease)
with no lease configured got no exclusion and no warning, and a backend
that reports leasing unsupported downgraded with only an INFO line. The
under-lease snapshot adoption in #1935 is a no-op in both setups.

Build now logs a WARN naming the lease flags when it resolves no lease for
any store other than the process-private memstore (a local StoreDir
already gets the flock lease). The run-entry and child-session
lease-unsupported downgrades log at WARN instead of INFO; the staleness
sweep's downgrade stays INFO since it only disables the sweep.

Follow-up agreed in the review of #1935 (the pre-existing concurrency
limitations). The storage-level gaps (no fencing token or aggregate
version on Save) are out of scope here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… checker

A stored top-level session in auto refuses to run after a restart that
leaves no enforcing checker, but a persisted Subagent child did not get
that check. resumedChildMode keeps the narrower of the stored mode and
the current parent's, so a child stored in auto and resumed by a yolo
parent stayed auto while running with advisory or no checking. That is
not an escalation, but it broke the invariant that auto always has an
enforcing checker and mislabelled the run.

The engine gains agent.WithChildModeAdmission, a host-supplied check run
on the child's re-admitted mode inside resolveResumeSession, before the
loaded copy is recovered, forked, or re-persisted. It covers foreground
and background resume and fails inline in both: the parent gets a tool
error naming the mode and the host's reason and suggesting a new
Subagent, the child calls no model and runs no tool, and its stored
session is unchanged. Fresh children are not checked: they inherit auto
only from a parent the host already admitted. A nil check keeps the
previous behaviour for other embedders. Parallel branches and team
members have no resume path.

The server's admission rule moves into server.ModeAdmission, which
admitModeFor wraps in ErrInvalidArgument with unchanged wording. Build
computes the enforcing-checker state once and feeds the same values to
server.Config and to both Subagent tool builders.

Wire assessment: no new shapes or event types. A Subagent resume now
returns an error tool result only when the child's mode is one the
server already refuses for stored sessions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed-mode recovery

The refused-mode recovery (SetMode to an admitted, lower mode on a stored
session parked awaiting an approval) cancelled the pending turn in the
snapshot only. The durable event log still ended on the permission.ask,
so WatchSessionEvents, StreamSessionLive, and any pending-approval
discovery kept seeing an open ask, and eventsource.Fold had nothing to
reconcile the cancelled snapshot with.

After the recovery save succeeds, SetMode now appends and publishes, all
stamped with the parked run's id, the same shape a live cancellation
produces: permission.retract for the ask, one cancellation tool result
per unanswered call, then result{cancelled}. The tool results carry the
exact close-out the next run's Interrupt writes into the conversation,
so the log stays foldable (an EvResult alone after a dangling call fails
ValidateToolPairing). The facts are captured before Session.Cancel
clears them. An append failure is a WARN through the injected
diagnostics and SetMode still succeeds, as for manual compaction,
because the snapshot is already committed.

engine/session gains Session.InterruptCloseOuts, a read-only preview of
the results Interrupt appends, so the host does not duplicate the
close-out text or the pairing scan. API snapshot and engine CHANGELOG
updated (Added, minor).

Wire assessment: additive. The recovery and its refused_mode_recovery
feature are unreleased; the change only appends existing event types
(permission.retract, tool.result, result) with the existing run-id
stamping on that feature-advertised path. Existing clients already
handle all three for a run; no new event type, no field change, no
major bump. The feature and SetMode proto comments describe the events.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A chat resumed with --resume while parked on an approval in a mode the
server no longer admits had no way out in the TUI. Approving failed with
FailedPrecondition, which the client folded into a generic "changed or
could not be acknowledged" fatal, and shift+tab never reached the
server's refused-mode recovery because the approval modal swallows every
key and the mode cycle is gated to idle/running.

- client: read the refused_mode_recovery feature into
  Capabilities.RefusedModeRecovery. SetMode returns a ModeResult with
  the authoritative mode and session state, carried on ModeChangedMsg;
  adapters and fakes follow. A FailedPrecondition control refusal keeps
  the server's reason behind PendingApprovalFailureDetail (other
  categories stay masked).
- ui: while a recovered approval is shown and no verdict is in flight,
  ModeSwitch (ahead of the approval surface) sends SetMode to the next
  lower mode in the cycle within the server posture, never arming the
  deferred pendingMode. Verdict keys and button clicks are blocked while
  it is in flight. Success retires the recovery, marks the gated call
  interrupted, closes the modal, and returns to idle with the draft
  kept. A refusal is final: the modal stays with the server's reason in
  the footer, and the next press tries a lower mode.
- The watch treats the server's close-out (retract, cancellation tool
  result, cancelled terminal) as the expected outcome of the user's own
  recovery in either arrival order; a retract the user did not cause is
  still fatal.
- A FailedPrecondition verdict refusal now names the server's reason.
  With the feature, the modal reopens with verdicts blocked so shift+tab
  can recover; without it, recovery ends with that reason. No verdict is
  resent.
- The recovery footer hints up front when the stored mode is above the
  server posture, and the help lists the new shift+tab meaning.

User docs describe the shortcut in the mecatui sessions guide and link
it from the permissions page, which also notes what watchers see.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The proto documents PLAN and UNSPECIFIED target_mode as a deny, and the
HTTP plan:approve path already treats an empty target that way. The gRPC
handler mapped target_mode through modeFromProto, which turns UNSPECIFIED
into default, so an omitted target approved the plan and started the
continuation run. Map it through planModeFromProto instead, which keeps
UNSPECIFIED as plan.

Wire assessment: a bug fix that restores the documented contract. A
conforming client already expects UNSPECIFIED to deny; the in-repo
clients (TypeScript SDK, mecatui) always send an explicit target.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed mode

A session left running by a crash under a mode the restarted server no
longer admits had no prompt recovery. StartRun's admission refuses before
its crash-orphan repair runs, the aggregate refuses SetMode from running,
CancelRun has no live run to cancel, and the background stale sweep only
applies after its staleness threshold, if it is configured at all.

SetMode now recovers it the way it recovers a refused awaiting session.
Holding runEntryMu and the mutation lease with no registered run is the
same proof StartRun's repair relies on that the running snapshot is
orphaned, so abandonRefusedRunning abandons the dead turn to idle, closing
out its unanswered tool calls, without running anything, and the mode
change lands in the same single save. A peer that still holds the lease
keeps the session refused. A running session whose mode is admitted is
left for StartRun's ordinary repair. The admission refusal for a running
session now names this recovery, next to the awaiting and authorizing
cases.

Wire assessment: additive. It extends the unreleased refused_mode_recovery
feature; SetMode only goes from InvalidArgument to success for a session
that has no recovery today, and no shapes or existing success paths
change. The feature, proto, and API-surface docs describe it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
admitStoredMode refused with a bare ErrFailedPrecondition, so mecatui
could only tell the refusal apart by its gRPC code. Every
FailedPrecondition from ResolveRunAsk (an ask another client already
resolved, a plan resolution required, a stale placement) was treated as
the refused-mode case: the user saw "nothing ran" with the server's text
and was offered a mode switch that could not recover it.

The refusal now wraps a new ErrStoredModeNotAdmitted sentinel with the
registry code session_mode_not_admitted (gRPC FailedPrecondition, HTTP
412, unchanged). mecatui maps only that ErrorInfo reason to a new closed
PendingApprovalModeNotAdmitted category, which alone keeps the server's
reason and opens the shift+tab recovery. Any other FailedPrecondition
goes back to the closed not-recoverable category with no server text.
The TypeScript SDK error-code list gains the code.

Wire assessment: additive. The stored-mode refusal is unreleased (not in
v0.0.49), so no deployed client branches on its previous
failed_precondition reason. Status codes and HTTP status are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
planModeFromProto only special-cased UNSPECIFIED, so an out-of-range
target_mode (proto3 enums are open) fell through modeFromProto's
default branch to ModeDefault and approved the plan. The HTTP path maps
an unknown target to plan. Map only the named modes through and send
everything else to ModePlan, so a malformed target never approves.

Wire assessment: a bug fix that brings gRPC in line with HTTP and the
documented deny-by-default mapping; the proto comment now names
unrecognized values. In-repo clients always send a defined value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Run the resumed-auto-subagent refusal for a no-FS parent as well as a
filesystem one, so dropping WithChildModeAdmission from the no-FS
Subagent builder fails a test. Pin that the mecatui client's SetMode
returns the server's resulting state over a real gRPC connection, which
the refused-mode recovery relies on to show the cancelled turn.

Document the resumed-approval meaning of shift+tab in the keybindings
reference.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aponcedeleonch

Copy link
Copy Markdown
Member Author

Both follow-ups from the note above are in, and the branch is rebased on main.

  • Crash-orphan recovery: 49e18aa. Once the lease proves no server drives the run, a switch to an admitted mode abandons the interrupted turn without running it. It's documented next to the awaiting/authorizing cases in docs/architecture/api-surface.md, the SetMode proto comment, and permissions-and-posture.md.
  • ApprovePlan with UNSPECIFIED now denies, matching the proto: 0b64e32. e2832eb extends that to an unrecognized target mode.

This branch was successfully deployed

1 active deployment
Preview — 7f8b496b Deployed Oct 26, 2026 by vercel[bot]
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