[t0saki]

Description

plugin-tests has been failing on nearly every PR with concurrent stale takeovers leave exactly one holder — 2 !== 1. It is a real bug in withSessionLock, not a flaky test.

withSessionLock aged a lock by the lock directory's mtime, but a taker that wins a takeover stamps the owner file before refreshing that mtime. A racer that lost the stamp retried immediately, still read the old directory mtime, judged the lock stale again, and claimStaleLock renamed the live winner's stamp aside to install its own — two holders in the critical section at once. On Linux the retry reliably lands inside that window; on macOS the timing happens to miss it, which is why it never reproduced locally.

Human Involvement

• A human participated in the implementation or review loop

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Changes Made

• Age a lock by its own owner stamp, falling back to the directory only while no stamp exists (a holder killed between mkdir and its write still ages out).
• Settle takeovers with a claim directory derived from the exact state the taker read: everyone who observed the same dead lock contends for one name via an atomic mkdir, exactly one wins, and the losers re-read a lock that now carries the winner's fresh stamp. Neither the directory nor the stamp is ever momentarily absent, so no racer can claim alongside the taker.
• Heartbeat refreshes the stamp as well as the directory.
• Release clears the lock recursively, so a claim left behind by a taker that died mid-takeover cannot keep the lock standing with a fresh mtime for a full staleMs.
• New regression test for that abandoned-claim path.

Testing

• I have added tests that prove my fix is effective
• New and existing unit tests pass locally with my changes
• macOS (the full CI plugin-tests command: 409 tests, 408 pass, 0 fail, 1 skipped)

Stress harness with 8 racers over 30 rounds on a stale lock: 4 concurrent holders before the fix, 1 after.

Additional Notes

Not specific to any one PR — the bug is on main, so every open PR picks up the failure.

[Copilot]

🟡 Changes recommended

The new claim-directory takeover flow still has a race that can allow overwriting a live winner’s stamp (and can throw on mid-takeover deletion) unless it re-validates the observed lock generation under the claim.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes a real race in withSessionLock that could allow concurrent “stale lock takeovers” to result in multiple holders entering the critical section, and adds regression coverage for an abandoned takeover-claim scenario.

Changes:

• Switch lock staleness aging to prefer the owner stamp mtime (fall back to directory mtime only before stamping exists).
• Rework stale takeover arbitration to use an atomic per-generation claim directory rather than manipulating the owner file directly.
• Refresh the owner stamp on heartbeat and make lock release recursively remove the lock directory (including leftover claim dirs), with a new regression test.

File summaries

File	Description
examples/codex-memory-plugin/scripts/session-state.mjs	Updates lock staleness and takeover mechanics; refreshes owner stamp and improves cleanup semantics.
examples/codex-memory-plugin/scripts/session-state.test.mjs	Adds regression coverage for abandoned claim directories not wedging the lock.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ZaynJarvis]

lgtm