fix(plugins): graceful settings fallback via safeParseSettings + graph hierarchical layout - #937
Merged
Conversation
…h hierarchical Closes #917. ## Bug fix (root) `app/src/plugins/graph/settings.ts:layout` now accepts `"hierarchical"` — the type was already in `component/src/charts/graph-chart.tsx:25` but the Zod enum was missing it, causing widgets to crash on previously-saved hierarchical-layout configs. ## Resilience pattern (cross-cutting) New helper `app/src/lib/plugin/safe-parse-settings.ts`: - Wraps `schema.safeParse` with a fallback to schema defaults - Logs a structured warning via `console.warn` on failure (browser-safe; pino is server-only — bundling it into plugin components blows up webpack with `node:crypto` unhandled scheme) - Re-throws only when the schema ITSELF is broken (schema.parse({}) fails) ## Adoption (mechanical, all 20 plugins) Every plugin component migrated from: const settings = <X>SettingsSchema.parse(raw); to: const settings = safeParseSettings(<X>SettingsSchema, raw, "<plugin-id>"); Includes `single-value` which had a manual safeParse fallback — replaced with the helper for consistency + logging. ## Schema audit Cross-checked Zod enums in all 20 plugin settings against chart-side TS types where the chart exports a named union. Only one drift found: graph layout (the root finding). Other plugins don't export named unions, so the safeParse helper provides defense-in-depth. ## Tests - 9 helper unit tests cover: success, failure with defaults, structured log payload, passthrough preservation, undefined/null, missing fields, broken-schema propagation, pluginId in payload - 2843/2843 total tests pass (+9 new) - Build + type-check green ## Out of scope (per drill) - UI badge on widget header when fallback fires (silent log decided) - Compile-time `satisfies` enforcement of schema ⊆ chart-type (deferred; filed as a possible follow-up if drift recurs) Drill brief: claude_code_docs/plans/issue-917.md Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alfredo1996
added
bug
8 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
WalkthroughThis PR introduces a safe settings parsing helper and applies it uniformly across all plugin components. On invalid settings, plugins now log a structured warning and fall back to defaults instead of crashing. The graph schema is extended to support hierarchical layout, addressing a schema-feature mismatch. ChangesSafe parsing and plugin adoption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ugins Adds a single parameterized render test that exercises every plugin's component with deliberately-invalid settings, covering the safeParseSettings call site in each of the 20 plugin component files. Why: SonarCloud new_coverage gate failed on #937 — the 20 mechanical 1-line plugin migrations counted as "new code" with no direct coverage. Plugin components don't have unit tests by convention (they're covered via E2E), but the gate doesn't know that. This test lifts new_coverage above the 80% threshold by exercising each plugin's component once. Each test: - Renders plugin.component with garbage settings via @testing-library/react - Asserts no throw (proves safeParseSettings caught the validation failure and returned defaults instead of crashing) Heavy deps stubbed: @neoboard/components widgets, next/dynamic, the heavier internal components that use TanStack Query (table-renderer, form-widget- renderer, graph-exploration-wrapper). 21 new tests pass (20 plugins + 1 sanity check on the list). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
Fixed SonarCloud coverage gate (commit 6d51192)Initial CI run was ✅ on tests/E2E/build but SonarCloud quality gate failed on Fix: added a single parameterized smoke test ( 21 new tests pass (20 plugins + 1 sanity check). Total: 2864/2864 tests pass. CI re-running on 6d51192. |
|
5 tasks
alfredo1996
added a commit
that referenced
this pull request
Jun 5, 2026
…, #898, #899) (#939) * chore(.claude): polish skills, agents, CLAUDE.md for release/1.1 Findings from pre-polish review (umbrella #895): - code: add E2E to after-coding; release/1.1 branch awareness - next: auto-detect release/X.Y as base branch (instead of hard-coded dev) - github-workflow: fix frontmatter name mismatch (was 'github'); expand label list to match repo - issue: expand label list to match real GH labels (a11y, design, devex, etc.) - code-reviewer: add E2E test step (was unit-only) - test-runner: sharpen Docker conditional; clarify destroy-before-E2E rule - ux-crawler + user-sim-creator: replace ghost user 'bob@example.com' with seeded 'creator@neoboard.local' (pending #921) - CLAUDE.md: /github -> /github-workflow skill ref; clarify code-reviewer test scope - NEW: skills/deploy — production-readiness audit skill (capture-don't-fix, 5 sections, destructive-step approval gate) Pre-polish review issues filed: #921, #922, #923, #924, #925. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * security(auth): gate SSO settings page + API by enterprise edition Resolves the gap where the SSO management UI rendered fully on community installs. The existing inline NEOBOARD_EDITION check on /api/sso-providers was kept (already returned 403 forbidden) — this PR replaces it with the canonical requireFeature("sso") guard (returns 402 ENTERPRISE_REQUIRED) and adds defense-in-depth + a client-side gate so the UI itself never renders on community. Changes: - /api/sso-providers (admin CRUD): replace inline edition check with requireFeature("sso"); now returns 402 ENTERPRISE_REQUIRED instead of 403 - /api/auth/sso-providers (public login route): short-circuit to empty response on community before any DB read (defense-in-depth — even stale rows or env-provider misconfig can't leak) - New useFeatures()/useFeature() hook: TanStack Query, 5-min staleTime, reads /api/features - New <FeatureGate feature="..."> component for declarative client gating - New <EnterpriseRequiredEmptyState feature="..."> reusable empty state with auto-generated copy per feature + upgrade CTA - Settings layout: filter Authentication tab by sso feature; hidden on community (avoids dead-end and UI flicker during initial load) - Settings/authentication page: wrap content in FeatureGate; community users see the EnterpriseRequiredEmptyState directly Tests: - 49 unit tests pass (6 new useFeatures + 6 FeatureGate + 1 new community- edition page test + updated route tests + all pre-existing tests) - New E2E spec app/e2e/sso-gating.spec.ts covers community-mode UI + API gating - Enterprise-mode E2E coverage filed as #933 (requires second Playwright worker — substantial global-setup overhaul, out of scope) Drill brief: claude_code_docs/plans/issue-906.md Closes #906 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auth): address CodeRabbit feedback on #934 Three fixes from the CodeRabbit review: 1. POST + PATCH beforeEach in route.test.ts now pin NEOBOARD_EDITION=enterprise (deterministic across ambient envs; previously could flip to 402 if community env leaked in) 2. Added 402 ENTERPRISE_REQUIRED contract tests for POST, DELETE, PATCH — previously only GET covered the new contract; now full CRUD surface is locked 3. sso-gating.spec.ts: /api/sso-providers test uses page.request.get(...) instead of the global request context. Fixes the CI ECONNRESET seen on E2E shard 4/5 (global request raced with server startup) and matches the authenticated journey set up in the describe block's beforeEach All 28 route tests pass. Local E2E deferred per user direction (Docker teardown cost outweighs benefit for this small fix-up; CI will verify). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(auth): mark FeatureGate + EnterpriseRequiredEmptyState props as readonly Addresses two SonarCloud findings on #934 (rule typescript:S6759, MINOR): - app/src/components/feature-gate.tsx:34 - app/src/components/enterprise-required-empty-state.tsx:78 Per the codebase pattern (e.g. save-template-dialog.tsx, dashboard-picker-dialog.tsx), mark each prop interface field with the `readonly` modifier. 6 feature-gate tests pass. Local E2E deferred to CI (no functional change). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(import): unify NeoBoard + NeoDash flow with mapping UI + notes Closes #916. Unblocks #915. ## Server (app/src/app/api/dashboards/import/route.ts) - Accept `connectionMapping` + `skippedConnections` for BOTH formats - NeoDash imports now honor a `neodash-default` placeholder mapping - Cross-tenant safety: mapping validation now scoped to (userId, tenantId) - Response envelope adds `notes: string[]` (additive — existing clients reading only `id` continue to work) - Notes thread through from the converter (chart-type downgrades) plus new import-time notes (skipped connections, unmapped widget counts) ## Converter (app/src/lib/dashboard/neodash-converter.ts) - `convertNeoDash(json, defaultConnectionId?)` — accepts a connection id to stamp on every widget. Falls back to "" (legacy behavior) when omitted. - `convertNeoDashWithNotes` mirrors the signature ## Dialog (app/src/app/(dashboard)/page.tsx) - NeoDash imports synthesize a single placeholder `neodash-default` with type `neo4j` and surface it in the mapping UI (no longer silently uses empty connectionId) - New "Skip" checkbox per mapping row — widgets using a skipped key import with connectionId="" and a note in the result - Empty-targets UX: when no compatible connection exists for the placeholder type, the select is disabled and a helper line offers "Create one" (opens /connections in a new tab) or "Skip" - Post-success view replaces the form: dashboard name + notes list + "Stay here" / "View dashboard" buttons (no longer auto-redirects so users can read import notes carefully) ## Hook (app/src/hooks/use-dashboards.ts) - `ImportDashboardResult extends DashboardDetail` adds `notes: string[]` - `ImportDashboardInput` accepts optional `skippedConnections` ## Tests (app/src/app/api/dashboards/import/__tests__/route.test.ts) - 5 new contract tests: - notes envelope is always present - NeoDash with mapped connection - NeoDash with skipped placeholder → warning note - NeoBoard with skipped → unmapped-widget note - cross-tenant mapping rejected (400) - All 54 unit tests pass ## Out of scope (filed for follow-up) - Inline "Create new connection" affordance inside the import dialog — current "open /connections in new tab" is the fallback flagged in the drill - NeoDash converter content fidelity (params, markdown body) — that's #915 which now has the notes infra it needs to land cleanly ## Drill brief claude_code_docs/plans/issue-916.md Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): update import E2E specs for new post-success view + NeoDash mapping The PR #935 dialog redesign changed two behaviors that the existing import E2E tests assumed: 1. No auto-redirect after Import — dialog now shows a notes summary with View/Stay buttons; tests must click "View dashboard" before waitForURL 2. NeoDash imports now show a synthesized Neo4j placeholder mapping row instead of skipping the mapping step entirely Fixes 4 failing E2E tests on shards 2/5 and 3/5: - dashboard-portability.spec.ts:52 — NeoBoard format import - dashboard-portability.spec.ts:141 — NeoDash chart-type mapping - dashboard-portability.spec.ts:189 — NeoDash unsupported-type fallback - import-validation.spec.ts:124 — multi-connection mapping happy path For the NeoDash tests, the new placeholder is skipped via the Skip checkbox (these tests assert chart-type behavior, not connection wiring — widgets render regardless of connection presence). Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(import): drop dashboard title from NeoDash placeholder name The synthesized placeholder name was "Neo4j connection (<title>)", which duplicated the dashboard title that's already shown above in the parsed- preview box. In strict-mode E2E selectors this caused dashboard-portability spec line 145 to fail: dialog.getByText("E2E NeoDash Import Test") resolved to 2 elements (the preview header AND the placeholder row). Placeholder name is now just "Neo4j connection" — semantically just as clear (user sees the type "neo4j" beneath it) and avoids the collision. Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(import): preserve NeoDash markdown + auto-generate parameter widgets Closes #915. Three NeoDash converter bugs fixed in one place. ## Bug A — top-level params dropped NeoDash stores dashboard-wide params in `nd.settings.parameters`; NeoBoard has no global params (they're outputs of parameter-select widgets). The converter now: 1. Regex-scans every converted widget query for `$param_<name>` references 2. For each defined param that's referenced → creates a parameter-select widget with inferred type + default value on a NEW "Filters" page (prepended as page 1) 3. For each defined param that's NOT referenced → skip + note 4. For each referenced-but-undefined param → create with no default + warn Type inference: array→multi-select, finite number→number-range, empty string→text, otherwise→select (NeoDash's most common case). Strips the legacy "neodash_" prefix from param names so the generated widget produces `$param_<name>` matching what queries reference (paired with `convertParamSyntax` which already rewrites `$neodash_X` → `$param_X` in queries before scanning). Filter widgets tile 4-per-row at w=3 h=2, connectionId="" (no data). ## Bug B — markdown content dropped NeoDash stored markdown body in `report.query`. Markdown widget reads from `settings.content`. Converter now: - Routes `report.query` into `settings.content` when chartType is markdown - Clears widget.query (markdown is content-only — no query path needed) - Emits per-widget note: 'Imported markdown content for "<title>"' ## Bug C — silent failure mode Uses the existing notes infrastructure from #916 / PR #935 to surface every conversion decision. Notes per the drill (#915 brief): per-param explicit notes so user knows exactly what happened. Acceptable verbosity trade-off — terse summary alternative was considered and rejected. ## Tests 30 new pure-function unit tests cover: - isNeoDashFormat (4) - inferParameterType — all branches (6) - extractParamReferences — happy + edges (5) - Markdown content routing (4) - Filters-page generation (8) - defaultConnectionId behavior (3) Plus all 54 existing tests across the converter / route / dashboard suite continue to pass. Build + type-check green. ## Out of scope (per drill) - Reference detection beyond queries (titles, click-action params, styling rules) — first pass scans queries only - Auto-wiring seed queries for select-typed params — user configures in the editor - Markdown that contains `$param_*` substitutions — NeoDash didn't do inline substitution; literal copy Drill brief: claude_code_docs/plans/issue-915.md Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(import): address CR + Sonar findings on #936 - Extract buildFiltersPage() helper (Sonar S3776: cognitive complexity 20 → 15) - Drop unnecessary type assertion on nd.settings?.parameters (Sonar S4325) - number-range rangeMin = min(default, 0) — supports negative defaults (CR) - Update test fixtures to use $neodash_* syntax so tests exercise the full conversion path instead of bypassing it (CR — 4 tests) - Add original-widget query-rewrite assertion (CR nitpick) - Add explicit negative-default test for rangeMin widening - Fix 2 pre-existing tests at app/src/lib/__tests__/dashboard/ to expect the Filters page at pages[0] (original page now at pages[1] when params are referenced) Local: 2834/2834 tests pass; build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(plugins): graceful settings fallback via safeParseSettings + graph hierarchical Closes #917. ## Bug fix (root) `app/src/plugins/graph/settings.ts:layout` now accepts `"hierarchical"` — the type was already in `component/src/charts/graph-chart.tsx:25` but the Zod enum was missing it, causing widgets to crash on previously-saved hierarchical-layout configs. ## Resilience pattern (cross-cutting) New helper `app/src/lib/plugin/safe-parse-settings.ts`: - Wraps `schema.safeParse` with a fallback to schema defaults - Logs a structured warning via `console.warn` on failure (browser-safe; pino is server-only — bundling it into plugin components blows up webpack with `node:crypto` unhandled scheme) - Re-throws only when the schema ITSELF is broken (schema.parse({}) fails) ## Adoption (mechanical, all 20 plugins) Every plugin component migrated from: const settings = <X>SettingsSchema.parse(raw); to: const settings = safeParseSettings(<X>SettingsSchema, raw, "<plugin-id>"); Includes `single-value` which had a manual safeParse fallback — replaced with the helper for consistency + logging. ## Schema audit Cross-checked Zod enums in all 20 plugin settings against chart-side TS types where the chart exports a named union. Only one drift found: graph layout (the root finding). Other plugins don't export named unions, so the safeParse helper provides defense-in-depth. ## Tests - 9 helper unit tests cover: success, failure with defaults, structured log payload, passthrough preservation, undefined/null, missing fields, broken-schema propagation, pluginId in payload - 2843/2843 total tests pass (+9 new) - Build + type-check green ## Out of scope (per drill) - UI badge on widget header when fallback fires (silent log decided) - Compile-time `satisfies` enforcement of schema ⊆ chart-type (deferred; filed as a possible follow-up if drift recurs) Drill brief: claude_code_docs/plans/issue-917.md Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(plugins): smoke test safeParseSettings adoption across all 20 plugins Adds a single parameterized render test that exercises every plugin's component with deliberately-invalid settings, covering the safeParseSettings call site in each of the 20 plugin component files. Why: SonarCloud new_coverage gate failed on #937 — the 20 mechanical 1-line plugin migrations counted as "new code" with no direct coverage. Plugin components don't have unit tests by convention (they're covered via E2E), but the gate doesn't know that. This test lifts new_coverage above the 80% threshold by exercising each plugin's component once. Each test: - Renders plugin.component with garbage settings via @testing-library/react - Asserts no throw (proves safeParseSettings caught the validation failure and returned defaults instead of crashing) Heavy deps stubbed: @neoboard/components widgets, next/dynamic, the heavier internal components that use TanStack Query (table-renderer, form-widget- renderer, graph-exploration-wrapper). 21 new tests pass (20 plugins + 1 sanity check on the list). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dashboard): suppress self-save 'updated by' banner on revisit Closes #904. Final Phase 2 PR. ## Root cause `app/src/app/(dashboard)/[id]/page.tsx` (lines 148-160) uses sessionStorage to baseline the dashboard version, then fires a "Dashboard updated by X" banner whenever the refetched server version exceeds the stored baseline. That comparison fires on every SELF-save: after a successful PUT, the server bumps version N → N+1; the refetch sees N+1; sessionStorage still says N; banner fires with the user's own name. Then on revisit the banner triggers again or stays stale. ## Fix Update `useUpdateDashboard.onSuccess` to write the new version to sessionStorage BEFORE invalidating the cache. TanStack Query guarantees onSuccess runs before invalidateQueries' refetch lands, so the baseline is in place by the time the detail page's effect reads it. Other-user saves still trigger the banner correctly — they don't run through this user's mutation onSuccess. ## Defense-in-depth via updatedBy === userId (NOT in this PR) Considered during drill but rejected: would require exposing `dashboard.updatedBy` (user UUID) in the API response, which isn't there today. The primary fix solves the actual race; defense-in-depth is unnecessary for the realistic threat model. ## Tests - Unit (4 new cases on `useUpdateDashboard`): - PUT call shape (mutationFn) - onSuccess writes new version to sessionStorage - onSuccess skips write when result has no version field - onSuccess skips write when version is non-numeric - E2E (`dashboard-states.spec.ts`): full flow — create dashboard, save, navigate away, navigate back, assert no "Dashboard updated by" banner 2868/2868 unit tests pass; build green. Drill brief: claude_code_docs/plans/issue-904.md Local E2E deferred to CI per session pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): fix dashboard-states #904 test — Back goes to view mode, not list CI E2E shard 2/5 failed because the test expected `page.waitForURL(/\/dashboards/)` after clicking "Back", but Back actually navigates to /<id> (view mode), not the dashboards list. View mode is where the version-bump effect runs anyway, so the simpler flow exercises the bug directly: 1. Create dashboard → edit mode (version=1) 2. Save → server bumps to version=2; onSuccess writes 2 to sessionStorage 3. Click Back → /<id> view mode 4. View page's effect: refetch sees version=2, sessionStorage says 2 → NO banner Also added per-test unique dashboard name (timestamp suffix) and cleanup at the end to avoid polluting later tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(devex): fail-fast HMAC, seed-from-host fix, dev DNS warning Closes #907 — promote API_KEY_HMAC_SECRET from optional to required so the app refuses to start without it. Previously API key creation would surface a cryptic runtime error; now we fail fast at startup like ENCRYPTION_KEY and NEXTAUTH_SECRET. CLI's `neoboard env init` now generates this secret alongside ENCRYPTION_KEY so a fresh setup is still one command. Closes #898 — hardcode `localhost` in scripts/seed-demo.mjs. Previously the script honoured NEO4J_HOST/PG_HOST env vars; when seeding ran inside the docker-app container, those resolved to container names (e.g. `db`) which the host-side dev server then couldn't reach. Docker compose publishes the ports to localhost anyway, so the host-form is correct everywhere. Closes #899 — add a dev-only DNS-resolution check that warns about seeded connections whose URIs point to unreachable hosts. Fire-and-forget so startup never blocks; falls back to a no-op outside development. Warns once per affected connection with a concrete fix hint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): type warn mock so app tsc accepts the assignment CI's TypeScript type-check rejected `vi.fn()` assigned to a `(message: string) => void` slot. Use the typed `vi.fn<T>()` overload so the mock satisfies the callable signature while still exposing `.mock`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(dev): redact URI in DNS warn, add tenant scope to diagnostic query Two CodeRabbit findings on the new dev-only DNS checker: - The warning included the full decrypted URI, which can be `scheme://user:password@host/...` — that violates the repo rule "NEVER log decrypted credentials." Print only the parsed hostname. - The diagnostic query selected from `connections` without a tenant filter, violating the multi-tenancy rule that every DB query include one. Scope to `process.env.TENANT_ID ?? "default"`. Test gains a credential-leak guard that fails if any URI-embedded username/password reaches the warn sink. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: alfredorubin96 <alfredo.rubin@neotechnology.com> Co-authored-by: Claude Opus 4.7 (1M context) <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
2 participants
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
Fixes #917 — three things in one PR:
layout: "hierarchical"because the Zod schema only allowed"force" | "circular". Adding the missing value.safeParseSettings()helper migrates all 20 plugin components fromschema.parse(raw)(throws → blank widget) to safe-parse-with-defaults (logs → degraded render).Drill brief:
claude_code_docs/plans/issue-917.md. Closes #917.What changed
New helper —
app/src/lib/plugin/safe-parse-settings.tsschema.safeParse(raw)console.warnwith{ pluginId, issues }payload, then returnsschema.parse({})(defaults)node:cryptounhandled scheme). The helper falls back toconsole.warnwith a structured payload so operators / devs can still spot drift in the browser console.default()value violates the schema) — surfaces to the error boundary instead of silently mis-rendering.Mechanical migration (all 20 plugins)
Every plugin component changed from:
to:
single-valuehad a manual safeParse fallback (no logging) — replaced with the helper for consistency + structured log on failure.Graph schema fix
app/src/plugins/graph/settings.ts:Brings the schema in sync with
component/src/charts/graph-chart.tsx:25(GraphLayout = "force" | "circular" | "hierarchical") and the exploration wrapper which already accepted all three.Schema enum audit
Cross-checked Zod enums in all 20 plugin settings against chart-side TS types where the chart exports a named union:
layouthierarchicalorientation("vertical"/"horizontal")BarOrientationstackMode("none"/"stacked"/"percent")BarStackModeFuture drift is caught at render time by the helper. If it becomes a recurring problem, a follow-up could add a compile-time
satisfiescheck.Tests
9 new unit tests in
app/src/lib/plugin/__tests__/safe-parse-settings.test.ts:.passthrough()2843/2843 total tests pass (was 2834, +9 from new helper file). Build + type-check green.
The helper test that uses the same
z.enum(["force", "circular"]).default("force")schema with input{ layout: "hierarchical" }doubles as a regression test for the original bug.Decisions locked from drill
safeParse→ defaults on failure (whole-object replace)console.warnwith structured payload (pino is server-only — would break browser bundle)Phase 2 sequence
This is PR #4 of 5 in the security & data-loss sweep:
Risk
console.warnmakes failures visible in DevTools; helper tests assert the log fires on failure.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests
Updates