Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08cf074f2e
ℹ️ 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".
tools/argument-comment-lint/run.sh
Outdated
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" | ||
| lint_path="$repo_root/tools/argument-comment-lint" | ||
| manifest_path="$repo_root/codex-rs/Cargo.toml" | ||
| release_manifest_url="https://github.com/openai/codex/releases/latest/download/argument-comment-lint" |
There was a problem hiding this comment.
Pin the packaged lint runner to the checked-out revision
When dotslash is installed, run.sh now pulls https://github.com/openai/codex/releases/latest/... and executes that package instead of the tools/argument-comment-lint code in the current checkout. That makes lint results non-reproducible across branches/tags and, more importantly, it means local validation of changes under tools/argument-comment-lint will silently ignore the branch's own lint/runner edits until a release is cut. Please tie the packaged runner to the checked-out version (or disable it when this directory differs) so ./tools/argument-comment-lint/run.sh actually tests the code being reviewed.
Useful? React with 👍 / 👎.
6d9c0c6 to
eef5b65
Compare
a1db96f to
eec3a8e
Compare
cbd0ecf to
bfe26bd
Compare
69544dc to
0a9e8c3
Compare
Why
The argument-comment lint now has a packaged DotSlash artifact from #15198, so the normal repo lint path should use that released payload instead of rebuilding the lint from source every time.
That keeps
just clippyand CI aligned with the shipped artifact while preserving a separate source-build path for people actively hacking on the lint crate.The current alpha package also exposed two integration wrinkles that the repo-side prebuilt wrapper needs to smooth over:
@nightly-2025-09-18-aarch64-apple-darwin, and Dylint derivesRUSTUP_TOOLCHAINfrom that filenameRUSTUP_HOMEto be present in the environmentWithout those adjustments, the prebuilt CI jobs fail during
cargo metadataor driver setup. This change makes the checked-in prebuilt wrapper normalize the packaged library name to the plainnightly-2025-09-18channel before invokingcargo-dylint, and it teaches both the wrapper and the packaged runner source to inferRUSTUP_HOMEfromrustup show homewhen the environment does not already provide it.After the prebuilt Windows lint job started running successfully, it also surfaced a handful of existing anonymous literal callsites in
windows-sandbox-rs. This PR now annotates those callsites so the new cross-platform lint job is green on the current tree.What Changed
tools/argument-comment-lint/argument-comment-lintDotSlash manifesttools/argument-comment-lint/run.shas the source-build wrapper for lint developmenttools/argument-comment-lint/run-prebuilt-linter.shas the normal enforcement path, using the checked-in DotSlash package and bundledcargo-dylintjust clippyandjust argument-comment-lintto use the prebuilt wrapper.github/workflows/rust-ci.ymlso source-package checks live in a dedicatedargument_comment_lint_packagejob, while the released lint runs in anargument_comment_lint_prebuiltmatrix on Linux, macOS, and Windowsnightly-2025-09-18toolchain install in the prebuilt CI matrix, since the prebuilt package still relies on rustup-provided toolchain componentstools/argument-comment-lint/run-prebuilt-linter.shto normalize host-qualified nightly library filenames, keep therustupshim directory ahead of direct toolchaincargobinaries, and exportRUSTUP_HOMEwhen needed for Windows Dylint driver setuptools/argument-comment-lint/src/bin/argument-comment-lint.rsso future published DotSlash artifacts apply the same nightly-filename normalization andRUSTUP_HOMEinference internallycodex-rs/windows-sandbox-rsby adding the required/*param*/comments at the reported callsitesRUSTUP_HOMErequirement intools/argument-comment-lint/README.md