Repository navigation
test: make ecosystem CLI checks portable on Windows - #2431
Merged
HAYDEN-OAI merged 3 commits intoAug 25, 2026
Merged
Conversation
HAYDEN-OAI
approved these changes
Aug 25, 2026
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the Windows portability changes: CRLF workflow normalization and the native npm.cmd fixture preserve existing POSIX behavior without changing SDK runtime or public APIs. LGTM.
HAYDEN-OAI
enabled auto-merge
August 25, 2026 17:11
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. |
HAYDEN-OAI
disabled auto-merge
August 25, 2026 17:26
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 25, 2026
## Summary GitHub omits `workflow_run.pull_requests` for external-fork runs, and querying the upstream repository's commit-association endpoint returns no pull requests for those fork commits. That leaves the required `Castiron / budget-only change` and `Castiron / custom-code budget` contexts permanently expected even when the candidate workflow succeeds. - Centralize trusted Python pull-request association in `custom_code_report.py` and reuse it from trusted report generation, comment publication, and budget evaluation. - Resolve missing associations from the authenticated source run's `head_repository`, including legitimately renamed forks, then independently re-fetch every candidate PR from `openai/openai-node`. - Apply equivalent validation in both privileged JavaScript publishers: required commit statuses and failure-report comments. - Refresh the candidate workflow's pinned reporter SHA-256 after the trusted reporter change. ## Security model Fork-side associations and PR numbers are discovery hints, never authorization. The trusted paths: 1. Re-fetch the workflow run from the upstream Actions API and verify its repository, immutable candidate SHA, workflow path, completion, and run attempt as applicable. 2. Strictly validate the source repository's owner/name syntax and consistency with authenticated `head_repository` metadata; reject traversal-like components and spoofed identities. 3. Re-fetch each hinted PR from the upstream repository and require an open PR with the exact candidate SHA, the exact source head repository, the intended upstream repository and `main` base ref, and exactly one valid current association. 4. Require the current `main` base SHA wherever budget evaluation or status publication needs freshness; preserve existing stale-run behavior and merge-group validation. The existing trusted `workflow_run`/main-checkout boundary, bare Git object store, candidate-artifact isolation, least-privilege job permissions, merge-queue protections, and exact required status names remain unchanged. No candidate workflow definition, mutable ref, contributor artifact, or fork-supplied PR number is trusted. ## Affected contributor PRs - #2463: live fork workflow run `32877584723` has `pull_requests: []`; the updated resolver correctly finds its upstream PR through `cmun2/openai-node`. - #2444 and #2431: independently reproduced the same fork-only association behavior; both PRs merged while this fix was being prepared. ## Verification - `env PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s scripts/castiron -p 'test_custom_code*.py'` — 60 tests pass, with one pre-existing skip. - `go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/castiron-custom-code.yml .github/workflows/castiron-custom-code-comment.yml` — both workflows pass actionlint v1.7.12. - `ruff format --check scripts/castiron/custom_code_report.py scripts/castiron/custom_code_budget.py scripts/castiron/test_custom_code_report.py scripts/castiron/test_custom_code_budget.py`. - Ruff lint passes when ignoring only the same pre-existing baseline rule findings; `git diff --check` passes. - Executable publisher and Python regression coverage includes external forks with empty run associations, fork-side lookup, same-repository and renamed-fork PRs, malformed/spoofed repositories, unrelated source heads, ambiguous/duplicate/invalid associations, stale heads/bases/run attempts, exact required contexts, and merge groups.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
tests/ecosystem-cli.test.tsfails on Windows even though its assertions are platform-independent. A CRLF checkout prevents the workflow-condition helper from finding theifblock, and the fake npm command is written only as a POSIX executable, while Windows command resolution expectsnpm.cmd.Baseline on Windows: 5 failed, 12 passed.
Root cause
The test helper parses workflow text with LF-only delimiters, and the subprocess fixture installs only an extensionless shell script on
PATH.Fix
npm.cmdon Windows while preserving the existing POSIX stub elsewhere.Tests
pnpm exec vitest run --config vitest.config.mts --update=none tests/ecosystem-cli.test.ts --reporter=verbose(18 passed)pnpm exec vitest run --config vitest.config.mts --update=none --exclude tests/ecosystem-cloudflare-credential-lifecycle-security.test.ts --reporter=dot(125 files, 3,966 tests passed)pnpm exec oxfmt --check tests/ecosystem-cli.test.tspnpm exec oxlint tests/ecosystem-cli.test.tspnpm exec tsc --pretty falsepnpm buildCompatibility / Risk
Test-only change. SDK runtime behavior and exports are unchanged. POSIX fixtures keep their existing shell-script behavior; Windows uses the native
.cmdlookup path. The broader Windows run excludes the pre-existing credential lifecycle suite because it exercises POSIX file-mode, link, and signal semantics outside this patch.