mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
2741bdad38
* fix(session): persist defaultTerminalTabsAppliedByWorktreeId Host persist snapshots wrote tabs but omitted this write-once map, and full session replaces / hydration treated omission as "never applied". Union the marker across persist and hydrate so default terminals are not re-spawned on every launch or re-attach. Fixes #18117 * fix(runtime): stop re-seeding a runtime-owned workspace the user emptied Focusing a workspace owned by a remote runtime created a terminal every time, and sometimes two. The mirror could never record the closed-last-terminal state. A host snapshot with no terminals produced `nextTerminalTabs === null`, which `withWorktreeEntry` turns into a deleted key -- and a missing row is exactly how every seeder spells "never initialized" (initial-terminal.ts). Keep an explicit empty row instead, so the remote path reads the same tombstone the local one already honours. A worktree that never had a terminal still gets no row, because `sameTerminalTabs` treats a missing row and an empty one as equal; removal frames and synthesized unpublished frames keep deleting, since neither is evidence the user emptied anything. The duplicate had a second cause. `requestedInitialTerminal` was a `let` inside the session-tabs subscription closure, so "one focus creates at most one terminal" held only for as long as that closure lived. Its effect re-runs whenever the environment, connection generation, pairing revision, or session-ready flag settles -- all of which move during a workspace switch -- so a second closure re-armed the flag while the first create was still in flight. That is the asymmetry in the report: one terminal when arriving from the landing screen, two when arriving from another workspace. Latch the bootstrap per worktree in a module-scoped set instead, modelled on web-runtime-wake-terminal-respawn.ts, released when the create settles. The closure flag stays alongside it so a failed create still does not retry on every later frame of the same subscription. Fixes STA-6173. * fix(runtime): harden the runtime-owned initial-terminal bootstrap latch Follow-up on the STA-6173 fix, addressing restore-time safety gaps found in review. - Decline the bootstrap on a synthesized unpublished frame (`UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH` at version 0). That frame is the runtime saying "ask me later", not a host with zero terminals; seeding on it can duplicate a pane the host is about to republish after a restart. This is the same "ask me later" frame the tombstone write already refuses to treat as the user emptying the workspace. - Release the module-scoped bootstrap latch on worktree tracking teardown and environment teardown, mirroring web-runtime-wake-terminal-respawn. A create RPC that never settles during a disconnect would otherwise leave the per-worktree key set and suppress the next bootstrap after reconnect. Tests: - New per-worktree and per-environment latch-release cases. - New "unpublished frame declines" bootstrap case. - New hook-level regression pinning the second defect end to end: a forced active-subscription re-run while the first create is in flight seeds exactly one terminal (two on the pre-fix tree). * fix(runtime): key the initial-terminal bootstrap latch per environment Addresses review on the STA-6173 hardening. - Key the bootstrap latch by (environment, worktree), not worktree alone. A worktree id is `repoId::path` with no host component, so the same id can be live on two paired runtimes at once. The latch was cleared wholesale on any environment teardown, so tearing down environment A released environment B's in-flight key and a fresh B subscription could seed a duplicate — the STA-6173 defect through another door. Environment teardown now clears only its own keys; worktree teardown clears only that (environment, worktree). - Hold the latch after a successful create until a mirrored `tabsByWorktree` row exists. The snapshot refresh the create awaits can resolve on an empty, unconfirmed frame that leaves no row; releasing then let a later effect re-run seed a second terminal. A failed create still releases for retry. The latch claim/create/release now lives in web-runtime-initial-terminal-bootstrap-dispatch.ts, keeping active-session-subscription within its line budget. Tests (each mutation-tested against its own regression): - Cross-environment: a create for env B in flight, env A torn down, a fresh B closure must still decline. Fails when env teardown sweeps all environments. - Hold-until-row: a create that resolved without mirroring a row must not let an effect re-run seed again. Fails on unconditional release. * fix(runtime): release the bootstrap latch when the create returns failed createWebRuntimeSessionTerminal never throws: the operation catches RPC and network failures and returns `{ status: 'failed' }`. The dispatch helper released the latch only from `catch` (dead for that path) or once a tabsByWorktree row existed (false after a failure), so a failed create left the latch held and suppressed every later auto-seed for that environment's worktree until teardown -- the opposite of its own doc comment. Capture the outcome and release on a returned failure as well, so the next focus can retry. Regression: a create that resolves `{ status: 'failed' }` followed by an effect re-run must create again. Fails on the previous release condition. * fix(runtime): release a parked bootstrap on the mirror's next frame The previous commit released the latch on a returned failure, but a create that *succeeded* with no mirrored row yet had no release at all: the row-conditional check was the only exit for the success path, so a host that accepted the tab while the mirror never got a frame held the latch until environment teardown and suppressed every later auto-seed for the worktree. Give the latch two phases. `creating` blocks other closures while the RPC is in flight. A success with no row is parked as `awaiting-mirror` instead of held, and the next frame the mirror accepts for that worktree releases it -- that frame is the mirror's answer either way (a row now exists and the predicate declines on its own, or the host genuinely has no terminal and a retry is right). A create still in flight keeps its claim: releasing it on a frame would reopen the re-armed-closure race the latch exists to close. Also correct the closure-flag comment: `requestedInitialTerminal` is set only after the dispatch resolves, so a thrown create never sets it and a later frame may retry. The flag records that this subscription already owned a create; it never described a failed one. Regressions, each mutation-tested against its own term: - success with no row, then the mirror's empty answer, then a fresh closure -> must create again (fails when success-with-no-row is held instead of parked, and when the subscription does not call the frame release) - a mirror frame releases an awaiting-mirror claim but never a pending create (fails when the release ignores the phase) * fix(runtime): let a failed create retry inside its own subscription The returned-failure release freed the module latch but the caller still latched its closure-local requestedInitialTerminal whenever the dispatch reported it owned the create, including a create that returned { status: 'failed' }. A thrown failure never set it and retried on the next frame; since every RPC and network failure is reported as a return, the live path was the one that suppressed the whole subscription. The dispatch now reports false for a failure whichever way it arrives. * fix(runtime): close the second re-seed door on an emptied workspace ensureWebRuntimeWorktreeTerminalAfterWake read tabsByWorktree through `?? []`, and the only guard that could have seen presence was &&-gated on the value that erased it, so it could never fire for a tombstone. It runs on every activateAndRevealWorktree, not only after a wake, and a tombstoned workspace routes exclusively here: the stream-frame path returns at localTerminalCount === 0 before reaching its own guard. So the workspace was re-seeded on every focus regardless of the mirror fix. Two states shared one line and now do not. With no rows the workspace is being seeded for the first time and the decision goes to shouldAutoCreateInitialTerminal with presence read by Object.hasOwn; with rows present the question is whether a woke workspace's chrome outlived its PTYs, which the tombstone says nothing about. Both arms are pinned, because reverting only the respawn arm broke no existing test. * fix(runtime): stop the worktree-id re-key inventing a tombstone canonicalizeTerminalSessionWorktreeId read the source row through `?? []` and always wrote the target one, so a workspace with NO tabsByWorktree row came out the other side with an explicit empty one. That is the closed-last-terminal tombstone, and this PR's new activation-path reader honours it, so the re-keyed workspace never gets its initial terminal. Guarded on the source row's presence, which is the guard the sibling keyed maps six lines below already use. Mutating the guard to test emptiness instead of presence breaks the assertion that a real tombstone still survives the re-key, which is the distinction that matters. * fix(runtime): reconcile session tab bootstrap integration * fix(runtime): preserve terminal wake launch options * test(session): annotate cross-project persistence fixture * test(session): remove obsolete typecheck suppression * test(session): keep persistence regression in renderer project * refactor(runtime): remove inert snapshot recovery wrapper * fix(runtime): keep removal-frame import merge-safe * fix(runtime): deduplicate merged removal-frame import --------- Co-authored-by: Wooseong Kim <innocarpe@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>