Skip to content

fix(mcp): match two tool vocabularies to what the tools accept - #958

Merged
drewstone merged 1 commit into
mainfrom
fix/mcp-tool-vocabularies
Aug 21, 2026
Merged

fix(mcp): match two tool vocabularies to what the tools accept#958
drewstone merged 1 commit into
mainfrom
fix/mcp-tool-vocabularies

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

Two MCP tools declare a vocabulary that disagrees with what they accept.

1. delegation_history cannot select the only profile anything writes

DelegationProfile has three members (src/mcp/types.ts:21): coder, researcher, ui-auditor. delegation_history names two, in three separate copies:

copy file:line says
the agent-facing description tools/delegation-history.ts:36 profile ("coder" | "researcher")
the JSON-Schema enum tools/delegation-history.ts:46 ['coder', 'researcher']
the runtime validator tools/delegation-history.ts:68 throws TypeError on anything else

There is exactly one queue.submit call in the product — tools/delegate-ui-audit.ts:313 — and it submits profile: 'ui-auditor'. coder and researcher are written only by tests (tests/mcp/task-queue.test.ts:35,101).

So an agent asking "did my last UI audit land?" gets a TypeError, and the two profiles it is allowed to ask for always return nothing. The root cause is recorded in the repo already: tools/delegate.ts:6 says the per-profile delegation tools were replaced by one generic delegate verb, and the per-profile vocabularies stayed behind.

2. answer_question advertises an open escalation target and accepts two

QuestionDecision's escalate arm reads to: 'parent' | 'user' | string (tools/coordination.ts:100). In TypeScript that union collapses to string, so the type promises any escalation target. The tool's schema (:2267) and its handler guard (:2325) accept exactly parent and user. Anything else falls through to:

answer_question: provide answer, deferReason, or escalateTo

which names the wrong cause: the caller did provide escalateTo.

Change

  • delegationProfiles is the one list (src/mcp/types.ts), and DelegationProfile derives from it. The description, the schema enum and the validator all read it, so a profile added there cannot be one a tool refuses. The validator's message names the accepted set instead of spelling two of them.
  • QuestionDecision.escalate.to narrows to 'parent' | 'user', from the same single list the schema enum reads, and a present-but-unaccepted escalateTo is refused with a message naming the value and the accepted set.
  • delegationProfiles, questionEscalationTargets and QuestionEscalationTarget are exported from ./mcp. That is not decoration: docs:api refuses a type referenced by an exported type that is not itself exported, so deriving a public type from a list makes the list public. It is also the useful half — a consumer building a profile filter or an escalation control reads the list instead of restating it.

A false positive I hit, and the note it earned

Rewriting DelegationProfile from a spelled-out union to (typeof delegationProfiles)[number] is the same type. #953's shape digest reads the declaration a build emitted, so it moved anyway and asked for a minor bump this change did not strictly owe:

shape changed: ./mcp DelegationProfile: shape cf178bc46884 -> 9adc902a4da3
shape changed: ./mcp QuestionDecision: shape 70add1e7b10b -> 39e32137f7f3

The second is a real narrowing. The first is not, and the gate cannot tell — deciding that two differently-written declarations denote one type is the subtyping question that record deliberately does not answer. It errs toward demanding a bump, which is the safe direction, and this is the third limit its header now states. That is a one-sentence addition to scripts/lib/api-surface.mjs, in this pull request because this is where the case was measured; nothing else in that file changes.

Proof

pnpm run lint                 615 files, no fixes
pnpm run typecheck            clean
pnpm run build                clean
pnpm run check:api-surface    2123 exports / 17 entry points, record current
                              bench: 223 exports / 41 entry points, record current
pnpm run check:testing-fixture  fixtures are current
pnpm run check:version-bump   package.json: 0 manifest and 5 export change(s) needing a minor
                              bump, paid for by 0.155.0 -> 0.156.0 (minor)
pnpm run docs:check           exit 0
pnpm test                     2807 passed / 170 failed across 20 files
  clean origin/main, same machine: 2797 passed / 171 failed across 21 files.
  The 20 failing files are a strict SUBSET of the baseline's 21. Zero new failures.
  All are macOS git-worktree and process-spawn timeouts; CI on Linux is the authority.

Simplification

Simplification: three copies of the delegation-profile list become one owner that the description, the schema and the validator all read; the escalation target's two copies become one.
Net: +59 / -16 lines across 5 source and test files (plus the version bump's regenerated record, fixtures and generated docs); 3 vocabulary copies removed, 2 lists now have one owner each.
Not done here: whether coder and researcher should survive at all. Nothing writes them since the generic delegate verb replaced the per-profile tools, and the same leftover explains DelegateResearchArgs and ResearchSource — a public-surface deletion that needs a decision, filed as item 8 of #954.

Tests: +1 (delegation_history accepts every profile a queued record can carry — today it throws for the only one the product writes), -0 deleted. The escalation change moves only which error text a refused target produces; both paths already threw, so it gets no test — the narrowed type is what makes the disagreement unrepresentable.

Refs #954

delegation_history filtered on `profile` and admitted `coder` and `researcher`
in three copies: the agent-facing description, the JSON-Schema enum, and the
runtime validator. The one tool in this package that submits a delegation
record is delegate_ui_audit, and it submits `ui-auditor`, so an agent asking
for its UI-audit history got a TypeError and the two profiles it could ask for
were written by nothing but tests.

`delegationProfiles` is now the one list the description, the schema and the
validator read, and DelegationProfile derives from it.

QuestionDecision's escalate arm declared `to: 'parent' | 'user' | string`,
which collapses to string, while answer_question accepted exactly two targets.
Anything else fell through to an error naming the wrong cause. The type narrows
to the two the tool accepts, from the same list the schema reads, and an
unaccepted escalateTo is refused with a message naming the value.

The api-surface record's header gains the third limit this change measured: the
digest reads the declaration a build emitted, so restating a type without
changing what it means still moves it.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — d34d5560

This PR was opened by the trusted drewstone account.

This approval is provisional and was applied by the local stand-in because the pr-reviewer webhook host is unreachable (2026-08-21). CI on this head is fully green. The full PR reviewer audit re-runs via the resweep when the service returns and will publish findings if it detects issues.

@drewstone
drewstone merged commit 059ed77 into main Aug 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants