feat(desktop): add Bestie sidebar entry - #6905
Conversation
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed by :bot: Jude’s code review agent at exact head 82051038a1e11cf17329f7325657625ab79b115c against stacked base 3b77a1a2d6ed6d7d77507877634c06c8cf625af5.
Blocking findings
-
[P2] Bind DM opening to the relay and signer that rendered the row.
BestieSidebarMenuItemresolves against the active relay, but its click passes only the selected pubkey (desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx:205-225). The AppShell callback likewise forwards only{ pubkeys }, awaits the open, and then navigates (desktop/src/app/AppShell.tsx:867-873). The existing fail-closed API already acceptsexpectedRelayUrlandexpectedSignerPubkey(desktop/src/shared/api/tauriChannels.ts:174-190;desktop/src-tauri/src/commands/dms.rs:21-43). A click racing a community or identity switch can therefore execute in the backend scope that wins the race and its stale continuation can navigate after the switch.Author action: capture the rendered relay and signer, pass both expected values through the open call, and add a delayed-open regression: click in community A, switch to B before resolution, then prove no B-scoped DM and no stale navigation; retain the ordinary A happy path.
-
[P2] Put the build-gated regression in a required CI lane. The new tests are isolated behind
desktop/playwright.bestie.config.ts, anddesktop/package.json:23adds the only caller,test:e2e:bestie. A scoped search of.github,Justfile, rootpackage.json, anddesktopfound no checked-in CI/Just caller. Ordinaryjust ciand Desktop smoke jobs can therefore remain green while the Bestie row, ordering, click, or opt-out behavior regresses.Author action: wire the eligible-build Bestie suite into a canonical required CI lane and prove that lane executes both tests.
Verification owner: author for fixes/regressions and CI wiring; reviewer for refreshed exact-head scope tracing and CI evidence.
Integrated exact-head evidence
- Desktop unit suite: 5,679/5,679 passed.
- Bestie Playwright suite: 2/2 passed when invoked directly with the eligible build command.
- Desktop typecheck and build passed in the product lane;
git diff --checkpassed; final reviewer trees were clean. - The full-row button, row placement, fail-closed feature mount, missing-persona hiding, and ordinary DM navigation path revealed no additional material defect.
Dependencies and residual risk
The consumed #6903 resolver also remains blocked: its relay normalization diverges from the repository's backend-compatible canonical semantics and does not define the inherited empty-relay case. Separately, #6902/#6903 already have unresolved exact-head requests for changes. Those are dependency gates, not substitutes for the PR-caused click-race and CI-coverage defects above.
No native Tauri artifact, AX snapshot, collapsed-sidebar/narrow-window pass, or pixel-level visual artifact was observed. Those remain reviewer/tooling confidence gaps, not additional author work.
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Reviewed 3b77a1a2d6ed6d7d77507877634c06c8cf625af5..82051038a1e11cf17329f7325657625ab79b115c at exact live head 82051038a1e11cf17329f7325657625ab79b115c (clean tree). Risk is high because this visible entry initiates a privileged DM-open across community and signer boundaries.
Blocking findings
-
The Bestie click is not bound to the community and signer for which the row was rendered.
BestieSidebarMenuItemcaptures only the agent pubkey and invokesonOpenDm({ pubkeys })(desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx:205-225); the AppShell callback likewise forwards onlypubkeys, awaits the native command, and then navigates (desktop/src/app/AppShell.tsx:867-873). This leaves a race where a click rendered under community/identity A can execute after a switch against B, and its stale continuation can navigate in B. The existing contract already supports fail-closed binding viaexpectedRelayUrlandexpectedSignerPubkey(desktop/src/shared/api/tauriChannels.ts:174-190; enforced indesktop/src-tauri/src/commands/dms.rs:21-43).Author action: capture the rendered active relay and signer and pass both expectations through the click/open call. Add a delayed-open regression that clicks in A, switches to B before resolution, and proves no B DM and no stale navigation; retain the ordinary A-path assertion.
-
The Bestie E2E is isolated from every checked-in CI/Just gate searched.
desktop/package.json:23adds a bespoketest:e2e:bestiecommand anddesktop/playwright.bestie.config.ts:8selects only this spec, but a scoped search of.github,Justfile,desktop, and the root package manifest found no caller. Ordinary Desktop CI can therefore remain green when the new row, ordering, click, or opt-out behavior regresses—the protective test is green by absence.Author action: add this command to the canonical eligible/internal-build CI lane, or include the spec in an existing CI project built with
VITE_BUZZ_BESTIE=1; provide an exact-head job showing both rows executed. -
Stack dependency #6903 does not implement the canonical active-relay contract consumed here.
desktop/src/features/agents/lib/bestie.ts:7-30only trims/lowercases, while the repository's backend-compatible canonicalizer folds loopback aliases and default ports (desktop/src/features/agents/managedAgentRuntimeStatus.ts:65-93). It also excludes records whose empty relay means “resolve to the active workspace at read time” (desktop/src-tauri/src/commands/agents.rs:453-461). Consequently an eligible Bestie can disappear—or a fallback can be selected incorrectly—forlocalhost/127.0.0.1, default-port, inherited-relay, or invalid-relay cases.Author action / owner: #6903 must use the shared canonical semantics and explicitly define/test empty inherited relay behavior, including alias/default-port, A/B collision, empty-relay, and invalid-relay cases. This stacked PR remains blocked until that dependency head is corrected and re-reviewed.
What was verified
The UI/product trace found no additional material defect: the row is immediately below Agents, the feature gate fails closed unless build availability and opt-in are both effective, loading/error/missing-persona states mount no dead control, and the established native button primitive preserves keyboard/focus/name behavior (desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx:165-237).
Independent exact-head validation on the pinned Hermit toolchain:
git diff --check 3b77a1a2d...HEAD— passcd desktop && pnpm check— exit 0 (pre-existing informational Biome diagnostics)cd desktop && pnpm typecheck— passcd desktop && pnpm test— 5,679/5,679 passcd desktop && pnpm build— passcd desktop && pnpm test:e2e:bestie— 2/2 pass (enabled happy path + explicit opt-out)
Current GitHub state at review time: #6905 Desktop Core and four smoke shards were still in progress; #6903 had failed Desktop / Smoke E2E (3). Those states do not displace the source findings above.
Verification owners: author for code, regression mutation, and CI wiring; reviewer to recheck exact-head scope propagation and executed CI coverage; #6903 author/reviewer for dependency correction.
Residual confidence gaps: no native Tauri artifact, screenshot/MP4/AX snapshot, narrow/collapsed-sidebar pass, or pixel-level alignment/contrast inspection was performed. Browser E2E proves DOM interaction/navigation against the mock bridge, not OS-delivered input or native rendering.
3b77a1a to
b8dc841
Compare
8205103 to
54ca696
Compare
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed by :bot: Jude’s code review agent at exact replacement head 54ca696dc20b2e3a9e26f8ac1888edd493fcae8a against base b8dc8412e55199e12d5d30ea1a31dae64cb4065f. The earlier requests for changes applied to superseded head 82051038a1e11cf17329f7325657625ab79b115c; every prior blocker was rechecked on this replacement.
Resolved blockers and integrated findings
No author-actionable defect remains.
- Relay and signer fail closed: the Bestie action is now bound to the rendered active relay and signer through
expectedRelayUrlandexpectedSignerPubkey. The native open contract validates that scope, and the navigation continuation rechecks the current canonical relay and signer after the async open completes. - Stale navigation is fenced: dedicated delayed-open coverage switches communities before resolution and proves the scoped request does not produce stale chat navigation. The ordinary enabled navigation path remains covered.
- Resolver dependency is corrected in the supplied base: active-relay selection now uses canonical relay semantics, rejects invalid/empty active relay, and covers loopback aliases, IPv6, and default-port handling. Selection remains scoped to the active community.
- The eligible-build regression now runs in canonical CI: exact-head GitHub Actions run
33026334582, job98368553451(Desktop Smoke E2E (1)), executedpnpm -C desktop test:e2e:bestie; the step reported 3 passed (6.5s). - Product/UI trace: row placement below Agents, build-plus-opt-in gating, fail-closed loading/missing behavior, native button keyboard/accessibility semantics, scoped DM navigation, and community/signer race handling revealed no further material defect.
Exact-head evidence
- Live PR head remained
54ca696dc20b2e3a9e26f8ac1888edd493fcae8a; base remainedb8dc8412e55199e12d5d30ea1a31dae64cb4065fat submission. - Canonical CI run
33026334582completed successfully at that head: Desktop Core, all four Desktop Smoke E2E shards, both Desktop E2E Integration shards, relay/backend integration, Rust, unit, mobile, security, cross-compile, macOS build, and aggregate required checks passed; Web was correctly skipped by path gating. - Independent pinned clean-tree Desktop validation passed 5,684/5,684 tests.
Author action: none.
Verification owner: release/product QA may optionally exercise a packaged native artifact and responsive/mobile/collapsed visual states. Those were not manually observed in this review and remain confidence gaps, not merge-blocking author work.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: b8dc8412e55199e12d5d30ea1a31dae64cb4065f..54ca696dc20b2e3a9e26f8ac1888edd493fcae8a (exact head 54ca696dc20b2e3a9e26f8ac1888edd493fcae8a)
Risk: high — a visible control opens a DM across active community and signer boundaries.
Behavior/contracts traced: build/opt-in gate propagation; active-community Bestie resolution; canonical relay matching; relay+signer-bound native DM open; post-await stale-navigation fencing; missing/loading behavior; row keyboard/AX semantics; canonical CI execution.
Findings: no unresolved author-actionable defect. The prior blockers are resolved: the caller supplies expectedRelayUrl and expectedSignerPubkey; scoped navigation rechecks the current canonical relay and signer after open_dm resolves; delayed A→B switching is covered without stale navigation; the supplied base provides canonical relay semantics; and the Bestie suite now executes in canonical CI.
Author action: none.
Verification owner: reviewer/release QA for any desired packaged-native or collapsed/responsive visual exercise; none for code correctness.
Validation: clean exact-head reviewer trees; git diff --check and focused Bestie test passed. Full Desktop unit suite passed 5,684/5,684. Exact-head Actions run 33026334582 completed successfully; job 98368553451 executed the Bestie experiment e2e step via pnpm -C desktop test:e2e:bestie with 3 passed. All required exact-head checks completed green (Web correctly skipped).
Manual/native evidence: source and browser E2E evidence covered placement, gating, native-button semantics, ordinary navigation, scoped payload, delayed community switch, and stale-navigation suppression. No packaged native artifact was manually exercised.
Residual risk: packaged-native and explicit collapsed/responsive pixel inspection remain optional reviewer/product-QA confidence work; no defect was established.
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>
On-behalf-of: mahanti <arjun.mahanti@gmail.com> Signed-off-by: Fizz <fizz@buzz.local>
On-behalf-of: mahanti <arjun.mahanti@gmail.com> Signed-off-by: Fizz <fizz@buzz.local>
On-behalf-of: mahanti <arjun.mahanti@gmail.com> Signed-off-by: Fizz <fizz@buzz.local>
b8dc841 to
3cf60f6
Compare
On-behalf-of: mahanti <arjun.mahanti@gmail.com> Signed-off-by: Fizz <fizz@buzz.local> Co-authored-by: Codex <noreply@openai.com>
54ca696 to
9bea9aa
Compare
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: 3cf60f63ac10617b5de2ee84b9fc1b4c2b1e57c8..9bea9aacccce509638173775b6eb36199875660b (exact head 9bea9aacccce509638173775b6eb36199875660b, clean reviewer tree)
Risk: high — the visible entry opens a DM across active-community/signer boundaries, and this stack enforces a compile-time boundary between protected and OSS artifacts.
Blocking finding
[P1] The OSS Desktop artifact now contains protected Bestie product code. Exact-head CI run 33185676054 reaches the artifact assertion and fails with Official OSS desktop artifact contains protected Bestie/Chief content in Desktop Core (98898131391), Desktop Windows Build (98898131396), and Desktop Build (macOS) (98898131471); the required aggregate Desktop check consequently fails too.
The OSS build selects the empty public feature manifest through the Vite alias (desktop/vite.config.ts:29-41; desktop/src/protectedFeatures/public.ts:3-7), but the PR imports and emits Bestie-specific sidebar implementation and text from ordinary application source (desktop/src/features/sidebar/ui/AppSidebarPinnedHeader.tsx:181-187,209-250). A runtime FeatureGate does not remove that implementation from the compiled bundle. The repository's artifact guard correctly detects it at desktop/scripts/build-protected-feature-artifacts.mjs:75-90.
Consequence: official OSS Desktop builds violate the protected-feature artifact contract and cannot pass required Linux, Windows, or macOS build/package gates.
Author action: move the Bestie sidebar implementation and product metadata behind the compile-time-selected protected-feature module boundary (or an equivalent compile-time contribution) so the OSS bundle contains no protected Bestie/Chief identifiers while the internal build retains the row. Preserve runtime opt-in gating within the internal path, and retain/prove the artifact-level matrix regression rather than only DOM absence.
Verification owner: author for the implementation and causal artifact regression; reviewer/required CI for fresh exact-head Desktop Core, Windows, macOS, and aggregate Desktop passes, plus retention of all three Bestie E2E scenarios.
Rechecked contracts and evidence
No additional author-actionable defect was found.
- The click captures rendered relay and signer expectations (
AppSidebarPinnedHeader.tsx:230-238); nativeopen_dmvalidates that scope; post-await navigation rechecks current canonical relay and signer (useScopedOpenDmNavigation.ts:23-43). - Exact-head Desktop Smoke E2E (1) executed all three Bestie tests: 3 passed (6.9s), including disabled-entry absence and delayed A→B switching without stale navigation. All four smoke shards and both Desktop integration shards passed.
- Sidebar visibility/placement, activation, native button semantics, loading/missing-agent behavior, opt-out behavior, and the changed-head hidden-DM resurface lifecycle revealed no further material defect.
git diff --check 3cf60f63ac10617b5de2ee84b9fc1b4c2b1e57c8..HEADpassed at the exact clean head. A local package rerun exceeded the tool timeout during dependency setup/build, so it is not claimed as validation; exact-head CI independently reproduces the blocking failure on three platforms.
Manual/native evidence: no packaged-app visual, screen-reader, or platform-specific pointer/keyboard exercise was completed.
Residual risk: native accessibility and pixel/platform behavior remain reviewer/release-QA confidence gaps, not additional author work. The concrete required-gate failure above is sufficient to block this head.
Summary
Flag-off contract
When Bestie is unavailable or the user has not opted in, this PR contributes no sidebar element or click behavior. The shared managed-agent roster may still be loaded by existing agent UI; this slice does not add a separate Bestie query.
Stack
am-bestie-02-persona-lifecycleVerification
just ci