Skip to content

Create GitHub releases for npm packages in gtb publish - #341

Merged
gtbuchanan merged 2 commits into
mainfrom
npm-release-channel
Aug 1, 2026
Merged

Create GitHub releases for npm packages in gtb publish#341
gtbuchanan merged 2 commits into
mainfrom
npm-release-channel

Conversation

@gtbuchanan

Copy link
Copy Markdown
Owner

What

npm packages have shipped untagged since the first release: changeset publish creates its release tags only in the runner's local clone, and changesets/action — whose publish input is what pushes tags and cuts GitHub releases upstream — never runs the publish phase in cd.yml, since publish lives in a separate OIDC release-environment job. Only hk-config@* had tags/releases, because the Pkl channel creates its own.

How

Add an npm channel to gtb publish, mirroring the Pkl channel rather than adopting changesets/action: after changeset publish, create a GitHub release per published npm package (title = tag, notes = the version's CHANGELOG section), which lands the release tag server-side at --target HEAD with no git push. Keeping the orchestration in gtb preserves the design goal that a release is reproducible locally when Actions is down — the channel needs only gh auth (with the workflow scope) — and creating tags via the release API sidesteps ref-count push rulesets.

  • The shared machinery (releaseTag — moved from manifest-sync's pklReleaseTag — changelog-notes extraction, skip-if-release-exists, release creation) is extracted to lib/github-release.ts; the Pkl channel consumes it.
  • Both channels now pass --target HEAD: without it, gh release create targets the remote default branch HEAD, which is only coincidentally correct in CI and silently wrong for a local re-run on an older commit.
  • Tag naming follows changesets' convention by repo shape: <name>@<version> (scoped) for monorepo members, v<version> for single-package repos — matching the backfilled historical tags.
  • Every step stays idempotent (changeset publish skips versions already on the registry; both release channels skip tags that already have a release), so CD runs gtb publish unconditionally and a partial failure resumes on re-run.
  • No workflow changes: CD's publish step already sets GH_TOKEN, proven sufficient by the existing Pkl releases.

Verify

  • Unit tests cover the npm channel (tag shape per repo shape, changelog notes, skip-if-exists, missing-identity error, no-op logging), the updated Pkl channel (--target), and publish ordering (npm publish → npm releases → non-npm channels).
  • Full turbo build is green. One unrelated flake surfaced during the run: eslint-config's agent-skills tests time out under full-pipeline load but pass in isolation.

🤖 Generated with Claude Code

npm releases shipped untagged: changeset publish creates its release
tags only in the runner's local clone, and changesets/action (which
would push them and cut releases) never runs the publish phase in
cd.yml — publish lives in a separate OIDC release-environment job.

Add an npm channel to gtb publish mirroring the Pkl channel: after
changeset publish, create a GitHub release per published package
(title = tag, notes = the version's CHANGELOG section), which lands
the tag server-side at --target HEAD with no git push. This keeps a
manual gtb publish reproducible locally (gh auth only) and sidesteps
ref-count push rules. The shared machinery (releaseTag, changelog
notes, skip-if-exists, release creation) moves to lib/github-release,
and the Pkl channel now also passes --target so local re-runs tag the
commit actually being published.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0c27dc22-948e-44f4-b652-2d56568ab2d7

📥 Commits

Reviewing files that changed from the base of the PR and between 8d9f1ff and 66fb7ae.

📒 Files selected for processing (6)
  • packages/cli/src/lib/github-release.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/test/pkl-release.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • gtbuchanan/tooling (manual)
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: CI / Build
🧰 Additional context used
📓 Path-based instructions (3)
packages/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

packages/**/*.test.{ts,tsx}: Use Vitest native tags for slow tests, such as { tags: ['slow'] } or /** @module-tag slow */; use !slow for fast tests and slow for slow-only runs.
Generate incidental test data with @gtbuchanan/test-utils/builders or @faker-js/faker, capture generated values in locals, and assert against those locals. Hard-code values only when the SUT branches on specific strings; add builders for reusable domain-shaped values.

Files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/test/pkl-release.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When asserting on a CommandResult, use expect(result).toMatchObject({ exitCode: 0 }) rather than asserting directly on result.exitCode, so failure output includes the full result including stderr.

Files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/src/lib/github-release.ts
**/*

📄 CodeRabbit inference engine (AGENTS.md)

On Termux, run pnpm build, pnpm test:slow, and pnpm test:e2e with --concurrency=1 to avoid out-of-memory failures.

Files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/src/lib/github-release.ts
🧠 Learnings (4)
📚 Learning: 2026-06-04T02:26:02.824Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 139
File: packages/cli/test/coverage-codecov-upload.test.ts:2-2
Timestamp: 2026-06-04T02:26:02.824Z
Learning: In tests within **/{test,e2e,__tests__}/**/*.{test,spec}.{ts,tsx,js,jsx}, follow the AGENTS.md faker convention: direct `faker-js/faker` usage is allowed only for one-off primitives when the produced value’s shape is exactly what faker returns (e.g., `faker.git.commitSha()`, `faker.string.uuid()`). Use `gtbuchanan/test-utils/builders` only when there is a domain-shaped value worth centralizing (e.g., scoped package names, semver ranges, GitHub URLs). Do not wrap a native faker generator in a builder for a plain primitive (e.g., a raw commit SHA), since that adds indirection without centralizing any domain shape.

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/test/pkl-release.test.ts
📚 Learning: 2026-06-15T19:58:14.171Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 162
File: packages/cli/test/discovery.test.ts:86-94
Timestamp: 2026-06-15T19:58:14.171Z
Learning: In the gtbuchanan/tooling repo, hand-authored `.pkl` source files are expected to live at the package root (top level), not under subdirectories like `src/`. Accordingly, when reviewing `discoverPackage` logic for `hasPkl`, ensure it only detects top-level `.pkl` files in the package root and does not require (or add) recursive/nested-directory `.pkl` discovery.

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/src/lib/github-release.ts
📚 Learning: 2026-06-15T19:58:27.007Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 162
File: packages/cli/test/typecheck-pkl.test.ts:8-23
Timestamp: 2026-06-15T19:58:27.007Z
Learning: When reviewing tests (e.g., in packages/**/test/**/*.test.ts), if the system-under-test explicitly branches on specific string-literal values (such as sort order, extension filters, or reserved filename exclusions like "PklProject"), require those branching inputs to be hardcoded literals in the test rather than generated via test-data builders. Do not recommend replacing these literals with builder patterns, because it would obscure what behavior is being exercised. Allow builder patterns only for incidental, domain-shaped data where the exact values do not affect the assertion logic (e.g., scoped package names or semver ranges used as opaque inputs).

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/test/pkl-release.test.ts
📚 Learning: 2026-06-18T19:47:41.809Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 186
File: packages/cli/src/lib/sort.ts:0-0
Timestamp: 2026-06-18T19:47:41.809Z
Learning: In the CLI package, keep deterministic string ordering by using the shared `localeComparer` comparator from `packages/cli/src/lib/sort.ts` (which pins locale via `left.localeCompare(right, 'en')`) instead of calling `string.localeCompare` inline. This prevents host/default-locale differences from changing generated outputs (e.g., task order in `turbo.json`). If adding new sorting logic in this package, route it through `localeComparer` to match existing behavior.

Applied to files:

  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/src/lib/github-release.ts
🔇 Additional comments (7)
packages/cli/test/github-release.test.ts (2)

16-16: Capture the generated package name.

Line 16 passes the generated name directly to releaseTag. Store it in a local and use that local in the assertion.

Proposed fix
   it('uses a plain v<version> for a single-package repo', ({ expect }) => {
+    const name = build.scopedPackageName();
     const version = build.semverVersion();

-    expect(releaseTag(build.scopedPackageName(), version, false)).toBe(`v${version}`);
+    expect(releaseTag(name, version, false)).toBe(`v${version}`);
   });

As per coding guidelines, “capture generated values in locals, and assert against those locals.”

Source: Coding guidelines


1-1: LGTM!

Also applies to: 7-10, 14-14

packages/cli/src/lib/github-release.ts (1)

121-159: LGTM!

packages/cli/src/lib/npm-release.ts (1)

2-2: LGTM!

Also applies to: 24-32

packages/cli/test/npm-release.test.ts (1)

3-3: LGTM!

Also applies to: 33-33

packages/cli/src/lib/pkl-release.ts (1)

4-4: LGTM!

Also applies to: 63-78

packages/cli/test/pkl-release.test.ts (1)

3-3: LGTM!

Also applies to: 33-33, 55-55


📝 Walkthrough

Walkthrough

The CLI now creates GitHub releases for published npm packages and reuses the release workflow for Pkl packages. Releases use derived tags, changelog notes, existing-release checks, and the current Git commit as the explicit target.

Changes

GitHub release publishing

Layer / File(s) Summary
Shared GitHub release helpers
packages/cli/src/lib/github-release.ts, packages/cli/test/github-release.test.ts
Added shared helpers for release tags, changelog notes, existing-release checks, HEAD resolution, and gh release create calls.
npm release orchestration
packages/cli/src/lib/npm-release.ts, packages/cli/src/commands/root/publish.ts, packages/cli/test/npm-release.test.ts, packages/cli/test/publish.test.ts, packages/cli/test/helpers.ts, .changeset/npm-release-channel.md, AGENTS.md, .github/workflows/cd.yml
Added npm workspace discovery and release creation. The publish flow runs npm releases after Changesets publishing and before non-npm channels. Documentation and tests describe and verify the flow.
Shared Pkl release handling
packages/cli/src/lib/pkl-release.ts, packages/cli/src/lib/manifest-sync.ts, packages/cli/test/pkl-release.test.ts, AGENTS.md
Moved Pkl release creation to the shared helpers, removed the Pkl-specific release helper and dependency contract, and verifies the resolved commit target.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • gtbuchanan/tooling#185: Both changes update publish orchestration around npm publishing and non-npm release handling.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change, although it is slightly above the preferred 50-character limit.
Description check ✅ Passed The description directly explains the npm release channel, shared GitHub release logic, tagging behavior, idempotency, and verification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.70%. Comparing base (feae032) to head (66fb7ae).

Files with missing lines Patch % Lines
packages/cli/src/commands/root/publish.ts 20.00% 4 Missing ⚠️
packages/cli/src/lib/github-release.ts 94.87% 1 Missing and 1 partial ⚠️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gtbuchanan
gtbuchanan marked this pull request as ready for review July 31, 2026 04:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
AGENTS.md (1)

171-178: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the scoped-package tag description.

Line 174 says that a monorepo tag uses an “unscoped” name. releaseTag preserves scoped names, so @scope/pkg produces @scope/pkg@<version>. Remove “unscoped” to prevent incorrect release-tag guidance.

Proposed change
-  single-package (root) repo, unscoped `<name>@<version>` for a monorepo
+  single-package (root) repo, `<name>@<version>` for a monorepo
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AGENTS.md` around lines 171 - 178, Update the releaseTag description in
AGENTS.md to remove “unscoped” from the monorepo member tag wording, while
preserving the guidance that scoped package names remain intact, such as
`@scope/pkg`@&lt;version&gt;.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli/src/lib/npm-release.ts`:
- Around line 22-49: Extract the shared target-resolution and release iteration
from executePublishNpmReleases and executePublishPkl into a generic
publishPendingReleases helper in github-release.ts. Have the helper accept the
package collection, metadata reader, missing-identity message builder, and
monorepo flag, while supporting optional assets in createGithubRelease; update
both callers to provide channel-specific metadata and preserve their existing
package filtering and error messages.
- Around line 22-49: The executePublishNpmReleases loop currently processes
every pkg.isPublished package, including packages unchanged by changeset
publish. Restrict the loop to packages changed by the publish operation, using
the existing changed-package information available after resolveHeadSha, before
readParsedManifest and releaseExists are called; preserve release creation and
skipping behavior for changed packages.

In `@packages/cli/test/github-release.test.ts`:
- Around line 5-10: Update the releaseTag tests to generate incidental package
names and versions using `@gtbuchanan/test-utils/builders` or faker, store them in
local variables, and construct expected tags from those captured values. Apply
this to both monorepo and single-package cases while preserving the existing
isMonorepo behavior assertions.

In `@packages/cli/test/npm-release.test.ts`:
- Line 32: Replace build.commitSha() with faker.git.commitSha() in the test
setup, removing the unnecessary builder indirection while preserving the raw
commit SHA value.

In `@packages/test-utils/src/builders.ts`:
- Around line 21-23: Remove the commitSha builder from builders.ts and update
its use site in npm-release.test.ts to call faker.git.commitSha() directly.
Remove any now-unused import or export references while preserving the generated
commit SHA behavior.

---

Outside diff comments:
In `@AGENTS.md`:
- Around line 171-178: Update the releaseTag description in AGENTS.md to remove
“unscoped” from the monorepo member tag wording, while preserving the guidance
that scoped package names remain intact, such as `@scope/pkg`@&lt;version&gt;.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 84ea227f-62c5-4e1e-a169-a4d8e10674e5

📥 Commits

Reviewing files that changed from the base of the PR and between feae032 and 8d9f1ff.

📒 Files selected for processing (14)
  • .changeset/npm-release-channel.md
  • .github/workflows/cd.yml
  • AGENTS.md
  • packages/cli/src/commands/root/publish.ts
  • packages/cli/src/lib/github-release.ts
  • packages/cli/src/lib/manifest-sync.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/test/github-release.test.ts
  • packages/cli/test/helpers.ts
  • packages/cli/test/npm-release.test.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/test-utils/src/builders.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • gtbuchanan/tooling (manual)
💤 Files with no reviewable changes (1)
  • packages/cli/src/lib/manifest-sync.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md

📄 CodeRabbit inference engine (AGENTS.md)

Every pull request requires a changeset listing affected published packages and bump types, or an empty changeset for changes that affect no published packages.

Files:

  • .changeset/npm-release-channel.md
**/*

📄 CodeRabbit inference engine (AGENTS.md)

On Termux, run pnpm build, pnpm test:slow, and pnpm test:e2e with --concurrency=1 to avoid out-of-memory failures.

Files:

  • packages/test-utils/src/builders.ts
  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/src/lib/npm-release.ts
  • AGENTS.md
  • packages/cli/test/helpers.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/src/commands/root/publish.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/src/lib/github-release.ts
  • packages/cli/test/npm-release.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When asserting on CommandResult, use expect(result).toMatchObject({ exitCode: 0 }) rather than asserting directly on result.exitCode, so failures include the complete result.

Files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/test/npm-release.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Generate incidental test data with @gtbuchanan/test-utils/builders or @faker-js/faker, capture it in a local, and assert against that local. Hard-code values only when the SUT depends on a specific literal.

Files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/test/npm-release.test.ts
**/.github/workflows/*.yml

📄 CodeRabbit inference engine (AGENTS.md)

**/.github/workflows/*.yml: Reusable workflows must be workflow_call-only; pipeline workflows such as pr.yml and release.yml own repository triggers and call the reusable workflows.
In reusable workflows, reference in-repository composite actions by full repository path, such as gtbuchanan/tooling/.github/actions/<name>@main``, never by a relative ./ path.

Files:

  • .github/workflows/cd.yml
🧠 Learnings (6)
📚 Learning: 2026-06-17T04:54:26.509Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 183
File: .changeset/mise-release-age-excludes.md:4-4
Timestamp: 2026-06-17T04:54:26.509Z
Learning: In the gtbuchanan/tooling repo, `.changeset/*.md` files should intentionally omit the Markdown H1 heading (MD041). Follow the changesets CLI convention: `frontmatter` followed by a blank line and then a plain summary text line. Do not prepend the summary with `# `, because the changesets CLI consumes that summary verbatim as the CHANGELOG entry; adding `# ` would introduce an unwanted stray H1 into the generated changelog. MD041 is not enforced by the repo’s `eslint-markdownlint` config for these files; any MD041 warnings for `.changeset/*.md` come from markdownlint-cli2 defaults rather than the repo’s own lint configuration.

Applied to files:

  • .changeset/npm-release-channel.md
📚 Learning: 2026-06-04T02:26:02.824Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 139
File: packages/cli/test/coverage-codecov-upload.test.ts:2-2
Timestamp: 2026-06-04T02:26:02.824Z
Learning: In tests within **/{test,e2e,__tests__}/**/*.{test,spec}.{ts,tsx,js,jsx}, follow the AGENTS.md faker convention: direct `faker-js/faker` usage is allowed only for one-off primitives when the produced value’s shape is exactly what faker returns (e.g., `faker.git.commitSha()`, `faker.string.uuid()`). Use `gtbuchanan/test-utils/builders` only when there is a domain-shaped value worth centralizing (e.g., scoped package names, semver ranges, GitHub URLs). Do not wrap a native faker generator in a builder for a plain primitive (e.g., a raw commit SHA), since that adds indirection without centralizing any domain shape.

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/test/npm-release.test.ts
📚 Learning: 2026-06-15T19:58:14.171Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 162
File: packages/cli/test/discovery.test.ts:86-94
Timestamp: 2026-06-15T19:58:14.171Z
Learning: In the gtbuchanan/tooling repo, hand-authored `.pkl` source files are expected to live at the package root (top level), not under subdirectories like `src/`. Accordingly, when reviewing `discoverPackage` logic for `hasPkl`, ensure it only detects top-level `.pkl` files in the package root and does not require (or add) recursive/nested-directory `.pkl` discovery.

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/src/lib/npm-release.ts
  • packages/cli/test/helpers.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/src/commands/root/publish.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/src/lib/github-release.ts
  • packages/cli/test/npm-release.test.ts
📚 Learning: 2026-06-15T19:58:27.007Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 162
File: packages/cli/test/typecheck-pkl.test.ts:8-23
Timestamp: 2026-06-15T19:58:27.007Z
Learning: When reviewing tests (e.g., in packages/**/test/**/*.test.ts), if the system-under-test explicitly branches on specific string-literal values (such as sort order, extension filters, or reserved filename exclusions like "PklProject"), require those branching inputs to be hardcoded literals in the test rather than generated via test-data builders. Do not recommend replacing these literals with builder patterns, because it would obscure what behavior is being exercised. Allow builder patterns only for incidental, domain-shaped data where the exact values do not affect the assertion logic (e.g., scoped package names or semver ranges used as opaque inputs).

Applied to files:

  • packages/cli/test/github-release.test.ts
  • packages/cli/test/publish.test.ts
  • packages/cli/test/pkl-release.test.ts
  • packages/cli/test/npm-release.test.ts
📚 Learning: 2026-06-18T19:47:41.809Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 186
File: packages/cli/src/lib/sort.ts:0-0
Timestamp: 2026-06-18T19:47:41.809Z
Learning: In the CLI package, keep deterministic string ordering by using the shared `localeComparer` comparator from `packages/cli/src/lib/sort.ts` (which pins locale via `left.localeCompare(right, 'en')`) instead of calling `string.localeCompare` inline. This prevents host/default-locale differences from changing generated outputs (e.g., task order in `turbo.json`). If adding new sorting logic in this package, route it through `localeComparer` to match existing behavior.

Applied to files:

  • packages/cli/src/lib/npm-release.ts
  • packages/cli/src/commands/root/publish.ts
  • packages/cli/src/lib/pkl-release.ts
  • packages/cli/src/lib/github-release.ts
📚 Learning: 2026-06-04T18:38:29.949Z
Learnt from: gtbuchanan
Repo: gtbuchanan/tooling PR: 141
File: .github/workflows/changeset-check.yml:15-15
Timestamp: 2026-06-04T18:38:29.949Z
Learning: In this repository (gtbuchanan/tooling), GitHub Actions used in workflows are intentionally referenced by moving tags/branches (e.g., `actions/checkoutv*`, `actions/upload-artifactv*`, `actions/cachev*`, `actions/create-github-app-tokenv*`, and other third-party actions like `rharkor/caching-for-turbov*`), and should never be pinned to a commit SHA. There is no repo-wide SHA-pinning policy; therefore, do not flag these as “unpinned” or suggest SHA-pinning (zizmor’s default `unpinned-uses` rule does not apply here). For in-repo composite actions referenced by reusable workflows, the deliberate documented convention is to use `main` (e.g., `gtbuchanan/tooling/.github/actions/mise-setupmain`), matching how consumers reference the reusable workflows themselves (`.../.github/workflows/<name>main`). Do not change `main` references to SHAs to avoid hash bumps and the self-reference paradox.

Applied to files:

  • .github/workflows/cd.yml
🪛 markdownlint-cli2 (0.23.1)
.changeset/npm-release-channel.md

[warning] 5-5: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🔇 Additional comments (14)
packages/cli/src/lib/github-release.ts (1)

1-119: LGTM!

packages/cli/test/github-release.test.ts (1)

15-30: LGTM!

.changeset/npm-release-channel.md (1)

1-10: LGTM!

AGENTS.md (1)

324-335: LGTM!

.github/workflows/cd.yml (1)

29-34: LGTM!

packages/cli/src/lib/pkl-release.ts (1)

4-10: LGTM!

Also applies to: 51-53, 55-61, 70-70, 83-87

packages/cli/test/pkl-release.test.ts (1)

5-7: LGTM!

Also applies to: 21-54, 82-93

packages/cli/src/commands/root/publish.ts (3)

3-13: LGTM!


15-34: LGTM!

The sequencing matches the documented contract, and packages/cli/test/publish.test.ts verifies npm publish completes, then releaseNpm settles, then publishNonNpm runs.


36-56: LGTM!

Constructing deps once and sharing it between releaseNpm and publishNonNpm avoids duplicated dependency wiring.

packages/cli/test/helpers.ts (1)

107-137: LGTM!

createNpmWorkspace mirrors the existing createPklWorkspace pattern and returns the metadata needed by the npm-release tests.

packages/cli/test/npm-release.test.ts (2)

26-56: LGTM!

stubDeps cleanly separates the git sha capture from the gh release-exists path, and each test captures generated values in a local before asserting against them.


58-138: LGTM!

The test cases cover tag creation, single-package tagging, CHANGELOG-derived notes, idempotent skipping, missing-identity errors, and the no-published-packages no-op.

packages/cli/test/publish.test.ts (1)

10-45: LGTM!

The updated ordering assertions correctly verify that releaseNpm runs after npm publish settles and before publishNonNpm starts, matching executePublish in packages/cli/src/commands/root/publish.ts.

Comment thread packages/cli/src/lib/npm-release.ts
Comment thread packages/cli/test/github-release.test.ts Outdated
Comment thread packages/cli/test/npm-release.test.ts Outdated
Comment thread packages/test-utils/src/builders.ts Outdated
CodeRabbit review feedback on the npm release channel:

- Move the per-package release iteration (tag, skip-if-exists,
  changelog notes, HEAD targeting) into a publishReleases helper in
  lib/github-release; the npm and Pkl channels now just resolve each
  package's identity and assets, so the channels can't drift apart.
  Identity validation moves ahead of any side effects as a result.
- Generate the releaseTag test inputs via builders instead of
  hard-coded literals, per the test-data convention.
- Drop the commitSha builder: it wrapped faker.git.commitSha() without
  adding any domain shape; call faker directly at the use sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gtbuchanan
gtbuchanan merged commit 78acd0d into main Aug 1, 2026
11 checks passed
@gtbuchanan
gtbuchanan deleted the npm-release-channel branch August 1, 2026 02:36
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