Repository navigation
fix(core): recognise ReleaseKitError across bundled copies of core - #651
Merged
Merged
Conversation
Contributor
|
Standing release PR: #614 · v0.42.0 queued (4 packages) · open 81d 21h 34m · ✅ ready to merge Release Preview — v0.41.2 → v0.41.3
These changes will be added to the release PR (#614) when merged: Changelogmonorepo 0.41.2 → 0.41.3
After merge — predicted release
Updated automatically by ReleaseKit |
13 of 20 tasks
@releasekit/release bundles version, notes and publish from their own builds, and each of those inlines its own copy of core, so the release bundle holds four distinct ReleaseKitError classes. isReleaseKitError was a bare instanceof, so an error thrown inside the version, notes or publish stage of `release` or `standing-pr` failed the check against release's copy: its envelope code degraded to GENERAL_ERROR and the exit code to 1, whatever the actual failure. Stamp every ReleaseKitError with a non-enumerable Symbol.for brand and accept it in isReleaseKitError alongside instanceof. Symbol.for is process-wide, so every copy stamps and recognises the same key. Verified with the built CLI: `release --json` whose push fails now reports PIPELINE_STAGE_ERROR instead of GENERAL_ERROR. The release-side spec uses the built @releasekit/publish (its own inlined core) and fails without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
Review follow-ups on #651: - Braces on the instanceof early return; brand comment reworded. - The cross-bundle spec now runs real errors from the built version, notes and publish packages through `release --json`, checking the envelope code and exit code per stage. The premise check moves into each case as an assertion message instead of a standalone test and comment. - Drops a core test comment that repeated the brand's JSDoc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
goosewobbler
force-pushed
the
ccr-1a06dbc5-dgp2t6
branch
from
October 22, 2026 15:42
4f176e0 to
b96f47b
Compare
7 of 19 tasks
Owner
Author
|
@greptileai review plz |
Merged
goosewobbler
pushed a commit
that referenced
this pull request
Oct 22, 2026
#651 is merged, so an error raised inside their version, notes or publish stage keeps its code instead of degrading to GENERAL_ERROR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e
7 of 19 tasks
goosewobbler
added a commit
that referenced
this pull request
Oct 23, 2026
…pipe (#616) * feat(cli): envelope the version and publish commands, unwrap on the pipe Completes the envelope scope of #544. `version --json` and `publish --json` were the two commands still printing a bare payload; both now emit the uniform envelope and accept `--output <path>` for parity with the orchestration commands. `notes` has no JSON output, so the surface is now uniform. The coupling this was deferred for is the pipe: `version | notes | publish` carries JSON between processes, so enveloping the producer means teaching the consumers to unwrap on input. Both do, via `unwrapEnvelope` in core — called once inside `parseVersionOutput` and `parseInput` so the string, file, and stdin entries are all covered. A bare `VersionOutput` still passes through, so a hand-assembled file or an older releasekit keeps working. Piping a failed stage forward now fails with that stage's message rather than a schema complaint about missing fields, which pointed at the wrong thing. Two things fell out of doing this honestly: `errorEnvelope` takes optional `data` and `changed`. Enveloping publish's failure path would otherwise have dropped `partialOutput` — what already landed before the failure — leaving the enveloped output less informative than the prose it replaced, and reporting `changed: false` for a partial publish that did change state. Publishes are idempotent and retried, so that is exactly what the retry reads. `failedStage` has no envelope field, so it is named in the error message rather than dropped. Readers that ignore `data` when `status` is `error` are unaffected, so schemaVersion is unchanged. The publish-changed predicate moved to `@releasekit/publish` as `publishDidChange`. The publish CLI and `standing-pr publish` must answer `changed` the same way, and the logic is subtle enough (tags, `git.pushed`, and already-existing GitHub releases all report success without anything having published) that two copies would drift. It is also now tolerant of absent registry arrays: it runs on the partial output of a failed pipeline, where throwing would replace the publish error the caller needs. Note `version` now exits with the mapped code from the documented table rather than always 1 — in practice only `GIT_ERROR`, which becomes 7. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27 * fix(cli): complete the exit-code table and validate envelopes on unwrap Both from Greptile review of #616; both were real. **Exit classification.** The code → exit-code map only held the nine family names, but packages throw specific codes — NOT_GIT_REPO, TAG_ALREADY_EXISTS, PACKAGES_NOT_FOUND, NPM_AUTH_ERROR and the rest — so almost every real failure fell through to exit 1 while docs/cli.md advertised a mapping. Automation could not tell a git failure from a config one. Every code the version, notes, and publish packages throw is now mapped to its family, with a test that fails when a new code isn't, since the degradation is silent. The family assignment is a judgement call in two places worth a second opinion: workspace/package-discovery failures (PACKAGES_NOT_FOUND, WORKSPACE_ERROR) are treated as config errors on the reasoning that the `packages` globs matched nothing, and manifest-parse failures (CARGO_TOML_ERROR, PUBSPEC_YAML_ERROR) likewise. **Envelope validation.** `unwrapEnvelope` accepted anything with the three keys, so an envelope from a newer producer, an unrecognised status, or a success envelope with a null payload all flowed through — the last surfacing in `notes` as a null-property error about `changelogs` rather than an input error. It now rejects each with a message naming the actual cause. This is what schemaVersion was for; nothing had been reading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FADRmu9uhtCk3StMoqwV27 * fix(cli): report publish commits as changes and exit publish by code family Review follow-ups for the envelope work: - `publish --json` reported `changed: false` after creating the release commit and tags whenever no registry published (GitHub-release-only, `--skip-publish`). `publishDidChange` now also counts `git.committed`, which is only set for a commit this run created; `standing-pr publish` never commits (`skipGitCommit`), so its `changed` is unaffected. - `publish` exited 9/1 regardless of the error code, contradicting the code-to-exit table. It now exits via `exitCodeForError`, like `version`: config failures exit 2, input failures 3, pipeline failures still 9. - `version --output` with a relative path landed inside `--project-dir` (the command chdirs first). Resolve it against the invocation directory up front, as `release --output` does. - docs: the `version | notes | publish` pipe never worked (`notes` does not pass the VersionOutput on); document chaining through a file. Correct the `changed`/retry wording and the exit-code example. Update the version and publish READMEs for the envelope and `--output`. - release: reuse core's `writeEnvelope` instead of a private copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * fix(cli): correct the chaining, retry and exit-code docs; move the --output test into CI Second-review follow-ups: - docs: the file-based `version` -> `notes` -> `publish` chain left the changelog out of the release commit (publish commits only the version files plus what is staged). Document staging it; verified end to end. - docs: a re-run of the standalone `publish` in the same tree stops at the git-commit stage, so drop the blanket "a retry is safe". - docs: the exit-code examples named codes `version` never throws (TAG_ALREADY_EXISTS / GIT_PROCESS_ERROR are unreachable). Use codes `publish` actually emits: input -> 3, config -> 2, pipeline -> 9. - test: the `--output` integration test lived in version's integration suite, which CI never runs. Replace it with a unit test in test/unit/cli.spec.ts (fails against the pre-fix command). - publishDidChange docstring: `committed` is also set in a dry run; the dry-run guard is what keeps that out of `changed`. - version README: the flag table listed --strict-reachable, --verbose and --quiet, which the command doesn't have, and omitted several it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs(cli): scope the exit-code contract to the commands it holds for Third-review follow-ups: - The specific-code / family-exit contract holds for version, notes and publish, but `release` and `standing-pr` report errors raised inside their bundled version/notes/publish stages as GENERAL_ERROR (exit 1): the release bundle carries several copies of core's ReleaseKitError, so the instanceof check misses errors from the other copies. Say so rather than claim otherwise; the cause predates this PR and is fixed separately. - `data` on error: describe the actual exception (a publish pipeline failure carries partial output even when nothing landed). - core README: VersionOutput is now the envelope's `data`. - Restore unit tests for releaseDidPublish, the standing-pr `changed` wiring, dropped when the predicate moved to @releasekit/publish. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * fix(cli): don't crash the error path on an unwritable --output; scope the notes exit claim Fourth-review follow-ups: - docs/cli.md claimed the family exit-code contract for `notes`, but notes exits 1 for config errors and most non-parse failures; only its input-parse failures exit 3. Say exactly that. - An unwritable `--output` path made the result write throw, then the error path threw again writing its envelope to the same file, crashing with a stack and no envelope. writeEnvelope still throws for a result write (the command must fail), but notes a failed error-envelope write on stderr instead of throwing out of the error handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs(cli): describe notes' parse check precisely; name a lost --output result Fifth-review nits: - notes only checks that its input has a `changelogs` array, so "a schema-invalid object" overstated the exit-3 case; say what it checks. - When the result write to --output fails, throw "Could not write the result to <path>: ..." instead of the bare ENOENT, so stderr says the command's work happened but its result never reached the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * test: drop low-value review additions Remove the releaseDidPublish unit tests (a one-line wrapper over publishDidChange, which publish's own tests cover) and rethrow the original error when a result can't be written to --output instead of re-wrapping it — the ENOENT already names the path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * test: drop the duplicate publish exit test and the notes exit clause INPUT→3 runs the same exit-code path as CONFIG→2, which stays as the guard against hard-coded exits. The cli.md notes clause is trimmed to the commands the table holds for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs(cli): release and standing-pr now report a stage's own error code #651 is merged, so an error raised inside their version, notes or publish stage keeps its code instead of degrading to GENERAL_ERROR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * refactor(cli): share one error-envelope writer and trim review leftovers - core: add writeErrorEnvelope next to writeEnvelope; version, publish and release's emitError now call it instead of each building the same single-error envelope. - core: isEnvelope is no longer exported — only unwrapEnvelope needs it, and the version test helper now reads output through unwrapEnvelope, the same boundary notes and publish cross. - publishDidChange: drop the `?.` / `?? []` guards. Every PublishOutput it sees comes from runPipeline's ctx.output (the only PipelineError is built from it), whose registry arrays are always initialised. - Drop comments that repeat the unwrapEnvelope JSDoc or another comment, the old-behaviour note on publish's exit, and the "pipe's contract is unchanged" claim (older consumers reject the envelope). - Braces on the one-line ifs and loop this PR added; give publishDidChange its own export section so runPipeline keeps its header. - Drop the "older producer" unwrap test: it used the current schemaVersion, so it duplicated the success-envelope case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * test(cli): cover the version and publish failure paths through the commands - version --json: an engine error reaches stdout as an error envelope with its own code and exits by its family (INVALID_CONFIG -> 2); --stable with --prerelease prints an INPUT_ERROR envelope and exits 3. - publish --output: a PipelineError writes its envelope, partial output in `data`, to the file and nothing to stdout. - publish --input holding a failed upstream envelope runs the real parseInput and exits 3 with INPUT_PARSE_ERROR. It replaces the "registry failure" test, which rejected runPipeline with a bare NPM_PUBLISH_ERROR that runPipeline never throws (it wraps every stage error in a PipelineError). - Exit-code coverage moves from a hardcoded list in core to each package's own error enums (PublishErrorCode; VersionErrorCode, GitErrorCode and StrictReachableError), so a new unmapped code now fails a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs: point --json stability at schemaVersion; keep the version README to this PR's flags - troubleshooting: "the JSON schema is stable across minor versions" no longer holds — this release changes the version/publish --json shape. Say what is versioned instead: the envelope's schemaVersion. - version README: revert the flag-table rewrite that rode along (short aliases, --stable, --allow-first-bump, --sync, --config, --include-prerequisites, and dropping --strict-reachable / --verbose / --quiet). Keep only the --json description and the new --output row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * style: add braces to every single-statement body Same sweep as #679, run on this branch so merging main resolves cleanly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * fix(cli): suppress per line instead of exempting cli.ts; test --output alone The --json print takes a one-line noConsole suppression and the unwritable error-envelope note goes through the logger, so core/src/cli.ts no longer needs a file-wide exemption. A version --output test now fails if --output alone stops collecting the result. Inline releaseDidPublish, drop a guard writeEnvelope already applies, and trim two comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * test(version): write --output through the real JSON writer Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs(core): trim the envelope doc comments; drop a stale e2e helper note Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e * docs: tighten the envelope docs and comments Fix the failed-upstream wording in docs/cli.md (it's an input error that names the upstream code), say plainly that a partial publish can't be resumed yet, and cut or shorten the comments that restated code or docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: link to the chaining section from architecture, simplify envelope comments Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
@releasekit/releasebundles version, notes and publish from their own builds, and each of those inlines its own copy of@releasekit/core— sorelease/distholds four distinctReleaseKitErrorclasses.ReleaseKitError.isReleaseKitErrorwas a bareinstanceof, so an error thrown inside the version, notes or publish stage ofreleaseorstanding-prfailed the check against release's copy.toEnvelopeErrorthen reported it asGENERAL_ERRORandexitCodeForErrorexited1, whatever actually failed — the JSON envelope's error codes were generic for exactly the commands CI and agents use most.Every
ReleaseKitErroris now stamped with a non-enumerableSymbol.for('@releasekit/core:ReleaseKitError')brand, andisReleaseKitErroraccepts it alongsideinstanceof.Symbol.forkeys a process-wide registry, so every copy stamps and recognises the same key. The brand is non-enumerable, so it never appears in spreads, JSON, ortoEqualcomparisons.Behaviour change:
release --json/standing-pr --jsonnow report the specific code of a stage failure (e.g.PIPELINE_STAGE_ERROR,INVALID_CONFIG) instead ofGENERAL_ERROR; those codes' categories (pipeline-stage,invalid-config, …) follow, and so doesretryable— an escaped notesLLMErrorcan now reportretryable: truewhere it was forced tofalse. Exit codes follow the code → exit table. Onmainthat table doesn't yet list the specific publish/version codes, so those still exit1until #616's expanded table lands (then e.g.PIPELINE_STAGE_ERROR→9); codes that already name a family change now — a stage'sCONFIG_ERROR→2,GIT_ERROR→7, and notes' input-parse/template/LLM errors →3/4/5. The repo's workflows and the Action only check for non-zero.Interaction with #616: #616 documents the current behaviour in
docs/cli.md("releaseandstanding-prcurrently report an error raised inside their version, notes, or publish stage asGENERAL_ERROR"). Whichever of the two merges second should drop that sentence.The roadmap housekeeping that used to ride on this branch is now #665.
Related Issue(s)
Part of #544 (structured error codes). Found while reviewing #616.
Type of Change
feat— New featurefix— Bug fixdocs— Documentation changerefactor— Code refactoringtest— Adding or updating testschore— Maintenance, dependencies, toolingScope
@releasekit/version@releasekit/notes@releasekit/publish@releasekit/config@releasekit/coreChecklist
pnpm lint)pnpm test) — core 197, release 847pnpm typecheck)Testing Notes
packages/release/test/unit/commands/cross-bundle-errors.spec.tsthrows a real error from each built stage package (versionGIT_ERROR, notesLLM_ERRORwithretryable: true, publishCONFIG_ERROR) throughrelease --json, and checks the envelope's code,retryableand message, and the exit code. Each case first asserts the error isn't an instance of release'sReleaseKitError, so it fails loudly if a stage build stops carrying its own copy of core. With the fix reverted, all three cases fail withGENERAL_ERROR.vi.resetModules) stands in for another copy and fails without the fix; the brand stays non-enumerable.release --jsonin a scratch repo: a failing push now reportsPIPELINE_STAGE_ERROR("Failed to push to remote: …") and a version-stage config errorINVALID_CONFIG, where both previously reportedGENERAL_ERROR.🤖 Generated with Claude Code
https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e


Confidence Score: 5/5
The PR appears safe to merge; no actionable issues were found.Summary
ReleaseKitErrornow uses a shared, non-enumerable symbol to recognize errors from separately bundled copies of core.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Stage throws an error] --> B[Shared symbol identifies the error] B --> C[JSON keeps its specific code and retry flag] B --> D[Exit code follows the existing table]Reviews (2) · Last reviewed commit: "test(release): drive cross-bundle errors..." · Reviewed by Greptile