diff --git a/cmd/entire/cli/lifecycle.go b/cmd/entire/cli/lifecycle.go index 44bda39a8d..f62868caf3 100644 --- a/cmd/entire/cli/lifecycle.go +++ b/cmd/entire/cli/lifecycle.go @@ -1113,7 +1113,10 @@ func adoptReviewEnv(ctx context.Context, state *session.State, expectedAgent str if state.Kind != "" { return } - if os.Getenv(review.EnvSession) != "1" { + if envSession := os.Getenv(review.EnvSession); envSession != "1" { + logging.Debug(ctx, "review env adoption skipped: ENTIRE_REVIEW_SESSION is not \"1\"", + slog.String("expected_agent", expectedAgent), + slog.String("observed_value", envSession)) return } envAgent := os.Getenv(review.EnvAgent) diff --git a/cmd/entire/cli/review/cmd.go b/cmd/entire/cli/review/cmd.go index 68865c154c..07b0f38c65 100644 --- a/cmd/entire/cli/review/cmd.go +++ b/cmd/entire/cli/review/cmd.go @@ -648,19 +648,35 @@ func writePostReviewManifest( manifest, err := localReviewManifestFromCurrentState(ctx, worktreeRoot, headSHA, summary, aggregateOutput) if err != nil { logging.Debug(ctx, "review manifest not written", slog.String("error", err.Error())) + warnManifestNotWritten(out, "could not load session state: "+err.Error()) return } if len(manifest.Sources) == 0 { logging.Debug(ctx, "review manifest not written: no matching review sessions") + warnManifestNotWritten(out, "review session was not tagged as a review (env-var handshake did not reach the hook)") return } if err := writeLocalReviewManifest(ctx, manifest); err != nil { logging.Debug(ctx, "review manifest write failed", slog.String("error", err.Error())) + warnManifestNotWritten(out, "write to disk failed: "+err.Error()) return } writeReviewCompletionFooter(out, manifest) } +// warnManifestNotWritten prints a user-visible note explaining that the +// review skills ran but findings were not persisted, so `entire review +// --findings` and `entire review --fix` will not see this run. The reason +// string is appended verbatim and should describe the underlying cause in +// terms the user can act on (or at least diagnose with debug logs). +func warnManifestNotWritten(out io.Writer, reason string) { + fmt.Fprintln(out) + fmt.Fprintln(out, "Note: review skills ran but findings were not persisted.") + fmt.Fprintf(out, " Reason: %s\n", reason) + fmt.Fprintln(out, " `entire review --findings` and `entire review --fix` will not see this run.") + fmt.Fprintln(out, " Re-run with `ENTIRE_LOG_LEVEL=debug` for diagnostic detail.") +} + func composeSingleAgentSinks(in singleAgentSinkInputs) []reviewtypes.Sink { if !in.isTTY || !in.canPrompt { fmt.Fprintf(in.out, "Running review with %s...\n", in.agentName) diff --git a/cmd/entire/cli/review/manifest_test.go b/cmd/entire/cli/review/manifest_test.go index ace0a17945..146bff19d7 100644 --- a/cmd/entire/cli/review/manifest_test.go +++ b/cmd/entire/cli/review/manifest_test.go @@ -379,3 +379,49 @@ func TestBuildLocalReviewManifestFromSummary_GroupsAgentSessionsAndAggregate(t * t.Fatalf("AggregateOutput = %q", manifest.AggregateOutput) } } + +func TestWarnManifestNotWritten_PrintsReasonAndDiagnosticHints(t *testing.T) { + var b strings.Builder + + warnManifestNotWritten(&b, "test reason text") + + got := b.String() + for _, want := range []string{ + "Note: review skills ran but findings were not persisted.", + "Reason: test reason text", + "`entire review --findings` and `entire review --fix` will not see this run.", + "`ENTIRE_LOG_LEVEL=debug`", + } { + if !strings.Contains(got, want) { + t.Fatalf("warning missing %q:\n%s", want, got) + } + } +} + +func TestWritePostReviewManifest_WarnsWhenNoMatchingSessions(t *testing.T) { + repoRoot := t.TempDir() + testutil.InitRepo(t, repoRoot) + t.Chdir(repoRoot) + + var out strings.Builder + summary := reviewtypes.RunSummary{ + StartedAt: time.Now(), + AgentRuns: []reviewtypes.AgentRun{ + {Name: "claude-code", Status: reviewtypes.AgentStatusSucceeded}, + }, + } + + // SHA is irrelevant: matcher never runs since no session states exist. + writePostReviewManifest(context.Background(), &out, repoRoot, "abc123", summary, "") + + got := out.String() + if !strings.Contains(got, "Note: review skills ran but findings were not persisted.") { + t.Fatalf("expected warning to fire when no sessions match; got:\n%s", got) + } + if !strings.Contains(got, "env-var handshake did not reach the hook") { + t.Fatalf("expected handshake-failure reason; got:\n%s", got) + } + if strings.Contains(got, "Review complete.") { + t.Fatalf("happy-path footer must not print when manifest is empty; got:\n%s", got) + } +}