Repository navigation
feat(permissions): follow-ups from the permission-mode review - #2425
Open
aponcedeleonch wants to merge 11 commits into
Open
aponcedeleonch wants to merge 11 commits into
aponcedeleonch wants to merge 11 commits into
Conversation
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 Besides the existing commits, this PR should take:
|
aponcedeleonch
force-pushed
the
impl/permission-mode-followups
branch
from
October 21, 2026 08:53
2e76739 to
b247951
Compare
aponcedeleonch
force-pushed
the
impl/permission-mode-followups
branch
from
October 21, 2026 11:29
1c9a6b3 to
bc0d397
Compare
…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
force-pushed
the
impl/permission-mode-followups
branch
from
October 26, 2026 10:26
bc0d397 to
7f8b496
Compare
Member
Author
|
Both follow-ups from the note above are in, and the branch is rebased on main.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage: Implementation. Implements the follow-ups agreed in the #1935 review (now merged).
What it does
Refused-mode recovery reachable from mecatui. When
mecatui --resumeshows 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'srefused_mode_recoverycancels 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 therefused_mode_recoveryfeature.Refused-mode recovery observable to watchers. After the recovery save,
SetModeappends and publishes the same shape a live cancellation produces, stamped with the parked run's id:permission.retractfor the ask, one cancellationtool.resultper unanswered call, thenresult{cancelled}. Watchers and live subscribers no longer see an ask that stays open forever, andeventsource.Foldagrees with the snapshot. The close-out text comes from the newSession.InterruptCloseOuts, the same text the next run'sInterruptwrites.A resumed auto subagent needs an enforcing checker. A persisted child stored in
autoand resumed under ayoloparent keptautobut 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 (newagent.WithChildModeAdmission, wired to the sameserver.ModeAdmissionthe 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_modeas 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.
admitStoredModenow refuses withsession_mode_not_admitted(still FailedPrecondition / HTTP 412) instead of the genericfailed_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.
TestPermissionModeTokenDerivesNoTrustseeds anAGENTS.mdsentinel 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'sSetModestate mapping is tested over real gRPC.Server CLI reference.
user-docs/reference/server-cli.mdis regenerated; it had drifted from the--permission-modehelp text changed in feat(permissions): one named permission-mode vocabulary and two admission gates #1935.API compatibility
Assessment: additive. No API major change.
refused_mode_recoveryfeature 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 andSetModeproto comments describe the events.resumereturns an error tool result only when the child's mode is one the server already refuses for stored sessions. No shapes change.refused_mode_recoveryfeature. No shapes change; the feature, protoSetModecomment, and API-surface doc describe it.ApprovePlanwith UNSPECIFIED or an unrecognizedtarget_modenow 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.failed_preconditionto the new stable codesession_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.refused_mode_recovery; without it, shift+tab in the approval view does nothing, as before.Go source: engine adds
Session.InterruptCloseOutsandagent.WithChildModeAdmission(both Added, minor); seeengine/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 inenvironment/microvm(untouched).task docs,task api:check,task generate(no drift): pass.task test:race: pass except tests that also fail onmainon the author's macOS machine (socket paths,/privatetemp symlinks, git worktrees, microVM, kind fixtures). Run with provider keys unset andKUBECONFIG=/dev/null.go run ./cmd/mecademo: tool call, permission ask/approval, and result.task lint,task docs,task generate, and the SDK typecheck/test/API/docs checks pass. Race tests ofinternal/adapter/server,internal/app, andcmd/mecatui/{client,ui}pass except eight macOS-only failures that reproduce without these commits. The fulltask test:racewas not rerun for them.🤖 Generated with Claude Code