withFileLock keeps the stale lock key when the key recheck throws, instead of releasing the acquired lock and propagating #2405

Where this was measured. Branch vps2f-1914-u5a at 3901239982bc8fa3ad96996539b4845aaa25bb31 (PR #2359) and branch vps2f-1914-u5b at d6fa7117e07b70455da24e4722b47eb0b202fa6e (PR #2357); src/utils/fileLock.ts is the same blob at both heads (git rev-parse <rev>:src/utils/fileLock.ts = 0705f771ccac98167f71562ed418da4106b8b31d, 241 numbered lines).

The finding. src/utils/fileLock.ts:206 re-derives the key under the lock with const currentKey = await canonicalLockPath(operationPath).catch(() => lockKey). The comment at :200-205 explains why the recheck exists: a directory component can be swapped while the call waits for the lock, so the key acquired above may no longer be the canonical key of the path the callback is about to mutate. The recheck handles movement: :207-216 releases the stale key (:208, with the release failure swallowed) and retakes under the new key. It does not handle failure to inspect. When canonicalLockPath throws, which its own missing-path walk does deliberately for an uninspectable ancestor (:94-99), the catch hands back lockKey, the if is false, and the callback runs while holding a lock named by the pre-wait spelling. A peer that reached the same file through the post-wait spelling holds the other key, so both run: exactly the case the comment says the recheck is for.

Reviewer ask and its caveat, quoted. Thread 4509350121 (coderabbitai[bot], submitted 2026-11-09T04:06:10Z, last edited 2026-11-09T07:47:28Z) at src/utils/fileLock.ts:206 asks: "Fail closed when the lock-key recheck fails." with "Release the acquired lock and propagate the inspection error instead." Its guidance reads: "Update the lock-key recheck in canonicalLockPath so an inspection failure releases the acquired lock and propagates the error instead of falling back to lockKey. Preserve the existing behavior when the recheck succeeds." In row 4510440989 (coderabbitai[bot], submitted 2026-11-09T07:47:47Z) the reviewer added the constraint this issue has to carry: "One detail for the eventual change: removing the catch alone would propagate the error without releasing the acquired lock. The failure path needs to release the lock before propagating the inspection error. Tests should cover that path, a successful recheck, and a retarget." and asked "The finding remains open. Would you like a follow-up issue for it?" This issue is that offer accepted, with the caveat carried as a requirement rather than a note.

Test status. The retarget ordering now has a row: commit ebd6d9e17df3bc0d3d234bb13dbe00b5be5be3bf on #2359 records both releases and expects the retargeted key's release after the callback, and it was shown to go red when src/utils/fileLock.ts:233 is edited to skip that release (1 failed | 8 passed, the retarget row) and green again on restore (9 passed). No row covers the throwing recheck, which is this finding's own half, and none covers the successful-recheck case the reviewer listed.

What is not claimed. No measured incident and no severity beyond the reviewer's (Minor, Data Integrity and Integration, Quick win). It is not claimed that the release on the failure path can be made to succeed unconditionally: :222-229 and :232-237 show that release failures on the two ordinary paths are logged rather than thrown, and a failure-path release inherits that problem.

Where it relates. #2357 and #2359 both carry the file and the thread; the fix belongs to #2393 (fix(utils): one file, one lock artefact spelling, one canonical key, head 2c79cd47d866), the unit that owns src/utils/fileLock.ts. #2350 (task-history delete semantics under the same lock) is the nearest reader of the behavior.

Price status. Unpriced, and it is production currency, not test currency: any fix changes executable lines in fileLock.ts. Both chain heads are cap-refused today (770 counted executable lines for #2357 at d6fa7117e07b70455da24e4722b47eb0b202fa6e, 1284 for #2359 at ebd6d9e17df3bc0d3d234bb13dbe00b5be5be3bf, against the 500-line cap in scripts/stryker-diff.mjs), measured with that script's own selectFromGit on the branch leg and the CI leg, so this travels with #2393 instead of onto either head.

Found while answering CodeRabbit threads on #2357 and #2359. Filed on the ruling that a finding recorded only inside a review reply gets collapsed, marked outdated and re-scored, while an issue does not.