fix(foreman): tell a spin from a real max-turns burn

[joryirving]

What

Give the foreman loop turn-accounting, and give a max-turns exhaustion that ran faster than a model physically can its own failure reason: LoopSpinning. The foreman.audit.v1 record now carries turnsWithCompletion / turnsWithToolCall alongside turnCount.

Why

Refs #1628. Two issue-fix tasks burned all 160 turns in 105s and 65s — 0.4–0.66 s/turn, faster than any model generates — and terminated INCOMPLETE with the same bare MaxTurnsExhausted that a genuine 1106s/1662s exhaustion reports. The audit record showed a turn count but no productivity signal, so a spin looked exactly like a model that gave up, and the operator's only lever (raise the budget / tighten the prompt) was the wrong one.

How

• Loop accounting (pkg/foreman/agent/loop.go): LoopResult gains TurnsWithCompletion (turns whose chat request yielded a choice — transport errors and empty-choices aborts don't count), TurnsWithToolCall, and TurnDuration (cumulative wall-clock inside runOneTurn). Each counter is incremented at a single point per turn, so the temperature-retry path can't double-count and TurnsWithCompletion <= Turns holds by construction.
• Spin guard (pkg/foreman/agent/executor_native.go): in mapLoopError, a max-turns exhaustion with Turns >= 10 and average turn duration strictly below 1s is classified LoopSpinning, with the observed rate in the summary; the [BUG] max-turns INCOMPLETE reports "model did not call submit_result" when it called it and was accepted #1713 rejected-submissions detail is kept when present. The guard lives in the executor, not the loop: the loop still returns the ErrMaxTurnsExhausted sentinel and knows nothing about CRD reasons.
• New API enum value LoopSpinning (api/foreman/v1alpha1/agentictask_types.go); CRDs regenerated (config/crd/bases, charts/foreman/templates/crds). Distinct fix path: check the endpoint/shim, not the prompt/budget. Runbook updated.
• Thresholds: 10 turns floor / 1 s average cap, derived from the observed spins (0.41–0.66 s/turn) vs healthy runs (~1.7 s+/turn); the floor keeps tiny fast runs on the ordinary reason.
• Audit (pkg/foreman/audit/record.go): the two new counters flow through resultPayload.Extra into the record; absent (pre-fix) result JSON leaves them zero and omitempty drops them, so old runs are never rewritten with fabricated values.

Deliberately out of scope: the no-tool-call / reasoning-only / truncated streak errors already have bounded streak budgets (their own circuit breakers), so only the MaxTurnsExhausted path — the unbounded one — gets the spin classification.

Checklist

• Tests added/updated (TestLoop_TurnAccounting, TestMapLoopError_MaxTurns_SpinGuard incl. floor/threshold boundaries, audit record presence + absence cases)
• make test passes locally
• make lint passes locally (also make lint-all across GOOS)
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance disclosed below, per CONTRIBUTING.md
• Documentation updated (docs/site/foreman/runbook-m3.md)

Assisted-by: OpenCode (qwen3.8-flash-next coordinator; loop counter placement reviewed against an independent local Gemma review pass); verified by running make test, make lint, make lint-all, make generate/manifests/chart CRD sync, and the new table-driven tests. Note: commit is DCO sign-off only — my machine's 1Password GPG signer was unavailable in the agent shell, so the commit is not GPG-signed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Defilan]

Approving, the spin trace holds up end to end. I re-derived the threshold arithmetic (1s times Turns is Turns seconds, not Turns nanoseconds) and ran the real 160-turn/65s and 1662s cases through mapLoopError; they classify the way the test says. Both new tests bite: I removed the two counter increments in loop.go and the guard condition in executor_native.go, and the respective tests failed with the counters at zero and the reason reverting to MaxTurnsExhausted. The guard's edges hold too; I probed 9 turns at a zero duration, a zero-value LoopResult, and both sides of the 1s/turn boundary, and only the sub-threshold side classifies. The audit wiring is consistent: incompleteResult writes turnsWithCompletion / turnsWithToolCall and resultPayload reads the same keys, and a pre-fix result JSON stays zero rather than fabricated. CRDs sync and CI is green.

One thing before merge: the body says Fixes #1628, but the issue's first ask is that a turn which neither completes nor calls a tool must not consume the budget silently, either by excluding it from the count or aborting with a distinct reason. This reports the ratio after the fact and does not stop the burn, so merging would close an issue that still has an open ask. Refs #1628 plus a follow-up, or fold that in.

Thanks for the boundary coverage, it made this quick to check.