Skip to content

poteto-mode orch store: stale-lock takeover can delete a live lock and admit two writers #1055

Open

Description

@MaxHarris-1

Found against pstack 0.15.34 at cursor/plugins 519d36aa98955bf680db01bcde5fdfba686dc901 (harness/pstack/claude-code/skills/poteto-mode/). Line numbers refer to that commit.

scripts/orch/store.ts (acquireLock, around :385-430) recovers a stale lock like this:

  1. open(path, "wx") fails with EEXIST.
  2. It reads the holder PID, and holderIsDead(holder) is true.
  3. takeOver() calls unlink(path) unconditionally (:394), then create() again.

Between steps 2 and 3, another process can take over the same stale lock and create its own lock file. The slower
process then unlinks that new, live lock and creates its own. Both believe they hold the lock. The same applies to
--force (options.force). Every store write is a whole-file read-modify-write (unit updates, ledger records, gates,
standing orders), so two writers lose updates: units or verification records disappear.

Repro (executed in a no-write harness, using the transpiled acquireLock body over an in-memory filesystem):

A and B both fail create() with EEXIST and read the same dead holder PID.
A: unlink, create -> A holds the lock.
B resumes: unlink (deletes A's live lock), create -> B holds the lock too.

Suggested fix:

  • Serialize recovery with a separate exclusive mechanism, such as an OS advisory lock (flock) or a separate
    wx-created recovery lock that is itself removed only by its creator.
  • Write a unique token with each acquisition (pid plus random nonce plus time), and have release and takeover check
    that token instead of only the PID.
  • Rereading the holder just before the unlink narrows the window but doesn't close it.
  • Add a concurrency regression test for the schedule above: two acquirers, one stale holder, interleaved
    read/unlink/create. Assert that at most one acquirer succeeds.

Activity

  1. deanrie commented on Oct 28, 2026

    @deanrie

    Hey @MaxHarris-1, this one has been overtaken: @poteto's #1043 (merged after the commit you tested) makes Orchestrate agentic and deletes scripts/orch/ entirely, in both the pstack tree and the Claude Code harness copy, so acquireLock and its takeOver() no longer exist on main. The same TOCTOU was reported in #730 and fixed in #733 (unlink-only-if-still-the-same-holder plus a recovery lock); #733 is now closed for the same reason. If a lock of this shape comes back with the Project store, your repro harness is the right test to bring along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions