-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(ci): trigger deployment fix #1510
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
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 implements a comprehensive trigger deployment fix that addresses multiple areas of the codebase. The changes primarily focus on standardizing trigger block configurations, enhancing the auto-layout system, optimizing the CI pipeline, and updating documentation across multiple languages.The core functional changes include adding the triggerAllowed: true property to all trigger block types (API, Chat, Manual, Input, and Schedule triggers) to ensure consistent trigger functionality across the workflow system. This standardization aligns all trigger blocks with the same configuration pattern and enables proper workflow initiation capabilities.
A significant portion of the PR involves refactoring the auto-layout system for workflow blocks. The changes introduce a more comprehensive BlockMetrics interface that includes padding and minimum dimension constraints, replacing the simpler BlockDimensions approach. The layout system now supports live measurements from the UI, allowing the auto-layout API to work with real-time block dimensions rather than potentially stale database values. Key improvements include better topological sorting for layer assignment, enhanced container sizing calculations, and pre-computation of block metrics for performance optimization.
The CI pipeline has been restructured to move Trigger.dev deployment earlier in the process (immediately after testing rather than after image builds), inline the image building logic from a separate workflow file, and introduce multi-architecture support for both AMD64 and ARM64 builds. The workflow now includes AWS integration for storing deployment versions in Parameter Store and removes the previous database migrations step.
Extensive documentation updates span multiple languages (German, Spanish, French, Japanese, and Chinese), standardizing content by removing redundant YAML code examples and improving translation quality. These changes make the documentation more maintainable while directing users to centralized references for detailed implementation guidance.
Additional enhancements include improving database migration idempotency, updating block ID generation to use crypto.randomUUID() for better uniqueness, and expanding layout state tracking to capture both width and height measurements for workflow blocks.
PR Description Notes:
- The PR description is incomplete and appears to use the default template without filling in the specific details about the changes made
- The summary section lacks a description of what the PR actually does
- No issue number is referenced despite the template format
- All checklist items remain unchecked
- No testing information or type of change is specified
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| apps/sim/blocks/blocks/api_trigger.ts | 5/5 | Added triggerAllowed: true property to enable API trigger functionality |
| apps/sim/blocks/blocks/schedule.ts | 5/5 | Added triggerAllowed: true property to standardize schedule trigger configuration |
| apps/sim/blocks/blocks/chat_trigger.ts | 5/5 | Added triggerAllowed: true property to align chat trigger with other trigger blocks |
| apps/sim/blocks/blocks/input_trigger.ts | 5/5 | Added triggerAllowed: true property to enable input trigger functionality |
| apps/sim/blocks/blocks/manual_trigger.ts | 4/5 | Added triggerAllowed: true property to complete trigger block standardization |
| apps/sim/lib/workflows/autolayout/containers.ts | 5/5 | Fixed critical bug where node.dimensions was referenced instead of node.metrics |
| apps/sim/lib/workflows/autolayout/utils.ts | 4/5 | Major refactor introducing BlockMetrics type and comprehensive layout utility functions |
| apps/sim/lib/workflows/autolayout/types.ts | 4/5 | Expanded BlockDimensions to BlockMetrics interface with padding and constraints |
| apps/sim/lib/workflows/autolayout/layering.ts | 4/5 | Rewritten layering algorithm with proper topological sort for dependency-based positioning |
| apps/sim/lib/workflows/autolayout/positioning.ts | 4/5 | Updated positioning calculations to use node.metrics instead of node.dimensions |
| apps/sim/lib/workflows/autolayout/incremental.ts | 5/5 | Refactored function name from getBlockDimensions to getBlockMetrics |
| apps/sim/lib/workflows/autolayout/index.ts | 5/5 | Added prepareBlockMetrics call and renamed exports for consistency |
| apps/sim/app/api/workflows/[id]/autolayout/route.ts | 4/5 | Enhanced API to accept live block measurements from UI for accurate layout calculations |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils.ts | 4/5 | Refactored getBlockDimensions to prioritize layout measurements and remove special cases |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/auto-layout.ts | 4/5 | Modified to send live block measurements to autolayout API instead of just options |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx | 4/5 | Extended layout tracking to capture both width and height dimensions with debounced updates |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 5/5 | Replaced timestamp-based ID generation with crypto.randomUUID() for better uniqueness |
| apps/sim/stores/workflows/workflow/store.ts | 5/5 | Added layout property to blocks for tracking measured dimensions |
| apps/sim/stores/workflows/workflow/types.ts | 4/5 | Added BlockLayoutState interface and integrated into BlockState for layout tracking |
| .github/workflows/ci.yml | 2/5 | Major CI restructuring with early Trigger.dev deployment, multi-arch builds, and removed migrations |
| .github/workflows/trigger-deploy.yml | 4/5 | Enhanced with AWS integration for version tracking in Parameter Store |
| packages/db/migrations/0095_cheerful_albert_cleary.sql | 4/5 | Made database migration idempotent with IF NOT EXISTS clauses |
| apps/docs/content/docs/es/triggers/api.mdx | 5/5 | Removed YAML code example and improved Spanish translation consistency |
| apps/docs/content/docs/ja/triggers/api.mdx | 5/5 | Removed YAML code example and improved Japanese phrasing |
| apps/docs/content/docs/fr/triggers/api.mdx | 5/5 | Removed YAML code example and standardized French terminology |
| apps/docs/content/docs/zh/triggers/api.mdx | 4/5 | Removed YAML code example and improved Chinese translation quality |
| apps/docs/content/docs/de/triggers/api.mdx | 4/5 | Removed YAML code example and improved German grammar |
Confidence score: 3/5
- This PR requires careful review due to significant CI pipeline changes and complex autolayout system refactoring
- Score reflects concerns about the major CI restructuring that removes migrations and moves deployment timing, plus the incomplete PR description
- Pay close attention to
.github/workflows/ci.ymland autolayout system files for potential deployment or layout calculation issues
Sequence Diagram
sequenceDiagram
participant Dev as Developer
participant GitHub as GitHub
participant CI as CI Pipeline
participant TestBuild as Test & Build Job
participant TriggerDep as Trigger Deploy Job
participant BuildAMD64 as Build AMD64 Job
participant BuildARM64 as Build ARM64 Job (GHCR)
participant Manifests as Create Manifests Job
participant ProcessDocs as Process Docs Job
participant TriggerAPI as Trigger.dev API
participant AWS as AWS ECR
participant GHCR as GitHub Container Registry
participant SSM as AWS Parameter Store
Dev->>GitHub: Push to main/staging branch
GitHub->>CI: Trigger CI workflow
CI->>TestBuild: Start test and build
TestBuild->>TestBuild: Run tests and build
TestBuild->>TriggerDep: Test & build complete
TriggerDep->>TriggerAPI: Deploy to Trigger.dev
TriggerAPI-->>TriggerDep: Return deployment version
TriggerDep->>SSM: Store version in Parameter Store
TriggerDep->>BuildAMD64: Trigger deployment complete
BuildAMD64->>AWS: Login to ECR
BuildAMD64->>GHCR: Login to GHCR (main only)
BuildAMD64->>BuildAMD64: Build AMD64 images
BuildAMD64->>AWS: Push to ECR (staging/latest tag)
BuildAMD64->>GHCR: Push AMD64 to GHCR (main only)
par Build ARM64 in parallel (main only)
TriggerDep->>BuildARM64: Trigger deployment complete
BuildARM64->>GHCR: Login to GHCR
BuildARM64->>BuildARM64: Build ARM64 images
BuildARM64->>GHCR: Push ARM64 to GHCR
end
BuildAMD64->>Manifests: AMD64 build complete (main only)
BuildARM64->>Manifests: ARM64 build complete (main only)
Manifests->>GHCR: Create multi-arch manifests
BuildAMD64->>ProcessDocs: AMD64 build complete
ProcessDocs->>ProcessDocs: Process documentation embeddings
Additional Comments (10)
-
apps/docs/content/docs/fr/yaml/blocks/loop.mdx, line 62 (link)syntax: The union type syntax 'string | string[]' may not be valid YAML schema syntax. Standard JSON Schema uses 'oneOf' or 'anyOf' for union types.
-
apps/docs/content/docs/fr/yaml/blocks/loop.mdx, line 70 (link)syntax: The 'note' property at the same level as 'properties' is not standard YAML schema syntax. Consider moving this to the description field or as a comment.
-
apps/docs/content/docs/es/yaml/blocks/loop.mdx, line 62 (link)syntax: Type definition syntax 'string | string[]' is not valid YAML schema syntax - should use 'oneOf' or 'anyOf' instead
-
apps/docs/content/docs/es/yaml/blocks/loop.mdx, line 65 (link)syntax: Same type definition issue here - 'string | string[]' should use proper YAML schema syntax
-
apps/docs/content/docs/de/yaml/blocks/loop.mdx, line 62 (link)syntax: Type definition
string | string[]may not be valid YAML schema syntax - typically union types useoneOforanyOf -
apps/docs/content/docs/de/yaml/blocks/loop.mdx, line 65 (link)syntax: Same type definition issue as above
-
apps/docs/content/docs/de/yaml/blocks/loop.mdx, line 70 (link)style: Note about mutual exclusivity should be in the description field rather than a standalone 'note' property in YAML schema
-
apps/sim/app/api/workflows/[id]/autolayout/route.ts, line 44 (link)style: Using
z.any()removes type safety for the workflow data structures. Consider using proper Zod schemas forBlockState,Edge,Loop, andParalleltypes to maintain validation and type safety.Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
-
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/auto-layout.ts, line 87 (link)style: The spread of layoutOptions followed by explicit block data could potentially overwrite properties if layoutOptions contains blocks/edges/loops/parallels. Consider restructuring to avoid conflicts
-
apps/docs/content/docs/zh/blocks/agent.mdx, line 179 (link)style: Tab labels should be translated to Chinese for consistency with the rest of the document. Consider changing to ['配置', '变量', '结果']
36 files reviewed, 10 comments
Summary
Fix trigger deployment
Type of Change
Testing
n/a
Checklist