From 1e29d112a6e209633b1c159c57b38d2fdfca858f Mon Sep 17 00:00:00 2001 From: Khaliq Date: Mon, 13 Apr 2026 11:45:17 +0200 Subject: [PATCH] fix(sdk): align WorkflowTrajectory writer with canonical layout + fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK's WorkflowTrajectory is a standalone writer that emits trajectory JSON without using the `agent-trajectories` library. Its output diverged from the canonical layout in two visible ways: 1. Completed files landed in a flat `completed/` root instead of `completed/YYYY-MM/{id}.json`, forcing downstream readers to grow legacy-layout fallbacks. 2. Top-level `commits`, `filesChanged`, and `tags` arrays were omitted, even though the canonical schema declares them. Readers had to default them at load time. These are now aligned: - `moveToCompleted()` computes a `YYYY-MM` bucket from `completedAt` (falling back to `startedAt`) and writes into `completed//{id}.json`. - `TrajectoryFile` interface declares `commits: string[]`, `filesChanged: string[]`, `tags: string[]`, and `start()` initializes them to empty arrays. Canonical output is defense-in-depth — `agent-trajectories` PR #22 (AgentWorkforce/trajectories#22) makes the reader tolerant of the prior shapes, so this change is not urgent. It does let the reader shed those fallbacks over time. Test coverage: - Existing `readCompletedTrajectoryFile` helper now walks completed/ recursively so tests don't hardcode the bucket name. - New: asserts the completed file path has exactly one `YYYY-MM` intermediate directory. - New: asserts `commits`/`filesChanged`/`tags` are populated as [] on `start()`. All 29 tests in `workflow-trajectory.test.ts` pass. Diff scoped to two files; unrelated full-suite failures (verification.test.ts, step-executor, etc.) are pre-existing and out of scope. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/workflow-trajectory.test.ts | 73 ++++++++++++++----- packages/sdk/src/workflows/trajectory.ts | 27 ++++++- 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/packages/sdk/src/__tests__/workflow-trajectory.test.ts b/packages/sdk/src/__tests__/workflow-trajectory.test.ts index f9fe21361..9c19b6558 100644 --- a/packages/sdk/src/__tests__/workflow-trajectory.test.ts +++ b/packages/sdk/src/__tests__/workflow-trajectory.test.ts @@ -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 ──────────────────────────────────────────────────────────────────── @@ -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 ──────────────────────────────────────────────────────── @@ -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 @@ -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); @@ -175,7 +213,7 @@ describe('WorkflowTrajectory', () => { { name: 'deploy', agent: 'deployer', task: 'Deploy' }, 'Connection refused', 1, - 3, + 3 ); const data = readTrajectoryFile(tmpDir); @@ -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); diff --git a/packages/sdk/src/workflows/trajectory.ts b/packages/sdk/src/workflows/trajectory.ts index eef05eb77..bccf4d991 100644 --- a/packages/sdk/src/workflows/trajectory.ts +++ b/packages/sdk/src/workflows/trajectory.ts @@ -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 ───────────────────────────────────────────────── @@ -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 @@ -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); await mkdir(completedDir, { recursive: true }); const activePath = path.join(activeDir, `${this.trajectory.id}.json`);