Skip to content

fix(worktrees): a listing that predates a create can no longer retire the new workspace - #22311

Merged
brennanb2025 merged 7 commits into
mainfrom
brennanb2025/fix-claude-agent-workspace
Sep 23, 2026
Merged

brennanb2025 merged 7 commits into
mainfrom
brennanb2025/fix-claude-agent-workspace

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​862 $\color{#cf222e}{\Huge{\mathbf{−}}}$​33 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​829
Prod 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​233 $\color{#cf222e}{\Huge{\mathbf{−}}}$​81 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​152

ELI5

When you created a workspace while another one was being deleted, Orca sometimes dropped you on the empty "Select a workspace" screen and the Claude session you asked for never appeared. A worktree list that was scanned a moment too early arrived late, and Orca believed it: since the brand-new workspace was not in that list, Orca treated it as deleted and threw its state away, including the agent launch that was in flight. Now the host notices that a workspace was added or removed while it was scanning, scans again before it answers, and only ever publishes a list that describes the catalog as it is.

What Changed

Before: create a workspace with Claude as the default agent while a delete of another workspace is in progress. Occasionally the new workspace's terminal and chat tabs vanish, the app shows the Landing screen, the sidebar row dims, and the Claude session is closed by Orca the moment the host publishes it. Navigating back and forward shows the workspace with only a rebuilt terminal and no chat.

After: the new workspace stays selected with its Claude chat tab; the deleted workspace still disappears. No scan-failure indicator, no hidden external cards, no other visible change.

Mechanism. Worktree listings hide a prepared checkout while it is still locked, and the lock is released only at the end of the prepared checkout's finalize. A scan that started inside that window omits the new worktree. The host returned such a scan as authoritative even after a mutation had invalidated it mid-flight, and every client's reconciliation treats an authoritative listing that lacks a known worktree as proof of deletion: the desktop renderer purged the tabs, silently tombstoned the pending structured launch (so the published host session was retired), and cleared the active worktree.

Three changes, all on the host. The renderer is unchanged.

  • The host re-runs a scan that a worktree change overtook, bounded, through the same guard chain, for cached and uncached hosts alike. A scan records the repo's worktree-change generation when it begins; if the generation moved before the scan settled (or the caller joined a scan that was invalidated), the listing scans again instead of publishing, up to twice. On local hosts the re-run goes back through the scan cache, so it scans afresh or joins the scan a sibling has already started, and its side-effect tokens, abort and currency checks are the ordinary ones. On SSH hosts, which have no scan cache, the provider listing takes the same generation witness and re-lists under the same bound. The runtime listing (worktree.detectedList, the one paired web clients and a desktop client attached to a remote runtime read) takes the same witness around its own scan and re-lists under the same bound, through the runtime's scan cache. One function (overtaken-scan-rerun.ts) holds the rule and the bound for all three listings. Folder repos read their rows from the store synchronously, so nothing can overtake them and they are not under the rule.
  • Both create paths (the prepared-checkout path and the runtime path) run the worktree-change invalidators as soon as the new worktree is listable, not only at the end of the create; the SSH create does the same right after the remote add.
  • The runtime registers with the worktree-change invalidators when it is constructed: a change bumps the generation its listing witnesses and clears the runtime's own scan cache, so the re-run lists again instead of being served the overtaken scan from cache. Before, the generation bump was registered only when the desktop IPC module loaded, which a headless host never does, so a headless host's create bumped nothing; and nothing cleared the runtime scan cache on a desktop-side create until the end of the create.

Past the bound. If three scans in a row are each overtaken (continuous churn), the desktop and direct-SSH listings answer "stale" for that request rather than publishing any of them. A stale reply is a request-level rejection the client already handles: it leaves client state untouched, no rows are replaced, nothing is purged, and no failure reason ships, so the sidebar's scan-failed indicator does not light up. The runtime listing has no stale reply on its wire, and a thrown error makes the client drop the repo's rows, so past the bound it publishes the last scan's rows as non-authoritative with the ordinary git source and no failure reason: the client purges nothing and lights no indicator, and neither metadata nor lineage is pruned from rows that may omit a worktree created under the scan. The one visible cost is that a row the last overtaking mutation created can be absent from that client until the follow-up listing. A listing that spends its bound logs a warning on the host with the repo id, so continuous churn is diagnosable from the host log. The listing that reflects the churn is guaranteed by the churn itself: every overtaking mutation ends by sending its change event after its last generation bump (the create paths bump when the worktree becomes listable, and the finishing change event bumps once more on the desktop and on the runtime alike, since the runtime's own change notification now runs the same invalidators before it sends), so the event is ahead of this reply on the wire. On the desktop, the renderer's change-refresh queue keeps one trailing refresh for events that land while a refresh is running, and that trailing refresh starts a fresh provider invocation after the stale one settles. For paired clients, the runtime's worktreesChanged client event travels the client-events subscription and lands in the same refresh queue, keyed to the runtime host, with the same trailing-refresh guarantee.

Why

The two failing creates in the trace were exactly the two with a concurrent delete; the other 38 creates since Sep 15 succeeded. The purge on a stale listing dates to late August; #20502 (Sep 15) made bulk purges cancel provisional structured launches, which turned "blank screen" into "blank screen and no agent", and #20722 (Sep 15) widened the window by keeping interactive and status listings from coalescing.

The host owns the worktree catalog, so the host is where a scan the catalog moved under must be re-run. Fixing it there protects every client that reads the listing (desktop renderer, paired web, mobile, and older clients) with one rule, and nothing downstream has to learn a weaker meaning of "authoritative".

Alternatives considered:

  • Publishing an overtaken scan as non-authoritative with an unavailableReason. That reason is what the sidebar's scan-failed indicator renders, external worktree cards and the visibility dialog only read an authoritative catalog, and a change notification that coalesces onto the overtaken invocation never re-lists, so a routine create/delete overlap would have surfaced as "Worktree scan failed" until an unrelated refresh.
  • Publishing an overtaken scan as non-authoritative without a reason once the re-scan bound is spent (an earlier revision of this PR). Non-authoritative rows still replace the client's rows for that host, so a worktree the last overtaking mutation created would vanish from the sidebar, and the active workspace could show Landing, until the next listing. Answering stale leaves the client's rows alone.
  • A renderer-side fence (an earlier revision of this PR): record the local create sequence when each provider invocation begins, thread it through coalescing joiners and the direct-SSH refresh, and have the merge refuse to drop rows created after that point. It protected one client with three coordinated structures that needed two follow-up fixes (joiner ordering, eviction order) and one extension (the direct-SSH path), and it left paired web and mobile clients exposed. Removed in favor of the host rule.

Linked Issue

Internal P0 report (screen recording, 2026-09-22); no tracker issue was filed.

Visual Proof

No visual change in the success path. The failure is a race: it reproduced twice in production and once in a dev rig with an artificially slow git worktree list (the new row vanished from the sidebar for nine seconds). The consequence the host now prevents is pinned by stale-listing-purges-created-worktree.test.ts, which shows what an authoritative listing that omits a known worktree does to the client (launch cancelled, tabs wiped, selection cleared) and that a listing which includes the row leaves everything intact. The host rule is pinned by detected-provider-listing-overtaken-scan.test.ts (local and SSH branches), the detected-scan-cache handler suite, runtime-managed-worktree-queries.test.ts (runtime listing: re-run, prune only on the settled scan, past-bound reply, failed scan not re-run) and orca-runtime-worktree-change-invalidation.test.ts (through the real runtime class and its scan cache, without the desktop IPC module loaded).

Testing

  • pnpm tc, pnpm run check:code-quality:changed, the runtime-electron ratchet, and whole-file oxlint and oxfmt --check on every changed file pass locally on macOS.

  • Vitest: src/main/ipc/worktrees, the detected-scan-cache, metadata-pruning, runtime-queries, runtime change-invalidation, create-rearm and scan-fingerprint suites: 44 files / 409 tests pass. The runtime spec aggregator (orca-runtime.test.ts): 1292 passed, 1 skipped. src/renderer/src/store/slices/worktrees, src/renderer/src/hooks/ipc-events, src/renderer/src/components/sidebar/worktree-list and the refresh-coalescing suite: 91 files / 762 tests pass (renderer untouched since).

  • Ablation, bound: with the re-scan bound set to 0 (one substitution, verified), 8 tests across the desktop overtaken-scan, runtime-queries and runtime change-invalidation suites go red, for "expected to be called 2 times, but got 1" / "3 times, but got 1" and for the runtime listing publishing authoritative: false; the 13 tests that do not depend on the bound stay green. Restored by single-file checkout and re-run green.

  • Ablation, registration: with the runtime's two invalidator registrations deleted, all 3 tests in the runtime change-invalidation suite go red: the generation does not move, the runtime scan cache serves the overtaken scan, and the re-run never lists again. Restored by single-file checkout and re-run green.

  • Dev Electron rig (fresh profile, structured Claude default, slow-listing git shim, create via the real composer while deleting another workspace): five creates kept their row, selection and Claude chat tab; the deleted workspace was removed. That rig ran against an earlier revision of this PR that still carried the renderer fence; the host re-scan it also carried is what the handler tests pin, and the renderer fence has since been removed.

  • Not tested on a real SSH host, Linux, or Windows. The SSH branch is covered by the provider-branch test (provider listing overtaken by a generation bump, provider called twice, authoritative result from the second list).

  • The runtime listing (worktree.detectedList, served by runtime-managed-worktree-queries over repo-worktree-resolution-scan) now applies the same rule: it runs the same lock-hiding git worktree list through the runtime's own cache, takes the same generation witness, re-lists under the same bound, and past the bound publishes the last scan non-authoritatively rather than as proof of deletion. Paired web clients, and a desktop client attached to a remote runtime, read that listing and reconcile it with the same purge as the desktop path, so they are covered by the same rule; the mobile client does not read it. Not tested against a real paired web client or a headless host; the rule is pinned through the real runtime class in orca-runtime-worktree-change-invalidation.test.ts, which deliberately never loads the desktop IPC module.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

… the new workspace

Creating a workspace while another one was being deleted could land the user on the
empty Landing screen with the new workspace's Claude launch cancelled. A worktree
listing whose scan began before the create finished (the prepared checkout is hidden
from listings while it is still locked) was applied after the create returned, and the
renderer read the new worktree's absence from that authoritative listing as a deletion:
it purged the workspace's tabs, tombstoned the pending structured launch (so the host
session was retired the moment it published) and cleared the active selection.

