Skip to content

Remove a fork that a still-loading worker creates after drop_component looked - #3818

Draft
dawsontoth wants to merge 5 commits into
mainfrom
claude/late-fork-census-3758
Draft

dawsontoth wants to merge 5 commits into
mainfrom
claude/late-fork-census-3758

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor
A badger plants a flag in an empty patch it has swept while a sleepy mole is still on its way to dig there

⊙ Problem

drop_component checked disk to decide whether to remove a fork. A worker still loading the application could create its first fork after the drop's last look. That fork had no marker. The absent application was never listed again, so the fork stayed, unlogged, for a same-name redeploy to adopt.

🤔 Does a rare race need a fix?

Yes. It needs a load running during the drop, but a preview's private data must not outlive its drop, and nothing logs it.

💡 Solution

A drop that discards branches now records the removal before changing anything if a fork could still appear. This applies when the root config declares branches or any thread may hold the application. The retry already waits for every holder, so the marker covers a late fork. A marker that covered nothing clears the same way.

⚖️ Alternatives

🤔 Why a durable marker instead of the recheck the issue suggested?

Framing-Verdict: better-alternative-exists (6b1407a5d94f)

I planned the issue's option 3, an in-memory recheck on each worker release. The planning review showed the retry already keeps an empty marker while anything may hold the application, so the marker is cheap. Unlike the recheck, it survives a crash of main or a drop that fails partway.

🤔 Is a marker for an application that never forked acceptable?

Yes. A plain shared application dropped without a restart keeps an empty marker and retries less often until the pool restarts. Nothing is deleted wrongly. Checking only unreported workers missed loads deferred past a report. Only a fork or a declaration can block a drop, so a full disk never blocks dropping a plain application.

Also considered

  • Refuse the fork in prepareBranches: races the drop unless every boot takes the component lock.
  • Log unmarked directories at start: a keepBranches drop leaves that state on purpose.

🔧 Changes

Fixes #3758. Part of #3042.

✅ Verification

  • Unit (branchRemovalRetry.test.js, a real worker thread): nine new tests; five fail on main (late fork, stale declaration, deferred load, declared drop that cannot record, drop failing after removal). This harness puts the test and default databases in different roots, so the late-fork case also covers a fork outside its marker's root.
  • Integration (branched-database-package.test.ts): dropping an application no worker loaded records and clears its removal, counted through the retry's log line.
  • Full gates (Linux, at ad291755b): unit main and resources fail the same 4 tests on head and base; integration is 3086/0 against 3085/0. Later commits: targeted suites only.
  • Not covered: a real load reaching prepareBranches after the drop's last look; the harness cannot pause a worker there. Also, hadWorker and mainServes alone.
  • Known limits (review minors): a power loss can lose the marker when the default storage root was missing two or more segments, or after an earlier failed fsync; a failed speculative write leaves a stale-config late fork unrecorded.
  • PR QA: skipped @ 7779da4 — dispatch unavailable in this session.

🤖 Generated with Claude Code

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

Release target: v5.4 — fixes code from #3600 and #3763, which is only on main (v5.4); not a backport
Related PRs: #3763 overlaps (merged; this builds on its retry and worker-release wake-ups)
Complexity: moderate

Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex,cursor-grok; late=cursor-grok:adjudicated; adjudicated=domain; declined=muse; rounds=6; full=3 @ 7779da4

Review-Attention: study ~13m (critical: branchDatabase.ts; decisions: speculative-marker-fail-open, conservative-census-for-unbranched, fallback-marker-location, unreadable-config-fails-closed) @ 7779da4

dawsontoth and others added 5 commits October 30, 2026 14:01
A worker still loading an application can create its first fork after
drop_component's last look at the disk. No marker covered that fork and
nothing listed the absent application again, so it stayed on disk with
nothing logged.

A discarding drop now records the removal before it changes anything
whenever the root config declares branches, even with none on disk yet,
in the default database's branches. One marked directory stands for all
of an application's, and the retry already waits for every worker that
may hold the application before it removes anything or clears the
marker. So a late fork lands under a debt that survives a crash of main
and a drop that fails after the application is gone, with no new state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…has not reported

A worker loads from its own view of the root config, which can still
declare branches the file no longer does, so the declaration alone could
miss a fork. A worker that has not reported its load is the only one that
can still create one, so its presence now records the removal too.

The marker write flushes the entries naming its directory every time
rather than only when it creates them, since an earlier drop that failed
can have created the directory without that, and drops the separate
directory helper. The end-to-end test counts the removal log line rather
than matching one an earlier test may have left.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d loads included

A worker reports an application placed before a load deferred behind its
preparation lock has run, so a reported worker can still create the first
fork. The drop now records the removal whenever the pool may hold the
application or a dedicated worker for it remains, the same checks its
retention already makes, instead of only while a worker has not reported.

The marker write also flushes the entry of the topmost directory it
created, in case that is the storage root itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With threads.count 0 main is the only worker, and a load still running
there can create the first fork after the drop looked, with no pool or
dedicated worker for the drop to see.

The retry now logs that it finished a removal rather than that it removed
storage, since a marker recorded in case a fork appeared may have had none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every drop of a loaded shared application now writes under the default
database's storage, so a full or read-only volume would have refused the
drop of an application with no branches at all, where before it touched
no database storage. Only a fork on disk or a declaration refuses the
drop now; for the rest a failed record is logged and the drop goes on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dawsontoth dawsontoth added this to the v5.4 milestone Oct 30, 2026

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request ensures that dropping an application with no active fork on disk (but which a worker might still be loading or could potentially fork) correctly records a pending branch removal. It introduces the declaresBranches helper to check the root configuration, speculatively records pending removals under certain conditions (e.g., if a worker pool may hold it), and adds robust integration and unit tests to verify these scenarios. It also ensures directory creation and fsyncing are properly handled when recording pending removals. No review comments were provided, so there is no feedback to address.

This branch has not been deployed

No deployments
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.

A fork that a still-loading worker creates after drop_component's last census is never removed

1 participant