You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Qualified AgentKit and Foundry harness-v2 runtimes can wait for human approval on the original MCP tool call and receive its actual result. An inventory lookup can finish while a proposed work order stays unexecuted until an authorized reviewer approves it.
The controller stores the executable request in an immutable, Task-owned Secret and binds the decision to the Task attempt, prompt, runtime, policy, and tool definition. Execution claims a durable effect record once. A saved receipt can replay; a started action with an uncertain outcome cannot execute again. Decline, expiry, cancellation, and stale authority return explicit outcomes without automatically resubmitting the prompt.
Bound review to 600 seconds, execution to 240 seconds, and the enclosing MCP call to 900 seconds, shortened by Task and prompt authority.
Require an explicitly qualified runtime profile and a default-off brokered approval capability. Native shell and file permissions remain separate.
Reuse the approval API and panel with safe input previews, deadlines, execution outcomes, reviewer authorization, and encoded approval IDs.
Preserve approved, unclaimed calls through temporary authority or storage read failures. Reject verified authority changes; cancel and preserve an unknown outcome if authority is lost after execution starts.
Recover pending and in-flight calls after controller or supervisor loss using original admission witnesses, retained runtime authority, durable receipts, and authenticated retirement proofs.
Add configuration guidance, a counted tool simulator, and a dedicated AgentKit/Foundry approval lifecycle E2E gate.
Split
This PR was rebuilt as the top of a stack. The unrelated work that rode along originally now lands separately:
feat/foundry-boot-recovery: Foundry boot retirement through the broker ledger.
test/e2e-cleanup-evidence (independent of this stack): E2E cleanup evidence harness.
The pre-split history is preserved on human-approval-v2-pre-split. Against that branch, the only code change here is the second commit: the CallBinding raw-field upgrade and the raw call-ID recovery identity branch are removed because no release ever wrote those forms, and five small helpers are shared between the broker and recovery paths. Everything else is byte-identical to the live-validated tree.
Remaining simplification follow-ups: the ExternalEffect Task-UID discovery label (written but only read by fallbacks for records without the spec field), the two denial receipt shapes, the continueApprovedCall shim, the 15-file approval test layout, and the acceptance runner living under examples/.
Validation
At the pre-split revision 2864ff69b: make lint-fix, make test, focused race/regression tests, and 67 fixture tests. The dedicated approval CI gate passed approval, decline, cancellation, reviewer authorization, full 600-second expiry, concurrent progress, supervisor loss, and pending/in-flight controller loss on both adapters. Both general E2E lanes passed all 81 selected specs.
All nine core images were deployed to sertac-aks in harness-v2 mode. Ten browser checks and the live approval scenarios passed on AgentKit and a real Azure kind: hosted agent, including approval holds over 125 seconds and the full expiry window. Live recovery passed two supervisor crashes and two controller crashes. All 29 acceptance Tasks, seven UI Tasks, and both runtime registrations completed normal cleanup.
Six historical crash Tasks from pre-fix validation remain retained because their original admission and deletion evidence is missing.
Current approval E2E dependencies: AgentKit 490bb6d6, merged through PR #27; Foundry runtime 9f994c30. The Foundry revision adds the separate response-acknowledgement cancellation fix on top of 32cd33f5. The gate's cancellation and recovery checks validate this combined dependency set; they do not qualify 32cd33f5 alone.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The security-sensitive, distributed exact-once workflow warrants final human review, and encoded approval IDs still lack API-level regression coverage.
Review tier: Balanced (auto) Findings: None
Note
Copilot is running an experiment and ran this review at Balanced.
This new decoding path has no API test with an approval ID that actually requires path encoding. Plain IDs do not verify that Fiber preserves the encoded segment and that it is unescaped exactly once. Add a handler test using characters such as /, %, or spaces and assert the decision resolves the original approval record.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The security-sensitive, exactly-once approval and crash-recovery protocol spans 135 files and requires final human validation despite extensive automated coverage.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The security-sensitive approval, recovery, and exactly-once execution changes span 135 files and warrant final human validation despite extensive automated coverage.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The exactly-once execution and recovery changes span 135 files and external runtime contracts, so final human review is warranted despite no newly verified defects.
Qualified AgentKit and Foundry harness-v2 runtimes can wait for human
approval on the original MCP tool call and receive its actual result.
The controller stores the executable request in an immutable, Task-owned
Secret and binds the decision to the Task attempt, prompt, runtime,
policy, and tool definition. Execution claims a durable effect record
once. A saved receipt can replay; a started action with an uncertain
outcome cannot execute again. Decline, expiry, cancellation, and stale
authority return explicit outcomes without resubmitting the prompt.
- Bound review to 600 seconds, execution to 240 seconds, and the
enclosing MCP call to 900 seconds, shortened by Task and prompt
authority.
- Require an explicitly qualified runtime profile and a default-off
brokered approval capability. Native permissions stay separate.
- Reuse the approval API and panel with safe input previews, deadlines,
execution outcomes, reviewer authorization, and encoded approval IDs.
- Preserve approved, unclaimed calls through temporary authority or
storage read failures. Reject verified authority changes; cancel and
preserve an unknown outcome if authority is lost after execution
starts.
- Recover pending and in-flight calls after controller or supervisor
loss using original admission witnesses, retained runtime authority,
durable receipts, and authenticated retirement proofs.
- Add configuration guidance, a counted tool simulator, and a dedicated
AgentKit/Foundry approval lifecycle E2E gate.
Split from the original human-approval-v2 branch: the cancellation and
session cleanup fixes, Foundry boot recovery, and the E2E cleanup
evidence harness now land in their own pull requests.
No release ever wrote approval events with raw runtime identifiers or
call IDs under the pre-digest identity domain; those forms only existed
on earlier revisions of this branch. Remove the CallBinding raw-field
upgrade and the raw-call-ID recovery branch along with their tests.
Share one detached settlement context helper, one poll-interval
default, one ApprovalExecutionUpdated payload type, one projection
currency predicate, and one current-Task reader between the broker and
recovery paths.
…uest
An exact redelivery that finds a terminal effect already has its
ApprovalRequested event from the original delivery. Appending it again
made the replay depend on event-store availability and returned 503
instead of the verified durable receipt during a transient outage.
Record the request only for an unstarted effect.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
The example agentkitfile pulled a mutable frontend tag through its
syntax directive, which BuildKit resolves before any build argument, so
the documented immutable-build requirement could not be met. Use a
required digest placeholder and say so in the README.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
An approved brokered wait_for_tasks call inherits the four-minute
execution budget as its context deadline, but the tool's own default
wait is ten minutes, so the budget cancelled it and the call was
recorded as an unknown outcome instead of returning its in-progress
result. Clamp the wait to the caller's context deadline.
Also run the approval E2E gate when the shared free-disk-space or
setup-kind actions change.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The security-sensitive approval, execution, recovery, and cleanup paths span 137 files and warrant final human review despite extensive automated coverage.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The E2E gate validates a later Foundry cancellation commit rather than the companion revision and split scope declared by the PR.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Gate validates the wrong Foundry companion revision
scripts/human-approval-v2-e2e.sh:45
The dedicated gate is not validating the Foundry companion revision advertised by this PR (32cd33f5). It instead builds 9f994c30, whose change is the separately split cancellation/response-acknowledgement fix, so cancellation and recovery scenarios can pass because of code explicitly described as out of this PR's stack. Pin the declared companion revision here, or update the dependency/PR description and gate scope so the validation result accurately identifies the required Foundry contract.
Addressed the Foundry dependency note in this review. The description now identifies the gate's actual Foundry pin, 9f994c30, from scripts/human-approval-v2-e2e.sh:45, including its separate response-acknowledgement cancellation fix. The cancellation and recovery checks validate that combined dependency set; this gate does not qualify 32cd33f5 alone. No code or dependency pin changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Qualified AgentKit and Foundry harness-v2 runtimes can wait for human approval on the original MCP tool call and receive its actual result. An inventory lookup can finish while a proposed work order stays unexecuted until an authorized reviewer approves it.
The controller stores the executable request in an immutable, Task-owned Secret and binds the decision to the Task attempt, prompt, runtime, policy, and tool definition. Execution claims a durable effect record once. A saved receipt can replay; a started action with an uncertain outcome cannot execute again. Decline, expiry, cancellation, and stale authority return explicit outcomes without automatically resubmitting the prompt.
Split
This PR was rebuilt as the top of a stack. The unrelated work that rode along originally now lands separately:
fix/acp-cancellation-and-cleanup: proxy EOF, session base-dir mode, session delete retry, cancellation across proxy revocation.feat/foundry-boot-recovery: Foundry boot retirement through the broker ledger.test/e2e-cleanup-evidence(independent of this stack): E2E cleanup evidence harness.The pre-split history is preserved on
human-approval-v2-pre-split. Against that branch, the only code change here is the second commit: theCallBindingraw-field upgrade and the raw call-ID recovery identity branch are removed because no release ever wrote those forms, and five small helpers are shared between the broker and recovery paths. Everything else is byte-identical to the live-validated tree.Remaining simplification follow-ups: the ExternalEffect Task-UID discovery label (written but only read by fallbacks for records without the spec field), the two denial receipt shapes, the
continueApprovedCallshim, the 15-file approval test layout, and the acceptance runner living underexamples/.Validation
At the pre-split revision
2864ff69b:make lint-fix,make test, focused race/regression tests, and 67 fixture tests. The dedicated approval CI gate passed approval, decline, cancellation, reviewer authorization, full 600-second expiry, concurrent progress, supervisor loss, and pending/in-flight controller loss on both adapters. Both general E2E lanes passed all 81 selected specs.All nine core images were deployed to
sertac-aksin harness-v2 mode. Ten browser checks and the live approval scenarios passed on AgentKit and a real Azurekind: hostedagent, including approval holds over 125 seconds and the full expiry window. Live recovery passed two supervisor crashes and two controller crashes. All 29 acceptance Tasks, seven UI Tasks, and both runtime registrations completed normal cleanup.Six historical crash Tasks from pre-fix validation remain retained because their original admission and deletion evidence is missing.
Current approval E2E dependencies: AgentKit
490bb6d6, merged through PR #27; Foundry runtime9f994c30. The Foundry revision adds the separate response-acknowledgement cancellation fix on top of32cd33f5. The gate's cancellation and recovery checks validate this combined dependency set; they do not qualify32cd33f5alone.Setup guide.
Closes #582.