refactor(sdk): unify WorkflowTrajectory on agent-trajectories SDK#732
Merged
khaliqgant merged 3 commits intomainfrom Apr 13, 2026
Merged
refactor(sdk): unify WorkflowTrajectory on agent-trajectories SDK#732khaliqgant merged 3 commits intomainfrom
khaliqgant merged 3 commits intomainfrom
Conversation
Collapse the hand-rolled trajectory writer in packages/sdk/src/workflows/
trajectory.ts onto the agent-trajectories SDK. The SDK writer was a parallel
implementation of the same on-disk format, with its own types, filesystem I/O,
and chapter bookkeeping — a steady source of drift against the canonical
reader/writer in agent-trajectories.
Why:
- Two implementations of the same trajectory schema means two places to fix
bugs, two places to evolve the schema, and inevitable skew over time.
- The SDK duplicated type shadows, mkdir/writeFile/rename plumbing, and
chapter open/close state that agent-trajectories already owns.
- One source of truth for trajectory persistence lets downstream tooling
(viewer, reconcile, compaction) trust a single format.
What changed (trajectory.ts: 779 → 314 lines, -465):
- Deleted: local Chapter/TrajectoryAgent/TrajectoryEvent type shadows,
hand-rolled flush()/moveToCompleted() filesystem I/O, manual chapter state
tracking, and ID generation.
- Kept: public WorkflowTrajectory API, workflow event shapes, chapter title
semantics, failure classification / diagnosis / synthesis helpers, and the
non-blocking guard() boundary (all workflow semantics are preserved).
- Added: agent-trajectories as a runtime dependency; the adapter now drives
TrajectoryBuilder / core functions + FileStorage directly.
- Tests: workflow-trajectory.test.ts updated for the new persistence path;
31/31 passing.
Chosen approach is the hybrid (B-leaning) plan from
workflows/unify-sdk-trajectory-writer/DESIGN.md — keep Proposal B's semantic
conservatism (public API, event strings, non-blocking boundary) but take
Proposal A's deletion targets (type shadows, hand-rolled I/O).
Verification — all 5 gates green:
1. structural: imports agent-trajectories ✓
2. quantitative: 314 ≤ 400 line budget ✓
3. build: tsc -p tsconfig.build.json ✓
4. unit: 31/31 vitest tests passing ✓
5. 80→100 round-trip: trajectory created, flushed, moved to completed,
re-read via FileStorage — id/title/status/chapters/agents all match ✓
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The unification in 95eafc4 relied on looser types from agent-trajectories (role as open string, expanded TrajectoryEventType, relaxed id regex). Those types only existed locally on fix/reconcile-stale-index when the worker implemented the PR, so CI (which installs from npm) hit 5 TS errors against the published 0.5.3. agent-trajectories@0.5.4 is now published with those looser types (AgentWorkforce/trajectories#22). Bumping: - packages/sdk/package.json: ^0.5.3 -> ^0.5.4 - root package.json (devDependencies): ^0.4.1 -> ^0.5.4 - package-lock.json regenerated via `npm install --package-lock-only` Verified locally: - `npm run build` in packages/sdk: clean tsc -p tsconfig.build.json - `npx vitest run src/__tests__/workflow-trajectory.test.ts`: 31 passed This should turn CI green on PR #732. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous bump in 5411666 placed agent-trajectories@^0.5.4 in the root devDependencies, which tripped scripts/audit-bundled-deps.mjs in CI: ❌ MISSING DEPENDENCIES "agent-trajectories": "^0.5.4", // used by: sdk @agent-relay/sdk is a bundled sub-package inside the root agent-relay tarball. When a user runs `npm install -g agent-relay`, only the root `dependencies` (not `devDependencies`) are installed as runtime deps. Since the SDK now imports agent-trajectories at runtime (the unified WorkflowTrajectory wraps TrajectoryClient from agent-trajectories/sdk), it must be a runtime dep of the root, not a dev dep. Move agent-trajectories from devDependencies to dependencies and regenerate the lock file. Local audit now passes: ✅ All bundled package dependencies are properly hoisted. Bundled packages: 9 External dependencies found: 15 Hoisted to root: 15 Missing from root: 0 Build + unit tests still green (31/31 in workflow-trajectory.test.ts). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+173
to
+174
| const dataDir = process.env.TRAJECTORIES_DATA_DIR ?? join(cwd, '.trajectories'); | ||
| this.storageBaseDir = process.env.TRAJECTORIES_DATA_DIR ? dirname(dataDir) : cwd; |
Contributor
There was a problem hiding this comment.
🟡 TRAJECTORIES_DATA_DIR contract silently broken when path doesn't end with .trajectories
The old code used TRAJECTORIES_DATA_DIR directly as the parent directory for active/ and completed/ subdirectories (e.g., TRAJECTORIES_DATA_DIR=/custom/path → writes to /custom/path/active/). The new code applies dirname() to it and passes the result to FileStorage, which internally re-appends .trajectories. This means the env var now implicitly requires the path to end with .trajectories — otherwise data is silently written to the wrong location.
For example, if TRAJECTORIES_DATA_DIR=/data/my-trajectories, old behavior wrote to /data/my-trajectories/active/, but new behavior writes to /data/.trajectories/active/. The test at packages/sdk/src/__tests__/workflow-trajectory.test.ts:174 only covers the happy path where the env var ends with .trajectories, so the regression isn't caught.
Prompt for agents
In the constructor of WorkflowTrajectory (trajectory.ts), the computation of storageBaseDir assumes that TRAJECTORIES_DATA_DIR always ends with .trajectories so that dirname() strips it and FileStorage re-adds it. The old code used TRAJECTORIES_DATA_DIR directly as the data directory (writing active/ and completed/ under it). The new code breaks this contract when TRAJECTORIES_DATA_DIR doesn't follow the .trajectories naming convention.
Consider either:
1. Documenting the requirement that TRAJECTORIES_DATA_DIR must end with .trajectories, OR
2. Accepting a separate TRAJECTORIES_BASE_DIR env var that points to the parent of .trajectories, OR
3. Checking at runtime whether the last path component is .trajectories and handling both cases.
Relevant code: packages/sdk/src/workflows/trajectory.ts constructor, lines 173-174.
Relevant test: packages/sdk/src/__tests__/workflow-trajectory.test.ts lines 173-181.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Collapses
packages/sdk/src/workflows/trajectory.tsfrom 779 → 314 lines (-465) by deleting the hand-rolled trajectory writer and delegating persistence to the canonicalagent-trajectoriesSDK. All workflow semantics (public API, event shapes, chapter titles, non-blocking boundary) are preserved.Why
The SDK shipped a parallel implementation of the same on-disk trajectory format — its own type shadows, its own
mkdir/writeFile/renameplumbing, its own chapter bookkeeping, its own ID generation. That meant two places to fix bugs, two places to evolve the schema, and inevitable drift against the reader/writer inagent-trajectories. Downstream tooling (viewer, reconcile, compaction) can now trust a single source of truth for trajectory persistence.Changes
packages/sdk/src/workflows/trajectory.ts(779 → 314 lines)Deleted:
Chapter/TrajectoryAgent/TrajectoryEventtype shadowsflush()/moveToCompleted()filesystem I/OKept:
WorkflowTrajectoryAPI (no caller changes)classifyFailure/diagnosisFor/ synthesis helpers (workflow-specific)guard()try/catch boundary — the non-blocking invariant (persistence failures must never break the workflow)Added:
agent-trajectoriesas a runtime dependency inpackages/sdk/package.jsoncreateTrajectory/completeTrajectory+FileStoragedirectlypackages/sdk/src/__tests__/workflow-trajectory.test.ts— updated for the new persistence path; 31/31 passing.Design
Chosen approach is the hybrid (B-leaning) plan from
workflows/unify-sdk-trajectory-writer/DESIGN.md— take Proposal B's semantic conservatism (keep public API, event strings, non-blocking boundary) but take Proposal A's deletion targets (type shadows, hand-rolled I/O).Verification
All 5 gates green (
verify-80-to-100):trajectory.tsimportsagent-trajectories✓tsc -p tsconfig.build.json✓completed/, re-read viaFileStorage— id, title, status, chapter count, and agent count all match ✓Companion PRs
Test plan
fix/sdk-unify-trajectory-writerpnpm --filter @agent-relay/sdk buildsucceedspnpm --filter @agent-relay/sdk test— 31/31 workflow-trajectory tests pass.trajectories/and moved tocompleted/with the expected shape🤖 Generated with Claude Code