feat(mcp): describe_tool check mode — in-band preflight (spec 099) - #998
Merged
Conversation
Related #969 Phase-2 of the required-tools preflight roadmap: the reporter-endorsed in-band surface (issue #969 comment 2026-08-13) — an optional `check` boolean on the existing describe_tool built-in rather than a new top-level MCP tool. ## Changes - specs/099-describe-check-mode/spec.md: 4 user stories, FR-001..FR-018 (+FR-009a, FR-012a), 13 edge cases, SC-001..SC-007, non-goals, priced alternatives. - specs/099-describe-check-mode/checklists/requirements.md: quality checklist incl. feature-specific gates. Locked contracts: - check:true = verdict-only results, batch cap 5 -> 50, spec-098 evaluator verbatim (15-code enum, precedence, tier disclosure). - Optional `filters` (three annotation filters) and `expect_hashes` pins, mirroring the REST body. - Plain mode byte-identical to today except ONE enumerated delta: out-of-scope ids report not_found instead of the existence-leaking `invisible` code (spec-085 contract amended). - Token budget: measured 135 -> 284 tokens (tiktoken cl100k_base); new explicit ceiling 300, pinned by the budget test AND a deliberately regenerated tools/list golden (documented exception to spec-098 FR-015's no-delta rule). - Disclosure tier is fail-closed and MUST NOT be inferred from AuthContext.IsAdmin(): the MCP middleware injects an admin context for unauthenticated /mcp requests by design. - Every new reason-surface cell gets sabotage-matrix rows on the existing 098 matrix infrastructure. ## Testing - Token measurements taken on this branch's base with the pinned encoder over the marshalled tool definition (scratch test, removed). - Cross-model review (opencode / gpt-5.6-sol): 8 P1 / 6 P2 / 1 P3, verdict REQUEST CHANGES; all findings folded in, one deliberately scoped out to spec 098.
…whole in-band surface, budget ≤250 Related #969 Amendments per 2026-08-16 decisions: FR-008 trimmed (in-band pins reserved, REST/CLI stays the pinning surface); FR-009 locked to the agent-token tier for the entire in-band surface (no credential-marker plumbing; IsAdmin never consulted; relaxable only additively); FR-015 budget re-set to ≤250; FR-016 exemptions encoded (hash_mismatch + server_not_in_scope are REST-only); FR-018a added (inherited 098 erratum: AuthTypeUser must not get operator tier).
An optional `check: true` on the existing describe_tool answers verdict-only availability from the spec-098 evaluator, so an agent can gate a multi-step plan without leaving the MCP session. - Schema: `check` + `filters` on the one builder both surfaces register (default /mcp + retrieve_tools mode); token budget raised 150 -> 250, the definition measures 243 under cl100k_base. - Handler: strict argument validation (non-boolean check, non-object/unknown/ non-boolean filters, filters without check, the reserved expect_hashes) as request errors rather than coercions; 50-id raw cap; trim -> dedup -> first-occurrence order; MCP tool errors for the 400/503 classes. - Tier is pinned to agent-token in band and the scope is the session's own: a new glue door (RunPreflightForSession) projects serverInScope over the server universe, so a check can never see what the session's retrieve_tools cannot, and out-of-scope/unconfigured collapse to plain not_found. - Activity: one synchronous preflight record per run, marked `mcp-check`; a failed write fails the call, and no internal_tool_call record is emitted. - Plain mode: out-of-scope ids report not_found; the `invisible` code is retired (remediation text unchanged). - REST erratum (FR-018a): disclosureTier grants the operator tier positively to admin-class contexts only, so a server-edition OAuth user no longer receives scope diagnostics or hash pins. - tools/list goldens regenerated for the two describe_tool surfaces only, and the spec-098 no-delta test converted to an enumerated-delta test against a frozen pre-099 capture; code_execution is byte-unchanged. Related #969
- Matrix rows now record the surface AND the disclosure tier. 25 new rows
cover the in-band surface: every reason code observable in band, the
scope/unconfigured collapse to not_found, missing_annotation and
policy_filtered per filter, the 50/51 cap boundary, filters-without-check,
the reserved expect_hashes, and the plain surface's out-of-scope code.
- Reflection gate extended: a code with no mcp-check row fails CI unless it
is listed in mcpCheckExemptReasons with the decision that makes it
unreachable in band. server_not_configured joins hash_mismatch and
server_not_in_scope there — FR-009 collapses it to not_found at the
agent-token tier, so it is REST-only for the same reason.
- Every in-band row is driven by preflight_mcp_check_matrix_test.go, and an
undriven row fails the suite: a committed expectation nothing asserts is a
claim, not a check.
- Inherited 098 erratum fixed: the mid_indexing note claimed existence
outranks connection state on a never-indexed server, which contradicts
FR-005 and the shipped evaluator. Note corrected and a
never_indexed_while_connecting row added, asserted against the evaluator.
- FR-017 parity: in-band and REST agree on {status, reason, retryable,
action} (plus detail/remediation) for 11 states at the agent-token tier,
with checked_at and hash excluded by name.
Related #969
- Feature page: an "In band" section with the agent-loop example, the check-vs-describe distinction, the deliberate differences from REST/CLI (50-id cap, `filters` vs `policy` naming, no pins, no wait, pinned tier), and where check mode does NOT exist (code_execution/direct) with the interim path. - Disclosure-tier table now states that the whole in-band surface is the agent-token tier and that a server-edition OAuth user is too. - spec-085 describe_tool contract amended in place with a banner: check mode, the retired `invisible` code, the ≤250-token budget. - REST API reference: describe_tool per-id vocabulary, the check-mode twin of POST /api/v1/preflight, tier wording. - CHANGELOG: both compatibility breaks under a Breaking Changes heading, each with its migration note. Related #969
…-002) An 18-scenario corpus of describe_tool calls WITHOUT check — definitions, every per-id error code, did-you-mean, duplicates, padded ids, both request errors — replayed against a capture taken from the pre-099 handler in a throwaway worktree, compared byte for byte. The only permitted difference is named as a substitution: an out-of-scope id's `"error":"invisible"` becomes `"error":"not_found"`, and the rest of the entry (including the remediation) must be identical. A scenario that changes in any other way, a scenario that changes and is not enumerated, and an enumerated scenario that stops changing all fail. A miscased out-of-scope id is deliberately NOT enumerated: it resolves not_found before the scope gate and did so before this change too. Related #969
Related #969 Third reflection-gate exemption (server_not_configured collapses at the token tier per FR-009); FR-018a restated as a positive admin-class grant (AdminUser keeps operator tier; unknown types fall to agent-token).
…atrix cells Round-1 cross-model review findings on the describe_tool check mode. FR-018a — a nil auth context received the operator tier, which is the exact residual-grant shape FR-018a exists to remove. disclosureTier now grants the operator tier POSITIVELY to admin-class contexts only; nil and every unknown type fall to the agent-token tier, and preflightParams no longer assumes a non-nil context. Nothing operator-facing is lost: apiKeyAuthMiddleware installs an explicit admin context for a validated API key and for the OS-authenticated socket / named pipe, which the new end-to-end test asserts through the real middleware rather than by constructing a context. The only path that reaches a handler with no context is the middleware's no-config passthrough, where an unreadable config is precisely not evidence of admin. FR-013 — the in-band record kept only deduped outcomes, so the raw requested-id count the spec promises was recoverable from nothing. The check surface now records its arguments as sent (raw tool_ids, annotation filters) under a new metadata key. ids_count stays the unique count both surfaces agree on, and the REST record is unchanged: PreflightActivity.Arguments is nil there, so no key is added to a payload nothing asked to change. FR-016/FR-017 — the connection-state and policy_filtered matrix cells ran the evaluator over a hand-built EvalContext, so the glue between handler and evaluator was never on their path. preflightSnapshot gains the one injectable seam (nil in production, mirroring preflightRecorder) and those cells now run the real describe_tool handler over the real scope/tier/annotation glue with only the snapshot injected. The FR-017 parity exclusions were structural by omission; they are now a named list with a reflection gate, so a field added to either payload must be compared or excluded by name. Related #969
Related #969 A globally excluded field let one surface's exclusion reason blanket the other (a future MCP-side hash would silently escape comparison). Exclusions are now per-surface maps; the reflection gate checks each struct against its own surface's list and prunes stale exclusions (an excluded name matching no real field fails). Reviewer: opencode/gpt-5.6-sol round 2 (1 finding).
Dumbris
force-pushed
the
099-describe-check-mode
branch
from
August 16, 2026 11:53
2b82957 to
56aeea9
Compare
Deploying mcpproxy-docs with
|
| Latest commit: |
56aeea9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://66eae50b.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://099-describe-check-mode.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 31945559931 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
enabled auto-merge (squash)
August 16, 2026 12:13
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related #969
Phase 2 of the preflight roadmap: the reporter-endorsed in-band surface — an optional
check: truemode on the existingdescribe_toolbuilt-in ('a check mode on an existing built-in rather than a new top-level MCP tool', issue #969 comment 2026-08-13). Same evaluator, same 15-code taxonomy, same precedence as the spec-098 REST surface, structurally unable to drift (one shared evaluation seam).What
describe_tool {check: true, tool_ids: […], filters?}on the default and retrieve_tools surfaces: verdict-only per-ID results (readyor one reason + retryable + action + remediation + scope-filtereddid_you_mean), set-level verdict,request_idfor activity correlation. Batch cap 50 (verdict-only output is what makes the lift safe).expect_hashesis trimmed/reserved (rejected loudly, never ignored) — pinning stays on REST/CLI.not_found;IsAdmin()never consulted (the MCP middleware hands unauthenticated requests an admin context for back-compat, so it proves nothing). Operators get the full diagnosis on REST over authenticated channels.invisibleis retired →not_found(closing the existence-probe differential the 098 evaluator already prevents). Verified by an 18-scenario replay corpus captured from the pre-099 commit.mcp-check, raw arguments recorded, write failure fails the call).disclosureTieris now a positive admin-class grant (nil/unknown contexts fall to agent-token; the server-edition non-admin OAuth leak is closed); the sabotage-matrixmid_indexingnote corrected + a never-indexed-while-connecting row added.Verification
25 new in-band sabotage-matrix rows (reflection gate extended with three recorded REST-only exemptions), in-band↔REST parity test with per-surface named field exclusions enforced by a two-directional reflection gate, plain-mode replay corpus, full race suite + server-edition + lint both tag sets + swagger/generate-types clean.
Review
opencode / gpt-5.6-sol: spec reviewed pre-implementation (15 findings folded); code round 1 = 1 P1 (latent nil-context operator grant) + 2 P2, all fixed with fail-first evidence; round 2 = 1 finding (global vs per-surface parity exclusions), fixed; round 3 → APPROVE.