From 78b5de8fda9161fbd6e1beaf95ec0fb8ba95bee5 Mon Sep 17 00:00:00 2001 From: tintinweb Date: Wed, 22 Jul 2026 18:26:18 +0200 Subject: [PATCH] fix: normalize legacy task records on load; guard widget render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task files written before the blocking feature have no blockedBy/blocks/ metadata. TaskStore.load() deserialized them as-is, so consumers reading those fields unguarded (e.g. task.blockedBy.length in the widget) threw a TypeError. Because the widget renders on a pi-tui timer, the throw was uncaught and killed the whole pi process on the first render after upgrade. - Normalize every record at the load() boundary (normalizeTask): default metadata/blocks/blockedBy, with type guards for hand-edited files. This is the single fix point — the same unguarded accesses exist across the store and index mutation paths, not just the widget. - Wrap the widget render in try/catch returning a safe fallback, so a render error can never again escape to the TUI timer and crash the host. Fixes #33. Normalization approach based on #30 (thanks @arrokh); reported by @eSaadster. Co-authored-by: Noor Octavian (Alvin) Anwar --- src/task-store.ts | 21 ++++++++++++++++++++- src/ui/task-widget.ts | 13 ++++++++++++- test/task-store.test.ts | 28 +++++++++++++++++++++++++++- test/task-widget.test.ts | 14 ++++++++++++++ 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/task-store.ts b/src/task-store.ts index 9598d4a..e0e872d 100644 --- a/src/task-store.ts +++ b/src/task-store.ts @@ -71,6 +71,25 @@ function isProcessRunning(pid: number): boolean { try { process.kill(pid, 0); return true; } catch { return false; } } +/** + * Fill defaults for tasks persisted by older versions. Task files written + * before the blocking feature have no `blockedBy`/`blocks`/`metadata`, so + * consumers that read those fields unguarded (e.g. `task.blockedBy.length`) + * would throw. Normalizing at the load boundary lets every consumer trust the + * shape. Also guards against wrong types in hand-edited files. + */ +function normalizeTask(t: Task): Task { + const now = Date.now(); + return { + ...t, + metadata: t.metadata && typeof t.metadata === "object" && !Array.isArray(t.metadata) ? t.metadata : {}, + blocks: Array.isArray(t.blocks) ? t.blocks : [], + blockedBy: Array.isArray(t.blockedBy) ? t.blockedBy : [], + createdAt: typeof t.createdAt === "number" ? t.createdAt : now, + updatedAt: typeof t.updatedAt === "number" ? t.updatedAt : now, + }; +} + export class TaskStore { private filePath: string | undefined; private lockPath: string | undefined; @@ -98,7 +117,7 @@ export class TaskStore { this.nextId = data.nextId; this.tasks.clear(); for (const t of data.tasks) { - this.tasks.set(t.id, t); + this.tasks.set(t.id, normalizeTask(t)); } } catch { /* corrupt file — start fresh */ } } diff --git a/src/ui/task-widget.ts b/src/ui/task-widget.ts index 41856c8..69bbbf7 100644 --- a/src/ui/task-widget.ts +++ b/src/ui/task-widget.ts @@ -134,8 +134,19 @@ export class TaskWidget { } } - /** Build widget lines from current live state. Called from the render callback. */ + /** Render callback entry point. Guarded so a render error can never escape to + * the TUI timer and crash the whole host process — worst case the widget is + * empty for one frame. */ private renderWidget(tui: any, theme: Theme): string[] { + try { + return this.buildWidgetLines(tui, theme); + } catch { + return []; + } + } + + /** Build widget lines from current live state. */ + private buildWidgetLines(tui: any, theme: Theme): string[] { const sortOrder = this.config.sortOrder ?? "id"; const tasks = this.store.list(sortOrder); const w = tui.terminal.columns; diff --git a/test/task-store.test.ts b/test/task-store.test.ts index a4455c6..acb9811 100644 --- a/test/task-store.test.ts +++ b/test/task-store.test.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -474,4 +474,30 @@ describe("TaskStore (absolute path)", () => { rmSync(parentDir, { recursive: true, force: true }); }); + + it("normalizes legacy task records missing blockedBy/blocks/metadata on load", () => { + const dir = join(tmpdir(), `pi-tasks-legacy-${process.pid}-${Date.now()}`); + const filePath = join(dir, "tasks.json"); + mkdirSync(dir, { recursive: true }); + try { + // A task file written before the blocking feature — no blockedBy/blocks/metadata. + writeFileSync(filePath, JSON.stringify({ + nextId: 2, + tasks: [{ + id: "1", + subject: "Legacy task", + description: "From an older version", + status: "pending", + }], + })); + + const task = new TaskStore(filePath).get("1")!; + expect(task.blockedBy).toEqual([]); + expect(task.blocks).toEqual([]); + expect(task.metadata).toEqual({}); + expect(task.subject).toBe("Legacy task"); // existing fields preserved + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/test/task-widget.test.ts b/test/task-widget.test.ts index 6332768..3272bac 100644 --- a/test/task-widget.test.ts +++ b/test/task-widget.test.ts @@ -134,6 +134,20 @@ describe("TaskWidget", () => { expect(blockedLine).not.toContain("blocked by"); }); + it("does not crash the host when a task is missing legacy fields", () => { + // Simulate a record persisted before the blocking feature — no blockedBy. + // A throw here would escape to the TUI timer and kill the whole pi process, + // so the render must never throw (the guard returns a safe fallback). + store.create("Legacy pending", "Desc"); + const raw = store.get("1") as any; + delete raw.blockedBy; + delete raw.blocks; + delete raw.metadata; + widget.update(); + + expect(() => renderWidget(ui.state)).not.toThrow(); + }); + it("shows status summary in header", () => { store.create("Task A", "Desc"); store.create("Task B", "Desc");