Skip to content

mcpx: design — sampling trust boundary (reviewable, validated, honestly error-coded) #1834

Open
2 of 3 issues completed

Description

@dezren39

Design: mcpx sampling trust boundary — reviewable, validated, and honestly error-coded

Design doc: files/designs/code-mcpx-sampling-trust-boundary.md in the keep-the-four-repos-healthy goal workspace (developing-today/code design architect, pass 10, 2026-10-28).
Origin: dezren39/nix#256 — sampling relay: human review, validation of results, error mapping (8 conformance rows). This design scopes only its remainder (audit below); no Drew direction decision is needed — nix#256's acceptance is explicit and its scope is conformance-only (sampling is deprecated in 2026-07-28; standing instruction is keep-working, add-nothing-new).

Problem

mcpx has no model, so sampling is a relay with a trust decision inside it: an upstream asks for a completion, something answering for the user produces one, and mcpx hands it back as if the user had spoken. Verified on main 528886eb:

  1. A person cannot review a sampling request — the prompt is not shown. sampleViaBroker (pkgs/mcpx/internal/daemon/hooks.go ~L198-214) stores the request's system prompt as the broker question's Message (fallback: "<server> asks for a model completion") and the full request in the question's Schema field. But mcpx elicit's renderQuestion (pkgs/mcpx/internal/cli/elicit.go) renders Schema as a JSON Schema: describeSchema looks for properties, a sampling request has none, so the messages — the actual prompt — render as nothing and the answer example offered is '{}'. Routing never offers a person the question either: elicit.Route (internal/elicit/elicit.go ~L502-503) sends every Sample question to the agent audience unconditionally. Denial exists (decline is conformance-tested), but the spec's review rows have no surface. Gaps: sampling-app-review-ui, sampling-app-view-edit-prompts, sampling-app-review-responses, sampling-client-user-approval-controls.
  2. Nothing validates sampling messages or results, either direction. sampleViaBroker unmarshals only maxTokens/systemPrompt; messages is never inspected — tool-result messages with non-tool_result content, or a tool_result id no earlier tool_use issued, pass through (register SMP-06 pairing). On return, the only checks are JSON-object / not-null (~L226-236); a missing role/model is then silently filled in ("unknown") instead of judged, and the result is never checked against its request (stopReason: "toolUse" / tool_use content against a tools-less request; era content shape per SMP-05). Client.answer validates nothing either. Gaps: sampling-both-validate-message-content (client + server rows).
  3. Every sampling failure is -32603. The 2025-11-25 sampling error table assigns: user rejected → -1; tool result missing / tool results mixed with other content → -32602. mcpx produces neither: a decline becomes errors.New("the sampling request was " + action), an expired wait becomes the raw Await error, and Client.answer's handler wrap (internal/mcpclient/modern.go ~L296-303) flattens every handler error to -32603. An upstream cannot tell "the user said no" from "mcpx broke". Gap: sampling-client-errors-user-rejected-and-tool-shape.

Already in place (not this design's work)

The design (decisions recorded in the doc)

  • One choke point, one validator. All sampling validation lives in one helper used by the broker path (sampleViaBroker, in and out) — where legacy wire requests and modern input_required answers already converge — not copied into mcpclient/mcpserver separately.
  • Errors first (Phase 1). Decline/cancel of a sampling answer → -1 ("user rejected", action named). Expiry/no-answerer stays -32603 with an explicit expired message — nobody chose, so it is not a rejection. Tool-shape violations → -32602 (raised by Phase 2's checks through Phase 1's typed error). Client.answer's wrap preserves a typed error's code.
  • Validation judges, doesn't repair (Phase 2). Request side, on open: messages decode per era (content single before 2025-11-25, single-or-array from 2025-11-25); tool-result messages hold only tool_result blocks, each id paired with a preceding tool_use — violations get -32602 before a question opens. Result side, on answer: result must carry role and model and era-legal content, and toolUse/tool_use is valid only against a tools-carrying request — and the silent role/model fill-in is removed (it manufactures fields the answerer never supplied). A bad answer is refused at the broker step with the question left open where it arrived through the broker. Model-hint mismatch is not a failure (hints are expressly ignorable).
  • Review on the existing surface (Phase 3). No new UI: mcpx elicit renders a Sample question as sampling (server, system prompt, message list; full JSON on show; a real CreateMessageResult answer skeleton instead of '{}'). Review-before-delivery takes the honest form this architecture has — inspect/override while pending, results recorded and visible via result — stated plainly in CLI help; a genuine pre-delivery staging hold for agent answers is an open question in the doc, not a phase.

Phases (sub-issues of this issue)

  • Phase 1 — typed sampling errors (-1 rejection, -32602 tool-shape type, expiry stays -32603 labelled). Independent.
  • Phase 2 — message + result validation at the broker (request pairing/era shape; result-vs-request; fill-in removed). Independent in code; its -32602 raising uses Phase 1's typed error (triage settles the seam if Phase 1 hasn't landed — not hard-gated).
  • Phase 3 — reviewable sampling in mcpx elicit + conformance-matrix/register truth-up + nix#256 completion evidence. Gated on Phase 2.

