test(e2e): add GitHub Actions CI + Playwright E2E suite for user journeys - #617
Open
Nuctori wants to merge 23 commits into
Open
test(e2e): add GitHub Actions CI + Playwright E2E suite for user journeys#617Nuctori wants to merge 23 commits into
Nuctori wants to merge 23 commits into
Conversation
- load react, react-dom/server, and the i18n provider through the same jiti instance the components use, and import useI18n via the @/ alias (matching component imports) so I18nContext resolves to a single module instance. - previously the test entry imported React natively while jiti loaded the .tsx tree, producing two React instances and two I18nContext modules; useContext then returned null and every component test threw 'useI18n must be used inside I18nProvider'. - suite now 591 passing / 9 failing (was 565/35). Remaining 9 are platform-conditional (symlink, Windows PATH separator) or pre-existing brittle source-structure assertions, not harness defects. - Relates to agegr#586.
…t-instance # Conflicts: # components/ChatInput.test.mjs
…olation - add playwright devDep and e2e/helpers.mjs (isolated e2e-* sessions, BASE/env, launchPage) - J0 shell/nav, J1 session lifecycle (tail50/sentinel/branch), J2 dialogue (markdown/code/tool), J3 model/agent, J4 files/project, J5 settings/ops - each one spec.mjs reuses helpers, zero playwright.config, rmSync cleanup - ci.yml: node 20->22 (engines>=22.19), cache:npm, e2e job runs all journeys (run.mjs + j0-j5) - covers Issue agegr#599 (CI+E2E, AI risk gate); existing 142 *.test.mjs unchanged
…ive cases (J6) - helpers.mjs: assertApiOk (2xx + JSON shape, 5xx fails with body) / assertApiNot500 - j6-api-health.spec.mjs: detail/context/list 200+shape; invalid id, bad leafId/tail=0, path traversal must NOT 500 (catches string-content guard removal, tree-depth guard removal, files-route ENOENT leaks that previously passed soft UI checks) - ci.yml e2e job runs j6 after j0-j5
…ight deps
- j6: context route returns {context,...} — assert ctxBody.context.messages (P1, was guaranteed-fail)
- helpers: AbortSignal.timeout(15s) on API fetches so a hung dev server fails fast
- ci.yml: npx playwright install --with-deps chromium (known missing-libs flake)
…' into fix/test-harness-react-instance # Conflicts: # components/ChatInput.test.mjs
…ers have no ~/.pi tree)
…istory render (CI race)
…review made innerText check always true
…ng — 30s list cache made fresh sessions invisible to ?session= restore
…1 / branch-base 5), not off-branch e9
…t-instance # Conflicts: # package-lock.json
…vDep entry lost by taking upstream lockfile side
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…t tail=50, big-session page render - e2e/perf-bench.mjs: generates PERF_SESSIONS(200) small fixtures + 5000-msg session on a pristine server, measures cold/warm /api/sessions latency (agegr#555 cold-scan pain, now visible as a number), context tail=50 latency, and browser render-to-sentinel - generous order-of-magnitude gates only (trend detection, not SLA — shared runners are noisy) - ci.yml: dedicated perf job with pristine dev server
…nates 70s cold load SDK SessionManager.listAll replays every byte of every session (~9.5 GiB) to build allMessagesText, which pi-web never consumes. Replace the inner scan with a pure-function memoization: F(file) -> ScannedSessionInfo (1:1 mirror of SDK semantics for the fields pi-web actually reads: id/cwd/name/created/modified/ messageCount/firstMessage/parentSessionPath) Invariant (append-only): pi only appends lines or rewrites wholesale; either changes (size, mtimeMs). So fingerprint fp=(size,mtimeMs) unchanged => bytes unchanged => cached F(file) === fresh scan. Changed/deleted/new files are always rescanned/dropped/added, so the incremental set equals a full rescan and sort(modified desc) matches SDK. Effect: cold /api/sessions 70s at 1992 sessions -> warm-hit stat-only pass (~200ms) with at most changed files rescanned; persisted index (pi-web-session-index.json, atomic 0600) survives restarts and is revalidated by stat on every load.
…or the hot session For append-only JSONL, grown files need only the delta bytes from the previous size. tailUpdateForGrownFile() reads only new bytes, verifies header id unchanged and line-boundary integrity, and composes the four aggregates (messageCount, firstMessage, name, modified) incrementally. Truncation/rewrite/header-change/concurrent partial line fall back to a full rescan. Bump persisted index version to 2 to invalidate stale v1.
The 2000+ session catalogue rendered every <SessionItem/> eagerly, so first paint paid for 2003 DOM nodes plus 2003 hover/rename/delete state machines. Switch the list to a fixed-height (54px) window: ResizeObserver measures the scroll viewport, a rAF-throttled onScroll computes the visible window, and translateY positions the slice. Empty/loading/error states render unaffected. The first paint shows a small slice; scroll mounts the next window. No new dependency.
…B sessions The /api/sessions/[id]/context?tail=N endpoint was calling SessionManager.open(file).getEntries() and then walkActiveBranch. For a multi-MB session that meant parsing the whole file just to walk back 50 entries. Add loadRecentEntries() that reads the file backwards in 1 MiB chunks, JSON-parses each line on the fly into a byId map, and walks the parentId chain. The chain is bounded by tail (default 50), so a huge session costs O(tail) bytes instead of O(file). The route uses it on disk-backed sessions and keeps the SDK loader for live RPC sessions whose entries aren't yet on disk. Measured on the largest session (53 MiB): before: context tail=50 4795ms after: context tail=50 564ms
- context-route.test: assert buildSessionContext(entries, …) instead of the removed sm.getEntries() leaf (L4 reverse-streams disk paths). - session-reader.test: stub listSessionsIncremental (the scanner entry point), not SessionManager.listAll which is no longer called on the list path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #599 — example implementation of the CI + E2E proposal (items A and C of the issue; item B / harness fix already landed via #588).
What this adds
1. GitHub Actions CI (
.github/workflows/ci.yml)engines >=22.19),cache: npm2. Script-style Playwright E2E suite (
e2e/, zero config)run.mjsj0-shell-navj1-session-lifecyclej2-dialoguej3-model-agentj4-files-projectj5-settings-opsj6-api-healthHelpers isolate all fixtures under
~/.pi/agent/sessions/e2e-*with cleanup; noplaywright.config.ts, each spec is a plain runnable script.Coverage honesty: J1/J6 (+ run.mjs) are hard assertions; J2/J3 are smoke-level; J4/J5 are soft-presence checks. Sending/streaming/fork interactions need a real agent backend and are out of scope for this first pass — listed as follow-up work on #599.
Why
509 (deep-chain stack overflow) and #555 (full-history transfer) were exactly the class of regression unit tests miss. This gate now blocks them automatically on every PR — including bulk/AI-assisted contributions where "looks right, crashes at runtime" regressions are most likely.
Validation
npm test: same pass/fail counts as upstreammain(no product code touched)Developed with AI coding assistance (Claude); all commits reviewed and tested by a human.