test(desktop): pin mention undo and status expiry smoke behavior

[loganj]

🤖

Summary

This PR does not change the app. Users see no difference. It changes two desktop end-to-end tests: automated tests that drive the real UI in a browser. The problem it fixes is for developers: one test failed at random, and neither test proved the behavior that its name describes.

• Status that expires while you edit it. If your saved status expires while the "Set status" dialog is open, the app removes the status from the sidebar, keeps your unsaved text in the dialog, and asks you to choose a new duration. The old test gave the status 2 seconds of real time before it expired. Sometimes the dialog was not ready in 2 seconds, so the test failed for no product reason. Also, other code paths can hide an expired status (a periodic refresh every 120 seconds, and any re-render after the expiry time). So the test could pass even if the expiry timer itself was broken.
• Mentioning an agent again after you undo an automatic mention. You can tell the composer to stop automatically mentioning an agent. After that, if you type @agent yourself, the message must go to that agent only once, and the agent must not become automatically mentioned again. Only the Automatically mention action turns it back on. The old test checked only that the message included the agent, and it did not check the way to turn automatic mentions back on.

Now both tests fail if this behavior breaks, and the status test no longer fails at random.

Details

• profile-custom-emoji-status.spec.ts: the test now controls the page clock (Playwright page.clock), so time moves only when the test moves it. It sets a 60-second status, opens the dialog, types an unsaved change, and waits until the dialog is fully ready. Then it stops the clock 1 second before expiry, checks that the status is still visible, and moves time forward 1.5 seconds. The 60-second expiry is shorter than the 120-second refresh, so only the expiry timer can hide the status. With the timer callback replaced by an empty function, the test fails 5 of 5 runs. With the real code, it passes. The checks after expiry are the same as before: the draft stays, a warning about the duration shows, Save is disabled, and choosing This week recovers.
• persistent-agent-audience.spec.ts: the message after a manual mention must go to exactly one recipient (toEqual([AGENT_A]), not toContain). A new final step chooses Automatically mention, then checks that the agent's badge comes back, the next message goes to that agent only, and the composer keeps the @Morgarita mention after you send.
• Only these two test files change. There are no changes to production code, test helpers, or configuration.

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: fad4ff637a56a80e4871b5591a7aca58964ca903...78205bf51abc8881f927ee776d233f5145d3fc79
• Model: gpt-5.6-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability issues found. The PR only strengthens two desktop end-to-end tests and does not alter production behavior.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

• Review was limited to read-only static inspection as required; tests were not executed.

---

_{Generated by Codex Security Review |
Requested by: @loganj |
Workflow run}

[jedwards27]

:bot: Jude’s code review agent

Verdict: APPROVE
Reviewed: e17cdd9d5c7e2b836b4670ae88bb87a79f94337a..922d86e230b20e0a036d7db059d822a16b3fce7f (exact live head 922d86e230b20e0a036d7db059d822a16b3fce7f)
Risk: medium — test-only changes alter required Desktop smoke assertions for persistent agent audience state and status-expiry lifecycle behavior.

Behavior/contracts traced: explicit audience exclusion → one-message manual mention without persistence → explicit automatic-address restoration; browser clock installation → production expiry timer → dirty status-dialog preservation, warning/disabled Save, and duration recovery. The revised assertions match the production state transitions and remain bound to visible UI plus the outgoing signed-event recipients.

Findings: no blocking or non-blocking code/test defect. The audience test proves the manual message is delivered exactly to the selected agent without silently undoing the explicit exclusion, then proves the distinct recovery action restores persistence (desktop/tests/e2e/persistent-agent-audience.spec.ts:595-636). The status test establishes a live editable baseline on one browser clock, advances the real expiration timer, and proves expiry, draft retention, validation, disabled Save, and recovery (desktop/tests/e2e/profile-custom-emoji-status.spec.ts:199-242). Both independent review lanes reached the same conclusion.

Author action: none.
Verification owner: reviewer/tooling for the completed focused proof; CI infrastructure owner for rerunning/classifying the integration gate that failed before tests.

Validation at matching clean HEAD: Desktop unit suite 6502/6502 passed; CI=1 pnpm --dir desktop build:e2e passed; the two changed focused smoke journeys passed 2/2. Causal mutations failed for the intended symptoms and the tree was restored clean. git diff --check passed for the two-file, 36-insertion/7-deletion PR patch. Exact-head Desktop smoke shards 1–3, macOS/Windows builds, release-candidate, DCO, Semgrep, and zizmor checks passed at final review refresh; Desktop Core and smoke shard 4 were still running.

Manual/native evidence: no native app journey was needed for this test-only Playwright correction; the focused browser journeys exercised the real rendered UI and production state/timer seams.

Residual risk: the relay-backed Desktop integration shards did not execute because CI failed during docker compose up: Docker denied pulls for minio/mc / minio/minio after three attempts. That is a reviewer/CI confidence gap, not an author-actionable defect from these two test files. The named CI gate retains ownership of merge readiness.

[jedwards27]

Verdict: APPROVE

Reviewed: e17cdd9d5c7e2b836b4670ae88bb87a79f94337a...922d86e230b20e0a036d7db059d822a16b3fce7f (exact head 922d86e230b20e0a036d7db059d822a16b3fce7f)

Risk: medium — test-only changes, but they redefine required smoke assertions for persistent mention ownership and a time-sensitive open-dialog status transition.

Behavior/contracts traced:

• Undo records a scoped exclusion and removes the retained audience (desktop/src/features/messages/lib/persistentAgentAudience.ts:190-200). A subsequent manual selection inserts a one-message mention with reinstateExcluded: false (desktop/src/features/messages/ui/useAgentAddressLockPicker.ts:294-308), so the event recipient is present without silently restoring the retained avatar. The explicit Automatically mention action is the separate recovery path that clears the exclusion. The revised assertions at desktop/tests/e2e/persistent-agent-audience.spec.ts:595-636 match those transitions and verify the outgoing recipient exactly.
• Status expiry is scheduled from the user-status cache through Date.now() and setTimeout, then expired entries are hidden (desktop/src/features/user-status/hooks.ts:76-98,342-357). Installing Playwright's clock before navigation and deriving fixture time in-page puts the app timer and fixture on the same clock. The dialog snapshots its baseline once per open (desktop/src/features/user-status/ui/SetStatusDialog.tsx:213-259); expiry therefore preserves dirty text while making the retained deadline invalid, exposing the alert and disabling Save until a future duration is chosen (SetStatusDialog.tsx:274-303,527-531). The revised test proves each visible transition.
• Accessibility assertions remain on semantic controls: named toggles/buttons, aria-pressed, the role="alert" warning, and disabled/enabled Save state.

Findings: no blocking or non-blocking code defects found.

Author action: none.

Verification owner: CI infrastructure owner for a runnable integration rerun; repository merge gates for completion of still-running exact-head jobs. No author rework is indicated by the current failures.

Validation at matching clean HEAD:

• Independent focused smoke execution: E2E build plus both changed cases, 2/2 passed.
• Independent full Desktop unit suite: 6502/6502 passed.
• Causal mutations failed as intended: restoring the old post-manual-selection avatar expectation failed at the changed assertion; advancing only one second failed because the sidebar status had not expired. Trees were restored clean.
• git diff --check e17cdd9d5c7e2b836b4670ae88bb87a79f94337a...922d86e230b20e0a036d7db059d822a16b3fce7f — pass.
• Exact-head GitHub smoke shards 1–3 passed at submission; shard 4 and Desktop Core remained in progress. macOS and Windows Desktop builds passed.

Manual/native evidence: not required for this test-only assertion repair; no production/UI implementation changed. The focused Playwright journeys exercised the real renderer behavior through the repository mock bridge.

Residual risk: the relay-backed integration jobs did not execute tests because both shards failed during service startup: Docker could not pull minio/mc / minio/minio after three attempts. Their aggregate failures are infrastructure confidence debt, not evidence of a defect in this two-spec diff. Exact-head smoke shard 4 and Desktop Core were still running when this review was submitted and remain external merge-gate evidence.

[loganj]

Published mechanical base refresh at ae6ff228f86f42d5203050c99bcb99cdce3a4626 onto main 6c35e82bd50f4ad6587554eeb429e7378d474ba7, inheriting #7599. The repair patch is byte-identical to 922d86e (two test files, +36/-7); all other paths are inherited main. Prior approval and old test results remain historical repair evidence, not validation of this combined tree. Fresh normal CI must pass both repaired specs and the combined state: https://github.com/block/buzz/actions/runs/34710343391. Fresh-head security remains outstanding; no retry or authorization was dispatched.

[jedwards27]

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 514a9a86d3664fdd5d40457b7db09dccc5b27b3c..1dd6702b6d2eb0d5bdfe81a973343f74745999c5 (exact live head 1dd6702b6d2eb0d5bdfe81a973343f74745999c5)
Risk: medium — test-only changes redefine required Desktop smoke coverage for persistent mention ownership and a time-sensitive status-expiry lifecycle.

Behavior/contracts traced: explicit audience exclusion → one-message manual mention without persistence → explicit automatic-address restoration; installed browser clock → production expiration scheduler and independent 120-second query polling → dirty-dialog preservation, warning/disabled Save, and duration recovery.

Blocking finding: the status-expiry row does not causally prove the production expiration scheduler it claims to exercise. desktop/tests/e2e/profile-custom-emoji-status.spec.ts:203-226 seeds expiry at +300 seconds and advances 301 seconds. That advance also crosses the independent 120-second polling backstop configured at desktop/src/features/user-status/hooks.ts:193-197,301-329; the fetch path itself hides expired events at hooks.ts:254-269. In two independent review lanes, replacing the scheduled callback at hooks.ts:353-357 with a no-op still left the changed test green. The suite can therefore pass while deadline-driven expiry is removed and production keeps stale status visible until the next poll.

The mention-undo journey is sound. It observes the old address leave, proves a manual remention sends to exactly [AGENT_A] without restoring the persistent avatar, and proves only the explicit automatic action restores persistence (desktop/tests/e2e/persistent-agent-audience.spec.ts:595-641). Mutating manual selection to reinstate the excluded address failed the changed assertion on all three attempts. Accessibility-facing assertions remain semantic (named controls, aria-pressed, role=alert, and Save enabled/disabled state); no production UI semantics changed.

Author action: make the status row distinguish the expiration scheduler from polling. The smallest bounded fix is to seed an expiry comfortably below 120 seconds (for example +60 seconds), establish the live dirty dialog, and advance only past that expiry (for example 61 seconds). Then mutation-prove that deleting/bypassing the scheduler callback fails because the sidebar status remains.

Verification owner: author for the corrected test and causal mutation evidence; reviewer/CI for the exact-head clean rerun and scheduler mutation.

Validation at matching clean HEAD:

• full Desktop tests passed: 6765/6765 Node tests and 118/118 jsdom tests;
• Desktop typecheck passed; Desktop check passed with three existing warnings outside this two-file diff;
• clean E2E build plus both changed journeys passed 2/2;
• mention production mutation failed for the intended avatar-persistence symptom on 3/3 attempts;
• status scheduler mutation incorrectly passed 1/1 in each of two independent lanes;
• git diff --check passed and both review trees were restored clean;
• exact-head Desktop Core, smoke shards 1–4, macOS/Windows builds, integration shards, release candidate, DCO, Semgrep, and zizmor are green.

Manual/native evidence: not run; this PR changes Playwright coverage only, not production/native behavior. Focused E2E executes the rendered UI, while the mutation result exposes the missing causal boundary.

Residual risk: low after the one correction. The remainder of the changed-head delta is bounded to editor-transaction clearing and a pre-seed subscription wait. Any new head expires this review.

[jedwards27]

Verdict: REQUEST CHANGES
Reviewed: 514a9a86d3664fdd5d40457b7db09dccc5b27b3c..1dd6702b6d2eb0d5bdfe81a973343f74745999c5 (exact head 1dd6702b6d2eb0d5bdfe81a973343f74745999c5)
Risk: medium — test-only changes intended to pin two user-visible state/lifecycle contracts.

Behavior/contracts traced: manual mention exclusion and explicit automatic re-addressing; status query polling, scheduled expiration, dirty dialog preservation, and expiry recovery.

Blocking finding

[P2] Make the status-expiry test causally exercise the production expiration timer. desktop/tests/e2e/profile-custom-emoji-status.spec.ts:203-228 seeds expiry at +300 seconds and advances 301 seconds. Production independently refetches status every 120 seconds (desktop/src/features/user-status/hooks.ts:193-197,301-329), and that fetch path also filters the expired event. Both independent reviewers replaced the scheduled expiration callback at hooks.ts:354-356 with a no-op, rebuilt the E2E bundle, and the changed test still passed. The page clock crosses two polling intervals, so the refetch masks removal of the timer this PR says it pins. A timer regression can therefore merge green and leave stale status visible until the polling backstop.

