diff --git a/internal/cli/cmd_agent_job.go b/internal/cli/cmd_agent_job.go index 80c0f204..207f0ae5 100644 --- a/internal/cli/cmd_agent_job.go +++ b/internal/cli/cmd_agent_job.go @@ -46,32 +46,19 @@ func cmdAgentJobStatus(w, wErr io.Writer, gf GlobalFlags, args []string, version return returnUsageError(w, wErr, gf, usage, version, nil) } + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "agent.job.status"} + store, err := newSendJobStore() if err != nil { - if gf.JSON { - ReturnError(w, "job_store_failed", err.Error(), nil, version) - } else { - Errorf(wErr, "failed to initialize send job store: %v", err) - } - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_store_failed", err.Error(), nil, fmt.Sprintf("failed to initialize send job store: %v", err)) } job, ok, err := store.get(jobID) if err != nil { - if gf.JSON { - ReturnError(w, "job_status_failed", err.Error(), map[string]any{"job_id": jobID}, version) - } else { - Errorf(wErr, "failed to read job %s: %v", jobID, err) - } - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_status_failed", err.Error(), map[string]any{"job_id": jobID}, fmt.Sprintf("failed to read job %s: %v", jobID, err)) } if !ok { - if gf.JSON { - ReturnError(w, "not_found", "job not found", map[string]any{"job_id": jobID}, version) - } else { - Errorf(wErr, "job %s not found", jobID) - } - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", "job not found", map[string]any{"job_id": jobID}, fmt.Sprintf("job %s not found", jobID)) } writeJobStatusResult(w, gf, version, job) @@ -89,44 +76,24 @@ func cmdAgentJobCancel(w, wErr io.Writer, gf GlobalFlags, args []string, version if jobID == "" { return returnUsageError(w, wErr, gf, usage, version, nil) } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "agent.job.cancel", *idempotencyKey, - ); handled { + + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "agent.job.cancel", idemKey: *idempotencyKey} + + if handled, code := ctx.maybeReplay(); handled { return code } store, err := newSendJobStore() if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.job.cancel", *idempotencyKey, - ExitInternalError, "job_store_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize send job store: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_store_failed", err.Error(), nil, fmt.Sprintf("failed to initialize send job store: %v", err)) } job, ok, canceled, err := store.cancel(jobID) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.job.cancel", *idempotencyKey, - ExitInternalError, "job_cancel_failed", err.Error(), map[string]any{"job_id": jobID}, - ) - } - Errorf(wErr, "failed to cancel job %s: %v", jobID, err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_cancel_failed", err.Error(), map[string]any{"job_id": jobID}, fmt.Sprintf("failed to cancel job %s: %v", jobID, err)) } if !ok { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.job.cancel", *idempotencyKey, - ExitNotFound, "not_found", "job not found", map[string]any{"job_id": jobID}, - ) - } - Errorf(wErr, "job %s not found", jobID) - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", "job not found", map[string]any{"job_id": jobID}, fmt.Sprintf("job %s not found", jobID)) } result := agentJobCancelResult{ @@ -135,9 +102,7 @@ func cmdAgentJobCancel(w, wErr io.Writer, gf GlobalFlags, args []string, version Canceled: canceled, } if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, "agent.job.cancel", *idempotencyKey, result, - ) + return ctx.successResult(result) } PrintHuman(w, func(w io.Writer) { @@ -166,34 +131,21 @@ func cmdAgentJobWait(w, wErr io.Writer, gf GlobalFlags, args []string, version s return returnUsageError(w, wErr, gf, usage, version, nil) } + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "agent.job.wait"} + store, err := newSendJobStore() if err != nil { - if gf.JSON { - ReturnError(w, "job_store_failed", err.Error(), nil, version) - } else { - Errorf(wErr, "failed to initialize send job store: %v", err) - } - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_store_failed", err.Error(), nil, fmt.Sprintf("failed to initialize send job store: %v", err)) } deadline := time.Now().Add(*timeout) for { job, ok, getErr := store.get(jobID) if getErr != nil { - if gf.JSON { - ReturnError(w, "job_status_failed", getErr.Error(), map[string]any{"job_id": jobID}, version) - } else { - Errorf(wErr, "failed to read job %s: %v", jobID, getErr) - } - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_status_failed", getErr.Error(), map[string]any{"job_id": jobID}, fmt.Sprintf("failed to read job %s: %v", jobID, getErr)) } if !ok { - if gf.JSON { - ReturnError(w, "not_found", "job not found", map[string]any{"job_id": jobID}, version) - } else { - Errorf(wErr, "job %s not found", jobID) - } - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", "job not found", map[string]any{"job_id": jobID}, fmt.Sprintf("job %s not found", jobID)) } if isTerminalSendJobStatus(job.Status) { writeJobStatusResult(w, gf, version, job) @@ -204,15 +156,16 @@ func cmdAgentJobWait(w, wErr io.Writer, gf GlobalFlags, args []string, version s } if time.Now().After(deadline) { - if gf.JSON { - ReturnError(w, "timeout", "timed out waiting for job completion", map[string]any{ + return ctx.errResult( + ExitInternalError, + "timeout", + "timed out waiting for job completion", + map[string]any{ "job_id": job.ID, "status": string(job.Status), - }, version) - } else { - Errorf(wErr, "timed out waiting for job %s completion (status: %s)", job.ID, job.Status) - } - return ExitInternalError + }, + fmt.Sprintf("timed out waiting for job %s completion (status: %s)", job.ID, job.Status), + ) } time.Sleep(*interval) } diff --git a/internal/cli/cmd_agent_run_liveness.go b/internal/cli/cmd_agent_run_liveness.go index 3ba680f2..0b12b57a 100644 --- a/internal/cli/cmd_agent_run_liveness.go +++ b/internal/cli/cmd_agent_run_liveness.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "io" "log/slog" "strings" "time" @@ -11,9 +10,8 @@ import ( ) func verifyStartedAgentSession( - w, wErr io.Writer, - gf GlobalFlags, - version, idempotencyKey, sessionName string, + ctx *cmdCtx, + sessionName string, tmuxOpts tmux.Options, ) int { state, err := tmuxSessionStateFor(sessionName, tmuxOpts) @@ -21,16 +19,13 @@ func verifyStartedAgentSession( if killErr := tmuxKillSession(sessionName, tmuxOpts); killErr != nil { slog.Debug("best-effort session kill failed", "session", sessionName, "error", killErr) } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", idempotencyKey, - ExitInternalError, "session_lookup_failed", err.Error(), map[string]any{ - "session_name": sessionName, - }, - ) - } - Errorf(wErr, "failed to verify session %s: %v", sessionName, err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "session_lookup_failed", + err.Error(), + map[string]any{"session_name": sessionName}, + fmt.Sprintf("failed to verify session %s: %v", sessionName, err), + ) } if state.Exists && state.HasLivePane { return ExitOK @@ -40,16 +35,9 @@ func verifyStartedAgentSession( slog.Debug("best-effort session kill failed", "session", sessionName, "error", err) } msg := fmt.Sprintf("assistant session %s exited before startup completed", sessionName) - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", idempotencyKey, - ExitInternalError, "session_exited", msg, map[string]any{ - "session_name": sessionName, - }, - ) - } - Errorf(wErr, msg) - return ExitInternalError + return ctx.errResult(ExitInternalError, "session_exited", msg, map[string]any{ + "session_name": sessionName, + }) } var ( @@ -73,9 +61,8 @@ var ( ) func sendAgentRunPromptIfRequested( - w, wErr io.Writer, - gf GlobalFlags, - version, idempotencyKey, sessionName, assistantName, prompt string, + ctx *cmdCtx, + sessionName, assistantName, prompt string, tmuxOpts tmux.Options, beforeSend func(), ) int { @@ -83,9 +70,6 @@ func sendAgentRunPromptIfRequested( return ExitOK } - // Wait for the agent TUI to render before sending. Agents like Codex can - // take several seconds to initialize; a fixed short sleep causes the Enter - // keystroke to arrive before the input handler is ready. waitForPaneOutput(sessionName, assistantName, tmuxOpts) if beforeSend != nil { beforeSend() @@ -95,25 +79,22 @@ func sendAgentRunPromptIfRequested( preSendHash := tmux.ContentHash(preSendContent) if err := tmuxSendKeys(sessionName, prompt, true, tmuxOpts); err != nil { - return handlePromptSendError(w, wErr, gf, version, idempotencyKey, sessionName, tmuxOpts, err, "send") + return handlePromptSendError(ctx, sessionName, tmuxOpts, err, "send") } - // Codex startup can still occasionally drop the very first prompt even when - // a cursor is visible. If pane output does not change after send, retry once. if strings.EqualFold(strings.TrimSpace(assistantName), "codex") && !waitForPromptDelivery(sessionName, preSendHash, tmuxOpts) { waitForPaneOutput(sessionName, assistantName, tmuxOpts) if err := tmuxSendKeys(sessionName, prompt, true, tmuxOpts); err != nil { - return handlePromptSendError(w, wErr, gf, version, idempotencyKey, sessionName, tmuxOpts, err, "retry") + return handlePromptSendError(ctx, sessionName, tmuxOpts, err, "retry") } } return ExitOK } func handlePromptSendError( - w, wErr io.Writer, - gf GlobalFlags, - version, idempotencyKey, sessionName string, + ctx *cmdCtx, + sessionName string, tmuxOpts tmux.Options, err error, action string, @@ -121,16 +102,13 @@ func handlePromptSendError( if killErr := tmuxKillSession(sessionName, tmuxOpts); killErr != nil { slog.Debug("best-effort session kill failed", "session", sessionName, "error", killErr) } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", idempotencyKey, - ExitInternalError, "prompt_send_failed", err.Error(), map[string]any{ - "session_name": sessionName, - }, - ) - } - Errorf(wErr, "failed to %s initial prompt to %s: %v", action, sessionName, err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "prompt_send_failed", + err.Error(), + map[string]any{"session_name": sessionName}, + fmt.Sprintf("failed to %s initial prompt to %s: %v", action, sessionName, err), + ) } // waitForPaneOutput polls the tmux pane until the output stabilizes (stops diff --git a/internal/cli/cmd_agent_run_liveness_test.go b/internal/cli/cmd_agent_run_liveness_test.go index 10f43f2a..89c2f7d2 100644 --- a/internal/cli/cmd_agent_run_liveness_test.go +++ b/internal/cli/cmd_agent_run_liveness_test.go @@ -12,6 +12,47 @@ import ( "github.com/andyrewlee/amux/internal/tmux" ) +func TestHandlePromptSendErrorHumanMessageIncludesAction(t *testing.T) { + origKillSession := tmuxKillSession + defer func() { + tmuxKillSession = origKillSession + }() + + tmuxKillSession = func(_ string, _ tmux.Options) error { return nil } + + tests := []struct { + action string + want string + }{ + {action: "send", want: "Error: failed to send initial prompt to session-a: send failed\n"}, + {action: "retry", want: "Error: failed to retry initial prompt to session-a: send failed\n"}, + } + + for _, tt := range tests { + t.Run(tt.action, func(t *testing.T) { + var out, errOut bytes.Buffer + ctx := &cmdCtx{ + w: &out, + wErr: &errOut, + gf: GlobalFlags{}, + version: "test-v1", + cmd: "agent.run", + } + + code := handlePromptSendError(ctx, "session-a", tmux.Options{}, errors.New("send failed"), tt.action) + if code != ExitInternalError { + t.Fatalf("handlePromptSendError() code = %d, want %d", code, ExitInternalError) + } + if out.Len() != 0 { + t.Fatalf("expected no stdout output in text mode, got %q", out.String()) + } + if got := errOut.String(); got != tt.want { + t.Fatalf("stderr = %q, want %q", got, tt.want) + } + }) + } +} + func TestCmdAgentRunSessionExitsBeforeStartupReturnsInternalErrorAndDoesNotPersistTab(t *testing.T) { t.Setenv("HOME", t.TempDir()) diff --git a/internal/cli/cmd_agent_run_prompt_ready_test.go b/internal/cli/cmd_agent_run_prompt_ready_test.go index a15b4c95..08bf4661 100644 --- a/internal/cli/cmd_agent_run_prompt_ready_test.go +++ b/internal/cli/cmd_agent_run_prompt_ready_test.go @@ -116,10 +116,7 @@ func TestSendAgentRunPromptIfRequested_CodexRetriesWhenPromptNotDelivered(t *tes } code := sendAgentRunPromptIfRequested( - nil, nil, - GlobalFlags{JSON: true}, - "test-v1", - "", + &cmdCtx{gf: GlobalFlags{JSON: true}, version: "test-v1", cmd: "agent.run"}, "session-a", "codex", "Reply with READY only.", @@ -163,10 +160,7 @@ func TestSendAgentRunPromptIfRequested_NonCodexDoesNotRetry(t *testing.T) { } code := sendAgentRunPromptIfRequested( - nil, nil, - GlobalFlags{JSON: true}, - "test-v1", - "", + &cmdCtx{gf: GlobalFlags{JSON: true}, version: "test-v1", cmd: "agent.run"}, "session-b", "claude", "Reply with READY only.", @@ -212,10 +206,7 @@ func TestSendAgentRunPromptIfRequested_BeforeSendHookRunsBeforeSend(t *testing.T } code := sendAgentRunPromptIfRequested( - nil, nil, - GlobalFlags{JSON: true}, - "test-v1", - "", + &cmdCtx{gf: GlobalFlags{JSON: true}, version: "test-v1", cmd: "agent.run"}, "session-c", "claude", "Reply with READY only.", diff --git a/internal/cli/cmd_agent_run_write.go b/internal/cli/cmd_agent_run_write.go index 9989793e..8f98fdc7 100644 --- a/internal/cli/cmd_agent_run_write.go +++ b/internal/cli/cmd_agent_run_write.go @@ -75,47 +75,33 @@ func cmdAgentRun(w, wErr io.Writer, gf GlobalFlags, args []string, version strin err, ) } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ); handled { + ctx := &cmdCtx{ + w: w, + wErr: wErr, + gf: gf, + version: version, + cmd: "agent.run", + idemKey: *idempotencyKey, + } + + if handled, code := ctx.maybeReplay(); handled { return code } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, fmt.Sprintf("failed to initialize: %v", err)) } ws, err := svc.Store.Load(wsID) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitNotFound, "not_found", fmt.Sprintf("workspace %s not found", wsID), nil, - ) - } - Errorf(wErr, "workspace %s not found", wsID) - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", fmt.Sprintf("workspace %s not found", wsID), nil) } agentAssistant := assistantName ac, ok := svc.Config.Assistants[agentAssistant] if !ok { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitUsage, "unknown_assistant", "unknown assistant: "+agentAssistant, nil, - ) - } - Errorf(wErr, "unknown assistant: %s", agentAssistant) - return ExitUsage + return ctx.errResult(ExitUsage, "unknown_assistant", "unknown assistant: "+agentAssistant, nil) } // Generate tab ID and session name. @@ -129,14 +115,7 @@ func cmdAgentRun(w, wErr io.Writer, gf GlobalFlags, args []string, version strin cmd, cancel := tmuxStartSession(svc.TmuxOpts, createArgs...) defer cancel() if err := cmd.Run(); err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitInternalError, "session_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to create tmux session: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "session_failed", err.Error(), nil, fmt.Sprintf("failed to create tmux session: %v", err)) } // Tag the session. @@ -160,29 +139,28 @@ func cmdAgentRun(w, wErr io.Writer, gf GlobalFlags, args []string, version strin if killErr := tmuxKillSession(sessionName, svc.TmuxOpts); killErr != nil { slog.Debug("best-effort session kill failed", "session", sessionName, "error", killErr) } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitInternalError, "session_tag_failed", err.Error(), map[string]any{ - "session_name": sessionName, - "tag": tag.Key, - }, - ) - } - Errorf(wErr, "failed to tag session %s (%s): %v", sessionName, tag.Key, err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "session_tag_failed", + err.Error(), + map[string]any{ + "session_name": sessionName, + "tag": tag.Key, + }, + fmt.Sprintf("failed to tag session %s (%s): %v", sessionName, tag.Key, err), + ) } } if code := verifyStartedAgentSession( - w, wErr, gf, version, *idempotencyKey, sessionName, svc.TmuxOpts, + ctx, sessionName, svc.TmuxOpts, ); code != ExitOK { return code } waitPreContent := "" if code := sendAgentRunPromptIfRequested( - w, wErr, gf, version, *idempotencyKey, sessionName, agentAssistant, *prompt, svc.TmuxOpts, + ctx, sessionName, agentAssistant, *prompt, svc.TmuxOpts, func() { if *wait && *prompt != "" { // Capture baseline after startup readiness wait but before prompt send. @@ -211,17 +189,16 @@ func cmdAgentRun(w, wErr io.Writer, gf GlobalFlags, args []string, version strin if killErr := tmuxKillSession(sessionName, svc.TmuxOpts); killErr != nil { slog.Debug("best-effort session kill failed", "session", sessionName, "error", killErr) } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.run", *idempotencyKey, - ExitInternalError, "metadata_save_failed", err.Error(), map[string]any{ - "workspace_id": string(wsID), - "session_name": sessionName, - }, - ) - } - Errorf(wErr, "failed to persist workspace metadata: %v", err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "metadata_save_failed", + err.Error(), + map[string]any{ + "workspace_id": string(wsID), + "session_name": sessionName, + }, + fmt.Sprintf("failed to persist workspace metadata: %v", err), + ) } result := agentRunResult{ @@ -238,9 +215,7 @@ func cmdAgentRun(w, wErr io.Writer, gf GlobalFlags, args []string, version strin } if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, "agent.run", *idempotencyKey, result, - ) + return ctx.successResult(result) } PrintHuman(w, func(w io.Writer) { diff --git a/internal/cli/cmd_agent_send_execute.go b/internal/cli/cmd_agent_send_execute.go index 09affa6d..33cd146f 100644 --- a/internal/cli/cmd_agent_send_execute.go +++ b/internal/cli/cmd_agent_send_execute.go @@ -12,11 +12,7 @@ import ( ) func resolveSendJobForExecution( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, jobStore *sendJobStore, requestedJobID string, sessionName string, @@ -25,62 +21,42 @@ func resolveSendJobForExecution( if requestedJobID != "" { existing, ok, getErr := jobStore.get(requestedJobID) if getErr != nil { - if gf.JSON { - return sendJob{}, sessionName, returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_status_failed", getErr.Error(), map[string]any{"job_id": requestedJobID}, - ) - } - Errorf(wErr, "failed to load send job status: %v", getErr) - return sendJob{}, sessionName, ExitInternalError + return sendJob{}, sessionName, ctx.errResult( + ExitInternalError, "job_status_failed", getErr.Error(), map[string]any{"job_id": requestedJobID}, + fmt.Sprintf("failed to load send job status: %v", getErr), + ) } if !ok { - if gf.JSON { - return sendJob{}, sessionName, returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitNotFound, "not_found", "send job not found", map[string]any{"job_id": requestedJobID}, - ) - } - Errorf(wErr, "send job %s not found", requestedJobID) - return sendJob{}, sessionName, ExitNotFound + return sendJob{}, sessionName, ctx.errResult( + ExitNotFound, "not_found", "send job not found", map[string]any{"job_id": requestedJobID}, + fmt.Sprintf("send job %s not found", requestedJobID), + ) } job := existing // For process-job retries, job metadata is the source of truth. sessionName = job.SessionName if sessionName == "" { _, _ = jobStore.setStatus(job.ID, sendJobFailed, "stored send job is missing session name") - if gf.JSON { - return sendJob{}, sessionName, returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_status_failed", "stored send job is missing session name", map[string]any{"job_id": job.ID}, - ) - } - Errorf(wErr, "stored send job %s is missing session name", job.ID) - return sendJob{}, sessionName, ExitInternalError + return sendJob{}, sessionName, ctx.errResult( + ExitInternalError, "job_status_failed", "stored send job is missing session name", map[string]any{"job_id": job.ID}, + fmt.Sprintf("stored send job %s is missing session name", job.ID), + ) } return job, sessionName, ExitOK } job, err := jobStore.create(sessionName, agentID) if err != nil { - if gf.JSON { - return sendJob{}, sessionName, returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_create_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to create send job: %v", err) - return sendJob{}, sessionName, ExitInternalError + return sendJob{}, sessionName, ctx.errResult( + ExitInternalError, "job_create_failed", err.Error(), nil, + fmt.Sprintf("failed to create send job: %v", err), + ) } return job, sessionName, ExitOK } func dispatchAsyncAgentSend( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, jobStore *sendJobStore, sessionName string, agentID string, @@ -100,14 +76,10 @@ func dispatchAsyncAgentSend( sendJobFailed, "failed to start async send processor: "+err.Error(), ) - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_dispatch_failed", err.Error(), map[string]any{"job_id": job.ID}, - ) - } - Errorf(wErr, "failed to start async send processor: %v", err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, "job_dispatch_failed", err.Error(), map[string]any{"job_id": job.ID}, + fmt.Sprintf("failed to start async send processor: %v", err), + ) } result := agentSendResult{ @@ -118,12 +90,10 @@ func dispatchAsyncAgentSend( Sent: false, Delivered: false, } - if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, result, - ) + if ctx.gf.JSON { + return ctx.successResult(result) } - PrintHuman(w, func(w io.Writer) { + PrintHuman(ctx.w, func(w io.Writer) { fmt.Fprintf(w, "Queued text to %s (job: %s)\n", sessionName, job.ID) }) return ExitOK @@ -133,11 +103,7 @@ func dispatchAsyncAgentSend( // writing output. Callers use this to optionally append --wait data before // serializing the response. func executeAgentSendJobCore( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, jobStore *sendJobStore, svc *Services, sessionName string, @@ -152,14 +118,10 @@ func executeAgentSendJobCore( queueLock, err := waitForSessionQueueTurnForJob(jobStore, sessionName, job.ID) if err != nil { _, _ = jobStore.setStatus(job.ID, sendJobFailed, err.Error()) - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_queue_failed", err.Error(), map[string]any{"job_id": job.ID}, - ) - } - Errorf(wErr, "failed to join send queue: %v", err) - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "job_queue_failed", err.Error(), map[string]any{"job_id": job.ID}, + fmt.Sprintf("failed to join send queue: %v", err), + ) } defer releaseSessionQueueTurn(queueLock) @@ -167,24 +129,16 @@ func executeAgentSendJobCore( job, ok, err := jobStore.get(jobID) if err != nil { _, _ = jobStore.setStatus(jobID, sendJobFailed, err.Error()) - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_status_failed", err.Error(), map[string]any{"job_id": jobID}, - ) - } - Errorf(wErr, "failed to load send job status: %v", err) - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "job_status_failed", err.Error(), map[string]any{"job_id": jobID}, + fmt.Sprintf("failed to load send job status: %v", err), + ) } if !ok { - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_not_found", "send job not found", map[string]any{"job_id": jobID}, - ) - } - Errorf(wErr, "send job %s not found", jobID) - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "job_not_found", "send job not found", map[string]any{"job_id": jobID}, + fmt.Sprintf("send job %s not found", jobID), + ) } if job.Status == sendJobCanceled || job.Status == sendJobCompleted { @@ -200,14 +154,10 @@ func executeAgentSendJobCore( job, err = jobStore.setStatus(job.ID, sendJobRunning, "") if err != nil { - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_status_failed", err.Error(), map[string]any{"job_id": job.ID}, - ) - } - Errorf(wErr, "failed to update send job status: %v", err) - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "job_status_failed", err.Error(), map[string]any{"job_id": job.ID}, + fmt.Sprintf("failed to update send job status: %v", err), + ) } if job.Status != sendJobRunning { if job.Status == sendJobCanceled || job.Status == sendJobCompleted { @@ -220,22 +170,18 @@ func executeAgentSendJobCore( Delivered: false, }, "", ExitOK } - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "job_status_conflict", "send job is not runnable", map[string]any{ - "job_id": job.ID, - "status": string(job.Status), - "error": job.Error, - }, - ) - } + humanMessage := fmt.Sprintf("send job %s is %s and cannot be executed", job.ID, job.Status) if strings.TrimSpace(job.Error) != "" { - Errorf(wErr, "send job %s is %s: %s", job.ID, job.Status, job.Error) - } else { - Errorf(wErr, "send job %s is %s and cannot be executed", job.ID, job.Status) + humanMessage = fmt.Sprintf("send job %s is %s: %s", job.ID, job.Status, job.Error) } - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "job_status_conflict", "send job is not runnable", map[string]any{ + "job_id": job.ID, + "status": string(job.Status), + "error": job.Error, + }, + humanMessage, + ) } preContent := "" @@ -249,25 +195,21 @@ func executeAgentSendJobCore( failedJob = job failedJob.Status = sendJobFailed } - if gf.JSON { - return agentSendResult{}, "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, - ExitInternalError, "send_failed", err.Error(), map[string]any{ - "job_id": failedJob.ID, - "status": string(failedJob.Status), - "agent_id": agentID, - }, - ) - } - Errorf(wErr, "failed to send keys: %v", err) - return agentSendResult{}, "", ExitInternalError + return agentSendResult{}, "", ctx.errResult( + ExitInternalError, "send_failed", err.Error(), map[string]any{ + "job_id": failedJob.ID, + "status": string(failedJob.Status), + "agent_id": agentID, + }, + fmt.Sprintf("failed to send keys: %v", err), + ) } if completedJob, setErr := jobStore.setStatus(job.ID, sendJobCompleted, ""); setErr == nil { job = completedJob } else { - if !gf.JSON { - Errorf(wErr, "warning: sent text but failed to persist completion for job %s: %v", job.ID, setErr) + if !ctx.gf.JSON { + Errorf(ctx.wErr, "warning: sent text but failed to persist completion for job %s: %v", job.ID, setErr) } job.Status = sendJobCompleted job.Error = "" @@ -293,11 +235,7 @@ type sendWaitConfig struct { } func executeAgentSendJob( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, jobStore *sendJobStore, svc *Services, sessionName string, @@ -308,7 +246,7 @@ func executeAgentSendJob( waitCfg sendWaitConfig, ) int { result, preContent, code := executeAgentSendJobCore( - w, wErr, gf, version, idempotencyKey, + ctx, jobStore, svc, sessionName, agentID, text, enter, job, waitCfg.Wait, ) if code != ExitOK { @@ -320,12 +258,10 @@ func executeAgentSendJob( result.Response = &resp } - if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, agentSendCommandName, idempotencyKey, result, - ) + if ctx.gf.JSON { + return ctx.successResult(result) } - PrintHuman(w, func(w io.Writer) { + PrintHuman(ctx.w, func(w io.Writer) { switch { case result.Status == string(sendJobCanceled): fmt.Fprintf(w, "Send job %s canceled before execution\n", result.JobID) diff --git a/internal/cli/cmd_agent_send_resolve.go b/internal/cli/cmd_agent_send_resolve.go index ee4b1c57..24b6eae7 100644 --- a/internal/cli/cmd_agent_send_resolve.go +++ b/internal/cli/cmd_agent_send_resolve.go @@ -2,17 +2,12 @@ package cli import ( "errors" - "io" "github.com/andyrewlee/amux/internal/tmux" ) func resolveSessionForAgentSend( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, agentID string, opts tmux.Options, ) (string, int, bool) { @@ -22,31 +17,10 @@ func resolveSessionForAgentSend( } if errors.Is(err, errInvalidAgentID) { - if gf.JSON { - return "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.send", idempotencyKey, - ExitUsage, "invalid_agent_id", err.Error(), map[string]any{"agent_id": agentID}, - ), true - } - Errorf(wErr, "invalid --agent: %v", err) - return "", ExitUsage, true + return "", ctx.errResult(ExitUsage, "invalid_agent_id", err.Error(), map[string]any{"agent_id": agentID}, "invalid --agent: "+err.Error()), true } if errors.Is(err, errAgentNotFound) { - if gf.JSON { - return "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.send", idempotencyKey, - ExitNotFound, "not_found", "agent not found", map[string]any{"agent_id": agentID}, - ), true - } - Errorf(wErr, "agent %s not found", agentID) - return "", ExitNotFound, true + return "", ctx.errResult(ExitNotFound, "not_found", "agent not found", map[string]any{"agent_id": agentID}, "agent "+agentID+" not found"), true } - if gf.JSON { - return "", returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.send", idempotencyKey, - ExitInternalError, "session_lookup_failed", err.Error(), map[string]any{"agent_id": agentID}, - ), true - } - Errorf(wErr, "failed to resolve --agent %s: %v", agentID, err) - return "", ExitInternalError, true + return "", ctx.errResult(ExitInternalError, "session_lookup_failed", err.Error(), map[string]any{"agent_id": agentID}, "failed to resolve --agent "+agentID+": "+err.Error()), true } diff --git a/internal/cli/cmd_agent_send_validate.go b/internal/cli/cmd_agent_send_validate.go index 6d291f7c..ba0ae284 100644 --- a/internal/cli/cmd_agent_send_validate.go +++ b/internal/cli/cmd_agent_send_validate.go @@ -2,17 +2,12 @@ package cli import ( "fmt" - "io" "github.com/andyrewlee/amux/internal/tmux" ) func validateAgentSendSession( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, - version string, - idempotencyKey string, + ctx *cmdCtx, sessionName string, requestedJobID string, opts tmux.Options, @@ -20,27 +15,17 @@ func validateAgentSendSession( state, err := tmuxSessionStateFor(sessionName, opts) if err != nil { markSendJobFailedIfPresent(requestedJobID, "session lookup failed: "+err.Error()) - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.send", idempotencyKey, - ExitInternalError, "session_lookup_failed", err.Error(), map[string]any{ - "session_name": sessionName, - }, - ) - } - Errorf(wErr, "failed to check session %s: %v", sessionName, err) - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "session_lookup_failed", + err.Error(), + map[string]any{"session_name": sessionName}, + fmt.Sprintf("failed to check session %s: %v", sessionName, err), + ) } if !state.Exists { markSendJobFailedIfPresent(requestedJobID, "session not found") - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.send", idempotencyKey, - ExitNotFound, "not_found", fmt.Sprintf("session %s not found", sessionName), nil, - ) - } - Errorf(wErr, "session %s not found", sessionName) - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", fmt.Sprintf("session %s not found", sessionName), nil, fmt.Sprintf("session %s not found", sessionName)) } return ExitOK } diff --git a/internal/cli/cmd_agent_send_write.go b/internal/cli/cmd_agent_send_write.go index 18cac9e8..e7fbbb6c 100644 --- a/internal/cli/cmd_agent_send_write.go +++ b/internal/cli/cmd_agent_send_write.go @@ -50,26 +50,26 @@ func cmdAgentSend(w, wErr io.Writer, gf GlobalFlags, args []string, version stri errors.New("--idle-threshold must be > 0"), ) } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, agentSendCommandName, *idempotencyKey, - ); handled { + ctx := &cmdCtx{ + w: w, + wErr: wErr, + gf: gf, + version: version, + cmd: agentSendCommandName, + idemKey: *idempotencyKey, + } + + if handled, code := ctx.maybeReplay(); handled { return code } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, "failed to initialize: "+err.Error()) } if *agentID != "" { resolved, code, handled := resolveSessionForAgentSend( - w, wErr, gf, version, *idempotencyKey, *agentID, svc.TmuxOpts, + ctx, *agentID, svc.TmuxOpts, ) if handled { return code @@ -79,22 +79,11 @@ func cmdAgentSend(w, wErr io.Writer, gf GlobalFlags, args []string, version stri jobStore, err := newSendJobStore() if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, agentSendCommandName, *idempotencyKey, - ExitInternalError, "job_store_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize send job store: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "job_store_failed", err.Error(), nil, "failed to initialize send job store: "+err.Error()) } job, resolvedSessionName, code := resolveSendJobForExecution( - w, - wErr, - gf, - version, - *idempotencyKey, + ctx, jobStore, strings.TrimSpace(*jobIDFlag), sessionName, @@ -106,7 +95,7 @@ func cmdAgentSend(w, wErr io.Writer, gf GlobalFlags, args []string, version stri sessionName = resolvedSessionName if code := validateAgentSendSession( - w, wErr, gf, version, *idempotencyKey, sessionName, job.ID, svc.TmuxOpts, + ctx, sessionName, job.ID, svc.TmuxOpts, ); code != ExitOK { return code } @@ -114,11 +103,7 @@ func cmdAgentSend(w, wErr io.Writer, gf GlobalFlags, args []string, version stri // Internal process-job retries always execute inline in the child process. if *async && !*processJob { return dispatchAsyncAgentSend( - w, - wErr, - gf, - version, - *idempotencyKey, + ctx, jobStore, sessionName, *agentID, @@ -129,11 +114,7 @@ func cmdAgentSend(w, wErr io.Writer, gf GlobalFlags, args []string, version stri } return executeAgentSendJob( - w, - wErr, - gf, - version, - *idempotencyKey, + ctx, jobStore, svc, sessionName, diff --git a/internal/cli/cmd_agent_stop.go b/internal/cli/cmd_agent_stop.go index c7ae9d8d..8b048ced 100644 --- a/internal/cli/cmd_agent_stop.go +++ b/internal/cli/cmd_agent_stop.go @@ -34,6 +34,7 @@ func cmdAgentStop(w, wErr io.Writer, gf GlobalFlags, args []string, version stri } if *all { + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "agent.stop.all", idemKey: *idempotencyKey} if !*yes { if gf.JSON { ReturnError(w, "confirmation_required", "pass --yes to confirm stopping all agents", nil, version) @@ -42,24 +43,15 @@ func cmdAgentStop(w, wErr io.Writer, gf GlobalFlags, args []string, version stri Errorf(wErr, "pass --yes to confirm stopping all agents") return ExitUnsafeBlocked } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "agent.stop.all", *idempotencyKey, - ); handled { + if handled, code := ctx.maybeReplay(); handled { return code } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop.all", *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, fmt.Sprintf("failed to initialize: %v", err)) } return stopAllAgents( - w, wErr, gf, svc, version, "agent.stop.all", *idempotencyKey, *graceful, *gracePeriod, + ctx, svc, *graceful, *gracePeriod, ) } if sessionName == "" && *agentID == "" { @@ -68,88 +60,40 @@ func cmdAgentStop(w, wErr io.Writer, gf GlobalFlags, args []string, version stri if sessionName != "" && *agentID != "" { return returnUsageError(w, wErr, gf, usage, version, nil) } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ); handled { + + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "agent.stop", idemKey: *idempotencyKey} + + if handled, code := ctx.maybeReplay(); handled { return code } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, fmt.Sprintf("failed to initialize: %v", err)) } if *agentID != "" { resolved, err := resolveSessionNameForAgentID(*agentID, svc.TmuxOpts) if err != nil { if errors.Is(err, errInvalidAgentID) { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitUsage, "invalid_agent_id", err.Error(), map[string]any{"agent_id": *agentID}, - ) - } - Errorf(wErr, "invalid --agent: %v", err) - return ExitUsage + return ctx.errResult(ExitUsage, "invalid_agent_id", err.Error(), map[string]any{"agent_id": *agentID}, fmt.Sprintf("invalid --agent: %v", err)) } if errors.Is(err, errAgentNotFound) { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitNotFound, "not_found", "agent not found", map[string]any{"agent_id": *agentID}, - ) - } - Errorf(wErr, "agent %s not found", *agentID) - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", "agent not found", map[string]any{"agent_id": *agentID}, fmt.Sprintf("agent %s not found", *agentID)) } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitInternalError, "stop_failed", err.Error(), map[string]any{"agent_id": *agentID}, - ) - } - Errorf(wErr, "failed to resolve --agent %s: %v", *agentID, err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "stop_failed", err.Error(), map[string]any{"agent_id": *agentID}, fmt.Sprintf("failed to resolve --agent %s: %v", *agentID, err)) } sessionName = resolved } state, err := tmuxSessionStateFor(sessionName, svc.TmuxOpts) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitInternalError, "stop_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to check session: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "stop_failed", err.Error(), nil, fmt.Sprintf("failed to check session: %v", err)) } if !state.Exists { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitNotFound, "not_found", fmt.Sprintf("session %s not found", sessionName), nil, - ) - } - Errorf(wErr, "session %s not found", sessionName) - return ExitNotFound + return ctx.errResult(ExitNotFound, "not_found", fmt.Sprintf("session %s not found", sessionName), nil, fmt.Sprintf("session %s not found", sessionName)) } if err := stopAgentSession(sessionName, svc, *graceful, *gracePeriod); err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "agent.stop", *idempotencyKey, - ExitInternalError, "stop_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to stop session: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "stop_failed", err.Error(), nil, fmt.Sprintf("failed to stop session: %v", err)) } removeTabFromStore(svc, sessionName) @@ -157,9 +101,7 @@ func cmdAgentStop(w, wErr io.Writer, gf GlobalFlags, args []string, version stri result := agentStopResult{Stopped: []string{sessionName}, AgentID: *agentID} if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, "agent.stop", *idempotencyKey, result, - ) + return ctx.successResult(result) } PrintHuman(w, func(w io.Writer) { diff --git a/internal/cli/cmd_agent_stop_all.go b/internal/cli/cmd_agent_stop_all.go index 78b5f3c2..26a1fc7c 100644 --- a/internal/cli/cmd_agent_stop_all.go +++ b/internal/cli/cmd_agent_stop_all.go @@ -15,26 +15,14 @@ var ( ) func stopAllAgents( - w io.Writer, - wErr io.Writer, - gf GlobalFlags, + ctx *cmdCtx, svc *Services, - version string, - command string, - idempotencyKey string, graceful bool, gracePeriod time.Duration, ) int { sessions, err := listAgentSessionsForStopAll(svc.TmuxOpts) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, command, idempotencyKey, - ExitInternalError, "list_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to list agents: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "list_failed", err.Error(), nil, fmt.Sprintf("failed to list agents: %v", err)) } stopped := []string{} @@ -55,20 +43,17 @@ func stopAllAgents( removeTabFromStore(svc, s.Name) } if len(failed) > 0 { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, command, idempotencyKey, - ExitInternalError, "stop_partial_failed", "failed to stop one or more agents", map[string]any{ - "stopped": stopped, - "stopped_agent_ids": stoppedAgentIDs, - "failed": failed, - }, - ) + if ctx.gf.JSON { + return ctx.errResult(ExitInternalError, "stop_partial_failed", "failed to stop one or more agents", map[string]any{ + "stopped": stopped, + "stopped_agent_ids": stoppedAgentIDs, + "failed": failed, + }) } for _, failure := range failed { - Errorf(wErr, "failed to stop %s: %s", failure["session"], failure["error"]) + Errorf(ctx.wErr, "failed to stop %s: %s", failure["session"], failure["error"]) } - PrintHuman(w, func(w io.Writer) { + PrintHuman(ctx.w, func(w io.Writer) { fmt.Fprintf(w, "Stopped %d agent(s); %d failed\n", len(stopped), len(failed)) }) return ExitInternalError @@ -76,13 +61,11 @@ func stopAllAgents( result := agentStopResult{Stopped: stopped, StoppedAgentIDs: stoppedAgentIDs} - if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, command, idempotencyKey, result, - ) + if ctx.gf.JSON { + return ctx.successResult(result) } - PrintHuman(w, func(w io.Writer) { + PrintHuman(ctx.w, func(w io.Writer) { fmt.Fprintf(w, "Stopped %d agent(s)\n", len(stopped)) }) return ExitOK diff --git a/internal/cli/cmd_agent_stop_all_test.go b/internal/cli/cmd_agent_stop_all_test.go index 4bc94207..dd33c3a2 100644 --- a/internal/cli/cmd_agent_stop_all_test.go +++ b/internal/cli/cmd_agent_stop_all_test.go @@ -133,6 +133,99 @@ func TestCmdAgentStopAllPartialFailureReturnsError(t *testing.T) { } } +func TestCmdAgentStopAllConfirmationRequiredDoesNotCacheIdempotencyError(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + origSessionsByActivity := tmuxActiveAgentSessionsByActivity + origSessionsWithTags := tmuxSessionsWithTags + origKillSession := tmuxKillSession + defer func() { + tmuxActiveAgentSessionsByActivity = origSessionsByActivity + tmuxSessionsWithTags = origSessionsWithTags + tmuxKillSession = origKillSession + }() + + tmuxActiveAgentSessionsByActivity = func(_ time.Duration, _ tmux.Options) ([]tmux.SessionActivity, error) { + return []tmux.SessionActivity{ + {Name: "session-a", WorkspaceID: "ws-a", TabID: "tab-a"}, + }, nil + } + tmuxSessionsWithTags = func(_ map[string]string, _ []string, _ tmux.Options) ([]tmux.SessionTagValues, error) { + return nil, nil + } + + killCalls := 0 + tmuxKillSession = func(sessionName string, _ tmux.Options) error { + if sessionName != "session-a" { + t.Fatalf("tmuxKillSession() session = %q, want session-a", sessionName) + } + killCalls++ + return nil + } + + args := []string{"--all", "--idempotency-key", "idem-stop-all-confirm"} + + var firstOut, firstErr bytes.Buffer + firstCode := cmdAgentStop(&firstOut, &firstErr, GlobalFlags{JSON: true}, args, "test-v1") + if firstCode != ExitUnsafeBlocked { + t.Fatalf("first cmdAgentStop() code = %d, want %d", firstCode, ExitUnsafeBlocked) + } + if firstErr.Len() != 0 { + t.Fatalf("expected no stderr output in JSON mode, got %q", firstErr.String()) + } + if killCalls != 0 { + t.Fatalf("kill calls after unconfirmed request = %d, want 0", killCalls) + } + + var firstEnv Envelope + if err := json.Unmarshal(firstOut.Bytes(), &firstEnv); err != nil { + t.Fatalf("json.Unmarshal(first) error = %v", err) + } + if firstEnv.OK { + t.Fatalf("expected ok=false for unconfirmed stop-all") + } + if firstEnv.Error == nil || firstEnv.Error.Code != "confirmation_required" { + t.Fatalf("expected confirmation_required, got %#v", firstEnv.Error) + } + + confirmedArgs := []string{"--all", "--yes", "--graceful=false", "--idempotency-key", "idem-stop-all-confirm"} + + var secondOut, secondErr bytes.Buffer + secondCode := cmdAgentStop(&secondOut, &secondErr, GlobalFlags{JSON: true}, confirmedArgs, "test-v1") + if secondCode != ExitOK { + t.Fatalf("confirmed cmdAgentStop() code = %d, want %d", secondCode, ExitOK) + } + if secondErr.Len() != 0 { + t.Fatalf("expected no stderr output in JSON mode, got %q", secondErr.String()) + } + if killCalls != 1 { + t.Fatalf("kill calls after confirmed retry = %d, want 1", killCalls) + } + + var secondEnv Envelope + if err := json.Unmarshal(secondOut.Bytes(), &secondEnv); err != nil { + t.Fatalf("json.Unmarshal(second) error = %v", err) + } + if !secondEnv.OK { + t.Fatalf("expected ok=true for confirmed stop-all, got %#v", secondEnv.Error) + } + + var replayOut, replayErr bytes.Buffer + replayCode := cmdAgentStop(&replayOut, &replayErr, GlobalFlags{JSON: true}, confirmedArgs, "test-v1") + if replayCode != ExitOK { + t.Fatalf("replay cmdAgentStop() code = %d, want %d", replayCode, ExitOK) + } + if replayErr.Len() != 0 { + t.Fatalf("expected no replay stderr output in JSON mode, got %q", replayErr.String()) + } + if replayOut.String() != secondOut.String() { + t.Fatalf("replayed output mismatch\nfirst confirmed:\n%s\nreplay:\n%s", secondOut.String(), replayOut.String()) + } + if killCalls != 1 { + t.Fatalf("kill calls after replay = %d, want 1", killCalls) + } +} + func TestCmdAgentStopAllExcludesPartiallyTaggedSessionsWithoutType(t *testing.T) { t.Setenv("HOME", t.TempDir()) diff --git a/internal/cli/cmd_agent_stop_target_test.go b/internal/cli/cmd_agent_stop_target_test.go index 9866a100..8ee4908f 100644 --- a/internal/cli/cmd_agent_stop_target_test.go +++ b/internal/cli/cmd_agent_stop_target_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "strings" "testing" "github.com/andyrewlee/amux/internal/tmux" @@ -38,6 +39,28 @@ func TestCmdAgentStopInvalidAgentIDReturnsUsage(t *testing.T) { } } +func TestCmdAgentStopInvalidAgentIDHumanErrorKeepsContext(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + var out, errOut bytes.Buffer + code := cmdAgentStop( + &out, + &errOut, + GlobalFlags{}, + []string{"--agent", "invalid-id"}, + "test-v1", + ) + if code != ExitUsage { + t.Fatalf("cmdAgentStop() code = %d, want %d", code, ExitUsage) + } + if out.Len() != 0 { + t.Fatalf("expected no stdout output in text mode, got %q", out.String()) + } + if got := errOut.String(); !strings.HasPrefix(got, "Error: invalid --agent:") { + t.Fatalf("stderr = %q, want prefix %q", got, "Error: invalid --agent:") + } +} + func TestCmdAgentStopStaleAgentIDReturnsNotFound(t *testing.T) { t.Setenv("HOME", t.TempDir()) diff --git a/internal/cli/cmd_context.go b/internal/cli/cmd_context.go new file mode 100644 index 00000000..7a06c9c7 --- /dev/null +++ b/internal/cli/cmd_context.go @@ -0,0 +1,43 @@ +package cli + +import "io" + +// cmdCtx bundles the common parameters threaded through CLI command handlers. +type cmdCtx struct { + w io.Writer + wErr io.Writer + gf GlobalFlags + version string + cmd string + idemKey string +} + +// errResult returns a JSON error (when --json is set) or a human-readable +// error message, and stores an idempotent response when an idempotency key +// is present. humanMessage optionally overrides the non-JSON stderr text. +func (c *cmdCtx) errResult(exitCode int, errorCode, message string, details any, humanMessage ...string) int { + if c.gf.JSON { + return returnJSONErrorMaybeIdempotent( + c.w, c.wErr, c.gf, c.version, c.cmd, c.idemKey, + exitCode, errorCode, message, details, + ) + } + text := message + if len(humanMessage) > 0 && humanMessage[0] != "" { + text = humanMessage[0] + } + Errorf(c.wErr, "%s", text) + return exitCode +} + +// successResult returns a JSON success envelope with idempotency support. +func (c *cmdCtx) successResult(data any) int { + return returnJSONSuccessWithIdempotency( + c.w, c.wErr, c.gf, c.version, c.cmd, c.idemKey, data, + ) +} + +// maybeReplay checks whether a previous idempotent response can be replayed. +func (c *cmdCtx) maybeReplay() (bool, int) { + return maybeReplayIdempotentResponse(c.w, c.wErr, c.gf, c.version, c.cmd, c.idemKey) +} diff --git a/internal/cli/cmd_context_test.go b/internal/cli/cmd_context_test.go new file mode 100644 index 00000000..3b7cfcbb --- /dev/null +++ b/internal/cli/cmd_context_test.go @@ -0,0 +1,59 @@ +package cli + +import ( + "bytes" + "encoding/json" + "testing" +) + +func TestCmdCtxErrResultUsesHumanOverrideInTextMode(t *testing.T) { + var out, errOut bytes.Buffer + ctx := &cmdCtx{ + w: &out, + wErr: &errOut, + gf: GlobalFlags{}, + version: "test-v1", + cmd: "test.command", + } + + code := ctx.errResult(ExitInternalError, "boom", "json message", nil, "human message") + if code != ExitInternalError { + t.Fatalf("errResult() code = %d, want %d", code, ExitInternalError) + } + if out.Len() != 0 { + t.Fatalf("expected no stdout output in text mode, got %q", out.String()) + } + if got := errOut.String(); got != "Error: human message\n" { + t.Fatalf("stderr = %q, want %q", got, "Error: human message\n") + } +} + +func TestCmdCtxErrResultKeepsJSONMessageWhenHumanOverrideProvided(t *testing.T) { + var out, errOut bytes.Buffer + ctx := &cmdCtx{ + w: &out, + wErr: &errOut, + gf: GlobalFlags{JSON: true}, + version: "test-v1", + cmd: "test.command", + } + + code := ctx.errResult(ExitInternalError, "boom", "json message", nil, "human message") + if code != ExitInternalError { + t.Fatalf("errResult() code = %d, want %d", code, ExitInternalError) + } + if errOut.Len() != 0 { + t.Fatalf("expected no stderr output in JSON mode, got %q", errOut.String()) + } + + var env Envelope + if err := json.Unmarshal(out.Bytes(), &env); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if env.OK { + t.Fatalf("expected ok=false") + } + if env.Error == nil || env.Error.Message != "json message" { + t.Fatalf("expected JSON message to be preserved, got %#v", env.Error) + } +} diff --git a/internal/cli/cmd_workspace_create.go b/internal/cli/cmd_workspace_create.go index bd5ad68d..4748a1ff 100644 --- a/internal/cli/cmd_workspace_create.go +++ b/internal/cli/cmd_workspace_create.go @@ -69,83 +69,41 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio ) } } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ); handled { + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "workspace.create", idemKey: *idempotencyKey} + + if handled, code := ctx.maybeReplay(); handled { return code } projectPath, err := canonicalizeProjectPath(*project) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitUsage, "invalid_project_path", err.Error(), map[string]any{"project": *project}, - ) - } - Errorf(wErr, "invalid --project path: %v", err) - return ExitUsage + return ctx.errResult(ExitUsage, "invalid_project_path", err.Error(), map[string]any{"project": *project}, fmt.Sprintf("invalid --project path: %v", err)) } if !git.IsGitRepository(projectPath) { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitUsage, "not_git_repo", projectPath+" is not a git repository", nil, - ) - } - Errorf(wErr, "%s is not a git repository", projectPath) - return ExitUsage + return ctx.errResult(ExitUsage, "not_git_repo", projectPath+" is not a git repository", nil) } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, fmt.Sprintf("failed to initialize: %v", err)) } assistantExplicit := assistantName != "" if assistantName == "" { assistantName = svc.Config.ResolvedDefaultAssistant() } if !svc.Config.IsAssistantKnown(assistantName) { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitUsage, "unknown_assistant", "unknown assistant: "+assistantName, nil, - ) - } - Errorf(wErr, "unknown assistant: %s", assistantName) - return ExitUsage + return ctx.errResult(ExitUsage, "unknown_assistant", "unknown assistant: "+assistantName, nil) } // Require project to be registered before creating a workspace. registered, err := svc.Registry.Projects() if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "registry_read_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to read project registry: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "registry_read_failed", err.Error(), nil, fmt.Sprintf("failed to read project registry: %v", err)) } if !isProjectRegistered(registered, projectPath) { msg := fmt.Sprintf("project %s is not registered; run `amux project add %s` first", projectPath, projectPath) - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitUsage, "project_not_registered", msg, map[string]any{"project": projectPath}, - ) - } - Errorf(wErr, "%s", msg) - return ExitUsage + return ctx.errResult(ExitUsage, "project_not_registered", msg, map[string]any{"project": projectPath}) } // Determine base branch @@ -163,21 +121,12 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio // Idempotent path: if the target worktree already exists for this repo, reuse it. existingWS, found, err := loadExistingWorkspaceAtPath(svc, projectPath, wsPath, name, baseBranch, assistantName, assistantExplicit) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "existing_workspace_check_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to check existing workspace: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "existing_workspace_check_failed", err.Error(), nil, fmt.Sprintf("failed to check existing workspace: %v", err)) } if found { info := workspaceToInfo(existingWS) if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, "workspace.create", *idempotencyKey, info, - ) + return ctx.successResult(info) } PrintHuman(w, func(w io.Writer) { fmt.Fprintf(w, "Using existing workspace %s (%s) at %s\n", info.Name, info.ID, info.Root) @@ -187,14 +136,7 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio // Create the worktree if err := git.CreateWorkspace(projectPath, wsPath, name, baseBranch); err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "create_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to create workspace: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "create_failed", err.Error(), nil, fmt.Sprintf("failed to create workspace: %v", err)) } // Wait for .git file to appear (same pattern as workspace_service.go) @@ -205,22 +147,15 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio if cleanupErr != nil { msg = fmt.Sprintf("%s (cleanup failed: %v)", msg, cleanupErr) } - if gf.JSON { - details := map[string]any{ - "workspace_root": wsPath, - "workspace_id": name, - "git_file": gitFile, - } - if cleanupErr != nil { - details["cleanup_error"] = cleanupErr.Error() - } - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "workspace_not_ready", msg, details, - ) + details := map[string]any{ + "workspace_root": wsPath, + "workspace_id": name, + "git_file": gitFile, } - Errorf(wErr, "%s", msg) - return ExitInternalError + if cleanupErr != nil { + details["cleanup_error"] = cleanupErr.Error() + } + return ctx.errResult(ExitInternalError, "workspace_not_ready", msg, details) } // Save metadata @@ -232,32 +167,26 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio if cleanupErr != nil { msg = fmt.Sprintf("%s (cleanup failed: %v)", msg, cleanupErr) } - if gf.JSON { - details := map[string]any{ - "workspace_root": wsPath, - "workspace_id": name, - } - if cleanupErr != nil { - details["cleanup_error"] = cleanupErr.Error() - } - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.create", *idempotencyKey, - ExitInternalError, "save_failed", msg, details, - ) + details := map[string]any{ + "workspace_root": wsPath, + "workspace_id": name, } - Errorf(wErr, "failed to save workspace metadata: %v", err) if cleanupErr != nil { - Errorf(wErr, "cleanup failed: %v", cleanupErr) + details["cleanup_error"] = cleanupErr.Error() } - return ExitInternalError + return ctx.errResult( + ExitInternalError, + "save_failed", + msg, + details, + workspaceCreateSaveFailedHumanMessage(err, cleanupErr), + ) } info := workspaceToInfo(ws) if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, wErr, gf, version, "workspace.create", *idempotencyKey, info, - ) + return ctx.successResult(info) } PrintHuman(w, func(w io.Writer) { @@ -266,6 +195,13 @@ func cmdWorkspaceCreate(w, wErr io.Writer, gf GlobalFlags, args []string, versio return ExitOK } +func workspaceCreateSaveFailedHumanMessage(saveErr, cleanupErr error) string { + if cleanupErr != nil { + return fmt.Sprintf("%v (cleanup failed: %v)", saveErr, cleanupErr) + } + return fmt.Sprintf("failed to save workspace metadata: %v", saveErr) +} + func rollbackWorkspaceCreate(repoPath, workspacePath, branch string, deleteBranch bool) error { var errs []string diff --git a/internal/cli/cmd_workspace_create_human_test.go b/internal/cli/cmd_workspace_create_human_test.go new file mode 100644 index 00000000..fb8f7970 --- /dev/null +++ b/internal/cli/cmd_workspace_create_human_test.go @@ -0,0 +1,35 @@ +package cli + +import ( + "errors" + "testing" +) + +func TestWorkspaceCreateSaveFailedHumanMessage(t *testing.T) { + tests := []struct { + name string + saveErr error + cleanupErr error + want string + }{ + { + name: "save only", + saveErr: errors.New("metadata write failed"), + want: "failed to save workspace metadata: metadata write failed", + }, + { + name: "save and cleanup", + saveErr: errors.New("metadata write failed"), + cleanupErr: errors.New("remove worktree: permission denied"), + want: "metadata write failed (cleanup failed: remove worktree: permission denied)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := workspaceCreateSaveFailedHumanMessage(tt.saveErr, tt.cleanupErr); got != tt.want { + t.Fatalf("workspaceCreateSaveFailedHumanMessage() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/cli/cmd_workspace_remove.go b/internal/cli/cmd_workspace_remove.go index 478696d5..e678b291 100644 --- a/internal/cli/cmd_workspace_remove.go +++ b/internal/cli/cmd_workspace_remove.go @@ -43,55 +43,27 @@ func cmdWorkspaceRemove(w, wErr io.Writer, gf GlobalFlags, args []string, versio fmt.Errorf("invalid workspace id: %s", wsIDArg), ) } - if handled, code := maybeReplayIdempotentResponse( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ); handled { + ctx := &cmdCtx{w: w, wErr: wErr, gf: gf, version: version, cmd: "workspace.remove", idemKey: *idempotencyKey} + + if handled, code := ctx.maybeReplay(); handled { return code } svc, err := NewServices(version) if err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitInternalError, "init_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to initialize: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "init_failed", err.Error(), nil, fmt.Sprintf("failed to initialize: %v", err)) } ws, err := svc.Store.Load(wsID) if err != nil { if os.IsNotExist(err) { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitNotFound, "not_found", fmt.Sprintf("workspace %s not found", wsID), nil, - ) - } - Errorf(wErr, "workspace %s not found", wsID) - return ExitNotFound - } - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitInternalError, "metadata_load_failed", err.Error(), map[string]any{"workspace_id": string(wsID)}, - ) + return ctx.errResult(ExitNotFound, "not_found", fmt.Sprintf("workspace %s not found", wsID), nil) } - Errorf(wErr, "failed to load workspace metadata %s: %v", wsID, err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "metadata_load_failed", err.Error(), map[string]any{"workspace_id": string(wsID)}, fmt.Sprintf("failed to load workspace metadata %s: %v", wsID, err)) } if ws.IsPrimaryCheckout() { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitUnsafeBlocked, "primary_checkout", "cannot remove primary checkout", nil, - ) - } - Errorf(wErr, "cannot remove primary checkout") - return ExitUnsafeBlocked + return ctx.errResult(ExitUnsafeBlocked, "primary_checkout", "cannot remove primary checkout", nil) } // Kill tmux sessions for this workspace @@ -101,14 +73,7 @@ func cmdWorkspaceRemove(w, wErr io.Writer, gf GlobalFlags, args []string, versio // Remove worktree if err := git.RemoveWorkspace(ws.Repo, ws.Root); err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitInternalError, "remove_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to remove worktree: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "remove_failed", err.Error(), nil, fmt.Sprintf("failed to remove worktree: %v", err)) } // Delete branch (best-effort) @@ -118,28 +83,13 @@ func cmdWorkspaceRemove(w, wErr io.Writer, gf GlobalFlags, args []string, versio // Delete metadata if err := svc.Store.Delete(wsID); err != nil { - if gf.JSON { - return returnJSONErrorMaybeIdempotent( - w, wErr, gf, version, "workspace.remove", *idempotencyKey, - ExitInternalError, "metadata_delete_failed", err.Error(), nil, - ) - } - Errorf(wErr, "failed to delete metadata: %v", err) - return ExitInternalError + return ctx.errResult(ExitInternalError, "metadata_delete_failed", err.Error(), nil, fmt.Sprintf("failed to delete metadata: %v", err)) } info := workspaceToInfo(ws) if gf.JSON { - return returnJSONSuccessWithIdempotency( - w, - wErr, - gf, - version, - "workspace.remove", - *idempotencyKey, - map[string]any{"removed": info}, - ) + return ctx.successResult(map[string]any{"removed": info}) } PrintHuman(w, func(w io.Writer) { diff --git a/internal/tmux/activity.go b/internal/tmux/activity.go index 6f3779d2..d3046f7c 100644 --- a/internal/tmux/activity.go +++ b/internal/tmux/activity.go @@ -2,7 +2,6 @@ package tmux import ( "crypto/md5" - "os/exec" "strconv" "strings" "time" @@ -22,21 +21,14 @@ func ActiveAgentSessionsByActivity(window time.Duration, opts Options) ([]Sessio defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil, nil - } + if isExitCode1(err) { + return nil, nil } return nil, err } now := time.Now() - lines := strings.Split(strings.TrimSpace(string(output)), "\n") latest := make(map[string]SessionActivity) - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } + for _, line := range parseOutputLines(output) { parts := strings.Split(line, "\t") if len(parts) < 6 { continue @@ -109,10 +101,8 @@ func SetMonitorActivityOn(opts Options) error { cmd, cancel := tmuxCommand(opts, "set-option", "-g", "monitor-activity", "on") defer cancel() if err := cmd.Run(); err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil - } + if isExitCode1(err) { + return nil } return err } @@ -124,10 +114,8 @@ func SetStatusOff(opts Options) error { cmd, cancel := tmuxCommand(opts, "set-option", "-g", "status", "off") defer cancel() if err := cmd.Run(); err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil - } + if isExitCode1(err) { + return nil } return err } diff --git a/internal/tmux/capture.go b/internal/tmux/capture.go index e1ee0e2f..c486e0b6 100644 --- a/internal/tmux/capture.go +++ b/internal/tmux/capture.go @@ -25,11 +25,7 @@ func sessionPaneID(sessionName string, opts Options) (string, error) { } var firstAlive string - for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } + for _, line := range parseOutputLines(output) { parts := strings.Split(line, "\t") if len(parts) < 3 { continue diff --git a/internal/tmux/clients.go b/internal/tmux/clients.go index aac0193a..d2fb8168 100644 --- a/internal/tmux/clients.go +++ b/internal/tmux/clients.go @@ -1,7 +1,6 @@ package tmux import ( - "os/exec" "strconv" "strings" ) @@ -17,7 +16,7 @@ func SessionNamesWithClients(opts Options) (map[string]bool, error) { defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 { + if isExitCode1(err) { stderr := strings.ToLower(strings.TrimSpace(string(output))) // No attached clients should not fail detached-session GC. if stderr == "" || strings.Contains(stderr, "no client") || strings.Contains(stderr, "can't find client") { @@ -26,11 +25,7 @@ func SessionNamesWithClients(opts Options) (map[string]bool, error) { } return attached, err } - for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { - name := strings.TrimSpace(line) - if name == "" { - continue - } + for _, name := range parseOutputLines(output) { attached[name] = true } return attached, nil @@ -52,10 +47,8 @@ func SessionHasClients(sessionName string, opts Options) (bool, error) { defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return false, nil - } + if isExitCode1(err) { + return false, nil } return false, err } diff --git a/internal/tmux/errors.go b/internal/tmux/errors.go index f8b940f0..81e232b8 100644 --- a/internal/tmux/errors.go +++ b/internal/tmux/errors.go @@ -1,6 +1,17 @@ package tmux -import "strings" +import ( + "errors" + "os/exec" + "strings" +) + +// isExitCode1 reports whether err is an exec.ExitError with exit code 1. +// tmux returns exit code 1 for "not found" conditions (no session, no server, etc.). +func isExitCode1(err error) bool { + var exitErr *exec.ExitError + return errors.As(err, &exitErr) && exitErr.ExitCode() == 1 +} // IsNoServerError reports whether err indicates that no tmux server is // currently running for the selected socket/server name. diff --git a/internal/tmux/tags.go b/internal/tmux/tags.go index 549a1f90..fb79793e 100644 --- a/internal/tmux/tags.go +++ b/internal/tmux/tags.go @@ -2,7 +2,6 @@ package tmux import ( "fmt" - "os/exec" "sort" "strings" ) @@ -76,20 +75,13 @@ func listSessionsWithTags(tags map[string]string, opts Options) ([]sessionTagRow defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil, keys, nil - } + if isExitCode1(err) { + return nil, keys, nil } return nil, keys, err } - lines := strings.Split(strings.TrimSpace(string(output)), "\n") var rows []sessionTagRow - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } + for _, line := range parseOutputLines(output) { parts := strings.Split(line, tagFieldSeparator) if len(parts) == 0 { continue @@ -148,16 +140,14 @@ func SetSessionTagValue(sessionName, key, value string, opts Options) error { defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - stderr := strings.TrimSpace(string(output)) - if strings.Contains(stderr, "session not found") || - strings.Contains(stderr, "no such session") || - strings.Contains(stderr, "can't find session") { - return nil - } - return fmt.Errorf("set-option -t %s %s: %s", sessionName, key, stderr) + if isExitCode1(err) { + stderr := strings.TrimSpace(string(output)) + if strings.Contains(stderr, "session not found") || + strings.Contains(stderr, "no such session") || + strings.Contains(stderr, "can't find session") { + return nil } + return fmt.Errorf("set-option -t %s %s: %s", sessionName, key, stderr) } return err } @@ -184,20 +174,8 @@ func SetSessionTagValues(sessionName string, tags []OptionValue, opts Options) e return nil } - args := make([]string, 0, len(tags)*6) - added := 0 target := exactSessionOptionTarget(sessionName) - for _, candidate := range tags { - key := strings.TrimSpace(candidate.Key) - if key == "" { - continue - } - if added > 0 { - args = append(args, ";") - } - args = append(args, "set-option", "-t", target, key, candidate.Value) - added++ - } + args, added := buildMultiSetOptionArgs([]string{"-t", target}, tags) if added == 0 { return nil } @@ -206,16 +184,14 @@ func SetSessionTagValues(sessionName string, tags []OptionValue, opts Options) e defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - stderr := strings.TrimSpace(string(output)) - if strings.Contains(stderr, "session not found") || - strings.Contains(stderr, "no such session") || - strings.Contains(stderr, "can't find session") { - return nil - } - return fmt.Errorf("set-option -t %s (multi): %s", sessionName, stderr) + if isExitCode1(err) { + stderr := strings.TrimSpace(string(output)) + if strings.Contains(stderr, "session not found") || + strings.Contains(stderr, "no such session") || + strings.Contains(stderr, "can't find session") { + return nil } + return fmt.Errorf("set-option -t %s (multi): %s", sessionName, stderr) } return err } diff --git a/internal/tmux/tmux.go b/internal/tmux/tmux.go index fa268619..d667fc5f 100644 --- a/internal/tmux/tmux.go +++ b/internal/tmux/tmux.go @@ -110,19 +110,13 @@ func AllSessionStates(opts Options) (map[string]SessionState, error) { defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return map[string]SessionState{}, nil - } + if isExitCode1(err) { + return map[string]SessionState{}, nil } return nil, err } states := make(map[string]SessionState) - for _, line := range strings.Split(strings.TrimSpace(string(output)), "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } + for _, line := range parseOutputLines(output) { parts := strings.SplitN(line, "\t", 2) if len(parts) != 2 { continue @@ -191,10 +185,8 @@ func hasSession(sessionName string, opts Options) (bool, error) { cmd, cancel := tmuxCommand(opts, "has-session", "-t", sessionTarget(sessionName)) defer cancel() if err := cmd.Run(); err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return false, nil - } + if isExitCode1(err) { + return false, nil } return false, err } @@ -213,13 +205,9 @@ func hasLivePane(sessionName string, opts Options) (bool, error) { defer cancel() output, err := cmd.Output() if err != nil { - // Treat exit code 1 as "no live pane" (session may have died between checks) - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return false, nil - } + if isExitCode1(err) { + return false, nil } - // Return actual error for unexpected failures (callers can decide tolerance) return false, err } lines := strings.Fields(string(output)) @@ -249,10 +237,8 @@ func KillSession(sessionName string, opts Options) error { cmd, cancel := tmuxCommand(opts, "kill-session", "-t", sessionTarget(sessionName)) defer cancel() if err := cmd.Run(); err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil - } + if isExitCode1(err) { + return nil } return err } @@ -273,10 +259,8 @@ func PanePIDs(sessionName string, opts Options) ([]int, error) { defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil, nil - } + if isExitCode1(err) { + return nil, nil } return nil, err } @@ -313,10 +297,8 @@ func SessionTagValue(sessionName, key string, opts Options) (string, error) { defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return "", nil - } + if isExitCode1(err) { + return "", nil } return "", err } @@ -340,14 +322,12 @@ func GlobalOptionValue(key string, opts Options) (string, error) { defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 && tmuxShowOptionMissingError(string(output)) { + if isExitCode1(err) { + if tmuxShowOptionMissingError(string(output)) { return "", nil } - if exitErr.ExitCode() == 1 { - stderr := strings.TrimSpace(string(output)) - return "", fmt.Errorf("show-options -g %s: %s", key, stderr) - } + stderr := strings.TrimSpace(string(output)) + return "", fmt.Errorf("show-options -g %s: %s", key, stderr) } return "", err } @@ -377,30 +357,21 @@ func SetGlobalOptionValue(key, value string, opts Options) error { defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - stderr := strings.TrimSpace(string(output)) - // Unknown/invalid options are tolerated on writes for compatibility - // with older tmux versions that may not recognize newer keys. - if strings.Contains(stderr, "invalid option") || strings.Contains(stderr, "unknown option") { - return nil - } - return fmt.Errorf("set-option -g %s: %s", key, stderr) + if isExitCode1(err) { + stderr := strings.TrimSpace(string(output)) + if strings.Contains(stderr, "invalid option") || strings.Contains(stderr, "unknown option") { + return nil } + return fmt.Errorf("set-option -g %s: %s", key, stderr) } return err } return nil } -// SetGlobalOptionValues sets multiple tmux global options in a single tmux command. -func SetGlobalOptionValues(values []OptionValue, opts Options) error { - if len(values) == 0 { - return nil - } - if err := EnsureAvailable(); err != nil { - return err - } +// buildMultiSetOptionArgs builds semicolon-separated tmux set-option arguments. +// scope provides the targeting flags (e.g. []string{"-g"} or []string{"-t", target}). +func buildMultiSetOptionArgs(scope []string, values []OptionValue) ([]string, int) { args := make([]string, 0, len(values)*6) added := 0 for _, candidate := range values { @@ -411,9 +382,23 @@ func SetGlobalOptionValues(values []OptionValue, opts Options) error { if added > 0 { args = append(args, ";") } - args = append(args, "set-option", "-g", key, candidate.Value) + args = append(args, "set-option") + args = append(args, scope...) + args = append(args, key, candidate.Value) added++ } + return args, added +} + +// SetGlobalOptionValues sets multiple tmux global options in a single tmux command. +func SetGlobalOptionValues(values []OptionValue, opts Options) error { + if len(values) == 0 { + return nil + } + if err := EnsureAvailable(); err != nil { + return err + } + args, added := buildMultiSetOptionArgs([]string{"-g"}, values) if added == 0 { return nil } @@ -421,16 +406,12 @@ func SetGlobalOptionValues(values []OptionValue, opts Options) error { defer cancel() output, err := cmd.CombinedOutput() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - stderr := strings.TrimSpace(string(output)) - // Keep parity with SetGlobalOptionValue: tolerate unknown/invalid - // option keys so mixed tmux versions don't fail batch writes. - if strings.Contains(stderr, "invalid option") || strings.Contains(stderr, "unknown option") { - return nil - } - return fmt.Errorf("set-option -g (multi): %s", stderr) + if isExitCode1(err) { + stderr := strings.TrimSpace(string(output)) + if strings.Contains(stderr, "invalid option") || strings.Contains(stderr, "unknown option") { + return nil } + return fmt.Errorf("set-option -g (multi): %s", stderr) } return err } @@ -458,13 +439,9 @@ func sanitize(value string) string { return strings.Trim(b.String(), "-") } -// exactTarget returns a tmux target string that forces exact session-name -// matching. Without the "=" prefix tmux falls back to prefix matching, -// which can cause commands aimed at "amux-ws-tab-1" to hit "amux-ws-tab-10". -func exactTarget(name string) string { return "=" + name } - // sessionTarget returns a tmux target for session-level commands. -// Uses "=" prefix for exact session matching. +// Uses "=" prefix for exact session matching, preventing tmux from +// prefix-matching "amux-ws-tab-1" to "amux-ws-tab-10". func sessionTarget(name string) string { return "=" + name } // exactSessionOptionTarget returns a tmux target for session-scoped options. @@ -474,6 +451,23 @@ func sessionTarget(name string) string { return "=" + name } // tab ID, making prefix collisions practically impossible. func exactSessionOptionTarget(name string) string { return name } +// parseOutputLines splits tmux command output into non-empty trimmed lines. +func parseOutputLines(output []byte) []string { + raw := strings.TrimSpace(string(output)) + if raw == "" { + return nil + } + lines := strings.Split(raw, "\n") + result := make([]string, 0, len(lines)) + for _, line := range lines { + line = strings.TrimSpace(line) + if line != "" { + result = append(result, line) + } + } + return result +} + func shellQuote(value string) string { if value == "" { return "''" diff --git a/internal/tmux/tmux_global_options.go b/internal/tmux/tmux_global_options.go index dada7856..894d913a 100644 --- a/internal/tmux/tmux_global_options.go +++ b/internal/tmux/tmux_global_options.go @@ -2,7 +2,6 @@ package tmux import ( "fmt" - "os/exec" "strings" ) @@ -37,7 +36,7 @@ func GlobalOptionValues(keys []string, opts Options) (map[string]string, error) if err != nil { // Unlike show-options -g -v, display-message does not provide a reliable // "missing option" sentinel. Treat exit 1 as an operational error. - if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 1 { + if isExitCode1(err) { stderr := strings.TrimSpace(string(output)) if stderr == "" { return values, err diff --git a/internal/tmux/tmux_sessions.go b/internal/tmux/tmux_sessions.go index cfb40d18..a23a0671 100644 --- a/internal/tmux/tmux_sessions.go +++ b/internal/tmux/tmux_sessions.go @@ -1,9 +1,6 @@ package tmux -import ( - "os/exec" - "strings" -) +import "strings" // ListSessions returns all tmux session names for the configured server. func ListSessions(opts Options) ([]string, error) { @@ -14,23 +11,12 @@ func ListSessions(opts Options) ([]string, error) { defer cancel() output, err := cmd.Output() if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() == 1 { - return nil, nil - } + if isExitCode1(err) { + return nil, nil } return nil, err } - lines := strings.Split(strings.TrimSpace(string(output)), "\n") - var sessions []string - for _, line := range lines { - name := strings.TrimSpace(line) - if name == "" { - continue - } - sessions = append(sessions, name) - } - return sessions, nil + return parseOutputLines(output), nil } // KillSessionsWithPrefix kills all sessions with a matching name prefix. diff --git a/internal/tmux/tmux_test.go b/internal/tmux/tmux_test.go index c0631be6..73577569 100644 --- a/internal/tmux/tmux_test.go +++ b/internal/tmux/tmux_test.go @@ -382,9 +382,6 @@ func TestCapturePaneNonexistentSession(t *testing.T) { func TestTargetHelpers(t *testing.T) { name := "my-session" - if got := exactTarget(name); got != "=my-session" { - t.Errorf("exactTarget(%q) = %q, want %q", name, got, "=my-session") - } if got := sessionTarget(name); got != "=my-session" { t.Errorf("sessionTarget(%q) = %q, want %q", name, got, "=my-session") }