Conversation
…cements Introduce /spec-bugfix-plan command for structured bug fix planning with root cause analysis. Spec dispatcher now auto-detects task type (feature vs bugfix) and routes accordingly. Console viewer shows spec type badges, extracted SpecHeaderCard component, and updated plan reader to parse Type field. Installer adds property-based testing tools. Documentation and workflow rules updated to reflect the dual-mode spec system.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Claude finished @maxritter's task in 3m 19s —— View job Code Review: feat: add bugfix spec workflow and viewer enhancements
OverviewThis PR adds a bugfix-specific spec planning flow alongside the existing feature flow, wires IssuesBug:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (UI)
participant Server as PlanRoutes
participant FS as FileSystem (worktree)
participant DB as Database
Client->>Server: GET /plans/:id (fetch plan)
Server->>FS: read plan file (parsePlanContent)
FS-->>Server: plan content + mtime (includes Type)
Server->>DB: fetch plan metadata / stats
DB-->>Server: metadata
Server-->>Client: JSON { planInfo: { ..., specType: "Feature"|"Bugfix" }, content, stats }
Client->>Client: render SpecHeaderCard and PlanStatus (show specType badge)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
console/src/ui/viewer/views/Spec/index.tsx (1)
285-291: Same year-omission issue asSpecHeaderCard.tsx.
toLocaleDateString(undefined, { month: "short", day: "numeric" })in the archived specs dropdown is missing theyearoption, mirroring the same issue flagged inSpecHeaderCard.tsxlines 147–152.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Spec/index.tsx` around lines 285 - 291, The archived-specs dropdown builds dateStr from spec.modifiedAt but omits the year; update the date formatting used when computing dateStr (the block that creates date and dateStr) to include the year option (e.g., year: "numeric") in the toLocaleDateString options so the displayed date shows month, day and year for spec.modifiedAt.
🧹 Nitpick comments (5)
installer/steps/dependencies.py (2)
546-556: [Optional] Move successreturntoelseto satisfy TRY300The
returnon the happy path sits inside thetryblock — ruff TRY300 flags this. Moving it to anelseclause also makes the control-flow intent explicit without changing behaviour. (S607 and BLE001 are consistent with_is_vtsls_installed/_is_ccusage_installedthroughout the file.)♻️ Proposed fix
def _is_fast_check_installed() -> bool: """Check if fast-check is installed globally via npm.""" try: result = subprocess.run( ["npm", "list", "-g", "fast-check", "--depth=0"], capture_output=True, text=True, ) - return result.returncode == 0 and "fast-check" in result.stdout except Exception: return False + else: + return result.returncode == 0 and "fast-check" in result.stdout🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@installer/steps/dependencies.py` around lines 546 - 556, The try block in _is_fast_check_installed currently returns on the success path; move the successful return out of the try into an else clause that executes when no exception is raised to satisfy TRY300 and match the pattern used by _is_vtsls_installed/_is_ccusage_installed: keep the subprocess.run call and its result inside try, catch exceptions and return False in except, then add an else that returns result.returncode == 0 and "fast-check" in result.stdout so control flow is explicit without changing behavior.
559-571: [Optional] Accumulate sub-step results to avoid false "installed" success signalBoth
_run_bash_with_retryreturn values are discarded and the function always returnsTrue, so_install_with_spinnerwill display "PBT tools … installed" and mark"pbt_tools"as done even when both installations fail. If accurate feedback is desired without blocking:♻️ Proposed fix
def install_pbt_tools() -> bool: """Install property-based testing packages: hypothesis (Python) and fast-check (TypeScript). Go PBT is handled by the built-in 'go test -fuzz' (Go 1.18+) — no install needed. Both packages are best-effort: failure does not block installation. """ + ok = True if not command_exists("hypothesis"): - _run_bash_with_retry("uv tool install hypothesis") + ok = _run_bash_with_retry("uv tool install hypothesis") and ok if not _is_fast_check_installed(): - _run_bash_with_retry(npm_global_cmd("npm install -g fast-check")) + ok = _run_bash_with_retry(npm_global_cmd("npm install -g fast-check")) and ok - return True + return ok🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@installer/steps/dependencies.py` around lines 559 - 571, install_pbt_tools always returns True even if the sub-installs fail; change it to capture the boolean results of the actions and return an aggregate success flag: call command_exists("hypothesis") and if False assign hypothesis_ok = _run_bash_with_retry("uv tool install hypothesis"), call _is_fast_check_installed() and if False assign fastcheck_ok = _run_bash_with_retry(npm_global_cmd("npm install -g fast-check")), then return combined_result = (hypothesis_ok if installation attempted else True) and (fastcheck_ok if installation attempted else True) so that install_pbt_tools reflects real success/failure.installer/tests/unit/steps/test_dependencies.py (1)
977-1037: [Optional] Add a test for the best-effort failure contractThe five tests cover all skip/install branches, but none verifies that
install_pbt_toolsstill returnsTruewhen_run_bash_with_retryfails for both tools. The documented contract ("failure does not block installation") would go undetected if a future change accidentally propagates failures.`@patch`("installer.steps.dependencies._run_bash_with_retry", return_value=False) `@patch`("installer.steps.dependencies._is_fast_check_installed", return_value=False) `@patch`("installer.steps.dependencies.command_exists", return_value=False) def test_install_pbt_tools_returns_true_on_install_failure(self, _mock_cmd, _mock_fc, _mock_run): """install_pbt_tools returns True even when both installations fail (best-effort).""" from installer.steps.dependencies import install_pbt_tools result = install_pbt_tools() assert result is True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@installer/tests/unit/steps/test_dependencies.py` around lines 977 - 1037, Add a unit test that asserts install_pbt_tools still returns True when both installs fail by patching _run_bash_with_retry to return False and ensuring both _is_fast_check_installed and command_exists return False (so installs are attempted but fail); call install_pbt_tools and assert result is True to validate the best-effort failure contract for the install_pbt_tools function.console/src/ui/viewer/views/Spec/index.tsx (1)
94-100: CallingsetStateduring render is non-idiomatic — prefer auseEffect.React permits this pattern (it re-renders the component immediately with the new state), but it can be surprising and triggers an extra render cycle. A
useEffectkeyed onselectedProjectis the conventional React approach:♻️ Proposed refactor
- const lastProjectRef = useRef(selectedProject); - - if (lastProjectRef.current !== selectedProject) { - lastProjectRef.current = selectedProject; - setSelectedSpec(null); - setContent(null); - setError(null); - setIsLoading(true); - } + useEffect(() => { + setSelectedSpec(null); + setContent(null); + setError(null); + setIsLoading(true); + }, [selectedProject]);Remove
useRefimport if it becomes unused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Spec/index.tsx` around lines 94 - 100, The component is calling set state during render when comparing lastProjectRef.current to selectedProject; move this reset logic into a useEffect that depends on [selectedProject] instead: in the effect compare lastProjectRef.current and selectedProject, update lastProjectRef.current, then call setSelectedSpec(null), setContent(null), setError(null), and setIsLoading(true); ensure you import/use useEffect and remove useRef import only if it's no longer needed elsewhere.console/src/services/worker/http/routes/utils/planFileReader.ts (1)
82-82: Conditional spread can be simplified.Because
PlanInfo.specTypeis already optional (specType?:), consumers rely on value equality (=== "Bugfix"), not key presence ("specType" in obj). AssigningspecTypedirectly is cleaner and equally correct:♻️ Proposed simplification
- ...(specType && { specType }), + specType,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/services/worker/http/routes/utils/planFileReader.ts` at line 82, The object currently uses a conditional spread ...(specType && { specType }) which is unnecessary because PlanInfo.specType is optional; replace that spread with a direct property assignment specType so the object literal includes specType: specType (or just specType) directly. Locate the object construction in planFileReader.ts where specType is added and change the conditional spread to a plain specType property to simplify the code and keep behavior identical for consumers checking value equality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/src/services/worker/http/routes/utils/planFileReader.ts`:
- Line 108: getWorktreePlansDirs currently swallows filesystem errors with an
empty catch block after calling fs.readdirSync; change that to catch the error
into a variable and log it the same way scanPlansDir does (e.g., use the same
logger and message style) and then return an empty array or otherwise handle the
failure rather than silently discarding it. Locate the catch following the
readdirSync call in getWorktreePlansDirs and replace the empty catch {} with a
catch (err) that logs the error (including err.message/stack) and returns/falls
back consistently with scanPlansDir's error handling.
- Around line 55-57: The parser currently maps any non-"Bugfix" Type to
undefined, so "Feature" in headers is never produced by specType; update the
extraction in planFileReader.ts to explicitly handle both "Bugfix" and "Feature"
(check typeMatch?.[1] and set specType to "Bugfix" | "Feature" accordingly) so
callers can distinguish real "Feature" values from unmarked, and then update the
related consumers (SpecHeaderCard.tsx and index.tsx) to accept the narrowed
union ("Bugfix" | "Feature") instead of assuming undefined represents "Feature".
In `@console/src/ui/viewer/views/Spec/index.tsx`:
- Around line 142-168: The deleteSpec frontend call is missing the project query
and handleDeletePlan backend uses a hardcoded project root; update deleteSpec to
append the same project parameter logic used in loadContent (add
`${selectedProject ? `&project=${encodeURIComponent(selectedProject)}` : ""}` to
the DELETE URL) so the request is project-scoped, and modify
PlanRoutes.handleDeletePlan to extract `const project = req.query.project as
string | undefined;` then replace the hardcoded `process.env.CLAUDE_PROJECT_ROOT
|| process.cwd()` with `const projectRoot = resolveProjectRoot(this.dbManager,
project);` so deletions use the correct project root and respect project-level
access controls.
In `@console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx`:
- Line 158: The code in SpecHeaderCard.tsx uses spec.filePath.split("/") which
fails on Windows backslashes; update the rendering of the filename (the span
showing spec.filePath.split("/").pop()) to derive the base name in a
cross-platform way — either have the server supply a basename (use path.basename
in planFileReader.ts) or change the client to split on both separators (e.g.,
split by / or \ using a regex) before taking the last segment so spec.filePath
always yields the correct filename regardless of OS.
- Around line 143-155: The displayed modified date for specs omits the year,
making dates ambiguous; update the rendering that uses new
Date(spec.modifiedAt).toLocaleString(...) (in SpecHeaderCard where
spec.modifiedAt is used) to include the year (e.g. add year: "numeric" to the
options) so the formatted string shows month, day, year, hour and minute
consistently.
---
Duplicate comments:
In `@console/src/ui/viewer/views/Spec/index.tsx`:
- Around line 285-291: The archived-specs dropdown builds dateStr from
spec.modifiedAt but omits the year; update the date formatting used when
computing dateStr (the block that creates date and dateStr) to include the year
option (e.g., year: "numeric") in the toLocaleDateString options so the
displayed date shows month, day and year for spec.modifiedAt.
---
Nitpick comments:
In `@console/src/services/worker/http/routes/utils/planFileReader.ts`:
- Line 82: The object currently uses a conditional spread ...(specType && {
specType }) which is unnecessary because PlanInfo.specType is optional; replace
that spread with a direct property assignment specType so the object literal
includes specType: specType (or just specType) directly. Locate the object
construction in planFileReader.ts where specType is added and change the
conditional spread to a plain specType property to simplify the code and keep
behavior identical for consumers checking value equality.
In `@console/src/ui/viewer/views/Spec/index.tsx`:
- Around line 94-100: The component is calling set state during render when
comparing lastProjectRef.current to selectedProject; move this reset logic into
a useEffect that depends on [selectedProject] instead: in the effect compare
lastProjectRef.current and selectedProject, update lastProjectRef.current, then
call setSelectedSpec(null), setContent(null), setError(null), and
setIsLoading(true); ensure you import/use useEffect and remove useRef import
only if it's no longer needed elsewhere.
In `@installer/steps/dependencies.py`:
- Around line 546-556: The try block in _is_fast_check_installed currently
returns on the success path; move the successful return out of the try into an
else clause that executes when no exception is raised to satisfy TRY300 and
match the pattern used by _is_vtsls_installed/_is_ccusage_installed: keep the
subprocess.run call and its result inside try, catch exceptions and return False
in except, then add an else that returns result.returncode == 0 and "fast-check"
in result.stdout so control flow is explicit without changing behavior.
- Around line 559-571: install_pbt_tools always returns True even if the
sub-installs fail; change it to capture the boolean results of the actions and
return an aggregate success flag: call command_exists("hypothesis") and if False
assign hypothesis_ok = _run_bash_with_retry("uv tool install hypothesis"), call
_is_fast_check_installed() and if False assign fastcheck_ok =
_run_bash_with_retry(npm_global_cmd("npm install -g fast-check")), then return
combined_result = (hypothesis_ok if installation attempted else True) and
(fastcheck_ok if installation attempted else True) so that install_pbt_tools
reflects real success/failure.
In `@installer/tests/unit/steps/test_dependencies.py`:
- Around line 977-1037: Add a unit test that asserts install_pbt_tools still
returns True when both installs fail by patching _run_bash_with_retry to return
False and ensuring both _is_fast_check_installed and command_exists return False
(so installs are attempted but fail); call install_pbt_tools and assert result
is True to validate the best-effort failure contract for the install_pbt_tools
function.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
launcher/statusline/formatter.pyis excluded by!launcher/**launcher/tests/unit/statusline/test_formatter.pyis excluded by!launcher/**launcher/tests/unit/test_worktree.pyis excluded by!launcher/**launcher/worktree.pyis excluded by!launcher/**pilot/commands/spec-bugfix-plan.mdis excluded by!pilot/**pilot/commands/spec-plan.mdis excluded by!pilot/**pilot/commands/spec.mdis excluded by!pilot/**pilot/rules/task-and-workflow.mdis excluded by!pilot/**pilot/scripts/mcp-server.cjsis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (14)
README.mdconsole/src/services/worker/http/routes/utils/planFileReader.tsconsole/src/ui/viewer/hooks/useStats.tsconsole/src/ui/viewer/views/Dashboard/PlanStatus.tsxconsole/src/ui/viewer/views/Spec/SpecHeaderCard.tsxconsole/src/ui/viewer/views/Spec/index.tsxconsole/tests/worker/plan-stats-endpoint.test.tsdocs/site/index.htmldocs/site/src/components/WorkflowSteps.tsxdocs/site/src/pages/docs/QuickModeSection.tsxdocs/site/src/pages/docs/SpecSection.tsxinstaller/steps/dependencies.pyinstaller/steps/finalize.pyinstaller/tests/unit/steps/test_dependencies.py
Comment on lines
+55
to
+57
| const typeMatch = content.match(/^Type:\s*(\w+)/m); | ||
| const specType = | ||
| typeMatch?.[1] === "Bugfix" ? ("Bugfix" as const) : undefined; |
There was a problem hiding this comment.
"Feature" is a dead value in the specType union — the parser never produces it.
typeMatch?.[1] === "Bugfix" maps every non-Bugfix value (including explicit Type: Feature) to undefined. The interface therefore exposes a value that can never occur at runtime, which can lead future readers/consumers to incorrectly branch on specType === "Feature".
Either also handle the "Feature" case so callers can distinguish "Feature" from "unmarked", or narrow the type to specType?: "Bugfix" to match what the parser actually produces.
🛠️ Option A – also capture "Feature" explicitly
- const specType =
- typeMatch?.[1] === "Bugfix" ? ("Bugfix" as const) : undefined;
+ const specType =
+ typeMatch?.[1] === "Bugfix"
+ ? ("Bugfix" as const)
+ : typeMatch?.[1] === "Feature"
+ ? ("Feature" as const)
+ : undefined;🛠️ Option B – narrow the interface to what is actually produced
- specType?: "Feature" | "Bugfix";
+ specType?: "Bugfix";(Same change needed in SpecHeaderCard.tsx and index.tsx.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const typeMatch = content.match(/^Type:\s*(\w+)/m); | |
| const specType = | |
| typeMatch?.[1] === "Bugfix" ? ("Bugfix" as const) : undefined; | |
| const typeMatch = content.match(/^Type:\s*(\w+)/m); | |
| const specType = | |
| typeMatch?.[1] === "Bugfix" | |
| ? ("Bugfix" as const) | |
| : typeMatch?.[1] === "Feature" | |
| ? ("Feature" as const) | |
| : undefined; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/utils/planFileReader.ts` around lines
55 - 57, The parser currently maps any non-"Bugfix" Type to undefined, so
"Feature" in headers is never produced by specType; update the extraction in
planFileReader.ts to explicitly handle both "Bugfix" and "Feature" (check
typeMatch?.[1] and set specType to "Bugfix" | "Feature" accordingly) so callers
can distinguish real "Feature" values from unmarked, and then update the related
consumers (SpecHeaderCard.tsx and index.tsx) to accept the narrowed union
("Bugfix" | "Feature") instead of assuming undefined represents "Feature".
| } | ||
| } catch { | ||
| } | ||
| } catch {} |
There was a problem hiding this comment.
Silent catch {} discards filesystem errors in getWorktreePlansDirs.
existsSync guards against a missing .worktrees directory, but readdirSync can still throw (e.g. EACCES, ENOTDIR). Those errors are silently swallowed here, while the same category of error in scanPlansDir is properly logged.
🛠️ Proposed fix
- } catch {}
+ } catch (error) {
+ logger.error("HTTP", "Failed to read worktrees directory", { worktreesDir }, error as Error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch {} | |
| } catch (error) { | |
| logger.error("HTTP", "Failed to read worktrees directory", { worktreesDir }, error as Error); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/utils/planFileReader.ts` at line 108,
getWorktreePlansDirs currently swallows filesystem errors with an empty catch
block after calling fs.readdirSync; change that to catch the error into a
variable and log it the same way scanPlansDir does (e.g., use the same logger
and message style) and then return an empty array or otherwise handle the
failure rather than silently discarding it. Locate the catch following the
readdirSync call in getWorktreePlansDirs and replace the empty catch {} with a
catch (err) that logs the error (including err.message/stack) and returns/falls
back consistently with scanPlansDir's error handling.
Comment on lines
+142
to
+168
| const deleteSpec = useCallback( | ||
| async (filePath: string) => { | ||
| if ( | ||
| !confirm( | ||
| `Delete spec "${filePath.split("/").pop()}"? This cannot be undone.`, | ||
| ) | ||
| ) | ||
| return; | ||
| setIsDeleting(true); | ||
| try { | ||
| const res = await fetch( | ||
| `/api/plan?path=${encodeURIComponent(filePath)}`, | ||
| { method: "DELETE" }, | ||
| ); | ||
| if (!res.ok) throw new Error("Failed to delete spec"); | ||
| setSelectedSpec(null); | ||
| setContent(null); | ||
| await loadSpecs(); | ||
| } catch (err) { | ||
| setError("Failed to delete spec"); | ||
| console.error("Failed to delete spec:", err); | ||
| } finally { | ||
| setIsDeleting(false); | ||
| } | ||
| setSelectedSpec(null); | ||
| setContent(null); | ||
| await loadSpecs(); | ||
| } catch (err) { | ||
| setError('Failed to delete spec'); | ||
| console.error('Failed to delete spec:', err); | ||
| } finally { | ||
| setIsDeleting(false); | ||
| } | ||
| }, [loadSpecs]); | ||
| }, | ||
| [loadSpecs], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the DELETE /api/plan route handler
rg -n "DELETE.*plan|plan.*DELETE" --type=ts --type=js -A 5 -B 2 | head -100Repository: maxritter/claude-pilot
Length of output: 50379
🏁 Script executed:
# Also search for the /api/plan route definition more broadly
rg -n "\/api\/plan" --type=ts --type=js -A 3 -B 3 | head -150Repository: maxritter/claude-pilot
Length of output: 143
🏁 Script executed:
# Look for the loadContent function to see how it uses the project parameter
rg -n "loadContent|const loadContent" --type=ts -A 10 -B 2 | head -100Repository: maxritter/claude-pilot
Length of output: 2674
🏁 Script executed:
# Find the deleteSpec function in the Spec file
rg -n "const deleteSpec|deleteSpec\s*=" console/src/ui/viewer/views/Spec/index.tsx -A 20Repository: maxritter/claude-pilot
Length of output: 801
🏁 Script executed:
# Search for the backend route handler for DELETE /api/plan
fd -e ts -e js -path "*/api/*" -type f | head -20Repository: maxritter/claude-pilot
Length of output: 234
🏁 Script executed:
# Search in the entire codebase for the plan route handler
rg -l "delete.*plan|plan.*delete" --type=ts --type=js -i | grep -v node_modules | grep -v dist | head -20Repository: maxritter/claude-pilot
Length of output: 188
🏁 Script executed:
# Examine the PlanRoutes file to find the DELETE handler
cat -n console/src/services/worker/http/routes/PlanRoutes.ts | head -150Repository: maxritter/claude-pilot
Length of output: 7206
🏁 Script executed:
# Search for DELETE method and plan handling
rg -n "DELETE|delete|router\.delete|app\.delete" console/src/services/worker/http/routes/PlanRoutes.ts -A 10 -B 2Repository: maxritter/claude-pilot
Length of output: 1413
🏁 Script executed:
# Find the handleDeletePlan method
rg -n "handleDeletePlan" console/src/services/worker/http/routes/PlanRoutes.ts -A 25Repository: maxritter/claude-pilot
Length of output: 2351
🏁 Script executed:
# Check the full file length to see if there's more context
wc -l console/src/services/worker/http/routes/PlanRoutes.tsRepository: maxritter/claude-pilot
Length of output: 123
🏁 Script executed:
# Check resolveProjectRoot to understand what it does with the project param
rg -n "resolveProjectRoot" console/src/services/worker/http/routes/PlanRoutes.ts -B 2 -A 2Repository: maxritter/claude-pilot
Length of output: 2179
🏁 Script executed:
# Look at the implementation of resolveProjectRoot
cat -n console/src/services/worker/http/routes/utils/resolveProjectRoot.tsRepository: maxritter/claude-pilot
Length of output: 1111
deleteSpec and its backend handler both omit project-scoped authorization—breaking multi-project support.
The frontend omits the project parameter when calling DELETE /api/plan, while loadContent (line 128) correctly includes it. More critically, the backend handleDeletePlan (PlanRoutes.ts:171) hardcodes process.env.CLAUDE_PROJECT_ROOT || process.cwd() instead of using resolveProjectRoot(this.dbManager, project) like every other handler in the class. This means deletions always target the default project root regardless of which project the user intended, breaking multi-project setups and potentially bypassing project-level access controls.
Fix required:
- Frontend: Append
${selectedProject ?&project=${encodeURIComponent(selectedProject)}: ""}to the DELETE request URL (like loadContent does). - Backend: Replace the hardcoded
process.env.CLAUDE_PROJECT_ROOT || process.cwd()withconst projectRoot = resolveProjectRoot(this.dbManager, project);at the start ofhandleDeletePlan, and extractconst project = req.query.project as string | undefined;like other handlers do.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/ui/viewer/views/Spec/index.tsx` around lines 142 - 168, The
deleteSpec frontend call is missing the project query and handleDeletePlan
backend uses a hardcoded project root; update deleteSpec to append the same
project parameter logic used in loadContent (add `${selectedProject ?
`&project=${encodeURIComponent(selectedProject)}` : ""}` to the DELETE URL) so
the request is project-scoped, and modify PlanRoutes.handleDeletePlan to extract
`const project = req.query.project as string | undefined;` then replace the
hardcoded `process.env.CLAUDE_PROJECT_ROOT || process.cwd()` with `const
projectRoot = resolveProjectRoot(this.dbManager, project);` so deletions use the
correct project root and respect project-level access controls.
Comment on lines
+143
to
+155
| {spec.modifiedAt && ( | ||
| <div className="flex items-center gap-1"> | ||
| <Icon icon="lucide:calendar" size={12} /> | ||
| <span> | ||
| {new Date(spec.modifiedAt).toLocaleString(undefined, { | ||
| month: "short", | ||
| day: "numeric", | ||
| hour: "2-digit", | ||
| minute: "2-digit", | ||
| })} | ||
| </span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Date format omits the year — ambiguous for multi-year archives.
toLocaleString(undefined, { month: "short", day: "numeric", hour: "2-digit", minute: "2-digit" }) renders e.g. "Feb 24, 10:30 AM" with no year. Verified/archived specs visible months or years later will be indistinguishable by date.
♻️ Proposed fix
{new Date(spec.modifiedAt).toLocaleString(undefined, {
+ year: "numeric",
month: "short",
day: "numeric",
hour: "2-digit",
minute: "2-digit",
})}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx` around lines 143 - 155,
The displayed modified date for specs omits the year, making dates ambiguous;
update the rendering that uses new Date(spec.modifiedAt).toLocaleString(...) (in
SpecHeaderCard where spec.modifiedAt is used) to include the year (e.g. add
year: "numeric" to the options) so the formatted string shows month, day, year,
hour and minute consistently.
| )} | ||
| <div className="flex items-center gap-1 ml-auto"> | ||
| <Icon icon="lucide:file" size={12} /> | ||
| <span className="font-mono">{spec.filePath.split("/").pop()}</span> |
There was a problem hiding this comment.
split("/") assumes Unix-style path separators.
spec.filePath originates from Node's path.join() in planFileReader.ts, which uses \ on Windows. Prefer path.basename (server-side) or a cross-platform split on the client:
♻️ Proposed fix
- <span className="font-mono">{spec.filePath.split("/").pop()}</span>
+ <span className="font-mono">{spec.filePath.split(/[\\/]/).pop()}</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span className="font-mono">{spec.filePath.split("/").pop()}</span> | |
| <span className="font-mono">{spec.filePath.split(/[\\/]/).pop()}</span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx` at line 158, The code in
SpecHeaderCard.tsx uses spec.filePath.split("/") which fails on Windows
backslashes; update the rendering of the filename (the span showing
spec.filePath.split("/").pop()) to derive the base name in a cross-platform way
— either have the server supply a basename (use path.basename in
planFileReader.ts) or change the client to split on both separators (e.g., split
by / or \ using a regex) before taking the last segment so spec.filePath always
yields the correct filename regardless of OS.
- Add project query param to deleteSpec frontend call and use
resolveProjectRoot in handleDeletePlan backend (multi-project fix)
- Replace unreliable command_exists("hypothesis") with _is_hypothesis_installed
using uv tool list
- Change ruff check to uv run ruff check in spec-bugfix-plan.md
- Add year to date formatting in SpecHeaderCard and archived specs dropdown
- Log errors instead of silently swallowing in getWorktreePlansDirs
- Return actual success/failure from install_pbt_tools instead of always True
|
Claude finished @maxritter's task in 3m 52s —— View job Incremental Code Review: feat: add bugfix spec workflow and viewer enhancements
Resolved IssuesThe following issues from the previous review have been fixed in this push: From prior Claude review:
From CodeRabbit review:
Still Open(One-line each — no full detail since unchanged)
New IssueMinor:
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
installer/tests/unit/steps/test_dependencies.py (1)
34-57:⚠️ Potential issue | 🟡 MinorAssert the new PBT tools step is invoked.
The test patches
install_pbt_toolsbut doesn’t assert it was called, so the new dependency step isn’t actually covered.✅ Suggested assertion
mock_python_tools.assert_called_once() mock_claude.assert_called_once() mock_plugin_deps.assert_called_once() + _mock_pbt_tools.assert_called_once()As per coding guidelines, review test code briefly. Focus on: Test coverage for the feature being tested.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@installer/tests/unit/steps/test_dependencies.py` around lines 34 - 57, The test test_dependencies_run_installs_core currently patches install_pbt_tools but never asserts it was invoked; update this test to add an assertion that the mocked install_pbt_tools was called (use the patched mock parameter named _mock_pbt_tools or rename it to mock_pbt_tools) after running the code under test, similar to how other patched installers are asserted; ensure the assertion checks at least called_once() (or called) so the new PBT tools step is actually covered.
♻️ Duplicate comments (1)
console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx (1)
158-160:⚠️ Potential issue | 🟡 MinorUse a cross‑platform basename for
spec.filePath.Line 160 uses
split("/"), which fails on Windows path separators. This pattern also appears in the Spec view delete confirmation. Use a/[\\/]/split or a shared helper.🛠️ Proposed fix
- <span className="font-mono">{spec.filePath.split("/").pop()}</span> + <span className="font-mono"> + {spec.filePath.split(/[\\/]/).pop()} + </span>#!/bin/bash # Find other filePath basename extractions that use "/" only rg -n 'filePath\.split\("/"\)' --type=ts --type=tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx` around lines 158 - 160, The code in SpecHeaderCard (and the duplicate in the Spec view delete confirmation) uses spec.filePath.split("/") which breaks on Windows; replace these occurrences with a cross-platform basename approach — either call a shared helper like getBasename(filePath) or change the split to use a RegExp split(/[\\/]/) and then .pop(), and update both SpecHeaderCard and the delete-confirmation code to use the same helper/function to avoid duplication.
🧹 Nitpick comments (2)
docs/site/src/pages/docs/SpecSection.tsx (1)
115-117: Remove the superfluous empty<span>.
{/* bugfix auto-detected */}is a JSX comment; it renders nothing at runtime. The parent<span>is therefore an empty DOM node with Tailwind classes applied to nothing. Move the comment inline and drop the span entirely.♻️ Proposed fix
- <span className="text-muted-foreground/60 text-xs ml-2"> - {/* bugfix auto-detected */} - </span> + {/* bugfix auto-detected */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/pages/docs/SpecSection.tsx` around lines 115 - 117, Remove the empty span that contains only a JSX comment in the SpecSection component: the <span className="text-muted-foreground/60 text-xs ml-2"> wrapping {/* bugfix auto-detected */} should be deleted and the JSX comment moved inline where it belongs (or removed entirely), ensuring no empty DOM node remains; search for the span in SpecSection.tsx to locate the exact element to remove.console/tests/worker/plan-stats-endpoint.test.ts (1)
57-62: SpecType test only asserts source strings—add a behavioral parse check.This test only inspects source text, so a regression in
parsePlanContentcould still pass. Consider a unit test that feeds a plan string withType: Bugfix(and one withoutType) and asserts the parsedspecType.As per coding guidelines,
**/tests/**: Review test code briefly. Focus on: Test coverage for the feature being tested🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/tests/worker/plan-stats-endpoint.test.ts` around lines 57 - 62, The current test only checks raw readerSource strings; instead add behavioral assertions that call parsePlanContent directly: create one plan string containing "Type: Bugfix" and another without a Type line, pass each to parsePlanContent and assert the returned object has specType === "Bugfix" for the first and specType === undefined (or null/empty per implementation) for the second; update the test in plan-stats-endpoint.test.ts (which currently references readerSource) to include these parsePlanContent calls and assertions so regressions in parsing are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/src/services/worker/http/routes/PlanRoutes.ts`:
- Around line 51-55: The worktree path checks are not robust: replace the
current startsWith(worktreesDir) and resolvedPath.includes("/docs/plans/") logic
with the same cross-platform, separator-aware checks used for mainPlansDir —
ensure you check for worktreesDir followed by a separator boundary (e.g.,
worktreesDir + path.sep and worktreesDir + "/") to avoid matching prefixes like
".worktreesX", and check for the docs/plans segment using both path.sep and "/"
(or build it via path.join and test both separators) so resolvedPath is properly
matched on Windows and POSIX; update the checks around worktreesDir,
normalizedRoot, and resolvedPath in PlanRoutes.ts accordingly.
In `@installer/steps/dependencies.py`:
- Around line 546-589: The current check/install flow wrongly uses `uv tool
install hypothesis` for a non-CLI library and silences errors; update
_is_hypothesis_installed to detect the correct distribution and change
install_pbt_tools to install the correct package (either use `uv tool install
"hypothesis[cli]"` if you truly need the CLI or switch to `uv add
hypothesis`/project dependency management if you mean the library), and ensure
you call _run_bash_with_retry with the corrected command instead of the existing
string; also modify _is_hypothesis_installed and _is_fast_check_installed to log
exceptions (include the exception text) rather than swallowing them, and when
install_pbt_tools returns False log which tool failed (reference functions:
_is_hypothesis_installed, _is_fast_check_installed, install_pbt_tools,
_run_bash_with_retry, npm_global_cmd) so the user sees clear feedback on
failure.
---
Outside diff comments:
In `@installer/tests/unit/steps/test_dependencies.py`:
- Around line 34-57: The test test_dependencies_run_installs_core currently
patches install_pbt_tools but never asserts it was invoked; update this test to
add an assertion that the mocked install_pbt_tools was called (use the patched
mock parameter named _mock_pbt_tools or rename it to mock_pbt_tools) after
running the code under test, similar to how other patched installers are
asserted; ensure the assertion checks at least called_once() (or called) so the
new PBT tools step is actually covered.
---
Duplicate comments:
In `@console/src/ui/viewer/views/Spec/SpecHeaderCard.tsx`:
- Around line 158-160: The code in SpecHeaderCard (and the duplicate in the Spec
view delete confirmation) uses spec.filePath.split("/") which breaks on Windows;
replace these occurrences with a cross-platform basename approach — either call
a shared helper like getBasename(filePath) or change the split to use a RegExp
split(/[\\/]/) and then .pop(), and update both SpecHeaderCard and the
delete-confirmation code to use the same helper/function to avoid duplication.
---
Nitpick comments:
In `@console/tests/worker/plan-stats-endpoint.test.ts`:
- Around line 57-62: The current test only checks raw readerSource strings;
instead add behavioral assertions that call parsePlanContent directly: create
one plan string containing "Type: Bugfix" and another without a Type line, pass
each to parsePlanContent and assert the returned object has specType ===
"Bugfix" for the first and specType === undefined (or null/empty per
implementation) for the second; update the test in plan-stats-endpoint.test.ts
(which currently references readerSource) to include these parsePlanContent
calls and assertions so regressions in parsing are caught.
In `@docs/site/src/pages/docs/SpecSection.tsx`:
- Around line 115-117: Remove the empty span that contains only a JSX comment in
the SpecSection component: the <span className="text-muted-foreground/60 text-xs
ml-2"> wrapping {/* bugfix auto-detected */} should be deleted and the JSX
comment moved inline where it belongs (or removed entirely), ensuring no empty
DOM node remains; search for the span in SpecSection.tsx to locate the exact
element to remove.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
launcher/statusline/formatter.pyis excluded by!launcher/**launcher/tests/unit/statusline/test_formatter.pyis excluded by!launcher/**launcher/tests/unit/test_worktree.pyis excluded by!launcher/**launcher/worktree.pyis excluded by!launcher/**pilot/commands/spec-bugfix-plan.mdis excluded by!pilot/**pilot/commands/spec-plan.mdis excluded by!pilot/**pilot/commands/spec.mdis excluded by!pilot/**pilot/rules/task-and-workflow.mdis excluded by!pilot/**pilot/scripts/mcp-server.cjsis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (15)
README.mdconsole/src/services/worker/http/routes/PlanRoutes.tsconsole/src/services/worker/http/routes/utils/planFileReader.tsconsole/src/ui/viewer/hooks/useStats.tsconsole/src/ui/viewer/views/Dashboard/PlanStatus.tsxconsole/src/ui/viewer/views/Spec/SpecHeaderCard.tsxconsole/src/ui/viewer/views/Spec/index.tsxconsole/tests/worker/plan-stats-endpoint.test.tsdocs/site/index.htmldocs/site/src/components/WorkflowSteps.tsxdocs/site/src/pages/docs/QuickModeSection.tsxdocs/site/src/pages/docs/SpecSection.tsxinstaller/steps/dependencies.pyinstaller/steps/finalize.pyinstaller/tests/unit/steps/test_dependencies.py
Comment on lines
51
to
+55
| const worktreesDir = path.join(normalizedRoot, ".worktrees"); | ||
| if (resolvedPath.startsWith(worktreesDir) && resolvedPath.includes("/docs/plans/")) { | ||
| if ( | ||
| resolvedPath.startsWith(worktreesDir) && | ||
| resolvedPath.includes("/docs/plans/") | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n console/src/services/worker/http/routes/PlanRoutes.ts | sed -n '45,60p'Repository: maxritter/claude-pilot
Length of output: 519
🏁 Script executed:
rg -n 'docs/plans' --type=tsRepository: maxritter/claude-pilot
Length of output: 6407
🏁 Script executed:
rg -n 'worktreesDir|\.worktrees' --type=ts -A 2 -B 2Repository: maxritter/claude-pilot
Length of output: 4949
Normalize worktree path checks to match mainPlansDir pattern.
The worktree validation on lines 53-54 is less robust than the mainPlansDir check immediately above it (lines 46-47). Line 54's hardcoded "/docs/plans/" will fail on Windows, and line 53's startsWith(worktreesDir) lacks a separator boundary, allowing .worktreesX prefixes to match. The mainPlansDir check already uses both path.sep and "/" for cross-platform safety; apply the same approach here:
Proposed fix
- const worktreesDir = path.join(normalizedRoot, ".worktrees");
- if (
- resolvedPath.startsWith(worktreesDir) &&
- resolvedPath.includes("/docs/plans/")
- ) {
+ const worktreesDir = path.join(normalizedRoot, ".worktrees");
+ if (
+ (resolvedPath.startsWith(worktreesDir + path.sep) ||
+ resolvedPath.startsWith(worktreesDir + "/")) &&
+ (resolvedPath.includes(`${path.sep}docs${path.sep}plans${path.sep}`) ||
+ resolvedPath.includes("/docs/plans/"))
+ ) {
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const worktreesDir = path.join(normalizedRoot, ".worktrees"); | |
| if (resolvedPath.startsWith(worktreesDir) && resolvedPath.includes("/docs/plans/")) { | |
| if ( | |
| resolvedPath.startsWith(worktreesDir) && | |
| resolvedPath.includes("/docs/plans/") | |
| ) { | |
| const worktreesDir = path.join(normalizedRoot, ".worktrees"); | |
| if ( | |
| (resolvedPath.startsWith(worktreesDir + path.sep) || | |
| resolvedPath.startsWith(worktreesDir + "/")) && | |
| (resolvedPath.includes(`${path.sep}docs${path.sep}plans${path.sep}`) || | |
| resolvedPath.includes("/docs/plans/")) | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/PlanRoutes.ts` around lines 51 - 55,
The worktree path checks are not robust: replace the current
startsWith(worktreesDir) and resolvedPath.includes("/docs/plans/") logic with
the same cross-platform, separator-aware checks used for mainPlansDir — ensure
you check for worktreesDir followed by a separator boundary (e.g., worktreesDir
+ path.sep and worktreesDir + "/") to avoid matching prefixes like
".worktreesX", and check for the docs/plans segment using both path.sep and "/"
(or build it via path.join and test both separators) so resolvedPath is properly
matched on Windows and POSIX; update the checks around worktreesDir,
normalizedRoot, and resolvedPath in PlanRoutes.ts accordingly.
Comment on lines
+546
to
+589
| def _is_hypothesis_installed() -> bool: | ||
| """Check if hypothesis is installed via uv tool.""" | ||
| try: | ||
| result = subprocess.run( | ||
| ["uv", "tool", "list"], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=10, | ||
| ) | ||
| return result.returncode == 0 and "hypothesis" in result.stdout | ||
| except Exception: | ||
| return False | ||
|
|
||
|
|
||
| def _is_fast_check_installed() -> bool: | ||
| """Check if fast-check is installed globally via npm.""" | ||
| try: | ||
| result = subprocess.run( | ||
| ["npm", "list", "-g", "fast-check", "--depth=0"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| return result.returncode == 0 and "fast-check" in result.stdout | ||
| except Exception: | ||
| return False | ||
|
|
||
|
|
||
| def install_pbt_tools() -> bool: | ||
| """Install property-based testing packages: hypothesis (Python) and fast-check (TypeScript). | ||
|
|
||
| Go PBT is handled by the built-in 'go test -fuzz' (Go 1.18+) — no install needed. | ||
| Both packages are best-effort: failure does not block installation. | ||
| """ | ||
| ok = True | ||
|
|
||
| if not _is_hypothesis_installed(): | ||
| if not _run_bash_with_retry("uv tool install hypothesis"): | ||
| ok = False | ||
|
|
||
| if not _is_fast_check_installed(): | ||
| if not _run_bash_with_retry(npm_global_cmd("npm install -g fast-check")): | ||
| ok = False | ||
|
|
||
| return ok |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uv tool install hypothesis Python package support CLI requirements
💡 Result:
uv tool install is for installing executables (“commands”) exposed by a Python package (i.e., console/script entry points) into an isolated “tool” environment and linking them onto your PATH. [1][2]
Hypothesis + CLI with uv tool install
Hypothesis can have a CLI, but the CLI dependencies are provided as an optional extra named cli (e.g., click, black, rich). [3]
So, if you want the Hypothesis CLI available as a globally-invokable tool, install it with the extra:
uv tool install "hypothesis[cli]"(Or run it without installing via uvx, which is an alias for uv tool run.) [2]
If you mean “requirements” (project dependencies) instead of a global tool
Use project/requirements.txt workflows instead of uv tool install, e.g.:
uv add hypothesis
# or
uv pip install -r requirements.txtSources: uv tool install reference [1], uv tools concept + uvx [2], Hypothesis optional cli extra [3].
Fix uv tool install hypothesis — wrong approach for non-CLI library and missing user feedback.
uv tool install is for CLI tools with console entry points; Hypothesis is a library without a CLI by default. This will always fail silently. The try/except blocks (lines 549–551, 564–566) catch exceptions but return False without logging or user feedback, leaving the developer unaware why the installation failed. Additionally, when install_pbt_tools() returns False (lines 586–587), there's no message explaining which tool failed or why.
Recommended fixes:
- Use
uv tool install "hypothesis[cli]"if a CLI is truly needed, or switch to a proper dependency management approach (e.g.,uv add hypothesisfor project dependencies). - Add logging or user-facing error messages when tool installations fail (violates "proper error handling and user feedback" guideline for installer code).
- Consider documenting the intent: are these testing libraries meant for the installer environment or for projects it manages?
🧰 Tools
🪛 Ruff (0.15.2)
[error] 550-550: Starting a process with a partial executable path
(S607)
[warning] 555-555: Consider moving this statement to an else block
(TRY300)
[warning] 556-556: Do not catch blind exception: Exception
(BLE001)
[error] 564-564: Starting a process with a partial executable path
(S607)
[warning] 568-568: Consider moving this statement to an else block
(TRY300)
[warning] 569-569: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@installer/steps/dependencies.py` around lines 546 - 589, The current
check/install flow wrongly uses `uv tool install hypothesis` for a non-CLI
library and silences errors; update _is_hypothesis_installed to detect the
correct distribution and change install_pbt_tools to install the correct package
(either use `uv tool install "hypothesis[cli]"` if you truly need the CLI or
switch to `uv add hypothesis`/project dependency management if you mean the
library), and ensure you call _run_bash_with_retry with the corrected command
instead of the existing string; also modify _is_hypothesis_installed and
_is_fast_check_installed to log exceptions (include the exception text) rather
than swallowing them, and when install_pbt_tools returns False log which tool
failed (reference functions: _is_hypothesis_installed, _is_fast_check_installed,
install_pbt_tools, _run_bash_with_retry, npm_global_cmd) so the user sees clear
feedback on failure.
…eping Playwright install tests and auth fallback test were hitting real time.sleep(2) during retry loops, adding ~14s to CI. Patch time.sleep in the 4 affected tests.
|
Claude finished @maxritter's task in 1m 48s —— View job Incremental Code Review: feat: add bugfix spec workflow and viewer enhancements
Push SummaryThis push ( Changes in This Push
Three tests had
The fix is correct. Mock parameter ordering matches decorator application order (bottom-to-top) in all three cases. The No new issues introduced. Still Open(Unresolved from prior reviews — one line each)
|
github-actions bot
pushed a commit
that referenced
this pull request
Feb 24, 2026
# [6.11.0](v6.10.3...v6.11.0) (2026-02-24) ### Features * add bugfix spec workflow and viewer enhancements ([#85](#85)) ([70c1119](70c1119))
Contributor
|
🎉 This PR is included in version 6.11.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/spec-bugfix-plancommand for structured bug fix planning with root cause analysis, behavior contracts, and fixed task structure/spec) auto-detects task type (feature vs bugfix) and routes to the appropriate planning flowType:field from plan files, show bugfix badges in dashboard and spec views, extractSpecHeaderCardcomponenttask-and-workflow.md) to document bugfix spec dispatch and verification agent behaviorTest plan
Summary by CodeRabbit
New Features
Documentation
Tests