-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(trigger): inject project id env var in correctly #1520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| run: bun install | ||
|
|
||
| - name: Deploy to Trigger.dev (Staging) | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/staging' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check is done earlier in ci.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR fixes a bug in the Trigger.dev deployment workflow by ensuring the project ID environment variable is properly injected during deployments. The changes address two main issues:Workflow Environment Variable Fix: The GitHub Actions workflow .github/workflows/trigger-deploy.yml now includes the missing TRIGGER_PROJECT_ID environment variable at the workflow level, making it available to all deployment steps. This environment variable is sourced from GitHub secrets and is required by the Trigger.dev CLI to identify which project to deploy to.
Deployment Condition Simplification: The workflow conditions have been simplified by removing redundant github.event_name == 'push' checks from the deployment steps. Since this is a reusable workflow called from the main CI pipeline (as seen in ci.yml), the push event filtering is already handled upstream. This change makes the conditions more reliable since workflow_call events don't have the same event_name property as push events.
Configuration Update: The Trigger.dev configuration file apps/sim/trigger.config.ts has been updated to use a non-null assertion operator (!) instead of a nullish coalescing fallback to an empty string. This change assumes the environment variable will always be present during deployment contexts where the GitHub secret is provided.
These changes work together as part of the broader CI/CD pipeline where the main ci.yml workflow calls the trigger-deploy.yml reusable workflow after successful image builds, ensuring proper deployment to both staging and production environments based on the target branch.
Changed Files
| Filename | Score | Overview |
|---|---|---|
| .github/workflows/trigger-deploy.yml | 4/5 | Adds missing TRIGGER_PROJECT_ID environment variable and simplifies deployment conditions by removing redundant push event checks |
| apps/sim/trigger.config.ts | 2/5 | Replaces safe fallback string with non-null assertion operator, potentially introducing runtime errors |
Confidence score: 3/5
- This PR addresses a legitimate deployment bug but introduces potential runtime risk in non-deployment contexts
- Score reflects the workflow improvements being solid while the config change could cause issues during local development
- Pay close attention to apps/sim/trigger.config.ts as the non-null assertion could fail if TRIGGER_PROJECT_ID is undefined locally
Sequence Diagram
sequenceDiagram
participant User
participant GitHub
participant TriggerWorkflow as "Trigger Deploy Workflow"
participant NodeSetup as "Node/Bun Setup"
participant BundleInstall as "Bundle Install"
participant TriggerDeploy as "Trigger.dev Deploy"
User->>GitHub: "Push to staging/main branch"
GitHub->>TriggerWorkflow: "Trigger workflow_call/workflow_dispatch"
Note over TriggerWorkflow: Set environment variables:<br/>TRIGGER_ACCESS_TOKEN<br/>TRIGGER_PROJECT_ID
TriggerWorkflow->>TriggerWorkflow: "Checkout code"
TriggerWorkflow->>NodeSetup: "Setup Node.js (latest)"
NodeSetup-->>TriggerWorkflow: "Node.js ready"
TriggerWorkflow->>NodeSetup: "Setup Bun (latest)"
NodeSetup-->>TriggerWorkflow: "Bun ready"
TriggerWorkflow->>BundleInstall: "bun install"
BundleInstall-->>TriggerWorkflow: "Dependencies installed"
alt Branch is staging
TriggerWorkflow->>TriggerDeploy: "Deploy to staging environment"
Note over TriggerDeploy: npx trigger.dev@4.0.4 deploy -e staging
else Branch is main
TriggerWorkflow->>TriggerDeploy: "Deploy to production environment"
Note over TriggerDeploy: npx trigger.dev@4.0.4 deploy
end
TriggerDeploy-->>TriggerWorkflow: "Deployment complete"
TriggerWorkflow-->>GitHub: "Workflow finished"
Context used:
Context from dashboard - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used... (source)
2 files reviewed, no comments
Summary
Injects trigger project id var in correctly. Check for push done earlier so just removing it here.
Type of Change
Testing
Directly deploying locally
Checklist