Skip to content

refactor: use @heroku/sdk for addons commands#3718

Merged
eablack merged 11 commits into
feat/heroku-sdk-integrationfrom
eb/refactor/sdk-addons-commands
May 22, 2026
Merged

refactor: use @heroku/sdk for addons commands#3718
eablack merged 11 commits into
feat/heroku-sdk-integrationfrom
eb/refactor/sdk-addons-commands

Conversation

@eablack
Copy link
Copy Markdown
Contributor

@eablack eablack commented May 20, 2026

Summary

Migrates addons:, maintenance:, and (as a consequence of rebasing onto feat/heroku-sdk-integration) pipelines: commands to use @heroku/sdk's new HerokuSDK + extensions architecture from sdk PR #25 / #26 / #27.

Migrated commands:

  • addons (list) — addOn.list/listByApp + addOnAttachment.list/listByApp. Accept-Expansion is scoped via withHeaders to the app-scoped list only (the global /addons endpoint rejects it).
  • addons:infoaddOn.describe. The SDK scopes the Accept-Expansion header to the resolve call only (the /addons/:id/addon-attachments endpoint uses Accept-Inclusion, not Expansion).
  • addons:createaddOn.createAndWait. Confirmation-required handling, polling-on-wait, and provisioning-failed errors now live in the SDK; the CLI hooks onProvisioning to preserve the two-phase status display ("Creating ... " → "Waiting for " → "Creating ... done"). The SDK also bakes in Accept-Expansion: addon_service,plan and X-Heroku-Legacy-Provider-Messages: true so the response includes the price for the action stop line and the provider's provision_message.
  • addons:upgradeaddOn.upgrade (onResolved callback drives the action line; bare plan name auto-qualifies via the SDK).
  • addons:servicesaddOnService.list.
  • addons:plansaddOn.listPlans. Native contract+cents sort replaces the lodash sort.
  • addons:renameaddOn.info + addOn.update (forwards the addon's current plan unchanged to satisfy the schema's plan required field).
  • addons:detachaddOnAttachment.infoByApp + addOnAttachment.delete + release.list (Range header via withHeaders).
  • maintenance (status / on / off) — app.info, app.enableMaintenance, app.disableMaintenance.
  • pipelines:info, pipelines:diff, pipelines:transfer, pipelines:promote — migrated from @heroku/sdk/compositions/pipeline (which no longer exists) to pipelineCouplingExtensions.listApps and the standalone promotePipeline function. Promote.promotePipeline is kept as a static reference so existing sinon stubs continue to work.

Helper migrations:

  • lib/addons/create-addon.ts — orchestration moved into the SDK's addOn.createAndWait. Helper is now a thin UX wrapper.
  • lib/addons/addons-wait.tswaitForAddonProvisioning migrated to the SDK polling path; waitForAddonDeprovisioning still uses legacy APIClient (used by deferred addons:destroy).
  • data:pg:create, data:pg:fork, data:pg:migrate, addons:wait — call-site signature follow-through (no more this.heroku first arg to createAddon).

Drops:

  • legacy lib/addons/resolve.ts dependency from addons:upgrade in favor of the SDK's typed AddonAmbiguousError.

Deferred to follow-up PRs (each has a single sticky API call not modeled in the SDK route registry):

  • addons:destroy — needs body: {force} on DELETE.
  • addons:attach — needs the /addons/:name/config/:namespace credential-config path variant.
  • addons:wait, addons:docs, addons:open — fully migratable, just out of scope for this PR.

Test fixture updates:

  • nock body matchers for POST /apps/:app/addons use plan: 'foo' instead of plan: {name: 'foo'} (matches AddOnCreateOpts canonical shape).
  • addons:rename PATCH body now includes plan (required by the schema; we forward the addon's current plan unchanged).
  • addons:detach mocks return JSON bodies for the DELETE response (the SDK's dispatcher parses, where the legacy client tolerated empty).
  • data:pg:migrate createAddonStub.args[0][N] indexes shifted down by 1 (no more heroku first arg).
  • addons:wait test replaces lolex with sinon's useFakeTimers({toFake: ['Date'], shouldAdvanceTime: true}) so the SDK's real setTimeout polling drives the test while Date.now() can still be fake-ticked for the >5s notifier threshold.
  • pipelines:promote test stubs Cmd.promotePipeline (a static reference to the standalone SDK function), with the callback signature shifted to 3-arg (_ctx, _body, options) and firstCall.args[1] for body assertions.
  • addons and addons:info tests split the nock scope so apiSdk (Accept-Expansion required) wraps only the resolve POSTs, while api (no reqheader constraint) wraps the global /addons and the /addons/<id>/addon-attachments calls — matching the SDK's per-call header scoping.

Pinned to @heroku/sdk branch eb/feat/addon-create-and-wait (PR heroku/heroku-sdk#27). Once that merges, the pin flips back to main.

Type of Change

  • refactor: Refactoring existing code without changing behavior

Testing

Run the touched-area unit tests:

npx mocha 'test/unit/commands/addons/*.unit.test.ts' \
          'test/unit/commands/maintenance/*.unit.test.ts' \
          'test/unit/commands/pipelines/*.unit.test.ts' \
          'test/unit/commands/data/pg/{create,fork,migrate}.unit.test.ts' \
          --timeout 120000

Expect 211 passing, 0 failing.

End-to-end smoke (against heroku-dev-tools team or any team you have access to):

npm run build
./bin/run addons --json                                # global list (no expansion)
heroku apps:create eb-addons-test --team heroku-dev-tools
./bin/run addons:create heroku-redis:mini -a eb-addons-test --wait
                                                       # exercises createAndWait + onProvisioning
                                                       # expect price line, provision message,
                                                       # 'Waiting for ...', 'Creating <name>... done'
./bin/run addons -a eb-addons-test                     # app-scoped list (with expansion)
./bin/run addons:info <returned-addon-name>            # exercises describe + grandfathered pricing
./bin/run addons:services
./bin/run addons:plans heroku-redis                    # exercises listPlans + native sort
./bin/run addons:upgrade <addon-name> premium-0        # bare plan auto-qualifies via SDK
./bin/run addons:rename <addon-name> renamed-redis     # exercises info + update + plan forward
heroku addons:attach renamed-redis --as REDIS_BACKUP -a eb-addons-test
./bin/run addons:detach REDIS_BACKUP -a eb-addons-test # exercises infoByApp + delete + release.list
./bin/run maintenance:on -a eb-addons-test
./bin/run maintenance -a eb-addons-test                # expect "on"
./bin/run maintenance:off -a eb-addons-test
heroku apps:destroy eb-addons-test --confirm eb-addons-test

For the confirmation-required path, repeat addons:create against an app whose owner team requires confirmation; expect the CLI to prompt for the team name, then succeed.

Related Issues

GUS work item: W-22265114

@eablack eablack requested a review from a team as a code owner May 20, 2026 23:34
eablack added 9 commits May 21, 2026 14:09
- addons (list) now calls SDK addOn.list / addOn.listByApp and
  addOnAttachment.list / addOnAttachment.listByApp. The
  Accept-Expansion: addon_service,plan header that drives nested
  service/plan inlining is now passed once via createPlatformClient
  options.
- addons:info now uses addOnAttachment.listByAddOn for the attachment
  fetch. resolveAddon keeps its existing /actions/addons/resolve flow
  and cache.
Replace direct Platform API calls in addons (list), addons:info,
addons:create, addons:services, addons:plans, and addons:upgrade
with @heroku/sdk equivalents:

- addOn.list / addOn.listByApp + addOnAttachment.list / listByApp
- describeAddon (resolves, fetches attachments, applies grandfathered
  pricing in one call)
- addOn.create
- addOnService.list
- plan.listByAddOn
- upgrade composition (resolves + updates in one call, with the
  onResolved callback firing between for the action display line)

Drops legacy lib/addons/resolve.ts dependency from upgrade in favor
of the SDK's typed AddonAmbiguousError. Tests updated for the SDK's
body/header/error shapes (UUIDs in PATCH paths, no `app: null` on
resolve bodies, etc.).

Adds tmp/ to the eslint ignore list (build artifacts produced 138k
unrelated lint errors).
The SDK was rearchitected to replace the compositions/ helpers with
a HerokuSDK + extension factory pattern. Update each command to
construct a HerokuSDK with the extensions it needs and call methods
through the resulting platform proxy.

Also bumps the SDK pin to the working branch so addOnExtensions.upgrade
exposes onResolved (previously dropped from the extension's options
type).
- rename: addOn.info + addOn.update (passes plan through unchanged
  to satisfy the schema's required field)
- detach: addOnAttachment.infoByApp + addOnAttachment.delete +
  release.list (Range header via withHeaders)
- plans: addOn.listPlans extension (replaces lodash sort with native
  contract+cents sort)

Test fixtures updated to return JSON bodies for the PATCH/DELETE
responses the SDK now parses.
- waitForAddonProvisioning: use createPlatformClient + addOn.infoByApp
  with Accept-Expansion header via withHeaders. Drops APIClient param.
- createAddon helper: drops APIClient param (no longer needed once
  waitForAddonProvisioning is migrated).
- create.ts, wait.ts, data:pg:{create,fork,migrate}: update call sites
  to match the new signature (no this.heroku passed in).
- addons:wait test: replace lolex install + setTimeout override with
  sinon.useFakeTimers({toFake: ['Date'], shouldAdvanceTime: true})
  so the SDK's real setTimeout polling drives the test while Date.now
  can still be fake-ticked for the >5s notifier threshold.
- pg create/fork test fixtures: nock body matchers changed from
  {plan: {name: 'foo'}} to {plan: 'foo'} to match the canonical
  AddOnCreateOpts shape the SDK sends.
- migrate test: shift createAddonStub argument indexes to reflect the
  new signature (heroku param removed).
Replaces the local trapConfirmationRequired + waitForAddonProvisioning
flow with a single platform.addOn.createAndWait call. The SDK now owns:

  - 423 confirmation_required → typed AddonConfirmationRequiredError,
    caught here and passed through ConfirmCommand for the UX prompt.
  - state=provisioning + wait=true → poll loop until terminal.
  - state=deprovisioned → typed AddonProvisioningFailedError.

The two-phase status display (Creating <plan>... <price>, then
Creating <addonName>... done while polling) is preserved by hooking
the SDK's onProvisioning callback to close the create-phase action,
print the provision message + 'Waiting for...' line, and start the
wait-phase action.

Bumps SDK pin to eb/feat/addon-create-and-wait.
Per @heroku/sdk#26 (chore!: cleans-up exports), HerokuSDK is now
exported from the package root and the './sdk' subpath is gone.

Bump the lockfile to pick up the SDK 0.4.0 build that includes both
this exports change and the createAndWait + onProvisioning callback
work that lib/addons/create-addon.ts depends on.
The global /addons endpoint rejects Accept-Expansion: addon_service,plan
('must be within ``'), but the per-app /apps/:id/addons endpoint
accepts it. Move the expansion off createPlatformClient's defaults and
onto a withHeaders-scoped client used only for the app-scoped list.

The attachment endpoints (list / listByApp) use Accept-Inclusion, not
Accept-Expansion, so they don't need the header either.

Bumps the SDK pin to pick up describeAddon's matching expansion-scoping
fix.
The integration branch's #3717 imports from @heroku/sdk/compositions/pipeline,
but the SDK's exports cleanup (heroku-sdk#26) replaced compositions/ with
resources/. Update each call site to use the new shape:

  - listPipelineApps → pipelineCouplingExtensions.listApps via HerokuSDK
  - promotePipeline → standalone import (kept as a static reference on
    the Promote command class so existing sinon stubs continue to work)
  - AppWithPipelineCoupling, ReleaseStreamContext → type imports from
    resources/platform/pipeline-{coupling,promotion}

The promote test's stub callback now sees a 3-arg signature (ctx, body,
options) instead of 2-arg, and firstCall.args[0] becomes [1] for body
assertions.

Test scope cleanup for addons/index and addons/info: split the 'apiSdk'
nock scope (Accept-Expansion required) from the 'api' scope (no
expansion). Global /addons and /addons/<id>/addon-attachments don't
accept the expansion header, matching the SDK's per-call header
scoping in heroku-sdk#27.
@eablack eablack force-pushed the eb/refactor/sdk-addons-commands branch from bb65ea4 to 937d495 Compare May 21, 2026 21:43
heroku-sdk PR #27 has merged; flip the pin from the feature branch
back to main. Resolves to commit efce1d4.

Also pulls in the eslint --fix import-sort cleanup across the seven
files we touched (each was importing '@heroku/sdk/extensions/platform'
or '@heroku/sdk/resources/platform/...' before '@heroku/sdk', which
the perfectionist/sort-imports rule flagged as an error).
Copy link
Copy Markdown
Contributor

@tlowrimore-heroku tlowrimore-heroku left a comment

Choose a reason for hiding this comment

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

Couple of things; no show-stoppers:

  • Curious to know why the different patterns for getting a reference to the SDK platform service. I see some cases where createPlatformClient is used and other cases where const {platform} = new HerokuSDK() is used.
  • Not sure if you were on the pointing session where we discussed doing away with using nock in favor of mocking the SDK. I realize now that I update the CLI apps update WI to mention this, but I missed the card for this work, so no big deal.

Otherwise, nice job!

Per review feedback on #3718: the PR mixed two patterns for getting a
reference to the platform service — some files used createPlatformClient,
others used new HerokuSDK({extensions: [...]}).platform. Pick the
HerokuSDK form everywhere so there's one shape in the codebase.

For files that don't need any extension methods (detach, index/list,
rename, services, maintenance/index, addons-wait), HerokuSDK is
constructed without an extensions array — the resulting platform proxy
exposes all the route-registry methods exactly the same as
createPlatformClient.
@eablack
Copy link
Copy Markdown
Contributor Author

eablack commented May 22, 2026

Thanks for the review!

Curious to know why the different patterns for getting a reference to the SDK platform service.

Good catch — the inconsistency was inherited from working through the migration in stages. Standardized on new HerokuSDK({extensions: [...]}).platform everywhere in 41f1bc7. For files that don't need extension methods, it's constructed with no extensions, which gives the same surface as createPlatformClient but keeps the construction shape uniform.

Not sure if you were on the pointing session where we discussed doing away with using nock in favor of mocking the SDK.

Yeah I was — for new tests we agreed to stub the SDK directly. The reason this PR still leans on nock so much is that I was adjusting existing nock-based tests rather than rewriting them, since updating in place was usually a smaller surgery than restructuring the whole fixture. Going forward I'll do net-new tests in the SDK-stub style.

@eablack eablack merged commit 5407005 into feat/heroku-sdk-integration May 22, 2026
4 of 17 checks passed
@eablack eablack deleted the eb/refactor/sdk-addons-commands branch May 22, 2026 17:03
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.

2 participants