Skip to content

ref(explorer): cleanup UI interrupt state and panel code#112446

Open
aliu39 wants to merge 3 commits intomasterfrom
aliu/timeout
Open

ref(explorer): cleanup UI interrupt state and panel code#112446
aliu39 wants to merge 3 commits intomasterfrom
aliu/timeout

Conversation

@aliu39
Copy link
Copy Markdown
Member

@aliu39 aliu39 commented Apr 8, 2026

Code organization and cleanup for explorer chat UI, before implementing a polling timeout. Should not affect current functionality.

Refactors:

  • Use helpers _onNewRequest and _onRequestError for repeated interrupt/waiting state management in useSeerExplorer. This will soon include timeout state

  • Moves the "response complete" detection from inline code (ran every render) into a useEffect. In general we could use a lot less useEffect's but seems correct here - reducing effects is a larger refactor for another day

  • Simplifies interrupt detection by removing the prevInterruptRequestedRef approach — wasJustInterrupted is now set directly when polling stops after an interrupt request.

  • startNewSession delegates to switchToRun(null) to deduplicate code

  • Simplifies the readOnly computation in explorerPanel by removing defensive type-checking around useUser(). The comment is not accurate

  • Simplify props for disabled textarea in case of read-only

Cleanup:

  • Removes the always-poll-when-drawer-open (isSeerDrawerOpen) condition. Chat is no longer embedded into seer drawer.

  • Removes clearWasJustInterrupted from hook API. Clearing on user request + switch session is sufficient

  • Removes isPending from the hook's return value (unused externally and not useful due to polling)

@aliu39 aliu39 requested a review from a team as a code owner April 8, 2026 03:08
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 8, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 178f2ed. Configure here.

Removed timeout management from waiting and interrupt helpers.
Comment on lines +148 to +151
const user = useUser();
const readOnly =
sessionData?.owner_user_id !== undefined &&
sessionData.owner_user_id.toString() !== user.id;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The code directly accesses user.id, but useUser() can return undefined before the application is fully hydrated. This can cause a runtime crash.
Severity: CRITICAL

Suggested Fix

Add a null check to ensure the user object is defined before attempting to access its id property. The comparison logic should only execute when user is not null or undefined.

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/seerExplorer/explorerPanel.tsx#L148-L151

Potential issue: The `useUser()` hook can return `undefined` before the `ConfigStore` is
hydrated with user data. The code accesses `user.id` without checking if `user` is
defined. If the component renders before user data is available and
`sessionData.owner_user_id` exists, the expression `sessionData.owner_user_id.toString()
!== user.id` will attempt to read the `id` property of `undefined`, causing a
`TypeError` and a runtime crash. The previous implementation guarded against this exact
scenario, and the test validating this guard was removed in this pull request.

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.

1 participant