mcp: server modes, elicitation gating, audit subsystem (PR2) (#652)

* mcp: server modes, elicitation gating, audit subsystem (PR2)

* robot: use canonical mocked SQL in new mode-contract scenarios

The new mode-contract scenarios (MCP HTTP Mode Read Only / Delete Safe /
Full Access / Audit Basic / Audit Disabled) all failed in CI with rc=2.

Root cause: I synthesised SQL like

  select name from google.compute.networks where project = 'stackql-demo'
  delete from google.compute.firewalls where project = 'p' and firewall = 'f'

against the test registry.  google.compute.networks is registered as
metadata (used by describe_resource / list_methods in earlier scenarios)
but is not mocked as an executable SELECT target.  The synthetic DELETE
values 'p' / 'f' don't match the canonical mock either.  The bundled
mcp_client panics on unknown-target query errors with exit code 2.

Switch all five scenarios to the canonical mocked SQL already used by
the working MCP HTTPS Server Query / Exec Query Canonical scenarios:

  select name, id from google.storage.buckets where project = 'stackql-demo'
  delete from google.compute.firewalls where project = 'mutable-project' and firewall = 'deletable-firewall'

The MCP HTTP Mode Safe Refuses Mutations Without Elicitation scenario
already happened to pass (it expected rc != 0, which the synthetic SQL
also produced) but the same SQL substitution makes its intent clearer.

Also:
- Clean up six empty stackql_mcp_server_*.log files inadvertently
  committed in the PR2 squash.  These were left by unit tests that
  exercised audit-on paths before connectInProcess gained its disable-
  audit default.
- Add stackql_mcp_server_*.log to .gitignore so future stray audit
  files don't get tracked.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* robot: fix Windows audit-path failure; preflight 9923 reachability

MCP HTTP Audit Basic Records Tool Calls failed on Windows CI with
rc=1 on the first SELECT against the audit-enabled 9923 server.

Root cause: the audit log path embedded in mcp.config was
`${CURDIR}/tmp/mcp-audit-9923.log`.  On Windows, ${CURDIR} expands to
`D:\a\stackql\stackql\test\robot\functional`.  Inside a JSON string,
the resulting `\a` `\s` `\t` etc are invalid backslash escapes, so
json.Unmarshal in cmd/mcp.go's runMCPServer fails silently and
config stays as zero-value.  The server then binds the default
address 127.0.0.1:9876 instead of 9923, the client sees connection
refused, and rc=1.

Fix the embedded path to a relative filename `mcp-audit-9923.log`.
The stackql process resolves it through filepath.Abs against its cwd
(the directory robot was invoked from), so the file lands in the
repo root regardless of OS path-separator semantics.  Update the
test's Get File call to use ${EXECDIR} to match.

Also:
- Add a preflight server_info call at the start of the audit test
  so a future audit-server startup failure surfaces with a clear
  message ("9923 server must be reachable...") instead of a generic
  rc-comparison failure.
- Add mcp-audit-*.log to .gitignore so the audit file from a robot
  run doesn't get accidentally committed.

The audit-disabled scenario is unaffected (its glob
stackql_mcp_server_*.log does not match mcp-audit-9923.log).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review updates

* updates

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jeffreyaven

.gitignore

@@ -35,3 +35,10 @@ y.output
 .stackql/
 
 .venv/
+
+## MCP audit logs generated by the file sink (default name + robot-test fixed name)
+stackql_mcp_server_*.log
+mcp-audit-*.log
+
+## Generic sink default name (used by pkg/sink tests / non-MCP callers)
+sink_*.log

docs/mcp.md

@@ -18,11 +18,43 @@ We have a nice debug config for running an MCP server with `vscode`, please see
 
 ```
 
-To run a read-only server (mutation and lifecycle tools refuse to execute):
+The default mode is `safe`: SELECTs proceed, mutations and lifecycle operations require user approval via the [MCP elicitation flow](#server-modes).  An audit log is written to `./stackql_mcp_server_<RFC3339-utc-second>.log` by default (see [Audit Log](#audit-log)).
+
+### Mode examples
+
+Read-only (mutations and lifecycle refused immediately):
+
+```bash
+
+./build/stackql mcp --mcp.server.type=http --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "mode": "read_only"} }'
+
+```
+
+Delete-safe (INSERTs/UPDATEs proceed; DELETEs and EXECs need approval):
+
+```bash
+
+./build/stackql mcp --mcp.server.type=http --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "mode": "delete_safe"} }'
+
+```
+
+Full access (everything proceeds without approval - use only when the client and operator are trusted):
 
 ```bash
 
