feat: read the engine's unfulfilled-keys report instead of diffing declared vs delivered

Merged
RA
1mo ago
+2021-104

Issue for this PR

Closes #1307

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

When a workspace is bound, the attach toast says "N of M declared integration tools available" and lists what is declared but absent. Until now that list came from a client-side diff: fetch the workspace's allowlist from the API, subtract the tool names the engine served. A diff can name keys, never reasons — an expired Jira token, an MCP server whose binary is not installed, an integration the tenant removed from the catalog, an extension tool with no VS Code window, and a key the provider does not offer all read the same.

@altimateai/datamate 0.7.3 (AltimateAI/altimate-mcp-engine#248, released by AltimateAI/altimate-mcp-engine#250) reports every declared-but-unserved key with a reason under _meta["ai.altimate/unfulfilled"] on each tools/list response. This PR reads it:

  • MCP catalog keeps the _meta of a server's last tools/list page per client. paginate keeps only each page's items, so the result object — the only carrier of _meta — was dropped. A listing starts with none; any page that carries one sets it; a listing without one clears it. Exposed as MCP.listMeta(name) (undefined while not connected).
  • Attach takes the gaps from the report instead of the diff. no-bridge entries stay out of the "missing" line, as absent extension tools without an IDE were already treated as expected; every other reason is named, grouped, with the engine's detail (e.g. spawn docker ENOENT) — Declared but not available — no usable connection: jira_search_issues; server could not be started or reached (spawn docker ENOENT): gh_list_prs, gh_create_pr. The detail is masked when the report is parsed (control characters collapsed, then Telemetry.maskString, as subprocess stderr is), so the toast and the attach log carry the masked text. The "N of M" headline still counts declared keys that are present, and the toast is a warning when a gap is reported or fewer declared tools are callable than declared. The attached outcome carries the full report for later surfaces.
  • No report means no claim. An engine that sends no _meta (nothing at or above the floor does) yields an outcome with neither missing nor unfulfilled, and a toast with no gap line — not "all served".
  • Floor moves to 0.7.3 (MIN_ENGINE_VERSION), the first engine that emits the report (0.7.2 was released without it). @altimateai/datamate@0.7.3 is on npm as latest.
  • MCP server lifecycle (found in review): add, connect, disconnect, remove, authenticate, startAuth and removeAuth each take a sequence number when they start, and OAuth completion continues the number of the call that began the flow. A call writes runtime state, and the enabled flag on disk, only while it is still the latest call for that server, so an older call that resumes late cannot bring a removed server back or overwrite a newer config.

The client no longer reads what it cannot know: the allowlist lookup (declared()) is kept only for the headline's denominator and the extension-tool count, and a report without a reachable allowlist still names the gaps.

Claims

  • C1 — Nothing is invented. missing and unfulfilled exist on the outcome only when the engine sent a well-formed report; a malformed or absent _meta yields neither, and a malformed one is logged as a warning once per transition into that state, even when the announcement itself is unchanged (tests: "an engine that sends no report is not read as having no gaps", "a report that turns malformed is logged once per transition, without a second toast"; parseUnfulfilled cases).
  • C2 — no-bridge never counts as missing, and every other reason does — including unknown-key on an extension key while a bridge is connected (reportedMissing; test "no-bridge entries in the report are expected, never missing").
  • C3 — The catalog keeps the report across the paths that list tools: initial connect, the tools/list_changed refresh, the post-OAuth reconnect all go through McpCatalog.defs → listTools, which is the only writer (catalog-list-meta.test.ts covers first page, multi-page, and clearing).
  • C4 — A gap whose reason, integration or error text changes is announced again; an identical report is not (the signature carries integration, key, reason and detail; tests "a gap whose reason changed is announced again" and "a gap whose error text changed under the same reason is announced again").
  • C5 — Servers other than the engine are unaffected: _meta is retained per client but read only for datamate; tool conversion and the stored defs are unchanged.
  • C6 — A detail is masked before anything shows or logs it: parseUnfulfilled collapses control characters and applies Telemetry.maskString, so the toast and log.info both carry the masked text (test "a detail is masked and flattened before anything shows or logs it").
  • C7 — Severity follows what is callable: warning when a gap is reported, or when a report is present and fewer declared tools are callable than declared (two raw keys that sanitise to one catalog entry); with no report nothing is claimed and it stays info (the collision test asserts warning, the no-report test info).
  • C8 — For one server, the latest-started lifecycle call wins in runtime state and in the persisted enabled flag, whatever order their suspensions settle in (lifecycle.test.ts: a remove, disconnect or newer add during a gated connect; a disconnect during a gated connect; a newer add while a disconnect is closing; oauth-auto-connect.test.ts: a remove while an already-authorized authenticate lists tools, or while that listing fails, or while finishAuth connects). A superseded call closes its own client and returns the winning call's status.
  • C9 — An OAuth flow finishes only as the call that began it, and clears only its own state. startAuth or authenticate records the flow's lifecycle token and transport. finishAuth finishes only a flow recorded that way, under its token, so a remove, disconnect or removeAuth issued after the flow began wins and the late callback connects nothing. A transport a connect left when it found the server needs auth cannot be finished, and a startAuth superseded while it sets up publishes no flow and clears its own OAuth state. A flow never finishes another flow's transport. A superseded removeAuth removes nothing: the removal re-checks its claim inside the auth file lock. Every OAuth-state clear compares the stored state inside the auth file lock (McpAuth.clearOAuthState(name, expected)), on the browser path too, mismatch included. remove, disconnect and removeAuth drop the server's pending flow and cancel a waiting callback (oauth-auto-connect.test.ts, oauth-browser.test.ts, each falsified by reverting its guard).

Residuals

  • R1 — Reasons outside the engine's current set are shown verbatim (a newer engine may add one) rather than dropped.
  • R2 — The toast shows at most five keys across groups and truncates a detail at 60 characters; the full report is on the outcome.
  • R3 — The floor bump refuses 0.7.2 and older engines; that is the intended contract, and the reason is in the MIN_ENGINE_VERSION comment.
  • R4 — Telemetry.maskString is the codebase's general masking, not a secret detector: it masks paths and known key shapes, and a short bare token passes through it. What keeps configuration out of detail is the engine at the floor, which sends only known error shapes and reduces a spawn failure to a plain command name.
  • R5 — The enabled write re-checks its call immediately before addMcpToConfig, but that shared config writer has its own read-then-write window. A disconnect already inside it when a later add for the same server commits can still leave the flag false on disk.
  • R7 — A removeAuth that lands while the SDK is exchanging an authorization code: the flow still connects nothing (its token is stale), but the exchange's own token write can store credentials again after the removal. The window is the exchange round trip.
  • R8 — Two overlapping startAuth calls on one server: the route's finishAuth carries only the authorization code, so a code from the older browser window is tried on the newer flow. The PKCE verifier does not match, the exchange fails and nothing connects; the user signs in again from the newer window. Binding a code to its flow needs the OAuth state passed through the route.

How did you verify your code works?

  • bun run typecheck clean; prettier clean on the files this PR touches (the files that were already non-conforming on main are left as they were).

  • test/altimate/workspace and test/mcp on the branch merged with current main: 964 pass, 6 fail. The 6 (5 mcp.headers, 1 oauth-auto-connect) fail the same way with main's src/mcp swapped in, so they are environmental, not this change.

  • New tests: 6 attach cases (reasons in the toast, no-bridge exclusion, no-report, report-without-allowlist, reason-change re-announce, the existing inventory case now stating the engine's report, error-text re-announce, and the collision case's warning), describeMissing/parseUnfulfilled/reportedMissing unit cases (including detail masking), 6 catalog cases through a scripted client (other test files mock the MCP SDK process-wide, so a real SDK client here would list nothing; the SDK's _meta passthrough is covered by the real-engine e2e run).

  • End to end through the real MCP service (test/mcp/engine-unfulfilled.e2e.test.ts, env-guarded, skipped in CI): the published @altimateai/datamate@0.7.3 (a clean npm i) is spawned over stdio by MCP.add exactly as the overlay spawns it, against a fake Altimate API, a real second MCP server and a missing binary; MCP.listMeta("datamate") returns the five-entry report with the expected reasons and the toast text reads Declared but not available — no usable connection: jira_search_issues; not offered by the integration: ghost; server could not be started or reached (spawn altimate-e2e-missing-binary ENOENT): whatever; no longer in the catalog: retired_tool. — 1 pass, and the run leaves no temporary HOME behind. Run it with ALTIMATE_ENGINE_E2E_ROOT=<engine checkout with dist/> bun test test/mcp/engine-unfulfilled.e2e.test.ts from packages/opencode.

  • Engine → CLI through the real attach path (evidence): bootstrap + beforeTurn on a bound directory against the published @altimateai/datamate@0.7.3 (a clean npm i, on PATH as datamate). Settled outcome attached with declared: 5, missing: [jira_search_issues, ghost, whatever, retired_tool], the full report incl. the no-bridge entry, and the exact toast text; 8/8 checks. A 0.7.2 build (the report present, the version that shipped without it) is refused as engine-too-old; 2/2.

Screenshots / recordings

Not a UI change beyond toast text; the exact strings are asserted in the tests above.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01GHBUvb843k1R7UAGi8Ya9b

Appendix — complexity delta (altimate-code: engine unfulfilled report)

e8c21c2af7 → 93c879af8 · only functions this diff touches · advisory, not a gate.

✅ No touched function changed in complexity (12 touched, 4 new, all under 10).

ℹ️ How to read these numbers

Cognitive (Sonar spec) counts breaks in linear reading flow — each if/loop/catch/ternary/boolean-operator switch adds 1, and nesting makes every further break cost more. It approximates how much you must hold in your head to follow the function: 0–5 trivial · 6–10 easy · 11–15 moderate (15 = Sonar's recommended per-function cap) · 16–25 hard to follow · >25 needs decomposition.

CCN (cyclomatic) counts independent paths — also the minimum number of test cases for full branch coverage of the function.

Only functions this diff touches are measured, as deltas — pre-existing complexity is not counted against this change. Rising numbers aren't automatically wrong; they're where review attention should go. Test files excluded.

Summary by cubic

Workspace attach now reads the engine's ai.altimate/unfulfilled report from each tools/list _meta instead of diffing declared and delivered tools, so missing-tool notices carry reasons and details rather than only tool names.

  • Excludes no-bridge entries, groups other gaps by reason and integration (numeric integration ids parse as strings), masks each detail like subprocess stderr, and keeps the full report on the attach outcome; missing and unfulfilled stay unset when the report is absent, and a malformed one is logged once per transition.
  • Re-announces on changed reasons, integrations, error details, or callable tools, including equal-count swaps and the no-report-to-empty-report transition; served counts are distinct sanitized catalog entries across ordinary and extension tool groups, never a key the report names.
  • Commits each listing's tools and its own _meta together (MCP.snapshot), keeps the last good pair when a refresh is pending or fails, and drops out-of-order refreshes.
  • Every add, connect, disconnect, remove, and OAuth completion takes a per-server token before its first suspension and re-checks after each one; a superseded call closes its own client and returns the winning call's status. The queued enabled write re-checks currency when it starts and before touching the file, auth-lock checks keep a stale call from deleting or overwriting a newer flow's state (clearOAuthState with an expected state, beginAuth before it connects), and removeAuth drops credentials and pending flows only while its claim holds. Each pending OAuth flow keeps its beginning call's token and transport — a startAuth superseded during setup publishes no flow — and finishAuth refuses a transport a connect left behind; disconnect, like remove, also drops a pending transport, so a late callback cannot reconnect a removed, disconnected, or disabled server.
  • Raises MIN_ENGINE_VERSION to 0.7.3, requiring @altimateai/datamate 0.7.3 or newer (0.7.2 shipped without the report).

Closes #1307.

Summary by CodeRabbit

  • New Features

    • Workspace connection notices identify unavailable tools by integration and reason, include relevant details, and summarize additional missing tools.
    • Notices reflect reported gaps—including tools absent from the declared catalog—and indicate how many declared tools are callable.
    • Without a missing-tools report, notices do not claim that tools are missing.

  • Improvements

    • Notices update when a missing tool’s reason or details change.
    • The minimum supported engine version is now 0.7.3.