Skip to content

fix(app): guard against non-array diffs in session review#19250

Closed
jackblackbla wants to merge 1 commit intoanomalyco:devfrom
jackblackbla:fix/session-review-diffs-guard
Closed

fix(app): guard against non-array diffs in session review#19250
jackblackbla wants to merge 1 commit intoanomalyco:devfrom
jackblackbla:fix/session-review-diffs-guard

Conversation

@jackblackbla
Copy link
Copy Markdown

Summary

  • Add Array.isArray guard in event-reducer before passing diff data to reconcile(), preventing non-array values from being stored in session_diff
  • Add safeDiffs() accessor in SessionReview component to normalize props.diffs, preventing .map() crash on non-array values
  • Add 3 test cases for session.diff event handling: normal array, undefined, and non-array object payloads

Problem

Users are seeing a common crash in the session review UI:

TypeError: e.diffs.map is not a function
    at Object.fn (session-BdJIdAn0.js:19:50326)

The root cause is in event-reducer.ts where session.diff event properties are cast with as { sessionID: string; diff: FileDiff[] } — an unsafe cast with no runtime validation. If the SSE event delivers malformed diff data (e.g., undefined or an object instead of an array), reconcile() stores a non-array value in the session_diff store. The ?? [] fallback in the memo doesn't catch truthy non-array values like {}, so the SessionReview component crashes when calling .map() on it.

Test plan

  • All 302 existing tests pass
  • 3 new tests added for session.diff event edge cases (undefined, non-array)
  • TypeScript typecheck passes across all 13 packages

🤖 Generated with Claude Code

@github-actions github-actions bot added the needs:compliance This means the issue will auto-close after 2 hours. label Mar 26, 2026
@github-actions
Copy link
Copy Markdown
Contributor

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Potential Duplicate Found

PR #19221: fix(app): guard diffs with Array.isArray to prevent .map crash
#19221

Why it's related: This PR appears to address the exact same issue - preventing .map() crashes by guarding diffs with Array.isArray(). Both PRs are fixing the SessionReview component crash where non-array diffs cause TypeError: e.diffs.map is not a function. This is likely a duplicate of the same fix.

The SessionReview component crashes with `TypeError: e.diffs.map is not
a function` when `props.diffs` is not an array. This can happen when
the `session.diff` SSE event delivers malformed properties that bypass
the unsafe `as` cast in the event reducer, causing `reconcile` to store
a non-array value.

- Add `Array.isArray` guard in event-reducer before passing to reconcile
- Add `safeDiffs()` accessor in session-review.tsx to normalize props
- Add tests for undefined and non-array diff payloads

Fixes the common `e.diffs.map is not a function` crash reported by
multiple users in the session review UI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jackblackbla jackblackbla force-pushed the fix/session-review-diffs-guard branch from a77e44c to 16f5bc2 Compare March 26, 2026 08:50
@jackblackbla
Copy link
Copy Markdown
Author

Closing as duplicate of #19221 which covers the same fix with broader scope. Arrived at the same root cause independently — the unsafe cast + reconcile path in event-reducer.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:compliance This means the issue will auto-close after 2 hours. needs:issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant