Skip to content

feat: import MyBMAD web dashboard alongside VS Code extension#5

Draft
DevHDI wants to merge 1 commit into
mainfrom
feature/import-mybmad-web-app
Draft

feat: import MyBMAD web dashboard alongside VS Code extension#5
DevHDI wants to merge 1 commit into
mainfrom
feature/import-mybmad-web-app

Conversation

@DevHDI
Copy link
Copy Markdown

@DevHDI DevHDI commented May 9, 2026

Summary

Coordinates the BMAD UI ecosystem under one repository (per @bmadcode's Discord proposal) by importing the standalone Next.js dashboard from DevHDI/my-bmad@81b28ea into web/, side-by-side with the existing VS Code extension under src/.

This PR is intentionally a draft — it asks for approval on the structure before any merge. No src/ file is touched; @evie's extension is unchanged.

Scope (intentionally minimal)

  • Exact verbatim import via git archive HEAD from DevHDI/my-bmad@81b28ea (the relicense-to-MIT commit on main)
  • Zero changes inside src/ — extension build, lint, type-check, tests are unaffected
  • Root eslint.config.mjs ignores web/** so the extension's strict TypeScript rules don't lint the Next.js code with extension-only policies
  • Root .prettierignore ignores web/ for the same reason
  • Root README.md updated with a Repository structure section documenting the side-by-side layout

Out of scope (deferred to follow-up PRs)

  • pnpm workspace / monorepo restructuring
  • A shared parser package extracted from both subprojects (the two implementations cover similar ground but have very different runtimes — VS Code extension vs. multi-user Next.js)
  • CI for the web dashboard
  • Preview deployment / hosting (needs Prisma DB, GitHub OAuth setup)

Note on web/.github/

The import is verbatim, so a nested .github/ folder is included for provenance. GitHub only executes workflows under the repository's top-level .github/workflows, so the nested ones at web/.github/workflows/ are imported as files but do not run as active GitHub Actions.

License

DevHDI/my-bmad was relicensed from AGPL-3.0 to MIT in DevHDI/my-bmad@81b28ea prior to this import, so the entire repository is now MIT-consistent. The imported web/LICENSE carries the standard MIT text.

Verification

Both subprojects pass all checks independently:

Extension (root) — unchanged:

  • pnpm install --frozen-lockfile — clean
  • pnpm typecheck (extension + webview) — clean
  • pnpm test — 706/706
  • pnpm build — produces extension + webview bundles
  • pnpm lint — clean (only the 2 pre-existing no-console warnings; no web/ files scanned)

Web (web/) — newly imported:

  • pnpm install --frozen-lockfile — clean
  • pnpm tsc --noEmit — clean
  • pnpm test — 151/151
  • pnpm build — clean

Open questions for @bmadcode and @evie

  1. Folder name: web/ (proposed), dashboard/, apps/web/?
  2. Workspace: should we plan a follow-up PR introducing a pnpm-workspace.yaml and an apps/ + packages/ layout? Doing it now would be a single big move; doing it later as its own PR keeps this one focused on the import.
  3. Shared parser: extracting a parser package from both src/extension/parsers/ and web/src/lib/bmad/ makes sense long-term, but it's a non-trivial refactor that touches both surfaces. Worth a dedicated discussion before any code change.
  4. Review and merge ownership going forward on this repo.
  5. DevHDI/my-bmad: archive after this lands, or keep it as a public mirror?

Note on review

216 files changed — most of them are an exact import. The history of those files lives at DevHDI/my-bmad and includes 5 already-reviewed-and-merged PRs. Please focus the review on:

  • The 4 root-level changes (README.md, eslint.config.mjs, .prettierignore, the new web/ directory at the top)
  • The folder name and the open questions above

Coordinates the BMAD UI ecosystem under one repository (per BMadCode's
Discord proposal) by importing the standalone Next.js dashboard from
DevHDI/my-bmad@81b28ea into web/, side-by-side with the existing VS
Code extension under src/.

Scope is intentionally minimal:
- Exact verbatim import via `git archive HEAD` from
  DevHDI/my-bmad@81b28ea (the relicense-to-MIT commit on main)
- Zero changes inside src/ — the extension build, lint, type-check,
  and tests are unaffected
- Root eslint.config.mjs ignores web/** so the extension's strict
  TypeScript rules do not lint the Next.js code with extension-only
  policies
- Root .prettierignore ignores web/ for the same reason
- Root README updated with a "Repository structure" section
  documenting the side-by-side layout

Out of scope (deferred to follow-up PRs once Brian and Evie weigh in):
- pnpm workspace / monorepo restructuring
- Shared parser package (each subproject keeps its own parser for now)
- CI for the web dashboard
- Preview deployment / hosting

Note on web/.github/: the import is verbatim, so a nested .github/
folder is included for provenance. GitHub only executes workflows
under the repository's top-level .github/workflows, so the nested
ones do not run.

Verification:
- pnpm install --frozen-lockfile (root): clean
- pnpm typecheck (extension + webview): clean
- pnpm test (extension): 706/706
- pnpm build (extension): produces extension + webview bundles
- pnpm lint (extension): clean (only the 2 pre-existing no-console
  warnings; no web/ files scanned)
- cd web && pnpm install --frozen-lockfile: clean
- cd web && pnpm tsc --noEmit: clean
- cd web && pnpm test: 151/151
- cd web && pnpm build: clean
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df2eba83-35ea-4be3-8c5b-8f391bbfc1d2

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
  • Commit unit tests in branch feature/import-mybmad-web-app

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

@elvince
Copy link
Copy Markdown

elvince commented May 12, 2026

here are my thoughts on this. thanks!

  • Folder name: web/ (proposed), dashboard/, apps/web/?

I would go for apps/web so it will be compliant with the monorepo.

  • Workspace: should we plan a follow-up PR introducing a pnpm-workspace.yaml and an apps/ + packages/ layout?

Doing it now would be a single big move; doing it later as its own PR keeps this one focused on the import.
Yes in another PR

  • Shared parser: extracting a parser package from both src/extension/parsers/ and web/src/lib/bmad/ makes sense long-term, but it's a non-trivial refactor that touches both surfaces. Worth a dedicated discussion before any code change.

Review and merge ownership going forward on this repo.
In my opinion, it should be done right after the monorepo in a shared package

@bmadcode
Copy link
Copy Markdown
Contributor

PR Review: #5

Title: feat: import MyBMAD web dashboard alongside VS Code extension
Author: @DevHDI
Branch: feature/import-mybmad-web-app → main
Review layers: Adversarial + Edge Case Hunter


Findings


1. Static imports hoisted above ALLOW_REGISTRATION env assignment in create-admin script [likely]

Severity: 🔴 Critical
Source: [Both]

web/scripts/create-admin.ts — The script sets process.env.ALLOW_REGISTRATION = "true" on line 9, then uses static import statements for prisma and auth. In Node.js ESM (and TypeScript compiled to ESM), static imports are hoisted and evaluated before any module-level code executes. By the time the assignment runs, the auth module has already initialized with ALLOW_REGISTRATION unset — so the registration gate remains closed and the sign-up call will fail silently in environments where registration is disabled.

Suggested fix:

process.env.ALLOW_REGISTRATION = "true";
// Use dynamic imports so the env var is set before module initialization
const { prisma } = await import("../src/lib/db/client");
const { auth } = await import("../src/lib/auth/auth");

2. Sign-up 422 error reveals whether an email is already registered [likely]

Severity: 🔴 Critical
Source: [Adversarial]

web/src/app/login/login-form.tsx — The error handler returns a distinct user-visible message for HTTP 422 ("Un compte existe déjà avec cet email.") versus any other sign-up failure. This allows an unauthenticated attacker to enumerate registered email addresses by iterating a list and observing response differences. No rate limit is visible in this code path.

Suggested fix: Use a single, ambiguous message for all non-403 sign-up failures — e.g., "Erreur lors de l'inscription." — and do not distinguish "email already taken" from other errors in the client-facing response.


3. Docker CMD starts the application unconditionally after a failed database migration [likely]

Severity: 🔴 Critical
Source: [Both]

web/Dockerfile line 164 — The shell expression prisma migrate deploy || echo 'Warning: migration skipped'; node server.js is parsed as (prisma migrate deploy || echo '...') ; node server.js. The semicolon binds at the statement level, so node server.js runs regardless of whether migrations succeeded. If the database schema is out of sync, the application starts against a mismatched schema, risking runtime errors or data corruption on first query.

Suggested fix:

CMD ["sh", "-c", "prisma migrate deploy && node server.js"]
# Or, to surface migration failures explicitly:
# CMD ["sh", "-c", "prisma migrate deploy || { echo 'Migration failed, aborting'; exit 1; }; node server.js"]

4. No Content-Security-Policy header [likely]

Severity: 🔴 Critical
Source: [Adversarial]

web/next.config.ts — The global headers configuration sets X-Frame-Options, X-Content-Type-Options, and Referrer-Policy, but omits Content-Security-Policy. For an authenticated application that handles session tokens and renders content from external sources (GitHub avatars, user-controlled repo content), the absence of CSP meaningfully broadens the impact of any XSS vector. A baseline policy (default-src 'self', locked-down exceptions for image sources, etc.) should accompany the other security headers.


5. Incomplete path traversal validation — absolute paths and URL-encoded sequences not blocked [likely]

Severity: 🔴 Critical
Source: [Both]

web/src/actions/repo-actions.tsfetchFileContentSchema — The schema refinement blocks .. sequences but does not block absolute paths (/etc/passwd) or URL-percent-encoded traversals (%2e%2e/secret). Relying on LocalProvider to enforce boundaries downstream is not a substitute for validation at the schema layer; a future change to the provider could silently remove that protection.

Suggested fix:

path: z.string().min(1).max(1024).trim()
  .refine((p) => {
    const decoded = decodeURIComponent(p);
    return (
      !decoded.startsWith("/") &&
      !decoded.includes("\0") &&
      !/(?:^|[\\/])\.\.(?:[\\/]|$)/.test(decoded)
    );
  }, { message: "Invalid path" }),

6. Cache tag invalidated before GitHub fetch succeeds in refreshGitHubRepo [likely]

Severity: 🔴 Critical
Source: [Edge Case]

web/src/actions/repo-actions.tsrefreshGitHubReporevalidateTag(...) is called unconditionally before the octokit.rest.git.getTree fetch. If that fetch throws (network error, rate limit, 404), the cache is already invalidated: the next render cycle fetches stale or empty data with no recovery path until a future successful refresh.

Suggested fix: Move revalidateTag to after the fetch and DB write succeed:

const { data: tree } = await octokit.rest.git.getTree({ ... });
await prisma.repo.update({ ... });
revalidateTag(repoTag(input.owner, input.name)); // only on success
return { success: true, ... };

7. listRepoBranches makes unscoped GitHub API call when repo does not belong to the requesting user [likely]

Severity: 🔴 Critical
Source: [Edge Case]

web/src/actions/repo-actions.tslistRepoBranches — After the local-repo guard, if repoConfig is null (repo does not exist in the dashboard or belongs to a different user), the function falls through and issues a GitHub listBranches call using owner/name values taken directly from user input. An authenticated user can supply arbitrary owner/name values to enumerate branches of repositories they do not own within the dashboard.

Suggested fix:

if (!repoConfig) {
  return { success: false, error: "Repository not found", code: "NOT_FOUND" };
}
if (repoConfig.sourceType === "local") {
  return { success: false, error: "Branch management is not available for local projects", code: "NOT_APPLICABLE" };
}
// ... proceed with GitHub call

8. Malformed JSON body causes unhandled exception (500) in revalidate route [likely]

Severity: 🔴 Critical
Source: [Edge Case]

web/src/app/api/revalidate/route.tsawait request.json() is not wrapped in a try/catch. A request with a malformed body (empty body, wrong Content-Type, truncated payload) throws a SyntaxError that is not caught, resulting in a 500 instead of a 400. This also bypasses any error-monitoring middleware that expects structured error responses.

Suggested fix:

let body: unknown;
try {
  body = await request.json();
} catch {
  return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 });
}
const parsed = bodySchema.safeParse(body);

9. revalidateTag called with a spurious second argument [likely]

Severity: 🟡 Moderate
Source: [Adversarial]

web/src/actions/repo-actions.ts — Multiple call sites pass "default" as a second argument to revalidateTag (e.g., revalidateTag(repoTag(input.owner, input.name), "default")). Next.js revalidateTag accepts exactly one argument; the second is silently ignored at runtime. If the intent is to also invalidate the "default" tag, a separate revalidateTag("default") call is needed. As written, the call is misleading and may mask a caching bug where "default" is never actually invalidated.


10. In-memory rate limiter does not survive restarts or horizontal scaling [likely]

Severity: 🟡 Moderate
Source: [Adversarial]

web/src/actions/repo-actions.tscheckRateLimit is backed by process memory. Every container restart, rolling deploy, crash loop, or new pod instance resets all rate limit state. In a containerized environment (which this Dockerfile implies), this limit is trivially bypassed. This is a known MVP tradeoff, but it is not documented anywhere in the codebase, which means the next developer will not know it is intentional.

Suggested fix: At minimum, add a comment noting the limitation and the intent to replace with a persistent store (e.g., Redis) before production scale-out. At maximum, use a shared cache for rate limit state.


11. .env.example ships with a realistic-looking dev database password [likely]

Severity: 🟡 Moderate
Source: [Adversarial]

web/.env.example line 19DATABASE_URL contains bmad_dev_password — a real-looking credential. Example files with plausible passwords have a consistent track record of being copied verbatim into production. The BETTER_AUTH_SECRET entry correctly leaves the value empty and instructs the developer to generate one. The same pattern should apply to the database password: use REPLACE_ME, <your-db-password>, or any clearly invalid placeholder.


12. role field is an unconstrained String — no database-level enforcement of valid values [likely]

Severity: 🟡 Moderate
Source: [Adversarial]

web/prisma/schema.prisma line 215role String @default("user") allows any string value at the database level. A direct DB write, a future missed validation branch, or a scripting error could insert values like "superadmin", "", or any arbitrary string. Switching to a Prisma enum enforces valid values at both the ORM and database (PostgreSQL CHECK constraint) levels and eliminates this class of privilege-escalation risk.

Suggested fix:

enum UserRole {
  user
  admin
}

model User {
  role  UserRole  @default(user)
  ...
}

13. Revalidate endpoint has no rate limit of its own [likely]

Severity: 🟡 Moderate
Source: [Adversarial]

web/src/app/api/revalidate/route.ts — Authentication relies on a static shared secret. If the secret is obtained or brute-forced, an attacker can issue unlimited revalidateTag calls, forcing continuous cache invalidation and elevating upstream load. A simple rate limit on this endpoint — even the same in-memory implementation used elsewhere — would meaningfully raise the bar.


14. detectBmadRepos accepts an unbounded input array [likely]

Severity: 🟡 Moderate
Source: [Edge Case]

web/src/actions/repo-actions.tsdetectBmadRepos — No maximum is enforced on repoIds.length. A caller with a valid session can pass thousands of repo IDs, causing ceil(N/30) sequential GraphQL requests against the GitHub API, exhausting the user's rate-limit quota and potentially blocking the event loop for multiple seconds.

Suggested fix:

const MAX_REPOS = 300;
if (repoIds.length > MAX_REPOS) {
  return { success: false, error: `Too many repos (max ${MAX_REPOS})`, code: "LIMIT_EXCEEDED" };
}

15. extendBmadDirs failure is silently swallowed in fetchFileContent [likely]

Severity: 🟡 Moderate
Source: [Edge Case]

web/src/actions/repo-actions.tsfetchFileContent (local path, lines ~1271–1278) — When provider.extendBmadDirs(topSegment) throws, the catch block discards the error and continues to provider.getFileContent(requestedPath). The comment assumes getFileContent will independently enforce the path restriction, but this is a behavioral assumption rather than a guarantee. If the provider's allow-list was not extended and getFileContent does not independently validate, a file outside the intended bmad directories may be returned. The silent catch also removes any observability into failures.

Suggested fix:

try {
  provider.extendBmadDirs(topSegment);
} catch (err) {
  console.warn("extendBmadDirs failed for segment:", topSegment, err);
  return { success: false, error: sanitizeError(null, "CONFIG_ERROR"), code: "CONFIG_ERROR" };
}

16. detectBmadRepos batch failure is indistinguishable from "no BMAD repos found" [likely]

Severity: 🟢 Minor
Source: [Adversarial]

web/src/actions/repo-actions.tsdetectBmadRepos — A failed GraphQL batch silently sets all repos in that chunk to false with only a console.warn. From the user's perspective, their BMAD repositories silently disappear. Returning null or a sentinel for "unknown" instead of false for "confirmed negative" would allow the UI to distinguish an error state from a clean absence, and surface the failure to the user.


17. Hardcoded parsingErrorRate: 0 presents unimplemented metric as real data [likely]

Severity: 🟢 Minor
Source: [Edge Case]

web/src/actions/admin-actions.tsgetUsageMetrics line ~560 — The metric is permanently 0 with no indication in the UI that tracking is not yet implemented. Admins may interpret 0% as "no parsing errors" when the data simply does not exist. The value should be null and the UI should render "N/A — not yet tracked" to avoid presenting a placeholder as an operational signal.


18. Redundant admin authentication at RSC level and in downstream server actions [likely]

Severity: 🟢 Minor
Source: [Adversarial]

web/src/app/(dashboard)/admin/page.tsx — The page performs getAuthenticatedSession() + role check and redirects before the server actions are ever invoked in this flow. Each action (getUsers, getUsageMetrics) then calls requireAdmin() again internally. In this call path the second check is dead code. While defense-in-depth at the action layer is generally good practice, the dual pattern without documentation creates ambiguity about where the authoritative auth boundary lives and may mislead future developers who call these actions from other callsites.


Summary

Critical: 8 | Moderate: 7 | Minor: 3
Sources: 5 adversarial | 5 edge case | 6 both

Note: No binary files were detected in this PR. The bulk of the 216 changed files are the full import of the web/ Next.js application. This review focused on authentication, authorization, server actions, database schema, and infrastructure configuration — the highest-risk surface areas for a new application import.


Review generated by Raven's Verdict. LLM-produced analysis — findings may be incorrect or lack context. Verify before acting.

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.

3 participants