docs(mcp): spec send_scpi v0.2 safety design + structured error contract#232
Open
cptkoolbeenz wants to merge 4 commits into
Open
docs(mcp): spec send_scpi v0.2 safety design + structured error contract#232cptkoolbeenz wants to merge 4 commits into
cptkoolbeenz wants to merge 4 commits into
Conversation
Three additions to the RFC (#231): 1. Safety: send_scpi v0.2 design — two-tier net (destructive deny-list per-call allowDestructive, wiki-pattern hallucination warn with edit-distance suggestions). Mode-interaction matrix. Wiki snapshot strategy. Non-blocking — firmware response is ground truth. 2. Structured error response contract (v0.1 work) — drain SYST:ERR? before/after, optional SYST:LOG? scrape, readback failures as structured errors. Uniform {success,response,errors[],warnings[]} shape across all SCPI tools so the model never has to grep free text. 3. v0.2 fast-follow priority note — per chat: ship v0.2 quickly so the MCP becomes the primary firmware-feature-testing harness, replacing the per-feature Python harness pattern. Tool table phasing, marketing, open questions, tech stack untouched. send_scpi stays in v0.2 per original RFC.
Review Summary by QodoSpec send_scpi v0.2 safety design and structured error contract
WalkthroughsDescription• Add comprehensive safety design for send_scpi v0.2 with two-tier protection
- Destructive deny-list (hard block) requiring explicit allowDestructive flag
- Wiki-pattern hallucination detection with edit-distance suggestions
• Define structured error response contract for all SCPI tools
- Uniform {success, response, errors[], warnings[]} shape across v0.1 and v0.2
- Drain SCPI error queue before/after, optional async log scraping, readback validation
• Establish v0.2 fast-follow priority to replace per-feature Python test harness
• Document mode interactions and wiki snapshot freshness strategy
Diagramflowchart LR
A["send_scpi v0.2<br/>Safety Design"] --> B["Tier 1:<br/>Destructive Deny-List"]
A --> C["Tier 2:<br/>Wiki-Pattern Warn"]
B --> D["allowDestructive Flag<br/>Per-Call Override"]
C --> E["Hallucination Detection<br/>Edit-Distance Suggestions"]
F["Structured Error<br/>Response Contract"] --> G["SYST:ERR? Queue<br/>Before/After Drain"]
F --> H["SYST:LOG? Buffer<br/>Async Errors"]
F --> I["Readback Validation<br/>Timeout Failures"]
G --> J["Uniform Response Shape<br/>success/response/errors/warnings"]
H --> J
I --> J
K["v0.2 Fast-Follow<br/>Priority"] --> L["Replace Python<br/>Test Harness"]
File Changes1. docs/planning/mcp/RFC.md
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Second wave of additions to the RFC, focused on items that aren't visible from outside the firmware team but materially affect the MCP UX. ## Firmware-aware constraints - **Quiescence rule** — any SCPI during a benchmarked stream throttles the encoder enough to corrupt the measurement (firmware CLAUDE.md + memory feedback_no_scpi_during_benchmark). #1 footgun for an LLM driving the device. MCP enforces via DeviceRegistry.isStreaming + RequiresQuiescence attribute; structured quiescence_violation error tells the model exactly what to do. Opt-out via respectQuiescence=false, logged in audit trail. - **Variant-aware defaults + capability-filtered tool surface** — NQ1/NQ2/NQ3 differ in ADC resolution, channels, peripherals (DAC NQ3-only). connect_device caches BoardCapabilities; start_streaming etc. apply variant-appropriate defaults (precision 4/6/7). DAC tools filter out on NQ1 handles. Capabilities exposed as daqifi://devices/{id}/capabilities. - **ADC channel enable: bitmask form** — looping configure_analog_channel N times triggers N firmware freq-cap recomputes. set_active_channels(int[]) should issue the bitmask SCPI form (one recompute) instead. - **Credentials handling** — WiFi passwords never echoed in responses / logs / audit. Direct response to 2026-05-07 Tesla AP password leak. ScpiAuditLog masks pass/secret/key/token keys at log-emission time. ## Cross-cutting infrastructure (v0.1 internal) - **ScpiAuditLog** — every SCPI call + response logged (with credential masking), exposed as daqifi://session/audit. Bounded at 500 entries. Also captures quiescence_violation opt-outs for forensics. - **RateLimiter** — per-handle 30/sec cap. Prevents runaway LLM loops; rate-limit errors are a useful loop-detection signal. - **Logging conventions** — Microsoft.Extensions.Logging to stderr; stdout reserved for JSON-RPC. ## Cross-cutting concern for daqifi-core Two API additions that would clean up the MCP layer and benefit daqifi-desktop too. Neither blocks v0.1; both batched into a follow-up daqifi-core PR if Tyler agrees: 1. **Task<DeviceMetadata> GetMetadataAsync()** on DaqifiDevice — replaces the TaskCompletionSource-vs-ChannelsPopulated dance for getting typed serial/IP/firmware after connect. 2. **public Task<ConfirmedScpiResult> ExecuteConfirmedAsync(command, drainErrorQueue=true, scrapeLogAfter=false, ct)** — wraps the currently-protected ExecuteTextCommandAsync + DrainErrorQueueAsync pattern that the structured error contract relies on. daqifi-desktop's SCPI console feature would benefit too. Builds on prior commit in this PR; tool table phasing still untouched.
Addresses the three Qodo flags on PR #232: - Tighten the UNDOCUMENTED_SCPI warning (~70% fewer tokens) while keeping the "you may have hallucinated" prompt-engineering hook; also fixes the sendRawScpi → send_scpi tool-name typo in the warning example. - Reframe wiki-snapshot CI freshness: it lands in v0.1 under #mcp-13 alongside the snapshot bundle itself, not as a v0.2 aspiration. - Unify destructive confirmation field: rename allowDestructive → confirmed across the send_scpi section so it matches the existing safety-model contract on delete_sd_file / format_sd_card / etc. Also syncs GITHUB_ISSUES.md with the new scope this PR added to the RFC: - #mcp-2: BoardCapabilities caching at connect time (per-variant smarts) - #mcp-5: RequiresQuiescence attribute + dispatcher gate (enforces the firmware quiescence rule mechanically) - #mcp-13 (new): structured SCPI error contract (ConfirmedScpiResult), bundled wiki snapshot, CI freshness check - #mcp-14 (new): cross-cutting infra (ScpiAuditLog, RateLimiter, stderr-only logging) - Updated epic table, success criteria, dependency graph - Total effort: 66h → 90h (~6-8 weeks part-time) - Daqifi.Core API additions (GetMetadataAsync, ExecuteConfirmedAsync) called out as a separate follow-up cleanup, not part of this epic Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Pushed
Strongest adds from your side that I left untouched
Still outstanding from the original RFCThe two blocking open questions are still unanswered:
Once you weigh in on those, I can start creating actual GitHub issues from |
Records Chris's 2026-05-27 decisions on Tyler's 8 open questions, restructured from a flat numbered list into per-question Answer blocks. Tyler can override any in PR review. ## Substantive content changes - **Recipe clarification** — added an explainer of what an 'MCP prompt' is mechanically (parameterized template surfaced as a slash-command / suggestion menu by Claude Desktop / Cursor / Cline) so future readers don't have to derive it. Cross-link to Q7 for the hosted-library deferred decision. - **Q4 telemetry — design specced** — new §Telemetry (opt-in) section under Cross-cutting infrastructure. Strict opt-in via --telemetry flag or DAQIFI_MCP_TELEMETRY=1. Per-event shape (tool name, mode, outcome, error_code, latency, server_version, client_id, os, device_variant, session_id, event_ts) and explicit no-list (tool args, response text, serials, MACs, IPs, hostnames, SSIDs, credentials, file contents/names, no per-user identifier). --telemetry-dry-run for procurement review. 90d raw / indefinite aggregates retention. - **Q5 stability contract — tightened**: tool names stable from v0.1; optional fields freely additive; required fields only via vN-optional → v(N+1)-required deprecation; tool removal needs ≥6-month deprecation window + major bump; experimental: true schemas carry no promise; resource URIs follow same rules as tool names. - **Q6 LabVIEW interop — inverted framing**: instead of MCP-calls-out-to-LabVIEW (permanent per-VI support burden), document the cleaner inverse: LabVIEW-as-client of the MCP via a small example VI library. Added a v0.3+ phasing line referencing it. ## Open-questions section restructured Each Q now has the original text + Answered (Chris, 2026-05-27): block with the resolution. Q1, Q2, Q3, Q7, Q8 mostly confirm Tyler's framing. Q4 reverses my earlier no-telemetry suggestion (Chris approved opt-in). Q5 + Q6 + recipe clarification land richer than the original questions. Tool table phasing, marketing, tech stack, distribution, appendix still untouched.
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.
Builds on #231. See the commit / diff for details. Adds: (1) §Safety: send_scpi v0.2 design — destructive deny-list (per-call allowDestructive) + wiki-pattern hallucination-warn (LLM-self-check phrasing). (2) §Structured error response contract for all SCPI tools in v0.1 — drain SYST:ERR? before/after + optional SYST:LOG? scrape + readback failures, uniform {success,response,errors,warnings} shape. (3) v0.2 fast-follow priority note per chat ('ship v.2 quickly so the mcp tests new features'). Tool table phasing untouched; send_scpi stays in v0.2. Targets your PR branch (claude/youthful-mclaren-c56954) so additions land inside #231 rather than racing it.