From 94723c017f868ae5c273f46684480b5359c90cdf Mon Sep 17 00:00:00 2001 From: jinhongkuan Date: Tue, 5 May 2026 18:01:39 -0700 Subject: [PATCH 1/3] chore(telemetry): drop rationale field; rewrite report-bug to be bash-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telemetry no longer collects the per-skill `rationale` one-liner — it was descriptive freeform text that risked leaking session context into PostHog without measurable analytic value. Removes the `rationale` parameter from `record_skill_event`, the `rationale` property from the `bicameral.skill_begin` tool schema, the `_skill_sessions[...]["rationale"]` plumbing in server.py, and the `rationale=...` examples from every skill's `skill_begin` snippet. Report-bug skill: replaces all Bash blocks (env probing, URL construction via `python3 -c`, `open`/`xdg-open` browser launch) with agent-native behavior — `Read` for `.bicameral/config.yaml`, agent-side URL encoding, and printing the URL for the user to click. Keeps the same redaction rules and the Step 3.5 consent gate. Co-Authored-By: Claude Opus 4.7 (1M context) --- server.py | 9 +- skills/bicameral-capture-corrections/SKILL.md | 3 +- skills/bicameral-history/SKILL.md | 3 +- skills/bicameral-ingest/SKILL.md | 3 +- skills/bicameral-preflight/SKILL.md | 3 +- skills/bicameral-report-bug/SKILL.md | 142 +++++++++--------- skills/bicameral-reset/SKILL.md | 3 +- skills/bicameral-sync/SKILL.md | 3 +- skills/bicameral-update/SKILL.md | 3 +- telemetry.py | 3 - 10 files changed, 80 insertions(+), 95 deletions(-) diff --git a/server.py b/server.py index d31a8edb..e6375d45 100644 --- a/server.py +++ b/server.py @@ -59,7 +59,7 @@ SERVER_NAME = "bicameral-mcp" -# In-process map of session_id → {t0, rationale} for skill timing. +# In-process map of session_id → {t0} for skill timing. # Populated by bicameral.skill_begin, consumed by bicameral.skill_end. _skill_sessions: dict[str, dict] = {} @@ -662,10 +662,6 @@ async def list_tools() -> list[Tool]: "type": "string", "description": "Caller-generated UUID that correlates this begin with the matching skill_end", }, - "rationale": { - "type": "string", - "description": "One-liner for why this skill was triggered (e.g. 'user pasted transcript and said track this'). Used for quality feedback analysis.", - }, }, "required": ["skill_name", "session_id"], }, @@ -963,7 +959,6 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: session_id = arguments["session_id"] _skill_sessions[session_id] = { "t0": time.monotonic(), - "rationale": arguments.get("rationale", ""), } return [ TextContent( @@ -991,7 +986,6 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: raw_diagnostic = arguments.get("diagnostic") or {} session_data = _skill_sessions.pop(session_id, None) t0 = session_data["t0"] if session_data else None - rationale = session_data.get("rationale") if session_data else None duration_ms = int((time.monotonic() - t0) * 1000) if t0 is not None else 0 # Validate diagnostic against the per-skill Pydantic model. @@ -1025,7 +1019,6 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: SERVER_VERSION, diagnostic=diagnostic, error_class=error_class, - rationale=rationale, ) response: dict = { "session_id": session_id, diff --git a/skills/bicameral-capture-corrections/SKILL.md b/skills/bicameral-capture-corrections/SKILL.md index 768a33f0..3567ebd9 100644 --- a/skills/bicameral-capture-corrections/SKILL.md +++ b/skills/bicameral-capture-corrections/SKILL.md @@ -27,8 +27,7 @@ Two modes: **At skill start** (before any tool calls): ``` -bicameral.skill_begin(skill_name="bicameral-capture-corrections", session_id=, - rationale="") +bicameral.skill_begin(skill_name="bicameral-capture-corrections", session_id=) ``` **At skill end** (after all work is complete): diff --git a/skills/bicameral-history/SKILL.md b/skills/bicameral-history/SKILL.md index 6465bd34..4bd31c92 100644 --- a/skills/bicameral-history/SKILL.md +++ b/skills/bicameral-history/SKILL.md @@ -28,8 +28,7 @@ directly. **At skill start**: ``` -bicameral.skill_begin(skill_name="bicameral-history", session_id=, - rationale="") +bicameral.skill_begin(skill_name="bicameral-history", session_id=) ``` **At skill end**: diff --git a/skills/bicameral-ingest/SKILL.md b/skills/bicameral-ingest/SKILL.md index 601dd7d9..839b8622 100644 --- a/skills/bicameral-ingest/SKILL.md +++ b/skills/bicameral-ingest/SKILL.md @@ -23,8 +23,7 @@ Ingest **implementation-relevant** decisions from a source document into the dec **At skill start** (before any other tool calls): ``` -bicameral.skill_begin(skill_name="bicameral-ingest", session_id=, - rationale="") +bicameral.skill_begin(skill_name="bicameral-ingest", session_id=) ``` **At skill end** (after all work is complete, including ratification): diff --git a/skills/bicameral-preflight/SKILL.md b/skills/bicameral-preflight/SKILL.md index 82a0f4d4..92111843 100644 --- a/skills/bicameral-preflight/SKILL.md +++ b/skills/bicameral-preflight/SKILL.md @@ -81,8 +81,7 @@ evolve the trigger surface; future configurability will deduplicate. **At skill start** (before any tool calls): ``` -bicameral.skill_begin(skill_name="bicameral-preflight", session_id=, - rationale="") +bicameral.skill_begin(skill_name="bicameral-preflight", session_id=) ``` **At skill end**: diff --git a/skills/bicameral-report-bug/SKILL.md b/skills/bicameral-report-bug/SKILL.md index 0b9a8729..d3d6cbbf 100644 --- a/skills/bicameral-report-bug/SKILL.md +++ b/skills/bicameral-report-bug/SKILL.md @@ -62,39 +62,25 @@ If the user picks "Other" for the title, prompt freely for a one-liner. ## Step 2 — Collect diagnostic context -Run a single Bash block to gather environment data. Output goes -straight into the issue body — do not surface raw output to the user. - -```bash -{ - echo "## Environment" - echo - echo "- bicameral-mcp version: $(bicameral-mcp --version 2>/dev/null || pip show bicameral-mcp 2>/dev/null | awk '/^Version:/ {print $2}' || echo 'unknown')" - echo "- Python: $(python3 --version 2>/dev/null || echo 'unknown')" - echo "- OS: $(uname -srm 2>/dev/null || echo 'unknown')" - echo "- IDE: ${CLAUDE_CODE_VERSION:+Claude Code $CLAUDE_CODE_VERSION}${CURSOR_TRACE_ID:+Cursor}${TERM_PROGRAM:+ ($TERM_PROGRAM)}" - echo "- Shell: $SHELL" - echo - echo "## Repo state" - echo - echo '```' - # Branch name and commit subjects often leak business context (initiative names, - # vendor partners, unannounced features). Redact by default; print only the shape - # of the state, not the content. - COMMIT_COUNT=$(git -C "$(pwd)" log --oneline -3 2>/dev/null | wc -l | tr -d ' ') - echo "branch: " - echo "$COMMIT_COUNT recent commit(s) (titles redacted)" - echo '```' - echo - if [ -f .bicameral/config.yaml ]; then - echo "## .bicameral/config.yaml" - echo - echo '```yaml' - cat .bicameral/config.yaml - echo '```' - fi -} 2>/dev/null -``` +Assemble the diagnostic context from the current session — **do not +shell out**. Use what the agent already knows plus the `Read` tool for +files the user wants to include. + +Sources (in order of preference, all read-only / non-bash): + +- **bicameral-mcp version**: any recent `bicameral.*` tool response + surfaces it; otherwise `Read` `pyproject.toml` and pull the + `version = "..."` line. If unknown, write `unknown`. +- **IDE / harness**: derive from the running environment context the + agent already has (e.g. "Claude Code", "Cursor"). If unknown, + write `unknown`. +- **OS**: derive from the platform the agent already knows (e.g. + `darwin`, `linux`). If unknown, write `unknown`. +- **Repo state**: do **not** read branch names or commit subjects — + they leak business context. Record only the shape: + `branch: ` and `recent commits: titles redacted`. +- **`.bicameral/config.yaml`**: use `Read` on `.bicameral/config.yaml` + if it exists. If `Read` errors (file missing), skip the section. Then assemble in your head (do NOT print to user yet): @@ -114,8 +100,29 @@ Then assemble in your head (do NOT print to user yet): *names*, not the verbatim payload — payloads almost always leak business context (feature names, vendor names, internal codenames). Also redact obvious secrets (`ANTHROPIC_API_KEY=…`, bearer tokens). -- **Environment** + **Repo state** + **config.yaml**: from the Bash - block above. +- **Environment** + **Repo state** + **config.yaml**: assembled as + above, with the same markdown shape: + + ```markdown + ## Environment + + - bicameral-mcp version: + - IDE: + - OS: + + ## Repo state + + ``` + branch: + recent commits: titles redacted + ``` + + ## .bicameral/config.yaml ← only if Read succeeded + + ```yaml + + ``` + ``` --- @@ -221,36 +228,32 @@ AskUserQuestion({ --- -## Step 4 — Open the prefilled GitHub issue +## Step 4 — Output the prefilled GitHub issue URL -Build the URL. Use `python3 -c` to URL-encode safely (do NOT hand-roll -encoding — `?` `&` `#` in the body will break it). +Build the URL inline — no shell, no `python3 -c`. URL-encode the +title and body using standard percent-encoding (RFC 3986) and assemble: -```bash -TITLE='' -BODY='<assembled markdown from Step 3>' +``` +https://github.com/BicameralAI/bicameral-mcp/issues/new?title=<URL-encoded title>&body=<URL-encoded body>&labels=dev,bug +``` + +Encoding requirements: +- Encode every `?`, `&`, `#`, `=`, space (→ `%20` or `+`), newline + (`%0A`), and any non-ASCII characters in both title and body. Triple + backticks and other markdown survive encoding fine. +- Keep the URL under **8000 characters** total. If it exceeds that, + loop back to Step 3 and drop sections per the truncation order + (git log → config.yaml → tool-call argument bodies). + +Then print the full URL on its own line in chat — the user clicks it +to land on the prefilled GitHub draft. Do **not** attempt to launch a +browser yourself; the user opens it. Format: -URL=$(python3 -c ' -import sys, urllib.parse -title, body = sys.argv[1], sys.argv[2] -q = urllib.parse.urlencode({ - "title": title, - "body": body, - "labels": "dev,bug", -}) -print("https://github.com/BicameralAI/bicameral-mcp/issues/new?" + q) -' "$TITLE" "$BODY") - -case "$(uname -s)" in - Darwin) open "$URL" ;; - Linux) xdg-open "$URL" >/dev/null 2>&1 || echo "$URL" ;; - MINGW*|MSYS*|CYGWIN*) start "" "$URL" ;; - *) echo "$URL" ;; -esac ``` +Open this prefilled GitHub issue (review on the page, then click Submit): -If `open` / `xdg-open` is unavailable (CI, headless), print the URL -so the user can copy it. +<full URL> +``` --- @@ -259,10 +262,9 @@ so the user can copy it. Short, factual confirmation. No emoji. Format: ``` -Opened a prefilled GitHub issue on BicameralAI/bicameral-mcp with -the `dev` label. Review, edit if needed, and submit on the page. - -If your browser didn't open, the URL is above. +Posted a prefilled GitHub issue link on BicameralAI/bicameral-mcp +with the `dev` label. Click the URL above, review the page, edit if +needed, and click Submit. ``` **Do not** post the full body back to the user — they'll see it on @@ -272,9 +274,10 @@ the GitHub page. Posting it again is noise. ## Privacy & safety rules -- **Never auto-submit.** The skill's whole contract is: assemble + open. - The user reviews on GitHub and clicks Submit. Anything that leaves - the machine leaves it through the user's hands. +- **Never auto-submit.** The skill's whole contract is: assemble + hand + the user a URL. The user opens it, reviews on GitHub, and clicks + Submit. Anything that leaves the machine leaves it through the + user's hands. - **Redact obvious secrets** before placing them in the body: anything matching `(api[_-]?key|token|secret|password|bearer)\s*[=:]\s*\S+` → replace value with `***REDACTED***`. @@ -314,8 +317,7 @@ the GitHub page. Posting it again is noise. **At skill start**: ``` bicameral.skill_begin(skill_name="bicameral-report-bug", - session_id=<uuid4>, - rationale="<one-liner: what triggered the report>") + session_id=<uuid4>) ``` **At skill end**: @@ -323,5 +325,5 @@ bicameral.skill_begin(skill_name="bicameral-report-bug", bicameral.skill_end(skill_name="bicameral-report-bug", session_id=<stored_id>, errored=<bool>, - error_class="url_open_failed" if browser launch failed else None) + error_class="user_cancelled" if user cancelled at preview else None) ``` diff --git a/skills/bicameral-reset/SKILL.md b/skills/bicameral-reset/SKILL.md index 0baf130b..66e7d601 100644 --- a/skills/bicameral-reset/SKILL.md +++ b/skills/bicameral-reset/SKILL.md @@ -97,8 +97,7 @@ If `wiped=false` with a `replay_errors` entry, the wipe failed before persisting **At skill start**: ``` -bicameral.skill_begin(skill_name="bicameral-reset", session_id=<uuid4>, - rationale="<one-liner: e.g. 'user said ledger looks wrong start over'>") +bicameral.skill_begin(skill_name="bicameral-reset", session_id=<uuid4>) ``` **At skill end** (after confirm or after user cancels at dry-run): diff --git a/skills/bicameral-sync/SKILL.md b/skills/bicameral-sync/SKILL.md index 3119ea35..dbe75811 100644 --- a/skills/bicameral-sync/SKILL.md +++ b/skills/bicameral-sync/SKILL.md @@ -27,8 +27,7 @@ upgrade requests; use `/bicameral-update` instead. **At skill start**: ``` -bicameral.skill_begin(skill_name="bicameral-sync", session_id=<uuid4>, - rationale="<one-liner: e.g. 'user committed and asked to sync decisions'>") +bicameral.skill_begin(skill_name="bicameral-sync", session_id=<uuid4>) ``` **At skill end**: diff --git a/skills/bicameral-update/SKILL.md b/skills/bicameral-update/SKILL.md index 1f5fea3a..1e33404d 100644 --- a/skills/bicameral-update/SKILL.md +++ b/skills/bicameral-update/SKILL.md @@ -14,8 +14,7 @@ with git commits, ledger sync, or compliance checks — those are `/bicameral-sy **At skill start**: ``` -bicameral.skill_begin(skill_name="bicameral-update", session_id=<uuid4>, - rationale="<one-liner: e.g. 'user asked to update to latest version'>") +bicameral.skill_begin(skill_name="bicameral-update", session_id=<uuid4>) ``` **At skill end**: diff --git a/telemetry.py b/telemetry.py index efe2d72c..0b492076 100644 --- a/telemetry.py +++ b/telemetry.py @@ -155,7 +155,6 @@ def record_skill_event( version: str, diagnostic: dict | None = None, error_class: str | None = None, - rationale: str | None = None, ) -> None: """Convenience wrapper for skill-level timing events.""" kwargs: dict = dict( @@ -166,6 +165,4 @@ def record_skill_event( ) if error_class: kwargs["error_class"] = error_class - if rationale: - kwargs["rationale"] = rationale send_event(version, diagnostic=diagnostic, **kwargs) From 3a62bece38133ee849c64d614839a42bce6bb69c Mon Sep 17 00:00:00 2001 From: jinhongkuan <jin@bicameral-ai.com> Date: Tue, 5 May 2026 18:22:47 -0700 Subject: [PATCH 2/3] feat(setup): pre-approve bicameral MCP tools at install (user-level) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements §1 of the v0 productization plan: the wizard offers to write a permissions allowlist into ~/.claude/settings.json so catch-up flows that fire pure-read ledger queries don't spam the user with permission prompts. Bash, Edit, Write, Read/Grep/Glob, and every non-bicameral tool stay prompted — only bicameral MCP tools are pre-approved, and bicameral.reset is deny-listed so it always confirms. Wizard shows the exact diff before writing and requires explicit y/N in interactive runs. Project-level .claude/settings.json is never touched (no commit pollution). Tests cover: user-level write target, no Bash/Edit/Write in the allow list, idempotency (preserves user's own allow entries), declined consent writes nothing, extract_symbols not auto-approved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --- setup_wizard.py | 136 +++++++++++++++++++++++++++++++++++++ tests/test_setup_wizard.py | 114 +++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+) diff --git a/setup_wizard.py b/setup_wizard.py index 1b44b796..1a4f2085 100644 --- a/setup_wizard.py +++ b/setup_wizard.py @@ -633,6 +633,138 @@ def _install_git_pre_push_hook(repo_path: Path) -> bool: return True +# Authoritative list of bicameral MCP tools the wizard offers to +# pre-approve at install time. Excludes bicameral_reset (destructive — +# always prompts) and extract_symbols (no longer exposed as an MCP tool). +_BICAMERAL_ALLOW_TOOLS: list[str] = sorted( + [ + "mcp__bicameral__bicameral_bind", + "mcp__bicameral__bicameral_dashboard", + "mcp__bicameral__bicameral_feedback", + "mcp__bicameral__bicameral_history", + "mcp__bicameral__bicameral_ingest", + "mcp__bicameral__bicameral_judge_gaps", + "mcp__bicameral__bicameral_link_commit", + "mcp__bicameral__bicameral_preflight", + "mcp__bicameral__bicameral_ratify", + "mcp__bicameral__bicameral_resolve_collision", + "mcp__bicameral__bicameral_resolve_compliance", + "mcp__bicameral__bicameral_skill_begin", + "mcp__bicameral__bicameral_skill_end", + "mcp__bicameral__bicameral_update", + "mcp__bicameral__bicameral_usage_summary", + "mcp__bicameral__get_neighbors", + "mcp__bicameral__validate_symbols", + ] +) +_BICAMERAL_DENY_TOOLS: list[str] = ["mcp__bicameral__bicameral_reset"] + + +def _confirm_permissions_diff( + settings_path: Path, new_allow: list[str], new_deny: list[str] +) -> bool: + """Show the diff that would be written and ask y/N. + + Non-interactive runs (CI, scripted setup) auto-approve. Interactive + runs default to "no" — explicit consent is required to write the + allowlist, since it persists across every Claude Code session on the + machine. + """ + print() + print(" Pre-approve bicameral MCP tools — about to write:") + print(f" {settings_path}") + print() + print(" permissions.allow += [") + for t in new_allow: + print(f' "{t}",') + print(" ]") + if new_deny: + print(" permissions.deny += [") + for t in new_deny: + print(f' "{t}",') + print(" ]") + print() + print(" Only the bicameral MCP tools above are pre-approved.") + print(" Bash, Edit, Write, and every non-bicameral tool still prompt") + print(" for permission every time — this wizard does not auto-approve") + print(" any shell call or file write.") + print() + print(" This writes to your *user-level* ~/.claude/settings.json —") + print(" the project .claude/settings.json is never touched, and you") + print(" can revoke any line by editing the file above.") + print() + + if not _is_interactive(): + return True + + raw = input(" Pre-approve these bicameral MCP tools? [y/N] ").strip().lower() + return raw in ("y", "yes") + + +def _install_user_permissions_allowlist() -> bool: + """Pre-approve bicameral MCP tools in ~/.claude/settings.json. + + Implements §1 of the v0 productization plan — moves the permission + friction to install time so catch-up flows (pure-read ledger queries + fired from a SessionStart brief) don't pop a prompt for every tool + call. The user explicitly consents once; after that, only writes + (Bash, Edit, Write) prompt. + + Scope discipline: + - User-level only. Never writes to project .claude/settings.json — + that file gets committed and dragging permission state into the + repo pollutes every clone. + - Bicameral MCP tools only. No Bash, no Read/Grep/Glob, no + non-bicameral tools. Shell calls keep their per-invocation + permission prompt. + - bicameral_reset is deny-listed, not allow-listed. It wipes the + ledger and must always prompt. + + Idempotent — re-running adds only entries the user is missing. + Returns True if anything was written. + """ + settings_path = Path.home() / ".claude" / "settings.json" + settings_path.parent.mkdir(parents=True, exist_ok=True) + + existing: dict = {} + if settings_path.exists(): + try: + existing = json.loads(settings_path.read_text()) + except (json.JSONDecodeError, OSError): + existing = {} + + permissions = existing.setdefault("permissions", {}) + allow_list = permissions.get("allow") + if not isinstance(allow_list, list): + allow_list = [] + permissions["allow"] = allow_list + deny_list = permissions.get("deny") + if not isinstance(deny_list, list): + deny_list = [] + permissions["deny"] = deny_list + + new_allow = [t for t in _BICAMERAL_ALLOW_TOOLS if t not in allow_list] + new_deny = [t for t in _BICAMERAL_DENY_TOOLS if t not in deny_list] + if not new_allow and not new_deny: + print(" Permissions: bicameral MCP tools already pre-approved — no change") + return False + + if not _confirm_permissions_diff(settings_path, new_allow, new_deny): + print(" Permissions: skipped — settings.json untouched") + return False + + allow_list.extend(new_allow) + deny_list.extend(new_deny) + settings_path.write_text(json.dumps(existing, indent=2) + "\n") + print(f" Permissions: pre-approved {len(new_allow)} bicameral MCP tool(s) in {settings_path}") + if new_deny: + print( + f" deny-listed {len(new_deny)} destructive tool(s) " + "(reset will always prompt)" + ) + return True + + def _install_skills(repo_path: Path) -> int: """Copy skill definitions into .claude/skills/ in the target repo.""" skills_src = Path(__file__).parent / "skills" @@ -909,6 +1041,10 @@ def run_setup( print( " Claude Code: installed hooks → link_commit on commit · capture-corrections on session end" ) + # Step 6b — pre-approve bicameral MCP tools at user-level so + # catch-up flows do not spam the user with permission prompts. + # Only bicameral MCP tools; shell calls remain prompted. + _install_user_permissions_allowlist() # Step 7: Git post-commit hook (Guided mode only) if guided: diff --git a/tests/test_setup_wizard.py b/tests/test_setup_wizard.py index 16a4b275..7af81eb0 100644 --- a/tests/test_setup_wizard.py +++ b/tests/test_setup_wizard.py @@ -11,6 +11,7 @@ from __future__ import annotations +import json import sys from pathlib import Path from unittest.mock import patch @@ -88,6 +89,119 @@ def test_session_end_command_invokes_queue_writer(): assert "claude -p" not in cmd +def test_install_user_permissions_allowlist_writes_user_level_only(tmp_path): + """The allowlist must land in ~/.claude/settings.json — never in a + project-level path. v0 productization §1: no commit pollution.""" + home = tmp_path / "home" + home.mkdir() + project = tmp_path / "project" + project.mkdir() + (project / ".claude").mkdir() + project_settings = project / ".claude" / "settings.json" + project_settings.write_text("{}\n") + + with patch.object(setup_wizard.Path, "home", staticmethod(lambda: home)): + with patch.object(setup_wizard, "_is_interactive", return_value=False): + wrote = setup_wizard._install_user_permissions_allowlist() + + assert wrote is True + user_settings = home / ".claude" / "settings.json" + assert user_settings.exists() + # Project file untouched. + assert project_settings.read_text() == "{}\n" + + payload = json.loads(user_settings.read_text()) + allow = payload["permissions"]["allow"] + deny = payload["permissions"]["deny"] + assert "mcp__bicameral__bicameral_preflight" in allow + assert "mcp__bicameral__bicameral_reset" in deny + assert "mcp__bicameral__bicameral_reset" not in allow + + +def test_install_user_permissions_allowlist_does_not_approve_bash(tmp_path): + """Only bicameral MCP tools get pre-approved. Bash/Edit/Write/Read + never enter the allow-list — shell calls keep their permission + prompt. This is the load-bearing UX claim of the wizard step.""" + home = tmp_path / "home" + home.mkdir() + + with patch.object(setup_wizard.Path, "home", staticmethod(lambda: home)): + with patch.object(setup_wizard, "_is_interactive", return_value=False): + setup_wizard._install_user_permissions_allowlist() + + payload = json.loads((home / ".claude" / "settings.json").read_text()) + allow = payload["permissions"]["allow"] + forbidden = {"Bash", "Edit", "Write", "Read", "Grep", "Glob", "NotebookEdit"} + assert not (forbidden & set(allow)), ( + f"wizard auto-approved a non-bicameral tool: {forbidden & set(allow)}" + ) + for entry in allow: + assert entry.startswith("mcp__bicameral__"), ( + f"allow-list contains non-bicameral entry: {entry!r}" + ) + + +def test_install_user_permissions_allowlist_is_idempotent(tmp_path): + """Re-running the wizard does not duplicate entries or wipe other + user-set permissions.""" + home = tmp_path / "home" + home.mkdir() + settings_dir = home / ".claude" + settings_dir.mkdir() + settings_path = settings_dir / "settings.json" + settings_path.write_text( + json.dumps( + { + "permissions": { + "allow": ["Read(./docs/**)"], # user's existing entry + "deny": [], + }, + "theme": "dark", + } + ) + ) + + with patch.object(setup_wizard.Path, "home", staticmethod(lambda: home)): + with patch.object(setup_wizard, "_is_interactive", return_value=False): + first = setup_wizard._install_user_permissions_allowlist() + second = setup_wizard._install_user_permissions_allowlist() + + assert first is True + assert second is False # nothing new to write second time + + payload = json.loads(settings_path.read_text()) + allow = payload["permissions"]["allow"] + # User's pre-existing entry preserved. + assert "Read(./docs/**)" in allow + # Unrelated keys preserved. + assert payload["theme"] == "dark" + # No duplicates. + assert len(allow) == len(set(allow)) + + +def test_install_user_permissions_allowlist_declined_writes_nothing(tmp_path, monkeypatch): + """If the user answers 'no' at the consent prompt, the wizard must + not write to settings.json — the consent moment is the load-bearing + contract.""" + home = tmp_path / "home" + home.mkdir() + + monkeypatch.setattr(setup_wizard, "_is_interactive", lambda: True) + monkeypatch.setattr("builtins.input", lambda *a, **kw: "n") + + with patch.object(setup_wizard.Path, "home", staticmethod(lambda: home)): + wrote = setup_wizard._install_user_permissions_allowlist() + + assert wrote is False + assert not (home / ".claude" / "settings.json").exists() + + +def test_install_user_permissions_allowlist_excludes_extract_symbols(): + """Regression guard: extract_symbols was retired as an MCP tool; + the wizard must not pre-approve a tool that no longer exists.""" + assert "mcp__bicameral__extract_symbols" not in setup_wizard._BICAMERAL_ALLOW_TOOLS + + def test_detect_runner_does_not_return_broken_module_fallback(): """Regression guard for issue #177: the previous `python -m bicameral_mcp` fallback produced a non-functional MCP config because no `bicameral_mcp` From 4c0ad151baa368eec7553ec8baf11a0bd617cf0c Mon Sep 17 00:00:00 2001 From: jinhongkuan <jin@bicameral-ai.com> Date: Tue, 5 May 2026 18:23:00 -0700 Subject: [PATCH 3/3] chore(server): drop extract_symbols MCP tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retires the public MCP-tool surface for extract_symbols. The internal adapter method (RealCodeLocatorAdapter.extract_symbols) and the tree-sitter helper (code_locator.indexing.symbol_extractor.extract_symbols) remain — handlers/detect_drift.py still calls them. What's gone is the MCP-callable tool that exposed the same capability to agents: - server.py: header doc count drops 15 → 14, _TOOL_NAMES entry removed, Tool(...) registration removed, dispatch branch removed. - README.md / docs/v0-architecture-current.md: drop extract_symbols from the deterministic-primitives table. - handlers/link_commit.py: tighten the ungrounded-decisions guidance string to point only at validate_symbols. The capability was redundant on the MCP surface — agents resolve files via Grep/Read and use validate_symbols / get_neighbors for index-aware work. Drift detection (the only internal consumer) keeps its direct call into the adapter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --- README.md | 1 - docs/v0-architecture-current.md | 2 +- handlers/link_commit.py | 6 +++--- server.py | 24 +----------------------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index aae015c2..aeadae3d 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,6 @@ symbol names back to the server. |---|---| | `validate_symbols` | Fuzzy-match symbol name hypotheses against the code index | | `get_neighbors` | 1-hop structural graph traversal (callers, callees, imports) | -| `extract_symbols` | Tree-sitter symbol extraction from a source file | </details> diff --git a/docs/v0-architecture-current.md b/docs/v0-architecture-current.md index 2d2fa7da..48e3c17f 100644 --- a/docs/v0-architecture-current.md +++ b/docs/v0-architecture-current.md @@ -82,7 +82,7 @@ Canonical type: `contracts.py` (see `DecisionStatus = Literal["reflected", "drif - `bicameral.list_unclassified_decisions` / `bicameral.set_decision_level` / `bicameral.evaluate_governance` — decision-tier classification surface **Code locator** (deterministic primitives, no LLM in path): -- `validate_symbols`, `search_code`, `get_neighbors`, `extract_symbols` +- `validate_symbols`, `get_neighbors` There is **no internal/external split** at the MCP boundary — every tool is callable from any client. Discipline lives in the skill layer (`skills/*/SKILL.md`), not in the tool surface. diff --git a/handlers/link_commit.py b/handlers/link_commit.py index a7e416c7..22eda26b 100644 --- a/handlers/link_commit.py +++ b/handlers/link_commit.py @@ -69,9 +69,9 @@ def _is_ephemeral_commit(commit_hash: str, repo_path: str, authoritative_ref: st _GROUNDING_INSTRUCTION_UNGROUNDED = ( " For pending_grounding_checks with reason='ungrounded': use your own " - "code search (Grep/Read), then validate_symbols / extract_symbols to " - "confirm the target, then call bicameral.bind with decision_id, " - "file_path, symbol_name, and optionally start_line/end_line." + "code search (Grep/Read), then validate_symbols to confirm the target, " + "then call bicameral.bind with decision_id, file_path, symbol_name, " + "and optionally start_line/end_line." ) # V1 D1 / Codex pass-12 finding #2: relocation cases (symbol_disappeared) diff --git a/server.py b/server.py index e6375d45..13b1f6af 100644 --- a/server.py +++ b/server.py @@ -1,6 +1,6 @@ """Bicameral MCP Server — Bicameral decision ledger + code locator tools. -15 tools: +14 tools: bicameral.link_commit — heartbeat: sync a commit into the decision ledger bicameral.ingest — ingest normalized decision/code evidence and advance source cursors bicameral.update — check for or apply a recommended bicameral-mcp update @@ -15,7 +15,6 @@ bicameral.set_decision_level — set decision_level (L1/L2/L3) on a single decision (#77) validate_symbols — fuzzy-match candidate symbol names against the code index get_neighbors — 1-hop structural graph traversal around a symbol - extract_symbols — tree-sitter symbol extraction from a source file Run with: bicameral-mcp (or python server.py) for stdio transport. @@ -114,7 +113,6 @@ def _resolve_server_version() -> str: "bicameral.record_bypass", "validate_symbols", "get_neighbors", - "extract_symbols", ] server = Server(SERVER_NAME) @@ -927,23 +925,6 @@ async def list_tools() -> list[Tool]: "required": ["symbol_id"], }, ), - Tool( - name="extract_symbols", - description=( - "Extract all symbols (functions, classes) from a source file via static parsing. " - "Returns symbol names, types, and line ranges. No index required." - ), - inputSchema={ - "type": "object", - "properties": { - "file_path": { - "type": "string", - "description": "Absolute or repo-relative path to the source file", - }, - }, - "required": ["file_path"], - }, - ), ] @@ -1222,9 +1203,6 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: elif name == "get_neighbors": data = await asyncio.to_thread(ctx.code_graph.get_neighbors, arguments["symbol_id"]) return [TextContent(type="text", text=json.dumps(data, indent=2))] - elif name == "extract_symbols": - data = await ctx.code_graph.extract_symbols(arguments["file_path"]) - return [TextContent(type="text", text=json.dumps(data, indent=2))] else: raise ValueError(f"Unknown tool: {name}")