Skip to content

feat(dashboard): warn on stale SPA bundle via build-id freshness check - #4913

Open
jstrunk wants to merge 1 commit into
kirodotdev:mainfrom
jstrunk:feat/dist-staleness-guard
Open

feat(dashboard): warn on stale SPA bundle via build-id freshness check#4913
jstrunk wants to merge 1 commit into
kirodotdev:mainfrom
jstrunk:feat/dist-staleness-guard

Conversation

@jstrunk

@jstrunk jstrunk commented Aug 21, 2026

Copy link
Copy Markdown

Supersedes #3396, which GitHub auto-closed when a squash force-push rewrote this cross-fork branch's history (the PR could not be reopened). Same single commit f893d06, same green CI, same content — re-filed as a fresh PR. The review discussion on #3396 remains there for history.

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 dist was 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 from stale_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 — new buildIdPlugin (registered next to swVersionPlugin) writes dist/build-id.json { buildId, commit, builtAt } at vite build time, reusing the exact ${pkg.version}-${gitShortSha} identity swVersionPlugin already computes. Staged into the package by the existing cp -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 build COMMIT into the generated _build_info.py (empty-safe; COMMIT imported separately so an older stamp without it can't unbind DISTRIBUTION). Falls back to git rev-parse HEAD in a source/dev checkout, resolved via trusted_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 → WARNING naming both build-ids with rebuild guidance. Every "cannot verify" case (missing build-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 via run_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:

  • Backend: 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_freshness never raises; _backend_commit prefers baked over git, resolves git only through trusted_system_bin, and handles git-unavailable / non-zero-exit / not-in-trusted-dirs (no spawn). test/test_beacon.py and test/test_stale_asset_watchdog.py also pass.
  • Spawn audit: test/test_spawn_audit.py::test_every_spawn_is_routed_or_allowlistedstale_bundle_guard.py::_backend_commit registered in BENIGN_SPAWNS (fixed git rev-parse HEAD list-argv, cwd is the module's own install dir, no agent input reaches command/args/cwd).
  • Static analysis (repo's blocking CI gates): mypy clean on the new module + beacon.py + gateway.py; isort --check-only and flake8 clean on all changed files.
  • Frontend: npm run typecheck (tsc -b) exits 0 with the new plugin.
  • End-to-end: a real vite build emits dist/build-id.json ({"buildId":"1.0.0-<sha7>","commit":"<full sha>","builtAt":"…"}); after staging via cp -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 staleness WARNING), 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

  • Single commit with a Conventional Commits title (feat: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — N/A; behavior is a startup log line, no docs surface
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

@jstrunk
jstrunk requested a review from a team as a code owner August 21, 2026 13:15
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 21, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) August 24, 2026 06:57
@iamwhatever iamwhatever added the needs-pr-triage PR scanner: awaiting automated triage label Aug 26, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 26, 2026
@bolichen97 bolichen97 added the needs-author-decision PR blocked on author input label Aug 26, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

🤖 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:

  • The substantive CI has not run. On head f893d06 the only checks present are the fork-guard/readiness signals (Fork workflow-change guard, Publish readiness signal) — all skipped/success. The backend/frontend test lanes, the five AI review lanes, and CodeQL/Semgrep have not executed, so the combined status is pending (total: 0) and the PR sits mergeable_state=blocked with readiness: checking.
  • This is the cross-fork approval gate: because the PR comes from jstrunk/KiroCrew, its gated workflow runs need a maintainer to approve them before CI can run. Driving fixes to the branch cannot clear this gate — there is no failing check or code defect to fix (the diff is clean; the git-rev-parse build-id probe is benign and fail-closed).

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 f893d06 are intact.)

Add pr-no-autofix to opt out of future automation.

@bolichen97 bolichen97 removed the needs-pr-triage PR scanner: awaiting automated triage label Aug 26, 2026
@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 3, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This 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

  • PR #2188 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #2188: CONTINUE_DEVELOPMENT. Different mechanisms and different files; the only shared file is a benign-spawn allowlist with disjoint entries. Worth noting to whoever lands both that cloud-shipped bundles will not be freshness-verifiable unless the commit is threaded into the isolated build. Files: src/kiro_crew/cloud/source.py.
  • PR #7916 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7916: REBASE. Adjacent, non-conflicting halves of the same problem (server-side advisory vs client-side recovery). Neither subsumes the other: 4913 cannot help a phone whose service worker is serving a days-old shell, and 7916's probe tells an operator nothing. Files: website/vite.config.ts.

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>
@bolichen97
bolichen97 force-pushed the feat/dist-staleness-guard branch from 97b50ff to 6fb23b4 Compare September 8, 2026 22:25
@bolichen97

Copy link
Copy Markdown
Collaborator

Rebased onto main 6c24f116e by a maintainer as part of the 2026-09-08 open-PR audit (branch was 2400+ commits behind, mergeable_state=dirty). New head: 6fb23b480.

Conflicts resolved (2, both mechanical):

  • src/kiro_crew/slack/gateway.py: kept main's multi-name stale_asset_watchdog import (now includes shutdown_exit_code) and re-added this PR's separate stale_bundle_guard import line.
  • website/vite.config.ts: took main's plugins array (it gained excalidrawFontsPlugin()) and re-inserted buildIdPlugin() right after swVersionPlugin().

Gates run locally on changed files: isort, flake8, pytest test/test_stale_bundle_guard.py test/test_spawn_audit.py (25 passed), plus a live import check confirming both shutdown_exit_code and check_bundle_freshness bind in gateway.py. black --check flags only pre-existing formatting in src/kiro_crew/beacon.py (a baselined file); untouched.

One note, not a rebase fix: main's swVersionPlugin now appends a -dev<ts> suffix on dirty trees, so buildIdPlugin's ${version}-${shortSha} is no longer the identical identity the docstring claims, and a dirty rebuild at the backend's own HEAD is a false negative.

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.

@github-actions github-actions Bot removed the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 6fb23b480bce88f69341665bdb8159d44675691a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff adds no dashboard UI — the only user-facing surfaces are a startup log WARNING and a stamp-script echo line; the rebuild guidance in the warning matches CONTRIBUTING.md's documented staging step (npm run build then copy into src/kiro_crew/static/dist), and the mismatch can only fire in a source checkout, exactly the audience that guidance fits. No JSX/TSX, no controls, no states, no persistent-element transitions — the lens-12 carve-out for control-free diffs applies, so there are no evidence gaps.

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

  • In the WARNING, "build 1.0.0-aaaaaaa (commit aaaaaaa)" repeats the short SHA the build-id already embeds; dropping the parenthetical (commit %s) when dist_build_id is present would tighten the line without losing information.

[UX-REVIEWED] 6fb23b4

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 6fb23b480bce88f69341665bdb8159d44675691a via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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.

Watch

In 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, _backend_commit() is git rev-parse HEAD, so every commit that touches no website/ file makes dist_commit != backend_commit and logs "Dashboard SPA bundle is STALE" against a byte-identical frontend. Most commits in this repo are backend-only, so the majority of warnings in the guard's target environment are false, and operators habituate to the exact signal this PR exists to create (the description's stated harm is "no signal"; a signal that usually cries wolf recreates it).
Clears when: the identity compares frontend content, not whole-tree HEAD — e.g. both writers stamp git rev-parse HEAD:website (the website tree hash), or the warning demotes to informational when the dist commit is an ancestor with no website/ changes since.

Suggestions

  • Don't await the executor call in startup — fire it as a background task like the adjacent vanish-watchdog, so a wedged dev-checkout git (up to the 5s timeout) cannot delay gateway boot for a purely advisory log line.

[DESIGN-REVIEWED] 6fb23b4

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 6fb23b480bce88f69341665bdb8159d44675691a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 6fb23b4

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 6fb23b480bce88f69341665bdb8159d44675691a via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The base tree contains code_fingerprint.py — a module whose stated job is "one token that names WHICH Kiro Crew code a process is running", with a hardened git probe (trusted_git_bin(), scrubbed GIT_* env, dirty-tree digest) — and swVersionPlugin already stamps a build identity into the dist with dirty-tree handling the new plugin lacks. Here is the review.

First-Principles-Verdict: CONCERNS

_backend_commit re-implements code_fingerprint's hardened git probe — weaker (sha-only, no dirty digest, PATH resolver that misses Windows) — beside the module built for exactly this.

Not justified as shipped

    1. Source-checkout git fallback — duplicate of src/kiro_crew/code_fingerprint.py:71 (_git_fingerprint): same git rev-parse HEAD, same 5s timeout, but resolved via trusted_system_bin("git") instead of trusted_git_bin() (platform_compat.py:1698, whose docstring binds "every caller that spawns git" and adds the Windows fallback — 6 existing callers counted), without _git_env() hygiene, plus a new BENIGN_SPAWNS entry the reuse would not need.
  • 2b. builtAt field — zero consumers (grepped builtAt repo-wide: only the writer and test fixtures).
    1. Sha-only identity is the level the repo already rejected: code_fingerprint's docstring names "two edits to the same HEAD read as the same code… in the exact setting (an editable install being hacked on) where it matters most", and swVersionPlugin suffixes dirty builds for the same reason — buildIdPlugin stamps clean HEAD regardless, so dirty dev builds (the guard's primary audience) compare falsely fresh. The description's "reusing the exact identity swVersionPlugin already computes" is contradicted by the diff: it recomputes it, minus the dirty suffix.

What this change ships

Inventory (8 items) — 5 justified

Intent: warn the operator at startup when the served dashboard bundle was built from a different commit than the running backend. ADDITION.

  1. Startup logs a WARNING when the served bundle's commit differs from the backend's — justified
  2. vite build now writes a second identity stamp, dist/build-id.json — duplicate of website/vite.config.ts (swVersionPlugin identity, recomputed without its dirty-tree suffix)
    2b. The stamp carries a builtAt timestamp — zero consumers
  3. Packaged installs bake the build commit into _build_info.py (COMMIT, beacon.baked_commit()) — justified
  4. In a source checkout the gateway spawns git rev-parse HEAD at boot — duplicate of src/kiro_crew/code_fingerprint.py (_git_fingerprint)
  5. Gateway boot now waits on the check (executor, capped, never raises) — justified
  6. stamp-distribution.sh prints the commit in its summary line — rides along
  7. Spawn-audit allowlist entry for the new probe — justified (mandated by test_every_spawn_is_routed_or_allowlisted)
  8. Both sides compare bare HEAD sha, ignoring dirty state — symptom-level (identity model the repo already fixed in code_fingerprint)

Watch

  • On Windows source checkouts the guard silently never runs: trusted_system_bin("git") never finds git there (the _WINDOWS_GIT_DIRS comment says so), which is the hole trusted_git_bin() exists to close. Clears when: the fallback resolves through trusted_git_bin().
  • Dirty dev builds — the staleness scenario's home turf — stamp HEAD's clean sha on both sides and compare fresh. Clears when: identity reuses code_fingerprint()'s <sha>[+digest] token (or the description states sha-only as the accepted level, matching Dashboard serves a stale SPA bundle silently — add a build-id freshness guard #3395's cause-level tracking).

Subtractions

  • Delete _backend_commit's subprocess block and the BENIGN_SPAWNS entry (test_spawn_audit.py) — call code_fingerprint._git_fingerprint(Path(__file__).parent) (code_fingerprint.py:71), which already pins git, scrubs GIT_*, and digests dirty trees.
  • Drop builtAt from buildIdPlugin's JSON — zero consumers.

[FIRST-PRINCIPLES-REVIEWED] 6fb23b4

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 6fb23b480bce88f69341665bdb8159d44675691a via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 6fb23b4

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) needs-author-decision PR blocked on author input readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants