fix(#19): vault all skips broken cache entries instead of aborting (#25)

* fix(#19): vault all skips broken cache entries instead of aborting

exportAllPapersToVault used Effect.forEach in fail-fast mode, so the
first cache entry missing paper.md aborted the whole batch — every
valid entry after it was silently dropped. Building any non-trivial
vault (10+ papers) reliably hit one stale entry and forced the user to
script around the command per-paper.

Each entry now runs through an isolated catch and produces either an
ok or failed attempt record. The command prints a summary

  Exported N of M papers to <vault>
  Skipped:
    <id> — <reason>

and exits 0 as long as at least one paper landed. Only when every
entry failed (and the cache wasn't empty to begin with) does the
command fail with a new VaultExportAllFailed error and exit 1.

Closes #19.

* fix(#19): narrow vault all skip boundary to typed cache errors

Replace the broad Effect.catch in exportAllPapersToVault with
Effect.catchTags({ VaultCacheMissing, VaultCacheMalformed }). Only
per-entry cache failures (missing paper.md, unsafe target path,
unparseable cached id) are converted to skipped entries; VaultFsError,
VaultInvalidPath, VaultConfigMissing now keep failing the whole
command instead of being reported as a skipped paper.

ExportAttempt and VaultExportFailure hold the typed error object until
the CLI renderer turns it into a display line. The visible 'Skipped:
<id> — <message>' format is unchanged.

Also drop the redundant Effect.fail(new VaultExportAllFailed(...)) in
favour of bare 'yield* new VaultExportAllFailed(...)' (Data.TaggedError
instances are themselves Effects), matching the existing codebase
convention.

Author: Vigtu

src/cli.ts

@@ -704,6 +704,8 @@ const formatVaultError = (error: VaultError): string => {
       return `error: vault export failed: ${error.message}`
     case "VaultFsError":
       return `error: vault export failed: ${error.message}`
+    case "VaultExportAllFailed":
+      return `error: vault export failed: ${error.message}`
   }
 }
 

src/commands/vault.ts

@@ -1,6 +1,6 @@
 import { Console, Effect } from "effect"
 import { CachePaths } from "../cache.js"
-import { exportAllPapersToVault, exportPaperToVault, initVault, type VaultError, type VaultInitResult, type VaultExportResult, type VaultExportAllResult, VaultPaths } from "../vault.js"
+import { exportAllPapersToVault, exportPaperToVault, initVault, type VaultError, VaultExportAllFailed, type VaultInitResult, type VaultExportResult, type VaultExportAllResult, VaultPaths } from "../vault.js"
 import type { CliCommand } from "../parser.js"
 
 export const runVaultCommand = (
@@ -21,6 +21,9 @@ export const runVaultCommand = (
       case "vault-all": {
         const result = yield* exportAllPapersToVault()
         yield* Console.log(renderVaultExportAll(result))
+        if (result.exported.length === 0 && result.total > 0) {
+          return yield* new VaultExportAllFailed({ message: "all cache entries failed to export" })
+        }
         return
       }
     }
@@ -30,6 +33,14 @@ const renderVaultInit = (result: VaultInitResult): string => `Configured vault:
 
 const renderVaultExport = (result: VaultExportResult): string => `Exported ${result.id} to ${result.path}`
 
-const renderVaultExportAll = (result: VaultExportAllResult): string => `Exported ${result.count} papers to ${result.path}`
+const renderVaultExportAll = (result: VaultExportAllResult): string => {
+  const header = `Exported ${result.exported.length} of ${result.total} papers to ${result.path}`
+  if (result.failed.length === 0) return header
+  return [
+    header,
+    "Skipped:",
+    ...result.failed.map((entry) => `  ${entry.id} — ${entry.error.message}`),
+  ].join("\n")
+}
 
 

src/vault.ts

@@ -36,12 +36,17 @@ export class VaultFsError extends Data.TaggedError("VaultFsError")<{
   readonly cause: unknown
 }> {}
 
+export class VaultExportAllFailed extends Data.TaggedError("VaultExportAllFailed")<{
+  readonly message: string
+}> {}
+
 export type VaultError =
   | VaultConfigMissing
   | VaultInvalidPath
   | VaultCacheMissing
   | VaultCacheMalformed
   | VaultFsError
+  | VaultExportAllFailed
 
 export type VaultInitResult = {
   readonly path: string
@@ -52,8 +57,15 @@ export type VaultExportResult = {
   readonly path: string
 }
 
+export type VaultExportFailure = {
+  readonly id: string
+  readonly error: VaultCacheMissing | VaultCacheMalformed
+}
+
 export type VaultExportAllResult = {
-  readonly count: number
+  readonly exported: ReadonlyArray<VaultExportResult>
+  readonly failed: ReadonlyArray<VaultExportFailure>
+  readonly total: number
   readonly path: string
 }
 
@@ -81,25 +93,49 @@ export const exportPaperToVault = (id: PaperIdentifier): Effect.Effect<VaultExpo
     Effect.flatMap((vaultPath) => exportOne(vaultPath, id))
   )
 
+type ExportAttempt =
+  | { readonly _tag: "Exported"; readonly result: VaultExportResult }
+  | { readonly _tag: "Skipped"; readonly id: string; readonly error: VaultCacheMissing | VaultCacheMalformed }
+
 export const exportAllPapersToVault = (): Effect.Effect<VaultExportAllResult, VaultError, VaultPaths | CachePathsContext> =>
   loadVaultPath().pipe(
     Effect.flatMap((vaultPath) =>
       listCachedPapers().pipe(
         Effect.mapError(cacheToVaultError),
         Effect.flatMap((result) => {
-          const exported = Effect.forEach(result.entries, (entry): Effect.Effect<VaultExportResult, VaultError, CachePathsContext> => {
+          const attempts = Effect.forEach(result.entries, (entry) => {
             const id = parsePaperIdentifier(entry.id)
             if (id === undefined) {
-              return Effect.fail(new VaultCacheMalformed({ message: `invalid cached paper id: ${entry.id}`, id: entry.id }))
+              return Effect.succeed<ExportAttempt>({
+                _tag: "Skipped",
+                id: entry.id,
+                error: new VaultCacheMalformed({ message: `invalid cached paper id: ${entry.id}`, id: entry.id }),
+              })
             }
-            return exportOne(vaultPath, id)
+            return exportOne(vaultPath, id).pipe(
+              Effect.map((written): ExportAttempt => ({ _tag: "Exported", result: written })),
+              Effect.catchTags({
+                VaultCacheMissing: (error) => Effect.succeed<ExportAttempt>({ _tag: "Skipped", id: entry.id, error }),
+                VaultCacheMalformed: (error) => Effect.succeed<ExportAttempt>({ _tag: "Skipped", id: entry.id, error }),
+              }),
+            )
           })
-          return exported.pipe(Effect.map((written) => ({ count: written.length, path: vaultPath })))
+          return attempts.pipe(Effect.map((all) => collectAttempts(all, vaultPath, result.entries.length)))
         })
       )
     )
   )
 
+const collectAttempts = (attempts: ReadonlyArray<ExportAttempt>, vaultPath: string, total: number): VaultExportAllResult => {
+  const exported: Array<VaultExportResult> = []
+  const failed: Array<VaultExportFailure> = []
+  for (const attempt of attempts) {
+    if (attempt._tag === "Exported") exported.push(attempt.result)
+    else failed.push({ id: attempt.id, error: attempt.error })
+  }
+  return { exported, failed, total, path: vaultPath }
+}
+
 const exportOne = (vaultPath: string, id: PaperIdentifier): Effect.Effect<VaultExportResult, VaultError, CachePathsContext> => {
   const formattedId = formatIdentifier(id)
   return CachePaths.use((paths) => readCachedPaper(cacheDirForIdentifierAt(paths.cacheRoot, id), formattedId)).pipe(

tests/vault.test.ts

@@ -97,6 +97,24 @@ const writeEntry = (
     await writeFile(join(dir, "paper.md"), markdown, { encoding: "utf8" })
   })
 
+const writeStaleEntry = (
+  cacheRoot: string,
+  dirname: string,
+  meta: { readonly id: string; readonly title: string }
+) =>
+  Effect.promise(async () => {
+    const dir = join(cacheRoot, dirname)
+    await mkdir(dir, { recursive: true })
+    await writeFile(join(dir, "meta.json"), JSON.stringify(meta), { encoding: "utf8" })
+  })
+
+const writeConfiguredVault = (paths: { readonly configPath: string; readonly vaultPath: string }) =>
+  Effect.promise(async () => {
+    await mkdir(paths.vaultPath, { recursive: true })
+    await mkdir(join(paths.configPath, ".."), { recursive: true })
+    await writeFile(paths.configPath, `PAPER7_VAULT=${paths.vaultPath}\n`, { encoding: "utf8" })
+  })
+
 describe("vault commands", () => {
   it.effect("initializes vault config through Effect CLI", () =>
     withTempVault((paths) => Effect.gen(function*() {
@@ -188,11 +206,7 @@ describe("vault commands", () => {
 
   it.effect("exports all cached papers with path-safe filenames", () =>
     withTempVault((paths) => Effect.gen(function*() {
-      yield* Effect.promise(async () => {
-        await mkdir(paths.vaultPath, { recursive: true })
-        await mkdir(join(paths.configPath, ".."), { recursive: true })
-        await writeFile(paths.configPath, `PAPER7_VAULT=${paths.vaultPath}\n`, { encoding: "utf8" })
-      })
+      yield* writeConfiguredVault(paths)
       yield* writeEntry(paths.cacheRoot, "2401.04088", { id: "2401.04088", title: "Fixture Get Paper" }, "# Fixture Get Paper\n")
       yield* writeEntry(paths.cacheRoot, "pmid-38903003", { id: "pmid:38903003", title: "Fixture PubMed Paper" }, "# Fixture PubMed Paper\n")
       yield* writeEntry(paths.cacheRoot, "doi-10.1101_2023.12.15.571821", { id: "doi:10.1101/2023.12.15.571821", title: "Unsafe / DOI: Paper" }, "# Unsafe / DOI: Paper\n")
@@ -204,9 +218,57 @@ describe("vault commands", () => {
 
       expect(result.exit._tag).toBe("Success")
       expect(result.stderr).toBe("")
-      expect(result.stdout).toBe(`Exported 3 papers to ${paths.vaultPath}`)
+      expect(result.stdout).toBe(`Exported 3 of 3 papers to ${paths.vaultPath}`)
       expect(arxiv).toContain("# Fixture Get Paper")
       expect(pubmed).toContain("# Fixture PubMed Paper")
       expect(doi).toContain("# Unsafe / DOI: Paper")
     })))
+
+  it.effect("vault all skips stale cache entries and exports the rest (issue #19)", () =>
+    withTempVault((paths) => Effect.gen(function*() {
+      yield* writeConfiguredVault(paths)
+      yield* writeEntry(paths.cacheRoot, "2401.04088", { id: "2401.04088", title: "Good Paper One" }, "# Good Paper One\n")
+      yield* writeStaleEntry(paths.cacheRoot, "2402.99999", { id: "2402.99999", title: "Stale Paper" })
+      yield* writeEntry(paths.cacheRoot, "2403.04088", { id: "2403.04088", title: "Good Paper Two" }, "# Good Paper Two\n")
+
+      const result = yield* runRootWith(paths.cacheRoot, paths.configPath, ["vault", "all"])
+      const first = yield* Effect.promise(() => readFile(join(paths.vaultPath, "2401.04088.md"), { encoding: "utf8" }))
+      const third = yield* Effect.promise(() => readFile(join(paths.vaultPath, "2403.04088.md"), { encoding: "utf8" }))
+
+      expect(result.exit._tag).toBe("Success")
+      expect(result.stderr).toBe("")
+      expect(result.stdout).toContain(`Exported 2 of 3 papers to ${paths.vaultPath}`)
+      expect(result.stdout).toContain("Skipped:")
+      expect(result.stdout).toContain("2402.99999")
+      expect(result.stdout).toContain("no cached paper for 2402.99999")
+      expect(first).toContain("# Good Paper One")
+      expect(third).toContain("# Good Paper Two")
+    })))
+
+  it.effect("vault all fails with exit 1 when every cache entry is broken", () =>
+    withTempVault((paths) => Effect.gen(function*() {
+      yield* writeConfiguredVault(paths)
+      yield* writeStaleEntry(paths.cacheRoot, "2401.99999", { id: "2401.99999", title: "Broken One" })
+      yield* writeStaleEntry(paths.cacheRoot, "2402.99999", { id: "2402.99999", title: "Broken Two" })
+
+      const result = yield* runRootWith(paths.cacheRoot, paths.configPath, ["vault", "all"])
+
+      expect(result.exit._tag).toBe("Failure")
+      expect(result.stdout).toContain(`Exported 0 of 2 papers to ${paths.vaultPath}`)
+      expect(result.stdout).toContain("Skipped:")
+      expect(result.stdout).toContain("2401.99999")
+      expect(result.stdout).toContain("2402.99999")
+      expect(result.stderr).toBe("error: vault export failed: all cache entries failed to export")
+    })))
+
+  it.effect("vault all on empty cache succeeds with a zero-count summary", () =>
+    withTempVault((paths) => Effect.gen(function*() {
+      yield* writeConfiguredVault(paths)
+
+      const result = yield* runRootWith(paths.cacheRoot, paths.configPath, ["vault", "all"])
+
+      expect(result.exit._tag).toBe("Success")
+      expect(result.stderr).toBe("")
+      expect(result.stdout).toBe(`Exported 0 of 0 papers to ${paths.vaultPath}`)
+    })))
 })