fix(dashboard): surface error states + safeParseJSON helper#34
Merged
Conversation
Adds apps/dashboard/src/lib/safeJson.ts with a generic safeParseJSON<T> helper that returns a typed fallback on empty/null/invalid JSON input and emits console.warn (with context label and 80-char snippet) only in DEV mode. Includes 10 vitest unit tests covering all specified acceptance criteria. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… pass
Cross-source review (Claude A + Claude B + Codex) surfaced 7 real issues in
the prior 6 commits that wired safeParseJSON + setApiError into the dashboard.
All fixed:
- Sticky apiError: refresh()/refreshList()/handleDelete()/executeWorkflow()
now setApiError(null) at start so a transient blip doesn't pin the red
banner forever (mirrors BoardDetailPage.load() pattern).
- Empty-string payload regression: BoardDetailPage.runDecisions and
RunDetailPage.parsed now use {} as fallback (was null), preserving the
old `JSON.parse(s || '{}')` behavior. Decision card and run map entries
no longer silently vanish on empty payloads.
- (error as Error).message unsafe cast: WorkflowsDashboard delete/execute
handlers now use `error instanceof Error ? error.message : String(error)`.
No more "Failed to delete workflow: undefined" on non-Error throws.
- Token/PII leak via console.warn: safeJson dev-mode warning now logs
structural metadata only (input length), never raw content. Sentry RUM
/ DevTools history can no longer capture API keys from malformed payloads.
- _setDevForTesting in production bundle: now a no-op constant in prod
builds via import.meta.env.DEV ternary so the override can't be flipped
by injected code at runtime.
- AgentsPanel.parseMetadata NPE risk: guard against valid JSON parsing to
primitives (`"null"`, `"false"`, etc.) so downstream `meta.x` access
doesn't crash if backend stores a non-object.
- Falsy-valid JSON pinning tests: add 4 tests for `"false"`, `"0"`, `"null"`,
`'""'` so a future refactor can't silently break round-tripping.
Tests: 36 pass (was 31). Typecheck clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Round-2 review (Codex + structured pass) caught 4 real issues including
a regression introduced by the round-1 fix-up. All addressed:
- BoardDetailPage runDecisions overwrite bug: the round-1 {} fallback
unconditionally wrote map[run_id]={}, which could overwrite a
previously-valid decision with an empty fallback when a later
FINAL_DECISION event for the same run was malformed. Now distinguishes
empty input (legitimate, write {}) from parse failure (skip, never
overwrite).
- RunDetailPage parsed memo: same distinction. Empty payload renders
the degraded card; malformed payload is treated as null so the card
doesn't render with NaN% / undefined garbage.
- WorkflowsDashboard refreshList race: removed setApiError(null) from
refreshList because it's auto-fired from the mount useEffect alongside
ensureDefaultBoard, where a concurrent error could be clobbered by
the null reset. User-initiated handlers (handleDelete, executeWorkflow)
still pre-clear, which is the intended pattern.
- AgentsPanel parseMetadata: tightened guard so the typeof-object branch
also rejects arrays. Single guard now covers both branches.
- AgentsPanel modal handlers (5 sites): handleAddInternal, handleAddExternal,
handleAddHuman, saveEdit, handleDeleteParticipant — all silent empty
catches replaced with apiError plumbing matching the refresh() pattern.
- EventTimeline polling catch: matches sibling getRunIds pattern with
console.warn.
Tests: 36 pass. Typecheck clean.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
Net-positive incremental improvement to dashboard error handling. Replaces silent
JSON.parsefallbacks and silent network failures with explicit error surfacing in components that render untrusted server data.Changes:
apps/dashboard/src/lib/safeJson.ts(new) — genericsafeParseJSON<T>(input, fallback, context?)helper. WrapsJSON.parsein try/catch, returns fallback on failure, warns in DEV mode with input length only (no PII leak).apps/dashboard/src/lib/__tests__/safeJson.test.ts(new) — 15 unit tests covering valid/invalid/null/undefined input, DEV-only warning, no-leak guarantee.safeParseJSONintoEventTimeline,BoardDetailPage,RunDetailPage,AgentsPanel.setApiErrorintoWorkflowsDashboarduser-action paths so failures surface visibly instead of console-only.Test Coverage
pnpm --filter @consensus-tools/dashboard test— 36 tests pass (2 files, +15 new in safeJson).pnpm --filter @consensus-tools/dashboard typecheck— clean.Known limitations (addressed in follow-up)
This branch went through 3 rounds of adversarial review. Round 3 found 5 multi-source-confirmed issues with the ad-hoc shape-check approach (
typeof === 'object' && !Array.isArray(), empty-string detection, last-write-wins maps). Rather than chase regressions in a 4th round, the structural fix is being implemented as a separate PR onfix/dashboard-zod-trust-boundary— runtime schema validation via zod at the trust boundary, replacing scattered ad-hoc checks.This PR ships as net-positive over
main(silent failures and missing error surfaces are the worst state). The zod plan supersedes thesafeParseJSONhelper but reuses the test patterns andsetApiErrorwiring done here.Pre-Landing Review
Branch was reviewed 3 times via
/reviewadversarial pass on commit1a561cf. Round 3 identified the 5 issues (B/D/E/F + implicit C/J) now scoped into~/.gstack/projects/consensus-tools-toolkit/ceo-plans/dashboard-zod-trust-boundary.md.Test plan
🤖 Generated with Claude Code