feat(workflowbuilder): broken workflow refs and cross-module delete guards#122
Merged
Merged
Conversation
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds WorkflowBuilder read models + EF migration, a Sync service and repository, gRPC proto/service for counting blocking form refs, FormBuilder deletion guard via gRPC, Kafka handlers to mark broken refs, DI/API and test updates, CI buf script, schema registration, and docs updates. ChangesWorkflow Reference Tracking and Deletion Guards
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
docs/PROGRESS.md (1)
50-54: ⚡ Quick winVerify if WorkflowBuilder service-boundary retrofit status should be ✅
Line 50 marks WorkflowBuilder's "Service-boundary retrofit" as
⚠️ , but line 54 describes the completed broken-refs work (read models, Kafka handlers, gRPC service, publish blocking,GetWorkflowenrichment) as ✅. If all the Phase 2 WorkflowBuilder contract work described in the PR objectives is now complete, the retrofit status should be updated to ✅ rather than⚠️ .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PROGRESS.md` around lines 50 - 54, The PROGRESS.md entry incorrectly leaves "Service-boundary retrofit" for WorkflowBuilder marked as ⚠️ while the diff shows Phase 2 work completed (workflow_form_references + workflow_model_references read models, ModelDeletedHandler, FormDeletedHandler Kafka handlers, WorkflowFormReferenceService gRPC, publish blocking on broken refs, and GetWorkflow exposing isBroken). Update the WorkflowBuilder "Service-boundary retrofit" status from ⚠️ to ✅, ensure the single-line summary near "WorkflowBuilder" reflects completion, and run a quick grep for the symbols WorkflowBuilder, workflow_form_references, workflow_model_references, ModelDeletedHandler, FormDeletedHandler, WorkflowFormReferenceService, and GetWorkflow to confirm no other inconsistent status lines remain.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddStepHandlerTests.cs (1)
20-23: ⚡ Quick winAssert reference-sync behavior in the success path.
The new dependency is injected but never verified, so a regression where sync is skipped would still pass. Please add an assertion in the happy-path test that the reference-sync call is executed once before commit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddStepHandlerTests.cs` around lines 20 - 23, Add an assertion in the happy-path test to verify the injected IWorkflowReferenceSync (_referenceSync) is actually called once and before the unit-of-work commit: use NSubstitute to assert the appropriate sync method on _referenceSync was invoked exactly once and then assert call ordering (e.g., Received.InOrder) that the _referenceSync call occurs prior to _uow.Commit/CommitAsync; locate this in the AddStepHandlerTests happy-path test that exercises AddStepHandler so the regression of skipping sync will fail.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ConfigureStepHandlerTests.cs (1)
20-23: ⚡ Quick winValidate that step reconfiguration triggers reference synchronization.
The constructor wiring is updated, but no test currently checks the sync call. Add a success-path interaction assertion on
IWorkflowReferenceSyncto cover the new contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ConfigureStepHandlerTests.cs` around lines 20 - 23, Add a test (or update the existing success-path test in ConfigureStepHandlerTests) that asserts the ConfigureStepHandler triggers reference synchronization by verifying the IWorkflowReferenceSync mock is called; after invoking ConfigureStepHandler.Handle (or the test's existing act step) use NSubstitute's Received(1) against the _referenceSync instance to assert a call to its SyncReferenceAsync (or the appropriate sync method) with the expected workflow id/step arguments to cover the new contract.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddTriggerHandlerTests.cs (1)
19-22: ⚡ Quick winAdd a verification that trigger updates invoke reference sync.
Since
IWorkflowReferenceSyncis now part of the handler contract, add a success-path assertion for its invocation to protect against silent regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddTriggerHandlerTests.cs` around lines 19 - 22, Add a success-path assertion in AddTriggerHandlerTests to verify the handler calls the IWorkflowReferenceSync dependency: after invoking AddTriggerHandler.Handle (via _handler) on the test request, assert that the mock _referenceSync received the expected call (e.g., Received(1)) to its sync method so the test validates that AddTriggerHandler invokes IWorkflowReferenceSync on success.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs (1)
19-22: ⚡ Quick winAdd assertions for the newly injected
IWorkflowReferenceSyncdependency.Please verify expected sync calls on success and absence of calls on not-found paths so this handler’s new side-effect contract is explicitly protected by tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs` around lines 19 - 22, Add assertions in RemoveTriggerHandlerTests to verify calls to the injected IWorkflowReferenceSync (_referenceSync) for both success and not-found scenarios: for the successful removal test that uses RemoveTriggerHandler, assert that _referenceSync.Sync (or the actual sync method name on IWorkflowReferenceSync used by RemoveTriggerHandler) was called with the expected workflow/trigger identifiers; for the not-found test assert that _referenceSync received no calls (e.g. DidNotReceive) to ensure no side-effects. Update the existing success test to include the positive assertion and the not-found test to include the negative assertion referencing the _referenceSync field and the handler under test.tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/DeleteFormHandlerTests.cs (1)
15-21: ⚡ Quick winStrengthen failure-path interaction assertions for
IFormDeletionGuard.Since guard-based behavior is now central, add assertions that guard is not called when form lookup fails, and
SaveChangesAsyncis not called when guard returns a business-rule failure.Proposed assertion additions
[Fact] public async Task DeleteForm_WhenReferencedByActiveWorkflow_ReturnsBusinessRuleFailure() { @@ result.IsFailure.Should().BeTrue(); result.ErrorCode.Should().Be(ErrorCodes.BusinessRule); result.Error.Should().Contain("workflow"); + await _uow.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>()); } [Fact] public async Task DeleteForm_WhenFormNotFound_ReturnsNotFound() { @@ result.IsFailure.Should().BeTrue(); result.ErrorCode.Should().Be(ErrorCodes.NotFound); result.Error.Should().Contain("not found"); + await _formDeletionGuard.DidNotReceive() + .ValidateCanDeleteAsync(Arg.Any<Guid>(), Arg.Any<Guid>(), Arg.Any<CancellationToken>()); }Also applies to: 43-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/DeleteFormHandlerTests.cs` around lines 15 - 21, Update the tests in DeleteFormHandlerTests to assert guard and uow interactions on failure paths: in the test where the form lookup returns null, add an assertion that _formDeletionGuard.DidNotReceive() was called (i.e., ensure CreateHandler/handler does not invoke _formDeletionGuard); and in the test where the guard returns a business-rule failure, assert that _uow.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>()) is called (i.e., ensure SaveChangesAsync is not invoked when _formDeletionGuard signals failure). Locate these checks around the existing Arrange/Act/Assert blocks that use CreateHandler(), _formDeletionGuard, _formRepo and _uow and add the DidNotReceive assertions accordingly.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs (1)
20-23: ⚡ Quick winAssert
IWorkflowReferenceSyncinteraction in success and conflict paths.With
_referenceSyncnewly injected, add assertions that it runs on successful imports and is not called whenNameExistsAsyncshort-circuits. That locks the new dependency contract in this suite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs` around lines 20 - 23, Add assertions in ImportWorkflowHandlerTests to verify the new _referenceSync dependency is invoked on the successful import path and not invoked on the NameExistsAsync short-circuit (conflict) path: after exercising ImportWorkflowHandler (the _handler) in the success test assert _referenceSync.Received(...) was called once with the expected arguments using NSubstitute Received, and in the conflict test assert _referenceSync.DidNotReceive() was not called; locate these asserts next to the existing success and conflict test arrangements where NameExistsAsync is faked to succeed or to short-circuit so the interaction contract is locked into the suite.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs (1)
20-23: ⚡ Quick winCover the new reference-sync collaborator contract in this test class.
Now that
_referenceSyncis injected, add assertions for expected calls on successful removal and no calls on early-return failures. This prevents silent regressions where step deletion stops updating reference state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs` around lines 20 - 23, The tests need to assert interactions with the newly injected _referenceSync collaborator: update the success-case test(s) in RemoveStepHandlerTests (which exercise RemoveStepHandler) to assert that the substitute _referenceSync received the expected call (e.g., Received(1) with the removed step id or workflow id) after a successful removal, and update the failure/early-return tests (not-found/validation-failure) to assert _referenceSync.DidNotReceiveAnyCalls() (or DidNotReceive() for the specific reference method) to ensure no reference-sync happens on early exit; use the same unique symbols (_referenceSync and RemoveStepHandler) and verify parameters match the step/workflow used in the test.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/GetWorkflowHandlerTests.cs (1)
16-19: ⚡ Quick winAdd short-circuit assertions for the new reference-repository dependency.
_referenceRepois now part of handler wiring, but the not-found paths don’t assert it is skipped. Please assert no_referenceRepocalls when workflow lookup returnsnullso extra reference queries can’t regress in later refactors.Proposed test assertions
[Fact] public async Task Handle_WhenWorkflowNotFound_ReturnsNull() { _repo.GetByIdAsync(Arg.Any<Guid>(), OrgId, Arg.Any<CancellationToken>()).Returns((WorkflowDefinition?)null); WorkflowDetailDto? dto = await _handler.Handle( new GetWorkflowQuery(Guid.NewGuid(), OrgId), CancellationToken.None); dto.Should().BeNull(); + await _referenceRepo.DidNotReceive() + .GetBrokenStepIdsAsync(Arg.Any<Guid>(), Arg.Any<CancellationToken>()); + await _referenceRepo.DidNotReceive() + .HasBrokenEventTriggerAsync(Arg.Any<Guid>(), Arg.Any<CancellationToken>()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/GetWorkflowHandlerTests.cs` around lines 16 - 19, The not-found-path tests need to assert that the new _referenceRepo dependency is not queried when the workflow lookup returns null; update the GetWorkflowHandlerTests not-found test(s) to verify no calls were made to _referenceRepo (e.g. assert _referenceRepo.ReceivedCalls().Should().BeEmpty() or use NSubstitute’s DidNotReceive/DidNotReceiveWithAnyArgs on the relevant repository methods) after arranging _repo to return null and invoking GetWorkflowHandler.Handle; reference the field _referenceRepo and the class GetWorkflowHandler in the assertion so future refactors can’t introduce unexpected reference lookups.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormWorkflowDeletionGuard.cs`:
- Around line 25-33: The RPC call in
FormWorkflowDeletionGuard.ValidateCanDeleteAsync to
CountBlockingFormReferencesAsync has no per-call deadline; wrap the call in a
short bounded timeout by creating a linked CancellationTokenSource with a small
timeout and passing its token to CountBlockingFormReferencesAsync (use the
existing headers and the new token) so the delete path can’t hang indefinitely,
and update the RpcException handling in ValidateCanDeleteAsync to treat
StatusCode.DeadlineExceeded the same as StatusCode.Unavailable (i.e., consider
it transient/unavailable) when deciding retry/abort logic.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/PublishWorkflow/PublishWorkflowHandler.cs`:
- Around line 24-31: PublishWorkflowHandler currently calls
referenceSync.SyncAsync(workflow, cancellationToken) but then checks
WorkflowReferenceRepository.HasBrokenReferencesAsync(workflow.Id, ...) which
queries the DB and therefore may see stale IsBroken values; after SyncAsync you
must either persist the sync results before re-checking or make the check
operate on the in-memory tracked references. Fix by calling the unit-of-work
save (uow.SaveChangesAsync(cancellationToken)) immediately after
referenceSync.SyncAsync(...) and before calling HasBrokenReferencesAsync, or
alternatively update WorkflowReferenceRepository.HasBrokenReferencesAsync to
examine the tracked WorkflowFormReferences/WorkflowModelReferences on the
DbContext/ChangeTracker for the given workflow Id so it reflects the fresh
in-memory IsBroken state.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ValueObjects/WorkflowTriggerExtensions.cs`:
- Around line 10-13: The extension method TryGetEventModelId should guard
against a null extension receiver: in WorkflowTriggerExtensions.cs add an early
null check for the trigger (e.g., if (trigger == null) return null;) before
accessing trigger.Type or trigger.Config so the method safely returns null when
called on a null WorkflowTrigger.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/Configurations/WorkflowModelReferenceConfiguration.cs`:
- Line 12: The primary key defined in WorkflowModelReferenceConfiguration using
builder.HasKey(r => r.WorkflowId) is too narrow (only allows one reference per
workflow); change the entity key to a composite key that uniquely identifies
each model reference (e.g., include the model/reference identifier property such
as ModelId or ReferenceId along with WorkflowId) by updating the HasKey(...)
call in WorkflowModelReferenceConfiguration and ensure the corresponding entity
has those properties configured as required, then regenerate the EF Core
migration and snapshot so the database constraints match the new composite key.
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs`:
- Around line 47-50: When the referenced ID hasn't changed the code currently
calls row.MarkHealthy() unconditionally and can clear a previously broken state;
instead, only clear IsBroken after verifying the target actually exists. In the
block handling unchanged IDs (where you see row.FormId, formId.Value and
row.MarkHealthy()), replace the unconditional MarkHealthy() with a check that
the target resource for formId.Value exists (use your repository/lookup used
elsewhere in this class) and only call row.MarkHealthy() if that existence check
succeeds; otherwise leave the row as broken (do not clear IsBroken) so
publish/guard logic remains correct. Ensure the same change is applied to the
other occurrence around lines 74-77 where row.MarkHealthy() is called.
---
Nitpick comments:
In `@docs/PROGRESS.md`:
- Around line 50-54: The PROGRESS.md entry incorrectly leaves "Service-boundary
retrofit" for WorkflowBuilder marked as ⚠️ while the diff shows Phase 2 work
completed (workflow_form_references + workflow_model_references read models,
ModelDeletedHandler, FormDeletedHandler Kafka handlers,
WorkflowFormReferenceService gRPC, publish blocking on broken refs, and
GetWorkflow exposing isBroken). Update the WorkflowBuilder "Service-boundary
retrofit" status from ⚠️ to ✅, ensure the single-line summary near
"WorkflowBuilder" reflects completion, and run a quick grep for the symbols
WorkflowBuilder, workflow_form_references, workflow_model_references,
ModelDeletedHandler, FormDeletedHandler, WorkflowFormReferenceService, and
GetWorkflow to confirm no other inconsistent status lines remain.
In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/DeleteFormHandlerTests.cs`:
- Around line 15-21: Update the tests in DeleteFormHandlerTests to assert guard
and uow interactions on failure paths: in the test where the form lookup returns
null, add an assertion that _formDeletionGuard.DidNotReceive() was called (i.e.,
ensure CreateHandler/handler does not invoke _formDeletionGuard); and in the
test where the guard returns a business-rule failure, assert that
_uow.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>()) is called
(i.e., ensure SaveChangesAsync is not invoked when _formDeletionGuard signals
failure). Locate these checks around the existing Arrange/Act/Assert blocks that
use CreateHandler(), _formDeletionGuard, _formRepo and _uow and add the
DidNotReceive assertions accordingly.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddStepHandlerTests.cs`:
- Around line 20-23: Add an assertion in the happy-path test to verify the
injected IWorkflowReferenceSync (_referenceSync) is actually called once and
before the unit-of-work commit: use NSubstitute to assert the appropriate sync
method on _referenceSync was invoked exactly once and then assert call ordering
(e.g., Received.InOrder) that the _referenceSync call occurs prior to
_uow.Commit/CommitAsync; locate this in the AddStepHandlerTests happy-path test
that exercises AddStepHandler so the regression of skipping sync will fail.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddTriggerHandlerTests.cs`:
- Around line 19-22: Add a success-path assertion in AddTriggerHandlerTests to
verify the handler calls the IWorkflowReferenceSync dependency: after invoking
AddTriggerHandler.Handle (via _handler) on the test request, assert that the
mock _referenceSync received the expected call (e.g., Received(1)) to its sync
method so the test validates that AddTriggerHandler invokes
IWorkflowReferenceSync on success.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ConfigureStepHandlerTests.cs`:
- Around line 20-23: Add a test (or update the existing success-path test in
ConfigureStepHandlerTests) that asserts the ConfigureStepHandler triggers
reference synchronization by verifying the IWorkflowReferenceSync mock is
called; after invoking ConfigureStepHandler.Handle (or the test's existing act
step) use NSubstitute's Received(1) against the _referenceSync instance to
assert a call to its SyncReferenceAsync (or the appropriate sync method) with
the expected workflow id/step arguments to cover the new contract.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/GetWorkflowHandlerTests.cs`:
- Around line 16-19: The not-found-path tests need to assert that the new
_referenceRepo dependency is not queried when the workflow lookup returns null;
update the GetWorkflowHandlerTests not-found test(s) to verify no calls were
made to _referenceRepo (e.g. assert
_referenceRepo.ReceivedCalls().Should().BeEmpty() or use NSubstitute’s
DidNotReceive/DidNotReceiveWithAnyArgs on the relevant repository methods) after
arranging _repo to return null and invoking GetWorkflowHandler.Handle; reference
the field _referenceRepo and the class GetWorkflowHandler in the assertion so
future refactors can’t introduce unexpected reference lookups.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs`:
- Around line 20-23: Add assertions in ImportWorkflowHandlerTests to verify the
new _referenceSync dependency is invoked on the successful import path and not
invoked on the NameExistsAsync short-circuit (conflict) path: after exercising
ImportWorkflowHandler (the _handler) in the success test assert
_referenceSync.Received(...) was called once with the expected arguments using
NSubstitute Received, and in the conflict test assert
_referenceSync.DidNotReceive() was not called; locate these asserts next to the
existing success and conflict test arrangements where NameExistsAsync is faked
to succeed or to short-circuit so the interaction contract is locked into the
suite.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs`:
- Around line 20-23: The tests need to assert interactions with the newly
injected _referenceSync collaborator: update the success-case test(s) in
RemoveStepHandlerTests (which exercise RemoveStepHandler) to assert that the
substitute _referenceSync received the expected call (e.g., Received(1) with the
removed step id or workflow id) after a successful removal, and update the
failure/early-return tests (not-found/validation-failure) to assert
_referenceSync.DidNotReceiveAnyCalls() (or DidNotReceive() for the specific
reference method) to ensure no reference-sync happens on early exit; use the
same unique symbols (_referenceSync and RemoveStepHandler) and verify parameters
match the step/workflow used in the test.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs`:
- Around line 19-22: Add assertions in RemoveTriggerHandlerTests to verify calls
to the injected IWorkflowReferenceSync (_referenceSync) for both success and
not-found scenarios: for the successful removal test that uses
RemoveTriggerHandler, assert that _referenceSync.Sync (or the actual sync method
name on IWorkflowReferenceSync used by RemoveTriggerHandler) was called with the
expected workflow/trigger identifiers; for the not-found test assert that
_referenceSync received no calls (e.g. DidNotReceive) to ensure no side-effects.
Update the existing success test to include the positive assertion and the
not-found test to include the negative assertion referencing the _referenceSync
field and the handler under test.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6df3a678-6f16-4ada-8d76-7346eaaf9107
⛔ Files ignored due to path filters (1)
src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Generated/axis/formbuilder/events/FormDeletedEvent.csis excluded by!**/generated/**
📒 Files selected for processing (63)
.github/workflows/build-and-test.ymlbuf.yamldocs/PROGRESS.mddocs/epics/E03-data-modeling/README.mddocs/epics/E03-data-modeling/features/F01-model-definition.mddocs/epics/E04-workflow-builder/README.mddocs/epics/E05-form-builder/README.mddocs/epics/README.mdscripts/buf-breaking-against-base.shscripts/register-avro-schemas.shsrc/Axis.Api/Program.cssrc/Axis.Api/appsettings.jsonsrc/Modules/FormBuilder/Axis.FormBuilder.Application/Commands/DeleteForm/DeleteFormHandler.cssrc/Modules/FormBuilder/Axis.FormBuilder.Application/Services/IFormDeletionGuard.cssrc/Modules/FormBuilder/Axis.FormBuilder.Contracts/FormBuilderEventExtensions.cssrc/Modules/FormBuilder/Axis.FormBuilder.Contracts/FormBuilderKafkaTopics.cssrc/Modules/FormBuilder/Axis.FormBuilder.Contracts/Schemas/FormDeletedEvent.avscsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Extensions/FormBuilderInfrastructureExtensions.cssrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormWorkflowDeletionGuard.cssrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Messaging/FormBuilderEventMapper.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/AddStep/AddStepHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/AddTrigger/AddTriggerHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/ConfigureStep/ConfigureStepHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/ImportWorkflow/ImportWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/PublishWorkflow/PublishWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/RemoveStep/RemoveStepHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/RemoveTrigger/RemoveTriggerHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Queries/GetWorkflow/GetWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Queries/GetWorkflow/WorkflowStepDto.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Queries/GetWorkflow/WorkflowTriggerDto.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Repositories/IWorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Services/IWorkflowReferenceSync.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Axis.WorkflowBuilder.Contracts.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Contracts/Protos/axis/workflowbuilder/v1/workflow_form_reference_service.protosrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ReadModels/WorkflowFormReference.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ReadModels/WorkflowModelReference.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ValueObjects/WorkflowTriggerExtensions.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/AssemblyInfo.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Axis.WorkflowBuilder.Infrastructure.csprojsrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Extensions/WorkflowBuilderInfrastructureExtensions.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Grpc/WorkflowFormReferenceGrpcService.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Handlers/FormDeletedHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Handlers/ModelDeletedHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260525120000_AddWorkflowReferenceReadModels.Designer.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260525120000_AddWorkflowReferenceReadModels.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/WorkflowBuilderDbContextModelSnapshot.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/Configurations/WorkflowFormReferenceConfiguration.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/Configurations/WorkflowModelReferenceConfiguration.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/WorkflowBuilderDbContext.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Repositories/WorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cstests/Api/Axis.Api.Tests/Helpers/ApiTestFixture.cstests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/DeleteFormHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddTriggerHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/Commands/PublishWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ConfigureStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/GetWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure.Tests/Handlers/FormDeletedHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure.Tests/Handlers/ModelDeletedHandlerTests.cs
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormWorkflowDeletionGuard.cs`:
- Around line 44-49: In ValidateCanDeleteAsync, preserve caller cancellation by
checking cancellationToken.IsCancellationRequested when catching RpcException
with StatusCode.Cancelled (and treat DeadlineExceeded as before); if the token
is cancelled rethrow or call cancellationToken.ThrowIfCancellationRequested() so
an OperationCanceledException propagates to the caller, otherwise continue to
return Result.Failure for non-caller cancellations/timeouts (keep the existing
message for StatusCode.DeadlineExceeded and non-caller Cancelled cases).
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs`:
- Around line 58-62: The current LINQ that builds currentModelIds from
workflow.Triggers via TryGetEventModelId() does not exclude Guid.Empty, allowing
invalid workflow_model_references to persist; update the chain in
WorkflowReferenceSync (the currentModelIds variable) to filter out empty GUIDs
(e.g., require id.HasValue && id.Value != Guid.Empty or use
id.GetValueOrDefault() != Guid.Empty) before selecting id.Value into the HashSet
so only valid non-empty GUIDs are included.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7771a938-5db1-43cc-9fa3-a940f75c8ce7
📒 Files selected for processing (13)
src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormWorkflowDeletionGuard.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/PublishWorkflow/PublishWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Queries/GetWorkflow/GetWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Repositories/IWorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ValueObjects/WorkflowTriggerExtensions.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260525120000_AddWorkflowReferenceReadModels.Designer.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260525120000_AddWorkflowReferenceReadModels.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/WorkflowBuilderDbContextModelSnapshot.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Persistence/Configurations/WorkflowModelReferenceConfiguration.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Repositories/WorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/Commands/PublishWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/GetWorkflowHandlerTests.cs
✅ Files skipped from review due to trivial changes (2)
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/WorkflowBuilderDbContextModelSnapshot.cs
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Migrations/20260525120000_AddWorkflowReferenceReadModels.Designer.cs
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs (1)
85-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExclude
Guid.Emptyfrom synchronized model references.Line [87]-Line [89] still permits
Guid.Emptymodel IDs, which can create invalidworkflow_model_referencesrows and skew broken-reference checks.🔧 Proposed fix
HashSet<Guid> currentModelIds = workflow.Triggers .Select(t => t.TryGetEventModelId()) - .Where(id => id.HasValue) + .Where(id => id.HasValue && id.Value != Guid.Empty) .Select(id => id!.Value) .ToHashSet();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs` around lines 85 - 89, The currentModelIds set is including Guid.Empty from workflow.Triggers via TryGetEventModelId, which allows invalid workflow_model_references; update the LINQ pipeline that builds currentModelIds (the expression using workflow.Triggers, TryGetEventModelId, and ToHashSet) to exclude Guid.Empty by filtering out nulls and Guid.Empty (e.g., require id.HasValue && id.Value != Guid.Empty or filter after the Select) so only valid non-empty GUIDs are added to currentModelIds before synchronization.
🧹 Nitpick comments (3)
tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs (1)
22-28: ⚡ Quick winAssert
IWorkflowReferenceSync.SyncAsyncbehavior explicitly in handler tests.The new dependency is stubbed, but tests still don’t verify it was actually called on success (and skipped on early-failure paths). Adding those assertions will make this change resilient to regressions.
Suggested test assertions
@@ public async Task Handle_WhenStepExists_RemovesStepAndSaves() @@ result.IsSuccess.Should().BeTrue(); wf.Steps.Should().NotContain(s => s.Id == step.Id); + await _referenceSync.Received(1) + .SyncAsync(wf, Arg.Any<CancellationToken>()); await _uow.Received(1).SaveChangesAsync(Arg.Any<CancellationToken>()); @@ public async Task Handle_WhenWorkflowNotFound_ReturnsNotFound() @@ result.IsSuccess.Should().BeFalse(); result.ErrorCode.Should().Be(ErrorCodes.NotFound); + await _referenceSync.DidNotReceive() + .SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs` around lines 22 - 28, Update the RemoveStepHandlerTests to explicitly assert calls to the IWorkflowReferenceSync mock: in successful-path tests verify _referenceSync.SyncAsync was called with the expected WorkflowDefinition and any CancellationToken (use Arg.Is or equivalent) after invoking RemoveStepHandler.Handle, and in early-failure/path tests verify _referenceSync.SyncAsync was NOT called; locate usages in the RemoveStepHandlerTests constructor/setup (where _referenceSync is stubbed) and add these call/verifications surrounding calls to RemoveStepHandler.Handle to ensure SyncAsync is invoked only on success.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs (1)
22-28: ⚡ Quick winStrengthen these tests by asserting the sync call contract.
IWorkflowReferenceSyncis now part of the import flow, but current assertions don’t validate that call. Add positive/negative interaction checks to lock in expected orchestration.Suggested test assertions
@@ public async Task Handle_WhenNameIsAvailable_CreatesNewDraftWorkflowAndSaves() @@ await _repo.Received(1).AddAsync( Arg.Is<WorkflowDefinition>(w => w.Name == exportDto.Name && w.Status == WorkflowStatus.Draft && w.OrganizationId == OrgId), Arg.Any<CancellationToken>()); + await _referenceSync.Received(1) + .SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>()); await _uow.Received(1).SaveChangesAsync(Arg.Any<CancellationToken>()); @@ public async Task Handle_WhenNameAlreadyExists_ReturnsConflict() @@ result.IsSuccess.Should().BeFalse(); result.ErrorCode.Should().Be(ErrorCodes.Conflict); + await _referenceSync.DidNotReceive() + .SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>()); await _uow.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs` around lines 22 - 28, The tests currently stub IWorkflowReferenceSync.SyncAsync but don’t assert it was called; update ImportWorkflowHandlerTests to verify the orchestration by adding interaction assertions on the _referenceSync mock: use _referenceSync.Received(1).SyncAsync(Arg.Is<WorkflowDefinition>(wd => /* optional checks on expected fields */), Arg.Any<CancellationToken>()) in the positive import test, and in negative/no-op cases assert _referenceSync.DidNotReceive().SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>()); ensure these assertions are placed after invoking the handler (e.g., after calling ImportWorkflowHandler.Handle/Execute) so the test locks in the expected calls.tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs (1)
21-27: ⚡ Quick winAdd interaction assertions for
SyncAsyncnow that it’s a required collaborator.The constructor setup is good, but these tests should also assert
SyncAsyncis called on success and not called when workflow lookup fails.Suggested test assertions
@@ public async Task Handle_WhenTriggerExists_RemovesAndSaves() @@ result.IsSuccess.Should().BeTrue(); wf.Triggers.Should().BeEmpty(); + await _referenceSync.Received(1) + .SyncAsync(wf, Arg.Any<CancellationToken>()); await _uow.Received(1).SaveChangesAsync(Arg.Any<CancellationToken>()); @@ public async Task Handle_WhenWorkflowNotFound_ReturnsNotFound() @@ result.IsSuccess.Should().BeFalse(); result.ErrorCode.Should().Be(ErrorCodes.NotFound); + await _referenceSync.DidNotReceive() + .SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs` around lines 21 - 27, Add assertions in the tests to verify that the collaborator _referenceSync.SyncAsync is invoked on successful removal and not invoked when the workflow lookup fails: in the successful-path test use _referenceSync.Received(1).SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>()) (or ReceivedWithAnyArgs if preferred) after calling RemoveTriggerHandler.Handle/HandleAsync, and in the failure-path test use _referenceSync.DidNotReceive().SyncAsync(Arg.Any<WorkflowDefinition>(), Arg.Any<CancellationToken>()) to assert it was not called.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs`:
- Around line 85-89: The currentModelIds set is including Guid.Empty from
workflow.Triggers via TryGetEventModelId, which allows invalid
workflow_model_references; update the LINQ pipeline that builds currentModelIds
(the expression using workflow.Triggers, TryGetEventModelId, and ToHashSet) to
exclude Guid.Empty by filtering out nulls and Guid.Empty (e.g., require
id.HasValue && id.Value != Guid.Empty or filter after the Select) so only valid
non-empty GUIDs are added to currentModelIds before synchronization.
---
Nitpick comments:
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cs`:
- Around line 22-28: The tests currently stub IWorkflowReferenceSync.SyncAsync
but don’t assert it was called; update ImportWorkflowHandlerTests to verify the
orchestration by adding interaction assertions on the _referenceSync mock: use
_referenceSync.Received(1).SyncAsync(Arg.Is<WorkflowDefinition>(wd => /*
optional checks on expected fields */), Arg.Any<CancellationToken>()) in the
positive import test, and in negative/no-op cases assert
_referenceSync.DidNotReceive().SyncAsync(Arg.Any<WorkflowDefinition>(),
Arg.Any<CancellationToken>()); ensure these assertions are placed after invoking
the handler (e.g., after calling ImportWorkflowHandler.Handle/Execute) so the
test locks in the expected calls.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cs`:
- Around line 22-28: Update the RemoveStepHandlerTests to explicitly assert
calls to the IWorkflowReferenceSync mock: in successful-path tests verify
_referenceSync.SyncAsync was called with the expected WorkflowDefinition and any
CancellationToken (use Arg.Is or equivalent) after invoking
RemoveStepHandler.Handle, and in early-failure/path tests verify
_referenceSync.SyncAsync was NOT called; locate usages in the
RemoveStepHandlerTests constructor/setup (where _referenceSync is stubbed) and
add these call/verifications surrounding calls to RemoveStepHandler.Handle to
ensure SyncAsync is invoked only on success.
In
`@tests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cs`:
- Around line 21-27: Add assertions in the tests to verify that the collaborator
_referenceSync.SyncAsync is invoked on successful removal and not invoked when
the workflow lookup fails: in the successful-path test use
_referenceSync.Received(1).SyncAsync(Arg.Any<WorkflowDefinition>(),
Arg.Any<CancellationToken>()) (or ReceivedWithAnyArgs if preferred) after
calling RemoveTriggerHandler.Handle/HandleAsync, and in the failure-path test
use _referenceSync.DidNotReceive().SyncAsync(Arg.Any<WorkflowDefinition>(),
Arg.Any<CancellationToken>()) to assert it was not called.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 696fa366-4719-4c28-b105-6b722c773e98
📒 Files selected for processing (14)
src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Commands/PublishWorkflow/PublishWorkflowHandler.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Repositories/IWorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Services/IWorkflowReferenceSync.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Services/WorkflowReferenceSyncResult.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Repositories/WorkflowReferenceRepository.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/AddTriggerHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/Commands/PublishWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ConfigureStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/ImportWorkflowHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveStepHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application.Tests/RemoveTriggerHandlerTests.cstests/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure.Tests/Services/WorkflowReferenceSyncTests.cs
✅ Files skipped from review due to trivial changes (1)
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Application/Services/WorkflowReferenceSyncResult.cs
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/playbooks/agent-checklist.md`:
- Line 129: Gate 3's retrospective question count is out of sync: you added an
eighth question ("Review fix band-aid (mid-command save / EF query hack)?") but
the summary still says "seven questions"; update the Gate 3 count text to "eight
questions" wherever it references "seven questions" (search for the string
"seven questions" and the "Gate 3" heading) so the checklist and count match.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5e482c4-58f9-4207-8a5a-d05856451715
📒 Files selected for processing (5)
docs/playbooks/agent-checklist.mdsrc/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormWorkflowDeletionGuard.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/Entities/WorkflowStep.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/ValueObjects/WorkflowTriggerExtensions.cssrc/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Infrastructure/Services/WorkflowReferenceSync.cs
✅ Files skipped from review due to trivial changes (1)
- src/Modules/WorkflowBuilder/Axis.WorkflowBuilder.Domain/Entities/WorkflowStep.cs
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
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
2 participants
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
Workflow reference read models, sync on workflow mutations, publish gating via
WorkflowReferenceSyncResult, gRPC deletion guard, Kafka broken-ref handlers, and supporting CI/docs.Review fixes: improved — single UoW publish path; generic agent checklist for review feedback (best-practice self-check, Gate 3 count).
Linked spec
Requirements
Summary by CodeRabbit
New Features
Chores
Tests
Documentation