test: isolate line-ending fixture from commit signing - #2674
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. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 34072948720 --repo openai/openai-node \
--name castiron-custom-code-34072948720-1 --dir /tmp/castiron-custom-code-34072948720-1
git apply --stat /tmp/castiron-custom-code-34072948720-1/custom-code.patch
cat /tmp/castiron-custom-code-34072948720-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin fdb038e21509264bcdf81741a40988dd694188b1 5aa0435e51567ffabccd9351b95c517cff0f3742
python3 scripts/castiron/custom_code_report.py report \
--base fdb038e21509264bcdf81741a40988dd694188b1 \
--head 5aa0435e51567ffabccd9351b95c517cff0f3742 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-5aa0435e5156
cat /tmp/castiron-custom-code-5aa0435e5156/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed the complete test and runner/config propagation. This is the direct fixture-level fix: runGit uses the newly initialized temporary repository, so commit.gpgsign is written locally; source/global settings and hook configuration are unchanged. Both commits, CRLF-to-LF assertions, clean-index check, and finally cleanup remain intact. File growth is 472 → 474 lines, with no structural regression or warranted extra abstraction.
Independent validation: the canonical focused test fails at the first commit on main with a harmless failing synthetic signer and passes at this head on Node 22.23.2 and 24.20.0. Native Git checks confirmed local config origin, two disposable commits, unchanged global signing/hook settings, and cleanup; source/user/review-store config hashes stayed unchanged. Changed-file formatting, lint, and diff checks pass.
Limits: the full changed-file run passed 10 tests; its separate public-package-command test was blocked by pnpm's store database access error, including after a private-store retry. Exact Node 22.0.0 gets past the commits but hits the existing Oxfmt TypeScript-config loader requirement. Repository tooling targets Node 24; this does not change the SDK consumer floor. I did not run the full SDK suite or live API tests.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head 5aa0435 against the scoped goal of isolating the disposable CRLF fixture repository from contributor commit-signing settings. The local commit.gpgsign=false is set after fixture initialization and before both commits, affects only the temporary repository, and leaves the line-ending assertions and source-repository configuration unchanged. Reproduced under a synthetic global SSH signing policy using /usr/bin/false; the focused regression passed 1/1. The changed file passed oxfmt and oxlint, and git diff --check passed. No unresolved threads or in-scope blockers; hosted build, lint, CodeQL, Castiron, and breaking-change checks are green while Node/ecosystem jobs continue.
3 participants
Summary
Keep the disposable Git commits in the CRLF checkout test independent of a contributor's commit-signing setup.
The fixture already supplies a synthetic Git identity, but inherits
commit.gpgsign=true. An unavailable SSH/GPG signer can therefore fail the test before it exercises formatting. Setcommit.gpgsign=falsein the temporary fixture repository's local configuration.This is two added lines in one handwritten test file, including the explanatory comment. All line-ending assertions, commits, cleanup, production code, and real-repository signing settings remain unchanged.
Validation
/usr/bin/falseas the signer, and a synthetic key identifier. Main fails at the first fixture commit; the fix passes under the identical configuration without using a real signer.The exact Node 22.0.0 probe gets past the commits but encounters the existing Oxfmt TypeScript-config loader requirement. Repository tooling continues to target Node 24; no SDK runtime requirement changes.