[lenny] fix(desktop): keep a lone live persona instance on its canonical card by mfethe1 · Pull Request #34 · mfethe1/buzz

mfethe1 · 2026-09-11T09:25:54Z

The queue-blocking test failure has a real cause

exact-key-profile.spec.ts:84 "archived exact key stays navigable (live sibling)" has been failing on every open PR (#15, #31, #32, #33), consuming the desktop E2E gates and blocking the merge queue. It was being treated as an environment flake. It is not — it is a fork-local regression with a deterministic reproduction.

Root cause

buildPersonaCards (desktop/src/features/agents/ui/unifiedAgentGroups.ts) splits a persona into per-name cards whenever more than one display group exists, keying each split card <persona id>::<folded name>. It collapsed back to the canonical persona card only when zero live groups remained:

if (liveGroups.length === 0) return [personaOnlyCard(agents)];

The failing scenario is one archived instance ("Archived A") plus one live sibling ("Sibling B"):

displayGroups.length === 2 → takes the split path
liveGroups.length === 1 → does not collapse

So the persona's only card is keyed P::sibling b instead of P. The spec's persona-agent-row-${PERSONA} locator then points at a card that no longer exists, locator.click times out after 30s, and archived-profile navigation dead-ends for real users too.

The sibling case (all archived) passes because liveGroups.length === 0 collapses correctly — which is why only one of the two parametrised cases ever failed.

Fix

A split card only earns its own identity when a rival live name exists. With one live name left (or none), the persona's name again stands for every surviving instance, so it keeps its canonical card and key:

if (liveGroups.length <= 1) return [personaOnlyCard(agents)];

Provenance — this is ours, not upstream

The split-card logic is fork-local; desktop/tests/e2e/exact-key-profile.spec.ts is byte-identical to upstream/main. The regression came from 47cc90626d / 3acf014ab3, neither of which is an ancestor of upstream/main. We broke an upstream test.

Verification (local, this branch, on top of `adeec6eb5c`)

exact-key-profile.spec.ts (smoke): 4 passed — previously 3 passed / 1 failed, failing all 3 retries
Desktop unit suite (npm test): 6583 passed, 0 failed, 90 suites
unifiedAgentGroups.test.mjs: 20/20 passed

Reproduced the failure locally before the change and confirmed it green after, using pnpm build:e2e (the E2E bundle — note npm run build produces the OSS variant and is not what the specs serve).

Note for reviewers

This should unblock the desktop E2E gates across the other open PRs. The remaining rollup gates (Windows Rust, PostgreSQL Tests, etc.) are separate 3-second Check <name> result jobs that fail because their underlying builds are SKIPPED — that is the ruleset-satisfiability issue, unrelated to this fix.

…cal card `buildPersonaCards` split a persona into per-name cards whenever more than one display group existed, keying each split card `<persona id>::<folded name>`. It only collapsed back to the canonical persona card when *zero* live groups remained. With one archived instance and one live sibling, that left a single live group still travelling the split path, so the persona's only card was keyed `<persona id>::<name>` instead of `<persona id>`. Every deep link to the persona — `persona-agent-row-<persona id>` — then pointed at a card that no longer existed, and archived-profile navigation dead-ended. A split card only earns its own identity when a rival live name exists. With one live name left, the persona's name again stands for every surviving instance, so collapse on `liveGroups.length <= 1`. This regression is fork-local (47cc906, 3acf014); it broke the upstream spec exact-key-profile.spec.ts:84 "archived exact key stays navigable (live sibling)", which had been failing on every PR and blocking the whole queue. Verified: exact-key-profile.spec.ts 4/4 passed (was 3 passed / 1 failed); desktop unit suite 6583/6583 passed.

coderabbitai · 2026-09-11T09:26:03Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 50373c4f-6ebd-45d2-9c94-e6e9496d08bc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

mfethe1 · 2026-09-11T09:51:19Z

CI verdict: the queue-blocking failure is fixed (run 34584166278)

The test this PR targets now passes on the WSL runners.

Smoke shard 2 (job 103214573256) — the shard that carries exact-key-profile:

✓ 66 exact-key-profile.spec.ts:11:3 › explicit relay-only identity has no local controls (persona only) (2.2s)
✓ 67 exact-key-profile.spec.ts:11:3 › explicit relay-only identity has no local controls (local sibling) (2.1s)
✓ 68 exact-key-profile.spec.ts:84:3 › archived exact key stays navigable (live sibling) (1.8s)
✓ 69 exact-key-profile.spec.ts:84:3 › archived exact key stays navigable (all archived) (1.8s)

397 passed, 0 failed, 3 flaky (20.4m)

Shard 2 previously reported 399 passed / 1 failed with :84 (live sibling) exhausting all three retries. Shards 1 and 2 are SUCCESS.

Remaining red is pre-existing, not from this diff

Shards 3 and 4 are still red. Every genuine failure in them also fails on #32 and #33, which do not contain this change:

Shard	Genuine failure	Also fails on
3	`messaging.spec.ts:3105` narrow thread view collapses channel header actions	#32 (job `103147548175`), #33 (job `103164166171`)
4	`spoiler.spec.ts:256` masked link does not leak URL	#32 (job `103147548222`), #33 (job `103164166193`)
4	`thread-unread.spec.ts:962` mark-read-unread-menu-single-toggle	#32, #33
4	`workflow-local-controls.spec.ts:148` inserts template variables with keyboard control	#32, #33

Shard 3 = 289 passed / 1 failed; shard 4 = 338 passed / 3 failed / 7 flaky. This diff touches exactly one function, buildPersonaCards, and none of those specs exercise it.

Gate evidence

pnpm run check → exit 0 (biome, 2725 files, 0 errors; plus check-px-text, check-pubkey-truncation, check-pubkey-normalization, check-agent-identity)
pnpm run typecheck → exit 0 (tsc --noEmit)
Desktop unit suite → 6583 passed / 0 failed, 90 suites
unifiedAgentGroups.test.mjs → 20/20
Zero diagnostics mention unifiedAgentGroups in either gate log

The remaining shard-3/4 red is a separate pre-existing problem worth its own fix; it is not a regression from this PR and was blocking the queue before this branch existed.

mfethe1 · 2026-09-11T11:33:38Z

Merging with documented exception (owner-approved)

Approved by @mfethe1 in-session. Every failing check on this PR is attributed to a cause outside this diff:

Failing check	Test	Attribution
Desktop E2E Integration (all)	`agents.spec.ts:297` built-in persona edits persist	Pre-existing on base — fails identically on `origin/product/main` without this diff. Fixed by #36.
Smoke E2E (4)	`spoiler.spec.ts:256`	Pre-existing — fixed by #35 (not in this branch).
Smoke E2E (4)	`thread-unread.spec.ts:962`	Known open item, documented on #35 (live region vs `getByText`).
Smoke E2E (4)	`workflow-local-controls.spec.ts:148`	Order-dependent flake; passes isolated.
Smoke E2E (3)	`messaging.spec.ts:3105`	CI-shard-composition only. Proven to pass on this code: 3/3 isolated and in a full-file run on `product/main`; DOM probe shows gap 20 vs limit 52.
Smoke E2E (3)	`navigation.spec.ts:448`	Order-dependent flake.
`Desktop`, `Desktop Domain / Desktop`	—	Aggregate gates; fail only because of the above shards.

This diff's own effect is positive and measured. exact-key-profile.spec.ts differential, fresh build:e2e each time, retries=0:

origin/product/main → 1 failed / 3 passed
[lenny] fix(desktop): stop the timeline live region leaking spoiler contents #35 branch (without this fix) → 1 failed / 3 passed
this branch → 4 passed

That is why this PR merges first: it repairs exact-key-profile.spec.ts:84 (archived exact key stays navigable (live sibling)), which is currently failing on #35 and #36 too. #35 and #36 get re-checked against the new trunk after this lands.

mfethe1 · 2026-09-11T12:58:49Z

`thread-unread.spec.ts:962` is pre-existing on base — not caused by this PR

Completing the attribution table above. This was the last failure I had left as an open item; I've now run the differential rather than assuming.

Method: clean worktree at origin/product/main, fresh pnpm install --frozen-lockfile + build:e2e, --retries=0, 3 runs.

ref	run 1	run 2	run 3
`origin/product/main` (base)	1 failed	1 failed	1 failed

Signature reproduced verbatim against CI:

Error: locator.hover: Error: strict mode violation: getByText('Toggle me') resolved to 2 elements:
    1) <div role="status" class="sr-only" aria-atomic="true" aria-live="polite"
       data-testid="message-timeline-announcements">…
    2) <p>Toggle me</p> aka getByTestId('message-timeline').getByText('Toggle me')

Cause: TimelineAnnouncementRegion's announcementForMessage() emits `${author}: ${body}` → "Alice: Toggle me". getByText('Toggle me') substring-matches both the live region and the message body, so the locator is ambiguous.

Why it isn't ours: the component is already present on origin/product/main (git cat-file -e confirms), and none of #34/#35/#36 touch thread-unread.spec.ts (git diff --name-only = 0 for each).

Recommended fix (spec-side, for the spec owner): scope the locator — page.getByTestId('message-timeline').getByText('Toggle me'), or { exact: true }. The live region content is correct a11y behaviour and should not be changed to satisfy a loose locator.

With this, all six failures feeding the Desktop aggregate gate are attributed outside these three PRs' diffs.

This was referenced Sep 11, 2026

[lenny] fix(desktop): stop the timeline live region leaking spoiler contents #35

Merged

fix(desktop): restore the authored description on persona cards #36

Merged

mfethe1 enabled auto-merge September 11, 2026 11:29

mfethe1 merged commit 61361b8 into product/main Sep 11, 2026
78 of 95 checks passed

mfethe1 mentioned this pull request Sep 11, 2026

[lenny] fix(ci): run CI on product/main pushes #38

Merged

Navigation Menu

[lenny] fix(desktop): keep a lone live persona instance on its canonical card - #34

[lenny] fix(desktop): keep a lone live persona instance on its canonical card#34
mfethe1 merged 1 commit into
product/mainfrom
fix/lenny/persona-split-card-lone-survivor

mfethe1 commented Sep 11, 2026

Uh oh!

coderabbitai Bot commented Sep 11, 2026

Review skipped

Uh oh!

mfethe1 commented Sep 11, 2026

Uh oh!

mfethe1 commented Sep 11, 2026

Uh oh!

mfethe1 commented Sep 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mfethe1 commented Sep 11, 2026

The queue-blocking test failure has a real cause

Root cause

Fix

Provenance — this is ours, not upstream

Verification (local, this branch, on top of adeec6eb5c)

Note for reviewers

Uh oh!

coderabbitai Bot commented Sep 11, 2026

Review skipped

Uh oh!

mfethe1 commented Sep 11, 2026

CI verdict: the queue-blocking failure is fixed (run 34584166278)

Remaining red is pre-existing, not from this diff

Gate evidence

Uh oh!

mfethe1 commented Sep 11, 2026

Merging with documented exception (owner-approved)

Uh oh!

mfethe1 commented Sep 11, 2026

thread-unread.spec.ts:962 is pre-existing on base — not caused by this PR

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Verification (local, this branch, on top of `adeec6eb5c`)

`thread-unread.spec.ts:962` is pre-existing on base — not caused by this PR