Skip to content

Initial extension contribution#3

Merged
bmadcode merged 72 commits into
bmad-code-org:mainfrom
elvince:initial-extension-contribution
Apr 2, 2026
Merged

Initial extension contribution#3
bmadcode merged 72 commits into
bmad-code-org:mainfrom
elvince:initial-extension-contribution

Conversation

@elvince
Copy link
Copy Markdown

@elvince elvince commented Feb 27, 2026

Initial upload for vscode extension:

BMAD Dashboard

A VS Code extension that acts as a real-time GPS for BMAD V6 projects. It monitors workflow artifacts, tracks sprint progress, and recommends next actions — all without leaving the editor.

Features

Sidebar Dashboard

Auto-activates when the workspace contains a _bmad/ directory. Appears as a custom icon in the Activity Bar.

Header Toolbar

  • Help icon — copies bmad help to clipboard
  • Overflow menu (⋮) — lists all available workflow commands with descriptions, plus a manual Refresh option; dismisses on click-outside or ESC

Sprint Progress

  • Visual progress bar with done / in-progress / backlog counts
  • Project name and completion percentage

Epic List

  • Collapsible cards per epic showing status, progress bar, and done/total story counts
  • Active epic highlighted (blue left border)
  • Done epics hidden by default; toggle to reveal them
  • Scrollable container (max 280 px)
  • Click epic title → expand/collapse story list
  • Shift+Click epic title → open epics.md in text editor
  • Click a story inside the list → open the story markdown file

Active Story Card

  • Shows current story's epic/story number, title, task progress bar, subtask count, and status badge
  • Progress combines tasks + subtasks
  • Click story title → open story file in preview
  • Shift+Click story title → open in text editor

Next Action Recommendation

State-machine-driven suggestion with mandatory/optional action kinds:

Condition Suggested Action
No sprint data Run Sprint Planning
Story in-progress Continue Story X.Y
Story in review Run Code Review
Story ready-for-dev Start Dev Story X.Y
No active story Create Next Story
Epic complete Run Retrospective
All done Sprint Complete

Each action has a Play button (execute in terminal) and a Copy button (clipboard).

Other Actions — Secondary workflow buttons that change based on project state (e.g., Correct Course, Create Story).

Planning Artifact Links — Quick links to PRD and Architecture docs. Click opens markdown preview; Shift+Click opens in text editor.

About Section — Displays BMAD version, last-updated date, and installed modules (from manifest.yaml).

Editor Panel

A multi-view editor panel (BMAD: Open Editor Panel command) with breadcrumb navigation:

  • Dashboard view — mirrors the sidebar dashboard
  • Epics Browser — browse epics and drill into story details
  • Stories Table & Kanban Board — view all stories in table or kanban layout
  • Document Library — file tree browser with markdown rendering, syntax highlighting, and table of contents

Real-Time Updates

  • File watcher monitors _bmad-output/**/*.{yaml,md} with 500 ms debounce
  • Any change to sprint-status.yaml or story files triggers a full state recompute and UI refresh

Configuration

Setting Default Purpose
bmad.outputRoot _bmad-output Root directory for BMAD output files
bmad.cliPrefix claude CLI prefix for terminal commands (e.g., claude, aider, copilot)
bmad.defaultClickBehavior markdown-preview Click behavior for doc links: markdown-preview or editor-panel
bmad.docLibraryPaths ["planning-artifacts", "implementation-artifacts", "docs"] Folders to display in the Document Library

Prerequisites

  • Node.js 22+
  • pnpm 10.26+ (corepack enable && corepack prepare pnpm@10.26.2)

Building

The extension uses a dual build system — esbuild for the extension host, Vite for the React webview.

pnpm install              # install dependencies
pnpm build                # full build (extension + webview)
pnpm build:extension      # build extension host only
pnpm build:webview        # build webview only
pnpm watch                # parallel watch mode for both

To package as a .vsix:

pnpm vscode:package       # produces out/bmad-dashboard-*.vsix

Testing

Webview tests (Vitest)

Runs in a jsdom environment using @testing-library/react.

pnpm test                 # run all Vitest tests once
pnpm test:watch           # watch mode
pnpm test:coverage        # generate coverage report (v8 provider)

Extension host tests (Mocha)

Runs under @vscode/test-electron for tests that need VS Code APIs.

pnpm test:extension       # run extension integration tests

Linting & type checking

pnpm lint                 # ESLint
pnpm typecheck            # typecheck both extension and webview

Release Process

Versioning and releases are fully automated via semantic-release and a GitHub Actions workflow.

How it works

  1. Push to main triggers the release workflow (.github/workflows/release.yml)
  2. Commit analysis@semantic-release/commit-analyzer determines the next version from Conventional Commits:
    • fix: → patch bump (1.2.x)
    • feat: → minor bump (1.x.0)
    • BREAKING CHANGE: / feat!: → major bump (x.0.0)
  3. ChangelogCHANGELOG.md is updated automatically
  4. Build & package — the extension is built and packaged as a .vsix
  5. Git commitpackage.json and CHANGELOG.md are committed with chore(release): <version> [skip ci]
  6. GitHub release — a release is created with the .vsix attached as a downloadable asset

CI pipeline

Pull requests to main run the CI workflow (.github/workflows/ci.yml):

  • Lint → Typecheck → Test → Build

Local dry run

pnpm release:dry          # preview what the next release would be

Project Structure

src/
├── extension/            # VS Code extension host (Node.js)
│   ├── extension.ts      # main entry point
│   ├── commands/         # command handlers
│   ├── parsers/          # YAML, epic, story file parsers
│   ├── providers/        # webview providers (dashboard, editor panel)
│   └── services/         # BMAD detector, file watcher, state manager, workflows
├── webviews/             # React webview (sandboxed)
│   ├── dashboard/        # sidebar dashboard components
│   └── editor-panel/     # multi-view editor (epics, stories, docs)
└── shared/               # shared types and message protocol

License

MIT

Vincent M. and others added 10 commits February 27, 2026 09:34
# [1.1.0](elvince/bmad-dashboard-extension@v1.0.0...v1.1.0) (2026-02-19)

### Features

* add workflow sequencing with mandatory/optional action kinds ([ee8498d](elvince/bmad-dashboard-extension@ee8498d))
* fall back to epics.md when clicking unwritten story in epic dropdown ([77f1b03](elvince/bmad-dashboard-extension@77f1b03))
# [1.2.0](elvince/bmad-dashboard-extension@v1.1.0...v1.2.0) (2026-02-20)

### Bug Fixes

* file naming from story/epic name parsing with dot ([39f7c28](elvince/bmad-dashboard-extension@39f7c28))
* split story like 5.5a/b/c are now properly managed ([b85aec9](elvince/bmad-dashboard-extension@b85aec9))

### Features

* 5-5a-editor-panel-infrastructure-and-build-setup ([93458e9](elvince/bmad-dashboard-extension@93458e9))
* 5-5b-navigation-shell-breadcrumbs-dashboard-view-and-click-behavior ([2cfbbb7](elvince/bmad-dashboard-extension@2cfbbb7))
* 5-6-epics-browser-and-story-detail-views ([e28ec4c](elvince/bmad-dashboard-extension@e28ec4c))
* 5-7-stories-table-and-kanban-board-views ([b49ee37](elvince/bmad-dashboard-extension@b49ee37))
* 5-8-document-library-and-markdown-viewer ([707f9c8](elvince/bmad-dashboard-extension@707f9c8))
* add viewer menu entry ([31df0ba](elvince/bmad-dashboard-extension@31df0ba))
## [1.2.1](elvince/bmad-dashboard-extension@v1.2.0...v1.2.1) (2026-02-20)

### Bug Fixes

* story criteria not properly displayed ([185efd1](elvince/bmad-dashboard-extension@185efd1))
## [1.2.2](elvince/bmad-dashboard-extension@v1.2.1...v1.2.2) (2026-02-23)

### Bug Fixes

* next action optional retro is no more proposed if the epic is done ([1afc482](elvince/bmad-dashboard-extension@1afc482))
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 27, 2026

Important

Review skipped

Too many files!

This PR contains 152 files, which is 2 over the limit of 150.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2663bb9-c381-479a-902e-e1303211a42d

📥 Commits

Reviewing files that changed from the base of the PR and between 67a1264 and 258b620.

⛔ Files ignored due to path filters (2)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • resources/bmad-icon.svg is excluded by !**/*.svg
📒 Files selected for processing (152)
  • .gitattributes
  • .gitignore
  • .npmrc
  • .prettierignore
  • .prettierrc
  • .releaserc.json
  • .vscode-test.mjs
  • .vscode/css_custom_data.json
  • .vscode/extensions.json
  • .vscode/launch.json
  • .vscode/settings.json
  • .vscode/tasks.json
  • .vscodeignore
  • CHANGELOG.md
  • LICENSE.md
  • README.md
  • docs/architecture.md
  • docs/component-inventory.md
  • docs/development-guide.md
  • docs/index.md
  • docs/project-overview.md
  • docs/project-scan-report.json
  • docs/source-tree-analysis.md
  • eslint.config.mjs
  • package.json
  • src/extension/commands/index.ts
  • src/extension/extension.test.ts
  • src/extension/extension.ts
  • src/extension/parsers/epic-parser.test.ts
  • src/extension/parsers/epic-parser.ts
  • src/extension/parsers/index.ts
  • src/extension/parsers/sprint-status.test.ts
  • src/extension/parsers/sprint-status.ts
  • src/extension/parsers/story-parser.test.ts
  • src/extension/parsers/story-parser.ts
  • src/extension/parsers/utils.ts
  • src/extension/providers/dashboard-view-provider.test.ts
  • src/extension/providers/dashboard-view-provider.ts
  • src/extension/providers/editor-panel-provider.test.ts
  • src/extension/providers/editor-panel-provider.ts
  • src/extension/providers/index.ts
  • src/extension/providers/message-handler.ts
  • src/extension/providers/webview-utils.ts
  • src/extension/services/bmad-detector.test.ts
  • src/extension/services/bmad-detector.ts
  • src/extension/services/file-tree-scanner.test.ts
  • src/extension/services/file-tree-scanner.ts
  • src/extension/services/file-watcher.test.ts
  • src/extension/services/file-watcher.ts
  • src/extension/services/index.ts
  • src/extension/services/state-manager.test.ts
  • src/extension/services/state-manager.ts
  • src/extension/services/workflow-discovery.test.ts
  • src/extension/services/workflow-discovery.ts
  • src/shared/messages.test.ts
  • src/shared/messages.ts
  • src/shared/types/bmad-metadata.ts
  • src/shared/types/dashboard-state.ts
  • src/shared/types/epic.ts
  • src/shared/types/file-tree.ts
  • src/shared/types/index.ts
  • src/shared/types/parse-result.test.ts
  • src/shared/types/parse-result.ts
  • src/shared/types/sprint-status.test.ts
  • src/shared/types/sprint-status.ts
  • src/shared/types/story.test.ts
  • src/shared/types/story.ts
  • src/shared/types/workflow.ts
  • src/webviews/app.test.tsx
  • src/webviews/app.tsx
  • src/webviews/dashboard/components/about-section.test.tsx
  • src/webviews/dashboard/components/about-section.tsx
  • src/webviews/dashboard/components/active-story-card.test.tsx
  • src/webviews/dashboard/components/active-story-card.tsx
  • src/webviews/dashboard/components/cta-buttons.test.tsx
  • src/webviews/dashboard/components/cta-buttons.tsx
  • src/webviews/dashboard/components/epic-list.test.tsx
  • src/webviews/dashboard/components/epic-list.tsx
  • src/webviews/dashboard/components/header-toolbar.test.tsx
  • src/webviews/dashboard/components/header-toolbar.tsx
  • src/webviews/dashboard/components/index.ts
  • src/webviews/dashboard/components/next-action-recommendation.test.tsx
  • src/webviews/dashboard/components/next-action-recommendation.tsx
  • src/webviews/dashboard/components/placeholder.test.tsx
  • src/webviews/dashboard/components/placeholder.tsx
  • src/webviews/dashboard/components/planning-artifact-links.test.tsx
  • src/webviews/dashboard/components/planning-artifact-links.tsx
  • src/webviews/dashboard/components/sprint-status.test.tsx
  • src/webviews/dashboard/components/sprint-status.tsx
  • src/webviews/dashboard/hooks/index.ts
  • src/webviews/dashboard/hooks/use-message-handler.test.ts
  • src/webviews/dashboard/hooks/use-message-handler.ts
  • src/webviews/dashboard/index.test.tsx
  • src/webviews/dashboard/index.tsx
  • src/webviews/dashboard/store.test.ts
  • src/webviews/dashboard/store.ts
  • src/webviews/dashboard/utils/build-command-with-story.test.ts
  • src/webviews/dashboard/utils/build-command-with-story.ts
  • src/webviews/dashboard/utils/get-next-action.test.ts
  • src/webviews/dashboard/utils/get-next-action.ts
  • src/webviews/editor-panel/components/breadcrumb-bar.test.tsx
  • src/webviews/editor-panel/components/breadcrumb-bar.tsx
  • src/webviews/editor-panel/components/file-tree.test.tsx
  • src/webviews/editor-panel/components/file-tree.tsx
  • src/webviews/editor-panel/components/index.ts
  • src/webviews/editor-panel/components/kanban-board.test.tsx
  • src/webviews/editor-panel/components/kanban-board.tsx
  • src/webviews/editor-panel/components/markdown-renderer.test.tsx
  • src/webviews/editor-panel/components/markdown-renderer.tsx
  • src/webviews/editor-panel/components/navigation-shell.test.tsx
  • src/webviews/editor-panel/components/navigation-shell.tsx
  • src/webviews/editor-panel/hooks/index.ts
  • src/webviews/editor-panel/hooks/use-message-handler.test.ts
  • src/webviews/editor-panel/hooks/use-message-handler.ts
  • src/webviews/editor-panel/index.tsx
  • src/webviews/editor-panel/store.test.ts
  • src/webviews/editor-panel/store.ts
  • src/webviews/editor-panel/styles/hljs-vscode.css
  • src/webviews/editor-panel/types.ts
  • src/webviews/editor-panel/utils/parse-story-content.test.ts
  • src/webviews/editor-panel/utils/parse-story-content.ts
  • src/webviews/editor-panel/views/dashboard-view.test.tsx
  • src/webviews/editor-panel/views/dashboard-view.tsx
  • src/webviews/editor-panel/views/docs-view.test.tsx
  • src/webviews/editor-panel/views/docs-view.tsx
  • src/webviews/editor-panel/views/epic-detail-view.test.tsx
  • src/webviews/editor-panel/views/epic-detail-view.tsx
  • src/webviews/editor-panel/views/epics-view.test.tsx
  • src/webviews/editor-panel/views/epics-view.tsx
  • src/webviews/editor-panel/views/index.ts
  • src/webviews/editor-panel/views/placeholder-view.tsx
  • src/webviews/editor-panel/views/stories-view.test.tsx
  • src/webviews/editor-panel/views/stories-view.tsx
  • src/webviews/editor-panel/views/story-detail-view.test.tsx
  • src/webviews/editor-panel/views/story-detail-view.tsx
  • src/webviews/index.css
  • src/webviews/index.tsx
  • src/webviews/shared/components/index.ts
  • src/webviews/shared/hooks/index.ts
  • src/webviews/shared/hooks/use-vscode-api.test.ts
  • src/webviews/shared/hooks/use-vscode-api.ts
  • src/webviews/shared/utils/cn.test.ts
  • src/webviews/shared/utils/cn.ts
  • src/webviews/shared/utils/document-link.test.ts
  • src/webviews/shared/utils/document-link.ts
  • src/webviews/shared/utils/status-styles.ts
  • tsconfig.extension.json
  • tsconfig.json
  • tsconfig.webview.json
  • vite.config.ts
  • vitest.config.ts
  • vitest.setup.ts

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wsmoak
Copy link
Copy Markdown
Collaborator

wsmoak commented Feb 28, 2026

Thanks for doing this! A few things, can you update the PR description with what it is, what it does, and what commands to build and install it locally?

Also the generated bmad stuff, the .claude/commands, _bmad and _bmad-output should not be in the PR.

Claude suggests git rm -r --cached .claude/commands (for each one) will remove them without deleting. -- then add them to .gitignore or .git/info/exclude to keep them out.

If that doesn't work I'd make a copy as a backup then git rm them and just re-install bmad after it's cleaned up.

@wsmoak
Copy link
Copy Markdown
Collaborator

wsmoak commented Mar 13, 2026

Coderabbit says:

This PR contains 152 files, which is 2 over the limit of 150.

LOL! So close.

I turned Codex loose on it since they are giving 2x usage until April with Codex Desktop and it called out these things. Can you take a look and see if you agree with them or not?

P1 Re-run BMAD detection before parsing artifacts
If the extension starts in a BMAD workspace before _bmad-output/ exists, detectBmadProject() caches outputRoot: null during activation and parseAll() keeps reusing that cached result. After the user runs sprint planning and the output folder is created, both bmad.refresh and the initial state load still return early here, so the dashboard never discovers the new artifacts until VS Code is reloaded. That blocks the first-run workflow for newly initialized projects.
src/extension/services/state-manager.ts:129-135

P2 Honor editor-panel click behavior for planning artifact links
These buttons always call OPEN_DOCUMENT, so when bmad.defaultClickBehavior is set to editor-panel the PRD and Architecture links still open Markdown preview instead of navigating into the editor panel. The new setting is advertised as controlling sidebar document links, so this leaves one of the main sidebar entry points ignoring the configured behavior.
src/webviews/dashboard/components/planning-artifact-links.tsx:27-48

P2 Do not append the current story id to create-story commands
WorkflowDiscoveryService exposes create-story as an optional action while the active story is in review, but buildCommandWithStory() treats create-story as story-specific and always appends story X.Y. In that state the only available story reference is the one already under review, so the generated CTA points at the current story instead of creating the next one, which is the whole purpose of offering this optional action.
src/webviews/dashboard/utils/build-command-with-story.ts:3-16

Vincent M. added 2 commits March 27, 2026 11:10
…behavior + P2 create-story)

- Re-run BMAD detection when outputRoot is null on refresh, so users
  don't need a full reload after project init
- Planning artifact links now respect bmad.defaultClickBehavior setting
- Remove create-story from story-aware workflows to avoid appending
  the current story ID to new-story commands
- Add unit tests for buildCommandWithStory
@wsmoak
Copy link
Copy Markdown
Collaborator

wsmoak commented Mar 29, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 29, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@pbean pbean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review -- Generated by Claude Code

Summary

This is a thorough automated review of the full 154-file, 49K-line initial extension contribution. The codebase is generally well-structured with strong patterns (discriminated union types, "never throw" parsers, clean disposal patterns, strict CSP). However, there are packaging, security, concurrency, and accessibility issues that should be addressed.

Inline comments are placed on specific files below. Here is the full priority breakdown:

Priority Count Key Items
CRITICAL 4 VSIX packaging gaps, dual lock files, missing marketplace icon
HIGH 5 Path traversal, concurrent state mutation, async race condition, a11y keyboard nav, deps classification
MEDIUM 9 Message race, disposal leak, stale closure, Tailwind syntax typo, ARIA gaps, dead config, etc.
LOW 18 Nonce RNG, silent catches, duplicated code, regex inconsistencies, no protocol versioning, etc.

Additional LOW severity findings (not annotated inline)

  • L2. state-manager.ts:230, 266 -- Silent catch blocks swallow errors in parsing logic after successful file reads.
  • L3. state-manager.ts:700, file-tree-scanner.ts:78, workflow-discovery.ts:393 -- readFile returns null for ALL errors (no distinction between missing file vs permission denied).
  • L4. state-manager.ts:384 -- detectPlanningArtifacts reads full file content just to check existence. Use vscode.workspace.fs.stat().
  • L5. extension.ts:8 -- BmadDetector never pushed to context.subscriptions; breaks the disposal pattern.
  • L7. epic-parser.ts:20 vs story-parser.ts:27 -- USER_STORY_REGEX inconsistency (one requires trailing period, the other doesn't).
  • L8. epic-parser.ts:106 -- Story content boundary uses hardcoded STORY_HEADER_PREFIX_LENGTH = 20; breaks for multi-digit numbers.
  • L9. epic-parser.ts:77, 231 -- Global regexes with /gm need manual lastIndex reset (fragile). story-parser.ts uses safer matchAll().
  • L10. epic.ts:27 -- EpicStoryEntry.status uses inline literal union instead of StoryStatusValue type; will drift.
  • L11. messages.ts -- No version field in message protocol; extension/webview version mismatch is undetectable.
  • L12. messages.ts -- No request/response correlation ID on REQUEST_DOCUMENT_CONTENT / DOCUMENT_CONTENT.
  • L13. epic-parser.ts:258 -- parseEpics silently drops unparseable epic sections with no visibility to callers.
  • L14. Both Zustand stores -- setError pushes to an unbounded array that is never displayed or capped.
  • L15. .gitignore contains boilerplate entries for unused tools (Playwright, Turbo, Vercel, Prisma, Next.js, etc.).
  • L16. package.json -- "workspaceContains:**/_bmad/**" deep glob scans entire workspace on startup; could delay activation in large workspaces.
  • L17. Neither Zustand store integrates with vscodeApi.setState/getState, causing loading flash on webview recreation.
  • L18. editor-panel-provider.ts:162 -- scanAndSendFileTree has no error handling; unhandled promise rejection if scan fails.

Test Suite Notes

Strengths: Parser tests are excellent (realistic fixtures, edge cases, perf regression). Security-conscious provider tests (shell metacharacter rejection, command injection). Good isolation with beforeEach/afterEach and sinon sandboxes.

Gaps to address:

  • extension.test.ts has only 2 smoke tests -- no command/provider registration verification.
  • EditorPanelProvider tests don't cover OPEN_DOCUMENT, EXECUTE_WORKFLOW, or other message handlers.
  • messages.test.ts constant checks are tautological (expect(X).toBe('X')).
  • StateManager never exercises the epic parsing path with valid content.
  • CSS class assertions in component tests test implementation details, not behavior.

Architecture Positives

  • Clean discriminated union ParseResult<T> with type guards and factory functions
  • "Never throw" parser pattern throughout
  • Two-tier disposable management in FileWatcher
  • Strict CSP on webviews with nonce-based script loading
  • react-markdown instead of dangerouslySetInnerHTML
  • Clean separation: detection -> watching -> state management -> workflow discovery -> providers

Comment thread .vscodeignore
Comment thread package.json Outdated
"vite": "^7.3.0",
"vitest": "^4.0.18"
},
"dependencies": {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] H5: All dependencies should be devDependencies

Everything is bundled (esbuild for extension, Vite for webview). Having react, zustand, gray-matter, etc. in dependencies is semantically misleading. While --no-dependencies on vsce package prevents them from being included in the VSIX, a future contributor omitting that flag would bloat the package.

Move all entries from dependencies to devDependencies.

Comment thread package.json
@@ -0,0 +1,172 @@
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] C3: Missing marketplace icon

No root-level icon field exists. The VS Code Marketplace requires a root "icon" field pointing to a PNG (128x128 or 256x256). The existing resources/bmad-icon.svg is only the activity bar icon, and SVG is not accepted for the marketplace listing.

Add a PNG icon and declare "icon": "resources/bmad-icon.png".

Comment thread package-lock.json Outdated
@@ -0,0 +1,16274 @@
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[CRITICAL] C2: Dual lock files

Both package-lock.json (npm) and pnpm-lock.yaml are committed. The project declares "packageManager": "pnpm@10.26.2" and uses pnpm throughout all scripts.

Delete this file and add package-lock.json to .gitignore to prevent CI/contributor confusion and non-deterministic builds.

Comment thread src/extension/providers/editor-panel-provider.ts
<tr
key={story.key}
data-testid={`story-table-row-${story.key}`}
role="button"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M7: <tr role="button"> overrides table semantics

Adding role="button" to a <tr> overrides the implicit row role. Screen readers will announce these as buttons rather than table rows, losing table navigation semantics. Consider using a click handler without overriding the role, or moving the interactive element to a cell.

Comment thread tsconfig.extension.json Outdated
"sourceMap": true,
"baseUrl": ".",
"paths": {
"@shared/*": ["src/shared/*"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M8: Dead @shared/* path alias

This path alias is declared but esbuild (used for the extension build) does not resolve TypeScript path aliases without an esbuild plugin. Extension code currently uses relative imports so this works by accident, but if anyone adds an @shared/ import the build will break silently.

Either remove this alias or add an esbuild alias plugin.

Comment thread .releaserc.json Outdated
[
"@semantic-release/exec",
{
"prepareCmd": "pnpm build && pnpm vscode:package"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M9: Double build during release

prepareCmd runs pnpm build && pnpm vscode:package, but vscode:package invokes npx @vscode/vsce package, which triggers the vscode:prepublish lifecycle script, which also runs pnpm build. The full build executes twice per release.

Either remove pnpm build from prepareCmd or skip prepublish in the package command.

let text = '';
const possible = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
for (let i = 0; i < 32; i++) {
text += possible.charAt(Math.floor(Math.random() * possible.length));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] L1: Nonce generated with Math.random()

Math.random() is not a CSPRNG. For a VS Code extension where the nonce is generated server-side this is acceptable in practice, but crypto.randomBytes(16).toString('hex') would be more correct for a CSP nonce.

Comment thread src/extension/parsers/story-parser.ts Outdated
* @param title - The story title to convert (e.g., "Story File Parser")
* @returns Kebab-case string suitable for use as a key (e.g., "story-file-parser")
*/
function toKebabCase(title: string): string {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] L6: toKebabCase is duplicated

This identical function also exists in epic-parser.ts:32. Both are used to generate story keys for joining epic story entries with their corresponding story files. If one copy is updated without the other, key generation will diverge and join operations will silently break.

Extract to a shared utility.

Resolve critical, high, medium, and low priority findings from PR review.

Critical: fix .vscodeignore packaging gaps, remove dual lock file,
move bundled deps to devDependencies, fix double build in release.

High: add path traversal validation in editor panel, cap unbounded
error arrays in Zustand stores.

Medium: fix tr role="button" accessibility violation, remove dead
@shared/* path alias, fix double build in .releaserc.json.

Low: use crypto.randomBytes for CSP nonce, extract duplicated
toKebabCase to shared utility, align USER_STORY_REGEX across parsers,
replace hardcoded header prefix length, add debug logging to silent
catch blocks.
@elvince
Copy link
Copy Markdown
Author

elvince commented Mar 30, 2026

I fixed all items except the icon that should be set later on by BMAD team

Copy link
Copy Markdown

@pbean pbean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review (Follow-Up Verification) -- Generated by Claude Code

Re-Review After Fix Commits

Verified findings after commits 7fa6620 and 16ad2f9 which addressed the initial review. Of the original 36 findings, 15 are fully fixed, 2 are partially fixed, and 19 remain open (mostly LOW severity).

What Was Fixed (Thank You!)

ID Finding Fix
C1/C4 VSIX packaging gaps .vscodeignore now has !out/** and excludes dev files
C2 Dual lock files package-lock.json deleted and git-ignored
H1 Path traversal in readAndSendDocumentContent path.resolve + startsWith guard added
H5 dependencies vs devDependencies All moved to devDependencies
M2 Disposal of onStateChange (see inline -- still has an issue)
M7 <tr role="button"> Role removed, proper tabIndex/aria-label/onKeyDown used
M8 Dead @shared/* path alias Removed from tsconfig.extension.json
M9 Double build during release prepareCmd now only runs pnpm vscode:package
L1 Nonce with Math.random() Now uses crypto.randomBytes(16).toString('hex')
L2 Silent catch blocks Now log via console.debug
L6 Duplicated toKebabCase Extracted to shared parsers/utils.ts
L7 USER_STORY_REGEX inconsistency Both parsers now use identical regex with trailing period
L8 Hardcoded story header offset Now uses actual match.index for boundary calculation
L14 Unbounded error array Both stores cap at 50 via .slice(-50)

Remaining Open Items (annotated inline below)

HIGH (3): H2 concurrent state mutation, H3 unguarded async handler, H4 dropdown keyboard a11y
MEDIUM (5): M1 webview ready race, M2 disposal leak (revised), M3 fragile path matching, M4 stale closure, M5 Tailwind typo
MEDIUM-PARTIAL (1): M6 file tree ARIA (roles added, but missing aria-expanded/aria-selected/keyboard nav)

LOW items still open (not annotated inline)

  • L3. readFile returns null for all error types (state-manager, file-tree-scanner, workflow-discovery)
  • L4. detectPlanningArtifacts reads full files just to check existence
  • L5. BmadDetector never pushed to context.subscriptions
  • L9. epic-parser.ts still uses manual lastIndex reset instead of matchAll()
  • L10. EpicStoryEntry.status still uses inline literal union instead of StoryStatusValue
  • L11. No message protocol versioning
  • L12. No request/response correlation ID
  • L13. parseEpics silently drops unparseable sections
  • L15. Boilerplate .gitignore with unused tool entries
  • L16. Deep glob activation event may slow startup in large workspaces
  • L17. No webview state persistence via vscodeApi.setState/getState
  • L18. scanAndSendFileTree has no error handling (unhandled rejection)

New Observations

  • N1. The H1 path traversal security fix has no test coverage (see inline)
  • N2. The shared toKebabCase in parsers/utils.ts has no dedicated unit tests
  • N3. import * as path from 'path' (editor-panel-provider.ts:2) uses bare specifier while rest of codebase uses node:path prefix -- minor inconsistency

Comment thread src/extension/services/state-manager.ts Outdated
const updatePromises = updateChanges.map(async ([filePath]) =>
this.handleFileUpdate(filePath, paths.outputRoot!)
);
await Promise.all(updatePromises);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] H2: Concurrent state mutation via Promise.all -- Still Open

This was flagged in the initial review and remains unchanged. handleFileUpdate calls run in parallel here, and each one mutates this._state via object spread independently. If two updates run concurrently (e.g., sprint-status.yaml and epics.md both changed), the second to finish overwrites the first's state changes (last-write-wins).

Suggested fix: either process updates sequentially (for...of with await), or have each handleFileUpdate return a partial state object and merge them all after Promise.all resolves.

// Subscribe to file watcher events
this.disposables.push(
this.fileWatcher.onDidChange((event) => {
void this.handleFileChanges(event);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] H3: Race condition -- unguarded async handler -- Still Open

This was flagged in the initial review and remains unchanged. void this.handleFileChanges(event) fires and forgets -- if two debounced file-change events arrive in quick succession, two handleFileChanges invocations run concurrently, both reading and mutating this._state without coordination. The _initializing guard only protects during the initial parseAll().

Suggested fix: add a promise chain (e.g., this._changeQueue = this._changeQueue.then(() => this.handleFileChanges(event))) to serialize calls.


{isOpen && (
<div
role="menu"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] H4: Dropdown menu lacks keyboard navigation -- Still Open

This was flagged in the initial review and remains unchanged. The menu has role="menu" and role="menuitem" but no arrow-key navigation, no focus management (focus doesn't move into the menu on open), and no roving tabindex. Only Escape dismissal and click-outside are handled.

Per WAI-ARIA menu pattern, role="menu" requires arrow-key focus movement among role="menuitem" children. Either implement the full pattern or downgrade to a simpler disclosure widget (role="list" / role="listitem") which has fewer keyboard requirements.

onNavigateEditorPanel: (view, params) => {
if (this.stateManager) {
EditorPanelProvider.createOrShow(this.extensionUri, this.stateManager);
EditorPanelProvider.postNavigateToView(view, params);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M1: Race -- postNavigateToView before webview ready -- Still Open

createOrShow() on line 47 followed immediately by postNavigateToView() on line 48. If the editor panel is being created for the first time, the webview JS hasn't loaded yet and the postMessage call is silently lost.

Common fix: have the webview send a "ready" message on load, and queue outbound messages until that acknowledgment arrives.


// Subscribe to state changes and forward to webview
if (this.stateManager) {
this.disposables.push(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M2: onStateChange subscription leak -- Still Open (revised)

The stateManager.onStateChange subscription is pushed into this.disposables here, but nothing ever iterates and disposes that array when the webview view is destroyed. The third-arg pattern on onDidReceiveMessage (line 54) and onDidChangeVisibility (line 78) only registers those specific listener disposables for tracking -- VS Code does NOT auto-dispose all items in the array on view destruction.

If resolveWebviewView is called again after the view is recreated, the old onStateChange subscription leaks.

Fix: add webviewView.onDidDispose(() => { this.disposables.forEach(d => d.dispose()); this.disposables.length = 0; }).

Comment thread src/extension/services/state-manager.ts Outdated

if (fileName === 'sprint-status.yaml') {
await this.parseSprintStatus(outputRoot);
} else if (filePath.includes('planning-artifacts') && fileName === 'epics.md') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M3: Fragile path matching via String.includes -- Still Open

filePath.includes('planning-artifacts') is a plain substring match. A workspace path containing planning-artifacts in an unrelated segment (e.g., /home/user/planning-artifacts-old/...) would match incorrectly.

Consider comparing against a known base path: filePath.startsWith(path.join(outputRoot, 'planning-artifacts') + path.sep).

return () => {
clearStoryDetail();
};
}, [storyKey, summary?.filePath]); // eslint-disable-line react-hooks/exhaustive-deps
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M4: Stale closure -- suppressed exhaustive-deps -- Still Open

The eslint-disable here masks a real dependency gap. The effect body reads storyDetail?.key (line 61) in the early-return guard, but storyDetail is not in the dependency array. If storyDetail updates to match storyKey after the effect has already run and skipped the fetch, the guard won't re-evaluate, leaving stale data visible.

<li key={ac.number} className="list-decimal">
<span className="font-medium">{ac.title}</span>
{ac.content && (
<pre className="mt-1 text-xs whitespace-pre-wrap text-(--vscode-descriptionForeground)">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M5: Likely invalid Tailwind class syntax -- Still Open

text-(--vscode-descriptionForeground) uses parentheses. Every other occurrence in this file (lines 74, 86, 101, 123, 140, 149, 159, 180, etc.) uses the bracket form: text-[var(--vscode-descriptionForeground)].

This is almost certainly a typo -- the parenthesis form is not valid Tailwind syntax and the style won't be applied.

);

return (
<div role="treeitem">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] M6: File tree ARIA attributes -- Partially Fixed

Good: role="tree", role="treeitem", and role="group" are now present.

Still missing:

  • aria-expanded on directory treeitem elements (should reflect open/closed state)
  • aria-selected on the currently selected file
  • Keyboard navigation (Arrow Up/Down/Left/Right per WAI-ARIA tree pattern)

These are needed for screen reader and keyboard-only users to navigate the tree.

const documentUri = vscode.Uri.joinPath(workspaceFolder.uri, relativePath);

// Prevent path traversal: resolved path must stay within workspace
const resolved = path.resolve(documentUri.fsPath);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] N1: Path traversal fix lacks test coverage -- New Finding

This security guard was added to fix H1 and the implementation is correct. However, editor-panel-provider.test.ts does not test readAndSendDocumentContent at all. A test confirming that ../../etc/passwd (or similar) returns empty content would guard against future regressions on this security boundary.

Vincent M. added 2 commits March 31, 2026 10:13
…/N3)

- Prevent concurrent state mutation by processing file updates sequentially
- Add error handling to command handlers and scanAndSendFileTree
- Add keyboard navigation (arrow keys, Home/End) to overflow menu dropdown
- Fix DashboardViewProvider disposal leak via onDidDispose cleanup
- Normalize path separators before includes() checks for cross-platform reliability
- Fix stale closure in story-detail-view useEffect dependencies
- Add aria-selected and aria-expanded to file tree items
- Cap error array at 50 entries in state-manager collectError
- Use node:path import for consistency
@elvince
Copy link
Copy Markdown
Author

elvince commented Mar 31, 2026

Addressed Remaining Code Review Findings

Fixes 9 of the 21 open items from the automated re-review. The remaining 12 were triaged as not applicable, intentional design decisions, or not worth the churn.

Fixed

Severity ID Fix
HIGH H2 Replaced Promise.all with sequential loop in handleFileChanges to prevent concurrent state mutation
HIGH H3 Wrapped bmad.refresh and bmad.openEditorPanel command handlers in try-catch with showErrorMessage
HIGH H4 Added ArrowUp/ArrowDown/Home/End keyboard navigation and auto-focus to overflow menu dropdown
MEDIUM M2 Added webviewView.onDidDispose cleanup in DashboardViewProvider to prevent listener leaks
MEDIUM M3 Normalized backslashes in handleFileUpdate before includes() path checks
MEDIUM M4 Fixed stale closure in StoryDetailView by extracting storyDetail?.key and adding all deps
MEDIUM M6 Added aria-selected and aria-expanded attributes to file tree items
LOW L14 Added .slice(-50) cap to collectError in state-manager (matching webview stores)
LOW L18/N3 Added try-catch to scanAndSendFileTree, fixed pathnode:path import

Triaged as Won't Fix

ID Reason
M1 VS Code queues postMessage until webview is ready — not a real race condition
M5 No Tailwind typo found in codebase
L3 Returning null for all readFile errors is intentional — callers don't need to distinguish
L4 File content from detectPlanningArtifacts is used downstream on initial load
L5 BmadDetector is stateless; services using it are already in context.subscriptions
L9 Manual lastIndex reset vs matchAll() is a style preference, not a bug
L10, L11, L12 Minor type hygiene / over-engineering for this extension's scope
L13 Partial parse success is intentional per inline comment
L15, L16 Harmless .gitignore boilerplate; glob activation pattern is standard
L17 Webview state persistence not needed — state comes from the extension
N1 Path traversal test already exists in dashboard-view-provider.test.ts
N2 toKebabCase unit tests are nice-to-have, not blocking

Verification

Lint (0 errors), build, and all 706 tests pass.

@wsmoak
Copy link
Copy Markdown
Collaborator

wsmoak commented Apr 2, 2026

I built and installed from the branch and got some screenshots. Claude is no longer speaking to me after YOLO'ing a prd.md and architecture.md and epics.md, so the Kanban board is empty.

bmad-vscode-commands-260401 bmad-vscode-dashboard-260401 bmad-vscode-epics-260401 bmad-vscode-kanban-260401

@bmadcode bmadcode merged commit ff3e33c into bmad-code-org:main Apr 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants