Skip to content

Add cooperative safe-boundary execution suspension - #67

Merged
Brian Krabach (bkrabach) merged 16 commits into
mainfrom
codex/maintenance-suspension-20261020
Oct 25, 2026
Merged

Brian Krabach (bkrabach) merged 16 commits into
mainfrom
codex/maintenance-suspension-20261020

Conversation

@bkrabach

@bkrabach Brian Krabach (bkrabach) commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose and contract

Add an opt-in public safe-boundary suspension contract for explicit maintenance pause/resume. Already-started model/tool calls settle; versioned frames retain prepared requests, ordered results, accounting, and goal/finalization progress. Resume validates current history/configuration and a host-owned one-time admission callback. Cooperating tools continue through their original resume_execution owner, never an execute replay. No coroutine serialization, new scheduler, automatic crash replay, or cancellation-as-safe-pause.

Accepted main e04bba7e / #68 scoped dispatch and ordinary nested inference are preserved. The host still owns durable persistence, current permission/budget/Stop admission, session binding, and unknown-outcome recovery.

Current successor: 395594ad9da191289ff75eb126bcf550a2b93694

The response to the two suspension-path review findings records resume-owner preflight/settled sibling outcomes and prepared true-stream retry checkpointing. Eight new public-entry regressions fail on the prior production head and pass on this successor. An additional mixed suspended/unknown outcome probe passes on unchanged successor code. Exact-head CI 40025223645 passes on Python 3.11–3.14; Python 3.11 reports 552 passed / one skipped. Intermediate failures and the limits of direct public-engine tests with installed native Core remain explicit in that response. This delta awaits independent re-review and normal maintainer approval.

Previously qualified lock successor: 1c2c28343816f002fdcfb5393fad06df1bd92b84

Addresses the P2 review on ccd958: resumed calls previously selected separate locks for per-call ResumingTool objects even when they invoked the same mounted tool.

Only dispatch.py and tests/test_approved_dispatch.py change from ccd958c. Lock selection now unwraps the engine's known GuardedTool/ResumingTool adapters to the mounted identity. Execution retains each original per-call adapter and continuation. Exclusions, authority callbacks, ordinary/nested concurrency, Stop/budget paths, workflow, and dependency inputs are unchanged.

For a ResumingTool(GuardedTool(x)) chain, the inner guard's existing __getattr__ forwards resume_execution directly to x; it does not invoke GuardedTool.execute or acquire the execution lock again. The identity test covers that chain and rewrapping; actual resumed-batch execution tests cover the ordinary mounted-tool path. No claim of arbitrary custom-wrapper support is made.

New discriminating checks

  • Red commit bd042fe (production exactly ccd958c): five failures, no collection errors. Four real-Core _settle_tool_batch cases reach peak 2: resumed/resumed and resumed/not-started, each under one name or two aliases for the same instance. The fifth shows distinct locks for adapter chains.
  • Final isolated request upstream-resumed-lock-final-cycle12-20261022: 79 passed, zero skipped across approved dispatch, execution suspension, cancellation sibling preservation, and dispatch context. All four cases now have peak 1, original ordered unique result IDs, exact continuation use, and no execute replay of started calls. Delegation exclusions and rewrapping remain covered.
  • The intermediate run is retained: 77 passed / two failed because the test incorrectly required lock-acquisition order. Only that assertion changed to compare exact continuation multiplicity; published result order remains explicitly asserted. Concurrent approvals may admit either call first.
  • Four isolated JUnit warnings concern record_property under xunit2; counter properties were retained and parsed. No hidden skip or weakened assertion for concurrency/results.
  • Exact-head CI run 39831026757: Python 3.11, 3.12, 3.13, 3.14 all pass. Python 3.11 reports 544 passed / one skipped.

Both requested independent second-opinion delta reviews returned on frozen 1c2c283: openai/gpt-6-astra (e17eb38b1c924b81_self) and fable/claude-fable-5-1 (2a86a756f3b94b83_self). Both found the narrow correction reviewable subject to exact-head CI, now observed. They reviewed the same source/evidence brief, not an exhaustive repository audit. Normal independent maintainer approval remains required; these reviews are not merge approval or an admin exception.

Earlier evidence — retained, not relabeled

On ccd958c, isolated request upstream-review-guards-cycle7-20261022 passed 156 Streaming checks and 105 paired Live checks / one existing compaction skip, with Live candidate 7c5e2d6c and Foundation 5265d6cc. Guest-owned offline/no-deps wheel installation and outside-source byte verification passed. CI 39826661275 was green for that earlier head (Python 3.11: 539 passed / one skipped).

Also on unchanged ccd958c, upstream-reviewed-model-held-probe-cycle9-20261022 passed four real-Core probes for serial/parallel nested-model holds with normal/raising error observers. Conversation-visible held/non-retryable results, zero provider calls, and one prior settled effect were verified. Main closed that conditional observer concern; it was not repeated as a new gate. Cycle8's native-method source-export failure and its passive cleanup-binding diagnostic caveat remain retained.

The initial PR description reported 218 earlier focused/adjacent checks and composed application tests. That historical report is not requalified or substituted for the exact-head evidence above.

Remaining boundaries

This is ready for normal maintainer review, not merged/released/adopted. Main's source re-review closes the P2 on exact 1c2c283; it is a source assessment, not an approving maintainer review. One normal review request is pending with microsoft/octo-made-amplifier-maintainers. The repository's required independent approval remains outstanding; no admin exception or bypass is requested.

Live #16's corrected local 7c5e2d6c stays unpublished until Streaming is actually accepted/merged; its stale remote dependency-failure CI is not retried early. Foundation #447 5265d6cc is unchanged and ready, with its existing maintainer request and prior reviews retained.

Main owns Unified #945 integration and real-host competing claims, Stop during admission, crash after claim consumption, exact session binding, and recorded child-result recovery after completion-hook failure. Module fixtures do not prove those host guarantees. Unsupported opaque execution and unknown outcomes remain explicit; no live-provider or remote-cancellation guarantee is claimed.

@bkrabach Brian Krabach (bkrabach) left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact successor ccd958c against accepted main e04bba7. One correction remains before source clearance.

[P2] Preserve the mounted tool's execution lock on resume. In _settle_tool_batch (lines 3570–3572), every continued call receives a distinct ResumingTool adapter. With approved dispatch enabled, _execute_tool_only passes each adapter to wrap_tools, which keys the lock by id(raw) (dispatch.py line 563) and unwraps only GuardedTool. Two resumed calls to the same mounted tool therefore receive different locks; a resumed call and a not-started sibling also differ. They all invoke the same underlying mounted instance, losing the serialization ordinary calls receive. This is a source-level call-path finding, not a newly executed reproduction.

Please retain per-call continuations and current authority checks while selecting the lock from the trusted mounted execution identity. A focused regression should cover two resumed calls and a resumed/not-started pair, requiring peak execution of one, ordered original results, correct continuations, and no execute replay of an already-started call. Preserve the existing delegation exclusions and ordinary/nested concurrency contracts.

Accepted #68 authority behavior and the workflow/dependency inputs remain preserved. The retained four real-Core probes cover the conditional provider:error observer concern, so that is not an additional finding. Current Python 3.11–3.14 CI is green. Please also refresh the description with the exact successor's 156 focused Streaming checks, paired Live checks and four Core probes, keeping older evidence and downstream host-acceptance limits distinct. Normal maintainer approval remains separate; no bypass or production acceptance is implied.

@bkrabach Brian Krabach (bkrabach) left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Focused successor source review at 1c2c283: the P2 finding is addressed; no remaining actionable finding in this delta.

Lock identity now unwraps only the engine-owned GuardedTool/ResumingTool adapters to the mounted instance, while actual execution retains each per-call continuation adapter. The nested guard chain forwards resume_execution without acquiring the same execution lock twice. Existing delegation exclusions, current authority checks and accepted #68 behavior remain unchanged.

The new real-Core batch tests distinguish the original peak-two defect from peak-one serialization for resumed/resumed and resumed/not-started calls, including aliases. They separately assert original ordered result IDs, exact continuation multiplicity and no execute replay. Removing the unsupported execution-start-order expectation is correct: result publication order remains asserted. Retained evidence reports 79 focused passes; the exact-head Python 3.11–3.14 matrix and CLA are green. This review is source re-review, not a new independent runtime run.

Ready for normal maintainer review. This comment is not an approving maintainer review or authorization to bypass protection. Live's accepted-Streaming update and Unified's durable host admission/result recovery remain separate future integration work; historical qualification retains its original identities.

@bkrabach
Brian Krabach (bkrabach) marked this pull request as ready for review October 22, 2026 19:04
@bkrabach
Brian Krabach (bkrabach) requested a review from a team October 22, 2026 19:04
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Review follow-up: two suspension-path regression requests

Reviewed head: 1c2c28343816f002fdcfb5393fad06df1bd92b84. This handoff concerns the cumulative safe-boundary suspension change, not a rejection of the latest mounted-tool lock-identity correction.

Independent Astra (openai/gpt-6-astra) and Fable (fable/claude-fable-5-1) reviews of the same frozen source/evidence packet both identified the following two acceptance gaps. These were brief-only model reviews; they are not eligible maintainer approvals. Exact-head CI run 39831026757 is green. The findings below concern cases not covered by the inspected regressions.

1. Exceptional resume setup can let a sibling outlive batch failure

Source:

ResumingTool(...) is constructed inside a scheduled dispatch task, before the ordinary tool exception-to-result boundary. Its constructor raises ValueError if the mounted completion owner lacks callable resume_execution. The dispatch task catches only ToolExecutionSuspended; the outer gather handles CancelledError, not an ordinary setup exception. Another sibling may already be running when the batch exits.

A bounded stdlib probe compiled the exact source method with fake tools: the first resumed sibling waited on an Event; the second had an unavailable resume owner. Observed:

exception: Tool does not support explicit execution resumption
sibling_started: true
sibling_finished_at_batch_exit: false
sibling_pending: true

The harness then released and awaited the sibling. This is method-level control-flow evidence, not an installed-Core/public-entry reproduction.

Requested regression: restore a two-call settling_tools frame with a valid blocking resumable tool first and a present mounted tool without resume_execution second. Exercise the public resume entry. Either establish that the frame is rejected before any sibling starts, or show that exceptional batch exit settles every started sibling and retains its known outcome. No fallback to execute(arguments) and no automatic reuse of a consumed/uncertain admission claim.

Suggested minimal direction: validate all needed resume owners before scheduling any sibling, and ensure unexpected setup/dispatch failures cannot leave started tasks unobserved. Do not equate local task cancellation with remote rollback or safe replay. Please choose the implementation consistent with the started-work settlement contract.

2. True-stream overflow recovery lacks the replacement-send suspension boundary

Source:

After awaiting recover_overflow(error), this wrapper replaces the request and starts another stream without checking the suspension owner. The non-streaming and finalization overflow paths already checkpoint prepared replacement requests. The recovery helper checks cancellation, not suspension.

A bounded exact-method probe used a fake first stream raising ContextLengthError before any chunk. Fake awaited recovery set the hold flag before returning the replacement request. Observed:

first send: hold_at_send=false
second send: hold_at_send=true

No real provider or network was used.

Requested regression: request hold during overflow recovery/preflight/rate-limit wait; assert that no replacement provider send occurs until a durable checkpoint and authorized resume. Retain the exact recovered request, the spent recovery allowance and call accounting; do not repeat preparation on resume. Also retain the existing rule that recovery is forbidden after the first provider chunk.

Scope and limits

These are opt-in suspension-path findings, not demonstrated default execution regressions. No assertion is made that ordinary transcript-session resume implements frame resume, or that current hosts have enabled suspension. Native-Core exception/frame transport, crash-safe admission, competing claims and live-provider behavior remain separate qualification boundaries.

The lock-wait admission definition, Stop-versus-checkpoint priority and queued-steering retention were not established as blocking defects by these reviews; they should not be conflated with the two focused requests above. No source changes or new runtime qualification were made in this review.

@bkrabach

Copy link
Copy Markdown
Collaborator Author

Response to the two suspension-path findings

Addressed in successor 395594ad9da191289ff75eb126bcf550a2b93694 (tree 0c7d1baa007e13f65478d97804af8528da4fde42), based on reviewed 1c2c283. The accepted mounted-tool lock correction and #68 dispatch authority code are unchanged.

1. Exceptional resume setup and sibling settlement

All required ResumingTool adapters are now constructed before any resumed-batch task is scheduled. A missing callable completion owner therefore rejects before any sibling starts. The existing host admission order remains unchanged: a claim already consumed before this validation is not released, reused, or retried automatically.

Unexpected ordinary exceptions that escape resumed dispatch are converted to an explicit unknown, non-retryable result. gather can then observe every other started sibling; known results retain their original call IDs and order. The full result envelope is preserved in conversation-visible serialization. No path falls back to execute for an already-started call. CancelledError and other BaseException control paths remain distinct, not mislabeled as ordinary failures or rollback.

New public-entry tests use a real Core coordinator, create an authentic settling_tools checkpoint through execute, then call a fresh engine's public resume:

  • Missing second completion owner: zero resumed sibling scheduling, no execute replay, and a later explicit attempt with the consumed frame is refused.
  • Unexpected dispatch failure while the first sibling is blocked: the first sibling settles; its known result and the second's unknown result are retained in original order.
  • An additional task-local probe on unchanged successor code passes through another suspension and resume. The unknown sibling result survives byte-for-byte and is never dispatched again; the remaining sibling completes under its original identity.

This preserves the conservative unknown-outcome classification for any first-time not_started call inside that resumed batch too. A dispatch exception alone cannot prove that no effect started, so it is not labeled none_started.

2. True-stream overflow recovery and hold admission

The streaming retry wrapper now invokes the existing prepared-request suspension boundary after recovery/preflight/rate-limit waits and before counting or sending the replacement. The checkpoint retains the exact recovered request, overflow_retried=true, and existing accounting. Authorized resume sends that request without rerunning preparation; it cannot acquire a second recovery allowance. The first-provider-chunk guard—including a non-text chunk—remains unchanged.

Six new public-entry cases cover hold during recovery, preflight, and rate-limit waiting, each with successful or failing replacement sends. They verify one send/call before checkpoint, explicit refusal while held, two total sends/calls after authorized resume, exact request equality, no repeated hook/context/budget/recovery/rate preparation, and closed iterators. A second overflow propagates instead of triggering a third send.

Evidence and limits

  • Test-first commit 85ded2e: all eight targeted regressions fail on the old production source; no collection errors. These are real-Core/public-engine tests, unlike the earlier exact-method diagnostic probes.
  • Initial repair 3680a3e: 99 passed / one failed across the affected dispatch, suspension, overflow and cancellation sets. The remaining failure exposed loss of unknown-outcome fields in Core's convenience serializer. 395594ad changes only that serialization line; unchanged assertions then pass 8/8.
  • Mixed-outcome follow-up: 1/1 passed on unchanged 395594ad; its first task-local attempt failed collection due to fixture import setup and is retained as a failed harness run, not a product pass.
  • Exact-head CI run 40025223645: Python 3.11–3.14 all pass; Python 3.11 reports 552 passed / one skipped. No workflow/dependency changes or manual CI retries.
  • Independent second-opinion delta reviews returned from openai/gpt-6-astra (8b40901145134cc2_self) and fable/claude-fable-5-1 (9009305212784580_self). Both found the requested cases repaired on the supplied evidence; exact-head CI and the mixed-outcome probe close their principal coverage requests. Remaining hardening suggestions and scope limits are retained, not treated as maintainer approvals.

The new tests directly call Python engine public methods with installed native Core 2.0.1, real hooks/context, and offline fake providers/tools. They do not establish native-kernel frame/exception transport, crash-safe or competing host claims, production Stop priority, or remote rollback. Main's Unified #945 host qualification remains separate. No live provider calls, host update, protection change, merge, or admin bypass occurred. All new task-owned environments were removed with retained evidence; historical failures and cleanup caveats remain preserved.

Ready for independent re-review of these two changes. Normal maintainer approval remains required. Live #16's unpublished candidate and Foundation #447 are unchanged; Live's accepted-Streaming dependency step still waits for actual Streaming merge.

@bkrabach

Copy link
Copy Markdown
Collaborator Author

Readiness's bounded follow-up review of exact 395594ad9da191289ff75eb126bcf550a2b93694 against previously reviewed 1c2c28343816f002fdcfb5393fad06df1bd92b84: no additional blocker found in the two requested repair paths. This is source/retained-CI review, not formal maintainer approval or host-integration acceptance.

I read the complete three-file delta, the production settlement/retry context and all eight new public-entry cases. Required resuming owners are constructed before sibling tasks are scheduled. Ordinary resumed-dispatch errors retain an unknown/non-retryable result so gather can settle other started siblings; cancellation/control exceptions remain distinct. The recovered stream checkpoint occurs after recovery and before retry accounting/send, with overflow_retried preserved so an explicit resume does not gain another allowance. The tests retain consumed-claim refusal, original call order, exact prepared request, unchanged preparation counts and iterator closure.

