From 775598ef4c9bb37cbb2cf0abd09882156f5f844b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Wed, 22 Apr 2026 16:24:57 -0400 Subject: [PATCH] fix(db): make task row state authoritative --- eslint.config.mjs | 34 +- src/main/database-base.ts | 57 --- src/main/database/migrations/v55-v55.ts | 29 ++ src/main/database/runs.ts | 11 - src/main/database/schema.ts | 2 + src/main/database/task-checkout.ts | 10 - src/main/database/task-status-rules.ts | 16 +- src/main/database/tasks-crud.ts | 13 - src/main/database/tasks-phase-consistency.ts | 164 ------- src/main/database/tasks-phase.ts | 163 ------- src/main/database/tasks-status.ts | 34 +- src/main/database/tasks-update.ts | 6 - src/main/database/tasks.ts | 2 - tests/unit/atomic-checkout.test.ts | 19 +- tests/unit/database.test.ts | 15 + tests/unit/tasks-phase.test.ts | 424 ------------------- 16 files changed, 75 insertions(+), 924 deletions(-) create mode 100644 src/main/database/migrations/v55-v55.ts delete mode 100644 src/main/database/tasks-phase-consistency.ts delete mode 100644 src/main/database/tasks-phase.ts delete mode 100644 tests/unit/tasks-phase.test.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 43582b4d..5ee3236f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -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.", }, ], }, diff --git a/src/main/database-base.ts b/src/main/database-base.ts index db4452fd..1d24d60e 100644 --- a/src/main/database-base.ts +++ b/src/main/database-base.ts @@ -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 })); } diff --git a/src/main/database/migrations/v55-v55.ts b/src/main/database/migrations/v55-v55.ts new file mode 100644 index 00000000..c0e72a46 --- /dev/null +++ b/src/main/database/migrations/v55-v55.ts @@ -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"); + } + }, + }, + ]; +} diff --git a/src/main/database/runs.ts b/src/main/database/runs.ts index c1f1077d..086ce293 100644 --- a/src/main/database/runs.ts +++ b/src/main/database/runs.ts @@ -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() diff --git a/src/main/database/schema.ts b/src/main/database/schema.ts index 9e318d15..2a30db6b 100644 --- a/src/main/database/schema.ts +++ b/src/main/database/schema.ts @@ -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 diff --git a/src/main/database/task-checkout.ts b/src/main/database/task-checkout.ts index 489d2530..710e718b 100644 --- a/src/main/database/task-checkout.ts +++ b/src/main/database/task-checkout.ts @@ -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; } diff --git a/src/main/database/task-status-rules.ts b/src/main/database/task-status-rules.ts index c3c96254..1eb0a161 100644 --- a/src/main/database/task-status-rules.ts +++ b/src/main/database/task-status-rules.ts @@ -8,14 +8,6 @@ export const VALID_TASK_STATUSES = new Set([ "cancelled", ]); -const STATUSES_THAT_CLEAR_LEASE = new Set([ - "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 }; } diff --git a/src/main/database/tasks-crud.ts b/src/main/database/tasks-crud.ts index c8861b7f..74271a2c 100644 --- a/src/main/database/tasks-crud.ts +++ b/src/main/database/tasks-crud.ts @@ -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( diff --git a/src/main/database/tasks-phase-consistency.ts b/src/main/database/tasks-phase-consistency.ts deleted file mode 100644 index abed3b5d..00000000 --- a/src/main/database/tasks-phase-consistency.ts +++ /dev/null @@ -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 = { - 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 ?? ""} 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 }; -} diff --git a/src/main/database/tasks-phase.ts b/src/main/database/tasks-phase.ts deleted file mode 100644 index f383eb37..00000000 --- a/src/main/database/tasks-phase.ts +++ /dev/null @@ -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); - }, - }; -} diff --git a/src/main/database/tasks-status.ts b/src/main/database/tasks-status.ts index e9007eaf..832ae805 100644 --- a/src/main/database/tasks-status.ts +++ b/src/main/database/tasks-status.ts @@ -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); }, diff --git a/src/main/database/tasks-update.ts b/src/main/database/tasks-update.ts index 8dd7012d..cef32d39 100644 --- a/src/main/database/tasks-update.ts +++ b/src/main/database/tasks-update.ts @@ -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); } diff --git a/src/main/database/tasks.ts b/src/main/database/tasks.ts index 0afbd055..3a936066 100644 --- a/src/main/database/tasks.ts +++ b/src/main/database/tasks.ts @@ -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), }; } diff --git a/tests/unit/atomic-checkout.test.ts b/tests/unit/atomic-checkout.test.ts index 6882f3fa..7c5e3499 100644 --- a/tests/unit/atomic-checkout.test.ts +++ b/tests/unit/atomic-checkout.test.ts @@ -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", () => { diff --git a/tests/unit/database.test.ts b/tests/unit/database.test.ts index 2cc3dcf7..5228067c 100644 --- a/tests/unit/database.test.ts +++ b/tests/unit/database.test.ts @@ -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", diff --git a/tests/unit/tasks-phase.test.ts b/tests/unit/tasks-phase.test.ts deleted file mode 100644 index ed249e2c..00000000 --- a/tests/unit/tasks-phase.test.ts +++ /dev/null @@ -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); - }); - }); -});