Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
60 changes: 60 additions & 0 deletions static/app/utils/useDeferredSessionStorage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import {useCallback, useEffect, useRef, useState} from 'react';

import {sessionStorageWrapper} from 'sentry/utils/sessionStorage';
import {readStorageValue, writeStorageValue} from 'sentry/utils/useSessionStorage';

function readValue<T>(key: string | null, initialValue: T): T {
if (key === null) {
return initialValue;
}
return readStorageValue(key, initialValue);
}

function writeValue(key: string | null, value: unknown): void {
if (key === null) {
return;
}
writeStorageValue(key, value);
}

/**
* Persists a value to sessionStorage under `key`. Unlike `useSessionStorage`, writes are deferred —
* storage is only updated on:
* - key change (flush old key, load new key)
* - unmount (flush current key)
* - explicit `reset()` (drop the persisted value and reset state to initialValue)
*
* When `key` is null the storage persistence is disabled - the value lives in React state only.
*/
export function useDeferredSessionStorage<T>(key: string | null, initialValue: T) {
const [value, setValue] = useState(() => readValue(key, initialValue));

const keyRef = useRef(key);
const valueRef = useRef(value);
valueRef.current = value;

// Flush to storage and update value on key change
useEffect(() => {
if (keyRef.current !== key) {
writeValue(keyRef.current, valueRef.current);
setValue(readValue(key, initialValue));
keyRef.current = key;
}
}, [key, initialValue]);

// Flush to storage on unmount
useEffect(() => {
return () => {
writeValue(keyRef.current, valueRef.current);
};
}, []);
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.

Unmount cleanup re-writes value after explicit reset

Low Severity

The unmount cleanup effect unconditionally writes valueRef.current to storage, which re-creates a storage entry that reset() explicitly removed via sessionStorageWrapper.removeItem. After handleSend calls clearInput() (i.e., reset()), closing the drawer triggers the unmount cleanup and writes the initialValue back under the same key, creating a zombie entry in sessionStorage. The test for "clears the persisted draft when a message is sent" only passes because it asserts before unmount occurs.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 158e036. Configure here.

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.

wont affect functionality, just an extra storage write on this edge case


const reset = useCallback(() => {
if (keyRef.current !== null) {
sessionStorageWrapper.removeItem(keyRef.current);
}
setValue(initialValue);
}, [initialValue]);

return {value, setValue, reset};
}
20 changes: 13 additions & 7 deletions static/app/utils/useSessionStorage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {sessionStorageWrapper} from 'sentry/utils/sessionStorage';

const isBrowser = typeof window !== 'undefined';

