Skip to content

chore: add changeset for type fix and null sanitization - #4

Merged
robert-j-y merged 1 commit into
mainfrom
devin/1775172837-add-changeset
Apr 2, 2026
Merged

chore: add changeset for type fix and null sanitization#4
robert-j-y merged 1 commit into
mainfrom
devin/1775172837-add-changeset

Conversation

@robert-j-y

Copy link
Copy Markdown
Collaborator

Summary

Adds a patch changeset for the changes merged in #3 (type error fixes, null→undefined sanitization in applyNextTurnParamsToRequest, release-gated publishing, changeset skill docs). This changeset was missing from the original PR.

Review & Testing Checklist for Human

  • Verify patch is the correct bump level — the changes were bug fixes and defensive coding (no new features or breaking changes), so patch should be appropriate
  • After merging, run the Release workflow with mode: version from the Actions tab to consume this changeset and create a Version Packages PR

Notes

  • This is just a changeset file — no functional code changes

Link to Devin session: https://app.devin.ai/sessions/7690662a430848d9be47c3b020fdb50f
Requested by: @robert-j-y

Co-Authored-By: Robert Yeakel <robert.yeakel@openrouter.ai>
@robert-j-y
robert-j-y merged commit 7c29c2f into main Apr 2, 2026
1 check passed
@github-actions github-actions Bot mentioned this pull request Apr 2, 2026
LukasParke added a commit that referenced this pull request Jul 24, 2026
…red publishing

Both reviewers flagged that the SKILL.md correction surfaced a real
contradiction rather than creating one: the 'publish' mode help text still
said to run it only AFTER merging the Version Packages PR, implying the merge
itself does not publish, while the push leg has been passing publish: to
changesets/action all along.

Documents current behavior in both places and records the divergence from the
original release-gating decision (@openrouter/agent@0.1.1, PR #4), including
that the npm-publish environment has no protection rules. No behavior change:
this PR does not touch publish triggering. Restoring the gate is an owner
decision, called out in the PR thread.
LukasParke added a commit that referenced this pull request Jul 28, 2026
…flow (#82)

* fix(release): use real npm trusted publishing (OIDC) for publish workflow

The release workflow claimed to use OIDC but never did. changesets/action
only checks `process.env.NPM_TOKEN`, and the workflow passed the secret as
`NODE_AUTH_TOKEN`, so the action logged "No NPM_TOKEN found, but OIDC is
available - using npm trusted publishing" while the actual credential came
from the .npmrc written by the `registry-url` option of actions/setup-node.

That token is scoped to selected packages, so the first publish of
@openrouter/mcp failed with `E404 PUT /@openrouter%2fmcp` -- npm returns 404,
not 403, when a credential may not create a package name.

Real OIDC could not have worked either: `node-version: 22` ships npm 10.x,
which has no `lib/utils/oidc.js` and therefore no token-exchange support
(requires npm >= 11.5.1).

Changes:
- Bump the runner to Node 24 (npm 11.16.0). `pnpm publish` shells out to
  whichever `npm` is on PATH, so this is what performs the exchange.
- Drop `registry-url` and every NODE_AUTH_TOKEN. With `registry-url` set but
  no token in the environment, npm leaves the literal `${NODE_AUTH_TOKEN}`
  placeholder in .npmrc and sends it as a bearer token, which breaks the
  `npm info` lookups changesets runs before publishing.
- Drop `--provenance` from the dry-run; trusted publishing attests
  automatically and dry runs never reach the exchange anyway.

Requires a trusted publisher on npmjs.com for each package:
repo OpenRouterTeam/typescript-agent, workflow file publish.yaml,
environment npm-publish.

* fix(release): guard npm version for OIDC + correct release docs

Addresses PR review feedback.

Add a "Verify npm supports trusted publishing" step that fails the release
run early if the runner's npm is older than 11.5.1. The original breakage was
hard to diagnose precisely because an npm without OIDC support surfaces as a
cryptic `E404 PUT` / `ENEEDAUTH` at publish time; this turns a node-version
regression into an explicit error instead. Boundary-tested: 11.5.0 fails,
11.5.1 passes.

Update .agents/skills/changeset-versioning/SKILL.md, which had drifted:

- It documented the publish step as `pnpm publish --provenance --access
  public`. The workflow runs `pnpm exec changeset publish --no-git-checks`,
  and under trusted publishing npm attests automatically with no
  `--provenance` flag.
- It claimed publishing is "release-gated via workflow_dispatch -- it does
  NOT auto-publish on push to main". The workflow has triggered on
  `push: main` for its whole history (that is exactly how the failing run
  that prompted this work was invoked), so the doc actively misled operators
  about when a release goes out. Reframed the coordination point as the
  Version Packages PR merge.
- Documented the OIDC prerequisites (id-token, Node 24+, per-package trusted
  publisher fields, npm's lack of validation on save) and the first-publish
  bootstrap limitation from npm/cli#8544.
- Noted that `--dry-run` never reaches the token exchange, so it cannot
  verify auth.
- Fixed stale details: single-package changeset prompt (mcp exists now) and
  "lint with eslint" (repo uses biome).

* docs(release): reconcile workflow_dispatch help text with push-triggered publishing

Both reviewers flagged that the SKILL.md correction surfaced a real
contradiction rather than creating one: the 'publish' mode help text still
said to run it only AFTER merging the Version Packages PR, implying the merge
itself does not publish, while the push leg has been passing publish: to
changesets/action all along.

Documents current behavior in both places and records the divergence from the
original release-gating decision (@openrouter/agent@0.1.1, PR #4), including
that the npm-publish environment has no protection rules. No behavior change:
this PR does not touch publish triggering. Restoring the gate is an owner
decision, called out in the PR thread.

* fix(release): restrict publishing to main and settle the auto-publish policy

Closes the two remaining review threads.

Reviewers noted that with the npm token gone, nothing stood between a merge
and a live publish. Investigating that turned up something sharper: npm's
GitHub trusted publisher can only pin org, repo, workflow filename, and
environment -- there is no branch or ref claim (see `npm trust github`
flags). The `npm-publish` environment also has no deployment branch policy
(`deployment_branch_policy: null`) and no required reviewers. Combined, a
`workflow_dispatch` from any branch could have minted an OIDC credential and
published to npm. Under token auth this mattered less; OIDC mints credentials
on demand from whatever ref reaches the workflow.

Add a job-level `if: github.ref == 'refs/heads/main'` as the in-repo backstop.
The `push` trigger is already limited to main, so this only constrains
`workflow_dispatch`: dispatch from main still works, dispatch from a feature
branch no longer runs.

Also settle the policy question the reviewers escalated rather than leaving it
open: auto-publish on push to main is the intended behavior. The changeset
flow already gates releases (no changeset -> no version bump -> nothing to
publish) and the Version Packages PR is the human review point, so a second
manual publish step adds ceremony without adding control. Documented the
alternative (condition `publish:` on `workflow_dispatch`) for whoever revisits
this, plus a table of what actually protects a release and which environment
settings remain unconfigured as defense-in-depth follow-ups.

* fix(release): correct ref-guard framing, restore off-main dry-run

Addresses the two findings on f8983f7.

The previous comment (and SKILL.md) claimed the job-level ref guard is what
prevents publishing from an arbitrary branch. That was wrong: workflow_dispatch
executes the workflow file from the selected ref, so a branch whose copy drops
the guard simply ignores it. The guard prevents *accidents*, not a determined
actor. Since npm's trusted publisher carries no branch/ref claim, the
`npm-publish` environment's deployment branch policy is the only server-side
ref restriction -- reframed as the actual fix rather than defense-in-depth.

Also restore the off-main `mode=publish` + `dry-run` diagnostic, which the
blanket guard had skipped. That leg never reaches the OIDC exchange, so it
cannot publish.

The exemption is deliberately narrower than the reviewers' suggested
`inputs.dry-run`: with `mode=version` the changesets step runs with `publish:`
wired up and ignores dry-run entirely, so exempting dry-run generally would
have let `mode=version` + `dry-run: true` from a feature branch publish for
real. Scoped to `mode == 'publish' && dry-run`, verified against a truth table
of all seven ref/event/mode/dry-run combinations: every legitimate path runs,
both off-main publishing paths are blocked.

`inputs.dry-run` is safe to use in expressions despite the hyphen -- confirmed
against run 27694931266, which took the live leg and skipped the dry-run leg.

* style(release): drop stray indent so the ref-guard expression stays single-line

The continuation line was indented 7 spaces against the folded block's 6, so
YAML treated it as more-indented and preserved the newline, embedding a \n in
the if expression. Actions' lexer treats \n as whitespace so it evaluated
correctly, but there is no reason to depend on that. Re-verified the full
ref/event/mode/dry-run truth table is unchanged.

* docs(release): finish reconciling skill doc with auto-publish behavior

Two spots still described publishing as a required manual workflow_dispatch
step, contradicting the auto-publish framing established at the top of the
same doc:

- "Step 2: Publish" read as a mandatory step after merging the Version
  Packages PR. That merge is a push to main, which already publishes.
  Reframed as the retry/force path, matching the mode input help text in
  publish.yaml that was corrected in 703e8a7.
- The Configuration list still labeled publish.yaml as "(workflow_dispatch)",
  omitting the push: main trigger that is the primary release path.

Same doc-drift class flagged in review; no behavior change.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant