fix(db): make filesystem lock intent explicit
This commit is contained in:
1 parent
0785c9ba47
commit
19f8494d09
15 files changed
+188
-125
No files matched your search
@@ -140,6 +140,7 @@ export function registerMeetingRoutes(server: AgentApiServer) {
|
||||
assigneeAgentId: item.assigneeAgentId ?? null,
|
||||
status: item.assigneeAgentId ? "todo" : "backlog",
|
||||
priority: "medium",
|
||||
requiresFilesystem: false,
|
||||
});
|
||||
if (item.assigneeAgentId) {
|
||||
onWakeAgent(item.assigneeAgentId, token.companyId, "assignment");
|
||||
|
||||
@@ -14,6 +14,7 @@ const taskUpdateSchema = z.object({
|
||||
description: z.string().max(4000).optional(),
|
||||
assigneeAgentId: z.string().uuid().nullable().optional(),
|
||||
priority: z.enum(["critical", "high", "medium", "low"]).optional(),
|
||||
requiresFilesystem: z.boolean().optional(),
|
||||
});
|
||||
|
||||
const checkoutSchema = z.object({
|
||||
@@ -184,6 +185,7 @@ export function registerTaskRoutes(server: AgentApiServer) {
|
||||
parentId: task.parentId,
|
||||
assigneeAgentId: newAssignee,
|
||||
workspaceId: task.workspaceId,
|
||||
requiresFilesystem: update.requiresFilesystem ?? task.requiresFilesystem,
|
||||
};
|
||||
db.tasks.saveTask(input as Parameters<typeof db.tasks.saveTask>[0]);
|
||||
|
||||
@@ -344,6 +346,7 @@ export function registerTaskRoutes(server: AgentApiServer) {
|
||||
priority: input.priority,
|
||||
status: input.status,
|
||||
workspaceId: input.workspaceId ?? null,
|
||||
requiresFilesystem: input.requiresFilesystem,
|
||||
});
|
||||
} catch (error) {
|
||||
server.json(res, 422, { error: error instanceof Error ? error.message : "Task could not be saved" });
|
||||
|
||||
@@ -24,6 +24,7 @@ export const createTaskConfigSchema = z.object({
|
||||
priority: taskPrioritySchema.optional(),
|
||||
assigneeAgentId: z.string().nullable().optional(),
|
||||
projectId: z.string().optional(),
|
||||
requiresFilesystem: z.boolean().default(false),
|
||||
});
|
||||
|
||||
export const notifyAgentConfigSchema = z.object({
|
||||
|
||||
@@ -100,6 +100,7 @@ export function executeAutomationAction(
|
||||
description: task.description,
|
||||
assigneeAgentId: ctx.agentId,
|
||||
workspaceId: task.workspaceId,
|
||||
requiresFilesystem: task.requiresFilesystem,
|
||||
priority: task.priority,
|
||||
status: "todo",
|
||||
});
|
||||
@@ -128,6 +129,7 @@ export function executeAutomationAction(
|
||||
assigneeAgentId,
|
||||
priority,
|
||||
status: assigneeAgentId ? "todo" : "backlog",
|
||||
requiresFilesystem: cfg.requiresFilesystem,
|
||||
});
|
||||
handleTaskCreated(taskId, ctx.companyId);
|
||||
logger.info(`[automation] Created task: ${title} (${taskId})`);
|
||||
|
||||
@@ -58,13 +58,9 @@ export function buildTaskCrudMethods(
|
||||
priority: string;
|
||||
status: string;
|
||||
/**
|
||||
* Per-task filesystem lock policy (v52+). When omitted the
|
||||
* column defaults to 0 at the SQL layer for inserts, and the
|
||||
* existing value is preserved on updates. Callers opt in with
|
||||
* `true` when they know the task will read/write the working
|
||||
* tree (CEO kickoff reads the repo, CTO code changes mutate
|
||||
* it, etc.). Leaving it undefined is the safe default for
|
||||
* purely cognitive tasks.
|
||||
* Per-task filesystem lock policy (v52+). Production callers
|
||||
* pass this explicitly. Direct DB fixtures may omit it: inserts
|
||||
* use the SQL default 0 and updates preserve the stored value.
|
||||
*/
|
||||
requiresFilesystem?: boolean;
|
||||
}): string {
|
||||
|
||||
@@ -9,9 +9,8 @@ import type { WorkspaceLockMode } from "./types";
|
||||
* `lockMode='none'` is the default. Previously every resolved
|
||||
* workspace was held exclusively, which folded four runnable
|
||||
* agents onto a single company workspace lock and serialised the
|
||||
* entire C-suite. The fix is not to pick a different fallback — it
|
||||
* is to stop inferring filesystem use from the shape of the graph
|
||||
* and ask the task row itself.
|
||||
* entire C-suite. The fix is to ask the task row for filesystem
|
||||
* intent.
|
||||
*
|
||||
* `inferLockMode` returns the pair of decisions the dispatcher
|
||||
* needs (mode + which workspace, if any). `resolveWorkspaceWithLockMode`
|
||||
@@ -20,7 +19,7 @@ import type { WorkspaceLockMode } from "./types";
|
||||
*
|
||||
* Silent fallbacks removed in this rewrite:
|
||||
*
|
||||
* • `company_fallback` — previously the fourth fallback step
|
||||
* - `company_fallback`: previously the fourth fallback step
|
||||
* returned *any* workspace row in the company as the lock target.
|
||||
* That was silent corruption's favourite mechanism: four agents
|
||||
* with no workspace binding would all land on the same row and
|
||||
@@ -29,17 +28,9 @@ import type { WorkspaceLockMode } from "./types";
|
||||
* time so the caller decides (skip heartbeat, surface error,
|
||||
* whatever) instead of the resolver guessing.
|
||||
*
|
||||
* • No `lockMode='exclusive'` ever returns a null workspace. The
|
||||
* dispatcher assumed this invariant (it claims a lock on
|
||||
* `workspace.id` when mode is exclusive). The types now enforce
|
||||
* it: exclusive always comes with a resolved workspace, none
|
||||
* always comes with `workspace=null`.
|
||||
* - `lockMode='exclusive'` always returns a resolved workspace.
|
||||
* `lockMode='none'` always returns `workspace=null`.
|
||||
*
|
||||
* Coding-role inference (rule 2) is a safety net for the one-
|
||||
* migration transition window where legacy rows may not yet have a
|
||||
* meaningful `requires_filesystem` column value set by the creator.
|
||||
* Once `saveTask` is taught to set the column explicitly at insert
|
||||
* time (follow-up commit), rule 2 becomes a no-op.
|
||||
*/
|
||||
|
||||
export type WorkspaceResolutionSource =
|
||||
@@ -51,7 +42,7 @@ export type WorkspaceResolutionSource =
|
||||
export interface WorkspaceResolution {
|
||||
/**
|
||||
* The workspace whose lock the dispatcher will claim. Non-null iff
|
||||
* `lockMode === 'exclusive'` — the type system enforces the pair.
|
||||
* `lockMode === 'exclusive'`; the type system enforces the pair.
|
||||
*/
|
||||
workspace: WorkspaceRecord | null;
|
||||
lockMode: WorkspaceLockMode;
|
||||
@@ -60,7 +51,7 @@ export interface WorkspaceResolution {
|
||||
* Where the artifact projector should *observe* for files this run
|
||||
* may produce, independent of the lock decision. Set whenever the
|
||||
* company has at least one workspace row, even when `lockMode='none'`
|
||||
* — cognitive agents still write deliverables to disk via `bash`,
|
||||
* because cognitive agents still write deliverables to disk via `bash`,
|
||||
* and the projector needs an observation root to scan against the
|
||||
* mtime baseline. Null only when the company has no workspace at
|
||||
* all (genuinely nowhere to look).
|
||||
@@ -68,7 +59,7 @@ export interface WorkspaceResolution {
|
||||
* For `lockMode='exclusive'` this is always the same path as
|
||||
* `workspace.localPath`. For `lockMode='none'` this is the
|
||||
* company's primary/oldest workspace localPath, picked by
|
||||
* `getCompanyObservationWorkspace` — *no lock claim*.
|
||||
* `getCompanyObservationWorkspace`.
|
||||
*
|
||||
* Decoupling observation from locking is what fixes the regression
|
||||
* where cognitive agents wrote 19 files but the DocumentCenter
|
||||
@@ -86,23 +77,12 @@ export class WorkspaceConfigurationError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(post-v52 stabilisation, target 2026-05-20): delete this safety net
|
||||
// once every task-creation path sets `requires_filesystem` explicitly.
|
||||
// Rationale: during v52 rollout some legacy creators still emit tasks with
|
||||
// a null `requires_filesystem`, and a CTO-typed task without the flag
|
||||
// would otherwise fall through to lockMode=none and happily write files
|
||||
// under no lock. Once seed data + every `db.tasks.saveTask` callsite is
|
||||
// audited (tracked in docs/architecture/workspace-concurrency.md
|
||||
// "Sunset coding-role inference"), this regex and the rule-2 role branch
|
||||
// below both get removed and the column becomes the single source of
|
||||
// truth. Do not extend this regex — add columns instead.
|
||||
const CODING_ROLE_PATTERN = /cto|engineer|developer|devops|sre/i;
|
||||
|
||||
/**
|
||||
* Accept the widened shapes call sites actually pass. `projectId`
|
||||
* lives as `string | null | undefined` in several historical call
|
||||
* sites (legacy inline types), and `role`/`department` are optional
|
||||
* because taskless heartbeats supply only the runtime-needed subset.
|
||||
* sites (legacy inline types). `role` and `department` remain in the
|
||||
* input shape for existing call sites; lock inference reads only
|
||||
* workspaceId from the agent.
|
||||
*/
|
||||
type TaskInput = {
|
||||
workspaceId: TaskRecord["workspaceId"];
|
||||
@@ -123,17 +103,16 @@ type AgentInput = Pick<AgentRecord, "workspaceId"> & {
|
||||
*
|
||||
* Decision table:
|
||||
*
|
||||
* 1. `task.workspaceId` set → exclusive on that workspace
|
||||
* 1. `task.workspaceId` set -> exclusive on that workspace
|
||||
* (explicit opt-in by the task author, e.g. PR branch tasks).
|
||||
*
|
||||
* 2. Task signals filesystem need (`requiresFilesystem === true`
|
||||
* OR agent role matches coding pattern OR department =
|
||||
* engineering) → exclusive on agent.workspaceId / project
|
||||
* primary. Throws if neither exists.
|
||||
* 2. Task signals filesystem need (`requiresFilesystem === true`)
|
||||
* -> exclusive on agent.workspaceId / project primary. Throws if
|
||||
* neither exists.
|
||||
*
|
||||
* 3. Otherwise → none / null. Pure cognitive work — strategy,
|
||||
* 3. Otherwise -> none / null. Pure cognitive work: strategy,
|
||||
* planning, document drafting, financial analysis, PM decisions
|
||||
* — runs with no workspace and no lock.
|
||||
* runs with no workspace and no lock.
|
||||
*/
|
||||
function inferLockMode(
|
||||
db: AppDatabase,
|
||||
@@ -142,24 +121,17 @@ function inferLockMode(
|
||||
companyId: string,
|
||||
): { lockMode: WorkspaceLockMode; workspaceId: string | null; source: WorkspaceResolutionSource } {
|
||||
// Rule 1: task-level explicit workspace binding is the strongest
|
||||
// signal available — the task row literally names the workspace.
|
||||
// signal available: the task row literally names the workspace.
|
||||
if (task?.workspaceId) {
|
||||
return { lockMode: "exclusive", workspaceId: task.workspaceId, source: "task" };
|
||||
}
|
||||
|
||||
const taskSignalsFilesystem = task?.requiresFilesystem === true;
|
||||
const agentSignalsFilesystem =
|
||||
(typeof agent.role === "string" && CODING_ROLE_PATTERN.test(agent.role)) ||
|
||||
agent.department === "engineering";
|
||||
|
||||
const needsFilesystem = taskSignalsFilesystem || agentSignalsFilesystem;
|
||||
if (!needsFilesystem) {
|
||||
if (task?.requiresFilesystem !== true) {
|
||||
return { lockMode: "none", workspaceId: null, source: "none" };
|
||||
}
|
||||
|
||||
// Rule 2: filesystem-touching task → agent workspace first, then
|
||||
// project primary. We do not fall back to `any company workspace`
|
||||
// any more — that path was the cause of the single-lock pile-up.
|
||||
// Rule 2: filesystem-touching task -> agent workspace first, then
|
||||
// project primary.
|
||||
if (agent.workspaceId) {
|
||||
return { lockMode: "exclusive", workspaceId: agent.workspaceId, source: "agent" };
|
||||
}
|
||||
@@ -185,8 +157,8 @@ function inferLockMode(
|
||||
* Resolve the workspace + lock mode for a run context.
|
||||
*
|
||||
* Contract:
|
||||
* • lockMode 'exclusive' ⇒ workspace is a live WorkspaceRecord.
|
||||
* • lockMode 'none' ⇒ workspace is null.
|
||||
* - lockMode 'exclusive' => workspace is a live WorkspaceRecord.
|
||||
* - lockMode 'none' => workspace is null.
|
||||
*
|
||||
* Violations throw `WorkspaceConfigurationError` at resolve time so
|
||||
* the caller chooses how to recover (skip heartbeat, cancel run,
|
||||
@@ -218,7 +190,7 @@ export function resolveWorkspaceWithLockMode(
|
||||
|
||||
if (!decision.workspaceId) {
|
||||
throw new WorkspaceConfigurationError(
|
||||
`inferLockMode returned exclusive with no workspaceId (companyId=${companyId}) — lock mode/workspace invariant broken.`,
|
||||
`inferLockMode returned exclusive with no workspaceId (companyId=${companyId}); lock mode/workspace invariant broken.`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -238,7 +210,7 @@ export function resolveWorkspaceWithLockMode(
|
||||
workspace,
|
||||
lockMode: "exclusive",
|
||||
source: decision.source,
|
||||
// Exclusive runs observe the same path they lock — single source.
|
||||
// Exclusive runs observe the same path they lock.
|
||||
observationPath: workspace.localPath,
|
||||
};
|
||||
}
|
||||
@@ -18,7 +18,7 @@ export function TaskCreator({ companyId, goals, projects, agents, workspaces, on
|
||||
|
||||
async function handleCreateTask() {
|
||||
setSubmitError(null); setSubmitting(true)
|
||||
try { unwrap(await window.agentCompany.saveTask({ companyId, projectId: projectId || null, goalId: goalId || null, title, description, assigneeAgentId: assigneeAgentId || null, workspaceId: workspaceId || null, priority: priority as TaskRecord["priority"], status: "todo" })); setTitle(""); setDescription(""); await onSaved() } catch (e) { setSubmitError(describeActionError(e)) } finally { setSubmitting(false) }
|
||||
try { unwrap(await window.agentCompany.saveTask({ companyId, projectId: projectId || null, goalId: goalId || null, title, description, assigneeAgentId: assigneeAgentId || null, workspaceId: workspaceId || null, requiresFilesystem: Boolean(workspaceId), priority: priority as TaskRecord["priority"], status: "todo" })); setTitle(""); setDescription(""); await onSaved() } catch (e) { setSubmitError(describeActionError(e)) } finally { setSubmitting(false) }
|
||||
}
|
||||
|
||||
const [showAdvanced, setShowAdvanced] = useState(false)
|
||||
|
||||
@@ -90,7 +90,8 @@ export function TaskInspector({
|
||||
unwrap(await window.agentCompany.saveTask({
|
||||
id: task.id, companyId: task.companyId, title: task.title, description: task.description,
|
||||
priority: task.priority, status, assigneeAgentId: task.assigneeAgentId,
|
||||
workspaceId: task.workspaceId, projectId: task.projectId, goalId: task.goalId,
|
||||
workspaceId: task.workspaceId, requiresFilesystem: task.requiresFilesystem,
|
||||
projectId: task.projectId, goalId: task.goalId,
|
||||
}))
|
||||
await onRefresh()
|
||||
setActionNotice(`Task moved to ${status.replaceAll("_", " ")}.`)
|
||||
|
||||
@@ -43,6 +43,7 @@ export function TaskInspectorEditForm({
|
||||
status,
|
||||
assigneeAgentId: assignee || null,
|
||||
workspaceId: workspace || null,
|
||||
requiresFilesystem: task.requiresFilesystem,
|
||||
projectId: project || null,
|
||||
goalId: goal || null,
|
||||
}))
|
||||
|
||||
@@ -32,6 +32,7 @@ export const taskInputSchema = z.object({
|
||||
description: z.string().default(""),
|
||||
assigneeAgentId: optionalId,
|
||||
workspaceId: optionalId,
|
||||
requiresFilesystem: z.boolean().default(false),
|
||||
priority: z.enum(["critical", "high", "medium", "low"]).default("medium"),
|
||||
status: z.enum(["backlog", "todo", "in_progress", "in_review", "done", "blocked", "cancelled"]).default("todo"),
|
||||
});
|
||||
|
||||
@@ -157,12 +157,10 @@ export interface TaskRecord {
|
||||
taskType: string | null;
|
||||
requiresUserReview: boolean;
|
||||
/**
|
||||
* Per-task filesystem lock policy (v52+). When `true` the
|
||||
* orchestrator resolves a workspace and holds it exclusively for
|
||||
* the duration of the run; when `false` the run executes with
|
||||
* `lockMode='none'` and agents run in parallel on shared read-only
|
||||
* context. Inferred from agent role at task creation and toggled
|
||||
* explicitly by callers that know their task mutates the tree.
|
||||
* Per-task filesystem lock policy (v52+). `true` makes the
|
||||
* orchestrator resolve a workspace and hold it exclusively for
|
||||
* the duration of the run. `false` lets a task with no explicit
|
||||
* workspace binding execute with `lockMode='none'`.
|
||||
*/
|
||||
requiresFilesystem: boolean;
|
||||
startedAt: string | null;
|
||||
@@ -395,4 +393,3 @@ export interface ModelCapability {
|
||||
costPerMtokOut: number;
|
||||
updatedAt: string;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user