allod/archetypes

Watch

Fork 0

Close five of the six static-site check defects #64

Closed

allod-agent wants to merge 11 commits from agent/static-site-check-defects into master

allod-agent commented

2026-09-01 15:05:09 +01:00

Member

Five of the six defects in allod/archetypes#62 are fixed; the sixth is not achievable as filed and is explained below rather than papered over. Four of the five were checks that passed without establishing what they claimed, so the substance of this change is the fixtures: every rule touched here now has a case that fails when the rule is removed, and each was watched failing. Nothing a correct site does changes — the shipped site and every existing fixture behave exactly as before. If it is wrong, revert the commit; no machine configuration changes.

Fixed

1 — the site root reached a shell unquoted. modules/static-site.nix wrote SITE_ROOT=${siteRoot} …, and types.pathInStore accepts a store path containing a space, so "${site}/public files" produced a command that word-split and tried to execute files. Both the module and the expectation in checks/static-site.nix now use lib.escapeShellArg, so neither can quietly lose the quoting.

2 — the reference scan failed open. The pipeline ended | grep '^/' || true. The || true is needed because grep exits 1 on a site with no root-relative references, but it also swallowed a failure anywhere earlier: a missing sed or a read error produced an empty reference set and the check passed having scanned nothing. Each stage now runs separately with its status inspected, and only the final filter may exit 1.

3 — the closed-port assertion could not fail for the right reason. pages.fail("ss -ltnH 'sport = :22' | grep -q .") is satisfied when ss errors on a malformed filter: grep matches nothing, the pipeline fails, and the subtest records port 22 as closed without having asked. It now asserts that ss succeeds and returns nothing, and — because an expression ss silently ignores would also return nothing — additionally asserts the same filter matches something on the administrative port, which is open by construction in that subtest.

5 — two gaps in the reference scanner. The scan was line-oriented, so <img src= followed by "/missing.png"> on the next line was never seen: valid HTML, a 404 in a browser, a green check. It is NUL-separated end to end now, so a match may contain the newline. Separately, a leading slash was read as proof of a local path, so a protocol-relative URL such as //cdn.example/app.js was looked for inside the site root and a valid page failed the build; only a single leading slash is treated as root-relative now.

6 — the title assertion accepted a comment. grep -q '<title>' was satisfied by , the shape a half-finished template leaves. It now requires a closing tag and at least one non-space character between them.

Not fixed, and why

4 — the context assertion cannot check whose context it is. The issue asks it to verify that the context names the path being served. That is not achievable at evaluation time: builtins.getContext "${drv}" names the derivation path, /nix/store/<hash>-name.drv, which is textually unrelated to the output path /nix/store/<other-hash>-name that the string itself spells. Mapping one to the other requires building. A context belonging to the right derivation and one belonging to a wrong derivation are therefore the same shape, and any test that appeared to distinguish them would be matching on something else.

This was established by writing the stronger assertion and watching it reject the legitimate site root, then confirming the context/output relationship directly, rather than by reasoning about it.

What would actually close it is making siteRoot a types.package, so the module holds the derivation and the closure is correct by construction. That is an interface change — siteRoot = "${drv}/public" stops working, and the module supports that spelling deliberately — so it is a decision for the owner rather than something to slip into a defect-clearing change. The assertion keeps its original sound form and the file now records this reasoning where the next reader will find it.

A defect introduced and caught while fixing 2

The first rewrite used find … | xargs grep. xargs reports 123 when the command it ran exited 1, so a site legitimately containing no references came back indistinguishable from a broken scan — exactly the confusion the rewrite existed to remove, reintroduced one layer down. Found by running the rewritten scanner against a reference-free tree. It uses grep -r now, and control-no-references is a permanent fixture for that case.

Risk

R1. Two of the changes can reject input the old code accepted (the stricter title rule, and the quoting change if a caller depended on the broken spelling); one accepts input the old code rejected (protocol-relative URLs, which were a false positive). Everything else is error handling. No machine configuration changes, and the shipped site is unaffected.

Validation

./check.sh passes: all 31 steps.

Every new rule has a fixture, and every fixture was proven non-vacuous by reverting its rule and watching the gate go red:

reverting the title rule to the substring grep fails both title fixtures;
reverting the root-relative filter to a bare leading slash fails the protocol-relative control with Referenced asset is absent from the site root: //cdn.example/app.js;
reverting the scanner to line-oriented fails the straddling-attribute fixture.

The scanner was also exercised directly against all six shapes — clean tree, comment-only title, straddling attribute, protocol-relative URL, genuinely missing asset, and a tree with no references — before any fixture was written.

Closes allod/archetypes#62

Five of the six defects in `allod/archetypes#62` are fixed; the sixth is not achievable as filed and is explained below rather than papered over. Four of the five were checks that passed without establishing what they claimed, so the substance of this change is the fixtures: every rule touched here now has a case that fails when the rule is removed, and each was watched failing. Nothing a correct site does changes — the shipped site and every existing fixture behave exactly as before. If it is wrong, revert the commit; no machine configuration changes. ## Fixed **1 — the site root reached a shell unquoted.** `modules/static-site.nix` wrote `SITE_ROOT=${siteRoot} …`, and `types.pathInStore` accepts a store path containing a space, so `"${site}/public files"` produced a command that word-split and tried to execute `files`. Both the module and the expectation in `checks/static-site.nix` now use `lib.escapeShellArg`, so neither can quietly lose the quoting. **2 — the reference scan failed open.** The pipeline ended `| grep '^/' || true`. The `|| true` is needed because grep exits 1 on a site with no root-relative references, but it also swallowed a failure anywhere earlier: a missing `sed` or a read error produced an empty reference set and the check passed having scanned nothing. Each stage now runs separately with its status inspected, and only the final filter may exit 1. **3 — the closed-port assertion could not fail for the right reason.** `pages.fail("ss -ltnH 'sport = :22' | grep -q .")` is satisfied when `ss` errors on a malformed filter: grep matches nothing, the pipeline fails, and the subtest records port 22 as closed without having asked. It now asserts that `ss` *succeeds* and returns nothing, and — because an expression `ss` silently ignores would also return nothing — additionally asserts the same filter matches something on the administrative port, which is open by construction in that subtest. **5 — two gaps in the reference scanner.** The scan was line-oriented, so `<img src=` followed by `"/missing.png">` on the next line was never seen: valid HTML, a 404 in a browser, a green check. It is NUL-separated end to end now, so a match may contain the newline. Separately, a leading slash was read as proof of a local path, so a protocol-relative URL such as `//cdn.example/app.js` was looked for inside the site root and a valid page failed the build; only a single leading slash is treated as root-relative now. **6 — the title assertion accepted a comment.** `grep -q '<title>'` was satisfied by ``, the shape a half-finished template leaves. It now requires a closing tag and at least one non-space character between them. ## Not fixed, and why **4 — the context assertion cannot check whose context it is.** The issue asks it to verify that the context names the path being served. That is not achievable at evaluation time: `builtins.getContext "${drv}"` names the *derivation* path, `/nix/store/<hash>-name.drv`, which is textually unrelated to the output path `/nix/store/<other-hash>-name` that the string itself spells. Mapping one to the other requires building. A context belonging to the right derivation and one belonging to a wrong derivation are therefore the same shape, and any test that appeared to distinguish them would be matching on something else. This was established by writing the stronger assertion and watching it reject the *legitimate* site root, then confirming the context/output relationship directly, rather than by reasoning about it. What would actually close it is making `siteRoot` a `types.package`, so the module holds the derivation and the closure is correct by construction. That is an interface change — `siteRoot = "${drv}/public"` stops working, and the module supports that spelling deliberately — so it is a decision for the owner rather than something to slip into a defect-clearing change. The assertion keeps its original sound form and the file now records this reasoning where the next reader will find it. ## A defect introduced and caught while fixing 2 The first rewrite used `find … | xargs grep`. `xargs` reports 123 when the command it ran exited 1, so a site legitimately containing no references came back indistinguishable from a broken scan — exactly the confusion the rewrite existed to remove, reintroduced one layer down. Found by running the rewritten scanner against a reference-free tree. It uses `grep -r` now, and `control-no-references` is a permanent fixture for that case. ## Risk R1. Two of the changes can reject input the old code accepted (the stricter title rule, and the quoting change if a caller depended on the broken spelling); one accepts input the old code rejected (protocol-relative URLs, which were a false positive). Everything else is error handling. No machine configuration changes, and the shipped site is unaffected. ## Validation `./check.sh` passes: all 31 steps. Every new rule has a fixture, and every fixture was proven non-vacuous by reverting its rule and watching the gate go red: - reverting the title rule to the substring grep fails both title fixtures; - reverting the root-relative filter to a bare leading slash fails the protocol-relative control with `Referenced asset is absent from the site root: //cdn.example/app.js`; - reverting the scanner to line-oriented fails the straddling-attribute fixture. The scanner was also exercised directly against all six shapes — clean tree, comment-only title, straddling attribute, protocol-relative URL, genuinely missing asset, and a tree with no references — before any fixture was written. Closes allod/archetypes#62

allod-agent added 1 commit

2026-09-01 15:05:10 +01:00

Close five static-site check defects and witness each rule 06201eb74e

allod-agent added 1 commit

2026-09-01 15:34:39 +01:00

Strip comments before the title match, scan markup only, and make two checks load-bearing 29f454fcc6

allod-agent commented

2026-09-01 15:35:00 +01:00

Author

Member

Review round 1 — six findings, all reproduced, all fixed

An independent review of 06201eb found six defects, including two of the "fixes" being incomplete and two of the new checks not being load-bearing. Every claim was reproduced here before acting on it. Fixed in 29f454f.

High, fixed — the title defect was still open. Requiring a closing tag was not enough:  has one. A commented-out complete element passed, so item 6 of the issue was not actually closed by the first attempt. Comments are now stripped before the match, which is the only way a rule of this shape can be about elements rather than about text.

High, fixed — the scanner read binary assets as markup. A site shipping an image whose bytes happen to contain href="/missing.png" failed the build. That is a valid site rejected, which is worse than a defect missed, and static sites ship binary files as a matter of course. grep -I does not help under -z, because the whole file is one "line" and the binary suppression does not apply. The scan is now restricted to markup extensions, chosen by extension rather than by content sniffing so the set of files it reads is legible.

Medium, fixed — <TITLE> was rejected. HTML tag names are ASCII case-insensitive, so that is the same element and a valid page. The match is case-insensitive now, matching how the attribute scanner in the same file already treats <IMG SRC=.

Medium, fixed — the quoting check was not load-bearing. lib.escapeShellArg is the identity for a clean store path, and the fixture site root is one, so reverting the module to the unquoted spelling left the comparison green. The check now also evaluates the module with "${siteRoot}/public files" — a spelling pathInStore accepts — and asserts the generated command contains the escaped form and not the bare one. Reverting the module now produces: a siteRoot containing a space is not shell-quoted in the generated symlink check; a siteRoot containing a space reaches the generated command unquoted, so it word-splits.

Low, fixed — the "title-with-attributes" control had no attributes. Deleting [^>]* from the rule left it green, so it could not fail for the thing it was named after. It now uses <title data-kind="main">.

Corrected — my claim about how to close item 4 was wrong. The PR said types.package would make the module hold a derivation. It would not: lib.types.package.check accepts a plain store-path string. Verified. The review also showed builtins.appendContext can attach one derivation's context to another's path text and pass the assertion outright, so the rule catches a discarded context and nothing finer.

The conclusion that item 4 cannot be closed at evaluation time stands — the context names the .drv path, textually unrelated to the output path, and mapping one to the other requires building. But the suggested remedy was wrong and is corrected in the module: closing it needs an option requiring an actual derivation (lib.isDerivation) with the subdirectory moved to a separate relative-path option, since siteRoot = "${drv}/public" is supported on purpose.

Fixtures

Four new cases, each proven non-vacuous by reverting the rule it targets:

dropping comment-stripping → accepted 'an index whose only complete title is commented out'
dropping -i → rejected 'an upper-case TITLE element', which it must accept
dropping the --include filters → rejected 'a binary asset containing bytes that resemble an attribute'
dropping [^>]* → rejected 'a title with attributes, beside a commented mention'

perl is added to the check's nativeBuildInputs for the comment strip. A sed loop could avoid the dependency, but clever regex in a validation script is what produced several of the defects being closed here.

Validation

./check.sh passes: all 31 steps.

## Review round 1 — six findings, all reproduced, all fixed An independent review of `06201eb` found six defects, including two of the "fixes" being incomplete and two of the new checks not being load-bearing. Every claim was reproduced here before acting on it. Fixed in `29f454f`. **High, fixed — the title defect was still open.** Requiring a closing tag was not enough: `` has one. A commented-out *complete* element passed, so item 6 of the issue was not actually closed by the first attempt. Comments are now stripped before the match, which is the only way a rule of this shape can be about elements rather than about text. **High, fixed — the scanner read binary assets as markup.** A site shipping an image whose bytes happen to contain `href="/missing.png"` failed the build. That is a valid site rejected, which is worse than a defect missed, and static sites ship binary files as a matter of course. `grep -I` does not help under `-z`, because the whole file is one "line" and the binary suppression does not apply. The scan is now restricted to markup extensions, chosen by extension rather than by content sniffing so the set of files it reads is legible. **Medium, fixed — `<TITLE>` was rejected.** HTML tag names are ASCII case-insensitive, so that is the same element and a valid page. The match is case-insensitive now, matching how the attribute scanner in the same file already treats `<IMG SRC=`. **Medium, fixed — the quoting check was not load-bearing.** `lib.escapeShellArg` is the identity for a clean store path, and the fixture site root is one, so reverting the module to the unquoted spelling left the comparison green. The check now also evaluates the module with `"${siteRoot}/public files"` — a spelling `pathInStore` accepts — and asserts the generated command contains the escaped form and *not* the bare one. Reverting the module now produces: `a siteRoot containing a space is not shell-quoted in the generated symlink check; a siteRoot containing a space reaches the generated command unquoted, so it word-splits`. **Low, fixed — the "title-with-attributes" control had no attributes.** Deleting `[^>]*` from the rule left it green, so it could not fail for the thing it was named after. It now uses `<title data-kind="main">`. **Corrected — my claim about how to close item 4 was wrong.** The PR said `types.package` would make the module hold a derivation. It would not: `lib.types.package.check` accepts a plain store-path string. Verified. The review also showed `builtins.appendContext` can attach one derivation's context to another's path text and pass the assertion outright, so the rule catches a *discarded* context and nothing finer. The conclusion that item 4 cannot be closed at evaluation time stands — the context names the `.drv` path, textually unrelated to the output path, and mapping one to the other requires building. But the suggested remedy was wrong and is corrected in the module: closing it needs an option requiring an actual derivation (`lib.isDerivation`) with the subdirectory moved to a separate relative-path option, since `siteRoot = "${drv}/public"` is supported on purpose. ## Fixtures Four new cases, each proven non-vacuous by reverting the rule it targets: - dropping comment-stripping → `accepted 'an index whose only complete title is commented out'` - dropping `-i` → `rejected 'an upper-case TITLE element', which it must accept` - dropping the `--include` filters → `rejected 'a binary asset containing bytes that resemble an attribute'` - dropping `[^>]*` → `rejected 'a title with attributes, beside a commented mention'` `perl` is added to the check's `nativeBuildInputs` for the comment strip. A `sed` loop could avoid the dependency, but clever regex in a validation script is what produced several of the defects being closed here. ## Validation `./check.sh` passes: all 31 steps.

allod-agent added 1 commit

2026-09-01 16:05:41 +01:00

Witness the reference scan's fail-closed guards 90f9b04b6f

allod-agent commented

2026-09-01 16:06:10 +01:00

Author

Member

Review round 2 — one fix landed, and a design decision this PR should not make alone

A second review, by a different model, returned seven findings. Two were already closed by 29f454f (the uppercase-filename gap and CSS being scanned with an HTML grammar — both found and fixed here between the review starting and finishing). One is fixed in 90f9b04. The remaining four are one problem, and it is the reason this PR is pausing rather than continuing.

Fixed — the fail-closed guards had no witness

Top severity, and the sharpest finding of the round. The reference scan distinguishes "matched nothing" (legitimate) from "the scan broke" (must fail closed), and nothing exercised the second half: replacing both guards with if false left every fixture in the file green. Those guards exist precisely because the previous version ended | grep '^/' || true and swallowed real failures — so the fix for a fail-open defect was itself unwitnessed, which is architecture principle 11 exactly.

Now witnessed by PATH wrappers that exit 2 for the scan's own grep -hoizP and sed -z invocations and delegate everything else to the real tool. The targeting is load-bearing: a blanket grep wrapper also breaks the title check, which runs earlier, and the fixture would then have passed for the wrong reason — the same defect class one level up.

The reviewer's exact sabotage now fails: site-content passed while 'grep' failed, so the scan fails open.

Not fixed — regex cannot parse HTML, and this is the third round of trying

Four findings are the same problem in different clothes. All four reproduced here against the current tree:

Input	Current behaviour	Correct behaviour
`<!-- <title>Fake</title> --!>`	accepted	rejected — `--!>` terminates a comment, so there is no title
`<textarea><title>Fake</title></textarea>`	accepted	rejected — raw-text element, not a title
`<meta content="<!--"><title>Real --> title</title>`	rejected	accepted — `<!--` is attribute text
`<title>1 < 2</title>`	rejected	accepted — `<` is ordinary title text

False acceptances and false rejections, in both directions. Each round of this PR has fixed the title rule and each fix has produced a new pair: substring grep, then closing-tag required, then comment-stripping. A fourth regex would be the same move a fourth time.

The reference scanner has the same shape of problem for .js and .json, which the review raised as a scope question rather than a defect: a module import "/missing.js" and a web-manifest icon are real browser requests that this scanner cannot soundly read.

The decision, which belongs to the owner

Three options, and this PR takes none of them without a ruling:

Parse. xmllint --html (libxml2, already in nixpkgs) settles every row of that table correctly. Costs a dependency; makes the check actually sound.
Narrow the claim. Reduce the rule to "index.html is non-empty" and stop asserting anything about a title element. Smaller and honest; loses a real if minor check.
Delete the title check. testing.md's deletion rule says drop a check when a cheaper witness pins the same property, and a missing <title> is visible the moment a live site is looked at — unlike a broken link, which ships silently.

The recommendation from here is 1 for the reference scanner, whose claims are load-bearing, and 3 for the title check specifically, which has now cost three rounds for a cosmetic property. But that is a scope call on this repository rather than one to settle inside a defect-clearing change.

Item 4 from the original issue remains as previously recorded: not closable at evaluation time, with the earlier types.package suggestion corrected.

Validation

./check.sh passes: all 31 steps.

The review also independently confirmed all nine fixtures added in 06201eb and 29f454f are load-bearing for their own rules, each failing under an isolated mutation and naming its intended case.

## Review round 2 — one fix landed, and a design decision this PR should not make alone A second review, by a different model, returned seven findings. Two were already closed by `29f454f` (the uppercase-filename gap and CSS being scanned with an HTML grammar — both found and fixed here between the review starting and finishing). One is fixed in `90f9b04`. The remaining four are one problem, and it is the reason this PR is pausing rather than continuing. ## Fixed — the fail-closed guards had no witness Top severity, and the sharpest finding of the round. The reference scan distinguishes "matched nothing" (legitimate) from "the scan broke" (must fail closed), and **nothing exercised the second half**: replacing both guards with `if false` left every fixture in the file green. Those guards exist precisely because the previous version ended `| grep '^/' || true` and swallowed real failures — so the fix for a fail-open defect was itself unwitnessed, which is architecture principle 11 exactly. Now witnessed by PATH wrappers that exit 2 for the scan's own `grep -hoizP` and `sed -z` invocations and delegate everything else to the real tool. The targeting is load-bearing: a blanket `grep` wrapper also breaks the title check, which runs earlier, and the fixture would then have passed for the wrong reason — the same defect class one level up. The reviewer's exact sabotage now fails: `site-content passed while 'grep' failed, so the scan fails open`. ## Not fixed — regex cannot parse HTML, and this is the third round of trying Four findings are the same problem in different clothes. All four reproduced here against the current tree: | Input | Current behaviour | Correct behaviour | |---|---|---| | ` title</title>` | rejected | accepted — `<!--` is attribute text | | `<title>1 < 2</title>` | rejected | accepted — `<` is ordinary title text | False acceptances and false rejections, in both directions. Each round of this PR has fixed the title rule and each fix has produced a new pair: substring grep, then closing-tag required, then comment-stripping. A fourth regex would be the same move a fourth time. The reference scanner has the same shape of problem for `.js` and `.json`, which the review raised as a scope question rather than a defect: a module `import "/missing.js"` and a web-manifest icon are real browser requests that this scanner cannot soundly read. ## The decision, which belongs to the owner Three options, and this PR takes none of them without a ruling: 1. **Parse.** `xmllint --html` (libxml2, already in nixpkgs) settles every row of that table correctly. Costs a dependency; makes the check actually sound. 2. **Narrow the claim.** Reduce the rule to "index.html is non-empty" and stop asserting anything about a title element. Smaller and honest; loses a real if minor check. 3. **Delete the title check.** `testing.md`'s deletion rule says drop a check when a cheaper witness pins the same property, and a missing `<title>` is visible the moment a live site is looked at — unlike a broken link, which ships silently. The recommendation from here is 1 for the reference scanner, whose claims are load-bearing, and 3 for the title check specifically, which has now cost three rounds for a cosmetic property. But that is a scope call on this repository rather than one to settle inside a defect-clearing change. Item 4 from the original issue remains as previously recorded: not closable at evaluation time, with the earlier `types.package` suggestion corrected. ## Validation `./check.sh` passes: all 31 steps. The review also independently confirmed all nine fixtures added in `06201eb` and `29f454f` are load-bearing for their own rules, each failing under an isolated mutation and naming its intended case.

allod-agent added 1 commit

2026-09-02 01:54:16 +01:00

Delete the title rule rather than parse it b21a372102

allod-agent commented

2026-09-02 01:54:35 +01:00

Author

Member

The title rule is deleted rather than parsed — owner's ruling

b21a372 removes the <title> check, its five fixtures, and the perl dependency that only existed to serve it. ./check.sh passes: all 31 steps. That closes the last open item on this PR; item 6 of the issue is answered by removing the rule rather than by fixing it a fourth time.

Why deletion rather than an HTML parser. Three versions of this rule were written and each was wrong in both directions. grep -q '<title>' accepted . Requiring a closing tag accepted . Stripping comments first still accepted a title inside a <textarea> and one hidden behind the --!> comment terminator, while rejecting two valid pages: a real title preceded by <!-- inside an attribute value, and the title 1 < 2.

That is not three unlucky regexes. Deciding whether text is a title element means knowing where markup is live, which is a parse; a regex cannot do it, and every attempt shipped false rejections — a valid site failing the build — alongside the false acceptances.

xmllint --html --xpath 'count(//title)' would settle all of it, and the deleted block records that for anyone who needs the rule later. It is not worth the dependency here, because of what the rule was worth: a missing page title is visible the moment anyone opens the site, so a live witness already pins it — allod/memory testing.md's deletion rule, drop a check when a cheaper witness pins the same property.

That is the opposite of the reference scan, whose findings are invisible until a visitor hits a 404 and which therefore earns its cost. The two rules were never equally valuable, and treating them as one bundle is what kept this open for three rounds.

[[ -s $site_root/index.html ]] stays: a homepage that exists and is non-empty is still checked.

PR status

Every item from allod/archetypes#62 is now closed:

1 site root reaching a shell unquoted — fixed, and the witness is a root spelling that actually needs quoting, since escapeShellArg is the identity on a clean store path
2 reference scan failing open — fixed, and the three fail-closed guards now have sabotage witnesses of their own, after review showed deleting both left every fixture green
3 closed-port assertion — fixed, asserting ss succeeds and returns nothing, with a positive control on the administrative port
4 context assertion — not closable at evaluation time, established by writing the stronger assertion and watching it reject the legitimate site root; the earlier types.package suggestion was itself wrong and is corrected in the module
5 scanner gaps — fixed: NUL-separated so a straddling attribute is seen, protocol-relative URLs no longer treated as local paths, markup selected case-insensitively by find -iname after --include globs proved case-sensitive, and binary assets no longer scanned as markup
6 title assertion — deleted, per the reasoning above

Two defects were introduced by fixes during this PR and caught before merge: xargs reporting 123 for a command that exited 1, which made a reference-free site indistinguishable from a broken scan; and case-sensitive --include globs, which silently stopped scanning PAGE.HTML. Both now have permanent fixtures.

## The title rule is deleted rather than parsed — owner's ruling `b21a372` removes the `<title>` check, its five fixtures, and the `perl` dependency that only existed to serve it. `./check.sh` passes: all 31 steps. That closes the last open item on this PR; item 6 of the issue is answered by removing the rule rather than by fixing it a fourth time. **Why deletion rather than an HTML parser.** Three versions of this rule were written and each was wrong in both directions. `grep -q '<title>'` accepted ``. Requiring a closing tag accepted ``. Stripping comments first still accepted a title inside a `<textarea>` and one hidden behind the `--!>` comment terminator, while *rejecting* two valid pages: a real title preceded by `<!--` inside an attribute value, and the title `1 < 2`. That is not three unlucky regexes. Deciding whether text is a title element means knowing where markup is live, which is a parse; a regex cannot do it, and every attempt shipped false rejections — a valid site failing the build — alongside the false acceptances. `xmllint --html --xpath 'count(//title)'` would settle all of it, and the deleted block records that for anyone who needs the rule later. It is not worth the dependency here, because of what the rule was worth: a missing page title is visible the moment anyone opens the site, so a live witness already pins it — `allod/memory` testing.md's deletion rule, drop a check when a cheaper witness pins the same property. That is the opposite of the reference scan, whose findings are invisible until a visitor hits a 404 and which therefore earns its cost. The two rules were never equally valuable, and treating them as one bundle is what kept this open for three rounds. `[[ -s $site_root/index.html ]]` stays: a homepage that exists and is non-empty is still checked. ## PR status Every item from `allod/archetypes#62` is now closed: - **1** site root reaching a shell unquoted — fixed, and the witness is a root spelling that actually needs quoting, since `escapeShellArg` is the identity on a clean store path - **2** reference scan failing open — fixed, and the three fail-closed guards now have sabotage witnesses of their own, after review showed deleting both left every fixture green - **3** closed-port assertion — fixed, asserting `ss` succeeds and returns nothing, with a positive control on the administrative port - **4** context assertion — **not closable at evaluation time**, established by writing the stronger assertion and watching it reject the legitimate site root; the earlier `types.package` suggestion was itself wrong and is corrected in the module - **5** scanner gaps — fixed: NUL-separated so a straddling attribute is seen, protocol-relative URLs no longer treated as local paths, markup selected case-insensitively by `find -iname` after `--include` globs proved case-sensitive, and binary assets no longer scanned as markup - **6** title assertion — **deleted**, per the reasoning above Two defects were introduced by fixes during this PR and caught before merge: `xargs` reporting 123 for a command that exited 1, which made a reference-free site indistinguishable from a broken scan; and case-sensitive `--include` globs, which silently stopped scanning `PAGE.HTML`. Both now have permanent fixtures.

vnprc force-pushed agent/static-site-check-defects from b21a372102 to a2bd206177

2026-09-02 03:01:57 +01:00

Compare

allod-agent added 7 commits

2026-09-02 04:34:57 +01:00

Close five static-site check defects and witness each rule 06201eb74e

Strip comments before the title match, scan markup only, and make two checks load-bearing 29f454fcc6

Witness the reference scan's fail-closed guards 90f9b04b6f

Delete the title rule rather than parse it b21a372102

Merge remote-tracking branch 'origin/master' into agent/static-site-check-defects f2d9e4dcf4

Cut the content check to the rule whose failure is silent 8f2aa44387

Merge branch 'agent/static-site-check-defects' of ssh://forge.anarch.diy:2222/allod/archetypes into agent/static-site-check-defects 621998b48d

# Conflicts:
#	checks/site-content-fixtures.sh
#	checks/site-content.sh

allod-agent commented

2026-09-02 06:04:34 +01:00

Author

Member

Superseded by allod/archetypes#65, which is the same tree in two commits instead of eleven.

This branch reached the right answer by a long route: five defects fixed, then two defects introduced by those fixes and caught in review, then three rules deleted once it became clear they could not be made correct. The reasoning is worth keeping and is carried over into #65's commit messages and body; the commit-by-commit thrash is not worth carrying into master.

git diff between the two branches is empty — #65 is byte-identical, branched fresh from master, and ./check.sh passes on it: all 31 steps.

Closing with the branch deleted. The review record stays here for anyone tracing how the content check went from 215 lines to 55.

Superseded by `allod/archetypes#65`, which is the same tree in two commits instead of eleven. This branch reached the right answer by a long route: five defects fixed, then two defects introduced by those fixes and caught in review, then three rules deleted once it became clear they could not be made correct. The reasoning is worth keeping and is carried over into `#65`'s commit messages and body; the commit-by-commit thrash is not worth carrying into master. `git diff` between the two branches is empty — `#65` is byte-identical, branched fresh from master, and `./check.sh` passes on it: all 31 steps. Closing with the branch deleted. The review record stays here for anyone tracing how the content check went from 215 lines to 55.

allod-agent closed this pull request

2026-09-02 06:04:37 +01:00