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

fix: harden startup recovery timeout and scrolling (#3654)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-13 21:57:10 +01:00
committed by GitHub
parent bc38ea1ec7
commit 7d4bd1104a
26 changed files with 1903 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/startup-recovery-scroll-timeout/plan.md
+139
View File
@@ -0,0 +1,139 @@
---
created: 2026-09-13
status: completed
requirements:
- REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-002
- REQ-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006
system_design:
- ../../specs/tasks/system-design/remote-contribution-tasks.md
- ../../specs/agents/system-design/session-recovery-failures.md
legacy_specs: []
---
# Implementation Plan: Startup Recovery Scrolling and Timeout
## Overview
Allow more time for contribution checks on a busy machine. Keep the recovery
card and transcript in one scroll area. Implement two sequential work orders.
## Evidence and scope
Task `539ebf53-b1a8-476c-829a-f8b466287d34` failed during resume on September 13.
The preflight HTTP request exceeded its 30-second deadline at 17:09:06 Lisbon time.
A later preflight completed in 18.749 seconds. Resume succeeded at 17:09:52.
The user reported 100% CPU usage on the host. Logs do not independently prove
which resource delayed Git. The two-minute budget is a proposed tolerance,
not a measured guarantee under arbitrary host starvation.
The screenshot shows two scrollbars. Source confirms that `PanelBody` uses
`overflow-auto`, while its full-height transcript has another scroll viewport.
The recovery card adds height above that full-height child.
In scope: bounded preflight tolerance, transport deadline alignment, shared
chat scroll ownership, recovery reveal, and desktop/phone regression coverage.
Out of scope: global timeout increases, adaptive CPU policy, automatic retries,
Git publication, recovery authorization changes, and provider session replacement.
## Requirement conformance
Existing recovery criterion 006.6 and its responsive design require shared
scroll ownership. Draft criterion 006.7 defines reveal and user-scroll behavior.
Draft contribution criterion 002.6 defines the longer bounded check.
Existing contribution criteria 002.3 and 002.4 retain rejection and all-repository gates.
The owning requirements and designs contain both amendments.
## Technical approach
Task 01 gives the full contribution preflight loop two minutes. A dedicated
bounded request path avoids the ordinary HTTP client's 60-second cutoff.
Preserve earlier deadlines, cancellation, response decoding, and shared transport.
The existing overall readiness default is 15 minutes and needs no increase.
Task 02 moves the recovery card inside the transcript viewport and disables
outer scrolling in the chat panel. Coordinate one-time reveal with the existing
scroll controller. Key bootstrap reveals by session and failure stamp, and
fallback recovery reveals by session, attempt, and outcome. Keep an active
recovery reveal at the transcript top while older pages settle, then return
prepend ownership to the reader after a downward gesture.
No new recovery state store or duplicate action owner is necessary.
## ASCII UI preview
Before: a scrollable body contains a recovery card plus a full-height scrolling
transcript. Their combined height exceeds the available body height.
```text
UI-01: Task Chat, active startup failure (desktop and phone)
+---------------------------------------+
| Session navigation | fixed
+---------------------------------------+
| Startup failure summary ^ |
| Resume / Restore / Start fresh | |
| Recovery details (expand inline) | | one scroll area
| | |
| Conversation history v |
+---------------------------------------+
| Composer | fixed
+---------------------------------------+
```
Desktop uses the existing compact action row. Phone stacks the same actions
with 44-pixel touch targets. Expanded details wrap in the same scroll area.
A new failure reveals the card once. The user can then scroll freely.
The composer retains safe-area clearance. Labels and spacing are illustrative.
Scroll ownership, action reachability, and one-time reveal are requirements.
## Mobile design contract
Entry: the dedicated task Chat surface in `mobile/session-mobile-layout.tsx`.
Exemplar: the existing inline recovery card and task mobile layout.
Hierarchy: cause, primary Resume action, secondary recovery actions, then details.
The card stays inline because it explains the current conversation failure.
Desktop and phone share recovery state and handlers. Phone uses stacked actions.
One transcript viewport owns vertical scrolling, including long expanded details.
No horizontal document overflow or new fixed-height phone wrapper is permitted.
## Tests
Task 01 covers contribution criteria 002.3, 002.4, and 002.6 with lifecycle and
agentctl client tests. Use controlled transport responses and short test budgets.
Prove extended-budget success, timeout, cancellation, earlier caller deadline,
and a mixed multi-repository result. No sustained CPU load is required.
## E2E tests
Task 02 extends `launch-failure-recovery.spec.ts` and
`mobile-launch-failure-recovery.spec.ts` for recovery criteria 006.1, 006.6, and 006.7.
Seed history taller than the viewport. Assert one scroll owner, initial/new-error
reveal, expanded details, user-controlled scrolling, reload, and composer access.
Check desktop, the configured phone project, and a short viewport.
Existing completed recovery packages retain their historical results.
## Work orders
- [x] [Task 01: Extend the bounded preflight budget](task-01-preflight-budget.md)
- [x] [Task 02: Use one recovery and transcript scroll area](task-02-chat-scroll.md)
## Verification results
Targeted product tests and review regressions: passed.
- `python3 scripts/list-docs.py validate`: passed (267 decisions, 868 specifications).
- `python3 scripts/lint-spec-files.py --all`: passed.
- `(cd apps/backend && go test -race ./internal/agent/runtime/agentctl ./internal/agent/runtime/lifecycle)`: passed.
- `(cd apps/web && pnpm exec vitest run components/task/chat/message-list-native.test.tsx components/task/chat/message-list-native-scroll.test.ts components/quick-chat/quick-chat-session-view.test.tsx hooks/domains/session/use-session-resumption.test.ts)`: passed (110 tests), including controlled older-page settling, an in-flight page when a new failure arrives, and same-session fallback recovery attempts.
- PR fixup regression suite: passed (119 tests), including visible task-wide and persisted-metadata recovery reveal keys and resumption of normal auto-scroll after the one-time recovery placement.
- `(cd apps/web && pnpm run lint && pnpm run typecheck && pnpm run i18n:check)`: passed.
- `(cd apps/backend && make build)`: passed.
- `(cd apps/backend && make test)`: reached unrelated environment-sensitive failures in process probes, config-home discovery, launcher, and Office SQLite migration tests; the changed `agentctl` and lifecycle packages passed.
- `(cd apps/web && pnpm run build)`: passed.
- Desktop and phone bootstrap recovery E2E scenarios: passed.
- `git diff --check`: passed.
- Plan status inventory: both work orders completed.
## Risks
A hung preflight can take longer to report failure. Cancellation must still work.
A new card reveal can conflict with bottom-follow or hidden-panel restoration.
Short viewports require the full card to share the transcript scroll area.
docs/plans/startup-recovery-scroll-timeout/task-01-preflight-budget.md
+92
View File
@@ -0,0 +1,92 @@
---
id: "01-preflight-budget"
title: "Extend the bounded preflight budget"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-002
acceptance_criteria:
- AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.3
- AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.4
- AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.6
system_design:
- ../../specs/tasks/system-design/remote-contribution-tasks.md
---
# Task 01: Extend the Bounded Preflight Budget
## Summary
Give contribution startup checks one two-minute budget across all repositories.
Make the preflight HTTP path honor that budget without changing unrelated requests.
## In scope
- Extend the lifecycle default and bound direct preflight callers.
- Reuse shared transport and response handling without mutating the shared client.
- Preserve earlier deadlines, cancellation, rejection, and history-only admission rules.
## Out of scope
Global HTTP, ACP load, workspace refresh, and readiness timeout changes.
No new operator setting, retry loop, or credential policy.
## Acceptance
- A preflight can complete within two minutes despite the ordinary client's shorter timeout.
- Expiry or cancellation blocks startup, with no late agent launch or permission bypass.
- All repositories share one budget. One rejection blocks the complete operation.
## Verification
First add `TestGitPushPreflightHonorsOperationBudget` with a short ordinary-client
timeout and a controlled response. Its expected RED is premature transport expiry.
Extend `TestStartAgentProcessBoundsContributionPreflight` for the default budget,
short injected expiry, and caller cancellation. Preserve existing admission tests.
Use barriers or a controlled transport. Do not run two-minute sleeps or CPU stress.
```bash
(cd apps/backend && go test -race ./internal/agent/runtime/agentctl ./internal/agent/runtime/lifecycle)
git diff --check
```
## Files likely touched
- `apps/backend/internal/agent/runtime/lifecycle/manager_startup.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_startup_test.go`
- `apps/backend/internal/agent/runtime/agentctl/git.go`
- `apps/backend/internal/agent/runtime/agentctl/git_test.go`
- `apps/backend/internal/agent/runtime/agentctl/client.go` (only if the scoped request path needs it)
## Dependencies
None.
## Risks
A transport-specific timeout or shared client mutation can defeat the intended bound.
Tests must cover ordinary request isolation and earlier caller deadlines.
## Parallelism
`sequential`
## Inputs
- [Plan](plan.md), evidence and technical approach.
- [Contribution requirements](../../specs/tasks/requirements/remote-contribution-tasks.md), requirement 002.
- [Contribution design](../../specs/tasks/system-design/remote-contribution-tasks.md), proposed timing amendment.
- Existing startup preflight and client Git request tests.
## Results
Implemented a two-minute default for the complete contribution preflight loop.
`GitPushPreflight` uses a per-operation client copy with the same transport and
headers, while the ordinary Git client keeps its 60-second timeout. Direct
preflight callers receive the same bounded context, and earlier cancellation or
deadlines still win. Rejections continue to block startup before agent launch.
Validation passed with the agentctl and lifecycle race suite, targeted preflight
tests, and `git diff --check`.
docs/plans/startup-recovery-scroll-timeout/task-02-chat-scroll.md
+164
View File
@@ -0,0 +1,164 @@
---
id: "02-chat-scroll"
title: "Use one recovery and transcript scroll area"
status: done
wave: 2
depends_on:
- "01-preflight-budget"
plan: "plan.md"
requirements:
- REQ-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006
acceptance_criteria:
- AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.1
- AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.6
- AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.7
system_design:
- ../../specs/agents/system-design/session-recovery-failures.md
---
# Task 02: Use One Recovery and Transcript Scroll Area
## Summary
Put the recovery card inside the transcript viewport. Reveal a new failure once,
then preserve user scrolling and the existing composer layout.
## In scope
- Remove outer chat scrolling without changing the shared PanelBody default.
- Pass the recovery content into the real MessageList scroll viewport.
- Coordinate failure reveal, bottom-follow, session switching, and panel visibility.
- Cover desktop and phone, including long history and expanded details.
- Audit preview and Quick Chat callers for the same composition defect.
## Out of scope
New recovery actions, duplicate banners, translation changes, and transcript virtualization.
No permanent fixed recovery panel or nested details scroller.
## Acceptance
- Exactly one chat content viewport scrolls when history exceeds its height.
- Initial and new failures reveal the card once. Repeated state events preserve user position.
- Expanded details, recovery actions, and composer remain reachable on desktop and phone.
## ASCII UI preview
[Full preview and mobile contract](plan.md#ascii-ui-preview).
Applies to recovery criteria 006.1, 006.6, and 006.7.
```text
UI-01: Task Chat, active startup failure (desktop and phone)
+---------------------------------------+
| Session navigation | fixed
+---------------------------------------+
| Startup failure summary ^ |
| Resume / Restore / Start fresh | |
| Recovery details (expand inline) | | one scroll area
| | |
| Conversation history v |
+---------------------------------------+
| Composer | fixed
+---------------------------------------+
```
Desktop uses the existing compact action row. Phone stacks the same actions
with 44-pixel touch targets. Expanded details wrap in the same scroll area.
A new failure reveals the card once. The user can then scroll freely.
The composer retains safe-area clearance. Labels and spacing are illustrative.
Scroll ownership, action reachability, and one-time reveal are requirements.
## Verification
Extend the existing bootstrap-card scenarios with history taller than the viewport.
The expected RED is two independently scrollable ancestors or a card outside the
active viewport. Do not use a missing selector as the behavioral failure.
Assert computed overflow and actual scroll movement. Scroll history manually and
send the same error stamp again. Assert that the viewport does not jump.
Send a new stamp and assert that the card enters the viewport.
Expand long details and reach the final recovery action at a short height.
Keep the normal no-error bottom-follow case. Capture desktop and phone screenshots.
Use the existing isolated fixtures. Never navigate a test browser to the live task.
If dependencies are absent, run the install command once before package commands.
```bash
(cd apps && pnpm install --frozen-lockfile)
(cd apps/web && pnpm e2e:run --project chromium tests/task/launch-failure-recovery.spec.ts)
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/task/mobile-launch-failure-recovery.spec.ts)
(cd apps/web && pnpm run typecheck)
git diff --check
```
If scroll-controller logic changes, add focused controller tests in the new
`message-list-native-scroll.test.ts` file. Run the exact command from the root:
```bash
(cd apps/web && pnpm exec vitest run components/task/chat/message-list-native-scroll.test.ts)
```
## Files likely touched
- `apps/web/components/task/task-chat-panel.tsx`
- `apps/web/components/task/chat/message-list-shared.tsx`
- `apps/web/components/task/chat/message-list-native.tsx`
- `apps/web/components/task/chat/message-list-native-scroll.ts`
- `apps/web/components/task/chat/message-list-native-scroll.test.ts`
- `apps/web/components/quick-chat/quick-chat-session-view.tsx` (if the same defect applies)
- `apps/web/e2e/tests/task/launch-failure-recovery.spec.ts`
- `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts`
- A shared E2E helper beside these specs, if both need the same geometry checks.
## Dependencies
Task 01 precedes this task for sequential delivery. There is no schema dependency.
## Risks
Reveal can conflict with initial bottom positioning or hidden-panel restoration.
Use session and error identity, not translated error text, for reveal ownership.
## Parallelism
`sequential`
## Inputs
- [Recovery requirements](../../specs/agents/requirements/session-recovery-failures.md), requirement 006.
- [Recovery design](../../specs/agents/system-design/session-recovery-failures.md), responsive and scrolling sections.
- The current native transcript controller and bootstrap recovery E2E scenarios.
## Results
Moved task and Quick Chat recovery content into the native transcript viewport,
disabled the outer chat body scroll, and coordinated one-time recovery reveals
with the existing placement and auto-scroll controllers. The reveal is keyed by
session and failure identity, preserves later user scrolling, and keeps the
existing phone action layout and composer allocation.
Added focused scroll-controller coverage plus long-history desktop and phone
recovery scenarios. Both browser scenarios passed with expanded details, one
scroll owner, same-stamp position preservation, new-stamp reveal, composer
visibility, and phone touch-target checks.
Review fixes:
- Recovery-owned prepend pages keep the newly revealed card at the transcript
top while a page settles, including a page already in flight when a new
failure arrives. A real downward wheel, key, pointer, or touch gesture gives
scroll ownership back to the reader, after which normal message anchoring
resumes.
- The reveal key now follows the recovery surface that is actually rendered,
including task-wide errors while another session is selected and persisted
session metadata fallbacks.
- The one-time recovery placement latch clears after the initial reveal even
while feedback remains visible, so normal bottom-follow, activation catch-up,
and persisted-offset restoration resume afterward.
- Quick Chat fallback recovery keys now use the session, monotonic recovery
attempt, and outcome identity. Clearing feedback re-arms the reveal latch.
Duplicate same-attempt state, including changed translated error text,
keeps the existing key.
- Added controlled pagination, same-session recovery, visible-error-key, and
post-reveal auto-scroll regression coverage. The review cleanup also names
the raw resumption setter clearly and removes the obsolete test harness cast.
docs/specs/agents/requirements/session-recovery-failures.md
+5
View File
@@ -45,8 +45,13 @@ results, including the shared recovery owner and phone touch-target checks.
- **AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.4:** Retry shall update the existing card and disable equivalent actions while pending. Successful resume shall clear the active failure; workspace-only success shall show a nonblocking notice that the agent remains stopped.
- **AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.5:** Reload, reconnect, and reversed event order shall converge on the current failure. A stale attempt or unrelated historical error shall neither replace nor be hidden by that failure.
- **AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.6:** Desktop and phone shall expose the same recovery choices and details. Phone actions shall have at least 44-pixel touch targets, with no horizontal page overflow or extra details scroller.
- **AC-AGENTS-AGENT-RESUME-RUNTIME-RECOVERY-006.7:** Chat shall use one vertical scroll area for the recovery card, expanded details, and transcript. Opening a session with an active failure, or receiving a new failure, shall reveal the card. Later user scrolling shall remain under user control. Recovery actions and the composer shall remain reachable on desktop and phone.
## Proposed recovery scrolling amendment
This amendment is draft. The [startup recovery fix package](../../../plans/startup-recovery-scroll-timeout/plan.md) owns implementation of criterion 006.7.
## Proposed recovery attempt amendment
The following requirement is draft. The existing requirements remain active.
docs/specs/agents/system-design/session-recovery-failures.md
+19
View File
@@ -97,6 +97,25 @@ Use 28-pixel fine-pointer buttons and at least 44-pixel phone/coarse-pointer
targets. The semantic disclosure supports Enter/Space and expanded state.
### Proposed recovery scroll ownership
The [startup recovery fix package](../../../plans/startup-recovery-scroll-timeout/plan.md) implements criterion 006.7.
Place the recovery card inside the existing transcript viewport, before the message content.
Disable outer `PanelBody` scrolling for the mounted chat.
Preserve the fixed composer and existing dynamic viewport and safe-area layout.
Expanded recovery details remain in the transcript scroll area.
Do not add a card scroller, duplicate banner, or permanent fixed recovery panel.
Reveal the active card once per session and failure stamp when the chat becomes visible.
Use the existing transcript scroll controller to coordinate this reveal with initial bottom positioning.
A repeated event with the same stamp must not pull the user away from older messages.
Scope reveal state to session identity and defer hidden-panel scrolling until visibility returns.
Preserve normal bottom-follow behavior when no active recovery card exists.
Task detail and preview share the chat panel. Audit Quick Chat for the same nested-scroll condition.
The dedicated phone Chat surface retains stacked touch actions and one-dimensional navigation.
The recovery card remains the sole explanation and action owner.
## Persistence and compatibility
Use the existing error records and optional fields specified by the
docs/specs/tasks/requirements/remote-contribution-tasks.md
+5
View File
@@ -41,6 +41,11 @@ user intent for destructive replacement, and evidence-based version comparison.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.3:** Authentication, permission, invalid destination, missing source branch, network, timeout, and unclassified preflight failures shall remain distinct from confirmed history rejection. History rejection shall not be represented as proof of write permission.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.4:** A task with multiple repositories shall start only when every required preflight passes or qualifies for the history-only resume exception. One qualifying repository shall not hide a blocking failure in another.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.5:** After resume, the existing Changes surface shall retain its provider/local version choices and their authorization conditions. A history-only rejection shall not create an agent failure or a recovery banner.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-002.6:** Startup contribution checks shall allow up to two minutes across all required repositories, unless an earlier caller deadline or cancellation applies. A check that finishes within this budget shall not fail because of a shorter internal transport timeout. Budget expiry shall block startup and retain recovery actions.
## Proposed preflight timing amendment
This amendment is draft. The [startup recovery fix package](../../../plans/startup-recovery-scroll-timeout/plan.md) owns implementation of criterion 002.6.
## Delivery
docs/specs/tasks/system-design/remote-contribution-tasks.md
+17
View File
@@ -64,6 +64,23 @@ that a future push will succeed. Actual pushes keep all existing checks.
Blocking startup failures use the
[task launch failure projection](task-launch-failure-recovery.md).
## Proposed preflight timing amendment
The [startup recovery fix package](../../../plans/startup-recovery-scroll-timeout/plan.md) implements criterion 002.6.
Use one two-minute context for the complete `preflightRemoteContributionPushes` loop.
An earlier caller deadline or cancellation still ends the operation.
`GitPushPreflight` currently uses the ordinary client with a 60-second timeout.
Give this operation a bounded transport path that honors the complete preflight budget.
Reuse the existing transport, authentication, response decoding, and error handling.
Do not mutate the shared HTTP client or increase unrelated Git request timeouts.
Direct preflight callers without a deadline also receive the two-minute bound.
The normal readiness budget already exceeds this preflight budget.
Keep readiness, ACP load, workspace refresh, and Git publication deadlines unchanged.
A delayed permission rejection remains a rejection. Expiry never grants admission.
Use short test budgets and controlled responses instead of CPU saturation or long sleeps.
## Migrated design source
## Why