Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 6 additions & 25 deletions static/app/views/seerExplorer/explorerPanel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,10 @@ describe('ExplorerPanel', () => {
startNewSession: jest.fn(),
isPolling: false,
isError: true, // isError
isPending: false,
deletedFromIndex: null,
interruptRun: jest.fn(),
interruptRequested: false,
wasJustInterrupted: false,
clearWasJustInterrupted: jest.fn(),
switchToRun: jest.fn(),
respondToUserInput: jest.fn(),
createPR: jest.fn(),
Expand Down Expand Up @@ -269,12 +267,10 @@ describe('ExplorerPanel', () => {
startNewSession: jest.fn(),
isPolling: false,
isError: false,
isPending: false,
deletedFromIndex: null,
interruptRun: jest.fn(),
interruptRequested: false,
wasJustInterrupted: false,
clearWasJustInterrupted: jest.fn(),
runId: null,
respondToUserInput: jest.fn(),
switchToRun: jest.fn(),
Expand Down Expand Up @@ -451,7 +447,7 @@ describe('ExplorerPanel', () => {
});
});

function mockSessionResponse(ownerUserId: number | null | undefined) {
function mockSessionResponse({ownerUserId}: {ownerUserId?: number}) {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/seer/explorer-chat/${runId}/`,
method: 'GET',
Expand All @@ -467,24 +463,9 @@ describe('ExplorerPanel', () => {
});
}

it('should have disabled input section when useUser returns none', async () => {
ConfigStore.set('user', undefined as any);
mockSessionResponse(2);

renderWithPanelContext(<ExplorerPanel />, true, {organization});

const textarea = await screen.findByTestId('seer-explorer-input');

expect(textarea).toBeDisabled();
expect(textarea).toHaveAttribute(
'placeholder',
'This conversation is owned by another user and is read-only'
);
});

it('should have disabled input section when explorer owner differs', async () => {
ConfigStore.set('user', UserFixture({id: '1'}));
mockSessionResponse(2);
mockSessionResponse({ownerUserId: 2});

renderWithPanelContext(<ExplorerPanel />, true, {organization});

Expand All @@ -497,9 +478,9 @@ describe('ExplorerPanel', () => {
);
});

it('enables input when owner id is null', async () => {
it('enables input when owner id matches', async () => {
ConfigStore.set('user', UserFixture({id: '1'}));
mockSessionResponse(null);
mockSessionResponse({ownerUserId: 1});

renderWithPanelContext(<ExplorerPanel />, true, {organization});

Expand All @@ -512,9 +493,9 @@ describe('ExplorerPanel', () => {
);
});

it('enables input when owner id matches', async () => {
it('enables input when owner id is undefined', async () => {
ConfigStore.set('user', UserFixture({id: '1'}));
mockSessionResponse(1);
mockSessionResponse({ownerUserId: undefined});

renderWithPanelContext(<ExplorerPanel />, true, {organization});

Expand Down
45 changes: 11 additions & 34 deletions static/app/views/seerExplorer/explorerPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {Text} from '@sentry/scraps/text';
import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
import {IconSeer} from 'sentry/icons';
import {t} from 'sentry/locale';
import type {User} from 'sentry/types/user';
import {trackAnalytics} from 'sentry/utils/analytics';
import {useFeedbackForm} from 'sentry/utils/useFeedbackForm';
import {useLocation} from 'sentry/utils/useLocation';
Expand Down Expand Up @@ -101,18 +100,17 @@ export function ExplorerPanel() {
const {
runId,
sessionData,
isPolling,
isError,
sendMessage,
deleteFromIndex,
startNewSession,
isPolling,
isError,
interruptRun,
interruptRequested,
wasJustInterrupted,
clearWasJustInterrupted,
switchToRun,
respondToUserInput,
createPR,
interruptRun,
interruptRequested,
wasJustInterrupted,
overrideCtxEngEnable,
setOverrideCtxEngEnable,
} = useSeerExplorer();
Expand All @@ -137,13 +135,6 @@ export function ExplorerPanel() {
onUnminimize: useCallback(() => setIsMinimized(false), []),
});

// Clear wasJustInterrupted when user starts typing
useEffect(() => {
if (inputValue.length > 0 && wasJustInterrupted) {
clearWasJustInterrupted();
}
}, [inputValue, wasJustInterrupted, clearWasJustInterrupted]);

// Extract repo_pr_states from session
const repoPRStates = useMemo(
() => sessionData?.repo_pr_states ?? {},
Expand All @@ -153,25 +144,11 @@ export function ExplorerPanel() {
// Get blocks from session data or empty array
const blocks = useMemo(() => sessionData?.blocks || [], [sessionData]);

// Check owner id to determine edit permission. Defensive against any useUser return shape.
// Despite the type annotation, useUser can return null or undefined when not logged in.
// This component is in the top-level index so we have to guard against this.
const rawUser = useUser() as unknown;
const ownerUserId = sessionData?.owner_user_id ?? undefined;
const readOnly = useMemo(() => {
const isUser = (value: unknown): value is User =>
Boolean(
value &&
typeof value === 'object' &&
'id' in value &&
typeof value.id === 'string'
);
const userId = isUser(rawUser) ? rawUser.id : undefined;
return (
userId === undefined ||
(ownerUserId !== undefined && ownerUserId?.toString() !== userId)
);
}, [rawUser, ownerUserId]);
// Check owner id to determine edit permission.
const user = useUser();
const readOnly =
sessionData?.owner_user_id !== undefined &&
sessionData.owner_user_id.toString() !== user.id;
Comment on lines +148 to +151
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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ConfigStore should be hydrated before component renders. Verified in dev-ui - the old defensive check was made by AI after an unrelated test flake


// Get PR widget data for menu
const {menuItems: prWidgetItems, menuFooter: prWidgetFooter} = usePRWidgetData({
Expand Down Expand Up @@ -735,10 +712,10 @@ export function ExplorerPanel() {
focusedBlockIndex={focusedBlockIndex}
inputValue={inputValue}
interruptRequested={interruptRequested}
wasJustInterrupted={wasJustInterrupted}
isMinimized={isMinimized}
isPolling={isPolling}
isVisible={isVisible}
wasJustInterrupted={wasJustInterrupted}
onClear={() => setInputValue('')}
onCreatePR={createPR}
onInputChange={handleInputChange}
Expand Down
Loading
Loading