fix(webapp): runs not appearing in local development #2599
Conversation
|
WalkthroughUpdates runsRepository.server.ts to determine the default RunsRepository based on environment variables when defaultRepository is not explicitly provided. It now imports env and selects "clickhouse" only if RUN_REPLICATION_ENABLED equals "1" and RUN_REPLICATION_CLICKHOUSE_URL is set; otherwise it selects "postgres". Previously, the fallback default was always "clickhouse". No changes to exported/public declarations. Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (21)
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. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.ts(2 hunks)apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts(2 hunks)apps/webapp/app/services/runsRepositoryFactory.server.ts(1 hunks)apps/webapp/app/v3/services/bulk/BulkActionV2.server.ts(3 hunks)apps/webapp/test/runsRepositoryFactory.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/test/runsRepositoryFactory.test.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/test/runsRepositoryFactory.test.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json
Files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/test/runsRepositoryFactory.test.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly
Files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead
Files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
apps/webapp/test/runsRepositoryFactory.test.ts
{apps/webapp/**/__tests__/**/*.{ts,tsx},apps/webapp/**/*.{test,spec}.{ts,tsx}}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
{apps/webapp/**/__tests__/**/*.{ts,tsx},apps/webapp/**/*.{test,spec}.{ts,tsx}}: Do not import app/env.server.ts into tests, either directly or indirectly
Tests should only import classes/functions from files under apps/webapp/app/**/*.ts
Files:
apps/webapp/test/runsRepositoryFactory.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests must use Vitest
Tests should avoid mocks or stubs and use helpers from @internal/testcontainers when Redis or Postgres are needed
Test files live beside the files under test and should use descriptive describe and it blocks
Files:
apps/webapp/test/runsRepositoryFactory.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/v3/services/bulk/BulkActionV2.server.tsapps/webapp/app/services/runsRepositoryFactory.server.tsapps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.tsapps/webapp/app/presenters/v3/NextRunListPresenter.server.ts
🧬 Code graph analysis (5)
apps/webapp/app/v3/services/bulk/BulkActionV2.server.ts (1)
apps/webapp/app/services/runsRepositoryFactory.server.ts (1)
createRunsRepository(6-21)
apps/webapp/app/services/runsRepositoryFactory.server.ts (2)
internal-packages/clickhouse/src/index.ts (1)
ClickHouse(61-172)apps/webapp/app/services/runsRepository/runsRepository.server.ts (1)
RunsRepository(120-294)
apps/webapp/test/runsRepositoryFactory.test.ts (2)
internal-packages/clickhouse/src/index.ts (1)
ClickHouse(61-172)apps/webapp/app/services/runsRepositoryFactory.server.ts (1)
createRunsRepository(6-21)
apps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.ts (1)
apps/webapp/app/services/runsRepositoryFactory.server.ts (1)
createRunsRepository(6-21)
apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts (1)
apps/webapp/app/services/runsRepositoryFactory.server.ts (1)
createRunsRepository(6-21)
⏰ 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). (22)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (5)
apps/webapp/test/runsRepositoryFactory.test.ts (1)
1-86: Update tests after factory refactor to avoid indirect env.server import.The test indirectly imports env.server by importing createRunsRepository, violating the guideline: "Do not import app/env.server.ts into tests, either directly or indirectly." The mocking approach (lines 2-8) is fragile.
Once the factory is refactored to accept configuration via options, update these tests to pass explicit configuration instead of mocking env.server.
As per coding guidelines
Example after factory refactor:
it("should default to postgres when replication is not enabled", () => { const repository = createRunsRepository({ clickhouse: mockClickhouse, prisma: mockPrisma, isReplicationEnabled: false, isClickHouseConfigured: true, }); expect(repository).toBeDefined(); expect(repository.listRuns).toBeDefined(); });This eliminates the need for mocking and aligns with coding guidelines.
apps/webapp/app/presenters/v3/NextRunListPresenter.server.ts (1)
156-159: LGTM!The migration to the factory pattern is correct. The factory call properly passes the required clickhouse and prisma clients.
apps/webapp/app/presenters/v3/CreateBulkActionPresenter.server.ts (1)
27-30: LGTM!Consistent migration to the factory pattern with correct parameters.
apps/webapp/app/v3/services/bulk/BulkActionV2.server.ts (2)
41-44: LGTM!The factory usage is correct and consistent with the refactor pattern.
148-151: LGTM!The second factory usage in the process method is also correct and consistent.
… replication and ClickHouse configuration
…itory instantiation
1 participant
🐛 Bug Description
Task runs execute successfully but do not appear on the dashboard's "Runs" page in local development environments.
Root Cause: The dashboard was always querying ClickHouse by default, but in local development:
RUN_REPLICATION_ENABLEDnot set)✅ Checklist
Testing
✅ Bug Fixed in Local Development
CONTRIBUTING.mdsetup (withoutRUN_REPLICATION_ENABLED=1)pnpm run dockerpnpm run dev --filter webapp✅ Production Behavior Unchanged
RUN_REPLICATION_ENABLED=1andRUN_REPLICATION_CLICKHOUSE_URL✅ No Lint Errors
Changelog
Screenshots
💯