Skip to content

Preserve saved account credentials after enrollment errors - #25059

Merged
nwparker merged 30 commits into
mainfrom
nwparker/opencode-account-enrollment-commit-recovery
Oct 4, 2026
Merged

nwparker merged 30 commits into
mainfrom
nwparker/opencode-account-enrollment-commit-recovery

Conversation

@nwparker

@nwparker nwparker commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 80 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10752 $\color{#cf222e}{\Huge{\mathbf{−}}}$​100 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10652
Prod 116 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1988 $\color{#cf222e}{\Huge{\mathbf{−}}}$​539 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1449

ELI5

A saved OpenCode or Devin account could lose its credentials if enrollment reported an error after saving. Keep those credentials, and keep the account selected correctly when its UUID is entered with different casing.

What Changed

Cleanup reads persisted registration before deleting the new profile. A registered account or unreadable metadata preserves the directory; a confirmed unregistered account is still cleaned up. Account lookup uses the existing lowercase directory and selection saves the registered UUID spelling, preserving the active marker and putting its transcripts first. One failing listener no longer rejects a completed enrollment or blocks the other listeners.

The credential-symlink test skips Windows because file symlink creation requires extra privileges; production rejection stays intact.

Why

A write error does not prove that registration failed. Reading the saved state prevents cleanup from deleting committed credentials. Returning the matched account ID from the existing lookup also keeps readers consistent without adding casing rules to each reader.

Linked Issue

Follow-up to #24636 and findings on #25031. Partially addresses #18561, reported by @afonsoft; broader provider and host validation remains open.

Visual Proof

N/A — no renderer changes. The account-list active marker and transcript order are verified against the real CLI formatter in the disposable-profile checks.

Testing

  • Tested locally with installed OpenCode 2.0.16
  • Regression tests added

The exact main-based branch passes 71 tests across three files, all three package typechecks, full-file lint and formatting, all six changed-code checks, SAFETY and the complete configured anti-slop scan. All other current-main source entries remain exact.

Actual before/after checks capture a dummy credential with OpenCode, inject errors after the real secure writer publishes, then read credentials through the selected environment. Separate two-profile checks reproduce the missing active marker and wrong transcript order on the previous repair and pass after this change, for both opposite-case selections. Twelve actual auth-list reads pass across those phases; all 26 test processes and 12 private endpoints close. The tested 33-module service and unchanged formatter exactly match this main-based branch.

Review

Independent source and evidence review passes. Earlier enrollment and directory-casing proofs remain preserved. Metadata access errors are simulated controls, not an actual OS ACL failure. The macOS volume aliases directory casing, so no case-sensitive filesystem failure is claimed. No paid provider requests, app build or rendered-app equivalence is claimed.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer material copied.

Notes

The execution host owns credentials and metadata. No RPC, status-store or provider protocol changes. Actual execution was macOS; Windows, WSL and SSH were considered but not newly exercised. Corrupt metadata must recover before launch.

Checklist

  • Small and focused
  • Before/after, mechanism and reason explained
  • Visual-proof N/A explained
  • Self-reviewed and independently reviewed
  • Cross-platform and SSH impact considered
  • Relevant local checks pass; fresh CI covers repository checks

nwparker and others added 4 commits October 3, 2026 12:29
…24901)

* refactor(orchestration): send every agent-to-agent message through one sendAgentTurn

The structured mail-pointer lane and the structured worker preamble each carried a copy of
"send, wait out a pending start, read the verdict", and four dispatch-preamble sites typed into
a terminal directly. They now share sendAgentTurn, built from host.send, the host's settlement
waiter and sendTerminalAgentPrompt. Every caller passes delivery 'now', so nothing sends
differently; a source-scan ratchet keeps new direct sends out.

* test(orchestration): the refusal fixture uses a real wire refusal code

* fix(orchestration): derive the send fingerprint inside sendAgentTurn and pair target with turn

A `queue` send was refused by a real host: callers supplied a fingerprint over the body alone
while the host digests body and delivery. sendAgentTurn now builds the envelope with the
composer's own builder (extracted to structured-agent-session-send-mutation), so the fingerprint
is always over exactly the fields sent; `now` sends keep the identical digest. The pointer lane
no longer carries a fingerprint it cannot get right.

sendAgentTurn takes one argument, a union on kind that carries its own target and turn, resolved
by an exhaustive switch; the terminal turn names its purpose (only the dispatch preamble today)
instead of every terminal send inheriting the task lead line. A queued outcome keeps the host's
draft receipt, and both structured callers read outcomes exhaustively with unchanged behaviour.

The boundary test now also fences direct structured host sends, catches optional, bracket and
bound member uses, has a planted-offender self-test, and pins the unmoved terminal mail pointer
and agent-teams tmux senders.

* fix(orchestration): keep federation.ts under max-lines and satisfy prefer-template in the send ratchet

* fix(orchestration): wait on the answered submission's id when a replayed queue turn was already handed off

* test(orchestration): pin that a replayed queue turn waits for its hand-off to settle

* test(orchestration): fence call-result and cast host sends, raw terminal writes and the launch-prompt helpers
…eads as waiting (#22634)

* feat(native-chat): a Claude subagent waiting on a permission prompt reads as waiting

A subagent's permission request reaches the parent session's callback naming the
subagent that asked (agent_id) and the tool call it gates (tool_use_id). The
pending request is recorded with the asking agent. On every drain the child-work
producer re-derives which children a pending request blocks and hands that set
to the Claude child decoder, the one owner of each child's live edges: a blocked
child reads waiting on every live edge it reports, and a child that starts or
stops waiting is a live edge of its own. Answering, denying or cancelling the
request returns the child to its prior live state; nothing is stored beyond the
pending requests.

A live task_updated carrying an error now reaches the record as the child's last
message, without an ending or a new state.

The replay test drives a scrubbed capture of the real CLI (foreground allow,
deny, interrupt, background allow, and the main agent's own request) through the
real adapter into the host's child records.

* test(native-chat): a subagent's request names it before its tool call is read

* docs(agent-status): a subagent asking for approval waits in every lane; the parent row keeps the session's own attention

* test(native-chat): hand canUseTool the asking agent without widening the helper's cast

* test(native-chat): an interrupted Claude subagent settles cancelled, not failed

A captured interrupt shows the spawn call's error result ("The user doesn't want
to proceed…") arriving before the subagent's own `task_updated {status: killed}`.
The spawn result ends nothing (the child ends only on its own terminal frame), so
the child stays live until its `killed` status settles it cancelled. A genuine
failure, captured with the subagent on a model that does not exist, sends its
`failed` status before the error result and still ends failed. Both captures now
replay through the real adapter into the host's records.

* fix(native-chat): a Claude subagent's prompt makes the parent row wait, not block

A subagent's pending prompt made the whole session `attention`, which reads as
the main agent's own `blocked` and outranks the fold's waiting arm, so the
parent row read blocked where a CLI Claude parent reads waiting. The main
agent's state now reads only its own pending prompts.

- A Claude prompt row carries the linkage of the agent that raised it: the one
  the permission request names, or the owner of the tool call it gates. The
  same join decides which child reads waiting, so the two cannot disagree.
- The status summary projects the session's own status from root prompts only;
  every other reader (delivery gates, teardown, restart) still asks whether
  anyone is waiting on a human.
- An answer keeps the prompt row's linkage by the journal's own rule: a
  revision that names no producer keeps the row's existing one.
- The child-tool queries gain the prompt's producer, so a prompt row and a
  child record answer "which agent" from the same join.

* test(native-chat): say which ids the permission capture scrubs and which are its own

* fix(native-chat): the status clock dates attention by the session's own asks only

The session's status is now `attention` only for its own pending prompt, so the
clock's fallback to a subagent's ask could no longer be reached, and it read the
journal by a different rule than the status it dates. Both now read root prompts.

The journal also stamps a Codex subagent's prompt with its thread (#22532), so a
Codex child's approval is that child's wait in the Codex lane too. Two tests
written for the earlier rule are updated: a subagent's ask leaves a running
session `working` on its turn's clock, and a Codex child's answered approval
leaves the settled parent's Activity row done with nothing unread.

* fix(native-chat): a completion still says the user is asked when a subagent asks

The turn-completion feed marked a completion `awaitingUser` from the status
summary's `attention`. The status now means the session's own agent is waiting,
so a subagent's pending approval stopped reaching the completion. The projection
now also says whether anyone is waiting on the user, as the delivery gates,
teardown and restart ask it, and the completion reads that.

The waiting-subagent replay answers its prompt with the adapter's current
response shape.

* revert(native-chat): a live Claude task's error stays out of the child's last message

No capture shows a live task_updated carrying an error, and it is unrelated to
a subagent waiting on a permission request; it leaves this PR.

* test(native-chat): settle the Claude session's startup before replaying a subagent's request

A startup frame drained child work during the first await, so answering a
request freed the child even with the answer's own republish removed.

* fix(native-chat): a Claude subagent's prompt row names it as its other rows do

The prompt row stamped only the asking agent's id, so a nested subagent's
request lost the agent that spawned it, its spawn call and its run. It now takes
the linkage the asker's own rows take: the gated tool call's, when that names the
same agent, else the one resolved through the agent's spawn call. The provider's
agent id stays the asker's id.

* fix(native-chat): a subagent's request makes the parent row wait without a child record

The parent row learned that a subagent needed the user only from that
subagent's child record, so a request no record carried (a Codex child the host
never registered, a Claude task past the live cap) left the row working or done
while the approval card sat in the chat.

"Someone in this session must answer" is now one derived session fact. The
projection names two facts instead of a mode flag: the main agent's own status
(attention only for its own request) and structuredAgentSessionAwaitsUser (any
pending prompt). The status summary publishes the second as an optional
awaitsUser, and the shared fold reads it: the main agent's own ask is blocked,
otherwise awaitsUser or a waiting child record makes the row wait. Every caller
picks the fact it means: the completion edge's awaitingUser and the delivery
gate read awaitsUser; the quit snapshot folds the same two inputs the sidebar
does.

* fix(native-chat): a client that predates awaitsUser still reads a subagent's request as attention

A status summary's status is now the main agent's own, so a client built before
the split would read a subagent's request as working (or idle) and fold it with
code that has no awaitsUser input. Clients advertise
agent-session.status-awaits-user.v1; at agentSession.subscribeStatus the host
sends any client that does not the pre-split summary: attention whenever
awaitsUser is set, without the main agent's own tool line, verdict and clock.
The feed and every in-process reader keep the canonical summary. Transitional,
like the turn-item downgrade.

* test(native-chat): a Codex subagent's approval makes its settled parent's Activity row wait

The test pinned the parent row done while a Codex child asked, through a harness
that fed no child records, so it proved nothing about the ask. It now drives the
ask twice through the real host status store: with no child record (the
session's awaitsUser alone) and with the child's own record waiting from
thread/status/changed. Both read waiting with needsAttention while the ask is
open, then done with nothing unread.

* docs(agent-status): a subagent's request reaches the parent row through awaitsUser in every structured lane

The store reference said a Codex child's request still read as the main agent's
blocked and that only the Codex hook lane fed a waiting child. Both structured
lanes stamp the asking child and feed child records, and awaitsUser carries the
request when no record does. The liveness comment goes back to main's: a child's
blocked is a failed task on an older host's legacy rows.

* fix(native-chat): the restart dialog still headlines a subagent's pending approval

The quit snapshot now records the main agent's own state, so a subagent asking
while the main agent worked recorded `working` and the dialog said "Was
mid-reply" where it used to say "Waiting for your approval". The headline now
comes from the snapshot's pending prompt, whoever raised it, with the existing
copy; `state` stays the main agent's own.

* test(orchestration): a subagent's pending approval holds structured mail delivery

Scoping the delivery gate to the main agent's own request left every gate test
green; a subagent's request now has its own case.

* fix(native-chat): a subagent's request is dated by when it was raised, on every client

Since the summary's clock became the main agent's own, nothing dated a wait
that only a subagent's request held: a pre-split client was sent attention with
no clock, where the old host dated it by the subagent's prompt, and a new
client's waiting row fell back to the time it first saw it, so after a reload a
request the user had already read could read unread again.

The session fact is now when someone started being asked: awaitsUserSince, the
oldest pending prompt whoever raised it, and its presence is what awaitsUser
meant. A row waiting on someone else's request takes that as its clock; the
downgrade for a client without the capability dates its attention by it, which
is what the old host published. A cross-version test pinned to the last
pre-split release runs the same journals through that release's projection and
through this one plus the downgrade, and compares the whole summary. The Codex
end-to-end test also reads the host's own status row, and keeps a read ask read
through a later row and a reload.

* test(native-chat): the pre-split parity check compares only the fields the split owns

An additive summary field is safe for old clients, so comparing whole summaries
against the pinned release would redden on one. The wire comment now says how
the downgrade dates attention: the main agent's own oldest ask, else
awaitsUserSince.

* test(runtime): an aged host-held working summary states that nobody is asked

The test built its working summary by overriding the status of a published
approval summary, which still carried awaitsUserSince, so the row correctly
read waiting. It now drops the request as its scenario says.

* fix(native-chat): the chat's subagent block says waiting when the strip does

While a Claude subagent's request was open, the sidebar and the composer strip
read waiting but the subagent block in the chat history a few pixels above
still read "Kicked off 1 subagent working": it shows the journal's roster
state, and the journal records no wait.

The structured chat now hands its transcript the subagents the strip shows
waiting, read from the host's child records through the strip's own row model
and matched by the provider id the roster names each one by. A running entry
the host says is waiting reads waiting in the group row, its entry and its
section head, with the strip's word and the question colour; it reads the
journal's state again as soon as the host stops reporting the wait.

* fix(native-chat): a collapsed subagent group shows a wait beside a failed sibling

A failed sibling took the group row's one alert slot, so a group with a waiting,
a working and a failed child read "1 working +1 failed" and hid the wait; it
now reads "1 working +1 waiting +1 failed". The waiting set keeps its identity
while a child frame changes no wait, so the transcript's subagent rows do not
re-render on every frame, and the test of a wait ending now updates one mounted
row instead of remounting it.

* refactor(claude): one needs-input state on the parent; the asking subagent alone reads waiting

Drop the split of the main agent's own status from a session-wide "someone must
answer" fact: awaitsUserSince, the agent-session.status-awaits-user.v1
capability and its old-client downgrade, and every reader change that only
consumed them (fold, equality, ingest, delivery gate, turn-completion feed,
quit snapshot, resume headline, status clock, status bridge, attention
dispatch) go back to main. The parent row again reads one needs-input state
for a pending request whoever asked, dated as before.

Kept: a request's owner recorded once on its prompt row with full producer
linkage; the asking subagent's own record reads waiting, re-derived on every
update; the chat history's subagent block reads that same state; an answered
subagent request stays in its subagent's group.

A subagent now waits only on a request the user can still answer (its card
open, no answer underway), and the adapter frees it before the host records an
answer or dismissal. So a waiting child record always sits beside the pending
card, and main's fold never reads the parent as waiting on it: no window after
an answer, and no ~3 s wait after a card dismissed by Stop.

* fix(claude): a subagent waits only beside its committed card

A subagent's wait was pushed to the host as soon as its request arrived,
while the request's card row reached the journal at least a microtask later.
So every subagent request published the parent row as waiting before
blocked (the main agent's own fold reads a waiting child that way), and
Activity got an extra unread "waiting" event that main never shows.

The card is now the one record of an open request. The translator records
the asker on the card once (its row's linkage) and counts the card open only
after the sink confirms its rows landed, then publishes the wait; anything
that closes the card (an answer underway, a dismissal handed to the host,
Claude's own withdrawal, the session's end) frees the subagent first. So
every publish that shows a subagent waiting also shows its pending card, and
the parent reads one needs-input state, exactly as on main.

This retires the registry's view of pending requests (unclaimed(), the
asking-child join) and the translator's holdsOpen. The prompt row's linkage
takes one rule: the agent the provider names, else the gated call's owner.

The parent-row proof now runs through the real deferred sink, durable
journal and status feed, publishing as production does, and checks at every
publish that waiting subagents have pending cards and that the parent row
matches a host fed no waits.

* fix(native-chat): a closed sink's dropped writes never read as landed

The sink's written() resolved ok when the sink was closed with writes still
queued, so a subagent's prompt card could count as open with no row in the
journal. written() now reports a close that dropped writes admitted so far
as not landed; drained() and lifecycleBarrier() keep reading a closed sink as
settled.

Tests: a card never opens when its sink closes first; a card Claude withdraws
while the sink holds the cancelled row back closes at once; two subagents
asking at once, and the main agent asking beside a subagent, keep the parent
row as before with each waiting subagent beside its own card; a process that
dies mid-request leaves no subagent waiting.

* fix(claude): a withdrawn subagent request frees its child before its card closes

Main's sink now hands each write to the journal as it is submitted, and an idle journal commits it
and runs the publication at once. Claude's own withdrawal of a subagent's request therefore closed
the card and published the parent row before the child's wait was freed, so one publish showed the
subagent waiting beside no pending card (fg-interrupt replay). The child's wait now also requires the
request to still be open in the registry, and a withdrawal republishes child work before the
journal takes the close.

* refactor(claude): trim subagent request waiting to the common pattern and its essential tests

The chat history no longer marks a subagent block as waiting: the approval card itself carries the
request, and the asking subagent's row in the sidebar and composer strip reads waiting, as before.
NativeChatWaitingSubagentsProvider, native-chat-waiting-subagents.ts and their renderer changes go.

A subagent waits while its request is still open and unanswered in the prompt registry and its card
has landed in the journal. The registry check also covers a withdrawal under backpressure, so the
card list no longer filters pending cancellations itself.

Tests: one integration file replays the captured CLI frames through the real adapter, sink, journal
and status feed (renamed claude-subagent-permission-request.test.ts), with the asking subagent's
state timeline, attribution, nested linkage and a card write that waits for the journal. The
producer-harness waiting test, the redundant prompt-card cases, the harness reducer swap and three
unused captures (deny, interrupt, main agent, failed subagent) are removed.

* test(claude): pin the parent row's dating when a subagent asked first, and narrow the oracle's claim
* feat(notifications): turn desktop notifications on or off per machine

Settings > Notifications lists each machine (this computer, SSH targets,
paired Orca servers) with a switch. Muted machines are stored as an
opt-out list so a newly added machine still notifies. Both notification
senders now name the machine a workspace runs on, and main skips the
desktop banner for a muted one; phone push is unchanged.

* fix(notifications): preserve phone alerts and honor machine mutes

* fix(notifications): bind machine mutes to configured sources

* fix(notifications): preserve chat completion subscriptions on owner collisions

* fix(notifications): reduce machine settings clutter with a collapsed section

* fix(notifications): align machine disclosure with settings rows

* fix(notifications): clarify machine switches affect only this computer
@nwparker
nwparker marked this pull request as ready for review October 3, 2026 21:25
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 46dd3c1f-fd7f-49b4-8e9e-3acdcfd8ffa0
📥 Commits

Reviewing files that changed from the base of the PR and between f713a8b and 52781b1.

📒 Files selected for processing (195)
  • .github/workflows/macos-updater-tests.yml
  • cloud/apps/relay/src/host-control-proof-cleanup.test.ts
  • cloud/apps/relay/src/host-session-registry.ts
  • config/build-plugins/plain-node-entry-guard.ts
  • config/reliability-gates.jsonc
  • config/scripts/plain-node-entry-guard.test.ts
  • config/scripts/stable-release-tag-selection-budget.test.mjs
  • config/scripts/stable-release-tags.mjs
  • docs/reference/agent-status-store.md
  • mobile/rpc-foundation/goldens/lifecycle-inventory-lifecycle.json
  • mobile/src/mobile-web-shell/bridge-host-subscription-start-cleanup.test.ts
  • mobile/src/mobile-web-shell/bridge-host-subscriptions.ts
  • mobile/src/platform/native-media-device.ts
  • mobile/src/platform/native-media-stage-cleanup.test.ts
  • mobile/src/session/mobile-file-search-owner-cleanup.test.tsx
  • mobile/src/session/mobile-session-toast-owner-cleanup.test.tsx
  • mobile/src/session/use-mobile-native-chat-file-search.ts
  • mobile/src/session/use-mobile-session-feedback-capabilities.ts
  • mobile/src/test-support/rpc-recording/derived-goldens.ts
  • mobile/src/test-support/rpc-recording/inventory-lifecycle-derivation.test.ts
  • mobile/src/transport/mobile-relay-e2ee-link.ts
  • mobile/src/transport/mobile-relay-error-timer-cleanup.test.ts
  • mobile/src/transport/mobile-relay-pairing-error-timer-cleanup.test.ts
  • mobile/src/transport/mobile-relay-physical-client.ts
  • mobile/src/worktree/agent-row-lineage-parity.test.ts
  • mobile/src/worktree/agent-row-lineage.ts
  • src/main/claude/__fixtures__/claude-subagent-permission-frames.json
  • src/main/claude/claude-can-use-tool-test-support.ts
  • src/main/claude/claude-child-tool-queries.ts
  • src/main/claude/claude-child-work-decoder.test.ts
  • src/main/claude/claude-child-work-decoder.ts
  • src/main/claude/claude-child-work-evidence-retention.test.ts
  • src/main/claude/claude-child-work-evidence.ts
  • src/main/claude/claude-journal-translator-contract.ts
  • src/main/claude/claude-prompt-registry.ts
  • src/main/claude/claude-structured-control-actions.test.ts
  • src/main/claude/claude-structured-inbound-control.ts
  • src/main/claude/claude-structured-journal-prompt-cards.test.ts
  • src/main/claude/claude-structured-journal-prompts.ts
  • src/main/claude/claude-structured-journal-translation.ts
  • src/main/claude/claude-structured-session-adapter.ts
  • src/main/claude/claude-subagent-linkage.ts
  • src/main/claude/claude-subagent-permission-request.test.ts
  • src/main/claude/claude-subagent-roster.ts
  • src/main/claude/claude-turn-ownership.test.ts
  • src/main/daemon/daemon-idle-inventory-budget.test.ts
  • src/main/daemon/daemon-server.ts
  • src/main/daemon/terminal-host.ts
  • src/main/macos-update-running-instances.integration.test.ts
  • src/main/macos-update-running-instances.test.ts
  • src/main/macos-update-running-instances.ts
  • src/main/managed-data-accounts/service-enrollment-persistence.test.ts
  • src/main/managed-data-accounts/service.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink-queue.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-event-sink.ts
  • src/main/notifications/notification-delivery-service.test.ts
  • src/main/notifications/notification-delivery-service.ts
  • src/main/persistence/applying-settings/notification-settings-normalization.test.ts
  • src/main/persistence/applying-settings/onboarding-normalization.ts
  • src/main/runtime/orchestration/coordinator-task-dispatch.ts
  • src/main/runtime/orchestration/db/decision-gates/decision-gate-store.ts
  • src/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts
  • src/main/runtime/orchestration/db/dispatch-depth.ts
  • src/main/runtime/orchestration/db/legacy/legacy-compatibility-candidates.ts
  • src/main/runtime/orchestration/db/messages/foreign-direct-mailbox-routing.ts
  • src/main/runtime/orchestration/db/remaining-dispatch-statement-compilation.test.ts
  • src/main/runtime/orchestration/send-agent-turn-boundary.test.ts
  • src/main/runtime/orchestration/send-agent-turn-host.test.ts
  • src/main/runtime/orchestration/send-agent-turn.test.ts
  • src/main/runtime/orchestration/send-agent-turn.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-host.test.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-host.ts
  • src/main/runtime/orchestration/structured-pointer-operation-id.test.ts
  • src/main/runtime/orchestration/structured-pointer-operation-id.ts
  • src/main/runtime/rpc/methods/orchestration-structured-worker-session.test.ts
  • src/main/runtime/rpc/methods/orchestration-structured-worker-session.ts
  • src/main/runtime/rpc/methods/orchestration/federation/federation.ts
  • src/main/runtime/rpc/methods/orchestration/runs/dispatch-methods.ts
  • src/main/runtime/rpc/methods/orchestration/worker/deliver-worker-dispatch-preamble.ts
  • src/main/startup/desktop-startup-ordering.test.ts
  • src/main/startup/main-process-quit-update-veto.test.ts
  • src/main/startup/main-process-quit.ts
  • src/main/updater-events.test.ts
  • src/main/updater-fallback.ts
  • src/main/updater-linux-package-recovery-actions.test.ts
  • src/main/updater-mac-install.ts
  • src/main/updater-mac-quit-guard.test.ts
  • src/main/updater-test-harness.ts
  • src/main/updater.check-failure.test.ts
  • src/main/updater.check-preflight.test.ts
  • src/main/updater.fallback.test.ts
  • src/main/updater.headless-serve-install.test.ts
  • src/main/updater.install-failure-cause.test.ts
  • src/main/updater.mac-install.test.ts
  • src/main/updater/updater-download-install.ts
  • src/main/updater/updater-install-execution.ts
  • src/main/updater/updater-menu-checks.ts
  • src/main/updater/updater-scheduling.ts
  • src/main/window/dashboard-popout-window.test.ts
  • src/main/window/dashboard-popout-window.ts
  • src/main/window/main-window-state-lifecycle.test.ts
  • src/main/window/main-window-state-lifecycle.ts
  • src/main/window/main-window-updater-deferred-setup.test.ts
  • src/main/window/main-window-updater.ts
  • src/relay/ai-vault-service-cancellation.test.ts
  • src/relay/ai-vault-service-entry.ts
  • src/relay/pty-handler-owner-inventory-clone-budget.test.ts
  • src/relay/pty-handler.ts
  • src/renderer/src/attention/notification-sources.test.ts
  • src/renderer/src/attention/notification-sources.ts
  • src/renderer/src/attention/notification-subject-owner.ts
  • src/renderer/src/components/UpdateCard.error-card.test.tsx
  • src/renderer/src/components/activity/activity-search-query-work-budget.test.tsx
  • src/renderer/src/components/activity/activity-thread-grouping.ts
  • src/renderer/src/components/activity/use-agent-pane-threads.ts
  • src/renderer/src/components/dashboard-popout/AgentTerminalPreview.frame-lifecycle.test.tsx
  • src/renderer/src/components/dashboard-popout/AgentTerminalPreview.settings-lifetime.test.tsx
  • src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx
  • src/renderer/src/components/dashboard-popout/preview-terminal-box-fit-lifecycle.test.ts
  • src/renderer/src/components/dashboard-popout/preview-terminal-box-fit.ts
  • src/renderer/src/components/maintenance/update-card/update-card-error-model.ts
  • src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.test.tsx
  • src/renderer/src/components/native-chat/StructuredAgentSessionAttentionBridge.tsx
  • src/renderer/src/components/native-chat/structured-attention-dispatch.test.ts
  • src/renderer/src/components/native-chat/structured-attention-dispatch.ts
  • src/renderer/src/components/onboarding/use-onboarding-flow-persistence.test.ts
  • src/renderer/src/components/settings/GeneralUpdateSettingsSection.test.tsx
  • src/renderer/src/components/settings/GeneralUpdateSettingsSection.tsx
  • src/renderer/src/components/settings/NotificationHostToggles.test.tsx
  • src/renderer/src/components/settings/NotificationHostToggles.tsx
  • src/renderer/src/components/settings/NotificationsPane.test.tsx
  • src/renderer/src/components/settings/NotificationsPane.tsx
  • src/renderer/src/components/settings/notification-host-pending-save.test.tsx
  • src/renderer/src/components/settings/notifications-search.ts
  • src/renderer/src/components/settings/use-notification-source-options.ts
  • src/renderer/src/components/status-bar/resource-manager-exit-render-budget.test.tsx
  • src/renderer/src/components/status-bar/resource-session-removal-render-budget.test.tsx
  • src/renderer/src/components/status-bar/use-resource-session-inventory.ts
  • src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts
  • src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts
  • src/renderer/src/components/terminal-pane/pty-connection-hook-completion-bell-arbitration.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-hook-completion-side-effects.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-hook-idle-arbitration.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-task-complete-dispatch.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-types.ts
  • src/renderer/src/components/terminal-pane/pty-connection/agent-task-complete-notify.ts
  • src/renderer/src/components/terminal-pane/pty-connection/pane-pty-visibility-bind.ts
  • src/renderer/src/components/terminal-pane/pty-output-side-effect-queue-reentrancy.test.ts
  • src/renderer/src/components/terminal-pane/pty-output-side-effect-queue-retention.test.ts
  • src/renderer/src/components/terminal-pane/pty-output-side-effect-queue.ts
  • src/renderer/src/components/terminal-pane/pty-side-effect-pending-census.test.ts
  • src/renderer/src/components/terminal-pane/pty-side-effect-pending-census.ts
  • src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts
  • src/renderer/src/components/terminal-pane/pty-transport.ts
  • src/renderer/src/components/terminal-pane/terminal-pane-lifecycle-types.ts
  • src/renderer/src/components/terminal-pane/terminal-pane-mount-cleanup.ts
  • src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts
  • src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.test.ts
  • src/renderer/src/components/terminal-pane/terminal-parked-watcher-reconciliation.ts
  • src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts
  • src/renderer/src/components/terminal-pane/use-notification-dispatch.ts
  • src/renderer/src/components/ui/collapsible.tsx
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/lib/pane-manager/pane-container-listener-lifecycle.test.ts
  • src/renderer/src/lib/pane-manager/pane-dom-focus-class-sync-lifecycle.test.ts
  • src/renderer/src/lib/pane-manager/pane-dom-focus-class-sync.ts
  • src/renderer/src/web/preload-api/web-preference-normalization.ts
  • src/shared/agent-session-option-agent-args.ts
  • src/shared/agent-session-option-catalog-antigravity.ts
  • src/shared/agent-session-option-catalog-claude-codex.ts
  • src/shared/agent-session-option-catalog-codebuddy.ts
  • src/shared/agent-session-option-catalog-gemini-cursor.ts
  • src/shared/agent-session-option-catalog-grok.test.ts
  • src/shared/agent-session-option-catalog-grok.ts
  • src/shared/agent-session-option-catalog-muse.ts
  • src/shared/agent-session-option-catalog-omp.test.ts
  • src/shared/agent-session-option-catalog-omp.ts
  • src/shared/agent-session-option-catalog-removers.test.ts
  • src/shared/agent-session-option-catalog-types.ts
  • src/shared/agent-session-option-launch.ts
  • src/shared/command-option-occurrence.ts
  • src/shared/main-agent-status-parity.test.ts
  • src/shared/notification-settings-defaults.ts
  • src/shared/notification-settings-types.ts
  • src/shared/notification-source.ts
  • src/shared/structured-agent-session-outbox.ts
  • src/shared/structured-agent-session-send-mutation.ts
  • src/shared/update-status-types.ts
💤 Files with no reviewable changes (5)
  • src/shared/agent-session-option-catalog-codebuddy.ts
  • src/shared/agent-session-option-catalog-muse.ts
  • src/shared/agent-session-option-catalog-antigravity.ts
  • src/main/runtime/orchestration/structured-mailbox-pointer-delivery.ts
  • src/main/runtime/orchestration/structured-pointer-operation-id.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The pull request updates several independent areas. It changes managed-account enrollment persistence and account lookup, adds Claude subagent permission-wait reporting, and centralizes agent-turn sending. It adds macOS updater install checks and quit-event handling, notification-source controls, and lifecycle cleanup in mobile and renderer code. Other changes update relay behavior, queue retention, daemon inventory, build checks, and agent argument handling.

Priority: ⬆️ High

Merge Risk: ⚪ Minimal · up to 52781

This change touches enrollment, updater, notification, send, and lifecycle cleanup paths. No outstanding defect is established, and previously raised concerns are addressed. Normal CI checks should be sufficient before merging.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 52781

The credential-preservation behavior improves recovery from enrollment failures. However, account lookup and deletion do not consistently resolve differently cased identities, which can leave credentials behind after removal. The broader delivery and installation changes also warrant caution because their failure and recovery behavior has only been partially assessed.

Retained concerns

  • Medium · security · inferred: Case-insensitive lookup resolves the lowercase profile directory, but removal still derives its directory from the caller's exact UUID spelling. For uppercase registered metadata backed by a lowercase directory on a case-sensitive filesystem, removal can publish deregistration without quarantining the actual credentials. Recovery scans rollback and pending-delete locations, not that ordinary orphan directory. This conflicts with the case-variant identity state explicitly supported by the change. Normal lowercase accounts are unaffected; whether the PR newly introduces or materially expands this condition remains unresolved because a reliable base comparison was unavailable.
Security review details

Security Blast Radius

  • inferred — The identified credential-retention condition is scoped to an affected account's local provider directory and requires case-variant persisted metadata on a case-sensitive filesystem. The evidence does not establish remote access, cross-tenant exposure, privilege gain, or credential leakage from this condition.

Trust Boundaries and Controls

  • observed — The inspected relay control path verifies a bearer token before host acceptance. Proof initiation binds the authenticated identity to the relay host identifier, host public key, assignment, challenge transcript, and expiry. Activation requires a matching challenge and constant-time proof verification; source inspection did not establish a bypass, but historical delta attribution remains unresolved.

Resilience and Maintainability Implications

  • observed — Relay proof reading removes its timer and listeners on completion. Replacement and orphan cleanup operate on the owning host session, with resume-secret and generation checks limiting stale-socket authority.

Hardening Proposals

  • proposed — Use one canonical account-identity resolution policy for lookup, removal, quarantine, rollback, and recovery so supported UUID casing cannot change which credential directory cleanup owns.
🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 25.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 51 files. (140 skippe… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main enrollment change: preserving saved account credentials after errors.
Description check ✅ Passed The description covers the required sections with clear change details, rationale, testing evidence, visual-proof explanation, and review notes. It identifies related issues and explains the platform …
Full details: Docstring Coverage

Explanation

Docstring coverage is 25.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 51 files. (140 skipped: 11 unsupported, 129 over the file limit.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a5b36433-5172-4d9e-bd29-a677177cae12
📥 Commits

Reviewing files that changed from the base of the PR and between fd030c4 and 5f5dddd.

📒 Files selected for processing (3)
  • src/main/managed-data-accounts/service-enrollment-persistence.test.ts
  • src/main/managed-data-accounts/service.test.ts
  • src/main/managed-data-accounts/service.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/main/managed-data-accounts/service.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — verified the OpenCode/Devin managed-account enrollment recovery and listener-isolation change: read the full diff, ran the affected suites (97 passing), and ablated the fix to confirm the new preservation tests fail without it.

  • Preserve committed profiles on persistence failure: add()'s catch re-reads the account list and deletes the profile directory only when the new UUID is provably unregistered, using a case-insensitive UUID match; an unreadable list preserves the directory. The original error still propagates.
  • Listener isolation: notifyChanged() catches per-listener throws and logs a generic warning, so one bad subscriber can no longer reject a committed enrollment or removal, and private listener detail does not reach the log.
  • Windows-safe test: the credential-symlink rejection test is now skipped on win32, where file symlink creation needs elevation; production symlink rejection is untouched.

ℹ️ Preserved unregistered profiles have no later garbage collector

The preserve-on-uncertainty path intentionally keeps a profile directory whose metadata could not be read. Nothing removes that directory later: the constructor's retry() only sweeps .pending-delete and accounts.json.*.rollback, never unregistered provider directories. So in the (rare) unreadable-metadata case, a directory holding captured credentials can linger indefinitely after the add fails. This matches the stated trade-off — credentials are recoverable, they are not deleted — so no change is requested; flagging it only so the retention lifecycle is a deliberate choice.

Technical details
# Orphaned enrolled profiles

## Affected sites
- `src/main/managed-data-accounts/service.ts:101-111` — preserves `directory` whenever the registered check is uncertain (list throws).
- `src/main/managed-data-accounts/profile-removal.ts:165-197` — `retry()` scans only pending-delete and rollback entries, never unknown provider directories.
- `src/main/managed-data-accounts/service.ts:53-64` — startup recovery warns and stops when `list()` throws, so no sweep runs while metadata is unreadable.

## Required outcome
- None required for this PR. Open question: should a profile directory whose registration never became provable ever be swept, or is indefinite retention acceptable?

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — incremental review of the delta since the prior pullfrog review at 5f5dddd, covering src/main/managed-data-accounts/service.ts and its tests.

  • Confirmed absence now requires ENOENT: add()'s catch reads metadata through a direct readState(...) call instead of list(), and treats only a missing metadata file (ENOENT) as proof the profile is unregistered; any other read/parse error leaves registered = true and preserves the directory. This closes the gap where list()'s existsSync guard turns an access error into apparent absence and deletes committed credentials.
  • Extracted readState helper: list() now delegates to a private readState(path) (assertOwned + parse), so the existence-check path and the direct catch path share one reader.
  • Filesystem-error regression test: keeps credentials when metadata existence cannot be checked mocks node:fs existsSync→false / lstatSync→EACCES for accounts.json and asserts the profile directory and launch environment survive. I confirmed it discriminates: reverting the catch to the prior this.list(provider) form fails this test for both providers (2 failed / 12 passed).

Full changed-file read, and the two affected suites pass (47 tests).

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

nwparker and others added 13 commits October 3, 2026 15:18
Consolidate override detection and removal into removeAgentArgs. Previously
catalogs used agentArgsOverride to detect conflicts and removeAgentArgs to
strip them; now removeAgentArgs handles both by returning stripped tokens.
This eliminates redundancy and makes the intent clearer.

Enhance removeAgentArgOption with optional value filtering to support
selective removal for complex cases like Codex config overrides.
Preserve traversal order and cycle guards using one call-local path Set rather than a copy at every depth.
…tart (#24547)

Dispose the failed subscription record’s existing terminal backlog before rethrowing its original start error.
Capture the existing host-age expression at its original time, then use one call-local fresh owner clone for the length check and unchanged published owner field.
* Reuse prepared statements across remaining Dispatch reads

Use the existing schema-pinned complete Dispatch column list in five remaining read modules, enabling the existing bounded statement cache without caching results.

* Reuse prepared statements for internal worker checks

Use the existing complete Dispatch projection only for named-field internal consumers; preserve original wildcard reads for every returned failure snapshot.
Lazily reuse a call-local search predicate inside the existing filter, using unchanged byte gates and thread text cache.
Reuse a successful unchanged chunk-code verdict within one synchronous writeBundle invocation, while preserving traversal and current-code reads.
Replace filter/sort/last selection with one traversal using the unchanged stable-tag regex and numeric comparator; update on equality to preserve the original last spelling. Both actual callers own private plain arrays from Git stdout.
…tory unchanged (#24806)

Reuse the private React state object only when the existing single/bulk removal helper returns its identical inventory; keep every lifecycle revision, tombstone, known-ID write and helper invocation in its original order.
Reuse the existing pane frame tracker to cancel owned deferred focus-class updates during existing cleanup, with disposed guards against late or reentrant delivery.
Reuse the existing frame tracker inside the box-fit owner, suppress scheduling after disposal and cancel pending frames at the start of the existing effect cleanup.
Reuse the existing mobile mounted-ref lifecycle pattern at toast presentation entry, preserving admitted clipboard outcomes and every live animation/sequence/timer operation.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

select() now accepts a case-variant UUID through requireAccount(), but still persists the caller's raw casing, so activeAccountId can disagree with every stored account id. Exact-match readers then misreport the active account.

Reviewed changes — incremental review of the delta since the prior pullfrog review at ff88f562, covering src/main/managed-data-accounts/service.ts and its enrollment test.

  • Case-insensitive account resolution: requireAccount() now matches a stored account by lowercased id and derives the profile directory from the canonical lowercase id, so a caller can resolve an account whose stored UUID differs in case.
  • UUID-case test expanded: the existing preserves a profile registered with another UUID case case now restores its writer mock and asserts the lowercase profile data directory, launchEnvironment(), transcriptEnvironments(), and selection by both case variants.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread src/main/managed-data-accounts/service.ts
Clear consumed queue slots after successful apply or overflow carry; preserve backing identity across reentrant callbacks and report actual retained slots.
Reuse the existing first-frame finish pattern to remove stage-owned timer/message/close callbacks on receipt, timeout or close; check OPEN after successful async assignment verification.
Check the existing irreversible closed flag before scheduling the existing missing-close fallback timer.
Check the existing pairing owner closed flag before allocating its missing-close fallback alarm.
Reuse existing provider-copy best-effort deletion for a newly created clipboard cache file whose write fails before its URI reaches the caller.
Reuse the existing mobile mounted-ref pattern only at legacy fallback entry after completed passive cleanup; preserve admitted work and all live search/authority/cache paths. Correct only the strict fully-unmounted inventory scenario and its sole golden.
nwparker and others added 6 commits October 3, 2026 15:47
Use the relay child's existing pending Set to ignore cancellation IDs after completion, matching its desktop sibling; preserve admitted active/queued cancellation and the bundled private sender's complete replies, errors and ownership.
Cancel completed deferred updater fallbacks in finally, clear only their exact pending callback, and drop the captured timer before the original fallback guard runs; preserve destroyed admission, successor ownership and updater/quit callbacks.
…es (#24749)

* Check daemon idle state without building discarded terminal inventories

Replace the private idle predicate full public inventory with a host-local full pass that still reads every Session liveness getter in original order.

* Remove overwritten daemon test mock assignment
…dates (#24952)

* fix(updater): guard macOS installs against running app instances

* fix(updater): match native app blockers and preserve quit lifecycle

* fix(updater): keep ordinary macOS quit on Squirrel's install-on-exit path

Converting every quit with a staged update into quitAndInstall made Cmd+Q
relaunch Orca, refused the quit when background instances existed, and
hijacked app.relaunch()+app.quit() restart flows (profile switch, admin
restart) into an update install racing the relaunched old app. Only
Update & Restart runs the running-instance preflight now; the
quit-without-install allowance is no longer reachable and is removed.

* fix(updater): preserve quit intent through macOS staging

* test(native-chat): explicitly model legacy published tab ownership

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Co-authored-by: m4air <m4air@m4airs-Air.localdomain>
Preserve every accepted component postimage and all unowned main entries.
Retain the public component ancestry and prior source-qualified proof.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — incremental review of the delta since the prior pullfrog review at f713a8b, covering src/main/managed-data-accounts/service.ts and its two test files. The 52781b1 head is a "Merge current main into account" commit; the authored delta remains exactly those three files.

  • Persisted the canonical account id on selection: select() now stores the account.id resolved by requireAccount() rather than the caller's raw accountId, so a case-variant selection no longer leaves activeAccountId disagreeing with every stored account id. This resolves the prior [!IMPORTANT] finding.
  • requireAccount() now returns { id, directory }: profileEnvironment() destructures the directory; no other callers exist, so the signature change is fully contained.
  • Test coverage strengthened: the UUID-case test now asserts the resolved activeAccountId for both case variants, and a new case verifies the registered spelling is persisted and the active transcript environment is searched first. Ablating the fix (persisting the raw casing) fails all four new assertions, so they genuinely discriminate.

Full 49-test run across the two suites passes.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

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.

4 participants