Skip to content

Preserve Rust session-message errors on the App SDK source line - #2912

Draft
dfrysinger wants to merge 1 commit into
dfrysinger/cross-session-runtime-admissionfrom
dfrysinger/remote-v1-messaging-error-backport
Draft

dfrysinger wants to merge 1 commit into
dfrysinger/cross-session-runtime-admissionfrom
dfrysinger/remote-v1-messaging-error-backport

Conversation

@dfrysinger

Copy link
Copy Markdown

Summary

This backport lets the Rust SDK report session-message delivery failures without losing the original Runtime or transport error. It also restores the discovery location types already supported by the App, so a normal SDK sync does not remove local and remote session selection.

The SDK makes one message request. A valid Runtime failure becomes a typed delivery result with the original error available through source(). A transport failure is ambiguous and does not invent a message ID. Malformed or unrelated RPC errors pass through unchanged. There is no retry or alternate delivery path.

flowchart TD
    Send["Send one session-message request"] --> Result{"Request result"}
    Result -->|Success| Success["Return the Runtime result"]
    Result -->|Transport failure| Ambiguous["Return ambiguous delivery without inventing an ID"]
    Result -->|RPC failure| Envelope{"Valid message-failure envelope?"}
    Envelope -->|Yes| Typed["Return typed delivery failure and retain the original cause"]
    Envelope -->|No| Original["Return the original RPC error"]
Loading

Source line and scope

The base is dfrysinger/cross-session-runtime-admission, at the App's existing SDK source commit 8f6a864323f1093a1cffd2ed2ff40747764305a6. This PR changes four Rust files on that line. It does not upgrade the SDK or downgrade main. The source line's earlier PR is closed and remains closed; integrating that foundation into main is separate work.

The discovery restoration copies the existing App contract into canonical SDK source. It adds no new selector or wire behavior. After this source commit is published, the App can use its normal sync script rather than editing vendor files.

This fixes SDK error handling used by the remote-messaging service work and the App integration.

Evidence and remaining acceptance

Seven focused send tests and three discovery tests pass, including original-error preservation, malformed envelopes, transport ambiguity, single-request behavior, and local/remote discovery serialization. Scoped Cargo check, Clippy, and formatting pass. Both focused source reviewers found no defects.

This is a draft source backport, not installed acceptance or release approval. Normal App intake, current Runtime integration, installed behavior, and any integration into SDK main remain separate gates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7ae87499-d464-4758-9172-947a5a2d05b1
@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR modifies only the Rust SDK (rust/src/errors.rs, rust/src/rpc.rs, rust/src/session.rs, rust/tests/session_test.rs), refining the experimental cross-session messaging API:

  • Adds MessageableSessionLocation (Local/Remote) and a location field on ListMessageableSessionsRequest, generalizing discovery from "local-only" to "local and remote" messageable sessions.
  • Adds a failure_code: Option<String> field to SessionErrorKind::SendSessionMessage, parsed from the error envelope's failureCode.
  • Tightens/relaxes error-classification logic in parse_send_session_message_error_data (e.g., all session_message_refused kinds now map to Refused regardless of runtime code; transport failures are classified as Ambiguous).
  • Expands test coverage for the new location field and failure classification.

Cross-SDK comparison:

  • The Node.js/TypeScript SDK (nodejs/src/session.ts) already implements the equivalent listMessageableSessions/sendSessionMessage API, but has not been updated in this PR to add the new location scoping option or the failureCode diagnostic field — flagged as inline review comments for parity.
  • Python, Go, .NET, and Java do not yet implement listMessageableSessions/sendSessionMessage at all, so this PR doesn't introduce a new inconsistency for them (this is a pre-existing gap, not something this PR needs to address).

Overall the change is internally consistent within Rust; the main follow-up suggestion is to mirror the location request field and failureCode error detail in the Node.js SDK to keep the two existing implementations of this experimental API in sync.

Generated by SDK Consistency Review Agent for #2912 · copilot · sonnet50 · 36.4 AIC · ⌖ 12 AIC · ⊞ 5.1K · ◷

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generated by SDK Consistency Review Agent for #2912 · copilot · sonnet50 · 36.4 AIC · ⌖ 12 AIC · ⊞ 5.1K

Comment thread rust/src/rpc.rs
#[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct ListMessageableSessionsRequest {
/// Optional exact session name query. Matching semantics are owned by the local host.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR extends ListMessageableSessionsRequest with a new location field (MessageableSessionLocation::Local/Remote) so cross-session messaging discovery can be scoped to local vs. remote producers, and broadens MessageableSession/SendSessionMessageRequest docs to cover remote sessions as well.

The Node.js/TypeScript SDK has a parallel listMessageableSessions/sendSessionMessage API (nodejs/src/session.ts) but its ListMessageableSessionsRequest only has name?: string — there's no location/MessageableSessionLocation equivalent. For feature parity, consider adding the same location option ("local" | "remote") to the Node.js request type and wiring it through listMessageableSessions.

Note: Python, Go, .NET, and Java don't appear to implement listMessageableSessions/sendSessionMessage at all yet, so this isn't a regression for them, just a pre-existing parity gap to be aware of for future work.

Comment thread rust/src/errors.rs
message_id: Option<String>,
/// Stable runtime failure identifier, when reported.
failure_code: Option<String>,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new failure_code: Option<String> field on SessionErrorKind::SendSessionMessage (a stable runtime failure identifier parsed from the error envelope's failureCode) doesn't have an equivalent in the Node.js SDK's SendSessionMessageError (nodejs/src/session.ts), which only carries code and messageId. Consider surfacing failureCode there too (and in any future Python/Go/.NET/Java implementations) so callers across languages get the same diagnostic detail from a failed cross-session message send.

This branch has not been deployed

No deployments
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.

1 participant