feat(serve,ui): what's-new popup with --no-banner opt-out - #663
Conversation
Features like the command palette were shipping undiscovered. The web UI now shows a curated per-version highlights modal on load, once per version, and `kbagent serve --no-banner` turns the unsolicited popup off. Frontend: - web/frontend/src/whatsnew.ts is the curated list — a hand-maintained WhatsNewRelease[], deliberately NOT the raw changelog. Release PRs that ship user-visible UI features add an entry here. A version with no entry shows no popup, which is the intended default rather than a bug. Seeded with 0.89.0's UI highlights, command palette first. - components/WhatsNew.tsx renders it: portaled, z-[55] (above drawers, below the palette), one subtle entrance transition. Esc, backdrop and "got it" all dismiss and persist. Dismissal is the release version in localStorage `kbagent.whatsnew.seen`; a PEP 440 pre-release suffix is stripped before matching so 0.90.0b1 sees the 0.90.0 reel. - The palette gains a "What's new" action that reopens it on demand, bypassing both the seen marker and the operator's flag — that flag governs what appears uninvited, not what the user asks for by name. Server: - `serve --no-banner` -> `create_app(ui_banner=False)` -> app.state, read back by the new `GET /ui-config`. The SPA fails CLOSED: no popup while that request is in flight, on error, or on anything but `banner: true`. The switch is an endpoint, not an injection into index.html, for two reasons. There is no injection point to extend — the one that existed (window.__KBAGENT_TOKEN) was removed in favour of the session cookie and test_serve_ui.py asserts it stays gone. And injection would cover only `GET /` and `GET /index.html`, while the StaticFiles html=True fallback serves the same shell for any unmatched path; that copy would carry no config and would silently re-enable the popup an operator had suppressed. For a suppression flag, failing open is the wrong direction. Also fixes a stale line in create_app's docstring that still described the removed token injection — the sentence that makes the injection point look like it still exists. Docs use the vNEXT placeholder per the #648 process; no version bump and no changelog entry.
| export const WHATS_NEW: WhatsNewRelease[] = [ | ||
| { | ||
| version: "0.89.0", | ||
| items: [ | ||
| { | ||
| title: "Command palette", | ||
| body: | ||
| "One keystroke to jump to any page, switch the active project, toggle the theme " + | ||
| "or open the Swagger docs. It resolves locally, so the list never waits on a request.", | ||
| hint: "ctrl+k / ⌘k", | ||
| }, | ||
| { | ||
| title: "Tokens page", | ||
| body: | ||
| "Create, rotate and revoke scoped Storage tokens without the web UI. The secret is " + | ||
| "shown once at mint; the opt-in \"derive last-used\" pass sorts dormant tokens first, " + | ||
| "so reading order is cleanup order.", | ||
| }, | ||
| { | ||
| title: "Trash & restore for configs", | ||
| body: | ||
| "Deleting a configuration is soft. The Configs page has a Trash tab listing what was " + | ||
| "deleted, with per-row restore -- no more digging through the API to undo a mistake.", | ||
| }, | ||
| { | ||
| title: "Re-run and terminate jobs", | ||
| body: | ||
| "Both actions are available straight from the Jobs table and from the job drawer. " + | ||
| "A re-run preserves the job's original branch, so a dev-branch job never silently " + | ||
| "re-fires against production.", | ||
| }, | ||
| { | ||
| title: "Editable column descriptions + table layout", | ||
| body: | ||
| "Click any description in a table's Schema tab to edit it in place -- written through " + | ||
| "the native endpoint the UI, the MCP server and the warehouse all read. The Info tab " + | ||
| "now also shows BigQuery partitioning and clustering.", | ||
| }, | ||
| { | ||
| title: "PAYG credits tile", | ||
| body: | ||
| "The Dashboard shows remaining credits and minutes for the active project, so you " + | ||
| "notice a draining balance before a job queue does.", | ||
| }, | ||
| { | ||
| title: "Flow notifications", | ||
| body: | ||
| "Every flow gets a read-only Notifications tab listing who actually gets paged, " + | ||
| "including the project-wide catch-all subscriptions that fire for every job.", | ||
| }, | ||
| ], | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🔍 Seeded 0.89.0 entry never auto-displays
whatsNewFor matches the running version exactly after stripping a pre-release suffix (web/frontend/src/whatsnew.ts:97-100). The only seeded entry is keyed 0.89.0, so any user on a later release (which is what ships this feature) gets no auto-popup. This matches the documented workflow where the release PR adds an entry keyed by the exact shipped version; the seed acts as a template. The feature stays dark until that entry is added.
Was this helpful? React with 👍 or 👎 to provide feedback.
Exact version matching made the feature ship dark. This popup first runs in the release AFTER the one whose highlights seeded the list — the only seeded entry is 0.89.0, which is already released, so no user could ever have both the code and a matching entry. The reel would have stayed invisible until a release PR happened to add one. `whatsNewFor` now returns the newest entry at or below the running version. That degrades correctly in every direction: a user who skipped a release still gets the most recent reel instead of nothing, and once a release PR adds an entry for the version actually shipping, that entry wins immediately. The seen-marker still caps each reel at one showing. Version comparison is numeric per segment, not lexicographic — 0.100.0 must sort above 0.92.0. Found by Devin Review on #663.
|
Good catch — this one is worth acting on rather than filing under "works as documented". Fixed in The finding is right that the workflow was documented, but the consequence was worse than "the seed acts as a template": the 0.89.0 reel could never have displayed for anybody. 0.89.0 is already released, so no user can have both this popup code and a matching entry. The feature would have shipped dark and stayed dark until a release PR happened to add an entry — which is a poor failure mode for a feature whose entire purpose is discovery.
Two details worth flagging:
Updated the file-header comment in
|
padak
left a comment
There was a problem hiding this comment.
Review of #663 — feat(serve,ui): what's-new popup with --no-banner opt-out
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR adds a curated, per-version "What's new" highlights popup to the web UI
(web/frontend/src/whatsnew.ts + WhatsNew.tsx), a new GET /ui-config endpoint
that surfaces a kbagent serve --no-banner opt-out flag, and updates the four
hand-maintained doc surfaces (CLAUDE.md, commands-reference.md, context.py,
docs/web-server.md) with the (since vNEXT) placeholder per the #648
version-bump-deferral process. I verified the auth-gating, the fail-closed
contract, the version-matching logic, and the design-language consistency by
running the code, not just reading it. Everything checks out cleanly — APPROVE.
No blocking findings; three non-blocking items worth the author's attention
(a missing gotchas.md entry, a "What's new" naming collision with the
pre-existing post-update CLI banner, and the project's total absence of
frontend unit tests for the new pure version-matching functions) and one nit.
Verdict
- Verdict: APPROVE
- Blocking findings: 0
- Non-blocking findings: 3
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] plugins/kbagent/skills/kbagent/references/gotchas.md — no entry for the new Web UI popup / --no-banner
CONTRIBUTING.md's per-command checklist calls for a gotchas.md entry when a
command's behavior is "non-obvious." This PR's mechanics are genuinely
non-obvious in a couple of ways an AI agent operating kbagent serve for an
operator might need: (1) --no-banner only suppresses the unsolicited popup
— the command palette's "What's new" action still opens it on request, and (2)
the fail-closed contract means a network hiccup or slow /ui-config fetch
silently suppresses the popup rather than erroring. There is direct precedent
for tracking web-UI-facing changes here (## Web UI Kai Chat is gone — replaced by Local AI (since v0.41.9) at gotchas.md:913). This is judgment,
not CI-enforced (scripts/check_command_sync.py explicitly excludes gotchas
tagging from its checks), and the feature is UI-cosmetic rather than a CLI
command trap, so I'm not calling it BLOCKING — but it's worth a short entry
tagged (since vNEXT).
[NB-2] src/keboola_agent_cli/auto_update.py:339-384 vs web/frontend/src/whatsnew.ts:1-20 — two unrelated "What's new" features now share a name
The repo already ships a "What's new" feature: format_whats_new()
(changelog.py, wired through auto_update.py:339,349,383-384) prints a
changelog diff to the terminal after a successful kbagent update
re-exec (also referenced in README.md:195 and docs/guide.md:35). This PR
adds a second, entirely independent "What's new" — a curated web UI
popup sourced from a hand-maintained whatsnew.ts reel, gated by
GET /ui-config / --no-banner. I confirmed the two do not actually
interact (--no-banner only threads into create_app(ui_banner=...),
never touches auto_update.py), so there's no functional bug — but the
identical name for two independently-triggered, independently-sourced
mechanisms is a foot-gun for a future maintainer or AI agent asked to "turn
off the what's-new banner" (which one?) or to "update the what's-new
content" (which file — changelog.py or whatsnew.ts?). A one-line
disambiguation in docs/web-server.md's new "What's-new popup" section
(e.g. "distinct from the terminal banner kbagent update prints") would
close the ambiguity cheaply.
[NB-3] web/frontend/src/whatsnew.ts:711-748 — no unit test for compareVersions / whatsNewFor
The focus of this review specifically asked me to stress the version-matching
logic, and there's already direct evidence it's worth testing: the second
commit (538df46) fixes a real bug in this exact function found by Devin
Review during this PR's own cycle (exact-match made the feature ship dark).
compareVersions/whatsNewFor are pure, easily-unit-testable functions with
exactly the kind of edge cases (PEP 440 suffix stripping, 0.100.0 vs
0.92.0 numeric-not-lexicographic ordering, fallback-to-newest-below)
that regress silently in a UI with npm run build as the only frontend gate.
I confirmed web/frontend has zero test files repo-wide
(vitest run → "No test files found") despite a configured npm run test
script, so this is a pre-existing, project-wide gap rather than something
unique to this PR — I'm not treating it as this PR's fault, but it's the
first PR where a real bug in exactly this kind of function was already
caught once, which is a good argument for seeding the frontend test harness
here.
Nits
[NIT-1]src/keboola_agent_cli/commands/serve.py:187,283,290,310— the
word "banner" now means two different things in the same function: the new
--no-banner/ui_banner(What's-new popup) switch, and the pre-existing
local variablebanner(the ASCII-art terminal startup banner text printed
a few lines later). Not a bug — just a readability trap for a future diff
in this function. Renaming the local var (e.g.startup_banner) would
remove the ambiguity.
Verification log
gh pr view 663 --json title,body,files,...→ 13 files, +535/-9,feat(serve,ui):prefix matches the mixed CLI+UI scope ✓- Isolated detached worktree created at PR head (
538df46) via
git worktree add --detach ... FETCH_HEAD; confirmed the invoking
worktree'sHEAD(claude/keboola-cli-issues-pr-review-eb4380) never moved ✓ - Read CONTRIBUTING.md §"Checklist: Adding a New CLI Command", §"Plugin
synchronization map", §"Releasing a new version"; CLAUDE.md convention #17
and## All CLI Commands;keboola-expert.md§1 and §3 — no §2 matrix row
needed (no new write/destructive command group, just a flag on existing
serve) ✓ gh issue view 648→ confirmed the(since vNEXT)placeholder process this
PR follows (no version bump, nochangelog.pyentry for feature PRs) ✓uv sync --extra server && make check(isolated worktree) → 6014 passed,
12 skipped, exit 0. This target chains lint, format-check, typecheck,
skill-check, version-check, version-gate-check, command-sync-check,
changelog-check, check-error-codes, check-sentinel-guards, loc-check, test
— all green ✓uv run pytest tests/test_serve_ui.py -v -k UiConfigBanner→ 5/5 new tests
pass individually ✓cd web/frontend && npm ci && npx tsc --noEmit && npm run build→ clean,
0 TS errors, build succeeds (pre-existing >500kB chunk-size warning is
unrelated to this diff) ✓npm run test(vitest) → "No test files found" — confirmed pre-existing,
repo-wide, not introduced by this PR (see NB-3)- Live
kbagent serve --port 18663 --no-banner(isolated config dir) +
curl /ui-config: noAuthorizationheader → 401; with
Authorization: Bearer <token>→{"banner":false}✓ (confirms auth-gating,
no unauthenticated leak, matchestest_ui_config_requires_auth) - Live
kbagent serve --port 18664(no flag) +curl /ui-configwith auth →
{"banner":true}✓ (confirms default-enabled) grep PUBLIC_PATHS src/keboola_agent_cli/server/auth.py→
{"/docs","/redoc","/openapi.json","/health/ping"}—/ui-configis NOT in
it, so it goes through the same bearer-auth gate as/version//doctor✓uv run kbagent context | grep -A2 ui-config→ renders literal
{"banner": bool}correctly (confirms the{{...}}f-string escape fix the
PR description calls out actually works, not just claimed) ✓grep -rn "z-\[50\]\|z-\[55\]\|z-\[60\]" web/frontend/src→ confirmed
Drawer.tsx/ConfirmModal.tsx/ManageTokenModal.tsxusez-50,
WhatsNew.tsxusesz-[55],CommandPalette.tsxusesz-[60]— the
layering claim in the PR description and the code comment is accurate ✓grep nerd-card nerd-btn nerd-pill-green web/frontend/src/index.css→
all three classes pre-exist in the shared design system;WhatsNew.tsx
extends the existing NERD UI, no parallel design language introduced ✓- Traced
VersionResp/UiConfigRespshapes inWhatsNew.tsxagainst
services/version_service.py:517-522({"kbagent": {"version": ...}}) and
server/routers/health.py:41-65({"banner": bool}) — both match exactly ✓ - Checked
web/frontend/src/layout/StatusBar.tsx:10-11— confirms the
queryKey: ["version"]cache-sharing claim inWhatsNew.tsx's comment is
accurate (same key, same query) ✓ git show 538df46— confirmed the second commit is a real, already-landed
fix (Devin Review caught the exact-match-ships-dark bug pre-merge; the
version-matching logic reviewed above is the POST-fix state) ✓- Manually re-derived the GitHub anchor slug for
### What's-new popup *(since vNEXT)*→#whats-new-popup-since-vnext,
matches the cross-reference link in the same file ✓ grep OPERATION_REGISTRY / router 1:1— not applicable: no new CLI leaf
command was added (--no-banneris a flag on the pre-existing, already
"admin"-registeredservecommand), so nopermissions.pyor
server/routers/*.pychange was owed ✓
Open questions for the author
(none)
* chore(release): 0.90.0 Bumps pyproject.toml to 0.90.0 and adds the changelog entry covering every PR merged since v0.89.0 (#658, #662, #661, #663, #665, #666, #664, #668, #667, #623), resolves the vNEXT placeholders those PRs left behind, and adds the curated What's new reel for the release. * docs(web-server): keep the What's-new anchor stable across releases The '### What's-new popup *(since vNEXT)*' heading put the version gate in the heading itself, so resolving the placeholder to 0.90.0 changed the generated slug to 'whats-new-popup-since-0900' and broke the in-page link at line 138 -- and would have broken it again on every future release. Moved the '(since 0.90.0)' tag to the first body line: the anchor is now the stable 'whats-new-popup', the gate stays visible, and check_version_gates.py still sees it (it scans the whole file, not just headings).
Features like the Ctrl+K command palette were shipping undiscovered. The web UI now surfaces a curated per-version highlights modal on load — once per version — and operators who don't want it get a flag.
Mechanism
Curated list —
web/frontend/src/whatsnew.tsThis is the file release PRs need to update. A hand-maintained
WhatsNewRelease[], deliberately not the rawchangelog.pyoutput: the changelog is the complete record and the Changelog page renders it; this is a short reel of the UI-visible things worth pointing at. Adding a release is one array element:Keyed by the exact
pyproject.tomlversion, newest first. A version with no entry shows no popup at all — intended default, not a bug: a release that only touches CLI internals has nothing to interrupt anyone about, and an empty modal is worse than silence. Seeded here with one0.89.0entry covering #658's UI highlights, command palette first.A PEP 440 pre-release suffix is stripped from the running version before matching, so
0.90.0b1sees the0.90.0reel.Popup —
web/frontend/src/components/WhatsNew.tsxThree independent gates decide whether the unsolicited popup appears: the operator has not disabled it, a curated entry exists for the running version, and the seen-marker isn't already that version.
localStorage["kbagent.whatsnew.seen"], value = last dismissed release version string. Both read and write are try/catch-wrapped (Safari private mode throws).document.bodyatz-[55]— above drawers (z-50), below the command palette (z-[60]). One subtle entrance transition, no keyframes added toindex.css. It never blocks the app.Server —
--no-bannerserve --no-banner→create_app(ui_banner=False)→app.state.ui_banner→ read back by the newGET /ui-config→{"banner": bool}. The SPA fails closed: no popup while that request is in flight, if it errors, or on anything butbanner: true. A release-notes modal is never important enough to appear against an operator who asked for it to be off.app.state.ui_banneris set unconditionally, not only underui_dist— the SPA also runs against a barekbagent servethrough the Vite dev server / Node BFF, where no UI is mounted.Why an endpoint and not an injection into
index.htmlThe brief suggested injecting a config object "the same way
--uiinjects the bearer token". That injection point no longer exists, and re-creating it would be wrong twice over:window.__KBAGENT_TOKENinjection was deliberately removed in favour of the HttpOnly session cookie (_install_ui's docstring records why: the injected token landed in the XSS-readable JS heap).tests/test_serve_ui.pyactively asserts"__KBAGENT_TOKEN" not in body. Nothing is injected into the shell today.GET /andGET /index.html. The SPA shell is also served by the StaticFileshtml=Truefallback for any unmatched path, and that copy would carry no config — so a deep link would silently re-enable the very popup the operator suppressed. For a suppression flag, failing open is the wrong direction.Reading from
app.stateis correct however the shell was served, and matches how the SPA already gets/versionand/doctor.test_banner_flag_not_injected_into_htmlpins the "still nothing in the HTML" half of that decision.Drive-by:
create_app's docstring still described the removed token injection. That stale sentence is what makes the injection point look like it still exists, so it's corrected here.Tests
tests/test_serve_ui.pygainsTestUiConfigBanner(5 cases, mirroring the existing cookie-injection tests): default enabled, disabled when the flag is set, available without a UI mount, not injected into the HTML, and auth-gated like/version.Process
Per #648: no version bump, no
changelog.pyentry. Docs use the(since vNEXT)placeholder for the release PR to resolve —CLAUDE.mdserve line,commands-reference.mdserve entry,docs/web-server.md(Web UI list + a new "What's-new popup" concept section), andcontext.py'sAGENT_CONTEXTserve entry.Gates
cd web/frontend && npm ci && npx tsc --noEmit && npm run build— clean.uv sync --extra server && make check— clean (6014 passed, 12 skipped).One thing
make checkcaught worth noting:AGENT_CONTEXTis an f-string, so the literal{"banner": bool}in the new serve docs parsed as a format field and raised at import time —kbagentwould not have started at all. Escaped to{{…}}and verified viakbagent contextthat it renders as single braces.