Conversation
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Wait for each signing gate so real crypto completion cannot reorder the requests under test. Assert the waiting request is rejected instead of dispatched after cooldown. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Assert the fixture publication is accepted and the options are rendered before Arrow Up. The CI trace showed the key moving the textarea caret while React still had a pending completion render. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Arjun Mahanti <arjun@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
Reviewed e3ecb47706be008960a9a904868dfe77a177dd42 against stacked base a3eda453e713bd2ef0dbf79b8c5fcb1d6310545c (feat/bestie-realtime-voice, not main). Two introduced accessibility regressions need correction:
-
[P2] Preserve focused End when a call terminates asynchronously. Bestie.tsx:38–48, with the outgoing branch at line 113. Keyboard startup focuses End and consumes
restoreFocus; only explicit footer activation sets it again. If the microphone then disconnects while End remains focused,voice.mjs:482–489callsfail(), andcall.ts:302–306publisheserror. That makesbusyfalse and End inert/aria-hidden, without moving focus to the enabled Call replacement. The base retained one start/end button, so its focus survived this transition. Restore focus when the outgoing End actually owns it, without stealing focus from elsewhere. Add a regression that starts by keyboard, leaves End focused, triggers microphoneended, and expects Call to receive focus. This is a source trace, not a claim about a measured browser-specific fallback focus destination. -
[P2] Include the visible “Call” text in the button’s accessible name. Bestie.tsx:156–169. The new button visibly says
Call, butaria-label="Start Bestie voice conversation"overrides that text and contains no “Call”. The shared Button forwards this label unchanged. This violates Label in Name and prevents speech-input targeting from relying on the displayed wording. Use a name such asCall Bestie, updating the focus selector at line 45 and exact-name test expectations. The base control was icon-only, so this mismatch is introduced here.
Scope and validation
Audio/analyzer ownership and cleanup, call lifecycle, controls/menus, transcript rendering, approval placement, panel integration, and changed test contracts were reviewed on pinned objects. The independent compact-approval concern did not establish keyboard inaccessibility from source; keep short-panel keyboard approval scrolling as an acceptance check, not a proven security defect.
All 11 existing exact-head hosted checks succeeded; GitHub reports clean mergeability against the stacked base. Source-only review on Blox: no checkout, builds, tests, PR-code execution, or CI reruns. Physical-device/model feel and native acceptance were not exercised. This is not approval.
Bestie’s call panel now centers the companion and keeps call controls in a stable footer. Thinking and tool approval move into the header’s ellipsis menu; transcripts are hidden by default and can be toggled during a call.
Stacked on #37 and targeting
feat/bestie-realtime-voice, so this PR contains only the polish changes.Screenshots
Real UI with synthetic audio, isolated identities, and fixture text. All nine states, including connecting, speaking, muted, dark mode, and the compact layout.
Validation
All hosted checks passed at
e3ecb47: CI run, including all four browser shards, measurements, JavaScript, Rust/tool integration, and CI required. DCO and both security checks also passed.All 1,119 unit tests and TypeScript passed in pre-push at
e3ecb47.All 18 Bestie browser cases and all 30 autocomplete cases passed across Chromium and WebKit. All 16 layout/completion-layout cases and 14 design-system browser cases also passed.
Screenshot review found and fixed avatar overlap in short panels; regression coverage checks that the image stays above the transcript.
Local batch checks passed: 35 integration tests, frontend builds, lint/design checks, Rust formatting, and workspace Clippy. The broad browser run had one stale companion-copy assertion, repaired and verified by rerunning the complete affected files.
Fixed two test-ordering races found during validation: establish each asynchronous signer before releasing its gate, and wait for autocomplete retry options to render before keyboard navigation. No runtime changes, retries, skipped tests, or timeout increases.
Local native limitation: 8 of 9 native tests passed on this Mac. The real-login-shell probe is blocked by a managed security acknowledgement prompt. No shell policy or test timeout was changed. Hosted Rust and tool integration validates the native tests on Linux.
Model/hardware feel remains a manual check; fixture screenshots are not inference benchmarks.