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
5 changes: 4 additions & 1 deletion cmd/entire/cli/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 16 additions & 0 deletions cmd/entire/cli/review/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
46 changes: 46 additions & 0 deletions cmd/entire/cli/review/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Loading