mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
ce26cd38576dbca21f800d934db5f80fa6eb0938
8516
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Neil
|
66bd8dd450 |
merge: paired-close-retirement-proof-publication (ed38308333)
|
||
|
Neil
|
df4e7a0c29 |
merge: adv3-failure-fixes (a5dfddf6fa)
# Conflicts: # src/renderer/src/lib/host-mirror-handle-gap-wait.ts # src/renderer/src/runtime/host-session-mirror-hydration.ts |
||
|
Neil
|
8bd642a9f3 |
merge: adv2-persistence-fixes (3962c1b4d1)
|
||
|
Neil
|
19d5128678 |
merge: adv2-crossplat-fixes (b2873d0bae)
|
||
|
Neil
|
90620adbfc |
merge: relay-lifecycle-fences (d4f7700062)
# Conflicts: # src/main/runtime/relay/desktop-relay-service.ts |
||
|
Neil
|
ed38308333 |
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. |
||
|
Neil
|
982d539df8 |
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. |
||
|
Neil
|
d4f7700062 |
docs(relay): address the teardown ratchet's docstring to whoever it goes red on
The reader who decides this test's fate is not the reviewer — it is someone hitting a red ratchet during an unrelated refactor, weighing whether to relax the pattern. Say the trade to them directly: a ratchet that fails loudly on a benign change beats one that passes silently on a harmful one, and the literal match is deliberate because a refactor that obscures which teardown runs at quit is exactly what should get a second look. |
||
|
Neil
|
762e1faa80 |
test(relay): ratchet which teardown each lifecycle event calls
Closes the gap the cross-reference comments could not: a reader arriving at stop() or fenceAndCloseNow() now finds the other, but nothing failed if someone edited main-process-quit.ts back to the fence, and that is the edit that reintroduces the defect. No harness needed — this reads the source text, like the other ratchets in src/main. Both methods take an optional argument and return void, so swapping them compiles clean and passes every unit test; the difference is that stop() is terminal and the fence is re-armable, and it only surfaces in timer state minutes later. This is the compile error that swap cannot produce. Both directions are asserted, because collapsing them either way destroys the distinction the defect came from: quit must not regain the re-armable fence, and sign-out/relaunch must not lose it. The regexes are qualified by receiver on purpose. The quit handler stops five unrelated services, so a bare `.stop()` would match rateLimits, starNag and the rest. Verified by mutation, both directions and a control: reverting quit to fenceAndCloseNow fails the first test only, collapsing sign-out onto stop() fails the second only, and renaming an unrelated service's stop() fires neither. |
||
|
Neil
|
ae4c130422 |
docs(relay): cross-reference the terminal stop() and the re-armable fence
main-process-quit.ts has no test harness — single export, ~20 heavy imports including electron — so the quit call site's choice between these two is pinned only by the service-level contract. The risk is not that the contract is untested; it is that someone editing the quit handler has no way to find it. Each method now names the other, says which lifecycle events call it, and says what the other one is for. Grep from either end lands on the counterpart, which is the only protection available when one side cannot be tested. |
||
|
Neil
|
9a8cc8c96e |
docs(runtime): the reused-tab-id class is closed at read time, not still open
The `ExpiredHandleGapVerdict` docstring told the next reader that a retracted tab id republished under the same id still inherits its predecessor's verdict, and that closing it "needs a fourth trigger, on row retraction". The test it names as its own pin says the opposite: class D in host-mirror-handle-gap-verdict-union.test.ts asserts the verdict does not answer, and explains it is closed at READ time rather than by any prune. Provenance, since two sources disagreeing is what made this expensive: the paragraph was last written in |
||
|
Neil
|
83369d0ffc |
test(relay): pin RelayControlClient teardown before, and twice after, connect
Characterisation only — no defect found, which is the result worth recording, because the same audit found the service-level fence was not hard. Covers closeNow() at 'idle' before any socket exists, connect() refused after teardown, a control request issued before connect, and closeNow() twice from both 'opening' and 'active'. Asserts the specific fields rather than absence of errors: socket handle dropped so the second pass cannot re-terminate, onClose not re-fired, pendingRequestCount back to 0, and vi.getTimerCount() 0 so neither the connect deadline nor the silence watchdog outlives the close. Mutation-verified: dropping the idle guard in connect(), `this.socket = null`, requests.rejectAll, or silenceWatchdog.stop() each fails one of these. |
||
|
Neil
|
ed35db892b |
fix(relay): stop a mid-read profile switch reporting as a sign-out
Same taxonomy error |
||
|
Neil
|
f1eb8037c8 |
fix(relay): drain the revoke outbox on one broker, not one open per item
flushAll runs unawaited from inside the coordinator's openBroker, so the broker it is draining is not registered yet. Refreshing demand after every removal therefore reconciled against a null ownership: each reconcile opened a fresh director assignment and abandoned the broker still working through the loop. Measured with three queued revocations and a real RelayRevokeOutbox on disk: 3 items produced 3 RelaySessionBroker.connect calls and 2 abandoned brokers, against a relay that rate-limits assignment. After: 1 connect, 3 revokes, one `connecting` transition, the draining broker never closed, outbox empty. Reconcile once after the loop instead, and only when something was actually removed. flushItem keeps its own refresh because onDeviceRevokeQueued calls it directly on an already-registered broker, where retiring the last demand is exactly what should reach standby. Tests use the durable file as the assertion rather than a mock, and cover the fence landing before the open resolves, the fence landing between two items, the multi-item drain, the last-demand revoke reaching standby, and the same reqId queued twice (dedup is the control layer's job — duplicate_relay_request_id — so the service must only settle both without stranding the item). Mutation note: moving onDrained one await later, inside the loop after flushItem returns, survives. The extra microtask lets the coordinator register the broker first, so no extra open occurs and the variant is genuinely inert here; the assertion does catch the real pre-fix placement, synchronous inside the revoke after remove. |
||
|
Neil
|
0671e9eca8 |
fix(runtime): a published handle retires the verdict it answered
The fourth eviction trigger on `expiredGenerationByPane`, and the reason it is not redundant with the three already there or with the two other agents' guards on this same map. A verdict records that a pane's 15s handle-gap wait ran out. Nothing retires it when that pane subsequently publishes its handle, so the NEXT gap on that pane gets no wait at all — #19735 with the bounded wait removed rather than merely shortened. Measured on the reconciled union tree ( |
||
|
Neil
|
0fa72f336a |
fix(runtime): an outage is not a handle-gap verdict
The per-pane handle-gap wait releases at a 15s deadline and records that
expiry as a verdict, which authorises the sleeping-agent resume. The
connection generation was the only thing voiding that verdict, and a plain
disconnect never advances it — runtime-status.ts advances on the reconnect,
under a new runtime id. So a network drop mid-turn expired the wait with a
generation that still matched, and the replay forked a second `--resume`
onto the transcript the host was still writing: #19735 through the
disconnect door.
Suppress the verdict while the client positively knows it is out of contact,
reusing the shared runtime-host connection derivation. The waiter still
releases and re-parks, so contact returning gets a full fresh budget and the
pane is still decided on real silence.
Not redundant with the landed-handle drain that follows this commit, nor with
the read-time pane identity from adv2-skew (
|
||
|
Neil
|
4733b21cdc |
fix(relay): make the quit fence hard, and route quit to the terminal stop()
DesktopRelayService.stop() has zero production callers. Quit, relaunch and
pre-sign-out all go through fenceAndCloseNow() (main-process-quit.ts,
main-window-core-services.ts). That matters because the two are not
equivalent: stop() sets `stopped` and clears both timers, while the fence
cleared only `livenessTimer`, left `demandExpiryTimer` armed, and left
`stopped` false. Its comment — "the next auth mutation re-arms via
refreshDemand" — is right for sign-out and exactly wrong for quit. The path
production takes was the one that permitted a resurrection, and the suite was
exercising the other one, which is why this survived.
Three doors re-armed the fence, each measured against a fresh service:
- a pending invite-expiry wake: getTimerCount() is 1 immediately after the
fence, and the timer fires refreshDemand and reopens the broker
- a mint settling after the fence, through withTransientDemand's finally:
0 timers at the fence, 1 once the mint settles, and connect runs again
- a power-resume ensureLive(), gated only on `stopped`: connect called twice,
a new relay control session opened after quit
Two intents, kept apart rather than unified. fenceAndCloseNow gains a `fenced`
latch cleared only by start()/authMutated(), so sign-out and relaunch keep the
re-arm the old comment protects while the dangerous window between the
pre-sign-out fence and the profile wipe stays shut. Quit calls stop() instead,
which is terminal; the wire behaviour is unchanged, since coordinator.stop()
fences reasonlessly exactly as before. The outbox flush gets the same latch,
because it runs unawaited from inside the broker open and a fence can land
between two revokes.
Deliberate on a vetoed quit: Relay stays down for the session. before-quit
already nulls the mobile pairing provider unconditionally, so pairing is dead
there regardless — a re-armed broker would be a zombie holding a control
session nothing can use.
Not fixed here: `fenced` is not set by stop(), so the two latches stay
independent and a future caller can still pick the wrong one.
|
||
|
Neil
|
cd175054c5 |
refactor(relay): extract the revoke-outbox flush into its own module
Behaviour-preserving move, no logic change: flushRevokeOutbox and flushRevoke leave DesktopRelayService as RelayRevokeOutboxFlusher.flushAll/flushItem. The two fixes that follow each add lines to this file, and it has no headroom under the 300-line cap; a max-lines disable is not an option. Why a new file rather than reusing something: nothing existing drains the outbox. RelayRevokeOutbox is the durable store — making it drain itself over a control would give the persistence layer a dependency on a broker. RelayControlRequests dedups in-flight reqIds on a single socket, not queued items across reconnects. RelayDemandLedger reads the outbox for demand and never mutates it. The drain only ever existed inline here, so this is an extraction, not a parallel implementation; the only new code is the options type and the class shell. |
||
|
Neil
|
1b621b6b13 |
docs(runtime): the two guards on the park-time binding are not redundant
Recording a reconciliation result that existed only in a review thread, and
correcting it in the process — measuring the claim changed it.
The claim under review was that the `?? ''` fallback in `recordExpiredWait` is
unreachable by two independent guards, either sufficient alone: the caller's
generation gate (a missing waiter fails `undefined === number`) and the
record-before-release ordering. That is not what the code does.
Measured, by removing each in turn:
- ordering removed, generation gate kept: the gate does NOT carry it. With the
waiter already deleted, the gate is false on every expiry, so nothing is ever
recorded — five failures, and the door is shut by breaking the mechanism rather
than by refusing ''.
- generation gate removed, ordering kept: 736 files green, one failure, and it is
`does not let a wait armed on the previous connection decide the new one` in
host-mirror-handle-gap-resume.test.ts — a different property entirely.
So the ordering alone makes `''` unreachable, and the generation gate is not a second
guard on it at all: it pins reconnect-void. Both are load-bearing, for different
reasons, which is a stronger argument against removing either than redundancy would
have been — redundancy invites deleting one.
Worth writing in the file because the two sit three lines apart and read as belt and
braces on the same thing. The `''` comment next to them already exists because an
unexplained guard on an unreachable value gets deleted as dead code in a year; a guard
that looks redundant is deleted sooner.
No behaviour change. One comment, corrected against measurement rather than against the
thread it came from.
|
||
|
Neil
|
a5dfddf6fa |
test(runtime): supply the host home the SSH orphan-cleanup gate now requires
The scenario registers an SSH filesystem provider directly. In production that provider is minted by the relay session that also reports the host's `$HOME`, so the test has to supply the other half rather than rely on the recursive-delete gate falling open on an unknown home. |
||
|
Neil
|
a3512fcd69 |
fix(runtime): stop the epoch-history cap evicting a fence that can still be beaten
The LRU cap added in
|
||
|
Neil
|
cdafc90d8f |
fix(runtime): a handle-gap verdict answers for its pane, not for the tab id
Folds adv2-skew's
|
||
|
Neil
|
20fb0b3f6e |
perf(runtime): index session-tabs tracking keys by environment, and bound the epoch history
Two defects that only appear at scale or over time, in one commit because the fix for the second calls into the structure the first introduces — see the coupling note at the end. 1. THE PER-ENVIRONMENT TEARDOWN SWEEP WAS QUADRATIC IN ENVIRONMENT COUNT. `clearWebSessionTabsTrackingForEnvironment` prefix-scanned every key of eight module maps, each holding E*W entries, plus one global walk — so a catalog update clearing E environments cost 8*E^2*W + E*W. It runs on every pairing-revision change. Measured with a counting probe on the real function (exact key visits, no clock, so a loaded machine cannot contaminate it): E=10 W=50 500/map 17,000 visits -> 500 (34x) E=20 W=50 1000/map 58,740 visits -> 1,000 (59x) E=40 W=50 2000/map 202,220 visits -> 2,000 (101x) E=50 W=20 1000/map 141,600 visits -> 1,000 (142x) Doubling E at fixed W: 17,000 -> 58,740 -> 202,220, i.e. 3.45x and 3.44x per doubling — quadratic, not linear-but-large. The indexed column over the same sweep is exactly 2x per doubling. Doubling W at fixed E is 1.84x and 1.78x, linear as expected. These are freshly-populated fixtures with nothing stale in them, so the quadratic term is a property of the scan itself and not of a leak. Wall clock for cross-reference only: 1.92ms -> 0.64ms at 20x50, 37.1ms -> 4.8ms at 100x50. The fix is the index shape this file already uses for `hostSessionTabMappingKeysByEnvironmentAndWorktree` — that precedent is why this is worth indexing rather than tolerating. The regression assertion is STRUCTURAL, not a timing threshold. A wall-clock ratio bound was tried first and flaked at 22.3x against a <20x limit under suite load, which is exactly the failure mode a timing assertion has on this machine. It now spies on `.keys()` for the nine per-worktree maps and asserts teardown never enumerates any of them — enumerating them IS the sweep, so it pins the property directly and is load-independent. Mutation-checked: restoring the prefix sweep kills it. 2. `sessionTabsPublicationEpochHistoryByWorktree` WAS UNBOUNDED IN KEY COUNT. Only its inner retired array was capped (at 8); the map itself grew 1:1 with worktree lifecycles, since the entry is deliberately retained as a tombstone fence after the live record is dropped. 10,000 create/remove cycles left 10,000 entries. Now LRU-capped at 512, re-inserting on every accepted frame so a still-publishing worktree is never the eviction victim — the tombstones, which are what actually accumulate, are evicted first. WHY ONE COMMIT: the eviction added by (2) calls `releaseSessionTabsEnvironmentKeyedWorktree`, the index added by (1), because an evicted tombstone must also release its index entry. Split, the first commit would leak the index on eviction or the second would not build. The coupling is real rather than incidental, so they land together. Verification: 191 files / 1611 tests pass, log grepped for ELIFECYCLE and failures = 0. pnpm tc and oxlint clean. |
||
|
Neil
|
46ad377ceb |
fix(test): repair two suites the union broke, neither visible on its own branch
Both were green on their own branch and failed only once the branches were combined. This is the interaction class the union exists to find. 1. relay-concurrency-policy-flip-mid-mint.test.ts stubbed `getRelayRevokeOutbox` with `pendingFor` alone. `hasDemand` now reads `demandingFor` (the revoke-demand expiry), so the stub threw INSIDE reconcile, the mint never settled, and the only symptom was a 30s test timeout with the real cause buried in stderr. A stub that implements part of an interface fails this way whenever the production caller moves to another method. 2. host-mirror-handle-gap-teardown.test.ts REPLACED `tabsByWorktree` on every park, so each pane unpublished the one before it. With the tab-death rule in the same loop those earlier verdicts were legitimately swept before teardown was reached, and the suite counted 2 where it expected 3. The fixture, not the rule, was wrong: this suite is about the class no recording-driven prune can reach, so its panes must all stay published. Rows now accumulate. |
||
|
Neil
|
794089794a |
fix(relay): keep the diagnostic the refusal path exists to produce
Two error paths that destroy their own evidence.
`parseHandshakeMessage`'s unknown-type refusal interpolated `String(t)` on a
peer-supplied value: `{"type":{"toString":1}}` makes String() throw "Cannot
convert object to primitive value", so the refusal arrives without naming what
was refused. `describeRelayProtocolVersion` guards this exact hazard two files
away; the sibling was missed.
`runRelayOrcaCliChannel`'s new `onDecodeError` wrote to stderr and then exited
synchronously. stderr is async on a pipe transport, so the one line recording
why the command died could be dropped — the reason relay-handshake.ts already
exits inside its write callback.
|
||
|
Neil
|
2051f16008 |
fix(mobile): a relay status we could not read is not "offline"
Two problems in the same newly-extracted collector, both in the artifact a user pastes into a bug report. `.catch(() => 'offline' as const)` reported the definite neighbour for a lookup that observed nothing, so a support engineer could not tell a host that reported offline from one that never answered. It now reports `unreadable`. And the collector could reject: `window.api.mobile.getRelayStatus()` throws synchronously when the bridge has no `mobile` — before `.catch` is attached — while both call sites fire this as `void copyRelayDiagnostics()` with the await sitting AHEAD of their try/catch. Measured: the click wrote no clipboard and showed no toast at all, where the pre-extraction inline payload build could not fail. The collector is total now, and the await moved inside the try so a future addition cannot silently kill the toast again. |
||
|
Neil
|
25096b39d8 |
fix(relay): the demand wake signal must not fail the grant it wakes for
`withTransientDemand` called `refreshDemand()` bare on both sides of the operation. It reaches the device registry (`nextPendingExpiry` -> `listDevices`) and the settings store (`hasDemand` -> `isRelayAllowedForDevice`), so it can fail on its own — and measured, each call site failed the operation instead: - the pre-call threw with the transient ref already acquired, so the ref was never released and the operation never ran. Transient refs have no expiry, so that ref holds relay demand for the rest of the process. - the teardown call replaced the operation's own result in both directions: a named mint failure arrived as `listDevices exploded`, and a SUCCESSFUL mint arrived as a rejection. Both are wake signals; the liveness tick and the next refresh re-ask, so a lost signal is recoverable where a stranded ref is not. The containment lives beside the ledger that owns the ref, because desktop-relay-service.ts is at its max-lines ceiling and this must not cost it a line. Also carries the original error as the `cause` of the mid-operation policy flip rewrite, which was discarding it — the one place in a commit about naming causes that destroyed one. |
||
|
Neil
|
94ba0b9f05 |
docs(relay): say plainly that the protocol negotiation is not reachable yet
Two claims that read as live and are not. `relay-protocol-version.ts` documents a negotiation that no live path can reach, and reads as the fix for #13852. It is not. The deploy path namespaces the relay directory by the CLIENT'S OWN build hash — `remoteInstallDirName` is `relay-<fullVersion>` — and the daemon socket lives inside it; the short-socket fallback derives its segment from the same directory, and every `runConnectHandshake` caller takes its path from that one deploy result. So a bridge can only ever meet a daemon of its own build, `msg.version === launchVersion` short-circuits, and `relayProtocolOfferAdmits` never decides anything. The stranded incumbent the feature exists for sits in `relay-<otherVersion>/`, which nothing dials. What ships is three log lines; `MIN_RELAY_PROTOCOL_VERSION` and the `minProtocolVersion` wire field have no live reader. Keeping it is right — the mechanism is correct and hostile-input-safe (20 malformed and hostile offers refused against a real daemon, which stayed up and still admitted a valid cross-build offer afterwards), and it is the part that has to exist first. What was missing is the statement of what else has to land with it, which is now written where someone editing this file will see it: route the bridge to the incumbent's socket, and stop `validateGrant` refusing on `serverBuildId`. That second one matters because its premise, "client and relay ship in one build", is still TRUE today and becomes false the instant the first lands — it is a second gate that would refuse what the handshake just admitted, and shipping only the routing change would look like a regression in the negotiation rather than a missed dependency. `lingerMs` on the coordinator contract reads as a policy knob. No production path sets it: the only `new RelayAuthCoordinator` is in `desktop-relay-service.ts` and passes neither it nor `random`, so every shipped linger is the hardcoded ten-minute default. Labelled a test seam so the next person tunes the default, which is the only thing a user can feel. No behaviour change. Both found by a dead-code and unread-field sweep over the stack. |
||
|
Neil
|
9ff0af8b43 |
test(session): make three data-loss test names match what their bodies check
Each of these named a guarantee it did not check. A test called "never replaces the host snapshot with an empty list" buys more confidence than it earned, which is worse than no test on a path where the failure is deletion. `ssh-host-partition-session-export` — WEAK, and now strengthened. The assertion was `not.toEqual([])`, which `undefined` satisfies. Under `replace-session` the exported projection is authoritative in whole, so an omitted path key destroys the host's copy exactly as an empty list does; the test named both failures and caught one. Measured A/B: with the export mutated to never write the path key, the old assertion PASSES and the new one (`Object.hasOwn` plus the exact id list) FAILS. `remote-workspace-session-merge-local-survival` — INERT, renamed. It was called "drops a tab closed locally rather than resurrecting it from the snapshot", but the tab is not dropped — the host still lists it, the body's own comment concedes it, and the only assertion is that every merged id came from one of the two inputs. Disabling `isSuppressedByClose` entirely leaves every test in the file green; the mechanism is covered by `remote-workspace-session-merge-close-tombstones.test.ts`. Renamed to the real invariant, which is worth keeping, with a pointer to where the close behaviour is actually pinned. No new assertion, because duplicating that file would be the second mistake. `workspace-session-ssh-partition-round-trip` — WEAK, renamed, and this one was actively misleading. "does not adopt a stale populated ssh row over a tombstone in the owning partition" describes a guarantee the code deliberately does NOT provide: the populated row would sit in the legacy `local` partition, and there `workspacesTheBaseOwns` (keyed on `tabs.length > 0`) makes the workspace un-adoptable, so the legacy tabs win and the tombstone is dropped. That trade is deliberate — a resurrected tab is recoverable, a deleted one is not — and is pinned in workspace-session-stranded-partition-adoption-tab- rows.test.ts. No fixture under the old name could have caught the case it named, so the name was a standing promise against the exact data loss the trade accepts. Found by a test-name-versus-body sweep; the first two verdicts were each confirmed by a mutation that survived, with a control mutation that did not. |
||
|
Neil
|
0b1b0eec36 |
fix(runtime): key the wake-respawn latch per environment, like its twin
The wake-respawn latch is the initial-terminal bootstrap latch's twin — both stop one focus from issuing two creates for a workspace, both are consulted from the same subscription closure, and they sit one line apart in the same teardown. The bootstrap latch got two fixes this stack; this one got neither, and still carried both defects: 1. NOT KEYED BY ENVIRONMENT. It was a bare `Set<worktreeId>`. A worktree id is `repoId::path` with no host component, so the same id can be live on two paired runtimes at once (STA-4343) — which is exactly why the bootstrap latch was re-keyed. Keyed by worktree alone, one runtime's respawn suppressed the other runtime's, and one runtime's `end` released the other's claim. 2. A CLEAR-EVERYTHING INSIDE A PER-ENVIRONMENT TEARDOWN. `clearWebSessionTabsTrackingForEnvironment` called `clearAllWebRuntimeWakeTerminalRespawn()`, so tearing one environment down freed every other environment's in-flight claim and a fresh closure for the sibling could issue a second respawn while the first create was still running. That is STA-6173, and the bootstrap latch's own docstring names it: "clearing every environment's latch would release a sibling environment's pending create and let a new subscription for it seed a duplicate, which is this very bug through another door." The correctly-scoped bootstrap clear is the very next line. Found by reading the teardown function as a list rather than as prose — a clear-everything is a one-line call that looks identical to a scoped one at a glance, which is how it sat next to the fix for its own defect. Both latches now have the same shape: `Map<environmentId, Set<worktreeId>>`, a per-worktree release, and a per-environment clear that touches only its own keys. The existing wake-respawn test is threaded with an environment id rather than rewritten: its contract was correct, only the signature moved. The new file pins the two defects themselves. Also documents `clearWebSessionTabsTrackingForEnvironment` as what it has become — the per-environment teardown registry. It now carries seven module clears, and a new per-environment map belongs in that list rather than behind a trigger of its own. The doc says each clear must be scoped to THIS environment and names the wake-respawn latch as the one that was not, because the next clear-everything will look just as harmless. Mutations: reverting the per-environment clear to a clear-all kills exactly the sibling-claim assertion; making the skip check ignore the environment (the old worktree-only keying) kills exactly the two-environments assertion and the sibling-claim one. No survivors. |
||
|
Neil
|
416b35732e |
fix(runtime): reconcile three branches' handle-gap verdict rules into one loop
Three agents changed `recordExpiredWait` on three branches and each verified only their own. This is the union, resolved into the agreed shape and proved on one tree. The rules are NOT alternatives — they have different safety properties, and flattening them to one scope is wrong in both directions. Both wrong shapes were independently written before this was reconciled, so the comments say why. GENERATION rule, per key across EVERY environment (adv2-skew's class). `hasHostMirrorHandleWaitExpired` compares a row against its own environment's CURRENT generation, so a row whose generation has moved can never return true for anybody; retiring it cannot cost a reader a verdict, whoever owns it. Scoped to the recording environment, an environment that reconnects and then goes quiet strands its rows forever. TAB-DEATH rule, recording environment ONLY (my class). Row absence is transient where a generation is not: a sibling mid-republish has no rows for a frame and would lose a verdict its pane still needs — reproduced before it was narrowed. Teardown drain (adv2-races' class) is unchanged and orthogonal: it is the only trigger that fires for a REMOVED environment, whose rows no rule above reaches because such an environment records no further verdict. Right predicate, wrong trigger. The union suite proves all four orphan classes simultaneously, plus the two properties none of the three rules may break: the verdict stays sticky enough to break the park/expire/replay loop, and no rule evicts a verdict a live pane still needs. It uses three environments throughout, because with two at one generation the candidate rules are indistinguishable and the naive fix survives. THE FOURTH CLASS IS UNOWNED AND ASSERTED AS A HAZARD. A retracted tab id that is republished inherits the old pane's verdict and skips its own wait. Unlike every other gap on this map it is NOT conservative: the others drop a verdict and re-park, holding longer, while this one retains a verdict and resumes on a handle that has not landed — the #19735 direction. No rule reaches it: the tab-death predicate stops matching once the id is republished, the teardown drain fires on environment teardown rather than tab retraction, and no waiter exists to observe the retraction because a pane holding a verdict never parks. Closing it needs a fourth trigger, on row retraction. The suite pins the current behaviour so it cannot be quietly forgotten. Union finding, recorded rather than merged: adv2-skew's `docs(relay): the live-broker wait budget does not bound the call` ( |
||
|
Neil
|
1bb7489a92 |
fix(relay): a throwing retry step must not kill the retry chain
`RelayRetrySchedule` ran the caller's recovery step bare inside its timer and
resolved `armed` only on the line after it. Injected a synchronous throw:
the error escaped the timer as an uncaughtException, `armed.resolve()` never
ran, and the schedule was left with no armed timer — so every waiter on
`settled` parked on a promise nothing would ever settle and nothing re-entered
the chain.
The reachable trigger is the origin pool's drain recovery, which calls
`onStatus('draining')` straight through to `webContents.send`. `state.mainWindow`
is nulled only on `'closed'`, so between destroy and that event the send throws
`Object has been destroyed` — every other `webContents.send` under
`src/main/startup/` already guards with `isDestroyed()`. Guarded here too, so
the trigger is removed as well as contained.
|
||
|
Neil
|
dd5f4376f7 |
test(runtime): carry the bootstrap-latch retention suite onto the union
Teardown semantics here are adv2-concurrency-fixes': an in-flight create survives teardown as `creating-after-teardown`. The suite seeds PARKED claims rather than in-flight ones so it pins the unambiguous half and passes against both shapes. Accessors added to their module rather than a second spelling of them. |
||
|
Neil
|
cbf18d46d4 |
fix(runtime): key the wake-respawn latch per environment, like its twin
The wake-respawn latch is the initial-terminal bootstrap latch's twin — both stop one focus from issuing two creates for a workspace, both are consulted from the same subscription closure, and they sit one line apart in the same teardown. The bootstrap latch got two fixes this stack; this one got neither, and still carried both defects: 1. NOT KEYED BY ENVIRONMENT. It was a bare `Set<worktreeId>`. A worktree id is `repoId::path` with no host component, so the same id can be live on two paired runtimes at once (STA-4343) — which is exactly why the bootstrap latch was re-keyed. Keyed by worktree alone, one runtime's respawn suppressed the other runtime's, and one runtime's `end` released the other's claim. 2. A CLEAR-EVERYTHING INSIDE A PER-ENVIRONMENT TEARDOWN. `clearWebSessionTabsTrackingForEnvironment` called `clearAllWebRuntimeWakeTerminalRespawn()`, so tearing one environment down freed every other environment's in-flight claim and a fresh closure for the sibling could issue a second respawn while the first create was still running. That is STA-6173, and the bootstrap latch's own docstring names it: "clearing every environment's latch would release a sibling environment's pending create and let a new subscription for it seed a duplicate, which is this very bug through another door." The correctly-scoped bootstrap clear is the very next line. Found by reading the teardown function as a list rather than as prose — a clear-everything is a one-line call that looks identical to a scoped one at a glance, which is how it sat next to the fix for its own defect. Both latches now have the same shape: `Map<environmentId, Set<worktreeId>>`, a per-worktree release, and a per-environment clear that touches only its own keys. The existing wake-respawn test is threaded with an environment id rather than rewritten: its contract was correct, only the signature moved. The new file pins the two defects themselves. Also documents `clearWebSessionTabsTrackingForEnvironment` as what it has become — the per-environment teardown registry. It now carries seven module clears, and a new per-environment map belongs in that list rather than behind a trigger of its own. The doc says each clear must be scoped to THIS environment and names the wake-respawn latch as the one that was not, because the next clear-everything will look just as harmless. Mutations: reverting the per-environment clear to a clear-all kills exactly the sibling-claim assertion; making the skip check ignore the environment (the old worktree-only keying) kills exactly the two-environments assertion and the sibling-claim one. No survivors. |
||
|
Neil
|
ae77dea3f3 |
fix(relay): stop a revoke that can never land from pinning the relay online
Reproduced first: enqueue a revoke, reject it 50 times the way a deleted device
is rejected, and `hasDemand` is still true. It stays true for the life of the
install.
Two deliberate properties combine into the bug. `flushRevoke` removes the outbox
item only on a SUCCESSFUL flush and swallows every failure in a bare `catch {}`
with no attempt cap and no expiry, so a permanently-rejected revoke is pending
forever. And the revoke arm of `hasDemand` is deliberately NOT filtered by the
host's pairing-connection policy, because a credential must be killed on the relay
whatever the user has since chosen. Forever-pending plus policy-exempt means the
relay is held online permanently and a `local-only` pick is defeated outright —
the regression path for what #19870 closes, previously with no test at all.
THE CHOICE, stated because the convenient one is wrong. The revoke is NOT
abandoned. Failing to revoke leaves a live credential on the relay, so discarding
the intent to tidy up demand would trade a visible stuck relay for an invisible
live credential. The item stays in the outbox and keeps retrying on every future
connection, unchanged.
What expires is only its CLAIM ON DEMAND, via `demandingFor` alongside the
unchanged `pendingFor`: after 24h an unlanded revoke stops being a reason to force
the relay online BY ITSELF. The reasoning is that holding the relay open is not
what makes the revoke succeed — it has had a day of attempts — so holding it open
buys nothing and costs the user the setting they chose. A revoke that can still
land is unaffected inside the window, and one that lands is removed as before.
Uses the existing `createdAt`, so nothing new is persisted and the window survives
restart.
Mutation-tested: putting `pendingFor` back in `hasDemand` fails the new test at
the post-window assertion. The success path is pinned separately so the fix cannot
be mistaken for aging out an item that should have been removed.
Audited the rest of the file for the same shape, since a bare `catch {}` on a
queue is rarely alone: `flushRevoke` is the only one in desktop-relay-service.ts.
|
||
|
Neil
|
6bd997b2cc |
fix(relay): clear the demand-expiry timeout on a fence, not only in stop()
Why the existing lifecycle test missed this: it stubs
`demandLedger: { nextPendingExpiry: () => null }`, so the second timer this class
owns is never armed and the fence is only ever asserted against the liveness
interval. Change that one stub to a future expiry and the test fails.
`fenceAndCloseNow` cleared `livenessTimer` and left `demandExpiryTimer` armed.
`stop()` cleared both — but `stop()` has no production caller, while all three
fence sites (quit, sign-out, SIGNED_OUT) go through the path that cleared only one.
The consequence is the sharp part. The orphaned timeout fires `refreshDemand`,
which reconciles *and* re-installs the liveness interval the fence just tore down.
A fence during an outstanding pairing invite therefore undid itself minutes later
— the post-fence resurrection the comment two lines above forbids in so many words.
Both teardown paths now share one `disarmTimers()`. That also resolves the
`max-lines` pressure the extra clause created: the two teardown blocks were byte
identical, so deduplicating them is the right fix rather than a `max-lines`
disable, which this repo forbids and which the next person under that cap will be
tempted by.
The new test asserts `vi.getTimerCount() === 0` after the fence rather than
inferring from an absence of errors, and pins that a fence is not a stop — the
next auth mutation still re-arms both timers.
Pre-existing, not introduced by this stack; verified identical at the base SHA.
Mutation-tested: removing the `demandExpiryTimer` clause from `disarmTimers`
fails the new test at the timer-count assertion.
Also carries a note at `waitForLiveBrokerResult`: the `pending !== latestReconcile`
arm re-enters the loop without re-checking the deadline, so under continuous
reconcile churn the documented 20s budget is not strictly enforced and the
caller's transient demand ref is held past it. Not a hang — every iteration awaits
a settling promise — so it is bounded by how long churn lasts.
|
||
|
Neil
|
1095e360c0 |
fix(runtime): release the bootstrap latch when the post-create read throws
The `tabsByWorktree` row read that decides between release and park sat outside the try. Measured with `useAppStore.getState` throwing once after a successful create: the dispatch rejects with the latch still in `creating`, and `releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame` only ever clears `awaiting-mirror` — so no mirror frame can rescue it and every later dispatch for that workspace returns false without creating, until environment teardown. |
||
|
Neil
|
58fac71ba0 |
fix(runtime): a landed handle retires the handle-gap timeout verdict
`expiredGenerationByPane` was cleared only by a later expiry on the same environment with a different connection generation. The module's escape hatch was "a reconnect bumps the connection generation and arms a fresh wait" — and the #19647 change in this same stack stops recording `status: null` for an unreachable host, so `connectionChanged` no longer fires across an outage on the same runtime. Measured: park, let the deadline fire, land the handle, republish rows ahead of handles on the same generation — `findUnhydratedHostMirrorForPane` returned null and the sweep resumed immediately with zero panes parked. That is #19735 with the bounded wait removed entirely rather than merely shortened. A published handle is positive host evidence and ends the gap episode the deadline was about, so it now retires the verdict. The store subscription outlives the waiter for exactly as long as an expiry needs watching. Also carries the worktree across a re-park: adopting an orphaned terminal re-keys `tabsByWorktree` without re-keying the record, so a live wait kept releasing on retraction evidence about the workspace it was no longer about. And contains the same throwing-replay shape in the sibling drain (`host-session-mirror-hydration`), which runs from the frame-apply path. |
||
|
Neil
|
e0bc16a8a1 |
test(session): pin which partition's terminal row survives adoption
workspace-session-stranded-partition-adoption.ts had no test anywhere in src/ or tests/, and its result is fed straight into a wholesale `replace-session` upload by persistedSessionForTarget — so whichever row wins here becomes the host's whole answer for that workspace, and every other client reads it. That is a lot of consequence resting on behaviour nothing records. Six cases, covering both directions the module can be asked about, the contested carve-out, and the no-host-partition short circuit. Two things are written down rather than left to be rediscovered: The known trade. `workspacesTheBaseOwns` keys on `tabs.length > 0`, which makes the direction the module argues for work (an empty base must not block adoption) and makes its mirror image unreachable (an empty HOST row cannot retract a populated base row). During the upgrade window a pre-partition build's rows sit in `local` while this build writes the owning `ssh:<targetId>` partition, so an emptied workspace's tombstone loses to the legacy row and the publish re-offers tabs the user closed. Bounded — a clean quit's `api.set` clears the legacy row. Deliberately not changed: letting the owning partition's empty row win trades a recoverable resurrection for a possible deletion, which is the direction this module's header argues against and the one that let `replace-session` delete the host's copy (#12721). The inert first draft. The contested case was first written with a populated base row and passed — for the wrong reason. A populated row makes the workspace un-adoptable before the contested check is consulted, so mutating that check to `if (true)` left it green. Rewritten with an empty base row, which is the only state where the check is load-bearing, the mutation now fails it. The comment keeps the warning, because "the assertion holds" and "the assertion could have failed" are different claims and reading only the first is how a data-loss bug survives review. |
||
|
Neil
|
9b74b61d21 |
fix(mobile): a scope refusal is not a missing method on the pairing probes
A phone decides whether a desktop can serve Relay by calling pairing.getEndpoints and pairing.provisionRelay and reading the failure. Both call sites treated only `method_not_found` as "this desktop is too old for Relay, stay on LAN" — and that code is the one answer a genuinely old desktop can never give. runtime-rpc-websocket-dispatch.ts runs the mobile allowlist gate BEFORE the RPC dispatcher. So for a mobile-scoped device `method_not_found` is reachable only for a method that IS allowlisted but unregistered; a method an older desktop predates is absent from both lists and the phone is refused with `forbidden`. The fallback could never fire against the exact desktop it exists for. In pre-profile-pairing-coordinator.ts that is the first-time QR pairing flow, so the failure is not a degraded mode — it throws and the phone ends up with no host profile at all: AssertionError: promise rejected "Error: forbidden: forbidden" instead of resolving ❯ requireSuccess src/transport/pre-profile-pairing-coordinator.ts:288 Both local `isMethodNotFound` predicates are replaced by one shared `isPairingRelayRpcUnavailable` that accepts either code. Safe in both directions: a desktop that serves the probes allowlists them, so `forbidden` on one can only mean "this desktop will not expose Relay pairing to a phone", which is exactly what the caller falls back for. See docs/reference/remote-wire-compatibility.md. The desktop-side test pins the mechanism rather than restating it: a mobile-scoped device gets `forbidden` for an unregistered method while a runtime-scoped peer gets `method_not_found` for the same one. Measured: mobile `vitest run src/transport` 98 files / 723 passed. Mutating away the `forbidden` arm fails both new cases; mutating away the `method_not_found` arm fails both pre-existing cases; mutating the desktop gate to emit `method_not_found` fails the new desktop case. No survivors. |
||
|
Neil
|
81dd2fe590 |
fix(worktrees): an unknown host home refuses the recursive delete
`resolveWorktreeRemovalHome` answers `{ kind: 'executionHost', homePath: null }`
whenever the relay session has left `activeSessions` or never resolved its host
env — an ordinary disconnect, and the default resolver before the registration
side effect runs at all. `resolveGuardHomePath` then correctly declines to
substitute the client's home, but `isHomeDirectoryRemovalPath` fell through to
path SHAPES only, and shapes do not know `/srv/homes/alice`,
`/export/home/alice` or `D:\Profiles\bob`.
Injected: the host-home fixture with the resolver answering null. Measured:
`removeRuntimeUnregisteredWorktree` called `deletePath('/srv/homes/alice', true)`
— a recursive delete of the host's own home, the exact path the two shipped
tests pin as refused when the resolver does answer.
"Could not ask the host where its home is" is unverifiable, so the two
recursive-delete gates now fail closed on it. `isDangerousWorktreeRemovalPath`
deliberately does not consult it: it also fences the registered
`git worktree remove` path, which must stay usable mid-reconnect.
The stated cost is pinned too: an ordinary orphan is also declined while the
home is unknown. Declining is recoverable — the row survives and the next
connected removal proceeds — and a recursive delete of the wrong directory is
not.
|
||
|
Neil
|
9de4e9dd21 |
fix(runtime): drain a removed environment's handle-gap verdicts on teardown
`expiredGenerationByPane` is pruned only by rules that run when a verdict is RECORDED — the stale-generation sweep here, and the tab-death sweep added separately ( |
||
|
Neil
|
aa98edf35a |
fix(runtime): contain a parked resume replay that throws
The handle-gap park releases its waiters from inside `useAppStore.subscribe`, and the replay it runs is `resumeSleepingAgentSessionsForWorktree` — a large synchronous sweep. Zustand notifies listeners in a plain loop, so a replay that threw escaped the `setState` that triggered it: measured, the exception left the store write, every listener registered after this module missed that write, and the sibling pane the same mirror frame made due was never released. The waiter's timer, map entry and subscription are already torn down before `run`, so containing the throw holds nothing back — it only stops one pane's failed recovery from taking the store notification and its siblings with it. |
||
|
Neil
|
c3d46edfea |
test(relay): pin the mid-mint policy flip under interleaving, not just in sequence
The mid-mint LAN-flip fix (
|
||
|
Neil
|
193119c18d |
fix(relay): scope nextPendingExpiry the way hasDemand is scoped
`hasDemand` requires four things of a standing binding — mobile scope, matching
owner identity, matching relay host, and allowed by the live pairing policy.
`nextPendingExpiry` applied NONE of them: it took the minimum `inviteExpiresAt`
across every device in the registry. So it armed the `demandExpiryTimer` wake in
`refreshDemand` off state that provably cannot produce demand.
Three cases measured returning a wake time where `hasDemand` was already false:
a binding for a different relayHostId, a runtime-scope (non-mobile) device, and
a phone the live LAN policy excludes.
Low severity on its own — the fired timer just reconciles to no demand — but it
is spurious wakeup churn from the class that owns the correct predicate three
lines above, which is the kind of drift that stops being harmless later. The
shared clause is now `demandCandidateBinding`; the owner check stays in
`hasDemand` because `nextPendingExpiry` takes no identity. The `hasDemand` side
is a pure conjunction reorder, confirmed behaviour-preserving by mutation rather
than by eye.
Re-arming after a policy exclusion is safe: `pairingPolicyChanged()` calls
`refreshDemand()`, so a flip back to automatic restores the timer. That round
trip is asserted rather than assumed.
Also records two things next to the code that would otherwise be lost:
- Transient refs carry NO OWNER IDENTITY, so `hasDemand`'s transient loop answers
true for any signed-in identity while the two branches below it filter on
`ownerIdentityKey`. Nothing defends the current behaviour OR that regression:
scoping the loop by owner fails exactly one test across the whole relay suite,
the characterisation test added for it. Deliberately not fixed here, and the
comment says why the in-file version is unsafe —
`withTransientDemand('provision')` calls `setMobileRelayBinding` INSIDE the
operation, so during a re-pair the device still holds the old owner's binding
and an inferred filter would drop demand mid-provision, tearing the broker down
under the operation holding the ref. The real fix threads identity through
`acquireTransient`, whose call site is desktop-relay-service.ts.
- The `if (!current) return` guard in the release closure is unreachable today
and mutating it away breaks nothing. Kept and labelled INERT rather than
deleted: it goes load-bearing the moment the ledger grows a dispose()/clear(),
and nothing in the suite would catch that.
The ref-counting core itself came back clean under every hazard exercised:
policy flip between acquire and release (the release closure is policy-blind by
construction, and the demand filter is a live pull per call rather than an
acquire-time snapshot, so neither a permanent pin nor a dropped-but-needed
demand); double release; opposite-order release of two refs on one key; and
cross-device release.
|
||
|
Neil
|
1673716c6d |
fix(relay): bound the live-broker wait over a chain of superseding reconciles
The budget bounded the armed-retry chain only. A waiter that kept following fresh reconciles — which the previous two commits made it correctly do — rode that chain with no deadline check at all. MEASURED, on a fake clock: budget 1,000 ms, reconciles landing every 200 ms, opens taking 400 ms and rejecting. Pre-fix the waiter NEVER SETTLED within a 300,000 ms horizon. Post-fix it settles at exactly 1,000 ms, the budget the caller asked for. Reachable in production because `withTransientDemand` reconciles on BOTH acquire and release, so a busy host supersedes a waiter's reconcile faster than opens settle, and the caller rides the chain indefinitely while holding its demand ref. The deadline check is gated on `joinedReconcile` so the contract that matters is preserved: the one open the waiter ARRIVED on is still never cut short, because cutting a slow-but-succeeding open short would fail a pairing that was about to work. The existing "slow first open past the budget still wins" test passes unchanged, which is the assertion that proves the gate. This is the third defect in this loop and all three share one root cause: `await pending` had no escape once the reconcile the waiter captured stopped being the one that mattered. |
||
|
Neil
|
6cfc1f2a4a |
fix(relay): release a live-broker waiter parked on an open that stop() abandons
`stop()` fences and closes, but did not wake waiters. A waiter parked on an open that never settles therefore had nothing left to release it — the wait is deliberately unbounded across the open it joined, and the coordinator it was waiting on is gone. Measured: the promise stayed pending for the life of the process. This is promise-shaped, not timer-shaped, which is why it survived the leak checks: `vi.getTimerCount()` reads zero at teardown and always did. The new test asserts on the settled value, and keeps the timer-count assertion so the two are not confused again. `fenceAndCloseNow` now fires the same authority-change signal a fresh reconcile does, which is all a parked waiter needs to re-read the world and return the settled cause. One line, because the wake mechanism it reuses landed in the previous commit. The sibling case — a waiter parked on an ARMED RETRY when the coordinator stops — was already clean; pinned in the same file so the distinction between the two teardown positions stays visible. |
||
|
Neil
|
8904488d3b |
fix(relay): wake a live-broker waiter when a newer reconcile takes the authority
A waiter joined `latestReconcile` and awaited it unbounded, on the reasoning that a reconcile always settles. True, but incomplete: once that reconcile is superseded its result is DISCARDED, so the waiter sits out an open nobody will use — measurably still parked after a newer reconcile had already registered a live broker. The wait now races the joined reconcile against an authority-change signal that `beginReconcile` fires, so a waiter follows the reconcile that actually matters instead of the one it happened to arrive on. The open the waiter arrived on is still never cut short. WORTH KNOWING FOR ANYONE TOUCHING THIS LOOP: the generation check that guards this — `if (pending !== source.reconcile()) continue` — was completely unprotected. Deleting it survived ALL 226 existing relay tests. It is the thing that keeps a waiter from answering out of a reconcile that no longer speaks for the coordinator, and nothing in the suite noticed its removal. The new test "follows a superseding reconcile that is still opening rather than answering from the old one" pins it, and is the only test that fails when it is deleted. The loop moved to relay-live-broker-wait.ts because the fix pushed relay-auth-coordinator.ts past the 300-line cap and AGENTS.md forbids disabling max-lines. The source is a record of getters rather than a snapshot because the wait re-reads all of it after each await. Also pins three interleavings that came back CLEAN, so the scope of "clean" is on the record: two waiters on one armed schedule (the short-budget waiter neither cancels the retry nor strands the long one); a retry firing on the exact tick the budget expires (the armed retry takes the tie deterministically and the waiter returns that attempt's cause, not a null one); and a terminal cause landing while a retryable wait is armed. |
||
|
Neil
|
6f514e4022 |
fix(runtime): isolate one worktree's replay from the mirror-hydration drain
The same fan-out hazard as the handle-gap drain, one module up. Settling an environment drains every worktree parked on it in a single loop, called from the frame apply, with `waiter.run()` unguarded. One replay that throws strands every waiter queued behind it and surfaces in the caller applying the frame. Found by looking for the sibling of a defect rather than by a separate interleaving: both modules park a `run` callback and drain N of them from one event, so both have the same blast radius. Kept as its own commit because the two modules route independently. Mutation: dropping the guard kills exactly the one new assertion. |
||
|
Neil
|
9a81146e13 |
test(runtime): pin sleeping-agent resume on a failed SSH target
The terminal-state floor in workspace-terminal-host-authority.ts has three consumers: initial-terminal seeding, the startup terminal watcher, and sleeping-agent resume. Seeding is covered end to end by worktree-agent-activation-seam.test.ts. Resume was covered only at the predicate, so nothing failed if the floor stopped reaching it — and the floor's own comment says the cost of losing it is a failed target left terminal-less with unresumable agents for the rest of the app session. Pins the resume half directly: an SSH git worktree on a target whose sync terminated in offline/error with an empty hydrated set resumes its sleeping agent. Two controls keep the floor from widening into "resume whenever we are unsure" — an in-flight 'pulling' sync and no sync status at all both stay unverifiable and resume nothing. Verified by mutation: emptying TERMINATED_WITHOUT_ANSWER_PHASES fails exactly the two floor assertions and leaves both controls passing. Routes independently of the two fixes on this branch: the floor predates this stack (#16750), and this only closes a coverage gap in it. |