Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
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
177 changes: 82 additions & 95 deletions static/app/views/seerExplorer/hooks/useSeerExplorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,24 @@ export const useSeerExplorer = () => {
}
}, [location, navigate, openExplorerPanel, setRunId]);

// Check if Seer drawer is open - if so, always poll
const isSeerDrawerOpen = !!location.query?.seerDrawer;

const [waitingForResponse, setWaitingForResponse] = useState<boolean>(false);
const [deletedFromIndex, setDeletedFromIndex] = useState<number | null>(null);
const [interruptRequested, setInterruptRequested] = useState<boolean>(false);
const [wasJustInterrupted, setWasJustInterrupted] = useState<boolean>(false);
const prevInterruptRequestedRef = useRef<boolean>(false);

// Helpers for managing waiting, timeout, and interrupt state.
const _onNewRequest = useCallback(() => {
setWaitingForResponse(true);
setInterruptRequested(false);
setWasJustInterrupted(false);
}, []);

const _onRequestError = useCallback(() => {
setWaitingForResponse(false);
setInterruptRequested(false);
setWasJustInterrupted(false);
}, []);

const [deletedFromIndex, setDeletedFromIndex] = useState<number | null>(null);
const [optimistic, setOptimistic] = useState<{
assistantBlockId: string;
assistantContent: string;
Expand All @@ -161,25 +171,46 @@ export const useSeerExplorer = () => {
} | null>(null);
const previousPRStatesRef = useRef<Record<string, RepoPRState>>({});

const {
data: apiData,
isPending,
isError,
} = useApiQuery<SeerExplorerResponse>(makeSeerExplorerQueryKey(orgSlug || '', runId), {
staleTime: 0,
retry: false,
enabled: !!runId && !!orgSlug,
refetchInterval: query => {
// Always poll when Seer drawer is open (actions triggered from drawer need updates)
if (isSeerDrawerOpen) {
return POLL_INTERVAL;
}
if (isPolling(query.state.data?.[0]?.session || null, waitingForResponse)) {
return POLL_INTERVAL;
const {data: apiData, isError} = useApiQuery<SeerExplorerResponse>(
makeSeerExplorerQueryKey(orgSlug || '', runId),
{
staleTime: 0,
retry: false,
enabled: !!runId && !!orgSlug,
refetchInterval: query => {
if (isPolling(query.state.data?.[0]?.session || null, waitingForResponse)) {
return POLL_INTERVAL;
}
return false;
},
} as UseApiQueryOptions<SeerExplorerResponse, RequestError>
);

/** Switches to a different run and fetches its latest state. */
const switchToRun = useCallback(
(newRunId: number | null) => {
// Set the new run ID
setRunId(newRunId);

// Clear any optimistic state from previous run
setOptimistic(null);
setDeletedFromIndex(null);
setWaitingForResponse(false);
setInterruptRequested(false);
setWasJustInterrupted(false);

// Invalidate the query to force a fresh fetch
if (orgSlug && newRunId !== null) {
queryClient.invalidateQueries({
queryKey: makeSeerExplorerQueryKey(orgSlug, newRunId),
});
}
return false;
},
} as UseApiQueryOptions<SeerExplorerResponse, RequestError>);
[orgSlug, queryClient, setRunId]
);

/** Resets the hook state. The session isn't actually created until the user sends a message. */
const startNewSession = useCallback(() => switchToRun(null), [switchToRun]);

const sendMessage = useCallback(
async (query: string, insertIndex?: number, explicitRunId?: number | null) => {
Expand Down Expand Up @@ -207,9 +238,6 @@ export const useSeerExplorer = () => {
screenshot = captureAsciiSnapshot?.();
}

setWaitingForResponse(true);
setWasJustInterrupted(false);

trackAnalytics('seer.explorer.message_sent', {
referrer: getPageReferrer(),
surface: 'global_panel',
Expand Down Expand Up @@ -255,6 +283,8 @@ export const useSeerExplorer = () => {
baselineUpdatedAt: apiData?.session?.updated_at,
});

_onNewRequest();

try {
const {url} = parseQueryKey(makeSeerExplorerQueryKey(orgSlug, effectiveRunId));
const response = (await api.requestPromise(url, {
Expand All @@ -278,7 +308,7 @@ export const useSeerExplorer = () => {
queryKey: makeSeerExplorerQueryKey(orgSlug, response.run_id),
});
} catch (e: any) {
setWaitingForResponse(false);
_onRequestError();
setOptimistic(null);
if (effectiveRunId !== null) {
// API data is disabled for null runId (new runs).
Expand All @@ -292,6 +322,8 @@ export const useSeerExplorer = () => {
}
},
[
_onNewRequest,
_onRequestError,
queryClient,
api,
orgSlug,
Expand Down Expand Up @@ -321,6 +353,7 @@ export const useSeerExplorer = () => {
}

setInterruptRequested(true);
setWasJustInterrupted(false);

try {
await api.requestPromise(
Expand All @@ -346,7 +379,7 @@ export const useSeerExplorer = () => {
return;
}

setWaitingForResponse(true);
_onNewRequest();

try {
await api.requestPromise(
Expand All @@ -368,15 +401,15 @@ export const useSeerExplorer = () => {
queryKey: makeSeerExplorerQueryKey(orgSlug, runId),
});
} catch (e: any) {
setWaitingForResponse(false);
_onRequestError();
setApiQueryData<SeerExplorerResponse>(
queryClient,
makeSeerExplorerQueryKey(orgSlug, runId),
makeErrorSeerExplorerData(e?.responseJSON?.detail ?? 'An error occurred')
);
}
},
[api, orgSlug, runId, queryClient]
[_onNewRequest, _onRequestError, api, orgSlug, runId, queryClient]
);

const createPR = useCallback(
Expand Down Expand Up @@ -523,78 +556,33 @@ export const useSeerExplorer = () => {
previousPRStatesRef.current = currentPRStates;
}, [sessionData?.repo_pr_states]);

if (
waitingForResponse &&
filteredSessionData &&
Array.isArray(filteredSessionData.blocks)
) {
// Stop waiting once we see the response is no longer loading
const hasLoadingMessage = filteredSessionData.blocks.some(block => block.loading);

if (!hasLoadingMessage && filteredSessionData.status !== 'processing') {
setWaitingForResponse(false);
setInterruptRequested(false);
// Clear deleted index once response is complete
setDeletedFromIndex(null);
}
}

// Detect when interrupt succeeds and set wasJustInterrupted
// On response load
useEffect(() => {
const prevInterruptRequested = prevInterruptRequestedRef.current;
const currentlyPolling = isPolling(filteredSessionData, waitingForResponse);

// Reset interruptRequested when polling stops after an interrupt was requested
if (interruptRequested && !currentlyPolling) {
setInterruptRequested(false);
}

// Detect successful interrupt: was requested, now not requested, and not polling
if (prevInterruptRequested && !interruptRequested && !currentlyPolling) {
setWasJustInterrupted(true);
}

prevInterruptRequestedRef.current = interruptRequested;
}, [interruptRequested, filteredSessionData, waitingForResponse]);

/** Resets the hook state. The session isn't actually created until the user sends a message. */
const startNewSession = useCallback(() => {
// Reset state.
setRunId(null);
setWaitingForResponse(false);
setDeletedFromIndex(null);
setOptimistic(null);
setInterruptRequested(false);
setWasJustInterrupted(false);
}, [setRunId]);

/** Switches to a different run and fetches its latest state. */
const switchToRun = useCallback(
(newRunId: number) => {
// Clear any optimistic state from previous run
setOptimistic(null);
setDeletedFromIndex(null);
setWaitingForResponse(false);
setInterruptRequested(false);
setWasJustInterrupted(false);

// Set the new run ID
setRunId(newRunId);
if (
waitingForResponse &&
filteredSessionData &&
Array.isArray(filteredSessionData.blocks)
) {
// Stop waiting once we see the response is no longer loading
if (
filteredSessionData.status !== 'processing' &&
filteredSessionData.blocks.every(block => !block.loading)
) {
setWaitingForResponse(false);
// Clear deleted index once response is complete
setDeletedFromIndex(null);

// Invalidate the query to force a fresh fetch
if (orgSlug) {
queryClient.invalidateQueries({
queryKey: makeSeerExplorerQueryKey(orgSlug, newRunId),
});
if (interruptRequested) {
setInterruptRequested(false);
setWasJustInterrupted(true); // set persistent UI flag until next request
}
}
},
[orgSlug, queryClient, setRunId]
);
}
}, [waitingForResponse, filteredSessionData, interruptRequested]);

return {
sessionData: filteredSessionData,
isPolling: isPolling(filteredSessionData, waitingForResponse),
isPending,
isError,
sendMessage,
runId,
Expand All @@ -608,7 +596,6 @@ export const useSeerExplorer = () => {
interruptRequested,
/** True after an interrupt succeeds, until the user sends a new message or switches sessions. */
wasJustInterrupted,
clearWasJustInterrupted: useCallback(() => setWasJustInterrupted(false), []),
respondToUserInput,
createPR,
overrideCtxEngEnable,
Expand Down
Loading
Loading