Skip to content

Conversation

@cvxluo
Copy link
Contributor

@cvxluo cvxluo commented Dec 28, 2025

Add dev tool to paste json for the new top issues view

@cvxluo cvxluo changed the base branch from master to cvxluo/show-code-change-top-issues December 28, 2025 10:38
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Dec 28, 2025
Base automatically changed from cvxluo/show-code-change-top-issues to master December 29, 2025 17:52
@cvxluo cvxluo marked this pull request as ready for review December 30, 2025 23:40
@cvxluo cvxluo requested a review from a team as a code owner December 30, 2025 23:40
@cvxluo cvxluo requested a review from a team December 30, 2025 23:40
Comment on lines 961 to 963

return filtered.sort((a, b) => (b.fixability_score ?? 0) - (a.fixability_score ?? 0));
}, [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The handleParseJson function unsafely casts parsed JSON to ClusterSummary[] without validating object properties, leading to a crash when required properties are accessed later.
Severity: HIGH | Confidence: High

🔍 Detailed Analysis

The handleParseJson function uses a type assertion as ClusterSummary[] on user-provided JSON without validating that the objects in the array conform to the ClusterSummary interface. Subsequent code, such as the filtering logic, accesses required properties like cluster.project_ids without defensive checks. If a user pastes JSON where objects are missing this property, the application will crash with a TypeError when cluster.project_ids.some() is called. This affects a developer-facing tool within the production UI.

💡 Suggested Fix

Implement a type guard or use a validation library (like Zod) to properly validate the structure of each object in the parsed JSON array against the ClusterSummary interface before casting and setting the state with setCustomClusterData. This ensures all required properties exist before they are accessed.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/issueList/pages/topIssues.tsx#L961-L963

Potential issue: The `handleParseJson` function uses a type assertion `as
ClusterSummary[]` on user-provided JSON without validating that the objects in the array
conform to the `ClusterSummary` interface. Subsequent code, such as the filtering logic,
accesses required properties like `cluster.project_ids` without defensive checks. If a
user pastes JSON where objects are missing this property, the application will crash
with a `TypeError` when `cluster.project_ids.some()` is called. This affects a
developer-facing tool within the production UI.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8050715

</Button>
</Flex>
</JsonInputContainer>
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading indicator shown when custom data is loaded

When custom JSON data is loaded via the dev tool, the isPending check doesn't account for customClusterData being present. If a user loads custom data before the API query completes (or if the query is slow/fails), isPending may still be true from the original query, causing the LoadingIndicator to display instead of the custom cluster data. The condition needs to also check !isUsingCustomData or !customClusterData.

Fix in Cursor Fix in Web

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

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants