source-control:babysit-prs: worker-scope per-PR lease did not prevent a concurrent worker on the same PR (duplicate P1 fixes)

[kyle-sexton]

Lane

babysit (worker tier)

Triggering example

Dispatched as a worker for PR #377. Sequence observed this session:

1. Acquired the worker lease for melodic-software/claude-code-plugins#377 via manage_babysit_lease.py acquire --scope worker at 2026-07-19T16:57:26Z. Response: "acquired": true, "reclaimed_stale": false — i.e. the lease manager detected no other active worker on this PR.
2. Head at cycle start was 72a4411. I validated the two open Codex P1 findings (metadata route segment boundary; multi-target credential-absence exit code), fixed both, committed (9230bc3), and attempted a normal push.
3. Push was rejected non-fast-forward: the remote head had already advanced to 93b66be — a commit authored 11 minutes into my lease (authored 2026-07-19T13:08 EDT / 17:08Z) titled fix(autonomy): metadata route segment boundaries; per-target credential-absence exit codes. It fixed the exact same two P1 findings via a different-but-valid approach.
4. That commit's co-author trailer is Claude Fable 5; mine is Claude Sonnet 5 — a genuinely separate concurrent worker session, not a re-entrant same-run invocation.
5. While I was reconciling, the same actor kept pushing (e07514a at 13:18 EDT, further findings) and resolved both P1 review threads. The PR converged to 0 unresolved threads without any contribution from me.

Net effect: two workers independently produced competing branch-owned fixes for the same findings on the same PR at the same time. My fix was wasted work; a bad duplicate push was avoided only because GitHub rejected the non-fast-forward.

Observed vs expected

