Repository navigation
fix(deep-scan): recover publication after a lost reply - #938
mldangelo-oai wants to merge 85 commits into
Conversation
…-stale-publication
|
@codex review Please review exact 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. 🎉 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". |
![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: f9aae3378e
ℹ️ 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: e021cf4c02
ℹ️ 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: 832a3c6694
ℹ️ 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
removed
the
area:deep-scan
# Conflicts: # plugins/codex-security/mcp-app/src/artifact-scan-draft.ts
|
@codex review |
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-language recovery, concurrency fencing, and platform-specific atomic filesystem behavior warrant final human review.
0 open findings
What changed in this PR
Adds safe Deep Scan publication recovery when canonical results are saved but the child-process response is lost.
Changes:
- Records and validates publication acknowledgments before replay.
- Preserves ownership, digest, warnings, checkpoints, and artifact bytes.
- Distinguishes optional receipt I/O failures from unsafe-path errors across platforms.
| File | Description |
|---|---|
plugins/codex-security/tests/test_deep_scan_publication_authority.py |
Tests replay, receipt failures, and path safety. |
plugins/codex-security/scripts/workbench_saved_results.py |
Creates and validates publication acknowledgments. |
plugins/codex-security/scripts/windows_scan_local_files.py |
Separates Windows path-integrity errors from I/O failures. |
plugins/codex-security/scripts/finalize_scan_contract.py |
Classifies scan-local storage failures. |
plugins/codex-security/mcp-app/tests/test_deep_scan_publication_integration.ts |
Exercises end-to-end recovery and ownership fencing. |
plugins/codex-security/mcp-app/tests/clock_workbench.py |
Injects synthetic receipt-write failures. |
plugins/codex-security/mcp-app/src/artifact-scan-draft.ts |
Retries acknowledged publications once and cleans receipts. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dbb1479dd
ℹ️ 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
|
@codex review |
|
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". |
zcrab-oai
left a comment
zcrab-oai
left a comment
There was a problem hiding this comment.
Reviewed the changes since the previous reviewed head in two passes, using exact source at 95f1160845c5 and prior review evidence. Unchanged portions of the large branch were not exhaustively re-audited.
Approved. No new actionable findings in the reviewed scope.
Validation: CI-helper TypeScript compilation and plugin bundling passed locally; native binaries came from CI with matching compiled source inputs. The full cross-platform suite was not run locally; Windows execution remains unverified here.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| descriptor = next_directory_handle | ||
| next_directory_handle -= 1 | ||
| else: | ||
| descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) |
| def fail_validation(*args): | ||
| raise OSError("Synthetic validation failure") | ||
|
|
||
| def fail_write(scan_dir, filename, contents): |
Summary
Recover the saved result when publication succeeds but its response is lost.
Depends on #916.
Changes
Testing
Validation on
95f1160845c5: 197 MCP tests and 141 Python tests passed, with five Windows-only tests skipped locally. Eighteen late path-integrity regressions failed against the previous Windows backend and now pass through the actual publication writer; 42 ordinary I/O and path controls remain green. Linux runs emulate Win32 API errors and exercise real POSIX path replacement; native Windows remains for hosted CI. Portable checks, types, formatting, three native reviews and independent verification passed. Older full-suite results are not claimed for this revision.Risk and rollout
Stacked on #916, which must land first. A missing acknowledgment still prevents recovery after response loss. The change adds no public command, flag, schema, default, or workflow activation.
Public disclosure review