fix(workspace): a failed turn says so, and Retry follows the billing evidence (#2320)

[obasilakis]

Summary

A Workspace turn that fails before or at start persists no assistant message and clears its in-flight marker on every exit path. The client learns an outcome exactly two ways — a new assistant row, or the marker still being set — so a fast failure produced neither, and after the 6s idle give-up it rendered the #2133 "we've lost track of this turn — it may still finish" copy for a turn the backend had diagnosed precisely and written to schedule_executions.error. Every clause of that message was false, and Retry was suppressed on the one path where re-sending is safe.

Two things turned out to be worse than the issue describes:

• The idle flag that distinguishes "the server says nothing is running" from "we ran out of budget" was returned and never read.
• deliver()'s retryable if (!data) branch — which PR fix(workspace): bound the reply poll to a turn's real life, and stop offering Retry when it is merely lost (#2133) #2150's body describes as preserved ("a genuine no-answer keeps its existing message and its Retry") — is unreachable. awaitPersistedReply has three returns and null is not one, so if (data?.lost) swallowed the idle case. That is why Retry was suppressed on the path fix(workspace): bound the reply poll to a turn's real life, and stop offering Retry when it is merely lost (#2133) #2150 believed was retryable.
• reattach() was the silent surface: it checked only for a reply, so a failed or lost turn rendered nothing at all — no message, no Retry, the spinner just stopped. Refreshing mid-turn showed less than staying put. Not mentioned in the issue.

Changes

The two bits live on the exception, decided at the raise site. ClientPortalError(status, detail, *, category, retryable). Not inferred downstream: _fail_unstarted_execution is reached from the pre-start branch and the generic except Exception, which can fire after execute_task already returned — so "was this billed" is not a property of the row being written. retryable defaults False.

Raise site	category	retryable
roster miss / stopped / containerless	agent_unavailable	✗ unbilled, but ent#286 settled that retrying cannot work
ResumeLockBusy	busy	✓ never reached the agent
CAPACITY	capacity	✓ admission refused; the queue drains
AUTH / BILLING	auth	✗ retry re-fails
TIMEOUT	timeout	✗ ran to the bound
generic turn failure	agent_error	✗ ran
uncaught crash	internal	✗ fixed sentence; raw text stays operator-only

Classification reads TaskExecutionResult.error_code instead of substring-matching the human-readable error, keeping the substring tests as the None-code fallback (additive — nothing that classified before stops classifying). AUTH/BILLING had no branch at all and fell through to the generic 502: that is the subscription-limit case this issue was reported from. _error_code_name reads .name rather than comparing members, because TaskExecutionErrorCode is @dataclass-decorated and AUTH == TIMEOUT is True (verified) — the #1085 footgun.

The verdict rides Redis beside the marker it is the terminal half of — portal_turn_outcome:{session_id}, TTL 900s, written in _run's except branches before the finally clears the marker. That ordering is the contract: the client's give-up timer starts when the marker vanishes, so an outcome written after it races a 6s window. Cleared at dispatch (turn N+1 never inherits turn N's verdict) and on success. Redis down ⇒ no outcome ⇒ the pre-#2320 message, never worse. Surfaced as PortalHistory.last_turn_outcome, declared on the model — the route's response_model strips undeclared keys, so a service-layer-only change would be a no-op.

No schema change. An error message row in enterprise_portal_messages was the obvious shape and is poison: _format_history_context replays any non-user role to the agent as its own words, and _persist_user_turn's dedupe reads recent[-1].role == "user", so Retry would duplicate the user message — breaking a #2120 pin that has a test.

Client believes a verdict only for the turn it is awaiting (outcome.execution_id === executionId), words the two give-ups distinctly, and offers Retry iff the verdict says nothing reached the agent. markLastUserTurnFailed marks only the thread's unanswered tail — two raise sites record a verdict without persisting a user row, and a backwards walk would pin the failure onto an earlier, answered turn.

Acceptance criteria

• A turn that fails before/at start renders its failure instead of "lost track"
• Raw schedule_executions.error never reaches a portal client verbatim (fixed sentence for the uncategorised path; raw text to log + execution row only)
• Retry appears only for failures recorded as never-started/unbilled; lost/idle keep suppressing it
• idle vs budget-exhausted worded distinctly
• Regression test naming this issue covers the fast-fail path

Test Plan

• cd tests && python3 -m pytest unit/test_2320_portal_failed_turn_visibility.py -q → 42 passed
• cd src/frontend && npx vitest run tests/unit/portalFailedTurn.spec.js → 25 passed
• Portal suites (ent286, 2133, ent358, 2196, 2320) → 137 passed, no regressions
• Full frontend suite → 53 files, 1112 passed
• Mutation-verified: 10 backend + 11 frontend mutations, each caught; restores byte-identical
• Reproduced and fixed on a live instance. Held the resume lock, dispatched a Workspace turn: before, in_flight=None + no reply + lost-track copy. After:

{"execution_id":"qT4FcFCfiwgn0asbQQC0Nw","category":"busy",
 "message":"This conversation is already handling a message. Please try again shortly.",
 "retryable":true}

Next turn: verdict cleared at dispatch, assistant "FIXED", outcome stays null.

Note: the frontend spec is the first to exercise PortalConversation.vue, and there was no executable coverage of the no-Retry rule anywhere before this — only a docstring at test_2133_bounded_reply_poll.py:15. There is no component-mount harness in the project (@vue/test-utils is not a dependency and vitest runs environment: 'node'), so rather than re-implement the decision expressions, the spec extracts the shipped ones and runs them.

Two entries added to docs/memory/learnings.md: the dead-caller-branch class, and applying a verdict to "the last matching row" instead of the row actually waiting.

Fixes #2320

🤖 Generated with Claude Code

[vybe]

Validated. Fixes #2320 (closing keyword present). Named regression test test_2320_portal_failed_turn_visibility.py + frontend portalFailedTurn.spec.js; feature flow workspace-absorbs-session.md updated. Security scan clean (the two secret-shaped hits are obvious test fixtures). No new backend package, no new env var, no host paths or mode flips.

CI note for the record: pytest (base, seed 99999) shows red but is a runner timeout on the base side — junit-base-99999.xml never materialised. All three head seeds are fully green (12145 passed / 0 failed) and regression diff reports No new failures on the surviving seeds. Not a regression signal.

[vybe]

Validated after three rebases onto dev. Fixes #2320 (closing keyword present). All checks pass on a fully clean board this time — no degraded seed: regression diff across all six seeds (base 12195 → head 12237, 0 failures either side, No new failures), e2e green (6m55s).

Worth recording since it was red twice on the way here: the earlier pytest (base, seed …) failures were cancelled base-side jobs whose junit-base-*.xml never materialised — runner timeouts on dev, not this PR. The head side was green all three times. This run confirms it against a full six-seed board.

Substance. The PR fixes a real dead-branch: awaitPersistedReply has exactly three returns and never returns null, so deliver()'s if (!data) branch and its retryable throw were unreachable from the day #2150 wrote them — while if (data?.lost) sat above and swallowed the idle case into the pessimistic "we've lost track of this turn" copy. Every failed Workspace turn for months was reported as possibly-still-running with Retry suppressed, including turns the backend had diagnosed precisely and written to schedule_executions.error. The idle discriminator was computed, returned, and read by nobody.

Good sign for the fix's own quality: the second learnings entry records a defect found in /review of this PR — markLastUserTurnFailed walking backwards to the last user row, which for the two raise sites that fire before _persist_user_turn would have pinned the failure onto an earlier, already-answered turn and offered a Retry that re-sends it. Not live, but correct only by a property of the current call graph. Fixing that before merge rather than shipping on "unreachable today" is the right call, and the same family as the #1804 lookup-vs-write rule.

Tests: test_2320_portal_failed_turn_visibility.py (named regression, 931 lines) and portalFailedTurn.spec.js (443). workspace-absorbs-session.md flow updated.

Rebase note. Three rebases — after #2333, #2357 and #2356 in turn. The first two were pure learnings.md append conflicts. The third overlapped #2356 on three real source files (client_portal/service.py, PortalConversation.vue, stores/clientPortal.js); git auto-merged all three, and I verified rather than assumed — ent#428's ask code is intact in each, this PR's own diff is unchanged at 8 files / 1808 insertions, and learnings.md ordering breaks still match origin/dev's exactly with no duplicate headings.

Security scan clean (portal-token, SUPER-SECRET-INTERNAL-abc123 are test fixtures). No new env var, no new backend package, no host paths or mode flips.