From 0fe915fe66730db1af0249ec1e6794f1936dd762 Mon Sep 17 00:00:00 2001 From: Pallavi Raiturkar Date: Mon, 29 Jun 2026 08:38:12 -0400 Subject: [PATCH 1/2] Add GitHub-anchored attachment variants to Attachment enum 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> --- rust/src/types.rs | 310 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 307 insertions(+), 3 deletions(-) diff --git a/rust/src/types.rs b/rust/src/types.rs index 75408db02..04cb216e6 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -3956,6 +3956,55 @@ pub enum GitHubReferenceType { Discussion, } +/// Pointer to a GitHub repository (owner/name plus optional numeric id). +/// +/// Used by the GitHub-anchored [`Attachment`] variants. Mirrors the field +/// shape of the generated `GitHubRepoRef`, but defined locally so it can +/// derive `Eq` for use inside the `Attachment` enum. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitHubRepoPointer { + /// Numeric GitHub repository id. + #[serde(skip_serializing_if = "Option::is_none")] + pub id: Option, + /// Repository name (without owner). + pub name: String, + /// Repository owner login (user or organization). + pub owner: String, +} + +/// One side (head or base) of a GitHub single-file diff. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitHubFileDiffSide { + /// Repository-relative path to the file. + pub path: String, + /// Git ref (branch, tag, or commit SHA) the file is read at. + pub r#ref: String, + /// Repository the file lives in. + pub repo: GitHubRepoPointer, +} + +/// One side (head or base) of a GitHub tree comparison. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitHubTreeComparisonSide { + /// Repository the revision belongs to. + pub repo: GitHubRepoPointer, + /// Git revision (branch, tag, or commit SHA). + pub revision: String, +} + +/// Line range covered by a GitHub snippet attachment (1-based, inclusive end). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitHubSnippetLineRange { + /// Start line number (1-based). + pub start: i64, + /// End line number (1-based, inclusive). + pub end: i64, +} + /// An attachment included with a user message. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde( @@ -4020,6 +4069,117 @@ pub enum Attachment { /// URL to the referenced item. url: String, }, + /// A pointer to a GitHub commit. + #[serde(rename = "github_commit")] + GitHubCommit { + /// First line of the commit message. + message: String, + /// Full commit SHA. + oid: String, + /// Repository the commit belongs to. + repo: GitHubRepoPointer, + /// URL to the commit on GitHub. + url: String, + }, + /// A pointer to a GitHub release. + #[serde(rename = "github_release")] + GitHubRelease { + /// Human-readable release name. + name: String, + /// Repository the release belongs to. + repo: GitHubRepoPointer, + /// Git tag the release is anchored to. + tag_name: String, + /// URL to the release on GitHub. + url: String, + }, + /// A pointer to a GitHub Actions job. + #[serde(rename = "github_actions_job")] + GitHubActionsJob { + /// Terminal conclusion of the job when finished (e.g. "success", + /// "failure", "cancelled"). Absent for in-progress jobs. + #[serde(skip_serializing_if = "Option::is_none")] + conclusion: Option, + /// Job id within the workflow run. + job_id: i64, + /// Display name of the job. + job_name: String, + /// Repository the workflow run belongs to. + repo: GitHubRepoPointer, + /// URL to the job on GitHub. + url: String, + /// Display name of the workflow the job ran in. + workflow_name: String, + }, + /// A pointer to a GitHub repository. + #[serde(rename = "github_repository")] + GitHubRepository { + /// Short description of the repository. + #[serde(skip_serializing_if = "Option::is_none")] + description: Option, + /// Git ref this attachment is anchored at (branch, tag, or commit). + /// When absent the default branch is implied. + #[serde(skip_serializing_if = "Option::is_none")] + r#ref: Option, + /// Repository pointer. + repo: GitHubRepoPointer, + /// URL to the repository on GitHub. + url: String, + }, + /// A pointer to a single-file diff. At least one of `head` and `base` is present. + #[serde(rename = "github_file_diff")] + GitHubFileDiff { + /// File location on the base side of the diff. Absent for additions. + #[serde(skip_serializing_if = "Option::is_none")] + base: Option, + /// File location on the head side of the diff. Absent for deletions. + #[serde(skip_serializing_if = "Option::is_none")] + head: Option, + /// URL to the diff on GitHub (e.g. a commit, compare, or PR-file URL). + url: String, + }, + /// A pointer to a comparison between two git revisions. + #[serde(rename = "github_tree_comparison")] + GitHubTreeComparison { + /// Base side of the comparison. + base: GitHubTreeComparisonSide, + /// Head side of the comparison. + head: GitHubTreeComparisonSide, + /// URL to the comparison on GitHub. + url: String, + }, + /// A generic GitHub URL reference. + #[serde(rename = "github_url")] + GitHubUrl { + /// URL to the GitHub resource. + url: String, + }, + /// A pointer to a file in a GitHub repository at a specific ref. + #[serde(rename = "github_file")] + GitHubFile { + /// Repository-relative path to the file. + path: String, + /// Git ref the file is read at (branch, tag, or commit SHA). + r#ref: String, + /// Repository the file lives in. + repo: GitHubRepoPointer, + /// URL to the file on GitHub. + url: String, + }, + /// A pointer to a line range inside a file in a GitHub repository. + #[serde(rename = "github_snippet")] + GitHubSnippet { + /// Line range the snippet covers. + line_range: GitHubSnippetLineRange, + /// Repository-relative path to the file. + path: String, + /// Git ref the file is read at (branch, tag, or commit SHA). + r#ref: String, + /// Repository the file lives in. + repo: GitHubRepoPointer, + /// URL to the snippet on GitHub (with line anchor). + url: String, + }, } impl Attachment { @@ -4030,7 +4190,7 @@ impl Attachment { | Self::Directory { display_name, .. } | Self::Selection { display_name, .. } | Self::Blob { display_name, .. } => display_name.as_deref(), - Self::GitHubReference { .. } => None, + _ => None, } } @@ -4073,7 +4233,7 @@ impl Attachment { | Self::Directory { display_name, .. } | Self::Selection { display_name, .. } | Self::Blob { display_name, .. } => *display_name = Some(derived_display_name), - Self::GitHubReference { .. } => {} + _ => {} } } @@ -4084,7 +4244,7 @@ impl Attachment { } Self::Selection { file_path, .. } => Some(attachment_name_from_path(file_path)), Self::Blob { .. } => Some("attachment".to_string()), - Self::GitHubReference { .. } => None, + _ => None, } } } @@ -6040,6 +6200,150 @@ mod tests { Some("Track regressions".to_string()) ); } + + #[test] + fn github_anchored_attachment_variants_round_trip() { + let cases = vec![ + ( + "github_commit", + json!({ + "type": "github_commit", + "message": "Fix the thing", + "oid": "abc123", + "repo": { "id": 1, "name": "repo", "owner": "octocat" }, + "url": "https://github.com/octocat/repo/commit/abc123" + }), + ), + ( + "github_release", + json!({ + "type": "github_release", + "name": "v1.2.3", + "repo": { "name": "repo", "owner": "octocat" }, + "tagName": "v1.2.3", + "url": "https://github.com/octocat/repo/releases/tag/v1.2.3" + }), + ), + ( + "github_actions_job", + json!({ + "type": "github_actions_job", + "conclusion": "failure", + "jobId": 99, + "jobName": "build", + "repo": { "name": "repo", "owner": "octocat" }, + "url": "https://github.com/octocat/repo/actions/runs/1/job/99", + "workflowName": "CI" + }), + ), + ( + "github_repository", + json!({ + "type": "github_repository", + "description": "An example repository", + "ref": "main", + "repo": { "name": "repo", "owner": "octocat" }, + "url": "https://github.com/octocat/repo" + }), + ), + ( + "github_file_diff", + json!({ + "type": "github_file_diff", + "base": { + "path": "src/lib.rs", + "ref": "main", + "repo": { "name": "repo", "owner": "octocat" } + }, + "head": { + "path": "src/lib.rs", + "ref": "feature", + "repo": { "name": "repo", "owner": "octocat" } + }, + "url": "https://github.com/octocat/repo/compare/main...feature" + }), + ), + ( + "github_tree_comparison", + json!({ + "type": "github_tree_comparison", + "base": { + "repo": { "name": "repo", "owner": "octocat" }, + "revision": "main" + }, + "head": { + "repo": { "name": "repo", "owner": "octocat" }, + "revision": "feature" + }, + "url": "https://github.com/octocat/repo/compare/main...feature" + }), + ), + ( + "github_url", + json!({ + "type": "github_url", + "url": "https://github.com/octocat/repo/wiki" + }), + ), + ( + "github_file", + json!({ + "type": "github_file", + "path": "src/main.rs", + "ref": "main", + "repo": { "name": "repo", "owner": "octocat" }, + "url": "https://github.com/octocat/repo/blob/main/src/main.rs" + }), + ), + ( + "github_snippet", + json!({ + "type": "github_snippet", + "lineRange": { "start": 10, "end": 20 }, + "path": "src/main.rs", + "ref": "main", + "repo": { "name": "repo", "owner": "octocat" }, + "url": "https://github.com/octocat/repo/blob/main/src/main.rs#L10-L20" + }), + ), + ]; + + for (expected_type, input) in cases { + let attachment: Attachment = serde_json::from_value(input.clone()) + .unwrap_or_else(|err| panic!("{expected_type} should deserialize: {err}")); + + let serialized = serde_json::to_value(&attachment) + .unwrap_or_else(|err| panic!("{expected_type} should serialize: {err}")); + + let object = serialized + .as_object() + .unwrap_or_else(|| panic!("{expected_type} should serialize to an object")); + + // Exactly one `type` key, carrying the expected discriminator. + assert_eq!( + object.keys().filter(|key| key.as_str() == "type").count(), + 1, + "{expected_type} must serialize a single `type` key" + ); + assert_eq!( + object.get("type").and_then(|value| value.as_str()), + Some(expected_type), + "{expected_type} must serialize the correct discriminator" + ); + + // Round-trips without dropping fields. + assert_eq!( + serialized, input, + "{expected_type} should round-trip without data loss" + ); + let reparsed: Attachment = serde_json::from_value(serialized) + .unwrap_or_else(|err| panic!("{expected_type} should re-deserialize: {err}")); + assert_eq!( + reparsed, attachment, + "{expected_type} should re-deserialize to the same value" + ); + } + } } #[cfg(test)] From 04451ae984d94c1ad976c1c46fea76b12c8505a9 Mon Sep 17 00:00:00 2001 From: Pallavi Raiturkar Date: Mon, 29 Jun 2026 08:46:05 -0400 Subject: [PATCH 2/2] Make attachment match arms explicit and harden duplicate-type test 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> --- rust/src/types.rs | 50 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/rust/src/types.rs b/rust/src/types.rs index 04cb216e6..06b97fbbd 100644 --- a/rust/src/types.rs +++ b/rust/src/types.rs @@ -4190,7 +4190,16 @@ impl Attachment { | Self::Directory { display_name, .. } | Self::Selection { display_name, .. } | Self::Blob { display_name, .. } => display_name.as_deref(), - _ => None, + Self::GitHubReference { .. } + | Self::GitHubCommit { .. } + | Self::GitHubRelease { .. } + | Self::GitHubActionsJob { .. } + | Self::GitHubRepository { .. } + | Self::GitHubFileDiff { .. } + | Self::GitHubTreeComparison { .. } + | Self::GitHubUrl { .. } + | Self::GitHubFile { .. } + | Self::GitHubSnippet { .. } => None, } } @@ -4233,7 +4242,16 @@ impl Attachment { | Self::Directory { display_name, .. } | Self::Selection { display_name, .. } | Self::Blob { display_name, .. } => *display_name = Some(derived_display_name), - _ => {} + Self::GitHubReference { .. } + | Self::GitHubCommit { .. } + | Self::GitHubRelease { .. } + | Self::GitHubActionsJob { .. } + | Self::GitHubRepository { .. } + | Self::GitHubFileDiff { .. } + | Self::GitHubTreeComparison { .. } + | Self::GitHubUrl { .. } + | Self::GitHubFile { .. } + | Self::GitHubSnippet { .. } => {} } } @@ -4244,7 +4262,16 @@ impl Attachment { } Self::Selection { file_path, .. } => Some(attachment_name_from_path(file_path)), Self::Blob { .. } => Some("attachment".to_string()), - _ => None, + Self::GitHubReference { .. } + | Self::GitHubCommit { .. } + | Self::GitHubRelease { .. } + | Self::GitHubActionsJob { .. } + | Self::GitHubRepository { .. } + | Self::GitHubFileDiff { .. } + | Self::GitHubTreeComparison { .. } + | Self::GitHubUrl { .. } + | Self::GitHubFile { .. } + | Self::GitHubSnippet { .. } => None, } } } @@ -6312,21 +6339,24 @@ mod tests { let attachment: Attachment = serde_json::from_value(input.clone()) .unwrap_or_else(|err| panic!("{expected_type} should deserialize: {err}")); - let serialized = serde_json::to_value(&attachment) + // Serialize to a string first: parsing into `serde_json::Value` would + // silently dedupe a duplicate `type` key, hiding the exact regression + // this test guards against (e.g. a wrapped generated struct emitting its + // own `type` alongside the enum tag). + let serialized_string = serde_json::to_string(&attachment) .unwrap_or_else(|err| panic!("{expected_type} should serialize: {err}")); - let object = serialized - .as_object() - .unwrap_or_else(|| panic!("{expected_type} should serialize to an object")); - // Exactly one `type` key, carrying the expected discriminator. assert_eq!( - object.keys().filter(|key| key.as_str() == "type").count(), + serialized_string.matches("\"type\":").count(), 1, "{expected_type} must serialize a single `type` key" ); + + let serialized: serde_json::Value = serde_json::from_str(&serialized_string) + .unwrap_or_else(|err| panic!("{expected_type} should reparse: {err}")); assert_eq!( - object.get("type").and_then(|value| value.as_str()), + serialized.get("type").and_then(|value| value.as_str()), Some(expected_type), "{expected_type} must serialize the correct discriminator" );