feat: migrate flashduty-cli onto go-flashduty (covered+non-enriched, dual-client)#24
Merged
Conversation
…lient) Begin moving flashduty-cli off the hand-written flashduty-sdk onto the generated go-flashduty client. This is a behavior-preserving, dual-client transition: go.mod requires both SDKs, migrated handlers call the new `*go-flashduty.Client` (wired via `RunContext.GFClient` / `newGFClientFn`), and every deferred handler keeps an in-code `TODO(go-flashduty migration)` naming exactly why it stays on the legacy client. Migrated (28 commands): incident batch ops (ack/unack/close→Resolve/wake/ merge/snooze/reopen/reassign→Assign/add-responder/comment/disable-merge/ remove/create + war-room list/get/delete), alert merge/events, alert-event list, insight team/channel/responder/top-alerts, audit search, statuspage create-timeline, monit-query diagnose, monit-agent catalog/invoke. Deferred on legacy SDK (each TODO-annotated): - endpoint gap (pending upstream): war-room create/add-member/default- observers, change list/trend, statuspage list, insight notifications, mcp create - shape/enrichment divergence: incident list/get/timeline/feed/similar/ postmortem, alert list/get/timeline, oncall who/schedule, statuspage changes/create-incident, incident update (/reset drops --field), monit-query rows (raw→structured) Other changes: - TOON output moved from `sdk.Marshal(.., TOON)` to `toon-go` directly (toon promoted to a direct dep); SDK stays a pure client. - New test seam: go-flashduty's client is a concrete type, not an interface, so migrated-command tests run against `gfStub`, an httptest server that records the path + decoded body and replies with a canned envelope (internal/cli/gfstub_test.go). - Preserve exact legacy wire on assignment: both `incident create` and `incident reassign` set assigned_to.type = "assign" explicitly (the hand-written SDK forced it; leaving it empty would let the backend relabel an already-assigned incident as "reassign"). Guarded by two new wire-assertion tests. Verified: go build + go test (all 6 packages) + go vet + gofmt clean; in-process e2e against api-dev passes for migrated commands with coverage (remaining e2e failures reproduce identically on a clean origin/main baseline — stale --title flag tests, 31-day-window/429/401 env issues, and the not-migrated `change list`).
Bump go-flashduty 0.3.0 -> 0.4.0, which now documents the endpoints these previously-deferred commands needed, and move them off the hand-written flashduty-sdk onto the typed ctx.GFClient. Migrated (legacy SDK call -> go-flashduty method): - change list ListChanges -> Changes.List - statuspage list ListStatusPages -> StatusPages.ReadPageList (page-id filter applied client-side) - incident war-room create CreateIncidentWarRoom -> Incidents.WarRoomCreate - incident war-room create (auto) ListWarRoomEnabledDataSources -> ImIntegrations.List - incident war-room add-member AddIncidentWarRoomMembers -> Incidents.WriteAddWarRoomMember - incident war-room default-observers GetIncidentWarRoomDefaultObservers -> Incidents.ReadGetWarRoomDefaultObservers - mcp create CreateMCPServer -> McpServers.WriteServerCreate Output (columns, order, footers, TOON) preserved exactly. change list now clamps non-positive --limit/--page to the legacy defaults (20/1) before sending, since go-flashduty forwards values verbatim and the server rejects < 1; the footer still shows the raw --page value, matching legacy behavior. statuspage list STATUS column stays empty (the /status-page/list endpoint and the legacy SDK never populated overall_status). Kept on legacy (no clean go-flashduty equivalent): - statuspage changes -> hits /status-page/change/active/list; go-flashduty only has the general /status-page/change/list (different semantics, requires status). - insight notifications -> go-flashduty Analytics has no notification-trend endpoint. Both annotated with TODO(go-flashduty migration). The shape/enrichment-deferred commands (incident list/get/timeline/feed/similar/postmortem/update, alert list/get/timeline, oncall who/schedule, statuspage create-incident, insight responder, monit-query rows) remain on legacy as before. Dropped the now-unused war-room/change/statuspage-list/mcp methods from the flashdutyClient interface (incl. 3 war-room methods left dead by the prior migration) and the orphaned mockClient/mockIncidentWarRoom stubs. Migrated the affected unit tests onto the gfStub httptest seam (added a path-aware payload hook for war-room create's two-call auto-discover flow) and added gfStub-backed tests for change list, statuspage list, and mcp create. flashduty-sdk dependency retained for the commands still on legacy.
The notifications command's backing report API (/report/* QueryNotificationTrend) is being retired, so it is intentionally excluded from go-flashduty's spec — there is no SDK method and there won't be one. Reword the TODO from a pending-migration note to an explicit do-not-migrate so it isn't re-pointed at the SDK later.
The notification-trend report API backing this command is being taken offline, so the command is removed outright rather than kept on the legacy SDK. Drops the cobra subcommand, its flashdutyClient.QueryNotificationTrend interface method, and the mockClient stub. No other caller referenced it.
Move the last two read commands that were pinned to the hand-written SDK onto go-flashduty: - insight responder: now calls Analytics.ByResponder. Drops the EMAIL column — the backend /insight/responder (RspdIncMetrics) never returns an email, so the legacy SDK's ResponderInsightItem.Email was always blank (a decode-only field). - incident feed: now calls Incidents.Feed. go-flashduty returns raw feed items, so resolveFeedOperators() replicates the legacy operator-name enrichment by resolving each entry's creator_id via Members.PersonInfos (best-effort; falls back to the numeric ID, or 'system' for creator_id 0). Removes the now-dead GetIncidentFeed + QueryInsightByResponder from the flashdutyClient interface and mockClient, and rewrites the feed-empty test to use the gfStub httptest seam. Build/vet/gofmt/test green; both commands live-verified against api-dev (responder lists real data sans EMAIL; feed resolves operator names).
… go-flashduty Drops the hand-written flashduty-sdk dependency entirely. Every command now builds a concrete *go-flashduty.Client; the shared flashdutyClient interface, mockClient, and the legacy newClient factory are removed. - Migrate the remaining ~25 legacy commands (alert, channel, escalation-rule, field, incident, insight, member, monit-query, oncall, postmortem, status-page, status-page-migrate, team) onto typed go-flashduty services. - whoami/login: resolve identity via Members.MemberInfo + Account.Info. - template: vendor the client-side authoring metadata (channels, size limits, variable/function catalogs) into internal/cli/templatemeta.go -- it is reference data, not an API surface, so the generated SDK does not carry it. - Delete the change-trend command (backed by the retired /report/* API). - Rewrite all legacy-mock tests onto the httptest gfStub harness. - go.mod: drop flashduty-sdk; bump go-flashduty 0.4.0 -> 0.4.1 (adds StatusPages.ChangeActiveList). Verified: build, vet, test, gofmt, go mod tidy clean; whoami, member list, incident list, template get-preset/variables live-verified against api-dev.
go-flashduty v0.5.0 turns tri-state request fields into Go pointers so the zero value (false/0) reaches the wire instead of being dropped by omitempty. Adopt it in the CLI consumers the compiler flagged: - alert list: --active -> IsActive=Bool(true), --recovered -> IsActive=Bool(false), --muted -> EverMuted=Bool(true). --recovered (is_active=false) now actually filters instead of being silently dropped. Also drop the dead --title no-op flag (no title filter on the API; 'follow the API'). - incident similar: Limit -> Int64(limit) (flag default 5, always sent). - statuspage migrate structure: forward --url-name as URLName=String(...) when provided; nil reuses the source page's name. Was previously rejected because the SDK lacked the field; v0.5.0 adds it. Wire-body regression tests added for alert is_active/ever_muted, the no-filter omit case, incident similar limit, and migrate url_name forwarding.
The dual-client migration is complete; there is one client (go-flashduty). Import it un-aliased as `flashduty`; rename newGFClient/runGFCommand/ RunContext.GFClient to newClient/runCommand/Client. Pure rename, no behavior change.
…duty # Conflicts: # go.mod
…0.5.2 - channel list: export channelRow fields with json tags so the json/toon printers (reflection-based, skip unexported fields) emit full rows instead of empty objects. - team list: call Teams.ReadList with the --name/--page/--limit/--orderby/ --asc/--person-id flags instead of Teams.ReadInfos with an empty body, which silently dropped every flag; use server-side Total for the footer. - go-flashduty v0.5.2: named string enums now implement fmt.Stringer, fixing --output-format toon for incident feed/timeline and alert timeline.
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
1 participant
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.
What
Begin migrating
flashduty-clioff the hand-written flashduty-sdk onto the generated go-flashduty client. Behavior-preserving, dual-client transition:go.modrequires both SDKs; migrated handlers call*go-flashduty.Client(wired viaRunContext.GFClient/newGFClientFn); every deferred handler keeps an in-codeTODO(go-flashduty migration)naming exactly why it stays on legacy.This is the CLI counterpart to flashduty-mcp-server#61 (same dual-client pattern, same go-flashduty v0.3.0).
Migrated (28 commands)
Resolve, wake, merge, snooze, reopen, reassign→Assign, add-responder→ResponderAdd, comment, disable-merge, remove, create + war-room list/get/deleteDeferred on legacy SDK (each TODO-annotated)
/incident/resetdrops--field), monit-query rows (raw-bytes→structured)Other changes
sdk.Marshal(.., TOON)totoon-godirectly (promoted to a direct dep); the SDK stays a pure client.gfStub— an httptest server that records path + decoded body and replies with a canned envelope (internal/cli/gfstub_test.go).incident createandincident reassignsetassigned_to.type = "assign"explicitly. The hand-written SDK forced it; leaving it empty would let the backend relabel an already-assigned incident asreassignin the feed/IM cards. Guarded by two new wire-assertion tests. (Whetherreassignshould say "reassign" is a separate product decision, intentionally not bundled here.)Verification
go build+go test(all 6 packages) +go vet+gofmt -lclean.origin/mainbaseline (stale--titleflag tests, 31-day-window / 429 / 401-override-key env issues, and the not-migratedchange list) — pre-existing, not introduced here.Follow-up (blocked on upstream)
Once the gap endpoints are documented upstream → re-vendored into go-flashduty → released, switch the deferred handlers off legacy and drop the
flashduty-sdkdep. Same convergence as the MCP PR.