Skip to content
Merged
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
31 changes: 30 additions & 1 deletion packages/cli/src/ui/commands/workflowsCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import { promises as fs } from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { workflowsCommand } from './workflowsCommand.js';
import { workflowsCommand, snapshotToTask } from './workflowsCommand.js';
import { type CommandContext } from './types.js';
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
import type { WorkflowTask, WorkflowSnapshot } from '@qwen-code/qwen-code-core';
Expand All @@ -28,10 +28,14 @@ function entry(overrides: Partial<WorkflowTask> = {}): WorkflowTask {
isBackgrounded: true,
abortController: new AbortController(),
currentPhase: null,
currentPhaseVisitId: null,
phases: [],
phaseVisits: [],
dispatches: [],
agentsDispatched: 0,
agentsCompleted: 0,
recentLogs: [],
events: [],
tokensSpent: 0,
tokenBudgetTotal: null,
perPhaseTokens: new Map<string | null, number>(),
Expand Down Expand Up @@ -670,3 +674,28 @@ describe('workflowsCommand', () => {
});
});
});

describe('snapshotToTask', () => {
it('preserves persisted lineage fields across the restart boundary', () => {
const task = snapshotToTask({
runId: 'wf_lineage',
sourceRunId: 'wf_origin',
startMode: 'retry',
meta: null,
status: 'completed',
script: '',
phases: [],
agentsDispatched: 0,
agentsCompleted: 0,
tokensSpent: 0,
tokenBudgetTotal: null,
perPhaseTokens: [],
recentLogs: [],
startTime: 1_700_000_000_000,
endTime: 1_700_000_005_000,
});

expect(task.sourceRunId).toBe('wf_origin');
expect(task.startMode).toBe('retry');
});
});
10 changes: 8 additions & 2 deletions packages/cli/src/ui/commands/workflowsCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { formatDuration, formatTokenCount } from '../utils/formatters.js';
* `outputFile`, etc.) are filled with inert values — a snapshot is always
* terminal, so the controls that read those fields are never reached.
*/
function snapshotToTask(s: WorkflowSnapshot): WorkflowTask {
export function snapshotToTask(s: WorkflowSnapshot): WorkflowTask {
return {
id: s.runId,
kind: 'workflow',
Expand All @@ -30,10 +30,16 @@ function snapshotToTask(s: WorkflowSnapshot): WorkflowTask {
meta: s.meta,
status: s.status,
currentPhase: null,
currentPhaseVisitId: null,
phases: s.phases ?? [],
phaseVisits: s.phaseVisits ?? [],
dispatches: s.dispatches ?? [],
Comment on lines +35 to +36

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] toSnapshot now persists sourceRunId/startMode (and description), but snapshotToTask — the only reconstruction path from persisted data — never reads them back (description coincidentally survives via the identical meta?.name ?? runId re-derivation). — Failure scenario: for a retry/rerun run, lineage is present in-session and on disk, but undefined on every post-restart reconstruction — any consumer of task.sourceRunId/task.startMode (the detail view, or the rerun flow these fields are staged for) sees lineage in-session and silently undefined after restart, even though the value sits in the snapshot file.

Suggested change
phaseVisits: s.phaseVisits ?? [],
dispatches: s.dispatches ?? [],
phaseVisits: s.phaseVisits ?? [],
dispatches: s.dispatches ?? [],
sourceRunId: s.sourceRunId,
startMode: s.startMode,
中文说明

[Suggestion] toSnapshot 现在会持久化 sourceRunId/startMode(以及 description),但 snapshotToTask——从持久化数据重建的唯一路径——从不读回它们(description 只是碰巧通过相同的 meta?.name ?? runId 重新推导而幸存)。失败场景:对 retry/rerun 运行,血缘信息在会话内与磁盘上都存在,但每次重启后重建时都是 undefined——任何 task.sourceRunId/task.startMode 的消费方(详情视图,或这些字段所预备的 rerun 流程)会话内能看到血缘、重启后却静默变为 undefined,尽管值就保存在快照文件里。

— qwen3.8-max via Qwen Code /review (v0.21.10)

sourceRunId: s.sourceRunId,
startMode: s.startMode,
agentsDispatched: s.agentsDispatched ?? 0,
agentsCompleted: s.agentsCompleted ?? 0,
recentLogs: s.recentLogs ?? [],
events: s.events ?? [],
tokensSpent: s.tokensSpent ?? 0,
tokenBudgetTotal: s.tokenBudgetTotal ?? null,
perPhaseTokens: new Map(s.perPhaseTokens ?? []),
Expand All @@ -48,7 +54,7 @@ function snapshotToTask(s: WorkflowSnapshot): WorkflowTask {
outputOffset: 0,
notified: true,
abortController: new AbortController(),
} as WorkflowTask;
};
}

/**
Expand Down
17 changes: 17 additions & 0 deletions packages/core/src/agents/runtime/workflow-journal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,23 @@ describe('WorkflowJournal', () => {
expect(replay.started.get('k1')).toHaveLength(1);
});

it('drain waits for fire-and-forget appends', async () => {
const j = new WorkflowJournal(path.join(dir, 'sub', 'journal.jsonl'));
void j.append({ type: 'started', key: 'k1', agentId: '1' });
void j.append({
type: 'result',
key: 'k1',
agentId: '1',
result: 'done',
});

await j.drain();

const replay = await j.load();
expect(replay.started.get('k1')).toHaveLength(1);
expect(replay.results.get('k1')?.result).toBe('done');
});

it('load on a missing file returns empty maps', async () => {
const j = new WorkflowJournal(path.join(dir, 'nope.jsonl'));
const replay = await j.load();
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/agents/runtime/workflow-journal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ export function buildReplay(entries: JournalEntry[]): JournalReplay {
* failure must not fail the dispatch).
*/
export class WorkflowJournal {
private pending = Promise.resolve();

constructor(readonly path: string) {}

/** Load + parse all entries into replay maps. Empty maps if no file. */
Expand All @@ -194,6 +196,13 @@ export class WorkflowJournal {

/** Append one entry. Rejects only on I/O error (callers `.catch`). */
append(entry: JournalEntry): Promise<void> {
return writeLine(this.path, entry);
const operation = this.pending.then(() => writeLine(this.path, entry));
this.pending = operation.catch(() => undefined);
Comment on lines +199 to +200

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The chain-poisoning guard that isolates a failed append from all subsequent appends (this.pending = operation.catch(() => undefined)) has zero test coverage — no test makes an append reject. Verified: the mutation this.pending = operation keeps 214/214 workflow tests and the full packages/core suite green. — Failure scenario: probe flips — with the mutation, one transient writeLine failure (EMFILE / disk full / AV lock) leaves this.pending rejected, and every later append short-circuits through .then without ever calling writeLine — the journal silently loses every remaining started/result entry, and the next Workflow({resumeFromRunId}) re-runs the whole suffix live: duplicated token spend, potentially different results, only a debug-level warning as signal.

Add one test: make the first append reject (e.g. mockRejectedValueOnce on fs.promises.appendFile), then assert the failing append rejects for its caller while the second append still writes and drain() resolves.

中文说明

[Suggestion] 隔离失败追加、保护后续所有追加的防中毒守卫(this.pending = operation.catch(() => undefined))零测试覆盖——没有任何测试让 append 被拒绝。已验证:变异 this.pending = operation 后 214/214 workflow 测试与整个 packages/core 套件仍为绿。失败场景(探针翻转):变异之后,一次瞬时 writeLine 失败(EMFILE/磁盘满/杀软锁)使 this.pending 保持 rejected,此后每次 append 都经 .then 短路、永不调用 writeLine ——journal 静默丢失剩余全部 started/result 条目,下一次 Workflow({resumeFromRunId}) 会完整 live 重跑整个后缀:重复消耗 token、结果可能不同,唯一的信号是一条 debug 级警告。建议补一个测试:令第一次 append 被拒绝,断言失败的 append 对调用方拒绝、第二次 append 仍写入、drain() 正常 resolve。

— qwen3.8-max via Qwen Code /review (v0.21.10)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred to the next round. Still planned: one journal test making the first append reject (mockRejectedValueOnce on fs.promises.appendFile), asserting the failing append rejects for its caller while the second append still writes and drain() resolves.

中文说明

延后至下一轮。仍计划:新增一个 journal 测试令首次 append 被拒绝(对 fs.promises.appendFile 使用 mockRejectedValueOnce),断言失败的 append 对调用方拒绝、第二次 append 仍写入、drain() 正常 resolve。

return operation;
}

/** Wait until every append issued so far has settled. */
drain(): Promise<void> {
return this.pending;
}
}
Loading
Loading