fix(review): address PR #1844 review findings

Review findings from five reviewers (four specialized agents + Copilot/
Bugbot on the PR), converging on two must-fix items and a set of
observability and comment-accuracy fixes:

- CondenseAndMarkFullyCondensed now re-checks Phase == ENDED under the
  state lock (flagged by three reviewers + Bugbot): the detached child
  can lose a race with a same-ID resume, and condensing a revived
  session would wipe its in-flight attribution and leave a sticky
  FullyCondensed=true that makes PostCommit skip the session's next end.
- handleLifecycleSessionEnd runs teardown on context.WithoutCancel
  (Copilot): the graceful half of hook cancellation is a SIGTERM, which
  cancels the root context — the ENDED mark must survive it.
- entire-dev runs the cached binary without exec and falls back to the
  PATH binary on launch failure (126/127): a concurrent rebuild rewrites
  the output in place, and a failed exec previously aborted the shell
  without reaching any fallback, breaking the never-block contract. Also:
  per-cause fallback messages, build stderr captured to
  .entire/tmp/entire-dev-build.log instead of discarded, and
  go build -C so invocation from outside the repo resolves the right
  module (latent pre-existing bug).
- SpawnDetached returns the start error and the SessionEnd hook logs the
  handoff and any spawn failure: the parent is the last process with
  working logging, and a silently failed spawn previously left zero
  evidence that a condense was ever requested.
- Comment corrections: the exited-session sweep cannot retry an ENDED
  session (it is ACTIVE-only — doctor is the real second retry surface);
  endSessionNow's stale lockstep framing; the ~1.5s Claude Code budget
  now stated once with provenance and referenced elsewhere; the spawn
  seam's rationale; the cached-binary reuse claim corrected to what go
  actually does (rewrites in place, skips only the relink).
- Tests: reactivated-session guard subtest (fails without the fix),
  rootErr sync-fallback test, seam records the worktree root, launcher
  launch-contract test (exit-status passthrough), __condense_session in
  the policy-warning table test, nil-guard in the polling test's timeout
  path.
- CLAUDE.md hidden-commands list updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 01KY8GX54M99KSD53H232HEKB4

Author: peyton-alt

CLAUDE.md

@@ -132,8 +132,9 @@ Deprecated top-level commands (functional, print a cobra deprecation message):
 deprecation as `checkpoint rewind`).
 
 Hidden infrastructure commands: `hooks`, `trail`,
-`curl-bash-post-install`, `__send_analytics`, `mcp` (MCP stdio server for
-MCP-host agents).
+`curl-bash-post-install`, `__send_analytics`, `__refresh_trail_enablement`,
+`__condense_session` (detached session-end condense child), `mcp` (MCP stdio
+server for MCP-host agents).
 
 The `hideAsAlias(cmd, canonical)` helper in `cmd/entire/cli/aliascmd.go`
 marks a command Hidden and sets cobra's `Deprecated` field so the hint

cmd/entire/cli/checkpoint_policy_warning_test.go

@@ -50,9 +50,13 @@ func TestShouldCheckCheckpointPolicyWarning(t *testing.T) {
 	refreshTrailEnablement := &cobra.Command{Use: "__refresh_trail_enablement", Hidden: true}
 	root.AddCommand(refreshTrailEnablement)
 
+	condenseSession := &cobra.Command{Use: "__condense_session", Hidden: true}
+	root.AddCommand(condenseSession)
+
 	require.True(t, ShouldCheckCheckpointPolicyWarning(visible))
 	require.True(t, ShouldCheckCheckpointPolicyWarning(hiddenAlias))
 	require.False(t, ShouldCheckCheckpointPolicyWarning(gitHook))
 	require.False(t, ShouldCheckCheckpointPolicyWarning(sendAnalytics))
 	require.False(t, ShouldCheckCheckpointPolicyWarning(refreshTrailEnablement))
+	require.False(t, ShouldCheckCheckpointPolicyWarning(condenseSession))
 }

cmd/entire/cli/execx/spawn_detached.go

@@ -2,6 +2,7 @@ package execx
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"os"
 	"os/exec"
@@ -13,19 +14,22 @@ import (
 // CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS on Windows, via detachFromTTY).
 // The child runs in dir (os.TempDir() when empty, so the child never holds the
 // parent's working directory), inherits the parent's environment, and has its
-// stdout/stderr discarded. Best-effort: every error is swallowed — callers
-// treat the spawn as advisory background work.
+// stdout/stderr discarded. Best-effort: the spawn is advisory background work
+// and the returned error exists only so callers can log that the child never
+// started (e.g. the running executable was deleted out from under us) — the
+// parent is the last process with working logging, so discarding it leaves no
+// evidence at all.
 //
 // In-process `go test` runs are a no-op: the current executable is the test
 // binary, and re-execing it would fork the whole suite. Tests exercise the
 // call sites through their spawn seams instead.
-func SpawnDetached(dir string, args ...string) {
+func SpawnDetached(dir string, args ...string) error {
 	if testing.Testing() {
-		return
+		return nil
 	}
 	executable, err := os.Executable()
 	if err != nil {
-		return
+		return fmt.Errorf("resolve current executable: %w", err)
 	}
 
 	// context.Background(): the child must outlive the parent, so it is never
@@ -41,9 +45,10 @@ func SpawnDetached(dir string, args ...string) {
 	cmd.Stderr = io.Discard
 
 	if err := cmd.Start(); err != nil {
-		return
+		return fmt.Errorf("start detached child (%v): %w", args, err)
 	}
 	// Release the process so it can run independently of the parent.
 	//nolint:errcheck // best effort — the child continues regardless
 	_ = cmd.Process.Release()
+	return nil
 }

cmd/entire/cli/integration_test/hooks.go

@@ -229,8 +229,9 @@ func (r *HookRunner) runHookInRepoDir(hookName string, inputJSON []byte) error {
 }
 
 // runHookInRepoDirWithExtraEnv is like runHookInRepoDir but appends additional
-// env vars to the subprocess environment. Used by review-env adoption tests that
-// need ENTIRE_REVIEW_* vars present in the hook child process.
+// env vars to the subprocess environment, for tests that need extra or
+// overriding env vars in the hook child process (exec.Cmd keeps the last
+// value for a duplicated key, so appending overrides).
 func (r *HookRunner) runHookInRepoDirWithExtraEnv(hookName string, inputJSON []byte, extraEnv []string) error {
 	// Run using the shared test binary
 	// Command structure: entire hooks claude-code <hook-name>

cmd/entire/cli/integration_test/session_end_detached_test.go

@@ -11,10 +11,10 @@ import (
 
 // TestSessionEnd_DetachedCondense exercises the production SessionEnd flow:
 // the hook marks the session ENDED inline (fast enough for agents' short
-// SessionEnd budgets — Claude Code cancels the hook after ~1.5s) and hands
-// the eager condense to a detached __condense_session child that survives
-// the hook process being killed. The ENDED mark must be observable as soon
-// as the hook returns; the condense lands asynchronously.
+// SessionEnd budgets — see spawnDetachedSessionEndCondense) and hands the
+// eager condense to a detached __condense_session child that survives the
+// hook process being killed. The ENDED mark must be observable as soon as
+// the hook returns; the condense lands asynchronously.
 func TestSessionEnd_DetachedCondense(t *testing.T) {
 	t.Parallel()
 
@@ -70,6 +70,9 @@ func TestSessionEnd_DetachedCondense(t *testing.T) {
 			break
 		}
 		if time.Now().After(deadline) {
+			if state == nil {
+				t.Fatal("detached condense did not complete within 15s and session state vanished")
+			}
 			t.Fatalf("detached condense did not mark session FullyCondensed within 15s (phase=%s)", state.Phase)
 		}
 		time.Sleep(100 * time.Millisecond)

cmd/entire/cli/lifecycle.go

@@ -1020,43 +1020,62 @@ func handleLifecycleSessionEnd(ctx context.Context, ag agent.Agent, event *agent
 	// the transcript to extract file changes. Cleanup is handled by
 	// `entire clean` or when the session state is fully removed.
 
-	// Agents give SessionEnd hooks a short budget and never wait for them on
-	// exit (Claude Code cancels after ~1.5s), so only the fast ENDED mark runs
-	// inline; the eager condense — transcript reads plus git tree building —
-	// is handed to a detached child that survives the hook being killed.
-	// ENTIRE_SESSION_END_SYNC keeps the inline sequence (integration tests
-	// use it for determinism), as does a worktree root that can't be resolved
-	// (nowhere to run the child; dropping the condense would regress #591).
-	worktreeRoot, rootErr := paths.WorktreeRoot(ctx)
+	// Agents give SessionEnd hooks a short budget and do not wait for them on
+	// exit (see spawnDetachedSessionEndCondense for the observed figure), so
+	// only the fast ENDED mark runs inline; the eager condense — transcript
+	// reads plus git tree building — is handed to a detached child that
+	// survives the hook being killed. ENTIRE_SESSION_END_SYNC keeps the
+	// inline sequence (integration tests use it for determinism), as does a
+	// worktree root that can't be resolved (nowhere to run the child;
+	// dropping the condense would regress #591).
+	//
+	// The graceful half of that cancellation is a SIGTERM, which cancels the
+	// root context (see main.go) — the very signal this teardown must
+	// survive. Run the short, bounded finalization on a cancellation-immune
+	// context so the ENDED mark can't be aborted mid-write; a force-quit
+	// (second signal → SIGKILL) still terminates the process outright.
+	endCtx := context.WithoutCancel(ctx)
+	worktreeRoot, rootErr := paths.WorktreeRoot(endCtx)
 	if os.Getenv(envSessionEndSyncCondense) != "" || rootErr != nil {
-		if _, err := endSessionNow(ctx, event, event.SessionID, nil); err != nil {
+		if _, err := endSessionNow(endCtx, event, event.SessionID, nil); err != nil {
 			logging.Warn(logCtx, "failed to mark session ended",
 				slog.String("error", err.Error()))
 		}
 		return nil
 	}
 
-	ended, err := markSessionEnded(ctx, event, event.SessionID, nil)
+	ended, err := markSessionEnded(endCtx, event, event.SessionID, nil)
 	if err != nil {
 		logging.Warn(logCtx, "failed to mark session ended",
 			slog.String("error", err.Error()))
 		return nil
 	}
 	if ended {
-		sessionEndCondenseSpawn(worktreeRoot, event.SessionID)
+		logging.Info(logCtx, "handing session-end condense to detached child",
+			slog.String("session_id", event.SessionID))
+		if spawnErr := sessionEndCondenseSpawn(worktreeRoot, event.SessionID); spawnErr != nil {
+			// Fail-open, but never silent: the session is ENDED with its
+			// condense outstanding, and PostCommit (next commit) or `entire
+			// doctor` are the remaining retry surfaces.
+			logging.Warn(logCtx, "failed to spawn detached session-end condense",
+				slog.String("session_id", event.SessionID),
+				slog.String("error", spawnErr.Error()))
+		}
 	}
 
 	return nil
 }
 
-// endSessionNow runs the canonical "this session is over" sequence: it marks the
-// session ended (firing the SessionStop transition → PhaseEnded + EndedAt) and
-// eagerly condenses its pending work so PostCommit need not. This prevents
+// endSessionNow runs the synchronous "this session is over" sequence: it marks
+// the session ended (firing the SessionStop transition → PhaseEnded + EndedAt)
+// and eagerly condenses its pending work so PostCommit need not. This prevents
 // zombie ENDED sessions from accumulating and causing O(N) overhead on every
-// future commit (GitHub issue #591). It is used by the exited-session sweep
-// (finalizeExitedSessions) and by the SessionEnd hook's synchronous fallback
-// (handleLifecycleSessionEnd, which normally detaches the condense instead),
-// so the two stay in lockstep.
+// future commit (GitHub issue #591). Callers: the exited-session sweep
+// (finalizeExitedSessions — status/doctor have no hook budget, so condensing
+// inline is fine there) and the SessionEnd hook's rarely-taken sync fallback.
+// The normal hook path deliberately does NOT use this sequence: it calls
+// markSessionEnded inline and hands the condense to a detached child — see
+// handleLifecycleSessionEnd.
 //
 // The condense is fail-open (PostCommit retries on the next commit); an error
 // marking the session ended is returned so callers can react, and skips the

cmd/entire/cli/session_end_condense.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"fmt"
 	"log/slog"
 
 	"github.com/entireio/cli/cmd/entire/cli/execx"
@@ -11,26 +12,36 @@ import (
 )
 
 // envSessionEndSyncCondense forces the SessionEnd hook to run the eager
-// condense inline instead of handing it to a detached child. Integration
-// tests set it for determinism; it also serves as an escape hatch when the
-// detached child is undesirable (e.g. debugging).
+// condense inline instead of handing it to a detached child. Any non-empty
+// value enables it (including "0"/"false", matching the ACCESSIBLE
+// convention). Integration tests set it for determinism; it also serves as
+// an escape hatch when the detached child is undesirable (e.g. debugging).
 const envSessionEndSyncCondense = "ENTIRE_SESSION_END_SYNC"
 
 // sessionEndCondenseSpawn is the process-spawn seam used by
-// handleLifecycleSessionEnd. Swapped in tests so they can assert the hook
-// requests a detached condense without forking a real subprocess (a real
-// `go test` binary doesn't understand `__condense_session` as an argument).
-// Production code always uses spawnDetachedSessionEndCondense.
+// handleLifecycleSessionEnd. SpawnDetached is already a no-op under `go test`
+// (see execx.SpawnDetached); this seam exists so unit tests can record that a
+// detached condense was requested, and with which arguments. Production code
+// always uses spawnDetachedSessionEndCondense.
 var sessionEndCondenseSpawn = spawnDetachedSessionEndCondense
 
 // spawnDetachedSessionEndCondense starts `entire __condense_session <id>` as
 // a detached child so the eager condense survives the SessionEnd hook being
-// cancelled (agents give these hooks a short budget — Claude Code cancels
-// after ~1.5s — and never wait for them on exit). The child runs from the
-// worktree root because the strategy resolves the repo and session store
-// from its working directory.
-func spawnDetachedSessionEndCondense(worktreeRoot, sessionID string) {
-	execx.SpawnDetached(worktreeRoot, "__condense_session", sessionID)
+// cancelled. Agents give SessionEnd hooks a short budget and do not wait for
+// them on exit — empirically ~1.5s for Claude Code (v2.1.x, observed 2026-07;
+// undocumented, so treat the figure as approximate). Other comments reference
+// this one rather than restating the number. The child runs from the worktree
+// root because the strategy resolves the repo and session store from its
+// working directory.
+//
+// The returned error means the child never started (e.g. the local-dev cache
+// binary was deleted between exec and spawn); callers stay fail-open but must
+// log it — the parent is the last process with working logging.
+func spawnDetachedSessionEndCondense(worktreeRoot, sessionID string) error {
+	if err := execx.SpawnDetached(worktreeRoot, "__condense_session", sessionID); err != nil {
+		return fmt.Errorf("spawn detached session-end condense: %w", err)
+	}
+	return nil
 }
 
 // newCondenseSessionCmd creates the hidden command that runs the eager
@@ -49,7 +60,8 @@ func newCondenseSessionCmd() *cobra.Command {
 			// .entire/logs/entire.log rather than vanishing. Guard on
 			// WorktreeRoot first — matching __refresh_trail_enablement — so a
 			// child whose worktree was removed between spawn and exec doesn't
-			// create a stray .entire/logs/ in an arbitrary directory.
+			// create a stray .entire/logs/ in an arbitrary directory
+			// (logging.Init falls back to cwd when WorktreeRoot fails).
 			if _, err := paths.WorktreeRoot(ctx); err == nil {
 				logging.SetLogLevelGetter(GetLogLevel)
 				if err := logging.Init(ctx, ""); err == nil {
@@ -59,8 +71,11 @@ func newCondenseSessionCmd() *cobra.Command {
 			sessionID := args[0]
 			if err := GetStrategy(ctx).CondenseAndMarkFullyCondensed(ctx, sessionID); err != nil {
 				// Fail-open, like the inline condense it replaces: PostCommit
-				// retries on the next commit and the exited-session sweep
-				// retries from status/doctor.
+				// retries on the next commit, and `entire doctor` detects and
+				// offers to condense stuck ended sessions. (The exited-session
+				// sweep does NOT cover this — it only finalizes ACTIVE
+				// sessions whose owner died, and this session is already
+				// ENDED.)
 				logging.Warn(logging.WithComponent(ctx, "lifecycle"),
 					"detached session-end condense failed",
 					slog.String("session_id", sessionID),

cmd/entire/cli/session_end_condense_test.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"context"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -13,15 +14,22 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// spawnCall records one request to the detached-condense spawn seam.
+type spawnCall struct {
+	worktreeRoot string
+	sessionID    string
+}
+
 // swapSessionEndCondenseSpawn replaces the detached-condense spawn seam with a
 // recorder and restores it on cleanup. Tests using it must not run in parallel
 // (package-level seam).
-func swapSessionEndCondenseSpawn(t *testing.T) *[]string {
+func swapSessionEndCondenseSpawn(t *testing.T) *[]spawnCall {
 	t.Helper()
-	var spawned []string
+	var spawned []spawnCall
 	orig := sessionEndCondenseSpawn
-	sessionEndCondenseSpawn = func(_, sessionID string) {
-		spawned = append(spawned, sessionID)
+	sessionEndCondenseSpawn = func(worktreeRoot, sessionID string) error {
+		spawned = append(spawned, spawnCall{worktreeRoot: worktreeRoot, sessionID: sessionID})
+		return nil
 	}
 	t.Cleanup(func() { sessionEndCondenseSpawn = orig })
 	return &spawned
@@ -60,8 +68,32 @@ func TestHandleLifecycleSessionEnd_SpawnsDetachedCondense(t *testing.T) {
 		"session must be marked ENDED synchronously")
 	assert.False(t, loaded.FullyCondensed,
 		"condense must not run inline on the hook path")
-	assert.Equal(t, []string{sessionID}, *spawned,
+	require.Len(t, *spawned, 1,
 		"exactly one detached condense must be requested for the ended session")
+	assert.Equal(t, sessionID, (*spawned)[0].sessionID)
+	// The child resolves the repo and session store from its working
+	// directory, so spawning anywhere but the worktree root would make the
+	// condense a silent no-op in the wrong place.
+	wantRoot, err := filepath.EvalSymlinks(dir)
+	require.NoError(t, err)
+	gotRoot, err := filepath.EvalSymlinks((*spawned)[0].worktreeRoot)
+	require.NoError(t, err)
+	assert.Equal(t, wantRoot, gotRoot, "child must be spawned from the worktree root")
+}
+
+// TestHandleLifecycleSessionEnd_NoRepoFallsBackToSync verifies that when the
+// worktree root can't be resolved there is nowhere to run the child, so the
+// handler takes the synchronous endSessionNow path instead of spawning —
+// dropping the condense silently would regress #591.
+func TestHandleLifecycleSessionEnd_NoRepoFallsBackToSync(t *testing.T) {
+	t.Chdir(t.TempDir()) // plain directory, not a git repo
+
+	spawned := swapSessionEndCondenseSpawn(t)
+
+	event := &agent.Event{Type: agent.SessionEnd, SessionID: "some-session"}
+	require.NoError(t, handleLifecycleSessionEnd(context.Background(), newMockAgent(), event))
+
+	assert.Empty(t, *spawned, "no detached condense without a resolvable worktree root")
 }
 
 // TestHandleLifecycleSessionEnd_SyncEnvCondensesInline verifies the