feat(mcp): onboarding tools — review, correct, reclassify via MCP - #249
Conversation
…euse The onboarding-report editability summarizer is moved out of src/api into src/lib/editability-summary.ts so the MCP get_onboarding_report tool (#140) can share the exact builder the REST report uses without crossing the api module boundary. src/api/onboarding-report.ts re-exports it to keep the REST import path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) Five MCP tools give AI agents parity with the REST onboarding loop. Each is a thin adapter over the SAME db/index.js query the matching REST handler calls, so the two surfaces cannot drift: - review_editability — per-paragraph value/confidence/evidence/override from the shared getSpecTree query; optional maxConfidence low-confidence review filter. - get_onboarding_report — spec-keyed report reusing the shared summarizeEditability builder plus persisted style-source state. styleDerivation/parseWarnings are import-time-only (raw bytes not persisted) and explicitly marked unavailable. - set_/clear_editability_override — the #136 PATCH as tools (shared override query). - reclassify_spec — POST /specs/:id/reclassify as a tool; returns the before/after ReclassifyReport. preview=true diffs without persisting. Handlers in onboarding-handlers.ts, registration in onboarding-tools.ts (keeps tools.ts under the 400-line cap). All run under the existing /mcp transport rate limiter (#69) — no new route or limiter. Handlers never throw: every failure returns { isError: true }. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#140) Add an MCP tools table to the README API Surface section listing all tools including the new review_editability, get_onboarding_report, set_/clear_editability_override, and reclassify_spec; extend the ARCHITECTURE file-structure map with onboarding-tools.ts / onboarding-handlers.ts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughFive MCP onboarding tools ( ChangesMCP Onboarding & Editability Tools
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
README.md (1)
63-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: Clarify the phrasing "read-or-correct over
db/index.jsqueries".The phrase could be more natural. Consider: "read or correct using
db/index.jsqueries" or "thin adapters overdb/index.jsqueries for reading and correction."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 63 - 65, The phrase "read-or-correct over `db/index.js` queries" in the README description of the Streamable HTTP server's POST /mcp endpoint is awkwardly phrased and unclear. Replace it with more natural wording such as "read or correct using `db/index.js` queries" or "thin adapters over `db/index.js` queries for reading and correction" to improve clarity and readability of the documentation.docs/superpowers/plans/2026-06-23-mcp-onboarding-tools.md (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: Remove redundant "exact" for brevity.
Line 5 contains a static analysis style flag: "exact same" is more verbose than necessary. Suggested revision: "each reusing the same
db/index.jsqueries the REST handlers use."📝 Suggested revision
-**Goal:** Add 5 MCP tools so AI agents can drive the onboarding loop (report, review, correct, reclassify) over `POST /mcp`, each reusing the exact same `db/index.js` queries the REST handlers use. +**Goal:** Add 5 MCP tools so AI agents can drive the onboarding loop (report, review, correct, reclassify) over `POST /mcp`, each reusing the same `db/index.js` queries the REST handlers use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-06-23-mcp-onboarding-tools.md` at line 5, The Goal statement contains redundant wording with "exact same" which can be simplified for brevity. In the Goal section where it states "each reusing the exact same `db/index.js` queries the REST handlers use", remove the word "exact" to make it read "each reusing the same `db/index.js` queries the REST handlers use." This maintains the meaning while improving conciseness.Source: Linters/SAST tools
ARCHITECTURE.md (1)
724-724: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
tools.tsdescription to acknowledge delegated onboarding tool registration.The tools.ts comment lists eight tools directly registered within that module, but per the context,
registerTools()also callsregisterOnboardingTools(server)(line 341 of tools.ts), which adds five more tools. The comment should clarify this delegation so readers consulting ARCHITECTURE.md understand the complete set of tools exposed viaregisterTools().📝 Suggested revision for line 724
-│ │ ├── tools.ts # registerTools(server): search_library, list_sections, get_spec, get_paragraph, parse_document, generate_docx, load_files, coordination_report +│ │ ├── tools.ts # registerTools(server): delegates to registerLibraryTools, registerProjectTools, registerSpecTools, registerParserTools, registerGeneratorTools, registerLoaderTools, registerCoordinationTools, and registerOnboardingToolsAlternatively, if you prefer a more concise approach:
-│ │ ├── tools.ts # registerTools(server): search_library, list_sections, get_spec, get_paragraph, parse_document, generate_docx, load_files, coordination_report +│ │ ├── tools.ts # registerTools(server): wires all tool groups (search, spec, parser, generator, loader, coordination, onboarding)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ARCHITECTURE.md` at line 724, The comment for tools.ts on line 724 of ARCHITECTURE.md currently only lists the eight tools that are directly registered within the tools module, but it does not acknowledge that the registerTools function also calls registerOnboardingTools(server) which registers five additional tools. Update the comment to clarify that registerTools() delegates tool registration to registerOnboardingTools(server) in addition to the eight tools directly registered, so that readers understand the complete set of tools exposed through the registerTools() function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ARCHITECTURE.md`:
- Line 724: The comment for tools.ts on line 724 of ARCHITECTURE.md currently
only lists the eight tools that are directly registered within the tools module,
but it does not acknowledge that the registerTools function also calls
registerOnboardingTools(server) which registers five additional tools. Update
the comment to clarify that registerTools() delegates tool registration to
registerOnboardingTools(server) in addition to the eight tools directly
registered, so that readers understand the complete set of tools exposed through
the registerTools() function.
In `@docs/superpowers/plans/2026-06-23-mcp-onboarding-tools.md`:
- Line 5: The Goal statement contains redundant wording with "exact same" which
can be simplified for brevity. In the Goal section where it states "each reusing
the exact same `db/index.js` queries the REST handlers use", remove the word
"exact" to make it read "each reusing the same `db/index.js` queries the REST
handlers use." This maintains the meaning while improving conciseness.
In `@README.md`:
- Around line 63-65: The phrase "read-or-correct over `db/index.js` queries" in
the README description of the Streamable HTTP server's POST /mcp endpoint is
awkwardly phrased and unclear. Replace it with more natural wording such as
"read or correct using `db/index.js` queries" or "thin adapters over
`db/index.js` queries for reading and correction" to improve clarity and
readability of the documentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2234037d-a89a-4452-9896-fa718c5c5207
📒 Files selected for processing (9)
ARCHITECTURE.mdREADME.mddocs/superpowers/plans/2026-06-23-mcp-onboarding-tools.mdsrc/api/onboarding-report.tssrc/lib/editability-summary.tssrc/mcp/onboarding-handlers.tssrc/mcp/onboarding-tools.tssrc/mcp/onboarding.integration.test.tssrc/mcp/tools.ts
…on note - README: 'reading or correcting via db/index.js queries' (clearer than 'read-or-correct over') - ARCHITECTURE: note registerTools delegates to registerOnboardingTools (tree comment was stale) - plan doc: drop redundant 'exact' from 'exact same' All three are CodeRabbit body nitpicks (🔵 trivial); doc-only, no src/ change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
CodeRabbit body nitpicks (🧹 Nitpick comments, 3) — all addressed in 15cbc55 (docs-only, no
|
#325) * docs(readme): sync capabilities to last month of merged PRs Reflect shipped work in the README's "Included Today", "API Surface", and MCP tool table, validated against the merged diffs and current main: - PDF ingest (text-layer + OCR + font-encoding recovery) accepted by POST /parse (#287, #290, #311) - coordination / E&O report + submittal register (#241, #269, #277, #282, #283, #284) and article-role tagging (#273) - onboarding pipeline: library import, editability review/override, reclassify, finalize/reopen, open-comments (#243, #247, #248, #249, #272) - spec/project soft-delete + restore (#257, #313), document concurrency (#197), revision/addendum manual rendering (#221), numbering profiles (#317, #322) - add missing MCP tools get_numbering_profile, submittal_register, open_comments_report; document GET /docs (Scalar) (#213, #285) - add Example Client pointer to examples/web_ui_demo (#225) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(roadmap): move shipped work to done; re-date to 2026-07-01 Reconcile the roadmap with merged reality (was stamped 2026-06-17). Moved from planned/in-progress to Included, each validated against the diff: - PDF ingest (#287, #290, #311) — remove from "Later" - deep paragraph nesting pr6/pr7 (#215) - revision nomenclature (#216) + revision/addendum manual rendering (#221) — the two "Near Term" Phase 2e items are done - coordination / E&O report, required-sections, article-role, submittal register (#239, #241, #269, #273, #277, #282, #283, #284) — new "Coordination and Semantics" section; removed "coordination report" from planned Phase 4 - onboarding APIs (#243, #247, #248, #249, #272) — API done; UI remains planned - soft-delete/withdraw (#257, #313), section-number format (#266, #271), external-content associations (#242), structural numbering profiles (#317) Kept as planned (foundation only): header/footer composition (#222, #314) and keynote surfacing (#315) — DB/AST exist, no resolution/render/export yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(architecture): reflect merged structural changes Update the architecture spec for shipped work, validated against the diffs and current schema/routes: - Tech Stack + Data Flow: Parse — PDF text-layer (unpdf/pdfjs-dist) + OCR (tesseract.js/@napi-rs/canvas) path and numberingProfileId override (#287, #290, #311, #317; ADR-034, ADR-039) - DB schema — specs.onboarding_status/withdrawn_at, projects.section_number_format /deleted_at/deleted_by, paragraphs.source_facts/classification/ editability_override; "Additional tables" summary for editing_conventions, paragraph_associations, required_sections, keynotes, header_footer_configs, numbering_profiles, revision_nomenclature_profiles (foundation-only tables flagged) (ADR-021/022/023/028/031/032; #187, #242) - new Coordination Report / E&O section (finding vocabulary) and Document Concurrency section (locks/optimistic/lifecycle) (#197, #241, #269, #277, #282, #283, #284; ADR-018, ADR-033/035/036/037) - AST meta.articleRole (#273, ADR-033); API-surface note pointing at the CI-enforced openapi.yaml + GET /docs; refreshed MCP tool list Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
MCP parity for the onboarding loop (#140, Wave 4 / O-13) so an AI agent can drive the whole flow over
POST /mcp— import a master, inspect what the classifier decided and why, correct it, and re-run after a convention change — without falling back to REST. Closes #140 (blockers #135/#136/#137/#139 all merged).What
Five new MCP tools, each a thin adapter over the same
db/index.jsquery its REST counterpart calls, so the two surfaces cannot drift:review_editability— per-paragraph effective editability (value,confidence,evidence, and a humanoverridewhen set) via the sharedgetSpecTreequery; optionalmaxConfidencefilter returns only the low-confidence review queue.get_onboarding_report— spec-keyed onboarding report reusing the sharedsummarizeEditabilitybuilder plus persisted style-source state.set_editability_override/clear_editability_override— the feat(api): editability corrections + reclassify with before/after diff #136PATCH /specs/:id/paragraphs/:nodeId/editabilityas tools (sharedsetSpecEditabilityOverride/clearSpecEditabilityOverride).reclassify_spec—POST /specs/:id/reclassifyas a tool; returns the sameReclassifyReportbefore/after diff.preview: truediffs without persisting.Handlers live in
src/mcp/onboarding-handlers.ts; registration insrc/mcp/onboarding-tools.ts(split out sotools.tsstays under the 400-line cap). The pure editability summarizer moved fromsrc/api/onboarding-report.tstosrc/lib/editability-summary.ts(re-exported from the API module) so MCP shares one implementation without reaching intosrc/api.Design decisions
get_onboarding_reportspec→report mapping. The REST report (GET /libraries/import/jobs/:jobId) is job-keyed and built at import time from three inputs, two of which depend on the original uploaded bytes that are not persisted:styleDerivation(consensus audit fromanalyzeDocxStyles(buffer)) andparseWarnings(tree.warnings, absent from the DB-stored tree). Only theeditabilitysummary (pure over the persisted tree) and the style-source assignment are reconstructable from aspecId. So the spec-keyed tool reuses the exactsummarizeEditabilitybuilder REST uses (single source) plusgetSpecStyleSource/getOnboardingStatus, and explicitly marks the two import-time-only sections as unavailable in anotefield rather than fabricating them. The pure summarizer was extracted tosrc/lib/to keep the MCP module off thesrc/apiboundary.review_editabilityevidence/confidence comes fromgetSpecTree'sderiveEditability— byte-identical to REST andget_spec. A parity test asserts this directly.registerToolsand run under the existing transport-levelmcpRateLimitonPOST /mcp(security(mcp): rate limit POST /mcp — parse_document and generate_docx DoS hardening #69); the expensivereclassify_specpath is already throttled. No second route or limiter added.{ isError: true }(unknown spec, wrong-spec node, malformed uuid, no resolvable convention, unsafe rules). No thrown handler reaches the transport.No
openapi.yamlchange — MCP tools are not part of the REST contract; no REST endpoint was touched.Testing
pnpm test— 1012 tests across 75 files green withDATABASE_URLset)onboarding.integration.test.ts: 13 tests covering each tool's happy path +isErrorshape, the single-source parity assertion,maxConfidencefiltering, override flip/clear round-trip, andpreview)pnpm lint: eslint + tsc + prettier)docs.integration.test.tsfailures are pre-existing and environmental — ExpresssendFilerejects the dot-prefixed.worktrees/path via its default dotfile guard (proven: a baresendFileofopenapi.yamlfrom this path returns 404). They are unrelated to this PR (no docs/openapi files touched) and pass on a normal checkout path.🤖 Co-authored by Claude Opus 4.8. Closes #140.
Summary by CodeRabbit
Release Notes
New Features
POST /mcpMCP tool surface.Documentation
Refactor
Tests