feat: add VS Code extension for BMad Method #1
Conversation
- add CodeLens actions for ready stories and terminal workflow runner - add epic and tech spec tree views with refresh/reveal commands - add parsers, tests, and Vitest config - add README, license/notice, ignore files, and package metadata
WalkthroughA complete VS Code extension for the BMad Method is introduced, providing story and tech-spec management features. The extension includes markdown parsing, tree view providers, code lenses, CLI integration, and comprehensive test coverage with mocked VS Code APIs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VSCode as VS Code
participant Extension as Extension<br/>(activate)
participant Providers as Providers<br/>(CodeLens,<br/>TreeViews)
participant Config as Config<br/>& Parsers
participant FS as Filesystem
User->>VSCode: Open Extension
VSCode->>Extension: activate()
Extension->>Providers: Register CodeLens
Extension->>Providers: Create Tree Providers
Extension->>Extension: Register Commands
Extension->>FS: Create File Watchers
User->>VSCode: Open Epic File
VSCode->>Providers: Request CodeLenses
Providers->>Config: Parse Stories
Config->>FS: Check Story Files
FS-->>Providers: File Exists?
Providers->>VSCode: Render CodeLens<br/>(Ready Stories)
User->>VSCode: Click CodeLens
VSCode->>Extension: Execute Command
Extension->>Extension: Build CLI Command
Extension->>Extension: Create Terminal
Extension->>VSCode: Run in Terminal
sequenceDiagram
actor User
participant VSCode as VS Code
participant TreeProvider as Tree Provider
participant Config as bmadConfig
participant Parser as epicsParser
participant FS as Filesystem
User->>VSCode: Activate Extension
VSCode->>TreeProvider: getChildren(root)
TreeProvider->>Config: getPlanningArtifactsPath()
Config->>FS: Read config.yaml
FS-->>Config: Artifact Path
Config-->>TreeProvider: Resolved Path
TreeProvider->>FS: findFiles(pattern)
FS-->>TreeProvider: Epic Files
loop For Each File
TreeProvider->>FS: readFile()
FS-->>TreeProvider: Content
TreeProvider->>Parser: parseEpicsFromText()
Parser-->>TreeProvider: Parsed Epics & Stories
end
TreeProvider->>VSCode: Render Tree Items<br/>(Files→Epics→Stories)
User->>TreeProvider: Click Story
TreeProvider->>VSCode: Execute revealStory Command
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 35: Replace the bare URL in the README sentence that warns about the
`codex` CLI with a Markdown link; locate the sentence mentioning "**Codex is not
supported.** The `codex` CLI..." and change the trailing plain URL
(https://github.com/openai/codex/issues/3641) to a markdown link such as
`[https://github.com/openai/codex/issues/3641](https://github.com/openai/codex/issues/3641)`
or better `[openai/codex#3641](https://github.com/openai/codex/issues/3641)` so
the URL is not bare and the line no longer triggers MD034.
In `@src/__tests__/bmadConfig.test.ts`:
- Around line 304-323: The test and parser currently allow planning_artifacts:
on its own line to capture the next line as its value; update the parsing in
getPlanningArtifactsPath to only match a value on the same line (e.g., require
non-newline characters after the colon or use a regex like
/planning_artifacts:\s*(\S.*)?$/ without the multiline capture) so an empty line
returns null, and update the test in bmadConfig.test.ts to expect null instead
of 'other_field: value'. Ensure you only modify getPlanningArtifactsPath's
regex/logic and the test assertion to reflect the corrected behavior.
In `@src/__tests__/epicsParser.test.ts`:
- Around line 510-519: The test documents a failure for CRLF inputs but the
parser should normalize line endings; update the implementation of
parseEpicsFromText to normalize CRLF ("\r\n") (and stray "\r") to "\n" before
any splitting or regex matching so patterns that rely on line-end anchors work,
and then update this test to assert the parsed epics/stories are returned (e.g.,
expect result.epics to include the Epic and its Story) instead of expecting
length 0; reference the parseEpicsFromText function to locate where to add the
normalization step and adjust the test expectation accordingly.
In `@src/__tests__/mocks/vscode.ts`:
- Around line 403-409: The mock's createTerminal implementation doesn't add the
newly created terminal object to the mock window.terminals array, so code that
calls vscode.window.terminals.find(...) can't discover created terminals; update
the createTerminal mock (the vi.fn() returned object) so when invoked it
constructs a terminal object (with name, show, sendText, dispose) and pushes
that object into the mock terminals array (terminals: [] as { name: string }[])
before returning it, ensuring subsequent calls to vscode.window.terminals will
include the created terminal for reuse logic to work.
In `@src/__tests__/storyCodeLensProvider.test.ts`:
- Around line 325-349: The test documents a false positive because the filename
matching in provideCodeLenses (in StoryCodeLensProvider /
provider.provideCodeLenses) currently uses startsWith and treats "1-10" as
matching "1-1"; change the matcher to require a boundary (e.g., compare
normalized story id segments or use a regex like ^1-1(?:[.-]|$) / explicit
split-and-compare) when checking workspace.fs.readDirectory entries so
"1-10-different-story.md" does not match story "1.1", and then update the test
expectation to assert no "Start Developing" code lens (expect codeLenses
toHaveLength(0) or that command is undefined) for this case.
In `@src/bmadConfig.ts`:
- Around line 3-25: The regex in getConfigPath currently anchors the key to
column 0 and interpolates the raw fieldName, causing misses for indented YAML
and regex injection; update the pattern to allow optional leading whitespace and
escape the fieldName before putting it into the RegExp (e.g. build an
escapedName via a small escapeRegExp helper or
replace(/[.*+?^${}()|[\]\\]/g,'\\$&') and then use something like
/^\s*${escapedName}:\s*["']?(.+?)["']?\s*$/m) so indented keys and special
characters in fieldName are matched correctly.
In `@src/cliTool.ts`:
- Line 3: SAFE_TOOL_PATTERN currently disallows parentheses which blocks valid
Windows paths like "Program Files (x86)"; update the SAFE_TOOL_PATTERN regex to
allow both '(' and ')' characters (escaped as needed) so tool paths containing
parentheses pass validation, e.g., modify the character class in the
SAFE_TOOL_PATTERN declaration to include "(" and ")" while preserving existing
allowed characters and escaping backslashes as appropriate.
In `@src/epicsParser.ts`:
- Around line 46-59: When an epic header is matched but the parsed epic number
is invalid, reset parser state so the invalid header doesn't leave currentEpic
or currentStory set and cause later stories to attach incorrectly or duplicate
epics; specifically, after handling epicMatch and before the continue in the
invalid epicNumber branch, set currentEpic = null (or undefined) and
currentStory = null (or undefined) so that EPIC_HEADER_PATTERN handling,
currentEpic, currentStory, and epics.push behave correctly for subsequent lines.
In `@src/extension.ts`:
- Around line 228-273: The executeInTerminal function currently interpolates
storyNumber directly into a shell command, allowing command injection; update
executeInTerminal (and/or buildCliCommand) to validate or sanitize storyNumber
before composing the CLI string: enforce a numeric dot-separated pattern like
/^[0-9]+(?:\.[0-9]+)*$/ (matching "1.1", "2.3.4", etc.) and reject or show an
error for invalid values, or alternatively escape any double quotes and shell
metacharacters in storyNumber (e.g., replace " with \" and strip/escape ; & | `
$) before calling buildCliCommand so the terminal command cannot be broken out
of its quoted context. Ensure the validation/sanitization occurs where
storyNumber is read in executeInTerminal and document that buildCliCommand
expects a already-sanitized storyNumber.
In `@src/storyCodeLensProvider.ts`:
- Around line 121-124: The current prefix check using
file.startsWith(storyNumberWithDashes) causes false positives (e.g., "1.1"
matching "1-10-..."); update the match logic (where variables files and
storyNumberWithDashes are used, e.g., the match constant) to require a boundary:
consider a file a match only if it equals `${storyNumberWithDashes}.md` or it
startsWith(`${storyNumberWithDashes}-`) and endsWith('.md'); replace the
existing startsWith(...) && endsWith('.md') condition with this stricter check
to prevent substring collisions.
In `@src/techSpecParser.ts`:
- Around line 53-58: The loop in the task-parsing logic uses a guard "if (i >
startIndex && /^#{1,3}\s+/.test(line)) { break; }" which prevents breaking when
a heading immediately follows the Tasks header; remove the "i > startIndex &&"
check so the condition simply tests the heading regex and breaks immediately.
Update the for loop in the task parsing function (the loop over lines with
variables lines and startIndex) to break on /^#{1,3}\s+/ regardless of i,
ensuring headings directly after the Tasks header stop task parsing.
In `@vitest.config.ts`:
- Around line 11-14: The alias entry uses new
URL('./src/__tests__/mocks/vscode.ts', import.meta.url).pathname which yields
POSIX-style paths on Windows; update the alias for the vscode key to use
fileURLToPath(new URL('./src/__tests__/mocks/vscode.ts', import.meta.url))
instead (import fileURLToPath from 'url' if not already) so the alias resolves
to a proper platform path; adjust the vitest.config.ts import list and replace
.pathname usage wherever alias.vscode is defined.
🧹 Nitpick comments (3)
.vscodeignore (1)
45-47: Consider if PNG exclusion is intentional.Excluding
*.pngwill prevent any custom icons from being packaged with the extension. This is fine if you're only using VS Code's built-in icons (like$(refresh)and$(list-tree)referenced in package.json), but keep in mind you'll need to remove this pattern if custom icons are added later.package.json (1)
53-67: Consider aligning view IDs with display names.The view ID
bmadStoriesdisplays as "BMad Epics", which creates a naming inconsistency. The ID suggests stories but the UI shows epics. This could cause confusion when debugging or referencing these views in code/documentation.Consider renaming to
bmadEpicsfor consistency, or keeping as-is if there's a semantic reason (e.g., stories are contained within epics).src/__tests__/mocks/vscode.ts (1)
104-109: Range.contains should honor character bounds.Right now it only checks line numbers, so positions outside the start/end character range can still return true. Aligning with VS Code’s semantics will reduce false positives in tests. Please verify against the VS Code API behavior.
🔎 Suggested update
- if (positionOrRange instanceof Position) { - return positionOrRange.line >= this.start.line && positionOrRange.line <= this.end.line; - } + if (positionOrRange instanceof Position) { + if (positionOrRange.line < this.start.line || positionOrRange.line > this.end.line) { + return false; + } + if (positionOrRange.line === this.start.line && positionOrRange.character < this.start.character) { + return false; + } + if (positionOrRange.line === this.end.line && positionOrRange.character > this.end.character) { + return false; + } + return true; + }
|
|
||
| ## Compatibility / Known Limitations | ||
|
|
||
| - **Codex is not supported.** The `codex` CLI has known issues and is incompatible with the extension's workflow; do not set `bmad.cliTool` to `codex`. See https://github.com/openai/codex/issues/3641 for details. |
There was a problem hiding this comment.
Replace the bare URL with a markdown link.
This avoids MD034 and improves readability.
💡 Suggested fix
-- **Codex is not supported.** The `codex` CLI has known issues and is incompatible with the extension's workflow; do not set `bmad.cliTool` to `codex`. See https://github.com/openai/codex/issues/3641 for details.
+- **Codex is not supported.** The `codex` CLI has known issues and is incompatible with the extension's workflow; do not set `bmad.cliTool` to `codex`. See [openai/codex#3641](https://github.com/openai/codex/issues/3641) for details.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Codex is not supported.** The `codex` CLI has known issues and is incompatible with the extension's workflow; do not set `bmad.cliTool` to `codex`. See https://github.com/openai/codex/issues/3641 for details. | |
| - **Codex is not supported.** The `codex` CLI has known issues and is incompatible with the extension's workflow; do not set `bmad.cliTool` to `codex`. See [openai/codex#3641](https://github.com/openai/codex/issues/3641) for details. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
35-35: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@README.md` at line 35, Replace the bare URL in the README sentence that warns
about the `codex` CLI with a Markdown link; locate the sentence mentioning
"**Codex is not supported.** The `codex` CLI..." and change the trailing plain
URL (https://github.com/openai/codex/issues/3641) to a markdown link such as
`[https://github.com/openai/codex/issues/3641](https://github.com/openai/codex/issues/3641)`
or better `[openai/codex#3641](https://github.com/openai/codex/issues/3641)` so
the URL is not bare and the line no longer triggers MD034.
| it('does not match field followed by another field on next line', async () => { | ||
| workspace.workspaceFolders = [ | ||
| { uri: Uri.file('/workspace'), name: 'workspace', index: 0 } | ||
| ]; | ||
|
|
||
| // The regex is multiline and matches planning_artifacts: followed by next line content | ||
| // This is a quirk of the current implementation | ||
| const configContent = `planning_artifacts: | ||
| other_field: value | ||
| `; | ||
| vi.mocked(workspace.fs.readFile).mockResolvedValue( | ||
| new TextEncoder().encode(configContent) | ||
| ); | ||
|
|
||
| const result = await getPlanningArtifactsPath(); | ||
|
|
||
| // Current behavior: regex with 'm' flag matches across lines in some cases | ||
| // The (.+) captures the next line - this documents actual behavior | ||
| expect(result).toBe('other_field: value'); | ||
| }); |
There was a problem hiding this comment.
Avoid treating the next line as a field value.
An empty planning_artifacts: line should not consume the next key as a value. This misconfigures artifact paths; return null instead by constraining matches to the same line.
🔧 Proposed updates (test + parser regex)
- expect(result).toBe('other_field: value');
+ expect(result).toBeNull();// src/bmadConfig.ts (example)
-const pattern = new RegExp(`^${fieldName}:\\s*["']?(.+)["']?$`, 'm');
+const pattern = new RegExp(
+ `^${fieldName}\\s*:\\s*(["']?)([^"'\\r\\n]+)\\1\\s*$`,
+ 'm'
+);🤖 Prompt for AI Agents
In `@src/__tests__/bmadConfig.test.ts` around lines 304 - 323, The test and parser
currently allow planning_artifacts: on its own line to capture the next line as
its value; update the parsing in getPlanningArtifactsPath to only match a value
on the same line (e.g., require non-newline characters after the colon or use a
regex like /planning_artifacts:\s*(\S.*)?$/ without the multiline capture) so an
empty line returns null, and update the test in bmadConfig.test.ts to expect
null instead of 'other_field: value'. Ensure you only modify
getPlanningArtifactsPath's regex/logic and the test assertion to reflect the
corrected behavior.
| it('does not parse Windows line endings (CRLF) - known limitation', () => { | ||
| // The parser splits on \n but \r remains at end of line | ||
| // This breaks regex matching since patterns end with $ which won't match before \r | ||
| // This documents the current behavior - CRLF files won't parse correctly | ||
| const text = '## Epic 1: Test\r\n\r\n### Story 1.1: Test\r\n**Status:** ready\r\n'; | ||
| const result = parseEpicsFromText(text, TEST_FILE_PATH); | ||
|
|
||
| // Current behavior: CRLF breaks parsing (the \r at line end prevents regex match) | ||
| expect(result.epics).toHaveLength(0); | ||
| }); |
There was a problem hiding this comment.
Handle CRLF line endings instead of asserting failure.
This locks in a Windows-incompatible behavior. Normalize CRLF in parseEpicsFromText and update the expectation so CRLF files parse correctly.
🔧 Proposed updates (test + parser normalization)
- // Current behavior: CRLF breaks parsing (the \r at line end prevents regex match)
- expect(result.epics).toHaveLength(0);
+ // After normalizing line endings, CRLF should parse correctly
+ expect(result.epics).toHaveLength(1);
+ expect(result.epics[0].stories).toHaveLength(1);// src/epicsParser.ts
- const lines = text.split('\n');
+ const normalizedText = text.replace(/\r\n/g, '\n').replace(/\r/g, '\n');
+ const lines = normalizedText.split('\n');🤖 Prompt for AI Agents
In `@src/__tests__/epicsParser.test.ts` around lines 510 - 519, The test documents
a failure for CRLF inputs but the parser should normalize line endings; update
the implementation of parseEpicsFromText to normalize CRLF ("\r\n") (and stray
"\r") to "\n" before any splitting or regex matching so patterns that rely on
line-end anchors work, and then update this test to assert the parsed
epics/stories are returned (e.g., expect result.epics to include the Epic and
its Story) instead of expecting length 0; reference the parseEpicsFromText
function to locate where to add the normalization step and adjust the test
expectation accordingly.
| createTerminal: vi.fn(() => ({ | ||
| name: 'Mock Terminal', | ||
| show: vi.fn(), | ||
| sendText: vi.fn(), | ||
| dispose: vi.fn() | ||
| })), | ||
| terminals: [] as { name: string }[], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the mock file to understand the full context
cat -n src/__tests__/mocks/vscode.ts | head -430 | tail -35Repository: bmad-code-org/bmad-method-vscode
Length of output: 1178
🏁 Script executed:
# Search for any references to window.terminals in the codebase
rg 'window\.terminals' --type ts --type js -A 2 -B 2Repository: bmad-code-org/bmad-method-vscode
Length of output: 1293
🏁 Script executed:
# Search for createTerminal usage in tests to see if terminal tracking matters
rg 'createTerminal' --type ts --type js -A 3 -B 2Repository: bmad-code-org/bmad-method-vscode
Length of output: 2729
🏁 Script executed:
# Check if there's any terminal disposal or cleanup logic elsewhere
rg 'dispose.*terminal|terminal.*dispose' --type ts --type js -iRepository: bmad-code-org/bmad-method-vscode
Length of output: 58
Update mock to add created terminals to window.terminals, matching VS Code behavior.
The real extension code checks vscode.window.terminals.find(t => t.name === terminalName) to reuse existing terminals. The current mock's createTerminal doesn't add terminals to the terminals array, so terminal reuse logic won't work correctly without manual test setup. Update the mock to push the terminal to window.terminals when created.
Suggested update
- createTerminal: vi.fn(() => ({
- name: 'Mock Terminal',
- show: vi.fn(),
- sendText: vi.fn(),
- dispose: vi.fn()
- })),
+ createTerminal: vi.fn(() => {
+ const terminal = {
+ name: 'Mock Terminal',
+ show: vi.fn(),
+ sendText: vi.fn(),
+ dispose: vi.fn()
+ };
+ window.terminals.push(terminal);
+ return terminal;
+ }),🤖 Prompt for AI Agents
In `@src/__tests__/mocks/vscode.ts` around lines 403 - 409, The mock's
createTerminal implementation doesn't add the newly created terminal object to
the mock window.terminals array, so code that calls
vscode.window.terminals.find(...) can't discover created terminals; update the
createTerminal mock (the vi.fn() returned object) so when invoked it constructs
a terminal object (with name, show, sendText, dispose) and pushes that object
into the mock terminals array (terminals: [] as { name: string }[]) before
returning it, ensuring subsequent calls to vscode.window.terminals will include
the created terminal for reuse logic to work.
| it('documents startsWith matching behavior - may match similar prefixes', async () => { | ||
| // NOTE: Current implementation uses startsWith() which means | ||
| // "1-10-story.md" WILL match story 1.1 (prefix "1-1") because "1-10" starts with "1-1" | ||
| // This documents the actual behavior, which may be a limitation | ||
| const content = `## Epic 1: Test Epic | ||
|
|
||
| ### Story 1.1: First Story | ||
| **Status:** ready | ||
| `; | ||
| const document = mockDoc(content, '/workspace/epics.md'); | ||
|
|
||
| const wsFolder = { uri: Uri.file('/workspace'), name: 'workspace', index: 0 }; | ||
| workspace.workspaceFolders = [wsFolder]; | ||
| vi.mocked(workspace.getWorkspaceFolder).mockReturnValue(wsFolder); | ||
| vi.mocked(getImplementationArtifactsPath).mockResolvedValue('_bmad-output/implementation'); | ||
| vi.mocked(workspace.fs.readDirectory).mockResolvedValue([ | ||
| ['1-10-different-story.md', 1], // "1-10" starts with "1-1" so this WILL match | ||
| ]); | ||
|
|
||
| const codeLenses = await provider.provideCodeLenses(document); | ||
|
|
||
| expect(codeLenses).toHaveLength(1); | ||
| // This matches because startsWith("1-1") is true for "1-10-different-story.md" | ||
| expect(codeLenses[0].command?.command).toBe('bmadMethod.developStory'); | ||
| }); |
There was a problem hiding this comment.
Prefix collisions should not count as existing story files.
This test currently asserts that 1-10 matches story 1.1. After fixing the matcher to require a boundary, update the expectation to avoid a false “Start Developing” action.
🔧 Proposed test update
- expect(codeLenses[0].command?.command).toBe('bmadMethod.developStory');
+ expect(codeLenses[0].command?.command).toBe('bmadMethod.createStory');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('documents startsWith matching behavior - may match similar prefixes', async () => { | |
| // NOTE: Current implementation uses startsWith() which means | |
| // "1-10-story.md" WILL match story 1.1 (prefix "1-1") because "1-10" starts with "1-1" | |
| // This documents the actual behavior, which may be a limitation | |
| const content = `## Epic 1: Test Epic | |
| ### Story 1.1: First Story | |
| **Status:** ready | |
| `; | |
| const document = mockDoc(content, '/workspace/epics.md'); | |
| const wsFolder = { uri: Uri.file('/workspace'), name: 'workspace', index: 0 }; | |
| workspace.workspaceFolders = [wsFolder]; | |
| vi.mocked(workspace.getWorkspaceFolder).mockReturnValue(wsFolder); | |
| vi.mocked(getImplementationArtifactsPath).mockResolvedValue('_bmad-output/implementation'); | |
| vi.mocked(workspace.fs.readDirectory).mockResolvedValue([ | |
| ['1-10-different-story.md', 1], // "1-10" starts with "1-1" so this WILL match | |
| ]); | |
| const codeLenses = await provider.provideCodeLenses(document); | |
| expect(codeLenses).toHaveLength(1); | |
| // This matches because startsWith("1-1") is true for "1-10-different-story.md" | |
| expect(codeLenses[0].command?.command).toBe('bmadMethod.developStory'); | |
| }); | |
| it('documents startsWith matching behavior - may match similar prefixes', async () => { | |
| // NOTE: Current implementation uses startsWith() which means | |
| // "1-10-story.md" WILL match story 1.1 (prefix "1-1") because "1-10" starts with "1-1" | |
| // This documents the actual behavior, which may be a limitation | |
| const content = `## Epic 1: Test Epic | |
| ### Story 1.1: First Story | |
| **Status:** ready | |
| `; | |
| const document = mockDoc(content, '/workspace/epics.md'); | |
| const wsFolder = { uri: Uri.file('/workspace'), name: 'workspace', index: 0 }; | |
| workspace.workspaceFolders = [wsFolder]; | |
| vi.mocked(workspace.getWorkspaceFolder).mockReturnValue(wsFolder); | |
| vi.mocked(getImplementationArtifactsPath).mockResolvedValue('_bmad-output/implementation'); | |
| vi.mocked(workspace.fs.readDirectory).mockResolvedValue([ | |
| ['1-10-different-story.md', 1], // "1-10" starts with "1-1" so this WILL match | |
| ]); | |
| const codeLenses = await provider.provideCodeLenses(document); | |
| expect(codeLenses).toHaveLength(1); | |
| // This matches because startsWith("1-1") is true for "1-10-different-story.md" | |
| expect(codeLenses[0].command?.command).toBe('bmadMethod.createStory'); | |
| }); |
🤖 Prompt for AI Agents
In `@src/__tests__/storyCodeLensProvider.test.ts` around lines 325 - 349, The test
documents a false positive because the filename matching in provideCodeLenses
(in StoryCodeLensProvider / provider.provideCodeLenses) currently uses
startsWith and treats "1-10" as matching "1-1"; change the matcher to require a
boundary (e.g., compare normalized story id segments or use a regex like
^1-1(?:[.-]|$) / explicit split-and-compare) when checking
workspace.fs.readDirectory entries so "1-10-different-story.md" does not match
story "1.1", and then update the test expectation to assert no "Start
Developing" code lens (expect codeLenses toHaveLength(0) or that command is
undefined) for this case.
| // Check for Epic header | ||
| const epicMatch = line.match(EPIC_HEADER_PATTERN); | ||
| if (epicMatch) { | ||
| // Save previous epic if exists | ||
| if (currentEpic) { | ||
| epics.push(currentEpic); | ||
| } | ||
|
|
||
| // Validate epic number | ||
| const epicNumber = parseInt(epicMatch[1], 10); | ||
| if (isNaN(epicNumber) || epicNumber < 0) { | ||
| console.log(`[BMAD] Warning: Invalid epic number '${epicMatch[1]}' at line ${i + 1} in ${filePath}`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Reset currentEpic/currentStory when epic number is invalid.
After pushing the previous epic, an invalid epic header leaves currentEpic set, so later stories can attach to the wrong epic and the previous epic may be pushed twice. Clear state before continuing.
🔧 Suggested fix
- // Validate epic number
- const epicNumber = parseInt(epicMatch[1], 10);
- if (isNaN(epicNumber) || epicNumber < 0) {
- console.log(`[BMAD] Warning: Invalid epic number '${epicMatch[1]}' at line ${i + 1} in ${filePath}`);
- continue;
- }
+ // Validate epic number
+ currentEpic = null;
+ currentStory = null;
+ const epicNumber = parseInt(epicMatch[1], 10);
+ if (isNaN(epicNumber) || epicNumber < 0) {
+ console.log(`[BMAD] Warning: Invalid epic number '${epicMatch[1]}' at line ${i + 1} in ${filePath}`);
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for Epic header | |
| const epicMatch = line.match(EPIC_HEADER_PATTERN); | |
| if (epicMatch) { | |
| // Save previous epic if exists | |
| if (currentEpic) { | |
| epics.push(currentEpic); | |
| } | |
| // Validate epic number | |
| const epicNumber = parseInt(epicMatch[1], 10); | |
| if (isNaN(epicNumber) || epicNumber < 0) { | |
| console.log(`[BMAD] Warning: Invalid epic number '${epicMatch[1]}' at line ${i + 1} in ${filePath}`); | |
| continue; | |
| } | |
| // Check for Epic header | |
| const epicMatch = line.match(EPIC_HEADER_PATTERN); | |
| if (epicMatch) { | |
| // Save previous epic if exists | |
| if (currentEpic) { | |
| epics.push(currentEpic); | |
| } | |
| // Validate epic number | |
| currentEpic = null; | |
| currentStory = null; | |
| const epicNumber = parseInt(epicMatch[1], 10); | |
| if (isNaN(epicNumber) || epicNumber < 0) { | |
| console.log(`[BMAD] Warning: Invalid epic number '${epicMatch[1]}' at line ${i + 1} in ${filePath}`); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In `@src/epicsParser.ts` around lines 46 - 59, When an epic header is matched but
the parsed epic number is invalid, reset parser state so the invalid header
doesn't leave currentEpic or currentStory set and cause later stories to attach
incorrectly or duplicate epics; specifically, after handling epicMatch and
before the continue in the invalid epicNumber branch, set currentEpic = null (or
undefined) and currentStory = null (or undefined) so that EPIC_HEADER_PATTERN
handling, currentEpic, currentStory, and epics.push behave correctly for
subsequent lines.
| async function executeInTerminal(storyNumber: string | undefined, workflow: 'create-story' | 'dev-story'): Promise<void> { | ||
| const workspaceFolder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; | ||
| const config = vscode.workspace.getConfiguration('bmad'); | ||
| const cliTool = normalizeCliTool(config.get<string>('cliTool')); | ||
|
|
||
| if (!workspaceFolder) { | ||
| vscode.window.showErrorMessage( | ||
| 'No workspace folder open. Please open a folder to execute stories.' | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (!isSafeCliTool(cliTool)) { | ||
| vscode.window.showErrorMessage( | ||
| `Invalid CLI tool name "${cliTool}". Use a simple tool name or path without shell metacharacters.` | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Check for CLI availability on first invocation | ||
| if (!cliChecked) { | ||
| cliChecked = true; | ||
| cliAvailable = await checkCliAvailable(cliTool); | ||
| if (!cliAvailable) { | ||
| vscode.window.showWarningMessage( | ||
| `CLI tool "${cliTool}" not found on PATH. Update bmad.cliTool or install the tool.` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const terminalName = storyNumber ? `Story ${storyNumber}` : `BMAD ${workflow}`; | ||
|
|
||
| // Check if terminal with this name already exists | ||
| let terminal = vscode.window.terminals.find(t => t.name === terminalName); | ||
|
|
||
| if (!terminal) { | ||
| terminal = vscode.window.createTerminal({ | ||
| name: terminalName, | ||
| cwd: workspaceFolder | ||
| }); | ||
| } | ||
|
|
||
| terminal.show(); | ||
| const command = buildCliCommand(cliTool, workflow, storyNumber); | ||
| terminal.sendText(command); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find buildCliCommand implementation
rg -n "buildCliCommand\s*\(" src/extension.ts -A 10Repository: bmad-code-org/bmad-method-vscode
Length of output: 484
🏁 Script executed:
# Search for story ID/number definitions and usage patterns
rg -n "storyNumber|storyId|story-id|story ID" src/ -t ts -B 2 -A 2Repository: bmad-code-org/bmad-method-vscode
Length of output: 11422
🏁 Script executed:
# Check for any existing validation or sanitization functions
rg -n "normalize|sanitize|escape|validate" src/ -t ts | head -30Repository: bmad-code-org/bmad-method-vscode
Length of output: 2116
🏁 Script executed:
# Look at the entire extension.ts file structure to understand context
wc -l src/extension.tsRepository: bmad-code-org/bmad-method-vscode
Length of output: 96
🏁 Script executed:
# View the buildCliCommand implementation in full
sed -n '20,35p' src/cliTool.tsRepository: bmad-code-org/bmad-method-vscode
Length of output: 665
🏁 Script executed:
# Also check how the command is actually executed in context
sed -n '24,32p' src/cliTool.tsRepository: bmad-code-org/bmad-method-vscode
Length of output: 488
Sanitize storyNumber before composing the shell command.
storyNumber is inserted into a shell string without escaping. A malicious story number containing double quotes and semicolons (e.g., 1.1"; echo malicious;) could break out of the quoted context and enable command injection in the terminal.
However, the proposed regex /^[A-Za-z0-9_-]+$/ does not match the actual story number format used in the codebase. Story numbers follow a numeric.numeric pattern (e.g., "1.1", "1.2") as parsed from epic files. Update the validation to match this format, for example: /^[0-9]+(?:\.[0-9]+)*$/. Alternatively, escape double quotes in storyNumber before passing to buildCliCommand.
🤖 Prompt for AI Agents
In `@src/extension.ts` around lines 228 - 273, The executeInTerminal function
currently interpolates storyNumber directly into a shell command, allowing
command injection; update executeInTerminal (and/or buildCliCommand) to validate
or sanitize storyNumber before composing the CLI string: enforce a numeric
dot-separated pattern like /^[0-9]+(?:\.[0-9]+)*$/ (matching "1.1", "2.3.4",
etc.) and reject or show an error for invalid values, or alternatively escape
any double quotes and shell metacharacters in storyNumber (e.g., replace " with
\" and strip/escape ; & | ` $) before calling buildCliCommand so the terminal
command cannot be broken out of its quoted context. Ensure the
validation/sanitization occurs where storyNumber is read in executeInTerminal
and document that buildCliCommand expects a already-sanitized storyNumber.
| // Simple glob match: pattern is "{number}*.md", check if file starts with the number prefix | ||
| const prefix = storyNumberWithDashes; | ||
| const match = files.some(file => file.startsWith(prefix) && file.endsWith('.md')); | ||
| console.log(`[BMAD] Match result: ${match}`); |
There was a problem hiding this comment.
Prevent false positives when matching story files by prefix.
startsWith(prefix) makes story 1.1 match 1-10-..., leading to incorrect “Start Developing” actions. Require a boundary (exact match or prefix-).
🔧 Proposed fix
- const match = files.some(file => file.startsWith(prefix) && file.endsWith('.md'));
+ const match = files.some(file => {
+ if (!file.endsWith('.md')) return false;
+ return file === `${prefix}.md` || file.startsWith(`${prefix}-`);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Simple glob match: pattern is "{number}*.md", check if file starts with the number prefix | |
| const prefix = storyNumberWithDashes; | |
| const match = files.some(file => file.startsWith(prefix) && file.endsWith('.md')); | |
| console.log(`[BMAD] Match result: ${match}`); | |
| // Simple glob match: pattern is "{number}*.md", check if file starts with the number prefix | |
| const prefix = storyNumberWithDashes; | |
| const match = files.some(file => { | |
| if (!file.endsWith('.md')) return false; | |
| return file === `${prefix}.md` || file.startsWith(`${prefix}-`); | |
| }); | |
| console.log(`[BMAD] Match result: ${match}`); |
🤖 Prompt for AI Agents
In `@src/storyCodeLensProvider.ts` around lines 121 - 124, The current prefix
check using file.startsWith(storyNumberWithDashes) causes false positives (e.g.,
"1.1" matching "1-10-..."); update the match logic (where variables files and
storyNumberWithDashes are used, e.g., the match constant) to require a boundary:
consider a file a match only if it equals `${storyNumberWithDashes}.md` or it
startsWith(`${storyNumberWithDashes}-`) and endsWith('.md'); replace the
existing startsWith(...) && endsWith('.md') condition with this stricter check
to prevent substring collisions.
| for (let i = startIndex; i < lines.length; i++) { | ||
| const line = lines[i]; | ||
|
|
||
| if (i > startIndex && /^#{1,3}\s+/.test(line)) { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Break on headings even when they immediately follow Tasks.
If the Tasks header is followed directly by another heading (no blank line), the current guard prevents breaking and tasks under the next section can be parsed. Dropping the guard avoids that edge case.
🔧 Suggested adjustment
- if (i > startIndex && /^#{1,3}\s+/.test(line)) {
+ if (/^#{1,3}\s+/.test(line)) {
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (let i = startIndex; i < lines.length; i++) { | |
| const line = lines[i]; | |
| if (i > startIndex && /^#{1,3}\s+/.test(line)) { | |
| break; | |
| } | |
| for (let i = startIndex; i < lines.length; i++) { | |
| const line = lines[i]; | |
| if (/^#{1,3}\s+/.test(line)) { | |
| break; | |
| } |
🤖 Prompt for AI Agents
In `@src/techSpecParser.ts` around lines 53 - 58, The loop in the task-parsing
logic uses a guard "if (i > startIndex && /^#{1,3}\s+/.test(line)) { break; }"
which prevents breaking when a heading immediately follows the Tasks header;
remove the "i > startIndex &&" check so the condition simply tests the heading
regex and breaks immediately. Update the for loop in the task parsing function
(the loop over lines with variables lines and startIndex) to break on
/^#{1,3}\s+/ regardless of i, ensuring headings directly after the Tasks header
stop task parsing.
| // Mock the vscode module | ||
| alias: { | ||
| vscode: new URL('./src/__tests__/mocks/vscode.ts', import.meta.url).pathname | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "vitest.config.ts" -type fRepository: bmad-code-org/bmad-method-vscode
Length of output: 94
🏁 Script executed:
cat -n vitest.config.ts 2>/dev/null || echo "File not found in root"Repository: bmad-code-org/bmad-method-vscode
Length of output: 2609
🌐 Web query:
Vitest fileURLToPath vs URL pathname Windows compatibility
💡 Result:
Short answer: use fileURLToPath (node:url) — not URL.pathname — to convert import.meta.url to a filesystem path on Windows. URL.pathname keeps a leading '/' (yielding "/C:/...") which breaks Windows paths; fileURLToPath returns a correct platform path ("C:..."). (nodejs.org)
Example:
- wrong: new URL('.', import.meta.url).pathname // produces "/C:/..." on Windows
- correct:
import { fileURLToPath } from 'node:url'
import { dirname } from 'node:path'
const __filename = fileURLToPath(import.meta.url)
const __dirname = dirname(__filename)
Vitest tip: when you set aliases or resolve paths in vitest/vite configs, wrap import.meta.url with fileURLToPath (or use path.resolve with __dirname if available) to avoid Windows issues. (github.com)
Citations:
- 1: https://nodejs.org/docs/latest-v20.x/api/url.html?utm_source=openai
- 2: Failed to load url resolved bug vitest-dev/vitest#3042
🌐 Web query:
Vite alias configuration fileURLToPath best practice
💡 Result:
Best-practice checklist for Vite alias + fileURLToPath
-
Use the URL + fileURLToPath pattern in ESM Vite configs:
import { fileURLToPath, URL } from 'node:url'
alias: { '@': fileURLToPath(new URL('./src', import.meta.url)) } — works in ESM and is what Vite docs/examples use. [1][2] -
Keep TypeScript and editor tooling in sync:
declare the same aliases in tsconfig.json (compilerOptions.paths) so TS and IDEs resolve imports. Vite’s resolve.alias does not replace the need for tsconfig paths. [3][4] -
Remember runtime differences (Node/Esm vs CommonJS):
- In ESM (vite.config.ts/.mjs) prefer fileURLToPath(new URL(..., import.meta.url)).
- In CommonJS (older setups) use path.resolve(__dirname, 'src'). Trying import.meta.url in CJS fails. [2][5]
-
If server code runs under Node (not processed by Vite), configure Node resolution too:
add package.json "imports" or use module-alias so Node can resolve the same aliases at runtime. Vite-only aliases don’t affect plain Node execution. [4] -
Keep aliases distinct from package names (use a prefix like @ or #) and prefer explicit mappings per alias rather than ad-hoc hacks. [4]
If you want, I can generate a ready-to-use vite.config.ts + tsconfig.json snippet that follows these practices for your project.
Replace .pathname with fileURLToPath for Windows path compatibility.
On Windows, new URL(...).pathname produces POSIX paths with a leading slash (e.g., /C:/...), which breaks Vite's alias resolution. Use fileURLToPath instead, as documented in Node.js and recommended by the Vitest community.
🛠️ Suggested fix
import { defineConfig } from 'vitest/config';
+import { fileURLToPath } from 'node:url';
export default defineConfig({
test: {
// Mock the vscode module
alias: {
- vscode: new URL('./src/__tests__/mocks/vscode.ts', import.meta.url).pathname
+ vscode: fileURLToPath(new URL('./src/__tests__/mocks/vscode.ts', import.meta.url))
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Mock the vscode module | |
| alias: { | |
| vscode: new URL('./src/__tests__/mocks/vscode.ts', import.meta.url).pathname | |
| }, | |
| import { defineConfig } from 'vitest/config'; | |
| import { fileURLToPath } from 'node:url'; | |
| export default defineConfig({ | |
| test: { | |
| // Mock the vscode module | |
| alias: { | |
| vscode: fileURLToPath(new URL('./src/__tests__/mocks/vscode.ts', import.meta.url)) | |
| }, |
🤖 Prompt for AI Agents
In `@vitest.config.ts` around lines 11 - 14, The alias entry uses new
URL('./src/__tests__/mocks/vscode.ts', import.meta.url).pathname which yields
POSIX-style paths on Windows; update the alias for the vscode key to use
fileURLToPath(new URL('./src/__tests__/mocks/vscode.ts', import.meta.url))
instead (import fileURLToPath from 'url' if not already) so the alias resolves
to a proper platform path; adjust the vitest.config.ts import list and replace
.pathname usage wherever alias.vscode is defined.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.