Repository navigation
test(evals): compare production Standard scan time budgets - #1679
mldangelo-oai wants to merge 22 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 453299b0aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e60b241fe3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
mldangelo-oai
changed the title
1fb684c to
f8d6983
Compare
fcdada1 to
972d96c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 972d96c5cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f8d6983 to
d176dcd
Compare
972d96c to
a843299
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a843299654
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
18ca884 to
e102079
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e102079737
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e102079 to
710f985
Compare
2b877d5 to
12abd7d
Compare
710f985 to
6dee75e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6dee75e349
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6dee75e to
0b8be06
Compare
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the production-pipeline comparison. My owner-level merge-safety review of 99b66c388e82d8af30524738884d5b4c8507c019 found one remaining case of the runtime-provenance issue: the supported CODEX_CLI_PATH override reaches the native launch, while experiment.json records only the package-pinned Codex version and does not hash the selected executable. Two distinct synthetic executables ran through the production runner’s default Codex factory and installed SDK with identical saved provenance reporting 0.163.0. Please record the resolved executable’s identity/version or hash, or explicitly pin the executable used by the benchmark. This needs no general environment sanitization. I have not duplicated the existing inline thread.
Builds, types, formatting and Markdown checks passed. Locally 132 of 134 deterministic eval tests passed; two production integration cases stop at the existing trusted-owner environment restriction. The exact matching hosted CI passed all 134, including those cases. Three independent entrypoint checks rejected missing API keys, an unrepresentable deadline and invalid pair count before any report staging or model work. The native-override probe used a minimal client adapter around the real SDK subprocess because the full protected path is locally blocked; no live model comparison or paid API request was made.
github-actions
Bot
added
the
skip-release-notes
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 998816e545
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…/codex/pr-refresh-1679
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0edcda960d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Current-head review of 0edcda960d6a08bbbc9c40f0895b9b86057462f9 confirms two existing issues: selected-executable provenance still omits a supported CODEX_CLI_PATH override, and state-cleanup failure still permits the next arm to read retained prior-arm artifacts. Both were reproduced again with synthetic executables/filesystem failures and no model calls; those inline threads are not duplicated. Please record the actual native executable and stop further arms when state cleanup fails.
Fresh builds, types, formatting, all 24 deterministic Standard evaluation tests and three entrypoint preflight checks pass. The evaluator and relevant runtime inputs are unchanged from the earlier review; its two ownership-restricted integration cases are not claimed as passing. One nonblocking inline suggestion removes duplicated CLI documentation.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Re-reviewed 9ef88655ad4f against its exact updated base. The following previously verified issues remain in unchanged production code and need attention:
- Record the selected native executable in evaluation provenance
- Stop the comparison after state-directory cleanup fails
The current delta consists of the separately reviewed main integration and its test/document merge adjustments, which I inspected. Relevant feature code and dependencies match the preceding review; its focused tests and reproductions are reused with their recorded limitations, rather than claimed as fresh execution. The shared owner/configuration/CLI integration passed 189 checks. No duplicate inline comments or full current-head platform pass are claimed. The duplicated documentation section has been removed; thanks for that cleanup.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for adding a reproducible comparison around the production Standard runner. The current head fixes executable provenance and stops the comparison when per-arm state cleanup fails; the grading and paired summaries preserve failed or uncertain observations. I found no remaining actionable issue. Builds, type checking, formatting, 134 local eval tests, and focused entrypoint/provenance/cleanup probes passed. The two locally ownership-blocked cases both passed in the exact-head hosted eval job (136/136 eval tests). Overall CI remains red because an SDK shard failed during package installation with a Microsoft repository HTTP 403; the aggregate jobs reflect that setup failure. No live paid-model experiment was run.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the update. Reviewed all 20 changed files against the current base. The evaluation code remains identical to the previous approved head; the new workflow change consistently gives all Windows diagnostic modes 90 minutes while preserving Linux limits, test deadlines and matrix coverage. No actionable issue found.
Fresh validation: 134 deterministic evaluation tests, 36 workflow/package tests, three entrypoint preflight checks, frozen installs, builds, types and formatting pass. The two unchanged ownership-blocked cases were excluded locally; their prior hosted passes remain supported by exact input comparison. Current hosted checks are still running, with two canceled behavior jobs; no complete platform pass or live model experiment is claimed.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the identity repair. The owner-level merge-safety pass found no actionable issue across all 23 changed files. File, directory and handle checks now preserve exact integer identities while retaining the existing path, permission and cleanup checks. The evaluation feature remains unchanged from the prior review.
All 11 setup checks passed. Three focused cases and two built-reader controls passed; 16 fixture cases stopped at the established ownership guard. Leaf and later-stage replacement behavior was source-reviewed, with no guard bypass. Full platform suites and live model evaluation were not run.
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This branch has not been deployed
3 participants
Summary
Compare production Standard scans with and without an advisory time allowance. The time-budget runtime support is already on
main; this PR contains evaluation code, fixtures, documentation, test/build wiring, and a file and directory identity repair found while validating the branch.Changes
Preserve exact integer file and directory identities while checking scan documents so distinct large identities cannot compare equal after numeric rounding. Keep the existing path checks and descriptor cleanup, with replacement and unchanged-file regression controls.
Run both arms through
CodexSecurity.run, including real prompt construction, canonical artifact validation, completion, and delegated-worker usage accounting. Only the treatment setsmaxTimeMinutes.Alternate arm order and use opaque per-arm workspaces outside evaluator artifacts. Check API-key and production budget prerequisites before either arm starts; this developer eval does not read or manage saved logins. Archive output and remove state even when client cleanup fails; retain scan, close, archive, and removal diagnostics. If state removal fails, save the failed attempt and stop before another arm can read retained state.
Capture actual prompts, evaluator/shared-helper/source/full-plugin/built-SDK hashes, package versions, the selected Codex executable paths and SHA-256 hashes (including
CODEX_CLI_PATHoverrides and npm launcher/native pairs), execution failures, monotonic elapsed time, and worker-inclusive token usage even when completion fails. Exclude generated reports from evaluator hashes.Grade a synthetic application with four vulnerability families and matching safe controls. Automatic credit requires a
root_controlorsinkcitation; retain other-role findings and control overlaps for manual adjudication rather than automatically crediting or labeling them false positives.Report completed-pair quality/time/token deltas alongside failures and missing usage. Reuse production types, canonical relative source paths, and existing fixture helpers instead of generalizing the unrelated secret-discovery runner.
Cover production completion and accounting with deterministic tests, wired into the existing source-eval CI path.
Give Windows diagnostic shards the existing 90-minute Windows job budget after both sharded modes reached their shorter job limit while tests were passing. Linux retains its 30-minute budget.
Testing
Local validation at
e4aa5926151a8b1616221e58c0097782fc858711, includingmainat5f521759e7c6ff8c3fbd5e81fd1a3567ac801b59, passed:git diff --check.These Linux checks used Node 22.23.3 and Bun 1.3.14, seed
12345. Existing dependencies and the generated plugin/native bundle were reused after matching dependency bytes, source/build inputs and artifact hashes. ShellCheck was unavailable and disabled. No fresh plugin/native build, full SDK suite, installed-package smoke run, or cross-platform pass is claimed; fresh exact-head hosted CI remains required.Earlier deterministic Standard evaluation and exact file/directory identity validation records carried by
bcae3d51c11db72777a3e13889761f5cd21a856dremain historical evidence for their named revisions. Their broader results are retained in the prior validation record and are not claimed as fresh execution for this main refresh.Risk and rollout
The file and directory identity repair changes internal filesystem metadata to exact integers without changing public settings, output, or replacement checks. The hosted failure log did not record raw file identities, so it does not prove numeric rounding caused that particular Windows failure.
The evaluator is opt-in and requires an existing
OPENAI_API_KEYorCODEX_API_KEY, with no effect on production CLI/SDK/plugin login or runtime defaults. Live model runs consume usage and have no enforced time or spending cap. Generated artifacts stay ignored and are not included in this PR.The small synthetic corpus is a smoke comparison, not evidence of real-world effectiveness. Source matching is heuristic; adjudicate findings and compare recall, false positives, completion, tokens, and elapsed time together. The arms are not spend-matched, and missing usage must remain unknown rather than zero.
Windows diagnostic shards can run for up to 90 minutes, matching the existing baseline and parallel modes. Individual test deadlines and assertions are unchanged. The main refresh aligns diagnostic shards with production partitions across all three modes.
Public disclosure review