Continuous Review, Work Items, and Refinery-Grade Promotion Queues#47
Continuous Review, Work Items, and Refinery-Grade Promotion Queues#47ScottArbeit merged 13 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55e407d5d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/Grace.Server/Queue.Server.fs
Outdated
| context.Items["Command"] <- nameof Enqueue | ||
| let command (_: EnqueueParameters) = PromotionQueueCommand.Enqueue candidateId |> returnValueTask | ||
| return! processCommand context (fun _ -> [||]) command |
There was a problem hiding this comment.
Avoid re-parsing the enqueue request body
In Enqueue, the body is parsed once (parse<EnqueueParameters>), then runEnqueue calls processCommand, which parses the body again to obtain TargetBranchId. Because the stream is not rewound between these reads, the second BindJsonAsync typically sees an empty body and Guid.Parse(parameters.TargetBranchId) inside processCommand throws, yielding a 500 for valid enqueue requests. Consider passing the already-parsed parameters/branch id into the command path or rewinding context.Request.Body before calling processCommand.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added
processCommandWithParametersto execute queue commands using pre-parsed parameters while preserving existing validation, metadata, and error handling behavior. src/Grace.Server/Queue.Server.fsL154-L243 - Updated the enqueue flow to reuse parsed parameters when issuing the enqueue command, avoiding a second request body parse. src/Grace.Server/Queue.Server.fsL345-L377
Testing
- Not run (not requested).
There was a problem hiding this comment.
@codex I don't see that code in a commit on this branch.
There was a problem hiding this comment.
Summary
- Added
processCommandWithParametersto run queue commands using pre-parsed parameters while preserving validation and response metadata behavior. src/Grace.Server/Queue.Server.fsL154-L270 - Updated enqueue handling to reuse the parsed request body instead of re-parsing it for the enqueue command. src/Grace.Server/Queue.Server.fsL345-L379
Testing
⚠️ dotnet build --configuration Release(dotnet not installed)⚠️ dotnet test --no-build(dotnet not installed)
src/Grace.Server/Eventing.Server.fs
Outdated
| OwnerId.Empty | ||
| OrganizationId.Empty | ||
| RepositoryId.Empty |
There was a problem hiding this comment.
Scope event envelopes to the repository
The WorkItem event envelope is built with RepositoryId.Empty (and the same pattern appears for review/candidate events below). In Notification.Server.fs, an empty RepositoryId causes NotifyEvent to broadcast to all connected clients instead of a repository group, so updates from one repo will leak to every client. Populate repository identifiers from the event payload or actor state before publishing to keep notifications scoped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Added repository-aware envelope construction by pulling repository identifiers from event payloads or metadata for work item, review, candidate, queue, gate attestation, and conflict receipt notifications. src/Grace.Server/Eventing.Server.fsL15-L193
- Included repository identifiers in server-generated event metadata when available from request context IDs. src/Grace.Server/Services.Server.fsL49-L62
Testing
⚠️ dotnet build --configuration Release(dotnet not installed)⚠️ dotnet test --no-build(dotnet not installed)
There was a problem hiding this comment.
Summary
- Ensured WorkItem, Review, and IntegrationCandidate events stamp repository IDs into event metadata before publishing to keep notifications scoped. src/Grace.Actors/WorkItem.Actor.fsL54-L75 src/Grace.Actors/Review.Actor.fsL90-L140 src/Grace.Actors/IntegrationCandidate.Actor.fsL52-L73
- Updated enqueue handling to reuse parsed parameters and send enqueue responses without re-reading the request body. src/Grace.Server/Queue.Server.fsL362-L427
Testing
- Not run (not requested).
Resolves #46.