dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: contain workflow reset failures (#3358) (#3637)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-14 09:28:11 +01:00
committed by GitHub
parent c86a16b784
commit f371804f5b
37 changed files with 3005 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/workflow-reset-failure-containment/plan.md
+283
View File
@@ -0,0 +1,283 @@
---
created: 2026-09-12
status: done
requirements:
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
system_design:
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
legacy_specs: []
---
# Implementation Plan: Workflow reset failure containment
## Overview
[Issue 3358](https://github.com/kdlbs/kandev/issues/3358) reports an escalated cancellation followed by a context reset that holds the session guard.
The residual cancellation defect is reproducible at `49df3794b`.
GitHub's current `main` also resolved to that commit during investigation.
The fix stops unsafe reset admission, bounds unanswered reset requests, and makes failed workflow entry visible.
The issue is assigned to `carlosflorencio`.
Tasks 01-04 are implemented and verified.
## Evidence and confidence
| Finding | Evidence | Assessment |
| --- | --- | --- |
| Escalation permits provider reset | Temporary `TestIssue3358ReproEscalationMustStopReset` calls the real orchestrator with an active durable turn and `ErrCancelEscalated`. It fails because reset returns success. | Confirmed residual defect |
| Ordinary cancellation failure blocks reset | Existing `TestResetAgentContext_CancelFailureStopsProviderReset` passes. | Control passes |
| Confirmed cancellation permits reset | Existing `TestResetAgentContext_QuiescesActiveTurnBeforeProviderReset` passes. | Control passes |
| Reset uses an unbounded response wait | `ResetSession -> createSessionRequest -> sendStreamRequest` supplies no deadline. The WebSocket wait ends only on reply, disconnect, or caller cancellation. | Confirmed source path |
| Caller cancellation releases the response wait | Temporary `TestIssue3358ReproResetWaitsForCallerCancellation` passes against a silent WebSocket peer. It receives the request, waits 100 ms, then cancels and observes release. | Transport characterization, not a hours-long hang reproduction |
| Startup ordering is already repaired | `Service.Start` starts watcher and scheduler before `startLifecycleSweepAsync`. Recovery no longer blocks readiness. | Source confirmed |
| Startup ordering is covered by the existing regression | `TestStartDoesNotBlockOnLifecycleSweep` passes with the current startup implementation. | Verified separately; no startup change was needed |
The escalation reproduction failed again when run alone.
Confidence is high for the residual reset correction.
The reporter's precise provider-side hang has no goroutine dump and was not reproduced with Claude ACP in this investigation.
The original report and all three comments were read.
The reported startup outage is already addressed by the current startup implementation, including the change in PR 2944.
The optional step re-entry behavior is not required to contain the reset defect.
Review remediation evidence is complete. The ACP adapter regression proves that
`session/new` returns before a blocked best-effort `session/close`, while a
delayed provider response after the manager deadline remains fenced and its
late setup event is discarded. The workflow barrier regression proves that a
successor admission and session deletion wait until reset-failure persistence
and publication finish. Backend and browser coverage also verifies a distinct
step prompt, prompt-call and user-message counts, and the persisted reset cause
in the expanded desktop and phone notices.
## Requirement conformance
The current implementation violates `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.1`: local reconciliation is not provider quiescence.
Retain `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4`, which forbids automatic prompting after reset failure.
The issue's suggestion to continue prompting after failure conflicts with that contract.
This package uses the issue's alternative of a visible error.
The owning [requirements](../../specs/tasks/requirements/workflow-step-agent-start-ownership.md) add criteria `.7`, `.8`, and `.9`.
They cover escalation, unanswered reset requests, and persistent recovery feedback.
The [system design](../../specs/tasks/system-design/workflow-step-agent-start-ownership.md#reset-failure-containment) records the implementation boundary.
The existing [quiescence ADR](../../decisions/2026-08-30-context-reset-quiesces-active-turn.md) remains applicable.
No new ADR is needed: the design records the local error-policy choice without creating a new architectural owner.
## Scope
### In scope
- Retain provider cancellation outcome separately from successful local reconciliation.
- Reject escalated cancellation only at the workflow reset admission boundary.
- Give the provider reset request a 10-second child deadline.
- Return reset timeout or caller cancellation without automatic restart fallback.
- Preserve ordinary unsupported-reset fallback and runtime configuration restoration.
- Persist reset failure through the existing error notice and preserve deletion access after cleanup.
### Out of scope
- Startup recovery scheduling changes or readiness changes.
- Repair of the unrelated startup test fixture.
- Automatic prompting after a failed reset.
- Automatic restart after escalated cancellation.
- Changing manual step re-entry eligibility for settled sessions.
- A new recovery button, API, runtime flag, or database migration.
- General cancellation of waits on shared mutexes or all provider RPCs.
- Retrospective repair of a reset goroutine already stuck in an older backend.
## Technical approach
### 1. Provider cancellation outcome
Change the coordinator in `task_operations.go` and `event_handlers_clarification.go`.
Capture the raw result before tolerated sentinels become reconciliation success.
Keep it on the operation that owns the captured execution and turn.
Read it through the exclusive reset helper after completion.
Do not change the common operation error for explicit cancellation or joined callers.
`quiesceActiveResetTurn` rejects escalation.
The existing guards, reset marker, silent reconciliation, and missing-execution cleanup remain authoritative.
### 2. Reset request timeout
Change `Manager.ResetAgentContext` in `manager_interaction.go`.
Use `context.WithTimeout` around `client.ResetSession`, with a 10-second internal constant.
Classify deadline expiry and caller cancellation before the ordinary fallback branch.
Release the client lease and return an error through normal deferred cleanup.
A timeout does not prove that the remote operation stopped.
Do not reuse the same stream for a speculative restart or treat a late reply as successful reset.
Preserve the current unsupported-reset fallback.
Test actual request dispatch before measuring timeout; independent mutex contention is outside this request-response bound.
### 3. Visible workflow failure
Add an error-bearing reset helper without forcing unrelated callers to change.
Use `persistLastAgentError` directly from workflow failure handling.
Do not route the failure through `handleAgentFailed`, which can dispatch workflow actions.
Persist safe reset-specific text, publish it, and reload session metadata before the waiting-state event.
Retain the automatic-prompt early return and omit the successful reset divider.
The existing desktop and phone error notice provides the presentation.
## ASCII UI preview
UI-01: Conversation after a workflow reset failure.
The desktop chat and phone conversation share this notice composition.
The phone uses its existing focused conversation and scroll owner.
Before:
```text
Destination step
Conversation history
[Composer]
```
After:
```text
Destination step
Conversation history
+--------------------------------------+
| Previous agent error [Hide] |
| Context reset failed. |
| The workflow step prompt did not |
| start. |
+--------------------------------------+
[Composer]
```
The hierarchy is required; wording is illustrative and must follow localization rules.
Reuse `LastAgentErrorNotice`, its wrapping, its alert semantics, and its existing dismissal control.
Increase only the coarse-pointer dismissal target from 32 pixels to at least 44 pixels.
No hover is needed to read the phone error.
Tests must prove visibility after reload and no horizontal document overflow.
## Tests
| Criteria | Test file and test |
| --- | --- |
| `.1`, `.2`, `.4`, `.7` | `event_handlers_workflow_reset_quiescence_test.go::TestResetAgentContext_EscalatedCancelStopsProviderReset` |
| `.2`, `.6`, `.7` | Same file: escalation with a late completion, a joined explicit caller, and missing execution cases |
| `.8` | `manager_interaction_reset_timeout_test.go::TestManager_ResetAgentContext_ResetRequestTimeout` and `TestManager_ResetAgentContext_CallerDeadlinePrecedesRequestTimeout` |
| `.3`, `.6`, `.8` | Same file and agentctl tests: caller cancellation, abandoned reply, terminal cleanup, normal fast reset, unsupported fallback |
| `.4`, `.9` | `event_handlers_workflow_reset_failure_test.go::TestProcessOnEnter_ResetFailurePersistsNotice` |
| `.4`, `.7`, `.9` | `internal/integration/workflow_context_reset_failure_test.go::TestWorkflowResetEscalationLeavesSessionDeletable` |
| `.3`, `.6`, `.8` | `adapter_session_test.go::TestResetSessionSerializesConcurrentLoadDuringClose`; `manager_interaction_reset_timeout_test.go::TestManager_ResetAgentContext_DelayedProviderSessionIsFenced` |
| `.4`, `.7`, `.9` | `event_handlers_workflow_reset_failure_test.go::TestProcessOnEnter_ResetFailureSettlementOwnsSuccessorAdmission` and `TestProcessOnEnter_SuccessfulResetDispatchesAutoStartPrompt` |
The integration test follows `cancel_after_crash_test.go`.
Drive workflow entry against an agent simulator that returns escalation.
Assert no provider reset, no automatic step prompt or user message, a persisted
cause, and successful `session.delete`. The unit fixture also covers provider
timeout and a successful reset that dispatches the distinct step prompt.
This is backend end-to-end evidence; it does not substitute for notice rendering.
## E2E tests
- Desktop: extend `apps/web/e2e/tests/session/agent-error-indicator.spec.ts` with `workflow reset failure remains visible after reload`.
- Phone: add `apps/web/e2e/tests/session/mobile-workflow-reset-error.spec.ts` for the same notice, reload, dismissal, and viewport containment.
- Reuse the existing error-metadata fixture. Backend integration proves production emission; seeded rendering tests prove the existing UI contract.
- Map both scenarios to `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.9`.
- Projects: `chromium` and `mobile-chrome`.
## Companion packages
The quiescence, idle-dispatch-gate, and runtime-configuration packages are already implemented.
Their historical test results remain historical evidence, not results of this package.
Their completed work orders do not need reopening.
The new work orders retain targeted compatibility checks for their contracts.
- [Workflow context reset quiescence](../workflow-context-reset-quiescence/plan.md)
- [Idle reset dispatch gate](../idle-context-reset-dispatch-gate/plan.md)
- [Reset runtime configuration](../acp-context-reset-runtime-configuration/plan.md)
## Work orders
Execute sequentially in this session after an explicit implementation request.
- [x] [Task 01: Preserve reset cancellation outcome](task-01-reset-cancellation-outcome.md)
- [x] [Task 02: Bound provider reset requests](task-02-provider-reset-timeout.md)
- [x] [Task 03: Surface workflow reset failures](task-03-workflow-reset-feedback.md)
- [x] [Task 04: Verify reset recovery surfaces](task-04-reset-recovery-surfaces.md)
## Verification results
Investigation commands:
```bash
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestIssue3358ReproEscalationMustStopReset$' -count=1 -v)
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/agentctl -run '^TestIssue3358ReproResetWaitsForCallerCancellation$' -count=1 -v)
```
The first command failed at the expected reset-admission assertion.
The second command passed.
Both diagnostic files are temporary and are removed before handoff.
The first combined diagnostic run also passed both control tests. The existing
startup regression was rerun after implementation and passed.
Permanent implementation verification is recorded in the completed work
orders. The final checks include backend package tests and build, frontend
unit tests, typecheck, desktop and phone E2E, i18n validation, public-docs
validation, specification validation, formatting, and diff checks.
Backend verification passed:
```text
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle ./internal/agent/runtime/agentctl ./internal/orchestrator ./internal/integration)
(cd apps/backend && make lint)
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle ./internal/agent/runtime/agentctl ./internal/agentctl/server/adapter/transport/acp ./internal/orchestrator ./internal/integration -count=1)
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle ./internal/agentctl/server/adapter/transport/acp -run 'TestManager_ResetAgentContext_|TestResetSession' -count=1)
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run 'TestProcessOnEnter_(ResetFailure|SuccessfulReset)' -count=1)
(cd apps/backend && go test -tags fts5 ./internal/integration -run 'TestWorkflowResetEscalationLeavesSessionDeletable' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/lifecycle -run 'TestManager_ResetAgentContext_|TestManager_RestartAgentProcess_|TestWaitForPendingDispatchedPrompt_' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/agentctl -run 'TestResetSession_|TestSendStreamRequest' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestProcessOnEnterResetAgentContext|TestProcessOnEnter_ResetFailure|TestResetAgentContext_' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/integration -run 'TestWorkflowResetEscalationLeavesSessionDeletable|TestOrchestratorCancelAfterAgentCrash_UnstiksSession|TestOrchestratorCancelWhenAgentHangs_UnsticksSession' -count=1)
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestStartDoesNotBlockOnLifecycleSweep$' -count=1 -v)
make -C apps/backend build
```
Frontend and documentation verification passed:
```text
(cd apps/web && pnpm exec vitest run components/task/chat/message-list-shared.test.tsx)
(cd apps/web && pnpm run typecheck)
(cd apps && pnpm --filter @kandev/web build:vite)
(cd apps/web && pnpm run i18n:check)
(cd apps/web && pnpm e2e:run --project chromium tests/session/agent-error-indicator.spec.ts)
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/session/mobile-workflow-reset-error.spec.ts)
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
python3 scripts/list-docs.py validate
python3 scripts/lint-spec-files.test.py
python3 scripts/lint-spec-files.py --all
git diff --check
```
The focused ACP, lifecycle, orchestrator, and integration suites passed after
the review fixes. Desktop and phone E2E both passed with the technical-details
disclosure expanded; the phone test measured zero document overflow while the
details were visible and a 44-pixel dismiss target. Backend lint, typecheck,
Vite build, i18n checks, documentation validation, and specification lint also
passed.
Documentation validation passed:
- `python3 scripts/list-docs.py validate`: 264 decisions and 818 specifications validated.
- `python3 scripts/lint-spec-files.test.py`: 36 tests passed.
- `python3 scripts/lint-spec-files.py --all`: all specification files passed.
- `git diff --check`: passed for tracked changes.
- Package reference and whitespace checks: passed for all five plan files, including untracked work orders.
Production code, permanent tests, frontend E2E coverage, and public guidance
changed. No PR was opened.
## Risks
- Changing the shared cancellation error can break explicit or joined cancellation. Preserve a separate provider outcome.
- An expired request does not cancel the remote provider operation. Block automatic prompting and ignore abandoned responses.
- Ordinary unsupported-reset fallback retains its current initialization behavior. This package does not bound every restart phase.
- A deadline cannot cancel mutex acquisition. The request-response guarantee starts after the peer receives the request.
- Stale waiting-state metadata can erase a newly published error. Reload after persistence.
- Startup recovery remains outside this change; the existing startup regression passes without a production startup modification.
docs/plans/workflow-reset-failure-containment/task-01-reset-cancellation-outcome.md
+107
View File
@@ -0,0 +1,107 @@
---
id: "01-reset-cancellation-outcome"
title: "Preserve reset cancellation outcome"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
acceptance_criteria:
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.1
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.2
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.7
system_design:
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
---
# Task 01: Preserve reset cancellation outcome
## Summary
Workflow reset must distinguish provider stop confirmation from local cancellation reconciliation.
Preserve the existing semantics for other cancellation callers.
## In scope
- Capture the raw lifecycle cancellation result on its owning coordinator operation.
- Expose a synchronized provider outcome to the exclusive reset path after local reconciliation completes.
- Reject escalation in `quiesceActiveResetTurn` without calling provider reset or restart.
- Preserve missing-execution cleanup and explicit, joined, peer, clarification, and queue behavior.
- Keep cancellation projection and turn identity cleanup correct on every exit.
## Out of scope
Provider request deadlines, visible failure reporting, startup recovery, and process replacement.
## Acceptance
1. The named regression fails before the correction: reset currently returns success after `ErrCancelEscalated`. After correction, it returns failure with no reset call.
2. Local reconciliation still settles the captured turn. Explicit and joined callers retain success, visible cancellation, and configured completion semantics.
3. Reset cleanup releases the marker and guard. A delayed old completion cannot finish a successor generation.
## Verification
Run from the repository root:
```bash
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestResetAgentContext_EscalatedCancelStopsProviderReset$' -count=1 -v)
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestResetAgentContext_|TestHasActiveResetTurn_|TestCancelAgent_|TestCancelAgentSilent_|TestCancellationSourcesShareLifecycleOwnership|TestUserCancelCompletion_|TestQueueAndInterruptForPeerMessage_' -count=1)
git diff --check
```
The first invocation supplies the required RED result before production edits.
Use the existing `newActiveResetTestService` fixture and `orderedResetAgentManager`.
Include direct and wrapped escalation, ordinary failure, confirmed stop, missing execution, and a joined explicit caller.
Use deterministic channels for concurrent completion tests.
## Files likely touched
- `apps/backend/internal/orchestrator/task_operations.go`
- `apps/backend/internal/orchestrator/event_handlers_clarification.go`
- `apps/backend/internal/orchestrator/event_handlers_workflow.go`
- `apps/backend/internal/orchestrator/event_handlers_workflow_reset_quiescence_test.go`
- `apps/backend/internal/orchestrator/task_operations_test.go`
## Dependencies
None.
## Risks
Do not set the common `cancelOperation.err` to escalation after successful reconciliation.
That error controls joined callers and registered actions.
Do not make every `cancellationKindInternal` caller require strict reset semantics.
## Parallelism
`sequential`
## Inputs
- [System design](../../specs/tasks/system-design/workflow-step-agent-start-ownership.md#preserve-the-provider-cancellation-outcome).
- [Plan evidence](plan.md#evidence-and-confidence).
- Existing cancellation identity, projection, and exclusive-claim tests.
## Results
Implemented the cancellation outcome boundary. The cancellation coordinator now
retains the raw provider cancellation result, while explicit and joined
cancellation callers keep their existing reconciliation result. Workflow reset
reads that outcome after the owned cancellation completes and fails closed on
direct or wrapped `lifecycle.ErrCancelEscalated` without calling provider reset.
Missing execution and ordinary cancellation failure behavior remain covered.
The workflow regression now configures reset and automatic step start with a
distinct prompt. Escalation records no prompt call or user message, while the
successful-reset control records one dispatch of that prompt.
Verification passed:
```text
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestResetAgentContext_EscalatedCancelStopsProviderReset$' -count=1 -v)
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestResetAgentContext_|TestHasActiveResetTurn_|TestCancelAgent_|TestCancelAgentSilent_|TestCancellationSourcesShareLifecycleOwnership|TestUserCancelCompletion_|TestQueueAndInterruptForPeerMessage_' -count=1)
git diff --check
```
docs/plans/workflow-reset-failure-containment/task-02-provider-reset-timeout.md
+117
View File
@@ -0,0 +1,117 @@
---
id: "02-provider-reset-timeout"
title: "Bound provider reset requests"
status: done
wave: 2
depends_on:
- "01-reset-cancellation-outcome"
plan: "plan.md"
requirements:
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
acceptance_criteria:
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.3
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.8
system_design:
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
---
# Task 02: Bound provider reset requests
## Summary
Give the provider context-reset request a 10-second deadline.
Return timeout and caller cancellation without entering the ordinary restart fallback.
## In scope
- Add a named internal reset-request timeout and a child context in `Manager.ResetAgentContext`.
- Cover the request write and response wait with that context.
- Release the child context and client lease on every return.
- Preserve ordinary unsupported-reset fallback and its captured runtime configuration.
- Prove abandoned replies cannot publish reset success or alter successor completion ownership.
## Out of scope
General WebSocket timeout policy, mutex cancellation, new configuration, startup scheduling, and automatic restart after escalation.
## Acceptance
1. A silent peer receives the reset request, then the manager returns a deadline error within 10 seconds. A shorter caller deadline wins.
2. Timeout or caller cancellation never calls restart and never publishes reset-success or boot-ready events. Pending request registration is removed.
3. Fast reset and ordinary unsupported-reset fallback retain configuration restoration and idle dispatch-gate behavior.
## Verification
Run from the repository root:
```bash
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle -run '^TestManager_ResetAgentContext_ResetRequestTimeout$' -count=1 -v)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/lifecycle -run 'TestManager_ResetAgentContext_|TestManager_RestartAgentProcess_|TestWaitForPendingDispatchedPrompt_' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/agentctl -run 'TestResetSession_|TestSendStreamRequest' -count=1)
git diff --check
```
The RED regression must observe an actual unanswered request, not a missing test hook.
Use an outer cancellation solely to release the pre-fix test after its assertion fails.
Use fake time where the existing transport fixture permits it.
Do not mutate a global timeout concurrently.
Cover the default deadline, shorter parent deadline, disconnect, late response, and unsupported reset.
A timeout while waiting on another writer is separate from this unanswered-response test.
## Files likely touched
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction_reset_timeout_test.go` (new)
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction_runtime_config_reset_test.go`
- `apps/backend/internal/agent/runtime/agentctl/agent_session_test.go`
Inspect `client_stream.go` for pending-request cleanup; change it only if the deadline regression proves a defect there.
## Dependencies
Task 01.
## Risks
The remote operation can outlive the local deadline.
Do not treat timeout as unsupported reset or use a detached goroutine to bypass cleanup.
Keep RPC correlation cleanup distinct from provider event generation checks.
## Parallelism
`sequential`
## Inputs
- [System design](../../specs/tasks/system-design/workflow-step-agent-start-ownership.md#bound-the-provider-reset-request).
- `newRestartMockAgentctlServer` and existing runtime-configuration reset tests.
- `sendStreamRequest` and `newTestClientWithStream`.
## Results
Implemented the bounded provider reset request and verified that an unanswered
reset returns its deadline error without restart fallback or success events.
Pending request correlation is removed before a late response arrives.
The caller-deadline regression also returns at 100 ms, before the internal
10-second request bound. The timeout fixture verifies that terminal session
stop can acquire the reset lock and delete the execution after cleanup.
The ACP adapter regression uses a real adapter transport with a fast
`session/new` and a blocked superseded-session close. `ResetSession` returns
after the new session commits, while a concurrent load waits for serialized
cleanup. The lifecycle delayed-response regression proves that a provider
session created after the caller deadline remains fenced, and its late setup
event cannot repopulate the discarded session state.
Verification passed:
```text
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle -run '^TestManager_ResetAgentContext_ResetRequestTimeout$' -count=1 -v)
(cd apps/backend && go test -tags fts5 ./internal/agent/runtime/lifecycle -run 'TestManager_ResetAgentContext_(ResetRequestTimeout|CallerDeadlinePrecedesRequestTimeout)' -count=1 -v)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/lifecycle -run 'TestManager_ResetAgentContext_|TestManager_RestartAgentProcess_|TestWaitForPendingDispatchedPrompt_' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/agent/runtime/agentctl -run 'TestResetSession_|TestSendStreamRequest' -count=1)
git diff --check
```
docs/plans/workflow-reset-failure-containment/task-03-workflow-reset-feedback.md
+118
View File
@@ -0,0 +1,118 @@
---
id: "03-workflow-reset-feedback"
title: "Surface workflow reset failures"
status: done
wave: 3
depends_on:
- "01-reset-cancellation-outcome"
- "02-provider-reset-timeout"
plan: "plan.md"
requirements:
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
acceptance_criteria:
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.7
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.8
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.9
system_design:
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
---
# Task 03: Surface workflow reset failures
## Summary
Preserve the reset error through workflow entry.
Persist it through the existing last-error metadata and event path without dispatching another workflow action.
## In scope
- Add an error-bearing reset helper and retain a boolean wrapper where it avoids unrelated caller changes.
- Report failed quiescence, timeout, provider reset, runtime restoration, and reset persistence.
- Use `persistLastAgentError` with sanitized reset-specific text and the captured execution identity.
- Persist the notice before publishing fresh waiting-state metadata.
- Use a bounded cleanup context after request expiry.
- Preserve the early return before automatic prompting and omit a successful reset divider.
- Prove session deletion through the real WebSocket handler after failure cleanup.
## Out of scope
New error UI, general agent-failure dispatch, automatic recovery, step re-entry semantics, and startup recovery.
## Acceptance
1. `TestProcessOnEnter_ResetFailurePersistsNotice` fails before the correction because reset failure produces no durable notice. It passes after persistence and publication.
2. Failed reset does not send a step prompt, show reset success, or evaluate `on_agent_error` or user cancellation completion. The error survives a fresh session read.
3. `TestWorkflowResetEscalationLeavesSessionDeletable` proves workflow entry, visible-error metadata, and successful `session.delete` within the client request timeout.
## Verification
Run from the repository root:
```bash
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestProcessOnEnter_ResetFailurePersistsNotice$' -count=1 -v)
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestProcessOnEnterResetAgentContext|TestProcessOnEnter_ResetFailure|TestResetAgentContext_' -count=1)
(cd apps/backend && go test -race -tags fts5 ./internal/integration -run 'TestWorkflowResetEscalationLeavesSessionDeletable|TestOrchestratorCancelAfterAgentCrash_UnsticksSession|TestOrchestratorCancelWhenAgentHangs_UnsticksSession' -count=1)
git diff --check
```
Use table-driven failure cases and assert persisted metadata plus the published last-error event.
Cover request cancellation during error persistence, a stale pre-reset session snapshot, and persistence failure logging.
Use the simulator's existing hang-on-cancel behavior.
Extend it only with the reset-call counters and workflow setup needed by this regression.
Assert that both reset and restart remain absent after escalation.
Add a bounded concurrent deletion test to the timeout fixture to prove guard release after the provider response deadline.
## Files likely touched
- `apps/backend/internal/orchestrator/event_handlers_workflow.go`
- `apps/backend/internal/orchestrator/event_handlers_workflow_reset_failure_test.go` (new)
- `apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go`
- `apps/backend/internal/orchestrator/event_handlers_agent.go` (only if a reusable error-persistence seam is needed)
- `apps/backend/internal/integration/workflow_context_reset_failure_test.go` (new)
- `apps/backend/internal/integration/simulated_agent_manager_test.go`
## Dependencies
Tasks 01 and 02.
## Risks
The general failure handler can trigger workflow transitions and execution cleanup.
Call the persistence helper directly.
Never publish the old session metadata after the new notice.
A database outage can prevent durable feedback; preserve failure logs and release locks.
## Parallelism
`sequential`
## Inputs
- [System design](../../specs/tasks/system-design/workflow-step-agent-start-ownership.md#persist-a-visible-failure).
- Existing `persistLastAgentError` and `publishSessionWaitingEvent`.
- `internal/integration/cancel_after_crash_test.go`.
- [UI-01](plan.md#ascii-ui-preview), verified by Task 04.
## Results
Implemented workflow reset failure containment. Workflow entry now persists a
sanitized last-agent-error notice for quiescence, provider reset, runtime
restoration, and persistence failures. It reloads session metadata before the
waiting-state event, skips automatic prompting and reset-success history, and
uses a bounded detached cleanup context after request cancellation.
Verification passed:
```text
(cd apps/backend && go test -tags fts5 ./internal/orchestrator -run '^TestProcessOnEnter_ResetFailure' -count=1 -v)
(cd apps/backend && go test -tags fts5 ./internal/integration -run '^TestWorkflowResetEscalationLeavesSessionDeletable$' -count=1 -v)
```
The integration test uses the real WebSocket session handler and confirms that
an escalated reset leaves the session deletable without reset, restart, or an
automatic workflow prompt. It also verifies the distinct prompt-call count,
user-message count, and sanitized persisted cancellation cause. The deterministic
unit barrier holds successor admission and deletion until failure persistence
and waiting-state publication finish, so neither can be overwritten by a stale
reset settlement.
docs/plans/workflow-reset-failure-containment/task-04-reset-recovery-surfaces.md
+150
View File
@@ -0,0 +1,150 @@
---
id: "04-reset-recovery-surfaces"
title: "Verify reset recovery surfaces"
status: done
wave: 4
depends_on:
- "03-workflow-reset-feedback"
plan: "plan.md"
requirements:
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
acceptance_criteria:
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.9
system_design:
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
---
# Task 04: Verify reset recovery surfaces
## Summary
Prove that the existing error notice shows reset failure on desktop and phone.
Document recovery after the implementation exists.
## In scope
- Add desktop reload coverage to the existing error-indicator spec.
- Add a phone spec for the same reset notice, reload, dismissal, and viewport containment.
- Reuse the existing metadata seeding pattern after Task 03 proves production emission.
- Preserve the current notice layout and localized heading.
- Make its dismiss target at least 44 pixels on coarse pointers. Keep desktop dimensions unchanged.
- Update the reset paragraph in the existing public workflow how-to guide.
## Out of scope
A new recovery control, new navigation, automatic restart, broad UI redesign, and changing desktop control size.
## Acceptance
1. Desktop and phone show the reset cause and blocked-step explanation after reload. Phone text wraps without document overflow.
2. Phone dismissal has a target of at least 44 pixels and needs no hover. Dismissal remains effective after reload.
3. Public docs explain that failed reset blocks the step prompt and leaves the error visible. Recovery guidance uses existing session deletion and creation.
## ASCII UI preview
UI-01: Existing conversation error notice after failed reset.
See [combined preview](plan.md#ascii-ui-preview).
```text
Conversation history
+--------------------------------------+
| Previous agent error [Hide] |
| Context reset failed. |
| The workflow step prompt did not |
| start. |
+--------------------------------------+
[Composer]
```
The same notice appears in the desktop chat and focused phone conversation.
The phone's existing conversation owns scrolling.
Retain the notice's bounded internal error-text scroll for long details.
Keep its alert role and localized dismissal label.
The existing `LastAgentErrorNotice` is the nearest shipped content exemplar.
Use the mobile control-sizing rule for its currently 32-pixel dismiss target.
The example wording is illustrative.
No new frontend literal bypasses localization.
Any new frontend key requires all five languages and the Traditional Chinese generator.
## Verification
Run from the repository root:
```bash
(cd apps && pnpm install --frozen-lockfile)
(cd apps/web && pnpm exec vitest run components/task/chat/message-list-shared.test.tsx)
(cd apps/web && pnpm e2e:run --project chromium tests/session/agent-error-indicator.spec.ts)
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/session/mobile-workflow-reset-error.spec.ts)
(cd apps/web && pnpm exec eslint components/task/chat/message-list-shared.tsx)
(cd apps/web && pnpm run i18n:check)
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
python3 scripts/list-docs.py validate
python3 scripts/lint-spec-files.test.py
python3 scripts/lint-spec-files.py --all
git diff --check
```
Run the desktop and phone E2E commands sequentially with managed production builds.
The backend unit and integration RED gates prove the missing error emission.
Seeded E2E proves rendering and persistence; do not describe it as an end-to-end provider-hang reproduction.
For the touch target, record a geometry RED assertion before changing the control.
Assert the active notice rather than selecting a hidden desktop mount.
Use the existing dismissal API and restore any changed worker-scoped fixture state.
## Files likely touched
- `apps/web/e2e/tests/session/agent-error-indicator.spec.ts`
- `apps/web/e2e/tests/session/mobile-workflow-reset-error.spec.ts` (new)
- `apps/web/e2e/tests/session/workflow-reset-error-helpers.ts` (new, only for shared setup)
- `apps/web/components/task/chat/message-list-shared.tsx`
- `docs/public/tasks-and-workflows.md`
- `docs/plans/workflow-reset-failure-containment/plan.md` and work-order results
## Dependencies
Task 03.
## Risks
A seeded notice does not prove backend workflow emission.
Keep the Task 03 integration evidence with the rendering results.
Do not copy the legacy test's hidden-sidebar `.first()` workaround.
Preserve desktop dimensions when adding coarse-pointer target sizing.
## Parallelism
`sequential`
## Inputs
- [System design](../../specs/tasks/system-design/workflow-step-agent-start-ownership.md#persist-a-visible-failure).
- `apps/web/AGENTS.md`, mobile-parity, e2e, and docs-maintainer skills.
- `LastAgentErrorNotice` in `message-list-shared.tsx`.
- Existing `agent-error-indicator.spec.ts` and `mobile-provider-remediation-link.spec.ts`.
## Results
Verified the existing last-agent-error notice on desktop and phone after a
reload. The coarse-pointer dismiss control is 44 pixels while desktop sizing
is unchanged. Phone coverage verifies touch dismissal, persisted dismissal,
and zero document horizontal overflow.
Updated `docs/public/tasks-and-workflows.md` with the failed-reset behavior
and recovery path through session deletion and new session creation.
The desktop and phone coverage expands the localized technical-details
disclosure after reload and asserts the exact reset cause. The phone test keeps
the disclosure open while measuring horizontal overflow and checks the coarse
pointer dismiss target at 44 by 44 pixels.
Verification passed:
```text
(cd apps/web && pnpm exec vitest run components/task/chat/message-list-shared.test.tsx)
(cd apps/web && pnpm e2e:run --project chromium tests/session/agent-error-indicator.spec.ts)
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/session/mobile-workflow-reset-error.spec.ts)
(cd apps/web && pnpm exec eslint components/task/chat/message-list-shared.tsx components/task/chat/message-list-shared.test.tsx e2e/tests/session/agent-error-indicator.spec.ts e2e/tests/session/mobile-workflow-reset-error.spec.ts e2e/tests/session/workflow-reset-error-helpers.ts)
(cd apps/web && pnpm run typecheck)
```
docs/public/tasks-and-workflows.md
+7 -2
View File
@@ -545,8 +545,13 @@ end behavior.
When **Reset agent context** creates a fresh ACP session, Kandev preserves the
selected ACP model, permission mode, and provider options. It restores these
settings before the next automatic prompt. If the provider rejects a setting,
the restoration fails and Kandev does not send the destination step's automatic
prompt.
the reset fails, or the provider does not answer the reset request, Kandev
leaves the session waiting for input. It does not send the destination step's
automatic prompt. The conversation keeps a visible previous-agent-error notice
with the reset cause. To recover, delete the affected conversation from its
session actions, then create a new session for the task. The task workspace and
files remain available to the new session. See [Sessions and review](sessions-and-review.md)
for the session actions and mobile session picker.
The WIP check also applies when a task is created. It runs for an explicit
`workflow_step_id` and for the workflow's resolved start step, and the
docs/specs/tasks/requirements/workflow-step-agent-start-ownership.md
+4 -1
View File
@@ -2,7 +2,7 @@
status: draft
system: tasks
created: 2026-08-05
updated: 2026-09-05
updated: 2026-09-12
owners:
- Kandev
---
@@ -34,6 +34,9 @@ This document is the migrated task-system source for the capability. The source
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4:** When turn quiescence or context replacement fails, the system shall not dispatch the automatic step prompt.
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.5:** When a prompt waits for an unresolved dispatch-only completion, the wait shall end within a bounded period and release session admission.
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6:** A delayed completion from the replaced turn shall not complete or release a later prompt generation.
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.7:** If cancellation ends without provider stop confirmation, workflow reset shall fail without replacing provider context or dispatching the automatic prompt.
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.8:** If a provider receives a context-reset request but does not respond, that request shall fail within 10 seconds. An earlier caller deadline shall take precedence. The failed request shall release the session reset guard instead of waiting indefinitely for a response.
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.9:** If workflow context reset fails, the session shall show a persistent error on desktop and mobile. The error shall identify the failed reset and explain that the step prompt did not start. After reset cleanup, session deletion shall remain available. A failed reset shall not appear as a successful context reset.
### REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003: Single-use task-description fallback
docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md
+69 -2
View File
@@ -23,7 +23,7 @@ The design preserves runtime configuration through the existing reset contract.
| Requirement | Design sections |
| --- | --- |
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001` | [Session states](#session-states) |
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002` | [Active-turn reset flow](#active-turn-reset-flow), [Bounded predecessor wait](#bounded-predecessor-wait) |
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002` | [Active-turn reset flow](#active-turn-reset-flow), [Bounded predecessor wait](#bounded-predecessor-wait), [Reset failure containment](#reset-failure-containment) |
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-003` | [Prompt fallback ownership](#prompt-fallback-ownership), [Prompt-history contract](#prompt-history-contract), [Workflow-entry prompt flow](#workflow-entry-prompt-flow) |
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-004` | [Creation destination routing](#creation-destination-routing) |
@@ -113,7 +113,74 @@ Provider completion events keep their `(agent_execution_id, prompt_generation)`
An unnumbered completion cannot release a pending numbered dispatch-only prompt. This prevents a delayed synthetic completion from being consumed as the predecessor's completion.
Internal cancellation finishes or escalates the old generation before provider replacement. The reset does not drain a completion signal from an active generation.
Internal cancellation reconciles the old generation before provider replacement. Escalation releases local completion state but does not prove that the provider stopped. The reset does not drain a completion signal from an active generation.
## Reset failure containment
This draft extension covers `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002`.
Its delivery package is [Workflow reset failure containment](../../../plans/workflow-reset-failure-containment/plan.md).
The task system owns this contract because workflow entry controls successor prompt admission.
### Preserve the provider cancellation outcome
The cancellation coordinator retains the raw provider outcome separately from its reconciliation result.
`cancelOperation` owns this outcome under the existing coordinator synchronization.
`cancelAgentWhileUnlocked` must not erase `lifecycle.ErrCancelEscalated` before that outcome is captured.
Explicit cancellation, clarification, peer interruption, and queue cancellation retain their current success semantics after escalation.
They still reconcile the captured turn and session. Their shared operation error must not become an escalation error.
Joined explicit callers must still perform their own reconciliation.
The exclusive reset helper reads the provider outcome after the owned cancellation operation completes.
`quiesceActiveResetTurn` rejects escalation even when local reconciliation succeeds.
An unset outcome cannot count as provider confirmation for an active execution.
Missing executions retain the existing persisted-resume cleanup behavior after lookup confirms their absence.
This path returns reset failure. It does not attempt automatic process replacement after escalation.
Process replacement needs stronger evidence than local cancellation reconciliation.
The existing restart helper tolerates a stop error, so calling it alone does not prove provider termination.
The reset marker and guard retain their existing ownership and release order.
### Bound the provider reset request
`Manager.ResetAgentContext` supplies a child context with a 10-second deadline to `client.ResetSession`.
The earlier caller deadline wins. The child context covers both the WebSocket write and response wait.
The HTTP client timeout does not apply to this WebSocket request.
On reset timeout or caller cancellation, return a wrapped error before the restart fallback.
Do not classify a timeout as unsupported reset. Do not launch a goroutine that returns while provider work still owns the guard.
The WebSocket pending-request cleanup removes the abandoned correlation ID.
Late replies cannot publish reset success, clear a successor generation, or dispatch a step prompt.
Ordinary unsupported-reset errors retain the existing restart fallback and captured runtime configuration.
Timeout changes neither that fallback's semantics nor its separate initialization budget.
Contention on `remoteInstanceLifecycleMu` or `streamWriteMu` is a separate wait from the unanswered request addressed here.
Tests must establish that the peer received the request before measuring its response deadline.
### Persist a visible failure
The workflow reset returns an error through an error-bearing helper.
The existing boolean wrapper can remain for callers that need only success or failure.
`processOnEnter` records the error and preserves its early return before automatic prompt dispatch.
It must not call the general agent-failure handler, which can run `on_agent_error` workflow actions.
Use the existing `LastAgentError` metadata and `persistLastAgentError` publication path.
Store a bounded, sanitized reset-specific message with the session and current execution identity.
The message states that context reset failed and that the workflow step prompt did not start.
Publish the waiting-state projection from fresh metadata so it cannot overwrite the error with the pre-reset session snapshot.
Use a short bounded cleanup context if the request context has expired.
Log persistence failure with task, session, and step identity; never report a successful reset after that failure.
Desktop and phone reuse `LastAgentErrorNotice` in the existing conversation flow.
The notice remains visible after reload and uses the existing dismissal behavior.
No new control, navigation surface, or workflow recovery action is introduced.
The phone conversation remains its own scroll surface, with the existing dismissal control.
Give that control a coarse-pointer target of at least 44 pixels without changing its desktop dimensions.
Any new static frontend copy must use the existing five-language localization contract.
Automatic prompting remains blocked on every reset failure.
An operator can remove the affected session after reset releases its guard.
This change does not make re-entering an idle step an automatic restart gesture.
## Prompt fallback ownership