ci(release): publish to npm through OIDC trusted publishing - #4684
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
GJC adversarial review — exact-head verdict (independent agent review, head
|
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The OIDC migration direction is sound, but the release path still has failure modes that can publish immutable npm packages with incomplete or incorrect GitHub Release metadata, and the retry path is not transaction-safe. Please address the required changes below before merging.
Findings / Required Changes
-
[P1] Grant and enforce the permissions required by release-note collection —
.github/workflows/ci.yml:444-450,scripts/release-notes.ts:158-161
The new collector calls PR APIs, while thepublishjob grants onlycontents: writeandid-token: write. A denied PR API request is currently converted to an empty result, so the release can silently publish commit-only notes and omit attribution/new contributors. Grant the needed read scope in the workflow-policy contract and make authentication, transport, and API failures fail closed; reserve an empty result for an explicit successful no-result response. -
[P1] Resolve and validate release notes before the first irreversible npm publication —
.github/workflows/ci.yml:526-590
History fetching and note generation run after packages have been published. A fetch, GitHub API, parsing, or note-generation failure then leaves published packages without the corresponding GitHub Release and without a same-version recovery path. Move this validation before registry mutation, or persist a deterministic validated body that can be used to complete the same release safely. -
[P1] Make previous-release selection correct and make its empty-history fallback reachable —
.github/workflows/ci.yml:567-570
The unanchored tag glob admits nightly/RC and unrelated-branch tags, so it can derive the range from the wrong base. In addition, underset -euo pipefail,grep -vFxexits non-zero when no candidate survives, aborting before the documented empty-notes fallback. Select only exact stable tags that are ancestors of the release head, and handle the no-candidate case without turning it into a shell failure. -
[P1] Make rejected atomic-push recovery transactional —
scripts/release.ts:428-440
On rejection, the new cleanup removes only the local tag even though the version/changelog commit was already created. A retry of the same version can encounter an empty Unreleased section, duplicate release headings, or nothing-to-commit rather than retrying the original transaction. Either resume by recreating the tag for the prepared commit or roll back the complete local release state before claiming the version is safe to retry. -
[P1] Narrow the OIDC credential exposure and pin the npm CLI exactly —
.github/workflows/ci.yml:445-459
id-token: writeis available to every step inpublish, including live npm installation, dependency installation, repository scripts, and later actions. The job also installs the mutablenpm@^11.5.1range inside that credential-bearing boundary. Split preparation from a minimal publish-only OIDC job, and use an exact, integrity-verifiable npm CLI version rather than a caret range.
CI / Verification
- Reviewed exact PR head:
4c2827dabd0c9b1f17ea4353d1fb94ba64bde23a. - The current Dev CI evidence includes successful affected-path runs for
scripts/release-notes.test.tsandscripts/release-policy.test.ts, along with the workflow YAML and CI dry-run checks. - The PR-contract verdict checks are the submitted
needs humanfailures; they are intentionally not used as a negative signal here. - The verification axis was evidence-only and did not execute PR code locally.
Axis Coverage
| Axis | Result |
|---|---|
| A1. Intent / Policy / Contract | Changes requested |
| A2. Architecture / Correctness / Failure | Changes requested |
| A3. Security / Privacy / Trust | Changes requested |
| A4. Verification / Tests / CI | Changes requested |
| A5. Context / Compatibility / Platform | Changes requested |
|
P1 blockers from review 4963962358 resolved at new head 3450a47 (see the P1 resolution section in the PR body): fail-closed notes API + |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The OIDC migration is directionally sound: the publish job is narrowly permissioned, the npm artifact is integrity-checked, and release preparation is moved before the irreversible publication boundary. Two P1 issues still make the release path unsafe: an ambiguous atomic-push failure can cause destructive local rollback after a successful remote release, and any matching stable tag can reach the OIDC/repository-write publish job without protected-main provenance.
Findings / Required Changes
-
[P1] Do not roll back after an ambiguous atomic-push failure.
scripts/release.ts:445-456treats every non-zerogit push --atomicresult as proof that no remote ref was created, then deletes the local tag and hard-resets the release commit. A transport failure after GitHub has accepted the atomic transaction leavesmainand the immutable release tag live, while the command reports a retryable failure and destroys its local release state. Query both remote refs before changing local state: reconcile a matching committed transaction as success, clean up only after proving neither ref moved, and preserve state with explicit recovery guidance for partial/unreachable results. Add a regression case for an accepted remote transaction with a client-side failed push result. -
[P1] Establish a protected-
maintrust boundary for stable tag releases.
.github/workflows/ci.yml:3-6, 73-103, 440-442, 590-637accepts anyv*tag whose version matchespackage.json; it does not prove that the tagged SHA is on protectedmain. Since a tag workflow is evaluated from the tagged revision and later grantscontents: writeplusid-token: write, an actor permitted to create a tag can use a side-branch commit to publish to npm and mutate release artifacts. Enforce tag protection for the release principal and protected-mainprovenance outside the tag-controlled workflow revision (for example, a default-branch-controlled privileged workflow protected by an approval environment), then validate the triggering SHA against fetched protected-main history. A check added only to this tag-resolved YAML is bypassable by the tagged revision. -
[P2] Make manual rollback recovery executable after a partial rollback.
scripts/release.ts:457-460always suggestsgit tag -d <tag> && git reset --hard <sha>. When tag deletion already succeeded but the reset failed, the first command fails and&&prevents the required reset. Generate recovery instructions from the individual rollback outcomes, or make tag deletion idempotent. -
[P2] Pin the credential-bearing Node bootstrap runtime exactly.
.github/workflows/ci.yml:606-627enablesid-token: writebefore usingsetup-nodewith mutablenode-version: "24";npm packruns before the pinned npm tarball is checksum-verified. Pin the Node patch version deliberately in this job, retaining the existing npm tarball integrity check. -
[P2] Do not render Git display names as GitHub mentions.
scripts/release-notes.ts:120-130, 195-207collects%an(a Git author display name) and formats it as@${author}. Fallback release notes can therefore publish invalid or misleading mentions. Resolve a GitHub login first, or render the author as a display name without@; add a non-login-author regression test. -
[P2] Put the new release regression suites on the normal release CI path.
package.json:100omitsscripts/release-notes.test.tsandscripts/release-retry.test.tsfromtest:release, whilescripts/ci-dev-affected.ts:238-243, 968-970makes that command the force-full release-contract task. Add both suites there or create explicit force-full CI tasks so the new note-derivation and retry behavior are continuously exercised.
CI / Verification
- Reviewed exact head:
3450a470161df734ca5c69827380daa282cc842e. - No PR code was executed for this review. The verification axis inspected the exact-head CI artifacts and changed test coverage.
- Dev CI run #13571 is waiting behind serialized run #13507: https://github.com/Yeachan-Heo/gajae-code/actions/runs/32169283805
- Public site sync #9778 passed: https://github.com/Yeachan-Heo/gajae-code/actions/runs/32169122438
- PR contract #1016 is
needs-human; its failure is excluded from this verdict as instructed.
Axis Coverage
| Axis | Result |
|---|---|
| A1 — Intent / Policy / Contract | No blocking policy or contract issue; identified the fallback-attribution P2. |
| A2 — Architecture / Correctness / Failure | P1 ambiguous atomic-push rollback; P2 partial-rollback recovery command. |
| A3 — Security / Privacy / Trust | P1 untrusted stable-tag publish path; P2 mutable Node bootstrap in an OIDC-capable job. |
| A4 — Verification / Tests / CI | Changed tests cover key failure paths; exact-head Dev CI remains queued; needs-human CI failure ignored. |
| A5 — Context / Compatibility / Platform | Confirmed release lifecycle impact and found the normal CI omission for the two new regression suites. |
|
Round-2 blockers from review 4964412257 resolved at new head c409d31 (see Round-2 resolution in the PR body): atomic-push reconciliation with pre-push baseline (committed=>success / proven-unmoved=>rollback / partial or unreachable=>preserve+guidance), protected-main ancestry gate + npm-release approval environment hook (owner settings documented), idempotent per-outcome recovery commands, exact Node 24.19.0 pin, login-resolved attribution, and both new suites added to test:release. New exact-head digest da1a3e3bbf2ca28149da90392d9af54348cea1dad0fd5826a2864618e9a4fc6c. Re-review requested. |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The OIDC migration is materially stronger: the workflow narrows explicit permissions, checks the release-notes artifact before publication, pins the Node/npm bootstrap, and adds atomic-push reconciliation. However, the current exact head still leaves a privileged job executing repository-controlled dependency installation, lacks an externally verified npm trusted-publisher migration for the new environment-scoped OIDC subject, and has failing non-human Dev CI checks.
Findings / Required Changes
-
[P1] Remove repository-controlled installation from the credential-bearing
publishjob. —.github/workflows/ci.yml:639-652
publishhas bothcontents: writeandid-token: write, checks out the repository with persisted GitHub credentials by default, then runsbun install --frozen-lockfile. A malicious or compromised lifecycle hook/dependency can execute with both capabilities, bypassing the intended separation between untrusted preparation and irreversible publishing. Build/seal the publish inputs inrelease_prepare; makepublishconsume verified artifacts without checkout, Bun, or dependency installation. At minimum, do not persist checkout credentials and do not execute package scripts in this boundary. -
[P1] Complete and verify the npm trusted-publisher migration for the environment-scoped OIDC identity. —
.github/workflows/ci.yml:644
Addingenvironment: npm-releasechanges GitHub's OIDCsubtorepo:Yeachan-Heo/gajae-code:environment:npm-release. The repository can only assert the environment string; it cannot prove npm has trusted-publisher registrations matching this workflow/environment for every published package. Update and inspect those external registrations before merging, then record a controlled validation of the OIDC release path. GitHub environment/ruleset protections must remain a documented release prerequisite. -
[P1] Resolve the non-human exact-head Dev CI failures. —
root-check,test shard 1/8
The rootci:check:fullgate exits 1, and shard 1/8 has four failingBisectToolintegration assertions (packages/coding-agent/test/tools/bisect.test.ts:186,211,237,272). These are not the permittedneeds-humancontract result. Diagnose and fix or establish a documented infrastructure-only cause, then rerun successfully on this head. -
[P1] Do not infer a rejected atomic push from two equal remote snapshots. —
scripts/release.ts:536-552
A client can receive a transport failure after a successful atomic push. Ifmainand the tag are restored to their baseline before the re-query, the implementation chooses rollback and deletes the local release state even though the transient tag may already have started the release workflow. Treat a client-observed failed push as ambiguous unless the server outcome conclusively proves rejection; otherwise preserve state and require durable operator reconciliation. -
[P2] Attribute release-note coverage by commit identity, not normalized subject. —
scripts/release-notes.ts:91-99
Repeated normalized subjects can make a partially shipped candidate PR appear at least 50% shipped, causing its full PR title to be credited for work that was not released. Retain and compare commit OIDs (including cherry-pick origins) for coverage, and add a duplicate-subject partial-cherry-pick regression test.
CI / Verification
- Reviewed remote exact head:
c409d31292fb8384d4787ca297598ae35de990b2. - Five independent subagent reviews completed across A1–A5; this review consolidates their independently verified findings.
- The changed release suites and workflow-permission checks are green in exact-head Dev CI, and
test:releaseincludes the new release-notes/retry suites. - The separate PR-contract
needs-humanresult is intentionally excluded. Other Dev CI work remains pending; the two non-human failures above must be resolved. - No PR code was executed locally for this review.
Axis Coverage
| Axis | Result |
|---|---|
| A1 — Intent / Policy / Contract | P1: environment-scoped OIDC requires a matching, verified npm trusted-publisher configuration. |
| A2 — Architecture / Correctness / Failure | P1: baseline-equal re-observation can select an unsafe rollback after an ambiguous push; P2: subject-based attribution can miscredit a partial curated release. |
| A3 — Security / Privacy / Trust | P1: dependency installation runs inside the OIDC/repository-write boundary. |
| A4 — Verification / Tests / CI | P1: exact-head Dev CI has root-check and non-human test-shard failures; targeted changed release coverage is green. |
| A5 — Context / Compatibility / Platform | P1: external npm publisher registrations must migrate to the npm-release environment identity. |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Independent maintainer review at exact head c409d312 — merge blocked. Reviewed as a supply-chain change, because that is what it is.
what is genuinely right
Workflow-wide permissions stay contents: read (ci.yml:19-24); only publish gains contents: write + id-token: write, at job scope (:613-627). No pull_request_target trigger exists, and I traced every path: a fork PR or ordinary pull_request run cannot reach release_prepare/publish, a fork's tag runs only in the fork, and workflow_dispatch values tag-build-verify/main-nontag cannot publish. Every third-party action in the OIDC job is pinned to a full 40-char SHA.
Artifact/ref binding is sound: prepare and publish both use github.sha, evidence records source_commit, publish re-reads checkout HEAD through assertReleaseSourceBinding, and retained tarballs are hash-validated (scripts/ci-release-publish.ts:1118-1144). The token path is genuinely gone — NPM_TOKEN, the ephemeral npmrc, NODE_AUTH_TOKEN and NPM_CONFIG_USERCONFIG are all deleted, and remaining _authToken text is either a negative policy assertion or the unrelated Verdaccio fixture. Omitting --provenance is correct for trusted publishing; npm emits it automatically for public packages from public repos.
major 1 — repository-controlled code still runs inside the credential-bearing job
ci.yml:639-703. publish checks out with persisted credentials, runs bun install --frozen-lockfile, and then executes bun scripts/ci-release-publish.ts — all while holding both GitHub write scope and a live OIDC token.
That is the classic escalation shape. A malicious lifecycle hook or compromised transitive dependency executing during install can request the npm credential and push with the checkout token. The whole point of moving to OIDC is to shrink the window in which a credential is reachable; installing dependencies inside that window gives most of it back.
Seal tarballs and evidence in release_prepare, then publish from validated artifacts with no checkout, no dependency install, and no repository script. If GitHub Release finalization needs contents: write, split it into its own job.
major 2 — the external trust boundary does not exist yet
ci.yml:628-644 names environment: npm-release, but I checked the live APIs: the public environments endpoint lists only copilot and website-release-sync-trigger — there is no npm-release environment — and the public rulesets endpoint returns [].
So the reviewer gate this design depends on is not provisioned. Concretely: a collaborator with write-level Actions access can dispatch nightly-release, and with no environment protection there is nothing to approve it.
Adding environment: also changes the OIDC subject from repo:Yeachan-Heo/gajae-code:ref:refs/tags/v0.14.1 to repo:Yeachan-Heo/gajae-code:environment:npm-release. The body cites the live v0.14.1 exchange as evidence the flow works — it does not validate the new subject, and the attestations on gajae-code@0.14.1 prove the old ref-scoped identity.
Before merge: create and protect the environment, restrict v* tag creation, confirm main/dev protection, update the trusted-publisher registration for all 13 public packages, disallow traditional token publishing, and record one controlled OIDC publish under the new subject.
major 3 — snapshot equality still triggers destructive rollback after an ambiguous push
scripts/release.ts:536-567, with the decision at :485-490:
if (input.postPushMain === input.prePushMain && input.postPushTag === input.prePushTag) return { kind: "rollback" };If GitHub accepted the atomic main+tag transaction and the refs were then restored or deleted before the re-query, the tag event may already be running while this code sees the baseline and hard-resets local state. Preserve state after any client-observed failed push unless there is durable server evidence of rejection, and add an accepted-then-restored regression.
The other retry cases (conclusive rejection, committed-but-client-failed, partial, unreachable) are correctly handled — this is the one remaining branch.
major 4 — exact-head CI is red
Dev CI run 32191987403 for c409d312 is failure: root-check plus four BisectTool shard failures in packages/coding-agent/test/tools/bisect.test.ts:186-272. Those are ordinary product failures, not the excluded needs-human contract result. Also mergeable_state: dirty — rebase and rerun.
minor — release attribution matches on subject text
scripts/release-notes.ts:91-99 uses shippedSubjects.has(normalizeSubject(subject)). Duplicate subjects count one shipped commit multiple times and can push a PR over the 50% attribution threshold. Compare commit OIDs / cherry-pick origins; add a duplicate-subject partial-cherry-pick case.
prior findings
Fixed: PR-API permission and fail-closed gh() handling; notes derived, hashed and uploaded in release_prepare before npm; stable previous-tag selection with ancestry check and no-history fallback; executable partial-rollback recovery; exact Node patch 24.19.0; git display names no longer rendered as mentions; both new suites wired into test:release.
Not fixed, all quoted above: repository-controlled install in publish, the environment-scoped publisher migration, the equal-snapshot rollback, subject-based attribution, and the red exact-head CI. npm is exactly pinned to 11.5.1 with a hardcoded sha512, but the exposure is not narrow while checkout/install/scripts remain.
scope
Workflow 198 lines, production scripts 499 (release-notes.ts 352 new, release.ts 147), tests/policy 613, package script 2, no docs. The release-note derivation and atomic-push transaction are broader than an auth migration but are disclosed in the body and incident rationale; I found no hidden product change.
what would catch a regression here
check-workflow-permissions.test.ts genuinely pins that only publish.contents and publish.id-token may be write-scoped. release-policy.test.ts pins token removal, exact npm/Node versions, ordering and the environment name. But the "minimal OIDC boundary" test does not reject checkout, persisted credentials, bun install, repository scripts, or a missing environment — which is exactly major 1. Extend it to assert no credential-bearing step performs any of those.
The external controls cannot be unit-tested from this repo. Before merge I want API evidence that npm-release has reviewers and ref restrictions, that tag rules exist, and a controlled dispatch-denial test for an unapproved actor.
Reviewed by @probepark — method: detached worktree at c409d312, trigger-by-trigger reachability trace to the publish job, live query of the repository environments and rulesets APIs, action-pinning audit, registry metadata inspection of the published 0.14.1 attestations, retry-branch analysis in release.ts, exact-head CI artifact review. Tests not executed.
gajae.pr-review-verdict.v1 merge-blocked sha256:da1a3e3bbf2ca28149da90392d9af54348cea1dad0fd5826a2864618e9a4fc6c reviewer:human reviewer-id:probepark evidence:exact-head-c409d312-repo-controlled-install-inside-oidc-job-and-npm-release-environment-does-not-exist
c409d31 to
95397b1
Compare
|
Round-3 fixes pushed at exact head 95397b1 (rebased onto dev 4b2883b, resolving the CONFLICTING state): ambiguous pushes preserve state with idempotent per-outcome guidance; publish job has no repository-controlled install and no persisted checkout credentials; the nonexistent npm-release environment is removed so the OIDC subject stays the proven ref-scoped identity; check-node20-baseline now accepts the exact 24.19.0 pin (root-check green); attribution coverage is OID-based. Bisect shard failures were environmental/native-sentinel, not this PR (suite 19/19 locally; bisect sources unchanged since old base). New digest 93630165b25e83e93025479997ee91a47b42a984a4224e601f2f3408bdff05ff. Re-review requested from snowykr and probepark. |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The updated release-path work resolves the earlier release-note and ambiguous-push concerns, and the compatibility review found no installer, package, artifact, or platform-surface regression. However, the credential-bearing publish path is not actually dependency-free, the PR removes an approval boundary without the stated replacement rulesets, and the exact-head non-human CI validation is red.
Findings / Required Changes
-
[P1] Keep dependency resolution out of the credential-bearing publish job —
scripts/ci-release-publish.ts:445-458, 1191;.github/workflows/ci.yml:671-705
The publish job no longer installs dependencies, but--publish-from-evidencestill unconditionally callscheckTypeDeclarations(), which runsbun x tsc. This can either fail because the workspace dependency/import graph is absent or fetch and execute a package inside theid-token: write/contents: writeboundary. Skip declaration checks in--publish-from-evidence, retain them inrelease_prepare, and add a dispatch regression that prevents dependency resolution from returning to the publish job. -
[P1] Restore an enforceable approval gate before OIDC publishing —
.github/workflows/ci.yml:624-642
This change removesenvironment: npm-releasewhile explicitly stating that the intendedmainandrefs/tags/v*rulesets are absent. The replacement job can mint an npm OIDC token and write releases; main ancestry cannot prevent a tag-creation-capable compromised actor from tagging a qualifying commit. Restore an environment with required reviewers using the corresponding trusted-publisher subject, or keep the current gate until the documented repository rulesets are configured and independently verified. -
[P1] Resolve the exact-head
root-checkfailure and rerun it successfully —.github/workflows/dev-ci.yml:71-75,scripts/ci-dev-affected.ts:778-779
Dev CI run32217547523for95397b14c573ce5a7a7f1a6b3282be5f603c097breportsAffected path validation / root-checkfailed afterRun affected task shardexited 1. This is separate from the explicitly human-gated PR-contract result;root-test:releasepassed. Identify and correct the root-check failure, then provide a successful exact-head rerun.
CI / Verification
- No PR code was executed locally for this review.
- Exact-head Dev CI evidence:
root-test:releasepassed;root-checkfailed. The explicitlyneeds-humanPR-contract gate was excluded from this assessment. - Updated release tests now cover release-note attribution/failure classifications, atomic-push reconciliation, publish ordering, and the Node baseline guard. The failed non-human root-check still blocks merge readiness.
Axis Coverage
| Axis | Result |
|---|---|
| A1 — Intent / Policy / Contract | Implementation intent is aligned; no independent code-contract finding retained. |
| A2 — Architecture / Correctness / Failure | [P1] Publish mode still performs declaration checks despite the dependency-free job boundary. |
| A3 — Security / Privacy / Trust | [P1] Approval gate removed before an OIDC-capable publish boundary. |
| A4 — Verification / Tests / CI | [P1] Exact-head non-human root-check failed. |
| A5 — Context / Compatibility / Platform | No actionable compatibility or platform regression found. |
|
Round-4 fixes at exact head 61cb8bc: declaration checks (bun x tsc) are prepare-path-only with a dispatch regression test; a permissionless release_approval job holds the npm-release environment gate ahead of publish so the OIDC subject stays ref-scoped (owner must still configure required reviewers + rulesets, documented); root-check failure root-caused to dev-side stale generated schema (bffcb50) and regenerated. ci:check:full green locally; test:release 154/154. New digest d340fb927ae91d2bf71d11ec4e9091085a42143b39f9dee2c594d1bc73f5b83a. Re-review requested. |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The OIDC split is thoughtfully scoped: preparation remains credential-free, publishing receives narrowly scoped permissions, and the transferred release-note artifact is integrity-checked. However, the new approval dependency leaves a release-capable OIDC path without the protection it claims, and it changes scheduled nightly publication from unattended to approval-gated.
Findings / Required Changes
-
[P1] Protect the release-capable OIDC path before relying on
npm-release—.github/workflows/ci.yml:618-633, 654-659
environment: npm-releaseis only a reference in workflow YAML; its protection rules live in repository settings. At the reviewed head, the environment/required-reviewer configuration is absent (and the PR notes describe the gate as a hook until it is configured). Consequently, a manualnightly-releasefromdevcan obtain the sameid-token: writepublishing credential without an enforced approval boundary. Before merging, configure the environment with required non-self reviewers, disable administrator bypass, restrict deployment refs to the intended release refs, and protect thedevrelease branch with the required review/status controls. Alternatively, remove the manualdevpublishing route. -
[P1] Keep unattended nightlies out of the stable approval gate —
.github/workflows/ci.yml:630-655
release_approvalruns after every successfulrelease_prepare, including scheduled and manual nightly releases, andpublishrequires that job. Once the required-reviewer environment above is configured, scheduled nightlies will wait for approval and time out instead of publishing automatically. Apply the production approval environment only to stable releases, or provide a separate unattended nightly path/environment; update the dependency/condition graph accordingly. Add a release-policy regression covering the scheduled/manual nightly graph.
CI / Verification
- Reviewed the exact remote head
61cb8bc72da9340497c7b5802e0b7e378ac0e713against base4b2883bc7521279b94dc71d37167987ed8e0029bthrough independent read-only review lanes; PR code was not executed. - Local public surfaces passed.
- The PR-contract check reports the explicit
needs-humanverdict; this is intentionally ignored for this review. - No completed Dev CI artifact for this exact head was available to establish the release-test result; author-reported local commands were not treated as CI evidence.
Axis Coverage
| Axis | Result |
|---|---|
| A1 — Intent / Policy / Contract | Required change: the new approval gate breaks the documented unattended-nightly contract once enabled. |
| A2 — Architecture / Correctness / Failure | No additional evidence-backed defect found. |
| A3 — Security / Privacy / Trust | Required change: configure/protect the release-capable OIDC path; the YAML environment reference alone is not a guard. |
| A4 — Verification / Tests / CI | No additional test-coverage defect found; exact-head Dev CI had no terminal evidence. |
| A5 — Context / Compatibility / Platform | Required changes: external environment prerequisite and nightly compatibility regression. |
|
Round-5 fixes at exact head 27f3243: release_approval is stable-only (publish admits nightly unattended, stable requires approval; graph regression-pinned), and the owner-controlled environment/ruleset prerequisites are pinned verbatim in the job comment. New digest 6ba7bae4c975f8f6bdd06ce8cf3d1f89c3d64950d704a39d02fd7107c84a2798. Re-review requested. |
0700104 to
0f3d436
Compare
|
Revalidation trigger for current exact head |
0f3d436 to
2ebc495
Compare
|
Revalidation trigger for current exact head |
2ebc495 to
ff97201
Compare
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Delta review at exact head bd160753 — merge blocked.
The OIDC workflow, permissions, provenance, package allowlist, ordering, and tag machinery are blob-identical to approved ff972010. The two-file delta introduces a release-note attribution regression:
scripts/release-notes.ts:119-121 now credits a referenced PR only when at least half of its branch commit OIDs appear in the release OID set. A normal squash merge has a distinct squash commit SHA, not any branch commit SHA and usually no cherry-pick trailer. The referenced PR therefore gets zero coverage, the fast path skips recovery, and the immutable GitHub Release body downgrades the shipped PR to a raw commit; New Contributors attribution may disappear permanently.
The new test incorrectly sets the PR commit OID equal to the release SHA and does not model the real squash shape. Include the PR merge/squash SHA in attribution evidence and add a regression where release and branch OIDs differ.
Reviewed by @probepark — method: direct-child/blob comparison against approved release surfaces; traced squash identity through coverage and release-body generation.
gajae.pr-review-verdict.v1 merge-blocked sha256:6836f35104ba8b92beb6ba1e3b0144f026eee8f2317144c7290b10deb7b02a35 reviewer:human reviewer-id:probepark evidence:exact-head-bd160753-squash-merge-release-attribution-regression
bd16075 to
7c4f3d5
Compare
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Delta review at exact head 7c4f3d56 — approved.
The squash-attribution blocker is fixed. Release-note candidates now carry GitHub's merge/squash commit OID; a matching shipped OID is definitive coverage, while the existing branch-OID threshold remains the conservative fallback. Tests model distinct branch and squash OIDs, including subject-referenced attribution.
All approved OIDC permissions, provenance, package allowlist, publication ordering, retry, evidence, and tag/release surfaces are blob-identical. No new credential, workflow, or release regression found.
Reviewed by @probepark — method: finite two-file delta/blob review from bd160753; verified merge-commit identity authority and unchanged release controls.
gajae.pr-review-verdict.v1 merge-approved sha256:5ad0ecdaa3d5e11955d5fa668ef2413e1cba70ec96e69891279a4a866ceef82a reviewer:human reviewer-id:probepark evidence:exact-head-7c4f3d56-squash-release-attribution-fixed
v0.14.1 published a release body containing nothing but a compare link.
generate_release_notes derives from pull requests merged into the target
branch, and a curated release is cherry-picked onto main, so GitHub had no
merged pull requests to describe.
Deriving from the commit range restores the historical What's Changed and
New Contributors sections, but naive attribution is worse than none here:
GET /commits/{sha}/pulls returns every branch containing a commit, which
credited shipped fixes to the 32-commit autoresearch workflow swap this
release deliberately excluded, and to the promotion PR that carried
everything. Preferring the smallest candidate is not enough, since an
excluded PR can be the only candidate. A candidate is therefore accepted
only when the release ships at least half of its commits, and a subject's
own (#N) reference always wins.
Lore-id: b8f2c714
Constraint: release notes must never credit a pull request this release did not ship
Rejected: keep generate_release_notes | produces a bare compare link for curated releases
Rejected: trust the first containing pull request | credits excluded and promotion PRs
Confidence: high
Scope-risk: narrow
Reversibility: easy
Directive: the publish checkout is shallow and tagless; fetch history before deriving
Tested: 17 unit tests plus a real derivation over v0.14.0..v0.14.1 reproducing 40 entries with zero excluded-PR credits
Not-tested: nightly channel derivation in CI
The 0.14.1 OIDC port still let a release publish immutable npm packages with incomplete or wrong GitHub Release metadata, and its retry path was not transaction-safe. This restructures the release pipeline so every fallible step lands before the irreversible npm publication: - release_prepare job (contents+pull-requests read, no id-token) does all untrusted work; publish keeps only contents:write + id-token:write. - Release notes are derived and validated (fail-closed GitHub API reads, deterministic double-derivation, sha256-pinned artifact) before publish. - Previous-release anchor is an exact stable vX.Y.Z ancestor of the head, with a pipe-free no-candidate empty-notes fallback. - Rejected atomic pushes roll back the full local release state (tag and release commit) so the same version retries the original transaction. - The npm CLI pin is exact (11.5.1) and sha512-verified against the registry tarball integrity, no caret range inside the OIDC boundary. Lore-id: b3f2a91c Constraint: publish keeps least privilege -- contents plus id-token only in the publish job Constraint: no release, tag, or version mutation; v0.14.1 is already shipped Rejected: keep notes derivation in publish after npm | published packages would outlive a failed release Rejected: tag-only rollback | retry hits empty Unreleased / nothing-to-commit instead of the original transaction Rejected: npm@^11.5.1 caret range | a drifting CLI inside the credential boundary is not integrity-verifiable Confidence: high Scope-risk: narrow Reversibility: easy Directive: notes must stay derived in release_prepare; never regenerate the body in publish Tested: release-policy, release-publish-order, release-notes, release-retry, check-workflow-permissions suites; check:tools (biome+tsc); live tag-anchor selection and npm tarball sha512 verification Not-tested: live OIDC exchange and end-to-end tagged release run
…r OIDC publish snowykr's second-pass review (4964412257) found the release path still unsafe in two load-bearing ways: a failed `git push --atomic` was treated as proof that nothing shipped, and any matching stable tag could reach the credential-bearing publish job without protected-main provenance. - pushReleaseRefsAtomically now captures a pre-push ls-remote baseline and, on failure, re-queries both remote refs before touching local state: a committed transaction reconciles as success, rollback runs only after proving neither ref moved, and partial/unreachable outcomes preserve the local release commit and tag with explicit guidance. Residual rollback instructions are generated per-outcome and idempotent. - release_metadata validates stable tags against fetched origin/main ancestry; publish is wired to the npm-release approval environment, and the owner-controlled settings that close the tag-creation bypass (main ruleset, v* tag ruleset, environment protection rules) are documented at the job. - The OIDC-capable Node bootstrap is pinned to an exact patch (24.19.0). - Release-notes fallback credits resolve a GitHub login first and render unresolved Git display names without an @ mention. - scripts/release-notes.test.ts and scripts/release-retry.test.ts are now part of test:release, the force-full release-contract CI task. Lore-id: c7d4e8f2 Constraint: no release, tag, or publish mutation; implementation only Rejected: roll back on any failed push | a committed transaction would lose its local release state Rejected: provenance check only in tag-resolved YAML | bypassable by the tagged revision; environment + documented settings carry the boundary Rejected: keep %an as @mention | display names are not logins and mislead attribution Confidence: high Scope-risk: narrow Reversibility: easy Directive: never roll back local release state on an ambiguous push outcome Tested: test:release 150/150 (incl. git-shim accepted-but-failed-push reconciliation, per-outcome rollback guidance, display-name attribution), check:tools, check-workflow-permissions Not-tested: live OIDC exchange, real repository environment/ruleset enforcement
…h boundary Third-pass exact-head review (snowykr 4966590320, probepark 4967848846) found the round-2 state still unsafe and the exact-head CI red: - A client-observed failed atomic push now ALWAYS preserves local release state unless the server proves the transaction committed; baseline-equal re-observation is documented as ambiguous (refs may have been restored, the tag workflow may already be running). Operators reconcile with durable evidence and guarded, idempotent per-artifact rollback commands. - The publish job no longer executes repository-controlled installation: checkout runs with persist-credentials: false, and bun install / cache / lifecycle scripts are gone (the publish script imports only node/bun builtins). The npm-release environment reference is removed: it does not exist, and an environment would change the OIDC subject away from the ref-scoped identity the trusted-publisher registrations and the shipped v0.14.1 exchange prove. Owner-controlled ruleset prerequisites are documented on the job. - check-node20-baseline now accepts an intentionally pinned exact 24.x.y for credential-bearing release jobs (bare "24" still satisfies the baseline; ranges and other majors stay violations), fixing root-check. - Release-notes coverage is computed by commit OID (cherry-pick origins included), so duplicate subjects cannot miscredit a partially shipped pull request. - Rebased onto current dev (1dcf777); the OIDC port commit was dropped as already upstream. Lore-id: e91a4c07 Constraint: no release, tag, publish, or merge; smallest repair on the existing PR branch Rejected: auto-rollback on baseline-equal snapshot | refs may have moved and been restored; tag workflow may be live Rejected: npm-release approval environment | environment does not exist and would silently change the proven OIDC subject Rejected: revert to node-version "24" | a floating bootstrap inside the id-token boundary is a security regression Confidence: high Scope-risk: narrow Reversibility: easy Directive: the OIDC publish job never runs installs and never changes the OIDC subject without re-registering trusted publishers Tested: test:release 152/152, check-node20-baseline suite 15/15, check-workflow-permissions, check:tools; bisect shard green locally after native rebuild (PR touches no product code; bisect source unchanged since old base) Not-tested: live OIDC exchange, CI rerun on this head
…proval environment snowykr's fourth exact-head pass found three remaining P1s: - --publish-from-evidence unconditionally ran checkTypeDeclarations() (`bun x tsc`), reintroducing package resolution/execution inside the id-token/contents-write boundary. Declaration checks now dispatch only on the prepare path, with a dispatch regression test. - The approval gate must not live on the publish job: an environment there would change the OIDC subject away from the proven ref-scoped trusted-publisher identity. A permissionless release_approval job now holds the npm-release environment hook ahead of publish, leaving the publish subject unchanged. Owner prerequisite (required reviewers on npm-release, main + v* tag rulesets) stays documented. - root-check was red on stale generated config schema drift from dev commit bffcb50 (Sentry DSN description); regenerated and committed schemas/config.schema.json. Ambient dev-side drift, unrelated to the release delta; ci:check:full is green locally on this head. Lore-id: 2f8c19d4 Constraint: no release, tag, publish, or merge; dev integration lane only Rejected: environment on the publish job | changes the OIDC sub and breaks the proven trusted-publisher registration Rejected: keep declaration checks in publish | bun x tsc resolves and executes packages inside the credential boundary Rejected: leave schema drift to a dev fix | exact-head CI must be green now; the regen is a disclosed two-line correction Confidence: high Scope-risk: narrow Reversibility: easy Directive: never add dependency resolution or an environment to the publish job Tested: test:release 154/154, ci:check:full (root-check equivalent) green, check-workflow-permissions, check:tools Not-tested: live OIDC exchange, environment approval flow (owner-configured)
snowykr's fifth pass found the release_approval gate ran for nightly channels too, which would turn the documented unattended nightly into an approval-waiting timeout once required reviewers are configured, and restated that the YAML environment reference alone is not a guard. - release_approval now runs only when the channel is stable; publish admits nightly without the approval result and requires it for stable. - The job comment pins the exact owner-controlled prerequisites: npm-release environment with required non-self reviewers and no admin bypass, deployment refs restricted to release refs, and main + v* tag rulesets. The publish OIDC subject stays ref-scoped. - Release-policy regressions cover the nightly-vs-stable graph. Lore-id: 7a3d51e9 Constraint: no release, tag, publish, or merge Rejected: environment on the publish job | changes the proven ref-scoped OIDC subject Rejected: approval-gated nightlies | breaks the documented unattended nightly contract Confidence: high Scope-risk: narrow Reversibility: easy Tested: test:release 155/155, check-workflow-permissions, check:tools Not-tested: live approval flow (owner-configured environment)
…itory code probepark's exact-head review (4979759448) confirmed the install path was fixed but blocked merge on the remaining exposure: `bun scripts/ci-release-publish.ts --publish-from-evidence` executed checkout-controlled code while id-token: write was live. - The publish job is now a fixed boundary: no checkout, no bun, no repository code, no contents scope — only pinned actions and workflow shell. It re-verifies each tarball's sealed sha512 at the boundary and publishes with the exact integrity-pinned npm 11.5.1, tolerating only exact-integrity concurrent publication, with bounded post-publish visibility/integrity/tag verification and a per-package receipt. - GitHub Release finalization moves to release_finalize (contents: write, no OIDC): it requires the boundary's publish receipt, assembles final and channel evidence via ci-release-publish.ts --finalize-evidence (registry re-observation only), creates and verifies the release. - release_prepare captures the pre-publication protected-tag snapshot for the channel evidence since the publish boundary no longer runs repo code. - The permissions allowlist now admits exactly release_finalize.contents and publish.id-token; release.ts watches release_finalize as the finalization job; policy tests pin the fixed boundary (no checkout/setup-bun/scripts/install in the OIDC job) and the new job topology, including negative controls. Owner hold checklist (settings, not mutated by this PR): create the npm-release environment with required non-self reviewers, no admin bypass, and deployment refs restricted to the release refs; protect main and restrict refs/tags/v* creation via rulesets. Lore-id: 4d9f27b1 Constraint: no release, tag, publish, merge, or settings mutation Rejected: keep ci-release-publish.ts as the publisher | it is checkout-controlled code inside the id-token boundary Rejected: environment on the publish job | changes the proven ref-scoped OIDC subject Rejected: drop final/channel evidence assets | public-site sync consumes them Confidence: high Scope-risk: wide Reversibility: migration-needed Directive: the OIDC job must never execute repository code; owner must provision npm-release + rulesets before the next stable release Tested: test:release 155/155, ci:check:full, check-workflow-permissions, check:tools; publish-loop smoke with stubbed npm (publish, skip-if-published, receipt write) Not-tested: live OIDC exchange, environment approval flow
…oundary probepark's re-review (2b9c1f7) closed the credential major and found the replacement introduced a release-integrity one: the fixed-boundary shell iterated the name-sorted evidence array, publishing @gajae-code/agent-core before its same-release workspace dependencies and advancing the dist-tag immediately per package. release_prepare now seals planExpectedEvidencePublication's topological order as gajae-release-publish-order-v1.json, and the fixed publish boundary iterates that sealed plan, verifying it covers exactly the expected package set before publishing. The workflow-consumption gap is covered by a new policy test: the boundary must read the order file and must not iterate the evidence array directly. Lore-id: 8c2e60a4 Constraint: no release, tag, publish, merge, or settings mutation Rejected: jq-side topological sort in the workflow | ordering logic belongs in tested repo code, sealed in prepare Confidence: high Scope-risk: narrow Reversibility: easy Tested: test:release 156/156, workflow-permissions 16/16, check:tools; stubbed-npm loop smoke incl. order-coverage rejection Not-tested: live OIDC exchange
…n time snowykr's latest pass found the new-contributor boundary used `--sort created`, which credits the earliest-created merged PR rather than the earliest-merged one. The search API cannot sort by merge time, so the lookup now fetches a bounded created-asc window with mergedAt and selects the minimum merge timestamp (ties break to the lower PR number). Lore-id: 1b7d93e6 Constraint: no release, tag, publish, or merge Confidence: high Scope-risk: narrow Reversibility: easy Tested: test:release 158/158 incl. merge-time selection and tie-break regressions Not-tested: live search API shape beyond the existing gh wrapper contract
Subject references and bounded GitHub lookups could credit unshipped work or miss the true first contribution. Validate referenced PR coverage and paginate the REST queries so release notes remain conservative and complete. Constraint: release-note attribution must never credit unshipped pull requests Constraint: no release, tag, publish, or merge Tested: bun run test:release; bun run ci:check:full; bun scripts/check-workflow-permissions.ts; bun run check:tools Confidence: high Scope-risk: narrow Reversibility: easy
A referenced pull request can be merged as a squash commit whose SHA differs from every branch commit. Treat the GitHub merge commit as shipped attribution evidence while retaining branch coverage checks for curated releases. Constraint: release notes must preserve shipped pull-request and contributor attribution Constraint: no release, tag, publish, or merge Tested: bun run test:release; bun run ci:check:full; bun scripts/check-workflow-permissions.ts; bun run check:tools Confidence: high Scope-risk: narrow Reversibility: easy
7c4f3d5 to
983356a
Compare
|
Terminal merge evidence for PR #4684:
Signed terminal disposition: MERGED_TO_DEV; post-merge unrelated baseline failures recorded; release boundary preserved. — [repo owner gaebal-gajae (clawdbot) 🦞] |
3 participants
What
Ports the 0.14.1 release-path fix (
19645a5a80, already onmain) todevso the branches do not diverge on the npm publish path.publishjob gainsid-token: writeand pinsnpm@^11.5.1; the ephemeral npmrc and the registry-token env wiring are removed.check-workflow-permissions.tsallowlists exactlypublish.contents+publish.id-token; the policy gates now assert the OIDC contract instead of the token contract.release.tsdeletes the local tag when the atomic push is rejected.scripts/release-notes.tsderives the GitHub Release body from the release range in the historical## What's Changed/## New Contributors/**Full Changelog**shape, and the publish job now passes it asbody_pathwithgenerate_release_notes: false.Why
The first 0.14.1 publish attempt failed with a registry 404 on
PUTbecause the publish credential had expired (last rotated 2026-05-27) while the release tag had already been pushed, so a dead secret could burn a version number. Trusted publishing mints a short-lived credential per run, leaving nothing to expire.The tag-rollback change addresses the second half of that failure: a rejected atomic push used to leave the local tag behind, which made the immutable-tag preflight reject the same version on the next attempt and force a version burn for a release that published nothing.
v0.14.1published a body containing only a compare link, becausegenerate_release_notesderives from pull requests merged into the target branch and a curated release is cherry-picked ontomain. Naive attribution is worse than none:GET /commits/{sha}/pullsreturns every branch containing a commit, which credited shipped fixes to the excluded 32-commitfeat(autoresearch)!swap and to the reverted promotion PR. A candidate is now accepted only when the release ships at least half of its commits, and a subject's own(#N)always wins.Testing
bun test scripts/release-policy.test.ts scripts/release-publish-order.test.ts scripts/check-workflow-permissions.test.ts-> 63 pass, 0 fail.bun scripts/check-workflow-permissions.ts-> ok on all four workflows.bun test scripts/release-notes.test.ts-> 17 pass, 0 fail; combined release gates 80 pass, 0 fail.v0.14.0..v0.14.1reproduced 40 entries with zero credits to excluded pull requests; that output is now the publishedv0.14.1body.v0.14.1tag run onmain; it is not reproducible from a PR branch.Risk classification
low-riskregression-riskhigh-riskGJC verdict
devbun checkpassesNo authenticated approving GitHub review exists for this exact head, so the verdict is
needs-humanrather than an approval. Self-approval would beBLOCK.P1 resolution (head 3450a47)
snowykr's CHANGES_REQUESTED (review 4963962358) is addressed without touching the shipped v0.14.1 release:
release_prepareholdspull-requests: read(policy test asserts it);release-notes.tsgh calls now throw on auth/transport/API errors; empty results only on explicit success or an explicit not-found response (isExplicitEmptyGhResult).release_prepare; the body is sha256-pinned through an artifact and publish only integrity-checks and consumes it. npm publish cannot start before notes exist (needs: [release_prepare, ...]).^v[0-9]+\.[0-9]+\.[0-9]+$tags that are ancestors of the release head only; the current tag is excluded; the no-candidate fallback is pipe-free so pipefail cannot abort it.scripts/release-retry.test.tsagainst real temp git remotes).Verification:
bun test scripts/release-policy.test.ts scripts/release-publish-order.test.ts scripts/release-notes.test.ts scripts/release-retry.test.ts scripts/check-workflow-permissions.test.ts(74 pass),bun run check:tools(biome + tsc clean),bun scripts/check-workflow-permissions.ts(4 workflows ok), live npm tarball sha512 check, live tag-anchor selection, liverelease-notes.tsderivation against the GitHub API.Round-2 resolution (head c409d31)
snowykr's second CHANGES_REQUESTED (review 4964412257) is addressed:
pushReleaseRefsAtomicallycaptures a pre-pushls-remotebaseline and re-queries both refs on failure: a committed transaction reconciles as success (regression-tested with a git shim that pushes but reports failure), rollback runs only when neither ref moved, and partial/unreachable outcomes preserve the local commit and tag with explicit guidance.release_metadatarejects any stable tag whose SHA is not an ancestor of the freshly fetchedorigin/main;publishis wired to thenpm-releaseenvironment. Owner-controlled blocker (cannot be set from code): amainbranch ruleset, arefs/tags/v*tag-creation ruleset restricted to maintainers, andnpm-releaseenvironment protection rules (required reviewers + restricted deployment refs) must be configured in repository settings; the exact settings are documented on the publish job.show-ref && tag -d,rev-parse HEAD && reset --hard), safe to re-run in any order.node-version: "24.19.0", no floating major.@(regression-tested).scripts/release-notes.test.tsandscripts/release-retry.test.tsare now intest:release, the force-full release-contract task.Verification:
bun run test:release150/150,bun run check:toolsclean,bun scripts/check-workflow-permissions.tsok on all four workflows.Round-3 resolution (head 95397b1, rebased onto dev 4b2883b)
snowykr 4966590320 and probepark 4967848846 (merge-blocked) are addressed:
show-ref && tag -d,rev-parse HEAD && reset --hard). Regression: accepted-then-restored reconciliation unit test + preserve-on-rejection integration test over real temp remotes + the git-shim accepted-but-failed case reconciling as success.persist-credentials: false, nobun install, no dependency cache, no lifecycle scripts; the publish script and release-evidence helper import only node/bun builtins, verified. Policy test now forbids install/cache/credential persistence in the publish job.npm-releaseenvironment reference is REMOVED (it does not exist; probepark verified the live environments API). The subject stays ref-scoped (repo:...:ref:refs/tags/v*), the identity the trusted-publisher registrations and the shipped v0.14.1 attestation already prove. Owner-controlled prerequisites (main ruleset,refs/tags/v*tag-creation ruleset) are documented on the publish job; no environment may be introduced without re-registering every package's trusted publisher.check-node20-baseline.tsrequired exactly"24"; it now accepts an intentionally pinned exact24.x.yon release-capable jobs (ranges/other majors still violations, regression-tested) — the exact pin snowykr required is kept. The bisect shard failures are unrelated to this PR: the PR touches no product code,bisectsource and test are byte-identical between the old base and current dev, and the suite passes locally (19/19) once the native addon matches the tree. Rebased onto current dev; exact-head CI reruns on push.Verification:
bun run test:release152/152,bun test scripts/check-node20-baseline.test.ts15/15,bun scripts/check-workflow-permissions.tsok,bun run check:toolsclean,bun test packages/coding-agent/test/tools/bisect.test.ts19/19.New exact-head digest:
93630165b25e83e93025479997ee91a47b42a984a4224e601f2f3408bdff05ffover4b2883bc75...95397b14c5.Round-4 resolution (head 61cb8bc)
snowykr's fourth CHANGES_REQUESTED is addressed:
--publish-from-evidenceno longer runscheckTypeDeclarations();bun x tscdispatch is prepare-path-only, pinned by a dispatch regression test (release-policy.test.ts).release_approvaljob (permissions: {}, no id-token) holds thenpm-releaseenvironment hook ahead of publish, so the publish OIDC subject stays ref-scoped and the existing trusted-publisher registrations/0.14.1 attestation remain valid. Owner prerequisite stays documented: configure required reviewers onnpm-releaseplusmainandrefs/tags/v*rulesets.schemas/config.schema.jsondrift from dev commit bffcb50 (Sentry DSN description), unrelated to this delta; regenerated and committed.bun run ci:check:full(the root-check command) is green locally on this exact head; exact-head CI reruns on push.Verification:
bun run test:release154/154,bun run ci:check:fullgreen,bun scripts/check-workflow-permissions.tsok,bun run check:toolsclean.New exact-head digest:
d340fb927ae91d2bf71d11ec4e9091085a42143b39f9dee2c594d1bc73f5b83aover4b2883bc75...61cb8bc72d.Round-5 resolution (head 27f3243)
snowykr's fifth CHANGES_REQUESTED is addressed:
release_approvalnow runs only forchannel == 'stable';publishadmits nightly without the approval result and requires it for stable, so enabling required reviewers cannot strand scheduled/manual nightlies. Policy regressions pin the graph.main+refs/tags/v*rulesets). These are repository settings and cannot be provisioned from this PR; they remain the documented release prerequisite. The publish OIDC subject stays ref-scoped, preserving the proven trusted-publisher identity.Verification:
bun run test:release155/155, workflow permissions gate ok,check:toolsclean.New exact-head digest:
6ba7bae4c975f8f6bdd06ce8cf3d1f89c3d64950d704a39d02fd7107c84a2798over4b2883bc75...27f3243c2a.Rebase note (head 8576e9e)
Rebased onto current dev tip 7a920df (GitHub refreshed the recorded base to 06f0d4d, which the prior head did not contain, and dev kept moving). Content is unchanged from reviewed head 27f3243 — the round-3 schema regen is now upstream via #4721 and dropped from the diff. Local gates rerun green on the rebased head: test:release 155/155, check:tools, workflow permissions.
New exact-head digest:
8a26399c7f9449f71be6b543d111ebc74f650798f49bca9b821f3d91b3e864acover7a920df9c1...8576e9ee66.Rebase note (head 757a506)
Transplanted onto current dev 02c739e (prior base 7a920df went stale). Content unchanged — the canonical digest 8a26399c… is byte-identical to the previous head's. Local gates rerun green on the transplant: test:release 155/155, ci:check:full (root-check equivalent) clean, workflow permissions ok, check:tools clean.
Rebase note (head 27d1c18)
Rebased onto dev 883ab16 to keep the head containing the live dev tip. Content unchanged; canonical digest still 8a26399c…. test:release 155/155 on the transplant.
Rebase note (head d2830e4)
Rebased onto dev d97b79e (owner rebase lane). Content unchanged; canonical digest still 8a26399c…. test:release 155/155, check:node20-baseline ok, workflow permissions ok, check:tools clean.
Round-6 resolution (head 2b9c1f7)
probepark's merge-block (4979759448) is addressed:
publishjob executes NO checkout-controlled code: no checkout, no bun, no repository scripts, no dependency install/cache, and nocontentsscope at all (id-token: writeonly). Publication runs from pinned actions + workflow shell: each tarball's sealed sha512 is re-verified at the boundary,npm publishuses the exact integrity-pinned npm 11.5.1, concurrent exact publication is tolerated by integrity comparison, post-publish visibility/integrity/tag are verified with bounded backoff, and a per-package receipt (gajae-release-oidc-publish-receipt-v1.json) is persisted.release_finalize(contents: write, no OIDC) requires the receipt, assembles final + channel evidence via the new--finalize-evidencemode (registry re-observation only), then creates and verifies the GitHub Release.release_preparecaptures the pre-publication protected-tag snapshot for channel evidence.release_finalize.contents+publish.id-token;release.tswatchesrelease_finalize; release-policy tests now rejectactions/checkout@,setup-bun@,bun,scripts/, installs, and caches inside the OIDC job, and pin the new topology. The publish-loop smoke was verified locally with a stubbed npm (publish, skip-if-already-published, receipt).Owner hold checklist (settings — not mutated by this PR, verified absent read-only): create environment
npm-releasewith required non-self reviewers, admin bypass disabled, deployment refs restricted to the release refs; protectmain; add a ruleset restrictingrefs/tags/v*creation to maintainers.Verification: test:release 155/155, ci:check:full green, check-workflow-permissions ok, check:tools clean.
New exact-head digest:
8a9a956057f690ae40a654a2762e1614c5f20e03bb84f2dc4e18572c2bb11c79over06b8761c65...2b9c1f7496.Round-7 resolution (head e0a2ac4)
probepark's 2b9c1f7 merge-block is addressed:
release_preparesealsplanExpectedEvidencePublication's topological order asgajae-release-publish-order-v1.json; the fixed boundary iterates the sealed plan, verifies it covers exactly the expected package set, and rejects unknown names. The workflow-consumption regression test pins that the boundary reads the order file and never iterates the evidence array directly.["publish.id-token", "release_finalize.contents"](exact set preserved, negative tests intact).refs/tags/v*creation ruleset plus protectedmainare the owner-controlled load-bearing prerequisites in the hold checklist.Verification: test:release 156/156, check-workflow-permissions.test.ts 16/16, check:tools clean, publish-loop smoke with stubbed npm (publish, skip, order-coverage rejection).
New exact-head digest:
e2e94092b45648a6e7c23bbe68ef1629d77d79cb368286499adc41f9060840f9over06b8761c65...e0a2ac4079.Round-8 note (head d428f2a)
snowykr's latest review arrived with a stale body (its findings 1–5 describe pre-round-3 state and were resolved in earlier rounds; its CI references predate this head) plus one live finding, now fixed: first-merged-PR selection for the New Contributors section compares merge timestamps (
earliestMergedPullRequest, tie-break by PR number) instead of creation order, with regressions. Note: probepark APPROVED the prior head e0a2ac4; this fix advances the head, so fresh exact-head review is required from a non-author collaborator.New exact-head digest:
0debfbf29341a5ffc28526f1546ad6480c051692652d6b57f0b1041b345f5c58over06b8761c65...d428f2a707.Current-dev fix-forward (head 983356a)
Rebased the reviewed PR onto current
dev90049afad30705d5a170ec2ed9fb5b16c2492d54afterdevadvanced. The cumulative source diff remains 13 files / 1,848 insertions and the canonical digest remainssha256:5ad0ecdaa3d5e11955d5fa668ef2413e1cba70ec96e69891279a4a866ceef82a. The exact-head squash-attribution fix is unchanged: GitHub merge/squash OIDs are retained as shipped evidence, with distinct branch/squash regression coverage.Local verification passed after the rebase:
bun run test:release162/162,bun run ci:check:full,bun scripts/check-workflow-permissions.ts, andbun run check:tools. The prior approval at7c4f3d56was invalidated by this required base rebase; an exact-head approval is requested for983356a393709b742caae6f2b34f3b644a81704e. No release, tag, publish, main, repository-settings, environment, ruleset, or merge mutation was performed.Dev-merge disposition
Probepark’s authenticated approval is bound to the exact current head
983356a393709b742caae6f2b34f3b644a81704e. Exact-head contract and state-gate checks are green. The affected-path run’s failed jobs are confined to unchangedpackages/aiandpackages/coding-agentbaseline tests; release-note, release-policy, publish-order, retry, CI self-test, YAML, workflow-permissions, TypeScript, and native build jobs passed. Merging intodevtriggersdev-ci.ymlvalidation only;.github/workflows/ci.ymlpublish paths requiremain,v*tags, schedule, or explicit workflow dispatch, so this merge does not execute release or publish. Owner-controlled release settings and post-merge release execution remain untouched.