feat(FN-2370): merge fusion/fn-2370 (auto-resolved)
- test(FN-2370): complete Step 3 — align qa-check template expectation - test(FN-2370): complete Step 2 — add regression coverage for addComment diagnostics - test(FN-2370): complete Step 2 — cover addComment warning regressions - feat(FN-2370): complete Step 1 — log addComment best-effort failures - feat(FN-2369): merge fusion/fn-2369 - feat(prompts): require lint alongside tests and typecheck in agent instructions - perf(test): parallelize harder — unlock worker count, split build-output, bump workspace concurrency - fix(core): recognize legacy kb-* backups and canonicalize .kb/backups settings - refactor: eliminate remaining 15 any warnings and ratchet rule to error - refactor: eliminate ~400 no-explicit-any warnings across the workspace - feat(core): add getErrorMessage helper for narrowing unknown errors - refactor: fix and tighten mechanical lint rules - chore(eslint): fix pre-existing errors surfaced by wider .cjs match - chore(eslint): promote @typescript-eslint/no-unused-vars from warn to error - refactor(dashboard,desktop,engine): remove unused imports, props, and locals - refactor(core): remove unused imports, helpers, and dead migration constant - refactor(cli): remove unused imports and variables - refactor: adapt resource loader and tool wiring to pi-coding-agent 0.70 - fix: adapt to AgentState.error → errorMessage rename - refactor: migrate @sinclair/typebox imports to typebox 1.x - refactor: migrate to ModelRegistry.create factory - chore: bump pi-coding-agent + pi-ai to 0.70.0 - refactor: remove legacy kb compatibility - feat: add "Anthropic — via Claude CLI" as a first-class provider - test(FN-2358): harden clean-worktree CI verification tests - fix(FN-2352): add structured terminal websocket diagnostics - fix: use live merge-base for task diff scope - feat: backfill Claude skills when useClaudeCli toggle flips on - fix: prevent nested .fusion/.fusion dir from PluginStore path bug
This commit is contained in:
@@ -9,8 +9,8 @@
|
||||
* - Error isolation (plugin crashes don't crash the loader)
|
||||
*/
|
||||
|
||||
import { copyFile, rm } from "node:fs/promises";
|
||||
import { basename, dirname, extname, isAbsolute, resolve } from "node:path";
|
||||
import { copyFile, rm } from "node:fs/promises";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { TaskStore } from "./store.js";
|
||||
|
||||
@@ -4370,6 +4370,148 @@ Task with acceptance criteria
|
||||
expect(updated.comments![0].text).toBe(" ");
|
||||
});
|
||||
|
||||
it("logs warning and still persists comment when best-effort auto-refinement fails", async () => {
|
||||
const task = await store.createTask({ description: "Original task" });
|
||||
await store.moveTask(task.id, "todo");
|
||||
await store.moveTask(task.id, "in-progress");
|
||||
await store.moveTask(task.id, "in-review");
|
||||
await store.moveTask(task.id, "done");
|
||||
|
||||
const runContext = { runId: "run-refinement-failure", agentId: "agent-refinement" };
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const refineSpy = vi.spyOn(store, "refineTask").mockRejectedValue(new Error("refine unavailable"));
|
||||
|
||||
try {
|
||||
const taskCountBefore = (await store.listTasks()).length;
|
||||
const updated = await store.addComment(task.id, "Need refinement", "user", undefined, runContext);
|
||||
|
||||
expect(updated.comments).toHaveLength(1);
|
||||
expect(updated.comments![0].text).toBe("Need refinement");
|
||||
|
||||
const taskCountAfter = (await store.listTasks()).length;
|
||||
expect(taskCountAfter).toBe(taskCountBefore);
|
||||
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.comments).toHaveLength(1);
|
||||
expect(persisted.comments![0].text).toBe("Need refinement");
|
||||
|
||||
expect(refineSpy).toHaveBeenCalledWith(task.id, "Need refinement");
|
||||
|
||||
const warningCall = warnSpy.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("[task-store] Best-effort post-comment auto-refinement failed"),
|
||||
);
|
||||
expect(warningCall).toBeDefined();
|
||||
|
||||
const [, context] = warningCall as [string, Record<string, unknown>];
|
||||
expect(context).toMatchObject({
|
||||
taskId: task.id,
|
||||
author: "user",
|
||||
commentLength: "Need refinement".length,
|
||||
column: "done",
|
||||
priorStatus: null,
|
||||
phase: "addComment:auto-refinement",
|
||||
runId: "run-refinement-failure",
|
||||
agentId: "agent-refinement",
|
||||
error: "refine unavailable",
|
||||
});
|
||||
} finally {
|
||||
refineSpy.mockRestore();
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("logs warning and still persists comment when status update fails during awaiting-approval invalidation", async () => {
|
||||
const task = await store.createTask({ description: "Task in triage" });
|
||||
await store.updateTask(task.id, { status: "awaiting-approval" });
|
||||
|
||||
const runContext = { runId: "run-invalidation-failure", agentId: "agent-invalidation" };
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const updateSpy = vi.spyOn(store, "updateTask").mockRejectedValueOnce(new Error("status update failed"));
|
||||
|
||||
try {
|
||||
const updated = await store.addComment(task.id, "New user feedback", "user", undefined, runContext);
|
||||
|
||||
expect(updated.comments).toHaveLength(1);
|
||||
expect(updated.comments![0].text).toBe("New user feedback");
|
||||
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.comments).toHaveLength(1);
|
||||
expect(persisted.comments![0].text).toBe("New user feedback");
|
||||
expect(persisted.status).toBe("awaiting-approval");
|
||||
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
|
||||
const warningCall = warnSpy.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("[task-store] Best-effort post-comment awaiting-approval invalidation failed"),
|
||||
);
|
||||
expect(warningCall).toBeDefined();
|
||||
|
||||
const [, context] = warningCall as [string, Record<string, unknown>];
|
||||
expect(context).toMatchObject({
|
||||
taskId: task.id,
|
||||
author: "user",
|
||||
commentLength: "New user feedback".length,
|
||||
column: "triage",
|
||||
priorStatus: "awaiting-approval",
|
||||
phase: "addComment:awaiting-approval-invalidation",
|
||||
stage: "status-update",
|
||||
nextStatus: "needs-respecify",
|
||||
runId: "run-invalidation-failure",
|
||||
agentId: "agent-invalidation",
|
||||
error: "status update failed",
|
||||
});
|
||||
} finally {
|
||||
updateSpy.mockRestore();
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("logs warning and keeps invalidated status when log entry fails after awaiting-approval invalidation", async () => {
|
||||
const task = await store.createTask({ description: "Task in triage" });
|
||||
await store.updateTask(task.id, { status: "awaiting-approval" });
|
||||
|
||||
const runContext = { runId: "run-post-invalidation-log-failure", agentId: "agent-invalidation" };
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const logEntrySpy = vi.spyOn(store, "logEntry").mockRejectedValueOnce(new Error("log entry failed"));
|
||||
|
||||
try {
|
||||
const updated = await store.addComment(task.id, "New user feedback", "user", undefined, runContext);
|
||||
|
||||
expect(updated.comments).toHaveLength(1);
|
||||
expect(updated.comments![0].text).toBe("New user feedback");
|
||||
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.comments).toHaveLength(1);
|
||||
expect(persisted.comments![0].text).toBe("New user feedback");
|
||||
expect(persisted.status).toBe("needs-respecify");
|
||||
|
||||
expect(logEntrySpy).toHaveBeenCalled();
|
||||
|
||||
const warningCall = warnSpy.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("[task-store] Best-effort post-comment awaiting-approval invalidation failed"),
|
||||
);
|
||||
expect(warningCall).toBeDefined();
|
||||
|
||||
const [, context] = warningCall as [string, Record<string, unknown>];
|
||||
expect(context).toMatchObject({
|
||||
taskId: task.id,
|
||||
author: "user",
|
||||
commentLength: "New user feedback".length,
|
||||
column: "triage",
|
||||
priorStatus: "awaiting-approval",
|
||||
phase: "addComment:awaiting-approval-invalidation",
|
||||
stage: "post-invalidation-log-entry",
|
||||
nextStatus: "needs-respecify",
|
||||
runId: "run-post-invalidation-log-failure",
|
||||
agentId: "agent-invalidation",
|
||||
error: "log entry failed",
|
||||
});
|
||||
} finally {
|
||||
logEntrySpy.mockRestore();
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("addSteeringComment on done task does NOT create a refinement task", async () => {
|
||||
const task = await store.createTask({ description: "Original task" });
|
||||
await store.moveTask(task.id, "todo");
|
||||
|
||||
@@ -4114,22 +4114,43 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
return task;
|
||||
});
|
||||
|
||||
const commentContextBase: Record<string, unknown> = {
|
||||
taskId: id,
|
||||
author,
|
||||
commentLength: text.length,
|
||||
column: task.column,
|
||||
priorStatus: task.status ?? null,
|
||||
};
|
||||
if (runContext) {
|
||||
commentContextBase.runId = runContext.runId;
|
||||
commentContextBase.agentId = runContext.agentId;
|
||||
if (runContext.source) {
|
||||
commentContextBase.runSource = runContext.source;
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 2: Auto-refinement OUTSIDE the lock (to avoid lock contention)
|
||||
// Only create refinement for user comments on done tasks
|
||||
// Steering comments skip refinement — they are injected into the agent stream instead
|
||||
// Only create refinement for user comments on done tasks.
|
||||
// This remains best-effort: failures are logged for observability but never
|
||||
// fail the comment add operation itself.
|
||||
// Steering comments skip refinement — they are injected into the agent stream instead.
|
||||
if (task.column === "done" && author === "user" && !options?.skipRefinement) {
|
||||
try {
|
||||
await this.refineTask(id, text);
|
||||
} catch {
|
||||
// Silently ignore - refinement is best-effort and shouldn't fail
|
||||
// the comment addition. refineTask already validates
|
||||
// feedback text, so empty/whitespace comments won't create refinements.
|
||||
} catch (err) {
|
||||
storeLog.warn("Best-effort post-comment auto-refinement failed", {
|
||||
...commentContextBase,
|
||||
phase: "addComment:auto-refinement",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 3: Invalidate stale spec approval when a user comments on
|
||||
// a triage task that is awaiting manual approval. The new comment
|
||||
// means the spec is now stale and must be re-specified/re-reviewed.
|
||||
// This remains best-effort: failures are logged for observability but
|
||||
// never fail the comment add operation itself.
|
||||
// Note: The `task` returned above reflects the state BEFORE this
|
||||
// transition. Callers that need the post-transition status should
|
||||
// re-read the task (e.g., via getTask).
|
||||
@@ -4138,18 +4159,39 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
&& task.status === "awaiting-approval"
|
||||
&& author === "user"
|
||||
) {
|
||||
let invalidatedStatus = false;
|
||||
try {
|
||||
await this.updateTask(id, {
|
||||
status: "needs-respecify",
|
||||
});
|
||||
await this.logEntry(
|
||||
id,
|
||||
`User comment invalidated spec approval — task needs re-specification`,
|
||||
undefined,
|
||||
runContext,
|
||||
);
|
||||
} catch {
|
||||
// Best-effort: don't fail the comment if the status update fails
|
||||
invalidatedStatus = true;
|
||||
} catch (err) {
|
||||
storeLog.warn("Best-effort post-comment awaiting-approval invalidation failed", {
|
||||
...commentContextBase,
|
||||
phase: "addComment:awaiting-approval-invalidation",
|
||||
stage: "status-update",
|
||||
nextStatus: "needs-respecify",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
|
||||
if (invalidatedStatus) {
|
||||
try {
|
||||
await this.logEntry(
|
||||
id,
|
||||
`User comment invalidated spec approval — task needs re-specification`,
|
||||
undefined,
|
||||
runContext,
|
||||
);
|
||||
} catch (err) {
|
||||
storeLog.warn("Best-effort post-comment awaiting-approval invalidation failed", {
|
||||
...commentContextBase,
|
||||
phase: "addComment:awaiting-approval-invalidation",
|
||||
stage: "post-invalidation-log-entry",
|
||||
nextStatus: "needs-respecify",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user