feat(templates): make the favicon path configurable like every sibling brand asset - #141
Merged
Conversation
templates/base.html hardcoded <link rel="icon" href="/img/favicon.svg"> while every sibling brand asset (logo_path, theme_color, og_image, font_preload) reads from config.toml [extra]. A consumer whose brand mark lives at any other path had to place a second copy at that literal path to get a working favicon. The consequence was filed against the ardent-site consumer separately: three byte-identical SVGs, one existing only to satisfy this hardcoded href. Route the href through config.extra.favicon_path and get_url(), matching the og_image pattern precisely, with a default of img/favicon.svg so an existing consumer that never sets the field is unaffected. Add ci/check-favicon-path.py: builds a minimal fixture site against the real theme via zola build and asserts the rendered icon link resolves the theme default when favicon_path is unset, and a custom asset when it is set. Wired into ci/run-fixtures.sh alongside the other standalone check stages. Refs #110
Adversarial review on the prior commit found the fix only touched templates/base.html; the issue's Desired correction is explicit that favicon_path belongs in config.toml [extra] alongside logo_path, and bin/typikon-init's scaffolded block is the artifact a new consumer starts from. Add the commented default there, matching logo_path's style. ci/check-init-favicon-path.py runs the real typikon-init against a throwaway destination (theme submodule cloned locally via GIT_ALLOW_PROTOCOL=file, no network/forge needed) and asserts the written config.toml carries favicon_path defaulted to img/favicon.svg. Refs #110
# Conflicts: # ci/run-fixtures.sh
…t has none ci/check-init-favicon-path.py is the first CI check to invoke bin/typikon-init end-to-end, and it exposed a latent defect: the scaffold's final `git commit` depended on an ambient user.name/user.email, which a fresh CI or agentic sandbox does not have. Reproduced locally with HOME pointed at an empty directory and no system-level identity configured: bin/typikon-init failed with 'fatal: empty ident name ... not allowed', matching the CI log exactly. Falls back to a fixed identity for that one commit only when none is configured, leaving an operator's real identity untouched when one exists.
added 2 commits
August 15, 2026 21:09
Making favicon_path configurable without also deriving `type=` left a consumer favicon that isn't SVG (e.g. .ico) with a hardcoded image/svg+xml mismatch — adversarial review on this PR named it as a known, dropped limitation. Adds an extension -> MIME-type mapping (svg/ico/png/gif/jpg/jpeg/webp, svg default) ahead of the <link>, and extends ci/check-favicon-path.py with a third fixture case (a .ico favicon_path) that asserts the rendered type= follows the extension. Verified against the real template via zola build: reverting the type-derivation block turns the new case red while leaving the two pre-existing cases green, then restoring turns it green again.
The fixture spread the caller's ambient os.environ into the bin/typikon-init subprocess, so it only incidentally exercised the no-identity fallback branch on a runner that happens to start with no git identity.
forkwright
added a commit
that referenced
this pull request
Aug 16, 2026
#153) Closes #64 Not closing #136 — see "Notes for the reviewer" below for why. ## Session update (rebase + genuine CI failure + comment-tag gap) This branch was cut before four PRs landed on `main` (#141 favicon config, #150 asset-provenance label fix, #152 home-page heading, #154 consumer schema registry). Rebased onto `origin/main` (3d9432a). One real conflict, in `ci/run-fixtures.sh` — main added four new check invocations (`check-favicon-path.py`, `check-init-favicon-path.py`, `check-home-heading.py`, `check-fixture-corpus-exemption.sh`) in the same region this branch adds two (`check-interactive-contrast.py`, `check-interactive-contrast-selftest.py`). Both sides were right; composed rather than picked one — every check from both branches now runs (`ci/run-fixtures.sh:8-24`). `gate / full-gate-build` was failing on this branch (run 31921507701) with a genuine defect, not a stale base: `interactive-state-contrast-selftest.spec.ts` imported `assertContrast`/`contrastRatio`/`textContrastFloor` directly from `interactive-state-contrast.spec.ts`. Playwright refuses to load a test file that another test file imports and fails the entire collection, not just the importing file: ``` Error: test file "smoke/interactive-state-contrast-selftest.spec.ts" should not import test file "smoke/interactive-state-contrast.spec.ts" Listing tests: Total: 0 tests in 0 files ``` That's what `{"stage":"playwright-smoke","status":"fail","duration_ms":1050,...}` in the CI log was — a collection-time failure before any test ran, which is why the log carried no further detail. (The `cargo` exit-101 also visible in that run's log is unrelated and pre-existing: `Swatinem/rust-cache`'s `cargo metadata` probe fails on every typikon run because this repo has no `Cargo.toml` — documented and treated as non-fatal in `.github/workflows/gate-attestation.yml`'s own header comment. It logs an error but the job continues past it; the actual failure is the playwright stage above.) Fixed by extracting the pure contrast/threshold math into a new non-spec module, `ci/smoke/interactive-contrast.ts`, and pointing both spec files at it instead of one importing the other (`ci/smoke/interactive-contrast.ts` new; `ci/smoke/interactive-state-contrast.spec.ts:26-33` + `ci/smoke/interactive-state-contrast-selftest.spec.ts:24` updated). `readEffectiveStyle` stays in the browser-driven spec — it needs a Playwright `Locator` and only that file uses it. **Negative fixture: `ci/smoke/interactive-state-contrast-selftest.spec.ts` — watched failing by `npx playwright test --config ci/playwright.config.ts --list`, which produced exactly the "should not import test file" / "Total: 0 tests in 0 files" error above before the fix, and now lists 7 tests across 3 files (verified again against the real `examples/sample-blog` build: `npx playwright test` — 16/16 passed, including the browser-driven interactive-state assertions on every real route).** This isn't a check I added — it's Playwright's own test-collection step, which already runs on every `bin/typikon-check`/CI invocation with no additional wiring; the defect was that this branch's own new file tripped it. While rebasing I re-audited `check-interactive-contrast.py` against its own established comment-tag convention (the earlier review round tagged two blocks — FAQ deep-link anchor, home-triad-mark — that had rationale prose with no `WHY:`/`NOTE:`/etc. tag). Two more blocks in the same file had the identical gap and were missed by that fix: the `.nav-links a:hover::after` "why no separate entry" note (`ci/check-interactive-contrast.py:216-219`) and the home-nav-secondary-nav fallthrough note (`:221-224`), plus the `NOT_TEXT_CONTRAST` dict's own rationale (`:337-343`). Tagged all three `WHY:`, matching the file's own convention. Also tagged `ci/smoke/interactive-state- contrast.spec.ts`'s `TARGETS` comment (`:56-63`) — same defect class, same file inconsistency (the file's other rationale comment, `readEffectiveStyle`'s, already carried `WHY:`). ## Addressed in the prior independent adversarial review An independent adversarial review returned `must_fix` on three findings against an earlier version of this PR. All three were fixed at that time and verified intact after this session's rebase: 1. **Correctness — the property regex had no left-boundary anchor.** `_prop_var_re()` (`ci/check-interactive-contrast.py:118-126`) built `(?:{prop_alt})\s*:\s*...` with nothing anchoring the start, unlike the sibling `COLOR_AFFECTING_RE` two lines above it, which uses `(?:^|;)\s*(...)`. `prop_alt="color"` matched the substring `color:` inside `background-color:`/`text-decoration-color:`/etc., so a selector with both properties on the same rule could silently resolve to the WRONG property's value with no error. Fixed by mirroring `COLOR_AFFECTING_RE`'s `(?:^|;)` boundary in both `_prop_var_re()` and its sibling `_prop_inherit_re()` (`ci/check-interactive-contrast.py:118-136`). Reproduce the pre-fix bug: ``` python3 -c " import sys; sys.path.insert(0, 'ci') import importlib.util spec = importlib.util.spec_from_file_location('m', 'ci/check-interactive-contrast.py') m = importlib.util.module_from_spec(spec); spec.loader.exec_module(m) print(m.find_declared_var('.widget { background-color: var(--danger); }', '.widget', 'color')) " ``` prints `None` now; printed `danger` before the fix. Re-verified this session: `python3 ci/check-interactive-contrast.py` still passes clean post-rebase. 2. **Missing negative-case fixture for the checker's own logic.** Fixed by committing `ci/check-interactive-contrast-selftest.py`, wired into `ci/run-fixtures.sh` immediately after the checker it tests — function-level synthetic-CSS assertions (including the reviewer's own two repro cases) plus an end-to-end mutation of the real `static/css/style.css` (reverts the #64 fix, asserts the exact WCAG-floor failure, restores byte-identical, confirms a second uncovered-rule mutation fails closed too). The Playwright side got the same treatment via `ci/smoke/interactive-state-contrast-selftest.spec.ts`. Re-run this session: both pass clean (`python3 ci/check-interactive-contrast-selftest.py`; `npx playwright test ci/smoke/interactive-state-contrast-selftest.spec.ts` — 5/5). 3. **Two comment blocks broke the closed comment-tag set.** `ci/check-interactive-contrast.py:295-299` (FAQ deep-link anchor) and the home-triad-mark block were tagged `WHY:`. (This session found and fixed two more instances of the same gap — see above.) ## Enumerated acceptance criteria Issue #64's `Done when:` (restated identically in the reopening comment): **"a matrix covering every token/background/usage combination, assertions across the interactive states, and a gate stage that goes red when any of them regresses."** | Bullet | Satisfied by | Evidence | |---|---|---| | a matrix covering every token/background/usage combination | `MATRIX` in `ci/check-interactive-contrast.py:200-335` — 41 entries, each resolved from the live CSS source (not hand-typed), covering every selector this theme renders in a non-default interactive state that touches `color`, a text-bearing `background`, or `opacity` | `ci/check-interactive-contrast.py:426-461` walks each entry via `resolve_chain()`; run output lists all 41 with their resolved token/ratio | | assertions across the interactive states | Static: `resolve_chain()` + WCAG floor check per `MATRIX` entry (`ci/check-interactive-contrast.py:426-461`). Browser: `ci/smoke/interactive-state-contrast.spec.ts:104-146` drives real Chromium through default → focus → hover → active per element via `getComputedStyle`. `:visited` covered by source-absence check (browser cannot observe it — history-sniffing protection, documented `ci/smoke/interactive-state-contrast.spec.ts:14-20`) | Real run against `examples/sample-blog`: 16/16 Playwright tests pass across every route in the build's sitemap | | a gate stage that goes red when any of them regresses | Coverage scan `ci/check-interactive-contrast.py:488-512` fails closed on any new/unreviewed state rule; `ci/run-fixtures.sh:8-24` runs both the static and selftest checks on every `bin/typikon-check`/CI invocation; `ci/smoke/` specs run in the same `npx playwright test` CI already invokes | Negative fixtures (this PR's own selftest files) prove the red path — see "Addressed" #2 above and the negative-fixture line above for this session's fix | Issue #136's `Done when:` — **not this PR's job**: its specific defect (`--rule` used as a form-control border) already merged in #140 (`ci/check-control-contrast.py` verifies `.purchase-box select` / `.buttondown-form input[type="email"]` both clear 3:1 — confirmed still passing post-rebase: `python3 ci/check-control-contrast.py`). The one thing that kept #136 open — the home page's missing `<h1>` — was split into #142 and landed on `main` as #152 before this branch rebased past it (`ci/check-home-heading.py`, now composed into `ci/run-fixtures.sh:16` alongside this PR's own additions). See "Notes for the reviewer" below. ## Evidence (from the original submission, still accurate) - `ci/check-interactive-contrast.py:200-335` (`MATRIX`) — every selector this theme renders in a non-default interactive state that touches `color`, a text-bearing `background`, or `opacity`, each entry resolved from the live CSS source rather than hand-typed. - `ci/check-interactive-contrast.py:139-160` (`find_declared_var`, left-boundary-anchored) + `:163-177` (`resolve_chain`) — walks an explicit selector chain exactly mirroring real CSS cascade/inheritance. - `ci/check-interactive-contrast.py:208-209` — `.nav-links a:nth-child(3):hover`, resolved from source, must equal `--aporia-interactive` (4.91:1 against `--bg`) — the exact rule #64 originally fixed. - `ci/check-interactive-contrast.py:488-512` (coverage scan) — parses every `:hover`/`:focus`/`:focus-visible`/`:active`/`:visited`/`:disabled` rule touching `color`/`background`/`opacity` and fails if it isn't in `MATRIX`, `NOT_TEXT_CONTRAST`, or the opacity special-case. - `ci/contrast.py` — the WCAG luminance/contrast formula and `:root` parser, extracted from `ci/check-control-contrast.py` (#136) so both checks share one formula. - `ci/smoke/interactive-state-contrast.spec.ts:128-148` (`readEffectiveStyle`) drives real Chromium and reads `getComputedStyle`'s actual composited output, catching a cascade bug a static parse structurally cannot see. - `:visited` is explicitly out of the Playwright spec's reach (`ci/smoke/interactive-state-contrast.spec.ts:14-20`): every major browser engine reports `:visited` computed style as if unvisited, to block history-sniffing. `ci/check-interactive-contrast.py` covers it instead by parsing the CSS source directly and confirming no `:visited` rule exists. - `ci/run-fixtures.sh:8-24` + `.kanon-ci.toml` — both checks and their negative-case fixtures wired into the gate. ## What a reviewer would have to disprove - That any `MATRIX` entry's resolved token is wrong — reproducible by running `python3 ci/check-interactive-contrast.py` and reading the printed line for that entry against `static/css/style.css` directly. - That the coverage scan misses a real state-changing rule — reproducible by adding any `:hover`/`:focus`/`:active`/`:visited`/`:disabled` color rule anywhere in `style.css` and confirming the script fails before making it pass by adding a matching entry. - That `find_declared_var()`'s property regex can still cross-match a wrong property — reproducible by running `python3 ci/check-interactive-contrast-selftest.py`, which fails closed on that exact class today. - That the Playwright cross-import defect from this session is still present — reproducible by running `npx playwright test --config ci/playwright.config.ts --list` and confirming it lists tests rather than erroring. ## Notes for the reviewer - Issue #136 is not closed by this PR and doesn't need to be — its form-control-border-contrast defect already merged in #140, and the one thing keeping #136 open (the home page's missing `<h1>`) was correctly split into #142, landed as #152, and is out of this PR's scope. - No CSS colors changed. `git diff origin/main...HEAD -- static/css/style.css` is empty by design. - `gate / full-gate-build` and `gate / gate` are green on this branch's current tip (run 31923043711, post-rebase + post-fix) — mergeable, clean. - `verda-build` (the fleet's local-gate box) is currently unusable for this repo's `fixtures-gate` stage for two reasons unrelated to this branch: its disk is at 100% (286M free on 2.4T — tracked in `metis-ops/collab/OPERATOR.md`), and separately it has no `pip`/`jsonschema` installed at all (CI's own `check_cmd` installs `jsonschema`/`fonttools`/`Brotli` via pip before running `ci/run-fixtures.sh`; verda's gate script does not). Both are environment gaps on that box, not code defects — confirmed by running every `ci/run-fixtures.sh` script individually on this branch, plus the real Playwright suite against a real `zola build`, all green. GitHub Actions' `gate / full-gate-build` is the authoritative signal here and is green. --------- Co-authored-by: forkwright <cody@forkwright.com>
forkwright
pushed a commit
that referenced
this pull request
Aug 16, 2026
🤖 I have created a release *beep* *boop* --- ## [0.4.0](v0.3.0...v0.4.0) (2026-08-16) ### Features * **schemas:** add a fail-closed consumer schema registry ([#154](#154)) ([3d9432a](3d9432a)) * **templates:** make the favicon path configurable like every sibling brand asset ([#141](#141)) ([729ce2e](729ce2e)) ### Bug Fixes * **ci:** stop exempting the gate's own fixture corpus from full-gate-build ([#151](#151)) ([8690c82](8690c82)) * **templates:** give the home page a genuine top-level heading ([#152](#152)) ([880f711](880f711)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
5 tasks
forkwright
added a commit
that referenced
this pull request
Aug 16, 2026
…g asset (#162) ## Finding `config.extra.favicon_path`, `config.extra.logo_path`, and `config.extra.og_image` are each rendered into `public/` through Zola's `get_url()` (`templates/base.html:16,44`) or raw string concatenation (`templates/partials/nav.html:8`, `templates/partials/ld-organization.html:14`). Neither `get_url()` nor `zola check`'s link graph (built from Markdown content links, not template-rendered asset references) verifies the referenced file actually exists under `static/`. A mistyped or stale value in any of the three keys builds and `zola check`s clean while shipping a broken `<link>`/`<img>`/JSON-LD reference. ## Evidence Reproduced live on unmodified `origin/main` before touching anything (fixture site, all three keys pointed at nonexistent files, `zola` 0.22.1): ``` === zola build === exit=0 === zola check === exit=0 <link rel="icon" href="https://fixture.example.com/img/does-not-exist-favicon.svg" type="image/svg+xml"> <meta property="og:image" content="https://fixture.example.com/img/does-not-exist-og.svg"> "logo": "https://fixture.example.com/img/does-not-exist-logo.svg" === do the three files exist under public/? === MISSING: img/does-not-exist-favicon.svg MISSING: img/does-not-exist-logo.svg MISSING: img/does-not-exist-og.svg ``` Both `zola build` and `zola check` exit 0 while none of the three referenced files exist — matches the issue's own reported reproduction, extended to all three keys. Fix (verified `file:line` on this branch): - `bin/typikon-check-assets` (new) — reads `config.extra.{favicon_path,logo_path,og_image}` (with `favicon_path`'s documented default `img/favicon.svg`, `templates/base.html:16`) and checks each against the **real, already-built** `public/` tree — the one place all three keys converge regardless of which template mechanism rendered them, so it needs no reimplementation of Zola's site-over-theme static merge. - `bin/typikon-check:281-290` — wires it in as a new stage immediately after `zola-build` (stage 3), following the file's existing `run_stage`/`require_or_skip` pattern. - `ci/run-fixtures.sh:15` — the new regression fixture runs as part of the standard local/CI gate alongside the sibling `check-favicon-path.py`/`check-init-favicon-path.py`. ## Why this matters The three consumer-side brand-asset paths documented in `docs/AGENTIC.md`'s brand-identity table share this gap (`favicon_path`, `logo_path`, `og_image`, `docs/AGENTIC.md:190-196`). A consumer site can ship a broken favicon, a broken header logo / JSON-LD `Organization.logo`, or a broken Open Graph image with a fully green build and a fully green `zola check` — the failure is silent until someone notices the icon/image is missing in production. ## Desired correction Implemented: `bin/typikon-check-assets` fails loud, naming the offending `config.extra` key and the resolved path that has no backing file, for all three keys — not just the one (`favicon_path`) that originally surfaced this as a follow-up from #141. ## Negative fixture > Negative fixture: `ci/check-asset-path-existence.py` — watched failing by > `python3 ci/check-asset-path-existence.py` after temporarily neutering the new > `bin/typikon-check-assets` existence check (`if resolved.is_file():` replaced with `if True:`), > which produced: > ``` > FAIL: favicon_path fixture: typikon-check-assets expected exit 1, got 0 > FAIL: logo_path fixture: typikon-check-assets expected exit 1, got 0 > FAIL: og_image fixture: typikon-check-assets expected exit 1, got 0 > ``` > Restoring the real check makes the same command pass (exit 0). The fixture runs the SHIPPED > `bin/typikon-check-assets` as a subprocess (not a reimplementation), and exercises each of the > three keys **in isolation** — corrupting exactly one while the other two stay valid — so a > failure that names the wrong key, or a check that only fires when everything is broken at once, > is caught. It also reproduces, inline, the issue's own evidence that `zola build`/`zola check` > both exit 0 on the corrupted `favicon_path` case. ## Review follow-up: path-escape hole (must_fix, addressed) An independent adversarial review found that the existence guard did not confine resolution to `public/`: `bin/typikon-check-assets` did `relative = raw.lstrip("/")` then `resolved = public / relative` then `if resolved.is_file():` — and `Path.is_file()` performs real OS resolution, which honors `..` and walks straight out of `public/`. A value like `favicon_path = "../static/img/favicon.svg"` (the exact source location of the asset — the single most plausible real mistake, not a contrived one) or `og_image = "../outside-public-secret.svg"` (a file that exists only at the site root and is never copied into `public/` by `zola build`) was reported **passing**, reproducing the exact silent-ship defect issue #155 exists to close, inside the fix itself. **Fix** (`bin/typikon-check-assets`): resolve the joined path (`(public / relative).resolve()`) against a resolved `public/` (`public_resolved = public.resolve()`), then reject anything that is not `resolved.is_relative_to(public_resolved)` **before** checking `is_file()` — containment is checked, not merely joining. A rejected escape reports a distinct error (`"... resolves to ..., outside public/ (...) — refusing"`) so it doesn't read as a generic missing-file failure. **Negative fixture** (extends `ci/check-asset-path-existence.py`, case `path-escape`): a `favicon_path = "../outside-public-secret.svg"` value where the target file genuinely exists on disk at the site root but `zola build` never copies it into `public/` (confirmed: `find public -iname '*outside*'` finds nothing after a real build). This closes the gap the review named in the original three fixture cases — they only corrupted the **filename** (`img/does-not-exist-*`), never the **path shape**, so they could never have caught a `..`-escape. > Negative fixture: `ci/check-asset-path-existence.py` (case `path-escape`) — watched failing by > running the SAME committed test module with `CHECK_ASSETS` pointed at the pre-fix script > (`git show 0d6645c:bin/typikon-check-assets`), which produced: > ``` > FAIL: path-escape fixture: typikon-check-assets expected exit 1, got 0 > stdout: {"checked": 3, "passed": 3, "skipped": 0, "failed": 0} > ``` > against `bin/typikon-check-assets` itself, directly, on the reviewer's own fixture shape: > pre-fix `{"checked": 3, "passed": 3, "skipped": 0, "failed": 0}` exit 0 (the escape silently > passes); post-fix `{"key": "favicon_path", "path": "../outside-public-secret.svg", "error": > "config.extra.favicon_path = '../outside-public-secret.svg' resolves to > .../outside-public-secret.svg, outside public/ (.../public) — refusing"}` then > `{"checked": 3, "passed": 2, "skipped": 0, "failed": 1}` exit 1. Restoring the real (fixed) > script and re-running `python3 ci/check-asset-path-existence.py` end-to-end: `OK: ...rejects a > \`..\`-escaping value pointing at a real file outside public/...`, exit 0. ## Acceptance criteria (issue #155 "Done when") > Done when: a consumer config with a stale/mistyped `favicon_path`, `logo_path`, or `og_image` > fails `bin/typikon-check` (or `zola build`/`check`) with a message naming the key and the > missing file, instead of building and deploying clean. - [x] **`favicon_path`** — `bin/typikon-check-assets`'s per-key check (`ASSET_DEFAULTS` loop, `bin/typikon-check-assets:108-141`) covers it, including its `img/favicon.svg` default. Verified failing in isolation by `ci/check-asset-path-existence.py`'s `favicon_path` case. - [x] **`logo_path`** — same loop, no default (matches `templates/partials/nav.html:8`'s `{%- if config.extra.logo_path %}` guard — only checked when the consumer sets it). Verified failing in isolation. - [x] **`og_image`** — same loop, no default (matches `templates/base.html:44`'s `{%- if config.extra.og_image %}` guard). Verified failing in isolation. - [x] **Fails `bin/typikon-check`, naming the key and the missing file** — `bin/typikon-check` now exits 1 with a `typikon-check-assets` stage failure (`bin/typikon-check:281-290`), and the underlying script's stderr names both, e.g. `{"key": "favicon_path", "path": "img/x.svg", "error": "no file at .../public/img/x.svg — ..."}`. - [x] **"once", not per-key duplication** — one script, one loop over the three keys, one place (the built `public/` tree) all three converge, satisfying the issue's "the fix belongs wherever those paths are resolved, once." ## Self-check against the fleet's measured failure shape *Error path*: config.toml missing/unparseable, or `public/` absent (build never ran) → exits 2 (runtime error), distinct from exit 1 (asset genuinely missing) and exit 0 (clean) — never a silent pass. *Write path*: none — this is a read-only existence check, no write path to protect. *Guard's own arithmetic*: considered whether a fully-qualified external URL (e.g. `og_image = "https://cdn.example.com/foo.png"`) could produce a false failure for a legitimately different reference shape — verified empirically that the existing templates do **not** special-case external URLs at all (`get_url()` on such a value renders a broken doubled URL, `https://fixture.example.com/https://cdn.example.com/foo.png`, confirmed by direct `zola build` against a fixture); the check flagging that as unresolved is therefore correct, not a new false positive on a working configuration — every real/documented/fixture usage of all three keys (`docs/SCHEMAS.md:104`, both `examples/*/config.toml`) is a plain `static/`-relative path. ## Verification run - `python3 ci/check-triad-schema.py` … `ci/check-interactive-contrast-selftest.py` (all 11 existing `ci/*.py` checks): all pass, unchanged. - `bash ci/check-workflow-template.sh`, `check-consumer-check-extension.sh`, `check-fixture-corpus-exemption.sh`: all pass, unchanged. - `python3 ci/check-asset-path-existence.py` (now including the `path-escape` case): pass. - `bash ci/run-fixtures.sh`: every `ci/*.py`/`ci/*.sh` check plus the full-mode `bin/typikon-check` run on both `examples/*` sites pass, including `typikon-check-assets`; the only non-pass line is `pa11y` (`fail` in full mode / `skip` in dev mode) because `pa11y-ci` is installed under this dev box's npm-global prefix but not on `PATH` — pre-existing, unrelated to this change, CI installs it on `PATH` directly. - `TYPIKON_CHECK_MODE=dev bin/typikon-check examples/sample-blog` and `.../sample-shop`: every stage passes including `typikon-check-assets` (exit 3 = documented "incomplete", not failure, since `pa11y` is the sole skip). - `kanon lint . --all`: 0 violations (real exit 0). - Full build/gate not run locally per this fleet's standing policy (metis load against a vgate admission limit) — pushed for CI to grade. Closes #155 --------- 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 #110
Finding
templates/base.html:16hardcoded<link rel="icon" href="/img/favicon.svg" type="image/svg+xml">. Of the sibling brand-identity values inconfig.toml [extra],og_image(templates/base.html:43-47) is the only one that resolves throughget_url()at all — though unlike thefavicon_pathfix, an unsetog_imageomits its<meta>tag entirely rather than falling back to adefault(), so "identical shape" overstates it too;logo_pathis a separate raw-concatenation shape used intemplates/partials/nav.html:8andtemplates/partials/ld-organization.html:14(notbase.html's head block, and not throughget_url()at all);theme_coloris a color string, not a path. The favicon alone had no config knob at all, so a consumer whose brand mark lives anywhere else had to place a second copy at typikon's exact path (forkwright/ardent-site#12is the filed consequence: three byte-identical SVGs, one of them existing only to satisfy this path).Evidence
templates/base.html:16-31now readsfavicon_pathfromconfig.extrathroughget_url()with adefault()ofimg/favicon.svg(unaffected existing consumers), and derives the<link>'stype=fromfavicon_path's own extension (svg/ico/png/gif/jpg/jpeg/webp) instead of leaving it hardcoded — see "Findings from adversarial review" below for why this half of the fix exists.bin/typikon-init:192scaffolds# favicon_path = "img/favicon.svg"into every new consumer'sconfig.toml [extra], next to the existing# logo_pathline.docs/AGENTIC.md:190documentsfavicon_pathin the brand-identity table, including the type-derivation behavior.ci/check-favicon-path.pybuilds a real fixture site withzola buildthree times:favicon_pathunset (theme default resolves, unaffected), set to a custom SVG (the original defect — before the fix,favicon_pathhad no effect on the rendered href at all), and set to a custom.ico(thetype=must follow the extension, not stay hardcoded). Locally:OK: favicon <link> resolves the theme default when unset, a consumer's favicon_path when set, and a type= matching that asset's own extension.ci/check-init-favicon-path.pyruns the realbin/typikon-initagainst two genuinely isolated environments and asserts the scaffoldedconfig.tomlcarriesfavicon_pathdefaulted toimg/favicon.svgin both. Locally:OK: bin/typikon-init scaffolds config.toml [extra] with a favicon_path key defaulted to img/favicon.svg in both a genuinely identity-less environment (fallback engages, commit uses the fixed typikon-init identity) and one with a real git identity configured (fallback stays out of the way, commit uses the caller's own identity).ci/run-fixtures.shwires both checks into the fixture gate.A second, independent defect, fixed in the same PR because it blocks this PR's own CI:
ci/check-init-favicon-path.pyis the first CI check to invokebin/typikon-initend-to-end. It exposed that the scaffold commit (previously an unconditionalgit commit -q -m "chore(${SITE_NAME}): scaffold from typikon") depends on an ambientuser.name/user.emailthat a fresh CI runner or agentic sandbox does not have.bin/typikon-init:305-316now checks whetheruser.email/user.nameresolve at all, and only when neither does, scopes a fixed identity (typikon-init <typikon-init@localhost>) to that one commit viagit -c.Findings from adversarial review, and how each was closed
An independent adversarial reviewer returned
must_fixagainst an earlier state of this PR. Every finding below is addressed in the diff or this body, not asserted away:gate / ai-attributionandgate / check-trailerruns (both scan thePR_BODYenv var as it exists when the job starts). This PR now carries a new commit (the.icoMIME-type fix) that triggers a fresh CI run against the body as it reads right now — the reviewer or anyone else can confirm the head SHA on the most recentgate-attestationrun matches this body's edit time.<link>'stype=staying hardcoded toimage/svg+xmlregardless offavicon_path— fixed, not just noted:templates/base.htmlnow derivestype=from the configured path's extension, andci/check-favicon-path.py's third case is the regression fixture. Verified as a genuine negative case: reverting the derivation to the old hardcodedtype="image/svg+xml"makes that fixture fail (FAIL: ico-favicon fixture: expected type image/x-icon ... got 'image/svg+xml') while the other two cases stay green (they never exercise a non-SVG extension); restoring the fix turns it green again.get_url()performing no existence check, so a mistypedfavicon_pathbuilds and ships a broken favicon silently — real, confirmed by direct reproduction (zola buildandzola checkboth exit 0 against afavicon_pathpointing at a file that does not exist; the rendered<link>references it anyway), not fixed here, filed as get_url() performs no existence check for consumer-declared asset paths (favicon_path, logo_path, og_image) #155. It is not new:logo_pathandog_imageshare the identical gap and predate this PR (also verified directly — a nonexistentlogo_pathbuilds clean too). Scoping the fix tofavicon_pathalone would import a known defect into a brand-new config key; the shared, pre-existing nature of the gap across three keys makes a general existence-check pass (issue get_url() performs no existence check for consumer-declared asset paths (favicon_path, logo_path, og_image) #155) the right unit of work, not something to hand-roll narrowly inside this PR..github/workflows/gate-attestation.yml:44-53confirmskanon lintis forge-only and not run by this repo's GitHub Actions gate, so this claim has never been CI-verified — it is asserted here as a locally-verified figure only. Running the exact command.kanon-ci.toml's lint stage specifies,kanon lint . --summary, against this branch's head produces 1 violation (PY/bare-exceptatci/asset-provenance-scan.py:311), not 33 — and that one violation is pre-existing and unrelated to this diff: the same line, unchanged, is present onorigin/main(confirmed viagit show origin/main:ci/asset-provenance-scan.py). This PR's own new/changed files (templates/base.html,bin/typikon-init,ci/check-favicon-path.py,ci/check-init-favicon-path.py,docs/AGENTIC.md) introduce zero new lint violations.ci/check-init-favicon-path.pyspread the caller's ambientos.environinto thebin/typikon-initsubprocess rather than neutralizing it, so it passed on GitHub Actions runners (which happen to start with no git identity) without ever proving the fallback works on a box that HAS one configured — or, more importantly, without proving the fallback stays OUT of the way when a real identity exists. Rewritten to build two genuinely isolatedHOMEenvironments (GIT_CONFIG_NOSYSTEM=1, no inheritedGIT_AUTHOR_*/GIT_COMMITTER_*): one with no identity anywhere the isolated env can see (forces the fallback branch), one with a real identity seeded into the isolatedHOME's.gitconfig(forces the fallback to correctly NOT engage). Both assert not just exit code but the actual commit author, so a fallback that fired when it shouldn't have would also be caught. Verified as a genuine negative case: revertingbin/typikon-init's fallback to the old unconditional commit and re-running the new fixture reproduces the original failure exactly (fatal: unable to auto-detect email address ... exit 128) on the no-identity case; restoring the fix turns it green again.get_url()" claim overstated its precedent. Corrected in the Finding section above:og_imageis the only sibling that usesget_url()at all, and even it differs from thefavicon_pathfix (omits its tag when unset rather than falling back to adefault());theme_colorisn't a path andlogo_pathis two raw-concatenation call sites, neither viaget_url(). This doesn't change what the code does, only what the body claims it's consistent with.Why this matters
The duplication is not a consumer mistake; it is the shape this template forces. Any consumer whose brand mark is not literally at
/img/favicon.svginherited the same duplicate SVG, and the defect was per-consumer and permanent rather than one-off.Separately: a scaffolding tool whose only job is to bootstrap a fresh site, and that hard-fails the moment it runs somewhere with no pre-existing git identity, defeats the repo's own stated design ("optimized for agentic operation... humans should not need to develop here" —
README.md:11). An agent or CI container is exactly the environment most likely to have no git identity configured.Desired correction
favicon_pathadded toconfig.toml [extra], defaulted toimg/favicon.svg, rendered throughget_url()— done (templates/base.html:16-31).<link>type=follows the configured asset's own extension — done (templates/base.html:16-31; not part of favicon path is hardcoded while every sibling brand asset is configurable, forcing consumers to duplicate their logo #110's original scope, added because making the path configurable without it is half a contract).docs/AGENTIC.md— done (docs/AGENTIC.md:190).bin/typikon-init's scaffold no longer requires an ambient git identity to complete — done (bin/typikon-init:305-316), and the regression fixture now genuinely forces both branches (ci/check-init-favicon-path.py).get_url()'s missing existence check acrossfavicon_path/logo_path/og_image— tracked, not fixed here: get_url() performs no existence check for consumer-declared asset paths (favicon_path, logo_path, og_image) #155.Done when: a consumer can point the favicon at an existing asset without placing a duplicate, and
ardent-sitecan delete two of its three identical SVGs.