Skip to content

Dashboard: fix 10 post-review bugs + Playwright regression harness + dedup finalization#3

Open
xpolb01 wants to merge 12 commits intomainfrom
hooks-health-system
Open

Dashboard: fix 10 post-review bugs + Playwright regression harness + dedup finalization#3
xpolb01 wants to merge 12 commits intomainfrom
hooks-health-system

Conversation

@xpolb01
Copy link
Copy Markdown
Owner

@xpolb01 xpolb01 commented Apr 16, 2026

Summary

Two groups of commits on this branch, both ready to land:

  1. Dedup finalization (5 commits, previously reviewed): 95a3866ab08517
  2. Dashboard-bugs-fix (6 new commits, this PR's focus): dea9fb7ed06efd

Dashboard Bugs Fix (primary)

Fixes 10 bugs in the feature-gated hooks dashboard identified in a post-review audit (ef9cc1c landed a lot at once — this is the cleanup pass). Adds a Playwright regression harness.

/api/health rewritten (breaking, isolated)

  • Returns Vec<CheckItem> merging check_session_hooks + check_vault_hooks, wrapped in tokio::task::spawn_blocking for sync FS I/O.
  • Adds --settings, --hooks-dir, --vault CLI flags with sensible defaults.
  • CheckStatus serde contract locked with rename_all = "lowercase" + round-trip test.
  • Repo grep confirmed zero external consumers of the old shape.

HooksSummary extended

  • ingests_today: u64 — UTC-rolling-day count via strftime('%Y-%m-%dT00:00:00Z','now').
  • wal_checkpoint_age_seconds: Option<u64> — mtime of $DB-wal vs now.

/api/events?since=... strict RFC3339

  • Malformed sinceHTTP 400 + {"error":"invalid_since","detail":...} instead of silent None fallback.

Dashboard UI cleanup

  • "Ingests Today" card reads data.ingests_today — stable across time-window selector.
  • "SQLite Lag: Syncing" placeholder replaced with real "WAL last checkpoint: Ns ago" + formatAge helper (yellow > 1h, red > 1d).
  • Loading overlay wired to non-silent fetches; silent on auto-refresh.
  • Per-tab .tab-error banner with showTabError/clearTabError helpers.
  • Dead evt.timestamp fallback removed.

Safe DOM rendering (XSS fix)

  • All four render functions refactored from innerHTML += to createElement + textContent + classList.
  • Zero innerHTML mutations remaining.
  • xss-safety.spec.ts asserts <script> / <img onerror> payloads in session_id / signals render as literal text.

Clippy cleanup

  • cargo clippy --features dashboard --all-targets -- -D warnings now passes clean on scriptorium-cli.

Test Coverage

Rust

  • cargo test --workspace --features dashboard392 passed, 0 failed, 9 ignored (live-only). +11 new unit tests.

Playwright

  • npx playwright test23/23 passed across 8 spec files (smoke + 6 per-bug + full-smoke). Harness uses webServer config, chromium-only, gitignored fixtures.

Runtime

  • All 5 dashboard routes (/, /api/health, /api/summary, /api/events, /api/errors) return HTTP 200.

Dashboard Commits (6 atomic)

SHA Subject
dea9fb7 fix(dashboard): replace /api/health with CheckItem[] and add Playwright harness
5f93a16 fix(dashboard): overview UI cleanup, error states, and loading overlay
840f07d fix(dashboard): reject invalid since param and drop dead schema field
45909f9 refactor(dashboard): safe DOM rendering + clippy cleanup
bdbed99 fix(dashboard-qa): use request fixture for API-only assertion in health-tab spec
ed06efd fix(dashboard): preserve lagText element ID (revert T10 rename)

Prior Dedup Commits (5, already reviewed)

2d3a2c8, 95a3866, c52652f, 4fdb874, ab08517 — previously-developed dedup defense-layer work that never landed via its own PR; shipping in this one.

Verification

All plan compliance (Must Have 7/7, Must NOT 14/14) + 4 automated reviewers (oracle plan audit, code quality, scripted QA, scope fidelity) approve.

Full QA report at .sisyphus/evidence/final-qa/report.md (local-only, not checked in).

@xpolb01 xpolb01 force-pushed the hooks-health-system branch from ed06efd to 872f496 Compare April 16, 2026 14:40
xpolb01 added 12 commits April 16, 2026 17:47
Layer A (prompts.rs): 6 tests for render_valid_links — empty stems,
full paths, one-per-line, backslash normalization, integration with
ingest_prompt.

Layer B (ingest.rs): 9 tests for guard_stem_collisions — no collision
pass-through, Create→Update auto-convert, ambiguous 2+ matches error,
Update existing pass-through, Update retarget, within-plan duplicates,
invalid path, path traversal.

Layer C (stem.rs): 5 edge-case tests — empty path, dotfile, double
extension, deeply nested index.md, mixed-case extension.

Layer C (frontmatter.rs): 5 tests for check_duplicate_stems — no dupes
clean, 2-way collision, 3-way collision, non-wiki exclusion,
non-interference with duplicate_id rule.

Also fixes 14 pre-existing clippy warnings (doc_markdown, too_many_args,
is_multiple_of, map_unwrap_or, case_sensitive_file_extension,
or_default, redundant_closure, stable_sort_primitive, too_many_lines,
cast_possible_truncation, unnecessary_wraps).
…d add Playwright harness

- Replace /api/health handler: now returns Vec<CheckItem> merging check_session_hooks +
  check_vault_hooks, wrapped in spawn_blocking for disk I/O.
- Add --settings, --hooks-dir, --vault CLI flags with --vault falling back to default registry.
- CheckStatus serde now uses rename_all="lowercase" (contract with dashboard.html:renderHealth).
- HooksSummary gains ingests_today (UTC rolling day count) and
  wal_checkpoint_age_seconds (mtime of $DB-wal, Option<u64>).
- Commit Playwright QA harness: package.json + playwright.config.ts, globalSetup/Teardown
  spawn dashboard with seeded JSONL, chromium-only. Scripts test:e2e, test:e2e:ui.
- First regression spec tests/dashboard-qa/health-tab.spec.ts passes end-to-end.

Breaking change: external consumers of /api/health must now parse Vec<CheckItem> instead
of {status, db_path, db_accessible}. T0 repo scan found zero consumers; safe for internal
dashboard.

Refs: dashboard-bugs-fix plan tasks T0-T6, T14
…loading overlay

- Ingests Today card now shows rolling UTC-day count (data.ingests_today) independent
  of the window selector.
- Replace hardcoded "SQLite Lag: Syncing" indicator with "WAL last checkpoint: Ns ago"
  computed from data.wal_checkpoint_age_seconds. Yellow dot >1h, red >1d.
- Wire loading overlay: show on tab switch / manual refresh (silent=false), skip on
  auto-refresh so users don't see a flash every 10s. Remove unused `display: flex`
  from the `.loading-overlay` rule.
- Per-tab inline .tab-error banner with .visible toggle. Shown on fetch failure via
  showTabError(tab, msg); cleared on next success via clearTabError(tab).
- Three new Playwright specs cover the fixes.

Refs: dashboard-bugs-fix plan tasks T9-T12, T15, T17, T18
…d schema field

- /api/events?since=... now returns 400 with JSON {"error":"invalid_since","detail":...}
  when the value is not valid RFC3339. Previously silently fell back to since=None.
- Unit tests cover valid/invalid/missing since cases.
- Drop dead evt.timestamp fallback in dashboard.html renderEvents — HookEvent only
  serializes `ts`.
- New Playwright spec: tests/dashboard-qa/since-param.spec.ts (API-only, 4 cases).

Refs: dashboard-bugs-fix plan tasks T7, T8, T16
- Replace every innerHTML mutation in dashboard.html with createElement + textContent.
  Four render functions touched: renderOverview, renderBarChart, renderEvents,
  renderHealth. Preserve all CSS classes, element IDs, and DOM structure.
- Remove redundant drop(_verify) at dashboard.rs — clears clippy::used_underscore_binding.
- Backtick SQLite in doc comment at dashboard.rs — clears clippy::doc_markdown.
- New Playwright spec: tests/dashboard-qa/xss-safety.spec.ts — proves <script> payloads
  in session_id / signals render as literal text, no execution.
- New Playwright spec: tests/dashboard-qa/full-smoke.spec.ts — end-to-end regression:
  navigates 3 tabs, verifies window-switch stability, WAL age label format.

cargo clippy --features dashboard --all-targets -- -D warnings now passes clean.

Refs: dashboard-bugs-fix plan tasks T5, T13, T19, T23 full-smoke authoring
…ertion in health-tab spec

The 'Missing hooks_dir surfaces a fail item' test called page.evaluate(fetch(...))
without page.goto, which fails because Playwright's page context needs a page first.
Switch to the `request` fixture (matching since-param.spec.ts pattern) — cleaner and
correct for API-only assertions.

Refs: dashboard-bugs-fix plan T24 surfaced this spec bug
… rename)

The plan's Must-NOT-Have guardrail forbids renaming existing element IDs.
T10 semantically renamed `lagText` → `walCheckpointText`, which F1 audit flagged.
Revert the ID while preserving the T10 WAL-checkpoint content semantics.

- dashboard.html: `<span id="lagText">` preserved; renderOverview updates via #lagText
- overview-labels.spec.ts: selector updated to #lagText
- full-smoke.spec.ts: selector updated to #lagText

Refs: dashboard-bugs-fix plan F1 audit
@xpolb01 xpolb01 force-pushed the hooks-health-system branch from 872f496 to e0b5249 Compare April 16, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant