[hkaltai]

Issue for this PR

No linked issue (new feature).

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

Lets external coding agents such as Claude Code call altimate-code's data tools directly, over MCP or from the shell, instead of delegating each task to a second altimate-code agent with its own model. It builds on #1543 (mcp serve) and #1576 (workspace tools), which are included here.

• One adapter, two entry points. altimate-code mcp serve and a new model-free altimate-code tool CLI (list, schema, run, context, daemon) execute tools through the same adapter: one allow-list, one permission layer, the same compact schemas, result spill files and answer_commit. Organisations that disallow MCP servers keep the same tools through tool run.
• Default MCP tools. The five feat(mcp): altimate-code mcp serve exposes data tools to MCP clients without an LLM #1543 tools plus impact_analysis, dbt_lineage, altimate_core_column_lineage, schema_detect_pii, data_diff, altimate_core_equivalence and sql_translate; --all-tools exposes the full allow-list.
• Warm per-project process. tool calls go through a per-project daemon (auto-start, exits after 10 idle minutes, socket scoped to version and connections) that keeps connections and the schema cache warm and releases DuckDB/SQLite file locks between calls.
• Workspace tools. workspace_context, workspace_skill and workspace_execute are on by default for tool commands in linked projects (--no-workspace-tools turns them off); mcp serve exposes them with --workspace-tools. tool context prints the analyst method, connections, the linked workspace and its skills.
• Workspace skills for Claude Code. Every workspace skill sync also writes sanitised copies (frontmatter reduced to name and description, no command blocks, no hook or plugin component files) into <project>/.claude/skills/, so linking or refreshing a project makes them available when a Claude Code session starts. User-written skills of the same name are never touched, unlink removes the copies, projects without .claude/ are left alone, ALTIMATE_CLAUDE_SKILL_SYNC=0 disables it, and a mirror failure never changes the sync result.
• Safety. MCP requests are read-only unless --allow-write; DROP DATABASE, DROP SCHEMA and TRUNCATE stay blocked even then.
• link --claude-md (opt-in) writes CLI routing guidance into CLAUDE.md for clients that cannot use MCP.

Behaviour changes in shared tools:

• schema_inspect without a table lists schemas, or up to 500 tables with catalog row estimates (no scans); MongoDB catalog discovery is bounded.
• answer_commit accepts asked and readings; its rejection checks moved to their own module with per-turn limits.
• The analyst requests high reasoning effort on adaptive-thinking models unless the user or agent config sets one.

The CI change from #1543 is intentionally not included. Companion Claude Code plugin: AltimateAI/altimate-claude-plugin#11.

How did you verify your code works?

• bun run typecheck clean after merging current main.
• bun test test/altimate/mcp-server test/altimate/tool-cli test/altimate/workspace test/altimate/analyst-effort.test.ts test/agent/analyst-data-explorer.test.ts: 1994 tests, 0 failures (includes the new .claude/skills mirror tests).
• End to end, earlier: workspace refresh against a linked workspace writing the Claude Code skill copies.
• Manual end to end at ee6add5: the CLI built from source, a local DuckDB warehouse seeded through tool run --allow-write, no test runner.
  • Discovery: tool list (70 tools, and --json), tool context, tool schema.
  • sql_execute reads:
    • a plain read;
    • tables value profiles;
    • chained queries using {{label.column}};
    • an omitted warehouse uses the default connection.
  • sql_execute refusals:
    • writes are refused without --allow-write (also a DELETE after a comment) and run with it;
    • DROP SCHEMA, DROP DATABASE and SELECT 1; TRUNCATE are blocked even with --allow-write;
    • a multi-statement read and an unknown warehouse get clear errors.
  • Other tools:
    • schema_inspect: schemas, then one table with value samples;
    • data_diff: found the 1 extra and 2 changed rows of 100;
    • sql_materialize into scratch, then a query in scratch;
    • sql_explain plan, sql_translate, lineage_check, schema_detect_pii.
  • Rejected calls: an unknown tool, an excluded coding tool (bash), invalid arguments, and warehouse_add without --allow-write.
  • Daemon:
    • starts on the first call;
    • warm calls take about 1 s, against about 3 s with --no-daemon;
    • reloads after the connections file is edited;
    • tool daemon status and stop work.
  • mcp serve over stdio:
    • initialize returns the instructions, and tools/list the 12 default tools;
    • reads run, a DELETE is refused ("Restart with --allow-write"), and DROP SCHEMA is blocked;
    • with --allow-write, the DELETE runs and DROP SCHEMA stays blocked.
  • With the Claude Code plugin (chore(deps-dev): Bump @parcel/watcher-darwin-arm64 from 2.5.1 to 2.5.6 #11): live sessions in MCP mode and in CLI mode answered warehouse questions correctly and reported the refused DELETE.
• Benchmarks with Claude Code (Sonnet 4.5):
  • 100 dbt and warehouse tasks: routing to altimate-code rose from 55/75 to 74/75 over MCP and 75/75 through tool run, with no false positives on the 25 tasks that should not use it and unchanged accuracy (94–95/100).
  • Questions that need workspace skills: setups using this PR had the skills available at the start of every session (100/100 vs 0/100 without), and accuracy matched the existing MCP flow (58% vs 55% on 100 questions) at 12% lower cost per run.

Cost side effects

No new model calls: the tool CLI and MCP server run tools without an LLM. The analyst agent now requests high reasoning effort on adaptive-thinking models when neither the agent nor the model config sets one, so analyst runs use more reasoning tokens; ALTIMATE_ANALYST_EFFORT (e.g. medium or off), the agent's options or the model's options override it. Warehouse queries run only when an agent calls a tool, with results capped by the existing output limits and spill files. schema_inspect listings are capped at 500 tables and use catalog estimates rather than scans. The per-project daemon exits after 10 idle minutes.

Screenshots / recordings

Not a UI change.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR
• I checked for cost side effects (model calls, queries or cloud resources that grow with user data) and described them, or wrote "none"

Review round (853bfa0, 7109869)

The 88 open review threads (cubic, CodeRabbit, Kilo) each have a reply: 60 fixed in 853bfa0, 22 kept with a reason, 4 out of scope, 2 false positives. bun test test/altimate/: 10,271 pass, 0 fail, 757 skip; ALTIMATE_MCP_E2E=1 bun test test/altimate/mcp-server/: 113 pass; bun test test/altimate/tool-cli: 75 pass; typecheck clean.

Review round (human consensus review of 7109869)

All 27 threads (9 review, 18 bot) and the companion comment have replies. Fixed in 4652bed:

• MAJOR: data_diff validates source, target and where_clause as single read-only queries, checks every statement it runs, and opens every connector read-only.
• isBlockedStatement classifies the whole pattern.
• Redshift backslash escaping.
• Daemon idle/release handling (only a request that ran a tool releases; a stop is answered without waiting) and an owner-only socket.
• Workspace refresh and snapshot-read fixes; analyst-effort guards; managed output-preference paths.
• .claude/skills copies are opt-in with ALTIMATE_CLAUDE_SKILL_SYNC=1 (the Claude Code plugin sets it).
• Docs, and tests for each.

Rebutted: the connector release in serverInstructions runs only at server construction.

Verified after merging main: typecheck clean; bun test test/altimate/tool-cli test/altimate/mcp-server test/altimate/workspace test/altimate/tools and the data-diff suites pass (0 fail), as does the DuckDB data-diff e2e (ALTIMATE_RUN_WAREHOUSE_E2E=1).

Known limit, unchanged here: outside Postgres, SQLite and DuckDB, read-only is enforced by SQL classification. A read that calls a stored function which writes is accepted by sql_execute, sql_check and data_diff alike. Sequence advances (nextval(), NEXT VALUE FOR, .NEXTVAL) are writes. Use a read-only database user, or one without DROP/TRUNCATE, for the strongest boundary.

Review round (human consensus review of c9a9cf9)

All three threads have replies. Fixed in dac4caa (merged with main in fa31614):

• MAJOR: E'...' is an escape string only in PostgreSQL, Redshift and DuckDB. For T-SQL (sqlserver, mssql, tsql, fabric) and any dialect the lexer does not model, a read may hold no statement keyword in its code, because statements there need no ;.

• Omitted warehouse: the SQL is classified under each dialect the call can reach:

  • the registry default;
  • dbt, only when it can take the call (a dbt project, not forced read-only), adding its manifest type and every target type of the project's profile;
  • every supported dialect plus generic lexing, only when dbt can take the call and its types cannot be read.

  The dbt project is the one dbt's routing uses: the cached adapter's project once it is created, before that the saved config's root, else discovery.

• Lexing per engine:

  • BigQuery: triple-quoted, raw and backslash-escaped strings.
  • Snowflake: // comments, and only $$ quotes.
  • Spark: a -- comment continuing past a trailing backslash.
  • MySQL: -- is a comment only before ASCII whitespace.
  • Non-ASCII characters in dollar tags and identifiers.
  • A /* inside a comment, where an engine's nesting is not known.
  • A quote after an odd run of backslashes, where it depends on server settings.

• Hard block:

  • Uncertain SQL is checked word by word. Fully read SQL is checked on its masked statement words.
  • Literals in dynamic SQL (EXEC(...), EXECUTE IMMEDIATE, PREPARE ... FROM, DO $$...$$) count too.
  • data_diff and the write ask check the whole SQL, not the preview.

Tests cover each case, plus runner tests with a fake connector for SQL Server and Fabric, named and default, with and without --allow-write, and for omitted-warehouse reads on Postgres, BigQuery, SQL Server and Trino defaults. Classification stays linear on large inputs.

Further hardening from later review rounds is in the stacked PR #1644, kept out of this approved PR. It covers sql_explain input checks, more write-as-read cases, a best-effort note in the docs, and the false refusals listed below. Most of those gaps also exist on main.

Scope of the DROP DATABASE / DROP SCHEMA / TRUNCATE block. The block reads the SQL text, so it is best effort. A statement the warehouse builds or decodes at run time can get past it, for example:

• dynamic SQL assembled by concatenation or functions;
• a stored routine created earlier;
• a literal encoding the check does not model.

The same bypasses exist on main. #1644 documents the block as best effort in docs/docs/usage/tool.md; making it a guarantee is left to a follow-up PR (below). Reads, and writes needing --allow-write, are enforced as before.

Known limits at this PR's head (#1644 fixes the first three and the last). All but the last refuse more than needed; the last can run a second statement:

• A column or alias named truncate (SELECT truncate FROM t) is refused even with --allow-write.
• SELECT 1 AS throw on SQL Server, and a column named nextval, are refused as writes.
• An empty target_warehouse in data_diff is checked against the source warehouse but run on the registry default.
• PostgreSQL literals ending in a backslash (LIKE ... ESCAPE '\') are refused as uncertain, since standard_conforming_strings may be off.
• Oracle q'...' quotes and ClickHouse heredocs are refused.
• BigQuery escaped-backtick identifiers can be refused by the parser.
• Dynamic SQL whose literals mention DROP DATABASE/SCHEMA or TRUNCATE is refused even with --allow-write.
• Privilege and trigger names (GRANT DROP DATABASE, GRANT TRUNCATE, BEFORE TRUNCATE) are refused.
• In SQL that cannot be read with certainty, TRUNCATE /* c */ ( is refused.
• The write ask names the registry warehouse even when dbt may run the write.
• sql_explain puts EXPLAIN in front of the text as given. On drivers that run every statement in it (Redshift, DuckDB), a second statement such as SELECT 1; DELETE FROM t runs. sql_explain is not in the default MCP tool set; fix: harden SQL classification for the tool CLI and MCP server #1644 refuses such input.

Follow-ups (not in this PR)

• The schema cache is keyed by warehouse name only, so schema_detect_pii and schema_search can answer from another database indexed under the same name (pre-existing on main).

Pre-existing on main:

• MySQL and Redshift drivers ignore the binds argument of execute (packages/drivers/src/mysql.ts, redshift.ts); this PR inlines an escaped literal where it needs a schema filter.

Deferred:

• The read-only SQL tokenizer does not lex backslash escapes for BigQuery and Databricks, so a batch step whose {{label.column}} value contains a single quote is refused on those dialects (7109869 escapes it with a backslash instead of sending '').
• Bound the fallback schema/table listing for drivers that only expose listTables() (Oracle, SQL Server, Databricks, ClickHouse, Trino).
• Add the ALTIMATE_MCP_E2E=1 suite to CI (CI changes are excluded from this PR).
• Decide whether answer_commit elicitation should also trigger on a single-entry readings list.
• Per-request answer_commit history if concurrent clients on one daemon become a real use.
• Whether per-shell dbt credentials (env_var, DBT_PROFILES_DIR) should be part of the daemon identity.
• Export Tool.isErrorResult and reuse it in mcp-server/adapter.ts.
• Windows named-pipe owner/ACL check for the tool daemon.
• Daemon client deadline with cancellation of the running request.
• warehouse_add accepting a dbt profile/target or env-var reference, so the agent does not have to read raw credentials.
• A separate flag for warehouse_add's local configuration writes in read-only MCP.
• sql_check passing forceReadOnly (DuckDB/SQLite file open).
• claude-md.ts: write with O_NOFOLLOW or a temp file plus rename, and avoid a second block when the markers were removed.
• A debug log for engine operations dropped from the workspace catalog.
• Carry the classified default connection through dispatch for an omitted warehouse (keeping dbt-first routing), so a connection change during one call cannot reach an unclassified dialect.
• Enforce the DROP DATABASE / DROP SCHEMA / TRUNCATE block in the database, not only by reading the SQL (pre-existing on main). Options: run agent calls with a restricted role, or in a read-only transaction where the driver supports it (Postgres already does for reads); otherwise refuse the call. This covers statements the warehouse assembles at run time, which no text check can see.
• One dialect mapper for Registry.canonicalType (permissions, sql_execute) and warehouseTypeToDialect (data_diff), which return different strings for SQL Server and Fabric.
• Name the side when a cross-dialect data_diff rejects where_clause in one dialect.
• tool daemon stop waiting for, or reporting, another client's in-flight call instead of closing it with "closed the connection before replying".
• Pin standard_conforming_strings=on on Postgres connections, so literals ending in a backslash can be read as reads.
• Lex Oracle q'...' quotes and ClickHouse heredocs instead of refusing them.
• Teach the tokenizer Snowflake/ClickHouse backslash escapes, so batch values with a backslash run there instead of being refused; also for an omitted warehouse whose default is one of them.
• config/config.ts:698 (on main): the agent loader takes dirname of a mobileconfig: managed-preferences source, so a relative {file:...} there does not resolve; the MCP output settings strip the prefix since fdeddbf.
• A CHANGELOG entry for the analyst high effort default, the answer_commit number check and the opt-in .claude/skills copies (documented in docs/docs/configure/).

Summary by CodeRabbit

• New Features
  • Added a headless tool CLI for listing, inspecting, and running tools, with optional daemon reuse and workspace tools.
  • Added an MCP server over stdio with configurable tools, workspace integration, output limits, and write permissions.
  • Added workspace client commands and optional Claude Code routing guidance.
  • Schema inspection can list schemas and tables; SQL execution supports batches and references to earlier results.
• Improvements
  • Added bounded catalog discovery across supported databases, including MongoDB collection counts.
  • Strengthened read-only checks for data comparisons and tool execution.
  • Improved answer validation, analyst reasoning defaults, and compact MCP result output.
  • Workspace skills can be mirrored to Claude Code when enabled; sign-out withdraws managed copies.
• Documentation
  • Added guidance for the tool CLI, MCP server, workspace tools, analyst settings, and Claude Code skill copies.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: fdeddbf
• Latest commit, not reviewed: ee6add5

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• Resume reviews
• Trigger review

Walkthrough

The PR adds MCP and model-free CLI interfaces for data tools and workspace operations. It adds bounded schema discovery and SQL execution, workspace skill mirroring, and changes to analyst answer validation and reasoning options.

Changes: External Tool Interfaces

Layer / File(s)	Summary
MCP server and request handling packages/opencode/src/altimate/mcp-server/*, packages/opencode/src/cli/cmd/mcp.ts, packages/opencode/src/index.ts, docs/docs/configure/mcp-servers.md, packages/opencode/test/altimate/mcp-server/*	Adds an MCP stdio server, configurable tool exposure and write access, an analyze prompt, result links, workspace routing, and shutdown handling.
SQL execution and catalog operations packages/opencode/src/altimate/tools/*, packages/opencode/src/altimate/native/connections/*, packages/drivers/src/mongodb.ts, related tests	Adds SQL batches and result references, MCP CSV spills, bounded schema listings, local catalog discovery, read-only propagation, and MongoDB cursor limits.
Model-free Tool CLI packages/opencode/src/altimate/tool-cli/*, packages/opencode/src/cli/cmd/tool.ts, packages/opencode/src/index.ts, docs/docs/usage/*, docs/mkdocs.yml, related tests	Adds tool listing, context, schema, run, and daemon commands. The CLI supports stdin JSON, connection reloads, optional workspace tools, and write permissions.
Workspace client and skill integrations packages/opencode/src/altimate/workspace/*, packages/opencode/src/altimate/mcp-server/workspace.ts, packages/opencode/src/altimate/api/*, related tests	Adds workspace identity, context, skill and memory operations; allow-listed workspace engine reads; and managed Claude Code skill mirroring.
Analyst answer and reasoning changes packages/opencode/src/altimate/tools/answer-commit*.ts, packages/opencode/src/altimate/prompts/analyst.txt, packages/opencode/src/altimate/session/analyst-effort.ts, packages/opencode/src/session/llm.ts, related tests	Adds answer coverage, reading, and number-provenance checks, plus analyst reasoning-effort selection.

Estimated code review effort: 5 (Critical) | ~120 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Client as MCP client or CLI
  participant Entry as MCP server or Tool CLI
  participant Adapter as Tool adapter
  participant Data as Data tools and connections
  Client->>Entry: Request tool list or execution
  Entry->>Adapter: Select and invoke exposed tool
  Adapter->>Data: Run query or workspace operation
  Data-->>Adapter: Return result
  Adapter-->>Entry: Format result and metadata
  Entry-->>Client: Return output and resource links

Loading

Possibly related PRs

• AltimateAI/altimate-code#1552: Both changes affect workspace skill delivery and synchronization.
• AltimateAI/altimate-code#1587: This PR uses the SQL side-effect classifier whose behavior PR #1587 changes.
• AltimateAI/altimate-code#1606: Both changes involve sign-out and Claude skill withdrawal.

Suggested reviewers: sahrizvi

Merge Risk: Moderate · up to fdedd

The opt-in CLAUDE.md writer can modify a symlink target outside the project if the file changes during the write, and managed output limits can be ignored. Resolve these before merging; also make the daemon shutdown test wait for the run it intends to check.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 58.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main changes: a model-free tool CLI and MCP server for external coding agents.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The pull request description includes all required template sections. It clearly explains the feature, verification steps, cost side effects, screenshots status, checklist completion, and known limita…

Full details: Description check

Explanation

The pull request description includes all required template sections. It clearly explains the feature, verification steps, cost side effects, screenshots status, checklist completion, and known limitations.

Full details: Docstring Coverage

Explanation

Docstring coverage is 58.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 175 functions across 77 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit taps the tools awake
Through pipes and prompts, new paths they make
A bounded query finds its way
While careful checks inspect the day
Workspace skills hop into view
Then carrot crumbs say, “Done for you!”}

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1d811c10-f911-4679-9471-f8ac59645cf1

Commits

Reviewing files that changed from the base of the PR and between fcc3c44 and fdeddbf.

Files selected for processing: 11

• packages/opencode/src/altimate/mcp-server/output-settings.ts
• packages/opencode/src/altimate/tool-cli/daemon.ts
• packages/opencode/src/altimate/tool-cli/permissions.ts
• packages/opencode/src/altimate/tools/data-diff.ts
• packages/opencode/src/altimate/tools/read-only-sql.ts
• packages/opencode/src/altimate/tools/sql-execute.ts
• packages/opencode/test/altimate/analyst-effort.test.ts
• packages/opencode/test/altimate/mcp-server/output-settings.test.ts
• packages/opencode/test/altimate/tool-cli/daemon.test.ts
• packages/opencode/test/altimate/tool-cli/runner.test.ts
• packages/opencode/test/altimate/tools/sql-execute-batch.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[github-actions]

Hey! Your PR title [DO NOT MERGE] Experiment: model-free tool CLI and MCP server on one adapter doesn't follow conventional commit format.

Please update it to start with one of:

• feat: or feat(scope): new feature
• fix: or fix(scope): bug fix
• docs: or docs(scope): documentation changes
• chore: or chore(scope): maintenance tasks
• refactor: or refactor(scope): code refactoring
• test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

Thanks for updating your PR! It now meets our contributing guidelines. 👍

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[kilo-code-bot]

WARNING: TRUNCATE inside an unquoted identifier is hard-blocked

maskSql blanks quoted text but leaves unquoted words in code, so this arm matches any unquoted identifier named truncate (SELECT truncate FROM t, SELECT 1 AS truncate, WITH truncate AS (...)). MySQL, BigQuery and SQLite do not reserve truncate, so those are valid reads, yet classifyAndCheck returns blocked: true and both sql_execute and askFor refuse with "This cannot be overridden" (quoting the identifier is the only workaround). BATCH_STATEMENT above already restricts the ambiguity-prone words (USE/FETCH/OPEN/CLOSE/...) to their statement forms; give TRUNCATE the same treatment (statement start, after ;, or TRUNCATE TABLE) instead of matching every occurrence in code.

[kilo-code-bot]

SUGGESTION: This is the third tokenization of the same SQL

classifyFallback already calls statementsOf (line 181) and tokenizeSql (line 184), and classifySql has already tokenized before calling this function, so a single classification now walks the input up to three times. Hoist one tokenizeSql result and reuse its tokens and complete here and at line 184.

[kilo-code-bot]

SUGGESTION: Third copy of the same dialect helper

This duplicates tool-cli/permissions.ts:21 and sql-execute.ts's defaultTarget dialectOf, with a slightly different guard (no typeof === "string" check). Extract one shared helper so the dialect a permission check reads cannot drift from the one the classification used.

[kilo-code-bot]

SUGGESTION: profiles.yml is re-read and re-parsed on every call

The manifest read above is cached by path/size/mtime, but profileTargetTypes re-reads dbt_project.yml and profiles.yml, re-parses the YAML and re-resolves every env_var on each call. dbtProjectAdapterType runs for every sql_execute that omits the warehouse outside the forced read-only path, so the profile read deserves the same mtime cache as the manifest.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[hkaltai]

Replying to the 20fb04e3 nits:

• data_diff ask on a 120-character prefix: fixed in dac4caa. The tool checks the hard block on the whole source and target SQL, each in its own warehouse's dialect, before it asks. A destructive input now gets "blocked for safety", and a long input whose cut lands inside a literal is no longer refused. where_clause is still checked by the native requireReadOnlyQuery.
• Empty target_warehouse: fixed in the stacked PR fix: harden SQL classification for the tool CLI and MCP server #1644, which keeps this approved PR unchanged. The tool passes "" to the native layer as undefined, so both layers run the target on the source warehouse, as the parameter description says.
• Two dialect mappers (Registry.canonicalType and warehouseTypeToDialect): listed as a follow-up in the PR description. Both are handled or fail closed today.
• Which side rejected where_clause in a cross-dialect diff: follow-up. The message is the only difference.
• tool daemon stop while another client's call runs: follow-up. As you note, nothing is re-sent, so only the message is affected.
• Missing tests: added through the runner. A BigQuery default with your backslash payload (and an ASSERT variant) and a MySQL default with # x\nDROP SCHEMA s are both blocked even with --allow-write. MCP calls go through the same adapter and askFor as tool run.

[hkaltai]

Replying to the c9a9cf91 nits:

• Oracle q'[it's]': still refused. In the stacked PR fix: harden SQL classification for the tool CLI and MCP server #1644, a q'/nq' quote also makes the SQL uncertain, so the hard block's word scan covers it. Lexing q-quotes properly is a follow-up in the PR description.
• Words-alone scan catching literals in uncertain SQL, and TRUNCATE -- c\n(x, 2): both kept as the stated trade-off. In uncertain SQL, TRUNCATE now counts unless ( follows it directly, so a comment of either kind before the ( is refused. That removed the comment-skipping lookahead, which later rounds showed could be misled where comment nesting is uncertain. Both cases are listed under known limits.
• Checked items: thanks. One change in the stacked PR fix: harden SQL classification for the tool CLI and MCP server #1644: the dbt helper uses the project of the adapter dbt has already created (register.ts's cache), and before that the same saved-config-then-discovery precedence. A saved root without dbt_project.yml now means no dbt project, as dispatch treats it.

A note on scope: later rounds kept finding ways to hide a destructive statement where the warehouse builds or decodes it at run time: dynamic SQL in literals, escape encodings, routine bodies, dbt Jinja. Many of those are now caught. Still, the DROP DATABASE / DROP SCHEMA / TRUNCATE block reads the SQL text, so the PR now documents it as best effort, the same as on main. Making it a guarantee in the database is a separate follow-up. See "Scope of the DROP DATABASE / DROP SCHEMA / TRUNCATE block" in the description.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[sahrizvi]

Approved at fa31614 (consensus re-review, round 4: five reviewers, one convergence round).

Earlier findings:

• MAJOR, SQL Server/Fabric E'\': fixed. Through createRunner().run with a recording connector, the DELETE payload is refused without --allow-write, named and omitted. Both DROP DATABASE payloads (with and without ;) are blocked in all four combinations.
• Minor, omitted-warehouse false refusals: fixed. Postgres $$...$$, BigQuery 'it\'s', SQL Server 'C:\reports\' and #temp, and Trino ESCAPE '\' run on a default connection.
• Minor, dbt manifest vs live target: fixed. Profile target types are added, the cached adapter's project is followed, and an unreadable profile adds every dialect.
• Earlier deferred minors (Windows pipe owner check, dbt env in the daemon identity, daemon client deadline, warehouse_add credential hint): still listed under Follow-ups. Reasonably deferred.

Minors posted inline, none blocking:

1. ClickHouse "/` backslash escapes are not modeled (sql-tokenize.ts:128). Not blocking: hardening, contained by single-statement execution.
2. A bare sp_executesql N'DROP ...' skips the hard block under --allow-write (sql-classify.ts:232). Not blocking: needs --allow-write and wrapped dynamic SQL, the documented best-effort class; a one-line fix is safe in a follow-up.
3. Postgres literals ending in a backslash are refused (sql-tokenize.ts:47). Not blocking: fails closed, documented known limit.
4. docs/docs/usage/tool.md:54 still promises an unconditional block. Not blocking: the gap predates this PR, and the best-effort wording is already in the description.

Nits and probe notes are in a separate comment.

[sahrizvi]

MINOR (hardening): ClickHouse "..." and `...` take backslash escapes, but only ' is treated as uncertain here.

ClickHouse's lexer (quotedString in Lexer.cpp) skips the character after \ in all three quote kinds. This tokenizer reads \` as a backslash, a closing backtick and then a doubled one, so the identifier runs on to the last backtick and the code between is masked.

Confirmed through createRunner().run with a recording fake connector ({ w: { type: "clickhouse" } }), named and omitted, with and without --allow-write. In every case the call returns ok and the driver receives the text:

• SELECT 1 AS `a\ ; DELETE FROM t; -- ` `` classifies as a read.
• SELECT 1 AS "a\"" ; DROP DATABASE x; -- " classifies as a read, and the DROP DATABASE is not blocked.

Related and older: the ClickHouse parser accepts SELECT 1 DROP TABLE t and SELECT 1 // c\nDROP TABLE t as one query.

Nothing runs today. The driver sends a single query over HTTP, the server rejects multi-statements, and a statement that starts with SELECT cannot write. So here the boundary rests on the driver, not the classifier.

Suggested fix: add clickhouse to the " backslash check on this line, add the same check for backticks, and add a test.

Blocker: no. Single-statement execution contains it; this is hardening.

[sahrizvi]

MINOR: a bare sp_executesql N'DROP DATABASE prod' (no EXEC) skips the hard block under --allow-write.

T-SQL lets a procedure call omit EXEC when it is the first statement of a batch. DYNAMIC_SQL matches only EXEC/EXECUTE/PREPARE/DO, so the literal's words are never read.

Confirmed through the runner with a recording connector, named and omitted:

• Without --allow-write, the call is refused.
• With --allow-write, it runs (the driver receives it). sp_executesql N'TRUNCATE TABLE dbo.t' behaves the same.
• EXEC sp_executesql N'DROP DATABASE prod' is blocked.

Concatenated dynamic SQL also passes with --allow-write: EXEC('DR' + 'OP DATABASE prod'), Snowflake EXECUTE IMMEDIATE 'DR' || 'OP DATABASE prod', and Postgres DO $$BEGIN EXECUTE 'DR' || 'OP SCHEMA s CASCADE'; END$$. Your description already lists that class as best effort, the same as on main, with enforcement in the database as a follow-up.

This was the same at c9a9cf9. The PR description's "This covers ... sp_executesql" describes code that is not at fa31614.

Suggested fix (one line): add sp_executesql|sp_sqlexec|xp_execresultset to this pattern, with a test.

Blocker: no. It needs --allow-write plus SQL deliberately wrapped as dynamic SQL, which is the documented best-effort class. It is worth folding into your next push.

[sahrizvi]

MINOR (false positive): on PostgreSQL, a literal ending in an odd run of backslashes is now refused, named or omitted.

Adding postgres here makes '...\' uncertain. These were reads at c9a9cf9 and are now refused with "Re-run with --allow-write". Confirmed through the runner on a named and a default Postgres warehouse:

• SELECT replace(p, '\', '/') FROM t
• SELECT * FROM t WHERE note LIKE '%\_%' ESCAPE '\'
• SELECT 'C:\reports\' AS p

Your description lists this as a known limit, because standard_conforming_strings may be off. An option that keeps the safety and the reads: have the Postgres driver set standard_conforming_strings = on for its session. The lexing is then certain, and Postgres can leave ambiguousBackslash.

Blocker: no. It fails closed and is documented.

[sahrizvi]

MINOR (docs): this line still says the block holds unconditionally.

At fa31614 it reads "DROP DATABASE, DROP SCHEMA and TRUNCATE stay blocked even with --allow-write." The PR description says this page documents the block as best effort, but that wording is not pushed yet. Dynamic SQL gets past the block with --allow-write (see the sql-classify.ts comment).

Suggested fix: push the best-effort wording from the description, e.g. "are refused even with --allow-write, by reading the SQL text; SQL the warehouse assembles at run time (dynamic SQL, stored routines) can get past it — use a role without these privileges for a guarantee."

Blocker: no. The gap predates this PR on main, and the wording is already written in the description.

[sahrizvi]

Consensus re-review at fa31614: nits and probe notes not tied to a diff line (companion to the approval above; none of these block)

Nits:

• The PR description is ahead of the head. It describes DROP CATALOG/DROP NAMESPACE, the dbt Jinja check, Oracle q' marked uncertain, sp_executesql coverage, the empty target_warehouse fix and the best-effort docs wording. None of these is at fa31614. This review covers the head as pushed.
• Ordinary reads still refused (all fail closed): Oracle q'[...]', Postgres/Trino U&'...' (with or without UESCAPE), Snowflake 'C:\\x', MySQL 'it\'s', and nested /* /* */ */ on Redshift, Snowflake and BigQuery.
• T-SQL statement rule: OPEN MASTER KEY ... and RECEIVE a, b FROM q do not match it. Both are still writes, because the parser rejects them.

Probed and fine (classifier at fa31614; key cases also run through createRunner().run with a recording connector):

• SQL Server/Fabric: N'', [a]]b], nested comments (T-SQL nests them, per Microsoft's docs), GO, bare procedure calls after a SELECT, IF/ELSE DELETE, UPDATE STATISTICS, COPY INTO, WAITFOR and mixed-case MERGE/EXEC are all writes. Columns named update/open_date, OFFSET ... FETCH, OPTION (USE HINT ...), WITH (NOLOCK) and #temp stay reads.
• Postgres/Redshift/DuckDB: E'\', U&'\', $tag$, non-ASCII tags, nested comments, 1E'\', x$$, Redshift '\'.
• Snowflake: //, $$, $a$ (not a quote) and '\''.
• BigQuery: '''/""", r'\', b'\'', `a\ `` and #.
• MySQL: '\'', -- followed by NBSP or DEL, and /*!.
• Databricks: '\', a -- comment continued by a trailing backslash, r'\' and "\"".
• Oracle/Trino: '\' and U&'\'.
• Unmodeled dialects: E'', $$, nested comments, // and -- ending in a backslash.
• Each of these classifies as the engine reads it, or fails closed. Apart from ClickHouse quoted identifiers (inline), the design now fails closed for SQL it cannot read with certainty in the target dialect.
• hardDenyChecked: the whole SQL is checked under every reachable dialect before the ask, so skipping the 200-character preview check loses nothing.
• Tests: sql-classify, dbt-adapter-type, tool-cli/*, sql-comment-safety and sql-execute pass (342 pass, 0 fail).

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[github-actions]

👋 This PR was automatically closed by our quality checks.

Common reasons:

• New GitHub account with limited contribution history
• PR description doesn't meet our guidelines
• Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

[altimate-harness-bot]

🔁 @hkaltai — this change was promoted to the help-docs hub: https://github.com/AltimateAI/help-docs/pull/147 — please review/merge to keep the docs in sync.