fix(deep): resume without historical worker prompts - #900
mldangelo-oai wants to merge 8 commits into
Conversation
|
@codex review Please review the current head, |
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. |
|
Codex Review: Didn't find any major issues. Bravo. 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.
🟡 Changes recommended
Completed-reducer recovery lacks regression coverage, and the coordinator retains an unused import.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Removes unused Deep Scan scheduler history so restarts rely only on durable result artifacts.
Changes:
- Removes execution audits and prompt hashes/path tracking.
- Stops recovery from reading historical prompts.
- Tests recovery after deleting discovery prompts.
File summaries
| File | Description |
|---|---|
worker-runner.ts |
Removes unused execution evidence. |
coordinator.ts |
Removes scheduler audit and prompt reconstruction. |
test_deep_scan_coordinator.mjs |
Tests restart without discovery prompts. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
mldangelo-oai
changed the title
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
|
@codex review Please review the latest cleanup at |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. 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". |
Summary
Deep Scan resume can fail when historical worker prompts are missing even though accepted results remain intact. Recovery now reads the accepted artifacts and ordered merge inputs it needs to continue.
Changes
5f09d59a) removes 52 net production lines with no test changes.Testing
On
5f09d59a:cb039309..5f09d59areported no findings; a separate verifier checked the complete outputs and source.02c5137ffound no interacting changes. A clean merge-tree check preserves the exact cleanup diff. This is not an end-to-end comparison.Earlier qualification and preserved failures
The recovery regression set has eight expected failures and seven passing controls on its unchanged baseline; the implementation passes all 15 cases. The previous public head,
cb039309, passed CI including Linux, macOS, Windows and installed-package checks. Those results do not qualify the new cleanup head.The latest real-store integration test initially failed under the sandbox's filesystem ownership mapping, then passed on the normal host without changing product guards. Initial offline dependency installation failures are retained; the frozen-lockfile install passed using the existing package store. Native reviewers' attempted coordinator test commands failed before execution because their source-only checkouts lacked
esbuild; these attempts are not counted as passing tests.Risk and rollout
Accepted-artifact validation, persisted worker records, retries, scheduling and public results keep their existing owners. No schema or public CLI change. Local cleanup checks ran on Linux. Hosted checks and review must run on the new head after publication; this change does not merge or release the PR.
Public disclosure review