From dc2d38d673a2b614c0877becee53474827c18168 Mon Sep 17 00:00:00 2001 From: ablancorobayna Date: Fri, 24 Apr 2026 18:43:00 +0100 Subject: [PATCH 1/3] feat: add discovery sessions (DS-xxx) for structured requirements elicitation Introduce a new "discovery" artifact type with a 9-tool lifecycle for conducting stakeholder elicitation sessions, recording findings and gaps, reviewing outcomes, and iterating via follow-ups. Tools: start_discovery, record_finding, record_gap, complete_discovery, list_discoveries, get_discovery, add_discovery_review, resolve_gap, request_followup. Also adds onboarding step, health check, persona/prompt integration, and plugin prompt fragments for both generic-agile and sap-aem. --- src/core/statuses.ts | 7 + src/doctor/health/checks/index.ts | 2 + src/doctor/health/checks/no-discoveries.ts | 32 ++ src/doctor/health/onboarding.ts | 13 +- src/personas/builtin/delivery-manager.ts | 12 +- src/personas/builtin/product-owner.ts | 2 +- src/personas/builtin/tech-lead.ts | 2 +- src/personas/prompt-builder.ts | 1 + src/plugins/builtin/generic-agile.ts | 40 +- src/plugins/builtin/sap-aem.ts | 35 +- src/plugins/builtin/tools/discoveries.ts | 533 ++++++++++++++++++++ src/plugins/common.ts | 3 + test/doctor/health/engine.test.ts | 38 ++ test/doctor/health/onboarding.test.ts | 22 +- test/plugins/registry.test.ts | 11 +- test/plugins/tools/discoveries.test.ts | 537 +++++++++++++++++++++ 16 files changed, 1279 insertions(+), 11 deletions(-) create mode 100644 src/doctor/health/checks/no-discoveries.ts create mode 100644 src/plugins/builtin/tools/discoveries.ts create mode 100644 test/plugins/tools/discoveries.test.ts diff --git a/src/core/statuses.ts b/src/core/statuses.ts index 7bdba17..eb71f67 100644 --- a/src/core/statuses.ts +++ b/src/core/statuses.ts @@ -16,3 +16,10 @@ export const ACTION_STATUSES = ["open", "in-progress", "done"] as const; export const MEETING_STATUSES = ["scheduled", "completed"] as const; export const DECISION_STATUSES = ["open", "decided", "superseded"] as const; export const QUESTION_STATUSES = ["open", "answered"] as const; +export const DISCOVERY_STATUSES = [ + "draft", + "in-review", + "needs-input", + "accepted", + "parked", +] as const; diff --git a/src/doctor/health/checks/index.ts b/src/doctor/health/checks/index.ts index 46ca602..837a790 100644 --- a/src/doctor/health/checks/index.ts +++ b/src/doctor/health/checks/index.ts @@ -1,6 +1,7 @@ import type { HealthCheck } from "../types.js"; import { emptyProjectCheck } from "./empty-project.js"; import { unprocessedSourcesCheck } from "./unprocessed-sources.js"; +import { noDiscoveriesCheck } from "./no-discoveries.js"; import { noSprintsCheck } from "./no-sprints.js"; import { unassignedActionsCheck } from "./unassigned-actions.js"; import { noJiraProjectCheck } from "./no-jira-project.js"; @@ -10,6 +11,7 @@ import { phaseReadinessCheck } from "./phase-readiness.js"; export const allHealthChecks: HealthCheck[] = [ emptyProjectCheck, unprocessedSourcesCheck, + noDiscoveriesCheck, noSprintsCheck, unassignedActionsCheck, noJiraProjectCheck, diff --git a/src/doctor/health/checks/no-discoveries.ts b/src/doctor/health/checks/no-discoveries.ts new file mode 100644 index 0000000..8c7245c --- /dev/null +++ b/src/doctor/health/checks/no-discoveries.ts @@ -0,0 +1,32 @@ +import type { HealthCheck, HealthContext, HealthFinding } from "../types.js"; + +const CHECK_ID = "no-discoveries"; +const CHECK_NAME = "Missing Discovery Sessions"; + +/** Flags projects that have features but no discovery sessions. */ +export const noDiscoveriesCheck: HealthCheck = { + id: CHECK_ID, + name: CHECK_NAME, + description: "Detects projects with features but no discovery sessions to validate requirements", + + run(ctx: HealthContext): HealthFinding[] { + const counts = ctx.store.counts(); + + const featureCount = counts["feature"] ?? 0; + if (featureCount === 0) return []; + + const hasDiscoveries = (counts["discovery"] ?? 0) > 0; + if (hasDiscoveries) return []; + + return [ + { + checkId: CHECK_ID, + checkName: CHECK_NAME, + severity: "recommendation", + message: `Project has ${featureCount} feature(s) but no discovery sessions.`, + suggestion: + "Consider conducting discovery sessions with stakeholders to validate requirements and identify gaps before refinement.", + }, + ]; + }, +}; diff --git a/src/doctor/health/onboarding.ts b/src/doctor/health/onboarding.ts index ec0e89d..5e55356 100644 --- a/src/doctor/health/onboarding.ts +++ b/src/doctor/health/onboarding.ts @@ -72,7 +72,18 @@ export function buildOnboardingGuide(ctx: HealthContext): OnboardingGuide { }); } - // Step 3: Capture decisions and actions + // Step 3: Conduct discovery sessions + const hasDiscoveries = (counts["discovery"] ?? 0) > 0; + steps.push({ + order: order++, + title: "Conduct discovery sessions", + description: + "As PO or DM, start discovery sessions with functional stakeholders to validate extracted requirements, identify gaps, and refine features before committing to decisions and epics.", + tool: "start_discovery", + done: hasDiscoveries, + }); + + // Step 4: Capture decisions and actions steps.push({ order: order++, title: "Capture key decisions and actions", diff --git a/src/personas/builtin/delivery-manager.ts b/src/personas/builtin/delivery-manager.ts index 9f356b1..7e172ff 100644 --- a/src/personas/builtin/delivery-manager.ts +++ b/src/personas/builtin/delivery-manager.ts @@ -46,6 +46,16 @@ Sprint 0 ends when the team is ready to start Sprint 1 with a refined backlog an "Epic scheduling and tracking", "Sprint planning and tracking", ], - documentTypes: ["action", "decision", "meeting", "question", "feature", "epic", "task", "sprint"], + documentTypes: [ + "action", + "decision", + "meeting", + "question", + "feature", + "epic", + "task", + "sprint", + "discovery", + ], contributionTypes: ["risk-finding", "blocker-report", "dependency-update", "status-assessment"], }; diff --git a/src/personas/builtin/product-owner.ts b/src/personas/builtin/product-owner.ts index 2e6a53a..165f74e 100644 --- a/src/personas/builtin/product-owner.ts +++ b/src/personas/builtin/product-owner.ts @@ -35,7 +35,7 @@ export const productOwner: PersonaDefinition = { "Acceptance criteria", "Feature definition and prioritization", ], - documentTypes: ["decision", "question", "action", "feature", "use-case"], + documentTypes: ["decision", "question", "action", "feature", "use-case", "discovery"], contributionTypes: [ "stakeholder-feedback", "acceptance-result", diff --git a/src/personas/builtin/tech-lead.ts b/src/personas/builtin/tech-lead.ts index f4e56bf..67f427d 100644 --- a/src/personas/builtin/tech-lead.ts +++ b/src/personas/builtin/tech-lead.ts @@ -37,7 +37,7 @@ export const techLead: PersonaDefinition = { "Task creation and breakdown", "Sprint scoping and technical execution", ], - documentTypes: ["decision", "action", "question", "epic", "task", "sprint"], + documentTypes: ["decision", "action", "question", "epic", "task", "sprint", "discovery"], contributionTypes: [ "action-result", "spike-findings", diff --git a/src/personas/prompt-builder.ts b/src/personas/prompt-builder.ts index 3cd4afe..957662c 100644 --- a/src/personas/prompt-builder.ts +++ b/src/personas/prompt-builder.ts @@ -41,6 +41,7 @@ You have access to governance tools for managing project artifacts: - **Features** (F-xxx): List, get, create, and update feature definitions - **Epics** (E-xxx): List, get, create, and update implementation epics (must link to approved features) - **Sprints** (SP-xxx): List, get, create, and update time-boxed iterations with linked epics and delivery dates +- **Discoveries** (DS-xxx): Start, record findings/gaps, complete, and review stakeholder elicitation sessions - **Documents**: Search and read any project document - **Sources**: List source documents and view their processing status and derived artifacts diff --git a/src/plugins/builtin/generic-agile.ts b/src/plugins/builtin/generic-agile.ts index 87fd04e..394e430 100644 --- a/src/plugins/builtin/generic-agile.ts +++ b/src/plugins/builtin/generic-agile.ts @@ -18,6 +18,7 @@ export const genericAgilePlugin: MarvinPlugin = { "contribution", "sprint", "task", + "discovery", ], documentTypeRegistrations: [...COMMON_REGISTRATIONS], tools: (store) => [...createCommonTools(store)], @@ -49,7 +50,18 @@ export const genericAgilePlugin: MarvinPlugin = { - Available contribution types: stakeholder-feedback, acceptance-result, priority-change, market-insight. **Sprint Tools (read-only for awareness):** -- **list_sprints** / **get_sprint**: View sprints to understand delivery timelines and iteration scope.`, +- **list_sprints** / **get_sprint**: View sprints to understand delivery timelines and iteration scope. + +**Discovery Tools:** +- **start_discovery**: Start a new discovery session with a stakeholder to validate requirements and elicit gaps. +- **record_finding** / **record_gap**: Record findings and gaps during the session. Use \`spawn_question: true\` on gaps to create linked Q-xxx artifacts. +- **complete_discovery**: Finalize the session and transition to in-review. +- **list_discoveries** / **get_discovery**: Browse discovery sessions. + +**Discovery Workflow:** +- Focus on value alignment: validate that findings map to business outcomes. +- Use gaps to identify missing acceptance criteria or scope ambiguities. +- Review completed discoveries to refine features before committing to epics.`, "tech-lead": `You own epics and break approved features into implementation work. @@ -92,6 +104,16 @@ export const genericAgilePlugin: MarvinPlugin = { - Tag technical actions and decisions with \`sprint:SP-xxx\` to associate them with a sprint. - Use **generate_sprint_progress** to track technical work completion within an iteration. +**Discovery Tools (review focus):** +- **list_discoveries** / **get_discovery**: Review discovery sessions for technical feasibility. +- **add_discovery_review**: Annotate findings and gaps with technical assessments (feasibility, NFRs, architecture impact). +- **resolve_gap**: Resolve technical gaps with rationale. + +**Discovery Review Guidelines:** +- Assess findings for architectural feasibility and non-functional requirement impacts. +- Flag gaps that require spikes or proof-of-concepts before resolution. +- Annotate gap resolutions with technical rationale and links to relevant decisions. + **Sprint Planning:** - When asked to plan or propose a sprint, ALWAYS call **gather_sprint_planning_context** first. - Focus on: technical readiness of each epic, open technical questions or spikes, effort balance across the sprint, and feature coverage. @@ -154,6 +176,20 @@ export const genericAgilePlugin: MarvinPlugin = { - Track delivery dates and flag at-risk sprints. - Register past/completed sprints for historical tracking. +**Discovery Tools:** +- **start_discovery**: Start discovery sessions with functional stakeholders. Chain sessions using \`parent\` to carry forward open gaps. +- **record_finding** / **record_gap**: Capture structured findings and gaps during elicitation. Use \`spawn_question: true\` to create linked Q-xxx for gaps. +- **complete_discovery**: Finalize sessions and transition to in-review. +- **add_discovery_review**: Add review annotations. +- **resolve_gap** / **request_followup**: Resolve gaps or request follow-up input. +- **list_discoveries** / **get_discovery**: Browse and read discovery sessions. + +**Discovery Workflow:** +- Use structured elicitation techniques (interviews, workshops, walk-throughs) to validate requirements. +- Track gaps systematically — spawn Q-xxx for items that need stakeholder input. +- Ensure all gaps are resolved or parked before moving features to epics. +- Chain discovery sessions to iterate on unresolved items. + **Sprint Planning:** - When asked to plan or propose a sprint, ALWAYS call **gather_sprint_planning_context** first. It aggregates approved features, backlog epics, active sprint status, velocity from recent sprints, blockers, and summary stats in one call. - Reason through: priority (critical/high features first), capacity (compare backlog effort to velocity reference), dependencies and blockers, balance across features, and risk. @@ -175,6 +211,8 @@ export const genericAgilePlugin: MarvinPlugin = { - **update_meeting**: Update meeting status or notes. - **analyze_meeting**: Analyze a meeting to extract decisions, actions, and questions as governance artifacts. +**Discoveries** (DS-xxx): Stakeholder elicitation sessions that validate requirements and identify gaps. Status: draft -> in-review -> needs-input -> accepted | parked. + **Contributions** (C-xxx): Structured inputs from personas outside of meetings (e.g. action results, risk findings, stakeholder feedback). Contributions are analyzed to produce governance effects. - **list_contributions** / **get_contribution**: Browse and read contribution records. - **create_contribution**: Record a contribution with persona, type, and optional related artifact. diff --git a/src/plugins/builtin/sap-aem.ts b/src/plugins/builtin/sap-aem.ts index 1fdac0a..b6ae208 100644 --- a/src/plugins/builtin/sap-aem.ts +++ b/src/plugins/builtin/sap-aem.ts @@ -23,6 +23,7 @@ export const sapAemPlugin: MarvinPlugin = { "use-case", "tech-assessment", "extension-design", + "discovery", ], documentTypeRegistrations: [ ...COMMON_REGISTRATIONS, @@ -62,7 +63,18 @@ export const sapAemPlugin: MarvinPlugin = { - Assess and approve use cases before they move to technology assessment. - Do NOT create tech assessments or extension designs — those are the Tech Lead's responsibility. - Use priorities (critical, high, medium, low) to communicate business value. -- Tag use cases with relevant business processes for traceability.`, +- Tag use cases with relevant business processes for traceability. + +**Discovery Tools:** +- **start_discovery**: Start discovery sessions with business stakeholders to validate extension use cases and elicit requirements. +- **record_finding** / **record_gap**: Capture findings about extension needs and gaps in business process understanding. +- **complete_discovery**: Finalize sessions and transition to in-review. +- **list_discoveries** / **get_discovery**: Browse discovery sessions. + +**Discovery Workflow for AEM:** +- Focus on validating extension use cases with business process owners. +- Use gaps to identify missing business scenarios or unclear extension requirements. +- Review discoveries to refine use cases before technology assessment.`, "tech-lead": `You are the Solution Architect in the SAP Application Extension Methodology (AEM). @@ -93,7 +105,12 @@ export const sapAemPlugin: MarvinPlugin = { - Only create tech assessments for assessed/approved use cases — the system enforces this. - Only create extension designs for recommended tech assessments — the system enforces this. - Document BTP services (e.g., SAP Build Work Zone, SAP Event Mesh, SAP Integration Suite) in assessments. -- Use epics to break extension designs into implementation work packages.`, +- Use epics to break extension designs into implementation work packages. + +**Discovery Tools (review focus):** +- **list_discoveries** / **get_discovery**: Review discovery sessions for technical feasibility. +- **add_discovery_review**: Annotate findings with BTP technology assessments and extension point feasibility. +- **resolve_gap**: Resolve technical gaps with rationale on BTP service capabilities.`, "delivery-manager": `You are the Project Manager in the SAP Application Extension Methodology (AEM). @@ -124,6 +141,18 @@ export const sapAemPlugin: MarvinPlugin = { - Generate tech readiness reports to identify BTP service gaps. - Track risks via actions and questions. Flag unresolved items before phase gates. +**Discovery Tools:** +- **start_discovery**: Start discovery sessions with business stakeholders to validate extension scenarios. +- **record_finding** / **record_gap**: Capture findings and gaps during elicitation. Use \`spawn_question: true\` to create linked Q-xxx for gaps. +- **complete_discovery**: Finalize sessions and transition to in-review. +- **add_discovery_review** / **resolve_gap** / **request_followup**: Review and resolve discovery outcomes. +- **list_discoveries** / **get_discovery**: Browse discovery sessions. + +**Discovery Workflow for AEM:** +- Conduct discovery sessions before each phase gate to validate readiness. +- Track gaps that block phase transitions and escalate via actions. +- Chain sessions to iterate on unresolved extension requirements. + **Sprint 0 for AEM Projects:** When setting up Sprint 0, also include AEM-specific bootstrapping: - **Phase gate preparation**: Define soft gate checklists with readiness criteria for each AEM phase transition. @@ -147,6 +176,8 @@ When setting up Sprint 0, also include AEM-specific bootstrapping: - **Meetings**: Meeting records. **Reports** (R-xxx): Persisted project reports. - Core governance: **Decisions** (D-xxx), **Actions** (A-xxx), **Questions** (Q-xxx). +**Discoveries** (DS-xxx): Stakeholder elicitation sessions that validate extension use cases and identify gaps. Status: draft -> in-review -> needs-input -> accepted | parked. + **Key Workflow:** Use cases → Tech assessments → Extension designs. Each level links to the previous. The system enforces that linked artifacts must be in the right status.`, }, }; diff --git a/src/plugins/builtin/tools/discoveries.ts b/src/plugins/builtin/tools/discoveries.ts new file mode 100644 index 0000000..756cf55 --- /dev/null +++ b/src/plugins/builtin/tools/discoveries.ts @@ -0,0 +1,533 @@ +import { z } from "zod/v4"; +import { tool, type SdkMcpToolDefinition } from "@anthropic-ai/claude-agent-sdk"; +import type { DocumentStore } from "../../../storage/store.js"; +import { ownerSchema, normalizeOwner } from "../../../personas/owner.js"; +import { DISCOVERY_STATUSES } from "../../../core/statuses.js"; + +/** Count occurrences of a heading pattern like `### F-N:` or `### GAP-N:` in content. */ +function countBlocks(content: string, prefix: string): number { + const re = new RegExp(`^### ${prefix}-(\\d+):`, "gm"); + let count = 0; + while (re.exec(content)) count++; + return count; +} + +export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition[] { + return [ + // ----------------------------------------------------------------------- + // 1. start_discovery + // ----------------------------------------------------------------------- + tool( + "start_discovery", + "Start a new discovery session. Optionally chain from a parent session to carry forward open gaps and auto-increment the session number. Tags can load prior context from related artifacts.", + { + title: z.string().describe("Discovery session title"), + content: z.string().describe("Initial session content / agenda"), + stakeholder: z.string().describe("Name or role of the functional stakeholder"), + parent: z + .string() + .optional() + .describe( + "Parent discovery ID (e.g. 'DS-001') to chain from — carries forward open gaps", + ), + tags: z + .array(z.string()) + .optional() + .describe("Tags for categorization and context loading"), + owner: ownerSchema.optional().describe("Persona role responsible (po, dm, tl)"), + }, + async (args) => { + let session = 1; + const contentParts: string[] = []; + + // If parent provided, carry forward open gaps and increment session + if (args.parent) { + const parentDoc = store.get(args.parent); + if (!parentDoc) { + return { + content: [ + { type: "text" as const, text: `Parent discovery ${args.parent} not found` }, + ], + isError: true, + }; + } + session = ((parentDoc.frontmatter.session as number) ?? 1) + 1; + + // Extract open gaps from parent content + const openGaps: string[] = []; + const lines = parentDoc.content.split("\n"); + let currentGap: string[] = []; + let inGap = false; + for (const line of lines) { + if (/^### GAP-\d+:/.test(line)) { + if (inGap && currentGap.length > 0) { + const block = currentGap.join("\n"); + if (block.includes("**Status:** open")) openGaps.push(block); + } + currentGap = [line]; + inGap = true; + } else if (inGap) { + if (/^### /.test(line) && !/^### GAP-\d+:/.test(line)) { + const block = currentGap.join("\n"); + if (block.includes("**Status:** open")) openGaps.push(block); + inGap = false; + currentGap = []; + } else { + currentGap.push(line); + } + } + } + if (inGap && currentGap.length > 0) { + const block = currentGap.join("\n"); + if (block.includes("**Status:** open")) openGaps.push(block); + } + + if (openGaps.length > 0) { + contentParts.push(`## Open Gaps from ${args.parent}\n\n${openGaps.join("\n\n")}`); + } + } + + // Auto-load context from tagged artifacts + if (args.tags && args.tags.length > 0) { + const contextItems: string[] = []; + for (const tag of args.tags) { + const docs = store.list({ tag }); + for (const d of docs) { + contextItems.push( + `- **${d.frontmatter.id}** (${d.frontmatter.type}): ${d.frontmatter.title}`, + ); + } + } + if (contextItems.length > 0) { + contentParts.push(`## Prior Context\n\n${contextItems.join("\n")}`); + } + } + + contentParts.push(args.content); + + const frontmatter: Record = { + title: args.title, + status: "draft", + stakeholder: args.stakeholder, + session, + tags: args.tags ?? [], + }; + if (args.owner) frontmatter.owner = normalizeOwner(args.owner); + if (args.parent) frontmatter.parent = args.parent; + + const doc = store.create("discovery", frontmatter as any, contentParts.join("\n\n")); + return { + content: [ + { + type: "text" as const, + text: `Created discovery ${doc.frontmatter.id}: ${doc.frontmatter.title} (session ${session})`, + }, + ], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 2. record_finding + // ----------------------------------------------------------------------- + tool( + "record_finding", + "Append a structured finding to a discovery session", + { + id: z.string().describe("Discovery ID (e.g. 'DS-001')"), + finding: z.string().describe("Finding title / summary"), + source: z + .string() + .describe("Where this finding came from (stakeholder quote, document, etc.)"), + impacts: z + .string() + .describe("What this finding impacts (features, architecture, scope, etc.)"), + confidence: z.enum(["high", "medium", "low"]).describe("Confidence level in this finding"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + const n = countBlocks(doc.content, "F") + 1; + const block = [ + `### F-${n}: ${args.finding}`, + `**Source:** ${args.source}`, + `**Impacts:** ${args.impacts}`, + `**Confidence:** ${args.confidence}`, + ].join("\n"); + + store.update(args.id, {}, `${doc.content}\n\n${block}`); + return { + content: [ + { type: "text" as const, text: `Recorded F-${n} in ${args.id}: ${args.finding}` }, + ], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 3. record_gap + // ----------------------------------------------------------------------- + tool( + "record_gap", + "Append a gap (open question / missing requirement) to a discovery session. Optionally spawn a Q-xxx question artifact.", + { + id: z.string().describe("Discovery ID (e.g. 'DS-001')"), + question: z.string().describe("The gap question"), + area: z + .enum(["product", "technical"]) + .describe("Gap area — product (business/UX) or technical (architecture/infra)"), + spawn_question: z + .boolean() + .optional() + .describe("If true, also create a Q-xxx question artifact linked to this gap"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + const n = countBlocks(doc.content, "GAP") + 1; + const block = [ + `### GAP-${n}: ${args.question}`, + `**Area:** ${args.area}`, + `**Status:** open`, + ].join("\n"); + + store.update(args.id, {}, `${doc.content}\n\n${block}`); + + const parts = [`Recorded GAP-${n} in ${args.id}: ${args.question}`]; + + if (args.spawn_question) { + const qDoc = store.create( + "question", + { + title: args.question, + status: "open", + tags: [`discovery:${args.id}`], + source: `${args.id}/GAP-${n}`, + } as any, + `Gap identified during discovery session ${args.id}.\n\nArea: ${args.area}`, + ); + parts.push(`Spawned ${qDoc.frontmatter.id} with tag discovery:${args.id}`); + } + + return { + content: [{ type: "text" as const, text: parts.join("\n") }], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 4. complete_discovery + // ----------------------------------------------------------------------- + tool( + "complete_discovery", + "Complete a discovery session — validates findings/gaps and transitions to in-review", + { + id: z.string().describe("Discovery ID to complete (e.g. 'DS-001')"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + const status = doc.frontmatter.status; + if (status !== "draft" && status !== "needs-input") { + return { + content: [ + { + type: "text" as const, + text: `Cannot complete ${args.id}: status is "${status}" (must be "draft" or "needs-input")`, + }, + ], + isError: true, + }; + } + + const findingCount = countBlocks(doc.content, "F"); + const gapCount = countBlocks(doc.content, "GAP"); + + const summary = [ + `## Session Summary`, + `- **Findings:** ${findingCount}`, + `- **Gaps:** ${gapCount}`, + `- **Status:** Ready for review`, + ].join("\n"); + + store.update(args.id, { status: "in-review" }, `${doc.content}\n\n${summary}`); + return { + content: [ + { + type: "text" as const, + text: `Completed ${args.id} — ${findingCount} finding(s), ${gapCount} gap(s). Status: in-review`, + }, + ], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 5. list_discoveries + // ----------------------------------------------------------------------- + tool( + "list_discoveries", + "List discovery sessions, optionally filtered by status or stakeholder", + { + status: z.enum(DISCOVERY_STATUSES).optional().describe("Filter by status"), + stakeholder: z.string().optional().describe("Filter by stakeholder name/role"), + }, + async (args) => { + const docs = store.list({ type: "discovery", status: args.status }); + const filtered = args.stakeholder + ? docs.filter((d) => d.frontmatter.stakeholder === args.stakeholder) + : docs; + const summary = filtered.map((d) => ({ + id: d.frontmatter.id, + title: d.frontmatter.title, + status: d.frontmatter.status, + stakeholder: d.frontmatter.stakeholder, + session: d.frontmatter.session, + owner: d.frontmatter.owner, + parent: d.frontmatter.parent, + tags: d.frontmatter.tags, + })); + return { + content: [{ type: "text" as const, text: JSON.stringify(summary, null, 2) }], + }; + }, + { annotations: { readOnlyHint: true } }, + ), + + // ----------------------------------------------------------------------- + // 6. get_discovery + // ----------------------------------------------------------------------- + tool( + "get_discovery", + "Get the full content of a specific discovery session by ID", + { id: z.string().describe("Discovery ID (e.g. 'DS-001')") }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + return { + content: [ + { + type: "text" as const, + text: JSON.stringify({ ...doc.frontmatter, content: doc.content }, null, 2), + }, + ], + }; + }, + { annotations: { readOnlyHint: true } }, + ), + + // ----------------------------------------------------------------------- + // 7. add_discovery_review + // ----------------------------------------------------------------------- + tool( + "add_discovery_review", + "Add a review annotation to a discovery session that is in-review", + { + id: z.string().describe("Discovery ID (e.g. 'DS-001')"), + reviewer: z.string().describe("Reviewer name or role"), + target: z.string().describe("What is being reviewed (e.g. 'F-1', 'GAP-2', 'overall')"), + comment: z.string().describe("Review comment"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + if (doc.frontmatter.status !== "in-review") { + return { + content: [ + { + type: "text" as const, + text: `Cannot add review to ${args.id}: status is "${doc.frontmatter.status}" (must be "in-review")`, + }, + ], + isError: true, + }; + } + + const block = `### Review [${args.reviewer}] on ${args.target}\n${args.comment}`; + store.update(args.id, {}, `${doc.content}\n\n${block}`); + return { + content: [ + { + type: "text" as const, + text: `Added review by ${args.reviewer} on ${args.target} in ${args.id}`, + }, + ], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 8. resolve_gap + // ----------------------------------------------------------------------- + tool( + "resolve_gap", + "Resolve an open gap in a discovery session and optionally update the spawned question", + { + id: z.string().describe("Discovery ID (e.g. 'DS-001')"), + gap_number: z.number().describe("Gap number to resolve (e.g. 1 for GAP-1)"), + rationale: z.string().describe("Resolution rationale"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + const gapHeading = `### GAP-${args.gap_number}:`; + if (!doc.content.includes(gapHeading)) { + return { + content: [ + { + type: "text" as const, + text: `GAP-${args.gap_number} not found in ${args.id}`, + }, + ], + isError: true, + }; + } + + const newContent = doc.content.replace( + new RegExp(`(### GAP-${args.gap_number}:[^]*?)\\*\\*Status:\\*\\* open`), + `$1**Status:** resolved\n**Resolution:** ${args.rationale}`, + ); + + store.update(args.id, {}, newContent); + + // Also update any spawned question + const parts = [`Resolved GAP-${args.gap_number} in ${args.id}`]; + const questions = store.list({ type: "question", tag: `discovery:${args.id}` }); + for (const q of questions) { + if ( + q.frontmatter.source === `${args.id}/GAP-${args.gap_number}` && + q.frontmatter.status !== "answered" + ) { + store.update(q.frontmatter.id, { status: "answered" }); + parts.push(`Updated ${q.frontmatter.id} to answered`); + } + } + + return { + content: [{ type: "text" as const, text: parts.join("\n") }], + }; + }, + ), + + // ----------------------------------------------------------------------- + // 9. request_followup + // ----------------------------------------------------------------------- + tool( + "request_followup", + "Request a follow-up on a discovery session — transitions to needs-input and lists unresolved items", + { + id: z.string().describe("Discovery ID (e.g. 'DS-001')"), + reason: z.string().describe("Reason for requesting follow-up"), + }, + async (args) => { + const doc = store.get(args.id); + if (!doc) { + return { + content: [{ type: "text" as const, text: `Discovery ${args.id} not found` }], + isError: true, + }; + } + + if (doc.frontmatter.status !== "in-review") { + return { + content: [ + { + type: "text" as const, + text: `Cannot request follow-up on ${args.id}: status is "${doc.frontmatter.status}" (must be "in-review")`, + }, + ], + isError: true, + }; + } + + // Collect unresolved gaps + const unresolvedGaps: string[] = []; + const lines = doc.content.split("\n"); + let currentGapTitle = ""; + let inGap = false; + let blockLines: string[] = []; + for (const line of lines) { + if (/^### GAP-\d+:/.test(line)) { + if (inGap && blockLines.join("\n").includes("**Status:** open")) { + unresolvedGaps.push(currentGapTitle); + } + currentGapTitle = line.replace(/^### /, "").trim(); + inGap = true; + blockLines = [line]; + } else if (inGap) { + if (/^### /.test(line) && !/^### GAP-\d+:/.test(line)) { + if (blockLines.join("\n").includes("**Status:** open")) { + unresolvedGaps.push(currentGapTitle); + } + inGap = false; + blockLines = []; + } else { + blockLines.push(line); + } + } + } + if (inGap && blockLines.join("\n").includes("**Status:** open")) { + unresolvedGaps.push(currentGapTitle); + } + + const followUpItems = + unresolvedGaps.length > 0 + ? unresolvedGaps.map((g) => `- ${g}`).join("\n") + : "- (none identified)"; + + const block = [ + `## Follow-up Requested`, + `**Reason:** ${args.reason}`, + ``, + `### Unresolved Items`, + followUpItems, + ].join("\n"); + + store.update(args.id, { status: "needs-input" }, `${doc.content}\n\n${block}`); + return { + content: [ + { + type: "text" as const, + text: `Requested follow-up on ${args.id}. Status: needs-input. ${unresolvedGaps.length} unresolved gap(s).`, + }, + ], + }; + }, + ), + ]; +} diff --git a/src/plugins/common.ts b/src/plugins/common.ts index dc0d564..75840e8 100644 --- a/src/plugins/common.ts +++ b/src/plugins/common.ts @@ -9,6 +9,7 @@ import { createContributionTools } from "./builtin/tools/contributions.js"; import { createSprintTools } from "./builtin/tools/sprints.js"; import { createSprintPlanningTools } from "./builtin/tools/sprint-planning.js"; import { createTaskTools } from "./builtin/tools/tasks.js"; +import { createDiscoveryTools } from "./builtin/tools/discoveries.js"; export const COMMON_REGISTRATIONS: DocumentTypeRegistration[] = [ { type: "meeting", dirName: "meetings", idPrefix: "M" }, @@ -18,6 +19,7 @@ export const COMMON_REGISTRATIONS: DocumentTypeRegistration[] = [ { type: "contribution", dirName: "contributions", idPrefix: "C" }, { type: "sprint", dirName: "sprints", idPrefix: "SP" }, { type: "task", dirName: "tasks", idPrefix: "T" }, + { type: "discovery", dirName: "discoveries", idPrefix: "DS" }, ]; export function createCommonTools(store: DocumentStore): SdkMcpToolDefinition[] { @@ -30,5 +32,6 @@ export function createCommonTools(store: DocumentStore): SdkMcpToolDefinition { expect(noJira).toBeUndefined(); }); + it("should flag no-discoveries when features exist but no discoveries", () => { + const env = setup(); + tmpDir = env.tmpDir; + + env.store.create("feature", { title: "Feature 1", status: "draft" }); + + const report = runHealthCheck(env.ctx); + + const noDiscoveries = report.findings.find((f) => f.checkId === "no-discoveries"); + expect(noDiscoveries).toBeDefined(); + expect(noDiscoveries!.severity).toBe("recommendation"); + expect(noDiscoveries!.message).toContain("1 feature(s)"); + }); + + it("should not flag no-discoveries when discoveries exist", () => { + const env = setup(); + tmpDir = env.tmpDir; + + env.store.create("feature", { title: "Feature 1", status: "draft" }); + env.store.create("discovery", { title: "Discovery 1", status: "draft" }); + + const report = runHealthCheck(env.ctx); + + const noDiscoveries = report.findings.find((f) => f.checkId === "no-discoveries"); + expect(noDiscoveries).toBeUndefined(); + }); + + it("should not flag no-discoveries when no features exist", () => { + const env = setup(); + tmpDir = env.tmpDir; + + const report = runHealthCheck(env.ctx); + + const noDiscoveries = report.findings.find((f) => f.checkId === "no-discoveries"); + expect(noDiscoveries).toBeUndefined(); + }); + it("should produce correct summary counts", () => { const env = setup(); tmpDir = env.tmpDir; diff --git a/test/doctor/health/onboarding.test.ts b/test/doctor/health/onboarding.test.ts index 0b57db6..4ef53bb 100644 --- a/test/doctor/health/onboarding.test.ts +++ b/test/doctor/health/onboarding.test.ts @@ -31,6 +31,7 @@ function setup(options?: { methodology?: string; jiraProjectKey?: string; aemPha { type: "epic", dirName: "epics", idPrefix: "E" }, { type: "sprint", dirName: "sprints", idPrefix: "S" }, { type: "use-case", dirName: "use-cases", idPrefix: "UC" }, + { type: "discovery", dirName: "discoveries", idPrefix: "DS" }, ]; const store = new DocumentStore(marvinDir, registrations); @@ -47,7 +48,7 @@ describe("Onboarding Guide", () => { if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it("should return empty status for a blank project with full 7-step checklist", () => { + it("should return empty status for a blank project with full 8-step checklist", () => { const env = setup(); tmpDir = env.tmpDir; @@ -55,12 +56,13 @@ describe("Onboarding Guide", () => { expect(guide.status).toBe("empty"); expect(guide.projectName).toBe("test-project"); - expect(guide.steps).toHaveLength(7); + expect(guide.steps).toHaveLength(8); const titles = guide.steps.map((s) => s.title); expect(titles).toEqual([ "Ingest source documents", "Define features", + "Conduct discovery sessions", "Capture key decisions and actions", "Break work into epics", "Set up Sprint 0", @@ -74,7 +76,7 @@ describe("Onboarding Guide", () => { } // Orders should be sequential - expect(guide.steps.map((s) => s.order)).toEqual([1, 2, 3, 4, 5, 6, 7]); + expect(guide.steps.map((s) => s.order)).toEqual([1, 2, 3, 4, 5, 6, 7, 8]); // Source step should prompt user to add files expect(guide.steps[0].description).toContain("No source files found"); @@ -118,6 +120,19 @@ describe("Onboarding Guide", () => { expect(scopeStep!.tool).toBe("create_feature"); }); + it("should mark discovery step as done when discoveries exist", () => { + const env = setup(); + tmpDir = env.tmpDir; + + env.store.create("discovery", { title: "Discovery 1", status: "draft" }); + + const guide = buildOnboardingGuide(env.ctx); + + const discoveryStep = guide.steps.find((s) => s.title === "Conduct discovery sessions"); + expect(discoveryStep).toBeDefined(); + expect(discoveryStep!.done).toBe(true); + }); + it("should mark Sprint 0 step as done when sprints exist", () => { const env = setup(); tmpDir = env.tmpDir; @@ -154,6 +169,7 @@ describe("Onboarding Guide", () => { env.store.create("decision", { title: "Use React" }); env.store.create("action", { title: "Set up CI/CD" }); env.store.create("feature", { title: "Feature 1", status: "approved" }); + env.store.create("discovery", { title: "Discovery 1", status: "draft" }); env.store.create("epic", { title: "Epic 1" }); env.store.create("sprint", { title: "Sprint 0", status: "active" }); diff --git a/test/plugins/registry.test.ts b/test/plugins/registry.test.ts index deeaac3..a4a2a24 100644 --- a/test/plugins/registry.test.ts +++ b/test/plugins/registry.test.ts @@ -88,7 +88,16 @@ describe("getPluginTools", () => { expect(toolNames).toContain("get_task"); expect(toolNames).toContain("create_task"); expect(toolNames).toContain("update_task"); - expect(tools).toHaveLength(35); + expect(toolNames).toContain("start_discovery"); + expect(toolNames).toContain("record_finding"); + expect(toolNames).toContain("record_gap"); + expect(toolNames).toContain("complete_discovery"); + expect(toolNames).toContain("list_discoveries"); + expect(toolNames).toContain("get_discovery"); + expect(toolNames).toContain("add_discovery_review"); + expect(toolNames).toContain("resolve_gap"); + expect(toolNames).toContain("request_followup"); + expect(tools).toHaveLength(44); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); } diff --git a/test/plugins/tools/discoveries.test.ts b/test/plugins/tools/discoveries.test.ts new file mode 100644 index 0000000..4890eb0 --- /dev/null +++ b/test/plugins/tools/discoveries.test.ts @@ -0,0 +1,537 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import * as os from "node:os"; +import { DocumentStore } from "../../../src/storage/store.js"; +import { createDiscoveryTools } from "../../../src/plugins/builtin/tools/discoveries.js"; +import { createFeatureTools } from "../../../src/plugins/builtin/tools/features.js"; +import { COMMON_REGISTRATIONS } from "../../../src/plugins/common.js"; + +describe("Discovery Tools", () => { + let tmpDir: string; + let store: DocumentStore; + let tools: Record Promise>; + let featureTools: Record Promise>; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "marvin-test-")); + const marvinDir = path.join(tmpDir, ".marvin"); + for (const dir of [ + "decisions", + "actions", + "questions", + "meetings", + "reports", + "features", + "epics", + "contributions", + "sprints", + "tasks", + "discoveries", + ]) { + fs.mkdirSync(path.join(marvinDir, "docs", dir), { recursive: true }); + } + store = new DocumentStore(marvinDir, COMMON_REGISTRATIONS); + + tools = {}; + for (const t of createDiscoveryTools(store)) { + tools[t.name] = (t as any).handler; + } + featureTools = {}; + for (const t of createFeatureTools(store)) { + featureTools[t.name] = (t as any).handler; + } + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + // ------------------------------------------------------------------------- + // start_discovery + // ------------------------------------------------------------------------- + describe("start_discovery", () => { + it("should create DS-001 with default draft status and frontmatter fields", async () => { + const result = await tools.start_discovery({ + title: "Requirements Review", + content: "Discuss payment flow", + stakeholder: "Finance Team", + }); + expect(result.content[0].text).toContain("DS-001"); + expect(result.content[0].text).toContain("session 1"); + + const doc = store.get("DS-001"); + expect(doc).toBeDefined(); + expect(doc!.frontmatter.type).toBe("discovery"); + expect(doc!.frontmatter.status).toBe("draft"); + expect(doc!.frontmatter.stakeholder).toBe("Finance Team"); + expect(doc!.frontmatter.session).toBe(1); + }); + + it("should carry forward open gaps from parent and set session to N+1", async () => { + // Create parent with gaps + await tools.start_discovery({ + title: "Session 1", + content: "Initial session", + stakeholder: "Product Team", + }); + await tools.record_gap({ + id: "DS-001", + question: "What about edge cases?", + area: "product", + }); + await tools.record_gap({ + id: "DS-001", + question: "Performance requirements?", + area: "technical", + }); + // Resolve one gap + await tools.complete_discovery({ id: "DS-001" }); + // We need the DS to be in a state with open gaps - let's work with content directly + // The gaps are still in content as open since we didn't resolve them + + const result = await tools.start_discovery({ + title: "Session 2", + content: "Follow-up session", + stakeholder: "Product Team", + parent: "DS-001", + }); + + expect(result.content[0].text).toContain("DS-002"); + expect(result.content[0].text).toContain("session 2"); + + const doc = store.get("DS-002"); + expect(doc!.frontmatter.session).toBe(2); + expect(doc!.frontmatter.parent).toBe("DS-001"); + // Open gaps should be carried forward + expect(doc!.content).toContain("Open Gaps from DS-001"); + expect(doc!.content).toContain("What about edge cases?"); + }); + + it("should auto-load context from tagged features into content", async () => { + await featureTools.create_feature({ + title: "Payment Flow", + content: "Handle payments", + tags: ["payments"], + }); + + const result = await tools.start_discovery({ + title: "Payment Discovery", + content: "Discuss payment requirements", + stakeholder: "Finance", + tags: ["payments"], + }); + + expect(result.content[0].text).toContain("DS-001"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("Prior Context"); + expect(doc!.content).toContain("F-001"); + expect(doc!.content).toContain("Payment Flow"); + }); + + it("should return error when parent not found", async () => { + const result = await tools.start_discovery({ + title: "Follow-up", + content: "Content", + stakeholder: "Team", + parent: "DS-999", + }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("not found"); + }); + }); + + // ------------------------------------------------------------------------- + // record_finding + // ------------------------------------------------------------------------- + describe("record_finding", () => { + it("should append F-1 block with source/impacts/confidence", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.record_finding({ + id: "DS-001", + finding: "Users need SSO", + source: "Stakeholder interview", + impacts: "Authentication module", + confidence: "high", + }); + + expect(result.content[0].text).toContain("F-1"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("### F-1: Users need SSO"); + expect(doc!.content).toContain("**Source:** Stakeholder interview"); + expect(doc!.content).toContain("**Impacts:** Authentication module"); + expect(doc!.content).toContain("**Confidence:** high"); + }); + + it("should increment to F-2 on second call", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_finding({ + id: "DS-001", + finding: "Finding one", + source: "Interview", + impacts: "Scope", + confidence: "high", + }); + const result = await tools.record_finding({ + id: "DS-001", + finding: "Finding two", + source: "Document review", + impacts: "Architecture", + confidence: "medium", + }); + + expect(result.content[0].text).toContain("F-2"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("### F-1: Finding one"); + expect(doc!.content).toContain("### F-2: Finding two"); + }); + + it("should return error for non-existent discovery", async () => { + const result = await tools.record_finding({ + id: "DS-999", + finding: "Test", + source: "Test", + impacts: "Test", + confidence: "high", + }); + expect(result.isError).toBe(true); + }); + }); + + // ------------------------------------------------------------------------- + // record_gap + // ------------------------------------------------------------------------- + describe("record_gap", () => { + it("should append GAP-1 with area and question", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.record_gap({ + id: "DS-001", + question: "What about mobile?", + area: "product", + }); + + expect(result.content[0].text).toContain("GAP-1"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("### GAP-1: What about mobile?"); + expect(doc!.content).toContain("**Area:** product"); + expect(doc!.content).toContain("**Status:** open"); + }); + + it("should spawn Q-xxx with discovery tag when spawn_question is true", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.record_gap({ + id: "DS-001", + question: "What is the SLA?", + area: "technical", + spawn_question: true, + }); + + expect(result.content[0].text).toContain("GAP-1"); + expect(result.content[0].text).toContain("Q-001"); + expect(result.content[0].text).toContain("discovery:DS-001"); + + const q = store.get("Q-001"); + expect(q).toBeDefined(); + expect(q!.frontmatter.status).toBe("open"); + expect(q!.frontmatter.tags).toContain("discovery:DS-001"); + expect(q!.frontmatter.source).toBe("DS-001/GAP-1"); + }); + }); + + // ------------------------------------------------------------------------- + // complete_discovery + // ------------------------------------------------------------------------- + describe("complete_discovery", () => { + it("should transition to in-review and append summary", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_finding({ + id: "DS-001", + finding: "Finding 1", + source: "Interview", + impacts: "Scope", + confidence: "high", + }); + await tools.record_gap({ + id: "DS-001", + question: "Open question", + area: "product", + }); + + const result = await tools.complete_discovery({ id: "DS-001" }); + + expect(result.content[0].text).toContain("1 finding(s)"); + expect(result.content[0].text).toContain("1 gap(s)"); + expect(result.content[0].text).toContain("in-review"); + + const doc = store.get("DS-001"); + expect(doc!.frontmatter.status).toBe("in-review"); + expect(doc!.content).toContain("## Session Summary"); + expect(doc!.content).toContain("**Findings:** 1"); + expect(doc!.content).toContain("**Gaps:** 1"); + }); + + it("should error on wrong status (already in-review)", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.complete_discovery({ id: "DS-001" }); + + const result = await tools.complete_discovery({ id: "DS-001" }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("in-review"); + }); + }); + + // ------------------------------------------------------------------------- + // list_discoveries + // ------------------------------------------------------------------------- + describe("list_discoveries", () => { + beforeEach(async () => { + await tools.start_discovery({ + title: "Session A", + content: "A", + stakeholder: "Finance", + }); + await tools.start_discovery({ + title: "Session B", + content: "B", + stakeholder: "HR", + }); + await tools.complete_discovery({ id: "DS-001" }); + }); + + it("should return all discoveries", async () => { + const result = await tools.list_discoveries({}); + const list = JSON.parse(result.content[0].text); + expect(list).toHaveLength(2); + }); + + it("should filter by status", async () => { + const result = await tools.list_discoveries({ status: "draft" }); + const list = JSON.parse(result.content[0].text); + expect(list).toHaveLength(1); + expect(list[0].title).toBe("Session B"); + }); + + it("should filter by stakeholder", async () => { + const result = await tools.list_discoveries({ stakeholder: "HR" }); + const list = JSON.parse(result.content[0].text); + expect(list).toHaveLength(1); + expect(list[0].stakeholder).toBe("HR"); + }); + }); + + // ------------------------------------------------------------------------- + // get_discovery + // ------------------------------------------------------------------------- + describe("get_discovery", () => { + it("should return full content", async () => { + await tools.start_discovery({ + title: "Session", + content: "Full content here", + stakeholder: "Team", + }); + + const result = await tools.get_discovery({ id: "DS-001" }); + const data = JSON.parse(result.content[0].text); + expect(data.title).toBe("Session"); + expect(data.content).toContain("Full content here"); + }); + + it("should return error for non-existent", async () => { + const result = await tools.get_discovery({ id: "DS-999" }); + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("not found"); + }); + }); + + // ------------------------------------------------------------------------- + // add_discovery_review + // ------------------------------------------------------------------------- + describe("add_discovery_review", () => { + it("should append review annotation with reviewer and target", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.complete_discovery({ id: "DS-001" }); + + const result = await tools.add_discovery_review({ + id: "DS-001", + reviewer: "TL", + target: "F-1", + comment: "Looks feasible", + }); + + expect(result.content[0].text).toContain("Added review by TL"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("### Review [TL] on F-1"); + expect(doc!.content).toContain("Looks feasible"); + }); + + it("should error when not in-review", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.add_discovery_review({ + id: "DS-001", + reviewer: "TL", + target: "F-1", + comment: "Comment", + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("draft"); + }); + }); + + // ------------------------------------------------------------------------- + // resolve_gap + // ------------------------------------------------------------------------- + describe("resolve_gap", () => { + it("should change gap status to resolved and append rationale", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_gap({ + id: "DS-001", + question: "What about auth?", + area: "technical", + }); + + const result = await tools.resolve_gap({ + id: "DS-001", + gap_number: 1, + rationale: "Will use OAuth2", + }); + + expect(result.content[0].text).toContain("Resolved GAP-1"); + const doc = store.get("DS-001"); + expect(doc!.content).toContain("**Status:** resolved"); + expect(doc!.content).toContain("**Resolution:** Will use OAuth2"); + expect(doc!.content).not.toContain("**Status:** open"); + }); + + it("should also update spawned Q-xxx to answered", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_gap({ + id: "DS-001", + question: "What about auth?", + area: "technical", + spawn_question: true, + }); + + const result = await tools.resolve_gap({ + id: "DS-001", + gap_number: 1, + rationale: "OAuth2", + }); + + expect(result.content[0].text).toContain("Updated Q-001 to answered"); + const q = store.get("Q-001"); + expect(q!.frontmatter.status).toBe("answered"); + }); + + it("should return error for non-existent gap", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.resolve_gap({ + id: "DS-001", + gap_number: 5, + rationale: "N/A", + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("GAP-5 not found"); + }); + }); + + // ------------------------------------------------------------------------- + // request_followup + // ------------------------------------------------------------------------- + describe("request_followup", () => { + it("should transition to needs-input and append follow-up section", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_gap({ + id: "DS-001", + question: "Open item", + area: "product", + }); + await tools.complete_discovery({ id: "DS-001" }); + + const result = await tools.request_followup({ + id: "DS-001", + reason: "Need more stakeholder input", + }); + + expect(result.content[0].text).toContain("needs-input"); + expect(result.content[0].text).toContain("1 unresolved gap(s)"); + + const doc = store.get("DS-001"); + expect(doc!.frontmatter.status).toBe("needs-input"); + expect(doc!.content).toContain("## Follow-up Requested"); + expect(doc!.content).toContain("Need more stakeholder input"); + expect(doc!.content).toContain("Open item"); + }); + + it("should error when not in-review", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + + const result = await tools.request_followup({ + id: "DS-001", + reason: "Need input", + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("draft"); + }); + }); +}); From e56c10a26d07f4999f04a6871abcf506552bce29 Mon Sep 17 00:00:00 2001 From: ablancorobayna Date: Fri, 24 Apr 2026 20:09:57 +0100 Subject: [PATCH 2/3] docs: update personas and document types references for discovery sessions Add discovery to persona document type lists and include the new DS-xxx type in the document types reference table with its statuses and fields. --- docs/guides/personas.md | 8 ++++---- docs/reference/document-types.md | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/guides/personas.md b/docs/guides/personas.md index f797b4b..94935d8 100644 --- a/docs/guides/personas.md +++ b/docs/guides/personas.md @@ -16,11 +16,11 @@ The Product Owner focuses on product vision, stakeholder needs, backlog prioriti - Make decisions about scope, priority, and trade-offs - Accept or reject work results based on acceptance criteria -**Document types:** decisions, questions, actions, features, use-cases +**Document types:** decisions, questions, actions, features, use-cases, discoveries **Contribution types:** stakeholder-feedback, acceptance-result, priority-change, market-insight -**When to use:** When you need to define what to build and why — prioritizing features, making scope decisions, capturing stakeholder requirements, or evaluating delivered work against acceptance criteria. +**When to use:** When you need to define what to build and why — prioritizing features, making scope decisions, capturing stakeholder requirements, conducting discovery sessions with stakeholders, or evaluating delivered work against acceptance criteria. ```bash marvin chat --as po @@ -40,7 +40,7 @@ The Delivery Manager focuses on project delivery, risk management, team coordina - Ensure governance processes are followed (decisions logged, actions tracked) - Facilitate meetings and ensure outcomes are captured -**Document types:** actions, decisions, meetings, questions, features, epics, tasks, sprints +**Document types:** actions, decisions, meetings, questions, features, epics, tasks, sprints, discoveries **Contribution types:** risk-finding, blocker-report, dependency-update, status-assessment @@ -66,7 +66,7 @@ The Technical Lead focuses on technical architecture, code quality, technical de - Guide the team on best practices and patterns - Evaluate technical risks and propose mitigations -**Document types:** decisions, actions, questions, epics, tasks, sprints +**Document types:** decisions, actions, questions, epics, tasks, sprints, discoveries **Contribution types:** action-result, spike-findings, technical-assessment, architecture-review diff --git a/docs/reference/document-types.md b/docs/reference/document-types.md index 896711b..cb3deb6 100644 --- a/docs/reference/document-types.md +++ b/docs/reference/document-types.md @@ -45,6 +45,7 @@ to specify exactly the fields they need... | meeting | M | meetings/ | scheduled, completed, cancelled | | report | R | reports/ | — | | contribution | C | contributions/ | — | +| discovery | DS | discoveries/ | draft, in-review, needs-input, accepted, parked | ## SAP AEM types (sap-aem methodology only) @@ -80,4 +81,4 @@ These fields are defined on `DocumentFrontmatter` in `src/storage/types.ts`. Add | `dueDate` | string | no | ISO date | | `source` | string | no | Source artifact ID | -Additional fields vary by type (e.g., sprints have `startDate`, `endDate`, `goal`, `linkedEpics`; features have `linkedEpics`; tasks have `linkedEpic`, `complexity`, `estimatedPoints`). +Additional fields vary by type (e.g., sprints have `startDate`, `endDate`, `goal`, `linkedEpics`; features have `linkedEpics`; tasks have `linkedEpic`, `complexity`, `estimatedPoints`; discoveries have `stakeholder`, `session`, `parent`). From 692fc4f44c8cca3397fb0081fa3f1a6bc414caf0 Mon Sep 17 00:00:00 2001 From: ablancorobayna Date: Fri, 24 Apr 2026 20:24:53 +0100 Subject: [PATCH 3/3] fix: address PR review findings for discovery tools - Extract shared gap-parsing logic into collectOpenGaps() helper, replacing duplicated scanners in start_discovery and request_followup - Replace existing "## Session Summary" on re-completion instead of appending a duplicate (needs-input -> in-review path) - Return explicit error when resolve_gap targets an already-resolved gap - Fix misleading test comment about complete_discovery - Renumber stale step comments in onboarding.ts (steps 5-8) - Add test for duplicate summary prevention and already-resolved gap --- src/doctor/health/onboarding.ts | 8 +- src/plugins/builtin/tools/discoveries.ts | 136 +++++++++++++---------- test/plugins/tools/discoveries.test.ts | 60 +++++++++- 3 files changed, 137 insertions(+), 67 deletions(-) diff --git a/src/doctor/health/onboarding.ts b/src/doctor/health/onboarding.ts index 5e55356..a312589 100644 --- a/src/doctor/health/onboarding.ts +++ b/src/doctor/health/onboarding.ts @@ -93,7 +93,7 @@ export function buildOnboardingGuide(ctx: HealthContext): OnboardingGuide { done: (counts["decision"] ?? 0) > 0 && hasActions, }); - // Step 4: Break down into epics + // Step 5: Break down into epics steps.push({ order: order++, title: "Break work into epics", @@ -104,7 +104,7 @@ export function buildOnboardingGuide(ctx: HealthContext): OnboardingGuide { done: hasEpics, }); - // Step 5: Set up Sprint 0 + // Step 6: Set up Sprint 0 steps.push({ order: order++, title: "Set up Sprint 0", @@ -114,7 +114,7 @@ export function buildOnboardingGuide(ctx: HealthContext): OnboardingGuide { done: hasSprints, }); - // Step 6: Configure Jira integration + // Step 7: Configure Jira integration steps.push({ order: order++, title: "Configure Jira integration", @@ -123,7 +123,7 @@ export function buildOnboardingGuide(ctx: HealthContext): OnboardingGuide { done: hasJira, }); - // Step 7: Run health check + // Step 8: Run health check steps.push({ order: order, title: "Run a health check", diff --git a/src/plugins/builtin/tools/discoveries.ts b/src/plugins/builtin/tools/discoveries.ts index 756cf55..6a9b50c 100644 --- a/src/plugins/builtin/tools/discoveries.ts +++ b/src/plugins/builtin/tools/discoveries.ts @@ -12,6 +12,44 @@ function countBlocks(content: string, prefix: string): number { return count; } +/** Parse content for GAP blocks that have `**Status:** open`. */ +function collectOpenGaps(content: string): { heading: string; block: string }[] { + const results: { heading: string; block: string }[] = []; + const lines = content.split("\n"); + let currentHeading = ""; + let blockLines: string[] = []; + let inGap = false; + + const flushGap = (): void => { + if (inGap && blockLines.length > 0) { + const block = blockLines.join("\n"); + if (block.includes("**Status:** open")) { + results.push({ heading: currentHeading, block }); + } + } + }; + + for (const line of lines) { + if (/^### GAP-\d+:/.test(line)) { + flushGap(); + currentHeading = line.replace(/^### /, "").trim(); + blockLines = [line]; + inGap = true; + } else if (inGap) { + if (/^### /.test(line) && !/^### GAP-\d+:/.test(line)) { + flushGap(); + inGap = false; + blockLines = []; + } else { + blockLines.push(line); + } + } + } + flushGap(); + + return results; +} + export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition[] { return [ // ----------------------------------------------------------------------- @@ -53,37 +91,12 @@ export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition } session = ((parentDoc.frontmatter.session as number) ?? 1) + 1; - // Extract open gaps from parent content - const openGaps: string[] = []; - const lines = parentDoc.content.split("\n"); - let currentGap: string[] = []; - let inGap = false; - for (const line of lines) { - if (/^### GAP-\d+:/.test(line)) { - if (inGap && currentGap.length > 0) { - const block = currentGap.join("\n"); - if (block.includes("**Status:** open")) openGaps.push(block); - } - currentGap = [line]; - inGap = true; - } else if (inGap) { - if (/^### /.test(line) && !/^### GAP-\d+:/.test(line)) { - const block = currentGap.join("\n"); - if (block.includes("**Status:** open")) openGaps.push(block); - inGap = false; - currentGap = []; - } else { - currentGap.push(line); - } - } - } - if (inGap && currentGap.length > 0) { - const block = currentGap.join("\n"); - if (block.includes("**Status:** open")) openGaps.push(block); - } - + // Carry forward open gaps from parent + const openGaps = collectOpenGaps(parentDoc.content); if (openGaps.length > 0) { - contentParts.push(`## Open Gaps from ${args.parent}\n\n${openGaps.join("\n\n")}`); + contentParts.push( + `## Open Gaps from ${args.parent}\n\n${openGaps.map((g) => g.block).join("\n\n")}`, + ); } } @@ -268,7 +281,22 @@ export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition `- **Status:** Ready for review`, ].join("\n"); - store.update(args.id, { status: "in-review" }, `${doc.content}\n\n${summary}`); + // Replace existing summary section or append new one + const summaryHeader = "## Session Summary"; + let newContent: string; + const summaryIdx = doc.content.indexOf(summaryHeader); + if (summaryIdx !== -1) { + // Find next ## heading after the summary (or end of content) + const afterSummary = doc.content.indexOf("\n## ", summaryIdx + summaryHeader.length); + newContent = + afterSummary !== -1 + ? doc.content.slice(0, summaryIdx) + summary + doc.content.slice(afterSummary) + : doc.content.slice(0, summaryIdx) + summary; + } else { + newContent = `${doc.content}\n\n${summary}`; + } + + store.update(args.id, { status: "in-review" }, newContent); return { content: [ { @@ -418,8 +446,23 @@ export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition }; } + const openPattern = new RegExp( + `(### GAP-${args.gap_number}:[^]*?)\\*\\*Status:\\*\\* open`, + ); + if (!openPattern.test(doc.content)) { + return { + content: [ + { + type: "text" as const, + text: `GAP-${args.gap_number} in ${args.id} is already resolved`, + }, + ], + isError: true, + }; + } + const newContent = doc.content.replace( - new RegExp(`(### GAP-${args.gap_number}:[^]*?)\\*\\*Status:\\*\\* open`), + openPattern, `$1**Status:** resolved\n**Resolution:** ${args.rationale}`, ); @@ -476,34 +519,7 @@ export function createDiscoveryTools(store: DocumentStore): SdkMcpToolDefinition } // Collect unresolved gaps - const unresolvedGaps: string[] = []; - const lines = doc.content.split("\n"); - let currentGapTitle = ""; - let inGap = false; - let blockLines: string[] = []; - for (const line of lines) { - if (/^### GAP-\d+:/.test(line)) { - if (inGap && blockLines.join("\n").includes("**Status:** open")) { - unresolvedGaps.push(currentGapTitle); - } - currentGapTitle = line.replace(/^### /, "").trim(); - inGap = true; - blockLines = [line]; - } else if (inGap) { - if (/^### /.test(line) && !/^### GAP-\d+:/.test(line)) { - if (blockLines.join("\n").includes("**Status:** open")) { - unresolvedGaps.push(currentGapTitle); - } - inGap = false; - blockLines = []; - } else { - blockLines.push(line); - } - } - } - if (inGap && blockLines.join("\n").includes("**Status:** open")) { - unresolvedGaps.push(currentGapTitle); - } + const unresolvedGaps = collectOpenGaps(doc.content).map((g) => g.heading); const followUpItems = unresolvedGaps.length > 0 diff --git a/test/plugins/tools/discoveries.test.ts b/test/plugins/tools/discoveries.test.ts index 4890eb0..ec11180 100644 --- a/test/plugins/tools/discoveries.test.ts +++ b/test/plugins/tools/discoveries.test.ts @@ -85,10 +85,8 @@ describe("Discovery Tools", () => { question: "Performance requirements?", area: "technical", }); - // Resolve one gap + // Complete transitions to in-review but gaps remain open in content await tools.complete_discovery({ id: "DS-001" }); - // We need the DS to be in a state with open gaps - let's work with content directly - // The gaps are still in content as open since we didn't resolve them const result = await tools.start_discovery({ title: "Session 2", @@ -293,6 +291,39 @@ describe("Discovery Tools", () => { expect(doc!.content).toContain("**Gaps:** 1"); }); + it("should replace existing summary when re-completing from needs-input", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_finding({ + id: "DS-001", + finding: "Finding 1", + source: "Interview", + impacts: "Scope", + confidence: "high", + }); + await tools.complete_discovery({ id: "DS-001" }); + + // Request follow-up transitions to needs-input, then re-complete + await tools.request_followup({ id: "DS-001", reason: "Need more info" }); + await tools.record_finding({ + id: "DS-001", + finding: "Finding 2", + source: "Workshop", + impacts: "Architecture", + confidence: "medium", + }); + await tools.complete_discovery({ id: "DS-001" }); + + const doc = store.get("DS-001"); + // Should have exactly one summary section, not two + const summaryCount = (doc!.content.match(/## Session Summary/g) || []).length; + expect(summaryCount).toBe(1); + expect(doc!.content).toContain("**Findings:** 2"); + }); + it("should error on wrong status (already in-review)", async () => { await tools.start_discovery({ title: "Session", @@ -468,6 +499,29 @@ describe("Discovery Tools", () => { expect(q!.frontmatter.status).toBe("answered"); }); + it("should error when gap is already resolved", async () => { + await tools.start_discovery({ + title: "Session", + content: "Agenda", + stakeholder: "Team", + }); + await tools.record_gap({ + id: "DS-001", + question: "Auth approach?", + area: "technical", + }); + await tools.resolve_gap({ id: "DS-001", gap_number: 1, rationale: "OAuth2" }); + + const result = await tools.resolve_gap({ + id: "DS-001", + gap_number: 1, + rationale: "SAML", + }); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain("already resolved"); + }); + it("should return error for non-existent gap", async () => { await tools.start_discovery({ title: "Session",