fix: block workflow proceed during clarification (#3574)
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildSubmitMessage } from "./chat-input-area";
|
||||
import { resolveStatusRowTaskId, shouldRenderChatStatusBar } from "./chat-status-bar";
|
||||
import { hasPendingClarification, shouldShowProceed } from "./types";
|
||||
import type { AgentMessageComment } from "@/lib/state/slices/comments";
|
||||
|
||||
const messageComment: AgentMessageComment = {
|
||||
@@ -51,6 +52,30 @@ describe("shouldRenderChatStatusBar", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldShowProceed", () => {
|
||||
it.each([
|
||||
["not busy without a clarification", false, false, true],
|
||||
["busy without a clarification", true, false, false],
|
||||
["waiting for input with a pending clarification", false, true, false],
|
||||
])("%s", (_state, isAgentBusy, hasPendingClarification, expected) => {
|
||||
expect(shouldShowProceed("Review", isAgentBusy, hasPendingClarification)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasPendingClarification", () => {
|
||||
it("retains the message-derived fallback", () => {
|
||||
expect(hasPendingClarification(true, null)).toBe(true);
|
||||
});
|
||||
|
||||
it("uses the durable session projection while messages hydrate", () => {
|
||||
expect(hasPendingClarification(false, "clarification")).toBe(true);
|
||||
});
|
||||
|
||||
it("does not treat a durable permission request as a clarification", () => {
|
||||
expect(hasPendingClarification(false, "permission")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveStatusRowTaskId", () => {
|
||||
it("prefers the session-derived task so existing hosts are unchanged", () => {
|
||||
expect(resolveStatusRowTaskId("from-session", "from-host")).toBe("from-session");
|
||||
|
||||
@@ -1,11 +1,19 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, cleanup, renderHook } from "@testing-library/react";
|
||||
import { act, cleanup, render, renderHook, screen } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
|
||||
const toastMock = vi.fn();
|
||||
const handleSendMessageMock = vi.fn();
|
||||
const useKeyboardShortcutMock = vi.hoisted(() => vi.fn());
|
||||
let mockProceedStepName: string | null = null;
|
||||
|
||||
const mockState = { userSettings: { keyboardShortcuts: {} } };
|
||||
const mockState = {
|
||||
userSettings: { keyboardShortcuts: {}, chatSubmitKey: "enter" },
|
||||
quickChat: { sessions: [] },
|
||||
kanban: { workflowId: null, tasks: [], steps: [] },
|
||||
kanbanMulti: { snapshots: {} },
|
||||
workflows: { items: [] },
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: typeof mockState) => unknown) => selector(mockState),
|
||||
@@ -41,6 +49,42 @@ vi.mock("@/components/task/chat/chat-input-container", () => ({
|
||||
ChatInputContainer: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/task/chat/queued-ghost-list", () => ({
|
||||
QueueAffordance: ({
|
||||
children,
|
||||
renderStatusBar,
|
||||
}: {
|
||||
children: ReactNode;
|
||||
renderStatusBar?: (queueChip: ReactNode) => ReactNode;
|
||||
}) => (
|
||||
<>
|
||||
{renderStatusBar?.(null)}
|
||||
{children}
|
||||
</>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("./composer-agent-start-hint", () => ({
|
||||
ComposerAgentStartHint: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./dynamic-route-recovery", () => ({
|
||||
DynamicRouteRecovery: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./chat-status-bar", () => ({
|
||||
ChatStatusBar: (props: {
|
||||
nextStepName: string | null;
|
||||
isAgentBusy: boolean;
|
||||
hasPendingClarification: boolean;
|
||||
}) =>
|
||||
props.nextStepName && !props.isAgentBusy && !props.hasPendingClarification ? (
|
||||
<button type="button" data-testid="proceed-next-step" />
|
||||
) : null,
|
||||
resolveStatusRowTaskId: (sessionTaskId: string | null, statusTaskId: string | null) =>
|
||||
sessionTaskId ?? statusTaskId,
|
||||
}));
|
||||
|
||||
vi.mock("@/components/task/chat/todo-indicator", () => ({
|
||||
TodoIndicator: () => null,
|
||||
}));
|
||||
@@ -62,7 +106,7 @@ vi.mock("@/hooks/use-message-handler", () => ({
|
||||
vi.mock("@/hooks/domains/kanban/use-plan-actions", () => ({
|
||||
usePlanActions: () => ({
|
||||
implementPlanHandler: vi.fn(),
|
||||
proceedStepName: null,
|
||||
proceedStepName: mockProceedStepName,
|
||||
proceed: vi.fn(),
|
||||
isMoving: false,
|
||||
}),
|
||||
@@ -79,7 +123,12 @@ vi.mock("@/lib/ws/connection", () => ({
|
||||
getWebSocketClient: () => ({ send: vi.fn() }),
|
||||
}));
|
||||
|
||||
import { resolveInputPlaceholder, useChatPanelHandlers, useSubmitHandler } from "./chat-input-area";
|
||||
import {
|
||||
ChatInputArea,
|
||||
resolveInputPlaceholder,
|
||||
useChatPanelHandlers,
|
||||
useSubmitHandler,
|
||||
} from "./chat-input-area";
|
||||
|
||||
beforeEach(() => {
|
||||
handleSendMessageMock.mockReset();
|
||||
@@ -89,6 +138,7 @@ beforeEach(() => {
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
mockProceedStepName = null;
|
||||
vi.restoreAllMocks();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
@@ -118,6 +168,51 @@ function panelState(overrides = {}) {
|
||||
} as never;
|
||||
}
|
||||
|
||||
function composerPanelState(overrides = {}) {
|
||||
return {
|
||||
...panelState(),
|
||||
session: { state: "WAITING_FOR_INPUT", pending_action: "clarification" },
|
||||
task: { id: "task-1", title: "Task" },
|
||||
taskDescription: "Task description",
|
||||
needsRecovery: false,
|
||||
planModeAvailable: true,
|
||||
mcpServers: [],
|
||||
mcpAttachmentHistory: [],
|
||||
supportsSteering: false,
|
||||
isStarting: false,
|
||||
isQueueReady: true,
|
||||
inputMode: "direct",
|
||||
isPreparingEnvironment: false,
|
||||
isFailed: false,
|
||||
isCompleted: false,
|
||||
agentCommands: [],
|
||||
todoItems: [],
|
||||
pendingCommentsByFile: {},
|
||||
handleToggleContextFile: vi.fn(),
|
||||
handleAddContextFile: vi.fn(),
|
||||
contextItems: [],
|
||||
planContextEnabled: false,
|
||||
handlePlanModeChange: vi.fn(),
|
||||
...overrides,
|
||||
} as never;
|
||||
}
|
||||
|
||||
function renderComposer(panelStateOverride = {}) {
|
||||
mockProceedStepName = "Review";
|
||||
return render(
|
||||
<ChatInputArea
|
||||
chatInputRef={{ current: null }}
|
||||
clarificationKey={0}
|
||||
onClarificationResolved={vi.fn()}
|
||||
handleSubmit={vi.fn()}
|
||||
handleCancelTurn={vi.fn().mockResolvedValue(undefined)}
|
||||
showRequestChangesTooltip={false}
|
||||
panelState={composerPanelState(panelStateOverride)}
|
||||
isSending={false}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
describe("resolveInputPlaceholder", () => {
|
||||
it("invites queueing while a clarification remains pending", () => {
|
||||
expect(resolveInputPlaceholder(false, undefined, false, true, false)).toBe(
|
||||
@@ -126,6 +221,31 @@ describe("resolveInputPlaceholder", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("ChatInputArea proceed visibility", () => {
|
||||
it("keeps proceed hidden until durable clarification hydration clears", () => {
|
||||
const { rerender } = renderComposer();
|
||||
|
||||
expect(screen.queryByTestId("proceed-next-step")).toBeNull();
|
||||
|
||||
rerender(
|
||||
<ChatInputArea
|
||||
chatInputRef={{ current: null }}
|
||||
clarificationKey={0}
|
||||
onClarificationResolved={vi.fn()}
|
||||
handleSubmit={vi.fn()}
|
||||
handleCancelTurn={vi.fn().mockResolvedValue(undefined)}
|
||||
showRequestChangesTooltip={false}
|
||||
panelState={composerPanelState({
|
||||
session: { state: "WAITING_FOR_INPUT", pending_action: null },
|
||||
})}
|
||||
isSending={false}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("proceed-next-step")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("useSubmitHandler", () => {
|
||||
it("shows a toast when sending fails", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
|
||||
@@ -34,6 +34,7 @@ import { resolveComposerWorkspaceId } from "./composer-workspace";
|
||||
import { t } from "@/lib/i18n";
|
||||
import { ChatStatusBar, resolveStatusRowTaskId } from "./chat-status-bar";
|
||||
import { DynamicRouteRecovery } from "./dynamic-route-recovery";
|
||||
import { hasPendingClarification } from "./types";
|
||||
|
||||
const PLAN_CONTEXT_PATH = "plan:context";
|
||||
|
||||
@@ -444,6 +445,10 @@ export function ChatInputArea({
|
||||
chatInputRef,
|
||||
placeholderOverride,
|
||||
);
|
||||
const clarificationPending = hasPendingClarification(
|
||||
Boolean(panelState.pendingClarification),
|
||||
panelState.session?.pending_action,
|
||||
);
|
||||
const { implementPlanHandler, proceedStepName, proceed, isMoving } = planActions;
|
||||
const composerProps = useComposerProps({
|
||||
panelState,
|
||||
@@ -487,6 +492,7 @@ export function ChatInputArea({
|
||||
nextStepName={proceedStepName}
|
||||
onProceed={proceed}
|
||||
isAgentBusy={isAgentBusy}
|
||||
hasPendingClarification={clarificationPending}
|
||||
isMoving={isMoving}
|
||||
queueChip={queueChip}
|
||||
showScrollToLastPrompt={showScrollToLastPrompt}
|
||||
|
||||
@@ -29,6 +29,7 @@ import { TodoIndicator } from "./todo-indicator";
|
||||
import { AutoScrollToggleButton } from "./auto-scroll-toggle-button";
|
||||
import { PRMergedBanner, PRClosedBanner } from "./pr-archive-banners";
|
||||
import { AutopilotChatChip, useTaskAutopilot } from "./task-autopilot-chat-chip";
|
||||
import { shouldShowProceed } from "./types";
|
||||
|
||||
type TodoDisplayItem = {
|
||||
text: string;
|
||||
@@ -109,6 +110,7 @@ export type ChatStatusBarProps = {
|
||||
nextStepName: string | null;
|
||||
onProceed: () => void;
|
||||
isAgentBusy: boolean;
|
||||
hasPendingClarification: boolean;
|
||||
isMoving: boolean;
|
||||
queueChip?: ReactNode;
|
||||
showScrollToLastPrompt?: boolean;
|
||||
@@ -126,6 +128,7 @@ export function ChatStatusBar({
|
||||
nextStepName,
|
||||
onProceed,
|
||||
isAgentBusy,
|
||||
hasPendingClarification,
|
||||
isMoving,
|
||||
queueChip,
|
||||
showScrollToLastPrompt,
|
||||
@@ -136,7 +139,7 @@ export function ChatStatusBar({
|
||||
}: ChatStatusBarProps) {
|
||||
const { t } = useTranslation();
|
||||
const showTodos = todoItems.length > 0;
|
||||
const showProceed = !!nextStepName && !isAgentBusy;
|
||||
const showProceed = shouldShowProceed(nextStepName, isAgentBusy, hasPendingClarification);
|
||||
const autopilot = useTaskAutopilot(taskId);
|
||||
const showAutoScrollControl = useAppStore(
|
||||
(state) => state.userSettings.showTranscriptAutoScrollControl,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import type { Message } from "@/lib/types/http";
|
||||
import type { Message, TaskPendingAction } from "@/lib/types/http";
|
||||
import type { TaskStatusSummaryActiveError } from "@/lib/types/task-status-summary";
|
||||
import { extractKandevStem } from "./messages/kandev/parse";
|
||||
|
||||
@@ -115,6 +115,23 @@ export function hasProjectedShellOutput(output: ShellExecOutputSummary | undefin
|
||||
);
|
||||
}
|
||||
|
||||
/** Shared composer eligibility predicates belong to the chat domain, not a rendering surface. */
|
||||
export function shouldShowProceed(
|
||||
nextStepName: string | null,
|
||||
isAgentBusy: boolean,
|
||||
hasPendingClarification: boolean,
|
||||
): boolean {
|
||||
return !!nextStepName && !isAgentBusy && !hasPendingClarification;
|
||||
}
|
||||
|
||||
/** Uses the durable session projection until message hydration can supply its fallback. */
|
||||
export function hasPendingClarification(
|
||||
hasPendingClarificationMessage: boolean,
|
||||
pendingAction: TaskPendingAction | null | undefined,
|
||||
): boolean {
|
||||
return hasPendingClarificationMessage || pendingAction === "clarification";
|
||||
}
|
||||
|
||||
export type ShellExecPayload = {
|
||||
command?: string;
|
||||
work_dir?: string;
|
||||
|
||||
@@ -31,6 +31,8 @@ const TID_SEND_COMMENTS = "passthrough-send-comments";
|
||||
|
||||
// --- Mutable state for per-test overrides ---
|
||||
let mockSessionState: string | null = null;
|
||||
let mockPendingClarification: object | null = null;
|
||||
let mockPendingAction: "clarification" | "permission" | null = null;
|
||||
let mockKeyboardShortcuts: Record<string, { key: string; modifiers?: Record<string, boolean> }> =
|
||||
{};
|
||||
const responsiveMock = vi.hoisted(() => ({
|
||||
@@ -68,7 +70,13 @@ vi.mock("@/components/state-provider", () => ({
|
||||
selector({
|
||||
taskSessions: {
|
||||
items: mockSessionState
|
||||
? { [SESSION_ID]: { id: SESSION_ID, state: mockSessionState } }
|
||||
? {
|
||||
[SESSION_ID]: {
|
||||
id: SESSION_ID,
|
||||
state: mockSessionState,
|
||||
pending_action: mockPendingAction,
|
||||
},
|
||||
}
|
||||
: {},
|
||||
},
|
||||
quickChat: { sessions: [] },
|
||||
@@ -189,6 +197,7 @@ vi.mock("./chat/use-chat-panel-state", () => ({
|
||||
taskId: TASK_ID,
|
||||
task: { id: TASK_ID, title: "Task title" },
|
||||
taskDescription: "Task description",
|
||||
session: { state: mockSessionState, pending_action: mockPendingAction },
|
||||
isCompleted: mockSessionState === "COMPLETED",
|
||||
planModeEnabled: mockPlanModeEnabled,
|
||||
planModeAvailable: true,
|
||||
@@ -209,6 +218,7 @@ vi.mock("./chat/use-chat-panel-state", () => ({
|
||||
pendingPRFeedback: [],
|
||||
walkthroughComments: [],
|
||||
messageComments: [],
|
||||
pendingClarification: mockPendingClarification,
|
||||
handleClearMessageComments: vi.fn(),
|
||||
pendingCommentsByFile: mockPendingByFile,
|
||||
}),
|
||||
@@ -299,6 +309,8 @@ async function openComposer() {
|
||||
|
||||
function resetMocks() {
|
||||
mockSessionState = null;
|
||||
mockPendingClarification = null;
|
||||
mockPendingAction = null;
|
||||
mockKeyboardShortcuts = {};
|
||||
responsiveMock.breakpoint = "desktop";
|
||||
mockPendingByFile = {};
|
||||
@@ -359,6 +371,21 @@ describe("PassthroughToolbar – default state", () => {
|
||||
expect(row.className).toContain("flex-wrap");
|
||||
expect(row.lastElementChild?.className).toContain("flex-wrap");
|
||||
});
|
||||
|
||||
it("hides proceed while a clarification barrier is pending", () => {
|
||||
mockSessionState = "WAITING_FOR_INPUT";
|
||||
mockPendingAction = "clarification";
|
||||
mockNextStep = { proceedStepName: "Review", proceed: vi.fn(), isMoving: false };
|
||||
|
||||
const view = renderToolbar();
|
||||
|
||||
expect(screen.queryByTestId(TID_PROCEED)).toBeNull();
|
||||
|
||||
mockPendingAction = null;
|
||||
view.rerender(<PassthroughToolbar sessionId={SESSION_ID} taskId={TASK_ID} />);
|
||||
|
||||
expect(screen.getByTestId(TID_PROCEED)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
function renderStatusRow(breakpoint: "mobile" | "tablet" | "desktop") {
|
||||
|
||||
@@ -41,6 +41,7 @@ import type { KeyboardShortcut } from "@/lib/keyboard/constants";
|
||||
import type { DiffComment } from "@/lib/diff/types";
|
||||
import { PassthroughTerminal } from "./passthrough-terminal";
|
||||
import { PassthroughComposerPanel, useSendPassthroughMessage } from "./passthrough-chat-composer";
|
||||
import { hasPendingClarification, shouldShowProceed } from "./chat/types";
|
||||
import { Trans, useTranslation } from "react-i18next";
|
||||
|
||||
function isEditableElement(element: Element | null) {
|
||||
@@ -136,7 +137,15 @@ export function PassthroughToolbar({
|
||||
handlePlanModeChange: panelState.handlePlanModeChange,
|
||||
chatInputRef,
|
||||
});
|
||||
const showProceed = !!planActions.proceedStepName && !isAgentBusy;
|
||||
const clarificationPending = hasPendingClarification(
|
||||
Boolean(panelState.pendingClarification),
|
||||
panelState.session?.pending_action,
|
||||
);
|
||||
const showProceed = shouldShowProceed(
|
||||
planActions.proceedStepName,
|
||||
isAgentBusy,
|
||||
clarificationPending,
|
||||
);
|
||||
const implementPlanHandler =
|
||||
isAgentBusy || !panelState.planModeEnabled ? undefined : planActions.implementPlanHandler;
|
||||
|
||||
|
||||
@@ -17,7 +17,8 @@ by the task UI. Then refine the shared next-step derivation so a signal-gated
|
||||
`on_turn_complete` `move_to_next` action remains available while ungated moves
|
||||
and gated moves to another configured destination suppress the adjacent-step
|
||||
composer action. Standard chat and passthrough composers retain their existing
|
||||
busy-state gate and manual task-move behavior.
|
||||
busy-state gate and manual task-move behavior, and suppress the action while a
|
||||
clarification barrier is pending.
|
||||
|
||||
## Scope
|
||||
|
||||
@@ -30,6 +31,9 @@ busy-state gate and manual task-move behavior.
|
||||
- Continue suppressing the action for ungated turn-complete moves and for
|
||||
signal-gated `move_to_previous` or `move_to_step` actions whose destinations
|
||||
the adjacent-step control cannot represent.
|
||||
- Hide the action in both composer surfaces while the current session has a
|
||||
pending clarification, including `WAITING_FOR_INPUT`, and reevaluate after
|
||||
the barrier clears.
|
||||
- Preserve the existing busy-state guard, click behavior, error handling, and
|
||||
mobile task-drawer path.
|
||||
- Add focused unit and production-build browser regression coverage.
|
||||
@@ -63,8 +67,11 @@ busy-state gate and manual task-move behavior.
|
||||
Only a signal-gated `move_to_next` action is an exception.
|
||||
- Preserve the Go boot-state mapper field so direct task-page hydration has the
|
||||
same signal-gated policy input as subsequent client refreshes.
|
||||
- Leave `ChatStatusBar` and `PassthroughToolbar` unchanged; both already hide
|
||||
the shared next-step action while the agent is busy.
|
||||
- Use one shared proceed-eligibility policy in `ChatStatusBar` and
|
||||
`PassthroughToolbar` from the chat domain types module so the busy-state and
|
||||
pending-clarification gates cannot drift between surfaces. Combine the
|
||||
durable session `pending_action` projection with the message-derived
|
||||
fallback during transcript hydration.
|
||||
- Leave `proceed()` unchanged so the action continues to use the normal task
|
||||
move API and existing error handling.
|
||||
|
||||
@@ -77,8 +84,10 @@ busy-state gate and manual task-move behavior.
|
||||
refresh, mobile workspace switching, workflow-step WebSocket updates, and
|
||||
live Kanban updates. A Go boot-state mapper test covers direct task-page
|
||||
hydration.
|
||||
- Existing composer and passthrough tests continue to cover the busy-state
|
||||
display gate because their input contract does not change.
|
||||
- Shared composer eligibility tests prove a pending clarification suppresses
|
||||
the action while `WAITING_FOR_INPUT`, and standard/passthrough rendering
|
||||
remains hidden while the durable projection is present before messages
|
||||
hydrate.
|
||||
|
||||
## E2E test
|
||||
|
||||
@@ -93,6 +102,12 @@ type to seed the existing workflow-step field.
|
||||
The existing mobile task-drawer move test remains the parity check for the
|
||||
phone-specific path. No new responsive markup is introduced.
|
||||
|
||||
The pending-clarification correction is covered at the shared eligibility and
|
||||
both composer rendering boundaries. The durable session projection covers the
|
||||
message hydration window, while the clarification overlay lifecycle remains
|
||||
owned by the session panel state. The desktop browser scenario remains the
|
||||
end-to-end proof for the eligible signal-gated move path.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [completed] [Task 01: Preserve signal-gated proceed visibility](task-01-preserve-signal-gated-proceed-visibility.md)
|
||||
@@ -108,6 +123,7 @@ go test ./internal/backendapp
|
||||
Run focused tests from `apps/web`:
|
||||
|
||||
```bash
|
||||
pnpm exec vitest run components/task/chat/chat-input-area.test.ts components/task/chat/chat-input-area.test.tsx components/task/passthrough-toolbar.test.tsx
|
||||
pnpm exec vitest run hooks/domains/kanban/use-plan-actions.test.ts lib/ssr/mapper.test.ts lib/ws/handlers/workflows.test.ts lib/ws/handlers/kanban.test.ts hooks/domains/kanban/use-all-workflow-snapshots.test.ts hooks/domains/kanban/use-all-workflow-snapshots.signal-gated.test.ts components/task/mobile/session-task-switcher-sheet-helpers.test.ts
|
||||
pnpm run typecheck
|
||||
pnpm exec eslint hooks/domains/kanban/use-plan-actions.ts hooks/domains/kanban/use-plan-actions.test.ts lib/state/slices/kanban/types.ts lib/ssr/mapper.ts lib/ssr/mapper.test.ts lib/ws/handlers/workflows.ts lib/ws/handlers/workflows.test.ts lib/ws/handlers/kanban.ts lib/ws/handlers/kanban.test.ts hooks/domains/kanban/use-all-workflow-snapshots.ts hooks/domains/kanban/use-all-workflow-snapshots.signal-gated.test.ts components/task/mobile/session-task-switcher-sheet-helpers.ts components/task/mobile/session-task-switcher-sheet-helpers.test.ts e2e/helpers/api-client.ts e2e/tests/workflow/workflow-step-proceed.spec.ts
|
||||
|
||||
+39
-7
@@ -12,6 +12,7 @@ acceptance_criteria:
|
||||
- AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.2
|
||||
- AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.3
|
||||
- AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.4
|
||||
- AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.5
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/workflow-signal-gated-manual-move-visibility.md
|
||||
---
|
||||
@@ -23,7 +24,8 @@ system_design:
|
||||
Carry the existing signal-gated workflow-step flag into task UI state and use it
|
||||
to distinguish a truly automatic `move_to_next` transition from a signal-gated
|
||||
one. Prove the adjacent-step composer action stays available after the gated
|
||||
agent becomes idle while unsupported gated destinations remain hidden.
|
||||
agent becomes idle while unsupported gated destinations remain hidden. Keep the
|
||||
action hidden in both composer surfaces while a clarification barrier is active.
|
||||
|
||||
## Inputs
|
||||
|
||||
@@ -47,12 +49,18 @@ agent becomes idle while unsupported gated destinations remain hidden.
|
||||
dropped by each mapper.
|
||||
3. **RED:** Add the focused Playwright scenario and confirm the next-step
|
||||
locator remains hidden after the mock agent returns idle.
|
||||
4. **GREEN:** Add the optional step field, preserve it through each mapper and
|
||||
4. **RED:** Add shared composer eligibility coverage and deferred-hydration
|
||||
rendering cases for standard and passthrough surfaces when
|
||||
`WAITING_FOR_INPUT` has a durable pending clarification.
|
||||
5. **GREEN:** Add the optional step field, preserve it through each mapper and
|
||||
the Go boot-state mapper, and narrow the automatic-transition suppression
|
||||
condition in `useNextWorkflowStep`.
|
||||
5. **REFACTOR:** Keep the policy in the shared hook. Do not duplicate it in
|
||||
standard chat, passthrough, or mobile components.
|
||||
6. Run the focused unit, type, lint, desktop E2E, and mobile parity commands.
|
||||
condition in `useNextWorkflowStep`. Apply one shared composer eligibility
|
||||
policy from chat types to standard chat and passthrough surfaces, using both
|
||||
the durable session projection and message-derived fallback.
|
||||
6. **REFACTOR:** Keep workflow projection in the shared hook and keep the
|
||||
busy-state and clarification gates in one shared UI policy. Do not duplicate
|
||||
them in mobile components.
|
||||
7. Run the focused unit, type, lint, desktop E2E, and mobile parity commands.
|
||||
|
||||
## Acceptance
|
||||
|
||||
@@ -62,6 +70,9 @@ agent becomes idle while unsupported gated destinations remain hidden.
|
||||
- Signal-gated `move_to_previous` and `move_to_step` actions remain hidden
|
||||
because the existing composer action submits the adjacent next step.
|
||||
- A busy agent still hides the action until it becomes idle.
|
||||
- A pending clarification hides the action while the session is
|
||||
`WAITING_FOR_INPUT`, including while messages are hydrating; clearing the
|
||||
durable and message-derived barrier makes it eligible again when idle.
|
||||
- Initial load, cached snapshot refresh, mobile workspace switching, and live
|
||||
workflow-step updates preserve identical behavior.
|
||||
- Selecting the action uses the existing manual task-move endpoint and advances
|
||||
@@ -84,6 +95,13 @@ agent becomes idle while unsupported gated destinations remain hidden.
|
||||
- `apps/web/components/task/mobile/session-task-switcher-sheet-helpers.test.ts`
|
||||
- `apps/web/hooks/domains/kanban/use-plan-actions.ts`
|
||||
- `apps/web/hooks/domains/kanban/use-plan-actions.test.ts`
|
||||
- `apps/web/components/task/chat/chat-status-bar.tsx`
|
||||
- `apps/web/components/task/chat/types.ts`
|
||||
- `apps/web/components/task/chat/chat-input-area.tsx`
|
||||
- `apps/web/components/task/chat/chat-input-area.test.ts`
|
||||
- `apps/web/components/task/chat/chat-input-area.test.tsx`
|
||||
- `apps/web/components/task/passthrough-toolbar.tsx`
|
||||
- `apps/web/components/task/passthrough-toolbar.test.tsx`
|
||||
- `apps/web/e2e/helpers/api-client.ts`
|
||||
- `apps/web/e2e/tests/workflow/workflow-step-proceed.spec.ts`
|
||||
- `apps/backend/internal/backendapp/boot_state_routes.go`
|
||||
@@ -95,6 +113,7 @@ agent becomes idle while unsupported gated destinations remain hidden.
|
||||
cd apps/backend
|
||||
go test ./internal/backendapp
|
||||
cd ../web
|
||||
pnpm exec vitest run components/task/chat/chat-input-area.test.ts components/task/chat/chat-input-area.test.tsx components/task/passthrough-toolbar.test.tsx
|
||||
pnpm exec vitest run hooks/domains/kanban/use-plan-actions.test.ts lib/ssr/mapper.test.ts lib/ws/handlers/workflows.test.ts lib/ws/handlers/kanban.test.ts hooks/domains/kanban/use-all-workflow-snapshots.test.ts hooks/domains/kanban/use-all-workflow-snapshots.signal-gated.test.ts components/task/mobile/session-task-switcher-sheet-helpers.test.ts
|
||||
pnpm run typecheck
|
||||
pnpm exec eslint hooks/domains/kanban/use-plan-actions.ts hooks/domains/kanban/use-plan-actions.test.ts lib/state/slices/kanban/types.ts lib/ssr/mapper.ts lib/ssr/mapper.test.ts lib/ws/handlers/workflows.ts lib/ws/handlers/workflows.test.ts lib/ws/handlers/kanban.ts lib/ws/handlers/kanban.test.ts hooks/domains/kanban/use-all-workflow-snapshots.ts hooks/domains/kanban/use-all-workflow-snapshots.signal-gated.test.ts components/task/mobile/session-task-switcher-sheet-helpers.ts components/task/mobile/session-task-switcher-sheet-helpers.test.ts e2e/helpers/api-client.ts e2e/tests/workflow/workflow-step-proceed.spec.ts
|
||||
@@ -116,6 +135,9 @@ None.
|
||||
- A loose boolean default could turn older payloads into gated steps.
|
||||
- Changing the display components could accidentally weaken the active-turn
|
||||
guard; they should remain consumers of the shared derived value.
|
||||
- Reading only `isAgentBusy` would treat `WAITING_FOR_INPUT` as eligible even
|
||||
while a durable clarification is pending; both composer surfaces must use the
|
||||
shared pending-clarification gate and retain the message-derived fallback.
|
||||
|
||||
## Parallelism
|
||||
|
||||
@@ -141,10 +163,20 @@ plan work-order checkbox to `completed` only after all acceptance criteria pass.
|
||||
file-size warnings.
|
||||
- Desktop browser regression passed: 1 test.
|
||||
- Mobile task-drawer parity regression passed: 1 test.
|
||||
- Specification tests passed: 30 tests. Full specification lint passed. Diff
|
||||
- Specification tests passed: 36 tests. Full specification lint passed. Diff
|
||||
check passed.
|
||||
- PR fixup remediation: the shared policy now exposes only gated
|
||||
`move_to_next`; gated `move_to_previous` and `move_to_step` remain hidden,
|
||||
and omitted flags remain ungated. The Go boot-state mapper now preserves the
|
||||
field for direct task-page hydration. The browser regression waits for the
|
||||
task API to report the target step before asserting the UI.
|
||||
- Replacement PR remediation: both composer surfaces hide the signal-gated
|
||||
proceed action during an active clarification barrier and restore eligibility
|
||||
after it clears. The shared predicates now live in `chat/types.ts`, and the
|
||||
durable session projection covers the message hydration window. Focused
|
||||
coverage includes standard and passthrough rendering in `WAITING_FOR_INPUT`
|
||||
with a pending clarification.
|
||||
- Replacement PR verification: 10 frontend test files, 145 tests passed;
|
||||
typecheck passed; specification tests (36) and full specification lint passed;
|
||||
diff check passed. Disposable desktop and mobile clarification captures each
|
||||
passed one browser test and were removed before commit.
|
||||
|
||||
@@ -49,6 +49,11 @@ when the agent did not emit its completion signal.
|
||||
- **AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.4:** The signal-gated
|
||||
setting shall produce the same next-step visibility after initial hydration,
|
||||
workflow snapshot refresh, and live workflow-step updates.
|
||||
- **AC-TASKS-WORKFLOW-EXPLICIT-COMPLETION-SIGNAL-002.5:** While the current
|
||||
session has a pending durable clarification, task composer surfaces shall
|
||||
hide the next-step action even when the session is waiting for input. After
|
||||
the clarification barrier clears, the surfaces shall reevaluate visibility
|
||||
and show the action when the signal-gated idle conditions are satisfied.
|
||||
|
||||
## Migrated source detail
|
||||
|
||||
@@ -132,6 +137,18 @@ The pending completion signal continues to use `TaskSession.Metadata` as specifi
|
||||
- **GIVEN** a signal-gated workflow step with a configured `move_to_next` move,
|
||||
**WHEN** its agent is busy, **THEN** the next-step action remains hidden until
|
||||
the agent returns to an idle state.
|
||||
- **GIVEN** a signal-gated workflow step with a configured `move_to_next` move
|
||||
and a pending clarification, **WHEN** its session is waiting for input,
|
||||
**THEN** the standard and passthrough next-step actions remain hidden until
|
||||
the clarification is answered or otherwise cleared.
|
||||
- **GIVEN** the same session's durable pending-action projection is
|
||||
`clarification` while its messages are still hydrating, **WHEN** its session
|
||||
is waiting for input, **THEN** both composer surfaces keep the next-step
|
||||
action hidden until the message-derived or durable clarification barrier
|
||||
clears.
|
||||
- **GIVEN** the same signal-gated workflow step after its clarification barrier
|
||||
clears, **WHEN** its session is idle, **THEN** the standard and passthrough
|
||||
next-step actions become eligible again.
|
||||
- **GIVEN** a signal-gated workflow step with a configured `move_to_previous` or
|
||||
`move_to_step` move, **WHEN** its composer is rendered, **THEN** the existing
|
||||
adjacent-next-step action remains hidden because it cannot submit the
|
||||
|
||||
@@ -16,8 +16,8 @@ The task system owns whether a workflow transition is automatic, signal-gated,
|
||||
or manually recoverable. The web application presents that state in the normal
|
||||
chat and passthrough composer controls. This design changes only the visibility
|
||||
policy for the existing next-step action. It does not change transition
|
||||
execution, completion-signal persistence, clarification handling, or the future
|
||||
ADR 0015 `manual_fallback` signal path.
|
||||
execution, completion-signal persistence, clarification lifecycle handling, or
|
||||
the future ADR 0015 `manual_fallback` signal path.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
@@ -39,8 +39,10 @@ ADR 0015 `manual_fallback` signal path.
|
||||
as do signal-gated `move_to_previous` and `move_to_step` actions, because the
|
||||
existing `moveTask` operation submits the adjacent next step rather than a
|
||||
configured arbitrary destination.
|
||||
- `ChatStatusBar` and `PassthroughToolbar` retain their existing busy-state gate
|
||||
and consume the shared next-step projection.
|
||||
- `ChatStatusBar` and `PassthroughToolbar` consume the shared next-step
|
||||
projection and shared eligibility policy. Both retain the busy-state gate
|
||||
and suppress the action while message-derived `pendingClarification` is
|
||||
present or the durable session `pending_action` is `clarification`.
|
||||
- The phone task drawer remains the alternate manual step-move surface. This
|
||||
correction adds no phone-only layout or interaction.
|
||||
|
||||
@@ -54,6 +56,11 @@ Every projection from `WorkflowSnapshot.steps` or workflow-step WebSocket
|
||||
payloads into `KanbanState.steps` copies the field without defaulting it. The
|
||||
visibility rule treats only the literal value `true` as signal-gated.
|
||||
|
||||
The session `pending_action` projection is authoritative while transcript
|
||||
messages hydrate. Composer surfaces retain the message-derived clarification
|
||||
fallback after hydration and combine both signals through the shared chat
|
||||
eligibility predicates.
|
||||
|
||||
## Control flow
|
||||
|
||||
1. Initial hydration, snapshot refresh, workspace switching, or a live
|
||||
@@ -68,7 +75,9 @@ visibility rule treats only the literal value `true` as signal-gated.
|
||||
`move_to_previous` and `move_to_step` actions remain suppressed because the
|
||||
adjacent-next-step control would submit the wrong destination.
|
||||
5. The standard and passthrough composer surfaces show the existing action only
|
||||
when the shared projection returns a next-step name and the agent is idle.
|
||||
when the shared projection returns a next-step name, the agent is idle, and
|
||||
neither the durable session projection nor the message-derived fallback
|
||||
reports a pending clarification.
|
||||
6. Selecting the action uses the existing manual task-move request and its
|
||||
existing error handling.
|
||||
|
||||
@@ -78,6 +87,10 @@ visibility rule treats only the literal value `true` as signal-gated.
|
||||
- An omitted signal-gated field preserves the legacy ungated behavior.
|
||||
- A busy agent keeps the action hidden. Returning to idle recomputes the surface
|
||||
without a reload.
|
||||
- A pending clarification keeps the action hidden while the session waits for
|
||||
the user's answer. The durable session projection covers the message
|
||||
hydration window; clearing both signals recomputes the surface and restores
|
||||
eligibility when the agent is idle.
|
||||
- A rejected manual move keeps the task on the current step and uses the current
|
||||
localized error toast.
|
||||
- No completion signal is synthesized, so this path cannot increment the future
|
||||
@@ -96,18 +109,19 @@ task-move API and backend policy checks.
|
||||
## Observability
|
||||
|
||||
No new production metric is required for visibility. Unit tests cover all
|
||||
client projection paths, the gated versus ungated decision, and the unsupported
|
||||
gated move destinations. The boot mapper has a regression test for direct
|
||||
client projection paths, the gated versus ungated decision, the unsupported
|
||||
gated move destinations, the durable clarification hydration window, and the
|
||||
message-derived fallback. The boot mapper has a regression test for direct
|
||||
task-page hydration. A browser test proves the user-visible action after an
|
||||
idle signal-gated turn and waits for the causal backend move before asserting
|
||||
the stepper.
|
||||
|
||||
## Responsive behavior
|
||||
|
||||
The standard and passthrough composer surfaces share the state policy. The
|
||||
phone task drawer continues to expose its existing step-move control, so no new
|
||||
compressed desktop control, touch target, breakpoint, or scroll behavior is
|
||||
introduced.
|
||||
The standard and passthrough composer surfaces share the state policy, including
|
||||
the clarification barrier. The phone task drawer continues to expose its
|
||||
existing step-move control, so no new compressed desktop control, touch target,
|
||||
breakpoint, or scroll behavior is introduced.
|
||||
|
||||
## Related decisions
|
||||
|
||||
|
||||
Reference in New Issue
Block a user