Skip to content

fix: remove legacy service-connection MCP field#234

Merged
jamesadevine merged 1 commit intomainfrom
fix/remove-legacy-service-connection
Apr 16, 2026
Merged

fix: remove legacy service-connection MCP field#234
jamesadevine merged 1 commit intomainfrom
fix/remove-legacy-service-connection

Conversation

@jamesadevine
Copy link
Copy Markdown
Collaborator

Summary

Remove the dead service_connection field from McpOptions and fix incorrect documentation claiming 1ES uses service connections for MCP routing. Both targets use MCPG — the field was parsed but never read.

Changes

  • src/compile/types.rs: Remove dead service_connection field from McpOptions
  • AGENTS.md: Remove service-connection: from MCP config docs
  • prompts/create-ado-agentic-workflow.md: Fix 3 incorrect 1ES MCP claims
  • prompts/update-ado-agentic-workflow.md: Fix target compatibility note

See analysis comment for full investigation.

Closes #233

@jamesadevine jamesadevine force-pushed the fix/remove-legacy-service-connection branch from 009e03a to 1eff495 Compare April 16, 2026 21:44
Remove the dead `service_connection` field from `McpOptions` and fix
incorrect documentation claiming 1ES uses service connections for MCP
routing. Both targets use MCPG — the field was parsed but never read.

- Remove `service_connection` field from `McpOptions` in types.rs
- Remove `service-connection:` from AGENTS.md MCP config docs
- Fix create/update prompts: 1ES supports containerized MCPs via MCPG

Closes #233

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine jamesadevine force-pushed the fix/remove-legacy-service-connection branch from 1eff495 to 4943826 Compare April 16, 2026 21:45
@jamesadevine jamesadevine merged commit baf5fe9 into main Apr 16, 2026
4 checks passed
@jamesadevine jamesadevine deleted the fix/remove-legacy-service-connection branch April 16, 2026 21:47
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good — clean dead-code removal with accurate documentation fixes.

Findings

✅ What Looks Good

  • Backward-compatible serde removal: McpOptions does not carry #[serde(deny_unknown_fields)] (that attribute is only on NetworkConfig), so removing service_connection is silently safe — any existing front matter with service-connection: will simply be ignored by serde going forward, which is exactly the correct behavior.
  • Confirmed dead field: The removed field has zero read-sites in the codebase. The service_connection references in common.rs are for ADO token acquisition (generate_acquire_ado_token, generate_copilot_ado_env, generate_executor_ado_env) — completely unrelated to McpOptions.
  • Documentation accuracy: The prompt corrections correctly reflect the actual compiler behavior — both standalone and 1es targets generate MCPG configs via {{ mcpg_config }}, no service-connection MCP routing exists in either path.
  • Scope is appropriately tight: 4 files, 13 changed lines, single well-defined concern. No unrelated churn.

Generated by Rust PR Reviewer for issue #234 · ● 340.6K ·

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.

Investigate: 1ES target MCP service connection mapping — legacy artifact?

1 participant