fix: reclaim stale local feature locks

[goutamadwant]

Closes #168

What Problem This Solves

Fixes an issue where users who restarted clawpatch review after a killed run would remain blocked by a stale per-feature lock when the recorded process was already gone on the same host.

Why This Change Was Made

The change reclaims only local dead-PID feature locks before claiming a feature, and adds clean-locks --stale-only for the same conservative cleanup path. Locks from live local processes or other hosts are preserved, and the existing full cleanup behavior remains available through clean-locks.

User Impact

Users can recover interrupted review runs without manually deleting lock files, while active or remote review work remains protected from accidental cleanup.

Evidence

Real CLI behavior proof from a disposable local fixture:

$ pnpm build
Result: passed.

$ node dist/cli.js --root <fixture> --json init
Result: initialized a git-backed fixture project.

$ node dist/cli.js --root <fixture> --json map --source heuristic
{
  "features": 6,
  "source": "heuristic",
  "usedAgent": false
}

Before cleanup:
[
  {
    "title": "CLI command stale",
    "status": "claimed",
    "lock": "stale-run",
    "pid": 0,
    "host": "same-host"
  },
  {
    "title": "CLI command live",
    "status": "claimed",
    "lock": "live-run",
    "pid": "<current process>",
    "host": "same-host"
  },
  {
    "title": "CLI command remote",
    "status": "claimed",
    "lock": "remote-run",
    "pid": 0,
    "host": "remote-host"
  }
]

$ node dist/cli.js --root <fixture> --json clean-locks --stale-only
{
  "cleared": 1,
  "lockFilesCleared": 1
}

$ node dist/cli.js --root <fixture> --json status
{
  "activeLocks": 2,
  "lockFiles": 2,
  "features": 6
}

After cleanup:
[
  {
    "title": "CLI command stale",
    "status": "pending",
    "lock": null
  },
  {
    "title": "CLI command live",
    "status": "claimed",
    "lock": "live-run"
  },
  {
    "title": "CLI command remote",
    "status": "claimed",
    "lock": "remote-run"
  }
]

Focused and broader validation:

• pnpm test src/workflow.test.ts
  • Result: 1 test file passed, 123 tests passed.
• pnpm typecheck
  • Result: passed.
• pnpm lint
  • Result: passed.
• pnpm format:check
  • Result: passed.
• pnpm test
  • Result: 29 test files passed, 897 tests passed, 1 skipped.
  • Note: run with Node v24.16.0 to satisfy the repository engines.node >=22 requirement.
• pnpm build
  • Result: passed.

Commit verification:

• GitHub reports the head commit signature as verified.

Disclosure: AI was used to understand the codebase and review the fix.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 1:44 AM ET / 05:44 UTC.

ClawSweeper review

What this changes

This PR reclaims only dead same-host feature locks during claims, adds clean-locks --stale-only, serializes cross-process lock mutation with proper-lockfile, and documents the conservative recovery behavior.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

The repaired PR head directly addresses the prior stale-unlink race: lock claims and stale cleanup now use the same per-feature filesystem mutation boundary, and the branch includes a focused replacement-lock regression test. The PR is still necessary because current main has no stale-lock reclamation; it is not yet in a release. The submitted terminal proof and the follow-up maintainer validation are sufficient, and no remaining actionable patch defect was found.

