chore: retire the legacy CI review pipeline - #32
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach Review: The approach looks good. The PR does two things: deletes the retired pr-review.yml workflow, and syncs "managed-file drift" that had accumulated behind it via the codebase's existing align tooling. Both are the correct tools for their respective jobs.
The three bug fixes riding along are targeted and well-reasoned:
- Pre-commit hook reordering (
pnpm installbeforepnpm dedupe --check): correct fix for a topology-mismatchednode_modulespurge. pnpm_config_verify_deps_before_run: "false": correct workaround for pnpm 11 silently re-running install with lifecycle scripts enabled, dropping--ignore-scriptshardening. The comment correctly notes only thepnpm_config_spelling works.npm_config_fetch_timeouttuning incommit-msg.yml: correct fix — bounding the request keeps retries useful, whereas bumping the job timeout just makes the tail longer.
The @types/node downgrade to ^24.0.0 correctly matches the runtime major (Node 24+), and the deliberate Node pin in commit-msg.yml (not reading from the PR tree) is a sound security-conscious choice for a workflow that runs version-pinned tooling.
No fundamentally better approach exists for this task.
Approach review by Approach Review 3 (OpenCode Wafer) (GLM-5.2)
![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: fd864938bd
ℹ️ 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".
| # pnpm 11 defaults verify-deps-before-run to "install": the first `pnpm | ||
| # <script>` or `pnpm exec` whose deps-status check sees drift re-runs a full | ||
| # install with lifecycle scripts enabled, silently dropping this workflow's | ||
| # install flags — a prepare hook the hardened --ignore-scripts install skipped | ||
| # then runs in CI. Only this spelling disables it: pnpm reads |
There was a problem hiding this comment.
This newly added seven-line comment block violates the repository requirement that comments remain one short line and avoid multi-line blocks. Retain only the non-obvious invariant—for example, that disabling automatic reinstalls preserves the hardened install flags—so the workflow remains compliant and maintainable.
AGENTS.md reference: AGENTS.md:L87-L87
Useful? React with 👍 / 👎.
| # npm's fetch-timeout defaults to 300s — exactly this job's cap — so one | ||
| # stalled request eats the whole budget in silence and the job is killed | ||
| # before npm's own retry ever fires. Observed as five consecutive 5m00s | ||
| # timeouts across align and cluster while the same install took 6s when | ||
| # it worked. Bounding the request keeps the retries useful and turns the |
There was a problem hiding this comment.
This six-line incident narrative violates the repository's explicit one-short-line limit for comments. Reduce it to the durable reason the settings exist, such as keeping retries within the job timeout, rather than preserving historical timing details in the workflow.
AGENTS.md reference: AGENTS.md:L87-L87
Useful? React with 👍 / 👎.
| # pnpm 11 defaults verify-deps-before-run to "install": the first `pnpm | ||
| # <script>` or `pnpm exec` whose deps-status check sees drift re-runs a full | ||
| # install with lifecycle scripts enabled, silently dropping this workflow's | ||
| # install flags — a prepare hook the hardened --ignore-scripts install skipped | ||
| # then runs in CI. Only this spelling disables it: pnpm reads |
There was a problem hiding this comment.
This newly introduced multi-line block also conflicts with the repository rule requiring comments to be a single short line. Preserve only the underlying safety constraint—that automatic dependency verification must not reinstall without the hardened flags—and remove the extended operational description.
AGENTS.md reference: AGENTS.md:L87-L87
Useful? React with 👍 / 👎.
1 participant
Removes
pr-review.yml, the GitHub half of the retired AI review path. Its writers and receipts are already gone, and align no longer renders it.The rest is managed-file drift that had accumulated behind it, so the diff is wider than the deletion:
@types/nodedrops from^25.9.3to^24.0.0to match the runtime baseline's major. Worth a look, since it's a downgrade rather than a sync..node-versionarrives at 26.5.0.checks.ymlgains a 24.15.0/26.5.0 matrix, and the other workflows read the toolchain from the file instead ofpackage.json.>=24.15.0, README follows, andknip.json, the tsconfigs,.oxfmtrc.json, and the pre-commit hook resync to current templates.release.config.mjsandrelease-npm.yml's release arm stay withheld. Their analyzer plugins load the conventionalcommits preset by string andconventional-changelog-conventionalcommitsisn't installed, so align skips them rather than shipping a release that would fail. That dependency is a separate change.