feat(web): scratchlist v1.1 — composer-toggle drawer + reusable FUE primitive (#798)

* feat(web): scratchlist v1.1 — composer-toggle drawer + reusable FUE primitive

The v1 always-visible amber band proved too heavy for what's a 20% feature
in a typical session. v1.1 dials it back to a composer-toggle that opens
an on-demand drawer, paired with a reusable FUE (First-User Experience)
primitive so existing operators get a subtle pulsing dot + on-click
explainer the first time they see the toggle.

UX changes:
- Notepad icon in the composer toolbar (next to schedule-send) toggles
  scratchlist mode. Drawer renders only while mode is on.
- Composer's send button repaints amber and reads "Send to scratchlist"
  while the mode is sticky; submit routes adds into the scratchlist
  instead of the chat. Click the icon again to leave.
- Small entry-counter badge appears on the toggle when entries exist;
  empty-state shows just the icon (no zero-state guilt UI).

New reusable FUE primitive:
- web/src/lib/use-fue.ts: state machine (unseen → engaging → acknowledged)
  with localStorage persistence, namespaced under hapi.fue.v1.<featureId>
  so it can't collide with any future upstream onboarding flow.
- web/src/components/Fue.tsx: <FueDot> (small pulsing badge) and
  <FueCallout> (portal-rendered popover with title/body + "Got it"
  affirmative-action dismiss). No auto-timeout — reading speed varies
  and silent disappearance undercuts user trust.
- AGENTS.md adds a "Adding new web features — consider an FUE" section
  so future contributors discover the primitive.

Refactors:
- ScratchlistPanel.tsx: split rendering into <ScratchlistInventory>
  (presentational list) and <ScratchlistDrawer> (composer-controlled
  drawer with hint copy). Original <ScratchlistPanel> kept exported
  for the existing fixture-based tests.
- SessionChat.tsx: scratchlist state lifted into useScratchlist hook
  so the composer-toolbar counter and the drawer share one source of
  truth. onSend wrapped to route through scratchlist.add when mode
  is on.

Tests:
- 9 useFue hook tests (initial state, engage idempotency, no
  auto-acknowledge, dismiss, featureId switching, post-acknowledged
  engage no-op, resetFue helper).
- 5 placement helper tests (above/below switching, viewport edge
  clamping, visualViewport offset support).
- All 21 existing scratchlist lib tests + 14 ScratchlistPanel tests
  continue to pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): prevent cross-session leak in useScratchlist hook

Per upstream review on PR #798 (github-actions[bot] [Major]):

  > useScratchlist persists current `entries` whenever `sessionId`
  > changes. On A -> B navigation, React first commits with B's id
  > and A's entries; after paint, this persist effect can write A's
  > entries to hapi.scratchlist.v1.B before the rehydrate effect
  > loads B. The previous keyed panel existed specifically to avoid
  > this race.

Lifting state out of the v1 panel (which sidestepped the race via
key={props.session.id} forced remount) re-introduced this same data-
loss window. The composer-controlled drawer in v1.1 cannot remount
on session change because its parent SessionChat doesn't either.

Fix: keep the loaded sessionId in state alongside the entries so they
swap atomically, and persist against the LOADED sessionId rather than
the prop. After A->B, the loaded sessionId is still A until rehydrate
runs, so a spurious persist re-writes A's storage with A's entries -
a no-op instead of a corruption.

Tests:
- New use-scratchlist.test.ts with 6 tests:
  - hydrates from localStorage on mount
  - add() persists to current session's storage only
  - rerender to a new session preserves the new session's existing entries
  - after switching, add() targets the new session
  - regression test that spies on Storage.prototype.setItem and asserts
    the rerender lifecycle never produces a (B-key, A-entries) write
  - remove()/move() target the loaded sessionId
- The setItem-spy test correctly fails against the buggy code (verified
  by temporarily reverting the fix) and passes with the fix in place.
- Full web suite: 88 files, 756 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): route attachment/scheduled submits to chat instead of dropping them

Per upstream review on PR #798 (github-actions[bot] [Major]):

  > Prevent scratchlist mode from dropping attachments — in scratchlist
  > mode the wrapper returns success after adding only `text`, while
  > HappyComposer still treats composer attachments as sendable input.
  > A text+attachment submit therefore routes through this branch,
  > stores only the text, and silently discards the attachment instead
  > of sending or preserving it.

Same hazard applies to scheduledAt: scratchlist entries are pure-text
notes - they can't represent attachments or schedule metadata - so any
submit carrying either MUST fall through to props.onSend (chat) even
when the scratchlist toggle is on. Otherwise the wrapper short-circuits
to scratchlist.add(text), reports success to the composer, and the
composer dutifully clears attachments + schedule that the user just
queued.

Fix: extracted the routing rule into shouldRouteToScratchlist(mode,
attachments, scheduledAt) - returns true only when mode is on AND the
payload is pure text. onSendForComposer uses it.

Tests:
- 5 new shouldRouteToScratchlist unit tests (mode off, mode on +
  text-only, mode on + attachments, mode on + schedule, mode on + both)
- All in web/src/components/SessionChat.test.ts (13 tests total now)
- Full web suite: 88 files, 761 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): clear pendingSchedule when scratchlist-mode submission falls back to chat

Per upstream review on PR #798 (github-actions[bot] [Major]):

  > The follow-up change correctly falls through to props.onSend when
  > scratchlist mode is on but scheduledAt is present, yet the
  > accepted-send cleanup still checks only !scratchlistMode. That
  > means a scheduled chat send made while the amber scratchlist UI is
  > active is accepted, but pendingSchedule stays set, so the next
  > normal send can accidentally reuse the same schedule.

Fix: handleSend now gates the cleanup branch on the actual route taken
(routedToScratchlist) rather than the scratchlist UI state. Reuses the
same shouldRouteToScratchlist helper so route + cleanup share a single
source of truth.

Tests:
- 2 new tests in SessionChat.test.ts that pin the decision matrix
  handleSend depends on:
  - 'cleanup gate: scheduled chat send while scratchlist toggle is on
     still clears schedule'
  - 'cleanup gate: pure-text scratchlist add does NOT clear schedule'
- Full web suite: 88 files, 763 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): UnifiedButton must reflect actual routing, not raw scratchlist toggle

Per upstream review on PR #798 (github-actions[bot] [Major]):

  > Send button advertises scratchlist routing even when the submit
  > will go to chat — shouldRouteToScratchlist correctly falls back
  > to normal chat for attachments or scheduledAt, but UnifiedButton
  > still turns amber and labels the action as "Send to scratchlist"
  > whenever scratchlistMode is true. A scheduled send or attachment
  > send made in that state will be submitted to chat while the UI
  > says it is being stashed, which can send content to the agent
  > unexpectedly.

Fix:
- UnifiedButton's prop renamed `scratchlistMode` -> `routesToScratchlist`
  to make the contract explicit: "this submit really will go to the
  scratchlist", not "the scratchlist toggle is on".
- The call site computes `routesToScratchlist` from
  `scratchlistMode && !hasAttachments && pendingSchedule == null`,
  mirroring SessionChat's shouldRouteToScratchlist exactly. The button
  is now amber + "Send to scratchlist" only when the actual send path
  will hit scratchlist; attachments / pending schedule force a chat-
  style render that matches the real routing.
- UnifiedButton exported so it can be unit-tested directly.

Tests:
- 3 new render tests in ComposerButtons.test.tsx covering:
  - routesToScratchlist=true → amber + "Send to scratchlist"
  - routesToScratchlist=false → black + "Send" (the regression case)
  - omitted prop → defaults to chat-style render
- Full web suite: 89 files, 766 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): exit scratchlist mode when promoting an entry to the composer

Per upstream review on PR #798 (HAPI Bot, follow-up after b256fe5):

  > Found one major issue: promoting a scratchlist item to the composer
  > keeps scratchlist mode enabled, so the next send re-adds it to the
  > scratchlist instead of sending to chat.

Promoting an entry to the composer means "I want to send this for real
now". With scratchlist mode still on, the next composer submit routes
back to scratchlist (per the v1.1 modal-mode contract), so the user's
click loop becomes promote -> send -> re-add -> nothing-actually-sent.

Fix: ScratchlistDrawerHost now calls onExitScratchlistMode whenever it
promotes an entry to the composer. Promote-to-queue does NOT exit the
mode (queue path bypasses the wrapper anyway, and the operator may
still be capturing related notes).

Tests:
- Exported ScratchlistDrawerHost so its host-level callbacks can be
  unit-tested in isolation (previously only ScratchlistDrawer was
  testable; the wiring was untested).
- New SessionChat.exit-mode.test.tsx with 2 tests:
  - promote-to-composer fires setText AND onExitScratchlistMode
  - promote-to-queue fires onSend but does NOT exit mode
- Full web suite: 90 files, 768 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(scratchlist): Ctrl/Cmd+Shift+S toggles scratchlist mode (v1.1 hotkey)

The v1 always-visible panel had Ctrl/Cmd+Shift+S to expand the panel and
focus the input. v1.1 mounts the drawer only when scratchlistMode is on,
so the v1 listener (inside the panel) is dead code: it can't fire while
the drawer is unmounted, and the user has no way to open the drawer
without clicking the toolbar icon. Re-bind the shortcut at SessionChat
scope so it's always alive and toggles the mode.

Convention matches sibling globals (Ctrl/Cmd-m cycles agent model).
Ctrl/Cmd-Shift-S is unreserved by Chrome / Firefox / Safari (browser
Save As is Ctrl-S / Cmd-S, no Shift), so the user's save-page muscle
memory keeps working. Modifier requirement (Ctrl/Cmd+Shift) means it
can't collide with literal-character typing in any input - no focus
suppression needed.

The matcher is extracted to a pure helper isScratchlistToggleHotkey
so it's unit testable without mounting SessionChat. 6 new tests pin
the modifier matrix:
  - Ctrl+Shift+S (Linux/Windows) -> match
  - Cmd+Shift+S (macOS)          -> match
  - Cmd/Ctrl+S without Shift     -> reject (browser Save reservation)
  - bare S / Shift+S             -> reject (literal typing)
  - Ctrl+Shift+Alt+S             -> reject (avoid OS clashes)
  - other modifier+key combos    -> reject

Tooltip + FUE body now mention the hotkey so it's discoverable from
the same UI surface that introduces the feature (en + zh-CN).

Web suite 90 files / 774 tests, all green. Typecheck clean.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): hotkey skips dialogs / inputs / contentEditable

Bot finding on PR #798 (PRRT_kwDOQuQOSc6HGtLn): the window-level
Ctrl/Cmd+Shift+S listener fires for every focus target, so the
shortcut can toggle scratchlist mode "behind" an open modal (rename
session, schedule picker, FUE callout, image preview), making the
next composer send route to scratchlist instead of chat. UX bug.

Add isScratchlistHotkeyBlockedTarget(target) and gate the listener
on it. Block targets:

  - any descendant of an open [role="dialog"] (Radix UI's
    DialogContent renders role="dialog"; FueCallout, ScheduleTimePicker,
    ImagePreview also use role="dialog")
  - HTMLInputElement (single-line inputs)
  - HTMLSelectElement
  - any contentEditable host (with attribute-based fallback for jsdom,
    which doesn't implement isContentEditable)

NOT blocked:
  - HTMLTextAreaElement (the composer textarea is the expected focus
    target when the operator presses the hotkey - blocking would
    defeat the shortcut)
  - the document body / unfocused targets

8 new unit tests pin the matrix. Function is exported / pure so
callers can reuse the same blocked-target rule for future global
shortcuts. Suggested fix from the bot applied modulo:

  - Use !== null on closest() result (explicit boolean for return type)
  - Add attribute-based contentEditable fallback for jsdom test env

* feat(scratchlist): copy-to-clipboard action on each entry

Add a per-entry "Copy to clipboard" button between the send-to-queue and
delete actions. On click, write the entry text via the shared
safeCopyToClipboard helper (which already handles the navigator.clipboard
primary path + the execCommand fallback for Safari / non-secure-context
edges); on success, briefly flip the icon to a check and the
aria-label/title to "Copied!" for 1500ms so the operator gets visual +
screen-reader confirmation. Failures (clipboard denied AND execCommand
fallback unavailable) silently no-op rather than throw at the click
handler.

Mirrored across both surfaces:
  - ScratchlistInventory (used by the v1.1 composer-toggle drawer)
  - ScratchlistPanel inline list (the v1 always-visible panel)

A small useCopiedFeedback() hook owns the "which entry just got copied"
state + the 1.5s auto-clear timeout. Pure state machine; the caller
wires safeCopyToClipboard separately so the hook itself stays free of
jsdom clipboard quirks. Cleared on unmount via the standard ref-tracked
timeout pattern, so promote-and-navigate-away can't leak.

Locale keys: scratchlist.action.copy / scratchlist.action.copied (en + zh-CN).

Three new tests:
  - v1 panel happy path: writeText called with the entry text, button
    flips to the "Copied!" label, entry is preserved (copy is non-destructive).
  - v1 panel failure path: writeText rejects AND execCommand returns
    false; button stays in "Copy to clipboard" state — no false success.
  - v1.1 drawer happy path: writeText called, label flips, and crucially
    no other entry handlers (onSend, onDelete, setText, onExitScratchlistMode)
    fire — copy is independent of all the other actions.

Web suite 90 files / 785 tests, all green. Typecheck clean.

* fix(scratchlist): reset all per-session state via keyed wrapper

Bot finding on PR #798 (PRRT_kwDOQuQOSc6HHOsa): when the operator
navigates between sessions on the same route (/sessions/A ->
/sessions/B), React reuses the SessionChat component instance.
Effects run AFTER the first paint, so for a single render window the
new session is rendered with the previous session's scratchlist
entries (useScratchlist's rehydrate-effect) AND drawer-open state
(scratchlistMode reset effect). Visual leak; drawer actions targeting
stale state.

Apply the bot's suggested fix verbatim modulo the type extraction:

    export function SessionChat(props) {
        return <SessionChatInner key={props.session.id} {...props} />
    }

Canonical React idiom for "fully reset state on prop change": the
keyed wrapper unmounts and re-mounts the inner component when
session.id changes, so every hook (useScratchlist's initial-state
factory, useState, useHappyRuntime, ...) starts fresh. This
supersedes the now-redundant effect-based reset:

  - useEffect(() => { setScratchlistMode(false) }, [session.id])  REMOVED

useScratchlist's atomic-loaded-sessionId persistence (added on the
prior PR round) stays as defense-in-depth for any caller that uses
the hook without the keyed-wrapper pattern.

Web suite 90 files / 785 tests, all green. Typecheck clean.

* fix(web): retain composer text on send failure (closes #776)

When the message composer submits and the hub responds with a 4xx/5xx
or the fetch fails outright, assistant-ui clears the composer
synchronously the moment send is invoked. Without intervention the
operator's typed text is destroyed at exactly the moment they most
need it preserved. SessionChat additionally clears any pending
schedule on accept, so a failed scheduled send was also silently
downgrading to immediate on the next attempt.

Behaviour:

- useSendMessage exposes onError({ sessionId, text, scheduledAt, error })
  so the route can hand the input back to the composer. sessionId is
  the resolved target (post-resolveSessionId), so an inactive-session
  resume that resolves a new id, kicks off async navigation, then
  fails the POST restores into the resumed session's composer rather
  than the old one.

- router.tsx stores sendErrors keyed by sessionId. Per-session lookup
  replaces the clear-on-session-change effect, so errors do not bleed
  between sessions and a session-scoped failure persists across
  navigation.

- HappyComposer accepts ComposerSendError, restores text via
  api.composer().setText() once per failure id, and re-establishes any
  pending schedule via onSchedule({ type: 'absolute', ms: scheduledAt }).
  It renders a red ring on the composer wrapper and a role="alert"
  inline message; both clear the moment the operator types or sends.

- onError forks on input.attachments. Text-only sends use the
  composer-restore path (removeOptimisticMessage drops the row so the
  failed bubble does not duplicate the restored text). Attachment
  sends keep the legacy failed-bubble UX (status='failed' + in-thread
  retry button) because the composer-restore path can't reinstate
  uploaded attachment metadata. retryMessage extracts attachments
  from the stored optimistic message via getMessageAttachments so
  failed-bubble retry of an attachment send re-fires with its files.

Acceptance (issue #776):

- Submit -> 500/502/503/network error -> composer text not cleared
- Submit -> 400/401/403 -> composer text not cleared, error inline
- Submit -> 2xx -> composer clears as today
- Operator can edit retained text and retry without re-typing
- Failed scheduled sends restore as scheduled, not as immediate

Tests in web/src/hooks/mutations/useSendMessage.test.tsx cover
text-only 4xx/5xx/network retention, scheduled-send carry-through,
optimistic-row removal on text-only failure, sessionId carry-through
under resolveSessionId, attachment failure fallback, and attachment
retry preservation. Full web suite passes (705 tests). bun typecheck
clean. No SCHEMA_VERSION bump (frontend-only).

* fix(test): correct AttachmentMetadata fixture shape + JSX namespace import

Two pre-existing test-only typecheck failures surfaced once scratchlist v1.1
was stacked into the driver soup.

* SessionChat.test.ts - the attachment() fixture used the legacy schema
  (kind, sizeBytes) instead of the current AttachmentMetadataSchema
  (filename, size, path). Updated to match the live shape so the cast
  is honest.

* ComposerButtons.test.tsx - JSX namespace is no longer global under
  the current TS lib config; switched the helper signature from
  JSX.Element to React's ReactElement (same runtime, named import).

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(scratchlist): soften panel chrome - drop strong amber fill, keep subtle accent border (#812)

Per #812 (and PR 827 from @swear01) the always-visible amber fill
on the scratchlist panel was too loud as a scroll element. This
swaps the warning *fill* for the chat-user-surface tone and uses
neutral text/pills/focus, but keeps the warning *border* as a soft
accent so the panel still reads as a different destination from a
normal user message.

The strong destination signal continues to live on the composer
Send button (it goes amber-500 only while scratchlist mode is
routing) and the active toggle button - those carry the
moment-of-action signal the user actually presses, and ComposerButtons
tests + the FUE copy already depend on that behavior, so they're
unchanged.

Credit to @swear01 (PR 827) for the styling note; this branch
absorbs that restyle and supersedes the Settings-toggle approach
because v1.1 hides the panel by default behind the composer drawer
toggle (no Settings entry needed).

Adds a regression-guard test asserting the panel uses the
chat-user-surface bg + warning-border (not the warning fill).

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
HeavyGee
2026-06-08 13:49:42 +08:00
committed by GitHub
co-authored by Cursor
parent deb05bb783
commit 393cd7bfbb
20 changed files with 2631 additions and 67 deletions
+12
View File
@@ -231,6 +231,7 @@ export default {
'chat.settings': 'Settings',
'chat.terminal': 'Terminal',
'chat.switchRemote': 'Switch to remote mode',
'chat.sendError.fallback': "Couldn't send your message. Edit and try again.",
// Codex review
'codexReview.title': 'Codex review',
@@ -404,6 +405,12 @@ export default {
'scratchlist.count.one': '1 item',
'scratchlist.count.other': '{n} items',
'scratchlist.emptyHint': 'Park notes, drafts, or ideas here. Nothing is sent until you promote it.',
'scratchlist.drawerHint': 'Type below — Send adds the next message to the scratchlist instead of the chat. Click the note icon again to leave.',
'scratchlist.toggleAriaLabel': 'Scratchlist drawer',
'scratchlist.toggleTooltip': 'Scratchlist — park notes & drafts (Ctrl/Cmd+Shift+S)',
'scratchlist.sendToScratchlist': 'Send to scratchlist',
'scratchlist.fueTitle': 'New: Scratchlist',
'scratchlist.fueBody': 'Park notes & drafts here without sending. The Send button glows amber while you stash; click the icon (or Ctrl/Cmd+Shift+S) again to leave.',
'scratchlist.addPlaceholder': 'Note, draft, or idea — Enter to add',
'scratchlist.addAriaLabel': 'Add scratchlist entry',
'scratchlist.add': 'Add',
@@ -414,7 +421,12 @@ export default {
'scratchlist.action.moveDown': 'Move entry down',
'scratchlist.action.promoteToComposer': 'Copy into composer',
'scratchlist.action.promoteToQueue': 'Send to queue',
'scratchlist.action.copy': 'Copy to clipboard',
'scratchlist.action.copied': 'Copied!',
'scratchlist.action.delete': 'Delete entry',
'fue.newFeatureDot': 'New feature available',
'fue.gotIt': 'Got it',
'fue.closeAriaLabel': 'Close explainer',
'composer.codexSlashUnsupported.title': 'Codex command unavailable',
'composer.codexSlashUnsupported.body': 'HAPI remote mode does not yet run built-in Codex slash commands like {command}. Use natural language instead, or run it in the local Codex TUI.',
+12
View File
@@ -234,6 +234,7 @@ export default {
'chat.settings': '设置',
'chat.terminal': '终端',
'chat.switchRemote': '切换到远程模式',
'chat.sendError.fallback': '消息未能发送。请修改后重试。',
// Codex review
'codexReview.title': 'Codex review',
@@ -407,6 +408,12 @@ export default {
'scratchlist.count.one': '1 条',
'scratchlist.count.other': '{n} 条',
'scratchlist.emptyHint': '在此暂存笔记、草稿或想法。需点击发送或编辑后才会真正发出。',
'scratchlist.drawerHint': '在下方输入 — 发送会把内容存入此暂存清单,而不是发到对话。再次点击便签图标可退出。',
'scratchlist.toggleAriaLabel': '暂存清单抽屉',
'scratchlist.toggleTooltip': '暂存清单 — 暂存笔记与草稿(Ctrl/Cmd+Shift+S',
'scratchlist.sendToScratchlist': '存入暂存清单',
'scratchlist.fueTitle': '新功能:暂存清单',
'scratchlist.fueBody': '在此暂存笔记和草稿,不会被发送。暂存模式下发送按钮会显示琥珀色;再次点击图标(或 Ctrl/Cmd+Shift+S)可退出。',
'scratchlist.addPlaceholder': '笔记、草稿或想法 — 回车键添加',
'scratchlist.addAriaLabel': '添加草稿夹条目',
'scratchlist.add': '添加',
@@ -417,7 +424,12 @@ export default {
'scratchlist.action.moveDown': '下移',
'scratchlist.action.promoteToComposer': '复制到输入框',
'scratchlist.action.promoteToQueue': '加入发送队列',
'scratchlist.action.copy': '复制到剪贴板',
'scratchlist.action.copied': '已复制!',
'scratchlist.action.delete': '删除条目',
'fue.newFeatureDot': '新功能可用',
'fue.gotIt': '知道了',
'fue.closeAriaLabel': '关闭说明',
'composer.codexSlashUnsupported.title': '无法执行 Codex 命令',
'composer.codexSlashUnsupported.body': 'HAPI 远程模式暂不支持 {command} 这类 Codex 内建 slash command,请改用自然语言,或在本地 Codex TUI 中执行。',
+103
View File
@@ -0,0 +1,103 @@
import { act, renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { resetFue, useFue } from './use-fue'
const FEATURE = 'test-feature'
const STORAGE_KEY = `hapi.fue.v1.${FEATURE}`
describe('useFue', () => {
beforeEach(() => {
localStorage.clear()
vi.useFakeTimers()
})
afterEach(() => {
vi.useRealTimers()
})
it('starts unseen for new features', () => {
const { result } = renderHook(() => useFue(FEATURE))
expect(result.current.status).toBe('unseen')
})
it('reads acknowledged state from localStorage on mount', () => {
localStorage.setItem(STORAGE_KEY, '1')
const { result } = renderHook(() => useFue(FEATURE))
expect(result.current.status).toBe('acknowledged')
})
it('engage() flips status to engaging once', () => {
const { result } = renderHook(() => useFue(FEATURE))
act(() => {
result.current.engage()
})
expect(result.current.status).toBe('engaging')
// Re-engage is a no-op (does not flip back).
act(() => {
result.current.engage()
})
expect(result.current.status).toBe('engaging')
})
it('does NOT auto-acknowledge — engaging persists until dismiss is called', () => {
const { result } = renderHook(() => useFue(FEATURE))
act(() => {
result.current.engage()
})
// Advance the clock far past any plausible timeout.
act(() => {
vi.advanceTimersByTime(60_000)
})
expect(result.current.status).toBe('engaging')
expect(localStorage.getItem(STORAGE_KEY)).toBeNull()
})
it('dismiss() acknowledges and persists', () => {
const { result } = renderHook(() => useFue(FEATURE))
act(() => {
result.current.engage()
})
act(() => {
result.current.dismiss()
})
expect(result.current.status).toBe('acknowledged')
expect(localStorage.getItem(STORAGE_KEY)).toBe('1')
})
it('dismiss() also works directly from unseen (caller may skip the engaging step)', () => {
const { result } = renderHook(() => useFue(FEATURE))
act(() => {
result.current.dismiss()
})
expect(result.current.status).toBe('acknowledged')
expect(localStorage.getItem(STORAGE_KEY)).toBe('1')
})
it('engage() is a no-op once acknowledged', () => {
localStorage.setItem(STORAGE_KEY, '1')
const { result } = renderHook(() => useFue(FEATURE))
act(() => {
result.current.engage()
})
expect(result.current.status).toBe('acknowledged')
})
it('resetFue() clears storage so the badge re-appears', () => {
localStorage.setItem(STORAGE_KEY, '1')
resetFue(FEATURE)
expect(localStorage.getItem(STORAGE_KEY)).toBeNull()
const { result } = renderHook(() => useFue(FEATURE))
expect(result.current.status).toBe('unseen')
})
it('switches state when featureId changes', () => {
localStorage.setItem('hapi.fue.v1.feature-a', '1')
const { result, rerender } = renderHook(
({ id }: { id: string }) => useFue(id),
{ initialProps: { id: 'feature-a' } }
)
expect(result.current.status).toBe('acknowledged')
rerender({ id: 'feature-b' })
expect(result.current.status).toBe('unseen')
})
})
+98
View File
@@ -0,0 +1,98 @@
import { useCallback, useEffect, useState } from 'react'
/**
* useFue — generic First-User-Experience badge / callout state machine.
*
* Goal: any new feature can advertise itself with a small dot on its
* affordance; the dot disappears for good once the user has engaged
* with it AND explicitly acknowledged the explainer. Hover surfaces a
* tooltip (caller's responsibility); click triggers `engage()`, which
* flips status to 'engaging' and renders the callout. Auto-timeout is
* deliberately not provided — reading speed varies, and a popover that
* disappears on its own undercuts the affirmative-action model.
*
* Storage: hapi.fue.v1.<featureId> ('1' once acknowledged, absent otherwise)
*
* Status machine:
* unseen — initial. Badge visible, callout primed.
* engaging — operator has clicked the affordance for the first time.
* Callout is showing; awaiting explicit dismiss.
* acknowledged — terminal. Persisted to localStorage. Badge + callout
* suppressed forever (until storage is cleared).
*
* Independence from any upstream FUE: the storage namespace is
* `hapi.fue.v1.*` and feature IDs are caller-defined. If upstream/tiann
* adds a different onboarding flow that uses other keys / mechanisms,
* this system stays out of the way (caller decides whether to wrap a
* given affordance with FUE or not).
*/
const STORAGE_PREFIX = 'hapi.fue.v1.'
export type FueStatus = 'unseen' | 'engaging' | 'acknowledged'
function readAcknowledged(featureId: string): boolean {
if (typeof window === 'undefined') return false
try {
return window.localStorage.getItem(STORAGE_PREFIX + featureId) === '1'
} catch {
return false
}
}
function writeAcknowledged(featureId: string): void {
if (typeof window === 'undefined') return
try {
window.localStorage.setItem(STORAGE_PREFIX + featureId, '1')
} catch {
// localStorage may be unavailable (private mode, quota). Non-fatal:
// worst case the user sees the badge again next session.
}
}
export function useFue(featureId: string): {
status: FueStatus
/** Call this on the first user-initiated engagement (typically the
* affordance's onClick). No-op if already engaging or acknowledged. */
engage: () => void
/** Acknowledge permanently (call from the callout's "Got it" button or
* any other explicit affirmative action). */
dismiss: () => void
} {
const [status, setStatus] = useState<FueStatus>(() =>
readAcknowledged(featureId) ? 'acknowledged' : 'unseen'
)
// Re-read on featureId change (different feature, different state).
useEffect(() => {
setStatus(readAcknowledged(featureId) ? 'acknowledged' : 'unseen')
}, [featureId])
const engage = useCallback(() => {
setStatus((prev) => (prev === 'unseen' ? 'engaging' : prev))
}, [])
const dismiss = useCallback(() => {
writeAcknowledged(featureId)
setStatus('acknowledged')
}, [featureId])
return { status, engage, dismiss }
}
/**
* Test / dev-tool helper: clear acknowledgement for a feature so the FUE
* badge re-appears. Not used at runtime; expose via window for manual QA:
*
* localStorage.removeItem('hapi.fue.v1.scratchlist-toggle')
*
* Or call from a dev console: resetFue('scratchlist-toggle').
*/
export function resetFue(featureId: string): void {
if (typeof window === 'undefined') return
try {
window.localStorage.removeItem(STORAGE_PREFIX + featureId)
} catch {
// ignore
}
}
+136
View File
@@ -0,0 +1,136 @@
import { act, renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { addScratchlistEntry, persistScratchlist, readScratchlist } from './scratchlist'
import { useScratchlist } from './use-scratchlist'
const SESSION_A = 'session-a'
const SESSION_B = 'session-b'
describe('useScratchlist', () => {
beforeEach(() => {
localStorage.clear()
})
afterEach(() => {
localStorage.clear()
})
it('hydrates from localStorage on mount', () => {
const { entries: seeded } = addScratchlistEntry([], 'a-only', 1000)
persistScratchlist(SESSION_A, seeded)
const { result } = renderHook(({ id }: { id: string }) => useScratchlist(id), {
initialProps: { id: SESSION_A },
})
expect(result.current.entries.map((e) => e.text)).toEqual(['a-only'])
})
it('add() persists to the current sessions storage', () => {
const { result } = renderHook(({ id }: { id: string }) => useScratchlist(id), {
initialProps: { id: SESSION_A },
})
act(() => {
result.current.add('first')
})
expect(readScratchlist(SESSION_A).map((e) => e.text)).toEqual(['first'])
expect(readScratchlist(SESSION_B)).toEqual([])
})
it('switching sessions does NOT overwrite the new sessions storage with stale entries', () => {
// Regression test for the cross-session leak found by upstream review on PR #798.
// Seed both sessions distinctly; mount with A; rerender with B.
// The persist effect must not write A's entries into B's localStorage
// key during the brief render where the prop has changed but the
// rehydrate effect hasn't run yet.
const { entries: aEntries } = addScratchlistEntry([], 'a-original', 1000)
const { entries: bEntries } = addScratchlistEntry([], 'b-original', 2000)
persistScratchlist(SESSION_A, aEntries)
persistScratchlist(SESSION_B, bEntries)
const { rerender } = renderHook(({ id }: { id: string }) => useScratchlist(id), {
initialProps: { id: SESSION_A },
})
rerender({ id: SESSION_B })
// After the session switch, B's storage must still contain B's
// entry (not A's). Reading from disk because that's what the next
// mount of any other component would see.
expect(readScratchlist(SESSION_B).map((e) => e.text)).toEqual(['b-original'])
// A's storage stays intact too.
expect(readScratchlist(SESSION_A).map((e) => e.text)).toEqual(['a-original'])
})
it('after switching sessions, add() targets the new session', () => {
const { entries: aEntries } = addScratchlistEntry([], 'a-original', 1000)
persistScratchlist(SESSION_A, aEntries)
const { result, rerender } = renderHook(
({ id }: { id: string }) => useScratchlist(id),
{ initialProps: { id: SESSION_A } }
)
rerender({ id: SESSION_B })
act(() => {
result.current.add('b-only')
})
expect(readScratchlist(SESSION_B).map((e) => e.text)).toEqual(['b-only'])
expect(readScratchlist(SESSION_A).map((e) => e.text)).toEqual(['a-original'])
})
it('switching sessions never writes the previous sessions entries to the new sessions storage key', () => {
// The bot's review on PR #798 specifically called out the write
// window: between commit-with-new-id and the rehydrate effect
// running, the persist effect can fire one corrupting write
// (sessionId=B, entries=A's). That write self-heals on the next
// render once the rehydrate completes, so a "read after rerender"
// assertion would falsely pass. This test inspects every setItem
// call that happens during the rerender lifecycle and asserts no
// call wrote A's entries to B's storage key.
const { entries: aEntries } = addScratchlistEntry([], 'a-original', 1000)
const { entries: bEntries } = addScratchlistEntry([], 'b-original', 2000)
persistScratchlist(SESSION_A, aEntries)
persistScratchlist(SESSION_B, bEntries)
const { rerender } = renderHook(({ id }: { id: string }) => useScratchlist(id), {
initialProps: { id: SESSION_A },
})
const setItemSpy = vi.spyOn(Storage.prototype, 'setItem')
rerender({ id: SESSION_B })
// Storage format is a top-level array of entries (see writeScratchlist
// in scratchlist.ts), so unpack and inspect each entry directly.
const corruptingWrites = setItemSpy.mock.calls.filter(([key, value]) => {
if (typeof key !== 'string' || typeof value !== 'string') return false
if (!key.endsWith(SESSION_B)) return false
try {
const parsed = JSON.parse(value)
if (!Array.isArray(parsed)) return false
return parsed.some(
(e: { text?: string }) => e?.text === 'a-original'
)
} catch {
return false
}
})
setItemSpy.mockRestore()
expect(corruptingWrites).toEqual([])
})
it('remove() and move() use the loaded sessionId', () => {
const { entries: seeded } = addScratchlistEntry([], 'first', 1000)
const { entries: seeded2 } = addScratchlistEntry(seeded, 'second', 2000)
persistScratchlist(SESSION_A, seeded2)
const { result } = renderHook(({ id }: { id: string }) => useScratchlist(id), {
initialProps: { id: SESSION_A },
})
const firstId = result.current.entries[0]!.id
act(() => {
result.current.remove(firstId)
})
expect(result.current.entries.map((e) => e.text)).toEqual(['first'])
expect(readScratchlist(SESSION_A).map((e) => e.text)).toEqual(['first'])
})
})
+84
View File
@@ -0,0 +1,84 @@
import { useCallback, useEffect, useState } from 'react'
import {
addScratchlistEntry,
deleteScratchlistEntry,
moveScratchlistEntry,
persistScratchlist,
readScratchlist,
type ScratchlistEntry,
} from '@/lib/scratchlist'
/**
* useScratchlist - per-session scratchlist state hook.
*
* Originally the entries lived inside ScratchlistPanel's useState. The
* composer-controlled drawer (v1.1) needs the same data exposed in two
* places (the drawer + the composer-toolbar counter), so the state is
* lifted here. localStorage stays the source of truth; this hook is the
* React mirror.
*
* Cross-session race protection
* -----------------------------
* The naive shape (entries: useState, sessionId: prop, two useEffects)
* leaks across session navigation:
*
* 1. Mount with sessionId=A → entries = readScratchlist(A) = [a1, a2]
* 2. Parent rerenders with sessionId=B (same component instance — the
* v1 panel sidestepped this with key={props.session.id}; the v1.1
* lifted hook can't, because its parent SessionChat *isn't*
* remounted on session switch).
* 3. React commits with sessionId=B but `entries` is still A's data.
* 4. Persist effect fires: persistScratchlist(B, [a1, a2]) —
* OVERWRITES B's storage with A's entries before the rehydrate
* effect has a chance to run.
*
* Fix (per upstream review on PR #798): keep the loaded sessionId in
* state alongside the entries so they can swap atomically, and persist
* against the LOADED sessionId, not the current prop. After step 2 the
* loaded sessionId is still A (until the rehydrate effect runs), so a
* spurious persist re-writes A's storage with A's entries — a no-op
* instead of a corruption.
*/
export function useScratchlist(sessionId: string) {
const [{ sessionId: loadedSessionId, entries }, setScratchlist] = useState<{
sessionId: string
entries: ScratchlistEntry[]
}>(() => ({ sessionId, entries: readScratchlist(sessionId) }))
// Rehydrate when the parent navigates to a different session. This
// atomically swaps both the loaded sessionId and the entries, so the
// persist effect below sees a consistent (sessionId, entries) pair.
useEffect(() => {
setScratchlist({ sessionId, entries: readScratchlist(sessionId) })
}, [sessionId])
// Persist using the LOADED sessionId, not the prop. If the prop has
// moved ahead of the rehydrate effect, this still writes back to the
// session whose entries we currently hold — no cross-session leak.
useEffect(() => {
persistScratchlist(loadedSessionId, entries)
}, [loadedSessionId, entries])
const add = useCallback((rawText: string): boolean => {
const result = addScratchlistEntry(entries, rawText)
if (result.entries === entries) return false
setScratchlist({ sessionId: loadedSessionId, entries: result.entries })
return true
}, [entries, loadedSessionId])
const remove = useCallback((id: string) => {
setScratchlist((prev) => ({
sessionId: prev.sessionId,
entries: deleteScratchlistEntry(prev.entries, id),
}))
}, [])
const move = useCallback((id: string, direction: 'up' | 'down') => {
setScratchlist((prev) => ({
sessionId: prev.sessionId,
entries: moveScratchlistEntry(prev.entries, id, direction),
}))
}, [])
return { entries, add, remove, move }
}