visual-artifacts v1: PR 3 sprint journal and custom stack views#219
Merged
Conversation
Wires the two aggregating renders so a sprint at a glance and a custom workflow stack get their own HTML views. Same trust + path- safety contract; both kinds aggregate sources rather than rendering a single phase artifact. bin/render-artifact.sh: - Drop the "journal/stack reserved for PR 3" exit-2 guard. Both kinds are now wired. - Argument parser accepts --today and --date YYYY-MM-DD for journal and a positional stack name for stack. The stack name is regex- validated against [a-zA-Z0-9_-]+ before any disk lookup so path traversal (../etc, a/b, "foo bar") fails fast with exit 1. - Source resolution skips for journal/stack; they aggregate. - Per-phase trust check skips; trust shows inline per source. Page header badge shows "aggregated" (data-trust=not_applicable). - render_journal_body: walks every registered phase, looks up each latest artifact via find-artifact.sh --no-session-sync (read-only, PR 1 pass 7 lesson), shows a timeline row with phase chip + trust status chip (verified / unverified / tampered / missing) + phase- specific summary line. Missing artifacts render explicitly. - render_stack_body: looks up the stack definition under examples/custom-stack-template/<name>/stack.json or $NANOSTACK_STORE/stacks/<name>/stack.json; falls back to the active phase_graph from .nanostack/config.json. Renders a table of phases with depends_on and trust state, plus an inline SVG DAG laid out by depth. - Manifest now writes AFTER the HTML body so the aggregated SOURCE_ARTIFACTS_JSON populated by the body renderer is recorded. --manifest-only still works because the body runs anyway for journal/stack (just discarded). - New scratch dir per render (TMP_ROOT_FALLBACK via mktemp -d) cleaned by the EXIT trap. Used by the stack DAG layout. bin/lib/visual-render.sh: - nano_visual_trust_badge_text adds "aggregated" for not_applicable. CSS gets a .trust-badge[data-trust="not_applicable"] rule. # url-allowlist marker on the screenshot reject pattern so the safety lint accepts the legitimate case-pattern URLs. ci/e2e-visual-artifacts.sh: - New cells 18-22: sprint journal happy path, journal flags missing/tampered phases, stack DAG for compliance-release, stack-name validation (rejects ../etc, a/b, space), journal --date YYYY-MM-DD shape validation (rejects 2026/05/11 and shell metachars). ci/check-visual-artifact-templates.sh: - Helper bug fixed: grep -nE patterns that start with `-` are now passed via -e -- so a pattern like `--no-session-sync` is not interpreted as a grep long-option. - 5 new checks: journal uses --no-session-sync, stack name regex rejects path traversal, --date regex shape, no SVG <image href, no SVG <foreignObject (would bypass CSP). - SVG xmlns="http://www.w3.org/..." is recognized as namespace identifier (no fetch) and excluded from the no-URL sweep. Test counts: - e2e: 154 -> 194 (+40 across 5 PR 3 cells) - template safety: 25 -> 30 (+5) - Total contract surface: 179 -> 224 (+45)
Codex PR 3 pass 1 findings. P2 — phase registry never sourced render_journal_body and render_stack_body checked for nano_all_phases and nano_phase_graph_json but bin/lib/phases.sh was never sourced, so the functions were undefined. Journal silently skipped custom phases and stack rendered "stack not found" instead of falling back to the project's phase_graph. Source bin/lib/phases.sh at the top of render-artifact.sh. P2 — --date filter render_journal_body --date 2026-05-09 still went through find-artifact.sh which returns the LATEST artifact in the last 30 days, not the latest on the requested date. The page banner said "2026-05-09" while the rows pointed at today's artifacts. Add _journal_latest_on_date that filters .nanostack/<phase>/*.json by filename prefix YYYYMMDD (matches save-artifact.sh's convention) and verifies the .project field. For today's date the find-artifact fallback still applies so artifacts saved through other means are still picked up. Adds three regression cells (22a, 22b, 22c): - --date filter: a plan artifact renamed to 20260509-100000 appears for --date 2026-05-09 and is missing for --date 2026-05-08. - stack falls back to .nanostack/config.json:.phase_graph when no stack file is found. Custom phase license-audit appears. - journal lists custom phases from custom_phases registry config. Test count: 194 -> 202.
Four findings from codex PR 3 pass 2. P2 — --strict on aggregate views was a no-op journal/stack set TRUST=not_applicable before sources were inspected, so --strict produced a "strict: true" manifest while aggregated sources could be tampered or unverified. Enforce strict AFTER the body renderer collects sources: any integrity_missing or integrity_mismatch in SOURCE_ARTIFACTS_JSON triggers exit 3. Plain "missing" sources (phase not run yet) stay acceptable because they are an expected state, not suspect data. P2 — bare `journal` left the date empty `render-artifact.sh journal` with no --today / --date produced a filename ending in `-journal-.html` and a header that read "journal · ". Default JOURNAL_DATE to today's UTC date when neither flag was passed. Same for STACK_NAME, which now defaults to "default" so a bare `stack` falls back to the project's phase_graph. P3 — TMP_ROOT_FALLBACK leak Every render created a /tmp/render-artifact.* dir and the trap was disabled before it was cleaned up, so the directories accumulated. Remove the scratch dir explicitly before `trap - EXIT` in every exit path (manifest-only, normal HTML success). P3 — stack manifest JSON escape The stack renderer hand-built the source_artifacts JSON with awk, escaping only double quotes; a project path with a backslash or control character broke the JSON. Use jq -n with --arg like the journal renderer does, so jq handles the full RFC 8259 escape set. Regression cells: - 22d: bare `journal` shows today's date, filename has no stray dash. - 22e: journal/stack --strict fail on integrity_missing and integrity_mismatch (exit 3). - 22f: scratch dir count is the same before and after running plan, stack, and journal renders. - 22g: stack manifest is parseable JSON, source_artifacts is an array. Test count: 202 -> 209.
Two codex PR 3 pass 3 findings. P2 — --manifest-only bypassed aggregate strict mode The strict check for journal/stack ran AFTER the manifest-only early exit, so `journal --strict --manifest-only` shipped a "strict: true" manifest even when an aggregated source was tampered. Move the strict block above the manifest-only branch. Adds cell 22h: both journal and stack with --strict --manifest-only return exit 3 when a source is tampered. P3 — SVG depth resolution capped at 10 rounds A custom phase_graph listed in reverse-topological order with more than 10 nodes left the tail of the chain out of depths.tsv, so those nodes appeared in the table but disappeared from the SVG. Replace the fixed 10-round loop with `seq 1 $((node_count + 1))` so a valid DAG is always fully resolved (linear chain = N-1 rounds worst case). Adds cell 22i: a 13-node reverse-topological chain renders every node in the SVG. Also fixes a flaky test in cell 22e: re-saving plan to set up the tamper case created a second artifact, and `ls | head -1` picked the older (un-tampered) file while the renderer picked the latest. rm -f before the re-save makes the latest-picker see the right artifact. Test count: 209 -> 215.
Codex PR 3 pass 4 findings. P2 — stack/journal pre-mkdir followed symlinks When visual/stack or visual/journal already existed as a symlink to an outside directory, the mkdir -p in the HTML_PATH derivation followed the symlink and created the stack-name subdirectory at the target before the descend safety check fired. The check would then exit 4, but the leak directory was already on disk. Stop pre-creating the kind subdirectory in the path-derivation branch. The post-validation `mkdir -p "$(dirname "$HTML_PATH")"` already creates the dir after nano_visual_assert_safe_descend confirms no symlink in the path components. P2 — --today fell back to find-artifact.sh (last 30 days) When today had no artifacts saved yet, render_journal_body fell back to find-artifact.sh which returned the latest artifact in the last 30 days. The page header said today's date but rows pointed at yesterday's (or last week's) artifacts. Drop the today fallback so strict date filtering applies uniformly. Regression cells: - 22j: symlinked visual/stack exits 4 and leaves no directory at the symlink target. - 22j (continued): symlinked visual/journal also exits 4. - 22l: today's journal does not surface yesterday's plan; yesterday's plan path does not appear in the HTML. Test count: 215 -> 220.
Two codex PR 3 pass 5 findings. P1 — tampered .project hid as missing A same-day artifact whose .project field was flipped (so the hash no longer matches and the project filter rejects it) rendered as "missing" in the journal timeline. Aggregate --strict allows "missing" (a legitimate "phase not run yet" state), so a tampered plan slipped through. Run nano_artifact_trust on every same-day candidate first: if the integrity hash is broken, surface that file with data-trust="integrity_mismatch" regardless of the .project filter. The renderer's row class flips to sev-bad and aggregate --strict catches it. P2 — malformed phase_graph crashed renderer A user-supplied stack.json with .phase_graph as a string, an object, or an array of scalars made render_stack_body iterate non-object entries and the script aborted under set -e with a raw jq error. Validate the graph shape upfront (array of objects with string .name) via a single jq -e expression. On failure render a graceful "Stack invalid" notice and return; the manifest still writes with an empty sources array. Regression cells: - 22m: tampered .project produces integrity_mismatch row; --strict exits 3. - 22n: stack with .phase_graph as a string renders "Stack invalid"; stack with .phase_graph as an array of scalars also renders "Stack invalid". No exit code crash. Test count: 220 -> 225.
Two codex PR 3 pass 6 findings. P2 — stack --strict bypass via tampered .project The journal renderer surfaced tampered same-day artifacts via the trust-aware _journal_latest_on_date helper, but render_stack_body still called find-artifact.sh directly. A user who flipped .project to "/other-project" made the file vanish from the stack table's "latest artifact" column and from the manifest's source_artifacts, so stack --strict allowed it as "missing". Add _latest_safe_artifact_for_stack: enumerate up to 30 most recent files in .nanostack/<phase>/, run nano_artifact_trust first, surface any integrity_mismatch immediately, otherwise apply the .project filter. Plug the helper into all three stack lookups (table row, SVG node, sources collection). P2 — graph validation too loose The pass-5 graph validator accepted depends_on as a string, empty arrays, and dangling references that crashed the renderer downstream. Tighten the validator with three sequential jq -e expressions: - non-empty array - each node has string non-empty .name and array-of-string .depends_on - every name in depends_on appears as a declared node On failure render a "Stack invalid" notice with the specific validation reason. Regression cells: - 22o: tampered .project produces integrity_mismatch row in stack table; stack --strict exits 3. - 22p: depends_on as string, empty graph, dangling dependency, and empty name all render "Stack invalid" gracefully (no crash). Test count: 225 -> 231.
Three codex PR 3 pass 7 findings. P1 — integrity_missing + .project flip slipped through Both helpers surfaced integrity_mismatch but treated integrity_missing as "trustworthy enough" until the .project filter, which dropped attacker-modified files and reported the row as missing. Aggregate --strict allows "missing", so the combined attack escaped both journal and stack strict modes. Surface integrity_missing alongside integrity_mismatch before the project filter in both helpers. P2 — cyclic phase_graph The validator accepted a -> b -> a because every name existed; the SVG depth resolver could not assign positions for any cycle node and the page rendered a table but an empty/truncated SVG. Add a Kahn-style cycle check in bash before the renderer continues. If any nodes remain unresolved after node_count+1 rounds (the worst-case for a linear chain), declare a cycle and bail with a "Stack invalid" notice that lists the unresolved phase names. Regression cells: - 22q: integrity stripped AND .project flipped surfaces as integrity_missing for both journal and stack; --strict exits 3 on both. - 22r: 2-node cycle (a<->b), 3-node cycle (a->c->b->a), and self-cycle (a->a) all render "Stack invalid" with "cycle" in the notice. Test count: 231 -> 238.
Codex PR 3 pass 8 finding (P2). A stack.json could pass the
previous validation with phase names containing whitespace
("license audit"), path separators ("bad/name"), or duplicates.
The shell-based depth resolver and SVG loop word-split on spaces,
the find under .nanostack/<phase>/ became ambiguous, and duplicate
names made dependencies ambiguous.
Add two more validation steps before rendering:
- regex `^[A-Za-z0-9_-]+$` for every phase name
- unique name check via `length == (unique | length)`
Regression cell 22s covers whitespace, slash, and duplicate cases.
Each renders the "Stack invalid" notice with a specific reason.
Test count: 238 -> 241.
Codex PR 3 pass 9 findings (2 P2). Malformed user-edited JSON aborted the renderer with raw jq exit codes instead of rendering the documented fallback notices. P2 — stack metadata reads A truncated stack.json passed the phase_graph guard (returns empty) but the display_name / description jq reads ran without `|| true`, so under set -e the script exited with jq's parse status before the "Stack invalid" page could be emitted. Wrap every read on the user-controlled file with `|| echo ""` and fall back to the requested name as display_name. P2 — journal same-day artifact reads A malformed same-day artifact reached the per-phase summary jq calls and aborted the whole journal render. Add `|| echo "(unreadable)"` to every summary read so a single corrupted file shows as "(unreadable)" in its row while the rest of the timeline renders normally. nano_artifact_trust already surfaced these as integrity_missing in the trust check. Regression cells: - 22t: truncated stack.json produces an HTML render (no crash). - 22u: a truncated plan artifact with today's date prefix lets the journal still render with the plan row visible. Test count: 241 -> 244.
Codex PR 3 pass 10 finding (P2). When a user-installed stack file existed under $NANOSTACK_STORE/stacks/<name>/stack.json but was malformed (missing .phase_graph, truncated JSON), the renderer silently fell back to the project's phase_graph and rendered "Custom stack · <name>" with the default DAG. Users would not see that their named stack was broken. Track named_stack_file_present and emit a "Stack invalid" notice when the named file was found but produced no graph. The fallback to the project graph still applies for the bare `stack` / `stack default` form, where no stack file was requested. Cell 22t updated: a named-but-malformed stack now renders the "Stack invalid" notice and does NOT show default phases like /think. Test count: 244 -> 246.
Codex PR 3 pass 11 finding (P2). When a stack included a phase whose latest artifact was truncated, _latest_safe_artifact_for_stack correctly selected it as integrity_missing, but the unguarded `jq -r '.integrity'` that follows ran under set -e and aborted the whole stack render before HTML/manifest could be moved into place. Mirror the journal path: append `|| echo ""` to the integrity read so a corrupted artifact surfaces as integrity_missing in the sources array (and aggregate --strict catches it cleanly). Cell 22v: truncated plan artifact + compliance-release stack. Without --strict: HTML renders and the plan row shows data-trust="integrity_missing". With --strict: exit 3. Test count: 246 -> 249.
Codex PR 3 pass 12 finding (P2). A misspelled stack name (e.g. `compliance-relase` instead of `compliance-release`) silently fell back to the project's default phase_graph and rendered the wrong DAG under the typo. The visual and manifest looked authoritative for a stack that did not exist. Restrict the fallback to `name == "default"` so only the bare `stack` / `stack default` form pulls the project graph. Any named miss falls through to the "Stack not found" notice below. Updates cell 22b to use `stack default` (the correct trigger). Adds cell 22w: typo `compliance-relase` renders "Stack not found" and does NOT show default phases; `stack default` still falls back to the project graph as documented. Test count: 249 -> 253.
Codex PR 3 pass 13 finding (P2). In a shared store (e.g. $HOME/.nanostack used by multiple projects), the tamper-surfacing helper returned the first integrity_missing/integrity_mismatch candidate regardless of which project the artifact belonged to. Another project's tampered artifact would pollute this project's journal/stack rows and make --strict exit 3 on unrelated work. Detect whether the store is project-local: git -C "$pwd" rev-parse --show-toplevel, then compare against $NANOSTACK_STORE. Both sides are canonicalized via realpath so macOS /tmp -> /private/tmp does not produce a false mismatch. When the store IS project-local (only this project writes there), surface integrity_missing/mismatch aggressively even if .project was flipped. When the store is shared, require .project to match before surfacing; foreign artifacts stay out of the view. Adds cell 22x: two projects share a store, one of them strips integrity on its plan; project B's journal shows plan as "missing" (not as integrity_missing), the OTHER project's data does not leak into B's HTML, and B's --strict exits 0. Existing pass 5/6/7 cells continue to pass under project-local stores. Same logic in render_journal_body and _latest_safe_artifact_for_stack. Test count: 253 -> 256.
Codex PR 3 pass 14 finding (P2). The stack helper capped the candidate list at 30 newest files BEFORE applying the project filter. In a shared store with 30+ newer artifacts from other projects, this project's older artifact never made it into the window and the stack row rendered as missing, even when the artifact existed (or was tampered) and should have surfaced. Drop the `| head -30` cap. The early-return inside the loop on the first match keeps the walk bounded; in the rare case where many candidates need checking, the cost is one jq -e per file until a match is found. Adds cell 22y: our project saves a plan first, then 35 newer artifacts from other projects appear in the same shared store. The stack render must still find OUR plan and reference it in the HTML. Test count: 256 -> 257.
Codex PR 3 pass 15 finding (P2). The visual artifact contract requires every manifest to include at least one source entry, but the early-return paths for "Stack invalid", "Stack not found", and graph validation errors set SOURCE_ARTIFACTS_JSON to [], producing manifests that downstream consumers would reject as schema-invalid. Add a _stack_synthetic_source helper that emits a one-element array referencing the stack file (when present) or just the requested name, with phase prefixed `stack:` to disambiguate from real phase artifacts. Used in all three failure modes: - Named stack file exists but phase_graph missing -> integrity_missing - Stack not found at any of the known paths -> not_found - Graph validation error (shape, cycle, etc.) -> integrity_missing Cell 22z verifies each failure mode produces a manifest with source_artifacts.length >= 1 and a phase that starts with `stack:`. Test count: 257 -> 261.
Codex PR 3 pass 16. Two findings. P2 — stack manifest didn't reference the stack file Successful stack renders only recorded per-phase artifact sources; the stack.json that defined the graph never appeared in the manifest, so a downstream consumer could not detect a change to the graph itself. Add a synthetic first source pointing at the canonical stack file with phase "stack:<name>" and trust "not_applicable" (stack definitions don't carry .integrity). P3 — journal lint was vacuous The template safety lint passed as long as `--no-session-sync` appeared anywhere in render-artifact.sh, including comments. It did not actually verify render_journal_body uses the documented read-only lookup. Tighten to check that the renderer references the dedicated _journal_latest_on_date helper, and keep the per-phase --no-session-sync check separately. Cell 23a: compliance-release stack manifest has 11 sources (1 stack definition + 10 phase artifacts) and the first source is the stack.json file. Test count: 261 -> 264; lint: 30 -> 31.
Two codex PR 3 pass 17 findings. P2 — stack used mtime ordering _latest_safe_artifact_for_stack used `ls -t` which orders by file mtime. A copied/restored/touched file gets a fresh mtime while keeping the original filename timestamp, so the stack table/SVG/ manifest could point at a stale artifact even when a newer one existed. save-artifact.sh names files YYYYMMDD-HHMMSS.json; the filename is the canonical ordering. Switch to `ls | sort -r` so the stack helper agrees with the per-phase resolver and journal. P2 — stack default manifest omitted config.json When `stack default` fell back to .nanostack/config.json via the phase registry, the manifest only listed per-phase artifacts. A change to custom_phases or phase_graph could completely rewrite the DAG without appearing in source_artifacts. Add an explicit case: when no named stack_file was used and `name == "default"`, record `$NANOSTACK_STORE/config.json` as the first source so the provenance contract holds for the default form too. Regression cells: - 23b: save plan twice, touch the older file. Stack picks the newer-by-filename file even though the older one has a newer mtime. - 23c: stack default with a custom config.json. First source in the manifest is the config.json path. Test count: 264 -> 266.
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
1 participant
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.
Summary
PR 3 of the Visual Artifacts v1 round. Wires the two aggregating renders so a sprint at a glance and a custom workflow stack get their own HTML views. Same trust + path-safety contract as PR 1+2.
What changed
Sprint journal view (
render_journal_body)bin/lib/phases.sh::nano_all_phases) for the requested date._journal_latest_on_date(YYYYMMDD-...json); strict, no 30-day fallback.verified/unverified/tampered/missing) plus a phase-specific summary line..projectwas flipped. Shared store: requires.projectmatch so foreign artifacts do not pollute the timeline.--strictrejects any source whose trust isintegrity_missingorintegrity_mismatch(exit 3);missingis allowed (phase not run yet).Custom stack view (
render_stack_body)examples/custom-stack-template/<name>/stack.jsonfirst, then$NANOSTACK_STORE/stacks/<name>/stack.json, then forstack defaultfalls back tonano_phase_graph_json(the project's.nanostack/config.json).[A-Za-z0-9_-]+before any disk lookup..name+ array-of-string.depends_on→ unique non-empty names → no dangling references → no cycles (Kahn-style pass in bash).phase,depends_on,trust,last artifact) plus an inline SVG DAG laid out by computed depth..nanostack/config.jsonforstack default), then one entry per phase artifact.Path safety
visual/beforenano_visual_assert_safe_descendruns (a symlinkedvisual/stackorvisual/journalexits 4 with no directory leak at the symlink target).data-testidmarkers — never a raw jq crash.--strictenforced BEFORE--manifest-onlyearly exit sojournal --strict --manifest-onlycannot ship a"strict": truemanifest with tampered sources.Codex review trail (18 passes)
--datefilterbin/lib/phases.sh;_journal_latest_on_dateJOURNAL_DATE/STACK_NAME; rm scratch in all exits; switch to jqnode_count + 1^[A-Za-z0-9_-]+$; unique-name check|| echoon every readname == "default"falls backstack:<name>source for failure modes.nanostack/config.jsonforstack defaultTest counts
ci/e2e-visual-artifacts.shci/check-visual-artifact-templates.shTest plan
ci/e2e-visual-artifacts.sh266/266ci/check-visual-artifact-templates.sh31/31