fix(streams): restore realtime stream writing for v3 tasks#2675
fix(streams): restore realtime stream writing for v3 tasks#2675
Conversation
|
WalkthroughThis change adds backwards-compatibility and authentication to a realtime streaming endpoint. It introduces a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (1)
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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/routes/realtime.v1.streams.$runId.$streamId.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/routes/realtime.v1.streams.$runId.$streamId.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/routes/realtime.v1.streams.$runId.$streamId.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/routes/realtime.v1.streams.$runId.$streamId.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/routes/realtime.v1.streams.$runId.$streamId.ts
🧬 Code graph analysis (1)
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts (2)
apps/webapp/app/db.server.ts (1)
$replica(103-106)apps/webapp/app/services/realtime/v1StreamsGlobal.server.ts (1)
getRealtimeStreamInstance(32-61)
⏰ 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). (23)
- 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 (6, 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 (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts (8)
13-21: LGTM!Parameter validation is properly implemented with appropriate error handling.
45-47: LGTM!Header extraction with sensible defaults is correctly implemented.
49-51: LGTM!Body presence validation is appropriate.
53-63: LGTM!Resume chunk validation is thorough with appropriate error handling for invalid numeric values.
77-107: LGTM!Authentication and authorization configuration is properly implemented with environment scoping and comprehensive scope mappings. This correctly contrasts with the backwards-compatible action path.
108-137: LGTM!The authenticated loader implementation is well-structured with proper timeout validation and uses the authenticated environment correctly.
1-6: No issues found - code is type-safe.The review comment incorrectly identifies a type compatibility problem. The Prisma query on lines 28-35 explicitly includes
project,organization, andorgMember, sorun.runtimeEnvironmenthas all required fields. TypeScript's structural typing allows values with required fields to satisfy parameters expecting optional fields. The developer already verified this correctness with the comment on line 65: "The runtimeEnvironment from the run is already in the correct shape for AuthenticatedEnvironment."Likely an incorrect or invalid review comment.
23-43: Backwards-compatible endpoint lacks security controls; verify scope of legacy client support.The
actionfunction (lines 14-70) intentionally bypasses environment scoping and requires no authentication—returning run data based onrunIdalone. While documented as backwards compatibility, this creates a security boundary different from the authenticatedloaderpath (lines 72+), which enforces environment scoping and authorization.Ensure:
- This endpoint is only available to legacy clients that cannot be updated
- The security implications of cross-environment
runIdlookup have been reviewed- There is a deprecation plan with timeline to remove this path
3 participants
No description provided.