Repository navigation
Conversation
The Playwright UI e2e suite had rotted across several intentional UI refactors and no longer matched the app. Two specs were known-failing (acceptance, review), but `bun run e2e:ui` exercises the whole suite and 8 more specs were stale from the same changes. The failures were NOT SSE/daemon bugs — verified the index/session-status SSE and the revision-banner SSE both emit correctly in the hermetic e2e environment. The specs encoded a pre-refactor UI: - index is now the app-shell sidebar (`.sl-row` + status glyphs), not `.card`/`.chip`; approved sessions sit in a collapsed disclosure. - the new-revision banner renders only from r2 (DECISIONS "banner from r2"); r1 is a first review with no banner. - an agent revision reply is a response shown inline on the open thread, not a `.thread-resolved` collapse (only the reviewer's Resolve closes). - a thread whose quote a later revision deletes detaches inline & muted — there is no orphan tray. - a Save-approve of the viewed session redirects home, so the artifact is asserted on disk rather than via the (unseen) approved note. - the live console no longer auto-expands; the header switcher is gone, replaced by the ☰ mobile session sheet. - settings gained an "update" section; the rail jump lives on the quote/ meta row, not the whole card. Also cap the harness at `workers: 2` + `retries: 2`: the whole suite shares one real daemon, and high concurrency saturates it (keep-alive `ECONNRESET`, scaling with worker count). This is a test-harness concern, not a daemon bug — production otacond serves one human plus their agent. Verification: `bun run e2e:ui` green (58 passed, exit 0), `bun test` (1066 pass), `bun run typecheck` clean. No product (`src/`) changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdates Playwright UI e2e tests to use sidebar row/glyph selectors, inline detached-thread assertions, approve-flow disk checks, responsive phone behavior changes, and Playwright concurrency limits. ChangesUI e2e Test Suite Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
… failure The Playwright UI e2e suite had no automated gate — only the release workflow runs (and it runs typecheck + bun unit tests + build, never e2e), which is how the suite silently rotted across 0.1.4–0.1.6. Add `.github/workflows/e2e.yml`: - `e2e` job runs `bun run e2e:ui` (bun + Playwright chromium, --with-deps) on every pull_request and on every push to main. PRs are now gated; merges to main are verified. - `autofix` job runs only when the suite fails on a push to main: it sets up bun + the browser, then hands the failure to the Claude Code action (anthropics/claude-code-action@v1) to reproduce, fix the right side (app or stale test), and open a PR. Skipped on PRs (the red check is the gate there). One-time setup the workflow needs: the ANTHROPIC_API_KEY repo secret, and "Allow GitHub Actions to create and approve pull requests" (Settings → Actions → General) so the autofix job can open the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zeroliu
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yml:
- Around line 29-35: The e2e job is checking out repository code without
restricting the job token, so harden the workflow before running untrusted
repo-controlled steps. In the e2e job, add an explicit read-only permissions
block and update the actions/checkout@v4 step to disable credential persistence
with persist-credentials set to false, so the job does not retain broader token
access than needed.
- Around line 32-33: The workflow uses tag-pinned actions for actions/checkout
and oven-sh/setup-bun, which can be retargeted upstream. Update the uses
references in the e2e workflow to immutable full commit SHAs, and if helpful
keep the current semver tags in comments for readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 10445395-57f9-4a60-9cfd-d9c05276ffb8
📒 Files selected for processing (1)
.github/workflows/e2e.yml
| e2e: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: oven-sh/setup-bun@v2 | ||
| - name: Install dependencies | ||
| run: bun install --frozen-lockfile |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Harden the e2e job before running repository-controlled code.
actions/checkout leaves the repo token available to the job, so add an explicit read-only permissions: block and set persist-credentials: false on checkout. That keeps the test job from inheriting broader token access than it needs.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 32-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 29-51: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 32-32: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 29-29: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e.yml around lines 29 - 35, The e2e job is checking out
repository code without restricting the job token, so harden the workflow before
running untrusted repo-controlled steps. In the e2e job, add an explicit
read-only permissions block and update the actions/checkout@v4 step to disable
credential persistence with persist-credentials set to false, so the job does
not retain broader token access than needed.
| - uses: actions/checkout@v4 | ||
| - uses: oven-sh/setup-bun@v2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin workflow actions to immutable SHAs. The current uses: refs are tag-pinned only, so they can be retargeted upstream; pin each action to a full commit SHA and, if needed, keep the semver in a comment for readability.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 32-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 32-32: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e.yml around lines 32 - 33, The workflow uses tag-pinned
actions for actions/checkout and oven-sh/setup-bun, which can be retargeted
upstream. Update the uses references in the e2e workflow to immutable full
commit SHAs, and if helpful keep the current semver tags in comments for
readability.
1 participant
What
Two related changes that make the Playwright UI e2e suite both correct and enforced:
test/ui/*.e2e.ts) — it had drifted across several intentional UI refactors and no longer matched the app..github/workflows/e2e.yml) — it previously had no automated gate, which is exactly how it rotted silently across 0.1.4–0.1.6 (the release workflow runs typecheck +bun test+ build, never e2e).All product code (
src/) is unchanged — this is test + harness + CI only.Why the specs failed (not SSE/daemon bugs)
I reproduced the failures and verified the index/session-status SSE and the revision-banner SSE both emit correctly in the hermetic e2e env. The specs simply encoded a pre-refactor UI:
.sl-row+ status glyphs), not.card/.chip; approved sessions live in a collapsedapproved (n)disclosure.revision ≥ 2, DECISIONS "banner from r2"); r1 is a first review with no banner..thread-resolvedcollapse — only the reviewer's Resolve closes a thread.Harness contention fix
The full suite shares one real daemon; high concurrency saturates it (keep-alive
ECONNRESET, scaling ≈1 → 8 → 24 at 2 → 4 → 9 workers).playwright.config.tsnow pinsworkers: 2+retries: 2(a test-harness concern, not a daemon bug — prod otacond serves one human + their agent). Recorded inDECISIONS.md.CI workflow (
.github/workflows/e2e.yml)e2ejob runsbun run e2e:ui(bun + Playwright chromium--with-deps) on every PR and on every push tomain. PRs are gated; merges to main are verified.autofixjob runs only when the suite fails on a push to main: it sets up bun + the browser, then hands the failure to the Claude Code action (anthropics/claude-code-action@v1) to reproduce, fix the right side (app or stale test), and open a PR. Skipped on PRs (there the red check is the gate).autofixjob needs (verified missing on this repo)ANTHROPIC_API_KEYrepository secret (Settings → Secrets and variables → Actions). (Currently no secrets are set.)The
e2egating works without either — only the auto-fix step depends on them.Verification
bun run e2e:ui→ 58 passed, exit 0 (run twice; the retry budget absorbed transient resets).bun test→ 1066 pass, 0 fail.bun run typecheck→ clean.e2echeck is the first real CI run of the suite (developed/verified on macOS; CI is Linux).🤖 Generated with Claude Code