Skip to content

Fix autocall implementation parity with MEAI - #1357

Open
Quim Muntal (qmuntal) wants to merge 2 commits into
mainfrom
fix/autocall-meai-parity
Open

Quim Muntal (qmuntal) wants to merge 2 commits into
mainfrom
fix/autocall-meai-parity

Conversation

@qmuntal

Copy link
Copy Markdown
Member

Summary

Fix automatic tool invocation parity with MEAI 10.11.0:

  • Exclude server-completed calls before creating pending approval state, preventing duplicate execution when service-managed history is resumed.
  • Preserve serial call order across hidden safe calls and explicit approvals, keeping tool results ahead of trailing caller messages.
  • Tell the model that rejected operations must not be retried, preserving the approver's reason.
  • Add graceful per-invocation termination through function middleware. Preserve completed/skipped outcomes, finish concurrent calls, isolate nested runs, and explicitly reject unsupported provider-managed termination.

Includes documentation and regression coverage for streaming, approval resumption, restored sessions, errors, and repeated runs.

Validation

  • Affected agent, autocall, and invocation-metadata package tests pass.
  • Full affected-package race tests pass under Windows x64 emulation.
  • Vet and changed-file golangci-lint pass.
  • Copilot, AG-UI, and OpenTelemetry provider test packages compile.

Exclude server-handled calls before creating approval state and preserve
serial call order when resuming hidden safe calls alongside approvals.
Include explicit do-not-retry guidance in rejection results.

Add graceful termination through function invocation middleware, preserving
tool outcomes and isolating nested and concurrent calls. Cover approved
calls, restored sessions, and provider-managed execution with regression
tests, and document the new control.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@qmuntal
Quim Muntal (qmuntal) requested a review from a team as a code owner November 4, 2026 19:09
Copilot AI balanced review requested due to automatic review settings November 4, 2026 19:09
@github-actions github-actions Bot added area:agent Changes files in the agent area area:internal Changes files in the internal area labels Nov 4, 2026
@github-actions github-actions Bot added parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs labels Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

API consistency review

Result Scope
aligned public API, user-visible behavior

Changed Go contract

  • New FunctionInvocationContext.Terminate field in agent/middleware.go. It gracefully stops automatic tool execution for the invocation, and provider-managed tool execution rejects it.
  • Autocall now skips calls the server has already completed before creating pending approval state.
  • Autocall keeps serial call order across safe and approval-required calls.
  • Rejected approvals now return "Tool call invocation was rejected by the approver and must not be retried." plus the approver's reason.

Upstream evidence reviewed

Implementation Source Contract checked
MEAI .NET (local snapshot cfefd350) FunctionInvocationContext.Terminate Per-invocation settable Terminate flag
MEAI .NET 10.11.0 (the revision the PR cites) FunctionInvokingChatClient.cs @ 25f07afa The "must not be retried" rejection text is present (2 matches). Local snapshot cfefd350 still has the older "Tool call invocation rejected." text.

Assessment

The PR ports MEAI 10.11.0 behavior. Terminate mirrors the .NET FunctionInvocationContext.Terminate, and the rejection wording tracks the 10.11.0 revision the PR links. I did not diff the full 10.11.0 file for the ordering and server-completed-call changes. I only confirmed the rejection text and the Terminate property upstream, so those two behaviors rest on the PR's own description and tests. No actionable parity issues found. public-api-change is added because an exported field was added.

Generated by Go API Consistency Review Agent for #1357 · copilot · auto · 26.6 AIC · ⌖ 0.629 AIC · ⊞ 5.2K · ◷

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Termination set on a copied invocation after next returns can be lost, allowing remaining serial tools and another provider request to execute.

1 open issue
What changed in this PR

This PR improves automatic tool invocation parity with MEAI 10.11.0 in the agent harness, focusing on approval handling and graceful termination.

Changes:

  • Excludes server-completed calls from local execution and approvals, while preserving serial order for hidden safe calls and approved calls.
  • Adds per-invocation termination through function middleware and rejects unsupported provider-managed termination.
  • Clarifies rejection results and expands documentation and regression coverage.
File Description
internal/​toolmiddleware/​wrapper.go Carries invocation identity and termination control through context.
docs/​agent-harness.md Documents buffering, approval ordering, and termination behavior.
agent/​middleware.go Adds and propagates middleware termination requests.
agent/​middleware_test.go Tests provider-managed termination rejection and error preservation.
agent/​harness/​toolautocall/​autocall.go Updates approval ordering, server-completed call filtering, and termination handling.
agent/​harness/​toolautocall/​autocall_test.go Covers termination across execution modes, nested calls, and repeated runs.
agent/​harness/​toolautocall/​autocall_log_test.go Verifies termination logging.
agent/​harness/​toolautocall/​autocall_approval_test.go Covers approval resumption, restored sessions, ordering, and conversation retention.

Review effort: Balanced

Comment thread agent/middleware.go
Comment on lines +172 to +175
defer func() {
if nextInvocation != invocation && nextInvocation.Terminate {
invocation.Terminate = true
}
@qmuntal

Copy link
Copy Markdown
Member Author

Copilot fix codereview comments

Co-authored-by: qmuntal <3747816+qmuntal@users.noreply.github.com>

Copilot AI commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Copilot fix codereview comments

Fixed in 65a5ca5: copied invocation termination is now observed after middleware returns, while preserving outer middleware’s ability to clear it. Added the combined copied-context/post-next regression case.

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

Labels

area:agent Changes files in the agent area area:internal Changes files in the internal area parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants