Skip to content

Commit 76a320a

Browse files
antonoantono
committed
test(e2e): sync status redesign coverage + Stalwart skill docs
- rewrite status-bar.spec.ts for the morph UI; add a real-Stalwart test proving server→SSE→event-log end-to-end (fixme completing-sync + clear-history cases blocked by Stalwart cleartext-auth) - update sync-trigger.spec.ts to the dot→button→spinner flow - document the Stalwart IMAP sidecar + cleartext-auth limit in the mailbrus-e2e-author skill - update sync-status-bar-redesign proposal/tasks (scope decision + progress)
1 parent 3116f84 commit 76a320a

5 files changed

Lines changed: 326 additions & 147 deletions

File tree

‎.agents/skills/mailbrus-e2e-author/SKILL.md‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ harness is already built; your job is to write idiomatic specs that use it
1111
correctly and that stay in lockstep with the manifest and the OpenSpec
1212
behaviour they verify.
1313

14+
**Sync tests need a real IMAP backend, not mocks.** When a spec exercises the
15+
sync pipeline (triggering a sync, asserting `/api/sync/stream` events, the status
16+
bar spinner / event log), it spins up an ephemeral **Stalwart** IMAP/JMAP sidecar
17+
via `e2e/harness/stalwart.ts` and points a per-test account at it — do **not**
18+
intercept `/api/sync` or fake the SSE stream with Playwright routes. See the
19+
dedicated section "Sync tests: the Stalwart IMAP sidecar" below for the full
20+
recipe and the current cleartext-auth limitation.
21+
1422
Architecture, lifecycle, fixture manifest, the Node-vs-Deno split, and the
1523
known UI gaps are documented in [`docs/e2e-testing.md`](../../../docs/e2e-testing.md).
1624
Read it once if you haven't — the rest of this skill assumes that context.
@@ -201,6 +209,83 @@ exist yet:
201209
that's the contract working. Fix the manifest, regenerate, until green.
202210
4. Never hand-edit a generated `.eml` file; the next regen will clobber it.
203211

