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
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@
"type": "git",
"url": "https://github.com/AgentWorkforce/trajectories"
},
"files": [
"dist"
],
"files": ["dist"],
"engines": {
"node": ">=20.0.0"
},
Expand Down
63 changes: 63 additions & 0 deletions src/cli/commands/doctor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* trail doctor command
*
* Diagnose and (optionally) repair trajectory files that fail to load.
* Reconcile silently skips bad files so the CLI keeps working; doctor
* surfaces the path + first validation error for each one and can move
* them into `.trajectories/invalid/` so reconcile stops complaining.
*/

import type { Command } from "commander";
import { FileStorage } from "../../storage/file.js";

export function registerDoctorCommand(program: Command): void {
program
.command("doctor")
.description(
"List trajectory files that fail to load; optionally quarantine them",
)
.option(
"--quarantine",
"Move invalid files to .trajectories/invalid/ so reconcile stops scanning them",
)
.action(async (opts: { quarantine?: boolean }) => {
const storage = new FileStorage();
await storage.initialize();

const summary = storage.getLastReconcileSummary();
const failures = summary?.failures ?? [];

if (failures.length === 0) {
console.log("No invalid trajectory files found.");
return;
}

console.log(`Found ${failures.length} invalid trajectory file(s):`);
for (const failure of failures) {
console.log(` ${failure.path}`);
console.log(` reason: ${failure.reason}`);
console.log(` detail: ${failure.message}`);
}

if (!opts.quarantine) {
console.log(
"\nRun `trail doctor --quarantine` to move these files into .trajectories/invalid/.",
);
return;
}

const result = await storage.quarantineInvalid();
if (result.moved.length === 0) {
console.log(
"\nNo files were moved (io_error failures are not auto-quarantined).",
);
return;
}
console.log(
`\nMoved ${result.moved.length} file(s) to ${result.targetDir}:`,
);
for (const failure of result.moved) {
console.log(` ${failure.path}`);
}
});
}
3 changes: 3 additions & 0 deletions src/cli/commands/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
* - enable: Install git hook for trajectory trailers
* - disable: Remove the trajectory git hook
* - compact: Compress trajectories into summarized form
* - doctor: Diagnose and quarantine trajectory files that fail to load
*/

import type { Command } from "commander";
import { registerAbandonCommand } from "./abandon.js";
import { registerCompactCommand } from "./compact.js";
import { registerCompleteCommand } from "./complete.js";
import { registerDecisionCommand } from "./decision.js";
import { registerDoctorCommand } from "./doctor.js";
import { registerEnableCommand } from "./enable.js";
import { registerExportCommand } from "./export.js";
import { registerListCommand } from "./list.js";
Expand All @@ -46,4 +48,5 @@ export function registerCommands(program: Command): void {
registerExportCommand(program);
registerEnableCommand(program);
registerCompactCommand(program);
registerDoctorCommand(program);
}
27 changes: 26 additions & 1 deletion src/cli/commands/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,35 @@ export function registerStatusCommand(program: Command): void {
program
.command("status")
.description("Show active trajectory status")
.action(async () => {
.option(
"-v, --verbose",
"Show paths and validation errors for any trajectory files that failed to load",
)
.action(async (opts: { verbose?: boolean }) => {
const storage = new FileStorage();
await storage.initialize();

if (opts.verbose) {
const summary = storage.getLastReconcileSummary();
const failures = summary?.failures ?? [];
if (failures.length === 0) {
console.log("All trajectory files loaded cleanly.");
} else {
console.log(
`Skipped ${failures.length} trajectory file(s) during reconcile:`,
);
for (const failure of failures) {
console.log(` ${failure.path}`);
console.log(` reason: ${failure.reason}`);
console.log(` detail: ${failure.message}`);
}
console.log(
"\nRun `trail doctor --quarantine` to move them aside and silence this warning.",
);
}
console.log("");
}

const active = await storage.getActive();

if (!active) {
Expand Down
149 changes: 147 additions & 2 deletions src/storage/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
unlink,
writeFile,
} from "node:fs/promises";
import { join } from "node:path";
import { basename, dirname, isAbsolute, join, relative } from "node:path";
import type { z } from "zod";
import { validateTrajectory } from "../core/schema.js";
import type {
Trajectory,
Expand Down Expand Up @@ -92,10 +93,32 @@ export type ReadTrajectoryResult =
error: unknown;
};

/**
* Reason a single trajectory file was skipped during reconcile.
*/
export type ReconcileFailureReason =
| "malformed_json"
| "schema_violation"
| "io_error";

/**
* Per-file failure record for a skipped trajectory. `message` is a
* pre-rendered, single-line description suitable for direct display in
* CLI output — callers should not have to know about Zod or fs error
* shapes to render diagnostics.
*/
export interface ReconcileFailure {
path: string;
reason: ReconcileFailureReason;
message: string;
}

/**
* Aggregated counts emitted by reconcileIndex for observability. Exposed
* on the return value so tests and callers can assert on counts without
* parsing log output.
* parsing log output. `failures` carries the per-file detail so a CLI
* doctor or `--verbose` mode can print actionable info without re-walking
* the directory tree.
*/
export interface ReconcileSummary {
scanned: number;
Expand All @@ -104,6 +127,35 @@ export interface ReconcileSummary {
skippedMalformedJson: number;
skippedSchemaViolation: number;
skippedIoError: number;
failures: ReconcileFailure[];
}

/**
* Render a read failure into a single-line, human-readable message.
* Knows enough about Zod and Node's fs errors to extract the most
* useful field; falls back to String(error) for anything else.
*/
function describeReadFailure(
reason: ReconcileFailureReason,
error: unknown,
): string {
if (
reason === "schema_violation" &&
error &&
typeof error === "object" &&
"issues" in error
) {
const issues = (error as z.ZodError).issues ?? [];
if (issues.length > 0) {
const first = issues[0];
const where = first.path.length > 0 ? first.path.join(".") : "root";
const extra = issues.length > 1 ? ` (+${issues.length - 1} more)` : "";
return `${where}: ${first.message}${extra}`;
}
return "schema validation failed";
}
if (error instanceof Error) return error.message;
return String(error);
}

/**
Expand Down Expand Up @@ -144,6 +196,7 @@ export class FileStorage implements StorageAdapter {
private activeDir: string;
private completedDir: string;
private indexPath: string;
private lastReconcileSummary?: ReconcileSummary;

constructor(baseDir?: string) {
this.baseDir = baseDir ?? process.cwd();
Expand Down Expand Up @@ -208,6 +261,7 @@ export class FileStorage implements StorageAdapter {
skippedMalformedJson: 0,
skippedSchemaViolation: 0,
skippedIoError: 0,
failures: [],
};

await withIndexLock(this.indexPath, async () => {
Expand Down Expand Up @@ -243,6 +297,11 @@ export class FileStorage implements StorageAdapter {
} else {
summary.skippedIoError += 1;
}
summary.failures.push({
path: result.path,
reason: result.reason,
message: describeReadFailure(result.reason, result.error),
});
continue;
}
const trajectory = result.trajectory;
Expand Down Expand Up @@ -286,9 +345,95 @@ export class FileStorage implements StorageAdapter {
console.warn(`[trajectories] ${parts.join(", ")}`);
}

this.lastReconcileSummary = summary;
return summary;
}

/**
* Returns the most recent reconcile summary, if any. Lets the CLI
* inspect the failures collected during `initialize()` without having
* to re-walk the directory tree (and re-emit the warn line).
*/
getLastReconcileSummary(): ReconcileSummary | undefined {
return this.lastReconcileSummary;
}

/**
* Move trajectory files that fail to load into `.trajectories/invalid/`
* so reconcile no longer scans them. Only quarantines parse and schema
* failures — transient io_error failures are left in place because the
* file may load fine on the next attempt.
*
* Returns the list of files that were moved (with their original paths
* and the destination directory) so the caller can report what changed.
*/
async quarantineInvalid(): Promise<{
moved: ReconcileFailure[];
targetDir: string;
}> {
const summary = await this.reconcileIndex();
const targetDir = join(this.trajectoriesDir, "invalid");
const candidates = summary.failures.filter((f) => f.reason !== "io_error");
if (candidates.length === 0) {
return { moved: [], targetDir };
}
await mkdir(targetDir, { recursive: true });
const moved: ReconcileFailure[] = [];
for (const failure of candidates) {
const dest = await this.resolveQuarantineDest(failure.path, targetDir);
try {
await mkdir(dirname(dest), { recursive: true });
await rename(failure.path, dest);
moved.push(failure);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} catch (error) {
// Skip and surface — never silently lose a file. The doctor
// command rolls these into its output so the user can intervene.
console.warn(
`[trajectories] failed to quarantine ${failure.path}: ${error instanceof Error ? error.message : String(error)}`,
);
}
}
return { moved, targetDir };
}

/**
* Pick a destination path under `targetDir` for a quarantined file.
*
* Preserves the file's relative location under the trajectories root
* (e.g. `completed/2026-04/foo.json` → `invalid/completed/2026-04/foo.json`)
* so two invalid files that share a basename across `active/` and
* `completed/` don't collapse onto each other and silently overwrite.
*
* Falls back to a numeric-suffix scheme for paths that live outside
* the trajectories directory or that, after relative resolution, would
* still collide with something already quarantined.
*/
private async resolveQuarantineDest(
sourcePath: string,
targetDir: string,
): Promise<string> {
const rel = relative(this.trajectoriesDir, sourcePath);
const safeRel =
rel && !rel.startsWith("..") && !isAbsolute(rel)
? rel
: basename(sourcePath);
let dest = join(targetDir, safeRel);
if (!existsSync(dest)) return dest;

// Defensive: someone (or a previous quarantine run) already put a
// file at this path. Append an incrementing suffix so we still keep
// both copies for inspection.
const ext = safeRel.endsWith(".json") ? ".json" : "";
const stem = ext ? safeRel.slice(0, -ext.length) : safeRel;
for (let i = 1; i < 1000; i += 1) {
dest = join(targetDir, `${stem}.${i}${ext}`);
if (!existsSync(dest)) return dest;
}
// 1000 collisions on the same basename is pathological; fall through
// with the last candidate so rename surfaces the EEXIST itself.
return dest;
}

/**
* Recursively collect all .json file paths under `dir` into `out`.
* Silently treats a missing directory as empty.
Expand Down
57 changes: 57 additions & 0 deletions tests/storage/reconcile-real-data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,61 @@ describe("FileStorage reconcile — real workforce fixtures", () => {
expect(summary.skippedMalformedJson).toBe(0);
expect(summary.skippedSchemaViolation).toBe(1);
});

it("quarantineInvalid preserves both files when basenames collide across dirs", async () => {
// Regression: an earlier version of quarantineInvalid used basename(),
// so two invalid files at active/foo.json and completed/.../foo.json
// collapsed onto the same destination and one was silently lost.
const { FileStorage } = await import("../../src/storage/file.js");
const { writeFile, readdir } = await import("node:fs/promises");

const trajRoot = join(tempDir, ".trajectories");
await mkdir(join(trajRoot, "active"), { recursive: true });
await mkdir(join(trajRoot, "completed", "2026-04"), { recursive: true });

// Same basename, different parent dirs, both schema-invalid.
const invalidPayload = JSON.stringify({
id: "traj_dup00000_0000",
version: 1,
});
await writeFile(
join(trajRoot, "active", "traj_dup00000_0000.json"),
invalidPayload,
"utf-8",
);
await writeFile(
join(trajRoot, "completed", "2026-04", "traj_dup00000_0000.json"),
invalidPayload,
"utf-8",
);

const storage = new FileStorage(tempDir);
const result = await storage.quarantineInvalid();

expect(result.moved).toHaveLength(2);

// Both files should now live under invalid/, with the relative path
// preserved so neither overwrites the other.
const activeQuarantined = join(
trajRoot,
"invalid",
"active",
"traj_dup00000_0000.json",
);
const completedQuarantined = join(
trajRoot,
"invalid",
"completed",
"2026-04",
"traj_dup00000_0000.json",
);
const activeContent = await readFile(activeQuarantined, "utf-8");
const completedContent = await readFile(completedQuarantined, "utf-8");
expect(activeContent).toBe(invalidPayload);
expect(completedContent).toBe(invalidPayload);

// And the originals must be gone from active/ and completed/.
const activeAfter = await readdir(join(trajRoot, "active"));
expect(activeAfter).not.toContain("traj_dup00000_0000.json");
});
Comment on lines +237 to +240
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert source removal in completed/2026-04 as well

The intent says both originals are removed, but only active/ is checked (Line 238). Please also assert that completed/2026-04/traj_dup00000_0000.json is gone.

Patch suggestion
     // And the originals must be gone from active/ and completed/.
     const activeAfter = await readdir(join(trajRoot, "active"));
     expect(activeAfter).not.toContain("traj_dup00000_0000.json");
+    const completedAfter = await readdir(join(trajRoot, "completed", "2026-04"));
+    expect(completedAfter).not.toContain("traj_dup00000_0000.json");
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// And the originals must be gone from active/ and completed/.
const activeAfter = await readdir(join(trajRoot, "active"));
expect(activeAfter).not.toContain("traj_dup00000_0000.json");
});
// And the originals must be gone from active/ and completed/.
const activeAfter = await readdir(join(trajRoot, "active"));
expect(activeAfter).not.toContain("traj_dup00000_0000.json");
const completedAfter = await readdir(join(trajRoot, "completed", "2026-04"));
expect(completedAfter).not.toContain("traj_dup00000_0000.json");
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/storage/reconcile-real-data.test.ts` around lines 237 - 240, The test
currently only asserts the original file was removed from active/ using the
variable activeAfter; add a corresponding check for completed/2026-04 by reading
its directory (e.g., a new completedAfter variable via readdir(join(trajRoot,
"completed", "2026-04"))) and assert completedAfter does not contain
"traj_dup00000_0000.json" so both active and completed removals are verified in
tests/storage/reconcile-real-data.test.ts.

});
Loading