Priority: P1
Reviewed head: 28382ac60a375332a2c9b23c83fcfb2ca18ac6b8

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Strong real CLI proof, a focused race repair on the final head, and targeted regression coverage support a high-confidence merge-ready review.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body and the August 2, 2026 maintainer follow-up show after-fix built-CLI recovery from a killed review, conservative stale-only cleanup, and the replacement-lock race regression; terminal output should remain redacted if reposted.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body and the August 2, 2026 maintainer follow-up show after-fix built-CLI recovery from a killed review, conservative stale-only cleanup, and the replacement-lock race regression; terminal output should remain redacted if reposted.
Evidence reviewed	5 items	Current main lacks stale-lock recovery: The current default-branch claimFeature writes the lock file with exclusive creation and immediately treats an existing file as a conflict; it does not inspect hostname or PID, so the linked killed-run scenario remains unresolved on main. Shared mutation boundary removes the reported race: The PR head wraps both claiming and stale-only cleanup in withFeatureLockMutation; reclaim re-reads both the feature record and file lock inside that boundary before deleting either representation. Race regression coverage: The final branch adds an interleaving test that ensures a delayed stale reclaimer cannot remove a replacement live lock, alongside live-local and remote-owner preservation tests and stale-only command coverage.
Findings	None	None.
Security	None	None.

How this fits together

Clawpatch review workers claim a feature record plus a matching .clawpatch/locks file to prevent concurrent review runs from processing the same feature. The state layer receives claims and cleanup requests, determines whether both recorded owners are provably stale on the current host, and then either preserves the lock or atomically clears and replaces it.

flowchart LR
  A[Review or cleanup command] --> B[Feature record and lock file]
  B --> C[Per-feature mutation lock]
  C --> D[Re-read both lock records]
  D --> E{Both owners stale locally?}
  E -->|Yes| F[Clear stale state]
  E -->|No| G[Keep live or remote owner]
  F --> H[Claim feature or report cleanup]
  G --> H

Loading

Before merge

• Resolve merge risk (P1) - This changes persisted feature-lock coordination; merging without preserving the shared mutation boundary could let a stale cleanup delete a newly claimed live lock. The final head’s regression test and maintainer evidence address that risk.
• Complete next step (P2) - No repair lane is needed: the prior P1 atomicity finding is fixed on the final PR head, and the remaining action is normal maintainer merge review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed surface	10 files; 610 added, 27 removed	The repair combines state coordination, CLI behavior, 326 added test lines, documentation, and a pinned runtime dependency.
Lock safety coverage	4 focused behavior classes	The tests cover dead local recovery, replacement-lock interleaving, live/remote preservation, and stale-only cleanup.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #168
Summary: This PR is the active candidate fix for the stale local feature-lock failure reported in the linked issue.

Members:

• canonical: Locks from killed runs are never reclaimed, permanently blocking a feature from review #168 - The PR explicitly closes the issue and implements its requested dead same-host PID recovery and conservative cleanup behavior.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Merge the serialized recovery implementation (recommended)
   Retain the final per-feature mutation boundary and its replacement-lock regression test, which together prevent stale cleanup from deleting a live replacement lock.
2. Pause if filesystem-lock dependency policy differs
   If maintainers do not want proper-lockfile as a runtime dependency, pause this branch and choose an equivalent repository-supported cross-process coordination primitive before landing.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Require the final lock-replacement regression test and preserve same-host dead-PID-only cleanup semantics.

Technical review

Best possible solution:

Merge the final serialized implementation after normal maintainer review, retaining the race regression and the conservative same-host-only rule; this resolves the linked lock-recovery issue without changing remote-lock behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue gives a concrete killed-review reproduction, and the PR includes after-fix built-CLI runs plus a focused interleaving regression for the stale-reclaim race.

Is this the best way to solve the issue?

Yes. Reclaiming only locks whose hostname matches and whose PID is dead, while serializing claims and cleanup around both stored representations, is the narrowest safe repair for the established lock contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4a5028c27f01.

Labels

Label justifications:

• P1: A killed review can leave one feature permanently blocked and fail later review runs until an operator manually intervenes.
• merge-risk: 🚨 session-state: The diff changes durable feature-lock records and the cross-process protocol that decides when they may be cleared.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and the August 2, 2026 maintainer follow-up show after-fix built-CLI recovery from a killed review, conservative stale-only cleanup, and the replacement-lock race regression; terminal output should remain redacted if reposted.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body and the August 2, 2026 maintainer follow-up show after-fix built-CLI recovery from a killed review, conservative stale-only cleanup, and the replacement-lock race regression; terminal output should remain redacted if reposted.

