feat(deep): prepare compatible readers for durable scan recovery - #919
mldangelo-oai wants to merge 134 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| from typing import Any | ||
|
|
||
| import pytest | ||
| import workbench_test_support |
Read committed execution settings and validate saved publication bindings before recovery. Preserve historical unsealed budget coverage through the existing finalizer, and reconcile the reader with current main.
Preserve upstream changes and saved-result recovery, including partial coverage and per-model cost attribution.
| selection_before = json.loads(run_before.pop("finalization_input_json")) | ||
| selection_after = json.loads(run_after.pop("finalization_input_json")) | ||
| assert run_after == run_before | ||
| assert selection_after.pop("publicationSha256") != selection_before.pop("publicationSha256") |
| ) | ||
| assert result.returncode == 0, result.stderr | ||
| private = json.loads(result.stdout) | ||
| assert private["deepScan"].pop("executionSettings") == (settings if bound else None) |
|
@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. |
Use the registered target contract when authoring the saved directory snapshot. Assert its format and preservation through budget-exhausted completion.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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. Already looking forward to the next diff. 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: aff95e7127
ℹ️ 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".
| session.responseIds.add(responseId); | ||
| const cumulative = tokenUsage(payload["thread_token_usage"]); | ||
| if (cumulative) | ||
| session.expectedResponseTokens = Math.max( | ||
| session.expectedResponseTokens, | ||
| cumulative.total_tokens, | ||
| ); | ||
| if (!session.responseUsageObserved) { | ||
| // Exact receipts include compaction and survive counter resets. Keep the | ||
| // legacy counter as an independent lower bound, never add it to receipts. | ||
| session.responseUsageObserved = true; | ||
| session.usage = null; | ||
| session.modelUsage.clear(); | ||
| } | ||
| session.responseTokens += usage.total_tokens; |
There was a problem hiding this comment.
When a shared owner session contains a token_usage_record from another turn or after the scan's completion—particularly when an older attributed turn was measured with legacy token_count events—this code records the response, clears the attributed usage/model totals, and updates cumulative receipt counters before isAttributedScanEvent rejects it below. The scan's known usage can consequently become unpriced or falsely partial, preventing reliable enforcement of a user-requested --max-cost; perform the attribution check before mutating receipt state (and apply the same ordering in the Python reader).
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L25
Useful? React with 👍 / 👎.
1 participant
Summary
Prepare compatible readers for saved Deep Scan execution and publication state before enabling the corresponding writers. Recovery preserves accepted evidence, unresolved coverage and the result conversation. Supported historical unsealed budget results complete through the existing finalizer; unsupported recovery is rejected before saved state changes.
Changes
New scans retain the existing workflow. New attempt, frozen-checkpoint and finalization writer activation remains a separate change. The reader corrections introduce no new public command, flag or environment variable.
Testing
Reviewed head:
aff95e71270c5c73fb928398dbf90e30e0288828, treeb0f2f20e6b9d8d42d1e0edaf6f624886406e6316. The latest update adds 13 test lines; production files are unchanged from8c5b632. Two fresh native review passes and an independent verifier found no actionable issues in this update; the exact-head receipt check passed. The publication review checked bundle integrity, ancestry, every changed-path identity, main incorporation, the complete public diff and source-bound qualification records. It did not repeat product suites.Previously recorded qualification on
8c5b632on macOS arm64 (its production files are unchanged in the current head):build:ci, plugin source compatibility, and the source-compatibility tests. Plugin build and all seven alias-build tests also passed.Linux qualification ran on the merge parent's tree,
30c869f8877abd7cfb44da158acfe7d8e684af24. All shipping SDK, MCP, native and dependency files match this head. Later test-only changes remove the unused test string and correct the snapshot-recovery fixture. Portable checks and full MCP 73/73 passed. Full Python passed 1339 / 8 skipped, with 109 passing subtests, under ordinary import defaults. Its initial inherited-environment failure remains recorded as 1338 passed / 8 skipped / 1 failed. Source, installed and detached recovery/replay and rejection controls passed; their overlapping counts are not summed.Current-head Linux SDK qualification used Node 24.15.0, pnpm 11.25.0, Bun 1.3.14, Python 3.12.13 and Git 2.43.0. Both
pnpm run test --seed 12345and ordinarypnpm run testpassed 3121 tests / 50 skipped / 0 failed, with 70412 assertions each. The default run recorded seed 2117022814. Builds, types, format and the focused regression passed. All 511 members of the retained reader package match the current build; the package retains its original source identity.The exact preceding head fails the snapshot-binding fixture. On macOS arm64, that parent case failed and the corrected full workbench module passed 35 tests / 265 assertions with seed 12345. This used the existing byte-verified bundled runtime; it is not a new package or full macOS SDK run. No production guard, assertion or timeout was weakened.
Remaining gates: current-head CI and final Windows qualification remain open. Earlier reader SDK failures, including the separate malformed-Git-config case, remain historical failures. The preceding head's Windows Node 24 package job timed out in
npm install; its deeper cause remains unknown. The original intermittent Mac MCP timeout and broader Mac SDK failures are not erased by later focused passes. This update does not claim those platform gates are complete.Risk and rollout
Release a qualified compatible reader before enabling new durable writers, including writers affecting historical workflows. Rollback must use a release that understands state already written. Supported selected-result completion does not imply support for newer discovery or reducer execution.
This reader adds migrations 45–48 and 51 without changing existing migration identities. Coordinate the separate writer's migration 44 and the two open proposals containing different migration 42 changes. The open helper-entrypoint refactor also needs composition review before landing together.
Main
7b77956c3067861559d31a3f3f791209b403643dis incorporated. The final comparison against main covers 114 files; this update from8c5b632changes only the snapshot-recovery test fixture. The rolling scheduler, result conversation, retry ownership and intentional Standard/Deep permission differences remain. This PR does not establish release readiness or production equivalence.Public disclosure review