docs: add design spec for RemoteA2A connection support and catalog in connections command#8302
docs: add design spec for RemoteA2A connection support and catalog in connections command#8302Nathandrake229 wants to merge 5 commits into
Conversation
…tegration Covers Linda's two follow-up requests on PR #8174: 1. RemoteA2A/WorkIQ connection support (A2A protocol tools) 2. Asset Catalog API integration for --from-catalog picker Key findings: - RemoteA2A is already an ARM-registered ConnectionCategory - CustomKeys and None auth types already work, OAuth2 blocked on SDK - normalizeKind alias required for list --kind filtering (case-sensitive) - Catalog integration is feasible with ~3-4 day effort Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new design spec documenting proposed follow-up work for the azd ai agent connection feature set: (1) RemoteA2A/WorkIQ connection support and (2) Asset Catalog-backed connection creation via a --from-catalog picker. This lives under cli/azd/docs/design/ alongside other azd design documents.
Changes:
- Document RemoteA2A connection category behavior, constraints, and a proposed
remote-a2akind alias for filtering. - Document an Asset Catalog integration proposal (
--from-catalog) including UX sketch, API field mapping, and rollout phases. - Add testing plan and open questions for the above follow-up work.
|
|
||
| ### 2.2 Current State | ||
|
|
||
| From E2E test fixtures (`yacflow/tests/fixtures/`), real `RemoteA2A` connections exist with three auth types: |
|
|
||
| ### 3.2 Catalog API Summary | ||
|
|
||
| **API:** `POST https://api.catalog.azureml.ms/asset-gallery/v1.0/tools` |
| #### Phase 1 — Immediate (small, no blockers) | ||
|
|
||
| **File: `internal/connections/cmd/connection.go`** | ||
|
|
||
| 1. **Add `remote-a2a` alias to `normalizeKind()`** (~1 line): |
|
|
||
| 1. **Add `remote-a2a` alias to `normalizeKind()`** (~1 line): | ||
| ```go | ||
| // In the kind normalization map (line 773-782): |
🔗 Linked Issue RequiredThanks for the contribution! Please link a GitHub issue to this PR by adding |
| User creates connection (category=RemoteA2A, target=<a2a-endpoint>) | ||
| → Agent definition references WorkIQPreviewTool(projectConnectionId) | ||
| → Agents service calls GetWorkspaceConnectionWithSecrets() | ||
| → Transforms to remote tool args: { protocol: "a2a", serverLabel, projectConnectionId } |
There was a problem hiding this comment.
nit: it wont be translated into remoteTool connection and doesn't use server_label
|
|
||
| | Auth Type | Example Connection | Target | | ||
| |-----------|-------------------|--------| | ||
| | `CustomKeys` | `testa2ahelloworld-apikey` | `https://a2a-samples-helloworld-apikey.calmforest-80564e74.eastus2.azurecontainerapps.io` | |
There was a problem hiding this comment.
it supports the same auth types as remoteTool: no auth, custom keys, oauth2, userentratoken, agenticidentity, etc
| | `--kind RemoteA2A` | ✅ Yes | ARM already accepts `RemoteA2A` as a valid `ConnectionCategory`. CLI's `normalizeKind()` passes unknown kinds through as-is. | | ||
| | `--auth-type custom-keys` | ✅ Yes | Already supported — maps to `CustomKeysConnectionProperties` | | ||
| | `--auth-type none` | ✅ Yes | Already supported — maps to `NoneAuthTypeConnectionProperties` | | ||
| | `--auth-type oauth2` | ❌ Not yet | ARM Go SDK v2.0.0 does not expose an `OAuth2AuthConnectionProperties` struct | |
There was a problem hiding this comment.
what is needed to add this?
| > `list`/`show` output displays `Kind: RemoteA2A` instead of `Kind: RemoteTool`. | ||
| > Same code paths, same auth structs, same credential handling. | ||
|
|
||
| #### Phase 2 — Future (blocked on ARM SDK) |
There was a problem hiding this comment.
is this impacting both remoteTool and remoteA2A connection?
| | Auth scheme | `customProperties.xMsSecuritySchemes` | `--auth-type` (needs mapping layer) | | ||
| | Kind | All tools are `kind=mcp` | `--kind remote-tool` (all MCP) | | ||
|
|
||
| **Current catalog stats (as of testing):** |
There was a problem hiding this comment.
there are two registries we want to support this one and the connector registry
|
|
||
| | Issue | Impact | Mitigation | | ||
| |-------|--------|------------| | ||
| | Endpoint URL missing for 9/37 tools | Can't auto-fill `--target` | Prompt user to enter manually; show warning | |
There was a problem hiding this comment.
im ok with this - these are local mcp servers customers need to host
| | Issue | Impact | Mitigation | | ||
| |-------|--------|------------| | ||
| | Endpoint URL missing for 9/37 tools | Can't auto-fill `--target` | Prompt user to enter manually; show warning | | ||
| | Auth schemes are vendor-specific strings | Can't reliably map to `--auth-type` | Best-effort mapping + manual override prompt | |
There was a problem hiding this comment.
there is some logic UX has implemented to tell the auth type, i can share that
| |-------|--------|------------| | ||
| | Endpoint URL missing for 9/37 tools | Can't auto-fill `--target` | Prompt user to enter manually; show warning | | ||
| | Auth schemes are vendor-specific strings | Can't reliably map to `--auth-type` | Best-effort mapping + manual override prompt | | ||
| | Catalog only has MCP tools (no A2A) | `--from-catalog` won't help with WorkIQ/A2A | Document that A2A connections are created manually | |
Key corrections based on Linda's review and validation: - OAuth2 is NOT blocked on ARM SDK (struct exists, just needs wiring) - Added full auth type inventory (12 SDK types + 2 SDK-missing) - Fixed server-side flow: transform produces remote_protocol, not remoteTool - RemoteA2A supports same auth types as RemoteTool (per Linda) - Added connector registry as second catalog source (per Linda) - Restructured spec into 4 requirements with phased plan - Updated testing plan for auth type coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
POC tested against hosted-agents-bugbash (northcentralus): - RemoteA2A + None: PASS - RemoteA2A + CustomKeys: PASS - OAuth2 + client-id/client-secret: PASS - AAD: REJECTED by ARM (not valid for RemoteTool/RemoteA2A) - ManagedIdentity: REJECTED by ARM (maps to RegistryIdentity) Key finding: ARM explicitly lists valid auth types per connection kind. AAD and ManagedIdentity are NOT applicable for RemoteTool/RemoteA2A. Removed them from Phase 1 plan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
left a comment
There was a problem hiding this comment.
Review Summary
Solid, well-researched design spec. Technical claims verified accurate against the existing codebase (RemoteA2A is ARM-registered, CustomKeys/None auth already work, OAuth2 SDK gap is real, and the normalizeKind() alias requirement is correct). Structure follows existing cli/azd/docs/design/ conventions and the PR description aligns with the doc content. Ready for implementation once the readability issue below is addressed.
🟠 High — Mojibake throughout the document
The file contains genuinely corrupted multi-byte sequences (not a display artifact). Byte-level inspection confirms CE 93 C3 87 C3 B6 ("ΓÇö") where UTF-8 em-dash (E2 80 94, "—") belongs — the classic UTF-8-misinterpreted-as-cp1252-then-re-encoded pattern.
Affects the title (line 1) and dozens of lines throughout — tables, tree diagrams, bullets, checkmark columns.
Suggested fix: Re-save the file as UTF-8 from a clean source, or run a find/replace pass:
| Mojibake | Should be |
|---|---|
ΓÇö |
— (em-dash) |
→ |
→ (right arrow) |
✅ |
✅ |
❌ |
❌ |
Γö£ΓöÇΓöÇ |
├── |
ΓööΓöÇΓöÇ |
└── |
🟡 Medium — remote-a2a alias framing is ambiguous
Section 2.4 / Phase 1 reads as if the normalizeKind() alias for remote-a2a already exists or just needs to be documented. It's actually a new mapping entry being proposed. Consider stating explicitly: "Add a new mapping entry "remote-a2a": "RemoteA2A" (does not currently exist in the map)."
🟡 Medium — Use full file paths to disambiguate extension vs. core
The doc uses shorthand like internal/connections/cmd/connection.go without making it clear this lives in the cli/azd/extensions/azure.ai.agents/ extension, not core azd. A reader new to the codebase could easily look in the wrong place. Recommend full paths, e.g., cli/azd/extensions/azure.ai.agents/internal/connections/cmd/connection.go:normalizeKind().
🟢 Nits (non-blocking)
- A few long lines and table cells wrap awkwardly — minor readability nit.
- Linda's quoted requirements in Section 3.2 would benefit from an inline link to the PR #8174 thread for traceability.
What's good
- Technical accuracy verified against actual code (ConnectionCategory registration, auth-type handling, OAuth2 SDK gap, case-sensitive
--kindmatching). - Follows existing
cli/azd/docs/design/doc conventions (Background → Requirements → Implementation → Testing → Open Questions → Phased rollout). - Clear phase breakdown makes downstream PR scoping straightforward.
- PR description fully aligned with doc contents.
POC Round 2 — raw REST validated against live workspace: - AgenticIdentityToken: PASS (both RemoteTool and RemoteA2A) - UserEntraToken: PASS (audience field stored correctly) - ProjectManagedIdentity: PASS (no credentials needed) - AgenticIdentity: REJECTED (ARM expects AgenticIdentityToken) Key findings: - agent.yaml schema uses AgenticIdentity but ARM expects AgenticIdentityToken - Connector registry = same Asset Catalog API (not a separate service) - Connectors are OAuth2-managed MCP servers in Foundry Connector Namespace - All identity-based auth types feasible via raw REST bypass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
left a comment
There was a problem hiding this comment.
Re-Review (3 new commits since prior review)
Scope has expanded from docs-only to docs + implementation code (connection.go +62/-3). Flagging items that are either new since my prior review or remain unresolved.
Status of prior findings
| Prior finding | Status |
|---|---|
🟠 Mojibake (ΓÇö, ΓåÆ, Γ£à, Γö£ΓöÇΓöÇ) |
Not fixed — byte-level check still shows CE 93 C3 87 C3 B6 where UTF-8 E2 80 94 (—) belongs |
🟡 remote-a2a alias framing |
Addressed |
| 🟡 Full file paths | Partially — now "extension's connection.go" but still no full path |
| 🟢 Long lines / #8174 link | Partially — link present; ~48 lines still >100 chars |
🔴 Blocking
1. Mojibake still throughout the design doc
Three commits later, the encoding hasn't been touched. Recommend re-saving the file from a UTF-8-clean source (or find/replace pass):
| Mojibake | Should be |
|---|---|
ΓÇö |
— |
→ |
→ |
✅ |
✅ |
❌ |
❌ |
Γö£ΓöÇΓöÇ |
├── |
ΓööΓöÇΓöÇ |
└── |
2. files/pr-8174-review-fixes-findings.md looks accidentally committed
This file is at repo root in a non-standard files/ directory and its content documents review fixes for a different PR (#8174). It does not belong in this PR. Suggest removing it.
🟠 High
3. Title/description no longer reflects scope
The PR is no longer docs-only — it now ships OAuth2 flag validation and a remote-a2a alias in connection.go. Consider:
- Renaming title from
docs:→feat:(or split impl into a sibling PR). - Adding a "what this changes in code" section to the description so reviewers and the changelog catch it.
4. No tests for the new code paths
connection_test.go is untouched, but connection.go adds:
- New
--client-id/--client-secretflag wiring. - A new validation branch (
if a.flags.authType == "oauth2") requiring both flags. - A new
"remote-a2a": "RemoteA2A"entry innormalizeKind().
Each of these is a new behavioral branch that should have a unit test in connection_test.go per the extension's testing conventions (see cli/azd/extensions/azure.ai.agents/AGENTS.md).
5. Asset Catalog API method inconsistency (design doc ~line 335)
Doc shows POST https://api.catalog.azureml.ms/asset-gallery/v1.0/tools, but the surrounding "Tested Findings" describes a list/read operation. Looks like it should be GET (or please clarify what's being created if POST is intentional). Also flagged by copilot-pull-request-reviewer and unresolved.
6. Non-existent yacflow/tests/fixtures/ path (design doc ~line 54)
There is no yacflow/ directory in this repo. Either link to the actual location (different repo?) or remove the citation. Also flagged by copilot-pull-request-reviewer.
🟡 Medium
7. OAuth2 flags silently ignored for non-oauth2 auth types
--client-id / --client-secret are accepted as global flags but only validated when --auth-type oauth2. If a user passes them with another auth type they're discarded with no warning. Consider mirroring the existing OAuth2-only guard so misuse fails loudly.
8. File path disambiguation still incomplete (design doc)
The doc still references connection.go without the full cli/azd/extensions/azure.ai.agents/internal/connections/cmd/connection.go prefix. A reader new to the repo can't navigate directly.
9. Hard-coded source line numbers in the design doc
Per copilot-pull-request-reviewer: line-number references (e.g., "line 773–782") drift quickly. Reference function or map names instead.
🟢 Nits
- ~48 lines still exceed 100 chars; minor readability nit.
- Once OAuth2 auth type lands, consider noting it in
docs/reference/feature-status.mdso the supported auth-type matrix stays current.
Code-side spot-check (cli/azd/extensions/azure.ai.agents/internal/connections/cmd/connection.go)
What looks good:
- OAuth2 validation logic is clean and uses an actionable error.
remote-a2aalias matches the design and addresses the case-sensitivity issue called out in the doc.- Flag registration uses modern Go patterns.
What needs follow-up: items 4 (tests) and 7 (silent flag ignoring) above.
- Revert connection.go changes (POC code, belongs in implementation PR) - Remove accidentally committed files/pr-8174-review-fixes-findings.md - Fix yacflow path: clarified as vienna repo path - Fix catalog API method: POST with filter body (not GET) - Use full file paths (cli/azd/extensions/azure.ai.agents/...) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
left a comment
There was a problem hiding this comment.
Follow-up — cleanup commit 21c93a74 verified
Nice cleanup pass. Confirming what's resolved and what remains.
✅ Resolved
- POC code in
connection.goreverted — PR is docs-only again, title now matches scope files/pr-8174-review-fixes-findings.mdremoved- yacflow path clarified as vienna repo path
- Catalog API method clarified: POST with filter body
- Full file paths now used for
connection.goandfoundry_projects_client.go
🔴 Still blocking — mojibake encoding
The markdown file's UTF-8 is still corrupted. Byte-level check on the current branch HEAD:
- 109 occurrences of the mojibake byte signature
CE 93 C3 …(theΓ…garbage sequences) - 0 occurrences of proper UTF-8 em-dash
E2 80 94(—) - The very first line (the doc title) still contains
CE 93 C3 87 C3 B6whereE2 80 94should be
This means on github.com, readers see literal —, →, ✅, ├── in the title, section headers, tables, and tree diagrams. None of the 5 commits have touched the file's encoding.
Suggested one-shot fix: open the file in VS Code (or any editor with UTF-8 support), do a find/replace pass on the 6 patterns below, save as UTF-8 without BOM:
| Mojibake | Replace with |
|---|---|
ΓÇö |
— |
→ |
→ |
✅ |
✅ |
❌ |
❌ |
Γö£ΓöÇΓöÇ |
├── |
ΓööΓöÇΓöÇ |
└── |
Once that lands, this should be good to go from a doc-review standpoint.
…tegration
Covers two follow-up requests on PR #8174:
Key findings: