chore: init workflow stores #757
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughOpenAPI and generated Go models now require Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 Fix all issues with AI agents
In `@apps/workspace-engine/pkg/workspace/store/workflow_task_templates.go`:
- Around line 29-31: In Upsert, ensure the passed id and workflowTaskTemplate.Id
are consistent before storing: in the WorkflowTaskTemplates.Upsert function,
check the oapi.WorkflowTaskTemplate.Id and if it's empty or differs from the id
parameter, set workflowTaskTemplate.Id = id so the object used with
w.repo.WorkflowTaskTemplates.Set and w.store.changeset.RecordUpsert has a
matching Id; this normalizes stored state and prevents tracking inconsistencies.
In `@apps/workspace-engine/pkg/workspace/store/workflow_templates.go`:
- Around line 29-31: In WorkflowTemplates.Upsert, ensure the provided id and
workflowTemplate.Id stay consistent before calling w.repo.WorkflowTemplates.Set
and w.store.changeset.RecordUpsert: if workflowTemplate.Id is empty, set it to
id; if it is non-empty and differs from id, either return an error or normalize
by overwriting it with id (choose based on project convention), and validate
that the final workflowTemplate.Id is non-empty; update only after this
normalization so repo.WorkflowTemplates.Set(id, workflowTemplate) and
store.changeset.RecordUpsert(workflowTemplate) operate on a consistent object.
🧹 Nitpick comments (6)
apps/workspace-engine/pkg/workspace/store/workflow_tasks.go (2)
9-41: Add Go doc comments for exported WorkflowTasks APIs.Please add GoDoc for
NewWorkflowTasks,WorkflowTasks, and the exported methods to satisfy workspace-engine guidelines.As per coding guidelines, please document exported functions/types/methods.📌 Example GoDoc additions
+// NewWorkflowTasks constructs a workflow-task store backed by the in-memory repository. func NewWorkflowTasks(store *Store) *WorkflowTasks { return &WorkflowTasks{ repo: store.repo, store: store, } } +// WorkflowTasks provides accessors and change-tracking for workflow tasks. type WorkflowTasks struct { repo *repository.InMemoryStore store *Store }
29-31: Normalize Upsert to use entity's Id field consistently.The method stores entities using the caller-supplied
idparameter but RecordUpsert tracks changes usingworkflowTask.Id(via CompactionKey). If these diverge, the in-memory map key and persistence key become inconsistent. Align the map key with the entity's Id to ensure consistency between in-memory and persistence layers.🔧 Suggested normalization
func (w *WorkflowTasks) Upsert(ctx context.Context, id string, workflowTask *oapi.WorkflowTask) { + if workflowTask != nil && workflowTask.Id != "" { + id = workflowTask.Id + } w.repo.WorkflowTasks.Set(id, workflowTask) w.store.changeset.RecordUpsert(workflowTask) }apps/workspace-engine/pkg/workspace/store/workflows.go (2)
9-41: Add Go doc comments for exported Workflows APIs.Please add GoDoc for
NewWorkflows,Workflows, and the exported methods to satisfy workspace-engine guidelines.As per coding guidelines, please document exported functions/types/methods.📌 Example GoDoc additions
+// NewWorkflows constructs a workflow store backed by the in-memory repository. func NewWorkflows(store *Store) *Workflows { return &Workflows{ repo: store.repo, store: store, } } +// Workflows provides accessors and change-tracking for workflows. type Workflows struct { repo *repository.InMemoryStore store *Store }
29-31: Normalize upsert key toworkflow.Idto prevent in-memory and persistence key drift.
Set(id, workflow)uses the caller-suppliedidparameter, whileRecordUpsert(workflow)usesworkflow.CompactionKey()which returnsworkflow.Id. If these diverge, the in-memory cache key and persistence entity key will mismatch. Normalize to useworkflow.Idconsistently:🔧 Suggested fix
func (w *Workflows) Upsert(ctx context.Context, id string, workflow *oapi.Workflow) { + if workflow != nil && workflow.Id != "" { + id = workflow.Id + } w.repo.Workflows.Set(id, workflow) w.store.changeset.RecordUpsert(workflow) }apps/workspace-engine/pkg/workspace/store/workflow_task_templates.go (1)
9-19: Add Go doc comments for exported symbols.Constructor and type are exported but undocumented. As per coding guidelines, please add Go doc comments for
NewWorkflowTaskTemplatesandWorkflowTaskTemplates(and ideally its exported methods) to satisfy the workspace-engine docs requirement.As per coding guidelines, please document exported functions/types/methods.
apps/workspace-engine/pkg/workspace/store/workflow_templates.go (1)
9-19: Add Go doc comments for exported symbols.Constructor and type are exported but undocumented. As per coding guidelines, please add Go doc comments for
NewWorkflowTemplatesandWorkflowTemplates(and ideally its exported methods) to satisfy the workspace-engine docs requirement.As per coding guidelines, please document exported functions/types/methods.
| func (w *WorkflowTaskTemplates) Upsert(ctx context.Context, id string, workflowTaskTemplate *oapi.WorkflowTaskTemplate) { | ||
| w.repo.WorkflowTaskTemplates.Set(id, workflowTaskTemplate) | ||
| w.store.changeset.RecordUpsert(workflowTaskTemplate) |
There was a problem hiding this comment.
Enforce id ↔ workflowTaskTemplate.Id consistency in Upsert.
With WorkflowTaskTemplate.Id now required, storing by id while allowing a mismatched or empty workflowTaskTemplate.Id can create inconsistent state and break change tracking. Normalize or validate before storing.
🛠️ Suggested fix
func (w *WorkflowTaskTemplates) Upsert(ctx context.Context, id string, workflowTaskTemplate *oapi.WorkflowTaskTemplate) {
+ if workflowTaskTemplate == nil {
+ return
+ }
+ if workflowTaskTemplate.Id != "" && workflowTaskTemplate.Id != id {
+ workflowTaskTemplate.Id = id
+ } else if workflowTaskTemplate.Id == "" {
+ workflowTaskTemplate.Id = id
+ }
w.repo.WorkflowTaskTemplates.Set(id, workflowTaskTemplate)
w.store.changeset.RecordUpsert(workflowTaskTemplate)
}🤖 Prompt for AI Agents
In `@apps/workspace-engine/pkg/workspace/store/workflow_task_templates.go` around
lines 29 - 31, In Upsert, ensure the passed id and workflowTaskTemplate.Id are
consistent before storing: in the WorkflowTaskTemplates.Upsert function, check
the oapi.WorkflowTaskTemplate.Id and if it's empty or differs from the id
parameter, set workflowTaskTemplate.Id = id so the object used with
w.repo.WorkflowTaskTemplates.Set and w.store.changeset.RecordUpsert has a
matching Id; this normalizes stored state and prevents tracking inconsistencies.
| func (w *WorkflowTemplates) Upsert(ctx context.Context, id string, workflowTemplate *oapi.WorkflowTemplate) { | ||
| w.repo.WorkflowTemplates.Set(id, workflowTemplate) | ||
| w.store.changeset.RecordUpsert(workflowTemplate) |
There was a problem hiding this comment.
Enforce id ↔ workflowTemplate.Id consistency in Upsert.
With WorkflowTemplate.Id required, storing by id while allowing a mismatched or empty workflowTemplate.Id can create inconsistent state and break change tracking. Normalize or validate before storing.
🛠️ Suggested fix
func (w *WorkflowTemplates) Upsert(ctx context.Context, id string, workflowTemplate *oapi.WorkflowTemplate) {
+ if workflowTemplate == nil {
+ return
+ }
+ if workflowTemplate.Id != "" && workflowTemplate.Id != id {
+ workflowTemplate.Id = id
+ } else if workflowTemplate.Id == "" {
+ workflowTemplate.Id = id
+ }
w.repo.WorkflowTemplates.Set(id, workflowTemplate)
w.store.changeset.RecordUpsert(workflowTemplate)
}📝 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.
| func (w *WorkflowTemplates) Upsert(ctx context.Context, id string, workflowTemplate *oapi.WorkflowTemplate) { | |
| w.repo.WorkflowTemplates.Set(id, workflowTemplate) | |
| w.store.changeset.RecordUpsert(workflowTemplate) | |
| func (w *WorkflowTemplates) Upsert(ctx context.Context, id string, workflowTemplate *oapi.WorkflowTemplate) { | |
| if workflowTemplate == nil { | |
| return | |
| } | |
| if workflowTemplate.Id != "" && workflowTemplate.Id != id { | |
| workflowTemplate.Id = id | |
| } else if workflowTemplate.Id == "" { | |
| workflowTemplate.Id = id | |
| } | |
| w.repo.WorkflowTemplates.Set(id, workflowTemplate) | |
| w.store.changeset.RecordUpsert(workflowTemplate) | |
| } |
🤖 Prompt for AI Agents
In `@apps/workspace-engine/pkg/workspace/store/workflow_templates.go` around lines
29 - 31, In WorkflowTemplates.Upsert, ensure the provided id and
workflowTemplate.Id stay consistent before calling w.repo.WorkflowTemplates.Set
and w.store.changeset.RecordUpsert: if workflowTemplate.Id is empty, set it to
id; if it is non-empty and differs from id, either return an error or normalize
by overwriting it with id (choose based on project convention), and validate
that the final workflowTemplate.Id is non-empty; update only after this
normalization so repo.WorkflowTemplates.Set(id, workflowTemplate) and
store.changeset.RecordUpsert(workflowTemplate) operate on a consistent object.
| WorkflowTaskTemplate: { | ||
| type: 'object', | ||
| required: ['name', 'jobAgent'], | ||
| required: ['id', 'name', 'jobAgent'], |
There was a problem hiding this comment.
follow github patterns, you dont provide an id to the steps
There was a problem hiding this comment.
after discussion - id is internal, later we will add referenceId which will be "id" in the yaml
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.