export function readStorageValue(key: string, initialValue: unknown) {
export function readStorageValue<T>(key: string, initialValue: T): T {
const value = sessionStorageWrapper.getItem(key);

// We check for 'undefined' because the value may have
Expand All @@ -23,6 +23,17 @@ export function readStorageValue(key: string, initialValue: unknown) {
}
}

export function writeStorageValue(key: string | null, value: unknown): void {
if (key === null) {
return;
}
try {
sessionStorageWrapper.setItem(key, JSON.stringify(value));
} catch {
// Best effort and just update the in-memory value.
}
}

export function useSessionStorage<T>(
key: string,
initialValue: T
Expand All @@ -45,12 +56,7 @@ export function useSessionStorage<T>(
? (valueOrUpdater as (prev: T) => T)(prev)
: valueOrUpdater;

try {
sessionStorageWrapper.setItem(key, JSON.stringify(next));
} catch {
// Best effort and just update the in-memory value.
}

writeStorageValue(key, next);
return next;
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrar

import {ConfigStore} from 'sentry/stores/configStore';
import {ExplorerDrawerContent} from 'sentry/views/seerExplorer/components/drawer/explorerDrawerContent';
import {INPUT_STORAGE_KEY_PREFIX} from 'sentry/views/seerExplorer/components/drawer/explorerDrawerContent';
import * as useSeerExplorerModule from 'sentry/views/seerExplorer/hooks/useSeerExplorer';
import {SeerExplorerSessionsProvider} from 'sentry/views/seerExplorer/seerExplorerSessionContext';
import type {SeerExplorerResponse} from 'sentry/views/seerExplorer/types';
Expand Down Expand Up @@ -472,6 +473,127 @@ describe('ExplorerDrawerContent', () => {
});
});

describe('Input Persistence', () => {
it('restores the persisted draft when the drawer remounts', async () => {
jest.spyOn(useSeerExplorerModule, 'useSeerExplorer').mockReturnValue({
...defaultHookReturn,
runId: 7,
});

const {unmount} = render(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>,
{organization}
);

await userEvent.type(
await screen.findByTestId('seer-explorer-input'),
'draft message'
);
unmount();

render(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>,
{organization}
);

expect(await screen.findByTestId('seer-explorer-input')).toHaveValue(
'draft message'
);
});

it('persists the draft per runId across run switches', async () => {
const useSeerExplorerSpy = jest.spyOn(useSeerExplorerModule, 'useSeerExplorer');
useSeerExplorerSpy.mockReturnValue({...defaultHookReturn, runId: 1});

const {rerender} = render(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>,
{organization}
);

await userEvent.type(
await screen.findByTestId('seer-explorer-input'),
'draft for run 1'
);

useSeerExplorerSpy.mockReturnValue({...defaultHookReturn, runId: 2});
rerender(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>
);

await waitFor(() =>
expect(screen.getByTestId('seer-explorer-input')).toHaveValue('')
);
expect(
JSON.parse(sessionStorage.getItem(`${INPUT_STORAGE_KEY_PREFIX}:1`) ?? '')
).toBe('draft for run 1');

useSeerExplorerSpy.mockReturnValue({...defaultHookReturn, runId: 1});
rerender(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>
);

await waitFor(() =>
expect(screen.getByTestId('seer-explorer-input')).toHaveValue('draft for run 1')
);
});

it('never writes to sessionStorage when runId is null (no session)', async () => {
const setItemSpy = jest.spyOn(Storage.prototype, 'setItem');

const {unmount} = render(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>,
{organization}
);

await userEvent.type(
await screen.findByTestId('seer-explorer-input'),
'unsaved draft'
);
unmount();

const draftWrites = setItemSpy.mock.calls.filter(([k]) =>
String(k).startsWith(`${INPUT_STORAGE_KEY_PREFIX}:`)
);
expect(draftWrites).toHaveLength(0);
});

it('clears the persisted draft when a message is sent', async () => {
const sendMessage = jest.fn();
jest.spyOn(useSeerExplorerModule, 'useSeerExplorer').mockReturnValue({
...defaultHookReturn,
sendMessage,
runId: 42,
});

render(
<SeerExplorerSessionsProvider>
<ExplorerDrawerContent getPageReferrer={mockGetPageReferrer} />
</SeerExplorerSessionsProvider>,
{organization}
);

const textarea = await screen.findByTestId('seer-explorer-input');
await userEvent.type(textarea, 'hello');
await userEvent.keyboard('{Enter}');

expect(sendMessage).toHaveBeenCalledWith('hello', 0);
expect(textarea).toHaveValue('');
expect(sessionStorage.getItem(`${INPUT_STORAGE_KEY_PREFIX}:42`)).toBeNull();
});
});

describe('Read-only State', () => {
it('disables input when session owner differs from current user', async () => {
ConfigStore.set('user', UserFixture({id: '1'}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {Stack} from '@sentry/scraps/layout';
import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
import {SEER_AGENTS_PROJECT_ID} from 'sentry/constants';
import {trackAnalytics} from 'sentry/utils/analytics';
import {useDeferredSessionStorage} from 'sentry/utils/useDeferredSessionStorage';
import {useFeedbackForm} from 'sentry/utils/useFeedbackForm';
import {useOrganization} from 'sentry/utils/useOrganization';
import {useProjects} from 'sentry/utils/useProjects';
Expand All @@ -31,6 +32,8 @@ import {
useSeerExplorerDeepLink,
} from 'sentry/views/seerExplorer/utils';

export const INPUT_STORAGE_KEY_PREFIX = 'seer-explorer-draft';

export function ExplorerDrawerContent({
getPageReferrer,
initialQuery,
Expand All @@ -43,7 +46,6 @@ export function ExplorerDrawerContent({
const user = useUser();
const {closeDrawer} = useDrawer();

const [inputValue, setInputValue] = useState('');
const [showThinking, setShowThinking] = useState(false);

const textareaRef = useRef<HTMLTextAreaElement>(null);
Expand All @@ -65,8 +67,8 @@ export function ExplorerDrawerContent({
errorStatusCode,
isTimedOut,
sendMessage,
startNewSession: startNewSessionBase,
switchToRun: switchToRunBase,
startNewSession,
switchToRun,
respondToUserInput,
createPR,
interruptRun,
Expand All @@ -76,10 +78,16 @@ export function ExplorerDrawerContent({
setOverrideCodeModeEnable,
} = useSeerExplorer();

const clearInput = () => setInputValue('');
const startNewSession = () => startNewSessionBase({onSuccess: clearInput});
const switchToRun = (newRunId: number | null) =>
switchToRunBase(newRunId, {onSuccess: clearInput});
// Persist the input draft per-run so drawer closes / run switches
// don't lose the user's in-progress text.
const {
value: inputValue,
setValue: setInputValue,
reset: clearInput,
} = useDeferredSessionStorage(
runId === null ? null : `${INPUT_STORAGE_KEY_PREFIX}:${runId}`,
''
);

const readOnly =
sessionData?.owner_user_id !== undefined &&
Expand Down Expand Up @@ -260,9 +268,9 @@ export function ExplorerDrawerContent({
return;
}
sendMessage(inputValue.trim(), blocks.length);
setInputValue('');
clearInput();
userScrolledUpRef.current = false;
}, [canSendMessage, inputValue, sendMessage, blocks.length]);
}, [canSendMessage, inputValue, sendMessage, blocks.length, clearInput]);

const handleInputKeyDown = useCallback(
(e: React.KeyboardEvent<HTMLTextAreaElement>) => {
Expand Down Expand Up @@ -452,7 +460,7 @@ export function ExplorerDrawerContent({
canSendMessage={canSendMessage}
interruptState={interruptState}
isTimedOut={isTimedOut}
onClear={() => setInputValue('')}
onClear={clearInput}
onCreatePR={createPR}
onInputChange={handleInputChange}
onInputClick={handleInputClick}
Expand Down
Loading