Add GitHub-anchored attachment variants to Attachment enum#1823
Conversation
Add the nine GitHub-anchored attachment variants (github_commit, github_release, github_actions_job, github_repository, github_file_diff, github_tree_comparison, github_url, github_file, github_snippet) to the hand-written `Attachment` enum so copilotd no longer drops them when (de)serializing at the SDK boundary. The fields are inlined directly into each variant, mirroring the existing github_reference variant, rather than wrapping the generated `AttachmentGitHub*` structs. Wrapping would emit a duplicate `type` JSON key (the generated structs embed their own discriminator) and break the enum's `Eq` derive (the generated structs do not derive Eq). Nested object fields use local sub-structs that derive Eq. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust SDK’s hand-written Attachment enum (the type used for (de)serializing user-message attachments) to include additional GitHub-anchored attachment variants that the CLI already emits, preventing those attachment payloads from being dropped at the SDK boundary.
Changes:
- Added 9 GitHub-anchored
Attachmentvariants (github_commit,github_release,github_actions_job,github_repository,github_file_diff,github_tree_comparison,github_url,github_file,github_snippet). - Introduced small local helper structs (e.g., repo pointer, diff sides, snippet line range) to mirror generated shapes while preserving
EqonAttachment. - Added a unit test intended to validate round-trip serialization/deserialization for the new variants.
Show a summary per file
| File | Description |
|---|---|
| rust/src/types.rs | Adds missing GitHub-anchored attachment variants + supporting structs + a round-trip unit test. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Low
List github_reference + the 9 GitHub-anchored variants explicitly in the display-name match arms instead of a wildcard, so future Attachment variants trigger a compile error and force an explicit display-behavior decision. Count raw "type": occurrences in the serialized string rather than keys on a parsed Value (which silently dedupes), so the test can actually catch a duplicate-type regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
It's not clear to me why these were hand written in the first place. @tclem, is there a reason these can't be auto-generated? |
All the server->client calls in the Rust SDK are not codegenerated. As far as I can tell it's simply debt. I've added a couple of lines to an "infra/debt" section of our July planning though don't know if we'll reach them without reprioritization, as they are beyond the cutline. |
|
The generated types for these 9 attachment kinds already landed across all languages in the routine schema-bump PR #1819 (Update @github/copilot to 1.0.66-1). I re-ran the code generator for all six languages (scripts/codegen + java/scripts/codegen) against the pinned 1.0.66-1 schema and git diff is empty, confirming they're already in sync. The only representation codegen doesn't own is Rust's hand-written Attachment enum (rust/src/types.rs), which is what this PR fixes. |
What & why
The hand-written Rust
Attachmentenum inrust/src/types.rs(the type copilotd (de)serializes user-message attachments through) only knew about 5 variants:File,Directory,Selection,Blob, andGitHubReference. The CLI already renders 9 additional GitHub-anchored reference types, and the corresponding codegen structs already exist inrust/src/generated/api_types.rs, but because the hand-written enum was missing the variants they were silently dropped at the SDK boundary.These reference types were added to support Dotcom Chat via:
Both are merged. This PR closes the SDK-side gap.
What's in this PR
Adds the 9 variants to the
Attachmentenum (serdetype→ variant):typegithub_commitGitHubCommitgithub_releaseGitHubReleasegithub_actions_jobGitHubActionsJobgithub_repositoryGitHubRepositorygithub_file_diffGitHubFileDiffgithub_tree_comparisonGitHubTreeComparisongithub_urlGitHubUrlgithub_fileGitHubFilegithub_snippetGitHubSnippetEach variant inlines its fields (using the generated
AttachmentGitHub*structs as the authoritative field spec), exactly mirroring the existinggithub_referencevariant.Nested object fields (repo pointer, file-diff sides, tree-comparison sides, snippet line range) use small local sub-structs that derive
PartialEq, Eqand match the generated JSON shapes.The three small helper match arms in
display_name(),ensure_display_name(), andderived_display_name()were relaxed from an explicitSelf::GitHubReference { .. }arm to a wildcard_, which is future-proof since the enum is#[non_exhaustive].Testing
cargo check,cargo fmt,cargo clippyall clean.github_anchored_attachment_variants_round_trip) that, for each of the 9 variants, deserializes a representative JSON blob intoAttachment, re-serializes it, and asserts the output has exactly onetypekey with the correct discriminator and round-trips with no data loss.Scope
Touches only
rust/src/types.rs(variants + sub-structs + test). No codegen was run; nothing undergenerated/was modified.