mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
0a7595bbbf16e0fcfb2100bd2e6bee52858ed3eb
1972
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Merge Sim
|
767569b2f9 | Merge main into native chat task lists | ||
|
Merge Sim
|
77cdcb9052 | Render native chat task lists with incremental checklist updates | ||
 Brennan BensonandMerge Sim
|
fa5ef99885 |
fix(native-chat): settle structured chat turns stranded by a restart (#19122)
* fix: settle structured chat turns after restart * fix: preserve unconfirmed turn cancellation state * test: preserve unconfirmed turn lifecycle * test: narrow unconfirmed cancellation coverage * fix: keep intentional TUI closes out of recovery * test: keep branch rename journal mock current * fix: settle dead TUI handoffs before reacquire * fix: preserve handoff stage after retry settlement --------- Co-authored-by: Merge Sim <sim@local> |
||
  
|
c300913f90 |
fix(mobile): stop double-scaling commit timestamps in history rows (#17731)
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com> |
||
|
Brennan BensonandMerge Sim
|
1ae7aa8bb4 |
feat(native-chat): resume an Agent Session History row into a new structured chat (#19176)
* feat(native-chat): resume an Agent Session History row into a new structured chat A Claude or Codex row in Agent Session History gains "Resume in New Chat": it opens a new structured native-chat tab that continues that provider conversation, with the prior turns already in the journal. Until now those rows could only be resumed into a PTY terminal; the structured branch could reveal a chat Orca already owned but could not adopt one it had never held. Almost all of the machinery existed. Both lanes already resume from the record's provider handle chain, the journal already has a transcript importer, and the handle chain already models `adopted` as an origin. The gap was that a create always minted an empty chain, so the adapters started a fresh conversation. This seeds that chain. The client names only the conversation. `agentSession.create` is reachable by paired mobile clients, so the transcript path and the account home are derived by the executing host and validated against the account homes it recognises — a client-supplied path would choose which file the host imports and which credential directory the provider child launches against. Failure refuses rather than degrades. A transcript that cannot be found refuses before anything is created; one that fails or decodes empty *after* the provider has resumed fails the attach, tearing the child down and publishing no tab, because an empty journal beside a context-carrying agent claims a continuity the provider never gave. Codex can resume into any workspace since it is handed the rollout path; Claude resolves transcripts under a project key derived from the launch cwd, so it is offered only for the workspace the conversation was recorded in. * fix(native-chat): widen adopted-home discovery and keep ordinary launches untouched Three corrections from review of the first commit. The adoption's account-home candidates now include the extra Codex homes session discovery already scans. A row this host listed could otherwise refuse to resume, which reads as the feature being broken rather than as a scope. Ordinary launches call `createStructuredAgentSessionLaunchIntent` with two arguments again. Passing the resume source unconditionally appended a trailing `undefined` that four existing call-site assertions had to absorb; the churn was the caller's fault, not the tests'. The transactional adoption guard's comment claimed the self-exemption is what lets a committed create replay. It is not: replay is settled earlier by the operation ledger, and an adoption always arrives with a null expected fence, so a request naming an existing session id is refused a few lines below either way. The exemption is part of what "another record" means, and the comment now says that instead. * fix: preserve history adoption through create and retries * fix: replay committed history adoption from durable identity * fix: validate history before claiming adopted sessions * fix: extract AI vault resume domains * fix: recognize typed history resume refusals --------- Co-authored-by: Merge Sim <sim@local> |
||
 Jinwoo Hong
|
d53cbed43f |
revert: hold mobile push feature for user testing (#19203)
Reverts
|
||
|
Brennan BensonandMerge Sim
|
f1d8545024 |
feat(chat): support structured /clear and /compact commands (#19164)
* feat(chat): support structured clear and compact commands * fix(chat): authorize mobile commands and bound clear-chain projection * fix(chat): localize conversation command send errors * fix(chat): retain clear pane identity with reopened history * test: account for combined structured session RPC additions --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
bf4e270504 |
fix(native-chat): list the slash commands and skills a structured Claude session actually loaded (#19127)
* fix(native-chat): list the slash commands and skills a structured Claude session actually loaded The chat composer's `/` menu was built from a curated five-command catalog plus a host disk scan of skill roots. Neither is what the running session can do: the session reports its own `/` surface, which carries this repo's `.claude/commands`, the skills that only reach it through plugin roots, and a hide-list of commands that mean nothing outside a terminal UI. On one local session the menu offered 6 commands and 17 skills where the session reported 62 commands and 33 skills. Read that surface per session and let it drive the picker: - A per-session catalog seeded from the frame that proves the session and kept current by every later report, exposed over a new `agentSession.commands` read. - The report is the authority on WHICH skills exist; the disk scan stays the source of scope and description for the names both know about, so a skill the session never loaded is no longer offered and one it loaded from a root the scan cannot see now is. - A host that predates the read answers `method_not_found` and the composer keeps its curated catalog, so mixed versions and the PTY lane are unchanged. * test: register agentSession.commands on the three surface ratchets The structured method count, the mobile allowlist, and the cross-version call table each enumerate the agentSession surface on purpose, so an additive method has to be declared in all three rather than counted around. * fix: preserve session catalog authority and publish live updates * fix(native-chat): publish authoritative command catalogs on session updates * fix: seed Claude slash catalog before the first prompt * test: verify unclassified catalogs survive session publication * test: complete structured rename journal fixtures --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
546fd9b21f |
fix(native-chat): remember structured chat model and effort picks (#19147)
* fix(native-chat): remember structured chat model and effort picks Structured Claude and Codex sessions already read the saved launch options at create, but nothing ever wrote them back. The only writer of `nativeChatSessionOptions` was the PTY picker, and the composer swaps in the structured surface for structured panes, so a structured pick went nowhere: it was forgotten when the session ended and every new session started at the CLI default. Persist a settled pick from both the desktop and mobile structured surfaces. Model and effort are stored as a pair, because a launch resolves a stored effort only under a stored model — so an effort-only pick adopts the model it was chosen against, otherwise the remembered effort never reaches a launch at all. Two things the persist path deliberately avoids: it writes what the provider committed rather than what was requested, since Codex reconciles an effort the newly selected model cannot run; and it never writes the provider readback, which is the CLI's own default and would pin a `-m` the user never chose. * fix(native-chat): persist session option picks atomically --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
68dd3909c7 |
feat(orchestration): orchestrate native-born structured chat sessions (#18827)
* feat(orchestration): orchestrate native-born structured chat sessions Orchestration resolves every worker through a terminal handle and a pane key backed by a live PTY. A session created directly as structured has neither, so it was not refused by orchestration — it was invisible. A coordinator could not start one, address one, or receive `worker_done` from one. Add a second authority source rather than a parameter channel. A registry maps a session id to the same three facts the PTY path supplies — a bearer handle, a pane key and a host scope — and the four runtime getters consult it before giving up on `ptysById`. `orchestration.send` and `verifyDispatchCapability` are untouched: authority stays host-derived and the CLI still cannot assert who it is. PTY handles short-circuit on the handle prefix, so the terminal path is unchanged. Mail travels as a session turn instead of as bytes, on a sibling lane that keeps the PTY lane's outstanding-run, waiter, reserved-type and batch rules. Orchestration's database stays the source of truth; the send is best-effort, exactly as the byte write is, and mail is consumed only on a proven-accepted dispatch. Delivery waits for the session to be between turns, because one provider refuses a mid-turn start outright and the other cannot acknowledge one inside the ack window. Security properties, each pinned by test: the pane key's leaf is random and persisted rather than derived, since `check` is identity-gated and accepts a caller-supplied pane key; the handle is a random bearer token; the child env carries no pane key, which would otherwise flow into hook pipelines that assume a PTY leaf; hook attestation stays closed for structured handles; and process continuity comes from record lineage, never the runtime fence, which the host bumps during its own crash recovery. Also remove the "Orchestration paused" notice, which gated only on dispatch status and rendered over bridge chat where orchestration always worked; refuse the implicit-sender fallback when a worktree has more than one candidate leaf instead of guessing; and collapse the archive kinds to one named type with a compile-time assertion that the capture set cannot drift ahead of the storable set. * fix(orchestration): answer the structured idle gate from the reduced timeline The structured pointer gate read a bounded 40-item tail page. A settled turn is tombstoned rather than rewritten, so an idle worker with any real history carries no turnLifecycle item at all and the "full page, no lifecycle item" guard read it as busy forever: every nudge after the worker's first substantial turn parked on a settle edge that had already passed, and the preamble tells workers not to poll. The attention gate had the mirror bug — a prompt older than the tail window was missed and the nudge was delivered into a session blocked on a human. Both facts now come from `journal.snapshot()`, the fully reduced timeline, via a new narrow `readGateFacts` host read; the policy module stays pure and still projects through the shared helpers the chat view reads. Also: - Park `session-not-attached` on the journal edge, so mail that arrives during a transient detach is redriven by the re-attach reset instead of sitting unread. - Resolve a structured worker's provider from the durable agent-session record when the registry entry was rehydrated, so a restarted Codex worker is no longer reported and archived as Claude. - Clear `structured_pointer_operations` in every `orchestration reset` scope. - Drop the per-chat-pane dispatch-status store subscription left behind by the removed paused notice, and re-pin the two terminal-pane ratchets it moves. - Hoist the identical pointer batch selection out of both delivery lanes into `selectOrchestrationPointerBatch`. - Refuse the pre-graph-ready focus-based guess for `requireUnambiguous` callers, matching the ready path. - Move the host teardown phase list into the teardown module it belongs to, which is what keeps the host inside its max-lines budget. * fix(orchestration): discard a structured worker session whose create settled unknown `commitStructuredAgentSessionCreate` answers `agent_session_operation_unknown` when `attach` SUCCEEDED and only the tab publish failed, so `created.ok === false` is not proof that nothing exists. The worker start read it that way and skipped `discardCreatedSession`, leaving a live provider child that took no hold, has no `bindingsByDispatchId` entry and no published tab — the outer `releaseStructuredWorkerSession` no-ops without a binding, and a session that never had a holder never starts the eviction clock, so nothing in the runtime ever retires it. A throw out of the commit half is past `attach` for the same reason; the pre-commit half refuses rather than throwing. Cleanup now asks whether the create MAY have committed, via the existing `isDefinitiveAgentSessionCreateRefusal` predicate. Also: - Strengthen the pre-ready `requireUnambiguous` test so it actually pins the guard: the snapshot now carries a focused terminal, so deleting the `? [] :` ternary turns the test red instead of leaving the refusal to the ambiguous `listTerminals` fallback. - Correct the guard's justification comment, which cited `orchestration check` as covered. `check` resolves through the `--terminal` scope and still guesses; the guard covers the implicit `--from` sender, and a structured worker is covered by the `ORCA_TERMINAL_HANDLE` baked into its child. * docs(orchestration): stop two structured-worker comments claiming guarantees the code does not give The send-time owner re-check reads `target.refusal`, the snapshot the resolver already admitted, so `decideStructuredPointerDelivery` can only agree with the resolve-time answer and `owner-not-settled-native` is unreachable from that call site. What actually fences an owner that moved is `expectedRuntimeFence`, which a handoff bumps. Say that, so nobody later drops the fence trusting a re-check that is structurally a tautology. `discardCreatedSession` was credited with retiring "a published background tab that no dispatch owns". It hides the DURABLE tab reference and closes the session; the live tab snapshot keeps the row, so the background tab this start published stays on screen until the app restarts. Same for stop and release. The comment now describes what the two calls do — including that both are no-ops on a session that was never attached, which is what makes the non-definitive-refusal path safe to reach unconditionally. * fix(orchestration): retire a structured worker's chat tab when the worker settles Starting a structured worker always publishes a real `agent-session:<id>` tab, but every settlement path only called `setSessionTabVisibility(sessionId, false)` plus `host.close(sessionId)`. That clears the DURABLE restore index and leaves the LIVE snapshot untouched, so stop, release and the half-started discard all left a dead "Claude Chat" / "Codex Chat" tab in the worktree's tab bar for the rest of the app session — five dispatches, five dead tabs — and opening one re-attached the released session, respawning a provider child outside orchestration's hold accounting. The snapshot-pruning half of `closeStructuredAgentSessionTab` is extracted into `structured-agent-session-tab-retirement.ts` and exposed on the runtime as `retireStructuredAgentSessionTabFromSnapshot`, so the user-initiated tab close and the three settlements share one implementation instead of a second copy. The settlement side is best-effort BY CONSTRUCTION: it runs only after the close is already proven, calls the runtime method optionally, and swallows any throw. It talks to no renderer, so the startup release reconciler can call it too. Nothing here can turn a proven stop into `release_unknown`. * fix(orchestration): stop a structured worker's nudges, archive and liveness from lying Five defects in the structured-worker lanes, each with the same shape: a check that answered from something other than what it claimed to measure. - The pointer lane gated a WORKER's `dispatch:` mailbox on its RUN's outstanding delivery. Delivery rows exist only for a `run:` address, so that row belongs to the coordinator — and a coordinator holds one for exactly as long as it is acting on received mail, which is when it replies to its workers. The gate is gone; there is no coordinator mailbox in this lane to protect. - `dispatch-rejected` now parks on the journal edge. A rejection consumes no mail and nothing else redrives the mailbox, so an unparked pointer left the worker idle on durable mail until unrelated mail happened to arrive. - The released journal archive bounded forward — keeping the HEAD — before capping newest-first, so a long worker's archive ended at its early exploration and dropped the answer it was released for, under a warning that said the oldest messages had gone. One newest-first pass now, and the warning is true. - The durable pointer operation id was reused on a matching BODY fingerprint, and the body names only the unread count. Two unrelated same-size batches collided, the host replayed its ledger answer as `accepted` with no turn sent, and the lane marked the new mail delivered. Reuse is keyed on the batch's message ids. - `worker-read` on a structured worker hardcoded `terminal: 'running'` and emitted no `liveness`, so a runtime that could not see the session reported the worker as alive. It now carries the observed verdict, as the PTY branch does. Also: the live journal cursor is an index into a re-derived tail window, so the page's oldest item joins its source identity — a slid window now answers `source_changed` instead of silently resuming past the items it skipped. And a stop that reached no host reports `processAction: 'none'`, after installing the host the way release already does. * fix(orchestration): stop a released structured archive claiming a close that never landed `worker-read` on a released structured worker hardcoded `liveness: 'exited'`. The archive is frozen BEFORE the close, so it proves nothing about the provider child, and the read is served for `release_state` in `releasing` / `unknown` too — the two states that exist precisely to record a close that did NOT land. A coordinator that read `exited` from a `release_unknown` worker would start a replacement over the same worktree while the original child was still attached, which is the outcome docs/reference/ssh-execution-boundary.md rule 2 exists to prevent, and it contradicts the release receipt's own "the structured session close was not proven" text. The verdict now comes from the resource row the read already holds: only a settled `released` row is `exited`, everything else is `unverifiable` — which the existing mapping renders as `terminal: 'unknown'`, the same way the live branch does. * fix(orchestration): stop a structured worker-start reporting a preamble it never delivered Two ways a structured `worker-start` handed the coordinator a receipt that did not describe the worker it got. `sendStructuredWorkerPreamble` threw only on a refusal and on `rejected`, so a submission that settled `unknown` fell through as success: the start pushed `dispatch_input: accepted` and marked the dispatch ready. `unknown` is not rare — `dispatchSafely` converts ANY thrown adapter call (provider child gone, transport dropped, ack window missed) into it, and `performSend` still returns ok. The worker then has no task spec while its coordinator blocks in `check --wait --types worker_done` until timeout. This PR's own mail lane already states the rule — "`pending` is not yet an acknowledgement; only `accepted` may consume mail" — so the preamble now applies it too, and raises `operation_unknown` for the states that prove neither delivery nor failure, which is the code `failWorkerStartWithReceipt` turns into the `outcome_unknown` receipt whose nextCommands send the coordinator to look. `rejected` stays a proven failure. `--structured` also accepted `--model` / `--effort` and dropped them: structured session creation takes no launch preferences, while `launch.receipt.effective` echoes whatever was requested either way, so `--model opus` ran on the workspace default and the receipt still said `opus`. Refused now, for the same reason `--terminal` refuses them, and the spec note records that refusal along with the new-child/new-top-level one it never mentioned. Tests: the refusal guard had no coverage at all, and `structured-mailbox-pointer-host` — where the full-timeline gate read lives — had none either; reinstating the bounded tail there left the whole repo green. Both are covered now, and the vacuous "never selects an exact provider session" case is re-pointed at the absent `ORCA_PANE_KEY` that actually keeps that selector shut. * fix(orchestration): let a structured worker actually reach the Orca CLI, and stop four settlements lying A structured worker's provider child runs `orca orchestration ...` exactly like a PTY worker's agent does, but it was handed the ambient PATH. On packaged Linux the CLI installs as `orca-ide` so it never claims GNOME Orca's /usr/bin/orca (#7904), so bare `orca` execs the screen reader and the worker can never read mail, reply or send worker_done; on packaged macOS/Windows the bundled launcher is only reachable from the app's own resources dir. The PTY lane already solves this inside `buildPtyHostEnv`; that block is now its own module and both lanes call it. Also: - a worker start that fails AFTER its session exists now discards the session, so a failed start stops stranding a dead chat tab that the durable restore index republishes on every launch; - a structured worker's resource reconciles to `released` after settlement forgot its identity, instead of answering `unverifiable` for the life of the DB; - `closeAttempted` is set only once a close is issued, so a tab-visibility failure can no longer report `closed_agent_terminal` for a running child; - `forgetSession` prunes only what the settled worker parked, not every sibling whose target momentarily fails to resolve; - release settles with an explicitly empty, warned archive when the journal is unreadable AND the session is proven exited — closing the chat tab is routine, and `archive_failed` there wedged release on evidence that could never arrive; - the new migration test uses mkdtemp and cleans up, so it stops failing Windows CI and leaking. * fix(orchestration): merge the duplicated release-receipts import The release-completion module imported ./orchestration-worker-release-receipts twice, which trips import/no-duplicates in audit:code-quality:native. The changed-file gate does not load that config, so only whole-tree CI saw it. * docs(runtime): note that a background structured tab re-publish is a no-op The activate:false branch for an already-published session returns without writing the snapshot or emitting, so it cannot re-surface a client whose mirror lost the tab. Orchestration is safe from this only incidentally. * feat(orchestration): make the worker mode the user's own default, not a flag `worker-start --structured` was an explicit opt-in that REFUSED --on, --terminal, --model/--effort and worktree-creating placements. The flag, its spec entry and the `structured` RPC param are gone: the mode now follows the user's setting for new agent tabs, so a local claude/codex worker is a structured chat session whenever the user's own default says agent tabs open as one. A setting is a preference, not a demand, so none of those combinations refuses any more. A dispatch that cannot be structured starts an ordinary PTY terminal worker and the receipt names the mode that ran and why, so the fallback is never silent: - a remote --on, an existing --terminal, a new-child/new-top-level worktree and --model/--effort are decided from the request; - the agent, TUI launch customization, Codex-on-Windows and the runtime capability are decided by the shared launch route; - WSL, remoteness and the Windows start-time gate are settled by the executing host's own agentSession.createSupport, asked once the worktree resolves and before anything is created, so a refusal is a terminal worker rather than a failed start. The decision is the renderer's, lifted rather than copied: `resolveAgentLaunchRoute`'s structured half and the settings predicate now live in shared/structured-native-chat-launch-route, which both surfaces call, and the TUI launch customization test moves to shared beside it. `getClientSettings` gains the two native-chat default booleans it was missing. No security invariant moves: the structured worker registry, bearer handle, persisted pane key, the absence of ORCA_PANE_KEY from the child env, hook attestation and lineage-derived process incarnation are untouched. * fix(orchestration): stop the worker mode leaking into the agent contract The mode a worker runs in is a runtime implementation detail. An agent should be taught the same verbs, run the same commands and read the same receipts whether it is a structured chat session or a PTY terminal — otherwise a settings-driven fallback silently changes what the agent can do. The real leak was `canDispatchSubWorkers`, which was forced false for a structured worker. That was not a wording choice: `worker-start` resolved `--from` through `showTerminal`, which needs a live PTY or renderer leaf, so a `structworker_` coordinator genuinely could not dispatch. Rather than withhold the capability, the one fact the command needs from `--from` — its worktree id — now comes from `getOrchestrationDispatchAuthority`, the same authority the pane-key and process-incarnation getters already answer structured handles from. Sub-dispatch is gated on depth alone, identically for both modes. `showTerminal` itself is deliberately NOT taught structured handles: it returns a ptyId, a leaf id and a pane runtime id, and synthesising those for a session with no PTY would hand every caller of a public terminal verb something that looks writable and is not. `inspectWorkerTerminal` already returns `terminal: null` for exactly that reason. Also neutralised three agent-visible refusals that named the worker's kind: a `worker-read --source terminal` on a worker with no terminal now names the sources that do work, and both archive refusals say "transcript output" rather than "structured chat output" (the PTY `transcript_pin` branch said "structured" too). New tests pin both properties: the two preambles are byte-identical once the handle and per-dispatch ids are normalised, and a structured coordinator starts a worker with `showTerminal` rejecting. * fix(orchestration): stop claiming a structured worker was checked for a prompt worker-show reported observation.agentWait: null for every structured worker. The field's own contract says null means Orca looked and found no wait, and absent means it never looked — and nothing looks here: a structured worker parks on a journal question item, which no terminal prompt scan can see. So null was a false negative on the one field a coordinator is explicitly told to read, and it was mode-dependent: the same worker as a PTY would have reported the wait. Absent is both the honest value and a state a PTY worker already reaches (an older host, an unreadable pane, a probe that did not answer), so it discloses nothing about which mode ran. * docs(cli): stop the worker-start spec pointing a caller at the worker kind The note said "the receipt mode field names the mode used and why", which is an instruction to read a field no verb behaves differently for — the one thing the mode was not supposed to become. It now says what a caller actually needs: the dispatch always starts, the options passed are the ones honoured, and every worker is driven the same way. The receipt still carries the mode for operators and telemetry; nothing tells an agent to look at it. * perf(orchestration): coalesce the structured redrive edge Every journal batch is a redrive candidate, because a settled turn is tombstoned rather than rewritten — there is no completed row to watch for. That is free while nothing is parked on the session, but once mail IS parked each batch re-resolved the dispatch, queried unread mail and read the host's gate facts, only to re-park because the turn was still running. A turn streaming tool calls paid that per batch. The edge now coalesces on a 300ms quiet window with a 2s starvation cap, so a streaming turn costs a handful of evaluations instead of one per batch and a settled turn still nudges promptly. Delivery semantics are untouched: the gate, the accepted/rejected/unknown handling and the retain rules all still run exactly as before, just fewer times. Nor is this the path fresh mail takes to an idle worker — that is `deliverForHandle` at enqueue time, which this does not touch — so the common case gains no latency. The mechanism is the session.tabs notify coalescer, generalised into `keyed-trailing-edge-coalescer` and called by both rather than duplicated; the session.tabs windows stay where they were, since 50ms is right for a spinner title and far too tight for a journal stream. Disposal drops the pending timer rather than flushing it, on the existing subscription disposer that every settlement already reaches, so a redrive can never fire for a session no dispatch owns. * fix(orchestration): deliver direct peer mail to a structured worker, and let a peer read it Two agent-to-agent verbs had no answer for a worker that IS a structured agent session, and both failed quietly. Mail addressed to a worker's own bearer handle — how agents mail each other outside a dispatch — fell between the lanes. The send stored durably and reported success, `getLiveTerminalPaneKey` resolved the recipient, and then neither lane claimed the mailbox: the structured resolver answered only `dispatch:` addresses, and the PTY lane refuses a structured handle outright. Nothing errored and nothing logged, so the worker never reacted and the peer waiting on a reply hung. The resolver now also answers a bare worker handle, preferring that worker's active dispatch so peer and coordinator nudges share one operation-ledger budget. A worker BETWEEN dispatches is still nudged, under a session-scoped key: a dispatch says nothing about whether delivery is safe — the idle gate and the lease fence do — and its own `check` reads exactly the direct mailbox the mail is sitting in. The dispatch caller key is left byte-identical, because the ledger is keyed on (callerKey, operationId) and reshaping it would re-mint nudges already in flight as second turns. `terminal read` had no structured branch, so the only peer-accessible read verb answered `terminal_handle_stale` for a live worker; `worker-read` is closed to a peer, which holds neither coordinator standing nor a dispatch id. It now serves the session's journal, projected to LINES and paged by the same reader the PTY tail uses, so the result stays a plain RuntimeTerminalRead and nothing an agent reads discloses which kind of worker answered. Bounding and dispatch-capability redaction are the archive path's, reused rather than rebuilt. A session that is not attached refuses with the existing not-attached code rather than returning an empty tail, which would read as "this worker has said nothing". `terminal.show` still refuses a structured handle. This is read-only on purpose: synthesising a ptyId/leafId/paneRuntimeId would hand every public terminal verb something that looks writable and is not. * fix(orchestration): stop three PTY-only probes answering for structured sessions Three defects, one shape: a probe that enumerates PTYs or resolves a pane was standing in for a question that is not about panes at all. `worktree rm` destroyed a live structured worker. `killAllProcessesForWorktree` sweeps the renderer graph, the provider session list and the local pty-registry, and a structured session is registered on none of them — so all three counted zero, nothing errored, and removal deleted the checkout out from under a running provider child, which kept running with its `cwd` gone while the dispatch still reported the worker live and exact. A fourth sweep now asks what the other three cannot: membership by `location.workspaceId`, which covers a plain chat session as well as a dispatched worker, and liveness by the same `live`/`unverifiable`/`exited` observation the rest of the structured surface uses. It REFUSES a destructive removal rather than auto-closing, on the same bargain and the same `--force` escape hatch as the unstopped-PTY gate — this is the verb that deletes a user's work, and a running agent is exactly what they would want to be told about. Force closes the sessions properly instead of orphaning a child. Best-effort reconciliation callers are excluded: they repair state, delete nothing, and must never be failed closed. Twelve coordinator verbs failed for a structured worker running as itself. `isLiveTerminalHandle` validated `ORCA_TERMINAL_HANDLE` with `terminal.show`, a PTY verb whose leaf lookup misses for a session that never had a pane; the pane remint that would have recovered it needs `ORCA_PANE_KEY`, which a structured child deliberately does not carry, so every one of them died on `no_active_sender_terminal` — including the ones the worker's own dispatch preamble tells it to run. The identity question gets its own probe, `terminal.resolveIdentity`: a handle and a boolean and nothing writable. `terminal.show` still refuses a structured handle, because synthesising ptyId/leafId/paneRuntimeId would hand every public terminal verb something that looks writable and is not. The PTY half is byte-for-byte today's check, `getLiveLeafForHandle` included, so its `rendererGraphEpoch` re-check still runs — that check is the whole reason the sender is validated at all, and a cheaper probe would have quietly started passing stale post-reload handles. A host that predates the method answers `method_not_found` and the client falls back to `terminal.show`, which is correct for that host: one without the identity probe has no structured workers to miss. `dispatch --inject` reported `no_agent_detected` for a structured worker, because `isTerminalRunningAgent` reaches `getLiveLeaf`, throws, and the catch returns false. A structured session IS the agent; there is no foreground process to recognise, so it answers before the PTY probes rather than through them. Also: a Run whose coordinator is structured now gets its `run:` mail. Both lanes declined and neither logged — the PTY lane because the owner is structured, the structured lane because the mailbox was not `dispatch:` — so each half believed the other owned it. The PTY lane's reasoning (a coordinator blocks in `check --wait`, where a waiter preempts pointer delivery) does not transfer: a structured coordinator is a chat session whose turn ends. Its `run:` deliveries take the `hasOutstandingRunDelivery` gate the PTY lane applies for exactly that mailbox, and only for that mailbox. The test that would have caught the twelve drives the CLI with `ORCA_TERMINAL_HANDLE=structworker_…` and no `--from`. Every existing orchestration CLI test passes `--from` explicitly, so the resolver a real worker goes through was never exercised — which is why the suite stayed green while the preamble failed on its first line. Two files crossed their line ceiling and are split rather than waived: `worktree-teardown.ts` sheds its two PTY-surface sweeps and the deadline arithmetic they share, and `orchestration.test.ts` — which sat exactly on 800 — sheds the two caller-identity suites this change rewrote. * fix(orchestration): arm the takeover signal for structured chat input `worker-release` closed a structured session a user had taken over, losing work mid-conversation, while `orchestration-worker-specs.ts:106` promised "Never closes … user-taken-over terminals". Every guard was already correct and simply never armed. `reportWorkerTerminalUserInput` has exactly one call site — the real-user-input signal on a PTY connection — so structured chat input never reached `orchestration.workerTerminalUserInput`, `markWorkerTerminalUserOwned` never ran, ownership stayed `owned` instead of `user_owned`, `retainedReason` never returned `user_takeover`, and `stopStructuredWorker` proceeded. The durable flag is reused as-is rather than given a parallel mechanism: it exists precisely so a restart, an SSH drop or a renderer remount cannot erase a takeover. Addressed by SESSION, never by pane key. A structured worker's pane key is a random identity credential — anyone holding it can read and consume that worker's mailbox, and session ids are embedded in tab ids in plain text — so it stays in main and the runtime resolves the session to it. Handing it to a renderer to echo back would make it learnable by anyone who can see a chat pane. The RPC gains an optional `sessionId` alongside `paneKey`; a host that predates it rejects the call, and the report is already best-effort with a catch, so that host degrades to exactly today's behaviour rather than failing a send. The signal fires from the composer send hook and only past `accepted`: the outbox dispatcher retries, and orchestration's own pointer nudges never pass through the composer at all — so neither can be mistaken for a user takeover. * fix(orchestration): reach structured workers through group addresses `orca orchestration send --to @all` — and `@idle`, `@claude`, `@codex`, `@worktree:<id>` — silently skipped every structured worker. Recipients came from `listTerminals`, which enumerates leaves and PTYs, and a structured session is on neither. The exclusion happened BEFORE per-recipient resolution, so the `SendRecipientWarning` machinery never ran: the caller got exit 0 and a receipt naming the workers that did resolve, and a broadcast "stop work" or "base moved" reached the PTY workers and nobody else. With every worker structured it degraded to `terminal_not_found`, which reads as "the group was empty". Fixed at the group-resolution site rather than inside `listTerminals`. That result is published to paired mobile and remote clients and to consumers that assume a summary carries a `ptyId` or is writable, so widening it is its own change under `docs/reference/remote-wire-compatibility.md`. Group addressing reads exactly three fields off a recipient, and `RuntimeTerminalSummary` already satisfies them structurally, so the resolver widens to that smaller shape and nothing here invents a `worktreePath` or a `branch`. Candidates are liveness- gated on the same observation the rest of the structured surface uses — mail addressed to a settled worker would be stored for a lane that will never deliver it — and once a worker IS a candidate, the existing per-recipient warnings cover it, so an unresolvable one is reported rather than dropped. `@idle` needed more than enumeration: `getAgentStatusForHandle` reaches a PTY probe that throws for a handle with no pane, so a structured worker would have been enumerated and then silently dropped from the one group address that selects on status. It now answers from the session's journal — and off the FULL reduced timeline, never a bounded tail. Settlement tombstones the running turn's lifecycle item rather than rewriting it, so on any page-sized read a long tool-calling turn looks identical to an idle session; `@idle` would then broadcast into a running turn, which Codex answers with `turn already running` and Claude queues behind. An unreadable session answers null, never idle. `terminal list` and `worktree ps` still omit structured workers; that is the wire-visible half and is deliberately not in this change. * fix(orchestration): refuse rather than guess when a chat session has no identity An ordinary structured chat session — not a dispatched worker — is spawned with no `ORCA_TERMINAL_HANDLE`, because `structuredWorkerChildIdentityEnv` early- returns for any session outside the worker registry. `orca orchestration check` then fell through to `terminal.resolveActive`, which picks the focused tab's active leaf or the first leaf in the worktree. It returned a valid handle, so nothing errored — and `check` is destructive by default, so it consumed another pane's oldest unacknowledged batch and marked it read. The rightful worker never saw that mail. `requireUnambiguous` does not fix this, only narrows it: it refuses when MULTIPLE leaves could be meant, and with exactly one terminal pane in the worktree the guess still resolves — to a sibling. "One terminal pane plus one chat tab" is a normal layout, so the common case stayed broken. The pinned test is that case. So the child now carries `ORCA_STRUCTURED_SESSION`, and every remaining route that would GUESS an implicit terminal refuses on it with an error naming the flag to pass. The marker names NOTHING — no handle, no pane key, no session id, no token — which is the whole reason it is safe: it cannot be replayed, cannot impersonate, and cannot flow into the hook-attestation, agent-row or mobile-projection pipelines the way a pane key would. That makes it a different decision from withholding `ORCA_PANE_KEY`, not a reversal of it. It also grants no CLI reachability, so packaged builds keep exactly today's exposure. The comment at `orca-runtime-adopt-terminal-orphans-from-inventory.ts` that justified the guess — "a structured worker is covered instead by the `ORCA_TERMINAL_HANDLE` its child is spawned with" — was true only for dispatched workers and false for every other structured session, a population this branch creates. It now says which case it covers and which case it does not. * fix(orchestration): stop two surfaces lying about a worker with no terminal `orca terminal <verb>` answered `terminal_handle_stale` for a structured worker's handle. Nothing went stale: the session is live and simply has no terminal, and it never had one — so callers acted on a false claim and went hunting for a remint that cannot exist. The refusal now carries its own code and names the structured equivalents (`orca terminal read`, `worker-read --source transcript`, `orca orchestration send`), so an agent that lands there learns what to run rather than what failed. A PTY handle that really did go stale keeps the old error, and so does a session this runtime no longer owns — that handle IS dead. `terminal.show` stays non-resolving: synthesising a ptyId/leafId/paneRuntimeId would hand every public terminal verb something that looks writable and is not. `orchestration-worker-specs.ts` promised "the same verbs, the same handle, and the same worker-read sources", and all three clauses were false for a worker with no terminal. A spec agents read must not carry a false promise, so it now states the limitation and the alternative that always works. Note this had to be reconciled with an invariant this branch already holds: the worker MODE must stay opaque, or a coordinator starts branching on something no verb it runs behaves differently for. So the note says "not every worker has a terminal" and points at `--source auto`/`--source transcript` WITHOUT naming a kind — the same mode-neutral wording `readStructuredWorkerOutput` already uses when it refuses `--source terminal`. Both properties are now pinned by tests, so neither can be restored by breaking the other. * fix(orchestration): close the review findings on the structured parity work Four defects and two follow-ups from the delta review. The `worktree rm` refusal was a dead end in the desktop UI. Its message matched no matcher in `classifyWorktreeForceDeleteReason`, and an ordinary desktop delete already passes `force=true` for the dirty-file skip, so classification returned null unconditionally: the toast showed raw CLI wording with no Force Delete button, and a user with a live chat session was stuck unless they knew to reach for the CLI. That is the #11960 shape `shared/worktree/removal.ts` documents, so the refusal now has its own prefix, matcher, `WorktreeForceDeleteReason` and toast copy, classified BEFORE the `force` guard and nulled once the waiver is spent — exactly how `unstopped-pty` is handled, with matcher and hint kept in the same file as that contract requires. The copy says Force Delete will close a running conversation rather than borrowing the "could not confirm" wording, because Orca watched these sessions stay attached; there is no doubt to waive. Structured `terminal read` cursors were unsound and are now refused. The PTY cursor indexes an append-only completed-line buffer with a monotone count; a session journal is a BOUNDED tail re-projected on every read, so a saved index addressed different lines as the journal grew — and `truncated` could never fire to say so, because it tests `cursor < oldestCursor` and `oldestCursor` was always 0. A poller got wrong or duplicated lines under `truncated:false`. Separately, a streaming turn's lines counted as completed with `partialLine` hardcoded empty, so a mid-turn cursor consumed a half-written line whose growth was never redelivered — the `"hel"`/`"hello"` hazard the PTY reader guards against. The journal does have stable item identity, but `terminal.read`'s cursor is a number on the wire and cannot carry it, so a cursor read now refuses and names `worker-read --source transcript`, which already has that contract including `source_changed`. No cursor space is advertised either: `nextCursor` is null and the cursor fields are absent, rather than claiming an index the next read cannot honour. The header claim that all four fields kept their meanings was true of the shape and false of the invariants; it now says which ones hold. Two fixes had no test at their real seam, which is the same failure that produced this whole set — the runtime tested directly, the seam tested by neither. The group-addressing test hand-composed the recipient list itself, so deleting the composition at the call site left it green; it now drives `sendGroupMessage` with no PTY terminals at all. Nothing referenced `isLiveStructuredAgent`, so the `dispatch --inject` fix had no red-then-green at all; it now has one driving `RuntimeTerminalAgentPresence.isRunning`. Both were ablated and confirmed red. Folder-workspace removals sweep and kill PTYs without `requirePhysicalStop`, so the structured sweep no-opped there and left a live session bound to a workspace about to be forgotten. They now close best-effort under an explicit `closeStructuredSessions` flag, kept separate from `requirePhysicalStop` because the two questions differ: that one asks whether a stop must be PROVEN before files are touched, and it is what licenses a refusal. These paths do not refuse — the root is shared so no checkout vanishes under the child, and one of them is a never-throw forget a refusal would wedge. Reconciliation sweeps set neither and still close nothing. Also: the force close is raced against the same sweep deadline every PTY surface is bounded by, so a wedged provider close reports the timeout instead of hanging `worktree rm --force` forever; and the refusal now prints a count and the providers instead of raw session ids, which our own marker rationale treats as one tab-id hop from a credential. * test: pin structured-session close on the folder-workspace removal path The folder and orphan removal callers now pass closeStructuredSessions so a live structured session is closed best-effort rather than left bound to a workspace Orca has forgotten. These three exact-args characterizations describe that call and had not been updated. * fix(orchestration): stop the structured worker-read cursor misdelivering silently `worker-read --source transcript` for a structured worker fingerprinted only the oldest item's id, so `source_changed` fired when the window slid off the front and could NOT fire when the page's contents changed under a stable oldest item — which is the normal case, because the journal is a reduced, mutable timeline. A `running` tool item gains its `[tool result]` at its original sequence once later items exist, the 60ms delta coalescer revises a message in place, settlement can rewrite an item smaller, and a pending approval projects to null until it resolves and then appears in the MIDDLE of the array. Two silent failures followed, both returning ok. Omission: a caller handed a coalesced `hel`, resuming past it, never received the revision to `hello world` — the same defect we refused to ship on the terminal read path, already shipped here. Duplication: a resolved approval inserted ahead of a saved index, which was still accepted, so the caller re-read content it already had. The blast radius is the coordinator polling loop, the verb's primary consumer. The anchor is now the oldest item PLUS every item whose projected message sits below the caller's position, by id and revision. `createWorkerOutputSourceIdentity` already takes an arbitrary string array and the cursor is already opaque base64url carrying its own position, so neither the wire shape nor the `source_changed` contract changes. Prefix-scoped rather than whole-page deliberately: fingerprinting every item on the page would flip the identity every 60ms with the coalescer window during an active turn, making the cursor unusable exactly while the worker is working — that trades a silent bug for a useless verb. Tail growth the caller has not read cannot invalidate; any change to what it already holds does. Position-dependence is safe because `p` rides in the same opaque payload as the identity, and the returned cursor is stamped with the identity of its own end, which is precisely what the next read recomputes. The frozen archive keeps a constant identity: no item can be revised under a caller there, so it has no prefix to fingerprint. Both silent shapes are pinned across a page boundary with the journal mutating between reads — a static-journal test passes either way. Two ablations at the real call site: reverting to the oldest-item-only anchor turns both red, and widening the prefix to the whole page turns the tail-growth case red, which is what proves the scoping is real in both directions. * docs(orchestration): stop the structured terminal-read refusal recommending a dead end The refusal told a peer to "page it with `orca orchestration worker-read --source transcript`", which is wrong three ways and this file said so itself: its own header explains that this verb exists BECAUSE `worker-read` demands a dispatch id and coordinator standing "a peer does not have" — and then the refusal sent that same peer there. The verb it named is also a window index over the same bounded page, so it is not a paging answer even for a caller who can reach it; under load it now answers `source_changed` on most polls, which is better than the silent hole it had before but still not what the sentence promised. The refusal now says what actually works — the tail is bounded and newest-last, so poll it and diff — and names no alternative, because there is none. That is the honest framing: a durable cursor is not achievable here at all, rather than blocked on the wire shape. The journal is a reduced, MUTABLE timeline: an item's projected text changes at its original sequence after later items exist, the delta coalescer revises repeatedly, settlement can rewrite an item smaller, a pending approval renders as nothing and then as something, and `sequence` resets on epoch rollover. No index, numeric or opaque, survives that. So the docstring's "pagination with a real anchor lives on `worker-read --source transcript`" is gone too — there is no real anchor there — and the file now records why no windowed alternative should be built later: a broken cursor fails UNSAFE, as a silent hole in a poller's output, while diffing a bounded tail fails safe as a harmless re-read, and a second paging-shaped verb would invite the PTY assumptions this one cannot honour. The test asserted the old advice, so it now pins the contract instead: the refusal explains the working approach and must never name `worker-read`. `worker-read --source transcript` remains a good bounded snapshot for a coordinator reading a worker it dispatched; only the "or page it with" clause was false. * fix(i18n): add the missing worktree-removal agent-session refusal string The structured-session removal refusal introduced a translate() key with no en.json entry. Nothing local catches that: typecheck passes, and the full suite passes, because a missing key falls back to its inline default at runtime. Only verify:localization-catalog fails on it, which is why CI's static analysis reddened on a branch that was green everywhere else. Fallback wording mirrors the sibling unstoppedPtyLive string, since the two refusals differ only in what is still running and what Force Delete does to it. * test(codex): expect the no-identity marker on an unregistered structured child The refuse-rather-than-guess marker landed after these expectations were written, and all three assert exact env equality on the unregistered path — the one branch that now carries ORCA_STRUCTURED_SESSION. One of the two files was added by this same branch, so this is a self-inflicted drift; the other predates the branch and was broken by it. The marker's presence is still pinned positively by structured-worker-child-identity-env.test.ts and the CLI's orchestration-structured-session-no-identity.test.ts, so relaxing these three exact-equality checks loses no coverage of the security property. * fix(orchestration): require exit evidence before settling structured close --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
3160b54c69 |
feat: real background push notifications for the mobile app (#8129) (#18554)
* feat(cloud): add the mobile push gateway and its contract package (#8129) A small open-source service that holds the APNs key and FCM credentials and sends background push to paired phones on the desktop's behalf. Hosts authenticate with a box challenge and HMAC proof on their pairing key, the same shape the relay uses, so signed-in and accountless desktops share one path. Tokens are stored; alert text is held only for the coalescing window. The contract doc in docs/reference is the source of truth for every wire shape. The interop test runs the real desktop answerer against a real gateway-issued challenge so transcript drift fails in CI. * feat(push): register phones and send background push from the desktop (#8129) Adds the notifications.remote-push.v1 capability, the registerPush and unregisterPush RPCs on the mobile allowlist, a gateway client with a cached session and 401 re-auth, a durable unregister outbox, and a dispatcher that offers every mobile notification to the gateway after the socket fan-out. The dispatcher is fire-and-forget with one retry and drops registrations the gateway reports dead. Puts agentState on the mobile frame and fixes the #4375 wording so a working agent is never announced as finished. The relay host-proof code moves onto a shared envelope module with no behaviour change. * feat(mobile): background push registration, receive, and settings (#8129) Fetches the native APNs or FCM token, registers it with every paired host that advertises the capability, and re-registers on token change. Foreground pushes are suppressed inside handleNotification against the same seen set the socket path uses, so nothing shows twice. Taps route by host fingerprint. One Background notifications switch, off by default, with the disclaimer and needs-input / finished sub-switches; hidden until a paired desktop is new enough. Adds google-services.json and the expo-notifications plugin. * chore(cloud): Terraform and deploy workflow for the push gateway (#8129) Declares the Cloud Run service, runtime account, secrets, and orca_push database behind push_gateway_enabled, true only in production. The deploy workflow is gated like the relay's, deploys with no traffic, probes /ready and a validate-only FCM send, then shifts traffic. It runs as the shared production deploy account because the Cloud SQL rollout lease grant is foundation-owned; its extra authority is three bindings on the push service. docs/push-gateway.md carries the import commands for the resources created by hand and the APNs key rotation procedure. * docs: describe background notifications on the phone (#8129) * docs: check in the mobile push contract (#8129) Seven committed files cite it as the source of truth for every wire shape; docs/reference is allowlisted per file, so add the entry. * test(push): replay one checked-in host-proof vector on both sides (#8129) Cloud Verify installs only the cloud workspace, so the gateway suite cannot import the desktop answerer. Replace the cross-workspace import with a fixed challenge vector generated from the contract package; the gateway fixture and the desktop answerer each replay it and must produce the same HMAC. A transcript drift on either side now fails in that side's own suite. * fix(cloud): open the push gateway with invoker_iam_disabled, not an allUsers binding (#8129) The production domain-restricted-sharing policy rejects an allUsers run.invoker member, which the runbook anticipated. Opt the service out of invoker IAM the way the relay director already does; the host proof is the authentication either way. * docs(cloud): the push.onorca.dev record exists and is hand-managed (#8129) * fix(push): close review findings in the gateway (#8129) - Quota reservation takes a per-host advisory lock; READ COMMITTED admitted a whole burst past the cap (80/80 without, 60/80 with, against Postgres 16). - Challenge issuance no longer writes push_hosts; the row lands on proof verification. Stale hosts prune after 30 days. Per-IP token bucket on the two unauthenticated routes. - Streaming body limit via hono bodyLimit; a chunked body bypassed the Content-Length check. - registrationIds deduped in the schema; per-host device cap of 64; list bounded to its schema. - Gateway-side challenge TTL is the specified 10 s, not 40 s. - APNs stream settles on close as well as end/error. * fix(push): close review findings in the desktop client (#8129) - A gateway registration the registry cannot persist is enqueued for delete instead of leaking a live token. - Unregister outbox re-reads pending per pass, honours enqueues during a drain, and retries with backoff instead of waiting for the next launch. - Dispatcher batches registrations by 20 rather than starving the rest. - 401 compare-and-clear; a 401 after re-auth is unreachable; refused handshakes and 429s are cached briefly instead of re-handshaking per event. - Service is stopped on quit. * fix(mobile): close review findings in push registration and receive (#8129) - Consent generation guards a register that finishes after the switch went off; the host is re-queued for unregister instead of recorded live. - Foreground pushes seed the watermark before adopting the epoch, so a push on a never-connected session cannot wipe a valid watermark. - aps-environment follows the build via app.config.js; the iOS release workflow sets it to production. A bare plugin entry wrote development. - Pushes the OS showed while closed are marked seen before catch-up replay. - Token null result is not cached; failed capability probes are retried and never block an unregister; coalesced summaries are shown but not marked. - Unresolvable fingerprint routes nowhere and is suppressed in foreground. - Android channel ensured at boot; capability hook diffs clients by identity. * fix(cloud): harden the push deploy workflow and size the gateway to the budget (#8129) - Roll traffic back on a failed post-shift check; delete a candidate that never took traffic; retry the origin probe and the FCM probe. - Assert Terraform-owned scaling instead of mutating it from the workflow. - Build before taking the Cloud SQL rollout lease. - Declare the database pool in Terraform (2 per instance, max 2 instances) and add the gateway to the connection budget; the previous default put the shared instance 65 connections over its ceiling. - State plainly that the shared deploy identity's relay authority is inherited. * fix(push): read the runtime from shared state at push startup (#8129) Threading the runtime through launchDesktopMode put the launch module one line over the 300-line lint budget after the rebase. * fix(push): key the unauthenticated rate limit on the hop Cloud Run wrote (#8129) Cloud Run appends the connecting peer to x-forwarded-for; the limiter read the left-most value, which the caller controls, so a forged first hop earned a fresh bucket per request. * fix(push): close the final security review findings in the gateway and infra (#8129) - app.onError logs only the error name and answers a bare 500; hono's default handler printed the whole error, and a pg error carries the row in detail - a second per-IP bucket (240/min) runs ahead of the bearer lookup on every authenticated route, so forged bearers cannot spend the two-connection pool - one live session per host: minting deletes the host's earlier row - device-less hosts are pruned after 1 h, not 30 d; any keypair mints one free - notificationId is printable ASCII, since it becomes the APNs collapse header - the impersonated FCM probe token is masked in the workflow log - prevent_destroy on the Apple secrets and the orca_push database * fix(push): close the final security review findings in the desktop client (#8129) - fetch never follows a redirect: a 307 would replay the host proof and the phone's token to whatever origin the redirect named - registerPush params are strict and the paired identity is spread last - a per-device bucket (10/min) bounds a phone looping registerPush, which costs a gateway write and a synchronous registry write each time * fix(mobile): close the final security review findings in push receive (#8129) - a push with no epoch can no longer claim a seq-derived dedup key, in the foreground or from the tray; a forged seq:N could otherwise swallow the real bell at that seq - a provider-delivered push with no host catalog, or no fingerprint at all, stays unrouted instead of falling back to the hostId its raw data carries * docs(push): record the ip buckets, session and host retention, and the token-ownership limit (#8129) * fix(push): apply the schema on an untimed pool and retry statement-timeout aborts (#8129) Ports the relay's #18722 pattern to the gateway: DDL runs on a one-connection pool with statement_timeout 0 that is closed before the serving pool opens, and SQLSTATE 57014 joins the bounded transaction retry path. * fix: harden mobile push delivery and deployment recovery * feat: align mobile notification preferences with desktop delivery * fix: accept variable-length APNs device tokens * fix: deduplicate native APNs and background socket notifications |
||
  weekbinandNeil
|
e48d83a5e1 |
Fix MiniMax China usage routing and credential handling (#14929)
* feat(minimax): endpoint selector, API key auth, weekly usage window (#14264) The MiniMax (MiniMax) Coding Plan usage fetch was hardcoded to the overseas platform (platform.minimax.io) and a single 5h session window, so users on the CN endpoint (www.minimaxi.com) got nothing. Three changes: - Add `minimaxEndpoint` (`overseas`|`cn`) and `minimaxApiKeyConfigured` settings fields with sensible defaults that preserve current behavior. The CN endpoint also accepts an API key (safeStorage-encrypted via a new `minimax-api-key-store.ts` + IPC pair) for users without a browser session cookie. Status-bar visibility now OR's both credential flags. - Cookie-jar origin now tracks the active endpoint. Previously cookies were stored under the overseas origin and silently dropped when the user picked CN — fixed by threading `endpointMode` through the request context, the manual cookie header path, and the cookie-jar clear. - Parse the weekly window in addition to the 5h session and surface both as per-window chips (`5h [bar] 10% wk [bar] 20%`). The status bar's compact section prefers the session window; the popover keeps the existing `Session` / `Weekly` labels. The MiniMax fetcher is split into three files (data / parse / main) to stay under the 300-line cap. i18n is scoped to the Settings-page text (en + zh only); the 5H/7D duration shorthands stay English across locales by project convention. Tests: 9 new/updated files; cookies + API key exercised end-to-end via the rate-limit service with the upstream-refactored test files (`service-minimax-usage.test.ts`, `web-preload-api-settings.test.ts`, `web-preload-api-agent-providers.test.ts`, `service-test-harness.ts`, and the runtime-home / reset-credit fixtures). Refs #14264 * Keep merge formatting scoped to MiniMax * Keep MiniMax credential status in rate-limit test fixtures * Use the China console origin for MiniMax request referer --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
 Andrey
|
14e0d40e06 | fix: recognize the kimi-code process as the kimi agent (#18634) | ||
|
Neil
|
8d8b9dad78 |
fix: keep macOS shell ownership proof within recovery budget (#18932)
* fix: keep macOS shell ownership proof within recovery budget * fix: parse the shell-proof column set with its own anchored parser The narrower macOS capture (`pid ppid pgid tpgid stat command`) was fed to the shared lenient parser, whose optional tty/start pair has no `tty=` column left to absorb it. It then eats the head of any argv shaped `python 3 app.py` (parsing command as `app.py`, tty as `/usr/bin/python`), and turns a command-less row into a garbage pid/stat pair. Either can flip a shell ownership verdict, which is what gates dead-TUI recovery. Give the column set a named constant and a parser anchored to exactly those six columns, beside its `CHEAP_PS_ARGS` sibling. A capture that yields no rows now raises `empty_capture` rather than reading as a machine with no processes. Update the `confirmShellForegroundProcess` fixtures from the 4-column legacy shape to the 6 columns the darwin reader actually emits; that describe block already forces `platform=darwin`, so the stale fixtures were failing. |
||
|
Brennan BensonandMerge Sim
|
f7d5216016 |
Show provider activity in chat turn tails (#19055)
* feat(chat): show turn-scoped activity tail * fix(chat): keep turn activity broad * feat(chat): surface provider activity in turn tail * fix(chat): keep reasoning headline as activity and widen redaction A Codex reasoning summary streams as a bold headline followed by body text. Folding the whole summary into the tail leaked literal ** markers and body prose; only the first non-empty line is activity copy, and an unterminated bold header mid-stream is unwrapped too. Redaction used a hyphen for GitHub token prefixes (they use an underscore), and missed fine-grained GitHub tokens, AWS access key ids, JWTs, URL userinfo passwords, and bare token= values. * fix(chat): wait for a complete reasoning headline A bold headline still streaming has no closing marker yet; holding the previous activity copy until it lands avoids flashing a half word. * refactor(chat): drop bespoke secret redaction from activity copy Reference agent hosts render provider-derived status text unredacted; this table was the only one of its kind and its GitHub pattern matched no real token. Bounding and the reasoning-headline extraction stay. * Bound provider headline updates and clear activity on reconnect --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
c7bcfa750a |
fix: restore the full sidebar agent row for structured native chat (#19137)
* fix: restore the full sidebar agent row for structured native chat The host status feed projected only state, prompt, and agent type, so a structured Claude/Codex row fell back to the tab title and the agent-type label where a hook-reported row shows the running tool, the agent's last message, and the model. Project the tool line and the newest assistant prose from the journal, and take the model from the session record's acknowledged options. The tool scan stops at the live turn's lifecycle row and only runs while a turn is running, so an abandoned call from a crashed turn is never reported as live work. The assistant line is bounded to the shared preview cap rather than the hook field's 8 KB body: a streamed reply re-projects on every journal checkpoint, and the row renders one line of it. * fix: keep structured session status current --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
20eea184cc |
feat(native-chat): offer the link-action popover for chat links (#19130)
* feat(native-chat): offer the link-action popover for chat links A plain click on an http(s) link in a native chat transcript opened the system browser outright, ignoring the link-routing preference the same link honors in the terminal. Chat now shows the terminal's destination popover, with the modifier chords routing straight to a destination. The popover, its request type, the destination policy and the routed open move out of terminal-pane so both surfaces share one implementation; the catalog keys keep their original namespace because they carry shipped translations. Chat resolves its link owner from the session workspace (runtime, then SSH, unresolved stays unknown) so a remote transcript only offers Orca Browser when that host's managed browser route is eligible. The existing toggle now governs both surfaces, so it is retitled; with it off a chat link still opens on a plain click instead of going dead. * Fix native chat link popover lifecycle and keyboard anchoring * test(native-chat): use one store mock for link actions * fix: update reliability gate for shared link popover tests --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
d07c47593d |
feat(mobile): structured native Claude chat (#18741)
* feat(mobile): structured native Claude chat Mobile already spoke the structured agent-session protocol for Codex, and the host already had a Claude capability gate — mobile just never advertised it, so `projectAgentSessionTabsOut` stripped every Claude tab before it left the desktop. The structured lane in mobile/ turned out to be agent-agnostic already (shared reducer, message projection, option catalog, prompt tokens), so this opens the gate rather than building a second lane: - advertise `agent-session.structured.claude.v1` - resolve any structured provider in `resolveMobileNativeChat` via the shared `isAgentSessionHandleProvider`, instead of a `'codex'` literal - widen the `agent-session` route type off `'codex'` - route bare Claude launches through `agentSession.createSupport` like Codex, which still degrades to a terminal when the host refuses (remote, WSL, win32, managed-account mismatch, or structured chat switched off) Deduplicate the create envelope. Renderer and mobile each assembled the `agentSession.create` params by hand; the fingerprint has to be computed over the same fields the host recomputes, so both now build it in one shared `structuredAgentSessionCreateParams`. Mobile's Codex-only launcher becomes `createMobileStructuredAgentSession(client, worktreeId, agent)` and reuses the shared display-name map; two copies of a random-UUID fallback collapse into one. Answer grouped Claude questions. A Claude AskUserQuestion carrying more than one question — or one multi-select question — is emitted with the real content in `body.questions` and the flat `options` left EMPTY, so mobile rendered a card with nothing to tap and the turn stalled with no way out. Codex never emits this shape. The phone has room for one question at a time, so the group is answered as steps and submitted once, reusing the shared `encodeAgentSessionQuestionAnswers` / `isValidAgentSessionQuestionAnswers` rather than a second encoding. Prompt responses move into `useMobileStructuredPromptResponses` because grouped questions carry a multi-step draft the rest of the session does not touch, and the session hook was at the 300-line cap. Pin the mobile capability list against the host's parser bounds: it fails closed to NO capabilities when the array exceeds 64 entries, which would look exactly like an old client. Re-pin mobile-session-route-parity: the create-actions edit drops one runtime string literal and changes one nested function body. Ablated to confirm that file is the sole cause. * fix(mobile): derive the grouped-question draft instead of clearing it in an effect The React Doctor gate flagged the session-change reset as a state adjustment after a prop change, which renders the stale draft for a frame. Store the session the answers were collected in alongside them and check it on read, so a session switch drops the draft during render with no effect at all. * test(mobile): pin that grouped steps key apart when the questions read identically Claude can ask the same text twice in one group (once per file, say). The view keys the question card by its projected content, so identical wording must still key apart or step 1's checkboxes would be submitted as step 2's answer. * fix(mobile): harden grouped Claude question answers * fix(mobile): retry transient structured support probes * fix(mobile): preserve grouped prompt response compatibility * fix(mobile): preserve tokenless duplicate choice identity * fix(mobile): point the launch tests at the generalized create API The rebase onto #18697 brought its definitive-refusal tests in cleanly, but they call the pre-rename createMobileStructuredCodexSession, and mobile tsc excludes test files so nothing caught it. Retarget them and give the agent-copy test a code that is actually in the definitive allowlist - agent_session_refused now correctly stays unknown, so it never reached the failure copy it asserted. * test(mobile): re-pin route parity after the rebase onto main Main moved its own runtime-string pin to 547; this branch drops the 'codex' literal from the create-actions gate. Ablated against main's pins to confirm that file is the sole cause before re-deriving. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Jinwoo Hong
|
06a607a1d7 |
feat(orchestration): make multi-agent workflows durable (#16904)
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->
| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 225 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$21666 | $\color{#cf222e}{\Huge{\mathbf{−}}}$2820 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$18846 |
| Prod | 348 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$17107 | $\color{#cf222e}{\Huge{\mathbf{−}}}$4706 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$12401 |
<!-- /orca-pr-loc -->
## ELI5
Orca now treats orchestration like a durable control plane instead of inferring success from terminal keystrokes. Agents can tell whether a prompt was accepted or a turn started, replay an ambiguous request without sending twice, and recover coordinator mail after a crash. Completed workers can be inspected, released, or retained, and their panes no longer auto-resume as if the work were still running.
## What changed
- **Run receipts** from `run-create/use/current/show/list` are the row without routing plumbing (`home_database`, `coordinator_pane_key`) and without the duplicate `binding` object.
- **`terminal send` receipts are honest and idempotent.** `input_accepted` and `turn_started` are the only stages; `--wait-submit` observes without resending; `--retry-request <uuid>` replays the exact request against the same process incarnation. A transport timeout keeps the retry ID; only a different runtime answering strips it. Value-less or non-UUID `--retry-request` is rejected on the CLI and the SSH shim.
- **Mailbox delivery is committed before wakeup.** Pointer writes are staged in the DB before any PTY byte, replayed once after restart, and never emit a naked Enter. The watermark that parks concurrent deliveries is released with the DB reservation. Restart rescans pointer-pending and `dispatch:` mailboxes.
- **Lifecycle is a guarded transition graph** (`lifecycle-transition.ts`) with a table-driven test over every caller edge. Task reopen/overturn stays in the public contract. A PTY exit during `worker-stop` is the stop succeeding, not a failure.
- **Worker lifecycle CLI:** `worker-start` (`--spec` creates Task + attempt in one call), `worker-show`, `worker-read` (provider transcript first, bounded terminal fallback with a typed reason, local/WSL/SSH), `worker-stop`, `worker-abandon`, `worker-release`, `worker-retain`, `worker-list` (rowid-fenced pagination, fleet liveness, `attention`, literal `nextAction`).
- **Release is an explicit ownership table** (`decideWorkerTerminalRelease`): only an `owned` resource can be settled, the archive is mandatory where reachable, and an owner whose process is proven exited can always get out of `retained` via `archive_status: unavailable`. User-taken-over, external, and transferred panes stay retained.
- **Settled-worker resume fence** (folds in #17651): a settled dispatch whose pane is still open is fenced at settlement, on stop/abandon/exit, and at startup; lifted on release, retain, takeover, and pane reuse.
- **Liveness is `live` / `unverifiable` / `exited` only**, from execution-host evidence. Fleet projection reads the evidence clock, not the relay delivery clock. A host-certified exit outranks the worker's settled state. `unverifiable` never authorizes stop, abandon, retry, or release, in code or in the guide.
- **Federation:** structured reads negotiate by `method_not_found` so every shipped host keeps transcript-first output; exited remote workers are closed before being reported closed; epoch fencing holds across peer restart, downgrade, and pairing rotation; no per-second forced capability probe.
- **Schema v35:** repairs databases stamped v34 by the pre-fix branch (mailbox_handle default, index predicates), drops the write-only `lifecycle_transition_receipts` ledger and five never-read v31 identity columns.
- **Schema v36:** `dispatch:<id>` mailboxes get a real consumer generation on `dispatch_contexts` and `remote_dispatch_attachments`, bumped and fenced in the same transaction on every re-attach (manual inject, worker-start, federated attach). A stale worker whose Dispatch moved to another process now gets `consumer_fenced` instead of silently acking the new worker's Delivery. Run mailboxes already worked this way.
- **Schema v37:** `dispatch_contexts` records its creator (`creator_handle`, `creator_pane_key`), so a coordinator's context-only self-dispatch is bookkeeping rather than a nesting parent; before this, one self-dispatch made every later `worker-start` from that coordinator fail the depth cap. Pre-v37 rows keep counting (fails closed).
- **Dispatch-mailbox ownership is checked, not inferred.** A `check` from a process whose pane no longer holds the Dispatch, or whose last Attempt was abandoned/failed and moved to another terminal, gets `consumer_fenced` instead of an empty inbox that reads as "no mail yet". `--peek`/`--all` stay readable. A paneless caller still gets `stable_pane_required` with the rebind recovery.
- **Liveness certification is stricter:** a `process_exited` stage whose termination reason is `unknown` (a stop that was issued but never observed) projects `unverifiable`, not `exited`. Federated `worker-show` carries the execution host's verdict and host kind instead of a local guess. A live, ready worker with nothing pending has `nextAction: none` rather than pointing at the `worker-show` that produced it.
- **Wire:** `workerShow` keeps `dispatch.task_id` next to `taskId` for shipped CLIs. `ask --json` uses the standard `{ok, result}` envelope like every sibling verb.
- **Migration start-version detection** treats the two v32 recovery columns as versioned. Before this, every shipped database stamped below 32 resolved to the v6 floor and replayed the whole chain (the v23 backfill synthesized 68 phantom retained workers on a real v30 profile). Verified on a copy of a real 62 MB v30 profile: starts at 30, no row delta, integrity ok, 11 ms.
- **Skill guide** rewritten as a ≤200-line kernel plus seven references, to the outcome-first standard (Result / Done / Safe failure first, conditions not case lists, one done bar, references loaded at the point of use). The canonical loop uses `worker-start --spec`, names `worker-list` for completion accounting, documents `--retry-request` / `request-show` / `--wait-submit`, and requires positive evidence before any stall action. The other seven guides get the same treatment in #18724, split out so this PR stays orchestration-only.
- **`rpc/methods/orchestration-*`** (126 flat files) regrouped into `orchestration/{worker,federation,messaging,runs,gates}/`.
## Why
User reports showed the same boundary failures: false `agent_prompt_stalled` causing duplicate sends (#15180), coordinators unable to trust screen scrapes, cold-parked terminals receiving a pointer without the submit, settled workers accumulating as live tabs and auto-resuming after restart, and no way to tell a stalled worker from a working one.
## Linked issues
Fixes #15180. Fixes #17935 (orchestration skill description is 866 characters; a guard now caps every bundled skill at 1,024). Supersedes #17651 (fence folded in). Advances #16660, #16522, #14907, #13047.
## Review record
This PR was reviewed adversarially after revival: eight independent lenses (lifecycle, mailbox, send, worker, federation, transcript, complexity, live ergonomics), each required to prove findings with a failing test. That produced 16 proven blockers, all fixed with red-then-green regression tests, followed by two re-review rounds and a third fix wave that caught 3 regressions introduced by the fixes and 7 fixes that missed their target; all closed. A final pass (five lenses incl. a live built-runtime smoke, then a re-review of the fix wave) found and fixed seven more, chiefly the stale-worker mailbox steal, the self-dispatch depth wedge, and the unproven-exit certification. Three independent Codex (gpt-6-astra) passes followed: the first found nothing new, the second found and fixed 3 defects (task-status reachability, WSL-local host classification, peer-capability epoch), the third found and fixed 6 (production PTY controller never installed settled writes, ambiguous in-flight pointer failures allowed duplicate replay, SSH/relay deadlines cut off a valid `--wait-submit`, stop-vs-exit race during inspection, and two release-recovery paths for vanished or exited terminals). The full record (findings, proof tests, triage, declines with reasons) is archived outside the repo.
**Rework after the live smoke.** A first live cross-host run on the shipped adhoc build (this Mac, a paired Windows host on the same build, a paired Mac on 1.4.195, and an SSH host) found a P1: a running local worker read `unverifiable`/`missing_status` because the fleet snapshot rows lacked the terminal handle the matcher keyed on. A 59-row failure table over every bug fixed during review showed the same two classes recurring: a fact dropped in transit through optional fields, and two authorities for one fact. Two blind designs (Opus, Codex) converged on the same mechanisms, and the scoped tranches landed here with red-then-green seam tests from the real producer to the real consumer, faults injected only at the transport or hook-ingest boundary:
- **Settlement (data-loss class):** one three-valued `WriteSettlement` (`accepted | refused{reason} | unverifiable{reason, bytesHandedToTransport}`) from the SSH multiplexer through daemon client, providers, controller, to pointer staging. No boolean, no rejection-as-third-state. The two silent degrades that fabricated a handoff are deleted; a provider that cannot settle refuses before any effect. Pointer text and Enter share the contract; a partial flush is `unverifiable`, never `refused`.
- **Evidence identity (false-liveness class):** fleet agent-status evidence is a tagged union (`binding: worker | pane | unresolved{reason}`, `clock: observed | delivery`) minted once at ingest, so a hook row captured on one process incarnation can never bind to a later dispatch on the same pane. The matcher's `!worker.paneKey ||` defaults are gone. One host-scope parser replaces two.
- **Small pre-merge items:** `capability_unsupported` from an old peer is no longer relabelled `host_unavailable`; a producer census test asserts every agent-status consumer path projects a pane-only hook row as `live`.
Two ergonomics defects the second live run surfaced on a real database are fixed here too: a pre-v3 dispatch already marked `completed` projected as `outcome_unknown` / `requiresAction: true` forever (three copies of the outcome ladder disagreed on legacy rows; now one resolver, legacy `completed` reads `succeeded` with nothing to act on, legacy `failed` stays actionable on the failure), and an unscoped `worker-list` enumerated the entire database (now defaults to the Run bound to the calling terminal, `--run` overrides, and the receipt's additive `scope` field says which).
A third live round on the shipped adhoc build of `b082443e1f` (same four hosts) plus an unscripted run in the user's own prompt style (a plain Claude Code shell, `/orchestration`, three workers, zero errors, bound-Run default confirmed) found two more branch defects, fixed with red-then-green tests: a worker freshly started on a paired server projected `unverifiable`/`host_indeterminate` with `requiresAction` for ~3 minutes, including after its own `worker_done`, because the host's federation observation returned `missing_liveness_verdict` for any PTY the liveness register had not yet swept (the host now reads a connected pane it owns locally as `live`; disconnected or SSH-scoped panes stay `unverifiable`); and six pre-v3 completed rows still carried an `input` category because settling through the task-status path or `failDispatch` never closed the Dispatch's pending question threads (both paths close them now, and schema v38 closes threads already pending on settled rows). The guide's `worker-start` examples now show `--model sonnet`, since an omitted model inherits the launcher's default.
A Codex adversarial pass on the tranche diff found one real design hole (identity minted at read time instead of ingest, now closed) and two daemon settlement paths that threw instead of settling (fixed). Two `@ts-nocheck` runtime mixins on these paths were extracted into checked modules; the repo-wide `@ts-nocheck` count is unchanged at 171.
Deletions during review: ~1,900 lines (write-only ledger, unread columns, dead v1 archive path, test harnesses shipped in prod, duplicated liveness and state-machine copies, self-capability checks that were compile-time true).
## Testing
- `pnpm typecheck:tsc:node|cli|web` clean
- `pnpm run check:code-quality:changed` 0 findings; `check:react-doctor:changed` 0
- `pnpm verify:bundled-skill-guides`, `verify:skill-bundle-manifest`
- full `pnpm test` on the integrated head: 72,332 pass / 292 skipped; the only failures were three non-PR files (two zsh live-shell suites hit a node-pty spawn-helper ENOENT while a concurrent native rebuild ran, 44/44 in isolation; `release-checkout.unit.test.ts` is a known 30 s load timeout that passes in isolation on `origin/main` too).
- CI on
|
||
|
Brennan BensonandMerge Sim
|
39cbc68f16 |
fix(native-chat): honor structured routing with saved options (#19040)
* fix(native-chat): keep saved options on structured route
* fix(native-chat): seed structured session options
* fix(native-chat): preserve create wire compatibility
* fix(native-chat): keep create replayable across an option change
Seeding host-resolved options into the attach fingerprint put a mutable
value into the durable operation identity. A create whose outcome was
unknown, retried under the same operation id after the user reselected a
model, re-resolved different options and hashed to a different
fingerprint — so the ledger refused it as a conflict instead of
replaying. That refusal is not definitive, so no legacy fallback fires
and the launch has no recovery.
Options are the session's initial state, not its identity, and the
reservation still carries them to the record. Excluding them also makes
the digest byte-identical to the pre-change one in every case, not just
when no options resolve.
* refactor(native-chat): name the structured launch option seed
The create-intent resolver narrowed saved options to model/effort with an
inline key literal, inside a file carrying @ts-nocheck — so neither the
key list nor the string narrowing had a typechecked or testable home, and
the repo already expresses this concept as a named shared shape.
Move it to resolveStructuredLaunchSeedOptions beside the persisted
settings it reads, where it is typechecked and unit-tested, and document
why the seed is exactly model and effort: they are the only ids the
picker persists that both providers also accept as strings.
No behavior change. Adds coverage for a non-string persisted effort,
which settings.json can hold and the durable record must not carry.
* test(native-chat): name the structured routing pin for what it asserts
The case drives a saved Codex model and effort through the launch path,
but the structured create intent it asserts on carries no options, so it
pins the route and not the preservation its name claimed. Preservation is
pinned host-side, where the seeding actually happens.
* test(native-chat): pin the empty seed the record cannot carry
valuesByModel is merged over the resolved model, so a stored `model` key
can blank it — the seed then empties out and must resolve to undefined.
Nothing covered that branch, so returning the empty map unguarded stayed
green.
It matters because emitting `{ model: '' }` fails the record's
bounded-string guard, and agent_session_options_invalid is not a wire
refusal code: classifyStoreFailure rethrows it, the client reads the raw
error as an unknown outcome, and the launch strands with no fallback.
Corrects
|
||
 
|
6494f2a4f0 |
fix(native-chat): resume a structured chat from Agent Session History (#18933)
* fix(native-chat): resume a structured chat from Agent Session History Clicking Resume on a chat-UI row could only reveal an already-open tab. If the chat had been closed, or this process had never published it, the click re-read an inventory that did not contain it and toasted "Retry in a moment" — advice that could never come true, because nothing republishes an unpublished tab. The legacy `claude --resume` fallback is deliberately refused for structured-owned rows, so the row had no way back at all. `close` already keeps the record and the journal on disk so a session can be attached again, and the hold path already resurrects one in full. What was missing was the tab: `restoreReadableSessions` is latched to run once, at startup, so nothing could ask for a single session later. Adds `agentSession.reveal`. The host looks up its own record, restores the session readable, and republishes the tab through the same call `agentSession.create` uses. Deliberately narrow: - It takes no hold. A provider child exists because a surface asked, and the chat pane asks when it binds. - A journal it cannot read is not a refusal. A chat whose journal predates the SQLite store restores to nothing here, but attach still recovers it, so the tab is published and the pane's hold finishes the job. - Workspace and provider come from the record, never the client, so a session id alone cannot aim the publication at another workspace. Claude and Codex both, by construction: eligibility is `adapterSupportsRecord`, which the router answers from the record's own provider. Gated on a new advertised capability rather than probing for method_not_found, matching agent-session.structured.hold.v1 — absence is visible during negotiation instead of by calling. * fix(native-chat): negotiate reveal against the host that owns the workspace The capability gate read the LOCAL runtime's advertised capabilities while the call went to the host that owns the workspace, which for a paired workspace is a different build. On desktop the renderer and its local host are always the same build, so the gate passed unconditionally and proved nothing about the host being called: an older paired host still received the unknown method and its method_not_found was reported to the user as 'this chat is no longer on this host'. The cache it read also starts empty and resets to empty when status.get fails, so 'not fetched yet' and 'unsupported' were the same value. Gate on the environment that will answer, the way agentSession.close already does, and skip the round trip entirely for a local host. Reveal now reports four outcomes instead of a boolean, so a host that is merely too old is not reported as a chat that is gone, and a host we could not reach keeps the retryable message. Also syncs the localization catalog: the 'gone' key shipped without an en.json entry, which reddens static analysis and verify while typecheck stays green. * fix(native-chat): tell a refused reveal apart from a missing chat The host raises two refusals here and they mean opposite things to a user: it holds no such record, or it holds one no adapter of its own can open. The client collapsed both into 'this chat is no longer on this host', which is a eulogy for a chat still sitting on disk. Read the refusal code, and fold the host-side case in with the too-old host under one honest message, since the remedy for both is the same. Adds the coverage the readiness pass found missing: the host's reveal answer itself (workspace and provider from the record, both refusals, an unreadable journal, a live session), and the activation branches for a host that cannot open the chat and for one that never answered. * fix(native-chat): read a host version block as the host's age, not a lost link The capability probe reaches assertRuntimeStatusCompatible, which throws a runtime_compat_block error. Treating that as unreachable told a user with an out-of-date host to retry, which is the one thing that cannot help. Branch on isRuntimeCompatBlockError the way remote-agent-session-launch already does for the same probe. Also adds the refusal-code case a previous commit claimed and did not deliver: nothing drove a structured_agent_session_unsupported reply through the reveal client, which is the branch that commit existed to add. Corrects a doc comment that reveal made wrong: attach is no longer the only call that builds the host. * fix(native-chat): let a dragged history row reach the same reveal as a click Dropping an Agent Session History row onto a pane activated the tab by id and, on a miss, raised the very toast this PR exists to remove — so the same row answered a click and a drop differently, and the drop kept the advice that can never come true. The structured branch never used the drop pane, so routing it through the shared activation loses nothing and gains the reveal. The helper only ever read one field, so its parameter narrows to that field and the drag payload satisfies it directly. A source ratchet holds both entry points to the reveal-capable path, since a mounted drag harness does not exist for this layer and what regresses is a call site, not a rendering. * fix(native-chat): stop an advisory refresh ending the click, and one click per row Manual QA found the reveal never ran: the inventory refresh that precedes it is an optimization, but its failure returned early with 'not available yet, retry in a moment' — reinstating the dead end this PR removes, one step earlier. A failed refresh now falls through to the reveal, which is the repair and does not need the refresh to have worked. The click can chain a refresh, a capability probe, a reveal and a second refresh, each with its own timeout, while nothing on the row says it is working. A per-session in-flight guard keeps an impatient second click from running the whole sequence again and landing its own toast. Also drops an unreachable owner scope: the snapshot apply discards any worktree whose execution host is not local before it reads one, so naming a remote scope there described a synchronisation that cannot happen. * fix(native-chat): bound the capability probe and stop naming the wrong machine The in-flight guard releases when the activation settles, so an await that never settles holds the row for the life of the process. The capability probe was the one call in the chain not raced against a deadline: on a cache hit it awaits a promise an earlier probe created, which may carry no deadline of its own. Race it like the two calls around it. A version block can name either side — evaluateRuntimeCompat reports client-too-old as well as host-too-old — so a message that blamed the host pointed half of those at the wrong machine. Name the remedy instead of the machine, which is true for every case that reaches it. * chore: remove a scratch repro file committed by mistake It was swept into the previous commit by a broad `git add` while a diagnostic ran in this worktree. It asserts the current renderer-sync defect as expected behaviour, so it would fail the moment that defect is fixed. * fix(native-chat): stop a reveal's own inventory refresh discarding its republished tab Manual QA: the host answered reveal with ok:true and republished the tab, and the chat still did not reopen — only a renderer reload brought it back. The renderer publishes under one epoch string for its whole lifetime, and a frame recorded under a different lineage retires that epoch permanently with nothing to un-retire it. The Resume click asks for an inventory first, and a worktree the host holds no entry for answers with the none/v0 sentinel; the structured path recorded it, retiring the renderer's own epoch, so the tab the reveal published a moment later was dropped. A reload minted a new epoch, which is why reloading appeared to fix it. A frame that carries no publication is not a later publication to fence against. Treat the sentinel and a removal frame as a cursor reset, the way the mainstream session-tabs path already clears its tracking — its comment names this exact hazard: recording that sentinel would retire the host epoch and reject the next live frame. Pre-existing, and it swallows an ordinary new-tab launch on an empty worktree too; the reveal is what turned a silent invisibility into a visible failure. * fix(native-chat): let a retraction prune its rows without retiring the epoch Correcting the previous commit. Skipping a retraction frame outright stopped it pruning the mirrored rows, so a worktree the host no longer publishes would have kept a chat on screen with nothing behind it. Apply the frame as before and clear its cursors instead of recording them, which is what the mainstream session-tabs path does. The unpublished sentinel keeps its cursor now too: it is skipped rather than cleared, so a stale frame arriving late is still fenced. Adds the case the earlier version would have broken. * fix(native-chat): keep the retraction's fences, and fence the reveal's refresh Correcting the retraction handling again. Clearing its cursors was more than the bug needed and cost a guard: the host mints a fresh epoch when it rebuilds a pruned entry, so a republication is never gated by the retained cursor, while dropping it left an inventory response issued before the close free to land afterwards and strand a chat row for a worktree the host no longer publishes. Skip only the recording. The mainstream path keeps its epoch history for the same reason, as a tombstone fence. The test that justified the stronger clearing asserted a host behaviour that does not exist — a rebuilt entry republishing under the renderer's epoch with a restarted counter. It now uses what publishStructuredAgentSessionTab actually mints for a pruned entry, and a new case covers the frame that would strand. Also fences the reveal's inventory refresh on the sync generation, which every other caller that applies an inventory already does: structured chat can be switched off mid-flight, and the answer would otherwise re-seed a row into a renderer that just discarded them. * fix(native-chat): drop the retraction's epoch history, keep its version cursor Third and final shape for this branch, and the only one of the three that holds. Keeping both maps re-poisons the epoch one cycle later: the consumer here is also the publisher, so the history's current is the renderer's own lifetime epoch, and recording the reveal's fresh epoch retires it. The next chat the renderer publishes is then dropped — this bug again, one close later. Deleting both loses the guard that stops a frame issued before the close landing after it and stranding a row nothing republishes. So: clear the history, keep the cursor. The mainstream path keeps its history as a tombstone because there the epochs belong to a remote publisher; that reasoning does not carry to a path that publishes under its own. Each of the three variants now fails a different test. * fix(native-chat): a retraction forgets what is current, not the tombstones The delete lost a fence the cursor cannot replace: the version cursor only compares within a lineage, so a delayed frame from an already-superseded epoch had nothing left to stop it putting a chat row back for a worktree the host no longer publishes. Keeping the record intact had the opposite fault — the renderer's own epoch is the history's current, so the next frame under any other epoch retired it. Clearing only current does neither: noteRetiredValue retires nothing when there is nothing current, and the tombstones stay. Each of the four shapes now fails a different test. * fix(native-chat): narrow the retraction frame through its own type Typecheck caught what the tests could not: `removed` is not on RuntimeMobileSessionTabsResult. The repo already names the shape — RuntimeMobileSessionTabsRemovedResult — so this reads it through a guard rather than the inline cast the mainstream path uses. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Merge Sim <sim@local> |
||
 NaoyaTatetsuandNeil
|
a567e33bf7 |
feat(github-projects): render Roadmap project views as a timeline (#17795)
* Add roadmap timeline view for GitHub Projects - Renders roadmap-layout project views as a scrollable timeline with date/iteration-based placement, zoom levels, and grouped lanes, instead of surfacing them as unsupported - Derives placement fields from view config or row-carried field values since GitHub's API never exposes a roadmap's date source directly - Falls back to the existing table list when no field can place items * Fix roadmap timeline edge cases: reject invalid calendar dates and refre - parseRoadmapDate previously let Date.UTC silently normalize overflowing dates (e.g. 2026-02-30 → Mar 2); now round-trips components to reject them - ProjectRoadmap's "today" marker was frozen at mount, so panes left open across midnight showed the wrong day; now re-derives and re-arms a timer * fix(github-projects): center roadmaps when dated rows arrive * fix(github-projects): keep pinned roadmap header opaque * fix: remove stale pnpm executable lockfile entries * fix(i18n): retain replaced project labels in runtime catalog --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
|
Neil
|
a63a4579cf |
Let worktree creation proceed during stale preparation reclamation (#18967)
* Stop obsolete worktree preparations when evicted or expired * Let worktree preparation proceed during stale reclamation * Verify creation during stalled stale worktree reclamation * Preserve preparation ownership until Git removal starts * test: keep artifact share fixtures unexpired across calendar dates (#18955) |
||
|
Neil
|
64a449df4e | perf(search): assemble fragmented subprocess lines incrementally (#18973) | ||
|
OrcaWinandOrca Worker
|
c252d855ac |
fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869)
* fix(windows): resolve npm/pnpm .cmd shims past cmd.exe
A `.cmd` target forces every spawn through `cmd.exe /c` with each argument
caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c`
line carrying caret-escaped natural language as obfuscation. `codex.cmd` is
named in the spawn cluster of the MDE incident this addresses.
npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body
is "find node, run this script". Read one, and the spawn can go straight to
`node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser
does not recognise exactly, or whose target cannot be confirmed on disk, keeps
the existing cmd.exe path.
Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the
quote state, so a multi-line agent prompt through a `.cmd` shim had to be
rejected. Resolved shims have no such limit.
* fix(windows): refuse drive-relative shim paths and run the win32 tests in CI
Two blocking findings from review.
A drive-relative path defeated the absolute-path guard:
`win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive
letter and lands on `D:\evil.js`, outside the shim directory. cmd would have
built `C:\shim\D:evil.js` and failed; we would have executed the wrong file.
Adding `:` to the unsafe-character set closes it, and the alternate-data-stream
spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on
this box still resolve, the same seven fall back.
Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in
the Windows package job's file list, so the whole filesystem/resolution half and
the real-spawn equivalence suite ran nowhere. Both are now in
`WINDOWS_PACKAGE_TESTS` and in the pr.yml step.
Also from review: clear `windowsVerbatimArguments` explicitly on the resolved
branch rather than inheriting it, since there is no caller-built command line
there; document the kill switch and the PTY/hook-wrapper scope limits in
docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*`
tampering in the platform-independent half of the tests.
* docs(windows): justify the shim-path colon guard from the filesystem rule
The guard was argued empirically ("none of the 91 shims on this box has one"),
which invites a future reader to relax it for a shim we have not seen. Windows
reserves `:` within a path segment, so a relative path cannot carry one at all:
the only spellings that can are drive-qualified, an alternate data stream, or a
`\?\` device path, and the last is already refused as absolute. That makes a
false refusal impossible rather than unobserved.
* refactor(child-process): move resolveSpawn into its own module
The merge with main pushed run-process.ts one line past the 300-line cap:
both sides grew it. The spawn-argv decision is already a pure, separately
tested unit, so it moves out rather than the cap moving up. run-process.ts
re-exports it, so no caller changes.
* perf(child-process): cache the shim interpreter lookup
The parse cache spared the shim read but not the PATH walk, so a second
resolution of the same .cmd did 0 reads and one statSync per PATH entry --
30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network
mount blocks the calling thread on every spawn.
Keyed by shim directory AND PATH, since the shim's own rule is
%~dp0\node.exe first then PATH, and a PATH edit between spawns must miss.
Corrects the stat comment, which accounted only for the shim itself.
* fix(child-process): revalidate a cached shim interpreter before using it
The node cache was held for process life and never rechecked, so a cached
node.exe that was later uninstalled -- or dropped from PATH by a version
manager -- was still handed to resolveSpawn, failing the spawn with ENOENT.
An uncached process in the same state returns null and falls back to
cmd.exe successfully, so the cache was strictly worse than no cache.
One statSync on a non-null hit, not one per PATH entry, so the walk this
cache exists to skip is still skipped. The stale-null direction stays
uncorrected on purpose: it only keeps the working cmd.exe fallback. Both
directions are now stated in the comment, along with the known miss for
callers that vary PATH per spawn.
* fix(child-process): honour PATHEXT when resolving the shim interpreter
The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to
cmd.exe. The scan actually skipped those entries and kept looking for a
node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to
B's node.exe while the shim runs A's node.com -- a different binary, chosen
silently, on the one axis this module must not get wrong.
The scan now follows cmd's rule: first PATH directory holding any PATHEXT
spelling wins, PATHEXT order decides within it, and only an .exe winner is
returned. Anything else gives up and keeps the cmd.exe path, which restores
the strict-subset-of-cmd property everywhere except the documented cwd case.
PATHEXT is read from the child's env and joined into the cache key, since it
now changes the answer. Costs one stat per PATHEXT entry per node-less
directory, paid once per process behind the cache.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>
|
||
|
OrcaWinandOrca Worker
|
0b6308f4d5 |
test(child-process): lower DIRECT_IMPORTER_PIN to the ground two PRs took (#19009)
#17861 and #17884 each migrated one file off node:child_process and each lowered the pin 158 -> 157 independently. Together they took two, so the true count is 156 and the two-sided assertion fails on main. Co-authored-by: Orca Worker <orca-worker@localhost> |
||
|
|
0cbb01ef4b |
fix(security): apply the Windows path-hardening ACL that never ran (#17884)
* fix(security): apply the Windows path-hardening ACL that never ran `buildWindowsRestrictAclArgs` invoked the hardening script as `powershell.exe -Command <script> <path> <sid> <isDir>`. `-Command` does not populate `$args`; it appends the trailing tokens to the command text. The script therefore read `$args[1]` as `$null`, threw `NullArrayIndex` at `$allowedSids[$sidText] = $true` under `$ErrorActionPreference = 'Stop'`, and exited 1. Both callers swallowed that: the async callback was empty and `applySecurePathRestriction` returned `true` regardless, while the sync `catch` returned `false` and nobody logged. Every Windows secure path has been left on its inherited ACL since the ACL was introduced (#5006), and nothing said so. Replace PowerShell with `icacls.exe`, which takes plain argv. That removes the quoting surface entirely rather than escaping it: interpolating a path into the command text would have turned a dead no-op into arbitrary PowerShell on a filesystem path, since `-Command` executes what it appends. It also drops the execution-policy dependency and the `powershell.exe` spawn an EDR flags, and runs ~25x faster than the PowerShell cold start. Hardening is now three passes: `/reset` to purge explicit ACEs that `/inheritance:r` leaves behind, `/inheritance:r` plus a `/grant:r` per allowed SID, then a read-back that checks the DACL is protected and grants only the intended rights. The predecessor's verification block was equally dead, and an apply that is never read back is only half a control. Failures stay non-fatal — non-NTFS volumes, network paths and restricted tokens fail legitimately and must not break startup — but they are no longer invisible: every failure is logged, and a failed async apply now evicts its cache entry so the next call retries instead of trusting a success that never happened. Routing through `runProcess`/`runProcessSync` also retires this file's `node:child_process` allowlist entry. * fix(security): verify the hardened ACL by identity, not by shape Review found the bug class this PR fixes surviving inside the fix. The verify pass checked rule count, absence of the inherited marker, and exact rights — never *who* the rules named. Granting Everyone full control satisfies all three, so hardening reported success on a DACL that handed the credential to every local account, and most of the real-filesystem tests still passed. Verification now reads the descriptor back with `icacls /save`, which emits SDDL with raw SIDs, and compares the principal set exactly. That is also locale-independent by construction: the previous parse read localized account names out of icacls' OEM-codepage stdout, where a non-ASCII path survived by accident rather than by the documented mechanism. SDDL parsing moves to `windows-security-descriptor.ts`. Two further self-inflicted problems, both measured: The post-rename re-harden led with `/reset`, which re-widened a DACL that was already correct — the staged file's protected DACL survives the rename, so the pass had nothing to do but open a window. Polling an external process during a write into a relocated root caught it: the e2ee keypair dropped to `BUILTIN\Users:(RX)` plus `Authenticated Users:(M)` — read *and* write — before tightening again. Hardening now verifies first and returns early when the DACL already reads back correct, which closes the window and cuts the steady state from three spawns to one. Re-measured: 158 samples, one DACL state, zero broad. Evicting the cache on every failed async apply reintroduced #4901. The env store re-hardens on the read path at ~2/s, so on a host where hardening cannot work (FAT32, network path, restricted token) that was two icacls spawns and two warnings a second, forever. Async retries now take a retry floor and a hard per-path attempt cap. The write path keeps retrying unthrottled — it is user-driven, and a failed credential ACL must still be retried on the next write. Also: failures route through a reporter hook that the main process points at the diagnostic tracer, because `console.warn` reaches nothing in a packaged GUI-subsystem build; `writeSecureFile` returns whether hardening took, and the async branch reports `pending` rather than claiming `applied`; a transient `whoami` failure no longer disables hardening for the process lifetime, and the SID is shape-validated; the `/c` guard now covers the synchronous runner too. * fix(security): re-probe hardening instead of latching a transient failure The per-process attempt cap added for the read-path storm was a permanent latch: one AV scan, momentary lock or %TEMP% blip and every later credential write in that session went unhardened, silently, on a host where hardening would now succeed. Same defect class as #17858's computer-use host, and worse here because what stops happening is security hardening on credential files and nothing said so. The retry budget now bounds the *rate*, not the lifetime: at most three attempts per path per minute, re-probing in every later window, forever. The transition is announced in both directions — `throttled` once per window on entry, `recovered` when a rate-limited path hardens again — so a host stuck in the degraded state is diagnosable rather than merely quiet. The reporter type covers both, and the main process ends the `recovered` span successfully rather than failing it. Extracted to secure-path-hardening-retry-budget.ts, which keeps secure-file.ts under its line cap without a max-lines disable. Also confirms the second flagged risk rather than assuming it: a real unwritable %TEMP% is now covered by a test proving verification fails closed, reports at the `verify` stage, and still leaves the ACL applied — so that path loses proof, not protection, and with the lifetime cap gone it can no longer combine into a permanent-off state. * fix(security): verify a directory's whole inheritance flag set The flag check tested only that `OI` was present — never that `CI` was, nor that nothing else was. That was harmless while `/reset` + `/grant` ran on every pass and repaired whatever was there. The verify-first short-circuit made it load-bearing: what verification accepts is now left alone, so a latent under-check went live because a different fix started depending on it. Two directory DACLs passed while being wrong — both protected, three non-inherited full-control rules, correct SIDs, differing from correct only in their flags: (OI)(F) - no CI, so subdirectories are left unprotected (OI)(CI)(IO) - inherit-only, so the directory object itself grants nobody anything; the next writeFileSync into it fails with EPERM, on a directory just cached as hardened Verification now compares the whole flag set, which also rejects IO and NP, and names the offending flags in the failure. Both shapes are planted in real-filesystem regression tests, including an assertion that a write into the repaired directory succeeds and its child inherits. Confirmed both tests fail against the old check and pass against this one. * fix(security): back the hardening retry off exponentially The fixed one-minute window bounded the retry rate but left a standing floor of three attempts per path per minute on a host where hardening can never succeed — FAT32/exFAT, a network path, a redirected profile. That budget is per path and there are several secure files, so the floor multiplied into tens of thousands of icacls spawns a day for work guaranteed to fail. The delay now doubles after each consecutive failure, from a one-minute floor to a thirty-minute ceiling, and the attempt cap is gone entirely: once the backoff elapses the path is re-probed however long it has been failing. A permanently incapable host settles at ~2 attempts/hour. Slowing the backstop costs almost nothing, because it is not the recovery mechanism: the synchronous write path is deliberately unthrottled, so a host that recovers hardens on its very next credential write regardless of what the read-path budget says. The `throttled`/`recovered` reports are unchanged and matter more here, since the quiet periods between probes are now much longer. The curve is pinned in a new unit test against the exported delay function rather than a copy of its constants, covering the doubling, the ceiling holding at 5000 consecutive failures, a 30-day failing path still re-probing, one announcement per degraded episode, and per-path isolation. The integration tests keep only what they uniquely prove: that the read path is wired to the budget, and that a day of failures still re-probes. Confirmed four of these fail against a reinstated lifetime cap. * ci(windows): run the real-icacls DACL suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * fix(security): describe the cache's real cost, which is icacls now Both cache comments still justified themselves with PowerShell -- "~1-1.5s" and "a PowerShell spawn every read" -- in the same file whose PR removed PowerShell from this path. The caches are still right, but for different numbers, and the old ones are the kind an engineer would reasonably delete a cache over. The real shape: hardening verifies first and returns early, so an already-correct DACL costs one synchronous icacls spawn and a rewrite costs four (verify, reset, grant, verify). Still worth caching on the read path, which polls at ~2/s. * test(security): make the DACL suite safe to schedule Registering this spec in the Windows lane put it under two rules it had never been measured against. Teardown now goes through `removeTreeSync`, which the lane's boundary test requires, and repairs the DACLs the suite plants on purpose first: those retries only cover transient locks, so a regressed `(OI)(CI)(IO)` repair leaves the root un-removable and `afterAll` throws EPERM. And the no-permission case decides by elevation before it writes anything. `windows-2022` runs elevated, where hardening succeeds: the old branch asserted nothing about denial and instead replaced the `hosts` DACL, then `icacls /reset` -- which is not a restore, it drops the explicit `SYSTEM:(F)` that file ships with. Ephemeral in CI; permanent for a developer running the lane from an elevated shell. Now it asserts or it skips. The probe reads the token integrity SID rather than `icacls /save`, which succeeds unelevated (`BUILTIN\Users:(RX)` carries READ_CONTROL) and would have skipped the case on every machine. * fix(security): measure the hardening latches on a clock that cannot go backwards `mayAttemptHardening` compared wall-clock times, so any backwards step -- an NTP correction, a VM snapshot restore, a user changing the clock -- made the elapsed time negative and held every failing path below its delay until the clock caught up. Measured at the 30-minute ceiling with the clock stepped back a year, the path was refused at +0d, +1d, +30d, +180d and +364d, and re-probed only at +366d. That is the permanent latch the exponential backoff was added to remove, and it contradicts the module's own "bounds the rate without ever bounding the lifetime". The SID lookup's own one-minute window had the identical shape and is worse: a failed lookup makes `planFor` return null, which disables the synchronous *write* path too, so the write-path exemption that recovers the read-path budget cannot recover it. Both now measure elapsed monotonic time, following the repo's existing `monotonicNowMs` spelling. Two things the write path was not doing, both found in the same pass: - A successful synchronous apply now records the outcome. It is exempt from the budget, but it was also invisible to it, so a host that had demonstrably recovered kept the read path backing off for up to 30 minutes and no `recovered` transition ever came from that lane. Only success is recorded; recording failure would put the exempt lane back under the budget. - `writeSecureFile`'s JSDoc now says its boolean covers the file only. The directory harden is fire-and-forget and answers `pending` on Windows regardless, so a `true` says nothing about the directory's ACL. * fix(security): stop the hardening test doubles from faking a no-op Three CI failures on this branch, one failure shape: hardening silently does nothing and the check that should have caught it agrees. The auth critical-path test hand-rolled a `node:child_process` factory with `execFileSync`/`execFile`. The rewritten ACL path goes through `runProcessSync`, i.e. `spawnSync`, which the factory never returned — so every spawn threw into the SID lookup's bare catch, `planFor` returned null, and hardening no-opped. It mocks `child-process/run-process` now, the boundary production code actually calls and the one sibling ACL tests already mock: an export missing there fails loudly by name instead of returning undefined. Its fake icacls writes a real UTF-16LE SDDL file, so the pinned spawn count per write is a property of the ACL path rather than of the double. The test forces `platform='win32'`, so this failed on every platform, Linux CI included. `windowsSystem32Binary` is a production bug, not a test bug: it builds a Windows path with the host `join`, which off-platform yields the mixed `C:\Windows/System32/whoami.exe`. On Windows the two joins agree, which is why it survived; on Linux the SID lookup's whoami match missed and 27 of secure-file's 32 tests exercised a lane that never ran. These are always Windows paths, so `path.win32.join` is what it should have been. The import-boundary pin still read 160 after this branch migrated secure-path-windows-acl.ts off `node:child_process`; the ratchet correctly refuses a pin left above reality. * fix(security): resolve the machine-relative SDDL alias, and stop a denied read destroying the file Path hardening verified the DACL it wrote by comparing the SIDs `icacls /save` reports. SDDL substitutes two-letter aliases for well-known SIDs, and the resolution table could only hold constants -- but `LA` and `LG` name an account by RID inside the *machine's own* SID, so on a box whose user is the built-in Administrator (a CI runner, an Administrator-only install) the current user read back as `LA`, matched nothing, and hardening reported failure for every path. Resolve those two against the machine authority derived from the user SID; without one they stay unresolved and the comparison still fails closed. Three secret stores treated any read failure as "malformed -- regenerate" and overwrote. A hardened file granting a SID this process does not hold reads as EPERM while its directory stays writable, so the overwrite succeeds: renaming over an unreadable file needs FILE_DELETE_CHILD on the parent, not DELETE on the file. That destroyed the E2EE secret key, every paired device's bearer token, and the plugin vault. Distinguish EPERM/EACCES from a parse failure and refuse. Also close the async lane's unhandled rejection: `void p.then(onSettled)` turned a throw from `onSettled` into a dead main process, and the retry budget it calls threw whenever nothing had configured it -- a contract held only by import order. The budget now defaults its own bounds. * test(windows): say which ACEs icacls listed when a planted DACL fails `toHaveLength` reports only a count and vitest elides the array, so three preconditions failing on the CI runner said "expected 3, got 6" and nothing about what the sixth entry was. Name the entries in the failure. * fix(security): stop three more stores overwriting what they were denied Same swallow-default-overwrite shape as the readers already fixed, found by sweeping every store that reads under a hardened root. - plugin-storage-store.ts returned `{}` on any read failure and set()/delete() wrote it back, losing the plugin KV store. It is the secrets store's shape line for line, so the two now behave identically. - relay-revoke-outbox.ts returned [] and save() wrote it, dropping revocations that never reached the relay -- a revoked device stays live. - profile-cloud-session-store.ts mapped an EPERM read onto `decrypt-failed`, which fails the `status === 'found'` guard in clearCloudSessionIfUnchanged and falls through to an rmSync of the account session. A denied read now reports `unreadable`, which licenses nothing; the refresh path bails on it and the auth status surfaces it rather than reporting a bare reconnect. All reuse isPermissionDeniedError. The predicate stays an EPERM/EACCES allow list rather than "ENOENT defaults, everything else throws": these stores are meant to self-heal a truncated or malformed file, and inverting it would turn a corrupt keypair into an app that cannot start. The distinction that matters is "could not read it" versus "read it and it was garbage". * test(windows): plant fixture DACLs that cannot inherit what they did not plant %TEMP% grants [SYSTEM, Administrators, <user>] (OI)(CI)(F) by default, and those propagate into every fixture. Three preconditions read back 4 and 6 ACEs where 3 were planted, and the extras looked like Orca's own hardening because the shape is identical -- on a runner whose user is the built-in Administrator, the inherited trio IS the trio production grants. Combining /inheritance:r with /grant:r leaves the argument order to icacls, and that combined form drops the inherited ACEs on Windows 11 but keeps them as explicit ones on the Windows Server runner. Removing inheritance in its own invocation makes the grant the whole DACL on either host, and the fixture root is de-inherited once up front so nothing propagates in. Rooting the fixtures outside %TEMP% would not have fixed this: any directory inherits from wherever it lives. The fix is to stop inheriting, not to move. No assertion is relaxed -- the counts stay exact. * test(windows): pick a foreign SID that stays foreign on an elevated runner `S-1-5-32-544` is only foreign to a token that is not an administrator. The CI runner is elevated AND logged in as the built-in Administrator, so granting Administrators granted the reader full control: the file stayed readable, and all six preservation assertions went vacuous rather than proving anything. BUILTIN\Guests is resolvable everywhere and no interactive token is a member, so the read is denied on an unelevated developer box and on the runner alike. An unresolvable SID would have been the stronger choice but icacls rejects one with ERROR_NONE_MAPPED (1332). The premise guard is what caught this -- it asserted the file was actually unreadable instead of trusting the grant, and named elevation as the suspect. * fix(security): refuse on any read that never reached the contents, not just a denied one isPermissionDeniedError becomes isUnreadableError, because "permission denied" was never the concept -- "could not read it", as opposed to "read it and it was garbage", is. EBUSY, EMFILE, ENFILE and EIO say exactly as little about a file's contents as EACCES does, and they fell into the branch that regenerates and overwrites. On Windows EBUSY is the likelier of the two: antivirus holding a credential open at the moment of a startup read produces it, which makes it a commoner path to the same permanent loss than the ACL case that motivated the original fix. Still an allow list, deliberately: ENOENT keeps licensing a create, and a parse failure keeps self-healing. The stores are built to recover from a truncated write, and turning that into a refusal would trade a recoverable state for an unrecoverable one on the startup path. Also fixes the regression suite's own premise on an elevated runner: makeUnreadable combined /inheritance:r with /grant:r, and that form keeps %TEMP%'s inherited [SYSTEM, Administrators, user] as explicit ACEs on Windows Server -- so the file stayed readable and all six assertions were vacuous. Same split-the-invocation fix as the ACL suite's planter. * test(windows): skip the preservation suite where a read cannot be denied An elevated token logged in as the built-in Administrator reads straight through a DACL that grants it nothing -- confirmed on the CI runner against both BUILTIN\Administrators and BUILTIN\Guests, and with the grant split into its own icacls invocation so the DACL really was the planted one. On such a host the premise these tests rest on does not hold, and every assertion would pass while proving nothing. So probe once at module scope and skip rather than assert vacuously -- the same trade the ACL suite already makes for its unelevated-only case. The gate stays in the compound `<win32 check> && <flag>` form the win32 lane ratchet detects, so the file stays registered in both lane lists. Coverage is not lost where it counts: isUnreadableError has unit tests that run on every platform and every host, and the stores' refusal is exercised in full on any machine where a denial is reproducible -- which is every developer box. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
|
OrcaWinandOrca Worker
|
975bbdedcc |
fix(windows): scan ports natively instead of encoded PowerShell (#17861)
* fix(windows): scan ports natively instead of encoded PowerShell Microsoft Defender for Endpoint scored the relay's Windows port scan as suspicious PowerShell plus network discovery (T1049). The command line was `-ExecutionPolicy Bypass -EncodedCommand <base64>` around a Get-NetTCPConnection/Get-Process join -- base64 next to a policy override is the highest-weighted token pair on a PowerShell command line, and netstat only ever ran as its fallback. Invert the chain. `netstat.exe -ano` is now the primary reader and the owning process name comes from the shared native process table, which exists to keep PID lookups off PowerShell. The payload survives only as a last resort, and without the override: execution policy gates script files, never `-Command`, so nothing needed it (verified: `-ExecutionPolicy Restricted -Command` runs). Drop `-p tcp` while inverting: on Windows that protocol name means IPv4 only, so as a primary reader it would have hidden every `[::]` listener the payload used to report. Names arrive as `sshd.exe` from the table and are published as `sshd`, keeping the sshd filter and old clients' rendering intact. Routes both spawns through runProcess, removing the file from the child_process and windowsHide ratchets. * fix(windows): read netstat state by shape and refuse a truncated table Review of the port-scan inversion found two ways the new primary path could be silently wrong, both of which would have kept the flagged PowerShell payload running on exactly the hosts this change targets. `LISTENING` is not in netstat.exe. It lives in System32\<locale>\netstat.exe.mui and MUI selection follows the UI language, so the pinned-locale env in relay-command-env.ts cannot reach it -- a German host prints `ABHOEREN` and the word test parsed zero rows. The zero-listeners guard then read that as a blocked reader and ran `Get-NetTCPConnection` every 12-30s forever, or returned nothing at all where PowerShell is also restricted. Keep the word as the fast path and, when it finds nothing over output that did contain TCP rows, re-read by shape: only a listening socket has no peer. Measured on this host across all four states present (LISTENING 47, ESTABLISHED 49, CLOSE_WAIT 29, TIME_WAIT 213): zero non-listening rows with a zero peer, zero listening rows without one, and the same 47 rows parse after substituting the German state words. Shape stays the fallback because `BOUND` also prints a zero peer. Truncation was invisible: createOutputSink discards overflow, ProcessResult carries no flag, so a capped read still exits 0 and its head still parses. netstat orders IPv4 TCP, then IPv6 TCP, then UDP, so a host with tens of thousands of TIME_WAIT rows would have lost every `[::]` listener -- the exact loss dropping `-p tcp` exists to prevent, and one the zero-listeners guard cannot see. Refuse the read instead. A `truncated` flag on the shared sink would be cleaner and is left as a follow-up rather than widened into this PR. Also: decline to wait on the shared process table once the request is aborted (it takes no signal and must not be cancelled for other callers); note the name lookup as best-effort, since a TTL-cached snapshot can hand a recycled PID its previous owner name; log once on either fall-through, because both are permanent and invisible when wrong; and drop a stderr assertion that any PowerShell autoload banner would redden. Correcting the cost claim in the previous commit: the aggregate win holds with the native addon (netstat 21ms vs the retired payload 860ms at 532 processes), not without it. The addon is optional, the snapshot TTL is 500ms and the scan cadence is 12-30s, so a relay with no active agent pane never warms its own cache and pays ~1.4s cold on the CIM path -- slower than what it replaced. * fix(windows): log the port-scan fall-through on the relay diagnostic stream Checked where this code actually runs before trusting the log. `console.warn` did reach a file, but relayLogLine is the right call and the reasoning is worth recording. `scanWindowsListeningPorts` runs only in the detached relay daemon: relay.ts returns early for --connect and --orca-cli, so PortScanHandler is reached only through runRelayDaemon, and both launchers start it detached with a log file (POSIX `> relay.log 2>&1`, Windows `1>relay.log 2>relay.err.log` via Win32_Process.Create). installRelayLogRotation then wraps both streams into relay.log, which is the file the documented diagnostics tail reads. Verified by installing the real rotation over a temp path and reading the file back. So the line surfaced -- but untimestamped, in a log whose format exists so reconnect flaps can be correlated with the events around them (#7773). relayLogLine is that format and the relay idiom in 41 other places, and "since when has this host been stuck on PowerShell" is most of what this line is for. The test spies on process.stderr to pin the stream and the ISO stamp rather than just asserting something was called, since a fall-through logged somewhere unread is the failure being guarded against. Also fixes a comment that ended its own block early: `relay-*/relay.log` in a doc comment contains `*/`. * fix(windows): keep the dominant zero-peer state when reading a localized netstat Shape alone promoted any zero-peer TCP row, not just listeners. `BOUND` and `CLOSED` print a zero peer too, and on a localized host their state words are exactly as unreadable as the listening one -- so a German host with listeners plus one BOUND socket published a phantom listener. Reachable on an English host too: with zero listeners a lone BOUND row is promoted AND, because the result is then non-empty, it suppresses the blocked-reader fall-through. Group the zero-peer rows by state word and keep only the largest group. A transient BOUND or CLOSED socket cannot outnumber the listeners (51 against 0 on this host), so this removes the class rather than special-casing the words, which would just be the localization bug again. An exact tie keeps every tied group rather than guessing -- no worse than reading shape alone. Verified against real netstat output: injecting a BOUND row into the localized capture leaves the result identical to the English answer (47 rows, no phantom 65001). The new test has teeth -- reverting the grouping fails it and nothing else. Corrects two claims that were slightly wrong: the docblock said shape was the fallback because BOUND prints a zero peer, which described the hazard without saying it was unhandled; and a test comment said an English host "never sees a bound socket", true only when it has at least one readable LISTENING row. Also gates the fall-through log per reason instead of per module, so a host that parses nothing today and truncates tomorrow reports both faults. Same one-shot cost, and the vocabulary is two fixed strings so the set cannot grow. That guard matters more than it looks: --log-file rotates stdout only, so the file stderr can land in is unrotated. * docs(windows): note the direction the zero-peer majority rule can fail in The docblock described the tie case and stopped there, which reads as a complete account of the limits when it is not: a majority rule inverts if the majority is wrong, and enough transient zero-peer sockets would publish the phantoms and drop the real listeners. Someone would reasonably have concluded the rule was safe in both directions. Trigger numbers and the repro stay in the PR discussion; the code only needs the reader to know the rule has a direction, and the hatch (defer to the PowerShell reader, which reads the state word instead of inferring it) since that is the part a future editor would otherwise re-derive. * ci(windows): run the real-netstat port scan suite in CI The win32 suite only self-skips off Windows, so it passed vacuously in every lane. Register it the way the cmd-shim suite is registered. * test(windows): lower both child-process ratchets to the ground this PR took Migrating the port scan off `node:child_process` onto `runProcess` drops `src/relay/windows-port-scan.ts` from both allowlists, so both offender counts fall by one. Each ratchet pins the count from below as well as above, so a pin left above reality fails and re-opens room for the next direct import to land for free. * docs(windows): qualify the no-PowerShell claim on the netstat scan The scan starts no PowerShell of its own, but no released relay carries the optional `windows-process-tree.node` addon (only dev-channel-win-build.yml builds it), so the shared process-table read falls back to a CIM scan that forks one `powershell.exe`. The EDR win is the removal of the `-EncodedCommand` / `-ExecutionPolicy Bypass` shape, not the elimination of PowerShell. Comment-only. * docs(windows): record the identity-reader follow-up and the perf table's addon attachWindowsProcessNames reads only `name`, so it should move to `readWindowsProcessIdentityTable` once #17866 lands -- on that PR's detailed reader it would open per-process handles for a field it discards. The reader does not exist on this branch, so the call stays as-is with the follow-up recorded rather than pulling #17866 in. The process-table perf table's two Toolhelp32 rows assume the optional `windows-process-tree.node` addon. The desktop bundles it; no released relay does, so on an SSH host the CIM row is the operative number. Comment-only. * docs(windows): state the CIM scan as the relay's normal path, not a fallback No released relay carries the optional `windows-process-tree.node` addon -- release-cut.yml has zero references to it and only dev-channel-win-build.yml builds it -- so the PowerShell CIM scan is what every SSH host runs. The call-site docstring read as a conditional fallback standalone. Comment-only. --------- Co-authored-by: Orca Worker <orca-worker@localhost> |
||
|
OrcaWinandOrca Worker
|
cff202c16a |
fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell (#17880)
* fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell
MDE flags `-ExecutionPolicy Bypass` paired with base64 `-EncodedCommand` as a
behavioural signal. Measured on Windows 11: neither `-Command` nor
`-EncodedCommand` is execution-policy gated (both run under an explicit
`-ExecutionPolicy Restricted` and `AllSigned`; only `-File` fails), so the
switch was a pure no-op on every one of these command lines.
Removes the switch from all four sites that spelled it, and de-encodes the one
site whose payload never passes through a re-parsing shell:
- ssh-remote-powershell: one chokepoint for ~40 remote-Windows call sites.
Base64 kept — the remote sshd DefaultShell re-parses this string.
- setup-agent-sequencing / windows-cmd-runner-delayed-launch: base64 kept —
these strings are typed into a terminal pane.
- windows-interactive-login-spawn: base64 kept — `cmd.exe /c start` re-parses,
and the cmd-safe-token guard rejects the `&` and `"` in the raw relay script.
- windows-mobile-firewall local runner: `-EncodedCommand` -> `-Command`, since
execFile reaches CreateProcess with no shell in between.
The setup startup gate keeps execution-policy relief in-payload (process scope),
because it evals a user-authored startup command that may invoke a `.ps1`, and a
`.ps1` IS gated. Caught by the real-process suite; mirrors the agent-hooks
launcher's trade.
The elevated firewall child deliberately stays encoded: `Start-Process
-ArgumentList` joins its array into one ShellExecuteEx string without quoting
and PowerShell re-splits on whitespace, measured to collapse `C:\My App\...`
to `C:\My App\...` — a firewall rule for the wrong program.
* test(ssh): enforce the no-script-file invariant remote payloads rely on
Dropping `-ExecutionPolicy Bypass` from `powerShellCommand` is a no-op only
while no remote payload loads a PowerShell script file — execution policy has
never gated anything else. That invariant held by inspection and was guarded by
nothing, so a future payload that dot-sourced, used `-File`, or imported a
`.psm1` would break only on a remote host with a Restricted/AllSigned
LocalMachine policy and no GPO: a failure on someone else's machine.
States the invariant at the wrapper, and adds a ratchet that scans every module
importing it for `.ps1`/`.psm1`, `Import-Module`, `-File`, and dot-sourcing.
The scan discovers importers itself (13 today) so new ones are covered, and
asserts it found some, so an emptied list cannot pass vacuously.
Mutation-checked: injecting each construct into a real importer fails the
matching case and names the file. The first dot-source pattern passed a
`;`-prefixed sample but missed `powerShellCommand(". '$x'")` — the likelier
shape — so the pattern now accepts a string-literal start and the self-test
samples carry their surrounding quotes.
* test(ssh): close two blind spots in the remote-payload ratchet
Both found by independent mutation testing of the ratchet itself, and both let
a real violation pass while the guard reported green.
`-File` was matched case-sensitively, so `-file $scriptVar` slipped through —
PowerShell switches are case-insensitive, and with a variable path the `.ps1`
pattern does not cover for it, so that shape escaped both nets. The naive fix
is wrong: bare /-File\b/i matches `--credential-file`, `--log-file` and
`--body-file`, which occur in three of these importers. Anchoring to a token
boundary catches the lowercase, odd-spacing and argv-element forms with zero
offenders across all 14.
Comment stripping paired a `/*` appearing inside a string (a glob such as
'src/*.ts') with any later comment close and deleted everything between, hiding
violations in the gap. Anchoring the block strip to line start, as the `//`
strip already was, fixes it — verified by injecting an `Import-Module` after a
glob string: the unanchored form misses it, the anchored form catches it.
Extends the same case-insensitivity to `.ps1`/`.psm1` and `Import-Module`,
which had the identical flaw (`import-module`, `DEPLOY.PS1` are legitimate
spellings); measured to add no false positive.
Each construct now carries the fixtures it must catch AND the near-misses it
must not, so a future tightening cannot quietly trade one for the other — the
negative fixtures are what would have caught the naive `-File` fix. Non-vacuity
bound tightened to >10 against 14 importers.
* docs(ssh): state what the remote-payload ratchet cannot see
The scan matches source text, so a script file reached only through a variable
(`& $scriptPath`) never appears in source and no pattern can catch it. The
ratchet narrows the hole; the invariant note on `powerShellCommand` covers the
remainder.
Recorded because a guard that reads as complete coverage when it is not is
worse than one that states its edge: the next author trusts it further than it
deserves, and should learn this limit from the test rather than an incident.
* test(ssh): scan remote payloads with the shared source walk
The ratchet had its own tree walk and comment stripper. The walk skipped
neither node_modules/dist/.git nor dot-directories and excluded tests by
`.test.ts` alone, so its importer count -- the guard's own goalpost -- could
be wrong about what it scanned. The stripper was anchored to line start to
dodge a `/*` inside a glob string, which silently skipped trailing comments;
`stripComments` tracks quote state and handles both.
Importer set re-derived against the shared walk: 15, floor unchanged at 10.
* fix(setup): report a failed execution-policy relief instead of swallowing it
The in-payload Set-ExecutionPolicy carried -ErrorAction SilentlyContinue and
an empty catch, so any failure vanished. A Windows PowerShell 5.1 install with
duplicate extended type data fails every cmdlet in Microsoft.PowerShell.Security
-- autoload, not policy -- and the user then saw only their own .ps1 being
refused, with no trace that the relief had been attempted or why.
-ErrorAction Stop is what routes a non-terminating failure into the catch at
all; the catch reports the FullyQualifiedErrorId to stderr and deliberately
does not rethrow, so a broken policy cmdlet cannot take down the startup this
gate exists to run. Success path is unchanged and stays stderr-clean.
Verified by execution on a clean child environment: success -> policy=Bypass,
stderr empty; shadowed failing cmdlet -> diagnostic on stderr and the gate
still continues; the old empty catch -> silent.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>
|
||
|
Neil
|
6f28e019b5 | perf(hooks): use native reverse search for transcript lines (#18936) | ||
|
Neil
|
388e9fb776 | perf: avoid rescanning emitted source in analysis guards (#18920) | ||
|
Neil
|
4204bdf717 | perf: avoid repeated Quick Open exclusion string allocations (#18916) | ||
|
Neil
|
926f3ff585 | perf(browser): assemble fragmented tunnel frames once (#18893) | ||
|
Neil
|
1c41d59203 |
perf(relay): drain fragmented frame buffers in linear time (#18891)
* perf(relay): drain fragmented frame buffers in linear time * style: follow block-body lint in relay buffer checks |
||
|
Jinwoo Hong
|
54a8afc91d |
fix(orchestration): typed error codes for dispatch and worker-start refusals (#18902)
* fix(orchestration): typed error codes for dispatch and worker-start refusals orchestration dispatch (and worker-start, which composes it) surfaced task not found, task not ready, and inject rejected as the same bare runtime_error, so an agent reading the receipt could not choose between creating the task, waiting on dependencies, or picking another terminal. Add task_not_found (data.taskId), task_not_ready (data.status, data.unmetDependencies), and inject_rejected (data.terminal, data.reason), each carrying data.nextSteps so every shipped CLI already prints the recovery. worker-start's not-ready refusal moves from task_not_startable to task_not_ready with the same detail. runtime_error stays for genuinely unexpected failures. Proven red-first from RpcDispatcher through the CLI's own failure formatting, plus an SSH bridge test that the host CLI's typed refusal relays unchanged. * test(orchestration): load CLI formatter at runtime in the dispatch-code test The composite node typecheck (config/tsconfig.node.json without --composite false, as CI runs it) rejects a static import of src/cli from a main test with TS6307. Load the formatter and error class dynamically behind narrow structural types, as the CLI/runtime boundary test does. * fix(orchestration): keep task_not_startable and split the CLI-format proof Review on #18902: - Drop task_not_ready. worker-start already published task_not_startable for a not-ready Task, so renaming it would change an existing receipt value under old clients. dispatch now emits task_not_startable too (it was a bare runtime_error before, so this is purely additive), with the new data.status / data.unmetDependencies / data.nextSteps. - Move the refusal receipts (code, message, data) into src/shared/orchestration-dispatch-refusal-contract.ts so the runtime emits them and the CLI test formats the identical envelope. The RPC test under src/main asserts toEqual against the contract; the new src/cli/orchestration-dispatch-refusal-format.test.ts feeds those same receipts to formatCliError / reportCliError. Neither tsconfig widens and the composite typecheck CI runs is clean. * fix(orchestration): keep published refusal messages and type the DB claim guards Codex review of #18902: - Every call site keeps the exact message it published on main ("Task not found: <id>", "only a ready Task can start.", "cannot retry from Dispatch"); the shared contract now takes the message per site and only owns the code and data. Baseline strings are pinned as literals. - createDispatchContext's own missing/non-ready guards, including the atomic-claim loser, now emit the same typed receipt instead of a bare Error, so a dispatch that races a status change no longer flattens to runtime_error. Covered by a dispatcher-level race test. - Invalid --retry-of keeps task_not_startable but now carries status, unmetDependencies, retryOf, and a retry-specific next step. - Dependency recovery text distinguishes waiting on running deps from retrying/unblocking failed ones. - CLI test adds an unknown-code case so the old-client claim rests on an assertion, not a comment; SSH test asserts exact stdout. - Guide table narrowed to the covered preflight cases; occupancy stays runtime_error and is named as such. |
||
|
Neil
|
abdee9ebd3 |
feat(automations): restore column sorting on the list (#18885)
The flat-table redesign in #16532 dropped the sort UI, orphaning AutomationListSortHeader, nextAutomationListSort and the whole AutomationListViewItem layer. Wire them back to the rendered list. Name and Last run become interactive header cells again; the other six columns stay plain text. Sorting now spans local and external rows as one list, so the panel renders per-row components from a single sorted collection instead of two independent sections. Two model fixes fall out of that: - View items key on the host-qualified row key, not the bare automation ID. The old builder predated automation-list-row-identity, so under All hosts two authorities returning the same ID collapsed in the sort tie-break. - sortAutomationListViewItems takes the locale as a parameter instead of reading getIntlLocale(). A hidden global read is invisible to a dependency array, and the list result is memoized. Keyboard traversal and focus recovery now read the sorted order, so arrow navigation matches what is on screen. The dead unified filter is removed in favor of the live row/entry filters the page already used. |
||
|
Brennan BensonandMerge Sim
|
f8780a2c86 |
feat(native-chat): stop monitored tasks individually (#18807)
* feat(native-chat): stop monitored tasks individually * test: expect Claude task stop capability --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Brennan BensonandMerge Sim
|
2513e21390 |
fix(native-chat): publish structured session status from the host so the sidebar never goes stale (#18776)
* fix(native-chat): publish structured session status from the host The sidebar learned whether a structured chat was mid-turn by replaying the session journal in the renderer, through a reader whose lifetime was tied to the chat pane. Hiding the pane stopped the reader before the turn's settlement arrived, so the row stayed on "working" until the chat was reopened. The same coupling meant a tab never opened this session showed no status at all, and a reloaded renderer lost every settled row. The host owns the journal, so it now projects each session's status once per journal publication and fans the changes out on one stream per client (`agentSession.subscribeStatus`). The projection survives eviction of an idle session's provider child and is republished when readable sessions are restored. The renderer bridge subscribes to that feed per runtime target and never opens a transcript reader; the observation hook is gone. Additive wire surface behind the existing structured capability; old hosts reject the method and the renderer retries, showing no status. * fix(native-chat): negotiate the status feed and stop losing a change on subscribe The status stream is additive to a surface that already shipped, so a host advertising agent-session.structured.v1 can still answer subscribeStatus with method_not_found. Every renderer error path reconnected, so a remote host one release behind got a relay round-trip every 5s and no sidebar status at all. Give the method its own capability and probe it before subscribing; a failed probe still retries, an absent capability does not. Re-projecting on subscribe also wrote straight into the shared cache, so a second client could pin the first to a stale summary. Route those diffs through publish() before the arriving subscriber is registered. * fix(native-chat): bound the status prompt, merge snapshots, and prove the unread path One status frame carries every retained session and a send admits 256 KB per prompt, so ~16 large-prompt sessions could push the snapshot past the 4 MB outbound guard and into the retry loop. Bound latestPrompt to the same 200-char single-line preview every other agent-status row already carries. A snapshot also replaced the cached map wholesale, so the empty first frame from a restarting host retracted every row before restore republished them. Merge instead; the tab map, not this feed, decides which sessions are listed. Tests: the hidden-pane claim now sits at the host, where a journal with no transcript subscriber is driven from running to idle; the RPC test reads a real projection instead of its own stub. * fix(native-chat): merge the duplicated status-event type import * test(native-chat): pin the restart status publication, and log the unsupported host Startup restore indexes a readable session and publishes its status, which is what puts a never-reopened tab back in the sidebar. Only an Electron screenshot covered that wiring; a sitting status subscriber now pins it directly. The terminal "host too old" branch was silent, so a mixed-version report showed an empty sidebar with nothing in the log to explain it. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
d7767fb196 |
perf(worktree): remove redundant creation and terminal startup work (#18793)
* perf(worktree): remove redundant creation and terminal startup work * test(worktree): cover optimized creation call signatures Preserve explicit branch adoption, WSL callback routing and sparse cleanup expectations. * perf: preserve user Git checkout worker settings * perf(git): skip malformed remote base probes * perf(cli): avoid loading other agent hooks for Codex preflight * fix(build): retain Codex preflight entry for packaged CLI * test(ssh): wait for replacement PTY before lease recovery input * test(ssh): verify recovered shell execution and lease ownership * test(electron): reap isolated macOS crash reporters on teardown * test: allow either observed self-exit snapshot ordering * test: capture frozen-host input recovery evidence |
||
|
Neil
|
51eed5a1bc |
feat(cli): report SSH host platforms (#18896)
* feat(cli): report SSH host platforms * feat(cli): include SSH connection status * fix(cli): preserve unknown SSH connection state |
||
|
Brennan BensonandMerge Sim
|
ddc5b75ac7 |
feat(native-chat): label Codex tool rows by what the command actually did (#18760)
* feat(native-chat): label Codex tool rows by what the command actually did
Codex's app-server `commandExecution` item carries `commandActions`, which
already classifies each command as a read, a search, or a directory listing
with the target path, name, or query extracted. Orca ignored the field, so
every shell call rendered as an undifferentiated row of raw argv.
Read it and name the row by its class, keeping the raw command and cwd for the
expanded view. Unclassified commands are untouched: absent, null, or malformed
`commandActions` produces byte-identical output to before.
Rank the search term above the command in the shared label keys so a classified
search row reads by what it looked for rather than the shell text that ran it.
No first-party tool input carries both keys today, so this only reaches the new
rows; an MCP tool supplying both would prefer its search term.
Note `commandActions` is the app-server spelling. `parsedCmd` is the rollout-file
shape and never arrives on this lane; a test pins that it stays ignored.
* feat(native-chat): give tool rows a category glyph beside their word
A row named only by a word makes the reader parse text to tell a read from
a search. Pair the word with an icon: icon for category, word for action,
argument for target.
Name the full eight-category vocabulary in `src/shared/native-chat-tool-icon.ts`
now — read/search/listFiles/unknown/fileChange/webSearch/mcpToolCall/
subAgentActivity — even though only the classified shell categories reach a row
today, so the MCP and web-search rows landing separately inherit these names
rather than coining their own. Glyph ids are the lucide spelling shared by
`lucide-react` and `lucide-react-native`, so mobile can resolve one name to its
own component when it adopts this; mobile rows stay text-only for now.
The glyph is decorative and `aria-hidden`: the word is the accessible name, and
never renders without it. One glyph per category, fixed across running,
completed, and failed — a row that swapped icons on completion would read as
changing identity — so the run header's active row also takes its category glyph
instead of the generic wrench it fell back to once these rows stopped being
called `shell`. A word outside the vocabulary gets the terminal glyph rather
than a blank slot, so rows stay left-aligned.
Also stand `.` in for a `listFiles` action whose `path` is null, which is what a
bare `ls` sends. The row named the action and then showed the raw argv as its
target; now it names the directory it listed.
* fix(native-chat): hold the tool run header's glyph fixed and size its slot to 16/14
The header swapped its leading glyph on settle: the active tool's icon while
running, a check once done. That is the identity swap a fixed per-category glyph
exists to prevent — the row appeared to become a different thing when it
finished. Name the header by the run's latest tool in both states and move the
completion check to the trailing edge, where the rest of the state signal already
lives.
Size both header slots to the mock's 16px slot with a 14px glyph, matching the
tool rows beneath them and the subagent summary row landing separately. They were
24/16, so the icon columns sat 8px apart and broke the left alignment the icon
treatment depends on.
The fixity test walks running, completed, and failed and pins the leading glyph
of every row by lucide's own class name, so a swap shows up as a different name
rather than a still-present icon.
* fix(codex): stop a classified shell row from asserting facts the command doesn't support
Three claims the `commandActions` row model was making on its own:
- `listFiles` with a null path was given `path: '.'`. Codex sends null for a
recursive walk and for the repo root, and the invented path flows into
`createToolInputDisplay().filePath`, which mobile turns into a tappable
"open file" link onto a directory — an affordance that can only fail. The row
now keeps the raw command, which is what the label logic already falls back to.
- A command whose actions classify as two different things (`cat a.txt && ls src`)
was named after the first one, silently dropping the rest. Recognized actions
must now agree on one class; a repeat of one class keeps the class and only a
target every entry names.
- `read` lifted `name` into the journal payload, where no label ever reads it —
`path` always wins — so it was bounded weight carrying nothing.
* fix(native-chat): give an unmodelled tool row a generic glyph, not a terminal
The row-word vocabulary named seven words, and everything else fell through to
the terminal glyph — which reads as "a shell ran here" for rows where nothing
says one did. Codex's own `apply_patch` row, `Grep`/`Glob`/`Task`/`WebFetch`/
`TodoWrite`, and every `mcp__*` tool all rendered a terminal, leaving the
declared `mcpToolCall` and `subAgentActivity` categories unreachable.
- Split the vocabulary: `unknown` stays the shell command Codex could not
classify and keeps the terminal, while a new `other` carries the generic
wrench that unmodelled words now fall back to.
- Read the edit family from `EDIT_TOOL_NAMES` and the command tools from
`isCommandToolName` rather than restating either. Command tools resolve first:
`isEditToolName` counts `shell`/`exec` as possible patch carriers, and a shell
row is not an edit.
- Result rows get no category glyph. Their word is `translate(…, 'Result')`, so
keying a category off it resolved a different glyph per locale; an empty slot
keeps the rows aligned.
- The header and the row now resolve through `NativeChatToolIcon`, so one `Grep`
run can no longer show a wrench in the header and a terminal on its line. The
glyph map and the unused `category` prop go with the duplication.
* fix(native-chat): give the projected Diff row the file-change glyph
Every Codex fileChange item projects to a tool call named `Diff`, which the
edit set does not name — it names the tools that carry the edit in their own
input. So a run whose body renders an edited-file card was headed by the
generic wrench.
* fix(codex): stop a classified shell row offering a folder as a file to open
A listFiles action's path is a directory, and a search action's path is the
root it scanned. Lifted under `path`, both became the row's file target, which
mobile renders as a tappable open-file link that can only fail — the same dead
link the removed `{ path: '.' }` stand-in would have produced. They lift to
`directory` instead, which still labels the row but is never a file target.
* fix(mobile): keep the terminal glyph on a classified Codex shell row
Mobile's run header picks between a terminal and a generic glyph by tool
name. Now that the host publishes `read`/`search`/`list` for the same
commands it used to publish as `shell`, that name check answers false and
a command that really ran heads its run with a wrench.
Ask the shared category vocabulary instead. Mobile keeps its two icons —
porting the full glyph set is a separate lane.
* fix(native-chat): say what the run header's glyph actually guarantees
The comment claimed the header names the same tool in both states, so its
glyph cannot change on settle. It can: the live header names the running
call while the settled one names the run's last tool call, and with
out-of-order completion those differ. The glyph is fixed for whichever
tool the header names — say that, and drop the never-taken running branch
from the settled header's call.
Also pin the other half of the file-target rule: `read` keeps `path`, so
its row stays tappable, where `list`/`search` lift a folder to
`directory` and offer no target at all.
* fix(native-chat): give a rollout-transcript shell row the terminal glyph
`exec` and `local_shell` are what the Codex rollout transcript names a
shell call — `native-chat-edit-normalize` already treats those three
words as the command tools — but the activity set the glyph vocabulary
reuses carries neither, so both rows headed a real command with the
generic-tool wrench.
Named in the vocabulary rather than in that activity set, because that
set also picks the running row's copy and this is only about the glyph.
* fix(mobile): pick the run-header glyph from the call's input, not its word
Codex now names a classified shell row `read` / `search` / `list`, which
lowercase to Claude's own `Read` / `Grep` / `Glob`. Mobile has only a terminal
and a wrench, so keying that choice on the row word gave Claude's filesystem
tools a terminal for a shell that never ran.
The input separates them: Codex keeps the raw command on a classified row,
while Claude's `Read` carries only a file path. `isShellActivityToolCall`
replaces `isShellActivityToolRow` and asks the command tool names first, then
the call's input.
* fix(native-chat): give the projected diff fixture its required digest
* fix(native-chat): head a settled run with a glyph the whole run shares
The settled run header drew the glyph of the run's last tool call while the
text beside it summarizes the run's first three, so a ten-call run ending in a
`read` showed an eye above "shell npm test · shell git status · …" — a category
the summary never described.
Resolve the header's glyph from every call in the run instead: the shared
category's glyph when all agree, the generic tool glyph when the run spans
categories, and no glyph when there are no tool calls. The running header still
names the active call, whose glyph is true of it.
---------
Co-authored-by: Merge Sim <sim@local>
|
||
|
Neil
|
0bbf86bb7a |
fix(renderer): stop a Node-only process-table module blanking the app at boot (#18814)
Every Electron E2E spec that boots the app has been failing on `workspaceSessionReady did not become true`, and the app itself has been launching to a blank white window: the renderer threw `ReferenceError: process is not defined` while evaluating a shared chunk, so React never mounted and no startup step ever ran. `agent-completion-poll-interval.ts` (renderer) imported one constant, `PROCESS_TABLE_SNAPSHOT_MAX_STALENESS_MS`, out of `shared/process-table-snapshot-reader.ts` — a `node:child_process` / `node:fs/promises` module whose dependency evaluates `process.platform` at module scope to pick `ps` columns. The renderer runs sandboxed with contextIsolation, where `process` is undefined, so that module-scope read threw and took the whole chunk with it. Introduced by #18742; #18780 added a second module-scope read next to the first. The constant now lives in `shared/process-table-snapshot.ts`, the environment-neutral half of the pair, and the reader re-exports it so host callers are unchanged. The two `ps` column sets read the platform behind a `typeof process` guard, which defuses the same landmine for any future renderer import of that module — only hosts ever run the argv. The regression test walks the renderer import graph (lazy routes included) from all three entries and refuses any module that reaches a `node:` builtin. It fails on the pre-fix import with the full 10-hop chain from `main.tsx`. |
||
|
Neil
|
e95d247be1 |
perf(terminal): cheap-tier process inspection for anchored local agent panes (#18780)
* perf(terminal): cheap-tier process inspection for anchored local agent panes Every idle local pane's completion cadence ran a full whole-host `ps` (with `tty=` and `command=`, 0.34-0.50s on a 1,900-process Mac, 1.15s on Linux) purely to build `foregroundProcessEvidence` that the renderer then discards for local ids. Add a cheap tier (same job-control columns, no tty/command, 0.03s) gated so that it introduces no user-facing trade-off: - Only a pane whose last FULL capture proved a recognized agent may take the cheap tier. Panes with no anchor always take the full capture, so start discovery keeps today's exact behaviour. - The cheap tick compares a per-pane fingerprint (root shell pid+start, tpgid, every descendant's pid+start+pgid+job-control state). Any change, a changed node-pty foreground name, an unreadable capture, or an incarnation mismatch escalates to the full capture. A recognized agent's exit is always a pid vanishing, which the fingerprint always sees. - A cheap answer OMITS evidence rather than fabricating a tty-less fence. Remote/restore consumers never send `steadyState`, so they keep the full capture unchanged. - `steadyState` is a new optional request field; an old daemon ignores it and answers with the full capture. Measured (8 idle panes, 60s, idle cadence, forks counted by column set): 30 full -> 1 full + 29 cheap. * fix(terminal): route the cheap ps capture through runProcess The cheap-tier reader imported node:child_process directly, which the child-process import-boundary and windowsHide ratchet tests reject (CI shards 1/8 and 3/8). Use Orca's single spawn entry point instead; it pins windowsHide and encodes argv. Map its result onto the capture-error vocabulary: outputTruncated -> capture_truncated, timedOut -> capture_timeout, non-zero exit -> ps_exit_<code>. Tests mock at the runProcess seam. * fix(perf): refuse a pane fingerprint when any descendant start marker is missing `buildPaneProcessFingerprint` rejected only a missing root start marker; a missing descendant marker was stamped as `?`. Two captures that both failed to read the same descendant therefore compared equal, which removes the pid-reuse protection the fingerprint exists to provide: a recycled pid could make a vanished agent look unchanged, and the cheap tier would keep serving its name instead of escalating. Reachable on Linux, where `readLinuxProcStartTime` legitimately returns null when a process exits between the `ps` capture and the `/proc/<pid>/stat` read. Every subtree member now needs a start marker or the fingerprint is refused, which sends the caller to the full capture — the same conservative default every other uncertain path takes. Reported by CodeRabbit on #18780. The two new tests fail against the previous code with `expected '4242@2400#4300:|4300@?:4300:+' to be null`. |
||
|
Brennan BensonandMerge Sim
|
cc07249e78 |
fix(agent-session): refuse a pre-commit structured create with an envelope (#18697)
* fix(agent-session): refuse a pre-commit structured create with an envelope The create route refused by throwing, which reaches a client as a generic transport error indistinguishable from a lost answer — so desktop parked the launch as visibility-unknown with no chat and no terminal. Convert the whole pre-commit span, everything before `attach`, into a refusal envelope carrying a code, and name the definitive-refusal allowlist the fallback decision needs. * fix(agent-session): gate legacy fallback on definitive refusals * fix(mobile): preserve unknown structured create outcomes --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
4c5077d57a |
perf(persistence): skip rewriting unchanged terminal scrollback snapshots (#18764)
* perf(terminal): tighten the partial-escape-tail benchmark and equivalence test
* perf(terminal): spell the ESC gate the same way as the sibling ingest gates
* test(terminal): differential-fuzz the ESC-free partial-escape-tail gate against the unguarded fold
* test(terminal): make the escape-tail fuzz exhaustive at symbol depth, and cap the fold expectation
Two review findings on the differential fuzz, both about the test faithfully modelling the
function it guards.
The odometer generated strings by symbol depth but the caller filtered on `chunk.length`, which
is the UTF-16 code-unit count. An astral symbol is two code units, so every depth-4 string
containing one was silently skipped and the corpus was not exhaustive at depth 4 the way the
test name claimed. The generator now yields `{ depth, text }` and the caller filters on depth.
That restores the missing strings and takes the pinned corpus from 516,566 to 593,468 - exactly
the count CodeRabbit derived for the intended corpus.
The pairing assertion in the sibling suite compared the capped `advancePartialEscapeTail`
against an uncapped `extractPartialEscapeTail(pending + chunk)`. It passed only because no
pairing in that corpus crosses MAX_PARTIAL_ESCAPE_TAIL_LENGTH; it would have stopped modelling
the function the moment one did. The cap now lives in the expectation, matching the fuzz
oracle.
Re-verified the fuzz still fails on a wrong guard: mutating the gate to a bracket check fails
all four tests with a `gate diverged` assertion on a lone ESC chunk.
Reported by CodeRabbit and pullfrog on #18748.
|
||
|
Brennan BensonandMerge Sim
|
e89deb63c9 |
Show Claude background task status in Native Chat (#18757)
* feat(native-chat): show Claude background task status * fix(native-chat): carry background task fence forward * fix(claude): bound background task stop requests * Show running Claude background task details * Harden Claude background task status updates --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
c36dd5f4c7 |
perf(terminal): skip the partial-escape-tail walk on ESC-free PTY chunks (#18748)
`advancePartialEscapeTail` runs once per PTY chunk, on the main thread, for every terminal — visible, hidden or parked — inside `HeadlessEmulator`'s write path. It unconditionally concatenated the pending tail with the whole chunk and then walked the result one code unit at a time through a VT500 state machine. `extractPartialEscapeTail` only leaves `ground` on an ESC byte, so with no pending tail and no ESC in the chunk the answer is always ''. Taking that case up front skips both the full-chunk concat and the walk. `String.prototype.includes` is a native scan, so the gate costs essentially nothing on the chunks it does not short-circuit. This is the same gate its two neighbours on the very same ingest path already apply — `TerminalOscCwdTitleScanner.scan` and `TerminalMouseModeMirror.scan`, both carrying a comment citing their measured share of a 2.2x ingest regression. This call was simply missed. Measured by `pnpm bench:terminal-partial-escape-tail` over 640 x 16 KB chunks (10 MB), median of 7 rounds: | stream shape | before | after | | | --- | --- | --- | --- | | ESC-free (build logs, `cat`, piped output) | 40.3 ms | 0.19 ms | 213x | | SGR-coloured output (gate does not apply) | 30.6 ms | 32.1 ms | 1.0x | The benchmark proves equivalence over a 226-case corpus before timing, and a new unit test pairs every pending-tail state the scanner can be left in against every chunk shape, asserting the gate is indistinguishable from the unconditional fold. |
||
|
Brennan BensonandMerge Sim
|
172aa1ac35 |
feat(native-chat): render agent file edits as inline diff cards (#18765)
* feat(native-chat): render agent file edits as inline diff cards An agent's file edit rendered as a flat list of every removed line followed by every added line, with no interleaving, no file header, and no line numbers. A Codex edit on the transcript lane rendered no diff at all: the patch arrives wrapped in the source string of its `exec` tool, which matched none of the shapes the old parser looked for. Adds one diff model shared by every edit shape the supported agents produce: - `native-chat-edit-lcs` interleaves a snippet pair, falling back to a linear prefix/suffix diff above the quadratic guard. - `native-chat-unified-patch` keeps the `@@` ranges as per-row line numbers instead of parsing them into display text and discarding them. - `native-chat-begin-patch` recovers the `*** Begin Patch` envelope from the JavaScript string literal Codex sends it in, so that lane renders a diff. - `native-chat-edit-normalize` folds all of it into one model, including the two Codex shapes that do not look like diffs: add and delete arrive as raw file content, and a rename is appended to the body as prose. Claude reports an edit as a snippet pair, which cannot locate the change in the file, so its result's resolved hunks are now carried on the tool-result block and preferred when present. The field is optional, so an older client reading a newer journal simply drops it. Where no resolved ranges exist the gutter stays blank rather than showing a snippet-relative number, which would read as a file position. The card renders the verb from the observed change kind rather than the tool name, pairs an edit's call and result into a single row, and takes its row and gutter grounds from new tokens derived from the git status palette, replacing the hardcoded Tailwind tints the old view used. Desktop only; mobile chat keeps its existing renderer and parser untouched. * fix(native-chat): stop the diff card from asserting an edit it cannot prove Every defect here shares one failure mode: the card stated something the input did not support, and stated it confidently. Parsing: - A hunk no longer ends on `--- `, `+++ ` or `\ No newline`. The first two are what a removed `-- comment` (SQL/Lua/Haskell) looks like once the marker is prepended, so they truncated the whole diff; the no-newline marker is emitted mid-hunk, between the removed old last line and the added new one. Real headers are recognised through `isFileHeaderPair`, lifted out of `native-chat-diff` so the rule has one home. - A `*** Begin Patch` envelope with no `*** End Patch` is declined. With no closing marker `indexOf` returned -1 and the slice swallowed the rest of the command line, so `… +y" && echo ok` rendered as file content the agent never wrote. - One splitter serves every shape, so a CRLF patch no longer keeps a `\r` on each row, in the phantom-row guard, or in the clipboard. It also tests for the trailing newline on the clipped body: on the un-clipped string that test deleted a real line whenever the slice fired. - Truncation is carried from each slice site to the card, so content past the character cap can no longer render as a complete unchanged file with no "Diff truncated" footer. Attribution: - A failed or still-running edit renders no card. It kept the generic tool view, whose result block carries the provider's own error — the card had been drawing "Edited file +1 −1" from the input while hiding the red error body, which is worse than what preceded this feature. - The result-as-patch fallback is scoped to `Diff`, the one tool whose call carries only a path. Any command tool's output could previously be read as a patch, so `git diff` through `exec` was reclassified as an edit of a file named "file" and its command line disappeared with the result. - A whole-content write claims a creation only on evidence — the editor tool's own `create` command, or the provider reporting one. Overwriting a large existing file had always read as "Added file". - `MultiEdit` reads its `edits[]`, and `NotebookEdit` leaves the set: it carries only the new cell source. Both previously fell through to the old renderer, so one turn could show two diff presentations at once. - Snippet-relative numbers are dropped at the model layer rather than hidden by a zero-width gutter, which the flex min-width floor re-exposed on top of the marker and the first characters of the row. The run memoizes its edit model, so a collapsed group no longer re-diffs on every streaming token, and the card's copy button says what it copies. * fix(native-chat): keep every edited file, and mark where the diff breaks A run of hunks was concatenated into one flat row list, so the gutter jumped from one region of the file to a distant one with nothing between them and the reader saw two unrelated spans as one continuous block. Rows now carry an explicit break: it holds no text and no position, counts toward neither side of the change, is trimmed from the end where it would mark nothing, and is left out of the copied text. The patch envelope lost files, and lost them silently: - An update chunk may carry no hunk header at all. The parser required one, returned nothing, and the caller dropped that file from a multi-file envelope with nothing to say it had gone. A header-less body now opens as a hunk of unknown position, and whether the rows are locatable is read off the rows themselves rather than off the header. - The envelope's own control lines rendered as content rows in the card. - A delete names its file and carries no body, which rendered as a card with an empty expandable row list. The header states the change and offers no disclosure behind it. - The header patterns are anchored and `.` excludes a carriage return, so a CRLF envelope matched no header at all and produced no card whatsoever. The envelope is split on both newline forms once, up front, rather than each pattern having to tolerate the extra character. A tool call's argument payload arrives as a string holding JSON. It was passed along undecoded, which is the only reason this code carried a hand-rolled string-literal unescaper. It is decoded once at the transcript decoder now — defensively, since the transcript is untrusted, so anything that is not a JSON object is left exactly as it arrived — and the unescaper is gone. Recovering the envelope no longer guesses at argument names either: it looks at the values, including the words of an argument vector, which is where the envelope actually sits once the payload is decoded. * fix(native-chat): only read a patch where a patch was actually run Recovering the patch envelope from any value of a tool's payload meant a write's own content was searched for one. A file documenting the patch format rendered a card for the file its example names, while the file actually written never appeared at all — the call and its result were consumed by that card, so nothing was left to correct it. Two changes: the envelope is recovered only for the tools that run one, never for a file edit whose payload is content; and only patch- or command-bearing arguments are searched, still including the words of an argument vector, which is where the envelope sits when a command tool applies it. The call payload is decoded back where it is needed rather than at the transcript decoder. Decoding it there changed the shape every reader of a tool's input sees, including the surface that recognises a question payload from any tool by shape alone: a tool whose arguments happened to carry that shape raised a question card pinned over the composer. That decode now happens inside the envelope recovery, the one consumer that needs the structure. A card also states an edit as made, so it now takes evidence that it landed — the provider reporting the call complete, or a result that is not an error. A turn that stopped before its call was answered reported an edit that may never have applied. This replaces the working-turn heuristic in the view, so the rule lives in one place. Two files still went missing. A multi-file patch has no per-file split, so it rendered as one card under the first file's name, with the later files' rows and their gutter numbers beneath it — a card asserting a false file position. Patch text is now split on its file boundaries, one card per file, each named by its own header, with a rename and a `/dev/null` side read from the same headers. And an envelope section that names a file but carries no body was dropped rather than reported, which is the same silent loss the delete case was fixed for. * fix(native-chat): type the patch-section scan and its test helper call The section under construction was only ever assigned inside the helper that opens one, which control-flow analysis does not see, so the variable stayed narrowed to its initial null and reading a field off it did not compile. The helper now only builds and records a section; the loop owns the assignment, which also fixes a real leak in the fall-through row: it opened a section it never made current, so the next row opened another one. The multi-file case also passed a possibly-undefined slice to a helper that takes an array or null. * fix(native-chat): stop the patch lane naming files it cannot name Splitting a patch into its files only ever looked for a boundary outside a hunk, and nothing reopened that state once the first hunk began, so every file after the first was swallowed as the first one's body. A `--- `/`+++ ` pair inside a hunk is now a boundary too, but only when a hunk header follows it immediately: a removed `-- x` over an added `++ y` is never followed by a column-0 header, which is what keeps the guard against reading content as structure intact. One producer cannot be recovered by any parser: it joins several files' patches and keeps a count where the path goes, so nothing in what reaches here names a file. That shape is refused rather than rendered under a name no file has. Recovering the per-file paths belongs to the producer and is filed separately. A clipped body carries its own marker in its text, and the bound that clips it is six times smaller than this module's, so it fires first. Read as content, the marker became a numbered line of the file and the rows before it were reported complete. It is recognised at the end of the text, removed, and reported as the truncation it is — the footer says so and the copied text no longer carries it. Also: a move appended to the body as prose is now read as a rename on every lane that carries the body as text, not just the one that also carries the destination as a field, where it had been rendering as a numbered line of the file it moved. The call's own path no longer wins over a rename's destination, which is only ever in the header, and only sections that name a file count toward deciding whether the call names the one file at hand. A command that merely quotes an envelope — writing documentation about the format — must now also invoke the tool that applies one. And two compared directories are no longer called a rename: only a header that states both sides as such is evidence of a move. * fix(native-chat): anchor the move marker to its own line The marker a producer appends to say where a file moved was matched anywhere on the body's last line, so a row whose own content mentions a move was cut in half at that point and the file it named claimed as the destination of a rename that never happened. It is now anchored to the start of the final line, on both lanes that carry the body as text. The command that applies a patch envelope has a second spelling the runner accepts and runs; requiring the first one refused a patch that really landed. Both are accepted, still matched against whole argument words rather than the payload at large. A clipped diff also said so only under its own rows, where a collapsed card — or one clipped down to no rows at all — showed nothing. It sits beside the change counts now, which are visible either way. * refactor(native-chat): tidy what the diff-card work left behind The copy text is joined from every row of the diff, which a collapsed card renders none of, and it was rebuilt on every render to seed a prop. It is memoized on the rows, matching how the run memoizes its edit model. The two scanners that read patch text kept the same file-section alternation verbatim, so they could drift apart while both looking correct; there is one definition now, beside the header-pair rule that already lives there. Also: the row that marks a break between regions is built in one place, so it is no longer exported; the move destination in the envelope reader was a function-wide binding written and read within one iteration, which read as if a move carried between sections; and a test comment named the wrong mechanism for keeping a card collapsed. Adds the missing pin on what the copy affordance actually copies. --------- Co-authored-by: Merge Sim <sim@local> |
||
|
Neil
|
149732df6d | perf(persistence): stop the session write re-scanning and rebuilding unchanged state (#18739) | ||
|
Brennan BensonandMerge Sim
|
b0c67eaf88 |
feat(mobile): port the restructured native-chat turn status and live tool progress (#18761)
* feat(mobile): port the restructured native-chat turn status and live tool progress Mobile chat had a single static "Agent is working" row and no live tool activity, while the desktop restructure (#17597, #18705) replaced that with a per-turn status row and a running-tool label. This brings mobile to parity and puts the derivation in one place instead of two. Shared (new, pure, RN-safe — desktop uses them as i18n fallbacks, mobile directly, matching the native-chat-empty-state pattern): - `native-chat-turn-status.ts`: duration formatting, label selection, the turn-timing state machine, and the active/settled split. - `native-chat-tool-activity.ts`: command-tool classification, the running-tool label descriptor, and running-call selection. Desktop now consumes both; `NativeChatWorkingStatus`, `NativeChatToolRun` and `use-native-chat-turn-status` keep their existing behavior and strings. Mobile gains the "Thinking" / "Working for 12s" / "Worked for 3m 4s" row with a caret that discloses the turn's tool activity, the pulsing "Running npm test" row with terminal-vs-wrench glyphs, and desktop's rule that a completed turn's tool run hides behind the turn caret. The bridge lane is untouched and keeps its three-dot indicator. Headings, quotes, code, lists and table cells are now selectable. Files at their max-lines cap were split rather than bumped: the tool-run subtree, the prompt card, the session-lane wiring, and the turn-disclosure state each move to their own module. * perf(mobile): stop the turn-status rows from re-rendering the whole transcript A streaming turn re-renders the chat list many times a second. The disclosure wiring handed every row a fresh status object and a fresh toggle closure on each of those renders, so `MobileNativeChatMessage`'s memo never held and every visible row re-rendered per tick — including settled turns that had not changed. Memoize the status selection on the timing map, and keep one stable toggle handler per turn (pruned when a turn leaves the transcript) attached only to the settled rows that can actually disclose anything. Now only the live turn's row changes identity while the agent works. * fix(mobile): keep the turn clock running when the optimistic echo is replaced An accepted send renders as `pending-N` until the transcript echo lands under its real message id. That flips the active turn key mid-turn, and the timing reducer treated the new key as a new turn — so a turn that had reached "Working for 8s" visibly restarted at "Working for 0s". The reducer now carries the start over when the previous key names a turn that has since left the transcript, which is exactly the echo-replacement case. A genuinely new turn (the previous key still in the transcript) and a turn that had already settled both keep their own clock; both are pinned by tests. Desktop does not pass the new key and is unaffected. * fix(mobile): keep the Tools toggle working on settled turns Hiding a settled turn's tool run behind the turn caret (desktop parity) also made the composer's global Tools control a no-op on every completed turn: the run it wanted to expand was not rendered at all. Let that toggle override the hiding, so it still reveals every run at once the way it did before. * fix(mobile): re-key the turn timing instead of only carrying its start The previous fix carried the start forward only while the turn was still working. When the transcript echo landed after the turn had already settled, the new key inherited nothing, the settled timing was pruned with the old key, and the turn's "Worked for N" row disappeared entirely. Move the timing onto the new key instead, which covers both orderings: an in-flight turn keeps counting from its original start (and later settles against it), and an already-settled turn keeps its duration. Both orderings are pinned. * test(mobile): pin the structured turn-status wiring at the view level Emulator QA could not reach the structured lane (mobile's Create Tab -> Codex falls back to a terminal tab when agentSession.createSupport says unsupported), so the view's own lane wiring had no coverage — the one seam between the shared turn-timing reducer and the rendered rows. Assert what the view hands each row: the live user turn gets a status object and the three-dot indicator is gone on the structured lane; the bridge lane keeps the indicator and gets no status; a finished turn settles to a numeric duration with a toggle; and an assistant row never carries a status row of its own. * fix(mobile): isolate structured chat turn state * fix(mobile): let the capability RPC actually store what a phone advertises `runtime.clientCapabilities.update` records the advertised set by assigning `authenticatedSocket.clientCapabilities`, but the socket handed to the dispatcher defined that property with a getter only. In strict mode the assignment throws `TypeError: Cannot set property clientCapabilities ... which has only a getter`, so the RPC answered `runtime_error` and the set was never stored. The consequence is not subtle: `supportsStructuredAgentSessions` requires the capability, so `projectSessionTabAgentStatus` removed every `agent-session` tab from a phone that had advertised it correctly. A paired phone saw ZERO tabs on a worktree whose only tab was a structured Codex chat — structured native chat was unreachable on mobile over this transport, not just missing its new turn UI. Give the socket a setter that writes through to the channel, which already owns the set for the connection's lifetime, so later requests on the same socket see it. Found while trying to capture emulator screenshots of the turn-status port: two full QA runs reported the new UI "missing" because the phone could only ever get a bridge/PTY tab. * fix(mobile): carry the turn key instead of caching a handler in a ref Builds on the scope-isolation fix: that kept (and extended) a ref that is written during render — once to memoize a per-turn handler, once to prune dead turns, once to reset on a scope change. React Doctor's "Ref mutated during render" is what CI's `check:react-doctor:changed` was failing on (x2), and on mobile it is a real hazard rather than a style note: react-freeze discards renders, and a discarded render would leave the cache mutated. Pass the settled turn's key down the row instead and let it call one stable handler with it. That preserves both properties the cache was bought for — per scope isolation, and identity stability so a streaming transcript does not defeat the row's memo — with no ref writes and no pruning to get wrong. The scope-keyed expanded set and the 128-turn cap are untouched; their tests move to the new contract and one now pins handler identity across a re-render. Note for future changes here: `check:code-quality:changed` does NOT cover this. CI additionally runs the standalone react-doctor CLI, which has rules the oxlint plugin config does not enable. * fix: ship native chat status translations * test(native-chat): pin the shared copy against the English catalog The shared constants are desktop's i18n fallback and mobile's actually-rendered string. If one changes without the other, desktop keeps rendering en.json while mobile renders the constant — and nothing fails, because a fallback is only used when the key is missing. That silent divergence is the exact thing the shared module exists to prevent, and it is now reachable precisely because these strings are runtime-required rather than statically extracted. Assert every key in both shared copy objects matches en.json byte for byte, plus the interpolation placeholders the catalog interpolates on. --------- Co-authored-by: Merge Sim <sim@local> |