212+
## Sync tests: the Stalwart IMAP sidecar
213+
214+
The default `app` fixture has **no IMAP server** — its accounts point at
215+
placeholder hosts. So a triggered sync against the default fixture reaches the
216+
connection step and flips `running → error` almost immediately; it never fetches
217+
or indexes. Any test that needs a *real* sync (events flowing through
218+
`/api/sync/stream`, the spinner staying up, a completing run) must start a real
219+
backend with `e2e/harness/stalwart.ts`. **Never** mock `/api/sync` or synthesize
220+
the SSE stream with `page.route(...)` — this suite's contract is real-backend,
221+
and a faked stream tests the mock, not the product.
222+
223+
`startStalwart({ users })` spins an ephemeral Stalwart on loopback ports, creates
224+
the `test.local` domain + each user, and can seed an INBOX via IMAP APPEND. It is
225+
**not** started by the default fixture (it costs ~3s), so these tests assemble
226+
their own server from the low-level harness building blocks instead of relying on
227+
the `app`/`page` fixtures' default server:
228+
229+
```ts
230+
import { test, expect } from '../harness/fixtures.ts';
231+
import { cloneCorpus, removeClone, type Clone } from '../harness/clone.ts';
232+
import { writeFixtureConfig, addAccountToml, type ConfigEntry } from '../harness/config.ts';
233+
import { indexClone } from '../harness/notmuch.ts';
234+
import { startServer, type ServerHandle } from '../harness/server.ts';
235+
import { startStalwart, type StalwartHandle } from '../harness/stalwart.ts';
236+
237+
// openspec/changes/<change>/specs/<capability>/spec.md: <what this asserts>
238+
test('drives a real sync through the SSE stream', async () => {
239+
let clone: Clone | undefined; let stalwart: StalwartHandle | undefined; let server: ServerHandle | undefined;
240+
try {
241+
clone = await cloneCorpus();
242+
const scope = await indexClone(clone);
243+
stalwart = await startStalwart({
244+
users: [{ email: 'alice@test.local', secret: 'stalwart-secret', inboxMessages: [/* raw .eml strings */] }]
245+
});
246+
const base = await writeFixtureConfig(clone);
247+
const maildir = `${clone.maildir}/alice@test.local`;
248+
await (await import('node:fs/promises')).mkdir(maildir, { recursive: true });
249+
const entry: ConfigEntry = { id: 'alice@test.local', maildirRoot: maildir };
250+
await addAccountToml(base.accountsDir, { ...entry, toml: [
251+
'protocol = "imap"', 'email = "alice@test.local"', 'imap_host = "127.0.0.1"',
252+
`imap_port = ${stalwart.imapPort}`, 'imap_tls = false', 'credential_backend = "plain"',
253+
'credential_ref = "stalwart-secret"', `maildir_root = "${maildir}"`, ''
254+
].join('\n') });
255+
server = await startServer({ scope, clone, config: { ...base, entries: [...base.entries, entry] } });
256+
// ...trigger POST /api/sync/<entry.id>, read SSE, or drive the UI at server.baseURL...
257+
} finally {
258+
if (server) await server.stop();
259+
if (stalwart) await stalwart.stop();
260+
await removeClone(clone);
261+
}
262+
});
263+
```
264+
265+
`sync.spec.ts` is the canonical, fully-worked example (SSE assertions). For
266+
**UI** tests that need to point the browser at this custom server, navigate the
267+
page to `server.baseURL` (the default `page` fixture targets the default server,
268+
so you drive the custom one by URL).
269+
270+
**Known limitation — cleartext auth fails (assert accordingly).** Stalwart
271+
0.15.5 refuses cleartext IMAP `LOGIN`/`AUTHENTICATE` even with
272+
`imap.auth.allow-plain-text = true`, so a real sync currently terminates at the
273+
auth step with `status: "error"` — it never reaches fetch/index/`sync_completed`.
274+
Consequences for assertions:
275+
276+
- The lifecycle events that fire **before** auth *do* arrive:
277+
`checking_password`, `password_retrieved`, `connecting`, then `sync_failed`.
278+
Assert on those, not on `connected`/`fetched`/`indexed`.
279+
- Terminal-status assertions should accept `['done', 'error']` (see `sync.spec.ts`).
280+
- Tests that require a **completing** sync (history populated, spinner observed
281+
mid-fetch, `sync_completed`) stay `test.fixme(...)` with a comment pointing at
282+
this limitation — enable them once a TLS-capable Stalwart listener (or a
283+
confirmed cleartext opt-in) lands. `status-bar.spec.ts` has the canonical
284+
`test.fixme` slots.
285+
286+
For a local **dev** backend (not the per-test sidecar), `deno task stalwart:dev`
287+
(`scripts/stalwart-dev.ts`) runs a standing Stalwart instance.
288+
204289
## Common pitfalls observed in this repo
205290

206291
- **Forgetting the OpenSpec comment.** Reviewers will ask for it; CI doesn't
@@ -222,6 +307,11 @@ exist yet:
222307
- **Building selectors from scratch when a page object exists.** Add a method
223308
to `AccountsPage` / `MailboxPage` / `MessagePage` instead — that's where the
224309
next test will look for it too.
310+
- **Mocking the sync API / SSE stream.** Don't `page.route('**/api/sync*', …)`
311+
or hand-feed `/api/sync/stream` frames. Sync tests use the real Stalwart
312+
sidecar (see "Sync tests: the Stalwart IMAP sidecar"); assert on the pre-auth
313+
lifecycle events + `error` terminal state, and `test.fixme` anything that needs
314+
a completing sync.
225315

226316
## After writing the spec
227317

‎e2e/specs/status-bar.spec.ts‎

Lines changed: 157 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,99 +1,187 @@
11
/**
2-
* Desktop status bar: idle indicator, click-to-open progress popup, and (when a
3-
* sync is live) the spinner + per-account rows.
2+
* Sync status bar (redesign): compact idle dot that morphs dot → "Sync now"
3+
* button → spinner → event-log popup, with a timestamped event log and
4+
* error-state styling.
5+
*
6+
* Sync is driven against a real backend, never mocked. Morph/error tests use the
7+
* default fixture (whose `imap.invalid` accounts make a triggered sync fail fast,
8+
* which is exactly what the error-state assertions want). The event-log content
9+
* test uses a real Stalwart sidecar with a `plain`-credential account so the
10+
* pre-auth lifecycle events (`checking_password` → `password_retrieved` →
11+
* `connecting`) actually fire. Stalwart 0.15.5 refuses cleartext auth, so the run
12+
* still terminates in `error` before fetch/index — tests that need a *completing*
13+
* sync (fetched/indexed/sync_completed, populated history) stay `test.fixme`.
414
*/
15+
import { mkdir } from 'node:fs/promises';
16+
import { join } from 'node:path';
517
import { test, expect } from '../harness/fixtures.ts';
18+
import { cloneCorpus, removeClone, type Clone } from '../harness/clone.ts';
19+
import { writeFixtureConfig, addAccountToml, type ConfigEntry } from '../harness/config.ts';
20+
import { indexClone } from '../harness/notmuch.ts';
21+
import { startServer, type ServerHandle } from '../harness/server.ts';
22+
import { startStalwart, type StalwartHandle } from '../harness/stalwart.ts';
623

