diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 692cee27f..982f099c3 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -45,7 +45,7 @@ import ( "golang.org/x/term" ) -const defaultCheckpointSummaryTimeout = 30 * time.Second +const defaultCheckpointSummaryTimeout = 5 * time.Minute const ( pagerEnvVar = "PAGER" @@ -59,6 +59,29 @@ var checkpointSummaryTimeout = defaultCheckpointSummaryTimeout var generateTranscriptSummary = summarize.GenerateFromTranscript +// resolveSummaryTimeout picks the effective deadline for `explain --generate` +// using the precedence: per-run flag > settings.summary_timeout_seconds > +// package default. Zero or negative values at any layer mean "unset; consult +// the next layer down" — matching SummaryTimeoutValue() semantics. +// +// Settings load failures are logged at debug and fall through to the default; +// a parsing hiccup must not break summary generation. +func resolveSummaryTimeout(ctx context.Context, flagSeconds int) time.Duration { + if flagSeconds > 0 { + return time.Duration(flagSeconds) * time.Second + } + s, err := settings.Load(ctx) + if err != nil { + logging.Debug(ctx, "summary timeout: settings load failed, using default", + slog.String("error", err.Error())) + return checkpointSummaryTimeout + } + if v := s.SummaryTimeoutValue(); v > 0 { + return v + } + return checkpointSummaryTimeout +} + // errCannotGenerateTemporaryCheckpoint is returned by runExplainCheckpoint when // --generate is requested for a target that does not match any committed // checkpoint. runExplainAuto uses errors.Is to detect this case and fall back @@ -227,6 +250,7 @@ func newExplainCmd() *cobra.Command { var searchAllFlag bool var jsonFlag bool var transcriptFlag bool + var summaryTimeoutSecondsFlag int sessionIndex := -1 listLimit := 0 // 0 means "use default (branchCheckpointsLimit)" @@ -344,6 +368,15 @@ Note: --session filters the list view; the positional arg, --commit, and --check return errors.New("--limit must be positive") } } + // --summary-timeout-seconds only makes sense with --generate. + if cmd.Flags().Changed("summary-timeout-seconds") { + if !generateFlag { + return errors.New("--summary-timeout-seconds only applies with --generate") + } + if summaryTimeoutSecondsFlag < 0 { + return errors.New("--summary-timeout-seconds must be non-negative") + } + } // Export modes — emit machine-readable output and skip the prose pipeline. // --raw-transcript also routes here when --session-index is explicit; the @@ -366,7 +399,7 @@ Note: --session filters the list view; the positional arg, --commit, and --check // Convert short flag to verbose (verbose = !short) verbose := !shortFlag - return runExplain(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), sessionFlag, commitFlag, checkpointFlag, positional, noPagerFlag, verbose, fullFlag, rawTranscriptFlag, generateFlag, forceFlag, searchAllFlag) + return runExplain(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), sessionFlag, commitFlag, checkpointFlag, positional, noPagerFlag, verbose, fullFlag, rawTranscriptFlag, generateFlag, forceFlag, searchAllFlag, summaryTimeoutSecondsFlag) }, } @@ -384,6 +417,7 @@ Note: --session filters the list view; the positional arg, --commit, and --check cmd.Flags().BoolVar(&transcriptFlag, "transcript", false, "Stream compact normalized transcript bytes to stdout (pair with --raw-transcript for the per-agent raw transcript)") cmd.Flags().IntVar(&sessionIndex, "session-index", -1, "Session index within a multi-session checkpoint (0-based, defaults to latest)") cmd.Flags().IntVar(&listLimit, "limit", 0, "Cap the list view at N checkpoints (default: 100). Only meaningful with --json.") + cmd.Flags().IntVar(&summaryTimeoutSecondsFlag, "summary-timeout-seconds", 0, "Hard deadline in seconds for --generate summary generation; overrides summary_timeout_seconds setting. 0 = use setting or 5m default.") // Verbosity / transcript output modes are mutually exclusive cmd.MarkFlagsMutuallyExclusive("short", "full", "raw-transcript", "transcript", "json") @@ -398,7 +432,7 @@ Note: --session filters the list view; the positional arg, --commit, and --check // runExplain routes to the appropriate explain function based on flags and the // optional positional target. -func runExplain(ctx context.Context, w, errW io.Writer, sessionID, commitRef, checkpointID, target string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool) error { +func runExplain(ctx context.Context, w, errW io.Writer, sessionID, commitRef, checkpointID, target string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, summaryTimeoutSeconds int) error { // Count mutually exclusive flags (--commit and --checkpoint are mutually exclusive) // --session is now a filter for the list view, not a separate mode flagCount := 0 @@ -418,13 +452,13 @@ func runExplain(ctx context.Context, w, errW io.Writer, sessionID, commitRef, ch // Route to appropriate handler if target != "" { - return runExplainAuto(ctx, w, errW, target, noPager, verbose, full, rawTranscript, generate, force, searchAll) + return runExplainAuto(ctx, w, errW, target, noPager, verbose, full, rawTranscript, generate, force, searchAll, summaryTimeoutSeconds) } if commitRef != "" { - return runExplainCommit(ctx, w, errW, commitRef, noPager, verbose, full, rawTranscript, generate, force, searchAll) + return runExplainCommit(ctx, w, errW, commitRef, noPager, verbose, full, rawTranscript, generate, force, searchAll, summaryTimeoutSeconds) } if checkpointID != "" { - return runExplainCheckpoint(ctx, w, errW, checkpointID, noPager, verbose, full, rawTranscript, generate, force, searchAll) + return runExplainCheckpoint(ctx, w, errW, checkpointID, noPager, verbose, full, rawTranscript, generate, force, searchAll, summaryTimeoutSeconds) } // Default or with session filter: show list view (optionally filtered by session) @@ -437,7 +471,7 @@ func runExplain(ctx context.Context, w, errW io.Writer, sessionID, commitRef, ch // resolution only on checkpoint.ErrCheckpointNotFound. --generate runs // an ambiguity pre-check to avoid writing a summary to the wrong // checkpoint on short-prefix collisions. -func runExplainAuto(ctx context.Context, w, errW io.Writer, target string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool) error { +func runExplainAuto(ctx context.Context, w, errW io.Writer, target string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, summaryTimeoutSeconds int) error { stop := startSpinner(errW, "Loading checkpoints") lookup, lookupErr := newExplainCheckpointLookup(ctx) stop(false) @@ -446,7 +480,7 @@ func runExplainAuto(ctx context.Context, w, errW io.Writer, target string, noPag return err } } - checkpointErr := runExplainCheckpointWithLookup(ctx, w, errW, target, noPager, verbose, full, rawTranscript, generate, force, searchAll, lookup, lookupErr) + checkpointErr := runExplainCheckpointWithLookup(ctx, w, errW, target, noPager, verbose, full, rawTranscript, generate, force, searchAll, lookup, lookupErr, summaryTimeoutSeconds) if checkpointErr == nil { return nil } @@ -496,7 +530,7 @@ func runExplainAuto(ctx context.Context, w, errW io.Writer, target string, noPag slog.String("target", target), slog.String("commit", abbreviateCommitHash(lookup.repo, hash)), slog.String("checkpoint_id", cpID.String())) - return runExplainCheckpointWithLookup(ctx, w, errW, cpID.String(), noPager, verbose, full, rawTranscript, generate, force, searchAll, lookup, nil) + return runExplainCheckpointWithLookup(ctx, w, errW, cpID.String(), noPager, verbose, full, rawTranscript, generate, force, searchAll, lookup, nil, summaryTimeoutSeconds) } // runExplainAutoAmbiguityGuard refuses --generate when the positional @@ -550,11 +584,11 @@ func runExplainAutoAmbiguityGuard(ctx context.Context, target string, lookup *ex // When searchAll is true, searches all commits without branch/depth limits (used for finding associated commits). // -func runExplainCheckpoint(ctx context.Context, w, errW io.Writer, checkpointIDPrefix string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool) error { - return runExplainCheckpointWithLookup(ctx, w, errW, checkpointIDPrefix, noPager, verbose, full, rawTranscript, generate, force, searchAll, nil, nil) +func runExplainCheckpoint(ctx context.Context, w, errW io.Writer, checkpointIDPrefix string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, summaryTimeoutSeconds int) error { + return runExplainCheckpointWithLookup(ctx, w, errW, checkpointIDPrefix, noPager, verbose, full, rawTranscript, generate, force, searchAll, nil, nil, summaryTimeoutSeconds) } -func runExplainCheckpointWithLookup(ctx context.Context, w, errW io.Writer, checkpointIDPrefix string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, lookup *explainCheckpointLookup, lookupErr error) error { +func runExplainCheckpointWithLookup(ctx context.Context, w, errW io.Writer, checkpointIDPrefix string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, lookup *explainCheckpointLookup, lookupErr error, summaryTimeoutSeconds int) error { if lookup == nil { var err error lookup, err = newExplainCheckpointLookup(ctx) @@ -625,7 +659,7 @@ func runExplainCheckpointWithLookup(ctx context.Context, w, errW io.Writer, chec // Handle summary generation — uses raw transcript. if generate { stopLoad(false) // generation prints its own progress to w/errW - if err := generateCheckpointSummary(ctx, w, errW, lookup.v1Store, lookup.v2Store, fullCheckpointID, summary, content, force); err != nil { + if err := generateCheckpointSummary(ctx, w, errW, lookup.v1Store, lookup.v2Store, fullCheckpointID, summary, content, force, summaryTimeoutSeconds); err != nil { return err } // Reload to get the updated summary. After generation we only need @@ -966,7 +1000,11 @@ func readV2ContentFromMain(ctx context.Context, v2Reader *checkpoint.V2GitStore, // generateCheckpointSummary generates an AI summary for a checkpoint and persists it. // The summary is generated from the scoped transcript (only this checkpoint's portion), // not the entire session transcript. -func generateCheckpointSummary(ctx context.Context, w, errW io.Writer, v1Store *checkpoint.GitStore, v2Store *checkpoint.V2GitStore, checkpointID id.CheckpointID, cpSummary *checkpoint.CheckpointSummary, content *checkpoint.SessionContent, force bool) error { +// +// summaryTimeoutSeconds is the per-invocation --summary-timeout-seconds flag +// value (0 = unset). Effective precedence for the deadline: flag > settings > +// package default. See resolveSummaryTimeout for the resolution. +func generateCheckpointSummary(ctx context.Context, w, errW io.Writer, v1Store *checkpoint.GitStore, v2Store *checkpoint.V2GitStore, checkpointID id.CheckpointID, cpSummary *checkpoint.CheckpointSummary, content *checkpoint.SessionContent, force bool, summaryTimeoutSeconds int) error { // Check if summary already exists if content.Metadata.Summary != nil && !force { return renderExplainFailure(errW, "Summary already exists", []explainRow{ @@ -1001,8 +1039,10 @@ func generateCheckpointSummary(ctx context.Context, w, errW io.Writer, v1Store * fmt.Fprintln(errW, "Generating checkpoint summary...") } + timeout := resolveSummaryTimeout(ctx, summaryTimeoutSeconds) + start := time.Now() - summary, appliedDeadline, err := generateCheckpointAISummary(ctx, scopedTranscript, cpSummary.FilesTouched, content.Metadata.Agent, provider.Generator) + summary, appliedDeadline, err := generateCheckpointAISummary(ctx, scopedTranscript, cpSummary.FilesTouched, content.Metadata.Agent, provider.Generator, timeout) if err != nil { label, rows, structured := formatCheckpointSummaryError(err, appliedDeadline) styles := newStatusStyles(errW) @@ -1131,14 +1171,14 @@ func transcriptHasSummaryContent(transcriptBytes []byte, agentType types.AgentTy } // generateCheckpointAISummary returns the generated summary, the effective -// deadline applied to the underlying call (which may be shorter than -// checkpointSummaryTimeout if the parent context had an earlier deadline), -// and any error. The effective deadline is returned so the caller can render -// the true timeout value in user-facing error messages instead of always -// showing the package default. -func generateCheckpointAISummary(ctx context.Context, scopedTranscript []byte, filesTouched []string, agentType types.AgentType, generator summarize.Generator) (*checkpoint.Summary, time.Duration, error) { - timeoutCtx, cancel := context.WithTimeout(ctx, checkpointSummaryTimeout) - timeoutDuration := checkpointSummaryTimeout +// deadline applied to the underlying call (which may be shorter than the +// requested timeout if the parent context had an earlier deadline), and any +// error. The effective deadline is returned so the caller can render the +// true timeout value in user-facing error messages instead of always +// showing the requested value. +func generateCheckpointAISummary(ctx context.Context, scopedTranscript []byte, filesTouched []string, agentType types.AgentType, generator summarize.Generator, timeout time.Duration) (*checkpoint.Summary, time.Duration, error) { + timeoutCtx, cancel := context.WithTimeout(ctx, timeout) + timeoutDuration := timeout if deadline, ok := timeoutCtx.Deadline(); ok { timeoutDuration = time.Until(deadline) } @@ -2411,7 +2451,7 @@ func outputExplainContent(w io.Writer, content string, noPager bool) { // runExplainCommit looks up the checkpoint associated with a commit. // Extracts the Entire-Checkpoint trailer and delegates to checkpoint detail view. // If no trailer found, shows a message indicating no associated checkpoint. -func runExplainCommit(ctx context.Context, w, errW io.Writer, commitRef string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool) error { +func runExplainCommit(ctx context.Context, w, errW io.Writer, commitRef string, noPager, verbose, full, rawTranscript, generate, force, searchAll bool, summaryTimeoutSeconds int) error { repo, err := openRepository(ctx) if err != nil { return fmt.Errorf("not a git repository: %w", err) @@ -2449,7 +2489,7 @@ func runExplainCommit(ctx context.Context, w, errW io.Writer, commitRef string, // Delegate to checkpoint detail view, forwarding the full flag set so // --generate / --raw-transcript / --force work via --commit as well. - return runExplainCheckpoint(ctx, w, errW, checkpointID.String(), noPager, verbose, full, rawTranscript, generate, force, searchAll) + return runExplainCheckpoint(ctx, w, errW, checkpointID.String(), noPager, verbose, full, rawTranscript, generate, force, searchAll, summaryTimeoutSeconds) } // formatSessionInfo formats session information for display. diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index 31f938585..45d102e53 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -306,6 +306,71 @@ func TestExplainCmd_PositionalArgConflictsWithFlags(t *testing.T) { } } +// TestExplainCmd_SummaryTimeoutSecondsValidation verifies the +// --summary-timeout-seconds flag is rejected when it can't take effect — +// regardless of whether the invocation routes to the prose pipeline or +// to an export mode (--json / --transcript / --raw-transcript). The +// validation must run before the export-mode early return so the flag +// never silently no-ops. +func TestExplainCmd_SummaryTimeoutSecondsValidation(t *testing.T) { + t.Parallel() + tests := []struct { + name string + args []string + wantErr string + }{ + { + "no --generate, prose path", + []string{"--summary-timeout-seconds", "10"}, + "--summary-timeout-seconds only applies with --generate", + }, + { + "no --generate, --json export", + []string{"--json", "--summary-timeout-seconds", "10"}, + "--summary-timeout-seconds only applies with --generate", + }, + { + "no --generate, --transcript export", + []string{"--transcript", "abc123", "--summary-timeout-seconds", "10"}, + "--summary-timeout-seconds only applies with --generate", + }, + { + "no --generate, --raw-transcript with --session-index export", + []string{"--raw-transcript", "abc123", "--session-index", "0", "--summary-timeout-seconds", "10"}, + "--summary-timeout-seconds only applies with --generate", + }, + { + "negative value with --generate", + []string{"--generate", "abc123", "--summary-timeout-seconds", "-5"}, + "--summary-timeout-seconds must be non-negative", + }, + { + "negative value with --json", + []string{"--json", "--summary-timeout-seconds", "-5"}, + "--summary-timeout-seconds only applies with --generate", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cmd := newExplainCmd() + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs(tt.args) + + err := cmd.Execute() + if err == nil { + t.Fatalf("expected error, got nil (stdout=%q stderr=%q)", stdout.String(), stderr.String()) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("expected error containing %q, got: %v", tt.wantErr, err) + } + }) + } +} + // runExplainAutoTestRepo seeds a git repo and returns the initial commit's hash. func runExplainAutoTestRepo(t *testing.T) (repo *git.Repository, initialCommit plumbing.Hash) { t.Helper() @@ -333,7 +398,7 @@ func TestRunExplainAuto_NoMatchReturnsCompositeError(t *testing.T) { runExplainAutoTestRepo(t) var out, errOut bytes.Buffer - err := runExplainAuto(context.Background(), &out, &errOut, "abababababab", false, false, false, false, false, false, false) + err := runExplainAuto(context.Background(), &out, &errOut, "abababababab", false, false, false, false, false, false, false, 0) require.Error(t, err) require.ErrorContains(t, err, `no checkpoint or commit found matching "abababababab"`) @@ -368,7 +433,7 @@ func TestRunExplainAuto_CommitRefWithCheckpointTrailer(t *testing.T) { require.NoError(t, err) var out, errOut bytes.Buffer - err = runExplainAuto(ctx, &out, &errOut, commitHash.String(), true, false, false, false, false, false, false) + err = runExplainAuto(ctx, &out, &errOut, commitHash.String(), true, false, false, false, false, false, false, 0) require.NoError(t, err) require.Contains(t, out.String(), cpID.String(), "expected checkpoint header resolved via trailer") } @@ -395,7 +460,7 @@ func TestRunExplainAuto_CommitWithoutTrailer(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { var out, errOut bytes.Buffer - err := runExplainAuto(context.Background(), &out, &errOut, initial.String(), true, false, false, tc.rawTrans, tc.generate, false, false) + err := runExplainAuto(context.Background(), &out, &errOut, initial.String(), true, false, false, tc.rawTrans, tc.generate, false, false, 0) if tc.wantErr { require.Error(t, err) require.ErrorContains(t, err, tc.wantContain) @@ -420,7 +485,7 @@ func TestRunExplainCheckpoint_NotFoundSentinels(t *testing.T) { for _, generate := range []bool{false, true} { t.Run(fmt.Sprintf("generate=%v", generate), func(t *testing.T) { var out, errOut bytes.Buffer - err := runExplainCheckpoint(context.Background(), &out, &errOut, "abababababab", false, false, false, false, generate, false, false) + err := runExplainCheckpoint(context.Background(), &out, &errOut, "abababababab", false, false, false, false, generate, false, false, 0) require.Error(t, err) require.ErrorIs(t, err, checkpoint.ErrCheckpointNotFound) @@ -481,7 +546,7 @@ func TestRunExplainAuto_GenerateTemporaryCheckpointDoesNotFallBackToCommit(t *te tempCheckpointSHA := writeTemporaryCheckpointForExplainTest(t) var out, errOut bytes.Buffer - err := runExplainAuto(context.Background(), &out, &errOut, tempCheckpointSHA, true, false, false, false, true, false, false) + err := runExplainAuto(context.Background(), &out, &errOut, tempCheckpointSHA, true, false, false, false, true, false, false, 0) require.Error(t, err) require.ErrorIs(t, err, errCannotGenerateTemporaryCheckpoint) @@ -499,7 +564,7 @@ func TestRunExplainAuto_TemporaryCheckpointRendersIdentityBullet(t *testing.T) { var out, errOut bytes.Buffer // noPager=true to suppress the pager's terminal-only path so output lands // in the buffer; generate=false so we read (and don't try to summarize). - err := runExplainAuto(context.Background(), &out, &errOut, tempCheckpointSHA, true, false, false, false, false, false, false) + err := runExplainAuto(context.Background(), &out, &errOut, tempCheckpointSHA, true, false, false, false, false, false, false, 0) require.NoError(t, err) output := out.String() @@ -575,7 +640,7 @@ func TestRunExplainCommit_AmbiguousPrintsToErrWAndReturnsSilent(t *testing.T) { prefix := collidingShaPrefix(t, repo, tmpDir) var out, errOut bytes.Buffer - err = runExplainCommit(context.Background(), &out, &errOut, prefix, true, false, false, false, false, false, false) + err = runExplainCommit(context.Background(), &out, &errOut, prefix, true, false, false, false, false, false, false, 0) var silent *SilentError if !errors.As(err, &silent) { @@ -619,7 +684,7 @@ func TestRunExplainCheckpoint_AmbiguousCommittedPrefixPrintsToErrWAndReturnsSile } var out, errOut bytes.Buffer - err := runExplainCheckpoint(ctx, &out, &errOut, "e7", true, false, false, false, false, false, false) + err := runExplainCheckpoint(ctx, &out, &errOut, "e7", true, false, false, false, false, false, false, 0) var silent *SilentError if !errors.As(err, &silent) { @@ -711,7 +776,7 @@ func TestRunExplainAuto_GenerateAmbiguousPrefixRefused(t *testing.T) { })) var out, errOut bytes.Buffer - err = runExplainAuto(ctx, &out, &errOut, commitPrefix, true, false, false, false, true, false, false) + err = runExplainAuto(ctx, &out, &errOut, commitPrefix, true, false, false, false, true, false, false, 0) require.Error(t, err) require.ErrorContains(t, err, "ambiguous target") @@ -770,7 +835,7 @@ func TestGenerateCheckpointAISummary_AddsDefaultTimeoutWithoutParentDeadline(t * } start := time.Now() - summary, _, err := generateCheckpointAISummary(context.Background(), []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + summary, _, err := generateCheckpointAISummary(context.Background(), []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil, checkpointSummaryTimeout) if err != nil { t.Fatalf("generateCheckpointAISummary() error = %v", err) } @@ -861,7 +926,7 @@ func TestGenerateCheckpointAISummary_UsesParentDeadlineAndWrapsSentinel(t *testi return nil, ctx.Err() } - _, appliedDeadline, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + _, appliedDeadline, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil, checkpointSummaryTimeout) if err == nil { t.Fatal("expected timeout error") } @@ -917,7 +982,7 @@ func TestGenerateCheckpointAISummary_PreservesClaudeErrorWhenCtxIsDone(t *testin return nil, claudeErr } - _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil, checkpointSummaryTimeout) var ce *claudecode.ClaudeError if !errors.As(err, &ce) { t.Fatalf("errors.As did not recover *ClaudeError; got %v", err) @@ -957,7 +1022,7 @@ func TestGenerateCheckpointAISummary_ClampsLongParentDeadlineToDefaultTimeout(t } start := time.Now() - summary, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + summary, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil, checkpointSummaryTimeout) if err != nil { t.Fatalf("generateCheckpointAISummary() error = %v", err) } @@ -994,7 +1059,7 @@ func TestGenerateCheckpointAISummary_UsesCancellationSentinel(t *testing.T) { return nil, ctx.Err() } - _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil, checkpointSummaryTimeout) if err == nil { t.Fatal("expected cancellation error") } @@ -1006,6 +1071,86 @@ func TestGenerateCheckpointAISummary_UsesCancellationSentinel(t *testing.T) { } } +// writeSummaryTimeoutSettings creates an entire-recognized settings file with +// the given timeout value (in seconds). Use 0 to omit the field entirely. +func writeSummaryTimeoutSettings(t *testing.T, dir string, timeoutSeconds int) { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".entire"), 0o755)) + var body string + if timeoutSeconds == 0 { + body = `{"enabled":true}` + } else { + body = fmt.Sprintf(`{"enabled":true,"summary_timeout_seconds":%d}`, timeoutSeconds) + } + require.NoError(t, os.WriteFile( + filepath.Join(dir, ".entire", "settings.json"), + []byte(body), + 0o644, + )) +} + +func TestResolveSummaryTimeout_FlagOverridesSetting(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + t.Chdir(tmpDir) + writeSummaryTimeoutSettings(t, tmpDir, 60) + + got := resolveSummaryTimeout(context.Background(), 120) + + if want := 120 * time.Second; got != want { + t.Fatalf("resolveSummaryTimeout(flag=120, setting=60) = %s, want %s", got, want) + } +} + +func TestResolveSummaryTimeout_SettingHonoredWhenFlagUnset(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + t.Chdir(tmpDir) + writeSummaryTimeoutSettings(t, tmpDir, 60) + + got := resolveSummaryTimeout(context.Background(), 0) + + if want := 60 * time.Second; got != want { + t.Fatalf("resolveSummaryTimeout(flag=0, setting=60) = %s, want %s", got, want) + } +} + +func TestResolveSummaryTimeout_DefaultWhenBothUnset(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + t.Chdir(tmpDir) + writeSummaryTimeoutSettings(t, tmpDir, 0) // no summary_timeout_seconds field + + got := resolveSummaryTimeout(context.Background(), 0) + + if got != checkpointSummaryTimeout { + t.Fatalf("resolveSummaryTimeout(flag=0, setting=0) = %s, want %s (package default)", got, checkpointSummaryTimeout) + } +} + +func TestResolveSummaryTimeout_NegativeSettingTreatedAsUnset(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + t.Chdir(tmpDir) + writeSummaryTimeoutSettings(t, tmpDir, -1) + + got := resolveSummaryTimeout(context.Background(), 0) + + if got != checkpointSummaryTimeout { + t.Fatalf("resolveSummaryTimeout(flag=0, setting=-1) = %s, want %s (package default)", got, checkpointSummaryTimeout) + } +} + +// Locks in 5 minutes as the package default so a casual edit doesn't silently +// regress to the prior 30s — issue #1198 raised the default specifically +// because 30s was too tight for large transcripts. +func TestDefaultCheckpointSummaryTimeout_IsFiveMinutes(t *testing.T) { + t.Parallel() + if defaultCheckpointSummaryTimeout != 5*time.Minute { + t.Fatalf("defaultCheckpointSummaryTimeout = %s, want 5m (see issue #1198)", defaultCheckpointSummaryTimeout) + } +} + func TestExplainCommit_NotFound(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) @@ -1014,7 +1159,7 @@ func TestExplainCommit_NotFound(t *testing.T) { testutil.InitRepo(t, tmpDir) var stdout bytes.Buffer - err := runExplainCommit(context.Background(), &stdout, &stdout, "nonexistent", false, false, false, false, false, false, false) + err := runExplainCommit(context.Background(), &stdout, &stdout, "nonexistent", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error for nonexistent commit, got nil") @@ -1057,7 +1202,7 @@ func TestExplainCommit_NoEntireData(t *testing.T) { } var stdout bytes.Buffer - err = runExplainCommit(context.Background(), &stdout, &stdout, commitHash.String(), false, false, false, false, false, false, false) + err = runExplainCommit(context.Background(), &stdout, &stdout, commitHash.String(), false, false, false, false, false, false, false, 0) if err != nil { t.Fatalf("runExplainCommit() should not error for non-Entire commits, got: %v", err) } @@ -1129,7 +1274,7 @@ func TestExplainCommit_WithMetadataTrailerButNoCheckpoint(t *testing.T) { } var stdout bytes.Buffer - err = runExplainCommit(context.Background(), &stdout, &stdout, commitHash.String(), false, false, false, false, false, false, false) + err = runExplainCommit(context.Background(), &stdout, &stdout, commitHash.String(), false, false, false, false, false, false, false, 0) if err != nil { t.Fatalf("runExplainCommit() error = %v", err) } @@ -1262,7 +1407,7 @@ func TestExplainDefault_NoCheckpoints_ShowsHelpfulMessage(t *testing.T) { func TestExplainBothFlagsError(t *testing.T) { // Test that providing both --session and --commit returns an error var stdout, stderr bytes.Buffer - err := runExplain(context.Background(), &stdout, &stderr, "session-id", "commit-sha", "", "", false, false, false, false, false, false, false) + err := runExplain(context.Background(), &stdout, &stderr, "session-id", "commit-sha", "", "", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error when both flags provided, got nil") @@ -1715,7 +1860,7 @@ func TestRunExplain_MutualExclusivityError(t *testing.T) { var buf, errBuf bytes.Buffer // Providing both --session and --checkpoint should error - err := runExplain(context.Background(), &buf, &errBuf, "session-id", "", "checkpoint-id", "", false, false, false, false, false, false, false) + err := runExplain(context.Background(), &buf, &errBuf, "session-id", "", "checkpoint-id", "", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error when multiple flags provided") @@ -1758,7 +1903,7 @@ func TestRunExplainCheckpoint_NotFound(t *testing.T) { } var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "nonexistent123", false, false, false, false, false, false, false) + err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "nonexistent123", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error for nonexistent checkpoint") @@ -1820,7 +1965,7 @@ func TestRunExplainCheckpoint_V2OnlyCheckpoint(t *testing.T) { } var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "777777", false, false, false, false, false, false, false) + err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "777777", false, false, false, false, false, false, false, 0) if err != nil { t.Fatalf("expected success for v2-only checkpoint, got error: %v", err) } @@ -1886,7 +2031,7 @@ func TestRunExplainCheckpoint_V2OnlyRawTranscript(t *testing.T) { } var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "888888", false, false, false, true, false, false, false) + err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "888888", false, false, false, true, false, false, false, 0) if err != nil { t.Fatalf("expected success for v2-only raw transcript, got error: %v", err) } @@ -1967,7 +2112,7 @@ exec git-upload-pack "$repo" )) var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(ctx, &buf, &errBuf, "121212", false, false, false, true, false, false, false) + err = runExplainCheckpoint(ctx, &buf, &errBuf, "121212", false, false, false, true, false, false, false, 0) require.NoError(t, err) require.Contains(t, buf.String(), "raw from checkpoint_remote") } @@ -2031,7 +2176,7 @@ func TestRunExplainCheckpoint_V2UsesCompactTranscriptForIntent(t *testing.T) { } var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "999999", false, false, false, false, false, false, false) + err = runExplainCheckpoint(context.Background(), &buf, &errBuf, "999999", false, false, false, false, false, false, false, 0) if err != nil { t.Fatalf("expected success for v2 checkpoint, got error: %v", err) } @@ -2098,7 +2243,7 @@ func TestRunExplainCheckpoint_V2PreferredGenerateWritesBothStores(t *testing.T) // generate=true, force=true — should succeed by writing to both v1 and v2 stores. var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(ctx, &buf, &errBuf, "aabbcc", false, false, false, false, true, true, false) + err = runExplainCheckpoint(ctx, &buf, &errBuf, "aabbcc", false, false, false, false, true, true, false, 0) // Generation requires an AI summarizer which isn't available in unit tests, // but the important thing is we don't get the old "only v1 checkpoints supported" error. if err != nil && strings.Contains(err.Error(), "summary updates are currently supported only for v1 checkpoints") { @@ -2151,7 +2296,7 @@ func TestRunExplainCheckpoint_V2OnlyGenerateSucceedsViaV2Store(t *testing.T) { // generate=true, force=true — should not fail with "failed to save summary" // because v2 store can persist even when v1 doesn't have the checkpoint. var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(ctx, &buf, &errBuf, "f1f2f3", false, false, false, false, true, true, false) + err = runExplainCheckpoint(ctx, &buf, &errBuf, "f1f2f3", false, false, false, false, true, true, false, 0) if err != nil { errMsg := err.Error() if strings.Contains(errMsg, "claude") || strings.Contains(errMsg, "executable file not found") { @@ -2209,7 +2354,7 @@ func TestRunExplainCheckpoint_V2FallsBackToFullWhenCompactMissing(t *testing.T) // Default explain (not --full) should fall back to /full/current transcript // when compact transcript is missing on /main. var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(ctx, &buf, &errBuf, "e1e2e3", false, false, false, false, false, false, false) + err = runExplainCheckpoint(ctx, &buf, &errBuf, "e1e2e3", false, false, false, false, false, false, false, 0) require.NoError(t, err) output := buf.String() @@ -2273,7 +2418,7 @@ func TestRunExplainCheckpoint_V2CompactTranscriptNotUsedForGenerate(t *testing.T // generate=true — should NOT fail with "no transcript content" which would // indicate the compact transcript was incorrectly fed to the summarizer. var buf, errBuf bytes.Buffer - err = runExplainCheckpoint(ctx, &buf, &errBuf, "c0c1c2", false, false, false, false, true, true, false) + err = runExplainCheckpoint(ctx, &buf, &errBuf, "c0c1c2", false, false, false, false, true, true, false, 0) if err != nil && strings.Contains(err.Error(), "no transcript content for this checkpoint") { t.Fatalf("compact transcript should not be used for --generate; raw transcript should be used instead: %v", err) } @@ -4386,7 +4531,7 @@ func TestRunExplainCommit_NoCheckpointTrailer(t *testing.T) { } var buf bytes.Buffer - err = runExplainCommit(context.Background(), &buf, &buf, hash.String()[:7], false, false, false, false, false, false, false) + err = runExplainCommit(context.Background(), &buf, &buf, hash.String()[:7], false, false, false, false, false, false, false, 0) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4436,7 +4581,7 @@ func TestRunExplainCommit_WithCheckpointTrailer(t *testing.T) { var buf bytes.Buffer // This should try to look up the checkpoint and fail (checkpoint doesn't exist in store) // but it should still attempt the lookup rather than showing commit details - err = runExplainCommit(context.Background(), &buf, &buf, hash.String()[:7], false, false, false, false, false, false, false) + err = runExplainCommit(context.Background(), &buf, &buf, hash.String()[:7], false, false, false, false, false, false, false, 0) // Should error because the checkpoint doesn't exist in the store if err == nil { @@ -4565,7 +4710,7 @@ func TestRunExplain_SessionFlagFiltersListView(t *testing.T) { // When session is specified alone, it should NOT error for mutual exclusivity // It should route to the list view with a filter (which may fail for other reasons // like not being in a git repo, but not for mutual exclusivity) - err := runExplain(context.Background(), &buf, &errBuf, "some-session", "", "", "", false, false, false, false, false, false, false) + err := runExplain(context.Background(), &buf, &errBuf, "some-session", "", "", "", false, false, false, false, false, false, false, 0) // Should NOT be a mutual exclusivity error if err != nil && strings.Contains(err.Error(), "cannot specify multiple") { @@ -4577,7 +4722,7 @@ func TestRunExplain_SessionWithCheckpointStillMutuallyExclusive(t *testing.T) { // Test that --session with --checkpoint is still an error var buf, errBuf bytes.Buffer - err := runExplain(context.Background(), &buf, &errBuf, "some-session", "", "some-checkpoint", "", false, false, false, false, false, false, false) + err := runExplain(context.Background(), &buf, &errBuf, "some-session", "", "some-checkpoint", "", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error when --session and --checkpoint both specified") @@ -4591,7 +4736,7 @@ func TestRunExplain_SessionWithCommitStillMutuallyExclusive(t *testing.T) { // Test that --session with --commit is still an error var buf, errBuf bytes.Buffer - err := runExplain(context.Background(), &buf, &errBuf, "some-session", "some-commit", "", "", false, false, false, false, false, false, false) + err := runExplain(context.Background(), &buf, &errBuf, "some-session", "some-commit", "", "", false, false, false, false, false, false, false, 0) if err == nil { t.Error("expected error when --session and --commit both specified") diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index d5637ae2d..cd1513df1 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -102,9 +102,9 @@ type EntireSettings struct { // SummaryTimeoutSeconds is an optional hard deadline (in seconds) for // `entire explain --generate` summary generation. Zero or negative means - // "unset" -- the caller picks the default. Not yet consumed by the - // generate path; present so settings round-trip for a follow-up change - // that wires it into the deadline selection. + // "unset" -- falls back to the per-run --summary-timeout-seconds flag + // (if set) or the package default (5 minutes). Raise for very large + // transcripts; lower (e.g. 30) for fast-fail in CI. SummaryTimeoutSeconds int `json:"summary_timeout_seconds,omitempty"` // SignCheckpointCommits controls whether checkpoint commits are signed. diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index 50533e79a..c656cb1d6 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -38,6 +38,7 @@ const ( flagSkipPushSessions = "skip-push-sessions" flagSummarizeModel = "summarize-model" flagSummarizeAgent = "summarize-provider" + flagSummarizeTimeout = "summarize-timeout-seconds" flagTelemetry = "telemetry" flagAbsoluteGitHookPath = "absolute-git-hook-path" flagForce = "force" @@ -101,6 +102,10 @@ func hasSummaryProviderFlags(cmd *cobra.Command) bool { return cmd.Flags().Changed(flagSummarizeAgent) || cmd.Flags().Changed(flagSummarizeModel) } +func hasSummaryTimeoutFlag(cmd *cobra.Command) bool { + return cmd.Flags().Changed(flagSummarizeTimeout) +} + // hasGlobalSettingsFlags reports whether any flag affects telemetry or // the entire-managed git hook (force / absolute path / local-dev). func hasGlobalSettingsFlags(cmd *cobra.Command) bool { @@ -113,7 +118,7 @@ func hasGlobalSettingsFlags(cmd *cobra.Command) bool { // hasConfigureSettingsFlags reports whether configure was invoked with any // flag that mutates settings or hooks. Bare invocation prints help instead. func hasConfigureSettingsFlags(cmd *cobra.Command) bool { - return hasStrategyFlags(cmd) || hasSummaryProviderFlags(cmd) || hasGlobalSettingsFlags(cmd) + return hasStrategyFlags(cmd) || hasSummaryProviderFlags(cmd) || hasSummaryTimeoutFlag(cmd) || hasGlobalSettingsFlags(cmd) } // enableUsesSetupFlow reports whether `entire enable` should delegate to the @@ -230,6 +235,41 @@ func updateSummaryGenerationSettings(ctx context.Context, w io.Writer, provider, return nil } +// updateSummaryTimeoutSetting persists the --summarize-timeout-seconds flag +// to settings. Pass 0 to clear (treated as "unset" by the generate path); +// negative values are rejected. +func updateSummaryTimeoutSetting(ctx context.Context, w io.Writer, timeoutSeconds int, opts EnableOptions) error { + if timeoutSeconds < 0 { + return errors.New("--summarize-timeout-seconds must be non-negative") + } + + targetFile, configDisplay := settingsTargetFile(ctx, opts.UseLocalSettings, opts.UseProjectSettings) + targetFileAbs, err := paths.AbsPath(ctx, targetFile) + if err != nil { + targetFileAbs = targetFile + } + + s, err := settings.LoadFromFile(targetFileAbs) + if err != nil { + return fmt.Errorf("failed to load settings: %w", err) + } + + s.SummaryTimeoutSeconds = timeoutSeconds + + if targetFile == settings.EntireSettingsLocalFile { + if err := SaveEntireSettingsLocal(ctx, s); err != nil { + return fmt.Errorf("failed to save settings: %w", err) + } + } else { + if err := SaveEntireSettings(ctx, s); err != nil { + return fmt.Errorf("failed to save settings: %w", err) + } + } + + fmt.Fprintf(w, "✓ Settings updated (%s)\n", configDisplay) + return nil +} + // updateGlobalSettings persists telemetry / hook-mode flags and reinstalls the // Entire git hook when --force, --absolute-git-hook-path, or --local-dev is set. func updateGlobalSettings(ctx context.Context, cmd *cobra.Command, w io.Writer, opts EnableOptions) error { @@ -627,6 +667,7 @@ func newSetupCmd() *cobra.Command { var opts EnableOptions var summarizeProvider string var summarizeModel string + var summarizeTimeoutSeconds int cmd := &cobra.Command{ Use: "configure", @@ -642,7 +683,8 @@ Examples: entire configure --absolute-git-hook-path # Reinstall git hook with absolute path entire configure --force # Reinstall git hook entire configure --checkpoint-remote github:org/checkpoints - entire configure --summarize-provider claude-code`, + entire configure --summarize-provider claude-code + entire configure --summarize-timeout-seconds 300 # 5m deadline for explain --generate`, RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() @@ -676,6 +718,11 @@ Examples: return err } } + if hasSummaryTimeoutFlag(cmd) { + if err := updateSummaryTimeoutSetting(ctx, cmd.OutOrStdout(), summarizeTimeoutSeconds, opts); err != nil { + return err + } + } if hasGlobalSettingsFlags(cmd) { if err := updateGlobalSettings(ctx, cmd, cmd.OutOrStdout(), opts); err != nil { return err @@ -694,6 +741,7 @@ Examples: cmd.Flags().StringVar(&opts.CheckpointRemote, flagCheckpointRemote, "", "Checkpoint remote in provider:owner/repo format (e.g., github:org/checkpoints-repo)") cmd.Flags().StringVar(&summarizeProvider, flagSummarizeAgent, "", "Set the provider used by explain --generate (e.g., claude-code, codex, gemini, cursor, copilot-cli)") cmd.Flags().StringVar(&summarizeModel, flagSummarizeModel, "", "Set the model hint used by explain --generate") + cmd.Flags().IntVar(&summarizeTimeoutSeconds, flagSummarizeTimeout, 0, "Set the hard deadline (seconds) for explain --generate summary generation. 0 clears (falls back to 5m default).") cmd.Flags().BoolVar(&opts.Telemetry, flagTelemetry, true, "Enable anonymous usage analytics") cmd.Flags().BoolVar(&opts.AbsoluteGitHookPath, flagAbsoluteGitHookPath, false, "Embed full binary path in git hooks (for GUI git clients that don't source shell profiles)") diff --git a/cmd/entire/cli/setup_test.go b/cmd/entire/cli/setup_test.go index eeecd26dd..01270ccc7 100644 --- a/cmd/entire/cli/setup_test.go +++ b/cmd/entire/cli/setup_test.go @@ -2278,6 +2278,111 @@ func TestConfigureCmd_CheckpointRemote_LocalOnlyRepo(t *testing.T) { } } +// Tests for configure --summarize-timeout-seconds (issue #1198) + +func TestConfigureCmd_SummarizeTimeoutSeconds_WritesProjectSettings(t *testing.T) { + setupTestRepo(t) + writeSettings(t, testSettingsEnabled) + + cmd := newSetupCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{"--summarize-timeout-seconds", "300"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("configure --summarize-timeout-seconds failed: %v", err) + } + + if !strings.Contains(stdout.String(), "Settings updated") { + t.Errorf("expected 'Settings updated' output, got: %s", stdout.String()) + } + + s, err := settings.LoadFromFile(EntireSettingsFile) + if err != nil { + t.Fatalf("failed to load settings: %v", err) + } + if s.SummaryTimeoutSeconds != 300 { + t.Errorf("SummaryTimeoutSeconds = %d, want 300", s.SummaryTimeoutSeconds) + } +} + +func TestConfigureCmd_SummarizeTimeoutSeconds_WritesLocalSettings(t *testing.T) { + setupTestRepo(t) + writeSettings(t, testSettingsEnabled) + + cmd := newSetupCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{"--local", "--summarize-timeout-seconds", "600"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("configure --local --summarize-timeout-seconds failed: %v", err) + } + + if !strings.Contains(stdout.String(), "settings.local.json") { + t.Errorf("expected output to reference settings.local.json, got: %s", stdout.String()) + } + + localS, err := settings.LoadFromFile(EntireSettingsLocalFile) + if err != nil { + t.Fatalf("failed to load local settings: %v", err) + } + if localS.SummaryTimeoutSeconds != 600 { + t.Errorf("local SummaryTimeoutSeconds = %d, want 600", localS.SummaryTimeoutSeconds) + } + + // Project settings must not have been mutated. + projectS, err := settings.LoadFromFile(EntireSettingsFile) + if err != nil { + t.Fatalf("failed to load project settings: %v", err) + } + if projectS.SummaryTimeoutSeconds != 0 { + t.Errorf("project SummaryTimeoutSeconds = %d, want 0 (unchanged)", projectS.SummaryTimeoutSeconds) + } +} + +func TestConfigureCmd_SummarizeTimeoutSeconds_ClearsValue(t *testing.T) { + setupTestRepo(t) + writeSettings(t, `{"enabled":true,"summary_timeout_seconds":300}`) + + cmd := newSetupCmd() + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{"--summarize-timeout-seconds", "0"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("configure --summarize-timeout-seconds 0 failed: %v", err) + } + + s, err := settings.LoadFromFile(EntireSettingsFile) + if err != nil { + t.Fatalf("failed to load settings: %v", err) + } + if s.SummaryTimeoutSeconds != 0 { + t.Errorf("SummaryTimeoutSeconds = %d, want 0 (cleared)", s.SummaryTimeoutSeconds) + } +} + +func TestConfigureCmd_SummarizeTimeoutSeconds_RejectsNegative(t *testing.T) { + setupTestRepo(t) + writeSettings(t, testSettingsEnabled) + + cmd := newSetupCmd() + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + cmd.SetArgs([]string{"--summarize-timeout-seconds", "-5"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for negative --summarize-timeout-seconds") + } + if !strings.Contains(err.Error(), "non-negative") { + t.Errorf("expected 'non-negative' in error, got: %v", err) + } +} + func TestConfigureCmd_CheckpointRemote_InvalidFormat(t *testing.T) { setupTestRepo(t) writeSettings(t, testSettingsEnabled)