fix(db): make task row state authoritative

This commit is contained in:
ZacharyZhang-NY committed 2026-04-22 16:24:57 -04:00
1 parent ebcc15c68d
commit 775598ef4c
16 files changed
+75 -924

No files matched your search

+16 -18
View File
@@ -119,23 +119,21 @@ export default tseslint.config(
},
},
{
// B2 Phase 1 step 5/5 — block new raw status-writes on tasks.
// C-01 — keep task state writes behind the database domain modules.
//
// During dual-write every `UPDATE tasks SET status = ...` must go
// through `advanceTaskPhase` (or the four grandfathered sites in
// database-base.ts / database/tasks-*.ts / database/runs.ts that
// already dual-write and carry `eslint-disable-next-line
// no-restricted-syntax` with a comment explaining the exemption).
// The no-restricted-syntax rule below intercepts string literals
// and template literals whose text starts with the offending SQL
// prefix. New code that needs to mutate `tasks.status` must either
// route through `advanceTaskPhase` (preferred) or add an
// `eslint-disable-next-line` with a rationale comment.
//
// Migrations are deliberately excluded — schema changes operate on
// the legacy column by design.
// `tasks.status` and the same-row runtime lock columns are the single
// source of truth. Callers outside the database task/run modules must
// use `db.tasks.*` / `db.runs.*` helpers instead of issuing ad hoc SQL.
files: ["src/main/**/*.ts"],
ignores: ["src/main/database/migrations/**/*.ts"],
ignores: [
"src/main/database/migrations/**/*.ts",
"src/main/database/task-checkout.ts",
"src/main/database-base.ts",
"src/main/database/tasks-crud.ts",
"src/main/database/tasks-status.ts",
"src/main/database/tasks-update.ts",
"src/main/database/runs.ts",
],
rules: {
"no-restricted-syntax": [
"error",
@@ -148,13 +146,13 @@ export default tseslint.config(
selector:
"Literal[value=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s[^;]*\\bstatus\\s*=/]",
message:
"B2 Phase 1: write task.status through advanceTaskPhase (or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale for grandfathered sites).",
"C-01: write task.status through the database task/run helpers.",
},
{
selector:
"TemplateElement[value.raw=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s[^;]*\\bstatus\\s*=/]",
message:
"B2 Phase 1: write task.status through advanceTaskPhase (or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale for grandfathered sites).",
"C-01: write task.status through the database task/run helpers.",
},
{
// Dynamic-SQL bypass: `UPDATE tasks SET ${...}` where the
@@ -166,7 +164,7 @@ export default tseslint.config(
selector:
"TemplateElement[value.raw=/\\b[Uu][Pp][Dd][Aa][Tt][Ee]\\s+tasks\\s+[Ss][Ee][Tt]\\s*$/]",
message:
"B2 Phase 1: dynamic `UPDATE tasks SET ${...}` helpers bypass the literal-SET guard; route through advanceTaskPhase or tag with `// eslint-disable-next-line no-restricted-syntax` + dual-write rationale.",
"C-01: write task.status through the database task/run helpers.",
},
],
},
-57
View File
@@ -30,10 +30,6 @@ import { buildCredentialMethods } from "./database/credentials";
import { buildModelCapabilityMethods } from "./database/model-capabilities-crud";
import { MODEL_CAPABILITY_SEED } from "./model-capability-seed";
import { initializeDatabase } from "./database/schema";
import {
verifyPhaseConsistency,
type PhaseConsistencyReport,
} from "./database/tasks-phase-consistency";
import { SecretVault } from "./vault";
import type { AgentStatus, ConnectorId, ProfileSnapshot, RunStatus } from "@shared/types";
@@ -196,42 +192,6 @@ export class AppDatabaseBase {
this._lastInterruptedCount = this.interruptStaleRuns();
this.settings.purgeExpiredRunLogs();
this.wakeup.purgeExpiredWakeupRequests();
// B2 Phase 1 step 4/5 — dev-mode dual-write consistency probe.
//
// Under NODE_ENV=development (vite dev, tests) we log the phase /
// lease divergence count every boot. Production builds skip the
// scan to keep startup cheap; the renderer snapshot path has no
// opinion on the shadow columns yet, so a daily sweep is fine
// once step 5/5 lands the lint rule and step 4 graduates to a
// background metric.
if (process.env.NODE_ENV !== "production") {
try {
const report = verifyPhaseConsistency(this.db, { sampleLimit: 5 });
if (report.inconsistent > 0) {
console.warn(
`[tasks-phase-consistency] ${report.inconsistent} row(s) diverge between legacy columns and phase/lease shadow`,
report.mismatches,
);
}
} catch {
// Best-effort probe: a query failure here is diagnostic noise,
// not a reason to abort bootstrap. The dev-mode logger above
// would have captured the error anyway.
}
}
}
/**
* Report divergence between the legacy lock columns and the new
* phase/lease shadow. Intended for diagnostics — the scan is O(task
* rows) and must not run on hot paths. See
* `tasks-phase-consistency.verifyPhaseConsistency` for the full
* invariant description.
*/
verifyPhaseConsistency(options: { sampleLimit?: number } = {}): PhaseConsistencyReport {
return verifyPhaseConsistency(this.db, options);
}
close() { this.db.close(); }
@@ -276,7 +236,6 @@ export class AppDatabaseBase {
interruptedCount = runIds.length;
this.db
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: crash-recovery sweep scoped to the exact run ids captured above. The trigger mirrors status→phase; the DELETE below drops shadow leases for the same id set.
`update tasks set status = 'blocked', updated_at = ? where id in (select task_id from runs where id in (${placeholders})) and status in ('in_progress', 'todo')`,
)
.run(ts, ...runIds);
@@ -285,15 +244,6 @@ export class AppDatabaseBase {
`update tasks set active_run_id = null where active_run_id in (${placeholders})`,
)
.run(...runIds);
// B2 Phase 1 step 3/5 — dual-write: drop shadow leases whose run
// has been interrupted. Scoped by explicit run id list so two
// sweeps with identical ISO-millisecond `finished_at` stamps can
// never alias each other's lease cleanup.
this.db
.prepare(
`DELETE FROM task_leases WHERE run_id IN (${placeholders})`,
)
.run(...runIds);
this.db.prepare("update agents set status = 'idle', updated_at = ? where status = 'running'").run(ts);
this.workspacesMod.releaseAllWorkspaceLocks();
});
@@ -310,16 +260,9 @@ export class AppDatabaseBase {
for (const task of stale) {
this.db
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: stale-lock recovery bounces the task back to todo and drops the shadow lease on the next statement; trigger mirrors status→phase.
`UPDATE tasks SET checkout_run_id = NULL, execution_locked_at = NULL, status = 'todo', updated_at = ? WHERE id = ?`,
)
.run(ts, task.id);
// B2 Phase 1 step 3/5 — dual-write: drop the shadow lease whose
// legacy counterpart was just torn down. Matches the legacy
// path's "clear everything for this stuck task" semantics; the
// trigger that mirrored status → phase has already advanced
// `tasks.phase` back to `todo` so the new surface stays coherent.
this.db.prepare(`DELETE FROM task_leases WHERE task_id = ?`).run(task.id);
}
return stale.map((t) => ({ taskId: t.id, companyId: t.companyId, title: t.title }));
}
+29
View File
@@ -0,0 +1,29 @@
import log from "electron-log/main.js";
import type { Migration, MigrationDeps } from "./types";
import { columnExists, tableExists } from "./types";
export function createMigrationsV55V55(deps: MigrationDeps): Migration[] {
const { db } = deps;
return [
{
version: 55,
name: "tasks_single_row_state",
fn: () => {
db.exec(`
DROP TRIGGER IF EXISTS trg_tasks_phase_from_status_insert;
DROP TRIGGER IF EXISTS trg_tasks_phase_from_status_update;
`);
if (tableExists(db, "task_leases")) {
db.exec("DROP TABLE task_leases");
log.info("[migration v55] task_leases shadow table dropped");
}
if (tableExists(db, "tasks") && columnExists(db, "tasks", "phase")) {
db.exec("ALTER TABLE tasks DROP COLUMN phase");
log.info("[migration v55] tasks.phase shadow column dropped");
}
},
},
];
}
-11
View File
@@ -207,11 +207,6 @@ export function buildRunMethods(
getDb()
.prepare("UPDATE tasks SET checkout_run_id = NULL, execution_locked_at = NULL, updated_at = ? WHERE id = ? AND checkout_run_id = ?")
.run(ts, run.taskId, input.runId);
// B2 Phase 1 step 3/5 — dual-write: release the shadow lease
// for this run at the same instant the legacy lock is cleared.
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ? AND run_id = ?")
.run(run.taskId, input.runId);
if (runNeverStarted) {
// Roll the task back to `todo` when no *other* live run claims
// it. Invariant: `task.status = 'in_progress'` ⇒ a queued or
@@ -223,7 +218,6 @@ export function buildRunMethods(
// NOT EXISTS clause is the canonical "no live owner" check.
getDb()
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: finishRun's "runNeverStarted" rollback; trigger mirrors status→phase, shadow lease already dropped above
`UPDATE tasks
SET status = 'todo',
checkout_run_id = NULL,
@@ -276,11 +270,6 @@ export function buildRunMethods(
getDb()
.prepare("UPDATE tasks SET checkout_run_id = NULL, execution_locked_at = NULL, updated_at = ? WHERE id = ? AND checkout_run_id = ?")
.run(ts, taskId, runId);
// B2 Phase 1 step 3/5 — dual-write: drop the shadow lease when
// the legacy cancellation clears checkout_run_id.
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ? AND run_id = ?")
.run(taskId, runId);
}
if (workspaceId) {
getDb()
+2
View File
@@ -20,6 +20,7 @@ import { createMigrationsV51V51 } from "./migrations/v51-v51";
import { createMigrationsV52V52 } from "./migrations/v52-v52";
import { createMigrationsV53V53 } from "./migrations/v53-v53";
import { createMigrationsV54V54 } from "./migrations/v54-v54";
import { createMigrationsV55V55 } from "./migrations/v55-v55";
import {
columnExists,
nowIso,
@@ -221,6 +222,7 @@ export function runMigrations(
...createMigrationsV52V52(deps),
...createMigrationsV53V53(deps),
...createMigrationsV54V54(deps),
...createMigrationsV55V55(deps),
];
// Pre-migration snapshot. If any migration is pending we take a full
-10
View File
@@ -1,7 +1,5 @@
import type { DatabaseSync } from "node:sqlite";
export const TASK_LEASE_TTL_MS = 15 * 60 * 1000;
function isoFromMs(ms: number): string {
return new Date(ms).toISOString();
}
@@ -25,7 +23,6 @@ export function checkoutTaskInTransaction(
const ts = isoFromMs(nowMs);
const result = db
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: checkout CAS is the single sanctioned status='in_progress' lock acquisition. The same transaction writes active_run_id and task_leases.
`UPDATE tasks
SET checkout_run_id = ?,
active_run_id = ?,
@@ -42,12 +39,5 @@ export function checkoutTaskInTransaction(
return false;
}
const expiresAt = isoFromMs(nowMs + TASK_LEASE_TTL_MS);
db.prepare(
`INSERT OR REPLACE INTO task_leases
(task_id, run_id, acquired_at, expires_at, checkout_run_id)
VALUES (?, ?, ?, ?, ?)`,
).run(taskId, runId, ts, expiresAt, runId);
return true;
}
+2 -14
View File
@@ -8,14 +8,6 @@ export const VALID_TASK_STATUSES = new Set([
"cancelled",
]);
const STATUSES_THAT_CLEAR_LEASE = new Set<string>([
"done",
"cancelled",
"blocked",
"todo",
"backlog",
]);
export function assertValidTaskStatus(status: string): void {
if (!VALID_TASK_STATUSES.has(status)) {
throw new Error(`Invalid task status: ${status}`);
@@ -25,7 +17,7 @@ export function assertValidTaskStatus(status: string): void {
export function buildTaskStatusTransition(
newStatus: string,
timestamp: string,
): { sets: string[]; values: (string | null)[]; clearsLease: boolean } {
): { sets: string[]; values: (string | null)[] } {
const sets: string[] = ["status = ?", "updated_at = ?"];
const values: (string | null)[] = [newStatus, timestamp];
@@ -52,9 +44,5 @@ export function buildTaskStatusTransition(
sets.push("active_run_id = NULL");
}
return {
sets,
values,
clearsLease: STATUSES_THAT_CLEAR_LEASE.has(newStatus),
};
return { sets, values };
}
-13
View File
@@ -77,7 +77,6 @@ export function buildTaskCrudMethods(
if (input.requiresFilesystem === undefined) {
getDb()
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: saveTask upsert path writes the full row (including status) as a single atomic UPDATE. The status→phase trigger keeps the shadow phase column in sync; no lease is ever held by saveTask callers (lease writes live behind checkoutTask).
"update tasks set project_id = ?, goal_id = ?, parent_id = ?, title = ?, description = ?, assignee_agent_id = ?, workspace_id = ?, priority = ?, status = ?, updated_at = ? where id = ? and company_id = ?",
)
.run(
@@ -97,7 +96,6 @@ export function buildTaskCrudMethods(
} else {
getDb()
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: saveTask upsert path writes the full row (including status) as a single atomic UPDATE. The status→phase trigger keeps the shadow phase column in sync; no lease is ever held by saveTask callers (lease writes live behind checkoutTask).
"update tasks set project_id = ?, goal_id = ?, parent_id = ?, title = ?, description = ?, assignee_agent_id = ?, workspace_id = ?, priority = ?, status = ?, requires_filesystem = ?, updated_at = ? where id = ? and company_id = ?",
)
.run(
@@ -167,17 +165,6 @@ export function buildTaskCrudMethods(
"UPDATE tasks SET checkout_run_id = NULL, execution_locked_at = NULL, updated_at = ? WHERE id = ? AND checkout_run_id = ?",
)
.run(ts, run.taskId, run.id);
// B2 Phase 1 step 3/5 — dual-write: cascade to the shadow
// lease row so a task-delete does not leave an orphan lease
// behind. The FK on `task_leases.task_id` would also cascade
// via the DELETE at the end of this transaction; we clear
// here explicitly so the intermediate state (task still
// present, lease already gone) matches what `releaseTaskLock`
// would have done if the delete had gone through the normal
// lifecycle path.
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ? AND run_id = ?")
.run(run.taskId, run.id);
if (run.workspaceId) {
getDb()
.prepare(
@@ -1,164 +0,0 @@
import type { DatabaseSync } from "node:sqlite";
/**
* B2 Phase 1 step 4/5 — dual-write consistency observability.
*
* `verifyPhaseConsistency` inspects the live invariants between the
* legacy `tasks.status` / `tasks.checkout_run_id` / `tasks.active_run_id`
* columns and the new `tasks.phase` / `task_leases` shadow surfaces.
* It does not mutate anything; it reports the divergence count and a
* small sample so an operator (or the startup hook in dev mode) can
* confirm dual-write landed the invariants intended by step 3/5.
*
* The check intentionally accepts that `status = 'in_review'` maps
* onto `phase = 'in_progress'` — the canonical phase enum supplied by
* the user collapses the review stage into the in-flight phase, so an
* `in_review` task with `phase = 'in_progress'` is *not* a divergence.
*
* This is observability only: step 5/5 adds the ESLint rule that
* prevents new divergence from reaching production, and a future
* phase swaps the reads over once the mismatch count stabilises at
* zero.
*/
export interface PhaseConsistencySample {
taskId: string;
status: string;
phase: string | null;
activeRunId: string | null;
checkoutRunId: string | null;
leaseRunId: string | null;
leaseExpiresAt: string | null;
reason: string;
}
export interface PhaseConsistencyReport {
/**
* Total number of task rows whose legacy columns disagree with the
* shadow surfaces in a way that dual-write should prevent. Zero is
* the healthy steady state.
*/
inconsistent: number;
/**
* Up to `sampleLimit` offending rows. Operators use these to drive
* root-cause investigation without materialising the entire diff.
*/
mismatches: PhaseConsistencySample[];
}
/**
* Map from legacy `status` to the expected `phase` under dual-write.
* Any status value missing from this map is an observability miss —
* the caller is expected to extend both this table and the v50
* backfill/trigger simultaneously. Leaving the entry unmapped here
* would cause every such row to report as inconsistent forever.
*/
const STATUS_TO_EXPECTED_PHASE: Record<string, string> = {
backlog: "backlog",
todo: "todo",
in_progress: "in_progress",
blocked: "blocked",
done: "completed",
cancelled: "cancelled",
in_review: "in_progress",
};
interface RawRow {
id: string;
status: string;
phase: string | null;
active_run_id: string | null;
checkout_run_id: string | null;
lease_run_id: string | null;
lease_expires_at: string | null;
}
/**
* Compute the diff between the legacy lock columns and the new phase
* + lease shadow. Reports three kinds of mismatch:
*
* - `phase_unexpected`: the shadow `phase` does not match what the
* current `status` predicts per the mapping table above. Indicates
* the trigger fired on an unexpected status value, or the row was
* written through a path that bypassed the trigger.
* - `lease_missing`: the legacy columns say the task is owned by a
* live run (`active_run_id` or `checkout_run_id` non-null) but the
* shadow `task_leases` row is absent. Indicates a dual-write site
* that still writes the legacy columns only.
* - `lease_orphan`: the shadow has a lease but neither legacy
* pointer is set. Indicates a lease release that only ran against
* the legacy path (or a race that landed the lease after the
* legacy clear).
*
* Expired leases whose legacy pointer is also cleared are ignored —
* they are a stale-lock sweep artefact that a later recoverStaleLocks
* call would flush anyway.
*/
export function verifyPhaseConsistency(
db: DatabaseSync,
options: { sampleLimit?: number } = {},
): PhaseConsistencyReport {
const sampleLimit = Math.max(0, options.sampleLimit ?? 20);
const rows = db
.prepare(
`SELECT t.id,
t.status,
t.phase,
t.active_run_id,
t.checkout_run_id,
l.run_id AS lease_run_id,
l.expires_at AS lease_expires_at
FROM tasks t
LEFT JOIN task_leases l ON l.task_id = t.id`,
)
.all() as unknown as RawRow[];
const mismatches: PhaseConsistencySample[] = [];
const nowMs = Date.now();
let inconsistent = 0;
for (const row of rows) {
const expectedPhase = STATUS_TO_EXPECTED_PHASE[row.status];
const hasLegacyOwnership =
row.active_run_id !== null || row.checkout_run_id !== null;
const hasLease = row.lease_run_id !== null;
const leaseExpiresMs = row.lease_expires_at
? Date.parse(row.lease_expires_at)
: Number.NaN;
const leaseExpired =
hasLease && Number.isFinite(leaseExpiresMs) && leaseExpiresMs <= nowMs;
const reasons: string[] = [];
if (expectedPhase === undefined) {
reasons.push(`status=${row.status} has no phase mapping`);
} else if (row.phase !== expectedPhase) {
reasons.push(`phase=${row.phase ?? "<null>"} expected ${expectedPhase}`);
}
if (hasLegacyOwnership && !hasLease) {
reasons.push("legacy owner set but no lease row");
}
if (!hasLegacyOwnership && hasLease && !leaseExpired) {
reasons.push("lease row present but no legacy owner");
}
if (reasons.length === 0) continue;
inconsistent += 1;
if (mismatches.length < sampleLimit) {
mismatches.push({
taskId: row.id,
status: row.status,
phase: row.phase,
activeRunId: row.active_run_id,
checkoutRunId: row.checkout_run_id,
leaseRunId: row.lease_run_id,
leaseExpiresAt: row.lease_expires_at,
reason: reasons.join("; "),
});
}
}
return { inconsistent, mismatches };
}
-163
View File
@@ -1,163 +0,0 @@
import type { DatabaseSync } from "node:sqlite";
import { nowIso } from "./helpers";
/**
* B2 Phase 1 step 2/5 — pure helpers for the new phase + lease shadow.
*
* These helpers expose the shape the state-machine refactor will
* eventually take — a single CAS function for phase transitions, and
* a lease table with explicit acquire/release/renew — without yet
* swapping the production call sites over. Step 3/5 wires the existing
* writes (checkoutTask, finishRun, interruptStaleRuns, recoverStaleLocks)
* to call these helpers in a dual-write arrangement alongside the
* legacy columns; until then the helpers are callable but unused.
*
* The enum here matches the v50 migration CHECK constraint exactly.
* Any drift between this module and v50-v50.ts is a bug — the TypeScript
* `TaskPhase` union is the compile-time mirror of the SQL constraint.
*/
/** Canonical phase enum matching the v50 CHECK constraint. */
export type TaskPhase =
| "todo"
| "backlog"
| "in_progress"
| "blocked"
| "completed"
| "cancelled";
export interface TaskLeaseRow {
taskId: string;
runId: string;
acquiredAt: string;
expiresAt: string;
checkoutRunId: string | null;
}
export interface AcquireTaskLeaseInput {
taskId: string;
runId: string;
/** Lease time-to-live in milliseconds. Mapped onto `expires_at`. */
ttlMs: number;
/** Optional `checkout_run_id` to mirror onto the lease row. */
checkoutRunId?: string | null;
}
export interface TasksPhaseMethods {
/**
* Compare-and-swap phase transition. Returns true when the row was
* updated (current phase matched `from`), false otherwise. The caller
* is responsible for wrapping lease writes in the same transaction
* when the transition is coupled to a lease acquire/release.
*/
advanceTaskPhase(taskId: string, from: TaskPhase, to: TaskPhase): boolean;
/**
* Insert or replace the lease row for `taskId`. When an unexpired
* lease for a *different* run already exists, returns null and writes
* nothing. Callers treat null as "another run owns this task; do not
* proceed".
*
* The staleness check is a clock comparison against `now()` — a lease
* whose `expires_at` is in the past is considered releasable, so a
* subsequent acquire from a new run replaces it. This mirrors the
* existing `recoverStaleLocks` behavior but scoped to the new
* lease table.
*/
acquireTaskLease(input: AcquireTaskLeaseInput): TaskLeaseRow | null;
/**
* Release the lease for `taskId` when the current row belongs to
* `runId`. Returns true when a row was deleted.
*/
releaseTaskLease(taskId: string, runId: string): boolean;
/**
* Read the current lease row (if any) for `taskId`.
*/
getTaskLease(taskId: string): TaskLeaseRow | null;
}
export function buildTasksPhaseMethods(
getDb: () => DatabaseSync,
): TasksPhaseMethods {
function readLease(taskId: string): TaskLeaseRow | null {
const row = getDb()
.prepare(
`SELECT task_id AS taskId,
run_id AS runId,
acquired_at AS acquiredAt,
expires_at AS expiresAt,
checkout_run_id AS checkoutRunId
FROM task_leases
WHERE task_id = ?`,
)
.get(taskId) as TaskLeaseRow | undefined;
return row ?? null;
}
return {
advanceTaskPhase(taskId, from, to) {
const ts = nowIso();
const result = getDb()
.prepare(
`UPDATE tasks
SET phase = ?, updated_at = ?
WHERE id = ? AND phase = ?`,
)
.run(to, ts, taskId, from);
return result.changes > 0;
},
acquireTaskLease({ taskId, runId, ttlMs, checkoutRunId }) {
if (!Number.isFinite(ttlMs) || ttlMs <= 0) {
throw new Error(
`acquireTaskLease: ttlMs must be a positive finite number, got ${ttlMs}`,
);
}
const nowMs = Date.now();
const acquiredAt = new Date(nowMs).toISOString();
const expiresAt = new Date(nowMs + ttlMs).toISOString();
const existing = readLease(taskId);
if (existing) {
const existingExpiresMs = Date.parse(existing.expiresAt);
const stillLive =
Number.isFinite(existingExpiresMs) && existingExpiresMs > nowMs;
if (stillLive && existing.runId !== runId) {
return null;
}
// Same run refreshing its own lease, or a stale lease to be
// replaced. Either way we rewrite the row with the new window.
getDb()
.prepare(
`UPDATE task_leases
SET run_id = ?, acquired_at = ?, expires_at = ?, checkout_run_id = ?
WHERE task_id = ?`,
)
.run(runId, acquiredAt, expiresAt, checkoutRunId ?? null, taskId);
return readLease(taskId);
}
getDb()
.prepare(
`INSERT INTO task_leases
(task_id, run_id, acquired_at, expires_at, checkout_run_id)
VALUES (?, ?, ?, ?, ?)`,
)
.run(taskId, runId, acquiredAt, expiresAt, checkoutRunId ?? null);
return readLease(taskId);
},
releaseTaskLease(taskId, runId) {
const result = getDb()
.prepare(`DELETE FROM task_leases WHERE task_id = ? AND run_id = ?`)
.run(taskId, runId);
return result.changes > 0;
},
getTaskLease(taskId) {
return readLease(taskId);
},
};
}
+1 -33
View File
@@ -7,7 +7,7 @@ import {
taskPriorityOrderSql,
} from "./helpers";
import { chooseBestTaskAssignee } from "./task-assignment";
import { TASK_LEASE_TTL_MS, checkoutTaskInTransaction } from "./task-checkout";
import { checkoutTaskInTransaction } from "./task-checkout";
import { guardInReviewHasReviewer } from "./task-review-guard";
import { assertValidTaskStatus, buildTaskStatusTransition } from "./task-status-rules";
@@ -49,22 +49,9 @@ export function buildTaskStatusMethods(
transaction(() => {
getDb()
.prepare(
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: advanceTaskStatus is the sanctioned dynamic-SQL helper; transition.sets[0] is always the literal "status = ?". Status→phase trigger plus the explicit DELETE FROM task_leases below keep the shadow surface coherent.
`UPDATE tasks SET ${transition.sets.join(", ")} WHERE id = ?`,
)
.run(...transition.values, taskId);
if (transition.clearsLease) {
// Drop any live lease row in the same transaction so no
// caller observes the intermediate state of "task in terminal
// or backlog phase, lease still present". `releaseTaskLock`
// clears the lease on the happy path (normal run completion),
// but terminal transitions can come from review timeouts,
// retries, automation rules, etc. without ever going through
// that helper.
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ?")
.run(taskId);
}
});
if (comment) {
@@ -108,37 +95,20 @@ export function buildTaskStatusMethods(
releaseTaskLock(taskId: string, runId: string): void {
const ts = nowIso();
getDb()
.prepare(
"UPDATE tasks SET checkout_run_id = NULL, execution_locked_at = NULL, updated_at = ? WHERE id = ? AND checkout_run_id = ?",
)
.run(ts, taskId, runId);
// B2 Phase 1 step 3/5 — dual-write: drop the shadow lease too.
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ? AND run_id = ?")
.run(taskId, runId);
},
renewTaskLease(taskId: string, runId: string): boolean {
const ts = nowIso();
const result = getDb()
.prepare(
"UPDATE tasks SET execution_locked_at = ?, updated_at = ? WHERE id = ? AND checkout_run_id = ?",
)
.run(ts, ts, taskId, runId);
if (result.changes > 0) {
// Shadow renewal: push `expires_at` forward in lockstep with
// `execution_locked_at` so the stale-lock sweep over the new
// table stays consistent with the legacy sweep.
const expiresAt = new Date(Date.now() + TASK_LEASE_TTL_MS).toISOString();
getDb()
.prepare(
`UPDATE task_leases SET expires_at = ? WHERE task_id = ? AND run_id = ?`,
)
.run(expiresAt, taskId, runId);
}
return result.changes > 0;
},
@@ -250,7 +220,6 @@ export function buildTaskStatusMethods(
): void {
const ts = nowIso();
getDb()
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: crash-recovery retry bounces task back to todo; trigger mirrors status→phase, no lease held at this point
.prepare("update tasks set status = 'todo', updated_at = ? where id = ?")
.run(ts, taskId);
getDb()
@@ -276,7 +245,6 @@ export function buildTaskStatusMethods(
markInterruptedTaskFailed(taskId: string): void {
const ts = nowIso();
getDb()
// eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: crash-recovery terminal transition; trigger mirrors status→phase, lease was already cleared by interruptStaleRuns
.prepare("update tasks set status = 'cancelled', updated_at = ? where id = ?")
.run(ts, taskId);
},
-6
View File
@@ -211,17 +211,11 @@ export function buildTaskUpdateMethods(
}
const updateResult = getDb()
// eslint-disable-next-line no-restricted-syntax -- R-03 sanctioned domain update helper: SET clauses come from fixed literals only; status->phase trigger and explicit task_leases delete keep the dual-write surface coherent inside this transaction.
.prepare(`update tasks set ${sets.join(", ")} where id = ? and company_id = ?`)
.run(...values, input.taskId, input.companyId);
if (updateResult.changes === 0) {
throw new Error(`Task not found: ${input.taskId}`);
}
if (transition?.clearsLease) {
getDb()
.prepare("DELETE FROM task_leases WHERE task_id = ?")
.run(input.taskId);
}
if (newStatus === "done" && current.status !== "done") {
incrementActiveSprintVelocity(input.companyId, 1);
}
-2
View File
@@ -4,7 +4,6 @@ import { buildTaskCrudMethods } from "./tasks-crud";
import { buildTaskQueryMethods } from "./task-queries";
import { buildTaskSessionMethods } from "./task-sessions-crud";
import { buildTaskStatusMethods } from "./tasks-status";
import { buildTasksPhaseMethods } from "./tasks-phase";
import { buildTaskUpdateMethods } from "./tasks-update";
export function buildTaskMethods(
@@ -33,6 +32,5 @@ export function buildTaskMethods(
...buildTaskSessionMethods(getDb, readAll, transaction),
...buildTaskStatusMethods(getDb, readAll, transaction, deps),
...buildTaskUpdateMethods(getDb, readAll, transaction, deps),
...buildTasksPhaseMethods(getDb),
};
}
+10 -9
View File
@@ -157,13 +157,14 @@ describe("CAS checkout", () => {
const task = db.entityCrud.getTask(taskId);
expect(task.activeRunId).toBeNull();
expect(task.status).toBe("todo");
const lease = (db as unknown as RawDb).db
.prepare("SELECT run_id FROM task_leases WHERE task_id = ?")
.get(taskId);
expect(lease).toBeUndefined();
const lock = (db as unknown as RawDb).db
.prepare("SELECT checkout_run_id, execution_locked_at FROM tasks WHERE id = ?")
.get(taskId) as { checkout_run_id: string | null; execution_locked_at: string | null };
expect(lock.checkout_run_id).toBeNull();
expect(lock.execution_locked_at).toBeNull();
});
it("createTaskRun atomically creates the run and claims the task lease", () => {
it("createTaskRun atomically creates the run and claims the task row lock", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
@@ -182,10 +183,10 @@ describe("CAS checkout", () => {
const task = db.entityCrud.getTask(taskId);
expect(task.status).toBe("in_progress");
expect(task.activeRunId).toBe(run?.id);
const lease = (db as unknown as RawDb).db
.prepare("SELECT run_id FROM task_leases WHERE task_id = ?")
.get(taskId) as { run_id: string } | undefined;
expect(lease?.run_id).toBe(run?.id);
const lock = (db as unknown as RawDb).db
.prepare("SELECT checkout_run_id FROM tasks WHERE id = ?")
.get(taskId) as { checkout_run_id: string | null };
expect(lock.checkout_run_id).toBe(run?.id);
});
it("createTaskRun rolls back the run row when checkout loses the race", () => {
+15
View File
@@ -33,6 +33,21 @@ describe("AppDatabase", () => {
await rm(tempDir, { recursive: true, force: true });
});
it("keeps task runtime state on the tasks row", () => {
const shadowTables = db.queryAll<{ name: string }>(
"select name from sqlite_master where type = 'table' and name = 'task_leases'",
);
const phaseColumns = db.queryAll<{ name: string }>("pragma table_info(tasks)")
.filter((column) => column.name === "phase");
const phaseTriggers = db.queryAll<{ name: string }>(
"select name from sqlite_master where type = 'trigger' and name like 'trg_tasks_phase_from_status_%'",
);
expect(shadowTables).toEqual([]);
expect(phaseColumns).toEqual([]);
expect(phaseTriggers).toEqual([]);
});
it("persists company-scoped entities and resolves secret env bindings", () => {
const companyId = db.company.saveCompany({
name: "Northline Systems",
-424
View File
@@ -1,424 +0,0 @@
import { join } from "node:path";
import { tmpdir } from "node:os";
import { mkdtemp, rm } from "node:fs/promises";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
vi.mock("electron", () => ({
safeStorage: {
isEncryptionAvailable: () => false,
getSelectedStorageBackend: () => "basic_text",
encryptString: (value: string) => Buffer.from(value, "utf8"),
decryptString: (value: Buffer) => value.toString("utf8"),
},
}));
import { AppDatabase } from "../../src/main/database";
/**
* B2 Phase 1 step 2/5 — unit coverage for the new phase + lease helpers.
*
* These tests exercise the shadow surface directly against a real
* SQLite database: v50 has run, the `tasks.phase` column and
* `task_leases` table exist, and the new helpers write and read them
* without going through any production code path. The goal is to pin
* the state machine semantics (CAS rejection on wrong `from`, lease
* replacement on staleness, release idempotency) before step 3/5 wires
* the helpers into `checkoutTask` / `finishRun`.
*/
function createTestCompany(db: AppDatabase): string {
return db.company.saveCompany({
name: "Phase Helper Company",
description: "B2 phase + lease unit tests",
status: "active",
});
}
function createTestConnector(db: AppDatabase): void {
db.connectorsCrud.saveConnector({
id: "openai-codex",
model: "gpt-5",
envBindingText: "",
notes: "Test connector",
});
}
function createTestWorkspace(db: AppDatabase, companyId: string): string {
return db.workspacesMod.saveWorkspace({
companyId,
name: "Phase Workspace",
localPath: "/tmp/phase-workspace",
repoUrl: "https://github.com/example/phase",
repoRef: "main",
isPrimary: true,
});
}
function createTestAgent(db: AppDatabase, companyId: string, workspaceId: string, name = "Agent"): string {
return db.agents.saveAgent({
companyId,
name,
role: "IC",
title: name,
status: "active",
reportsTo: null,
connectorId: "openai-codex",
workspaceId,
model: null,
capabilities: "Run task helpers",
budgetMonthlyUsd: 100,
});
}
function createTestTask(
db: AppDatabase,
companyId: string,
agentId: string,
workspaceId: string,
): string {
return db.tasks.saveTask({
companyId,
title: "Phase helper task",
description: "dual-write seed",
assigneeAgentId: agentId,
workspaceId,
priority: "medium",
status: "todo",
});
}
function createTestRun(
db: AppDatabase,
companyId: string,
taskId: string,
agentId: string,
workspaceId: string,
): string {
const { id } = db.runs.createRun({
companyId,
taskId,
agentId,
workspaceId,
connectorId: "openai-codex",
});
return id;
}
describe("tasks-phase helpers", () => {
let tempDir: string;
let db: AppDatabase;
beforeEach(async () => {
tempDir = await mkdtemp(join(tmpdir(), "agentcompany-phase-"));
db = new AppDatabase(tempDir);
db.init();
});
afterEach(async () => {
db.close();
await rm(tempDir, { recursive: true, force: true });
});
describe("advanceTaskPhase", () => {
it("backfill leaves every existing task with a phase matching its status", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
// `saveTask` ran with the v50 migration already applied, so the
// row must carry a phase value immediately.
const row = (db as unknown as {
db: { prepare: (sql: string) => { get: (...args: unknown[]) => unknown } };
}).db
.prepare("SELECT phase FROM tasks WHERE id = ?")
.get(taskId) as { phase: string } | undefined;
expect(row?.phase).toBe("todo");
});
it("CAS transition succeeds when the current phase matches `from`", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const ok = db.tasks.advanceTaskPhase(taskId, "todo", "in_progress");
expect(ok).toBe(true);
const after = db.tasks.getTaskLease(taskId);
expect(after).toBeNull(); // phase transitions do not auto-create leases
const row = (db as unknown as {
db: { prepare: (sql: string) => { get: (...args: unknown[]) => unknown } };
}).db
.prepare("SELECT phase FROM tasks WHERE id = ?")
.get(taskId) as { phase: string } | undefined;
expect(row?.phase).toBe("in_progress");
});
it("CAS transition refuses when the current phase does not match `from`", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
// Row is at phase=todo; pretending it was in_progress must fail.
const ok = db.tasks.advanceTaskPhase(taskId, "in_progress", "completed");
expect(ok).toBe(false);
});
it("CHECK constraint rejects a write to an unknown phase value", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
// Using the raw DB to bypass the TypeScript enum — the CHECK
// constraint installed in v50 must still reject the write.
expect(() => {
(db as unknown as {
db: { prepare: (sql: string) => { run: (...args: unknown[]) => unknown } };
}).db
.prepare("UPDATE tasks SET phase = ? WHERE id = ?")
.run("bogus", taskId);
}).toThrow();
});
});
describe("acquireTaskLease", () => {
it("acquires a lease when none exists and returns the row", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
const lease = db.tasks.acquireTaskLease({
taskId,
runId,
ttlMs: 60_000,
checkoutRunId: runId,
});
expect(lease).not.toBeNull();
expect(lease?.taskId).toBe(taskId);
expect(lease?.runId).toBe(runId);
expect(lease?.checkoutRunId).toBe(runId);
});
it("refreshing the lease for the same run succeeds and moves expires_at forward", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
const first = db.tasks.acquireTaskLease({ taskId, runId, ttlMs: 10_000 });
expect(first).not.toBeNull();
const renewed = db.tasks.acquireTaskLease({ taskId, runId, ttlMs: 60_000 });
expect(renewed).not.toBeNull();
expect(Date.parse(renewed!.expiresAt)).toBeGreaterThanOrEqual(Date.parse(first!.expiresAt));
});
it("refuses to hand the lease to a different live run", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runA = createTestRun(db, companyId, taskId, agentId, workspaceId);
const runB = createTestRun(db, companyId, taskId, agentId, workspaceId);
const ownedByA = db.tasks.acquireTaskLease({ taskId, runId: runA, ttlMs: 60_000 });
expect(ownedByA).not.toBeNull();
const blocked = db.tasks.acquireTaskLease({ taskId, runId: runB, ttlMs: 60_000 });
expect(blocked).toBeNull();
const current = db.tasks.getTaskLease(taskId);
expect(current?.runId).toBe(runA);
});
it("replaces a stale lease when the previous window has expired", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runA = createTestRun(db, companyId, taskId, agentId, workspaceId);
const runB = createTestRun(db, companyId, taskId, agentId, workspaceId);
const ownedByA = db.tasks.acquireTaskLease({ taskId, runId: runA, ttlMs: 60_000 });
expect(ownedByA).not.toBeNull();
// Force the lease into the past by rewriting expires_at directly.
const pastIso = new Date(Date.now() - 1_000).toISOString();
(db as unknown as {
db: { prepare: (sql: string) => { run: (...args: unknown[]) => unknown } };
}).db
.prepare("UPDATE task_leases SET expires_at = ? WHERE task_id = ?")
.run(pastIso, taskId);
const takenByB = db.tasks.acquireTaskLease({ taskId, runId: runB, ttlMs: 60_000 });
expect(takenByB).not.toBeNull();
expect(takenByB?.runId).toBe(runB);
});
it("throws when ttlMs is not a positive finite number", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
expect(() =>
db.tasks.acquireTaskLease({ taskId, runId, ttlMs: 0 }),
).toThrow(/positive finite number/);
expect(() =>
db.tasks.acquireTaskLease({ taskId, runId, ttlMs: Number.NaN }),
).toThrow(/positive finite number/);
});
});
describe("advanceTaskStatus lease cleanup", () => {
// B2 Phase 1 follow-up (Linus review ask #1): advanceTaskStatus
// used to write only the status column, leaving any lease row
// behind as an orphan. The helpers below seed a lease directly,
// then drive status transitions and assert the shadow row lifecycle.
function seedLease(taskId: string, runId: string): void {
const acquired = new Date().toISOString();
const expires = new Date(Date.now() + 60_000).toISOString();
(db as unknown as {
db: { prepare: (sql: string) => { run: (...args: unknown[]) => unknown } };
}).db
.prepare(
`INSERT OR REPLACE INTO task_leases
(task_id, run_id, acquired_at, expires_at, checkout_run_id)
VALUES (?, ?, ?, ?, ?)`,
)
.run(taskId, runId, acquired, expires, runId);
}
function readLease(taskId: string): { run_id: string } | undefined {
return (db as unknown as {
db: { prepare: (sql: string) => { get: (...args: unknown[]) => unknown } };
}).db
.prepare("SELECT run_id FROM task_leases WHERE task_id = ?")
.get(taskId) as { run_id: string } | undefined;
}
it("drops the lease when the task transitions to done", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.advanceTaskStatus(taskId, "in_progress");
seedLease(taskId, runId);
expect(readLease(taskId)?.run_id).toBe(runId);
db.tasks.advanceTaskStatus(taskId, "done");
expect(readLease(taskId)).toBeUndefined();
});
it("drops the lease when the task transitions to cancelled", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.advanceTaskStatus(taskId, "in_progress");
seedLease(taskId, runId);
db.tasks.advanceTaskStatus(taskId, "cancelled");
expect(readLease(taskId)).toBeUndefined();
});
it("drops the lease when the task transitions to blocked", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.advanceTaskStatus(taskId, "in_progress");
seedLease(taskId, runId);
db.tasks.advanceTaskStatus(taskId, "blocked");
expect(readLease(taskId)).toBeUndefined();
});
it("drops the lease when the task is bounced back to todo", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.advanceTaskStatus(taskId, "in_progress");
seedLease(taskId, runId);
db.tasks.advanceTaskStatus(taskId, "todo");
expect(readLease(taskId)).toBeUndefined();
});
it("leaves the lease intact when the task transitions to in_review", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
// in_review guard requires either a reports_to or requires_user_review.
db.tasks.setTaskRequiresUserReview(
createTestTask(db, companyId, agentId, workspaceId),
true,
);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
db.tasks.setTaskRequiresUserReview(taskId, true);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.advanceTaskStatus(taskId, "in_progress");
seedLease(taskId, runId);
db.tasks.advanceTaskStatus(taskId, "in_review");
expect(readLease(taskId)?.run_id).toBe(runId);
});
});
describe("releaseTaskLease", () => {
it("releases the lease when the runId matches", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runId = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.acquireTaskLease({ taskId, runId, ttlMs: 60_000 });
const released = db.tasks.releaseTaskLease(taskId, runId);
expect(released).toBe(true);
expect(db.tasks.getTaskLease(taskId)).toBeNull();
});
it("does nothing when a different runId asks to release", () => {
const companyId = createTestCompany(db);
createTestConnector(db);
const workspaceId = createTestWorkspace(db, companyId);
const agentId = createTestAgent(db, companyId, workspaceId);
const taskId = createTestTask(db, companyId, agentId, workspaceId);
const runA = createTestRun(db, companyId, taskId, agentId, workspaceId);
const runB = createTestRun(db, companyId, taskId, agentId, workspaceId);
db.tasks.acquireTaskLease({ taskId, runId: runA, ttlMs: 60_000 });
const released = db.tasks.releaseTaskLease(taskId, runB);
expect(released).toBe(false);
const current = db.tasks.getTaskLease(taskId);
expect(current?.runId).toBe(runA);
});
});
});