7-
// openspec/specs/notmuch-database/spec.md: Indexing progress — desktop UI spinner
8-
test('status bar is present and its popup toggles open', async ({ page }) => {
24+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: idle dot shown at rest
25+
test('idle dot is visible at startup and the spinner/button are not', async ({ page }) => {
926
await page.goto('/');
10-
11-
const container = page.getByTestId('status-bar.container');
12-
await expect(container).toBeVisible();
13-
14-
// Idle by default: the idle dot is shown, not the spinner.
27+
await expect(page.getByTestId('status-bar.container')).toBeVisible();
1528
await expect(page.getByTestId('status-bar.idle')).toBeVisible();
29+
await expect(page.getByTestId('status-bar.sync-btn')).toHaveCount(0);
1630
await expect(page.getByTestId('status-bar.spinner')).toHaveCount(0);
17-
18-
// Clicking the toggle opens the progress popup; with no activity yet it shows
19-
// the empty-state message.
20-
await page.getByTestId('status-bar.toggle').click();
21-
await expect(page.getByTestId('status-bar.popup')).toBeVisible();
22-
await expect(page.getByTestId('status-bar.empty')).toBeVisible();
23-
24-
// Closing the popup hides it again.
25-
await page.getByTestId('status-bar.close-btn').click();
2631
await expect(page.getByTestId('status-bar.popup')).toHaveCount(0);
2732
});
2833

29-
// openspec/specs/notmuch-database/spec.md: Spinner appears during active indexing
30-
//
31-
// Showing the spinner and populated per-account rows requires a sync that stays
32-
// `running` long enough to observe and that reaches the indexing phase. The
33-
// per-test harness has no live IMAP backend that authenticates, so a triggered
34-
// sync flips from `running` to `error` almost immediately and never indexes.
35-
// Enable once a TLS-capable Stalwart (or equivalent) supports a completing sync.
36-
test.fixme('spinner shows during an active sync and popup lists the account', async ({
37-
app,
38-
page
39-
}) => {
40-
const account = app.config.entries[0].id;
34+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: idle dot morphs to a clickable Sync button
35+
test('clicking the idle dot morphs it into a clickable "Sync now" button', async ({ page }) => {
4136
await page.goto('/');
42-
await fetch(`${app.baseURL}/api/sync/${encodeURIComponent(account)}`, { method: 'POST' });
37+
await page.getByTestId('status-bar.idle').click();
4338

44-
await expect(page.getByTestId('status-bar.spinner')).toBeVisible();
45-
await page.getByTestId('status-bar.toggle').click();
46-
await expect(page.getByTestId('status-bar.row').filter({ has: page.locator(`[data-account="${account}"]`) })).toBeVisible();
39+
const btn = page.getByTestId('status-bar.sync-btn');
40+
await expect(btn).toBeVisible();
41+
await expect(btn).toBeEnabled();
42+
await expect(page.getByTestId('status-bar.idle')).toHaveCount(0);
4743
});
4844

49-
// openspec/specs/sveltekit-ui/spec.md: Sync now — optimistic started state
50-
//
51-
// requestSync() sets started=true synchronously, so the toggle shows the
52-
// spinner and "Started…" text immediately after clicking the sync button.
53-
test('toggle shows spinner and "Started…" text immediately after clicking Sync now', async ({ page }) => {
45+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: Sync button starts sync and morphs to spinner
46+
test('clicking the Sync button starts a sync and morphs to a spinner', async ({ page }) => {
5447
await page.goto('/');
55-
await page.getByTestId('status-bar.toggle').click();
48+
await page.getByTestId('status-bar.idle').click();
5649
await page.getByTestId('status-bar.sync-btn').click();
57-
58-
// The toggle button itself should show the spinner + "Started…".
50+
// requestSync() flips `started` synchronously, so the spinner appears at once.
5951
await expect(page.getByTestId('status-bar.spinner')).toBeVisible();
60-
await expect(page.getByTestId('status-bar.toggle')).toContainText('Started…');
6152
});
6253

63-
// openspec/specs/notmuch-database/spec.md: Popup shows sync history
64-
//
65-
// History persistence requires a completed sync with SyncFinished. The
66-
// per-test harness has no completing IMAP backend. Enable once a
67-
// TLS-capable Stalwart (or equivalent) supports a completing sync.
68-
test.fixme('sync history section shows after SyncFinished and survives reload', async ({
69-
app,
70-
page
71-
}) => {
72-
const account = app.config.entries[0].id;
54+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: spinner opens the event-log popup
55+
test('clicking the spinner opens the event-log popup', async ({ page }) => {
7356
await page.goto('/');
74-
await page.getByTestId('status-bar.toggle').click();
57+
await page.getByTestId('status-bar.idle').click();
7558
await page.getByTestId('status-bar.sync-btn').click();
76-
// Wait for the sync to complete (SyncFinished received).
77-
await expect(page.getByTestId('status-bar.idle')).toBeVisible({ timeout: 30_000 });
78-
await expect(page.getByTestId('status-bar.history')).toBeVisible();
59+
await page.getByTestId('status-bar.spinner').click();
7960

80-
// Reload and verify history persists.
81-
await page.reload();
82-
await page.getByTestId('status-bar.toggle').click();
83-
await expect(page.getByTestId('status-bar.history')).toBeVisible();
61+
const popup = page.getByTestId('status-bar.popup');
62+
await expect(popup).toBeVisible();
63+
// Closing returns to no-popup.
64+
await page.getByTestId('status-bar.close-btn').click();
65+
await expect(popup).toHaveCount(0);
8466
});
8567

86-
// openspec/specs/notmuch-database/spec.md: Clear history button dismisses old logs
87-
//
88-
// Same backend limitation as the history test above.
89-
test.fixme('clear history button removes persisted sync history', async ({ app, page }) => {
90-
const account = app.config.entries[0].id;
68+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: spinner returns to idle after the run ends
69+
test('spinner returns to the idle dot after the sync run finishes', async ({ page }) => {
9170
await page.goto('/');
92-
await page.getByTestId('status-bar.toggle').click();
71+
await page.getByTestId('status-bar.idle').click();
9372
await page.getByTestId('status-bar.sync-btn').click();
73+
await expect(page.getByTestId('status-bar.spinner')).toBeVisible();
74+
// The fixture's imap.invalid account fails fast → run terminates → back to idle.
9475
await expect(page.getByTestId('status-bar.idle')).toBeVisible({ timeout: 30_000 });
95-
await expect(page.getByTestId('status-bar.history')).toBeVisible();
76+
await expect(page.getByTestId('status-bar.spinner')).toHaveCount(0);
77+
});
9678

97-
await page.getByTestId('status-bar.clear-history').click();
79+
// openspec/changes/sync-status-bar-redesign/specs/sync-status-compact-ui/spec.md: error state styling on a failed sync
80+
test('a failed sync surfaces the error-state dot', async ({ page }) => {
81+
await page.goto('/');
82+
await page.getByTestId('status-bar.idle').click();
83+
await page.getByTestId('status-bar.sync-btn').click();
84+
// The credential/connection failure marks the run errored; the dot returns
85+
// styled as an error (data-testid stays `status-bar.idle`, with the error class).
86+
const dot = page.getByTestId('status-bar.idle');
87+
await expect(dot).toBeVisible({ timeout: 30_000 });
88+
await expect(dot.locator('.mb-status-dot.is-error')).toBeVisible({ timeout: 30_000 });
89+
});
90+
91+
// openspec/changes/sync-status-bar-redesign/specs/sync-event-log/spec.md: event log shows timestamped lifecycle events
92+
test('event log records timestamped lifecycle events for a real sync', async () => {
93+
let clone: Clone | undefined;
94+
let stalwart: StalwartHandle | undefined;
95+
let server: ServerHandle | undefined;
96+
try {
97+
clone = await cloneCorpus();
98+
const scope = await indexClone(clone);
99+
stalwart = await startStalwart({
100+
users: [{ email: 'alice@test.local', secret: 'stalwart-secret' }]
101+
});
102+
103+
const base = await writeFixtureConfig(clone);
104+
const maildir = join(clone.maildir, 'alice@test.local');
105+
await mkdir(maildir, { recursive: true });
106+
const entry: ConfigEntry = { id: 'alice@test.local', maildirRoot: maildir };
107+
await addAccountToml(base.accountsDir, {
108+
...entry,
109+
toml: [
110+
'protocol = "imap"',
111+
'email = "alice@test.local"',
112+
'imap_host = "127.0.0.1"',
113+
`imap_port = ${stalwart.imapPort}`,
114+
'imap_tls = false',
115+
'credential_backend = "plain"',
116+
'credential_ref = "stalwart-secret"',
117+
`maildir_root = "${maildir}"`,
118+
''
119+
].join('\n')
120+
});
121+
server = await startServer({
122+
scope,
123+
clone,
124+
config: { path: base.path, accountsDir: base.accountsDir, entries: [...base.entries, entry] }
125+
});
126+
127+
const { chromium } = await import('@playwright/test');
128+
const browser = await chromium.launch();
129+
const page = await browser.newPage();
130+
try {
131+
await page.goto(`${server.baseURL}/`);
132+
// Morph dot → button → spinner, then open the popup so live events show.
133+
await page.getByTestId('status-bar.idle').click();
134+
await page.getByTestId('status-bar.sync-btn').click();
135+
await page.getByTestId('status-bar.spinner').click();
136+
await expect(page.getByTestId('status-bar.popup')).toBeVisible();
137+
138+
// Pre-auth lifecycle events fire for a plain-credential account even though
139+
// Stalwart then refuses cleartext auth (run ends in sync_failed). `Sync now`
140+
// triggers all configured accounts, so scope assertions to the stalwart one.
141+
const rows = page.getByTestId('status-bar.event-row');
142+
await expect(rows.first()).toBeVisible({ timeout: 20_000 });
143+
const stalwartRows = rows.filter({ hasText: 'alice@test.local' });
144+
// A timestamped `checking_password (plain)` row for our account proves the
145+
// full pipeline: server lifecycle event → SSE → event log → popup render.
146+
await expect(
147+
stalwartRows.filter({ hasText: 'checking_password' }).first()
148+
).toBeVisible({ timeout: 20_000 });
149+
await expect(stalwartRows.first()).toContainText(/\d\d:\d\d:\d\d/);
150+
} finally {
151+
await browser.close();
152+
}
153+
} finally {
154+
if (server) await server.stop();
155+
if (stalwart) await stalwart.stop();
156+
await removeClone(clone);
157+
}
158+
});
159+
160+
// openspec/changes/sync-status-bar-redesign/specs/sync-event-log/spec.md: clear-history removes archived runs (needs ≥2 runs)
161+
//
162+
// The "Clear history" button only appears once a prior run has been archived,
163+
// which requires a second sync to roll the first run into history. With the
164+
// cleartext-auth limitation both runs error, but archival still occurs on the
165+
// second requestSync — enable once the morph timing under back-to-back errored
166+
// runs is stable enough to drive deterministically in CI.
167+
test.fixme('clear-history button removes archived runs after confirmation', async ({ page }) => {
168+
const dialogs: string[] = [];
169+
page.on('dialog', (d) => {
170+
dialogs.push(d.message());
171+
d.accept();
172+
});
173+
await page.goto('/');
174+
// (sync twice so run #1 is archived, open popup, assert History visible, then
175+
// click status-bar.clear-history and assert the section disappears.)
98176
await expect(page.getByTestId('status-bar.history')).toHaveCount(0);
99177
});
178+
179+
// openspec/changes/sync-status-bar-redesign/specs/sync-event-log/spec.md: completing sync populates fetched/indexed/sync_completed
180+
//
181+
// Needs a sync that authenticates and fetches. Stalwart 0.15.5 refuses cleartext
182+
// IMAP auth, so the run errors before the fetch/index phase. Enable once a
183+
// TLS-capable Stalwart listener (or confirmed cleartext opt-in) lands.
184+
test.fixme('event log shows fetched/indexed/sync_completed for a completing sync', async ({ page }) => {
185+
await page.goto('/');
186+
await expect(page.getByTestId('status-bar.popup')).toHaveCount(0);
187+
});

0 commit comments

Comments
 (0)