Repository navigation
fix: preserve Daybreak settings across scans and remediation - #1740
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: 631dcb92d3
ℹ️ 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.
Thanks for preserving Daybreak consistently across scans and remediation. No blocking findings in this owner-level merge-safety and simplification review of 631dcb92d3a00f6ac016b9809acdaa220b1bf4cc. Explicit per-turn selections retain precedence, and standalone Deep Scan workers inherit the exact persisted parent turn.
Validated 59 focused SDK/CLI cases on the pinned Bun version, the parent-turn/preflight suite, builds, types, formatting, portable source checks, and package contents. A real Codex 0.163.0 localhost-only smoke test confirmed active Blue/Standard/Red/absent parent selections, resume persistence, and ephemeral fallback. Broader local SDK/MCP suites could not complete because of this environment’s fixture ownership/Git constraints; the matching-head CI run is green. The resolved catalog-failure comment matches native Codex behavior.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the follow-up fixes. Owner-level merge-safety re-review of b113857f3d5f7b315081528c33d3f42a2aa9ebce: no material findings. I reviewed all 15 changed files and retained prior coverage only for unchanged bytes. Stored-login settings remain per launch, native project and explicit overrides retain precedence, and inherited worker programs preserve explicit per-scan feature choices.
Validation: 59 focused SDK/CLI tests, the parent preflight module, 60 worker-setting launches, builds, types, formatting and portable checks passed. A real Codex 0.163.0 localhost probe confirmed effective features, program selection, resume persistence and ephemeral fallback; five native configuration-origin cases validated override selection.
Limits: 21 stored-login integration fixtures stop at an existing trusted-ancestor check, and the broader executor suite stops later at that fixture constraint; those suites are not counted as passing. The current CI run is incomplete and has a finalizer source-size failure also present in its main merge parent. No live account entitlements or Desktop UI were exercised.
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for the merge follow-up. Owner-level merge-safety re-review of 229d24f9435bd9566ea7a9564057f9c6b0b9ece1 against 5df9b7793a5bb3fea4443719b90c8e71dc5a2ab3: no actionable findings. All previously reviewed Daybreak files are unchanged; the additional fixture simplification preserves both path assertions and restores the source-size check.
The full finalizer suite passed (179 tests, 148 subtests; two platform skips), along with 52 focused SDK/CLI tests, parent-turn preflight, builds and portable checks. A fresh real Codex 0.163.0 localhost smoke passed effective feature/program selection, native request propagation, resume persistence and ephemeral fallback. The inherited SDK contract fixtures remain blocked locally by the existing trusted-owner environment restriction. Current CI has passed source-contract and package checks but is still running; no full CI pass or live account/Desktop test is claimed.
|
@codex review Please review integration head |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ 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". |
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-process authentication, configuration precedence, and worker-resume changes need human review backed by complete current-head regression results.
No open issues 🎉
What changed in this PR
Preserves Daybreak settings across CLI and SDK scans, remediation, and plugin Deep Scan workers while retaining configuration precedence and per-scan isolation.
Changes:
- Forward Daybreak settings through saved recipes, stored-login launches, and resumed workers.
- Resolve app-server access programs and inherit the exact parent turn’s selection.
- Expand regression coverage and document configuration options.
| File | Description |
|---|---|
| sdk/typescript/tests-ts/scan-resume.test.ts | Check resumed scan settings. |
| sdk/typescript/tests-ts/scan-comparison.test.ts | Check comparison feature precedence. |
| sdk/typescript/tests-ts/fixtures/stored-daybreak-workers.mjs | Capture native worker settings. |
| sdk/typescript/tests-ts/fixtures/skill-provider-auth.mjs | Mock native feature discovery. |
| sdk/typescript/tests-ts/fixtures/skill-native-config.mjs | Report effective Daybreak configuration. |
| sdk/typescript/tests-ts/fixtures/skill-auth.mjs | Support configuration and feature queries. |
| sdk/typescript/tests-ts/fixtures/inherited-codex-pipes.mjs | Handle configuration reads. |
| sdk/typescript/tests-ts/cli-skills.test.ts | Cover overrides and app-server propagation. |
| sdk/typescript/tests-ts/cli-skill-native-config.test.ts | Cover stored-login precedence and isolation. |
| sdk/typescript/tests-ts/cli-scan-patch-settings.test.ts | Verify patch settings across process boundaries. |
| sdk/typescript/tests-ts/cli-patch.test.ts | Check Daybreak override forwarding. |
| sdk/typescript/tests-ts/cli-patch-results.test.ts | Update protocol fixtures. |
| sdk/typescript/tests-ts/cli-daybreak.test.ts | Test access-program resolution. |
| sdk/typescript/tests-ts/cli-authentication.test.ts | Update expected authentication request sequence. |
| sdk/typescript/tests-ts/api-cyber-access.test.ts | Verify configuration and recipe preservation. |
| sdk/typescript/tests-ts/api-attribution-concurrency.test.ts | Check concurrent scan isolation. |
| sdk/typescript/src/scan-comparison.ts | Preserve the Daybreak feature gate. |
| sdk/typescript/src/cli.ts | Forward settings through remediation launches. |
| sdk/typescript/src/cli-daybreak.ts | Resolve advertised access programs. |
| sdk/typescript/src/api.ts | Retain Daybreak in preflight configuration. |
| sdk/typescript/scripts/package-dist-files.mjs | Package the new resolver module. |
| sdk/typescript/README.md | Document SDK configuration. |
| sdk/typescript/docs/cli.md | Explain opt-in behavior and precedence. |
| README.md | Link Daybreak usage guidance. |
| plugins/codex-security/mcp-app/tests/test_deep_scan_stdio_lifecycle.ts | Verify parent-program inheritance. |
| plugins/codex-security/mcp-app/tests/test_deep_scan_permission_profile_preflight.ts | Test exact-turn lookup and failures. |
| plugins/codex-security/mcp-app/tests/test_deep_scan_executor.ts | Integrate expanded worker settings tests. |
| plugins/codex-security/mcp-app/tests/support/fake-codex-source.ts | Simulate parent reads and transient failures. |
| plugins/codex-security/mcp-app/tests/deep_scan_worker_settings_cases.ts | Cover worker inheritance, retries, and resumes. |
| plugins/codex-security/mcp-app/src/deep-scan/permission-profile-preflight.ts | Read native origins and persisted parent programs. |
| plugins/codex-security/mcp-app/src/deep-scan/executor.ts | Snapshot and forward inherited programs. |
| plugins/codex-security/mcp-app/src/deep-scan/engine.ts | Pass parent context to workers. |
| plugins/codex-security/mcp-app/server.ts | Extract parent thread and turn identifiers. |
| docs/project-configuration.md | Document project-level Daybreak settings. |
Review effort: Balanced
alandelong-oai
left a comment
alandelong-oai
left a comment
There was a problem hiding this comment.
Thanks for preserving Daybreak across scans and remediation. Owner-level merge-safety and simplification re-review of 4135f2eb83de99b697cf97c8b7df059f1b069493 against 26e53dc18451c7a791f889fdd0ece85556282b9f: no actionable findings. The integration retains native configuration precedence, per-scan isolation, and exact parent-turn selection for fresh and resumed discovery/reducer workers.
Validated 63 focused SDK/CLI cases, parent-turn preflight, builds, types, formatting and portable checks. Real Codex 0.163.0 localhost probes passed program selection, native configuration precedence, resume persistence and ephemeral fallback. Broader local fixtures hit existing filesystem-environment restrictions; current-head CI passes all Linux SDK shards and the full MCP suite, including stored-login and worker lifecycle coverage. macOS/Windows SDK jobs remain in progress; no full CI or live entitlement check is claimed.
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Reviewed all 34 changed files at 4135f2eb83de99b697cf97c8b7df059f1b069493 against 26e53dc18451c7a791f889fdd0ece85556282b9f. One P2 compatibility issue is noted inline; no P0/P1 findings.
Validation: required portable checks, builds, types, formatting, package contents, parent-turn preflight, and synthetic settings checks passed. The broader local SDK run had 344 passes, 3 skips, and 151 failures, mostly from fixture ownership and Git discovery errors. Local worker lifecycle and installed-package smoke also failed. I checked this head's successful Linux SDK and full MCP CI logs, including stored-login and worker lifecycle coverage. The latest CI snapshot has 48 successful jobs and 2 skipped.
Approval candidate for interactive review with the P2 finding and these testing limits disclosed. Approval remains pending.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ 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 fixing the managed API-key feature selection. Owner-level merge-safety and simplification re-review of 228da532e65b9d96081f77840a051b7834203cb7: the effective-gate finding is addressed, with no remaining actionable findings. Both native flags are read across pages; raw configuration no longer controls automatic Standard selection.
Fresh validation passed 68 focused SDK/CLI cases, parent-turn preflight, builds, types, formatting and all portable checks. Actual Codex 0.163.0 localhost probes confirmed contradictory raw/effective API-gate settings and native pagination, plus parent-program inheritance and resume behavior. Prior green CI supports verified unchanged paths only; current-head CI remains in progress. Full local suites remain limited by existing fixture filesystem constraints; live managed policies and entitlements were not exercised.
Summary
Codex 0.163 defaults Daybreak to off, but scan configuration filtering, credential-home switches, and worker snapshots could discard the native settings. App-server commands also need an explicit per-turn access program, and standalone Deep Scan workers need the parent turn's effective selection. Preserve that behavior across CLI, SDK, and plugin execution.
Changes
daybreakand its feature settings through preflight, saved recipes, concurrent scans, comparison helpers, Deep Scan workers, remediation with stored credentials, andscan --patch.cli_daybreakandapi_key_cyber_access_programsfeatures, including managed overrides. Read both flags across paginated native responses rather than inferring the API-key gate from raw configuration. Save the toggle so native Codex resumes retain it.daybreak=true|falsethrough the existing--codexoption on validation and remediation commands, and document CLI, SDK, project, and plugin configuration.Testing
build:ci, the generated source-compatibility checker, and its nine tests. The earlier local dependency and bundled-native prerequisites are now resolved.Risk and rollout
Daybreak remains opt-in and requires compatible access. No new CLI flag or dependency upgrade is introduced. Explicit per-turn programs retain precedence, and CLI/SDK scans preserve their isolated settings.
Standalone plugin Deep Scan inherits the active program from a persistent Codex session, so a desktop or interactive selection reaches its workers without a separate configuration change. Hosts without a persisted parent turn, including ephemeral sessions, retain runner configuration behavior.
Public disclosure review