chore: workflows engine working e2e#892
Merged
adityachoudhari26 merged 2 commits intomainfrom Mar 30, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR wires a PostgreSQL connection pool through HTTP service initialization layers and updates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/workspace-engine/svc/controllers/jobdispatch/setters_postgres.go (1)
102-104: Emit a trace event before returning on empty/zero release ID.Line 103 returns silently, while other dispatch-skip branches add span events. Add one event here for trace/debug parity.
Suggested patch
releaseID := existingJob.ReleaseId if releaseID == "" || releaseID == "00000000-0000-0000-0000-000000000000" { + span.AddEvent("skipping progression dispatch - release id is empty or zero UUID") return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/workspace-engine/svc/controllers/jobdispatch/setters_postgres.go` around lines 102 - 104, The early return when releaseID == "" || releaseID == "00000000-0000-0000-0000-000000000000" returns silently; add a trace/span event just before the return for parity with other skip branches. In the block that checks releaseID, call the existing span.AddEvent (or span.Event equivalent used elsewhere in this file) with a clear message like "dispatch skip: empty/zero releaseID" and include the releaseID value as an attribute if your span API supports attributes, then return nil as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/workspace-engine/svc/http/server/openapi/workflows/setters.go`:
- Around line 106-109: The error message for the uuid.Parse call should
reference the actual field being parsed: change the fmt.Errorf in the
uuid.Parse(jobAgent.Ref) error branch so it mentions "job agent ref" (or
jobAgent.Ref) instead of "job agent id"; update the error creation where
uuid.Parse(jobAgent.Ref) is used (the block that assigns jobAgentIDUUID) to
return a matching message that includes the underlying error.
---
Nitpick comments:
In `@apps/workspace-engine/svc/controllers/jobdispatch/setters_postgres.go`:
- Around line 102-104: The early return when releaseID == "" || releaseID ==
"00000000-0000-0000-0000-000000000000" returns silently; add a trace/span event
just before the return for parity with other skip branches. In the block that
checks releaseID, call the existing span.AddEvent (or span.Event equivalent used
elsewhere in this file) with a clear message like "dispatch skip: empty/zero
releaseID" and include the releaseID value as an attribute if your span API
supports attributes, then return nil as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bc90b208-c83e-4301-84cc-09a4ca3af7d8
📒 Files selected for processing (12)
apps/workspace-engine/main.goapps/workspace-engine/oapi/openapi.jsonapps/workspace-engine/oapi/spec/schemas/workflows.jsonnetapps/workspace-engine/pkg/oapi/oapi.gen.goapps/workspace-engine/pkg/oapi/persistence.goapps/workspace-engine/svc/controllers/jobdispatch/setters_postgres.goapps/workspace-engine/svc/http/http.goapps/workspace-engine/svc/http/server/openapi/server.goapps/workspace-engine/svc/http/server/openapi/workflows/setters.goapps/workspace-engine/svc/http/server/openapi/workflows/workflows.goapps/workspace-engine/svc/http/server/openapi/workflows/workflows_test.goapps/workspace-engine/svc/http/server/server.go
💤 Files with no reviewable changes (2)
- apps/workspace-engine/pkg/oapi/oapi.gen.go
- apps/workspace-engine/oapi/openapi.json
Comment on lines
+106
to
109
| jobAgentIDUUID, err := uuid.Parse(jobAgent.Ref) | ||
| if err != nil { | ||
| return fmt.Errorf("parse job agent id: %w", err) | ||
| } |
Contributor
There was a problem hiding this comment.
Update error message to match the field being parsed.
The error message references "job agent id" but the code now parses jobAgent.Ref. This inconsistency could confuse debugging efforts.
Suggested fix
jobAgentIDUUID, err := uuid.Parse(jobAgent.Ref)
if err != nil {
- return fmt.Errorf("parse job agent id: %w", err)
+ return fmt.Errorf("parse job agent ref: %w", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| jobAgentIDUUID, err := uuid.Parse(jobAgent.Ref) | |
| if err != nil { | |
| return fmt.Errorf("parse job agent id: %w", err) | |
| } | |
| jobAgentIDUUID, err := uuid.Parse(jobAgent.Ref) | |
| if err != nil { | |
| return fmt.Errorf("parse job agent ref: %w", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/workspace-engine/svc/http/server/openapi/workflows/setters.go` around
lines 106 - 109, The error message for the uuid.Parse call should reference the
actual field being parsed: change the fmt.Errorf in the uuid.Parse(jobAgent.Ref)
error branch so it mentions "job agent ref" (or jobAgent.Ref) instead of "job
agent id"; update the error creation where uuid.Parse(jobAgent.Ref) is used (the
block that assigns jobAgentIDUUID) to return a matching message that includes
the underlying error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
API Changes
idfield; now uses alternative identifier for job agent references.Bug Fixes
Tests