Open questions (kept in the design doc, NOT filed)

  • Genuine pre-delivery staging of agent sampling answers (a latency-contract change; only on evidence inspect-while-pending is insufficient).
  • Era threading to the broker: if the upstream's era can't reach sampleViaBroker without new plumbing, union-shape acceptance vs plumbing for a deprecated feature is Phase 2 triage's recorded call.
  • Whether a validated-bad request should also be recorded (count/log naming the server) for chronic-malformation visibility.
  • Sampling as a client-hosted task (2025-11-25 only): untouched; would inherit the validator if it ever becomes real.

Acceptance (design level)

  • Phases 1–3 landed. A person can read a pending sampling request's actual messages in mcpx elicit and answer with a well-formed skeleton; a malformed request is refused -32602 before any question opens; a malformed or request-mismatched result never reaches the upstream and missing role/model is refused, not filled; a decline reaches the upstream as -1 and an expiry as -32603 naming expiry.
  • The seven scoped #256 conformance gaps retired or explicitly recorded partial in the matrix; mcpx: sampling relay -- human review, validation of results, error mapping (8 conformance rows) dezren39/darwin-nix#256 carries completion evidence against each acceptance bullet and is closed iff all are met.

Acceptance criteria

  • Phases 1–3 (sub-issues of this design) have landed: typed sampling errors, broker message+result validation, and reviewable sampling in mcpx elicit with matrix/register truth-up.
  • A declined sampling request reaches the upstream as -1; tool-shape violations as -32602; an expired wait as -32603 naming expiry — no sampling failure is flattened into an unlabelled internal error.
  • Sampling requests are validated before a broker question opens (era content shape; tool-result pairing per SMP-06) and results are validated against their request before relay (role/model present, era-legal content, toolUse only against a tools-carrying request); the silent role/model fill-in is gone.
  • mcpx elicit renders a Sample question's server, system prompt and messages, and offers a real CreateMessageResult answer skeleton.
  • The seven scoped mcpx: sampling relay -- human review, validation of results, error mapping (8 conformance rows) dezren39/darwin-nix#256 conformance gaps are retired or explicitly recorded partial, and mcpx: sampling relay -- human review, validation of results, error mapping (8 conformance rows) dezren39/darwin-nix#256 carries completion evidence against each of its acceptance bullets, closed if and only if all are met.

