Skip to content

Ignore main's context in the review-gate diff id; fix the round-lock race (work-system 1.28.1) - #91

Merged
gering merged 3 commits into
mainfrom
task/harden-mandate-diff-id-and-round-lock
Oct 26, 2026
Merged

gering merged 3 commits into
mainfrom
task/harden-mandate-diff-id-and-round-lock

Conversation

@gering

@gering gering commented Oct 22, 2026

Copy link
Copy Markdown
Owner

Two fixes in plugins/work-system/scripts/mandate.sh.

1. A rebase over main's neighbouring lines keeps coverage

On #89, a rebase flipped covered to no (changed-since-review) although the lane's own lines were unchanged. lane_diff_id hashed a -U3 diff, whose three context lines per hunk come from main.

  • The id now hashes a -U0 diff (measured: -U0 alone is enough; stripping context after -U3 would keep -U3's hunk merging). A hunk's place is its file, its order among the file's hunks, and git's function-context line. Line numbers are not part of it, because any main insertion above would shift them.
  • Still flips to no: a lane line that changes, or that moves under another header or past another lane hunk. A main edit to the hunk's function-context line still fails closed.
  • Accepted residuals (raised in the local review, see below): a lane line moved within one function past no other lane hunk keeps the id, and so does a main edit that changes what a lane hunk means (a guard deleted right above it).
  • Compat: reviews recorded by 1.28.0 (diff_id_form=context, masked -U3) or earlier (legacy) still match their unchanged lane, with the old argv, so this also holds under a non-default diff config. The tests recompute both old ids from the old algorithms. delta still shows context; diff_form=masked is now context-free.

2. acquire_round_lock TOCTOU (CI flake, rc=4)

After a failed mkdir, the lock code tested [ ! -d "$lock" ] to tell "cannot create" from contention. The holder can release the lock in between, so a concurrent booking exited 4. Reproduced under parallel load: 16 of 72 runs failed.

  • The verdict now comes from mkdir's own error (File exists = contention). The fix passed 168 runs under load with 0 failures.
  • The pid is written aside and renamed in, so lock_is_stale never reads a partial pid. A young lock without a pid is never stale, which covers the mkdir-to-pid gap.
  • A file or symlink at the lock path now exits 4 at once with a hint.
  • Deterministic test: a PATH mkdir shim that fails once with "File exists" and leaves no lock behind.

Review

Local /swarm:review (Claude + gpt-6.1-sol + grok-4.7; 2 grok clusters timed out): 11 findings. Agreed fixes are applied and recorded (covered=yes via=agreed-fixes). Not changed:

  • A 1.28.0 session cannot read a 1.28.1 record. This is fail-closed version skew, which /sessions fixes.
  • The test oracle's version regex only needs to match the tested versions.

Checks

  • test_mandate.py: all tests pass
  • scripts/check-structure.py: OK (pre-existing pr-flow /open description warning)

work-system 1.28.0 → 1.28.1 (patch).

🤖 Generated with Claude Code

https://claude.ai/code/session_01BJZ4tA8d432yqatzZMJqFw

@gering
gering force-pushed the task/harden-mandate-diff-id-and-round-lock branch 2 times, most recently from 079af77 to 1119bf9 Compare October 22, 2026 21:20
gering and others added 3 commits October 23, 2026 16:42
A -U3 diff carries three of main's lines around every lane hunk, so a
rebase over main edits next to a lane hunk flipped covered to no (#89).

- lane_diff_id hashes a -U0 diff: the lane's +/- lines, their file, hunk
  order and git's function context as the place; line numbers stay out
- inter-hunk context and the indent heuristic are pinned
- reviews recorded by 1.28 (context) and 1.23-1.27 (legacy) still match
  their unchanged lane via lane_compat_ids; delta keeps showing context
- tests recompute the old ids from the old algorithms

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJZ4tA8d432yqatzZMJqFw
acquire_round_lock told "cannot create" from contention by checking for
the lock after a failed mkdir, but the holder can release it in between,
so a concurrent booking exited 4 (CI flake on #86; 16 of 72 stressed
runs locally).

- decide from mkdir's own error: "File exists" is contention, anything
  else exits 4 at once
- write the pid aside and rename it in, so lock_is_stale never reads a
  partial pid
- deterministic test via a mkdir shim that fails once with "File exists"

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJZ4tA8d432yqatzZMJqFw
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BJZ4tA8d432yqatzZMJqFw
@gering
gering force-pushed the task/harden-mandate-diff-id-and-round-lock branch from 1119bf9 to 22d295e Compare October 23, 2026 14:55
@gering
gering merged commit bc9534c into main Oct 26, 2026
2 checks passed
@gering
gering deleted the task/harden-mandate-diff-id-and-round-lock branch October 26, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant