fix(gate): capture the log a failure points at, and stop leaking the server - #103
Merged
Conversation
…server run_stage sent stdout to /dev/null and captured only stderr, then named that stderr file in the failure detail. Which stream carries a diagnosis is per-tool, so the practical impact had to be measured rather than assumed. Measured: zola, lychee and csp-enforce all report failures on stderr, so those were captured. pa11y-ci does not — a failing run writes 1425 bytes to stdout and nothing at all to stderr. The accessibility stage therefore emitted "see $LOGDIR/pa11y.err" pointing at an empty file while every finding went to /dev/null. Both streams now land in one .log, which also preserves interleaving that a split pair cannot. The background server was worse than the issue described. cleanup() only knew about the playwright server, so an interrupt during pa11y leaked that one — but `kill "$SRV_PID"` never worked on ANY path, including success. `cd public-local && python3 -m http.server &` runs the list in a background subshell, so $! is the subshell and the server is its child; killing the wrapper leaves the server holding the port. Verified directly: $! resolves to a process whose comm is bash, and the python server survives the kill. That was not theoretical. A leaked http.server was found bound to 8080, reparented to init, still serving a public-local directory that had already been deleted — which is also why a second gate run would fail to bind. Both servers now start with --directory instead of a cd list, so there is no subshell and $! is the server itself, and both are registered in the trap. The header claimed the gate runs fail-fast. It does not: every stage runs and failures accumulate into one report. That is the better behaviour for a gate, so the claim is corrected rather than the code. Verified: full gate green against examples/sample-blog, every stage writes a non-empty .log, and after an interrupt no http.server survives and port 8080 is free. Closes #84 Closes #85
forkwright
added a commit
that referenced
this pull request
Aug 17, 2026
…d claim-accuracy defects (#171) ## Summary Fixes 16 of the 18 findings in #92; the other 2 were already fixed on `main` by prior PRs (evidence below, per `#5a` in the fleet agent brief). One additional fix (JSON-LD script-injection escaping) generalizes beyond the single file the issue cited to all six `partials/ld-*.html` partials, since every sibling shared the identical unescaped `json_encode` pattern — fixing only the named file would have left the same defect live in five others. ## Acceptance criteria Issue #92: `Done when: every bullet is either fixed, or closed with a stated reason it is not a defect.` ### `bin/` (7) 1. **`bin/typikon-check:65` — `base_host()` single-quoted TOML.** Fixed — `bin/typikon-check:103-112`. `base_host()`'s `sed` now matches both `"..."` and `'...'` TOML string forms. Verified: piping a single-quoted `base_url = 'https://example.com'` through the pre-fix pipeline produced `base_url = 'https:` (garbage); through the fixed pipeline, `example.com` for both quote styles. 2. **`bin/typikon-check:7` — "fail-fast" header claim.** Already fixed — `bin/typikon-check:7-9` currently reads "Runs every gate stage in order. Stages do NOT stop at the first failure." Fixed in PR #103 (`3. The header claimed fail-fast; it isn't`). No change in this PR. 3. **`bin/typikon-init:107` — "always refresh" comment vs skip-if-exists code.** Fixed — `bin/typikon-init:131-137`. Corrected the comment: these files are create-once (matches `typikon-refresh`'s own documented contract, which explicitly does NOT re-render `_headers`/`_redirects`). 4. **`bin/typikon-init:91` — swallowed theme-submodule refresh failure.** Fixed — `bin/typikon-init:90-97`. A failed `git pull --ff-only` now prints `warn themes/typikon: refresh failed, continuing with existing checkout: <git's own error>` to stderr instead of `|| true`-swallowing it silently. Still non-fatal (an offline forge on a re-run must not abort the whole scaffold). 5. **`bin/typikon-validate:138` — JSON Pointer root emits `"/"` not `""`.** Fixed — `bin/typikon-validate:420-428`. A root-level failure (`err.absolute_path` empty) now emits `pointer: ""` per RFC 6901. 6. **`bin/typikon-validate:155` — `load_schemas()` raw traceback on malformed schema.** Fixed — `bin/typikon-validate:493-499` and `:592-599` (both the per-slug loop and the `.core.schema.json` loop carried the identical unwrapped `json.loads()`). Both now catch and report `{"error": "malformed schema <path>: <exc>"}` + exit 2. 7. **`bin/typikon-validate:29` — exit-code docstring contradicts implementation for malformed frontmatter.** Fixed — `bin/typikon-validate:57-64`. Malformed frontmatter is folded into the per-file `failures` list (exit 1), not the runtime-error class (exit 2); the docstring said the opposite. Corrected the doc; the code was already the intended behavior (same JSONL per-file reporting shape as a schema failure). ### `ci/` (4, + 1 bonus from the issue thread) 8. **`ci/csp-enforce.sh:46` — unbounded `grep`/argv, E2BIG risk.** Fixed — `ci/csp-enforce.sh:65-70`, `ci/csp-scan.py:106-124`. File paths now go over stdin (bash builtin `printf`, no `execve()` of its own) instead of argv; `csp-scan.py` reads newline-delimited paths from stdin when argv is empty. 9. **`ci/kanon-ci.toml.tmpl:40` — missing TLS pinning vs the GitHub template it claims to mirror.** Fixed — `ci/kanon-ci.toml.tmpl:41,75`. Both `curl` calls (zola, lychee) now carry `--proto '=https' --tlsv1.2`, matching `ci/github-workflow.yml.tmpl:64,110`. 10. **`ci/lychee.toml:12` — false cross-run cache-persistence claim.** Fixed — `ci/lychee.toml:11-16`. Corrected: neither CI template persists `.lychee_cache/` across runs today (verified: no `actions/cache` / forge equivalent in either template). `cache = true` stays on — it still dedupes repeat hits to the same URL within one run. 11. **`ci/playwright.config.ts:10` — header claims `public/`, gate actually serves `public-local/`.** Already fixed — `ci/playwright.config.ts:16-21` currently describes `public-local/` correctly, with an OS-allocated port (no hardcoded 8080 claim). Fixed in PR #116. No change in this PR. 12. *(Bonus, from the issue thread, not one of the 18 — fixed while here)* **`ci/check-consumer-check-extension.sh:32-33` — freeform `# Usage:` comment tag.** Fixed — `ci/check-consumer-check-extension.sh:32`, now `# NOTE: usage — ...`, per the fix already applied to the identical shape in PR #151. ### `schemas/` (1) 13. **`schemas/page.schema.json:66` — `og_image` extension allowlist inconsistent across schemas.** Fixed — `schemas/page.core.schema.json:64` and `schemas/journal-entry.schema.json:52` (the file split into `page.schema.json` + `page.core.schema.json` since the issue was filed; the actual `og_image` pattern lives in `page.core.schema.json` now). Both now accept `.jpeg`, matching `faq`/`sizing-guide`/`product` schemas, which already did. All 5 schemas' `og_image` patterns now agree. ### `templates/` (6) 14. **`templates/atom.xml:25` — `<updated>` from newest-DATED entry, not most-recently-EDITED entry.** Fixed — `templates/atom.xml:24-45`. Now scans every entry's own `updated`-or-`date` and takes the max (`set_global` + epoch-int comparison, since Tera's `>` only compares numbers). Verified against a real `zola build`: an older-dated entry given a newer `updated` now correctly wins. 15. **`templates/faq.html:36` — auto-slugify anchor collision.** Fixed — `templates/faq.html:42-59`. A colliding anchor (explicit or slugify-derived) gets a `-{{ loop.index }}` suffix instead of colliding on `id=`. Verified against a real `zola build`: two questions that previously rendered two `id="do-you-ship-internationally"` elements now render `id="do-you-ship-internationally"` + `id="do-you-ship-internationally-2"`. 16. **`templates/journal-entry.html:16` — hardcoded `og_type`.** Fixed — removed the override; `journal-entry.html` now inherits `page.html`'s `og_type` block (`templates/page.html:13`), which already reads `page.extra.og_type | default(value="article")` correctly. Verified: a real build's `og:type` meta still reads `article` by default with no regression. 17. **`templates/partials/ld-article.html:21` — hand-concatenated image/logo URLs instead of `get_url()`.** Fixed — `templates/partials/ld-article.html:20-28`, `templates/partials/ld-product.html:27-32`, `templates/partials/ld-organization.html:29-34` (the issue: "**three** JSON-LD partials"). All three now call `get_url(path=..., trailing_slash= false)`, matching the same field's resolution in `base.html`/`page.html`. Verified: with `base_url = "https://example.com/"` (trailing slash), hand-concatenation produced `https://example.com//img/og.png` (double-slash, broken); `get_url()` produces `https://example.com/img/og.png`. 18. **`templates/partials/ld-organization.html:14` — no `assert::required` guard, unlike every sibling.** Fixed — `templates/partials/ld-organization.html:13,26`. Now imports `partials/assert.html` and asserts `config.extra.logo_path`, matching every sibling `ld-*.html`. Verified: pre-fix, an absent `logo_path` still failed the build, but with a bare Tera engine error ("Variable `config.extra.logo_path` not found in context") naming neither the field's role nor which template needs it; post-fix, the same missing field now fails with `partials/ld-organization.html: missing required metadata` — the same clear, actionable message every sibling partial already gives. 19. *(the issue's `ld-organization.html:19` finding — same file, listed separately below since it's a distinct defect)* **JSON-LD `</script>` breakout via unescaped `json_encode`.** Fixed — new `templates/partials/ld-json.html` (shared `ldjson::str()` macro, escapes `/` → `\/`, RFC 8259 §7-legal) wired into **all six** `partials/ld-*.html` (article, breadcrumb, faq, howto, organization, product) — not only the file the issue cited, since every sibling shared the identical `json_encode | safe` pattern with no escaping. Verified: a field value containing `"Hello</script><script>alert(1)</script>"` round-trips through `json.loads()` to the exact original string, and the literal substring `</script` no longer occurs anywhere in the `<script type="application/ld+json">` payload. Note: fixing `ld-organization.html`'s `assert::required` call surfaced a real cross-file Tera scoping gap — `base.html` (which `{% include %}`s `ld-organization.html` inside a block every page reaches via `super()`) did not itself import `partials/assert.html`, and Tera resolves a macro namespace by which template an `include` tag is textually written in, not by the included partial's own imports, once reached via `extends` + `super()`. Fixed by importing `assert` in `templates/base.html:20` too; documented + verified (see commit message). **18/18 accounted for**: 16 fixed in this PR, 2 already fixed on `main` (items 2 and 11 above) with cited evidence. ## Negative fixtures > Negative fixture: `bin/typikon-check` (`base_host()`) — watched failing by > piping a single-quoted `base_url = 'https://example.com'` config line > through the pre-fix `sed` pipeline, which produced the literal garbage > `base_url = 'https:` before the fix and `example.com` after (both quote > styles), run against the exact shipped `sed` expression from each > revision. > Negative fixture: `bin/typikon-validate` (root JSON Pointer) — watched > failing by running `origin/main`'s own shipped `bin/typikon-validate` > against a fixture page missing the required `title` field, which produced > `"pointer": "/"` before the fix; the current worktree's shipped > `bin/typikon-validate` against the identical fixture produces > `"pointer": ""` after. > Negative fixture: `bin/typikon-validate` (`load_schemas()` crash) — > watched failing by running `origin/main`'s own shipped > `bin/typikon-validate` against a schema directory with `faq.schema.json` > truncated mid-object (invalid JSON), which produced a raw > `json.decoder.JSONDecodeError` Python traceback (exit 1) before the fix; > the current worktree's shipped `bin/typikon-validate` against the > identical corrupted schema produces a clean > `{"error": "malformed schema ...: Unterminated string ..."}` (exit 2) > after. > Negative fixture: `bin/typikon-init` (swallowed refresh failure) — > watched failing by extracting the exact shipped refresh line from > `origin/main`'s `bin/typikon-init` (`(cd themes/typikon && git pull > --ff-only origin main 2>/dev/null) || true`) and running it verbatim > against a `themes/typikon` checkout whose `origin` remote is > unreachable, which produced silent success (exit 0, empty stdout AND > stderr) before the fix; the current worktree's shipped replacement lines > against the identical unreachable remote produce exit 0 (still > non-fatal, correctly) but with `warn themes/typikon: refresh failed, > continuing with existing checkout: fatal: ... does not appear to be a > git repository ...` on stderr after. > Negative fixture: `templates/atom.xml` (`<updated>`) — watched failing > by building `examples/sample-blog` (real `zola build`, shipped > templates) with `origin/main`'s `templates/atom.xml` against a fixture > where the older-dated journal entry has a newer `updated` than the > newer-dated entry's own value, which produced `<updated>2026-02-10T00:00:00+00:00</updated>` > (the newer entry's own date, ignoring the true most-recent edit) before > the fix; the current worktree's `templates/atom.xml` against the > identical fixture produces `<updated>2026-08-10T00:00:00+00:00</updated>` > (the actual most-recent edit) after. > Negative fixture: `templates/faq.html` (anchor collision) — watched > failing by building a FAQ page (real `zola build`, shipped templates) > with two questions that slugify identically, against `origin/main`'s > `templates/faq.html`, which produced two `id="do-you-ship-internationally"` > elements before the fix; the current worktree's `templates/faq.html` > against the identical fixture produces `id="do-you-ship-internationally"` > + `id="do-you-ship-internationally-2"` after. > Negative fixture: `templates/partials/ld-organization.html` > (`assert::required` + `get_url()`) — watched failing by building > `examples/sample-blog` (real `zola build`) with `config.extra.logo_path` > removed from `config.toml`, against `origin/main`'s > `templates/partials/ld-organization.html`, which produced a bare Tera > engine error (`Variable 'config.extra.logo_path' not found in context`) > before the fix; the current worktree's version against the identical > fixture produces `partials/ld-organization.html: missing required > metadata 'config.extra.logo_path'` after — matching the message shape > every sibling `ld-*.html` partial already uses. > Negative fixture: every `partials/ld-*.html` (JSON-LD escaping) — > watched failing by piping `"Hello</script><script>alert(1)</script>"` > through the pre-fix `value | json_encode | safe` pattern (shared, > unescaped, across all six partials), which left the literal substring > `</script` intact in the `<script type="application/ld+json">` payload; > the current worktree's `partials/ld-json.html::str()` macro against the > identical input produces `Hello<\/script><script>alert(1)<\/script>` — > `json.loads()`-equivalent to the original string, with `</script` > nowhere in the byte stream. All 16 real JSON-LD blocks rendered across > both `examples/*` sites (blog + shop, every template) were re-verified > to still `json.loads()` cleanly after the change. All ten fixtures ran the actual shipped file content at each git revision (`origin/main` vs this branch) — extracted verbatim (`git show origin/main:<path>`) or symlinked directly at the theme root for a real `zola build` — never a re-implementation. ## Verification - `kanon lint <path> --all` — zero violations across all 21 changed/added files (individually checked). - `python3 bin/typikon-validate examples/sample-blog` / `examples/sample-shop` — both `{"failed": 0}` against the updated schemas. - Real `zola build` + `zola check` — both example sites build and check clean. - `ci/check-xml-output.sh`, `ci/csp-enforce.sh` — both example builds pass. - Every `ci/check-*.py` / `ci/check-*.sh` regression script in the repo (triad-schema, consumer-schema-registry, page-template-cascade, font-coverage, release-config, asset-provenance, favicon-path, init-favicon-path, asset-path-existence, init-staging-scope, control-contrast, home-heading, content-heading-collision, interactive-contrast (+selftest), workflow-template, consumer-check-extension, fixture-corpus-exemption, deploy-bundle-gate, cf-deploy-gate) — all pass. - `TYPIKON_CHECK_MODE=dev bin/typikon-check examples/sample-blog` and `.../sample-shop` — every stage passes except `pa11y` (`pa11y-ci` genuinely absent on this box; `lychee` and `playwright-smoke`, both present, pass real link-checking and browser smoke tests against the fixed templates). Not run: GitHub Actions CI (this box does not run cargo/full builds per fleet policy; this repo's own CI is zola/python/node-based and this PR's branch push will trigger it — this repo is public, so Actions minutes are unmetered). Closes #92 Co-authored-by: forkwright <cody@forkwright.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #84
Closes #85
1. The failure log was missing the half that mattered (#84)
run_stagesent stdout to/dev/null, captured stderr, then named that stderr file in the failure detail. Which stream carries a diagnosis is per-tool, so I measured rather than assumed:So for three of four the old capture worked. pa11y-ci is the real case: a failing accessibility stage emitted
see $LOGDIR/pa11y.errpointing at an empty file, while all 1425 bytes of findings went to/dev/null.Both streams now land in one
.log, which also preserves interleaving a split pair cannot.2. The server leak was worse than the issue said (#85)
The issue is that
cleanup()only tracked the playwright server, so an interrupt during pa11y leaked that one. True — but while testing I foundkill "$SRV_PID"never worked on any path, including success:cd X && cmd &runs the list in a background subshell, so$!is the subshell and the server is its child. Killing the wrapper leaves the server holding the port.This was live, not theoretical. A leaked
http.serverwas found bound to 8080, reparented to init, still serving apublic-localdirectory that had already been deleted:That is also why a second gate run would fail to bind the port.
Both servers now start with
python3 -m http.server --directory public-local— no list, so$!is the server — and both are registered in the trap.3. The header claimed fail-fast; it isn't
Every stage runs and failures accumulate into one report. That is the better behaviour for a gate — one pass surfaces every problem — so the claim is corrected rather than the code. (One bullet of #92.)
Verified
examples/sample-blog; every stage writes a non-empty.log0realhttp.serverprocesses, port 8080 free