René's URL Explorer Experiment

Title: fix: mcp server CRLF stdio tolerance (#668), json render option (#669), upstream http error propagation (#670) + robot coverage by jeffreyaven · Pull Request #680 · stackql/stackql · GitHub

Open Graph Title: fix: mcp server CRLF stdio tolerance (#668), json render option (#669), upstream http error propagation (#670) + robot coverage by jeffreyaven · Pull Request #680 · stackql/stackql

X Title: fix: mcp server CRLF stdio tolerance (#668), json render option (#669), upstream http error propagation (#670) + robot coverage by jeffreyaven · Pull Request #680 · stackql/stackql

Description: Description Closes #668, #669, #670 Fixes three defects/gaps in the MCP server surface. Stdio transport dies silently on CRLF-terminated JSON-RPC (#668). The go-sdk ndjson decoder treats the bare carriage return before the newline as invalid trailing data and kills the session; the resulting error was discarded in cmd/mcp.go, so the process exited 0 with no output. The stdio transport now feeds the SDK through a CR-stripping stdin pipe (lossless, since a raw CR is illegal inside a JSON string - control characters must be escaped), and any server run failure is now printed and exits non-zero. Clean client EOF still exits 0. JSON render option for tool result text content (#669). Query/metadata/registry tool inputs accept a per-call format argument ("markdown" default, "json"; unknown values fail fast), and --mcp.config accepts a server-level default: {"server": {"render": "json"}} JSON mode renders the same DTO carried in structuredContent as compact JSON in the text content, with database/sql nullable wrappers unwrapped, so text-only MCP consumers get a machine-readable payload. The bundled test client (stackql_mcp_client) gained a {"prefer_text": true} client-cfg option to surface the text blocks rather than the structured payload. Upstream HTTP errors returned as empty result sets (#670). SELECTs that failed upstream (eg 403 rate limit, 5xx) returned {"rows": []} with isError: false, so MCP clients could not distinguish "zero rows" from "query failed". A new strict-upstream-errors flag on HandlerContext - set only by the mcp command - converts data acquisition failures into statement errors, which the MCP backend classifies for clients: tool run_select_query: upstream http error: {"http_status": 403, "retryable": false}: upstream provider error: Response error. Status code 403. Body: {...} 408/429/5xx are classified retryable; other 4xx are not. HTTP 404 is deliberately exempt: under stackql's database semantics, querying an absent resource legitimately yields a provider 404 and maps onto an empty result set (zero rows), so MCP keeps returning {"rows": []} for absence, exactly as the CLI and pgsrv always have. The exemption is applied at the execution layer (not the MCP surface) so join/union legs that 404 keep contributing zero rows without failing the statement. Design note (surfacing for debate once, per house rules): #670 is deliberately opt-in at the handler-context level rather than a global execution-layer change. CLI and pgsrv sessions keep the historical empty-result-plus-stderr-notice semantics for all statuses, which existing robot scenarios pin (PG Session Debug Off Behaviour Canonical et al). Consequently the reverse-proxy MCP backend (which reaches the engine over the PG wire) also retains legacy behaviour. A related consequence of the #668 fix: srv-embedded MCP servers now share the loud-failure path, so an MCP transport failure inside srv exits the process rather than silently dropping the MCP endpoint. Type of change Bug fix (non-breaking change to fix a bug). Feature (non-breaking change to add functionality). Breaking change. Other (eg: documentation change). Please explain. Issues referenced. Closes #668 Closes #669 Closes #670 Evidence Nine new robot scenarios in test/robot/functional/mcp.robot, plus supporting assets (a python stdio harness test/python/stackql_test_tooling/mcp_stdio_client.py that drives the binary over raw byte pipes so the line terminator arrives verbatim, and three github mock routes serving 403/429/404 with JSON object bodies as GitHub sends): MCP Stdio Server Handles LF Terminated JSON RPC (control) and MCP Stdio Server Tolerates CRLF Terminated JSON RPC (#668) MCP HTTP Text Content Defaults To Markdown (control), MCP HTTP Per Call JSON Format Renders JSON Text, MCP HTTP Server Level JSON Render Default (new server at 9924 with render: json), MCP HTTP Invalid Format Argument Is Rejected (#669) MCP HTTP Upstream 403 Surfaces As Non Retryable Tool Error, MCP HTTP Upstream 429 Surfaces As Retryable Tool Error and MCP HTTP Upstream 404 Returns Empty Result Set (#670) Local results (WSL, sqlite backend): robot --suite Mcp test/robot/functional 46 tests, 46 passed, 0 failed go test -timeout 1200s --tags "sqlite_stackql" ./... exit 0 golangci-lint run (v2.5.0, matching the CI pin) 0 issues. Regression check: robot --suite "Stackql Sessions" was run to confirm the pgsrv 403 semantics are untouched; PG Session Debug Off Behaviour Canonical (which pins empty stderr for a 403 SELECT) passes. The mTLS scenarios in that suite fail locally for a pre-existing environmental reason only (psql rejects the client key file's world-readable permissions on a Windows-mounted filesystem); they are expected to pass in CI on a native filesystem. New unit tests: internal/stackql/mcpbackend/error_classification_test.go (status extraction and retryability classification incl. 403/408/429/503 and the non-HTTP fallback), internal/stackql/execution/upstream_not_found_test.go (404-exemption message classification incl. mixed-status and unparseable cases), plus additions to pkg/mcp_server/render/render_test.go (format legality, compact JSON, wrapper unwrapping) and pkg/mcp_server/client_format_test.go (prefer_text). Checklist: A full round of testing has been completed, and there are no test failures as a result of these changes. The changes are covered with functional and/or integration robot testing. The changes work on all supported platforms. Unit tests pass locally, as per the developer guide. Robot tests pass locally, as per the developer guide. Linter passes locally, as per the developer guide. Variations "All supported platforms" is left to CI: local verification was Linux (WSL) only. The stdio harness is deliberately binary-mode (subprocess without text=True) so the CRLF scenarios behave identically on Windows runners. "Robot tests pass locally" is asserted for the full Mcp suite (46/46) and the Stackql Sessions suite excluding the mTLS scenarios, which fail locally for the pre-existing environmental reason described above (drvfs key-file permissions), unrelated to this change set. Tech Debt One item, small and contained: the upstream HTTP status code is not available as a value at the stackql layer - the any-sdk invoker consumes the *http.Response and returns only {Body, Messages} - so both the 404 exemption and the MCP error classification recover the status by parsing the fixed "Status code NNN" fragments any-sdk embeds in its error strings (one regex each in internal/stackql/execution and internal/stackql/mcpbackend). Error bodies that any-sdk publishes verbatim (non-object JSON bodies) carry no such fragment and degrade to the unclassified generic error. The clean fix is any-sdk-first: carry the naked StatusCode through providerinvoker.Result (or a typed upstream error) and delete both regexes; proposed as a follow-up issue to be linked here prior to merge, targeted at the next any-sdk bump.

Open Graph Description: Description Closes #668, #669, #670 Fixes three defects/gaps in the MCP server surface. Stdio transport dies silently on CRLF-terminated JSON-RPC (#668). The go-sdk ndjson decoder treats the bare c...

X Description: Description Closes #668, #669, #670 Fixes three defects/gaps in the MCP server surface. Stdio transport dies silently on CRLF-terminated JSON-RPC (#668). The go-sdk ndjson decoder treats the bare c...

Opengraph URL: https://github.com/stackql/stackql/pull/680

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width