[nwparker]

	Files	Added	Deleted	Net
Test	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​772	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​770
Prod	8	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​228	$\color{#cf222e}{\Huge{\mathbf{−}}}$​11	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​217

Fixes STA-6173 / #17681.

ELI5

When you close the last terminal in a workspace, Orca is supposed to remember "the user emptied this on purpose" and leave it empty next time you look at it. That memory worked for workspaces on your own machine but not for workspaces owned by a remote orca serve runtime: every time you focused a remote-owned workspace it made a new terminal, and coming back from another workspace sometimes made two. Two independent bugs caused it, and this PR fixes both, then hardens the fix so it can't misbehave at app-restart time.

1. The remote path had no closed-last-terminal tombstone

The local seeder tells apart never initialized (no tabsByWorktree row) from the user closed the last terminal (an explicit empty row) — shouldAutoCreateInitialTerminal in initial-terminal.ts. The mirror could never record the second state: a host snapshot with no terminals produced nextTerminalTabs === null, which withWorktreeEntry turns into a deleted key. Every focus therefore read the workspace as never-initialized and re-seeded.

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 — sameTerminalTabs treats a missing row and an empty one as equal, so the write is a no-op.
• Worktree removal frames keep deleting; the workspace is genuinely gone and a retained row would leak.
• Synthesized unpublished frames (UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH at version 0) keep deleting too. That frame is the runtime saying ask me later, not evidence the user emptied anything — hostSnapshotAffirmsWorktreeContents already spells this out for browser rows.

Because tabsByWorktree is part of the persisted workspace session, the tombstone survives an app restart, matching local behaviour. (Verified it round-trips through the host split/merge and hydration — see restore-time safety below.)

2. The bootstrap latch was closure-scoped

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 exactly the asymmetry in the report: one terminal arriving from the landing screen, two arriving from another workspace, and the capture where the visible count stayed at 1 but the PTY id was replaced within 4s (two creates, one lost the race).

web-runtime-initial-terminal-bootstrap.ts latches the bootstrap per worktree in a module-scoped set, modelled on the existing web-runtime-wake-terminal-respawn.ts. It is released when the create settles with a mirrored row, or at once on a thrown or returned failure so a later focus can retry; a create that succeeded but whose row has not landed yet is parked until the next frame the mirror accepts for that worktree (see §4). The closure flag stays alongside it and records that this subscription already owned a create, so the same closure never seeds twice once the shared latch is released — a create that threw never set it, so a later frame may retry.

3. Hardening (follow-up commit, from review)

Two restore-time safety gaps, both fixed with mutation-tested pins:

• Decline the bootstrap on a synthesized unpublished frame. An UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH/v0 frame is "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. The predicate now requires hostSnapshotAffirmsWorktreeContents(event), symmetric with the tombstone write.
• Release the module latch on teardown. A create RPC that never settles during a disconnect would leave the per-worktree key set and suppress the next bootstrap after reconnect. Tracking teardown (clearWebSessionTabsTrackingForWorktree) and environment teardown (clearWebSessionTabsTrackingForEnvironment) now release it, mirroring the wake-respawn latch exactly.

4. Follow-ups from review (commits 3 and 4)

• Key the latch per (environment, worktree). A repoId::path id has no host component and can be live on two paired runtimes at once, so a worktree-only key let one environment's teardown release a sibling environment's in-flight create — the STA-6173 defect through another door. Environment teardown now clears only its own keys.
• Hold the latch until a mirrored row exists. The create's awaited refresh can resolve on an empty, unconfirmed frame with no tabsByWorktree row; releasing then let a later effect re-run seed again. The claim/create/release now live in web-runtime-initial-terminal-bootstrap-dispatch.ts.
• Release the latch on a returned failure (pullfrog), and on the mirror's next answer for a success with no row (readiness review). createWebRuntimeSessionTerminal never throws — it reports RPC and network failures as { status: 'failed' } — so a release that lived only in catch or behind the row check left the latch held after a failed create and would have suppressed every later auto-seed for that worktree until teardown. The dispatch helper now captures the outcome and releases on status === 'failed'. The inverse hazard existed too: a create that succeeded but whose frame never landed had no release at all until environment teardown. That case is now parked as awaiting-mirror and released by the next frame the mirror accepts for the worktree — the mirror's answer, whichever way it goes — while a create still in flight keeps its claim (releasing that on a frame would reopen the re-armed-closure race). These are the changes in the series that remove suppressions rather than add guards, and they are the reason the restore-time claim below still holds.

Reproduction

1. Pair a desktop Orca to a remote runtime started with orca serve.
2. Open a workspace the remote runtime owns.
3. Close every tab in that workspace.
4. Click another workspace in the sidebar, then click the remote workspace again.
5. Before the fix: two terminal tabs appear (new shells, nothing ran in them). After: the workspace stays empty.

Before / after proof (measured, not inferred)

Both defects are pinned by tests that fail on origin/main and pass here, run against a built baseline worktree at the current main tip:

• Defect 1 (mirror tombstone). web-session-tabs-sync-emptied-workspace-tombstone.test.ts → "leaves an explicit empty row when the host retracts the last terminal": on baseline applyWebSessionTabsSnapshot deletes the key (Object.hasOwn(...) === false); here it keeps [].
• Defect 2 (closure re-arm), end to end. New web-session-tabs-sync-initial-terminal-relaunch.test.tsx drives the real useWebSessionTabsSync hook: an empty active snapshot (create left pending), then a forced active-subscription re-run (connection-generation bump = the workspace-switch-back shape), then a second empty snapshot on the new closure. Baseline calls createWebRuntimeSessionTerminal 2 times; here 1.

Per-assertion mutation testing (each specific production line reverted in isolation; each breaks only its own assertion, the rest stay green):

Mutation	Assertion that fails
tombstone write [] → null	tombstone: leaves an explicit empty row
predicate → localTerminalCount === 0	bootstrap: declines when explicit empty row
drop hostSnapshotAffirmsWorktreeContents guard	bootstrap: declines on unpublished frame
latch begin always returns true	bootstrap: declines a second bootstrap in flight
latch records nothing (fully defeated)	latch-in-flight + relaunch double-create + both teardown tests
drop worktree-scoped teardown	latch released on worktree tracking clear
drop env-scoped teardown	latch released on environment teardown

(Removing the subscription's || isInFlight(...) read alone failed no test — determined to be an inert mutation, not a weak test: the begin() gate is a redundant second consumer of the same latch, and defeating the latch entirely does fail the end-to-end test.)

Restore-time safety (P0-adjacent check)

This PR changes terminal-seeding decisions on runtime-owned (paired orca serve) workspaces only. It was checked against the in-flight "restart restores no SSH tabs" report:

• The active session-tabs subscription and the mirror only run when a runtime environment id resolves. Pure ssh: hosts resolve to runtimeEnvironmentId: null, so direct-SSH workspaces never reach either path. SSH tabs restore through persistence/lease recovery, which this PR does not touch.
• shouldAutoCreateInitialTerminal's existing local behaviour is unchanged — this PR only adds a new caller of it in the runtime predicate.
• The empty-row tombstone round-trips through restore: the host split copies [] (assignWorktreeKeyed), the merge Object.assignes it back, and hydration's row plan explicitly keeps length === 0 rows. A restored workspace with real terminals has a non-empty row → not seeded (they're mirrored/restored); only an explicitly-emptied row suppresses seeding.
• The hardening in §3 removes the one way this change could add a terminal at restart (seeding on an "ask me later" frame) and the one way it could suppress one after reconnect (a stuck latch).

Notes on the report's other hypotheses

• clientMutationId is a real gap but secondary. Fixing the latch removes the second create at the source, so no mutation id is added here.
• Not the setup-script tab (queueHookCommandsForFirstWorktreeTab attaches to an existing tab).
• STA-4908's host-authority verdict gates the seeding path; the bootstrap predicate is the different path fixed here.

Unblocks STA-6174 (keep a workspace selected and empty when its last tab closes) on runtime-owned workspaces.

Testing

• pnpm tc clean; pnpm run check:code-quality:changed — 0 findings across 9 changed files (incl. React Doctor); pnpm format (staged only).
• Rebased/cherry-picked cleanly onto current origin/main.
• web-session-tabs-sync-* suites: 51 tests pass (bootstrap, tombstone, relaunch, window-visibility, visibility-collision).
• Every review-driven change is pinned by a test that fails when only that change is reverted: cross-environment teardown, hold-until-row, and returned-failure release each have a mutation that breaks exactly one assertion.

Not verified against a live desktop↔orca serve UI session — the pins are hook-level and mirror-level, run against a built baseline for before/after; the end-to-end double-create is reproduced through the real subscription hook.

Second review pass: the third suppression path, and the second re-seed door

Two reviewers had each found one way the latch could stay held. A third existed, and — more seriously — the fix as it stood did not actually deliver the PR's own headline.

5. The third suppression path: a returned failure suppressed its own subscription

Commit 0b7b1612f01 released the module latch on a create that returns { status: 'failed' }. But the caller sets its closure-local requestedInitialTerminal whenever the dispatch reports it owned the create — and that included the failure:

if (await dispatchWebRuntimeInitialTerminalBootstrap(environmentId, activeWorktreeId)) {
  requestedInitialTerminal = true
}

A thrown failure never set it, so the very next frame could retry; the comment beside it says exactly that. A returned failure did set it. Since createWebRuntimeSessionTerminal reports every RPC and network failure as a return rather than a throw, the live path was the one that suppressed the whole subscription: after one transient failure, that closure never seeded again for as long as it lived — a workspace stuck empty until an environment, connection-generation or pairing-revision change reinstalled the effect.

The same suppression, one level up from the one that was just fixed. The dispatch now reports false for a failure whichever way it arrives, so the two paths are symmetric.

Why the existing returned-failure test did not catch it: it bumps the connection generation between the two frames, so it always retries from a fresh closure. The new assertion publishes the second frame on the same subscription.

Every exit from the dispatch helper, enumerated — the outcome union is closed (created | failed), so this is exhaustive:

exit	latch	closure flag
begin() returns false (another closure holds it)	untouched	not set
create throws	released	not set — retry allowed
create returns failed	released	not set — fixed here
create returns created, row exists	released	set
create returns created, no row	parked awaiting-mirror	set

6. The second re-seed door — the PR did not deliver its title without this

ensureWebRuntimeWorktreeTerminalAfterWake (src/renderer/src/lib/web-runtime-worktree-terminal-after-wake.ts) read the row through ?? [], and the only guard that could have seen presence was &&-gated on the value that erased it:

const tabs = state.tabsByWorktree[worktreeId] ?? []      // missing and empty collapse here
…
if (tabs.length > 0 && renderableTabCount === 0) return  // so this can never fire for a tombstone

It is called from worktree-activation.ts:329 on every activateAndRevealWorktree — "after wake" names one reason activation runs, not a precondition. And the tombstone state routes exclusively here: with rows present the stream-frame path handles it, and with rows absent-or-empty that path returns at localTerminalCount === 0 before reaching its own guard. So an emptied runtime-owned workspace was re-seeded on every focus regardless of the mirror tombstone this PR adds.

This is the defect shape the tombstone exists to prevent, arriving through a reader that never consults the record: the write side was correct in isolation and one consumer decided seeding on a count.

Deliberately not a predicate swap. Two states shared that line and now do not:

• no rows — the workspace is being seeded for the first time, and the decision goes to shouldAutoCreateInitialTerminal(renderableTabCount, Object.hasOwn(state.tabsByWorktree, worktreeId));
• rows present — the question is instead whether a woke workspace's tab chrome outlived its PTYs, which the tombstone says nothing about, and renderableTabCount === 0 still declines.

A blind swap would have silently dropped the respawn arm. The mutation proves it: reverting only that arm broke no test in the repo, so an assertion was added for it and the mutation now fails exactly that one. Recording it because it is the honest reading — that mutation survived because the test was weak, not because the mutation was inert.

What was checked and deliberately not changed

getLastKnownHostTerminalTabCount returns 0 both for "the host has no terminals" and for "we have never heard from this environment", which is unverifiable read as exited and would normally deserve the hostSnapshotAffirmsWorktreeContents treatment applied in §3. It does not apply at this call site: having no tracking entry is the normal state on a first activation, and declining there would break the primary case this function exists for — creates a host terminal when waking an empty remote workspace covers exactly that. Left alone on evidence rather than on principle.

Sibling latch: still out of scope, now filed

web-runtime-wake-terminal-respawn.ts keeps main's worktree-only keying and is still cleared globally on per-environment teardown. Re-verified unreachable from this repro — shouldRespawnWebRuntimeTerminalAfterWake returns false when localTerminalCount === 0, so an emptied workspace never reaches it through the stream-frame path. Not widened here; filed as #19899's sibling, #19898, with the fix shape (mirror the bootstrap latch's (environmentId, worktreeId) keying and per-environment teardown).

The shared-semantics boundary with #19572

The tombstone this PR writes and #19572's adoption read the same value in opposite directions: [] means "the user closed the last terminal" here, and "a gap to fill" there. Six assertions in #19572 pin the boundary through the shipping write path and this PR's own shouldAutoCreateInitialTerminal — including that the legacy-transition resurrection happens once and cannot recur, which had been argued in that gate's knownGaps rather than driven.

Both merge orders were built and run. The two PRs share no file, so main + #18513 + #19572 and main + #19572 + #18513 produce a byte-identical tree (c02d18c18386f51689b18763290ce1f0b4e2e521); the combined suites pass identically on both — 56 tests.

Mutation testing, second pass

Mutation	Assertion that fails
the failed branch reports true again (pre-fix behaviour)	retries on the next frame of the same subscription after a create that returned failed
the failed branch reports false but never releases the shared latch	retries the bootstrap on the next focus after a create that returned failed (+ the same-closure one)
a mirror frame releases a create still in creating	releases an awaiting-mirror bootstrap on a mirror frame but never a pending create
revert the activation branch split	does not create a terminal for a workspace the user emptied
drop only the respawn arm of the branch split	declines when tab rows exist but none of them render — this failed nothing before the assertion was added

Confirming §4's claim directly: the awaiting-mirror guard is pinned — a create still in creating is never released by a frame, and mutating that check to release unconditionally fails exactly the assertion that says so.

Residual risk, recorded

The latch has no claim token, so a teardown that lands mid-create followed by a new closure claiming the latch lets the stale create's resolution mutate the new claim (end releases it, or markAwaitingMirror downgrades creating to awaiting-mirror so a frame may release a genuinely in-flight create). It requires teardown plus re-subscribe plus a new frame inside one create RPC. Teardown already accepts a duplicate over permanent suppression by design (§3), so this is a second-order effect of an accepted trade rather than a new one — but it is the narrow case where "one focus creates at most one terminal" can still fail, and a claim token is the fix if it is ever observed.

Verification, this pass

pnpm tc clean · pnpm test src/renderer src/shared — 38381 passed, 0 failed · oxlint 0 · pnpm run check:code-quality:changed 0 new findings across 12 files (incl. React Doctor) · pnpm run check:max-lines-ratchet · pnpm format. No max-lines suppression added.

7. A re-key that invented the tombstone

canonicalizeTerminalSessionWorktreeId (src/main/runtime/workspace-session-worktree-id.ts) read the source row through ?? [] and then always wrote the target one:

const tabs = session.tabsByWorktree[sourceWorktreeId] ?? []
delete session.tabsByWorktree[sourceWorktreeId]
session.tabsByWorktree[targetWorktreeId] = tabs.map(…)   // writes [] when there was no row

So a workspace with no tabsByWorktree row came out of a worktree-id re-key with an explicit empty one — a closed-last-terminal tombstone the user never created. Six lines below, the sibling keyed maps (tabGroupLayouts, activeTabIdByWorktree, activeGroupIdByWorktree) already guard with Object.hasOwn; only this one did not.

This belongs to this PR rather than to a cleanup, because §6 adds the reader that makes it bite: with the activation path now honouring the tombstone, a fabricated one means the re-keyed workspace never gets its initial terminal. The normal orphan-adoption flow overwrites the fabricated row with the claimed tabs, so the reachable case is the narrow one where the claim set is empty.

The guard is on the source row's presence, not on the tabs being non-empty, and the mutation is what says that matters: guarding on length > 0 instead breaks carries a real closed-last-terminal tombstone across the re-key, because a deliberate empty row must survive a re-key too. Dropping the guard entirely breaks does not fabricate a tombstone for a workspace that had no tabs row. One assertion each.

Findings filed rather than fixed

Sweeping the rest of the readers turned up three more that collapse an absent row into an empty one, and one that looks mechanical and is not. All are on #19899, with the reviewer-facing summary: six readers lose the missing-vs-empty distinction; two are predicate-shaped and four need the presence test this codebase already uses correctly at several sites — none requires new logic. Two of the six are fixed here (§6, §7); the rest are client-side and independent, and one of them — the remote-workspace publish gate — carries the only version boundary in the set, so it wants its own PR and its own wire-compat paragraph rather than being smuggled in here.

CI, read from job steps

All 8 test / tests node 24 shards, typecheck, static analysis, pullfrog, e2e / changed e2e specs, e2e / build e2e app, package (windows), test vs non-test LoC and root directory guard green.

One failure, investigated rather than waved through: the Linux package job failed at step 5, Test Linux Electron lifecycle boundary, on src/main/browser/browser-route-webrtc-egress.electron.test.ts > blocks direct UDP after applying the exact guest policy, with ContextResult::kTransientFailure: Failed to send GpuControl.CreateCommandBuffer in the log. This PR contains no src/main/browser code and no WebRTC or GPU surface, package (windows) passed on the same commit, and the sibling PR #19572 — which touches strictly more main-process code — passed the same Linux package job on the same infrastructure. Re-run rather than argued from the diff alone, and it passed.

Final run on ae38bb1: every check green, 20 non-skipped, including verify, pullfrog, package, package (windows) and e2e / changed e2e specs.

On the skips: e2e / ssh docker watcher isolation is skipped here and that is correct rather than a gap — this PR changes seeding on runtime-owned (paired orca serve) workspaces only, and a pure ssh: host resolves to runtimeEnvironmentId: null, so direct-SSH workspaces never reach either changed path. The dimension this PR does exist to check is e2e / changed e2e specs, which ran and passed.

Skips are path-filtered on dimensions this PR does not touch (Git compatibility, real WSL terminal, shell contracts, cross-version wire compatibility, the Docker SSH e2e jobs). None of them is the dimension this PR exists to check — that one is e2e / changed e2e specs, which ran and passed.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The change preserves explicit empty terminal rows when the host closes the last terminal. It omits rows for removed worktrees and unpublished snapshots. Initial terminal bootstrap state is keyed by environment and worktree. Bootstrap state now tracks creation and mirror-wait phases. Bootstrap dispatch releases state after failed creation, and mirror frames release completed claims. Tests cover snapshot handling, bootstrap conditions, relaunches, environment isolation, and test-state reset.

Priority: ➖ Normal — Impact reflects medium issue severity.

Merge Risk: 🟡 Moderate · up to 4f775

During a teardown and reconnect race, runtime workspaces can create duplicate initial terminals after a user has closed them. The bootstrap claim needs ownership protection before this change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 10 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the primary fix: preventing repeated terminal creation in a runtime-owned workspace that the user emptied.
Description check	✅ Passed	The description is detailed and covers the problem, implementation, linked issue, behavior proof, testing, limitations, and residual risks. It does not use every template heading or checklist item, bu…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 69995875-a7d7-4cce-9153-896d5ac24c01

📥 Commits

Reviewing files that changed from the base of the PR and between 16e2624 and 455a74f.

📒 Files selected for processing (7)

• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts
• src/renderer/src/runtime/web-session-tabs-sync-emptied-workspace-tombstone.test.ts
• src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts
• src/renderer/src/runtime/web-session-tabs-sync-test-harness.ts
• src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts
• src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts
• src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[pullfrog]

Important

The bootstrap latch is missing the teardown wiring its model has — see the inline comment. Otherwise the two-part fix is sound: the empty-row tombstone aligns the runtime mirror with the Object.hasOwn(tabsByWorktree, worktreeId) convention the local seeder and direct-SSH merge already honour, and the latch correctly closes the two-terminals race.

Reviewed changes

• Empty-row tombstone — prepareWebSessionTabsSnapshotBase now writes [] for an empty affirmed host snapshot (retraction of the last terminal) instead of null/deleted key, while removal frames and synthesized unpublished frames still delete.
• Shared bootstrap latch — a module-scoped per-worktree Set (web-runtime-initial-terminal-bootstrap.ts) replaces the closure-local requestedInitialTerminal, so "one focus → one terminal" survives the subscription effect re-runs a workspace switch triggers.
• Shared seeding predicate — shouldBootstrapInitialWebRuntimeTerminal delegates to shouldAutoCreateInitialTerminal(localTerminalCount, hasPersistedTerminalState) rather than a bare localTerminalCount === 0.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 86308740-927e-4db8-b45d-06c9be8cff82

📥 Commits

Reviewing files that changed from the base of the PR and between 455a74f and 53c5932.

📒 Files selected for processing (4)

• src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx
• src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts
• src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts
• src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 82244c4d-b645-4698-a57b-33400c1e0fa7

📥 Commits

Reviewing files that changed from the base of the PR and between 53c5932 and 3a62734.

📒 Files selected for processing (6)

• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts
• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts
• src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx
• src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts
• src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts
• src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts
• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts
• src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

[pullfrog]

Important

The teardown wiring from the prior review is in place and the per-environment keying is a correct refinement — but the new dispatch helper never releases the latch when a create fails with a returned { status: 'failed' } outcome, only when it throws. See the inline comment.

Reviewed changes

• Added the requested teardown wiring — clearWebSessionTabsTrackingForWorktree and clearWebSessionTabsTrackingForEnvironment now release the bootstrap latch, and the seeding predicate additionally declines on synthesized unpublished frames.
• Re-keyed the latch per environment — the module latch became a Map<environmentId, Set<worktreeId>>, so a per-environment teardown releases only its own in-flight keys and preserves a sibling environment's pending create.
• Extracted claim/create/release into a dispatch helper — dispatchWebRuntimeInitialTerminalBootstrap now holds the latch until a tabsByWorktree row exists (row-conditional release) instead of a plain .finally.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

The two commits since the prior Pullfrog review close the returned-failure gap it flagged and add an awaiting-mirror phase so a successful-but-unmirrored create cannot silently suppress later auto-seeds.

• Released the latch on a returned failed create — dispatchWebRuntimeInitialTerminalBootstrap now captures the create outcome and releases the latch on status === 'failed' (in addition to the thrown path and the existing-row path), so an RPC/network failure that returns rather than throws no longer suppresses every later auto-seed until teardown.
• Added an awaiting-mirror phase to the latch — a successful create whose tabsByWorktree row has not landed parks the latch in awaiting-mirror (instead of holding it until environment teardown), and the active subscription releases it on the next applied frame for the worktree while a still-in-flight create keeps its claim.
• Pinned both paths with regression tests — two relaunch tests drive the real useWebSessionTabsSync hook through a returned-failure retry and a never-mirrored-success retry, and latch-phase unit tests cover the awaiting-mirror release and the never-claimed no-op.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8bdba7cd-b49b-485e-9312-6486dbcb037a

📥 Commits

Reviewing files that changed from the base of the PR and between 0b7b161 and 4f775d6.

📒 Files selected for processing (5)

• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts
• src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts
• src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx
• src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts
• src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Give each bootstrap claim an ownership token.

A create can resolve after teardown clears its latch and a replacement subscription claims the same environment and worktree. At Line 60, the stale dispatcher can change the replacement claim from creating to awaiting-mirror. Its later cleanup can also delete that replacement claim. A mirror frame can then release a still-pending replacement create and allow a duplicate terminal request.

Store a unique claim token with each phase. Apply markWebRuntimeInitialTerminalBootstrapAwaitingMirror and endWebRuntimeInitialTerminalBootstrap only when the caller owns that token. Add a teardown-and-reconnect race test.

The stated teardown lifecycle makes this race reachable.

[pullfrog]

✅ No new issues found.

Reviewed changes

The two commits since the prior review close the last re-seed door and fix a third suppression path, both pinned with regression tests.

• Let a failed create retry within its own subscription — dispatchWebRuntimeInitialTerminalBootstrap now returns false on a returned { status: 'failed' } outcome (not only on a throw), so the caller no longer latches its closure-local requestedInitialTerminal and the same subscription can retry on a later frame.
• Closed the second re-seed door — ensureWebRuntimeWorktreeTerminalAfterWake now consults shouldAutoCreateInitialTerminal when the workspace has no tab rows, so an explicit empty-row tombstone (the user emptied it) declines the wake-time respawn instead of seeding on every activateAndRevealWorktree.
• Pinned both paths — a relaunch test drives the real hook through a returned-failure retry within one subscription, and three activation tests cover the emptied-workspace decline, the no-render decline, and the outlived-PTY respawn.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

One commit since the prior Pullfrog review closes the last tombstone-fabrication path this PR introduced.

• Guarded the worktree-id re-key on source-row presence — canonicalizeTerminalSessionWorktreeId now wraps the tabsByWorktree move in Object.hasOwn(session.tabsByWorktree, sourceWorktreeId), so a workspace with no row no longer exits the re-key with a fabricated [] closed-last-terminal tombstone (which the new activation-path reader would have honoured, suppressing its initial terminal). A deliberate empty row still survives the re-key, matching the sibling keyed maps' presence check.
• Pinned the guard with three tests — workspace-session-worktree-id.test.ts covers the no-fabrication case, the tombstone-carry case, and the re-key of the tabs' own worktree ids. The suite passes locally.

  ^{｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏}

[nwparker]

Closing as superseded after checking current main.

Merged #21630 incorporated the empty-workspace initialization fix and subsequent hardening. Current main preserves explicit empty terminal tombstones and uses an environment/workspace bootstrap latch, so a workspace the user emptied is not re-seeded by this path.

Closing this PR only; its branch is being retained.