Skip to content

fix: make npm deprecation reruns idempotent - #694

Merged
haasonsaas merged 3 commits into
mainfrom
codex/npm-deprecate-idempotent-20260529
May 30, 2026
Merged

haasonsaas merged 3 commits into
mainfrom
codex/npm-deprecate-idempotent-20260529

Conversation

@haasonsaas

@haasonsaas haasonsaas commented May 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • verify registry deprecation state when npm returns E422 during deprecate
  • treat already-deprecated targets as successful reruns
  • cover the E422 rerun path in the deprecation script test

Internal PR

  • evalops/maestro-internal#2358

Verification

  • node scripts/run-vitest.js --run test/scripts/deprecate-release.test.ts
  • node --check scripts/deprecate-release.js
  • pre-commit guardian/build hook passed

@github-actions

Copy link
Copy Markdown
Contributor

This PR changes mirrored Maestro source files in the public repo, but it does not link the matching private source-of-truth PR.

Add one of these to the PR body, then re-run the check:

  • https://github.com/evalops/maestro-internal/pull/<number>
  • evalops/maestro-internal#<number>
  • maestro-internal#<number>

Mirrored files touched:

  • test/scripts/deprecate-release.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9847ef80d1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread scripts/deprecate-release.js Outdated
Comment on lines +124 to +126
if (Array.isArray(value)) {
return value.some(hasDeprecationValue);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require every matched version to be deprecated

For range targets such as the workflow default >=0.10.8 <=0.10.20, npm view <pkg>@<range> deprecated --json can return one value per matching version (npm docs: ranges matching multiple versions produce a value for each version), but this helper accepts the rerun as successful if any array/object entry has a deprecation message. If npm deprecate returns E422 because only one version in the range was already deprecated while later versions are still missing the field, this exits 0 and the workflow can leave part of the bad release range visible as supported. The verification should ensure all matched versions have a non-empty deprecation value, not just some of them.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e9b50e8b1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

const result = await runNpmCommand(npmCommand, npmArgs);
if (result.status !== 0) {
const output = `${result.stdout}\n${result.stderr}`;
if (output.includes("E422") && (await verifyDeprecation(npmCommand, spec))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Capture E422 output before checking it

When this script is rerun from an interactive terminal, runNpmCommand takes the stdio: "inherit" branch, so result.stdout and result.stderr are empty even though npm printed E422. In that context this check never calls verifyDeprecation, and an already-deprecated target still exits 1, which leaves the new idempotent rerun path working in CI but not for the local/OTP workflow the script supports.

Useful? React with 👍 / 👎.

@haasonsaas
haasonsaas merged commit 1c951bd into main May 30, 2026
14 of 15 checks passed
@haasonsaas
haasonsaas deleted the codex/npm-deprecate-idempotent-20260529 branch May 30, 2026 01:24

This branch was previously deployed

1 inactive deployment
npm-release — 1e9b50e8 Deployed May 30, 2026 by haasonsaas via deprecate #17
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