Skip to content

Add webhook CLI commands - #96

Merged
robzolkos merged 5 commits into
masterfrom
add-webhook-cli
Mar 10, 2026
Merged

Add webhook CLI commands#96
robzolkos merged 5 commits into
masterfrom
add-webhook-cli

Conversation

@robzolkos

@robzolkos robzolkos commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add fizzy webhook commands: list, show, create, update, delete, and reactivate
  • All commands are scoped to a board via --board flag (with fallback to configured default)
  • Create supports --actions flag for subscribing to specific events (card_published, card_closed, comment_created, etc.)
  • Update agent skill documentation with webhook command reference, schema, and supported actions
  • 14 unit tests covering all commands including error handling and validation
  • Fix next-page breadcrumb bug in all paginated list commands (board, card, notification, tag, user, webhook) — the guard was dead code causing the "next" breadcrumb to point to page 1 when already on page 1
  • Add breadcrumb assertion test to prevent regression

Add full CRUD support for managing webhooks on boards: list, show,
create, update, delete, and reactivate. All commands require --board
flag (with fallback to configured default board) and account admin
access on the API side.

Update agent skill documentation with webhook command reference,
schema, and supported actions.
Copilot AI review requested due to automatic review settings March 10, 2026 02:02
@robzolkos
robzolkos requested a review from a team as a code owner March 10, 2026 02:02
@github-actions github-actions Bot added the enhancement New feature or request label Mar 10, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/webhook.go">

<violation number="1" location="internal/commands/webhook.go:68">
P2: When `--page` is not specified (default `0`), the "next page" breadcrumb points to `--page 1` instead of `--page 2`. The guard `if nextPage == 0` is unreachable because `nextPage` is computed as `webhookListPage + 1`, which is `1` when the default is `0`. The condition should check `webhookListPage` instead.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread internal/commands/webhook.go Outdated
When --page defaults to 0, the next-page breadcrumb should point to
page 2 (since page 1 is what the user is already viewing). The guard
condition was checking nextPage == 0 which is unreachable; check
webhookListPage == 0 instead.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds full CLI support for managing Fizzy board webhooks and updates the agent skill documentation/surface to reflect the new command group.

Changes:

  • Introduces fizzy webhook commands: list, show, create, update, delete, and reactivate (board-scoped via --board with default-board fallback).
  • Adds table column rendering for webhook list output.
  • Updates skill docs (both external and internal copies) and SURFACE.txt to document and expose the new webhook command surface.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
skills/fizzy/SKILL.md Documents webhook commands, supported actions, and webhook schema for the agent skill.
internal/skills/SKILL.md Mirrors the webhook documentation updates for internal skill distribution.
internal/commands/webhook.go Implements the webhook Cobra command group and subcommands, including pagination and mutations.
internal/commands/webhook_test.go Adds unit tests covering webhook command behaviors and error cases.
internal/commands/columns.go Adds webhookColumns for list/table rendering.
SURFACE.txt Registers the new webhook commands/flags in the surfaced CLI contract.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/webhook.go">

<violation number="1" location="internal/commands/webhook.go:68">
P1: Inconsistent pagination guard pattern across list commands</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread internal/commands/webhook.go
Keep the nextPage == 0 guard consistent with board list, card list,
and other paginated commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/commands/webhook.go">

<violation number="1" location="internal/commands/webhook.go:68">
P1: This condition is now unreachable for the intended case and introduces a regression. When `--page` is not set, `webhookListPage` is 0, so `nextPage = 0 + 1 = 1`. The check `nextPage == 0` is false, so the "next page" breadcrumb will point to page 1 (the page already being displayed) instead of page 2. The original check `webhookListPage == 0` was correct.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread internal/commands/webhook.go Outdated
The guard `if nextPage == 0` was unreachable dead code since nextPage
is always at least 1 (page variable defaults to 0, plus 1). This
caused the "next page" breadcrumb to point to page 1 when the user
was already viewing page 1 (the default).

Fix by checking the page variable itself instead, and add a test
that asserts the breadcrumb points to page 2.
Copilot AI review requested due to automatic review settings March 10, 2026 02:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/commands/webhook.go Outdated
Comment thread internal/commands/webhook.go
Generate the Long description and flag help text from
validWebhookActions instead of duplicating the list. Also add
supported actions to the update command's --actions flag help.
@robzolkos
robzolkos merged commit 48fb6f1 into master Mar 10, 2026
19 checks passed
@robzolkos
robzolkos deleted the add-webhook-cli branch March 10, 2026 08:08
jeremy added a commit that referenced this pull request Mar 10, 2026
The webhook commands (merged to master in #96) call
printSuccessWithLocation which was added to root.go on master.
Our branch has a diverged root.go, so CI's merge produces an
undefined reference. Add the function here so the merge builds.
jeremy added a commit that referenced this pull request Mar 10, 2026
The webhook commands (merged to master in #96) call
printSuccessWithLocation which was added to root.go on master.
Our branch has a diverged root.go, so CI's merge produces an
undefined reference. Add the function here so the merge builds.
jeremy added a commit that referenced this pull request Mar 10, 2026
The breadcrumb test merged from master (#96) used SetTestMode
(legacy client), but board list uses getSDK() on this branch.
Switch to SetTestModeWithSDK to match.
jeremy added a commit that referenced this pull request Mar 10, 2026
* Add SDK integration layer: client init, error mapping, and test infrastructure

Wire up fizzy-sdk as a dependency with SDK client initialization in root
PersistentPreRunE, typed error conversion (sdk_errors.go), normalizeAny()
for struct→map round-trips, and httptest-backed mock infrastructure that
lets existing MockClient tests exercise real SDK service methods.

* Migrate all commands to typed SDK service methods

Replace raw HTTP calls (getClient().Get/Post/Patch/Delete) with typed
SDK service methods (Cards().Create, Boards().Update, etc.) across all
command files. Legacy getClient() retained only for file upload/download,
multipart PATCH (avatar), and board migration.

* Update AGENTS.md for SDK architecture and remove dead --tag-ids flag

* Address PR review findings: 7 fixes across commands

- step update: bypass omitempty on bool via raw Patch when --not_completed set
- requireAuth: decouple from SDK init so legacy commands aren't blocked
- card create: follow Location header when API returns empty body
- setup: nil-guard accMap["id"] in parseAccounts
- pin_test: use validation error (422) instead of duplicate not-found
- setup_test: use toJSON helper instead of manual json.Marshal
- test_helpers: replace fragile "nauthorized" match with ToLower

* Fix e2e harness isolation and attachment test panics

Use FIZZY_PROFILE env var + clean HOME temp dir instead of --account
flag to avoid profile store resolution. Guard type assertions in
attachment download tests to prevent nil panics.

* Fix auth e2e tests: pass FIZZY_PROFILE and FIZZY_NO_KEYRING to Execute

Auth login/logout tests use harness.Execute() directly (not h.Run()),
so they must explicitly provide the env vars that globalEnv() normally
injects. Without FIZZY_PROFILE the CLI errors with "No profile configured".

* Address PR review feedback

- Fix CodeQL allocation overflow alert in column.go (len+3 flagged as
  potentially large)
- Remove double normalizeAny in step update --not_completed path
- Add configHome temp dir to NewWithConfig for test isolation
- Set Retryable: true on transient sentinel errors (circuit breaker,
  bulkhead, rate limit)

* Address PR review feedback: omitempty bools, network errors, upload SDK

- board update --all_access false: use raw Patch when setting false to
  avoid omitempty dropping the zero value (same pattern as step
  --not_completed)
- comment update: only set Body on request when --body or --body_file
  provided, preventing empty string from clearing comment text
- sdk_errors: catch raw net.Error as CodeNetwork for proper exit codes
- upload: use requireAuth+requireAccount instead of requireAuthAndAccount
  since upload only uses legacy client, not SDK
- AGENTS.md: update stale jsonAny docs to reflect normalizeAny

* Upgrade fizzy-sdk to v0.1.0

No breaking changes for the CLI — just moving from pre-release
commit pin to the tagged release.

* Add printSuccessWithLocation for webhook merge compatibility

The webhook commands (merged to master in #96) call
printSuccessWithLocation which was added to root.go on master.
Our branch has a diverged root.go, so CI's merge produces an
undefined reference. Add the function here so the merge builds.

* Fix board breadcrumb test to use SDK test mode

The breadcrumb test merged from master (#96) used SetTestMode
(legacy client), but board list uses getSDK() on this branch.
Switch to SetTestModeWithSDK to match.

* Fix CodeQL allocation overflow alert in column list

Guard the slice capacity computation to satisfy CodeQL's
size-overflow analysis.

* Address PR review feedback: marshal errors and URL validation

- Handle json.Marshal errors in signup identity data processing
  instead of silently discarding them
- Validate hostname presence for all URL schemes in validateAPIURL,
  not just http://
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants