feat(ai): steer resumable Responses - #9705
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 20 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Wei Meng (m5i-work)
left a comment
Wei Meng (m5i-work)
left a comment
There was a problem hiding this comment.
Two steering contract issues need addressing, plus one help-text typo.
9a190ce to
bbc4285
Compare
|
Azure Pipelines: Successfully started running 1 pipeline(s). 20 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds message-bearing --continue for steering or continuing background Responses.
Changes:
- Routes message-bearing continuation to steering.
- Reuses saved conversation/session context.
- Persists and reconnects replacement Responses.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
invoke.go |
Routes steering and refreshes saved status. |
invoke_response_steer.go |
Implements steering and reconnection. |
invoke_response_steer_test.go |
Tests continuation request shapes. |
invoke_background.go |
Updates active-response guidance. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
bbc4285 to
061fe83
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:98
- The new reconnect branch is not covered by an automated test: the end-to-end steering test only returns a complete stream. Add a test that disconnects after
response.created, verifies the replacement ID was persisted, and confirms the follow request resumes that replacement from the saved cursor through completion. This is the failure mode the branch is intended to protect.
if streamErr != nil && ctx.Err() == nil && progressPersister.latest.ResponseID != "" &&
!isTerminalResponseStatus(progressPersister.latest.Status) && isRetryableBackgroundStreamError(streamErr) {
return a.responsesContinueRemote(ctx)
061fe83 to
52e1300
Compare
017c6fe to
8ef0192
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Steering failures need structured classification, and replacement-stream recovery lacks automated coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:60
- This turns every steering rejection into an unclassified plain error, including the conversation-lock and steering-queue failures that the lifecycle spec requires to be actionable service errors (
docs/specs/long-running-agent-invoke.md:476). Decode the service error and return a structuredexterrors.Service/azdext.ServiceErrorwith a stable operation code and remediation so telemetry and the host preserve the classification and suggestion, as required byazure.ai.agents/AGENTS.md:73-111.
if resp.StatusCode >= 400 {
responseBody, _ := io.ReadAll(resp.Body)
return fmt.Errorf("POST %s failed with HTTP %d: %s\n%s", responseURL, resp.StatusCode, resp.Status, responseBody)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:102
- The create-stream reconnect path has no automated coverage; the added tests exercise only request-map construction. Add an
httptest.Servercase that emits the replacement identity/cursor, disconnects, and verifies the follow GET resumes that replacement and reaches terminal state. This protects the PR's central recovery guarantee and the record replacement behavior required by the lifecycle spec.
if streamErr != nil && ctx.Err() == nil && progressPersister.latest.ResponseID != "" &&
!isTerminalResponseStatus(progressPersister.latest.Status) && isRetryableBackgroundStreamError(streamErr) &&
flushErr == nil && closeErr == nil {
latest := progressPersister.latest
return a.followBackgroundResponse(ctx, rc, store, latest, os.Stdout)
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Steering errors need structured classification, recovery guidance, and orchestration-level tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:60
- This collapses every conversation-lock or steering-queue rejection into an unclassified plain error, so the host records
ext.run.failedand loses the HTTP/service metadata and any actionable remediation.docs/specs/long-running-agent-invoke.md:640,647explicitly requires these steering failures to be actionable service errors, anddocs/extensions/extensions-style-guide.md:124-149requires service failures to be classified at the orchestration boundary. Preserve the response status/service code and return a structuredexterrorsservice error with a useful retry suggestion.
if resp.StatusCode >= 400 {
responseBody, _ := io.ReadAll(resp.Body)
return fmt.Errorf("POST %s failed with HTTP %d: %s\n%s", responseURL, resp.StatusCode, resp.Status, responseBody)
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
8ef0192 to
69bd7e5
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The new create-stream recovery path lacks automated end-to-end coverage, and help omits terminal next-turn behavior.
Review details
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:136
- The help only describes revising an active turn, but this same command starts the next background turn when the saved Response is terminal. State both outcomes so users know they can continue after completion.
work. Use --resume without input to reconnect to saved work, or with input to revise
the current turn. Use --cancel to cancel saved work. In multi-agent projects, use
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:102
- Add an automated test for the new create-stream recovery path. The current tests only exercise the request map, so a regression where the replacement identity/cursor is not saved or the follow GET targets the superseded Response would still pass; drive an SSE stream that emits the replacement identity and then disconnects, and assert the follow uses that replacement ID and cursor.
if streamErr != nil && ctx.Err() == nil && progressPersister.latest.ResponseID != "" &&
!isTerminalResponseStatus(progressPersister.latest.Status) && isRetryableBackgroundStreamError(streamErr) &&
flushErr == nil && closeErr == nil {
latest := progressPersister.latest
return a.followBackgroundResponse(ctx, rc, store, latest, os.Stdout)
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
69bd7e5 to
f062750
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Steering errors need structured classification, and the reconnect path needs direct automated coverage.
Review details
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:60
- This orchestration boundary returns every Foundry rejection as an unstructured error, so the host falls back to
ext.run.failedand conversation-lock or steering-queue failures lose service attribution and actionable guidance. Classify the parsed service failure as anexterrors.Service/ServiceError; this is required bycli/azd/extensions/azure.ai.agents/AGENTS.md:73-98and the steering contract atdocs/specs/long-running-agent-invoke.md:476.
if resp.StatusCode >= 400 {
responseBody, _ := io.ReadAll(resp.Body)
return fmt.Errorf("POST %s failed with HTTP %d: %s\n%s", responseURL, resp.StatusCode, resp.Status, responseBody)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:136
- This wording says input always revises the current turn, but the new path starts the next background turn when the saved response is terminal. Describe both outcomes so users do not mistake a new turn for modification of completed work.
work. Use --resume without input to reconnect to saved work, or with input to revise
the current turn. Use --cancel to cancel saved work. In multi-agent projects, use
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:102
- The tests only exercise request-map construction; none executes this reconnect branch, which is the PR's core recovery behavior. Add an HTTP-level test where the POST emits the replacement identity and cursor, disconnects, and verify the follow GET targets that replacement with
starting_afterset to the persisted cursor.
if streamErr != nil && ctx.Err() == nil && progressPersister.latest.ResponseID != "" &&
!isTerminalResponseStatus(progressPersister.latest.Status) && isRetryableBackgroundStreamError(streamErr) &&
flushErr == nil && closeErr == nil {
latest := progressPersister.latest
return a.followBackgroundResponse(ctx, rc, store, latest, os.Stdout)
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
f062750 to
6075dbf
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The reconnect branch lacks automated coverage, and the help text misstates terminal continuation behavior.
Review details
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:136
- The help omits the terminal case: with a completed saved Response, input does not revise that turn; it starts the next background turn. State both behaviors so users do not mistake this for mutating completed work.
work. Use --resume without input to reconnect to saved work, or with input to revise
the current turn. Use --cancel to cancel saved work. In multi-agent projects, use
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:102
- Add automated coverage for the advertised create-stream reconnect path. The new tests only exercise
buildConversationContinuationRequest, so CI would not detect this branch failing to persist and follow the accepted replacement. Feed the steering action a stream that emits the replacement identity/cursor and then disconnects, and assert the follow request targets that replacement after its cursor.
if streamErr != nil && ctx.Err() == nil && progressPersister.latest.ResponseID != "" &&
!isTerminalResponseStatus(progressPersister.latest.Status) && isRetryableBackgroundStreamError(streamErr) &&
flushErr == nil && closeErr == nil {
latest := progressPersister.latest
return a.followBackgroundResponse(ctx, rc, store, latest, os.Stdout)
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Steering errors need structured service classification, and the reconnect workflow lacks automated coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer.go:60
- Classify steering HTTP rejections as service errors instead of returning the raw body in a plain error. The adjacent resume/cancel paths convert
responseLifecycleHTTPErrorthroughclassifyResponseLifecycleHTTPError(invoke_background.go:282-285, 647-658), while this path loses the operation, service, and status metadata needed for actionable409 conversation_lockedand queue failures. Add a steering operation code and apply the same classification to both the create and reconnect paths.
if resp.StatusCode >= 400 {
responseBody, _ := io.ReadAll(resp.Body)
return fmt.Errorf("POST %s failed with HTTP %d: %s\n%s", responseURL, resp.StatusCode, resp.Status, responseBody)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_response_steer_test.go:13
- These tests never exercise
responsesSteerRemote, leaving the claimed replacement reconnect path unverified. Add an HTTP-level test that emits a replacement identity, drops the create SSE stream, and asserts the follow GET uses that replacement ID/cursor and preserves its session/conversation; the existing background lifecycle suite demonstrates this level of coverage.
func TestBuildConversationContinuationRequestUsesSameShapeForAnyStatus(t *testing.T) {
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Reconnect resilience, actionable conflict guidance, and orchestration coverage need improvement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
A pre-identity stream disconnect can leave an accepted replacement unrecoverable and make duplicate retries likely.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
azure.ai.agents PR buildNote This is an unsigned development build. Install it only if you trust this PR. Install the extension: azd ext install "https://azuresdkartifacts.z5.web.core.windows.net/azd/extensions/pr/9705/azure-ai-agents.zip"
|
Summary
Third vertical slice of #9676, stacked on #9704.
--steerwith input to revise active work or start the next background turn--resumemessage-free and uses the existing positional agent/message grammarconversation.idprevious_response_idchainsIf the saved conversation is busy and the agent enables
steerable_conversations, the service queues the new turn and winds down the active handler. If it is idle, the identical request starts the normal next background turn.Validation
go test ./...incli/azd/extensions/azure.ai.agentsgo fix ./...--resumeStack
Fixes #9779