Conversation
Make `npm i -g @lex0c/forja` work over the same release binaries, so npm is a convenience channel while install.sh + GitHub Releases stay the primary verified path. Uses the standard prebuilt-binary layout — a launcher package with a bin shim plus os/cpu-gated per-platform packages as optionalDependencies — so a manager fetches only the matching binary and no install-time code runs. Scoped because the unscoped `forja` name is npm-retained. The tag becomes the single version source, stamped into version.ts at build time, which also fixes the pre-existing 0.0.0 drift in `--version` and asset names. The npm job reuses the same release-assets artifact, re-verifies each binary against SHA256SUMS (fail-closed), and publishes platform-first/launcher-last and idempotently over OIDC Trusted Publishing with provenance, gated behind the same jobs as the GitHub Release. Spec-first: PERFORMANCE.md §18.6. First npm publish needs a one-time operator bootstrap (own the @lex0c scope, create the package names, enable Trusted Publishing per package).
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9019b2fbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The npm-publish job was appended immediately after the `gh release create` block, landing between it and the `fi` that closed the release step's if/else — orphaning the `fi` onto the npm step, where YAML folded it into the plain `run` scalar (`bun run scripts/npm-publish.ts --out=dist-npm fi`). The result stayed valid YAML (so a YAML-only check passed) but left the GitHub Release shell script with an unterminated `if`, failing every release before publishing. Move the `fi` back into the release block scalar and drop the stray one. Verified with `bash -n` on every run block in the workflow.
A malformed `run:` block (unterminated if/else, stray fi, unclosed quote) can stay valid YAML while breaking the shell at release time — the failure mode that just orphaned the GitHub Release step's `fi`. This walks every .github/workflows/*.yml, extracts each step's run scalar, and asserts `bash -n` accepts it, so the break surfaces in `bun test` instead of mid-release.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee55a4273
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
publish-npm depended only on [build, reproducibility, exec-check], so it ran in parallel with publish (the GitHub Release). npm versions are immutable (no --clobber) and the packages point users back to the GitHub Release (README install + the launcher's missing-binary error), so a failed or partial gh release while npm succeeds would ship irreversible packages whose primary verified fallback isn't there. Add publish to publish-npm's needs so npm goes last, only after the release channel is known-good. publish still does not depend on npm, so a broken npm side never wedges the release. Graph: build -> (reproducibility || exec-check) -> publish -> publish-npm. No cycle; the run-block bash -n guard stays green.
npm tags a publish `latest` unless --tag is given, so publishing a prerelease (v1.2.3-rc.1 — accepted by the `v*` trigger and normalizeVersion) would move `latest` onto the RC and `npm i -g @lex0c/forja` would hand every user a prerelease build. Add distTag(): prereleases route to `next`, stable stays `latest`, and SemVer build metadata (after `+`, which may contain `-`) is correctly not treated as a prerelease. publishAll now passes --tag explicitly on every publish so the dist-tag is never left to npm's default. The symmetric GitHub Release path (a prerelease tag becoming `/releases/latest` for install.sh unless marked --prerelease) is not addressed here.
Owner
Author
|
@codex review |
Symmetric with the npm `next` dist-tag fix: install.sh resolves the version via github.com/<repo>/releases/latest, which GitHub excludes prereleases from only when they are marked --prerelease. Without it a vX.Y.Z-rc.N tag becomes the 'latest' release and install.sh (no --version) would hand every user an RC. Add --prerelease to gh release create for prerelease tags (and gh release edit on the clobber path), using the same SemVer detection as distTag (prerelease is the `-` segment; build metadata after `+` is not). Also bring docs in line with the code: PERFORMANCE.md 18.6 now documents publish-npm depending on `publish` (npm publishes after the GitHub Release is known-good, since npm versions are immutable) and the prerelease routing on both channels; BACKLOG records the post-commit CI hardening (fi fix, bash -n guard, gate-on-publish, prerelease dist-tags). Verified: bash -n on every workflow run block (tests/ci) stays green; prerelease detection checked for stable / -rc / +build / -rc+build tags.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Trusted Publishing (OIDC) can only be configured on a package that already exists, so the very first release must create the 6 package names with token auth. This step writes ~/.npmrc from the NPM_TOKEN secret before publishing; --provenance still attaches (id-token:write is granted on the job), so only auth comes from the token. REMOVE this step and delete the NPM_TOKEN secret once Trusted Publishing is enabled per package — a revert commit follows the first release. TEMPORARY: not intended to reach main. Guard test (bash -n on every run block) stays green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make
npm i -g @lex0c/forjawork over the same release binaries, so npm is a convenience channel while install.sh + GitHub Releases stay the primary verified path. Uses the standard prebuilt-binary layout — a launcher package with a bin shim plus os/cpu-gated per-platform packages as optionalDependencies — so a manager fetches only the matching binary and no install-time code runs. Scoped because the unscopedforjaname is npm-retained.The tag becomes the single version source, stamped into version.ts at build time, which also fixes the pre-existing 0.0.0 drift in
--versionand asset names. The npm job reuses the same release-assets artifact, re-verifies each binary against SHA256SUMS (fail-closed), and publishes platform-first/launcher-last and idempotently over OIDC Trusted Publishing with provenance, gated behind the same jobs as the GitHub Release.Spec-first: PERFORMANCE.md §18.6. First npm publish needs a one-time operator bootstrap (own the @lex0c scope, create the package names, enable Trusted Publishing per package).