• Observed: a --scope worker lease acquire returned acquired: true, reclaimed_stale: false for a PR that another worker was concurrently editing and pushing to. The per-PR worker lease provided no mutual exclusion.
• Expected: either (a) the second worker's acquire should have observed the first worker's active lease and returned acquired: false so the second worker STOPs-and-reports (per the skill's step-1 contract), or (b) if the first worker's lease had legitimately expired/released mid-flight (heartbeat gap while it kept working), the lease TTL/heartbeat model needs to cover long-running in-flight work so a still-active worker is not seen as absent.

Category

bug (coordination/lease correctness)

Related

Distinct from #472, which covers queue-lease re-entrancy for overlapping cron fires within the same run ("heartbeat and wait"). This is two separate worker sessions both believing they hold an exclusive worker-scope lease on the same PR. Likely the same underlying overlap root cause (#472's long fan-out cycles), surfacing here as a worker-lease exclusivity failure rather than a queue-lease re-entrancy gap.

Impact

Duplicate Opus/Fable worker work on the largest PR in the queue; competing commits on the same lines/fixtures; near-miss on a redundant/conflicting push. If a force-push were used to reconcile two such workers, one worker's committed work could be destroyed.

[kyle-sexton]

This was generated by AI during triage. · Category: bug (coordination/lease correctness) · Priority: priority: high (source-control north-star; near-miss on destroying committed work — thin margin held only by GitHub's non-fast-forward rejection). Routed needs-human + wayfind: design.

#524 — Two concurrent --scope worker holders on PR #377: root cause & fix plan

1. Root cause — the coordination substrate, not the acquire logic

The babysit worker lease is a local filesystem file. manage_babysit_lease.py acquire --scope worker --pr owner/repo#N writes a per-PR JSON record under the run's --state-dir (active-workers/<owner>__<repo>__pr-<n>.json), serialized by an OS advisory file lock + atomic temp-file rename. There is no GitHub-side lock, comment, issue, or label backing a worker lease — the record lives only on the acquiring session's disk.

acquire's decision is exactly right for what it can see:

• If a live record for that PR exists (expires_at in the future) and the caller's --token doesn't match → LeaseHeldError → exit 3 (HELD) → step-1 STOP-and-report / --steal-stale retry.
• Fresh acquired: true only when it reads no record or an expired one.
• reclaimed_stale: true only on the --steal-stale takeover branch.

Hypothesis (b) — legitimate TTL expiry — is refuted by arithmetic. Worker A acquired 16:57:26Z; worker TTL default 5400s (90 min) → valid to ~18:27Z. At the ~11-minute mark when B was already editing/pushing, A's lease was neither expired (11 min « 90 min) nor stale (~660s < the 900s stale-takeover window, which itself requires --steal-stale). Had B been reading A's record, bare acquire returns HELD and --steal-stale also refuses (holder live) → skip. B did not skip. Therefore B was never reading A's lease record at all.

That is the defect — hypothesis (a)'s outcome via an architectural cause, not an acquire bug: worker-lease exclusivity holds only among sessions sharing one <state-dir> on one filesystem. Two independent babysit sessions (here distinct model identities "Claude Sonnet 5" / "Claude Fable 5") each acquire against their own state directory, each reads an absent record, each legitimately gets acquired: true. Neither can observe the other. (Same-dir key-collision ruled out — parse_repo_number casefolds the PR key, so URL and owner/repo#N forms map to one filename; a shared state-dir would have collided correctly.)

Today's only cross-session backstop is the snapshot engine's foreign_activity arm (diffs the run's mutation ledger vs GitHub timeline under the same self-logins) — but it is a per-cycle dispatch suppressor, not a lease: best-effort, GitHub-timeline-lagged, evaluated at snapshot time, not a fail-closed gate at the moment of mutation. It cannot reliably stop two acquires ~11 min apart.

2. Why needs-human

Making the worker lease exclusive across independent sessions cannot be done on the current local-file substrate — it requires choosing a different coordination primitive, a design decision with real tradeoffs, not a one-line acquire/TTL/heartbeat patch.

3. Fix as a behavioral contract (substrate-agnostic — whatever primitive is chosen must satisfy)

• Cross-session exclusivity. A worker acquire for a PR returns acquired: false (→ STOP-and-report) whenever any other session is actively working that PR — regardless of session/machine/state-dir — not only when a competing record sits in the same local active-workers/.
• Liveness stays provable + self-fencing. Preserve today's guarantees: a live holder keeping its heartbeat_interval_seconds can never be stolen; a stale takeover rotates the token so the evicted run's next heartbeat/release fails ownership; TTL remains the backstop; reap clears dead records.
• The mutation gate is the enforcement point. Exclusivity must be re-proven immediately before each branch write / push / thread-resolve / merge — not only at dispatch (the failure surfaced during pushes, after acquire).

RECOMMENDED direction (veto candidate): promote a GitHub-side claim into a fail-closed pre-mutation gate rather than trusting the local file across sessions — harden foreign_activity from a per-cycle suppressor into a hard gate that must pass right before any mutation, backed by a durable GitHub-visible worker-claim marker a competing session can observe. Tradeoff to weigh: GitHub-as-lock is itself racy / eventually-consistent — timeline lag can miss a genuine competitor (false-negative) or false-positive on our own just-written activity — so it narrows but does not fully close a simultaneous-start race. The alternative (shared external lock service / shared state-dir requirement) removes the race but adds an infra dependency and only works where sessions share it. This is the decision to make.

4. Acceptance criteria

• With Worker A holding a live, actively-heartbeating worker lease on owner/repo#N mid-push, a Worker B from a separate session / state-dir attempting acquire --scope worker receives acquired: false (HELD) and STOP-and-reports — it does NOT edit/push.
• --steal-stale still refuses A while live; still succeeds only against a provably dead (missed-heartbeat-beyond-window) holder.
• Two independent sessions can never both hold a mutating claim on the same PR; the second is deterministically routed to STOP-and-report before its first mutation.
• No regression to single-state-dir behavior, token re-affirmation, reap, heartbeat/TTL, or source-control:babysit-prs + loop: /loop 15m fires overlap in-flight fan-out cycles when batch-cap-10 dispatch runs long #472's re-entrancy handling.

5. Force-push / destroy-work near-miss

The worker lease is the FIRST line of defense against two concurrent writers, and it failed silently. Data loss was prevented ONLY by GitHub's non-fast-forward rejection + this skill's never-force-push invariant — a thin margin. The one sanctioned path that legitimately force-resolves divergence (a conflict-resolution merge reconcile) could, if reached first, overwrite a competitor's committed work. Any fix must close "two sessions both believed they held exclusive rights and both mutated" as the failure class, not just the specific push race caught this time.

Relation to #472

Shared root: the lease is a local-file primitive whose exclusivity/identity is scoped to one filesystem. Distinct failures — #472 = a run correctly re-affirming its own queue lease via --token; #524 = worker-lease exclusivity failing between independent sessions. A substrate change here should subsume #472's re-entrancy guarantee, not diverge from it.

[kyle-sexton]

This was generated by AI (needs-human decision-brief pass).

Decision brief (2-minute read)

Problem. A --scope worker lease acquire returned acquired: true for PR #377 while a separate session was concurrently editing and pushing to it — the per-PR worker lease gave no mutual exclusion across sessions. Data loss was avoided only by GitHub's non-fast-forward rejection. The triage comment above has the full root-cause; this brief builds on it and surfaces one constraint it misses.

Today (verified — confirming the triage analysis):

• The worker lease is a local-filesystem primitive: babysit_lease.py:active_workers_dir() → <state-dir>/active-workers/…; babysit_lease.py imports no gh — there is no GitHub-side lock. Exclusivity holds only among sessions sharing one state-dir on one filesystem.
• TTL arithmetic confirms "legitimate expiry" is refuted: worker TTL 5400s (90 min), stale-takeover 900s, heartbeat 300s (babysit_lease.py:29-33). At the ~11-min mark A's lease was neither expired nor stale → the second worker was never reading A's record.
• The only cross-session backstop, foreign_activity, lives in the snapshot engine (babysit_delta.py, pr_queue_snapshot.py), not the lease — a per-cycle dispatch suppressor, not a fail-closed gate.
• Not subsumed by source-control:babysit-prs: one shared authorship/finding/approval classifier + golden fixtures; gate script owns the merge-ready boolean (6-issue class) #534: source-control:babysit-prs: one shared authorship/finding/approval classifier + golden fixtures; gate script owns the merge-ready boolean (6-issue class) #534 fixes merge-readiness classification (authorship/finding/approval); source-control:babysit-prs: worker-scope per-PR lease did not prevent a concurrent worker on the same PR (duplicate P1 fixes) #524 is cross-session lease exclusivity — a different defect class.

Options. (Substrate-agnostic acceptance criteria in the triage comment still hold; the addition below is the #440 reconciliation.)

• RECOMMENDED — Fail-closed pre-mutation exclusivity gate backed by a durable GitHub-visible worker-claim, BUT reconcile with source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 first. source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 decided outward_write_mode: report-only is the default and suppresses all outward writes. A GitHub-visible worker-claim marker is an outward write → under the default it would be suppressed/inert, defeating the lock. This option therefore requires either (i) an explicit coordination-write exemption in source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440's contract (a lease claim is infra coordination, not a product outward-write), or (ii) the lock engaging only where write mode is on. Reversibility: LOW-MEDIUM — a GitHub-marker protocol plus a source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 carve-out is a contract change other sessions come to depend on, hard to walk back.
• Shared external lock service / mandated shared state-dir. Tradeoff: removes the race deterministically and has no source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 collision (no outward write); but adds an infra dependency and only protects sessions that share the substrate — cross-machine / cross-operator sessions that don't are still exposed. Reversibility: MEDIUM.
• Interim — harden foreign_activity into a fail-closed pre-mutation gate. Re-prove no competitor immediately before each push / thread-resolve / merge, keeping the local lease. Tradeoff: no new infra, reads GitHub (no outward write → no source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 collision), narrows the window massively — but timeline lag means it narrows, not closes, a simultaneous-start race. Reversibility: HIGH — a revertible gate tightening.

Cross-links.

• source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 (outward_write_mode: report-only default suppresses the proposed GitHub-visible claim marker — resolve the stance on coordination-writes before committing to a marker-based lock; this is the decision the triage comment omits).
• source-control:babysit-prs + loop: /loop 15m fires overlap in-flight fan-out cycles when batch-cap-10 dispatch runs long #472 (shared root: a local-file lease scoped to one filesystem; a substrate change here should subsume its re-entrancy guarantee, not diverge).
• feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 (autopilot merges raise the stakes — "two writers both believing they hold exclusivity, both mutate" must be closed before merges auto-land).
• source-control:babysit-prs: one shared authorship/finding/approval classifier + golden fixtures; gate script owns the merge-ready boolean (6-issue class) #534 (adjacent but distinct — classification, not exclusivity).

[kyle-sexton]

Fresh recurrence (2026-07-19, PR #377): a worker session held the worker-scope lease for #377 from 21:11Z (lease file present, token-held), yet a differently-identified concurrent session (git co-author "Claude Fable 5") pushed two more commits to the PR branch at 21:03Z and 21:39Z — ungated by the lease. The second session was extending the point-fix approach the human had explicitly superseded via the #549 redesign, producing a 25-file semantic merge conflict against the redesign worker's committed work (overlapping rewrites of the same validator function + 23 modify/delete conflicts on fixtures the redesign removed that the point-fix session had just modified). Confirms the lease provides no mutual exclusion across separately-launched sessions on the same PR; the redesign work was preserved on branch redesign/549-security-binding-allowlists and the landing escalated rather than raced.