Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/design/experimental-session-plan-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,35 @@ approving exits Plan Mode.
- Fall back to the existing text-only approval when no matching snapshot is
available.

### Phase 3: current-session cockpit

- Add an experimental Workflow full-page view beside the existing Chat view.
- Reuse the active Todo snapshot, daemon task polling, linked Agent tools, and
the existing artifact panel instead of introducing another workflow model.
- Open the Workflow view when a matching `exit_plan_mode` approval arrives.
After the approval resolves, keep the Workflow visible for observation; the
user can return to Chat at any time.
- Keep Chat mounted while Workflow is visible so switching views does not
interrupt execution or discard composer state.
- Summarize overall completion, active Agents, and steps needing attention from
the same Todo and daemon-task snapshots used by the graph.
- Let a selected step show its upstream and downstream relationships plus the
linked Agent's latest activity and runtime metrics. Opening an Agent continues
into the existing transcript and artifact panel.
- Keep the Workflow entry available after completion, later chat turns, and
session resume by reading the latest Todo snapshot from the transcript. The
compact Todo panel still clears on the next user turn.
- Preserve an active Todo's existing dependencies when an update for the same
ID omits `blockedBy`; an explicit empty array removes dependencies.

## Boundaries

The Workflow remains observational. It does not schedule dependencies, retry
Agents, propagate completion, or add a Workflow store. `blockedBy` and
`todo_id` remain optional for sessions outside Plan & Review.

The standalone cockpit mock remains a product reference rather than a second
application embedded through an iframe. The Web Shell Workflow page reuses the
mock's plan, progress, Agent activity, and detail concepts while leaving
DataWorks-specific scheduling, retry, and approval queues to their owning
product.
63 changes: 61 additions & 2 deletions packages/core/src/tools/todoWrite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,19 +341,30 @@ describe('TodoWriteTool', () => {
mockFs.readFile.mockResolvedValue(
JSON.stringify({
planId: 'finished-plan',
todos: [{ id: '1', content: 'Done', status: 'completed' }],
todos: [
{ id: 'prepare', content: 'Prepare', status: 'completed' },
{
id: 'ship',
content: 'Done',
status: 'completed',
blockedBy: ['prepare'],
},
],
}),
);
mockFs.mkdir.mockResolvedValue(undefined);
mockAtomicWrite.mockResolvedValue(undefined);

const result = await tool
.build({ todos: [{ id: '1', content: 'New', status: 'pending' }] })
.build({ todos: [{ id: 'ship', content: 'New', status: 'pending' }] })
.execute(mockAbortSignal);
const display = result.returnDisplay as { planId?: string };

expect(display.planId).toEqual(expect.any(String));
expect(display.planId).not.toBe('finished-plan');
expect(
JSON.parse(mockAtomicWrite.mock.calls[0][1] as string).todos,
).toEqual([{ id: 'ship', content: 'New', status: 'pending' }]);
});

it('should start a new plan for a distinct all-completed snapshot', async () => {
Expand Down Expand Up @@ -484,6 +495,54 @@ describe('TodoWriteTool', () => {
expect(reminder).not.toContain('Updated Task');
});

it('preserves dependencies when a status update omits blockedBy', async () => {
mockFs.readFile.mockResolvedValue(
JSON.stringify({
todos: [
{ id: 'prepare', content: 'Prepare', status: 'completed' },
{
id: 'ship',
content: 'Ship',
status: 'pending',
blockedBy: ['prepare'],
},
{
id: 'note',
content: 'Old note',
status: 'pending',
blockedBy: ['prepare'],
},
],
}),
);
mockFs.mkdir.mockResolvedValue(undefined);
mockAtomicWrite.mockResolvedValue(undefined);

await tool
.build({
todos: [
{ id: 'prepare', content: 'Prepare', status: 'completed' },
{ id: 'ship', content: 'Ship', status: 'completed' },
{
id: 'note',
content: 'Updated note',
status: 'pending',
blockedBy: [],
},
],
})
.execute(mockAbortSignal);

expect(
JSON.parse(mockAtomicWrite.mock.calls[0][1] as string).todos,
).toContainEqual(
expect.objectContaining({ id: 'ship', blockedBy: ['prepare'] }),
);
expect(
JSON.parse(mockAtomicWrite.mock.calls[0][1] as string).todos,
).toContainEqual(expect.objectContaining({ id: 'note', blockedBy: [] }));
});

it('should handle file write errors', async () => {
const params: TodoWriteParams = {
todos: [
Expand Down
16 changes: 12 additions & 4 deletions packages/core/src/tools/todoWrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ const todoWriteToolSchemaData: FunctionDeclaration = {
type: 'array',
items: { type: 'string', maxLength: 500 },
uniqueItems: true,
description: 'Todo IDs that must be completed before this item',
description:
'Todo IDs that must be completed before this item. Active-plan updates preserve omitted dependencies for existing IDs; use [] to remove them.',
},
},
required: ['content', 'status', 'id'],
Expand Down Expand Up @@ -282,6 +283,10 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
// 1. Read current todos (for change detection)
const previousPlan = await readTodoPlanFromFile(sessionId);
const oldTodos = previousPlan.todos;
const oldTodosMap = new Map(oldTodos.map((todo) => [todo.id, todo]));
const hasActivePlan = oldTodos.some(
(todo) => todo.status !== 'completed',
);

let candidateTodos: unknown;

Expand All @@ -290,8 +295,12 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
const data = JSON.parse(modified_content) as Record<string, unknown>;
candidateTodos = data['todos'];
} else {
// Use the normal todo logic - simply replace with new todos
candidateTodos = todos;
candidateTodos = todos.map((todo) => {
const blockedBy =
todo.blockedBy ??
(hasActivePlan ? oldTodosMap.get(todo.id)?.blockedBy : undefined);
return blockedBy === undefined ? todo : { ...todo, blockedBy };
});
}

const validationError = validateTodos(candidateTodos);
Expand All @@ -300,7 +309,6 @@ class TodoWriteToolInvocation extends BaseToolInvocation<

// 2. Detect changes
const changes = detectTodoChanges(oldTodos, finalTodos);
const oldTodosMap = new Map(oldTodos.map((t) => [t.id, t]));

// 3. VALIDATION PHASE: Execute all hooks with Validation phase
// Hooks should only check and return block/approve decisions, no side effects
Expand Down
8 changes: 8 additions & 0 deletions packages/web-shell/client/App.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,14 @@
padding: 20px 24px;
}

.workflowPageBody {
flex: 1 1 auto;
min-width: 0;
min-height: 0;
overflow: auto;
padding: 20px 24px;
}

.mobileDrawer {
display: contents;
}
Expand Down
127 changes: 118 additions & 9 deletions packages/web-shell/client/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2459,18 +2459,52 @@ describe('App plan todos', () => {
}),
];

const { rerender } = renderApp();
const { container, rerender } = renderApp();
await flush();

expect(testState.latestToolApprovalPlanTodos).toEqual([]);

await act(async () => {
container
.querySelector<HTMLButtonElement>('[data-testid="open-split-view"]')
?.click();
await Promise.resolve();
});

testState.settings = [sessionWorkflowSetting()];
rerender();
await flush();

expect(
container.querySelector('[data-testid="split-view-page"]'),
).not.toBeNull();
await act(async () => {
container
.querySelector('[data-testid="split-approval-notice"] button')
?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
await Promise.resolve();
});

expect(
testState.latestToolApprovalPlanTodos.map((todo) => todo.id),
).toEqual(['prepare', 'ship']);
expect(
container.querySelector('[data-testid="workflow-page"]'),
).not.toBeNull();

testState.blocks = [
makePendingPermissionBlock({
resolved: true,
toolName: 'exit_plan_mode',
kind: 'switch_mode',
todoPlan: { planId: 'plan-1', sourceCallId: 'todo-approved' },
}),
];
rerender();
await flush();
expect(
container.querySelector('[data-testid="workflow-page"]'),
).not.toBeNull();
});

