feat(ai): retrieve the latest saved Invocation - #9706
feat(ai): retrieve the latest saved Invocation#9706Wei Meng (m5i-work) wants to merge 3 commits into
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. |
0800530 to
a522a7f
Compare
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 25 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
6d85641 to
cdf3d31
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Redirect handling can violate the one-request contract, and persistence failures are hidden from users.
Review details
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:1864
- This
log.Printfis hidden unless--debugis enabled, but this failure means a later--resumewill retrieve stale state or report no saved Invocation. Surface the warning to the user, as the background Response persistence path does ininvoke_background.go:235-245; the extension output convention inAGENTS.md:178-184reserveslogfor non-actionable diagnostics.
log.Printf("warning: failed to save invocation %s for later retrieval: %v", invocationID, err)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go:387
- The broadened
--resumebehavior leaves the existing validation guidance inaccurate for Invocations:--localsays only Responses are supported, the raw-output error refers to a Response cursor, and the timeout error describes attached background work. Use protocol-neutral saved-work wording or branch after protocol resolution so Invocations users receive valid remediation.
"Reconnect to or revise saved background work, or retrieve the latest saved Invocation once",
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_invocation_resume.go:56
- The default
http.Clientfollows redirects, so a same-host redirect forwards the bearer token and issues a second authenticated GET. That violates the one-shot retrieval contract. Disable redirect following and treat every non-2xx response as a failure.
resp, err := (&http.Client{Timeout: a.httpTimeout()}).Do(req) //nolint:gosec // validated Foundry endpoint
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
cdf3d31 to
3cac3f3
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Endpoint-based resume is admitted despite requiring project-backed positional selection.
Review details
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_invocation_resume.go:19
resolveRemoteContextmay attach an azd client even in--agent-endpointmode, so this check allows endpoint-based Invocation resume when run through azd but rejects the same command standalone. The one-shot retrieval contract requires project-backed positional selection and explicitly rejects--agent-endpoint; includea.endpoint != nilin this guard so the unsupported form fails consistently before loading state.
if rc.azdClient == nil || rc.agentKey == "" {
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
3cac3f3 to
55a23e7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Endpoint validation contradicts the documented contract, and several validation messages remain Responses-specific.
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_invocation_resume.go:15
--agent-endpointreaches this path even though the one-shot retrieval contract explicitly rejects it (docs/specs/long-running-agent-invoke.md:214). In normal extension execution,resolveRemoteContextcan attach to azd and derive an agent key, so this combination can succeed instead of returning the documented validation error. Reject it during flag validation and add command-level coverage.
rc, err := a.resolveRemoteContext(ctx)
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Redirect handling violates the one-request contract, and HTTP failure guidance is not status-appropriate.
Review details
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_invocation_resume.go:77
- This appends the “unsupported / not registered / expired” hint to every HTTP failure, so 401/403, 429, and 5xx responses send users down the wrong recovery path. It also returns a plain error, losing the status/service classification used by the analogous lifecycle boundary in
invoke_background.go:308-338. Branch on status and return a structured service error: keep this hint for 404, mention authentication/identity for 401/403, manual retry for 429, and service failure for 5xx.
if resp.StatusCode >= 400 {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf(
"GET %s failed with HTTP %d: %s\n%s\n"+
"the agent might not support retrieval, the invocation might not be registered yet or might have expired",
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_invocation_resume.go:68
- The default HTTP client follows redirects, so a 301/302/307/308 can issue an additional authenticated GET even though this slice promises exactly one request; non-followed 3xx statuses are also currently reported as success. Disable redirects and treat every non-2xx response as a failure so the one-shot contract is enforced.
resp, err := (&http.Client{Timeout: a.httpTimeout()}).Do(req) //nolint:gosec // validated Foundry endpoint
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
f933b2a to
f253aa3
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Retrieval HTTP failures bypass the extension’s structured service-error classification.
Review details
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_invocation_resume.go:75
- This HTTP service failure is returned as a plain
fmt.Errorf, so the extension host classifies every 4xx/5xx retrieval as the genericext.run.failedand loses the status/service telemetry. The extension error guidance requires orchestration code to create a structured service error when the operation and HTTP status are known (cli/azd/extensions/azure.ai.agents/AGENTS.md:73-111), and the adjacent Response lifecycle path preserves this metadata ininvoke_background.go:288-338. Add a retrieval operation code and return anexterrors.ServicewithStatusCodepopulated while keeping the user-facing guidance.
if resp.StatusCode >= 400 {
body, _ := io.ReadAll(resp.Body)
return fmt.Errorf(
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Superseded by #9901, which adds orthogonal |
|
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/9706/azure-ai-agents.zip"
|
Summary
Fourth vertical slice of #9676, stacked on the steering PR.
--resumewith positional agent selection--resumeunsupportedValidation
go test ./...incli/azd/extensions/azure.ai.agentsgo fix ./...Stack
Fixes #9780