Skip to content

fix: apply a thread's permissions pick right away - #70

Merged
chrisohearn merged 5 commits into
integration/mcp-events-20261109from
fix/web-apply-runtime-mode-now
Nov 12, 2026
Merged

chrisohearn merged 5 commits into
integration/mcp-events-20261109from
fix/web-apply-runtime-mode-now

Conversation

@chrisohearn

@chrisohearn chrisohearn commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Changing Permissions on an existing server thread only wrote the composer draft. thread.runtime-mode.set went out only with the next message, so an idle thread (for example the Jarvis inbox, where any message wakes another agent) could not change mode without posting into it. Meanwhile the UI showed a mode the database did not have.

Fix

Web (useComposerSelection, useApplyThreadRuntimeMode, runtimeModePickStore)

  • Idle server thread: a pick dispatches thread.runtime-mode.set right away. The pick is held in an unsaved per-thread store and shows until the thread shell has that exact mode. It is also released after a timeout or when the composer unmounts. Releasing on the command reply flashed the old mode for about 66 ms.
  • Pick identity: each pick object is its own token, so a reply to an older pick never takes over a newer one.
  • Refusal: shows "Could not change permissions" with the server's reason, and the control returns to the server's mode.
  • Busy: a pick is refused in the client while a send or run is under way (isWorking, or either runtime source shows preparing, starting, running or waiting).
  • Server threads ignore a saved draft's mode. The control shows the pending pick or the server's mode. Sends no longer set the runtime mode, because a pick applies itself.
  • Local draft threads: unchanged. The mode still applies when the thread is created.

Server (Orchestrator.ts)

  • An owner setting the thread's current mode is a no-op: no event, no detach.
  • An owner change is refused while a run is active, if it would detach a live session. Claude cannot switch mode in a live session, so applying it would end the run. This protects every client, not just this one. The MCP lower-only rules are unchanged.

Scope

Tests

  • ChatView.runtimeMode.browser.test.tsx (9 cases):
    • an idle pick reaches the server before any message, and holds until the shell shows it
    • a refusal shows the reason and resets the control
    • returning to the server's mode while a pick is pending sends it, and the old refusal is ignored
    • a send while a pick is pending sends no second set
    • a mode in an older saved draft neither shows nor is sent
    • a pick is refused in each of preparing, starting, running and waiting
  • runtimeLayer.test.ts: an owner's set to the current mode commits no events, and an owner's set during a run with a session that cannot switch in place is refused while the session stays running.
  • The new cases fail on the previous head or on base. vp run check:changed passes.

Live check

Ran a dev server on an isolated worktree database with a Replay (dev) thread, then picked a mode in the composer:

  • The DB runtime_mode changed from full-access to approval-required and back, with no message posted.
  • The control's label changed once, with no intermediate value.

🤖 Generated with Claude Code (Claude Opus 5.5)

rallymewrx and others added 2 commits November 12, 2026 13:51
Changing Permissions on an existing server thread only wrote the composer
draft. The server kept the old runtime mode until the next message was sent,
so an idle thread could not change mode without posting into it.

An idle server thread now dispatches thread.runtime-mode.set on pick. The
draft clears once the command settles, so the control shows the server's
value. A refusal shows the server's reason. While a run is active, the pick
is refused, since applying it would end a live Claude session. Local draft
threads keep applying the mode when they are created.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Waits for the thread shell to show the new mode before releasing the
composer draft, so the control no longer flashes the old mode. Also
treats waiting runs as busy, falls back to the shell's runtime before the
thread detail loads, and sends a return to the server's mode while an
earlier pick is still in flight.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-12T21:05:49.945890Z
We couldn’t complete this review. Comment @codex review to try again.
1d96561 PR opened
ℹ️ 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" or "@codex security review".

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

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

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

Copy link
Copy Markdown
Owner Author

Substitute review (Claude Opus 5.5), done in place of the hosted Codex reviews, which are blocked by a usage limit. Reviewed 1d96561; efdb4a5 is an empty commit. Verdict: merge after fixes: M1 (the client guard) and M2.

The main change is sound. All 4 browser tests pass at the head and fail against the base hook.

M1. A run can start between the client's idle check and the server applying the change (useComposerSelection.ts:162,168).

  • Scenario: Send, then pick a mode before "preparing" reaches the client. Per-thread commands are serialised (threadCommands.ts:78-81), so the set lands just after the turn starts.
  • Why it matters: for the owner, the server refuses only when a session has background work (Orchestrator.ts:4308). Claude's check (ClaudeAdapterV2.ts:13024) ignores the turn in progress, so the set is accepted and detaches the live Claude session, which ends the run. Jarvis, MCP or mobile starting a run in that window triggers the same thing.
  • Client fix (before merge): also refuse while isSendBusy/isWorking (ChatView.tsx:1418,1470), and when either runtime source shows activity.
  • Server fix (follow-up is fine): for owner sets, refuse when a run is active and a session would be detached.

M2. A pending pick is saved in the composer draft, but only in-memory code releases it (:111-132; composerDraftStore.ts:2230,2357).

  • Scenario: reload mid-flight, or a draft saved by an older build. The draft is never cleared, the control shows the draft's mode, and the menu ignores re-picking the shown value (ComposerFooterControls.tsx:124).
  • Effect: the next send, including a queued send during a run, re-dispatches it (ChatView.tsx:2078 via useComposerSend.ts:1197). That can change the mode mid-run, or undo a lower mode set by MCP, Jarvis or mobile.
  • Fix:
    • Hold pending picks in a per-thread map that isn't saved, with a sequence token.
    • Drop runtimeMode from saved drafts of server threads, by migration or on load.
    • Skip the runtime-mode set in persistThreadSettingsForNextTurn during a run.

Low:

  • L1. An owner set to the thread's current mode still emits an event and detaches an idle Claude session (Orchestrator.ts:3894-3945, :4286-4298, :4868). The PR triggers this after a refusal revert, on Send while a pick is pending, and when clearing a stuck draft. Server fix: return early when the mode is unchanged.
  • L2. A newer pick is matched by value, not by token (:113). With A, then FA, then A, the first refusal clears the third pick. Use a per-thread counter.
  • L3. "Caught up" means any change from the old mode (:122), so a revert can briefly show A. If the shell never changes, the subscription at :128 is never removed.
  • L4. The local ACTIVE_RUN_STATUSES leaves out "queued" (models.ts:120). Probably fine; add a comment saying why.
  • L5. The refusal also applies to Codex, which can switch mode in a live session without ending it. Product call.

Other paths that change the mode mid-run:

  • The web shortcut and the policy banner go through the new handler, so they're covered. What's left on the web is M1, and M2 on a queued send.
  • Mobile still writes the draft and dispatches on delivery, including queued messages (ThreadSettingsSheet.tsx:1427, threadOutboxDelivery.ts:812-826). Out of scope here; worth a follow-up.

Security: nothing is widened. The client check, server authorisation, the policy ceiling (:15364) and the MCP lower-only rule are unchanged. Note: raising an agent-woken thread such as the Jarvis inbox now applies with no message, so a raise confirmation might be worth adding.

Test gaps:

  • Only "running" is covered, not preparing, starting or waiting.
  • Nothing sends while a pick is pending.
  • The revert test only asserts which commands were dispatched.
  • Nothing covers a reload with a saved draft.

🤖 Generated with Claude Code

Addresses the substitute review on #70.

Web:
- Pending picks live in an unsaved per-thread store. Each pick object is
  its own token. A pick releases when the thread shell shows its exact
  mode, after a timeout, or when the composer unmounts.
- Server threads show the pending pick or the server's mode, and ignore a
  mode saved in an older draft.
- Sends no longer set the runtime mode, because a pick applies itself.
- The pick is refused while a send or run is under way, from either
  runtime source, including waiting.

Server:
- An owner setting the thread's current mode is a no-op: no event, no
  detach.
- An owner change that would detach a live session is refused while a
  run is active.

Mobile follow-up: #72.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chrisohearn chrisohearn changed the title fix(web): apply a thread's permissions pick right away fix: apply a thread's permissions pick right away Nov 12, 2026
@chrisohearn

Copy link
Copy Markdown
Owner Author

Substitute re-review (Claude Opus 5.5) of the fixes 1d96561..918d0cc. Verdict: merge after fixing N1.

Earlier findings: M1 (client and server), M2, L1, L2, L3 and L4 are fixed. L5 stays as a product call; the client comment says "Claude", but the client refusal applies to every provider. Tests: 9 browser tests, 182 runtimeLayer tests and 184 related server tests pass. Both halves of the new server test fail without their fix.

N1 (Medium, blocking): a mobile message sent mid-run with a different mode gets stuck permanently.

  • Where: the mid-run refusal (Orchestrator.ts:4313-4320) goes through the receipt catch (:15222-15258), which records a rejected receipt.
  • Why it sticks: mobile resends the setting under the same command id, ${message.commandId}:runtime-mode (threadOutboxDelivery.ts:202, :812-827), and retries on failure. The server checks the receipt first (:15175), so every retry gets OrchestratorCommandPreviouslyRejectedError, even after the run ends. The outbox delivers one message at a time, so the message and everything queued behind it never go out.
  • Fix (server only, no contract change): skip commitRejectedCommand for this refusal, as the catch already does for ceiling refusals. Keep OrchestratorDispatchError with an exported reason constant. Consider doing the same for the background-work refusal at :4326.
  • Test: add a runtimeLayer case: refused mid-run, the run ends, then the same command id succeeds.

N2 (info): the nightly desktop app still sends the mode with its next message, using a random command id. If its draft mode differs from the thread's, sends during a Claude run now fail with the server's reason and restore the composer text. Before this PR, the send ended the run.

N3 (low): runtimeLayer.test.ts:3089 covers two behaviours in one case; split it.

Confirmed correct:

🤖 Generated with Claude Code

Addresses the substitute re-review on #70.

- The mid-run and background-work refusals no longer record a rejected
  receipt. Mobile retries under the same command id, and a recorded
  rejection blocked its outbox forever.
- The owner mode tests are split in two. A retry with the same command id
  succeeds after the run ends.
- Comments now tie the zero-event allowlist entry to the early return,
  and say the client refusal applies to every provider.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chrisohearn
chrisohearn merged commit 4fe4fc9 into integration/mcp-events-20261109 Nov 12, 2026
19 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.

2 participants