Activity

  1. added
    priority/normalOrdinary priority (the default)
    app/mcpxPackage/app: mcpx (pkgs/ or apps/ in this repo)
    epicMeta-issue grouping multiple items (org Project Type=Epic)
    effort/lLarge: multi-day or open-ended; expect staged check-ins
    kind/designA design proposal (design-architect): multi-step, phased, decision-bearing
    on Oct 28, 2026
  2. developing-today commented on Oct 28, 2026

    @developing-today
    Contributor
  3. dezren39 commented on Oct 28, 2026

    @dezren39
    MemberAuthor

    Design filed by the design architect (pass 10, 2026-10-28). Phases as sub-issues: #1835 (Phase 1, typed sampling errors), #1836 (Phase 2, broker validation), #1837 (Phase 3, reviewable sampling + truth-up, status/blocked gated on Phase 2). Triage packets enqueued to repo-health-code-pilot: task 2718 (#1835), task 2719 (#1836), task 2720 (#1837) — all verified queued with last_error=null. Design doc: files/designs/code-mcpx-sampling-trust-boundary.md in the keep-the-four-repos-healthy goal workspace; central register entry added (status: proposed). No user/review: nix#256's acceptance is explicit and conformance-only in scope.

  4. dezren39 commented on Oct 28, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · handed off

  5. added
    queue/queuedHas a queued task in the Supabase agent queue
    task/triageQueue packet kind: verify, reproduce, and route this item
    on Nov 3, 2026
  6. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    agent-note:v1
    state: triaged (coverage pass)
    Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 40 (priority/normal). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
    Found via: coverage pass 2026-11-03

  7. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 3, 2026
  8. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · picked up

    • Agent: queue worker repo-worker-code, OpenChamber scheduled lane (lane 335a5d1f-bfc1-4831-99ea-850c8939a727)
    • Found via: coverage-pass triage packet (2026-11-03)
    • Task: Triage design epic mcpx: design — sampling trust boundary (reviewable, validated, honestly error-coded) #1834 (mcpx sampling trust boundary) at current main — confirm the fix path is scoped, name blockers and ordering, close if already handled. No implementation in this task.
    • Expectation: verdict comment with file-level evidence at origin/main d3476bce7, labels synced, remaining work delegated to the phase issues.
  9. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    ✅ Triage verdict — epic is live, fully decomposed, and not superseded. Keep it open as the tracker; the only remaining scope is Phase 3 (#1837), whose gate cleared today.

    • Agent: queue worker repo-worker-code, OpenChamber scheduled lane (queue task 7749, repo-health-code-pilot)
    • Found via: coverage-pass triage packet (2026-11-03)
    • Checked at: origin/main d3476bce7 (2026-11-03), fresh gh state for every named issue/PR.

    Current state of the phase set

    Phase Issue Code on main Verdict
    1 — typed sampling errors #1835 closed, PR #2440 merged 2026-10-30 pkgs/mcpx/internal/mcpclient/client.go:1729-1770 — codeUserRejected = -1, UserRejectedError, ToolShapeError (-32602), expiry left -32603 and labelled landed
    2 — broker validation #1836 closed today (PR #2443 merged 2026-10-30) pkgs/mcpx/internal/elicit/sampling.go:158 ValidateSampleRequest, :213 ValidateSampleResult; wired at pkgs/mcpx/internal/daemon/hooks.go:329 (request, before the question opens) and :394 (result), plus broker accept path internal/elicit/elicit.go:507; the silent role/model fill-in is gone (no "unknown" assignment in hooks.go) landed
    3 — reviewable sampling + truth-up #1837 open, labelled status/blocked not landed: pkgs/mcpx/internal/cli/elicit.go:268 still renders every question through describeSchema (a sampling request has no properties, so messages render as nothing and the answer skeleton is '{}'), and pkgs/mcpx/internal/elicit/elicit.go:831-834 still routes every Sample question to the agent unconditionally remaining work

    The parent acceptance ("Phases 1–3 landed") is therefore not met, so the epic stays open — but no implementation packet belongs on this issue: its scope lives entirely in the phase children.

    Revalidation checks (all fresh)

    1. Still open / still valid: mcpx: design — sampling trust boundary (reviewable, validated, honestly error-coded) #1834 is OPEN as the design epic; no superseding design or PR. Searches for 1834 across issues and PRs return only the phase set (mcpx: Phase 1 of #1834 — typed sampling errors (-1 on rejection, -32602 tool-shape, expiry stays -32603) #1835/mcpx: Phase 2 of #1834 — sampling message + result validation at the broker (pairing, era shape, no silent fill-in) #1836/mcpx: Phase 3 of #1834 — reviewable sampling in mcpx elicit + matrix/register truth-up + nix#256 evidence #1837), the mcpx: sampling relay -- human review, validation of results, error mapping (8 conformance rows) #1955 relocation pointer, and unrelated #1834x numbers. Sibling designs Design: mcpx upstream question rate limits — one limiter at the client choke point, keyed by the configured server #2333 (question rate limits), Design: mcpx elicitation era edges — downgrade rich enums for old hosts, complete the -32042 circuit, close the stale url row #2077 (elicitation era edges) and design(mcpx): completion, finished — one rate limit at the /v1 choke point, relevance-ranked local answers #2386 (completion at /v1) are adjacent, not duplicates.
    2. No competing branch/PR: git ls-remote --heads origin has no live 1834/1835/1836/1837 branch; no open PR touches sampling review.
    3. Origin issue: dezren39/nix#256 is CLOSED (relocated here; its scope is the mcpx: design — sampling trust boundary (reviewable, validated, honestly error-coded) #1834 set, recorded by the 2026-10-28 note on mcpx: Phase 3 of #1834 — reviewable sampling in mcpx elicit + matrix/register truth-up + nix#256 evidence #1837). No action needed on the nix side.
    4. Hard stops: none — no held, user/owned, do-not-merge, do-not-auto-edit.

    Blocker and ordering (recorded)

    No follow-up implementation task is filed against #1834 itself; the work belongs to #1837 and its task 2720.

  10. added
    status/triagedTriaged and routed (see agent note)
    and removed
    queue/claimedQueue worker lifecycle state
    work/assignedQueue worker lifecycle state
    on Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

app/mcpxPackage/app: mcpx (pkgs/ or apps/ in this repo)effort/lLarge: multi-day or open-ended; expect staged check-insepicMeta-issue grouping multiple items (org Project Type=Epic)kind/designA design proposal (design-architect): multi-step, phased, decision-bearingpriority/normalOrdinary priority (the default)source/design-architectFiled by the repo design architect jobstatus/triagedTriaged and routed (see agent note)status/waiting-dependencyWaiting on a dependencytask/triageQueue packet kind: verify, reproduce, and route this item

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions