test(desktop): pin mention undo and status expiry smoke behavior - #7592
test(desktop): pin mention undo and status expiry smoke behavior#7592loganj wants to merge 1 commit into
Conversation
🔐 Codex Security Review
|
jedwards27
left a comment
There was a problem hiding this comment.
: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
left a comment
There was a problem hiding this comment.
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 withreinstateExcluded: 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 atdesktop/tests/e2e/persistent-agent-audience.spec.ts:595-636match those transitions and verify the outgoing recipient exactly. - Status expiry is scheduled from the user-status cache through
Date.now()andsetTimeout, 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, therole="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.
Keep two desktop smoke specs pinned to the behavior the app actually ships so the suite stops failing on an assertion the product never implemented and on a real-time expiry race. - persistent-agent-audience.spec.ts: after undoing the automatic mention of an agent, a manual remention from the mention menu must not reinstate the excluded audience address. Observe the old address exit before the manual selection and keep it absent after, with the outgoing message going to exactly that agent. Only the explicit "Automatically mention" action reinstates the address, which then persists across sends with the draft retained. - profile-custom-emoji-status.spec.ts: install the page clock before navigation, read the seed time inside the page, and give the seeded status a five-minute expiry, then fast-forward 301 seconds so the real expiration timer fires mid-dialog deterministically. Establish a live, dirty dialog baseline first; every post-expiry recovery assertion is unchanged. Test-only: no production, helper, or config changes. Local dependency provisioning is network-policy blocked, so these specs run first in normal CI. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com>
922d86e to
ae6ff22
Compare
|
Published mechanical base refresh at |
🤖
Summary
Make two desktop smoke tests reliably check existing behavior: manually mentioning an agent after turning off automatic mentions, and editing a status while the saved status expires. This changes only tests, not production UI behavior, helpers, or configuration.
The two-file repair is unchanged from its previous version. This branch includes current main, including the MinIO startup fix in #7599; that infrastructure change is not part of this PR's test-only diff.
Related issue
none found
Testing
Both specs exercise the UI through the existing test fixtures. Fresh normal CI for
ae6ff228f86f42d5203050c99bcb99cdce3a4626is pending; historical test results and review of the earlier version do not validate this combined tree. Local dependency installation was blocked by network policy in the authoring environment, so no new local pass is claimed.