add az stop cmd.#8881
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “stop session” capability to the azure.ai.agents extension so users can terminate a hosted agent session’s running compute while retaining the session’s persistent filesystem for later resumption.
Changes:
- Add
StopSessionAPI operation and unit tests for success/error and request path. - Add
azd ai agent sessions stop <session-id>CLI subcommand with error classification consistent with other session operations. - Document the new command in the extension changelog.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/operations.go | Adds StopSession HTTP operation to the agent API client. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/operations_test.go | Adds unit tests covering StopSession status handling and request path. |
| cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go | Adds OpStopSession operation name for consistent service error wrapping. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/session.go | Adds sessions stop command implementation and wires it into the session command group. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/session_test.go | Extends command structure tests and adds stop-session error classification tests. |
| cli/azd/extensions/azure.ai.agents/CHANGELOG.md | Adds a changelog entry describing the new sessions stop command. |
jongio
left a comment
There was a problem hiding this comment.
Clean addition of sessions stop that follows the same structure as sessions delete. The API client, command wiring, error classification, and test coverage all match the established patterns.
One suggestion below about using the existing addSessionFlags helper instead of manually registering the same flags. Not blocking, just a consistency improvement.
The PR Governance check is failing because no GitHub issue is linked. Add Fixes #NNN to the PR description (or create an issue and link it) to unblock merge.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
- StopSession: url.PathEscape agentName/sessionID in the request path - sessions stop: use addSessionFlags helper instead of manual flag registration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Previous feedback addressed: addSessionFlags helper adopted, url.PathEscape added for path parameters.
One note on a pre-existing sibling inconsistency (non-blocking, just flagging for a follow-up):
- DeleteSession in operations.go (line 1341-1343) still interpolates agentName and sessionID without url.PathEscape, unlike the new StopSession. Same pattern exists in the delete command's manual flag registration vs the stop command's use of addSessionFlags. Both are good candidates for a quick cleanup PR.
The implementation is clean, follows the established patterns, and has solid test coverage for the happy path (204), error classification (404, 500), and argument validation.
Swallow the 409 session_already_stopped response so 'azd ai agent sessions stop' exits 0 with an 'already stopped' message instead of surfacing a hard error, matching the idempotent behavior of delete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Incremental review of 2354bd0. The idempotent 409 handling is narrowly scoped to session_already_stopped and correctly rejects other conflict codes.
Pre-existing sibling gap (flagged in my prior review, reiterating): DeleteSession in operations.go (L1341-1343) still interpolates agentName/sessionID without url.PathEscape, unlike the new StopSession. Low-risk cleanup candidate for a follow-up PR.
|
@copilot resolve the merge conflicts in this pull request |
Resolved the merge conflict in |
|
@copilot resolve the merge conflicts in this pull request |
The CHANGELOG.md conflict has been resolved locally (merge commit |
Closes #8882
Summary
Adds
azd ai agent sessions stop <session-id>to theazure.ai.agentsextension. The command stops a running hosted agent session while preserving its persistent filesystem, so (unlikesessions delete) the session is retained and can be resumed by a later invocation.Backed by the new service endpoint
POST /agents/{agent_name}/endpoint/sessions/{session_id}:stop(returns204 No Content).Changes
internal/pkg/agents/agent_api/operations.go— newStopSession(...)client method (POST to:stop, accepts 204/200).internal/exterrors/codes.go— newOpStopSessionoperation constant.internal/cmd/session.go— newstopsubcommand +SessionStopAction(modeled ondelete:ExactArgs(1),--agent-name/--user-identity, 404 ->CodeSessionNotFound, elseServiceFromAzure). Registered in thesessionsgroup and updated help text.session_test.goandoperations_test.go.CHANGELOG.mdentry.