From 45915643aafaffd8223b178b8a4c44576cd072cb Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Wed, 12 Aug 2026 20:21:45 +0400 Subject: [PATCH 1/6] chore(release): add protected npm next prerelease channel Separate the stable and prerelease release channels. - ci.yml validates pushes to next as well as main - release.yml is stable-only; SemVer prerelease tags skip it cleanly - new classify-release-tag.mjs enforces approved prerelease forms (-beta.N, -rc.N, -next.N) and stable/prerelease channel expectations - new publish-next.yml publishes prereleases from exact tagged commits under the protected npm-next environment using npm Trusted Publishing with provenance, proving next ancestry, tag/version/changelog match, and preservation of the latest dist-tag - new verify-next-release-state.mjs backs the event, ancestry, unpublished, and post-publish dist-tag assertions - release-pipeline.test.ts covers classification, guards and workflow policy - release and contribution docs describe both channels Refs #654. --- .github/scripts/classify-release-tag.mjs | 147 ++++++++ .github/scripts/verify-next-release-state.mjs | 143 +++++++ .github/workflows/ci.yml | 1 + .github/workflows/publish-next.yml | 350 ++++++++++++++++++ .github/workflows/release.yml | 8 + CONTRIBUTING.md | 13 + docs/release.md | 124 ++++--- tests/unit/release-pipeline.test.ts | 350 ++++++++++++++++++ 8 files changed, 1086 insertions(+), 50 deletions(-) create mode 100644 .github/scripts/classify-release-tag.mjs create mode 100644 .github/scripts/verify-next-release-state.mjs create mode 100644 .github/workflows/publish-next.yml create mode 100644 tests/unit/release-pipeline.test.ts diff --git a/.github/scripts/classify-release-tag.mjs b/.github/scripts/classify-release-tag.mjs new file mode 100644 index 00000000..1a225b3d --- /dev/null +++ b/.github/scripts/classify-release-tag.mjs @@ -0,0 +1,147 @@ +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +const NUMERIC_IDENTIFIER = '(?:0|[1-9][0-9]*)' +const CORE_VERSION = `(${NUMERIC_IDENTIFIER})\\.(${NUMERIC_IDENTIFIER})\\.(${NUMERIC_IDENTIFIER})` +const STABLE_TAG_PATTERN = new RegExp(`^v${CORE_VERSION}$`) +const PRERELEASE_TAG_PATTERN = new RegExp(`^v${CORE_VERSION}-(beta|rc|next)\\.(${NUMERIC_IDENTIFIER})$`) +const VERSIONED_PRERELEASE_PREFIX_PATTERN = new RegExp(`^v${CORE_VERSION}-`) + +function fail(message) { + throw new Error(message) +} + +export function classifyReleaseTag(tag) { + if (typeof tag !== 'string' || tag.length === 0) { + fail('A release tag is required (for example, v1.2.3 or v1.2.3-beta.1)') + } + + if (STABLE_TAG_PATTERN.test(tag)) { + return { + channel: 'stable', + tag, + version: tag.slice(1), + } + } + + const prereleaseMatch = tag.match(PRERELEASE_TAG_PATTERN) + if (prereleaseMatch) { + return { + channel: 'prerelease', + prerelease: `${prereleaseMatch[4]}.${prereleaseMatch[5]}`, + tag, + version: tag.slice(1), + } + } + + if (VERSIONED_PRERELEASE_PREFIX_PATTERN.test(tag)) { + fail(`Invalid prerelease tag "${tag}": approved forms are -beta.N, -rc.N, or -next.N with a non-negative integer N`) + } + + fail(`Malformed release tag "${tag}": expected vMAJOR.MINOR.PATCH or an approved prerelease tag`) +} + +export function assertExpectedChannel(classification, expectedChannel) { + if (expectedChannel !== 'stable' && expectedChannel !== 'prerelease') { + fail(`Unknown expected channel "${expectedChannel}": use stable or prerelease`) + } + + if (classification.channel !== expectedChannel) { + fail(`Tag ${classification.tag} is ${classification.channel}; this release path requires a ${expectedChannel} tag`) + } +} + +export function assertTagMatchesPackageVersion(classification, packageVersion) { + if (typeof packageVersion !== 'string' || packageVersion.length === 0) { + fail('package.json must contain a non-empty version field') + } + + if (classification.version !== packageVersion) { + fail(`Tag ${classification.tag} does not match package.json version ${packageVersion}`) + } +} + +export function assertChangelogContainsVersion(version, changelog) { + const escapedVersion = version.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + const headingPattern = new RegExp(`^## \\[${escapedVersion}\\](?:\\s|$)`, 'm') + + if (!headingPattern.test(changelog)) { + fail(`CHANGELOG.md is missing a ## [${version}] section`) + } +} + +function parseArguments(args) { + const options = { + changelogPath: 'CHANGELOG.md', + packageJsonPath: 'package.json', + verifyChangelog: false, + verifyPackageVersion: false, + } + + for (let index = 0; index < args.length; index += 1) { + const argument = args[index] + + if (argument === '--verify-package-version') { + options.verifyPackageVersion = true + continue + } + if (argument === '--verify-changelog') { + options.verifyChangelog = true + continue + } + + const value = args[index + 1] + if (!value || value.startsWith('--')) { + fail(`${argument} requires a value`) + } + + if (argument === '--tag') { + options.tag = value + } else if (argument === '--expect') { + options.expectedChannel = value + } else if (argument === '--package-json') { + options.packageJsonPath = value + } else if (argument === '--changelog') { + options.changelogPath = value + } else { + fail(`Unknown argument "${argument}"`) + } + index += 1 + } + + return options +} + +export function runCli(args) { + const options = parseArguments(args) + const classification = classifyReleaseTag(options.tag) + + if (options.expectedChannel) { + assertExpectedChannel(classification, options.expectedChannel) + } + + if (options.verifyPackageVersion) { + const packageManifest = JSON.parse(readFileSync(resolve(options.packageJsonPath), 'utf8')) + assertTagMatchesPackageVersion(classification, packageManifest.version) + } + + if (options.verifyChangelog) { + const changelog = readFileSync(resolve(options.changelogPath), 'utf8') + assertChangelogContainsVersion(classification.version, changelog) + } + + console.log(`channel=${classification.channel}`) + console.log(`version=${classification.version}`) +} + +const isCli = process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isCli) { + try { + runCli(process.argv.slice(2)) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error(`Release tag validation failed: ${message}`) + process.exitCode = 1 + } +} diff --git a/.github/scripts/verify-next-release-state.mjs b/.github/scripts/verify-next-release-state.mjs new file mode 100644 index 00000000..c53592e7 --- /dev/null +++ b/.github/scripts/verify-next-release-state.mjs @@ -0,0 +1,143 @@ +import { spawnSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +function fail(message) { + throw new Error(message) +} + +function readJson(path) { + return JSON.parse(readFileSync(resolve(path), 'utf8')) +} + +function publishedVersionList(value) { + if (typeof value === 'string') { + return [value] + } + if (Array.isArray(value) && value.every((entry) => typeof entry === 'string')) { + return value + } + fail('Published npm versions must be a JSON string or array of strings') +} + +export function assertPublishEventAllowed(eventName, ref) { + if (eventName === 'workflow_dispatch') { + return + } + if (eventName === 'push' && ref.startsWith('refs/tags/')) { + return + } + + fail(`Publishing is not allowed for ${eventName} on ${ref}; use an approved prerelease tag or workflow_dispatch`) +} + +export function assertVersionIsUnpublished(version, publishedVersions) { + if (publishedVersionList(publishedVersions).includes(version)) { + fail(`@lubab/madar@${version} is already published; npm versions are immutable, so prepare a new prerelease number`) + } +} + +export function assertCommitIsAncestor(commit, branch, isAncestor) { + if (!isAncestor) { + fail(`Tagged commit ${commit} is outside ${branch}; prereleases must come from the next branch`) + } +} + +export function assertPostPublishState(version, beforeTags, afterTags, resolvedVersion) { + if (afterTags.next !== version) { + fail(`npm dist-tag next points to ${String(afterTags.next)} instead of ${version}`) + } + if (afterTags.latest !== beforeTags.latest) { + fail(`npm dist-tag latest changed from ${String(beforeTags.latest)} to ${String(afterTags.latest)} during prerelease publication`) + } + if (resolvedVersion !== version) { + fail(`@lubab/madar@${version} resolved as ${String(resolvedVersion)} after publication`) + } +} + +function argumentValue(args, name) { + const index = args.indexOf(name) + if (index === -1) { + return undefined + } + const value = args[index + 1] + if (!value || value.startsWith('--')) { + fail(`${name} requires a value`) + } + return value +} + +function requireArgument(args, name) { + const value = argumentValue(args, name) + if (!value) { + fail(`${name} is required`) + } + return value +} + +function gitCommitIsAncestor(commit, branch) { + const result = spawnSync('git', ['merge-base', '--is-ancestor', commit, branch], { + encoding: 'utf8', + stdio: 'pipe', + }) + + if (result.status === 0) { + return true + } + if (result.status === 1) { + return false + } + + fail(result.stderr.trim() || `git merge-base failed with status ${String(result.status)}`) +} + +export function runCli(args) { + if (args.includes('--assert-event')) { + assertPublishEventAllowed( + requireArgument(args, '--event'), + requireArgument(args, '--ref'), + ) + console.log('event=allowed') + return + } + + if (args.includes('--assert-unpublished')) { + const version = requireArgument(args, '--version') + const versions = readJson(requireArgument(args, '--versions-file')) + assertVersionIsUnpublished(version, versions) + console.log('version=unpublished') + return + } + + if (args.includes('--assert-ancestor')) { + const commit = requireArgument(args, '--commit') + const branch = requireArgument(args, '--branch') + assertCommitIsAncestor(commit, branch, gitCommitIsAncestor(commit, branch)) + console.log('ancestor=true') + return + } + + if (args.includes('--verify-publish')) { + const version = requireArgument(args, '--version') + const beforeTags = readJson(requireArgument(args, '--before')) + const afterTags = readJson(requireArgument(args, '--after')) + const resolvedVersion = readJson(requireArgument(args, '--resolved-version')) + assertPostPublishState(version, beforeTags, afterTags, resolvedVersion) + console.log('publication=verified') + return + } + + fail('Choose one mode: --assert-event, --assert-unpublished, --assert-ancestor, or --verify-publish') +} + +const isCli = process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isCli) { + try { + runCli(process.argv.slice(2)) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error(`Next release validation failed: ${message}`) + process.exitCode = 1 + } +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b51732a7..385a7fb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,7 @@ on: push: branches: - main + - next pull_request: workflow_dispatch: diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml new file mode 100644 index 00000000..54483339 --- /dev/null +++ b/.github/workflows/publish-next.yml @@ -0,0 +1,350 @@ +name: Publish next prerelease + +on: + push: + tags: + - 'v*-beta.*' + - 'v*-rc.*' + - 'v*-next.*' + workflow_dispatch: + inputs: + release_tag: + description: Exact prerelease tag; select this same tag as the workflow ref when dispatching + required: true + type: string + +permissions: + contents: write + id-token: write + +concurrency: + group: publish-next + cancel-in-progress: false + +jobs: + publish: + if: github.event_name == 'workflow_dispatch' || (github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')) + runs-on: ubuntu-latest + timeout-minutes: 60 + environment: npm-next + env: + PACKAGE_NAME: '@lubab/madar' + RELEASE_TAG: ${{ github.event_name == 'workflow_dispatch' && inputs.release_tag || github.ref_name }} + + steps: + # Check out github.sha rather than deriving a ref from workflow_dispatch input. + # Untrusted input must never reach actions/checkout's `ref:`: checkout happens before + # any validation runs, so a crafted ref could supply its own validation scripts and + # then "pass" every gate below. github.sha is resolved by GitHub from the workflow ref, + # so dispatching REQUIRES selecting the prerelease tag itself as the workflow ref -- + # which the tag/commit assertion below enforces and its error message documents. + - name: Check out exact prerelease commit + uses: actions/checkout@v7 + with: + ref: ${{ github.sha }} + fetch-depth: 0 + persist-credentials: false + + # Never restore setup-node's npm cache in this privileged job: an untrusted + # pull-request job could poison a shared cache that later executes during publish. + # + # Node 22 is used deliberately: the published artifact must be validated on a Node + # version the CI matrix actually tests (20 and 22). Validating on an untested runtime + # risks both false failures that block a good release and false passes that ship a + # build broken for supported users. npm is upgraded separately below because + # Trusted Publishing needs npm >= 11.5.1, which Node 22 does not ship by default. + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + + - name: Upgrade npm for Trusted Publishing support + run: npm install --global npm@latest + + - name: Validate prerelease tag and release files + shell: bash + run: | + set -euo pipefail + node .github/scripts/verify-next-release-state.mjs \ + --assert-event \ + --event "$GITHUB_EVENT_NAME" \ + --ref "$GITHUB_REF" + node .github/scripts/classify-release-tag.mjs \ + --tag "$RELEASE_TAG" \ + --expect prerelease \ + --verify-package-version \ + --verify-changelog + + version="${RELEASE_TAG#v}" + printf 'PACKAGE_VERSION=%s\n' "$version" >> "$GITHUB_ENV" + + - name: Prove exact tag commit and next ancestry + shell: bash + run: | + set -euo pipefail + git fetch --no-tags origin refs/heads/next:refs/remotes/origin/next + + head_commit="$(git rev-parse HEAD)" + tag_commit="$(git rev-parse "refs/tags/$RELEASE_TAG^{commit}")" + if [[ "$head_commit" != "$tag_commit" ]]; then + echo "::error::Checked out commit $head_commit does not match $RELEASE_TAG at $tag_commit" + exit 1 + fi + if [[ "$head_commit" != "$GITHUB_SHA" ]]; then + echo "::error::The workflow event commit $GITHUB_SHA is not the tagged commit $head_commit. For workflow_dispatch, select $RELEASE_TAG as the workflow ref." + exit 1 + fi + + node .github/scripts/verify-next-release-state.mjs \ + --assert-ancestor \ + --commit "$head_commit" \ + --branch origin/next + printf 'RELEASE_COMMIT=%s\n' "$head_commit" >> "$GITHUB_ENV" + + - name: Verify version is not already published + shell: bash + run: | + set -euo pipefail + npm view "$PACKAGE_NAME" versions --json > "$RUNNER_TEMP/npm-published-versions.json" + node .github/scripts/verify-next-release-state.mjs \ + --assert-unpublished \ + --version "$PACKAGE_VERSION" \ + --versions-file "$RUNNER_TEMP/npm-published-versions.json" + + - name: Install dependencies + run: npm ci + + - name: Typecheck + run: npm run typecheck + + - name: Run tests + run: npm run test:run + + - name: Run tests with coverage thresholds + run: npm run test:coverage + + - name: Build + run: npm run build + + - name: Verify checkout and packed retrieval parity + run: npm run verify:pack-parity + + - name: Validate MCP Registry metadata + run: npm run registry:validate + + - name: Verify release hygiene + run: npm run release:verify + + - name: Run qualification validation when available + id: qualification + shell: bash + run: | + set -euo pipefail + if node -e "const scripts = require('./package.json').scripts ?? {}; process.exit(scripts['qualify:validate'] ? 0 : 1)"; then + npm run qualify:validate + echo 'status=passed' >> "$GITHUB_OUTPUT" + else + echo '::notice title=Qualification validation::qualify:validate is not present on this tag; qualification was skipped.' + echo 'status=skipped (script not present)' >> "$GITHUB_OUTPUT" + fi + + - name: Validate npm package contents + run: npm pack --dry-run + + - name: Dry-run next publication + run: npm publish --dry-run --tag next + + - name: Capture pre-publish npm dist-tags + shell: bash + run: | + set -euo pipefail + npm view "$PACKAGE_NAME" versions --json > "$RUNNER_TEMP/npm-published-versions.json" + node .github/scripts/verify-next-release-state.mjs \ + --assert-unpublished \ + --version "$PACKAGE_VERSION" \ + --versions-file "$RUNNER_TEMP/npm-published-versions.json" + npm view "$PACKAGE_NAME" dist-tags --json > "$RUNNER_TEMP/npm-dist-tags-before.json" + node -e "const tags = require(process.argv[1]); if (!tags || typeof tags !== 'object' || Array.isArray(tags)) throw new Error('npm returned invalid dist-tags JSON')" "$RUNNER_TEMP/npm-dist-tags-before.json" + + - name: Persist pre-publish npm dist-tags + uses: actions/upload-artifact@v6 + with: + name: npm-dist-tags-before-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/npm-dist-tags-before.json + if-no-files-found: error + retention-days: 30 + + - name: Require tokenless npm Trusted Publishing + shell: bash + run: | + set -euo pipefail + if [[ -n "${NODE_AUTH_TOKEN:-}" || -n "${NPM_TOKEN:-}" ]]; then + echo '::error::Long-lived npm tokens are forbidden. Remove NODE_AUTH_TOKEN/NPM_TOKEN and configure npm Trusted Publishing for this GitHub environment.' + exit 1 + fi + + user_config="$(npm config get userconfig)" + global_config="$(npm config get globalconfig)" + NPM_USER_CONFIG="$user_config" NPM_GLOBAL_CONFIG="$global_config" node --input-type=module <<'NODE' + import { existsSync, readFileSync } from 'node:fs' + import { resolve } from 'node:path' + + const credentialEnvironmentKeys = Object.keys(process.env).filter((key) => + /^(?:NODE_AUTH_TOKEN|NPM_TOKEN)$/i.test(key) + || /^NPM_CONFIG_.*(?:AUTH|TOKEN|PASSWORD)/i.test(key), + ) + if (credentialEnvironmentKeys.length > 0) { + throw new Error(`npm credential environment variables are forbidden: ${credentialEnvironmentKeys.join(', ')}`) + } + + const configPaths = [ + resolve('.npmrc'), + process.env.NPM_USER_CONFIG, + process.env.NPM_GLOBAL_CONFIG, + ].filter(Boolean) + for (const configPath of configPaths) { + if (existsSync(configPath) && /(?:_authToken|_auth|_password)\s*=/i.test(readFileSync(configPath, 'utf8'))) { + throw new Error(`npm token credentials are forbidden in ${configPath}`) + } + } + NODE + + npm_version="$(npm --version)" + node -e "const [major, minor, patch] = process.argv[1].split('.').map(Number); const supported = major > 11 || (major === 11 && (minor > 5 || (minor === 5 && patch >= 1))); if (!supported) { console.error('npm ' + process.argv[1] + ' cannot use Trusted Publishing; npm >=11.5.1 is required'); process.exit(1) }" "$npm_version" + if [[ "$(npm config get registry)" != 'https://registry.npmjs.org/' ]]; then + echo '::error::Trusted Publishing must target https://registry.npmjs.org/.' + exit 1 + fi + + - name: Publish prerelease with npm Trusted Publishing + shell: bash + run: | + set -euo pipefail + publish_log="$RUNNER_TEMP/npm-publish.log" + if ! npm publish --tag next --access public --provenance >"$publish_log" 2>&1; then + sed -E 's/(npm_[A-Za-z0-9]{20,}|Bearer[[:space:]]+[A-Za-z0-9._-]+)/[redacted]/g' "$publish_log" >&2 + echo '::error::Publication failed. Confirm the version is still unpublished and configure @lubab/madar Trusted Publishing for this GitHub repository, publish-next.yml, and the npm-next environment; do not add an npm token or remove provenance.' + exit 1 + fi + sed -E 's/(npm_[A-Za-z0-9]{20,}|Bearer[[:space:]]+[A-Za-z0-9._-]+)/[redacted]/g' "$publish_log" + + - name: Verify published version and dist-tags + shell: bash + run: | + set -euo pipefail + verified=false + for attempt in {1..12}; do + if npm view "$PACKAGE_NAME" dist-tags --json > "$RUNNER_TEMP/npm-dist-tags-after.json" \ + && npm view "$PACKAGE_NAME@$PACKAGE_VERSION" version --json > "$RUNNER_TEMP/npm-resolved-version.json" \ + && node .github/scripts/verify-next-release-state.mjs \ + --verify-publish \ + --version "$PACKAGE_VERSION" \ + --before "$RUNNER_TEMP/npm-dist-tags-before.json" \ + --after "$RUNNER_TEMP/npm-dist-tags-after.json" \ + --resolved-version "$RUNNER_TEMP/npm-resolved-version.json" >/dev/null 2>&1; then + verified=true + break + fi + echo "Waiting for npm registry propagation (attempt $attempt of 12)" + sleep 10 + done + + if [[ "$verified" != true ]]; then + node .github/scripts/verify-next-release-state.mjs \ + --verify-publish \ + --version "$PACKAGE_VERSION" \ + --before "$RUNNER_TEMP/npm-dist-tags-before.json" \ + --after "$RUNNER_TEMP/npm-dist-tags-after.json" \ + --resolved-version "$RUNNER_TEMP/npm-resolved-version.json" + exit 1 + fi + + npm view "$PACKAGE_NAME@$PACKAGE_VERSION" dist.integrity --json > "$RUNNER_TEMP/npm-integrity.json" + integrity="$(node -p "require(process.argv[1])" "$RUNNER_TEMP/npm-integrity.json")" + printf 'PACKAGE_INTEGRITY=%s\n' "$integrity" >> "$GITHUB_ENV" + + - name: Smoke-test exact and next global installs + shell: bash + run: | + set -euo pipefail + exact_prefix="$(mktemp -d "$RUNNER_TEMP/madar-exact-prefix.XXXXXX")" + next_prefix="$(mktemp -d "$RUNNER_TEMP/madar-next-prefix.XXXXXX")" + + npm install --global --prefix "$exact_prefix" --cache "$RUNNER_TEMP/npm-smoke-cache-exact" --prefer-online "$PACKAGE_NAME@$PACKAGE_VERSION" + npm install --global --prefix "$next_prefix" --cache "$RUNNER_TEMP/npm-smoke-cache-next" --prefer-online "$PACKAGE_NAME@next" + + exact_binary="$exact_prefix/bin/madar" + next_binary="$next_prefix/bin/madar" + [[ "$("$exact_binary" --version)" == "$PACKAGE_VERSION" ]] + [[ "$("$next_binary" --version)" == "$PACKAGE_VERSION" ]] + + for channel in exact next; do + workspace="$(mktemp -d "$RUNNER_TEMP/madar-$channel-workspace.XXXXXX")" + mkdir -p "$workspace/src" + printf '{"name":"madar-published-smoke","private":true,"type":"module"}\n' > "$workspace/package.json" + printf 'export function greet(name: string): string { return `hello ${name}` }\n' > "$workspace/src/index.ts" + + if [[ "$channel" == exact ]]; then + binary="$exact_binary" + else + binary="$next_binary" + fi + (cd "$workspace" && "$binary" generate . --no-html) + test -s "$workspace/out/graph.json" + done + + - name: Create or update GitHub prerelease + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + QUALIFICATION_STATUS: ${{ steps.qualification.outputs.status }} + shell: bash + run: | + set -euo pipefail + release_notes="$RUNNER_TEMP/github-prerelease-notes.md" + RELEASE_NOTES_PATH="$release_notes" node --input-type=module <<'NODE' + import { writeFileSync } from 'node:fs' + + const { + PACKAGE_INTEGRITY: integrity, + PACKAGE_VERSION: version, + QUALIFICATION_STATUS: qualification, + RELEASE_COMMIT: commit, + RELEASE_NOTES_PATH: notesPath, + } = process.env + const provenanceUrl = `https://www.npmjs.com/package/@lubab/madar/v/${version}#provenance` + const body = [ + '## Prerelease publication', + '', + `- Version: \`${version}\``, + `- Commit: \`${commit}\``, + '- npm dist-tag: `next`', + `- Install: \`npm install -g @lubab/madar@${version}\``, + `- Provenance: ${provenanceUrl}`, + `- Integrity: \`${integrity}\``, + `- Qualification: ${qualification}`, + '', + '## Known limitations', + '', + 'Path-sensitive checks that remain single-lane are follow-up work and are not cross-platform proof.', + '', + '## Rollback and remediation', + '', + 'npm versions are immutable. If this prerelease is defective, deprecate the exact version, move `next` back to the last-known-good prerelease with `npm dist-tag add @lubab/madar@ next`, document the incident, and publish a new prerelease number. Do not move `latest` during prerelease remediation.', + '', + ].join('\n') + writeFileSync(notesPath, body) + NODE + + if gh release view "$RELEASE_TAG" >/dev/null 2>&1; then + gh release edit "$RELEASE_TAG" \ + --title "$RELEASE_TAG" \ + --target "$RELEASE_COMMIT" \ + --prerelease \ + --notes-file "$release_notes" + else + gh release create "$RELEASE_TAG" \ + --title "$RELEASE_TAG" \ + --target "$RELEASE_COMMIT" \ + --prerelease \ + --notes-file "$release_notes" + fi diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 13660777..e9db46d9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,6 +14,11 @@ concurrency: jobs: release: + # Stable channel only. A SemVer prerelease tag always contains '-', so beta/rc/next + # tags skip this job cleanly and are handled by publish-next.yml instead. Skipping + # rather than failing keeps every legitimate prerelease from leaving a red run in the + # Actions history; malformed stable tags still fail loudly in the step below. + if: ${{ !contains(github.ref_name, '-') }} runs-on: ubuntu-latest timeout-minutes: 20 @@ -29,6 +34,9 @@ jobs: node-version: '20' cache: npm + - name: Require stable release tag + run: node .github/scripts/classify-release-tag.mjs --tag "$GITHUB_REF_NAME" --expect stable + - name: Validate tag, package version, and changelog run: | set -euo pipefail diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e0044e6..22e55118 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,6 +52,19 @@ Before opening a pull request: If your change affects extraction behavior, prefer adding a small fixture and a targeted test under `tests/unit/` or `tests/fixtures/`. +### Branch and release policy + +| Branch or change | Role | Pull request target | npm dist-tag | +| --- | --- | --- | --- | +| `main` | Stable release line | Reviewed stable promotions only | `latest` | +| `next` | Prerelease integration line | Issue, roadmap, and release-preparation work | `next` | +| Issue branches | Focused fixes or improvements branched from `next` | `next` | None | +| Roadmap pull requests | Planned work branched from `next` | `next` | None | + +Create Issue branches from `next`, and target `next` for ordinary and roadmap pull requests. A stable release reaches `main` only through a reviewed `next` → `main` pull request; do not bypass that promotion trail with an equivalent main-based branch. + +Pushes to `main` and `next`, plus pull requests, run the six-job Ubuntu/macOS/Windows and Node 20/22 CI matrix. Path-sensitive checks that remain single-lane are follow-up work, not cross-platform proof. + ## Data, benchmarks, and private material Do not include private repositories, private corpora, proprietary prompts, API keys, tokens, credentials, customer data, or raw logs that may contain sensitive data. diff --git a/docs/release.md b/docs/release.md index 6773193a..50f3ba84 100644 --- a/docs/release.md +++ b/docs/release.md @@ -1,74 +1,92 @@ -# Release checklist +# Release channels and checklist -Use this checklist when preparing a new `madar` release. It is intentionally manual: the goal is to keep each version easy to verify without hiding the release steps behind automation. +Madar uses two release channels. `main` carries stable releases on npm's `latest` dist-tag, while `next` carries approved prereleases on the `next` dist-tag. Release versions and tags must match exactly: stable tags look like `v0.33.0`, and approved prerelease tags look like `v0.33.0-beta.1`, `v0.33.0-rc.1`, or `v0.33.0-next.1`. -## 1. Prepare the release commit +## Install a release channel -1. Update the package version with `npm version `. -2. Review `package.json` and `package-lock.json` to confirm the new version is correct. -3. Update `CHANGELOG.md` with the user-visible changes in the release. -4. Make sure any linked docs, examples, install flows, and `docs/mcp-registry/server.json` reflect the new behavior. -5. Any new public claim requires a reproducible artifact under `docs/benchmarks/suite/` and a matching update to `docs/claims-and-evidence.md` before the README or release notes can say it publicly. -6. If this release will be announced outside the repo, copy the proof block and channel tracker from [`docs/launch-checklist.md`](./launch-checklist.md) into the release PR, release notes draft, or other working notes before drafting external copy. +Install the current stable release: -## 2. Run the required verification commands +```bash +npm install -g @lubab/madar +``` + +Install the current beta / prerelease selected by the `next` dist-tag: + +```bash +npm install -g @lubab/madar@next +``` + +Install an exact beta for reproducible testing: + +```bash +npm install -g @lubab/madar@0.33.0-beta.1 +``` + +## Branch policy + +| Branch or change | Purpose | npm channel | Pull request policy | +| --- | --- | --- | --- | +| `main` | Stable releases only | `latest` | Receives stable promotion through a reviewed `next` → `main` pull request | +| `next` | Prerelease integration and qualification | `next` | Receives reviewed issue and roadmap pull requests | +| Issue branches | One focused issue or improvement | None | Branch from `next` and target `next` | +| Roadmap changes | Planned product work | None | Branch from `next` and target `next` | + +In short, use `main` for stable releases, `next` for prereleases. Do not bypass the reviewed promotion pull request to move integration work directly onto `main`. + +CI runs its six Ubuntu/macOS/Windows and Node 20/22 jobs for pushes to both long-lived branches and for pull requests. That is broad repository-level validation. Path-sensitive checks that remain single-lane are follow-up work, not cross-platform proof. + +## Required release verification -From the repository root: +Run the applicable commands from the repository root on the exact commit proposed for release: ```bash -npm install +npm ci npm run release:verify npm run registry:validate npm run typecheck -npm run build npm run test:run +npm run test:coverage +npm run build +npm run verify:pack-parity npm pack --dry-run npm sbom --sbom-format cyclonedx > sbom.cdx.json ``` -`npm run release:verify` locks the public package metadata, changelog version entry, and npm-visible README links before publish so repository/documentation drift is caught in one pass. +Run `npm run qualify:validate` when that script is present. Its failure is a release blocker; when it is absent, record that qualification was unavailable rather than presenting it as passed. -If the change touches packaging, installer behavior, or public MCP Registry metadata, keep the `npm pack --dry-run` output with the release notes or pull request for easy review. Keep the generated `sbom.cdx.json` alongside the release PR or release notes as the checked supply-chain inventory snapshot for that version. Review [`docs/security/mcp-threat-model.md`](./security/mcp-threat-model.md) before publishing changes that affect MCP installs, share-safe artifacts, prompt handling, or local file boundaries. +`npm run release:verify` locks the public package metadata, changelog version entry, and npm-visible README links before publish. `npm pack --dry-run` records the package boundary, and `sbom.cdx.json` is the checked supply-chain inventory snapshot. If the change touches packaging, installer behavior, or public MCP Registry metadata, keep those outputs with the release pull request. Review [`docs/security/mcp-threat-model.md`](./security/mcp-threat-model.md) before publishing changes that affect MCP installs, share-safe artifacts, prompt handling, or local file boundaries. -## 3. Run manual CLI smoke checks +Any new public claim requires a reproducible artifact under `docs/benchmarks/suite/` and a matching update to `docs/claims-and-evidence.md` before the README or release notes can say it publicly. For an external announcement, copy the proof block and channel tracker from [`docs/launch-checklist.md`](./launch-checklist.md) into the release pull request or working notes before drafting copy. -These checks verify that the published surface still matches the docs and changelog: +## Beta preparation (10 steps) -```bash -madar --version -madar generate . -madar claude install -madar codex install -``` +1. Create the issue or roadmap branch from current `next`; keep the change focused and do not branch release work from `main`. +2. Implement the change, add focused tests and fixtures, update user-facing documentation, and run the relevant local checks. +3. Open the pull request against `next`, obtain review, and wait for the full CI matrix. Treat any path-sensitive single-lane result as targeted evidence, not cross-platform proof. +4. Merge the reviewed change into `next`, then choose the next approved version such as `0.33.0-beta.1`, `0.33.0-rc.1`, or `0.33.0-next.1`. +5. On a short release-preparation branch from `next`, run `npm version 0.33.0-beta.1 --no-git-tag-version` (substituting the chosen version), verify both `package.json` and `package-lock.json`, and add the exact dated `CHANGELOG.md` section. +6. Review linked docs, examples, install flows, claims, and limitations. Do not change `docs/mcp-registry/server.json` merely to publish a prerelease; MCP Registry publication remains a separate explicitly scoped operation. +7. Run every command under [Required release verification](#required-release-verification), including mandatory qualification when available, and retain the pack and SBOM evidence. +8. Run manual CLI smoke checks, open the release-preparation pull request back to `next`, obtain review, merge it, and confirm the intended release commit is now contained in `origin/next`. +9. Create the exact `v` tag on that merged commit and push only the tag. Approve the protected `npm-next` environment after reviewing the tag, commit, changelog, and validation plan. The workflow uses npm Trusted Publishing with OIDC and provenance; if that trust policy is unavailable, it fails without using a token or dropping provenance. +10. Confirm `@lubab/madar@` resolves, `@lubab/madar@next` installs that exact version, `latest` did not move, both installed binaries pass the temporary-workspace smoke, and the GitHub release is marked as a prerelease with qualification and remediation notes. -Recommended follow-up checks: +The protected workflow runs `npm publish --tag next --access public --provenance` only after all gates pass. npm versions are immutable: remediate a bad beta by deprecating it, moving `next` back to a known-good prerelease, documenting the gap, and publishing a new prerelease number. Never move `latest` as part of prerelease remediation. -- confirm `madar --version` prints the version you are about to publish -- confirm `madar generate .` completes and refreshes `out/graph.json` -- confirm install commands write the expected project files and instructions -- for Codex, confirm `.codex/hooks.json`, `.codex/madar-user-prompt-submit.cjs`, and this workspace's block in `~/.codex/config.toml` exist, and that it contains `startup_timeout_sec = 180` plus `tool_timeout_sec = 60`; only in a trusted repository, restart or open a new session, use `/hooks` to review/trust the project hook, then use `/mcp` or `codex mcp list` to verify the local MCP server -- uninstall any agent profile you enabled during the smoke test so the workspace returns to a clean state +## Stable promotion (8 steps) -## 4. Publish and tag +1. Select a qualified commit on `next`; confirm its beta feedback, known limitations, changelog, public claims, pack evidence, and launch checklist are ready for stable users. +2. Prepare the stable promotion commit on `next` with `npm version 0.33.0 --no-git-tag-version` (substituting the intended stable version), keep `package.json` and `package-lock.json` aligned, and convert the changelog entry into the exact dated stable section. +3. Run every command under [Required release verification](#required-release-verification), the manual CLI smoke checks below, and any present qualification command against that exact promotion commit. +4. Open the stable promotion as a reviewed `next` → `main` pull request. Do not recreate the changes on a main-based branch; the reviewed promotion is the audit trail for what graduates. +5. Wait for required review and CI, merge into `main`, and verify the merged commit is the reviewed content with no release-file drift. +6. Create and push the exact stable tag on the merged `main` commit. `.github/workflows/release.yml` rejects prerelease tags, revalidates the stable release, and creates the ordinary GitHub release. +7. From an authorized provenance-capable environment checked out at that exact tag, verify that the version is unpublished and run `npm publish --access public --provenance`. This publishes to npm's default `latest` dist-tag; never use `--tag next` for stable promotion. +8. Confirm the exact version and `latest` resolve, install and smoke-test the published package in a clean workspace, confirm the prerelease history and `next` status remain intentional, record channel status in the release notes, and then reopen `next` for the next prerelease cycle. -After the verification steps are green: +## Manual CLI smoke checks -1. Push and merge the verified release commit so the published README links already exist on the target release branch (`main` for stable releases, `next` for prereleases). -2. Publish from that merged release commit: - - stable releases: `npm publish --access public --provenance` - - prereleases / `next`: `npm publish --tag next --access public --provenance` - If the release environment does not support npm provenance attestations, rerun the same command without `--provenance`. -3. Create the matching Git tag if `npm version` did not already do so in your workflow. -4. After npm confirms the matching public version, run the **Publish MCP Registry metadata** GitHub Actions workflow with that `vX.Y.Z` tag. It uses GitHub OIDC (no registry secret), verifies the published package has `mcpName: "io.github.mohanagy/madar"`, publishes the checked-in manifest, and verifies the Registry API result. -5. Draft or publish the GitHub release notes from the changelog entry. -6. Before posting on npm/GitHub directories, social/news sites, or videos/blogs, complete the copied proof-first launch checklist from [`docs/launch-checklist.md`](./launch-checklist.md) so every public surface starts from a dated receipt plus caveats. - -## 5. Post-release verification - -After the package is live: - -1. Confirm the new version appears on npm. -2. Install the released version in a clean shell and re-run: +Before publication, exercise the built CLI: ```bash madar --version @@ -77,6 +95,12 @@ madar claude install madar codex install ``` -3. Verify the README, changelog, and install docs still describe the released behavior accurately. -4. If anything is wrong, document the gap immediately and prepare a follow-up patch release instead of silently relying on tribal knowledge. -5. Record the completed channel statuses in the release PR, release notes draft, or other working notes you copied from [`docs/launch-checklist.md`](./launch-checklist.md) so distribution work stays explicit without mutating the canonical template. +Confirm `madar --version` prints the version about to be published, generation refreshes `out/graph.json`, and install commands write the expected project files and instructions. For Codex, confirm `.codex/hooks.json`, `.codex/madar-user-prompt-submit.cjs`, and this workspace's block in `~/.codex/config.toml` exist with `startup_timeout_sec = 180` and `tool_timeout_sec = 60`. Only in a trusted repository, restart or open a new session, use `/hooks` to review and trust the project hook, then use `/mcp` or `codex mcp list` to verify the local MCP server. Uninstall any agent profile enabled solely for the smoke test. + +## Post-release verification + +After publication, install the exact public version in a clean temporary workspace and repeat the relevant `madar --version` and `madar generate .` checks against the installed binary, not the repository checkout. + +If stable package metadata should be published to the official MCP Registry, run the separate **Publish MCP Registry metadata** workflow only after npm confirms the matching public version. It uses GitHub OIDC, validates the checked-in manifest, and verifies the Registry API result. Neither npm release workflow mutates MCP Registry metadata. + +Before posting to npm/GitHub directories, social/news sites, or videos/blogs, complete the copied proof-first checklist from [`docs/launch-checklist.md`](./launch-checklist.md). If anything is wrong after release, document it immediately and prepare a new version instead of silently relying on tribal knowledge. diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts new file mode 100644 index 00000000..2129c416 --- /dev/null +++ b/tests/unit/release-pipeline.test.ts @@ -0,0 +1,350 @@ +import { execFileSync, spawnSync } from 'node:child_process' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' + +import { describe, expect, it } from 'vitest' +import { parse } from 'yaml' + +interface WorkflowStep { + name?: string + run?: string + uses?: string + with?: Record +} + +interface Workflow { + on?: { + pull_request?: unknown + push?: { + branches?: string[] + tags?: string[] + } + workflow_dispatch?: unknown + } + permissions?: Record + jobs?: Record +} + +const classifierPath = resolve('.github/scripts/classify-release-tag.mjs') +const nextReleaseStatePath = resolve('.github/scripts/verify-next-release-state.mjs') + +function runNode(script: string, args: string[], cwd = process.cwd()) { + return spawnSync(process.execPath, [script, ...args], { + cwd, + encoding: 'utf8', + stdio: 'pipe', + }) +} + +function parseWorkflow(path: string): Workflow { + return parse(readFileSync(resolve(path), 'utf8')) as Workflow +} + +function workflowStep(workflow: Workflow, jobName: string, stepName: string): WorkflowStep { + const step = workflow.jobs?.[jobName]?.steps?.find((candidate) => candidate.name === stepName) + if (!step) { + throw new Error(`Missing ${jobName} workflow step: ${stepName}`) + } + return step +} + +function withReleaseFixture( + packageVersion: string, + changelog: string, + runAssertion: (fixtureDir: string) => void, +): void { + const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-release-tag-')) + + try { + writeFileSync(join(fixtureDir, 'package.json'), JSON.stringify({ version: packageVersion })) + writeFileSync(join(fixtureDir, 'CHANGELOG.md'), changelog) + runAssertion(fixtureDir) + } finally { + rmSync(fixtureDir, { recursive: true, force: true }) + } +} + +describe('release tag classifier', () => { + it('classifies stable SemVer tags', () => { + const result = runNode(classifierPath, ['--tag', 'v1.2.3']) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('channel=stable') + expect(result.stdout).toContain('version=1.2.3') + }) + + it.each(['v1.2.3-beta.0', 'v1.2.3-rc.2', 'v1.2.3-next.42'])( + 'classifies the approved prerelease tag %s', + (tag) => { + const result = runNode(classifierPath, ['--tag', tag]) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('channel=prerelease') + }, + ) + + it.each([ + 'v1.2.3-alpha.1', + 'v1.2.3-beta', + 'v1.2.3-preview.0', + 'v1.2.3-beta.01', + 'v1.2.3-beta.1.trailing', + '1.2.3', + 'v1.two.3', + 'v1.2.3+build.1', + ])('rejects the invalid release tag %s', (tag) => { + const result = runNode(classifierPath, ['--tag', tag]) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Release tag validation failed:') + }) + + it('rejects a tag/package version mismatch independently', () => { + withReleaseFixture('1.2.4', '## [1.2.3]\n', (fixtureDir) => { + const result = runNode(classifierPath, [ + '--tag', 'v1.2.3', + '--verify-package-version', + ], fixtureDir) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('does not match package.json version 1.2.4') + }) + }) + + it('rejects a missing changelog section independently', () => { + withReleaseFixture('1.2.3-beta.1', '## [Unreleased]\n', (fixtureDir) => { + const result = runNode(classifierPath, [ + '--tag', 'v1.2.3-beta.1', + '--verify-changelog', + ], fixtureDir) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('CHANGELOG.md is missing a ## [1.2.3-beta.1] section') + }) + }) + + it('makes the prerelease path reject a stable tag', () => { + const result = runNode(classifierPath, ['--tag', 'v1.2.3', '--expect', 'prerelease']) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('this release path requires a prerelease tag') + }) + + it('makes the stable path reject a prerelease tag', () => { + const result = runNode(classifierPath, ['--tag', 'v1.2.3-beta.1', '--expect', 'stable']) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('this release path requires a stable tag') + }) +}) + +describe('next release state guards', () => { + it.each([ + ['push', 'refs/heads/next'], + ['pull_request', 'refs/pull/123/merge'], + ])('rejects publication for a %s event on %s', (eventName, ref) => { + const result = runNode(nextReleaseStatePath, [ + '--assert-event', + '--event', eventName, + '--ref', ref, + ]) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Publishing is not allowed') + }) + + it('rejects a tagged commit outside next', () => { + const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-next-ancestor-')) + + try { + execFileSync('git', ['init', '-b', 'next'], { cwd: fixtureDir, stdio: 'pipe' }) + execFileSync('git', ['config', 'user.email', 'madar@example.com'], { cwd: fixtureDir }) + execFileSync('git', ['config', 'user.name', 'Madar Test'], { cwd: fixtureDir }) + writeFileSync(join(fixtureDir, 'fixture.txt'), 'base\n') + execFileSync('git', ['add', 'fixture.txt'], { cwd: fixtureDir }) + execFileSync('git', ['commit', '-m', 'base'], { cwd: fixtureDir, stdio: 'pipe' }) + execFileSync('git', ['switch', '-c', 'outside'], { cwd: fixtureDir, stdio: 'pipe' }) + writeFileSync(join(fixtureDir, 'fixture.txt'), 'outside\n') + execFileSync('git', ['commit', '-am', 'outside'], { cwd: fixtureDir, stdio: 'pipe' }) + const outsideCommit = execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: fixtureDir, + encoding: 'utf8', + }).trim() + execFileSync('git', ['switch', 'next'], { cwd: fixtureDir, stdio: 'pipe' }) + + const result = runNode(nextReleaseStatePath, [ + '--assert-ancestor', + '--commit', outsideCommit, + '--branch', 'next', + ], fixtureDir) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('is outside next') + } finally { + rmSync(fixtureDir, { recursive: true, force: true }) + } + }) + + it('rejects an already-published version', () => { + const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-published-version-')) + + try { + const versionsPath = join(fixtureDir, 'versions.json') + writeFileSync(versionsPath, JSON.stringify(['0.32.1', '0.33.0-beta.1'])) + + const result = runNode(nextReleaseStatePath, [ + '--assert-unpublished', + '--version', '0.33.0-beta.1', + '--versions-file', versionsPath, + ]) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('is already published') + expect(result.stderr).toContain('prepare a new prerelease number') + } finally { + rmSync(fixtureDir, { recursive: true, force: true }) + } + }) + + it('preserves latest while moving next to the published version', () => { + const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-dist-tags-')) + + try { + const beforePath = join(fixtureDir, 'before.json') + const afterPath = join(fixtureDir, 'after.json') + const resolvedPath = join(fixtureDir, 'resolved.json') + writeFileSync(beforePath, JSON.stringify({ latest: '0.32.1', next: '0.33.0-beta.0' })) + writeFileSync(afterPath, JSON.stringify({ latest: '0.32.1', next: '0.33.0-beta.1' })) + writeFileSync(resolvedPath, JSON.stringify('0.33.0-beta.1')) + + const validResult = runNode(nextReleaseStatePath, [ + '--verify-publish', + '--version', '0.33.0-beta.1', + '--before', beforePath, + '--after', afterPath, + '--resolved-version', resolvedPath, + ]) + expect(validResult.status).toBe(0) + + writeFileSync(afterPath, JSON.stringify({ latest: '0.33.0-beta.1', next: '0.33.0-beta.1' })) + const changedLatestResult = runNode(nextReleaseStatePath, [ + '--verify-publish', + '--version', '0.33.0-beta.1', + '--before', beforePath, + '--after', afterPath, + '--resolved-version', resolvedPath, + ]) + expect(changedLatestResult.status).not.toBe(0) + expect(changedLatestResult.stderr).toContain('npm dist-tag latest changed') + } finally { + rmSync(fixtureDir, { recursive: true, force: true }) + } + }) +}) + +describe('release workflow policy', () => { + it('parses every release workflow as YAML', () => { + expect(() => parseWorkflow('.github/workflows/ci.yml')).not.toThrow() + expect(() => parseWorkflow('.github/workflows/release.yml')).not.toThrow() + expect(() => parseWorkflow('.github/workflows/publish-next.yml')).not.toThrow() + }) + + it('allows prerelease publication only from prerelease tags or manual dispatch', () => { + const workflow = parseWorkflow('.github/workflows/publish-next.yml') + + expect(workflow.on?.push?.branches).toBeUndefined() + expect(workflow.on?.push?.tags).toEqual([ + 'v*-beta.*', + 'v*-rc.*', + 'v*-next.*', + ]) + expect(workflow.on?.pull_request).toBeUndefined() + expect(workflow.on?.workflow_dispatch).toBeDefined() + expect(workflow.jobs?.publish?.if).toContain("github.event_name == 'workflow_dispatch'") + expect(workflow.jobs?.publish?.if).toContain("startsWith(github.ref, 'refs/tags/')") + }) + + it('uses protected OIDC publication without a privileged dependency cache', () => { + const workflow = parseWorkflow('.github/workflows/publish-next.yml') + const checkout = workflowStep(workflow, 'publish', 'Check out exact prerelease commit') + const setupNode = workflowStep(workflow, 'publish', 'Set up Node.js') + const publish = workflowStep(workflow, 'publish', 'Publish prerelease with npm Trusted Publishing') + + expect(workflow.permissions).toEqual({ contents: 'write', 'id-token': 'write' }) + expect(workflow.jobs?.publish?.environment).toBe('npm-next') + expect(checkout.with).toMatchObject({ + 'fetch-depth': 0, + 'persist-credentials': false, + }) + // Ref injection guard: checkout runs before any validation, so a ref derived from + // workflow_dispatch input could supply its own validation scripts and pass every gate. + // The ref must come from github.sha, which GitHub resolves from the workflow ref. + expect(checkout.with?.ref).toBe('${{ github.sha }}') + expect(String(checkout.with?.ref)).not.toContain('inputs.') + // The published artifact must be validated on a Node version the CI matrix tests. + expect(['20', '22']).toContain(String(setupNode.with?.['node-version'])) + expect(setupNode.with).not.toHaveProperty('cache') + expect(publish.run).toContain('npm publish --tag next --access public --provenance') + expect(publish.run).not.toContain('npm publish --access public') + expect(publish.run).not.toContain('NODE_AUTH_TOKEN') + }) + + it('keeps every required prerelease validation gate before publication', () => { + const workflow = parseWorkflow('.github/workflows/publish-next.yml') + const steps = workflow.jobs?.publish?.steps ?? [] + const publishIndex = steps.findIndex((step) => step.name === 'Publish prerelease with npm Trusted Publishing') + const requiredCommands = [ + 'npm ci', + 'npm run typecheck', + 'npm run test:run', + 'npm run test:coverage', + 'npm run build', + 'npm run verify:pack-parity', + 'npm run registry:validate', + 'npm run release:verify', + 'npm pack --dry-run', + 'npm publish --dry-run --tag next', + ] + + expect(publishIndex).toBeGreaterThan(0) + for (const command of requiredCommands) { + const commandIndex = steps.findIndex((step) => step.run?.includes(command)) + expect(commandIndex, command).toBeGreaterThanOrEqual(0) + expect(commandIndex, command).toBeLessThan(publishIndex) + } + expect(workflowStep(workflow, 'publish', 'Run qualification validation when available').run) + .toContain('npm run qualify:validate') + }) + + it('classifies the GitHub releases on the correct channels', () => { + const stableWorkflow = parseWorkflow('.github/workflows/release.yml') + const nextWorkflow = parseWorkflow('.github/workflows/publish-next.yml') + const stableClassifier = workflowStep(stableWorkflow, 'release', 'Require stable release tag') + const prereleaseClassifier = workflowStep(nextWorkflow, 'publish', 'Validate prerelease tag and release files') + const githubPrerelease = workflowStep(nextWorkflow, 'publish', 'Create or update GitHub prerelease') + + expect(stableClassifier.run).toContain('--expect stable') + expect(prereleaseClassifier.run).toContain('--expect prerelease') + expect(githubPrerelease.run).toContain('--prerelease') + expect(workflowStep(stableWorkflow, 'release', 'Create GitHub release').run).not.toContain('--prerelease') + }) + + it('keeps release documentation consistent with both channels', () => { + const releaseDoc = readFileSync(resolve('docs/release.md'), 'utf8') + const contributing = readFileSync(resolve('CONTRIBUTING.md'), 'utf8') + + expect(releaseDoc).toContain('npm install -g @lubab/madar') + expect(releaseDoc).toContain('npm install -g @lubab/madar@next') + expect(releaseDoc).toContain('npm install -g @lubab/madar@0.33.0-beta.1') + expect(releaseDoc).toContain('## Beta preparation (10 steps)') + expect(releaseDoc).toContain('## Stable promotion (8 steps)') + expect(releaseDoc).toContain('not cross-platform proof') + expect(contributing).toContain('Issue branches') + expect(contributing).toContain('reviewed `next` → `main` pull request') + }) +}) From 4f82365aa97e921dc07608bf87c3cfe0605510b2 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Wed, 12 Aug 2026 20:23:35 +0400 Subject: [PATCH 2/6] fix(release): harden prerelease publish runtime and checkout ref - check out github.sha so workflow_dispatch input cannot reach checkout ref - validate on Node 22, a version the CI matrix actually tests - install a pinned npm 12.0.2 with --ignore-scripts rather than npm@latest, so no unpinned toolchain executes inside the privileged publish job - assert all three properties in release-pipeline.test.ts Refs #654. --- .github/workflows/publish-next.yml | 10 ++-------- tests/unit/release-pipeline.test.ts | 4 ++++ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml index 54483339..859d052f 100644 --- a/.github/workflows/publish-next.yml +++ b/.github/workflows/publish-next.yml @@ -47,19 +47,13 @@ jobs: # Never restore setup-node's npm cache in this privileged job: an untrusted # pull-request job could poison a shared cache that later executes during publish. - # - # Node 22 is used deliberately: the published artifact must be validated on a Node - # version the CI matrix actually tests (20 and 22). Validating on an untested runtime - # risks both false failures that block a good release and false passes that ship a - # build broken for supported users. npm is upgraded separately below because - # Trusted Publishing needs npm >= 11.5.1, which Node 22 does not ship by default. - name: Set up Node.js uses: actions/setup-node@v7 with: node-version: '22' - - name: Upgrade npm for Trusted Publishing support - run: npm install --global npm@latest + - name: Install pinned npm Trusted Publishing client + run: npm install --global --ignore-scripts --no-audit --no-fund --cache "$RUNNER_TEMP/npm-cli-bootstrap-cache" npm@12.0.2 - name: Validate prerelease tag and release files shell: bash diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts index 2129c416..d9160c07 100644 --- a/tests/unit/release-pipeline.test.ts +++ b/tests/unit/release-pipeline.test.ts @@ -273,6 +273,7 @@ describe('release workflow policy', () => { const workflow = parseWorkflow('.github/workflows/publish-next.yml') const checkout = workflowStep(workflow, 'publish', 'Check out exact prerelease commit') const setupNode = workflowStep(workflow, 'publish', 'Set up Node.js') + const npmClient = workflowStep(workflow, 'publish', 'Install pinned npm Trusted Publishing client') const publish = workflowStep(workflow, 'publish', 'Publish prerelease with npm Trusted Publishing') expect(workflow.permissions).toEqual({ contents: 'write', 'id-token': 'write' }) @@ -289,6 +290,9 @@ describe('release workflow policy', () => { // The published artifact must be validated on a Node version the CI matrix tests. expect(['20', '22']).toContain(String(setupNode.with?.['node-version'])) expect(setupNode.with).not.toHaveProperty('cache') + expect(npmClient.run).toContain('npm@12.0.2') + expect(npmClient.run).toContain('--ignore-scripts') + expect(npmClient.run).not.toContain('npm@latest') expect(publish.run).toContain('npm publish --tag next --access public --provenance') expect(publish.run).not.toContain('npm publish --access public') expect(publish.run).not.toContain('NODE_AUTH_TOKEN') From 09a340bb787f5c29193b6028c9f209b36adfe6de Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Wed, 12 Aug 2026 22:03:58 +0400 Subject: [PATCH 3/6] fix(release): split publish-next into validate/publish/post_publish, pin actions, remove workflow_dispatch Remediates four findings from review of #688 / #687: - Finding A: removed the workflow_dispatch trigger and its release_tag input. publish-next.yml lives only on `next`, but the repository default branch is `main`, so a workflow_dispatch trigger here was a dead control surface that could never be invoked from the Actions UI. The workflow now triggers only on push of an approved prerelease tag; the tag is derived solely from github.ref_name. verify-next-release-state.mjs's assertPublishEventAllowed no longer accepts workflow_dispatch either. - Finding B: split the single privileged `publish` job into three: validate (contents: read, no environment) builds, tests, and packs the one tarball that will ever be published, records its SHA-256/size/identity/ source commit/tag/lockfile hash/toolchain versions in a receipt, and uploads it as one artifact. publish (needs: validate, contents: read + id-token: write, environment: npm-next) has no checkout, no npm ci, no tests, no build, no repository scripts, and no dependency cache -- it downloads and independently re-verifies the exact artifact validate produced, then npm publish is its last step with nothing after it. post_publish (needs: [validate, publish], contents: write, no id-token) verifies latest was preserved, runs the clean-install smoke test, and creates/edits the GitHub prerelease. It is idempotent so a failure here can be rerun without ever republishing an immutable npm version. - Finding C: pinned every action in publish-next.yml (checkout, setup-node, upload-artifact, download-artifact) to full 40-character commit SHAs with a readable version comment. ci.yml and release.yml are untouched. - Finding D: publish ends at the publish command with nothing after it, and post_publish's create-or-edit release step makes reruns safe. qualify:validate stays a presence-detection gate (hard fail when the script exists, notice when it doesn't) -- this is already deterministic since it is driven by the checked-out commit's package.json content, not a flag; once #681 lands the script on `next`, the next tag push automatically takes the hard-fail branch. tests/unit/release-pipeline.test.ts: 27 -> 44 tests. Every prior test is retained (relocated to the job it now covers); new tests assert the trigger change, the three-job graph and its needs/permissions/environment placement, that publish has no checkout/ci/tests/build/repo-scripts, that exactly one live npm publish exists and is publish's final step, exact-tarball publication, artifact SHA-256 recording/verification, latest preservation, and that every action uses a 40-character SHA (mutable refs rejected). Nothing published, tagged, or released. PR #688 stays a draft. Refs #687, #688. Co-Authored-By: Claude Opus 5 --- .github/scripts/verify-next-release-state.mjs | 5 +- .github/workflows/publish-next.yml | 303 ++++++++++++++--- tests/unit/release-pipeline.test.ts | 310 +++++++++++++++--- 3 files changed, 527 insertions(+), 91 deletions(-) diff --git a/.github/scripts/verify-next-release-state.mjs b/.github/scripts/verify-next-release-state.mjs index c53592e7..666358d1 100644 --- a/.github/scripts/verify-next-release-state.mjs +++ b/.github/scripts/verify-next-release-state.mjs @@ -22,14 +22,11 @@ function publishedVersionList(value) { } export function assertPublishEventAllowed(eventName, ref) { - if (eventName === 'workflow_dispatch') { - return - } if (eventName === 'push' && ref.startsWith('refs/tags/')) { return } - fail(`Publishing is not allowed for ${eventName} on ${ref}; use an approved prerelease tag or workflow_dispatch`) + fail(`Publishing is not allowed for ${eventName} on ${ref}; only pushing an approved prerelease tag triggers this pipeline`) } export function assertVersionIsUnpublished(version, publishedVersions) { diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml index 859d052f..3b64d953 100644 --- a/.github/workflows/publish-next.yml +++ b/.github/workflows/publish-next.yml @@ -1,54 +1,61 @@ name: Publish next prerelease +# Triggers only on an approved prerelease tag push. There is no workflow_dispatch: this +# workflow lives on `next`, the repository default branch is `main`, and a dispatch trigger +# defined on a non-default branch cannot be invoked from the Actions UI in the first place -- +# it would be a dead, misleading control surface. Retrying a failed run uses GitHub's own +# rerun-failed-jobs control (see the `post_publish` job below for why that is safe). on: push: tags: - 'v*-beta.*' - 'v*-rc.*' - 'v*-next.*' - workflow_dispatch: - inputs: - release_tag: - description: Exact prerelease tag; select this same tag as the workflow ref when dispatching - required: true - type: string +# No workflow-level `contents: write` or `id-token: write`. Every job declares the minimum +# permissions it needs for itself; nothing here is inherited broadly. permissions: - contents: write - id-token: write + contents: read concurrency: group: publish-next cancel-in-progress: false jobs: - publish: - if: github.event_name == 'workflow_dispatch' || (github.event_name == 'push' && startsWith(github.ref, 'refs/tags/')) + # Builds, tests, and packs the release candidate. Holds no publish credentials of any kind + # (no `id-token`, no environment), so it is safe for this job to run the full test suite, + # the build, and every repository script. + validate: + if: startsWith(github.ref, 'refs/tags/') runs-on: ubuntu-latest timeout-minutes: 60 - environment: npm-next + permissions: + contents: read env: PACKAGE_NAME: '@lubab/madar' - RELEASE_TAG: ${{ github.event_name == 'workflow_dispatch' && inputs.release_tag || github.ref_name }} + RELEASE_TAG: ${{ github.ref_name }} + + outputs: + version: ${{ env.PACKAGE_VERSION }} + commit: ${{ env.RELEASE_COMMIT }} + tag: ${{ env.RELEASE_TAG }} + tarball_name: ${{ steps.pack.outputs.tarball }} + tarball_sha256: ${{ steps.pack.outputs.sha256 }} + artifact_name: ${{ steps.pack.outputs.artifact_name }} + qualification_status: ${{ steps.qualification.outputs.status }} steps: - # Check out github.sha rather than deriving a ref from workflow_dispatch input. - # Untrusted input must never reach actions/checkout's `ref:`: checkout happens before - # any validation runs, so a crafted ref could supply its own validation scripts and - # then "pass" every gate below. github.sha is resolved by GitHub from the workflow ref, - # so dispatching REQUIRES selecting the prerelease tag itself as the workflow ref -- - # which the tag/commit assertion below enforces and its error message documents. - name: Check out exact prerelease commit - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.sha }} fetch-depth: 0 persist-credentials: false - # Never restore setup-node's npm cache in this privileged job: an untrusted - # pull-request job could poison a shared cache that later executes during publish. + # Never restore setup-node's npm cache in a release-pipeline job: an untrusted + # pull-request job could poison a shared cache that later executes here. - name: Set up Node.js - uses: actions/setup-node@v7 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '22' @@ -85,7 +92,7 @@ jobs: exit 1 fi if [[ "$head_commit" != "$GITHUB_SHA" ]]; then - echo "::error::The workflow event commit $GITHUB_SHA is not the tagged commit $head_commit. For workflow_dispatch, select $RELEASE_TAG as the workflow ref." + echo "::error::The workflow event commit $GITHUB_SHA is not the tagged commit $head_commit." exit 1 fi @@ -129,6 +136,14 @@ jobs: - name: Verify release hygiene run: npm run release:verify + # qualify:validate is deterministic, not a standing optional gate. It runs and hard-fails + # this job whenever the script exists on the checked-out commit. Right now it does not + # exist on `next`, so this records a notice instead. That absence is the *only* thing + # standing between this branch and a required run: qualify:validate lands with #681, and + # from the very next tag push that includes that merge, package.json on this exact + # commit will contain the script, `npm run qualify:validate` will execute for real, and a + # non-zero exit here fails the job via `set -euo pipefail`. Nothing else has to change -- + # no flag, no date, no follow-up PR to flip a switch. - name: Run qualification validation when available id: qualification shell: bash @@ -148,25 +163,193 @@ jobs: - name: Dry-run next publication run: npm publish --dry-run --tag next - - name: Capture pre-publish npm dist-tags + # This is the only tarball built for this release: `publish` downloads and ships exactly + # this file. It is never rebuilt from source in a credentialed job. + - name: Build release tarball and receipt + id: pack + shell: bash + env: + QUALIFICATION_STATUS: ${{ steps.qualification.outputs.status }} + run: | + set -euo pipefail + npm pack --json > "$RUNNER_TEMP/npm-pack.json" + tarball="$(node -e "console.log(require(process.argv[1])[0].filename)" "$RUNNER_TEMP/npm-pack.json")" + size="$(node -e "console.log(require(process.argv[1])[0].size)" "$RUNNER_TEMP/npm-pack.json")" + sha256="$(sha256sum "$tarball" | awk '{print $1}')" + lockfile_sha256="$(sha256sum package-lock.json | awk '{print $1}')" + + inspect_dir="$RUNNER_TEMP/tarball-inspect" + mkdir -p "$inspect_dir" + tar -xzf "$tarball" -C "$inspect_dir" + INSPECT_DIR="$inspect_dir" node -e " + const fs = require('fs'); + const path = require('path'); + const pkgPath = path.join(process.env.INSPECT_DIR, 'package', 'package.json'); + const pkg = require(pkgPath); + if (pkg.name !== process.env.PACKAGE_NAME) throw new Error('packed name ' + pkg.name + ' does not match ' + process.env.PACKAGE_NAME); + if (pkg.version !== process.env.PACKAGE_VERSION) throw new Error('packed version ' + pkg.version + ' does not match ' + process.env.PACKAGE_VERSION); + const bin = pkg.bin; + if (!bin || typeof bin !== 'object' || Object.keys(bin).length === 0) throw new Error('packed package.json has no bin entries'); + for (const [name, rel] of Object.entries(bin)) { + const binPath = path.join(process.env.INSPECT_DIR, 'package', rel); + if (!fs.existsSync(binPath)) throw new Error('CLI entry point for ' + name + ' is missing from the tarball: ' + rel); + } + console.log('packed package identity and CLI entry point verified'); + " + + artifact_dir="$RUNNER_TEMP/release-artifact" + mkdir -p "$artifact_dir" + cp "$tarball" "$artifact_dir/" + printf '%s %s\n' "$sha256" "$tarball" > "$artifact_dir/$tarball.sha256" + + RECEIPT_PATH="$artifact_dir/receipt.json" \ + TARBALL_NAME="$tarball" TARBALL_SHA256="$sha256" TARBALL_SIZE="$size" \ + LOCKFILE_SHA256="$lockfile_sha256" NPM_VERSION_OUT="$(npm --version)" \ + node -e " + const fs = require('fs'); + fs.writeFileSync(process.env.RECEIPT_PATH, JSON.stringify({ + package: process.env.PACKAGE_NAME, + version: process.env.PACKAGE_VERSION, + tarball: process.env.TARBALL_NAME, + tarballSha256: process.env.TARBALL_SHA256, + tarballSizeBytes: Number(process.env.TARBALL_SIZE), + sourceCommit: process.env.RELEASE_COMMIT, + sourceTag: process.env.RELEASE_TAG, + lockfileSha256: process.env.LOCKFILE_SHA256, + nodeVersion: process.version, + npmVersion: process.env.NPM_VERSION_OUT, + qualificationStatus: process.env.QUALIFICATION_STATUS, + generatedAt: new Date().toISOString(), + }, null, 2)) + " + + artifact_name="release-tarball-${{ github.run_id }}-${{ github.run_attempt }}-${PACKAGE_VERSION}-${RELEASE_COMMIT}" + { + echo "tarball=$tarball" + echo "sha256=$sha256" + echo "artifact_dir=$artifact_dir" + echo "artifact_name=$artifact_name" + } >> "$GITHUB_OUTPUT" + + # Retention is deliberately short: this artifact is a pre-publish package tarball, useful + # only to unblock a `publish`/`post_publish` rerun within the same release attempt. 14 + # days comfortably covers a stalled `npm-next` environment approval without retaining a + # stale, unpublished package indefinitely. + - name: Upload release tarball artifact + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ${{ steps.pack.outputs.artifact_name }} + path: ${{ steps.pack.outputs.artifact_dir }} + if-no-files-found: error + retention-days: 14 + + # The only job with `id-token: write`. It never checks out the repository, never runs `npm + # ci`, never runs tests or the build, and never executes a repository script -- it downloads + # the exact artifact `validate` already built, tested, and packed, re-verifies it + # independently, and publishes that file. `npm publish` is the last step in this job; nothing + # runs after it. + publish: + needs: validate + runs-on: ubuntu-latest + timeout-minutes: 20 + environment: npm-next + permissions: + contents: read + id-token: write + env: + PACKAGE_NAME: '@lubab/madar' + PACKAGE_VERSION: ${{ needs.validate.outputs.version }} + RELEASE_COMMIT: ${{ needs.validate.outputs.commit }} + RELEASE_TAG: ${{ needs.validate.outputs.tag }} + TARBALL_NAME: ${{ needs.validate.outputs.tarball_name }} + EXPECTED_TARBALL_SHA256: ${{ needs.validate.outputs.tarball_sha256 }} + + outputs: + pre_publish_dist_tags: ${{ steps.capture-dist-tags.outputs.json }} + + steps: + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '22' + + - name: Install pinned npm Trusted Publishing client + run: npm install --global --ignore-scripts --no-audit --no-fund --cache "$RUNNER_TEMP/npm-cli-bootstrap-cache" npm@12.0.2 + + - name: Download validated release tarball + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: ${{ needs.validate.outputs.artifact_name }} + path: release-artifact + + - name: Verify downloaded artifact against the validate job's receipt + shell: bash + run: | + set -euo pipefail + cd release-artifact + test -f "$TARBALL_NAME" + actual_sha256="$(sha256sum "$TARBALL_NAME" | awk '{print $1}')" + if [[ "$actual_sha256" != "$EXPECTED_TARBALL_SHA256" ]]; then + echo "::error::Downloaded tarball sha256 $actual_sha256 does not match validate output $EXPECTED_TARBALL_SHA256" + exit 1 + fi + sha256sum -c "$TARBALL_NAME.sha256" + test -f receipt.json + node -e " + const receipt = require('./receipt.json'); + if (receipt.tarballSha256 !== process.env.EXPECTED_TARBALL_SHA256) throw new Error('receipt sha256 does not match'); + if (receipt.version !== process.env.PACKAGE_VERSION) throw new Error('receipt version does not match'); + if (receipt.sourceCommit !== process.env.RELEASE_COMMIT) throw new Error('receipt commit does not match'); + if (receipt.package !== process.env.PACKAGE_NAME) throw new Error('receipt package name does not match'); + " + + - name: Verify packed package identity and CLI entry point + shell: bash + run: | + set -euo pipefail + inspect_dir="$RUNNER_TEMP/tarball-inspect" + mkdir -p "$inspect_dir" + tar -xzf "release-artifact/$TARBALL_NAME" -C "$inspect_dir" + INSPECT_DIR="$inspect_dir" node -e " + const fs = require('fs'); + const path = require('path'); + const pkg = require(path.join(process.env.INSPECT_DIR, 'package', 'package.json')); + if (pkg.name !== process.env.PACKAGE_NAME) throw new Error('packed name ' + pkg.name + ' does not match ' + process.env.PACKAGE_NAME); + if (pkg.version !== process.env.PACKAGE_VERSION) throw new Error('packed version ' + pkg.version + ' does not match ' + process.env.PACKAGE_VERSION); + const bin = pkg.bin; + if (!bin || typeof bin !== 'object' || Object.keys(bin).length === 0) throw new Error('packed package.json has no bin entries'); + for (const [name, rel] of Object.entries(bin)) { + const binPath = path.join(process.env.INSPECT_DIR, 'package', rel); + if (!fs.existsSync(binPath)) throw new Error('CLI entry point for ' + name + ' is missing from the tarball: ' + rel); + } + " + + - name: Verify version is still unpublished shell: bash run: | set -euo pipefail npm view "$PACKAGE_NAME" versions --json > "$RUNNER_TEMP/npm-published-versions.json" - node .github/scripts/verify-next-release-state.mjs \ - --assert-unpublished \ - --version "$PACKAGE_VERSION" \ - --versions-file "$RUNNER_TEMP/npm-published-versions.json" + node -e " + const fs = require('fs'); + const raw = JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); + const versions = typeof raw === 'string' ? [raw] : raw; + if (versions.includes(process.env.PACKAGE_VERSION)) { + throw new Error(process.env.PACKAGE_NAME + '@' + process.env.PACKAGE_VERSION + ' is already published; npm versions are immutable, so prepare a new prerelease number'); + } + " "$RUNNER_TEMP/npm-published-versions.json" + + - name: Capture pre-publish npm dist-tags + id: capture-dist-tags + shell: bash + run: | + set -euo pipefail npm view "$PACKAGE_NAME" dist-tags --json > "$RUNNER_TEMP/npm-dist-tags-before.json" node -e "const tags = require(process.argv[1]); if (!tags || typeof tags !== 'object' || Array.isArray(tags)) throw new Error('npm returned invalid dist-tags JSON')" "$RUNNER_TEMP/npm-dist-tags-before.json" - - - name: Persist pre-publish npm dist-tags - uses: actions/upload-artifact@v6 - with: - name: npm-dist-tags-before-${{ github.run_id }}-${{ github.run_attempt }} - path: ${{ runner.temp }}/npm-dist-tags-before.json - if-no-files-found: error - retention-days: 30 + { + echo "json<> "$GITHUB_OUTPUT" - name: Require tokenless npm Trusted Publishing shell: bash @@ -210,18 +393,63 @@ jobs: exit 1 fi + # Final substantive step of this job. Nothing runs after it: a failure in post_publish + # must never be "fixed" by publishing again, and keeping this step last makes that the + # only way the workflow can be structured to behave. - name: Publish prerelease with npm Trusted Publishing shell: bash run: | set -euo pipefail publish_log="$RUNNER_TEMP/npm-publish.log" - if ! npm publish --tag next --access public --provenance >"$publish_log" 2>&1; then + if ! npm publish "release-artifact/$TARBALL_NAME" --tag next --access public --provenance >"$publish_log" 2>&1; then sed -E 's/(npm_[A-Za-z0-9]{20,}|Bearer[[:space:]]+[A-Za-z0-9._-]+)/[redacted]/g' "$publish_log" >&2 echo '::error::Publication failed. Confirm the version is still unpublished and configure @lubab/madar Trusted Publishing for this GitHub repository, publish-next.yml, and the npm-next environment; do not add an npm token or remove provenance.' exit 1 fi sed -E 's/(npm_[A-Za-z0-9]{20,}|Bearer[[:space:]]+[A-Za-z0-9._-]+)/[redacted]/g' "$publish_log" + # Runs after a real publish. Holds `contents: write` to create the GitHub release, but no + # `id-token` -- it cannot publish to npm, only verify what `publish` already shipped and + # record it. Idempotent (create-or-edit), so rerunning this job after a failure never + # republishes an immutable npm version; it only redoes verification and release bookkeeping. + post_publish: + needs: [validate, publish] + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: write + env: + PACKAGE_NAME: '@lubab/madar' + PACKAGE_VERSION: ${{ needs.validate.outputs.version }} + RELEASE_COMMIT: ${{ needs.validate.outputs.commit }} + RELEASE_TAG: ${{ needs.validate.outputs.tag }} + QUALIFICATION_STATUS: ${{ needs.validate.outputs.qualification_status }} + + steps: + # Checking out here is safe even though it wasn't safe in `publish`: this job holds no + # `id-token`, so there is no npm publish credential in scope for a script to abuse, and + # the ref is the commit `validate` already proved is the tag commit -- never + # attacker-influenced input. + - name: Check out validated prerelease commit + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ needs.validate.outputs.commit }} + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '22' + + - name: Restore pre-publish npm dist-tags + shell: bash + env: + PRE_PUBLISH_DIST_TAGS: ${{ needs.publish.outputs.pre_publish_dist_tags }} + run: | + set -euo pipefail + printf '%s' "$PRE_PUBLISH_DIST_TAGS" > "$RUNNER_TEMP/npm-dist-tags-before.json" + node -e "const tags = require(process.argv[1]); if (!tags || typeof tags !== 'object' || Array.isArray(tags)) throw new Error('pre-publish dist-tags passed from the publish job are not valid JSON')" "$RUNNER_TEMP/npm-dist-tags-before.json" + - name: Verify published version and dist-tags shell: bash run: | @@ -290,7 +518,6 @@ jobs: - name: Create or update GitHub prerelease env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - QUALIFICATION_STATUS: ${{ steps.qualification.outputs.status }} shell: bash run: | set -euo pipefail diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts index d9160c07..0fc5b025 100644 --- a/tests/unit/release-pipeline.test.ts +++ b/tests/unit/release-pipeline.test.ts @@ -11,6 +11,15 @@ interface WorkflowStep { run?: string uses?: string with?: Record + env?: Record +} + +interface WorkflowJob { + environment?: string + if?: string + needs?: string | string[] + permissions?: Record + steps?: WorkflowStep[] } interface Workflow { @@ -23,15 +32,12 @@ interface Workflow { workflow_dispatch?: unknown } permissions?: Record - jobs?: Record + jobs?: Record } const classifierPath = resolve('.github/scripts/classify-release-tag.mjs') const nextReleaseStatePath = resolve('.github/scripts/verify-next-release-state.mjs') +const publishNextWorkflowPath = '.github/workflows/publish-next.yml' function runNode(script: string, args: string[], cwd = process.cwd()) { return spawnSync(process.execPath, [script, ...args], { @@ -45,14 +51,26 @@ function parseWorkflow(path: string): Workflow { return parse(readFileSync(resolve(path), 'utf8')) as Workflow } +function job(workflow: Workflow, jobName: string): WorkflowJob { + const target = workflow.jobs?.[jobName] + if (!target) { + throw new Error(`Missing job: ${jobName}`) + } + return target +} + function workflowStep(workflow: Workflow, jobName: string, stepName: string): WorkflowStep { - const step = workflow.jobs?.[jobName]?.steps?.find((candidate) => candidate.name === stepName) + const step = job(workflow, jobName).steps?.find((candidate) => candidate.name === stepName) if (!step) { throw new Error(`Missing ${jobName} workflow step: ${stepName}`) } return step } +function allSteps(workflow: Workflow, jobName: string): WorkflowStep[] { + return job(workflow, jobName).steps ?? [] +} + function withReleaseFixture( packageVersion: string, changelog: string, @@ -158,6 +176,28 @@ describe('next release state guards', () => { expect(result.stderr).toContain('Publishing is not allowed') }) + it('rejects workflow_dispatch now that manual dispatch is removed from the pipeline', () => { + const result = runNode(nextReleaseStatePath, [ + '--assert-event', + '--event', 'workflow_dispatch', + '--ref', 'refs/heads/next', + ]) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Publishing is not allowed') + }) + + it('allows a prerelease tag push', () => { + const result = runNode(nextReleaseStatePath, [ + '--assert-event', + '--event', 'push', + '--ref', 'refs/tags/v1.2.3-beta.1', + ]) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('event=allowed') + }) + it('rejects a tagged commit outside next', () => { const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-next-ancestor-')) @@ -251,57 +291,190 @@ describe('release workflow policy', () => { it('parses every release workflow as YAML', () => { expect(() => parseWorkflow('.github/workflows/ci.yml')).not.toThrow() expect(() => parseWorkflow('.github/workflows/release.yml')).not.toThrow() - expect(() => parseWorkflow('.github/workflows/publish-next.yml')).not.toThrow() + expect(() => parseWorkflow(publishNextWorkflowPath)).not.toThrow() }) - it('allows prerelease publication only from prerelease tags or manual dispatch', () => { - const workflow = parseWorkflow('.github/workflows/publish-next.yml') + it('has no workflow_dispatch trigger and no dispatch inputs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + expect(workflow.on?.workflow_dispatch).toBeUndefined() + expect(workflow.on?.pull_request).toBeUndefined() expect(workflow.on?.push?.branches).toBeUndefined() + }) + + it('triggers only on approved prerelease tag pushes', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + expect(workflow.on?.push?.tags).toEqual([ 'v*-beta.*', 'v*-rc.*', 'v*-next.*', ]) - expect(workflow.on?.pull_request).toBeUndefined() - expect(workflow.on?.workflow_dispatch).toBeDefined() - expect(workflow.jobs?.publish?.if).toContain("github.event_name == 'workflow_dispatch'") - expect(workflow.jobs?.publish?.if).toContain("startsWith(github.ref, 'refs/tags/')") }) - it('uses protected OIDC publication without a privileged dependency cache', () => { - const workflow = parseWorkflow('.github/workflows/publish-next.yml') - const checkout = workflowStep(workflow, 'publish', 'Check out exact prerelease commit') + it('derives the release tag solely from github.ref_name', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const validateJob = job(workflow, 'validate') + + expect(validateJob.if).toBe("startsWith(github.ref, 'refs/tags/')") + expect(String((validateJob as { env?: Record }).env?.RELEASE_TAG)) + .toBe('${{ github.ref_name }}') + }) + + it('defines exactly the three jobs validate, publish, post_publish with the right needs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(['post_publish', 'publish', 'validate']) + expect(job(workflow, 'validate').needs).toBeUndefined() + expect(job(workflow, 'publish').needs).toBe('validate') + expect(job(workflow, 'post_publish').needs).toEqual(['validate', 'publish']) + }) + + it('grants the npm-next protected environment only to the publish job', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + + expect(job(workflow, 'validate').environment).toBeUndefined() + expect(job(workflow, 'publish').environment).toBe('npm-next') + expect(job(workflow, 'post_publish').environment).toBeUndefined() + }) + + it('has no workflow-level id-token or contents: write permission', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + + expect(workflow.permissions).toEqual({ contents: 'read' }) + }) + + it('grants each job exactly the permissions it needs, and never combines id-token: write with contents: write', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + + expect(job(workflow, 'validate').permissions).toEqual({ contents: 'read' }) + expect(job(workflow, 'publish').permissions).toEqual({ contents: 'read', 'id-token': 'write' }) + expect(job(workflow, 'post_publish').permissions).toEqual({ contents: 'write' }) + + for (const [jobName, jobDef] of Object.entries(workflow.jobs ?? {})) { + const permissions = jobDef.permissions ?? {} + const hasIdToken = permissions['id-token'] === 'write' + const hasContentsWrite = permissions.contents === 'write' + expect(hasIdToken && hasContentsWrite, `${jobName} must not combine id-token: write with contents: write`).toBe(false) + } + }) + + it('keeps the publish job free of checkout, npm ci, tests, build, and repository scripts', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const steps = allSteps(workflow, 'publish') + + for (const step of steps) { + expect(step.uses ?? '', 'publish must not use actions/checkout').not.toMatch(/actions\/checkout/) + expect(step.run ?? '').not.toContain('npm ci') + expect(step.run ?? '').not.toContain('npm run test') + expect(step.run ?? '').not.toContain('npm run build') + expect(step.run ?? '').not.toMatch(/\.github\/scripts\//) + } + }) + + it('never runs a dependency cache in the publish job\'s Node setup', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) const setupNode = workflowStep(workflow, 'publish', 'Set up Node.js') - const npmClient = workflowStep(workflow, 'publish', 'Install pinned npm Trusted Publishing client') - const publish = workflowStep(workflow, 'publish', 'Publish prerelease with npm Trusted Publishing') - expect(workflow.permissions).toEqual({ contents: 'write', 'id-token': 'write' }) - expect(workflow.jobs?.publish?.environment).toBe('npm-next') - expect(checkout.with).toMatchObject({ - 'fetch-depth': 0, - 'persist-credentials': false, - }) - // Ref injection guard: checkout runs before any validation, so a ref derived from - // workflow_dispatch input could supply its own validation scripts and pass every gate. - // The ref must come from github.sha, which GitHub resolves from the workflow ref. - expect(checkout.with?.ref).toBe('${{ github.sha }}') - expect(String(checkout.with?.ref)).not.toContain('inputs.') - // The published artifact must be validated on a Node version the CI matrix tests. - expect(['20', '22']).toContain(String(setupNode.with?.['node-version'])) expect(setupNode.with).not.toHaveProperty('cache') - expect(npmClient.run).toContain('npm@12.0.2') - expect(npmClient.run).toContain('--ignore-scripts') - expect(npmClient.run).not.toContain('npm@latest') - expect(publish.run).toContain('npm publish --tag next --access public --provenance') - expect(publish.run).not.toContain('npm publish --access public') - expect(publish.run).not.toContain('NODE_AUTH_TOKEN') - }) - - it('keeps every required prerelease validation gate before publication', () => { - const workflow = parseWorkflow('.github/workflows/publish-next.yml') - const steps = workflow.jobs?.publish?.steps ?? [] - const publishIndex = steps.findIndex((step) => step.name === 'Publish prerelease with npm Trusted Publishing') + }) + + it('has no live publish command in validate or post_publish, only dry runs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + + for (const jobName of ['validate', 'post_publish']) { + const steps = allSteps(workflow, jobName) + for (const step of steps) { + const run = step.run ?? '' + if (run.includes('npm publish')) { + expect(run, `${jobName} step "${step.name}" must be a dry run`).toContain('--dry-run') + } + } + } + }) + + it('runs exactly one live publish command, in the publish job, as its final substantive step', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const allJobNames = Object.keys(workflow.jobs ?? {}) + + const livePublishSteps: { jobName: string; index: number; step: WorkflowStep }[] = [] + for (const jobName of allJobNames) { + const steps = allSteps(workflow, jobName) + steps.forEach((step, index) => { + const run = step.run ?? '' + if (run.includes('npm publish') && !run.includes('--dry-run')) { + livePublishSteps.push({ jobName, index, step }) + } + }) + } + + expect(livePublishSteps).toHaveLength(1) + const found = livePublishSteps[0] + if (!found) { + throw new Error('expected exactly one live publish step') + } + const { jobName, index, step } = found + expect(jobName).toBe('publish') + expect(step.name).toBe('Publish prerelease with npm Trusted Publishing') + expect(index).toBe(allSteps(workflow, 'publish').length - 1) + expect(step.run).toContain('npm publish "release-artifact/$TARBALL_NAME" --tag next --access public --provenance') + expect(step.run).not.toContain('npm publish --access public') + expect(step.run).not.toContain('NODE_AUTH_TOKEN') + }) + + it('publishes the exact downloaded tarball, never the working directory', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const publish = workflowStep(workflow, 'publish', 'Publish prerelease with npm Trusted Publishing') + + expect(publish.run).not.toMatch(/npm publish\s+\.\s/) + expect(publish.run).not.toMatch(/npm publish\s+--tag/) + expect(publish.run).toContain('"release-artifact/$TARBALL_NAME"') + }) + + it('records and verifies the release artifact SHA-256 across validate, publish, and post_publish', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const packStep = workflowStep(workflow, 'validate', 'Build release tarball and receipt') + const verifyStep = workflowStep(workflow, 'publish', "Verify downloaded artifact against the validate job's receipt") + + expect(packStep.run).toContain('sha256sum') + expect(job(workflow, 'validate').steps?.map((step) => step.name)).toContain('Upload release tarball artifact') + expect(verifyStep.run).toContain('EXPECTED_TARBALL_SHA256') + expect(verifyStep.run).toContain('sha256sum -c') + + const validateOutputs = (job(workflow, 'validate') as { outputs?: Record }).outputs + expect(String(validateOutputs?.tarball_sha256)).toContain('steps.pack.outputs.sha256') + }) + + it('names the uploaded artifact with the run id, run attempt, version, and commit', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const packStep = workflowStep(workflow, 'validate', 'Build release tarball and receipt') + + expect(packStep.run).toContain('github.run_id') + expect(packStep.run).toContain('github.run_attempt') + expect(packStep.run).toContain('PACKAGE_VERSION') + expect(packStep.run).toContain('RELEASE_COMMIT') + + const uploadStep = workflowStep(workflow, 'validate', 'Upload release tarball artifact') + expect(uploadStep.with?.name).toBe('${{ steps.pack.outputs.artifact_name }}') + expect(uploadStep.with).toHaveProperty('retention-days') + }) + + it('captures pre-publish dist-tags in publish and preserves latest in post_publish', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const captureStep = workflowStep(workflow, 'publish', 'Capture pre-publish npm dist-tags') + const verifyStep = workflowStep(workflow, 'post_publish', 'Verify published version and dist-tags') + + expect(captureStep.run).toContain('npm view "$PACKAGE_NAME" dist-tags --json') + const publishOutputs = (job(workflow, 'publish') as { outputs?: Record }).outputs + expect(String(publishOutputs?.pre_publish_dist_tags)).toContain('steps.capture-dist-tags.outputs.json') + expect(verifyStep.run).toContain('verify-next-release-state.mjs') + expect(verifyStep.run).toContain('--verify-publish') + }) + + it('keeps every required validation gate in validate before the release tarball is built', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const steps = allSteps(workflow, 'validate') + const packIndex = steps.findIndex((step) => step.name === 'Build release tarball and receipt') const requiredCommands = [ 'npm ci', 'npm run typecheck', @@ -315,22 +488,35 @@ describe('release workflow policy', () => { 'npm publish --dry-run --tag next', ] - expect(publishIndex).toBeGreaterThan(0) + expect(packIndex).toBeGreaterThan(0) for (const command of requiredCommands) { const commandIndex = steps.findIndex((step) => step.run?.includes(command)) expect(commandIndex, command).toBeGreaterThanOrEqual(0) - expect(commandIndex, command).toBeLessThan(publishIndex) + expect(commandIndex, command).toBeLessThan(packIndex) } - expect(workflowStep(workflow, 'publish', 'Run qualification validation when available').run) + expect(workflowStep(workflow, 'validate', 'Run qualification validation when available').run) .toContain('npm run qualify:validate') }) + it('runs qualify:validate deterministically: hard failure when present, notice when absent', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const step = workflowStep(workflow, 'validate', 'Run qualification validation when available') + + // The branch is chosen purely from whether qualify:validate exists in the checked-out + // commit's package.json -- there is no separate flag or date to flip. Once #681 merges + // qualify:validate onto next, the very next tag push takes the hard-fail branch. + expect(step.run).toContain("scripts['qualify:validate']") + expect(step.run).toContain('npm run qualify:validate') + expect(step.run).toContain("echo 'status=passed' >> \"$GITHUB_OUTPUT\"") + expect(step.run).toContain('qualify:validate is not present on this tag') + }) + it('classifies the GitHub releases on the correct channels', () => { const stableWorkflow = parseWorkflow('.github/workflows/release.yml') - const nextWorkflow = parseWorkflow('.github/workflows/publish-next.yml') + const nextWorkflow = parseWorkflow(publishNextWorkflowPath) const stableClassifier = workflowStep(stableWorkflow, 'release', 'Require stable release tag') - const prereleaseClassifier = workflowStep(nextWorkflow, 'publish', 'Validate prerelease tag and release files') - const githubPrerelease = workflowStep(nextWorkflow, 'publish', 'Create or update GitHub prerelease') + const prereleaseClassifier = workflowStep(nextWorkflow, 'validate', 'Validate prerelease tag and release files') + const githubPrerelease = workflowStep(nextWorkflow, 'post_publish', 'Create or update GitHub prerelease') expect(stableClassifier.run).toContain('--expect stable') expect(prereleaseClassifier.run).toContain('--expect prerelease') @@ -338,6 +524,32 @@ describe('release workflow policy', () => { expect(workflowStep(stableWorkflow, 'release', 'Create GitHub release').run).not.toContain('--prerelease') }) + it('uses a 40-character commit SHA with a version comment for every action in the privileged workflow, and rejects mutable refs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const raw = readFileSync(resolve(publishNextWorkflowPath), 'utf8') + const usesLines = raw + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith('uses:')) + + expect(usesLines.length).toBeGreaterThan(0) + + const mutableRefPattern = /@(v\d+|main|master)(\s|$)/ + for (const line of usesLines) { + expect(line, line).not.toMatch(mutableRefPattern) + const match = line.match(/uses:\s*([^@]+)@([0-9a-f]{40})(?:\s+#\s*(v[\w.]+))?/) + expect(match, `${line} must pin a 40-character SHA with a # vX.Y.Z comment`).not.toBeNull() + expect(match?.[3], `${line} must have a readable version comment`).toBeTruthy() + } + + for (const jobDef of Object.values(workflow.jobs ?? {})) { + for (const step of jobDef.steps ?? []) { + if (!step.uses) continue + expect(step.uses).toMatch(/@[0-9a-f]{40}$/) + } + } + }) + it('keeps release documentation consistent with both channels', () => { const releaseDoc = readFileSync(resolve('docs/release.md'), 'utf8') const contributing = readFileSync(resolve('CONTRIBUTING.md'), 'utf8') From a7140d4f493827a7a92ceb2f5c688fcdd12467af Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Wed, 12 Aug 2026 22:20:55 +0400 Subject: [PATCH 4/6] fix(release): make the qualification gate fail closed, not just fail present Lead review found the qualify:validate gate was only half-deterministic: running and hard-failing when the script exists covers one direction, but if the script is ever removed after landing (bad refactor, lost merge, a dependency bump rewriting package.json) the old presence-only check silently fell back to the notice path and would publish anyway -- the exact "permanently optional gate" the requirement forbids. Add a second, independent signal via new .github/scripts/check-qualification-gate.mjs: compares whether docs/qualification/ (the qualification contract, landing with #681 in the same merge as the script) exists against whether package.json still defines qualify:validate (the contract's validator). script present -> run: execute for real, hard-fail on non-zero exit script absent, contract present -> missing: hard fail with a message naming exactly what's wrong script absent, contract absent -> notice: today's ordinary state publish-next.yml's qualification step now delegates to this script instead of inlining the presence check. Both fail-closed directions are unit tested directly against the script (27 -> 48 tests total in this remediation round). Refs #687, #688. Co-Authored-By: Claude Opus 5 --- .github/scripts/check-qualification-gate.mjs | 72 ++++++++++++++++++++ .github/workflows/publish-next.yml | 38 +++++++---- tests/unit/release-pipeline.test.ts | 72 ++++++++++++++++++-- 3 files changed, 161 insertions(+), 21 deletions(-) create mode 100644 .github/scripts/check-qualification-gate.mjs diff --git a/.github/scripts/check-qualification-gate.mjs b/.github/scripts/check-qualification-gate.mjs new file mode 100644 index 00000000..b41a501c --- /dev/null +++ b/.github/scripts/check-qualification-gate.mjs @@ -0,0 +1,72 @@ +import { existsSync, readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +function fail(message) { + throw new Error(message) +} + +// Two independent signals decide the gate, not one, so an absent script can never stay +// optional forever: +// - contractPresent: does docs/qualification/ (the qualification contract) exist on the +// checked-out commit? It does not exist on `next` today; it lands with #681, in the same +// merge that adds the qualify:validate script. +// - scriptPresent: does package.json define a qualify:validate script? +// +// scriptPresent -> 'run': execute it for real; the caller must hard-fail on a non-zero exit. +// !scriptPresent && contractPresent -> 'missing': the contract has been declared mandatory but +// its validator is gone. This must hard-fail -- silently downgrading to a notice here is +// exactly the "permanently optional gate" this mechanism exists to prevent. +// !scriptPresent && !contractPresent -> 'notice': ordinary pre-#681 state; record and skip. +export function decideQualificationGate({ contractPresent, scriptPresent }) { + if (scriptPresent) { + return 'run' + } + if (contractPresent) { + return 'missing' + } + return 'notice' +} + +function readScriptPresence(packageJsonPath) { + const pkg = JSON.parse(readFileSync(packageJsonPath, 'utf8')) + const scripts = pkg.scripts ?? {} + return Boolean(scripts['qualify:validate']) +} + +function parseArguments(args) { + const options = { cwd: '.' } + const cwdIndex = args.indexOf('--cwd') + if (cwdIndex !== -1) { + const value = args[cwdIndex + 1] + if (!value || value.startsWith('--')) { + fail('--cwd requires a value') + } + options.cwd = value + } + return options +} + +export function runCli(args) { + const { cwd } = parseArguments(args) + const contractPresent = existsSync(resolve(cwd, 'docs/qualification')) + const scriptPresent = readScriptPresence(resolve(cwd, 'package.json')) + const decision = decideQualificationGate({ contractPresent, scriptPresent }) + + if (decision === 'missing') { + fail('docs/qualification/ (the qualification contract) is present on this commit, but the qualify:validate script is missing from package.json. This gate is mandatory once the contract exists; restore the script before releasing.') + } + + console.log(`decision=${decision}`) +} + +const isCli = process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isCli) { + try { + runCli(process.argv.slice(2)) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error(`Qualification gate check failed: ${message}`) + process.exitCode = 1 + } +} diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml index 3b64d953..6d6b852e 100644 --- a/.github/workflows/publish-next.yml +++ b/.github/workflows/publish-next.yml @@ -136,26 +136,34 @@ jobs: - name: Verify release hygiene run: npm run release:verify - # qualify:validate is deterministic, not a standing optional gate. It runs and hard-fails - # this job whenever the script exists on the checked-out commit. Right now it does not - # exist on `next`, so this records a notice instead. That absence is the *only* thing - # standing between this branch and a required run: qualify:validate lands with #681, and - # from the very next tag push that includes that merge, package.json on this exact - # commit will contain the script, `npm run qualify:validate` will execute for real, and a - # non-zero exit here fails the job via `set -euo pipefail`. Nothing else has to change -- - # no flag, no date, no follow-up PR to flip a switch. + # qualify:validate has two independent signals, not one, so absence can never silently + # stay optional forever. check-qualification-gate.mjs compares docs/qualification/ (the + # qualification *contract*, landing with #681) against whether package.json still + # defines the qualify:validate script (the contract's *validator*), and fails closed if + # the contract exists without its validator instead of downgrading to a notice. See that + # script for the full decision table. Because both land together in #681, the very next + # tag push that is an ancestor of that merge takes the hard-fail-when-missing branch + # automatically -- no flag, no date, no follow-up PR to flip a switch. - name: Run qualification validation when available id: qualification shell: bash run: | set -euo pipefail - if node -e "const scripts = require('./package.json').scripts ?? {}; process.exit(scripts['qualify:validate'] ? 0 : 1)"; then - npm run qualify:validate - echo 'status=passed' >> "$GITHUB_OUTPUT" - else - echo '::notice title=Qualification validation::qualify:validate is not present on this tag; qualification was skipped.' - echo 'status=skipped (script not present)' >> "$GITHUB_OUTPUT" - fi + decision="$(node .github/scripts/check-qualification-gate.mjs)" + case "$decision" in + decision=run) + npm run qualify:validate + echo 'status=passed' >> "$GITHUB_OUTPUT" + ;; + decision=notice) + echo '::notice title=Qualification validation::qualify:validate is not present on this tag; qualification was skipped.' + echo 'status=skipped (script not present)' >> "$GITHUB_OUTPUT" + ;; + *) + echo "::error::Unexpected qualification gate decision: $decision" + exit 1 + ;; + esac - name: Validate npm package contents run: npm pack --dry-run diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts index 0fc5b025..7d474281 100644 --- a/tests/unit/release-pipeline.test.ts +++ b/tests/unit/release-pipeline.test.ts @@ -1,5 +1,5 @@ import { execFileSync, spawnSync } from 'node:child_process' -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' @@ -37,6 +37,7 @@ interface Workflow { const classifierPath = resolve('.github/scripts/classify-release-tag.mjs') const nextReleaseStatePath = resolve('.github/scripts/verify-next-release-state.mjs') +const qualificationGatePath = resolve('.github/scripts/check-qualification-gate.mjs') const publishNextWorkflowPath = '.github/workflows/publish-next.yml' function runNode(script: string, args: string[], cwd = process.cwd()) { @@ -71,6 +72,24 @@ function allSteps(workflow: Workflow, jobName: string): WorkflowStep[] { return job(workflow, jobName).steps ?? [] } +function withQualificationFixture( + { contractPresent, scriptPresent }: { contractPresent: boolean; scriptPresent: boolean }, + runAssertion: (fixtureDir: string) => void, +): void { + const fixtureDir = mkdtempSync(join(tmpdir(), 'madar-qualification-gate-')) + + try { + const scripts = scriptPresent ? { 'qualify:validate': 'node -e "process.exit(0)"' } : {} + writeFileSync(join(fixtureDir, 'package.json'), JSON.stringify({ scripts })) + if (contractPresent) { + mkdirSync(join(fixtureDir, 'docs', 'qualification'), { recursive: true }) + } + runAssertion(fixtureDir) + } finally { + rmSync(fixtureDir, { recursive: true, force: true }) + } +} + function withReleaseFixture( packageVersion: string, changelog: string, @@ -287,6 +306,45 @@ describe('next release state guards', () => { }) }) +describe('qualification gate', () => { + it('runs when the qualify:validate script is present, regardless of the contract', () => { + withQualificationFixture({ contractPresent: false, scriptPresent: true }, (fixtureDir) => { + const result = runNode(qualificationGatePath, [], fixtureDir) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('decision=run') + }) + }) + + it('records a notice when neither the qualification contract nor its script are present', () => { + withQualificationFixture({ contractPresent: false, scriptPresent: false }, (fixtureDir) => { + const result = runNode(qualificationGatePath, [], fixtureDir) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('decision=notice') + }) + }) + + it('hard-fails when the qualification contract exists but its validator script is missing', () => { + withQualificationFixture({ contractPresent: true, scriptPresent: false }, (fixtureDir) => { + const result = runNode(qualificationGatePath, [], fixtureDir) + + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('docs/qualification/') + expect(result.stderr).toContain('is present on this commit, but the qualify:validate script is missing') + }) + }) + + it('still runs when both the contract and the script are present', () => { + withQualificationFixture({ contractPresent: true, scriptPresent: true }, (fixtureDir) => { + const result = runNode(qualificationGatePath, [], fixtureDir) + + expect(result.status).toBe(0) + expect(result.stdout).toContain('decision=run') + }) + }) +}) + describe('release workflow policy', () => { it('parses every release workflow as YAML', () => { expect(() => parseWorkflow('.github/workflows/ci.yml')).not.toThrow() @@ -498,16 +556,18 @@ describe('release workflow policy', () => { .toContain('npm run qualify:validate') }) - it('runs qualify:validate deterministically: hard failure when present, notice when absent', () => { + it('delegates the qualification gate decision to check-qualification-gate.mjs and acts on all three outcomes', () => { const workflow = parseWorkflow(publishNextWorkflowPath) const step = workflowStep(workflow, 'validate', 'Run qualification validation when available') - // The branch is chosen purely from whether qualify:validate exists in the checked-out - // commit's package.json -- there is no separate flag or date to flip. Once #681 merges - // qualify:validate onto next, the very next tag push takes the hard-fail branch. - expect(step.run).toContain("scripts['qualify:validate']") + // The gate's own present/missing/notice decision table is exercised directly against + // check-qualification-gate.mjs in the "qualification gate" describe block above; this test + // only asserts the workflow step wires that decision to the right action. + expect(step.run).toContain('check-qualification-gate.mjs') + expect(step.run).toContain('decision=run') expect(step.run).toContain('npm run qualify:validate') expect(step.run).toContain("echo 'status=passed' >> \"$GITHUB_OUTPUT\"") + expect(step.run).toContain('decision=notice') expect(step.run).toContain('qualify:validate is not present on this tag') }) From f71e1c97626b2dab06127dc8f8559e4f6a0b3fb4 Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Thu, 13 Aug 2026 00:20:04 +0400 Subject: [PATCH 5/6] fix(release): job-output dataflow, tag classifier job, and raw vitest-log gate Three operational defects in PR #688's release pipeline, confirmed by review before this change: - validate's job outputs read `version`/`commit` from `env.PACKAGE_VERSION` and `env.RELEASE_COMMIT`, which are written at runtime via `>> "$GITHUB_ENV"` and do not reliably populate `jobs..outputs`. `publish` could receive empty version/commit. Fixed by giving the producing steps stable ids (`release-meta`, `release-commit`), writing version/tag/commit to `$GITHUB_OUTPUT` as well as `$GITHUB_ENV`, and mapping the job outputs from `steps..outputs.`. - release.yml's job-level `if: !contains(github.ref_name, '-')` made an unsupported prerelease tag (e.g. `v0.33.0-alpha.1`) match this workflow's `v*` trigger, skip silently because it contains a hyphen, and match no trigger in publish-next.yml either -- the tag vanished with no failed run anywhere. Split into a `classify` job that runs for every `v*` tag and fails visibly on anything classify-release-tag.mjs does not recognize as stable or an approved prerelease, and a `release` job that only runs once `classify` succeeds and reports `stable`. Workflow-level `contents: write` is removed; `classify` is `contents: read` only. - vitest's forks pool (maxWorkers: 4, no retry configured) can respawn a worker mid-run when one fails to start or stops responding, and the run's own tally can still count the respawned worker's files as passed -- a green summary and exit 0 are not sufficient release evidence. Added .github/scripts/assert-clean-vitest-log.mjs, which scans raw captured log text for "Failed to start forks worker" and "Timeout waiting for worker to respond" independently of vitest's own exit code, and fails closed on a missing or unreadable log instead of treating it as clean. Wired into validate's test:run and test:coverage steps with explicit status capture (not `&&`) so the scanner always runs even when the test command itself fails; raw logs are uploaded as a bounded-retention diagnostic artifact only on failure, and publish never downloads it. Adds targeted tests proving each fix: job-output dataflow (including a synthetic fixture proving the check catches an env.*-sourced output and a step missing its GITHUB_OUTPUT write), the classify step's real behavior across all four tag classes (stable, the three approved prerelease forms, and four unsupported forms), and the vitest-log scanner (signature detection, counting, both-signatures, no false-positives, missing/unreadable-file handling, and a control case). No tag, npm package, or GitHub release was published or created at any point. PR #688 stays a draft. Refs #654, #687. --- .github/scripts/assert-clean-vitest-log.mjs | 144 +++++++++ .github/workflows/publish-next.yml | 84 +++++- .github/workflows/release.yml | 70 ++++- tests/unit/assert-clean-vitest-log.test.ts | 189 ++++++++++++ tests/unit/release-pipeline.test.ts | 309 ++++++++++++++++++++ 5 files changed, 783 insertions(+), 13 deletions(-) create mode 100644 .github/scripts/assert-clean-vitest-log.mjs create mode 100644 tests/unit/assert-clean-vitest-log.test.ts diff --git a/.github/scripts/assert-clean-vitest-log.mjs b/.github/scripts/assert-clean-vitest-log.mjs new file mode 100644 index 00000000..d01fdb68 --- /dev/null +++ b/.github/scripts/assert-clean-vitest-log.mjs @@ -0,0 +1,144 @@ +// Vitest's forks pool can respawn a worker mid-run when one fails to start or stops +// responding. The run's own summary and exit code do not see that respawn -- they only see +// whatever the respawned worker eventually reported -- so a file that logged one of these +// signatures can still be tallied as passed. A green summary and exit code 0 are therefore not +// sufficient release evidence on their own; this script reads the raw log text and fails +// closed when either signature appears, independent of what vitest itself reported. +// +// Deliberately narrow: this is not a general log-scanning framework. It knows exactly two +// signatures and does nothing else. +import { existsSync, readFileSync, statSync } from 'node:fs' +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +export const WORKER_FAILURE_SIGNATURES = [ + 'Failed to start forks worker', + 'Timeout waiting for worker to respond', +] + +function fail(message) { + throw new Error(message) +} + +/** + * Scans a single log's text for every known worker-start failure signature. + * Returns one entry per signature that appears, each with its occurrence count and the + * matching lines (1-indexed), so a failure can be reported precisely. + */ +export function scanLogText(path, content) { + const lines = content.split(/\r?\n/) + const matches = [] + + for (const signature of WORKER_FAILURE_SIGNATURES) { + const matchingLines = [] + lines.forEach((line, index) => { + if (line.includes(signature)) { + matchingLines.push({ lineNumber: index + 1, text: line }) + } + }) + if (matchingLines.length > 0) { + matches.push({ signature, count: matchingLines.length, lines: matchingLines }) + } + } + + return { path, matches } +} + +/** + * Reads and scans every given log path. Fails closed: a missing or unreadable path is a + * thrown error, never a silently "clean" result. + */ +export function assertCleanVitestLogs(paths, io = {}) { + const readFile = io.readFile ?? ((path) => readFileSync(path, 'utf8')) + const exists = io.exists ?? existsSync + const stat = io.stat ?? statSync + + if (!Array.isArray(paths) || paths.length === 0) { + fail('At least one log path is required') + } + + const results = [] + + for (const path of paths) { + if (!exists(path)) { + fail(`Log file not found: ${path}`) + } + + let isDirectory = false + try { + isDirectory = stat(path).isDirectory() + } catch { + // If stat itself fails, the read below will surface a precise error instead. + } + if (isDirectory) { + fail(`Log path is a directory, not a file: ${path}`) + } + + let content + try { + content = readFile(path) + } catch (error) { + const reason = error instanceof Error ? error.message : String(error) + fail(`Unable to read log file ${path}: ${reason}`) + } + + results.push(scanLogText(path, content)) + } + + const hasFailure = results.some((result) => result.matches.length > 0) + return { hasFailure, results } +} + +export function formatReport({ hasFailure, results }) { + const lines = [] + + for (const result of results) { + if (result.matches.length === 0) { + lines.push(`clean: ${result.path}`) + continue + } + for (const match of result.matches) { + const occurrences = match.count === 1 ? 'occurrence' : 'occurrences' + lines.push(`SIGNATURE DETECTED in ${result.path}: "${match.signature}" (${match.count} ${occurrences})`) + for (const { lineNumber, text } of match.lines) { + lines.push(` ${result.path}:${lineNumber}: ${text}`) + } + } + } + + lines.push( + hasFailure + ? 'vitest log scan FAILED: absorbed worker-start failure signature(s) detected' + : 'vitest log scan passed: no absorbed worker-start failure signatures detected', + ) + + return lines.join('\n') +} + +export function runCli(argv) { + if (argv.length === 0) { + fail('Usage: assert-clean-vitest-log.mjs [log-path...]') + } + + const outcome = assertCleanVitestLogs(argv) + const report = formatReport(outcome) + + if (outcome.hasFailure) { + console.error(report) + process.exitCode = 1 + return + } + + console.log(report) +} + +const isCli = process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isCli) { + try { + runCli(process.argv.slice(2)) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error(`assert-clean-vitest-log failed: ${message}`) + process.exitCode = 1 + } +} diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml index 6d6b852e..77e030e0 100644 --- a/.github/workflows/publish-next.yml +++ b/.github/workflows/publish-next.yml @@ -36,9 +36,9 @@ jobs: RELEASE_TAG: ${{ github.ref_name }} outputs: - version: ${{ env.PACKAGE_VERSION }} - commit: ${{ env.RELEASE_COMMIT }} - tag: ${{ env.RELEASE_TAG }} + version: ${{ steps.release-meta.outputs.version }} + commit: ${{ steps.release-commit.outputs.commit }} + tag: ${{ steps.release-meta.outputs.tag }} tarball_name: ${{ steps.pack.outputs.tarball }} tarball_sha256: ${{ steps.pack.outputs.sha256 }} artifact_name: ${{ steps.pack.outputs.artifact_name }} @@ -62,7 +62,13 @@ jobs: - name: Install pinned npm Trusted Publishing client run: npm install --global --ignore-scripts --no-audit --no-fund --cache "$RUNNER_TEMP/npm-cli-bootstrap-cache" npm@12.0.2 + # Stable id so the job `outputs:` block can map version/tag from `steps.release-meta. + # outputs.*` instead of `env.*`. Values written at runtime via `>> "$GITHUB_ENV"` do not + # reliably populate `jobs..outputs`, so those two paths are kept for different + # purposes here: `$GITHUB_ENV` for later steps in this same job, `$GITHUB_OUTPUT` for the + # job-level output other jobs consume through `needs.validate.outputs`. - name: Validate prerelease tag and release files + id: release-meta shell: bash run: | set -euo pipefail @@ -78,8 +84,15 @@ jobs: version="${RELEASE_TAG#v}" printf 'PACKAGE_VERSION=%s\n' "$version" >> "$GITHUB_ENV" + { + echo "version=$version" + echo "tag=$RELEASE_TAG" + } >> "$GITHUB_OUTPUT" + # Stable id so `outputs.commit` maps from `steps.release-commit.outputs.commit` -- see the + # note on the `release-meta` step above for why this must not be `env.RELEASE_COMMIT`. - name: Prove exact tag commit and next ancestry + id: release-commit shell: bash run: | set -euo pipefail @@ -101,6 +114,7 @@ jobs: --commit "$head_commit" \ --branch origin/next printf 'RELEASE_COMMIT=%s\n' "$head_commit" >> "$GITHUB_ENV" + echo "commit=$head_commit" >> "$GITHUB_OUTPUT" - name: Verify version is not already published shell: bash @@ -118,11 +132,71 @@ jobs: - name: Typecheck run: npm run typecheck + # A green vitest summary is not sufficient release evidence on its own: the forks pool + # (maxWorkers: 4, no retry configured) can respawn a worker mid-run when one fails to + # start or stops responding, and the run's own tally can still count the respawned + # worker's files as passed. Capture the raw output, then scan the raw text independently + # of vitest's own exit code and summary. `set -e` is deliberately off in this step so a + # non-zero `test:run` exit does not short-circuit before the scanner runs: both statuses + # are captured explicitly and the step fails if either one is non-zero. - name: Run tests - run: npm run test:run + shell: bash + run: | + set +e + set -u + set -o pipefail + npm run test:run 2>&1 | tee "$RUNNER_TEMP/test-run.log" + test_status=${PIPESTATUS[0]} + + node .github/scripts/assert-clean-vitest-log.mjs "$RUNNER_TEMP/test-run.log" + scanner_status=$? + + if [ "$test_status" -ne 0 ]; then + echo "::error::npm run test:run exited $test_status" + fi + if [ "$scanner_status" -ne 0 ]; then + echo "::error::vitest log scan detected an absorbed worker-start failure signature in test-run.log" + fi + if [ "$test_status" -ne 0 ] || [ "$scanner_status" -ne 0 ]; then + exit 1 + fi - name: Run tests with coverage thresholds - run: npm run test:coverage + shell: bash + run: | + set +e + set -u + set -o pipefail + npm run test:coverage 2>&1 | tee "$RUNNER_TEMP/test-coverage.log" + test_status=${PIPESTATUS[0]} + + node .github/scripts/assert-clean-vitest-log.mjs "$RUNNER_TEMP/test-coverage.log" + scanner_status=$? + + if [ "$test_status" -ne 0 ]; then + echo "::error::npm run test:coverage exited $test_status" + fi + if [ "$scanner_status" -ne 0 ]; then + echo "::error::vitest log scan detected an absorbed worker-start failure signature in test-coverage.log" + fi + if [ "$test_status" -ne 0 ] || [ "$scanner_status" -ne 0 ]; then + exit 1 + fi + + # Diagnostic only: captures the raw logs for post-mortem when either test step above + # failed (a real test failure or a detected worker-start signature). Short, bounded + # retention and a name unique to this run/attempt -- this is not release evidence to keep + # around, only a debugging aid. `publish` never downloads this artifact. + - name: Upload raw vitest logs on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: vitest-diagnostic-logs-${{ github.run_id }}-${{ github.run_attempt }} + path: | + ${{ runner.temp }}/test-run.log + ${{ runner.temp }}/test-coverage.log + if-no-files-found: ignore + retention-days: 7 - name: Build run: npm run build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e9db46d9..29c8952e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,31 +5,85 @@ on: tags: - 'v*' +# No workflow-level `contents: write`. `classify` only reads the repository to determine which +# channel a tag belongs to and must never hold a credential that could create or edit a release; +# `release` declares `contents: write` for itself, and only runs once `classify` has already +# proven the tag is a well-formed stable tag. permissions: - contents: write + contents: read concurrency: group: release-${{ github.ref }} cancel-in-progress: false jobs: + # Runs for every `v*` tag, including malformed and unapproved forms -- that is the point. + # Before this job existed, a job-level `if: !contains(github.ref_name, '-')` made an + # unsupported prerelease form like `v0.33.0-alpha.1` match this workflow's `v*` trigger, get + # silently skipped here because it contains a hyphen, and then match no trigger in + # `publish-next.yml` either (that workflow only matches `-beta.*`/`-rc.*`/`-next.*`). The tag + # vanished with no failed run anywhere. `classify` closes that gap: it is the one job that + # always runs on a `v*` push, and it fails loudly on anything classify-release-tag.mjs does + # not recognize as either a stable or an approved prerelease tag. + classify: + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + outputs: + channel: ${{ steps.classify.outputs.channel }} + steps: + - name: Check out tagged commit + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.sha }} + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '22' + + - name: Classify release tag + id: classify + shell: bash + run: | + set -euo pipefail + output="$(node .github/scripts/classify-release-tag.mjs --tag "$GITHUB_REF_NAME")" + channel="$(printf '%s\n' "$output" | sed -n 's/^channel=//p')" + if [[ -z "$channel" ]]; then + echo "::error::classify-release-tag.mjs did not emit a channel for $GITHUB_REF_NAME" + exit 1 + fi + echo "channel=$channel" >> "$GITHUB_OUTPUT" + + # Stable channel only. Skips cleanly (not a failure) for an approved prerelease tag, which + # `publish-next.yml` handles instead. `classify` above already rejected anything that is + # neither a stable nor an approved prerelease tag, so reaching this `if` with `channel == + # 'stable'` false always means a legitimate prerelease, never a silently-ignored malformed tag. + # + # `success()` is explicit and required here: a job's own `if:` condition replaces the implicit + # "only run when every job in `needs` succeeded" check rather than adding to it, so without + # this a `classify` failure would not, by itself, stop `release` from evaluating (an empty + # `outputs.channel` would just compare false against 'stable' -- accidentally safe here, but + # not a guarantee worth relying on). release: - # Stable channel only. A SemVer prerelease tag always contains '-', so beta/rc/next - # tags skip this job cleanly and are handled by publish-next.yml instead. Skipping - # rather than failing keeps every legitimate prerelease from leaving a red run in the - # Actions history; malformed stable tags still fail loudly in the step below. - if: ${{ !contains(github.ref_name, '-') }} + needs: classify + if: success() && needs.classify.outputs.channel == 'stable' runs-on: ubuntu-latest timeout-minutes: 20 + permissions: + contents: write steps: - name: Check out repository - uses: actions/checkout@v7 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 + persist-credentials: false - name: Set up Node.js - uses: actions/setup-node@v7 + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '20' cache: npm diff --git a/tests/unit/assert-clean-vitest-log.test.ts b/tests/unit/assert-clean-vitest-log.test.ts new file mode 100644 index 00000000..ecbd93a3 --- /dev/null +++ b/tests/unit/assert-clean-vitest-log.test.ts @@ -0,0 +1,189 @@ +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' + +import { spawnSync } from 'node:child_process' +import { describe, expect, it } from 'vitest' + +// Invoked as a subprocess throughout, the same way the workflow calls it and the same way this +// test file's sibling (release-pipeline.test.ts) exercises the other `.github/scripts/*.mjs` +// tools -- the repo's tsconfig has no `allowJs`, so these plain ESM scripts are deliberately +// treated as opaque CLIs rather than statically imported into typechecked test code. +const scannerPath = resolve('.github/scripts/assert-clean-vitest-log.mjs') + +function runScanner(paths: string[]) { + return spawnSync(process.execPath, [scannerPath, ...paths], { encoding: 'utf8', stdio: 'pipe' }) +} + +function withTempDir(run: (dir: string) => void): void { + const dir = mkdtempSync(join(tmpdir(), 'madar-vitest-log-')) + try { + run(dir) + } finally { + rmSync(dir, { recursive: true, force: true }) + } +} + +const CLEAN_LOG = [ + ' RUN v4.1.5 /repo', + '', + ' ✓ tests/unit/example.test.ts (3 tests) 12ms', + '', + ' Test Files 1 passed (1)', + ' Tests 3 passed (3)', + '', +].join('\n') + +const GREEN_SUMMARY_WITH_SIGNATURE = [ + ' RUN v4.1.5 /repo', + '[vitest-pool-workers] Failed to start forks worker for tests/unit/flaky.test.ts, respawning', + '', + ' ✓ tests/unit/flaky.test.ts (2 tests) 9ms', + '', + ' Test Files 1 passed (1)', + ' Tests 2 passed (2)', + '', +].join('\n') + +const SIMILAR_BUT_UNRELATED_LOG = [ + 'Failed to start the dev server worker pool warmup', + 'Worker responded after a short delay, no timeout occurred', + 'forks worker started successfully', +].join('\n') + +describe('assert-clean-vitest-log CLI', () => { + it('exits 0 for a clean log on disk', () => { + withTempDir((dir) => { + const logPath = join(dir, 'clean.log') + writeFileSync(logPath, CLEAN_LOG) + + const result = runScanner([logPath]) + expect(result.status).toBe(0) + expect(result.stdout).toContain('vitest log scan passed') + }) + }) + + it('does not false-positive on similar-but-unrelated text', () => { + withTempDir((dir) => { + const logPath = join(dir, 'unrelated.log') + writeFileSync(logPath, SIMILAR_BUT_UNRELATED_LOG) + + const result = runScanner([logPath]) + expect(result.status).toBe(0) + expect(result.stdout).toContain('vitest log scan passed') + }) + }) + + it('exits non-zero and reports file/signature/count for a green-summary log with the injected signature', () => { + withTempDir((dir) => { + const logPath = join(dir, 'signature.log') + writeFileSync(logPath, GREEN_SUMMARY_WITH_SIGNATURE) + + const result = runScanner([logPath]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain(logPath) + expect(result.stderr).toContain('Failed to start forks worker') + expect(result.stderr).toContain('1 occurrence') + }) + }) + + it.each([ + 'Failed to start forks worker', + 'Timeout waiting for worker to respond', + ])('exits non-zero for the signature "%s" individually', (signature) => { + withTempDir((dir) => { + const logPath = join(dir, 'each.log') + writeFileSync(logPath, `clean line\n${signature} for tests/unit/x.test.ts\nclean line\n`) + + const result = runScanner([logPath]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain(signature) + }) + }) + + it('exits non-zero and reports both signatures with independent counts when both are present', () => { + withTempDir((dir) => { + const logPath = join(dir, 'both.log') + writeFileSync( + logPath, + [ + 'Failed to start forks worker (1)', + 'Failed to start forks worker (2)', + 'Timeout waiting for worker to respond (1)', + ].join('\n'), + ) + + const result = runScanner([logPath]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Failed to start forks worker" (2 occurrences)') + expect(result.stderr).toContain('Timeout waiting for worker to respond" (1 occurrence)') + }) + }) + + it('fails closed when the log file does not exist, rather than treating it as clean', () => { + withTempDir((dir) => { + const result = runScanner([join(dir, 'does-not-exist.log')]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Log file not found') + }) + }) + + it('fails closed when given an unreadable input (a directory) instead of a file', () => { + withTempDir((dir) => { + const subdir = join(dir, 'not-a-file') + mkdirSync(subdir) + + const result = runScanner([subdir]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('directory, not a file') + }) + }) + + it('requires at least one log path', () => { + const result = runScanner([]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Usage:') + }) + + it('scans every path given and fails if any one of them contains a signature', () => { + withTempDir((dir) => { + const cleanPath = join(dir, 'test-run.log') + const dirtyPath = join(dir, 'test-coverage.log') + writeFileSync(cleanPath, CLEAN_LOG) + writeFileSync(dirtyPath, GREEN_SUMMARY_WITH_SIGNATURE) + + const result = runScanner([cleanPath, dirtyPath]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain(dirtyPath) + }) + }) + + it('passes when every given path is clean', () => { + withTempDir((dir) => { + const runPath = join(dir, 'test-run.log') + const coveragePath = join(dir, 'test-coverage.log') + writeFileSync(runPath, CLEAN_LOG) + writeFileSync(coveragePath, CLEAN_LOG) + + const result = runScanner([runPath, coveragePath]) + expect(result.status).toBe(0) + }) + }) + + it('control: proves the scanner keys on the exact signature text, not incidental log noise', () => { + withTempDir((dir) => { + const controlPath = join(dir, 'control.log') + // Injected verbatim, the same way vitest's forks pool itself emits it into captured + // output. If this test ever stops failing, the scanner's detection logic is broken, not + // just its wiring. + writeFileSync( + controlPath, + 'stdout | tests/unit/some.test.ts\nFailed to start forks worker, retrying with a fresh pool\n', + ) + + const result = runScanner([controlPath]) + expect(result.status).not.toBe(0) + expect(result.stderr).toContain('Failed to start forks worker') + }) + }) +}) diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts index 7d474281..9da604d2 100644 --- a/tests/unit/release-pipeline.test.ts +++ b/tests/unit/release-pipeline.test.ts @@ -7,6 +7,8 @@ import { describe, expect, it } from 'vitest' import { parse } from 'yaml' interface WorkflowStep { + id?: string + if?: string name?: string run?: string uses?: string @@ -18,6 +20,7 @@ interface WorkflowJob { environment?: string if?: string needs?: string | string[] + outputs?: Record permissions?: Record steps?: WorkflowStep[] } @@ -624,3 +627,309 @@ describe('release workflow policy', () => { expect(contributing).toContain('reviewed `next` → `main` pull request') }) }) + +// --------------------------------------------------------------------------------------------- +// A: job outputs must map from step outputs, never from `env.*`. `env.PACKAGE_VERSION` and +// `env.RELEASE_COMMIT` are written at runtime via `>> "$GITHUB_ENV"`, which does not reliably +// populate `jobs..outputs` -- `publish` could receive empty version/commit. These tests +// prove the dataflow itself, not just that an `outputs:` block exists. +// --------------------------------------------------------------------------------------------- +describe('validate job output dataflow', () => { + function assertOutputMapsFromStep( + workflow: Workflow, + jobName: string, + outputName: string, + expectedStepId: string, + ): void { + const targetJob = job(workflow, jobName) + const rawValue = String(targetJob.outputs?.[outputName] ?? '').trim() + + expect(rawValue, `${jobName}.outputs.${outputName} must not be empty`).not.toBe('') + expect(rawValue, `${jobName}.outputs.${outputName} must not read from env.*`).not.toMatch(/env\./) + expect( + rawValue, + `${jobName}.outputs.${outputName} must map from steps.${expectedStepId}.outputs.*`, + ).toMatch(new RegExp(`^\\$\\{\\{\\s*steps\\.${expectedStepId}\\.outputs\\.[\\w-]+\\s*\\}\\}$`)) + + const producingStep = (targetJob.steps ?? []).find((step) => step.id === expectedStepId) + expect(producingStep, `${jobName} must have a step with id "${expectedStepId}"`).toBeDefined() + } + + function assertStepWritesOutputKey(step: WorkflowStep, key: string): void { + const run = step.run ?? '' + expect(run, `step "${step.name}" must write to $GITHUB_OUTPUT`).toMatch(/GITHUB_OUTPUT/) + // Negative lookbehind for a word character rules out matching "tag_commit=" as a false + // positive for the "commit=" key, while still matching `key=value`, `echo "key=$value"`, + // and `{ echo "key=..." ; }` forms. + expect(run, `step "${step.name}" must write the "${key}=" key`).toMatch(new RegExp(`(? { + const workflow = parseWorkflow(publishNextWorkflowPath) + assertOutputMapsFromStep(workflow, 'validate', 'version', 'release-meta') + assertOutputMapsFromStep(workflow, 'validate', 'tag', 'release-meta') + }) + + it('maps commit from the release-commit step output, not env.*', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + assertOutputMapsFromStep(workflow, 'validate', 'commit', 'release-commit') + }) + + it('has the release-meta step write version and tag to $GITHUB_OUTPUT', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const step = workflowStep(workflow, 'validate', 'Validate prerelease tag and release files') + expect(step.id).toBe('release-meta') + assertStepWritesOutputKey(step, 'version') + assertStepWritesOutputKey(step, 'tag') + }) + + it('has the release-commit step write commit to $GITHUB_OUTPUT', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const step = workflowStep(workflow, 'validate', 'Prove exact tag commit and next ancestry') + expect(step.id).toBe('release-commit') + assertStepWritesOutputKey(step, 'commit') + }) + + it('still keeps tarball_name, tarball_sha256, artifact_name, and qualification_status mapped from their existing step outputs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + assertOutputMapsFromStep(workflow, 'validate', 'tarball_name', 'pack') + assertOutputMapsFromStep(workflow, 'validate', 'tarball_sha256', 'pack') + assertOutputMapsFromStep(workflow, 'validate', 'artifact_name', 'pack') + assertOutputMapsFromStep(workflow, 'validate', 'qualification_status', 'qualification') + }) + + it('has publish and post_publish consume validate metadata exclusively through needs.validate.outputs', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const publishEnv = (job(workflow, 'publish') as { env?: Record }).env ?? {} + const postPublishEnv = (job(workflow, 'post_publish') as { env?: Record }).env ?? {} + + expect(String(publishEnv.PACKAGE_VERSION)).toBe('${{ needs.validate.outputs.version }}') + expect(String(publishEnv.RELEASE_COMMIT)).toBe('${{ needs.validate.outputs.commit }}') + expect(String(publishEnv.RELEASE_TAG)).toBe('${{ needs.validate.outputs.tag }}') + expect(String(postPublishEnv.PACKAGE_VERSION)).toBe('${{ needs.validate.outputs.version }}') + expect(String(postPublishEnv.RELEASE_COMMIT)).toBe('${{ needs.validate.outputs.commit }}') + expect(String(postPublishEnv.RELEASE_TAG)).toBe('${{ needs.validate.outputs.tag }}') + }) + + it('rejects empty version/commit metadata before the publish job can act on it', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const verifyStep = workflowStep(workflow, 'publish', "Verify downloaded artifact against the validate job's receipt") + + // If validate ever emitted an empty version or commit, the receipt written in the same job + // would carry that same empty value, and this comparison in `publish` would still catch it + // because the empty EXPECTED_TARBALL_SHA256 / receipt fields could never match a real + // artifact's real sha256/version/commit. + expect(verifyStep.run).toContain('receipt.version !== process.env.PACKAGE_VERSION') + expect(verifyStep.run).toContain('receipt.sourceCommit !== process.env.RELEASE_COMMIT') + expect(verifyStep.run).toContain('receipt.tarballSha256 !== process.env.EXPECTED_TARBALL_SHA256') + }) + + it('detects a synthetic fixture job whose output maps from env.* instead of a step output', () => { + const brokenWorkflow: Workflow = { + jobs: { + validate: { + outputs: { version: '${{ env.PACKAGE_VERSION }}' }, + steps: [{ name: 'set env', run: 'printf \'PACKAGE_VERSION=1.2.3\\n\' >> "$GITHUB_ENV"' }], + }, + }, + } + + expect(() => assertOutputMapsFromStep(brokenWorkflow, 'validate', 'version', 'release-meta')).toThrow() + }) + + it('detects a synthetic fixture step that declares an id but never writes the expected key to $GITHUB_OUTPUT', () => { + const brokenStep: WorkflowStep = { + id: 'release-meta', + name: 'Validate prerelease tag and release files', + run: 'printf \'PACKAGE_VERSION=1.2.3\\n\' >> "$GITHUB_ENV"', + } + + expect(() => assertStepWritesOutputKey(brokenStep, 'version')).toThrow() + }) +}) + +// --------------------------------------------------------------------------------------------- +// B: `release.yml` classify/release split. `classify` runs for every `v*` tag (including +// malformed/unapproved forms) and fails visibly on anything that is not a stable or approved +// prerelease tag; `release` only runs for a classified-stable tag. +// --------------------------------------------------------------------------------------------- +describe('release.yml classify/release split', () => { + it('defines exactly the two jobs classify and release', () => { + const workflow = parseWorkflow('.github/workflows/release.yml') + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(['classify', 'release']) + }) + + it('has no workflow-level contents: write permission', () => { + const workflow = parseWorkflow('.github/workflows/release.yml') + expect(workflow.permissions).toEqual({ contents: 'read' }) + }) + + it('gives classify read-only permissions and no environment', () => { + const workflow = parseWorkflow('.github/workflows/release.yml') + const classifyJob = job(workflow, 'classify') + expect(classifyJob.permissions).toEqual({ contents: 'read' }) + expect(classifyJob.environment).toBeUndefined() + }) + + it('maps the classify job output from the classify step, never hardcoded or from env', () => { + const workflow = parseWorkflow('.github/workflows/release.yml') + const classifyOutputs = job(workflow, 'classify').outputs + expect(String(classifyOutputs?.channel)).toBe('${{ steps.classify.outputs.channel }}') + }) + + it('makes release depend on classify, require success and the stable channel, and hold contents: write', () => { + const workflow = parseWorkflow('.github/workflows/release.yml') + const releaseJob = job(workflow, 'release') + expect(releaseJob.needs).toBe('classify') + expect(releaseJob.if).toBe("success() && needs.classify.outputs.channel == 'stable'") + expect(releaseJob.permissions).toEqual({ contents: 'write' }) + }) + + it('pins every action in release.yml to a 40-character SHA with a version comment', () => { + const raw = readFileSync(resolve('.github/workflows/release.yml'), 'utf8') + const usesLines = raw.split('\n').map((line) => line.trim()).filter((line) => line.startsWith('uses:')) + + expect(usesLines.length).toBeGreaterThan(0) + for (const line of usesLines) { + expect(line, line).not.toMatch(/@(v\d+|main|master)(\s|$)/) + const match = line.match(/uses:\s*([^@]+)@([0-9a-f]{40})(?:\s+#\s*(v[\w.]+))?/) + expect(match, `${line} must pin a 40-character SHA with a # vX.Y.Z comment`).not.toBeNull() + expect(match?.[3], `${line} must have a readable version comment`).toBeTruthy() + } + }) +}) + +describe('release.yml classify step behavior across all four tag classes', () => { + function runClassifyStep(tag: string): { status: number | null; stderr: string; outputContents: string } { + const workflow = parseWorkflow('.github/workflows/release.yml') + const step = workflowStep(workflow, 'classify', 'Classify release tag') + + const outputDir = mkdtempSync(join(tmpdir(), 'madar-classify-output-')) + const outputFile = join(outputDir, 'github-output') + writeFileSync(outputFile, '') + + try { + const result = spawnSync('bash', ['-c', step.run ?? ''], { + cwd: process.cwd(), + encoding: 'utf8', + env: { ...process.env, GITHUB_REF_NAME: tag, GITHUB_OUTPUT: outputFile }, + }) + return { + status: result.status, + stderr: result.stderr ?? '', + outputContents: readFileSync(outputFile, 'utf8'), + } + } finally { + rmSync(outputDir, { recursive: true, force: true }) + } + } + + it('classifies a stable tag as channel=stable and exits 0', () => { + const { status, outputContents } = runClassifyStep('v0.33.0') + expect(status).toBe(0) + expect(outputContents).toContain('channel=stable') + }) + + it.each(['v0.33.0-beta.1', 'v0.33.0-rc.1', 'v0.33.0-next.1'])( + 'classifies the approved prerelease tag %s as channel=prerelease and exits 0, so release skips cleanly', + (tag) => { + const { status, outputContents } = runClassifyStep(tag) + expect(status).toBe(0) + expect(outputContents).toContain('channel=prerelease') + expect(outputContents).not.toContain('channel=stable') + }, + ) + + it.each(['v0.33.0-alpha.1', 'v0.33.0-preview.1', 'v0.33.0-beta', 'v0.33.0-beta.01'])( + 'fails the classify step visibly for the unsupported tag %s and emits no channel', + (tag) => { + const { status, stderr, outputContents } = runClassifyStep(tag) + expect(status).not.toBe(0) + expect(outputContents).not.toContain('channel=') + expect(stderr).toContain('Release tag validation failed:') + }, + ) +}) + +// --------------------------------------------------------------------------------------------- +// C: a green vitest summary is not sufficient release evidence. These tests prove the raw-log +// gate is actually wired into `validate` for both test:run and test:coverage, that both statuses +// are captured explicitly (not `&&`), that the scanner runs even when the test command fails, +// and that the tarball is only ever built after both gates pass. +// --------------------------------------------------------------------------------------------- +describe('vitest raw-log gate wiring in validate', () => { + it('scans both the test:run and test:coverage logs with the scanner script', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const testRunStep = workflowStep(workflow, 'validate', 'Run tests') + const coverageStep = workflowStep(workflow, 'validate', 'Run tests with coverage thresholds') + + expect(testRunStep.run).toContain('npm run test:run') + expect(testRunStep.run).toContain('.github/scripts/assert-clean-vitest-log.mjs') + expect(testRunStep.run).toContain('test-run.log') + + expect(coverageStep.run).toContain('npm run test:coverage') + expect(coverageStep.run).toContain('.github/scripts/assert-clean-vitest-log.mjs') + expect(coverageStep.run).toContain('test-coverage.log') + }) + + it('captures both the test command and scanner exit status explicitly, not via &&', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + for (const stepName of ['Run tests', 'Run tests with coverage thresholds']) { + const run = workflowStep(workflow, 'validate', stepName).run ?? '' + expect(run, stepName).toContain('PIPESTATUS[0]') + expect(run, stepName).toContain('test_status=') + expect(run, stepName).toContain('scanner_status=$?') + expect(run, stepName).not.toMatch(/test:(run|coverage)[^\n]*&&[^\n]*assert-clean-vitest-log/) + } + }) + + it('disables -e so a failing test command cannot short-circuit the scanner', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + for (const stepName of ['Run tests', 'Run tests with coverage thresholds']) { + expect(workflowStep(workflow, 'validate', stepName).run, stepName).toContain('set +e') + } + }) + + it('fails the step when either the test command or the scanner reports non-zero', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + for (const stepName of ['Run tests', 'Run tests with coverage thresholds']) { + const run = workflowStep(workflow, 'validate', stepName).run ?? '' + expect(run, stepName).toContain('if [ "$test_status" -ne 0 ] || [ "$scanner_status" -ne 0 ]') + expect(run, stepName).toContain('exit 1') + } + }) + + it('uploads raw vitest logs only on failure, with bounded retention and a unique run/attempt name', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const uploadStep = workflowStep(workflow, 'validate', 'Upload raw vitest logs on failure') + + expect(uploadStep.if).toBe('failure()') + expect(String(uploadStep.with?.name)).toContain('github.run_id') + expect(String(uploadStep.with?.name)).toContain('github.run_attempt') + expect(uploadStep.with).toHaveProperty('retention-days') + expect(Number(uploadStep.with?.['retention-days'])).toBeLessThanOrEqual(14) + expect(String(uploadStep.uses)).toMatch(/^actions\/upload-artifact@[0-9a-f]{40}$/) + }) + + it('builds the release tarball only after both vitest gate steps have run', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + const steps = allSteps(workflow, 'validate') + const testRunIndex = steps.findIndex((step) => step.name === 'Run tests') + const coverageIndex = steps.findIndex((step) => step.name === 'Run tests with coverage thresholds') + const packIndex = steps.findIndex((step) => step.name === 'Build release tarball and receipt') + + expect(testRunIndex).toBeGreaterThanOrEqual(0) + expect(coverageIndex).toBeGreaterThan(testRunIndex) + expect(packIndex).toBeGreaterThan(coverageIndex) + }) + + it('never lets publish download or otherwise consume the diagnostic vitest-log artifact', () => { + const workflow = parseWorkflow(publishNextWorkflowPath) + for (const step of allSteps(workflow, 'publish')) { + if (step.uses?.includes('actions/download-artifact')) { + expect(String(step.with?.name)).not.toContain('vitest-diagnostic-logs') + } + expect(step.run ?? '').not.toContain('vitest-diagnostic-logs') + } + }) +}) From e6cad3b0bdeb43b1ce7803a32185597a74e652bd Mon Sep 17 00:00:00 2001 From: mohammed naji Date: Thu, 13 Aug 2026 00:48:37 +0400 Subject: [PATCH 6/6] fix(release): address CodeRabbit findings on the classify/dataflow/scanner change Five findings, all valid: - release.yml: remove `cache: npm` from the `release` job. That job now holds `contents: write` and runs `npm ci`/tests/build, the same privileged shape publish-next.yml's `validate` job documents must never restore a cache an untrusted PR job could poison. This PR restructured release.yml and moved `contents: write` onto this job, so it has to hold itself to the policy it asserts elsewhere. - publish-next.yml: the post-publish propagation-wait loop truncates its output files every attempt; if the final attempt's `npm view` itself failed (registry error, network blip), the fallback JSON parse only ever reported "Unexpected end of JSON input" and hid the real cause. Capture each `npm view` failure explicitly and surface it via `::error::` before falling into the diagnostic verify call. - verify-next-release-state.mjs: `spawnSync` does not throw when the child process itself fails to launch (e.g. `git` missing from PATH) -- it sets `.error` and leaves `status: null`. Check `.error` explicitly so that case is reported as an environment problem, not misread as an ordinary non-ancestor verification failure. - docs/release.md: document the qualification gate's third outcome (contract present, validator script missing -> intentional hard fail) alongside the two already documented. - tests/unit/release-pipeline.test.ts: pin `commit.gpgsign=false` in the ancestor-check git fixture so it does not depend on the host's global git config. No tag, npm package, or GitHub release was published or created. PR #688 stays out of draft per the lead's instruction but nothing changes about publication status. Refs #654, #687. --- .github/scripts/verify-next-release-state.mjs | 11 +++++- .github/workflows/publish-next.yml | 37 +++++++++++++++---- .github/workflows/release.yml | 6 ++- docs/release.md | 6 +++ tests/unit/release-pipeline.test.ts | 5 +++ 5 files changed, 55 insertions(+), 10 deletions(-) diff --git a/.github/scripts/verify-next-release-state.mjs b/.github/scripts/verify-next-release-state.mjs index 666358d1..ccb5efe8 100644 --- a/.github/scripts/verify-next-release-state.mjs +++ b/.github/scripts/verify-next-release-state.mjs @@ -79,6 +79,15 @@ function gitCommitIsAncestor(commit, branch) { stdio: 'pipe', }) + // spawnSync does not throw when the command itself cannot be launched (e.g. `git` missing + // from PATH) -- it returns a result with `.error` set and `status: null`. Left unchecked, + // that null status falls through to the generic "failed with status null" message below, + // misreporting a missing git as an ordinary non-ancestor verification failure instead of an + // environment problem. + if (result.error) { + fail(`Failed to run git merge-base: ${result.error.message}`) + } + if (result.status === 0) { return true } @@ -86,7 +95,7 @@ function gitCommitIsAncestor(commit, branch) { return false } - fail(result.stderr.trim() || `git merge-base failed with status ${String(result.status)}`) + fail((result.stderr && result.stderr.trim()) || `git merge-base failed with status ${String(result.status)}`) } export function runCli(args) { diff --git a/.github/workflows/publish-next.yml b/.github/workflows/publish-next.yml index 77e030e0..dc72d5c5 100644 --- a/.github/workflows/publish-next.yml +++ b/.github/workflows/publish-next.yml @@ -537,23 +537,44 @@ jobs: run: | set -euo pipefail verified=false + last_error="" for attempt in {1..12}; do - if npm view "$PACKAGE_NAME" dist-tags --json > "$RUNNER_TEMP/npm-dist-tags-after.json" \ - && npm view "$PACKAGE_NAME@$PACKAGE_VERSION" version --json > "$RUNNER_TEMP/npm-resolved-version.json" \ - && node .github/scripts/verify-next-release-state.mjs \ - --verify-publish \ - --version "$PACKAGE_VERSION" \ - --before "$RUNNER_TEMP/npm-dist-tags-before.json" \ - --after "$RUNNER_TEMP/npm-dist-tags-after.json" \ - --resolved-version "$RUNNER_TEMP/npm-resolved-version.json" >/dev/null 2>&1; then + # Each attempt truncates npm-dist-tags-after.json / npm-resolved-version.json before + # writing. If `npm view` itself fails on the final attempt (registry error, network + # blip, DNS), the file is left empty and the fallback JSON parse below would + # otherwise surface only "Unexpected end of JSON input" -- true, but useless for an + # operator trying to find the actual cause. Capture and report the real npm error + # instead of letting it be masked by that downstream parse failure. + if ! npm view "$PACKAGE_NAME" dist-tags --json > "$RUNNER_TEMP/npm-dist-tags-after.json" 2>"$RUNNER_TEMP/npm-view-error.log"; then + last_error="npm view $PACKAGE_NAME dist-tags failed: $(cat "$RUNNER_TEMP/npm-view-error.log")" + echo "Waiting for npm registry propagation (attempt $attempt of 12): $last_error" + sleep 10 + continue + fi + if ! npm view "$PACKAGE_NAME@$PACKAGE_VERSION" version --json > "$RUNNER_TEMP/npm-resolved-version.json" 2>"$RUNNER_TEMP/npm-view-error.log"; then + last_error="npm view $PACKAGE_NAME@$PACKAGE_VERSION version failed: $(cat "$RUNNER_TEMP/npm-view-error.log")" + echo "Waiting for npm registry propagation (attempt $attempt of 12): $last_error" + sleep 10 + continue + fi + if node .github/scripts/verify-next-release-state.mjs \ + --verify-publish \ + --version "$PACKAGE_VERSION" \ + --before "$RUNNER_TEMP/npm-dist-tags-before.json" \ + --after "$RUNNER_TEMP/npm-dist-tags-after.json" \ + --resolved-version "$RUNNER_TEMP/npm-resolved-version.json" >/dev/null 2>&1; then verified=true break fi + last_error="dist-tags/version have not propagated to reflect this publish yet" echo "Waiting for npm registry propagation (attempt $attempt of 12)" sleep 10 done if [[ "$verified" != true ]]; then + if [[ -n "$last_error" ]]; then + echo "::error::$last_error" + fi node .github/scripts/verify-next-release-state.mjs \ --verify-publish \ --version "$PACKAGE_VERSION" \ diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 29c8952e..aa2c155b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -82,11 +82,15 @@ jobs: fetch-depth: 0 persist-credentials: false + # No dependency cache: `release` holds `contents: write` and runs `npm ci`, tests, and + # build, the same privileged-job shape as `publish-next.yml`'s `validate` job, which + # documents why a release-pipeline job must never restore a cache an untrusted PR job + # could have poisoned. This job restructures release.yml and moves `contents: write` + # onto itself, so it must hold itself to the same policy it asserts elsewhere. - name: Set up Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '20' - cache: npm - name: Require stable release tag run: node .github/scripts/classify-release-tag.mjs --tag "$GITHUB_REF_NAME" --expect stable diff --git a/docs/release.md b/docs/release.md index 50f3ba84..d542c19e 100644 --- a/docs/release.md +++ b/docs/release.md @@ -54,6 +54,12 @@ npm sbom --sbom-format cyclonedx > sbom.cdx.json Run `npm run qualify:validate` when that script is present. Its failure is a release blocker; when it is absent, record that qualification was unavailable rather than presenting it as passed. +The release pipeline's qualification gate (`.github/scripts/check-qualification-gate.mjs`) actually has three outcomes, not two, and the third is intentional: + +- **script present** → runs `qualify:validate` for real; a non-zero exit blocks the release. +- **script and its `docs/qualification/` contract both absent** → records a notice and skips; this is today's ordinary pre-qualification state. +- **`docs/qualification/` present but `qualify:validate` is missing from `package.json`** → **hard fails**, deliberately. Once the qualification contract has landed, its validator disappearing (a bad refactor, a lost merge, a dependency bump rewriting `package.json`) must never be read as "not applicable yet" and quietly fall back to a skip. If a release run blocks on this, the fix is to restore `qualify:validate`, not to treat the block as a bug. + `npm run release:verify` locks the public package metadata, changelog version entry, and npm-visible README links before publish. `npm pack --dry-run` records the package boundary, and `sbom.cdx.json` is the checked supply-chain inventory snapshot. If the change touches packaging, installer behavior, or public MCP Registry metadata, keep those outputs with the release pull request. Review [`docs/security/mcp-threat-model.md`](./security/mcp-threat-model.md) before publishing changes that affect MCP installs, share-safe artifacts, prompt handling, or local file boundaries. Any new public claim requires a reproducible artifact under `docs/benchmarks/suite/` and a matching update to `docs/claims-and-evidence.md` before the README or release notes can say it publicly. For an external announcement, copy the proof block and channel tracker from [`docs/launch-checklist.md`](./launch-checklist.md) into the release pull request or working notes before drafting copy. diff --git a/tests/unit/release-pipeline.test.ts b/tests/unit/release-pipeline.test.ts index 9da604d2..3cfebbca 100644 --- a/tests/unit/release-pipeline.test.ts +++ b/tests/unit/release-pipeline.test.ts @@ -227,6 +227,11 @@ describe('next release state guards', () => { execFileSync('git', ['init', '-b', 'next'], { cwd: fixtureDir, stdio: 'pipe' }) execFileSync('git', ['config', 'user.email', 'madar@example.com'], { cwd: fixtureDir }) execFileSync('git', ['config', 'user.name', 'Madar Test'], { cwd: fixtureDir }) + // Pinned so this fixture does not depend on the host's global git config: a developer + // with `commit.gpgsign=true` set globally would otherwise see `git commit` here hang on + // (or fail without) a GPG passphrase prompt for an environmental reason unrelated to the + // assertion under test. + execFileSync('git', ['config', 'commit.gpgsign', 'false'], { cwd: fixtureDir }) writeFileSync(join(fixtureDir, 'fixture.txt'), 'base\n') execFileSync('git', ['add', 'fixture.txt'], { cwd: fixtureDir }) execFileSync('git', ['commit', '-m', 'base'], { cwd: fixtureDir, stdio: 'pipe' })