ci: promote e2e artifacts to required check - #200
Conversation
Astro-Han
added
enhancement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🧰 Additional context used📓 Path-based instructions (2)packages/opencode/**/*.ts📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
Files:
packages/opencode/test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
Files:
🧠 Learnings (7)📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.017ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.017ZApplied to files:
📚 Learning: 2026-04-23T08:51:00.819ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe PR changes the e2e-artifacts GitHub Actions workflow to make E2E failures blocking by removing Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner
participant Playwright as Playwright Tests
participant Uploader as Artifact Uploader
rect rgba(0,128,0,0.5)
GH->>Runner: Start e2e-artifacts job
end
Runner->>Playwright: Run E2E smoke/tests
alt tests pass
Playwright-->>Runner: Success
Runner->>Uploader: Upload artifacts (normal)
else tests fail
Playwright-->>Runner: Failure
Runner->>GH: Mark job as failed
Runner->>Uploader: Upload artifacts (if: always())
Runner->>GH: Emit "Warn on E2E failure" message with test results artifact link
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the test suite for the e2e-artifacts workflow to reflect that the workflow is now a required, blocking process. The changes include updating the test description, asserting that 'continue-on-error' is disabled for the job and run steps, removing 'Non-blocking' from warning expectations, and ensuring the artifact upload step uses the 'always()' condition. I have no feedback to provide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-artifacts.yml:
- Around line 85-88: The step name "Warn on smoke failure" is misleading; rename
the workflow step (the YAML step with name "Warn on smoke failure") to a name
that reflects generic E2E failures (e.g., "Warn on E2E failure" or "Warn on E2E
tests failure") and ensure the run echo message remains consistent with that new
name so Actions logs and triage reflect the updated semantics.
🪄 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: 96158467-2fab-4433-985f-2ca942d1b0de
📒 Files selected for processing (2)
.github/workflows/e2e-artifacts.ymlpackages/opencode/test/config/e2e-artifacts-workflow.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
🧠 Learnings (6)
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/packages/app/src/testing/**/*.ts : Test-only hooks must be inert unless explicitly enabled and should not add normal-runtime listeners, reactive subscriptions, or per-update allocations
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use `testEffect(...)` from `test/lib/effect.ts` for tests that exercise Effect services or Effect-based workflows.
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test files should be named with the pattern `feature-name.spec.ts`
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
📚 Learning: 2026-04-20T14:36:31.017Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.017Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/config/e2e-artifacts-workflow.test.ts
🔇 Additional comments (2)
.github/workflows/e2e-artifacts.yml (1)
1-4: Good load-bearing contract documentation in the workflow header.These comments clearly capture the required-check intent and rollout sequencing for
devruleset enforcement.packages/opencode/test/config/e2e-artifacts-workflow.test.ts (1)
9-50: Strong contract assertions for required-check and artifact-retention behavior.The added checks on Lines 39, 45, 48, and 50 accurately lock the intended behavior change (blocking E2E + always-upload artifacts).
6cd31ba to
a6f7ccd
Compare
1 participant
Summary
Promotes
e2e-artifactsfrom advisory diagnostics to a real PR E2E check by removing the job-levelcontinue-on-error.Keeps Playwright artifacts uploading on failure so failed runs still expose the report, junit output, and test results.
Updates the workflow contract test to guard required-check behavior and retained artifact upload.
Why
PR #198 showed that rendered UI regressions can be caught by
e2e-artifactswhile typecheck and unit tests stay green. The workflow must fail before it can safely be added to thedev-merge-gaterequired checks.Related Issue
Part of #199. The ruleset update remains a post-merge operations step.
How To Verify
bun --cwd packages/opencode test test/github test/config/e2e-artifacts-workflow.test.tsBroader full typecheck, full test:ci, desktop smoke, and E2E are left to PR CI per PawWork local verification guidance.
Screenshots or Recordings
Not applicable. No visible UI change.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Chores
Tests