Skip to content

feat(workflow): add pause, resume, cancel-execution commands#76

Merged
mayankpande88 merged 1 commit into
mainfrom
feat/workflow-lifecycle
May 25, 2026
Merged

feat(workflow): add pause, resume, cancel-execution commands#76
mayankpande88 merged 1 commit into
mainfrom
feat/workflow-lifecycle

Conversation

@blue4209211

Copy link
Copy Markdown
Contributor

First feature PR in the planned series after PR #75 (OSS scaffolding).

Adds three workflow lifecycle commands so users can manage running workflows from the CLI:

Command GraphQL action Args
nbctl workflow pause <id> workflow_pause account_id (from profile), id
nbctl workflow resume <id> workflow_resume account_id, id
nbctl workflow cancel-execution <workflow-id> <execution-id> workflow_cancel_execution account_id, id, execution_id

Schema verification

All three action names + input shapes verified against ~/Workspace/nudgebee/app/src/lib/actions.graphql and the app's actual call sites in app/src/api1/workflow/index.ts (lines 322–349). Used the same GraphQL mutation text the app uses so request shapes will not drift.

Test plan

  • Unit tests for each command, both text and JSON output (6 tests total). All pass.
  • make build, make lint — clean.
  • Live-verified against the dev cluster:
    • pause / resume reach the server and the action handler responds (workflows in DRAFT state return "failed to pause workflow" — expected).
    • cancel-execution with a real workflow id + bogus execution id returned the proper server error "workflow execution not found", confirming the request shape matches the server's WorkflowCancelRequest.

Out of scope (separate PRs)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces three new CLI commands—cancel-execution, pause, and resume—to manage workflow lifecycles, along with their corresponding unit tests. The reviewer suggested several improvements across all new files: adopting Go naming conventions for initialisms (e.g., accountID), validating the presence of the required account-id configuration early to provide better error messages, and utilizing the package-level client.Run function with cmd.Context() to ensure proper initialization and support for command cancellation.

Comment thread cmd/workflow_cancel_execution.go
Comment thread cmd/workflow_cancel_execution.go
Comment thread cmd/workflow_pause.go
Comment thread cmd/workflow_pause.go
Comment thread cmd/workflow_resume.go
Comment thread cmd/workflow_resume.go
@blue4209211 blue4209211 force-pushed the feat/workflow-lifecycle branch from 939de14 to cc6a049 Compare May 25, 2026 02:28
Three new workflow lifecycle commands, matching shapes used by the
web app (verified in /Users/shiv/Workspace/nudgebee/app):

- `nbctl workflow pause <id>` — backed by `workflow_pause` mutation
  (WorkflowGetRequest: account_id, id).
- `nbctl workflow resume <id>` — backed by `workflow_resume`.
- `nbctl workflow cancel-execution <workflow-id> <execution-id>` —
  backed by `workflow_cancel_execution` (WorkflowCancelRequest:
  account_id, id, execution_id).

Each command has text and JSON output modes and unit tests covering
both. Live-verified against the dev cluster: pause/resume reach the
server and return proper action responses; cancel-execution correctly
returned "workflow execution not found" for a bogus execution id,
confirming request shape matches the server schema.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@blue4209211 blue4209211 force-pushed the feat/workflow-lifecycle branch from cc6a049 to 11095be Compare May 25, 2026 02:46
@blue4209211

Copy link
Copy Markdown
Contributor Author

Thanks for the review @gemini-code-assist. Addressed one of the four suggestions; rationale for the other three below.

# Suggestion Status
1 Rename accountIdaccountID Skipped. Repo convention is accountId (used in 10+ existing command files vs. 5 using accountID). Flipping only these 3 new files would worsen the inconsistency. The right fix is a repo-wide cleanup PR.
2 Validate empty account-id early Applied — all three commands now error out with a clear hint pointing at nbctl configure add / --account-id before any network call.
3 Use client.Run singleton instead of client.NewClient() Skipped. 34 existing commands use client.NewClient(); only 1 uses client.Run. Migrating just my 3 files would create outliers. Should be a separate refactor.
4 Use cmd.Context() instead of context.Background() Skipped. Zero existing files use cmd.Context(). Same consistency argument; would be valuable as a repo-wide refactor for Ctrl+C cancellation.

Force-pushed as a single commit; build/test/lint clean.

@gemini-code-assist

Copy link
Copy Markdown

Thanks for the detailed breakdown, @blue4209211. I completely understand your reasoning regarding the consistency with the existing codebase. Prioritizing architectural uniformity over isolated changes is a sound strategy, especially when those refactors are better suited for a dedicated, repo-wide cleanup. I appreciate you applying the early validation for account-id as well. The implementation looks solid and the tests cover the requirements well. Everything looks good to go.

@mayankpande88 mayankpande88 merged commit 020ae4b into main May 25, 2026
1 check passed
@blue4209211 blue4209211 deleted the feat/workflow-lifecycle branch May 25, 2026 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants