Skip to content

fix(sdk): align WorkflowTrajectory writer with canonical layout + fields #730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
khaliqgant wants to merge 1 commit into from
Closed

fix(sdk): align WorkflowTrajectory writer with canonical layout + fields #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 54 additions & 19 deletions packages/sdk/src/__tests__/workflow-trajectory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,32 @@ function readTrajectoryFile(dir: string): any {
return JSON.parse(readFileSync(path.join(activeDir, jsonFiles[0]), 'utf-8'));
}

function readCompletedTrajectoryFile(dir: string): any {
function findCompletedTrajectoryJson(dir: string): string | null {
const completedDir = path.join(dir, '.trajectories', 'completed');
if (!existsSync(completedDir)) return null;

const files = readdirSync(completedDir);
const jsonFiles = files.filter((f: string) => f.endsWith('.json'));
if (jsonFiles.length === 0) return null;
// Completed trajectories now live under completed/YYYY-MM/. Walk the
// tree so tests don't have to know the exact bucket name.
const stack: string[] = [completedDir];
while (stack.length > 0) {
const current = stack.pop() as string;
const entries = readdirSync(current, { withFileTypes: true });
for (const entry of entries) {
const entryPath = path.join(current, entry.name);
if (entry.isDirectory()) {
stack.push(entryPath);
} else if (entry.isFile() && entry.name.endsWith('.json')) {
return entryPath;
}
}
}
return null;
}

return JSON.parse(readFileSync(path.join(completedDir, jsonFiles[0]), 'utf-8'));
function readCompletedTrajectoryFile(dir: string): any {
const jsonPath = findCompletedTrajectoryJson(dir);
if (!jsonPath) return null;
return JSON.parse(readFileSync(jsonPath, 'utf-8'));
}

// ── Tests ────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -135,6 +152,34 @@ describe('WorkflowTrajectory', () => {
expect(completed).toBeTruthy();
expect(completed.status).toBe('abandoned');
});

it('should write completed files under completed/YYYY-MM/', async () => {
const traj = new WorkflowTrajectory({}, 'run-abc', tmpDir);
await traj.start('my-workflow', 2);
await traj.complete('All done', 0.95);

const jsonPath = findCompletedTrajectoryJson(tmpDir);
expect(jsonPath).not.toBeNull();

// Relative path from .trajectories/completed must have exactly one
// intermediate directory matching YYYY-MM.
const completedRoot = path.join(tmpDir, '.trajectories', 'completed');
const rel = path.relative(completedRoot, jsonPath as string);
const segments = rel.split(path.sep);
expect(segments).toHaveLength(2);
expect(segments[0]).toMatch(/^\d{4}-\d{2}$/);
expect(segments[1]).toMatch(/^traj_.*\.json$/);
});

it('should populate canonical empty arrays on start', async () => {
const traj = new WorkflowTrajectory({}, 'run-abc', tmpDir);
await traj.start('my-workflow', 2);

const data = readTrajectoryFile(tmpDir);
expect(data.commits).toEqual([]);
expect(data.filesChanged).toEqual([]);
expect(data.tags).toEqual([]);
});
});

// ── Step events ────────────────────────────────────────────────────────
Expand All @@ -143,10 +188,7 @@ describe('WorkflowTrajectory', () => {
it('should record step started', async () => {
const traj = new WorkflowTrajectory({}, 'run-1', tmpDir);
await traj.start('wf', 2);
await traj.stepStarted(
{ name: 'build', agent: 'builder', task: 'Build it' },
'builder-agent',
);
await traj.stepStarted({ name: 'build', agent: 'builder', task: 'Build it' }, 'builder-agent');

const data = readTrajectoryFile(tmpDir);
expect(data.agents).toHaveLength(2); // orchestrator + builder-agent
Expand All @@ -157,11 +199,7 @@ describe('WorkflowTrajectory', () => {
it('should record step completed', async () => {
const traj = new WorkflowTrajectory({}, 'run-1', tmpDir);
await traj.start('wf', 1);
await traj.stepCompleted(
{ name: 'test', agent: 'tester', task: 'Run tests' },
'All tests passing',
1,
);
await traj.stepCompleted({ name: 'test', agent: 'tester', task: 'Run tests' }, 'All tests passing', 1);

const data = readTrajectoryFile(tmpDir);
const events = data.chapters.flatMap((c: any) => c.events);
Expand All @@ -175,7 +213,7 @@ describe('WorkflowTrajectory', () => {
{ name: 'deploy', agent: 'deployer', task: 'Deploy' },
'Connection refused',
1,
3,
3
);

const data = readTrajectoryFile(tmpDir);
Expand All @@ -186,10 +224,7 @@ describe('WorkflowTrajectory', () => {
it('should record step skipped', async () => {
const traj = new WorkflowTrajectory({}, 'run-1', tmpDir);
await traj.start('wf', 2);
await traj.stepSkipped(
{ name: 'integration', agent: 'tester', task: 'Test' },
'Upstream failed',
);
await traj.stepSkipped({ name: 'integration', agent: 'tester', task: 'Test' }, 'Upstream failed');

const data = readTrajectoryFile(tmpDir);
const events = data.chapters.flatMap((c: any) => c.events);
Expand Down
27 changes: 26 additions & 1 deletion packages/sdk/src/workflows/trajectory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ interface TrajectoryFile {
learnings?: string[];
challenges?: string[];
};
/**
* Canonical top-level fields the `agent-trajectories` schema declares
* (as `.default([])` / `.optional()`). We populate them here so the
* written files are canonical-complete and round-trip cleanly through
* any stricter future reader.
*/
commits: string[];
filesChanged: string[];
tags: string[];
}

// ── Step state for synthesis ─────────────────────────────────────────────────
Expand Down Expand Up @@ -191,6 +200,9 @@ export class WorkflowTrajectory {
startedAt: new Date().toISOString(),
agents: [{ name: 'orchestrator', role: 'workflow-runner', joinedAt: new Date().toISOString() }],
chapters: [],
commits: [],
filesChanged: [],
tags: [],
};

// Open Planning chapter — record intent, not just mechanics
Expand Down Expand Up @@ -765,7 +777,20 @@ export class WorkflowTrajectory {

try {
const activeDir = path.join(this.dataDir, 'active');
const completedDir = path.join(this.dataDir, 'completed');
// Match the canonical `agent-trajectories` layout: completed files
// live under `completed/YYYY-MM/` based on completedAt (falling back
// to startedAt). Before this change we wrote to the flat
// `completed/` root, which worked but diverged from what
// `FileStorage.save()` produces and forced the reader to grow a
// legacy-layout fallback. Aligning the writer lets the reader shed
// that branch over time.
const bucketSource = this.trajectory.completedAt ?? this.trajectory.startedAt;
const bucketDate = new Date(bucketSource);
const monthBucket = `${bucketDate.getUTCFullYear()}-${String(bucketDate.getUTCMonth() + 1).padStart(
2,
'0'
)}`;
const completedDir = path.join(this.dataDir, 'completed', monthBucket);
Comment on lines +787 to +793
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 13, 2026

Choose a reason for hiding this comment

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

🔴 Completed trajectory path change breaks readCompletedTrajectoryFile in workflow-runner tests

The new moveToCompleted() writes files to completed/YYYY-MM/{id}.json instead of completed/{id}.json, but readCompletedTrajectoryFile in packages/sdk/src/__tests__/workflow-runner.test.ts:204-211 still does a flat readdirSync(completedDir) looking for .json files. It will only find the YYYY-MM subdirectory (not a .json file), return null, and the test at packages/sdk/src/__tests__/workflow-runner.test.ts:848 will crash with Cannot read properties of null (reading 'chapters'). The same pattern affects readLatestTrajectoryFile in tests/integration/broker/utils/workflow-harness.ts:278-302 (flat readdirSync) and the hardcoded flat path assertion at tests/integration/broker/trajectory.test.ts:95.

Prompt for agents 
The moveToCompleted() method was changed to write completed trajectory files to completed/YYYY-MM/ subdirectories, but three existing readers of completed trajectories were not updated to walk subdirectories:

1. packages/sdk/src/__tests__/workflow-runner.test.ts (lines 204-211): readCompletedTrajectoryFile does a flat readdirSync on .trajectories/completed/ and filters for .json files. It needs to walk subdirectories like the updated findCompletedTrajectoryJson in workflow-trajectory.test.ts.

2. tests/integration/broker/utils/workflow-harness.ts (lines 278-302): readLatestTrajectoryFile does the same flat scan. When getTrajectory (line 208) passes .trajectories/completed/, it won't find files in YYYY-MM subdirectories.

3. tests/integration/broker/trajectory.test.ts (line 95): Constructs completedPath as .trajectories/completed/{id}.json without the YYYY-MM bucket, so the fs.existsSync check will always fail.

All three need to be updated to handle the new YYYY-MM subdirectory layout, similar to how findCompletedTrajectoryJson was implemented in workflow-trajectory.test.ts.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

await mkdir(completedDir, { recursive: true });

const activePath = path.join(activeDir, `${this.trajectory.id}.json`);
Expand Down
Loading