fix: resolve web public assets via alias by rev2ret · Pull Request #822 · accomplish-ai/coworker

rev2ret · 2026-03-26T03:23:11Z

Summary

add an @assets alias for the web app in Vite, Vitest, and TypeScript config
switch web client asset imports from root-absolute /assets/... paths to @assets/...
avoid the Windows file:///assets/... resolution failure that was preventing several integration suites from loading

Testing

corepack pnpm --filter @accomplish/web typecheck
corepack pnpm --filter @accomplish/web build
corepack pnpm --filter @accomplish/web exec vitest run __tests__/integration/renderer/components/TaskLauncher.integration.test.tsx --config vitest.integration.config.ts

Notes

corepack pnpm --filter @accomplish/web test:integration now gets past the previous asset import crash on Windows.
That full run still reports existing assertion failures in SettingsDialog.integration.test.tsx and Execution.integration.test.tsx, which are outside the scope of this asset-resolution fix.

Summary by CodeRabbit

Chores
- Updated test configuration to resolve asset paths correctly during testing.

orcaman · 2026-03-26T03:23:24Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

coderabbitai · 2026-03-26T03:23:31Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01f45626-2ccd-4dbe-82de-265b7f9c8037

📥 Commits

Reviewing files that changed from the base of the PR and between accb796 and e6b58d7.

📒 Files selected for processing (2)

apps/web/vitest.integration.config.ts
apps/web/vitest.unit.config.ts

✅ Files skipped from review due to trivial changes (2)

apps/web/vitest.integration.config.ts
apps/web/vitest.unit.config.ts

📝 Walkthrough

Walkthrough

Both Vitest configuration files now include a /assets path alias that resolves to the public/assets directory. This enables tests to reference assets using the /assets path instead of relative paths, applied uniformly across integration and unit test configurations.

Changes

Cohort / File(s)	Summary
Test Configuration Aliases `apps/web/vitest.integration.config.ts`, `apps/web/vitest.unit.config.ts`	Added `/assets` resolve alias mapping to `public/assets` directory in both Vitest configurations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

mavishay

Poem

🐰 Hop, hop—the assets find their way,
A path alias here to stay,
Tests now dance with /assets bright,
No relative paths needed tonight! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: resolve web public assets via alias' directly describes the main change—adding asset path aliases to Vitest configuration files to resolve web public assets.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mavishay

Thanks for the contribution and for digging into the Windows test resolution issue — this is a real problem worth solving.

After reviewing the PR, we have a few concerns before we can merge:

1. Potential conflict with CLAUDE.md guidance
Our codebase guidelines specify that image assets should use ES module imports with /assets/... paths to ensure they work correctly in the packaged Electron app. Switching to an @assets alias changes how Vite handles these at build time — assets imported via alias go through Vite's asset pipeline (content-hashed, potentially inlined), whereas /assets/... files from the public/ directory are served as-is. This is a subtle but meaningful behavioral difference that could affect the packaged desktop build.

2. Needs verification in the packaged app
Could you confirm that the Electron app still loads all assets correctly after this change? Specifically:

Run pnpm build:desktop and verify the packaged app renders logos and icons as expected.
The key concern is whether the @assets alias resolves correctly in the renderer process at runtime in the packaged (non-dev) build.

3. Scope of the fix
A global refactor of 24 files is a large blast radius for a Windows test environment issue. Have you explored a more targeted fix — for example, adjusting the jsdom/vitest configuration to handle public/ assets correctly in tests without changing how the app imports them?

We appreciate the effort here and would love to find a path to merging a fix for the Windows issue. Happy to collaborate on a scoped solution if the full alias approach turns out to have packaged-app regressions.

mavishay

Thanks for the contribution and for digging into the Windows test resolution issue — this is a real problem worth solving.

After reviewing the PR, we have a few concerns before we can merge:

1. Potential conflict with our asset import convention
Our codebase guidelines specify that image assets should use ES module imports with /assets/... paths to ensure they work correctly in the packaged Electron app. Switching to an @assets alias changes how Vite handles these at build time — assets imported via alias go through Vite's asset pipeline (content-hashed, potentially inlined), whereas /assets/... files from the public/ directory are served as-is. This is a subtle but meaningful behavioral difference that could affect the packaged desktop build.

2. Needs verification in the packaged app
Could you confirm that the Electron app still loads all assets correctly after this change? Specifically:

Run pnpm build:desktop and verify the packaged app renders logos and icons as expected.
The key concern is whether the @assets alias resolves correctly in the renderer process at runtime in the packaged (non-dev) build.

3. Scope of the fix
A global refactor of 24 files is a large blast radius for a Windows test environment issue. Have you explored a more targeted fix — for example, adjusting the jsdom/vitest configuration to handle public/ assets correctly in tests without changing how the app imports them?

We appreciate the effort here and would love to find a path to merging a fix for the Windows issue. Happy to collaborate on a scoped solution if the packaged-app concerns are addressed.

rev2ret · 2026-04-17T13:24:50Z

Reworked this PR to keep the app/runtime behavior aligned with the existing /assets/... convention and scope the Windows fix to Vitest only.

What changed:

restored the web client imports back to /assets/...
removed the @assets alias from Vite and TS config
added a /assets -> public/assets resolver only in vitest.integration.config.ts and vitest.unit.config.ts

Verification:

corepack pnpm --filter @accomplish/web typecheck
corepack pnpm --filter @accomplish/web build
corepack pnpm --filter @accomplish/desktop exec tsc
corepack pnpm --filter @accomplish/desktop exec vite build
corepack pnpm --filter @accomplish/web exec vitest run __tests__/integration/renderer/components/TaskLauncher.integration.test.tsx --config vitest.integration.config.ts

I also tried the remaining desktop MCP bundle step, but build:mcp-tools:dev is currently failing in this environment on unresolved @modelcontextprotocol/sdk/* imports during bundling, which appears unrelated to this asset-resolution change.

fix: resolve web public assets via alias

accb796

mavishay reviewed Apr 15, 2026

View reviewed changes

fix: scope Windows asset resolution to vitest

e6b58d7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: resolve web public assets via alias#822

fix: resolve web public assets via alias#822
rev2ret wants to merge 2 commits into
accomplish-ai:mainfrom
rev2ret:fix/windows-web-public-asset-urls

rev2ret commented Mar 26, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

orcaman commented Mar 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Mar 26, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

mavishay left a comment

Uh oh!

mavishay left a comment

Uh oh!

rev2ret commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rev2ret commented Mar 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Notes

Summary by CodeRabbit

Uh oh!

orcaman commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

coderabbitai Bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

mavishay left a comment

Choose a reason for hiding this comment

Uh oh!

mavishay left a comment

Choose a reason for hiding this comment

Uh oh!

rev2ret commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rev2ret commented Mar 26, 2026 •

edited by coderabbitai Bot

Loading

orcaman commented Mar 26, 2026 •

edited

Loading

coderabbitai Bot commented Mar 26, 2026 •

edited

Loading