feat(review): warn user when review manifest is not persisted

[peyton-alt]

https://entire.io/gh/entireio/cli/trails/340

Summary

• Silent-failure observability fix for entire review: when manifest persistence skips (env-handshake didn't reach the hook, state-store load fails, or disk write fails), the user now sees a clear note explaining what happened and how to diagnose.
• Adds a debug log on the previously-silent EnvSession-not-set return in adoptReviewEnv, completing observability across all five adoption decisions.
• Happy path is unchanged: successful reviews print identical output.

Background

After running entire review, three downstream features (entire review --findings, entire review --fix, and the end-of-run "To apply all review findings:" footer) all read from the local manifest at .git/entire-review/manifests/. If the spawned agent's lifecycle hook does not adopt the ENTIRE_REVIEW_* env vars set by entire review — for any reason — the session is never tagged as KindAgentReview, the manifest matcher finds zero sources, and writePostReviewManifest returns silently. All three features then quietly no-op with no error, no warning, no missing-file message that a user could grep.

We hit this in practice today: a review ran end-to-end, agents produced findings, the synthesis verdict printed — and then entire review --findings claimed there was nothing to show, with no breadcrumb in the user-visible output explaining why. The actual root cause for that specific reproduction is still unidentified (running from a fresh terminal worked; running from the same shell a few hours earlier did not, with identical shell env). Without observability in the failure path, every future reproduction will be equally opaque.

What this PR does

cmd/entire/cli/lifecycle.go — Two-line debug log added to the EnvSession-not-set silent return. The other four branches in adoptReviewEnv already log at WARN (agent mismatch, SHA mismatch, malformed skills JSON) or DEBUG (success). This was the only branch with no breadcrumb, so a "review env didn't take" failure produced zero log lines diagnosing it.

cmd/entire/cli/review/cmd.go — Each of the three silent early returns in writePostReviewManifest now also calls a new helper warnManifestNotWritten which prints a user-visible four-line note to the same out writer the success-path footer uses. The helper is centralized so the wording is consistent and editable in one place.

What this PR does NOT do

• Does not attempt to fix the root-cause env-propagation problem. The conditions under which the handshake breaks are not yet reliably reproducible. This PR makes the next reproduction debuggable in 30 seconds (one log grep) instead of an hour (reverse-engineering process state from logs).
• Does not change behavior on the happy path. Working reviews print the same output they do today.
• Does not touch the matcher logic, spawn path, or contract between agents and hooks.

User-visible change

Today, when manifest persistence skips:

2 agent(s) done — 2 succeeded, 0 failed, 0 cancelled
Generating summary...
[synthesis output]

[end of output — no error, no warning, no signal that --findings won't see this run]

After this PR, same scenario:

2 agent(s) done — 2 succeeded, 0 failed, 0 cancelled
Generating summary...
[synthesis output]

Note: review skills ran successfully but findings were not persisted.
  Reason: review session was not tagged as a review (env-var handshake did not reach the hook)
  `entire review --findings` and `entire review --fix` will not see this run.
  Re-run with `ENTIRE_LOG_LEVEL=debug` for diagnostic detail.

Test plan

• mise run fmt clean
• mise run lint — 0 issues
• go test ./cmd/entire/cli/review/ passes (incl. two new tests: TestWarnManifestNotWritten_PrintsReasonAndDiagnosticHints, TestWritePostReviewManifest_WarnsWhenNoMatchingSessions)
• go test ./cmd/entire/cli/ passes
• mise run test:integration passes (incl. existing TestReview_EnvVarAdoptionCondensesReviewMetadataOnNextCommit — confirms happy-path env adoption still works)
• Manual smoke test: real entire review --agent codex from a fresh terminal on this branch successfully writes a manifest and prints the existing "Review complete." footer with --fix <id> commands. Warning correctly does not fire on success.

Decisions worth flagging for review

• Debug-level (not WARN) for the new lifecycle log. This branch fires on every normal coding turn (any session that isn't a review), so logging at WARN/INFO would flood the operator's log with one line per user prompt. DEBUG keeps it silent unless someone explicitly opts in via ENTIRE_LOG_LEVEL=debug.
• No log-capture test for the new lifecycle log line. The logging package's package-level singleton makes log assertions heavy. The existing behavioral test (TestAdoptReviewEnv_NormalSession) covers the path; if the log line is ever removed, behavior is unchanged but observability degrades silently. Acceptable risk for v1; revisit if a future reproduction shows this matters.
• Wording of the failure note. The "env-var handshake did not reach the hook" phrasing is precise but jargony. Open to wording suggestions from review.

🤖 Generated with Claude Code

---

Note

Low Risk
Low risk: changes are limited to logging/user messaging on failure paths and new tests, with the successful review flow unchanged.

Overview
Prevents silent failures in entire review when local manifest persistence is skipped: writePostReviewManifest now prints a user-visible note (via new warnManifestNotWritten) when session state can’t be loaded, no matching review sessions are found, or manifest disk writes fail.

Adds a missing Debug log in adoptReviewEnv when ENTIRE_REVIEW_SESSION is not set, improving diagnostics for env-var adoption issues, and includes new tests covering the warning output and the “no matching sessions” warning path.

^{Reviewed by Cursor Bugbot for commit 4189e7c. Configure here.}

[Copilot]

Pull request overview

Improves observability for entire review when post-run manifest persistence is skipped, ensuring users get a clear, actionable warning and operators have an extra debug breadcrumb for env adoption.

Changes:

• Print a user-visible note when the review manifest isn’t written (state load failure, no matching review sessions, or disk write failure).
• Add a DEBUG log line for the previously silent ENTIRE_REVIEW_SESSION-not-set branch in adoptReviewEnv.
• Add unit tests covering the new warning helper and the “no matching sessions” warning path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
cmd/entire/cli/review/cmd.go	Adds warnManifestNotWritten and calls it on the manifest non-persistence early returns.
cmd/entire/cli/lifecycle.go	Adds a DEBUG log breadcrumb when review env adoption is skipped due to ENTIRE_REVIEW_SESSION not being "1".
cmd/entire/cli/review/manifest_test.go	Adds tests asserting the warning text and that it fires when no sessions match.

Comments suppressed due to low confidence (1)

cmd/entire/cli/lifecycle.go:1045

• This branch triggers when ENTIRE_REVIEW_SESSION is anything other than "1" (including "0"); the log message says "not set", which is inaccurate in that case. Consider rewording to something like "not enabled" / "not '1'", or include the observed value in a field for clarity.

		}
		if transErr := strategy.TransitionAndLog(ctx, state, session.EventSessionStop, session.TransitionContext{}, session.NoOpActionHandler{}); transErr != nil {
			logging.Warn(logging.WithComponent(ctx, "lifecycle"), "session stop transition failed",
				slog.String("error", transErr.Error()))