fix(kanban-simple): wrap JSON.parse calls with safe fallback#568
Open
data-bot-coasys wants to merge 1 commit intodevfrom
Open
fix(kanban-simple): wrap JSON.parse calls with safe fallback#568data-bot-coasys wants to merge 1 commit intodevfrom
data-bot-coasys wants to merge 1 commit intodevfrom
Conversation
When orderedTaskIds or orderedColumnIds contain literal:// URI strings (e.g. when tasks are created via MCP/agent tools), JSON.parse throws: SyntaxError: Unexpected token 'l', "literal://"... is not valid JSON Added safeJsonParse helper that catches parse errors and falls back to an empty array, preventing the view from crashing. Fixes all JSON.parse calls in Board.tsx and TaskSettings.tsx.
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis pull request adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx (1)
9-18: ExtractsafeJsonParseinto a shared utilityThis helper is duplicated in
TaskSettings.tsxandBoard.tsx. Centralizing it avoids behavioral drift and keeps warning/validation behavior consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx` around lines 9 - 18, Duplicate safeJsonParse implementation exists in TaskSettings.tsx and Board.tsx; extract it into a single shared utility module (e.g., utils/json.ts or similar), export the function as safeJsonParse<T>(value: string | undefined | null, fallback: T): T, then remove the local definitions in TaskSettings.tsx and Board.tsx and import the shared safeJsonParse where used. Ensure the exported function preserves the same behavior (including the console.warn message and slice call) and TypeScript generic signature so existing callers in TaskSettings and Board compile without changes.views/kanban-view-simple/src/components/Board/Board.tsx (1)
18-29: Prefer a shared parse helper instead of per-component copies
safeJsonParseis duplicated here and inTaskSettings.tsx. Move it to a shared utility module so fixes (like schema checks) are applied once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@views/kanban-view-simple/src/components/Board/Board.tsx` around lines 18 - 29, safeJsonParse is duplicated in Board.tsx and TaskSettings.tsx; extract it into a single shared utility (e.g., export function safeJsonParse<T>(...) in a new/shared utils module) and replace the local copies by importing that exported helper in both Board.tsx and TaskSettings.tsx, removing the duplicate definitions; ensure the helper is exported (named export) and update any call sites to use the imported safeJsonParse so future schema checks/fixes are centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@views/kanban-view-simple/src/components/Board/Board.tsx`:
- Around line 165-167: The code calls columns.find(...) and immediately
dereferences column.orderedTaskIds and column.delete(), which can crash if the
find returns undefined; update the deleteColumn logic to check the result of
columns.find(...) (the variable column) and bail out early (return or throw a
controlled error) if it's undefined, then use column.orderedTaskIds only after
that guard (you can still call safeJsonParse on a fallback value if preferred)
and only invoke column.delete() inside the guarded block; apply the same
presence check to the other occurrence that references column (around the
Task.findAll / column.delete usage).
- Around line 21-29: safeJsonParse<T> currently returns parsed JSON without
validating its shape, which lets valid-but-incorrect values (objects, numbers,
etc.) reach callers that immediately call .splice/.filter/.map in the board
drag-and-drop logic; update safeJsonParse to perform runtime shape checks before
returning: for the kanban use case add an overload or optional validator
parameter (or a boolean branch) that verifies Array.isArray(parsed) and that
each item is a string (e.g., typeof item === 'string') when callers expect
string[]; if validation fails, log a warning and return the provided fallback.
Ensure callers (the reorder/drag-and-drop functions that call
.splice/.filter/.map) keep using safeJsonParse<string[]>(...) so the validated
type is enforced at runtime.
In `@views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx`:
- Around line 73-75: The code assumes .find(...) returned objects (e.g., source,
destination, columnModel) and dereferences them immediately (e.g., using
source.orderedTaskIds and source.save(batchId)); add defensive null checks after
each .find call in TaskSettings.tsx and bail out or handle the missing object
(early return, user-facing error, or log and skip) before using properties or
calling methods; specifically guard the variables named source, destination, and
columnModel where they are used around the shown snippets so you never call
.orderedTaskIds, .save(...), or access columnModel properties on undefined.
- Around line 10-17: The safeJsonParse<T> helper should reject parsed non-array
values to avoid runtime crashes when callers expect string[]; modify
safeJsonParse to accept an optional type guard (validator: (v: unknown) => v is
T) and after JSON.parse run the validator and return fallback if it fails.
Update all callers that expect string[] (the safeJsonParse<string[]>() usages in
TaskSettings where the result is immediately spread or .filter() is called) to
pass Array.isArray as the validator (or a stricter element-type guard) so only
actual arrays are returned; keep the existing try/catch fallback behavior for
parse errors.
---
Nitpick comments:
In `@views/kanban-view-simple/src/components/Board/Board.tsx`:
- Around line 18-29: safeJsonParse is duplicated in Board.tsx and
TaskSettings.tsx; extract it into a single shared utility (e.g., export function
safeJsonParse<T>(...) in a new/shared utils module) and replace the local copies
by importing that exported helper in both Board.tsx and TaskSettings.tsx,
removing the duplicate definitions; ensure the helper is exported (named export)
and update any call sites to use the imported safeJsonParse so future schema
checks/fixes are centralized.
In `@views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx`:
- Around line 9-18: Duplicate safeJsonParse implementation exists in
TaskSettings.tsx and Board.tsx; extract it into a single shared utility module
(e.g., utils/json.ts or similar), export the function as safeJsonParse<T>(value:
string | undefined | null, fallback: T): T, then remove the local definitions in
TaskSettings.tsx and Board.tsx and import the shared safeJsonParse where used.
Ensure the exported function preserves the same behavior (including the
console.warn message and slice call) and TypeScript generic signature so
existing callers in TaskSettings and Board compile without changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51185060-167a-4a88-a16e-856301146232
📒 Files selected for processing (2)
views/kanban-view-simple/src/components/Board/Board.tsxviews/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx
Comment on lines
+21
to
+29
| function safeJsonParse<T>(value: string | undefined | null, fallback: T): T { | ||
| if (!value) return fallback; | ||
| try { | ||
| return JSON.parse(value); | ||
| } catch { | ||
| console.warn('Failed to parse JSON value, using fallback:', value?.slice(0, 80)); | ||
| return fallback; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "Board\.tsx|kanban-view" | head -20Repository: coasys/flux
Length of output: 1144
🏁 Script executed:
wc -l views/kanban-view-simple/src/components/Board/Board.tsxRepository: coasys/flux
Length of output: 114
🏁 Script executed:
cat -n views/kanban-view-simple/src/components/Board/Board.tsx | head -40Repository: coasys/flux
Length of output: 2066
🏁 Script executed:
cat -n views/kanban-view-simple/src/components/Board/Board.tsx | sed -n '115,130p;145,160p;158,172p;210,225p;238,255p;282,295p;313,325p;340,355p'Repository: coasys/flux
Length of output: 6141
🏁 Script executed:
cat -n views/kanban-view-simple/src/components/Board/Board.tsx | sed -n '200,260p'Repository: coasys/flux
Length of output: 3205
🏁 Script executed:
cat -n views/kanban-view-simple/src/components/Board/Board.tsx | sed -n '275,310p'Repository: coasys/flux
Length of output: 1803
Add runtime type validation to safeJsonParse<T> before returning parsed values
The function catches JSON parse errors but cannot prevent valid-but-wrong JSON (e.g., objects, numbers, or non-string arrays) from breaking array operations at call sites. This is especially critical in drag-and-drop reorder logic (lines 122, 153, 166, 218, 246, 251, 289, 320, 348) where .splice(), .filter(), or .map() are called immediately on results without type guards. Add an Array.isArray() check before returning, or implement shape validation for string[] expectations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@views/kanban-view-simple/src/components/Board/Board.tsx` around lines 21 -
29, safeJsonParse<T> currently returns parsed JSON without validating its shape,
which lets valid-but-incorrect values (objects, numbers, etc.) reach callers
that immediately call .splice/.filter/.map in the board drag-and-drop logic;
update safeJsonParse to perform runtime shape checks before returning: for the
kanban use case add an overload or optional validator parameter (or a boolean
branch) that verifies Array.isArray(parsed) and that each item is a string
(e.g., typeof item === 'string') when callers expect string[]; if validation
fails, log a warning and return the provided fallback. Ensure callers (the
reorder/drag-and-drop functions that call .splice/.filter/.map) keep using
safeJsonParse<string[]>(...) so the validated type is enforced at runtime.
Comment on lines
165
to
167
| const column = columns.find((col) => col.id === columnId); | ||
| const taskIds = column.orderedTaskIds ? JSON.parse(column.orderedTaskIds) : []; | ||
| const taskIds = safeJsonParse<string[]>(column.orderedTaskIds, []); | ||
| const columnTasks = await Task.findAll(perspective, { where: { id: taskIds } }); |
There was a problem hiding this comment.
deleteColumn can crash when column lookup misses
columns.find(...) can return undefined, but column.orderedTaskIds and column.delete() are called unguarded. Add a guard before dereference/delete.
Also applies to: 185-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@views/kanban-view-simple/src/components/Board/Board.tsx` around lines 165 -
167, The code calls columns.find(...) and immediately dereferences
column.orderedTaskIds and column.delete(), which can crash if the find returns
undefined; update the deleteColumn logic to check the result of
columns.find(...) (the variable column) and bail out early (return or throw a
controlled error) if it's undefined, then use column.orderedTaskIds only after
that guard (you can still call safeJsonParse on a fallback value if preferred)
and only invoke column.delete() inside the guarded block; apply the same
presence check to the other occurrence that references column (around the
Task.findAll / column.delete usage).
Comment on lines
+10
to
+17
| function safeJsonParse<T>(value: string | undefined | null, fallback: T): T { | ||
| if (!value) return fallback; | ||
| try { | ||
| return JSON.parse(value); | ||
| } catch { | ||
| console.warn('Failed to parse JSON value, using fallback:', value?.slice(0, 80)); | ||
| return fallback; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd TaskSettings.tsxRepository: coasys/flux
Length of output: 124
🏁 Script executed:
cat -n views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx | head -200Repository: coasys/flux
Length of output: 8566
🏁 Script executed:
wc -l views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsxRepository: coasys/flux
Length of output: 128
Add runtime validation to safeJsonParse<string[]> to prevent runtime crashes
JSON.parse can succeed with non-array payloads (objects, strings, numbers). The call sites on lines 73, 78, 124, and 152 immediately invoke array methods (.filter(), spread operator), which will crash if the parsed value is not an array. The current fallback of [] only activates on parse errors, not when parsing succeeds with invalid data.
Add a type guard parameter to validate the parsed value:
Proposed fix
-function safeJsonParse<T>(value: string | undefined | null, fallback: T): T {
+function safeJsonParse<T>(
+ value: string | undefined | null,
+ fallback: T,
+ isValid: (parsed: unknown) => parsed is T,
+): T {
if (!value) return fallback;
try {
- return JSON.parse(value);
+ const parsed: unknown = JSON.parse(value);
+ return isValid(parsed) ? parsed : fallback;
} catch {
console.warn('Failed to parse JSON value, using fallback:', value?.slice(0, 80));
return fallback;
}
}
+
+const isStringArray = (v: unknown): v is string[] =>
+ Array.isArray(v) && v.every((x) => typeof x === 'string');Then update call sites to pass the validator:
-const sourceOrderedTaskIds = safeJsonParse<string[]>(source.orderedTaskIds, []).filter((id) => id !== task.id);
+const sourceOrderedTaskIds = safeJsonParse<string[]>(source.orderedTaskIds, [], isStringArray).filter((id) => id !== task.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx` around
lines 10 - 17, The safeJsonParse<T> helper should reject parsed non-array values
to avoid runtime crashes when callers expect string[]; modify safeJsonParse to
accept an optional type guard (validator: (v: unknown) => v is T) and after
JSON.parse run the validator and return fallback if it fails. Update all callers
that expect string[] (the safeJsonParse<string[]>() usages in TaskSettings where
the result is immediately spread or .filter() is called) to pass Array.isArray
as the validator (or a stricter element-type guard) so only actual arrays are
returned; keep the existing try/catch fallback behavior for parse errors.
Comment on lines
+73
to
75
| const sourceOrderedTaskIds = safeJsonParse<string[]>(source.orderedTaskIds, []).filter((id) => id !== task.id); | ||
| source.orderedTaskIds = JSON.stringify(sourceOrderedTaskIds); | ||
| await source.save(batchId); |
There was a problem hiding this comment.
Guard .find(...) results before dereferencing
The code dereferences source, destination, and columnModel immediately after .find(...). If live data changes between UI action and save/delete, this will crash.
Proposed fix (defensive null checks)
const source = columns.find((col) => col.columnName === column.columnName);
const destination = columns.find((col) => col.columnName === taskColumn);
+if (!source || !destination) {
+ console.error('Source or destination column not found');
+ setSaving(false);
+ return;
+}
@@
const columnModel = columns.find((col) => col.columnName === taskColumn);
+if (!columnModel) {
+ console.error('Column not found when creating task');
+ setSaving(false);
+ return;
+}
@@
const columnModel = columns.find((col) => col.id === column.id);
+if (!columnModel) {
+ console.error('Column not found when deleting task');
+ setDeleting(false);
+ return;
+}Also applies to: 78-80, 123-126, 151-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@views/kanban-view-simple/src/components/TaskSettings/TaskSettings.tsx` around
lines 73 - 75, The code assumes .find(...) returned objects (e.g., source,
destination, columnModel) and dereferences them immediately (e.g., using
source.orderedTaskIds and source.save(batchId)); add defensive null checks after
each .find call in TaskSettings.tsx and bail out or handle the missing object
(early return, user-facing error, or log and skip) before using properties or
calling methods; specifically guard the variables named source, destination, and
columnModel where they are used around the shown snippets so you never call
.orderedTaskIds, .save(...), or access columnModel properties on undefined.
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.
Problem
When
orderedTaskIdsororderedColumnIdscontainliteral://URI strings (e.g. when tasks are created via MCP/agent tools),JSON.parsethrows:This crashes the Kanban Simple View entirely.
Root Cause
The Kanban view calls
JSON.parse(column.orderedTaskIds)in multiple places without any error handling. When an AI agent writes task IDs via MCP tools, the value may be stored as a literal URI string that cannot be parsed as JSON.Fix
Added a
safeJsonParse<T>()helper function that:Applied to all
JSON.parsecalls in:Board.tsx(8 occurrences)TaskSettings.tsx(4 occurrences)Testing
Found during live testing with an AI agent (Data) creating tasks in a Flux neighbourhood via AD4M MCP tools.
Summary by CodeRabbit