-./build/stackql mcp --mcp.server.type=http --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "read_only": true} }'
+./build/stackql mcp --mcp.server.type=http --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "mode": "full_access"} }'
+
+```
+
+The legacy `"read_only": true` JSON key is still accepted and is treated as equivalent to `"mode": "read_only"`.  When both are set, `mode` wins.
+
+### Disabling audit
+
+Audit is on by default.  To opt out:
+
+```bash
+
+./build/stackql mcp --mcp.server.type=http --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "audit": {"disabled": true}} }'
 
 ```
 
@@ -74,11 +106,14 @@ Then, assuming you have a `stackql` MCP server serving streamable HTTP on port `
 ## Run a SELECT.
 ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:9992 --exec.action run_select_query --exec.args '{"sql": "select name from google.compute.networks where project = '"'"'stackql-demo'"'"';"}'
 
-## Mutation (INSERT/UPDATE/REPLACE/DELETE).  Refused in read-only mode.
-## Tread carefully -- real side effects.
+## Mutation (INSERT/UPDATE/REPLACE/DELETE).  Tread carefully -- real side effects.
+## Gated by the server mode.  Under the default `safe` mode this call returns
+## an error because the bundled client does not advertise elicitation (it
+## cannot respond to the server's approval prompt).  Start the server with
+## `mode: full_access` to run it without an approval prompt.
 # ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:9992 --exec.action run_mutation_query --exec.args '{"sql": "delete from google.compute.networks where project = '"'"'stackql-demo'"'"' and network = '"'"'returning-test-03'"'"' ;"}'
 
