From 7d4bd1104ad5e24b6931a1d6ebed10d2df2b1d8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sun, 13 Sep 2026 21:57:10 +0100 Subject: [PATCH] fix: harden startup recovery timeout and scrolling (#3654) --- .../internal/agent/runtime/agentctl/git.go | 22 +- .../agent/runtime/agentctl/git_test.go | 26 + .../runtime/lifecycle/manager_startup.go | 11 +- .../runtime/lifecycle/manager_startup_test.go | 231 ++++++++ .../quick-chat/quick-chat-content.tsx | 8 +- .../quick-chat-session-view.test.tsx | 79 ++- .../quick-chat/quick-chat-session-view.tsx | 40 +- .../chat/message-list-native-scroll.test.ts | 39 ++ .../task/chat/message-list-native-scroll.ts | 495 +++++++++++++----- .../task/chat/message-list-native.test.tsx | 328 +++++++++++- .../task/chat/message-list-native.tsx | 43 +- .../task/chat/message-list-shared.tsx | 4 + .../task-chat-panel.launch-error.test.tsx | 54 +- apps/web/components/task/task-chat-panel.tsx | 44 +- apps/web/e2e/helpers/transcript-scroll.ts | 44 ++ .../task/launch-failure-recovery.spec.ts | 68 ++- .../mobile-launch-failure-recovery.spec.ts | 71 ++- .../session/use-session-resumption.test.ts | 2 + .../domains/session/use-session-resumption.ts | 18 +- .../startup-recovery-scroll-timeout/plan.md | 139 +++++ .../task-01-preflight-budget.md | 92 ++++ .../task-02-chat-scroll.md | 164 ++++++ .../requirements/session-recovery-failures.md | 5 + .../session-recovery-failures.md | 19 + .../requirements/remote-contribution-tasks.md | 5 + .../remote-contribution-tasks.md | 17 + 26 files changed, 1903 insertions(+), 165 deletions(-) create mode 100644 apps/web/components/task/chat/message-list-native-scroll.test.ts create mode 100644 apps/web/e2e/helpers/transcript-scroll.ts create mode 100644 docs/plans/startup-recovery-scroll-timeout/plan.md create mode 100644 docs/plans/startup-recovery-scroll-timeout/task-01-preflight-budget.md create mode 100644 docs/plans/startup-recovery-scroll-timeout/task-02-chat-scroll.md diff --git a/apps/backend/internal/agent/runtime/agentctl/git.go b/apps/backend/internal/agent/runtime/agentctl/git.go index 9835c76531..56b696382c 100644 --- a/apps/backend/internal/agent/runtime/agentctl/git.go +++ b/apps/backend/internal/agent/runtime/agentctl/git.go @@ -7,10 +7,13 @@ import ( "fmt" "net/http" "net/url" + "time" ) const GitPushPreflightHistoryUpdateRequired = "history_update_required" +const gitPushPreflightTimeout = 2 * time.Minute + // GitOperationResult represents the result of a git operation. // This matches the server-side process.GitOperationResult. type GitOperationResult struct { @@ -105,7 +108,13 @@ func (c *Client) GitPushPreflight(ctx context.Context, repo string, opts PushOpt Remote: opts.Remote, ExpectedBranch: opts.ExpectedBranch, } - return c.gitOperation(ctx, "/api/v1/git/push-preflight", payload) + return c.gitOperationWithClient(ctx, "/api/v1/git/push-preflight", payload, c.gitPushPreflightClient()) +} + +func (c *Client) gitPushPreflightClient() *http.Client { + client := *c.httpClient + client.Timeout = gitPushPreflightTimeout + return &client } // GitReplaceRemoteContribution replaces the bound contribution branch when @@ -335,6 +344,15 @@ func (c *Client) GitCreatePR(ctx context.Context, title, body, baseBranch string // gitOperation is a helper that performs a git operation via HTTP POST. func (c *Client) gitOperation(ctx context.Context, path string, payload interface{}) (*GitOperationResult, error) { + return c.gitOperationWithClient(ctx, path, payload, c.httpClient) +} + +func (c *Client) gitOperationWithClient( + ctx context.Context, + path string, + payload interface{}, + httpClient *http.Client, +) (*GitOperationResult, error) { body, err := json.Marshal(payload) if err != nil { return nil, fmt.Errorf("failed to marshal request: %w", err) @@ -346,7 +364,7 @@ func (c *Client) gitOperation(ctx context.Context, path string, payload interfac } req.Header.Set("Content-Type", "application/json") - resp, err := c.httpClient.Do(req) + resp, err := httpClient.Do(req) if err != nil { return nil, fmt.Errorf("failed to execute request: %w", err) } diff --git a/apps/backend/internal/agent/runtime/agentctl/git_test.go b/apps/backend/internal/agent/runtime/agentctl/git_test.go index bba4febe2e..31e2c257a5 100644 --- a/apps/backend/internal/agent/runtime/agentctl/git_test.go +++ b/apps/backend/internal/agent/runtime/agentctl/git_test.go @@ -4,13 +4,39 @@ import ( "context" "encoding/json" "net/http" + "net/http/httptest" "strings" "testing" + "time" ) // gitOperationOK is the canonical success body every /api/v1/git/* POST returns. const gitOperationOK = `{"success":true,"operation":"pull","output":"Already up to date."}` +func TestGitPushPreflightHonorsOperationBudget(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + timer := time.NewTimer(25 * time.Millisecond) + defer timer.Stop() + select { + case <-timer.C: + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"operation":"push-preflight"}`)) + case <-r.Context().Done(): + } + })) + t.Cleanup(server.Close) + + client := newHTTPOnlyClient(server.URL) + client.httpClient.Timeout = 10 * time.Millisecond + result, err := client.GitPushPreflight(context.Background(), "", PushOptions{}) + if err != nil || result == nil || !result.Success { + t.Fatalf("GitPushPreflight() = result %#v, error %v; want delayed response to succeed", result, err) + } + if got := client.httpClient.Timeout; got != 10*time.Millisecond { + t.Fatalf("ordinary client timeout = %s, want unchanged 10ms", got) + } +} + func TestGitPushSendsPushOptions(t *testing.T) { srv, got := captureServer(t, jsonResponder(http.StatusOK, gitOperationOK)) _, err := newHTTPOnlyClient(srv.URL).GitPush(context.Background(), "svc", PushOptions{ diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go b/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go index f09b6d8966..d64f80f0e1 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go @@ -19,7 +19,7 @@ import ( "github.com/kandev/kandev/internal/task/models" ) -const defaultRemoteContributionPreflightTimeout = 30 * time.Second +const defaultRemoteContributionPreflightTimeout = 2 * time.Minute func (m *Manager) contributionPreflightTimeout() time.Duration { if m.remoteContributionPreflightTimeout > 0 { @@ -171,9 +171,7 @@ func (m *Manager) startAgentProcess(ctx context.Context, executionID string) (re m.updateExecutionError(executionID, "agentctl not ready: "+err.Error()) return fmt.Errorf("agentctl not ready: %w", err) } - preflightCtx, cancelPreflight := context.WithTimeout(operationCtx, m.contributionPreflightTimeout()) - err = m.preflightRemoteContributionPushes(preflightCtx, execution) - cancelPreflight() + err = m.preflightRemoteContributionPushes(operationCtx, execution) if err != nil { m.updateExecutionError(executionID, "contribution push preflight failed: "+err.Error()) return err @@ -221,6 +219,9 @@ func (m *Manager) startAgentProcess(ctx context.Context, executionID string) (re } func (m *Manager) preflightRemoteContributionPushes(ctx context.Context, execution *AgentExecution) error { + preflightCtx, cancel := context.WithTimeout(ctx, m.contributionPreflightTimeout()) + defer cancel() + if execution == nil { return nil } @@ -242,7 +243,7 @@ func (m *Manager) preflightRemoteContributionPushes(ctx context.Context, executi } sort.Strings(keys) for _, key := range keys { - result, err := client.GitPushPreflight(ctx, key, agentctl.PushOptions{}) + result, err := client.GitPushPreflight(preflightCtx, key, agentctl.PushOptions{}) if err != nil { return &BootstrapFailure{ Operation: bootstrapOperation(execution), diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_startup_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_startup_test.go index ae7e7cc3be..e71bc2f207 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_startup_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_startup_test.go @@ -178,6 +178,10 @@ func TestStartAgentProcess_RunsContributionPreflightBeforeAgentStart(t *testing. } func TestStartAgentProcessBoundsContributionPreflight(t *testing.T) { + if got := (&Manager{}).contributionPreflightTimeout(); got != 2*time.Minute { + t.Fatalf("default contribution preflight timeout = %s, want 2m", got) + } + started := make(chan struct{}) release := make(chan struct{}) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -250,6 +254,233 @@ func TestStartAgentProcessBoundsContributionPreflight(t *testing.T) { } } +func newContributionPreflightExecution( + t *testing.T, + serverURL, id, sessionID, agentCommand string, + contributions map[string]models.RemoteContribution, +) *AgentExecution { + t.Helper() + host, portText, err := net.SplitHostPort(strings.TrimPrefix(serverURL, "http://")) + if err != nil { + t.Fatalf("parse agentctl test URL: %v", err) + } + port, err := strconv.Atoi(portText) + if err != nil { + t.Fatalf("parse agentctl test port: %v", err) + } + return &AgentExecution{ + ID: id, SessionID: sessionID, AgentCommand: agentCommand, + metadata: map[string]interface{}{MetadataKeyRemoteContributions: contributions}, + agentctl: agentctl.NewClient(host, port, newTestLogger()), + } +} + +func TestPreflightRemoteContributionPushesBoundsDirectCaller(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/git/push-preflight": + close(started) + <-release + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + + mgr := newTestManager(t) + mgr.remoteContributionPreflightTimeout = 10 * time.Millisecond + execution := newContributionPreflightExecution(t, server.URL, "exec-direct-preflight-timeout", "", "", + map[string]models.RemoteContribution{"": contributionPreflightTestBinding()}) + + errCh := make(chan error, 1) + go func() { errCh <- mgr.preflightRemoteContributionPushes(context.Background(), execution) }() + select { + case <-started: + case <-time.After(time.Second): + close(release) + t.Fatal("direct contribution preflight did not start") + } + select { + case err := <-errCh: + close(release) + if err == nil { + t.Fatal("preflight unexpectedly succeeded after its direct budget expired") + } + case <-time.After(time.Second): + close(release) + t.Fatal("direct preflight caller was not bounded") + } +} + +func TestPreflightRemoteContributionPushesUsesOneBudgetForAllRepositories(t *testing.T) { + firstStarted := make(chan struct{}) + secondStarted := make(chan struct{}) + firstRelease := make(chan struct{}) + firstReleased := false + releaseFirst := func() { + if !firstReleased { + firstReleased = true + close(firstRelease) + } + } + requestCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/git/push-preflight" { + http.NotFound(w, r) + return + } + requestCount++ + switch requestCount { + case 1: + close(firstStarted) + <-firstRelease + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + case 2: + close(secondStarted) + timer := time.NewTimer(70 * time.Millisecond) + defer timer.Stop() + select { + case <-timer.C: + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + case <-r.Context().Done(): + } + } + })) + t.Cleanup(func() { + releaseFirst() + server.Close() + }) + + mgr := newTestManager(t) + mgr.remoteContributionPreflightTimeout = 80 * time.Millisecond + execution := newContributionPreflightExecution(t, server.URL, "exec-multi-preflight-timeout", "", "", + map[string]models.RemoteContribution{ + "a": contributionPreflightTestBinding(), + "b": contributionPreflightTestBinding(), + }) + + errCh := make(chan error, 1) + go func() { errCh <- mgr.preflightRemoteContributionPushes(context.Background(), execution) }() + select { + case <-firstStarted: + case <-time.After(time.Second): + t.Fatal("first contribution preflight did not start") + } + timer := time.NewTimer(10 * time.Millisecond) + <-timer.C + timer.Stop() + releaseFirst() + select { + case <-secondStarted: + case <-time.After(time.Second): + t.Fatal("second contribution preflight did not start") + } + + select { + case err := <-errCh: + if err == nil { + t.Fatal("preflight succeeded after the shared budget expired") + } + case <-time.After(time.Second): + t.Fatal("preflight did not finish at the shared budget") + } + if requestCount != 2 { + t.Fatalf("preflight request count = %d, want both repositories checked", requestCount) + } +} + +func TestPreflightRemoteContributionPushesRejectsWhenAnyRepositoryFails(t *testing.T) { + requestCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/git/push-preflight" { + http.NotFound(w, r) + return + } + requestCount++ + if requestCount == 1 { + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"success": false, "error": "source branch is read-only"}) + })) + t.Cleanup(server.Close) + + mgr := newTestManager(t) + execution := newContributionPreflightExecution(t, server.URL, "exec-multi-preflight-rejected", "", "", + map[string]models.RemoteContribution{ + "a": contributionPreflightTestBinding(), + "b": contributionPreflightTestBinding(), + }) + + err := mgr.preflightRemoteContributionPushes(context.Background(), execution) + if err == nil || !strings.Contains(err.Error(), "source branch is read-only") { + t.Fatalf("preflight error = %v, want the failing repository rejection", err) + } + if requestCount != 2 { + t.Fatalf("preflight request count = %d, want both repositories checked", requestCount) + } +} + +func TestStartAgentProcessPreservesCallerCancellationDuringContributionPreflight(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + requests := make(chan string, 4) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests <- r.URL.Path + switch r.URL.Path { + case "/health": + w.WriteHeader(http.StatusOK) + case "/api/v1/git/push-preflight": + close(started) + <-release + default: + t.Errorf("unexpected agentctl request %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + } + })) + t.Cleanup(func() { + close(release) + server.Close() + }) + + mgr := newTestManager(t) + execution := newContributionPreflightExecution(t, server.URL, "exec-preflight-cancelled", "session-preflight-cancelled", "agent", + map[string]models.RemoteContribution{ + "": contributionPreflightTestBinding(), + }) + if err := mgr.executionStore.Add(execution); err != nil { + t.Fatalf("seed execution: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + errCh := make(chan error, 1) + go func() { errCh <- mgr.StartAgentProcess(ctx, execution.ID) }() + select { + case <-started: + case <-time.After(time.Second): + t.Fatal("contribution preflight did not start") + } + cancel() + + var startErr error + select { + case startErr = <-errCh: + case <-time.After(time.Second): + t.Fatal("StartAgentProcess did not return after caller cancellation") + } + if !errors.Is(startErr, context.Canceled) { + t.Fatalf("StartAgentProcess() error = %v, want context cancellation", startErr) + } + + gotRequests := []string{<-requests, <-requests} + if gotRequests[0] != "/health" || gotRequests[1] != "/api/v1/git/push-preflight" { + t.Fatalf("agentctl requests = %v, want readiness and preflight only", gotRequests) + } +} + func TestFailedConcurrentStartDoesNotReleaseLiveStartActivity(t *testing.T) { mgr := newTestManager(t) coordinator := activity.NewCoordinator(activity.Options{}) diff --git a/apps/web/components/quick-chat/quick-chat-content.tsx b/apps/web/components/quick-chat/quick-chat-content.tsx index 4e66cc6154..7c6d6e952a 100644 --- a/apps/web/components/quick-chat/quick-chat-content.tsx +++ b/apps/web/components/quick-chat/quick-chat-content.tsx @@ -1,6 +1,6 @@ "use client"; -import { memo, useCallback, useEffect, useRef, useState } from "react"; +import { memo, useCallback, useEffect, useRef, useState, type ReactNode } from "react"; import { useSettingsData } from "@/hooks/domains/settings/use-settings-data"; import { type ChatInputContainerHandle } from "@/components/task/chat/chat-input-container"; import { MessageList } from "@/components/task/chat/message-list"; @@ -21,6 +21,8 @@ type QuickChatContentProps = { placeholderOverride?: string; initialPrompt?: string; onInitialPromptAttempted?: () => void; + recoveryContent?: ReactNode; + recoveryRevealKey?: string | null; }; function useQuickChatState(sessionId: string) { @@ -50,6 +52,8 @@ export const QuickChatContent = memo(function QuickChatContent({ placeholderOverride, initialPrompt, onInitialPromptAttempted, + recoveryContent, + recoveryRevealKey, }: QuickChatContentProps) { const [clarificationKey, setClarificationKey] = useState(0); const shortcutScopeRef = useRef(null); @@ -108,6 +112,8 @@ export const QuickChatContent = memo(function QuickChatContent({ sessionState={panelState.session?.state} worktreePath={getSessionWorkspacePath(panelState.session)} onOpenFile={undefined} + prependContent={recoveryContent} + recoveryRevealKey={recoveryRevealKey} /> [] as Array<{ sessionId: string; taskId?: string; agentProfileId?: string }>, ); +const quickChatContentProps = vi.hoisted(() => [] as Array<{ recoveryRevealKey?: string | null }>); const useEnsureTaskSession = vi.hoisted(() => vi.fn()); const useTask = vi.hoisted(() => vi.fn()); const useSessionResumption = vi.hoisted(() => @@ -25,6 +27,7 @@ const useSessionResumption = vi.hoisted(() => error: null, notice: null, recoveryFailure: null, + recoveryAttemptId: 0, taskSessionState: null, worktreePath: null, worktreeBranch: null, @@ -50,7 +53,16 @@ vi.mock("@/components/task/passthrough-terminal", () => ({ PassthroughTerminal: () =>
, })); vi.mock("./quick-chat-content", () => ({ - QuickChatContent: () =>
, + QuickChatContent: ({ + recoveryContent, + recoveryRevealKey, + }: { + recoveryContent?: ReactNode; + recoveryRevealKey?: string | null; + }) => { + quickChatContentProps.push({ recoveryRevealKey }); + return
{recoveryContent}
; + }, })); vi.mock("@/components/task/chat/session-bootstrap-recovery-card", () => ({ SessionBootstrapRecoveryCard: ({ @@ -95,6 +107,7 @@ afterEach(() => { cleanup(); delete sessionRows[session.sessionId]; quickChatSessions.length = 0; + quickChatContentProps.length = 0; vi.clearAllMocks(); useTask.mockReset(); useTask.mockReturnValue(null); @@ -261,6 +274,70 @@ describe("QuickChatSessionView session resumption", () => { expect(screen.getByTestId(RECOVERY_CARD_TEST_ID).textContent).toContain("recovery_failed"); }); + it("keys unstamped recovery feedback by attempt and outcome while preserving duplicate state", () => { + const firstFailure = { + outcome: "recovery_failed" as const, + resumeError: "first resume failure", + restoreError: "first restore failure", + }; + const firstAttempt = { + resumptionState: "error" as const, + sessionStatus: null, + error: "first translated failure", + notice: null, + recoveryFailure: firstFailure, + recoveryAttemptId: 4, + taskSessionState: "FAILED" as const, + worktreePath: null, + worktreeBranch: null, + resumeSession: vi.fn(), + }; + useSessionResumption.mockReturnValue(firstAttempt as never); + + const view = render(); + const firstKey = quickChatContentProps.at(-1)?.recoveryRevealKey; + expect(firstKey).toBe("session-1:recovery:4:recovery_failed"); + + useSessionResumption.mockReturnValue({ + ...firstAttempt, + error: "same attempt, different translated failure", + } as never); + view.rerender(); + expect(quickChatContentProps.at(-1)?.recoveryRevealKey).toBe(firstKey); + + useSessionResumption.mockReturnValue({ + ...firstAttempt, + recoveryFailure: { + outcome: "status_unavailable" as const, + kind: "timeout" as const, + statusError: "status request failed", + }, + } as never); + view.rerender(); + expect(quickChatContentProps.at(-1)?.recoveryRevealKey).toBe( + "session-1:recovery:4:status_unavailable", + ); + + useSessionResumption.mockReturnValue({ + ...firstAttempt, + error: null, + recoveryFailure: null, + recoveryAttemptId: 4, + } as never); + view.rerender(); + expect(quickChatContentProps.at(-1)?.recoveryRevealKey).toBeNull(); + + useSessionResumption.mockReturnValue({ + ...firstAttempt, + error: "second translated failure", + recoveryAttemptId: 5, + } as never); + view.rerender(); + expect(quickChatContentProps.at(-1)?.recoveryRevealKey).toBe( + "session-1:recovery:5:recovery_failed", + ); + }); + it("keeps the bootstrap recovery card and automatic outcome in passthrough Quick Chat", () => { sessionRows[session.sessionId] = { task_id: HYDRATED_TASK_ID, is_passthrough: true }; useTask.mockReturnValue({ isArchived: false, workspaceId: WORKSPACE_ID }); diff --git a/apps/web/components/quick-chat/quick-chat-session-view.tsx b/apps/web/components/quick-chat/quick-chat-session-view.tsx index b04e851460..81d856a29c 100644 --- a/apps/web/components/quick-chat/quick-chat-session-view.tsx +++ b/apps/web/components/quick-chat/quick-chat-session-view.tsx @@ -13,6 +13,9 @@ import type { QuickChatSession } from "@/lib/state/slices/ui/types"; import { QuickChatContent } from "./quick-chat-content"; import { useTranslation } from "react-i18next"; +// i18n-exempt: internal recovery identity token, not user-facing copy. +const GENERIC_RECOVERY_OUTCOME = "feedback"; + function useIsQuickChatPassthrough(sessionId: string) { return useAppStore((state) => { const session = state.taskSessions.items[sessionId]; @@ -42,6 +45,33 @@ function resolveTaskArchiveState( return quickChatTaskId === taskId ? false : null; } +function resolveRecoveryRevealKey( + sessionId: string, + bootstrapRecoveryError: { stamp: string; occurred_at: string } | null, + hasRecoveryFeedback: boolean, + recoveryAttemptId: number | undefined, + recoveryOutcome: string | null, +): string | null { + if (bootstrapRecoveryError) { + return `${sessionId}:${bootstrapRecoveryError.stamp || bootstrapRecoveryError.occurred_at}`; + } + if (!hasRecoveryFeedback) return null; + return `${sessionId}:recovery:${recoveryAttemptId ?? 0}:${recoveryOutcome ?? GENERIC_RECOVERY_OUTCOME}`; +} + +function hasRecoveryFeedback(state: { + error: string | null; + notice: string | null; + recoveryFailure: unknown; +}): boolean { + return Boolean(state.error) || Boolean(state.notice) || state.recoveryFailure !== null; +} + +function recoveryOutcome(failure: { outcome: string } | null): string | null { + if (!failure) return null; + return failure.outcome; +} + export function QuickChatSessionView({ session, onInitialPromptAttempted, @@ -90,6 +120,13 @@ export function QuickChatSessionView({ ) : ( recoveryFeedback ); + const recoveryRevealKey = resolveRecoveryRevealKey( + session.sessionId, + bootstrapRecoveryError, + hasRecoveryFeedback(resumption), + resumption.recoveryAttemptId, + recoveryOutcome(resumption.recoveryFailure), + ); if (isPassthrough) { return (
@@ -103,7 +140,6 @@ export function QuickChatSessionView({ const isConfig = session.kind === "config"; return (
- {recoverySurface}
diff --git a/apps/web/components/task/chat/message-list-native-scroll.test.ts b/apps/web/components/task/chat/message-list-native-scroll.test.ts new file mode 100644 index 0000000000..ddfc20052f --- /dev/null +++ b/apps/web/components/task/chat/message-list-native-scroll.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { resolveCompetingInitialScrollOwner } from "./message-list-native-scroll"; + +const noProgrammaticLock = () => false; + +describe("resolveCompetingInitialScrollOwner", () => { + it("gives a new recovery failure the initial reveal before unread placement", () => { + expect( + resolveCompetingInitialScrollOwner({ + hasPendingLayoutRestore: false, + hasExplicitScrollTarget: false, + hasRecoveryReveal: true, + hasUnreadDivider: true, + isProgrammaticScrollLocked: noProgrammaticLock, + }), + ).toBe("recovery"); + }); + + it("preserves layout and explicit message placement precedence", () => { + expect( + resolveCompetingInitialScrollOwner({ + hasPendingLayoutRestore: true, + hasExplicitScrollTarget: true, + hasRecoveryReveal: true, + hasUnreadDivider: true, + isProgrammaticScrollLocked: noProgrammaticLock, + }), + ).toBe("layout-restore"); + expect( + resolveCompetingInitialScrollOwner({ + hasPendingLayoutRestore: false, + hasExplicitScrollTarget: true, + hasRecoveryReveal: true, + hasUnreadDivider: true, + isProgrammaticScrollLocked: noProgrammaticLock, + }), + ).toBe("explicit-target"); + }); +}); diff --git a/apps/web/components/task/chat/message-list-native-scroll.ts b/apps/web/components/task/chat/message-list-native-scroll.ts index 537e354bed..8486a23e5b 100644 --- a/apps/web/components/task/chat/message-list-native-scroll.ts +++ b/apps/web/components/task/chat/message-list-native-scroll.ts @@ -45,6 +45,7 @@ const SCROLL_KEYS = new Set([ "PageUp", " ", ]); +const RECOVERY_SCROLL_KEYS = new Set(["ArrowDown", "End", "PageDown", " "]); /** Writes a clamped maximum so the browser resolves the native bottom without * forcing a synchronous scrollHeight layout read. */ @@ -226,6 +227,121 @@ function reportPaginationSettleDebug(params: { }); } +type PrependScrollState = { + scrollHeight: number; + scrollTop: number; + anchorKey: string | null; + anchorTop: number | null; +}; + +type RecoveryRevealState = { + key: string | null; + keepAtTop: boolean; +}; + +/** Transfers recovery scroll ownership only after a real downward gesture. */ +function useRecoveryPrependScrollOwnership({ + scrollRef, + recoveryRevealStateRef, + isLoadingMoreRef, + olderLoadPendingRef, + newestItemKeyRef, + scrollState, +}: { + scrollRef: React.RefObject; + recoveryRevealStateRef: React.MutableRefObject; + isLoadingMoreRef: React.MutableRefObject; + olderLoadPendingRef: React.MutableRefObject; + newestItemKeyRef: React.MutableRefObject; + scrollState: React.MutableRefObject; +}) { + useEffect(() => { + const el = scrollRef.current; + if (!el) return; + let userScrollIntentUntil = 0; + let pointerActive = false; + let touchActive = false; + let touchStartY: number | null = null; + const markUserScrollIntent = () => { + userScrollIntentUntil = Date.now() + USER_SCROLL_INTENT_WINDOW_MS; + }; + const onWheel = (event: WheelEvent) => { + if (event.deltaY > 0) markUserScrollIntent(); + }; + const onKeyDown = (event: KeyboardEvent) => { + if (RECOVERY_SCROLL_KEYS.has(event.key)) markUserScrollIntent(); + }; + const onPointerDown = () => { + pointerActive = true; + markUserScrollIntent(); + }; + const onPointerUp = () => { + pointerActive = false; + }; + const onTouchStart = (event: TouchEvent) => { + touchActive = true; + touchStartY = event.touches[0]?.clientY ?? null; + }; + const onTouchMove = (event: TouchEvent) => { + const nextY = event.touches[0]?.clientY; + if (touchStartY !== null && nextY !== undefined && nextY < touchStartY) { + markUserScrollIntent(); + } + }; + const onTouchEnd = () => { + touchActive = false; + touchStartY = null; + }; + const onScroll = () => { + const recoveryScrolledAway = + recoveryRevealStateRef.current.keepAtTop && + el.scrollTop > 0 && + (userScrollIntentUntil >= Date.now() || pointerActive || touchActive); + if (recoveryScrolledAway) { + recoveryRevealStateRef.current.keepAtTop = false; + if (olderLoadPendingRef.current || isLoadingMoreRef.current) { + scrollState.current = capturePrependScrollState(el, newestItemKeyRef.current); + } + } + // Native overflow anchoring can adjust scrollTop while the older page is + // being inserted. Keep the pre-request baseline until our layout effect + // has restored the visual position explicitly. + if (olderLoadPendingRef.current || isLoadingMoreRef.current) return; + scrollState.current = capturePrependScrollState(el, newestItemKeyRef.current); + }; + scrollState.current = capturePrependScrollState(el, newestItemKeyRef.current); + el.addEventListener("wheel", onWheel, { passive: true }); + el.addEventListener("keydown", onKeyDown); + el.addEventListener("pointerdown", onPointerDown, { passive: true }); + el.addEventListener("pointerup", onPointerUp, { passive: true }); + el.addEventListener("pointercancel", onPointerUp, { passive: true }); + el.addEventListener("touchstart", onTouchStart, { passive: true }); + el.addEventListener("touchmove", onTouchMove, { passive: true }); + el.addEventListener("touchend", onTouchEnd, { passive: true }); + el.addEventListener("touchcancel", onTouchEnd, { passive: true }); + el.addEventListener("scroll", onScroll, { passive: true }); + return () => { + el.removeEventListener("wheel", onWheel); + el.removeEventListener("keydown", onKeyDown); + el.removeEventListener("pointerdown", onPointerDown); + el.removeEventListener("pointerup", onPointerUp); + el.removeEventListener("pointercancel", onPointerUp); + el.removeEventListener("touchstart", onTouchStart); + el.removeEventListener("touchmove", onTouchMove); + el.removeEventListener("touchend", onTouchEnd); + el.removeEventListener("touchcancel", onTouchEnd); + el.removeEventListener("scroll", onScroll); + }; + }, [ + isLoadingMoreRef, + newestItemKeyRef, + olderLoadPendingRef, + recoveryRevealStateRef, + scrollRef, + scrollState, + ]); +} + /** * Continuously captures scroll state via scroll listener. * On a genuine prepend (older messages loaded above the current view, so the @@ -243,19 +359,38 @@ function useScrollPositionOnPrepend( items: RenderItem[], isLoadingMore: boolean, isProgrammaticScrollLocked: () => boolean, + recoveryRevealKey: string | null, ): () => void { - const scrollState = useRef<{ - scrollHeight: number; - scrollTop: number; - anchorKey: string | null; - anchorTop: number | null; - }>({ scrollHeight: 0, scrollTop: 0, anchorKey: null, anchorTop: null }); + const scrollState = useRef({ + scrollHeight: 0, + scrollTop: 0, + anchorKey: null, + anchorTop: null, + }); const isLoadingMoreRef = useRef(isLoadingMore); const olderLoadPendingRef = useRef(false); const newestItemKeyRef = useRef(getNewestNonSyntheticItemKey(items)); + const recoveryRevealStateRef = useRef({ + key: recoveryRevealKey, + keepAtTop: recoveryRevealKey !== null, + }); + if (recoveryRevealStateRef.current.key !== recoveryRevealKey) { + recoveryRevealStateRef.current = { + key: recoveryRevealKey, + keepAtTop: recoveryRevealKey !== null, + }; + } isLoadingMoreRef.current = isLoadingMore; if (isLoadingMore) olderLoadPendingRef.current = true; newestItemKeyRef.current = getNewestNonSyntheticItemKey(items); + useRecoveryPrependScrollOwnership({ + scrollRef, + recoveryRevealStateRef, + isLoadingMoreRef, + olderLoadPendingRef, + newestItemKeyRef, + scrollState, + }); const prevItemCountRef = useRef(items.length); const prevFirstKeyRef = useRef(getOldestNonSyntheticItemKey(items)); const beginOlderLoad = useCallback(() => { @@ -265,23 +400,6 @@ function useScrollPositionOnPrepend( olderLoadPendingRef.current = true; }, [scrollRef]); - useEffect(() => { - const el = scrollRef.current; - if (!el) return; - /** Captures the container's current scrollHeight/scrollTop so a later - * prepend can restore the visual position. */ - const onScroll = () => { - // Native overflow anchoring can adjust scrollTop while the older page is - // being inserted. Keep the pre-request baseline until our layout effect - // has restored the visual position explicitly. - if (olderLoadPendingRef.current || isLoadingMoreRef.current) return; - scrollState.current = capturePrependScrollState(el, newestItemKeyRef.current); - }; - onScroll(); - el.addEventListener("scroll", onScroll, { passive: true }); - return () => el.removeEventListener("scroll", onScroll); - }, [scrollRef]); - useLayoutEffect(() => { const el = scrollRef.current; const nextFirstKey = getOldestNonSyntheticItemKey(items); @@ -299,6 +417,11 @@ function useScrollPositionOnPrepend( prevFirstKeyRef.current = nextFirstKey; if (olderLoadSettled) olderLoadPendingRef.current = false; if (!el || !prepend || isProgrammaticScrollLocked()) return; + if (recoveryRevealStateRef.current.key !== null && recoveryRevealStateRef.current.keepAtTop) { + if (el.scrollTop !== 0) el.scrollTop = 0; + scrollState.current = capturePrependScrollState(el, newestItemKeyRef.current); + return; + } const prev = scrollState.current; const anchor = findMessageRow(el, prev.anchorKey); if (anchor && prev.anchorTop !== null) { @@ -672,6 +795,7 @@ export function useAutoScroll(params: { hasUnreadDividerRef, sessionIdRef, isProgrammaticScrollLockedRef, + initialPlacementPending, }); return { isNearBottomRef, resyncIsNearBottom, markNotNearBottom }; @@ -910,6 +1034,7 @@ function useCatchUpOnVisible({ hasUnreadDividerRef, sessionIdRef, isProgrammaticScrollLockedRef, + initialPlacementPending, }: { scrollRef: React.RefObject; isVisible: boolean; @@ -918,10 +1043,15 @@ function useCatchUpOnVisible({ hasUnreadDividerRef: React.RefObject; sessionIdRef: React.RefObject; isProgrammaticScrollLockedRef: React.RefObject<() => boolean>; + initialPlacementPending: boolean; }) { const { isVisibleRef, activationPendingRef } = useActivationPending(isVisible); useEffect(() => { if (!isVisible || !activationPendingRef.current) return; + if (initialPlacementPending) { + activationPendingRef.current = false; + return; + } return scheduleAfterPanelRestore(() => { if (!isVisibleRef.current) return; activationPendingRef.current = false; @@ -944,7 +1074,7 @@ function useCatchUpOnVisible({ scrollNativeToBottom(el); isNearBottomRef.current = true; }); - }, [isVisible, scrollRef]); + }, [initialPlacementPending, isVisible, scrollRef]); } /** @@ -1181,6 +1311,8 @@ type InitialScrollApplyParams = { isNearBottomRef: React.RefObject; envSwitchPlacementToken: number | null; hasUnreadDivider: boolean; + recoveryRevealKey: string | null; + recoveryPlacementPendingRef: React.RefObject; isProgrammaticScrollLocked: () => boolean; phase: "provisional" | "final"; }; @@ -1188,9 +1320,11 @@ type InitialScrollApplyParams = { function markInitialScrollConsumed( didInitialScroll: React.RefObject, activationPendingRef: React.RefObject, + recoveryPlacementPendingRef: React.RefObject, ): void { didInitialScroll.current = true; activationPendingRef.current = false; + recoveryPlacementPendingRef.current = false; } function completeEnvSwitchPlacement(envSwitchPlacementToken: number | null): void { @@ -1211,40 +1345,52 @@ function isCurrentEnvSwitchPlacement( return pending?.token === token && pending.sessionId === sessionId; } -type CompetingInitialScrollOwner = +export type CompetingInitialScrollOwner = | "layout-restore" | "explicit-target" + | "recovery" | "unread-divider" | "programmatic-scroll"; -function resolveCompetingInitialScrollOwner(params: { +export function resolveCompetingInitialScrollOwner(params: { hasPendingLayoutRestore: boolean; hasExplicitScrollTarget: boolean; + hasRecoveryReveal: boolean; hasUnreadDivider: boolean; isProgrammaticScrollLocked: () => boolean; }): CompetingInitialScrollOwner | null { if (params.hasPendingLayoutRestore) return "layout-restore"; if (params.hasExplicitScrollTarget) return "explicit-target"; + if (params.hasRecoveryReveal) return "recovery"; if (params.hasUnreadDivider) return "unread-divider"; if (params.isProgrammaticScrollLocked()) return "programmatic-scroll"; return null; } -function reportInitialPlacement( - phase: InitialScrollApplyParams["phase"], - element: HTMLDivElement, - sessionId: string | null, - token: number | null, +function resolveInitialPlacementOwner( enabled: boolean, -): void { + recoveryRevealKey: string | null, +): "recovery" | "bottom" | "saved-position" { + if (recoveryRevealKey !== null) return "recovery"; + return enabled ? "bottom" : "saved-position"; +} + +function reportInitialPlacement(params: { + phase: InitialScrollApplyParams["phase"]; + element: HTMLDivElement; + sessionId: string | null; + token: number | null; + enabled: boolean; + recoveryRevealKey: string | null; +}): void { if (!isDebug()) return; - placementDebug(`${phase} placement`, { - sessionId, - token, - owner: enabled ? "bottom" : "saved-position", - scrollTop: element.scrollTop, - scrollHeight: element.scrollHeight, - clientHeight: element.clientHeight, + placementDebug(`${params.phase} placement`, { + sessionId: params.sessionId, + token: params.token, + owner: resolveInitialPlacementOwner(params.enabled, params.recoveryRevealKey), + scrollTop: params.element.scrollTop, + scrollHeight: params.element.scrollHeight, + clientHeight: params.element.clientHeight, }); } @@ -1288,6 +1434,8 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { isNearBottomRef, envSwitchPlacementToken, hasUnreadDivider, + recoveryRevealKey, + recoveryPlacementPendingRef, isProgrammaticScrollLocked, phase, } = params; @@ -1307,17 +1455,18 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { const competingOwner = resolveCompetingInitialScrollOwner({ hasPendingLayoutRestore, hasExplicitScrollTarget, + hasRecoveryReveal: recoveryRevealKey !== null, hasUnreadDivider, isProgrammaticScrollLocked, }); - if (competingOwner) { + if (competingOwner && competingOwner !== "recovery") { if (phase === "provisional") return; placementDebug(`${phase} placement delegated`, { sessionId, owner: competingOwner, token: envSwitchPlacementToken, }); - markInitialScrollConsumed(didInitialScroll, activationPendingRef); + markInitialScrollConsumed(didInitialScroll, activationPendingRef, recoveryPlacementPendingRef); completeEnvSwitchPlacement(envSwitchPlacementToken); return; } @@ -1326,14 +1475,17 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { getStoredAutoScrollTop(sessionId) ?? undefined) : undefined; - const scrollTop = resolveNativeInitialScrollTop({ - enabled, - hasPendingLayoutRestore, - savedScrollTop, - scrollHeight: element.scrollHeight, - }); + const scrollTop = + competingOwner === "recovery" + ? 0 + : resolveNativeInitialScrollTop({ + enabled, + hasPendingLayoutRestore, + savedScrollTop, + scrollHeight: element.scrollHeight, + }); if (phase === "final") { - markInitialScrollConsumed(didInitialScroll, activationPendingRef); + markInitialScrollConsumed(didInitialScroll, activationPendingRef, recoveryPlacementPendingRef); } if (scrollTop === null) { completeFinalEnvSwitchPlacement(phase, envSwitchPlacementToken); @@ -1349,7 +1501,14 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { }; element.scrollTop = scrollTop; syncNearBottom(); - reportInitialPlacement(phase, element, sessionId, envSwitchPlacementToken, enabled); + reportInitialPlacement({ + phase, + element, + sessionId, + token: envSwitchPlacementToken, + enabled, + recoveryRevealKey, + }); finishAppliedInitialPlacement({ phase, enabled, @@ -1369,16 +1528,128 @@ function shouldSkipProvisionalPlacement( return phase === "provisional" && (hasUnreadDivider || appliedToken === currentToken); } -function useInitialPlacementLatches(envSwitchPlacementToken: number | null) { +function useInitialPlacementLatches( + envSwitchPlacementToken: number | null, + recoveryRevealKey: string | null, +) { const didInitialScroll = useRef(false); const lastTokenRef = useRef(null); const provisionalTokenRef = useRef(null); + const lastRecoveryRevealKeyRef = useRef(null); + const recoveryPlacementPendingRef = useRef(recoveryRevealKey !== null); if (envSwitchPlacementToken !== null && envSwitchPlacementToken !== lastTokenRef.current) { lastTokenRef.current = envSwitchPlacementToken; provisionalTokenRef.current = null; didInitialScroll.current = false; } - return { didInitialScroll, provisionalTokenRef }; + if (recoveryRevealKey !== null && recoveryRevealKey !== lastRecoveryRevealKeyRef.current) { + lastRecoveryRevealKeyRef.current = recoveryRevealKey; + provisionalTokenRef.current = null; + didInitialScroll.current = false; + recoveryPlacementPendingRef.current = true; + } + if (recoveryRevealKey === null) { + lastRecoveryRevealKeyRef.current = null; + recoveryPlacementPendingRef.current = false; + } + return { didInitialScroll, provisionalTokenRef, recoveryPlacementPendingRef }; +} + +type InitialPlacementLatches = ReturnType; + +type InitialScrollPositionParams = { + scrollRef: React.RefObject; + itemCount: number; + sessionId: string | null; + enabled: boolean; + hasUnreadDivider: boolean; + isNearBottomRef: React.RefObject; + isVisible: boolean; + historyRefreshPending: boolean; + envSwitchPlacementToken: number | null; + isRestoringLayout: boolean; + isProgrammaticScrollLocked: () => boolean; + recoveryRevealKey: string | null; + placementLatches: InitialPlacementLatches; +}; + +type InitialScrollPositionEffectParams = Omit & { + storeApi: ReturnType; + didInitialScroll: React.RefObject; + provisionalTokenRef: React.RefObject; + isVisibleRef: React.RefObject; + activationPendingRef: React.RefObject; + recoveryPlacementPendingRef: React.RefObject; +}; + +function runInitialScrollPositionEffect( + params: InitialScrollPositionEffectParams, +): void | (() => void) { + const { + scrollRef, + itemCount, + sessionId, + enabled, + hasUnreadDivider, + isNearBottomRef, + isVisible, + historyRefreshPending, + envSwitchPlacementToken, + isRestoringLayout, + isProgrammaticScrollLocked, + recoveryRevealKey, + storeApi, + didInitialScroll, + provisionalTokenRef, + isVisibleRef, + activationPendingRef, + recoveryPlacementPendingRef, + } = params; + if (!isVisible || (envSwitchPlacementToken !== null && isRestoringLayout)) return; + const hasEmptyTranscript = itemCount === 0 && recoveryRevealKey === null; + if (envSwitchPlacementToken !== null && hasEmptyTranscript) { + if (historyRefreshPending) return; + markInitialScrollConsumed(didInitialScroll, activationPendingRef, recoveryPlacementPendingRef); + completeEnvSwitchPlacement(envSwitchPlacementToken); + return; + } + if (didInitialScroll.current || hasEmptyTranscript) return; + const element = scrollRef.current; + if (!element) return; + + const phase = envSwitchPlacementToken !== null && historyRefreshPending ? "provisional" : "final"; + if ( + shouldSkipProvisionalPlacement( + phase, + hasUnreadDivider, + provisionalTokenRef.current, + envSwitchPlacementToken, + ) + ) { + return; + } + const applyInitialScroll = () => { + applyInitialScrollPosition({ + element, + sessionId, + enabled, + storeApi, + didInitialScroll, + activationPendingRef, + isVisibleRef, + isNearBottomRef, + envSwitchPlacementToken, + hasUnreadDivider, + recoveryRevealKey, + recoveryPlacementPendingRef, + isProgrammaticScrollLocked, + phase, + }); + if (phase === "provisional") provisionalTokenRef.current = envSwitchPlacementToken; + }; + + if (activationPendingRef.current) return scheduleAfterPanelRestore(applyInitialScroll); + applyInitialScroll(); } function useInitialScrollPosition({ @@ -1393,91 +1664,53 @@ function useInitialScrollPosition({ envSwitchPlacementToken, isRestoringLayout, isProgrammaticScrollLocked, -}: { - scrollRef: React.RefObject; - itemCount: number; - sessionId: string | null; - enabled: boolean; - hasUnreadDivider: boolean; - isNearBottomRef: React.RefObject; - isVisible: boolean; - historyRefreshPending: boolean; - envSwitchPlacementToken: number | null; - isRestoringLayout: boolean; - isProgrammaticScrollLocked: () => boolean; -}) { + recoveryRevealKey, + placementLatches, +}: InitialScrollPositionParams) { const storeApi = useAppStoreApi(); - const { didInitialScroll, provisionalTokenRef } = - useInitialPlacementLatches(envSwitchPlacementToken); + const { didInitialScroll, provisionalTokenRef, recoveryPlacementPendingRef } = placementLatches; const { isVisibleRef, activationPendingRef } = useActivationPending( isVisible, envSwitchPlacementToken, ); - useEffect(() => { - if (!isVisible) { - return; - } - if (envSwitchPlacementToken !== null && isRestoringLayout) return; - if (envSwitchPlacementToken !== null && itemCount === 0) { - if (historyRefreshPending) return; - markInitialScrollConsumed(didInitialScroll, activationPendingRef); - completeEnvSwitchPlacement(envSwitchPlacementToken); - return; - } - if (didInitialScroll.current || itemCount === 0) return; - const el = scrollRef.current; - if (!el) return; - - const phase = - envSwitchPlacementToken !== null && historyRefreshPending ? "provisional" : "final"; - if ( - shouldSkipProvisionalPlacement( - phase, - hasUnreadDivider, - provisionalTokenRef.current, - envSwitchPlacementToken, - ) - ) { - return; - } - const applyInitialScroll = () => { - applyInitialScrollPosition({ - element: el, + useEffect( + () => + runInitialScrollPositionEffect({ + scrollRef, + itemCount, sessionId, enabled, + hasUnreadDivider, + isNearBottomRef, + isVisible, + historyRefreshPending, + envSwitchPlacementToken, + isRestoringLayout, + isProgrammaticScrollLocked, + recoveryRevealKey, storeApi, didInitialScroll, - activationPendingRef, + provisionalTokenRef, isVisibleRef, - isNearBottomRef, - envSwitchPlacementToken, - hasUnreadDivider, - isProgrammaticScrollLocked, - phase, - }); - if (phase === "provisional") { - provisionalTokenRef.current = envSwitchPlacementToken; - } - }; - - if (activationPendingRef.current) { - return scheduleAfterPanelRestore(applyInitialScroll); - } - applyInitialScroll(); - }, [ - itemCount, - sessionId, - enabled, - hasUnreadDivider, - isNearBottomRef, - storeApi, - isVisible, - historyRefreshPending, - envSwitchPlacementToken, - isRestoringLayout, - isProgrammaticScrollLocked, - scrollRef, - ]); + activationPendingRef, + recoveryPlacementPendingRef, + }), + [ + itemCount, + sessionId, + enabled, + hasUnreadDivider, + isNearBottomRef, + storeApi, + isVisible, + historyRefreshPending, + envSwitchPlacementToken, + isRestoringLayout, + isProgrammaticScrollLocked, + recoveryRevealKey, + scrollRef, + ], + ); } type NativeScrollManagementParams = { @@ -1495,6 +1728,7 @@ type NativeScrollManagementParams = { isLoadingMore: boolean; loadMore: () => Promise; isVisible: boolean; + recoveryRevealKey?: string | null; }; /** @@ -1521,6 +1755,7 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) isLoadingMore, loadMore, isVisible, + recoveryRevealKey = null, } = params; const pendingChatInitialPlacement = useDockviewStore( (state) => state.pendingChatInitialPlacement, @@ -1528,6 +1763,9 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) const isRestoringLayout = useDockviewStore((state) => state.isRestoringLayout); const envSwitchPlacementToken = pendingChatInitialPlacement?.sessionId === sessionId ? pendingChatInitialPlacement.token : null; + const placementLatches = useInitialPlacementLatches(envSwitchPlacementToken, recoveryRevealKey); + const initialPlacementPending = + envSwitchPlacementToken !== null || placementLatches.recoveryPlacementPendingRef.current; const programmaticScrollLockRef = useRef(false); const isProgrammaticScrollLocked = useCallback(() => programmaticScrollLockRef.current, []); const { isNearBottomRef, resyncIsNearBottom, markNotNearBottom } = useAutoScroll({ @@ -1539,7 +1777,7 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) hasUnreadDivider, isProgrammaticScrollLocked, isVisible, - initialPlacementPending: envSwitchPlacementToken !== null, + initialPlacementPending, }); const runGuardedScroll = useProgrammaticScrollGuard( scrollRef, @@ -1552,6 +1790,7 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) items, isLoadingMore, isProgrammaticScrollLocked, + recoveryRevealKey, ); const loadMoreWithPrependBaseline = useCallback(() => { beginOlderLoad(); @@ -1586,6 +1825,8 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) envSwitchPlacementToken, isRestoringLayout, isProgrammaticScrollLocked, + recoveryRevealKey, + placementLatches, }); return { diff --git a/apps/web/components/task/chat/message-list-native.test.tsx b/apps/web/components/task/chat/message-list-native.test.tsx index f8e4c89752..22668b1fd7 100644 --- a/apps/web/components/task/chat/message-list-native.test.tsx +++ b/apps/web/components/task/chat/message-list-native.test.tsx @@ -86,6 +86,11 @@ const HARNESS_RENDER_ERROR = "harness did not render"; const NATIVE_SCROLL_MANAGEMENT_TEST_ID = "native-scroll-management-container"; const AUTO_SCROLL_CONTAINER_TEST_ID = "auto-scroll-container"; const CACHED_MESSAGE_ID = "cached-message"; +const RECOVERY_SESSION_ID = "session-recovery"; +const RECOVERY_MESSAGE_ID = "recovery-message"; +const OLDEST_ACTIVITY_ID = "activity-old"; +const NEWEST_MESSAGE_ID = "newest"; +const OLDER_MESSAGE_ID = "older"; const TEST_MESSAGES = [{} as Message]; /** Always returns false: the harness never locks programmatic scrolling. */ const NEVER_LOCKED = () => false; @@ -109,6 +114,7 @@ function Harness({ enabled = true, sessionId = null, isProgrammaticScrollLocked = NEVER_LOCKED, + recoveryRevealKey = null, }: { itemCount: number; anchoredBarOffsetPx: number; @@ -121,6 +127,7 @@ function Harness({ enabled?: boolean; sessionId?: string | null; isProgrammaticScrollLocked?: () => boolean; + recoveryRevealKey?: string | null; }) { const scrollRef = useRef(null); useLayoutEffect(() => { @@ -148,6 +155,7 @@ function Harness({ sessionId, isProgrammaticScrollLocked, isVisible, + recoveryRevealKey, } as Parameters[4]; useScrollToDividerOrBottom(scrollRef, itemCount, dividerKey, anchoredBarOffsetPx, scrollOptions); return ( @@ -271,6 +279,7 @@ function useNativeScrollMetrics( function NativeScrollManagementHarness({ items, + messages = [], metrics, loadMore = async () => 0, sessionId = null, @@ -280,8 +289,11 @@ function NativeScrollManagementHarness({ enabled = false, historyRefreshPending = false, hasUnreadDivider = false, + hasMore = true, + recoveryRevealKey = null, }: { items: RenderItem[]; + messages?: Message[]; metrics?: NativeScrollMetrics; loadMore?: () => Promise; sessionId?: string | null; @@ -291,23 +303,26 @@ function NativeScrollManagementHarness({ enabled?: boolean; historyRefreshPending?: boolean; hasUnreadDivider?: boolean; + hasMore?: boolean; + recoveryRevealKey?: string | null; }) { const scrollRef = useRef(null); useNativeScrollMetrics(scrollRef, metrics); const { sentinelRef, showRecovery } = useNativeScrollManagement({ scrollRef, items, - messages: [], + messages, isWorking: false, sessionId, enabled, hasUnreadDivider, messagesLoading: false, - hasMore: true, + hasMore, isLoadingMore, loadMore, isVisible, historyRefreshPending, + recoveryRevealKey, }); if (recoveryRef) recoveryRef.current = showRecovery; return ( @@ -356,6 +371,301 @@ describe("isElementInPreloadRegion", () => { // eslint-disable-next-line max-lines-per-function -- pagination invariants share one fixture and lifecycle. describe("useNativeScrollManagement transcript pagination", () => { + it("reveals recovery at the top once per error stamp and preserves same-stamp scrolling", () => { + const metrics = { scrollHeight: 1000, scrollTop: 240, clientHeight: 400 }; + const { rerender } = render( + , + ); + + expect(metrics.scrollTop).toBe(0); + + metrics.scrollTop = 240; + rerender( + , + ); + expect(metrics.scrollTop).toBe(240); + + rerender( + , + ); + expect(metrics.scrollTop).toBe(240); + + rerender( + , + ); + expect(metrics.scrollTop).toBe(0); + + metrics.scrollTop = 240; + rerender( + , + ); + expect(metrics.scrollTop).toBe(0); + }); + + // eslint-disable-next-line max-lines-per-function -- recovery ownership and the subsequent prepend handoff share one controlled lifecycle. + it("keeps recovery visible while an older page settles, then resumes normal anchoring after user scroll", async () => { + const page = Promise.withResolvers(); + const metrics = { scrollHeight: 100, scrollTop: 0, clientHeight: 50 }; + const activity = transcriptActivity(OLDEST_ACTIVITY_ID, "turn-1"); + const newest = transcriptMessage(NEWEST_MESSAGE_ID); + const { rerender } = render( + page.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + const scroller = screen.getByTestId(NATIVE_SCROLL_MANAGEMENT_TEST_ID); + const wrappedLoadMore = sharedSentinelCalls.at(-1)?.[4] as () => Promise; + + const pendingLoad = wrappedLoadMore(); + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + metrics.scrollHeight = 200; + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + + expect(metrics.scrollTop).toBe(0); + + page.resolve(1); + await act(async () => { + await pendingLoad; + }); + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + expect(metrics.scrollTop).toBe(0); + + const nextPage = Promise.withResolvers(); + const nextLoadMore = sharedSentinelCalls.at(-1)?.[4] as () => Promise; + const pendingNextLoad = nextLoadMore(); + rerender( + nextPage.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + act(() => { + scroller.dispatchEvent(new WheelEvent("wheel", { deltaY: 100 })); + metrics.scrollTop = 40; + scroller.dispatchEvent(new Event("scroll")); + }); + expect(metrics.scrollTop).toBe(40); + + metrics.scrollHeight = 300; + rerender( + nextPage.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + expect(metrics.scrollTop).toBe(140); + + nextPage.resolve(1); + await act(async () => { + await pendingNextLoad; + }); + rerender( + nextPage.promise} + recoveryRevealKey="session-recovery:failure-1" + />, + ); + expect(metrics.scrollTop).toBe(140); + }); + + it("keeps an already-loading older page below a newly revealed recovery card", async () => { + const page = Promise.withResolvers(); + const metrics = { scrollHeight: 100, scrollTop: 0, clientHeight: 50 }; + const activity = transcriptActivity(OLDEST_ACTIVITY_ID, "turn-1"); + const newest = transcriptMessage(NEWEST_MESSAGE_ID); + const { rerender } = render( + page.promise} + />, + ); + const wrappedLoadMore = sharedSentinelCalls.at(-1)?.[4] as () => Promise; + const pendingLoad = wrappedLoadMore(); + + rerender( + page.promise} + />, + ); + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-2" + />, + ); + expect(metrics.scrollTop).toBe(0); + + metrics.scrollHeight = 200; + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-2" + />, + ); + expect(metrics.scrollTop).toBe(0); + + page.resolve(1); + await act(async () => { + await pendingLoad; + }); + rerender( + page.promise} + recoveryRevealKey="session-recovery:failure-2" + />, + ); + expect(metrics.scrollTop).toBe(0); + }); + + it("resumes normal auto-scroll after the initial recovery placement is consumed", () => { + const metrics = { scrollHeight: 100, scrollTop: 0, clientHeight: 50 }; + const firstMessage = { id: "first-message" } as Message; + const scrollerMessages = [firstMessage]; + const activity = transcriptActivity(OLDEST_ACTIVITY_ID, "turn-1"); + const newest = transcriptMessage(NEWEST_MESSAGE_ID); + const { rerender } = render( + , + ); + const scroller = screen.getByTestId(NATIVE_SCROLL_MANAGEMENT_TEST_ID); + + metrics.scrollTop = 50; + act(() => { + scroller.dispatchEvent(new WheelEvent("wheel", { deltaY: 100 })); + scroller.dispatchEvent(new Event("scroll")); + }); + metrics.scrollHeight = 150; + rerender( + , + ); + + expect(metrics.scrollTop).toBe(2_147_483_647); + }); + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.11 // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.14 it("places cached enabled history before refresh and reconciles after it settles", () => { @@ -1041,6 +1351,20 @@ describe("useNativeScrollManagement transcript pagination", () => { // eslint-disable-next-line max-lines-per-function -- this suite keeps the related scroll invariants together. describe("useScrollToDividerOrBottom — anchored-bar offset", () => { + it("leaves an active recovery transcript at the top for its first reveal", () => { + render( + , + ); + + expect(screen.getByTestId(DIVIDER_SCROLL_CONTAINER_TEST_ID).scrollTop).toBe(0); + }); + it("waits for an inactive transcript to become visible before placing the initial view", () => { const frames: Array = []; vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { diff --git a/apps/web/components/task/chat/message-list-native.tsx b/apps/web/components/task/chat/message-list-native.tsx index 55b932b77d..fce6222c59 100644 --- a/apps/web/components/task/chat/message-list-native.tsx +++ b/apps/web/components/task/chat/message-list-native.tsx @@ -2,7 +2,15 @@ /* eslint-disable max-lines -- native transcript composition owns scrolling. */ -import { useEffect, useMemo, useRef, memo, forwardRef, useImperativeHandle } from "react"; +import { + useEffect, + useMemo, + useRef, + memo, + forwardRef, + useImperativeHandle, + type ReactNode, +} from "react"; import { SessionPanelContent } from "@kandev/ui/pannel-session"; import type { Message, TaskSessionState } from "@/lib/types/http"; import type { RenderItem } from "@/hooks/use-processed-messages"; @@ -102,6 +110,7 @@ type NativeMessageListScrollParams = { /** Changes when transcript status rows can add/remove space above messages. */ scrollLayoutKey: string; isVisible: boolean; + recoveryRevealKey?: string | null; }; type ScrollToDividerOptions = { @@ -112,8 +121,22 @@ type ScrollToDividerOptions = { isProgrammaticScrollLocked?: () => boolean; isVisible?: boolean; historyRefreshPending?: boolean; + recoveryRevealKey?: string | null; }; +function consumeRecoveryRevealForDivider( + recoveryRevealKey: string | null, + didInitialScroll: React.RefObject, + activationPendingRef: React.RefObject, +): boolean { + if (recoveryRevealKey === null) return false; + // The native initial-position hook owns the recovery reveal. Mark this + // hook consumed so its normal bottom/divider placement cannot override it. + didInitialScroll.current = true; + activationPendingRef.current = false; + return true; +} + function useNativeMessageListScroll(params: NativeMessageListScrollParams) { const { scrollRef, @@ -136,6 +159,7 @@ function useNativeMessageListScroll(params: NativeMessageListScrollParams) { onFirstMessageHiddenChange, scrollLayoutKey, isVisible, + recoveryRevealKey, } = params; const { handleScrollToMessage, @@ -158,6 +182,7 @@ function useNativeMessageListScroll(params: NativeMessageListScrollParams) { isLoadingMore, loadMore, isVisible, + recoveryRevealKey, }); const anchoredBarOffsetPx = anchoredBarScrollOffsetPx(anchoredBarHeight); useEffect(() => { @@ -171,6 +196,7 @@ function useNativeMessageListScroll(params: NativeMessageListScrollParams) { isProgrammaticScrollLocked, isVisible, historyRefreshPending, + recoveryRevealKey, }); useImperativeHandle(ref, () => ({ scrollToMessage: handleScrollToMessage }), [ handleScrollToMessage, @@ -278,6 +304,7 @@ type NativeMessageListBodyProps = { onScrollToMessage: (messageId: string, options?: { align?: "start" | "center" }) => void; autoScrollEnabled: boolean; dividerBeforeItemKey?: string | null; + prependContent?: ReactNode; launchErrorOwned: boolean; launchErrorStamp?: string; launchErrorOccurredAt?: string; @@ -319,6 +346,7 @@ type NativeMessageListBodyProps = { * keep their existing visible-host behavior without participating in read * tracking. */ +// eslint-disable-next-line max-lines-per-function -- divider and recovery placement share one scroll lifecycle. export function useScrollToDividerOrBottom( scrollRef: React.RefObject, itemCount: number, @@ -334,6 +362,7 @@ export function useScrollToDividerOrBottom( isProgrammaticScrollLocked = () => false, isVisible = true, historyRefreshPending = false, + recoveryRevealKey = null, } = options; const { isVisibleRef, activationPendingRef } = useActivationPending(isVisible); const isUserScrollingRef = useDividerUserScrolling(scrollRef); @@ -363,7 +392,10 @@ export function useScrollToDividerOrBottom( settlingDeadlineRef.current = Date.now() + DIVIDER_SETTLING_WINDOW_MS; } const el = scrollRef.current; - if (!el || itemCount === 0 || historyRefreshPending) return; + if (!el || historyRefreshPending) return; + if (consumeRecoveryRevealForDivider(recoveryRevealKey, didInitialScroll, activationPendingRef)) + return; + if (itemCount === 0) return; const placeInitialPosition = () => { if (!isVisibleRef.current) return; @@ -432,6 +464,7 @@ export function useScrollToDividerOrBottom( isProgrammaticScrollLocked, isVisible, historyRefreshPending, + recoveryRevealKey, scrollRef, ]); } @@ -487,12 +520,14 @@ function NativeMessageListBody({ onScrollToMessage, autoScrollEnabled, dividerBeforeItemKey, + prependContent, launchErrorOwned, launchErrorStamp, launchErrorOccurredAt, }: NativeMessageListBodyProps) { return (
+ {prependContent ?
{prependContent}
: null} {/* Sentinel for lazy loading older messages */} {hasMore &&
} @@ -588,6 +623,8 @@ export const NativeMessageList = memo( launchErrorOwned = false, launchErrorStamp, launchErrorOccurredAt, + prependContent, + recoveryRevealKey, }: MessageListProps, ref, ) { @@ -662,6 +699,7 @@ export const NativeMessageList = memo( isWorking, ].join(":"), isVisible, + recoveryRevealKey, }); return ( @@ -701,6 +739,7 @@ export const NativeMessageList = memo( onScrollToMessage={handleScrollToMessage} autoScrollEnabled={autoScrollEnabled} dividerBeforeItemKey={dividerBeforeItemKey} + prependContent={prependContent} launchErrorOwned={launchErrorOwned} launchErrorStamp={launchErrorStamp} launchErrorOccurredAt={launchErrorOccurredAt} diff --git a/apps/web/components/task/chat/message-list-shared.tsx b/apps/web/components/task/chat/message-list-shared.tsx index c45807962a..40ce20c167 100644 --- a/apps/web/components/task/chat/message-list-shared.tsx +++ b/apps/web/components/task/chat/message-list-shared.tsx @@ -64,6 +64,10 @@ export type MessageListProps = { * top — the desktop-only, opt-in anchored last-prompt bar. `null`/`undefined` * when the setting is off or on mobile. */ stickyPromptBar?: ReactNode; + /** Rendered above transcript status and messages inside the native viewport. */ + prependContent?: ReactNode; + /** Identity of the active recovery content for one-time initial reveal. */ + recoveryRevealKey?: string | null; /** Current rendered height (px) of the anchored last-prompt bar's pinned * overlay, or 0/undefined when it isn't showing. Lets a target scrolled * to the top of the transcript (e.g. the unread "New" divider) reserve diff --git a/apps/web/components/task/task-chat-panel.launch-error.test.tsx b/apps/web/components/task/task-chat-panel.launch-error.test.tsx index af1f609ad5..76fcd1591c 100644 --- a/apps/web/components/task/task-chat-panel.launch-error.test.tsx +++ b/apps/web/components/task/task-chat-panel.launch-error.test.tsx @@ -10,6 +10,8 @@ const launchError = { category: "workspace_checkout_failed", }; +const messageListRecoveryRevealKeys = vi.hoisted(() => [] as Array); + const priorTranscriptMessage = { id: "prior-transcript", author_type: "agent", @@ -35,7 +37,11 @@ const appStoreState = { const panelState = { resolvedSessionId: "prior-session", - session: { state: "FAILED", error_message: "The prior session stopped." }, + session: { + state: "FAILED", + error_message: "The prior session stopped.", + metadata: null as Record | null, + }, taskId: "task-1", isWorking: false, messagesLoading: false, @@ -114,15 +120,23 @@ vi.mock("@/components/task/chat/message-list", () => ({ MessageList: ({ messages, launchErrorOwned, + prependContent, + recoveryRevealKey, }: { messages: Message[]; launchErrorOwned?: boolean; - }) => ( -
- {!launchErrorOwned && - messages.map((message) =>
{message.content}
)} -
- ), + prependContent?: ReactNode; + recoveryRevealKey?: string | null; + }) => { + messageListRecoveryRevealKeys.push(recoveryRevealKey); + return ( +
+ {prependContent} + {!launchErrorOwned && + messages.map((message) =>
{message.content}
)} +
+ ); + }, })); vi.mock("./simple/components/task-chat-launch-error", () => ({ @@ -198,7 +212,10 @@ vi.mock("react-i18next", () => ({ import { TaskChatPanel } from "./task-chat-panel"; -afterEach(() => cleanup()); +afterEach(() => { + cleanup(); + messageListRecoveryRevealKeys.length = 0; +}); describe("TaskChatPanel launch-error ownership", () => { it("keeps a task-wide card and prior failed-session surfaces together", () => { @@ -209,5 +226,26 @@ describe("TaskChatPanel launch-error ownership", () => { ); expect(screen.getByTestId("session-stopped-banner")).toBeTruthy(); expect(screen.getByText(priorTranscriptMessage.content)).toBeTruthy(); + expect(messageListRecoveryRevealKeys.at(-1)).toBe("prior-session:task-wide-launch-error"); + }); + + it("reveals a persisted bootstrap fallback for the selected session", () => { + const previousMetadata = panelState.session.metadata; + panelState.session.metadata = { + last_agent_error: { + message: "The selected session could not start.", + phase: "bootstrap", + occurred_at: "2026-08-20T11:00:00Z", + stamp: "persisted-bootstrap-error", + }, + }; + + try { + render(); + + expect(messageListRecoveryRevealKeys.at(-1)).toBe("prior-session:persisted-bootstrap-error"); + } finally { + panelState.session.metadata = previousMetadata; + } }); }); diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index 351253bec6..ca7988f60a 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -56,8 +56,12 @@ import { TaskChatLaunchError } from "./simple/components/task-chat-launch-error" import { isTypedTaskLaunchError } from "./simple/components/task-launch-error-entry"; import { useTaskLaunchErrorContext } from "./task-launch-error-context"; import { useTaskStatusSummary } from "@/hooks/domains/task/use-task-status-summary"; -import { isTaskLaunchErrorOwnedBySession } from "@/components/task/chat/types"; +import { + isTaskLaunchErrorOwnedBySession, + isTaskLaunchErrorVisibleForSession, +} from "@/components/task/chat/types"; import { TaskMarkdownFileLinkProvider } from "@/components/shared/task-markdown-file-link-provider"; +import { selectSessionRecoveryError } from "@/lib/session-recovery-presentation"; /** Returns a `clarificationKey` that increments each time a pending * clarification is resolved, letting the composer reset its input state for @@ -1031,6 +1035,17 @@ export const TaskChatPanel = memo(function TaskChatPanel({ pendingClarificationGroup, } = panelState; const activeLaunchError = launchStatusSummary?.active_error; + const selectedSessionRecoveryError = selectSessionRecoveryError( + activeLaunchError, + resolvedSessionId, + session?.metadata, + ); + const visibleRecoveryError = + selectedSessionRecoveryError ?? + (isTypedTaskLaunchError(activeLaunchError) && + isTaskLaunchErrorVisibleForSession(activeLaunchError, resolvedSessionId) + ? activeLaunchError + : null); const launchErrorOwned = Boolean( isTypedTaskLaunchError(activeLaunchError) && isTaskLaunchErrorOwnedBySession(activeLaunchError, resolvedSessionId), @@ -1159,6 +1174,19 @@ export const TaskChatPanel = memo(function TaskChatPanel({ (e: React.MouseEvent) => routePanelMouseDown(e, panelRef), [], ); + const launchErrorContent = launchErrorContext ? ( + + ) : null; + const recoveryRevealKey = visibleRecoveryError + ? `${resolvedSessionId ?? ""}:${visibleRecoveryError.stamp || visibleRecoveryError.occurred_at}` + : null; return ( - - {launchErrorContext && ( - - )} + { + return transcript.evaluate((node) => { + const element = node as HTMLElement; + let current: HTMLElement | null = element; + let scrollOwnerCount = 0; + while (current) { + const overflowY = getComputedStyle(current).overflowY; + if (overflowY === "auto" || overflowY === "scroll") scrollOwnerCount += 1; + if (current.dataset.testid === "session-chat") break; + current = current.parentElement; + } + return { + scrollTop: element.scrollTop, + scrollHeight: element.scrollHeight, + clientHeight: element.clientHeight, + scrollOwnerCount, + }; + }); +} + +/** Applies a user-like transcript scroll and returns the resulting offset. */ +export async function setTranscriptScrollTop( + transcript: Locator, + scrollTop: number, +): Promise { + return transcript.evaluate((node, nextScrollTop) => { + const element = node as HTMLElement; + element.scrollTop = nextScrollTop; + element.dispatchEvent(new Event("scroll", { bubbles: true })); + return element.scrollTop; + }, scrollTop); +} diff --git a/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts b/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts index 3eb8f6d151..4d89aa6794 100644 --- a/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts +++ b/apps/web/e2e/tests/task/launch-failure-recovery.spec.ts @@ -8,6 +8,7 @@ import { import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; import type { ApiClient } from "../../helpers/api-client"; import { waitForSessionDone } from "../../helpers/session"; +import { readTranscriptScrollState, setTranscriptScrollTop } from "../../helpers/transcript-scroll"; import { SessionPage } from "../../pages/session-page"; type LaunchError = { @@ -378,8 +379,13 @@ test.describe("task launch failure recovery", () => { task.session_id, "Waiting for bootstrap recovery fixture to settle", ); + await apiClient.seedAgentMessages(task.session_id, 40, "bootstrap history"); const occurredAt = new Date(Date.now() + 60 * 60 * 1000).toISOString(); + const longDetails = Array.from( + { length: 12 }, + (_, index) => `agent_bootstrap; diagnostic_line=${index + 1}; cause=permission_denied`, + ).join("\n"); await apiClient.seedTaskSession(task.id, { state: "WAITING_FOR_INPUT", sessionId: task.session_id, @@ -393,7 +399,7 @@ test.describe("task launch failure recovery", () => { phase: "bootstrap", attempt_id: "bootstrap-execution-e2e", code: "generic_launch_failure", - details: "agent_bootstrap; cause=permission_denied", + details: longDetails, stamp: "bootstrap-presentation-e2e", causes: [ { @@ -416,12 +422,72 @@ test.describe("task launch failure recovery", () => { await expect(card).toContainText("Session startup needs attention"); await expect(card).toContainText("Required contribution access was denied."); await expect(card).not.toContainText("remote-secret"); + const transcript = session.activeChat().locator(".chat-message-list").first(); + await expect + .poll(async () => (await readTranscriptScrollState(transcript)).scrollOwnerCount) + .toBe(1); + const initialScroll = await readTranscriptScrollState(transcript); + expect(initialScroll.scrollHeight).toBeGreaterThan(initialScroll.clientHeight); + expect(initialScroll.scrollTop).toBe(0); + await expect(session.activeChat().getByTestId("chat-input-area")).toBeVisible(); const details = card.getByTestId("session-bootstrap-recovery-details"); await expect(details).not.toHaveAttribute("open"); await details.getByText("Recovery details").click(); await expect(details).toHaveAttribute("open", ""); await expect(card).toContainText("The required contribution access was denied."); + const expandedScroll = await readTranscriptScrollState(transcript); + expect(expandedScroll.scrollOwnerCount).toBe(1); + expect(expandedScroll.scrollHeight).toBeGreaterThan(expandedScroll.clientHeight); + const middleScrollTop = await setTranscriptScrollTop( + transcript, + Math.floor((expandedScroll.scrollHeight - expandedScroll.clientHeight) / 2), + ); + expect(middleScrollTop).toBeGreaterThan(0); + + const sameStampMetadata = { + last_agent_error: { + message: "The agent could not start.", + occurred_at: occurredAt, + agent_execution_id: "bootstrap-execution-e2e", + execution_id: "bootstrap-execution-e2e", + phase: "bootstrap", + attempt_id: "bootstrap-execution-e2e", + code: "generic_launch_failure", + details: longDetails, + stamp: "bootstrap-presentation-e2e", + causes: [ + { + operation: "resume", + code: "permission_denied", + detail: "The required contribution access was denied.", + }, + ], + }, + }; + await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + sessionId: task.session_id, + agentProfileId: seedData.agentProfileId, + metadata: sameStampMetadata, + }); + await expect + .poll(async () => (await readTranscriptScrollState(transcript)).scrollTop) + .toBeGreaterThanOrEqual(Math.max(0, middleScrollTop - 2)); + + await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + sessionId: task.session_id, + agentProfileId: seedData.agentProfileId, + metadata: { + ...sameStampMetadata, + last_agent_error: { + ...sameStampMetadata.last_agent_error, + stamp: "bootstrap-presentation-e2e-new", + }, + }, + }); + await expect.poll(async () => (await readTranscriptScrollState(transcript)).scrollTop).toBe(0); for (const testId of [ "recovery-resume-button", "recovery-restore-workspace-button", diff --git a/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts b/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts index 9b57d7cca6..a9f9230a97 100644 --- a/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts +++ b/apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts @@ -10,6 +10,7 @@ import { import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; import type { ApiClient } from "../../helpers/api-client"; import { waitForSessionDone } from "../../helpers/session"; +import { readTranscriptScrollState, setTranscriptScrollTop } from "../../helpers/transcript-scroll"; import { SessionPage } from "../../pages/session-page"; async function waitForLaunchError(apiClient: ApiClient, workspaceId: string, taskId: string) { @@ -261,7 +262,13 @@ test.describe("mobile task launch failure recovery", () => { task.session_id, "Waiting for mobile bootstrap recovery fixture to settle", ); + await apiClient.seedAgentMessages(task.session_id, 40, "mobile bootstrap history"); + const occurredAt = new Date(Date.now() + 60 * 60 * 1000).toISOString(); + const longDetails = Array.from( + { length: 12 }, + (_, index) => `agent_bootstrap; diagnostic_line=${index + 1}; cause=permission_denied`, + ).join("\n"); await apiClient.seedTaskSession(task.id, { state: "WAITING_FOR_INPUT", sessionId: task.session_id, @@ -269,13 +276,13 @@ test.describe("mobile task launch failure recovery", () => { metadata: { last_agent_error: { message: "The agent could not start.", - occurred_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + occurred_at: occurredAt, agent_execution_id: "mobile-bootstrap-execution-e2e", execution_id: "mobile-bootstrap-execution-e2e", phase: "bootstrap", attempt_id: "mobile-bootstrap-execution-e2e", code: "generic_launch_failure", - details: "agent_bootstrap; cause=permission_denied", + details: longDetails, stamp: "mobile-bootstrap-presentation-e2e", causes: [ { @@ -295,6 +302,14 @@ test.describe("mobile task launch failure recovery", () => { await expect(card).toHaveCount(1, { timeout: 30_000 }); await expect(testPage.getByTestId("task-launch-error-entry")).toHaveCount(0); await expect(testPage.getByTestId("session-recovery-error")).toHaveCount(0); + const transcript = session.activeChat().locator(".chat-message-list").first(); + await expect + .poll(async () => (await readTranscriptScrollState(transcript)).scrollOwnerCount) + .toBe(1); + const initialScroll = await readTranscriptScrollState(transcript); + expect(initialScroll.scrollHeight).toBeGreaterThan(initialScroll.clientHeight); + expect(initialScroll.scrollTop).toBe(0); + await expect(session.activeChat().getByTestId("chat-input-area")).toBeVisible(); for (const testId of [ "recovery-resume-button", @@ -314,6 +329,58 @@ test.describe("mobile task launch failure recovery", () => { await details.getByText("Recovery details").tap(); await expect(details).toHaveAttribute("open", ""); await expect(card).toContainText("The required contribution access was denied."); + const expandedScroll = await readTranscriptScrollState(transcript); + expect(expandedScroll.scrollOwnerCount).toBe(1); + expect(expandedScroll.scrollHeight).toBeGreaterThan(expandedScroll.clientHeight); + const middleScrollTop = await setTranscriptScrollTop( + transcript, + Math.floor((expandedScroll.scrollHeight - expandedScroll.clientHeight) / 2), + ); + expect(middleScrollTop).toBeGreaterThan(0); + + const sameStampMetadata = { + last_agent_error: { + message: "The agent could not start.", + occurred_at: occurredAt, + agent_execution_id: "mobile-bootstrap-execution-e2e", + execution_id: "mobile-bootstrap-execution-e2e", + phase: "bootstrap", + attempt_id: "mobile-bootstrap-execution-e2e", + code: "generic_launch_failure", + details: longDetails, + stamp: "mobile-bootstrap-presentation-e2e", + causes: [ + { + operation: "resume", + code: "permission_denied", + detail: "The required contribution access was denied.", + }, + ], + }, + }; + await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + sessionId: task.session_id, + agentProfileId: seedData.agentProfileId, + metadata: sameStampMetadata, + }); + await expect + .poll(async () => (await readTranscriptScrollState(transcript)).scrollTop) + .toBeGreaterThanOrEqual(Math.max(0, middleScrollTop - 2)); + + await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + sessionId: task.session_id, + agentProfileId: seedData.agentProfileId, + metadata: { + ...sameStampMetadata, + last_agent_error: { + ...sameStampMetadata.last_agent_error, + stamp: "mobile-bootstrap-presentation-e2e-new", + }, + }, + }); + await expect.poll(async () => (await readTranscriptScrollState(transcript)).scrollTop).toBe(0); await assertNoDocumentHorizontalOverflow(testPage, "mobile bootstrap recovery presentation"); await testPage.screenshot({ diff --git a/apps/web/hooks/domains/session/use-session-resumption.test.ts b/apps/web/hooks/domains/session/use-session-resumption.test.ts index bee21c34ca..96598d1a7d 100644 --- a/apps/web/hooks/domains/session/use-session-resumption.test.ts +++ b/apps/web/hooks/domains/session/use-session-resumption.test.ts @@ -565,6 +565,7 @@ describe("useSessionResumption", () => { kind: "timeout", statusError: "WebSocket request timed out: task.session.status", }); + expect(result.current.recoveryAttemptId).toBe(1); expect(mockRequest).toHaveBeenCalledTimes(2); mockRequest.mockResolvedValueOnce({ @@ -587,6 +588,7 @@ describe("useSessionResumption", () => { ); expect(result.current.recoveryFailure).toBeNull(); expect(result.current.resumptionState).toBe("running"); + expect(result.current.recoveryAttemptId).toBe(2); }); it("keeps a workspace restore failure as launch feedback with a launch retry", async () => { diff --git a/apps/web/hooks/domains/session/use-session-resumption.ts b/apps/web/hooks/domains/session/use-session-resumption.ts index 984deb9e43..65ddcc4c9e 100644 --- a/apps/web/hooks/domains/session/use-session-resumption.ts +++ b/apps/web/hooks/domains/session/use-session-resumption.ts @@ -422,6 +422,7 @@ interface UseSessionResumptionReturn { error: string | null; notice: string | null; recoveryFailure: SessionRecoveryFailure | null; + recoveryAttemptId: number; taskSessionState: TaskSessionState | null; worktreePath: string | null; worktreeBranch: string | null; @@ -724,7 +725,21 @@ export function useSessionResumption( taskArchiveState: TaskArchiveState = false, options: SessionResumptionOptions = {}, ): UseSessionResumptionReturn { - const [resumptionState, setResumptionState] = useState("idle"); + const [resumptionState, setResumptionStateRaw] = useState("idle"); + const recoveryAttemptIdRef = useRef(0); + const recoveryAttemptActiveRef = useRef(false); + const setResumptionState = useCallback((nextState: ResumptionState) => { + const startsRecoveryAttempt = nextState === "checking" || nextState === "resuming"; + if (startsRecoveryAttempt) { + if (!recoveryAttemptActiveRef.current) { + recoveryAttemptIdRef.current += 1; + recoveryAttemptActiveRef.current = true; + } + } else { + recoveryAttemptActiveRef.current = false; + } + setResumptionStateRaw(nextState); + }, []); const [error, setError] = useState(null); const [notice, setNotice] = useState(null); const [recoveryFailure, setRecoveryFailure] = useState(null); @@ -787,6 +802,7 @@ export function useSessionResumption( error, notice, recoveryFailure, + recoveryAttemptId: recoveryAttemptIdRef.current, taskSessionState: session?.state ?? null, worktreePath, worktreeBranch, diff --git a/docs/plans/startup-recovery-scroll-timeout/plan.md b/docs/plans/startup-recovery-scroll-timeout/plan.md new file mode 100644 index 0000000000..83c9b90053 --- /dev/null +++ b/docs/plans/startup-recovery-scroll-timeout/plan.md @@ -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. diff --git a/docs/plans/startup-recovery-scroll-timeout/task-01-preflight-budget.md b/docs/plans/startup-recovery-scroll-timeout/task-01-preflight-budget.md new file mode 100644 index 0000000000..7195bc7ade --- /dev/null +++ b/docs/plans/startup-recovery-scroll-timeout/task-01-preflight-budget.md @@ -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`. diff --git a/docs/plans/startup-recovery-scroll-timeout/task-02-chat-scroll.md b/docs/plans/startup-recovery-scroll-timeout/task-02-chat-scroll.md new file mode 100644 index 0000000000..634b3157c5 --- /dev/null +++ b/docs/plans/startup-recovery-scroll-timeout/task-02-chat-scroll.md @@ -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. diff --git a/docs/specs/agents/requirements/session-recovery-failures.md b/docs/specs/agents/requirements/session-recovery-failures.md index 8f1d6c7857..220342c5a6 100644 --- a/docs/specs/agents/requirements/session-recovery-failures.md +++ b/docs/specs/agents/requirements/session-recovery-failures.md @@ -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. diff --git a/docs/specs/agents/system-design/session-recovery-failures.md b/docs/specs/agents/system-design/session-recovery-failures.md index 4114825e3c..70ec77d44c 100644 --- a/docs/specs/agents/system-design/session-recovery-failures.md +++ b/docs/specs/agents/system-design/session-recovery-failures.md @@ -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 diff --git a/docs/specs/tasks/requirements/remote-contribution-tasks.md b/docs/specs/tasks/requirements/remote-contribution-tasks.md index d2e8608d68..453dd82a12 100644 --- a/docs/specs/tasks/requirements/remote-contribution-tasks.md +++ b/docs/specs/tasks/requirements/remote-contribution-tasks.md @@ -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 diff --git a/docs/specs/tasks/system-design/remote-contribution-tasks.md b/docs/specs/tasks/system-design/remote-contribution-tasks.md index 3e5bd25adc..ee872dc201 100644 --- a/docs/specs/tasks/system-design/remote-contribution-tasks.md +++ b/docs/specs/tasks/system-design/remote-contribution-tasks.md @@ -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