Independently read exact-head CI 40025223645, attempt 1: all four Python 3.11–3.14 jobs succeeded. The Python 3.11 raw log explicitly contains all eight new case IDs as PASS and 552 passed / 1 skipped. No test or CI replay was performed for this review. The task-local mixed-outcome probe and cleanup summaries remain owner-attributed here; their raw private packet was not independently reread by this reviewer.

Keep native-kernel transport, crash-safe/competing claims, real Stop and Unified #945 acceptance separate. The historical failures remain failures. Normal upstream maintainer approval/order still applies; this note grants no merge, downstream publication or live-adoption authority. No duplicate review request is being made.

Handoff key: readiness-streaming67-395594-source-ci-review-v1

@bkrabach

Copy link
Copy Markdown
Collaborator Author

Bounded re-review: 395594ad vs 1c2c283

Both requests in #issuecomment-6373144951 are addressed at source level. Resume adapters validate before sibling tasks are scheduled; ordinary exceptions escaping resumed dispatch preserve the full unknown/non-retryable envelope while started siblings settle. True-stream retry checkpoints the exact recovered request after waits and before retry counting or sending; overflow_retried survives restore without granting another allowance.

I independently inspected exact-head CI run 40025223645: Python 3.11–3.14 all succeeded. The raw Python 3.11 log shows all eight new case IDs passing, with 552 passed / 1 skipped. Astra (openai/gpt-6-astra, session 4eff98da80844a41_self) and Fable (fable/claude-fable-5-1, session f9380603d9e14e8f_self) received the same frozen source/CI packet and found no new blocker in this repair. These were brief-only reviews, not formal approval.

Optional hardening: add a public-entry unknown-result case through another suspension/resume, asserting the byte-identical canonical envelope, original IDs, and no redispatch. The task-local probe is author-reported and separate. Keep the repeated prepared-hold test optional too, not as a reopened blocker. The independent installed-wheel runtime attempt was blocked by host storage; the first eight-case invocation also had a missing-temp-parent fixture error, so no runtime pass is claimed.

Native-kernel exception/frame transport, crash-safe competing claims, Stop/budget/session binding, and host adoption remain separate. No merge, admin bypass, or source change was performed.

@bkrabach
Brian Krabach (bkrabach) merged commit 941806a into main Oct 25, 2026
5 checks passed
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Admin merge completed — October 25

Squash-merged reviewed head 395594ad9da191289ff75eb126bcf550a2b93694 as 941806a4a42c84c7c239fd8e0f8ef1260f1ec32f on October 25, 2026, at 8:18:42 a.m. PDT, under the user's explicit admin-merge authorization.

  • Protection audit: GitHub rule-suite 4711768928 records bypass for ruleset 15834159: the normal one-approval requirement remained outstanding. Linear-history, non-fast-forward and deletion rules passed. No approval was fabricated; no rules or settings changed.
  • Commit verification: sole parent is accepted base e04bba7e85bff9836bfc48cd8ac1db676f4f5fab. The entire merged tree is 0c7d1baa007e13f65478d97804af8528da4fde42, identical to the reviewed head. The cumulative change is 10 files, +3,247/−1,650, not merely the final three-file repair delta. The supplied commit body and Amplifier footer match exactly.
  • Accepted qualification: both focused findings are closed. Reviewed-head CI 40025223645 passed Python 3.11–3.14 and CLA passed. Retained independent offline installed-wheel qualification reports 8/8 focused and 554/554 full-suite checks with native Core 2.0.2; the separate second-suspension unknown-result probe passed 1/1. These runtime checks were not rerun by this merge lifecycle. Astra (openai/gpt-6-astra, 833d9458a7e840ad_self) and Fable (fable/claude-fable-5-1, 5c5559c0b9a94024_self) returned advisory READY recommendations, not maintainer approvals.
  • Post-merge CI: passed. One snapshot at 8:19:33 a.m. PDT inspected push run 40206331285, pinned to 941806a4a42c84c7c239fd8e0f8ef1260f1ec32f: Python 3.11 PASS, 3.12 PASS, 3.13 PASS, 3.14 PASS. Commit check-runs agree; there are no legacy status contexts. No polling or CI retry was performed.
  • Limits unchanged: upstream acceptance is not host adoption. Native-kernel frame/exception transport, durable competing claims, crash recovery, current Stop/permission/budget/session binding and host child-result recovery remain separate. No live-provider, remote-rollback or exactly-once guarantee is claimed. Committing the second-suspension unknown-result regression remains optional hardening. No downstream PR or host deployment was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants