[Juliusolsson05]

Follows CI for #1266 (head 0c962aa, which doesn't touch the picker). It failed ConversationsPicker > resumes the highlighted row under its own cwd and provider on Enter: replaceSession was called with row 0 instead of the highlighted row 1. Assessing that test found three real picker bugs, all fixed here. No issue was filed for them; this PR is their record.

What merges

• Highlight reset keyed by content, applied during render (steering q27).
  • The bug: the reset was keyed on the identity of the providers array, and the enablement effect rebuilds that array on every store update. So an enablement refresh with an unchanged filter moved the highlight back to row 0, and Enter resumed a conversation the user hadn't chosen.
  • The reset key is now content: the head row's id, the query, the scope, the providers' contents, includeChildren and stale.
  • It runs during render ("adjust state when a prop changes"), not in a passive effect.
  • Because stale is in the key, the highlight also resets when the fresh page lands (review C1).
• Keyboard ignored while the rows answer an older query (review A, round 1). useConversationList exposes stale: the response key differs from the current parameters (cwd, scope, providers, includeChildren, trimmed query).
  • While stale, the picker ignores ArrowUp, ArrowDown and Enter, unless a focused control owns Enter.
  • Without this, Enter resumed a row the new scope excludes, or a row the arriving page then replaced.
  • Mouse clicks on visible rows still work, by design.
• No paging while stale (review A, round 2). loadMore does nothing while stale. Before, a scroll during a filter change fetched the old query's next page, appended it to the old rows and labelled the mix as the new scope.
• Hover ignored while stale (review C1). A hover over an old row used to stick when the new page kept its head, so Enter resumed the row that replaced it.
• "loading…" while stale. The status line says so as soon as the rows are stale, which explains why the keys are ignored.
• The CI test flushes pending effects, presses ArrowDown and waits for row 1's aria-selected before Enter. No timeout is widened.

Tests

ConversationsPicker.renderer.test.tsx has 18 tests. They cover:

• the content-keyed reset, fail-first on main;
• the held-request stale guard for Enter and the arrow keys;
• the reset to row 0 on a new query;
• a hover on old rows during the wait (review C1);
• paging while stale, driven by a scroll of the list. An ArrowDown is swallowed by the stale key guard before it reaches loadMore. The scroll version fails with the stale guard in loadMore removed.

npx tsc -b is clean.

Residual

• feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 rewrites this picker's key handling without stale. The integration requirement is posted on feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221: every navigation key and hover must respect stale, with one reset.
• The stale guard's key paths are exercised mainly through the scope chip, and the reset through the query. The other axes (providers, includeChildren, cwd) are unpinned. Per-axis tests are a possible follow-up. A test comment claims to pin that an append does not reset the highlight; it does not.
• A trailing space resets the highlight: the reset key uses the raw query, while the stale key uses the trimmed one.
• If the commanded pane disappears while the picker is open, the status stays "loading…" and the keys stay ignored until it closes.

🤖 Generated with Claude Code

[Juliusolsson05]

Final disposition (pre-merge), head 4533252 (the reviewed code plus a test fix and a main remerge). Two independent reviewers (A: picker keyboard selection; B: tests), two rounds each. Round 2 was on 5c91a90. The round-2 fix, a9138ad, came after that and was not re-reviewed.

Reviewer	Round 1	Round 2
A (keyboard selection)	FIX-BEFORE-MERGE	FIX-BEFORE-MERGE
B (tests)	FIX-BEFORE-MERGE	MERGE-READY

Findings and dispositions:

Round 2

• A1, major: loadMore during a filter change used the old cursor with the new scope, added the page to the old rows and labelled the mix fresh. That reopened Enter on an out-of-scope row.
  • Fixed in a9138ad: loadMore returns early while stale (useConversationList.ts). I checked this at the head.
  • The test gap is closed in the head's parent commit. The first version of "does not page old rows after the scope changed" pressed ArrowDown, which the stale key guard swallows before loadMore(). It now scrolls the list, which is the real path in, and it fails with if (stale) return removed (mutation-checked).
• A2, minor: arrow keys and Enter were silently ignored during the debounce, with no "loading" shown. Fixed in a9138ad: the status reads "loading…" whenever loading || stale. The new test asserts it.
• B1, minor (test gap): the stale guard and the reset key are each pinned on one axis only.
  • The stale guard is tested only through the scope chip. The query, providers, includeChildren and cwd axes are untested.
  • The reset key is tested only through the query. The scope, providers and includeChildren axes are untested.
  • Not fixed. The code lists all five axes in both keys. B suggested a follow-up; not a merge bar.

Round 1

• A1, major: Enter could resume a row from the previous scope while the new request was pending. Fixed in 291f56e: the hook exposes stale (the response key differs from the current parameters), and the picker ignores ArrowUp, ArrowDown and Enter while stale. Held-request tests cover it; A re-ran the probe in round 2 and it passed.
• A2, major: a delayed response could move the highlight onto a different conversation at the same index. Fixed in 291f56e by the same stale guard; A's probe passed in round 2.
• B1, major: the reset itself was not pinned; deleting it left every test green. Fixed in 291f56e with the test "moves the highlight back to row 0 for a new query". The mutants that delete the reset or drop the query from its key are now caught.
• B2, minor: a test comment still described the old passive-effect reset. Fixed in 291f56e.

Caveats / residuals:

• The per-axis test gaps listed above.
• Mouse clicks on stale rows are still allowed, by design.
• The CI ordering behind the original flake could not be reproduced locally. The reset during render is a safety measure, not a proven fix for that ordering.

The PR body is rewritten to describe what merges; it had described only the first test commit.

🤖 Generated with Claude Code

[Juliusolsson05]

Correction (steering q37): the disposition above covers only reviewers A and B. The plan requires three independent reviewers. A third reviewer (C, Grok) is now reviewing the final head, with focus on stale-row selection, paging and overlap with #1221. This PR will not merge until C's verdict is added here.

🤖 Generated with Claude Code

[Juliusolsson05]

Final disposition (pre-merge), head 6112e84. This replaces the A/B-only disposition posted earlier. Three independent reviewers:

• A (picker keyboard selection) and B (tests): two rounds each; round 2 was at 5c91a90.
• C (Grok, third reviewer): round 1 at 4533252, then a verification round at 6112e84.

Reviewer	Round 1	Round 2
A (keyboard selection)	FIX-BEFORE-MERGE	FIX-BEFORE-MERGE (at 5c91a90)
B (tests)	FIX-BEFORE-MERGE	MERGE-READY (at 5c91a90)
C (Grok)	FIX-BEFORE-MERGE (at 4533252)	MERGE-READY (at 6112e84)

Findings and dispositions:

Round 1, A and B (fixed in 291f56e)

• A1, major: Enter could resume a row from the previous scope while the new request was pending. Fixed. The hook exposes stale, and the picker ignores ArrowUp, ArrowDown and Enter while stale is true. A re-ran the probe in round 2, and it passed.
• A2, major: a delayed response could move the highlight onto a different conversation at the same index. Fixed by the same guard; A's probe passed in round 2.
• B1, major: nothing pinned the reset itself. Fixed by "moves the highlight back to row 0 for a new query".
• B2, minor: a stale test comment. Fixed.

Round 2, A and B

• A1, major: loadMore during a filter change used the old cursor with the new scope and marked the mixed rows fresh. Fixed:
  • a9138ad: loadMore returns while stale.
  • 5482be8: the test now scrolls, which is the real path; ArrowDown was swallowed by the key guard.
  • C confirmed that deleting if (stale) return fails the test.
• A2, minor: keys were ignored during the debounce without feedback. Fixed in a9138ad: the status reads "loading…" while loading || stale.
• B1, minor: the stale guard and the reset key are each pinned on one axis only. Not fixed; follow-up.

Round 1, C (at 4533252)

• C1, major: hovering a stale row set the highlight, and if the new page kept its head, Enter resumed the row that replaced it. Fixed in 4b066f7:
  • onHover ignores a stale list;
  • stale is in the reset key, so the highlight resets again when the fresh page lands;
  • the new test "does not carry a hover on old rows over to the new page" covers it.
    C verified this in round 2 with four variants and two mutants.
• C2, major: feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 (open) rewrites this picker's key handling and does not read stale. Handled by an integration note on feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 (2026-09-25 16:47Z). The note covers:
  • guarding before nav.onKeyDown, including PageUp/PageDown, Home/End and Ctrl+N/P;
  • ignoring hover;
  • one reset key with stale in it;
  • showing "loading…" while loading || stale.
    C judged this adequate.
• C3, minor: a trailing space resets the highlight, because the reset key uses the raw query while paramsKey uses the trimmed one. Not fixed.
• C4, minor: if the commanded pane disappears, stale stays true, the status stays "loading… · ↑↓ ↵ resume" and the keys stay dead until the picker closes. Not fixed.
• C5, minor: the PR body described the key contents wrongly. Fixed in the body. A test comment still claims to pin that an append does not reset the highlight, which it does not. Not fixed.

C round 2: no new findings. C1 fixed and C2 handled.

Residuals:

• The per-axis test gaps; C's trailing-space and lost-pane minors; the append comment.
• Mouse clicks on stale rows still resume, by design: the click resumes the row under the pointer.
• The CI ordering behind the original flake could not be reproduced locally, so the reset during render is a safety measure.
• feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221 must port the stale guard when it merges. The note is on feat(ui): make every surface keyboard-operable with compact key hints and consistent chrome #1221.

Merges through batch PR #1318 once every member passes its gate.

🤖 Generated with Claude Code