Carry over hosted-agent session across azd deploy#8942
Conversation
Automatically stop the current hosted-agent session during `azd deploy` and re-point the newly deployed version's session pointer at it, so the next `azd ai agent invoke` resumes the same session on the new code with its /home/session volume intact instead of minting a fresh session. - session_carryover.go: capture (predeploy) + stop-and-carry-forward (postdeploy) logic, opt-out via AZD_AGENT_SKIP_SESSION_CARRYOVER. - listen.go: wire capture into predeployHandler and carry-over into postdeployHandler (best-effort, never blocks deploy). - session_carryover_test.go: env-flag and guard no-op tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds best-effort “session carry-over” behavior for azure.ai.agents hosted agents during azd deploy, so the next azd ai agent invoke can resume the same session (and its /home/session volume) after a new agent version is deployed.
Changes:
- Introduces predeploy capture and postdeploy stop+repoint logic for carried sessions, with opt-out via
AZD_AGENT_SKIP_SESSION_CARRYOVER. - Wires session capture into
predeployHandlerand session carry-over intopostdeployHandlerfor hosted agents. - Adds unit tests for env-flag parsing and guard/no-op behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/cmd/session_carryover.go | Implements in-process stash, capture, stop, and session-pointer repointing across deploys. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/session_carryover_test.go | Adds tests for opt-out behavior and early-return guards. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go | Hooks session capture into predeploy and carry-over into postdeploy for hosted agents. |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The session carry-over design is solid: pre-deploy capture, post-deploy stop+repoint, with best-effort guarantees that never block deploy. The code uses modern Go patterns correctly (�rrors.AsType, mutex-guarded stash) and the error handling is thorough with informative log messages at every failure point.
One suggestion on test coverage: the current tests verify guard/no-op behavior (disabled flag, nil params), but the interesting logic -- the StopSession error classification (409 conflict vs 404 vs unexpected), the endpoint re-read after deploy, and the config-store write -- is untested. A table-driven test with a fake AgentClient (or httptest server) covering the session_already_stopped proceeds path, the 404 bails-out path, and the successful end-to-end repoint would strengthen confidence in the branching logic and prevent regressions.
Replace the AZD_AGENT_SKIP_SESSION_CARRYOVER opt-out env var with an opt-in per-service azure.yaml field, resumeSessionOnDeploy (default false). This is more discoverable, versioned with the project, and scoped per agent service. - config.go: add ResumeSessionOnDeploy to ServiceTargetAgentConfig. - session_carryover.go: gate capture/carry-over on the per-service field via LoadServiceTargetAgentConfig instead of the env var. - schemas: document resumeSessionOnDeploy in Agent.json and azure.ai.agent.json. - tests: cover the opt-in/opt-out/absent/nil cases. Validated live on a Foundry hosted agent: with resumeSessionOnDeploy: true and no env var, a redeploy + plain invoke resumed the same session on the new code (BUILD v9 -> v10, /home/session 2 -> 3, volume preserved). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
The switch from a global env-var opt-out to per-service resumeSessionOnDeploy config is the right design: more granular, opt-in by default, and source-controlled. Implementation is clean with proper nil/error guards.
My earlier suggestion about testing the deeper carry-over logic (StopSession 409/404 classification, endpoint re-read after deploy, config-store write) still applies as follow-up work for confidence in the branching paths, but is not blocking for this PR.
- Serialize the carry-over read-modify-write of the shared UserConfig 'sessions' map with a package mutex, since concurrent postdeploy handlers could otherwise clobber each other's carried session (last-writer-wins). - Extract stop-error classification into classifyStopSessionErr and add a table-driven test covering nil/409 already-stopped/404/other-409/500/ non-response cases. - Document that carry-over consumes the stashed session single-shot and is intentionally non-retriable (best-effort contract). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssion-carryover-on-deploy
…carryover Main refactored postdeployHandler and removed the local agentClient and the agent_api import; the session carry-over call referenced the now-undefined agentClient (typecheck failure). Rebuild agentClient from the in-scope endpoint and credential and re-add the agent_api import. Also apply gofmt to session_carryover.go (stray trailing blank line) so the gofmt lint passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Incremental check: the two new commits (merge main + rebuild agentClient in postdeploy) are mechanical CI fixes. The agent_api.NewAgentClient(endpointResp.Value, cred) construction in postdeployHandler correctly targets the post-deploy endpoint for the StopSession call. No design changes, no new risk.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
…zure.yaml field Revert the opt-in control from a per-service azure.yaml field back to an environment variable, named resumeSessionOnDeploy. Carry-over is opt-in: off unless resumeSessionOnDeploy is truthy (1/true/yes/on). Removes the ServiceTargetAgentConfig.ResumeSessionOnDeploy field and its schema entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The new commit documents resumeSessionOnDeploy (opt-in, truthy 1/true/yes/on) and matches the implementation in session_carryover.go.
One thing to fix before merge: the PR description still says carry-over is automatic with opt-out via AZD_AGENT_SKIP_SESSION_CARRYOVER, which is the opposite of the actual opt-in resumeSessionOnDeploy behavior. Update the description so it doesn't mislead.
Resolve semantic conflict: postdeployHandler was refactored on main to use gatherPostdeployInputs; carry-over agentClient now uses the returned endpoint (FOUNDRY_PROJECT_ENDPOINT) instead of the removed endpointResp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Re-approving after the origin/main merge (fbe243b). The merge is mechanical: the carry-over wiring in listen.go and session_carryover.go is unchanged from my last review, and CI is green across all platforms.
Still worth fixing before merge: the PR description says carry-over is automatic with opt-out via AZD_AGENT_SKIP_SESSION_CARRYOVER, but the code and README use opt-in via resumeSessionOnDeploy. Update the description so it matches.
Address PR review: captureSessionForCarryover and carryOverSessionAfterDeploy now early-return on a nil azdClient (and empty envName) alongside the existing nil-svc/agentClient guards, keeping these best-effort helpers panic-safe if ever misused. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Re-approving after 27c13d1. The nil-guard fix is correct: captureSessionForCarryover now returns early on a nil azdClient before it reaches currentEnvName and Environment().GetValue, and carryOverSessionAfterDeploy guards azdClient == nil and empty envName ahead of the endpoint re-read. That matches the best-effort, never-panic design of these helpers. No new issues in the delta.
Address PR review: align the opt-in env var with the extension's UPPER_SNAKE_CASE convention (e.g. AZD_AGENT_SKIP_ROLE_ASSIGNMENTS) instead of the lowerCamelCase resumeSessionOnDeploy. The var is unreleased, so no backward-compatible alias is needed. Updates the const, doc comments, and README. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Carry-over stop call has a correctness gap (inline on line 209): it passes the azure.yaml service key where the session API path needs the deployed Foundry agent name. It is best-effort so a deploy won't fail, but when the deployed agent name differs from the service key the stop request 404s and carry-over silently no-ops instead of resuming the session. A success-path test covering a service-key vs agent-name mismatch would catch this and also closes the test-coverage gap already noted on the file.
Separately, the PR description still describes a default-on AZD_AGENT_SKIP_SESSION_CARRYOVER opt-out, but the shipped behavior is opt-in via AZD_AGENT_RESUME_SESSION_ON_DEPLOY. Worth updating the description to match.
Closes #8957
Automatically stop the current hosted-agent session during
azd deployand re-point the newly deployed version's session pointer at it, so the nextazd ai agent invokeresumes the same session on the new code with its /home/session volume intact instead of minting a fresh session. - session_carryover.go: capture (predeploy) + stop-and-carry-forward (postdeploy) logic, opt-out via AZD_AGENT_SKIP_SESSION_CARRYOVER. - listen.go: wire capture into predeployHandler and carry-over into postdeployHandler (best-effort, never blocks deploy). - session_carryover_test.go: env-flag and guard no-op tests.