feat(HAI-020): complete Step 2 — simplify safeReadTaskJson to readTaskJson, remove truncation recovery
This commit is contained in:
@@ -90,19 +90,17 @@ describe("TaskStore", () => {
|
|||||||
// ── Defensive parsing test ───────────────────────────────────────
|
// ── Defensive parsing test ───────────────────────────────────────
|
||||||
|
|
||||||
describe("defensive JSON parsing", () => {
|
describe("defensive JSON parsing", () => {
|
||||||
it("recovers from corrupted task.json with trailing duplicate content", async () => {
|
it("throws on corrupted task.json with trailing duplicate content (atomic writes prevent this)", async () => {
|
||||||
const task = await createTestTask();
|
const task = await createTestTask();
|
||||||
const taskJsonPath = join(rootDir, ".hai", "tasks", task.id, "task.json");
|
const taskJsonPath = join(rootDir, ".hai", "tasks", task.id, "task.json");
|
||||||
|
|
||||||
// Corrupt the file: append duplicate trailing content (like HAI-015)
|
// Corrupt the file: append duplicate trailing content
|
||||||
const validJson = await readFile(taskJsonPath, "utf-8");
|
const validJson = await readFile(taskJsonPath, "utf-8");
|
||||||
const corrupted = validJson + validJson.slice(validJson.length / 2);
|
const corrupted = validJson + validJson.slice(validJson.length / 2);
|
||||||
await writeFile(taskJsonPath, corrupted);
|
await writeFile(taskJsonPath, corrupted);
|
||||||
|
|
||||||
// getTask should recover
|
// With atomic writes, corruption indicates a real bug — should throw
|
||||||
const recovered = await store.getTask(task.id);
|
await expect(store.getTask(task.id)).rejects.toThrow("Failed to parse task.json");
|
||||||
expect(recovered.id).toBe(task.id);
|
|
||||||
expect(recovered.description).toBe("Test task");
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("throws a clear error when JSON is completely unrecoverable", async () => {
|
it("throws a clear error when JSON is completely unrecoverable", async () => {
|
||||||
|
|||||||
@@ -91,32 +91,16 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Safely read and parse a task.json file. On `SyntaxError`, attempts to
|
* Read and parse a task.json file. Throws immediately on invalid JSON —
|
||||||
* recover by truncating the content at the last valid `}` and re-parsing.
|
* atomic writes (write-to-temp-then-rename) prevent partial-write
|
||||||
* Logs a warning to stderr when truncation-repair is used.
|
* corruption, so a `SyntaxError` indicates a real bug rather than a race.
|
||||||
*/
|
*/
|
||||||
private async safeReadTaskJson(dir: string): Promise<Task> {
|
private async readTaskJson(dir: string): Promise<Task> {
|
||||||
const filePath = join(dir, "task.json");
|
const filePath = join(dir, "task.json");
|
||||||
const raw = await readFile(filePath, "utf-8");
|
const raw = await readFile(filePath, "utf-8");
|
||||||
try {
|
try {
|
||||||
return JSON.parse(raw) as Task;
|
return JSON.parse(raw) as Task;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (!(err instanceof SyntaxError)) throw err;
|
|
||||||
|
|
||||||
// Attempt recovery: try truncating at each '}' from the end until valid
|
|
||||||
let pos = raw.length;
|
|
||||||
while ((pos = raw.lastIndexOf("}", pos - 1)) > 0) {
|
|
||||||
try {
|
|
||||||
const task = JSON.parse(raw.slice(0, pos + 1)) as Task;
|
|
||||||
console.warn(
|
|
||||||
`[hai] Warning: repaired corrupted task.json at ${filePath} (truncated ${raw.length - pos - 1} trailing bytes)`,
|
|
||||||
);
|
|
||||||
return task;
|
|
||||||
} catch {
|
|
||||||
// Try next position
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
throw new Error(
|
throw new Error(
|
||||||
`Failed to parse task.json at ${filePath}: ${(err as Error).message}`,
|
`Failed to parse task.json at ${filePath}: ${(err as Error).message}`,
|
||||||
);
|
);
|
||||||
@@ -232,7 +216,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
*/
|
*/
|
||||||
async getTask(id: string): Promise<TaskDetail> {
|
async getTask(id: string): Promise<TaskDetail> {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
let prompt = "";
|
let prompt = "";
|
||||||
const promptPath = join(dir, "PROMPT.md");
|
const promptPath = join(dir, "PROMPT.md");
|
||||||
@@ -252,7 +236,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
for (const entry of entries) {
|
for (const entry of entries) {
|
||||||
if (entry.isDirectory() && entry.name.startsWith("HAI-")) {
|
if (entry.isDirectory() && entry.name.startsWith("HAI-")) {
|
||||||
try {
|
try {
|
||||||
tasks.push(await this.safeReadTaskJson(join(this.tasksDir, entry.name)));
|
tasks.push(await this.readTaskJson(join(this.tasksDir, entry.name)));
|
||||||
} catch {
|
} catch {
|
||||||
// skip invalid task dirs
|
// skip invalid task dirs
|
||||||
}
|
}
|
||||||
@@ -265,7 +249,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
async moveTask(id: string, toColumn: Column): Promise<Task> {
|
async moveTask(id: string, toColumn: Column): Promise<Task> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
const validTargets = VALID_TRANSITIONS[task.column];
|
const validTargets = VALID_TRANSITIONS[task.column];
|
||||||
if (!validTargets.includes(toColumn)) {
|
if (!validTargets.includes(toColumn)) {
|
||||||
@@ -301,7 +285,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
): Promise<Task> {
|
): Promise<Task> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
if (updates.title !== undefined) task.title = updates.title;
|
if (updates.title !== undefined) task.title = updates.title;
|
||||||
if (updates.description !== undefined) task.description = updates.description;
|
if (updates.description !== undefined) task.description = updates.description;
|
||||||
@@ -337,7 +321,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
): Promise<Task> {
|
): Promise<Task> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
// Auto-initialize steps from PROMPT.md if empty
|
// Auto-initialize steps from PROMPT.md if empty
|
||||||
if (task.steps.length === 0) {
|
if (task.steps.length === 0) {
|
||||||
@@ -385,7 +369,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
async logEntry(id: string, action: string, outcome?: string): Promise<Task> {
|
async logEntry(id: string, action: string, outcome?: string): Promise<Task> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
task.log.push({
|
task.log.push({
|
||||||
timestamp: new Date().toISOString(),
|
timestamp: new Date().toISOString(),
|
||||||
@@ -452,7 +436,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
async deleteTask(id: string): Promise<Task> {
|
async deleteTask(id: string): Promise<Task> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
const taskJsonPath = join(dir, "task.json");
|
const taskJsonPath = join(dir, "task.json");
|
||||||
this.suppressWatcher(taskJsonPath);
|
this.suppressWatcher(taskJsonPath);
|
||||||
@@ -475,7 +459,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
async mergeTask(id: string): Promise<MergeResult> {
|
async mergeTask(id: string): Promise<MergeResult> {
|
||||||
return this.withTaskLock(id, async () => {
|
return this.withTaskLock(id, async () => {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const task = await this.safeReadTaskJson(dir);
|
const task = await this.readTaskJson(dir);
|
||||||
|
|
||||||
if (task.column !== "in-review") {
|
if (task.column !== "in-review") {
|
||||||
throw new Error(
|
throw new Error(
|
||||||
@@ -698,7 +682,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
let task: Task;
|
let task: Task;
|
||||||
try {
|
try {
|
||||||
const taskDir = join(this.tasksDir, taskId);
|
const taskDir = join(this.tasksDir, taskId);
|
||||||
task = await this.safeReadTaskJson(taskDir);
|
task = await this.readTaskJson(taskDir);
|
||||||
} catch {
|
} catch {
|
||||||
return; // File not readable or invalid JSON
|
return; // File not readable or invalid JSON
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user