Skip to content

Folder: a mutation waits for this process's own short read instead of failing busy (#272) - #317

Merged
immakermatty merged 1 commit into
mainfrom
agent/LP-272-own-read-wait
Oct 11, 2026
Merged

immakermatty merged 1 commit into
mainfrom
agent/LP-272-own-read-wait

Conversation

@immakermatty

Copy link
Copy Markdown
Contributor

Status: ready for review

Fixes #272.

Why

Since #270 (F36 addendum of 2026-10-08), setup.forget() starts the shell setup cache's background read right away. That read holds the Folder's operation lock (withFolderReadLock). A Folder mutation never waited for anyone (withFolderOperationLock refuses at once). So a person's change right after a profile change, or the "used by agents" toggle after a sign-in, could be answered 400 operation-failed, recoveryMayBeRequired: true although it would have succeeded a moment later.

On 2026-10-11 the same race failed the required Linux CI often enough to block merges:

Details and evidence are in #272.

What changes

This is direction 1 of #272, in src/folder/lock.ts:

  • withFolderReadLock counts its holds per state directory (resolved path) while it holds the lock. The count drops only after the lock is released, so a waiting mutation never sees zero while the read still holds the lock.
  • withFolderOperationLock takes a new optional waitForOwnReadsMs (default 3 s). On a busy refusal it retries only while this process's own short read holds the lock, until that bound.

What does not change

  • The kernel lock stays exclusive.
  • A holder in another process, or this process's own mutation, is still refused at once.
  • No caller changes; every caller gets the default.
  • Reads behave as before (they already waited for each other).

Verification

New tests in tests/folder-lock.test.ts:

  • a mutation waits for an in-process read and runs after it, while any other holder is refused at once (< 500 ms);
  • the wait is bounded: past it the mutation fails as busy, and it goes through at once once the read is done.

bun run check locally: static checks pass. In the test run, one unrelated timing test failed under machine load (guarded-process, "Fixture start timed out" at 3 s); that file passes 3/3 alone. The real gate is this PR's Linux CI. If it is green here, it also unblocks #316.

Risk

A mutation can now take up to 3 s longer when its own Launchpad's background read is slow; before, it failed. Nothing waits on another process.

🤖 Generated with Claude Code

… failing busy (#272)

Since #270 the shell setup cache starts a background read right after a
profile change, under the Folder's operation lock. A person's next change, or
a tools change after a sign-in, then found the lock held and answered
400 operation-failed with recoveryMayBeRequired, although it would have
succeeded a moment later. The same race failed CI often enough on 2026-10-11
to block merges (main's rerun and PR #316, three times).

withFolderOperationLock now waits, at most 3 s, while the holder is this
process's own short read (withFolderReadLock counts its holds per state
directory until the lock is released). Another process's holder, or this
process's own mutation, is still refused at once, so the kernel lock stays
exclusive and a mutation from elsewhere keeps the immediate refusal.

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

@agentrozjedemeai agentrozjedemeai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

APPROVED on bcce02a (base 342db2f). The own-read count starts only after acquisition and ends after release; the mutation retries only a busy lock while that count is positive, preserving immediate refusal for unrelated holders and other mutations. Focused macOS folder-lock tests: 10 passed; exact-head Linux CI static checks and both test shards passed. No unresolved review threads. Non-blocking test hardening: replace the 30 ms startup sleeps in the two new tests with an explicit read-acquired signal to avoid scheduler-dependent assertions under load.

@immakermatty
immakermatty merged commit 1cd5818 into main Oct 11, 2026
7 checks passed
@immakermatty
immakermatty deleted the agent/LP-272-own-read-wait branch October 11, 2026 17:49
immakermatty added a commit that referenced this pull request Oct 18, 2026
…de's own background work, fixed at the cause (#494)

* Folder: a mutation waits for its own read from the moment the kernel grants it

#272 (#317) let a mutation of this process wait for the process's own
short read instead of failing as busy. The read was counted only once
acquireFolderOperationLock returned, after the kernel had already granted
the flock and the read went on verifying and fsyncing the lock directory.
A mutation refused in that window saw no own read and failed at once:
`/api/update`, `/api/tools/update`, `/api/preview` answered 400
operation-failed right after a change whose shell setup re-read had just
started, and a Launchpad start found the Folder lock unavailable. Under a
loaded CI runner (four workers on four vCPUs, ext4 fsync) the window is
long enough to be hit regularly; it was the most frequent flake of the
suite (issue #413).

The read is now counted in the same synchronous step as the kernel's grant
and uncounted when its descriptor is closed; a mutation refused while an
own read ended on the way is retried too. Another process's lock is still
refused at once. The new test starts a mutation at every moment of a read's
acquisition and release; it failed on the old code and passes now.

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

* Tests: loopback ports for configured listeners come from outside the ephemeral range

22 test files picked a listener's port (a handover's listenPort, which the
Launchpad binds itself) by binding port 0 and releasing it. That is an
ephemeral port: until the Launchpad binds it, the kernel may hand the same
port to any other socket, and with four workers per runner opening
connections and port-0 listeners all the time it sometimes did, failing the
start with "Failed to start server. Is port <n> in use?"
(launchpad-organization-settings, environment-browser-view-identity on
#487's runs).

One fixture, tests/fixtures/ports.ts, replaces the copies: ports 20000-31999,
below the Linux (32768-60999) and macOS (49152-65535) ephemeral ranges so
the kernel never hands them out, a range of 1000 per `bun test --parallel`
worker (BUN_TEST_WORKER_ID), never the same port twice in one file, and a
port something already listens on is skipped. Servers that bind port 0
themselves are unchanged.

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

* Launchpad: settled() for the start's background reads; tests observe the request

The Launchpad decides whose Personalspace this is in the background at its
start (a GitHub viewer and repository read) and re-reads the shell setup
after a change. Tests that assert "this request asks GitHub nothing" took
their snapshot of the stub's calls while that start work could still land,
so under load the start's own `viewer` call showed up as the request's
(launchpad-prompts "hosted personal: new-personal-module …", content-routes
"root access (root decision 0201 point 8) …"; issues #413, #425, #490).

The started Launchpad now has settled(): it resolves when those background
reads have ended. No answer waits for it. The tests wait for it before they
count what a request asks, and the work-Environment case also checks that
the start itself asked nothing.

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

* Tests: two fakes wait for what they assert instead of racing it

qualify-launchpad-once: the fake Launchpad printed its start line before it
installed its TERM trap. launchpadOnce stops it as soon as the line and the
page arrive, so on a loaded runner the TERM came first and the shell ended
with 143 instead of 0. The trap is now installed first.

launchpad-vault-page "the installed Launchpad looks at the audience after
its start": the test waited until the fake vault listed no Správce, then
counted the journal's removals. The look removes a member and journals it
after the vault answers, so the second removal could still be unjournaled.
It now waits until both removals are journaled and the vault lists none.

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

* Content: the holder's settled() also waits for a decision a running peek is about to start

The Launchpad's start peeks whose Personalspace this is without waiting.
peek() reads the checkout's layout first and only then starts the decision,
so settled() called meanwhile found nothing being decided and resolved at
once; the start's GitHub viewer call then still landed in the prompts
test's measurement (reproduced in a Linux container with the previous
commit). settled() now first waits for the peeks still reading the layout.
The new test failed on the old code and passes now.

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

---------

Co-authored-by: Claude Opus 5.5 <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

2 participants