diff --git a/.claude/skills/to-acceptance-plan/scripts/check-acceptance-plan-test.sh b/.claude/skills/to-acceptance-plan/scripts/check-acceptance-plan-test.sh index f455110e5f..4b530fbcaf 100755 --- a/.claude/skills/to-acceptance-plan/scripts/check-acceptance-plan-test.sh +++ b/.claude/skills/to-acceptance-plan/scripts/check-acceptance-plan-test.sh @@ -113,6 +113,13 @@ sed_in_place 's/\*\*Expected tasks:\*\* deferred to orchestration/\*\*Expected t sed_in_place 's||**Combined rationale:** The fixture is one indivisible documentation check, so separate plan review adds no value.|' "$combined_valid" bash "$checker" "$combined_valid" >/dev/null +# Category extraction must survive an interface block larger than a pipe buffer. +for source in "$valid" "$combined_valid"; do + large="$root/acceptance/large-$(basename "$source")" + awk '/^### Scenario/ { for (i = 0; i < 4096; i++) print "Additional interface detail for the large-block fixture." } { print }' "$source" >"$large" + bash "$checker" "$large" >/dev/null +done + for case_name in missing-contract invalid-contract missing-category bare-none placeholder bad-delivery bad-status proposed-unchecked missing-human placeholder-human checked-without-decision; do cp "$valid" "$root/acceptance/$case_name.md" done diff --git a/docs/acceptance/README.md b/docs/acceptance/README.md index a9c6a9ad86..f61a90b4de 100644 --- a/docs/acceptance/README.md +++ b/docs/acceptance/README.md @@ -155,6 +155,7 @@ PR after verification. There is no cleanup or status-only PR. attributable Linux and macOS command/job temporary storage with a permission-visible system escape and deterministic crash-residue reaping. Status: landed. - [Session title generation and token usage](session-title-generation.md) — mecatui `/title`, an opt-in routed model title after up to three genuine prompts, and durable title-model token attribution. Status: draft. +- [MCP source reconciliation](mcp-source-reconciliation.md) — bounded ToolHive polling, MCP notifications, and manual refresh publish immutable direct MCP runtimes while preserving exact-name authority; explicit owner refresh unions additions. Status: proposed. - [Mecatui-owned configurable terminal titles](mecatui-terminal-title-controller.md) — replaces Bubble Tea title emission with a renderer-serialized OSC 0 controller, user-global plain-text title templates over display-safe status facts, explicit disablement precedence, and live-run `/session` identity access. Status: proposed. - [Per-upstream MCP broker OAuth grants](mcp-broker-multi-upstream-oauth.md) — accept multiple broker OAuth upstreams while keeping grants, callback state, authenticated discovery, and workspace-enrollment progression backend-scoped. Status: draft. - [Broker MCP status](broker-mcp-status.md) — approved owner-scoped broker connector diff --git a/docs/acceptance/mcp-source-reconciliation.md b/docs/acceptance/mcp-source-reconciliation.md new file mode 100644 index 0000000000..ec7aaad045 --- /dev/null +++ b/docs/acceptance/mcp-source-reconciliation.md @@ -0,0 +1,127 @@ +# MCP source reconciliation — acceptance plan + +**Contract:** human-reviewed/v2 +**Work classification:** Architectural — changes process-wide MCP publication, runtime ownership, a public refresh control, and one importable aggregate API. +**Decision record:** [ADR 0355](../adr/0355-mcp-source-reconciliation.md) +**Phase:** Minimal stale direct/global MCP source reconciliation +**Status:** proposed, 2026-09-16. Directing-human decisions are settled; ready for amended Plan / Interface review. +**Baseline synchronization:** merged `origin/main` at `36a618a621b5ec53b593c72930bc229e60c2adf7`; the proposed technical contract is unchanged, while its Proposed ADR and two ADR-named verification functions now use the next free number, 0355, after synchronized main allocated 0351 through 0354. +**Delivery:** Split. Runtime publication and additive public controls require contract review. The directing human explicitly authorizes a stacked implementation PR before this Plan PR merges, based on the exact amended plan commit and targeting `plan/mcp-source-reconciliation`; this does not approve or merge either PR, and contract-drift gates remain. +**Expected tasks:** deferred to orchestration after parent advisory review. +**Issue:** [#1511](https://github.com/stacklok/mecatl/issues/1511) — tracks the work; this plan does not close it. +**Plan PR:** [#1527](https://github.com/stacklok/mecatl/pull/1527) +**Approved baseline:** absent until the Plan / Interface PR merges; the authorized stacked implementation records the exact proposed plan commit it follows. + +This amendment replaces the prior digest/binding/revocation design with the original stale-source scope. It preserves current exact-name authority semantics: disappearance filters availability without deleting the durable grant, exact-name reappearance remains authorized, and same-name endpoint/schema/read-only drift remains the same name grant. Automatic ToolHive polling, notification-driven reconciliation, and explicit `/mcp-refresh` additions remain required. + +## Human decisions + +- [x] Authority semantics — Decision: retain existing exact-name `session.Authority.CapabilitySet.Tools`; add no digest, `DirectMCPBinding`, revoked-record list, migration, or credential-free URL policy. Current availability filters durable grants. +- [x] Removal and reappearance — Decision: successful source removal is automatically absent at the next operation boundary; stale calls get a permanent unavailable result. The durable name may remain, and exact-name reappearance is authorized. This is not permanent revocation. +- [x] Additions — Decision: only explicit owner `/mcp-refresh` stable-unions current active direct MCP names into the existing ceiling, preserving unrelated names. Refresh is not durable direct-subset replacement. +- [x] Triggers — Decision: bounded jittered polling is enabled only for ToolHive; MCP list-change notifications and manual requests share one serialized/coalesced bounded reconciler. +- [x] Source failure — Decision: consultation failure retains source LKG; successful empty is authoritative withdrawal. +- [x] Candidate failure — Decision: candidates are complete/all-or-nothing. Any desired-server construction failure closes the candidate and retains the previous runtime; this may delay an unrelated removal, reported through the existing `stale` flag, until a later retry. +- [x] Runtime ownership — Decision: one immutable current direct runtime plus bounded retiring runtimes; operation/run pins protect active use and are inherited by delegation. Revision tags rebuild stale shared/cached engines; idle engine existence does not lease a runtime. +- [x] Refresh lifecycle — Decision: idle and quiescent completed ordinary roots are eligible without reopening completed state. Failed/cancelled and every active/pending/non-root form are rejected. Mutation exclusion and confirmed save are required only when the union adds authority. +- [x] Unified UX — Decision: `/mcp-refresh` capability-routes to direct `RefreshMcpSources` or existing broker `ConnectWorkspaceServices`; broker consent/cancellation/disclosure remains distinct, and `/tools-connect` remains a deprecated broker-only alias. + +## Interface contract + +- **gRPC / protobuf:** Add unary `HarnessService.RefreshMcpSources(RefreshMcpSourcesRequest) returns (RefreshMcpSourcesResponse)`. Request: `string session_id = 1`. Response: `uint64 revision = 1`, `bool changed = 2`. `revision` identifies the successfully pinned active runtime snapshot whose direct names were considered for the request (and, when needed, granted); it need not still be the latest revision when delivery completes. `changed` compares the request's observed active snapshot before and after its shared reconciliation cycle and reports whether that cycle published a different runtime or the precise authority union added names; a coalesced successor that supersedes the pinned snapshot does not rewrite this request's response. Add direct-only `ServerCapabilities.mcp_refresh = 30`; field 30 is unallocated on synchronized `origin/main` (`ca993a6ea1b648e8e18a102b6810da5dea8c494f`). Extend `ListMcpSourcesResponse` with `uint64 revision = 2`, `bool stale = 3`, and `bool reconciling = 4`; source rows remain the current published/pre-shadow inventory and the call performs no independent probe. `stale` also covers candidate/source degradation; no separate absent `degraded` field is implied. Add bodyless HTTP `POST /v1/sessions/{id}/mcp-refresh`. Existing broker RPC/HTTP/capability surfaces are unchanged. +- **Exported Go APIs / interfaces:** Add `engine/session.(*Session).GrantToolAuthority([]string) error`, additive idempotent `engine/agent.(*Supervisor).Close()`, and `engine/agent.ErrSupervisorClosed`. `GrantToolAuthority` clones the input and validates every added name as non-empty, valid UTF-8, control-free, and at most 256 bytes, then stable-unions it into already-bound authority; duplicate input or existing names are idempotent. A rejected batch is nonmutating, and unrelated existing names are neither revalidated nor altered. It preserves unrelated authority axes and all other aggregate state; accepts only idle or completed state with no pending control; and never reopens completed state. `Supervisor.Close` cancels an active run, waits for settlement, and performs fork, inline-MCP closer, cancellation, registry/liveness, and other enrolled-member cleanup exactly once across repeated or concurrent calls; `Run` uses the same close path. Enrolment after close or run start returns `ErrSupervisorClosed`. Ordinary-root/provenance/owner checks stay in Service/composition. No new authority field, digest helper, agent run option, source port, or ToolHive import enters `engine/`. +- **Tool schemas:** No model-visible tool is added or changed. Current catalog membership intersects existing exact-name authority. An authority-granted but catalog-absent call returns the exact permanent error `tool is currently unavailable; do not retry unless the catalog changes`; an ungranted absent name retains existing unknown-tool behavior. +- **CLI / config:** Add no flag, key, URL rule, selector, credential input, or poll tuning. ToolHive discovery alone enables bounded automatic polling. Mecatui adds argument-free `/mcp-refresh`: direct-only capability invokes only `RefreshMcpSources`; broker-only capability invokes only `ConnectWorkspaceServices` with existing consent/cancellation/disclosure. Both/neither incompatible modes or missing collaborators fail closed. `/tools-connect` remains a deprecated broker-only alias and `/tools-cancel` is unchanged. +- **Events / persistence:** Add no event, snapshot revision, binding, revoked-name record, or migration. `Authority.CapabilitySet.Tools` remains the durable name ceiling. Explicit refresh performs at most one snapshot save only when its stable union adds names. The old and candidate session values remain detached from the published live aggregate. A successful save makes the exact candidate authoritative. Because a save error can be an ambiguous commit, the Service performs a bounded authoritative reload while still holding the same exclusion: exact candidate means durable success, exact old value means durable failure, and mismatch or unavailable confirmation returns an uncertain fail-closed result that never claims authority is unchanged. No speculative candidate is published into a live aggregate, and repeating the same stable union converges. Runtime removal/status/revisions remain process state. +- **Security / authority:** Refresh authenticates ownership before caller-selected locking. Every request takes `runEntryMu` and performs owner, ordinary-root, lifecycle, and same-process liveness checks at its local linearization point. Authority addition additionally requires mutation lease/capability, fresh authoritative load, aggregate union, and authoritative save confirmation. A no-op refresh acquires no mutation capability and performs zero writes. Before an authority-mutating save starts, caller cancellation stops session mutation while shared reconciliation continues. Once save starts it may commit despite cancellation; confirmation detaches from caller cancellation but remains bounded and under the same exclusion, and cancelled delivery never promises rollback. Automatic reconciliation never widens authority or launches consent. Existing endpoint/credential validation and URL projection remain unchanged. +- **Compatibility / migration:** Existing sessions and snapshots require no migration. Existing granted names remain granted even while unavailable. Same-name source/endpoint/schema/read-only drift follows current name semantics. Additive protobuf fields are ignored by old clients. Broker/client MCP behavior and configured-over-ToolHive name precedence remain unchanged. + +## In scope — 4 scenarios, in implementation order + +### Scenario 1 — Ordered sources reconcile automatically and boundedly + +The production Build path constructs one reconciler over the existing ordered source resolver. ToolHive polling, current-runtime list notifications, and manual requests enter the same bounded coalescing path defined by [ADR 0355](../adr/0355-mcp-source-reconciliation.md). + +**Acceptance:** +- AC1.1: Configured/static entries retain precedence over ToolHive collisions and pre-shadow source rows remain visible. Source failure retains LKG; successful empty withdraws that source. The production source-resolution path, not a synthetic merge helper alone, proves all cases. + - verify: `TestMCPSourceReconciliation_Scenario1_OrderedSourcesLKGAndEmpty` +- AC1.2: Polling is absent without ToolHive and bounded/jittered with it. Tool/resource/prompt notifications from current servers mark their source dirty and trigger a list-based candidate refresh; an actual dirty event refreshes every relevant list and publication comparison includes tools, resources, and prompts. A stable ToolHive observation with no dirty signal reuses the existing live snapshot (apart from bounded health retry) and does not reconnect every server merely to inspect unchanged contracts; when ordered sources are unchanged and no source is dirty, the reconciler may no-op before candidate connection. Manual requests share the same serialized/coalesced path, stale-runtime notifications are ignored, and caller cancellation does not cancel shared work. + - named subcases: `unchanged-poll-no-reconnect`, `dirty-refreshes-all-lists`, `resources-prompts-affect-publication`, `stale-notification-ignored`, `caller-cancel-shared-work-continues` + - verify: `TestMCPSourceReconciliation_Scenario1_ProductionTriggerMatrix` +- AC1.3: Source, server, tool/list, cycle, and retained-runtime bounds fail stale with bounded secret-safe diagnostics. Finite Service/reconciler constants separately cap active-list cardinality, total candidate page count and bytes, per-refresh union grant count, and accumulated historical granted names before save; boundary behavior is tested without exporting another engine field or reviving a public/global 512-record bound. Automatic cycles never invoke OAuth/browser consent and shutdown joins polling/reconciliation. + - named subcases: `active-list-bound`, `candidate-page-byte-bound`, `union-grant-bound`, `historical-accumulation-bound`, `no-public-global-512-bound`, `consent-never-launched`, `shutdown-joins` + - verify: `TestADR_0355_ReconciliationBoundsConsentAndShutdown` + +### Scenario 2 — Complete runtimes publish atomically and drain safely + +A candidate builds a complete manager/provider/tool/source snapshot and generation-bound `assembleCatalog` contribution. Publication atomically swaps current runtime only after candidate completion, preserving [ADR 0355](../adr/0355-mcp-source-reconciliation.md)'s consistency boundary. + +**Acceptance:** +- AC2.1: A complete candidate publishes additions and removals, including successful empty. Candidate equality compares bounded runtime metadata/config for tools, resources, prompts, and resolved source/server state—not a durable security/authority digest. An unchanged candidate leaves revision, published engine/factory contribution, caches, and retirement state byte-for-byte unchanged and is closed/discarded if one was built. Changed all-or-nothing state may reconnect the full desired server set at bounded cost. Any connect/initialize/list/validation failure closes the entire candidate, retains the previous usable runtime, marks the same `stale` status, and delays otherwise valid changes until a later successful retry. + - named subcases: `unchanged-candidate-no-publication-side-effects`, `resource-only-change-publishes`, `prompt-only-change-publishes`, `changed-full-reconnect-allowed`, `failed-candidate-stale` + - verify: `TestMCPSourceReconciliation_Scenario2_AllOrNothingPublication` +- AC2.2: The real shared and session factory paths cover shared/default, selector, no-FS, client-MCP, mode-specific, specialist, and debug engines. Revision mismatch rebuilds before use; a debug name ceiling admits no additions and fails when selected names are absent. `assembleCatalog` remains the single complete registration path. + - verify: `TestMCPSourceReconciliation_Scenario2_ProductionEngineRevisionMatrix` +- AC2.3: A production-path race matrix covers prompt start, failed-step retry, restored approval, direct `RunTeam`, Subagent, Parallel, Team member, named specialist, referenced-agent MCP, in-run prompt expansion, and out-of-run list/read/get resource/prompt calls. Each operation observes one runtime across schema, lookup, authority, permission, dispatch, provider, and manager; delegation inherits the root pin. Shared-engine construction captures its manager and revision under one construction-scoped pin, releases that pin when `Build` returns, and cannot publish a mismatched shared-engine tag if reconciliation advances concurrently. Revision zero carried by an operation pin is distinct from no operation pin. Retiring runtimes close once after pins drain. When a full retirement set prevents publication, the candidate is closed/discarded and no runtime is force-closed; the reconciler retains one coalesced successor, marks stale, and retries after drain without polling or allocating candidates without bound. + - named subcases: `retirement-full-defers-publish`, `unpublishable-candidate-closed`, `single-successor-after-drain`, `no-force-close`, `no-unbounded-candidate-allocation` + - verify: `TestMCPSourceReconciliation_Scenario2_RuntimeConsistencyMatrix` + +### Scenario 3 — Name authority filters availability and widens only explicitly + +The current runtime contributes only active direct names. Existing authority remains name-based and the aggregate supplies one narrow union operation under [ADR 0355](../adr/0355-mcp-source-reconciliation.md). + +**Acceptance:** +- AC3.1: A removed granted name is absent from model specs and dispatch, and a stale generated call gets exactly `tool is currently unavailable; do not retry unless the catalog changes`. An ungranted absent name stays an unknown-tool error. Exact-name reappearance automatically becomes available under the existing grant; same-name endpoint/schema/read-only drift requires no regrant. + - verify: `TestMCPSourceReconciliation_Scenario3_NameAuthorityAvailabilityMatrix` +- AC3.2: `Session.GrantToolAuthority` clones its input, rejects the whole batch without mutation unless every added name is non-empty, valid UTF-8, control-free, and at most 256 bytes, and stable-unions accepted names in input order. Duplicate input and already-present names are idempotent. It does not revalidate, reorder, or alter unrelated existing names or authority axes and preserves conversation/history, placement, owner, counters, and the exact lifecycle state; pending controls and states other than idle/completed are rejected without mutation. + - named subcases: `invalid-batch-atomic`, `input-cloned`, `duplicate-stable-union`, `unrelated-legacy-name-untouched`, `name-256-byte-boundary`, `state-preservation` + - verify: `TestADR_0355_GrantToolAuthorityPreservesAggregateState` +- AC3.3: Delegated authority remains the existing name intersection. Children started before publication inherit the pinned old runtime; children started after publication use the new active set. Resume permits an already granted same name when currently available and returns the permanent unavailable result when absent, without digest or migration state. + - verify: `TestMCPSourceReconciliation_Scenario3_DelegationAndResumeNameSemantics` + +### Scenario 4 — Explicit direct refresh and unified UX preserve boundaries + +Direct refresh validates an eligible owned root under run-entry exclusion before reconciling shared state, then adds only missing active direct names. Broker refresh remains the existing enrollment operation in [ADR 0335](../adr/0335-idle-session-broker-workspace-refresh.md). + +**Acceptance:** +- AC4.1: A Service-level matrix covers idle and quiescent completed success without reopening. Every path authenticates owner and takes `runEntryMu` for ordinary-root, state, and same-process liveness checks at the local linearization point. A precise no-op performs zero writes and needs no cross-host mutation lease/capability. An addition then acquires mutation authority, freshly loads detached old state, builds a detached candidate with one aggregate union, and starts at most one save. Direct save success makes the exact candidate durable. On save error, a bounded authoritative reload under the same exclusion distinguishes exact candidate (success), exact old (failure), mismatch (uncertain), and reload unavailable (uncertain); uncertain outcomes fail closed, never claim known-unchanged authority, never publish a speculative live aggregate, and leave model/history untouched. Repeating the same stable union converges. Cancellation before save begins stops session mutation while shared reconciliation continues; after save begins it may commit, bounded confirmation is detached from caller cancellation, and cancelled delivery promises no rollback. All active/running/awaiting/authorizing/failed/cancelled/child/schedule/debug/broker-conflicting sessions remain rejected. The proof uses the real Service control, not direct aggregate calls or a fake pin-only harness. + - named subcases: `no-op-zero-write-no-mutation-capability`, `save-direct-success`, `save-error-reload-exact-candidate`, `save-error-reload-exact-old`, `save-error-reload-mismatch-uncertain`, `save-error-reload-unavailable-uncertain`, `cancel-before-save`, `cancel-after-save-start`, `stable-union-retry-converges` + - verify: `TestMCPSourceReconciliation_Scenario4_ServiceRefreshMutationMatrix` +- AC4.2: gRPC, HTTP, and mecatui direct paths return the revision of the successfully pinned active snapshot whose direct names that request considered/granted, not necessarily the latest revision at delivery. `changed` compares that request's observed snapshot before/after its shared cycle and its exact union; a coalesced successor may supersede the snapshot without changing the earlier response. The matrix covers unchanged, runtime-only change, authority-only change, both, stale candidate, pre-save and post-save-start cancellation, uncertain save confirmation, ownership concealment, and unsupported states. `ListMcpSources` returns cached revision/stale/reconciling, uses the single stale flag for degradation, and performs no probe. + - named subcases: `pinned-revision-response`, `superseded-response-stays-pinned`, `changed-request-local`, `cancelled-delivery-no-rollback-claim`, `stale-not-degraded-field` + - verify: `TestMCPSourceReconciliation_Scenario4_DirectTransportStatusMatrix` +- AC4.3: Mecatui routes direct-only `mcp_refresh` solely to `RefreshMcpSources` without consent and broker-only `workspace_enrollment` solely to `ConnectWorkspaceServices` with destructive-reconnection disclosure, consent/presentation, observation, cancellation, and existing outcomes. Both bits, neither bit, or a missing collaborator fail closed; `/tools-connect` remains broker-only even when direct refresh exists, and client-provided MCP is untouched. + - verify: `TestMCPSourceReconciliation_Scenario4_UnifiedCommandRoutingMatrix` +- AC4.4: Direct-team startup revalidates the exact registry entry while claiming `starting`, so successful concurrent cleanup cannot leave an orphan start or escape the team cap. Before enrollment, a configured atomic member creator must expose supported rollback deletion. A later-member failure invokes idempotent supervisor close without inference/synthesis, deletes every startup snapshot under a detached bounded context only while its mutation lease is still held, then releases leases; cancellation follows the same cleanup path, lease loss refuses deletion, and retry cleans each fork, inline closer, cancel, liveness registration, and snapshot exactly once. Creator-only and explicitly nondeleting stores fail before factory, fork, lease, or snapshot side effects. + - named subcases: `second-member-failure-rolls-back-first`, `cancelled-startup-rolls-back`, `lost-lease-refuses-delete`, `creator-only-rejected-before-enrollment`, `cleanup-run-start-identity-barrier` + - verify: `TestRunTeamConstructionFailureRetainsDeclarationsForRetry`, `TestRunTeamRejectsCreatorStoreWithoutRollbackBeforeEnrollment`, `TestSupervisorCloseIsConcurrentAndIdempotent` + +## Out of scope + +| Item | Defer-to | Decision | +|---|---|---| +| Digest/canonical-JSON capability identity, endpoint identity normalization, `DirectMCPBinding`, revoked records, or legacy migration | Separate security proposal if ever desired | Existing exact-name authority is preserved. | +| Live operator settings/CLI/environment reload | Future operator/admin design | Configured input remains process-start state; only ToolHive is polled. | +| Project-tier MCP | Existing trust/config policy | It remains ignored. | +| Broker/protected MCP and client-provided MCP lifecycle | Existing ADRs/session lifecycle | Only mecatui's preferred command spelling is shared. | +| Partial-active candidate publication or connection reuse | Future optimization with its own ownership proof | Complete candidates retain the previous runtime on any construction failure. | +| Mid-operation catalog/manager mutation | Prohibited | Immutable runtime revisions are the consistency boundary. | +| Poll/bound tuning surface | Not planned | Finite values are internal constants with boundary tests. | +| Shipped architecture/user documentation in this Plan PR | Implementation PR | Living/user docs describe shipped behavior only. | + +## Definition of done + +1. Stacked implementation records the exact amended plan commit and targets `plan/mcp-source-reconciliation`; parent advisory review precedes any plan push. Plan merge remains approval and humans alone merge. +2. The named production-path tests prove all 12 ACs, including automatic ToolHive polling, notifications, LKG/empty, complete-candidate failure, runtime consistency, name reappearance, no-op versus widening refresh, transport parity, and broker/client isolation. +3. The exported engine additions, `Session.GrantToolAuthority`, `Supervisor.Close`, and `ErrSupervisorClosed`, are recorded by `task api:update` and as Added/minor in `engine/CHANGELOG.md`. No other engine API widening is needed. +4. ADR 0027 inventories the poller/timer, reconciler/coalescing state, source LKG/status, immutable current and retiring runtimes, revision-tagged caches, and run/operation pins with restart decisions. +5. Implementation updates `docs/architecture.md`, `docs/architecture/extensibility.md`, `docs/design/IMPLEMENTATION-NOTES.md`, owning `user-docs/` MCP/mecatui pages, generated API/config references, and ADR 0057's index annotation. +6. `task generate`, `task lint`, `task test`, `task api:check`, `task docs`, `task site:build`, `task ac-trace-strict`, and `go run ./cmd/mecademo` pass at implementation completion. +7. The implementation PR remains stacked until plan merge, reports exact interface conformance, and has no unwaived review blocker. This plan tracks #1511 but does not close it. + +## Deferred decisions and known risks + +- Exact poll, cooldown, source/server/tool/list, connect, and retirement bounds are finite tested implementation constants. +- Complete candidates can delay a successful removal while an unrelated desired server fails. The cached `stale` status (also representing degradation) and all three triggers make this visible and retryable; partial-active publication is intentionally not required. +- Cached engines may reference a retired runtime object but cannot use it: every root operation pins current runtime and verifies the engine revision first. Engine close paths must not own the shared direct manager; only runtime retirement closes it. +- Refresh can grant names from a runtime superseded immediately after the pin is released. This is safe under name authority: the grant remains durable but unavailable until that exact name is current, and no live catalog is widened by the save. diff --git a/docs/adr/0355-mcp-source-reconciliation.md b/docs/adr/0355-mcp-source-reconciliation.md new file mode 100644 index 0000000000..bd575d96fb --- /dev/null +++ b/docs/adr/0355-mcp-source-reconciliation.md @@ -0,0 +1,72 @@ +# ADR 0355 — Reconcile stale direct MCP source snapshots + +- Status: Proposed +- Date: 2026-09-16 +- Scope: direct/global MCP source refresh, immutable runtime publication, and explicit name-authority additions +- Supersedes: ADR 0057 only for its deferred “no live catalog mutation” decision; notification transport, dirty invalidation, bounded lazy-list, reconnect, and teardown decisions remain +- Superseded by: None + +## Context + +Mecatl resolves configured direct MCP servers before ToolHive workloads, so configured names win collisions. It currently builds one process-lifetime manager and copies its tools into shared and per-session catalogs. ToolHive can change after startup, and MCP servers can send tool/resource/prompt list-change notifications, but ADR 0057 deliberately stopped at refreshing adapter-local snapshots because `tool.Catalog` is append-only. A removed tool can therefore remain advertised until restart, while an added ToolHive workload remains undiscoverable. + +The stale-source defect does not require a new capability identity. Mecatl's durable authority is intentionally name-based: `session.Authority.CapabilitySet.Tools` is the exact name ceiling, delegation narrows that set, and execution independently checks the current catalog. Under those existing semantics a same-name endpoint, schema, or read-only change remains the same grant; disappearance does not permanently revoke the name; and reappearance of an already granted name is authorized. This ADR preserves that contract rather than adding digests, binding records, provenance subtraction, or a migration. + +Protected broker MCP is separate. ADR 0335 owns `ConnectWorkspaceServices`, `workspace_enrollment`, consent, cancellation, and whole-bundle replacement. Client-provided MCP remains session-local. Mecatui may present a unified `/mcp-refresh` spelling while capability-routing to these distinct operations. + +## Decision + +1. **Reconcile the existing ordered source seam.** One Build-owned reconciler consults configured/static and ToolHive sources in their existing order and applies the existing first-name-wins resolution. Configured process-start input remains static. Automatic polling is enabled only when ToolHive discovery is configured. Tool/resource/prompt list-change notifications and explicit refresh requests enter the same serialized reconciler. + +2. **Coalesce and bound all work.** At most one reconciliation cycle runs and one later invalidation is remembered. Polling is bounded and jittered; source consultation, candidate construction, server connection, active-list cardinality, total candidate page count and bytes, per-refresh union grants, historical grant accumulation before save, and retained runtimes have finite tested internal limits. These are Service/reconciler constants, not exported engine fields; there is no public/global 512-record revision bound. Caller cancellation stops waiting but does not cancel shared work. Automatic work never opens browser/OAuth consent. Poll periods and bounds are not public configuration or persisted schema. + +3. **Keep source last-known-good state and refresh only dirty contracts.** A source consultation failure retains that source's last successful snapshot and marks status stale. A successful snapshot, including an empty snapshot, is authoritative: a successful empty ToolHive observation withdraws its prior entries. Tool/resource/prompt notifications from current servers mark the source dirty; a dirty refresh relists every relevant contract and publication comparison includes tools, resources, and prompts. A stable ToolHive observation with no dirty signal uses the existing live snapshot except for bounded health retry and need not reconnect servers merely to rediscover unchanged contracts; if ordered sources are unchanged and none is dirty, reconciliation may short-circuit before candidate connection. Stale-runtime notifications are ignored. The ordered resolver then computes one complete desired direct-server set. + +4. **Build candidates all-or-nothing.** A candidate creates a complete immutable direct runtime containing the manager, resource/prompt provider, resolved tools, source inventory/status snapshot, and one revision-tagged contribution consumed by `assembleCatalog`. Publication happens only after every desired server has connected, initialized, and listed successfully. Runtime equality compares bounded tool/resource/prompt and resolved source/server metadata/config, not a durable security or authority digest. An unchanged candidate leaves the revision, published engine/factory contribution, caches, and retirement state unchanged and is closed if it was built. A changed candidate may reconnect the complete desired set at bounded cost; premature connection sharing is not required. A failed candidate closes all resources it created and leaves the previous runtime usable. This deliberately favors a simple coherent snapshot over partial-active publication: a valid removal can wait behind an unrelated failed addition, but a successful empty or otherwise complete candidate still publishes removals. Candidate/source degradation uses the existing `stale` status—there is no separate degraded field—and a later notification, poll, or manual request retries it. + +5. **Publish atomically and retire safely.** The reconciler atomically replaces one immutable current direct runtime. It never mutates an active catalog or manager. A bounded set of displaced runtimes may retire. Root runs and direct `RunTeam` operations pin the exact runtime revision they start with; delegation and generation-bound factories inherit that pin. Out-of-run resource/prompt operations take an operation pin. A retired runtime closes only after its pins drain. If publishing would exceed the retirement bound because an old runtime is still pinned, publication is deferred, the unpublishable candidate is closed/discarded, status is stale, and the current runtime remains active; no runtime is force-closed. Exactly one coalesced successor is retained and retried after drain, without unbounded polling or candidate allocation. + +6. **Use revision tags, not engine-lifetime leases.** The shared engine and every cached normal/selector, no-FS, client-MCP, mode-specific, specialist, and debug engine are tagged with the direct runtime revision used to build them. Publication swaps the shared generation-bound factory contribution. At every actual operation boundary, a cached engine whose revision differs from the pinned runtime is evicted and rebuilt before use. The Service-level shared engine is the one exception: it is not rebuilt. Once the current revision differs from the shared engine's build revision, a default-FS session is promoted to a per-session engine built under its pinned runtime and keeps that engine until the session closes; the shared engine then serves only operations that still pin its build revision. This is correct but costs one engine per open session that has run since the first republication, and each later republication rebuilds those engines at their sessions' next runs. Rebuilding the shared engine once per revision instead is tracked in [#1802](https://github.com/stacklok/mecatl/issues/1802). Cached engines do not keep runtimes alive merely by existing. Failed-step retry, restored approval, prompt runs, direct `RunTeam`, delegation/reference MCP, and prompt expansion all resolve under the same pinned runtime; an operation never mixes schema projection, lookup, permission, dispatch, resources/prompts, or manager identity across revisions. + +7. **Availability filters durable name authority.** Current catalogs contain only tools from the published runtime. A granted direct name that is absent is therefore unadvertised and cannot execute, while its durable grant may remain unchanged. If the model emits a stale call whose exact name is granted but absent from the current catalog, the engine returns a permanent “currently unavailable; do not retry unless the catalog changes” tool error. An ungranted absent name retains ordinary unknown-tool behavior. When the exact name reappears, it is advertised and executable again under existing permission checks. Same-name source, endpoint, schema, and read-only drift follows this same current name-grant policy; no URL normalization, digest, or new endpoint credential policy is introduced. + +8. **Make additions explicit.** Automatic polling and notifications never widen a session's durable name ceiling. Owner-invoked direct `/mcp-refresh` requests reconciliation and then stable-unions only the successfully pinned active snapshot's direct tool names into the session's existing tool ceiling. It preserves every unrelated capability and never performs durable direct-name subset replacement or provenance subtraction. Historical direct names may remain granted but unavailable until their exact names are current again. + +9. **Add one narrow aggregate operation.** Add `Session.GrantToolAuthority([]string) error`, the minimum aggregate API needed because authority may not be mutated by composition directly. It clones caller input and requires every added name to be non-empty, valid UTF-8, control-free, and no longer than 256 bytes. It rejects an invalid batch without mutation and stable-unions accepted names in input order; duplicate input or already-present names are idempotent. Existing unrelated names are not revalidated, reordered, or altered. The operation preserves all non-tool axes, conversation, placement, ownership, counters, and lifecycle state. It is legal only in `StateIdle` or quiescent `StateCompleted` with no pending control and does not reopen a completed session. Ordinary-main/root eligibility and current-direct-name provenance remain Service/composition checks. No new `agent.RunRequest`, durable authority field, digest API, or store API is added. + +10. **Mutate only when additions exist, and confirm ambiguous saves authoritatively.** Direct refresh authenticates the owner before caller-selected coordination, then takes `runEntryMu` and checks durable taxonomy/state and same-process liveness at its local linearization point before invoking shared reconciliation. Ineligible requests therefore perform no source work. It supports idle and quiescent completed ordinary roots and rejects running, awaiting, authorizing, failed, cancelled, child, schedule, debug, and broker-conflicting sessions; `Service.IsLive` is not the sole quiescence proof. If the pinned direct names add nothing, refresh performs zero writes and returns without cross-host mutation lease/capability. Otherwise it acquires mutation authority, freshly loads a detached old session, constructs a detached candidate, calls `GrantToolAuthority` once, and issues at most one `Save`. Direct save success makes the exact candidate authoritative. Because a save error may mean the write committed, the Service performs a bounded authoritative reload while holding the same exclusion: exact candidate is durable success, exact old state is durable failure, and mismatch or unavailable confirmation is uncertain and fails closed without claiming known unchanged state. Neither old nor candidate is published as a speculative live aggregate; model/history are unaffected, and retrying the same stable union converges. + + Cancellation before the authority-mutating `Save` starts stops session mutation, although shared reconciliation continues. Once `Save` starts it may commit; bounded confirmation is detached from caller cancellation and remains under the same exclusion. Delivery to a cancelled caller does not promise rollback. + +11. **Keep direct and broker controls distinct.** Add direct-only `HarnessService.RefreshMcpSources`, HTTP `POST /v1/sessions/{id}/mcp-refresh`, and `ServerCapabilities.mcp_refresh = 30` (free on the synchronized `origin/main` contract). The response `revision` identifies the successfully pinned active snapshot whose direct names were considered/granted for that request, not necessarily the latest revision when delivery completes. `changed` compares that request's observed active snapshot before and after its shared reconciliation cycle and the exact stable union; a coalesced successor may supersede the pin without rewriting the earlier response. `ListMcpSourcesResponse` adds current revision, stale, and reconciling fields over the reconciler's cached published/source status; it does not independently probe, and `stale` also represents degradation. Mecatui offers argument-free `/mcp-refresh`: direct capability routes only to `RefreshMcpSources`; broker-only `workspace_enrollment` routes only to `ConnectWorkspaceServices` with existing disclosure, consent, observation, and cancellation. Both capabilities or a missing matching collaborator fail closed. `/tools-connect` remains a deprecated broker-only alias and `/tools-cancel` remains broker-only. + +12. **Make direct-team construction rollback-capable.** Direct `RunTeam` materializes every member under the operation pin. Add an idempotent `Supervisor.Close` lifecycle endpoint and use it from `Run`; repeated or concurrent close cancels an active run, waits for settlement, and releases enrolled forks, inline managers, cancellation, registry, and liveness resources exactly once. A configured store that atomically creates member snapshots must support deletion before direct-team enrollment begins. A later construction failure closes the partial supervisor, deletes startup snapshots under a detached bounded context only while their mutation leases remain held, and releases those leases afterwards. The declaration stays retryable. The startup claim revalidates the exact registry entry under the registry lock so a successful concurrent cleanup cannot launch an orphan supervisor. + +## Consequences + +Stale removals take effect automatically at the next operation boundary without mutating durable authority. Existing sessions do not discover new names until an owner explicitly refreshes, but an already authorized name can disappear and later return without reauthorization. This is availability filtering, not permanent revocation. + +All-or-nothing candidates make runtime ownership and failure behavior small enough to audit. They can delay an otherwise valid removal while another desired server is failing; the existing `stale` status also represents this degradation and bounded polling/notifications/manual refresh retry it. The previous complete runtime is preferable to a mixed generation. + +The process gains a polling goroutine/timer, one coalescing reconciler, source LKG/status state, one current runtime, bounded retiring runtimes, and run/operation pins. Implementation must inventory these resources and restart decisions in ADR 0027. No credential, URL, digest, durable direct-name ledger, or legacy migration is added. + +## Rejected alternatives + +- **Digest or binding-record authority.** This changes existing name semantics and exceeds the original stale-source scope. +- **Permanently revoke removed names.** Disappearance is availability, not revocation; exact-name reappearance remains authorized. +- **Replace the durable direct subset on refresh.** Existing authority has no direct-name provenance. Unioning active names is sufficient; availability handles removal. +- **Mutate catalogs/managers in place.** Active runs could mix schemas, dispatch targets, and closed connections. +- **Lease every engine for its useful lifetime.** Revision checks at operation boundaries plus run/operation pins protect use without letting idle caches retain old runtimes. +- **Force-close undrained runtimes.** This risks live users; bounded retirement instead defers publication. +- **Publish a partial active candidate.** Per-server ownership/reuse machinery is unnecessary for the original defect. Complete candidates provide one coherent failure boundary. +- **Treat source failure as empty.** A transient ToolHive or server failure would masquerade as removal. +- **Remove polling and rely on notifications/manual refresh.** ToolHive inventory changes are not guaranteed to arrive as MCP list notifications; bounded automatic polling is required. +- **Unify direct and broker server operations.** Their authority, consent, and lifecycle contracts differ even though the client spelling is shared. + +## See also + +- [MCP source reconciliation acceptance plan](../acceptance/mcp-source-reconciliation.md) +- [ADR 0057 — MCP client server notifications](./0057-mcp-server-notifications.md) +- [ADR 0335 — Idle-session MCP broker workspace refresh](./0335-idle-session-broker-workspace-refresh.md) +- [ADR 0027 — Cloud-native arc and resource inventory](./0027-cloud-native.md) +- [Extensibility architecture](../architecture/extensibility.md) diff --git a/docs/adr/README.md b/docs/adr/README.md index e64379d59c..352e9efee5 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -202,7 +202,8 @@ Documentation/citation conventions are in [`docs/design/README.md`](../design/RE ### MCP - [0056 — MCP client reconnect](./0056-mcp-client-reconnect.md) -- [0057 — MCP server notifications](./0057-mcp-server-notifications.md) +- [0057 — MCP server notifications](./0057-mcp-server-notifications.md) *(deferred “no live catalog mutation” decision proposed to be superseded by 0355; notification transport, bounded lazy-list, reconnect, and teardown decisions retained)* +- [0355 — Reconcile stale direct MCP source snapshots](./0355-mcp-source-reconciliation.md) *(proposed; preserves exact-name authority and supersedes 0057 only for its deferred “no live catalog mutation” decision)* - [0063 — MCP structured results: fail-closed + CallMcpWithQuery](./0063-mcp-structured-failclosed-callmcpwithquery.md) - [0078 — MCP typed tool results](./0078-mcp-typed-tool-results.md) - [0218 — Internal encrypted credential-store substrate](./0218-credential-store.md)