diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/context-pack.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/context-pack.md new file mode 100644 index 0000000000..94fec2e6f7 --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/context-pack.md @@ -0,0 +1,70 @@ +# Context Pack: OMB S8 existing-machinery correctness fixes + +## Run Metadata + +| Field | Value | +| --- | --- | +| Run ID | `fix-mcp-truncation-receipt-ordering--s8` | +| Branch | `fix/mcp-truncation-receipt-ordering` | +| Current phase | `implement` | +| Archetype | `2 - Integration` | +| Scope overlays | `none` | + +## Current State + +Clean worktree baseline equals `origin/main` at `fb75cf6f`. Research and design are locked, and a +separate local open-model PLAN-EVAL returned `PASS`. No package source has changed yet. + +## Completed + +- Read issue #1134 and RFC #1123 §4 plus seed S-13/S-15 findings. +- Read required harness, PR, doctrine, JSR, tooling, and Archetype-2 authorities. +- Re-baselined actual MCP receipt/truncation code and adjacent debt. +- Ran baseline full-export doc lint: 0 diagnostics across `mod.ts` and `cli.ts`. +- Obtained tracked PLAN-EVAL `PASS` from Claude Code + OpenRouter Qwen. + +## In Progress + +- Slice 1: receipt settlement after validation and failed-attempt fixture. + +## Next Steps + +1. Implement and gate receipt settlement after validation. +2. Perform opposite-family slice review, commit, push, and comment. +3. Implement and gate truncation metadata + byte ceiling. + +## Key Decisions + +| Decision | Source | Notes | +| --- | --- | --- | +| No public export change | user contract / plan D1-D4 | Internal callback and fixed byte ceiling only | +| Full Archetype-2 column | RFC S-20 | quality gate + scoped wrappers + targeted runtime fixtures | +| Adjacent MCP v2 debt untouched | issue/user contract | `MCP-A6-V2-SHAPE` remains open | + +## Files Changed + +| Path | Status | Notes | +| --- | --- | --- | +| `.llm/runs/fix-mcp-truncation-receipt-ordering--s8/*` | new | Harness bootstrap/plan artifacts only | + +## Gates + +| Gate family | Current status | Evidence | +| --- | --- | --- | +| Static | baseline PASS | package doc lint 0 diagnostics | +| Fitness | plan-gate PASS | full Archetype-2 matrix selected and approved | +| Runtime | planned | two required fixtures | +| Consumer | N/A unless drift | exports locked unchanged | + +## Open Questions + +- None blocking PLAN-EVAL. + +## Drift and Debt + +- Drift: minor file-location clarification logged. +- Debt: adjacent `MCP-A6-V2-SHAPE` unchanged; no new debt expected. + +## Commits + +- See the draft PR's commit list + per-slice PR comments. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/drift.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/drift.md new file mode 100644 index 0000000000..a887ec5b9d --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/drift.md @@ -0,0 +1,55 @@ +# Drift Log: OMB S8 existing-machinery correctness fixes + +Drift is append-only. Record facts that diverge from the plan, RFC, doctrine, or current-state +documentation. + +## 2026-08-03 — Receipt wrapper location clarified + +- **What:** The user brief names `mcp-server.ts` `withReceipt` near lines 96–112; current baseline + defines `withReceipt` in `packages/mcp/cli.ts`, while `mcp-server.ts` owns output validation at + the cited lifecycle point. +- **Source:** `packages/mcp/cli.ts:175-211`; `packages/mcp/src/application/runner/mcp-server.ts:96-116`. +- **Expected:** Receipt-ordering change centered on `mcp-server.ts`. +- **Actual:** Correctness requires a narrow wrapper/runner integration across both existing files. +- **Severity:** minor +- **Action:** fix +- **Evidence:** RFC S-15 already calls this a `withReceipt`/runner integration and anticipates both + locations; scope remains ordering only. + +## 2026-08-03 — First local PLAN-EVAL transport timed out + +- **What:** The canonical guarded local evaluator launched on Claude Code + OpenRouter Qwen but hit + the suite child-process timeout before writing `plan-eval.md`. +- **Source:** local evaluator outcome for command `plan-eval-1134`. +- **Expected:** A tracked `PASS` or `FAIL_PLAN` artifact. +- **Actual:** Transport exit 1 with diagnostic `timeout`; no verdict artifact and no tree changes. +- **Severity:** minor +- **Action:** fix +- **Evidence:** Retry the same route with a concise prompt and a longer bounded process timeout; + this is not counted as a `FAIL_PLAN` cycle because no evaluator verdict was emitted. + +## 2026-08-03 — Ordinary review primary route unavailable + +- **What:** The installed Claude CLI rejected the policy-configured Fable model id before the + slice-1 ordinary review began. +- **Source:** `review_codex` launch exit 1, API 404 `model_not_found`. +- **Expected:** Claude Fable low ordinary review. +- **Actual:** No review turn and no file changes; policy-declared Claude Opus low fallback available. +- **Severity:** minor +- **Action:** accept +- **Evidence:** Use the `review_codex` `token_limit_fallback` entry from + `workflow/lane-policy.md` / `runtime/routing-policy.ts`; opposite-family invariant remains intact. + +## 2026-08-03 — IMPL-EVAL transport retries were guard-clean + +- **What:** The first IMPL-EVAL launch omitted the runtime profile's OpenRouter source-to-Claude + credential mapping and received 401 before a turn. The next guarded Qwen attempt tried to spawn a + closed-model lookup child, which the evaluator model guard terminated before a verdict. +- **Source:** local Claude Code evaluator launch transcripts; evaluator policy audit guard. +- **Expected:** One complete open-model Qwen parent evaluation turn. +- **Actual:** Neither failed attempt produced a verdict or source change. A tightened no-delegation + parent-only Qwen retry completed and wrote `evaluate.md` with `PASS`. +- **Severity:** minor +- **Action:** accept +- **Evidence:** Final evaluator session records model `qwen/qwen3.7-max`, guard enabled, full gate + execution, and exact `PASS`; no closed-model evaluator work was admitted. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/evaluate.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/evaluate.md new file mode 100644 index 0000000000..bb8dbc82b6 --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/evaluate.md @@ -0,0 +1,137 @@ +# IMPL-EVAL — fix-mcp-truncation-receipt-ordering--s8 + +- Evaluator session: Qwen parent session (no subagent delegation, no closed-model child) +- Run: `fix-mcp-truncation-receipt-ordering--s8` +- Branch: `fix/mcp-truncation-receipt-ordering` +- Baseline: `fb75cf6fc5ad02130ada0ac42e6f44035ac03a9b` (origin/main at plan lock) +- HEAD: `8a0314931ca000604f49b240f45523d333a7c2b6` +- Commits evaluated: `5192b07a9` (plan), `0ade6736a` (gate), `f00d6338e` (slice 1), `8a0314931` (slice 2) +- Surface / archetype: `packages/mcp` / Archetype 2 — Integration (RFC #1123 S-20) +- Scope overlays: none +- PR: #1183 (OPEN, `status:impl`); referenced issue #1134 + +## Plan-Gate prerequisite + +`plan-eval.md` = `PASS`. Both implementation slices were authorized before any package source +change. The Plan-Gate commit `0ade6736a` precedes the first implementation commit `f00d6338e`. + +## Design checkpoint verification + +`worklog.md` contains a `## Design` section with all seven required subsections: Public Surface, +Domain Vocabulary, Ports, Constants, Commit Slices (2 slices, both < 30), Deferred Scope, and +Contributor Path. Every file created during implementation traces back to a concept named in the +design: `receipt-lifecycle.ts` (FlowReceiptRecorder + internal callback seam), truncation changes +(TruncationOutcome + ResultByteLimitError). + +## Commit slice conformance + +| Slice | Design plan | Commit | Gate evidence | +| --- | --- | --- | --- | +| 1 | Receipt-after-validation lifecycle and failed-attempt fixture | `f00d6338e` | targeted `drift-evidence_test.ts` + `doctor_test.ts` 16/16 PASS | +| 2 | Honest truncation metadata and whole-result byte ceiling | `8a0314931` | targeted `truncation_test.ts` + full package tests 66/66 PASS | + +Both slices match the design checkpoint ordering and named gates. Supervisor sign-off commit +(`f00d6338e`) names what the slice proves, not what it contains. + +## Verification of the five close-gated behaviors + +### 1. Invalid/throwing/failed output settles a failed receipt; green settlement occurs only after output validation, central bounds, and revalidation + +**PASS.** `mcp-server.ts` diff confirms the lifecycle ordering: + +- **Throw path**: `tool.flow(input)` wrapped in try/catch. On throw → `settleFlowReceipt(flow, input, false)` then `rpcError(-32603, 'tool_execution_failed')`. +- **Error result path**: `!execution.ok` → `settleFlowReceipt(flow, input, false)` then `rpcResult` with `isError: true`. +- **Validation/bound path**: `validateSchema(outputSchema, execution.value)` → `truncateResult(execution.value, policy)` → `validateSchema(outputSchema, bounded)`. On any throw → `settleFlowReceipt(flow, input, false)` then `rpcError(-32603, 'invalid_tool_result')` or `tool_result_too_large`. +- **Green path**: only after all three stages succeed → `settleFlowReceipt(flow, input, resultSucceeded(execution.value))` where `resultSucceeded` returns false when `status === 'fail'`. + +**Evidence:** `mcp-server.ts:107-148` diff; test `invalid MCP tool output replaces stale green evidence with a failed receipt` (PASS); test `throwing MCP tool flow replaces stale green evidence with a failed receipt` (PASS). + +### 2. A 75-row result capped to 50 cannot retain `truncated: false`; nested caps propagate only into already-existing boolean metadata + +**PASS.** `truncation.ts` recursive `truncateValue` returns `TruncationOutcome` pairs. For arrays: +`truncated: value.length > policy.maxItems || outcomes.some((o) => o.truncated)`. For objects: if +any descendant was truncated AND the object has an own boolean `truncated` property, it is flipped +to `true`. The rewrite is guarded by `typeof bounded.truncated === 'boolean'` — no new metadata +fields are invented, and non-boolean properties named `truncated` are untouched. + +**Evidence:** `truncation.ts:31-55` diff; test `75 rows cannot become 50 rows with false truncation metadata` (PASS: `rows.length === 50`, `truncated === true`); test `nested truncation marks existing metadata on every declaring ancestor` (PASS: both `page.truncated` and root `truncated` flipped). + +### 3. A fixed post-truncation UTF-8 byte ceiling rejects irreducibly oversized results before response and receipt success + +**PASS.** `truncateResult` computes `JSON.stringify(outcome.value)` byte length via `TextEncoder`. +If `byteLength > MAX_SERIALIZED_RESULT_BYTES` (65,536 bytes, 64 KiB), throws `ResultByteLimitError`. +The runner catches this in the validation/bound try/catch, settles a failed receipt, and returns +`rpcError(-32603, 'tool_result_too_large')` — before any green receipt can be written. + +**Evidence:** `truncation.ts:12-14, 23-28` diff; `mcp-server.ts:119-127` diff; test `irreducibly large bounded results fail the UTF-8 byte ceiling` (PASS: `assertThrows` with `ResultByteLimitError` and message `'after truncation; limit is 65536'`). + +### 4. No public export-map/type surface, `deno.lock`, MCP v2 registration shape, prohibited ignore, or double-cast movement occurred + +**PASS.** + +- `git diff fb75cf6fc..HEAD -- packages/mcp/deno.json packages/mcp/mod.ts deno.lock` — zero output (no changes). +- `git diff` prohibited pattern audit — zero occurrences of `deno-lint-ignore`, `@ts-ignore`, `as unknown as`, `as any`. +- `ResultByteLimitError` and `withFlowReceipt` are consumed only by the runner and by test files via internal paths (`../src/application/runner/...`); neither is re-exported from `mod.ts` or `cli.ts`. +- `createMcpServer`, `createMcpCliServer`, `truncateResult`, `TruncationPolicy`, `DEFAULT_TRUNCATION_POLICY` retain their existing source-compatible signatures. +- `MCP-A6-V2-SHAPE` debt (arch-debt.md line 2096) is adjacent and untouched: no folder structure, tool registration, or command tree changes. + +**Evidence:** raw `git diff` empty for `deno.json`/`mod.ts`/`deno.lock`; `grep -cE` prohibited patterns returned 0; `deno task doc:lint --root packages/mcp --pretty` — 2 entrypoints, 0 diagnostics (unchanged from baseline). + +### 5. Required targeted/package tests, scoped check/lint/fmt, `quality:gate`, close-gate/DoD evidence, commit slices, and debt disposition satisfy the protocol + +**PASS.** + +| Gate | Command | Result | +| --- | --- | --- | +| Receipt/doctor fixture | `deno test --allow-env --allow-net --allow-run --allow-read --allow-write tests/drift-evidence_test.ts tests/doctor_test.ts` | 16 passed, 0 failed | +| Truncation fixture | `deno test tests/truncation_test.ts` | 4 passed, 0 failed | +| Full package tests | `deno test --allow-env --allow-net --allow-run --allow-read --allow-write tests/` | 66 passed, 0 failed | +| Scoped check | `run-deno-check.ts --root packages/mcp --ext ts,tsx` | 68 files, 0 occurrences | +| Scoped lint | `run-deno-lint.ts --root packages/mcp --ext ts,tsx --config packages/mcp/deno.json` | 68 files, 0 occurrences | +| Scoped format | `run-deno-fmt.ts --root packages/mcp --ext ts,tsx --config packages/mcp/deno.json` | 68 files, 0 findings | +| Quality scan | `deno task quality:scan` | 0 findings, 7 allowances (all pre-existing, none in `packages/mcp`) | +| Architecture check | `deno task arch:check` | exit 0; `@netscript/mcp` 0 FAIL, 0 WARN, 0 INFO | +| Baseline doc lint | `deno task doc:lint --root packages/mcp --pretty` | 2 entrypoints, 0 diagnostics | +| Publish dry-run | conditional | N/A — no export drift occurred per verification point 4 | + +`e2e:cli` is explicitly N/A by the orchestrator brief (non-release PR slice). + +**Close-gate / Definition-of-Done disposition:** PR #1183 body explicitly states _"the closing keyword will be added only after both close-gated fixtures pass with quoted evidence"_ and the Definition-of-Done checkboxes are intentionally unchecked pending this evaluator verdict. The orchestrator brief names this as intentional lifecycle ordering, not an implementation defect. The evaluator confirms the underlying evidence exists: both close-gated fixtures pass with the exact behaviors named in #1134's acceptance criteria (invalid output → failed receipt; 75 rows → `truncated: true`). The PR's per-slice comments (`[PHASE: RESEARCH]`, `[PHASE: PLAN]`, `[PHASE: PLAN-EVAL] [VERDICT: APPROVED]`) are present; post-implementation slice comments are pending per the orchestrator's post-verdict finalization lifecycle. + +**Debt disposition:** `MCP-A6-V2-SHAPE` (arch-debt.md line 2096) is adjacent and unchanged — the PR neither closes nor deepens it. No new debt entries were created. `arch-debt.md` was not modified in this branch. + +**Commit slices:** Two implementation commits (`f00d6338e`, `8a0314931`) plus two plan/gate commits (`5192b07a9`, `0ade6736a`), matching the design checkpoint's two-slice structure. + +## Archetype-2 full-column verification + +| Gate family | Required | Result | Evidence | +| --- | --- | --- | --- | +| Fitness F-1..F-5 | yes | PASS | `quality:gate` (scan + arch:check) exit 0; scoped check/lint/fmt 0 findings | +| Fitness F-6/F-7 | yes | PASS | Baseline doc-lint 0 diagnostics; publish dry-run N/A (no export drift) | +| Fitness F-8..F-12 | yes | PASS | `quality:gate` + targeted fixtures 66/66 | +| Fitness F-14..F-19 | yes | PASS | `quality:gate` + diff audit (0 prohibited patterns) + wrapper-sourced results | +| Static | yes | PASS | scoped check/lint/fmt + doc-lint + prohibited-pattern diff scan | +| Runtime/consumer | touched | PASS | targeted receipt/doctor/truncation fixtures; consumer import gate N/A (exports locked) | + +## Findings + +No implementation findings. The implementation is complete, correct, and satisfies every locked +decision (D1–D5) and every risk mitigation from the approved plan. + +One lifecycle observation: the PR body's Definition-of-Done checkboxes, issue #1134's acceptance +checkboxes, and the closing keyword are all intentionally deferred until after this IMPL-EVAL +verdict, per the orchestrator brief and the PR body's own explicit statement. This is a +post-verdict administrative step, not an implementation or documentation defect. + +## Verdict + +`PASS` + +## Evidence summary + +- Source diff: 4 package source files changed (cli.ts, mcp-server.ts, receipt-lifecycle.ts, truncation.ts), 2 test files changed (drift-evidence_test.ts, truncation_test.ts), 7 harness artifacts. +- Test evidence: 66 passed, 0 failed across the full MCP package suite; 3 new close-gated fixtures all pass. +- Static evidence: scoped check/lint/fmt 0 findings on 68 TypeScript files; quality:scan 0 findings; arch:check exit 0; doc-lint 0 diagnostics. +- No export drift, no lock churn, no prohibited patterns, no new debt. +- Plan-Gate passed before implementation; design checkpoint present and followed; commit slices match design. +- Close-gate evidence is complete in substance (all acceptance behaviors verified by passing fixtures); PR body and issue checkbox updates are the documented post-verdict lifecycle step. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan-eval.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan-eval.md new file mode 100644 index 0000000000..4cecd1b4c3 --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan-eval.md @@ -0,0 +1,72 @@ +# PLAN-EVAL — fix-mcp-truncation-receipt-ordering--s8 + +- Plan evaluator session: Claude Code + OpenRouter / qwen3.7-max, 2026-08-03 +- Run: `fix-mcp-truncation-receipt-ordering--s8` +- Surface / archetype: `packages/mcp` / Archetype 2 — Integration (RFC #1123 S-20) +- Scope overlays: none + +## Checklist results + +| Plan-Gate item | Result | Evidence / location | +| --------------------------------------- | ------ | ------------------- | +| Research present and current | PASS | `research.md` exists; re-baselined against `main @ fb75cf6fc`; 6 findings each with verification path; spot-checked F-1 (truncation.ts:22-23 caps arrays without `truncated` flag) and F-3 (cli.ts:175-209 `withReceipt` writes receipt before runner output validation at mcp-server.ts:104-112) — both confirmed against tree. | +| Decisions locked | PASS | Plan §Locked Decisions D1–D5 each state decision + rationale. D1 (internal lifecycle callback) keeps persistence at CLI composition without changing `ToolFlow`/options. D2 (settle failure for all failure modes, success only after validated bounding) addresses F-3/F-4 root cause. D3 (recursive mutation metadata) solves the `{ rows: 75, truncated: false }` impossibility. D4 (fixed internal byte ceiling; reject rather than delete) enforces bound without schema damage. D5 (two semantic slices after bootstrap) keeps each close-gated behavior independently reviewable. | +| Open-decision sweep | PASS | Plan §Open-Decision Sweep lists 3 items: publicly configurable byte ceiling (safe to defer — fixed internal ceiling suffices; public option = export drift), generic `count` recomputation (safe to defer — only explicit `truncated` metadata is safe), MCP v2-shape migration (safe to defer — adjacent debt `MCP-A6-V2-SHAPE` owned elsewhere). None would force rework if deferred. Evaluator-run sweep found no additional open decisions. | +| Commit slices (< 30, gate + files each) | PASS | 2 slices (worklog Design §Commit Slices). Slice 1: receipt-after-validation lifecycle + failed-attempt fixture; gate: targeted `drift-evidence_test.ts` + `doctor_test.ts`; files: `cli.ts`, `mcp-server.ts`, one internal runner lifecycle file, receipt tests, run artifacts. Slice 2: honest truncation metadata + byte ceiling; gate: targeted `truncation_test.ts` + runner/package tests; files: `truncation.ts`, `mcp-server.ts` if error mapping needed, truncation tests, run artifacts. Both < 30, each names gate and files. | +| Risk register | PASS | Plan §Risk Register lists 5 risks with mitigations: evidence callback failure masking tool result (preserve warning-only behavior + fixture), receipt settling before bound failure (bound+revalidate first, settle success last), byte enforcement breaking schemas (never delete object properties — bounded runner error instead), generic metadata rewrite (only rewrite `truncated` key with boolean value), lock/export churn (verify raw diff per slice; stop on `deno.lock`/`deno.json`/`mod.ts`/exported-type movement). | +| Gate set selected | PASS | Plan §Fitness Gates covers the full Archetype-2 column: F-1..F-5 via `quality:gate` + scoped wrappers, F-6/F-7 via baseline doc-lint + conditional publish dry-run, F-8..F-12 via `quality:gate` + scoped lint/check/fmt + targeted fixtures, F-14..F-19 via `quality:gate` + manual diff audit + wrapper-sourced results. Runtime/consumer: targeted runner/receipt/truncation tests; consumer import gate N/A (exports unchanged). Archetype-2 fitness gate column fully addressed per `archetype-gate-matrix.md`. Static gates, runtime (optional for A2 — targeted fixtures suffice, no external backend), and consumer (optional for A2 — locked surface) gates correctly scoped. | +| Deferred scope explicit | PASS | Plan §Non-Scope names 5 categories: MCP v2 folder/registration shapes + `MCP-A6-V2-SHAPE` debt, OpenAPI introspection tools, evidence-class receipt keys (S-16), receipt acceptance (#1136), public export-map/symbol changes/lockfile/CLI scaffold E2E/package restructuring. Worklog Design §Deferred Scope mirrors. | +| jsr-audit surface scan (pkg/plugin) | PASS | `research.md` §jsr-audit surface scan: scanned `deno.json`, `mod.ts`, `cli.ts`, full 2-entrypoint `doc:lint` result. Current metadata complete (scoped name, version, description, license, exports, publish include/exclude, ESM entrypoints). Doc-lint baseline: 0 diagnostics. Planned surface risk: **none** — fix adds no export-map entry, changes no symbol re-exported by `mod.ts` or `cli.ts`. Conditional contract: if implementation forces an exported type/signature change → stop, log drift, add scoped doc-lint + publish dry-run evidence. Validation plan step 9 codifies this: `deno task doc:lint` always runs; publish dry-run runs only if exported surface changes. | + +## Open-decision sweep (evaluator-run) + +None. The three plan-listed deferred decisions are correctly classified: + +1. **Publicly configurable byte ceiling** — deferring avoids export drift; the internal fixed ceiling satisfies this correctness slice. No rework. +2. **Generic `count` recomputation** — only explicit `truncated` metadata is safe to mutate generically; existing `count` fields have different per-domain semantics. Correct deferral. +3. **MCP v2-shape migration** — adjacent `MCP-A6-V2-SHAPE` debt has its own owner (#721/S7), target, and gate. Touching it would rescope this PR. + +No additional open decisions found that would force rework if deferred. + +## Archetype-2 full-column verification + +The plan correctly applies the **complete** Archetype-2 gate column from `archetype-gate-matrix.md`: + +- **Fitness gates F-1..F-12**: all required for A2, all addressed via `quality:gate`, scoped wrappers, and targeted fixtures. F-13 is `n/a` for A2 (saga invariants). +- **Fitness gates F-14..F-19**: all required for A2, all addressed via `quality:gate`, manual diff audit, and wrapper-sourced results. +- **Static gates**: required for A2, addressed via scoped check/lint/fmt/doc-lint and prohibited-pattern diff scan (validation steps 4-8). +- **Runtime/Aspire validation**: `optional` for A2; plan correctly scopes to targeted MCP fixtures without requiring an external backend. +- **Consumer import validation**: `required` for A2; plan correctly classifies as N/A because exports remain unchanged — and provides the escalation trigger (stop if surface moves). + +## MCP-A6-V2-SHAPE debt interaction + +The adjacent `MCP-A6-V2-SHAPE` debt (arch-debt.md line 2096) classifies the broader MCP package shape as a horizontal Archetype-6 skeleton deviation, owned by epic #721 / S7 CLI integration slice. This PR's Archetype-2 classification is RFC-locked (S-20) for the runner/evidence seam and does not touch folder structure, tool registration, or command tree shapes. The plan correctly states "none" for debt action and "no folder/tool-registration restructure" in the arch-debt implications table. This PR neither closes nor deepens the debt entry. + +## Conditional doc-lint / publish contract + +The plan's F-6/F-7 evidence contract is sound: +- Baseline `deno task doc:lint --root packages/mcp --pretty` runs unconditionally (validation step 9, already evidenced as PASS in worklog §Static Gates). +- Publish dry-run (`deno task publish:dry-run` scoped to `packages/mcp`) is mandatory **only if** export drift occurs per the slice contract. +- The research.md surface scan and the plan's locked decisions both confirm no export drift is planned. +- If implementation forces an exported type/signature change, the plan mandates: stop, log significant drift, add scoped doc-lint + publish dry-run evidence before proceeding. + +This conditional approach is appropriate for a correctness fix that explicitly does not change the public surface. + +## Verdict + +`PASS` + +## Implementation authorization + +Implementation may begin. Two commit slices are authorized: + +1. **Slice 1** — Receipt-after-validation lifecycle and failed-attempt fixture (cli.ts, mcp-server.ts, internal runner lifecycle file, receipt tests). +2. **Slice 2** — Honest truncation metadata and whole-result byte ceiling (truncation.ts, mcp-server.ts if error mapping needed, truncation tests). + +Implementation must honor the locked decisions D1–D5, the no-export-drift constraint, and the conditional publish dry-run trigger. Any exported surface change stops the slice and triggers drift logging + full F-6/F-7 evidence before proceeding. + +## Notes + +- The plan's "Hidden Scope" section correctly identifies the settlement ordering hazard (receipt must settle after **both** schema validation and central bounding) — this is the core correctness fix and the most implementation-sensitive element. +- The D3 recursive truncation metadata propagation (flip every ancestor `truncated` boolean when a descendant is capped) is the most novel mechanism; the plan's risk register correctly guards against generic metadata rewrites touching unrelated booleans. +- The D4 "reject rather than delete" byte ceiling policy is architecturally sound — it preserves advertised schemas and avoids a public `TruncationPolicy` change. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan.md new file mode 100644 index 0000000000..57daa633f8 --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/plan.md @@ -0,0 +1,139 @@ +# Plan: OMB S8 existing-machinery correctness fixes (#1134) + +## Run Metadata + +| Field | Value | +| --- | --- | +| Run ID | `fix-mcp-truncation-receipt-ordering--s8` | +| Branch | `fix/mcp-truncation-receipt-ordering` | +| Phase | `plan` | +| Target | `packages/mcp` | +| Archetype | `2 - Integration` (RFC S-20 authority for this wave) | +| Scope overlays | `none` | + +## Archetype + +Archetype 2 is locked by RFC #1123 S-20: MCP tools are bounded flows at external telemetry, +filesystem, process, and protocol boundaries. This slice changes only the existing runner policy +and evidence-persistence seam. It adds no runtime lifecycle, command tree, port, adapter, or plugin. +The full Archetype-2 gate column applies even though the adjacent historical debt classifies the +broader MCP package shape differently. + +## Current Doctrine Verdict + +The old doctrine census predates `@netscript/mcp` and has no package row. The persistent registry +contains `MCP-A6-V2-SHAPE`, an accepted horizontal-shape debt. This PR neither closes nor deepens it; +the RFC's slice-specific Archetype-2 gate classification governs validation. + +## Axioms in Play + +| Axiom | Why it matters | +| --- | --- | +| A1/A2 | Preserve the published MCP result and evidence contracts; no export drift. | +| A6/A8 | One named internal receipt-lifecycle seam; no generic helper or package restructure. | +| A10 | Evidence persistence remains wired at the CLI composition edge. | +| A13/A14 | Thrown/invalid results become explicit failed attempts and fixtures preserve the rule. | + +## Goal + +Make central truncation honest and make diagnostic receipts reflect only completely validated, +bounded tool attempts, satisfying both close-gated fixtures in #1134. + +## Scope + +- Propagate central truncation into existing `truncated` metadata. +- Enforce a fixed UTF-8 serialized-result byte ceiling without changing public policy types. +- Move receipt settlement to the runner lifecycle after output validation and bounding. +- Record failed receipts for flow failures, throws, validation failures, and bound failures. +- Add focused fixtures for invalid output and 75-row truncation metadata. + +## Non-Scope + +- MCP v2 folder/registration shapes and `MCP-A6-V2-SHAPE` debt. +- OpenAPI introspection tools, evidence-class receipt keys (S-16), and receipt acceptance (#1136). +- Public export-map or symbol changes, lockfile updates, CLI scaffold E2E, package restructuring. + +## Hidden Scope + +- The receipt must settle after both schema validation and central bounding; otherwise an oversized + result could recreate S-15 with a green receipt followed by a runner error. +- The bounded value is revalidated before success settlement so central policy cannot make the + advertised output contract false. + +## Locked Decisions + +| ID | Decision | Rationale | +| --- | --- | --- | +| D1 | Add an internal flow-receipt lifecycle marker/callback shared by `cli.ts` and the runner. | Keeps persistence at composition, validation in the runner, and public `ToolFlow`/options unchanged. | +| D2 | Settle failure for flow-returned errors, throws, invalid output, and byte-limit failure; settle success only after validated bounding. | Prevents green and stale-green evidence for incomplete attempts. | +| D3 | Recursion returns mutation metadata and flips every existing ancestor `truncated` boolean when a descendant is capped. | Makes `{ rows: 75, truncated: false }` impossible after a 50-row central cap without inventing schema fields. | +| D4 | Use an internal fixed UTF-8 serialized-result ceiling; reject still-oversized results instead of deleting object properties. | Enforces the bound while preserving advertised schemas and avoiding a public `TruncationPolicy` change. | +| D5 | Keep the two semantic changes as separate implementation slices after one plan/bootstrap commit. | Each close-gated behavior remains independently reviewable and evidenced. | + +## Open-Decision Sweep + +| Decision | Status | Notes | +| --- | --- | --- | +| Publicly configurable byte ceiling | safe to defer | A fixed internal ceiling satisfies this correctness slice; a public option would be export drift. | +| Generic `count` recomputation | safe to defer | Existing `count` fields have different meanings; only explicit `truncated` metadata is safe to mutate generically. | +| MCP v2-shape migration | safe to defer | Explicit adjacent debt owned elsewhere; touching it would rescope this PR. | + +## Risk Register + +| Risk | Mitigation | +| --- | --- | +| Evidence callback failure masks the tool result | Preserve existing warning-only behavior in the CLI callback and fixture it. | +| Receipt settles before a later bound failure | Bound and revalidate first; settle success last. | +| Byte enforcement breaks valid schemas by deleting keys | Never delete object properties for the byte ceiling; return a bounded runner error instead. | +| Generic metadata rewrite changes unrelated booleans | Only rewrite properties whose exact key is `truncated` and whose value is boolean. | +| Lock or export churn enters the slice | Verify raw git diff/status per slice; stop on `deno.lock`, `deno.json`, `mod.ts`, or exported-type movement. | + +## Anti-Patterns to Resolve or Avoid + +| AP | Status | Plan | +| --- | --- | --- | +| AP-1 | risk | Keep new internal lifecycle module and fixtures focused and below size gates. | +| AP-8/AP-9 | risk | Use one concrete callback seam; no container, port, registry, or configurable strategy. | +| AP-11 | risk | No mutable module-global recorder map; attach lifecycle data to the wrapped flow. | +| AP-13/AP-25 | risk | Filesystem writes stay in CLI composition; runner invokes only an injected callback. | +| AP-16/AP-22 | risk | Use existing role folders and no new barrel. | + +## Fitness Gates + +| Gate | Required | Expected evidence | +| --- | --- | --- | +| F-1..F-5 | yes | `quality:gate`, scoped wrappers, source review, unchanged public exports | +| F-6/F-7 | yes | Baseline full-export doc-lint is green; publish dry-run becomes mandatory only if export drift occurs per slice contract | +| F-8..F-12 | yes | `quality:gate` plus scoped lint/check/fmt and targeted fixtures | +| F-14..F-19 | yes | `quality:gate`, manual diff audit, wrapper-sourced results | +| Runtime/consumer | touched | Targeted runner/receipt/truncation tests; no consumer import gate because exports remain unchanged | + +## Arch-Debt Implications + +| Entry | Action | Notes | +| --- | --- | --- | +| `MCP-A6-V2-SHAPE` | none | Adjacent and untouched; no folder/tool-registration restructure. | +| New debt | none expected | Any required public surface or package-shape movement triggers drift/rescope, not silent debt. | + +## Validation Plan + +| Order | Gate | Command or check | Expected result | +| --- | --- | --- | --- | +| 1 | receipt fixture | `deno test --allow-env --allow-net --allow-run --allow-read packages/mcp/tests/drift-evidence_test.ts packages/mcp/tests/doctor_test.ts` | invalid/throwing output writes exit 1; ordinary receipt behavior remains green | +| 2 | truncation fixture | `deno test packages/mcp/tests/truncation_test.ts` | 75 rows cap to 50 with `truncated: true`; byte ceiling enforced | +| 3 | package tests | `deno test --allow-env --allow-net --allow-run --allow-read packages/mcp/tests/` | all MCP fixtures pass | +| 4 | scoped check | `deno run --allow-read --allow-run .llm/tools/run-deno-check.ts --root packages/mcp --ext ts,tsx` | PASS | +| 5 | scoped lint | `deno run --allow-read --allow-run .llm/tools/run-deno-lint.ts --root packages/mcp --ext ts,tsx` | PASS | +| 6 | scoped format | `deno run --allow-read --allow-run .llm/tools/run-deno-fmt.ts --root packages/mcp --ext ts,tsx` | PASS | +| 7 | framework law | `deno task quality:gate` | quality scan + architecture check PASS | +| 8 | prohibited-pattern audit | diff scan for `deno-lint-ignore`, `as unknown as`, `@ts-ignore`, and `deno.lock` | no additions/churn | +| 9 | conditional JSR | `deno task doc:lint --root packages/mcp --pretty` and package publish dry-run | run only if exported surface changes; otherwise baseline doc-lint + unchanged-surface evidence | + +## Dependencies + +- Hard-blocks #1136 and #1132 semantics; no implementation dependency within this branch. + +## Drift Watch + +- Exported types/options, output schema changes, byte-policy configurability, MCP folder shape, + receipt-key semantics, and any lockfile mutation. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/research.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/research.md new file mode 100644 index 0000000000..e1f7e1ed6e --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/research.md @@ -0,0 +1,40 @@ +# Research — fix-mcp-truncation-receipt-ordering--s8 + +## Re-baseline + +- Carried-in sources: issue #1134; RFC PR #1123 §4; seed-run findings S-13 and S-15 under + `.llm/runs/plan-openapi-mcp-plugin--seed/`. +- Re-derived against `main` @ `fb75cf6fc5ad02130ada0ac42e6f44035ac03a9b` on 2026-08-03. +- What changed vs the carried-in version: + - Nothing material. The central truncator still slices arrays to 50 without propagating + truncation metadata and still has no serialized-result byte ceiling. + - Receipt persistence still occurs in the CLI-local `withReceipt` wrapper before the runner's + output-schema validation. The prompt's approximate location names `mcp-server.ts`, while the + wrapper itself is in `cli.ts`; the RFC already describes this as a wrapper/runner integration. + +## Findings + +| # | Finding | How to verify | +| - | ------- | ------------- | +| 1 | `truncateResult` recursively caps strings/arrays but drops array elements without setting an existing sibling `truncated` flag. | `packages/mcp/src/application/runner/truncation.ts`; `packages/mcp/tests/truncation_test.ts` | +| 2 | The central pass has no UTF-8 serialized-byte ceiling and object property count is unbounded. | `packages/mcp/src/application/runner/truncation.ts:13-31`; S-13 in `adversarial-sol.md` | +| 3 | `withReceipt` writes a success receipt as soon as a flow resolves; output validation happens later in `createMcpServer().handle`. | `packages/mcp/cli.ts:175-211`; `packages/mcp/src/application/runner/mcp-server.ts:96-116` | +| 4 | A throwing flow bypasses receipt writing and escapes the runner, so an older green receipt can remain current. | Same files; no catch around `tool.flow` on baseline | +| 5 | Both package entrypoints are doc-lint clean and the requested behavior can remain internal. | `deno task doc:lint --root packages/mcp --pretty` → 2 entrypoints, 0 diagnostics | +| 6 | The adjacent `MCP-A6-V2-SHAPE` debt is active and explicitly out of scope. | `.llm/harness/debt/arch-debt.md` entry `MCP-A6-V2-SHAPE` | + +## jsr-audit surface scan (package/plugin waves) + +- Surface scanned: `packages/mcp/deno.json`, `mod.ts`, `cli.ts`, and the full two-entrypoint + `doc:lint` result. +- Current metadata: scoped name, version, description, license, exports, publish include/exclude, + and ESM entrypoints are present. +- Current documentation bar: `doc:lint` reports 0 private-type, missing-JSDoc, or other errors. +- Planned surface risk: none. The fix adds no export-map entry and changes no symbol re-exported by + `mod.ts` or `cli.ts`. If implementation forces an exported type/signature change, stop, log + significant drift, and add scoped doc-lint plus publish dry-run evidence before proceeding. + +## Open questions + +- None that force rework. The byte ceiling is locked as an internal fixed policy so the exported + `TruncationPolicy` shape does not move. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/supervisor.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/supervisor.md new file mode 100644 index 0000000000..d3223d0b3f --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/supervisor.md @@ -0,0 +1,41 @@ +# Supervisor Identity — fix-mcp-truncation-receipt-ordering--s8 + +Written at run start per `workflow/lane-policy.md` § Supervisor identity. + +| Field | Value | +| --- | --- | +| Model | Codex · OpenAI · GPT-5 | +| Session | Codex root session (this worktree) | +| Host | Linux native ext4 workspace | +| Checkout | `/home/codex/repos/ns005-receipts` | +| Worktree | `/home/codex/repos/ns005-receipts` | +| Branch | `fix/mcp-truncation-receipt-ordering` | +| Baseline | `fb75cf6fc5ad02130ada0ac42e6f44035ac03a9b` (`origin/main`, 2026-08-03) | +| Run ID | `fix-mcp-truncation-receipt-ordering--s8` | + +## Routes in force + +| Task lane | Provider / model / effort | Role in this run | +| --- | --- | --- | +| `planning_decisions` | Codex root-session fallback already selected by host | Supervisor, research, plan, implementation sign-off | +| `normal_implementation` | Codex · OpenAI · Sol-class implementation route | Two focused source/test slices | +| `review_codex` | Claude · Anthropic · Fable-class review route | Ordinary opposite-family slice review | +| `formal_evaluation` | Claude Code · OpenRouter · `qwen/qwen3.7-max` · high | Separate PLAN-EVAL and IMPL-EVAL sessions | + +## Recorded lane/eval overrides + +The active Codex host is the supervisor/generator session rather than the canonical Claude +orchestrator. This is the owner-provided implementation-supervisor context; formal evaluation +remains on the canonical open-model route and ordinary source review remains opposite-family. + +The slice-1 `review_codex` primary could not start because the installed Claude CLI rejected the +configured Fable model id. The policy-declared Claude-family `token_limit_fallback` route (Opus, +same low effort) is used for ordinary review; formal evaluation is unchanged. + +## Supervisor sign-off + +- Slice 1: receipt settlement remains an internal composition detail; flow identity is preserved + in the registry and receipt writes retain CLI warning-only failure behavior. +- Slice 2: metadata changes only where the result already declares a boolean `truncated` field, + avoiding output-schema invention. Byte accounting uses `TextEncoder` after recursive bounds and + before the runner's second schema validation and receipt settlement. diff --git a/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/worklog.md b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/worklog.md new file mode 100644 index 0000000000..dd648bb591 --- /dev/null +++ b/.llm/runs/fix-mcp-truncation-receipt-ordering--s8/worklog.md @@ -0,0 +1,118 @@ +# Worklog: OMB S8 existing-machinery correctness fixes + +## Run Metadata + +| Field | Value | +| --- | --- | +| Run ID | `fix-mcp-truncation-receipt-ordering--s8` | +| Branch | `fix/mcp-truncation-receipt-ordering` | +| Archetype | `2 - Integration` | +| Scope overlays | `none` | + +## Design + +### Public Surface + +- No new or changed package export. `.` and `./cli` retain their current symbol sets. +- Existing `createMcpServer`, `createMcpCliServer`, `truncateResult`, `TruncationPolicy`, and + diagnostic evidence contracts remain source-compatible. + +### Domain Vocabulary + +- `FlowReceiptRecorder` — internal callback that settles one diagnostic attempt after the runner + determines its final outcome. +- `TruncationOutcome` — internal recursive pair of bounded value plus whether any descendant was + capped. +- `ResultByteLimitError` — internal signal that the post-truncation serialized value is still too + large to return honestly. + +### Ports + +- No new port. `DiagnosticEvidencePort` remains the existing persistence boundary and is consumed + only by CLI composition. + +### Constants + +- Internal serialized-result byte ceiling — one fixed runner policy value, not exported. +- Existing `DEFAULT_TRUNCATION_POLICY` — unchanged `{ maxItems, maxStringLength }` public shape. + +### Commit Slices + +| # | Slice | Gate | Files | +| - | ----- | ---- | ----- | +| 1 | Receipt-after-validation lifecycle and failed-attempt fixture | targeted `drift-evidence_test.ts` + `doctor_test.ts` | `cli.ts`, `src/application/runner/mcp-server.ts`, one internal runner lifecycle file, receipt tests, run artifacts | +| 2 | Honest truncation metadata and whole-result byte ceiling | targeted `truncation_test.ts` + runner/package tests | `src/application/runner/truncation.ts`, `mcp-server.ts` if error mapping is needed, truncation tests, run artifacts | + +### Deferred Scope + +- Evidence-class keys (S-16), introspection receipt acceptance, OpenAPI tool wiring, public byte + policy configuration, and MCP v2-shape work remain outside #1134. + +### Contributor Path + +Future diagnostic flows continue to use the CLI's `withReceipt` composition wrapper; the runner +owns settlement ordering centrally. Future bounded list results expose a boolean `truncated` +property if clients need truncation metadata; the central policy will flip that existing property +when it applies any descendant cap. + +## Progress Log + +| Time | Slice | Step | Notes | +| --- | --- | --- | --- | +| 2026-08-03 | bootstrap | research/design | Re-baselined #1134 and RFC S-13/S-15 against clean `origin/main`; baseline doc-lint 0 diagnostics. | +| 2026-08-03 | plan-eval | transport | Canonical Qwen evaluator launch timed out before any verdict/file change; retrying same route with a bounded timeout extension. | +| 2026-08-03 | plan-eval | verdict | Separate Claude Code + OpenRouter Qwen session returned `PASS`; both implementation slices authorized. | +| 2026-08-03 | slice 1 | gate | Targeted receipt/doctor tests PASS (16/16) with required `--allow-write`; scoped check PASS. Scoped lint/fmt wrappers PASS with explicit `packages/mcp/deno.json` after the root workspace shorthand produced a Deno 2.9 parser error. `quality:gate` PASS. | +| 2026-08-03 | slice 1 | review route | Fable primary rejected by installed Claude CLI before review; launching policy-declared Opus low fallback. | +| 2026-08-03 | slice 1 | supervisor sign-off | Diff reviewed against D1/D2; focused receipt/doctor gate repeated at 16 passed, 0 failed; committed as `f00d6338e`. | +| 2026-08-03 | slice 2 | implementation | Central recursive outcome propagation flips existing boolean `truncated` fields only; a fixed 65,536-byte UTF-8 post-bound ceiling rejects irreducibly large results with an internal error signal. | +| 2026-08-03 | slice 2 | targeted gate | Truncation + receipt + doctor fixtures PASS: 20 passed, 0 failed. Scoped package check/lint/fmt PASS across 68 TypeScript files with zero findings. | +| 2026-08-03 | final gates | framework wave | Full MCP suite PASS (66/66); `quality:gate` exit 0; doc lint 2 entrypoints/0 diagnostics; no export, lock, or prohibited-pattern churn. | +| 2026-08-03 | impl-eval | verdict | Separate guarded Claude Code + OpenRouter Qwen parent session returned `PASS`; it independently repeated tests, quality/doc gates, and the five close-gated behavior audits. | + +## Decisions + +| Decision | Reason | Source | +| --- | --- | --- | +| Internal lifecycle callback | Runner must settle after validation without changing public flow/options types. | plan D1; S-15 | +| Reject irreducibly oversized values | Enforces byte ceiling without deleting schema-required properties. | plan D4; S-13 | + +## Drift + +| Drift | Severity | Logged in drift.md | +| --- | --- | --- | +| Prompt locates `withReceipt` near `mcp-server.ts`; baseline wrapper is in `cli.ts`, with validation in `mcp-server.ts`. | minor | yes | + +## Gate Results + +### Static Gates + +| Gate | Command or check | Result | Notes | +| --- | --- | --- | --- | +| Baseline doc lint | `deno task doc:lint --root packages/mcp --pretty` | PASS | 2 entrypoints, 0 diagnostics before source work | + +### Fitness Gates + +| Gate | Result | Evidence | Notes | +| --- | --- | --- | --- | +| Full Archetype-2 column | PASS | `evaluate.md`; full package/static/framework evidence | No new or deepened doctrine debt | +| Plan-Gate | PASS | `plan-eval.md` | All eight checklist items passed; no unresolved decisions | +| Implementation evaluation | PASS | `evaluate.md` | Separate open-model session; no findings | + +### Runtime Gates + +| Gate | Result | Evidence | Notes | +| --- | --- | --- | --- | +| Receipt/doctor fixtures | PASS | 16 passed, 0 failed | Includes invalid-output and thrown-flow failed receipts | +| Truncation/receipt/doctor fixtures | PASS | 20 passed, 0 failed | Includes 75→50 with `truncated: true`, nested metadata propagation, and the UTF-8 byte ceiling | + +### Consumer Gates + +| Consumer | Result | Evidence | Notes | +| --- | --- | --- | --- | +| Public export consumers | N/A | locked unchanged surface | Escalates if surface moves | + +## Handoff Notes + +- PLAN-EVAL should inspect D1/D4 and verify the full Archetype-2 gate disposition, especially the + conditional F-6/F-7 evidence contract and the no-public-surface constraint. diff --git a/packages/mcp/cli.ts b/packages/mcp/cli.ts index 170b416bdb..201cc1ab44 100644 --- a/packages/mcp/cli.ts +++ b/packages/mcp/cli.ts @@ -41,8 +41,9 @@ import { SpawnCommandExecutor } from './src/infrastructure/spawn-command-executo import type { EmbeddedDocsSource } from './src/infrastructure/embedded-docs-corpus.ts'; import { FilesystemDiagnosticEvidence } from './src/infrastructure/filesystem-diagnostic-evidence.ts'; import { createRecordDriftFlow } from './src/application/flows/record-drift-flow.ts'; -import type { ToolExecutionResult, ToolFlow } from './src/domain/tool-types.ts'; +import type { ToolFlow } from './src/domain/tool-types.ts'; import type { DiagnosticEvidencePort } from './src/domain/diagnostic-evidence-port.ts'; +import { withFlowReceipt } from './src/application/runner/receipt-lifecycle.ts'; export * from './mod.ts'; @@ -178,18 +179,13 @@ function withReceipt( command: string, warn: (message: string) => void, ): ToolFlow { - return async (input): Promise => { - const result = await flow(input); + return withFlowReceipt(flow, async (input, succeeded): Promise => { const record = input && typeof input === 'object' ? input as Record : {}; const resource = typeof record.resource === 'string' ? record.resource : typeof record.service === 'string' ? record.service : 'project'; - const value = result.ok && result.value && typeof result.value === 'object' - ? result.value as Record - : undefined; - const succeeded = result.ok && value?.status !== 'fail'; try { await evidence.write({ resource, @@ -204,8 +200,7 @@ function withReceipt( `record_drift will refuse until a successful diagnostic writes a receipt. ${reason}`, ); } - return result; - }; + }); } if (import.meta.main) await runMcpStdioServer(); diff --git a/packages/mcp/src/application/runner/mcp-server.ts b/packages/mcp/src/application/runner/mcp-server.ts index 06980e7a67..33f891a996 100644 --- a/packages/mcp/src/application/runner/mcp-server.ts +++ b/packages/mcp/src/application/runner/mcp-server.ts @@ -2,10 +2,21 @@ import { createDoctorFlow } from '../flows/doctor-flow.ts'; import { createToolRegistry } from '../tool-registry.ts'; import type { TelemetryProbePort } from '../../domain/telemetry-probe-port.ts'; import { validateSchema } from '../../domain/schema.ts'; -import type { ToolDefinition, ToolFlow, ToolName } from '../../domain/tool-types.ts'; +import type { + ToolDefinition, + ToolExecutionResult, + ToolFlow, + ToolName, +} from '../../domain/tool-types.ts'; import { type JsonRpcResponse, parseJsonRpcRequest } from '../../domain/json-rpc.ts'; -import { DEFAULT_TRUNCATION_POLICY, truncateResult, type TruncationPolicy } from './truncation.ts'; +import { + DEFAULT_TRUNCATION_POLICY, + ResultByteLimitError, + truncateResult, + type TruncationPolicy, +} from './truncation.ts'; import { MCP_PACKAGE_VERSION } from '../../publish-assets.generated.ts'; +import { settleFlowReceipt } from './receipt-lifecycle.ts'; /** Current stable MCP protocol revision implemented by the runner. */ export const MCP_PROTOCOL_VERSION = '2025-11-25'; @@ -93,23 +104,43 @@ export function createMcpServer(options: McpServerOptions): McpServer { error instanceof Error ? error.message : 'Invalid tool arguments', ); } - const execution = await tool.flow(input); + let execution: ToolExecutionResult; + try { + execution = await tool.flow(input); + } catch { + await settleFlowReceipt(tool.flow, input, false); + return rpcError(request.id, -32603, 'Tool execution failed', { + code: 'tool_execution_failed', + message: 'The tool flow did not complete.', + }); + } if (!execution.ok) { + await settleFlowReceipt(tool.flow, input, false); return rpcResult(request.id, { content: [{ type: 'text', text: execution.error.message }], structuredContent: execution.error, isError: true, }); } + let bounded: unknown; try { validateSchema(tool.outputSchema, execution.value); + bounded = truncateResult(execution.value, policy); + validateSchema(tool.outputSchema, bounded); } catch (error) { + await settleFlowReceipt(tool.flow, input, false); + if (error instanceof ResultByteLimitError) { + return rpcError(request.id, -32603, 'Tool result exceeds the transport limit', { + code: 'tool_result_too_large', + message: error.message, + }); + } return rpcError(request.id, -32603, 'Tool returned an invalid structured result', { code: 'invalid_tool_result', message: error instanceof Error ? error.message : 'Output contract validation failed', }); } - const bounded = truncateResult(execution.value, policy); + await settleFlowReceipt(tool.flow, input, resultSucceeded(execution.value)); return rpcResult(request.id, { content: [{ type: 'text', text: JSON.stringify(bounded) }], structuredContent: bounded as Record, @@ -119,6 +150,10 @@ export function createMcpServer(options: McpServerOptions): McpServer { }; } +function resultSucceeded(value: unknown): boolean { + return !(value !== null && typeof value === 'object' && Reflect.get(value, 'status') === 'fail'); +} + function rpcResult(id: string | number, result: Record): JsonRpcResponse { return { jsonrpc: '2.0', id, result }; } diff --git a/packages/mcp/src/application/runner/receipt-lifecycle.ts b/packages/mcp/src/application/runner/receipt-lifecycle.ts new file mode 100644 index 0000000000..69af6b82e3 --- /dev/null +++ b/packages/mcp/src/application/runner/receipt-lifecycle.ts @@ -0,0 +1,26 @@ +import type { ToolFlow } from '../../domain/tool-types.ts'; + +/** Internal receipt settlement callback attached to one composed flow. */ +export type FlowReceiptRecorder = (input: unknown, succeeded: boolean) => Promise; + +const FLOW_RECEIPT_RECORDER = Symbol('netscript.mcp.flow-receipt-recorder'); + +/** Attach receipt settlement to a flow without changing the public flow contract. */ +export function withFlowReceipt( + flow: ToolFlow, + recorder: FlowReceiptRecorder, +): ToolFlow { + const composed: ToolFlow = (input) => flow(input); + Object.defineProperty(composed, FLOW_RECEIPT_RECORDER, { value: recorder }); + return composed; +} + +/** Settle a composed flow's receipt after the runner determines its final outcome. */ +export async function settleFlowReceipt( + flow: ToolFlow, + input: unknown, + succeeded: boolean, +): Promise { + const recorder = Reflect.get(flow, FLOW_RECEIPT_RECORDER); + if (typeof recorder === 'function') await recorder(input, succeeded); +} diff --git a/packages/mcp/src/application/runner/truncation.ts b/packages/mcp/src/application/runner/truncation.ts index 31c27d22a4..81a4b021c2 100644 --- a/packages/mcp/src/application/runner/truncation.ts +++ b/packages/mcp/src/application/runner/truncation.ts @@ -9,23 +9,60 @@ export interface TruncationPolicy { /** Default token-discipline limits. */ export const DEFAULT_TRUNCATION_POLICY: TruncationPolicy = { maxItems: 50, maxStringLength: 2000 }; +const MAX_SERIALIZED_RESULT_BYTES = 64 * 1024; + +interface TruncationOutcome { + readonly value: unknown; + readonly truncated: boolean; +} + +/** Internal signal that a bounded result still exceeds the transport budget. */ +export class ResultByteLimitError extends Error { + constructor(readonly actualBytes: number) { + super( + `Tool result is ${actualBytes} UTF-8 bytes after truncation; limit is ${MAX_SERIALIZED_RESULT_BYTES}`, + ); + this.name = 'ResultByteLimitError'; + } +} + /** Recursively bound arrays and strings in a JSON-compatible result. */ export function truncateResult( value: unknown, policy: TruncationPolicy = DEFAULT_TRUNCATION_POLICY, ): unknown { + const outcome = truncateValue(value, policy); + const serialized = JSON.stringify(outcome.value); + const byteLength = new TextEncoder().encode(serialized).byteLength; + if (byteLength > MAX_SERIALIZED_RESULT_BYTES) throw new ResultByteLimitError(byteLength); + return outcome.value; +} + +function truncateValue(value: unknown, policy: TruncationPolicy): TruncationOutcome { if (typeof value === 'string') { - return value.length <= policy.maxStringLength - ? value - : `${value.slice(0, policy.maxStringLength)}…[truncated]`; + return value.length <= policy.maxStringLength ? { value, truncated: false } : { + value: `${value.slice(0, policy.maxStringLength)}…[truncated]`, + truncated: true, + }; } if (Array.isArray(value)) { - return value.slice(0, policy.maxItems).map((item) => truncateResult(item, policy)); + const outcomes = value.slice(0, policy.maxItems).map((item) => truncateValue(item, policy)); + return { + value: outcomes.map((outcome) => outcome.value), + truncated: value.length > policy.maxItems || outcomes.some((outcome) => outcome.truncated), + }; } if (value !== null && typeof value === 'object') { - return Object.fromEntries( - Object.entries(value).map(([key, item]) => [key, truncateResult(item, policy)]), + const outcomes = Object.entries(value).map(([key, item]) => + [ + key, + truncateValue(item, policy), + ] as const ); + const truncated = outcomes.some(([, outcome]) => outcome.truncated); + const bounded = Object.fromEntries(outcomes.map(([key, outcome]) => [key, outcome.value])); + if (truncated && typeof bounded.truncated === 'boolean') bounded.truncated = true; + return { value: bounded, truncated }; } - return value; + return { value, truncated: false }; } diff --git a/packages/mcp/tests/drift-evidence_test.ts b/packages/mcp/tests/drift-evidence_test.ts index bf234a6cac..676ffd40eb 100644 --- a/packages/mcp/tests/drift-evidence_test.ts +++ b/packages/mcp/tests/drift-evidence_test.ts @@ -3,6 +3,8 @@ import { DIAGNOSTIC_RECEIPT_TTL_MS, recordDrift } from '../mod.ts'; import type { DiagnosticEvidencePort, DiagnosticEvidenceReceipt } from '../mod.ts'; import { FilesystemDiagnosticEvidence } from '../mod.ts'; import { createMcpCliServer } from '../cli.ts'; +import { createMcpServer } from '../mod.ts'; +import { withFlowReceipt } from '../src/application/runner/receipt-lifecycle.ts'; class MemoryEvidence implements DiagnosticEvidencePort { receipt?: DiagnosticEvidenceReceipt; @@ -135,11 +137,78 @@ Deno.test('an actual MCP doctor call writes a diagnostic receipt', async () => { const receipt = await new FilesystemDiagnosticEvidence(root).read('api'); assertEquals(receipt?.command, 'mcp doctor'); assertEquals(receipt?.resource, 'api'); + assertEquals(receipt?.exitStatus, 1); } finally { await Deno.remove(root, { recursive: true }); } }); +Deno.test('invalid MCP tool output replaces stale green evidence with a failed receipt', async () => { + const evidence = new MemoryEvidence(); + evidence.receipt = { + resource: 'api', + command: 'mcp doctor', + timestamp: new Date(0).toISOString(), + exitStatus: 0, + }; + const invalidDoctor = withFlowReceipt( + () => Promise.resolve({ ok: true, value: { status: 'unknown' } }), + (_input, succeeded) => + evidence.write({ + resource: 'api', + command: 'mcp doctor', + timestamp: new Date().toISOString(), + exitStatus: succeeded ? 0 : 1, + }), + ); + const response = await createMcpServer({ + probe: { probe: () => Promise.resolve({ reachable: true, message: 'ready' }) }, + flows: { doctor: invalidDoctor }, + }).handle({ + jsonrpc: '2.0', + id: 4, + method: 'tools/call', + params: { name: 'doctor', arguments: { resource: 'api' } }, + }); + + assertEquals(response?.error?.code, -32603); + assertEquals((response?.error?.data as Record).code, 'invalid_tool_result'); + assertEquals(evidence.receipt?.exitStatus, 1); + assertEquals(evidence.receipt?.resource, 'api'); +}); + +Deno.test('throwing MCP tool flow replaces stale green evidence with a failed receipt', async () => { + const evidence = new MemoryEvidence(); + evidence.receipt = { + resource: 'api', + command: 'mcp doctor', + timestamp: new Date(0).toISOString(), + exitStatus: 0, + }; + const throwingDoctor = withFlowReceipt( + () => Promise.reject(new Error('fixture failure')), + (_input, succeeded) => + evidence.write({ + resource: 'api', + command: 'mcp doctor', + timestamp: new Date().toISOString(), + exitStatus: succeeded ? 0 : 1, + }), + ); + const response = await createMcpServer({ + probe: { probe: () => Promise.resolve({ reachable: true, message: 'ready' }) }, + flows: { doctor: throwingDoctor }, + }).handle({ + jsonrpc: '2.0', + id: 5, + method: 'tools/call', + params: { name: 'doctor', arguments: { resource: 'api' } }, + }); + + assertEquals((response?.error?.data as Record).code, 'tool_execution_failed'); + assertEquals(evidence.receipt?.exitStatus, 1); +}); + Deno.test('MCP doctor result survives a diagnostic evidence write failure', async () => { const warnings: string[] = []; const evidence: DiagnosticEvidencePort = { diff --git a/packages/mcp/tests/truncation_test.ts b/packages/mcp/tests/truncation_test.ts index 2719de57dd..5e6b9f8703 100644 --- a/packages/mcp/tests/truncation_test.ts +++ b/packages/mcp/tests/truncation_test.ts @@ -1,5 +1,6 @@ -import { assertEquals, assertStringIncludes } from '@std/assert'; +import { assertEquals, assertStringIncludes, assertThrows } from '@std/assert'; import { truncateResult } from '../mod.ts'; +import { ResultByteLimitError } from '../src/application/runner/truncation.ts'; Deno.test('truncation recursively bounds arrays and strings', () => { const result = truncateResult( @@ -9,3 +10,41 @@ Deno.test('truncation recursively bounds arrays and strings', () => { assertEquals(result.rows, ['abcd…[truncated]', 'two']); assertStringIncludes(JSON.stringify(result.nested), '1234…[truncated]'); }); + +Deno.test('75 rows cannot become 50 rows with false truncation metadata', () => { + const result = truncateResult({ + rows: Array.from({ length: 75 }, (_, index) => ({ index })), + truncated: false, + }) as { rows: unknown[]; truncated: boolean }; + + assertEquals(result.rows.length, 50); + assertEquals(result.truncated, true); +}); + +Deno.test('nested truncation marks existing metadata on every declaring ancestor', () => { + const result = truncateResult( + { + page: { + rows: ['long value'], + truncated: false, + }, + truncated: false, + }, + { maxItems: 50, maxStringLength: 4 }, + ) as { page: { truncated: boolean }; truncated: boolean }; + + assertEquals(result.page.truncated, true); + assertEquals(result.truncated, true); +}); + +Deno.test('irreducibly large bounded results fail the UTF-8 byte ceiling', () => { + const value = Object.fromEntries( + Array.from({ length: 40 }, (_, index) => [`field${index}`, 'x'.repeat(2000)]), + ); + + assertThrows( + () => truncateResult(value), + ResultByteLimitError, + 'after truncation; limit is 65536', + ); +});