feat(mcp): wave 7c — required-sections config tools - #343
Conversation
Expose the required-sections REST surface as MCP tools (ADR-044):
get_required_sections & get_package_required_sections (read),
set_required_sections & set_package_required_sections (write) — project
baseline and design-package scope. All four move MCP_UNEXPOSED → OP_TO_TOOL
with read/write tiers.
The set tools accept either an explicit `sections` list or `seedFrom`
(baseline / toc / { packageId }) — never both — reusing RequiredSectionsBody
Schema's cross-field refine (validated separately from the path ids so the
refine survives). Handlers mirror the REST applyBody + seedSourceFrom and map
the typed project/package-not-found, seed-conflict, and invalid-seed errors to
tool errors. These required sections drive the coordination_report's
required-but-absent check.
New required-sections.integration.test.ts: project + package set→get round
trips, the sections-xor-seedFrom refine, duplicate rejection, and missing
project/package errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Codex (GPT-5.5, xhigh) adversarial review — ran against |
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds MCP tool support for required sections at project and package scope. It introduces new handlers, tool registrations, capability-tier entries, contract mappings, and integration tests for read/write required-sections operations. ChangesRequired-Sections MCP Tools
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleSetRequiredSections
participant RequiredSectionsBodySchema
participant applyRequiredSections
Client->>handleSetRequiredSections: call with projectId + body
handleSetRequiredSections->>RequiredSectionsBodySchema: safeParse(args)
RequiredSectionsBodySchema-->>handleSetRequiredSections: parsed sections or seedFrom
handleSetRequiredSections->>applyRequiredSections: apply scope + request data
applyRequiredSections-->>handleSetRequiredSections: success or domain error
handleSetRequiredSections-->>Client: ok(...) or toolError(...)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/mcp/required-sections.integration.test.ts (1)
66-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
seedFromand malformed-UUID paths.The PR objectives call out
seed-conflictandinvalid-seedtyped-error mappings, and the handlers distinguish malformed UUID input (toolError('... must be a UUID')) from well-formed-but-missing UUIDs (not-found business error). This suite only tests mutual exclusion and not-found via a valid-but-absent UUID (MISSING); it never exercises a successfulseedFrom: 'baseline'/'toc'/{ packageId }set, a seed-conflict scenario, an invalid-seed value, or a syntactically invalid UUID string. These are meaningfully different code paths in the handlers (schema validation vs. business logic) and are worth covering given they're explicitly mentioned as handled behavior.🤖 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 `@src/mcp/required-sections.integration.test.ts` around lines 66 - 88, Add integration coverage in required-sections tests for the seedFrom and UUID validation branches handled by handleSetRequiredSections, handleGetRequiredSections, and handleGetPackageRequiredSections. Extend the suite to verify a successful seedFrom case for baseline/toc or a packageId seed, plus a seed-conflict typed error and an invalid-seed typed error. Also add a malformed UUID case distinct from the existing MISSING not-found check so the toolError("... must be a UUID") path is exercised separately from the business-not-found path.
🤖 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.
Inline comments:
In `@src/mcp/required-sections-handlers.ts`:
- Around line 110-114: The package-scope UUID validation in the
required-sections handlers is using a fixed `toolError(...)` message, which
hides which field failed; update the `PackageScopeArgs.safeParse` failure path
to use `issues(...)` like the other multi-field handlers. Keep the validation
logic in `get_package_required_sections` and the corresponding package-scope
handler aligned so errors report the specific field issues for `projectId` and
`packageId`.
---
Nitpick comments:
In `@src/mcp/required-sections.integration.test.ts`:
- Around line 66-88: Add integration coverage in required-sections tests for the
seedFrom and UUID validation branches handled by handleSetRequiredSections,
handleGetRequiredSections, and handleGetPackageRequiredSections. Extend the
suite to verify a successful seedFrom case for baseline/toc or a packageId seed,
plus a seed-conflict typed error and an invalid-seed typed error. Also add a
malformed UUID case distinct from the existing MISSING not-found check so the
toolError("... must be a UUID") path is exercised separately from the
business-not-found path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8012b338-31eb-4b36-8e38-452e07d389fb
📒 Files selected for processing (6)
src/mcp/capabilities.tssrc/mcp/contract-map.tssrc/mcp/required-sections-handlers.tssrc/mcp/required-sections-tools.tssrc/mcp/required-sections.integration.test.tssrc/mcp/tools.ts
CI's 80% branch-coverage gate dipped to 79.95% because the wave-7c handlers
added seed/error branches the initial suite didn't exercise (only the explicit
sections path was tested). Add coverage for the real seed behavior:
- seed an empty package from the project baseline (seedFrom: 'baseline')
- seed an empty package from another package (seedFrom: { packageId })
- reject seeding a baseline from a non-toc source (InvalidSeed — baseline may
only seed from 'toc')
- reject seeding an already-populated scope (SeedConflict)
Seeds require an empty target scope, so each seed test uses a fresh project.
Branch coverage back to 80.19%.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions (CodeRabbit) - The two package-scope handlers validate two UUIDs, so switch their fixed 'projectId and packageId must be UUIDs' message to issues(parsed.error) — consistent with every other multi-field handler (single-field handlers keep the fixed message). - Add a malformed-UUID test (schema-validation path, distinct from the valid-but-absent not-found path already covered). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CodeRabbit nitpick addressed. The seedFrom/seed-conflict/invalid-seed coverage gap was closed in e80b0b5 — |
Why
Wave 7 (config-CRUD parity, ADR-044), sub-wave 7c. Exposes the required-sections REST endpoints as MCP tools so an agent can define what sections a project/package is expected to contain — the input to the
coordination_report's required-but-absent check.What
Four tools,
MCP_UNEXPOSED→OP_TO_TOOL:get_required_sectionsGET /projects/{id}/required-sectionsset_required_sectionsPUT /projects/{id}/required-sectionsget_package_required_sectionsGET /projects/{id}/packages/{packageId}/required-sectionsset_package_required_sectionsPUT /projects/{id}/packages/{packageId}/required-sectionssettools take either an explicitsectionslist (section + optional title, no dupes) orseedFrom(baseline/toc/{ packageId }) — never both — reusingRequiredSectionsBodySchema's cross-field refine. The body is validated separately from the path ids so the refine survives (the schema strips the id keys).applyBody+seedSourceFromand map the typed errors (project/package-not-found → not-found, seed-conflict, invalid-seed) to tool errors.openapi.yamlchange (wraps existing ops).Review
Testing
required-sections.integration.test.ts(project + package set→get, sections-xor-seedFrom refine, duplicate rejection, missing project/package)🤖 Co-authored by Claude Opus 4.8. Part of the Wave 7 config-CRUD parity series (ADR-044/045).
Summary by CodeRabbit
get_*required_sectionsandset_*required_sections).