Follow-up: deferred review findings from PR #900

[philcunliffe]

PR #900 (fix/issue-887, head 2c29e66558e2d7eecb840fccb8b3575969cc597b) exhausted its review-round budget. Triage read both review records and the code at head: every finding from both rounds is fixed at the current head except the two below, and both are non-blocking. They are deferred here so the PR can merge.

Deferred findings

1. test/core/service-command-timeout.test.js case 2 costs ~5s of tier-1 runtime (preference, accepted by the reviewer)

The second case drives the real SERVICE_PROBE_TIMEOUT_MS (5000ms) end-to-end through collectHypAwareStatus (see src/core/daemon/status.js), which has no injection point for the constant, adding ~5.1s to a ~16s npm test and growing if the constant is ever raised. Round 1 accepted it as-is because the case's whole value is a real collector, real probes, and a real hanging child; case 1 already pins the bound cheaply at timeoutMs: 250. Revisit only if SERVICE_PROBE_TIMEOUT_MS grows or the suite budget tightens; the fix would be a test-only injection seam for the probe deadline.

2. llp/0266-rearm-fires-at-both-attach-success-exits.decision.md may need renumbering at merge (docs numbering, tracked in #907)

Three other open branches also mint an llp/0266-* document (#850/#914, #893, #866); master currently claims none, so PR #900 is clean if it lands first. Whichever claimant lands second must renumber (a mechanical edit per the repo's LLP conventions), updating the doc filename, its @refs in src/core/commands/clients.js and test/core/attach-daemon-managed-tails.test.js, and the forward-ref on llp/0186-reconciler-refused-marker.design.md's Extended-by: line. The cross-branch race itself is #907's scope; this entry only records the per-PR renumber duty.

Backlink: #900.

[philcunliffe]

What neutral was doing

Picked this up off the neutral:fix label and went for the standard reproduce -> root-cause -> fix loop: fresh worktree off origin/master (12154929), reproduce each deferred finding with a failing test, fix, re-run npm test, open a Fixes #919 PR.

Why it cannot proceed

Neither deferred finding is reproducible on master, because everything they name exists only on the unmerged head of PR #900.

Ground truth, re-derived after git fetch --prune:

• PR Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 is still open. state: OPEN, isDraft: true, mergeStateStatus: DIRTY, mergeable: CONFLICTING, head still 2c29e66558e2d7eecb840fccb8b3575969cc597b. Its ten changed files are entirely absent from master.
• Finding 1 has no subject on master. test/core/service-command-timeout.test.js does not exist (test/core/ has only service-manager-test-sandbox.test.js matching service), and grep -rn SERVICE_PROBE_TIMEOUT_MS over the whole tree returns nothing. src/core/daemon/status.js on master has TRUST_PROBE_TIMEOUT_MS = 5_000, and it already carries injection seams at the collectHypAwareStatus call sites (?? ((args) => probeCaTrusted({ ...args, timeoutMs: TRUST_PROBE_TIMEOUT_MS }))). The constant the finding is about, and the test case that costs the ~5s, both arrive with Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900.
• Finding 1's own revisit condition is not met. The entry says "revisit only if SERVICE_PROBE_TIMEOUT_MS grows or the suite budget tightens". On master the constant does not exist yet, and npm test is green in 16.0s (4528 pass / 0 fail / 1 skipped), so the budget has not tightened either.
• Finding 2's renumber duty is not yet due. git ls-tree origin/master llp/ shows no llp/0266-* at all. The occupied numbers around there are 0265, then 0273, 0276, 0277 (commit 60e33eaa, whose subject says "LLP 0266 + 0267", actually landed as 0276/0277 after a renumber at merge). So 0266 through 0272 plus 0274 and 0275 are free, master still claims none, and Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 is still clean if it lands first. There is nothing on master to renumber and nothing to repoint.

So there is no failing-then-passing regression test to write against master, which is the LLP 0002 gate for opening a PR. The only branch where either finding is real is fix/issue-887, and fixing there would mean basing off another open PR's head rather than master, moving #900's head and re-opening its review ladder. That is outside what an issue-fix worker should do unilaterally, and #900 needs a conflict rebase from its own owner first regardless.

Concrete questions

1. Should Follow-up: deferred review findings from PR #900 #919 simply stay parked until Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 merges? That is neutral's read: both entries are explicitly recorded as non-blocking deferrals whose subject code arrives with Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900. If so, please drop neutral:fix for now and re-apply it after Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 lands, when both items become ordinary master-side work.
2. Finding 1: is the deferral still standing, or do you want the seam built regardless? The entry's own trigger has not fired. If you want it anyway once Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 lands, the shape neutral would build is a probeDeadlineMs (or a serviceProbeFn) opt on collectHypAwareStatus, matching the seam already used for the two trust probes, so case 2 keeps its real collector, real probes, and real hanging child at a 250ms bound. Confirm that is the intended shape before it is built.
3. Finding 2: is the per-PR renumber duty the right durable fix, or should neutral go after the detector instead? Three open branches each mint llp/0266-*, so the second one merged silently collides #907's item 2 asks for a /ref-check rule that fails when one LLP number resolves to more than one file. That is the only part of this cluster that is buildable on master today, and Attach and status residuals: bound the macOS status probes, drop the contradictory enablement line, reach both attach tails #900 is in fact a fourth claimant of 0266 beyond the three Three open branches each mint llp/0266-*, so the second one merged silently collides #907 lists (fix(cli): reject unknown arguments on every visible core command #850, Use native prepared batches through query sources #866, Client-scoped materialize keeps a dest another client still contributes #893). Neutral can take that on if you file or point it at an issue scoped to it, but it is Three open branches each mint llp/0266-*, so the second one merged silently collides #907's scope and not Follow-up: deferred review findings from PR #900 #919's, so it did not act on it here.

Reply with a comment on this issue; neutral monitors this thread and will re-engage with your guidance on its next tick.

[philcunliffe]

Update from the re-triage of PR #900 at head 1df26b5 (merge of master): finding 1 still stands, but the merge kept master's #833 timeout mechanism, so the constant driving the ~5s case is now TRUST_PROBE_TIMEOUT_MS in src/core/daemon/status.js (SERVICE_PROBE_TIMEOUT_MS no longer exists in the tree). Finding 2 is unchanged: master still claims no llp/0266, and the merged tree carries exactly one.