Renderer: each provider invocation records the local create sequence when it begins
(shared by every caller that coalesces onto it). The merge no longer drops or retires a
worktree this client finished creating after that point, and the change-event diff
purge defers to what the merge actually dropped. Hydrated rows keep being reaped.

Host: a scan a worktree mutation overtook is published as non-authoritative instead of
authoritative, and the create paths invalidate scans as soon as the new worktree is
listable rather than only at the end of the create.
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 25c3fb46-5154-41ef-bddf-a833b63605ca

📥 Commits

Reviewing files that changed from the base of the PR and between e62eac3 and f9c6e89.

📒 Files selected for processing (1)
  • src/main/runtime/orca-runtime-worktree-change-invalidation.test.ts

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


📝 Walkthrough

Walkthrough

This change invalidates active worktree scans after local or remote worktree creation. Scan results now report when a scan was superseded. Local and captured SSH listings rerun superseded scans up to a fixed limit. Runtime metadata pruning applies only to settled scans. Tests cover authoritative reruns, stale results, replacement caching, and renderer cleanup when an authoritative listing omits a created worktree.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to f9c6e

The retry logic closes the original race in most cases, but a mutation during metadata processing can still make a newly created workspace appear deleted, clearing its selection and tabs and cancelling its pending agent launch. This should be resolved before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 25 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 describes the main fix: preventing an outdated worktree listing from retiring a newly created workspace. It is specific and concise enough for repository history.
Description check ✅ Passed The description is detailed and covers the user impact, mechanism, rationale, alternatives, testing, limitations, and checklist. The Linked Issue section states that no tracker issue was filed, so the…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: bb017d4e-4541-4dcc-a9ad-eaa2c392c858

📥 Commits

Reviewing files that changed from the base of the PR and between a8786c0 and 9dd83dc.

📒 Files selected for processing (13)
  • src/main/ipc/worktree-remote.ts
  • src/main/ipc/worktrees-detected-scan-cache.test.ts
  • src/main/ipc/worktrees/listing/detected-provider-listing.ts
  • src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts
  • src/main/ipc/worktrees/listing/detected-worktree-scan-superseded.test.ts
  • src/main/runtime/runtime-local-git-worktree-create.ts
  • src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts
  • src/renderer/src/store/slices/stale-listing-purges-created-worktree.test.ts
  • src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts
  • src/renderer/src/store/slices/worktrees/create/created-worktree-state-merge.ts
  • src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
  • src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts
  • src/renderer/src/store/slices/worktrees/listing/worktree-slice-types.ts

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The host supersession signal and the renderer create fence fix the local race, but the fence is not threaded through the direct-SSH refresh path (acquireDirectSshDetectedWorktreeRefresh), so the SSH variant of the same race is still unguarded. See the section below.

Reviewed changes

  • Host scan supersession — DetectedWorktreeScanResult gains a required superseded flag; a scan invalidated mid-flight (or joined after invalidation) is published authoritative: false with its rows and an unavailableReason instead of vouching for what it never saw.
  • Early host invalidation — runWorktreeChangeInvalidators now runs as soon as the worktree is listable in both local create paths (worktree-remote.ts, runtime-local-git-worktree-create.ts), not only at the end of the create.
  • Renderer create fence — a monotonic local create sequence is recorded per worktree; every provider invocation captures the sequence at start and coalesced joiners inherit the first caller's value; the merge re-adds rows the scan did not see but this client created after the scan began, and filters them out of removedIds.
  • Diff-purge deference — the worktree-change diff purge no longer retires an id the merge deliberately kept.
  • Tests — new detected-worktree-scan-superseded.test.ts and stale-listing-purges-created-worktree.test.ts; the existing invalidated-scan case now asserts the non-authoritative result.

⚠️ The create fence does not reach the direct-SSH refresh path

The PR states the renderer fence "applies to every host", but the direct-SSH refresh path builds an admitted refresh without createSequenceAtRequestStart, so mergeFetchedWorktrees sees createSequence === undefined and skips the fence entirely. That path runs git worktree list over SSH, has no host-side scan cache (so no superseded signal either), and merges authoritatively — so a listing that began before a create and settles after it can still drop the new row and purge its state, i.e. the same symptom this PR fixes for local.

Technical details
# Direct-SSH refresh bypasses the create fence

## Affected sites
- `src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts:254-260` — `acquireDirectSshDetectedWorktreeRefresh` constructs `AdmittedDetectedWorktreeRefresh` with `status`, `result`, `providerResult`, `executionHostId`, `directSshAuthority` but no `createSequenceAtRequestStart`.
- `src/renderer/src/hooks/direct-ssh-reconnect-preparation.ts:154` — the reconnect/hydration coordinator calls `scheduler.request(...)`, which is how this merge runs concurrently with a create.
- `src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts:123,216` — `worktrees:list` / `worktrees:listAll` also call `listDetectedGitWorktrees` and ignore `scan.superseded`; the legacy `worktrees:list` reply is then wrapped as `authoritative: true` (`toLegacyDetectedWorktreeResult`).

