[dawsontoth]

⊙ Problem

Kris's review of #3600 found four gaps in its owed-removal retry:

• With threads.count: 0, main is the only worker and serves its apps until restart. The retry never counted main as a holder, so a drop deleted a fork main still served.
• An unreadable root config stopped the retry timer.
• A removal waiting on a worker ran again only on its timer, up to an hour later.
• A crash-left `removal-pending`.partial was never retried and kept the app's branches directory forever.

🤔 Does this belong in one follow-up?

Yes. The first deletes a fork main still serves, and the others leak forks or delay removing them. The fifth finding is an older race that needs a design choice, so it is #3758.

💡 Solution

A drop keeps the forks main serves until the next start. The retry timer survives an unreadable config. A worker's exit or load report triggers a retry. The retry counts a crash-left .partial as owed and clears it before the marker.

⚖️ Alternatives

🤔 The planning review's alternative was adopted

Framing-Verdict: better-alternative-exists (8b40215737fa)

I planned a new config helper. Codex showed readRootConfigEntry rejects malformed files without refreshing config. I used it, removed the .partial before the marker, and added the tests it asked for.

Also considered

• Count main inside poolMayHold. withPoolFor also reads it, and with no workers a pool restart replaces nothing.
• Unload the app from main on drop. Nothing in Harper unloads an application.
• Reuse onThreadExit. It can fire before the worker leaves workers, and it never sees a load report.

🔧 Changes

Part of #3042. Follows up #3600.

• components/operations.ts:
  • Adds mainServesApplication. It is true when main is the worker and its loader placed the app, and it loads the placement lookup once.
  • dropApplication reads it under the lock and checks for serving after keepBranches and before loaded, with its own message.
  • retryBranchRemoval checks the root-config entry and then main. It does not log the parser's message, which can quote the file.
  • The first retryPendingBranchRemovals call registers an onWorkerRelease listener. That listener requests a pass only while the timer is set or a pass is running, and never during shutdown.
• server/threads/manageThreads.ts: adds onWorkerRelease. It is notified from recordLoadedApplications, and at the end of a worker's exit handler, after workers.splice and any auto-restart. A listener that throws is caught and logged.
• resources/branchDatabase.ts: the owed-removal checks also count the marker's temp file, which only a discarding drop writes and a crash can leave unrenamed after the app is gone. clearPendingBranchRemoval removes it durably before the marker.
• components/DESIGN.md: the drop_component note lists the new retry triggers and what each kind of owed removal waits for.

✅ Verification

• Unit (branchRemovalRetry.test.js): five new tests, and each fails on main. They cover:
  • a root config that does not parse, and one that cannot be read;
  • a real HTTP worker's load report, and separately its exit, each waking a pass while the timer is set to an hour, using an idle worker fixture;
  • a crash that left the temp file without its marker.
• Integration (branched-database-package.test.ts, Linux): a threads.count: 0 start loads the package through the real loader. A restart: true drop, then a drop of the absent app, both keep the fork while the app still answers, and the next start removes it. On main, the first drop answers branchesRemoved: true. It does not check the base table, which With threads.count 0, an application's writes to its branched databases land in the base table #3767 shows leaks on main too.
• Full gates in a Linux container, each failure compared by name with main:
  • test:integration:all: 3006 pass, 0 fail (main: 3005, 0).
  • test:unit:main: 3 of 8456 fail, all 3 also on main (nonInteractiveSpawn, and two tests that read the container's LOGGING_LEVEL).
  • test:unit:resources: 1 fails, DerivedIndexRuntime epoch handoff, also failing alone on main 3 of 3.
  • Under LMDB the touched suites pass, with forks skipped.
• PR QA: skipped @ af3c6c0 — dispatch unavailable in this session.

🤖 Generated with Claude Code

_{🤖 Generated by Anthropic Claude (Opus 5.5); posted via @dawsontoth.}

Release target: main — fixes code from #3600, which is only on main (v5.4); not a backport

Related PRs: #3600 overlaps (merged; this follows up its review)
Complexity: moderate

Late Grok findings

3 surviving (highest minor) — Grok reviewed fc4fb33

• minor — unitTests/components/branchRemovalRetry.test.js:140 — the main-serves test stubs componentLoader.applicationsPlacedOnThisThread with sinon, which AGENTS.md:218 rules out. mainServesApplication reads the function through a native import() (components/operations.ts:2080), and that import keeps its own copy of the CommonJS exports. There are two
• nit — unitTests/components/fixtures/idleWorker.js:3 — new comments describe the code next to them: idleWorker.js:3, branchRemovalRetry.test.js:139 and :158. The comment at resources/branchDatabase.ts:867 explains why the partial file is removed before the marker, which is allowed. The rest of the Gemini group is folded in here
• nit — components/operations.ts:2078 — mainServesApplication calls import('./componentLoader.ts') each time it runs. It only runs on the drop and retry path, never per request, and after the first load the import resolves from the module cache. So the cost is negligible; hoisting the import is optional

Not standing: 1 merged. Grok ran 20m (20m from launch to report); the fold adjudication (cursor-grok@fc4fb3315d45-2941044b, claude) took 1m.

Deferred findings

• 205923d1d2d513b0 nit data-integrity — resources/branchDatabase.ts — crash-left .partial from a Retry a branched database removal that drop_component could not finish, instead of only logging the failure #3600-only build revives removal intent a keepBranches drop cancelled — filed A crash-left branch-removal temp file from a pre-#3763 dev build could revive a cancelled removal #3773

Late Grok findings

none surviving — Grok reviewed af3c6c0

No finding survived adjudication.

Not standing: none. Grok ran 18m (18m from launch to report); the fold adjudication (cursor-grok@af3c6c00d994-cc79f5d5, claude) took 1m.

_{Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex,cursor-grok; late=cursor-grok:adjudicated; adjudicated=domain; declined=muse; rounds=7; full=1 @ af3c6c0}

_{Review-Attention: study ~20m (critical: branchDatabase.ts; hot: manageThreads.ts; decisions: temp-file-counts-as-debt, shutdown-suppresses-wakeups-only, main-serves-waits-for-restart, unreadable-config-polls, do-less-alternative) @ af3c6c0}

[gemini-code-assist]

Code Review

This pull request introduces support for retaining and cleaning up branched database storage when the main thread acts as the sole worker (i.e., when threads.count is 0). It adds mechanisms to retry pending branch removals upon worker exits or load reports, and updates the cleanup process to handle partial marker files. The review feedback identifies two critical issues: first, a test simulating a crash during marker writing is flawed because the final marker is not deleted, masking a leak; second, applicationsWithPendingBranchRemoval must be updated to check for .partial files to prevent directories from being leaked permanently if a crash occurs mid-write.

[kriszyp]

_{🤖 Reviewed with Muse}

[kriszyp]

[nit] Worker-release wake-ups depend on an implicit timer invariant; document that every timer-worthy debt leaves a timer armed to prevent future retry loss