From d13528a136daf027913009d3cbe248be8080319d Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 16:59:14 +0100 Subject: [PATCH 01/32] Add wingman automated code review feature Adds `entire wingman` command group (enable/disable/status) and a background review loop that analyzes agent code changes via Claude, writes suggestions to .entire/REVIEW.md, and auto-applies them when the session is idle. Includes lock file to prevent concurrent spawns and base commit capture for deterministic diffs. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 71d4670c34ac --- cmd/entire/cli/hooks_claudecode_handlers.go | 24 ++ cmd/entire/cli/root.go | 1 + cmd/entire/cli/settings/settings.go | 26 ++ cmd/entire/cli/wingman.go | 291 ++++++++++++++++++++ cmd/entire/cli/wingman_prompt.go | 81 ++++++ cmd/entire/cli/wingman_review.go | 257 +++++++++++++++++ cmd/entire/cli/wingman_spawn_other.go | 11 + cmd/entire/cli/wingman_spawn_unix.go | 48 ++++ cmd/entire/cli/wingman_test.go | 284 +++++++++++++++++++ 9 files changed, 1023 insertions(+) create mode 100644 cmd/entire/cli/wingman.go create mode 100644 cmd/entire/cli/wingman_prompt.go create mode 100644 cmd/entire/cli/wingman_review.go create mode 100644 cmd/entire/cli/wingman_spawn_other.go create mode 100644 cmd/entire/cli/wingman_spawn_unix.go create mode 100644 cmd/entire/cli/wingman_test.go diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 8189eb142..b1ee0ef9a 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -17,6 +17,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/strategy" ) @@ -75,6 +76,16 @@ func captureInitialState() error { return err } + // Notify user if a wingman review is pending + if settings.IsWingmanEnabled() { + repoRoot, rootErr := paths.RepoRoot() + if rootErr == nil { + if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { + fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md — review and apply suggestions\n") + } + } + } + // If strategy implements SessionInitializer, call it to initialize session state strat := GetStrategy() if initializer, ok := strat.(strategy.SessionInitializer); ok { @@ -377,6 +388,19 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba // For ACTIVE_COMMITTED → IDLE, HandleTurnEnd dispatches ActionCondense. transitionSessionTurnEnd(sessionID) + // Trigger wingman review if enabled and files changed + if totalChanges > 0 && settings.IsWingmanEnabled() { + triggerWingmanReview(WingmanPayload{ + SessionID: sessionID, + RepoRoot: repoRoot, + ModifiedFiles: relModifiedFiles, + NewFiles: relNewFiles, + DeletedFiles: relDeletedFiles, + Prompts: allPrompts, + CommitMessage: commitMessage, + }) + } + // Clean up pre-prompt state (CLI responsibility) if err := CleanupPrePromptState(sessionID); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to cleanup pre-prompt state: %v\n", err) diff --git a/cmd/entire/cli/root.go b/cmd/entire/cli/root.go index 5fedf6ad4..d84008279 100644 --- a/cmd/entire/cli/root.go +++ b/cmd/entire/cli/root.go @@ -85,6 +85,7 @@ func NewRootCmd() *cobra.Command { cmd.AddCommand(newDoctorCmd()) cmd.AddCommand(newSendAnalyticsCmd()) cmd.AddCommand(newCurlBashPostInstallCmd()) + cmd.AddCommand(newWingmanCmd()) // Replace default help command with custom one that supports -t flag cmd.SetHelpCommand(NewHelpCmd(cmd)) diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index 381c9993a..3aa1f4a69 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -239,6 +239,32 @@ func (s *EntireSettings) IsSummarizeEnabled() bool { return enabled } +// IsWingmanEnabled checks if wingman auto-review is enabled in settings. +// Returns false by default if settings cannot be loaded or the key is missing. +func IsWingmanEnabled() bool { + s, err := Load() + if err != nil { + return false + } + return s.IsWingmanEnabled() +} + +// IsWingmanEnabled checks if wingman auto-review is enabled in this settings instance. +func (s *EntireSettings) IsWingmanEnabled() bool { + if s.StrategyOptions == nil { + return false + } + wingmanOpts, ok := s.StrategyOptions["wingman"].(map[string]any) + if !ok { + return false + } + enabled, ok := wingmanOpts["enabled"].(bool) + if !ok { + return false + } + return enabled +} + // IsPushSessionsDisabled checks if push_sessions is disabled in settings. // Returns true if push_sessions is explicitly set to false. func (s *EntireSettings) IsPushSessionsDisabled() bool { diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go new file mode 100644 index 000000000..a509990f0 --- /dev/null +++ b/cmd/entire/cli/wingman.go @@ -0,0 +1,291 @@ +package cli + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/settings" + "github.com/spf13/cobra" +) + +// WingmanPayload is the data passed to the detached review subprocess. +type WingmanPayload struct { + SessionID string `json:"session_id"` + RepoRoot string `json:"repo_root"` + BaseCommit string `json:"base_commit"` + ModifiedFiles []string `json:"modified_files"` + NewFiles []string `json:"new_files"` + DeletedFiles []string `json:"deleted_files"` + Prompts []string `json:"prompts"` + CommitMessage string `json:"commit_message"` +} + +// WingmanState tracks deduplication and review state. +type WingmanState struct { + SessionID string `json:"session_id"` + FilesHash string `json:"files_hash"` + ReviewedAt time.Time `json:"reviewed_at"` + ReviewApplied bool `json:"review_applied"` +} + +const ( + wingmanStateFile = ".entire/wingman-state.json" + wingmanReviewFile = ".entire/REVIEW.md" + wingmanLockFile = ".entire/wingman.lock" +) + +func newWingmanCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "wingman", + Short: "Automated code review for agent sessions", + Long: `Wingman provides automated code review after agent turns. + +When enabled, wingman automatically reviews code changes made by agents, +writes suggestions to .entire/REVIEW.md, and optionally triggers the agent +to apply them.`, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + } + + cmd.AddCommand(newWingmanEnableCmd()) + cmd.AddCommand(newWingmanDisableCmd()) + cmd.AddCommand(newWingmanStatusCmd()) + cmd.AddCommand(newWingmanReviewCmd()) + + return cmd +} + +func newWingmanEnableCmd() *cobra.Command { + return &cobra.Command{ + Use: "enable", + Short: "Enable wingman auto-review", + RunE: func(cmd *cobra.Command, _ []string) error { + if _, err := paths.RepoRoot(); err != nil { + fmt.Fprintln(cmd.ErrOrStderr(), "Not a git repository. Please run from within a git repository.") + return NewSilentError(errors.New("not a git repository")) + } + + s, err := settings.Load() + if err != nil { + return fmt.Errorf("failed to load settings: %w", err) + } + + if !s.Enabled { + fmt.Fprintln(cmd.ErrOrStderr(), "Entire is not enabled. Run 'entire enable' first.") + return NewSilentError(errors.New("entire not enabled")) + } + + if s.StrategyOptions == nil { + s.StrategyOptions = make(map[string]any) + } + s.StrategyOptions["wingman"] = map[string]any{"enabled": true} + + if err := settings.Save(s); err != nil { + return fmt.Errorf("failed to save settings: %w", err) + } + + fmt.Fprintln(cmd.OutOrStdout(), "Wingman enabled. Code changes will be automatically reviewed after agent turns.") + return nil + }, + } +} + +func newWingmanDisableCmd() *cobra.Command { + return &cobra.Command{ + Use: "disable", + Short: "Disable wingman auto-review", + RunE: func(cmd *cobra.Command, _ []string) error { + s, err := settings.Load() + if err != nil { + return fmt.Errorf("failed to load settings: %w", err) + } + + if s.StrategyOptions == nil { + s.StrategyOptions = make(map[string]any) + } + s.StrategyOptions["wingman"] = map[string]any{"enabled": false} + + if err := settings.Save(s); err != nil { + return fmt.Errorf("failed to save settings: %w", err) + } + + // Clean up pending review file if it exists + reviewPath, err := paths.AbsPath(wingmanReviewFile) + if err == nil { + _ = os.Remove(reviewPath) + } + + fmt.Fprintln(cmd.OutOrStdout(), "Wingman disabled.") + return nil + }, + } +} + +func newWingmanStatusCmd() *cobra.Command { + return &cobra.Command{ + Use: "status", + Short: "Show wingman status", + RunE: func(cmd *cobra.Command, _ []string) error { + s, err := settings.Load() + if err != nil { + return fmt.Errorf("failed to load settings: %w", err) + } + + if s.IsWingmanEnabled() { + fmt.Fprintln(cmd.OutOrStdout(), "Wingman: enabled") + } else { + fmt.Fprintln(cmd.OutOrStdout(), "Wingman: disabled") + } + + // Show last review info if available + state, err := loadWingmanState() + if err == nil && state != nil { + fmt.Fprintf(cmd.OutOrStdout(), "Last review: %s\n", state.ReviewedAt.Format(time.RFC3339)) + if state.ReviewApplied { + fmt.Fprintln(cmd.OutOrStdout(), "Status: applied") + } else { + fmt.Fprintln(cmd.OutOrStdout(), "Status: pending") + } + } + + // Check for pending REVIEW.md + reviewPath, err := paths.AbsPath(wingmanReviewFile) + if err == nil { + if _, statErr := os.Stat(reviewPath); statErr == nil { + fmt.Fprintln(cmd.OutOrStdout(), "Pending review: .entire/REVIEW.md") + } + } + + return nil + }, + } +} + +// triggerWingmanReview checks preconditions and spawns the detached review process. +func triggerWingmanReview(payload WingmanPayload) { + repoRoot := payload.RepoRoot + + // Check if a pending REVIEW.md already exists + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, err := os.Stat(reviewPath); err == nil { + fmt.Fprintf(os.Stderr, "[wingman] Pending review exists, skipping\n") + return + } + + // Lock file prevents concurrent review spawns. Multiple rapid stop hooks + // could race past the dedup check (which reads stale state) before any + // review completes and updates the state file. + lockPath := filepath.Join(repoRoot, wingmanLockFile) + if _, err := os.Stat(lockPath); err == nil { + fmt.Fprintf(os.Stderr, "[wingman] Review already in progress, skipping\n") + return + } + + // Dedup check: compute hash of sorted file paths + allFiles := make([]string, 0, len(payload.ModifiedFiles)+len(payload.NewFiles)+len(payload.DeletedFiles)) + allFiles = append(allFiles, payload.ModifiedFiles...) + allFiles = append(allFiles, payload.NewFiles...) + allFiles = append(allFiles, payload.DeletedFiles...) + filesHash := computeFilesHash(allFiles) + + state, _ := loadWingmanState() //nolint:errcheck // best-effort dedup + if state != nil && state.FilesHash == filesHash && state.SessionID == payload.SessionID { + fmt.Fprintf(os.Stderr, "[wingman] Already reviewed these changes, skipping\n") + return + } + + // Capture HEAD at trigger time so the detached review diffs against + // the correct commit even if HEAD moves during the initial delay. + payload.BaseCommit = resolveHEAD(repoRoot) + + // Write lock file synchronously before spawning to prevent races + //nolint:gosec // G306: lock file is not secrets + _ = os.WriteFile(lockPath, []byte(payload.SessionID), 0o644) //nolint:errcheck // best-effort lock + + // Marshal payload + payloadJSON, err := json.Marshal(payload) + if err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Failed to marshal payload: %v\n", err) + _ = os.Remove(lockPath) + return + } + + // Spawn detached review process + spawnDetachedWingmanReview(string(payloadJSON)) + fmt.Fprintf(os.Stderr, "[wingman] Review starting in background...\n") +} + +// resolveHEAD returns the current HEAD commit hash, or empty string on error. +func resolveHEAD(repoRoot string) string { + cmd := exec.CommandContext(context.Background(), "git", "rev-parse", "HEAD") + cmd.Dir = repoRoot + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// computeFilesHash returns a SHA256 hex digest of the sorted file paths. +func computeFilesHash(files []string) string { + sorted := make([]string, len(files)) + copy(sorted, files) + sort.Strings(sorted) + h := sha256.Sum256([]byte(strings.Join(sorted, "\n"))) + return hex.EncodeToString(h[:]) +} + +// loadWingmanState loads the wingman state from .entire/wingman-state.json. +func loadWingmanState() (*WingmanState, error) { + statePath, err := paths.AbsPath(wingmanStateFile) + if err != nil { + return nil, fmt.Errorf("resolving wingman state path: %w", err) + } + + data, err := os.ReadFile(statePath) //nolint:gosec // path is repo-relative + if err != nil { + return nil, fmt.Errorf("reading wingman state: %w", err) + } + + var state WingmanState + if err := json.Unmarshal(data, &state); err != nil { + return nil, fmt.Errorf("parsing wingman state: %w", err) + } + return &state, nil +} + +// saveWingmanState saves the wingman state to .entire/wingman-state.json. +func saveWingmanState(state *WingmanState) error { + statePath, err := paths.AbsPath(wingmanStateFile) + if err != nil { + return fmt.Errorf("resolving wingman state path: %w", err) + } + + dir := filepath.Dir(statePath) + if err := os.MkdirAll(dir, 0o750); err != nil { + return fmt.Errorf("creating wingman state directory: %w", err) + } + + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return fmt.Errorf("marshaling wingman state: %w", err) + } + + //nolint:gosec // G306: state file is config, not secrets + if err := os.WriteFile(statePath, data, 0o644); err != nil { + return fmt.Errorf("writing wingman state: %w", err) + } + return nil +} diff --git a/cmd/entire/cli/wingman_prompt.go b/cmd/entire/cli/wingman_prompt.go new file mode 100644 index 000000000..20040ed90 --- /dev/null +++ b/cmd/entire/cli/wingman_prompt.go @@ -0,0 +1,81 @@ +package cli + +import ( + "fmt" + "strings" +) + +// maxDiffSize is the maximum size of the diff included in the review prompt. +// Large diffs degrade review quality, so we truncate. +const maxDiffSize = 100 * 1024 // 100KB + +// reviewPromptTemplate is the prompt sent to Claude for code review. +// +// Security note: User content (diff, prompts) is wrapped in XML tags to provide +// clear boundary markers, similar to the summarization prompt pattern. +const reviewPromptTemplate = `You are a senior code reviewer. Review the following code changes and provide actionable feedback. + +## Context + +The developer's intent (from their prompts): + +%s + + +## Code Changes + +Files changed: %s + + +%s + + +## Instructions + +Review the code changes above. Focus on: +- Bugs or logic errors +- Security vulnerabilities +- Performance issues +- Missing error handling +- Code style and readability issues + +For each issue found, provide: +1. Severity: CRITICAL, WARNING, or SUGGESTION +2. File path and line number (from the diff) +3. Description of the issue +4. Suggested fix + +Format your response as Markdown with this structure: + +# Code Review + +## Summary +Brief overview of the changes and overall assessment. + +## Issues + +### [SEVERITY] Short description +**File:** ` + "`path/to/file.go:42`" + ` +**Description:** What the issue is and why it matters. +**Suggestion:** +` + "```" + ` +// suggested fix +` + "```" + ` + +If no issues are found, say so and optionally provide suggestions for improvement. +Do NOT include any preamble or explanation outside the Markdown structure above.` + +// buildReviewPrompt constructs the review prompt from the payload and diff. +func buildReviewPrompt(prompts []string, fileList string, diff string) string { + promptText := strings.Join(prompts, "\n\n---\n\n") + if promptText == "" { + promptText = "(no prompts captured)" + } + + // Truncate large diffs + if len(diff) > maxDiffSize { + diff = diff[:maxDiffSize] + "\n\n... (diff truncated at 100KB)" + } + + return fmt.Sprintf(reviewPromptTemplate, promptText, fileList, diff) +} diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go new file mode 100644 index 000000000..bcd630729 --- /dev/null +++ b/cmd/entire/cli/wingman_review.go @@ -0,0 +1,257 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/entireio/cli/cmd/entire/cli/strategy" + "github.com/spf13/cobra" +) + +const ( + // wingmanInitialDelay is how long to wait before starting the review, + // letting the agent turn fully settle. + wingmanInitialDelay = 10 * time.Second + + // wingmanApplyDelay is how long to wait after writing REVIEW.md + // before attempting to auto-apply. + wingmanApplyDelay = 30 * time.Second + + // wingmanReviewModel is the Claude model used for reviews. + wingmanReviewModel = "sonnet" +) + +// wingmanCLIResponse represents the JSON response from the Claude CLI --output-format json. +type wingmanCLIResponse struct { + Result string `json:"result"` +} + +func newWingmanReviewCmd() *cobra.Command { + return &cobra.Command{ + Use: "__review", + Hidden: true, + Args: cobra.ExactArgs(1), + RunE: func(_ *cobra.Command, args []string) error { + return runWingmanReview(args[0]) + }, + } +} + +func runWingmanReview(payloadJSON string) error { + var payload WingmanPayload + if err := json.Unmarshal([]byte(payloadJSON), &payload); err != nil { + return fmt.Errorf("failed to unmarshal payload: %w", err) + } + + repoRoot := payload.RepoRoot + if repoRoot == "" { + return errors.New("repo_root is required in payload") + } + + // Clean up lock file when review completes (regardless of success/failure) + lockPath := filepath.Join(repoRoot, wingmanLockFile) + defer os.Remove(lockPath) + + // Initial delay: let the agent turn fully settle + time.Sleep(wingmanInitialDelay) + + // Compute diff using the base commit captured at trigger time + diff, err := computeDiff(repoRoot, payload.BaseCommit) + if err != nil { + return fmt.Errorf("failed to compute diff: %w", err) + } + + if strings.TrimSpace(diff) == "" { + return nil // No changes to review + } + + // Build file list for the prompt + allFiles := make([]string, 0, len(payload.ModifiedFiles)+len(payload.NewFiles)+len(payload.DeletedFiles)) + for _, f := range payload.ModifiedFiles { + allFiles = append(allFiles, f+" (modified)") + } + for _, f := range payload.NewFiles { + allFiles = append(allFiles, f+" (new)") + } + for _, f := range payload.DeletedFiles { + allFiles = append(allFiles, f+" (deleted)") + } + fileList := strings.Join(allFiles, ", ") + + // Build review prompt + prompt := buildReviewPrompt(payload.Prompts, fileList, diff) + + // Call Claude CLI for review + reviewText, err := callClaudeForReview(prompt) + if err != nil { + return fmt.Errorf("failed to get review from Claude: %w", err) + } + + // Write REVIEW.md + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if err := os.MkdirAll(filepath.Dir(reviewPath), 0o750); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + //nolint:gosec // G306: review file is not secrets + if err := os.WriteFile(reviewPath, []byte(reviewText), 0o644); err != nil { + return fmt.Errorf("failed to write REVIEW.md: %w", err) + } + + // Update dedup state + allFilePaths := make([]string, 0, len(payload.ModifiedFiles)+len(payload.NewFiles)+len(payload.DeletedFiles)) + allFilePaths = append(allFilePaths, payload.ModifiedFiles...) + allFilePaths = append(allFilePaths, payload.NewFiles...) + allFilePaths = append(allFilePaths, payload.DeletedFiles...) + + // Save state from repo root context + if err := os.Chdir(repoRoot); err == nil { + _ = saveWingmanState(&WingmanState{ //nolint:errcheck // best-effort state save + SessionID: payload.SessionID, + FilesHash: computeFilesHash(allFilePaths), + ReviewedAt: time.Now(), + ReviewApplied: false, + }) + } + + // Auto-apply phase: wait then check if session is idle + time.Sleep(wingmanApplyDelay) + + if !isSessionIdle(payload.SessionID) { + return nil // User is active, leave REVIEW.md for notification + } + + // Trigger auto-apply via claude --continue + if err := triggerAutoApply(repoRoot); err != nil { + return fmt.Errorf("failed to trigger auto-apply: %w", err) + } + + return nil +} + +// computeDiff gets the code changes to review. baseCommit is the HEAD hash +// captured at trigger time so the diff is stable even if HEAD moves during +// the initial delay (e.g., auto-commit creates another commit, or user commits). +func computeDiff(repoRoot string, baseCommit string) (string, error) { + // Use the captured base commit when available for deterministic diffs + diffRef := "HEAD" + if baseCommit != "" { + diffRef = baseCommit + } + + // Try uncommitted changes against the base commit + + cmd := exec.CommandContext(context.Background(), "git", "diff", diffRef) + cmd.Dir = repoRoot + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + // If the ref doesn't exist (initial commit), try diff of staged/unstaged + cmd2 := exec.CommandContext(context.Background(), "git", "diff") + cmd2.Dir = repoRoot + var stdout2 bytes.Buffer + cmd2.Stdout = &stdout2 + if err2 := cmd2.Run(); err2 != nil { + return "", fmt.Errorf("git diff failed: %w (stderr: %s)", err, stderr.String()) + } + return stdout2.String(), nil + } + + // If no uncommitted changes, the commit itself may contain the changes + // (auto-commit strategy creates commits on the active branch) + if strings.TrimSpace(stdout.String()) == "" { + + cmd2 := exec.CommandContext(context.Background(), "git", "diff", diffRef+"~1", diffRef) + cmd2.Dir = repoRoot + var stdout2 bytes.Buffer + cmd2.Stdout = &stdout2 + if err := cmd2.Run(); err == nil && strings.TrimSpace(stdout2.String()) != "" { + return stdout2.String(), nil + } + } + + return stdout.String(), nil +} + +// callClaudeForReview calls the claude CLI to perform the review. +func callClaudeForReview(prompt string) (string, error) { + cmd := exec.CommandContext(context.Background(), "claude", "--print", "--output-format", "json", "--model", wingmanReviewModel, "--setting-sources", "") + + // Isolate from git repo to prevent hooks and index pollution + cmd.Dir = os.TempDir() + cmd.Env = wingmanStripGitEnv(os.Environ()) + + cmd.Stdin = strings.NewReader(prompt) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + var execErr *exec.Error + if errors.As(err, &execErr) { + return "", fmt.Errorf("claude CLI not found: %w", err) + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return "", fmt.Errorf("claude CLI failed (exit %d): %s", exitErr.ExitCode(), stderr.String()) + } + return "", fmt.Errorf("failed to run claude CLI: %w", err) + } + + // Parse the CLI response + var cliResponse wingmanCLIResponse + if err := json.Unmarshal(stdout.Bytes(), &cliResponse); err != nil { + return "", fmt.Errorf("failed to parse claude CLI response: %w", err) + } + + return cliResponse.Result, nil +} + +// isSessionIdle checks if the given session is in the IDLE phase. +func isSessionIdle(sessionID string) bool { + state, err := strategy.LoadSessionState(sessionID) + if err != nil || state == nil { + return false + } + return state.Phase == session.PhaseIdle +} + +// triggerAutoApply spawns claude --continue to apply the review suggestions. +func triggerAutoApply(repoRoot string) error { + applyPrompt := "Read .entire/REVIEW.md. Apply each suggestion that you agree with. When done, delete .entire/REVIEW.md." + + cmd := exec.CommandContext(context.Background(), "claude", "--continue", "-p", applyPrompt, "--setting-sources", "") + cmd.Dir = repoRoot + // Strip GIT_* env vars to prevent hook interference, but keep other env + cmd.Env = wingmanStripGitEnv(os.Environ()) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + return cmd.Run() //nolint:wrapcheck // caller wraps +} + +// wingmanStripGitEnv returns a copy of env with all GIT_* variables removed. +// This prevents a subprocess from discovering or modifying the parent's git repo +// when we want isolation (e.g., running claude --print for review). +func wingmanStripGitEnv(env []string) []string { + filtered := make([]string, 0, len(env)) + for _, e := range env { + if !strings.HasPrefix(e, "GIT_") { + filtered = append(filtered, e) + } + } + return filtered +} diff --git a/cmd/entire/cli/wingman_spawn_other.go b/cmd/entire/cli/wingman_spawn_other.go new file mode 100644 index 000000000..59139bebe --- /dev/null +++ b/cmd/entire/cli/wingman_spawn_other.go @@ -0,0 +1,11 @@ +//go:build !unix + +package cli + +// spawnDetachedWingmanReview is a no-op on non-Unix platforms. +// Windows support for detached processes would require different syscall flags +// (CREATE_NEW_PROCESS_GROUP, DETACHED_PROCESS), but wingman is best-effort +// so we simply skip it on unsupported platforms. +func spawnDetachedWingmanReview(string) { + // No-op: detached subprocess spawning not implemented for this platform +} diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go new file mode 100644 index 000000000..f1f23eaa5 --- /dev/null +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -0,0 +1,48 @@ +//go:build unix + +package cli + +import ( + "context" + "io" + "os" + "os/exec" + "syscall" +) + +// spawnDetachedWingmanReview spawns a detached subprocess to run the wingman review. +// On Unix, this uses process group detachment so the subprocess continues +// after the parent exits. +func spawnDetachedWingmanReview(payloadJSON string) { + executable, err := os.Executable() + if err != nil { + return + } + + //nolint:gosec // G204: payloadJSON is controlled internally, not user input + cmd := exec.CommandContext(context.Background(), executable, "wingman", "__review", payloadJSON) + + // Detach from parent process group so subprocess survives parent exit + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + } + + // Don't hold the working directory + cmd.Dir = "/" + + // Inherit environment (needed for PATH, git config, etc.) + cmd.Env = os.Environ() + + // Discard stdout/stderr to prevent output leaking to parent's terminal + cmd.Stdout = io.Discard + cmd.Stderr = io.Discard + + // Start the process (non-blocking) + if err := cmd.Start(); err != nil { + return + } + + // Release the process so it can run independently + //nolint:errcheck // Best effort - process should continue regardless + _ = cmd.Process.Release() +} diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go new file mode 100644 index 000000000..08d0b0958 --- /dev/null +++ b/cmd/entire/cli/wingman_test.go @@ -0,0 +1,284 @@ +package cli + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestComputeFilesHash_Deterministic(t *testing.T) { + t.Parallel() + + files := []string{"b.go", "a.go", "c.go"} + hash1 := computeFilesHash(files) + hash2 := computeFilesHash(files) + + if hash1 != hash2 { + t.Errorf("expected deterministic hash, got %s and %s", hash1, hash2) + } + + // Order shouldn't matter + files2 := []string{"c.go", "a.go", "b.go"} + hash3 := computeFilesHash(files2) + + if hash1 != hash3 { + t.Errorf("expected order-independent hash, got %s and %s", hash1, hash3) + } +} + +func TestComputeFilesHash_DifferentFiles(t *testing.T) { + t.Parallel() + + hash1 := computeFilesHash([]string{"a.go", "b.go"}) + hash2 := computeFilesHash([]string{"a.go", "c.go"}) + + if hash1 == hash2 { + t.Error("expected different hashes for different file lists") + } +} + +func TestComputeFilesHash_Empty(t *testing.T) { + t.Parallel() + + hash := computeFilesHash(nil) + if hash == "" { + t.Error("expected non-empty hash for empty file list") + } +} + +func TestWingmanPayload_RoundTrip(t *testing.T) { + t.Parallel() + + payload := WingmanPayload{ + SessionID: "test-session-123", + RepoRoot: "/tmp/repo", + BaseCommit: "abc123def456", + ModifiedFiles: []string{"main.go", "util.go"}, + NewFiles: []string{"new.go"}, + DeletedFiles: []string{"old.go"}, + Prompts: []string{"Fix the bug", "Add tests"}, + CommitMessage: "fix: resolve issue", + } + + data, err := json.Marshal(payload) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var decoded WingmanPayload + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + if decoded.SessionID != payload.SessionID { + t.Errorf("session_id: got %q, want %q", decoded.SessionID, payload.SessionID) + } + if decoded.RepoRoot != payload.RepoRoot { + t.Errorf("repo_root: got %q, want %q", decoded.RepoRoot, payload.RepoRoot) + } + if decoded.BaseCommit != payload.BaseCommit { + t.Errorf("base_commit: got %q, want %q", decoded.BaseCommit, payload.BaseCommit) + } + if len(decoded.ModifiedFiles) != len(payload.ModifiedFiles) { + t.Errorf("modified_files: got %d, want %d", len(decoded.ModifiedFiles), len(payload.ModifiedFiles)) + } + if len(decoded.NewFiles) != len(payload.NewFiles) { + t.Errorf("new_files: got %d, want %d", len(decoded.NewFiles), len(payload.NewFiles)) + } + if len(decoded.DeletedFiles) != len(payload.DeletedFiles) { + t.Errorf("deleted_files: got %d, want %d", len(decoded.DeletedFiles), len(payload.DeletedFiles)) + } + if len(decoded.Prompts) != len(payload.Prompts) { + t.Errorf("prompts: got %d, want %d", len(decoded.Prompts), len(payload.Prompts)) + } + if decoded.CommitMessage != payload.CommitMessage { + t.Errorf("commit_message: got %q, want %q", decoded.CommitMessage, payload.CommitMessage) + } +} + +func TestBuildReviewPrompt_IncludesAllSections(t *testing.T) { + t.Parallel() + + prompts := []string{"Fix the authentication bug"} + fileList := "auth.go (modified), auth_test.go (new)" + diff := `diff --git a/auth.go b/auth.go +--- a/auth.go ++++ b/auth.go +@@ -10,6 +10,8 @@ func Login(user, pass string) error { ++ if user == "" { ++ return errors.New("empty user") ++ } +` + + result := buildReviewPrompt(prompts, fileList, diff) + + if !strings.Contains(result, "Fix the authentication bug") { + t.Error("prompt should contain user prompt") + } + if !strings.Contains(result, "auth.go (modified)") { + t.Error("prompt should contain file list") + } + if !strings.Contains(result, "diff --git") { + t.Error("prompt should contain diff") + } + if !strings.Contains(result, "senior code reviewer") { + t.Error("prompt should contain reviewer instruction") + } +} + +func TestBuildReviewPrompt_EmptyPrompts(t *testing.T) { + t.Parallel() + + result := buildReviewPrompt(nil, "file.go (modified)", "some diff") + + if !strings.Contains(result, "(no prompts captured)") { + t.Error("should show no-prompts placeholder for empty prompts") + } +} + +func TestBuildReviewPrompt_TruncatesLargeDiff(t *testing.T) { + t.Parallel() + + // Create a diff larger than maxDiffSize + largeDiff := strings.Repeat("x", maxDiffSize+1000) + + result := buildReviewPrompt([]string{"test"}, "file.go", largeDiff) + + if !strings.Contains(result, "diff truncated at 100KB") { + t.Error("should truncate large diffs") + } + // The prompt should not contain the full diff + if strings.Contains(result, strings.Repeat("x", maxDiffSize+1000)) { + t.Error("should not contain the full oversized diff") + } +} + +func TestWingmanState_SaveLoad(t *testing.T) { + // Uses t.Chdir so cannot be parallel + tmpDir := t.TempDir() + + // Initialize a real git repo (paths.AbsPath needs git rev-parse) + cmd := exec.CommandContext(context.Background(), "git", "init", tmpDir) + if err := cmd.Run(); err != nil { + t.Fatalf("failed to git init: %v", err) + } + + // Create .entire directory + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatalf("failed to create .entire directory: %v", err) + } + + t.Chdir(tmpDir) + + state := &WingmanState{ + SessionID: "test-session", + FilesHash: "abc123", + ReviewApplied: false, + } + + if err := saveWingmanState(state); err != nil { + t.Fatalf("failed to save state: %v", err) + } + + loaded, err := loadWingmanState() + if err != nil { + t.Fatalf("failed to load state: %v", err) + } + + if loaded.SessionID != state.SessionID { + t.Errorf("session_id: got %q, want %q", loaded.SessionID, state.SessionID) + } + if loaded.FilesHash != state.FilesHash { + t.Errorf("files_hash: got %q, want %q", loaded.FilesHash, state.FilesHash) + } + if loaded.ReviewApplied != state.ReviewApplied { + t.Errorf("review_applied: got %v, want %v", loaded.ReviewApplied, state.ReviewApplied) + } +} + +func TestWingmanStripGitEnv(t *testing.T) { + t.Parallel() + + env := []string{ + "PATH=/usr/bin", + "HOME=/home/user", + "GIT_DIR=/repo/.git", + "GIT_WORK_TREE=/repo", + "EDITOR=vim", + } + + filtered := wingmanStripGitEnv(env) + + for _, e := range filtered { + if strings.HasPrefix(e, "GIT_") { + t.Errorf("GIT_ variable should be stripped: %s", e) + } + } + + if len(filtered) != 3 { + t.Errorf("expected 3 non-GIT vars, got %d", len(filtered)) + } +} + +func TestIsWingmanEnabled_Settings(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + options map[string]any + expected bool + }{ + { + name: "nil options", + options: nil, + expected: false, + }, + { + name: "empty options", + options: map[string]any{}, + expected: false, + }, + { + name: "wingman not present", + options: map[string]any{"summarize": map[string]any{"enabled": true}}, + expected: false, + }, + { + name: "wingman enabled", + options: map[string]any{"wingman": map[string]any{"enabled": true}}, + expected: true, + }, + { + name: "wingman disabled", + options: map[string]any{"wingman": map[string]any{"enabled": false}}, + expected: false, + }, + { + name: "wingman wrong type", + options: map[string]any{"wingman": "invalid"}, + expected: false, + }, + { + name: "wingman enabled wrong type", + options: map[string]any{"wingman": map[string]any{"enabled": "yes"}}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + s := &EntireSettings{ + StrategyOptions: tt.options, + } + if got := s.IsWingmanEnabled(); got != tt.expected { + t.Errorf("IsWingmanEnabled() = %v, want %v", got, tt.expected) + } + }) + } +} From a6c2d1b0cecdf76821513fe97d5e914069bbdfdb Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 17:55:54 +0100 Subject: [PATCH 02/32] Add debug logging to wingman background review process Adds visibility into the detached wingman subprocess which previously discarded all output. Stderr is now redirected to .entire/logs/wingman.log with timestamped step-by-step logging, and the parent trigger uses the structured logging package for trigger-side events. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 778aa463e2ba --- cmd/entire/cli/wingman.go | 91 ++++++++++++++++--- cmd/entire/cli/wingman_review.go | 124 ++++++++++++++++++++++---- cmd/entire/cli/wingman_spawn_other.go | 2 +- cmd/entire/cli/wingman_spawn_unix.go | 26 ++++-- 4 files changed, 205 insertions(+), 38 deletions(-) diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index a509990f0..9b59947ba 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "os" "os/exec" "path/filepath" @@ -14,6 +15,7 @@ import ( "strings" "time" + "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/spf13/cobra" @@ -175,21 +177,28 @@ func newWingmanStatusCmd() *cobra.Command { // triggerWingmanReview checks preconditions and spawns the detached review process. func triggerWingmanReview(payload WingmanPayload) { + logCtx := logging.WithComponent(context.Background(), "wingman") repoRoot := payload.RepoRoot + totalFiles := len(payload.ModifiedFiles) + len(payload.NewFiles) + len(payload.DeletedFiles) + logging.Info(logCtx, "wingman trigger evaluating", + slog.String("session_id", payload.SessionID), + slog.Int("file_count", totalFiles), + ) + // Check if a pending REVIEW.md already exists reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if _, err := os.Stat(reviewPath); err == nil { + logging.Info(logCtx, "wingman skipped: pending review exists") fmt.Fprintf(os.Stderr, "[wingman] Pending review exists, skipping\n") return } - // Lock file prevents concurrent review spawns. Multiple rapid stop hooks - // could race past the dedup check (which reads stale state) before any - // review completes and updates the state file. + // Atomic lock file prevents concurrent review spawns. O_CREATE|O_EXCL + // is atomic on all platforms, avoiding the TOCTOU race of Stat+WriteFile. lockPath := filepath.Join(repoRoot, wingmanLockFile) - if _, err := os.Stat(lockPath); err == nil { - fmt.Fprintf(os.Stderr, "[wingman] Review already in progress, skipping\n") + if !acquireWingmanLock(lockPath, payload.SessionID) { + logging.Info(logCtx, "wingman skipped: could not acquire lock") return } @@ -202,6 +211,9 @@ func triggerWingmanReview(payload WingmanPayload) { state, _ := loadWingmanState() //nolint:errcheck // best-effort dedup if state != nil && state.FilesHash == filesHash && state.SessionID == payload.SessionID { + logging.Info(logCtx, "wingman skipped: dedup hash match", + slog.String("files_hash", filesHash[:12]), + ) fmt.Fprintf(os.Stderr, "[wingman] Already reviewed these changes, skipping\n") return } @@ -209,24 +221,76 @@ func triggerWingmanReview(payload WingmanPayload) { // Capture HEAD at trigger time so the detached review diffs against // the correct commit even if HEAD moves during the initial delay. payload.BaseCommit = resolveHEAD(repoRoot) + logging.Debug(logCtx, "wingman captured base commit", + slog.String("base_commit", payload.BaseCommit), + ) - // Write lock file synchronously before spawning to prevent races - //nolint:gosec // G306: lock file is not secrets - _ = os.WriteFile(lockPath, []byte(payload.SessionID), 0o644) //nolint:errcheck // best-effort lock - - // Marshal payload + // Write payload to a temp file instead of passing as a CLI argument, + // which can exceed OS argv limits (~128KB Linux, ~256KB macOS) with + // many files or long prompts. payloadJSON, err := json.Marshal(payload) if err != nil { + logging.Error(logCtx, "wingman failed to marshal payload", slog.Any("error", err)) fmt.Fprintf(os.Stderr, "[wingman] Failed to marshal payload: %v\n", err) _ = os.Remove(lockPath) return } + payloadPath := filepath.Join(repoRoot, ".entire", "wingman-payload.json") + //nolint:gosec // G306: payload file is not secrets + if err := os.WriteFile(payloadPath, payloadJSON, 0o644); err != nil { + logging.Error(logCtx, "wingman failed to write payload file", slog.Any("error", err)) + fmt.Fprintf(os.Stderr, "[wingman] Failed to write payload file: %v\n", err) + _ = os.Remove(lockPath) + return + } - // Spawn detached review process - spawnDetachedWingmanReview(string(payloadJSON)) + // Spawn detached review process with path to payload file + spawnDetachedWingmanReview(repoRoot, payloadPath) + logging.Info(logCtx, "wingman review spawned", + slog.String("session_id", payload.SessionID), + slog.String("base_commit", payload.BaseCommit), + slog.Int("file_count", totalFiles), + ) fmt.Fprintf(os.Stderr, "[wingman] Review starting in background...\n") } +// staleLockThreshold is how old a lock file can be before we consider it stale +// (e.g., the detached process was SIGKILLed and the defer never ran). +const staleLockThreshold = 30 * time.Minute + +// acquireWingmanLock atomically creates the lock file. Returns true if acquired. +// If the lock already exists but is older than staleLockThreshold, it is removed +// and re-acquired (handles crashed detached processes). +func acquireWingmanLock(lockPath, sessionID string) bool { + //nolint:gosec // G304: lockPath is constructed from repoRoot + constant + f, err := os.OpenFile(lockPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err != nil { + if !errors.Is(err, os.ErrExist) { + fmt.Fprintf(os.Stderr, "[wingman] Failed to create lock: %v\n", err) + return false + } + // Lock exists — check if it's stale + info, statErr := os.Stat(lockPath) + if statErr != nil || time.Since(info.ModTime()) <= staleLockThreshold { + fmt.Fprintf(os.Stderr, "[wingman] Review already in progress, skipping\n") + return false + } + fmt.Fprintf(os.Stderr, "[wingman] Removing stale lock (age: %s)\n", + time.Since(info.ModTime()).Round(time.Second)) + _ = os.Remove(lockPath) + // Retry the atomic create + //nolint:gosec // G304: lockPath is constructed from repoRoot + constant + f, err = os.OpenFile(lockPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Failed to create lock after stale removal: %v\n", err) + return false + } + } + _, _ = f.WriteString(sessionID) //nolint:errcheck // best-effort session ID write + _ = f.Close() + return true +} + // resolveHEAD returns the current HEAD commit hash, or empty string on error. func resolveHEAD(repoRoot string) string { cmd := exec.CommandContext(context.Background(), "git", "rev-parse", "HEAD") @@ -239,11 +303,12 @@ func resolveHEAD(repoRoot string) string { } // computeFilesHash returns a SHA256 hex digest of the sorted file paths. +// Uses null byte separator (impossible in filenames) to avoid ambiguity. func computeFilesHash(files []string) string { sorted := make([]string, len(files)) copy(sorted, files) sort.Strings(sorted) - h := sha256.Sum256([]byte(strings.Join(sorted, "\n"))) + h := sha256.Sum256([]byte(strings.Join(sorted, "\x00"))) return hex.EncodeToString(h[:]) } diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index bcd630729..48f53c95c 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -17,6 +17,13 @@ import ( "github.com/spf13/cobra" ) +// wingmanLog writes a timestamped log line to stderr. In the detached subprocess, +// stderr is redirected to .entire/logs/wingman.log by the spawner. +func wingmanLog(format string, args ...any) { + msg := fmt.Sprintf(format, args...) + fmt.Fprintf(os.Stderr, "%s [wingman] %s\n", time.Now().Format(time.RFC3339), msg) +} + const ( // wingmanInitialDelay is how long to wait before starting the review, // letting the agent turn fully settle. @@ -28,6 +35,15 @@ const ( // wingmanReviewModel is the Claude model used for reviews. wingmanReviewModel = "sonnet" + + // wingmanGitTimeout is the timeout for git diff operations. + wingmanGitTimeout = 60 * time.Second + + // wingmanReviewTimeout is the timeout for the claude --print review call. + wingmanReviewTimeout = 5 * time.Minute + + // wingmanApplyTimeout is the timeout for the claude --continue auto-apply call. + wingmanApplyTimeout = 15 * time.Minute ) // wingmanCLIResponse represents the JSON response from the Claude CLI --output-format json. @@ -46,33 +62,60 @@ func newWingmanReviewCmd() *cobra.Command { } } -func runWingmanReview(payloadJSON string) error { +func runWingmanReview(payloadPath string) error { + wingmanLog("review process started (pid=%d)", os.Getpid()) + wingmanLog("reading payload from %s", payloadPath) + + // Read payload from file (avoids OS argv limits with large payloads) + payloadJSON, err := os.ReadFile(payloadPath) //nolint:gosec // path is from our own spawn + if err != nil { + wingmanLog("ERROR reading payload: %v", err) + return fmt.Errorf("failed to read payload file: %w", err) + } + // Clean up payload file after reading + _ = os.Remove(payloadPath) + var payload WingmanPayload - if err := json.Unmarshal([]byte(payloadJSON), &payload); err != nil { + if err := json.Unmarshal(payloadJSON, &payload); err != nil { + wingmanLog("ERROR unmarshaling payload: %v", err) return fmt.Errorf("failed to unmarshal payload: %w", err) } repoRoot := payload.RepoRoot if repoRoot == "" { + wingmanLog("ERROR repo_root is empty in payload") return errors.New("repo_root is required in payload") } + totalFiles := len(payload.ModifiedFiles) + len(payload.NewFiles) + len(payload.DeletedFiles) + wingmanLog("session=%s repo=%s base_commit=%s files=%d", + payload.SessionID, repoRoot, payload.BaseCommit, totalFiles) + // Clean up lock file when review completes (regardless of success/failure) lockPath := filepath.Join(repoRoot, wingmanLockFile) - defer os.Remove(lockPath) + defer func() { + _ = os.Remove(lockPath) + wingmanLog("lock file removed") + }() // Initial delay: let the agent turn fully settle + wingmanLog("waiting %s for agent turn to settle", wingmanInitialDelay) time.Sleep(wingmanInitialDelay) // Compute diff using the base commit captured at trigger time + wingmanLog("computing diff against %s", payload.BaseCommit) + diffStart := time.Now() diff, err := computeDiff(repoRoot, payload.BaseCommit) if err != nil { + wingmanLog("ERROR computing diff: %v", err) return fmt.Errorf("failed to compute diff: %w", err) } if strings.TrimSpace(diff) == "" { + wingmanLog("no changes found in diff, exiting") return nil // No changes to review } + wingmanLog("diff computed: %d bytes in %s", len(diff), time.Since(diffStart).Round(time.Millisecond)) // Build file list for the prompt allFiles := make([]string, 0, len(payload.ModifiedFiles)+len(payload.NewFiles)+len(payload.DeletedFiles)) @@ -89,50 +132,66 @@ func runWingmanReview(payloadJSON string) error { // Build review prompt prompt := buildReviewPrompt(payload.Prompts, fileList, diff) + wingmanLog("review prompt built: %d bytes", len(prompt)) // Call Claude CLI for review + wingmanLog("calling claude CLI (model=%s, timeout=%s)", wingmanReviewModel, wingmanReviewTimeout) + reviewStart := time.Now() reviewText, err := callClaudeForReview(prompt) if err != nil { + wingmanLog("ERROR claude CLI failed after %s: %v", time.Since(reviewStart).Round(time.Millisecond), err) return fmt.Errorf("failed to get review from Claude: %w", err) } + wingmanLog("review received: %d bytes in %s", len(reviewText), time.Since(reviewStart).Round(time.Millisecond)) // Write REVIEW.md reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if err := os.MkdirAll(filepath.Dir(reviewPath), 0o750); err != nil { + wingmanLog("ERROR creating directory: %v", err) return fmt.Errorf("failed to create directory: %w", err) } //nolint:gosec // G306: review file is not secrets if err := os.WriteFile(reviewPath, []byte(reviewText), 0o644); err != nil { + wingmanLog("ERROR writing REVIEW.md: %v", err) return fmt.Errorf("failed to write REVIEW.md: %w", err) } + wingmanLog("REVIEW.md written to %s", reviewPath) - // Update dedup state + // Update dedup state — write directly to known path instead of using + // os.Chdir (which mutates process-wide state). allFilePaths := make([]string, 0, len(payload.ModifiedFiles)+len(payload.NewFiles)+len(payload.DeletedFiles)) allFilePaths = append(allFilePaths, payload.ModifiedFiles...) allFilePaths = append(allFilePaths, payload.NewFiles...) allFilePaths = append(allFilePaths, payload.DeletedFiles...) - // Save state from repo root context - if err := os.Chdir(repoRoot); err == nil { - _ = saveWingmanState(&WingmanState{ //nolint:errcheck // best-effort state save - SessionID: payload.SessionID, - FilesHash: computeFilesHash(allFilePaths), - ReviewedAt: time.Now(), - ReviewApplied: false, - }) - } + saveWingmanStateDirect(repoRoot, &WingmanState{ + SessionID: payload.SessionID, + FilesHash: computeFilesHash(allFilePaths), + ReviewedAt: time.Now(), + ReviewApplied: false, + }) + wingmanLog("dedup state saved") // Auto-apply phase: wait then check if session is idle + wingmanLog("waiting %s before auto-apply check", wingmanApplyDelay) time.Sleep(wingmanApplyDelay) - if !isSessionIdle(payload.SessionID) { + idle := isSessionIdle(payload.SessionID) + wingmanLog("session idle check: idle=%v", idle) + + if !idle { + wingmanLog("session is active, leaving REVIEW.md for notification") return nil // User is active, leave REVIEW.md for notification } // Trigger auto-apply via claude --continue + wingmanLog("triggering auto-apply via claude --continue") + applyStart := time.Now() if err := triggerAutoApply(repoRoot); err != nil { + wingmanLog("ERROR auto-apply failed after %s: %v", time.Since(applyStart).Round(time.Millisecond), err) return fmt.Errorf("failed to trigger auto-apply: %w", err) } + wingmanLog("auto-apply completed in %s", time.Since(applyStart).Round(time.Millisecond)) return nil } @@ -148,8 +207,10 @@ func computeDiff(repoRoot string, baseCommit string) (string, error) { } // Try uncommitted changes against the base commit + ctx, cancel := context.WithTimeout(context.Background(), wingmanGitTimeout) + defer cancel() - cmd := exec.CommandContext(context.Background(), "git", "diff", diffRef) + cmd := exec.CommandContext(ctx, "git", "diff", diffRef) cmd.Dir = repoRoot var stdout, stderr bytes.Buffer cmd.Stdout = &stdout @@ -157,7 +218,9 @@ func computeDiff(repoRoot string, baseCommit string) (string, error) { if err := cmd.Run(); err != nil { // If the ref doesn't exist (initial commit), try diff of staged/unstaged - cmd2 := exec.CommandContext(context.Background(), "git", "diff") + ctx2, cancel2 := context.WithTimeout(context.Background(), wingmanGitTimeout) + defer cancel2() + cmd2 := exec.CommandContext(ctx2, "git", "diff") cmd2.Dir = repoRoot var stdout2 bytes.Buffer cmd2.Stdout = &stdout2 @@ -170,8 +233,10 @@ func computeDiff(repoRoot string, baseCommit string) (string, error) { // If no uncommitted changes, the commit itself may contain the changes // (auto-commit strategy creates commits on the active branch) if strings.TrimSpace(stdout.String()) == "" { + ctx2, cancel2 := context.WithTimeout(context.Background(), wingmanGitTimeout) + defer cancel2() - cmd2 := exec.CommandContext(context.Background(), "git", "diff", diffRef+"~1", diffRef) + cmd2 := exec.CommandContext(ctx2, "git", "diff", diffRef+"~1", diffRef) cmd2.Dir = repoRoot var stdout2 bytes.Buffer cmd2.Stdout = &stdout2 @@ -185,7 +250,10 @@ func computeDiff(repoRoot string, baseCommit string) (string, error) { // callClaudeForReview calls the claude CLI to perform the review. func callClaudeForReview(prompt string) (string, error) { - cmd := exec.CommandContext(context.Background(), "claude", "--print", "--output-format", "json", "--model", wingmanReviewModel, "--setting-sources", "") + ctx, cancel := context.WithTimeout(context.Background(), wingmanReviewTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "claude", "--print", "--output-format", "json", "--model", wingmanReviewModel, "--setting-sources", "") // Isolate from git repo to prevent hooks and index pollution cmd.Dir = os.TempDir() @@ -231,7 +299,10 @@ func isSessionIdle(sessionID string) bool { func triggerAutoApply(repoRoot string) error { applyPrompt := "Read .entire/REVIEW.md. Apply each suggestion that you agree with. When done, delete .entire/REVIEW.md." - cmd := exec.CommandContext(context.Background(), "claude", "--continue", "-p", applyPrompt, "--setting-sources", "") + ctx, cancel := context.WithTimeout(context.Background(), wingmanApplyTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "claude", "--continue", "-p", applyPrompt, "--setting-sources", "") cmd.Dir = repoRoot // Strip GIT_* env vars to prevent hook interference, but keep other env cmd.Env = wingmanStripGitEnv(os.Environ()) @@ -255,3 +326,18 @@ func wingmanStripGitEnv(env []string) []string { } return filtered } + +// saveWingmanStateDirect writes the wingman state file directly to a known path +// under repoRoot, avoiding os.Chdir (which mutates process-wide state). +func saveWingmanStateDirect(repoRoot string, state *WingmanState) { + statePath := filepath.Join(repoRoot, wingmanStateFile) + if err := os.MkdirAll(filepath.Dir(statePath), 0o750); err != nil { + return + } + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return + } + //nolint:gosec,errcheck // G306: state file is config, not secrets; best-effort write + _ = os.WriteFile(statePath, data, 0o644) +} diff --git a/cmd/entire/cli/wingman_spawn_other.go b/cmd/entire/cli/wingman_spawn_other.go index 59139bebe..1be15ade6 100644 --- a/cmd/entire/cli/wingman_spawn_other.go +++ b/cmd/entire/cli/wingman_spawn_other.go @@ -6,6 +6,6 @@ package cli // Windows support for detached processes would require different syscall flags // (CREATE_NEW_PROCESS_GROUP, DETACHED_PROCESS), but wingman is best-effort // so we simply skip it on unsupported platforms. -func spawnDetachedWingmanReview(string) { +func spawnDetachedWingmanReview(_, _ string) { // No-op: detached subprocess spawning not implemented for this platform } diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index f1f23eaa5..a39e08183 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -7,20 +7,23 @@ import ( "io" "os" "os/exec" + "path/filepath" "syscall" ) // spawnDetachedWingmanReview spawns a detached subprocess to run the wingman review. +// repoRoot is the repository root used to locate the log file. +// payloadPath is the path to the JSON payload file. // On Unix, this uses process group detachment so the subprocess continues // after the parent exits. -func spawnDetachedWingmanReview(payloadJSON string) { +func spawnDetachedWingmanReview(repoRoot, payloadPath string) { executable, err := os.Executable() if err != nil { return } - //nolint:gosec // G204: payloadJSON is controlled internally, not user input - cmd := exec.CommandContext(context.Background(), executable, "wingman", "__review", payloadJSON) + //nolint:gosec // G204: payloadPath is controlled internally, not user input + cmd := exec.CommandContext(context.Background(), executable, "wingman", "__review", payloadPath) // Detach from parent process group so subprocess survives parent exit cmd.SysProcAttr = &syscall.SysProcAttr{ @@ -33,9 +36,22 @@ func spawnDetachedWingmanReview(payloadJSON string) { // Inherit environment (needed for PATH, git config, etc.) cmd.Env = os.Environ() - // Discard stdout/stderr to prevent output leaking to parent's terminal + // Redirect stderr to a log file for debugging the background process. + // This catches panics, errors, and all wingmanLog() output. cmd.Stdout = io.Discard - cmd.Stderr = io.Discard + logDir := filepath.Join(repoRoot, ".entire", "logs") + if mkErr := os.MkdirAll(logDir, 0o750); mkErr == nil { + //nolint:gosec // G304: path is constructed from repoRoot + constants + if f, openErr := os.OpenFile(filepath.Join(logDir, "wingman.log"), + os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600); openErr == nil { + cmd.Stderr = f + // f stays open in the child process; the OS closes it on exit + } else { + cmd.Stderr = io.Discard + } + } else { + cmd.Stderr = io.Discard + } // Start the process (non-blocking) if err := cmd.Start(); err != nil { From 41456e62cef6ffc3066771de96c35f54bf754e0f Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 18:47:32 +0100 Subject: [PATCH 03/32] Enhance wingman: intent-aware review, post-commit trigger, recursion prevention - Move wingman trigger from stop hook to git post-commit hook for manual-commit strategy (auto-commit still triggers from stop hook) - Add ENTIRE_WINGMAN_APPLY env var to prevent infinite hook recursion - Rewrite review prompt to be intent-aware using checkpoint data (prompts, commit message, session context, checkpoint file paths) - Give reviewer read-only repo access (--allowedTools Read,Glob,Grep) - Give auto-apply agent edit permissions (--permission-mode acceptEdits) - Add session context reading from .entire/metadata//context.md - Add Agent field to settings parser for playground compatibility Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 3110b32e99ff --- cmd/entire/cli/hooks_claudecode_handlers.go | 6 +- cmd/entire/cli/hooks_git_cmd.go | 4 + cmd/entire/cli/settings/settings.go | 14 +++ cmd/entire/cli/wingman.go | 80 +++++++++++++++++ cmd/entire/cli/wingman_prompt.go | 99 ++++++++++++++++----- cmd/entire/cli/wingman_review.go | 65 +++++++++++--- cmd/entire/cli/wingman_test.go | 32 ++++++- 7 files changed, 264 insertions(+), 36 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index b1ee0ef9a..777972eb0 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -388,8 +388,10 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba // For ACTIVE_COMMITTED → IDLE, HandleTurnEnd dispatches ActionCondense. transitionSessionTurnEnd(sessionID) - // Trigger wingman review if enabled and files changed - if totalChanges > 0 && settings.IsWingmanEnabled() { + // Trigger wingman review for auto-commit strategy (commit already happened + // in SaveChanges). Manual-commit triggers wingman from the git post-commit hook + // instead, since the user commits manually. + if totalChanges > 0 && strat.Name() == strategy.StrategyNameAutoCommit && settings.IsWingmanEnabled() { triggerWingmanReview(WingmanPayload{ SessionID: sessionID, RepoRoot: repoRoot, diff --git a/cmd/entire/cli/hooks_git_cmd.go b/cmd/entire/cli/hooks_git_cmd.go index e97156676..c98e6dc22 100644 --- a/cmd/entire/cli/hooks_git_cmd.go +++ b/cmd/entire/cli/hooks_git_cmd.go @@ -163,6 +163,10 @@ func newHooksGitPostCommitCmd() *cobra.Command { g.logCompleted(hookErr) } + // Trigger wingman review after commit (manual-commit strategy). + // Auto-commit triggers from the stop hook instead. + triggerWingmanFromCommit() + return nil }, } diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index 3aa1f4a69..841604fe2 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -30,6 +30,9 @@ type EntireSettings struct { // Strategy is the name of the git strategy to use Strategy string `json:"strategy"` + // Agent is the name of the agent (e.g., "claude-code", "amp") + Agent string `json:"agent,omitempty"` + // Enabled indicates whether Entire is active. When false, CLI commands // show a disabled message and hooks exit silently. Defaults to true. Enabled bool `json:"enabled"` @@ -151,6 +154,17 @@ func mergeJSON(settings *EntireSettings, data []byte) error { } } + // Override agent if present and non-empty + if agentRaw, ok := raw["agent"]; ok { + var a string + if err := json.Unmarshal(agentRaw, &a); err != nil { + return fmt.Errorf("parsing agent field: %w", err) + } + if a != "" { + settings.Agent = a + } + } + // Override enabled if present if enabledRaw, ok := raw["enabled"]; ok { var e bool diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index 9b59947ba..240bcebd0 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -18,6 +18,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" + "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/spf13/cobra" ) @@ -177,6 +178,12 @@ func newWingmanStatusCmd() *cobra.Command { // triggerWingmanReview checks preconditions and spawns the detached review process. func triggerWingmanReview(payload WingmanPayload) { + // Prevent infinite recursion: if we're inside a wingman auto-apply, + // don't trigger another review. The env var is set by triggerAutoApply. + if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + return + } + logCtx := logging.WithComponent(context.Background(), "wingman") repoRoot := payload.RepoRoot @@ -254,6 +261,79 @@ func triggerWingmanReview(payload WingmanPayload) { fmt.Fprintf(os.Stderr, "[wingman] Review starting in background...\n") } +// triggerWingmanFromCommit builds a wingman payload from the HEAD commit and +// triggers a review. Used by the git post-commit hook for manual-commit strategy +// where files are committed by the user (not by SaveChanges). +func triggerWingmanFromCommit() { + // Prevent infinite recursion: skip if inside wingman auto-apply + if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + return + } + if !settings.IsWingmanEnabled() { + return + } + + repoRoot, err := paths.RepoRoot() + if err != nil { + return + } + + head := resolveHEAD(repoRoot) + if head == "" { + return + } + + // Get changed files from the commit + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + //nolint:gosec // G204: head is from git rev-parse, not user input + cmd := exec.CommandContext(ctx, "git", "diff-tree", "--no-commit-id", "--name-status", "-r", head) + cmd.Dir = repoRoot + out, err := cmd.Output() + if err != nil || len(out) == 0 { + return + } + + var modified, newFiles, deleted []string + for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + if len(line) < 3 { + continue + } + status := line[0] + file := strings.TrimSpace(line[1:]) + switch status { + case 'M': + modified = append(modified, file) + case 'A': + newFiles = append(newFiles, file) + case 'D': + deleted = append(deleted, file) + } + } + + if len(modified)+len(newFiles)+len(deleted) == 0 { + return + } + + // Get commit message + //nolint:gosec // G204: head is from git rev-parse, not user input + msgCmd := exec.CommandContext(ctx, "git", "log", "-1", "--format=%B", head) + msgCmd.Dir = repoRoot + msgOut, _ := msgCmd.Output() //nolint:errcheck // best-effort commit message + commitMessage := strings.TrimSpace(string(msgOut)) + + sessionID := strategy.FindMostRecentSession() + + triggerWingmanReview(WingmanPayload{ + SessionID: sessionID, + RepoRoot: repoRoot, + ModifiedFiles: modified, + NewFiles: newFiles, + DeletedFiles: deleted, + CommitMessage: commitMessage, + }) +} + // staleLockThreshold is how old a lock file can be before we consider it stale // (e.g., the detached process was SIGKILLed and the defer never ran). const staleLockThreshold = 30 * time.Minute diff --git a/cmd/entire/cli/wingman_prompt.go b/cmd/entire/cli/wingman_prompt.go index 20040ed90..50701385a 100644 --- a/cmd/entire/cli/wingman_prompt.go +++ b/cmd/entire/cli/wingman_prompt.go @@ -11,17 +11,36 @@ const maxDiffSize = 100 * 1024 // 100KB // reviewPromptTemplate is the prompt sent to Claude for code review. // -// Security note: User content (diff, prompts) is wrapped in XML tags to provide -// clear boundary markers, similar to the summarization prompt pattern. -const reviewPromptTemplate = `You are a senior code reviewer. Review the following code changes and provide actionable feedback. +// Security note: User content (diff, prompts, context) is wrapped in XML tags +// to provide clear boundary markers, similar to the summarization prompt pattern. +const reviewPromptTemplate = `You are a senior code reviewer performing an intent-aware review. Your job is not just to find bugs in the code — it is to evaluate whether the changes correctly and completely fulfill what the developer was trying to accomplish. -## Context +## Session Context -The developer's intent (from their prompts): +This review is part of an automated development workflow. The developer works with an AI coding agent that makes changes on their behalf. Below is the checkpoint data captured during the session, which tells you WHY these changes were made. + +### Developer's Prompts +The original instructions the developer gave to the agent: %s +### Commit Message +%s + +### Session Context +Checkpoint data including the session summary and key actions taken: + +%s + + +### Checkpoint Files +You have read-only access to the repository. If you need deeper context about the session, +you can read these checkpoint files (they may or may not exist): +- ` + "`%s`" + ` — session transcript (JSONL format) +- ` + "`%s`" + ` — user prompts collected during session +- ` + "`%s`" + ` — generated session context/summary + ## Code Changes Files changed: %s @@ -30,27 +49,47 @@ Files changed: %s %s -## Instructions +## Review Instructions + +Use the session context above to understand the developer's intent. Then review the code changes with that intent in mind. + +**Intent alignment** (most important): +- Do the changes actually accomplish what the developer asked for? +- Are there any prompts or requirements that were missed or only partially implemented? +- Does the implementation match the stated approach in the session context? + +**Correctness**: +- Bugs, logic errors, or off-by-one mistakes +- Race conditions or concurrency issues +- Missing error handling for failure paths that matter -Review the code changes above. Focus on: -- Bugs or logic errors -- Security vulnerabilities -- Performance issues -- Missing error handling -- Code style and readability issues +**Security**: +- Injection vulnerabilities (SQL, command, XSS) +- Hardcoded secrets or credentials +- Unsafe file operations or path traversal + +**Robustness**: +- Edge cases not handled (empty inputs, nil pointers, large data) +- Resource leaks (unclosed files, connections, goroutines) +- Missing timeouts on external calls + +Do NOT flag: +- Style preferences or formatting (the linter handles that) +- Missing comments or documentation on clear code +- Theoretical issues that cannot happen given the actual call sites For each issue found, provide: 1. Severity: CRITICAL, WARNING, or SUGGESTION -2. File path and line number (from the diff) +2. File path and approximate line reference (from the diff) 3. Description of the issue -4. Suggested fix +4. Suggested fix (code snippet when helpful) -Format your response as Markdown with this structure: +Format your response as Markdown: # Code Review ## Summary -Brief overview of the changes and overall assessment. +Brief assessment: does this change accomplish its stated goal? What's the overall quality? ## Issues @@ -62,20 +101,40 @@ Brief overview of the changes and overall assessment. // suggested fix ` + "```" + ` -If no issues are found, say so and optionally provide suggestions for improvement. +If no issues are found, confirm the changes look correct and match the intent. Do NOT include any preamble or explanation outside the Markdown structure above.` -// buildReviewPrompt constructs the review prompt from the payload and diff. -func buildReviewPrompt(prompts []string, fileList string, diff string) string { +// buildReviewPrompt constructs the review prompt from the payload, context, and diff. +func buildReviewPrompt(prompts []string, commitMessage, sessionContext, sessionID, fileList, diff string) string { promptText := strings.Join(prompts, "\n\n---\n\n") if promptText == "" { promptText = "(no prompts captured)" } + commitMsgText := commitMessage + if commitMsgText == "" { + commitMsgText = "(no commit message)" + } + + contextText := sessionContext + if contextText == "" { + contextText = "(no session context available)" + } + + // Build checkpoint file paths + metadataDir := ".entire/metadata/" + sessionID + if sessionID == "" { + metadataDir = ".entire/metadata/" + } + transcriptPath := metadataDir + "/full.jsonl" + promptPath := metadataDir + "/prompt.txt" + contextPath := metadataDir + "/context.md" + // Truncate large diffs if len(diff) > maxDiffSize { diff = diff[:maxDiffSize] + "\n\n... (diff truncated at 100KB)" } - return fmt.Sprintf(reviewPromptTemplate, promptText, fileList, diff) + return fmt.Sprintf(reviewPromptTemplate, promptText, commitMsgText, contextText, + transcriptPath, promptPath, contextPath, fileList, diff) } diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 48f53c95c..8251b0ab2 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -130,14 +130,20 @@ func runWingmanReview(payloadPath string) error { } fileList := strings.Join(allFiles, ", ") + // Read session context from checkpoint data (best-effort) + sessionContext := readSessionContext(repoRoot, payload.SessionID) + if sessionContext != "" { + wingmanLog("session context loaded: %d bytes", len(sessionContext)) + } + // Build review prompt - prompt := buildReviewPrompt(payload.Prompts, fileList, diff) + prompt := buildReviewPrompt(payload.Prompts, payload.CommitMessage, sessionContext, payload.SessionID, fileList, diff) wingmanLog("review prompt built: %d bytes", len(prompt)) // Call Claude CLI for review wingmanLog("calling claude CLI (model=%s, timeout=%s)", wingmanReviewModel, wingmanReviewTimeout) reviewStart := time.Now() - reviewText, err := callClaudeForReview(prompt) + reviewText, err := callClaudeForReview(repoRoot, prompt) if err != nil { wingmanLog("ERROR claude CLI failed after %s: %v", time.Since(reviewStart).Round(time.Millisecond), err) return fmt.Errorf("failed to get review from Claude: %w", err) @@ -249,14 +255,27 @@ func computeDiff(repoRoot string, baseCommit string) (string, error) { } // callClaudeForReview calls the claude CLI to perform the review. -func callClaudeForReview(prompt string) (string, error) { +// repoRoot is the repository root so the reviewer can access the full codebase. +func callClaudeForReview(repoRoot, prompt string) (string, error) { ctx, cancel := context.WithTimeout(context.Background(), wingmanReviewTimeout) defer cancel() - cmd := exec.CommandContext(ctx, "claude", "--print", "--output-format", "json", "--model", wingmanReviewModel, "--setting-sources", "") - - // Isolate from git repo to prevent hooks and index pollution - cmd.Dir = os.TempDir() + cmd := exec.CommandContext(ctx, "claude", + "--print", + "--output-format", "json", + "--model", wingmanReviewModel, + "--setting-sources", "", + // Grant read-only tool access so the reviewer can inspect source files + // beyond just the diff. Permission bypass is safe here since tools are + // restricted to read-only operations. + "--allowedTools", "Read,Glob,Grep", + "--permission-mode", "bypassPermissions", + ) + + // Run from repo root so the reviewer can read source files for context. + // Loop-breaking is handled by --setting-sources "" (disables hooks) and + // wingmanStripGitEnv (prevents git index pollution). + cmd.Dir = repoRoot cmd.Env = wingmanStripGitEnv(os.Environ()) cmd.Stdin = strings.NewReader(prompt) @@ -286,6 +305,20 @@ func callClaudeForReview(prompt string) (string, error) { return cliResponse.Result, nil } +// readSessionContext reads the context.md file from the session's checkpoint +// metadata directory. Returns empty string if unavailable. +func readSessionContext(repoRoot, sessionID string) string { + if sessionID == "" { + return "" + } + contextPath := filepath.Join(repoRoot, ".entire", "metadata", sessionID, "context.md") + data, err := os.ReadFile(contextPath) //nolint:gosec // path constructed from repoRoot + session ID + if err != nil { + return "" + } + return string(data) +} + // isSessionIdle checks if the given session is in the IDLE phase. func isSessionIdle(sessionID string) bool { state, err := strategy.LoadSessionState(sessionID) @@ -302,10 +335,22 @@ func triggerAutoApply(repoRoot string) error { ctx, cancel := context.WithTimeout(context.Background(), wingmanApplyTimeout) defer cancel() - cmd := exec.CommandContext(ctx, "claude", "--continue", "-p", applyPrompt, "--setting-sources", "") + cmd := exec.CommandContext(ctx, "claude", + "--continue", + "--print", + "--setting-sources", "", + // Auto-accept edits so the agent can modify files and delete REVIEW.md + // without requiring user consent (this runs in a background process). + "--permission-mode", "acceptEdits", + applyPrompt, + ) cmd.Dir = repoRoot - // Strip GIT_* env vars to prevent hook interference, but keep other env - cmd.Env = wingmanStripGitEnv(os.Environ()) + // Strip GIT_* env vars to prevent hook interference, and set + // ENTIRE_WINGMAN_APPLY=1 so git hooks (post-commit) know not to + // trigger another wingman review (prevents infinite recursion). + env := wingmanStripGitEnv(os.Environ()) + env = append(env, "ENTIRE_WINGMAN_APPLY=1") + cmd.Env = env var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 08d0b0958..491644702 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -114,7 +114,10 @@ func TestBuildReviewPrompt_IncludesAllSections(t *testing.T) { + } ` - result := buildReviewPrompt(prompts, fileList, diff) + commitMsg := "Fix empty user login crash" + sessionCtx := "## Summary\nFixed authentication bug for empty usernames" + + result := buildReviewPrompt(prompts, commitMsg, sessionCtx, "test-session-456", fileList, diff) if !strings.Contains(result, "Fix the authentication bug") { t.Error("prompt should contain user prompt") @@ -125,19 +128,40 @@ func TestBuildReviewPrompt_IncludesAllSections(t *testing.T) { if !strings.Contains(result, "diff --git") { t.Error("prompt should contain diff") } - if !strings.Contains(result, "senior code reviewer") { + if !strings.Contains(result, "intent-aware review") { t.Error("prompt should contain reviewer instruction") } + if !strings.Contains(result, "Fix empty user login crash") { + t.Error("prompt should contain commit message") + } + if !strings.Contains(result, "Fixed authentication bug") { + t.Error("prompt should contain session context") + } + if !strings.Contains(result, ".entire/metadata/test-session-456/full.jsonl") { + t.Error("prompt should contain checkpoint transcript path") + } + if !strings.Contains(result, ".entire/metadata/test-session-456/prompt.txt") { + t.Error("prompt should contain checkpoint prompt path") + } + if !strings.Contains(result, ".entire/metadata/test-session-456/context.md") { + t.Error("prompt should contain checkpoint context path") + } } func TestBuildReviewPrompt_EmptyPrompts(t *testing.T) { t.Parallel() - result := buildReviewPrompt(nil, "file.go (modified)", "some diff") + result := buildReviewPrompt(nil, "", "", "", "file.go (modified)", "some diff") if !strings.Contains(result, "(no prompts captured)") { t.Error("should show no-prompts placeholder for empty prompts") } + if !strings.Contains(result, "(no commit message)") { + t.Error("should show placeholder for empty commit message") + } + if !strings.Contains(result, "(no session context available)") { + t.Error("should show placeholder for empty session context") + } } func TestBuildReviewPrompt_TruncatesLargeDiff(t *testing.T) { @@ -146,7 +170,7 @@ func TestBuildReviewPrompt_TruncatesLargeDiff(t *testing.T) { // Create a diff larger than maxDiffSize largeDiff := strings.Repeat("x", maxDiffSize+1000) - result := buildReviewPrompt([]string{"test"}, "file.go", largeDiff) + result := buildReviewPrompt([]string{"test"}, "", "", "test-session", "file.go", largeDiff) if !strings.Contains(result, "diff truncated at 100KB") { t.Error("should truncate large diffs") From 73d03f210f2cb703ff8c36248ebe28024c6cc7c8 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 18:58:26 +0100 Subject: [PATCH 04/32] Inject wingman review instruction into agent context on prompt-submit When a pending REVIEW.md exists and the user submits a prompt, inject a systemMessage via the hook response so the agent automatically reads and applies the review suggestions before proceeding with the user's request. Previously this was only a stderr notification to the user. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 652d24e097d2 --- cmd/entire/cli/hooks_claudecode_handlers.go | 8 ++++++-- cmd/entire/cli/wingman.go | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 777972eb0..17edb8880 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -76,12 +76,16 @@ func captureInitialState() error { return err } - // Notify user if a wingman review is pending + // If a wingman review is pending, inject instruction into the agent's context + // so it automatically reads and applies the suggestions. if settings.IsWingmanEnabled() { repoRoot, rootErr := paths.RepoRoot() if rootErr == nil { if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { - fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md — review and apply suggestions\n") + fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") + if err := outputHookResponse(wingmanApplySystemMessage); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) + } } } } diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index 240bcebd0..82bf2a351 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -46,6 +46,13 @@ const ( wingmanStateFile = ".entire/wingman-state.json" wingmanReviewFile = ".entire/REVIEW.md" wingmanLockFile = ".entire/wingman.lock" + + // wingmanApplySystemMessage is injected into the agent's context via the + // user-prompt-submit hook when a pending REVIEW.md exists. This instructs + // the agent to apply the review suggestions before proceeding. + wingmanApplySystemMessage = "\n\n[Wingman Review] A code review is available at .entire/REVIEW.md. " + + "Before addressing the user's request, read .entire/REVIEW.md and apply any suggestions you agree with. " + + "After applying, delete .entire/REVIEW.md." ) func newWingmanCmd() *cobra.Command { From d4dbd802751cd33999b23c7bef761cd6b328b749 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 19:02:55 +0100 Subject: [PATCH 05/32] Extract wingman apply instruction into embedded markdown file Both the prompt-submit hook injection and the auto-apply prompt now use the same instruction from wingman_apply.md via go:embed, replacing two divergent hardcoded strings with a single source of truth. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 01916e5b6e76 --- cmd/entire/cli/hooks_claudecode_handlers.go | 2 +- cmd/entire/cli/wingman.go | 11 ++++------- cmd/entire/cli/wingman_apply.md | 3 +++ cmd/entire/cli/wingman_review.go | 4 +--- 4 files changed, 9 insertions(+), 11 deletions(-) create mode 100644 cmd/entire/cli/wingman_apply.md diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 17edb8880..44f22d323 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -83,7 +83,7 @@ func captureInitialState() error { if rootErr == nil { if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") - if err := outputHookResponse(wingmanApplySystemMessage); err != nil { + if err := outputHookResponse(wingmanApplyInstruction); err != nil { fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) } } diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index 82bf2a351..44cd6d0d8 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -3,6 +3,7 @@ package cli import ( "context" "crypto/sha256" + _ "embed" "encoding/hex" "encoding/json" "errors" @@ -42,17 +43,13 @@ type WingmanState struct { ReviewApplied bool `json:"review_applied"` } +//go:embed wingman_apply.md +var wingmanApplyInstruction string + const ( wingmanStateFile = ".entire/wingman-state.json" wingmanReviewFile = ".entire/REVIEW.md" wingmanLockFile = ".entire/wingman.lock" - - // wingmanApplySystemMessage is injected into the agent's context via the - // user-prompt-submit hook when a pending REVIEW.md exists. This instructs - // the agent to apply the review suggestions before proceeding. - wingmanApplySystemMessage = "\n\n[Wingman Review] A code review is available at .entire/REVIEW.md. " + - "Before addressing the user's request, read .entire/REVIEW.md and apply any suggestions you agree with. " + - "After applying, delete .entire/REVIEW.md." ) func newWingmanCmd() *cobra.Command { diff --git a/cmd/entire/cli/wingman_apply.md b/cmd/entire/cli/wingman_apply.md new file mode 100644 index 000000000..8037062ba --- /dev/null +++ b/cmd/entire/cli/wingman_apply.md @@ -0,0 +1,3 @@ +[Wingman Review] A code review is available at `.entire/REVIEW.md`. + +Read `.entire/REVIEW.md` and apply each suggestion you agree with. After applying, delete `.entire/REVIEW.md`. diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 8251b0ab2..7e235cc6e 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -330,8 +330,6 @@ func isSessionIdle(sessionID string) bool { // triggerAutoApply spawns claude --continue to apply the review suggestions. func triggerAutoApply(repoRoot string) error { - applyPrompt := "Read .entire/REVIEW.md. Apply each suggestion that you agree with. When done, delete .entire/REVIEW.md." - ctx, cancel := context.WithTimeout(context.Background(), wingmanApplyTimeout) defer cancel() @@ -342,7 +340,7 @@ func triggerAutoApply(repoRoot string) error { // Auto-accept edits so the agent can modify files and delete REVIEW.md // without requiring user consent (this runs in a background process). "--permission-mode", "acceptEdits", - applyPrompt, + wingmanApplyInstruction, ) cmd.Dir = repoRoot // Strip GIT_* env vars to prevent hook interference, and set From 962a3c9d112a911401dc73e8e39305b662104d52 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 19:08:35 +0100 Subject: [PATCH 06/32] Rename wingman_apply.md to wingman_instruction.md Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: dabe38b90045 --- cmd/entire/cli/wingman.go | 2 +- cmd/entire/cli/wingman_apply.md | 3 --- cmd/entire/cli/wingman_instruction.md | 3 +++ 3 files changed, 4 insertions(+), 4 deletions(-) delete mode 100644 cmd/entire/cli/wingman_apply.md create mode 100644 cmd/entire/cli/wingman_instruction.md diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index 44cd6d0d8..f98cb4d1c 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -43,7 +43,7 @@ type WingmanState struct { ReviewApplied bool `json:"review_applied"` } -//go:embed wingman_apply.md +//go:embed wingman_instruction.md var wingmanApplyInstruction string const ( diff --git a/cmd/entire/cli/wingman_apply.md b/cmd/entire/cli/wingman_apply.md deleted file mode 100644 index 8037062ba..000000000 --- a/cmd/entire/cli/wingman_apply.md +++ /dev/null @@ -1,3 +0,0 @@ -[Wingman Review] A code review is available at `.entire/REVIEW.md`. - -Read `.entire/REVIEW.md` and apply each suggestion you agree with. After applying, delete `.entire/REVIEW.md`. diff --git a/cmd/entire/cli/wingman_instruction.md b/cmd/entire/cli/wingman_instruction.md new file mode 100644 index 000000000..dfd23f701 --- /dev/null +++ b/cmd/entire/cli/wingman_instruction.md @@ -0,0 +1,3 @@ +A code review is available at `.entire/REVIEW.md`. + +Read `.entire/REVIEW.md` and address each suggestion you agree with. Be critical. When done, delete `.entire/REVIEW.md`. From b5c8d265d3eefce3c04c170e90d5f9a5187f0a30 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 19:11:22 +0100 Subject: [PATCH 07/32] Use branch-level diff for wingman reviews instead of single commit Diff against the merge base with main/master so the reviewer sees all branch changes holistically, not just the latest commit in isolation. Falls back to HEAD diff if no merge base is found (e.g., on main itself). Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 7c8a0f30fbef --- cmd/entire/cli/wingman_review.go | 90 ++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 39 deletions(-) diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 7e235cc6e..eccccfee1 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -202,55 +202,67 @@ func runWingmanReview(payloadPath string) error { return nil } -// computeDiff gets the code changes to review. baseCommit is the HEAD hash -// captured at trigger time so the diff is stable even if HEAD moves during -// the initial delay (e.g., auto-commit creates another commit, or user commits). -func computeDiff(repoRoot string, baseCommit string) (string, error) { - // Use the captured base commit when available for deterministic diffs - diffRef := "HEAD" - if baseCommit != "" { - diffRef = baseCommit - } - - // Try uncommitted changes against the base commit +// computeDiff gets the full branch diff for review. It diffs the current HEAD +// against the merge base with the default branch (main/master), giving the +// reviewer a holistic view of all branch changes rather than just one commit. +func computeDiff(repoRoot string, _ string) (string, error) { ctx, cancel := context.WithTimeout(context.Background(), wingmanGitTimeout) defer cancel() - cmd := exec.CommandContext(ctx, "git", "diff", diffRef) - cmd.Dir = repoRoot - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr + // Find the merge base with the default branch for a holistic branch diff. + mergeBase := findMergeBase(ctx, repoRoot) + if mergeBase != "" { + wingmanLog("using merge-base %s for branch diff", mergeBase) + diff, err := gitDiff(ctx, repoRoot, mergeBase) + if err == nil && strings.TrimSpace(diff) != "" { + return diff, nil + } + // Fall through to HEAD diff if merge-base diff fails or is empty + } - if err := cmd.Run(); err != nil { - // If the ref doesn't exist (initial commit), try diff of staged/unstaged - ctx2, cancel2 := context.WithTimeout(context.Background(), wingmanGitTimeout) - defer cancel2() - cmd2 := exec.CommandContext(ctx2, "git", "diff") - cmd2.Dir = repoRoot - var stdout2 bytes.Buffer - cmd2.Stdout = &stdout2 - if err2 := cmd2.Run(); err2 != nil { - return "", fmt.Errorf("git diff failed: %w (stderr: %s)", err, stderr.String()) + // Fallback: diff uncommitted changes against HEAD + wingmanLog("falling back to HEAD diff") + diff, err := gitDiff(ctx, repoRoot, "HEAD") + if err != nil { + return "", fmt.Errorf("git diff failed: %w", err) + } + + // If no uncommitted changes, try the latest commit itself + if strings.TrimSpace(diff) == "" { + diff, err = gitDiff(ctx, repoRoot, "HEAD~1", "HEAD") + if err != nil { + return "", nil //nolint:nilerr // no diff is not an error } - return stdout2.String(), nil } - // If no uncommitted changes, the commit itself may contain the changes - // (auto-commit strategy creates commits on the active branch) - if strings.TrimSpace(stdout.String()) == "" { - ctx2, cancel2 := context.WithTimeout(context.Background(), wingmanGitTimeout) - defer cancel2() - - cmd2 := exec.CommandContext(ctx2, "git", "diff", diffRef+"~1", diffRef) - cmd2.Dir = repoRoot - var stdout2 bytes.Buffer - cmd2.Stdout = &stdout2 - if err := cmd2.Run(); err == nil && strings.TrimSpace(stdout2.String()) != "" { - return stdout2.String(), nil + return diff, nil +} + +// findMergeBase returns the merge-base commit between HEAD and the default +// branch (tries main, then master). Returns empty string if not found. +func findMergeBase(ctx context.Context, repoRoot string) string { + for _, branch := range []string{"main", "master"} { + cmd := exec.CommandContext(ctx, "git", "merge-base", branch, "HEAD") + cmd.Dir = repoRoot + out, err := cmd.Output() + if err == nil { + return strings.TrimSpace(string(out)) } } + return "" +} +// gitDiff runs git diff with the given args and returns stdout. +func gitDiff(ctx context.Context, repoRoot string, args ...string) (string, error) { + fullArgs := append([]string{"diff"}, args...) + cmd := exec.CommandContext(ctx, "git", fullArgs...) //nolint:gosec // args are from internal logic + cmd.Dir = repoRoot + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("git diff %v: %w (stderr: %s)", args, err, stderr.String()) + } return stdout.String(), nil } From 6289e2b26b707b3fa79a793cdcadeac0c325d4c8 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Wed, 11 Feb 2026 19:47:19 +0100 Subject: [PATCH 08/32] Show wingman status in session start message Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 05a188c59540 --- cmd/entire/cli/hooks.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 363b528f1..885147db0 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -12,6 +12,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/strategy" ) @@ -265,6 +266,11 @@ func handleSessionStartCommon() error { // Build informational message message := "\n\nPowered by Entire:\n This conversation will be linked to your next commit." + // Append wingman note if enabled + if settings.IsWingmanEnabled() { + message += "\n Wingman is active: your changes will be automatically reviewed." + } + // Check for concurrent sessions and append count if any strat := GetStrategy() if concurrentChecker, ok := strat.(strategy.ConcurrentSessionChecker); ok { From 3dceabaad1fc3cf94b604efce9b5fa666c3980bf Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 09:31:15 +0100 Subject: [PATCH 09/32] Add reliable wingman auto-apply via stop hook and stale review cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wingman review was never being applied because: (1) stale REVIEW.md from previous sessions blocked new reviews indefinitely, and (2) the systemMessage injection on prompt-submit was unreliable since the agent deprioritized it vs the user's actual request. This adds stop-hook-based auto-apply as the primary delivery mechanism. When an agent turn ends (ACTIVE → IDLE), the stop hook checks for pending REVIEW.md and spawns a detached `entire wingman __apply` process that triggers `claude --continue`. Also adds session-aware stale review cleanup (different session, orphaned state, >1hr TTL) and retry prevention via ApplyAttemptedAt field. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: d4cbcc6d0339 --- cmd/entire/cli/hooks_claudecode_handlers.go | 31 ++- cmd/entire/cli/wingman.go | 83 +++++++- cmd/entire/cli/wingman_review.go | 75 +++++-- cmd/entire/cli/wingman_spawn_other.go | 5 + cmd/entire/cli/wingman_spawn_unix.go | 41 ++++ cmd/entire/cli/wingman_test.go | 208 ++++++++++++++++++++ 6 files changed, 416 insertions(+), 27 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 4fa490ac5..b0b7c3242 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -76,15 +76,22 @@ func captureInitialState() error { return err } - // If a wingman review is pending, inject instruction into the agent's context - // so it automatically reads and applies the suggestions. + // If a wingman review is pending for the CURRENT session and auto-apply + // hasn't been attempted yet, inject instruction as a belt-and-suspenders + // backup. The primary delivery mechanism is the stop hook auto-apply. if settings.IsWingmanEnabled() { repoRoot, rootErr := paths.RepoRoot() if rootErr == nil { if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { - fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") - if err := outputHookResponse(wingmanApplyInstruction); err != nil { - fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) + wingmanState := loadWingmanStateDirect(repoRoot) + shouldInject := wingmanState != nil && + wingmanState.SessionID == hookData.sessionID && + wingmanState.ApplyAttemptedAt == nil + if shouldInject { + fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") + if err := outputHookResponse(wingmanApplyInstruction); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) + } } } } @@ -415,6 +422,20 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba }) } + // Auto-apply pending wingman review: the stop hook fires when the agent turn + // ends (ACTIVE → IDLE), which is the natural safe moment to trigger --continue. + // This is the PRIMARY delivery mechanism for wingman reviews. + if settings.IsWingmanEnabled() && os.Getenv("ENTIRE_WINGMAN_APPLY") == "" { + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, statErr := os.Stat(reviewPath); statErr == nil { + wingmanState := loadWingmanStateDirect(repoRoot) + if wingmanState == nil || wingmanState.ApplyAttemptedAt == nil { + fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply\n") + spawnDetachedWingmanApply(repoRoot) + } + } + } + // Clean up pre-prompt state (CLI responsibility) if err := CleanupPrePromptState(sessionID); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to cleanup pre-prompt state: %v\n", err) diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index f98cb4d1c..b05fce55c 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -37,10 +37,11 @@ type WingmanPayload struct { // WingmanState tracks deduplication and review state. type WingmanState struct { - SessionID string `json:"session_id"` - FilesHash string `json:"files_hash"` - ReviewedAt time.Time `json:"reviewed_at"` - ReviewApplied bool `json:"review_applied"` + SessionID string `json:"session_id"` + FilesHash string `json:"files_hash"` + ReviewedAt time.Time `json:"reviewed_at"` + ReviewApplied bool `json:"review_applied"` + ApplyAttemptedAt *time.Time `json:"apply_attempted_at,omitempty"` } //go:embed wingman_instruction.md @@ -50,6 +51,10 @@ const ( wingmanStateFile = ".entire/wingman-state.json" wingmanReviewFile = ".entire/REVIEW.md" wingmanLockFile = ".entire/wingman.lock" + + // wingmanStaleReviewTTL is the maximum age of a pending REVIEW.md before + // it's considered stale and automatically cleaned up. + wingmanStaleReviewTTL = 1 * time.Hour ) func newWingmanCmd() *cobra.Command { @@ -70,6 +75,7 @@ to apply them.`, cmd.AddCommand(newWingmanDisableCmd()) cmd.AddCommand(newWingmanStatusCmd()) cmd.AddCommand(newWingmanReviewCmd()) + cmd.AddCommand(newWingmanApplyCmd()) return cmd } @@ -197,10 +203,9 @@ func triggerWingmanReview(payload WingmanPayload) { slog.Int("file_count", totalFiles), ) - // Check if a pending REVIEW.md already exists - reviewPath := filepath.Join(repoRoot, wingmanReviewFile) - if _, err := os.Stat(reviewPath); err == nil { - logging.Info(logCtx, "wingman skipped: pending review exists") + // Check if a pending REVIEW.md already exists for the current session + if shouldSkipPendingReview(repoRoot, payload.SessionID) { + logging.Info(logCtx, "wingman skipped: pending review exists for current session") fmt.Fprintf(os.Stderr, "[wingman] Pending review exists, skipping\n") return } @@ -438,3 +443,65 @@ func saveWingmanState(state *WingmanState) error { } return nil } + +// loadWingmanStateDirect reads the wingman state file directly from a known +// path under repoRoot. Returns nil if the file doesn't exist or can't be parsed. +func loadWingmanStateDirect(repoRoot string) *WingmanState { + statePath := filepath.Join(repoRoot, wingmanStateFile) + data, err := os.ReadFile(statePath) //nolint:gosec // path is repo-relative constant + if err != nil { + return nil + } + var state WingmanState + if err := json.Unmarshal(data, &state); err != nil { + return nil + } + return &state +} + +// newWingmanApplyCmd returns a hidden subcommand used by the stop hook to +// apply a pending REVIEW.md via claude --continue in a detached subprocess. +func newWingmanApplyCmd() *cobra.Command { + return &cobra.Command{ + Use: "__apply", + Hidden: true, + Args: cobra.ExactArgs(1), + RunE: func(_ *cobra.Command, args []string) error { + return runWingmanApply(args[0]) + }, + } +} + +// shouldSkipPendingReview checks whether a pending REVIEW.md should prevent +// a new review from being triggered. It cleans up stale/orphaned reviews. +// +// Returns true only when REVIEW.md exists AND belongs to the current session +// AND is younger than wingmanStaleReviewTTL. +func shouldSkipPendingReview(repoRoot, currentSessionID string) bool { + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, err := os.Stat(reviewPath); err != nil { + return false // No REVIEW.md, don't skip + } + + // REVIEW.md exists — check state to determine if it's current or stale + state := loadWingmanStateDirect(repoRoot) + if state == nil { + // Orphan: REVIEW.md without state file — clean up + _ = os.Remove(reviewPath) + return false + } + + if state.SessionID != currentSessionID { + // Different session — stale review, clean up + _ = os.Remove(reviewPath) + return false + } + + if time.Since(state.ReviewedAt) > wingmanStaleReviewTTL { + // Same session but too old — clean up + _ = os.Remove(reviewPath) + return false + } + + return true // Current session, fresh review — skip +} diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index eccccfee1..cc72338ed 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -29,10 +29,6 @@ const ( // letting the agent turn fully settle. wingmanInitialDelay = 10 * time.Second - // wingmanApplyDelay is how long to wait after writing REVIEW.md - // before attempting to auto-apply. - wingmanApplyDelay = 30 * time.Second - // wingmanReviewModel is the Claude model used for reviews. wingmanReviewModel = "sonnet" @@ -178,20 +174,26 @@ func runWingmanReview(payloadPath string) error { }) wingmanLog("dedup state saved") - // Auto-apply phase: wait then check if session is idle - wingmanLog("waiting %s before auto-apply check", wingmanApplyDelay) - time.Sleep(wingmanApplyDelay) - + // Check if session is idle right now (rare — user usually starts typing + // during the 10s settle + review time). If idle, apply immediately. + // Otherwise, the stop hook will handle auto-apply when the current turn ends. idle := isSessionIdle(payload.SessionID) wingmanLog("session idle check: idle=%v", idle) if !idle { - wingmanLog("session is active, leaving REVIEW.md for notification") - return nil // User is active, leave REVIEW.md for notification + wingmanLog("session is active, stop hook will handle auto-apply when turn ends") + return nil } - // Trigger auto-apply via claude --continue - wingmanLog("triggering auto-apply via claude --continue") + // Mark apply as attempted before triggering (retry prevention) + now := time.Now() + state := loadWingmanStateDirect(repoRoot) + if state != nil { + state.ApplyAttemptedAt = &now + saveWingmanStateDirect(repoRoot, state) + } + + wingmanLog("triggering auto-apply via claude --continue (session idle)") applyStart := time.Now() if err := triggerAutoApply(repoRoot); err != nil { wingmanLog("ERROR auto-apply failed after %s: %v", time.Since(applyStart).Round(time.Millisecond), err) @@ -231,7 +233,7 @@ func computeDiff(repoRoot string, _ string) (string, error) { if strings.TrimSpace(diff) == "" { diff, err = gitDiff(ctx, repoRoot, "HEAD~1", "HEAD") if err != nil { - return "", nil //nolint:nilerr // no diff is not an error + return "", nil } } @@ -242,7 +244,7 @@ func computeDiff(repoRoot string, _ string) (string, error) { // branch (tries main, then master). Returns empty string if not found. func findMergeBase(ctx context.Context, repoRoot string) string { for _, branch := range []string{"main", "master"} { - cmd := exec.CommandContext(ctx, "git", "merge-base", branch, "HEAD") + cmd := exec.CommandContext(ctx, "git", "merge-base", branch, "HEAD") //nolint:gosec // branch is from hardcoded slice cmd.Dir = repoRoot out, err := cmd.Output() if err == nil { @@ -340,6 +342,51 @@ func isSessionIdle(sessionID string) bool { return state.Phase == session.PhaseIdle } +// runWingmanApply is the entrypoint for the __apply subcommand, spawned by the +// stop hook when a pending REVIEW.md is detected. It re-checks preconditions +// and triggers claude --continue to apply the review. +func runWingmanApply(repoRoot string) error { + wingmanLog("apply process started (pid=%d)", os.Getpid()) + + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if !fileExists(reviewPath) { + wingmanLog("no REVIEW.md found, nothing to apply") + return nil + } + + // Retry prevention: check if apply was already attempted for this review + state := loadWingmanStateDirect(repoRoot) + if state != nil && state.ApplyAttemptedAt != nil { + wingmanLog("apply already attempted at %s, skipping", state.ApplyAttemptedAt.Format(time.RFC3339)) + return nil + } + + // Re-check session is still idle (user may have typed during spawn delay) + if state != nil && state.SessionID != "" { + if !isSessionIdle(state.SessionID) { + wingmanLog("session became active during spawn, aborting (next stop hook will retry)") + return nil + } + } + + // Mark apply as attempted BEFORE triggering + if state != nil { + now := time.Now() + state.ApplyAttemptedAt = &now + saveWingmanStateDirect(repoRoot, state) + } + + wingmanLog("triggering auto-apply via claude --continue") + applyStart := time.Now() + if err := triggerAutoApply(repoRoot); err != nil { + wingmanLog("ERROR auto-apply failed after %s: %v", time.Since(applyStart).Round(time.Millisecond), err) + return fmt.Errorf("failed to trigger auto-apply: %w", err) + } + wingmanLog("auto-apply completed in %s", time.Since(applyStart).Round(time.Millisecond)) + + return nil +} + // triggerAutoApply spawns claude --continue to apply the review suggestions. func triggerAutoApply(repoRoot string) error { ctx, cancel := context.WithTimeout(context.Background(), wingmanApplyTimeout) diff --git a/cmd/entire/cli/wingman_spawn_other.go b/cmd/entire/cli/wingman_spawn_other.go index 1be15ade6..efc804462 100644 --- a/cmd/entire/cli/wingman_spawn_other.go +++ b/cmd/entire/cli/wingman_spawn_other.go @@ -9,3 +9,8 @@ package cli func spawnDetachedWingmanReview(_, _ string) { // No-op: detached subprocess spawning not implemented for this platform } + +// spawnDetachedWingmanApply is a no-op on non-Unix platforms. +func spawnDetachedWingmanApply(_ string) { + // No-op: detached subprocess spawning not implemented for this platform +} diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index a39e08183..6a34afb75 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -62,3 +62,44 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() } + +// spawnDetachedWingmanApply spawns a detached subprocess to auto-apply the +// pending REVIEW.md via claude --continue. Called from the stop hook when a +// review is pending after the agent turn ends. +func spawnDetachedWingmanApply(repoRoot string) { + executable, err := os.Executable() + if err != nil { + return + } + + //nolint:gosec // G204: repoRoot is from paths.RepoRoot(), not user input + cmd := exec.CommandContext(context.Background(), executable, "wingman", "__apply", repoRoot) + + cmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + } + + cmd.Dir = "/" + cmd.Env = os.Environ() + + cmd.Stdout = io.Discard + logDir := filepath.Join(repoRoot, ".entire", "logs") + if mkErr := os.MkdirAll(logDir, 0o750); mkErr == nil { + //nolint:gosec // G304: path is constructed from repoRoot + constants + if f, openErr := os.OpenFile(filepath.Join(logDir, "wingman.log"), + os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600); openErr == nil { + cmd.Stderr = f + } else { + cmd.Stderr = io.Discard + } + } else { + cmd.Stderr = io.Discard + } + + if err := cmd.Start(); err != nil { + return + } + + //nolint:errcheck // Best effort - process should continue regardless + _ = cmd.Process.Release() +} diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 491644702..137fa3a7d 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestComputeFilesHash_Deterministic(t *testing.T) { @@ -306,3 +307,210 @@ func TestIsWingmanEnabled_Settings(t *testing.T) { }) } } + +func TestWingmanState_ApplyAttemptedAt_RoundTrip(t *testing.T) { + t.Parallel() + + now := time.Now().Truncate(time.Second) + state := &WingmanState{ + SessionID: "sess-1", + FilesHash: "hash1", + ReviewedAt: now, + ReviewApplied: false, + ApplyAttemptedAt: &now, + } + + data, err := json.Marshal(state) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var decoded WingmanState + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + if decoded.ApplyAttemptedAt == nil { + t.Fatal("ApplyAttemptedAt should not be nil after round-trip") + } + if !decoded.ApplyAttemptedAt.Truncate(time.Second).Equal(now) { + t.Errorf("ApplyAttemptedAt: got %v, want %v", decoded.ApplyAttemptedAt, now) + } +} + +func TestWingmanState_ApplyAttemptedAt_OmitEmpty(t *testing.T) { + t.Parallel() + + state := &WingmanState{ + SessionID: "sess-1", + FilesHash: "hash1", + } + + data, err := json.Marshal(state) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + if strings.Contains(string(data), "apply_attempted_at") { + t.Error("ApplyAttemptedAt should be omitted when nil") + } +} + +func TestLoadWingmanStateDirect(t *testing.T) { + t.Parallel() + + t.Run("missing file returns nil", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + got := loadWingmanStateDirect(tmpDir) + if got != nil { + t.Errorf("expected nil for missing file, got %+v", got) + } + }) + + t.Run("valid file returns state", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + now := time.Now() + stateJSON := `{"session_id":"sess-1","files_hash":"hash1","reviewed_at":"` + now.Format(time.RFC3339Nano) + `","review_applied":false}` + if err := os.WriteFile(filepath.Join(entireDir, "wingman-state.json"), []byte(stateJSON), 0o644); err != nil { + t.Fatal(err) + } + + got := loadWingmanStateDirect(tmpDir) + if got == nil { + t.Fatal("expected non-nil state") + } + if got.SessionID != "sess-1" { + t.Errorf("SessionID: got %q, want %q", got.SessionID, "sess-1") + } + }) +} + +func TestShouldSkipPendingReview_NoReviewFile(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + if err := os.MkdirAll(filepath.Join(tmpDir, ".entire"), 0o755); err != nil { + t.Fatal(err) + } + + if shouldSkipPendingReview(tmpDir, "sess-1") { + t.Error("should not skip when no REVIEW.md exists") + } +} + +func TestShouldSkipPendingReview_SameSession(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Write REVIEW.md + if err := os.WriteFile(filepath.Join(entireDir, "REVIEW.md"), []byte("review"), 0o644); err != nil { + t.Fatal(err) + } + + // Write state with same session + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-1", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + if !shouldSkipPendingReview(tmpDir, "sess-1") { + t.Error("should skip when same session has fresh review") + } +} + +func TestShouldSkipPendingReview_DifferentSession(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + reviewPath := filepath.Join(entireDir, "REVIEW.md") + if err := os.WriteFile(reviewPath, []byte("stale review"), 0o644); err != nil { + t.Fatal(err) + } + + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "old-session", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + if shouldSkipPendingReview(tmpDir, "new-session") { + t.Error("should not skip when review is from different session") + } + + // REVIEW.md should have been cleaned up + if _, err := os.Stat(reviewPath); err == nil { + t.Error("stale REVIEW.md should have been deleted") + } +} + +func TestShouldSkipPendingReview_StaleTTL(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + reviewPath := filepath.Join(entireDir, "REVIEW.md") + if err := os.WriteFile(reviewPath, []byte("old review"), 0o644); err != nil { + t.Fatal(err) + } + + // State with same session but old ReviewedAt + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-1", + FilesHash: "hash1", + ReviewedAt: time.Now().Add(-2 * time.Hour), // 2 hours old + }) + + if shouldSkipPendingReview(tmpDir, "sess-1") { + t.Error("should not skip when review is older than TTL") + } + + if _, err := os.Stat(reviewPath); err == nil { + t.Error("stale REVIEW.md should have been deleted") + } +} + +func TestShouldSkipPendingReview_OrphanNoState(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + reviewPath := filepath.Join(entireDir, "REVIEW.md") + if err := os.WriteFile(reviewPath, []byte("orphan review"), 0o644); err != nil { + t.Fatal(err) + } + + // No state file + if shouldSkipPendingReview(tmpDir, "sess-1") { + t.Error("should not skip when no state file exists (orphan)") + } + + if _, err := os.Stat(reviewPath); err == nil { + t.Error("orphan REVIEW.md should have been deleted") + } +} From 7dcd9430a868c22df9d3f1f7171c2721e7c07ee9 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 10:32:08 +0100 Subject: [PATCH 10/32] Fix wingman auto-apply not triggering on no-changes turn end The auto-apply trigger was only in the code path where the turn had file changes. When the agent turn ended without modifications (e.g. answering a question), commitWithMetadata returned early and never reached the auto-apply check. Extract triggerWingmanAutoApplyIfPending helper and call it from both the no-changes early return and the main path. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 2695fc81fb3d --- cmd/entire/cli/hooks_claudecode_handlers.go | 36 +++++++++++++-------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index b0b7c3242..2c090aa38 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -279,6 +279,8 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba fmt.Fprintf(os.Stderr, "Skipping commit\n") // Still transition phase even when skipping commit — the turn is ending. transitionSessionTurnEnd(sessionID) + // Auto-apply pending wingman review even when no file changes this turn + triggerWingmanAutoApplyIfPending(repoRoot) // Clean up state even when skipping if err := CleanupPrePromptState(sessionID); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to cleanup pre-prompt state: %v\n", err) @@ -422,19 +424,8 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba }) } - // Auto-apply pending wingman review: the stop hook fires when the agent turn - // ends (ACTIVE → IDLE), which is the natural safe moment to trigger --continue. - // This is the PRIMARY delivery mechanism for wingman reviews. - if settings.IsWingmanEnabled() && os.Getenv("ENTIRE_WINGMAN_APPLY") == "" { - reviewPath := filepath.Join(repoRoot, wingmanReviewFile) - if _, statErr := os.Stat(reviewPath); statErr == nil { - wingmanState := loadWingmanStateDirect(repoRoot) - if wingmanState == nil || wingmanState.ApplyAttemptedAt == nil { - fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply\n") - spawnDetachedWingmanApply(repoRoot) - } - } - } + // Auto-apply pending wingman review on turn end + triggerWingmanAutoApplyIfPending(repoRoot) // Clean up pre-prompt state (CLI responsibility) if err := CleanupPrePromptState(sessionID); err != nil { @@ -821,6 +812,25 @@ func transitionSessionTurnEnd(sessionID string) { } } +// triggerWingmanAutoApplyIfPending checks for a pending REVIEW.md and spawns +// the auto-apply subprocess if conditions are met. Called from the stop hook +// on every turn end (both with-changes and no-changes paths). +func triggerWingmanAutoApplyIfPending(repoRoot string) { + if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + return + } + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, statErr := os.Stat(reviewPath); statErr != nil { + return + } + wingmanState := loadWingmanStateDirect(repoRoot) + if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { + return + } + fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply\n") + spawnDetachedWingmanApply(repoRoot) +} + // markSessionEnded transitions the session to ENDED phase via the state machine. func markSessionEnded(sessionID string) error { state, err := strategy.LoadSessionState(sessionID) From 9ff20c86cf0a87de47d85a3cdc872677d62e7245 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 10:37:40 +0100 Subject: [PATCH 11/32] Add structured logging for wingman review lifecycle Add logging to entire.log for key wingman events: review in progress indicator on prompt-submit, auto-apply spawn, instruction injection, and review-already-running detection. This gives better visibility into the wingman review lifecycle beyond just wingman.log in the detached process. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 63095cc2fdae --- cmd/entire/cli/hooks_claudecode_handlers.go | 20 ++++++++++++++++++++ cmd/entire/cli/wingman.go | 3 ++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 2c090aa38..f2e6f4e27 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -82,6 +82,7 @@ func captureInitialState() error { if settings.IsWingmanEnabled() { repoRoot, rootErr := paths.RepoRoot() if rootErr == nil { + wingmanLogCtx := logging.WithComponent(context.Background(), "wingman") if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { wingmanState := loadWingmanStateDirect(repoRoot) shouldInject := wingmanState != nil && @@ -89,11 +90,25 @@ func captureInitialState() error { wingmanState.ApplyAttemptedAt == nil if shouldInject { fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") + logging.Info(wingmanLogCtx, "wingman injecting review instruction on prompt-submit", + slog.String("session_id", hookData.sessionID), + ) if err := outputHookResponse(wingmanApplyInstruction); err != nil { fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) } + } else if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { + logging.Debug(wingmanLogCtx, "wingman review pending but auto-apply already attempted, skipping injection") } } + + // Log if a review is currently in progress (lock file exists) + lockPath := filepath.Join(repoRoot, wingmanLockFile) + if _, statErr := os.Stat(lockPath); statErr == nil { + fmt.Fprintf(os.Stderr, "[wingman] Review in progress...\n") + logging.Info(wingmanLogCtx, "wingman review in progress", + slog.String("session_id", hookData.sessionID), + ) + } } } @@ -819,15 +834,20 @@ func triggerWingmanAutoApplyIfPending(repoRoot string) { if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { return } + logCtx := logging.WithComponent(context.Background(), "wingman") reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if _, statErr := os.Stat(reviewPath); statErr != nil { return } wingmanState := loadWingmanStateDirect(repoRoot) if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { + logging.Debug(logCtx, "wingman auto-apply already attempted, skipping") return } fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply\n") + logging.Info(logCtx, "wingman auto-apply spawning", + slog.String("review_path", reviewPath), + ) spawnDetachedWingmanApply(repoRoot) } diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index b05fce55c..c953a9a4f 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -214,7 +214,8 @@ func triggerWingmanReview(payload WingmanPayload) { // is atomic on all platforms, avoiding the TOCTOU race of Stat+WriteFile. lockPath := filepath.Join(repoRoot, wingmanLockFile) if !acquireWingmanLock(lockPath, payload.SessionID) { - logging.Info(logCtx, "wingman skipped: could not acquire lock") + logging.Info(logCtx, "wingman skipped: review already in progress") + fmt.Fprintf(os.Stderr, "[wingman] Review in progress, skipping\n") return } From 5913a8e1443dcbcd378f5689c530faa467ea5d9e Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 10:48:09 +0100 Subject: [PATCH 12/32] Fix isSessionIdle failing in detached wingman subprocesses Two bugs prevented auto-apply from triggering: 1. The detached review/apply processes run with cwd=/ so git commands in strategy.LoadSessionState (git rev-parse --git-common-dir) failed, causing isSessionIdle to always return false. Fix: read session state files directly using repoRoot to locate .git/entire-sessions/. 2. The session ID in the wingman payload could be from an ended session (user closed and reopened Claude). isSessionIdle only checked that specific session. Fix: fall back to checking ALL sessions, so if any session is idle the review gets applied. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 9f7e019204ae --- cmd/entire/cli/wingman_review.go | 103 ++++++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index cc72338ed..ef281fdc9 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -13,7 +13,6 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/session" - "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/spf13/cobra" ) @@ -177,7 +176,7 @@ func runWingmanReview(payloadPath string) error { // Check if session is idle right now (rare — user usually starts typing // during the 10s settle + review time). If idle, apply immediately. // Otherwise, the stop hook will handle auto-apply when the current turn ends. - idle := isSessionIdle(payload.SessionID) + idle := isSessionIdle(repoRoot, payload.SessionID) wingmanLog("session idle check: idle=%v", idle) if !idle { @@ -333,13 +332,101 @@ func readSessionContext(repoRoot, sessionID string) string { return string(data) } -// isSessionIdle checks if the given session is in the IDLE phase. -func isSessionIdle(sessionID string) bool { - state, err := strategy.LoadSessionState(sessionID) - if err != nil || state == nil { +// isSessionIdle checks if any session is in the IDLE phase by reading +// session state files directly from the repo. Uses repoRoot to locate +// .git/entire-sessions/ without relying on the current working directory +// (which is "/" in detached subprocesses). +func isSessionIdle(repoRoot, sessionID string) bool { + sessDir := findSessionStateDir(repoRoot) + if sessDir == "" { + wingmanLog("cannot find session state dir for %s", repoRoot) return false } - return state.Phase == session.PhaseIdle + + // Fast path: check the specific session + if sessionID != "" { + if phase := readSessionPhase(sessDir, sessionID); phase == string(session.PhaseIdle) { + return true + } + } + + // Slow path: check if ANY session is idle + // (covers session restart, ended sessions, etc.) + entries, err := os.ReadDir(sessDir) + if err != nil { + return false + } + for _, entry := range entries { + name := entry.Name() + if !strings.HasSuffix(name, ".json") { + continue + } + sid := strings.TrimSuffix(name, ".json") + if sid == sessionID { + continue // Already checked + } + if phase := readSessionPhase(sessDir, sid); phase == string(session.PhaseIdle) { + wingmanLog("found idle session %s (original session %s was not idle)", sid, sessionID) + return true + } + } + + return false +} + +// findSessionStateDir locates the .git/entire-sessions/ directory by +// reading .git to handle both normal repos and worktrees. +func findSessionStateDir(repoRoot string) string { + gitPath := filepath.Join(repoRoot, ".git") + info, err := os.Stat(gitPath) + if err != nil { + return "" + } + + var gitDir string + if info.IsDir() { + // Normal repo: .git is a directory + gitDir = gitPath + } else { + // Worktree: .git is a file containing "gitdir: " + data, readErr := os.ReadFile(gitPath) //nolint:gosec // path from repoRoot + if readErr != nil { + return "" + } + content := strings.TrimSpace(string(data)) + if !strings.HasPrefix(content, "gitdir: ") { + return "" + } + gitDir = strings.TrimPrefix(content, "gitdir: ") + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(repoRoot, gitDir) + } + // For worktrees, session state is in the common dir + // .git/worktrees/ → ../../ is the common .git dir + commonDir := filepath.Join(gitDir, "..", "..") + gitDir = filepath.Clean(commonDir) + } + + sessDir := filepath.Join(gitDir, "entire-sessions") + if _, statErr := os.Stat(sessDir); statErr != nil { + return "" + } + return sessDir +} + +// readSessionPhase reads just the phase field from a session state JSON file. +func readSessionPhase(sessDir, sessionID string) string { + data, err := os.ReadFile(filepath.Join(sessDir, sessionID+".json")) //nolint:gosec // sessDir is from git internals + if err != nil { + return "" + } + var partial struct { + Phase string `json:"phase"` + } + if json.Unmarshal(data, &partial) != nil { + return "" + } + return partial.Phase } // runWingmanApply is the entrypoint for the __apply subcommand, spawned by the @@ -363,7 +450,7 @@ func runWingmanApply(repoRoot string) error { // Re-check session is still idle (user may have typed during spawn delay) if state != nil && state.SessionID != "" { - if !isSessionIdle(state.SessionID) { + if !isSessionIdle(repoRoot, state.SessionID) { wingmanLog("session became active during spawn, aborting (next stop hook will retry)") return nil } From 18accf72f0ef4ba27fb4274b990ea6c6da36dcfe Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 11:53:34 +0100 Subject: [PATCH 13/32] Use additionalContext for wingman review injection to ensure it's addressed first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the review instruction was injected as systemMessage (a warning) which the agent could deprioritize vs the user's direct request. Now uses additionalContext which is added to Claude's context as mandatory instructions. Also strengthened the instruction text to explicitly require the review be addressed BEFORE the user's request, and simplified the injection logic — inject whenever REVIEW.md exists regardless of session ID or apply-attempted state (the file's existence is the source of truth). Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 45a53baac1f3 --- cmd/entire/cli/hooks.go | 16 ++++++++++++- cmd/entire/cli/hooks_claudecode_handlers.go | 26 +++++++-------------- cmd/entire/cli/wingman_instruction.md | 10 ++++++-- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 885147db0..4bcab386e 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -302,7 +302,8 @@ func handleSessionStartCommon() error { // hookResponse represents a JSON response. // Used to control whether Agent continues processing the prompt. type hookResponse struct { - SystemMessage string `json:"systemMessage,omitempty"` + SystemMessage string `json:"systemMessage,omitempty"` + AdditionalContext string `json:"additionalContext,omitempty"` } // outputHookResponse outputs a JSON response to stdout @@ -315,3 +316,16 @@ func outputHookResponse(reason string) error { } return nil } + +// outputHookResponseWithContext outputs a JSON response that adds context to +// the agent's conversation. additionalContext is injected into Claude's context +// and treated as required instructions, not just a warning. +func outputHookResponseWithContext(context string) error { + resp := hookResponse{ + AdditionalContext: context, + } + if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { + return fmt.Errorf("failed to encode hook response: %w", err) + } + return nil +} diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index f2e6f4e27..22906fbdc 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -76,28 +76,20 @@ func captureInitialState() error { return err } - // If a wingman review is pending for the CURRENT session and auto-apply - // hasn't been attempted yet, inject instruction as a belt-and-suspenders - // backup. The primary delivery mechanism is the stop hook auto-apply. + // If a wingman review is pending, inject it as additionalContext so the + // agent addresses it BEFORE the user's request. This is the primary + // delivery mechanism — the agent sees the instruction as mandatory context. if settings.IsWingmanEnabled() { repoRoot, rootErr := paths.RepoRoot() if rootErr == nil { wingmanLogCtx := logging.WithComponent(context.Background(), "wingman") if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { - wingmanState := loadWingmanStateDirect(repoRoot) - shouldInject := wingmanState != nil && - wingmanState.SessionID == hookData.sessionID && - wingmanState.ApplyAttemptedAt == nil - if shouldInject { - fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md\n") - logging.Info(wingmanLogCtx, "wingman injecting review instruction on prompt-submit", - slog.String("session_id", hookData.sessionID), - ) - if err := outputHookResponse(wingmanApplyInstruction); err != nil { - fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) - } - } else if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { - logging.Debug(wingmanLogCtx, "wingman review pending but auto-apply already attempted, skipping injection") + fmt.Fprintf(os.Stderr, "[wingman] Review available: .entire/REVIEW.md — injecting into context\n") + logging.Info(wingmanLogCtx, "wingman injecting review instruction on prompt-submit", + slog.String("session_id", hookData.sessionID), + ) + if err := outputHookResponseWithContext(wingmanApplyInstruction); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) } } diff --git a/cmd/entire/cli/wingman_instruction.md b/cmd/entire/cli/wingman_instruction.md index dfd23f701..d960c5e66 100644 --- a/cmd/entire/cli/wingman_instruction.md +++ b/cmd/entire/cli/wingman_instruction.md @@ -1,3 +1,9 @@ -A code review is available at `.entire/REVIEW.md`. +IMPORTANT — MANDATORY PRE-STEP: A code review has been generated at `.entire/REVIEW.md`. You MUST address it BEFORE responding to the user's request. -Read `.entire/REVIEW.md` and address each suggestion you agree with. Be critical. When done, delete `.entire/REVIEW.md`. +Steps (do these FIRST, before anything else): +1. Read `.entire/REVIEW.md` +2. Address each suggestion you agree with — be critical, skip anything you disagree with +3. Delete `.entire/REVIEW.md` when done +4. Briefly tell the user what you changed from the review + +Then proceed with the user's request. From 6627812af61362cca96e53c8ce7e26a17ab62f83 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 12:37:00 +0100 Subject: [PATCH 14/32] Prefer visible review delivery and add session-end auto-apply trigger When a live session exists, defer wingman review delivery to the prompt-submit injection (visible in terminal) instead of running a background auto-apply that the user can't see. Background auto-apply is now only used when no sessions are alive. Also triggers auto-apply from the SessionEnd hook when the last session closes with a pending review. Adds comprehensive wingman lifecycle documentation. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: f25c3e798f10 --- cmd/entire/cli/hooks_claudecode_handlers.go | 53 ++++- cmd/entire/cli/hooks_geminicli_handlers.go | 5 + cmd/entire/cli/wingman_review.go | 70 ++++-- cmd/entire/cli/wingman_test.go | 103 +++++++++ docs/architecture/wingman.md | 226 ++++++++++++++++++++ 5 files changed, 443 insertions(+), 14 deletions(-) create mode 100644 docs/architecture/wingman.md diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 22906fbdc..a149085e6 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -787,6 +787,12 @@ func handleClaudeCodeSessionEnd() error { if err := markSessionEnded(input.SessionID); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to mark session ended: %v\n", err) } + + // If a wingman review is pending and this was the last live session, + // trigger background auto-apply. No more prompts will come from this + // session, so the prompt-submit injection can't deliver the review. + triggerWingmanAutoApplyOnSessionEnd() + return nil } @@ -822,6 +828,10 @@ func transitionSessionTurnEnd(sessionID string) { // triggerWingmanAutoApplyIfPending checks for a pending REVIEW.md and spawns // the auto-apply subprocess if conditions are met. Called from the stop hook // on every turn end (both with-changes and no-changes paths). +// +// When a live session exists, this is a no-op: the prompt-submit injection +// will deliver the review visibly in the user's terminal instead. Background +// auto-apply is only used when no sessions are alive (all ended). func triggerWingmanAutoApplyIfPending(repoRoot string) { if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { return @@ -836,8 +846,47 @@ func triggerWingmanAutoApplyIfPending(repoRoot string) { logging.Debug(logCtx, "wingman auto-apply already attempted, skipping") return } - fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply\n") - logging.Info(logCtx, "wingman auto-apply spawning", + // Don't spawn background auto-apply if a live session exists. + // The prompt-submit hook will inject REVIEW.md as additionalContext, + // which is visible to the user in their terminal. + if hasAnyLiveSession(repoRoot) { + logging.Debug(logCtx, "wingman auto-apply deferred: live session will handle via injection") + fmt.Fprintf(os.Stderr, "[wingman] Review pending — will be injected on next prompt\n") + return + } + fmt.Fprintf(os.Stderr, "[wingman] Pending review found, spawning auto-apply (no live sessions)\n") + logging.Info(logCtx, "wingman auto-apply spawning (no live sessions)", + slog.String("review_path", reviewPath), + ) + spawnDetachedWingmanApply(repoRoot) +} + +// triggerWingmanAutoApplyOnSessionEnd checks for a pending REVIEW.md after a +// session ends. If no live sessions remain, spawns background auto-apply. +// Called from both Claude Code and Gemini CLI SessionEnd hooks. +func triggerWingmanAutoApplyOnSessionEnd() { + if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + return + } + repoRoot, err := paths.RepoRoot() + if err != nil { + return + } + logCtx := logging.WithComponent(context.Background(), "wingman") + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, statErr := os.Stat(reviewPath); statErr != nil { + return + } + wingmanState := loadWingmanStateDirect(repoRoot) + if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { + return + } + if hasAnyLiveSession(repoRoot) { + logging.Debug(logCtx, "wingman auto-apply deferred on session-end: other live sessions remain") + return + } + fmt.Fprintf(os.Stderr, "[wingman] Last session ended with pending review, spawning auto-apply\n") + logging.Info(logCtx, "wingman auto-apply on session-end (no live sessions remain)", slog.String("review_path", reviewPath), ) spawnDetachedWingmanApply(repoRoot) diff --git a/cmd/entire/cli/hooks_geminicli_handlers.go b/cmd/entire/cli/hooks_geminicli_handlers.go index 1e2535244..da1bc9c54 100644 --- a/cmd/entire/cli/hooks_geminicli_handlers.go +++ b/cmd/entire/cli/hooks_geminicli_handlers.go @@ -44,6 +44,11 @@ func handleGeminiSessionEnd() error { // Parse stdin once upfront — all subseq fmt.Fprintf(os.Stderr, "Warning: failed to mark session ended: %v\n", err) } + // If a wingman review is pending and this was the last live session, + // trigger background auto-apply. No more prompts will come from this + // session, so the prompt-submit injection can't deliver the review. + triggerWingmanAutoApplyOnSessionEnd() + return nil } diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index ef281fdc9..30e62ef83 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -89,8 +89,11 @@ func runWingmanReview(payloadPath string) error { // Clean up lock file when review completes (regardless of success/failure) lockPath := filepath.Join(repoRoot, wingmanLockFile) defer func() { - _ = os.Remove(lockPath) - wingmanLog("lock file removed") + if err := os.Remove(lockPath); err != nil && !errors.Is(err, os.ErrNotExist) { + wingmanLog("WARNING: failed to remove lock file: %v", err) + } else { + wingmanLog("lock file removed") + } }() // Initial delay: let the agent turn fully settle @@ -173,18 +176,18 @@ func runWingmanReview(payloadPath string) error { }) wingmanLog("dedup state saved") - // Check if session is idle right now (rare — user usually starts typing - // during the 10s settle + review time). If idle, apply immediately. - // Otherwise, the stop hook will handle auto-apply when the current turn ends. - idle := isSessionIdle(repoRoot, payload.SessionID) - wingmanLog("session idle check: idle=%v", idle) - - if !idle { - wingmanLog("session is active, stop hook will handle auto-apply when turn ends") + // If any session is live (IDLE/ACTIVE/ACTIVE_COMMITTED), don't auto-apply + // in the background. The prompt-submit hook will inject REVIEW.md as + // additionalContext when the user sends their next prompt — this is VISIBLE + // in their terminal. Only use background auto-apply when no sessions are + // alive (e.g., user closed all sessions). + if hasAnyLiveSession(repoRoot) { + wingmanLog("live session detected, deferring to prompt-submit injection (visible)") return nil } - // Mark apply as attempted before triggering (retry prevention) + // No live sessions — apply in background as fallback + wingmanLog("no live sessions, triggering background auto-apply") now := time.Now() state := loadWingmanStateDirect(repoRoot) if state != nil { @@ -192,7 +195,6 @@ func runWingmanReview(payloadPath string) error { saveWingmanStateDirect(repoRoot, state) } - wingmanLog("triggering auto-apply via claude --continue (session idle)") applyStart := time.Now() if err := triggerAutoApply(repoRoot); err != nil { wingmanLog("ERROR auto-apply failed after %s: %v", time.Since(applyStart).Round(time.Millisecond), err) @@ -356,7 +358,13 @@ func isSessionIdle(repoRoot, sessionID string) bool { if err != nil { return false } + const maxSessionCheck = 50 + checked := 0 for _, entry := range entries { + if checked >= maxSessionCheck { + wingmanLog("hit session check limit (%d), assuming not idle", maxSessionCheck) + return false + } name := entry.Name() if !strings.HasSuffix(name, ".json") { continue @@ -365,6 +373,7 @@ func isSessionIdle(repoRoot, sessionID string) bool { if sid == sessionID { continue // Already checked } + checked++ if phase := readSessionPhase(sessDir, sid); phase == string(session.PhaseIdle) { wingmanLog("found idle session %s (original session %s was not idle)", sid, sessionID) return true @@ -374,6 +383,43 @@ func isSessionIdle(repoRoot, sessionID string) bool { return false } +// hasAnyLiveSession checks if any session is in a non-ENDED phase (IDLE, +// ACTIVE, or ACTIVE_COMMITTED). Used to decide whether to defer review +// application to the prompt-submit injection (visible to user) vs background +// auto-apply (invisible). +func hasAnyLiveSession(repoRoot string) bool { + sessDir := findSessionStateDir(repoRoot) + if sessDir == "" { + return false + } + + entries, err := os.ReadDir(sessDir) + if err != nil { + return false + } + + const maxCheck = 50 + checked := 0 + for _, entry := range entries { + if checked >= maxCheck { + return false + } + name := entry.Name() + if !strings.HasSuffix(name, ".json") { + continue + } + checked++ + sid := strings.TrimSuffix(name, ".json") + phase := readSessionPhase(sessDir, sid) + if phase != "" && phase != string(session.PhaseEnded) { + wingmanLog("found live session %s (phase=%s)", sid, phase) + return true + } + } + + return false +} + // findSessionStateDir locates the .git/entire-sessions/ directory by // reading .git to handle both normal repos and worktrees. func findSessionStateDir(repoRoot string) string { diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 137fa3a7d..fcea73996 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -491,6 +491,109 @@ func TestShouldSkipPendingReview_StaleTTL(t *testing.T) { } } +func TestHasAnyLiveSession_NoSessionDir(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + // No .git at all + if hasAnyLiveSession(tmpDir) { + t.Error("should return false with no .git directory") + } +} + +func TestHasAnyLiveSession_EmptySessionDir(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + gitDir := filepath.Join(tmpDir, ".git") + if err := os.MkdirAll(filepath.Join(gitDir, "entire-sessions"), 0o755); err != nil { + t.Fatal(err) + } + + if hasAnyLiveSession(tmpDir) { + t.Error("should return false with empty session dir") + } +} + +func TestHasAnyLiveSession_IdleSession(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create an IDLE session + if err := os.WriteFile(filepath.Join(sessDir, "sess-idle.json"), []byte(`{"phase":"idle"}`), 0o644); err != nil { + t.Fatal(err) + } + + if !hasAnyLiveSession(tmpDir) { + t.Error("should return true when IDLE session exists") + } +} + +func TestHasAnyLiveSession_ActiveSession(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(sessDir, "sess-active.json"), []byte(`{"phase":"active"}`), 0o644); err != nil { + t.Fatal(err) + } + + if !hasAnyLiveSession(tmpDir) { + t.Error("should return true when ACTIVE session exists") + } +} + +func TestHasAnyLiveSession_AllEnded(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(sessDir, "sess-1.json"), []byte(`{"phase":"ended"}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-2.json"), []byte(`{"phase":"ended"}`), 0o644); err != nil { + t.Fatal(err) + } + + if hasAnyLiveSession(tmpDir) { + t.Error("should return false when all sessions are ended") + } +} + +func TestHasAnyLiveSession_MixedPhases(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(sessDir, "sess-ended.json"), []byte(`{"phase":"ended"}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-idle.json"), []byte(`{"phase":"idle"}`), 0o644); err != nil { + t.Fatal(err) + } + + if !hasAnyLiveSession(tmpDir) { + t.Error("should return true when at least one non-ended session exists") + } +} + func TestShouldSkipPendingReview_OrphanNoState(t *testing.T) { t.Parallel() diff --git a/docs/architecture/wingman.md b/docs/architecture/wingman.md new file mode 100644 index 000000000..1ceab36c9 --- /dev/null +++ b/docs/architecture/wingman.md @@ -0,0 +1,226 @@ +# Wingman: Automated Code Review + +Wingman is an automated code review system that reviews agent-produced code changes after each commit and delivers actionable suggestions back to the agent. + +## Overview + +When enabled, wingman runs a background review after each commit (or checkpoint), writes suggestions to `.entire/REVIEW.md`, and ensures the agent addresses them. The system prioritizes **visible delivery** — the user should see the agent reading and applying the review in their terminal. + +## Architecture + +### Components + +| Component | File | Purpose | +|-----------|------|---------| +| Trigger & state | `wingman.go` | Payload, state management, dedup, lock files, stale review cleanup | +| Review process | `wingman_review.go` | Detached subprocess: diff, Claude review call, REVIEW.md creation, session detection | +| Review prompt | `wingman_prompt.go` | Builds the review prompt from diff, prompts, context | +| Instruction | `wingman_instruction.md` | Embedded instruction injected into agent context | +| Process spawning | `wingman_spawn_unix.go` | Detached subprocess spawning (Unix) | +| Process spawning | `wingman_spawn_other.go` | No-op stubs (non-Unix) | +| Hook integration | `hooks_claudecode_handlers.go` | Prompt-submit injection, stop hook trigger, session-end trigger | + +### State Files + +| File | Purpose | +|------|---------| +| `.entire/REVIEW.md` | The review itself, read by the agent | +| `.entire/wingman-state.json` | Dedup state, session ID, apply tracking | +| `.entire/wingman.lock` | Prevents concurrent review spawns | +| `.entire/wingman-payload.json` | Payload passed to detached review process | +| `.entire/logs/wingman.log` | Review process logs (stderr redirect) | + +## Lifecycle + +### Phase 1: Trigger + +A wingman review is triggered after code changes are committed: + +- **Manual-commit strategy**: Git `post-commit` hook calls `triggerWingmanFromCommit()` +- **Auto-commit strategy**: Stop hook calls `triggerWingmanReview()` after `SaveChanges()` + +Before spawning, preconditions are checked: +1. `ENTIRE_WINGMAN_APPLY` env var not set (prevents recursion from auto-apply) +2. No pending REVIEW.md for the current session (`shouldSkipPendingReview()`) +3. Lock file acquired atomically (`acquireWingmanLock()`) +4. File hash dedup — skip if same files were already reviewed this session + +### Phase 2: Review (Detached Process) + +The review runs in a fully detached subprocess (`entire wingman __review `): + +``` +┌─ Detached Process ─────────────────────────────────────┐ +│ 1. Read payload from file │ +│ 2. Wait 10s for agent turn to settle │ +│ 3. Compute diff (merge-base with main/master) │ +│ 4. Load session context from checkpoint metadata │ +│ 5. Build review prompt (diff + prompts + context) │ +│ 6. Call claude --print (sonnet model, read-only tools) │ +│ 7. Write REVIEW.md │ +│ 8. Save dedup state │ +│ 9. Determine delivery path (see Phase 3) │ +│ 10. Remove lock file │ +└────────────────────────────────────────────────────────┘ +``` + +The review process uses `--setting-sources ""` to disable hooks (prevents recursion) and strips `GIT_*` environment variables for isolation. + +### Phase 3: Delivery + +There are two delivery mechanisms. The system chooses based on whether any session is still alive. + +#### Primary: Prompt-Submit Injection (Visible) + +When a live session exists (IDLE, ACTIVE, or ACTIVE_COMMITTED phase), the review is delivered through the agent's next prompt: + +``` +Review finishes → REVIEW.md written → live session detected → defer to injection + +User sends prompt → UserPromptSubmit hook fires + → REVIEW.md exists on disk + → Inject as additionalContext (mandatory agent instruction) + → Agent reads REVIEW.md, applies suggestions, deletes file + → Agent then proceeds with user's actual request +``` + +The `additionalContext` hook response field adds the instruction directly to Claude's context, making it a mandatory pre-step rather than an ignorable warning. The embedded instruction (`wingman_instruction.md`) tells the agent to: + +1. Read `.entire/REVIEW.md` +2. Address each suggestion (skip any it disagrees with) +3. Delete `.entire/REVIEW.md` when done +4. Briefly tell the user what changed + +This path is **visible** — the user sees the agent working through the review in their terminal. + +#### Fallback: Background Auto-Apply (Invisible) + +When no live sessions exist (all ENDED or none), REVIEW.md is applied via a background process: + +``` +Review finishes → REVIEW.md written → no live sessions → background auto-apply + +entire wingman __apply + → Verify REVIEW.md exists + → Check ApplyAttemptedAt not set (retry prevention) + → Re-check session idle (guard against race) + → claude --continue --print --permission-mode acceptEdits +``` + +This path is **invisible** — it runs silently. It exists as a fallback for when no session will receive the injection (e.g., user closed all sessions during the review window). + +### Decision Flow + +``` + REVIEW.md written + │ + ▼ + ┌─────────────────┐ + │ Any live session │ + │ exists? │ + └────────┬────────┘ + │ │ + Yes No + │ │ + ▼ ▼ + ┌──────────┐ ┌──────────────┐ + │ Defer │ │ Background │ + │ to next │ │ auto-apply │ + │ prompt │ │ immediately │ + └──────────┘ └──────────────┘ + │ + ▼ + User sends prompt + │ + ▼ + additionalContext + injection fires + │ + ▼ + Agent applies review + (visible in terminal) +``` + +### Trigger Points Summary + +| Trigger | When | What Happens | +|---------|------|-------------| +| **Review process** (`runWingmanReview`) | Review finishes | If no live sessions → background auto-apply. Otherwise defer. | +| **Prompt-submit hook** (`captureInitialState`) | User sends prompt | If REVIEW.md exists → inject as `additionalContext`. | +| **Stop hook** (`triggerWingmanAutoApplyIfPending`) | Agent turn ends | If REVIEW.md exists + no live sessions → spawn `__apply`. | +| **Session-end hook** (`triggerWingmanAutoApplyOnSessionEnd`) | User closes session | If REVIEW.md exists + no remaining live sessions → spawn `__apply`. | + +## Timing + +Typical timeline for a review cycle: + +``` +T+0s Commit happens → wingman review triggered +T+0s Lock file created, payload written +T+10s Initial settle delay completes +T+10s Diff computed (~30-50ms) +T+11s Claude review API call starts +T+30-50s Review received, REVIEW.md written +T+30-50s Delivery path determined +``` + +The 10-second initial delay lets the agent turn fully settle before computing the diff, ensuring all file writes are flushed. + +## Stale Review Cleanup + +Reviews can become stale in several scenarios. The `shouldSkipPendingReview()` function handles cleanup: + +| Scenario | Detection | Action | +|----------|-----------|--------| +| REVIEW.md without state file | `state == nil` | Delete REVIEW.md (orphan) | +| REVIEW.md from different session | `state.SessionID != currentSessionID` | Delete REVIEW.md (stale) | +| REVIEW.md older than 1 hour | `time.Since(state.ReviewedAt) > 1h` | Delete REVIEW.md (TTL expired) | +| REVIEW.md from current session | Session matches + fresh | Keep (skip new review) | + +## Retry Prevention + +The `ApplyAttemptedAt` field in `WingmanState` prevents infinite auto-apply attempts: + +- Set to current time before triggering auto-apply +- Reset to `nil` when a new review is written +- Checked before every auto-apply attempt — if set, skip + +## Concurrency Safety + +- **Lock file**: Atomic `O_CREATE|O_EXCL` prevents concurrent review spawns. Stale locks (>30 min) are auto-cleaned. +- **Dedup hash**: SHA256 of sorted file paths prevents re-reviewing identical change sets. +- **Detached processes**: Review and apply run in their own process groups (`Setpgid: true`), surviving parent exit. +- **GIT_* stripping**: Subprocesses strip all `GIT_*` env vars to prevent index corruption. +- **ENTIRE_WINGMAN_APPLY=1**: Set during auto-apply to prevent the post-commit hook from triggering another review (recursion prevention). + +## Configuration + +```bash +entire wingman enable # Enable wingman auto-review +entire wingman disable # Disable and clean up pending reviews +entire wingman status # Show current status +``` + +Wingman state is stored in `.entire/settings.json`: + +```json +{ + "strategy_options": { + "wingman": { + "enabled": true + } + } +} +``` + +## Key Constants + +| Constant | Value | Purpose | +|----------|-------|---------| +| `wingmanInitialDelay` | 10s | Settle time before computing diff | +| `wingmanReviewModel` | `sonnet` | Model used for reviews | +| `wingmanGitTimeout` | 60s | Timeout for git diff operations | +| `wingmanReviewTimeout` | 5m | Timeout for Claude review API call | +| `wingmanApplyTimeout` | 15m | Timeout for auto-apply process | +| `wingmanStaleReviewTTL` | 1h | Max age before review is cleaned up | +| `staleLockThreshold` | 30m | Max age before lock is considered stale | From 7491e85185bd61e6a7cee99a84ab0be73d2416f6 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 14:15:47 +0100 Subject: [PATCH 15/32] Add timeout to resolveHEAD and fix log file cleanup in spawn helpers Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 6e7f40877703 --- cmd/entire/cli/wingman.go | 4 +++- cmd/entire/cli/wingman_spawn_unix.go | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index c953a9a4f..4252a65c2 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -383,7 +383,9 @@ func acquireWingmanLock(lockPath, sessionID string) bool { // resolveHEAD returns the current HEAD commit hash, or empty string on error. func resolveHEAD(repoRoot string) string { - cmd := exec.CommandContext(context.Background(), "git", "rev-parse", "HEAD") + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "rev-parse", "HEAD") cmd.Dir = repoRoot out, err := cmd.Output() if err != nil { diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index 6a34afb75..7af25168f 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -39,13 +39,14 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { // Redirect stderr to a log file for debugging the background process. // This catches panics, errors, and all wingmanLog() output. cmd.Stdout = io.Discard + var logFile *os.File logDir := filepath.Join(repoRoot, ".entire", "logs") if mkErr := os.MkdirAll(logDir, 0o750); mkErr == nil { //nolint:gosec // G304: path is constructed from repoRoot + constants if f, openErr := os.OpenFile(filepath.Join(logDir, "wingman.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600); openErr == nil { + logFile = f cmd.Stderr = f - // f stays open in the child process; the OS closes it on exit } else { cmd.Stderr = io.Discard } @@ -55,6 +56,9 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { // Start the process (non-blocking) if err := cmd.Start(); err != nil { + if logFile != nil { + _ = logFile.Close() + } return } @@ -83,11 +87,13 @@ func spawnDetachedWingmanApply(repoRoot string) { cmd.Env = os.Environ() cmd.Stdout = io.Discard + var applyLogFile *os.File logDir := filepath.Join(repoRoot, ".entire", "logs") if mkErr := os.MkdirAll(logDir, 0o750); mkErr == nil { //nolint:gosec // G304: path is constructed from repoRoot + constants if f, openErr := os.OpenFile(filepath.Join(logDir, "wingman.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600); openErr == nil { + applyLogFile = f cmd.Stderr = f } else { cmd.Stderr = io.Discard @@ -97,6 +103,9 @@ func spawnDetachedWingmanApply(repoRoot string) { } if err := cmd.Start(); err != nil { + if applyLogFile != nil { + _ = applyLogFile.Close() + } return } From ef4377584e94dc1b7bf7eb9e4f531c342ee4290f Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 14:20:35 +0100 Subject: [PATCH 16/32] Document review prompt construction and context sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review prompt is the core value proposition: it leverages Entire's checkpoint data (prompts, session context, commit message) to enable intent-aware review that catches misalignment between what was asked and what was built — not just code bugs. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 90fe9697fc50 --- docs/architecture/wingman.md | 82 ++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/docs/architecture/wingman.md b/docs/architecture/wingman.md index 1ceab36c9..f1aad4940 100644 --- a/docs/architecture/wingman.md +++ b/docs/architecture/wingman.md @@ -166,6 +166,88 @@ T+30-50s Delivery path determined The 10-second initial delay lets the agent turn fully settle before computing the diff, ensuring all file writes are flushed. +## Review Prompt Construction + +The review prompt is the core of wingman's value. Unlike a naive diff-only review, it leverages Entire's checkpoint data to give the reviewer **full context about what the developer was trying to accomplish**. This enables intent-aware review — catching not just bugs, but misalignment between what was asked and what was built. + +### Context Sources + +The prompt is assembled from six sources, each contributing a different layer of understanding: + +| Source | Origin | What It Provides | +|--------|--------|-----------------| +| **Developer prompts** | `prompt.txt` from checkpoint metadata | The original instructions given to the agent — the ground truth of intent | +| **Commit message** | Git commit or auto-commit message | A summary of what was done (may differ from what was asked) | +| **Session context** | `context.md` from checkpoint metadata | Generated summary of key actions, decisions, and session flow | +| **Checkpoint files** | `.entire/metadata//` | Paths provided so the reviewer can read the full transcript, prompts, or context if needed | +| **File list** | Payload from trigger | Which files changed and how (modified/new/deleted) | +| **Branch diff** | `git diff` against merge-base | The actual code changes — computed against `main`/`master` for a holistic branch-level view | + +### Prompt Structure + +``` +┌─ System Role ──────────────────────────────────────────┐ +│ "You are a senior code reviewer performing an │ +│ intent-aware review." │ +├─ Session Context ──────────────────────────────────────┤ +│ Developer's Prompts ... │ +│ Commit Message (plain text) │ +│ Session Context ... │ +│ Checkpoint File Paths (for deeper investigation) │ +├─ Code Changes ─────────────────────────────────────────┤ +│ Files changed: file.go (modified), ... │ +│ Diff ... │ +├─ Review Instructions ──────────────────────────────────┤ +│ Intent alignment (most important) │ +│ Correctness bugs, logic errors, races │ +│ Security injection, secrets, path traversal │ +│ Robustness edge cases, leaks, timeouts │ +│ Do NOT flag style, docs on clear code │ +│ Output format Markdown with severity levels │ +└────────────────────────────────────────────────────────┘ +``` + +### Intent-Aware Review + +The review instructions prioritize **intent alignment** as the most important category: + +1. Do the changes actually accomplish what the developer asked for? +2. Are there any prompts or requirements that were missed or only partially implemented? +3. Does the implementation match the stated approach in the session context? + +This is only possible because Entire captures the developer's prompts and session context as checkpoint metadata. A reviewer that only sees the diff cannot evaluate whether the code matches the original request. + +### Diff Strategy + +The diff is computed against the **merge-base** with `main`/`master`, not just the latest commit. This gives the reviewer a holistic view of all branch changes rather than a narrow single-commit diff. Fallback chain: + +1. `git merge-base main HEAD` → diff against merge-base (branch-level view) +2. `git merge-base master HEAD` → try master if main doesn't exist +3. `git diff HEAD` → uncommitted changes only +4. `git diff HEAD~1 HEAD` → latest commit if no uncommitted changes + +### Read-Only Tool Access + +The reviewer Claude instance has access to `Read`, `Glob`, and `Grep` tools with `--permission-mode bypassPermissions`. This allows it to: + +- Read source files beyond the diff for additional context +- Search for related code patterns or usages +- Inspect the checkpoint metadata files referenced in the prompt + +Tools are restricted to read-only operations — the reviewer cannot modify files. + +### Truncation + +Diffs larger than 100KB are truncated to maintain review quality. Very large diffs tend to produce unfocused reviews. + +### Output Format + +The reviewer outputs structured Markdown with: +- **Summary**: Does the change accomplish its goal? Overall quality assessment. +- **Issues**: Each with severity (`CRITICAL`, `WARNING`, `SUGGESTION`), file path with line reference, description, and suggested fix. + +This output is written directly to `.entire/REVIEW.md` and later read by the agent during delivery. + ## Stale Review Cleanup Reviews can become stale in several scenarios. The `shouldSkipPendingReview()` function handles cleanup: From 2bd71c8c6e69198f7c4f0e004205dcd49821c1a9 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 14:36:42 +0100 Subject: [PATCH 17/32] Add --local flag to wingman enable/disable commands Allows writing wingman settings to settings.local.json instead of settings.json, matching the --local pattern used by entire enable. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 303bd4b4630f --- cmd/entire/cli/wingman.go | 81 ++++++++++++++++++++--- cmd/entire/cli/wingman_test.go | 113 +++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/wingman.go b/cmd/entire/cli/wingman.go index 4252a65c2..e1c969c8e 100644 --- a/cmd/entire/cli/wingman.go +++ b/cmd/entire/cli/wingman.go @@ -81,7 +81,9 @@ to apply them.`, } func newWingmanEnableCmd() *cobra.Command { - return &cobra.Command{ + var useLocal bool + + cmd := &cobra.Command{ Use: "enable", Short: "Enable wingman auto-review", RunE: func(cmd *cobra.Command, _ []string) error { @@ -90,37 +92,54 @@ func newWingmanEnableCmd() *cobra.Command { return NewSilentError(errors.New("not a git repository")) } - s, err := settings.Load() + // Load merged settings to check preconditions + merged, err := settings.Load() if err != nil { return fmt.Errorf("failed to load settings: %w", err) } - if !s.Enabled { + if !merged.Enabled { fmt.Fprintln(cmd.ErrOrStderr(), "Entire is not enabled. Run 'entire enable' first.") return NewSilentError(errors.New("entire not enabled")) } + // Load the target file specifically so we don't bloat it with merged values + s, err := loadSettingsTarget(useLocal) + if err != nil { + return fmt.Errorf("failed to load settings: %w", err) + } + if s.StrategyOptions == nil { s.StrategyOptions = make(map[string]any) } s.StrategyOptions["wingman"] = map[string]any{"enabled": true} - if err := settings.Save(s); err != nil { + if err := saveSettingsTarget(s, useLocal); err != nil { return fmt.Errorf("failed to save settings: %w", err) } - fmt.Fprintln(cmd.OutOrStdout(), "Wingman enabled. Code changes will be automatically reviewed after agent turns.") + msg := "Wingman enabled. Code changes will be automatically reviewed after agent turns." + if useLocal { + msg += " (saved to settings.local.json)" + } + fmt.Fprintln(cmd.OutOrStdout(), msg) return nil }, } + + cmd.Flags().BoolVar(&useLocal, "local", false, "Write to settings.local.json instead of settings.json") + + return cmd } func newWingmanDisableCmd() *cobra.Command { - return &cobra.Command{ + var useLocal bool + + cmd := &cobra.Command{ Use: "disable", Short: "Disable wingman auto-review", RunE: func(cmd *cobra.Command, _ []string) error { - s, err := settings.Load() + s, err := loadSettingsTarget(useLocal) if err != nil { return fmt.Errorf("failed to load settings: %w", err) } @@ -130,7 +149,7 @@ func newWingmanDisableCmd() *cobra.Command { } s.StrategyOptions["wingman"] = map[string]any{"enabled": false} - if err := settings.Save(s); err != nil { + if err := saveSettingsTarget(s, useLocal); err != nil { return fmt.Errorf("failed to save settings: %w", err) } @@ -140,10 +159,54 @@ func newWingmanDisableCmd() *cobra.Command { _ = os.Remove(reviewPath) } - fmt.Fprintln(cmd.OutOrStdout(), "Wingman disabled.") + msg := "Wingman disabled." + if useLocal { + msg += " (saved to settings.local.json)" + } + fmt.Fprintln(cmd.OutOrStdout(), msg) return nil }, } + + cmd.Flags().BoolVar(&useLocal, "local", false, "Write to settings.local.json instead of settings.json") + + return cmd +} + +// loadSettingsTarget loads settings from the appropriate file based on the --local flag. +// When local is true, loads from settings.local.json only (without merging). +// When local is false, loads the merged settings (project + local). +func loadSettingsTarget(local bool) (*settings.EntireSettings, error) { + if !local { + s, err := settings.Load() + if err != nil { + return nil, fmt.Errorf("loading settings: %w", err) + } + return s, nil + } + absPath, err := paths.AbsPath(settings.EntireSettingsLocalFile) + if err != nil { + absPath = settings.EntireSettingsLocalFile + } + s, err := settings.LoadFromFile(absPath) + if err != nil { + return nil, fmt.Errorf("loading local settings: %w", err) + } + return s, nil +} + +// saveSettingsTarget saves settings to the appropriate file based on the --local flag. +func saveSettingsTarget(s *settings.EntireSettings, local bool) error { + if local { + if err := settings.SaveLocal(s); err != nil { + return fmt.Errorf("saving local settings: %w", err) + } + return nil + } + if err := settings.Save(s); err != nil { + return fmt.Errorf("saving settings: %w", err) + } + return nil } func newWingmanStatusCmd() *cobra.Command { diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index fcea73996..3cc3b2ca9 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -594,6 +594,119 @@ func TestHasAnyLiveSession_MixedPhases(t *testing.T) { } } +func TestLoadSettingsTarget_Local(t *testing.T) { + // Uses t.Chdir so cannot be parallel + tmpDir := t.TempDir() + + cmd := exec.CommandContext(context.Background(), "git", "init", tmpDir) + if err := cmd.Run(); err != nil { + t.Fatalf("failed to git init: %v", err) + } + + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Write project settings with wingman disabled + projectSettings := `{"strategy": "manual-commit", "enabled": true, "strategy_options": {"wingman": {"enabled": false}}}` + if err := os.WriteFile(filepath.Join(entireDir, "settings.json"), []byte(projectSettings), 0o644); err != nil { + t.Fatal(err) + } + + // Write local settings with different strategy + localSettings := `{"strategy": "` + strategyDisplayAutoCommit + `"}` + if err := os.WriteFile(filepath.Join(entireDir, "settings.local.json"), []byte(localSettings), 0o644); err != nil { + t.Fatal(err) + } + + t.Chdir(tmpDir) + + t.Run("local=false returns merged settings", func(t *testing.T) { + s, err := loadSettingsTarget(false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Merged: local overrides project strategy + if s.Strategy != strategyDisplayAutoCommit { + t.Errorf("Strategy = %q, want %q", s.Strategy, strategyDisplayAutoCommit) + } + }) + + t.Run("local=true returns only local settings", func(t *testing.T) { + s, err := loadSettingsTarget(true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if s.Strategy != strategyDisplayAutoCommit { + t.Errorf("Strategy = %q, want %q", s.Strategy, strategyDisplayAutoCommit) + } + // Local file has no wingman settings + if s.IsWingmanEnabled() { + t.Error("local settings should not have wingman enabled") + } + }) +} + +func TestSaveSettingsTarget_Local(t *testing.T) { + // Uses t.Chdir so cannot be parallel + tmpDir := t.TempDir() + + cmd := exec.CommandContext(context.Background(), "git", "init", tmpDir) + if err := cmd.Run(); err != nil { + t.Fatalf("failed to git init: %v", err) + } + + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + t.Chdir(tmpDir) + + s := &EntireSettings{ + StrategyOptions: map[string]any{ + "wingman": map[string]any{"enabled": true}, + }, + } + + t.Run("local=true saves to local file", func(t *testing.T) { + if err := saveSettingsTarget(s, true); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + localPath := filepath.Join(entireDir, "settings.local.json") + data, err := os.ReadFile(localPath) + if err != nil { + t.Fatalf("local settings file should exist: %v", err) + } + if !strings.Contains(string(data), `"wingman"`) { + t.Error("local settings should contain wingman config") + } + + // Project file should not exist + projectPath := filepath.Join(entireDir, "settings.json") + if _, err := os.Stat(projectPath); err == nil { + t.Error("project settings file should not have been created") + } + }) + + t.Run("local=false saves to project file", func(t *testing.T) { + if err := saveSettingsTarget(s, false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + projectPath := filepath.Join(entireDir, "settings.json") + data, err := os.ReadFile(projectPath) + if err != nil { + t.Fatalf("project settings file should exist: %v", err) + } + if !strings.Contains(string(data), `"wingman"`) { + t.Error("project settings should contain wingman config") + } + }) +} + func TestShouldSkipPendingReview_OrphanNoState(t *testing.T) { t.Parallel() From 5fddc49ce59e8deeaa0100aa4e2dc0989ba4c6f5 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 15:00:54 +0100 Subject: [PATCH 18/32] Fix hook response format to nest additionalContext under hookSpecificOutput Claude Code expects additionalContext nested under hookSpecificOutput, not at the top level. Also adds a user-visible systemMessage warning when a wingman review is about to be injected on prompt submit. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: fc64fce284b3 --- cmd/entire/cli/hooks.go | 41 +++-- cmd/entire/cli/hooks_claudecode_handlers.go | 5 +- cmd/entire/cli/hooks_test.go | 169 ++++++++++++++++++++ 3 files changed, 202 insertions(+), 13 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 4bcab386e..806f70dea 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -299,17 +299,30 @@ func handleSessionStartCommon() error { return nil } -// hookResponse represents a JSON response. -// Used to control whether Agent continues processing the prompt. -type hookResponse struct { - SystemMessage string `json:"systemMessage,omitempty"` +// hookSpecificOutput contains event-specific fields nested under hookSpecificOutput +// in the hook response JSON. Claude Code requires this nesting for additionalContext +// to be injected into the agent's conversation. +type hookSpecificOutput struct { + HookEventName string `json:"hookEventName"` AdditionalContext string `json:"additionalContext,omitempty"` } -// outputHookResponse outputs a JSON response to stdout -func outputHookResponse(reason string) error { +// hookResponse represents a JSON response to a Claude Code hook. +// systemMessage is shown to the user as a warning/info message. +// hookSpecificOutput contains event-specific fields like additionalContext. +type hookResponse struct { + SystemMessage string `json:"systemMessage,omitempty"` + HookSpecificOutput *hookSpecificOutput `json:"hookSpecificOutput,omitempty"` +} + +// outputHookResponse outputs a JSON response with additionalContext for +// SessionStart hooks. The context is injected into the agent's conversation. +func outputHookResponse(context string) error { resp := hookResponse{ - SystemMessage: reason, + HookSpecificOutput: &hookSpecificOutput{ + HookEventName: "SessionStart", + AdditionalContext: context, + }, } if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { return fmt.Errorf("failed to encode hook response: %w", err) @@ -317,12 +330,16 @@ func outputHookResponse(reason string) error { return nil } -// outputHookResponseWithContext outputs a JSON response that adds context to -// the agent's conversation. additionalContext is injected into Claude's context -// and treated as required instructions, not just a warning. -func outputHookResponseWithContext(context string) error { +// outputHookResponseWithContextAndMessage outputs a JSON response with both +// additionalContext (injected into agent conversation) and a systemMessage +// (shown to the user as a warning/info). +func outputHookResponseWithContextAndMessage(context, message string) error { resp := hookResponse{ - AdditionalContext: context, + SystemMessage: message, + HookSpecificOutput: &hookSpecificOutput{ + HookEventName: "UserPromptSubmit", + AdditionalContext: context, + }, } if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { return fmt.Errorf("failed to encode hook response: %w", err) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index a149085e6..aa4f225bd 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -88,7 +88,10 @@ func captureInitialState() error { logging.Info(wingmanLogCtx, "wingman injecting review instruction on prompt-submit", slog.String("session_id", hookData.sessionID), ) - if err := outputHookResponseWithContext(wingmanApplyInstruction); err != nil { + if err := outputHookResponseWithContextAndMessage( + wingmanApplyInstruction, + "[Wingman] A code review is pending and will be addressed before your request.", + ); err != nil { fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) } } diff --git a/cmd/entire/cli/hooks_test.go b/cmd/entire/cli/hooks_test.go index b84dda1f5..fe57353a1 100644 --- a/cmd/entire/cli/hooks_test.go +++ b/cmd/entire/cli/hooks_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "encoding/json" "strings" "testing" ) @@ -539,3 +540,171 @@ func TestLogPostTaskHookContext(t *testing.T) { }) } } + +func TestHookResponse_SessionStart(t *testing.T) { + t.Parallel() + + resp := hookResponse{ + HookSpecificOutput: &hookSpecificOutput{ + HookEventName: "SessionStart", + AdditionalContext: "Powered by Entire", + }, + } + + data, err := json.Marshal(resp) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + // Verify the nested structure + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + // systemMessage should be absent (omitempty) + if _, ok := raw["systemMessage"]; ok { + t.Error("systemMessage should be omitted when empty") + } + + // hookSpecificOutput should be present + hsoRaw, ok := raw["hookSpecificOutput"] + if !ok { + t.Fatal("hookSpecificOutput missing from response") + } + + var hso map[string]string + if err := json.Unmarshal(hsoRaw, &hso); err != nil { + t.Fatalf("failed to unmarshal hookSpecificOutput: %v", err) + } + + if hso["hookEventName"] != "SessionStart" { + t.Errorf("hookEventName = %q, want %q", hso["hookEventName"], "SessionStart") + } + if hso["additionalContext"] != "Powered by Entire" { + t.Errorf("additionalContext = %q, want %q", hso["additionalContext"], "Powered by Entire") + } +} + +func TestHookResponse_UserPromptSubmit(t *testing.T) { + t.Parallel() + + resp := hookResponse{ + HookSpecificOutput: &hookSpecificOutput{ + HookEventName: "UserPromptSubmit", + AdditionalContext: "Review instructions here", + }, + } + + data, err := json.Marshal(resp) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + // systemMessage should be absent + if _, ok := raw["systemMessage"]; ok { + t.Error("systemMessage should be omitted when empty") + } + + hsoRaw, ok := raw["hookSpecificOutput"] + if !ok { + t.Fatal("hookSpecificOutput missing from response") + } + + var hso map[string]string + if err := json.Unmarshal(hsoRaw, &hso); err != nil { + t.Fatalf("failed to unmarshal hookSpecificOutput: %v", err) + } + + if hso["hookEventName"] != "UserPromptSubmit" { + t.Errorf("hookEventName = %q, want %q", hso["hookEventName"], "UserPromptSubmit") + } + if hso["additionalContext"] != "Review instructions here" { + t.Errorf("additionalContext = %q, want %q", hso["additionalContext"], "Review instructions here") + } +} + +func TestHookResponse_WithContextAndMessage(t *testing.T) { + t.Parallel() + + resp := hookResponse{ + SystemMessage: "[Wingman] A code review is pending.", + HookSpecificOutput: &hookSpecificOutput{ + HookEventName: "UserPromptSubmit", + AdditionalContext: "Apply the review", + }, + } + + data, err := json.Marshal(resp) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + // systemMessage should be present + var sysMsg string + if err := json.Unmarshal(raw["systemMessage"], &sysMsg); err != nil { + t.Fatalf("failed to unmarshal systemMessage: %v", err) + } + if sysMsg != "[Wingman] A code review is pending." { + t.Errorf("systemMessage = %q, want %q", sysMsg, "[Wingman] A code review is pending.") + } + + // hookSpecificOutput should also be present + hsoRaw, ok := raw["hookSpecificOutput"] + if !ok { + t.Fatal("hookSpecificOutput missing from response") + } + + var hso map[string]string + if err := json.Unmarshal(hsoRaw, &hso); err != nil { + t.Fatalf("failed to unmarshal hookSpecificOutput: %v", err) + } + + if hso["hookEventName"] != "UserPromptSubmit" { + t.Errorf("hookEventName = %q, want %q", hso["hookEventName"], "UserPromptSubmit") + } + if hso["additionalContext"] != "Apply the review" { + t.Errorf("additionalContext = %q, want %q", hso["additionalContext"], "Apply the review") + } +} + +func TestHookResponse_NilHookSpecificOutput(t *testing.T) { + t.Parallel() + + resp := hookResponse{ + SystemMessage: "Just a message", + } + + data, err := json.Marshal(resp) + if err != nil { + t.Fatalf("failed to marshal: %v", err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + + // hookSpecificOutput should be absent (omitempty on pointer) + if _, ok := raw["hookSpecificOutput"]; ok { + t.Error("hookSpecificOutput should be omitted when nil") + } + + var sysMsg string + if err := json.Unmarshal(raw["systemMessage"], &sysMsg); err != nil { + t.Fatalf("failed to unmarshal systemMessage: %v", err) + } + if sysMsg != "Just a message" { + t.Errorf("systemMessage = %q, want %q", sysMsg, "Just a message") + } +} From b01e5547e50bf4daba35866a142552f901aa5bbd Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 15:14:43 +0100 Subject: [PATCH 19/32] Restore user-visible SessionStart message and fix hook control flow The format fix in 5fddc49c moved the "Powered by Entire" message from systemMessage (user-visible) to additionalContext only (agent-only), causing it to disappear from the terminal. Now sets both fields so the message is visible to the user and injected into agent context. Also returns immediately after successful wingman review injection to prevent subsequent code from corrupting the stdout JSON response, and removes unnecessary ireturn nolint directives that lint no longer requires. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 64267478c135 --- cmd/entire/cli/hooks.go | 1 + cmd/entire/cli/hooks_claudecode_handlers.go | 4 ++++ cmd/entire/cli/hooks_test.go | 7 ++++--- cmd/entire/cli/strategy/auto_commit.go | 2 +- cmd/entire/cli/strategy/manual_commit.go | 4 ++-- cmd/entire/cli/strategy/registry.go | 4 ++-- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 806f70dea..8914a2ed7 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -319,6 +319,7 @@ type hookResponse struct { // SessionStart hooks. The context is injected into the agent's conversation. func outputHookResponse(context string) error { resp := hookResponse{ + SystemMessage: context, HookSpecificOutput: &hookSpecificOutput{ HookEventName: "SessionStart", AdditionalContext: context, diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index aa4f225bd..8f63d92b7 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -93,6 +93,10 @@ func captureInitialState() error { "[Wingman] A code review is pending and will be addressed before your request.", ); err != nil { fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to inject review instruction: %v\n", err) + } else { + // Hook response written to stdout — must return immediately + // to avoid corrupting the JSON with additional output. + return nil } } diff --git a/cmd/entire/cli/hooks_test.go b/cmd/entire/cli/hooks_test.go index fe57353a1..bcb9e2a0a 100644 --- a/cmd/entire/cli/hooks_test.go +++ b/cmd/entire/cli/hooks_test.go @@ -545,6 +545,7 @@ func TestHookResponse_SessionStart(t *testing.T) { t.Parallel() resp := hookResponse{ + SystemMessage: "Powered by Entire", HookSpecificOutput: &hookSpecificOutput{ HookEventName: "SessionStart", AdditionalContext: "Powered by Entire", @@ -562,9 +563,9 @@ func TestHookResponse_SessionStart(t *testing.T) { t.Fatalf("failed to unmarshal: %v", err) } - // systemMessage should be absent (omitempty) - if _, ok := raw["systemMessage"]; ok { - t.Error("systemMessage should be omitted when empty") + // systemMessage should be present (same as additionalContext for user visibility) + if _, ok := raw["systemMessage"]; !ok { + t.Error("systemMessage should be present for SessionStart") } // hookSpecificOutput should be present diff --git a/cmd/entire/cli/strategy/auto_commit.go b/cmd/entire/cli/strategy/auto_commit.go index 7012e98b8..8f6e8f5ee 100644 --- a/cmd/entire/cli/strategy/auto_commit.go +++ b/cmd/entire/cli/strategy/auto_commit.go @@ -84,7 +84,7 @@ func (s *AutoCommitStrategy) getCheckpointStore() (*checkpoint.GitStore, error) // NewAutoCommitStrategy creates a new AutoCommitStrategy instance // -func NewAutoCommitStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewAutoCommitStrategy() Strategy { return &AutoCommitStrategy{} } diff --git a/cmd/entire/cli/strategy/manual_commit.go b/cmd/entire/cli/strategy/manual_commit.go index 3164df738..5c1a875af 100644 --- a/cmd/entire/cli/strategy/manual_commit.go +++ b/cmd/entire/cli/strategy/manual_commit.go @@ -58,7 +58,7 @@ func (s *ManualCommitStrategy) getCheckpointStore() (*checkpoint.GitStore, error // NewManualCommitStrategy creates a new manual-commit strategy instance. // -func NewManualCommitStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewManualCommitStrategy() Strategy { return &ManualCommitStrategy{} } @@ -66,7 +66,7 @@ func NewManualCommitStrategy() Strategy { //nolint:ireturn // already present in // This legacy constructor delegates to NewManualCommitStrategy. // -func NewShadowStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewShadowStrategy() Strategy { return NewManualCommitStrategy() } diff --git a/cmd/entire/cli/strategy/registry.go b/cmd/entire/cli/strategy/registry.go index a57842427..5d226590d 100644 --- a/cmd/entire/cli/strategy/registry.go +++ b/cmd/entire/cli/strategy/registry.go @@ -24,7 +24,7 @@ func Register(name string, factory Factory) { // Get retrieves a strategy by name. // Returns an error if the strategy is not registered. -func Get(name string) (Strategy, error) { //nolint:ireturn // already present in codebase +func Get(name string) (Strategy, error) { registryMu.RLock() defer registryMu.RUnlock() @@ -61,7 +61,7 @@ const DefaultStrategyName = StrategyNameManualCommit // Default returns the default strategy. // Falls back to returning nil if no strategies are registered. -func Default() Strategy { //nolint:ireturn // already present in codebase +func Default() Strategy { s, err := Get(DefaultStrategyName) if err != nil { // Fallback: return the first registered strategy From 5f69d94b9334fec19c84069b68e92bd324373d5c Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 15:54:15 +0100 Subject: [PATCH 20/32] Fix wingman auto-apply never triggering on session close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auto-apply subprocess aborted when triggered from SessionEnd because runWingmanApply checked isSessionIdle() which only returned true for PhaseIdle. After SessionEnd marks the session as PhaseEnded, the check failed and the subprocess exited with "session became active during spawn." Replace with a direct Phase.IsActive() check that only blocks on ACTIVE/ACTIVE_COMMITTED — both IDLE and ENDED are safe to auto-apply. Also add a 2-hour staleness threshold to hasAnyLiveSession so orphaned session state files cannot permanently block auto-apply, and improve diagnostic logging throughout the auto-apply flow (skip reasons, spawned PIDs, decision points). Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: c4e9f1a814ea --- cmd/entire/cli/hooks_claudecode_handlers.go | 28 ++- cmd/entire/cli/wingman_review.go | 91 +++----- cmd/entire/cli/wingman_spawn_unix.go | 9 + cmd/entire/cli/wingman_test.go | 244 ++++++++++++++++++++ 4 files changed, 313 insertions(+), 59 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 8f63d92b7..dcd3e4119 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -840,17 +840,25 @@ func transitionSessionTurnEnd(sessionID string) { // will deliver the review visibly in the user's terminal instead. Background // auto-apply is only used when no sessions are alive (all ended). func triggerWingmanAutoApplyIfPending(repoRoot string) { - if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + logCtx := logging.WithComponent(context.Background(), "wingman") + if !settings.IsWingmanEnabled() { + logging.Debug(logCtx, "wingman auto-apply skip: wingman not enabled") + return + } + if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + logging.Debug(logCtx, "wingman auto-apply skip: already in apply subprocess") return } - logCtx := logging.WithComponent(context.Background(), "wingman") reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if _, statErr := os.Stat(reviewPath); statErr != nil { + logging.Debug(logCtx, "wingman auto-apply skip: no REVIEW.md pending") return } wingmanState := loadWingmanStateDirect(repoRoot) if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { - logging.Debug(logCtx, "wingman auto-apply already attempted, skipping") + logging.Debug(logCtx, "wingman auto-apply already attempted, skipping", + slog.Time("attempted_at", *wingmanState.ApplyAttemptedAt), + ) return } // Don't spawn background auto-apply if a live session exists. @@ -872,20 +880,30 @@ func triggerWingmanAutoApplyIfPending(repoRoot string) { // session ends. If no live sessions remain, spawns background auto-apply. // Called from both Claude Code and Gemini CLI SessionEnd hooks. func triggerWingmanAutoApplyOnSessionEnd() { - if !settings.IsWingmanEnabled() || os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + logCtx := logging.WithComponent(context.Background(), "wingman") + if !settings.IsWingmanEnabled() { + logging.Debug(logCtx, "wingman auto-apply on session-end skip: wingman not enabled") + return + } + if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + logging.Debug(logCtx, "wingman auto-apply on session-end skip: already in apply subprocess") return } repoRoot, err := paths.RepoRoot() if err != nil { + logging.Debug(logCtx, "wingman auto-apply on session-end skip: cannot resolve repo root") return } - logCtx := logging.WithComponent(context.Background(), "wingman") reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if _, statErr := os.Stat(reviewPath); statErr != nil { + logging.Debug(logCtx, "wingman auto-apply on session-end skip: no REVIEW.md pending") return } wingmanState := loadWingmanStateDirect(repoRoot) if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { + logging.Debug(logCtx, "wingman auto-apply on session-end skip: already attempted", + slog.Time("attempted_at", *wingmanState.ApplyAttemptedAt), + ) return } if hasAnyLiveSession(repoRoot) { diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 30e62ef83..913f5c418 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -334,59 +334,18 @@ func readSessionContext(repoRoot, sessionID string) string { return string(data) } -// isSessionIdle checks if any session is in the IDLE phase by reading -// session state files directly from the repo. Uses repoRoot to locate -// .git/entire-sessions/ without relying on the current working directory -// (which is "/" in detached subprocesses). -func isSessionIdle(repoRoot, sessionID string) bool { - sessDir := findSessionStateDir(repoRoot) - if sessDir == "" { - wingmanLog("cannot find session state dir for %s", repoRoot) - return false - } - - // Fast path: check the specific session - if sessionID != "" { - if phase := readSessionPhase(sessDir, sessionID); phase == string(session.PhaseIdle) { - return true - } - } - - // Slow path: check if ANY session is idle - // (covers session restart, ended sessions, etc.) - entries, err := os.ReadDir(sessDir) - if err != nil { - return false - } - const maxSessionCheck = 50 - checked := 0 - for _, entry := range entries { - if checked >= maxSessionCheck { - wingmanLog("hit session check limit (%d), assuming not idle", maxSessionCheck) - return false - } - name := entry.Name() - if !strings.HasSuffix(name, ".json") { - continue - } - sid := strings.TrimSuffix(name, ".json") - if sid == sessionID { - continue // Already checked - } - checked++ - if phase := readSessionPhase(sessDir, sid); phase == string(session.PhaseIdle) { - wingmanLog("found idle session %s (original session %s was not idle)", sid, sessionID) - return true - } - } - - return false -} +// staleSessionThreshold is the maximum age of a session state file before it +// is considered stale and ignored by hasAnyLiveSession. This prevents orphaned +// session files (e.g., from crashed processes) from permanently blocking auto-apply. +const staleSessionThreshold = 2 * time.Hour // hasAnyLiveSession checks if any session is in a non-ENDED phase (IDLE, // ACTIVE, or ACTIVE_COMMITTED). Used to decide whether to defer review // application to the prompt-submit injection (visible to user) vs background // auto-apply (invisible). +// +// Session state files older than staleSessionThreshold are skipped to prevent +// orphaned sessions from permanently blocking auto-apply. func hasAnyLiveSession(repoRoot string) bool { sessDir := findSessionStateDir(repoRoot) if sessDir == "" { @@ -409,6 +368,18 @@ func hasAnyLiveSession(repoRoot string) bool { continue } checked++ + + // Skip stale session files that haven't been updated recently. + // These are likely orphaned from crashed processes. + info, statErr := entry.Info() + if statErr != nil { + continue + } + if time.Since(info.ModTime()) > staleSessionThreshold { + wingmanLog("skipping stale session file %s (age=%s)", name, time.Since(info.ModTime()).Round(time.Second)) + continue + } + sid := strings.TrimSuffix(name, ".json") phase := readSessionPhase(sessDir, sid) if phase != "" && phase != string(session.PhaseEnded) { @@ -483,22 +454,34 @@ func runWingmanApply(repoRoot string) error { reviewPath := filepath.Join(repoRoot, wingmanReviewFile) if !fileExists(reviewPath) { - wingmanLog("no REVIEW.md found, nothing to apply") + wingmanLog("no REVIEW.md found at %s, nothing to apply", reviewPath) return nil } + wingmanLog("REVIEW.md found at %s", reviewPath) // Retry prevention: check if apply was already attempted for this review state := loadWingmanStateDirect(repoRoot) - if state != nil && state.ApplyAttemptedAt != nil { + switch { + case state == nil: + wingmanLog("no wingman state found, proceeding without session check") + case state.ApplyAttemptedAt != nil: wingmanLog("apply already attempted at %s, skipping", state.ApplyAttemptedAt.Format(time.RFC3339)) return nil + default: + wingmanLog("wingman state loaded: session=%s", state.SessionID) } - // Re-check session is still idle (user may have typed during spawn delay) + // Re-check session hasn't become active (user may have typed during spawn delay). + // IDLE and ENDED are safe — only ACTIVE/ACTIVE_COMMITTED should block. if state != nil && state.SessionID != "" { - if !isSessionIdle(repoRoot, state.SessionID) { - wingmanLog("session became active during spawn, aborting (next stop hook will retry)") - return nil + sessDir := findSessionStateDir(repoRoot) + if sessDir != "" { + phase := readSessionPhase(sessDir, state.SessionID) + if phase != "" && session.Phase(phase).IsActive() { + wingmanLog("session is active (phase=%s), aborting (next stop hook will retry)", phase) + return nil + } + wingmanLog("session phase=%s, safe to proceed", phase) } } diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index 7af25168f..93ba206c4 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -4,6 +4,7 @@ package cli import ( "context" + "fmt" "io" "os" "os/exec" @@ -56,12 +57,16 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { // Start the process (non-blocking) if err := cmd.Start(); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Failed to spawn review subprocess: %v\n", err) if logFile != nil { _ = logFile.Close() } return } + pid := cmd.Process.Pid + fmt.Fprintf(os.Stderr, "[wingman] Review subprocess spawned (pid=%d)\n", pid) + // Release the process so it can run independently //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() @@ -103,12 +108,16 @@ func spawnDetachedWingmanApply(repoRoot string) { } if err := cmd.Start(); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Failed to spawn apply subprocess: %v\n", err) if applyLogFile != nil { _ = applyLogFile.Close() } return } + pid := cmd.Process.Pid + fmt.Fprintf(os.Stderr, "[wingman] Apply subprocess spawned (pid=%d)\n", pid) + //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() } diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 3cc3b2ca9..71783fff6 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -730,3 +730,247 @@ func TestShouldSkipPendingReview_OrphanNoState(t *testing.T) { t.Error("orphan REVIEW.md should have been deleted") } } + +func TestHasAnyLiveSession_StaleSessionSkipped(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create an ACTIVE session file with a very old modification time + sessFile := filepath.Join(sessDir, "stale-active.json") + if err := os.WriteFile(sessFile, []byte(`{"phase":"active"}`), 0o644); err != nil { + t.Fatal(err) + } + // Set modification time to 3 hours ago (beyond staleSessionThreshold) + staleTime := time.Now().Add(-3 * time.Hour) + if err := os.Chtimes(sessFile, staleTime, staleTime); err != nil { + t.Fatal(err) + } + + if hasAnyLiveSession(tmpDir) { + t.Error("should return false when only session is stale (3h old)") + } +} + +func TestHasAnyLiveSession_FreshSessionNotSkipped(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create a stale ACTIVE session (should be skipped) + staleFile := filepath.Join(sessDir, "stale.json") + if err := os.WriteFile(staleFile, []byte(`{"phase":"active_committed"}`), 0o644); err != nil { + t.Fatal(err) + } + staleTime := time.Now().Add(-3 * time.Hour) + if err := os.Chtimes(staleFile, staleTime, staleTime); err != nil { + t.Fatal(err) + } + + // Create a fresh IDLE session (should not be skipped) + freshFile := filepath.Join(sessDir, "fresh.json") + if err := os.WriteFile(freshFile, []byte(`{"phase":"idle"}`), 0o644); err != nil { + t.Fatal(err) + } + + if !hasAnyLiveSession(tmpDir) { + t.Error("should return true when a fresh live session exists alongside stale ones") + } +} + +func TestRunWingmanApply_EndedPhaseProceeds(t *testing.T) { + t.Parallel() + + // This test verifies that runWingmanApply does NOT abort when the session + // phase is ENDED (the bug fix). We can't test the full auto-apply (it + // spawns claude CLI), but we can verify it passes the phase check. + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create REVIEW.md + reviewPath := filepath.Join(entireDir, "REVIEW.md") + if err := os.WriteFile(reviewPath, []byte("review content"), 0o644); err != nil { + t.Fatal(err) + } + + // Create wingman state + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-ended", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + // Create session state dir with ENDED phase + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-ended.json"), []byte(`{"phase":"ended"}`), 0o644); err != nil { + t.Fatal(err) + } + + // runWingmanApply will pass the phase check but fail at triggerAutoApply + // (no claude CLI). The important thing is it doesn't return nil early + // with "session became active" — it should get past the phase check. + err := runWingmanApply(tmpDir) + if err == nil { + t.Log("runWingmanApply returned nil (auto-apply succeeded or no claude CLI)") + } else { + // Expected: fails at triggerAutoApply because claude CLI isn't available + if strings.Contains(err.Error(), "session became active") { + t.Error("should not abort with 'session became active' for ENDED phase") + } + t.Logf("runWingmanApply failed as expected (no claude CLI): %v", err) + } + + // Verify apply was attempted (state should be updated) + state := loadWingmanStateDirect(tmpDir) + if state == nil { + t.Fatal("expected wingman state to exist") + } + if state.ApplyAttemptedAt == nil { + t.Error("ApplyAttemptedAt should be set after passing phase check") + } +} + +func TestRunWingmanApply_ActivePhaseAborts(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create REVIEW.md + reviewPath := filepath.Join(entireDir, "REVIEW.md") + if err := os.WriteFile(reviewPath, []byte("review content"), 0o644); err != nil { + t.Fatal(err) + } + + // Create wingman state + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-active", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + // Create session state dir with ACTIVE phase + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-active.json"), []byte(`{"phase":"active"}`), 0o644); err != nil { + t.Fatal(err) + } + + // runWingmanApply should return nil (aborted) without attempting apply + err := runWingmanApply(tmpDir) + if err != nil { + t.Errorf("expected nil error (clean abort), got: %v", err) + } + + // Verify apply was NOT attempted + state := loadWingmanStateDirect(tmpDir) + if state != nil && state.ApplyAttemptedAt != nil { + t.Error("ApplyAttemptedAt should not be set when phase is ACTIVE") + } +} + +func TestRunWingmanApply_ActiveCommittedPhaseAborts(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create REVIEW.md + if err := os.WriteFile(filepath.Join(entireDir, "REVIEW.md"), []byte("review"), 0o644); err != nil { + t.Fatal(err) + } + + // Create wingman state + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-ac", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + // Create session state dir with ACTIVE_COMMITTED phase + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-ac.json"), []byte(`{"phase":"active_committed"}`), 0o644); err != nil { + t.Fatal(err) + } + + err := runWingmanApply(tmpDir) + if err != nil { + t.Errorf("expected nil error (clean abort), got: %v", err) + } + + // Verify apply was NOT attempted + state := loadWingmanStateDirect(tmpDir) + if state != nil && state.ApplyAttemptedAt != nil { + t.Error("ApplyAttemptedAt should not be set when phase is ACTIVE_COMMITTED") + } +} + +func TestRunWingmanApply_IdlePhaseProceeds(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + entireDir := filepath.Join(tmpDir, ".entire") + if err := os.MkdirAll(entireDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create REVIEW.md + if err := os.WriteFile(filepath.Join(entireDir, "REVIEW.md"), []byte("review"), 0o644); err != nil { + t.Fatal(err) + } + + // Create wingman state + saveWingmanStateDirect(tmpDir, &WingmanState{ + SessionID: "sess-idle", + FilesHash: "hash1", + ReviewedAt: time.Now(), + }) + + // Create session state dir with IDLE phase + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sessDir, "sess-idle.json"), []byte(`{"phase":"idle"}`), 0o644); err != nil { + t.Fatal(err) + } + + // Should pass phase check (IDLE is safe) then fail at triggerAutoApply + err := runWingmanApply(tmpDir) + + // Verify apply was attempted (passes the phase check) + state := loadWingmanStateDirect(tmpDir) + if state == nil { + t.Fatal("expected wingman state to exist") + } + if state.ApplyAttemptedAt == nil { + t.Error("ApplyAttemptedAt should be set after passing phase check") + } + // We expect an error from triggerAutoApply (no claude CLI), that's fine + _ = err +} From ff06729236bb671e883f3ecd1548cd46046eb5f8 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:14:52 +0100 Subject: [PATCH 21/32] Add wingman status notifications visible in agent terminal Wingman status messages (review in progress, review pending) previously only went to stderr, which is invisible in Claude Code's UI. Use the new outputHookMessage helper to emit systemMessage-only JSON responses so users can see what wingman is doing in their terminal. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 3cbad94bdc0c --- cmd/entire/cli/hooks.go | 12 +++++++ cmd/entire/cli/hooks_claudecode_handlers.go | 36 +++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 8914a2ed7..d1e520acb 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -331,6 +331,18 @@ func outputHookResponse(context string) error { return nil } +// outputHookMessage outputs a JSON response with only a systemMessage — shown +// to the user in the terminal but NOT injected into the agent's conversation. +// Use this for informational notifications (e.g., wingman status) that the user +// should see but the agent should not act on. +func outputHookMessage(message string) error { + resp := hookResponse{SystemMessage: message} + if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { + return fmt.Errorf("failed to encode hook response: %w", err) + } + return nil +} + // outputHookResponseWithContextAndMessage outputs a JSON response with both // additionalContext (injected into agent conversation) and a systemMessage // (shown to the user as a warning/info). diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index dcd3e4119..0063a13db 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -100,13 +100,17 @@ func captureInitialState() error { } } - // Log if a review is currently in progress (lock file exists) + // Notify if a review is currently in progress (lock file exists). + // outputHookMessage writes JSON to stdout; session initialization + // below only touches disk/stderr, so there's no double-write risk. lockPath := filepath.Join(repoRoot, wingmanLockFile) if _, statErr := os.Stat(lockPath); statErr == nil { - fmt.Fprintf(os.Stderr, "[wingman] Review in progress...\n") logging.Info(wingmanLogCtx, "wingman review in progress", slog.String("session_id", hookData.sessionID), ) + if err := outputHookMessage("[Wingman] Review in progress..."); err != nil { + fmt.Fprintf(os.Stderr, "[wingman] Warning: failed to output review-in-progress message: %v\n", err) + } } } } @@ -299,6 +303,7 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba if err := CleanupPrePromptState(sessionID); err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to cleanup pre-prompt state: %v\n", err) } + outputWingmanStopNotification(repoRoot) return nil } @@ -446,6 +451,8 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba fmt.Fprintf(os.Stderr, "Warning: failed to cleanup pre-prompt state: %v\n", err) } + outputWingmanStopNotification(repoRoot) + return nil } @@ -803,6 +810,31 @@ func handleClaudeCodeSessionEnd() error { return nil } +// outputWingmanStopNotification outputs a systemMessage notification about +// wingman status at the end of a stop hook. This makes wingman activity visible +// in the agent terminal without injecting context into the agent's conversation. +// Best-effort: status may be stale due to concurrent wingman processes. +func outputWingmanStopNotification(repoRoot string) { + if !settings.IsWingmanEnabled() { + return + } + if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { + return + } + + lockPath := filepath.Join(repoRoot, wingmanLockFile) + if _, err := os.Stat(lockPath); err == nil { + _ = outputHookMessage("[Wingman] Reviewing your changes...") //nolint:errcheck // best-effort notification + return + } + + reviewPath := filepath.Join(repoRoot, wingmanReviewFile) + if _, err := os.Stat(reviewPath); err == nil { + _ = outputHookMessage("[Wingman] Review pending \u2014 will be addressed on your next prompt") //nolint:errcheck // best-effort notification + return + } +} + // transitionSessionTurnEnd fires EventTurnEnd to move the session from // ACTIVE → IDLE (or ACTIVE_COMMITTED → IDLE). Best-effort: logs warnings // on failure rather than returning errors. From 118936db70582a5e227cb6df8ffb87901e2f725e Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:31:02 +0100 Subject: [PATCH 22/32] Address PR review comments and update wingman documentation Code fixes from PR review: - Fix hasAnyLiveSession to return true when maxCheck limit hit (safer to assume a live session exists than to incorrectly auto-apply) - Return error when fallback git diff HEAD~1 fails instead of silently returning empty string - Remove unused baseCommit parameter from computeDiff Documentation updates: - Add user-visible messages table documenting all systemMessage notifications - Fix inaccurate claim that detached process strips GIT_* vars (it's the Claude CLI calls that strip them via wingmanStripGitEnv) - Remove redundant Decision Flow diagram and Prompt Structure box - Consolidate Intent-Aware Review into Review Prompt Construction intro Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 64df6ebeea39 --- cmd/entire/cli/wingman_review.go | 56 ++++++++------ docs/architecture/wingman.md | 126 +++++++------------------------ 2 files changed, 58 insertions(+), 124 deletions(-) diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 913f5c418..b6a666b00 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -101,9 +101,9 @@ func runWingmanReview(payloadPath string) error { time.Sleep(wingmanInitialDelay) // Compute diff using the base commit captured at trigger time - wingmanLog("computing diff against %s", payload.BaseCommit) + wingmanLog("computing diff (merge-base with main/master)") diffStart := time.Now() - diff, err := computeDiff(repoRoot, payload.BaseCommit) + diff, err := computeDiff(repoRoot) if err != nil { wingmanLog("ERROR computing diff: %v", err) return fmt.Errorf("failed to compute diff: %w", err) @@ -208,7 +208,7 @@ func runWingmanReview(payloadPath string) error { // computeDiff gets the full branch diff for review. It diffs the current HEAD // against the merge base with the default branch (main/master), giving the // reviewer a holistic view of all branch changes rather than just one commit. -func computeDiff(repoRoot string, _ string) (string, error) { +func computeDiff(repoRoot string) (string, error) { ctx, cancel := context.WithTimeout(context.Background(), wingmanGitTimeout) defer cancel() @@ -234,7 +234,7 @@ func computeDiff(repoRoot string, _ string) (string, error) { if strings.TrimSpace(diff) == "" { diff, err = gitDiff(ctx, repoRoot, "HEAD~1", "HEAD") if err != nil { - return "", nil + return "", fmt.Errorf("git diff for latest commit failed: %w", err) } } @@ -334,18 +334,20 @@ func readSessionContext(repoRoot, sessionID string) string { return string(data) } -// staleSessionThreshold is the maximum age of a session state file before it -// is considered stale and ignored by hasAnyLiveSession. This prevents orphaned -// session files (e.g., from crashed processes) from permanently blocking auto-apply. -const staleSessionThreshold = 2 * time.Hour +// staleActiveSessionThreshold is the maximum age of a session state file in an +// ACTIVE phase before it is considered stale (crashed agent) and ignored by +// hasAnyLiveSession. Only applies to ACTIVE/ACTIVE_COMMITTED phases — an IDLE +// session is always considered live regardless of age (user may just be away). +const staleActiveSessionThreshold = 4 * time.Hour // hasAnyLiveSession checks if any session is in a non-ENDED phase (IDLE, // ACTIVE, or ACTIVE_COMMITTED). Used to decide whether to defer review // application to the prompt-submit injection (visible to user) vs background // auto-apply (invisible). // -// Session state files older than staleSessionThreshold are skipped to prevent -// orphaned sessions from permanently blocking auto-apply. +// ACTIVE/ACTIVE_COMMITTED sessions older than staleActiveSessionThreshold are +// skipped as likely orphaned from crashed processes. IDLE sessions are always +// considered live regardless of age. func hasAnyLiveSession(repoRoot string) bool { sessDir := findSessionStateDir(repoRoot) if sessDir == "" { @@ -361,7 +363,8 @@ func hasAnyLiveSession(repoRoot string) bool { checked := 0 for _, entry := range entries { if checked >= maxCheck { - return false + wingmanLog("stopping live-session scan after %d entries; assuming live session may exist", checked) + return true } name := entry.Name() if !strings.HasSuffix(name, ".json") { @@ -369,23 +372,28 @@ func hasAnyLiveSession(repoRoot string) bool { } checked++ - // Skip stale session files that haven't been updated recently. - // These are likely orphaned from crashed processes. - info, statErr := entry.Info() - if statErr != nil { - continue - } - if time.Since(info.ModTime()) > staleSessionThreshold { - wingmanLog("skipping stale session file %s (age=%s)", name, time.Since(info.ModTime()).Round(time.Second)) + sid := strings.TrimSuffix(name, ".json") + phase := readSessionPhase(sessDir, sid) + if phase == "" || phase == string(session.PhaseEnded) { continue } - sid := strings.TrimSuffix(name, ".json") - phase := readSessionPhase(sessDir, sid) - if phase != "" && phase != string(session.PhaseEnded) { - wingmanLog("found live session %s (phase=%s)", sid, phase) - return true + // ACTIVE/ACTIVE_COMMITTED sessions that haven't been updated in a + // long time are likely orphaned from crashed agents — skip them. + // IDLE sessions are always considered live (user may just be away). + if session.Phase(phase).IsActive() { + info, statErr := entry.Info() + if statErr != nil { + continue + } + if time.Since(info.ModTime()) > staleActiveSessionThreshold { + wingmanLog("skipping stale active session %s (phase=%s, age=%s)", sid, phase, time.Since(info.ModTime()).Round(time.Second)) + continue + } } + + wingmanLog("found live session %s (phase=%s)", sid, phase) + return true } return false diff --git a/docs/architecture/wingman.md b/docs/architecture/wingman.md index f1aad4940..82d14adcc 100644 --- a/docs/architecture/wingman.md +++ b/docs/architecture/wingman.md @@ -18,7 +18,7 @@ When enabled, wingman runs a background review after each commit (or checkpoint) | Instruction | `wingman_instruction.md` | Embedded instruction injected into agent context | | Process spawning | `wingman_spawn_unix.go` | Detached subprocess spawning (Unix) | | Process spawning | `wingman_spawn_other.go` | No-op stubs (non-Unix) | -| Hook integration | `hooks_claudecode_handlers.go` | Prompt-submit injection, stop hook trigger, session-end trigger | +| Hook integration | `hooks_claudecode_handlers.go` | Prompt-submit injection, stop hook notifications, session-end trigger | ### State Files @@ -64,7 +64,7 @@ The review runs in a fully detached subprocess (`entire wingman __review → Verify REVIEW.md exists → Check ApplyAttemptedAt not set (retry prevention) - → Re-check session idle (guard against race) + → Re-check session phase is not ACTIVE/ACTIVE_COMMITTED (guard against race) → claude --continue --print --permission-mode acceptEdits ``` -This path is **invisible** — it runs silently. It exists as a fallback for when no session will receive the injection (e.g., user closed all sessions during the review window). +This path is **invisible** — it runs silently. It exists as a fallback for when no session will receive the injection (e.g., user closed all sessions during the review window). Both IDLE and ENDED phases are considered safe for auto-apply — only truly active sessions (ACTIVE/ACTIVE_COMMITTED) block it. -### Decision Flow - -``` - REVIEW.md written - │ - ▼ - ┌─────────────────┐ - │ Any live session │ - │ exists? │ - └────────┬────────┘ - │ │ - Yes No - │ │ - ▼ ▼ - ┌──────────┐ ┌──────────────┐ - │ Defer │ │ Background │ - │ to next │ │ auto-apply │ - │ prompt │ │ immediately │ - └──────────┘ └──────────────┘ - │ - ▼ - User sends prompt - │ - ▼ - additionalContext - injection fires - │ - ▼ - Agent applies review - (visible in terminal) -``` - -### Trigger Points Summary +### Trigger Points | Trigger | When | What Happens | |---------|------|-------------| @@ -150,6 +107,19 @@ This path is **invisible** — it runs silently. It exists as a fallback for whe | **Stop hook** (`triggerWingmanAutoApplyIfPending`) | Agent turn ends | If REVIEW.md exists + no live sessions → spawn `__apply`. | | **Session-end hook** (`triggerWingmanAutoApplyOnSessionEnd`) | User closes session | If REVIEW.md exists + no remaining live sessions → spawn `__apply`. | +## User-Visible Messages + +Wingman outputs `systemMessage` notifications at key points so the user can see what wingman is doing in their agent terminal. These are informational only — they are NOT injected into the agent's context. + +| Message | Hook | Condition | Purpose | +|---------|------|-----------|---------| +| `[Wingman] A code review is pending and will be addressed before your request.` | Prompt-submit | REVIEW.md exists (with `additionalContext` injection) | Tells user the agent will apply a review first | +| `[Wingman] Review in progress...` | Prompt-submit | Lock file exists (no REVIEW.md) | Tells user a review is running in the background | +| `[Wingman] Reviewing your changes...` | Stop | Lock file exists | Tells user a review was triggered and is still running | +| `[Wingman] Review pending — will be addressed on your next prompt` | Stop | REVIEW.md exists (no lock file) | Tells user a completed review will be delivered next prompt | + +The prompt-submit REVIEW.md injection message is paired with `additionalContext` — the agent sees and acts on the review. All other messages use `outputHookMessage()` which emits `systemMessage`-only JSON (visible in terminal, invisible to agent). + ## Timing Typical timeline for a review cycle: @@ -168,12 +138,10 @@ The 10-second initial delay lets the agent turn fully settle before computing th ## Review Prompt Construction -The review prompt is the core of wingman's value. Unlike a naive diff-only review, it leverages Entire's checkpoint data to give the reviewer **full context about what the developer was trying to accomplish**. This enables intent-aware review — catching not just bugs, but misalignment between what was asked and what was built. +The review prompt leverages Entire's checkpoint data to give the reviewer **full context about what the developer was trying to accomplish**. This enables intent-aware review — catching not just bugs, but misalignment between what was asked and what was built. A reviewer that only sees the diff cannot evaluate whether the code matches the original request. ### Context Sources -The prompt is assembled from six sources, each contributing a different layer of understanding: - | Source | Origin | What It Provides | |--------|--------|-----------------| | **Developer prompts** | `prompt.txt` from checkpoint metadata | The original instructions given to the agent — the ground truth of intent | @@ -183,40 +151,6 @@ The prompt is assembled from six sources, each contributing a different layer of | **File list** | Payload from trigger | Which files changed and how (modified/new/deleted) | | **Branch diff** | `git diff` against merge-base | The actual code changes — computed against `main`/`master` for a holistic branch-level view | -### Prompt Structure - -``` -┌─ System Role ──────────────────────────────────────────┐ -│ "You are a senior code reviewer performing an │ -│ intent-aware review." │ -├─ Session Context ──────────────────────────────────────┤ -│ Developer's Prompts ... │ -│ Commit Message (plain text) │ -│ Session Context ... │ -│ Checkpoint File Paths (for deeper investigation) │ -├─ Code Changes ─────────────────────────────────────────┤ -│ Files changed: file.go (modified), ... │ -│ Diff ... │ -├─ Review Instructions ──────────────────────────────────┤ -│ Intent alignment (most important) │ -│ Correctness bugs, logic errors, races │ -│ Security injection, secrets, path traversal │ -│ Robustness edge cases, leaks, timeouts │ -│ Do NOT flag style, docs on clear code │ -│ Output format Markdown with severity levels │ -└────────────────────────────────────────────────────────┘ -``` - -### Intent-Aware Review - -The review instructions prioritize **intent alignment** as the most important category: - -1. Do the changes actually accomplish what the developer asked for? -2. Are there any prompts or requirements that were missed or only partially implemented? -3. Does the implementation match the stated approach in the session context? - -This is only possible because Entire captures the developer's prompts and session context as checkpoint metadata. A reviewer that only sees the diff cannot evaluate whether the code matches the original request. - ### Diff Strategy The diff is computed against the **merge-base** with `main`/`master`, not just the latest commit. This gives the reviewer a holistic view of all branch changes rather than a narrow single-commit diff. Fallback chain: @@ -228,17 +162,7 @@ The diff is computed against the **merge-base** with `main`/`master`, not just t ### Read-Only Tool Access -The reviewer Claude instance has access to `Read`, `Glob`, and `Grep` tools with `--permission-mode bypassPermissions`. This allows it to: - -- Read source files beyond the diff for additional context -- Search for related code patterns or usages -- Inspect the checkpoint metadata files referenced in the prompt - -Tools are restricted to read-only operations — the reviewer cannot modify files. - -### Truncation - -Diffs larger than 100KB are truncated to maintain review quality. Very large diffs tend to produce unfocused reviews. +The reviewer Claude instance has access to `Read`, `Glob`, and `Grep` tools with `--permission-mode bypassPermissions`. This allows it to read source files beyond the diff, search for related patterns, and inspect checkpoint metadata. Tools are restricted to read-only operations. ### Output Format @@ -246,7 +170,7 @@ The reviewer outputs structured Markdown with: - **Summary**: Does the change accomplish its goal? Overall quality assessment. - **Issues**: Each with severity (`CRITICAL`, `WARNING`, `SUGGESTION`), file path with line reference, description, and suggested fix. -This output is written directly to `.entire/REVIEW.md` and later read by the agent during delivery. +Diffs larger than 100KB are truncated to maintain review quality. The output is written directly to `.entire/REVIEW.md`. ## Stale Review Cleanup @@ -272,8 +196,9 @@ The `ApplyAttemptedAt` field in `WingmanState` prevents infinite auto-apply atte - **Lock file**: Atomic `O_CREATE|O_EXCL` prevents concurrent review spawns. Stale locks (>30 min) are auto-cleaned. - **Dedup hash**: SHA256 of sorted file paths prevents re-reviewing identical change sets. - **Detached processes**: Review and apply run in their own process groups (`Setpgid: true`), surviving parent exit. -- **GIT_* stripping**: Subprocesses strip all `GIT_*` env vars to prevent index corruption. +- **GIT_* stripping**: Claude CLI calls strip `GIT_*` env vars via `wingmanStripGitEnv()` to prevent index corruption. Applied in `callClaudeForReview()` and `triggerAutoApply()`, not at process spawn time. - **ENTIRE_WINGMAN_APPLY=1**: Set during auto-apply to prevent the post-commit hook from triggering another review (recursion prevention). +- **Stale session detection**: ACTIVE/ACTIVE_COMMITTED sessions not updated in 4+ hours are considered orphaned (crashed agent) and ignored by `hasAnyLiveSession`. IDLE sessions are always considered live regardless of age. ## Configuration @@ -306,3 +231,4 @@ Wingman state is stored in `.entire/settings.json`: | `wingmanApplyTimeout` | 15m | Timeout for auto-apply process | | `wingmanStaleReviewTTL` | 1h | Max age before review is cleaned up | | `staleLockThreshold` | 30m | Max age before lock is considered stale | +| `staleActiveSessionThreshold` | 4h | Max age before ACTIVE session is considered stale/orphaned | From 121565db2fb2d711cffa9c9ac321f6efb854f3c6 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:31:36 +0100 Subject: [PATCH 23/32] Restore prompt structure diagram in wingman docs Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: bb5c3ec3fd2b --- docs/architecture/wingman.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/architecture/wingman.md b/docs/architecture/wingman.md index 82d14adcc..b7e5cffca 100644 --- a/docs/architecture/wingman.md +++ b/docs/architecture/wingman.md @@ -151,6 +151,30 @@ The review prompt leverages Entire's checkpoint data to give the reviewer **full | **File list** | Payload from trigger | Which files changed and how (modified/new/deleted) | | **Branch diff** | `git diff` against merge-base | The actual code changes — computed against `main`/`master` for a holistic branch-level view | +### Prompt Structure + +``` +┌─ System Role ──────────────────────────────────────────┐ +│ "You are a senior code reviewer performing an │ +│ intent-aware review." │ +├─ Session Context ──────────────────────────────────────┤ +│ Developer's Prompts ... │ +│ Commit Message (plain text) │ +│ Session Context ... │ +│ Checkpoint File Paths (for deeper investigation) │ +├─ Code Changes ─────────────────────────────────────────┤ +│ Files changed: file.go (modified), ... │ +│ Diff ... │ +├─ Review Instructions ──────────────────────────────────┤ +│ Intent alignment (most important) │ +│ Correctness bugs, logic errors, races │ +│ Security injection, secrets, path traversal │ +│ Robustness edge cases, leaks, timeouts │ +│ Do NOT flag style, docs on clear code │ +│ Output format Markdown with severity levels │ +└────────────────────────────────────────────────────────┘ +``` + ### Diff Strategy The diff is computed against the **merge-base** with `main`/`master`, not just the latest commit. This gives the reviewer a holistic view of all branch changes rather than a narrow single-commit diff. Fallback chain: From 0825633afdbf131dc7ebc71c96aae2bf64bf7434 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:37:36 +0100 Subject: [PATCH 24/32] Skip wingman lock file notifications when lock is stale The "Reviewing your changes..." and "Review in progress..." messages fired on every stop/prompt-submit hook if a lock file existed, even if it was stale from a crashed review process. Now checks lock file age against staleLockThreshold (30min) before showing the notification. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 6695a23da010 --- cmd/entire/cli/hooks_claudecode_handlers.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 0063a13db..4713d7908 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -100,11 +100,13 @@ func captureInitialState() error { } } - // Notify if a review is currently in progress (lock file exists). + // Notify if a review is currently in progress (fresh lock file). // outputHookMessage writes JSON to stdout; session initialization // below only touches disk/stderr, so there's no double-write risk. + // Stale locks (>staleLockThreshold) are ignored — the review process + // likely crashed and the lock was never cleaned up. lockPath := filepath.Join(repoRoot, wingmanLockFile) - if _, statErr := os.Stat(lockPath); statErr == nil { + if lockInfo, statErr := os.Stat(lockPath); statErr == nil && time.Since(lockInfo.ModTime()) <= staleLockThreshold { logging.Info(wingmanLogCtx, "wingman review in progress", slog.String("session_id", hookData.sessionID), ) @@ -823,7 +825,7 @@ func outputWingmanStopNotification(repoRoot string) { } lockPath := filepath.Join(repoRoot, wingmanLockFile) - if _, err := os.Stat(lockPath); err == nil { + if info, err := os.Stat(lockPath); err == nil && time.Since(info.ModTime()) <= staleLockThreshold { _ = outputHookMessage("[Wingman] Reviewing your changes...") //nolint:errcheck // best-effort notification return } From d2ef7835e3b1136fe7717353578870f80e1e1fd7 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:40:26 +0100 Subject: [PATCH 25/32] Allow strategy.Strategy in ireturn lint config CI's golangci-lint v2.8.0 flags functions returning the Strategy interface. Add it to the ireturn allow list alongside agent.Agent. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 02f99bbe85ed --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index f20c1a11b..0bf4d0f02 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -103,6 +103,7 @@ linters: - stdlib - grpc.DialOption - github.com/entireio/cli/cmd/entire/cli/agent.Agent + - github.com/entireio/cli/cmd/entire/cli/strategy.Strategy - github.com/go-git/go-git/v6/plumbing/storer.ReferenceIter - github.com/go-git/go-git/v6/plumbing.EncodedObject - github.com/go-git/go-git/v6/storage.Storer From 6808d513892776e5d8e3c8b7d3b2f9cb9097e639 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:46:26 +0100 Subject: [PATCH 26/32] Use tighter lock threshold for wingman status notifications The 30-minute staleLockThreshold is for lock acquisition (overwriting stale locks). For notifications, a much tighter 10-minute window is appropriate since a real review takes at most ~6 minutes (10s delay + 5min API timeout). This prevents showing "Reviewing your changes..." for lock files that are clearly stale. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: f3e92b9f757a --- cmd/entire/cli/hooks_claudecode_handlers.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 4713d7908..5d1caa984 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -103,10 +103,10 @@ func captureInitialState() error { // Notify if a review is currently in progress (fresh lock file). // outputHookMessage writes JSON to stdout; session initialization // below only touches disk/stderr, so there's no double-write risk. - // Stale locks (>staleLockThreshold) are ignored — the review process - // likely crashed and the lock was never cleaned up. + // Uses wingmanNotificationLockThreshold (10min) — tighter than the + // 30min staleLockThreshold used for lock acquisition. lockPath := filepath.Join(repoRoot, wingmanLockFile) - if lockInfo, statErr := os.Stat(lockPath); statErr == nil && time.Since(lockInfo.ModTime()) <= staleLockThreshold { + if lockInfo, statErr := os.Stat(lockPath); statErr == nil && time.Since(lockInfo.ModTime()) <= wingmanNotificationLockThreshold { logging.Info(wingmanLogCtx, "wingman review in progress", slog.String("session_id", hookData.sessionID), ) @@ -812,6 +812,13 @@ func handleClaudeCodeSessionEnd() error { return nil } +// wingmanNotificationLockThreshold is the maximum lock file age for showing +// "Reviewing your changes..." notifications. Much tighter than staleLockThreshold +// (used for lock acquisition) because a real review takes at most +// wingmanInitialDelay (10s) + wingmanReviewTimeout (5m) ≈ 6 minutes. +// A lock older than this is almost certainly stale for notification purposes. +const wingmanNotificationLockThreshold = 10 * time.Minute + // outputWingmanStopNotification outputs a systemMessage notification about // wingman status at the end of a stop hook. This makes wingman activity visible // in the agent terminal without injecting context into the agent's conversation. @@ -825,7 +832,7 @@ func outputWingmanStopNotification(repoRoot string) { } lockPath := filepath.Join(repoRoot, wingmanLockFile) - if info, err := os.Stat(lockPath); err == nil && time.Since(info.ModTime()) <= staleLockThreshold { + if info, err := os.Stat(lockPath); err == nil && time.Since(info.ModTime()) <= wingmanNotificationLockThreshold { _ = outputHookMessage("[Wingman] Reviewing your changes...") //nolint:errcheck // best-effort notification return } From 69a35cf4c757d4a1e03121809fd26b5165279fe1 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 16:48:54 +0100 Subject: [PATCH 27/32] Clean up ireturn nolint, add spawn comments, update stale session tests - Remove obsolete //nolint:ireturn on GetStrategy (strategy.Strategy is now in the ireturn allow list) - Add comments explaining log file handle inheritance in spawn functions - Update hasAnyLiveSession tests: rename for clarity, adjust thresholds to 5h (beyond staleActiveSessionThreshold of 4h), add test verifying stale IDLE sessions are still considered live Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 0fc364a22bea --- cmd/entire/cli/config.go | 2 +- cmd/entire/cli/wingman_spawn_unix.go | 6 +++++ cmd/entire/cli/wingman_test.go | 39 +++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/config.go b/cmd/entire/cli/config.go index 48246c5be..6eb704cbb 100644 --- a/cmd/entire/cli/config.go +++ b/cmd/entire/cli/config.go @@ -65,7 +65,7 @@ func IsEnabled() (bool, error) { // GetStrategy returns the configured strategy instance. // Falls back to default if the configured strategy is not found. // -//nolint:ireturn // Factory pattern requires returning the interface + func GetStrategy() strategy.Strategy { s, err := settings.Load() if err != nil { diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index 93ba206c4..3adc498db 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -67,6 +67,9 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { pid := cmd.Process.Pid fmt.Fprintf(os.Stderr, "[wingman] Review subprocess spawned (pid=%d)\n", pid) + // The log file handle is inherited by the child process and will be + // closed when that process exits. Don't close it here. + // Release the process so it can run independently //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() @@ -118,6 +121,9 @@ func spawnDetachedWingmanApply(repoRoot string) { pid := cmd.Process.Pid fmt.Fprintf(os.Stderr, "[wingman] Apply subprocess spawned (pid=%d)\n", pid) + // The log file handle is inherited by the child process and will be + // closed when that process exits. Don't close it here. + //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() } diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 71783fff6..ed6ad4935 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -731,7 +731,7 @@ func TestShouldSkipPendingReview_OrphanNoState(t *testing.T) { } } -func TestHasAnyLiveSession_StaleSessionSkipped(t *testing.T) { +func TestHasAnyLiveSession_StaleActiveSessionSkipped(t *testing.T) { t.Parallel() tmpDir := t.TempDir() @@ -745,18 +745,18 @@ func TestHasAnyLiveSession_StaleSessionSkipped(t *testing.T) { if err := os.WriteFile(sessFile, []byte(`{"phase":"active"}`), 0o644); err != nil { t.Fatal(err) } - // Set modification time to 3 hours ago (beyond staleSessionThreshold) - staleTime := time.Now().Add(-3 * time.Hour) + // Set modification time to 5 hours ago (beyond staleActiveSessionThreshold) + staleTime := time.Now().Add(-5 * time.Hour) if err := os.Chtimes(sessFile, staleTime, staleTime); err != nil { t.Fatal(err) } if hasAnyLiveSession(tmpDir) { - t.Error("should return false when only session is stale (3h old)") + t.Error("should return false when only active session is stale (5h old)") } } -func TestHasAnyLiveSession_FreshSessionNotSkipped(t *testing.T) { +func TestHasAnyLiveSession_StaleIdleSessionNotSkipped(t *testing.T) { t.Parallel() tmpDir := t.TempDir() @@ -765,12 +765,37 @@ func TestHasAnyLiveSession_FreshSessionNotSkipped(t *testing.T) { t.Fatal(err) } - // Create a stale ACTIVE session (should be skipped) + // Create an IDLE session file with an old modification time. + // IDLE sessions should always count as live (user may just be away). + sessFile := filepath.Join(sessDir, "old-idle.json") + if err := os.WriteFile(sessFile, []byte(`{"phase":"idle"}`), 0o644); err != nil { + t.Fatal(err) + } + oldTime := time.Now().Add(-5 * time.Hour) + if err := os.Chtimes(sessFile, oldTime, oldTime); err != nil { + t.Fatal(err) + } + + if !hasAnyLiveSession(tmpDir) { + t.Error("should return true for IDLE session regardless of age (user may be away)") + } +} + +func TestHasAnyLiveSession_FreshActiveNotSkipped(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create a stale ACTIVE_COMMITTED session (should be skipped) staleFile := filepath.Join(sessDir, "stale.json") if err := os.WriteFile(staleFile, []byte(`{"phase":"active_committed"}`), 0o644); err != nil { t.Fatal(err) } - staleTime := time.Now().Add(-3 * time.Hour) + staleTime := time.Now().Add(-5 * time.Hour) if err := os.Chtimes(staleFile, staleTime, staleTime); err != nil { t.Fatal(err) } From e794c911ceea566c9e023c1f54133260ee257801 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 17:01:29 +0100 Subject: [PATCH 28/32] Show pending review status in SessionStart wingman message Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 0fc364a22bea --- cmd/entire/cli/hooks.go | 14 ++++++++++++-- cmd/entire/cli/wingman_test.go | 10 +++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index d1e520acb..f3f24de0d 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -8,9 +8,11 @@ import ( "io" "log/slog" "os" + "path/filepath" "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/session" "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/strategy" @@ -266,9 +268,17 @@ func handleSessionStartCommon() error { // Build informational message message := "\n\nPowered by Entire:\n This conversation will be linked to your next commit." - // Append wingman note if enabled + // Append wingman note if enabled, with pending review indication if settings.IsWingmanEnabled() { - message += "\n Wingman is active: your changes will be automatically reviewed." + if repoRoot, rootErr := paths.RepoRoot(); rootErr == nil { + if _, statErr := os.Stat(filepath.Join(repoRoot, wingmanReviewFile)); statErr == nil { + message += "\n Wingman: a review is pending and will be addressed on your next prompt." + } else { + message += "\n Wingman is active: your changes will be automatically reviewed." + } + } else { + message += "\n Wingman is active: your changes will be automatically reviewed." + } } // Check for concurrent sessions and append count if any diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index ed6ad4935..4571539c6 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -745,14 +745,14 @@ func TestHasAnyLiveSession_StaleActiveSessionSkipped(t *testing.T) { if err := os.WriteFile(sessFile, []byte(`{"phase":"active"}`), 0o644); err != nil { t.Fatal(err) } - // Set modification time to 5 hours ago (beyond staleActiveSessionThreshold) - staleTime := time.Now().Add(-5 * time.Hour) + // Set modification time beyond staleActiveSessionThreshold + staleTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) if err := os.Chtimes(sessFile, staleTime, staleTime); err != nil { t.Fatal(err) } if hasAnyLiveSession(tmpDir) { - t.Error("should return false when only active session is stale (5h old)") + t.Error("should return false when only active session is beyond staleActiveSessionThreshold") } } @@ -771,7 +771,7 @@ func TestHasAnyLiveSession_StaleIdleSessionNotSkipped(t *testing.T) { if err := os.WriteFile(sessFile, []byte(`{"phase":"idle"}`), 0o644); err != nil { t.Fatal(err) } - oldTime := time.Now().Add(-5 * time.Hour) + oldTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) if err := os.Chtimes(sessFile, oldTime, oldTime); err != nil { t.Fatal(err) } @@ -795,7 +795,7 @@ func TestHasAnyLiveSession_FreshActiveNotSkipped(t *testing.T) { if err := os.WriteFile(staleFile, []byte(`{"phase":"active_committed"}`), 0o644); err != nil { t.Fatal(err) } - staleTime := time.Now().Add(-5 * time.Hour) + staleTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) if err := os.Chtimes(staleFile, staleTime, staleTime); err != nil { t.Fatal(err) } From 0ce3f3f850592bba9710f1f0ec7a723d8be4bc08 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 17:15:52 +0100 Subject: [PATCH 29/32] Fix stale active sessions blocking wingman auto-apply PostCommit processes all session state files on every commit, which refreshed both file modtime and LastInteractionTime for sessions stuck in ACTIVE_COMMITTED phase. This prevented hasAnyLiveSession from detecting truly stale sessions, blocking background auto-apply. Two fixes: - Remove ActionUpdateLastInteraction from ACTIVE_COMMITTED + GitCommit self-loop (a commit is not proof the agent is alive) - Use LastInteractionTime from JSON instead of file modtime for staleness checks in hasAnyLiveSession Also adds readSessionContext warning log for non-ErrNotExist failures. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 0fc364a22bea --- cmd/entire/cli/session/phase.go | 6 ++- cmd/entire/cli/session/phase_test.go | 4 +- cmd/entire/cli/wingman_review.go | 55 ++++++++++++++++----------- cmd/entire/cli/wingman_test.go | 57 +++++++++++++++++++--------- 4 files changed, 79 insertions(+), 43 deletions(-) diff --git a/cmd/entire/cli/session/phase.go b/cmd/entire/cli/session/phase.go index f89975849..b5b6d7751 100644 --- a/cmd/entire/cli/session/phase.go +++ b/cmd/entire/cli/session/phase.go @@ -239,9 +239,13 @@ func transitionFromActiveCommitted(event Event, ctx TransitionContext) Transitio if ctx.IsRebaseInProgress { return TransitionResult{NewPhase: PhaseActiveCommitted} } + // Don't update LastInteractionTime on self-loop: a git commit is not + // proof the agent is alive. Only TurnStart/TurnEnd prove liveness. + // Without this, PostCommit refreshes all sessions on every commit, + // preventing hasAnyLiveSession from detecting truly stale sessions. return TransitionResult{ NewPhase: PhaseActiveCommitted, - Actions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + Actions: []Action{ActionMigrateShadowBranch}, } case EventSessionStart: return TransitionResult{ diff --git a/cmd/entire/cli/session/phase_test.go b/cmd/entire/cli/session/phase_test.go index 009b874e9..edc04baf1 100644 --- a/cmd/entire/cli/session/phase_test.go +++ b/cmd/entire/cli/session/phase_test.go @@ -234,11 +234,11 @@ func TestTransitionFromActiveCommitted(t *testing.T) { wantActions: []Action{ActionCondense, ActionUpdateLastInteraction}, }, { - name: "GitCommit_stays_with_migrate", + name: "GitCommit_stays_with_migrate_no_interaction_update", current: PhaseActiveCommitted, event: EventGitCommit, wantPhase: PhaseActiveCommitted, - wantActions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + wantActions: []Action{ActionMigrateShadowBranch}, }, { name: "GitCommit_rebase_skips_everything", diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index b6a666b00..969cc6a27 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -329,6 +329,9 @@ func readSessionContext(repoRoot, sessionID string) string { contextPath := filepath.Join(repoRoot, ".entire", "metadata", sessionID, "context.md") data, err := os.ReadFile(contextPath) //nolint:gosec // path constructed from repoRoot + session ID if err != nil { + if !errors.Is(err, os.ErrNotExist) { + wingmanLog("WARNING: failed to read session context: %v", err) + } return "" } return string(data) @@ -373,26 +376,25 @@ func hasAnyLiveSession(repoRoot string) bool { checked++ sid := strings.TrimSuffix(name, ".json") - phase := readSessionPhase(sessDir, sid) - if phase == "" || phase == string(session.PhaseEnded) { + info := readSessionPhaseInfo(sessDir, sid) + if info.Phase == "" || info.Phase == string(session.PhaseEnded) { continue } - // ACTIVE/ACTIVE_COMMITTED sessions that haven't been updated in a - // long time are likely orphaned from crashed agents — skip them. + // ACTIVE/ACTIVE_COMMITTED sessions that haven't had a real agent + // interaction in a long time are likely orphaned from crashed agents. + // Uses LastInteractionTime from JSON (not file modtime) because + // PostCommit saves all session files on every commit, refreshing + // modtime even for stale sessions. // IDLE sessions are always considered live (user may just be away). - if session.Phase(phase).IsActive() { - info, statErr := entry.Info() - if statErr != nil { - continue - } - if time.Since(info.ModTime()) > staleActiveSessionThreshold { - wingmanLog("skipping stale active session %s (phase=%s, age=%s)", sid, phase, time.Since(info.ModTime()).Round(time.Second)) + if session.Phase(info.Phase).IsActive() { + if info.LastInteractionTime != nil && time.Since(*info.LastInteractionTime) > staleActiveSessionThreshold { + wingmanLog("skipping stale active session %s (phase=%s, last_interaction=%s ago)", sid, info.Phase, time.Since(*info.LastInteractionTime).Round(time.Second)) continue } } - wingmanLog("found live session %s (phase=%s)", sid, phase) + wingmanLog("found live session %s (phase=%s)", sid, info.Phase) return true } @@ -439,19 +441,28 @@ func findSessionStateDir(repoRoot string) string { return sessDir } -// readSessionPhase reads just the phase field from a session state JSON file. -func readSessionPhase(sessDir, sessionID string) string { +// sessionPhaseInfo holds the subset of session state needed for liveness checks. +type sessionPhaseInfo struct { + Phase string + LastInteractionTime *time.Time +} + +func readSessionPhaseInfo(sessDir, sessionID string) sessionPhaseInfo { data, err := os.ReadFile(filepath.Join(sessDir, sessionID+".json")) //nolint:gosec // sessDir is from git internals if err != nil { - return "" + return sessionPhaseInfo{} } var partial struct { - Phase string `json:"phase"` + Phase string `json:"phase"` + LastInteractionTime *time.Time `json:"last_interaction_time,omitempty"` } if json.Unmarshal(data, &partial) != nil { - return "" + return sessionPhaseInfo{} + } + return sessionPhaseInfo{ + Phase: partial.Phase, + LastInteractionTime: partial.LastInteractionTime, } - return partial.Phase } // runWingmanApply is the entrypoint for the __apply subcommand, spawned by the @@ -484,12 +495,12 @@ func runWingmanApply(repoRoot string) error { if state != nil && state.SessionID != "" { sessDir := findSessionStateDir(repoRoot) if sessDir != "" { - phase := readSessionPhase(sessDir, state.SessionID) - if phase != "" && session.Phase(phase).IsActive() { - wingmanLog("session is active (phase=%s), aborting (next stop hook will retry)", phase) + phaseInfo := readSessionPhaseInfo(sessDir, state.SessionID) + if phaseInfo.Phase != "" && session.Phase(phaseInfo.Phase).IsActive() { + wingmanLog("session is active (phase=%s), aborting (next stop hook will retry)", phaseInfo.Phase) return nil } - wingmanLog("session phase=%s, safe to proceed", phase) + wingmanLog("session phase=%s, safe to proceed", phaseInfo.Phase) } } diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 4571539c6..6b6c6d513 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "encoding/json" + "fmt" "os" "os/exec" "path/filepath" @@ -740,14 +741,12 @@ func TestHasAnyLiveSession_StaleActiveSessionSkipped(t *testing.T) { t.Fatal(err) } - // Create an ACTIVE session file with a very old modification time - sessFile := filepath.Join(sessDir, "stale-active.json") - if err := os.WriteFile(sessFile, []byte(`{"phase":"active"}`), 0o644); err != nil { - t.Fatal(err) - } - // Set modification time beyond staleActiveSessionThreshold + // Create an ACTIVE session with last_interaction_time beyond threshold. + // Uses JSON field (not file modtime) for staleness detection. staleTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) - if err := os.Chtimes(sessFile, staleTime, staleTime); err != nil { + sessData := fmt.Sprintf(`{"phase":"active","last_interaction_time":"%s"}`, staleTime.Format(time.RFC3339Nano)) + sessFile := filepath.Join(sessDir, "stale-active.json") + if err := os.WriteFile(sessFile, []byte(sessData), 0o644); err != nil { t.Fatal(err) } @@ -765,14 +764,12 @@ func TestHasAnyLiveSession_StaleIdleSessionNotSkipped(t *testing.T) { t.Fatal(err) } - // Create an IDLE session file with an old modification time. + // Create an IDLE session with an old last_interaction_time. // IDLE sessions should always count as live (user may just be away). - sessFile := filepath.Join(sessDir, "old-idle.json") - if err := os.WriteFile(sessFile, []byte(`{"phase":"idle"}`), 0o644); err != nil { - t.Fatal(err) - } oldTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) - if err := os.Chtimes(sessFile, oldTime, oldTime); err != nil { + sessData := fmt.Sprintf(`{"phase":"idle","last_interaction_time":"%s"}`, oldTime.Format(time.RFC3339Nano)) + sessFile := filepath.Join(sessDir, "old-idle.json") + if err := os.WriteFile(sessFile, []byte(sessData), 0o644); err != nil { t.Fatal(err) } @@ -791,12 +788,10 @@ func TestHasAnyLiveSession_FreshActiveNotSkipped(t *testing.T) { } // Create a stale ACTIVE_COMMITTED session (should be skipped) - staleFile := filepath.Join(sessDir, "stale.json") - if err := os.WriteFile(staleFile, []byte(`{"phase":"active_committed"}`), 0o644); err != nil { - t.Fatal(err) - } staleTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) - if err := os.Chtimes(staleFile, staleTime, staleTime); err != nil { + staleData := fmt.Sprintf(`{"phase":"active_committed","last_interaction_time":"%s"}`, staleTime.Format(time.RFC3339Nano)) + staleFile := filepath.Join(sessDir, "stale.json") + if err := os.WriteFile(staleFile, []byte(staleData), 0o644); err != nil { t.Fatal(err) } @@ -811,6 +806,32 @@ func TestHasAnyLiveSession_FreshActiveNotSkipped(t *testing.T) { } } +func TestHasAnyLiveSession_RecentModtimeButStaleInteraction(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + sessDir := filepath.Join(tmpDir, ".git", "entire-sessions") + if err := os.MkdirAll(sessDir, 0o755); err != nil { + t.Fatal(err) + } + + // Simulate the PostCommit bug: file modtime is recent (just written), + // but LastInteractionTime is old because no TurnStart/TurnEnd occurred. + // This reproduces the scenario where PostCommit saves stale sessions, + // refreshing modtime without updating LastInteractionTime. + staleTime := time.Now().Add(-staleActiveSessionThreshold - 1*time.Hour) + sessData := fmt.Sprintf(`{"phase":"active_committed","last_interaction_time":"%s"}`, staleTime.Format(time.RFC3339Nano)) + sessFile := filepath.Join(sessDir, "stale-but-recent-modtime.json") + if err := os.WriteFile(sessFile, []byte(sessData), 0o644); err != nil { + t.Fatal(err) + } + // File modtime is NOW (just created) — but LastInteractionTime is old. + + if hasAnyLiveSession(tmpDir) { + t.Error("should return false: LastInteractionTime is stale even though file modtime is recent") + } +} + func TestRunWingmanApply_EndedPhaseProceeds(t *testing.T) { t.Parallel() From 46815aebdf7db81aed85e09ea72fbadca80947d9 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Thu, 12 Feb 2026 18:16:08 +0100 Subject: [PATCH 30/32] Fix context param shadowing and log file FD leak in parent process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename 'context' parameter to 'additionalContext' in hook response functions to avoid shadowing the imported context package. Close the parent's copy of log file descriptors after spawning detached wingman subprocesses — the child already has its own copy via dup from Start(). Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 6a54c404f24f --- cmd/entire/cli/hooks.go | 10 +++++----- cmd/entire/cli/wingman_spawn_unix.go | 14 ++++++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index f3f24de0d..7050be113 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -327,12 +327,12 @@ type hookResponse struct { // outputHookResponse outputs a JSON response with additionalContext for // SessionStart hooks. The context is injected into the agent's conversation. -func outputHookResponse(context string) error { +func outputHookResponse(additionalContext string) error { resp := hookResponse{ - SystemMessage: context, + SystemMessage: additionalContext, HookSpecificOutput: &hookSpecificOutput{ HookEventName: "SessionStart", - AdditionalContext: context, + AdditionalContext: additionalContext, }, } if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { @@ -356,12 +356,12 @@ func outputHookMessage(message string) error { // outputHookResponseWithContextAndMessage outputs a JSON response with both // additionalContext (injected into agent conversation) and a systemMessage // (shown to the user as a warning/info). -func outputHookResponseWithContextAndMessage(context, message string) error { +func outputHookResponseWithContextAndMessage(additionalContext, message string) error { resp := hookResponse{ SystemMessage: message, HookSpecificOutput: &hookSpecificOutput{ HookEventName: "UserPromptSubmit", - AdditionalContext: context, + AdditionalContext: additionalContext, }, } if err := json.NewEncoder(os.Stdout).Encode(resp); err != nil { diff --git a/cmd/entire/cli/wingman_spawn_unix.go b/cmd/entire/cli/wingman_spawn_unix.go index 3adc498db..bffab08c6 100644 --- a/cmd/entire/cli/wingman_spawn_unix.go +++ b/cmd/entire/cli/wingman_spawn_unix.go @@ -67,8 +67,11 @@ func spawnDetachedWingmanReview(repoRoot, payloadPath string) { pid := cmd.Process.Pid fmt.Fprintf(os.Stderr, "[wingman] Review subprocess spawned (pid=%d)\n", pid) - // The log file handle is inherited by the child process and will be - // closed when that process exits. Don't close it here. + // Close the parent's copy of the log file descriptor. The child process + // received its own copy via dup during Start(), so this won't affect it. + if logFile != nil { + _ = logFile.Close() + } // Release the process so it can run independently //nolint:errcheck // Best effort - process should continue regardless @@ -121,8 +124,11 @@ func spawnDetachedWingmanApply(repoRoot string) { pid := cmd.Process.Pid fmt.Fprintf(os.Stderr, "[wingman] Apply subprocess spawned (pid=%d)\n", pid) - // The log file handle is inherited by the child process and will be - // closed when that process exits. Don't close it here. + // Close the parent's copy of the log file descriptor. The child process + // received its own copy via dup during Start(), so this won't affect it. + if applyLogFile != nil { + _ = applyLogFile.Close() + } //nolint:errcheck // Best effort - process should continue regardless _ = cmd.Process.Release() From 8161293d750436aa0325b967aa07f5e9362c286c Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Fri, 13 Feb 2026 09:48:34 +0100 Subject: [PATCH 31/32] fix: strip CLAUDECODE env var in wingman/summarize subprocess calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code sets CLAUDECODE in its environment, which causes nested claude CLI invocations (wingman review, auto-apply, summarize) to fail with "cannot be launched inside another Claude Code session". Strip CLAUDECODE alongside GIT_* vars in wingmanStripGitEnv() and summarize.stripGitEnv(). Also removes triggerWingmanAutoApplyOnSessionEnd — the session-end auto-apply path never worked reliably and is unnecessary since pending reviews are picked up by prompt-submit injection or go stale. Updates wingman architecture docs with --local flag, status output, hidden subcommands, missing constants, and Claude CLI invocation details. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 81de29e2290e --- cmd/entire/cli/hooks_claudecode_handlers.go | 45 ----------- cmd/entire/cli/hooks_geminicli_handlers.go | 5 -- cmd/entire/cli/summarize/claude.go | 11 ++- cmd/entire/cli/summarize/claude_test.go | 8 +- cmd/entire/cli/wingman_review.go | 12 +-- cmd/entire/cli/wingman_test.go | 6 +- docs/architecture/wingman.md | 84 +++++++++++++++++++-- 7 files changed, 102 insertions(+), 69 deletions(-) diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 7cb846013..ba29d45d8 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -795,11 +795,6 @@ func handleClaudeCodeSessionEnd() error { fmt.Fprintf(os.Stderr, "Warning: failed to mark session ended: %v\n", err) } - // If a wingman review is pending and this was the last live session, - // trigger background auto-apply. No more prompts will come from this - // session, so the prompt-submit injection can't deliver the review. - triggerWingmanAutoApplyOnSessionEnd() - return nil } @@ -908,46 +903,6 @@ func triggerWingmanAutoApplyIfPending(repoRoot string) { spawnDetachedWingmanApply(repoRoot) } -// triggerWingmanAutoApplyOnSessionEnd checks for a pending REVIEW.md after a -// session ends. If no live sessions remain, spawns background auto-apply. -// Called from both Claude Code and Gemini CLI SessionEnd hooks. -func triggerWingmanAutoApplyOnSessionEnd() { - logCtx := logging.WithComponent(context.Background(), "wingman") - if !settings.IsWingmanEnabled() { - logging.Debug(logCtx, "wingman auto-apply on session-end skip: wingman not enabled") - return - } - if os.Getenv("ENTIRE_WINGMAN_APPLY") != "" { - logging.Debug(logCtx, "wingman auto-apply on session-end skip: already in apply subprocess") - return - } - repoRoot, err := paths.RepoRoot() - if err != nil { - logging.Debug(logCtx, "wingman auto-apply on session-end skip: cannot resolve repo root") - return - } - reviewPath := filepath.Join(repoRoot, wingmanReviewFile) - if _, statErr := os.Stat(reviewPath); statErr != nil { - logging.Debug(logCtx, "wingman auto-apply on session-end skip: no REVIEW.md pending") - return - } - wingmanState := loadWingmanStateDirect(repoRoot) - if wingmanState != nil && wingmanState.ApplyAttemptedAt != nil { - logging.Debug(logCtx, "wingman auto-apply on session-end skip: already attempted", - slog.Time("attempted_at", *wingmanState.ApplyAttemptedAt), - ) - return - } - if hasAnyLiveSession(repoRoot) { - logging.Debug(logCtx, "wingman auto-apply deferred on session-end: other live sessions remain") - return - } - fmt.Fprintf(os.Stderr, "[wingman] Last session ended with pending review, spawning auto-apply\n") - logging.Info(logCtx, "wingman auto-apply on session-end (no live sessions remain)", - slog.String("review_path", reviewPath), - ) - spawnDetachedWingmanApply(repoRoot) -} // markSessionEnded transitions the session to ENDED phase via the state machine. func markSessionEnded(sessionID string) error { diff --git a/cmd/entire/cli/hooks_geminicli_handlers.go b/cmd/entire/cli/hooks_geminicli_handlers.go index 0cf5b1419..c6520179f 100644 --- a/cmd/entire/cli/hooks_geminicli_handlers.go +++ b/cmd/entire/cli/hooks_geminicli_handlers.go @@ -44,11 +44,6 @@ func handleGeminiSessionEnd() error { // Parse stdin once upfront — all subseq fmt.Fprintf(os.Stderr, "Warning: failed to mark session ended: %v\n", err) } - // If a wingman review is pending and this was the last live session, - // trigger background auto-apply. No more prompts will come from this - // session, so the prompt-submit injection can't deliver the review. - triggerWingmanAutoApplyOnSessionEnd() - return nil } diff --git a/cmd/entire/cli/summarize/claude.go b/cmd/entire/cli/summarize/claude.go index 7b2e6dd3e..6691bda68 100644 --- a/cmd/entire/cli/summarize/claude.go +++ b/cmd/entire/cli/summarize/claude.go @@ -159,14 +159,17 @@ func buildSummarizationPrompt(transcriptText string) string { return fmt.Sprintf(summarizationPromptTemplate, transcriptText) } -// stripGitEnv returns a copy of env with all GIT_* variables removed. -// This prevents a subprocess from discovering or modifying the parent's git repo. +// stripGitEnv returns a copy of env with all GIT_* variables removed and the +// CLAUDECODE variable unset. GIT_* removal prevents a subprocess from discovering +// or modifying the parent's git repo. CLAUDECODE removal prevents the Claude CLI +// from refusing to start due to nested-session detection. func stripGitEnv(env []string) []string { filtered := make([]string, 0, len(env)) for _, e := range env { - if !strings.HasPrefix(e, "GIT_") { - filtered = append(filtered, e) + if strings.HasPrefix(e, "GIT_") || strings.HasPrefix(e, "CLAUDECODE=") { + continue } + filtered = append(filtered, e) } return filtered } diff --git a/cmd/entire/cli/summarize/claude_test.go b/cmd/entire/cli/summarize/claude_test.go index 58eb40435..7922db4bb 100644 --- a/cmd/entire/cli/summarize/claude_test.go +++ b/cmd/entire/cli/summarize/claude_test.go @@ -26,6 +26,8 @@ func TestClaudeGenerator_GitIsolation(t *testing.T) { t.Setenv("GIT_DIR", "/some/repo/.git") t.Setenv("GIT_WORK_TREE", "/some/repo") t.Setenv("GIT_INDEX_FILE", "/some/repo/.git/index") + // Set CLAUDECODE which is set when running inside Claude Code + t.Setenv("CLAUDECODE", "1") input := Input{ Transcript: []Entry{ @@ -47,11 +49,14 @@ func TestClaudeGenerator_GitIsolation(t *testing.T) { t.Errorf("cmd.Dir = %q, want %q", capturedCmd.Dir, os.TempDir()) } - // Verify no GIT_* env vars in the command's environment + // Verify no GIT_* or CLAUDECODE env vars in the command's environment for _, env := range capturedCmd.Env { if strings.HasPrefix(env, "GIT_") { t.Errorf("found GIT_* env var in subprocess: %s", env) } + if strings.HasPrefix(env, "CLAUDECODE=") { + t.Errorf("found CLAUDECODE env var in subprocess: %s", env) + } } } @@ -63,6 +68,7 @@ func TestStripGitEnv(t *testing.T) { "GIT_WORK_TREE=/repo", "GIT_INDEX_FILE=/repo/.git/index", "SHELL=/bin/zsh", + "CLAUDECODE=1", } filtered := stripGitEnv(env) diff --git a/cmd/entire/cli/wingman_review.go b/cmd/entire/cli/wingman_review.go index 969cc6a27..827089a2b 100644 --- a/cmd/entire/cli/wingman_review.go +++ b/cmd/entire/cli/wingman_review.go @@ -551,15 +551,17 @@ func triggerAutoApply(repoRoot string) error { return cmd.Run() //nolint:wrapcheck // caller wraps } -// wingmanStripGitEnv returns a copy of env with all GIT_* variables removed. -// This prevents a subprocess from discovering or modifying the parent's git repo -// when we want isolation (e.g., running claude --print for review). +// wingmanStripGitEnv returns a copy of env with all GIT_* variables removed +// and the CLAUDECODE variable unset. GIT_* removal prevents a subprocess from +// discovering or modifying the parent's git repo. CLAUDECODE removal prevents +// the Claude CLI from refusing to start due to nested-session detection. func wingmanStripGitEnv(env []string) []string { filtered := make([]string, 0, len(env)) for _, e := range env { - if !strings.HasPrefix(e, "GIT_") { - filtered = append(filtered, e) + if strings.HasPrefix(e, "GIT_") || strings.HasPrefix(e, "CLAUDECODE=") { + continue } + filtered = append(filtered, e) } return filtered } diff --git a/cmd/entire/cli/wingman_test.go b/cmd/entire/cli/wingman_test.go index 6b6c6d513..5cfbf82f2 100644 --- a/cmd/entire/cli/wingman_test.go +++ b/cmd/entire/cli/wingman_test.go @@ -236,6 +236,7 @@ func TestWingmanStripGitEnv(t *testing.T) { "GIT_DIR=/repo/.git", "GIT_WORK_TREE=/repo", "EDITOR=vim", + "CLAUDECODE=1", } filtered := wingmanStripGitEnv(env) @@ -244,10 +245,13 @@ func TestWingmanStripGitEnv(t *testing.T) { if strings.HasPrefix(e, "GIT_") { t.Errorf("GIT_ variable should be stripped: %s", e) } + if strings.HasPrefix(e, "CLAUDECODE=") { + t.Errorf("CLAUDECODE variable should be stripped: %s", e) + } } if len(filtered) != 3 { - t.Errorf("expected 3 non-GIT vars, got %d", len(filtered)) + t.Errorf("expected 3 non-stripped vars, got %d", len(filtered)) } } diff --git a/docs/architecture/wingman.md b/docs/architecture/wingman.md index b7e5cffca..beb9db132 100644 --- a/docs/architecture/wingman.md +++ b/docs/architecture/wingman.md @@ -64,7 +64,7 @@ The review runs in a fully detached subprocess (`entire wingman __review 30 min) are auto-cleaned. - **Dedup hash**: SHA256 of sorted file paths prevents re-reviewing identical change sets. - **Detached processes**: Review and apply run in their own process groups (`Setpgid: true`), surviving parent exit. -- **GIT_* stripping**: Claude CLI calls strip `GIT_*` env vars via `wingmanStripGitEnv()` to prevent index corruption. Applied in `callClaudeForReview()` and `triggerAutoApply()`, not at process spawn time. +- **Environment stripping**: Claude CLI calls strip `GIT_*` env vars (prevents index corruption) and `CLAUDECODE` (prevents nested-session detection refusal) via `wingmanStripGitEnv()`. Applied in `callClaudeForReview()` and `triggerAutoApply()`, not at process spawn time. The summarize package (`summarize/claude.go`) uses an identical `stripGitEnv()` for the same purpose. - **ENTIRE_WINGMAN_APPLY=1**: Set during auto-apply to prevent the post-commit hook from triggering another review (recursion prevention). - **Stale session detection**: ACTIVE/ACTIVE_COMMITTED sessions not updated in 4+ hours are considered orphaned (crashed agent) and ignored by `hasAnyLiveSession`. IDLE sessions are always considered live regardless of age. ## Configuration +### Commands + ```bash -entire wingman enable # Enable wingman auto-review -entire wingman disable # Disable and clean up pending reviews -entire wingman status # Show current status +entire wingman enable [--local] # Enable wingman auto-review +entire wingman disable [--local] # Disable and clean up pending reviews +entire wingman status # Show current status ``` -Wingman state is stored in `.entire/settings.json`: +**Precondition:** `entire wingman enable` requires Entire to be enabled first (`entire enable`). + +### `--local` Flag + +The `--local` flag controls which settings file is written: + +| Flag | File | Committed to git | Purpose | +|------|------|------------------|---------| +| (default) | `.entire/settings.json` | Yes | Project-wide setting shared with team | +| `--local` | `.entire/settings.local.json` | No (gitignored) | User-specific override | + +When loading settings, both files are merged — local overrides project. The `--local` flag only affects which file is *written* to. + +### Settings Structure ```json { @@ -244,6 +258,26 @@ Wingman state is stored in `.entire/settings.json`: } ``` +### Status Output + +`entire wingman status` shows: + +``` +Wingman: enabled +Last review: 2026-02-13T08:45:11+01:00 +Status: pending +Pending review: .entire/REVIEW.md +``` + +Fields shown: enabled/disabled status, last review timestamp (if available), applied/pending status, and pending review file path (if exists). + +### Hidden Subcommands + +| Command | Purpose | +|---------|---------| +| `entire wingman __review ` | Internal: spawned as detached review subprocess | +| `entire wingman __apply ` | Internal: spawned to auto-apply pending REVIEW.md | + ## Key Constants | Constant | Value | Purpose | @@ -254,5 +288,39 @@ Wingman state is stored in `.entire/settings.json`: | `wingmanReviewTimeout` | 5m | Timeout for Claude review API call | | `wingmanApplyTimeout` | 15m | Timeout for auto-apply process | | `wingmanStaleReviewTTL` | 1h | Max age before review is cleaned up | -| `staleLockThreshold` | 30m | Max age before lock is considered stale | +| `staleLockThreshold` | 30m | Max age before lock is considered stale (for lock acquisition) | +| `wingmanNotificationLockThreshold` | 10m | Max lock age for showing "Review in progress" notifications | | `staleActiveSessionThreshold` | 4h | Max age before ACTIVE session is considered stale/orphaned | +| `maxDiffSize` | 100KB | Maximum diff size included in review prompt (truncates beyond) | + +None of these constants are user-configurable — they are internal to the implementation. + +## Claude CLI Invocations + +### Review Call (`callClaudeForReview`) + +```bash +claude --print \ + --output-format json \ + --model sonnet \ + --setting-sources "" \ + --allowedTools "Read,Glob,Grep" \ + --permission-mode bypassPermissions +``` + +- Working directory: repo root (reviewer can access source files) +- Environment: `wingmanStripGitEnv()` strips `GIT_*` and `CLAUDECODE` +- Input: review prompt via stdin + +### Auto-Apply Call (`triggerAutoApply`) + +```bash +claude --continue \ + --print \ + --setting-sources "" \ + --permission-mode acceptEdits \ + +``` + +- Working directory: repo root +- Environment: `wingmanStripGitEnv()` strips `GIT_*` and `CLAUDECODE`, adds `ENTIRE_WINGMAN_APPLY=1` From 6706f449a5df3f1680790f66e120002312eaea32 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Fri, 13 Feb 2026 15:56:45 +0100 Subject: [PATCH 32/32] fix: auto-gitignore wingman runtime files Add wingman.lock, wingman-state.json, wingman-payload.json, and REVIEW.md to EnsureEntireGitignore() so they are automatically excluded from version control for all users. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: ac0df591859d --- .entire/.gitignore | 4 ++++ cmd/entire/cli/strategy/common.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/.entire/.gitignore b/.entire/.gitignore index 0b700c83e..a3573f5b8 100644 --- a/.entire/.gitignore +++ b/.entire/.gitignore @@ -3,3 +3,7 @@ settings.local.json metadata/ current_session logs/ +wingman.lock +wingman-state.json +wingman-payload.json +REVIEW.md diff --git a/cmd/entire/cli/strategy/common.go b/cmd/entire/cli/strategy/common.go index d815be17a..c981ab85c 100644 --- a/cmd/entire/cli/strategy/common.go +++ b/cmd/entire/cli/strategy/common.go @@ -698,6 +698,10 @@ func EnsureEntireGitignore() error { "settings.local.json", "metadata/", "logs/", + "wingman.lock", + "wingman-state.json", + "wingman-payload.json", + "REVIEW.md", } // Track what needs to be added