diff --git a/package.json b/package.json index 38df068..5ae8c08 100644 --- a/package.json +++ b/package.json @@ -48,9 +48,7 @@ "type": "git", "url": "https://github.com/AgentWorkforce/trajectories" }, - "files": [ - "dist" - ], + "files": ["dist"], "engines": { "node": ">=20.0.0" }, diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts new file mode 100644 index 0000000..1c474f2 --- /dev/null +++ b/src/cli/commands/doctor.ts @@ -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}`); + } + }); +} diff --git a/src/cli/commands/index.ts b/src/cli/commands/index.ts index 98bd755..3a69b2a 100644 --- a/src/cli/commands/index.ts +++ b/src/cli/commands/index.ts @@ -16,6 +16,7 @@ * - 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"; @@ -23,6 +24,7 @@ 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"; @@ -46,4 +48,5 @@ export function registerCommands(program: Command): void { registerExportCommand(program); registerEnableCommand(program); registerCompactCommand(program); + registerDoctorCommand(program); } diff --git a/src/cli/commands/status.ts b/src/cli/commands/status.ts index 103c576..6b21f6a 100644 --- a/src/cli/commands/status.ts +++ b/src/cli/commands/status.ts @@ -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) { diff --git a/src/storage/file.ts b/src/storage/file.ts index 573172d..7fc7c8a 100644 --- a/src/storage/file.ts +++ b/src/storage/file.ts @@ -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, @@ -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; @@ -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); } /** @@ -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(); @@ -208,6 +261,7 @@ export class FileStorage implements StorageAdapter { skippedMalformedJson: 0, skippedSchemaViolation: 0, skippedIoError: 0, + failures: [], }; await withIndexLock(this.indexPath, async () => { @@ -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; @@ -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); + } 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 { + 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. diff --git a/tests/storage/reconcile-real-data.test.ts b/tests/storage/reconcile-real-data.test.ts index 8021506..f72bcdc 100644 --- a/tests/storage/reconcile-real-data.test.ts +++ b/tests/storage/reconcile-real-data.test.ts @@ -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"); + }); });