Repository navigation
Refactor MCP command tool reference pinning into a focused module #1344
Description
Summary
Refactor MCP reference-pin state into a focused module whose interface preserves the command/result types already established by the command surface. Keep runtime validation only where data actually crosses an untrusted boundary.
Motivation
Work on #1340, #1341, and #1343 clarified two different categories that the current module had treated too similarly:
- MCP/JSON-RPC arguments are external data. They enter as
unknownand must be validated once before becoming command input. - Command results are produced by code in this repository. After dispatch, their type is
CommandExecutionResult<Name>and should remain correlated withName; they should not be carried asunknown, repeatedly converted to optional records, or protected by fallbacks for impossible outer shapes.
PR #1343 applies that distinction to the command executor and structured-result projection. Reference pinning is the next locality problem immediately adjacent to those fixes:
src/mcp/command-tools.tsstill answers several separate questions: tool registration/execution, MCP/client config parsing, reference-pin collection and retention, error-result construction, and text rendering.- Reference pinning owns independent scoped state, command classification, extraction, retention, and argument rewriting.
- Its nested evidence fields are currently broad enough to require focused runtime narrowing. Where those fields are owned by command producers, the extraction module should reuse or improve the producer result types rather than propagate
unknownthrough internal APIs. src/mcp/__tests__/command-tools.test.tsaggregates tests for these concerns instead of mirroring production-module ownership.
The state model is intentionally simple: bounded Map/Set/record collections are sufficient. The problem is lost type correlation and mixed ownership, not a need for a framework, generic registry, or more defensive layers.
Proposed scope
- Extract reference-pin behavior and scoped state into a focused MCP module with a small typed interface used by the command-tool executor.
- Preserve command/result correlation at that interface. A known command result should enter as its producer-owned type, not
unknownor an uncorrelated generic record. - Validate JSON-RPC/MCP input only at the external boundary, then pass the narrowed command input internally without repeating shape checks.
- Narrow genuinely optional or dynamic nested evidence once, close to extraction. If the repository controls a producer and can state a stronger honest contract, improve/reuse that producer type instead of retaining defensive parsing.
- Make command-family branches exhaustive at compile time where practical, so adding a new ref-issuing or interaction command requires an explicit decision.
- Keep
command-tools.tsas the composition/execution boundary: list tools, create the executor, invoke the client, update pin state, and assemble the final MCP result. - Move focused reference-pin tests into a sibling test file that mirrors the extracted module.
- Split remaining executor/config/rendering tests only where the production boundary makes ownership clear; do not create mechanical files solely to hit a line target.
- Preserve the current bounded 1,000-pin behavior using simple collections.
Out of scope: splitting command-output-schemas.ts. That registry has similar size pressure, but schema ownership is a separate change.
Acceptance criteria
command-tools.tsno longer owns reference-pin classification, extraction, scoping, or retention logic.- The reference-pin interface uses command/result domain types and does not accept
unknownfor repository-produced command results. unknownremains only at actual trust boundaries such as JSON-RPC input, parsed JSON, catch values, or genuinely external runtime payloads, and is narrowed once before internal use.- No outer result-shape probes, impossible missing-value branches, generic output-format fallback, or repeated validation remains where
CommandExecutionResult<Name>already proves the shape. - Genuinely optional nested fields retain explicit handling; stronger types are introduced only when producer behavior guarantees them.
- Command-family handling is exhaustive or backed by a type-level completeness check.
- Internal state remains simple and encapsulated;
Set/Map/record shapes do not leak into executor orchestration. - Tests mirror the resulting source topology and no MCP test file remains over the repository's 1,000-line architecture-debt threshold.
- Existing behavior remains unchanged for:
- snapshot/find issued refs;
- settle evidence refs;
- target and wait ref arguments;
- divergence screen refs;
- per-session/device pin scoping;
- bounded pin retention.
- No new generic registry, dependency-injection seam, compatibility fallback, or abstraction is introduced solely for the refactor.
pnpm check:affected --base origin/main --runpasses.
Sequencing
Blocked by: #1343
Implement after that PR lands, or base the work on it, to reuse its typed command-result surface and avoid conflicts in the same MCP modules.
Activity
thymikee commentedon Jul 20, 2026
Triage: valid and understood as a focused maintainability refactor. It is not
ready-for-agentyet because it is explicitly blocked by #1343. Current PR #1345 also needs to be rebased onto the final #1343 head (or ontomainafter #1343 merges) and needs one executor-level snapshot-to-pinned-interaction composition regression before readiness. Removedneeds-triageand classified this as an enhancement; promote it after the dependency and branch sequence are resolved.refactor(mcp): extract reference-pin state into tool-ref-pins module (#…