Follow-up: deferred review findings from PR #777

[philcunliffe]

Deferred from PR #777's review (round 2, resolved there as option (b): comment corrected, contract pinned by test). This issue is option (a), the change that was deliberately not made because it needs a policy decision first.

What to do

Move the cleaning of the remote_config_rolled_back diagnostic message out of the collector and into the single ${d.message} render interpolation, so --json carries the raw assembled prose for diagnostics too, matching LLP 0225 #decision's general rule (the text surface escapes what a person reads; --json stays byte-exact).

• Assembly today: src/core/daemon/status.js:1062 builds the message from sanitizeLabel'd components, so hyp status --json receives the cleaned sentence at diagnostics[].message (each component stripped, clamped to 120). The long comment above it (src/core/daemon/status.js:1038-1057) states this plainly and names this alternative.
• Target choke point: src/core/commands/status.js:559, the one ${d.message} interpolation in renderStatusText's diagnostics loop. Cleaning there would incidentally cover the other diagnostic kinds, several of which interpolate untrusted-ish strings raw (an err.message, a validator pointer, file paths).

The open question that blocks it: one clamp width for all 14 diagnostic kinds

A clamp at the render applies to the whole assembled sentence, not per component, and it must hold for all 14 diagnostic kinds in src/core/daemon/status.js (14 distinct kind: values at head 078d762). The longest template, client_attached_not_configured at src/core/daemon/status.js:963, is already roughly 200 characters of the project's own prose before any captured value is substituted. So:

• the label default of 120 cannot even hold the fixed prose;
• the existing error clamp of 400 (MAX_ERROR_CHARS, defined at src/core/commands/status.js:300 and src/core/commands/daemon.js:66) is tight enough that a legitimate message naming a full path could be truncated.

Someone has to pick that number and defend it across every kind. That is wider than #776 asked for, which is why it was filed rather than done.

What changes with it

• Revert src/core/daemon/status.js:1062 to raw interpolation and trim the comment above it to match.
• Clean at src/core/commands/status.js:559 with the chosen width.
• Update the two round-2 tests in test/core/status-hostile-non-status-file.test.js that pin the current contract (hyp status --json carries the cleaned rollback prose, and the raw values beside it and a long rollback etag is clamped in the --json message but whole in the --json values): under option (a) the --json message becomes byte-exact and the text surface stays clean.
• remote_config.last_rollback stays byte-exact either way; it already is.

Classification

Preference, not a blocker (PR #777 round 2's classification, confirmed at triage). Behaviour on the merged branch is safe: nothing hostile reaches a terminal on either surface, the structured values a program should read are byte-exact, and the code describes itself accurately.

Backlink: PR #777 (head 078d7627c9e06207c942bd56bb6740f5d62e44f4), fixing #776; review rounds at #777 (comment) and #777 (comment).

[philcunliffe]

This issue cannot be fixed on master yet: its targets exist only on PR #777's branch, which is held and unmerged.

Verified against origin/master: printableNumber in src/core/commands/daemon.js, and test/core/status-hostile-non-status-file.test.js is not present there.

Neutral is therefore not opening a fix/issue-784 PR. There would be nothing to edit, and pushing the change onto #777's branch instead would move its head and send a currently mergeable, green, reviewed PR back through triage. That tradeoff belongs to the maintainer.

The practical options are to land the change as a fixup on #777 before merging it, or to leave this issue for after #777 merges, at which point a normal fix PR against master works.

Until then this issue keeps reading as needs-fix to the reconciler, because no fix branch exists and none should. Recording that here so it is not re-attempted every tick.

[philcunliffe]

Still blocked, for two independent reasons. Labelling neutral:stuck and asking rather than guessing.

I took a fresh detached worktree at origin/master (04330abb, "Proxy mode is the default and hyp attach claude migrates old installs (#794)") and tried to carry out option (a). I could not, and I did not open a PR.

Blocker 1: the code option (a) edits is not on master

PR #777 is still open (neutral:approved, MERGEABLE / CLEAN, head 37a79168, unmerged). Every target this issue names is introduced by that PR and exists only on its branch. Verified at 04330abb:

• test/core/status-hostile-non-status-file.test.js - absent. The two round-2 tests this issue says to update do not exist to update.
• printable() in src/core/commands/ - absent (grep -rn "function printable" finds nothing).
• MAX_ERROR_CHARS in src/core/commands/status.js and src/core/commands/daemon.js - absent, so the "existing error clamp of 400" the issue weighs a new width against is not a thing on master.
• The assembly site is already raw on master: src/core/daemon/status.js:1044 interpolates remote_config_rolled_back with no sanitizeLabel, and the long explaining comment is not there either. The "revert src/core/daemon/status.js:1062 to raw interpolation" half of option (a) is therefore a no-op here.
• The render choke point is src/core/commands/status.js:545 on master ([${tag}] ${d.kind}: ${d.message}), plain and uncleaned.

So on master there is no option (b) to convert into option (a). Cleaning at line 545 here would not be this follow-up: it would be a second, competing implementation of #776, racing an approved and mergeable #777 on the same lines. Pushing onto #777's branch instead would move the head of a green, reviewed, approved PR and send it back through triage. Both calls are the maintainer's, not mine.

This repeats the note left at 2026-08-15; nothing about it has changed except that master has moved on (#789, #792, #794) while #777 has not landed.

Blocker 2: the clamp width is still an open policy question

Even on #777's branch, the issue is explicit that a number has to be chosen and defended across all 14 diagnostic kinds. That number is still unchosen, and I confirmed the bind is real at master:

• 14 distinct kind: values in src/core/daemon/status.js (unchanged; hyp status reports proxy-mode trust and the launchd env (LLP 0237/0239) #792 added a report field, not a kind).
• The longest template, client_attached_not_configured at src/core/daemon/status.js:967, is 160 characters of fixed prose before any substitution, and about 188 with realistic values (claude twice, @hypaware/claude once). sanitizeLabel's 120 default cannot hold the project's own sentence, so a naive reuse would truncate a correct message on a healthy machine.

Picking a width is a product judgement about how much of an operator's own repair sentence may be cut to bound a hostile one. I am not going to invent it and call it verified.

Questions

1. Sequencing. Which do you want?
   • (a) Merge hyp daemon status cleans what it reads out of status.json, plus the etag and probe-error residue from #681 (#776) #777 first, then I open a normal fix/issue-784 PR against master. Cleanest, and the option this issue was written for.
   • (b) I land option (a) as a fixup commit on fix/issue-776 before hyp daemon status cleans what it reads out of status.json, plus the etag and probe-error residue from #681 (#776) #777 merges. Cheapest in total diff, but it moves an approved PR's head and it re-opens review on it.
   • (c) I open a stacked PR based on fix/issue-776 rather than master, to be merged after it. Keeps hyp daemon status cleans what it reads out of status.json, plus the etag and probe-error residue from #681 (#776) #777 untouched, at the cost of a stacked branch.
2. The clamp width, needed under any of the three. Options:
   • 400, reusing MAX_ERROR_CHARS so there is one number in the file. Tight: a message naming a full path plus the 160-character template can approach it.
   • 600 or 800, chosen to leave clear headroom over the longest template with a substituted path. Needs your blessing as a new constant.
   • Clamp per interpolated component at the collector and leave the render doing strip-only with no length cap, so the assembled sentence is never truncated. This departs from "one clamp at the choke point" but keeps every fixed prose intact.
3. Is this still worth doing at all? Both hyp daemon status cleans what it reads out of status.json, plus the etag and probe-error residue from #681 (#776) #777 round 2 and triage classified it a preference, not a blocker: nothing hostile reaches a terminal on either surface today and --json's structured values are already byte-exact. If the answer is that the LLP 0225 purity of diagnostics[].message is not worth a new clamp constant, closing this issue is a legitimate outcome and I will not re-attempt it.

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

[philcunliffe]

Re-triage of PR #777 at 5a392193: this issue still stands, plus three small additions to its record

PR #777's head moved twice since the last triage (d21b7479, the conflict resolution when #681/#794/#797 and the rest of the batch merged to master; 5a392193, a refresh merge of a now-green master). Neither move changed this issue's substance. Re-verified at the new head, with line drift and two related deferred findings recorded here so they are not lost.

1. The option (a) targets drifted again

• Assembly of remote_config_rolled_back from sanitizeLabel'd components: now src/core/daemon/status.js:1166 (issue body says 1062, prior note said 1066).
• The single ${d.message} render interpolation: now src/core/commands/status.js:607 (issue body says 559, prior note said 590).
• MAX_ERROR_CHARS: now src/core/commands/status.js:319 and src/core/commands/daemon.js:66.
• Still 14 distinct diagnostic kind: values; the contract-pinning tests are unchanged at test/core/status-hostile-non-status-file.test.js and pass at head.

2. New deferred finding from the #681 merge: three copies of the same one-line helper

Foreseen in the merge-order note on PR #777 and now real on its branch. After the conflict resolution collapsed the two independently-invented printable()s in src/core/commands/status.js into one (both @refs kept, {unknown} signature kept, one MAX_ERROR_CHARS), the repo still carries:

• src/core/commands/status.js:352 printable()
• src/core/commands/daemon.js:88 printable() (byte-identical body) plus daemon.js:101 printableNumber()
• MAX_ERROR_CHARS = 400 defined once per file (status.js:319, daemon.js:66)

The obvious follow-up is to lift printable / printableNumber and the one constant into a small shared module and have all call sites read from it. Classification: preference (non-behavioural refactor); duplicating a one-liner is safe, just untidy.

3. PR #800 (displayableCaHosts) adjacency, checked

#800 sanitizes certificate-derived host lists in the collector via its own displayableCaHosts in src/core/tls/ca.js. It defines no printable and no MAX_ERROR_CHARS, so there is no symbol collision with #777's surface; its hunks in src/core/commands/status.js and src/core/daemon/status.js sit in disjoint regions (proxy-trust block and a new describePermittedHosts helper), so at worst a trivial textual merge. It is a different value class with its own documented rationale; nothing for this issue to absorb beyond awareness.

Backlink: PR #777, head 5a392193ec9f299f8e9dc24ba01daa412106f1ff.

[philcunliffe]

Closing as a declined preference. The merged behavior is safe and tested; changing where diagnostic prose is clamped requires inventing a universal width across all diagnostic kinds, while the issue itself records no current terminal leak or structured-value loss.