feat: status command shipped#7
Conversation
📝 WalkthroughWalkthroughA new ChangesStatus command flow
Sequence Diagram(s)sequenceDiagram
participant rootCmd
participant statusCmd
participant validateDevLocalFilesystem as "filesystem.ValidateDevLocalFilesystem"
participant statusRun as "status.Run"
participant loadDevlocalConfig as "filesystem.LoadDevlocalConfig"
participant fileExists as "filesystem.FileExists"
participant isSkipWorktree as "git.IsSkipWorktree"
rootCmd->>statusCmd: invoke status subcommand
statusCmd->>validateDevLocalFilesystem: PreRunE validation
validateDevLocalFilesystem-->>statusCmd: validation result
statusCmd->>statusRun: RunE execution
statusRun->>loadDevlocalConfig: load devlocal config
loadDevlocalConfig-->>statusRun: config data
statusRun->>fileExists: check overlooked file path
fileExists-->>statusRun: existence / error
statusRun->>isSkipWorktree: inspect git state
isSkipWorktree-->>statusRun: skip-worktree flag
statusRun-->>statusCmd: status output or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/git/worktrees.go`:
- Around line 78-90: `IsSkipWorktree` currently treats an empty `git ls-files
-v` result as success, so unmatched paths are misreported downstream. Update
`IsSkipWorktree` in the worktrees helper to return an error when
`CombinedOutput` yields no output for the requested file, and then change
`displayOverlookStatus` in the status reporter to handle the returned error from
`git.IsSkipWorktree(filename)` instead of discarding it so non-tracked paths do
not show as “Being tracked”.
In `@internal/service/status/reporter.go`:
- Around line 41-47: The displayPatchesStatus function currently prints the
section header and returns without rendering anything when patches is non-empty,
so add the missing iteration over the patches slice inside displayPatchesStatus
to print each configured patch’s status before returning. Keep the empty-slice
branch for the “No patches registed…” message, and use the existing
displayPatchesStatus symbol so the status command shows patch entries whenever
configuration exists.
- Around line 27-29: The status reporter is ignoring errors from
git.IsSkipWorktree, which can leave the default “Being tracked” state even when
the check failed. Update the logic in the reporter’s skip-worktree check to
handle the returned error explicitly instead of using the blank identifier; if
git.IsSkipWorktree fails, surface that failure in the reported status/comment
rather than treating the file as healthy, and keep the normal
overlookStatus/overlookComment assignment only when the check succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5eeb8c27-932e-4c94-93cc-b6ed8cb63d8d
📒 Files selected for processing (4)
cmd/status.gointernal/filesystem/utils.gointernal/git/worktrees.gointernal/service/status/reporter.go
| func IsSkipWorktree(file string) (bool, error) { | ||
| out, err := exec.Command( | ||
| "git", | ||
| "ls-files", | ||
| "-v", | ||
| file, | ||
| ).CombinedOutput() | ||
|
|
||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| return len(out) > 0 && out[0] == 'S', nil |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
set +e
echo '$ git ls-files -v __coderabbit_unmatched_probe__'
git ls-files -v __coderabbit_unmatched_probe__
echo "exit=$?"
echo
echo '$ git ls-files -v --error-unmatch -- __coderabbit_unmatched_probe__'
git ls-files -v --error-unmatch -- __coderabbit_unmatched_probe__
echo "exit=$?"Repository: RohitRavindra-dev/devlocal
Length of output: 413
🏁 Script executed:
#!/bin/bash
set -u
echo '### internal/git/worktrees.go'
sed -n '1,180p' internal/git/worktrees.go
echo
echo '### internal/service/status/reporter.go'
sed -n '1,220p' internal/service/status/reporter.goRepository: RohitRavindra-dev/devlocal
Length of output: 3574
🏁 Script executed:
#!/bin/bash
set -u
rg -n "IsSkipWorktree\(" .Repository: RohitRavindra-dev/devlocal
Length of output: 355
🏁 Script executed:
#!/bin/bash
set -u
rg -n "IsSkipWorktree\(" .Repository: RohitRavindra-dev/devlocal
Length of output: 355
Make IsSkipWorktree failures propagate to status output. git ls-files -v can return no output for an unmatched path, and displayOverlookStatus currently discards the error from git.IsSkipWorktree(filename), so non-tracked paths still render as “Being tracked”. Make unmatched paths return an error in internal/git/worktrees.go and handle that error in internal/service/status/reporter.go instead of ignoring it.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 79-79: os/exec.Command must not be called. use os/exec.CommandContext
(noctx)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/git/worktrees.go` around lines 78 - 90, `IsSkipWorktree` currently
treats an empty `git ls-files -v` result as success, so unmatched paths are
misreported downstream. Update `IsSkipWorktree` in the worktrees helper to
return an error when `CombinedOutput` yields no output for the requested file,
and then change `displayOverlookStatus` in the status reporter to handle the
returned error from `git.IsSkipWorktree(filename)` instead of discarding it so
non-tracked paths do not show as “Being tracked”.
| if isOverlooked, _ := git.IsSkipWorktree(filename); isOverlooked { | ||
| overlookStatus = "✓" | ||
| overlookComment = "Overlooked" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't swallow IsSkipWorktree failures.
When git.IsSkipWorktree errors, the blank identifier leaves the default "Being tracked" message in place. That turns git/read failures into a false healthy status instead of surfacing that the inspection failed.
Suggested fix
- if isOverlooked, _ := git.IsSkipWorktree(filename); isOverlooked {
+ isOverlooked, err := git.IsSkipWorktree(filename)
+ if err != nil {
+ overlookComment = err.Error()
+ } else if isOverlooked {
overlookStatus = "✓"
overlookComment = "Overlooked"
}📝 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.
| if isOverlooked, _ := git.IsSkipWorktree(filename); isOverlooked { | |
| overlookStatus = "✓" | |
| overlookComment = "Overlooked" | |
| isOverlooked, err := git.IsSkipWorktree(filename) | |
| if err != nil { | |
| overlookComment = err.Error() | |
| } else if isOverlooked { | |
| overlookStatus = "✓" | |
| overlookComment = "Overlooked" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/status/reporter.go` around lines 27 - 29, The status
reporter is ignoring errors from git.IsSkipWorktree, which can leave the default
“Being tracked” state even when the check failed. Update the logic in the
reporter’s skip-worktree check to handle the returned error explicitly instead
of using the blank identifier; if git.IsSkipWorktree fails, surface that failure
in the reported status/comment rather than treating the file as healthy, and
keep the normal overlookStatus/overlookComment assignment only when the check
succeeds.
| func displayPatchesStatus(patches []string) { | ||
| fmt.Println("[Patches]") | ||
| if len(patches) == 0 { | ||
| fmt.Println("\t- No patches registed in config for patching!") | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Configured patches never render any status.
For any non-empty patches, this function prints [Patches] and exits without listing a single entry. The new status command therefore omits patch state exactly when patch configuration exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/service/status/reporter.go` around lines 41 - 47, The
displayPatchesStatus function currently prints the section header and returns
without rendering anything when patches is non-empty, so add the missing
iteration over the patches slice inside displayPatchesStatus to print each
configured patch’s status before returning. Keep the empty-slice branch for the
“No patches registed…” message, and use the existing displayPatchesStatus symbol
so the status command shows patch entries whenever configuration exists.
…into feat/v1/status
Summary by CodeRabbit
New Features
statuscommand to show the current devlocal setup status.Bug Fixes