## Required outcome
A direct-SSH listing that began before a locally-created worktree must not drop that row or purge its state, matching the local behavior.

## Suggested approach (optional)
`claimInvocationCreateSequence` is module-private in `detected-worktree-refresh.ts`; export it (or capture the sequence in the lease registry) and pass `createSequenceAtRequestStart` into the `refresh` object in `acquireDirectSshDetectedWorktreeRefresh`. For the bare-array catalog handlers, decide whether the superseded signal can be carried at all or whether the renderer fence is the accepted mitigation.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…ate fence rides every listing

A scan a worktree mutation overtook was published as non-authoritative with a
failure reason, which lit the sidebar's scan-failed indicator, hid external
worktree cards and emptied the visibility dialog after a routine create/delete
overlap, and its early return skipped the abort, currency and dual-owner guards
every other branch honours. The listing now re-scans a bounded number of times
instead (the invalidation already dropped the cache, so it scans afresh or joins
a sibling's scan) and past the bound answers non-authoritative through the same
guard chain, with no failure reason.

The renderer's create-sequence inception is now captured where the provider
invocation starts and released only when it settles, so a waiter released early
cannot reset it for a later joiner, and the direct-SSH scheduler's refresh --
the only listing on hosts with no scan cache -- now carries it too. A re-created
worktree id is re-inserted at the back of the create-record eviction order so
the bound holds.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 01745255-2e75-43e9-971b-9a6096edb3b3

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd83dc and f65be5e.

📒 Files selected for processing (9)
  • src/main/ipc/worktrees-authoritative-local-metadata-pruning.test.ts
  • src/main/ipc/worktrees-detected-scan-cache.test.ts
  • src/main/ipc/worktrees/listing/detected-provider-listing.ts
  • src/renderer/src/store/slices/worktrees-fetch-refresh-coalescing.test.ts
  • src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.test.ts
  • src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts
  • src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
  • src/renderer/src/store/slices/worktrees/listing/known-ssh-worktree-fetch.ts
  • src/renderer/src/store/slices/worktrees/listing/stale-listing-purges-created-worktree.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
  • src/main/ipc/worktrees/listing/detected-provider-listing.ts

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

Comment on lines +16 to +18
// Why delete first: a re-created id keeps its old iteration slot on `set`, and the eviction loop
// below stops at the first retained entry, so it would shield every record behind it.
createSequenceByWorktreeId.delete(worktreeId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- created-worktree-sequence.ts ---'
cat -n src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts
printf '%s\n' '--- detected-worktree-refresh.ts relevant sections ---'
sed -n '1,120p' src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
sed -n '160,245p' src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
printf '%s\n' '--- fetched-worktree-merge.ts relevant sections ---'
sed -n '160,240p' src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts
printf '%s\n' '--- sequence symbol usages ---'
rg -n -C 3 'currentWorktreeCreateSequence|recordLocallyCreatedWorktree|worktreeCreatedAfter|resetWorktreeCreateSequenceForTests' src/renderer/src

Repository: stablyai/orca

Length of output: 36366


🏁 Script executed:

set -e
cat -n src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts
printf '\n--- refresh ---\n'
sed -n '1,110p' src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
sed -n '180,235p' src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
printf '\n--- merge ---\n'
sed -n '175,235p' src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts

Repository: stablyai/orca

Length of output: 11614


🏁 Script executed:

set -e
printf '%s\n' '--- provider request implementation ---'
cat -n src/renderer/src/store/slices/worktrees/listing/detected-worktree-provider-request.ts
printf '%s\n' '--- refresh lease registry start contract ---'
rg -n -C 8 'startProviderRequest|createDetectedWorktreeRefreshLeaseRegistry' src/renderer/src/store/slices/worktrees

Repository: stablyai/orca

Length of output: 10786


Retain creation records for active listings

If a detected listing remains active while 257 local worktrees are created, the 256-entry eviction removes a record created after that listing started. worktreeCreatedAfter then returns false when the result is admitted. The merge can omit the worktree and purge its state, leaving a blank Landing view and cancelling an agent launch.

Retain records newer than the oldest active listing sequence until each provider or runtime request settles. Apply this to both listing paths. Keep delete-before-set ordering for recreated IDs.

Suggested fix
diff --git a/src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts b/src/renderer/src/store/slices/worktrees/create/created-worktree-sequence.ts
@@
 let createSequence = 0
 const createSequenceByWorktreeId = new Map<string, number>()
+const activeCreateFenceCounts = new Map<number, number>()
+
+export function retainWorktreeCreateRecordsAfter(sequenceAtRequestStart: number): () => void {
+  activeCreateFenceCounts.set(
+    sequenceAtRequestStart,
+    (activeCreateFenceCounts.get(sequenceAtRequestStart) ?? 0) + 1
+  )
+  let released = false
+  return () => {
+    if (released) return
+    released = true
+    const count = activeCreateFenceCounts.get(sequenceAtRequestStart) ?? 0
+    if (count <= 1) {
+      activeCreateFenceCounts.delete(sequenceAtRequestStart)
+    } else {
+      activeCreateFenceCounts.set(sequenceAtRequestStart, count - 1)
+    }
+  }
+}
@@
   createSequenceByWorktreeId.delete(worktreeId)
   createSequenceByWorktreeId.set(worktreeId, createSequence)
+  let oldestActiveCreateSequence = Number.POSITIVE_INFINITY
+  for (const activeSequence of activeCreateFenceCounts.keys()) {
+    oldestActiveCreateSequence = Math.min(oldestActiveCreateSequence, activeSequence)
+  }
   for (const [id, sequence] of createSequenceByWorktreeId) {
-    if (createSequence - sequence <= RETAINED_CREATE_RECORDS) {
+    if (
+      createSequence - sequence <= RETAINED_CREATE_RECORDS ||
+      sequence > oldestActiveCreateSequence
+    ) {
       break
     }
     createSequenceByWorktreeId.delete(id)
@@
   createSequence = 0
   createSequenceByWorktreeId.clear()
+  activeCreateFenceCounts.clear()
 }
diff --git a/src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts b/src/renderer/src/store/slices/worktrees/listing/detected-worktree-refresh.ts
@@
-import { currentWorktreeCreateSequence } from '../create/created-worktree-sequence'
+import {
+  currentWorktreeCreateSequence,
+  retainWorktreeCreateRecordsAfter
+} from '../create/created-worktree-sequence'
@@
   startProviderRequest: (request) => {
-    createSequenceAtInvocationStart.set(request.providerRequestId, currentWorktreeCreateSequence())
+    const sequenceAtInvocationStart = currentWorktreeCreateSequence()
+    const releaseCreateFence = retainWorktreeCreateRecordsAfter(sequenceAtInvocationStart)
+    createSequenceAtInvocationStart.set(request.providerRequestId, sequenceAtInvocationStart)
     const forget = (): void => {
+      releaseCreateFence()
       createSequenceAtInvocationStart.delete(request.providerRequestId)
     }
@@
   let refresh = runtimeDetectedWorktreeRefreshesInFlight.get(key)
   if (!refresh) {
+    const createSequenceAtRequestStart = currentWorktreeCreateSequence()
+    const releaseCreateFence = retainWorktreeCreateRecordsAfter(createSequenceAtRequestStart)
+    const result = listDetectedWorktreesForRepo(settings, repoId, {
+      reuseRecentCompatibilityFailure: options.reuseRecentCompatibilityFailure
+    })
+    void result.then(releaseCreateFence, releaseCreateFence)
     refresh = {
-      result: listDetectedWorktreesForRepo(settings, repoId, {
-        reuseRecentCompatibilityFailure: options.reuseRecentCompatibilityFailure
-      }),
-      createSequenceAtRequestStart: currentWorktreeCreateSequence()
+      result,
+      createSequenceAtRequestStart
     }

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Incremental re-review of f65be5e, the commit that closes the prior review's direct-SSH coverage gap and the eviction-ordering nit.

  • Re-scan instead of weakening the reply. A superseded local scan is re-run up to twice; only past that bound do the rows still ship as authoritative: false, and without an unavailableReason, so a routine create/delete overlap no longer reads as a scan failure.
  • Invocation-keyed create fence. The lease registry's startProviderRequest records the local create sequence against the ProviderRequestId and releases it when the provider promise settles, so coalesced joiners inherit the scan's inception and no early-leaving waiter can reset it.
  • Direct-SSH fence. acquireDirectSshDetectedWorktreeRefresh now threads the same sequence into its admitted refresh, closing the SSH variant of the race the prior review flagged.
  • Eviction ordering. recordLocallyCreatedWorktree deletes before re-inserting, so a re-created id moves to the back of the eviction order and no longer shields older records.
  • Tests. New cases cover the re-scan, the two-rescan bound, the direct-SSH fence, and re-created-id eviction; the affected suites pass locally (39 tests).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…or cached and SSH listings alike; the renderer create fence is removed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟠 Major · Check generation freshness after side-effect completion before… · detected-provider-listing.ts:193

src/main/ipc/worktrees/listing/detected-provider-listing.ts:193
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Check generation freshness after side-effect completion before publishing authoritative rows.

A worktree created while applyFreshDetectedWorktreeScanSideEffects is awaited can invalidate the scan after scanUntilNotOvertaken has accepted it as current. The generation check occurs before the scan runs. After the side-effect await, the code checks isCurrent() (request ownership) but does not verify that the scan generation remains current. The sideEffectToken.generation captured at scan start is available and should be re-checked after the await. If the generation has advanced, return a stale result or rescan instead of publishing these rows.

🐛 Suggested fix
    if (freshScan) {
      await applyFreshDetectedWorktreeScanSideEffects(store, repo, gitWorktrees, metadataPrune, {
        isCurrent: () => isCurrent() && !providerAbort?.signal.aborted,
        sideEffectToken,
        signal: providerAbort?.signal,
        ...(hygieneDue === undefined ? {} : { hygieneDue })
      })
      const aborted = abortedResult()
      if (aborted) {
        return aborted
      }
      if (!isCurrent()) {
        return null
      }
+     if (!isLocalWorktreeScanGenerationCurrent(repo.id, sideEffectToken.generation)) {
+       return null
+     }
    }

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: acecf742-c2bc-41d9-beb4-290c70e5a47a

📥 Commits

Reviewing files that changed from the base of the PR and between f65be5e and a1dcb9f.

📒 Files selected for processing (5)
  • src/main/ipc/worktree-remote.ts
  • src/main/ipc/worktrees-detected-scan-cache.test.ts
  • src/main/ipc/worktrees/listing/detected-provider-listing-overtaken-scan.test.ts
  • src/main/ipc/worktrees/listing/detected-provider-listing.ts
  • src/renderer/src/store/slices/worktrees/listing/stale-listing-purges-created-worktree.test.ts

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

throw err
}
// Why: the worktree is listable from here on; a scan that began before it appeared is overtaken.
runWorktreeChangeInvalidators(repo.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '2095,2150p' src/main/ipc/worktree-remote.ts
sed -n '100,215p' src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts
rg -n 'removeWorktree|runWorktreeChangeInvalidators|notifyWorktreesChanged' src/main/ipc/worktree-remote.ts src/main/providers/ssh-git-worktree-provider.ts

Repository: stablyai/orca

Length of output: 7887


🏁 Script executed:

sed -n '1798,1815p' src/main/ipc/worktree-remote.ts
sed -n '1,100p' src/main/ipc/worktree-change-invalidators.ts
sed -n '75,98p' src/main/providers/ssh-git-worktree-provider.ts
rg -n 'worktreesChanged|worktree.*changed|notifyWorktreesChanged' src/main src/renderer

Repository: stablyai/orca

Length of output: 41550


🏁 Script executed:

sed -n '1,105p' src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts
sed -n '95,130p' src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts
rg -n 'registerWorktreeChangeInvalidator' src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts src/main/ipc/worktrees

Repository: stablyai/orca

Length of output: 6279


🏁 Script executed:

cat -n src/main/ipc/worktrees/listing/register-detected-worktree-scan-invalidation.ts
rg -n 'registerDetectedWorktreeScanInvalidation' src/main
sed -n '1,180p' src/renderer/src/hooks/worktree-change-refresh-queue.ts
rg -n 'worktreeChangeRefreshQueue|enqueue\\(' src/renderer/src/hooks/worktree-change-refresh-queue.ts

Repository: stablyai/orca

Length of output: 4256


🏁 Script executed:

rg -n 'createWorktreeChangeRefreshQueue' src/renderer
rg -n 'worktreeChangeRefreshQueue' src/renderer/src/hooks/ipc-events/runtime-client-ipc-bridge.ts

Repository: stablyai/orca

Length of output: 2287


🏁 Script executed:

sed -n '1,165p' src/renderer/src/hooks/ipc-events/worktree-event-runtime.ts

Repository: stablyai/orca

Length of output: 7319


Notify observers after successful sparse rollback.

A detected scan can start after the invalidation and cache the temporary worktree before sparse setup fails. removeWorktree clears the SSH provider’s Git-read cache, but not the detected-worktree scan cache. After rollback succeeds, call notifyWorktreesChanged. It invalidates detected scans before sending the change event, which prompts the renderer to fetch the updated worktree list.

🐛 Suggested fix
       } catch (rollbackError) {
         console.warn('[worktree-create] Failed to roll back remote sparse worktree:', rollbackError)
       }
+      if (rollbackSucceeded) {
+        notifyWorktreesChanged(mainWindow, repo.id)
+      }
       if (!rollbackSucceeded && shouldRetireGeneratedName) {
         await retireGeneratedWorktreeName(store, repo, settings, effectiveSanitizedName)
       }

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Incremental re-review of a1dcb9f, the revision that removes the renderer create fence and makes the host the single owner of the rule for the cached local listing and the SSH listing alike. Prior feedback (the direct-SSH fence gap) is covered by the new host witness; the eviction-ordering thread was resolved earlier.

  • Removed the renderer create fence. created-worktree-sequence.ts, its invocation-keyed threading through the refresh/SSH-merge paths, and the diff-purge deference are gone; the client keeps no fence of its own. No dangling references remain.
  • Generalized the re-scan to SSH. scanUntilNotOvertaken re-runs an overtaken scan up to twice through the same thunk, with a new listSshWorktreesWithMutationWitness branch witnessing a mutation against the same scan generation the cache compares.
  • Answers stale past the bound. Instead of a non-authoritative listing that would still replace the client's rows, three overtaken passes return null, mapped to rejected('stale'), leaving client state untouched.
  • Tests. New detected-provider-listing-overtaken-scan.test.ts covers the local and SSH branches (re-run, bound spent, nothing-overtook); the scan-cache, metadata-pruning, and client-characterization suites were updated. All six affected files pass locally.

ℹ️ The host rule reaches the detected listing but not the legacy catalog channels

The re-scan lives in listDetectedWorktreesForCapturedRepo, but worktrees:list/worktrees:listAll still call listDetectedGitWorktrees directly and ignore scan.superseded, and toLegacyDetectedWorktreeResult stamps their bare arrays authoritative: true. Now that the renderer fence is gone, those channels have no mitigation for the same race. Not a blocker: both current preloads expose worktrees.listDetected, so only older/pre-listDetected preloads reach them — but the description's "protects … older clients" claim is slightly broader than the implementation.

Technical details
# Legacy catalog channels bypass the host re-scan

## Affected sites
- `src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts:123` — `worktrees:listAll` reads `scan.gitWorktrees`/`scan.fresh` but not `scan.superseded`.
- `src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts:216` — `worktrees:list` likewise.
- `src/renderer/src/store/slices/worktrees/listing/worktree-host-ownership.ts:281` — `toLegacyDetectedWorktreeResult` wraps the bare array as `authoritative: true`.
- `src/renderer/src/store/slices/worktrees/listing/detected-worktree-provider-request.ts:111` — the renderer falls back to `worktrees.list` only when the preload lacks `listDetected`.

## Required outcome
No action required for this PR. If the bare-array channels are still reachable in supported clients, either route them through the same re-scan or narrow the coverage claim in the description.

## Open question for the human
Are `worktrees:list`/`worktrees:listAll` still consumed by any shipped client, or are they legacy-only?

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Electron QA — create while delete (fixed head a1dcb9f)

Validated head a1dcb9f7ace2408e107a041c4bcf3328a6a3f512 (brennanb2025/fix-claude-agent-workspace). The dev renderer on port 5178 returned 200 for src/renderer/src/main.tsx via /@fs/ under this worktree, and the app identity was this worktree. Fresh profile, hidden launch, Claude set as the default agent, repo qa22311-repo added in the app.

Three runs. Each one created a workspace with Claude, waited until that workspace was selected and its Claude session tab was open, then deleted it from the sidebar (row → Delete → Delete Workspace) and, within 1 second, created the next workspace through the New workspace flow with Claude. The git shim held that create's finalize checkout --no-track for 12 seconds. In every run the shim log shows a git worktree list that started inside that locked window (list sleep 14) and therefore returned after the create. Assertions were taken 30 seconds after the create.

The Claude session tab's chrome title stays "Terminal 1". The pane is Claude (claude --dangerously-skip-permissions) on the workspace trust prompt, not the Landing screen.

Run 1 — delete victim-a1, create survivor-b1 (518ms after the delete confirm)

16:50:13 finalize checkout: locked window open 12s cwd=survivor-b1
16:50:13 list sleep 14
16:50:16 list sleep 14
16:50:25 list sleep 1

The 12s window opened at 16:50:13. Both list sleep 14 lines started inside it, so those listings returned after the checkout hold ended (~16:50:25). The next listing, at 16:50:25, is outside the window (list sleep 1).

  • survivor-b1 is the active workspace and its Claude session tab is open. The Landing screen is not showing.
  • victim-a1 is gone from the sidebar.
  • No red "Worktree scan failed" indicator.
  • The survivor-b1 row is not dimmed (opacity 1, no grayscale, not marked busy).

run 1 full window
run 1 sidebar

Run 2 — delete victim-a2, create survivor-b2 (769ms after the delete confirm)

16:51:52 finalize checkout: locked window open 12s cwd=survivor-b2
16:51:53 list sleep 14
16:51:53 list sleep 14
16:52:05 list sleep 1

Both list sleep 14 lines started one second into the locked window and returned after it. The 16:52:05 listing is after the window (list sleep 1).

  • survivor-b2 is the active workspace and its Claude session tab is open. The Landing screen is not showing.
  • victim-a2 is gone from the sidebar.
  • No red "Worktree scan failed" indicator.
  • The survivor-b2 row is not dimmed (opacity 1, no grayscale, not marked busy).

run 2 full window
run 2 sidebar

Run 3 — delete victim-a3, create survivor-b3 (773ms after the delete confirm)

16:53:22 finalize checkout: locked window open 12s cwd=survivor-b3
16:53:22 list sleep 14
16:53:22 list sleep 14
16:53:34 list sleep 1

Both list sleep 14 lines started inside the locked window and returned after it. The 16:53:34 listing is after the window (list sleep 1).

  • survivor-b3 is the active workspace and its Claude session tab is open. The Landing screen is not showing.
  • victim-a3 is gone from the sidebar.
  • No red "Worktree scan failed" indicator.
  • The survivor-b3 row is not dimmed (opacity 1, no grayscale, not marked busy).

run 3 full window
run 3 sidebar

…overtook, and a headless host bumps the generation it witnesses

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Incremental re-review of e62eac3, which extends the host re-scan rule to the runtime listing that paired web clients and remote-attached desktops read, and makes a headless host bump the generation its listing witnesses.

  • Shared re-scan helper. scanUntilNotOvertaken moved to overtaken-scan-rerun.ts and now returns the final scan instead of null, so each listing chooses its own reply shape; the desktop listing still maps a still-superseded scan to its stale rejection.
  • Runtime listing under the rule. RuntimeManagedWorktreeQueries.listDetected re-runs an overtaken scan up to twice through the runtime's own scan cache; past the bound it publishes the last scan non-authoritatively (source: 'git', no failure reason) and skips both prunes, rather than vouching for rows that omit a worktree created under the scan.
  • Headless generation witness. The runtime constructor registers the worktree-change invalidators, so a create bumps the process-wide scan generation and clears the runtime scan cache even when the desktop ipc/worktrees module never loads.
  • Tests. New orca-runtime-worktree-change-invalidation.test.ts drives the real runtime class without the IPC module; runtime-managed-worktree-queries.test.ts covers the re-run, the published pass's expectation/generation, the bound-spent fallback, and the failed-scan no-rerun.

Verified the past-bound non-authoritative runtime reply is safe on the read paths it feeds: getRemovedWorktreeIdsAfterAuthoritativeScan returns no removals for a non-authoritative result and teardownMissingWorktreeTerminalsBestEffort returns early on !authoritative, so no purge or terminal teardown occurs. The acknowledged residual is limited to a transiently absent sidebar row until the create's own change event refreshes.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

… runtime method instead of casting to a protected one

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: a11362bb-c715-41f2-ab34-6a3615c90c0a

📥 Commits

Reviewing files that changed from the base of the PR and between a1dcb9f and e62eac3.

📒 Files selected for processing (7)
  • src/main/ipc/worktrees/listing/detected-provider-listing.ts
  • src/main/ipc/worktrees/listing/overtaken-scan-rerun.ts
  • src/main/runtime/orca-runtime-worktree-change-invalidation.test.ts
  • src/main/runtime/orca-runtime.ts
  • src/main/runtime/runtime-managed-worktree-queries.test.ts
  • src/main/runtime/runtime-managed-worktree-queries.ts
  • src/main/runtime/runtime-witnessed-worktree-scan.ts

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

// through this runtime's scan cache, so a worktree change must reach both. The desktop IPC
// module registers the generation bump at load; a headless host never loads it.
registerDetectedWorktreeScanInvalidation()
registerWorktreeChangeInvalidator((repoId) => this.invalidateWorktreeCatalog(repoId))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect runtime construction, teardown, and invalidator cleanup.
rg -n -C4 \
  'new OrcaRuntimeService|registerWorktreeChangeInvalidator|unregister|dispose|destroy|shutdown' \
  src docs

Repository: stablyai/orca

Length of output: 45543


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- runtime file ---'
cat -n src/main/runtime/orca-runtime.ts

printf '%s\n' '--- relevant definitions and uses under src/main ---'
rg -n -C5 \
  'registerWorktreeChangeInvalidator|registerDetectedWorktreeScanInvalidation|OrcaRuntimeService|OrcaRuntimeWithResolveWaiter|dispose\(|destroy\(|shutdown\(' \
  src/main

printf '%s\n' '--- runtime file metadata ---'
wc -l src/main/runtime/orca-runtime.ts

Repository: stablyai/orca

Length of output: 45546


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions

Length of output: 32134


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n src/main/runtime/orca-runtime.ts
rg -n -C5 'registerWorktreeChangeInvalidator|registerDetectedWorktreeScanInvalidation|OrcaRuntimeService|OrcaRuntimeWithResolveWaiter|dispose\(|destroy\(|shutdown\(' src/main

Repository: stablyai/orca

Length of output: 45546


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- src/main/runtime/orca-runtime.ts ---'
cat -n src/main/runtime/orca-runtime.ts

printf '%s\n' '--- files defining or importing the invalidator ---'
rg -l 'registerWorktreeChangeInvalidator|registerDetectedWorktreeScanInvalidation' src/main | sort

printf '%s\n' '--- non-test OrcaRuntimeService construction sites ---'
rg -n -C3 'new OrcaRuntimeService' src/main -g '!**/*.test.ts' -g '!**/*.spec.ts'

printf '%s\n' '--- lifecycle methods on the runtime class or its bases ---'
rg -n -C3 'class OrcaRuntime|dispose|destroy|shutdown|close' src/main/runtime/orca-runtime*.ts src/main/runtime/*runtime*.ts -g '!**/*.test.ts'

Repository: stablyai/orca

Length of output: 45539


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n src/main/runtime/orca-runtime.ts
printf '%s\n' '--- invalidator files ---'
rg -l 'registerWorktreeChangeInvalidator|registerDetectedWorktreeScanInvalidation' src/main | sort
printf '%s\n' '--- production construction sites ---'
rg -n -C3 'new OrcaRuntimeService' src/main -g '!**/*.test.ts' -g '!**/*.spec.ts'
printf '%s\n' '--- runtime lifecycle methods ---'
rg -n -C3 'class OrcaRuntime|dispose|destroy|shutdown|close' src/main/runtime/orca-runtime*.ts src/main/runtime/*runtime*.ts -g '!**/*.test.ts'

Repository: stablyai/orca

Length of output: 45531


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- runtime file ---'
cat -n src/main/runtime/orca-runtime.ts

printf '%s\n' '--- invalidator registration symbol ---'
rg -n -C8 'registerWorktreeChangeInvalidator' src/main

printf '%s\n' '--- detected-scan invalidator symbol ---'
rg -n -C8 'registerDetectedWorktreeScanInvalidation' src/main

printf '%s\n' '--- non-test constructions ---'
rg -n -C4 'new OrcaRuntimeService' src/main \
  -g '!**/*.test.ts' -g '!**/*.spec.ts' -g '!**/*test-support*'

Repository: stablyai/orca

Length of output: 29201


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n src/main/runtime/orca-runtime.ts
printf '%s\n' '--- invalidator registration ---'
rg -n -C8 'registerWorktreeChangeInvalidator' src/main
printf '%s\n' '--- detected-scan invalidator ---'
rg -n -C8 'registerDetectedWorktreeScanInvalidation' src/main
printf '%s\n' '--- non-test constructions ---'
rg -n -C4 'new OrcaRuntimeService' src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' -g '!**/*test-support*'

Repository: stablyai/orca

Length of output: 29166


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- main-process runtime construction ---'
sed -n '1,150p' src/main/startup/main-process-runtime-service.ts

printf '%s\n' '--- main-process runtime factory callers ---'
rg -n -C5 'createMainProcessRuntime|main-process-runtime-service|MainProcessRuntime|runtimeService' src/main/startup src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 500

printf '%s\n' '--- orcad construction and entry lifecycle ---'
sed -n '170,270p' src/main/orcad/orcad-entry.ts
rg -n -C5 'startOrcad|runOrcad|createOrcad|start.*Daemon|runtime' src/main/orcad/orcad-entry.ts -g '!**/*.test.ts' | head -n 300

Repository: stablyai/orca

Length of output: 42678


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- main-process runtime construction ---'
sed -n '1,150p' src/main/startup/main-process-runtime-service.ts
printf '%s\n' '--- main-process runtime callers ---'
rg -n -C5 'createMainProcessRuntime|main-process-runtime-service|MainProcessRuntime|runtimeService' src/main/startup src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 500
printf '%s\n' '--- orcad construction and lifecycle ---'
sed -n '170,270p' src/main/orcad/orcad-entry.ts
rg -n -C5 'startOrcad|runOrcad|createOrcad|start.*Daemon|runtime' src/main/orcad/orcad-entry.ts -g '!**/*.test.ts' | head -n 300

Repository: stablyai/orca

Length of output: 42366


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- orcad lifecycle ---'
cat -n src/main/orcad/orcad-lifecycle.ts

printf '%s\n' '--- desktop ready runtime state wiring ---'
sed -n '1,100p' src/main/startup/main-process-ready-runtime.ts
rg -n -C8 'state\\.runtime|initializeReadyRuntimeServices|initializeMainProcessReady' src/main/startup src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 400

printf '%s\n' '--- production startOrcad callers ---'
rg -n -C6 'startOrcad\\(' src -g '!**/*.test.ts' -g '!**/*.spec.ts' -g '!**/*test-support*'

Repository: stablyai/orca

Length of output: 20649


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- orcad lifecycle ---'
cat -n src/main/orcad/orcad-lifecycle.ts
printf '%s\n' '--- desktop runtime state wiring ---'
sed -n '1,100p' src/main/startup/main-process-ready-runtime.ts
rg -n -C8 'state\.runtime|initializeReadyRuntimeServices|initializeMainProcessReady' src/main/startup src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 400
printf '%s\n' '--- production startOrcad callers ---'
rg -n -C6 'startOrcad\(' src -g '!**/*.test.ts' -g '!**/*.spec.ts' -g '!**/*test-support*'

Repository: stablyai/orca

Length of output: 41688


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- invalidator registry and dispatcher ---'
cat -n src/main/ipc/worktree-change-invalidators.ts
rg -n -C6 'worktreeChangeInvalidators|run.*Invalidator|invalidate.*Worktree|worktree.*changed' src/main/ipc src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 500

printf '%s\n' '--- orcad cleanup registration ---'
sed -n '155,185p' src/main/orcad/orcad-entry.ts
sed -n '360,430p' src/main/orcad/orcad-entry.ts

Repository: stablyai/orca

Length of output: 41900


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- invalidator registry and dispatcher ---'
cat -n src/main/ipc/worktree-change-invalidators.ts
rg -n -C6 'worktreeChangeInvalidators|run.*Invalidator|invalidate.*Worktree|worktree.*changed' src/main/ipc src/main -g '!**/*.test.ts' -g '!**/*.spec.ts' | head -n 500
printf '%s\n' '--- orcad cleanup registration ---'
sed -n '155,185p' src/main/orcad/orcad-entry.ts
sed -n '360,430p' src/main/orcad/orcad-entry.ts

Repository: stablyai/orca

Length of output: 41990


Unregister the runtime worktree invalidator during shutdown.

When a runtime is reconstructed in the same process, the module-level invalidator set retains the old runtime through its callback. Later worktree changes invoke both the old and current runtimes. Retain the returned unregister function and call it from the runtime shutdown path.

Comment on lines 145 to 189
@@ -212,7 +183,7 @@ export class RuntimeManagedWorktreeQueries {
})
return {
repoId: repo.id,
authoritative: scan.ok,
authoritative: scan.ok && !scan.superseded,
source: scan.ok ? 'git' : 'metadata-fallback',
worktrees: projectResolvedWorktreeLineage(detected, store.getAllWorktreeLineage?.() ?? {})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

cat src/main/ipc/worktrees/listing/register-detected-worktree-scan-invalidation.ts
rg -n "function invalidateDetectedWorktreeScanCache|const invalidateDetectedWorktreeScanCache|export .*invalidateDetectedWorktreeScanCache|bumpLocalWorktreeScanGeneration|isLocalWorktreeScanGenerationCurrent" src/main/ipc/worktrees/listing src/main/local-worktree-scan-generation.ts
sed -n '1,230p' src/main/ipc/worktrees/listing/detected-worktree-scan-cache.ts
sed -n '40,130p' src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts

Repository: stablyai/orca

Length of output: 14926


🏁 Script executed:

set -e
printf '%s\n' '--- invalidator registry ---'
rg -n -A80 -B20 "runWorktreeChangeInvalidators|registerWorktreeChangeInvalidator" src/main/worktree-change-invalidators.ts src/main -g '*.ts' | head -240
printf '%s\n' '--- local generation ---'
cat -n src/main/local-worktree-scan-generation.ts
printf '%s\n' '--- creation callers ---'
rg -n -A12 -B12 "runWorktreeChangeInvalidators\(repo\.id\)" src/main/runtime src/main/ipc/worktree-remote.ts
printf '%s\n' '--- listDetected caller flow ---'
rg -n -A90 -B30 "scanRuntimeWorktreesUntilNotOvertaken|pruneMetadataMissingFromAuthoritativeLocalScan|authoritative: scan\.ok" src/main/runtime/runtime-managed-worktree-queries.ts

Repository: stablyai/orca

Length of output: 38529


🏁 Script executed:

set -e
printf '%s\n' '--- registry file candidates ---'
fd -t f -i 'worktree-change-invalidators' src
printf '%s\n' '--- registry definitions and usages ---'
rg -n -A70 -B20 "export (async )?function (runWorktreeChangeInvalidators|registerWorktreeChangeInvalidator)|const invalidators|Set<.*Invalidator|type .*Invalidator" src
printf '%s\n' '--- scan helper ---'
fd -t f -i 'runtime-witnessed-worktree-scan' src
rg -n -A140 -B30 "scanRuntimeWorktreesUntilNotOvertaken" src/main/runtime

Repository: stablyai/orca

Length of output: 33182


Preserve the pruning freshness result before publishing authority.

runWorktreeChangeInvalidators(repo.id) synchronously invokes the registered invalidateDetectedWorktreeScanCache callback. That callback advances the same generation captured by scanRuntimeWorktreesUntilNotOvertaken. If creation occurs during the pruning await, the helper returns scanGenerationCurrent: false, but listDetected ignores it and still publishes authoritative: true from the earlier scan.superseded value. The response can therefore mark rows that omit the newly created worktree as authoritative.

Suggested fix
     const scan = await scanRuntimeWorktreesUntilNotOvertaken(store, repo, (target) =>
       this.deps.scanRepo(target)
     )
+    let scanSuperseded = scan.superseded
...
       // so a headless host -- which has no renderer -- never pruned its own repos' rows (`#17776`).
       if (scan.metadataPruneExpectation) {
-        await pruneMetadataMissingFromAuthoritativeLocalScan({
+        const pruneResult = await pruneMetadataMissingFromAuthoritativeLocalScan({
           store: store as unknown as Store,
           repo,
           gitWorktrees: scan.worktrees,
           scan: scan.metadataPruneExpectation,
           scanGeneration: scan.scanGeneration
         })
+        if (!pruneResult.scanGenerationCurrent) {
+          scanSuperseded = true
+        }
       }
...
-      authoritative: scan.ok && !scan.superseded,
+      authoritative: scan.ok && !scanSuperseded,

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Incremental re-review of f9c6e89, a test-only commit on top of the host re-scan work.

  • The change-invalidation suite lists through the public entry point. orca-runtime-worktree-change-invalidation.test.ts now calls runtime.listDetectedManagedWorktrees('id:repo-local') instead of casting the runtime to a protected listDetectedWorktreesForResolvedRepo; the removed RuntimeInternals cast leaves no dangling reference. This is the same method the RPC layer uses (src/main/runtime/rpc/methods/worktree-catalog-methods.ts:47), and it additionally exercises selector resolution before the witnessed scan, so the suite is a closer match to production with no loss of the three assertions (generation moves, cache clears, overtaken scan re-lists).

Verified locally: the suite's 3 tests pass and oxlint reports 0 warnings/0 errors on the changed file. No production behavior changed.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…generation its listing witnesses

A headless host has no window notifier, so a delete (and every other change the runtime announces) never moved the generation the runtime listing re-runs against; a listing the delete overtook published the removed row as authoritative until the next refresh. The bump now rides the runtime's change event, ahead of the send, as the desktop notifier already does.
…n on the host

Past the bound the listing answered without a trace, so continuous create/delete churn could not be diagnosed from a host log. One warning names the repo and the number of overtaken scans; a caller that withdrew stays silent.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status — ready to merge pending CI

Final head: 8ecea401d1af36e0016ac4f3be123e00f7330197.

What changed during review. The PR as opened fixed the race with three pieces: a renderer-side create-sequence fence, a host that published an overtaken scan as non-authoritative with a failure reason, and listable-time cache invalidation. Review found that the non-authoritative publish lit the sidebar's red "Worktree scan failed" indicator and hid external worktree cards on a routine create-during-scan, that the renderer fence had two ordering/eviction defects and did not cover the direct-SSH listing, and that the runtime listing used by paired web clients (and by a desktop client on a remote runtime) still carried the original hazard. The fix was reworked so the host owns it: a scan that a worktree change overtook is re-run (bounded) before it is published, on the desktop cached path, the SSH path, and the runtime path. The renderer sources are byte-identical to main. Details are in the PR body.

Loops. Six same-model review-and-fix loops plus one architecture challenge. Loop 1 fixed the indicator regression and two fence defects; loop 2 replaced the fence with the host re-scan and covered SSH; loop 3 was clean on code; loop 4 extended the rule to the runtime listing and found that a headless host never registered the scan-generation bump at all; loop 5 removed one avoidable test cast; loop 6 was clean and added two hardening commits from the closing readiness scan (the runtime's own change event now bumps the witnessed generation on headless hosts; a spent re-scan bound logs a warning with the repo id).

Readiness checklist ran at the start and the end. Start: two unvalidated P2s (guard-chain bypass in the early return; direct-SSH path uncovered), both resolved by the rework. End: pass, no P0/P1; the two P2s it raised were folded in by loop 6. Counsel validation was skipped, so severities were candidates only.

Electron QA. Three runs on a fresh dev profile with a git shim that holds the create's finalize checkout for 12 s and delays any listing that starts inside that window past the create's return. Each run's shim log shows the race actually happened; in every run the new workspace stayed selected with its Claude pane open, the deleted workspace disappeared, no scan-failed indicator appeared, and the new row was not dimmed. Screenshots are in the QA comment above. That run was captured at a1dcb9f7; the two production commits since only extend the same rule to the runtime listing and add a log line, and the desktop listing's behavior is pinned by the host tests at the final head.

Validation at the final head. pnpm tc clean; changed-lines code-quality gate clean; whole-file oxlint and oxfmt clean on every changed file; targeted vitest 92 files / 816 tests pass; runtime aggregator 1292 pass / 1 skipped. Every fix landed with a test that was ablated red for the stated reason and restored green.

Not exercised. A real SSH host, a headless Linux host, a paired web client, Windows, and Linux. The rule on those paths is pinned by host unit tests, including one that constructs the real runtime class without loading the desktop IPC module.

Known follow-ups (not blocking). A desktop-initiated create or delete sends no runtime client event to paired web clients, so their follow-up listing after a spent bound relies on polling (pre-existing). The folder-rename notification in the runtime's scan-refresh module still bumps nothing on a headless host (that file carries a no-typecheck split header, so it was left for a separate change).

CI on the final head. 21 checks green (typecheck, static analysis, all eight test shards, relay integration, RPC recording pin, e2e build and changed specs, macOS and Windows packaging, root directory guard, verify), 15 skipped by path filters, none failed. At posting time a second verify run and the pullfrog bot were still in progress.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Incremental re-review of 1c6a061 and 8ecea40, the two commits since the last pullfrog review (f9c6e89). Both are small host-side changes on top of the re-scan rule.

  • The runtime's own change event now bumps the generation its listing witnesses. notifyWorktreesChanged runs runWorktreeChangeInvalidators(repoId) before it notifies the window and emits the client event, so a headless host with no window notifier moves the process-wide scan generation and clears the runtime scan cache, matching what the desktop notifier already does. The event still leaves after the last bump, preserving the ordering the runtime listing relies on.
  • A spent re-scan bound is logged on the host. scanUntilNotOvertaken now takes the repo id, checks the bound before mayRescan(), and warns once with the repo id and the number of overtaken scans before answering; a caller that withdrew stays silent.
  • Tests. The removal path is pinned to move the witnessed generation (worktree-removal-and-reconciliation.spec.ts), and notifyBranchRenamed — the runtime's own change event with no notifier attached — is pinned to bump it in orca-runtime-worktree-change-invalidation.test.ts. Both assertions fail without the new invalidator call, so they pin the behavior rather than restating it.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 merged commit 0b6f60a into main Sep 23, 2026
38 checks passed
brennanb2025 added a commit that referenced this pull request Sep 23, 2026
…cannot undo a create

A worktree listing is a snapshot from when its scan began. The renderer treated any
authoritative listing that lacked a known worktree as proof of deletion, judged at apply
time against the live store, so a listing delayed past a create reply purged the new
workspace: tabs wiped, selection cleared to the landing, pending structured launch
tombstoned so the host session was closed the moment it published. #22311 re-runs a scan a
mutation overtakes, which covers a bump during the scan but not a reply that is simply
applied late, on the host or in the renderer, or a refresh that joined an older one.

The host now stamps every listing with the catalog version its scan began at (the existing
per-repo scan generation, scoped by a per-process epoch) and every create and remove reply
with the version the mutation produced. Clients keep the newest version applied per repo
and host; a listing older than that is not applied at all, not its rows, not its purge, not
the pre-merge terminal teardown. Coalesced joiners inherit the reply and therefore the rule.
Fields are optional on the wire; older hosts and clients keep today's behavior.
brennanb2025 added a commit that referenced this pull request Sep 24, 2026
…cannot undo a create (#22507)

* fix(worktrees): version every catalog publication so a stale listing cannot undo a create

A worktree listing is a snapshot from when its scan began. The renderer treated any
authoritative listing that lacked a known worktree as proof of deletion, judged at apply
time against the live store, so a listing delayed past a create reply purged the new
workspace: tabs wiped, selection cleared to the landing, pending structured launch
tombstoned so the host session was closed the moment it published. #22311 re-runs a scan a
mutation overtakes, which covers a bump during the scan but not a reply that is simply
applied late, on the host or in the renderer, or a refresh that joined an older one.

The host now stamps every listing with the catalog version its scan began at (the existing
per-repo scan generation, scoped by a per-process epoch) and every create and remove reply
with the version the mutation produced. Clients keep the newest version applied per repo
and host; a listing older than that is not applied at all, not its rows, not its purge, not
the pre-merge terminal teardown. Coalesced joiners inherit the reply and therefore the rule.
Fields are optional on the wire; older hosts and clients keep today's behavior.

* fix(worktrees): relist after a refused stale listing and stop version churn

- A refused listing can be the only answer a caller gets (a change-event
  refresh that joined an older in-flight listing), so fetchWorktrees lists
  once more; that listing scans at or past the applied version.
- An equal catalog version keeps the held object, so a no-op listing no
  longer writes store state on every refresh.
- Versions the client cannot order are treated as unstamped at ingest.
- worktree.rm takes the repo from its id selector instead of resolving the
  worktree a second time, which also stamped nothing for an id two hosts share.
- A removal on one of two hosts sharing a worktree id records its version.

* test(worktrees): pin the scan-generation bump right after git worktree add on every create path

A listing is stamped with the generation its scan began at, so a create must
advance it before any post-add work can yield. Pins the local desktop, SSH and
runtime local create paths.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(worktrees): bump the scan generation right after git worktree remove

A listing is stamped with the generation its scan began at. Removals bumped it
only at the end, after watcher release, push-target cleanup and the metadata
purge, so a listing scanned before the git removal and one scanned after it
could carry the same sequence. Applied out of order, the older one restored the
removed row until the removal reply. Bump right after the git removal on the
desktop local, desktop SSH and runtime paths, as creates already do.

The ordering pins now witness the generation at the first step after the git
mutation rather than at the re-list, so moving a bump past any intervening
await fails them.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(runtime): stop leftover worker-recovery retries from firing into later tests

The legacy worker terminal recovery retry timer re-arms itself and had no
way to end, so a runtime from one aggregator test kept rescanning repo-1
through the shared listing mock during later tests, consuming the listing a
lineage create expected ("Worktree created but not found in listing").

Give the controller a dispose() that cancels pending retries and refuses to
re-arm, and have the runtime test harness dispose every controller it
constructed after each test.

* Revert "fix(runtime): stop leftover worker-recovery retries from firing into later tests"

This reverts commit 5879110. The leaked
recovery retry timer is a pre-existing test-harness flake that also hits
main; it belongs in its own change, not in the catalog-version fix.

* test(worktrees): pin the relist bound and the teardown gate on fetch-all and paired runtime listings

The relist after a refused stale listing had no test holding it to one retry, and the
pre-merge terminal teardown gate was pinned only on the direct fetchWorktrees path: removing
it from fetchAllWorktrees or from the paired-runtime listing path left every suite green.

* fix(worktrees): keep an SSH reconnect going when its listing is older than an applied create

A direct SSH listing refused because a newer catalog is already applied for
that host reported 'stale', the same result as a moved connection. The
reconnect preparation ends on any 'stale' repo and skips the post-connect
workspace sync and terminal correction, and nothing retries that while the
connection holds. The refused listing now reports the host's answer, since
the store already holds a newer catalog; 'stale' stays for a moved
connection or owner.

* test(worktrees): pin the listing teardown gate on the fetchAllWorktrees startup hydration pass

The hydration pass lists through its own call site, and removing its gate left every suite green.

* fix(worktrees): decide a listing's refusal reason inside the merge, and defer the startup purge behind a newer create

The listing merge now returns 'applied', 'superseded' (a newer catalog is already
applied) or 'not-current' (its connection or repo owner moved), decided against live
state inside the store update. Callers switch on it instead of re-deriving the reason
afterwards, which misreported an owner that went away during an older listing as current.

The one-shot startup purge keeps only ids from scanned rows. A create applied after a
repo's listing but before the purge wrote only the visible rows, so the purge closed the
new workspace's tabs, chat tab included. It now defers when any repo's listing is older
than that repo's applied catalog, like a refused listing, and runs on the next pass.

* test(worktrees): pin the startup purge deferral on a listing refused for a changed repo owner

* test(worktrees): pin a direct-authority fetch reporting an older listing as current

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant