Follow-up: deferred review findings from PR #502

[philcunliffe]

Context

Follow-up from TRIAGE on PR #502 (fix/issue-421, head 2487861d346566da1b79d45838d92833f6fb6ff7).

The two-round review fix-loop (comments on PR #502 marked
<!-- neutral-review: 7d1b111...findings --> and
<!-- neutral-review: be9ce7f...findings -->) raised F1-F5 across both
rounds (unsanitized/unbounded client-supplied labels reaching status.json
and the terminal, a partial sanitizer missing bidi-override and zero-width
characters, a missing test pinning the eviction cap, a wrong threat-model
note in docs, an uncapped read-side entry count, and a hung/throwing plugin
status() probe that could freeze status.json and make hyp daemon stop
unstoppable). All were fixed by the reviewer's own fix-forward commits
(be9ce7f, 2487861) within the PR itself.

TRIAGE independently re-verified this rather than trusting the review
comments: read src/core/util/json_util.js (sanitizeLabel strips C0/C1,
bidi overrides/isolates, and zero-width/default-ignorable characters, clamps
to 120 chars without a stranded surrogate), src/core/daemon/status.js
(MAX_RECENT_ENTRYPOINTS = 32 applied on the read side after sort), and
src/core/daemon/runtime.js (probeSourceDetails under a 5s timeout,
in-flight-skip, transition-only daemon.source_status_failed/_recovered
logging). Ran the full npm test (3055/3055 non-pre-existing tests pass,
identical 8 pre-existing test/core/leave-command.test.js failures),
npm run typecheck (clean), and npm run smoke -- gateway_codex_capture
(ok) against a fresh worktree at head 2487861. The dedicated hang-probe
test (test/core/daemon-source-details-refresh.test.js, "a source whose
status() never settles cannot freeze the status file or the shutdown")
passes and is non-vacuous: it stages a real hanging plugin, asserts
status.json's mtime keeps advancing, that stop() completes, and that the
failure is logged at least once and at most 4 times.

Conclusion: no TRUE BLOCKER is open at head 2487861. This PR is safe
to ship as-is.

Deferred (non-blocking)

Author sign-off on two constants chosen unilaterally during round-2
fix-forward. Quoting the round-2 review comment (be9ce7f review,
posted 2026-07-31T03:42:34Z on PR #502):

The 5s probe timeout and the transition-only log shape are my calls, made
because I am the last round and the alternative was handing triage an
unstoppable-daemon vector. Both are documented in LLP 0164. If you prefer
a different bound or a different event shape, they are one constant and
one function.

Concretely:

• SOURCE_STATUS_TIMEOUT_MS = 5000 in src/core/daemon/runtime.js (the
  ceiling a plugin's status() probe gets before the tick gives up on it).
• The log event names daemon.source_status_failed /
  daemon.source_status_recovered, also in src/core/daemon/runtime.js,
  and their transition-only (not per-tick) emission policy.

Both are safe, tested, and documented in LLP 0164, but the PR author never
explicitly confirmed these specific values/shapes — they were introduced by
the reviewer while fixing the PR forward in the final round rather than by
the author. Worth a quick author glance to confirm the timeout value and
log-event naming match what they'd have chosen, or adjust if not. Not
blocking: nothing about shipping the current values is unsafe.

Backlink

PR: #502

[philcunliffe]

🤖 neutral: stuck - needs your sign-off, not a code change

What neutral was doing: the issue-fix rung on this issue (reproduce -> root-cause -> fix -> Fixes #511 PR), against origin/master @ bd7dd0b.

Why it cannot proceed - two independent reasons.

1. Every enumerated finding is already fixed. F1-F5 (unsanitized/unbounded client-supplied labels reaching status.json and the terminal; the sanitizer missing bidi-override and zero-width characters; the missing eviction-cap test; the wrong threat-model doc note; the uncapped read-side entry count; the hung/throwing plugin status() probe) were all closed by the reviewer's own fix-forward commits be9ce7f and 2487861 inside PR #502, and independently re-verified by the triage rung at head 2487861. None is open.

The only residual is the item the round-2 reviewer filed under a section literally titled "Design notes (no action, recorded only)":

The 5s probe timeout and the transition-only log shape are my calls, made because I am the last round and the alternative was handing triage an unstoppable-daemon vector. Both are documented in LLP 0164. If you prefer a different bound or a different event shape, they are one constant and one function.

That is not a defect. The values are safe, tested (the non-vacuous hung-probe test in test/core/daemon-source-details-refresh.test.js asserts status.json's mtime keeps advancing, stop() completes, and the failure logs at least once and at most 4 times over ~500 ticks), and documented. There is no missing test, no doc gap, no style violation. Changing 5000 to some other number, or renaming the events, would be inventing a change to justify a PR - so neutral did not.

2. The code does not exist on master anyway. Verified independently: a whole-tree git grep on origin/master for SOURCE_STATUS_TIMEOUT, source_status_failed, source_status_recovered, refreshSourceDetails, recentEntrypointsFromSources, sanitizeLabel, and MAX_RECENT_ENTRYPOINTS returns zero hits. All of it lives only on the unmerged fix/issue-421:

• origin/fix/issue-421:src/core/daemon/runtime.js:485 - const SOURCE_STATUS_TIMEOUT_MS = 5000
• origin/fix/issue-421:src/core/daemon/runtime.js:557 - daemon.source_status_failed
• origin/fix/issue-421:llp/0164-...decision.md:137-138 - the documented shape

So a fix/issue-511 branch off today's master has nothing to edit.

What it needs from you - two yes/no answers:

1. Is SOURCE_STATUS_TIMEOUT_MS = 5000 the bound you would have picked for a plugin status() probe?
2. Are daemon.source_status_failed / daemon.source_status_recovered, emitted on transition only rather than per tick, the event names and policy you want?

If either answer is no, the change belongs on fix/issue-421 before PR #502 merges, or in a follow-up branched off master after #502 lands - not on a branch off today's master.

If both are yes, this issue can simply be closed once #502 merges.

How to unstick: reply with a comment on this issue; neutral monitors this thread and will re-engage with your guidance on its next tick.

[philcunliffe]

🤖 neutral: correcting my own earlier stuck report

My stuck report above gave two reasons this could not proceed. The second one is now false and I do not want it sitting in the record.

Reason 2 has cleared. I wrote that the code "does not exist on master anyway". PR #502 has since merged (e35c307), so it does:

• src/core/daemon/runtime.js - SOURCE_STATUS_TIMEOUT_MS, daemon.source_status_failed / _recovered
• llp/0164-status-names-recent-clients-from-gateway-entrypoints.decision.md
• test/core/daemon-source-details-refresh.test.js

A fix/issue-511 branch off today's master would now have something to edit.

Reason 1 still stands, and it is the one that matters. There is no defect here. The values are safe, tested, and documented; the round-2 reviewer filed them under "Design notes (no action, recorded only)". Changing 5000 to some other number, or renaming the events, would be inventing a change to justify a PR. So this stays held on your sign-off, not on the branch situation:

1. Is SOURCE_STATUS_TIMEOUT_MS = 5000 the bound you would have picked for a plugin status() probe?
2. Are daemon.source_status_failed / daemon.source_status_recovered, emitted on transition only rather than per tick, the event names and policy you want?

If both are yes, this issue can simply be closed. If either is no, it is now a one-line change on master rather than something that had to wait.

How to unstick: reply on this thread; neutral will re-engage on its next tick.