docs(skills): enforce Web UI as sole method for connection info setup#1455
Conversation
…setup Add clear instructions that connection info can ONLY be set via the Web UI. Agents were attempting to configure connections via ibis-server API calls, which doesn't work — the ibis-server has no public write endpoint for connection info. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- wren-connection-info: remove "or directly in API calls" wording that gave agents a reason to bypass the Web UI - wren-usage: clarify that credential changes go through the Web UI, and @wren-connection-info is a field reference only Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- getting_started_with_claude_code: remove setup_connection from MCP tool table, add note that connection is Web UI only, fix wren-connection-info description to "field reference" - mcp-server README: replace setup_connection call in generate-mdl example with health_check + Web UI step - quickstart: clarify "Connect a different database" points to Web UI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- generate-mdl: 1.3 → 1.4 - wren-connection-info: 1.3 → 1.4 - wren-usage: 1.0 → 1.1 Updated SKILL.md frontmatter, versions.json, and index.json. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
bot
added
the
documentation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughDocs and skill metadata changed to require configuring database connection info via the MCP Web UI (http://localhost:9001). MCP server registration and MDL generation flows were updated to use MCP tooling (claude mcp add/list, health_check, list_remote_*), skill versions bumped, and programmatic credential setup removed. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/wren-usage/SKILL.md (2)
15-18:⚠️ Potential issue | 🟡 MinorUpdate version references in the version check section.
The version check section still references version
1.0, but the skill was bumped to1.1. This will cause incorrect version comparison behavior.🛠️ Proposed fix
Silently fetch `https://raw.githubusercontent.com/Canner/wren-engine/main/skills/versions.json`. -Compare the `wren-usage` key with this skill's version (`1.0`). +Compare the `wren-usage` key with this skill's version (`1.1`). If the remote version is newer, notify the user before proceeding: -> A newer version of the **wren-usage** skill is available (remote: X.Y, installed: 1.0). +> A newer version of the **wren-usage** skill is available (remote: X.Y, installed: 1.1). > Update with:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/wren-usage/SKILL.md` around lines 15 - 18, Update the hardcoded installed-version string in the version check section from "1.0" to the current "1.1" so the comparison against the remote `wren-usage` key is correct; locate the text that reads "installed: 1.0" (the version check / notification block for the wren-usage skill) and change it to "installed: 1.1" so the message "A newer version of the **wren-usage** skill is available (remote: X.Y, installed: 1.1)" reflects the actual installed version.
140-140:⚠️ Potential issue | 🟡 MinorRemove
setup_connection(...)from the MCP tools table.This tool does not exist in the MCP server. The PR objective is to remove references to
setup_connectionsince connection info can only be configured via the Web UI. The code atmcp-server/app/wren.py:244-301confirms there is nosetup_connectiontool registered.🛠️ Proposed fix
| `deploy(mdl_file_path=...)` | Load a compiled `mdl.json` | -| `setup_connection(...)` | Configure data source credentials | | `list_remote_tables(...)` | Introspect database schema |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/wren-usage/SKILL.md` at line 140, Remove the non-existent tool entry `setup_connection(...)` from the MCP tools table in SKILL.md: delete the table row that lists `setup_connection(...) | Configure data source credentials` and search the SKILL.md for any other references to `setup_connection` to remove or replace them; confirm there is no corresponding registration in the Wren tools code (the wren module's tool registration does not include a setup_connection tool) so no code changes are required beyond removing the documentation references.
🧹 Nitpick comments (1)
mcp-server/README.md (1)
208-208: Consider adding a language specifier to the fenced code block.Static analysis flagged this code block as missing a language identifier. Adding
textor another appropriate specifier improves rendering consistency.💅 Proposed fix
-``` +```text User → "Generate an MDL for my PostgreSQL ecommerce database"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-server/README.md` at line 208, Replace the plain fenced code block delimiter in the README example with a language specifier (e.g., change the opening ``` to ```text) so the snippet is rendered consistently; update the block containing `User → "Generate an MDL for my PostgreSQL ecommerce database"` to begin with ```text and leave the closing ``` unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@skills/wren-usage/SKILL.md`:
- Around line 15-18: Update the hardcoded installed-version string in the
version check section from "1.0" to the current "1.1" so the comparison against
the remote `wren-usage` key is correct; locate the text that reads "installed:
1.0" (the version check / notification block for the wren-usage skill) and
change it to "installed: 1.1" so the message "A newer version of the
**wren-usage** skill is available (remote: X.Y, installed: 1.1)" reflects the
actual installed version.
- Line 140: Remove the non-existent tool entry `setup_connection(...)` from the
MCP tools table in SKILL.md: delete the table row that lists
`setup_connection(...) | Configure data source credentials` and search the
SKILL.md for any other references to `setup_connection` to remove or replace
them; confirm there is no corresponding registration in the Wren tools code (the
wren module's tool registration does not include a setup_connection tool) so no
code changes are required beyond removing the documentation references.
---
Nitpick comments:
In `@mcp-server/README.md`:
- Line 208: Replace the plain fenced code block delimiter in the README example
with a language specifier (e.g., change the opening ``` to ```text) so the
snippet is rendered consistently; update the block containing `User → "Generate
an MDL for my PostgreSQL ecommerce database"` to begin with ```text and leave
the closing ``` unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 734ebb8b-ae3f-45b4-9e1e-0ea7fb87e220
📒 Files selected for processing (8)
docs/getting_started_with_claude_code.mddocs/quickstart.mdmcp-server/README.mdskills/generate-mdl/SKILL.mdskills/index.jsonskills/versions.jsonskills/wren-connection-info/SKILL.mdskills/wren-usage/SKILL.md
The previous ordering had generate-mdl (Phase 2) before MCP registration (Phase 3). This meant agents had no MCP tools available and fell back to calling ibis-server API directly, bypassing connection info and hitting confusing errors. New order across all docs and skills: 1. Start Docker container 2. Configure connection (Web UI) + Register MCP + New session 3. Generate MDL (via MCP tools) 4. Verify and query Affected files: - wren-quickstart skill (1.2 → 1.3) - docs/quickstart.md Option B - docs/getting_started_with_claude_code.md manual setup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Recommend enabling read-only mode in the Web UI after setup is confirmed working. This prevents agents from modifying connection info or directly introspecting the database beyond the defined MDL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/wren-quickstart/SKILL.md`:
- Around line 81-83: Replace unlabeled fenced code blocks used for directives
with language-labeled fences to satisfy markdownlint MD040; locate the three
occurrences containing the directive tokens "@wren-mcp-setup", "@generate-mdl",
and "@wren-project" (around the blocks at lines referenced in the review) and
change each opening triple-backtick to include a language identifier such as
"text" (e.g., ```text) so the fenced blocks become labeled and the lint warnings
are resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f747c290-a645-46e6-aa83-bca73b9106d3
📒 Files selected for processing (5)
docs/getting_started_with_claude_code.mddocs/quickstart.mdskills/index.jsonskills/versions.jsonskills/wren-quickstart/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/index.json
- docs/quickstart.md
| ``` | ||
| @generate-mdl | ||
| @wren-mcp-setup | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (markdownlint MD040).
Line 81, Line 121, and Line 136 use unlabeled fenced blocks, which will keep triggering lint warnings.
Suggested fix
-```
+```text
`@wren-mcp-setup`- +text
@generate-mdl
-```
+```text
`@wren-project`
</details>
Also applies to: 121-123, 136-138
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @skills/wren-quickstart/SKILL.md around lines 81 - 83, Replace unlabeled
fenced code blocks used for directives with language-labeled fences to satisfy
markdownlint MD040; locate the three occurrences containing the directive tokens
"@wren-mcp-setup", "@generate-mdl", and "@wren-project" (around the blocks at
lines referenced in the review) and change each opening triple-backtick to
include a language identifier such as "text" (e.g., ```text) so the fenced
blocks become labeled and the lint warnings are resolved.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
2 participants
Summary
@wren-connection-infois field reference onlysetup_connection(...)from MCP tool table, fix skill descriptionsetup_connection()in generate-mdl example withhealth_check()+ Web UI stepContext
A user's AI agent attempted to configure DuckDB connection info programmatically via ibis-server API calls instead of using the Web UI. Root cause: multiple docs and skills either implied or explicitly stated that API-based configuration was possible (
setup_connection(...)tool reference, "or directly in API calls for advanced workflows"). The ibis-server has no public write endpoint for connection info — only the Web UI can do this.Test plan
/generate-mdlwith an unconfigured connection — agent should direct user to Web UI🤖 Generated with Claude Code
Summary by CodeRabbit