test: fix the two intermittent CI lane failures at their causes

[liugddx]

Summary

Fixes the two intermittent CI lane failures tracked in #3289 and #3290, each at its cause. No assertion is weakened and no timeout is raised.

Since the Apache migration a single flaked lane costs a full CI cycle plus a re-review round (gh run rerun needs admin; the only contributor recovery is an empty-commit push that cancels in-flight lanes and re-triggers every bot), which is why these two are worth fixing at the source rather than retrying around.

#3290 — computer-use ENOTEMPTY teardown race

maka-cu-service.test.ts's after() disposed every service and immediately rm -r'd the shared work directory. dispose() is deliberately fire-and-forget: it SIGTERMs the child and schedules an asynchronous image-directory purge on child exit (or on the 3s shutdown-grace SIGKILL), plus one more purge when an in-flight start() settles. The recursive rm therefore raced concurrent purges — and a child still flushing its ndjson log — and failed with ENOTEMPTY (maka-cu-service.ts dispose(), the purge closure).

Fix (test-side only; the production fire-and-forget contract is intentional and untouched):

• makeService now records each per-service image directory.
• after() waits (bounded, 10s; grace is 3s) for every image directory to vanish — each one is deleted by its service's own purge, so this is a deterministic "children exited and purges finished" barrier. The wait is tolerant rather than asserting, because purge failure is explicitly permitted by the production contract ("the next spawn purges it again") and is not what this teardown tests.
• The final rm(workDir) gets maxRetries: 10, retryDelay: 100 as a backstop for same-tree stragglers (e.g. a final log flush).

#3289 — slash-command e2e toBeVisible 10s timeouts

dd4b2d0 (#3291) already fixed the /side spec by removing the racy steering chain and documented the underlying race: after the composer remounts, fill() can land before the contentEditable is focused, so the draft never populates.

The remaining flaky spec, compacts the active session, hits the same race one step later: right after the compact completes (composer clears/remounts), it did fill('after compact') + Enter — if the fill lands before focus returns, Enter submits nothing and Fake backend received: after compact never appears. That is exactly the pinned failure: run 32157698387 attempt 1 fails at line 83 on getByText('Fake backend received: after compact'), 10s element(s) not found, after all preceding compact assertions passed.

Fix mirrors dd4b2d0 in the same file: click() + pressSequentially('after compact') + expect.poll(textContent).toBe('after compact') before pressing Enter — the dispatch only happens once the draft has demonstrably settled.

Verification

• L2 (local, Windows dev machine): @maka/computer-use builds (tsc) and the service test file runs its new teardown cleanly; pass/fail counts are byte-identical to unmodified main (4 pass / 13 fail, all pre-existing environmental spawn EFTYPE — Windows cannot spawn the .cjs mock directly; the Linux test lane is the authoritative runner for this suite). Desktop typecheck and biome pass.
• L3 (authoritative, CI): the Linux test and e2e lanes on this PR execute both changed suites for real. Local Playwright execution was attempted and is not possible on this machine (Electron firstWindow timeout at fixture level, before any test body, for all specs including unmodified ones).
• The flake is intermittent, so a single green lane proves non-regression, not the absence of the race; the determinism argument above is the actual fix rationale.

Closes #3289
Closes #3290

[hqhq1025]

No actionable findings.

Problem and root cause

• The computer-use test lane could fail during teardown because MakaCuService.dispose() deliberately completes child shutdown and image-directory cleanup asynchronously, while the suite immediately removed the shared work directory.
• The slash-command E2E could submit an empty draft after /compact because the composer is cleared/remounted and fill() could race its focus/state transition.

How this revision solves them

• The computer-use teardown now waits on the service-owned image directories to disappear, then performs a bounded recursive cleanup with retries.
• The E2E now clicks the remounted composer, types through the normal keyboard path, and verifies the observable draft text before submitting.

Assessment

• The problem definitions match the captured failures and the relevant production lifecycles.
• Both fixes follow first principles and Occam's razor: they synchronize on the state that must actually become true instead of increasing arbitrary sleeps, weakening assertions, or adding production compatibility paths.
• The current solution is appropriately scoped and optimal for these test-only races.
• Production code to delete: none; this PR does not change production code.
• Low-quality tests to delete or replace: none. The existing shutdown test still strictly verifies image-directory removal, so the more tolerant suite teardown does not become the sole coverage for that behavior.
• Deeper refactor required: no. The production ownership boundaries remain unchanged, and the fixes live at the test synchronization boundary where the races occur.

Verification

• CI test: passed at 9269c5fe7de3fc69c891129a77362a5518e23b06.
• maka-cu-service.test.js: 17/17 passed, then the complete file passed 10/10 repeated runs.
• Electron compacts the active session E2E: 5/5 repeated runs passed.
• Desktop workspace build, Biome checks for both changed files, and git diff --check: passed.

Merge verdict

Ready to merge at the reviewed head.

Residual risk: intermittent failures cannot be disproved absolutely by a finite repetition count. Future CI should still be watched for a different lifecycle race, but this revision directly addresses the two observed failure mechanisms without masking them.

[Astro-Han]

Thanks — both fixes are at the cause, and the write-up is the kind I wish more flake fixes had: it names the mechanism, cites the failing run and line, and is explicit about what the verification does and does not prove. Reviewed exact head 9269c5fe7de3fc69c891129a77362a5518e23b06.

The e2e half is right and consistent with precedent — it mirrors dd4b2d0 in the same file, and the diagnosis holds: after a compact the composer clears and can remount, fill() can land before the contentEditable regains focus, and Enter then submits an empty draft. Typing through the focused element and polling until the draft has settled makes the dispatch depend on observed state rather than on timing. The teardown half is also the right shape: dispose() being fire-and-forget is a deliberate production contract, so fixing this test-side rather than making dispose() awaitable is correct, and using each service's own image-directory purge as the barrier is a real signal rather than a sleep. I checked that existsSync is already imported, that every makeService gets a randomUUID-unique image directory so the barrier cannot alias, and that a service whose directory was never created simply passes immediately.

One note on verification, in your favour: the e2e run is a step inside the test check gated on the change plan, not a separate reported check — and since this PR touches apps/desktop/e2e/*.spec.ts, the green test at this head does cover the changed spec. The description reads as though a separate e2e lane should have reported; it did run, inside test.

One P3 inline. Approving — nothing here should hold the merge.

Review disclosure: this review was prepared with Claude Code, which read the diff at this head, checked the import and image-directory uniqueness against the file at head, and read the CI workflow to confirm how the e2e step is gated. Neither suite was executed locally. I reviewed the diff and the findings myself before posting; this approval is my own judgment.

[Astro-Han]

[P3] Consider waiting on child exit rather than on directory removal, so a contract-permitted purge failure does not cost ten seconds. The comment is right that purge failure is allowed ("the next spawn purges it again") and right that the tolerant wait plus the retrying rm is safe — but the two facts combine into a cost: whenever a purge legitimately fails, this loop cannot distinguish that from a purge still running, so it spins the full ten seconds before the backstop takes over, on a suite whose whole point in this PR is CI time. Waiting on the thing the barrier actually means — the children having exited — would return immediately in both cases, and the maxRetries rm would still absorb the log-flush stragglers it is there for. Not worth blocking on: the failure is rare and the outcome is slow rather than wrong.