fix(core): reclaim stale metadata locks and report contention as busy

[BrainerVirus]

What

• .workit/metadata.lock is now reclaimed automatically when its owner is provably gone:

  • the pid is not running,
  • the pid now belongs to a different process (the /proc start time differs),
  • the lock comes from another host and is older than 10 min,
  • the lock is unreadable and older than 30 s.

  An abandoned metadata.lock.reclaim guard older than 30 s is also cleared.

• When a live process holds the lock, a write retries with jittered backoff for up to 2 s. After that it returns a new retryable error code, busy, never recovery_required. A writer that loses a reclaim race retries instead of failing.

• recovery_required still comes back for real state damage: corrupt or foreign records, an invalid workspace binding. The explicit state.recover protocol keeps its fail-closed lock semantics.

• workit doctor has a new workspace_lock check. It warns on a stale lock or an abandoned guard. workit doctor --fix-lock clears both and never removes a live or unverifiable lock.

• The bootstrap now tells agents to retry busy and points them to --fix-lock. README, AGENTS.md and CHANGELOG are updated.

New module: packages/workit-core/src/core/store-lock.ts. It holds the lock schema and parser (moved out of task-store.ts), the owner classifier, and the inspect/clear helpers that doctor uses.

Why

The architecture audit (§6 #1–2, §7.2 #1) found two problems:

• A dead-pid lock bricked the store permanently. The lock options were shouldReclaim: () => false and staleRecovery: "fail-closed", nothing could clear the lock, and AGENTS.md forbids deleting it by hand.
• Ordinary contention (timeoutMs: 0, retries: 0) was reported as recovery_required, which the bootstrap treats as a stop condition.

Given/When/Then covered (test/workit-core/store-lock.test.ts, test/workit-cli/doctor.test.ts)

• Given a lock held by a dead pid, When a write runs, Then the lock is reclaimed and the write succeeds
• Given a lock whose pid now belongs to a different process, When a write runs, Then the lock is reclaimed and the write succeeds (Linux)
• Given a lock from another host older than the TTL, When a write runs, Then the lock is reclaimed and the write succeeds
• Given a fresh lock from another host, When a write runs, Then it returns busy and keeps the lock
• Given a lock held by a live writer, When another write runs, Then it returns retryable busy, not recovery_required, and the lock stays
• Given a live writer that releases its lock within the retry window, When another write runs, Then the write succeeds
• Given an abandoned reclaim guard older than the TTL, When a write runs, Then the guard is cleared and the write succeeds
• Given a corrupt task record, When a write runs, Then recovery_required still surfaces
• Given three processes each making 40 writes to one task, When they contend, Then none returns recovery_required. Every result is ok or busy: under full-suite load one run saw 117 ok and 3 busy, so the follow-up commit d6ece39 accepts busy there.
• Given a stale lock left by a dead pid, When workit doctor runs, Then it warns and names --fix-lock
• Given a stale lock and an abandoned reclaim guard, When workit doctor --fix-lock runs, Then both are cleared
• Given a lock held by a live process, When workit doctor --fix-lock runs, Then the lock is kept

Existing tests updated, none deleted:

• task-store.test.ts: the "leftover locks stay inspectable" test asserted the old bricking behavior. It now asserts that reads stay inspectable and that the write reclaims the expired foreign lock.
• reliability-report.test.ts: doctor check counts go from 20 to 21 for the new check.
• install-scripts.test.ts: the stub copies store-lock.ts, which doctor.ts now imports.

Verification (measured, Node 24.20.0, Bun 1.3.14, in the worktree)

• Red before the fix: 8 of the 9 new store-lock tests failed. The in-process stress test returned 76/120 recovery_required.

• Green after: 9/9 store-lock tests, 23/23 task-store, 6/6 CLI doctor, 4/4 reliability-report, 17/17 install-scripts.

• End to end through the built CLI (node packages/workit-cli/dist/index.js task progress, 3 processes × 40 calls):

  Build	ok	recovery_required	revision_conflict
  origin/main	106	9	5
  This branch	104	0	16

• bun run lint ✅, bun run format:check ✅, bun run typecheck ✅, bun run knip ✅ (only the existing configuration hints), bun run build ✅.

• Full bun test (run on d636352): 1583 pass / 1 fail. The one failure, AR-14: negative fixtures never leak raw git usage… in test/workit-core/doctor.test.ts, already fails on origin/main. Its repoRoot resolves to the parent of the checkout, so it runs bun test across sibling worktrees. I ran it on an origin/main worktree and it failed there too. It is not touched here.

Review fixes (commit 61ffbc9)

An independent adversarial review (kill -9 stress, containers) found these. All fixed:

1. Containers sharing the hostname. The lock's host is now hostname#<pid-ns inode>:<boot_id> on Linux. It is folded into the existing string, so older strict schemas still parse it. A lock from a different pid namespace or boot, or one written by an older Workit, is treated as foreign: TTL rules only, and its pid is never checked against this host's process table. Test: a pid 1 lock with a foreign namespace suffix returns busy while fresh and is reclaimed past the TTL. A legacy plain-hostname lock is also handled by TTL only.
2. doctor --fix-lock race. Clearing now holds the same .reclaim guard that writers take. It skips when a fresh guard exists and re-checks the bytes just before unlinking. I ported the reviewer's doctor-race.ts as a test (doctor preempted 400 ms before the rm while writer W reclaims): the critical sections of the two writers never overlap.
3. Event-loop freeze. The default wait budget is now 250 ms, which applies to in-process hosts (OpenCode, MCP, Pi). The CLI raises it to 2 s for its own process with setDefaultLockTimeout. A retry after a lost reclaim race uses the remaining budget instead of starting a new one.
4. Start time and escape hatch. The /proc/<pid>/stat start time is parsed after the last ), so a process name with spaces or parentheses no longer shifts it (unit test). macOS and FreeBSD use ps -o lstart=. doctor --fix-lock --force prints the holder and refuses without --yes or a TTY confirmation. --fix-lock and the doctor lock check honour WORKFLOW_WORKSPACE_ROOT, like task commands (CLI tests).
5. Bootstrap. It now says that a revision_conflict on a call without expectedRevision is retryable: re-read and retry. Revisions are still checked against the pre-lock read; a bounded engine retry is left for a follow-up slice.

