diff --git a/.claude/skills/sdk-command-migration/SKILL.md b/.claude/skills/sdk-command-migration/SKILL.md index 277a28c8b6..6822ca6433 100644 --- a/.claude/skills/sdk-command-migration/SKILL.md +++ b/.claude/skills/sdk-command-migration/SKILL.md @@ -28,7 +28,9 @@ Apply once per command in `src/commands/`. Each application produces one PR-read - TypeScript with NodeNext ESM, `module: "NodeNext"`, target `ES2022`. - oclif 4 command framework. -- `@heroku/sdk` (current branch tracks GitHub `main`); the bare entry exports `HerokuSDK` and `HerokuSDKOptions`. +- `@heroku/sdk` published beta (currently `^0.1.0-beta`); the bare entry exports `HerokuSDK` and `HerokuSDKOptions`. `@heroku/types` (the route metadata package) is `^3.0.0-beta`. +- `@heroku/heroku-fetch` is a direct dependency exporting `HerokuApiClient`. The SDK uses it internally; commands also use it directly for CLI-only escape hatches (see "CLI-only escape hatches" below). +- **`@heroku-cli/schema` is deprecated.** Replace `import * as Heroku from '@heroku-cli/schema'` with imports from `@heroku/types/3.sdk` (entity types: `App`, `AddOn`, `Collaborator`, `Dyno`, etc.) and from the SDK composite resource files (`AppInfo`, `PipelineCouplingDetail`, etc., from `@heroku/sdk/resources///`). See "Replacing @heroku-cli/schema" below. - Tests use `mocha` + `chai` + `chai-as-promised` + `sinon`. Existing tests use `nock` to intercept HTTP; the rewrite drops `nock` in favor of direct SDK stubbing. ## Process @@ -45,24 +47,15 @@ Each step is required. Do not skip. Run these once per command, before any code change. They prevent the most common surprises. -### Step P1: Confirm working tree is clean and SDK is on disk +### Steps P1+P2: Working tree, SDK probe, and baselines ```bash -git status -sb -node -e "console.log(require('@heroku/sdk/package.json').version)" -node -e "const {HerokuSDK} = require('@heroku/sdk'); console.log(typeof HerokuSDK)" +bash scripts/codemods/sdk-migration/preflight.sh test/unit/commands/.unit.test.ts ``` -Expected: no unmerged paths (`UU`), `HerokuSDK` is `function`. If the working tree is dirty, resolve before proceeding (commit, stash, or restore — depending on what's there). If `HerokuSDK` is `undefined`, the SDK is on the wrong version and the rest of this skill will not apply cleanly. +Expected: no unmerged paths (`UU`), `HerokuSDK is function`, tsc baseline written to `/tmp/tsc-baseline.txt`, target test file pass/fail captured. If the working tree is dirty, resolve before proceeding (commit, stash, or restore). If `HerokuSDK` prints `undefined`, the SDK is on the wrong version and the rest of this skill will not apply cleanly. If the test file was already failing, stop and ask the user. -### Step P2: Capture baseline of pre-existing failures - -```bash -npx tsc --noEmit -p tsconfig.json 2>&1 | tee /tmp/tsc-baseline.txt | tail -20 -npx mocha 'test/unit/commands/.unit.test.ts' --reporter min 2>&1 | tail -5 -``` - -Save the `tsc` baseline. Any errors present here are NOT your responsibility — your goal is "no *new* errors after migration." For the test file, capture pass/fail status; if it was already failing, stop and ask the user before continuing. +Any tsc errors in the baseline are NOT your responsibility — your goal is "no *new* errors after migration." The script's source comments explain why the SDK probe uses `--input-type=module` (top-level await + missing `./package.json` export). ### Step P3: Verify the command's call surface @@ -80,6 +73,53 @@ grep -nE "(^|[^.])\bheroku\.(get|post|patch|put|delete)\(" src/commands// +``` + +Files other than `index.{js,d.ts}` are composite or extension methods. Read the `.d.ts` to see the input/output shape. + +When a composite fits, prefer it over the codemod output: the SDK encapsulates the parallelism and soft-fail logic that today lives inline in the command. The win is a "deep module" interface (one method, one arg) hiding substantial internal complexity. Note the field-name mapping you may need to apply — composites tend to use camelCase return fields, but CLI output contracts are often snake_case (e.g., `pipelineCoupling` from the SDK → `pipeline_coupling` in the command's output). + +**Composites are extension methods — register them on the SDK.** `platform.app.describe`, `addOn.create` (with the user-friendly options shape), `pipelineCoupling.infoByApp`, etc. are spliced onto the resource via `extendResource` and only exist when you pass the corresponding extension into the SDK constructor: + +```ts +import {HerokuSDK} from '@heroku/sdk' +import {appExtensions} from '@heroku/sdk/extensions/platform' + +const {platform} = new HerokuSDK({extensions: [appExtensions]}) +await platform.app.describe('myapp') // works +``` + +Without the `extensions: [...]` argument, `platform.app.describe` is `undefined` at runtime and you get a TypeError. **Test stubs will mask the runtime miss** — `fakePlatform.app.describe = sinon.stub()` is structurally fine even when production wiring is missing, so confirm by reading other commands' wiring (e.g., `src/commands/addons/info.ts`) and run a smoke test against a real app before merging. + +**Helper signatures need the extensions threaded through too.** If you extract a helper that takes `platform` as a parameter, the obvious-looking `type Platform = HerokuSDK['platform']` resolves to the *unextended* `PlatformClient` — the class's `Exts` generic falls back to its default empty tuple when you index without supplying it, so `describe` is statically missing on the parameter type even when the runtime call site has the extension. Parameterize the alias instead: + +```ts +type Platform = HerokuSDK['platform'] + +async function getInfo(app: string, platform: Platform) { + const described = await platform.app.describe(app) // ok +} +``` + +For helpers that only call route-derived methods (e.g., `app.info`, `app.update`), the bare `HerokuSDK['platform']` alias is fine. + +The extension exports are at `@heroku/sdk/extensions/platform` and `@heroku/sdk/extensions/data`. Existing usage: +- `appExtensions` — `describe`, `enableMaintenance`, `disableMaintenance`, `getGeneration`, `getProcessTier`, `isShielded` +- `addOnExtensions` — `create` with the rich options shape used in `addons:create` +- `pipelineCouplingExtensions` — `infoByApp` and related composites used in pipelines commands +- `databaseExtensions` / `postgresDatabaseExtensions` — pg-resource composites +- `logSessionExtensions` — used in `lib/run/log-displayer.ts` for streaming + +For commands whose `Promise.all` doesn't match an existing composite, run the codemod normally — the per-call replacements still produce a clean migration. + --- ## Task 1: Migrate the command source @@ -133,11 +173,7 @@ The CLI passed http-call options the SDK doesn't accept. For data routes, a lone The SDK's dispatcher only forwards a request body when the route metadata has `hasRequestBody: true`. If `@heroku/types` is missing that flag for a route, the generated SDK method's TS signature won't accept a body parameter *and* the dispatcher will silently drop any body you cast through. Symptoms: tests fail with "request body did not match" or the migrated PATCH/POST behaves as if it sent an empty payload. Diagnostic: ```bash -npx tsx -e " -import {RouteIndex} from './scripts/codemods/sdk-migration/routes-index.ts'; -const r = RouteIndex.load().lookup('PATCH', '/apps/example/config-vars'); -console.log('hasRequestBody:', r?.entry.hasRequestBody, 'method:', r?.entry.resource + '.' + r?.entry.method); -" +bash scripts/codemods/sdk-migration/check-route.sh PATCH /apps/example/config-vars ``` If `hasRequestBody` is `false` but the endpoint logically requires a body, the fix is upstream: the user (or you) needs to bump `@heroku/types` to a version where the route metadata is correct. Stop and ask the user before working around this — escape-hatching to the underlying client defeats the migration's purpose. Once the dependency is updated, re-run `npm install`, re-verify the diagnostic shows `hasRequestBody: true`, and refresh the Pre-flight P2 baseline (the bump may resolve unrelated `tsc` errors too). @@ -166,13 +202,122 @@ For body shapes the codemod would have unwrapped, do the unwrap by hand. The pat Both shapes (direct and helper-threaded) can coexist in the same command. If the codemod migrated some call sites and "no change"d others, finish the remaining ones manually using this step before moving on. +### Step 1.2c: Replace `@heroku-cli/schema` imports + +`@heroku-cli/schema` is deprecated. Every reference to `Heroku.` in the migrated file needs to be replaced with the canonical type. There are three sources to choose from, in priority order: + +1. **Entity types — `@heroku/types/3.sdk`.** The base wire-format types: `App`, `AddOn`, `Collaborator`, `Dyno`, `PipelineCoupling`, `TeamApp`, `Release`, etc. Use these wherever the original code used `Heroku.App`, `Heroku.AddOn`, etc., for individual platform entities. + + ```ts + import {AddOn, App, Collaborator, Dyno} from '@heroku/types/3.sdk' + ``` + + Use the value-form `import` (no `type` modifier) to avoid the `n/no-extraneous-import` lint quirk on transitive deps. + +2. **Composite return types — `@heroku/sdk/resources///`.** When the migrated code uses a composite SDK method (`platform.app.describe`, etc., from Step P4), the composite's return type is exported from the resource file alongside the function: + + ```ts + import type {AppInfo, PipelineCouplingDetail} from '@heroku/sdk/resources/platform/app/info' + ``` + + The SDK's `package.json` exports map uses `./resources/*` with a wildcard; the path must be specific enough to disambiguate the file (e.g., `app/info`, not `app` — the latter resolves to a sibling file at the parent level). + +3. **Extension types — `src/lib/types/`, with local declarations only as a stepping stone.** Strict types from `@heroku/types/3.sdk` are *narrower* than the loose `@heroku-cli/schema` types they replace. They will surface fields the CLI reads but `@heroku/types` doesn't declare (e.g., `cron_finished_at`, `cron_next_run`, `database_size`, `create_status`, `extended` on `App` — all platform-returned but absent from the strict schema). Don't paper over with `as any`. + + Check `src/lib/types/.d.ts` first — the gaps that have already surfaced in earlier migrations are exported there. For `App`, the existing extension is `ExtendedApp` in `src/lib/types/app.d.ts`: + + ```ts + import {ExtendedApp} from '../../lib/types/app.js' + ``` + + If the gap you're hitting isn't already exported, add the extension type next to the existing `App`/`Apps` exports rather than declaring it locally in the command. Use a name that describes the *use case* (`ExtendedApp` = the `App` shape under `--extended`), not the file it lives in (avoid `LocalApp`, `MyApp`). A local declaration in the command file is fine as a transient stepping stone, but promote it to `src/lib/types/` the moment a second command needs the same fields. + + When the same gap accumulates across many commands, propose an upstream `@heroku/types` bump and remove the extension once the strict schema catches up. + +**Derive composite-shaped helper types from the SDK; don't restate them.** When a command uses a composite return type (`AppInfo`, etc.) and needs to widen one field — e.g., the `app` field is the `ExtendedApp` shape under `--extended` — derive structurally instead of restating the whole type: + +```ts +// Good — one field overridden, the rest stays in sync with the SDK. +type Info = Omit & {app: ExtendedApp} + +async function getInfo(...): Promise { + return platform.app.describe(app) // structural assignment, no field copy +} + +// Bad — restates the SDK shape and copies fields one-by-one. New SDK fields +// silently drop on the floor; an upstream rename type-checks but breaks at runtime. +type Info = { + addons: AddOn[] + app: ExtendedApp + collaborators: Collaborator[] + dynos: Dyno[] + pipeline_coupling: null | PipelineCouplingDetail +} + +async function getInfo(...): Promise { + const described = await platform.app.describe(app) + return { + addons: described.addons, + app: described.app, + collaborators: described.collaborators, + dynos: described.dynos, + pipeline_coupling: described.pipelineCoupling, + } +} +``` + +The pathology of restating: `pipeline_coupling: described.pipelineCoupling` still type-checks if the SDK renames `pipelineCoupling` upstream — the bug surfaces only at runtime as `undefined`. + +**Localize CLI/SDK contract translations to the output boundary.** The CLI's JSON/shell output contract is snake_case; SDK return shapes are camelCase. Don't thread renamed fields through the entire helper pipeline — keep the SDK shape internal and rename only at the output sites that are the CLI contract: + +```ts +// Good — rename happens at the JSON serialization boundary. +} else if (flags.json) { + const {pipelineCoupling, ...rest} = info + hux.styledJSON({...rest, pipeline_coupling: pipelineCoupling}) +} + +// Bad — pipeline_coupling threaded through every read site upstream. +if (info.pipeline_coupling) print('pipeline', `${info.pipeline_coupling.pipeline.name}:...`) +``` + +The CLI is the bounded context owning snake_case output; the SDK is a different bounded context using camelCase. Translation belongs at the seam, not inside either domain. This pattern recurs on every command whose JSON output uses snake_case — which is most of them. + +**Strict-null findings need careful handling.** Where `@heroku-cli/schema` had `web_url: string`, `@heroku/types/3.sdk` has `web_url: string | null` (correct per the OpenAPI spec). The migrated code may pass these through to functions expecting non-null arguments — e.g., `filesize(repo_size, ...)` or `print('web_url', web_url)`. **Don't silently coalesce nulls (`?? ''`, `?? 0`)**: that changes observable output (`web_url=null` becomes `web_url=`). Prefer one of: + +- **`as` cast at the call site** when the runtime path is gated upstream (truthiness check) but TS can't prove it: `filesize(info.app.repo_size as number, ...)`. +- **Loosen the consumer's signature** if the consumer is local and just stringifies: `function print(k: string, v: unknown)` instead of `(k: string, v: string)`. This preserves the original behavior of printing `null` / `undefined` literally if they slipped through. + +Don't just gate with truthiness if the original code didn't — that's a behavior change disguised as a type fix. + +### Step 1.2b: CLI-only escape hatches via `HerokuApiClient` + +Some endpoints exist on the platform but aren't exposed by the SDK because they're CLI-only concerns — the canonical example is `GET /apps/{id}?extended=true`, which the route table collapses to plain `app.info` (the query string is lost in lookup). When the user confirms the variant should stay a CLI concern, drop to `HerokuApiClient` directly rather than keeping a `this.heroku.` thread: + +```ts +import {HerokuApiClient} from '@heroku/heroku-fetch' + +const client = new HerokuApiClient() +const response = await client.get(`/apps/${app}?extended=true`) +const appExtended = await response.json() as ExtendedApp +``` + +Why this is preferable to keeping `this.heroku.`: +- It removes the `APIClient` thread from the command (no need to keep `client: Command` parameters around just for one call). +- It stubs at one well-defined prototype boundary in tests (`HerokuApiClient.prototype.get`), parallel to the SDK stubbing pattern. +- It's the same client the SDK uses internally, so there's no behavioral divergence. + +The return value is a `Response`-shaped object — call `.json()` to get the body. Don't construct real `Response` objects in tests; duck-type the stub return as `{json: async () => fixture}` (avoids `n/no-unsupported-features/node-builtins` warnings and only stubs what the command actually uses). + +Confirm with the user before reaching for this — it's a deliberate escape hatch, not a default. Document it in the source commit body. + ### Step 1.3: Type-check ```bash -npx tsc --noEmit -p tsconfig.json 2>&1 | grep -v -F -f /tmp/tsc-baseline.txt | tail -20 +bash scripts/codemods/sdk-migration/tsc-delta.sh 20 ``` -Expected: empty output (no new errors). If new errors appear, they typically fall into: +Expected: empty output (no new errors). The script's source comments explain the empty-baseline trap it guards against. If new errors appear, they typically fall into: - **Local-type incompatibility** → use a single-step cast (`as App[]`, not `as unknown as App[]`) at the call site. Reach for `as unknown as X` only if the single-step is rejected. - **Helper signature mismatch** → if a helper parameter was typed as `Heroku.X` to mean an array, fix the helper to `App[]` honestly. Anticipate that `lodash` operations like `_.partition` return tuples — destructure: `const [a, b] = _.partition(...)`. @@ -254,6 +399,26 @@ This works because `HerokuSDK.platform` is a class-prototype getter (configurabl If you encounter `TypeError: Descriptor for property platform is non-configurable and non-writable`, escalate — the SDK's class shape changed and this skill needs updating. +### Step 2.2b: Stub `HerokuApiClient.prototype.get` if the command uses the escape hatch + +When the source command uses `HerokuApiClient` directly (Step 1.2b), add a parallel prototype stub: + +```ts +import {HerokuApiClient} from '@heroku/heroku-fetch' + +let apiGet: sinon.SinonStub + +beforeEach(function () { + // ...platform stub from 2.2... + apiGet = sinon.stub(HerokuApiClient.prototype, 'get') +}) + +// Per test: +apiGet.withArgs('/apps/myapp?extended=true').resolves({json: async () => appExtendedFixture}) +``` + +Use a duck-typed `{json: async () => fixture}` return rather than constructing a real `Response`. The `Response` global is flagged as experimental on Node 20 by `n/no-unsupported-features/node-builtins`; the duck-type also captures the SDK contract more precisely (the migrated code only calls `.json()`, nothing else). + ### Step 2.3: Wire individual stubs per test, drop `nock` interceptors ```ts @@ -299,17 +464,38 @@ Use the parent directory of the migrated command. Expected: all passing. If a si ### Step V2: Type-check delta ```bash -npx tsc --noEmit -p tsconfig.json 2>&1 | grep -v -F -f /tmp/tsc-baseline.txt +bash scripts/codemods/sdk-migration/tsc-delta.sh ``` Expected: empty. New errors mean the migration introduced a regression. ### Step V3: Push and open PR +**Base branch is `feat/heroku-sdk-integration`, not `main`.** All SDK-migration work stacks onto the integration branch until that branch lands. Opening against `main` would surface ~120 files of integration-branch noise to reviewers; opening against `feat/heroku-sdk-integration` shows only the per-command diff (~4 files). When the integration branch eventually merges, GitHub automatically updates the open child PRs to target `main` with the same minimal diff. + +```bash +git push -u origin +gh pr create --draft --base feat/heroku-sdk-integration \ + --title "refactor: use @heroku/sdk for command" \ + --body "$(cat <<'EOF' +## Summary +... +## Test plan +- [x] tsc clean vs. baseline +- [x] eslint clean +- [x] mocha for the migrated test file passes +- [x] mocha for sibling tests in the same dir passes +- [ ] Manual smoke test against a real app (list the flag combinations to exercise) +EOF +)" +``` + The PR contains exactly two commits per command: - `refactor: use @heroku/sdk for command` - `test(): stub @heroku/sdk directly, drop nock` +If the source migration uses the `HerokuApiClient` escape hatch (Step 1.2b), append `and @heroku/heroku-fetch` to the test commit subject: `test(): stub @heroku/sdk and @heroku/heroku-fetch directly, drop nock`. + Each PR migrates exactly one command. Don't bundle multiple command migrations — review surface stays small and bisect remains useful if a regression slips through. --- @@ -318,17 +504,24 @@ Each PR migrates exactly one command. Don't bundle multiple command migrations Before opening the PR: -- [ ] No `this.heroku.` calls remain in the migrated file. +- [ ] No `this.heroku.` calls remain in the migrated file. (Escape-hatch calls go through `HerokuApiClient` per Step 1.2b — never via `this.heroku`.) - [ ] No bare `heroku.(...)` calls remain in helper functions (helper-threading shape — see Step 1.2a). - [ ] No `// TODO(sdk-migration):` markers remain. -- [ ] No `import * as Heroku from '@heroku-cli/schema'` if no longer used. +- [ ] Composite resource methods checked (Step P4) before falling back to per-call replacements when the command had a `Promise.all` over multiple endpoints. +- [ ] If a composite method is used (e.g., `platform.app.describe`), the corresponding extension (`appExtensions`, `addOnExtensions`, etc.) is imported from `@heroku/sdk/extensions/` and passed to the `HerokuSDK` constructor via `{extensions: [...]}`. Without this, the method is `undefined` at runtime — but stub-based tests will pass anyway. Verify by reading a sibling command's wiring or smoke-testing against a real app. +- [ ] No `import * as Heroku from '@heroku-cli/schema'` (deprecated). Entity types come from `@heroku/types/3.sdk`; composite return types from `@heroku/sdk/resources///`; CLI-only field gaps captured by extension types in `src/lib/types/` (Step 1.2c). +- [ ] Helper return types derived from the SDK composite (`Omit & {app: ExtendedApp}`) rather than restated structurally; helpers spread/return the SDK result instead of copying fields one-by-one. +- [ ] CLI/SDK contract renames (e.g., `pipelineCoupling` → `pipeline_coupling`) happen at output boundaries (JSON/shell), not threaded through internal helpers. +- [ ] Strict-null findings handled without changing observable output: `as` cast or loosened consumer signature, never `?? ''` / `?? 0` for fields the original printed verbatim. - [ ] No `import {APIClient} from '@heroku-cli/command'` if no longer used. - [ ] No `as unknown as X` cast where `as X` would suffice. - [ ] No new `tsc` errors (verify against the Pre-flight P2 baseline). -- [ ] Tests rewritten per Task 2: `nock` removed, SDK stubbed via `HerokuSDK.prototype.platform`. +- [ ] Tests rewritten per Task 2: `nock` removed, SDK stubbed via `HerokuSDK.prototype.platform`. If escape hatch in use: `HerokuApiClient.prototype.get` also stubbed (Step 2.2b). +- [ ] Test stubs for the escape hatch return duck-typed `{json: async () => fixture}` — no `new Response(...)`. - [ ] Lint clean on changed files. -- [ ] One source file changed per source commit; commit messages follow the convention. A `package-lock.json` bump driven by a route-metadata gap (Step 1.2) is allowed in the source commit and should be called out in the commit body. +- [ ] One source file changed per source commit; commit messages follow the convention. A `package.json`/`package-lock.json` bump (route-metadata gap from Step 1.2, or a new direct dep like `@heroku/heroku-fetch` for the escape hatch from Step 1.2b) is allowed in the source commit and should be called out in the commit body. - [ ] No incidental edits to other unrelated files (type defs, sibling commands). +- [ ] PR opened with `--base feat/heroku-sdk-integration`, not `main` (Step V3). --- @@ -337,5 +530,9 @@ Before opening the PR: - **Platform service:** `sdk.platform.*` — methods covering Apps, Spaces, Teams, Account, Pipelines, etc. - **Data service:** `sdk.data.*` — methods covering Postgres / data-stores. The codemod migrates these alongside platform calls; the SDK supplies the data hostname automatically. - **Bare entry:** `import {HerokuSDK} from '@heroku/sdk'` — the canonical import. Do not use `@heroku/sdk/sdk` (removed in 0.4) or deep relative imports. +- **Composite method:** a resource method that fans out multiple underlying calls and soft-fails optional ones (e.g., `platform.app.describe`). Lives in `node_modules/@heroku/sdk/dist/resources///` as a separate file from `index.{js,d.ts}`. Not in the codemod's route table — discovered manually per Step P4. +- **Entity types vs. composite return types:** entity types (`App`, `AddOn`, etc.) come from `@heroku/types/3.sdk` and describe the wire format. Composite return types (`AppInfo`, `PipelineCouplingDetail`, etc.) come from the SDK resource file that owns the composite (e.g., `@heroku/sdk/resources/platform/app/info`). +- **Escape hatch:** a direct `HerokuApiClient` call from `@heroku/heroku-fetch` for endpoints/variants the SDK doesn't expose. Sanctioned for CLI-only concerns like `?extended=true` query variants — see Step 1.2b. - **Pre-flight baseline:** the snapshot of `tsc`/test state captured before any migration work, used to filter pre-existing noise out of post-migration verification. - **Codemod:** `scripts/codemods/sdk-migration/migrate-command.ts` — the deterministic transform run in Task 1, Step 1.1. +- **Integration branch:** `feat/heroku-sdk-integration` — the base branch for all per-command migration PRs until the integration lands. See Step V3. diff --git a/package-lock.json b/package-lock.json index 97d33049f9..f5db5d76f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "@heroku-cli/schema": "^1.0.25", "@heroku/buildpack-registry": "^1.0.1", "@heroku/heroku-cli-util": "^10.8.0", + "@heroku/heroku-fetch": "^0.1.1-beta", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", "@heroku/sdk": "^0.5.0-beta.0", diff --git a/package.json b/package.json index 74d7326bc9..41c2bd4b9b 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "@heroku-cli/schema": "^1.0.25", "@heroku/buildpack-registry": "^1.0.1", "@heroku/heroku-cli-util": "^10.8.0", + "@heroku/heroku-fetch": "^0.1.1-beta", "@heroku/http-call": "^5.5.1", "@heroku/mcp-server": "^1.2.0", "@heroku/sdk": "^0.5.0-beta.0", diff --git a/scripts/codemods/sdk-migration/check-route.sh b/scripts/codemods/sdk-migration/check-route.sh new file mode 100755 index 0000000000..b0f184260e --- /dev/null +++ b/scripts/codemods/sdk-migration/check-route.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# Print SDK route metadata for a given (verb, path). +# +# Use to diagnose "body silently dropped at runtime" symptoms: if +# `hasRequestBody` is false but the endpoint logically requires a body, the +# fix is upstream — bump @heroku/types to a version where the route metadata +# is correct. Don't escape-hatch around it. +# +# Usage: check-route.sh +# e.g. check-route.sh PATCH /apps/example/config-vars +set -euo pipefail + +VERB="${1:-}" +ROUTE_PATH="${2:-}" + +if [ -z "$VERB" ] || [ -z "$ROUTE_PATH" ]; then + echo "usage: $0 " >&2 + echo " e.g. $0 PATCH /apps/example/config-vars" >&2 + exit 2 +fi + +npx tsx -e " +import {RouteIndex} from './scripts/codemods/sdk-migration/routes-index.ts'; +const r = RouteIndex.load().lookup('$VERB', '$ROUTE_PATH'); +if (!r) { console.log('no match for $VERB $ROUTE_PATH'); process.exit(0); } +console.log('hasRequestBody:', r.entry.hasRequestBody, 'method:', r.entry.resource + '.' + r.entry.method); +" diff --git a/scripts/codemods/sdk-migration/preflight.sh b/scripts/codemods/sdk-migration/preflight.sh new file mode 100755 index 0000000000..167cf128f5 --- /dev/null +++ b/scripts/codemods/sdk-migration/preflight.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Pre-flight checks for sdk-command-migration: P1 (clean working tree, SDK on +# disk) and P2 (capture baselines). +# +# Why the SDK probe looks the way it does: the SDK's `exports` map doesn't +# expose `./package.json` (so `require('@heroku/sdk/package.json')` fails with +# ERR_PACKAGE_PATH_NOT_EXPORTED), and the SDK's entry uses top-level `await` +# (so `require('@heroku/sdk')` fails with ERR_REQUIRE_ASYNC_MODULE). Reading +# `node_modules/@heroku/sdk/package.json` directly and using +# `--input-type=module` for the import sidesteps both. +# +# Usage: preflight.sh +# e.g. preflight.sh test/unit/commands/apps/info.unit.test.ts +set -euo pipefail + +TEST_PATH="${1:-}" +if [ -z "$TEST_PATH" ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +BASELINE="${TSC_BASELINE:-/tmp/tsc-baseline.txt}" + +echo "=== P1: working tree ===" +git status -sb + +echo +echo "=== P1: SDK on disk ===" +grep -E '"(name|version)"' node_modules/@heroku/sdk/package.json +node --input-type=module -e "import {HerokuSDK} from '@heroku/sdk'; console.log('HerokuSDK is', typeof HerokuSDK)" + +echo +echo "=== P2: tsc baseline -> $BASELINE ===" +npx tsc --noEmit -p tsconfig.json 2>&1 | tee "$BASELINE" | tail -20 + +echo +echo "=== P2: target test file ===" +npx mocha "$TEST_PATH" --reporter min 2>&1 | tail -5 diff --git a/scripts/codemods/sdk-migration/tsc-delta.sh b/scripts/codemods/sdk-migration/tsc-delta.sh new file mode 100755 index 0000000000..e8c0a4c64c --- /dev/null +++ b/scripts/codemods/sdk-migration/tsc-delta.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# Run tsc and filter pre-existing errors against /tmp/tsc-baseline.txt. +# +# The `[ -s "$BASELINE" ]` guard is load-bearing: when the baseline file is +# empty (clean repo at pre-flight time), `grep -v -F -f` matches *nothing* +# because the empty pattern file has no patterns — yielding a false "no +# errors" signal in exactly the case where strict checking matters most. +# +# Usage: tsc-delta.sh [tail-lines] +# tail-lines: optional, pipes through `tail -N` (use "all" for unfiltered) +set -euo pipefail + +BASELINE="${TSC_BASELINE:-/tmp/tsc-baseline.txt}" +TAIL="${1:-all}" + +run_tsc() { + if [ -s "$BASELINE" ]; then + npx tsc --noEmit -p tsconfig.json 2>&1 | grep -v -F -f "$BASELINE" || true + else + npx tsc --noEmit -p tsconfig.json 2>&1 || true + fi +} + +if [ "$TAIL" = "all" ]; then + run_tsc +else + run_tsc | tail -"$TAIL" +fi diff --git a/src/commands/apps/info.ts b/src/commands/apps/info.ts index 31c2f09b61..f0dbdaa02f 100644 --- a/src/commands/apps/info.ts +++ b/src/commands/apps/info.ts @@ -1,12 +1,21 @@ +import type {AppInfo} from '@heroku/sdk/resources/platform/app/info' + import {Command, flags} from '@heroku-cli/command' -import * as Heroku from '@heroku-cli/schema' import {color, hux} from '@heroku/heroku-cli-util' +import {HerokuApiClient} from '@heroku/heroku-fetch' +import {HerokuSDK} from '@heroku/sdk' +import {appExtensions} from '@heroku/sdk/extensions/platform' import {Args, ux} from '@oclif/core' import {filesize} from 'filesize' import {inspect} from 'node:util' import {getGeneration} from '../../lib/apps/generation.js' import {lazyModuleLoader} from '../../lib/lazy-module-loader.js' +import {ExtendedApp} from '../../lib/types/app.js' + +type Platform = HerokuSDK['platform'] + +type Info = Omit & {app: ExtendedApp} export default class AppsInfo extends Command { static args = { @@ -45,14 +54,16 @@ repo_size=5000000 const app = args.app || flags.app if (!app) throw new Error('No app specified.\nUSAGE: heroku apps:info --app my-app') - const info = await getInfo(app, this, flags.extended) - const addons = info.addons.map((a: Heroku.AddOn) => a.plan?.name).sort() - const collaborators = info.collaborators.map((c: Heroku.Collaborator) => c.user.email) - .filter((c: Heroku.Collaborator) => c !== info.app.owner.email) + const {platform} = new HerokuSDK({extensions: [appExtensions]}) + + const info = await getInfo(app, platform, flags.extended) + const addons = info.addons.map(a => a.plan?.name).sort() + const collaborators = info.collaborators.map(c => c.user.email) + .filter(c => c !== info.app.owner.email) .sort() function shell() { - function print(k: string, v: string) { + function print(k: string, v: unknown) { ux.stdout(`${_.snakeCase(k)}=${v}`) } @@ -65,12 +76,12 @@ repo_size=5000000 if (info.app.cron_next_run) print('cron_next_run', formatDate(new Date(info.app.cron_next_run))) if (info.app.database_size) print('database_size', filesize(info.app.database_size, {round: 0, standard: 'jedec'})) if (info.app.create_status !== 'complete') print('create_status', info.app.create_status) - if (info.pipeline_coupling) print('pipeline', `${info.pipeline_coupling.pipeline.name}:${info.pipeline_coupling.stage}`) + if (info.pipelineCoupling) print('pipeline', `${info.pipelineCoupling.pipeline.name}:${info.pipelineCoupling.stage}`) print('git_url', info.app.git_url) - 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) + print('repo_size', filesize(info.app.repo_size as number, {round: 0, standard: 'jedec'})) + if (getGeneration(info.app) !== 'fir') print('slug_size', filesize(info.app.slug_size as number, {round: 0, standard: 'jedec'})) print('owner', info.app.owner.email) print('region', info.app.region.name) print('dynos', inspect(_.countBy(info.dynos, 'type'))) @@ -80,7 +91,8 @@ repo_size=5000000 if (flags.shell) { shell() } else if (flags.json) { - hux.styledJSON(info) + const {pipelineCoupling, ...rest} = info + hux.styledJSON({...rest, pipeline_coupling: pipelineCoupling}) } else { print(info, addons, collaborators, flags.extended, _) } @@ -91,51 +103,22 @@ function formatDate(date: Date) { return date.toISOString() } -async function getInfo(app: string, client: Command, extended: boolean) { - const promises = [ - client.heroku.get(`/apps/${app}/addons`), - client.heroku.request(`/apps/${app}`), - client.heroku.get(`/apps/${app}/dynos`).catch(() => ({body: []})), - client.heroku.get(`/apps/${app}/collaborators`).catch(() => ({body: []})), - client.heroku.get(`/apps/${app}/pipeline-couplings`).catch(() => ({body: null})), - ] - - if (extended) { - promises.push(client.heroku.get(`/apps/${app}?extended=true`)) - } - - const [ - {body: addons}, - {body: appWithMoreInfo}, - {body: dynos}, - {body: collaborators}, - {body: pipelineCouplings}, - appExtendedResponse, - ] = await Promise.all(promises) - - const data: Heroku.App = { - addons, - app: appWithMoreInfo, - collaborators, - dynos, - pipeline_coupling: pipelineCouplings, - } - - if (appExtendedResponse) { - data.appExtended = appExtendedResponse.body - } +async function getInfo(app: string, platform: Platform, extended: boolean): Promise { + const data: Info = await platform.app.describe(app) if (extended) { - data.appExtended.acm = data.app.acm - data.app = data.appExtended - delete data.appExtended + const client = new HerokuApiClient() + const response = await client.get(`/apps/${app}?extended=true`) + const appExtended = await response.json() as ExtendedApp + appExtended.acm = data.app.acm + data.app = appExtended } return data } -function print(info: Heroku.App, addons: Heroku.AddOn[], collaborators: Heroku.Collaborator[], extended: boolean, _: any) { - const data: Heroku.App = {} +function print(info: Info, addons: (string | undefined)[], collaborators: (string | undefined)[], extended: boolean, _: any) { + const data: Record = {} data.Addons = addons data.Collaborators = collaborators @@ -144,15 +127,15 @@ function print(info: Heroku.App, addons: Heroku.AddOn[], collaborators: Heroku.C if (info.app.cron_next_run) data['Cron Next Run'] = formatDate(new Date(info.app.cron_next_run)) if (info.app.database_size) data['Database Size'] = filesize(info.app.database_size, {round: 0, standard: 'jedec'}) if (info.app.create_status !== 'complete') data['Create Status'] = info.app.create_status - if (info.app.space) data.Space = color.space(info.app.space.name) + if (info.app.space) data.Space = color.space(info.app.space.name as string) if (info.app.space && info.app.internal_routing) data['Internal Routing'] = info.app.internal_routing - if (info.pipeline_coupling) data.Pipeline = `${color.pipeline(info.pipeline_coupling.pipeline.name)} - ${info.pipeline_coupling.stage}` + if (info.pipelineCoupling) data.Pipeline = `${color.pipeline(info.pipelineCoupling.pipeline.name)} - ${info.pipelineCoupling.stage}` data['Auto Cert Mgmt'] = info.app.acm data['Git URL'] = info.app.git_url - data['Web URL'] = color.info(info.app.web_url) - data['Repo Size'] = filesize(info.app.repo_size, {round: 0, standard: 'jedec'}) - if (getGeneration(info.app) !== 'fir') data['Slug Size'] = filesize(info.app.slug_size, {round: 0, standard: 'jedec'}) + data['Web URL'] = color.info(info.app.web_url as string) + data['Repo Size'] = filesize(info.app.repo_size as number, {round: 0, standard: 'jedec'}) + if (getGeneration(info.app) !== 'fir') data['Slug Size'] = filesize(info.app.slug_size as number, {round: 0, standard: 'jedec'}) data.Owner = color.user(info.app.owner.email) data.Region = info.app.region.name data.Dynos = _.countBy(info.dynos, 'type') diff --git a/src/lib/types/app.d.ts b/src/lib/types/app.d.ts index 61b5ca4a22..80db5f8431 100644 --- a/src/lib/types/app.d.ts +++ b/src/lib/types/app.d.ts @@ -2,3 +2,11 @@ import type {App as BaseApp, TeamApp} from '@heroku/types/3.sdk' export type App = BaseApp & Pick export type Apps = App[] + +export type ExtendedApp = App & { + create_status?: string + cron_finished_at?: null | string + cron_next_run?: null | string + database_size?: null | number + extended?: unknown +} diff --git a/test/unit/commands/apps/info.unit.test.ts b/test/unit/commands/apps/info.unit.test.ts index aea2ba13aa..21c2989076 100644 --- a/test/unit/commands/apps/info.unit.test.ts +++ b/test/unit/commands/apps/info.unit.test.ts @@ -1,10 +1,22 @@ import {runCommand} from '@heroku-cli/test-utils' +import {HerokuApiClient} from '@heroku/heroku-fetch' +import {HerokuSDK} from '@heroku/sdk' import {expect} from 'chai' -import nock from 'nock' +import * as sinon from 'sinon' import Info from '../../../../src/commands/apps/info.js' import {unwrap} from '../../../helpers/utils/unwrap.js' +type FakePlatform = { + app: {describe: sinon.SinonStub} +} + +function buildFakePlatform(): FakePlatform { + return { + app: {describe: sinon.stub()}, + } +} + describe('apps:info', function () { const app = { build_stack: {name: 'cedar-14'}, @@ -60,6 +72,8 @@ describe('apps:info', function () { {user: {email: 'foo2@foo.com'}}, ] + const dynos = [{quantity: 2, size: 'Standard-1X', type: 'web'}] + const BASE_INFO = `=== ⬢ myapp Addons: heroku-redis @@ -97,48 +111,44 @@ Dynos: web: 1 Stack: cedar-14 ` - let api: nock.Scope + let fakePlatform: FakePlatform + let apiGet: sinon.SinonStub beforeEach(function () { - api = nock('https://api.heroku.com') + fakePlatform = buildFakePlatform() + sinon.stub(HerokuSDK.prototype, 'platform').get(() => fakePlatform) + apiGet = sinon.stub(HerokuApiClient.prototype, 'get') }) afterEach(function () { - api.done() - nock.cleanAll() + sinon.restore() }) it('shows app info', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/addons') - .reply(200, addons) - .get('/apps/myapp/collaborators') - .reply(200, collaborators) - .get('/apps/myapp/dynos') - .reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['-a', 'myapp']) expect(stdout).to.equal(BASE_INFO) expect(unwrap(stderr)).to.contains('') + expect(fakePlatform.app.describe.calledOnceWithExactly('myapp')).to.equal(true) }) it('shows extended app info', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp?extended=true') - .reply(200, appExtended) - .get('/apps/myapp/addons') - .reply(200, addons) - .get('/apps/myapp/collaborators') - .reply(200, collaborators) - .get('/apps/myapp/dynos') - .reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) + apiGet.withArgs('/apps/myapp?extended=true').resolves({json: async () => appExtended}) const {stderr, stdout} = await runCommand(Info, ['-a', 'myapp', '--extended']) @@ -153,14 +163,14 @@ Stack: cedar-14 }) it('shows empty extended app info when not defined', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp?extended=true').reply(200, appAcm) - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) + apiGet.withArgs('/apps/myapp?extended=true').resolves({json: async () => appAcm}) const {stderr, stdout} = await runCommand(Info, ['-a', 'myapp', '--extended']) @@ -174,13 +184,13 @@ Stack: cedar-14 }) it('shows app info via arg', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['myapp']) @@ -189,14 +199,13 @@ Stack: cedar-14 }) it('shows app info via arg when the app is in a pipeline', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/pipeline-couplings').reply(200, {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}, stage: 'production'}) - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}, stage: 'production'}, + }) const {stderr, stdout} = await runCommand(Info, ['myapp']) @@ -223,13 +232,13 @@ Stack: cedar-14 }) it('shows app info in shell format', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['myapp', '--shell']) @@ -250,14 +259,13 @@ stack=cedar-14 }) it('shows app info in shell format when the app is in pipeline', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/pipeline-couplings').reply(200, {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}, stage: 'production'}) - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}, stage: 'production'}, + }) const {stderr, stdout} = await runCommand(Info, ['myapp', '--shell']) @@ -279,14 +287,14 @@ stack=cedar-14 }) it('shows extended app info in json format', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp?extended=true').reply(200, appExtended) - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) + apiGet.withArgs('/apps/myapp?extended=true').resolves({json: async () => appExtended}) const {stderr, stdout} = await runCommand(Info, ['myapp', '--extended', '--json']) @@ -298,14 +306,13 @@ stack=cedar-14 }) it('shows app info in json format', async function () { - api - .get('/apps/myapp') - .reply(200, appAcm) - api - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) - .get('/apps/myapp/pipeline-couplings').reply(200, {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}}) + fakePlatform.app.describe.resolves({ + addons, + app: appAcm, + collaborators, + dynos, + pipelineCoupling: {app: {id: appAcm.id}, pipeline: {name: 'my-pipeline'}}, + }) const {stderr, stdout} = await runCommand(Info, ['myapp', '--json']) @@ -320,13 +327,13 @@ stack=cedar-14 }) it('shows app info with a stack change', async function () { - api - .get('/apps/myapp') - .reply(200, appStackChange) - api - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: appStackChange, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['myapp']) @@ -351,16 +358,13 @@ Stack: cedar-14 (next build will use heroku-24) }) it('shows fir app info without slug size', async function () { - api - .get('/apps/myapp') - .reply(200, firAppAcm) - api - .get('/apps/myapp/addons') - .reply(200, addons) - .get('/apps/myapp/collaborators') - .reply(200, collaborators) - .get('/apps/myapp/dynos') - .reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: firAppAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['-a', 'myapp']) @@ -369,13 +373,13 @@ Stack: cedar-14 (next build will use heroku-24) }) it('shows fir app info in shell format without slug size', async function () { - api - .get('/apps/myapp') - .reply(200, firAppAcm) - api - .get('/apps/myapp/addons').reply(200, addons) - .get('/apps/myapp/collaborators').reply(200, collaborators) - .get('/apps/myapp/dynos').reply(200, [{quantity: 2, size: 'Standard-1X', type: 'web'}]) + fakePlatform.app.describe.resolves({ + addons, + app: firAppAcm, + collaborators, + dynos, + pipelineCoupling: null, + }) const {stderr, stdout} = await runCommand(Info, ['myapp', '--shell'])