From 244167436dda0be1c0a5ac108ee67c5178402d0f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 16:27:17 +0000 Subject: [PATCH 1/2] fix(core): recognise ReleaseKitError across bundled copies of core @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 Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e --- packages/core/src/errors.ts | 19 ++++++++++++- packages/core/test/unit/errors.spec.ts | 21 ++++++++++++++ .../unit/commands/cross-bundle-errors.spec.ts | 28 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 packages/release/test/unit/commands/cross-bundle-errors.spec.ts diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index b74a92cc..c7e5f009 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -1,5 +1,14 @@ import { log } from './logger.js'; +/** + * Brand recognised across copies of this module. `@releasekit/release` bundles version, notes and + * publish from their own builds, and each of those inlines its own copy of core — so one process can + * hold several distinct `ReleaseKitError` classes, and `instanceof` against any one of them misses an + * error thrown through another (its code then degrades to GENERAL_ERROR). `Symbol.for` keys a + * process-wide registry, so every copy stamps and recognises the same brand. + */ +const RELEASEKIT_ERROR_BRAND = Symbol.for('@releasekit/core:ReleaseKitError'); + /** * Base error class that all releasekit errors should extend. * Provides consistent error handling with codes and suggestions. @@ -11,6 +20,8 @@ export abstract class ReleaseKitError extends Error { constructor(message: string) { super(message); this.name = this.constructor.name; + // Non-enumerable, so it never shows up in a spread, JSON output, or a test's toEqual. + Object.defineProperty(this, RELEASEKIT_ERROR_BRAND, { value: true }); } logError(): void { @@ -24,8 +35,14 @@ export abstract class ReleaseKitError extends Error { } } + /** True for an error from any copy of core (see {@link RELEASEKIT_ERROR_BRAND}), not just this one. */ static isReleaseKitError(error: unknown): error is ReleaseKitError { - return error instanceof ReleaseKitError; + if (error instanceof ReleaseKitError) return true; + return ( + typeof error === 'object' && + error !== null && + (error as { [RELEASEKIT_ERROR_BRAND]?: unknown })[RELEASEKIT_ERROR_BRAND] === true + ); } } diff --git a/packages/core/test/unit/errors.spec.ts b/packages/core/test/unit/errors.spec.ts index c2e89a23..7d15bdce 100644 --- a/packages/core/test/unit/errors.spec.ts +++ b/packages/core/test/unit/errors.spec.ts @@ -93,6 +93,27 @@ describe('ReleaseKitError', () => { expect(ReleaseKitError.isReleaseKitError('error')).toBe(false); expect(ReleaseKitError.isReleaseKitError({})).toBe(false); }); + + it('should recognise an error from another copy of this module', async () => { + // A bundle that inlines core more than once holds several ReleaseKitError classes; a fresh + // module instance stands in for one of the other copies. + vi.resetModules(); + const other = await import('../../src/errors.js'); + class OtherCopyError extends other.ReleaseKitError { + readonly code = 'CONFIG_ERROR'; + readonly suggestions: string[] = []; + } + const error = new OtherCopyError('from the other copy'); + + expect(other.ReleaseKitError).not.toBe(ReleaseKitError); + expect(error instanceof ReleaseKitError).toBe(false); + expect(ReleaseKitError.isReleaseKitError(error)).toBe(true); + }); + + it('should keep the brand out of enumerable properties', () => { + const error = new TestError('Test'); + expect(Object.getOwnPropertySymbols({ ...error })).toEqual([]); + }); }); }); diff --git a/packages/release/test/unit/commands/cross-bundle-errors.spec.ts b/packages/release/test/unit/commands/cross-bundle-errors.spec.ts new file mode 100644 index 00000000..18952f2f --- /dev/null +++ b/packages/release/test/unit/commands/cross-bundle-errors.spec.ts @@ -0,0 +1,28 @@ +import { EXIT_CODES, exitCodeForError, ReleaseKitError, toEnvelopeError } from '@releasekit/core'; +import { BasePublishError, PipelineError, PublishErrorCode } from '@releasekit/publish'; +import { describe, expect, it } from 'vitest'; + +// @releasekit/publish's build inlines its own copy of core, so its errors extend a different +// ReleaseKitError class than the one this package's error handling imports — the situation the +// release bundle is in for every version/notes/publish error it reports. +describe('errors raised through another copy of core', () => { + it('should come from a distinct ReleaseKitError class (the premise of these tests)', () => { + expect(new PipelineError('boom', 'npm', undefined as never) instanceof ReleaseKitError).toBe(false); + }); + + it('should still be recognised as releasekit errors', () => { + expect(ReleaseKitError.isReleaseKitError(new PipelineError('boom', 'npm', undefined as never))).toBe(true); + }); + + it('should keep their specific code in the envelope instead of degrading to GENERAL_ERROR', () => { + const error = toEnvelopeError(new PipelineError('registry rejected', 'npm', undefined as never)); + expect(error.code).toBe(PublishErrorCode.PIPELINE_STAGE_ERROR); + expect(error.message).toBe('registry rejected'); + }); + + it("should exit with their code's family instead of the generic exit code", () => { + expect(exitCodeForError(new BasePublishError('bad config', PublishErrorCode.CONFIG_ERROR))).toBe( + EXIT_CODES.CONFIG_ERROR, + ); + }); +}); From b96f47bb2a7a07bff48e4c7fd9c58eef3c6c3a3e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 15:42:26 +0000 Subject: [PATCH 2/2] test(release): drive cross-bundle errors through the release command 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 Claude-Session: https://claude.ai/code/session_01HQN3v9Z4f1mu3xK4Z7K44e --- packages/core/src/errors.ts | 6 +- packages/core/test/unit/errors.spec.ts | 2 - .../unit/commands/cross-bundle-errors.spec.ts | 75 +++++++++++++------ 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index c7e5f009..9cfc65b7 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -20,7 +20,7 @@ export abstract class ReleaseKitError extends Error { constructor(message: string) { super(message); this.name = this.constructor.name; - // Non-enumerable, so it never shows up in a spread, JSON output, or a test's toEqual. + // Non-enumerable, so the brand never shows up in a spread, JSON output, or a test's toEqual. Object.defineProperty(this, RELEASEKIT_ERROR_BRAND, { value: true }); } @@ -37,7 +37,9 @@ export abstract class ReleaseKitError extends Error { /** True for an error from any copy of core (see {@link RELEASEKIT_ERROR_BRAND}), not just this one. */ static isReleaseKitError(error: unknown): error is ReleaseKitError { - if (error instanceof ReleaseKitError) return true; + if (error instanceof ReleaseKitError) { + return true; + } return ( typeof error === 'object' && error !== null && diff --git a/packages/core/test/unit/errors.spec.ts b/packages/core/test/unit/errors.spec.ts index 7d15bdce..c41d5074 100644 --- a/packages/core/test/unit/errors.spec.ts +++ b/packages/core/test/unit/errors.spec.ts @@ -95,8 +95,6 @@ describe('ReleaseKitError', () => { }); it('should recognise an error from another copy of this module', async () => { - // A bundle that inlines core more than once holds several ReleaseKitError classes; a fresh - // module instance stands in for one of the other copies. vi.resetModules(); const other = await import('../../src/errors.js'); class OtherCopyError extends other.ReleaseKitError { diff --git a/packages/release/test/unit/commands/cross-bundle-errors.spec.ts b/packages/release/test/unit/commands/cross-bundle-errors.spec.ts index 18952f2f..cff03f3f 100644 --- a/packages/release/test/unit/commands/cross-bundle-errors.spec.ts +++ b/packages/release/test/unit/commands/cross-bundle-errors.spec.ts @@ -1,28 +1,61 @@ -import { EXIT_CODES, exitCodeForError, ReleaseKitError, toEnvelopeError } from '@releasekit/core'; -import { BasePublishError, PipelineError, PublishErrorCode } from '@releasekit/publish'; -import { describe, expect, it } from 'vitest'; - -// @releasekit/publish's build inlines its own copy of core, so its errors extend a different -// ReleaseKitError class than the one this package's error handling imports — the situation the -// release bundle is in for every version/notes/publish error it reports. -describe('errors raised through another copy of core', () => { - it('should come from a distinct ReleaseKitError class (the premise of these tests)', () => { - expect(new PipelineError('boom', 'npm', undefined as never) instanceof ReleaseKitError).toBe(false); - }); +import { EXIT_CODES, ReleaseKitError } from '@releasekit/core'; +import { LLMError } from '@releasekit/notes'; +import { BasePublishError, PublishErrorCode } from '@releasekit/publish'; +import { BaseVersionError } from '@releasekit/version'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../src/release.js'); + +import { createReleaseCommand } from '../../../src/commands/release-command.js'; +import { runRelease } from '../../../src/release.js'; - it('should still be recognised as releasekit errors', () => { - expect(ReleaseKitError.isReleaseKitError(new PipelineError('boom', 'npm', undefined as never))).toBe(true); +describe('release --json when a stage fails', () => { + let exitSpy: ReturnType; + let logSpy: ReturnType; + + beforeEach(() => { + exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'error').mockImplementation(() => undefined); }); - it('should keep their specific code in the envelope instead of degrading to GENERAL_ERROR', () => { - const error = toEnvelopeError(new PipelineError('registry rejected', 'npm', undefined as never)); - expect(error.code).toBe(PublishErrorCode.PIPELINE_STAGE_ERROR); - expect(error.message).toBe('registry rejected'); + afterEach(() => { + vi.restoreAllMocks(); }); - it("should exit with their code's family instead of the generic exit code", () => { - expect(exitCodeForError(new BasePublishError('bad config', PublishErrorCode.CONFIG_ERROR))).toBe( - EXIT_CODES.CONFIG_ERROR, - ); + it.each([ + { + stage: 'version', + error: new BaseVersionError('git tag failed', 'GIT_ERROR'), + code: 'GIT_ERROR', + retryable: false, + exit: EXIT_CODES.GIT_ERROR, + }, + { + stage: 'notes', + error: new LLMError('provider timed out', { retryable: true }), + code: 'LLM_ERROR', + retryable: true, + exit: EXIT_CODES.LLM_ERROR, + }, + { + stage: 'publish', + error: new BasePublishError('npm auth is not configured', PublishErrorCode.CONFIG_ERROR), + code: 'CONFIG_ERROR', + retryable: false, + exit: EXIT_CODES.CONFIG_ERROR, + }, + ])('should report $code from the $stage stage and exit $exit', async ({ stage, error, code, retryable, exit }) => { + expect( + error, + `the ${stage} build shares release's ReleaseKitError, so this case no longer tests a separate copy of core`, + ).not.toBeInstanceOf(ReleaseKitError); + vi.mocked(runRelease).mockRejectedValue(error); + + await createReleaseCommand().parseAsync(['node', 'release', '--json']); + + const envelope = JSON.parse(logSpy.mock.calls[0]?.[0] as string); + expect(envelope.errors).toMatchObject([{ code, retryable, message: error.message }]); + expect(exitSpy).toHaveBeenCalledWith(exit); }); });