Conversation
Castiron-Internal-PR: openai/openai-java-internal#209 Castiron-Source-SHA: 23bc30f0693bf258a1b6ff2b68c402c7e7e09dae Castiron-Public-Base-SHA: ef6d7ec
Castiron custom code✅ No new custom-code files detected. 90 mixed files remain; 0 existing customizations changed; 3 customizations removed; 7 generated baselines changed. Compared
86 existing customizations unchanged
46 more in the full report. 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 37029182029 --repo openai/openai-java \
--name castiron-custom-code-37029182029-1 --dir /tmp/castiron-custom-code-37029182029-1
git apply --stat /tmp/castiron-custom-code-37029182029-1/custom-code.patch
cat /tmp/castiron-custom-code-37029182029-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin ef6d7ec0aff7eb6baa52090d839469cef85823ad 6afee1091d77c40873cf44abf4e982ba0908fc5e
python3 scripts/castiron/custom_code_report.py report \
--base ef6d7ec0aff7eb6baa52090d839469cef85823ad \
--head 6afee1091d77c40873cf44abf4e982ba0908fc5e --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-6afee1091d77
cat /tmp/castiron-custom-code-6afee1091d77/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
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. |
![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: 6afee1091d
ℹ️ 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".
| concurrency: | ||
| group: castiron-custom-code-comment-${{ github.event.workflow_run.head_repository.id }}-${{ github.event.workflow_run.head_branch }} | ||
| cancel-in-progress: false | ||
| queue: max # Preserve newer pending evaluations if older runs arrive out of order. |
There was a problem hiding this comment.
GitHub's workflow syntax only supports group and cancel-in-progress in the concurrency mapping; queue is not valid there. This makes the trusted workflow_run workflow invalid, preventing all custom-code status and comment jobs from running. Remove this key or implement ordering with supported workflow logic, and update the authoritative generator template rather than only its generated output.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
| for (const pull of pulls) { | ||
| const {data: current} = await github.rest.pulls.get({...context.repo, pull_number: pull.number}); | ||
| if (current.state !== 'open' || current.head.sha !== run.head_sha) continue; |
There was a problem hiding this comment.
When workflow_run.pull_requests is empty—common for fork runs—and a commit is associated with multiple PRs, this fallback loops over every associated PR and verifies only that it is open at the same SHA. A failed computation can therefore create or overwrite the Castiron comment on another PR sharing that commit, including one targeting a non-main branch because the producer has an unrestricted pull_request trigger. Restore the main-target/repository checks and require exactly one matching PR before writing, with a regression case covering multiple associations.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 6afee1091d77 against its base and the shared Python/Castiron implementation. The generated files match Python, but this promotion introduces 2 publication/discovery regressions relative to this SDK's previous guards, detailed inline. Please fix the canonical implementation and regenerate all SDKs together. The Kotlin edits are KDoc-only.
Source-only review; no repository workloads run. All reported hosted checks in the inspected exact-head snapshot were successful or skipped. I checked the moved test coverage and file sizes; there is no new 1,000-line crossing. The intentional captured-main behavior is not itself a finding. queue: max is supported by current GitHub concurrency documentation, so the existing unsupported-key warning is not a blocker.
| }); | ||
| for (const pull of pulls) { | ||
| const {data: current} = await github.rest.pulls.get({...context.repo, pull_number: pull.number}); | ||
| if (current.state !== 'open' || current.head.sha !== run.head_sha) continue; |
There was a problem hiding this comment.
[P2] Keep failure reports behind the same live-run/PR validation
This replaces the previous live-run and unique-target checks with the event payload and an open/same-SHA test. A delayed attempt-1 publisher can reach trusted_report while attempt 2 is in progress: the live-status check raises, so compute fails and this fallback runs. With no newer comment marker yet, it can overwrite the report with an obsolete failure. The loop can also write to multiple same-SHA PRs, including a different base/source branch that the previous resolver rejected.
Please fix this in the shared template and regenerate: resolve the current source run/attempt and exactly one matching source-repository/branch PR targeting main once, and reuse that decision for success and failure publication. Recheck freshness before writing. Cover a newer retry still in progress and an unrelated same-SHA PR; the current failure fixtures omit these identity fields.
| "GET", f"{root}/commits/{require_sha(run['head_sha'])}/pulls?per_page=100" | ||
| ) | ||
| if associated: | ||
| return cast(list[dict[str, Any]], associated) |
There was a problem hiding this comment.
[P2] Fall back after validating commit-associated PRs
Previously an empty run association used the paginated, scoped open-branch lookup directly. Now any nonempty commit-association response returns before checking whether its PRs are still open/current and target main. If that response contains only a closed or stale PR, trusted_report filters it out and exits without ever discovering the valid current fork PR. The commit lookup also reads only its first 100 results. This can leave the current PR without its report and required budget statuses.
Please keep candidate discovery and current-target validation together in the canonical resolver: paginate, validate, and use the scoped open-branch lookup when no valid candidate remains (or keep the previous direct lookup). Preserve rejection of multiple valid targets and add a nonempty stale-association/valid-branch regression. Regenerate the SDK copies together.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Requesting changes on 6afee1091d77 for two independently confirmed regressions already described inline:
- Failure publication loses live-run/current-target guards.
- Nonempty stale commit associations suppress valid fork-PR discovery: the new resolver returns before live validation, so callers can discard every result without trying the scoped open-branch fallback.
Please fix the canonical Castiron resolver/publisher and regenerate together. I have not duplicated the inline threads. Shared output matches the merged templates; the Kotlin changes are documentation-only. Hosted head checks passed or were skipped. Source-only review; no local contributor code executed.
This branch has not been deployed
3 participants
Standardizes custom-code reporting, budget evaluation, publication tooling, and offline tests. PR evaluations use a captured main checkout and source-run head; publication retains pending results and rejects stale publications. Also clarifies stream-event visitor fallback comments.
The existing API reference, runtime behavior, public API, and repository-owned budget policy remain unchanged.
Validation: SDK formatting and all 59 offline custom-code tests passed locally, including the compiler hash contract.