docs(solutions): compound five queued learning proposals - #3868
Conversation
fro-bot
left a comment
There was a problem hiding this comment.
Four docs that argue "a check which cannot fail is worse than a missing check," and one of them ships a code example that cannot demonstrate the vulnerability it exists to teach. The PR description walks back two overclaims; here is the third.
I verified what is verifiable: all five merge SHAs resolve (3bd59db, 498f33d, 91388ec, 69a99af, 796ddf2), every cross-link target exists on disk, guardedPatterns() is a function at scripts/check-wiki-authority.ts:36 with no g/y flags as claimed, and the corpus frontmatter gate passes (scripts/solutions-query.test.ts — 60 passed). The prose is accurate. The examples are where it drifts.
Verdict: CONDITIONAL
Blocking issues
docs/solutions/best-practices/exact-match-trust-gates-need-type-discipline-2026-09-08.md:84-90 — the Examples block demonstrates neither the vector nor the fix.
// Before: a one-element array reaches this and compares equal.
if (source.url === expectedUrl) { /* trusted */ }It does not compare equal. === performs no type conversion — ["https://github.com/acme/widget"] === "https://github.com/acme/widget" is false. Confirmed at runtime:
=== : false
== : true
URL : https://github.com/acme/widget
So the "Before" line already rejects the decoy (it over-blocks, it does not fail open), and the "After" line adds a typeof conjunct in front of a comparison that was never the hazard. A reader who applies this patch has changed nothing.
The doc's own prose at line 33 gets it right — "== coerces; === against an already-coerced intermediate is no better" — and the repo's real code and real test both agree. sourceUrlMatchesRepo calls new URL(sourceUrl) at scripts/check-wiki-private-presence.ts:118; the URL constructor's WebIDL USVString conversion stringifies its argument, so the array parses cleanly and matches. The implemented defence is the typeof url === 'string' filter in parseFrontmatterSources at line 91, and scripts/check-wiki-private-presence.test.ts:698 pins exactly that with a comment naming new URL(arrayValue) as the coercion site.
Remediation: replace the example with the actual coercion site so the "Before" fails open as written.
// Before: `new URL()` stringifies its argument, so a one-element array parses and matches.
if (sourceUrlMatchesRepo(src.url, owner, name)) { /* trusted */ }
// After: shape is established at extraction, before any coercing consumer sees the value.
if (typeof src.url === 'string') urls.push(src.url)Also narrow the title. "A one-element array compares equal to its contents" is true only under toString(); as stated it asserts the thing the example gets wrong.
Why this blocks: scripts/solutions-query.ts injects these docs verbatim into agent prompts (this review run received one in its own context). A wrong example in a retrieval corpus is not a typo — it is a future agent confidently applying a no-op at a promotion-path trust gate.
Non-blocking concerns
Missing last_updated and verified on all four new docs. isStale() at scripts/solutions-query.ts:253 returns false when lastUpdated === null. These docs are therefore structurally incapable of being flagged stale, and render Last updated: unknown forever. The immediately preceding doc in this same series, equivalence-refactors-need-differential-proofs-past-the-bound-2026-09-06.md, carries both keys — so this is drift within the series, not inherited debt (18 of 59 corpus docs lack it). A doc arguing that a vacuous check reads as coverage should not opt itself out of the freshness signal. Add last_updated: 2026-09-08 and verified: 2026-09-08.
Example fidelity in the other two docs. Neither changes the lesson, both weaken a corpus whose thesis is docs-vs-code drift:
a-docstring-that-enumerates-forms-is-a-test-table-2026-09-08.md:63showsisPureReexportBarrel(line). Real signature isisPureReexportBarrel(repoRelativeFilePath, readSource)(scripts/mutation-guards-config.test.ts:236) — it reads a file, it does not classify a line. The real truth table already exists at line 739 of that file; point at it.a-mutation-score-can-measure-nothing-2026-09-08.md:76,80illustrates/^knowledge\/wiki\/[^/]+\/[^/]+\.md$/. The shipped pattern is.+\.md$in the second segment (scripts/check-wiki-authority.ts:38) — a different matcher with different nesting behavior.
Dedup rationale is sound. Folding #3849 and #3852 into one doc is correct — static-const mutants and dist-imported tests are two mechanisms for the same silence, and splitting them would have produced two docs competing for the same retrieval query.
Missing tests
None required — docs-only, and the corpus frontmatter gate covers parseability.
Worth noting as the gap that let the blocking finding through: nothing verifies that ts blocks in docs/solutions/** compile, or that identifiers they name exist with the arity shown. Three of these four docs argue for precisely that discipline — "drive every enumerated form through the actual matcher," "prove each rule discriminates." The corpus does not apply it to itself. Not this PR's job, but it is the obvious next unit.
Risk assessment
LOW-MED.
LOW on the conventional axes: no workflow, permission, dependency, or action-pin surface; no any, @ts-ignore, or console; no executable code paths touched; the two modified files are additive cross-links only.
MED on the one that matters here: this corpus is a retrieval surface feeding autonomous agents. An incorrect example about coercion at a trust boundary does not sit inert in a directory — it gets selected, injected, and acted on. The docs land clean; the example in one of them teaches a defence that isn't one.
Fix the Examples block in the trust-gate doc, add the freshness keys, and this merges.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/.github |
| Run ID | 34182648733 |
| Cache | hit |
| Session | ses_f80ff94efffeRZHohPo0EoN7tp |
a697871 to
0137ba1
Compare
fro-bot
left a comment
There was a problem hiding this comment.
All three findings from the prior review are resolved, and the blocking one was fixed at the right layer rather than patched at the surface.
The trust-gate doc no longer teaches a no-op. Context now states the mechanism correctly — === rejects the array outright, and the hazard is every downstream consumer that stringifies — and the Examples block names the real coercion site:
// Before: sourceUrlMatchesRepo calls `new URL(sourceUrl)`, which stringifies its
// argument, so the one-element array parses and matches.The "After" is now parseFrontmatterSources' actual filter, lines 90-91 of scripts/check-wiki-private-presence.ts, reproduced faithfully including the optional-chain cast. The closing line — "the fix belongs at extraction, not at the comparison" — is the part that makes this doc worth retrieving. A reader now learns the guard goes upstream of new URL, which is the non-obvious half.
Verdict: PASS
Blocking issues
None.
Non-blocking concerns
Stale H1 in exact-match-trust-gates-need-type-discipline-2026-09-08.md:23. The frontmatter title was narrowed to "establish shape before a coercing consumer sees the value" — correct. The # heading one line below still reads "a one-element array compares equal to its contents," the claim the very next paragraph refutes. Retrieval renders the frontmatter title as the section header and the H1 inside the body excerpt, so both reach an agent's context two lines apart, disagreeing. Low blast radius because the correction is immediately adjacent, but it is the last residue of the original error. Sync line 23 to the frontmatter title.
Observation, not a finding against this PR: verified: 2026-09-08 short-circuits isStale() at scripts/solutions-query.ts:252 before last_updated is ever consulted, so these docs are exempt from the staleness signal by a different route than the one I flagged. That is the corpus convention — equivalence-refactors-need-differential-proofs-past-the-bound-2026-09-06.md does the same — and last_updated now renders a real date instead of unknown, which was the actual retrieval defect. Whether verified should mean "never stale" or "stale-clock reset" is a corpus-level question worth its own issue, not a change to make here.
Missing tests
None required. Docs-only; the corpus frontmatter gate passes (scripts/solutions-query.test.ts — 60 passed, 22 skipped).
Verification I ran on this revision:
last_updated: 2026-09-08parses as a string, not a YAML timestamp, under this repo'syamlpackage — sosolutions-query.ts:167retains it rather than nulling it. Confirmed empirically and corroborated by this run's own injected context, which rendersLast updated: 2026-09-08for the policy-scanner doc.guardedPatterns(): readonly RegExp[]and the/^knowledge\/wiki\/[^/]+\/.+\.md$/pattern now matchscripts/check-wiki-authority.ts:36-38exactly, return type included.isPureReexportBarrel('fixture.ts', () => source)matches the real 2-arity signature, and() => sourceis a validSourceReaderperscripts/check-mutation-guards.ts:996.
The doc-example gate I raised last round is still the obvious next unit — nothing verifies that ts blocks under docs/solutions/** name real identifiers at real arity. Three of these four docs argue for exactly that discipline, and the corpus still does not apply it to itself. That is an issue, not a merge condition.
Risk assessment
LOW.
No workflow, permission, dependency, or action-pin surface. No any, @ts-ignore, or console. No executable path touched; the two modified files remain additive cross-links.
The MED I carried last time was retrieval-corpus poisoning — an incorrect coercion example getting selected, injected, and acted on at a promotion-path trust gate. That vector is closed. What ships now is a doc that traces the decoy from YAML through new URL to the extraction filter that stops it, which is the shape a future agent actually needs when it meets this at 3am with nobody watching.
Merge it.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/.github |
| Run ID | 34183013811 |
| Cache | hit |
| Session | ses_f80ff94efffeRZHohPo0EoN7tp |
The capture-learnings loop had five proposals sitting undispositioned from the counterexample-proven-guards work. Four docs, because #3849 and #3852 are one learning: a mutation score that reports a number while measuring nothing, by two mechanisms. A top-level const is evaluated once per Stryker worker, so mutants inside a decision table are structurally unreachable and the module reports zero killed. check-wiki-authority.ts went 0 to 146 killed with no new tests. Separately, a test importing through compiled dist exercises the artifact rather than the instrumented source, so a module can sit in mutate with tests that read like coverage and produce no signal. A one-element array stringifies to exactly its element, so an exact-match attribution gate accepts ["https://..."] as the URL it contains. And deleting a defensive conjunct relocates the check into an invariant no signature enforces. A docstring enumerating the forms a matcher accepts is a specification nothing verifies; drift fails open. A policy scanner has to parse tokens rather than substring-match prose that contains author-written text, and cross-check report keys against the configured scope, because silence about a module is not evidence about it. The report-level empty-reason check could never fire. Closes #3849, #3850, #3851, #3852, #3853
0137ba1 to
630223b
Compare
fro-bot
left a comment
There was a problem hiding this comment.
The H1 residue is cleared. All four docs now carry a # heading byte-identical to their frontmatter title, so the two strings that reach an agent's context — section header from title, body excerpt from the H1 — no longer disagree.
Nothing else moved. Same +384 across the same six files, and the substance I verified on the previous revision still holds:
- Trust-gate Examples block names the real coercion site (
sourceUrlMatchesRepo→new URL) and puts the fix at extraction, matchingparseFrontmatterSourceslines 90-91. /^knowledge\/wiki\/[^/]+\/.+\.md$/andreadonly RegExp[]matchscripts/check-wiki-authority.ts:36-38.isPureReexportBarrel('fixture.ts', () => source)matches the real 2-arity signature and a validSourceReader.- Corpus frontmatter gate green —
scripts/solutions-query.test.ts, 60 passed, 22 skipped.
Verdict: PASS
Blocking issues
None.
Non-blocking concerns
None. Every finding from the two prior rounds is resolved.
The verified: semantics question stands as noted last round — verified: 2026-09-08 short-circuits isStale() at scripts/solutions-query.ts:252 before last_updated is read, so "verified" currently means "never stale" rather than "stale-clock reset." That is the existing corpus convention across every doc that sets the key, not a defect this PR introduced. Worth an issue against solutions-query.ts, not a change here.
Missing tests
None required. Docs-only; parseability is gated and passing.
The doc-example gate remains the open follow-up: nothing verifies that ts blocks under docs/solutions/** name real identifiers at real arity. It took three review rounds to converge these four examples by hand, which is itself the argument for the check. Three of these docs make that case explicitly — "drive every enumerated form through the actual matcher," "prove each rule discriminates." Filing it is the right next unit.
Risk assessment
LOW.
No workflow, permission, dependency, or action-pin surface. No any, @ts-ignore, or console. No executable path touched; the two modified files remain additive cross-links.
The retrieval-poisoning vector that carried MED in round one is closed. What lands is a corpus entry that traces the decoy from YAML through new URL to the filter that stops it — and, now, one whose title says the same thing in both places it gets read from.
Merge it.
Run Summary
| Field | Value |
|---|---|
| Event | pull_request |
| Repository | fro-bot/.github |
| Run ID | 34183326804 |
| Cache | hit |
| Session | ses_f80ff94efffeRZHohPo0EoN7tp |
… gaps Fro Bot's PR #3870 review reproduced two blocking defects in the gate added in a5d346d: 1. Both directive forms (the `<!-- verify -->` annotation comment and a fence's `fragment` info-string modifier) were accept-or-ignore, with no reject path. A typo'd keyword, a malformed annotation body, or an unrecognized fence modifier silently disabled the check for that block instead of failing the gate -- reproducing the exact #3868 failure shape (a defect that reads as verified but isn't) inside the tool built to catch it. Both now emit a directiveFindings entry and fail the gate on anything that looks like it was meant to be a directive but isn't parseable. A near-miss fence language tag (e.g. `typescrpt`) is also flagged via a small Levenshtein check bounded to a distance of 2 and gated on an allowlist of every non-code fence language actually used in the corpus, so real tags (yaml, bash, sh, ...) never false-positive. 2. The fence-detection regex was anchored to the start of the line, so a fence indented inside a list item (three confirmed in the corpus) was never matched at all -- 'blocks checked' undercounted the real corpus. Fences are now matched at any indentation and dedented by the opening fence's own indent before parsing; block count goes from 99 to 101 (net of the two indented-fence-only line-51 relabel already landed). Also fixes three non-blocking issues Fro Bot flagged: - findSignature now resolves only sourceFile.statements (real top-level declarations), not a full-tree walk -- a nested same-named helper no longer shadows the intended top-level symbol. - checkAnnotation returns every wrong-arity call in a block instead of the first, so a block with several violations is reported in one pass. - An annotation's file path is resolved and checked against the repo root before being read, rejecting any path (e.g. via ../../) that would resolve outside it. 12 new tests (26 total), each new reject path proven red before the fix.
The
capture-learningsloop had five proposals sitting undispositioned from the counterexample-proven-guards work. It opens them automatically and dedups againstdocs/solutions/, so leaving them queued stalls the loop at its last step.Four docs, not five. #3849 and #3852 are one learning — a mutation score that reports a number while measuring nothing — by two mechanisms that share a search intent.
What each one carries
A mutation score can measure nothing. A module-level
constis evaluated once per Stryker worker, so mutants inside a decision table are fixed for the worker's lifetime and unkillable.check-wiki-authority.tswent 0 → 146 killed with no new tests written; the assertions had always been adequate. Separately, a test importing through compileddistexercises the artifact rather than the instrumented source, so a module can sit inmutatewith tests that read like coverage and contribute nothing. Includes the conditions under which the const→function conversion stops being neutral —g/yflags makinglastIndexobservable, post-construction mutation, encoding changes.An exact-match trust gate needs type discipline. A one-element array stringifies to exactly its element, so
["https://github.com/acme/widget"]satisfies an exact-match attribution comparison. And deleting a defensive!== undefinedconjunct relocates the check into an invariant no signature enforces.A docstring that enumerates accepted forms is a test table. The enumeration is a specification nothing verifies; drift between it and the pattern fails open.
A policy scanner must parse tokens, not prose. Substring-matching a directive line also matches author-written reason text. Report keys must be cross-checked against the configured
mutateset, because silence about a module is not evidence about it.Two claims I had to walk back
The scanner doc originally asserted that Stryker substitutes a default
statusReason, making the report-level empty-reason check "structurally incapable of firing." Reading the code, the check is wired correctly — a missing reason is preserved asundefinedand treated as a violation — and a comment incheck-mutation-guards.tsrecording a live Stryker run says framework-ignored mutants arrive with a non-empty reason. So the honest statement is narrower: the check would fire on an absent reason, and whether a reasonless directive can produce that state is upstream behaviour not traced here. That correction matters in a doc whose own thesis is verify where enforcement actually lives.The trust-gate doc presented
typeof value === 'string'as the fix. The implemented defence incheck-wiki-private-presence.tsis three layers — structuredsourcesauthoritative, comparison throughnew URL(...), substring fallback only when structured sources are absent and deliberately so. A reader taking only the type check closes the coercion vector and leaves the weaker comparison in place.Also cross-linked from
size-subprocess-buffers-selectively-at-call-sites, which states the static-constmechanism in a code comment and is easy to mistake for covering it.Closes #3849, #3850, #3851, #3852, #3853