feat(audit): portal inspection UI — drawer, compare page, walkthrough docs#12
Merged
Conversation
Closes the remaining subtasks of issue #8. Drawer: 4-tab side panel (Overview / Request / Response / Notifications) opened by clicking any audit row; deep-linkable via ?id=, Esc/backdrop close, role=dialog/aria-modal for assistive tech, Replay button posts to the existing replay endpoint with banner output, Compare button stashes the event id in localStorage so two rows can be opened side-by-side. Notifications tab count appends '+' when the captured list was truncated. Compare page: /portal/audit/compare?a=&b= renders a JSON-path-aware structural diff. Walks objects/arrays by key/index so reordered keys don't show as changes; type-vs-tree mismatches collapse to a single diff at the path. One-side-undefined trees still show per-key only-A or only-B leaves rather than a single root-level "(undefined) -> {...}" line. Indentation comes from each nested ul's own padding so deep trees don't double-indent off-panel. Audit page: live-tail toggle subscribes to the SSE stream into a 20-event buffer above the table (the table stays a historical filter view to avoid refetch storms under load). JSONB filter editor produces the parseQueryFilter syntax (?param.<dotted>=v / ?response.<dotted>=v / ?header.<name>=v / ?has=<col>); compare-stash bar exposes the stashed event id with a one-click "compare with selected". API client: streamAuditEvents uses fetch + ReadableStream rather than EventSource so the X-API-Key header is carried; SSE framer follows spec on leading-space stripping, surfaces 401 to the unauthorized handler without flashing a per-stream error banner, caps the line buffer at 1 MiB so a misbehaving producer can't OOM the tab, drops events without an id at parse time, and reports server-close so the operator can re-enable. Docs: docs/operations/inspection.md walks the full workflow end-to-end, cross-referenced against the actual replayBurst / replayRefill / maxExportEvents constants. Per-identity rate-limit scope and the current export-truncation behavior are both called out explicitly. Pre-commit adversarial review: 4 review rounds across two phases (initial 3 rounds during development plus the gate's own 2 rounds at commit-time). Findings F1-F19 plus regression N1 all resolved before the first commit.
Fixes the 4 MAJORs and 6 Notable MINORs surfaced by the post-push critical review. Server (Go): - Header redaction at the source: auth.WithHeaders now stashes a RedactHeaders-cloned copy of the inbound HTTP headers, so credential- bearing names (Authorization, Proxy-Authorization, Cookie, Set-Cookie, X-API-Key in any case) land in audit_payloads.request_headers as "[redacted]" rather than verbatim. Pre-existing leak (the doc comment claimed redaction; the implementation didn't); PR #12 first put those bytes in front of UI users so the fix lands here. - Replay rate-limit ordering: pkg/httpsrv/portal_api.auditReplay now consumes a token only after all four validation checks pass (event exists, payload captured, no redacted params, tool registered). An operator clicking Replay on five summary-only rows in a row no longer loses their burst budget. Auth check still runs first so unauthenticated callers can't fan out reads. - Filter contract endpoint: GET /api/v1/portal/audit/meta returns {has_keys, json_sources, replay, export} so a UI can build its filter editor against the server's source of truth instead of duplicating allow-lists. AllowedHasKeysList / AllowedJSONSourcesList are derived from the existing exported vars (single source of truth); TestAllowList_FunctionAndSliceAgree extended to enforce symmetry and slice-isolation. UI (TypeScript): - Drawer header on error: spinner during loading, AlertCircle on detail-fetch failure, title reads "Failed to load event" rather than "Loading...". - Replay confirmation: the Replay button now opens a ConfirmModal that calls out the side-effect re-run; default focus is on Cancel so a reflexive Enter dismisses rather than fires. Esc cancels via capture-phase handler so it doesn't bubble to the drawer's own Esc. - Replay client-side preflight: button is disabled with a tooltip reason when the row has no captured payload or any param is redacted. Mirrors the server's hasRedactedParam check via a small hasRedactedValue helper. - Stale-replay guard: the mutation now takes the event id as a variable; ReplayBanner is only rendered when replay.variables matches the open drawer, so a navigate-away-mid-flight no longer shows the prior result against the new event. - Replay reset race: skip replay.reset() while replay.isPending so switching events mid-flight doesn't clobber the in-flight UI state. - Drawer focus management: save the previously-focused element on open, focus the close button, restore on unmount. - Filter editor sources its has-keys list from /audit/meta via TanStack Query (cached, no retry); HAS_KEYS_FALLBACK kept for offline-first rendering. - Compare stash cleared on signOut and on 401: extracted COMPARE_KEY to ui/src/lib/storage-keys.ts; auth.signOut and the 401 handler call clearSessionScopedState so a stashed event id doesn't survive the session. Docs: - inspection.md: header-redaction policy added to the Request-tab section; replay-button confirmation behavior and "tokens consumed only after validation" called out; "ring buffer" replaced with "fixed-cap most-recent-first list (cap 20)". - http-api.md: new /audit/meta row; replay row updated to mention the post-validation token-consumption contract. Pre-commit gate: 2 review rounds. Round 1 caught the original 11 findings + 1 regression (ConfirmModal capture-phase Enter would have fired Confirm regardless of focus); fixed by removing the Enter handler entirely so native button activation handles the Cancel default. Round 2 verified all fixes, surfaced one asymmetric-test gap on AllowedJSONSourcesList mutation isolation; fixed in-tree. make verify + make codeql + pnpm tsc + pnpm build all green.
…oad missing
Three bugs surfaced by the first end-to-end `make dev` against a fresh
local stack.
Makefile dev-* targets:
docker compose interpolates ${MCPTEST_COOKIE_SECRET:?required} at
parse time on every invocation (up, exec, down, logs), and Make
recipes run in fresh subshells so the env state from a prior target
doesn't carry over. Every dev-* target that touches
docker-compose.dev.yml now declares dev-secrets as a prereq and
sources .env.dev inline before invoking compose. Affected: dev-up,
dev-anon, dev-wait, dev-down, dev-logs.
Also: `make verify` now writes the diff hash to
.claude/.last-verify-passed so the pre-commit review-gate hook can
confirm verify is green for the exact tree state being committed.
Empty audit log crashed the portal:
Go marshals a nil []audit.Event as JSON null. The dashboard handler
returned `recent: null` on a fresh DB and the SPA's
`recent.map(...)` threw, hitting the ErrorBoundary with "Cannot read
properties of null (reading 'map')". Fixed at the audit-store layer
so every consumer benefits: Postgres Store.Query / TimeSeries /
Breakdown and MemoryLogger.Breakdown initialize with `[]T{}`
instead of `var out []T` so empty results marshal as `[]`. Audit
page and Dashboard get `?? []` belt-and-braces. New
TestMemoryLogger_EmptyResultsAreNotNil enforces the invariant.
Try-It rows had no captured payload:
pkg/httpsrv/admin_api.recordTryitAudit built the audit Event with
WithParameters but never attached an audit.Payload sibling. Result:
rows tagged source=portal-tryit had `payload=null`, and the audit
drawer's Response/Notifications tabs correctly reported "No
response captured" — making it look like capture was disabled.
Fix mirrors recordReplayAudit: build *audit.Payload with
JSONRPCMethod / RequestRemoteAddr / sanitized RequestParams /
callToolResultToMap(res) for ResponseResult / ResponseError when
callErr or IsError. errCategory precedence aligned with
pkg/mcpmw/audit.go (auth -> tool -> handler) so Try-It rows filter
the same as native MCP rows. New TestAdminAPI_TryitCapturesPayload
asserts the payload lands.
Pre-commit gate: 1 round, 1 latent MAJOR (auditTimeseries /
auditBreakdown carry the same nil-slice bug for endpoints not yet
wired to the SPA). Fixed at the store layer in this commit so future
SPA work doesn't trip on it. make verify + UI build green.
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.
Closes the remaining three subtasks of #8. Branch follows the focused-PR sequence (PR #9 / #10 / #11 shipped the backend; this is the frontend + walkthrough doc landing the UX layer on top of it).
Summary
/portal/auditto open a side drawer with Overview / Request / Response / Notifications tabs. Deep-linked via?id=<event-id>so URLs are shareable. Replay + Compare buttons inline. Esc/backdrop close.role=dialog+aria-modalfor assistive tech./portal/audit/compare?a=<id>&b=<id>renders a JSON-path-aware structural diff. Walks objects and arrays by key/index so reordered keys don't masquerade as changes; one-side-undefined trees still show per-key only-A / only-B leaves; deep trees indent linearly (one level of padding each nested ul, no compounding off-panel)./portal/auditsubscribes to the SSE stream into a 20-event buffer above the table. The table itself stays a historical filter view; the buffer is the live read. Refetch-per-event would have stormed/audit/eventson busy deployments.parseQueryFiltersyntax already shipped in PR feat(audit): JSONB path filters and NDJSON export (#8) #10:?param.<dotted>=v/?response.<dotted>=v/?header.<name>=v/?has=<col>. Source dropdown switch resets path/value so a staleparam.user.iddoesn't get pushed ashas=user.id.docs/operations/inspection.mdis the operator-facing end-to-end: capture a call → open the drawer → read each tab → replay → compare → filter → live-tail → export. Cross-referenced against the actualreplayBurst/replayRefill/maxExportEventsconstants inpkg/httpsrv.Files
ui/src/components/EventDrawer.tsx(new)ui/src/pages/Compare.tsx(new)ui/src/components/JsonView.tsx(new)docs/operations/inspection.md(new)ui/src/lib/api.ts(mod)ui/src/pages/Audit.tsx(mod)ui/src/main.tsx(mod)Total: 1316 insertions, 19 deletions. Zero Go changes — UI + docs only.
Backend touchpoints (read-only)
GET /api/v1/portal/audit/events(existing) — list filterGET /api/v1/portal/audit/events/{id}(existing) — drawer detail fetchPOST /api/v1/portal/audit/events/{id}/replay(PR feat(audit): replay endpoint and SSE live tail (#8) #11) — drawer Replay buttonGET /api/v1/portal/audit/stream(PR feat(audit): replay endpoint and SSE live tail (#8) #11) — live-tail SSEGET /api/v1/portal/audit/export?format=jsonl(PR feat(audit): JSONB path filters and NDJSON export (#8) #10) — referenced from inspection.mdSSE client design
streamAuditEventsinui/src/lib/api.tsusesfetch+ReadableStreamrather thanEventSourcebecauseEventSourcecannot send custom headers — cookie-only auth would lock out CLI/API-key callers. The frame parser:data:/event:lines (per W3C SSE);:-prefixed) used for the opening: connectedand 30s: keepalive;idat parse time;clearApiKey()+onUnauthorized()and returns silently (no per-stream error banner racing the redirect);done: true) throws"stream closed by server"so the operator can re-enable.Comparison diff design
buildDiffTreeinui/src/pages/Compare.tsxwalks two values structurally:(obj, obj)→ recursive per-key tree, sorted-key union, only-A / only-B markers per missing key.(arr, arr)→ recursive per-index tree.(undefined, obj)or(obj, undefined)→ treats the missing side as{}so the result is a per-key only-B (or only-A) tree, not a single root-level "(undefined) → {…}" leaf.JSON.stringify(JSONB columns are already canonicalized at storage).PayloadSectionrenders one of three shapes per payload field:DiffTreeView.diffCount > 0→ single-line(root)before/after.diffCount === 0→ italic "No differences."DiffTreeViewputs indentation on each nested<ul>'s ownpl-3rather than cumulativepaddingLeft: indent*12on each<li>, so deep trees don't double-indent off-panel.Pre-commit adversarial-review gate
Per the gate installed before PR #11 (
~/.claude/hooks/review-gate.sh), this branch landed on the first commit with no review-fix follow-ups. Two phases:data:line trimming (F5), filter source-switch path reset (F6), missing-payload hint (F7), plus N1 regression on equal-empty objects, plus F11/F12 doc accuracy on rate limit and export silent truncation.n.params?.messagerender on Compare (React-child crash on object value), per-key tree omission for(undefined, obj), cumulativeDiffTreeViewindentation, missingrole=dialog/aria-modal, dead comment block, 401 banner-flash race, missing truncation marker on the notifications tab count, unbounded SSE line buffer, and missing event-id guard.Verdict: CLEAN. Artifact:
.claude/.last-review.md(gitignored).Test plan
make verify✅ (already run locally; 80.2% filtered coverage gate).pnpm build✅ (already run locally; tsc + vite clean)./portal/audit, click a row, verify all 4 tabs render correctly for a captured tool call.portal-replayrow appears and the banner links to it.param.user.id=alice, verify the resulting?param.user.id=alicequery string narrows the table./portal/audit?id=<id>and confirm the drawer auto-opens to that event./portal/audit/compare?a=<id>&b=<id>directly and confirm two events render side-by-side.response_error: {}, open Compare, verify the "Response error" panel reads "No differences." (not "(root): (undefined)").Out of scope (deferred follow-ups)
storageevent (single-tab assumption is fine for now).