feat(FN-1634): promote mailbox to first-class navigation view
- Add MailboxView as a full-page navigation view replacing the modal-based approach - Migrate MessageStore from filesystem to SQLite backend for message persistence - Implement conversation grouping for inbox display with unread badge state - Remove modal plumbing (useModalManager, AppModals exports) and simplify App.tsx - Add evictStaleProcessing() to TriageProcessor for self-healing hung triage sessions - Add comprehensive MailboxView tests and Header mailbox tests - Update README documentation with MailboxView features - Add CSS styles for MailboxView component - Fix MobileNavBar tests for mailbox tab visibility
This commit is contained in:
@@ -2737,7 +2737,7 @@ ${failureFeedback}
|
||||
|
||||
**Retry:** ${retryCount}/${MAX_WORKFLOW_STEP_RETRIES} (${remainingRetries} remaining)
|
||||
|
||||
**Important:** This is a workflow step failure — fix the issues above by making the necessary code changes. The task will be retried automatically. If all ${MAX_WORKFLOW_STEP_RETRIES} retries are exhausted, the task will be moved to in-review for manual inspection.
|
||||
**Important:** This is a workflow step failure — fix the issues above by making the necessary code changes. The task has been sent back to in-progress for remediation. The executor will attempt to fix the issues on the next pass.
|
||||
|
||||
`;
|
||||
|
||||
|
||||
@@ -479,6 +479,7 @@ export class InProcessRuntime
|
||||
getExecutingTaskIds: () => this.executor.getExecutingTaskIds(),
|
||||
recoverApprovedTriageTask: (task) => this.triageProcessor?.recoverApprovedTask(task) ?? Promise.resolve(false),
|
||||
getSpecifyingTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(),
|
||||
evictStaleTriageProcessing: () => this.triageProcessor?.evictStaleProcessing() ?? new Set<string>(),
|
||||
});
|
||||
this.selfHealingManager.start();
|
||||
this.stuckTaskDetector.start();
|
||||
|
||||
@@ -1435,3 +1435,123 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("stale triage processing eviction before recovery", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("calls evictStaleTriageProcessing before recoverApprovedTriageTasks", async () => {
|
||||
const store = createMockStore();
|
||||
const evictFn = vi.fn().mockReturnValue(new Set<string>());
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const getSpecifying = vi.fn().mockReturnValue(new Set(["FN-100"]));
|
||||
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverApprovedTriageTask: recoverFn,
|
||||
getSpecifyingTaskIds: getSpecifying,
|
||||
evictStaleTriageProcessing: evictFn,
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-100",
|
||||
column: "triage",
|
||||
status: "specifying",
|
||||
paused: false,
|
||||
log: [{ action: "Spec review: APPROVE" }],
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
]);
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:05:00.000Z"));
|
||||
|
||||
// FN-100 is in specifyingIds — would normally be skipped.
|
||||
// But evictStaleTriageProcessing was called first (even though it evicted nothing here).
|
||||
await manager.recoverApprovedTriageTasks();
|
||||
|
||||
// Eviction was called before the recovery check
|
||||
expect(evictFn).toHaveBeenCalledTimes(1);
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("recovers approved task after eviction removes it from specifyingIds", async () => {
|
||||
const store = createMockStore();
|
||||
let specifyingIds = new Set(["FN-100"]);
|
||||
const evictFn = vi.fn().mockImplementation(() => {
|
||||
// Simulate eviction removing FN-100 from the processing set
|
||||
specifyingIds = new Set<string>();
|
||||
return new Set(["FN-100"]);
|
||||
});
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const getSpecifying = vi.fn().mockImplementation(() => specifyingIds);
|
||||
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverApprovedTriageTask: recoverFn,
|
||||
getSpecifyingTaskIds: getSpecifying,
|
||||
evictStaleTriageProcessing: evictFn,
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-100",
|
||||
column: "triage",
|
||||
status: "specifying",
|
||||
paused: false,
|
||||
log: [{ action: "Spec review: APPROVE" }],
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
]);
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:05:00.000Z"));
|
||||
|
||||
const result = await manager.recoverApprovedTriageTasks();
|
||||
|
||||
// After eviction cleared the specifying set, the task was recovered
|
||||
expect(result).toBe(1);
|
||||
expect(recoverFn).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: "FN-100" }),
|
||||
);
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("calls evictStaleTriageProcessing before recoverOrphanedSpecifyingTasks", async () => {
|
||||
const store = createMockStore();
|
||||
const evictFn = vi.fn().mockReturnValue(new Set<string>());
|
||||
const getSpecifying = vi.fn().mockReturnValue(new Set(["FN-101"]));
|
||||
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getSpecifyingTaskIds: getSpecifying,
|
||||
evictStaleTriageProcessing: evictFn,
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-101",
|
||||
column: "triage",
|
||||
status: "specifying",
|
||||
paused: false,
|
||||
log: [{ action: "Spec review: REVISE" }],
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
]);
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:05:00.000Z"));
|
||||
|
||||
await manager.recoverOrphanedSpecifyingTasks();
|
||||
|
||||
expect(evictFn).toHaveBeenCalledTimes(1);
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -53,6 +53,12 @@ export interface SelfHealingOptions {
|
||||
* Used to avoid recovering active triage sessions.
|
||||
*/
|
||||
getSpecifyingTaskIds?: () => Set<string>;
|
||||
/**
|
||||
* Evict tasks from the triage processor's `processing` set that have been
|
||||
* there longer than the staleness threshold (hung promises from stuck kills).
|
||||
* Called before recovery checks so stale entries don't block recovery.
|
||||
*/
|
||||
evictStaleTriageProcessing?: () => Set<string>;
|
||||
}
|
||||
|
||||
const APPROVED_TRIAGE_RECOVERY_GRACE_MS = 60_000;
|
||||
@@ -795,6 +801,11 @@ export class SelfHealingManager {
|
||||
if (!recoverFn) return 0;
|
||||
|
||||
try {
|
||||
// Evict stale entries from the triage processor's in-memory set before
|
||||
// checking — tasks with hung promises (from stuck kills) would otherwise
|
||||
// block recovery indefinitely.
|
||||
this.options.evictStaleTriageProcessing?.();
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "triage" });
|
||||
const specifyingIds = this.options.getSpecifyingTaskIds?.() ?? new Set<string>();
|
||||
const now = Date.now();
|
||||
@@ -843,6 +854,11 @@ export class SelfHealingManager {
|
||||
*/
|
||||
async recoverOrphanedSpecifyingTasks(): Promise<number> {
|
||||
try {
|
||||
// Evict stale entries from the triage processor's in-memory set before
|
||||
// checking — tasks with hung promises (from stuck kills) would otherwise
|
||||
// block recovery indefinitely.
|
||||
this.options.evictStaleTriageProcessing?.();
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "triage" });
|
||||
const specifyingIds = this.options.getSpecifyingTaskIds?.() ?? new Set<string>();
|
||||
const now = Date.now();
|
||||
|
||||
@@ -2437,3 +2437,107 @@ describe("TriageProcessor skillSelection regression (FN-1511)", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("evictStaleProcessing", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("evicts tasks that have been in processing longer than 30 minutes", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
// Simulate a task that entered processing 31 minutes ago
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-001");
|
||||
(processor as any).processingSince.set("FN-001", Date.now());
|
||||
|
||||
// Advance time 31 minutes
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted).toEqual(new Set(["FN-001"]));
|
||||
expect(processor.getProcessingTaskIds().has("FN-001")).toBe(false);
|
||||
});
|
||||
|
||||
it("does not evict tasks that have been in processing less than 30 minutes", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-001");
|
||||
(processor as any).processingSince.set("FN-001", Date.now());
|
||||
|
||||
// Advance time 29 minutes — not stale yet
|
||||
vi.setSystemTime(new Date("2026-01-01T00:29:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted.size).toBe(0);
|
||||
expect(processor.getProcessingTaskIds().has("FN-001")).toBe(true);
|
||||
});
|
||||
|
||||
it("cleans up activeSessions and stuckAborted when evicting", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-001");
|
||||
(processor as any).processingSince.set("FN-001", Date.now());
|
||||
(processor as any).activeSessions.set("FN-001", { dispose: vi.fn() });
|
||||
(processor as any).stuckAborted.add("FN-001");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted).toEqual(new Set(["FN-001"]));
|
||||
expect((processor as any).activeSessions.has("FN-001")).toBe(false);
|
||||
expect((processor as any).stuckAborted.has("FN-001")).toBe(false);
|
||||
});
|
||||
|
||||
it("returns empty set when no tasks are stale", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-001");
|
||||
(processor as any).processingSince.set("FN-001", Date.now());
|
||||
|
||||
// Only 5 minutes — well within threshold
|
||||
vi.setSystemTime(new Date("2026-01-01T00:05:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
expect(evicted.size).toBe(0);
|
||||
expect(processor.getProcessingTaskIds().has("FN-001")).toBe(true);
|
||||
});
|
||||
|
||||
it("evicts multiple stale tasks while keeping fresh ones", () => {
|
||||
const store = createMockStore();
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
|
||||
(processor as any).processing.add("FN-001");
|
||||
(processor as any).processingSince.set("FN-001", Date.now());
|
||||
|
||||
vi.setSystemTime(new Date("2026-01-01T00:15:00.000Z"));
|
||||
(processor as any).processing.add("FN-002");
|
||||
(processor as any).processingSince.set("FN-002", Date.now());
|
||||
|
||||
// FN-001 entered at 00:00, FN-002 at 00:15
|
||||
vi.setSystemTime(new Date("2026-01-01T00:35:00.000Z"));
|
||||
|
||||
const evicted = processor.evictStaleProcessing();
|
||||
|
||||
// FN-001 has been in for 35min → evicted. FN-002 for 20min → not stale.
|
||||
expect(evicted).toEqual(new Set(["FN-001"]));
|
||||
expect(processor.getProcessingTaskIds().has("FN-001")).toBe(false);
|
||||
expect(processor.getProcessingTaskIds().has("FN-002")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -276,6 +276,8 @@ export class TriageProcessor {
|
||||
/** The interval (ms) of the currently active `setInterval` timer. */
|
||||
private activePollMs: number | null = null;
|
||||
private processing = new Set<string>();
|
||||
/** Timestamps when tasks entered the `processing` set, for staleness detection. */
|
||||
private processingSince = new Map<string, number>();
|
||||
private wasGlobalPaused = false;
|
||||
private wasEnginePaused = false;
|
||||
/** Active agent sessions per task, used to terminate on pause. */
|
||||
@@ -403,6 +405,47 @@ export class TriageProcessor {
|
||||
return new Set(this.processing);
|
||||
}
|
||||
|
||||
/**
|
||||
* Maximum time a task can remain in the `processing` set before it's
|
||||
* considered stale (30 minutes). By this point the stuck detector
|
||||
* (default 20-min timeout) should have already killed the session
|
||||
* and the `finally` block should have cleaned up. If it hasn't,
|
||||
* the promise is hung (e.g., `promptWithFallback` never settled
|
||||
* after dispose) and self-healing recovery needs to force-evict it.
|
||||
*/
|
||||
private static readonly STALE_PROCESSING_THRESHOLD_MS = 30 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* Evict tasks from the `processing` set that have been there longer than
|
||||
* the staleness threshold. This handles the case where a stuck-kill
|
||||
* disposes the session but the `specifyTask` promise never settles
|
||||
* (hung `promptWithFallback`), leaving the task in `processing` forever
|
||||
* and blocking self-healing recovery.
|
||||
*
|
||||
* @returns the set of evicted task IDs
|
||||
*/
|
||||
evictStaleProcessing(): Set<string> {
|
||||
const now = Date.now();
|
||||
const threshold = TriageProcessor.STALE_PROCESSING_THRESHOLD_MS;
|
||||
const evicted = new Set<string>();
|
||||
|
||||
for (const [taskId, since] of this.processingSince) {
|
||||
if (now - since >= threshold) {
|
||||
triageLog.warn(
|
||||
`${taskId} has been in processing for ${Math.round((now - since) / 60_000)}min ` +
|
||||
`(threshold: ${Math.round(threshold / 60_000)}min) — evicting (likely hung promise)`,
|
||||
);
|
||||
this.processing.delete(taskId);
|
||||
this.processingSince.delete(taskId);
|
||||
this.activeSessions.delete(taskId);
|
||||
this.stuckAborted.delete(taskId);
|
||||
evicted.add(taskId);
|
||||
}
|
||||
}
|
||||
|
||||
return evicted;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover a triage task whose spec was already approved but the final
|
||||
* handoff out of `status: "specifying"` never completed.
|
||||
@@ -549,6 +592,7 @@ export class TriageProcessor {
|
||||
async specifyTask(task: Task): Promise<void> {
|
||||
if (this.processing.has(task.id)) return;
|
||||
this.processing.add(task.id);
|
||||
this.processingSince.set(task.id, Date.now());
|
||||
|
||||
triageLog.log(
|
||||
`Specifying ${task.id}: ${task.title || task.description.slice(0, 60)}`,
|
||||
@@ -896,6 +940,7 @@ export class TriageProcessor {
|
||||
}
|
||||
} finally {
|
||||
this.processing.delete(task.id);
|
||||
this.processingSince.delete(task.id);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user