37 Commits

Include renames

Author	SHA1	Message	Date
Brennan BensonandClaude	7a24d3d335	fix(native-chat): the conversation outlives its agent (#22835) ... * refactor(native-chat): remove the unused terminal handoff No client ever called agentSession.requestHandoff or mounted the handoff chrome. Delete the handoff coordinator, the terminal-owner runtime, the proof write path and the unmounted UI. Keep agentSession.handoffStatus, which released desktop clients read for worktree activation, and let records an older build left mid handoff reconcile through the ordinary restart and recovery paths. * fix(native-chat): never let the pre-stop snapshot hold a chat's stop Eviction now drains delivered events before quit's resume-offer snapshot. An unbounded wait there sits ahead of the provider stop, so a sink whose journal write stalls kept the child running until the step deadline aborted the eviction. The offer is advisory: bound the drain and stop the child regardless. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(native-chat): drop helpers only the terminal handoff called `claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and `queryWindowsProcessRowsFresh` lost their last caller with the handoff. The fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`, the teardown path that still depends on that contract. Co-Authored-By: Claude <noreply@anthropic.com> * docs(native-chat): stop citing the removed handoff in lifecycle comments Six comments still named the handoff coordinator, a handoff suspend, or a terminal-owned session as live participants in the flows they describe. Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): type the stalled snapshot drain without a cast Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): pin that a start dead before proving owes no settlement The removed restart handoff test pinned this branch; nothing else did. Co-Authored-By: Claude <noreply@anthropic.com> * fix(native-chat): keep the owner-status read behind an in-flight attach The handoff removal dropped the per-session queue from `handoffStatus`, so a read landing mid-start reported the reservation (no owner) instead of the settled chat owner, and shipped desktop clients blocked worktree activation on it. The read is queued again, as it was before the removal. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(terminal): remove the agent-session PTY write gate The gate only refused a write when a PTY had been bound to a chat session, and the only code that ever bound one was the terminal handoff this branch removes. With it gone, every admit/readmit returned "admitted" unconditionally, so the checks on the renderer write path, the runtime controller backstop, terminal.send, agent prompts, preview input and orchestration pointers, the refusal fields on terminal.send and worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane orchestration routing could no longer run. Ordinary writes take the same path in the same order as before. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(native-chat): drop the transcript helpers only the handoff called appendLegacyTranscriptMessages fed the terminal transcript catch-up and proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost their last caller with the handoff. Their tests now go through the live entry points instead: the roster bounds through the legacy import, the pinned-read and growth tests through the ancestry replay the history window uses, and the marker rules through the string proof in their own file rather than the session-file resolver's. Co-Authored-By: Claude <noreply@anthropic.com> * fix(native-chat): stop calling a starting chat "mid-handoff" A send refused because the chat's owner is not settled showed "The session is mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that reach it are a chat that is still starting, or one whose previous agent process has not yet been confirmed stopped. The message now says which of the two it is. The refusal code is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): type the stand-in roster decoder without a cast Co-Authored-By: Claude <noreply@anthropic.com> * refactor(codex): name the pinned rollout lookup for what it does With the terminal handoff gone, the module named codex-tui-rollout-proof holds only the pinned rollout lookup that structured Codex launches use to resume a thread, so the name described code that no longer exists. Rename the module and its options type. Also drop a mobile allowlist assertion that pinned the removed agentSession.requestHandoff method, which no longer exists to allow. * refactor(native-chat): type the owner-status reply as the host sends it The handoffStatus reply type still listed the terminal handoff's fields and states (terminal placement, host label, proof retry, queued and waiting phases, the to-terminal direction). No host writes them any more and the only client reader parses the reply as unknown, so they described nothing. The reply on the wire is unchanged. * refactor(native-chat): normalize terminal-handoff lease values once at decode Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types still admitted them, so readers across the host kept branches for values no path produces and the compiler could not point at them. The store now validates the on-disk shape, which still accepts those values so an older record is not quarantined, and maps them once while parsing: - `preparing` and `old-owner-stopped` become `recovering` - a `tui` lease becomes `native`; when it records a process it also becomes `conflicted`, the claim every build probes but never stops. A plain native owner would be stopped by restart recovery, here and in older builds. Revisions are taken over the normalized state on both sides of every compare, and the mapped record reaches disk with the store's first transaction, the same way the tab-id backfill does. The in-memory types narrow to what this build writes, and the branches that existed only for the removed values go. Structured-worker identity keeps its verdict for a former terminal owner by refusing a conflicted claim rather than a non-native kind. * refactor(native-chat): stop threading the owner kind through a reservation A reservation only ever names a native owner now, so the request no longer carries a kind and the reserved lease records `native` directly. The attach params keep `runtimeKind`: agentSession.ensure and create accept it, and the operation fingerprint stored in the ledger covers it. * test(native-chat): pin the legacy-lease rewrite with a transaction that changes nothing else Hiding a tab also committed the visibility index, so the no-op transaction wrote the file even when its open-time revision was wrong. Committing the index first leaves the pending rewrite as the only reason to write. * fix(native-chat): name a chat write by its target, not the owner generation A write carried the fence of the last frame the pane read, and the host refused it unless that fence was still current. An idle release and the restart after it each move the fence, and the release publishes nothing, so a send after a release was refused "Expected runtime fence 1; the session is at 3", and a Stop queued behind a cold start was refused as stale. Every write already names what it acts on: a send its conversation, a cancel its turn, a prompt answer its item revision, a rewind its epoch; an option is last-writer-wins. So admission stops comparing the client's fence, and the rebase that papered over one restart (admitAtResumedFence, resumedFromFence) goes with it. The writer-lease check stays, and so does the attach's compare-and-swap. Frames now stamp the fence read when each frame is sent instead of a copy each subscriber kept, which went stale on the same release. * fix(native-chat): every journal append reaches the chats that are open A journal write and its delivery to open readers were two calls, and some writers made only the first. A failed start whose lease could not be handed back, a provider revision with no frame behind it, and eviction's settlement were all journaled without reaching an open chat. A journal handle now reports every durable change, and the host's session map binds that report to the session's readers when the handle is set. Writers no longer publish what they append; the per-writer publish calls are deleted. * test(native-chat): an epoch replacement reaches the open chat * test(native-chat): each row reaches an open chat once, and a live handle enters only through the map * test(native-chat): give the legacy-lease store test a tab id so the backfill cannot supply its rewrite The seeded record had no surface tab id, so the next open backfilled one and that rewrite alone made the no-op transaction write. The test passed with the legacy-lease rewrite signal removed. * test(worktree-activation): restore the OMP surfaced-agent resume test The handoff removal deleted it alongside the terminal-owner tests, but it covers the surfaced-PTY block that still guards resume, including an agent whose ownership is unknown. * perf(native-chat): a publish behind a delivered commit reads nothing Each commit now delivers itself, so the publish a provider frame still sends afterwards found every reader caught up but still read rows and rebuilt the timeline for each one. A caught-up reader now skips the read. * test(native-chat): state why the teardown test's fake journal is safe to cast * docs(native-chat): say mutation admission checks only the writer lease * docs(native-chat): drop the send rebase from comments that still described it * fix(native-chat): a message is accepted, then delivered A send to a chat with no running agent restarted the agent inside the send call, before the message was recorded, so the client waited for the whole start and a failed restart refused the message. Claude held prompts sent during startup, and those could settle as "unconfirmed". A send is now accepted inside the session's serialized queue: one ledger row and one submission row marked handoverRecorded, published, answered pending. A per-session delivery loop exists while a message is queued. It starts the agent through the same serialized attach a hold uses, waits outside the queue for a Claude child to prove its start, and hands the oldest queued message over as its own serialized step, writing dispatch{pending} before the adapter call. A start it needed and did not get writes one error-tone row and rejects every queued message with the same words; a start Stop cancelled writes none. Settlement follows from the rows. A queued message is provably unwritten, so a close, an eviction or an exit rejects it. A handed-over message stays in doubt. A queued row at or below the sequence a handle found when it opened was left by an earlier process and is rejected at open, with no latch. Stop withdraws queued messages with no writer lease and no fence. An attach failure keeps the conversation open, and the attach adopts its journal. Owed work counts the loop and queued rows. A compaction or rewind found prepared when a conversation opens was started under a child this process no longer has, so the open settles it rather than leaving it to refuse every send until a view attaches. The open cursor is scoped to its epoch, because sequences restart when an epoch is replaced. Deleted: restart-before-admission, recordFailedRestart, the fence rebase, Claude's startup gate, the attach's forget on failure and its own crash boundary. Clients without agent-session.accepted-send.v1 get their reply held until the handover; the desktop and paired desktop lists advertise it. * fix(native-chat): settle queued messages only for the child that ended A child that proved its start and then exited before its message was handed over left the message queued: the exit settlement returned early when nothing else was in flight. Delivery then started another child for it, and a child that died the same way started another, without end and without a row. A retried settlement for an earlier generation, run by the attach that delivery started, did the opposite: with that generation's turn unfinished it rejected the message queued for the child being attached. The settlement now takes the rejection for queued messages from its caller. The unexpected exit and the eviction pass one, and it applies even with no other work in flight; the retry for an earlier generation passes none. * fix(native-chat): an adoption that fails to import keeps the conversation open The attach now writes into the conversation's own open journal, but a failed transcript import still closed it as if it were the attach's provisional one. The conversation stayed indexed with a closed journal, so every later send answered "could not be recorded" and every attach failed again until the app restarted. The import now closes only a journal the attach opened for itself. * perf(native-chat): the recovering open reads the journal once Every conversation open now goes through the recovering open, including the read restore of every chat at startup, which used to replay its journal once. The recovering open replayed it twice: once to probe it and again inside the open. The probe is now handed to the open as its load. * fix(native-chat): an attach that fails after indexing its child leaves no child behind A failed attach now keeps the conversation open, but a failure after `onAttached` indexed the child (the rewind or compaction recovery, or the attach's own success record) left that entry claiming a child the failure path had already released. The next send found the phantom, skipped the start, and wrote at a fence the journal had moved past, so the message stayed queued for good. The entry now drops the released child and its event sink, and follows the record's fence, as a failure before indexing already did. * fix(native-chat): a withdrawn message shows no error, and a rejection outlasts the send's answer The error strip for a message the host accepted and then did not deliver matched the entry before the outbox reconciled, so a Stop's withdrawal, which the reconcile drops, showed "Orca could not send your message" with nothing to retry. It now reads the reconciled entry. A rejection the journal records before the send's own pending answer lands is final as well: that answer no longer puts the entry back to dispatching with no Retry. * fix(orchestration): a structured worker whose agent outlasts the preamble wait is left unknown, not torn down The preamble waits for its submission to be delivered while the worker's agent starts. When that wait ran out it threw operation_unknown, and the failed-start teardown then closed the session, which rejected the very preamble the host was about to deliver. It now reports a turn start nobody observed yet: the worker is start-unknown with its session kept, the host delivers the preamble when the agent starts, and the worker's report settles the dispatch as for any unobserved start. The receipt no longer suggests reading a screen a structured worker lacks. * fix(native-chat): a message rejected while its chat was closed reads as not sent A remount reads an entry it left dispatching as unconfirmed. When the journal had rejected it meanwhile, as a failed start or a quit now does, the reconcile left it unconfirmed: it blocked every later message behind a Retry and no reason, and the delivery probe, seeing the journal already answered, never ran. The reconcile now settles it as rejected like a dispatching one. * test(orchestration): name why the readiness settlement fakes are cast * fix(native-chat): keep each pane's own fence on frames so a failed restart is not resent * docs(native-chat): drop the fence from the admission the send effects run behind * docs(native-chat): give the fence move on release the reason that still holds * docs(native-chat): stop citing a write fence check in launch and mailbox comments Three places still gave the removed fence check as a reason: the launch replay said admission puts the ledger ahead of the fence, the launch surface said a send must name the lease it was admitted against, and the direct-mailbox path said the lease fence decides whether delivery is safe. Admission now checks only the writer lease. * refactor(native-chat): the provider child is its own record A conversation now outlives any number of provider children, so the child is one record on the conversation's entry instead of five loose fields beside its journal. It is written in one place: indexed only once an attach has fully succeeded, and ended through one function that an exit, a failed re-attach, a Stop and an eviction all share, matched on the child's generation and fence. - A failed attach writes no child, so there is nothing to unwind: the field unwind and the fence patch after it are gone. - Conversation writes read the record's fence, the way mutation admission already does; a child's own writes use its fence. The four stored-fence patches, and the settlement retry's overwrite of the conversation's fence, are gone. - The owed wind-down is its own tombstone, carrying the child it is owed for, and is no longer dropped when an attach replaced the whole entry. - Stop on a child still proving its start stops only the child: its lease goes back and the chat is told it is idle, but the journal, the holders and the readers stay. Close is that stop plus the conversation's close. - The settlement retry uses the conversation's own journal, opened through the host's one open. * fix(native-chat): the delivery loop alone settles a message its start or child failed A queued message was settled by whichever path happened to end the child first: the loop, the unexpected exit, eviction's work settlement, the open's leftover rule, and the startup branch that rejected every pending row. That gave two failure rows with different tones for one start, a loop that could hand over to a different child than the one it waited on, and a Claude start that died while starting reading unlike every other failed start. - The loop remembers the child it waited on. At handover, if that child is gone or replaced, it reads how it ended: a Stop continues; anything else writes one failure row and rejects every queued message with the same words, then stops. A child still starting whose start the adapter says did not land fails the same way. The exit, eviction and the settlement retry only settle the handed-over and legacy rows of the child that ended. - One failure row, always an error, keyed by the start. A start a view began that dies with nothing queued writes the same row through the same builder, so a second report revises it. - The open no longer rejects leftovers; the loop's first step does, and the open wakes it. - `awaitStarted` answers why a start did not land, so the row says it even when the loop sees the failure before the exit is processed. - Quit closes every conversation the way closing a chat does: what is still queued is rejected as closed, with or without a child, and a start the loop already has in flight is waited for so the child it produces is stopped rather than left behind. * refactor(native-chat): a stopped child ends on the one reading of its stop The eviction step reads a stop's result through `stopAgentSessionProviderRoot` and hands that verdict to the child's ending, so the host never forms a second view of whether the root is gone. Every ending carries it: a stop's comes from that reading, an exit's root is gone by definition, and a failed re-attach passes what its release saw. The end-of-child record can therefore also carry a stop whose root was not seen to go, which nothing ends on yet. * feat(native-chat): the host says it accepts a send before any agent has it The host now lists agent-session.accepted-send.v1 among its own runtime capabilities, the same string capable clients already send. A client can then tell a host that answers a send at acceptance, and admits a Stop with no writer before a turn starts, from an older one that still restarts the agent inside the send. Additive: an older client ignores a capability it does not know. * refactor(native-chat): an attach never opens a journal of its own The attach adopts the conversation's open journal, which outlives it, so it no longer opens one for a direct caller either. That leaves nothing for a failed adopted import to close, and the flag that told the two cases apart is gone. Tests that attach without a host open the conversation the way a host does. * fix(native-chat): a moved fence resends nothing on a host that accepts first The outbox treated any fence change as a new owner: it dropped the answer of a send in flight, queued that send to go out again under the same id, and unblocked a refused head. On an older host that is how a send the restart refused, unrecorded, gets another try. On a host that records every send before it starts an agent, a fence moves because that start ran, so the same rule resent into every failed start. With a fence stamped on every frame, that became a loop. The outbox now reacts to a fence change only when the host has not advertised that it accepts a send before any agent has it. On such a host, only a Retry or a new send goes out, and a failed start reaches the client as a rejected message it keeps with its Retry. Against an older host, or before one has answered, the outbox behaves as it did. Desktop and paired web share this hook. * refactor(native-chat): a child's end says whether the user or the host stopped it The end-of-child record's cause now tells a user's Stop from the host stopping the child for a cause of its own: `user-stop` and `host-stop` replace `stop`. The delivery loop goes on after a user's Stop, as before, and fails the start it was waiting on after a host stop, with the one error row and every queued message rejected, in the stop's reason when it gave one. The reason stays description only. Stop passes `user-stop`; nothing passes `host-stop` yet. * fix(native-chat): a chat whose only work is a queued message is not offered for resume A message accepted while the agent was starting counts as working in the chat, and quit rejects it as never sent. The teardown snapshot read the same working rule, so a relaunch offered to resume a chat whose agent never had the message. The snapshot now reads only what was handed over. * fix(native-chat): the conversation outlives its agent Opening a chat no longer starts its agent. A conversation is reached through one host accessor that opens its journal at rest, and a send is what starts the agent, through the delivery loop. One idle sweep, every five minutes, stops an agent that has been quiet for thirty minutes and owes no work, then drops an open journal handle that is only a cache. Its record, tab, status row and readers stay. - hold and release are no-ops; hold still builds the host for shipped mobile builds. - The holders, the holds, the release clock and the exit respawn are deleted. - Options, the model list, the goal and the context meter answer at rest; a model pick at rest is recorded as intent for the next start. - Compact, rewind, clear and goal changes start the agent first. A send does too when a rewind is still in doubt after the conversation opens. - Orchestration routes mail and group addresses on ownership (the record plus the chat tab), not on whether the process runs. An open dispatch keeps its worker running. - The restart continuation is a send; Resume all holds each slot until the message is handed over or rejected. - A read error never replaces a loaded transcript, and shows the host's own words. * test(native-chat): type the queued-message fixtures in the resume-offer tests * fix(native-chat): a start that dies while a message waits on it is that message's failed start Opening a chat's tab starts an agent for the view, and a send accepted meanwhile waits on it. When that start died, its exit wrote the start's error row and left the message queued, so the delivery loop started a second agent into the same failure and wrote a second row. A child's end now records where the conversation's journal stood, and the loop settles a message accepted before a failed start ended with that start: one row, under its key, and no second start. A message sent after the failure still gets a fresh start. * docs(native-chat): say what an attach's open conversation and unconfirmed ids are now * test(native-chat): pin what a failed start settles, and what a resume offer names A view's child that dies while a sent message waits settles that message only when it died starting and no child has taken its place: a proven child's crash, or a second start since, gets the message delivered. The resume offer names the handed-over message, never a newer one still queued. * test(native-chat): the failed-start pins fail on what the message became, not on a timeout * fix(native-chat): a restart offer ends when the chat's agent starts again The offer used to end only when the chat's newest user message changed, because opening a chat started its agent and that start could not be told apart from real activity. Opening a chat starts nothing now, so the host reads the fact it already publishes: a chat's status row goes from not host-owned to host-owned exactly when its agent is started. At that edge the offer and any failure record for the chat are withdrawn, unless the start is a resume action's own (its continuation is the oldest undelivered message). A continuation and a message racing to be first are decided at acceptance: the continuation is refused, quietly and with nothing filed, when any other message was accepted since the restart. A failed continuation start leaves the offer retryable, and each resume action sends its own message id. Deleted: the newest-user-message comparison, its journal reader, the continuation filter, and the failure ledger's own "answered by the chat" check. The marker still carries its message id for one release, so the previous build can read it. * fix(runtime): end a transcript stream when its client unsubscribes Desktop: the IPC subscription controller was dropped as soon as the streaming handler returned, which for most streams is right after it binds. A later runtime:unsubscribe then found nothing to abort, so the host kept the subscriber and derived and sent every publish to a channel no one listened to. The controller now lives until the renderer unsubscribes, resubscribes the same id, or goes away. Mobile: disposing an agentSession.subscribe stream now sends agentSession.unsubscribe with the stream's frame id, so the host ends that subscriber and leaves a sibling stream on the same socket running. The direct path now passes the frame id the relay path already passed. * test(orchestration): the preamble's host stub is typed, not cast The preamble send now takes only what it reads of the host, the send, the settlement wait and the record's fence, so its test builds that host with real types instead of `as never`. * fix(native-chat): one fact ends a restart offer: the chat moved on since the restart The offer is live while no other message has been accepted in the chat since the restart and its agent has not proved a start since. The offer list, the resume's reservation check and the continuation's acceptance check all read that one fact, so a message whose start then failed withdraws the offer too, and a stale click finds nothing to act on. The fact is read off the conversation's open handle, which the restart closed, so it is retired durably whenever it may have changed: a message accepted, a start proven. A close and reopen within the same run therefore cannot bring the offer back. A continuation rejected before it reached the agent does not count, so a retry after a failed start still runs. Deleted: the quit-time gate on withdrawal, which changed nothing because the withdrawal and the quit's own offer write share one queue; the per-action "withdrawn" flag and the separate acceptance check it paired with. * test(native-chat): an older build reads the restart offer this build records The offer lives in a file the previous release reads after a downgrade. Pin that against the pinned release's own capsule, and run the lane when the marker or the capsule changes. * fix(native-chat): read a restart offer against where the journal stood when it was taken "Since the restart" was read off the conversation's open handle, which the idle sweep closes: after a reopen, a message the user had already sent looked older than the handle and the withdrawn offer came back. The offer now records the journal position (epoch and sequence) at the moment it is taken, and a message accepted after that position, or a journal on another epoch, means the chat moved on. That is derived from the journal, so it holds across any number of closes and reopens. An older build's offer has no position; only a start withdraws it. Because the message half is now durable, the offer is no longer rewritten in the recovery file on every accepted message; a proven start still writes it, since only the host that saw the start knows of it. * test(native-chat): wait for the listing's retire write before reading the recovery file * fix(native-chat): keep the terminal-backed chat's read error over its local echoes Messages winning over a read error is right for the structured chat, whose read retries and whose messages came from the transcript. The terminal-backed view assembles its list from local echoes too (a launch prompt, a pending send), so a failed read there showed only those bubbles and no error. Only the structured pane now keeps messages over an error. * fix(native-chat): a start retries the exit settlement a failed journal write left owed An agent exit whose journal settlement write failed releases the lease latched until a retry lands. Reopening the chat used to be that retry; with reveal now only opening the journal, nothing retried it before the next app launch, and every send was refused. The start the send needs now runs the retry first, where the attach would. * perf(native-chat): answer the owner check without opening the chat Worktree activation calls agentSession.handoffStatus for every chat tab in the worktree, and the answer comes from the session record alone. Reaching it through the accessor opened each resting chat's journal (a full read, the crash-boundary write and a restored status publish), then kept it open for the idle window. It now checks the record and the adapter's support, as before this series, and opens nothing. * fix(native-chat): a read waiting on the session lock opens nothing once quit began The accessor checked for quit before queueing the open, so a read queued behind a session task ran its open after teardown had begun and indexed a journal no teardown step would close. The check now runs at the open itself. * fix(native-chat): read a failed resume's chat before calling it retryable Whether a failed resume is retryable is the offer's own rule: the chat has not moved on since the restart, read from its journal. The failure list read it only for a chat already open, so once the idle sweep closed a chat the user had moved on in, its failure showed Retry again, and the click did nothing. The list now opens the failed chats first, as the offer list does. * test(native-chat): type the provider event sink the settlement test reaches for * fix(native-chat): say the structured read keeps trying only where it does The structured pane's "Orca keeps trying to load it" line never showed: the view state filled in an untranslated fallback whenever the read error had no text, and the empty state prefers any message. The view state now leaves the message out, so the structured pane shows that line and the terminal-backed pane its own translated one. Mobile's structured lane does not resubscribe after an error frame, so it no longer makes the claim. * test(native-chat): await the send's settlement instead of polling for the start The at-rest send tests polled for the provider start with vi.waitFor's one-second default, which a loaded machine outran. They now await the host's own settlement of the message. * fix(native-chat): a restart offer resumes any time after the quit, and knows its own continuations The continuation's message id was dated by the quit, and the ledger refuses a new id dated more than a day back, so Resume or Retry a day after quitting was always refused (on main too). It is now dated by the resume action. Telling a rejected continuation from the user's own message read the operation ledger, whose rows expire after about a day; after that a failed resume stopped being retryable. The offer now records the continuation each action sends on its own capsule entry, bounded to the newest 16, so the ids end with the offer. The ledger read is deleted. * fix(orchestration): route no mail to a structured worker its orchestration released A structured worker is routed on ownership, and a resting worker's lease is released, so ownership held while its chat tab stayed listed. A worker the coordinator abandoned and then released, found at rest by the release, therefore still took peer mail and @worktree: broadcasts, and each one restarted its agent. Routing now also reads the orchestration's own resource row: once it is released, direct mail, group addressing and worker-show's addressable answer drop the worker, as they would a terminal worker whose terminal closed. The chat tab stays, and nothing new is stored. * fix(native-chat): a failed retry names the user's prompt, not Orca's continuation A resume's continuation is written to the chat before its start, so after a failed attempt the chat's newest user message is that rejected continuation. A second failure then showed Orca's own restart text as the chat's prompt. A retry now keeps the prompt its first failure named. * fix(orchestration): read the released row optionally, as the authority does worker-show's observation called the row lookup directly, which a runtime double without it threw on and failed the structured tab-retirement release. * fix(native-chat): the status bar drops a restart offer the chat moved on from The renderer re-read the host's restart offer only when a failed chat showed activity, so after a message withdrew a pending offer the host answered no chats while the status bar kept counting one, and clicking it opened nothing. The same watch now covers pending offers: a status change in an offered chat asks the host again, once. * test(native-chat): a roster of idle or finished children does not keep an agent awake The sweep reads owed background work through the shared child-work liveness that upstream's release clock adopted; a child that went idle or finished is not work the agent still owes. * fix(orchestration): a task dispatched into a resting structured worker keeps it running The sweep's open-dispatch check read only the worker-start dispatch that owns the worker's terminal resource, so a task later dispatched to the same worker (orchestration dispatch --to, which writes a dispatch with no worker row) did not count: after thirty quiet minutes the worker was stopped while that task was open, and its coordinator read exited. Any unsettled dispatch addressed to the worker's process incarnation now counts, derived from the existing rows. * docs(native-chat): comments stop describing the hold this PR removed Eight comments still justified orderings and teardown choices by a viewer or dispatch hold that pinned the provider child. Nothing holds any more; the orderings stand for the binding's redrive subscription and parked mail, and a chat's agent runs from a send until the idle sweep rests it. Comment-only. * fix(native-chat): a restart offer keeps the start its own continuation made Whose start ended an offer was decided at read time, from whether the offer's continuation was still the queued message. Once the provider refused that continuation, the child it had started read as someone else's start, so the offer ended and its failure showed no Retry. The delivery loop now records which queued message a start is for on the in-memory child, and the child's end carries it; the offer counts a start as its own when that message is one of its continuations. * fix(native-chat): an agent gets a full idle window after its owed work ends The sweep measured quiet only from the last journal row, so once a subagent, command, monitor or dispatch that had outlived the window ended, the agent was stopped at the next tick. A child can read done before the lead's wake-up turn writes anything, and stopping in that gap loses the wake-up. The sweep now counts owed work it observes as activity, which gives the agent the full window afterwards, as the release clock it replaced did. * test(claude): the options-read fixture runs a live child The fixture marked its conversation running with a hasProviderChild field the session type does not have, so the read took the at-rest path and refused a session with no record. It now carries a child, which is what the read checks. * test(native-chat): host tests reach its collaborators through a typed seam The rest-test rig and three test files read the host's private members with Reflect.get and cast the result. The host now exposes one test-only accessor, collaboratorsForTests(), and the subscribers class a subscriberCountForTests() beside its existing retainedActivityCountForTests(), so the tests are checked against the real types and the casts are gone. * refactor(orchestration): one owner answers a structured worker's custody Routing, group addressing, worker-show and the idle sweep each composed their own reading of whether orchestration still holds a structured worker, so each new obligation or retirement state had to be added to every reader. structured-worker-custody now derives both answers from the worker-terminal list state coordinators see in worker-list: addressable is owned and not released, and owed work is an active custody or an unsettled task dispatched to the same incarnation. The owner's state is read through the remote dispatch attachment too, as the terminal transfer lookup already does. Behaviour is unchanged; a settled worker awaiting its coordinator still rests. * refactor(orchestration): owed work is an open dispatch on the worker's incarnation A supervised worker's own dispatch context stays open exactly while the worker is active, so the separate active-custody branch only repeated it. Owed work is now one fact, which also states the policy that a worker awaiting its coordinator's decision may rest, and both custody decisions are written once at the top of the module. * fix(native-chat): a restart offer knows its continuations by a tag in their id The offer recorded each continuation id in a list on its capsule entry, capped at 16, and a running action's id in memory. Both could disagree with the journal: past the cap an old rejected continuation read as the chat moving on, and a crash during a retry restored the failure's older entry, which lacked the retry's id. Each continuation id now carries a tag derived from the offer (its teardown and chat), then the action's own part, so any continuation of this offer, queued or rejected, is recognised from the journal row and the marker alone. The persisted list, its cap and the in-memory action map are deleted; the agent-start withdrawal keeps an offer whose own continuation the start was for, read against the stored marker. * test(runtime): the legacy-worker reveal test judges its stale snapshot inside the wait The tui-idle probe reads through readTerminal, which now awaits the structured worker check before the PTY read, so the probe's snapshot request starts a microtask later. vi.waitFor missed it on its first check and polled again at 50 ms, the same moment the wait's own 50 ms timeout fired. The stale snapshot then resolved after the wait had already timed out, so the test passed without judging it, and the rejection landed before any handler was attached. Vitest reported that as an unhandled error and failed the shard. Polling every 1 ms sees the request within a few ms, so the snapshot is judged while the wait is still pending. * fix(native-chat): a view never restarts a chat whose last start failed A Claude chat whose CLI exits during startup left one red row per start, and every time a view bound to it (the chat opening right after its create died, or the user switching back to it) the hold started the CLI again, so the same launch-failure row repeated. Only a send retries a failed start now, the same rule provider-exit recovery already applied; the rule lives in one predicate the hold, exit recovery and the delivery loop share. * fix(native-chat): the idle sweep reads owed work every tick Owed work counted as activity, but the sweep read it only once the idle window had elapsed, so it refreshed the clock at most once a window. Work that ended just before the next read left the agent to be stopped at that read, moments after the work ended, which is the gap the refresh was meant to cover. The sweep now reads owed work on every tick for a started agent, so the window always runs from the last tick that saw work owed. * fix(native-chat): a continuation handed to the agent stays sent The offer read its own continuation as not reaching the agent while its dispatch was pending, which also covered one already handed over and still unanswered. When the wait for that answer ended first, the failure it filed read as retryable, and a retry sent a second continuation to an agent that may have acted on the first. Only a continuation still queued, or rejected, is now read as unsent. * test(native-chat): start the child the loop waits on with an attach, not a second view A view no longer starts a child whose last start failed, so the R2 case that waits on a child started since the failure now gets that child from a client attach, the one non-send starter left. * fix(native-chat): settle a gone generation's turn wherever a conversation opens A send that opens a chat this process had not read yet (after a crash, from a phone or the CLI) went through the delivery open, which never settled what the dead generation left running; only the read restore and a successful acquire did. When the send's start then failed, the turn stayed running for every reader. The settlement now runs in the one journal open, at the crash boundary, for every opener except an acquisition, which settles from the evidence it read before its reserve; the read restore's separate step is gone. * test(native-chat): prove the next child's start settles the turn an earlier child left The R1 case lost its only settlement assertion when the latch it checked was deleted. It now seeds the running turn the earlier child left and asserts it ends at the exit's receipt, with the exit's row, before the message is handed to the new child. * test(native-chat): count a failed start's rows by row, not by text Comparing the set of texts passed when two different rows carried the same words, which is the duplicate the test exists to catch. * test(native-chat): give the failed-start and stale-turn waits a loaded runner's budget * test(native-chat): the interrupted create's own retry continues again The merge of main's lease-latch fix replaced that test's retry of the interrupted create, under its own operation id, with a fresh start whose result nothing read. That fresh start passes with the released-reservation continuation deleted, so the case the fix exists for went untested. The retry and its assertion are main's again. * docs(native-chat): three comments that still had views starting agents A start with nothing queued now comes from a command, goal change or rewind; an interrupted compaction left alone would refuse every send, so no agent would ever start to finish it; and a current host raises the unattached read refusal only once quit began, with the attach window belonging to an older host. * test(native-chat): pin the open's and the send's start and row counts, however the view binds Opening a fresh chat whose starts fail makes one start and one row, with two views bound before or after the create's child died; one send makes one more of each. * test(native-chat): a reader's open settles the turn a failed exit settlement left running An exit whose settlement write failed leaves its turn running in the open journal. PR 1's open now settles it, and this pins the two reads that reach it here: a reader reopening a chat the idle sweep closed, and a read that opens the chat before the restart restore reaches it. * test(native-chat): the view-start test's starting window outlasts two subscriptions on a loaded runner A subscription reads the conversation before it returns, so under load the two views took longer than the create child's 300 ms start, which then exited before the test checked that it had not. The child now takes a second to fail. * fix(native-chat): settle a gone generation's turn at every open but an acquisition's The journal open skipped the settlement whenever the lease read reserved or live, to leave an acquisition's own open to the acquisition. But a lease a crashed process left in recovery also reads live, until the next acquire resolves it. A send that opened such a chat, from a phone or the CLI after a crash on a host that could not prove the old owner gone, skipped the settlement; when its start then failed, the dead turn stayed running for every reader. The acquisition now says it is the opener, and every other open settles, whatever the lease still claims. * test(native-chat): hold the create's start open until the views bind The "view binds while the create is still starting" case gave the create a 300 ms head start and asserted the views bound before it died. On a loaded runner the holds took longer, the create's exit landed first, and the case failed its own precondition. The create's initialize now waits on a gate the test releases once the views are bound. * test(native-chat): a read that reaches a crashed chat before the startup reconcile settles its turn On desktop the chat on screen at relaunch reads before startup reconciles the leases, while the dead process's lease still reads live. The open settles the turn it left running anyway, and the restore that follows finds it settled. * refactor(native-chat): drop the composer's second error formatter After the merge with main, every chat write in the composer path reports its failure as a typed outcome worded by the refusal-notice table, so the send's catch sees only a local throw. The {code, message} formatter this branch added for it has no payload left to format, and its claim to be the one way a chat words a failure is no longer true. The composer send is main's again. * test(native-chat): pin the reason on a message rejected while its chat was closed The reopen test checked only that the message reads as not sent; it now also checks the Retry row carries the host's reason. * docs(native-chat): drop the removed dispatch hold from six comments A worker's session no longer takes a dispatch hold, and no release clock rests a chat by visibility; the agent-launch comments, the abandon test, the teardown test and the refusal census still said so. * test(native-chat): rest the owner-status chat through the idle sweep, not a hold The activation-gate test from #22808 put its chat at rest by holding and releasing it, and passed the release-clock grace. This branch deleted both, so the case threw before it reached its assertions. It now moves the host's clock past the idle window and lets the sweep stop the agent and close the conversation, then asserts the same owner answer and activation gate. * fix(native-chat): show the structured pane's retrying line when a read fails The read transport always hands the pane the host's words, so the error state's "Orca keeps trying to load it" line, which showed only when there were none, was never seen: the pane showed the host's text twice, as its subtitle and on the status line under it. The structured pane now always says its read keeps retrying, and the host's text stays on the status line. The terminal-backed chat is unchanged. * test(native-chat): wait for a send's background start before the refusal oracle removes its store An accepted send wakes the delivery loop, which starts the agent in the background. The oracle's teardown disposed the loop but did not wait for that start, so its lease write could create a temp file in the store directory while the directory was being removed, failing the test with ENOTEMPTY about one run in four. The teardown now drains tracked starts before it closes the journals. * fix(native-chat): a start a message waited on gets one failure row, the delivery loop's When a queued message's start failed, two writers could report it under the same row: the delivery loop, when the adapter settled the start without proving it, and the exit settlement, when the child's exit landed. The last one won, so the chat's row could name a different cause than the one the message was rejected with, or be written twice. The exit settlement now writes the start's row only when no message is queued and the loop has not already recorded that start. A start for a command, goal change or rewind, with nothing queued, still gets its row from the exit. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-09-27 23:46:14 -07:00
Brennan BensonandClaude	85067494a1	fix(native-chat): a request that failed reads as failed (#22944) ... * refactor(native-chat): remove the unused terminal handoff No client ever called agentSession.requestHandoff or mounted the handoff chrome. Delete the handoff coordinator, the terminal-owner runtime, the proof write path and the unmounted UI. Keep agentSession.handoffStatus, which released desktop clients read for worktree activation, and let records an older build left mid handoff reconcile through the ordinary restart and recovery paths. * fix(native-chat): never let the pre-stop snapshot hold a chat's stop Eviction now drains delivered events before quit's resume-offer snapshot. An unbounded wait there sits ahead of the provider stop, so a sink whose journal write stalls kept the child running until the step deadline aborted the eviction. The offer is advisory: bound the drain and stop the child regardless. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(native-chat): drop helpers only the terminal handoff called `claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and `queryWindowsProcessRowsFresh` lost their last caller with the handoff. The fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`, the teardown path that still depends on that contract. Co-Authored-By: Claude <noreply@anthropic.com> * docs(native-chat): stop citing the removed handoff in lifecycle comments Six comments still named the handoff coordinator, a handoff suspend, or a terminal-owned session as live participants in the flows they describe. Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): type the stalled snapshot drain without a cast Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): pin that a start dead before proving owes no settlement The removed restart handoff test pinned this branch; nothing else did. Co-Authored-By: Claude <noreply@anthropic.com> * fix(native-chat): keep the owner-status read behind an in-flight attach The handoff removal dropped the per-session queue from `handoffStatus`, so a read landing mid-start reported the reservation (no owner) instead of the settled chat owner, and shipped desktop clients blocked worktree activation on it. The read is queued again, as it was before the removal. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(terminal): remove the agent-session PTY write gate The gate only refused a write when a PTY had been bound to a chat session, and the only code that ever bound one was the terminal handoff this branch removes. With it gone, every admit/readmit returned "admitted" unconditionally, so the checks on the renderer write path, the runtime controller backstop, terminal.send, agent prompts, preview input and orchestration pointers, the refusal fields on terminal.send and worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane orchestration routing could no longer run. Ordinary writes take the same path in the same order as before. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(native-chat): drop the transcript helpers only the handoff called appendLegacyTranscriptMessages fed the terminal transcript catch-up and proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost their last caller with the handoff. Their tests now go through the live entry points instead: the roster bounds through the legacy import, the pinned-read and growth tests through the ancestry replay the history window uses, and the marker rules through the string proof in their own file rather than the session-file resolver's. Co-Authored-By: Claude <noreply@anthropic.com> * fix(native-chat): stop calling a starting chat "mid-handoff" A send refused because the chat's owner is not settled showed "The session is mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that reach it are a chat that is still starting, or one whose previous agent process has not yet been confirmed stopped. The message now says which of the two it is. The refusal code is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> * test(native-chat): type the stand-in roster decoder without a cast Co-Authored-By: Claude <noreply@anthropic.com> * refactor(codex): name the pinned rollout lookup for what it does With the terminal handoff gone, the module named codex-tui-rollout-proof holds only the pinned rollout lookup that structured Codex launches use to resume a thread, so the name described code that no longer exists. Rename the module and its options type. Also drop a mobile allowlist assertion that pinned the removed agentSession.requestHandoff method, which no longer exists to allow. * refactor(native-chat): type the owner-status reply as the host sends it The handoffStatus reply type still listed the terminal handoff's fields and states (terminal placement, host label, proof retry, queued and waiting phases, the to-terminal direction). No host writes them any more and the only client reader parses the reply as unknown, so they described nothing. The reply on the wire is unchanged. * refactor(native-chat): normalize terminal-handoff lease values once at decode Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types still admitted them, so readers across the host kept branches for values no path produces and the compiler could not point at them. The store now validates the on-disk shape, which still accepts those values so an older record is not quarantined, and maps them once while parsing: - `preparing` and `old-owner-stopped` become `recovering` - a `tui` lease becomes `native`; when it records a process it also becomes `conflicted`, the claim every build probes but never stops. A plain native owner would be stopped by restart recovery, here and in older builds. Revisions are taken over the normalized state on both sides of every compare, and the mapped record reaches disk with the store's first transaction, the same way the tab-id backfill does. The in-memory types narrow to what this build writes, and the branches that existed only for the removed values go. Structured-worker identity keeps its verdict for a former terminal owner by refusing a conflicted claim rather than a non-native kind. * refactor(native-chat): stop threading the owner kind through a reservation A reservation only ever names a native owner now, so the request no longer carries a kind and the reserved lease records `native` directly. The attach params keep `runtimeKind`: agentSession.ensure and create accept it, and the operation fingerprint stored in the ledger covers it. * test(native-chat): pin the legacy-lease rewrite with a transaction that changes nothing else Hiding a tab also committed the visibility index, so the no-op transaction wrote the file even when its open-time revision was wrong. Committing the index first leaves the pending rewrite as the only reason to write. * fix(native-chat): name a chat write by its target, not the owner generation A write carried the fence of the last frame the pane read, and the host refused it unless that fence was still current. An idle release and the restart after it each move the fence, and the release publishes nothing, so a send after a release was refused "Expected runtime fence 1; the session is at 3", and a Stop queued behind a cold start was refused as stale. Every write already names what it acts on: a send its conversation, a cancel its turn, a prompt answer its item revision, a rewind its epoch; an option is last-writer-wins. So admission stops comparing the client's fence, and the rebase that papered over one restart (admitAtResumedFence, resumedFromFence) goes with it. The writer-lease check stays, and so does the attach's compare-and-swap. Frames now stamp the fence read when each frame is sent instead of a copy each subscriber kept, which went stale on the same release. * fix(native-chat): every journal append reaches the chats that are open A journal write and its delivery to open readers were two calls, and some writers made only the first. A failed start whose lease could not be handed back, a provider revision with no frame behind it, and eviction's settlement were all journaled without reaching an open chat. A journal handle now reports every durable change, and the host's session map binds that report to the session's readers when the handle is set. Writers no longer publish what they append; the per-writer publish calls are deleted. * test(native-chat): an epoch replacement reaches the open chat * test(native-chat): each row reaches an open chat once, and a live handle enters only through the map * test(native-chat): give the legacy-lease store test a tab id so the backfill cannot supply its rewrite The seeded record had no surface tab id, so the next open backfilled one and that rewrite alone made the no-op transaction write. The test passed with the legacy-lease rewrite signal removed. * test(worktree-activation): restore the OMP surfaced-agent resume test The handoff removal deleted it alongside the terminal-owner tests, but it covers the surfaced-PTY block that still guards resume, including an agent whose ownership is unknown. * perf(native-chat): a publish behind a delivered commit reads nothing Each commit now delivers itself, so the publish a provider frame still sends afterwards found every reader caught up but still read rows and rebuilt the timeline for each one. A caught-up reader now skips the read. * test(native-chat): state why the teardown test's fake journal is safe to cast * docs(native-chat): say mutation admission checks only the writer lease * docs(native-chat): drop the send rebase from comments that still described it * fix(native-chat): a message is accepted, then delivered A send to a chat with no running agent restarted the agent inside the send call, before the message was recorded, so the client waited for the whole start and a failed restart refused the message. Claude held prompts sent during startup, and those could settle as "unconfirmed". A send is now accepted inside the session's serialized queue: one ledger row and one submission row marked handoverRecorded, published, answered pending. A per-session delivery loop exists while a message is queued. It starts the agent through the same serialized attach a hold uses, waits outside the queue for a Claude child to prove its start, and hands the oldest queued message over as its own serialized step, writing dispatch{pending} before the adapter call. A start it needed and did not get writes one error-tone row and rejects every queued message with the same words; a start Stop cancelled writes none. Settlement follows from the rows. A queued message is provably unwritten, so a close, an eviction or an exit rejects it. A handed-over message stays in doubt. A queued row at or below the sequence a handle found when it opened was left by an earlier process and is rejected at open, with no latch. Stop withdraws queued messages with no writer lease and no fence. An attach failure keeps the conversation open, and the attach adopts its journal. Owed work counts the loop and queued rows. A compaction or rewind found prepared when a conversation opens was started under a child this process no longer has, so the open settles it rather than leaving it to refuse every send until a view attaches. The open cursor is scoped to its epoch, because sequences restart when an epoch is replaced. Deleted: restart-before-admission, recordFailedRestart, the fence rebase, Claude's startup gate, the attach's forget on failure and its own crash boundary. Clients without agent-session.accepted-send.v1 get their reply held until the handover; the desktop and paired desktop lists advertise it. * fix(native-chat): settle queued messages only for the child that ended A child that proved its start and then exited before its message was handed over left the message queued: the exit settlement returned early when nothing else was in flight. Delivery then started another child for it, and a child that died the same way started another, without end and without a row. A retried settlement for an earlier generation, run by the attach that delivery started, did the opposite: with that generation's turn unfinished it rejected the message queued for the child being attached. The settlement now takes the rejection for queued messages from its caller. The unexpected exit and the eviction pass one, and it applies even with no other work in flight; the retry for an earlier generation passes none. * fix(native-chat): an adoption that fails to import keeps the conversation open The attach now writes into the conversation's own open journal, but a failed transcript import still closed it as if it were the attach's provisional one. The conversation stayed indexed with a closed journal, so every later send answered "could not be recorded" and every attach failed again until the app restarted. The import now closes only a journal the attach opened for itself. * perf(native-chat): the recovering open reads the journal once Every conversation open now goes through the recovering open, including the read restore of every chat at startup, which used to replay its journal once. The recovering open replayed it twice: once to probe it and again inside the open. The probe is now handed to the open as its load. * fix(native-chat): an attach that fails after indexing its child leaves no child behind A failed attach now keeps the conversation open, but a failure after `onAttached` indexed the child (the rewind or compaction recovery, or the attach's own success record) left that entry claiming a child the failure path had already released. The next send found the phantom, skipped the start, and wrote at a fence the journal had moved past, so the message stayed queued for good. The entry now drops the released child and its event sink, and follows the record's fence, as a failure before indexing already did. * fix(native-chat): a withdrawn message shows no error, and a rejection outlasts the send's answer The error strip for a message the host accepted and then did not deliver matched the entry before the outbox reconciled, so a Stop's withdrawal, which the reconcile drops, showed "Orca could not send your message" with nothing to retry. It now reads the reconciled entry. A rejection the journal records before the send's own pending answer lands is final as well: that answer no longer puts the entry back to dispatching with no Retry. * fix(orchestration): a structured worker whose agent outlasts the preamble wait is left unknown, not torn down The preamble waits for its submission to be delivered while the worker's agent starts. When that wait ran out it threw operation_unknown, and the failed-start teardown then closed the session, which rejected the very preamble the host was about to deliver. It now reports a turn start nobody observed yet: the worker is start-unknown with its session kept, the host delivers the preamble when the agent starts, and the worker's report settles the dispatch as for any unobserved start. The receipt no longer suggests reading a screen a structured worker lacks. * fix(native-chat): a message rejected while its chat was closed reads as not sent A remount reads an entry it left dispatching as unconfirmed. When the journal had rejected it meanwhile, as a failed start or a quit now does, the reconcile left it unconfirmed: it blocked every later message behind a Retry and no reason, and the delivery probe, seeing the journal already answered, never ran. The reconcile now settles it as rejected like a dispatching one. * test(orchestration): name why the readiness settlement fakes are cast * fix(native-chat): keep each pane's own fence on frames so a failed restart is not resent * docs(native-chat): drop the fence from the admission the send effects run behind * docs(native-chat): give the fence move on release the reason that still holds * docs(native-chat): stop citing a write fence check in launch and mailbox comments Three places still gave the removed fence check as a reason: the launch replay said admission puts the ledger ahead of the fence, the launch surface said a send must name the lease it was admitted against, and the direct-mailbox path said the lease fence decides whether delivery is safe. Admission now checks only the writer lease. * refactor(native-chat): the provider child is its own record A conversation now outlives any number of provider children, so the child is one record on the conversation's entry instead of five loose fields beside its journal. It is written in one place: indexed only once an attach has fully succeeded, and ended through one function that an exit, a failed re-attach, a Stop and an eviction all share, matched on the child's generation and fence. - A failed attach writes no child, so there is nothing to unwind: the field unwind and the fence patch after it are gone. - Conversation writes read the record's fence, the way mutation admission already does; a child's own writes use its fence. The four stored-fence patches, and the settlement retry's overwrite of the conversation's fence, are gone. - The owed wind-down is its own tombstone, carrying the child it is owed for, and is no longer dropped when an attach replaced the whole entry. - Stop on a child still proving its start stops only the child: its lease goes back and the chat is told it is idle, but the journal, the holders and the readers stay. Close is that stop plus the conversation's close. - The settlement retry uses the conversation's own journal, opened through the host's one open. * fix(native-chat): the delivery loop alone settles a message its start or child failed A queued message was settled by whichever path happened to end the child first: the loop, the unexpected exit, eviction's work settlement, the open's leftover rule, and the startup branch that rejected every pending row. That gave two failure rows with different tones for one start, a loop that could hand over to a different child than the one it waited on, and a Claude start that died while starting reading unlike every other failed start. - The loop remembers the child it waited on. At handover, if that child is gone or replaced, it reads how it ended: a Stop continues; anything else writes one failure row and rejects every queued message with the same words, then stops. A child still starting whose start the adapter says did not land fails the same way. The exit, eviction and the settlement retry only settle the handed-over and legacy rows of the child that ended. - One failure row, always an error, keyed by the start. A start a view began that dies with nothing queued writes the same row through the same builder, so a second report revises it. - The open no longer rejects leftovers; the loop's first step does, and the open wakes it. - `awaitStarted` answers why a start did not land, so the row says it even when the loop sees the failure before the exit is processed. - Quit closes every conversation the way closing a chat does: what is still queued is rejected as closed, with or without a child, and a start the loop already has in flight is waited for so the child it produces is stopped rather than left behind. * refactor(native-chat): a stopped child ends on the one reading of its stop The eviction step reads a stop's result through `stopAgentSessionProviderRoot` and hands that verdict to the child's ending, so the host never forms a second view of whether the root is gone. Every ending carries it: a stop's comes from that reading, an exit's root is gone by definition, and a failed re-attach passes what its release saw. The end-of-child record can therefore also carry a stop whose root was not seen to go, which nothing ends on yet. * feat(native-chat): the host says it accepts a send before any agent has it The host now lists agent-session.accepted-send.v1 among its own runtime capabilities, the same string capable clients already send. A client can then tell a host that answers a send at acceptance, and admits a Stop with no writer before a turn starts, from an older one that still restarts the agent inside the send. Additive: an older client ignores a capability it does not know. * refactor(native-chat): an attach never opens a journal of its own The attach adopts the conversation's open journal, which outlives it, so it no longer opens one for a direct caller either. That leaves nothing for a failed adopted import to close, and the flag that told the two cases apart is gone. Tests that attach without a host open the conversation the way a host does. * fix(native-chat): a moved fence resends nothing on a host that accepts first The outbox treated any fence change as a new owner: it dropped the answer of a send in flight, queued that send to go out again under the same id, and unblocked a refused head. On an older host that is how a send the restart refused, unrecorded, gets another try. On a host that records every send before it starts an agent, a fence moves because that start ran, so the same rule resent into every failed start. With a fence stamped on every frame, that became a loop. The outbox now reacts to a fence change only when the host has not advertised that it accepts a send before any agent has it. On such a host, only a Retry or a new send goes out, and a failed start reaches the client as a rejected message it keeps with its Retry. Against an older host, or before one has answered, the outbox behaves as it did. Desktop and paired web share this hook. * refactor(native-chat): a child's end says whether the user or the host stopped it The end-of-child record's cause now tells a user's Stop from the host stopping the child for a cause of its own: `user-stop` and `host-stop` replace `stop`. The delivery loop goes on after a user's Stop, as before, and fails the start it was waiting on after a host stop, with the one error row and every queued message rejected, in the stop's reason when it gave one. The reason stays description only. Stop passes `user-stop`; nothing passes `host-stop` yet. * fix(native-chat): a chat whose only work is a queued message is not offered for resume A message accepted while the agent was starting counts as working in the chat, and quit rejects it as never sent. The teardown snapshot read the same working rule, so a relaunch offered to resume a chat whose agent never had the message. The snapshot now reads only what was handed over. * test(native-chat): type the queued-message fixtures in the resume-offer tests * fix(native-chat): a start that dies while a message waits on it is that message's failed start Opening a chat's tab starts an agent for the view, and a send accepted meanwhile waits on it. When that start died, its exit wrote the start's error row and left the message queued, so the delivery loop started a second agent into the same failure and wrote a second row. A child's end now records where the conversation's journal stood, and the loop settles a message accepted before a failed start ended with that start: one row, under its key, and no second start. A message sent after the failure still gets a fresh start. * fix(native-chat): a request that failed reads as failed A structured chat whose only message the agent's start refused read as a green finish, and a cancelled structured turn did too: the host published a verdict only for turn records, and structured rows carried no `interrupted`. The host projection now reads the session's latest request: its turn's outcome, or `failure` for a send the agent or its start refused. A send that was withdrawn, or left undelivered by a restart or a close, fails nobody and makes nothing listable. The ingest publishes `interrupted` as the hook lanes do, and every reader decodes the verdict through one accessor, so a failure reads Failed on the dot, the rollups, history and `worktree ps`, behaves like a cancellation in every clean-finish policy, and notifies as "failed". * docs(native-chat): say what an attach's open conversation and unconfirmed ids are now * test(native-chat): a verdict change republishes the mobile status projection * refactor(native-chat): the store's retention trigger keeps its flag compare A verdict change always moves the completion clock the same check already reads, so a second verdict compare there caught nothing new. * test(native-chat): a user message the provider journaled keeps its session listed * test(native-chat): pin what a failed start settles, and what a resume offer names A view's child that dies while a sent message waits settles that message only when it died starting and no child has taken its place: a proven child's crash, or a second start since, gets the message delivered. The resume offer names the handed-over message, never a newer one still queued. * test(native-chat): the failed-start pins fail on what the message became, not on a timeout * fix(native-chat): a late provider-session update keeps a failed recovery record failed A provider-session heartbeat that rewrites a completed recovery record kept its interrupted flag but dropped the outcome it was copied with, so a live failed checkpoint read as a clean finish until the next status write. * test(orchestration): the preamble's host stub is typed, not cast The preamble send now takes only what it reads of the host, the send, the settlement wait and the record's fence, so its test builds that host with real types instead of `as never`. * test(native-chat): the terminal-bell check asserts the renamed verdict field The bell notification test still checked for agentInterrupted, which no longer exists, so it could not catch a verdict leaking into a bell dispatch. * fix(native-chat): a failed turn ranks like a completion for attention Attention readers (completion time, Smart Sort, sticky retention, Cmd+J Recent) now demote only a turn the user stopped. A failure is news the user has not seen, so it keeps its completion time, ranks in the Done class, stays retained after its pane goes away, and a retained failure reads failed in the worktree rollup instead of done. Clean-finish policy (hibernation, pane ownership, the value moment) still treats a failure like a stop. The retention trigger compares verdicts again: success -> failure no longer moves the completion clock. * fix(native-chat): a failed main agent reads failed while its subagents still work The verdict is now read from the main agent's own state, not the folded row: a main agent that is done and failed has a verdict even while its subagents keep the row working. Without mainAgent (history, worktree ps, older hosts) the old combined-done rule stands. Display marks the verdict through agentVerdictDisplayMark: a failure outranks every combined state on the agent's dot, label, tab badge, dashboard and activity rows; a stop marks only a done row, so a successful or stopped main agent with live subagents still reads working. Subagent rows keep their own state. The worktree card, terminal tab and Cmd+J rollups share one pane fold and rank a pending question, then failed, then working, monitoring, interrupted and done. worktree ps publishes the main agent's outcome on a working row, and the mobile mirror reads it. The store's change check, the paired-client mirror's equality and its epoch now see a verdict change on a working row, which otherwise moves no state or clock and left the worktree card reading working. Clean-finish policy is unchanged: a working row is never hibernated and has no completion time. * docs(native-chat): the worktree ps outcome comment no longer claims old hosts send it The field is new: an old host sends no outcome at all, so a reader falls back to interrupted. The removed clause said old hosts send it on done rows, which never shipped. * docs(native-chat): the status-store listing rule names provider-journaled user messages * fix(native-chat): a refused send notifies failed through the completion feed The host's completion feed followed only the newest turn, so a send the agent or its start refused, which creates no turn, read Failed on its row but sent no notification. The feed now follows the session's latest request, read from the projection the status feed already makes for the commit: a turn keeps its id, a refused send is named by its journal item key. It announces only while the session is idle, as the row reports a verdict, so queued sends refused one commit at a time notify once, and a withdrawn send falls back to a request already announced. * fix(native-chat): every copy of a row carries the main agent's own status History entries, sleep records and `worktree ps` rows carried a flattened top-level `outcome`, copied under different gates and without the main agent's clock. They now carry `mainAgent` (state, outcome, stateStartedAt), the type the live row already persists and sends, and every copy site takes it with `interrupted` through one function, `agentVerdictFields`. - The accessor reads `mainAgent` then the legacy flag; the mobile mirror matches it line for line. - Sleep records admit `mainAgent` with `normalizeMainAgentStatusField`, so a malformed value drops the field, never the record. - Mobile dates a main agent that failed under live subagents by its own clock, as desktop does, and its row equality compares `mainAgent`. - The activity feed reads a history entry's own `mainAgent` instead of rebuilding one; the sync key and history equality compare it. * test(native-chat): pin the worktree ps verdict across host and phone versions Pairs the real v1.4.212 host and phone row reader with this build: an old phone reads a new host's rows by `interrupted`, a new phone reads an old host's rows (no `mainAgent`) the same way, and a new phone reads a failure under live subagents as Failed, dated by `mainAgent.stateStartedAt`. The release checkout now carries the phone's self-contained row reader, and the lane runs when the `worktree ps` row producers change. * test(mobile): name the parity table's row for its role * fix(native-chat): a request that settles while the user is asked something notifies once The completion edge waited for an idle session, and a pending prompt (including a subagent's approval) is not idle. Structured chat has no other attention producer, so a main turn that finished while a subagent waited on the user sent nothing until the prompt was answered. The edge now waits only on owed work (a running turn or an unanswered send), which the projection reports even beneath a pending prompt. A request that settles with a prompt pending announces once; the renderer words it "needs input" from the host status mirror's `attention`, and answering the prompt keeps the same request identity, so it does not announce again. The wire shape is unchanged. * fix(native-chat): the completion says when the user is being asked A request that settles while a prompt waits on the user was worded "needs input" from the renderer's status-feed mirror. Remote clients receive the status and completion streams over separate sockets, so they can arrive in either order and the wording could be wrong both ways. The host already knows at emit time, so the completion now carries an optional `awaitingUser: true` in that case and omits it otherwise. The renderer words the notification from that field alone and no longer reads the status mirror. Old clients ignore the field and word by outcome; old hosts never send it. * fix(worktree-status): a departed agent's failure yields to live work on the worktree card A retained failed agent has no expiry, so ranking it with a live failure pinned the card to Failed over other panes' live work. It now ranks below working, monitoring and permission, and above every finished outcome. * docs(agent-status): a departed agent's failure ranks below live work on the worktree card * fix(native-chat): a view never restarts a chat whose last start failed A Claude chat whose CLI exits during startup left one red row per start, and every time a view bound to it (the chat opening right after its create died, or the user switching back to it) the hold started the CLI again, so the same launch-failure row repeated. Only a send retries a failed start now, the same rule provider-exit recovery already applied; the rule lives in one predicate the hold, exit recovery and the delivery loop share. * test(native-chat): start the child the loop waits on with an attach, not a second view A view no longer starts a child whose last start failed, so the R2 case that waits on a child started since the failure now gets that child from a client attach, the one non-send starter left. * fix(native-chat): settle a gone generation's turn wherever a conversation opens A send that opens a chat this process had not read yet (after a crash, from a phone or the CLI) went through the delivery open, which never settled what the dead generation left running; only the read restore and a successful acquire did. When the send's start then failed, the turn stayed running for every reader. The settlement now runs in the one journal open, at the crash boundary, for every opener except an acquisition, which settles from the evidence it read before its reserve; the read restore's separate step is gone. * test(native-chat): prove the next child's start settles the turn an earlier child left The R1 case lost its only settlement assertion when the latch it checked was deleted. It now seeds the running turn the earlier child left and asserts it ends at the exit's receipt, with the exit's row, before the message is handed to the new child. * test(native-chat): count a failed start's rows by row, not by text Comparing the set of texts passed when two different rows carried the same words, which is the duplicate the test exists to catch. * test(cross-version): load the phone row readers without mobile's toolchain Vite transforms a file against its nearest tsconfig, and mobile/tsconfig.json extends expo/tsconfig.base.json, which the root-only cross-version lane never installs. The worktree ps verdict suite imported the current phone row reader from mobile/ directly, so CI failed with TSConfckParseError before any test ran. The harness now imports a copy of the working-tree reader placed under the checkout cache, where the root tsconfig applies, as it already does for the release checkout's copy. Both readers are still the real files. * test(cross-version): keep the checkout path-guard message and justify the copy import's cast * test(native-chat): give the failed-start and stale-turn waits a loaded runner's budget * test(native-chat): pin the open's and the send's start and row counts, however the view binds Opening a fresh chat whose starts fail makes one start and one row, with two views bound before or after the create's child died; one send makes one more of each. * fix(native-chat): settle a gone generation's turn at every open but an acquisition's The journal open skipped the settlement whenever the lease read reserved or live, to leave an acquisition's own open to the acquisition. But a lease a crashed process left in recovery also reads live, until the next acquire resolves it. A send that opened such a chat, from a phone or the CLI after a crash on a host that could not prove the old owner gone, skipped the settlement; when its start then failed, the dead turn stayed running for every reader. The acquisition now says it is the opener, and every other open settles, whatever the lease still claims. * test(native-chat): hold the create's start open until the views bind The "view binds while the create is still starting" case gave the create a 300 ms head start and asserted the views bound before it died. On a loaded runner the holds took longer, the create's exit landed first, and the case failed its own precondition. The create's initialize now waits on a gate the test releases once the views are bound. * refactor(native-chat): drop the composer's second error formatter After the merge with main, every chat write in the composer path reports its failure as a typed outcome worded by the refusal-notice table, so the send's catch sees only a local throw. The {code, message} formatter this branch added for it has no payload left to format, and its claim to be the one way a chat words a failure is no longer true. The composer send is main's again. * test(native-chat): pin the reason on a message rejected while its chat was closed The reopen test checked only that the message reads as not sent; it now also checks the Retry row carries the host's reason. * fix(native-chat): a send the provider never received after a restart has no verdict Restart reconciliation rejects a crash-stranded send that is absent from a trustworthy provider history with reason 'not_delivered'. Nobody failed that send, but the verdict allowlist did not name it, so after a crash the chat read Failed, was listed, and could notify "failed". Give the reason a shared constant (persisted value unchanged), add it to the no-verdict set, and treat it as an internal marker so the Retry row no longer shows the raw string. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-09-27 22:23:49 -07:00
OrcaWinandm4air	b5dec85a4e	ci: reuse mobile web route analysis and skip unrelated mobile tests (#23329) ... * ci: share mobile route analysis and scope mobile test runs * ci: cover mobile web runner process dependencies * test: verify mobile web selectors through the new runner --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-26 21:37:40 -07:00
OrcaWinandm4air	1882458f44	ci: scope orcad smoke and parallelize Linux packages (#23314) ... * ci: scope orcad smoke and parallelize Linux package formats * ci: validate packaging when its copy dependency changes --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-26 20:44:15 -07:00
OrcaWinandm4air	3eb1adec20	ci: reuse fixture setup and scope localization extraction (#23291) ... Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-26 18:46:26 -07:00
OrcaWinandm4air	4b6fe95943	fix(windows): preserve relocated terminals and native process scans (#22872) ... * fix(windows): ship the process-table addon to the relocated daemon host The Windows terminal daemon runs from a copy of the app under %LOCALAPPDATA%\Orca\daemon-host\<version>. That copy took node-pty but not @vscode/windows-process-tree, so the daemon's bare require of the addon found nothing and every process-table read (foreground tracking, descendant sweeps) fell back to a powershell.exe Get-CimInstance scan (#16905). - Copy the addon's runtime files (package.json, lib/, the .node binary) into the host; the ~25MB of gyp intermediates beside them are filtered out. - Treat a host missing those files as unmaterialized, so hosts built before this are rebuilt, and skip relocation if the install itself lacks them. - Log the daemon's native/CIM capability at startup and warn once when the process table falls back to CIM. Revives #19525 on current main. * test(windows): locate update-survival loss before relaunch * test(windows): preserve daemon tree before update-survival proof * test(windows): distinguish Electron exit from launcher close timeout * test(windows): verify process exit when inherited pipes delay close * test(windows): trace installer process checks in isolated survival runs * fix(windows): probe process-query capability before installer sweep * fix(windows): match installer probe and process-check profile behavior * fix(windows): use NSIS separators for the process-check include * test(windows): dismiss session-search overlay in survival harness --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-26 14:31:07 -07:00
OrcaWinandm4air	3a081abf71	fix(persistence): reclaim Windows profile locks after PID reuse (#23122) ... * fix(persistence): identify reused Windows profile-owner processes * fix(persistence): preserve absent-owner recovery without native registry * ci: build Windows registry before native profile identity checks * fix(cli): include native profile-owner dependencies in typecheck * test: register native profile owner test in Windows PR lane * test: use resilient Windows profile-owner cleanup --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-26 02:26:10 -07:00
OrcaWinandm4air	38bcdf76ac	perf(ci): reduce queue pressure without paid runners (#23053) ... * ci: measure complete unit file costs for shard balancing * perf(ci): reduce repeated PR setup and capture complete shard timings * perf(ci): seed reusable main-branch native and typecheck caches * fix(ci): stop superseded unit workflows from resisting cancellation * perf(ci): reuse bundle fixtures and share the baseline Git build * ci: record hosted gains and refresh main hook parity * ci: retain default workers after performance-budget regression * docs: record hosted mobile timing flake --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain>	2026-09-25 22:48:28 -07:00
Brennan Benson	067975bfd1	fix(native-chat): every lease latch has a way to die (#22820) ... * fix(native-chat): every lease latch has a way to die A failed exit settlement no longer leaves the lease in recovery: the release writes no stage and keeps the exit in its death evidence, and whatever the dead generation left running is settled from that evidence at the next acquire or read restore. The settlement retry flag, its disposition and every branch that read it are gone. A reservation that recorded no process is released at startup and after a failed start, the never-written conflicted status and the processless proof are deleted, recovery resolution always concludes, and Codex records its child's identity at spawn, before the handshake. * test(native-chat): a re-create needs a release proven by death evidence * test(codex): the child's pid is reported before the handshake * test(native-chat): type the crash and exit fixtures without casts * fix(native-chat): wait out a terminal owner an older build recorded, in recovery rather than manual recovery * test(native-chat): a chat mid-turn at quit reopens idle, and an older build reads an unproven release * test(native-chat): explain the baseline store cast * fix(native-chat): a terminal owner's refusal names the process instead of recursing Opening a chat whose terminal owner an older build recorded threw a stack overflow instead of the refusal that names the process to quit. * fix(native-chat): wait out a terminal owner recovery cannot verify instead of releasing it A terminal agent an older build recorded keeps its PTY across an Orca restart, so a probe that cannot answer (a start-time read that fails on a loaded host) is not evidence its transport is gone. Releasing it let a native child resume the same conversation beside the live terminal agent. Only proof of its exit now ends the claim. * ci(cross-version): run the unproven-release downgrade test The sharded unit job excludes tests/e2e/cross-version-wire, and the cross-version job runs an explicit list that did not name the new test, so it never ran in CI. A change to the record validator now also starts the job. * refactor(native-chat): map the retired manual-recovery stage to recovering at decode Nothing in this build writes manual-recovery, and restart reconciliation already rewrites it. Mapping it where the other retired handoff stages are mapped removes it from the in-memory lease type and deletes the branches that could only see it: the acquisition refusal, the renewer skip, the unproven-release stage check, and the handoff-status 'manual recovery is required' answer. Older builds accept recovering, so a record written back still loads after a downgrade. * docs(native-chat): say what happens to a live child an ownerless reservation leaves The reaper runs once at store open, while the unreconciled lease still claims the child's token, so it does not stop that child on this launch. The comment claimed it did. * test(native-chat): name the each-case label for its role * fix(native-chat): continue a create retried after recovery released its reservation The client retries a create it never heard back from under the same operation id. Recovery had released that create's reservation, so the retry was refused agent_session_ownership_unknown while its row was pending, and agent_session_operation_expired once the row aged out, and the chat never started. A retry whose lease nothing holds now continues as a fresh reservation at the next fence, which also stops the old reservation's spawn from committing. * test(native-chat): name the refusal a replayed create used to get * fix(native-chat): one quit-the-terminal-agent message for a chat a terminal agent holds A chat held by a terminal agent an older build recorded frees only when that agent exits. Sending said to reopen the chat and opening it said two runtimes claimed it; both now say the chat is open in a terminal agent, name its process, and say to quit it. Error codes are unchanged. * ci: run PR checks on the rebased head * fix(native-chat): name a terminal owner's process only when its start time can tell it from a reused pid * test(native-chat): relaunch from the dying host's durable state, so its still-pending attach cannot race the new host	2026-09-25 21:04:31 -07:00
Jinwoo Hong	a05649de91	fix(terminal): prove an idle Git Bash prompt through its bin launcher (#22752) ... * fix(terminal): prove an idle Git Bash prompt through its bin launcher Git for Windows' bin\bash.exe is a launcher that runs usr\bin\bash.exe as a child and waits, so an idle Git Bash pane's job always holds two pids and the Windows shell proof never confirmed it. Accept exactly the launcher plus its direct bash.exe child, checked against the identity process table. * test(terminal): wait for the Git Bash prompt before asserting the hand-off job * fix(terminal): prove a Git Bash prompt as one unbranched MSYS bash chain Orca launches Git Bash as bin\bash.exe -c "chcp.com ...; exec \"$BASH\" ... -i", and each MSYS exec leaves its pre-exec process alive as a stub, so an idle pane's job is launcher -> stub -> interactive bash. Accept any job that is one parent-to-child chain rooted at the launcher whose every later member is bash.exe, instead of a fixed two-process shape. * ci: register the Git Bash shell-proof win32 test in the package-test list * fix(terminal): read the spawned shell as a path, and keep one shell map A spawned shell path with a space (/Users/John Doe/bin/zsh) was split as a command line, so the POSIX proof compared against "john" and never confirmed. Local panes now keep only the spawned shell path and derive the name from it; the Git Bash chain walk drops guards the member check already covers.	2026-09-25 01:00:36 -04:00
OrcaWinandm4air	632ae1320b	fix(daemon): reap terminal descendants during shutdown (#22232) ... Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>	2026-09-22 04:32:26 -07:00
Jinwoo Hong	da1c322b00	feat(mobile): one build-time switch picks native or OTA, default native (OTA phase E1) (#22193) ... * feat(mobile): one build-time constant decides native or OTA, default native EXPO_PUBLIC_MOBILE_SHELL is read in exactly one place, mobileShellBuildKind in preferences.ts. Expo's babel preset inlines a literal process.env member expression at build time, so a release bundle carries the answer as a constant and anything but the exact string 'ota' — unset, empty, a typo — is native. Every default build is therefore the native app, unchanged. mobileWebShellFlagCanBeOn now answers __DEV__ or an OTA build, so the ability to mount the page comes from the build and never from storage: a native binary installed over an OTA one, same bundle id and same data container, still refuses a stored 'true' without reading the key. An unset key reads on only in an OTA build; a development build keeps its opt-in, and a stored 'false' wins everywhere so the Troubleshoot toggle can switch an OTA build back to native. That toggle now mounts wherever the flag can be on, which is the only way back to the native screens in an OTA build, and its label names the build kind rather than saying "(dev)". The bundle probe row beside it stays development-only: it fetches. The flag census gains two rules — one module reads the switch, in the member form Expo inlines and not the bracket form, and one named function answers the build kind — and the build-kind fence now lists the Troubleshoot route that asks it. Docblocks that said a store build can never mount the shell now say it mounts only when built for OTA. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci(mobile): one workflow input picks the shell, and no input means native Both release workflows gain a `shell` workflow_dispatch choice, options native and ota, default native, and hand it to the step that bundles the JavaScript as EXPO_PUBLIC_MOBILE_SHELL. That is the Gradle assembleRelease step on Android and the fastlane build_and_upload step on iOS; nothing else in either file sets it. A tag push and a schedule carry no inputs at all, so `inputs.shell || 'native'` yields native for them — the first OTA release is a dispatch with one field changed, and every other run is the app we ship today. Each build step prints the value it is about to build with, read back from the same variable rather than from a second copy of the expression, so a run's log cannot claim a shell the build did not use. The new contract test evaluates that expression rather than matching its text: absent, empty and 'native' all resolve to native, 'ota' to ota, and any expression shape it cannot evaluate is a failure rather than a pass. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * build: the desktop packages the real page, and the placeholder is retired build:mobile-web now runs the app builder and app verifier, and both take their output root from MOBILE_WEB_BUNDLE_DIR in the packaging guard rather than each carrying a constant of their own — one definition of where the bundle lives, so a drift cannot leave electron-builder's beforePack looking at an empty directory while the builder reports a tree it wrote elsewhere. build:mobile-web:app is gone; it was the same two commands. src/mobile-web/ and its two scripts go with it. What the app builder shared with them is split into three modules named for what they hold rather than for the bundle that used to own them: mobile-web-bundle-manifest.mjs (content types, the canonical asset serialization, buildId, hashed assets, the protocol window and the manifest write), script-entry-detection.mjs (isDirectInvocation, whose two failure modes are Windows paths and symlinked entries), and mobile-web-source-line-endings.mjs (the CRLF guard, now with a required directory rather than a default pointing at the deleted tree). The two suites that only needed *a* valid tree on disk — the beforePack guard and the packaged-bundle guard — build one from mobile-web-bundle-fixture-tree instead of bundling the whole mobile graph. It goes through the same manifest writer the page does, so a manifest shape change still reaches them. Also retired: the placeholder's tsconfig project and its typecheck lane, its knip entry, its electron-builder exclusion and .gitattributes pins, and the app-bundle test that asserted the shims stayed out of a builder that no longer exists. pr.yml's page job builds the same bundle the package job ships. Inert for native phones: they never fetch it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(config): one import of node:fs/promises in the entry-detection suite The changed-code quality gate's focused plugins read the two as a duplicate import; the readFile line was left over from the split. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs: the comments that still describe the retired placeholder bundle The web entry said it was built by `build:mobile-web:app` into out/mobile-web-app and shipped by nothing. That script, that directory and that fact are all gone: it is built by `build:mobile-web` into the packaged bundle dir, and a phone mounts it only when the binary was built with EXPO_PUBLIC_MOBILE_SHELL=ota. Two Windows cache keys explained themselves by naming src/mobile-web and "the two bundle builders"; config/** now covers the builder, the verifier and the manifest writer, and the spike's key no longer waits on a Phase C flip that has happened. The keys themselves are unchanged. Three scratch directories in the app-bundle suites and one in the verifier still spelled the retired output root. Renamed to mobile-web, which is what the build writes; they are temp subdirectory names and nothing reads them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-22 06:18:33 -04:00
Jinwoo Hong	381a3da46f	feat(build): Route A, the phone's host routes bundled for the web, dark (OTA phase C, C0.7) (#21449) ... * refactor(mobile-web): share the bundle manifest assembly with a second builder Manifest assembly and the on-disk write move to writeMobileWebBundleTree, and the helpers the Phase C app builder needs become exports. No behaviour change to the shipped bootstrap bundle. The CRLF guard grows two exemptions it needs once it is pointed at mobile/src: the image and font extensions .gitattributes already pins -text, and the gitignored webview engine modules the postinstall writes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): web entry for the host route tree, and its two transport siblings The entry mounts app/h on react-native-web through expo-router's own ExpoRoot. It lives inside mobile/ so one React resolves, and supplies RpcClientProvider itself: the route tree starts below the native root layout that owns it. route-manifest.ts is a real typed module whose body the builder replaces -- esbuild has no require.context. A virtual specifier would need an ambient declaration and would leave the entry unchecked. Two .web.* siblings, both listed with a reason in web-overrides.json: the transport substitution point (a placeholder client until C0.4 lands BridgeRpcClient) and the device token store, whose native path imports expo-secure-store, which is {} on web. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): build:mobile-web:app, the phone's host routes bundled for the web Same builder shape as the Phase A bootstrap into a separate out/mobile-web-app, with the same manifest and the same two-scratch-build determinism check. Dark: build:mobile-web, packaging and the A2 census are untouched, and C1 is what flips build:release. Six shims, each a named Metro or RN Web gap. Images are emitted as same-origin hashed assets rather than data: URLs, because the shell's CSP sets img-src 'self'; the render check under that exact header is what found it. The script is referenced root-absolute for the same reason a <base> tag cannot be used: the document is served at every route depth and base-uri is 'none'. The budget sits below the contract's per-asset ceiling so growth trips a build rather than a refused asset on a phone. esbuild splitting does not lower it: one entry with only static imports emits one chunk (measured). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): let React Native Web paint under the shell CSP RN Web 0.21.2 injects its stylesheet at runtime with no nonce support, so style-src 'self' blocks every rule and the page renders unstyled. Measured, not predicted: the render check serves the document under this exact header and reported the violation. 'unsafe-inline' is granted to style-src and nothing else. script-src 'self' holds, which is the directive that decides whether page code can arrive any way other than as a fetched same-origin script. The test now pins that scoping rather than rejecting the token everywhere. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: prove the Route A app bundle on every PR A dedicated job, for the same reason the browser provider has one: it needs mobile/node_modules and a real browser, and the sharded test matrix would pay for both on every shard. It builds the bundle, verifies it, and runs the builder, override-census and render suites. It ships nothing. The mobile_web_app signal is lifted out of should_run the way static_analysis is. A mobile-only diff is desktop-irrelevant and skips every gated job, and that is exactly the diff that changes the page this job builds. Also the C0.6 review follow-up: mobile/package.json and mobile/pnpm-lock.yaml join the installer cache keys in the two workflows that build an installer off a hashFiles key, since beforePack requires out/mobile-web and a mobile-only change must miss those caches rather than reuse a stale build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): pin the shipped builder against the app builder's own module name The assertion named a specifier that no longer exists, so it held vacuously. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the RN Web style-src grant in the Swift checks The Swift twin of the Kotlin CSP test still required style-src 'self' and no unsafe-inline anywhere, so it trapped on the approved grant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): make the Route A render check name what each route paints The check asserted only "some html, no errors", which expo-router's Unmatched screen satisfies: pointing HOST_ROUTE at /zzz/not-a-real-prefix stayed green. Each route now asserts content only its own component produces, and the unmatched case asserts the screen positively so the negatives discriminate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): read the shell CSP past the comments that quote directives Both constants document themselves with // comments containing quoted directive text, which the quoted-string scan picked up as directives. One parser now drops comment lines, and iOS and Android go through it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): honour a .web.* route sibling in the app bundle Routes were imported by absolute path with the extension, so esbuild's resolveExtensions never applied and a .web.tsx under app/ was dead code the census still accepted. The manifest now carries a key and a module: the key stays the native filename so the URL does not move, and the module is the web sibling when one exists. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): tie each named shim to the esbuild option that implements it The shim list was asserted against a literal copy of itself, which passes however the build is configured. Each entry now carries an appliesTo that reads its own option, checked against the real options object. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): line up the CRLF exemptions, the budget comment, and the job scope The builder loads .gif as a file but neither .gitattributes nor the CRLF scan exempted it, so the blanket eol=lf pin would have rewritten one. A test now keeps the two lists in step. The Phase C byte budget's comment sat on the asset count, and a root package.json edit could change build:mobile-web:app without running the job that proves it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): satisfy the index-check lint rule in the CSP parser Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: key the installer caches on the mobile page trees too beforePack builds the mobile web bundle into the installer. Today those bytes are Phase A's, which src/** already covers, but once C1 flips the entry to mobile/app a page-only change would hit a cache holding a stale installer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): skip the bundling tests where mobile dependencies are absent The sharded `test` job collects config/scripts/**/*.test.mjs and installs no mobile dependencies, so the two new suites failed there on "Could not resolve react-native-web". They now skip themselves with a message naming the job that runs them, and that job sets ORCA_MOBILE_WEB_APP_DEPS_REQUIRED so a missing install fails it instead of skipping everything it exists to prove. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): scan mobile/packages in the .web.* census The census claimed the app entry never resolves into packages/, but the dictation hook imports @orca/expo-two-way-audio and the built script carries ExpoTwoWayAudioModule.web.ts. That file is now listed with its reason, and planting a .web.* in each scanned tree proves the scan is not passing because a tree happens to be empty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): assert the route exclusions against a tree that has them mobile/app holds no test, spec or +api file, so the exclusion rule was asserted against a tree it could not fire on. A scratch tree plants one of each; dropping the rule now fails this test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): 404 unknown file paths in the render check's page server The server answered every path with the document, so pointing publicPath at /wrong-prefix still rendered three green routes: the script is fetched from the one prefix that is served. A path naming a file now has to come out of the bundle, which is what the shell's manifest map does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): cover the app bundle verifier's own checks The verifier had no test. One doctors the buildId, which the packaged assert catches; the other rewrites the tree so every digest still agrees and only the two fresh builds can tell, which is what a stale out/ looks like. Deleting either check now fails a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): tidy the app bundle comments and the job's path prefixes Drops an export nothing read, merges two comments that had drifted apart from the constant they describe, and corrects the claim that the job runs on every PR when it is path-gated. package.json leaves the prefix list because GLOBAL_FORCE_FILES already forces every job on it; mobile/packages/ joins it, since the page resolves a .web.ts out of there. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): merge the duplicate node:fs/promises import in the census Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): redirect the hybrid shell route on the web page app/h/[hostId]/web.tsx reaches OrcaMobileWebShellView, whose module calls requireNativeViewManager at import. In a browser that throws before React mounts, and the route manifest imports every route statically, so one native route left the whole page blank at every URL. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): fail the render check with the error that stopped the mount The check waited on "#root has children" with Playwright's animation-frame polling, so a route module that threw at import read as a bare 30s timeout naming nothing. It now waits on a mount attribute the entry sets after the router commits, polls on a timer, and races the wait against the first uncaught error so the failure carries it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): answer the favicon the render browser asks for CI resolves the runner's Google Chrome, which requests /favicon.ico; the bundled headless shell does not. The bundle carries no icon, so the server answers 204 rather than turning a browser habit into a console error the render assertions read as a page fault. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): settle the render check's uncaught-error race without rejecting The entry throws during goto, before anything awaits the race, so a rejected promise surfaced as an unhandled rejection beside the real failure. The same signal now resolves with the error. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): list the page transport in the raw request port inventory The placeholder client implements the port, so the boundary test counts it as an unlisted file. It belongs under OWNERS until C0.4's BridgeRpcClient replaces it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 09:50:37 -04:00
Neil	ea01cd0ccd	fix(windows): reject a node-pty addon that predates the MSYS breakaway denial (#20047) ... * docs(windows): record the measured MSYS job-breakaway mechanism The per-PTY job already denies JOB_OBJECT_LIMIT_BREAKAWAY_OK for Cygwin/MSYS shells (#19068), but nothing records why, and a conpty.node built before that commit fails windows-msys-job.win32.test.ts in a way that reads as a source defect. Measured on a real Windows 11 host: both the plain and the exec- replacement Git Bash shapes leak, the escape is the MSYS runtime's own spawn/exec (fork keeps membership), and a single-variable A/B on usesCygwinRuntime flips the result 0/2 -> 4/4. Also names the gap the failure hid behind: node-pty-job-ownership.cjs asserts symbol presence, which cannot distinguish patch revisions. * fix(windows): reject a node-pty addon that predates the MSYS breakaway denial The native-runtime gate asserted only that terminateJob, listJobProcessIds and assignCurrentProcessToJob were exported. All three predate the Cygwin/MSYS breakaway denial, so an addon built before it passes every gate, isPtyJobOwnershipAvailable() returns true, and windows-pty-job.win32.test.ts passes 6/6 -- while every Git Bash child is created outside its pane's job and survives terminatePtyJob. Read the resolved .node and require the wide msys-2.0.dll literal that usesCygwinRuntime holds, the way stagedRelayAddonIsUnpatched() already tells a patched windows-process-tree addon from a published one. An addon the caller cannot name is refused rather than skipped: a gate that cannot see its subject is not a gate. Verified against real binaries on a Windows 11 host: the shared checkout's pre-#19068 build errors, a build from current patched source passes, a missing path errors. Also closes the cross-host packaging skip. The export half has to load the addon so it cannot run when the packaging host is not the target, which is how a Windows release built elsewhere could ship this. The marker is a file read and needs neither; an unrecognised layout warns rather than fails a release that was packaging fine. * fix(windows): check the MSYS breakaway denial on the rebuild path too The Electron probe carried the marker check, but it lives inside probeElectronNativeModules, which returns early whenever the Electron package binary is unusable. Covered by another path is not this path checks -- and the defect this whole change closes was a gate that looked like it checked. Reading the binary needs neither a loadable Electron nor an executable target arch, so assert it after the rebuild, beside the windows-process-tree assertion that exists for the same reason: this is the addon copied into the packaged app. Absent warns (a cross-platform rebuild need not leave a win32 addon on this disk); present and unmarked is fatal. The fixtures now write a real addon file, because the gate reads the binary it was told about rather than trusting the exports. Verified against the two real binaries measured on the Windows host: the pre-#19068 build fails this path, the build from current patched source passes. * fix(windows): check the marker on every ConPTY path the packaged app can load The packaged marker check read one hard-coded path, `build/Release/conpty.node`, and warned when it was absent. `loadNativeModule` tries `build/Release`, then `build/Debug`, then `prebuilds/win32-<arch>`, swallowing each failure, and `prunePackagedNodePty` drops the published prebuild only when a same-arch `build/Release` exists to replace it. So the two packages the check was added for were the two it could not see: - cross-host: no host but Windows can build conpty.node, so there is no `build/Release` and the prebuild is what ships. The check warned and returned. - cross-arch: `build/Release` is the packaging host's own arch, patched and marked, so the check printed OK -- while the target app cannot load it and falls through to the unmarked prebuild underneath. Measured, not assumed: both published Windows prebuilds in the node-pty tarball contain neither `msys-2.0.dll` nor `cygwin1.dll` in any encoding. They are the binary that leaks every MSYS pane child out of its job. It now sweeps every candidate present for the *target* arch and refuses a package with no candidate at all, which is a package with no ConPTY backend rather than a layout to shrug at. It runs for every Windows slice instead of only the branch the export check skips, so deleting the export check cannot silently take it too. A stale source build keeps the rebuild advice; the prebuild gets the advice that actually works, which is to package the slice on a Windows host of that arch. Also: the marker constant was re-typed in four places and was tied to the C++ literal that produces it by nothing at all, so editing the patch would have left a gate that fails every correctly rebuilt addon and tells the developer to do the one thing that cannot help. The fixtures now take the constant from the gate, and a test asserts the patch still adds `L"msys-2.0.dll"` to conpty.cc. And the rebuild path treated a missing addon as a warning even on the host that will run the install, where node-pty would fall through to that same prebuild. The verdict is now a value, so it is tested without a platform gate. * fix(windows): resolve the packaged ConPTY the way its loader does Sweeping every candidate and demanding the marker on all of them was wrong in the one case it was meant to make safe. `beforeBuild` runs `rebuild-native-deps.mjs --platform=win32 --arch=<target>`, so a cross-arch slice normally does get a patched `build/Release` for the target; `prunePackagedNodePty` keeps the prebuild anyway because its guard is `electronArch === process.arch` rather than the arch of the binary. That package is correct and its leftover prebuild is never reached, and the sweep failed it -- telling whoever ran it to package on a Windows arm64 host, which is both the wrong remedy and one no runner here can offer. Presence cannot separate that package from the one whose cross-arch rebuild quietly emitted the host's architecture, because the only difference is the arch of `build/Release`. So the gate now resolves the addon the way `loadNativeModule` does -- first candidate whose PE `IMAGE_FILE_HEADER.Machine` matches the target, walking root-then-lib for each layout in node-pty's own order -- and checks the marker on the one that will actually run. A package with no candidate, or none of the target's architecture, is refused: it has no ConPTY backend either way, and the second is exactly what a silently host-arch cross-build looks like. The PE machine reader already existed, privately, in the relay addon builder that needed the same "a cross-build cannot silently emit host arch" guarantee. It is now shared rather than copied. Two seams were unreachable from anything but Windows, so nothing tested them: - the afterPack hook's win32 block was an inline if/else that only a source-text assertion could inspect, and that assertion could not tell the difference between the check running and the check being wrapped in `try {} catch {}`. It is now `verifyPackagedWindowsNodePty`, and "the marker check runs even where the export check cannot" is four spied assertions instead of a string match. - the rebuild path's verdict read `process` directly, so the branch that fires only on the host being rebuilt for was dead on every other host. It now takes the host as arguments, and the fs checks, the warning and the failure are all exercised from macOS. Fixtures write a real PE header rather than `MZ fake addon`, since the gate now reads one. The machine table is pinned to the documented IMAGE_FILE_MACHINE values, because every fixture builds its header from that table and a table wrong in both entries would otherwise agree with itself. * fix(windows): say why the packaged ConPTY fell back, not just that it did The previous commit resolved the addon by architecture but still had one message for every way the resolution could land on the published prebuild. Those ways want opposite remedies, and the one it printed was the remedy the commit before it had just called wrong: - no source build in the package at all — the slice has to be built somewhere that can build node-pty for the target arch. - a source build that is there but is the packaging host's architecture, because the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the fix, and "package on a Windows arm64 host" is neither necessary nor possible. The second is the common one, since node-pty publishes a prebuild for both Windows arches and prune keeps the target's on every cross-arch package. So the old text fired mostly on the case it described least. It now reports which source builds were skipped and the machine field each carried, and names the rebuild command. "Nothing the target can load" had the same problem in reverse: a zero-length or truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is now named with what was actually read, including "not a PE image". The rebuild path asserts the architecture too. A rebuild that ignored `--arch` was otherwise only visible at packaging, two steps from the command that fixes it. Arches with no known machine value are left unjudged rather than guessed at. Two things the extraction broke or nearly broke, both found by mutation: - the shared PE reader answers `null` where the relay builder's private copy returned a number, which would have turned its "node-gyp ignored --arch" error into a `TypeError`. Both callers now go through `describePeMachine`. - the rebuild fixtures stage a script's co-located modules by walking its imports, and the walker only understood `from '...'` — so the gate's new `require('./windows-pe-machine.cjs')` was left behind and every subprocess test failed with a resolution error, which is the exact failure its own comment warns about. It now follows `require` and bare side-effect `import` as well, and has tests; the fixture stages the gate by walking it rather than by naming one file. Fixtures write real PE headers through one shared builder instead of three hand-rolled ones. * fix(windows): run the node-pty addon gates on the Windows job that can `rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')` tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit file list that never named this file -- so those tests were skipped on Linux and never reached anywhere else. Three of them predate this branch. The Windows job is added the four node-pty addon suites plus the module-walker one; the comment above that list already says why it is the right place, which is that the addon assertions only hold once natives have been rebuilt. Running the path-joining suites there also covers the separator this gate's candidate list is built from. The rest is round-three review: - the rebuild-time arch assertion told a reader "node-gyp did not honour --arch" about a file that was not a PE image at all, which is a truncated or quarantined artifact and a different command to run. The two now read differently, and neither claims the other's cause. Same fix the packaged gate had one commit ago, in the place that had not had it yet. - the missing-addon error said node-pty "would load" a prebuild without checking it is there. It says "fall through to" now, which is true either way. - `isLoadableByArch` had no caller left once the packaged gate started needing the raw machine field for its message. Removed rather than kept warm. - each candidate's header is read once instead of up to three times. - the module walker's comment claimed every shape that reaches a co-located module; it does not follow `projectRequire`/`requireLocal`, and it must not -- those specifiers resolve against the project root, so following one stages the wrong path and the copy fails. Proven by trying: widening the pattern to require-shaped names broke nine tests on `projectRequire('./config/scripts/...')`. The comment now says what it follows and why it stops there. - a new test resolved a file URL with `.pathname`, which keeps the drive-letter slash on Windows -- the very job this commit adds it to. * docs(windows): put the superseded export-only gate in the past tense It describes what used to pass a broken addon, so present tense reads as a description of the gate the same document then explains replacing it. * fix(windows): repair what running the node-pty suites on Windows exposed Putting these files on the Windows job turned four assertions red on the first run. Three of them were in tests that carried `skipIf(platform !== 'win32')` and had therefore never executed anywhere, on any branch. - `writeFakeElectronRebuild` emitted the `windows-process-tree` addon a real rebuild leaves but never node-pty's, so every Windows test of the rebuild path ran against a tree no real rebuild can produce: node-pty "rebuilt" with nothing in `build/Release`. The new same-host check reads that state correctly and said so. The fake rebuild now writes `build/Release/conpty.node` when it was asked to rebuild node-pty for win32, with the marker and the target machine. - `mkTempProject` never staged `windows-process-tree-creation-time.cjs`. The rebuild script reaches it through `projectRequire`, which resolves against the project root, so the module walker cannot follow it and must not try. Staged by name, with a comment saying which of the two it is. Without it the windows-process-tree probe failed to load its own checker and the module joined `modulesToRebuild`, which is the second and third red assertion. - the two `nodePtyAddonPath` cases compared against a literal POSIX string. `resolve` returns a drive letter and backslashes on Windows, so they could only ever pass off it. Built from segments now, which still pins the `..` traversal that is the point of the test. Verified on macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime -- 109 passed, 6 skipped. The 6 are the Windows-gated rebuild tests, which is the job this change is aimed at; Windows CI is the arbiter. * fix(windows): give the packaged fallback a third verdict, for a file that is no image The packaged gate had two remedies for landing on the published prebuild and picked between them on `!prebuilt`, which puts a truncated, empty or quarantined `build/Release/conpty.node` in the cross-arch bucket: "the source build beside it is the wrong architecture ... re-run with --arch". It is not the wrong architecture, it is not an architecture, and `--arch` is not the command. The rebuild-path gate was split for exactly this a commit ago; this is the same split in the place that had not had it. Also from review of the settled state: - the stale-source-build branch ended in a call that happened to throw, so a reader could not see it was terminal and the file was read twice to get there. The verdict is now an Error the caller throws, built once from the read it already did, and shared with `assertCygwinBreakawayDenied` rather than copied. - four injection seams had no consumer in production or in tests (`deniesBreakaway`, `peMachine`, and `exists`/`peMachine` on the rebuild verdict). An unused seam is a way for the tested path and the real one to drift apart; the tests drive both with real files. Removed. - the loader table existed in a docblock and in the reference doc, already disagreeing about row four. The docblock cites the doc now. - `peImage` stamped machine `0x0000` for an arch it had no value for, because `writeUInt16LE(undefined)` coerces to zero. A fixture that quietly invents the field the gates read is the same species of silent lie the gates exist to catch; it throws, and a test holds it to that. - a test named for refusing an unreadable candidate asserted only that something threw. Renamed to what it proves. * fix(windows): make the rebuild fixtures represent a tree that can exist Second round of what running these suites on Windows exposed. The module the walker could not stage is now staged, so the probe reached its own checker and the real reasons surfaced: - `writeFakeWindowsProcessTree` exported `{}`. The creation-time gate reads `supportedProcessDataFlags` off the addon and calls its absence "the tarball prebuilt, not a build of the patched source" — correctly. The fixture predates that gate and, being Windows-only, never met it. The healthy fake now reports the flag, taken from the gate's own constant. Two tests were failing on this, the second only because the module then joined `modulesToRebuild`. - `rebuilds a loadable ConPTY native that lacks Orca job ownership` asked for a node-pty rebuild in a tree where node-pty had none of the payload its package ships. It gets `writeFakeNodePtyConptyPayload` like its two siblings. I also tried making the fake rebuild emit `build/Release/conpty.node` the way a real one does, and backed it out: `restoreNodePtyWindowsConptyRuntime` keys off that file and then reads `third_party/conpty`, so emitting it in a tree without the package payload turns one honest gap into an ENOENT two steps away. The payload fixture is where "node-pty has its addon" belongs. macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime — 112 passed, 6 skipped. The 6 are the Windows-gated rebuild tests; Windows CI is the arbiter and is why they are on that job now. * fix(windows): register the node-pty addon suites in the scope list too Putting the five suites in the Windows lane's vitest argv gets them run once the job starts; `WINDOWS_PACKAGE_TESTS` in `pr-code-change-scope.mjs` is what decides whether the job starts at all. Only the argv was updated, so a PR touching just `rebuild-native-deps-node-pty.test.mjs` would not have started the Windows job, and its four Windows-only cases — including the same-host-absent one added here — would have run on no machine for that PR. Exactly the shape of gap this branch is about. Both lists now name all five, and `windows-pe-machine`, `windows-pe-image-fixture` and `script-module-dependencies` join `NATIVE_RUNTIME_PREFIXES` so a change to the modules themselves starts it too. `win32-test-lane-registration.test.mjs` exists to catch precisely this and did not, because its matcher only recognises suite-level gates (`describe.runIf` / `describe.skipIf`) and a `.win32.` filename. These tests gate per `it`. Widening it is not this branch's change to make: about thirty files across the repo carry per-`it` Windows gates and are unregistered, so the ratchet would move far beyond node-pty. Flagged rather than done. Message repairs from the same review: - the non-PE arm of the rebuild-time arch error read "... is not a PE image, so nothing can load it, so node-pty would fall back ...". The shared consequence clause already opens with ", so". - the no-source-build packaging error ended "Package this Windows slice on such a host", which is wrong advice for the case where the host IS such a host and the rebuild simply left nothing — reachable when the artifact is removed before prune runs. It now names both readings and points at the beforeBuild output. - the relay-addon builder blamed `--arch` for a build output that is not a PE at all, the same guess the node-pty gate was taught to stop making. - the patch-drift assertion was a bare `toBe(true)`, so a real drift read as "expected false to be true". It now names the two things that can have drifted and what happens until they agree.	2026-09-16 22:23:30 -07:00
Brennan BensonandMerge Sim	97aa5ff19b	fix(mobile): open native chat when a new worktree launches a default agent (#19850) ... * refactor(agent-launch): make the launch-mode decision surface-neutral `decideWorkerStartMode` was the only shared answer to "structured chat session or terminal agent?", but it lived in an orchestration-named module and spoke orchestration's vocabulary, so the other launch surfaces could not call it. Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave `orchestration-worker-start-mode` as the adapter that supplies the noun. A worker is not a special kind of launch; it is the same launch with a dispatch attached. Naming the receipt's subject is the only thing orchestration actually contributed, so that is the only thing the adapter keeps: "worker" in both sentences, plus the `--terminal` wording, which reads as nonsense anywhere a `--terminal` flag does not exist. Both are pinned, because they are asserted. No behavior change. The receipts are byte-identical for every reachable case, proven by running the new pin against both implementations. Also pins the wording, which nothing was holding. The existing suites assert `toContain` fragments ('terminal agent', 'cannot create') and the CLI suite asserts a receipt handed to it by a mock rather than one this code produced; all six files stayed green against a deliberately corrupted vocabulary. A dispatch receipt is the only place a structured-to-terminal downgrade explains itself, so the whole sentence is the contract, not a fragment of it. * feat(agent-launch): add the launch intent and the one executor that runs it The sequencing around the launch decision was duplicated per surface, and the duplicate is where the bug lives. A new worktree was created agent-first, so its startup terminal WAS the agent and the structured branch below it could never be reached — every new-worktree launch was a PTY regardless of the user's default. Orchestration fixed that for itself in #19431; mobile and the CLI still have it. `executeAgentLaunch` inverts the order once, for everyone. When the preference is structured the worktree is created with NO startup agent, the executing host is then asked whether it can host a session for the workspace that now exists, and only then is a surface created. The host verdict cannot be hoisted above creation: `agentSession.createSupport` only answers for a workspace it can resolve, which is why the decision stays in two halves. Agent-first creation is deliberately preserved for PTY launches — it is what sequences the agent's startup command behind the setup runner, so wait-for-setup comes for free there. What actually differs per surface is only how a surface is built (an orchestration worker's session takes a dispatch hold and a mailbox a plain launch must not take), so that is injected as a factory rather than branched on. The intent also strips the reserved agent fields from a migrated create payload: a caller moving off `worktree.create` passes its existing params, and a stale `startupAgent` in there would re-create the very path this replaces. Tests assert order and arguments, not just the resulting mode. Reintroducing agent-first creation reddens 4 of 11. * feat(agent-launch): expose the launch executor as the agent.launch RPC Adds `agent.launch` — one host-side method that decides structured-vs-terminal and creates the surface — wired to the real runtime factories: `createManagedWorktree` for the workspace, forking on `startupAgent` exactly as the orchestration worker path does; `createStructuredAgentSessionForWorktree` for a chat session; and `createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the routing gap was reported on. `worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent" verbatim, because it answers with `agentTerminalHandle` only on that path: a host that quietly routed it to a structured session would hand every older client a response with no handle and no error. All new behaviour sits behind `agent.launch.v1`, which the host now advertises and a remote client must negotiate, so a client that does not gets today's behaviour unchanged. * feat(mobile): route workspace creates through agent.launch Picking an agent on the mobile create sheet always produced a terminal, even when the user's default was native chat, because all three create paths put `startupAgent` on `worktree.create`. That means "create the worktree agent-first", so its startup terminal IS the agent and the structured branch below it is unreachable — while the same phone's in-workspace "+" button opened a chat. The blank, branch and new-branch creates now send the same payload through `agent.launch` and let the host settle the surface. `worktree.create` is untouched, and a host that does not advertise `agent.launch.v1` (read from the existing `status.get` probe) keeps today's path exactly. Work-item creates stay on `worktree.create`: they pre-fill the issue/PR URL as an unsent `startupDraft`, which a structured session cannot hold yet, so routing them would submit the URL as a first turn. * fix(agent-launch): drop the deleted draft-prompt blocker from the reason map main removed the draft-prompt blocker in #19681 (a structured session now holds an unsent draft), so the exhaustive Record no longer typechecks. * chore(agent-launch): carry a SAFETY rationale on the agent placement cast The type-assertion gate landed after this branch's base, so the new file's copy of the worker-start cast is now a changed-code finding. * chore(agent-launch): carry agent.launch through main's RPC typing and casting gates The typed-method contract, the generated params catalog and the `assertionStyle: never` casting scan all landed after this branch's base. - AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its method name to `string` and broke assignability; every sibling infers instead. - `agent.launch` binds a schema under src/main, so it joins the catalog's RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin. - The now-typed methods make most test casts unnecessary; the few that remain carry the line-specific SAFETY rationale the casting gate requires. * test(mobile): supply the agent-launch fixture the create-submit recording needs The golden RPC recordings landed upstream while this branch was out, so they first met agent.launch here. Three things had to happen, and only one of them is a fixture bump. 1. workspace-settings-mounts.ts mounts useNewWorkspaceCreateSubmit against a fixture model that throws on any member it was not given. This PR added a required getAgentLaunchSupport, so the submit aborted with "Missing model fixture" before it ever issued the create, and three cleanup checkpoints vanished. That read like a product regression and was not one. Supplying the member restores the recording byte-for-byte; it is pinned false for the same reason the cutover probe is, so the baseline stays on worktree.create. 2. Editing that adapter moves adapterSha256 for the twelve settings goldens it mounts. Their recordings are unchanged - header only, by design: the digest is per-golden so editing a module fails exactly the goldens that mounted it. 3. Five goldens changed behaviourally, and both changes are this PR's: the capability probe now reports agentLaunch, and a create whose reply carries no worktree returns "Failed to create workspace" instead of throwing a TypeError off an unguarded result.worktree read. The launch route needs that guard, since a receipt can arrive without a worktreeId. * refactor(mobile): decode the launch receipt instead of asserting its shape The changed-code quality gate refuses type assertions, and the eight it flagged were worth removing rather than suppressing. The production one was the point. readAgentLaunchCreateOutcome asserted the RPC payload into Partial<AgentLaunchResult> and then runtime-checked it anyway, so the assertion bought nothing and claimed a contract the host had not proven. It now narrows with `in` and validates each hop, which is the same nullability question readCreateResult already answers on the sibling path - a launch receipt can legitimately arrive without a worktreeId. AgentLaunchCreateOutcome ties worktreeId to the shared contract so a change there fails this reader's typecheck rather than passing a differently-typed field through. The test fakes claimed a whole RpcClient via `as unknown as RpcClient` while implementing one member. They now build a typed literal, matching the pattern in use-mobile-structured-agent-options.test.ts. The read sites cast params and then read one field; they now assert the payload with toMatchObject, which removes the cast and pins more of the shape than the cast did. Also pins the warning passthrough, which nothing covered: a terminal launch that seats the workspace but cannot start the pty reports why, and the absent, blank, non-string and structured-surface cases report nothing. Writing that test caught a real drop I had introduced in the reader. * ci(mobile): re-run Mobile Checks when a shared capability changes Mobile Checks is path-filtered to mobile/**, but mobile imports the negotiated capability names straight from src/shared/protocol-version.ts and records the whole capability read verbatim in its goldens. So a capability added desktop-side rewrites a mobile fixture while never triggering the suite that would catch it. That is what happened here: #19849 introduced agent.launch.v1 and Mobile Checks never ran on it. Verified at the run level rather than by check name - the window-free check-runs API on 3837ae8d51 returns 49 check-runs across six runs (PR Checks x2, PR test LoC x2, Track Community PRs, Review) and no Mobile Checks among them. The breakage surfaced only in this PR, which happens to touch mobile/**. The workflow already concedes this pattern for terminal-file-link-conformance.ts; protocol-version.ts has the stronger claim, since mobile records its output. Also corrects the mount adapter's SAFETY comment. It claimed the recorder supplies only the members the hook reads, which was false the moment the hook gained a required getAgentLaunchSupport - and the assertion it annotates is exactly what stopped the compiler from saying so. The twelve goldens are adapterSha256 churn from that comment: every body is byte-identical, which is the digest doing its job. * docs(agent-launch): stop the receipt-wording comment claiming a migration The decision was never moved out of orchestration-worker-start-mode; this PR adds a second copy beside it. Say so, and name the unenforced agreement. * docs(agent-launch): stop the executor comment claiming a migration that has not happened The header asserted two things the tree does not support: that every launch surface routes through the executor, and that the mode decision "already lived" in `agent-launch-mode`. `agent.launch` is the executor's only consumer, and `orchestration-worker-start-mode.ts` is byte-identical (blob 92dc5c644a, 217 lines) at the merge base and all three stack heads, still used by workers.ts. Describe the two live copies and leave the cutover to later stack work. * fix(agent-launch): preserve setup and refusal fallbacks * refactor(mobile): parse the launch outcome into a named type at its boundary anti-slop/no-object-parameters flagged terminalLaunchWarning's `result: object`. The rule is pointing at a real seam rather than a style nit: the helper advertised a loose object and did the narrowing inside itself, so every caller handed it unparsed wire data and nothing downstream held a real type. Parsed at the boundary instead. parseTerminalLaunchOutcome takes `unknown` and returns TerminalLaunchOutcome | null, so the narrowing happens once, where the untrusted payload enters, and the consumer works with a named type. The type is taken from the shared contract rather than restated - a Pick over the terminal member of AgentLaunchOutcome - so a change to that union fails here instead of flowing through. `handle` is deliberately excluded: nothing reads it, and requiring it would drop the warning off a reply that omitted one, which is a behaviour change smuggled in under a typing change. No assertion and no config exemption: reintroducing `as Partial<AgentLaunchResult>` would trade this finding for the defect removed earlier in this branch, and the rule is correct here. The rule arrived with the merge-forward (#20781, newer than this branch's merge-base), and anti-slop is not one of the changed-code gate's six scans - it runs only repo-wide - which is why a clean local gate did not predict it. Behaviour is unchanged across all five warning cases, and the positive case was re-ablated on the new parser: dropping the warning reddens exactly it, 1 failed | 18 passed, restored byte-identical to 19 passed. * fix(agent-launch): dedupe complete launch and cancel setup wait * fix(agent-launch): memoize the whole launch so a replay cannot mint a second session A replayed agent.launch could create a second structured session in the same worktree, with activate: true. dedupeWorktreeCreate wrapped only the worktree half, inside the workspace factory. On a replay the create was reused, and the executor then continued to createSurface and built another surface inside it. The terminal route hid this: its cached create carries a startup terminal handle, so the executor returns on early. A structured create has no handle by construction - that is the whole point of the structured fork - so it fell through every time. Mobile replays this method deliberately on a delivery-ambiguous response, up to five attempts, so the path is reachable by design rather than in theory. The handler now wraps the entire launch in the same dedupe, on the same (repo, clientMutationId) identity, exactly as worktree.create wraps its own body. A replay returns the original AgentLaunchResult instead of re-running createSurface, which makes the two routes replay-identical. The inner dedupe is removed rather than kept. Wrapping both levels on one key deadlocks: dedupeWorktreeCreate stores the in-flight promise before the inner call runs, so the inner call would be handed the outer's promise, which is waiting on it. The launch-level memo subsumes the worktree-level one. Failures are still dropped rather than cached, so an unknown outcome stays unknown instead of replaying as a fabricated success. The guard replays a STRUCTURED launch: the terminal route cannot reproduce this and a test there would pass either way. Ablated against the pre-fix files - 1 failed | 22 passed, "expected vi.fn() to be called 1 times, but got 2 times", which is the duplicate session - then restored to 23 passed. The stub's dedupe had to be made faithful for that to be observable; the shared one passes through so other tests can see raw calls. * Revert "fix(agent-launch): memoize the whole launch so a replay cannot mint a second session" This reverts commit 59bc5e9b04. The same defect was already fixed upstream on this stack's base branch by 539e283c0f, which landed while this was being written. That change is broader (it also cancels the setup wait) and namespaces the dedupe key, so it supersedes this one. Reverting rather than hand-merging keeps a single implementation instead of a hybrid nobody chose. The behavioural guard from this commit is ported back on top of the upstream implementation separately: it asserts exactly one structured session survives a replay, where the upstream tests assert the dedupe wiring. * ci(mobile): close the round-1 signal gaps around agent.launch Three review findings, all narrow. Mobile Checks is path-filtered, and this branch made mobile's types depend on the shared RPC contract: rpc-params-contract.ts is a type-only re-export of the generated params catalog, and mobile/tsconfig.json includes **/*.ts. So a desktop-only edit under src/shared/rpc-contract/ could break mobile's typecheck with no mobile signal at all - the same blind spot the protocol-version.ts entry closed, one directory over. Added src/shared/rpc-contract/** to the paths filter. agent.launch had no cross-version trigger. Added the three prefixes a paired peer actually exchanges: the intent contract, the wire schema, and the RPC method. src/main/agent-launch/ is deliberately NOT listed - the executor shapes behaviour but is not itself wire, and AgentLaunchResult's shape is already covered by agent-launch-intent. Extending the cross-version SUITE to cover a negotiated handshake is separate work, not this. The break branch that answers an accepted-but-empty reply with "Failed to create workspace" had no unit coverage; the golden that used to discriminate it collapsed five partitions into one shared error when the null guard replaced the unchecked read. Covered on BOTH routes - worktree.create with no worktree.id and agent.launch with no worktreeId - since the branch serves both. Ablated by bypassing the guard: 2 failed | 11 passed, the two new cases returning a fabricated worktree instead of the error, restored to 13 passed. * fix(agent-launch): give a launch one place to say the workspace is incomplete createManagedWorktree reports an unspawned startup terminal or an uncopied working tree as a top-level `warning`, and worktree.create hands it straight to mobile. The launch path narrowed that result down to {worktreeId, startupTerminalHandle} and dropped it, so every agent.launch create lost a warning the old method surfaces - on both arms. The channel was also asymmetric by accident rather than design: a terminal outcome could carry `warning`, a structured one had nowhere to put it, so the arm this PR exists to enable was the arm that could not report an incomplete create at all. Now there is exactly one place a launch warning lives: AgentLaunchResult.warning, at the top level. It is about the create as often as the surface, it applies to a structured session and a terminal alike, and a reader should not branch on outcome.kind to discover the workspace it just opened is missing something. The terminal arm's own `warning?` is removed rather than left beside it - two homes for one fact is how they drift. Every producer folds in: the create, the surface, and the refusal downgrade. Consumer census before removing it: one production reader (mobile's readAgentLaunchCreateOutcome) and no others - the renderer and mobile launch call sites never read it. The mobile reader now reads the top-level field, which also lets its outcome parser go away entirely. Guard ablated by restoring the pre-fix narrowing: 2 failed | 24 passed, both carriers reporting `expected undefined`, which is the dropped warning itself; restored to 26 passed. The third case asserts an absence and stays green under the mutation by construction - it pins shape, not the defect. * fix(agent-launch): combine both launch warnings instead of dropping one Round 2 found the comment here was false. A create warning and a surface warning CAN both be set, on two reachable paths: 1. The create warns precisely BECAUSE it produced no startup terminal - didSpawnStartup stays false when that spawn throws, and orca-runtime-create-managed-worktree.ts:283 gates startupTerminal on it - so the executor's early return is skipped and a second surface is built, which can warn too. 2. An untracked-copy warning, then a definitive structured refusal downgrading to a terminal that also warns. `??` kept the first and lost the second with nothing saying so. They are now combined the way the create combines its own failures - appendFailure in runtime-local-worktree-terminal-startup.ts, and the startup-terminal catch in runtime-remote-managed-worktree-create.ts - which append rather than replace. The comment is rewritten to say what is true, and records the gap NOT fixed here: a create warning about a failed startup terminal is stale once the launch recovers by building a working one, so a user can be told the agent did not start while looking at it. Distinguishing those needs createManagedWorktree to stop multiplexing two unrelated failures into one string. Guarded and ablated: restoring `??` reddens exactly the new test, with the surface clause missing from the received string; restored to 27 passed. The structured-create stub had to admit its real ok-or-refusal union for the downgrade path to be modellable at all - it previously declared only the ok arm. Also: mobile.yml gains src/shared/agent-launch-intent.ts. It is the sole holder of the agent.launch RESULT shape - the rpc-contract catalog holds params only - and mobile imports it as a value. CROSS_VERSION_WIRE_PREFIXES already treats it as wire-critical; without this, one gate does and the other cannot see it. And the agent-first warning test no longer pairs "startup terminal failed" with a returned handle, a combination the producer cannot emit. * fix(mobile): read a launch warning an older host nests on the outcome agent.launch moved `warning` from the terminal outcome to the top level of the result. That is the right shape - a reader should not branch on `outcome.kind` to learn the workspace it just opened is incomplete - but on the wire it is a REMOVAL, and mobile only read the new place. A host built before the move still advertises the same `agent.launch.v1` capability, so the capability probe cannot tell the two apart and mobile takes this route against one: protocol-version.ts:360 AGENT_LAUNCH_RUNTIME_CAPABILITY is in RUNTIME_CAPABILITIES, the host list orca-runtime-get-status.ts:64 publishes it via status.get; the filter drops only browser.screencast.v1 and three E2E-gated capabilities, never agent.launch agent-launch-executor.ts such a host writes warning INSIDE outcome The result was a regression rather than a contract cleanup: the worktree.create path this replaces returned the warning at the top level and mobile read it, so a create that seated the workspace but could not start the agent surface - pty exhaustion, untracked files not copied - stopped explaining itself on the phone. Read both shapes for as long as such a host can be paired. Top level wins, and cannot be shadowed: AgentLaunchOutcome has no `warning` on either arm, so a current host cannot nest one. The test that pinned the old behaviour is inverted here. Its comment was the actual defect - it framed a legitimate warning from an older peer as a stale shape to defend against, which is what made dropping it look deliberate. * chore(mobile): raise the unchecked-reader ceiling for the agent.launch receipt main landed `unchecked-rpc-reader-inventory.ts`, a ratchet on RpcOperation readers that re-type their reply instead of validating it. Its ceiling for mobile-workspace-create-operations.ts is 4, counted on a tree without this branch's `agentLaunchRun`, so the merge produced "listed 4, found 5". The inventory's own header prescribes this case: a merge is the one time a line goes up without a migration undoing itself, and the instruction is to raise it and name the PR that brought it. It describes main landing an operation the branch never saw; here it is the mirror - the branch holds one main had not seen - so the line is annotated with #19850 rather than left bare. Not converted to `rpcResultVariant(variant, schema)`, which would lower the line instead. That is a validation change rather than a migration, which is exactly what the file's own comment says these five readers deliberately are not; the agent.launch reply is already guarded at the consumer, where readAgentLaunchCreateOutcome returns null on a malformed payload and the create surfaces "Failed to create workspace". Writing a schema now would also target a reply shape #20999 is actively redefining. Ablated: with the line back at 4 the ratchet fails "listed 4, found 5"; at 5 it passes. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-16 13:15:20 -07:00
Brennan Benson	170ebce1f2	fix(ci): run static analysis for every tree the repo-wide audits scan (#20918) ... A mobile-only diff is desktop-irrelevant, so should_run was false and every PR check skipped -- including the audits that do lint mobile/. The violation then landed on main and failed the same gate on every later PR's merge ref. Derive the trigger from the audit commands' own scan roots so the two cannot drift.	2026-09-16 01:13:06 -07:00
Neil	d62328aa4d	fix(codex): remove redundant Windows hook launcher for Unicode profiles (#20952) ... * fix(codex): reuse the Windows hook shell for Unicode profile paths * test(codex): register Unicode hook tests in Windows CI * test(codex): pin trust hash replacement during Windows upgrade * test(codex): retry transient Windows teardown locks	2026-09-16 00:30:52 -07:00
Neil	5127d1eb3b	refactor(windows): vendor the registry addon as @orca/windows-registry (#20438) ... * refactor(windows): vendor the registry addon as @orca/windows-registry windows-native-registry@3.2.2 was last published in 2023 by a single maintainer. Orca called two of its exports, both read-only, so the whole dependency is replaced by a local N-API addon under native/. The vendored addon is read-only by construction: setValue, createKey and deleteKey are gone, so RegDeleteTreeW no longer ships in the app. Two upstream defects are also fixed rather than carried over — the name/data scratch buffers were file-scope statics that concurrent reads would scribble over, and createKey/deleteKey called .c_str() on a temporary. Build wiring keeps the existing shape: still an optionalDependency gated to win32, still excluded from pnpm's allowBuilds so only Orca's own Windows rebuild runs node-gyp for it, still copied into the packaged resources. The CI native caches now key on the vendored sources so an addon.cc edit cannot restore a stale .node. * test(windows): check the vendored registry addon against reg.exe The addon is vendored source, so no upstream release proves it still decodes values the way Orca's PATH readers expect. reg.exe is the only independent oracle on the box. * ci(windows): register the registry addon test on the Windows runner A Windows-gated file self-skips on ubuntu, so without both registrations it reports success while running on no machine at all. * fix(build): link the registry addon as a workspace package, not file: As a `file:` dependency pnpm re-resolved and re-linked the package on every install, including `--frozen-lockfile` (measured: "added 1" on a repeat no-op install). That virtual-store churn ran concurrently with node-gyp reading the same tree and cost @vscode/windows-process-tree its binding.gyp mid-rebuild, failing package (windows) whenever the native cache hit and only that module needed building. The linux packaging job hit the same race from the other side, as a pnpm staging move failure. A workspace link resolves once and leaves the store alone; repeat installs are now 55ms no-ops. native/windows-registry is listed explicitly so `packages:` still does not auto-discover mobile/. * fix(build): stop tracking node-gyp output for the vendored addon The build/ tree is generated per host and ABI; the committed copy was macOS-specific gyp scaffolding from a local build and would have shipped stale Makefiles to every checkout. * chore: ignore the vendored addon's node-gyp bin output too node-gyp also emits bin/<platform>-<abi>/ beside build/; both are per-host generated output that must never be committed.	2026-09-12 20:10:49 -07:00
Neil	cbc7bb418c	fix(ci): read the changed-path list past the first pipe buffer (#19409) ... `pr-code-change-scope.mjs` read stdin with `readFileSync(0, 'utf8')`, a single read of fd 0. Once the writer outgrows the 64 KB pipe buffer that read returns early or throws EAGAIN, the script exits 0 having emitted no `name=value` pairs, and `tee -a "$GITHUB_OUTPUT"` records nothing -- so every lane the classifier gates is silently skipped rather than failing loudly. A PR opened long ago carries a stale `pull_request.base.sha`, so the gate's merge-base diff spans the whole base branch. PR #13178 diffed 13,294 files (773 KB) against a base 1,592 commits behind main and lost typecheck, test, static analysis, xterm patch sync, package and e2e to this. Stream stdin instead, matching how the sibling `pr-e2e-source-routing.mjs` already reads the same list in the same workflow.	2026-09-07 17:57:05 -07:00
Neil	314506003a	fix: retain MSYS shell descendants in their terminal job (#19068) ... * fix: retain MSYS shell descendants in their terminal job * test: complete MSYS regression CI registration and teardown contract * fix(windows): deny job breakaway for the whole Cygwin/MSYS shell family The per-PTY job probed only msys-2.0.dll, and only for bash.exe/sh.exe. Cygwin ships the same spawn.cc breakaway logic under cygwin1.dll, and an MSYS2 zsh escapes exactly like its bash does, so both kept the orphan bug. Probe the runtime DLL on the shell's own search path instead of matching shell names: that is the property that decides whether the runtime will ask for CREATE_BREAKAWAY_FROM_JOB, and it drops the name special-casing. * chore(patch): restore the conpty.cc index line The earlier hand-edit dropped it while every sibling section kept one. Recomputed against the real blobs: applying this patch to 7b286d3d yields exactly 4b06d185, so git apply -3 has its fallback back.	2026-09-07 00:35:55 -07:00
Brennan BensonandMerge Sim	1478101342	fix(windows): unblock structured native chat by exposing process creation time (#18986) ... * fix(windows): guard process creation times * fix(windows): ask the relay's bare addon for creation times too The relay addon build now emits creationTimeMs, but the runtime binding for the bare addon still declared only CommandLine, so a Windows relay host requested flag 2 and every row came back without a creation time. That leaves captureWindowsDescendantSnapshot returning null and verifyWindowsProcessIdentity false forever on those hosts -- the relay half of the patch was unreachable. Naming CreationTime in the adapter is safe because the bare addon is a content-hashed relay artifact: it ships in the same immutable relay directory as the bundle reading it, so it can never be older than the code asking for the bit. Also bound the win32 guard test on our own row, which the addon can never fail to answer, so an unconverted FILETIME or a 1601-epoch stamp fails instead of satisfying a bare count. * fix(windows): make the compiled addon prove its own CreationTime support CI caught the real defect: the win32 guard test read isWindowsProcessStartTimeAvailable() as true and then found 0 rows carrying creationTimeMs. Unlike node-pty, this package publishes a prebuilt .node at the same build/Release path node-gyp writes to, so pnpm patches the source tree and leaves that binary alone. A host then holds a patched lib/index.js -- ProcessDataFlag.CreationTime and all -- over a binary that ignores flag 4, and neither a load check nor a path check can see the difference. So the binary now says so itself: addon.cc exports supportedProcessDataFlags, lib/index.js re-exports it, and - windows-process-tree-creation-time.cjs asserts it during install, which is what forces a from-source rebuild. It is shared by the Node probe in ensure-native-runtime.mjs and the Electron probe in rebuild-native-deps.mjs, exactly as node-pty-job-ownership.cjs is -- the Electron half matters because that probe decides onlyModules, so without it the packaged app would ship the stale prebuilt. - isWindowsProcessStartTimeAvailable() gates on the reported bit, not the enum. Believing the enum is worse than reporting false: the descendant snapshot returns null forever and the exit proof latches unverifiable while structured chat believes it has a reaper. rebuildNodeRuntimeModules could not actually have rebuilt this package: the patched binding.gyp includes deps/node-addon-api, which the tarball does not ship, and node-gyp must run from the physical dir. Also closes the relay repair path's divergence: repairCreationTimeSources wrote the C++ but not the buildNode splat or the tree-node typing, and assertPatchApplied checked neither, so a repaired tree passed as patched with buildProcessTree silently dropping the field. The guard test is unchanged. * fix(windows): keep the process-tree patch LF-only windows-process-tree-patch-contract.test.mjs requires the patch file to carry no CR bytes. Regenerating through pnpm patch-commit emitted 199 of them, because the creation-time change is the first to touch files the package ships as CRLF (src/process.h, src/process_worker.cc, src/addon.cc, lib/index.js, lib/index.ts, the typings) -- and #17886's own hunks over binding.gyp and src/process_commandline.cc carry the rest. Stripping them is safe and changes nothing the lockfile records: pnpm hashes patches CRLF-normalized, so the digest stays e66202cc623996d02040c93449eb9ae353fddadf426cb53202a59ee710ee6fe7 and now equals the file's plain sha256 too. It also still applies -- verified against a deleted store entry, not a warm one -- and the precedent was already there: the previous patch was LF-only and had been patching those same CRLF files all along. ensure-native-runtime.test.mjs stages the siblings the script loads at module scope into its temp project. The import walk added by #17886 sees `from './x.mjs'` only, so the createRequire'd .cjs siblings still have to be named, and this PR adds a second one. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-06 13:59:59 -07:00
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>	2026-09-05 21:33:16 -07:00
3 people	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>	2026-09-05 21:13:06 -07:00
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>	2026-09-05 21:12:59 -07:00
3 people	bfc6a262a7	fix(windows): read command lines from the kernel, not each process's PEB (#17886) ... * fix(windows): read command lines from the kernel, not each process's PEB MDE incident D scored Orca for suspicious memory activity: the vendored `@vscode/windows-process-tree` recovered every process's command line by opening it with `PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and chaining three `ReadProcessMemory` calls through the PEB and `RTL_USER_PROCESS_PARAMETERS`. On a 750ms/2s cadence over the whole table that is the credential-dumping primitive, whatever the intent. Windows 8.1 added `NtQueryInformationProcess`'s `ProcessCommandLineInformation` class (60), which returns the same string as a kernel-built `UNICODE_STRING` under `PROCESS_QUERY_LIMITED_INFORMATION` alone. Electron's floor is Windows 10, so every supported OS has it. The PEB reader stays behind a process-wide latch that only `STATUS_INVALID_INFO_CLASS`/`NOT_SUPPORTED`/`NOT_IMPLEMENTED` can set; a pid that merely denied a handle does not re-arm it, because `PROCESS_QUERY_INFORMATION` implicitly grants the limited right and so cannot be obtained where the weaker open already failed. The same hunk drops `PROCESS_VM_READ` from `GetProcessMemoryUsage` and `GetCpuUsage`, which acquired it and never read an address space. Measured on Windows 11 (514 processes), counted in-process by swapping the addon's import table entries for counting stubs, per CommandLine scan: `ReadProcessMemory` 1128 -> 0, desired access 0x0410 -> 0x1000, p50 12.7ms -> 9.3ms. Command lines were byte-identical on every process both readers recovered (376/376, 379/379 across runs), including a 24,068-character argv with quotes, non-ASCII and trailing whitespace, and a WOW64 target. Three processes that refused the old rights granted the new one; none went the other way. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * fix(windows): drop the PEB fallback and detect the unpatched prebuilt Review of #17886 found three ways the reader could still perform, or silently resume, the primitive it exists to remove. The class-missing latch was a permanent, process-wide, one-way downgrade back to the PEB read, and any single target returning STATUS_INVALID_INFO_CLASS / NOT_SUPPORTED / NOT_IMPLEMENTED could trip it. On an EDR-hooked ntdll -- the entire premise of this change -- a hook that does not recognise class 60 would have restored PROCESS_VM_READ plus three ReadProcessMemory per pid per scan for the life of the process, unobservably, on precisely the machines this was written for. The fallback is deleted rather than guarded: GetProcessCommandLine now returns false and leaves the command line empty, which callers already handle, so the addon imports no ReadProcessMemory at all. That absence is what makes the property checkable on the artifact. The published 0.8.0 tarball ships a loadable prebuilt built from unpatched source; it is node-addon-api, so a bare require() accepts it, allowBuilds is false and CI installs with --ignore-scripts, and a rebuild that soft-exits on a Windows file lock leaves it in place. Source-text guards could never see it. windowsProcessTreeAddonReadsProcessMemory() checks the compiled binary instead, and is wired into the install check, the rebuild, and the relay build. The repair itself never worked: `git apply` run inside a work tree prefixes patch paths with the cwd-relative prefix, skips what does not match, and exits 0, so the branch always fell through to its own post-check throw. The package dir is always under the project root, while the fixture that covered it was in %TEMP%, outside any repo. Blinding git with GIT_DIR fixes it, and the test now runs inside a real work tree. Also from review: bounds-check the returned UNICODE_STRING against the allocation (not the size the second query clobbers) and cap the probe so a bogus length cannot bad_alloc a whole scan; test NT_SUCCESS explicitly; value- initialize ProcessInfo, which left `memory` as stack garbage -- measured, 82 processes reported the same bogus working set; and correct a comment in windows-process-table.ts that still described the command line as a PEB read. Re-measured on Windows 11 (543 processes): ReadProcessMemory 1128 -> 0, with the symbol absent from the import table so the IAT hook finds no slot to count; desired access 0x0410 -> 0x1000 on all 543 opens; p50 13.5 -> 12.3ms; 405/405 command lines byte-identical including a 24,087-character quoted non-ASCII argv and a WOW64 target; 3 processes recovered only by the new path, 0 only by the old. * chore(deps): refresh the windows-process-tree patch hash in the lockfile * test(scripts): stage a script's local imports into the native-runtime fixture ensure-native-runtime.mjs gained an import of windows-process-tree-gyp-rebuild.mjs, but the fixture copied only the script itself, so every case in the suite died with ERR_MODULE_NOT_FOUND before reaching its own assertions. copyScriptWithLocalModules already walks a script's co-located imports for exactly this reason -- its own doc comment names this failure -- so use it rather than listing files by hand. The two Windows cases still fail here, on a missing node-pty ConPTY runtime that also fails on main; this only stops a resolution error from standing in front of whatever they were meant to catch. * fix(windows): route a locked stale addon to the Windows file-lock message `pnpm install` with Orca running aborted with a raw EPERM stack. The stale-binary guard -- which deletes an addon that still imports ReadProcessMemory so a skipped rebuild cannot use it -- ran outside the try whose catch classifies Windows file locks, and whose message is literally "Close running Orca/Electron/dev processes for this worktree": exactly this situation. Measured rather than assumed: rmSync against a loaded (memory-mapped) addon throws EPERM, and `force: true` does not help, since it only swallows ENOENT. Cold copies of the same file delete fine. So the delete threw a page before the handler that knows what it means. Moving the guard inside the try is the whole fix; the classifier already matches the EPERM text. The new case runs the real script against a temp project whose stale addon is held open by a live child process, and fails against the old placement with the raw `syscall: 'rm'` stack the report described. * feat(windows): warn once when command-line recovery is refused host-wide Removing the PEB fallback removed a total-defeat vector, but it left a cliff: if NtQueryInformationProcess(ProcessCommandLineInformation) is refused -- a hooked ntdll that does not know class 60 -- every command line comes back empty and agent identity matching silently degrades to image names. The addon still loads and still enumerates, so every health check the app has stays green. A cliff nobody can see is the failure mode this area keeps producing. The querying process is the unambiguous probe. A process can always open itself with PROCESS_QUERY_LIMITED_INFORMATION, so its own command line coming back empty means the query is refused for every process -- not that some target denied a handle, which is normal for roughly a quarter of the table. Keying on our own row rather than a fraction means no threshold to tune and no false positive on a hardened box where most processes deny. One warning per session, gated on the CommandLine flag actually being requested so a future identity-only reader cannot trip it. The suite's own SELF fixture gains a command line for the same reason: a self row without one is the alarm, not a detail. * fix(windows): check the relay's staged addon at load, and answer tri-state Two gaps in the ReadProcessMemory check, both about what it does not see. It only ever looked at node_modules/@vscode/windows-process-tree. A relay host has no node_modules of ours: it loads ./windows-process-tree.node staged beside the bundle. The relay build asserts the symbol on the artifact it produces, but a bundle and the addon beside it redeploy independently, so a host that has not taken a new bundle keeps whatever binary is already there -- and the published prebuilt is node-addon-api, so it binds cleanly and then walks every process's address space. loadWindowsProcessTree now checks that file too and refuses it, falling back to the CIM scan: slower, but not the thing an EDR quarantines a host for. The predicate is duplicated rather than imported, because the config-script copy is install-time tooling that drags in node-gyp and child_process, and this module is bundled into the app and the relay. And it returned false for a binary that is not there. All three callers happened to be safe, but the name read as a safety predicate, so a future caller would take a missing binary as verified. inspectWindowsProcessTreeAddon() now answers clean/unpatched/missing over an explicit binary path -- which is also what lets the relay's staged addon be checked at all -- and each caller states which state it acts on. Both are covered by cases that fail against the old code: without the load-time check the unpatched staged addon is bound and the CIM fallback never runs, and with 'missing' folded back into 'clean' the absence case fails outright. * test(windows): load the addon in beforeAll, not at collection time loadAddon() ran while the file was being collected, so on a Windows checkout with no built addon the require threw before any case existed and took the seven patch-text cases down with it -- cases that read only the patch file and need no binary at all. Verified both ways against a deliberately unresolvable addon path: at collection time vitest reports "no tests" for the file; from beforeAll the seven text cases pass and only the three addon cases go. * fix(deps): normalize the windows-process-tree patch to LF and let pnpm own its hash `pnpm install --frozen-lockfile` failed on this branch on every platform with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, which breaks CI and the release build. Two coupled defects. The patch file was committed with CRLF -- 174 CR bytes, against zero on main -- and `.gitattributes` pins `/config/patches/*.patch -text` precisely so checkout cannot convert it, so those bytes reached every runner. And pnpm hashes a patch **LF-normalized**, so the raw sha256 of a CRLF file is a value pnpm never computes: raw sha256 322965470c05f63d8527f7d8e892ee26ee444136b66b57fd64c362a9f2ff05d1 LF-normalized f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e The lockfile carried the raw one, at all three sites. It is the only one of the seven patches where the two digests differ, which is why the other six passed. Normalized the patch to LF and took pnpm's own value from `pnpm install --no-frozen-lockfile`; nothing here is hand-computed. With the file LF-only the two interpretations coincide, so the lockfile, the contract test's no-CR assertion and its hash assertion all agree at one number -- and `config/scripts/windows-process-tree-patch-contract.test.mjs`, which was red on this branch for the same reason, is green again. The lockfile diff is exactly the three hash lines. The regression check is the installer, not a digest. Two separate reviews "verified" the shipped hash by recomputing sha256(patchBytes) and matching the lockfile; both were wrong, because both repeated the same wrong assumption about which bytes pnpm hashes. A check that reproduces the original mistake is not independent. So the new case runs `pnpm install --frozen-lockfile --lockfile-only --ignore-scripts` against a copy of the manifest, lockfile and patches, and asserts exit 0 -- verified by deletion: restoring the shipped hash fails it with the exact ERR_PNPM_LOCKFILE_CONFIG_MISMATCH from the branch's package (windows) job. Also corrected the `.gitattributes` comment claiming pnpm hashes patches byte-for-byte. The `-text` setting is right -- `git apply` needs the exact bytes -- but that sentence is the claim that produced the wrong hash twice. * ci(windows): run the process-tree patch suites in CI Both suites only self-skip off Windows, so the binary-level check that the addon carries no ReadProcessMemory passed vacuously in every lane. * fix(windows): force core.autocrlf=input for the patch repair My LF normalization of the windows-process-tree patch broke the `git apply` repair path introduced in this PR. The two are coupled and I checked only one. Those 174 CR bytes were not editor noise. They sat on exactly the pre-image lines and nowhere else -- 107/107 in src/process.cc, 67/67 in src/process_commandline.cc, 0 on every added or context line -- because @vscode/windows-process-tree@0.8.0 ships those two sources as CRLF. Normalizing the patch made its pre-image stop matching the file it is applied against. Measured, reconstructing the true CRLF pre-image from the pre-normalization blob and applying the current LF patch: core.autocrlf plain -c core.autocrlf=input true exit 0 exit 0 input exit 0 exit 0 false exit 1 exit 0 `false` is Git's own built-in default and what "checkout as-is" selects in the Git for Windows installer -- on this box the `true` that hides it comes from the installer's system gitconfig, not from anything in the repo. There the repair throws, ensureWindowsProcessTreeCommandLinePatch reports "still reads the PEB, and repairing it ... failed", isWindowsNativeLockError does not match that text, and `pnpm install` dies with no path forward. Forcing the mode rather than `--ignore-whitespace`: both fix every cell and both leave the applied file fully LF, but `input` relaxes line endings only, so a hunk whose real content drifted is still rejected. The repair rewrites a security-relevant source file; it should stay strict about everything except the thing that is legitimately ambiguous. Not reverting the patch to CRLF: windows-process-tree-patch-contract.test.mjs (pre-existing on main) forbids CR bytes in it, and pnpm computes the same hash either way. LF plus the forced mode is the end state. The suite could not have caught this. The fixture built its pre-image from the patch itself and joined with '\n', so fixture and patch agreed by construction on any encoding -- once again a test that passes without its fix. It now emits the CRLF the real package ships, and the case runs under both autocrlf modes pinned through a temp HOME gitconfig, because the repair blinds git to the repo and so reads global config. Verified by deletion in both directions: with the flag removed the autocrlf=false case fails with the exact "still reads the PEB" dead end while autocrlf=true still passes, and with the fixture back on LF all eight cases pass with no fix present at all. Also corrected the .gitattributes comment I added last commit. It said `git apply` needs the bytes the patch was written against, which is now false -- the pinned bytes are LF and the bytes it was written against are CRLF. That is the same class of confident-and-wrong claim that produced the bad hash twice. * fix(windows): assert the rebuilt addon, and install the patch for real in tests Three follow-ups from review. **The packaged binary had no check.** The relay build asserts its own artifact and ensure-native-runtime asserts what it loads, but nothing looked at the addon copied into the packaged app -- so a rebuild that silently produced the upstream reader shipped. `rebuild-native-deps.mjs` now asserts `clean` on it after `rebuild()`. This is also the caller D4's tri-state was missing: every existing site branches on `=== 'unpatched'`, so `missing` still behaved exactly like `clean` everywhere, which was the thing making it a state rather than a boolean. Here both non-clean states fail, and they fail differently: after a rebuild that reported success, an absent binary is a broken build, not an absence to shrug at. The fake `rebuild()` had to start producing a binary for that to mean anything, so it now emits stand-in bytes and takes `addon: 'clean' | 'unpatched' | 'none'`. Verified by deletion: with the assertion removed both new cases pass. **The frozen-install case could not see a patch at all.** `--lockfile-only` resolves and never applies one, so its coverage stops at hash consistency. Added a case that installs `@vscode/windows-process-tree@0.8.0` for real with the patch and asserts the materialized `src/process_commandline.cc` carries the marker and no longer carries `ReadProcessMemory` -- about 1.5s for the pair. Correcting the brief on that one: it does **not** catch the `git apply` breakage from the previous commit. Measured -- with `-c core.autocrlf=input` removed it passes cleanly, because `pnpm install` uses pnpm's own patch applier and never runs our repair script. What it does catch is a patch pnpm can no longer apply: corrupting one pre-image line fails both cases. The repair path stays covered by the CRLF fixture in rebuild-native-deps-node-pty.test.mjs. Worth recording, since it decides whether the LF normalization was safe at all: pnpm applies the LF patch to the CRLF tarball sources without complaint, and materializes them as LF with the marker present and `ReadProcessMemory` absent. The primary install path was never affected -- only the `git apply` fallback was. **Dead timeout.** The frozen-install case passed `timeoutMs: 300_000` to the spawn while vitest capped the case itself at 30s, so on a cold runner vitest would have killed it first. Both cases now declare the budget they use. * test(windows): route the frozen-install check through the pnpm invocation owner The new patched-dependencies check hand-rolled a PATH walk naming 'pnpm.cmd', which the windows batch shim spawn boundary ratchet rejects: pnpm-cli-invocation already owns that decision for every other script, and its allowlist only shrinks. Reuse resolvePnpmCliInvocation for the command and prefixArgs, and the shared resolveCliCommand for the presence check, so no shim name is spelled here. Its `shell` flag is dropped because runProcessSync refuses it and already drives a shim through the interpreter itself. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>	2026-09-05 21:12:47 -07:00
OrcaWinandOrca Worker	687a22e1ee	fix(computer-use): run the Windows runtime as one persistent helper (#17858) ... * fix(computer-use): run the Windows runtime as one persistent helper Microsoft Defender for Endpoint raised multi-stage Execution + Collection incidents against Orca on Windows ("Screenshots were taken unexpectedly on this device... Screen capture code was found in a script launched by powershell.exe", factor "Executes suspicious MSIL code"). The desktop script provider spawned a fresh powershell.exe per operation, so a single computer-use session produced a burst of short-lived PIDs and re-emitted runtime.ps1's inline Add-Type P/Invoke assembly on every click. runtime.ps1 gains a -Serve mode that loads its assemblies once and then reads NDJSON requests from stdin, and a new DesktopScriptRuntimeHost owns one long-lived child: lazy spawn, strict serialization, a 30s per-request timeout, restart on crash, a 120s idle shutdown, and dispose() on provider teardown. The one-shot -OperationPath path stays as the fallback, and Linux keeps its python3 bridge unchanged. Both Windows spawn sites now use -ExecutionPolicy RemoteSigned instead of Bypass, falling back once to Bypass (and logging) when a Restricted host refuses the unsigned script. * fix(computer-use): recover the runtime host instead of latching it off Review follow-up on the persistent Windows computer-use helper. A helper that died before producing a line set an unavailable flag nothing ever cleared, and the client then dropped the host for the life of the session. One transient bad spawn — a Defender scan, a locked CSC temp directory — silently restored the per-click powershell.exe burst and per-operation MSIL emission this work exists to remove, with computer use still working so nothing looked wrong. Start failures are now retried, then cool down for 60s, then re-probed; the client keeps the host so it can come back. Repeated post-answer crashes cool down too, and a single reply no longer clears the failure count. The one-shot bridge decided its execution-policy retry from a message that fell back to stdout, so a window title containing "SecurityError" could replay a non-idempotent operation — a double click, keystroke or paste — and stick the session on Bypass. The retry now requires empty stdout and a matching stderr. Serve-mode replies carry an echoed request id. Without one a single stray stdout line would make every later response answer the previous request, acting on stale element indexes with no error raised; a mismatch now kills the child. Non-JSON noise is ignored rather than counted as the helper having answered. Also: warnings reach the main process over the sidecar's IPC channel rather than its piped, unread stdio; the child is watched on close rather than exit; dispose latches so a queued request cannot respawn during teardown; and the host is split into a serve channel and an availability policy to stay under max-lines. * fix(computer-use): prove a helper never started before replaying its request The retry that replaced the permanent-latch bug could deliver unrequested input. send() re-sent the same request whenever the helper died without replying, but "no reply came back" is not "the operation did not run": runtime.ps1 synthesizes the click and only then builds the snapshot, which allocates a full-window bitmap and walks the UIA tree — a native GDI+/UIA fault there is uncatchable, and leaves the click already delivered. A deterministic fault meant three clicks from the host plus a fourth from the one-shot bridge, surfaced as a single failed operation. -Serve now writes one {"ready":true} line after its Add-Type work and before its first read, so "never started" is a fact rather than an inference. A request is replayed only when the helper died before announcing. A runtime.ps1 that predates the announcement — reachable through the provider path override — is covered by an observation-tool allowlist until a ready line proves otherwise. Host-detected aborts (timeout, desynchronised reply, oversized line) suppress the exit handler, so they were bypassing failure accounting entirely and a helper failing that way was respawned once per operation forever. They now count and are logged. Also stop charging twice for one outage: entering the cooldown resets the failure count, so the first death after recovery no longer re-enters a full cooldown and an interleaved workload cannot be stranded on the one-shot bridge. * fix(computer-use): ignore a stdin write callback from a torn-down helper stop() destroys stdin, so a write still queued at teardown calls back with ERR_STREAM_DESTROYED. The callback carried no channel or request identity and write() had no closed guard, so it ran abortChannel a second time: stopChannel no-opped but recordFailure and the warning did not, charging two failures for one operation and reaching the 3-strike cooldown at half the intended rate. That feeds the same accounting that keeps a persistently broken helper from respawning once per operation. The same root also allowed a late callback landing after a replacement channel existed to stop that channel and reject a different request with the previous one's error. Node fires the destroyed-stream callback on the next tick, well before a new request arrives, so the double-count is the reachable effect; binding the callback closes both. write() now drops payloads and error reports once closed, and the host ignores any report whose channel or request id is no longer current. * test(computer-use): pin each stale-write guard independently The channel's closed guard and the host's request-identity check are redundant by design, and the existing tests only failed when both were absent. Someone deleting one, believing the other was the covered one, would have got a green suite and a live regression — the same shape as a test that passes without the fix it was written for. Each is now pinned on its own. The channel's half is tested against the channel directly: after stop() it takes no writes and reports no error from one already queued, which the host cannot observe because it drops the channel at the same moment. The host's half is pinned by the case the channel cannot see — a live channel whose request was already answered, where backpressure delivers a write callback for a request that is no longer pending. Removing either guard alone now fails a test. Both carry a comment saying they are deliberately redundant and separately pinned, so the next reader does not have to rediscover this from the diff. * ci(windows): run the computer-use runtime host 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(computer-use): time the runtime host cooldown on a monotonic clock The start-failure cooldown was a wall-clock deadline, so a backwards step — an NTP correction, a VM snapshot restore, a user changing the clock — left `remainingCooldown()` returning the cooldown plus the whole step. A one-hour step measured 3,660,000ms, and ten real minutes later still 3,060,000ms. Nothing shortens it from there. Only `recordSuccess()` clears the cooldown on a non-dispose path, and no request can reach a helper to succeed while it holds, so every `send()` throws `runtime_host_unavailable` first. The host is built with no `now` override and its lifecycle is a module-level singleton that shuts down at process exit, so the latch held for the sidecar's life — computer use kept working via the one-shot bridge while the per-click powershell.exe burst this host exists to remove came back silently. Store the instant the cooldown began and compare elapsed monotonic time, following the two fixes in #17884. The field is `number | null` rather than sentinel 0 because `performance.now()` legitimately returns 0. Both new tests leave `now` unset, because the bug was in the default the host picks and a test that injects a clock cannot see it. * fix(computer-use): give a queued request its own deadline The 30s request timeout was armed only in `sendOnce`, once a request reached a helper. A request behind N timing-out ones therefore waited roughly N times that with no deadline of its own: bounded, but the caller sees an `await` that looks hung for minutes and gets no error to act on. Move the serialization tail into its own class and arm a deadline at enqueue time. Only the wait is bounded — a request that reaches a helper still gets its full execution budget, so nothing that used to succeed now fails. An expired request is dropped rather than sent late: the caller has already been told it failed, and a click delivered after that is worse than no click. The tail keeps its never-rejecting shape and chains on the turn rather than on the raced promise, so a caller giving up early cannot release the next request while its predecessor is still in flight. * fix(computer-use): stop reading a locked file as an execution policy block `UnauthorizedAccess` is the FullyQualifiedErrorId PowerShell reports for a policy block, and it is also a strict prefix of `UnauthorizedAccessException`, which .NET raises for any ordinary locked or ACL-denied file. The predicate matched the token unanchored, so an AV scan holding runtime.ps1 or a locked CSC temp directory was read as a policy block. Two consequences, both bad. `escalateExecutionPolicy()` has no path back, so one false match spent the rest of the session on `-ExecutionPolicy Bypass` — the exact command line token this stack exists to stop emitting. And on the one-shot path `isPolicyBlockedStart` re-runs the operation: one-shot mode writes stdout only after the operation returns, so a crash partway through an action is indistinguishable from a helper that never started, and the click lands twice. Measured on Windows against all three records, which the test carries verbatim as fixtures: policy/Restricted FullyQualifiedErrorId: UnauthorizedAccess policy/RemoteSigned FullyQualifiedErrorId: UnauthorizedAccess genuine access denied FullyQualifiedErrorId: UnauthorizedAccessException `\b` is the whole discriminator: between `s` and `E` both sides are word characters, so no boundary exists there and the exception cannot match. Dropped two alternatives that measurement showed were wrong. `PSSecurityException` never appears — the record surfaces through a native-command wrapper and reports `ParentContainsErrorRecordException`. The prose is wrong three times over: it differs by policy, it is localized, and PowerShell hard-wraps it mid-sentence. Anchoring on the `FullyQualifiedErrorId:`/`CategoryInfo:` labels would be more precise again, but those labels are localized where the values are not, so it would lose a real block on a non-English host and strand it with no fallback. Matching the values with word boundaries keeps both directions; a fixture with translated labels pins it. The escalation stays sticky. With the predicate correct, it only fires on a machine that really does block, where re-probing the preferred policy would buy a guaranteed failed spawn per operation. * fix(computer-use): route a malformed request back to the request that caused it `ConvertFrom-Json` throws before `$requestId` is read, so the serve loop answered an unparseable request with an untagged error. On the client that is not an error at all: `deliver()` sees no matching id, calls `abortChannel`, kills the helper and charges a failure — and the helper's own message is discarded. A parse failure was reported as a stream desync with no trace of the real cause, and three of them walked into the 60s cooldown behind three misleading "did not match" messages. Recover the id from the raw line when the parse fails. No wire change: the response shape is untouched and `BridgeResponse.requestId` already documents this echo. It is the same shape the helper already returns for `not_a_tool`, where the id survives because it is read before the operation runs. Both mixed pairings degrade safely — a new script with an old client resolves the error normally, and an old script with a new client still aborts, but now reports what the helper said. When the line is mangled past recovering an id, the desync abort is the honest outcome, so keep it and carry the helper's text into it rather than replacing it. A line the helper could not tag is usually the only account of the cause. Proven against the real `runtime.ps1 -Serve`: the host can only write well-formed JSON, so the parse-failure branch is unreachable through it and the test drives the channel directly. * fix(computer-use): keep the Bypass escalation only when Bypass actually works AppLocker and WDAC constrained language mode raise PSSecurityException under the same SecurityError category a real execution-policy block uses, so the predicate matches them - correctly, on the evidence available. But those block the script at parse time, which `-ExecutionPolicy Bypass` cannot lift. The escalation was sticky unconditionally, so on a WDAC host we misdiagnosed, retried, failed again, and then latched: every later command line carried the most heavily weighted MDE token there is, on exactly the hardened, monitored enterprise machine that is watching for it. Treat the escalation as the diagnosis it is. A fallback that cannot start a helper either disproves it - the policy was not what stopped the first attempt - so revert to RemoteSigned instead of latching. When Bypass does start a helper the diagnosis is confirmed and it stays sticky exactly as before, so a genuinely Restricted machine still never pays a re-probe per operation. The revert lands inside the outage rather than only at its end, so a misdiagnosis costs one Bypass command line instead of one per attempt, and an escalation that never proved itself does not outlive the cooldown that ends the outage. Deliberately not a permanent "fallback is useless" flag: a Bypass attempt that failed for a transient reason would then disable the fallback for the session, which is the same latch in the other direction. Only `runtime_host_unavailable` proves no helper started, so only that reverts; a helper that started and then died proves Bypass works. That also makes the policy branch reachable on a final attempt for the first time, so it now rejects as unavailable rather than a generic error - that code is what routes the operation to the one-shot bridge, which carries its own policy fallback, and without it an all-blocked host would fail operations outright instead of degrading. The pre-existing "reports itself unavailable when Bypass is also refused" test pins that. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 21:12:33 -07:00
OrcaWinandOrca Worker	b6ca8dad99	fix(hooks): register the Claude hook script directly on Windows (#18875) (#18905) ... * fix(hooks): register the Claude hook script directly on Windows (#18875) The Windows Claude Code lifecycle hook was registered as `powershell.exe -NoProfile -EncodedCommand <...>` whose entire decoded payload was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. Every hook event paid a full PowerShell start-up to reach a script that exits at its first `ORCA_PANE_KEY` guard, so sessions outside Orca paid it to do nothing. Register the script path itself instead, with `|| echo {}` for the neutral-JSON-when-missing contract (#14818). Measured on Windows 11, invoked as Claude Code invokes it (`printf payload | bash -c -l "<command>"`): idle (n=12) baseline 177ms | before 471ms | after 213ms 10-way conc (n=40) -- | before 656ms | after 296ms p95 under load -- | before 696ms | after 337ms It also drops an interpreter from the chain the hook's timeout kill must tear down. Killing the hook does not kill its PowerShell grandchild, which still holds the stdout handle the agent reads to EOF -- measured, EOF arrived 352ms AFTER the kill, when the orphan exited by itself. msys2 creates children suspended and resumes them after, so a kill landing in that window strands one that never exits and EOF never comes; that is the reported frozen session. The encoded launcher stays as the fallback for profile paths the shells cannot carry bare (space, `%`, `^`, `&`, non-ASCII) and for hosts where Git Bash is not resolvable, because PowerShell 5.1 rejects `||`. Every other agent's hook is untouched, as is the remote/SSH path. Not adopted from the report: `cmd.exe /d /c <path>` (MSYS rewrites the `/c` under Git Bash -- measured, the invocation fails), and raising the 10s timeout (the orphan survives the kill regardless; the fast path puts the hook 30x under the budget so the kill effectively stops firing). * fix(build): list the new hook launcher modules in the CLI tsconfig project config/tsconfig.cli.json enumerates its files explicitly, so the two new imports reached by src/main/claude/hook-settings.ts failed tc:cli with TS6307. src/main/git-bash.ts pulls in only node:fs, node:path and a shared constant, so it adds nothing heavy to the CLI project. * fix(hooks): address review of the direct Windows Claude hook launcher - Make the Windows hook suites host-independent. A box with a cmd.exe AutoRun (HKCU\...\Command Processor\AutoRun) failed them at HEAD too: the tests redirect USERPROFILE, the AutoRun target vanishes, and MSYS spawns a .cmd without /d so AutoRun runs and lands on the hook's stderr. Seed an empty target, including under the deliberately-absent profile. - Note in managed-hook-stdin-lifecycle why the "missing managed script" case no longer exercises the fallback for the direct shape (it carries an absolute path, so a redirected profile changes nothing); that path is covered live in windows-direct-cmd-hook-command.test.ts. - Keep the direct shape off UNC profiles: WINDOWS_CMD_SAFE_PATH admits them, but //server/share/... is not a command cmd.exe reliably starts. - Correct the comments: `|| echo {}` also fires when cmd.exe itself exits non-zero (failing AutoRun), printing {} twice. The encoded launcher exited 1 on that same box, so neither shape is clean there. - Test the contract that replaced runtime %USERPROFILE% resolution (STA-3348): a stale absolute path reports not_installed and is rewritten on install. - Record the standing unmeasured assumption in windows-edr-posture.md: `||` does not parse in Windows PowerShell 5.1, so a compat consumer that hosts hook strings there would fail closed. Measure before widening to another agent. - Trim the launcher comments per AGENTS.md; the numbers live in the doc. * test(win32): register the new Windows-gated hook test in the CI lane win32-test-lane-registration guards against exactly this: a Windows-gated file that self-skips on ubuntu and reports success, so it runs on no machine. The new windows-direct-cmd-hook-command.test.ts needs both entries — WINDOWS_PACKAGE_TESTS decides whether package_windows runs for a diff, and the workflow argv decides whether the file runs once that job started. * test(win32): remove the hook temp tree through the retrying helper windows-lane-tree-removal-boundary scans exactly the specs in the Windows CI lane, so registering windows-direct-cmd-hook-command.test.ts subjected it to the rule: cmd.exe and bash have just exited in that tree, and a raw recursive rm throws EPERM on Windows while their handles drain, turning a green spec into a lane failure. Use removeTreeSync, which carries the repo's maxRetries policy. --------- Co-authored-by: Orca Worker <orca-worker@localhost>	2026-09-05 17:50:33 -07:00
Neil	0a821e5bc8	fix(crash-reporting): make the own-Chromium gate a real choke point, and stop a refusal leaking the root (#18459) ... * fix(crash-reporting): make the own-Chromium gate a real choke point Round-3 review found the guard was not the choke point its own comments claimed: six pid-addressed `taskkill /pid <pid> /t /f` families in main were ungated and uninstrumented, so the stale-pid shape stayed producible and a `selfInitiatedTreeKillCount: 0` could read as exculpatory when it was not. - Gate the remaining main-process families: the git command-runner abort, the notebook-cell and automation-precheck timeouts. - Turn the `src/shared` seam into the gate itself (`process-tree-kill-gate`), so the runProcess choke point, the codex app-server deadline kill and the ephemeral-VM recipe kill ask the same decision. Those three are compiled into the CLI/relay too and cannot import main; main installs the guard at preflight. - Ratchet (`main-process-tree-kill-gate.test.ts`): a new pid-addressed taskkill in main that skips the gate fails, and the allowlist entries must still exist. - Give pid-addressed kills eviction priority in the 32-entry ring: 32 routine `win-pty-job` teardowns from a window-close burst no longer evict the one entry that discriminates a self-kill from an external one. - Correct the coverage doc, which described the uninstrumented Windows sites as POSIX `process.kill(-pid)` group kills and omitted the git and codex paths. * fix(crash-reporting): keep a refused tree-kill from leaking the root it owns A refusal must block the pid-addressed tree walk, not the termination. Five of the six gated sites returned on refusal with no fallback, so a refused `taskkill /pid /t /f` left git.exe, a timed-out notebook cell, an automation precheck or an ephemeral-VM recipe running while the caller reported it stopped. The root kill is addressed by the child handle, which cannot reach the recycled pid the refusal is about, so it stays correct and required on that path. Also fixes the ring eviction the scope preference introduced: with the ring saturated by pid-addressed kills, the only non-pid-addressed entry is the one just pushed, so the splice evicted itself and the detail came back `{}` -- byte-identical to the external-kill arm, in the window-close case the guard exists for. Eviction now excludes the newest entry and falls back to FIFO. Tests: refusal now asserts the root kill at all six sites, and the ring covers the saturated-pid ordering as well as round 3's group-burst ordering. * fix(crash-reporting): stop a refused tree-kill leaking the commit-message agent, and count call sites Two round-5 blocking findings, both open on main and on both branches. `killSourceControlAgentProcess` had no root-kill fallback on its win32 arm: the taskkill was the only termination, so once the own-Chromium gate could refuse it the promise resolved having killed nothing. Both callers do `terminationComplete ??= killSourceControlAgentProcess(child)` and then release the managed-home lock on that promise, so a refusal left the local Codex/Claude commit-message agent running while the caller reported it stopped -- the lock-contention failure the taskkill was added for. Same fix as the six sibling sites: the handle-addressed root kill cannot reach the recycled pid the refusal is about, so it stays correct and required on that path. The ratchet was file-granular, not call-site granular: one gate mention anywhere in a file exempted every taskkill in it, which left the six files that now ask the gate ratchet-blind -- the inverse of what it is for. It now counts `/pid` call sites against gate admissions per file, so a second ungated kill inside an existing family fails. Keying on the `/pid` argument rather than a quoted `taskkill` also catches a kill whose program name comes from a constant. The three comments that claimed more than the old scan enforced now state the rule and its two remaining blind spots. Also: the recording in `admitSelfInitiatedTreeKill` is now wrapped the way the `admitProcessTreeKill` seam already wraps it, with the refusal decision taken before anything that can throw so a diagnostics failure cannot flip it; and `orca-chromium-process-pids` documents the false-positive direction (a stale `getAppMetrics()` entry plus pid reuse refuses a live unrelated child), which is the mechanism the root-kill fallback exists to bound. Tests: refusal now asserts the root kill at all seven sites; the ratchet asserts call-site counting and the constant-program form. * test(crash-reporting): run the own-Chromium gate against real Windows trees Nothing on this branch had ever executed on Windows. The unit tests pin the gate's decision against a mocked taskkill, which cannot show that the decision does anything to a real process: that `/T /F` reaps a detached grandchild, that a refusal leaves that tree standing, or that the handle-addressed root kill the refusal path falls back to reaps the root while orphaning descendants. Adds a win32-gated live test covering all four, registered in both the `package_windows` CI lane and `WINDOWS_PACKAGE_TESTS` as `win32-test-lane-registration` requires. Also completes the coverage doc's "never instrumented" list, which omitted the macOS keyboard-input-source probe's POSIX group kill in `ipc/app.ts`. * fix(crash-reporting): pin the commit-message root kill on the Windows arm The first Windows run of this branch found nine failures the macOS suite cannot see: `commit-message-text-generation-test-harness` asserts `expect(child.kill).not.toHaveBeenCalled()` on `process.platform === 'win32'`, which is the contract the previous commit deliberately replaced — and it branches on the real platform, so it is dead code everywhere CI runs today. The harness now asserts the handle-addressed root kill on every platform. On win32 it lands after the tree walk, so the expectation waits rather than reading one tick early, and its ten call sites await it. Red against the pre-fix arm at all seven sites; the production code is unchanged. * test(crash-reporting): remove the Windows lane marker tree through the retrying helper The new win32 spec teardown used a raw rmSync, which the windows-lane-tree-removal boundary ratchet rejects — and which is exactly the EPERM the ratchet exists to prevent, since this spec's marker directory is written by processes it has just force-killed. * fix(crash-reporting): only refuse pid-addressed tree walks, disclose the handle-less codex site The own-Chromium gate refused the POSIX process-group arm of signalProcessTree as well, which was new macOS/Linux behaviour: a stale getAppMetrics() entry plus pid reuse would orphan a group that main reaps today. A POSIX group only holds what Orca put in it, so the refusal is now scoped to win-taskkill-tree and the POSIX arm is recorded and admitted like the other group kills in main. That also drops the synchronous getAppMetrics() read from every POSIX termination. codex-turn-added-roots kills roots found by a table walk, so a refusal has no handle to fall back to. Pin that the refusal is visible - crumb written, turn reported as not cancelled - rather than fixing what cannot be fixed. * test(crash-reporting): detach the Windows survival fixture and observe real spawns	2026-09-04 16:42:42 -07:00
3 people	f36c03e84a	fix(windows): make the install-dir ACL repair rescue the launch it runs in (#18361) ... * fix(windows): repair the poisoned install-dir ACL before the window, not after The install-dir LPAC ACL poison (electron/electron#51761) still costs every affected machine at least one crash: the probe that detects it is setImmediate-deferred and answers 0.9-3.0s in, while createMainWindow runs synchronously in the same frame and its renderer dies at init 48-1373ms later. - Persist the poison verdict the moment the probe reports it, and await the repair (bounded at 20s) before any window is created on a launch that already carries the marker. - Do not engage the GPU safe-graphics fallback while the install-dir ACL verdict is poisoned or still outstanding. Safe graphics does not rescue a poisoned tree, and --in-process-gpu removes the GPU child, erasing the sibling-death evidence that identifies the shape (4 field reports landed in 'misc' this way). - Clear the safe-graphics marker once the repair lands, so a repaired machine stops launching software-rendered for the rest of that build. - Give the repair marker a bounded retry budget: it was written on failure and matched regardless of outcome, so one transient failure pinned a machine to 'marker-hit' for the life of that version. * test(windows): pin the install-dir ACL repair against the real icacls binary * fix(windows): stop the install-DACL verdict from outliving the evidence Adversarial review round 1. Five blocking findings, all addressed. 1. gpu-lifecycle guard had only a source grep (green with the polarity inverted). The stated justification -- that gpu-lifecycle's import graph cannot be driven in-process -- was wrong: mocking `electron` plus `@electron-toolkit/utils` imports it fine. Replaced with gpu-lifecycle-install-dir-acl-guard.test.ts, which drives the real handleGpuChildCrash against a stub tracker. All four cases go red when the guard is flipped to `if (!isInstallDirAclSuspect())`. 2. A clean probe verdict retired the on-disk marker but not the in-memory `poison` verdict, so a machine the probe just proved healthy kept suppressing the GPU safe-graphics fallback and kept the dialog accusing the install folder -- permanently, since a `status:'failed'` probe deliberately keeps the marker. A positive clean reading now latches `installDirReadClean`, drops the verdict, and outranks a repair result that lands after it (a 'failed' from a repair with nothing left to fix must not re-accuse). 'repaired' is kept: it is not a contradiction and it is what tells the user to reload. 3. `noteWindowsInstallDirAclProbePending()` ran on every `openMainWindow` while the probe is once-per-process, so every tray/second-instance reopen armed a 15s window in which `recordGpuCrash` was never called at all -- on healthy machines. `probeWindowsInstallDirAcl` now reports whether THIS call dispatched, and only a dispatch arms the grace window. 4. The pre-window ordering guarantee was defeatable and untested. `focusExistingMainWindow` opens a window whenever there is none and the app is ready -- true for the whole 20s gate, which is exactly when a user double-clicks the shortcut again. Added a `canOpenWindow` seam (same 'pending' semantics as the existing `!app.isReady()` case) wired to `isBlockingInstallDirAclRepairInFlight()`, plus windows-install-dir-acl-startup-wiring.test.ts pinning the await ahead of both window-creation paths and both new call sites. 5. windows-install-dir-acl-repair.win32.test.ts was absent from the pr.yml win32 allowlist, so it ran nowhere. Added. Also from the non-blocking list: - The repair no longer clears a `userConfirmed: true` safe-graphics marker; "keep safe graphics" is a user choice, not Orca's automatic latch. - `repairWindowsInstallDirPackageAcl` now reports its dispatch too, so a second entry into the gate resolves immediately instead of eating the full 20s budget waiting on an `onDone` that is never coming. - The gate is wrapped in try/catch/finally, matching the contract the probe documents as mandatory for anything upstream of window creation. Rebutted, not applied: - "Gate should be conditioned on app.isPackaged." A dev launch only carries the poison marker if a dev launch actually probed that tree and found the signature, in which case the dev renderer is dying the same way and the repair is exactly what is needed. The adjacent `isPackaged` check guards a packaged-only early-window optimisation, not a correctness boundary. - "Fold the poison marker into the repair marker's `outcome`." They answer different questions with different lifetimes. The repair marker is a retry budget (`attempts >= 3` disables the repair for that version) and is never cleared; the poison marker is cleared by a successful repair and by a clean probe. A `'pending'` outcome written before the attempt would bump `attempts`, so three launches killed mid-repair would permanently disable a repair that never once ran icacls to completion. * fix(windows): keep counting GPU crashes while the install-DACL verdict is pending Adversarial review round 2. Both blocking findings addressed. 1. handleGpuChildCrash early-returned on isInstallDirAclSuspect() BEFORE recordGpuCrash, so the crash left no trace in the 30s rolling window. The suspect window is armed on every win32 non-serve launch, and the field bundles put it at 0.8-1.7s after main_window_created on hosts whose DACL is clean (matchesPoisonSignature=false) -- squarely inside the 2.1-6.2s bad-driver bursts this repo already pinned in gpu-crash-fallback-field-sessions.test.ts. A healthy machine with a failing driver could lose an entire coalesced burst and never engage safe graphics. The crash is now always recorded; only the engagement consults the verdict, and it waits for the verdict rather than acting on the suspicion (waitForInstallDirAclVerdict, resolved by the probe's onDone or by the existing 15s grace, whichever lands first). Deviation from the review's suggested shape, deliberately: awaiting the verdict before persisting anything reintroduces the exact race gpu-fallback-engagement.ts documents -- Chromium aborts the whole browser process on the 6th GPU crash, ~1.3s after the 3rd, which is less than the probe takes to answer. So the unconfirmed marker is written up front and withdrawn if the verdict comes back poisoned. A machine killed mid-wait still comes back software-rendered, and its marker is unconfirmed, which is the state the repair's own clear already retires. gpu-lifecycle-install-dir-acl-guard.test.ts now drives the real GpuCrashFallbackTracker and the real engagement path (the restart prompt firing is the signal) instead of a stub tracker, and covers the case the previous suite could not express: a burst that lands entirely inside the pending window still engages once the probe reports clean. Four reverts go red -- restoring the pre-record guard (2 tests), dropping the wait, dropping the post-wait re-check, and dropping the pre-wait marker write (2 tests). 2. The round-1 evidence block quoted commits, a test name and pass counts that no longer exist, and its real-icacls Windows run predated the commit that rewrote the gate. Re-run at this commit; counts and the live-Windows result are restated in the handoff rather than carried forward. Also from the non-blocking list: - 'marker-hit' conflated "already repaired" with "retry budget spent", because hasMarkerFor matches outcome === 'repaired' too. The result now carries alreadyRepaired, and the recovery maps that to stage 'repaired' -- so a launch killed between a successful repair and its marker clear no longer tells the user the folder needs an administrator, no longer latches isInstallDirAclSuspect() for the session, and does retire the poison marker. Not applied, with reasoning: - "clearGpuFallbackMarker narrowed to userConfirmed === false leaves the target population software-rendered after a repair." The summary was overstated and is corrected, but the narrowing stands: a userConfirmed marker now requires a clean DACL verdict, because the restart prompt that writes it is exactly what the gate above withholds while the install is a suspect. The population this family targets can no longer reach confirmMarker while poisoned. - "writeInstallDirAclPoisonMarker re-stamps on a budget-exhausted machine forever." True, but on that machine the tree really is still poisoned and the gate resolves immediately ('skipped', no icacls spawn, no 20s wait), so the marker is telling the truth. Retiring it would be wrong; only a clean probe reading should. * fix(windows): register the real-icacls spec and stop its teardown racing icacls Two ratchets were red: - windows-lane-tree-removal-boundary: the win32 spec's afterAll used raw rmSync on a tree two icacls.exe children had just rewritten DACLs on, which is the EPERM race removeTreeSync exists for. - win32-test-lane-registration: the spec was in the pr.yml argv but not in WINDOWS_PACKAGE_TESTS, so a future diff touching only test files would not select package_windows and the spec would self-skip on ubuntu and report success. * fix(windows): re-arm the GPU fallback latch when the install-DACL verdict withholds it recordGpuCrash reports the threshold crossing exactly once and latches `engaged`. handleGpuChildCrash consumes that report before consulting the DACL verdict, and installDirAclClearsGpuFallback then discards it — so nothing could ever engage safe graphics again in that process. A machine whose tree the repair fixes and whose driver is genuinely broken stayed hardware-accelerated through an unbounded crash loop, with no prompt and no marker. disengage() releases only the one-shot latch; the crash window is untouched, so a real driver burst is still never erased. Test is RED without the re-arm. * fix(windows): keep the safe-graphics marker while an install-DACL repair is in flight The gate dispatches a repair without arming the probe clock, so waitForInstallDirAclVerdict() returns immediately and the withdrawal deleted the marker inside Chromium's FATAL window (crash 6 lands ~1.3s after crash 3, well inside the 20s gate). The process then died mid-repair, spent no attempt, and relaunched hardware accelerated into the same gate — spawning the same GPU children, FATALing again, forever. Hold the marker while poison.stage is 'pending' so that launch comes back software rendered and the next gate runs to completion. Still not engaged this launch, so --in-process-gpu does not erase the sibling-death evidence. A terminal verdict has no next step to rescue, so it still withdraws. Both new tests are RED without the retention. * fix(windows): stop a repaired marker outranking a fresh poison verdict The probe reads the install DACL and finds it poisoned; `startRepair` dispatches; `markerHitFor` sees a repair marker recording `outcome: 'repaired'` for the same installDir+appVersion and reports `alreadyRepaired`, which the recovery module maps to stage 'repaired'. So the launch that just proved the tree poisoned runs no icacls, deletes the poison marker that arms the next launch's pre-window gate, clears the suspect flag so `--in-process-gpu` can engage on a tree safe graphics cannot rescue, and tells the user "Orca repaired the permissions." Reachable whenever the tree is re-poisoned after one successful repair of the same version, and whenever a repair reports success without clearing the tree — the silent icacls no-op this module exists to document. A DACL reading taken this launch now outranks the marker: `probeConfirmedPoisoned` stops `outcome: 'repaired'` short-circuiting the repair. The attempt budget still bounds it, so an unrepairable tree does not re-spawn icacls forever. The pre-window gate does not set the flag — it acts on a marker from an earlier launch, not on evidence of its own, so a recorded repair still outranks it there. Also drives the GPU-fallback re-arm test through a repair that actually completes 'repaired', rather than a later clean probe, which is the route the review exercised. * fix(windows): make the pre-window ACL gate act on the poison evidence it fired on The gate fired on a poison marker — an earlier launch's DACL reading that nothing has retired — but withheld `probeConfirmedPoisoned` from the repair, so a repair marker recording an older success still short-circuited it. On the three-launch shape the gate exists for (repair succeeds; tree is re-poisoned; the next launch's probe records the poison but dies before writing its repair marker) the gate ran no icacls, deleted the poison marker that arms every later gate, un-suspected the tree so --in-process-gpu could engage, and told the user "Orca repaired the permissions." `applyInstallDirAclProbeVerdict` then swallowed that launch's own reading behind `if (poison) return`. Both callers of `startRepair` hold outstanding poison evidence, so the flag is now unconditional (renamed `poisonEvidenceOutstanding`) and `marker-hit` means only that the attempt budget is spent. The probe guard is narrowed to an in-flight gate repair: a reading taken after the gate finished re-arms the poison marker and downgrades a claimed repair. Also: withholding safe graphics now ends with the repair budget. A machine whose attempts are spent while the signature persists was denied safe graphics on every launch for the life of that appVersion — and had its marker deleted each time — including the healthy installs the probe's flag-blind ACE match over-matches, where the driver really is broken. Non-blocking, same lane: re-read `isQuitting` after the up-to-15s verdict wait, and skip the recovered-launch prompt when the ACL gate retired the marker read before whenReady. * fix(windows): stop a timed-out gate repair outranking a later poison reading The gate's 20s budget expires while icacls runs on under its own 120s cap, so the probe can read the tree poisoned while that repair is still in flight. Its success claim then deleted the poison marker, un-suspected the tree and told the user their permissions were fixed. The reading is now latched and outranks it. * fix(windows): stop a gate repair claim pre-empting this launch's probe reading Round-7 adversarial findings, both driven against the real modules: - isInstallDirAclSuspect returned false the moment the pre-window gate set stage 'repaired', short-circuiting ahead of the probe-pending grace check. The GPU children die 48-1373ms after window creation while the probe answers 0.9-3.0s in, so an icacls that silently no-opped (exit 0, tree untouched) opened exactly that interval to --in-process-gpu on a still-poisoned tree - and a 'keep safe graphics' answer then pinned a userConfirmed marker no later repair may clear, with the poison marker already deleted so no later launch gates. The claim now stays provisional until this launch's probe corroborates it or the grace window lapses. - A probe reading that disproves a 'repaired' claim re-armed the poison marker but never restored the unconfirmed safe-graphics marker the claim had cleared, so the next launch relaunched hardware-accelerated into the re-armed gate. The clear is now captured and handed back on disproof. * test(windows): pin the nested and update-inherited grants against real icacls The live spec asserted the grant landed on the root-level module file only. It now also pins that the flagless /T pass reaches a nested file carrying its own protected DACL (the shape app.asar.unpacked and node_modules have), and that a file written after the repair inherits the (OI)(CI) root grant - the stated reason that grant form exists. * fix(windows): keep the recovered-launch prompt silent while the tree is the suspect Round-8 fresh-eyes finding, driven against the real modules: the prompt re-read the marker the pre-window gate may have retired, but never consulted isInstallDirAclSuspect() - so after a FAILED gate (tree still a live suspect, window blank behind the 10s reveal fallback, Keep as both defaultId and cancelId) a 'keep it' answer pinned a userConfirmed marker no later repair may clear, on the exact victim class the repair cannot help. The guard now covers both gate outcomes; staying silent leaves the marker unconfirmed, which a successful repair still retires. --------- Co-authored-by: Orca Worker <orca-worker@localhost> Co-authored-by: OrcaWin <alpha-eng@stably.ai>	2026-09-03 21:39:34 -07:00
Brennan BensonandMerge Sim	98e77ef1a7	feat(mobile): structured native Codex chat (#18074) ... * feat(mobile): finalize structured native Codex chat * fix(mobile): close structured chat lifecycle gaps * wip(mobile): fence stale structured inventory and bound operation-id retention Fence local structured-session inventory and subscription responses with a sync generation so a toggle-off clear, reconnect restore, or retry cannot apply a mirror from a superseded instance. Bound mobile ambiguous operation-ID retention at 128 with unmount cleanup. Staged on the reconcile branch only: the sync module is now 312 lines and needs a real split before this can reach the PR head. * fix(ci): split the structured session-tabs sync and give static analysis mobile types The local structured session-tabs sync module outgrew the 300-line cap once it took on generation fencing, so split it along its real seams instead of raising the cap: the generation/cursor fence, snapshot projection, snapshot apply, inventory refresh, and the subscription loop. The original path stays as a barrel so no importer moves. Repoint the host-session-mirror settle census at the apply module, which owns two receipts now — the snapshot it mirrors in, and the toggle-off teardown that retracts what it published. The teardown receipt is named rather than anonymous so the pin says which direction it settles. The changed-code quality gate lints mobile files and resolves their types from mobile/node_modules, but mobile is a separate pnpm project that the root install never populates, so every mobile type degraded to an `error` type and the gate reported phantom findings. Install mobile dependencies in static analysis when the diff touches mobile, gated on a new classifier output. * fix(mobile): let a slow capability handshake still reach connected The mobile capability update is an advisory whose result is discarded, yet an unanswered one was fatal while an explicit rejection was tolerated. A 5s timeout on the direct client force-closed the socket, and on the relay path it failed `confirmResume` before `connected` was ever published, so a consistently slow link redialled forever. Both paths now share one helper that settles every ambiguous outcome (timeout, mid-flight drop) like a rejection and rejects only when the frame never reached the wire — the one case nothing else recovers from, since the socket's own desync force-close is gated on already being connected. The generation guard still keeps a replaced session from connecting. Retained structured-session operation ids were capped at 128 with oldest-first eviction, but every retained id belongs to a send whose outcome is unknown, so eviction turned a user's retry into a second message on the host. Bound the map by expiry against the id's own embedded timestamp instead, mirroring the host's operation ledger, so no id is released while the host would still honour it. Also give the mobile CI install the root install's lockfile drift guard (mobile's lockfile carries patchedDependencies a silent rewrite would drop), gate mobile_dependencies on should_run, and key the pnpm store cache on both lockfiles. * refactor(mobile): extract the relay pending-request registry The merge composed two independently-sized changes — this branch's capability handshake settle and main's dial-stage tracking — pushing the relay session file to 304 lines against a 300 cap. Neither side broke it alone. Move the in-flight request registry (id generation, tracking, settlement, and reject-all with its delivery-ambiguity marking) into RelayPendingRequests, matching the existing collaborator pattern alongside RelayDialStageTracker and RpcSessionLivenessWatchdog. No behavior change. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-03 15:19:26 -07:00
Jinwoo Hong	3eec77c11a	chore(cloud): add the relay fence broker, ops console, Terraform root, scripts, and 24 cloud-* workflows (#18413) ... Phase 6 of the relay split: the relay's deploy/operate surface moves under cloud/ with 24 cloud-* workflows gated on ORCA_CLOUD_OPERATIONS_ENABLED, the Cloud SQL rollout lease action, the relay Terraform root (dual-accept identities for both repositories), scripts, docs, CODEOWNERS, and a terraform validate job in Cloud Verify.	2026-09-03 06:55:14 -04:00
Neil	f37d2fec97	fix(linux): land the reviewed Linux packaging stack on main (#18100) ... * fix(linux): give the CLI one entrypoint by extracting the AppImage once * refactor(linux): trim AppImage CLI registration seams * test(cli): assert registration lock serialization * fix(linux): fence AppImage terminal shim mounts * fix(linux): accept extracted AppImage runtimes with APPDIR only * docs(linux): make headless AppImage extraction runnable * refactor(linux): import bundled launcher directly * fix(linux): reclaim superseded AppImage payloads and packaged symlinks Pruning removed 3215 of 3216 files from a superseded generation and always stranded resources/app.asar, leaking ~105 MB per version update. Electron's asar shim reports a *.asar file as a directory, so the recursive remove tried to rmdir a real file and failed with ENOTEMPTY; the .catch(() => {}) hid it. Reproduced end to end on Ubuntu 24.04: 519M -> 623M across one update, and 519M again once the payload is actually reclaimed. removeExtractedAppImagePayload holds process.noAsar for the removal, counted so overlapping removals cannot hand the shim back early, and the prune site now warns with the path instead of swallowing the rejection. All three removal sites use it -- staging cleanup and displaced roots leaked the same way. Also reclaim symlinks left by a packaged deb/rpm install, which the extracted-cache-only rule turned into a hard conflict on a deb -> AppImage migration, and name the remedy in the conflict error. * fix(linux): bound the CLI registration lock wait `retries: 1000` caps the attempt count, not elapsed time, so at up to 1s per attempt an IPC-driven registration could hang ~16 minutes against a wedged holder with no feedback. A legitimate holder is bounded by the extraction timeout, so wait that plus slack and then fail with a message naming the lock file, rather than hanging. `maxRetryTime` is forwarded verbatim to the `retry` package by proper-lockfile. * fix(linux): stop re-extracting the AppImage on inode metadata churn The extracted-payload cache key hashed ctime alongside dev/ino/size/mtime. ctime moves on any inode metadata write -- `chmod +x`, which every AppImage user is told to run, plus `chown`, an ACL or SELinux relabel, and a backup restore -- none of which alter a byte of the payload. Measured on Ubuntu 24.04: `chmod +x` leaves dev, ino, size and mtime identical and moves ctime alone, so the key changed and the next launch paid a full ~519 MB re-extraction and a multi-second stall to rebuild a payload it already had, then pruned the old generation. Key on content identity instead. An in-place content change moves mtime and almost always size; a replacement moves the inode. The existing replace-in-place test still passes. * fix(linux): stop CLI commands from falling through to Chromium startup * refactor(cli): remove redundant command membership check * test(cli): cover command-named project selectors * fix(cli): redirect the open-url command before startup * test(linux): cover AUR serve wrapper flags * fix(linux): tighten CLI launch detection * fix(linux): respect CLI flag value boundaries * fix(linux): strip injected Chromium switches from CLI args * fix(linux): report a missing display instead of dying in uv_close * refactor(linux): read display locks without a preflight race * fix(linux): preserve unverified external displays * chore: format reliability gate manifest * test(packaging): split runtime resource checks * fix(linux): fail serve when no display is available * fix(linux): do not treat a lockless X socket as a dead display An X server writes its lock beside its socket and both survive a crash (verified against Xvfb under SIGKILL), so a socket with no lock was never left by a crashed server. It is an endpoint published from elsewhere: a container bind-mounting only /tmp/.X11-unix, WSLg, or a foreign PID namespace. Declaring those dead made the desktop gate exit(1) on displays that work, with no workaround, and the serve gate refuse to start. Liveness now splits by ownership. A foreign DISPLAY trusts a lockless socket; Orca's own :99 does not, because removeStaleDisplayArtifacts unlinks the lock before the socket and so manufactures that state itself -- adopting it would resurrect the orphan-socket bug and stop the cleanup from self-healing. The stale-lock rejection is unchanged. Also correct four doc statements this behaviour falsified. * fix(linux): fail closed when a stale socket blocks the Xvfb rebind Readiness only checked that /tmp/.X11-unix/X99 exists. A stale socket we could not unlink still exists after our own Xvfb refused to bind, so Orca set DISPLAY to a dead server and Chromium died in Ozone init. Measured on Ubuntu 24.04 against the pre-fix build: with a leftover :99 socket and no lock, serve exits 139 (SIGSEGV), the socket inode is unchanged before and after, and no lock is recreated -- it neither cleaned up nor respawned. To a user that is a crash, not a misconfiguration. This is reachable in the documented topology, where orca-xvfb.service has no User= and runs as root while serve runs as User=orca: /tmp is sticky, so the orca uid cannot unlink a root-owned socket, rmSync fails, and Xvfb exits with the display already active. Readiness now requires the display to actually be live -- our socket plus a lock naming a running process -- so the same state reports an unusable display and exits 1 with the existing diagnosis. * fix(linux): recognise abstract X sockets and inherited Wayland fds Two display setups this gate could not prove were refused outright, and on the desktop path that is app.exit(1) with no workaround. An X server may bind only the abstract namespace (`@/tmp/.X11-unix/X0`), which leaves no filesystem socket to stat. Abstract addresses are kernel-owned and vanish the moment the owner exits, so an entry in /proc/net/unix is proof of a live server -- no lock file needed and no stale entry possible. Verified on Ubuntu 24.04, where 139 such addresses were present. WAYLAND_SOCKET is an already-connected fd handed over by the compositor, so there is no path to stat and WAYLAND_DISPLAY may be unset entirely. Its presence is the display. Both are consulted only after the filesystem-socket check fails, so no existing verdict changes. * fix(linux): never treat Orca's own display number as a foreign endpoint Recognising a lockless X socket as live is correct for an endpoint published from elsewhere -- a container bind mount, WSLg -- because an X server writes its lock beside its socket and both survive a crash. It is wrong for VIRTUAL_DISPLAY_NUMBER, because Orca's own teardown unlinks the lock before the socket and so manufactures that exact state. The managed branch was already strict, but a caller that sets DISPLAY=:99 explicitly takes the foreign path and skipped it, accepting a dead display left by Orca's own interrupted cleanup. Route the managed number through the strict probe on both paths. Found by an adversarial audit of the asymmetry introduced earlier in this branch; the documented systemd topology is unaffected because its Xvfb writes a real lock. * test(linux): add a packaged-artifact contract for the CLI launch paths * test(linux): avoid buffered serve readiness detection * test(linux): signal AppImage serve owner directly * test(linux): tolerate readiness timeout boundary * test(linux): add startup margin to shutdown oracle * ci(linux): give package contracts timeout headroom * fix(ci): route all Linux packaging contract changes * test(linux): poll shutdown readiness without tail leaks * test(linux): bound shutdown cleanup grace * test(linux): assert on CLI output, not the harness's own control lines run-cli-case.sh echoes `RESULT status=N case=<name>`, and the two cases named *-skills asserted `expectOutput: 'skills'`. That substring was satisfied by the case name in the harness's own line, so 2 of 8 cases asserted nothing about the command -- gutting `skills` entirely would still have gone green. Control lines are now excluded before matching, and both cases assert the rendered help header, which only real help output produces. Verified on an Ubuntu 24.04 host: 8/8 still pass against a stack-tip AppImage. Also register the gate in reliability-gates.jsonc, which #15085 added a CI Docker gate without. Red/green is recorded from a stock release AppImage failing 4 of 8, three of them at status 133 (SIGTRAP). * fix(linux): require static AppImage runtimes (#17319) * test(linux): reject a wrong-architecture native binary at packaging time Cross-building the arm64 slice on an x64 host silently packed an x86-64 `pty.node` -- the rebuild logged "Forcing native rebuild for linux-arm64" and shipped the host's binary anyway. Every gate here inspects symbol versions, which are perfectly valid on the wrong architecture, so nothing noticed. Observed on a Raspberry Pi 5: the packaged app loaded, then failed with "Failed to load native module: pty.node", and the launch contract reported 3 of 8 cases crashed rather than naming the cause. Swapping in the aarch64 `pty.node` took the same build to 8/8. Compare ELF `e_machine` against the slice being packaged and fail with the offending path. Checked before the glibc pass, because a wrong-architecture binary's symbol versions are valid but meaningless and would send the reader down the wrong path. Release CI builds arm64 on a native runner, so this guards local and future cross-builds rather than a shipped artifact. * test(linux): judge per-arch vendored binaries against their own path The first CI run of the architecture gate failed the x64 package job on `@parcel/watcher-linux-arm64-glibc/watcher.node`. That binary is arm64 on purpose: the package ships every architecture and its loader picks the match, so its presence in an x64 build is correct. Judge a binary against the architecture its own path names, falling back to the slice when the path names none. That keeps the case this gate exists for -- `bin/linux-arm64-*/node-pty.node` holding an x86-64 binary, which is what shipped to a Raspberry Pi 5 -- while letting multi-arch dependencies through. Dry-run over the real dependency tree flags nothing for either target arch. * fix(linux): move deb/rpm update installation outside Orca (#17318) * fix(linux): complete deb/rpm package metadata * fix(linux): preserve CLI link during package upgrades * docs(linux): document local RPM build prerequisites * fix(linux): move deb/rpm update installation outside Orca * fix(updater): preserve Linux recovery across stale events * fix(updater): fence stale downloaded events by active target * fix(updater): preserve active Linux package recovery * test(linux): keep workflow order assertion in scope * test(updater): assert stale recovery stays silent * fix(updater): preserve Linux package recovery after checks * refactor(updater): keep Linux marker message with status * fix(linux): describe the right manual update path for deb/rpm hosts A remote host installed from .deb or .rpm now reports manual-service-update-required, and the guidance told the operator to "update through the service manager that starts this server" -- which is correct for unsupported-headless-serve but wrong for a package install, where nothing about the remedy involves the service manager. Say both, keyed on how the host was installed. * docs(linux): document orcad update restart safety * docs(linux): scope restart census omissions * docs(linux): use absolute service CLI launcher * fix(serve): validate in-process serve options before startup (#17683) * fix(linux): stop offering updates a distro-managed install cannot apply (#17918) Closes #17702. The resources/package-type marker is authoritative but never checked against the host, so any repackager that unpacks Orca's .deb -- AUR, Nix, a container rebuild -- inherits `deb` verbatim. Install feasibility was then computed after a ~165 MB download, so those users got check -> download -> a card promising an install command -> a dead end. Validate the marker against the host: a deb/rpm marker with no matching package manager in the trusted directories means a package manager owns this install. This reuses the exact lists and resolver that buildLinuxPackageInstallCommand already loops over, so a false positive is impossible by construction -- any host flagged here would have failed with no-package-manager after the download anyway. The gate only moves that verdict earlier. Verified across Debian 12, Ubuntu 24.04, Arch, Fedora 40 and openSUSE Leap: no false positive on a real deb host, correct on every repackaging host. The release is still reported, because the user does want to know 1.4.194 exists and to update through their distro; only the download path is closed. `externallyManaged` is an additive optional field on the existing `available` status, so older paired clients decode it unchanged. downloadUpdate() refuses authoritatively, since main owns this verdict rather than the card, and unwinds any pinned-build state first -- a Linux pinned jump resolves to 'release', and stranding isPinnedBuildActive would silently kill every background check for the rest of the process. Note the fix the issue suggests cannot work: electron-updater builds a PacmanUpdater whose doDownloadUpdate looks for a .pacman asset Orca does not publish, then dereferences undefined. * style(cli): restore prettier wrapping on install error copy * test(linux): re-pin the child-process ratchets and the batch-shim allowlist after the merge	2026-09-02 03:08:01 -07:00
Brennan BensonandMerge Sim	585b4086d3	test(codex): pin Codex read-repair with a real-binary contract check (#17300) ... * test(codex): pin Codex read-repair with a real-binary contract check Orca's session index-heal depends on a Codex behavior: a `thread/read` of an unindexed rollout performs a read-repair that inserts the `threads` row. All 55 existing heal tests drive a stub app-server and assert "healed" as "the call did not error", so if Codex ever dropped the repair they would all stay green while the subsystem went silently inert. Adds a real-binary contract check built to the same shape as the Git binary compatibility contract (src/shared/git-binary-compatibility.test.ts): env-gated test file, version asserted against the binary, dedicated path-filtered PR job. Pins only the four arms ablation established Orca relies on: - a read of an unindexed rollout inserts the state row - a session with no read inserts nothing (the negative control that makes the insert causal rather than incidental) - re-reading an indexed thread inserts nothing - an archived thread stays archived rather than being resurrected Written against codex-cli 0.150.1. The job sets ORCA_CODEX_CONTRACT_REQUIRED=1 so a missing or failed CLI install fails red instead of silently skipping. Existing heal tests are unchanged. * test(codex): register the contract job in the verify aggregate contract `pr-workflow-parallelism.test.mjs` pins `verify.needs` exactly, so adding the job to pr.yml without updating that list failed the shard. Adds the entry, and adds a workflow contract test mirroring `git-binary-compatibility-workflow.test.mjs`: - the pinned CODEX_CLI_VERSION is the single source for both the npm install and the runtime version assertion, so the two cannot drift apart - the install prefix and the binary path the test is pointed at are the same tree - ORCA_CODEX_CONTRACT_REQUIRED=1 is set, so a failed install fails red rather than turning the job into a green no-op Removing the REQUIRED env from pr.yml reddens the new test, confirming it is live. * test(codex): make binary version guard exact and bounded * ci(codex): cover index-heal transport dependencies * test(ci): pin Codex contract dependency coverage * test(codex): align contract watchdog with child deadlines * test(codex): cover three-session contract watchdog * fix(codex): add sqlite sync-database to index-heal scope --------- Co-authored-by: Merge Sim <sim@local>	2026-08-30 14:39:46 -07:00
Brennan Benson	fd9125ea8c	feat(native-chat): Codex structured native chat restructure (#16729) ... * feat(native-chat): port structured Codex sessions from restructure-recovery Rebuilds the desktop structured native-chat implementation from brennanb2025/native-chat-restructure-recovery (tip 4e31c08db3) on top of current main as a single commit, scoped to the local Codex path. Ported: - Structured agent-session core: durable record store + single-writer lease, canonical journal, agent-session wire host/attach/eviction/subscribers, `agentSession.*` RPC surface (registered via ALL_RPC_METHODS; host-side mobile allowlist included for wire compat), pty write gate, transcript additions, and the Codex app-server adapter/launch resolution. - Renderer: NativeChatStructuredSession view/composer stack, structured launch path with the single-flight guard, local structured session tabs sync, activation gate + structured inventory (read-only `agentSession.handoffStatus` probe), agent-session tabs in the tab strip, AI-vault structured session activation, and the settings pane with the parent Experimental Chat UI toggle plus the nested "Use updated structured native chat" toggle. New sessions require both flags, agent codex, no prompt, and a local non-WSL, non-Windows-host execution host (structured-native-chat-availability). - Fixes 72c013cea6 (verified Codex launch recovery), 8ddbaf5e3d (defer native terminal view switching affordances), and 4e31c08db3 (release the launch gate after a visibility retry) with their regression tests, including the third-launch-after-retry guard case. - Cross-version agent-session wire test + CI lane, packaging entries (proper-lockfile, agent-tooling asar excludes), and the wire-compat doc section. Deliberately not ported: mobile/ changes, the Claude structured runtime (only the claude-transcript-branch-proof and claude-structured-owner-identity leaf modules remain, backing the kept TUI-recovery arms), the terminal↔chat adoption/handoff flow (`agentSession.adoptTerminal`/`requestHandoff`, the handoff request engine, TUI adoption machinery, orca-runtime adoption methods), renderer switching affordances and their dead leftovers, the hook/subagent-status refactor cluster, and unrelated branch changes. The crash-during-acquisition recovery path (restart handoff adjudication, restore/reverse re-acquire, lease schema handoff keys) is kept because every plain direct launch depends on it; a trimmed handoff coordinator exposes only status/restore/close. Branch edits that targeted files main has since split (ipc/pty.ts, worktrees.ts, rpc/methods/terminal.ts, useIpcEvents, pty-connection, store/slices/terminals.ts, runtime-types, web preload) were re-applied to the split modules, preserving main's newer logic (Windows CIM fallback, browser tab close rework, cold-restore resume flow, dispatcher threading). Known seam: the mobile clipboard image-provenance CONSUMER gate ships (agentSession.send refuses unproven mobile image refs with agent_session_image_untrusted) but the producer hunk in rpc/methods/clipboard.ts stays with the unported mobile cluster, so mobile image sends into structured chat fail closed until that side ports. * fix(native-chat): trust only authenticated local image uploads * fix(build): preserve Windows process-tree patch application * test(windows): include process creation time in addon fixture * fix(build): run windows-process-tree node-gyp from the physical package dir gyp expands the node-addon-api dependency by probing node, whose cwd resolves to the package's physical directory in the store, so the emitted target is a store-relative ../../../../node-addon-api@... hop. gyp then resolves that hop against the rebuild cwd; from the node_modules symlink/junction it escapes the store and configure fails with "node_addon_api.gyp not found" (run 32999886072). Rebuild from realpath(package dir) so both bases agree, matching how the package manager itself runs native install scripts. The regression test replays gyp's expansion+resolution against the planned cwd and fails without the fix. * fix(native-chat): keep chat tabs visible through terminal closes and empty-worktree launches Two proven blockers in the native Codex tab contract: closeTerminalTab pre-empted the canonical unified close. With one terminal left it deactivated the worktree on a terminal/editor/browser-only check, blanking a workspace that still held a renderable agent-session tab; with two or more it pre-picked a successor from terminal entities only, re-stamping the group active before closeUnifiedTab's MRU/neighbor repair could land on the chat tab. Successor choice now defers to the unified contract whenever the terminal has a unified row, and deactivation is gated on the unified renderable count (matching leaveWorktreeIfEmpty), with the legacy pre-pick kept only for terminals without a unified row. A structured session created on an empty worktree was published into the host's headless group while preserveLocalLayout froze the local layout, leaving the tab in store but permanently off screen. A preserveLocalLayout owner now always takes client-owned placement — repairing a rendered leaf whose group record is missing, or materializing a rendered group on a truly empty worktree — and applies the client-derived layout repair while still rejecting host-authored layout. Regression tests drive the real store through closeTerminalTab (git worktree and folder workspace) and the real snapshot applier for the empty-worktree adoption states; all fail without the fixes. * fix(native-chat): close stale turns and retry rejected sends * fix(native-chat): retire hosted rows on structured tab activation * fix(native-chat): preserve rpc defaults across main merge * chore: format remote wire compatibility guide * test(native-chat): cover retry after unconfirmed send * fix(native-chat): reload outbox on session switch * docs(settings): disclose structured chat platform limits * fix(native-chat): await Codex launch-home preparation * fix(codex): align child-process allowlist with async trust bridge * test(identity): update inventory for tab surface refactor * fix(windows): preserve process-tree CRLF patch sources * fix(native-chat): anchor an unmatched chat echo where it was sent (#16117) * fix(native-chat): anchor an unmatched chat echo where it was sent The reported symptom was old user messages replaying below every new turn, so the conversation read as scrambled. The cause was not that the echo failed to match a transcript row. Claude consumes a mid-turn send through a `queued_command` attachment and writes no `type:"user"` record for it, so some echoes can never match, and no amount of matching will change that. The cause was WHERE an unmatched echo rendered: buildMobileNativeChatTransientData appended every pending item after the entire transcript, so it re-read below each turn that landed afterwards. Render each echo directly after the transcript row it was sent against, using the baseline the send already captures. An unmatched echo is then at worst a duplicate in the right position rather than a scrambled one, and it stays visible. Echoes sharing an anchor keep send order; a send with no baseline, or one whose anchor folding dropped, still falls back to the tail. Deliberately NOT fixed by deleting the echo. Inferring from send ordering that an echo can never match, then removing it, loses the user's own text for a message the agent did receive, and it cannot fire in the common case anyway - measured drain groups are 1,017 of size 1 against 55 larger. It also escalates an existing gap: the count pass has no baseline-tail guard, unlike the glue pass, while `messages` is a 40-row window that head-trims, resets on reconnect and grows at the front on loadEarlier, so a false landing there would license deleting a DIFFERENT outstanding message. That count-pass gap is real and left for a separate change; anchoring makes its worst case a duplicate in place rather than a scrambled conversation. * fix(native-chat): preserve folded echo anchors * fix(native-chat): preserve forward-folded echo anchors * fix(native-chat): keep leading folded echoes in place * fix(workspace-cleanup): show git status for every row (#16690) * fix(native-chat): refuse structured chat on every Windows execution path canUseStructuredNativeChat only refused win32 when a project runtime resolved, so folder-workspace keys (and other keys with no project runtime) failed open into structured chat on Windows. Fail closed on win32 unconditionally after the host check, matching the settings copy: local macOS/Linux only; Windows/WSL/SSH stay on terminal chat. * fix(native-chat): restore runtime refusals behind the win32 gate 506d375de3 replaced the project-runtime checks with a bare platform test, so a WSL or repair-required runtime resolution would no longer refuse structured chat off-win32. Keep the unconditional win32 refusal and re-run the runtime resolution after it, so the gate does not depend on the resolver's own platform guard. Tests inject WSL and repair-required resolutions on darwin/linux and fail against the regressed gate. * fix structured session journal durability * fix structured tab active pointer after restart * fix(native-chat): await optional lease renewal callbacks * refactor(skills): extract install error messages * fix(agent-session): harden recovery ownership * fix(native-chat): retain panes across tab activation * fix(native-chat): address round-one review findings * test(native-chat): align integration coverage after main merge * fix(native-chat): harden round-two reliability * fix(native-chat): harden round-three reliability * fix(native-chat): close round-four recovery gaps * fix(native-chat): separate bounded journal key forms * fix(native-chat): reset outbox error in render on session switch The switch effect adjusted error state after the sessionId prop changed, tripping react-doctor's no-adjust-state-on-prop-change on the changed-code gate and flashing the old session's banner for a frame. Reset it with the render-time previous-value guard instead. * fix(native-chat): invalidate stale outbox settlements * test(native-chat): restore settled-error session-switch regression a6e2379bd1 replaced this test with the in-flight settlement race test, leaving the render-time error reset unpinned: deleting the reset block still passed the whole native-chat suite. Keep both scenarios pinned; they are distinct (settled error clears on switch vs stale settlement invalidated in the commit-to-passive window). * test(wire): make release checkouts race safe * test(wire): pin cross-process checkout single-flight and importer specifier contract * test(wire): harden release checkout lifecycle * fix(build): drop CR-byte residue from windows-process-tree patch The two trailing CR bytes on the patch's deletion lines are a proven no-op: pnpm hashes patches CRLF-normalized (both forms hash to the lockfile's 946ffb2b) and materializes this package without applying the patch in either form, so the load-bearing build edits come solely from applyWindowsProcessTreeBuildFixes() (#16947), which handles both source EOL forms. Restore byte-identity with main and repin the contract test to the post-#16947 reality: LF-only patch bytes plus lockfile hash sync. * fix(native-chat): skip empty startup recovery	2026-08-28 16:45:58 -07:00
Neil	ebe6b559ef	Prime changed native caches before test fanout (#16918)	2026-08-27 18:34:03 -07:00
Neil	350423b7cb	Speed up PR CI with path skips, native caches, and fewer shards (#16863) ... * Speed up PR CI with per-job path skips and native caches Skip git-compat, xterm, packaging, and shell jobs when their inputs are unchanged, reuse the composite install action (including Windows node-pty cache), skip compiling the Windows CLI launcher on a cache hit, and cut the test matrix from 16x2 to 8x2 shards without dropping coverage. * Widen PR job skip prefixes for orcad browser and live shells Chrome session/tab modules and zsh/fish wrapper templates are inputs to required jobs the classifier previously skipped. Include that implementation graph so those jobs still run when the files they load change. * Fix CI cache safety and required gates * Build scriptless Windows addons explicitly * Preserve node-pty Windows support prebuild * Remove duplicated Windows launcher unit lane	2026-08-27 16:09:07 -07:00
Neil	9d06b3ba93	ci: stop docs-only commits from starting the skill-roundtrip matrix (#15474) ... A cancelled Skill update round trip on a README merge painted main red because push to main had no path filter. Share the PR path list on push, and skip expensive PR Checks when every changed file is docs.	2026-08-19 23:33:05 -07:00