Skip to content

test: cover message identity compatibility across SDKs - #2468

Open
cschleiden wants to merge 3 commits into
mainfrom
cschleiden-queue-message-identity
Open

test: cover message identity compatibility across SDKs#2468
cschleiden wants to merge 3 commits into
mainfrom
cschleiden-queue-message-identity

Conversation

@cschleiden

@cschleiden cschleiden commented Sep 1, 2026

Copy link
Copy Markdown
Member

The runtime now exposes one stable user-message UUID across SendResult.messageId, visible queue.pendingItems rows, and user.message events. With @github/copilot 1.0.83-2 pinned on main, all six SDKs generate the optional identity fields from the canonical schemas; this PR adds compatibility coverage to prevent wire-name or older-runtime regressions.

This change:

  • covers QueuePendingItems.messageId and UserMessageData.messageId in Node, .NET, Python, Go, Rust, and Java
  • verifies the camelCase wire name in serialization and deserialization paths where applicable
  • verifies payloads from older runtimes remain valid when messageId is absent
  • drops the former Rust-only schema augmentation approach in favor of the normal deterministic generators

Validation:

  • shared TypeScript, C#, Python, Go, and Rust codegen, with no generated diff
  • Java Maven codegen, with no generated diff
  • full Node format, lint, typecheck, build, and test suite
  • full Python format, lint, typecheck, and test suite
  • go test ./...
  • full .NET test project
  • mvn verify
  • Rust format, Clippy, and all-feature tests

@cschleiden
cschleiden requested a review from a team as a code owner September 1, 2026 22:12
Copilot AI balanced review requested due to automatic review settings September 1, 2026 22:12

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Adding a field to the stable public UserMessageData struct introduces an unresolved Rust source-compatibility break.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity scripts/​codegen/​rust.ts — Injecting this field into UserMessageData is a Rust source-breaking API change: the type is…
What changed in this PR

Exposes stable queued-message UUIDs in Rust for correlating send results, queue entries, and user-message events.

Changes:

  • Adds temporary Rust codegen schema bridges for messageId.
  • Regenerates Rust API/event types with optional message IDs.
  • Adds serde compatibility tests for current and legacy payloads.
File Description
scripts/​codegen/​utils.ts Adds schema augmentation helpers.
scripts/​codegen/​rust.ts Applies augmentations during Rust generation.
rust/​src/​generated/​api_types.rs Exposes queue-item message IDs.
rust/​src/​generated/​session_events.rs Exposes user-event message IDs.
rust/​tests/​api_types_test.rs Tests queue-item serde behavior.
rust/​tests/​session_events_test.rs Tests event-payload serde behavior.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/codegen/rust.ts Outdated
@SteveSandersonMS
SteveSandersonMS marked this pull request as draft September 2, 2026 13:25
@SteveSandersonMS

SteveSandersonMS commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for addressing the real correlation gap: callers need one stable message UUID shared by SendResult.messageId, pending queue rows, and user.message events.

Suggested scope and timing:

  • Since this capability is language-neutral, it would be best for the SDK exposure to cover all six languages (Node, C#, Python, Go, Rust, and Java).
  • The corresponding fields are not yet present in the currently pinned runtime schema. To keep the SDK aligned with the shipped runtime contract, I suggest waiting for those fields to land, then regenerating the affected SDKs through the normal code-generation pipeline and adding focused compatibility tests for each language as appropriate.

For tracking, I've moved this to draft - please mark it ready to review when the runtime fields have landed and the six SDKs are generated from the updated contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: adfd8891-a94a-4365-9d3a-9662f967d4cf
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: adfd8891-a94a-4365-9d3a-9662f967d4cf
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: adfd8891-a94a-4365-9d3a-9662f967d4cf
@cschleiden
cschleiden force-pushed the cschleiden-queue-message-identity branch from 89df40c to 0e43824 Compare September 3, 2026 01:37
@cschleiden cschleiden changed the title Expose stable queued message identity in Rust SDK test: cover message identity compatibility across SDKs Sep 3, 2026
@cschleiden
cschleiden marked this pull request as ready for review September 3, 2026 01:37
@cschleiden

Copy link
Copy Markdown
Member Author

The runtime fields are now available through the pinned @github/copilot 1.0.83-2 schemas. I rebased onto current main, removed the temporary Rust-only augmentation, added compatibility coverage for all six SDKs, and completed the full validation set, so this is ready for review.

@github-actions github-actions Bot mentioned this pull request Sep 3, 2026
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.

3 participants