harden review findings fallback · Entire

harden review findings fallback

885cf3a→main·

pfleidi·2w ago·3 files·+114 added/-13 removed

Keep --profile separate from findings handles, fail requested handles when no findings exist, and quote generated view commands for copy-paste safety.

Add focused regressions for the non-interactive findings paths.

Sessions

76341f0fe352View transcript

Changes

3

149 unmodified lines

150
151
152
153
153
154
155
156
31 unmodified lines

188
189
190
191
191
192
193
193
194
195
196
197
198
199
200
20 unmodified lines

221
222
223
220
224
225
226
227

149 unmodified lines

if len(args) > 1 {
                return fmt.Errorf("accepts at most one argument, received %d", len(args))
            }
            if len(args) == 1 && profileOverride != "" {
            if len(args) == 1 && profileOverride != "" && !findings {
                return errors.New("pass profile either positionally or with --profile, not both")
            }
            return nil
            }
31 unmodified lines

if modelOverride != "" && agentOverride == "" {
                return errors.New("--model requires --agent (the model applies to a single reviewer)")
            }
            profileName := profileOverride
            positionalArg := ""
            if len(args) == 1 {
                profileName = args[0]
                positionalArg = args[0]
            }
            profileName := profileOverride
            if positionalArg != "" && !findings {
                profileName = positionalArg
            }
            if configure {
                return runReviewConfigure(ctx, cmd, profileName, reviewConfigureOptions{
20 unmodified lines

return RunReviewProfileConfigPicker(ctx, cmd.OutOrStdout(), deps.GetAgentsWithHooksInstalled, profileName)
            }
            if findings {
                return runReviewFindings(ctx, cmd, profileName, deps.NewSilentError)
                return runReviewFindings(ctx, cmd, positionalArg, deps.NewSilentError)
            }
            // Map the flag to the RunConfig timeout convention: a non-positive
            // value (the user passed --timeout 0) means "disable", encoded as the

Mcmd/entire/cli/review/cmd.go+8/-4

30 unmodified lines

31
32
33
34
35
36
37
38
39
40
41
42
43
44
38
45
46
47
48
75 unmodified lines

124
125
126
120
127
128
129
130
14 unmodified lines

145
146
147
141
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163

30 unmodified lines

if err != nil {
        return err
    }
    handle = strings.TrimSpace(handle)
    if len(manifests) == 0 {
        if handle != "" {
            err := fmt.Errorf("no local review findings match %q", handle)
            cmd.SilenceUsage = true
            fmt.Fprintln(cmd.ErrOrStderr(), err.Error())
            return wrapReviewSilentError(silentErr, err)
        }
        fmt.Fprintln(cmd.OutOrStdout(), "No local review findings found.")
        return nil
    }
    if handle = strings.TrimSpace(handle); handle != "" {
    if handle != "" {
        manifest, findErr := findReviewManifestByHandle(manifests, handle)
        if findErr != nil {
            cmd.SilenceUsage = true
75 unmodified lines

}
    fmt.Fprintln(w)
    fmt.Fprintln(w, "Browse findings:")
    fmt.Fprintf(w, "  %s --findings %s\n", reviewCommandBinary, handle)
    fmt.Fprintf(w, "  %s\n", reviewFindingsCommand(handle))
}

func reviewManifestHandle(manifest LocalReviewManifest) string {
14 unmodified lines

for _, manifest := range manifests {
        fmt.Fprintf(w, "%s\n", reviewManifestListLabel(manifest))
        if handle := reviewManifestHandle(manifest); handle != "" {
            fmt.Fprintf(w, "  view: %s --findings %s\n", reviewCommandBinary, handle)
            fmt.Fprintf(w, "  view: %s\n", reviewFindingsCommand(handle))
        }
    }
}

func reviewFindingsCommand(handle string) string {
    return fmt.Sprintf("%s --findings %s", reviewCommandBinary, quoteReviewFindingsHandle(handle))
}

func quoteReviewFindingsHandle(s string) string {
    return "'" + strings.ReplaceAll(s, "'", "'\\'\'") + "'"
}

func printReviewFindingsHandles(w io.Writer, manifests []LocalReviewManifest) {
    handles := reviewManifestHandleList(manifests)
    if len(handles) == 0 {

Mcmd/entire/cli/review/fix.go+18/-3

20 unmodified lines

21
22
23
24
25
26
27
28
133 unmodified lines

162
163
164
163
165
166
167
168
3 unmodified lines

172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
24 unmodified lines

225
226
227
203
228
229
230
206
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
17 unmodified lines

291
292
293
234
294
295
296
297
10 unmodified lines

308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
7 unmodified lines

343
344
345
264
346
347
348
349
2 unmodified lines

352
353
354
273
355
356
357
358

20 unmodified lines

const manifestTestCodexAgent = "codex"
const manifestTokenTestAgentName agenttypes.AgentName = "review-token-test"
const manifestTokenTestAgentType agenttypes.AgentType = "Review Token Test"
const manifestFindingsFlag = "--findings"
const manifestTestFinding = "finding"

func TestHydrateReviewSummaryTokensFromStates_PopulatesTokensFromSessionState(t *testing.T) {
    t.Parallel()
133 unmodified lines

writeReviewCompletionFooter(&b, manifest)

got := b.String()
    for _, want := range []string{"Review complete.", "entire review --findings claude-session"} {
    for _, want := range []string{"Review complete.", "entire review --findings 'claude-session'"} {
        if !strings.Contains(got, want) {
            t.Fatalf("footer missing %q:\n%s", want, got)
        }
3 unmodified lines

}
}

func TestReviewFindingCommandsQuoteShellHandles(t *testing.T) {
    manifest := LocalReviewManifest{
        Sources: []ManifestSource{{
            SessionID: "sid; echo pwned",
            Label: "Claude Code",
            Output: manifestTestFinding,
        }},
    }
    var list strings.Builder
    var footer strings.Builder

printReviewFindingsList(&list, []LocalReviewManifest{manifest})
    writeReviewCompletionFooter(&footer, manifest)

want := "entire review --findings 'sid; echo pwned'"
    if !strings.Contains(list.String(), "view: "+want) {
        t.Fatalf("list output missing quoted view command:\n%s", list.String())
    }
    if !strings.Contains(footer.String(), want) {
        t.Fatalf("footer output missing quoted view command:\n%s", footer.String())
    }
}

func TestPrintReviewFindingsList_ListsSessionsWithoutLocalPath(t *testing.T) {
    oldArgs := os.Args
    t.Cleanup(func() { os.Args = oldArgs })
24 unmodified lines

if !strings.Contains(got, "claude-session") {
        t.Fatalf("findings list missing session handle:\n%s", got)
    }
    if !strings.Contains(got, "view: entire review --findings claude-session") {
    if !strings.Contains(got, "view: entire review --findings 'claude-session'") {
        t.Fatalf("findings list missing view command:\n%s", got)
    }
    if !strings.Contains(got, "view: entire review --findings 20260508T110000") {
    if !strings.Contains(got, "view: entire review --findings '20260508T110000'") {
        t.Fatalf("findings list missing timestamp fallback handle:\n%s", got)
    }
}

func TestReviewFindingsCommand_ProfileFlagListsFindings(t *testing.T) {
    tmp := t.TempDir()
    testutil.InitRepo(t, tmp)
    testutil.WriteFile(t, tmp, "f.txt", "init")
    testutil.GitAdd(t, tmp, "f.txt")
    testutil.GitCommit(t, tmp, "init")
    t.Chdir(tmp)

if err := writeLocalReviewManifest(context.Background(), LocalReviewManifest{
        CreatedAt: time.Date(2026, 5, 7, 10, 0, 0, 0, time.UTC),
        Sources: []ManifestSource{{
            SessionID: "claude-session",
            Label: "Claude Code",
            Output: manifestTestFinding,
        }},
    }); err != nil {
        t.Fatalf("writeLocalReviewManifest: %v", err)
    }

cmd := NewCommand(Deps{})
    var out strings.Builder
    cmd.SetOut(&out)
    cmd.SetArgs([]string{manifestFindingsFlag, "--profile", "general"})

if err := cmd.Execute(); err != nil {
        t.Fatalf("execute review --findings --profile: %v", err)
    }
    got := out.String()
    for _, want := range []string{"Review Findings", "view: entire review --findings 'claude-session'"} {
        if !strings.Contains(got, want) {
            t.Fatalf("findings list missing %q:\n%s", want, got)
        }
    }
}

func TestReviewFindingsCommand_WithHandlePrintsFullDetail(t *testing.T) {
    tmp := t.TempDir()
    testutil.InitRepo(t, tmp)
17 unmodified lines

cmd := NewCommand(Deps{})
    var out strings.Builder
    cmd.SetOut(&out)
    cmd.SetArgs([]string{"--findings", "claude-session"})
    cmd.SetArgs([]string{manifestFindingsFlag, "claude-session"})

if err := cmd.Execute(); err != nil {
        t.Fatalf("execute review --findings handle: %v", err)
    }
}

func TestReviewFindingsCommand_HandleWithNoManifestsFails(t *testing.T) {
    tmp := t.TempDir()
    testutil.InitRepo(t, tmp)
    testutil.WriteFile(t, tmp, "f.txt", "init")
    testutil.GitAdd(t, tmp, "f.txt")
    testutil.GitCommit(t, tmp, "init")
    t.Chdir(tmp)

cmd := NewCommand(Deps{})
    var errOut strings.Builder
    cmd.SetErr(&errOut)
    cmd.SetArgs([]string{manifestFindingsFlag, "missing-session"})

if err := cmd.Execute(); err == nil {
        t.Fatal("expected unknown findings handle to fail")
    }
    got := errOut.String()
    if !strings.Contains(got, `no local review findings match "missing-session"`) {
        t.Fatalf("no-manifest handle error mismatch:\n%s", got)
    }
}

func TestReviewFindingsCommand_UnknownHandleListsValidHandles(t *testing.T) {
    tmp := t.TempDir()
    testutil.InitRepo(t, tmp)
7 unmodified lines

Sources: []ManifestSource{{
            SessionID: "claude-session",
            Label: "Claude Code",
            Output: "finding",
            Output: manifestTestFinding,
        }},
    }); err != nil {
        t.Fatalf("writeLocalReviewManifest: %v", err)