Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions cmd/roborev/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ Examples:

var gitRef string
var diffContent string
var dirtyFiles []string

if branch != "" {
// Branch review - review all commits since diverging from base
Expand Down Expand Up @@ -277,6 +278,10 @@ Examples:
if !hasChanges {
return fmt.Errorf("no uncommitted changes to review")
}
dirtyFiles, err = git.GetDirtyFilesChanged(root)
if err != nil {
return fmt.Errorf("get dirty files: %w", err)
}

// Generate dirty diff (includes untracked files).
// Use working-tree repo config (not default branch) for
Expand All @@ -296,7 +301,7 @@ Examples:
len(diffContent), MaxDirtyDiffSize)
}

if diffContent == "" {
if diffContent == "" && !prompt.HasDependencyMetadataFiles(dirtyFiles) {
return fmt.Errorf("no changes to review (diff is empty)")
}

Expand Down Expand Up @@ -325,7 +330,7 @@ Examples:
if panel != "" && panel != "none" && !quiet {
cmd.PrintErrf("Note: --panel %q is ignored with --local (panels require the daemon); running a single-agent review.\n", panel)
}
return runLocalReview(cmd, root, gitRef, diffContent, agent, model, provider, reasoning, reviewType, quiet, minSeverity)
return runLocalReview(cmd, root, gitRef, diffContent, dirtyFiles, agent, model, provider, reasoning, reviewType, quiet, minSeverity)
}

// Build request body
Expand All @@ -339,6 +344,7 @@ Examples:
Reasoning: reasoning,
ReviewType: reviewType,
DiffContent: diffContent,
DirtyFiles: dirtyFiles,
MinSeverity: minSeverity,
Panel: panel,
}
Expand Down Expand Up @@ -425,7 +431,7 @@ Examples:
}

// runLocalReview runs a review directly without the daemon
func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName, model, provider, reasoning, reviewType string, quiet bool, minSeverity string) error {
func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent string, dirtyFiles []string, agentName, model, provider, reasoning, reviewType string, quiet bool, minSeverity string) error {
ctx := cmd.Context()

// Load config
Expand Down Expand Up @@ -509,9 +515,9 @@ func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName
pb := prompt.NewBuilderWithConfig(nil, cfg).WithContext(ctx).ForRepo(repoPath, 0)
var reviewPrompt string
var snapshotCleanup func()
if diffContent != "" {
if diffContent != "" || len(dirtyFiles) > 0 {
// Dirty review
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshot(diffContent, cfg.ReviewContextCount, a.Name(), reviewType, resolvedMinSev)
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshotAndFiles(diffContent, dirtyFiles, cfg.ReviewContextCount, a.Name(), reviewType, resolvedMinSev)
reviewPrompt = dirtyResult.Prompt
snapshotCleanup = dirtyResult.Cleanup
err = dirtyErr
Expand Down
41 changes: 41 additions & 0 deletions internal/daemon/enqueue_descriptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,47 @@ func TestEnqueueDirtyUnchanged(t *testing.T) {
assert.Equal(diff, *claimed.DiffContent)
}

func TestEnqueueDirtyRejectsEmptyNonMetadataDiff(t *testing.T) {
server, _, _ := newTestServer(t)

repo := testutil.NewGitRepo(t)
repo.CommitFile("a.txt", "a", "add a")

req := testutil.MakeJSONRequest(t, http.MethodPost, "/api/enqueue", EnqueueRequest{
RepoPath: repo.Path(),
GitRef: "dirty",
Agent: "test",
DirtyFiles: []string{".cache/generated.bin"},
})
w := httptest.NewRecorder()
server.httpServer.Handler.ServeHTTP(w, req)

require.Equal(t, http.StatusBadRequest, w.Code, w.Body.String())
assert.Contains(t, w.Body.String(), "diff_content required")
}

func TestEnqueueDirtyMetadataOnlyPersistsDirtyFiles(t *testing.T) {
server, db, _ := newTestServer(t)

repo := testutil.NewGitRepo(t)
repo.CommitFile("a.txt", "a", "add a")

job := enqueueViaHTTP(t, server, EnqueueRequest{
RepoPath: repo.Path(),
GitRef: "dirty",
Agent: "test",
DirtyFiles: []string{"go.sum"},
})

claimed, err := db.ClaimJob("worker")
require.NoError(t, err)
require.Equal(t, job.ID, claimed.ID)
assert.Equal(t, storage.JobTypeDirty, claimed.JobType)
assert.Nil(t, claimed.DiffContent)
assert.Equal(t, []string{"go.sum"}, claimed.DirtyFiles)
assert.False(t, claimed.PromptPrebuilt, "dirty metadata should be rebuilt by the worker")
}

// TestEnqueueRangeUnchanged pins the range enqueue path: the symbolic
// "<a>..<b>" request is frozen to "<sha>..<sha>" and classified as a range.
func TestEnqueueRangeUnchanged(t *testing.T) {
Expand Down
9 changes: 7 additions & 2 deletions internal/daemon/panel_enqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"log"
"net/http"
"slices"
"strings"
"time"

Expand All @@ -15,6 +16,7 @@ import (
"go.kenn.io/roborev/internal/agent"
"go.kenn.io/roborev/internal/config"
"go.kenn.io/roborev/internal/git"
"go.kenn.io/roborev/internal/prompt"
"go.kenn.io/roborev/internal/storage"
)

Expand All @@ -31,6 +33,7 @@ type targetDescriptor struct {
sessionSHA string // SHA to key session reuse on ("" for prompt jobs)
patchID string
diffContent string
dirtyFiles []string
minSeverity string
worktreePath string
jobType string // req.JobType for prompt; "" lets EnqueueJob infer dirty/range/review
Expand All @@ -50,7 +53,7 @@ type targetDescriptor struct {
func (d targetDescriptor) baseOpts() storage.EnqueueOpts {
return storage.EnqueueOpts{
RepoID: d.repoID, CommitID: d.commitID, GitRef: d.gitRef, Branch: d.branch,
PatchID: d.patchID, DiffContent: d.diffContent, MinSeverity: d.minSeverity,
PatchID: d.patchID, DiffContent: d.diffContent, DirtyFiles: d.dirtyFiles, MinSeverity: d.minSeverity,
WorktreePath: d.worktreePath, JobType: d.jobType, Prompt: d.prompt,
PromptPrebuilt: d.promptPrebuilt, OutputPrefix: d.outputPrefix, Label: d.label,
Agentic: d.agentic, RequestedModel: d.requestedModel, RequestedProvider: d.requestedProvider,
Expand Down Expand Up @@ -198,7 +201,7 @@ func (s *Server) descriptorForPrompt(in freezeInputs) targetDescriptor {
func (s *Server) descriptorForDirty(
ctx context.Context, in freezeInputs,
) (targetDescriptor, *RawJSONOutput) {
if in.req.DiffContent == "" {
if in.req.DiffContent == "" && !prompt.HasDependencyMetadataFiles(in.req.DirtyFiles) {
out, _ := rawJSONOutput(http.StatusBadRequest,
ErrorResponse{Error: "diff_content required for dirty review"})
return targetDescriptor{}, out
Expand Down Expand Up @@ -231,6 +234,8 @@ func (s *Server) descriptorForDirty(
branch: in.req.Branch,
sessionSHA: targetSHA,
diffContent: in.req.DiffContent,
dirtyFiles: slices.Clone(in.req.DirtyFiles),
jobType: storage.JobTypeDirty,
minSeverity: in.normalizedMinSev,
worktreePath: in.worktreePath,
requestedModel: in.requestedModel,
Expand Down
20 changes: 16 additions & 4 deletions internal/daemon/rerun_panel.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,25 @@ func (s *Server) rerunPanelRun(job *storage.ReviewJob) (*RerunJobOutput, error)
return nil, huma.Error500InternalServerError(
fmt.Sprintf("load member diff: %v", diffErr))
}
memberOpts[i] = panelRerunMemberOpts(members[i], runUUID, diff, source)
dirtyFiles, filesErr := s.db.GetJobDirtyFiles(members[i].ID)
if filesErr != nil {
return nil, huma.Error500InternalServerError(
fmt.Sprintf("load member dirty files: %v", filesErr))
}
memberOpts[i] = panelRerunMemberOpts(members[i], runUUID, diff, dirtyFiles, source)
}

synthDiff, err := s.db.GetJobDiffContent(job.ID)
if err != nil {
return nil, huma.Error500InternalServerError(
fmt.Sprintf("load synthesis diff: %v", err))
}
synthOpts := panelRerunSynthesisOpts(job, runUUID, synthDiff, source)
synthDirtyFiles, err := s.db.GetJobDirtyFiles(job.ID)
if err != nil {
return nil, huma.Error500InternalServerError(
fmt.Sprintf("load synthesis dirty files: %v", err))
}
synthOpts := panelRerunSynthesisOpts(job, runUUID, synthDiff, synthDirtyFiles, source)

if _, _, err := s.db.EnqueuePanelRun(memberOpts, synthOpts); err != nil {
return nil, huma.Error500InternalServerError(
Expand Down Expand Up @@ -97,7 +107,7 @@ func (s *Server) panelRerunSource(job *storage.ReviewJob) (string, error) {
// (name/index/config), reassigning only the run UUID. Review/range/dirty prompts
// are rebuilt by the worker so reruns do not reuse stale prebuilt prompts. diff
// is the member's stored dirty diff (empty for commit/range targets).
func panelRerunMemberOpts(m storage.ReviewJob, runUUID, diff, source string) storage.EnqueueOpts {
func panelRerunMemberOpts(m storage.ReviewJob, runUUID, diff string, dirtyFiles []string, source string) storage.EnqueueOpts {
prompt := ""
if m.UsesStoredPrompt() {
prompt = m.Prompt
Expand All @@ -116,6 +126,7 @@ func panelRerunMemberOpts(m storage.ReviewJob, runUUID, diff, source string) sto
ReviewType: m.ReviewType,
PatchID: m.PatchID,
DiffContent: diff,
DirtyFiles: dirtyFiles,
Prompt: prompt,
PromptPrebuilt: false,
Source: source,
Expand All @@ -138,7 +149,7 @@ func panelRerunMemberOpts(m storage.ReviewJob, runUUID, diff, source string) sto
// panelRerunSynthesisOpts clones the synthesis parent into fresh EnqueueOpts for
// a new run. EnqueuePanelRun re-enforces JobType=synthesis, role=synthesis, and
// ClaimBlocked, but they are set here too so the opts are self-describing.
func panelRerunSynthesisOpts(job *storage.ReviewJob, runUUID, diff, source string) storage.EnqueueOpts {
func panelRerunSynthesisOpts(job *storage.ReviewJob, runUUID, diff string, dirtyFiles []string, source string) storage.EnqueueOpts {
return storage.EnqueueOpts{
RepoID: job.RepoID,
CommitID: job.CommitIDValue(),
Expand All @@ -153,6 +164,7 @@ func panelRerunSynthesisOpts(job *storage.ReviewJob, runUUID, diff, source strin
ReviewType: job.ReviewType,
PatchID: job.PatchID,
DiffContent: diff,
DirtyFiles: dirtyFiles,
OutputPrefix: job.OutputPrefix,
Source: source,
Agentic: job.Agentic,
Expand Down
37 changes: 19 additions & 18 deletions internal/daemon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,25 @@ package daemon
import "go.kenn.io/roborev/internal/storage"

type EnqueueRequest struct {
RepoPath string `json:"repo_path"`
CommitSHA string `json:"commit_sha,omitempty"` // Single commit (for backwards compat)
GitRef string `json:"git_ref,omitempty"` // Single commit, range like "abc..def", or "dirty"
Branch string `json:"branch,omitempty"` // Branch name at time of job creation
Since string `json:"since,omitempty"` // RFC3339 lower bound for insights datasets
Agent string `json:"agent,omitempty"`
Model string `json:"model,omitempty"` // Model to use (for opencode: provider/model format)
DiffContent string `json:"diff_content,omitempty"` // Pre-captured diff for dirty reviews
Reasoning string `json:"reasoning,omitempty"` // Reasoning level: thorough, standard, fast
ReviewType string `json:"review_type,omitempty"` // Review type (e.g., "security") — changes system prompt
CustomPrompt string `json:"custom_prompt,omitempty"` // Custom prompt for ad-hoc agent work
Agentic bool `json:"agentic,omitempty"` // Enable agentic mode (allow file edits)
OutputPrefix string `json:"output_prefix,omitempty"` // Prefix to prepend to review output
JobType string `json:"job_type,omitempty"` // Explicit job type (review/range/dirty/task/insights/compact/fix)
Provider string `json:"provider,omitempty"` // Provider for pi agent (e.g., "anthropic")
MinSeverity string `json:"min_severity,omitempty"` // Minimum severity filter: critical, high, medium, low
Panel string `json:"panel,omitempty"` // Panel name; "none" forces single-agent
Source string `json:"source,omitempty"` // Provenance, e.g. "post_commit" (empty = foreground)
RepoPath string `json:"repo_path"`
CommitSHA string `json:"commit_sha,omitempty"` // Single commit (for backwards compat)
GitRef string `json:"git_ref,omitempty"` // Single commit, range like "abc..def", or "dirty"
Branch string `json:"branch,omitempty"` // Branch name at time of job creation
Since string `json:"since,omitempty"` // RFC3339 lower bound for insights datasets
Agent string `json:"agent,omitempty"`
Model string `json:"model,omitempty"` // Model to use (for opencode: provider/model format)
DiffContent string `json:"diff_content,omitempty"` // Pre-captured diff for dirty reviews
DirtyFiles []string `json:"dirty_files,omitempty"` // Unfiltered dirty file names for prompt metadata
Reasoning string `json:"reasoning,omitempty"` // Reasoning level: thorough, standard, fast
ReviewType string `json:"review_type,omitempty"` // Review type (e.g., "security") — changes system prompt
CustomPrompt string `json:"custom_prompt,omitempty"` // Custom prompt for ad-hoc agent work
Agentic bool `json:"agentic,omitempty"` // Enable agentic mode (allow file edits)
OutputPrefix string `json:"output_prefix,omitempty"` // Prefix to prepend to review output
JobType string `json:"job_type,omitempty"` // Explicit job type (review/range/dirty/task/insights/compact/fix)
Provider string `json:"provider,omitempty"` // Provider for pi agent (e.g., "anthropic")
MinSeverity string `json:"min_severity,omitempty"` // Minimum severity filter: critical, high, medium, low
Panel string `json:"panel,omitempty"` // Panel name; "none" forces single-agent
Source string `json:"source,omitempty"` // Provenance, e.g. "post_commit" (empty = foreground)
}

// PanelEnqueueResponse is returned when an enqueue fans out into a panel run.
Expand Down
10 changes: 7 additions & 3 deletions internal/daemon/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,10 +599,14 @@ func (wp *WorkerPool) processJob(workerID string, job *storage.ReviewJob) {
minSev = resolved
}

if job.DiffContent != nil {
if job.IsDirtyJob() {
// Dirty job - use pre-captured diff
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshotTarget(
*job.DiffContent, cfg.ReviewContextCount, job.Agent, job.ReviewType, minSev,
diffContent := ""
if job.DiffContent != nil {
diffContent = *job.DiffContent
}
dirtyResult, dirtyErr := pb.BuildDirtyWithSnapshotTargetAndFiles(
diffContent, job.DirtyFiles, cfg.ReviewContextCount, job.Agent, job.ReviewType, minSev,
checkout.snapshotTarget,
)
if dirtyResult.Cleanup != nil {
Expand Down
34 changes: 34 additions & 0 deletions internal/daemon/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,40 @@ func TestProcessJob_UsesStoredReviewPromptOverride(t *testing.T) {
assert.Equal(t, job.Prompt, updated.Prompt)
}

func TestProcessJob_BuildsDirtyPromptFromPersistedDirtyFiles(t *testing.T) {
tc := newWorkerTestContext(t, 1)

var capturedPrompt string
agentName := "dirty-files-prompt-capture"
agent.Register(&agent.FakeAgent{
NameStr: agentName,
ReviewFn: func(ctx context.Context, repoPath, commitSHA, reviewPrompt string, output io.Writer) (string, error) {
capturedPrompt = reviewPrompt
return "No issues found.", nil
},
})
t.Cleanup(func() { agent.Unregister(agentName) })

job, err := tc.DB.EnqueueJob(storage.EnqueueOpts{
RepoID: tc.Repo.ID,
GitRef: "dirty",
Agent: agentName,
JobType: storage.JobTypeDirty,
DirtyFiles: []string{"go.sum"},
})
require.NoError(t, err)

claimed, err := tc.DB.ClaimJob(testWorkerID)
require.NoError(t, err)
require.Equal(t, job.ID, claimed.ID)

tc.Pool.processJob(testWorkerID, claimed)

tc.assertJobStatus(t, job.ID, storage.JobStatusDone)
assert.Contains(t, capturedPrompt, "## Dependency Metadata")
assert.Contains(t, capturedPrompt, "go.sum changed")
}

func TestProcessJob_PromotedAutoDesignAppendsExistingClassifierLog(t *testing.T) {
setupTestEnv(t)
tc := newWorkerTestContext(t, 1)
Expand Down
49 changes: 46 additions & 3 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,48 @@ func HasUncommittedChanges(repoPath string) (bool, error) {
return len(strings.TrimSpace(string(out))) > 0, nil
}

// GetDirtyFilesChanged returns changed file names for staged, unstaged, and
// untracked working-tree changes before review diff exclusions are applied.
func GetDirtyFilesChanged(repoPath string) ([]string, error) {
cmd := exec.Command("git", "status", "--porcelain=v1", "-uall")
cmd.Dir = repoPath

out, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("git status: %w", err)
}

seen := make(map[string]struct{})
for raw := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") {
line := strings.TrimSpace(raw)
if line == "" {
continue
}
path := strings.TrimSpace(line[2:])
if before, after, ok := strings.Cut(path, " -> "); ok {
seen[normalizeStatusPath(before)] = struct{}{}
seen[normalizeStatusPath(after)] = struct{}{}
continue
}
seen[normalizeStatusPath(path)] = struct{}{}
}

files := make([]string, 0, len(seen))
for file := range seen {
if file != "" {
files = append(files, file)
}
}
slices.Sort(files)
return files, nil
}

func normalizeStatusPath(path string) string {
path = strings.TrimSpace(path)
path = strings.Trim(path, `"`)
return path
}

// EmptyTreeSHA is the SHA of an empty tree in git, used for diffing against
// the root commit or repos with no commits.
const EmptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
Expand Down Expand Up @@ -901,8 +943,10 @@ func GetDirtyDiff(
return result.String(), nil
}

// excludedPathPatterns contains pathspec patterns for files that should be excluded from diffs.
// These are typically generated files that add noise to code reviews.
// excludedPathPatterns contains pathspec patterns for generated or mechanical
// files that add noise to code reviews. Prompt construction summarizes omitted
// dependency metadata separately so reviewers can still verify manifest/lockfile
// consistency without inlining large lockfile bodies.
// Uses :(exclude,glob)**/ form so patterns match at any directory depth.
// Directory patterns use :(exclude) without glob since git recognizes them as trees.
var excludedPathPatterns = []string{
Expand Down Expand Up @@ -938,7 +982,6 @@ var excludedPathPatterns = []string{
":(exclude,glob)**/Podfile.lock",
// Nix
":(exclude,glob)**/flake.lock",
// Directories — trailing /** matches all files inside at any depth
":(exclude,glob)**/.beads/**",
":(exclude,glob)**/.gocache/**",
":(exclude,glob)**/.cache/**",
Expand Down
Loading