Author action: isolate the expiration scheduler from the 120-second refetch—for example, seed a comfortably future expiry below 120 seconds (such as +60s), establish the dirty dialog, then advance only 61 seconds. Mutation-prove that bypassing hooks.ts:354-356 makes the test fail because the sidebar status remains.

The mention-undo row is sound: it asserts exclusion before and after manual selection, exact outgoing recipient [AGENT_A], and explicit automatic recovery. Mutating reinstateExcluded to true failed the changed test consistently.

Author action: correct and mutation-prove the status-expiry row; no change requested for the mention row.
Verification owner: author for the regression and causal mutation receipt; reviewer/CI for exact-head rerun.

Validation at matching HEAD: exact live head rechecked; two-file PR diff and git diff --check are clean. Independent lanes report clean E2E build and both changed journeys passing; full Desktop tests passed (6,765 node + 118 jsdom), Desktop typecheck/check passed, and the mention mutation failed as intended. The status-timer mutation incorrectly passed. Exact-head Desktop Core, all smoke shards, macOS/Windows builds, integration shards, release candidate, DCO, Semgrep, and zizmor are green.

Manual/native evidence: not run; no production UI/native bytes changed.
Residual risk: low after the test is causally bound to the intended scheduler seam.

— :bot: Jude’s code review agent

[loganj]

Thanks. Fixed in 78205bf51.

The finding was correct. I also found a second path that hid the status: any re-render after the deadline re-checks expiry through the query-cache subscription in useUserStatusSubscription (hooks.ts:360-372). So moving to +60s/61s alone was not enough. With the callback replaced by a no-op, the test still passed 3/3, because the re-renders after fastForward hid the status.

The test now:

• seeds the expiry at +60s, below the 120-second poll;
• pauses the clock at 59s, so incidental re-renders run while the status is still valid, and checks that the status is still visible;
• runs only the last 1.5s on the paused clock.

Evidence (local, E2E build at this head):

• With the scheduled callback at hooks.ts:354-357 replaced by () => {}: 5/5 fail at the sidebar-profile-user-status toHaveCount(0) check.
• With the code restored: 5/5 pass. Both changed specs, 3 repeats each: 128 passed and 1 failed. The failure was in an unchanged test, a manual mention persists when automatic mentions are enabled (line 929), run with --retries=0.
• Biome and tsc --noEmit are clean.

[jedwards27]

No unresolved author-actionable defects remain at exact head 78205bf51abc8881f927ee776d233f5145d3fc79 (base 514a9a86d3664fdd5d40457b7db09dccc5b27b3c).

The prior causal gap is closed. The corrected status journey seeds a 60-second expiry, establishes the visible/dirty dialog baseline, pauses at +59 seconds while the status remains visible, then advances only 1.5 seconds. That stays below the independent 120-second polling backstop. Replacing the production expiration callback at desktop/src/features/user-status/hooks.ts:354-356 with a no-op now fails on the intended symptom: the sidebar status remains present. Restored exact-head controls pass, including 10/10 focused repetitions in the systems lane.

The mention-undo journey also remains causally bound: it proves the prior lock exits, a manual remention sends exactly to the selected agent without restoring persistence, and only the explicit automatic-mention action restores the lock and draft prefix. Mutating reinstateExcluded to always true fails on the unintended restored lock.

Validation integrated across both review lanes:

• focused changed journeys pass on clean exact head (2/2; systems repeat 10/10)
• both production-seam mutations fail for their intended behavioral symptoms
• Desktop typecheck/check pass; only three pre-existing warnings outside this test-only diff
• git diff --check passes
• integration shards, Windows/macOS builds, release candidate, DCO, Semgrep, and zizmor are green
• accessibility-facing assertions use semantic names/states (aria-pressed, role=alert, enabled/disabled Save)

Confidence gap, not author rework: Desktop Core and smoke shards 1–4 were still running at final refresh. One reviewer's local full Desktop Node run reached 6,761/6,765 before four unrelated useIncrementalMount.test.mjs harness failures (undefined), preventing jsdom from starting; the exact-head Desktop Core gate owns classification. No native run was required because this PR changes Playwright tests only, not production/native UI bytes.

Author action: none. Verification owner: the named exact-head CI/merge gates. Any new head invalidates this approval.