fix(exec): follow-ups from independent review of #304/#306/#290

[MendixMau]

Five defects in project-bin/exec.sh that independent reviews of the merged PRs #304, #306 and #290 found. Each fix comes with fixture cases that fail on master.

1. Stale-lock takeover allowed more than one holder (#304, medium)

Problem. To take over a stale lock, a contender moved it aside. If the file it moved turned out to be a fresh lock, it put it back with mv -f. That is check-then-act. With 3 or more contenders, B moved A's fresh lock aside, C's noclobber write landed in the gap, and B then saw a lock and deleted A's.
Fix. Takeover now goes through a second O_EXCL file, .exec.lock.takeover. While holding it, a contender re-reads the lock and removes it only if it still holds exactly the stale line it read earlier. The fresh lock is never moved aside, so there is nothing to restore.

• A takeover file whose pid is dead, or which is older than 10 s, is stale. An empty one is stale once it is a minute old.
• A stale takeover file is moved aside. If what was moved is not the file that was judged stale, it is put back with ln, which never clobbers.
• When the takeover file is busy, the contender sleeps 1 s and looks again; there are now 6 attempts.
• Plain bash 3.2, noclobber, mv and ln; no flock.

Evidence. The new case in tests/wave2/test-exec-lock-watch.sh runs 10 rounds of 12 contenders started behind a barrier against a dead-pid lock. A probe in the snapshot stub counts how many runs are inside the locked section at once.

• On master, 2 to 3 of 10 rounds had 2 or 3 holders at once (two separate runs).
• With the fix, 0 of 10 in three runs.

Further cases: a dead contender's takeover file does not block, and an old empty takeover file does not block (CLAUDE.md instrument rule 7).

2. --watch-compat overclaimed "watch can apply this" (#304, low)

Problem. Any statement the classifier did not recognise was reported as watch-safe.
Fix. Only the statements on skills/ui-loop.md's watch-safe list now say "watch can apply this": ALTER PAGE, ALTER SNIPPET, CREATE [OR …] SNIPPET, and CREATE OR REPLACE|MODIFY PAGE. Read-only statements are neutral. Every other statement prints unknown — restart if behaviour looks stale: <line N: head>, both in the advisory mode and after a real run.

This changes two outputs:

• ALTER ENTITY … MODIFY (no ADD) is now unknown.
• CREATE MODULE alone is now unknown.

Golden input:

• create association taken verbatim from a pending bug issue.
• CREATE ENUMERATION from skills/build/workflow-example.mdl.
• create java action with a $$ body, verbatim from the bug87 issue.
• The captured DESCRIBE NAVIGATION fixture.
• The repo has no captured ALTER ENTITY … MODIFY ATTRIBUTE or CREATE CONSTANT statement. Those two are written from the grammar, and the fixture says so.

3. Two-tree coverage (#304, low)

New case: a checkout with the .mpr under app/. It checks that:

• the lock is taken at the project root;
• a live holder refuses a second run;
• a SIGKILLed holder's lock is taken over;
• a dirty app/ model is refused;
• --commit-first commits exactly app/mprcontents/* and leaves an unrelated staged file staged.

These cases pass on master too. They add coverage for CLAUDE.md field-proof rule 2 rather than proving a bug.

4. --cr after --commit-first (#306)

Problem. --cr was only read as the first argument. In the documented order, --commit-first --cr CR-07 x.mdl, the model was committed and then "--cr" was used as the script path.
Fix. --cr, --commit-first and then --patch or an advisory mode are now parsed in one loop, in any order. An unknown flag, or a script name starting with --, exits 2 before the lock, the commit or the snapshot.
Fixture. The documented order runs, and its BUILD-LOG row carries cr: CR-07. --commit-first --bogus x.mdl is refused with no commit, no snapshot and no exec.

5. ui-only test lane text (#290)

"no other module's journeys" is now "re-run the journeys of every page the change reaches (embedders of a changed snippet/layout)". A grep case for this is in test-exec-change-class.sh.

Checks

Fixtures:

• test-exec-lock-watch.sh: 77/0. On master it fails 14 cases (the race, the takeover file, flag order, misplaced flag, 6 watch-compat cases).
• test-exec-change-class.sh: 21/0 (1 failure on master).
• test-bug07-08.sh project-bin/exec.sh: 43/0.
• test-metrics-ledger.sh: 42/0.
• test-change-log.sh bin/gate-check.sh: 39/0. One run showed 1 failure while two heavy fixture runs were going on in parallel; the rerun passed.
• test-lint-gate-accept.sh: 20/0.
• test-exec-approval.sh: 43/0.

Repo checks:

• check-scripts, check-portability and check-no-private-citations are clean.
• The leak guard is clean (run without a denylist).
• check-pr-discipline origin/master is clean.
• The forbidden-word grep of the added lines has no hits.

Not covered / follow-ups

• Field run still owed. These fixes have only run against fixtures, never on a real project.
• project-bin/test-scratch.sh has the same bug. It takes the same lock with a blind mv -f takeover (lines 81–82), so it is not mutually exclusive with exec.sh either. It should use the same .exec.lock.takeover protocol. That file is outside this PR's scope (another change owns it).
• skills/ui-loop.md needs a one-line update. Lines ~208–212 list two verdicts; they should mention the third (unknown — restart if behaviour looks stale). Not edited here, to stay in scope.
• Not tested on macOS or Git Bash. The constructs used are portable (noclobber, mv, ln, find -mmin, sleep 0.01 in the fixture only).

Refs #287 #289 #274

🤖 Generated with Claude Code

https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw

---

Generated by Claude Code

[MendixMau]

Landed on master as part of the integration squash #329 (commit 31e90ae), after independent review — its Review: line is in that commit message. Closing as superseded.

---

Generated by Claude Code