fix: dashboard UI remaining TODOs + review-applied fixes #81
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/dashboard-ui-remaining-todos"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Finishes the remaining dashboard UI TODOs and folds in the fixes surfaced by
/review(multi-agent review of the branch). Two distinct bodies of work:1. Dashboard UI TODOs (pre-existing branch work):
status.jsmodule (124 lines, was inline inapp.js).panels.css(was inline instyles.css).dashboard-config.jscentralising polling constants (STATS_POLL_MS,HEALTH_POLL_MS,SERVER_HEALTH_POLL_MS).auth.rsSessionMetadata RBAC branch for the new metadata tool wiring.2. Review-applied fixes (5 commits from review team
dashboard-review-fixes):b569291get_sessionresponse now additively emits bothidandsession_id(production/mock parity, dashboard preferssession_id).4ce4c7dAbsent+no-bound; deny onAbsent+bound-but-no-role withsession_idaudit; malformed UUID returns JSON-RPC-32602invalid_paramsinstead of 403 (was info-leaky). 4 canonical reasons extracted aspub(super) const.c950102mcpCallLograce fixed via snapshot-and-compare; tightened permission-denied assertion to full canonical message + session header; armedwaitForRequestbefore improve auth-refresh assertion. Mockget_sessionmirrors prod's additive response.67435d0readPollingConfig()helper that copies into a local frozen object rather than freezing the caller-ownedwindow.__COGNIX_TEST_POLLING__);status.jsgets per-fnAbortControllerto cancel stale in-flight polls; first-render gate fixed so one side recovering re-renders;graph.jsaccepts{ shouldRender }to short-circuit before DOM mutation. New asset tests pin MIME/bytes and constants.2d14d72list_sessionssteal id=1). Fix: only passsignalwhen there's a prior controller to abort.22 of 29 review findings applied; 10 deferred (cosmetic/structural — listed in branch fix report).
Test Plan
cargo fmt --all -- --check— cleancargo clippy --workspace --features ui,sse --all-targets -- -D warnings -D clippy::unwrap_used -D clippy::panic -D clippy::dbg_macro -D clippy::print_stdout -D clippy::print_stderr -D clippy::wildcard_imports— cleancargo test -p cognix-mcp --lib— 282 passed (baseline 278; +4 new RBAC tests)cargo test -p cognix-server --lib— 327 passed (baseline 327)cargo test -p cognix-server --features ui,sse --test ui_tool_schema_shapes— 6 passedcargo test -p cognix-server --lib --features ui,sse— 410 passed, 8 failed (8 failures all pre-existing onc8cc3eccheckpoint; flagged for separate triage intransport::ui::tests/transport::sse::testsreturning HTTP 500)pnpm playwright test e2e/dashboard/— 39/39 passednode --checkon every modified assetSelf-Review Checklist
unwrap()in library codeprintln!/dbg!/print_stderrpermission_denied_response; new-32602path is for malformed input only)session_id/agent_idonly)cargo auditandcargo deny checkwill run in CIKnown follow-ups (not in this PR)
--features ui,ssetest failures intransport::ui::tests/transport::sse::tests(HTTP 500 from router — likely shared init issue). Verified pre-existing onc8cc3ec.app.js; 124-lineinitStatus()split) — tracked in branch fix report.Root cause: `withAbortSignal(P, signal)` in rpc.js (line 140) wraps the SSE handshake promise in a new Promise when a signal is passed, which adds an extra microtask hop between P resolving and `mcpCall` resuming. `sessions.refresh()` calls `mcpCall('list_sessions', {})` with no signal, so its `await` continues directly on P (1 microtask). The two new signal-wrapped status calls now lose the post-SSE microtask race and let list_sessions steal RPC id=1. Fix: pass `signal` to mcpCall only on subsequent ticks where there's an actual in-flight request to cancel. The first bootstrap tick of each refresh has no prior controller, so we omit the option — avoiding the withAbortSignal wrapper and preserving the dashboard.spec.ts:29 contract that get_health and get_server_health receive ids 1 and 2 in that order. Subsequent ticks still pass signal and still abort the prior in-flight fetch, so the brief's bounded-concurrency + silent-supersession semantics are unchanged. The first call simply can't be cancelled — an edge case that doesn't matter in practice since there's nothing to cancel yet. Test contract in ui_tool_schema_shapes.rs is updated to pin both the new call site (`mcpCall(...callOptions)`) AND the option-construction shape (the conditional ternary). Strict literal contract maintained; not loosened.