[lokewate]

Note

For reviewers, lines to review (blank lines not counted): code +115 / −18, plus 77 / −20 comment lines · tests +408 / −5 · docs +7 / −3. Diff against #355.

Summary

An admin can override a workflow node only after it parks itself on an error. A node that is healthy and waiting on an external event or human action — such as a warranting step that happened offline, or an open trader task that needs to be taken out of the trader's hands — has no way in.

Built on top of #355 (feat/wait-outside-activity), this adds the workflow and taskflow primitives a host application needs to force-park a waiting step and close the task behind it:

• workflow.ActivationParker (ForceParkActivation): sends a ForceParkActivation Workflow Update that settles the waiting step's pendingCompletions entry with an AdminForcedPark error (ForcedParkErrorType), causing the TASK node waiting on it to park immediately in NodeStatusAwaitingAdmin with the new ParkCategoryAdminForced (ADMIN_FORCED) so it can be resolved (COMPLETE, RETRY, ABORT) like any parked node. Returns ErrActivationNotPending if the step is not waiting.
• workflow.WorkflowTerminator (TerminateWorkflow): stops a running workflow outright without running any further nodes or completion callbacks, returning ErrWorkflowNotFound if the workflow is not running.
• taskflow/store.StateCancelled and TaskStore.CancelTask: marks an open task CANCELLED (refusing COMPLETED tasks and making repeated calls on CANCELLED tasks a no-op) while advancing seq so any in-flight step write is dropped as stale.
• taskflow/orchestrator.TaskManager.CancelTask: marks the task row CANCELLED and terminates its task workflow; CompleteTaskStep and SaveDraft reject CANCELLED tasks with ErrStaleStep.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Other (please describe):

Changes Made

• workflow/completion.go: register the ForceParkActivation Workflow Update alongside CompleteActivation, sharing validatePendingCompletion and settling pendingCompletions[activationID] with an AdminForcedPark application error.
• workflow/park_category.go: add ParkCategoryAdminForced (ADMIN_FORCED), the category a force-parked node parks under.
• workflow/workflow.go: handleTaskNode also reads a force-park that arrived while the step's Activity was still running, so the node parks instead of ignoring it.
• workflow/manager.go: add ForcedParkErrorType, ActivationParker (ForceParkActivation), and WorkflowTerminator (TerminateWorkflow), implemented on temporalManagerImpl.
• workflow/completion_test.go, workflow/manager_test.go, workflow/force_park_integration_test.go: in-memory workflow tests, unit tests, and Temporal integration tests for ForceParkActivation and TerminateWorkflow.
• taskflow/store/db.go, taskflow/store/gorm/store.go: add StateCancelled and CancelTask(ctx, taskID) on TaskStore, InMemoryTaskStore, and GormTaskStore; guard WriteDraft against StateCancelled.
• taskflow/orchestrator/manager.go, taskflow/orchestrator/draft.go: add ErrTaskNotOpen and TaskManager.CancelTask(ctx, taskID, reason); reject StateCancelled in CompleteTaskStep and SaveDraft.
• taskflow/orchestrator/cancel_task_test.go, taskflow/orchestrator/manager_test.go, taskflow/orchestrator/draft_test.go, taskflow/store/db_test.go, taskflow/store/gorm/store_test.go: unit tests covering task cancellation, idempotency, draft/step rejection after cancellation, and seq fencing.
• taskflow/README.md, taskflow/docs/architecture.md: document CANCELLED task state and transition.

Testing

• I have tested this change locally
• I have added tests that prove my fix is effective or that my feature works
• I have tested edge cases
• All existing tests pass

Test details:

• go test -race ./workflow/... ./taskflow/...

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have checked that there are no merge conflicts

Related Issues

Depends on #355.
Related to LSFLK/lsf-govtech-tnsw#243

Additional Notes

Breaking change: store.TaskStore adds CancelTask(ctx context.Context, taskID string) (int64, error). External TaskStore implementations must implement this method.

Summary by CodeRabbit

• New Features
  • Tasks can now be cancelled while in progress. Cancellation stops the workflow and prevents pending or late step updates from changing the cancelled task.
  • Administrators can park a waiting workflow step for manual resolution, with a reason recorded. The step can be retried after resolution.
• Bug Fixes
  • Late workflow completions no longer override a cancelled task or trigger its completion handling.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 20b8346a-1bd4-4245-b097-ae990bc32e3b

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: b194bb0a-15ea-4858-883a-4f5c6f9bb982

Commits

Reviewing files that changed from the base of the PR and between 62fc09f and 4c6300f.

Files selected for processing: 18

• taskflow/README.md
• taskflow/docs/architecture.md
• taskflow/orchestrator/cancel_task_test.go
• taskflow/orchestrator/draft.go
• taskflow/orchestrator/draft_test.go
• taskflow/orchestrator/manager.go
• taskflow/orchestrator/manager_test.go
• taskflow/store/db.go
• taskflow/store/db_test.go
• taskflow/store/gorm/store.go
• taskflow/store/gorm/store_test.go
• workflow/completion.go
• workflow/completion_test.go
• workflow/force_park_integration_test.go
• workflow/manager.go
• workflow/manager_test.go
• workflow/park_category.go
• workflow/workflow.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

This change adds task cancellation that terminates its workflow and advances the task sequence. It also adds workflow activation force-parking for admin resolution. Store guards and workflow handling reject or ignore stale operations after these state changes.

Changes: Task cancellation and forced parking

Layer / File(s)	Summary
Cancelled task state and storage guards taskflow/store/db.go, taskflow/store/gorm/store.go, taskflow/store/gorm/store_test.go, taskflow/store/db_test.go, taskflow/README.md, taskflow/docs/architecture.md	The store adds the CANCELLED state and CancelTask, which advances seq and rejects completed tasks. Store guards reject writes and claims for cancelled tasks. Documentation describes the cancellation state and guarded operations.
Workflow termination and forced parking workflow/manager.go, workflow/completion.go, workflow/workflow.go, workflow/park_category.go, workflow/*_test.go	The workflow manager adds workflow termination and forced parking for pending activations. Accepted force-parks resolve the activation with an admin-forced error. Task nodes consume ready completion signals after successful activities.
Task cancellation orchestration and stale operations taskflow/orchestrator/manager.go, taskflow/orchestrator/draft.go, taskflow/orchestrator/*_test.go, taskflow/docs/architecture.md	TaskManager.CancelTask terminates the workflow before storing cancellation. Drafts and step completions for cancelled tasks are rejected as stale, and late workflow completions do not invoke the completion hook.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ActivationParker
  participant WorkflowManager
  participant PendingActivation
  participant TaskNode
  ActivationParker->>WorkflowManager: Request force-park with activation ID and reason
  WorkflowManager->>PendingActivation: Submit workflow update
  PendingActivation->>TaskNode: Resolve completion future with forced-park error
  TaskNode->>TaskNode: Park for admin resolution

Loading

Suggested reviewers: webcooper

Merge Risk: Minimal · previous review 4c630

No merge-blocking issue is established; merge after normal validation.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 47.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies both primary changes: force-parking a waiting workflow step and cancelling a task externally. It is specific and concise enough for pull request history.
Description check		The description is complete and aligned with the template. It explains the purpose, identifies the breaking change, lists implementation changes, documents testing, records checklist items, and provid…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is complete and aligned with the template. It explains the purpose, identifies the breaking change, lists implementation changes, documents testing, records checklist items, and provides issue context. The optional Screenshots/Demo section is omitted.

Full details: Docstring Coverage

Explanation

Docstring coverage is 47.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 16 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}