Follow-up from review of #93.
JsonlCacheStore.loadInto() (src/cache/store.ts) parses every line of pool.jsonl/tenant.jsonl at construction time with a bare JSON.parse(trimmed) — no try/catch:
private loadInto(file: string): void {
if (!existsSync(file)) return;
const text = readFileSync(file, "utf8");
for (const line of text.split("\n")) {
const trimmed = line.trim();
if (!trimmed) continue;
this.index(JSON.parse(trimmed) as CacheRow);
}
}
The existing test (tolerates a trailing newline and blank lines) only covers benign whitespace. A genuinely truncated/partial last line — e.g. process killed mid-appendFileSync, or a disk-full condition — would throw out of the constructor. Because the file is append-only and the bad line is never removed, this isn't a transient failure: every future new JsonlCacheStore({dir}) against that directory throws, permanently blocking the cache until someone manually edits the file.
Not urgent to fix right now: nothing outside src/cache/ and tests constructs a JsonlCacheStore yet (architecture.md stub 7 — the cache read/write path isn't wired into the runtime), so this can't happen in production today. Worth a regression test + a decision (skip-and-warn the bad line vs. surface a clear "corrupt cache, delete <file> to reset" error) before the cache store is wired into a live path.
Follow-up from review of #93.
JsonlCacheStore.loadInto()(src/cache/store.ts) parses every line ofpool.jsonl/tenant.jsonlat construction time with a bareJSON.parse(trimmed)— no try/catch:The existing test (
tolerates a trailing newline and blank lines) only covers benign whitespace. A genuinely truncated/partial last line — e.g. process killed mid-appendFileSync, or a disk-full condition — would throw out of the constructor. Because the file is append-only and the bad line is never removed, this isn't a transient failure: every futurenew JsonlCacheStore({dir})against that directory throws, permanently blocking the cache until someone manually edits the file.Not urgent to fix right now: nothing outside
src/cache/and tests constructs aJsonlCacheStoreyet (architecture.md stub 7 — the cache read/write path isn't wired into the runtime), so this can't happen in production today. Worth a regression test + a decision (skip-and-warn the bad line vs. surface a clear "corrupt cache, delete<file>to reset" error) before the cache store is wired into a live path.