Measured after the fixes (Node 24.20.0):

• Reviewer orchestrate.sh: 4 workers for 30 s, 118 kill -9s, plus a clearStaleMetadataLock loop (1.18M calls, 28 stale locks cleared). Results: 831 ok, 186 busy, 66 revision_conflict, 0 critical-section violations, 0 recovery_required, and the final write succeeded.
• Reviewer doctor-race.ts, with the lock written in the new identity format: the lock still existed while the doctor held the guard, W returned retryable busy, and X wrote afterwards with no overlap. The reviewer's original script writes a legacy plain-hostname lock, which is now foreign and TTL-only, so the doctor correctly leaves it alone.
• Event-loop gap in-process with a live holder: 256 ms, down from the reviewer's 2020 ms.
• store-lock 14/14, CLI doctor 8/8. lint, format:check, typecheck and knip pass. Full bun test: 1590 pass / 1 fail (AR-14, which also fails on main).

Re-review fixes (commit 33d8426)

• Regression test for the original theft path. A plain-hostname() lock naming live pid 1 with a mismatched start time (a container's lock written before namespace identity) stays busy and is not reclaimed. Measured: this test fails on d6ece39 and passes now.
• Reboot. A lock from the same host and pid namespace but a different boot_id is stale at once, with no 10-minute wait after a crash plus reboot. Measured: the test fails on d6ece39 and 61ffbc9 and passes now.
• Doctor. workspace_lock warns when an unverifiable lock (another host, another pid namespace, or a legacy lock) has blocked writes for more than 30 s, and gives the fix workit doctor --fix-lock --force --yes. A fresh lock of the same kind still passes. Measured: the test fails on 61ffbc9 and passes now.
• 8d3e7b5: the plain-hostname container test now runs only where locks carry namespace identity (Linux). On macOS and Windows a plain hostname is the host's own lock format, which is why that test failed there in CI.
• 591f521 merges origin/main to resolve a task-store.ts conflict with the task index from perf(opencode): task index and cached, capture-free per-turn context #149. Main's index code is kept, and the lock schema stays in store-lock.ts. After the merge: bun run test 1204/0, test:packaging 247/0, lint, format:check, typecheck and knip pass, and CI is 15/15 green.

Known limits / follow-ups

• revision_conflict still shows up under contention when the caller omits expectedRevision. The engine reads the revision before it takes the lock, so another writer can win in between. This conflict is retryable and is not recovery_required. Fixing it means resolving revisions under the lock in the engine, which is out of scope here.
• Writes block the calling thread while a live holder keeps the lock (sync retry): up to 250 ms in-process and 2 s in the CLI. Before this change they failed immediately. In-process nested acquisition still fails fast.
• Start time comes from /proc on Linux and ps -o lstart= on macOS/FreeBSD; elsewhere reclaim relies on pid liveness alone. doctor --fix-lock --force --yes is the explicit escape hatch.
• After upgrading, a lock written by an older Workit is honoured for up to 10 min (TTL), because it carries no namespace identity.

🤖 Generated with Claude Code

[github-actions]

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

• v2.2.0
• GitHub release

Your semantic-release bot 📦🚀