reload-credentials-redesign - #761

Summary

Redesigns the reload_credentials MCP tool around a demonstrated bug, moves MCP tool descriptions out of Go literals into pkg/mcp_server/content/tools/*.md, and aligns the server instructions and tool descriptions so agents spend zero turns on auth in the happy path and at most two calls (reload + retry) in any failure path.

The bug

Found on a fresh stdio session (0.11.669, Windows, aws provider installed, valid creds in the process env):

1. reload_credentials unscoped, before any query: no results
2. reload_credentials provider aws, before any aws query: cannot find AUTH context for provider = 'aws'
3. run_select_query against aws.s3.buckets: 75 rows
4. reload_credentials provider aws, after the query: aws / aws_signing_v4 / env:AWS_SECRET_ACCESS_KEY / ok

Root cause: the report read the auth context registry, which is populated lazily on a provider's first query. The env re-source ran before the failing lookup, so the shipped behaviour was a silently working reload with a misleading failure report.

reload_credentials design

The handler is now three ordered phases, and the report is a pure function of post-reload state.

1. Re-source the configured env file into the process env (keys absent from the file are left untouched). A configured file that has gone missing is an explicit error naming the path.
2. Invalidate auth contexts registered lazily from provider documents (all, or the scoped provider's). Contexts supplied at startup via --auth are user configuration, not a cache, and are retained.
3. Report every installed provider (the list_providers set) against the now-current env; the provider arg is a filter and an unknown name is the only scoped error.

Each row gains a changed field: a sha256 comparison of the resolved credential material (successor chain included) before and after the re-source. Values are compared, never emitted. changed: false after a failure is the agent's stop signal.

Invariants: an installed provider always produces a row; the report is identical before and after a provider's first query; a second reload against an unchanged file is idempotent (changed: false throughout); no secret values in any output or error text.

Finding that changed the design

The work order assumed "lazy registration rebuilds on next query". It does not: doc-default auth registers only when the provider object is first created (GetProvider !ok branch). Dropping a context alone leaves a cached provider with no auth and every later query fails. InvalidateAuthContexts therefore drops the cached provider object together with its doc-registered context, tracked via a new docAuthProviders set on the handler context, and never touches --auth contexts. Lock order matches GetProvider (providers mutex outer, auth mutex inner).

Query tool errors

Credential resolution failures on run_select_query, run_mutation_query and run_lifecycle_operation now read:

credential resolution failed for provider 'aws' (credentials error: credentialsenvvar references empty string) - fix the configured env file, call reload_credentials, then retry

The provider is parsed from the statement (queryProviderName, first provider-qualified table or EXEC method). execQuery now propagates statement errors: mutation, lifecycle and pull_provider previously collapsed any error into {messages: [], timestamp}, which read as success.

Tool descriptions mastered as markdown

Tool descriptions were inline Go strings across server.go and query_library.go while instructions, prompts and resources already lived under pkg/mcp_server/content. They now live at content/tools/<tool_name>.md (frontmatter name must match the file stem; body is the prose; soft line breaks collapse to spaces, blank lines keep paragraph breaks) and load through the same embedded content framework behind a private toolDescriptions carrier. addToolWithGate attaches the description and returns an error for a registered tool with no file, so registration failures surface at server construction like prompt and resource failures. Published bytes were verified identical to the removed literals for all 16 tools.

The descriptions were then reworded for turn economy: server_info and reload_credentials are on-demand, never session-start or pre-query steps; list_methods is for composing from scratch, not for executing a vetted library template; validate_select_query is a pre-flight for expensive or failing queries, not routine. The server instructions were updated to say the same thing (defaults sqlite3 and safe, credentials resolve at query time, fail -> fix file -> reload -> retry, 401 retry gated on changed: true, query library before drill-down, EXEC statement shape).

Design notes to debate once

• Status vocabulary kept as ok / unresolved / not_checked. The work order's table said not_resolved; existing docs, tests and the plugin smoke test use unresolved, and changed is the field that actually answers the rotation question.
• execQuery error propagation is a behaviour change beyond credentials: any failing mutation, lifecycle op or provider pull now returns an error instead of an empty success payload. Required for the error text, and honest.
• Canonical prompts untouched. cloud_audit, cost_cleanup, drift_report, iam_access_review, public_exposure_scan, getting_started and create_deploy_stack still call reload_credentials at session setup as a multi-cloud status probe. That is the work order's scenario 4 (idempotent status check when the user asks which providers resolve), not the preemptive anti-pattern, so left as is.
• validate_select_query requires credentials. A draft description claimed otherwise; checked against the binary, EXPLAIN resolves credentials and reports the credential error before a bad column. The description says so now.
• Data carriers follow the private interface + constructor pattern (toolDescriptions / newToolDescriptions). No new package-level globals.

Test plan

Robot (test/robot/functional/mcp.robot), all driven through a new scripted stdio harness run_stdio_credential_script in mcp_stdio_client.py against the existing stackql_auth_testing dummy-credential provider (custom auth via DD_API_KEY / DD_APPLICATION_KEY; the static_auth mock rejects any other key value):

• MCP Stdio Reload Credentials Reports Installed Providers On Fresh Session - unscoped and scoped before any query, unknown provider is the only scoped error, no secret values.
• MCP Stdio Reload Credentials Rotates Key After Auth Context Registered - stale key rejected by the mock, rotate the file, one reload reports changed: true, next query uses the new key, no-op reload shows changed: false, deleted file is an explicit error naming the path.
• MCP Stdio Reload Credentials Rotates Key Before First Query - full report pre-query, first query uses the rotated key.
• MCP Stdio Reload Credentials Recovers From Failed Resolution - select and mutation error text name the provider and the recovery, unchanged reload shows changed: false, fixed reload changed: true, retry succeeds.
• MCP Stdio Reload Credentials Sources Env File Mid Session - existing scenario, assertions tightened to the new error text and row shape.
• MCP HTTP Pull Provider Installs Known Provider - was passing vacuously (local-mode REGISTRY PULL error swallowed); now runs against a new full_access server on 9927 backed by the mocked HTTP registry with a throwaway approot and asserts successfully installed.
• MCP HTTP Current Revision Client Runs Without Handshake Or Session - the stateless roundtrip now opens with server/discover and asserts the 2026-07-28 revision and the embedded instructions are served.

Removed: MCP Stdio Reload Credentials Without Auth Contexts (asserted the empty report that was the bug) and its helper.

Go unit tests: credentialFingerprint, queryProviderName, error text; loadToolDescriptionsFrom validation and prose collapsing; two-way coverage between published tools and content/tools files; a disk-read check that the markdown prose is what tools/list publishes.

Local run: full Mcp robot suite 64/69 with the 4 known version-stamp environmental failures on an unstamped binary, plus the retargeted pull scenario passing separately. golangci-lint clean on touched packages.

Docs

docs/mcp.md and pkg/mcp_server/README.md: reload semantics, changed field, per-process reload note for replicated stateless HTTP deployments (rotation there belongs at the deployment layer), and the content/tools directory in the embedded content section.

Out of scope

• Multi-replica rotation (documented only).
• Loading creds from an arbitrary file without touching the process env (different tool).
• query_library_search reporting empty required_params for aws/s3/buckets-list (library index metadata, likely the query library repo).
• The MCP Implementation version is hardcoded v0.1.1 at server.go; wiring the real build version through is a separate one-liner.