Skip to content

refactor: use @heroku/sdk for apps:info command#3749

Merged
tlowrimore-heroku merged 11 commits into
feat/heroku-sdk-integrationfrom
feat/apps-info-sdk-update
Jun 5, 2026
Merged

refactor: use @heroku/sdk for apps:info command#3749
tlowrimore-heroku merged 11 commits into
feat/heroku-sdk-integrationfrom
feat/apps-info-sdk-update

Conversation

@tlowrimore-heroku
Copy link
Copy Markdown
Contributor

@tlowrimore-heroku tlowrimore-heroku commented Jun 4, 2026

Summary

  • Migrates src/commands/apps/info.ts to call platform.app.describe() from @heroku/sdk in place of the inline Promise.all that fanned out raw API calls for add-ons, the app, dynos, collaborators, and the pipeline coupling. The SDK composite encapsulates the same fan-out plus the soft-fail behavior on the optional resources, so ~25 lines of orchestration collapse to a single call.
  • Keeps a direct @heroku/heroku-fetch HerokuApiClient call for the --extended path. The ?extended=true query variant is a CLI-only concern not exposed through the SDK, and the user explicitly endorsed this hybrid approach during planning. @heroku/heroku-fetch is added as a direct dependency in this PR (it was already on disk as a transitive dep of @heroku/sdk).
  • Rewrites test/unit/commands/apps/info.unit.test.ts to drop nock entirely. SDK calls are stubbed via HerokuSDK.prototype.platform; the --extended escape-hatch call is stubbed via HerokuApiClient.prototype.get. This keeps tests asserting on the SDK contract the migrated command depends on, rather than on URL shape.

The pipelineCoupling field returned by the SDK is mapped back to snake-case (pipeline_coupling) before output to preserve the existing JSON-output contract that the tests already assert on.

Test plan

  • npx tsc --noEmit -p tsconfig.json — no new errors vs. baseline.
  • npx eslint src/commands/apps/info.ts test/unit/commands/apps/info.unit.test.ts — clean (the two pre-existing warnings on print()'s 5-param signature are unchanged).
  • npx mocha 'test/unit/commands/apps/info.unit.test.ts' — 12 passing.
  • npx mocha 'test/unit/commands/apps/**/*.unit.test.ts' — 89 passing, no sibling regressions.
  • Manually exercise heroku apps:info, heroku apps:info --shell, heroku apps:info --json, heroku apps:info --extended, and heroku apps:info --extended --json against a real app before merging.

@tlowrimore-heroku
test(apps:info): stub @heroku/sdk and @heroku/heroku-fetch directly, …
0b2a965 
…drop nock

Replaces the nock-based HTTP interception with prototype stubs on
HerokuSDK.platform (for the platform.app.describe call) and on
HerokuApiClient.get (for the --extended escape-hatch call). This keeps
the tests asserting on the SDK contract the migrated command actually
depends on, rather than on URL shape.
@tlowrimore-heroku
chore: extend SDK migration skill with composite-method and escape-ha…
f8633ed 
…tch guidance

Captures lessons from the apps:info migration:

- Pre-flight P1: replace the broken Node diagnostics. The SDK's exports
  map blocks `require('@heroku/sdk/package.json')` and the entry's
  top-level await blocks `require('@heroku/sdk')`. Read the installed
  package.json directly and use `node --input-type=module` for the
  HerokuSDK probe.
- Pre-flight P4 (new): look for composite resource methods before
  running the codemod. The codemod walks the route table and can never
  suggest composites like `platform.app.describe`, but they collapse
  Promise.all-of-N-endpoints patterns into a single call with built-in
  soft-fail behavior. Note the snake_case / camelCase mapping that
  composites often require to preserve CLI output contracts.
- Step 1.2b (new): document the `HerokuApiClient` escape hatch from
  `@heroku/heroku-fetch` for CLI-only query variants (e.g.,
  ?extended=true) the SDK doesn't expose. Preferable to keeping a
  `this.heroku.<verb>` thread because it cleanly removes the APIClient
  dependency and stubs at one prototype boundary in tests.
- Step 2.2b (new): parallel test-stub recipe for the escape hatch.
  Stub HerokuApiClient.prototype.get and use a duck-typed
  `{json: async () => fixture}` return — avoids the
  n/no-unsupported-features/node-builtins warning on `new Response()`.
- Step V3: codify `feat/heroku-sdk-integration` as the base branch for
  all per-command migration PRs (not main). Avoids surfacing
  ~120 files of integration-branch noise to reviewers.
- Tech-stack section: SDK is now a published beta (^0.1.0-beta), no
  longer tracking GitHub main. Added @heroku/heroku-fetch as a direct
  dep.
- Self-review checklist: add composite-method check, escape-hatch test
  shape, and base-branch verification.
- Glossary: composite method, escape hatch, integration branch.
@tlowrimore-heroku
fix(apps:info): register appExtensions so platform.app.describe is wired
0fcf0dd 
The previous commit constructed the SDK with no extensions, so
platform.app.describe was undefined at runtime — production would have
hit a TypeError on the first invocation. Stub-based tests didn't catch
it because the test fixtures define `app.describe` directly on the fake
platform, bypassing the missing extension entirely.

Also extends the SDK migration skill: P4 now spells out that composite
methods are extensions and must be registered on the SDK constructor,
and a new self-review item verifies the wiring. Lists the extension
exports currently used in this repo (apps, addons, pipeline-coupling,
database, postgres-database, log-session) to make the right choice
discoverable at migration time.
@tlowrimore-heroku
fix(apps:info): thread appExtensions through Platform type alias
990f8a1 
The bare `type Platform = HerokuSDK['platform']` indexes the class's
prototype-level getter without supplying the `Exts` generic, so the
type falls back to its default empty tuple and `describe` is missing
from the resolved type. Runtime wiring was correct (the constructor
got `{extensions: [appExtensions]}`), but the helper's parameter type
didn't see the extension. Parameterizing the alias as
`HerokuSDK<readonly [typeof appExtensions]>['platform']` aligns the
helper's static type with the constructed SDK's actual shape.

Why this wasn't caught earlier: the skill's tsc-delta verification
recipe (`grep -v -F -f /tmp/tsc-baseline.txt`) outputs nothing when
the baseline file is empty, because grep treats an empty pattern file
as "no patterns to match" and -v inverts that to "exclude everything"
— a false "no errors" signal in the clean-baseline case.

Skill updates:
- Step 1.3 and V2: guard the baseline filter with `[ -s … ]` so an
  empty baseline falls back to running tsc unfiltered.
- Step P4: extended composite-method guidance with the type-level
  threading recipe — `type Platform = HerokuSDK<readonly [typeof
  appExtensions]>['platform']` for helpers that call extension methods.
@tlowrimore-heroku
refactor(apps:info): replace @heroku-cli/schema with @heroku/types an…
7a58cbe 
…d SDK composites

@heroku-cli/schema is deprecated. Switches the file to:

- Entity types (`App`, `AddOn`, `Collaborator`, `Dyno`) from
  `@heroku/types/3.sdk`.
- Composite return types (`AppInfo`, `PipelineCouplingDetail`) from
  `@heroku/sdk/resources/platform/app/info`.
- A local `LocalApp` extension that captures fields the platform
  returns but `@heroku/types/3.sdk` doesn't declare yet
  (`cron_finished_at`, `cron_next_run`, `database_size`,
  `create_status`, `extended`). Makes the schema gap visible and
  reviewable rather than papering over with `as any`.
- A local `Info` type for the helper return shape (snake_case
  `pipeline_coupling`) that mirrors what print/shell consume,
  replacing the previous misuse of `Heroku.App` as a struct holder.

Strict-null fields (`web_url`, `repo_size`, `slug_size`) reach
non-null-accepting consumers (`filesize`, `color.info`); cast at the
call sites to preserve the original behavior of printing the value
literally when null. The local `print(k, v)` inside `shell()` is
loosened from `(k: string, v: string)` to `(k: string, v: unknown)`
since runtime is template-literal interpolation, which preserves the
existing behavior of printing booleans, arrays, and undefined values
verbatim.

Skill updates:
- Tech stack: call out @heroku-cli/schema as deprecated.
- Step 1.2c (new): replacement playbook covering entity types,
  composite return types, and the local-extension recipe for fields
  the strict schema doesn't declare. Documents the safe handling of
  strict-null findings (no silent `?? ''` coalescing — preserve
  observable output).
- Self-review checklist: replace the "no schema import if unused"
  item with a stronger "no schema import, period" plus a strict-null
  preservation check.
- Glossary: distinguish entity types vs. composite return types and
  their import sources.
@tlowrimore-heroku
refactor(apps:info): derive Info from AppInfo, promote ExtendedApp to…
29d65dd 
… lib/types

Replace the field-by-field Info copy with `Omit<AppInfo, 'app'> & {app: ExtendedApp}`
so new SDK fields propagate automatically. Localize the
`pipelineCoupling` -> `pipeline_coupling` rename to the JSON output
boundary instead of threading it through internal helpers.

Promote `LocalApp` -> `ExtendedApp` into `src/lib/types/app.d.ts` next to
the existing `App`/`Apps` exports so future migrations can reuse it.
@tlowrimore-heroku
chore: capture derive-don't-restate and boundary-translation patterns…
04ea66b 
… in SDK migration skill

Add two principle subsections to Step 1.2c capturing patterns that will
recur across command migrations:

- Derive composite-shaped helper types from the SDK rather than restating
  them; field-by-field copies type-check but break at runtime if the SDK
  renames a field upstream.
- Localize CLI/SDK contract translations (snake_case <-> camelCase) to
  the output boundary instead of threading renamed fields through
  internal helpers.

Also redirect bullet 3 of Step 1.2c to `src/lib/types/` first (with
`ExtendedApp` named explicitly), and frame local extension type
declarations as transient stepping stones promoted on second use.
@tlowrimore-heroku tlowrimore-heroku force-pushed the feat/apps-info-sdk-update branch from 8953abd to 04ea66b Compare June 5, 2026 15:27
@tlowrimore-heroku
Merge branch 'feat/heroku-sdk-integration' into feat/apps-info-sdk-up…
c2467a4 
…date

Signed-off-by: Timothy Lowrimore <154477569+tlowrimore-heroku@users.noreply.github.com>
michaelmalave
michaelmalave approved these changes Jun 5, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@michaelmalave michaelmalave left a comment

Choose a reason for hiding this comment

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

I think its unclear now how tight we're working with the SDK at this point? I think there are some things we should consider for the SDK but not necessarily blocking because clean migration otherwise. Nice.

Comment thread src/commands/apps/info.ts
print('web_url', info.app.web_url)
print('repo_size', filesize(info.app.repo_size, {round: 0, standard: 'jedec'}))
if (getGeneration(info.app) !== 'fir') print('slug_size', filesize(info.app.slug_size, {round: 0, standard: 'jedec'}))
print('web_url', info.app.web_url as string)
Copy link
Copy Markdown
Contributor

@michaelmalave michaelmalave Jun 5, 2026

Choose a reason for hiding this comment

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

We're casting web_url as string, repo_size as number, slug_size as number because @heroku/security-starter-projects defines them as T | null. The command assumes they're always populated for an existing app. Should we add a DescribedApp narrowed type in the SDK's info.ts that asserts these fields are non-null post-fetch (since describe only returns real apps, not create-in-progress shells), so CLI commands don't need as casts?

Comment thread src/commands/apps/info.ts
async function getInfo(app: string, platform: Platform, extended: boolean): Promise<Info> {
const data: Info = await platform.app.describe(app)

if (extended) {
Copy link
Copy Markdown
Contributor

@michaelmalave michaelmalave Jun 5, 2026

Choose a reason for hiding this comment

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

We're using HerokuApiClient directly for the extended endpoint. Since the SDK already owns the composite describe, should describe accept an extended?: boolean option, fetch ?extended=true internally, and merge the result — pushing that logic out of the CLI?

Copy link
Copy Markdown
Contributor Author

@tlowrimore-heroku tlowrimore-heroku Jun 5, 2026

Choose a reason for hiding this comment

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

I went back and forth about this, and landed on this being uniquely a CLI concern, as I understand it. The other thought is that SDK will be customer-facing and this extended flag pertains only to sudo users.

Comment thread src/lib/types/app.d.ts
export type App = BaseApp & Pick<TeamApp, 'locked' | 'joined'>
export type Apps = App[]

export type ExtendedApp = App & {
Copy link
Copy Markdown
Contributor

@michaelmalave michaelmalave Jun 5, 2026

Choose a reason for hiding this comment

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

We're defining ExtendedApp locally with create_status, cron_*, database_size, extended fields. If the SDK's describe gains the extended option, these fields belong on the SDK's return type. Should we move this type into @heroku/sdk or @heroku/types so it's the SDK's contract, not a CLI-local workaround?

Copy link
Copy Markdown
Contributor Author

@tlowrimore-heroku tlowrimore-heroku Jun 5, 2026

Choose a reason for hiding this comment

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

See my previous note on the extended arg to the describe function.

@tlowrimore-heroku tlowrimore-heroku merged commit 3ec2f64 into feat/heroku-sdk-integration Jun 5, 2026
17 checks passed
@tlowrimore-heroku tlowrimore-heroku deleted the feat/apps-info-sdk-update branch June 5, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelmalave michaelmalave michaelmalave approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tlowrimore-heroku @michaelmalave