--- name: extension-provider-onboarding description: Use when porting an OpenConnector provider, REST API, or MCP service into a AgentTeam Builtin Extension under internal/extension/providers. --- # Extension Provider Onboarding ## Scope Onboards one or more providers as trusted Builtin Extensions under `internal/extension/providers//`. The currently implemented shape is bounded REST over HTTPS with JSON, form, text, or empty responses and `no_auth` / API-key / custom-header-token / HTTP Basic credentials (including dynamic username+token Basic). A self-hosted provider may declare its exact HTTPS base URL as a non-secret App connection field when only workspace administrators can write that connection. Cache clients by canonical URL, resolve DNS on every dial, and use the private-origin guarded transport: it may reach RFC1918/IPv6 ULA addresses, but loopback, link-local, CGNAT, multicast, unspecified, and cloud metadata addresses remain mandatory denies. OAuth (authorize/callback/refresh) is NOT implemented — the `runtime.AuthSpec` contract reserves the extension point; do not wire OAuth providers yet. Upstream reference: the OpenConnector clone with the pinned baseline `providers.OpenConnectorBaseline` in `internal/extension/providers/providers.go` (bump it when ports move to a newer upstream commit). The maintained provider gap matrix — including upstream/current MCP mode and concrete credential placement — lives in [docs/research/openconnector-agentteam-provider-comparison.md](../../docs/research/openconnector-agentteam-provider-comparison.md); update it in the same PR as every port. ## Choose REST, MCP, or a hybrid split Classify from executable upstream behavior, not names or protocol fragments: - JSON-RPC, SSE, a `tools` field, or a repository named `mcp` is not enough to call a provider MCP. MCP requires session negotiation plus MCP tool/resource semantics. - A REST+MCP upstream is **hybrid**. Split its action families and record both upstream and current AgentTeam modes in the matrix; do not label the whole provider non-MCP when AgentTeam ports only its REST subset. - Record the concrete credential wire placement (`Bearer`, named header, query, body, Basic, HMAC/signature, OAuth, or custom/mixed). Preserve the coarse upstream `authTypes` separately. Use `未核` instead of inferring a placement from `api_key` or `custom_credential`. REST providers use the shared fixed-provider client described below. MCP-backed actions use only `internal/extension/runner/mcp`; a provider never imports the SDK or creates its own MCP client, session, transport, retry agentteam, SSE reader, or cleanup path. If the upstream requires runtime-discovered MCP endpoints, stop for an Extension-platform design review rather than building a provider-local exception. ### Shared MCP architecture For every MCP-backed provider: 1. Construct `mcp.Client` definitions from exact, code-owned HTTPS endpoints, auth contracts, and `AllowedTools` and/or `AllowedResourceURIs`. At least one allowlist is non-empty. Endpoint, server, tool name, and resource URI are never Agent input. 2. Expose static semantic Actions only. Never expose `list-tools`, generic `call-tool`/`read-resource`, SDK types, or JSON-RPC envelopes. 3. Use the parent runtime's guarded auth/egress seam. The reusable client is immutable and concurrent-safe; each invocation gets a fresh credential snapshot, SDK client, transport, session, timeout, protocol state, request count, and cumulative byte budgets. 4. Keep client capabilities empty. Disable retries, standalone SSE, and automatic multi-round-trip behavior. One total invocation deadline covers connect, negotiation, the allowlisted operation, and any SDK-issued cancellation POST; POST-SSE responses remain allowed. 5. Accept only reviewed protocol versions. The legacy initialize fallback must clear stale modern protocol-version state before session cleanup. 6. Map HTTP, JSON-RPC, protocol, timeout, malformed result, `NeedsInput`, and tool `IsError` failures into the closed runtime taxonomy. Never retain raw SDK, endpoint, session, credential, header, payload, or upstream-body causes. 7. Attempt one independently bounded DELETE whenever a session ID was seen, including Connect failure, operation failure, timeout, cancellation, and exhausted invocation budgets. Cleanup uses detached context and independent request/response budgets; accept 2xx/404/410. Cleanup failure, including 500, never overwrites a completed operation/result or its primary error. 8. When no reviewed read-only tool/resource exists, connection validation may use a `ProbeOnly` `mcp.Client`: it carries no tool/resource authority and performs Connect + protocol negotiation + cleanup only, never `tools/list` or another dynamic discovery operation. Probe every fixed endpoint required by the selected topology. 9. If validation chooses among pre-registered static topologies, persist only the bounded code-owned enum through the generic connection config seam. The provider must declare the exact key and complete ASCII value enum at startup; validate it before persistence, after DB decode, and after runtime resolution. Executors exhaustive-switch the enum and never interpret it as an endpoint/tool/resource. Browsers cannot supply/read it; never store endpoint/tool/resource/session, profile, or secret-derived metadata. MCP providers still use the normal `extension.Extension` assembly, static `Definition`, executor dispatch, registration, App seeding, digest pins, and connection lifecycle below. Their executor calls only `mcp.Client.CallTool` or `ReadResource`; the REST client/handler bullets do not apply to MCP actions. The platform contract and reviewed dependency/license boundary are authoritative in [specs/extension/extension-platform.md](../../specs/extension/extension-platform.md#59-mcp-backed-builtin-extensions). ### MCP provider test checklist - Constructor and call reject invalid/duplicate/Agent-derived allowlists, endpoints, tool names, and resource URIs before network access. - Wire tests cover JSON and POST-SSE responses, modern and legacy negotiation, endpoint/origin guards, redirects, forbidden GET, and credential injection. - Budget tests cover request count, cumulative request/response bytes, final output, timeout, detached SDK cancellation, and typed sanitized errors. - Lifecycle tests cover Connect failure with a session ID and cleanup after success/failure/cancellation/exhaustion; successful results survive cleanup 404/410/500. - Concurrency tests prove credential, session, protocol-version, counter, and byte-budget isolation. - Boundary tests pin the reviewed SDK version/license, reject `replace`, and fail any SDK import outside `internal/extension/runner/mcp`. ## Non-negotiable wire rules These came out of review; every one of them was a real defect once. 1. **Port the upstream contract verbatim.** Input/output property names keep upstream casing exactly. Do not re-wrap payloads (`{page: ...}` boxes, camelCase renames of `next_cursor`/`block_children`, etc.) — return the upstream shape, synthesizing only the envelopes the upstream executor synthesizes. 2. **Three-state nullability.** Absent omits the field; explicit `null` forwards `null`; a value forwards the value. Decode nullable update fields as `json.RawMessage` (or a `Nullable[T]`), never `*T` — a typed nil boxed in `any` marshals as `null` and collapses the states. 3. **Schema and handler must accept/reject the same set.** If the schema allows `integer|string`, the handler must decode both losslessly (see the todoist `integerLike` type: number-or-1-18-digit-string, `big.Rat` exact numeric semantics, no `ParseFloat`). Prove it with a table test that runs the binding validator AND the executor against the same lexicals. 4. **Action names are kebab-case** (registry contract); upstream snake_case ports as `get_current_user → get-current-user`. Safety classes are the closed set `read` / `mutation` / `destructive` (not "write"). 5. **Every schema root carries `$schema` "https://json-schema.org/draft/2020-12/schema"** or catalog validation rejects it. 6. **Declare what you return.** Schemas must match real output: nullable fields where the API really returns null, loose objects for passthrough envelopes. Fail closed on malformed 2xx bodies you synthesize envelopes from (null feed → error, not `{"story_ids":null}`). ## REST provider package layout ``` internal/extension/providers// ├── extension.go # Domain/Revision/BaseURL consts, Extension(), AuthSpec(), ValidateCredentials ├── definition.go # Definition() catalog — keep under ~120 lines, split per resource group ├── schemas.go # schema builders (dialect on every root) ├── handlers.go # Handlers() dispatch table + per-action input structs/builders └── tree|client.go # optional: multi-step assembly (see hackernews/tree.go) ``` REST `extension.go` essentials: - `Revision` starts at `-rest-v1`; bump ONLY when a released implementation contract changes (review iterations before merge do not count — squash-merge erases them). - `runtime.NewRESTClient` with the fixed or connection-selected exact `https` base URL; the client confines every request to that origin, follows no redirects, bounds responses, and applies credentials AFTER all other headers. Form/text/empty behavior and exposed response headers must be declared in a code-defined `RESTResponseSpec`, never Agent input. - `runtime.Implementation{Executor, Revision, ConnectionPolicy, Auth, ValidateCredentials}` — authenticated providers use `registry.ConnectionRequired`; no-auth providers `ConnectionNone` with a zero `AuthSpec`. - `ValidateCredentials` does one bounded authenticated GET and requires a non-empty identity field. Register in `providers.Extensions(deps)` (add to the aggregate list) and let `bootstrap.Register` fail closed on inconsistency. ## Recursive / fan-out actions Any action that walks graphs or expands lists needs per-invocation budgets that fail closed, not just per-response limits (see hackernews tree): - fetch/node COUNT caps (soft: truncate with honest flags), - cumulative RAW upstream bytes (charge `client.CallWithMetrics`'s second return on every fetch — trimmed payload size is spoofable by whitespace), - final output size cap before returning, - wall-clock budget where only the action's OWN deadline maps to the budget error; caller cancellation/deadlines and client timeouts propagate with their original classification. ## App seeding For each new provider App: 1. `internal/store/app/registry/product_apps.go`: App ID/slug constants + map entry. 2. `internal/store/app/installation_extensions.go`: `app_` case in `builtinExtensionCapabilityConfig` with the digest pin constant. 3. `internal/store/model/extension_capability.go`: `BuiltinExtensionDefinitionDigest` constant. 4. ONE migration per PR (next number) seeding `apps`, `app_versions`, `service_definitions` (SELECT-based), `service_capabilities` with the extensions capability JSON — copy the block shape from `migrations/00339_extension_provider_apps.sql` and mirror the rows into `internal/tidb/bootstrap.sql`. Do NOT rename pre-existing user rows that already own the slug: the canonical inserts fail the migration through the unique slug / identity guards so operators resolve the collision by hand. Compatibility SQL that renames or rewrites published user data is allowed only when there is real already-published compatible data AND an approved migration design. 5. Icon in `web/public/app-icons/.svg|png` + provenance line in that directory's README. 6. Add the App slug to the fixed Product App catalog expectation in `TestUnlistedAppCatalogVisibilityIsCreatorScopedForMembers`; this TiDB-only integration assertion will otherwise fail after the seed lands. **Computing digest pins**: register via `providers.Extensions` + `bootstrap.Register` in a throwaway test, print `reg.LookupByIdentity(domain, 1).Definition.Digest`, paste into the three places. `TestBuiltinDigestsMatchPinnedAppCapabilities` then enforces the pin forever — any catalog edit must re-pin. ## Tests (minimum bar that passed review) - Contract: `Handlers()` keys == `Definition()` action names; digest pin == registered digest; Distribution == product app registry. - Wire table: EVERY action against a TLS loopback double asserting method, path, query, body fragments — including absent-null-value cases for each nullable field and any normalization the baseline performs. - Error classification: 401/403→provider_auth_unavailable, 404→not_found, 429→rate_limited, 5xx→upstream_failure; malformed 2xx bodies fail closed. - Budgets (if applicable): adversarial origins (self-cycles, whitespace padding, wide/deep trees) with request counts asserted at the double. - TiDB: migration applies; seed contract (migration + bootstrap tokens); connection lifecycle (install → digest-pinned selection → AES-GCM connect → runtime resolve → peer forbidden → delete) — extend `extension_provider_connections_integration_test.go` patterns. - Self-hosted connection UI: App settings exposes non-secret URL/username values only to authorized administrators; secrets stay write-only. Changing the URL or another non-secret auth field must require an explicit new secret so an old token is never forwarded to a different target. ## CI ratchets — hard limits (do not grow baselines) `make harness-fast` ratchets bit us once; design within them: - production files < 1000 lines, test files < 800 lines (split files), - functions < 120 lines (split `Definition()` per resource group or share a builder for repetitive entries), - no new single-statement delegate helpers (`return f(...)` bodies count as thin wrappers; use a named intermediate if the helper is genuinely needed), - no `time.Sleep` in tests — use `<-time.After` / `time.AfterFunc`, - required CI shard list `scripts/ci-backend-required.sh` must cover every new package (run `... validate` with `AGENTTEAM_TIDB_MIGRATION_BASE_REF` set). ## Process 1. Work in a dedicated worktree branch off latest `origin/main`. 2. Incremental commits only — NEVER squash/force-push during review; the merge flow auto-squashes. 3. Open the PR with `tea`, title in Conventional Commits describing the net change; subscribe it to the coordination thread (`agentteam exec gitea watch-pull-request '{"pullRequest":"pingcap/agentteam#", "target":"","notifyPrincipal":true}'`). 4. No repo-wide sed/gofmt — exact-prefix replacements, explicit file lists, `git diff --name-only` scope audit, build ALL four modules (root, desktop, presets, x/agent-git-service/cli). 5. After review approval, `/merge` as a PR comment via `tea comment /merge`; never merge manually.