Fix #1232: Follow-up: deferred review findings from PR #1227

[philcunliffe]

Feature or issue

Issue #1232 carries three review findings deferred from PR #1227 at the review-round cap, each triaged as a preference rather than a production risk. (1) warnDroppedOptionals (src/core/registry/commands.js) carried @ref LLP 0329#stderr-mirror [implements], but 0329 settles that opt-in for containment refusals and #not-every-warn declines to widen it, while this site refuses nothing: registration succeeds by design and only the absence is left. LLP 0335 #not-a-fifth-mirror still read "four named containment refusals", so the corpus did not predict the tree. (2) A command class whose base supplies defaults as prototype getters warns registered without the declared 'aliases', 'hidden' at every process start, although the stored record behaves identically. (3) OPTIONAL_MEMBERS is a hand-written copy of CommandRegistration's optional keys with nothing keeping the two in step, so an optional member added to the published interface would silently escape the diagnostic.

Solution

• Item 1 takes the issue's option (a): LLP 0362 extends LLP 0329 and states the rule by the property it always turned on, so a report whose subject is observable only as an absence takes the mirror whether or not its site refused. 0329's Extended-by: gains it, 0335 #not-a-fifth-mirror gains a forward-ref note that "four" is a census at its head, and the @ref above warnDroppedOptionals and its test counterpart point at LLP 0362#absence-not-refusal. Annotation-only, so no test can reproduce it: verified by inspection of the committed tree and by a ref-check pass over both files (8 references, 0 broken, both new ones resolving to a live anchor in an Accepted doc).
• Item 2 is recorded, not coded, in LLP 0362 #prototype-default: telling a default from a declared value means reading the member, and the probe is presence-only by design (pinned at reads === 0), so the line is an accepted cost with the same remedy docs/PLUGIN_AUTHORING.md already gives.
• Item 3 adds the warning covers every optional member CommandRegistration declares to test/core/command-registry-register.test.js. It reads the optional keys out of hypaware-plugin-kernel-types.d.ts, puts every one of them on a prototype, and asserts the WARN names exactly that set, so the guard holds however the coverage is spelled and no production code changes. Verified by adding a probe optional member to the interface: the test fails (+ 'ordinal') and passes again once removed.

npm test (5818 pass, 0 fail, 1 skipped), npm run typecheck, and npm run build:types are green.

Code: +6 / -5 lines

Fixes #1232

[philcunliffe]

Verdict: changes requested, and applied. Three findings, all fixed on this branch and verified in the committed tree. Head is now 7322605c. npm test (5818 pass / 0 fail / 1 skipped), npm run typecheck and npm run build:types are green, and the corpus ref-check is clean (3414 refs, 0 broken).

Findings

1. MEDIUM. llp/0362-an-absence-only-degradation-opts-into-the-stderr-mirror.decision.md:65 (was #fifth-mirror): the census stated in the new decision does not match the tree.
The section claimed "the mirrored lines are now the four refusals plus this report" and "this one is the first that is not a refusal". At fd34b7ca there are eight mirrorStderr: true call sites, not five: src/core/capture_spool.js:78, src/core/cache/spool.js:543, src/core/cache/sweep_guard.js:47, src/core/cache/partition.js:445, :478, :533, :586, and src/core/registry/commands.js:328. LLP 0344 (#1162) added the unreadable-cursor refusal after 0329 named its four, and LLP 0334 #recovery-is-announced added two .info mirrors (noteEscapeCleared, noteUnreadableCleared) that retract a refusal rather than make one, so "the first that is not a refusal" was false too. This matters because the section instructs an auditor to count from it, and once 0362 is Accepted the error is only correctable by minting another LLP.
Fixed: the section is now ## The opt-in set is open, and no refusal stands behind this entry {#not-a-refusal}. It names the eight sites' provenance, drops the arithmetic, and states the claim that is actually true and load-bearing: no containment refusal stands behind this report at all, neither made nor retracted. The same wording was propagated to the header glosses and to the inline note in LLP 0335 (:189).
Also fixed, same finding: #absence-not-refusal stated the rule as a set identity ("the opt-in set is the set of reports whose subject is observable only as an absence"), which the two recovery announcements do not satisfy. Restated as the sufficient condition it is ("a report whose subject is observable only as an absence is inside the opt-in set").
Note: the @ref target anchor #absence-not-refusal is unchanged, so both annotations still resolve. Nothing referenced the retired #fifth-mirror anchor.

2. MEDIUM. test/core/command-registry-register.test.js:423: the drift guard failed open on two of the three spellings the declaration file uses.
declaredOptionalMembers() matched only /^[ \t]*(name)\?:/. An optional method (complete?(argv: string[]): string[]) and a readonly foo?: T member each escaped the parse; OPTIONAL_MEMBERS in src/core/registry/commands.js:256 would also omit them, the two sides then agreed, and the test passed. The optional method is the exact shape that always lives on a class prototype, which is what { ...command } drops, and hypaware-plugin-kernel-types.d.ts already uses foo?(...) about ten times (deactivate?, unregister?, flush?, ...). A guard that cannot fail on the drift it was minted for (issue #1232 item 3) is worse than none.
Fixed: capture widened to /^[ \t]*(?:readonly[ \t]+)?([A-Za-z_$][\w$]*)\?[:(]/gm, with the JSDoc saying why. Consequence bullet added to LLP 0362 #consequences.
Mutation evidence, probe inserted into interface CommandRegistration and reverted each time (git status clean on the .d.ts after all three):

probe	before	after
ordinal?: number	test 18 fails	test 18 fails
readonly ordinal?: number	passes (hole)	test 18 fails
complete?(argv: string[]): string[]	passes (hole)	test 18 fails

Exactly one test fails in each case, with the diff naming the probe.

3. LOW. llp/0335-...decision.md:13 and llp/0362-...decision.md:9: the 0335 extension was recorded only inline, not in either header.
0362's body and Related: line both said it extends 0335 #not-a-fifth-mirror's census, and 0335 carried the inline **Extended-by [LLP 0362]** note, but 0335's header Extended-by: named only LLP 0337 and 0362's Extends: named only LLP 0329. The corpus records this relation on both headers plus the inline note (LLP 0337 / 0335 is the precedent in this very file).
Fixed: 0362 added to 0335's header Extended-by:, 0335 added to 0362's Extends:, 0335 removed from 0362's Related: (and 0334 / 0344 added there, since 0362 now cites them). Mechanical forward-refs only; nothing either Accepted doc settled is edited.

Verified clean, no finding

• LLP numbering: no collision. After git fetch --prune --all (1077 refs), node scripts/llp-numbers.js check on this branch reports "1 LLP number minted against refs/remotes/origin/master, no collision". Enumerating llp/036* across every ref yields exactly one filename per number: 0360, 0361, 0362-an-absence-only-degradation-opts-into-the-stderr-mirror.decision.md (this branch only), 0363. The sibling that briefly claimed 0362 has renumbered; next returns 0364.
• Immutability of 0329 / 0335. Both diffs are additive forward-refs. No sentence either doc settled is altered, added to, or removed.
• 0362's header and anchors. Type: Decision, Status: Accepted, Systems, Generated-by, Author, Date all present and in corpus form. Every ./NNNN-*.md link in the doc resolves. #absence-not-refusal exists and is what both new @refs point at.
• ref-check: 3414 references corpus-wide, 0 broken. The 4 warnings (superseded targets in LLP 0101, 0102, 0172, 0173) predate this PR. llp-ref-hygiene and llp-number-minting tests pass.
• Style: no em dash (U+2014) and no NUL byte in any changed file; no semicolons added in JS; no new runtime dependency (node:fs is stdlib); the new test reuses the existing stderrTextFrom helper rather than adding one.
• Finding 2 of the issue (prototype-resident defaults reporting as degraded) is correctly recorded rather than coded, in LLP 0362 #prototype-default, with the presence-only probe (reads === 0) as the reason.
• Production change in src/core/registry/commands.js is comment and @ref only; no behavior change.

Residuals

None blocking. One observation, not filed: declaredOptionalMembers() terminates the interface body at the first \n}, which is correct today and fails loudly (not open) if it ever truncates early, since a short declared list no longer matches the WARN. Left alone.

[philcunliffe]

Verdict: changes requested, and applied. Round 2 of 2. Two findings, both fixed on this branch and verified in the committed tree. Head is now 723e75e4. npm test (5818 pass / 0 fail / 1 skipped, three consecutive runs), npm run typecheck and npm run build:types are green; corpus ref-check is clean (3456 refs, 0 broken); node scripts/llp-numbers.js check after git fetch --prune --all reports no collision.

Re-verification of round 1's fixes

The mirrorStderr recount is independent and agrees. Eight mirrorStderr: true call sites under src/, no more and no fewer: capture_spool.js:78, cache/spool.js:543, cache/sweep_guard.js:47, cache/partition.js:445, :478, :533, :586, registry/commands.js:328. I then read each one to check the claim rather than the count. Seven either make a containment refusal (emptying nothing, draining nothing, reclaiming nothing, reportUnreadableCursor, reportEscapingTableDir) or retract one (noteEscapeCleared, noteUnreadableCleared, both INFO, both @ref LLP 0334#recovery-is-announced). The eighth, warnDroppedOptionals, does neither. #not-a-refusal's restated claim holds as written.

#absence-not-refusal's sufficient-condition wording holds. "A report whose subject is observable only as an absence is inside the opt-in set" is a one-way implication, so the two recovery announcements (which are in the set without being absence-only) cannot falsify it, where the old set-identity phrasing they did falsify. Round 1's restatement is correct, and 0329's new Extended-by: gloss carries the same one-way form.

Anchor integrity across the rename is intact. #absence-not-refusal, #not-a-refusal, #still-not-every-warn, #prototype-default, #consequences, #references all exist in 0362; both @refs (src/core/registry/commands.js:322 [implements], test/core/command-registry-register.test.js:268 [tests]) point at #absence-not-refusal and resolve; no reference anywhere targets the retired #fifth-mirror. Every ./NNNN-*.md link in 0329, 0335 and 0362 resolves, and every prose anchor 0362 cites resolves (0329#stderr-mirror/#not-every-warn/#testable/#consequences, 0335#not-a-fifth-mirror/#never-throws, 0248#semantic-boot, 0334#recovery-is-announced, 0323#say-it/#one-gate, 0332#transition-plus-rewarn, 0189#choke-point).

0329 and 0335 receive mechanical forward-refs only. Both diffs are additive: a header Extended-by: entry each, plus one inline **Extended-by [LLP 0362]** note in 0335 following the 0337 precedent already in that file. No sentence either Accepted doc settled is altered or removed.

Findings

1. MEDIUM. llp/0362-...decision.md:70 (#not-a-refusal): the corrected census got the provenance of the fifth refusal wrong.
The section read "because LLP 0344 added the unreadable-cursor refusal". LLP 0344 is a Draft RFC ("Retention Under Fault") whose own preamble says "It decides nothing." Nothing in it settles a stderr mirror; its #unreadable-cursor section is about what a retention window means for such a partition. The refusal is reportUnreadableCursor in src/core/cache/partition.js:533, annotated @ref LLP 0323#say-it and @ref LLP 0329#stderr-mirror, and landed in 7c15f9ea (#1162), the PR that also carried the 0344 RFC. Same class of error round 1 was fixing: a section that instructs an auditor to trace the set points them at a document that decided nothing, and once 0362 is Accepted only another LLP can correct it.
Fixed: the sentence now names the function, the decision it implements, and the PR: "reportUnreadableCursor in src/core/cache/partition.js became a fifth containment refusal on 0329's own terms (LLP 0323 #say-it, added in #1162)". Related: swaps LLP 0344 for LLP 0323, which the body now cites and 0344 no longer appears in the doc at all (verified: zero occurrences of 0344 in the committed 0362).

2. LOW. test/core/command-registry-register.test.js:428: the widened drift guard still failed open on a spelling, because it required the ? to sit hard against the : or (.
/...([A-Za-z_$][\w$]*)\?[:(]/gm does not match ordinal ?: number or ordinal? : number, both legal TypeScript. There is no formatter in this repo (no prettier/eslint/biome config, no format script), so hypaware-plugin-kernel-types.d.ts is hand-written and no spelling is normalized away. A member written that way would escape declaredOptionalMembers() and OPTIONAL_MEMBERS, the two sides would agree, and the test would pass covering nothing: exactly the failure mode round 1 minted the widened capture to close.
Fixed: capture is now /^[ \t]*(?:readonly[ \t]+)?([A-Za-z_$][\w$]*)[ \t]*\?[ \t]*[:(]/gm, with the JSDoc saying why.

Mutation evidence. Every probe inserted into interface CommandRegistration, run, then reverted; git status on the .d.ts clean after each. Baseline 18/18 pass.

probe	at 7322605c	at 723e75e4
ordinal?: number	test 18 fails (- 'ordinal')	test 18 fails
readonly ordinal?: number	test 18 fails	test 18 fails
complete?(argv: string[]): string[]	test 18 fails (- 'complete')	test 18 fails
ordinal ?: number	passes (hole)	test 18 fails
ordinal? : number	passes (hole)	test 18 fails
readonly complete? (argv: string[]): string[]	passes (hole)	test 18 fails

Exactly one test fails in each case and the diff names the probe. Negative probes, all still 0 failures (no false match): /** Note: ordinal?: number was considered. */ (a ?: inside a JSDoc line), a multi-line JSDoc block containing ordinal?: number, a required member ordinal: number, a required method run2(...), and a one-line conditional type tagged: string extends never ? never : string. The ^[ \t]* prefix is what makes comment lines unmatchable: a JSDoc continuation begins with *, never an identifier.

3. LOW, fixed as part of finding 1. llp/0362-...decision.md:148 (#consequences): the pin was recorded as bidirectional; the test runs one way.
"OPTIONAL_MEMBERS ... is now pinned to the optional members of CommandRegistration by test" overstates it. A key left in OPTIONAL_MEMBERS after its member is removed from the interface is never put on the test's prototype, so copyMiss returns '', the WARN never names it, and the assertion still passes. The consequence clause that follows ("a member added ... cannot escape") is the direction that holds. Worth correcting before the doc lands, since an Accepted doc's overstatement is not editable afterwards.
Fixed: restated as "a test now pins it to cover every optional member CommandRegistration declares ... The pin runs that direction only: a key left in the list after its member leaves the interface is named by nothing and costs nothing." No code change: closing the reverse direction would mean exporting OPTIONAL_MEMBERS from src/core/registry/commands.js purely for a test, and a stale key costs at most one name in a line that only fires on an already-degraded registration.

Verified clean, no finding

• LLP numbering. After git fetch --prune --all, check reports "1 LLP number minted against refs/remotes/origin/master, no collision". Enumerating llp/036* across every remote ref: 0360 and 0361 on bundle-github-plugin, 0362 on fix/issue-1232 only, 0363 on fix/issue-999. next returns 0364. (survey reports pre-existing duplicates at 0249/0265/0266/0267/0268 on unrelated branches; none is ours.)
• ref-check: 3456 references corpus-wide, 0 broken. The same 4 warnings (superseded targets in LLP 0101, 0102, 0172, 0173) predate this PR. llp-ref-hygiene, llp-ref-hygiene-absent-path, house-style-nul-byte and llp-number-minting all pass.
• Style: no U+2014 and no NUL byte in any changed file (byte-level scan, not a visual one); no semicolons added in JS; JSDoc types only; no new runtime dependency (node:fs is stdlib).
• Production surface: src/core/registry/commands.js is still comment and @ref only. No behavior change anywhere in the PR.
• Item 2 of issue Follow-up: deferred review findings from PR #1227 #1232 (prototype-resident defaults reporting degraded) remains correctly recorded rather than coded, in #prototype-default, with the presence-only probe (reads === 0) as the reason.

Residuals

LOW, not fixed, and round 1's stated reason for leaving it was wrong. declaredOptionalMembers() terminates the interface body at types.indexOf('\n}', start), the first } in column 0. Round 1's residual note said an early truncation "fails loudly (not open)". It does not: the test builds its prototype from declared, so a short declared produces a correspondingly short WARN and assert.deepEqual still passes, with assert.ok(declared.length > 0) the only check on the parse. It is left alone on the merits instead: for a \n} to fall inside the interface body a line would have to begin with } in column 0, which no JSDoc continuation ( *) or fenced example inside one can produce, and which npm run typecheck / npm run build:types would reject as malformed. Recorded here so the triage rung has the correct reasoning rather than the reassuring one.

Observation, not a finding. One npm test invocation out of four reported 5817 pass / 1 fail; the next three runs were 5818 pass / 0 fail and the failing subtest name was lost to output truncation. Nothing in this PR is time-, order- or IO-dependent (three LLP docs, two comments, one regex over a file read synchronously), so this reads as a pre-existing intermittent in the suite rather than anything this branch introduced. Flagging it because it is unproven either way.

[philcunliffe]

Triage of round 2's residuals at head 723e75e4b2dbf9a5ad50480fff7baa8bb615de18: no residual is a production risk, so each open finding is deferred to its own issue.

Finding 1 -> #1248. declaredOptionalMembers() terminating the interface body at the first column-0 } (test/core/command-registry-register.test.js:427). Verified round 2's corrected reasoning against the committed tree: the failure mode is fail-open (a truncated declared list shrinks the WARN to match and the test still passes), but the body today holds no nested braces, no JSDoc continuation can start a line with }, and a genuine early close of the interface would orphan the remaining members and fail typecheck / build:types. The only reachable hole is a nested object-type member with an unindented closing brace, a spelling this uniformly indented hand-written file does not use. Test-only robustness preference; deferral is safe.

Round 2's unproven observation (one npm test run of four showing a single unnamed failure): not reproduced. Nine consecutive npm test runs at head 723e75e4 in a fresh worktree all reported 5818 pass / 0 fail / 1 skipped, with full output captured per run. Nothing in this PR is time-, order- or IO-dependent, and no failure could be observed to attribute. No issue filed; if it recurs with a captured subtest name, file it against the suite then.

[philcunliffe]

Ship risk: low

Who could be affected: No one. Nothing here can change what a person using HypAware sees or does.

Why this level: Only one file in this change ships to users, and the edit to it is a comment. Compared instruction by instruction, the program before and after is identical, so every command, every recording, and every stored result behaves exactly as it does today. The rest of the change is written design notes and one internal test, and neither is installed or run by users. Nothing touches recorded data, privacy, credentials, configuration, or the background service.

What was checked:

• The shipped file was proved unchanged in behaviour, rather than judged by reading the patch.
• The full test suite passed 12 times in a row with no failures, so the single earlier failure someone reported once did not reappear.
• The new internal test was run 30 more times under varied machine settings, and deliberately broken eight ways, confirming it only complains when it genuinely should and cannot disrupt unrelated work.
• All eight automated checks are passing.

Sitting one commit behind the main branch does not affect this: that commit changes entirely separate files.