mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
Neil
5c2d3322c1
* fix(runtime): name a terminal whose pane the graph dropped `buildPtyTerminalSummary` decided `orphaned` from the PTY record's agreement with itself — `!pty.tabId || !pane || pane.tabId !== pty.tabId`. A record whose `paneKey` still parses to its own `tabId` passes that forever, including long after the session graph dropped the pane, so a terminal that had lost its surface reported `orphaned: false, connected: true, writable: true` and a `tabId` no tab has: field-for-field identical to a healthy one (#18191). Consult the leaf topology instead, gated on a graph statement having had the standing to contradict the record. `graphSequence` counts authoritative graph statements; every statement re-records the surface of every pane it publishes, so a pane the current graph holds carries the current stamp and is answered without touching the leaf map. That covers the two absences that are not evidence, without a second flag: a surface recorded since the last statement (spawn records the pane before the graph carrying it arrives, #7587), and a lost graph clearing every leaf at once without advancing the sequence. A pane already observed dropped keeps its stale stamp and stays named, because losing the ability to re-check is not a reason to un-see it. `orphaned: true` is shipped vocabulary that both consumers already read, so no capability gate is needed: adoption keys on it (`hasStrongOrphanIdentity`) and now reaches this population, and the duplicate-surface index (`indexLiveTerminalSurfaceOwners`) stops recording a destroyed pane as a PTY's live owner. * fix(runtime): publish a terminal retirement proof on the exit's own evidence A paired client may drop a mirrored terminal on exactly two kinds of host evidence: a `retiredTerminalSurfaces` proof naming the handle, or two authoritative `terminal.list` inventories that omit it. The second needs two host publications, and a quiet workspace publishes one, so the proof is the only evidence that rides the frame carrying the retraction. That proof was minted only as a byproduct of persistence *accepting a change*, which made one value carry two meanings: "a change was accepted" and "the PTY exited". The host renderer's close transaction de-persists the surface and republishes without it, so when it got there first the exit found nothing left to accept and the attestation died with it. Measured on a real paired client: the host retracted in under 500ms, published no proof, then froze its snapshotVersion for 60s while the client kept a dead pane in its tab bar. Persistence still gates *removal* — publishing absence before the membership fence is durable would let a crash resurrect the surface. It no longer gates the proof: the observed exit is itself the attestation. The exit-first ordering already had a passing test; the renderer-first ordering had none, and that is the one users hit. Both orderings are now pinned, with exit-first as the control that makes the renderer-first failures mean something. Wire: `retiredTerminalSurfaces` is an existing optional field on an existing path, already negotiated as `session-tabs.retirement-proof-delta.v1`. This is Rule 1 — an old client that ignores it degrades to the two-inventory route it already uses today, so no capability gate is needed. The sentence "the host starts sending a frame it did not send before" reads like Rule 3; it is not, because the frame shape, the field, and the reader contract are all unchanged. * test(runtime): pin the removal frame retiring a still-live publisher KNOWN RED (`it.fails`), no product change. Found while verifying the close retraction fix: once the emptying actually reaches paired clients — a state the previous behaviour never allowed, because nothing propagated — re-adoption of a later create is flaky. Measured 1 failure in 6 runs of the two-client journey. `decideWebSessionTabsSnapshot` treats the host's synthetic `removed:<t>` retraction as a publisher handover: it retires the still-live renderer epoch and installs the retraction as current, while the removal also clears the live freshness record. The next frame from that same running publisher then matches no lineage and reads as a retired generation, so it is outranked and the publisher is locked out of the worktree until its generation changes. `local-structured-session-tabs-sync/snapshot-apply.ts` documents this exact scenario and has a revive escape; the mirror path has none. The suffix case explains the 1-in-6: `hasRetiredValue` is an exact string match, so a republication carrying `:headless-merge:` walks past the fence and only a bare same-epoch republication is locked out. Not fixed here on purpose. Dropping the retirement makes the red case pass but breaks `web-session-tabs-sync.test.ts > keeps a removed worktree fenced against delayed predecessor epochs`, which asserts a same-epoch higher-version frame after a removal must be rejected. At this layer those are the same frame — this function holds no `receivedFrame`, so it cannot separate a delayed predecessor from the live publisher speaking again. The fix belongs in `shouldApplyRecoveredWebSessionTabsSnapshot`, which does hold that ordering and currently defers to the same epoch fence. That is a contract change across two functions and an existing invariant, not a one-liner. * fix(runtime): a removal retraction is not a publisher handover The host drops a worktree's entry when its last tab closes and announces it with a synthetic `removed:<t>` epoch. Both receipt sites treated that as a publication: `decideWebSessionTabsSnapshot` and `recordReceivedWebSessionTabsSnapshot` each noted the retraction epoch as current, which pushed the still-live renderer epoch onto `retired`. The removal also drops the live freshness record, so the next frame from that same running publisher matched no lineage, read as a retired generation, and was outranked. The live publisher was locked out of its own worktree until its generation changed. That is fail-closed, and it is why re-adoption after an emptying was flaky once the emptying actually reached paired clients. A retraction and the live publisher's next frame are the same epoch at a higher version, so epoch identity cannot separate them and never could. Delivery order can. `recordReceivedWebSessionTabsRemoval` now records the retraction as the worktree's newest received evidence instead of deleting the ledger, so `shouldApplyRecoveredWebSessionTabsSnapshot` — the gate every production apply path passes before `decideWebSessionTabsSnapshot` — fences a frame that reserved its received frame before the retraction while admitting one that arrives after it. The boundary carries the retraction's own epoch, which never matches a host publication, so a later live frame may still restart its version counter. `local-structured-session-tabs-sync/snapshot-apply.ts` documents the same conclusion for the local path: a retired epoch is not proof of a dead generation. `keeps a removed worktree fenced against delayed predecessor epochs` pinned the delayed predecessor at the raw decision layer, which is the same call as the live publisher's republication. It now pins the identical scenario — same epoch, higher version, still rejected — through the receive-and-apply path that actually holds the ordering, plus the composed gate as production spells it. The committed `it.fails` repro is not sufficient on its own: it records no received frame, so dropping only the `decideWebSessionTabsSnapshot` retirement turns it green while the publisher stays locked out on every real path. A receive-and-apply case is added alongside it to close that gap. * test(runtime): pin the retraction boundary against a stale inventory omission Mutation testing left a survivor: writing the boundary unconditionally, instead of only when it advances the ledger, passed the whole runtime suite. It is not inert. A visibility-resume inventory reserves its received frame before it lists, so an omission it reports can be older than a stream frame that landed meanwhile; without the guard that stale omission rewinds the ledger, forgetting the stream frame's version, and a delayed list reserved in between is then readmitted instead of outranked. This pins that ordering. The one remaining survivor is the boundary's `snapshotVersion`, and it is inert: the ledger's version is read at exactly two sites, both reachable only when the incoming frame's epoch equals the stored one, and a retraction epoch never equals a live publication. * test(runtime): cover the fences the retraction change narrowed Two gaps found by mutating the fences themselves rather than the fix. Deleting the epoch fence in `shouldApplyRecoveredWebSessionTabsSnapshot` passed the entire runtime suite. It is not unreachable: a superseded generation whose sibling stream delivers its frame after the handover outranks the successor on delivery order, and only the retired-epoch check rejects it. Retractions used to exercise that fence too; now that they no longer retire anything, a genuine handover is the only thing left that reaches it, and nothing covered that. The fence is narrower than it was, not dead. The second case pins rate-independence. The defect surfaced 1 run in 6 because `hasRetiredValue` is an exact string match while `sameSessionTabsPublicationLineage` treats `:headless-merge:` as the same publisher, so a merged republication walked past a fence a bare one hit. The removal path is now asserted over both epoch shapes through the full path, so a fix that only re-rated the defect instead of removing it would fail here. * fix(runtime): give "same publisher" one answer across the epoch fences Separable from the retraction fix beneath it, and it changes handover-path behaviour: a superseded generation that republishes under a merged epoch is now rejected where it was previously accepted. Take it independently or not at all. `publisher-identity-fences.ts` held two answers to "is this the same publisher". `noteRetiredValue` treated a `:headless-merge:` epoch as a SUCCESSOR of its base and retired the base when the merged form became current, while `sameSessionTabsPublicationLineage` treated the two as ONE publisher. Those are contradictory, and the retired-value check's exact-string match was the shim that kept them from ever meeting: a merged frame was a different string, so it never looked retired no matter what had been retired. The cost was that the same predecessor was accepted or rejected depending on which shape it arrived in. A generation a successor had replaced was fenced when it republished bare and admitted when it republished merged — the fail-open half of the same disagreement whose fail-closed half was the removal defect, and the reason that defect reproduced 1 run in 6 rather than every time. This cannot be fixed in the fence alone. Making the fence lineage-aware while a merged epoch still retires its base has the generation retire itself: the rebuild arrives, retires its own base, and the fence then rejects it as a retired generation. So both sides move together — a lineage sibling advances the current epoch instead of superseding it, and inherits its generation's retirement instead of escaping it. Scoped to the publication-epoch functions. Runtime-id retirement keeps exact matching, and `local-structured-session-tabs-sync` keeps its own `hasRetiredValue` call, where a lineage sibling is already excused explicitly and a retired epoch is deliberately not treated as proof of a dead generation. * test(e2e): journeys for a reopened client and two clients on one host Two gaps this suite had no coverage for, both driven end to end against a real paired desktop client rather than at a seam. A relaunched client holding a live remote terminal: every paired restart spec here restarts around a browser pane, none around the terminal the user is actually mid-work in. The host-side fixture's on-disk sink is the oracle — one READY for the whole run proves the host never re-spawned the session, and a recorded line for input sent after the relaunch proves the restored pane is wired to that same process rather than painted with its scrollback. Two clients on one host across an emptied workspace: the tombstone is client-local on the runtime path, so a client that never held a row still seeds into a workspace another client deliberately emptied. That asymmetry is by design; a client falling out of step with the host and staying there is not. Phase 0 is the control — without it a later divergence cannot be attributed to the emptying rather than to mirroring never having worked. The input probe goes through `pane.terminal.input`, not `window.api.pty.write`: a mirrored pane's handle is a `remote:` id that no local PTY answers to, so a direct write is swallowed and the assertion passes on nothing. The pre-restart control exists to catch exactly that, and did. * test(e2e): keep the two-client journey spec type-clean * test(e2e): pin the close retraction a paired host does not publish * docs(e2e): say why the red close-retraction spec sits on this PR The spec was written on a branch carrying neither of this PR's publish-side fixes, and its own diagnosis -- the fault is the host's publish-after-close, not any client's mirror -- names exactly what they change. Landing it here makes CI the measurement rather than leaving a red spec parked on a branch with no fix in it. Records the one thing a reader needs to not do: skip-tagging it. And why the obvious split is not a block move -- phase 2 depends on phase 1b's emptying and both share the two-client pairing fixture, so splitting means duplicating the fixture. * test(e2e): the close-retraction spec is green on this branch, measured It was written to pin a defect and was red where it was written. On this branch, with `publish a terminal retirement proof on the exit's own evidence` and `a removal retraction is not a publisher handover` both present, it passes -- twice, independently: phase1a A=9ms/B=158ms then A=2ms/B=1ms, against a prior baseline of "none reached either client within 90 seconds". So the KNOWN RED header had become the thing it warned about: a test carrying prose asserting the very behaviour the commits beside it remove. Rewritten to record the measurement and the numbers to regress against, and to keep the one instruction that still applies -- if it reddens again, do not skip-tag it; the failure shape is a 90s timeout on both clients at once while creates still propagate. No assertion changed. Comment only. * test(wire): pair the session-tabs retirement proof across two builds The stack makes a host start sending a retirement proof on its own frame when no surface removal carries one. The change argues Rule 1; Rule 3's fourth bullet covers a frame the host starts sending on an existing path, so the claim is measured against v1.4.199 rather than accepted. Neither existing cross-version suite reaches session-tabs: the terminal one covers the binary stream, the agent-session one covers agentSession.*. Result: the old client acts on the proof-only frame, because the whole client half of this surface is unchanged. The old-host cells are pinned to a release that cannot publish the frame at all, which is what makes the new-host cells mean something. * fix(lint): clear the casting gate on the surface-lost inventory main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. The retraction read narrows on the property instead of casting; the fixture and cross-build-import casts carry per-site SAFETY rationales. * fix(lint): bind the protected-stamp cast to a name The leading-semicolon parenthesised call put the suppression on a line oxfmt then reflowed away from the assertion it covers. Naming the narrowed handle keeps the directive next to the cast. * fix(runtime): route every non-null surface write through the stamped writer `ptyHoldsRecordedSurface` trusts a record only while its stamp is current; after that the leaf map answers. Four writers still named a pane with a bare `tabId = / paneKey =` — orphan adoption (both branches), split, create on an adopted stable pane, and TUI-owner recovery — so a record that had already been contradicted stayed contradicted after the claim, and `terminal list` reported the just-claimed PTY `orphaned: true` until the renderer's next graph statement re-recorded it. Before this branch those sites read as attached at once, so this was a regression window of one round-trip, and `indexLiveTerminalSurfaceOwners` reads `orphaned` as "unowned". `recordPtySurface` is now the one writer; the adoption module reaches it through a port because it has no `graphSequence` of its own. The nulling writers are untouched: a null surface is never held, stamped or not. * test(runtime): keep one copy of each publisher-fence case The removed-frame suite asserted four properties that another case in the same suite or the lineage suite already pinned: - the decide-only readmit and the bare full-path readmit are the bare arm of the parameterized full-path readmit, verbatim; - the merged-suffix decide-only readmit is the merged arm of the same loop; - "still fences a predecessor a successor replaced" is the lineage suite's bare arm with different version numbers; - the recovery-gate handover case is the lineage suite's recovery-gate case with a bare late frame instead of a merged one, so that test now runs both shapes and this copy goes. Mutation-checked: reverting each of the five renderer changes on this branch (retire-on-removal in decide, noting a retraction current, the exact-match retired fence, merge-supersedes-base, dropping the ledger on removal) still fails at least one of the remaining ten cases. Also corrects the suite header: a retraction carries a synthetic `removed:` epoch, so it is the in-flight predecessor frame, not the retraction, that shares the live publisher's epoch and needs delivery order to be separated. * test(e2e): fail the two-client journey when phase 1a cannot run Phase 1a sat inside `if (beforePartialClose.length > 1)`. A host workspace that starts with one terminal skipped the control silently while 1b and 2 still ran, and the spec passed green without ever exercising the close-with-others-open retraction it was written to measure. The skip is now a recorded failure naming the host count. * fix(runtime): order every session-tabs apply path against the retraction A closed terminal came back on the other client because "this worktree was retracted" was neither durable nor universal: - `refreshWebRuntimeSessionTabsSnapshot` reached `decide` with no place in receipt order at all, so a list the host answered before the close applied after the retraction had already cleared the worktree. It is a production path for close, create, activation, split and PTY reconnect. - the boundary lived in a single receipt slot the next stream frame overwrote, and in a fence that only existed when a recovery happened to be pending when the retraction landed, so a pre-close list could out-rank the republication on `snapshotVersion` alone. Replace both with one raise-only removal watermark per (environment, worktree) and give the list path a receipt position, reserved by the request and carried in its answer so a dedupe joiner inherits it rather than minting a newer one. The pending-recovery fence and its bookkeeping are dead once the boundary is monotonic. The exact-match retirement check in the receipt ledger becomes the one lineage-aware predicate, so a `:headless-merge:` rebuild can no longer be noted as current and retire the live publisher out of its own worktree. On the main side, `recordPtyWorktree` stamped `surfaceRecordedAtGraphSequence` at write time, so any `paneKey` write claimed the standing of a fresh graph statement. The inventory restore in `terminal list` therefore un-dropped the very pane the read was meant to report, on every listing. A surface claim now carries no graph standing unless its writer names one: the graph statement, live leaf output and spawn do, while the inventory restore, the floating liveness restore and the mobile projection replay do not. Defaulting this way means a writer that says nothing fails safe and self-corrects, which the type alone could not guarantee across the projection contract's own `recordPty`. Spawn claims now span the one graph statement the renderer may already have in flight, and retirement proofs compare by identity instead of by position, so a re-delivered exit no longer fans out a `snapshotVersion` bump carrying nothing. * fix(runtime): stop an unpublished-worktree placeholder retiring the live publisher A worktree the host has published nothing for still answers a forced list, with a synthesized `none`/v0 frame that means "ask me later" (host-session-snapshot-authority.ts). Every post-close list and every activation of an emptied worktree gets one. Noting it as a publication retired the renderer generation that is still live, and because that epoch is per-process, the terminal the user created next never reached this client — the same lockout the retraction path was already careful to avoid, through a door it did not cover. `local-structured-session-tabs-sync` already skips the placeholder for this exact reason; the web mirror now does too, on both the receipt ledger and the frame decision. Bound the receipt ledgers by frame age rather than entry count. One bootstrap inventory records a receipt per worktree under a single reserved frame, so evicting by insertion order dropped that batch's own earlier entries, and an absent receipt is what the recovery gate reads as "no evidence for this worktree". Only a receipt no in-flight frame can still be ranked against is droppable. Take the receipt gate off the `web-session-tabs-sync` barrel in the refresh path. Ordering is that path's gate, not an optional collaborator a caller's module mock may leave out, and being reachable only through the barrel is how the path came to have no ordering at all. * fix(runtime): let the TUI-owner recovery name its pane without claiming the graph holds it `recoverStructuredTuiOwner` rebinds a recovered PTY from the persisted owner binding — the same replayed-evidence class as the inventory restore — but stamped it with the current graph sequence, so a pane the renderer had already dropped read as attached for one more statement. The guard below it needs the tabId and paneKey, not the standing. Also say plainly in `decideWebSessionTabsSnapshot` what the affirms check does and does not cover: an unpublished-worktree placeholder is withheld from epoch noting only. It still applies, because rejecting it outright would drop the terminal reconciliation that legitimately rides on it. * fix(runtime): keep the retraction boundary out of the receipt bound Bounding the removal watermark alongside the receipt ledger reintroduced the defect the watermark exists to prevent: past 512 retracted worktrees, evicting a boundary readmits every pre-close frame it was fencing, and a delayed list resurrects the closed tab. A boundary is not a cache. One number per worktree ever retracted on an environment is the cheaper price, and environment teardown drains it; only the receipt ledger stays bounded, by frame age. Split the orphan-adoption port by provenance so the last writer that disagreed with the surface-standing rule stops disagreeing. `adoptRuntimeTerminalOrphans` replays the persisted binding when the claim already matches it and writes a new one otherwise, and both went through a single `recordSurface` that stamped the current graph sequence — so re-adopting an already-adopted orphan lifted a dropped pane's stale stamp and reported it attached, in a quiet workspace possibly forever. The replay now names the pane without standing and the fresh claim takes spawn standing, like every other writer. Replace a receipt-count assertion that was vacuous for a map keyed by environment and worktree with the mirror state and freshness it was standing in for. * fix(runtime): keep a closed-tab worktree under the epoch already publishing it `closeHeadlessMobileTerminalTab` minted `headless:<now>` on every close. Its sibling headless writers carry the stored `publicationEpoch` forward and mint only when there is no snapshot to inherit from — because a write to a worktree is not a claim to publish it. The close was the one writer that claimed. A paired client retires the epoch a new publisher displaces, and the web mirror's retirement is final: there is no revive lane, and the per-worktree tracking teardown deliberately keeps the epoch history. So an ordinary close published a stranger for a worktree the renderer generation still owned, retired that generation on every client, and the renderer's next publication — carrying the epoch the close had just retired — was rejected forever. The user emptied a workspace, created a terminal, and it never arrived on either machine while `session.tabs.list` showed the host holding it. This is the same thesis the retraction path already states, through the door next to it: a retraction is not a handover, and neither is a close. Measured on `paired-two-client-emptied-workspace-reseed.spec.ts`, six runs each: phase 2 failed 3/6 before (`A=null B=null`, both clients blind for the full 30s budget) and 0/6 after, with both clients adopting in single-digit milliseconds. * fix(lint): give the fixtures real types instead of casting past them The casting gate failed on eight assertions this branch added. All eight were suppressible, but the suppressions were not the problem: the casts were hiding fixtures that did not match the contracts they stood in for. `sessionStillHoldingBothPanes` built tabs as `{id, title, type}` — `type` is not a `TerminalTab` field and eight required ones were missing — and layouts holding only `ptyIdsByLeafId`. `as never` made both compile. They are now real `TerminalTab` / `TerminalLayoutSnapshot` values, so the fixture is checked against the type `listTerminals` actually reads. `terminalTab` in the epoch suite built a *client* tab (`status`, `terminal`) for a field typed with *snapshot* tabs, which forced `as never` at the call and a cast on the snapshot itself. Production reads only `type`, `parentTabId`, `leafId`, `ptyId` and `parentLayout` from that tab, so the two client-only fields were inert; dropping them lets the declared `RuntimeMobileSessionTerminalTab` type the fixture end to end, and the closed tab is now held by name rather than recovered from `snapshot.tabs[0]`. The remaining three casts are unchanged in kind and now carry correctly placed SAFETY rationales: reaching a protected member is the only way to drive these paths. `graphSequence` folds into the reach-through that was already there rather than opening a second one, and the map read narrows instead of asserting. Mutation-tested, all three suites, regression re-introduced for each: - epoch mint on close restored -> 1 failed | 1 passed - orphan check reverted to self-consistency -> 4 failed | 4 passed - placeholder retirement guard removed -> 1 failed | 7 passed src/main/runtime 8169 passed | 31 skipped; src/renderer/src/runtime 1581 passed. `check:code-quality:changed` goes 8 findings -> 0. `pnpm tc` clean. * fix(runtime): stop the headless placeholder graph from dropping every restored pane A headless server publishes one empty graph at launch so status clients see a ready server. It names no renderer pane and is never replaced, but it was counted as an authoritative graph statement all the same: `graphSequence` went 0 -> 1 while the leaf map stayed empty for the life of the process. Every surface claim written without standing - a persisted replay, an inventory restore, the TUI-owner recovery - is stamped 0. Against `graphSequence` 1 the `>=` guard fails, the empty leaf map answers "no pane holds this", and the terminal reports `orphaned: true` under a `pty:` tabId. Nothing can re-stamp it, because the only graph that host will ever publish has already been published. On a headless or SSH host that is permanent, and it is the same lie #18191 is about, pointed the other way. The placeholder no longer spends a graph statement. A renderer graph still does, so a pane a real graph drops is still reported dropped - including on a desktop window promoted from headless, which the third case pins as a negative control. Mutation: restoring the unconditional bump fails the first two cases ("expected 1 to be +0", "expected true to be false"); the promoted-window control passes either way, as a control should. Also registers tests/e2e/cross-version-wire/cross-version-session-tabs-retirement-proof.unit.test.ts in the cross-version-wire job. The file matches CROSS_VERSION_WIRE_PREFIXES, so adding it had switched the job's gate on, but the job runs an explicit file list that omitted it - the test executed nowhere in CI. It passes 8/8.