Add tasks.requires_filesystem column (v52) — per-task filesystem lock policy

Before v52, resolveWorkspaceWithLockMode walked a four-step fallback
chain (task → agent → project → any company workspace) and marked
every resolved row exclusive. In a multi-agent company every heartbeat
collapsed onto the same company workspace row, every run claimed the
same exclusive lock, and only one agent ever executed at a time.

Root cause is a missing column: per-task 'does this task touch the
filesystem?' had no place to live, so the orchestrator defaulted to
yes on everything. v52 gives that question its own column.

Backfill is conservative:
  - in-flight tasks (in_progress + active_run_id) → 1 (preserve
    the lease the dispatcher already holds)
  - todo/backlog tasks whose assignee is an engineering-flavoured
    role (CTO/engineer/developer/devops/sre or department =
    engineering) → 1
  - everything else → 0

TaskRecord now exposes requiresFilesystem on both entityCrud.getTask
and the snapshot read path.
This commit is contained in:
ZacharyZhang-NY committed 2026-04-20 15:25:36 -04:00
1 parent 3ff891c6ec
commit 7829acc7f0
5 files changed
+165 -3

No files matched your search

+2 -1
View File
@@ -53,11 +53,12 @@ export function buildEntityCrudMethods(
getTask(id: string): TaskRecord {
const row = getDb()
.prepare(
"select id, company_id as companyId, project_id as projectId, goal_id as goalId, parent_id as parentId, title, description, assignee_agent_id as assigneeAgentId, workspace_id as workspaceId, status, priority, active_run_id as activeRunId, cost_usd as costUsd, task_type as taskType, requires_user_review as requiresUserReview, started_at as startedAt, completed_at as completedAt, cancelled_at as cancelledAt, created_at as createdAt, updated_at as updatedAt from tasks where id = ?",
"select id, company_id as companyId, project_id as projectId, goal_id as goalId, parent_id as parentId, title, description, assignee_agent_id as assigneeAgentId, workspace_id as workspaceId, status, priority, active_run_id as activeRunId, cost_usd as costUsd, task_type as taskType, requires_user_review as requiresUserReview, requires_filesystem as requiresFilesystem, started_at as startedAt, completed_at as completedAt, cancelled_at as cancelledAt, created_at as createdAt, updated_at as updatedAt from tasks where id = ?",
)
.get(id) as TaskRecord | undefined;
if (!row) throw new Error(`Task not found: ${id}`);
row.requiresUserReview = Boolean(row.requiresUserReview);
row.requiresFilesystem = Boolean(row.requiresFilesystem);
row.startedAt = row.startedAt ?? null;
row.completedAt = row.completedAt ?? null;
row.cancelledAt = row.cancelledAt ?? null;
+150
View File
@@ -0,0 +1,150 @@
import log from "electron-log/main.js";
import type { Migration, MigrationDeps } from "./types";
import { columnExists, tableExists } from "./types";
/**
* Per-task filesystem lock policy — `tasks.requires_filesystem`.
*
* Background: before v52 `resolveWorkspaceWithLockMode` walked a four-
* step fallback chain (task → agent → project → any company workspace)
* and unconditionally marked the resolved row `exclusive`. In a
* multi-agent company every heartbeat collapsed onto the single
* company workspace row, every run claimed the same exclusive lock,
* and only one agent ever executed at a time — the "parallel C-suite"
* promise degraded silently into serial execution.
*
* Root cause is a schema one: the per-task shape of the question
* "does this task actually touch the filesystem?" had no column to
* live in, so the orchestrator defaulted to yes on everything. v52
* gives that question its own column.
*
* Invariants after this migration:
* • Every task row carries `requires_filesystem` ∈ {0, 1}.
* • `0` is the safe default — purely cognitive tasks (strategy,
* planning, document drafting, financial analysis) never take
* a workspace lock and run fully in parallel.
* • `1` is opt-in — explicitly signals "this task mutates the
* working tree, hold the workspace exclusive for the run".
*
* Backfill strategy (conservative — we only set `1` when we have
* positive evidence of filesystem use):
*
* a) In-flight tasks (`status = 'in_progress'` AND `active_run_id
* IS NOT NULL`): these currently hold a lease on a workspace;
* changing their flag mid-run would desynchronise the live
* dispatcher from the backing row. Mark `1` to preserve the
* exclusive semantics already in effect.
*
* b) Actionable tasks (`status IN ('todo','backlog')`) whose
* assignee is an engineering-flavoured role: CTO / engineer /
* developer / devops / SRE, or department = 'engineering'.
* These are the roles that `createCodingTools()` equips; every
* other role's task bag is cognitive by default.
*
* c) Everything else (done / cancelled / blocked / in_review
* non-engineering / etc.): `0`. Completed or cancelled tasks
* will never run again so the column value is advisory;
* blocked tasks will re-enter the dispatcher through an update
* path that will set the flag explicitly when it matters.
*
* Assertion: after backfill, zero NULL rows (the DEFAULT 0 on the
* ALTER TABLE guarantees this; the assertion is belt-and-braces).
*/
export function createMigrationsV52V52(deps: MigrationDeps): Migration[] {
const { db } = deps;
return [
{
version: 52,
name: "tasks_requires_filesystem",
fn: () => {
if (!tableExists(db, "tasks")) {
log.info("[migration v52] tasks table missing; nothing to do");
return;
}
// ── 1. Add the column. NOT NULL DEFAULT 0 so existing rows
// get a deterministic value at ALTER time — no NULL window
// between the ALTER and the backfill UPDATE.
if (!columnExists(db, "tasks", "requires_filesystem")) {
db.exec(
`ALTER TABLE tasks ADD COLUMN requires_filesystem INTEGER NOT NULL DEFAULT 0`,
);
log.info("[migration v52] tasks.requires_filesystem column added");
} else {
log.info("[migration v52] tasks.requires_filesystem already present; reusing");
}
// ── 2. Backfill rule (a): in-flight rows preserve the lease
// the dispatcher is already holding. Turning these off
// would leave a running lease pointing at a task whose
// current policy says "no lease needed".
const inflightUpdate = db.prepare(
`UPDATE tasks
SET requires_filesystem = 1
WHERE status = 'in_progress'
AND active_run_id IS NOT NULL`,
).run();
log.info(
`[migration v52] backfill (a) inflight: ${Number(inflightUpdate.changes)} rows set to 1`,
);
// ── 3. Backfill rule (b): actionable tasks for coding roles.
// LIKE comparisons are case-insensitive in SQLite by
// default (NOCASE collation on text literal compares).
// We match both the free-text `agents.role` (where the
// UI lets users write "CTO", "Senior Engineer", etc.) and
// the structured `agents.department` column where present.
const codingRoleUpdate = db.prepare(
`UPDATE tasks
SET requires_filesystem = 1
WHERE status IN ('todo', 'backlog')
AND assignee_agent_id IS NOT NULL
AND EXISTS (
SELECT 1 FROM agents a
WHERE a.id = tasks.assignee_agent_id
AND (
a.role LIKE '%cto%'
OR a.role LIKE '%engineer%'
OR a.role LIKE '%developer%'
OR a.role LIKE '%devops%'
OR a.role LIKE '%sre%'
OR a.department = 'engineering'
)
)`,
).run();
log.info(
`[migration v52] backfill (b) coding role actionable: ${Number(codingRoleUpdate.changes)} rows set to 1`,
);
// ── 4. Invariant assertion. DEFAULT 0 combined with the two
// targeted UPDATEs above means the column is never NULL,
// but the assertion makes the post-condition explicit so
// a future migration that removes the DEFAULT cannot
// silently violate it.
const nullCountRow = db.prepare(
`SELECT COUNT(*) AS count FROM tasks WHERE requires_filesystem IS NULL`,
).get() as { count: number } | undefined;
const nullCount = nullCountRow?.count ?? 0;
if (nullCount > 0) {
throw new Error(
`[migration v52] invariant violated: ${nullCount} tasks row(s) have requires_filesystem IS NULL after backfill`,
);
}
// Diagnostic counts — read back the final distribution so the
// upgrade log carries evidence that backfill matched the
// expected order of magnitude.
const totals = db.prepare(
`SELECT
SUM(CASE WHEN requires_filesystem = 1 THEN 1 ELSE 0 END) AS fsOn,
SUM(CASE WHEN requires_filesystem = 0 THEN 1 ELSE 0 END) AS fsOff
FROM tasks`,
).get() as { fsOn: number | null; fsOff: number | null } | undefined;
log.info(
`[migration v52] final distribution: requires_filesystem=1 → ${totals?.fsOn ?? 0}, requires_filesystem=0 → ${totals?.fsOff ?? 0}`,
);
},
},
];
}
+2
View File
@@ -17,6 +17,7 @@ import { createMigrationsV48V48 } from "./migrations/v48-v48";
import { createMigrationsV49V49 } from "./migrations/v49-v49";
import { createMigrationsV50V50 } from "./migrations/v50-v50";
import { createMigrationsV51V51 } from "./migrations/v51-v51";
import { createMigrationsV52V52 } from "./migrations/v52-v52";
import {
columnExists,
nowIso,
@@ -214,6 +215,7 @@ export function runMigrations(
...createMigrationsV49V49(deps),
...createMigrationsV50V50(deps),
...createMigrationsV51V51(deps),
...createMigrationsV52V52(deps),
];
// Pre-migration snapshot. If any migration is pending we take a full
+2 -2
View File
@@ -83,9 +83,9 @@ export function buildSnapshotMethods(
...companyArgs,
),
tasks: readAll<TaskRecord>(
`select id, company_id as companyId, project_id as projectId, goal_id as goalId, parent_id as parentId, title, description, assignee_agent_id as assigneeAgentId, workspace_id as workspaceId, status, priority, active_run_id as activeRunId, cost_usd as costUsd, task_type as taskType, requires_user_review as requiresUserReview, started_at as startedAt, completed_at as completedAt, cancelled_at as cancelledAt, created_at as createdAt, updated_at as updatedAt from tasks${companyWhere} order by created_at desc limit 2000`,
`select id, company_id as companyId, project_id as projectId, goal_id as goalId, parent_id as parentId, title, description, assignee_agent_id as assigneeAgentId, workspace_id as workspaceId, status, priority, active_run_id as activeRunId, cost_usd as costUsd, task_type as taskType, requires_user_review as requiresUserReview, requires_filesystem as requiresFilesystem, started_at as startedAt, completed_at as completedAt, cancelled_at as cancelledAt, created_at as createdAt, updated_at as updatedAt from tasks${companyWhere} order by created_at desc limit 2000`,
...companyArgs,
).map((record) => ({ ...record, requiresUserReview: toBoolean(record.requiresUserReview), startedAt: record.startedAt ?? null, completedAt: record.completedAt ?? null, cancelledAt: record.cancelledAt ?? null })) as TaskRecord[],
).map((record) => ({ ...record, requiresUserReview: toBoolean(record.requiresUserReview), requiresFilesystem: toBoolean(record.requiresFilesystem), startedAt: record.startedAt ?? null, completedAt: record.completedAt ?? null, cancelledAt: record.cancelledAt ?? null })) as TaskRecord[],
approvals: readAll<ApprovalRecord>(
`select id, company_id as companyId, related_task_id as relatedTaskId, related_agent_id as relatedAgentId, requested_by_agent_id as requestedByAgentId, type, payload_summary as payloadSummary, impact_summary as impactSummary, payload_json as payloadJson, decision_note as decisionNote, state, created_at as createdAt, updated_at as updatedAt from approvals${companyWhere} order by created_at desc limit 1000`,
...companyArgs,
+9
View File
@@ -156,6 +156,15 @@ export interface TaskRecord {
costUsd: number;
taskType: string | null;
requiresUserReview: boolean;
/**
* Per-task filesystem lock policy (v52+). When `true` the
* orchestrator resolves a workspace and holds it exclusively for
* the duration of the run; when `false` the run executes with
* `lockMode='none'` and agents run in parallel on shared read-only
* context. Inferred from agent role at task creation and toggled
* explicitly by callers that know their task mutates the tree.
*/
requiresFilesystem: boolean;
startedAt: string | null;
completedAt: string | null;
cancelledAt: string | null;