From 6afee1091d77c40873cf44abf4e982ba0908fc5e Mon Sep 17 00:00:00 2001 From: apcha-oai <228803254+apcha-oai@users.noreply.github.com> Date: Fri, 2 Oct 2026 15:26:45 +0000 Subject: [PATCH] ci: standardize custom-code budget reporting Castiron-Internal-PR: https://github.com/openai/openai-java-internal/pull/209 Castiron-Source-SHA: 23bc30f0693bf258a1b6ff2b68c402c7e7e09dae Castiron-Public-Base-SHA: ef6d7ec0aff7eb6baa52090d839469cef85823ad --- .castiron.stats.yml | 8 +- .../castiron-custom-code-comment.yml | 106 ++-- .github/workflows/castiron-custom-code.yml | 4 +- .../TranscriptionStreamEvent.kt | 4 +- .../models/beta/agents/AgentSessionEvent.kt | 4 +- .../beta/assistants/AssistantStreamEvent.kt | 4 +- .../beta/responses/BetaResponseStreamEvent.kt | 4 +- .../responses/BetaResponsesServerEvent.kt | 4 +- .../models/images/ImageEditStreamEvent.kt | 4 +- .../models/images/ImageGenStreamEvent.kt | 4 +- .../models/live/forks/ForkServerEvent.kt | 4 +- .../models/realtime/RealtimeServerEvent.kt | 4 +- .../models/responses/ResponseStreamEvent.kt | 4 +- .../models/responses/ResponsesServerEvent.kt | 4 +- scripts/castiron/CUSTOM_CODE.md | 29 +- scripts/castiron/README.md | 12 +- scripts/castiron/custom_code_budget.py | 33 +- scripts/castiron/custom_code_report.py | 160 ++--- scripts/castiron/custom_code_test_support.py | 74 +++ .../castiron/fixtures/github_publisher.cjs | 38 ++ scripts/castiron/test_custom_code_budget.py | 542 ++--------------- scripts/castiron/test_custom_code_github.py | 475 +++++++++++++++ .../castiron/test_custom_code_publication.py | 462 +++++++++++++++ scripts/castiron/test_custom_code_report.py | 557 +----------------- 24 files changed, 1298 insertions(+), 1246 deletions(-) create mode 100644 scripts/castiron/custom_code_test_support.py create mode 100644 scripts/castiron/fixtures/github_publisher.cjs create mode 100644 scripts/castiron/test_custom_code_github.py create mode 100644 scripts/castiron/test_custom_code_publication.py diff --git a/.castiron.stats.yml b/.castiron.stats.yml index e371ebe7d..666413edb 100644 --- a/.castiron.stats.yml +++ b/.castiron.stats.yml @@ -1,8 +1,8 @@ schema_version: 1 -generation_id: 37722dc7-eca2-41b0-bfce-0f6baee2f2aa +generation_id: d52034e7-c4ef-4643-b0a5-177be0c8df24 openapi_spec_hash: 641f7e0f4f9849ca42dc9d9ba4f938f6 openapi_transformed_spec_hash: e9bfbf2ff383ae7da0b32dec03b168e9 config_hash: e51f06a3c5b16edb58e2aebe45a67903 -codegen_sha: f09aa003172bb9fe7c58d8d394b439b0f5892b67 -codegen_hash: d5980a6ee9429c1ab2cc493e826c10245a3608e19a1540cd38a23e857f0d484a -public_codegen_sha: 813494a2e0ea4ff52130a75c3e959b4db72534de +codegen_sha: 0bc5285b80b377a352358b04f36fa219e2c96a7c +codegen_hash: d97aec850c8938ce86ee2a2703797a71015582003e7e323b81005c0b4bf1d3b2 +public_codegen_sha: af5a23f281b751563e117b974a795e26fa8943d5 diff --git a/.github/workflows/castiron-custom-code-comment.yml b/.github/workflows/castiron-custom-code-comment.yml index dc6e39f64..5eee3db7e 100644 --- a/.github/workflows/castiron-custom-code-comment.yml +++ b/.github/workflows/castiron-custom-code-comment.yml @@ -11,6 +11,7 @@ permissions: {} 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. jobs: compute: @@ -31,13 +32,15 @@ jobs: isolation: ${{ steps.budget.outputs.isolation }} budget: ${{ steps.budget.outputs.budget }} steps: + # Selecting main here pins both the executable checker and PR policy base. + # Subsequent steps use this checkout's SHA even if main advances. - name: Check out the trusted reporter - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: main persist-credentials: false - - name: Compute from the current pull request Git objects + - name: Compute from the selected main and pull request Git objects id: report if: github.event.workflow_run.event == 'pull_request' env: @@ -53,6 +56,7 @@ jobs: run: | python3 -I scripts/castiron/custom_code_report.py trusted-report \ --repo "$RUNNER_TEMP/castiron-objects.git" \ + --base "$(git rev-parse HEAD)" \ --repository "$REPOSITORY" --run-id "$RUN_ID" --run-attempt "$RUN_ATTEMPT" \ --out "$RUNNER_TEMP/castiron-custom-code" if test -f "$RUNNER_TEMP/castiron-custom-code/context.json"; then @@ -133,52 +137,61 @@ jobs: HEAD_SHA: ${{ needs.compute.outputs.head-sha }} ISOLATION_RESULT: ${{ needs.compute.outputs.isolation }} BUDGET_RESULT: ${{ needs.compute.outputs.budget }} + PUBLISH_ATTEMPT: ${{ github.run_attempt }} with: script: | const event = context.payload.workflow_run; const {data: run} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: event.id}); - if (run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt || + if (run.event !== event.event || run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt || run.status !== 'completed' || run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml' || run.repository.full_name !== `${context.repo.owner}/${context.repo.repo}`) return; const head = run.head_sha; if (!/^[0-9a-f]{40}$/.test(head)) throw new Error('Invalid candidate SHA'); - const {data: repository} = await github.rest.repos.get(context.repo); - const branch = repository.default_branch; - const {data: main} = await github.rest.git.getRef({...context.repo, ref: `heads/${branch}`}); - const base = main.object.sha; + const base = process.env.BASE_SHA; + let fresh = /^[0-9a-f]{40}$/.test(base) && head === process.env.HEAD_SHA; if (run.event === 'pull_request') { - const headRepository = run.head_repository; - if (!headRepository || !Number.isInteger(headRepository.id) || headRepository.id <= 0 || - !headRepository.full_name || !headRepository.owner?.login || !run.head_branch) return; - const pulls = run.pull_requests.length ? run.pull_requests : await github.paginate( - github.rest.pulls.list, {...context.repo, state: 'open', - head: `${headRepository.owner.login}:${run.head_branch}`, base: branch, per_page: 100}); + let pulls = run.pull_requests.length ? run.pull_requests : await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: head}); + if (!pulls.length) pulls = await github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', head: `${run.head_repository.owner.login}:${run.head_branch}`, + }); const current = []; - for (const number of [...new Set(pulls.map(pull => pull.number))].sort((a, b) => a - b)) { - if (!Number.isInteger(number) || number <= 0) return; - const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: number}); - if (pr.state === 'open' && pr.head.sha === head && pr.base.sha === base && - pr.head.ref === run.head_branch && pr.head.repo?.id === headRepository.id && - pr.head.repo?.full_name === headRepository.full_name && - pr.base.ref === branch && - pr.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}`) current.push(pr); + for (const pull of pulls) { + const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: pull.number}); + if (pr.state === 'open' && pr.head.sha === head && + pr.base.ref === 'main' && pr.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}`) current.push(pr); } if (current.length !== 1) return; - } else if (run.event !== 'merge_group' || !run.head_branch.startsWith(`gh-readonly-queue/${branch}/`)) { + } else if (run.event !== 'merge_group' || !run.head_branch.startsWith('gh-readonly-queue/main/')) { return; + } else { + const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'}); + fresh = fresh && base === main.object.sha; } - const fresh = base === process.env.BASE_SHA && head === process.env.HEAD_SHA; const url = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + // Concurrency serializes writes, but does not order runs or retries. + // Keep an older evaluation from overwriting a newer result for this head. + const order = [run.id, run.run_attempt, Number(context.runId), Number(process.env.PUBLISH_ATTEMPT)]; + const marker = `[evaluation ${order.join(':')}]`; + const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, {...context.repo, ref: head}); + const names = ['Castiron / budget-only change', 'Castiron / custom-code budget']; + for (const status of statuses) { + if (!names.includes(status.context) || status.creator?.login !== 'github-actions[bot]') continue; + const prior = status.description?.match(/\[evaluation (\d+):(\d+):(\d+):(\d+)\]$/); + if (!prior || status.target_url !== `${url.slice(0, url.lastIndexOf('/') + 1)}${prior[3]}`) continue; + const previous = prior.slice(1).map(Number); + const different = previous.findIndex((value, index) => value !== order[index]); + if (different !== -1 && previous[different] > order[different]) return; + } for (const [name, result] of [ ['Castiron / budget-only change', process.env.ISOLATION_RESULT], ['Castiron / custom-code budget', process.env.BUDGET_RESULT], ]) { const state = fresh && result === 'success' ? 'success' : 'failure'; - const description = !fresh ? 'Evaluation unavailable or base changed; rerun against current main.' - : state === 'success' ? 'Passed against main policy. See the trusted run summary.' - : 'Budget check failed. See the trusted run summary.'; + const description = !fresh ? 'Evaluation unavailable or queue base changed; inspect the trusted run and rerun.' + : `${state === 'success' ? 'Passed' : 'Failed'} against main ${base.slice(0, 12)}. See the trusted run summary.`; await github.rest.repos.createCommitStatus({...context.repo, sha: head, context: name, - state, description, target_url: url}); + state, description: `${description} ${marker}`, target_url: url}); } comment: @@ -193,7 +206,7 @@ jobs: pull-requests: write steps: - name: Check out the trusted publisher - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.workflow_sha }} persist-credentials: false @@ -230,34 +243,15 @@ jobs: with: script: | const marker = ''; - const event = context.payload.workflow_run; - const {data: run} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: event.id}); - if (run.event !== 'pull_request' || - run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt || - run.status !== 'completed' || - run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml' || - run.repository.full_name !== `${context.repo.owner}/${context.repo.repo}`) return; - const {data: repository} = await github.rest.repos.get(context.repo); - const branch = repository.default_branch; - const {data: main} = await github.rest.git.getRef({...context.repo, ref: `heads/${branch}`}); - const headRepository = run.head_repository; - if (!headRepository || !Number.isInteger(headRepository.id) || headRepository.id <= 0 || - !headRepository.full_name || !headRepository.owner?.login || !run.head_branch) return; - const pulls = run.pull_requests?.length ? run.pull_requests : await github.paginate( - github.rest.pulls.list, {...context.repo, state: 'open', - head: `${headRepository.owner.login}:${run.head_branch}`, base: branch, per_page: 100}); - const current = []; - for (const number of [...new Set(pulls.map(pull => pull.number))].sort((a, b) => a - b)) { - if (!Number.isInteger(number) || number <= 0) return; - const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: number}); - if (pr.state === 'open' && pr.head.sha === run.head_sha && - pr.head.ref === run.head_branch && pr.head.repo?.id === headRepository.id && - pr.head.repo?.full_name === headRepository.full_name && - pr.base.sha === main.object.sha && pr.base.ref === branch && - pr.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}`) current.push(pr); - } - if (current.length !== 1) return; - for (const pull of current) { + const run = context.payload.workflow_run; + if (run.event !== 'pull_request' || run.path !== '.github/workflows/castiron-custom-code.yml') return; + let pulls = run.pull_requests?.length ? run.pull_requests : await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: run.head_sha}); + if (!pulls.length) pulls = await github.paginate(github.rest.pulls.list, { + ...context.repo, state: 'open', head: `${run.head_repository.owner.login}:${run.head_branch}`, + }); + 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; const comments = await github.paginate(github.rest.issues.listComments, {...context.repo, issue_number: pull.number}); const previous = comments.find(c => c.user?.type === 'Bot' && c.user?.login === 'github-actions[bot]' && c.body?.startsWith(marker)); const prior = previous?.body?.match(//); diff --git a/.github/workflows/castiron-custom-code.yml b/.github/workflows/castiron-custom-code.yml index 09debc00e..35d81a023 100644 --- a/.github/workflows/castiron-custom-code.yml +++ b/.github/workflows/castiron-custom-code.yml @@ -16,7 +16,7 @@ concurrency: cancel-in-progress: false env: - REPORTER_SHA256: 66b128590c674d4dec7c59b99dc0de54d7d1672eee4ab6d193bfc6b369a664d0 + REPORTER_SHA256: d92cc331cea526bb3230ea4b0c2430ad974338a8373134e0b07b3a1ad3535e2a jobs: queue-signal: @@ -47,7 +47,7 @@ jobs: > "$RUNNER_TEMP/castiron-custom-code/context.json" - name: Check out the pull request - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 diff --git a/openai-java-core/src/main/kotlin/com/openai/models/audio/transcriptions/TranscriptionStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/audio/transcriptions/TranscriptionStreamEvent.kt index 6704ce4b7..16b16e877 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/audio/transcriptions/TranscriptionStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/audio/transcriptions/TranscriptionStreamEvent.kt @@ -306,8 +306,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/beta/agents/AgentSessionEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/beta/agents/AgentSessionEvent.kt index 4ca7cfc50..f94c71c99 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/beta/agents/AgentSessionEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/beta/agents/AgentSessionEvent.kt @@ -1165,8 +1165,8 @@ private constructor( * version than the API, then the API may respond with new variants that the SDK is unaware * of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/beta/assistants/AssistantStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/beta/assistants/AssistantStreamEvent.kt index 106632f2a..4232269f3 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/beta/assistants/AssistantStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/beta/assistants/AssistantStreamEvent.kt @@ -1221,8 +1221,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponseStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponseStreamEvent.kt index e39a9a079..f0b612f3b 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponseStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponseStreamEvent.kt @@ -2526,8 +2526,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponsesServerEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponsesServerEvent.kt index 85e00c506..0d8587fef 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponsesServerEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/beta/responses/BetaResponsesServerEvent.kt @@ -3125,8 +3125,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/images/ImageEditStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/images/ImageEditStreamEvent.kt index f760bc3fa..1b5767c50 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/images/ImageEditStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/images/ImageEditStreamEvent.kt @@ -193,8 +193,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/images/ImageGenStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/images/ImageGenStreamEvent.kt index 52d10749f..1d6001cee 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/images/ImageGenStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/images/ImageGenStreamEvent.kt @@ -203,8 +203,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/live/forks/ForkServerEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/live/forks/ForkServerEvent.kt index 9e06acd8d..f8176368a 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/live/forks/ForkServerEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/live/forks/ForkServerEvent.kt @@ -1153,8 +1153,8 @@ private constructor( * version than the API, then the API may respond with new variants that the SDK is unaware * of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/realtime/RealtimeServerEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/realtime/RealtimeServerEvent.kt index 24e750138..bdbc548fa 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/realtime/RealtimeServerEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/realtime/RealtimeServerEvent.kt @@ -2536,8 +2536,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponseStreamEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponseStreamEvent.kt index ffe342027..f3779c7ae 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponseStreamEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponseStreamEvent.kt @@ -2240,8 +2240,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponsesServerEvent.kt b/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponsesServerEvent.kt index b8bb89463..eab2ec31e 100644 --- a/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponsesServerEvent.kt +++ b/openai-java-core/src/main/kotlin/com/openai/models/responses/ResponsesServerEvent.kt @@ -2959,8 +2959,8 @@ private constructor( * on an older version than the API, then the API may respond with new variants that the SDK * is unaware of. * - * Recognized events also reach this method when their visit method is not overridden. This - * allows existing visitors to handle event variants added by newer SDK versions. + * Recognized variants also reach this method when their visit method is not overridden. + * This allows existing visitors to handle variants added by newer SDK versions. * * @throws OpenAIInvalidDataException in the default implementation. */ diff --git a/scripts/castiron/CUSTOM_CODE.md b/scripts/castiron/CUSTOM_CODE.md index 47ed32431..ce02af964 100644 --- a/scripts/castiron/CUSTOM_CODE.md +++ b/scripts/castiron/CUSTOM_CODE.md @@ -1,3 +1,5 @@ + + # Custom code The custom-code reporter measures the SDK's remaining customization of generated @@ -38,7 +40,8 @@ The custom-code workflow pair separates candidate execution from trusted checks: - `castiron-custom-code.yml` runs proposed offline tests and the advisory report on `pull_request` with read-only permissions. - `castiron-custom-code-comment.yml` handles `workflow_run` from **main**. Its - read-only compute job runs main's reporter against candidate Git objects in a + read-only compute job captures the main checkout SHA and the source run's PR + head as one immutable evaluation pair. It runs that main reporter against Git objects in a new bare repository, then reuses that verified report to check main's budget. It never checks out, imports, installs, or executes candidate code. - An unprivileged `merge_group` job in the first workflow only signals that a candidate @@ -50,23 +53,19 @@ The custom-code workflow pair separates candidate execution from trusted checks: grant the SDK PR a higher limit. A separate publisher with no checkout attaches these statuses to the exact PR -head or merge-group SHA, after rechecking head/base freshness: +head or merge-group SHA, after rechecking the PR head/target or queue base freshness: - `Castiron / budget-only change` - `Castiron / custom-code budget` -For fork PRs, GitHub can omit `workflow_run.pull_requests` while the PR is open. -The trusted handler then uses the authenticated run's head repository owner and -branch to list open PRs targeting the default branch, fetches every candidate, -and accepts exactly one only after matching its head SHA, head repository and -ref, base repository and ref, and current default-branch SHA. Missing, -ambiguous, spoofed, or stale metadata still fails closed. - -The policy is read from the current base commit, not the PR or its merge base. +The policy is read from the captured main commit, not the PR or its merge base. Reporter changes in a PR cannot change the checker executing on that PR. Missing snapshots, invalid hashes, unavailable queue membership, and policy errors fail -closed. If main moves during evaluation, rerun the workflow; reruns check out the -new main. PR-head statuses are feedback at a point in time: a main update alone +closed. PR base metadata may lag main; it does not select the evaluation base. +If main moves during a PR evaluation, the result still publishes for the captured +pair, with the evaluated base recorded in the report and status. Merge-group checks +still require current main and independently evaluate the combined candidate. +PR-head statuses are feedback at a point in time: a main update alone does not rerun them. The checker therefore fails unless main has an effective **require merge queue** rule. The queue must recheck combined usage against current main before merging; do not replace that protection with PR-head statuses alone. @@ -80,8 +79,10 @@ The trusted run summary reports additions, deletions, total, mixed-file count, headroom, largest patches, and exact policy/candidate/generated revisions. The existing custom-code comment remains unchanged, including when the budget fails. The trusted compute job reuses its own report, never the candidate's artifacts. -The checker, policy, and workflows are maintained in the SDK and preserved -through the normal three-way merge during generation. +The checker, workflows, and offline tests are generated from shared Castiron +templates. The budget policy remains repository-owned and is never generated. +Repository-specific customizations are preserved through the normal three-way +merge during generation. ## Local verification diff --git a/scripts/castiron/README.md b/scripts/castiron/README.md index 16dc15c26..c445a641b 100644 --- a/scripts/castiron/README.md +++ b/scripts/castiron/README.md @@ -7,7 +7,9 @@ for cross-SDK improvements; repository-specific customizations use the normal three-way merge and are allowed. The reporter uses Python 3.10+, Git, and `gh`; it does not import SDK code. -Run `python3 scripts/castiron/test_custom_code_report.py` for focused tests. +Run `python3 -m unittest discover -s scripts/castiron -p 'test_custom_code*.py'` +for the offline suite. Install Node.js to exercise the workflow publishers too. +See [CUSTOM_CODE.md](CUSTOM_CODE.md) for budget policy and activation. The report comment includes commands to inspect the exact custom-code patch. Public reporting uses only public snapshots and needs no private repository access. @@ -16,8 +18,12 @@ Its hash format is documented in the reporter. Only `.github/actions/` and `.github/workflows/` are excluded from the content hash. The read-only pull-request workflow runs on every branch, including drafts and -forks. A separate `workflow_run` publisher reads its report as untrusted data and -uses only code from the trusted default branch to update the PR comment. The +forks. A separate read-only `workflow_run` job computes the authoritative report from +the captured main checkout and GitHub-associated PR head. Merge groups are +checked independently against current main. It fetches those objects into a new bare repository and never checks +out or executes PR code. The comment-writing job consumes only the artifact +from that trusted job, rechecks freshness, and links to its report and patch. +PR-produced reports are advisory run output, not the published assessment. The publisher becomes active once its workflow is on the default branch. No branch allowlist or repository variable is needed. Never execute PR-controlled code with write credentials. Changing either workflow may require one-time AM permission. diff --git a/scripts/castiron/custom_code_budget.py b/scripts/castiron/custom_code_budget.py index ff7f48673..87767d027 100644 --- a/scripts/castiron/custom_code_budget.py +++ b/scripts/castiron/custom_code_budget.py @@ -1,8 +1,9 @@ #!/usr/bin/env python3 +# File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. """SDK custom-code budget gate. Run only from a trusted checkout, never PR code. Reuses Castiron's vendored snapshot verifier and generated-file accounting. This -file and its workflow are maintained in the SDK repository. +file and its workflows are generated from shared Castiron templates. """ from __future__ import annotations @@ -282,9 +283,9 @@ def github_evaluate( if event["repository"]["full_name"] != repository: raise ValueError("event repository mismatch") branch = metadata["default_branch"] - main = report.require_sha(report.api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"]) - if report.require_sha(trusted_sha) != main: - raise ValueError("trusted checkout is stale; rerun against current main") + if branch != "main": + raise ValueError("budget gate requires main as the default branch") + main = report.require_sha(trusted_sha) signal = event["workflow_run"] run_id = signal["id"] if type(run_id) is not int or run_id <= 0: @@ -300,9 +301,29 @@ def github_evaluate( raise ValueError("unexpected or superseded source workflow run") head = report.require_sha(run["head_sha"]) if run["event"] == "pull_request": - if report.current_pull_request(root, repository, run, branch, main) is None: + associated = report.associated_pulls(repository, run) + current: list[int] = [] + for number in sorted({int(pr["number"]) for pr in associated}): + if number <= 0: + raise ValueError("invalid associated PR number") + pull = report.api("GET", f"{root}/pulls/{number}") + if ( + pull["state"] == "open" + and pull["head"]["sha"] == head + and pull["base"]["repo"]["full_name"] == repository + and pull["base"]["ref"] == branch + ): + current.append(number) + if len(current) != 1: raise ValueError("source run must identify exactly one current PR targeting main") elif run["event"] == "merge_group": + # Queue candidates independently validate actual current main. A PR's + # captured snapshot must never authorize a different merged candidate. + current_main = report.require_sha( + report.api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"] + ) + if main != current_main: + raise ValueError("trusted checkout is stale; rerun against current main") if not run["head_branch"].startswith(f"gh-readonly-queue/{branch}/"): raise ValueError("queue signal does not target main") else: @@ -321,7 +342,7 @@ def github_evaluate( raise ValueError("only PR runs can reuse the trusted report") measured = json.loads((trusted_report_dir / "report.json").read_text()) if measured["target_base_sha"] != main or measured["head_sha"] != head: - raise ValueError("trusted report is stale; rerun against current main") + raise ValueError("trusted report does not match the captured base/head") if report.git(repo, "rev-parse", "--is-bare-repository").strip() != b"true": raise ValueError("trusted report must use a bare object store") measurement = (measured, (trusted_report_dir / "custom-code.patch").read_bytes()) diff --git a/scripts/castiron/custom_code_report.py b/scripts/castiron/custom_code_report.py index b5c5307bb..b07820336 100644 --- a/scripts/castiron/custom_code_report.py +++ b/scripts/castiron/custom_code_report.py @@ -22,10 +22,10 @@ import struct import subprocess import sys -import urllib.parse from dataclasses import dataclass from pathlib import Path from typing import Any, cast +from urllib.parse import urlencode DOMAIN = b"castiron-codegen-v1\0" MARKER = "" @@ -605,6 +605,7 @@ def render_report( ) -> str: head = require_sha(report["head_sha"]) lines = [MARKER, "", "## Castiron custom code", ""] + lines.extend([f"Evaluated main: `{require_sha(report['target_base_sha'])}`.", ""]) if report.get("status") != "ok": reason = html.escape(str(report.get("error", "Report could not be computed"))) lines.extend([f"⚠️ Report unavailable for `{head[:12]}`.", "", reason]) @@ -767,82 +768,23 @@ def api(method: str, path: str, payload: dict[str, Any] | None = None) -> Any: return json.loads(result.stdout) -def api_list(path: str) -> list[dict[str, Any]]: - """Read every GitHub REST list page or fail closed on malformed pagination.""" - results: list[dict[str, Any]] = [] +def associated_pulls(repository: str, run: dict[str, Any]) -> list[dict[str, Any]]: + root = f"repos/{repository}" + associated = run["pull_requests"] or api( + "GET", f"{root}/commits/{require_sha(run['head_sha'])}/pulls?per_page=100" + ) + if associated: + return cast(list[dict[str, Any]], associated) + # Fork runs can be absent from both association endpoints. Discover candidates + # by branch; callers still verify the current PR head and target through GitHub. + head = f"{run['head_repository']['owner']['login']}:{run['head_branch']}" for page in range(1, 101): - suffix = "" if page == 1 else f"&page={page}" - entries = api("GET", path + suffix) - if not isinstance(entries, list): - raise ReportError("GitHub list response is invalid") - results.extend(entries) - if len(entries) < 100: - return results - raise ReportError("GitHub list pagination did not terminate") - - -def current_pull_request( - root: str, - repository: str, - run: dict[str, Any], - branch: str, - main: str, -) -> dict[str, Any] | None: - """Resolve one live PR from trusted run metadata, including fork runs.""" - head = require_sha(run["head_sha"]) - head_repository = run.get("head_repository") - if not isinstance(head_repository, dict): - raise ReportError("source run has no head repository") - head_repository_name = head_repository.get("full_name") - head_repository_id = head_repository.get("id") - owner = head_repository.get("owner") - owner_login = owner.get("login") if isinstance(owner, dict) else None - head_branch = run.get("head_branch") - if ( - not isinstance(head_repository_name, str) - or not REPOSITORY.fullmatch(head_repository_name) - or type(head_repository_id) is not int - or head_repository_id <= 0 - or not isinstance(owner_login, str) - or not owner_login - or not isinstance(head_branch, str) - or not head_branch - ): - raise ReportError("invalid source run head repository or branch") - associated = run["pull_requests"] - if not associated: - query = urllib.parse.urlencode( - { - "state": "open", - "head": f"{owner_login}:{head_branch}", - "base": branch, - "per_page": 100, - } - ) - associated = api_list(f"{root}/pulls?{query}") - numbers = sorted({int(pull["number"]) for pull in associated}) - current: list[dict[str, Any]] = [] - for number in numbers: - if number <= 0: - raise ReportError("invalid associated pull request") - pull = api("GET", f"{root}/pulls/{number}") - pull_head_repository = pull["head"]["repo"] - if ( - pull["state"] == "open" - and pull["head"]["sha"] == head - and pull["head"]["ref"] == head_branch - and pull_head_repository["id"] == head_repository_id - and pull_head_repository["full_name"] == head_repository_name - and pull["base"]["repo"]["full_name"] == repository - and pull["base"]["ref"] == branch - and pull["base"]["sha"] == main - ): - current.append(pull) - if not current: - return None - if len(current) != 1: - raise ReportError("workflow run has multiple current pull requests") - return current[0] + query = urlencode({"state": "open", "head": head, "per_page": 100, "page": page}) + pulls = api("GET", f"{root}/pulls?{query}") + associated.extend(pulls) + if len(pulls) < 100: + return cast(list[dict[str, Any]], associated) + raise ReportError("too many pull requests for source branch") def publish_comment( @@ -858,22 +800,24 @@ def publish_comment( if not REPOSITORY.fullmatch(repository) or min(number, run_id, run_attempt) <= 0: raise ReportError("invalid GitHub publication target") root = f"repos/{repository}" + pull = api("GET", f"{root}/pulls/{number}") + if ( + pull["state"] != "open" + or pull["head"]["sha"] != report["head_sha"] + or pull["base"]["ref"] != "main" + or pull["base"]["repo"]["full_name"] != repository + ): + return "Skipped stale report" run = api("GET", f"{root}/actions/runs/{run_id}") if ( run["event"] != "pull_request" or run.get("path", "").split("@", 1)[0] != ".github/workflows/castiron-custom-code.yml" or run["head_sha"] != report["head_sha"] - or run["repository"]["full_name"] != repository ): raise ReportError("workflow run does not match report PR/head") - metadata = api("GET", root) - branch = metadata["default_branch"] - main = require_sha(api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"]) - pull = current_pull_request(root, repository, run, branch, main) - if pull is None or pull["number"] != number: + associated = associated_pulls(repository, run) + if not any(pr["number"] == number for pr in associated): raise ReportError("workflow run does not match report PR/head") - if pull["head"]["sha"] != report["head_sha"] or main != report["target_base_sha"]: - return "Skipped stale report" if run["run_attempt"] != run_attempt: return "Skipped stale report" artifact_run_id = artifact_run_id or run_id @@ -904,8 +848,13 @@ def publish_comment( if found["body"] == body: return str(found["html_url"]) # The workflow serializes publishers; recheck after pagination before writing. - pull = current_pull_request(root, repository, run, branch, main) - if pull is None or pull["number"] != number: + pull = api("GET", f"{root}/pulls/{number}") + if ( + pull["state"] != "open" + or pull["head"]["sha"] != report["head_sha"] + or pull["base"]["ref"] != "main" + or pull["base"]["repo"]["full_name"] != repository + ): return "Skipped stale report" if found is not None: result = api("PATCH", f"{root}/issues/comments/{found['id']}", {"body": body}) @@ -946,8 +895,13 @@ def write_report( return report -def trusted_report(repo: Path, repository: str, run_id: int, run_attempt: int, out: Path) -> None: +def trusted_report( + repo: Path, repository: str, run_id: int, run_attempt: int, out: Path, *, base: str +) -> None: """Recompute from GitHub-associated Git objects, never from PR-produced artifacts.""" + # The caller pins this to the trusted checkout selected from main. PR base + # metadata may lag behind main, and main may move again during computation. + base = require_sha(base) if not REPOSITORY.fullmatch(repository) or min(run_id, run_attempt) <= 0: raise ReportError("invalid GitHub report target") root = f"repos/{repository}" @@ -956,21 +910,30 @@ def trusted_report(repo: Path, repository: str, run_id: int, run_attempt: int, o run["event"] != "pull_request" or run.get("path", "").split("@", 1)[0] != ".github/workflows/castiron-custom-code.yml" or run["status"] != "completed" - or run["repository"]["full_name"] != repository ): raise ReportError("unexpected source workflow run") if run["run_attempt"] != run_attempt: return head = require_sha(run["head_sha"]) - metadata = api("GET", root) - branch = metadata["default_branch"] - main = require_sha(api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"]) - pull = current_pull_request(root, repository, run, branch, main) - if pull is None: + associated = associated_pulls(repository, run) + current: list[int] = [] + for number in sorted({int(pr["number"]) for pr in associated}): + if number <= 0: + raise ReportError("invalid associated pull request") + pull = api("GET", f"{root}/pulls/{number}") + if ( + pull["state"] == "open" + and pull["head"]["sha"] == head + and pull["base"]["repo"]["full_name"] == repository + and pull["base"]["ref"] == "main" + ): + current.append(number) + if not current: return - number = int(pull["number"]) - base = main - public = not metadata["private"] + if len(current) != 1: + raise ReportError("workflow run has multiple current pull requests") + number = current[0] + public = not api("GET", root)["private"] # This must be a new, bare repository: no PR worktree, hooks, configuration, # submodules, or Python imports can affect the trusted reporter. repo.mkdir() @@ -1004,6 +967,7 @@ def main() -> int: trusted.add_argument("--run-id", type=int, required=True) trusted.add_argument("--run-attempt", type=int, required=True) trusted.add_argument("--out", type=Path, required=True) + trusted.add_argument("--base", required=True, help="immutable main SHA of the trusted checkout") preparing = commands.add_parser("prepare-public") preparing.add_argument("--source-repo", type=Path, required=True) preparing.add_argument("--source-base", required=True) @@ -1038,7 +1002,9 @@ def main() -> int: + "\n" ) elif args.command == "trusted-report": - trusted_report(args.repo, args.repository, args.run_id, args.run_attempt, args.out) + trusted_report( + args.repo, args.repository, args.run_id, args.run_attempt, args.out, base=args.base + ) elif args.command == "comment": if args.report.stat().st_size > 5_000_000: raise ReportError("report artifact is too large") diff --git a/scripts/castiron/custom_code_test_support.py b/scripts/castiron/custom_code_test_support.py new file mode 100644 index 000000000..7c9404b02 --- /dev/null +++ b/scripts/castiron/custom_code_test_support.py @@ -0,0 +1,74 @@ +# File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. +"""Small Git/checkpoint fixture shared by the offline Castiron tests.""" + +from __future__ import annotations + +import base64 +import json +import subprocess +import tempfile +import unittest +from pathlib import Path +from typing import Any + +import custom_code_report as report + +GENERATION = "550e8400-e29b-41d4-a716-446655440000" + + +class GitTestCase(unittest.TestCase): + def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] + self.temporary = tempfile.TemporaryDirectory() + self.addCleanup(self.temporary.cleanup) + self.repo = Path(self.temporary.name) + self.git("init", "-q", "-b", "main") + self.git("config", "user.name", "Castiron test") + self.git("config", "user.email", "castiron@example.test") + + def git(self, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(self.repo), *args], check=True, capture_output=True, text=True + ).stdout.strip() + + def write(self, path: str, body: str) -> None: + target = self.repo / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(body) + + def commit(self, message: str = "fixture") -> str: + self.git("add", "-A") + self.git("commit", "-q", "--allow-empty", "-m", message) + return self.git("rev-parse", "HEAD") + + def baseline(self) -> tuple[str, str]: + self.write("generated.py", "generated\n") + metadata = { + "generation_id": GENERATION, + "source_branch": "test", + "target": "openai-python", + "language": "python", + } + encoded = base64.b64encode(json.dumps(metadata).encode()).decode() + generated = self.commit(f"codegen\n\nGeneration metadata: {encoded}") + self.git("update-ref", "refs/remotes/origin/codegen/test", generated) + self.write( + ".castiron.stats.yml", + f"schema_version: 1\ngeneration_id: {GENERATION}\ncodegen_sha: {generated}\ncodegen_hash: {report.hash_codegen_commit(self.repo, generated)}\n", + ) + return generated, self.commit("integrated") + + +def source_run(head: str, event: str = "pull_request") -> dict[str, Any]: + return { + "id": 123, + "event": event, + "head_sha": head, + "head_branch": "gh-readonly-queue/main/pr-7-example" if event == "merge_group" else "sdk", + "repository": {"full_name": "openai/example"}, + "head_repository": {"owner": {"login": "contributor"}}, + "path": ".github/workflows/castiron-custom-code.yml", + "status": "completed", + "run_attempt": 1, + "pull_requests": [{"number": 3}] if event == "pull_request" else [], + "conclusion": "failure", # The candidate's result is deliberately ignored. + } diff --git a/scripts/castiron/fixtures/github_publisher.cjs b/scripts/castiron/fixtures/github_publisher.cjs new file mode 100644 index 000000000..569421ba1 --- /dev/null +++ b/scripts/castiron/fixtures/github_publisher.cjs @@ -0,0 +1,38 @@ +// File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. +// Execute the real workflow script with an offline GitHub API and capture writes. +const fs = require('node:fs'); +const data = JSON.parse(fs.readFileSync(0, 'utf8')); +const writes = []; +const github = { + rest: { + actions: { getWorkflowRun: async () => ({ data: data.run }) }, + pulls: { get: async () => ({ data: data.current }), list: 'pulls' }, + git: { getRef: async () => ({ data: { object: { sha: data.current.base.sha } } }) }, + repos: { + createCommitStatus: async (value) => writes.push(value), + listCommitStatusesForRef: 'statuses', + listPullRequestsAssociatedWithCommit: 'associations', + }, + issues: { + listComments: 'comments', + createComment: async (value) => writes.push({ operation: 'create', ...value }), + updateComment: async (value) => writes.push({ operation: 'update', ...value }), + }, + }, + paginate: async (method, params) => { + if (method === 'statuses') return data.previous_statuses || []; + if (method === 'comments') return data.comments || []; + if (method === 'associations') return []; + if (method === 'pulls' && params.head === 'contributor:sdk' && params.state === 'open') { + return data.fallback_pulls || []; + } + throw new Error(`Unexpected GitHub lookup: ${method}`); + }, +}; +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +new AsyncFunction('github', 'context', 'process', data.script)(github, data.context, { env: data.env || {} }) + .then(() => process.stdout.write(JSON.stringify(writes))) + .catch((error) => { + console.error(error); + process.exitCode = 1; + }); diff --git a/scripts/castiron/test_custom_code_budget.py b/scripts/castiron/test_custom_code_budget.py index 90370e0a5..3e6e2a0ba 100644 --- a/scripts/castiron/test_custom_code_budget.py +++ b/scripts/castiron/test_custom_code_budget.py @@ -1,53 +1,28 @@ +# File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. # Regression tests for the custom-code budget. from __future__ import annotations import json -import shutil import subprocess import sys -import tempfile import unittest from pathlib import Path -from typing import Any, cast +from typing import Any from unittest import mock import custom_code_budget as budget -import test_custom_code_report as fixtures +from custom_code_test_support import GitTestCase -def source_run(head: str, event: str = "pull_request") -> dict[str, Any]: - return { - "id": 123, - "event": event, - "head_sha": head, - "head_branch": "gh-readonly-queue/main/pr-7-example" if event == "merge_group" else "sdk", - "head_repository": { - "id": 7, - "full_name": "fork/example", - "owner": {"login": "fork"}, - }, - "repository": {"full_name": "openai/example"}, - "path": ".github/workflows/castiron-custom-code.yml", - "status": "completed", - "run_attempt": 1, - "pull_requests": [{"number": 3}] if event == "pull_request" else [], - "conclusion": "failure", # The candidate's result is deliberately ignored. - } - - -class BudgetTests(unittest.TestCase): +class BudgetTests(GitTestCase): def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] - # Reuse the reporter's real Git/checkpoint fixture without inheriting its tests. - self.fixture = fixtures.CustomCodeTests() - self.fixture.setUp() - self.addCleanup(self.fixture.doCleanups) - self.repo = self.fixture.repo - self.generated, _ = self.fixture.baseline() + super().setUp() + self.generated, _ = self.baseline() self.policy(10) - self.base = self.fixture.commit("human-owned budget") + self.base = self.commit("human-owned budget") def policy(self, limit: int) -> None: - self.fixture.write( + self.write( budget.POLICY, json.dumps({"schema_version": 1, "max_custom_patch_lines": limit}) + "\n" ) @@ -55,16 +30,16 @@ def evaluate(self, head: str, base: str | None = None, **kwargs: Any) -> dict[st return budget.evaluate(self.repo, base or self.base, head, public=False, **kwargs)[0] def test_additions_and_deletions_do_not_cancel(self) -> None: - self.fixture.write("generated.py", "replacement\n") - result = self.evaluate(self.fixture.commit()) + self.write("generated.py", "replacement\n") + result = self.evaluate(self.commit()) self.assertEqual((result["additions"], result["deletions"], result["total"]), (1, 1, 2)) self.assertEqual(result["checks"]["budget"]["state"], "success") def test_below_equal_and_above_limit(self) -> None: for additions in (9, 10, 11): with self.subTest(additions=additions): - self.fixture.write("generated.py", "generated\n" + "custom\n" * additions) - result = self.evaluate(self.fixture.commit()) + self.write("generated.py", "generated\n" + "custom\n" * additions) + result = self.evaluate(self.commit()) self.assertEqual(result["total"], additions) self.assertEqual( result["checks"]["budget"]["state"], "failure" if additions > 10 else "success" @@ -72,71 +47,71 @@ def test_below_equal_and_above_limit(self) -> None: def test_whole_generated_file_deletion_is_counted(self) -> None: (self.repo / "generated.py").unlink() - result = self.evaluate(self.fixture.commit()) + result = self.evaluate(self.commit()) self.assertEqual((result["additions"], result["deletions"]), (0, 1)) self.assertEqual(result["mixed_files"], 1) def test_restoring_generated_content_removes_customization(self) -> None: - self.fixture.write("generated.py", "generated\ncustom\n") - customized = self.fixture.commit() - self.fixture.write("generated.py", "generated\n") - result = self.evaluate(self.fixture.commit(), base=customized) + self.write("generated.py", "generated\ncustom\n") + customized = self.commit() + self.write("generated.py", "generated\n") + result = self.evaluate(self.commit(), base=customized) self.assertEqual(result["total"], 0) self.assertEqual(result["mixed_files"], 0) def test_handwritten_only_files_keep_existing_report_scope(self) -> None: - self.fixture.write("handwritten.py", "custom\n" * 100) - self.assertEqual(self.evaluate(self.fixture.commit())["total"], 0) + self.write("handwritten.py", "custom\n" * 100) + self.assertEqual(self.evaluate(self.commit())["total"], 0) def test_increase_is_isolated_but_does_not_apply_to_itself(self) -> None: self.policy(20) - result = self.evaluate(self.fixture.commit()) + result = self.evaluate(self.commit()) self.assertEqual(result["checks"]["isolation"]["state"], "success") self.assertEqual(result["limit"], 10) self.assertEqual(result["checked_limit"], 10) def test_entire_pr_must_be_budget_only_not_just_latest_commit(self) -> None: - self.fixture.write("generated.py", "generated\n" + "custom\n" * 11) - self.fixture.commit("SDK change first") + self.write("generated.py", "generated\n" + "custom\n" * 11) + self.commit("SDK change first") self.policy(100) - result = self.evaluate(self.fixture.commit("budget-only last commit")) + result = self.evaluate(self.commit("budget-only last commit")) self.assertEqual(result["checks"]["isolation"]["state"], "failure") self.assertIn("separate, budget-only PR", result["checks"]["isolation"]["description"]) self.assertEqual(result["checks"]["budget"]["state"], "failure") self.assertEqual(result["limit"], 10) def test_new_base_budget_is_used_for_stale_pr_branch(self) -> None: - self.fixture.git("checkout", "-q", "-b", "sdk", self.base) - self.fixture.write("generated.py", "generated\n" + "custom\n" * 11) - head = self.fixture.commit() - self.fixture.git("checkout", "-q", "main") + self.git("checkout", "-q", "-b", "sdk", self.base) + self.write("generated.py", "generated\n" + "custom\n" * 11) + head = self.commit() + self.git("checkout", "-q", "main") self.policy(20) - new_base = self.fixture.commit("separate approved increase") + new_base = self.commit("separate approved increase") result = self.evaluate(head, base=new_base) self.assertEqual(result["limit"], 20) self.assertEqual(result["checks"]["isolation"]["state"], "success") self.assertEqual(result["checks"]["budget"]["state"], "success") def test_decrease_must_fit_current_usage(self) -> None: - self.fixture.write("generated.py", "generated\ncustom\ncustom\n") - base = self.fixture.commit() + self.write("generated.py", "generated\ncustom\ncustom\n") + base = self.commit() self.policy(1) - result = self.evaluate(self.fixture.commit(), base=base) + result = self.evaluate(self.commit(), base=base) self.assertEqual(result["checks"]["isolation"]["state"], "success") self.assertEqual(result["checks"]["budget"]["state"], "failure") self.assertEqual(result["checked_limit"], 1) def test_missing_base_policy_fails_closed(self) -> None: (self.repo / budget.POLICY).unlink() - base = self.fixture.commit() + base = self.commit() self.policy(100) - result = self.evaluate(self.fixture.commit(), base=base) + result = self.evaluate(self.commit(), base=base) self.assertTrue(all(c["state"] == "failure" for c in result["checks"].values())) def test_policy_deletion_rename_symlink_and_mode_change_fail(self) -> None: for change in ("delete", "rename", "symlink", "executable"): with self.subTest(change=change): - self.fixture.git("checkout", "--detach", "-q", self.base) + self.git("checkout", "--detach", "-q", self.base) path = self.repo / budget.POLICY if change == "delete": path.unlink() @@ -147,7 +122,7 @@ def test_policy_deletion_rename_symlink_and_mode_change_fail(self) -> None: path.symlink_to("generated.py") else: path.chmod(0o755) - result = self.evaluate(self.fixture.commit()) + result = self.evaluate(self.commit()) self.assertEqual(result["checks"]["isolation"]["state"], "failure") def test_invalid_policy_values_fail(self) -> None: @@ -167,22 +142,22 @@ def test_invalid_policy_values_fail(self) -> None: ] for contents in invalid: with self.subTest(contents=contents[:100]): - self.fixture.write(budget.POLICY, contents) - result = self.evaluate(self.fixture.commit()) + self.write(budget.POLICY, contents) + result = self.evaluate(self.commit()) self.assertEqual(result["checks"]["isolation"]["state"], "failure") def test_bad_snapshot_and_binary_change_fail_budget(self) -> None: - self.fixture.write("generated.py", "binary\0content") - result = self.evaluate(self.fixture.commit()) + self.write("generated.py", "binary\0content") + result = self.evaluate(self.commit()) self.assertIn("non-text", result["checks"]["budget"]["description"]) - self.fixture.write("generated.py", "generated\n") + self.write("generated.py", "generated\n") path = self.repo / ".castiron.stats.yml" path.write_text( path.read_text().replace( budget.report.hash_codegen_commit(self.repo, self.generated), "f" * 64 ) ) - result = self.evaluate(self.fixture.commit()) + result = self.evaluate(self.commit()) self.assertIn("codegen_hash mismatch", result["checks"]["budget"]["description"]) def test_budget_uses_existing_reporter_with_strict_verification(self) -> None: @@ -195,18 +170,16 @@ def test_budget_uses_existing_reporter_with_strict_verification(self) -> None: ) def test_queue_isolates_prs_but_uses_main_budget_for_combined_tree(self) -> None: - self.fixture.git("checkout", "-q", "-b", "policy", self.base) + self.git("checkout", "-q", "-b", "policy", self.base) self.policy(100) - policy_head = self.fixture.commit() - self.fixture.git("checkout", "-q", "-b", "sdk", self.base) - self.fixture.write("generated.py", "generated\n" + "custom\n" * 11) - sdk_head = self.fixture.commit() - self.fixture.git("checkout", "-q", "-b", "queue", self.base) - self.fixture.git("merge", "--no-ff", "-m", "queue policy", policy_head) - self.fixture.git("merge", "--no-ff", "-m", "queue SDK", sdk_head) - result = self.evaluate( - self.fixture.git("rev-parse", "HEAD"), pull_heads=[policy_head, sdk_head] - ) + policy_head = self.commit() + self.git("checkout", "-q", "-b", "sdk", self.base) + self.write("generated.py", "generated\n" + "custom\n" * 11) + sdk_head = self.commit() + self.git("checkout", "-q", "-b", "queue", self.base) + self.git("merge", "--no-ff", "-m", "queue policy", policy_head) + self.git("merge", "--no-ff", "-m", "queue SDK", sdk_head) + result = self.evaluate(self.git("rev-parse", "HEAD"), pull_heads=[policy_head, sdk_head]) self.assertEqual(result["checks"]["isolation"]["state"], "success") self.assertEqual(result["checks"]["budget"]["state"], "failure") self.assertEqual(result["limit"], 10) @@ -216,63 +189,12 @@ def test_queue_without_verified_members_fails(self) -> None: self.evaluate(self.base, pull_heads=[])["checks"]["isolation"]["state"], "failure" ) - def test_queue_membership_uses_synthetic_commit_not_original_pr_ancestry(self) -> None: - self.fixture.git("checkout", "-q", "-b", "sdk", self.base) - self.fixture.write("generated.py", "generated\ncustom\n") - pr_head = self.fixture.commit() - self.fixture.git("checkout", "-q", "-b", "queue", self.base) - self.fixture.git("merge", "--squash", "sdk") - queue_head = self.fixture.commit("synthetic queue commit") - self.assertNotEqual(self.fixture.git("merge-base", pr_head, queue_head), pr_head) - event = { - "repository": {"full_name": "openai/example"}, - "workflow_run": source_run(queue_head, "merge_group"), - } - original_git = budget.report.git - - def local_fetch(repo: Path, *args: str) -> bytes: - if args[0] == "fetch": - args = tuple(str(self.repo) if arg == "origin" else arg for arg in args) - return cast(bytes, original_git(repo, *args)) - - with ( - tempfile.TemporaryDirectory() as temp, - mock.patch.object( - budget.report, - "api", - side_effect=[ - {"default_branch": "main", "private": False}, - {"object": {"sha": self.base}}, - source_run(queue_head, "merge_group"), - [{"type": "merge_queue"}], - ], - ), - mock.patch.object(budget, "queued_entries", return_value=[(pr_head, queue_head)]), - mock.patch.object( - budget.report, - "git", - side_effect=local_fetch, - ), - mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, - ): - repo = Path(temp) / "queue.git" - budget.github_evaluate(repo, "openai/example", event, self.base) - evaluate.assert_called_once_with( - repo, - self.base, - queue_head, - public=True, - fetch=True, - pull_heads=[pr_head], - measurement=None, - ) - def test_cli_executes_trusted_reporter_not_inspected_repo(self) -> None: - self.fixture.write( + self.write( "scripts/castiron/custom_code_report.py", 'raise RuntimeError("PR CODE EXECUTED")\n' ) - self.fixture.write("sitecustomize.py", 'raise RuntimeError("PR IMPORTED")\n') - head = self.fixture.commit() + self.write("sitecustomize.py", 'raise RuntimeError("PR IMPORTED")\n') + head = self.commit() out = self.repo / "result" command = [ sys.executable, @@ -293,8 +215,8 @@ def test_cli_executes_trusted_reporter_not_inspected_repo(self) -> None: self.assertEqual(json.loads((out / "budget.json").read_text())["total"], 0) def test_summary_has_counts_revisions_and_no_longer_generated(self) -> None: - self.fixture.write("generated.py", "replacement\n") - result = self.evaluate(self.fixture.commit()) + self.write("generated.py", "replacement\n") + result = self.evaluate(self.commit()) out = self.repo / "report-output" budget.write_result(out, result) summary = (out / "summary.md").read_text() @@ -304,359 +226,5 @@ def test_summary_has_counts_revisions_and_no_longer_generated(self) -> None: self.assertIn("human approving review", summary) -@unittest.skipUnless(shutil.which("node"), "Node is needed to execute the status-publisher fixture") -class StatusPublisherTests(unittest.TestCase): - def publish( - self, - *, - event_name: str = "pull_request", - head_changed: bool = False, - base_changed: bool = False, - no_result: bool = False, - failed_budget: bool = False, - run_overrides: dict[str, Any] | None = None, - ) -> list[dict[str, Any]]: - path = ( - Path(__file__).resolve().parents[2] - / ".github/workflows/castiron-custom-code-comment.yml" - ) - section = path.read_text().split("\n budget-status:\n", 1)[1].split("\n comment:\n", 1)[0] - publisher = section.split(" script: |\n", 1)[1] - script = "\n".join(line[12:] for line in publisher.splitlines()) - base, head = "a" * 40, "b" * 40 - payload = { - "script": script, - "context": { - "eventName": "workflow_run", - "repo": {"owner": "openai", "repo": "example"}, - "serverUrl": "https://github.com", - "runId": 123, - "payload": { - "workflow_run": source_run(head, event_name), - }, - }, - "run": {**source_run(head, event_name), **(run_overrides or {})}, - "candidates": [{"number": 3}], - "current": { - "state": "open", - "head": { - "sha": "c" * 40 if head_changed else head, - "ref": "sdk", - "repo": {"id": 7, "full_name": "fork/example"}, - }, - "base": { - "sha": "c" * 40 if base_changed else base, - "ref": "main", - "repo": {"full_name": "openai/example"}, - }, - }, - "env": { - "BASE_SHA": "" if no_result else base, - "HEAD_SHA": "" if no_result else head, - "ISOLATION_RESULT": "success", - "BUDGET_RESULT": "failure" if failed_budget else "success", - }, - } - harness = """ - const fs = require('node:fs'); - const data = JSON.parse(fs.readFileSync(0, 'utf8')); - const published = []; - const github = { - paginate: async (_method, options) => { - if (options.head !== 'fork:sdk' || options.base !== 'main' || - options.state !== 'open') throw new Error('unsafe fork lookup'); - return data.candidates; - }, - rest: { - pulls: {list() {}, get: async () => ({data: data.current})}, - actions: {getWorkflowRun: async () => ({data: data.run})}, - git: {getRef: async () => ({data: {object: {sha: data.current.base.sha}}})}, - repos: { - get: async () => ({data: {default_branch: 'main'}}), - createCommitStatus: async value => published.push(value), - }, - }}; - const AsyncFunction = Object.getPrototypeOf(async function(){}).constructor; - new AsyncFunction('github','context','process', data.script)(github, data.context, {env:data.env}) - .then(() => process.stdout.write(JSON.stringify(published))) - .catch(error => { console.error(error); process.exitCode = 1; }); - """ - output = subprocess.run( - ["node", "-e", harness], - input=json.dumps(payload), - text=True, - capture_output=True, - check=True, - ) - return cast(list[dict[str, Any]], json.loads(output.stdout)) - - def test_statuses_attach_to_candidate_not_main(self) -> None: - for event in ("pull_request", "merge_group"): - with self.subTest(event=event): - results = self.publish(event_name=event) - self.assertEqual(len(results), 2) - self.assertTrue( - all(r["sha"] == "b" * 40 and r["state"] == "success" for r in results) - ) - - def test_stale_pr_head_is_not_published(self) -> None: - self.assertEqual(self.publish(head_changed=True), []) - - def test_fork_run_without_association_uses_scoped_open_pr_lookup(self) -> None: - results = self.publish(run_overrides={"pull_requests": []}) - self.assertEqual(len(results), 2) - - def test_stale_base_and_missing_evaluation_cannot_publish_success(self) -> None: - for event in ("pull_request", "merge_group"): - for base_changed, no_result in ((True, False), (False, True)): - results = self.publish( - event_name=event, base_changed=base_changed, no_result=no_result - ) - self.assertEqual(len(results), 2) - self.assertTrue(all(r["state"] == "failure" for r in results)) - - def test_superseded_or_wrong_source_run_cannot_publish(self) -> None: - for overrides in ( - {"run_attempt": 2}, - {"head_sha": "c" * 40}, - {"event": "push"}, - {"path": "other.yml"}, - ): - with self.subTest(overrides=overrides): - self.assertEqual(self.publish(run_overrides=overrides), []) - - def test_independent_check_failures_are_preserved(self) -> None: - results = self.publish(failed_budget=True) - self.assertEqual([r["state"] for r in results], ["success", "failure"]) - - -class GitHubBudgetTests(unittest.TestCase): - def test_merge_queue_rule_is_required_before_passing(self) -> None: - base, head = "a" * 40, "b" * 40 - event = { - "repository": {"full_name": "openai/example"}, - "workflow_run": source_run(head, "merge_group"), - } - with ( - tempfile.TemporaryDirectory() as temp, - mock.patch.object( - budget.report, - "api", - side_effect=[ - {"default_branch": "main"}, - {"object": {"sha": base}}, - source_run(head, "merge_group"), - [{"type": "required_status_checks"}], - ], - ), - ): - repo = Path(temp) / "objects.git" - with self.assertRaisesRegex(ValueError, "must require a merge queue"): - budget.github_evaluate(repo, "openai/example", event, base) - self.assertFalse(repo.exists()) - - def test_wrong_or_superseded_source_runs_fail_before_fetching(self) -> None: - base, head = "a" * 40, "b" * 40 - event = {"repository": {"full_name": "openai/example"}, "workflow_run": source_run(head)} - for overrides in ( - {"run_attempt": 2}, - {"status": "in_progress"}, - {"path": "other.yml"}, - {"head_sha": "c" * 40}, - {"repository": {"full_name": "wrong/repo"}}, - ): - with ( - self.subTest(overrides=overrides), - tempfile.TemporaryDirectory() as temp, - mock.patch.object( - budget.report, - "api", - side_effect=[ - {"default_branch": "main"}, - {"object": {"sha": base}}, - {**source_run(head), **overrides}, - ], - ), - ): - repo = Path(temp) / "objects.git" - with self.assertRaisesRegex(ValueError, "source workflow run"): - budget.github_evaluate(repo, "openai/example", event, base) - self.assertFalse(repo.exists()) - - def test_reuses_only_matching_main_job_report(self) -> None: - base, head = "a" * 40, "b" * 40 - event = {"repository": {"full_name": "openai/example"}, "workflow_run": source_run(head)} - pull = { - "state": "open", - "head": {"sha": head, "ref": "sdk", "repo": {"id": 7, "full_name": "fork/example"}}, - "base": { - "sha": base, - "ref": "main", - "repo": {"full_name": "openai/example"}, - }, - } - for stale in (False, True): - with self.subTest(stale=stale), tempfile.TemporaryDirectory() as temp: - trusted = Path(temp) - measured = {"target_base_sha": "c" * 40 if stale else base, "head_sha": head} - (trusted / "report.json").write_text(json.dumps(measured)) - (trusted / "custom-code.patch").write_bytes(b"verified patch") - repo = trusted / "objects.git" - repo.mkdir() - with ( - mock.patch.object( - budget.report, - "api", - side_effect=[ - {"default_branch": "main", "private": False}, - {"object": {"sha": base}}, - source_run(head), - pull, - [{"type": "merge_queue"}], - ], - ), - mock.patch.object(budget.report, "git", return_value=b"true\n") as git, - mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, - ): - if stale: - with self.assertRaisesRegex(ValueError, "trusted report is stale"): - budget.github_evaluate(repo, "openai/example", event, base, trusted) - evaluate.assert_not_called() - else: - budget.github_evaluate(repo, "openai/example", event, base, trusted) - evaluate.assert_called_once_with( - repo, - base, - head, - public=True, - fetch=True, - pull_heads=None, - measurement=(measured, b"verified patch"), - ) - git.assert_called_once_with(repo, "rev-parse", "--is-bare-repository") - - def test_queue_pagination(self) -> None: - pages = [ - { - "nodes": [ - {"pullRequest": {"headRefOid": "a" * 40}, "headCommit": {"oid": "c" * 40}} - ], - "pageInfo": {"hasNextPage": True, "endCursor": "cursor"}, - }, - { - "nodes": [ - {"pullRequest": {"headRefOid": "b" * 40}, "headCommit": {"oid": "d" * 40}} - ], - "pageInfo": {"hasNextPage": False, "endCursor": None}, - }, - ] - with mock.patch.object( - budget.report, - "api", - side_effect=[{"data": {"repository": {"mergeQueue": {"entries": p}}}} for p in pages], - ) as api: - self.assertEqual( - budget.queued_entries("openai/example", "main"), - [("a" * 40, "c" * 40), ("b" * 40, "d" * 40)], - ) - self.assertEqual(api.call_args_list[1].args[2]["variables"]["cursor"], "cursor") - - def test_pull_context_refuses_stale_checkout_before_fetching(self) -> None: - with ( - tempfile.TemporaryDirectory() as temp, - mock.patch.object( - budget.report, - "api", - side_effect=[{"default_branch": "main"}, {"object": {"sha": "a" * 40}}], - ), - ): - repo = Path(temp) / "objects.git" - with self.assertRaisesRegex(ValueError, "trusted checkout is stale"): - budget.github_evaluate( - repo, - "openai/example", - {"repository": {"full_name": "openai/example"}}, - "b" * 40, - ) - self.assertFalse(repo.exists()) - - def test_github_uses_fresh_bare_data_repo_and_checks_current_head(self) -> None: - base, head = "a" * 40, "b" * 40 - event = { - "repository": {"full_name": "openai/example"}, - "workflow_run": source_run(head), - } - pull = { - "state": "open", - "head": {"sha": head, "ref": "sdk", "repo": {"id": 7, "full_name": "fork/example"}}, - "base": {"sha": base, "ref": "main", "repo": {"full_name": "openai/example"}}, - } - responses = [ - {"default_branch": "main", "private": False}, - {"object": {"sha": base}}, - source_run(head), - pull, - [{"type": "merge_queue"}], - ] - with ( - tempfile.TemporaryDirectory() as temp, - mock.patch.object(budget.report, "api", side_effect=responses), - mock.patch.object(budget.report, "git") as git, - mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, - ): - repo = Path(temp) / "objects.git" - budget.github_evaluate(repo, "openai/example", event, base) - self.assertTrue((repo / "HEAD").exists()) - self.assertFalse((repo / "src").exists()) - self.assertIn( - mock.call(repo, "fetch", "--quiet", "--no-tags", "origin", base, head), - git.call_args_list, - ) - evaluate.assert_called_once_with( - repo, base, head, public=True, fetch=True, pull_heads=None, measurement=None - ) - - def test_stale_pull_and_wrong_target_fail_before_objects_created(self) -> None: - for kind in ("head", "base", "repository", "branch", "closed"): - with self.subTest(kind=kind), tempfile.TemporaryDirectory() as temp: - base, head = "a" * 40, "b" * 40 - event = { - "repository": {"full_name": "openai/example"}, - "workflow_run": source_run(head), - } - pull: dict[str, Any] = { - "state": "open", - "head": {"sha": head, "ref": "sdk", "repo": {"id": 7, "full_name": "fork/example"}}, - "base": {"sha": base, "ref": "main", "repo": {"full_name": "openai/example"}}, - } - if kind == "head": - pull["head"]["sha"] = "c" * 40 - elif kind == "base": - pull["base"]["sha"] = "c" * 40 - elif kind == "repository": - pull["base"]["repo"]["full_name"] = "wrong/repo" - elif kind == "branch": - pull["base"]["ref"] = "other" - else: - pull["state"] = "closed" - with mock.patch.object( - budget.report, - "api", - side_effect=[ - {"default_branch": "main"}, - {"object": {"sha": base}}, - source_run(head), - pull, - ], - ): - with self.assertRaisesRegex(ValueError, "exactly one current PR"): - budget.github_evaluate( - Path(temp) / "objects.git", - "openai/example", - event, - base, - ) - - if __name__ == "__main__": unittest.main() diff --git a/scripts/castiron/test_custom_code_github.py b/scripts/castiron/test_custom_code_github.py new file mode 100644 index 000000000..8dece1210 --- /dev/null +++ b/scripts/castiron/test_custom_code_github.py @@ -0,0 +1,475 @@ +# File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. +"""Trusted GitHub evaluation with real local Git objects and an offline API.""" + +from __future__ import annotations + +import json +import os +import tempfile +import sys +import unittest +from pathlib import Path +from typing import Any +from unittest import mock + +import custom_code_report as report +import custom_code_budget as budget +from custom_code_test_support import GENERATION, GitTestCase, source_run + +REPOSITORY = "openai/example" + + +class GitHubAPI: + """Mutable remote state; unexpected requests fail instead of reaching GitHub.""" + + def __init__(self, base: str, head: str, event: str = "pull_request") -> None: + self.main = base + self.run = source_run(head, event) + self.pull: dict[str, Any] = { + "state": "open", + "head": {"sha": head}, + "base": {"sha": base, "ref": "main", "repo": {"full_name": REPOSITORY}}, + } + self.associations = [{"number": 3}] + self.branches = [{"number": 3}] + self.rules = [{"type": "merge_queue"}] + self.bodies: list[str] = [] + + def __call__(self, method: str, path: str, payload: dict[str, Any] | None = None) -> Any: + root = f"repos/{REPOSITORY}" + if method == "POST" and path == f"{root}/issues/3/comments" and payload: + self.bodies.append(payload["body"]) + return {"html_url": "published"} + responses = { + root: {"default_branch": "main", "private": False}, + f"{root}/actions/runs/123": self.run, + f"{root}/pulls/3": self.pull, + f"{root}/pulls/4": self.pull, + f"{root}/git/ref/heads/main": {"object": {"sha": self.main}}, + f"{root}/rules/branches/main": self.rules, + f"{root}/commits/{self.run['head_sha']}/pulls?per_page=100": self.associations, + f"{root}/pulls?state=open&head=contributor%3Asdk&per_page=100&page=1": self.branches, + f"{root}/issues/3/comments?per_page=100&page=1": [], + } + if method != "GET" or path not in responses: + raise AssertionError(f"Unexpected GitHub request: {method} {path}") + return responses[path] + + +class GitHubReportTests(GitTestCase): + def test_trusted_report_recomputes_pr_output_in_a_bare_repository(self) -> None: + generated, _ = self.baseline() + content_hash = report.hash_codegen_commit(self.repo, generated) + snapshot = report.create_public_snapshot( + self.repo, + self.git("rev-parse", f"{generated}^{{tree}}"), + GENERATION, + content_hash, + "codegen/public-test", + None, + ) + self.git("branch", "codegen/public-test", snapshot) + stats = (self.repo / ".castiron.stats.yml").read_text() + self.write(".castiron.stats.yml", stats + f"public_codegen_sha: {snapshot}\n") + self.write(budget.POLICY, json.dumps({"schema_version": 1, "max_custom_patch_lines": 10})) + base = self.commit() + legitimate, _ = report.build_report(self.repo, base, base, require_head_hash=True) + self.write("generated.py", "generated\n# custom\n") + self.write("scripts/castiron/custom_code_report.py", "raise RuntimeError('PR code ran')\n") + self.write("report.json", json.dumps(legitimate)) + head = self.commit() + self.write( + ".castiron.stats.yml", + (self.repo / ".castiron.stats.yml").read_text().replace(content_hash, "0" * 64), + ) + broken = self.commit() + remote = self.repo / "public.git" + self.git("clone", "--bare", str(self.repo), str(remote)) + real_git = report.git + + def local_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[:3] == ("remote", "add", "origin"): + self.assertEqual(args[3], f"https://github.com/{REPOSITORY}.git") + args = (*args[:3], str(remote)) + self.assertNotIn("checkout", args) + return real_git(repo, *args, input_bytes=input_bytes) + + for label, revision, total in ( + ("baseline", base, 0), + ("custom", head, 1), + ("fork", head, 1), + ("bad-hash", broken, None), + ): + with self.subTest(label=label): + api = GitHubAPI( + "d" * 40, revision + ) # PR metadata already differs from captured main. + if label == "fork": + api.run["pull_requests"] = [] + api.associations = [] + objects, out = self.repo / f"{label}.git", self.repo / f"{label}-report" + with ( + mock.patch.object(report, "api", side_effect=api), + mock.patch.object(budget.report, "api", side_effect=api), + mock.patch.object(report, "git", side_effect=local_git), + ): + report.trusted_report(objects, REPOSITORY, 123, 1, out, base=base) + self.assertEqual( + real_git(objects, "rev-parse", "--is-bare-repository"), b"true\n" + ) + self.assertFalse((objects / "scripts").exists()) + self.assertEqual(api.bodies, []) + measured = json.loads((out / "report.json").read_text()) + self.assertEqual( + (measured["target_base_sha"], measured["head_sha"]), (base, revision) + ) + # Advance main before both budget evaluation and publication. + api.main = api.pull["base"]["sha"] = "e" * 40 + result, _ = budget.github_evaluate( + objects, + REPOSITORY, + {"repository": {"full_name": REPOSITORY}, "workflow_run": api.run}, + base, + out, + ) + self.assertEqual((result["base_sha"], result["head_sha"]), (base, revision)) + report.publish_comment( + measured, REPOSITORY, 3, 123, 1, artifact_run_id=9, artifact_run_attempt=3 + ) + self.assertEqual(len(api.bodies), 1) + body = api.bodies[0] + self.assertIn("castiron:run:v1:123:1", body) + self.assertIn(base, body) + self.assertIn("/actions/runs/9", body) + if total is None: + self.assertEqual(result["checks"]["budget"]["state"], "failure") + self.assertIn("could not verify", result["checks"]["budget"]["description"]) + self.assertIn("codegen_hash mismatch", measured["error"]) + self.assertIn("Report unavailable", body) + self.assertNotIn("Generated baselines verified", body) + else: + self.assertEqual(result["total"], total) + self.assertEqual(result["checks"]["budget"]["state"], "success") + self.assertIn("Generated baselines verified", body) + self.assertIn("--name castiron-custom-code-9-3", body) + if total: + self.assertIn("1 newly customized", body) + self.assertIn("generated.py", body) + self.assertIn(b"+# custom", (out / "custom-code.patch").read_bytes()) + self.assertNotIn("No new custom-code files detected", body) + else: + self.assertIn("No new custom-code files detected", body) + + def test_fork_association_fallback_paginates_candidates(self) -> None: + run: dict[str, Any] = { + "head_sha": "a" * 40, + "pull_requests": [], + "head_repository": {"owner": {"login": "contributor"}}, + "head_branch": "fix/branch&other=value", + } + first = [{"number": number} for number in range(1, 101)] + with mock.patch.object(report, "api", side_effect=[[], first, [{"number": 101}]]) as api: + self.assertEqual(len(report.associated_pulls("openai/example", run)), 101) + self.assertEqual( + api.call_args.args, + ( + "GET", + "repos/openai/example/pulls?state=open&head=contributor%3Afix%2Fbranch%26other%3Dvalue&per_page=100&page=2", + ), + ) + + def test_trusted_report_rejects_invalid_or_stale_association_before_fetch(self) -> None: + cases: dict[str, tuple[dict[str, Any], dict[str, Any], bool]] = { + "wrong workflow": ({"path": "other.yml"}, {}, True), + "unfinished run": ({"status": "in_progress"}, {}, True), + "old attempt": ({"run_attempt": 2}, {}, False), + "closed PR": ({}, {"state": "closed"}, False), + "changed head": ({}, {"head": {"sha": "c" * 40}}, False), + "other branch": ( + {}, + {"base": {"ref": "other", "repo": {"full_name": REPOSITORY}}}, + False, + ), + "other repository": ( + {}, + {"base": {"ref": "main", "repo": {"full_name": "other/repo"}}}, + False, + ), + "multiple PRs": ({"pull_requests": [{"number": 3}, {"number": 4}]}, {}, True), + } + for fallback in (False, True): + for label, (run, pull, raises) in cases.items(): + with self.subTest(label=label, fallback=fallback): + api = GitHubAPI("b" * 40, "a" * 40) + api.run.update(run) + api.pull.update(pull) + if fallback: + api.branches = api.run["pull_requests"] + api.run["pull_requests"] = [] + api.associations = [] + with ( + mock.patch.object(report, "api", side_effect=api), + mock.patch.object(report, "git") as git, + ): + if raises: + with self.assertRaises(report.ReportError): + report.trusted_report( + self.repo / "objects", + REPOSITORY, + 123, + 1, + self.repo / "out", + base="b" * 40, + ) + else: + report.trusted_report( + self.repo / "objects", + REPOSITORY, + 123, + 1, + self.repo / "out", + base="b" * 40, + ) + git.assert_not_called() + self.assertFalse((self.repo / "out").exists()) + api = GitHubAPI("b" * 40, "a" * 40) + api.run["pull_requests"] = api.associations = api.branches = [] + with ( + mock.patch.object(report, "api", side_effect=api), + mock.patch.object(report, "git") as git, + ): + report.trusted_report( + self.repo / "objects", REPOSITORY, 123, 1, self.repo / "out", base="b" * 40 + ) + git.assert_not_called() + + def test_queue_membership_uses_synthetic_commit_not_original_pr_ancestry(self) -> None: + generated, _ = self.baseline() + self.git("branch", "codegen/test", generated) + self.write(budget.POLICY, json.dumps({"schema_version": 1, "max_custom_patch_lines": 10})) + base = self.commit("budget") + self.git("checkout", "-q", "-b", "sdk", base) + self.write("generated.py", "generated\ncustom\n") + pr_head = self.commit() + self.git("checkout", "-q", "-b", "queue", base) + self.git("merge", "--squash", "sdk") + queue_head = self.commit("synthetic queue commit") + self.assertNotEqual(self.git("merge-base", pr_head, queue_head), pr_head) + event = { + "repository": {"full_name": "openai/example"}, + "workflow_run": source_run(queue_head, "merge_group"), + } + original_git = report.git + + def local_fetch(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[0] == "fetch": + args = tuple(str(self.repo) if arg == "origin" else arg for arg in args) + return original_git(repo, *args, input_bytes=input_bytes) + + with ( + tempfile.TemporaryDirectory() as temp, + mock.patch.object( + budget.report, + "api", + side_effect=[ + {"default_branch": "main", "private": True}, + source_run(queue_head, "merge_group"), + {"object": {"sha": base}}, + [{"type": "merge_queue"}], + ], + ), + mock.patch.object(budget, "queued_entries", return_value=[(pr_head, queue_head)]), + mock.patch.object( + budget.report, + "git", + side_effect=local_fetch, + ), + ): + repo = Path(temp) / "queue.git" + result, _ = budget.github_evaluate(repo, "openai/example", event, base) + self.assertEqual(result["head_sha"], queue_head) + self.assertEqual(result["checks"]["budget"]["state"], "success", result["checks"]) + self.assertEqual(result["total"], 1) + self.assertEqual( + [c["state"] for c in result["checks"].values()], ["success", "success"] + ) + + +class GitHubBudgetTests(unittest.TestCase): + def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] + temporary = tempfile.TemporaryDirectory() + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.repo = self.root / "objects.git" + self.base, self.head = "a" * 40, "b" * 40 + self.api = GitHubAPI(self.base, self.head) + patch = mock.patch.object(budget.report, "api", side_effect=self.api) + patch.start() + self.addCleanup(patch.stop) + + def evaluate(self, *, trusted_report: Path | None = None) -> tuple[dict[str, Any], bytes]: + event = { + "repository": {"full_name": REPOSITORY}, + "workflow_run": source_run(self.head, self.api.run["event"]), + } + return budget.github_evaluate(self.repo, REPOSITORY, event, self.base, trusted_report) + + def test_merge_queue_rule_is_required_before_passing(self) -> None: + self.api.run = source_run(self.head, "merge_group") + self.api.rules = [{"type": "required_status_checks"}] + with self.assertRaisesRegex(ValueError, "must require a merge queue"): + self.evaluate() + self.assertFalse(self.repo.exists()) + + def test_wrong_or_superseded_source_runs_fail_before_fetching(self) -> None: + for changes in ( + {"run_attempt": 2}, + {"status": "in_progress"}, + {"path": "other.yml"}, + {"head_sha": "c" * 40}, + {"repository": {"full_name": "wrong/repo"}}, + ): + with self.subTest(changes=changes): + self.api.run = {**source_run(self.head), **changes} + with self.assertRaisesRegex(ValueError, "source workflow run"): + self.evaluate() + self.assertFalse(self.repo.exists()) + + def test_reuses_only_matching_main_job_report(self) -> None: + self.api.main = self.api.pull["base"]["sha"] = "d" * 40 + self.repo.mkdir() + (self.root / "custom-code.patch").write_bytes(b"verified patch") + for stale in (None, "base", "head"): + with self.subTest(stale=stale): + measured = { + "target_base_sha": "c" * 40 if stale == "base" else self.base, + "head_sha": "c" * 40 if stale == "head" else self.head, + } + (self.root / "report.json").write_text(json.dumps(measured)) + with ( + mock.patch.object(budget.report, "git", return_value=b"true\n"), + mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, + ): + if stale: + with self.assertRaisesRegex(ValueError, "trusted report does not match"): + self.evaluate(trusted_report=self.root) + evaluate.assert_not_called() + else: + self.evaluate(trusted_report=self.root) + self.assertEqual(evaluate.call_args.args, (self.repo, self.base, self.head)) + self.assertEqual( + evaluate.call_args.kwargs["measurement"], (measured, b"verified patch") + ) + + def test_queue_cannot_reuse_pr_measurement(self) -> None: + self.api.run = source_run(self.head, "merge_group") + with self.assertRaisesRegex(ValueError, "only PR runs can reuse"): + self.evaluate(trusted_report=self.root) + self.assertFalse(self.repo.exists()) + + def test_evaluation_error_emits_failure_outputs_and_diagnostic_summary(self) -> None: + event = self.root / "event.json" + event.write_text( + json.dumps({"repository": {"full_name": REPOSITORY}, "workflow_run": self.api.run}) + ) + output, result = self.root / "outputs", self.root / "result" + self.api.rules = [] # Real validation failure, before any Git objects are fetched. + args = [ + "budget", + "github", + "--repository", + REPOSITORY, + "--event-path", + str(event), + "--trusted-sha", + self.base, + "--repo", + str(self.repo), + "--out", + str(result), + ] + with ( + mock.patch.object(sys, "argv", args), + mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(output)}), + ): + self.assertEqual(budget.main(), 1) + self.assertIn("isolation=failure\nbudget=failure\n", output.read_text()) + self.assertIn("must require a merge queue", (result / "summary.md").read_text()) + + def test_queue_pagination(self) -> None: + pages = [ + { + "nodes": [ + {"pullRequest": {"headRefOid": "a" * 40}, "headCommit": {"oid": "c" * 40}} + ], + "pageInfo": {"hasNextPage": True, "endCursor": "cursor"}, + }, + { + "nodes": [ + {"pullRequest": {"headRefOid": "b" * 40}, "headCommit": {"oid": "d" * 40}} + ], + "pageInfo": {"hasNextPage": False, "endCursor": None}, + }, + ] + with mock.patch.object( + budget.report, + "api", + side_effect=[{"data": {"repository": {"mergeQueue": {"entries": p}}}} for p in pages], + ) as api: + self.assertEqual( + budget.queued_entries("openai/example", "main"), + [("a" * 40, "c" * 40), ("b" * 40, "d" * 40)], + ) + self.assertEqual(api.call_args_list[1].args[2]["variables"]["cursor"], "cursor") + + def test_queue_context_refuses_stale_checkout_before_fetching(self) -> None: + self.api.run = source_run(self.head, "merge_group") + self.api.main = "c" * 40 + with self.assertRaisesRegex(ValueError, "trusted checkout is stale"): + self.evaluate() + self.assertFalse(self.repo.exists()) + + def test_github_uses_fresh_bare_data_repo_and_checks_current_head(self) -> None: + self.api.run["pull_requests"] = self.api.associations = [] + self.api.pull["base"]["sha"] = "c" * 40 + with ( + mock.patch.object(budget.report, "git") as git, + mock.patch.object(budget, "evaluate", return_value=({}, b"")) as evaluate, + ): + self.evaluate() + self.assertTrue((self.repo / "HEAD").exists()) + self.assertFalse((self.repo / "src").exists()) + self.assertIn( + mock.call( + self.repo, "fetch", "--quiet", "--no-tags", "origin", self.base, self.head + ), + git.call_args_list, + ) + evaluate.assert_called_once_with( + self.repo, + self.base, + self.head, + public=True, + fetch=True, + pull_heads=None, + measurement=None, + ) + + def test_stale_pull_and_wrong_target_fail_before_objects_created(self) -> None: + original = self.api.pull + self.api.run["pull_requests"] = self.api.associations = [] + for changes in ( + {"head": {"sha": "c" * 40}}, + {"base": {**original["base"], "repo": {"full_name": "wrong/repo"}}}, + {"base": {**original["base"], "ref": "other"}}, + {"state": "closed"}, + ): + with self.subTest(changes=changes): + self.api.pull = {**original, **changes} + with self.assertRaisesRegex(ValueError, "exactly one current PR"): + self.evaluate() + self.assertFalse(self.repo.exists()) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/castiron/test_custom_code_publication.py b/scripts/castiron/test_custom_code_publication.py new file mode 100644 index 000000000..6ed1a877d --- /dev/null +++ b/scripts/castiron/test_custom_code_publication.py @@ -0,0 +1,462 @@ +# File generated from our OpenAPI spec by Castiron. See CONTRIBUTING.md for details. +"""Run the trusted publishers against offline GitHub state.""" + +from __future__ import annotations + +import hashlib +import json +import shutil +import subprocess +import unittest +from pathlib import Path +from typing import Any, cast +from unittest import mock + +import custom_code_report as report +from custom_code_test_support import GitTestCase, source_run + + +WORKFLOW = ( + Path(__file__).resolve().parents[2] / ".github/workflows/castiron-custom-code-comment.yml" +) + + +def run_publisher(step: str, payload: dict[str, Any]) -> list[dict[str, Any]]: + section = WORKFLOW.read_text().split(f"- name: {step}\n", 1)[1] + script = section.split(" script: |\n", 1)[1] + lines: list[str] = [] + for line in script.splitlines(): + if line and not line.startswith(" "): + break + lines.append(line[12:]) + output = subprocess.run( + ["node", str(Path(__file__).with_name("fixtures") / "github_publisher.cjs")], + input=json.dumps({**payload, "script": "\n".join(lines)}), + text=True, + capture_output=True, + check=True, + ) + return cast(list[dict[str, Any]], json.loads(output.stdout)) + + +class WorkflowTests(unittest.TestCase): + @unittest.skipUnless(shutil.which("node"), "GitHub Actions JavaScript runtime") + def test_trusted_failure_publisher_updates_one_current_comment(self) -> None: + head = "a" * 40 + run = {**source_run(head), "id": 20} + current = {"state": "open", "head": {"sha": head}} + previous = { + "id": 42, + "user": {"type": "Bot", "login": "github-actions[bot]"}, + "body": report.MARKER + "\n", + } + cases: dict[str, tuple[list[dict[str, Any]], dict[str, Any], str | None]] = { + "create": ([], current, "create"), + "update": ([previous], current, "update"), + "changed head": ([previous], {**current, "head": {"sha": "c" * 40}}, None), + "newer comment": ( + [{**previous, "body": report.MARKER + "\n"}], + current, + None, + ), + } + for fork in (False, True): + for name, (comments, pull, expected) in cases.items(): + with self.subTest(name=name, fork=fork): + event: dict[str, Any] = {**run, "pull_requests": []} if fork else run + writes = run_publisher( + "Publish a trusted failure status", + { + "context": { + "payload": {"workflow_run": event}, + "runId": 20, + "repo": {"owner": "openai", "repo": "example"}, + "serverUrl": "https://github.com", + }, + "current": pull, + "comments": comments, + "fallback_pulls": [{"number": 3}], + }, + ) + self.assertEqual( + [w["operation"] for w in writes], [expected] if expected else [] + ) + if expected: + self.assertIn("Report unavailable", writes[0]["body"]) + self.assertIn("castiron:run:v1:20:1", writes[0]["body"]) + + def test_workflow_reports_all_branches_without_write_credentials(self) -> None: + workflows = Path(__file__).resolve().parents[2] / ".github/workflows" + producer = (workflows / "castiron-custom-code.yml").read_text() + publisher = (workflows / "castiron-custom-code-comment.yml").read_text() + self.assertIn("pull_request:", producer) + self.assertNotIn("CASTIRON_CUSTOM_CODE_BRANCHES", producer + publisher) + self.assertNotIn("pull-requests: write", producer) + self.assertNotIn("head.repo.full_name ==", producer) + self.assertIn("workflow_run:", publisher) + concurrency = publisher.split("\nconcurrency:\n", 1)[1].split("\njobs:\n", 1)[0] + self.assertIn("queue: max", concurrency) + self.assertIn("cancel-in-progress: false", concurrency) + self.assertIn("ref: ${{ github.workflow_sha }}", publisher) + self.assertNotIn("ref: ${{ github.event.pull_request.head.sha }}", publisher) + self.assertIn("persist-credentials: false", publisher) + self.assertIn("--report", publisher) + compute, comment = publisher.split("\n comment:\n", 1) + self.assertNotIn("pull-requests: write", compute) + self.assertIn("pull-requests: read", compute) + self.assertIn(" trusted-report ", compute) + self.assertIn("ref: main", compute) + self.assertIn('--base "$(git rev-parse HEAD)"', compute) + self.assertIn("trusted_sha=$(git rev-parse HEAD)", compute) + self.assertIn("merge_group:", producer) + self.assertNotIn("download-artifact@", compute) + self.assertNotIn("unittest", compute) + self.assertIn("needs: compute", comment) + self.assertIn("artifact-ids: ${{ needs.compute.outputs.artifact-id }}", comment) + self.assertNotIn("run-id: ${{ github.event.workflow_run.id }}", comment) + self.assertNotIn("git fetch", comment) + self.assertIn("--artifact-run-id", comment) + digest = hashlib.sha256( + (workflows.parents[1] / "scripts/castiron/custom_code_report.py").read_bytes() + ).hexdigest() + self.assertIn(f"REPORTER_SHA256: {digest}", producer) + + +class CommentTests(GitTestCase): + def test_comment_only_rerun_links_to_the_compute_artifact_attempt(self) -> None: + workflow = ( + Path(__file__).resolve().parents[2] + / ".github/workflows/castiron-custom-code-comment.yml" + ).read_text() + compute, comment = workflow.split("\n comment:\n", 1) + + def field(section: str, prefix: str) -> str: + return next( + line.removeprefix(prefix) + for line in section.splitlines() + if line.startswith(prefix) + ) + + def resolve(value: str, context: dict[str, str]) -> str: + for key, replacement in context.items(): + value = value.replace("${{ " + key + " }}", replacement) + self.assertNotIn("${{", value) + return value + + # A successful compute job's outputs survive a comment-only rerun. + compute_context = {"github.run_id": "9", "github.run_attempt": "3"} + uploaded_name = resolve(field(compute, " name: "), compute_context) + saved_attempt = resolve(field(compute, " artifact-run-attempt: "), compute_context) + comment_context = { + "github.run_id": "9", + "github.run_attempt": "4", + "needs.compute.outputs.artifact-run-attempt": saved_attempt, + } + artifact_attempt = resolve( + field(comment, " ARTIFACT_RUN_ATTEMPT: "), comment_context + ) + self.assertEqual(uploaded_name, "castiron-custom-code-9-3") + self.assertEqual(artifact_attempt, "3") + + _, base = self.baseline() + result, _ = report.build_report(self.repo, base, base) + pull: dict[str, Any] = { + "state": "open", + "head": {"sha": base}, + "base": {"sha": base, "ref": "main", "repo": {"full_name": "openai/example"}}, + } + run = { + "event": "pull_request", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": base, + "run_attempt": 1, + "pull_requests": [{"number": 1}], + } + with mock.patch.object( + report, "api", side_effect=[pull, run, [], pull, {"html_url": "published"}] + ) as api: + self.assertEqual( + report.publish_comment( + result, + "openai/example", + 1, + 2, + 1, + artifact_run_id=9, + artifact_run_attempt=int(artifact_attempt), + ), + "published", + ) + body = api.call_args.args[2]["body"] + self.assertIn(f"--name {uploaded_name}", body) + self.assertNotIn("--name castiron-custom-code-9-4", body) + self.assertIn("castiron:run:v1:2:1", body) + + def test_comment_updates_existing_bot_comment_and_skips_stale(self) -> None: + _, base = self.baseline() + result, _ = report.build_report(self.repo, base, base) + calls: list[tuple[str, str, object]] = [] + + def fake_api(method: str, path: str, payload: object = None) -> object: + calls.append((method, path, payload)) + if "/pulls/" in path: + return { + "state": "open", + "head": {"sha": base}, + "base": {"sha": base, "ref": "main", "repo": {"full_name": "openai/example"}}, + } + if "/actions/runs/" in path: + return { + "event": "pull_request", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": base, + "run_attempt": 1, + "pull_requests": [{"number": 1}], + } + if "/comments?" in path: + return [ + {"id": 7, "user": {"login": "someone"}, "body": report.MARKER}, + { + "id": 8, + "user": {"login": "github-actions[bot]"}, + "body": report.MARKER, + "html_url": "existing", + }, + ] + return {"html_url": "updated"} + + with mock.patch.object(report, "api", side_effect=fake_api): + self.assertEqual(report.publish_comment(result, "openai/example", 1, 2, 1), "updated") + self.assertEqual(calls[-1][:2], ("PATCH", "repos/openai/example/issues/comments/8")) + calls.clear() + result["head_sha"] = "f" * 40 + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 1), "Skipped stale report" + ) + self.assertEqual(len(calls), 1) + + def test_comment_rejects_older_runs_attempts_and_wrong_pr(self) -> None: + _, base = self.baseline() + result, _ = report.build_report(self.repo, base, base) + pull: dict[str, Any] = { + "state": "open", + "head": {"sha": base}, + "base": {"sha": base, "ref": "main", "repo": {"full_name": "openai/example"}}, + } + run = { + "event": "pull_request", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": base, + "run_attempt": 2, + "pull_requests": [{"number": 1}], + "head_repository": {"owner": {"login": "contributor"}}, + "head_branch": "fix/branch", + } + comment = { + "id": 8, + "user": {"login": "github-actions[bot]"}, + "body": report.MARKER + "\n", + "html_url": "existing", + } + with mock.patch.object(report, "api", side_effect=[pull, run]) as api: + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 1), "Skipped stale report" + ) + self.assertEqual(api.call_count, 2) + with mock.patch.object(report, "api", side_effect=[pull, run, [comment]]) as api: + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" + ) + self.assertEqual(api.call_count, 3) + with ( + mock.patch.object( + report, "api", side_effect=[pull, {**run, "pull_requests": []}, [], []] + ), + self.assertRaisesRegex(report.ReportError, "does not match report PR"), + ): + report.publish_comment(result, "openai/example", 1, 2, 2) + with ( + mock.patch.object(report, "api", side_effect=[pull, {**run, "path": "other.yml"}]), + self.assertRaisesRegex(report.ReportError, "does not match report PR"), + ): + report.publish_comment(result, "openai/example", 1, 2, 2) + with mock.patch.object( + report, + "api", + side_effect=[pull, {**run, "pull_requests": []}, [{"number": 1}], [comment]], + ) as api: + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" + ) + self.assertIn(f"/commits/{base}/pulls", api.call_args_list[2].args[1]) + changed = {**pull, "head": {"sha": "f" * 40}} + with mock.patch.object(report, "api", side_effect=[pull, run, [], changed]) as api: + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" + ) + self.assertEqual(api.call_count, 4) + for target in ( + {**pull["base"], "ref": "other"}, + {**pull["base"], "repo": {"full_name": "other/repo"}}, + ): + with mock.patch.object( + report, "api", side_effect=[pull, run, [], {**pull, "base": target}] + ): + self.assertEqual( + report.publish_comment(result, "openai/example", 1, 2, 2), + "Skipped stale report", + ) + + +@unittest.skipUnless(shutil.which("node"), "Node is needed to execute the status-publisher fixture") +class StatusPublisherTests(unittest.TestCase): + def publish( + self, + *, + event_name: str = "pull_request", + head_changed: bool = False, + base_changed: bool = False, + no_result: bool = False, + failed_budget: bool = False, + fallback_pulls: list[dict[str, int]] | None = None, + run_overrides: dict[str, Any] | None = None, + previous_statuses: list[dict[str, Any]] | None = None, + ) -> list[dict[str, Any]]: + base, head = "a" * 40, "b" * 40 + payload = { + "fallback_pulls": fallback_pulls, + "previous_statuses": previous_statuses or [], + "context": { + "eventName": "workflow_run", + "repo": {"owner": "openai", "repo": "example"}, + "serverUrl": "https://github.com", + "runId": 123, + "payload": { + "workflow_run": source_run(head, event_name), + }, + }, + "run": {**source_run(head, event_name), **(run_overrides or {})}, + "current": { + "state": "open", + "head": {"sha": "c" * 40 if head_changed else head}, + "base": { + "sha": "c" * 40 if base_changed else base, + "ref": "main", + "repo": {"full_name": "openai/example"}, + }, + }, + "env": { + "BASE_SHA": "" if no_result else base, + "HEAD_SHA": "" if no_result else head, + "ISOLATION_RESULT": "success", + "BUDGET_RESULT": "failure" if failed_budget else "success", + "PUBLISH_ATTEMPT": "1", + }, + } + return run_publisher("Publish exact-head statuses after checking freshness", payload) + + def test_statuses_attach_to_candidate_not_main(self) -> None: + for event in ("pull_request", "merge_group"): + with self.subTest(event=event): + results = self.publish(event_name=event) + self.assertEqual(len(results), 2) + self.assertTrue( + all(r["sha"] == "b" * 40 and r["state"] == "success" for r in results) + ) + + def test_fork_statuses_with_no_commit_association(self) -> None: + options: dict[str, Any] = { + "run_overrides": {"pull_requests": []}, + "fallback_pulls": [{"number": 3}], + } + results = self.publish(**options) + self.assertEqual(len(results), 2) + self.assertTrue(all(r["sha"] == "b" * 40 and r["state"] == "success" for r in results)) + self.assertEqual(self.publish(**options, head_changed=True), []) + self.assertTrue( + all(r["state"] == "failure" for r in self.publish(**options, no_result=True)) + ) + self.assertEqual(self.publish(run_overrides={"pull_requests": []}, fallback_pulls=[]), []) + self.assertEqual( + self.publish( + run_overrides={"pull_requests": []}, fallback_pulls=[{"number": 3}, {"number": 4}] + ), + [], + ) + + def test_stale_pr_head_is_not_published(self) -> None: + self.assertEqual(self.publish(head_changed=True), []) + + def test_pr_snapshot_survives_main_advancing(self) -> None: + results = self.publish(base_changed=True) + self.assertEqual(len(results), 2) + self.assertTrue(all(r["state"] == "success" for r in results)) + self.assertTrue(all("a" * 12 in r["description"] for r in results)) + + def test_queue_still_rejects_main_advancing(self) -> None: + results = self.publish(event_name="merge_group", base_changed=True) + self.assertEqual(len(results), 2) + self.assertTrue(all(r["state"] == "failure" for r in results)) + + def test_missing_evaluation_publishes_actionable_failures(self) -> None: + for event in ("pull_request", "merge_group"): + results = self.publish(event_name=event, no_result=True, base_changed=True) + self.assertEqual(len(results), 2) + for result in results: + self.assertEqual(result["state"], "failure") + self.assertIn("inspect the trusted run", result["description"]) + self.assertTrue(result["target_url"].endswith("/actions/runs/123")) + + def test_superseded_or_wrong_source_run_cannot_publish(self) -> None: + for overrides in ( + {"run_attempt": 2}, + {"head_sha": "c" * 40}, + {"event": "push"}, + {"path": "other.yml"}, + ): + with self.subTest(overrides=overrides): + self.assertEqual(self.publish(run_overrides=overrides), []) + + def test_independent_check_failures_are_preserved(self) -> None: + results = self.publish(failed_budget=True) + self.assertEqual([r["state"] for r in results], ["success", "failure"]) + + def test_older_evaluation_cannot_overwrite_newer_failure(self) -> None: + for order in ("124:1:125:1", "123:2:125:1", "123:1:125:1", "123:1:123:2"): + with self.subTest(order=order): + previous = { + "context": "Castiron / custom-code budget", + "creator": {"login": "github-actions[bot]"}, + "state": "failure", + "description": f"Failed against newer main. [evaluation {order}]", + "target_url": f"https://github.com/openai/example/actions/runs/{order.split(':')[2]}", + } + self.assertEqual(self.publish(previous_statuses=[previous]), []) + + def test_publication_guard_allows_current_retry_and_ignores_unrelated_statuses(self) -> None: + previous = { + "context": "Castiron / custom-code budget", + "creator": {"login": "github-actions[bot]"}, + "description": "Failed. [evaluation 123:1:123:1]", + "target_url": "https://github.com/openai/example/actions/runs/123", + } + for overrides in ( + {}, # A partial publication can retry the same evaluation. + { + "description": "Failed. [evaluation 122:9:999:9]", + "target_url": "https://github.com/openai/example/actions/runs/999", + }, + {"description": "Legacy status without evaluation marker"}, + {"context": "Other check", "description": "[evaluation 999:9:999:9]"}, + {"creator": {"login": "someone"}, "description": "[evaluation 999:9:999:9]"}, + {"description": "[evaluation 999:9:999:9]", "target_url": "https://example.com/999"}, + ): + with self.subTest(overrides=overrides): + results = self.publish(previous_statuses=[{**previous, **overrides}]) + self.assertEqual(len(results), 2) + self.assertTrue(all(r["state"] == "success" for r in results)) + self.assertTrue(all(len(r["description"]) <= 140 for r in results)) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/castiron/test_custom_code_report.py b/scripts/castiron/test_custom_code_report.py index 8af76e25f..ec29d95f4 100644 --- a/scripts/castiron/test_custom_code_report.py +++ b/scripts/castiron/test_custom_code_report.py @@ -5,98 +5,17 @@ import hashlib import json import os -import shutil import struct import subprocess -import tempfile -import textwrap import unittest import unittest.mock as mock -from pathlib import Path -from typing import Any import custom_code_report as report -GENERATION = "550e8400-e29b-41d4-a716-446655440000" +from custom_code_test_support import GENERATION, GitTestCase -def source_run(head: str, pull_requests: list[dict[str, int]] | None = None) -> dict[str, Any]: - return { - "event": "pull_request", - "status": "completed", - "path": ".github/workflows/castiron-custom-code.yml", - "head_sha": head, - "head_branch": "sdk", - "head_repository": { - "id": 7, - "full_name": "fork/example", - "owner": {"login": "fork"}, - }, - "repository": {"full_name": "openai/example"}, - "run_attempt": 1, - "pull_requests": [{"number": 1}] if pull_requests is None else pull_requests, - } - - -def source_pull(head: str, base: str, number: int = 1) -> dict[str, Any]: - return { - "number": number, - "state": "open", - "head": { - "sha": head, - "ref": "sdk", - "repo": {"id": 7, "full_name": "fork/example"}, - }, - "base": { - "sha": base, - "ref": "main", - "repo": {"full_name": "openai/example"}, - }, - } - - -class CustomCodeTests(unittest.TestCase): - # Keep the vendored test stdlib-only on Python 3.10 (no typing.override yet). - def setUp(self) -> None: # pyright: ignore[reportImplicitOverride] - self.temporary = tempfile.TemporaryDirectory() - self.addCleanup(self.temporary.cleanup) - self.repo = Path(self.temporary.name) - self.git("init", "-q", "-b", "main") - self.git("config", "user.name", "Castiron test") - self.git("config", "user.email", "castiron@example.test") - - def git(self, *args: str) -> str: - return subprocess.run( - ["git", "-C", str(self.repo), *args], check=True, capture_output=True, text=True - ).stdout.strip() - - def write(self, path: str, body: str) -> None: - target = self.repo / path - target.parent.mkdir(parents=True, exist_ok=True) - target.write_text(body) - - def commit(self, message: str = "fixture") -> str: - self.git("add", "-A") - self.git("commit", "-q", "--allow-empty", "-m", message) - return self.git("rev-parse", "HEAD") - - def baseline(self) -> tuple[str, str]: - self.write("generated.py", "generated\n") - metadata = { - "generation_id": GENERATION, - "source_branch": "test", - "target": "openai-python", - "language": "python", - } - encoded = base64.b64encode(json.dumps(metadata).encode()).decode() - generated = self.commit(f"codegen\n\nGeneration metadata: {encoded}") - self.git("update-ref", "refs/remotes/origin/codegen/test", generated) - self.write( - ".castiron.stats.yml", - f"schema_version: 1\ngeneration_id: {GENERATION}\ncodegen_sha: {generated}\ncodegen_hash: {report.hash_codegen_commit(self.repo, generated)}\n", - ) - return generated, self.commit("integrated") - +class CustomCodeTests(GitTestCase): def test_git_preserves_authentication_without_inherited_repository_routing(self) -> None: environment = { "GIT_CONFIG_GLOBAL": "/ordinary/gitconfig", @@ -307,386 +226,6 @@ def test_new_mixed_file_headline_includes_changed_generated_baselines(self) -> N self.assertNotIn("No new custom-code files", body) self.assertIn("1 newly customized", body) - @unittest.skipUnless(shutil.which("node"), "GitHub Actions JavaScript runtime") - def test_trusted_failure_publisher_updates_one_current_comment(self) -> None: - workflow = ( - Path(__file__).resolve().parents[2] - / ".github/workflows/castiron-custom-code-comment.yml" - ) - section = workflow.read_text().split("- name: Publish a trusted failure status\n", 1)[1] - script = textwrap.dedent(section.split("script: |\n", 1)[1]) - harness = r""" -const assert = require('node:assert/strict'); -const AsyncFunction = Object.getPrototypeOf(async function(){}).constructor; -async function check(stale, exists, priorRun, expected) { - const writes = []; - const event = {id: 20, run_attempt: 1, event: 'pull_request', path: '.github/workflows/castiron-custom-code.yml', head_sha: 'a'.repeat(40), pull_requests: [{number: 1}]}; - const run = {...event, status: 'completed', repository: {full_name: 'openai/example'}, - head_branch: 'sdk', head_repository: {id: 7, full_name: 'fork/example', owner: {login: 'fork'}}}; - const current = {number: 1, state: 'open', - head: {sha: (stale ? 'c' : 'a').repeat(40), ref: 'sdk', repo: {id: 7, full_name: 'fork/example'}}, - base: {sha: 'b'.repeat(40), ref: 'main', repo: {full_name: 'openai/example'}}}; - const previous = {id: 42, user: {type: 'Bot', login: 'github-actions[bot]'}, - body: `\n`}; - const github = {paginate: async () => exists ? [previous] : [], rest: { - pulls: {get: async () => ({data: current})}, - actions: {getWorkflowRun: async () => ({data: run})}, - repos: {get: async () => ({data: {default_branch: 'main'}})}, - git: {getRef: async () => ({data: {object: {sha: 'b'.repeat(40)}}})}, - issues: {listComments() {}, updateComment: async x => writes.push(['update', x]), - createComment: async x => writes.push(['create', x])}}}; - const context = {payload: {workflow_run: event}, repo: {owner: 'openai', repo: 'example'}, - runId: 20, serverUrl: 'https://github.com'}; - await new AsyncFunction('github', 'context', SCRIPT)(github, context); - assert.equal(writes.length, expected ? 1 : 0); - if (expected) { - assert.equal(writes[0][0], expected); - assert.match(writes[0][1].body, /Report unavailable/); - assert.match(writes[0][1].body, /castiron:run:v1:20:1/); - } -} -(async () => { - await check(false, true, 10, 'update'); - await check(false, false, 10, 'create'); - await check(true, true, 10, null); - await check(false, true, 21, null); -})().catch(error => { console.error(error); process.exitCode = 1; }); -""" - subprocess.run( - ["node", "-e", "const SCRIPT = " + json.dumps(script) + ";\n" + harness], - check=True, - env={**os.environ, "GITHUB_RUN_ATTEMPT": "1"}, - ) - - def test_workflow_reports_all_branches_without_write_credentials(self) -> None: - workflows = Path(__file__).resolve().parents[2] / ".github/workflows" - producer = (workflows / "castiron-custom-code.yml").read_text() - publisher = (workflows / "castiron-custom-code-comment.yml").read_text() - self.assertIn("pull_request:", producer) - self.assertNotIn("CASTIRON_CUSTOM_CODE_BRANCHES", producer + publisher) - self.assertNotIn("pull-requests: write", producer) - self.assertNotIn("head.repo.full_name ==", producer) - self.assertIn("workflow_run:", publisher) - self.assertIn("ref: ${{ github.workflow_sha }}", publisher) - self.assertNotIn("ref: ${{ github.event.pull_request.head.sha }}", publisher) - self.assertIn("persist-credentials: false", publisher) - self.assertIn("--report", publisher) - compute, comment = publisher.split("\n comment:\n", 1) - self.assertNotIn("pull-requests: write", compute) - self.assertIn("pull-requests: read", compute) - self.assertIn(" trusted-report ", compute) - self.assertNotIn("download-artifact@", compute) - self.assertNotIn("unittest", compute) - self.assertIn("needs: compute", comment) - self.assertIn("artifact-ids: ${{ needs.compute.outputs.artifact-id }}", comment) - self.assertNotIn("run-id: ${{ github.event.workflow_run.id }}", comment) - self.assertNotIn("git fetch", comment) - self.assertIn("--artifact-run-id", comment) - digest = hashlib.sha256( - (workflows.parents[1] / "scripts/castiron/custom_code_report.py").read_bytes() - ).hexdigest() - self.assertIn(f"REPORTER_SHA256: {digest}", producer) - - def test_comment_only_rerun_links_to_the_compute_artifact_attempt(self) -> None: - workflow = ( - Path(__file__).resolve().parents[2] - / ".github/workflows/castiron-custom-code-comment.yml" - ).read_text() - compute, comment = workflow.split("\n comment:\n", 1) - - def field(section: str, prefix: str) -> str: - return next( - line.removeprefix(prefix) - for line in section.splitlines() - if line.startswith(prefix) - ) - - def resolve(value: str, context: dict[str, str]) -> str: - for key, replacement in context.items(): - value = value.replace("${{ " + key + " }}", replacement) - self.assertNotIn("${{", value) - return value - - # A successful compute job's outputs survive a comment-only rerun. - compute_context = {"github.run_id": "9", "github.run_attempt": "3"} - uploaded_name = resolve(field(compute, " name: "), compute_context) - saved_attempt = resolve(field(compute, " artifact-run-attempt: "), compute_context) - comment_context = { - "github.run_id": "9", - "github.run_attempt": "4", - "needs.compute.outputs.artifact-run-attempt": saved_attempt, - } - artifact_attempt = resolve( - field(comment, " ARTIFACT_RUN_ATTEMPT: "), comment_context - ) - self.assertEqual(uploaded_name, "castiron-custom-code-9-3") - self.assertEqual(artifact_attempt, "3") - - _, base = self.baseline() - result, _ = report.build_report(self.repo, base, base) - pull = source_pull(base, base) - run = source_run(base) - with mock.patch.object( - report, - "api", - side_effect=[ - run, - {"default_branch": "main"}, - {"object": {"sha": base}}, - pull, - [], - pull, - {"html_url": "published"}, - ], - ) as api: - self.assertEqual( - report.publish_comment( - result, - "openai/example", - 1, - 2, - 1, - artifact_run_id=9, - artifact_run_attempt=int(artifact_attempt), - ), - "published", - ) - body = api.call_args.args[2]["body"] - self.assertIn(f"--name {uploaded_name}", body) - self.assertNotIn("--name castiron-custom-code-9-4", body) - self.assertIn("castiron:run:v1:2:1", body) - - def test_trusted_report_recomputes_pr_output_in_a_bare_repository(self) -> None: - generated, _ = self.baseline() - content_hash = report.hash_codegen_commit(self.repo, generated) - snapshot = report.create_public_snapshot( - self.repo, - self.git("rev-parse", f"{generated}^{{tree}}"), - GENERATION, - content_hash, - "codegen/public-test", - None, - ) - self.git("branch", "codegen/public-test", snapshot) - stats = (self.repo / ".castiron.stats.yml").read_text() - self.write(".castiron.stats.yml", stats + f"public_codegen_sha: {snapshot}\n") - base = self.commit() - legitimate, _ = report.build_report(self.repo, base, base, require_head_hash=True) - self.write("generated.py", "generated\n# custom\n") - # Neither a replacement reporter nor its claimed result may be executed - # or read by the trusted job. - self.write("scripts/castiron/custom_code_report.py", "raise RuntimeError('PR code ran')\n") - self.write("report.json", json.dumps(legitimate)) - head = self.commit() - broken_stats = ( - (self.repo / ".castiron.stats.yml").read_text().replace(content_hash, "0" * 64) - ) - self.write(".castiron.stats.yml", broken_stats) - broken = self.commit() - remote = self.repo / "public.git" - self.git("clone", "--bare", str(self.repo), str(remote)) - real_git = report.git - - def local_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: - if args[:3] == ("remote", "add", "origin"): - self.assertEqual(args[3], "https://github.com/openai/example.git") - args = (*args[:3], str(remote)) - self.assertNotIn("checkout", args) - return real_git(repo, *args, input_bytes=input_bytes) - - for label, revision in (("genuine", base), ("custom", head), ("broken", broken)): - with self.subTest(label=label): - calls: list[tuple[str, str]] = [] - bodies: list[str] = [] - pull = source_pull(revision, base) - run = source_run(revision, []) - forged: dict[str, Any] = {**legitimate, "head_sha": revision, "files": []} - self.assertIn("Generated baselines verified", report.render_report(forged)) - producer = self.repo / f"producer-{label}" - producer.mkdir() - (producer / "report.json").write_text(json.dumps(forged)) - - def fake_api(method: str, path: str, payload: dict[str, Any] | None = None) -> Any: - calls.append((method, path)) - if method == "GET": - responses: dict[str, Any] = { - "repos/openai/example": {"default_branch": "main", "private": False}, - "repos/openai/example/actions/runs/2": run, - "repos/openai/example/git/ref/heads/main": {"object": {"sha": base}}, - "repos/openai/example/pulls?state=open&head=fork%3Asdk&base=main&per_page=100": [ - {"number": 1} - ], - "repos/openai/example/pulls/1": pull, - "repos/openai/example/issues/1/comments?per_page=100&page=1": [], - } - if path in responses: - return responses[path] - if ( - method == "POST" - and path == "repos/openai/example/issues/1/comments" - and payload - ): - bodies.append(payload["body"]) - return { - "html_url": "https://github.com/openai/example/pull/1#issuecomment-1" - } - raise AssertionError(f"unexpected API call: {method} {path}") - - objects = self.repo / f"objects-{label}.git" - out = self.repo / f"trusted-{label}" - with ( - mock.patch.object(report, "api", side_effect=fake_api), - mock.patch.object(report, "git", side_effect=local_git), - ): - report.trusted_report(objects, "openai/example", 2, 1, out) - self.assertTrue(all(method == "GET" for method, _ in calls)) - self.assertEqual( - real_git(objects, "rev-parse", "--is-bare-repository"), b"true\n" - ) - self.assertFalse((objects / "scripts").exists()) - actual = json.loads((out / "report.json").read_text()) - report.publish_comment( - actual, - "openai/example", - 1, - 2, - 1, - artifact_run_id=9, - artifact_run_attempt=3, - ) - self.assertEqual(len(bodies), 1) - body = bodies[0] - self.assertIn("castiron:run:v1:2:1", body) - self.assertIn("/actions/runs/9", body) - if label == "broken": - self.assertIn("Report unavailable", body) - self.assertNotIn("Generated baselines verified", body) - elif label == "custom": - self.assertIn("1 newly customized", body) - self.assertIn("generated.py", body) - self.assertIn(b"+# custom", (out / "custom-code.patch").read_bytes()) - self.assertNotIn("No new custom-code files detected", body) - else: - self.assertIn("No new custom-code files detected", body) - self.assertIn("Generated baselines verified", body) - self.assertIn("--name castiron-custom-code-9-3", body) - - def test_trusted_report_rejects_invalid_or_stale_association_before_fetch(self) -> None: - run = source_run("a" * 40) - pull = source_pull("a" * 40, "b" * 40) - metadata = {"default_branch": "main", "private": False} - main = {"object": {"sha": "b" * 40}} - cases: list[tuple[list[Any], bool]] = [ - ([{**run, "path": "other.yml"}], True), - ([{**run, "status": "in_progress"}], True), - ([{**run, "run_attempt": 2}], False), - ([run, metadata, main, {**pull, "state": "closed"}], False), - ([run, metadata, main, {**pull, "head": {**pull["head"], "sha": "c" * 40}}], False), - ( - [run, metadata, main, {**pull, "base": {**pull["base"], "repo": {"full_name": "other/repo"}}}], - False, - ), - ([{**run, "pull_requests": []}, metadata, main, []], False), - ([{**run, "pull_requests": [{"number": 1}, {"number": 2}]}, metadata, main, pull, {**pull, "number": 2}], True), - ] - for responses, raises in cases: - with ( - self.subTest(responses=responses), - mock.patch.object(report, "api", side_effect=responses), - mock.patch.object(report, "git") as git, - ): - if raises: - with self.assertRaises(report.ReportError): - report.trusted_report( - self.repo / "objects", "openai/example", 2, 1, self.repo / "out" - ) - else: - report.trusted_report( - self.repo / "objects", "openai/example", 2, 1, self.repo / "out" - ) - git.assert_not_called() - self.assertFalse((self.repo / "out").exists()) - - def test_fork_run_uses_scoped_open_pr_lookup_and_rejects_unsafe_matches(self) -> None: - head, base = "a" * 40, "b" * 40 - run = source_run(head, []) - pull = source_pull(head, base) - with mock.patch.object(report, "api", side_effect=[[{"number": 1}], pull]) as api: - self.assertEqual( - report.current_pull_request( - "repos/openai/example", "openai/example", run, "main", base - ), - pull, - ) - self.assertEqual( - api.call_args_list[0].args[1], - "repos/openai/example/pulls?state=open&head=fork%3Asdk&base=main&per_page=100", - ) - cases = ( - ( - [[{"number": 1}], {**pull, "head": {**pull["head"], "ref": "spoofed"}}], - None, - ), - ( - [[{"number": 1}], {**pull, "base": {**pull["base"], "sha": "c" * 40}}], - None, - ), - ( - [ - [{"number": 1}, {"number": 2}], - pull, - {**pull, "number": 2}, - ], - report.ReportError, - ), - ) - for responses, expected in cases: - with self.subTest(expected=expected), mock.patch.object( - report, "api", side_effect=responses - ): - if expected is report.ReportError: - with self.assertRaises(report.ReportError): - report.current_pull_request( - "repos/openai/example", "openai/example", run, "main", base - ) - else: - self.assertIsNone( - report.current_pull_request( - "repos/openai/example", "openai/example", run, "main", base - ) - ) - - def later_page( - method: str, path: str, payload: dict[str, Any] | None = None - ) -> Any: - self.assertEqual(method, "GET") - self.assertIsNone(payload) - if path.endswith("per_page=100"): - return [{"number": number} for number in range(1, 101)] - if path.endswith("per_page=100&page=2"): - return [{"number": 101}] - number = int(path.rsplit("/", 1)[1]) - if number == 101 or (ambiguous and number == 1): - return source_pull(head, base, number) - return {**source_pull(head, base, number), "state": "closed"} - - for ambiguous in (False, True): - with self.subTest(ambiguous=ambiguous), mock.patch.object( - report, "api", side_effect=later_page - ): - if ambiguous: - with self.assertRaises(report.ReportError): - report.current_pull_request( - "repos/openai/example", "openai/example", run, "main", base - ) - else: - self.assertEqual( - report.current_pull_request( - "repos/openai/example", "openai/example", run, "main", base - )["number"], - 101, - ) - def test_removals_include_changed_baselines_but_not_handwritten_only_files(self) -> None: _, base = self.baseline() result, _ = report.build_report(self.repo, base, base) @@ -915,98 +454,6 @@ def test_public_snapshot_has_no_private_history_and_reports_without_private_remo self.assertEqual(merged_report["after"]["commit"], advanced["commit"]) self.assertEqual(merged_report["counts"]["newly_customized"], 1) - def test_comment_updates_existing_bot_comment_and_skips_stale(self) -> None: - _, base = self.baseline() - result, _ = report.build_report(self.repo, base, base) - calls: list[tuple[str, str, object]] = [] - - def fake_api(method: str, path: str, payload: object = None) -> object: - calls.append((method, path, payload)) - if "/pulls/" in path: - return source_pull(base, base) - if "/actions/runs/" in path: - return source_run(base) - if path == "repos/openai/example": - return {"default_branch": "main"} - if path == "repos/openai/example/git/ref/heads/main": - return {"object": {"sha": base}} - if "/comments?" in path: - return [ - {"id": 7, "user": {"login": "someone"}, "body": report.MARKER}, - { - "id": 8, - "user": {"login": "github-actions[bot]"}, - "body": report.MARKER, - "html_url": "existing", - }, - ] - return {"html_url": "updated"} - - with mock.patch.object(report, "api", side_effect=fake_api): - self.assertEqual(report.publish_comment(result, "openai/example", 1, 2, 1), "updated") - self.assertEqual(calls[-1][:2], ("PATCH", "repos/openai/example/issues/comments/8")) - calls.clear() - result["head_sha"] = "f" * 40 - with self.assertRaisesRegex(report.ReportError, "does not match report PR"): - report.publish_comment(result, "openai/example", 1, 2, 1) - self.assertFalse(any(method in {"PATCH", "POST"} for method, _, _ in calls)) - - def test_comment_rejects_older_runs_attempts_and_wrong_pr(self) -> None: - _, base = self.baseline() - result, _ = report.build_report(self.repo, base, base) - pull = source_pull(base, base) - run = {**source_run(base), "run_attempt": 2} - metadata = {"default_branch": "main"} - main = {"object": {"sha": base}} - comment = { - "id": 8, - "user": {"login": "github-actions[bot]"}, - "body": report.MARKER + "\n", - "html_url": "existing", - } - with mock.patch.object(report, "api", side_effect=[run, metadata, main, pull]) as api: - self.assertEqual( - report.publish_comment(result, "openai/example", 1, 2, 1), "Skipped stale report" - ) - self.assertEqual(api.call_count, 4) - with mock.patch.object(report, "api", side_effect=[run, metadata, main, pull, [comment]]) as api: - self.assertEqual( - report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" - ) - self.assertEqual(api.call_count, 5) - with ( - mock.patch.object(report, "api", side_effect=[{**run, "pull_requests": []}, metadata, main, []]), - self.assertRaisesRegex(report.ReportError, "does not match report PR"), - ): - report.publish_comment(result, "openai/example", 1, 2, 2) - with ( - mock.patch.object(report, "api", side_effect=[{**run, "path": "other.yml"}]), - self.assertRaisesRegex(report.ReportError, "does not match report PR"), - ): - report.publish_comment(result, "openai/example", 1, 2, 2) - with mock.patch.object( - report, - "api", - side_effect=[ - {**run, "pull_requests": []}, - metadata, - main, - [{"number": 1}], - pull, - [comment], - ], - ) as api: - self.assertEqual( - report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" - ) - self.assertIn("/pulls?state=open&head=fork%3Asdk", api.call_args_list[3].args[1]) - changed = {**pull, "head": {**pull["head"], "sha": "f" * 40}} - with mock.patch.object(report, "api", side_effect=[run, metadata, main, pull, [], changed]) as api: - self.assertEqual( - report.publish_comment(result, "openai/example", 1, 2, 2), "Skipped stale report" - ) - self.assertEqual(api.call_count, 6) - if __name__ == "__main__": unittest.main()