sdd-webapp: adopt shadcn/ui + tailwind, show archived, fix navigation#12
sdd-webapp: adopt shadcn/ui + tailwind, show archived, fix navigation#12
Conversation
WalkthroughAdds Tailwind/ShadCN UI primitives, client fetch/routes and utilities, tree/status helpers, archive-aware status flows (client + server + parser), project-path validation and atomic config writes, MCP helpers, type extensions, and multiple UI/layout refactors. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Web Client
participant UI as Components/Hooks
participant API as fetchJson / apiRoutes
participant Server as Node Server
participant Parser as status-parser
participant FS as Filesystem
Browser->>UI: request project status (includeArchived=true)
UI->>API: fetchJson GET apiRoutes.projectStatus(id,{includeArchived:true})
API->>Server: HTTP GET /api/projects/:id/status?includeArchived=1
Server->>Parser: parseStatusFromSpecs(specsPath,{includeArchived:true})
Parser->>FS: read specs dir and optional `_archived` dir
FS-->>Parser: files & status.md contents
Parser-->>Server: StatusSummary + tasks (with isArchived flags)
Server-->>API: JSON response
API-->>UI: parsed JSON
UI-->>Browser: render tasks list, archived count, and UI state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📚 Learning: 2025-12-18T06:09:50.220ZApplied to files:
📚 Learning: 2025-12-18T06:16:40.499ZApplied to files:
📚 Learning: 2025-12-19T03:50:44.474ZApplied to files:
📚 Learning: 2025-12-19T03:50:44.474ZApplied to files:
🔇 Additional comments (7)
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: 7
🧹 Nitpick comments (16)
packages/sdd-webapp/src/lib/project-path.ts (1)
20-24: Consider usingrealpathfor consistency with specs-scanner.The validation function uses
access()to check existence, but doesn't resolve symlinks. In contrast,specs-scanner.ts(lines 51-76) usesrealpathSync()to resolve symlinks and perform security checks. While not immediately problematic, usingrealpathhere would ensure consistent path resolution across the codebase and catch broken symlinks early.Also applies to: 26-31
packages/sdd-webapp/package.json (1)
52-53: Consider ifautoprefixerandpostcssare necessary with Tailwind v4.Tailwind CSS v4 includes built-in Lightning CSS integration that handles vendor prefixes automatically. Since you're using
@tailwindcss/vitewith Tailwind v4, these may be redundant unless explicitly required by other tooling.packages/sdd-webapp/src/client/lib/tree.ts (1)
47-62: Consider extracting shared traversal logic.
findDirectoryChildrenByPathandfindTaskFileTreeshare similar recursive patterns. A generic helper could reduce duplication:🔎 Optional: Generic tree search helper
function findDirectory( nodes: FileNode[], predicate: (node: FileNode) => boolean, ): FileNode | null { for (const node of nodes) { if (node.type !== "directory") continue; if (predicate(node)) return node; if (node.children) { const found = findDirectory(node.children, predicate); if (found) return found; } } return null; } // Usage: export function findTaskFileTree(nodes: FileNode[], taskName: string): FileNode[] { const dir = findDirectory(nodes, (n) => n.name === taskName); return dir?.children ?? []; } export function findDirectoryChildrenByPath(nodes: FileNode[], dirPath: string): FileNode[] { const dir = findDirectory(nodes, (n) => n.path === dirPath); return dir?.children ?? []; }packages/sdd-webapp/src/mcp-server.ts (1)
177-187: Consider wrappingremoveProjectin try/catch for consistency.Other tool handlers (
sdd_webapp_add_project,sdd_webapp_list_projects,sdd_webapp_get_status) wrap their main operations in try/catch blocks. TheremoveProjectcall could throw if there's a filesystem issue when saving the updated projects list.🔎 Proposed fix
case "sdd_webapp_remove_project": { if (!isRemoveProjectArgs(args)) { return mcpError("Error: Invalid arguments. 'id' is required."); } + try { const success = await removeProject(args.id); if (!success) { return mcpError(`Error: Project not found: ${args.id}`); } return mcpText(`Project removed: ${args.id}`); + } catch (error) { + return mcpError( + `Error removing project: ${error instanceof Error ? error.message : String(error)}`, + ); + } }packages/sdd-webapp/vite.config.ts (1)
9-13:__dirnameworks in Vite config files, but consider the explicit ESM pattern for clarity.Vite replaces
__dirnameandimport.meta.urlin config files, so the current implementation functions correctly. However, for explicit ESM compatibility and clarity, the recommended pattern is to define__dirnameusingimport { fileURLToPath } from 'node:url'withimport.meta.url. Consider updating to:import { dirname } from "node:path"; import { fileURLToPath } from "node:url"; const __dirname = dirname(fileURLToPath(import.meta.url));This makes the ESM compatibility explicit and removes reliance on Vite's internal transformation.
packages/sdd-webapp/src/client/lib/status.ts (1)
5-17: Consider explicit handling for "不明" status.The
TaskStatustype includes"不明"which isn't explicitly handled in the switch. While it falls through to the default case safely, explicit handling would improve code clarity and catch any future status additions at compile time if you use exhaustive switch checking.🔎 Optional: Add explicit case for "不明"
export function getStatusDotClasses(status: Status): string { switch (status) { case "進行中": return "bg-primary"; case "完了": return "bg-emerald-600"; case "却下": return "bg-destructive"; case "未着手": + case "不明": return "bg-muted-foreground"; default: return "bg-muted-foreground"; } }packages/sdd-webapp/src/client/components/SpecViewer.tsx (1)
25-27: Consider extending TOC indentation for deeper heading levels.Currently only level 3 headings receive
pl-3indentation. If your specs contain h4-h6 headings (whichSpecContent.tocsupports per the type definition), they won't have progressive indentation.🔎 Optional: Add progressive indentation
<li key={item.id} - className={item.level === 3 ? "pl-3" : undefined} + className={item.level > 2 ? `pl-${(item.level - 2) * 3}` : undefined} >Note: This uses dynamic class names. For Tailwind to include these classes, you may need to safelist them or use a mapping object instead.
packages/sdd-webapp/src/lib/config.ts (1)
94-102: Good atomic write implementation, consider cleanup on failure.The atomic write pattern (write to temp → rename) is correct and prevents data corruption from partial writes. However, if
writeFilefails, the temp file remains on disk.🔎 Optional: Clean up temp file on failure
+import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; async function writeJsonAtomic( filePath: string, value: unknown, ): Promise<void> { const dir = dirname(filePath); const tmpPath = join(dir, `${basename(filePath)}.${randomUUID()}.tmp`); - await writeFile(tmpPath, JSON.stringify(value, null, 2)); - await rename(tmpPath, filePath); + try { + await writeFile(tmpPath, JSON.stringify(value, null, 2)); + await rename(tmpPath, filePath); + } catch (err) { + await unlink(tmpPath).catch(() => {}); + throw err; + } }packages/sdd-webapp/src/client/components/ProjectSelector.tsx (2)
107-116: Consider clearing form fields on cancel for consistent UX.When the user cancels, the error is cleared but
newPathandnewNameretain their values. This could be intentional (preserving user input), but clearing them would provide a fresh start when reopening.🔎 Optional: Clear fields on cancel
<Button type="button" variant="secondary" onClick={() => { setShowAddForm(false); setError(null); + setNewPath(""); + setNewName(""); }} > Cancel </Button>
92-101: Consider adding keyboard submission support.Users may expect to press Enter to submit the form. Currently, form submission requires clicking the Add button.
🔎 Optional: Add Enter key handler
<Input placeholder="Project path (e.g., /path/to/project)" value={newPath} onChange={(e) => setNewPath(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleAdd()} /> <Input placeholder="Project name (optional)" value={newName} onChange={(e) => setNewName(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleAdd()} />Alternatively, wrap the inputs in a
<form>element withonSubmit.packages/sdd-webapp/src/client/App.tsx (1)
51-58: Consider showing both errors when they co-exist.When both
projectsErrorandspecsErrorare present, onlyprojectsErroris displayed due to the short-circuit evaluation. Consider showing both if they can occur simultaneously.Proposed fix
{(projectsError || specsError) && ( <div className="border-b border-destructive/20 bg-destructive/10 px-6 py-3 text-sm text-destructive" role="alert" > - {projectsError || specsError} + {[projectsError, specsError].filter(Boolean).join(" | ")} </div> )}packages/sdd-webapp/src/client/components/FileTree.tsx (1)
58-60: Consider using icon components instead of emoji.The emoji icons (📂, 📁, 📄) work but may render inconsistently across platforms. Consider using
lucide-reacticons (e.g.,Folder,FolderOpen,File) for visual consistency with the shadcn/ui design system.packages/sdd-webapp/src/client/components/TaskDetailView.tsx (1)
135-139: Consider simplifying the ref assignment.The ref callback pattern works but is more verbose than necessary. You can pass
mainScrollRefdirectly to therefprop.🔎 Proposed simplification
<main - ref={(node) => { - mainScrollRef.current = node; - }} + ref={mainScrollRef} className="min-h-0 flex-1 overflow-y-auto bg-muted/30" >packages/sdd-webapp/src/client/hooks/useSpecs.ts (1)
68-75: Consider optimizing refetch behavior whenincludeArchivedchanges.When
includeArchivedchanges,fetchStatusidentity changes, triggering this effect and calling bothfetchFileTree()andfetchStatus(). However, only status needs refetching since the file tree doesn't depend on the archive filter. This is minor overhead but could be optimized by separating the effects.packages/sdd-webapp/src/client/components/Dashboard.tsx (2)
42-59: Consider showing both counts when archived is enabled.When
includeArchivedis true, the "Archived" card replaces "Not Started". Users lose visibility of the not-started count. Consider showing both metrics or adding a fifth card when archived view is enabled, depending on UX requirements.
136-150: Consider extracting phase status color logic to a utility.The phase indicator color logic duplicates patterns from
getStatusDotClasses. For consistency, consider creating a utility function (e.g.,getPhaseStatusClasses) inlib/status.ts.🔎 Example utility
In
packages/sdd-webapp/src/client/lib/status.ts:export function getPhaseStatusBarClasses(status: PhaseStatus["status"]): string { switch (status) { case "完了": return "bg-emerald-600"; case "進行中": return "bg-primary"; default: return "bg-muted"; } }Then in Dashboard.tsx:
-className={`h-1.5 w-6 rounded-full ${ - phase.status === "完了" - ? "bg-emerald-600" - : phase.status === "進行中" - ? "bg-primary" - : "bg-muted" -}`} +className={`h-1.5 w-6 rounded-full ${getPhaseStatusBarClasses(phase.status)}`}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
packages/sdd-webapp/biome.json(1 hunks)packages/sdd-webapp/components.json(1 hunks)packages/sdd-webapp/package.json(1 hunks)packages/sdd-webapp/src/client/App.tsx(4 hunks)packages/sdd-webapp/src/client/components/Dashboard.tsx(2 hunks)packages/sdd-webapp/src/client/components/FileTree.tsx(3 hunks)packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/components/SpecViewer.tsx(1 hunks)packages/sdd-webapp/src/client/components/TaskDetailView.tsx(2 hunks)packages/sdd-webapp/src/client/components/ui/button.tsx(1 hunks)packages/sdd-webapp/src/client/components/ui/input.tsx(1 hunks)packages/sdd-webapp/src/client/components/ui/popover.tsx(1 hunks)packages/sdd-webapp/src/client/hooks/useProjects.ts(3 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(6 hunks)packages/sdd-webapp/src/client/index.html(0 hunks)packages/sdd-webapp/src/client/lib/api.ts(1 hunks)packages/sdd-webapp/src/client/lib/routes.ts(1 hunks)packages/sdd-webapp/src/client/lib/status.ts(1 hunks)packages/sdd-webapp/src/client/lib/tree.ts(1 hunks)packages/sdd-webapp/src/client/lib/utils.ts(1 hunks)packages/sdd-webapp/src/client/main.tsx(1 hunks)packages/sdd-webapp/src/client/styles.css(1 hunks)packages/sdd-webapp/src/client/tailwind.config.js(1 hunks)packages/sdd-webapp/src/lib/config.ts(4 hunks)packages/sdd-webapp/src/lib/markdown.ts(4 hunks)packages/sdd-webapp/src/lib/mcp.ts(1 hunks)packages/sdd-webapp/src/lib/project-path.ts(1 hunks)packages/sdd-webapp/src/lib/status-parser.ts(6 hunks)packages/sdd-webapp/src/mcp-server.ts(3 hunks)packages/sdd-webapp/src/server.ts(6 hunks)packages/sdd-webapp/src/types.ts(2 hunks)packages/sdd-webapp/tsconfig.json(1 hunks)packages/sdd-webapp/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/sdd-webapp/src/client/index.html
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/lib/project-path.tspackages/sdd-webapp/src/types.tspackages/sdd-webapp/src/lib/mcp.tspackages/sdd-webapp/src/client/lib/tree.tspackages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/src/lib/markdown.tspackages/sdd-webapp/src/client/lib/routes.tspackages/sdd-webapp/src/client/lib/status.tspackages/sdd-webapp/vite.config.tspackages/sdd-webapp/src/client/hooks/useSpecs.tspackages/sdd-webapp/src/lib/status-parser.tspackages/sdd-webapp/src/client/lib/utils.tspackages/sdd-webapp/src/lib/config.tspackages/sdd-webapp/src/mcp-server.tspackages/sdd-webapp/src/client/lib/api.tspackages/sdd-webapp/src/server.ts
🧠 Learnings (13)
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/src/client/main.tsxpackages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/vite.config.tspackages/sdd-webapp/src/client/components/SpecViewer.tsxpackages/sdd-webapp/src/client/hooks/useSpecs.tspackages/sdd-webapp/package.jsonpackages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/lib/config.tspackages/sdd-webapp/src/client/App.tsxpackages/sdd-webapp/src/server.tspackages/sdd-webapp/components.json
📚 Learning: 2025-12-18T06:10:43.172Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:21-21
Timestamp: 2025-12-18T06:10:43.172Z
Learning: In packages/sdd-webapp/package.json, the "typecheck" script uses `tsgo --noEmit`, where `tsgo` is the TypeScript 7 native CLI from the `typescript/native-preview` package (Microsoft's TypeScript native port, codename "Corsa"). This is intentional for faster type checking and is not a typo of `tsc`.
Applied to files:
packages/sdd-webapp/src/client/main.tsxpackages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/lib/config.tspackages/sdd-webapp/src/server.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Use 'type' for TypeScript type definitions; use 'interface' only when there is a clear reason such as extending existing library types, and document the reason in a comment
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/lib/config.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Avoid creating tables in markdown unnecessarily
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Do not add numbers to markdown headings
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/lib/status.ts
📚 Learning: 2025-12-18T06:10:43.172Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:21-21
Timestamp: 2025-12-18T06:10:43.172Z
Learning: For package.json files that define a type-check script, prefer using tsgo --noEmit (TypeScript native preview CLI from typescript/native-preview) for faster type checking. This should be documented as intentional to avoid confusion with tsc, and the script should be consistent across similar packages where a native TS preview is desired. If a project does not use the native preview, ensure the rationale is clearly explained in comments or repo docs.
Applied to files:
packages/sdd-webapp/package.json
📚 Learning: 2025-12-19T03:50:04.190Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:04.190Z
Learning: Run TypeScript type checking using `bun run typecheck` within each plugin directory before committing
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Prohibit barrel imports and exports
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:04.190Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:04.190Z
Learning: Applies to **/*.ts : Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Write detailed TSDoc in Japanese including purpose, content, and precautions
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Write clear comments in Japanese when code has any complexity to reduce cognitive load
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Use Biome v2 as linter and formatter for new projects
Applied to files:
packages/sdd-webapp/biome.json
🧬 Code graph analysis (16)
packages/sdd-webapp/src/client/components/ui/input.tsx (1)
packages/sdd-webapp/src/client/lib/utils.ts (1)
cn(4-6)
packages/sdd-webapp/src/lib/project-path.ts (1)
packages/sdd-webapp/src/lib/specs-scanner.ts (2)
readSpecFile(52-77)scanSpecsDirectory(5-12)
packages/sdd-webapp/src/client/components/ui/button.tsx (1)
packages/sdd-webapp/src/client/lib/utils.ts (1)
cn(4-6)
packages/sdd-webapp/src/client/lib/tree.ts (1)
packages/sdd-webapp/src/types.ts (1)
FileNode(3-9)
packages/sdd-webapp/src/client/hooks/useProjects.ts (3)
packages/sdd-webapp/src/client/lib/api.ts (1)
fetchJson(3-21)packages/sdd-webapp/src/types.ts (1)
Project(42-48)packages/sdd-webapp/src/client/lib/routes.ts (1)
apiRoutes(8-20)
packages/sdd-webapp/src/client/lib/status.ts (1)
packages/sdd-webapp/src/types.ts (2)
TaskStatus(24-30)PhaseStatus(18-22)
packages/sdd-webapp/src/client/components/Dashboard.tsx (3)
packages/sdd-webapp/src/types.ts (2)
StatusSummary(32-40)TaskStatus(24-30)packages/sdd-webapp/src/client/lib/status.ts (1)
getStatusDotClasses(5-18)packages/sdd-webapp/src/client/components/ui/button.tsx (1)
Button(57-57)
packages/sdd-webapp/src/client/components/ui/popover.tsx (1)
packages/sdd-webapp/src/client/lib/utils.ts (1)
cn(4-6)
packages/sdd-webapp/src/client/components/SpecViewer.tsx (1)
packages/sdd-webapp/src/types.ts (1)
SpecContent(11-16)
packages/sdd-webapp/src/client/hooks/useSpecs.ts (3)
packages/sdd-webapp/src/client/lib/api.ts (1)
fetchJson(3-21)packages/sdd-webapp/src/types.ts (3)
FileNode(3-9)SpecContent(11-16)StatusSummary(32-40)packages/sdd-webapp/src/client/lib/routes.ts (1)
apiRoutes(8-20)
packages/sdd-webapp/src/lib/status-parser.ts (1)
packages/sdd-webapp/src/types.ts (1)
TaskStatus(24-30)
packages/sdd-webapp/src/client/components/TaskDetailView.tsx (4)
packages/sdd-webapp/src/client/lib/tree.ts (4)
findDirectoryChildrenByPath(47-62)isPathWithinDir(64-66)findFirstFileByName(33-45)countFiles(23-31)packages/sdd-webapp/src/client/lib/status.ts (1)
getStatusBadgeClasses(20-33)packages/sdd-webapp/src/client/components/FileTree.tsx (1)
FileTree(12-25)packages/sdd-webapp/src/client/components/SpecViewer.tsx (1)
SpecViewer(7-43)
packages/sdd-webapp/src/lib/config.ts (1)
packages/sdd-webapp/src/types.ts (2)
Project(42-48)Config(50-52)
packages/sdd-webapp/src/mcp-server.ts (3)
packages/sdd-webapp/src/lib/mcp.ts (2)
mcpError(7-9)mcpText(3-5)packages/sdd-webapp/src/lib/project-path.ts (1)
validateProjectPath(8-34)packages/sdd-webapp/src/lib/config.ts (4)
addProject(31-54)loadProjects(15-24)removeProject(56-65)getProject(67-70)
packages/sdd-webapp/src/server.ts (3)
packages/sdd-webapp/src/lib/project-path.ts (1)
validateProjectPath(8-34)packages/sdd-webapp/src/lib/config.ts (1)
addProject(31-54)packages/sdd-webapp/src/lib/status-parser.ts (1)
parseStatusFromSpecs(5-74)
packages/sdd-webapp/src/client/components/FileTree.tsx (2)
packages/sdd-webapp/src/client/components/ui/button.tsx (1)
Button(57-57)packages/sdd-webapp/src/client/lib/utils.ts (1)
cn(4-6)
🪛 ast-grep (0.40.0)
packages/sdd-webapp/src/client/components/SpecViewer.tsx
[warning] 13-13: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (53)
packages/sdd-webapp/src/lib/markdown.ts (3)
16-24: LGTM! Clean collision-safe slug generation.The slugger correctly handles duplicate headings by tracking counts and appending suffixes. The fallback to
"section"for empty slugs is a good defensive choice.
26-35: LGTM! Proper Unicode normalization for Japanese headings.The use of NFKC normalization and Unicode property escapes (
\p{L},\p{N}) correctly handles Japanese and other non-ASCII characters, which aligns with the PR objective to support Japanese headings.
72-93: LGTM! Robust lexer-based TOC extraction.The lexer-based approach is more reliable than regex parsing. The independent slugger instance is acceptable since both
renderMarkdownandextractTableOfContentsprocess the same content in the same order, ensuring consistent ID generation.packages/sdd-webapp/biome.json (1)
21-25: LGTM! Proper Biome CSS configuration for Tailwind.The CSS parser configuration correctly enables Tailwind directive support, which is essential for parsing
@import "tailwindcss"and other Tailwind-specific CSS directives in the new styling system.packages/sdd-webapp/tsconfig.json (1)
16-18: LGTM! Path alias aligns with shadcn/ui conventions.The
@/*path alias mapping to./src/client/*enables cleaner imports throughout the client codebase and follows the standard shadcn/ui project structure.packages/sdd-webapp/src/client/lib/utils.ts (1)
1-6: LGTM! Standard shadcn/ui utility implementation.The
cnutility correctly combinesclsxfor conditional class handling andtwMergefor Tailwind-specific class merging, following the recommended shadcn/ui pattern.packages/sdd-webapp/src/types.ts (1)
26-26: LGTM! Type updates properly support archived task feature.The new fields correctly enable the archived task functionality:
pathprovides task location for navigationisArchivedoptional flag maintains backward compatibilityarchivedcounter supports summary statisticsAlso applies to: 29-29, 38-38
packages/sdd-webapp/src/lib/mcp.ts (1)
3-9: LGTM! MCP response helpers follow SDK conventions.The helper functions correctly construct MCP
CallToolResultpayloads, centralizing response formatting and ensuring consistency across tool responses.packages/sdd-webapp/src/client/lib/api.ts (1)
3-21: LGTM! Robust generic fetch utility with proper error handling.The implementation correctly:
- Validates responses and throws on errors
- Attempts to extract meaningful error messages from JSON bodies
- Gracefully handles JSON parse failures with a catch block
- Provides type-safe responses through generics
packages/sdd-webapp/src/client/components/ui/input.tsx (1)
5-22: LGTM! Standard shadcn/ui Input component implementation.The component correctly:
- Uses
React.forwardRefwith proper typing forHTMLInputElement- Merges className props using the
cnutility- Sets
displayNamefor better debugging- Follows the standard shadcn/ui component pattern
packages/sdd-webapp/package.json (1)
28-56: LGTM on the overall dependency additions.The shadcn/ui stack (Radix UI primitives, class-variance-authority, clsx, tailwind-merge, lucide-react) is well-chosen and follows established patterns. Based on learnings, the
@typescript/native-previewusage is intentional for faster type checking.packages/sdd-webapp/src/client/lib/tree.ts (4)
3-21: LGTM: Clean recursive tree traversal.The implementation correctly handles the recursive search for a task directory and returns its children. The early return pattern is efficient.
23-31: LGTM: Simple and correct file counting.The recursive counting logic handles both files and directories appropriately.
33-45: LGTM: Depth-first search for file by name.Clean implementation that returns the first match found.
64-66: LGTM: Path containment check.Simple and correct for normalized paths. The trailing slash ensures proper directory boundary matching.
packages/sdd-webapp/src/mcp-server.ts (3)
14-15: LGTM: Good use of centralized helpers.The
mcpTextandmcpErrorhelpers standardize MCP responses and reduce boilerplate across all tool handlers.
143-156: LGTM: Proper validation before project addition.Using
validateProjectPathensures consistent validation (path exists, is absolute, has specs/) before attempting to add the project.
189-217: LGTM: Clean status retrieval and error handling.The status handler properly validates arguments, checks project existence, and handles parsing errors appropriately.
packages/sdd-webapp/src/client/main.tsx (1)
4-5: LGTM: Consolidating CSS imports in the JS bundle.Moving the highlight.js theme import from HTML to the JS entry point is a clean approach for Vite-based builds. The import order (styles.css before highlight.js) allows your custom styles to take precedence if needed.
packages/sdd-webapp/vite.config.ts (2)
2-3: LGTM: Correct Tailwind v4 integration.Using
@tailwindcss/viteis the recommended approach for Tailwind CSS v4 with Vite.
6-27: LGTM: Well-structured Vite configuration.The configuration properly sets up:
- Tailwind v4 plugin with correct ordering
- Path alias for cleaner imports
- Build output directory
- Development server with API proxy
packages/sdd-webapp/src/client/components/ui/button.tsx (1)
1-57: LGTM!This is a well-structured shadcn/ui Button component following established patterns. The use of
class-variance-authorityfor variant management,forwardReffor ref forwarding, and theasChildprop via RadixSlotare all best practices for composable UI components.packages/sdd-webapp/components.json (1)
1-22: LGTM!The shadcn/ui configuration is properly set up with
rsc: falsefor client-side rendering, correct paths to the Tailwind config and CSS files, and appropriate path aliases that align with the project structure.packages/sdd-webapp/src/client/tailwind.config.js (1)
1-47: LGTM!The Tailwind configuration follows the standard shadcn/ui pattern with CSS variable-based theming. The HSL color system and border radius customizations align well with the design tokens defined in
styles.css.packages/sdd-webapp/src/client/styles.css (3)
1-8: Good adoption of Tailwind v4 CSS-first configuration.The use of
@import "tailwindcss",@plugin,@custom-variant, and@theme inlinefollows the Tailwind v4 syntax correctly. This is the modern approach for CSS-first configuration.
50-117: Well-structured theming with OKLCH colors.The OKLCH color space provides perceptually uniform colors with better gamut coverage. Both light and dark themes are comprehensively defined with consistent token naming.
128-135: Good accessibility improvement for anchor navigation.The
scroll-margin-top: 6remon prose headings ensures anchored headings aren't hidden behind sticky headers when navigating via TOC links.packages/sdd-webapp/src/lib/config.ts (1)
19-20: Good defensive parsing with runtime type validation.Validating that
parsedis an array before casting prevents runtime errors from malformed JSON files. The same pattern inloadConfig(lines 76-79) correctly validates for object type.packages/sdd-webapp/src/client/components/ProjectSelector.tsx (1)
47-64: LGTM on the async form handling.Good implementation with
submittingstate to prevent double-submission, proper error handling, and cleanup infinally. The form fields are correctly cleared on successful submission.packages/sdd-webapp/src/client/components/SpecViewer.tsx (1)
11-15: Server-side HTML sanitization is properly implemented.The markdown HTML is sanitized server-side via the
sanitizeHtml()function inpackages/sdd-webapp/src/lib/markdown.tsbefore being sent to the client. TherenderMarkdown()function applies sanitization immediately after parsing markdown with marked, removing script tags, event handlers, and dangerous protocols. The biome-ignore comment is justified and the implementation is secure.Likely an incorrect or invalid review comment.
packages/sdd-webapp/src/client/App.tsx (2)
59-90: Clean header implementation with good accessibility.The sticky header with backdrop blur, semantic
<header>element, and proper button usage for the back navigation breadcrumb is well-structured. The use oftype="button"on the Button component is a good practice.
109-115: Props wiring for archived toggle looks correct.The
includeArchivedstate andsetIncludeArchivedare properly passed to the Dashboard component for controlling archived task visibility.packages/sdd-webapp/src/client/lib/routes.ts (2)
1-6: Well-implemented path encoding utility.The
encodePathPreservingSlashesfunction correctly handles encoding each path segment while preserving the directory structure. This prevents issues with special characters in filenames while maintaining valid URL paths.
8-20: Clean centralized route builders.The
apiRoutesobject provides a type-safe, centralized approach to API route construction. Good use ofencodeURIComponentfor IDs and the custom encoder for paths. Theas constassertion ensures immutability.packages/sdd-webapp/src/client/hooks/useProjects.ts (1)
12-29: Good error handling pattern in fetchProjects.The try/catch/finally structure with
setError(null)reset, error message extraction, and loading state management is well implemented.packages/sdd-webapp/src/client/components/FileTree.tsx (2)
46-62: Good Button-based implementation with proper variant handling.The Button component usage with conditional variants (
secondaryfor active,ghostotherwise) and thecnutility for class merging follows shadcn/ui patterns correctly. The archived styling with muted foreground and italic is a nice touch.
63-77: Recursive directory rendering is correct.The conditional rendering of children with proper nested
<ul>structure and recursiveFileTreeItemusage maintains semantic HTML and correct indentation viaml-4.packages/sdd-webapp/src/client/components/ui/popover.tsx (1)
1-31: Standard shadcn/ui Popover implementation.This follows the established shadcn/ui pattern with proper Radix UI primitive wrapping, forwardRef usage with correct TypeScript types, Portal rendering, and animation classes. The
displayNameassignment aids debugging in React DevTools.packages/sdd-webapp/src/client/components/TaskDetailView.tsx (2)
1-12: LGTM!Imports are well-organized with clear separation between React, UI components, types, utilities, and local components. The tree utility functions (
countFiles,findDirectoryChildrenByPath,findFirstFileByName,isPathWithinDir) are appropriately imported for the component's needs.
58-163: LGTM!Well-structured component with clear separation of concerns:
- Header with back navigation, task name, archived/status badges, and phase indicators
- Sidebar with file tree and file count
- Main content area with proper conditional rendering for spec content, missing overview, and empty state
The Japanese text for the overview-missing state is appropriate. Based on learnings, Japanese comments/text is expected for this project.
packages/sdd-webapp/src/client/hooks/useSpecs.ts (2)
6-9: LGTM!Good extension of the hook signature to support optional archived filtering. The options pattern is clean and extensible.
77-89: LGTM!The
refreshfunction properly runs both fetches concurrently withPromise.all. The returned object exposes all necessary state and actions.packages/sdd-webapp/src/lib/status-parser.ts (3)
5-21: LGTM!Good addition of the
includeArchivedoption with a sensible default (emptyarchivedcount initialized to 0). The early return for missing specsPath properly includes thearchivedfield.
76-98: LGTM!The
collectArchivedTaskPromisesfunction correctly:
- Returns an array of promises (not awaited) for efficient parallel processing with
Promise.all- Uses
_archiveddirectory convention- Sets
taskRelativePathto include the_archived/prefix- Omits
statusFromMdsince archived tasks likely aren't tracked in status.md
130-164: LGTM!Good refactor to use a params object for better readability and extensibility. The pattern
isArchived: params.isArchived || undefinedcorrectly omits the field when false (aligning with the optionalisArchived?: booleaninTaskStatus).packages/sdd-webapp/src/server.ts (5)
27-27: LGTM!Scoping CORS to
/api/*is a good security practice - static files don't need CORS headers.
38-45: LGTM!Good use of centralized
validateProjectPathwith clear error handling. The validation ensures the path exists and contains aspecs/directory before adding the project.
67-91: LGTM!Good defensive handling of the file path:
- Retrieves wildcard parameter with fallback to path extraction
- Uses
safeDecodeURIComponentto handle potentially malformed URIs- Returns proper 404 for missing files
98-105: LGTM!Clean implementation of the
includeArchivedquery parameter accepting both"1"and"true"for flexibility. Properly passed toparseStatusFromSpecs.
135-141: LGTM!Good defensive utility that prevents
URIErrorfrom malformed URI components. Returning the original value is a safe fallback.packages/sdd-webapp/src/client/components/Dashboard.tsx (3)
1-11: LGTM!Good extension of
DashboardPropswith the archived toggle functionality. Clean imports with proper type imports.
112-156: LGTM!Well-structured
TaskItemcomponent using theButtoncomponent for proper accessibility. The status dot, archived badge, phase indicators, and navigation arrow provide good visual feedback.
158-228: LGTM!
NextCommandSuggestionsprovides helpful command suggestions based on task state. The copy-to-clipboard functionality with focus styling is well implemented.
masseater
force-pushed
the
feat/sdd-webapp-webui-refactor
branch
from
d9c57f9 to
0e99f9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (5)
packages/sdd-webapp/src/lib/markdown.ts (1)
62-62: Previous feedback not yet addressed: add{ async: false }to improve type safety.The type assertion
as stringcan be avoided by explicitly passing{ async: false }tomarked.parse(), which guarantees a synchronous string return type.- const html = marked.parse(content, { renderer }) as string; + const html = marked.parse(content, { renderer, async: false });packages/sdd-webapp/src/client/components/TaskDetailView.tsx (1)
39-51: EnsureonSelectFileis memoized in parent component.This concern was raised in a previous review. The effect depends on
onSelectFile, which will trigger re-runs if the parent passes a new function reference on each render.packages/sdd-webapp/src/client/hooks/useSpecs.ts (1)
53-66: Inconsistent loading state management infetchStatus.Unlike
fetchFileTreeandfetchSpecContent, this function doesn't setloadingtotrue/false. This meansrefresh()won't show loading state while status is being fetched.packages/sdd-webapp/src/client/hooks/useProjects.ts (2)
31-40: Missing error handling inaddProject.This function doesn't wrap the
fetchJsoncall in try/catch. If the request fails, the error propagates to the caller without updating the error state or providing user feedback.
42-48: Missing error handling inremoveProject.Same issue as
addProject- failures won't update the error state displayed in the UI.
🧹 Nitpick comments (8)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (1)
89-119: Consider wrapping inputs in a form element.The form controls are not wrapped in a
<form>element, which means:
- Users cannot submit by pressing Enter
- Screen readers won't announce it as a form
- Standard form semantics are missing
This impacts accessibility and keyboard navigation.
🔎 Proposed refactor to add form element
<PopoverContent> - <div className="flex flex-col gap-2"> + <form + className="flex flex-col gap-2" + onSubmit={(e) => { + e.preventDefault(); + handleAdd(); + }} + > <div className="text-sm font-medium">Add project</div> <Input placeholder="Project path (e.g., /path/to/project)" value={newPath} onChange={(e) => setNewPath(e.target.value)} /> <Input placeholder="Project name (optional)" value={newName} onChange={(e) => setNewName(e.target.value)} /> {error && <div className="text-xs text-destructive">{error}</div>} <div className="flex gap-2 pt-2"> - <Button type="button" onClick={handleAdd} disabled={submitting}> + <Button type="submit" disabled={submitting}> Add </Button> <Button type="button" variant="secondary" onClick={() => { setShowAddForm(false); setError(null); }} > Cancel </Button> </div> - </div> + </form> </PopoverContent>packages/sdd-webapp/src/client/lib/api.ts (1)
3-21: LGTM! Generic fetch utility with good error handling.The
fetchJsonutility provides consistent API error handling across the application. The error message extraction attempts to parse a JSON error body and falls back gracefully to the HTTP status text if parsing fails.Optional: Consider a more descriptive comment for the catch block
try { const body = (await response.json()) as ApiErrorBody; if (body.error) message = body.error; } catch { - // ignore + // Ignore JSON parse errors; fall back to status message }packages/sdd-webapp/src/client/components/SpecViewer.tsx (2)
23-36: Consider extending TOC indentation for deeper heading levels.The current implementation only indents level 3 headings with
pl-3. If the markdown content contains level 4+ headings, they won't have progressive indentation. This may be acceptable if your specs only use h2/h3, but if deeper nesting is expected, consider a more dynamic approach.🔎 Optional: Dynamic indentation based on heading level
<li key={item.id} - className={item.level === 3 ? "pl-3" : undefined} + className={item.level > 2 ? `pl-${(item.level - 2) * 3}` : undefined} >Note: This requires Tailwind's safelist or JIT to recognize dynamic classes, or use inline styles instead.
11-15: Server-side HTML sanitization is implemented; consider using an established library.The
renderMarkdown()function inpackages/sdd-webapp/src/lib/markdown.tsalready sanitizes the HTML before it reaches the client. The customsanitizeHtml()implementation removes script tags, event handlers, and protocols that commonly enable XSS. However, it relies on regex patterns and may miss edge cases. For more comprehensive protection against XSS vectors, consider replacing the custom implementation with a battle-tested library like DOMPurify or sanitize-html, which handle a broader range of attack vectors through well-maintained parsers.packages/sdd-webapp/src/mcp-server.ts (1)
177-187: Consider adding error handling forremoveProject.Unlike the other tool handlers,
sdd_webapp_remove_projectdoesn't wrap theremoveProjectcall in a try/catch. IfremoveProjectthrows (e.g., filesystem error during save), the error will propagate uncaught.🔎 Proposed fix with try/catch
case "sdd_webapp_remove_project": { if (!isRemoveProjectArgs(args)) { return mcpError("Error: Invalid arguments. 'id' is required."); } - const success = await removeProject(args.id); - if (!success) { - return mcpError(`Error: Project not found: ${args.id}`); + try { + const success = await removeProject(args.id); + if (!success) { + return mcpError(`Error: Project not found: ${args.id}`); + } + return mcpText(`Project removed: ${args.id}`); + } catch (error) { + return mcpError( + `Error removing project: ${error instanceof Error ? error.message : String(error)}`, + ); } - return mcpText(`Project removed: ${args.id}`); }packages/sdd-webapp/src/lib/config.ts (1)
94-102: Atomic write doesn't clean up temp file on failure.If
renamethrows (e.g., permission issues, disk full), the temp file remains orphaned. Consider wrapping in try/finally to unlink the temp file on failure.🔎 Proposed fix
async function writeJsonAtomic( filePath: string, value: unknown, ): Promise<void> { const dir = dirname(filePath); const tmpPath = join(dir, `${basename(filePath)}.${randomUUID()}.tmp`); - await writeFile(tmpPath, JSON.stringify(value, null, 2)); - await rename(tmpPath, filePath); + await writeFile(tmpPath, JSON.stringify(value, null, 2)); + try { + await rename(tmpPath, filePath); + } catch (err) { + await unlink(tmpPath).catch(() => {}); + throw err; + } }Note: You'll need to add
unlinkto the imports fromnode:fs/promises.packages/sdd-webapp/src/server.ts (1)
71-75: Redundant fallback logic for file path extraction.The
rawWildcardfromc.req.param("*")should capture the entire path after/specs/. The fallback usingc.req.path.replace(...)appears to be defensive coding, but if the wildcard capture fails, the route matching itself would likely have issues. Consider if this fallback is truly needed or can be simplified.🔎 Proposed simplification
- const rawWildcard = c.req.param("*") || ""; - const rawFilePath = - rawWildcard || - c.req.path.replace(`/api/projects/${project.id}/specs/`, ""); - const filePath = safeDecodeURIComponent(rawFilePath); + const rawFilePath = c.req.param("*") || ""; + const filePath = safeDecodeURIComponent(rawFilePath);packages/sdd-webapp/src/client/components/Dashboard.tsx (1)
33-38: Consider using shadcn/ui Checkbox for design consistency.The native checkbox works, but using the shadcn/ui
Checkboxcomponent would provide consistent styling with the rest of the UI. This is optional since functionality is correct.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
packages/sdd-webapp/biome.json(1 hunks)packages/sdd-webapp/components.json(1 hunks)packages/sdd-webapp/package.json(1 hunks)packages/sdd-webapp/src/client/App.tsx(4 hunks)packages/sdd-webapp/src/client/components/Dashboard.tsx(2 hunks)packages/sdd-webapp/src/client/components/FileTree.tsx(3 hunks)packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/components/SpecViewer.tsx(1 hunks)packages/sdd-webapp/src/client/components/TaskDetailView.tsx(2 hunks)packages/sdd-webapp/src/client/components/ui/button.tsx(1 hunks)packages/sdd-webapp/src/client/components/ui/input.tsx(1 hunks)packages/sdd-webapp/src/client/components/ui/popover.tsx(1 hunks)packages/sdd-webapp/src/client/hooks/useProjects.ts(3 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(6 hunks)packages/sdd-webapp/src/client/index.html(0 hunks)packages/sdd-webapp/src/client/lib/api.ts(1 hunks)packages/sdd-webapp/src/client/lib/routes.ts(1 hunks)packages/sdd-webapp/src/client/lib/status.ts(1 hunks)packages/sdd-webapp/src/client/lib/tree.ts(1 hunks)packages/sdd-webapp/src/client/lib/utils.ts(1 hunks)packages/sdd-webapp/src/client/main.tsx(1 hunks)packages/sdd-webapp/src/client/styles.css(1 hunks)packages/sdd-webapp/src/client/tailwind.config.js(1 hunks)packages/sdd-webapp/src/lib/config.ts(4 hunks)packages/sdd-webapp/src/lib/markdown.ts(4 hunks)packages/sdd-webapp/src/lib/mcp.ts(1 hunks)packages/sdd-webapp/src/lib/project-path.ts(1 hunks)packages/sdd-webapp/src/lib/status-parser.ts(6 hunks)packages/sdd-webapp/src/mcp-server.ts(3 hunks)packages/sdd-webapp/src/server.ts(6 hunks)packages/sdd-webapp/src/types.ts(2 hunks)packages/sdd-webapp/tsconfig.json(1 hunks)packages/sdd-webapp/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/sdd-webapp/src/client/index.html
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/sdd-webapp/src/client/components/ui/input.tsx
- packages/sdd-webapp/src/client/components/ui/popover.tsx
- packages/sdd-webapp/src/client/lib/status.ts
- packages/sdd-webapp/vite.config.ts
- packages/sdd-webapp/src/client/components/FileTree.tsx
- packages/sdd-webapp/src/client/tailwind.config.js
- packages/sdd-webapp/src/client/components/ui/button.tsx
- packages/sdd-webapp/package.json
- packages/sdd-webapp/src/lib/project-path.ts
- packages/sdd-webapp/biome.json
- packages/sdd-webapp/src/client/lib/utils.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/client/lib/routes.tspackages/sdd-webapp/src/lib/mcp.tspackages/sdd-webapp/src/mcp-server.tspackages/sdd-webapp/src/types.tspackages/sdd-webapp/src/lib/markdown.tspackages/sdd-webapp/src/client/lib/api.tspackages/sdd-webapp/src/client/hooks/useSpecs.tspackages/sdd-webapp/src/client/lib/tree.tspackages/sdd-webapp/src/server.tspackages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/src/lib/config.tspackages/sdd-webapp/src/lib/status-parser.ts
🧠 Learnings (12)
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/client/main.tsxpackages/sdd-webapp/src/client/hooks/useSpecs.tspackages/sdd-webapp/src/server.tspackages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/src/lib/config.tspackages/sdd-webapp/components.jsonpackages/sdd-webapp/src/client/App.tsxpackages/sdd-webapp/src/client/components/SpecViewer.tsx
📚 Learning: 2025-12-18T06:10:43.172Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:21-21
Timestamp: 2025-12-18T06:10:43.172Z
Learning: In packages/sdd-webapp/package.json, the "typecheck" script uses `tsgo --noEmit`, where `tsgo` is the TypeScript 7 native CLI from the `typescript/native-preview` package (Microsoft's TypeScript native port, codename "Corsa"). This is intentional for faster type checking and is not a typo of `tsc`.
Applied to files:
packages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/client/main.tsxpackages/sdd-webapp/src/server.tspackages/sdd-webapp/src/lib/config.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Use 'type' for TypeScript type definitions; use 'interface' only when there is a clear reason such as extending existing library types, and document the reason in a comment
Applied to files:
packages/sdd-webapp/tsconfig.jsonpackages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:04.190Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:04.190Z
Learning: Run TypeScript type checking using `bun run typecheck` within each plugin directory before committing
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Prohibit barrel imports and exports
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:04.190Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:04.190Z
Learning: Applies to **/*.ts : Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Applied to files:
packages/sdd-webapp/tsconfig.json
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Avoid creating tables in markdown unnecessarily
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Do not add numbers to markdown headings
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Prohibit implicit fallbacks in logic; make all fallbacks explicit and intentional
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Review API documentation and implement appropriate retry handling for potential errors in API calls
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Implement fail-fast principles in code to catch errors early
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
🧬 Code graph analysis (7)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (3)
packages/sdd-webapp/src/client/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)packages/sdd-webapp/src/client/components/ui/button.tsx (1)
Button(57-57)packages/sdd-webapp/src/client/components/ui/input.tsx (1)
Input(22-22)
packages/sdd-webapp/src/mcp-server.ts (3)
packages/sdd-webapp/src/lib/mcp.ts (2)
mcpError(7-9)mcpText(3-5)packages/sdd-webapp/src/lib/project-path.ts (1)
validateProjectPath(8-34)packages/sdd-webapp/src/lib/config.ts (4)
addProject(31-54)loadProjects(15-24)removeProject(56-65)getProject(67-70)
packages/sdd-webapp/src/client/lib/tree.ts (1)
packages/sdd-webapp/src/types.ts (1)
FileNode(3-9)
packages/sdd-webapp/src/server.ts (3)
packages/sdd-webapp/src/lib/project-path.ts (1)
validateProjectPath(8-34)packages/sdd-webapp/src/lib/config.ts (1)
addProject(31-54)packages/sdd-webapp/src/lib/status-parser.ts (1)
parseStatusFromSpecs(5-74)
packages/sdd-webapp/src/client/hooks/useProjects.ts (3)
packages/sdd-webapp/src/client/lib/api.ts (1)
fetchJson(3-21)packages/sdd-webapp/src/types.ts (1)
Project(42-48)packages/sdd-webapp/src/client/lib/routes.ts (1)
apiRoutes(8-20)
packages/sdd-webapp/src/client/components/SpecViewer.tsx (1)
packages/sdd-webapp/src/types.ts (1)
SpecContent(11-16)
packages/sdd-webapp/src/lib/status-parser.ts (1)
packages/sdd-webapp/src/types.ts (1)
TaskStatus(24-30)
🪛 ast-grep (0.40.0)
packages/sdd-webapp/src/client/components/SpecViewer.tsx
[warning] 13-13: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🔇 Additional comments (37)
packages/sdd-webapp/src/lib/markdown.ts (2)
16-35: Well-designed slugging utilities with good internationalization support.The
createSluggerfactory provides collision-safe IDs, andslugifycorrectly handles Unicode text (including Japanese headings per PR objectives) via NFKC normalization and Unicode property escapes. The fallback to"section"for empty slugs is a sensible defensive measure.
72-93: Clean token-based TOC extraction.Using
marked.lexer()is more robust than regex-based approaches. The separate slugger instance ensures ID consistency withrenderMarkdownwhen both process the same content in document order.packages/sdd-webapp/src/client/components/ProjectSelector.tsx (3)
2-8: LGTM: shadcn/ui components imported correctly.The imports align with the PR objective to adopt shadcn/ui and Tailwind, and the component APIs match their usage throughout the file.
30-30: LGTM: Submitting state prevents double submission.The
submittingstate correctly prevents users from clicking the Add button multiple times while a request is in flight.
47-64: LGTM: handleAdd logic is well-structured.The try-catch-finally pattern ensures proper cleanup, and the popover correctly closes on success while preserving error states for user feedback.
packages/sdd-webapp/tsconfig.json (1)
15-18: LGTM! Path alias configuration is correct.The
@/*path alias correctly maps to./src/client/*, enabling the import pattern used throughout the new UI components and aligning with the shadcn/ui configuration incomponents.json.packages/sdd-webapp/src/client/main.tsx (1)
5-5: LGTM! Syntax highlighting stylesheet added.The highlight.js GitHub theme import provides consistent code syntax highlighting for the specification viewer.
packages/sdd-webapp/src/lib/mcp.ts (1)
1-9: LGTM! Clean abstraction for MCP responses.The helper functions provide a consistent way to construct MCP tool results for both success and error cases, improving code maintainability across the MCP server.
packages/sdd-webapp/components.json (1)
1-22: LGTM! shadcn/ui configuration is properly set up.The configuration correctly defines the UI framework setup with appropriate style, TypeScript, and Tailwind paths. The aliases align with the TypeScript path mapping in
tsconfig.json.packages/sdd-webapp/src/types.ts (2)
24-30: LGTM! Type extensions support archive functionality.The additions of
path,isArchived, andarchivedproperties properly extend the types to support the new archived task features while maintaining backward compatibility through optional fields.
32-40: LGTM! StatusSummary correctly tracks archived count.The
archivedfield addition enables dashboard statistics to include archived task counts.packages/sdd-webapp/src/client/styles.css (3)
1-48: LGTM! Tailwind CSS migration with comprehensive theming.The migration from custom CSS to Tailwind introduces a well-structured theming system using CSS variables and supports the new shadcn/ui components. The inline theme definitions provide consistent spacing and color tokens across the application.
50-117: LGTM! Complete light and dark mode color systems.The color tokens for both light and dark modes use the oklch color space for better perceptual uniformity. The comprehensive set of semantic color variables (background, foreground, card, muted, etc.) enables consistent theming across all UI components.
119-135: LGTM! Base styles and improved anchor navigation.The base layer correctly applies Tailwind utilities for consistent borders and backgrounds. The
scroll-margin-toprule on prose headings enhances the user experience when navigating via table-of-contents anchors in the specification viewer.packages/sdd-webapp/src/client/App.tsx (5)
1-9: LGTM! Import updates align with new UI structure.The updated imports correctly reference the new Button component and types, supporting the enhanced UI and archived task functionality.
11-29: LGTM! State management for archive support and error handling.The addition of
includeArchivedstate and extraction of bothprojectsErrorandspecsErrorenables comprehensive error display and archived task filtering as specified in the PR objectives.
51-58: LGTM! Unified error display for better UX.The error alert provides clear visual feedback when either projects or specs encounter errors, improving the user experience with proper ARIA role and styling.
59-90: LGTM! Improved navigation with sticky header.The new sticky header with breadcrumb-style navigation and back button fixes the navigation issues mentioned in the PR objectives. The backdrop blur effect provides good visual separation while maintaining context.
99-116: LGTM! Archive controls properly wired to Dashboard.The
includeArchivedstate andonToggleArchivedcallback are correctly passed to the Dashboard component, enabling the toggle functionality for archived task visibility as described in the PR objectives.packages/sdd-webapp/src/client/lib/routes.ts (1)
1-20: Clean and well-structured route helpers.The implementation correctly handles URL encoding:
encodePathPreservingSlashesproperly encodes individual path segments while preserving the directory structure- All dynamic IDs are encoded with
encodeURIComponent- The optional query parameter pattern in
projectStatusis clean and extensiblepackages/sdd-webapp/src/client/lib/tree.ts (1)
1-6: LGTM on the remaining utility functions.The implementations are clean and handle edge cases appropriately:
findTaskFileTreecorrectly searches by directory namecountFilesaccurately counts file nodes recursivelyfindFirstFileByNameprovides a useful DFS searchisPathWithinDiris a simple and correct path checkAlso applies to: 23-31, 33-45, 64-66
packages/sdd-webapp/src/client/components/TaskDetailView.tsx (1)
31-37: Well-structured two-pane layout with good UX patterns.The implementation includes:
- Properly memoized
taskFileTreederived from props- Scroll-to-top behavior on file selection changes
- Responsive layout with sidebar and main content
- Good use of semantic elements (
<aside>,<main>)- Accessible aria-label on the back button
Also applies to: 53-56, 58-165
packages/sdd-webapp/src/mcp-server.ts (1)
14-15: Good refactoring with centralized MCP helpers.The changes improve code consistency:
mcpTextandmcpErrorhelpers reduce boilerplate and ensure consistent response structurevalidateProjectPathcentralizes path validation logic with clear error messages- All tool handlers follow a consistent pattern for argument validation and error handling
Also applies to: 138-157, 160-174, 189-213, 215-217
packages/sdd-webapp/src/lib/config.ts (2)
19-20: Good defensive parsing for loadProjects.The type guard
Array.isArray(parsed)ensures malformed JSON doesn't crash the app and gracefully falls back to an empty array.
76-79: Good defensive parsing for loadConfig.The null check and type guard ensure robustness against malformed config files.
packages/sdd-webapp/src/client/hooks/useSpecs.ts (2)
77-79: Well-structured concurrent refresh.Using
Promise.allfor parallel fetching is efficient. The refresh function properly depends on both fetch callbacks.
17-31: Good migration to centralized fetch utilities.The switch to
fetchJsonandapiRoutesimproves consistency and maintainability across the codebase.packages/sdd-webapp/src/server.ts (3)
40-43: Good centralization of path validation.Using
validateProjectPathconsolidates existence checks, absolute path validation, and specs directory verification in one place. This improves maintainability and ensures consistent validation.
135-141: Good defensive URI decoding.
safeDecodeURIComponentgracefully handles malformed percent-encoded sequences by returning the original value, preventing server crashes from malicious or buggy client input.
98-103: Query parameter parsing accepts multiple truthy values.Accepting both
"1"and"true"forincludeArchivedis user-friendly. Consider documenting this in API docs if applicable.packages/sdd-webapp/src/client/hooks/useProjects.ts (1)
12-29: Good error handling pattern infetchProjects.Clearing error state before fetch, catching errors with proper type narrowing, and using finally for loading state is the correct pattern. Apply this same pattern to
addProjectandremoveProject.packages/sdd-webapp/src/lib/status-parser.ts (3)
76-98: Well-structured archived task collection.The function properly guards against missing
_archiveddirectory and uses the same params-based pattern as active tasks, maintaining consistency.
130-136: Good refactor to params object pattern.Using a single params object instead of positional arguments improves readability and makes future extensions easier without breaking existing call sites.
53-60: Efficient conditional archived collection.The spread pattern with
Promise.allcleanly handles the optional archived tasks without duplicating logic.packages/sdd-webapp/src/client/components/Dashboard.tsx (3)
86-110: Well-designed SummaryCard component.The component is focused, reusable, and has a clean API with optional
toneprop for styling variations.
119-154: Good use of status utilities and Button component.Using
getStatusDotClassesfor consistent status styling and theButtoncomponent withvariant="ghost"maintains design system coherence while providing good hover/focus states.
54-58: Clear conditional metric display.The toggle between "Archived" and "Not Started" cards based on
includeArchivedis intuitive UX.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdd-webapp/src/lib/markdown.ts (1)
72-93: ID mismatch between TOC and rendered headings for duplicate text.
extractTableOfContentscreates a separate slugger and only iterates h2/h3, whilerenderMarkdownprocesses all heading levels (h1–h6) with its own slugger. When duplicate heading text exists across levels, the generated IDs will diverge.Example:
# Foo ← renderMarkdown: id="foo" ## Foo ← renderMarkdown: id="foo-2", but TOC generates id="foo"The TOC link points to
#foo, but the actual heading hasid="foo-2".🔎 Proposed fix: share slugger state or filter consistently
Option 1: Filter renderMarkdown to same levels as TOC
If only h2/h3 headings need IDs, skip ID assignment for other levels:
renderer.heading = function headingWithId(token: Tokens.Heading) { const innerHtml = this.parser.parseInline(token.tokens); + if (token.depth !== 2 && token.depth !== 3) { + return `<h${token.depth}>${innerHtml}</h${token.depth}>\n`; + } const plainText = this.parser.parseInline( token.tokens, this.parser.textRenderer, ); const id = slugger(plainText); return `<h${token.depth} id="${id}">${innerHtml}</h${token.depth}>\n`; };Option 2: Export shared slugger factory for coordinated usage
If all heading levels need IDs, refactor so TOC extraction and rendering share the same slugger instance (e.g., pass slugger as parameter or run extraction on the same token stream during rendering).
🧹 Nitpick comments (2)
packages/sdd-webapp/src/lib/markdown.ts (1)
37-46: Consider a DOM-based sanitizer for stronger XSS protection.Regex-based HTML sanitization can be bypassed via edge cases (encoding tricks, parser differentials). As defense-in-depth for markdown-generated HTML this is acceptable, but for a webapp, a library like DOMPurify would provide more robust guarantees.
packages/sdd-webapp/src/client/hooks/useSpecs.ts (1)
89-91: Efficient concurrent refresh implementation.Using
Promise.allto fetch file tree and status concurrently improves performance. Each operation independently manages loading state and errors through its own try-catch-finally block. The loading count mechanism correctly tracks both operations in flight.Minor note: If both operations fail, only the last error will be visible since they both call
setError. This is acceptable for most scenarios, though you could consider aggregating errors if precise error reporting becomes important.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
packages/sdd-webapp/package.json(1 hunks)packages/sdd-webapp/src/client/hooks/useProjects.ts(3 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(3 hunks)packages/sdd-webapp/src/client/styles.css(1 hunks)packages/sdd-webapp/src/lib/markdown.ts(4 hunks)packages/sdd-webapp/src/lib/project-path.ts(1 hunks)packages/sdd-webapp/src/mcp-server.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/sdd-webapp/src/client/hooks/useProjects.ts
- packages/sdd-webapp/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/lib/markdown.tspackages/sdd-webapp/src/mcp-server.tspackages/sdd-webapp/src/lib/project-path.tspackages/sdd-webapp/src/client/hooks/useSpecs.ts
🧠 Learnings (4)
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Avoid creating tables in markdown unnecessarily
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.md : Do not add numbers to markdown headings
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Prohibit implicit fallbacks in logic; make all fallbacks explicit and intentional
Applied to files:
packages/sdd-webapp/src/lib/markdown.ts
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
🧬 Code graph analysis (2)
packages/sdd-webapp/src/mcp-server.ts (3)
packages/sdd-webapp/src/lib/mcp.ts (2)
mcpError(7-9)mcpText(3-5)packages/sdd-webapp/src/lib/project-path.ts (1)
validateProjectPath(8-35)packages/sdd-webapp/src/lib/config.ts (4)
addProject(31-54)loadProjects(15-24)removeProject(56-65)getProject(67-70)
packages/sdd-webapp/src/client/hooks/useSpecs.ts (3)
packages/sdd-webapp/src/types.ts (3)
FileNode(3-9)SpecContent(11-16)StatusSummary(32-40)packages/sdd-webapp/src/client/lib/api.ts (1)
fetchJson(3-21)packages/sdd-webapp/src/client/lib/routes.ts (1)
apiRoutes(8-20)
🔇 Additional comments (19)
packages/sdd-webapp/src/lib/markdown.ts (3)
16-24: LGTM!Clean closure-based slugger with proper duplicate handling. The fallback to "section" for empty slugs and the numbering scheme (base, base-2, base-3) are well implemented.
26-35: LGTM!Good use of NFKC normalization and Unicode property escapes (
\p{L},\p{N}) to properly handle international text including Japanese headings, as noted in the PR objectives.
48-64: LGTM — previous review comment addressed.The
async: falseoption is now explicitly passed tomarked.parse(), ensuring a synchronous string return and improving type safety. The defensivetypeofcheck on line 63 is slightly redundant but harmless.packages/sdd-webapp/src/client/hooks/useSpecs.ts (2)
14-24: Excellent loading state mechanism for concurrent operations.The count-based loading approach correctly handles overlapping async operations. Using stable callbacks and
Math.max(0, count - 1)prevents edge cases. This design supports the new concurrentrefresh()function well.
62-78: Loading state management now consistent across all fetch functions.The previous concern about
fetchStatusnot managing loading state has been resolved. Lines 65 and 76 now callstartLoading()andstopLoading(), making the loading behavior consistent withfetchFileTreeandfetchSpecContent. TheincludeArchivedoption is also properly passed to the API route.packages/sdd-webapp/src/lib/project-path.ts (3)
1-2: LGTM!Clean imports with all utilities properly used throughout the validation function.
4-6: LGTM!Well-designed discriminated union type that enables type-safe error handling in consuming code.
8-34: Excellent validation flow and fix from previous review!The function implements a clean, sequential validation pattern. The use of
isAbsolute()on line 15 properly addresses the previous review concern about usingresolve(projectPath) !== projectPath, which was unreliable due to path normalization and platform differences. The current implementation follows the standard idiom for absolute path detection.The validation logic is comprehensive and returns helpful error messages at each stage.
packages/sdd-webapp/src/mcp-server.ts (6)
14-15: LGTM!The new imports support the refactoring to centralized validation and standardized MCP response helpers, improving code consistency across all tool handlers.
138-158: LGTM!Excellent refactoring of the add project handler:
- Proper validation flow using the centralized
validateProjectPathhelper- Uses
validated.projectPath(line 149) to ensure the canonical resolved path is passed toaddProject- Consistent error handling with descriptive messages via
mcpError/mcpTexthelpers- Clear success response with project details
160-175: LGTM!The list projects handler is well-structured with good UX considerations:
- Helpful message for empty project list
- Clear, formatted output for project details
- Consistent use of
mcpTexthelper for success responses
177-193: LGTM!Comprehensive error handling in the remove project handler:
- Validates arguments before processing
- Try-catch wrapper protects against exceptions
- Explicit check for non-existent project (line 184) provides clear feedback
- Consistent use of
mcpError/mcpTexthelpers
195-219: LGTM!Well-implemented status handler with thorough validation and formatting:
- Arguments and project existence validated upfront
- Rich, structured output with summary statistics and task details
- Good UX with fallback message for empty task lists
- Comprehensive error handling throughout
221-234: LGTM!Clean completion of the refactoring:
- Default case properly handles unknown tools with
mcpError- Exported
startMcpServerfunction enables programmatic server controlimport.meta.maincheck supports both module import and direct script executionpackages/sdd-webapp/src/client/styles.css (5)
8-47: LGTM!The theme configuration follows shadcn/ui conventions correctly. The radius scale is well-structured, and color token mappings provide proper theming flexibility.
84-116: LGTM!The dark theme implementation is well-structured with properly inverted colors and appropriate use of alpha transparency for borders and inputs.
118-125: LGTM!The base layer correctly applies global border and outline utilities while setting body theming. The use of
@applyis appropriate for Tailwind v4's architecture.
51-82: Verify oklch color format aligns with project browser support requirements.The CSS uses the
oklch()color format throughout, which is supported in all modern browsers (Chrome 111+, Firefox 113+, Safari 15.4+, Edge 111+). Ensure this aligns with your project's browser support targets.
127-134: 6rem scroll-margin-top accounts for maximum header height but lacks flexibility.The sticky header is
h-16(4rem) with an optional error message above it. The currentscroll-margin-top: 6remvalue assumes both are always present. When the error is hidden, this leaves excessive whitespace. Consider reducing to5remfor better spacing, or use CSS variables to match your header height dynamically:scroll-margin-top: var(--header-height, 5rem).
- Add error handling to removeProject in useProjects hook - Clear error state when Popover closes in ProjectSelector - Translate Japanese error message to English in TaskDetailView - Fix empty directory search logic in tree.ts using null sentinel - Fix isArchived falsy conversion (false -> undefined) in status-parser 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/sdd-webapp/src/client/lib/tree.ts (2)
47-73: Past review feedback addressed.The internal helper now correctly uses
nullas the "not found" sentinel, allowing the public function to distinguish between an empty directory and a missing one.
3-21: Same empty-directory bug as was previously fixed infindDirectoryChildrenByPath.Line 16 checks
found.length > 0, which cannot distinguish between "directory found but empty" and "not found." If a task directory exists with no children, the search incorrectly continues to other branches.Apply the same internal-helper pattern used for
findDirectoryChildrenByPath:🔎 Proposed fix
export function findTaskFileTree( nodes: FileNode[], taskName: string, ): FileNode[] { + const result = findTaskFileTreeInternal(nodes, taskName); + return result ?? []; +} + +function findTaskFileTreeInternal( + nodes: FileNode[], + taskName: string, +): FileNode[] | null { for (const node of nodes) { if (node.type !== "directory") continue; if (node.name === taskName && node.children) { return node.children; } if (node.children) { - const found = findTaskFileTree(node.children, taskName); - if (found.length > 0) return found; + const found = findTaskFileTreeInternal(node.children, taskName); + if (found !== null) return found; } } - return []; + return null; }packages/sdd-webapp/src/client/components/TaskDetailView.tsx (2)
39-51: Parent must memoizeonSelectFileto prevent unnecessary effect re-runs.The effect depends on
onSelectFile(line 51). If the parent component doesn't wrap this callback withuseCallback, every parent render creates a new function reference, causing this effect to re-run. While the guard at line 42 prevents infinite loops, unnecessary effect executions still occur.
143-150: Past review feedback addressed: Japanese message replaced with English.The "overview.md not found" message is now in English, consistent with the rest of the UI.
🧹 Nitpick comments (3)
packages/sdd-webapp/src/client/hooks/useProjects.ts (1)
31-62: Consider adding loading indicators for add/remove operations.While the error handling is solid, users might benefit from loading state during
addProjectandremoveProjectoperations. This could be implemented by adding anisSavingstate similar to the existingloadingstate.Example implementation
export function useProjects() { const [projects, setProjects] = useState<Project[]>([]); const [selectedProject, setSelectedProject] = useState<Project | null>(null); const [loading, setLoading] = useState(true); + const [isSaving, setIsSaving] = useState(false); const [error, setError] = useState<string | null>(null); const addProject = useCallback(async (path: string, name?: string) => { try { + setIsSaving(true); setError(null); const project = await fetchJson<Project>(apiRoutes.projects(), { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ path, name }), }); setProjects((prev) => [...prev, project]); setSelectedProject(project); return project; } catch (err) { setError(err instanceof Error ? err.message : "Unknown error"); throw err; + } finally { + setIsSaving(false); } }, []); const removeProject = useCallback(async (id: string) => { try { + setIsSaving(true); setError(null); await fetchJson<{ success: true }>(apiRoutes.project(id), { method: "DELETE", }); setProjects((prev) => prev.filter((p) => p.id !== id)); setSelectedProject((prev) => (prev?.id === id ? null : prev)); } catch (err) { const message = err instanceof Error ? err.message : "Failed to remove project"; setError(message); throw err; + } finally { + setIsSaving(false); } }, []); return { projects, selectedProject, setSelectedProject, addProject, removeProject, loading, + isSaving, error, refresh: fetchProjects, }; }packages/sdd-webapp/src/lib/status-parser.ts (1)
53-70: Consider more explicit archived count filter.The logic correctly combines active and archived tasks. However, line 69's filter could be more explicit:
archived: tasks.filter((t) => t.isArchived === true).length,This makes the intent clearer and avoids any potential truthy/falsy ambiguity, especially since
isArchivedis an optional boolean.🔎 Proposed refinement
- archived: tasks.filter((t) => t.isArchived).length, + archived: tasks.filter((t) => t.isArchived === true).length,packages/sdd-webapp/src/client/components/ProjectSelector.tsx (1)
109-123: Optional: Consider form field clearing behavior.Two minor refinements to consider:
The Cancel button calls
setError(null)(line 118), but the Popover'sonOpenChangehandler (line 87) will also clear the error when the popover closes. The explicit call is redundant but harmless.Clicking Cancel preserves the input field values (
newPathandnewName), so reopening the popover shows the previous draft. This might be intentional to help users continue interrupted work, but if you want a fresh form each time, consider clearing the fields on cancel.🔎 Optional improvements
If you want to clear form fields on cancel:
<Button type="button" variant="secondary" onClick={() => { setShowAddForm(false); - setError(null); + setNewPath(""); + setNewName(""); }} > Cancel </Button>Note: Removing
setError(null)from Cancel is safe sinceonOpenChangehandles it.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/components/TaskDetailView.tsx(2 hunks)packages/sdd-webapp/src/client/hooks/useProjects.ts(3 hunks)packages/sdd-webapp/src/client/lib/tree.ts(1 hunks)packages/sdd-webapp/src/lib/status-parser.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/lib/status-parser.tspackages/sdd-webapp/src/client/lib/tree.tspackages/sdd-webapp/src/client/hooks/useProjects.ts
🧠 Learnings (7)
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Write clear comments in Japanese when code has any complexity to reduce cognitive load
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{test,spec}.{ts,tsx,js,jsx} : Document prerequisites, preconditions, and verification items in comments in Japanese within test code
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Write detailed TSDoc in Japanese including purpose, content, and precautions
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{test,spec}.{ts,tsx,js,jsx} : Use test data strings close to actual names; at minimum use Japanese strings with immediately clear meaning
Applied to files:
packages/sdd-webapp/src/client/components/TaskDetailView.tsx
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Use 'type' for TypeScript type definitions; use 'interface' only when there is a clear reason such as extending existing library types, and document the reason in a comment
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Review API documentation and implement appropriate retry handling for potential errors in API calls
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
🧬 Code graph analysis (2)
packages/sdd-webapp/src/lib/status-parser.ts (1)
packages/sdd-webapp/src/types.ts (1)
TaskStatus(24-30)
packages/sdd-webapp/src/client/lib/tree.ts (1)
packages/sdd-webapp/src/types.ts (1)
FileNode(3-9)
🔇 Additional comments (20)
packages/sdd-webapp/src/client/hooks/useProjects.ts (4)
1-4: LGTM! Clean centralization of types and API utilities.The migration to centralized
fetchJsonandapiRoutesimproves maintainability, and using the importedProjecttype aligns with TypeScript best practices.
12-29: LGTM! Solid error handling and loading state management.The function properly handles errors, manages loading state in a finally block, and includes smart auto-selection logic that preserves user choices.
31-46: Error handling successfully implemented.The try/catch pattern properly addresses the previous review concern. The function now sets error state, updates projects list on success, and re-throws errors for caller handling.
48-62: Error handling successfully implemented.The function now properly handles errors and updates state consistently. The logic for clearing the selected project when it's deleted is correct.
packages/sdd-webapp/src/lib/status-parser.ts (3)
5-21: LGTM! Clean addition of archived support.The optional
includeArchivedparameter and fallback initialization are well-structured and consistent with the new archived tracking functionality.
35-51: LGTM! Excellent refactoring to params object.The structured params object improves readability and maintainability. Explicitly setting
isArchived: falsefor active tasks is clear and correct.
76-98: Excellent work! Past issue resolved and implementation is solid.The previous issue with
isArchived: falsebeing coerced toundefinedhas been properly fixed at line 162. The refactoring to a params object and the newcollectArchivedTaskPromisesfunction are well-structured and maintainable.Key improvements:
- Line 162 now correctly preserves the boolean value without
|| undefined- Params object pattern improves clarity and extensibility
- Archived task collection is properly isolated with appropriate error handling
- Explicit
isArchived: true(line 95) andisArchived: false(line 48) values are clearNote: Archived tasks don't receive
statusFromMdfallback (line 91-96), which appears intentional since archived tasks likely shouldn't be tracked in the active status.md.Also applies to: 130-164
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (3)
1-9: LGTM! Clean shadcn/ui integration.The new UI component imports are well-structured and align with the PR's goal of adopting shadcn/ui primitives.
30-30: Excellent async state management.The
submittingstate properly tracks the add operation lifecycle and prevents double-submissions by disabling the button during async operations.Also applies to: 47-64
83-88: Past review concern resolved.The
onOpenChangehandler now correctly clears the error state when the popover closes, preventing stale error messages from appearing on reopen.packages/sdd-webapp/src/client/lib/tree.ts (4)
1-1: LGTM!Clean import of the
FileNodetype from the shared types module.
23-31: LGTM!Clean recursive implementation for counting files.
33-45: LGTM!Correct use of
nullsentinel for the not-found case, avoiding the empty-array ambiguity.
75-77: LGTM!Simple and correct for normalized paths without trailing slashes. If
FileNode.pathvalues can have trailing slashes, consider normalizing them first, but this is likely fine given the expected data format.packages/sdd-webapp/src/client/components/TaskDetailView.tsx (6)
1-12: LGTM!Clean imports, well-organized with React hooks, UI components, types, and utilities properly separated.
14-30: LGTM!Well-defined props interface with appropriate types.
32-37: LGTM!Correct memoization of
taskFileTreewith appropriate dependencies. State and ref declarations are properly typed.
53-56: LGTM!Simple, focused effect with correct dependencies for scrolling behavior.
136-138: Ref callback pattern is intentional and correct.Using a callback ref
ref={(node) => { mainScrollRef.current = node; }}instead ofref={mainScrollRef}is a valid pattern that provides more explicit control over when the ref is set.
58-162: Clean two-column layout with proper conditional rendering.The responsive layout with persistent sidebar and main content area is well-structured. Status badges, phases, and archived indicators are appropriately styled.
- Extract getErrorMessage utility for consistent error handling - Remove redundant setError(null) from Cancel button (handled by onOpenChange) - Make TaskStatus.isArchived required property instead of optional - Convert tree.ts functions to functional style using reduce 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (1)
98-107: Add accessible labels for form inputs.The form inputs rely solely on placeholders for context. Screen readers cannot properly announce the field purpose, and placeholders disappear when users begin typing. This was flagged in a previous review but remains unaddressed.
🔎 Proposed fix to add proper labels
<PopoverContent> <div className="flex flex-col gap-2"> <div className="text-sm font-medium">Add project</div> + <label htmlFor="project-path" className="sr-only"> + Project path + </label> <Input + id="project-path" placeholder="Project path (e.g., /path/to/project)" value={newPath} onChange={(e) => setNewPath(e.target.value)} /> + <label htmlFor="project-name" className="sr-only"> + Project name + </label> <Input + id="project-name" placeholder="Project name (optional)" value={newName} onChange={(e) => setNewName(e.target.value)} />packages/sdd-webapp/src/client/lib/tree.ts (1)
10-21: Inconsistent handling of undefined children compared to the fixed version below.Line 17 requires
node.childrento be truthy before returning, but thechildrenproperty is optional onFileNode. If a directory matchestaskNamebut haschildren: undefined, the function returnsnull(meaning "not found") instead of[](meaning "found but empty").This is inconsistent with the corrected pattern at line 61 in
findDirectoryChildrenByPathInternal, which uses the nullish coalescing operator to handle undefined children.🔎 Proposed fix to match the pattern at line 61
function findTaskFileTreeInternal( nodes: FileNode[], taskName: string, ): FileNode[] | null { return nodes.reduce<FileNode[] | null>((found, node) => { if (found !== null) return found; if (node.type !== "directory") return null; - if (node.name === taskName && node.children) return node.children; + if (node.name === taskName) return node.children ?? []; if (node.children) return findTaskFileTreeInternal(node.children, taskName); return null; }, null); }
🧹 Nitpick comments (4)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (2)
68-71: Add an accessible label to the select element.The
<select>lacks an accessible name for screen readers. Add anaria-labelto describe its purpose.🔎 Proposed fix
<select + aria-label="Select project" className="h-9 min-w-[150px] max-w-[320px] rounded-md border bg-background px-3 text-sm" value={selectedProject?.id ?? ""} onChange={handleChange} >
96-120: Consider adding keyboard submission support.The form doesn't submit when users press Enter, which is a common expectation. Wrapping the content in a
<form>element with anonSubmithandler would improve keyboard accessibility.🔎 Proposed enhancement
<PopoverContent> - <div className="flex flex-col gap-2"> + <form + className="flex flex-col gap-2" + onSubmit={(e) => { + e.preventDefault(); + handleAdd(); + }} + > <div className="text-sm font-medium">Add project</div> <Input placeholder="Project path (e.g., /path/to/project)" value={newPath} onChange={(e) => setNewPath(e.target.value)} /> <Input placeholder="Project name (optional)" value={newName} onChange={(e) => setNewName(e.target.value)} /> {error && <div className="text-xs text-destructive">{error}</div>} <div className="flex gap-2 pt-2"> - <Button type="button" onClick={handleAdd} disabled={submitting}> + <Button type="submit" disabled={submitting}> Add </Button> <Button type="button" variant="secondary" onClick={() => setShowAddForm(false)} > Cancel </Button> </div> - </div> + </form> </PopoverContent>packages/sdd-webapp/src/client/hooks/useProjects.ts (1)
26-26: Consider using consistent, descriptive error fallback messages.Line 58 uses a specific fallback (
"Failed to remove project"), while Lines 26 and 44 use the generic"Unknown error". For better user experience, consider using descriptive fallback messages consistently across all operations:
- Line 26:
"Failed to fetch projects"- Line 44:
"Failed to add project"🔎 Proposed fix for consistent error messages
- setError(getErrorMessage(err, "Unknown error")); + setError(getErrorMessage(err, "Failed to fetch projects"));- setError(getErrorMessage(err, "Unknown error")); + setError(getErrorMessage(err, "Failed to add project"));Also applies to: 44-44, 58-58
packages/sdd-webapp/src/client/lib/tree.ts (1)
69-71: Consider normalizing dirPath to handle trailing slashes.If
dirPathends with a trailing slash (e.g.,"foo/"), thestartsWithcheck would look for"foo//", which would fail for valid paths like"foo/bar". While file paths typically don't have trailing slashes, adding normalization would make the function more defensive.🔎 Proposed defensive handling
export function isPathWithinDir(path: string, dirPath: string): boolean { - return path === dirPath || path.startsWith(`${dirPath}/`); + const normalizedDir = dirPath.endsWith('/') ? dirPath.slice(0, -1) : dirPath; + return path === normalizedDir || path.startsWith(`${normalizedDir}/`); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/hooks/useProjects.ts(3 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(3 hunks)packages/sdd-webapp/src/client/lib/error.ts(1 hunks)packages/sdd-webapp/src/client/lib/tree.ts(1 hunks)packages/sdd-webapp/src/types.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdd-webapp/src/types.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/client/lib/error.tspackages/sdd-webapp/src/client/hooks/useSpecs.tspackages/sdd-webapp/src/client/hooks/useProjects.tspackages/sdd-webapp/src/client/lib/tree.ts
🧠 Learnings (4)
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx} : Use 'type' for TypeScript type definitions; use 'interface' only when there is a clear reason such as extending existing library types, and document the reason in a comment
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Review API documentation and implement appropriate retry handling for potential errors in API calls
Applied to files:
packages/sdd-webapp/src/client/hooks/useProjects.ts
🧬 Code graph analysis (4)
packages/sdd-webapp/src/client/lib/error.ts (3)
packages/sdd-webapp/src/cli.ts (1)
error(78-81)packages/cc-plugin-lib/src/logger.ts (1)
fatal(138-144)packages/sdd-webapp/src/client/App.tsx (1)
task(21-23)
packages/sdd-webapp/src/client/hooks/useSpecs.ts (4)
packages/sdd-webapp/src/types.ts (3)
FileNode(3-9)SpecContent(11-16)StatusSummary(32-40)packages/sdd-webapp/src/client/lib/api.ts (1)
fetchJson(3-21)packages/sdd-webapp/src/client/lib/routes.ts (1)
apiRoutes(8-20)packages/sdd-webapp/src/client/lib/error.ts (1)
getErrorMessage(8-10)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (3)
packages/sdd-webapp/src/client/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)packages/sdd-webapp/src/client/components/ui/button.tsx (1)
Button(57-57)packages/sdd-webapp/src/client/components/ui/input.tsx (1)
Input(22-22)
packages/sdd-webapp/src/client/lib/tree.ts (1)
packages/sdd-webapp/src/types.ts (1)
FileNode(3-9)
🔇 Additional comments (18)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (4)
1-9: LGTM!Clean imports using the new shadcn/ui components with proper path aliases.
26-30: LGTM!Good addition of
submittingstate to prevent double-submission during async operations.
47-64: LGTM!Proper async handling with
try/finallyensuressubmittingis always reset, even on errors. The form state cleanup on success is thorough.
83-88: LGTM!Good implementation clearing the error state when the popover closes, preventing stale error messages on reopen.
packages/sdd-webapp/src/client/lib/error.ts (1)
8-10: LGTM! Clean error message extraction utility.The implementation correctly handles unknown error types with a proper type guard and provides a fallback for non-Error values. This centralizes error message extraction across the codebase.
packages/sdd-webapp/src/client/hooks/useProjects.ts (3)
2-5: LGTM! Proper use of centralized utilities.The refactored imports correctly use
import typefor the Project type and pull in the centralizedfetchJson,getErrorMessage, andapiRoutesutilities, eliminating manual fetch handling.
32-47: Error handling properly implemented.The try/catch block correctly handles errors as requested in past reviews. The function now:
- Clears previous errors before the operation
- Surfaces failures via
getErrorMessage- Updates state only on success
- Re-throws for caller handling
49-61: Error handling properly implemented.The try/catch block correctly handles errors as requested in past reviews. The function properly filters state and clears selection when the delete succeeds, and surfaces errors when it fails.
packages/sdd-webapp/src/client/hooks/useSpecs.ts (7)
1-5: LGTM! Clean import structure.The imports are well-organized and all utilities are properly used throughout the hook implementation.
7-10: LGTM! Signature change supports archived tasks.The optional
optionsparameter withincludeArchivedflag aligns with the PR objectives for archived task support. The change is backward compatible.
15-25: LGTM! Counter-based loading pattern handles concurrent operations correctly.The loading count mechanism properly tracks multiple concurrent operations (e.g., when
refresh()calls bothfetchFileTreeandfetchStatus). TheMath.max(0, count - 1)safeguard instopLoadingprevents negative counts.
27-41: LGTM! Consistent fetch pattern with proper loading and error handling.The function correctly uses the new utilities (
fetchJson,apiRoutes,getErrorMessage) and properly manages loading state withstartLoading/stopLoadingin a try-finally pattern.
63-79: LGTM! Loading state now properly managed.The function correctly implements loading state management with
startLoading/stopLoading, addressing the previous review comment. TheincludeArchivedoption is properly threaded through to the API route, and the dependency onoptions?.includeArchivedensures re-fetching when the option changes.
90-92: LGTM! Proper concurrent refresh implementation.The
refreshfunction correctly usesPromise.allto fetch file tree and status concurrently, which is more efficient than sequential fetches. The loading count mechanism ensures the UI loading indicator behaves correctly during concurrent operations.
94-103: LGTM! Complete API surface with refresh capability.The returned object provides a well-structured API with both state values and action functions. The addition of
refreshenables callers to reload both file tree and status data.packages/sdd-webapp/src/client/lib/tree.ts (3)
23-31: LGTM!The recursive file counting logic is correct and handles all node types appropriately.
33-45: LGTM!The DFS logic correctly locates the first file matching the given name, with proper early-exit optimization.
47-67: LGTM!The null-sentinel pattern correctly distinguishes between "directory not found" (
null→[]) and "directory found but empty" ([]). The use of?? []at line 61 properly handles undefined children.
- Add htmlFor/id labels to form inputs in ProjectSelector - Update fetchSpecContent to use getErrorMessage utility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add REMOVE_PROJECT_VALUE constant in ProjectSelector - Add DEFAULT_ERROR_MESSAGE constant in useSpecs - Change interface to type for ProjectSelectorProps - Use getErrorMessage in ProjectSelector handleAdd 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (1)
95-136: Consider adding Enter key submission support.Users typically expect pressing Enter in form inputs to submit. Currently, the form requires clicking the "Add" button. You could wrap the inputs in a form element or add onKeyDown handlers.
🔎 Option 1: Wrap in a form element
<PopoverContent> - <div className="flex flex-col gap-2"> + <form + className="flex flex-col gap-2" + onSubmit={(e) => { + e.preventDefault(); + handleAdd(); + }} + > <div className="text-sm font-medium">Add project</div> <label htmlFor="project-path" className="text-xs text-muted-foreground" > Project path </label> <Input id="project-path" placeholder="e.g., /path/to/project" value={newPath} onChange={(e) => setNewPath(e.target.value)} /> <label htmlFor="project-name" className="text-xs text-muted-foreground" > Project name (optional) </label> <Input id="project-name" placeholder="e.g., My Project" value={newName} onChange={(e) => setNewName(e.target.value)} /> {error && <div className="text-xs text-destructive">{error}</div>} <div className="flex gap-2 pt-2"> - <Button type="button" onClick={handleAdd} disabled={submitting}> + <Button type="submit" disabled={submitting}> Add </Button> <Button type="button" variant="secondary" onClick={() => setShowAddForm(false)} > Cancel </Button> </div> - </div> + </form> </PopoverContent>🔎 Option 2: Add onKeyDown to inputs
<Input id="project-path" placeholder="e.g., /path/to/project" value={newPath} onChange={(e) => setNewPath(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter" && !submitting) { + e.preventDefault(); + handleAdd(); + } + }} /> <label htmlFor="project-name" className="text-xs text-muted-foreground" > Project name (optional) </label> <Input id="project-name" placeholder="e.g., My Project" value={newName} onChange={(e) => setNewName(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter" && !submitting) { + e.preventDefault(); + handleAdd(); + } + }} />packages/sdd-webapp/src/client/hooks/useSpecs.ts (1)
81-88: Consider splitting effects to avoid unnecessary file tree refetch.When
options?.includeArchivedchanges,fetchStatusis recreated (due to its dependency onoptions?.includeArchived), which triggers this effect to run, causing both file tree and status to refetch. Since the file tree doesn't depend onincludeArchived, it's refetched unnecessarily.For better efficiency, consider either:
- Creating separate effects for initial load vs. status refresh
- Memoizing just the
includeArchivedvalue if a stable reference pattern is preferredGiven that file tree fetches are likely fast/cached, this optimization can be deferred.
🔎 Example approach: separate effects
useEffect(() => { if (projectId) { fetchFileTree(); - fetchStatus(); setSelectedFile(null); setSpecContent(null); } -}, [projectId, fetchFileTree, fetchStatus]); +}, [projectId, fetchFileTree]); + +useEffect(() => { + if (projectId) { + fetchStatus(); + } +}, [projectId, fetchStatus]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
🧠 Learnings (5)
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Avoid creating unnecessary options; implement minimal code needed to fulfill requirements without setting default values
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Review API documentation and implement appropriate retry handling for potential errors in API calls
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Implement fail-fast principles in code to catch errors early
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
🧬 Code graph analysis (1)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (4)
packages/sdd-webapp/src/client/components/ui/popover.tsx (3)
Popover(31-31)PopoverTrigger(31-31)PopoverContent(31-31)packages/sdd-webapp/src/client/components/ui/button.tsx (1)
Button(57-57)packages/sdd-webapp/src/client/components/ui/input.tsx (1)
Input(22-22)packages/cc-plugin-lib/src/logger.ts (1)
error(130-136)
🔇 Additional comments (10)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx (4)
1-9: LGTM! Clean migration to shadcn/ui components.The imports are well-organized and all components are properly utilized in the refactored Popover-based UI.
30-30: LGTM! Solid async submission pattern.The submitting state properly tracks the in-flight operation, the finally block ensures cleanup, and the error handling gracefully keeps the popover open when submission fails. The button disable on line 124 prevents double-submission.
Also applies to: 47-64
83-89: LGTM! Previous review comment addressed.The onOpenChange handler now properly clears the error state when the popover closes, preventing stale error messages when reopening.
98-121: LGTM! Accessibility properly addressed.Both inputs now have proper accessible labels with htmlFor/id associations. Screen readers can announce field purposes, and the placeholders provide additional inline guidance.
packages/sdd-webapp/src/client/hooks/useSpecs.ts (6)
7-10: LGTM! Options parameter properly integrated.The function signature now accepts an optional
optionsparameter withincludeArchived, which aligns with the PR objective to add archived task support.
15-25: LGTM! Robust loading state management.The count-based loading mechanism correctly handles concurrent fetch operations. The defensive
Math.max(0, count - 1)prevents negative counts, and stableuseCallbackwrappers ensure predictable dependency behavior.
27-41: LGTM! Consistent fetch pattern.The function correctly manages loading state, error handling, and uses centralized API helpers.
43-61: ✅ Previous review issue resolved: error handling is now consistent.Line 55 now uses
getErrorMessageinstead of inline error handling, matching the pattern used in other fetch functions. The implementation is correct.
63-79: ✅ Previous review issue resolved: loading state is now managed.Lines 66 and 77 now properly call
startLoadingandstopLoading, ensuring the UI loading indicator reflects status fetch operations. The implementation is correct.
90-92: LGTM! Efficient concurrent refresh.Using
Promise.allto refresh both file tree and status concurrently is the correct approach.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdd-webapp/src/client/hooks/useSpecs.ts (1)
83-90: Missingoptionsdependency in useEffect will prevent archived status updates.The
useEffectdoesn't includeoptionsin its dependency array (line 90), butfetchStatusdepends onoptions?.includeArchived(line 81). When a user toggles the archived task visibility, the status won't refresh until the project changes, breaking the core archived task feature.🔎 Proposed fix to add options dependency
useEffect(() => { if (projectId) { fetchFileTree(); fetchStatus(); setSelectedFile(null); setSpecContent(null); } - }, [projectId, fetchFileTree, fetchStatus]); + }, [projectId, fetchFileTree, fetchStatus, options]);Note: Adding
optionsto the dependency array will cause the effect to re-run wheneveroptionschanges (including when the reference changes even if values are the same). If this causes unnecessary re-fetches, consider usingoptions?.includeArchiveddirectly in the dependency array instead, or memoize the options object in the parent component.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdd-webapp/src/client/components/ProjectSelector.tsx(4 hunks)packages/sdd-webapp/src/client/hooks/useSpecs.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdd-webapp/src/client/components/ProjectSelector.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Hook implementations must use TypeScript with Bun and cc-hooks-ts library, following the pattern: define hook with trigger and run properties, wrap run logic with wrapRun error handler, and include import.meta.main check for execution
Use HookLogger.fromFile(import.meta.filename) for file-based logging in hook implementations and import from @r_masseater/cc-plugin-lib
Files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
🧠 Learnings (4)
📚 Learning: 2025-12-18T06:09:50.220Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: packages/sdd-webapp/package.json:41-41
Timestamp: 2025-12-18T06:09:50.220Z
Learning: In packages/sdd-webapp/package.json, the dependency "typescript/native-preview" (TypeScript 7 native port) is intentionally used for faster type checking during development. The nightly version is tracked and updated regularly as part of the project's development workflow.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-18T06:16:40.499Z
Learnt from: masseater
Repo: masseater/claude-code-plugin PR: 8
File: plugins/sdd/hooks/require-phase-status.ts:29-53
Timestamp: 2025-12-18T06:16:40.499Z
Learning: In plugins/sdd/hooks/require-phase-status.ts, the Edit operation intentionally only checks for status removal (to prevent accidental deletion), while Write operation enforces the full status requirement for new phase files. Existing files being edited are assumed to already have valid status from their initial Write operation.
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Review API documentation and implement appropriate retry handling for potential errors in API calls
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
📚 Learning: 2025-12-19T03:50:44.474Z
Learnt from: CR
Repo: masseater/claude-code-plugin PR: 0
File: plugins/mutils/assets/AGENTS.md:0-0
Timestamp: 2025-12-19T03:50:44.474Z
Learning: Applies to plugins/mutils/assets/**/*.{ts,tsx,js,jsx} : Implement fail-fast principles in code to catch errors early
Applied to files:
packages/sdd-webapp/src/client/hooks/useSpecs.ts
🔇 Additional comments (7)
packages/sdd-webapp/src/client/hooks/useSpecs.ts (7)
1-7: LGTM! Clean migration to centralized utilities.The imports correctly reference the centralized API, error handling, and routing utilities. The
DEFAULT_ERROR_MESSAGEconstant provides consistency across all error handling.
9-12: LGTM! Options parameter enables archived task support.The optional
includeArchivedflag correctly implements the archived task functionality mentioned in the PR objectives while maintaining backward compatibility.
17-27: LGTM! Robust concurrent loading state management.The
loadingCountmechanism correctly handles multiple concurrent fetch operations. TheMath.max(0, count - 1)guard instopLoadingprevents negative counts, and both helpers are properly memoized.
29-43: LGTM! Consistent implementation with centralized utilities.The function correctly uses the refactored loading state management, centralized
fetchJson, and consistent error handling pattern.
45-63: LGTM! Consistent with the refactored fetch pattern.The function follows the same robust pattern as
fetchFileTreewith proper loading state, error handling, and memoization.
65-81: LGTM! Loading and error handling issues resolved.The function now correctly manages loading state (addresses previous review comment) and uses consistent error handling. The
includeArchivedoption is properly passed to the API route.
92-94: LGTM! Efficient concurrent refresh implementation.The
refreshfunction correctly usesPromise.allto fetch file tree and status concurrently, and is properly memoized with the correct dependencies.
1 participant
Summary
Testing
Notes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.