-## Lifecycle EXEC operation.  Refused in read-only mode.
+## Lifecycle EXEC operation.  Same gating rules as run_mutation_query.
 # ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:9992 --exec.action run_lifecycle_operation --exec.args '{"sql": "EXEC google.compute.instances.start @project = '"'"'mutable-project'"'"', @zone = '"'"'us-central1-a'"'"', @instance = '"'"'demo'"'"';"}'
 
 ```
@@ -90,7 +125,7 @@ The server publishes 11 tools.  Each returns both rendered text (for the LLM) an
 
 | Tool | Renderer | Description |
 |---|---|---|
-| `server_info` | KV | Server identity and runtime: stackql version, backing SQL engine, provider registry location, read-only flag.  Call once at session start. |
+| `server_info` | KV | Server identity and runtime: stackql version, backing SQL engine, provider registry location, mode, read-only flag.  Call once at session start. |
 | `list_providers` | Table | Available cloud/SaaS providers (top of the hierarchy).  No inputs. |
 | `list_services` | Table | Services under a provider.  Requires `provider`. |
 | `list_resources` | Table | Resources under a `provider`.`service`.  Requires `provider` and `service`. |
@@ -99,8 +134,8 @@ The server publishes 11 tools.  Each returns both rendered text (for the LLM) an
 | `describe_method` | KV | Full I/O contract for one method (always EXTENDED).  Requires `provider`, `service`, `resource`, `method`. |
 | `validate_select_query` | KV | Parse and plan a SELECT without executing.  Returns `{valid, errors}`.  SELECT only. |
 | `run_select_query` | Table | Execute a SELECT.  Returns `{rows}`.  Reads only. |
-| `run_mutation_query` | KV | Execute INSERT/UPDATE/REPLACE/DELETE.  **Real side effects.** Returns `{messages, timestamp}`.  Refused in read-only mode. |
-| `run_lifecycle_operation` | KV | Execute a stackql `EXEC` lifecycle operation.  Returns `{messages, timestamp}`.  Refused in read-only mode. |
+| `run_mutation_query` | KV | Execute INSERT/UPDATE/REPLACE/DELETE.  **Real side effects.** Returns `{messages, timestamp}`.  Gated by the server [mode](#server-modes). |
+| `run_lifecycle_operation` | KV | Execute a stackql `EXEC` lifecycle operation.  Returns `{messages, timestamp}`.  Gated by the server [mode](#server-modes). |
 
 ## Canonical agent prompts
 
@@ -111,6 +146,89 @@ One static prompt is published:
 `EnabledTools` and `EnabledPrompts` on `Config` are independent allowlists.  When omitted or empty everything is published; when populated they restrict the published surface to the named items.  See [the `pkg/mcp_server` README](/pkg/mcp_server/README.md) for details.
 
 
+## Server modes
+
+`Config.Server.Mode` picks one of four safety contracts.  All four allow SELECTs and metadata reads; they differ in how they handle mutations and lifecycle operations.
+
+| Mode | SELECT / metadata | INSERT / UPDATE / REPLACE / MERGE / UPSERT | DELETE | EXEC (lifecycle) |
+|---|---|---|---|---|
+| `read_only` | allow | refuse | refuse | refuse |
+| `safe` (default) | allow | needs approval | needs approval | needs approval |
+| `delete_safe` | allow | allow | needs approval | needs approval |
+| `full_access` | allow | allow | allow | allow |
+
+**Refuse** returns an error immediately.
+
+**Needs approval** uses the MCP elicitation flow:
+
+- If the client advertised elicitation at initialise, the server sends an `elicitation/create` request describing the action (tool name, query class, SQL).  Branch on the user response: `accept` -> proceed; `decline` or `cancel` -> return an error.
+- If the client did **not** advertise elicitation, the tool is refused with a message that points the operator at `full_access` mode.
+
+The bundled `stackql_mcp_client` does NOT advertise elicitation, so against a `safe` or `delete_safe` server every mutation/lifecycle call is refused with the no-elicitation message.  This is by design - the bundled client exists for scripting and regression tests, not interactive use.  Elicitation-capable MCP clients (eg Claude Desktop, Cursor) prompt the user normally.
+
+### Breaking change vs PR1
+
+PR1 had a single `read_only: true/false` flag with a default of "no enforcement; mutations proceed."  PR2 replaces that flag with `mode: safe` as the default, which means **mutations now require user approval out of the box.**  Operators running an elicitation-capable client see one approval prompt per mutation.  Operators running automation or the bundled client must explicitly opt into `full_access`.
+
+The legacy `read_only: true` JSON / YAML key still parses for back-compat and is treated as equivalent to `mode: read_only`.  `mode` wins when both are set.
+
+
+## Audit log
+
+Every tool call writes one JSONL record.  The audit answers "what did the agent do," not "what did the agent see" - result rows from SELECTs are intentionally not recorded.
+
+Recorded per event:
+
+| Field | Notes |
+|---|---|
+| `timestamp` | RFC3339 start-of-call wall clock |
+| `tool` | Tool name (eg `run_select_query`) |
+| `mode` | Server mode in effect at call time |
+| `decision` | `allow` / `refuse_immediate` / `needs_approval_accepted` / `needs_approval_declined` / `needs_approval_cancelled` / `needs_approval_unavailable` |
+| `query_class` | `select` / `mutation_create` / `mutation_delete` / `lifecycle` / `unknown` |
+| `sql` | For query tools (`run_select_query`, `run_mutation_query`, `run_lifecycle_operation`, `validate_select_query`) |
+| `args` | Hierarchy fields for metadata tools (`list_*`, `describe_*`); SQL + row_limit for query tools |
+| `duration_ms` | Wall-clock duration of the gate + handler |
+| `error` | Error message if the tool errored or was refused |
+
+### File sink
+
+The only sink shipped in this release is `file`.  One JSON object per line, fsynced after each record.  Lumberjack-style rotation.
+
+```bash
+./build/stackql mcp \
+  --mcp.server.type=http \
+  --mcp.config '{"server": {"transport": "http", "address": "127.0.0.1:9992", "audit": {"file": {"path": "/var/log/stackql-mcp.log", "max_size_mb": 100, "max_backups": 5, "max_age_days": 30}}} }'
+```
+
+Two ways to specify the location:
+
+- `audit.file.path` -- a complete file path (absolute, or relative to cwd).
+- `audit.file.dir` -- a directory; the sink chooses the basename (`stackql_mcp_server_<RFC3339-utc-second>.log`) inside it.
+
+When neither is set in `mcp.config`, the MCP server defaults `dir` to cwd so existing operators see PR2 behaviour.  The underlying [generic `pkg/sink`](/pkg/sink) package itself refuses to silently pick a directory -- the "where do logs land" decision is always made by the caller (here, the MCP server's `initAuditSink`).
+
+The resolved absolute path is logged to stderr at startup as `sink file: /path/to/file.log`.
+
+### Failure modes
+
+When the sink returns an error, the response behaviour depends on `failure_mode`:
+
+| failure_mode | Effect |
+|---|---|
+| `strict` (default) | The tool call returns the audit error to the client, even if the underlying tool succeeded.  Intentional: better an ambiguous client than an undetected DELETE. |
+| `strict_mutations` | SELECT / metadata reads proceed; mutations and lifecycle ops surface the audit error. |
+| `best_effort` | Always log to stderr and proceed. |
+
+### Sequencing
+
+The audit write happens AFTER the tool executes (or is skipped because it was gated out) but BEFORE the response returns to the client.  In strict mode an audit-write failure on a successful DELETE means the row is gone but the client sees an error - by design.
+
+### Breaking change vs PR1
+
+PR1 had no audit subsystem; PR2 enables audit by default.  To preserve PR1 behaviour, pass `"audit": {"disabled": true}` in `mcp.config`.
+
+
 ## Example responses
 
 Responses below show the typed structured payload (what `--exec` prints as JSON).  The MCP `Content` block also carries a rendered text view (markdown table or KV) that the LLM consumes; it is not shown here.
@@ -126,6 +244,7 @@ $ ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:999
   "transport": "http",
   "sql_backend": "sqlite3",
   "provider_registry": "https://registry.stackql.app",
+  "mode": "safe",
   "is_read_only": false
 }
 
@@ -209,8 +328,17 @@ $ ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:999
 }
 
 
-## Mutations and lifecycle operations have real side effects.  Refused on
-## a read-only server.  Response shape is {messages, timestamp}.
+## Mutations and lifecycle operations have real side effects.  Their behaviour
+## depends on the server [mode](#server-modes):
+##   - read_only         -> refused immediately
+##   - safe              -> elicits user approval; refused if the client does
+##                          not advertise elicitation (the bundled client does
+##                          NOT, so this returns an error)
+##   - delete_safe       -> mutation_create proceeds; DELETE / EXEC elicit
+##   - full_access       -> proceeds without prompting
+##
+## Example below assumes a full_access server.  Response shape is
+## {messages, timestamp}.
 $ ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:9992 --exec.action run_mutation_query --exec.args '{"sql": "delete from google.compute.networks where project = '"'"'stackql-demo'"'"' and network = '"'"'returning-test-01'"'"' ;"}' 2>/dev/null | jq
 {
   "messages": [
@@ -219,4 +347,8 @@ $ ./build/stackql_mcp_client exec --client-type=http  --url=http://127.0.0.1:999
   "timestamp": "2026-05-16T10:47:33+10:00 AEST"
 }
 
+
+## Example audit log line (`mcp-audit.log`) for the call above.
+{"timestamp":"2026-05-16T00:47:33Z","tool":"run_mutation_query","mode":"full_access","decision":"allow","query_class":"mutation_delete","sql":"delete from google.compute.networks where project = 'stackql-demo' and network = 'returning-test-01' ;","args":{"sql":"delete from google.compute.networks where project = 'stackql-demo' and network = 'returning-test-01' ;","row_limit":0},"duration_ms":42}
+
 ```

go.mod

@@ -136,6 +136,7 @@ require (
 	google.golang.org/grpc v1.67.1 // indirect
 	google.golang.org/protobuf v1.35.1 // indirect
 	gopkg.in/ini.v1 v1.66.2 // indirect
+	gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 

go.sum

@@ -967,6 +967,8 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s=
 gopkg.in/ini.v1 v1.66.2 h1:XfR1dOYubytKy4Shzc2LHrrGhU0lDCfDGG1yLPmpgsI=
 gopkg.in/ini.v1 v1.66.2/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
+gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc=
+gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

internal/stackql/cmd/mcp.go

@@ -110,18 +110,16 @@ func runMCPServer(handlerCtx handler.HandlerContext) {
 	if config.Server.Transport == "" {
 		config.Server.Transport = mcpServerType
 	}
-	var isReadOnly bool
-	if config.Server.IsReadOnly != nil {
-		isReadOnly = *config.Server.IsReadOnly
-	}
 	bi := buildinfo.Get()
 	transport := config.Server.Transport
 	runtimeContext := handlerCtx.GetRuntimeContext()
+	mode := config.GetMode()
 	serverInfo := mcpbackend.NewServerBuildInfo(
 		bi,
 		transport,
 		sqlBackendIdentifier(runtimeContext),
 		providerRegistryIdentifier(runtimeContext),
+		mode,
 	)
 	var backend mcp_server.Backend
 	var backendErr error
@@ -135,7 +133,6 @@ func runMCPServer(handlerCtx handler.HandlerContext) {
 		db, err := db_util.GetDB("pgx", "postgres", cfg)
 		iqlerror.PrintErrorAndExitOneIfError(err)
 		backend, backendErr = mcpbackend.NewStackqlMCPReverseProxyService(
-			isReadOnly,
 			dsn,
 			db,
 			handlerCtx,
@@ -148,7 +145,6 @@ func runMCPServer(handlerCtx handler.HandlerContext) {
 		iqlerror.PrintErrorAndExitOneIfError(orchestratorErr)
 		iqlerror.PrintErrorAndExitOneIfNil(orchestrator, "orchestrator is unexpectedly nil")
 		backend, backendErr = mcpbackend.NewStackqlMCPBackendService(
-			isReadOnly,
 			orchestrator,
 			handlerCtx,
 			logging.GetLogger(),

internal/stackql/mcpbackend/mcp_reverse_proxy_backend_service.go

@@ -12,7 +12,6 @@ import (
 )
 
 type stackqlMCPReverseProxyService struct {
-	isReadOnly   bool
 	dsn          string
 	handlerCtx   handler.HandlerContext
 	logger       *logrus.Logger
@@ -22,7 +21,6 @@ type stackqlMCPReverseProxyService struct {
 }
 
 func NewStackqlMCPReverseProxyService(
-	isReadOnly bool,
 	dsn string,
 	db *sql.DB,
 	handlerCtx handler.HandlerContext,
@@ -38,7 +36,6 @@ func NewStackqlMCPReverseProxyService(
 	}
 	return &stackqlMCPReverseProxyService{
 		dsn:          dsn,
-		isReadOnly:   isReadOnly,
 		interrogator: NewSimpleStackqlInterrogator(),
 		logger:       logger,
 		handlerCtx:   handlerCtx,
@@ -56,6 +53,7 @@ func (b *stackqlMCPReverseProxyService) Close() error {
 }
 
 func (b *stackqlMCPReverseProxyService) ServerInfo(_ context.Context, _ any) (dto.ServerInfoOutput, error) {
+	mode := b.serverInfo.mode()
 	return dto.ServerInfoOutput{
 		Version:          b.serverInfo.version(),
 		Commit:           b.serverInfo.commit(),
@@ -64,7 +62,8 @@ func (b *stackqlMCPReverseProxyService) ServerInfo(_ context.Context, _ any) (dt
 		Transport:        b.serverInfo.transport(),
 		SQLBackend:       b.serverInfo.sqlBackend(),
 		ProviderRegistry: b.serverInfo.providerRegistry(),
-		ReadOnly:         b.isReadOnly,
+		Mode:             mode,
+		ReadOnly:         mode == modeReadOnly,
 	}, nil
 }