it('refreshes dependencies when only blockedBy changes', async () => {
Expand Down Expand Up @@ -2515,13 +2549,23 @@ describe('App plan todos', () => {
expect(testState.latestTodoPanelTodos[1]?.blockedBy).toEqual([]);
});

it('opens the workflow dialog with plan todos and linked agents', async () => {
it('opens the workflow page with plan todos and linked agents', async () => {
testState.settings = [sessionWorkflowSetting()];
testState.messages = [
{
id: 'plan',
role: 'plan',
todos: [{ id: 'work', content: 'Work', status: 'in_progress' }],
role: 'tool_group',
tools: [
{
callId: 'todo-start',
toolName: 'todo_write',
status: 'completed',
args: {
todos: [{ id: 'work', content: 'Work', status: 'in_progress' }],
},
rawOutput: { plan: { id: 'plan-1' } },
},
],
},
{
id: 'agents',
Expand All @@ -2530,13 +2574,14 @@ describe('App plan todos', () => {
{
callId: 'agent-call',
toolName: 'Agent',
title: 'Worker agent',
status: 'in_progress',
args: { todo_id: 'work' },
},
],
},
];
renderApp();
const { container, rerender } = renderApp();
await flush();

await act(async () => {
Expand All @@ -2545,11 +2590,75 @@ describe('App plan todos', () => {
});

expect(
testState.latestTasksStatusProps?.planTodos?.map((todo) => todo.id),
).toEqual(['work']);
container.querySelector('[data-testid="workflow-page"]'),
).not.toBeNull();
expect(
container.querySelector('[data-plan-node-id="work"]'),
).not.toBeNull();
expect(
container.querySelector('[data-testid="workflow-page"]')?.textContent,
).toContain('Worker agent');
expect(document.activeElement).toBe(
container.querySelector(
'[data-testid="workflow-page"] button[aria-label="back"]',
),
);

await act(async () => {
container
.querySelector<HTMLButtonElement>(
'[data-testid="workflow-page"] button[aria-label="back"]',
)
?.click();
await Promise.resolve();
});
testState.messages = [
...testState.messages.map((message) => {
if (message.id !== 'agents' || message.role !== 'tool_group') {
return message;
}
return {
...message,
tools: message.tools.map((tool) => ({
...tool,
status: 'completed' as const,
})),
};
}),
{
id: 'plan-complete',
role: 'tool_group',
tools: [
{
callId: 'todo-complete',
toolName: 'todo_write',
status: 'completed',
args: {
todos: [{ id: 'work', content: 'Work', status: 'completed' }],
},
rawOutput: { plan: { id: 'plan-1' } },
},
],
},
{ id: 'follow-up', role: 'user', content: 'What happened?' },
{ id: 'reply', role: 'assistant', content: 'The work completed.' },
];
rerender();
await flush();

await act(async () => {
container
.querySelector<HTMLButtonElement>('[data-testid="open-workflow"]')
?.click();
await Promise.resolve();
});

expect(
container.querySelector('[data-testid="workflow-page"]')?.textContent,
).toContain('100%');
expect(
testState.latestTasksStatusProps?.agentTools?.map((tool) => tool.callId),
).toEqual(['agent-call']);
container.querySelector('[data-testid="workflow-page"]')?.textContent,
).toContain('Worker agent');
});

it('keeps the tasks dialog plain when Session Workflow is off', async () => {
Expand Down
Loading
Loading