feat: remove Argos CI from apps/web and add to apps/storybook #1999
Conversation
- Remove @argos-ci/playwright from apps/web - Remove Argos reporter from apps/web/playwright.config.ts - Remove apps/web/tests/visual.spec.ts - Remove Argos-related steps from .github/workflows/web_ci.yaml - Add @argos-ci/storybook, @storybook/addon-vitest, vitest, and related packages to apps/storybook - Create vitest.config.ts with storybookTest and argosVitestPlugin - Create .storybook/vitest.setup.ts - Add @storybook/addon-vitest to .storybook/main.ts - Add screenshots to apps/storybook/.gitignore - Create .github/workflows/storybook_ci.yaml for visual testing Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a Storybook CI workflow and Storybook test-runner that capture Argos screenshots; removes Argos-driven visual tests and reporter from the web app and web CI; updates Storybook devDependencies and .gitignore; removes a sitemap route. Changes
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions
participant Repo as Repository
participant Runner as CI Runner
participant PW as Playwright/Chromium
participant TR as Storybook Test Runner
participant Argos as Argos Service
GH->>Repo: trigger (push/PR/workflow_dispatch)
GH->>Runner: start Storybook job
Runner->>Repo: checkout code
Runner->>Runner: pnpm install (local action)
Runner->>Runner: pnpm -F ui build
Runner->>Runner: pnpm -F storybook build
Runner->>PW: install Playwright + Chromium (apps/storybook)
Runner->>Runner: start static server for Storybook
Runner->>Runner: poll server until ready
Runner->>TR: run Storybook test runner (CI mode) against local URL
loop per story
TR->>PW: render story page
TR->>Argos: argosScreenshot(page, context)
Argos-->>Argos: store/compare screenshot
end
Argos-->>Runner: return upload/report result (via ARGOS_TOKEN)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/playwright.config.ts (1)
9-9: Reporter config is valid but can be simplified now that Argos is removed
reporter: [[process.env.CI ? "dot" : "list"]]is type‑correct (one reporter tuple in an array), but with only a single reporter left you can simplify it for readability to either of these:reporter: process.env.CI ? "dot" : "list",or, if you prefer the tuple form while still supporting future additional reporters:
reporter: [[process.env.CI ? "dot" : "list"]], // becomes, if you add more later: // reporter: [[process.env.CI ? "dot" : "list"], ["some-other-reporter", { ... }]]Given the current setup, the first form is the clearest and avoids the extra array nesting.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/storybook_ci.yaml(1 hunks).github/workflows/web_ci.yaml(0 hunks)apps/storybook/.gitignore(1 hunks)apps/storybook/.storybook/main.ts(1 hunks)apps/storybook/.storybook/vitest.setup.ts(1 hunks)apps/storybook/package.json(2 hunks)apps/storybook/vitest.config.ts(1 hunks)apps/web/package.json(0 hunks)apps/web/playwright.config.ts(1 hunks)apps/web/tests/visual.spec.ts(0 hunks)
💤 Files with no reviewable changes (3)
- apps/web/tests/visual.spec.ts
- apps/web/package.json
- .github/workflows/web_ci.yaml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/storybook/vitest.config.tsapps/web/playwright.config.ts
**/*.config.{ts,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Agent configuration should be centralized and externalized from implementation logic
Files:
apps/storybook/vitest.config.tsapps/web/playwright.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/storybook/vitest.config.tsapps/web/playwright.config.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:29.314Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/web/content/changelog/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:29.314Z
Learning: Applies to apps/web/content/changelog/** : Only keep desktop-related changes when maintaining changelog entries from commits and diffs
Applied to files:
apps/storybook/.gitignore
📚 Learning: 2025-11-24T16:32:24.348Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: apps/desktop-e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:24.348Z
Learning: Applies to apps/desktop-e2e/**/*.{test,spec}.{js,ts,tsx} : Refer to `scripts/setup-desktop-e2e.sh` for end-to-end test environment setup if setup is missing
Applied to files:
apps/storybook/package.json
🧬 Code graph analysis (1)
apps/storybook/vitest.config.ts (1)
apps/web/scripts/gen-agents.js (1)
__dirname(7-7)
🪛 actionlint (1.7.9)
.github/workflows/storybook_ci.yaml
30-30: description is required in metadata of "" action at "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
30-30: name is required in action metadata "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
🔇 Additional comments (8)
apps/storybook/.gitignore (1)
3-3: LGTM!Adding "screenshots" to .gitignore is appropriate for excluding Playwright test artifacts from version control.
.github/workflows/storybook_ci.yaml (1)
1-34: Well-structured CI workflow.The workflow properly:
- Filters paths to relevant packages
- Securely references ARGOS_TOKEN from secrets
- Builds dependencies before running tests
- Installs Playwright with Chromium browser
Note: Static analysis flagged that the local
pnpm_installaction is missingdescriptionandnamein its metadata (Line 30). This is a pre-existing issue with the action definition at.github/actions/pnpm_install/action.yaml, not introduced by this PR.apps/storybook/.storybook/main.ts (1)
9-9: LGTM!Correctly adds the Vitest addon to enable Storybook-Vitest integration, which aligns with the new test infrastructure added in this PR.
apps/storybook/vitest.config.ts (2)
7-10: Good ESM compatibility pattern.The
__dirnamefallback correctly handles both CommonJS and ES module environments.
12-44: Well-structured Vitest configuration.The configuration properly:
- Aliases workspace packages for test resolution
- Integrates Storybook via
storybookTestplugin- Conditionally uploads to Argos based on CI environment
- Configures headless Chromium browser testing
- References the setup file for Storybook initialization
apps/storybook/package.json (2)
8-9: LGTM!The test script correctly invokes Vitest, and the trailing comma addition follows consistent JSON formatting.
18-31: Dependencies properly added.All testing-related dependencies are correctly placed in
devDependencieswith appropriate versions:
- Vitest ecosystem packages for browser testing
- Playwright for browser automation
- Argos integration for visual regression testing
- Storybook Vitest addon
The versions are consistent and recent.
apps/storybook/.storybook/vitest.setup.ts (1)
1-8: Preview file exists and exports correct Storybook annotations.The preview file (
./preview.ts) is properly configured in the.storybookdirectory and exports a validPreviewtype object with parameters and controls configuration. The Vitest setup correctly wires these annotations throughsetProjectAnnotations()and thebeforeAllhook.
The @storybook/addon-vitest package only supports Storybook 9+/10+, but this project uses Storybook 8.6.14. Switched to the Storybook Test Runner approach which is compatible with Storybook v8+. Changes: - Replace @storybook/addon-vitest with @storybook/test-runner - Replace vitest.config.ts and vitest.setup.ts with test-runner.ts - Update package.json with correct dependencies and test script - Update CI workflow to build Storybook, run test-runner, and upload to Argos - Update pnpm-lock.yaml with new dependencies Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
- Add @playwright/test and playwright to devDependencies - Use pnpm exec playwright install instead of npx - Add working-directory for CI steps to run in apps/storybook Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
- Build Storybook static before running tests - Use curl loop to wait for http-server instead of wait-on package - Properly handle server lifecycle with background process and cleanup - This avoids the 'wait-on: not found' error from previous approach Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/storybook_ci.yaml (1)
28-28: Remove redundant ARGOS_TOKEN environment variable definition.
ARGOS_TOKENis already set at the job level (line 28), so redefining it in the upload step's environment block (lines 70–71) is unnecessary. The job-level env is inherited by all steps.Apply this diff to remove the redundant definition:
- name: Upload screenshots to Argos working-directory: apps/storybook - env: - ARGOS_TOKEN: ${{ secrets.ARGOS_TOKEN }} run: pnpm exec argos upload --token "$ARGOS_TOKEN" ./screenshotsAlso applies to: 71-71
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/storybook_ci.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/storybook_ci.yaml
31-31: description is required in metadata of "" action at "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
31-31: name is required in action metadata "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (3)
.github/workflows/storybook_ci.yaml (3)
31-31: Verify pnpm_install action metadata.Static analysis flagged that the
pnpm_installaction at.github/actions/pnpm_install/action.yamlis missing required metadata fields (nameanddescription). This should be addressed in that action's definition to comply with GitHub Actions best practices.
28-28: Verify ARGOS_TOKEN secret is configured in the repository.The workflow relies on
secrets.ARGOS_TOKENbeing available. Ensure this secret is set in the repository settings and contains a valid Argos CI token. This should be part of the PR testing checklist mentioned in the PR objectives.Also applies to: 71-71
38-67: HTTP server lifecycle management is robust.The test runner step handles server startup, readiness checks, test execution, and cleanup well:
- Starts http-server in the background with PID capture.
- Waits up to 60 seconds with explicit curl health checks.
- Handles startup failures by logging and exiting early.
- Ensures the server is killed on both success and failure paths.
- Changed from 'pnpm test-storybook -- --url' to 'pnpm exec test-storybook --url' - The -- separator was causing --url to be passed to Jest instead of test-storybook - Added NODE_NO_WARNINGS and NODE_OPTIONS directly in the command - Verified locally: all 25 tests pass and screenshots are generated Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/storybook_ci.yaml (1)
27-28: Remove redundant ARGOS_TOKEN environment variable declaration.The
ARGOS_TOKENis declared at the job level (line 28) and then re-declared at the step level (line 71). The step-level declaration is redundant since it will inherit from the job-level env. Simplify by removing the duplicate.- name: Upload screenshots to Argos working-directory: apps/storybook - env: - ARGOS_TOKEN: ${{ secrets.ARGOS_TOKEN }} run: pnpm exec argos upload --token "$ARGOS_TOKEN" ./screenshotsAlso applies to: 70-72
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/storybook_ci.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/storybook_ci.yaml
31-31: description is required in metadata of "" action at "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
31-31: name is required in action metadata "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
⏰ 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). (7)
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: ci (macos, macos-14)
- GitHub Check: fmt
🔇 Additional comments (3)
.github/workflows/storybook_ci.yaml (3)
1-16: Trigger configuration is appropriate.The workflow correctly filters for changes in storybook, ui, and tiptap packages and includes manual trigger via workflow_dispatch for ad-hoc runs.
38-67: Server startup and polling logic is solid.The bash script correctly captures the server PID, implements a reasonable polling loop (~60s total timeout with 2s intervals), defensively handles cleanup with
||true, and properly propagates the test runner exit code. The explicitkill $SERVER_PID || truein the cleanup ensures the server is terminated regardless of test success or failure.
68-72: The upload path is correct and matches the @argos-ci/storybook default behavior.The
./screenshotspath referenced in the CI workflow is the standard output directory created by theargosScreenshotfunction from@argos-ci/storybook, which is used in.storybook/test-runner.ts. While there's no explicit local configuration of the output directory, the path is correctly aligned with the library's default behavior, and the.gitignoreentry confirms this is the expected generated artifact location.
- Remove NODE_NO_WARNINGS=1 from storybook_ci.yaml (CodeRabbit review) - Remove /product/opensource from sitemap.ts (fixes typecheck error) Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
2 similar comments
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/storybook_ci.yaml (1)
27-28: Remove redundant ARGOS_TOKEN env var declaration at step level.
ARGOS_TOKENis already defined at the job level (line 28), so the redeclaration at line 71 is unnecessary. GitHub Actions automatically makes job-level env vars available to all steps.Apply this diff to remove the redundancy:
- name: Upload screenshots to Argos working-directory: apps/storybook - env: - ARGOS_TOKEN: ${{ secrets.ARGOS_TOKEN }} run: pnpm exec argos upload --token "$ARGOS_TOKEN" ./screenshotsAlso applies to: 68-72
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/storybook_ci.yaml(1 hunks)apps/web/src/utils/sitemap.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/utils/sitemap.ts
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/storybook_ci.yaml
31-31: description is required in metadata of "" action at "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
31-31: name is required in action metadata "/home/jailuser/git/.github/actions/pnpm_install/action.yaml"
(action)
⏰ 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). (8)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: ci (macos, macos-14)
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (1)
.github/workflows/storybook_ci.yaml (1)
1-72: Workflow structure is solid and aligns with PR objectives.The workflow correctly builds Storybook statically, serves it locally, runs the test-runner with Argos screenshot capture, and uploads results. The server readiness polling (up to 60s), exit code preservation, and cleanup (killing the http-server process) are all properly implemented. The
NODE_OPTIONS=--experimental-vm-modulesflag at line 63 is correctly set for ESM/Jest compatibility required by the test-runner.
- Change UI CSS import from source file (src/styles/globals.css) to built file (dist/globals.css) - This ensures Tailwind CSS is properly processed and included in Storybook static build - Fixes Argos screenshots showing unstyled components Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/storybook/.storybook/preview.tsx (1)
3-4: Confirmdist/globals.cssavailability in Storybook builds and CISwitching from
src/styles/globals.csstodist/globals.cssis fine as long as thepackages/ui/distartifacts exist beforepnpm -F storybook buildruns (locally and in CI). Ifdistis only produced by a separate build step and not committed, this import will break the Storybook build.Two follow-ups to consider:
- Ensure your CI pipeline (and local workflow) always builds or ships
packages/ui/dist/globals.cssbefore Storybook runs.- Optionally, if
@hypr/uiexposes this stylesheet as a public entry (e.g.@hypr/ui/dist/globals.cssor similar), prefer a package-level import over a deep relative path for better encapsulation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/storybook/.storybook/preview.tsx(1 hunks)
⏰ 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). (5)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (macos, macos-14)
The Storybook preview imports built CSS from packages/ui/dist/globals.css, which requires the UI package to be built first. This updates the Netlify build command to run 'pnpm -F ui build' before building Storybook. Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Summary
This PR migrates Argos CI visual regression testing from
apps/webtoapps/storybook.Removed from apps/web:
@argos-ci/playwrightdependencyplaywright.config.tsvisual.spec.tstest fileweb_ci.yamlAdded to apps/storybook:
@argos-ci/cli,@argos-ci/storybook,@storybook/test-runner,@playwright/test,playwrightdependencies.storybook/test-runner.tswith Argos screenshot integrationtest-storybooknpm scriptstorybook_ci.yamlworkflow for visual testingscreenshotsadded to.gitignoreCI Workflow approach:
pnpm -F ui build) to generate processed Tailwind CSSpnpm -F storybook build)npx http-serverin backgroundpnpm exec test-storybook --url http://127.0.0.1:6006 --ciNote: The Storybook Test Runner approach was used instead of
@storybook/addon-vitestbecause the addon-vitest package only supports Storybook 9+/10+, while this project uses Storybook 8.6.14.Updates since last revision
NODE_NO_WARNINGS=1fromstorybook_ci.yaml(per CodeRabbit review - this flag suppresses critical warnings including security alerts)/product/opensourcefromapps/web/src/utils/sitemap.tsto fix the TypeScript error causing macOS CI to fail (the route doesn't exist in the route types)packages/ui/dist/globals.css) instead of source CSS (packages/ui/src/styles/globals.css). The source file contains unprocessed@tailwinddirectives that weren't being compiled in the static build.pnpm -F ui buildstep before Storybook build innetlify.tomlto ensure the UI package's CSS is built first.Review & Testing Checklist for Human
ARGOS_TOKENsecret is configured in GitHub repository settings/product/opensourceroute removal from sitemap is intentional (the route appears to not exist based on TypeScript types)Recommended test plan:
Notes
test-storybookcommand requiresNODE_OPTIONS=--experimental-vm-modulesfor the Storybook test runnerRequested by: yujonglee (@yujonglee)
Devin Session: https://app.devin.ai/sessions/cb3d3716c0614d119e7704ea65f55cec