🤖 tests: declined review findings of the phase 4 call-record PRs

Read the review findings and mutation survivor lists in this issue, then inspect the referenced tests and source files, including src/node/utils/concurrency/fileLock.ts, src/node/utils/main/crossProcessLock.ts, and src/node/services/artifactVersionStore.ts. Start by checking the PR and test references listed for each finding; mark an item done only when its required test kills the survivor or addresses the finding. The issue assigns unresolved work across later phase 4 PRs and PR 17, so coordinate with that work before starting.

Tracks the review findings that the phase 4 call-record PRs (#7290, #7291) declined. Each one names the PR that must close it.

Refs #7255

• Owner: Phase 4 builder.
• Trigger: before phase 4 PR 16 merges, every item below is done or has a reasoned close.

Findings

1. Lease protocol order (#7290, PRRT_kwDOPxxmWM6vjS7Z, round 6). The record sorts its rows, so it cannot show if WorkspaceUseLeases.hold() inspects the mutation gate before it acquires the use-lease file. That order keeps a mutator and a turn from both going ahead.
   • Closed by: phase 4 PR 4 (SessionLocks service, the first PR that routes hold() through the new layer). Its first new commit adds an explicit order check in the turn scenario, like the removal check: use-lease file acquired before the gate is inspected.
   • Status: in #7345 (first commit e217b89ce5).
2. Digest names follow the production hash builders (#7290, PRRT_kwDOPxxmWM6vjS7d, round 6). The test names digests with historyWriteLockPath, refineApplyLockPath, workspaceMutationLockPath, workspaceUseLockDir and targetMutationLockFilePath. A move that changes one hash scheme changes both the recorded path and its name, so the fixture still matches.
   • Closed by: phase 4 PR 4 (the first move PR, before PRs 9, 12 and 13 touch the files of these builders). Its first new commit pins fixed digests for the fixed test IDs and keys, and checks them against the builders.
   • Status: in #7345 (first commit e217b89ce5).
3. Seed HistoryService in beforeEach (#7290, PRRT_kwDOPxxmWM6vgtmT, P1, round 4). The refine scenario appends its rows inside the scenario, before its try, so a failed append leaks the temp root of createTestHistoryService(). Test hygiene, not a gap in the record: an append failure fails the test at once.
   • Closed by: any later phase 4 PR that edits the scenario harness, or a reasoned close.
4. Windows path separators (#7290, PRRT_kwDOPxxmWM6vehmU, P2, round 2). The temp-root masks expect forward slashes. No CI lane runs this test on Windows (the test-windows job runs a fixed smoke list).
   • Closed by: a reasoned close, unless a Windows lane starts running the file.

Mutation survivors on lines that PR 4 does not change

Phase 4 PR 4 (#7345) found these survivors in its mutation run (no test-file cap, lane with the call-record and contract tests, head 907f6b054e). Each one sits on a line that PR 4 does not change. Line numbers are on the PR 4 head.

Rule for every later phase 4 PR: before a PR changes or moves a line below, it kills that survivor first. The test goes in a commit at the start of the PR, and it must pass on the code before the change. PR 17 is too late for such a line, because its tests would describe the code after the change. Each PR checks this list, and marks an item done with the PR and the test that kills it. The items that no PR touches go to PR 17.

M1. fileLock.ts:260 in acquireProcessFileLock: 0 -> 1

• File: src/node/utils/concurrency/fileLock.ts:260
• Line: assert(lockPath.length > 0, "acquireProcessFileLock requires a lock path");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M2. fileLock.ts:260 in acquireProcessFileLock: > -> >=

• File: src/node/utils/concurrency/fileLock.ts:260
• Line: assert(lockPath.length > 0, "acquireProcessFileLock requires a lock path");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M3. fileLock.ts:261 in acquireProcessFileLock: > -> >=

• File: src/node/utils/concurrency/fileLock.ts:261
• Line: assert(timeoutMs > 0, "acquireProcessFileLock timeoutMs must be positive");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M4. fileLock.ts:265 in acquireProcessFileLock: false -> true

• File: src/node/utils/concurrency/fileLock.ts:265
• Line: let acquired = false;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M5. fileLock.ts:278 in acquireProcessFileLock: ?? -> ||

• File: src/node/utils/concurrency/fileLock.ts:278
• Line: const renewIntervalMs = options.renewIntervalMs ?? FILE_LOCK_RENEW_INTERVAL_MS;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M6. fileLock.ts:279 in acquireProcessFileLock: 0 -> 1

• File: src/node/utils/concurrency/fileLock.ts:279
• Line: assert(renewIntervalMs > 0, "renewIntervalMs must be positive");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M7. fileLock.ts:279 in acquireProcessFileLock: > -> >=

• File: src/node/utils/concurrency/fileLock.ts:279
• Line: assert(renewIntervalMs > 0, "renewIntervalMs must be positive");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M8. fileLock.ts:296 in acquireProcessFileLock: !== -> ===

• File: src/node/utils/concurrency/fileLock.ts:296
• Line: if (renewInFlight !== null) await renewInFlight;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M9. fileLock.ts:306 in acquireProcessFileLock: 0 -> 1

• File: src/node/utils/concurrency/fileLock.ts:306
• Line: const blocker = await reclaimStaleFileLock(lockPath, label, 0, options.testOnlyReclaimSeam);
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M10. fileLock.ts:307 in acquireProcessFileLock: >= -> >

• File: src/node/utils/concurrency/fileLock.ts:307
• Line: if (Date.now() >= deadline) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M11. fileLock.ts:311 in acquireProcessFileLock: * -> /

• File: src/node/utils/concurrency/fileLock.ts:311
• Line: setTimeout(resolve, FILE_LOCK_RETRY_MS + Math.random() * FILE_LOCK_RETRY_MS)
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M12. fileLock.ts:311 in acquireProcessFileLock: + -> -

• File: src/node/utils/concurrency/fileLock.ts:311
• Line: setTimeout(resolve, FILE_LOCK_RETRY_MS + Math.random() * FILE_LOCK_RETRY_MS)
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M13. crossProcessLock.ts:130 in parseHolder: || -> && (site 1 of 2 on the line, left to right)

• File: src/node/utils/main/crossProcessLock.ts:130
• Line: if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M14. crossProcessLock.ts:130 in parseHolder: || -> && (site 2 of 2 on the line, left to right)

• File: src/node/utils/main/crossProcessLock.ts:130
• Line: if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M15. crossProcessLock.ts:140 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:140
• Line: typeof pid !== "number" ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M16. crossProcessLock.ts:141 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:141
• Line: !Number.isInteger(pid) ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M17. crossProcessLock.ts:142 in parseHolder: 0 -> 1

• File: src/node/utils/main/crossProcessLock.ts:142
• Line: pid <= 0 ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M18. crossProcessLock.ts:142 in parseHolder: <= -> <

• File: src/node/utils/main/crossProcessLock.ts:142
• Line: pid <= 0 ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M19. crossProcessLock.ts:142 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:142
• Line: pid <= 0 ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M20. crossProcessLock.ts:143 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:143
• Line: typeof token !== "string" ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M21. crossProcessLock.ts:144 in parseHolder: 0 -> 1

• File: src/node/utils/main/crossProcessLock.ts:144
• Line: token.length === 0 ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M22. crossProcessLock.ts:144 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:144
• Line: token.length === 0 ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M23. crossProcessLock.ts:145 in parseHolder: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:145
• Line: typeof acquiredAt !== "number" ||
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M24. crossProcessLock.ts:226 in describeHolder: ?? -> ||

• File: src/node/utils/main/crossProcessLock.ts:226
• Line: ``pid ${holder.pid} (started ${holder.identity?.birth ?? "at an unknown time"} +
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M25. crossProcessLock.ts:499 in acquireCrossProcessLock: || -> &&

• File: src/node/utils/main/crossProcessLock.ts:499
• Line: if (stopped || inFlight !== undefined) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M26. crossProcessLock.ts:504 in acquireCrossProcessLock: && -> ||

• File: src/node/utils/main/crossProcessLock.ts:504
• Line: if (current.kind === "holder" && current.key === token) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M27. crossProcessLock.ts:517 in acquireCrossProcessLock: true -> false

• File: src/node/utils/main/crossProcessLock.ts:517
• Line: stopped = true;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M28. crossProcessLock.ts:528 in acquireCrossProcessLock: 0 -> 1

• File: src/node/utils/main/crossProcessLock.ts:528
• Line: for (let attempt = 0; attempt < 40; attempt++) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M29. crossProcessLock.ts:528 in acquireCrossProcessLock: < -> <=

• File: src/node/utils/main/crossProcessLock.ts:528
• Line: for (let attempt = 0; attempt < 40; attempt++) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M30. crossProcessLock.ts:543 in acquireCrossProcessLock: true -> false

• File: src/node/utils/main/crossProcessLock.ts:543
• Line: await fsPromises.rm(lockPath, { force: true });
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M31. crossProcessLock.ts:569 in acquireCrossProcessLock: false -> true

• File: src/node/utils/main/crossProcessLock.ts:569
• Line: let owned = false;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M32. crossProcessLock.ts:585 in acquireCrossProcessLock: >= -> >

• File: src/node/utils/main/crossProcessLock.ts:585
• Line: if (Date.now() >= deadline) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

Mutation survivors of PR 8 (#7354) on lines that it does not change

The same rule as above applies. Line numbers are on the #7354 head 8d7daf617a.

M33. artifactVersionStore.ts:152 in recordArtifactVersion: ?? -> ||

• File: src/node/services/artifactVersionStore.ts:152
• Line: const nowMs = params.nowMs ?? Date.now();
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M34. artifactVersionStore.ts:166 in recordArtifactVersion: 0 -> 1

• File: src/node/services/artifactVersionStore.ts:166
• Line: versions: [...index.versions.slice(0, -1), refreshed],
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M35. artifactVersionStore.ts:166 in recordArtifactVersion: 1 -> 0

• File: src/node/services/artifactVersionStore.ts:166
• Line: versions: [...index.versions.slice(0, -1), refreshed],
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M36. artifactVersionStore.ts:185 in recordArtifactVersion: != -> ==

• File: src/node/services/artifactVersionStore.ts:185
• Line: ...(params.kind != null ? { kind: params.kind } : {}),
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M37. artifactVersionStore.ts:198 in recordArtifactVersion: true -> false

• File: src/node/services/artifactVersionStore.ts:198
• Line: return { artifactId, version: entry, created: true, pin: nextPin };
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M38. artifactVersionStore.ts:224 in acquireIndexFileLock: ?. -> .

• File: src/node/services/artifactVersionStore.ts:224
• Line: if (abortSignal?.aborted) throw new Error(ARTIFACT_PUBLISH_INTERRUPTED);
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M39. WorkflowRunStore.ts:228 in constructor: 0 -> 1

• File: src/node/services/workflows/WorkflowRunStore.ts:228
• Line: assert(options.sessionDir.length > 0, "WorkflowRunStore: sessionDir is required");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M40. WorkflowRunStore.ts:228 in constructor: > -> >=

• File: src/node/services/workflows/WorkflowRunStore.ts:228
• Line: assert(options.sessionDir.length > 0, "WorkflowRunStore: sessionDir is required");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M41. WorkflowRunStore.ts:230 in constructor: ?? -> ||

• File: src/node/services/workflows/WorkflowRunStore.ts:230
• Line: this.staleLeaseMs = options.staleLeaseMs ?? 30_000;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M42. WorkflowRunStore.ts:232 in constructor: 0 -> 1

• File: src/node/services/workflows/WorkflowRunStore.ts:232
• Line: options.mutationLockWaitTimeoutMs == null || options.mutationLockWaitTimeoutMs > 0,
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M43. WorkflowRunStore.ts:232 in constructor: > -> >=

• File: src/node/services/workflows/WorkflowRunStore.ts:232
• Line: options.mutationLockWaitTimeoutMs == null || options.mutationLockWaitTimeoutMs > 0,
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M44. WorkflowRunStore.ts:1110 in acquireLease: 0 -> 1

• File: src/node/services/workflows/WorkflowRunStore.ts:1110
• Line: assert(ownerId.length > 0, "WorkflowRunStore.acquireLease: ownerId is required");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M45. WorkflowRunStore.ts:1110 in acquireLease: > -> >=

• File: src/node/services/workflows/WorkflowRunStore.ts:1110
• Line: assert(ownerId.length > 0, "WorkflowRunStore.acquireLease: ownerId is required");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M46. WorkflowRunStore.ts:1119 in acquireLease: <= -> <

• File: src/node/services/workflows/WorkflowRunStore.ts:1119
• Line: if (existing != null && nowMs - existing.acquiredAtMs <= this.staleLeaseMs) {
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M47. WorkflowRunStore.ts:1778 in withWorkflowFileLock: 0 -> 1

• File: src/node/services/workflows/WorkflowRunStore.ts:1778
• Line: Math.max(0, deadline - Date.now()),
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M48. journal.ts:92 in constructor: 0 -> 1

• File: src/node/utils/journal/journal.ts:92
• Line: assert(options.filePath.length > 0, "Journal requires a file path");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M49. journal.ts:92 in constructor: > -> >=

• File: src/node/utils/journal/journal.ts:92
• Line: assert(options.filePath.length > 0, "Journal requires a file path");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M50. journal.ts:98 in constructor: ?? -> ||

• File: src/node/utils/journal/journal.ts:98
• Line: this.appendLockTimeoutMs = options.appendLockTimeoutMs ?? APPEND_LOCK_TIMEOUT_MS;
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M51. journal.ts:99 in constructor: 0 -> 1

• File: src/node/utils/journal/journal.ts:99
• Line: assert(this.appendLockTimeoutMs > 0, "Journal appendLockTimeoutMs must be positive");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M52. journal.ts:99 in constructor: > -> >=

• File: src/node/utils/journal/journal.ts:99
• Line: assert(this.appendLockTimeoutMs > 0, "Journal appendLockTimeoutMs must be positive");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

M53. journal.ts:152 in append: + -> -

• File: src/node/utils/journal/journal.ts:152
• Line: const postAppendFileSize = fileSize + Buffer.byteLength(payload, "utf-8");
• Closed by: PR 17, or the first phase 4 PR that changes or moves this line.

---

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high • Cost: $70.87