Evidence

What I checked:

• Current main lacks stale-lock recovery: The current default-branch claimFeature writes the lock file with exclusive creation and immediately treats an existing file as a conflict; it does not inspect hostname or PID, so the linked killed-run scenario remains unresolved on main. (src/state.ts:99, 4a5028c27f01)
• Shared mutation boundary removes the reported race: The PR head wraps both claiming and stale-only cleanup in withFeatureLockMutation; reclaim re-reads both the feature record and file lock inside that boundary before deleting either representation. (src/state.ts:99, 28382ac60a37)
• Race regression coverage: The final branch adds an interleaving test that ensures a delayed stale reclaimer cannot remove a replacement live lock, alongside live-local and remote-owner preservation tests and stale-only command coverage. (src/workflow.test.ts:1720, 28382ac60a37)
• Follow-up repair provenance: The final head commit was authored after the previous review’s atomicity finding and is explicitly titled fix(locks): serialize stale lock reclamation. (src/state.ts:106, 28382ac60a37)
• Not yet released: The latest recorded release is v0.7.1 at f6e00010cc2a97747c73c28179e951e807d9b23a, while the candidate fix is only on the unmerged PR head; this is not an implemented-on-main close. (CHANGELOG.md:3, f6e00010cc2a)

Likely related people:

• Peter Steinberger: Authored the final lock-serialization repair and documented real CLI validation in the PR discussion. (role: recent area contributor; confidence: high; commits: 28382ac60a37, f6e00010cc2a; files: src/state.ts, src/workflow.test.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (25 earlier review cycles; latest 8 shown)

• reviewed 2026-08-01T07:11:02.507Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale lock reclamation with new claims
• reviewed 2026-08-01T09:35:11.435Z sha df92a88 :: needs changes before merge. :: [P1] Make stale-lock reclamation atomic
• reviewed 2026-08-01T11:32:43.828Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale-lock reclamation with new claims
• reviewed 2026-08-01T12:44:37.594Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale-lock reclamation with new claims
• reviewed 2026-08-01T14:27:20.791Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale-lock reclamation with new claims
• reviewed 2026-08-01T20:13:35.646Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale-lock reclamation with new claims
• reviewed 2026-08-01T22:37:14.326Z sha df92a88 :: needs changes before merge. :: [P1] Serialize stale-lock reclamation with new claims
• reviewed 2026-08-02T01:42:28.660Z sha df92a88 :: needs changes before merge. :: [P1] Make stale-lock reclamation atomic

[steipete]

I reproduced the cleanup/claim race reported by ClawSweeper and repaired it on this branch while preserving the contributor commit.

The final branch now serializes claim and stale-reclaim mutations across processes, re-reads both persistent lock representations inside that boundary, and conservatively preserves a feature whenever either representation is live or belongs to another host. It also documents the automatic reclaim and stale-only cleanup behavior.

Proof on the final branch:

• formatting, type-checking, linting, production build: pass
• full test suite: 898 passed, 1 skipped
• regression: a delayed stale reclaimer cannot delete a replacement live lock
• built CLI: killed a real review after it acquired a lock; a fresh review automatically reclaimed it and completed
• built CLI stale-only cleanup: removed one dead same-host lock while preserving one live same-host lock and one foreign-host lock; after killing the live process, a second cleanup removed that lock and continued to preserve the foreign-host lock
• source-blind behavior validation: 5 passed, 0 failed, 0 blocked
• independent full-branch autoreview: no actionable findings, 0.98 confidence
• secret scan and public model-identifier gate: clean

Dependency note: the coordination layer uses proper-lockfile 4.1.2. It is mature and widely adopted; its atomic-directory locking and stale-owner recovery match this filesystem coordination boundary.

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: reclaim stale local feature locks This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.