Skip to content

feat: allow to emit end exchange event from client for stopping response#510

Merged
norman-le merged 16 commits into
mainfrom
feat/client-end-exchange-for-stop-response
Jun 23, 2026
Merged

feat: allow to emit end exchange event from client for stopping response#510
norman-le merged 16 commits into
mainfrom
feat/client-end-exchange-for-stop-response

Conversation

@norman-le

@norman-le norman-le commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Adds tests demonstrating client-side exchange termination via sendExchangeEnd() on ExchangeStream. This validates that the existing WebSocket event path correctly supports the stop-response flow added in UiPath/AgentInterfaces#1018, where CAS now handles client endExchange events by ending the exchange in DB and killing the running Orchestrator job.

What changed:

  • Added 4 unit tests covering client-initiated stop scenarios:
    • Stopping mid-stream while assistant message is being chunked
    • Blocking further message operations after stop
    • onExchangeEnd handler firing on client-initiated stop (echo mode)
    • Stopping during an in-progress tool call

Why no REST API end() method:
The WebSocket sendExchangeEnd() is the consistent pattern — all other exchange lifecycle operations (start, message, tool calls) use the WebSocket ConversationEvent protocol, not REST. The CAS PR wired up the server-side handler for client endExchange events, making the existing SDK method fully functional for stop-response without needing a new HTTP endpoint.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a client-facing “end exchange” capability to the Conversational Agent SDK so callers can explicitly stop an in-progress response by ending an exchange via a dedicated REST endpoint, while keeping the returned exchange shape consistent with existing exchange retrieval APIs.

Changes:

  • Added a new EXCHANGE_ENDPOINTS.END(...) endpoint constant and an ExchangeService.end(conversationId, exchangeId) method (telemetry-tracked) that POSTs to it and returns a transformed exchange.
  • Extended exchange models/types to include an optional endedAt timestamp and introduced ExchangeEndResponse as an alias of ExchangeGetResponse.
  • Added unit coverage for both the service method (exchanges.end) and the conversation-scoped delegate (conversation.exchanges.end), plus documented OAuth scope requirements.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unit/services/conversational-agent/exchanges.test.ts Adds unit tests validating the new ExchangeService.end behavior, transformation, and error handling.
tests/unit/models/conversational-agent/conversations.test.ts Adds tests ensuring conversation.exchanges.end() delegates correctly and validates required prerequisites.
src/utils/constants/endpoints/conversational-agent.ts Introduces the /end exchange endpoint builder.
src/services/conversational-agent/conversations/exchanges.ts Implements ExchangeService.end() with telemetry and transformation.
src/models/conversational-agent/conversations/types/core.types.ts Adds endedAt?: string to the Exchange model.
src/models/conversational-agent/conversations/exchanges.types.ts Adds ExchangeEndResponse type alias.
src/models/conversational-agent/conversations/exchanges.models.ts Extends service model interfaces to include end(...).
src/models/conversational-agent/conversations/conversations.models.ts Adds conversation-scoped exchanges.end(exchangeId) delegate method.
docs/oauth-scopes.md Documents OAuth scopes required for end().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit/models/conversational-agent/conversations.test.ts Outdated
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comment posted:

  • tests/unit/models/conversational-agent/conversations.test.ts:62 — The end method was added to the mock object but the "should have all methods available" assertion block (~line 98) was not updated to verify typeof conversation.exchanges.end === 'function'.

Missing integration test (no file exists):

Per CLAUDE.md (agent_docs/rules.md): "Every new method must also have an integration test in tests/integration/shared/{domain}/". No integration test for exchanges.end() was added in this PR. An integration test under tests/integration/shared/agents/ (or a conversational-agent subfolder) is required to catch endpoint mismatches, transform errors, and auth issues that unit tests cannot detect.

Comment thread src/models/conversational-agent/conversations/exchanges.models.ts Outdated
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comment posted:

  • src/models/conversational-agent/conversations/exchanges.models.ts:245ConversationExchangeServiceModel.end() JSDoc @returns is missing the {@link ExchangeEndResponse} type link. The parallel ExchangeServiceModel.end() at line 134 uses the correct pattern; the wrapper interface should match.

Still outstanding from previous review:

