docs(release): OIDC setup walkthrough + workflow diagnostics#139
docs(release): OIDC setup walkthrough + workflow diagnostics#139thegdsks wants to merge 1 commit into
Conversation
… fallback in workflow
📝 WalkthroughWalkthroughThe changes add error handling to the npm publish step in the auto-release workflow to validate publish success with detailed failure diagnostics, and introduce comprehensive documentation describing the repository's release automation process across npm, PyPI, and GitHub Releases, including OIDC setup requirements and troubleshooting guidance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/RELEASING.md (1)
101-104: Nit: fenced code block missing language (MD040).markdownlint flags this; tag it as
text(orlog) for consistency with the other fenced blocks in the file.-``` +```text npm notice publish Signed provenance statement with source and build information from GitHub Actions npm notice Provenance statement published to transparency log: https://search.sigstore.dev/...<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/RELEASING.mdaround lines 101 - 104, Update the fenced code block that
contains the "npm notice publish Signed provenance statement..." lines to
include a language tag (use "text" or "log") after the opening triple backticks
so markdownlint MD040 is satisfied; locate the block with the npm notice lines
and change the opening fence fromtotext (or ```log) to match other
blocks in the file.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-release.yml:
- Around line 303-324: The npm publish failure banner currently triggers on any
non-zero exit from the npm publish command; change the logic around the npm
publish invocation (the if ! npm publish --tag "$NPM_TAG" --provenance --access
public; then ... fi block) to capture both the exit status and stderr/stdout
(e.g., run npm publish ... 2>&1 into a variable or temp file and save $?), then
only emit the "trusted-publisher / 404" banner when the captured output or
status matches a 404/ENOENT/not found pattern indicating a trusted-publisher
mismatch; otherwise emit a generic failure message pointing to docs/RELEASING.md
and include the captured npm output for troubleshooting (this keeps the existing
heredoc for the 404 case but replaces the unconditional banner with a
discriminating branch based on the npm publish exit code/output).
Nitpick comments:
In@docs/RELEASING.md:
- Around line 101-104: Update the fenced code block that contains the "npm
notice publish Signed provenance statement..." lines to include a language tag
(use "text" or "log") after the opening triple backticks so markdownlint MD040
is satisfied; locate the block with the npm notice lines and change the opening
fence fromtotext (or ```log) to match other blocks in the file.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `2b3e8490-932e-4b05-9f63-eaaeeb9092ff` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a446a8fde71dacf2e67dce837caf827db3145f5b and f45deec92f0d059459050b78c11944a6a6878dee. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `.github/workflows/auto-release.yml` * `docs/RELEASING.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| if ! npm publish --tag "$NPM_TAG" --provenance --access public; then | ||
| cat >&2 <<'EOF' | ||
|
|
||
| ============================================================= | ||
| npm publish failed. This is almost always because the npm | ||
| trusted-publisher registration on this package does not match | ||
| the workflow that just ran. | ||
|
|
||
| Fix: | ||
| 1. Visit https://www.npmjs.com/package/glin-profanity/access | ||
| 2. Add a GitHub Actions trusted publisher with exactly: | ||
| Organization: glincker | ||
| Repository: glin-profanity | ||
| Workflow: auto-release.yml | ||
| Environment: npm-publish | ||
| 3. Re-run this workflow (gh run rerun <id> --failed). | ||
|
|
||
| See docs/RELEASING.md for the full one-time setup. | ||
| ============================================================= | ||
| EOF | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Banner fires for every publish failure, not just 404.
The fix instructions in the emitted banner assume a trusted-publisher mismatch (404), but this branch runs on any non-zero exit — including 403 (2FA/ownership), network errors, registry outages, or the "version already exists" race (if the pre-check at Line 275 missed it). Operators hitting a 403 will be pointed at the wrong remediation. The "almost always" hedge helps but the explicit 4-step fix is still misleading.
Consider either (a) scoping the banner to 404 by capturing npm's exit or stderr, or (b) restructuring the banner so the 404 fix is clearly one branch alongside the 403/other guidance already documented in docs/RELEASING.md.
♻️ Sketch of a more discriminating variant
- if ! npm publish --tag "$NPM_TAG" --provenance --access public; then
- cat >&2 <<'EOF'
-
-=============================================================
-npm publish failed. This is almost always because the npm
-trusted-publisher registration on this package does not match
-the workflow that just ran.
-
-Fix:
- 1. Visit https://www.npmjs.com/package/glin-profanity/access
- 2. Add a GitHub Actions trusted publisher with exactly:
- Organization: glincker
- Repository: glin-profanity
- Workflow: auto-release.yml
- Environment: npm-publish
- 3. Re-run this workflow (gh run rerun <id> --failed).
-
-See docs/RELEASING.md for the full one-time setup.
-=============================================================
-EOF
- exit 1
- fi
+ PUBLISH_LOG=$(mktemp)
+ if ! npm publish --tag "$NPM_TAG" --provenance --access public 2> >(tee "$PUBLISH_LOG" >&2); then
+ cat >&2 <<'EOF'
+
+=============================================================
+npm publish failed. See docs/RELEASING.md#diagnosing-publish-failures.
+EOF
+ if grep -qE '404|Not Found' "$PUBLISH_LOG"; then
+ cat >&2 <<'EOF'
+
+Likely cause: trusted-publisher mismatch. Fix:
+ 1. https://www.npmjs.com/package/glin-profanity/access
+ 2. Add a GitHub Actions trusted publisher with exactly:
+ Organization: glincker
+ Repository: glin-profanity
+ Workflow: auto-release.yml
+ Environment: npm-publish
+ 3. gh run rerun <id> --failed
+EOF
+ elif grep -qE '403|Two-factor|OTP' "$PUBLISH_LOG"; then
+ echo "Likely cause: 2FA/ownership. See RELEASING.md §'403'." >&2
+ fi
+ echo "=============================================================" >&2
+ exit 1
+ fiNote: the actionlint/yamllint errors on lines 306–307 are false positives — the heredoc contents live inside a run: | bash string, so YAML doesn't parse them.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 306-306: could not parse as YAML: could not find expected ':'
(syntax-check)
🪛 YAMLlint (1.38.0)
[error] 307-307: syntax error: could not find expected ':'
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-release.yml around lines 303 - 324, The npm publish
failure banner currently triggers on any non-zero exit from the npm publish
command; change the logic around the npm publish invocation (the if ! npm
publish --tag "$NPM_TAG" --provenance --access public; then ... fi block) to
capture both the exit status and stderr/stdout (e.g., run npm publish ... 2>&1
into a variable or temp file and save $?), then only emit the "trusted-publisher
/ 404" banner when the captured output or status matches a 404/ENOENT/not found
pattern indicating a trusted-publisher mismatch; otherwise emit a generic
failure message pointing to docs/RELEASING.md and include the captured npm
output for troubleshooting (this keeps the existing heredoc for the 404 case but
replaces the unconditional banner with a discriminating branch based on the npm
publish exit code/output).
There was a problem hiding this comment.
Pull request overview
Adds release documentation and improves workflow diagnostics to unblock npm publishes via OIDC trusted publishing.
Changes:
- Added
docs/RELEASING.mdwith OIDC setup walkthrough, failure-mode diagnostics, and manual publish fallback. - Updated
.github/workflows/auto-release.ymlto print a self-serve trusted-publisher setup banner whennpm publishfails.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| docs/RELEASING.md | New end-to-end release + OIDC setup guide and troubleshooting notes |
| .github/workflows/auto-release.yml | Adds an npm publish failure banner intended to speed up diagnosis and remediation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `release: patch` | patch bump | | ||
| | `release: minor` | minor bump | | ||
| | `release: major` | major bump | | ||
| | `release: beta-minor` | minor bump on beta channel | | ||
| | `release: alpha-patch` | patch bump on alpha channel | |
There was a problem hiding this comment.
The trigger table lists patterns like release: patch / release: minor without the trailing space that the current detection code expects (it matches release: patch etc.). Consider updating the table entries to release: patch <...> / release: minor <...> (or loosen the detection regex) to avoid false expectations when someone copies these literals.
| | `release: patch` | patch bump | | |
| | `release: minor` | minor bump | | |
| | `release: major` | major bump | | |
| | `release: beta-minor` | minor bump on beta channel | | |
| | `release: alpha-patch` | patch bump on alpha channel | | |
| | `release: patch <...>` | patch bump | | |
| | `release: minor <...>` | minor bump | | |
| | `release: major <...>` | major bump | | |
| | `release: beta-minor <...>` | minor bump on beta channel | | |
| | `release: alpha-patch <...>` | patch bump on alpha channel | |
| glin-profanity publishes to **npm** (JS/TS), **PyPI** (Python), and cuts a **GitHub Release** with a signed tag. All three fire automatically from a single merge to `release` via `.github/workflows/auto-release.yml`. | ||
|
|
There was a problem hiding this comment.
This doc states releases fire from a merge/push to the release branch, but the workflow is configured to run on pushes to both main and release (on.push.branches: [ main, release ]). Either update the doc to mention main as well, or adjust the workflow triggers if release is intended to be the sole release branch.
|
|
||
| - Merge a conventional-commit PR to `release`, or push a commit whose subject starts with `release: minor`, `release: patch`, etc. | ||
| - The workflow detects the bump, builds, publishes to npm + PyPI, and creates a GitHub Release. | ||
| - **No tokens in GitHub Secrets.** Trust is established via OIDC. |
There was a problem hiding this comment.
The doc says "No tokens in GitHub Secrets" but the release workflow still uses ${{ secrets.PYPI_TOKEN }} for the PyPI publish job. This is misleading—consider narrowing this to "No NPM_TOKEN in GitHub Secrets" (OIDC for npm), or explicitly call out that PyPI currently still uses a secret until/if it’s migrated to trusted publishing.
| - **No tokens in GitHub Secrets.** Trust is established via OIDC. | |
| - **No `NPM_TOKEN` in GitHub Secrets.** npm publishing uses OIDC; PyPI may still use `PYPI_TOKEN` until trusted publishing is configured there. |
| The workflow triggers on every push to `release`. `scripts/sync-versions.js detect` parses the latest commit subject. Patterns that trigger a bump: | ||
|
|
||
| | Commit subject starts with | Result | | ||
| |---|---| | ||
| | `release: patch` | patch bump | | ||
| | `release: minor` | minor bump | | ||
| | `release: major` | major bump | | ||
| | `release: beta-minor` | minor bump on beta channel | | ||
| | `release: alpha-patch` | patch bump on alpha channel | |
There was a problem hiding this comment.
The doc says commit subjects starting with release: patch, release: minor, etc. trigger bumps, but the current detection regex in scripts/sync-versions.js requires a trailing space after the type (e.g. ^release: patch ). A subject exactly release: patch would not match—update the doc examples to include an additional message after the type, or adjust the regex to accept end-of-line.
| The workflow triggers on every push to `release`. `scripts/sync-versions.js detect` parses the latest commit subject. Patterns that trigger a bump: | |
| | Commit subject starts with | Result | | |
| |---|---| | |
| | `release: patch` | patch bump | | |
| | `release: minor` | minor bump | | |
| | `release: major` | major bump | | |
| | `release: beta-minor` | minor bump on beta channel | | |
| | `release: alpha-patch` | patch bump on alpha channel | | |
| The workflow triggers on every push to `release`. `scripts/sync-versions.js detect` parses the latest commit subject. Example commit subjects that trigger a bump: | |
| | Commit subject | Result | | |
| |---|---| | |
| | `release: patch cut release` | patch bump | | |
| | `release: minor cut release` | minor bump | | |
| | `release: major cut release` | major bump | | |
| | `release: beta-minor cut release` | minor bump on beta channel | | |
| | `release: alpha-patch cut release` | patch bump on alpha channel | |
| if ! npm publish --tag "$NPM_TAG" --provenance --access public; then | ||
| cat >&2 <<'EOF' | ||
|
|
||
| ============================================================= | ||
| npm publish failed. This is almost always because the npm | ||
| trusted-publisher registration on this package does not match | ||
| the workflow that just ran. |
There was a problem hiding this comment.
The trusted-publisher guidance banner is printed for any npm publish failure, but the surrounding comments/PR description frame it as 404-specific. Consider capturing npm publish output and only printing this banner when the output indicates a 404/E404; otherwise, emit a generic failure message so 403/network/other errors don’t get misdiagnosed.
Why
npm publish via OIDC has been failing with
404 Not Foundbecause the npm-side trusted-publisher config isn't registered for this repo. The workflow is already correctly OIDC-ready; only the npm settings page needs a one-time configuration.What
docs/RELEASING.md— walks through the exact npm UI click-path, explains why each field matters, lists the failure modes (404 vs 403 vs skip), and documents the manual emergency-publish fallback..github/workflows/auto-release.yml— wrapsnpm publishin a trap that prints a self-documenting error banner with the trusted-publisher fix when 404 hits, so future maintainers don't have to hunt through logs.After merge
glincker, repoglin-profanity, workflowauto-release.yml, environmentnpm-publishgh workflow run auto-release.yml -f release_type=patch -f channel=stable— next publish will succeed via OIDC and carry a sigstore provenance attestation.No NPM_TOKEN secret ever enters the repo.
Summary by CodeRabbit