[integrations] Thought management — update and delete tools

[alanshurafa]

Summary

• Adds update_thought and delete_thought MCP tools for managing existing thoughts
• Update re-embeds and re-classifies automatically via OpenRouter
• Two deployment options: add to core server (Option A) or standalone Edge Function (Option B)
• Follows existing OB1 patterns (Hono + MCP SDK + Supabase client)

Why

The core server has capture (write) and search (read), but no way for AI clients to edit or remove thoughts. As your second brain grows, you need to maintain it — fix errors, remove outdated info, reclassify. This closes the CRUD loop.

Design Decision

Packaged as an extension (not a core server modification) to respect the contribution guidelines. Users opt in by adding the tools to their server or deploying the standalone function.

Test plan

• Update re-embeds content correctly
• Update re-classifies metadata
• Delete removes row
• Error handling for missing IDs
• Auth middleware prevents unauthorized access

🤖 Generated with Claude Code

[justfinethanku]

Saw your Discord post — impressive work across all of these. I'm going to personally review each one. Not today, but they're on my list.

[justfinethanku]

Review of PR #55: Thought Management Tools

✅ What's Good

Strong concept and execution:

• Fills a real gap in the Open Brain ecosystem — the core server has create (capture) and read (search), but this completes the CRUD loop with update and delete
• Well-structured code following the existing OB1 patterns (Hono + MCP SDK + Supabase client)
• Two deployment options (add to core vs standalone) show good understanding of user needs
• Clean TypeScript implementation with proper error handling
• Automatic re-embedding and re-classification on update is a thoughtful touch

Good documentation:

• README is clear and well-organized with proper sections
• Troubleshooting section included
• Code examples are complete and copy-paste ready
• Both deployment paths are documented

Metadata compliant:

• All required fields present
• Valid JSON structure
• Appropriate tags and difficulty level

⚠️ Issues That Need Addressing

1. Extensions are Curated — Requires Maintainer Approval

Per CONTRIBUTING.md and extensions/README.md, the extensions/ directory is curated, not open for direct community contributions:

Extensions are curated — discuss with maintainers before submitting. Propose a new extension.

This contribution should either:

• Option A: Be moved to integrations/ (which is open for community contributions and fits the pattern — this is an MCP extension)
• Option B: Go through the extension proposal process first with maintainers

The distinction matters because extensions form a progressive learning path (currently 6 builds: household-knowledge → home-maintenance → family-calendar → meal-planning → professional-crm → job-hunt). This contribution doesn't fit that pedagogical sequence.

2. Missing Required Extension Fields in metadata.json

If this stays as an extension, the metadata.json is missing required extension-specific fields:

• learning_order — integer position in the extension learning path (1-6)
• requires_primitives — array of primitive dependencies (can be empty [] if none)

Per the metadata schema:

"learning_order": {
  "type": "integer",
  "minimum": 1,
  "description": "Position in the extension learning path (1-6). Extensions only."
}

3. Missing SQL Files

Per automated review rule #6:

extensions/ have both SQL and code files

Extensions are expected to include database schema changes (table creation, indexes, RLS policies, etc.). This contribution only has code files. If this is truly meant to work with the existing thoughts table without schema changes, that's another indicator it should be in integrations/ rather than extensions/.

4. Remote MCP Pattern Requirement

The README shows both options using local implementation patterns, but per CONTRIBUTING.md rule #14:

Extensions and integrations must use remote MCP via Supabase Edge Functions. No claude_desktop_config.json, no local Node.js stdio servers.

The Option B deployment should be clearer about deploying to Supabase Edge Functions and connecting via Claude Desktop's custom connectors UI (Settings → Connectors → Add custom connector → paste URL). See docs/01-getting-started.md Step 7 for the correct pattern.

The Edge Function code itself looks correct, but the deployment instructions should explicitly state:

1. Deploy to Supabase: supabase functions deploy thought-management
2. Set secrets: supabase secrets set OPENROUTER_API_KEY=... MCP_ACCESS_KEY=...
3. Connect in Claude Desktop via custom connector with the deployed URL

5. GRANT Step Missing

Per the CONTRIBUTING.md requirements:

GRANT step — Every extension that creates tables MUST include a GRANT step. Supabase no longer auto-grants CRUD permissions to service_role on new projects

While this doesn't create new tables, if it were to become an extension with schema changes, it would need explicit GRANT statements.

📋 Required Changes

1. Decide on category: Either move to integrations/thought-management/ or go through the extension proposal process with maintainers
2. If staying as extension: Add learning_order and requires_primitives to metadata.json
3. If staying as extension: Add database schema files (or explain why none are needed)
4. Update deployment instructions for Option B to clarify the remote MCP deployment pattern
5. Update metadata.json category if moving to integrations

💡 Recommendation

Move this to integrations/ — it's a perfect fit:

• Extends the MCP server with new tools (which is what integrations do)
• Doesn't require new database tables (integrations can work with existing schema)
• Doesn't need to fit into the progressive learning path
• Still valuable and well-implemented

Then update:

• Folder path: integrations/thought-management/
• metadata.json: "category": "integrations"
• PR title: [integrations] Thought management — update and delete tools
• Remove learning_order from metadata (not needed for integrations)

Verdict

Verdict: Significant changes needed

The contribution is technically sound and well-documented, but it's in the wrong category. Extensions are curated and require maintainer approval + fit into the learning path. This would be an excellent integrations/ contribution and could be merged quickly with that change. If the author wants it as an extension, they need to go through the proposal process first.

---

Great work on the implementation, @alanshurafa! The code quality and documentation are solid. Just needs to land in the right part of the repo. Happy to see this merged once the category is corrected.

[matthallett1]

@claude review this PR

[github-actions]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[matthallett1]

@claude review this PR

[github-actions]

Claude finished @matthallett1's task in 1m 55s —— View job

---

PR Review: [integrations] Thought Management Tools

• Gather context and read changed files
• Review code quality, safety, and OB1 alignment
• Post final review

---

First, the good news: The main issue @justfinethanku flagged in their earlier review — wrong category — has been addressed. The contribution is now correctly under integrations/thought-management/ with "category": "integrations" in metadata.json. That was the right call, and the structural requirements for integrations (code file, README, metadata.json) are all present.

Here's my focused review on things that require judgment:

---

Alignment and Value

This fills a genuine gap. The core MCP server is append-only (capture + search); there's no way for an AI client to correct or remove thoughts as a second brain grows. update_thought with automatic re-embedding and re-classification is a thoughtful touch — editing a thought's content also updates its vector and metadata, so search quality doesn't degrade over time. This is the right design.

The choice to package it as a standalone integration (rather than modifying the core server) is correct per contribution guidelines.

---

Issues That Need Addressing

1. Missing supabase secrets set step in deployment (blocker for users)

README.md lines 82–91 walk through deployment but never tell the user to set the required environment variables. A user following these steps would deploy the function and then get cryptic undefined errors when it tries to call OpenRouter with a null API key.

The deployment steps need a Set secrets step between deploy and connect:

supabase secrets set OPENROUTER_API_KEY=your_key MCP_ACCESS_KEY=your_key

Fix this →

2. Unhandled API errors in getEmbedding (index.ts lines 13–24)

If OpenRouter returns a non-200 response (rate limit, bad key, model error), json.data[0].embedding will throw a runtime exception rather than surfacing a clean error to the MCP client. The extractMetadata function has a try-catch for JSON parsing but not for the fetch itself.

Compare:

// current — will throw if OpenRouter errors
const json = await res.json();
return json.data[0].embedding;

// safer
if (!res.ok) throw new Error(`OpenRouter error: ${res.status}`);
const json = await res.json();
if (!json.data?.[0]?.embedding) throw new Error("Unexpected OpenRouter response shape");
return json.data[0].embedding;

These failures would show up as unhandled 500s rather than the graceful { content: [{ type: "text", text: "Error: ..." }] } pattern used everywhere else in the tool handlers.

Fix this →

3. Deployment section doesn't distinguish Option A vs Option B

README.md has two setup options (add to existing server vs deploy standalone), but the Deployment section at line 82 only covers the standalone path. A user choosing Option A doesn't need any of those supabase functions commands — they're just copying code into their existing function. Someone skimming could be confused about whether deployment steps are required for Option A.

Suggest splitting into Option A: done (no extra deployment steps; your existing function handles it) and Option B: standalone deployment with the current steps.

---

Minor Observations

Metadata field replacement: index.ts line 63 replaces the entire metadata JSONB field:

metadata: { ...metadata, source: "mcp", updated: true }

This spreads the newly extracted metadata over fixed fields, which is correct for re-classification. But it means any metadata that was added outside this tool (e.g., a contact_id from the CRM extension) will be silently dropped on update. The troubleshooting section could mention this: "Updating a thought replaces its metadata — any custom metadata fields will be lost."

--no-verify-jwt not explained: The deploy command uses this flag but doesn't explain why. A security-conscious user might wonder why JWT verification is disabled. A one-liner — "we use our own MCP_ACCESS_KEY check instead of Supabase JWTs" — would remove the concern.

---

What's Good

• Code follows Hono + MCP SDK + Supabase client pattern correctly
• Auth middleware (x-brain-key check) matches existing integrations
• Promise.all for parallel embedding + classification is correct
• Tool audit link (../../docs/05-tool-audit.md) is present ✅
• metadata.json has all required fields, correct category ✅
• No credentials hardcoded ✅
• No destructive SQL ✅

---

Verdict

Close, but the missing secrets step is a real usability gap — a user following the README as written will hit a confusing failure at runtime. Fix that, and consider the error handling in getEmbedding. The deployment section restructure (Option A vs B) would improve clarity. None of these are architectural concerns — the design and implementation are sound.

[matthallett1]

The concept is solid — update/delete with automatic re-embedding fills a real gap. But Codex review (gpt-5.4) found 3 critical issues that need to be addressed:

P1: Auth fails open when MCP_ACCESS_KEY is unset. At index.ts:89, if the function is deployed without MCP_ACCESS_KEY configured, a request that omits x-brain-key passes the check because undefined !== undefined is false. This makes the delete endpoint publicly writable. Fix: check that MCP_ACCESS_KEY is defined before comparing, and reject all requests if it's not set.

P1: Shared MCP transport across sessions. StreamableHTTPTransport is sessionful (mcp-session-id), but the current code creates one instance at module scope. Concurrent clients will collide. The other MCP servers in this repo (e.g., server/index.ts, the extensions) create the transport inside the request handler. Match that pattern.

P1: Missing Accept header workaround for Claude Desktop. The handler forwards requests straight to StreamableHTTPTransport without the Accept-header patch used in server/index.ts and the extensions. Claude Desktop will get a 406 on the initial POST, meaning the advertised primary client can't connect at all.

Also: the automated Claude review flagged the missing supabase secrets set step in the README and unhandled errors in getEmbedding — both worth fixing too.

The design is right. These are implementation issues that can be fixed by following the patterns already established in the core server and extensions.

[alanshurafa]

Closing this PR as part of the OB1 Alpha Milestone consolidation. This feature is being rebuilt as one of 12 clean, gate-compliant PRs that together form the alpha upgrade path. The consolidated PRs will be submitted once verified on the fork. See the full plan for details.

[alanshurafa]

Closing as part of the OB1 Alpha Milestone consolidation. This feature is being rebuilt as one of 12 clean, gate-compliant PRs that together form the alpha upgrade path. The consolidated PRs will be submitted once verified on the fork.