1 Commits

Include renames

Author	SHA1	Message	Date
Brennan Benson	434365d2de	Offer to reconnect native chats that were working when Orca restarted (#21096) ... * feat(native-chat): resume structured chats that were working at restart Teardown records a marker for every session this host was genuinely running a turn for, derived from the LIVE runtime rather than a persisted status row, so a stale `running` row left by an older crash can never trigger a resume. On the next launch a modal lists exactly which chats would resume and resumes them via native continuation (Claude resume/resumeSessionAt, Codex thread id) — never by re-sending the prompt, which is what makes an agent redo finished work. A session resumes only when all of these hold: a teardown marker exists and has not expired, the record's lease is released and reconciled, a provider resume cursor exists and still matches the marker, the journal's own turn record names the same turn, and the marker has not already been spent. Markers are consumed before the resume is submitted, so a crash mid-resume cannot double-fire, and an admission gate refuses a second concurrent resume for one session. Resumes are staggered three at a time rather than spawning every provider at once. The modal's "Don't ask again" checkbox writes the nativeChatResumeWorkOnRestart setting, which Settings can turn back off; automatic mode runs the identical predicate and staggering and reports what it did. Declining consumes the markers so the prompt cannot return every launch — nothing is lost, because opening a chat still re-acquires it at the same cursor. * fix(native-chat): compare handle ROOT and turn state when offering a resume Four defects QA found in the restart-resume offer, fixed together because the first two interact: shipping the root fix without the state fix would convert a silent no-op into actively offering finished chats. 1. Claude was never offered (0/4). The marker recorded agentSessionProviderHandleKey, which embeds Claude's leaf uuid — a branch cursor. The adapter's own close path appends a `resumed` link with an advanced leaf during the SAME teardown, so the marker went stale seconds after it was written and the drift guard refused every Claude session forever. Record and compare agentSessionProviderHandleRoot instead: the root is the part a resume must preserve, and changing it is a fork, which is exactly what this guard is for. Codex is unaffected (its thread id is the whole key) but uses the root too, so the rule is uniform. 2. The predicate compared turn IDENTITY but discarded turn STATE, so a `completed` turn satisfied it as readily as an interrupted one. Eviction rewrites `running` to `interrupted` and never to `completed`, so the state is what separates work that was cut off from work that finished. Require `interrupted` or `unverifiable`. 3. A chat blocked on a pending approval or question was marked as working, because the teardown reader accepted any `running` turn while the product's own projection calls that state `attention`. Teardown now defers to that projection: an agent waiting on the USER is not interrupted work. 4. "Resume all" could silently no-op. The modal fetched candidates at mount; by click time the chat's own pane may have bound and taken the hold, moving the lease to `live` so the predicate dropped it and the call returned no results, leaving the dialog open behind a dead button. Re-derive at click time and settle an already-live session as resumed — it is running, which is what the user asked for. Test fakes now model the Claude close path that advances the leaf, which is why no unit test could previously exhibit defect 1. Ablation covers all eleven guards. * fix(native-chat): gate the already-live settlement on the full resume predicate Two follow-ups from re-QA, both cases of a rule stated by intent rather than by discriminator. 1. The already-live path bypassed the predicate. "Resume all" sends no session ids, so the fallback's target set was every marker, and it was gated only on the session having a live provider child. A chat the predicate had refused -- a completed turn, say -- whose pane happened to own the lease was therefore settled as `already_live` and had its marker spent, inflating the "Resumed N" count with chats that were never eligible. No provider spawned and no tokens were spent, but a marker the predicate rejected must never be consumed. The resumable set now takes an explicit `leaseState`. The already-live path derives a second set with ONLY the released-lease clause relaxed, and settles a session just when it is in that set. Every other clause still applies. 2. The `attention` rule was one-sided. Teardown refuses to mint a marker for a chat blocked on the user, but the set predicate had no equivalent, so a marker arriving by any other route was offered once eviction rewrote its turn to `interrupted` -- the same asymmetry the completed-turn case had. Gated on projectStructuredAgentSessionStatus === 'attention'. That projection tests for a pending approval or question BEFORE it looks at turn state, so it still reports `attention` after the turn is settled, which makes it the durable signal and keeps one source of truth with teardown. Ablation now covers thirteen guards, including one for each of the above. * fix(native-chat): capture awaits-user on the marker instead of re-deriving it The awaits-user clause could never fire. It asked the live projection for `attention`, which needs a prompt whose resolution is still `pending` -- but teardown CANCELS that prompt a few phases after it writes the marker. By the next launch the evidence is gone, for precisely the sessions the clause was written for. QA measured the injection still being offered and then resumed. This is the same shape as the leaf-drift bug: state read after teardown is not the state that justified the marker. The discriminator, now applied across the whole predicate: - a fact teardown itself destroys or mutates must be CAPTURED on the marker while it is still true; - a fact that evolves on its own must be RE-DERIVED at read time, never snapshotted. So `awaitsUser` is now recorded at teardown and the predicate reads the recorded value. Teardown still declines to mint a marker for such a session, so the recorded flag is the second line rather than the only one. Audit of every other clause against the same test: - turn id (captured) -- teardown rewrites turn STATE but never the id. Correct. - provider handle root (captured) -- the close path appends a resumed link, and appendAgentSessionProviderHandleLink refuses one that changes the root, so the root is invariant under exactly the mutation that broke the key. Correct. - turn state (re-derived) -- DELIBERATE exception, stated here rather than left implicit: we are not reading the state that justified the marker, we are reading teardown's receipt that it settled the turn. A turn still `running` means eviction never finished, and we refuse. Correct, and intentionally so. - lease reconciled / released / handoff stage (re-derived) -- these answer a different, launch-time question: may this host take the lease NOW. The teardown-time value would be meaningless, and `unreconciled` is cleared by this launch's own reconciliation. Correct. - adapter support, marker TTL, marker consumption (re-derived) -- all evolve independently of teardown. Correct. Only awaitsUser was on the wrong side. * fix(native-chat): drop the unreachable awaits-user marker flag The captured flag was dead code. `awaitsUser` could only be true when the projected status was `attention`, and `attention` hits the `continue` above the push -- so every marker teardown can ever write carries `false` (QA measured 22 of 22 across two real teardowns). The predicate clause reading it was unreachable by any production path. A flag that is structurally always false is worse than no flag: it reads as a safeguard, so the next person to touch this trusts it. The asymmetry it was added to close was only ever reachable by fault injection, because teardown is the sole writer of markers and already refuses attention sessions. Removing it also drops an upgrade discontinuity: as a required field it made a marker written by the previous build fail validation and be silently discarded, costing a resume offer on precisely the upgrade where the user was mid-turn. Markers predating the providerHandleRoot rename still will not parse, but those carry a leaf-sensitive key the predicate would refuse anyway, so nothing usable is lost. In its place the teardown gate now states that `status !== 'working'` is the SINGLE gate for awaiting-user sessions, why a predicate-side mirror would be unreachable, and why it could not even re-derive the fact -- so the reasoning is inherited rather than rediscovered. Ablation is back to twelve guards; every other clause is unchanged. * fix(native-chat): say reconnect, not resume, and show each offer's age Two changes, both independent of the parked continuation decision. 1. The copy claimed something QA disproved. "Resuming continues each agent where it left off" is false: reconnection restores the session at the point it stopped, with full context and without re-sending the prompt, but the interrupted reply does not continue on its own. The toast's "Resumed N chats" implied work had restarted. Audited every user-facing string against the rule that none may claim work continues or that a reply resumes -- which caught more than the three strings the fix started from. The title, the row button, "Resume all", "Resuming...", the not-now hint ("picks it up where it left off"), the checkbox and its hint ("resume on their own"), the list's aria-label and the Settings row all made the same claim. The user-facing verb is now reconnect throughout; the body and update variant state outright that the interrupted reply will not continue. en.json synced, runtime boot catalog regenerated. If we later decide to send a continuation instruction, this is one commit to change back. Shipping text we know to be false was the worse option. 2. Rows now show each offer's age. The TTL is 24 hours and a stale offer looked identical to a fresh one. The marker already carried `recordedAt`, so this is a render change plus one field on the renderer's candidate type, formatted with the existing formatUiRelativeTime helper rather than a new one. The clock is stamped once when the list arrives rather than read during render: ages then stay stable across re-renders, and the render stays pure, which the react(purity) rule requires. Guards, predicate and RPC are untouched; ablation still covers twelve. * feat(native-chat): show the workspace name on each reconnect row A row read `codex · folder:8f3a1c22-… · 8 hours ago`. Recognising which chats would reconnect is the entire point of the list, and at twenty rows a UUID identifies nothing. No RPC or host change was needed: the renderer can already resolve this id. Resolved the way automation dispatch resolves the same id space (resolveAutomationDispatchWorkspace) -- a folder workspace by its full `folder:<uuid>` key via getKnownWorktreeById, a git worktree by its bare `repoId::path` id via allWorktrees. Both return a Worktree, whose displayName is a required field, and DetectedWorktree extends Worktree so either shape answers. Falls back to the id when nothing resolves, which is what the row showed before and also covers the window before the worktree store has hydrated. The lookup lives in a per-row subcomponent because a hook cannot run inside `map`, and its selector returns a primitive string so repeated selector runs cannot churn referential equality. * feat(native-chat): group the reconnect modal by worktree and add opt-in continuation Grouping. Rows are now grouped under a worktree heading with the repo glyph and an agent count, using the sidebar's own collapse mechanics. Only presentational pieces are reused -- RepoIconGlyph, CompactAgentExpansion, AgentIcon and formatShortTimeAgo. The sidebar's agent row cannot be: worktree-card-compact-agent-row imports DashboardAgentRow, the dashboard's own type, so both surfaces render one live-agent model requiring a pane, tab and status entry. Every chat offered here is by definition stopped, so supplying that would mean inventing live state. Two things I had assumed were reusable and were not: - DashboardHostBadge returns null unless hostKind is ssh or remote. Structured chat is local-only, so it would always render nothing. The host line is omitted rather than faked; the badge is the right element to add if and when structured chat gains remote support. - No state dot. Every AgentDotState misleads here: idle and unverifiable both presuppose a live pane, interrupted renders red like an error, done green, working a spinner. A missing dot beats one saying these agents are running. One worktree renders flat with no heading -- a name, count and chevron around a single group says nothing the dialog has not already said. The age column now uses formatShortTimeAgo for sidebar consistency. It takes (timestamp, now) and subtracts internally rather than taking a delta, so the call is (recordedAt, listedAt); passing the old delta would have rendered plausible nonsense. The clock is still stamped once into state, so ages stay stable and the render stays pure. Continuation. A secondary "Reconnect and continue" action sends one message, from a single shared constant, identical for both providers. Reconnect is unchanged and still sends nothing. An info popover quotes the literal message read from that same constant, so what is shown cannot drift from what is sent. Ablation now covers fourteen guards. Two are new: continuation only follows a reconnect that actually happened, and -- inversely -- a send injected into the reconnect path must turn the test red, since "don't ask again" rests on reconnect never sending. * feat(native-chat): say terminal sessions kept running, and clear the quality gate The modal lists stopped chats with no way to tell that CLI agents are fine, and the true state of the world is counterintuitive: the terminal sessions survived the restart and the chats did not. One line now says so, next to the heading where it frames the list rather than as a footnote at the bottom. Wording follows the app's own vocabulary rather than inventing a term: the catalog settles on "terminal sessions" (terminalSessionCount, "Terminal sessions are grouped by workspace", "No terminal sessions yet"), and UpdateCard already reassures with "Your terminal sessions won't be interrupted during the update" in the same text-xs text-muted-foreground treatment. "kept running" rather than "were restored" -- nothing reconnected them, they never stopped, and the line says nothing about why. Also clears check:code-quality:changed, which I had not been running -- oxlint alone covers neither the design-system nor the casting audit, so 18 findings had accumulated across the branch. - design system (4): Button spacing hand-rolled as gap-1/px-2 is just size="xs"; PopoverContent and DialogTitle own their typography and spacing, so the text-xs moved to the popover's own children and the title's icon gap moved to a plain wrapper. - casting (14): production code loses its assertions outright via Reflect.get, the idiom already used in managed-hook-detection-commands and worktree-name-retirement. The marker validator reads each field through Reflect.get and now checks recordedAt is a number rather than asserting it; the store-file parse uses the existing `file` shape instead of a second assertion; the runner narrows the admission error's owner with typeof. Test fixtures keep their assertions behind the line-specific SAFETY: rationale the repo mandates for exactly this case. One trap worth recording: the audit reports an assertion at the line its EXPRESSION OPENS, not where `as` appears, so a disable-next-line above the closing brace of a multi-line literal is inert and silently changes nothing. Guards unchanged; ablation re-proved 14/14 at this head. * fix(native-chat): give the reconnect row's provider icon an accessible name Every row rendered the provider as a bare AgentIcon, whose svg carries no aria-label, title or alt. With a Claude chat and a Codex chat in one worktree the two rows were identical to any non-visual consumer, and the dialog offered several identically-named "Reconnect" buttons with nothing to tell them apart. A regression from 233e37b2bd, where the row read `${agent} · ${workspace} · …` as text. Moving the workspace name into the group heading was right; dropping the provider to an unlabelled glyph is what lost the information. AgentIcon takes no label prop, so the icon is wrapped the way NativeChatSupportedAgents already names it: a span with role="img" and an aria-label from formatAgentTypeLabel, the same labeller the sidebar and dashboard rows use. The per-row button also names its agent now ("Reconnect Claude chat"). The identical buttons were half the reported harm, and an accessible name that opens with the visible word keeps WCAG 2.5.3 satisfied. Say so if you would rather ship only the icon label -- it is one attribute and one catalog key to drop. Age code untouched, as asked: formatShortTimeAgo still takes (timestamp, now) and is still called with (recordedAt, listedAt). * fix(native-chat): scope resume markers to one launch and report the real dispatch Three defects in the restart-resume path, all of which could resume a session that was not genuinely working or claim one was continued when it was not. Launch scoping. A durable marker with a 24h TTL is a write-ahead latch: a teardown write that failed or timed out, or a store restored from its backup, left a previous generation's marker actionable, and automatic reconnect would have acted on it silently. Markers now carry the id of the launch that wrote them, and only the launch immediately after may claim them. The launch id lives in its own file with no backup mechanism, so it cannot roll back in step with the markers it is proving adjacency for. Startup claims the previous launch's markers into launch-scoped memory and deletes every durable copy in the same step, so the durable fact dies at claim time rather than at use time. Both halves fail closed: an unprovable predecessor and a clear that throws each claim nothing. Dispatch states. The send layer answers ok as soon as Orca owns the message; the provider's own answer lives in the submission. Continuation read only the envelope, so a rejected turn/start was reported as continued and stamped the journal saying the agent had been asked to carry on. All four states are now preserved, and only an accepted dispatch appends the attribution note. Claude pre-echo sends. Claude cannot write a running turn until the SDK echoes the user message back, which is seconds on a real journal, so a turn-id-only marker dropped exactly the sessions that were working hardest. A send that has not become a turn now carries its own identity, and the launch-side predicate asks the journal about that submission's dispatch state instead. * fix(native-chat): follow an accepted send to its turn, and settle before judging Two defects found in QA, both reproduced twice. Follow the submission forward. The launch-side predicate accepted a submission-shaped marker only while its dispatch was pending or unknown, but the window in which work is submission-shaped is precisely the window in which the dispatch is about to be accepted: the send settles during teardown and the turn it opened is then cut off as interrupted. Judgement was frozen at the moment the marker was written, so the predicate refused the very sessions this was built for and fired only when the send never reached the provider. An accepted submission is now followed to the turn it opened -- matched through the user item key a turn names and a submission is aliased by -- and that turn is judged by the existing turn rule. Accepted alone still proves nothing: without the link, or with a turn that completed, this refuses as before. Settle before judging. A send resolves as soon as Orca owns the message, while its dispatch is still pending; that is the ordinary successful path. Reading the dispatch off the send result therefore reported every delivered continuation as pending and never wrote the attribution note. The outcome is now decided on the settled submission, through the host's existing settlement waiter, with the send result as fallback when nothing settles in time. The failed-note path no longer swallows its error. It stays best effort -- a journal that refuses the note must not turn a delivered continuation into a failure -- but the failure is reported through the host's error sink instead of being discarded, so it cannot regress unseen again. The surface's send is typed against the wire result rather than a hand-written subset, which is what let a test assert a shape the host never returns. Binding the surface to the host moves into its own file: the host was one line under the line cap, and the bindings carry decisions that belong beside their consumer. * feat(native-chat): show the reconnect offer the way the worktree sidebar does The offer is a list of workspaces, so it should read like the one users already know. Rows are now three tiers -- repo or project, then workspace, then the agent sessions inside it -- and each agent carries a checkbox rather than its own button, checked by default, with the footer acting on whatever is ticked. Reused rather than rebuilt. The host chip is the sidebar's own: its markup lived inline in the card's meta row, so it moves to a shared component both surfaces render, and the label comes from getHostContextLabel, which is where "Local Mac" has always come from. The repo glyph is RepoIconGlyph; a group with no repo uses the FolderTree the sidebar's own project-group metadata uses. The agent row reuses AgentIcon, the agent-type label helpers, formatShortTimeAgo and the same model treatment. Two things could NOT be reused, and both are deliberate. The sidebar's CompactAgentRow needs a live pane, tab and status entry, and every chat here is stopped by definition. And the sidebar has no git-worktree-vs-folder glyph resolver at all -- both kinds render the same card, and the difference people read is its status lane choosing GitBranch when a workspace has branch identity; that single precedent is what the workspace glyph follows. The model, the execution host and the workspace kind now travel with each offered chat. All three are read off the durable record the predicate already holds -- the model through the same normalizer the status feed uses -- so the glyph is never inferred from a name and no new data source appears. They are optional on the wire, so an older host still renders a row. Selection changes which ELIGIBLE chats are acted on, never what is eligible. Ids are seeded from the host's own answer and intersected back against it before any call, and the host re-derives the predicate regardless of what it is sent. Continuing still requires an explicit click, and the automatic path still calls the reconnect method, which contains no send. The badge's treatment becomes a variant instead of a pile of overrides, which is what the design-system gate asks for once the markup is somewhere it can see it. * fix(native-chat): title a folder workspace group with its project name A folder workspace's synthetic worktree borrows the `repoId` slot to name the project group it belongs to, so that field is NEVER null. The reconnect offer read a non-null `repoId` as proof of a git repo, looked it up in the repos list, found nothing, and rendered the raw `folder-workspace:<uuid>` string as the group header. The project glyph written for the no-repo case was unreachable for the one workspace kind it was meant for, and the string fallback behind it was dead for the same reason. The project group name was available all along and the sidebar already titles these with it, which is what this list is meant to mirror. Recognising the id now lives beside the code that mints it, so the two cannot drift: there was no such helper, only forward constructions of the same prefix in five places. The header choice itself moved into a pure resolver, so the branch that was wrong is now the branch under test. The dead fallback string is gone, along with its catalog entries. * fix(native-chat): offer an accepted send the provider never opened a turn for QA: a chat that was genuinely working was silently dropped from the offer. The discriminator was how far the send had progressed -- it was the last chat prompted before quitting, reachable by quitting a second or two after sending. Mechanism, reproduced against the predicate. The marker was written while the send was still pending, so it is submission-shaped. During teardown the dispatch then settled to `accepted`, which took it out of the pending/unknown branch and into the follow-forward branch. But the provider died before writing a turn row for that send, so there was no turn to follow forward TO, and the branch demanded a proved link before it would answer. Both the no-turn-at-all case and the newest-turn-belongs-to-an-earlier-exchange case therefore refused. An accepted send that never became a turn cannot be finished work, because finishing writes a turn row. The marked send is also the newest work in the session, so any turn it opened would be the newest turn. That makes the link unnecessary to prove for a safe answer. When the newest turn is interrupted or unverifiable the two readings agree: if the row really is this send's under a key we failed to match, it was cut off; if it belongs to an earlier exchange, this send opened no turn at all. Either way the work was interrupted. A journal with no turn row at all is the same case with nothing to disagree about. The readings only diverge on a `completed` row, where an unmatched one might be this very send's finished turn under a key we did not recognise. That stays refused. Ambiguity resolves to no, because resuming finished work is the one outcome never worth risking. * fix: write the grouping separators as escapes so the files stay text Five separators in the reconnect-offer redesign were written as raw NUL bytes instead of the `\0` escape. The runtime strings were correct and the app behaved, but git classifies a file containing a NUL as binary -- so the two central files of that redesign rendered as "Binary file not shown" in review, and `rg` skipped them silently, returning no matches rather than an error. The escape produces the identical string, so the NUL separator is kept: the previous separator was a space, and a workspace id containing one would corrupt the join/split pair this grouping depends on. Nothing could have caught this. Typecheck, lint, the quality gate, the localization verifiers and the full suite all passed throughout, because none of them look at file encoding. So this adds a check that does, wired into the pre-commit hook where it costs nothing and catches the next one at the moment it is written. Two files already on main carry a raw NUL for the same reason -- one a template separator, one a deliberately tricky test alphabet whose neighbours are all written as escapes. They are grandfathered rather than fixed here, since they belong to their own change, and the gate fails if the list ever grows or goes stale. * fix: parse markers into a domain type, and declare the four restart methods Two CI failures, both ours. Static analysis. `Reflect.get` was adopted to clear the casting audit, and the anti-slop rule forbids it -- the two gates disagree, and the rule text says what both want: parse dynamic input into a named type once, then read typed fields off it. Markers re-enter from a file this process may not have written and decide whether an agent is handed a provider child, so they now go through a single zod parse. Unknown keys still pass, and a malformed marker is still dropped rather than thrown, so a bad entry cannot make a user's sessions unreadable. The launch stamp is parsed the same way, the resume-admission refusal becomes a named error carrying a typed `owner` instead of a bag assigned onto `new Error`, and the test harness gets a named journal type instead of reaching into `unknown`. Cross-version wire. The four restart methods are added to the manifest rather than the count being bumped, so the suite now exercises them in both skews. They are bare additions, not capability-negotiated: an unknown RPC method answers `method_not_found`, which is explicit and visible during negotiation, unlike a stream opcode that is dropped in silence. The whole `agentSession.*` surface already sits behind its runtime capability, so an old client is told it does not exist and never reaches a host method. The stub's spies stay a flat map because callers iterate it asserting each entry is a spy that did not run; a composer reassembles the member the host really exposes. The manifest and its params builders move to their own module, which is what keeps the suite under its line cap as the surface grows. * Prevent duplicate restart continuation and release reconnect holds * fix: recheck interrupted work when admitting restart continuation * fix(native-chat): invalidate restart offers after newer user work * Consume restart recovery offers from an isolated advisory capsule * Refuse completed restart work and report recovery outcomes * fix(native-chat): honor queued completion and uncertain restart delivery * fix(native-chat): preserve restart refusal and teardown evidence * fix(native-chat): rederive recovery evidence before continuing * Validate restart continuation at provider dispatch * fix(native-chat): finish restart refusal and attribution delivery * fix(native-chat): keep recovery teardown errors out of logs * fix(native-chat): validate restart continuation at provider dispatch * Revalidate restart continuation when Claude dequeues input Check continuation authority after the SDK input queue wait and arm replay correlation only after authorization. Preserve typed pre-dispatch refusal, ordinary send behavior, and cleanup when the provider exits or capacity fills during authorization. * Deduplicate settlement test import * Keep merge update scoped to restart recovery * Polish continuation popover spacing	2026-09-17 12:59:03 -07:00