Per agent_docs/rules.md: "Every new method must also have an integration test in tests/integration/shared/{domain}/". No integration test for exchanges.end() has been added in this PR. An integration test under tests/integration/shared/conversational-agent/ (or equivalent) is required to catch endpoint mismatches, transform errors, and auth issues that unit tests cannot detect.

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread src/models/conversational-agent/conversations/exchanges.types.ts Outdated
Comment thread src/models/conversational-agent/conversations/types/core.types.ts Outdated
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comments posted this run:

  • src/models/conversational-agent/conversations/exchanges.types.ts:68–69ExchangeEndResponse is tagged @internal but lives in exchanges.types.ts, which is re-exported through the public barrel. Convention says internal-only types belong in *.internal-types.ts. Simplest fix: drop the alias and use ExchangeGetResponse directly in all end() signatures.

  • src/models/conversational-agent/conversations/types/core.types.ts:301–305@internal on a field of a public interface is a leaky abstraction. TypeDoc hides it but TypeScript consumers still see endedAt in autocomplete on any ExchangeGetResponse. Either remove @internal (option 1 — lower friction, field is already visible) or make ExchangeEndResponse a proper distinct type extending Exchange with endedAt, rather than a plain alias.

Still outstanding from previous review:

Per agent_docs/rules.md: "Every new method must also have an integration test in tests/integration/shared/{domain}/". No integration test for exchanges.end() was added. An integration test under tests/integration/shared/conversational-agent/ is needed to catch endpoint mismatches, transform errors, and auth issues that unit tests cannot detect.

/**
* Ends an exchange, setting the endedAt timestamp and stopping any running agent job
*
* @internal

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since we are adding the scopes on OAuth lets change this to @experimental

@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread src/models/conversational-agent/conversations/types/core.types.ts Outdated
Comment thread tests/unit/services/conversational-agent/exchanges.test.ts Outdated
Comment thread tests/unit/services/conversational-agent/exchanges.test.ts Outdated
@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comment thread src/models/conversational-agent/conversations/exchanges.types.ts Outdated
@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comment posted this run:

  • src/models/conversational-agent/conversations/exchanges.types.ts:68export type ExchangeEndResponse = ExchangeGetResponse is a single-type alias (type X = Y) that breaks TypeDoc rendering. Per conventions.md, this pattern requires interface extends instead: export interface ExchangeEndResponse extends ExchangeGetResponse {}.

Comment thread tests/unit/services/conversational-agent/exchanges.test.ts Outdated
@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comment posted this run:

  • tests/unit/services/conversational-agent/exchanges.test.ts:458 — The transform test asserts result.endedTime has the correct value but does not assert that (result as any).endedAt is undefined. Per agent_docs/rules.md, transform completeness requires verifying both the renamed field is present with the right value AND the original field name is absent after transformation.

@claude

claude Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread src/models/conversational-agent/conversations/exchanges.models.ts Outdated
@maxduu

maxduu commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Hm, I thought we were going with the event-through-websocket approach? It seems like this is using the API rather than the end-event?

@norman-le

norman-le commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Hm, I thought we were going with the event-through-websocket approach? It seems like this is using the API rather than the end-event?

@maxduu
Hm that's a good point. I was following the file since it had REST API for all the operations, but those aren't really session operations (e.g. getById, getAll, feedback, etc.). They both should technically work , but for consistency I'll change it to only use the webSocket approach (like start/end exchanges, send messages, chunks, tool calls)

Edit: Actually it seems like endExchange was already set up through the websocket - it was just ignored by CAS, so we shouldnt need to add anything here (it's just handled in CAS now instead of a no-op)

Comment thread tests/unit/helpers/conversational-agent/exchange-event-helper.test.ts Outdated
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review findings

New inline comment posted this run:

  • exchange-event-helper.test.ts line 758 — Test named 'should trigger onExchangeEnd handler when client sends stop' never calls sendExchangeEnd(). It only dispatches the server echo (dispatch endExchange), making it a description mismatch (per agent_docs/rules.md) and largely redundant with the pre-existing 'should dispatch endExchange to end handlers' test at line 314. Either rename + complete the round-trip test (add sendExchangeEnd() before the dispatch), or remove it as redundant.

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

@maxduu maxduu self-requested a review June 23, 2026 18:30
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

1 similar comment
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

@sonarqubecloud

Copy link
Copy Markdown

@norman-le norman-le merged commit ee36b52 into main Jun 23, 2026
18 checks passed
@norman-le norman-le deleted the feat/client-end-exchange-for-stop-response branch June 23, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants