fix(daemon): do not retry workers that finished with a final exit code

[PierrunoYT]

Summary

Pool.Run stopped retrying only on exit 0 and ExitPermanent (76) and treated everything else, except 75 (tempfail), as a crash to retry up to MaxAttempts (5) with backoff. zero exec also exits 2 (usage), 3 (provider), 4 (incomplete) and 130 (interrupted) (internal/cli/exec.go), all of which mean the run is over. Exit 4 in particular means the agent already ran (edits, shell commands, pushes), so every retry repeated those side effects, re-billed the provider, and appended another run to the same session buffer.

This change treats 2, 3, 4 and 130 as final: Run returns the worker's exit code with an error wrapping ErrPermanent and does not relaunch. Crash-type exits (1, signals) and launch/read errors still retry as before. The codes are duplicated in internal/daemon/pool.go because the daemon cannot import the CLI package; a comment says to keep them in sync.

Not included: the issue's "better" option of never retrying once a worker has emitted output. That also changes how a crash after partial output behaves, so I kept this PR to the exit-code mapping.

Linked issue

Fixes #1100

Note: #1100 does not currently carry the issue-approved label. Opening this anyway at the author's request; it can be held until the issue is approved.

Verification

• New TestPoolRunFinalExitCodesDoNotRetry uses a fake launcher for exit codes 2, 3, 4 and 130, with a second worker queued that would succeed. It asserts one launch, the worker's exit code, ErrPermanent, and that only the first run's output reached the sink. On the old code all four subtests fail (Run err = <nil>, want ErrPermanent, because the retry succeeded); on the new code they pass.
• Existing retry tests (crash exit 1 retries, cap exhaustion, tempfail, permanent) still pass.
• go build ./..., go vet ./internal/daemon/, gofmt -l internal/daemon, git diff HEAD --check clean; go test ./internal/daemon/... passes.
• go test ./internal/cli/ has failures on this Windows machine (for example TestRunExecLogsMCPRuntimeCloseError, TestRunProvidersUseEnvDerivedJSONIncludesConfigPath, and TestRunChangesBareRemotePushThenPRUsable with git init failing on NUL). I re-ran three of them with this change stashed and they fail the same way, so they are not caused by it.
• Not run locally: go test -race (cgo unavailable here), full go test ./..., smoke, make targets. Relying on CI.

Checklist

• The linked issue already has the issue-approved label. (not yet)
• go build ./..., go vet ./..., and go test ./... pass locally. (build passes; vet and tests only for the daemon packages)
• gofmt clean.
• Tests added/updated for the change (and run under -race where relevant). (test added; -race not run locally)
• UI changes include screenshots or a short recording where possible. (no UI change)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Worker runs that exit with usage errors, provider failures, incomplete runs, or interruptions are now treated as final. The reported exit code and initial output are preserved, and the run is not retried.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Required repository-wide validation and the issue-approval gate remain outstanding.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents daemon retries for final zero exec outcomes, avoiding repeated side effects and provider charges.

Changes:

• Classifies exit codes 2, 3, 4, and 130 as final.
• Adds regression coverage confirming no retry or duplicated output.

File	Description
internal/​daemon/​pool.go	Handles final worker exit codes without retrying.
internal/​daemon/​pool_test.go	Tests final-code handling and output behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: Twigpine/zero/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 261bd557-009c-4838-9158-25b422bd59c5

📥 Commits

Reviewing files that changed from the base of the PR and between 99721c7 and 403a59a.

📒 Files selected for processing (2)

• internal/daemon/pool.go
• internal/daemon/pool_test.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

---

Walkthrough

Pool.Run now treats worker exits 2, 3, 4, and 130 as permanent. Tests verify that these exits do not cause retries and that the original exit code and first-run output are preserved.

Changes

Pool exit handling

Layer / File(s)	Summary
Classify and handle final exits internal/daemon/pool.go, internal/daemon/pool_test.go	Pool.Run classifies exits 2, 3, 4, and 130 as non-retriable, logs the exit, and returns its code with an error wrapping ErrPermanent. The updated documentation describes these cases. Tests verify one launch, the returned code, and output from the first run.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 403a5

The pool now stops retrying the specified completed worker outcomes while preserving their exit codes; no actionable merge risk is evident.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 403a5

The change prevents repeated execution after explicitly terminal worker outcomes. The inspected completion path preserves output and cleanup without expanding access or worker privileges. No introduced or worsened security finding was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed policy applies to worker runs supervised by this daemon pool and their session outputs. The inspected change does not add callers, broaden resource selection, or grant additional worker authority; broader tenant and deployment exposure was not established.

Security Findings and Attack Paths

• observed — Crash-like exits and stdout read errors still enter the existing retry path, potentially after output has been forwarded. This residual replay behavior predates the PR and is not an introduced or worsened attack path.

Trust Boundaries and Controls

• observed — The new branch consumes a completed worker's exit status and selects a terminal error result. It does not change session identity, authorization, launcher selection, or credential handling in the inspected path.

Resilience and Maintainability Implications

• observed — Terminal classification preserves existing ownership cleanup: worker tracking is removed, the pool slot is released, and session completion closes subscribers and Done. The inspected return path does not leave these resources waiting for another retry.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: daemon workers with final exit codes are not retried.
Linked Issues check	✅ Passed	The PR meets the coding requirements in [#1100]. Pool.Run classifies exit codes 2, 3, 4, and 130 as final, returns the worker code with ErrPermanent, and does not relaunch the worker. The reported…
Out of Scope Changes check	✅ Passed	The changes are limited to daemon pool exit-code classification, related Pool.Run documentation, and focused tests. These changes directly support [#1100]. No unrelated implementation or test change…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}