feat(dashboard): warn on stale SPA bundle via build-id freshness check - #4913
feat(dashboard): warn on stale SPA bundle via build-id freshness check#4913jstrunk wants to merge 1 commit into
Conversation
|
🤖 Kiro Crew [operator: bolichen97#66809557]: This PR has been inactive for 7+ days. I reviewed the blockers but they require a human action the automated pipeline cannot take:
Decision needed (maintainer): approve the pending workflow runs on this fork PR so CI executes, then the pipeline will re-assess on its next cycle. (Author: nothing to change on your side — the code and single commit Add |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
The gateway serves the dashboard SPA from a gitignored, build-copied
`src/kiro_crew/static/dist/`. Nothing verifies that the served bundle
was built from the same tree as the running backend, so a restart that
did not rebuild/restage the frontend keeps serving an OLD bundle
silently — the dashboard renders (assets are present), so the gap only
surfaces when a behavioral test fails.
This adds a startup, WARN-only freshness guard — the present-but-stale
counterpart to `stale_asset_watchdog.py`'s present-vs-vanished check. It
deliberately does NOT reuse the vanish watchdog's shutdown response: a
stale dist still serves a working (if outdated) dashboard, and a restart
alone re-serves the same dist, so warning (not shutting down) is the
correct response.
Mechanism (part of the dist-staleness guard; see issue):
- `website/vite.config.ts`: new `buildIdPlugin` writes
`dist/build-id.json` {buildId, commit, builtAt}, reusing the exact
`${pkg.version}-${gitShortSha}` identity `swVersionPlugin` computes.
Staged into the package by the existing `cp -R website/dist ...`.
- `scripts/stamp-distribution.sh` + `beacon.baked_commit()`: bake the
backend's build `COMMIT` into `_build_info.py` (empty-safe), with a
`git rev-parse HEAD` fallback for source/dev checkouts.
- `stale_bundle_guard.check_bundle_freshness()`: compares the two; a
confident commit mismatch → WARNING naming both build-ids with rebuild
guidance. Any unknown side (missing stamp, unknown backend commit)
skips silently so a transitional dist or git-less build never
false-warns. Best-effort; never raises.
- Wired once at gateway startup near the vanish-watchdog task.
Tested: `pytest test/test_stale_bundle_guard.py` (12 tests) +
`test/test_beacon.py` + `test/test_stale_asset_watchdog.py` all pass;
mypy/isort/flake8 clean on the changed backend files; frontend `tsc`
typechecks; a real `vite build` emits `dist/build-id.json` and the guard
verifies silent-on-match / warn-on-mismatch end to end. Warn-only: does
NOT shut the gateway down.
Drive-to-green (originally authored by Jeff Strunk): rebased onto main
(composed buildIdPlugin with main's new precompressPlugin in
website/vite.config.ts) and registered the fixed-argv
stale_bundle_guard._backend_commit startup probe in the spawn-audit
BENIGN_SPAWNS allowlist with justification.
Review round 2 (GPT): run check_bundle_freshness() on the subprocess
executor so the dev-checkout git fallback cannot block the event loop,
and hoist the beacon import in stale_bundle_guard.py to module scope.
Review round 3 (GPT): pin the git fallback through trusted_system_bin
(fixed system dirs, never PATH) so a shim planted in an agent-writable
PATH directory cannot execute with the gateway's environment; skip the
probe when no trusted git exists. Tests updated + new no-spawn test.
Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
97b50ff to
6fb23b4
Compare
|
Rebased onto main Conflicts resolved (2, both mechanical):
Gates run locally on changed files: isort, flake8, One note, not a rebase fix: main's Please review the resolution. A maintainer push makes the maintainer the last pusher, so a second approver is needed under the repo's last-push rule. Reply if anything looks wrong. |
UX Review (Fable 5, fork) — ✅ PASSUX-level review of The diff adds no dashboard UI — the only user-facing surfaces are a startup log UX-Verdict: PASS Log-line-only change: the warning asserts the exact state it holds (both commits), explains why restart won't fix it, and gives the documented repair command. Suggestions
[UX-REVIEWED] 6fb23b4 |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Commit-equality is a noisy staleness proxy: in the only environment where the guard can fire, backend-only commits produce false STALE warnings. WatchIn a packaged install both sides are stamped from the same tree at packaging, so they always match — the guard is live essentially only in source/dev checkouts. There, Suggestions
[DESIGN-REVIEWED] 6fb23b4 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All evidence gathered. The base tree contains First-Principles-Verdict: CONCERNS
Not justified as shipped
What this change shipsInventory (8 items) — 5 justifiedIntent: warn the operator at startup when the served dashboard bundle was built from a different commit than the running backend. ADDITION.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 6fb23b4 |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
Problem / Motivation
The gateway serves the dashboard SPA from a gitignored, build-copied
src/kiro_crew/static/dist/. Nothing verifies that the served bundle was built from the same tree as the running backend, so a restart that did not rebuild/restage the frontend keeps serving an OLD bundle silently. The dashboard renders (assets are present), so the gap only surfaces when a behavioral test fails against a stale UI. See #3395 for the full write-up.Why it matters
Silent staleness is the worst-case shape: no error, no missing asset, no crash — the UI just quietly serves outdated behavior, and the operator has no signal that a rebuild/restage was skipped. The existing vanish-watchdog can't catch it (the dist is present, just old). Left undone, developers and operators debug "fixed" behavior that never shipped to the served bundle, and lose time chasing a frontend fix that is actually a staging gap, not a code gap.
What changed (motivation → approach → change)
Goal: give the operator a startup signal when the served
distwas built from a different commit than the running backend. Approach: reuse the build-identity the frontend already stamps rather than invent a new one, and make the response warn-only — a stale dist still serves a working (if outdated) dashboard, and a restart alone re-serves the same dist, so a shutdown response would just loop. This is deliberately distinct fromstale_asset_watchdog.py, which is a presence check (_DIST_INDEX.is_file()) with a CRITICAL + graceful-shutdown response, correct for a vanished dist. Different failure mode, deliberately different response.What was built:
website/vite.config.ts— newbuildIdPlugin(registered next toswVersionPlugin) writesdist/build-id.json{ buildId, commit, builtAt }atvite buildtime, reusing the exact${pkg.version}-${gitShortSha}identityswVersionPluginalready computes. Staged into the package by the existingcp -R website/dist ...every packaging path runs. Same git-unavailable tolerance: no git →commit: ""→ backend skips.scripts/stamp-distribution.sh+beacon.baked_commit()— bake the backend's buildCOMMITinto the generated_build_info.py(empty-safe;COMMITimported separately so an older stamp without it can't unbindDISTRIBUTION). Falls back togit rev-parse HEADin a source/dev checkout, resolved viatrusted_system_bin("git")(fixed system dirs only, never PATH; skips with no spawn when unavailable).src/kiro_crew/dashboard/stale_bundle_guard.py(new) —check_bundle_freshness()compares the two commits. A confident mismatch →WARNINGnaming both build-ids with rebuild guidance. Every "cannot verify" case (missingbuild-id.json, unknown backend commit, git-less build, malformed JSON) skips silently so a transitional dist never false-warns. Best-effort; never raises, never shuts down. Dispatched off the event loop viarun_in_executor(subprocess_executor(), ...)at the startup call site so the dev-checkout git fallback cannot stall the loop.src/kiro_crew/slack/gateway.py— one call at startup, next to the vanish-watchdog wiring.Tests
All run locally and green — no output claimed that wasn't observed:
pytest test/test_stale_bundle_guard.py— mismatch warns; match / missing-stamp / unknown-backend-commit / empty-dist-commit / malformed-JSON all skip silently;check_bundle_freshnessnever raises;_backend_commitprefers baked over git, resolves git only throughtrusted_system_bin, and handles git-unavailable / non-zero-exit / not-in-trusted-dirs (no spawn).test/test_beacon.pyandtest/test_stale_asset_watchdog.pyalso pass.test/test_spawn_audit.py::test_every_spawn_is_routed_or_allowlisted—stale_bundle_guard.py::_backend_commitregistered inBENIGN_SPAWNS(fixedgit rev-parse HEADlist-argv, cwd is the module's own install dir, no agent input reaches command/args/cwd).mypyclean on the new module +beacon.py+gateway.py;isort --check-onlyandflake8clean on all changed files.npm run typecheck(tsc -b) exits 0 with the new plugin.vite buildemitsdist/build-id.json({"buildId":"1.0.0-<sha7>","commit":"<full sha>","builtAt":"…"}); after staging viacp -R, the guard is silent when the dist commit matches the backend, and WARNs (does not shut down) when the staged commit is rewritten to an older value.Build artifacts (
dist/,_build_info.py) remain gitignored and are not part of this diff.Manual verification
N/A — unit coverage plus the end-to-end
vite build+ stage + rewrite check above exercise every branch. The only user-visible surface is a single backend log line (the stalenessWARNING), asserted by the unit tests; there is no UI change to click through.Related Issues
Implements the runtime, warn-only approach from #3395. Open to the alternatives raised there (sw.js identity reuse, content-hash identity, CI arm, periodic re-check) before merge — the bundle-identity model is tracked at cause level in #3395.
Checklist
feat: ...)Contribution License Agreement