Skip to content

feat(mcp): MCP UX improvement#201

Merged
Knapp-Kevin merged 3 commits into
devfrom
chore/remove-rationale-and-bash-from-report-bug
May 6, 2026
Merged

feat(mcp): MCP UX improvement#201
Knapp-Kevin merged 3 commits into
devfrom
chore/remove-rationale-and-bash-from-report-bug

Conversation

@jinhongkuan

@jinhongkuan jinhongkuan commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

User-facing UX wins, in install order:

  • Setup wizard pre-approves bicameral MCP tools at the user level so post-meeting catch-up flows (pure-read ledger queries fired from a SessionStart brief) don't pop a permission prompt for every tool call. Implements §1 of the v0 productization plan. Writes only to ~/.claude/settings.json after showing the exact diff and requiring explicit y/N. Project .claude/settings.json is never touched.
  • Bash, Edit, Write, Read/Grep/Glob still prompt every time. The wizard auto-approves bicameral MCP tools only — bicameral.reset is deny-listed so destructive work always confirms. Tests guard the no-Bash invariant.
  • /bicameral-report-bug no longer shells out. Removed every Bash block (env probing, python3 -c URL construction, open/xdg-open/start). The skill now derives env from agent context + Read and prints a clickable URL — no shell-permission friction in the bug-report flow.
  • Skill telemetry no longer collects rationale. It was descriptive freeform text that risked leaking session context into PostHog without measurable analytic value.
  • extract_symbols retired from the MCP surface. Redundant — agents resolve files via Grep/Read and use validate_symbols/get_neighbors. Internal adapter method stays (drift detection still uses it).

Linked issues

Refs the v0 Productization Notion page (§1 — Allowlist changes during setup).

Plan / Audit / Seal

Plan: trivial; risk:L1.

  • No schema changes.
  • No new dependencies.
  • New code is wizard-only; only the extract_symbols tool surface and the rationale telemetry param are removed (no callers in production code).

Test plan

  • pytest tests/test_setup_wizard.py -q (11/11)
  • pytest tests/test_preflight_telemetry.py -q (19/19)
  • pytest tests/test_phase1_code_locator.py -q (passes — internal extract_symbols still works)
  • pytest tests/test_hook_command_registration.py -q (passes)
  • Manual: run bicameral-mcp setup in a scratch dir, accept allowlist, confirm ~/.claude/settings.json has the new entries and project .claude/settings.json is untouched.
  • Manual: trigger /bicameral-report-bug in Claude Code, verify zero Bash calls and a clickable GitHub-issue URL with dev,bug labels.

🤖 Generated with Claude Code

…-free

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) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e09300ac-b524-4278-9a2f-6bd4dc4c994b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/remove-rationale-and-bash-from-report-bug

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

Comment @coderabbitai help to get the list of available commands and usage tips.

jinhongkuan and others added 2 commits May 5, 2026 18:22
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>
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>
@jinhongkuan jinhongkuan had a problem deploying to recording-approval May 6, 2026 01:23 — with GitHub Actions Failure
@jinhongkuan jinhongkuan changed the title chore(telemetry): drop rationale; make report-bug bash-free feat(mcp): MCP UX improvement May 6, 2026
@jinhongkuan jinhongkuan added the flow:feature Standard feature/fix PR targeting BicameralAI/dev (the default flow) label May 6, 2026
@Knapp-Kevin Knapp-Kevin merged commit ddc47ba into dev May 6, 2026
8 of 9 checks passed
@Knapp-Kevin Knapp-Kevin deleted the chore/remove-rationale-and-bash-from-report-bug branch May 6, 2026 03:29
Knapp-Kevin added a commit to Knapp-Kevin/bicameral-mcp that referenced this pull request May 21, 2026
…icameralAI#200 A4)

Closes A4 of BicameralAI#200's audit findings (privacy hardening). The current
bug-report skill (post BicameralAI#201) does Read on .bicameral/config.yaml and
dumps <contents> verbatim into the issue body. Workspace IDs, tokens,
allowlists, and env-specific values leak into GitHub issues — exactly
the kind of "presence not value" data the user's "transparency +
accuracy + minimum data shared" directive flagged.

Changes (skills/bicameral-report-bug/SKILL.md, three edits):

1. Step 2 §config.yaml: default extraction is now top-level keys only
   (sorted, one per line, no values / nested keys / comments). Sufficient
   diagnostic signal for "is this bug in the config loader?" while
   leaking zero values by default.

2. Step 3 body-assembly template: replace the verbatim ```yaml <contents>
   ``` block with the keys-only shape. Add a "values redacted by default
   — opt in via Step 3.5 to include verbatim" sentinel line.

3. Step 3.5 transparency preview: add the explicit verbatim toggle as
   a new option in the AskUserQuestion. When the operator picks "Yes,
   but include config.yaml verbatim", the body regenerates with the
   verbatim block and the preview re-displays with the new shape so the
   operator sees what's actually being shipped before clicking through.
   Update the Auto-redacted summary block to print the chosen shape.
   Defense-in-depth: the secret-redaction regex (api_key|token|secret|
   password|bearer) still runs on verbatim contents.

Other findings status:
- A1 (python3 portability) — closed by BicameralAI#201 (full bash removal)
- A6 (browser-open success) — closed by BicameralAI#201 (URL printed, user clicks)
- A7 (telemetry transparency) — partially closed by BicameralAI#201 (rationale
  field dropped from skill_begin telemetry)

Test functionality carve-out justified per doctrine-test-functionality
and the precedent across plan-156 PR A Phase 2, plan-156b Phase 1,
plan-187 Phase 2, plan-197 Phase 1: skill markdown is LLM-consumed
agent-instruction, not pytest-invocable.

Plan-grounding lint (BicameralAI#114) self-test exit 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flow:feature Standard feature/fix PR targeting BicameralAI/dev (the default flow)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants