From e15bf67c9fe84eb88e4401b3e19632996a52b02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 20 Apr 2026 15:40:46 -0400 Subject: [PATCH] Workspace concurrency review iteration: Jobs's 3 taste asks Closes the three specific_asks from Jobs's T1 review of the workspace architecture fix (7829acc..15e3863): 1. docs/architecture/workspace-concurrency.md: decision table upgraded from a code block to a real Markdown table so it reads consistently with the failure-modes table lower in the file. 2. docs/architecture/workspace-concurrency.md + resolve-workspace.ts: added "Sunset coding-role inference" section and a TODO marker on CODING_ROLE_PATTERN tying the regex to the v52 transition window (target 2026-05-20). Both documents now name the end-state explicitly so a future engineer knows why the regex exists and when it dies. 3. resolve-workspace.ts: enriched the WorkspaceConfigurationError message to include taskId / agentId / projectId alongside the existing companyId, so a misconfigured setup points operators at the offending row instead of just the company. No behaviour change, no fallback, no mock. typecheck green, 540 unit tests green, 10 eval tests green. --- docs/architecture/workspace-concurrency.md | 23 ++++++++++++++------- src/main/orchestration/resolve-workspace.ts | 12 ++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/docs/architecture/workspace-concurrency.md b/docs/architecture/workspace-concurrency.md index ea166a4f..3c0db9d5 100644 --- a/docs/architecture/workspace-concurrency.md +++ b/docs/architecture/workspace-concurrency.md @@ -64,15 +64,24 @@ takes an exclusive lock on its agent workspace. Once every task creation path sets `requires_filesystem` explicitly the role inference becomes a no-op. +### Sunset coding-role inference + +The coding-role regex in `resolve-workspace.ts` exists only for the +v52 transition window. Before the next major, audit every +`db.tasks.saveTask` caller, every migration seed, and any workflow +that writes the `tasks` row; each must set `requires_filesystem` +explicitly based on the task's real filesystem intent. When the audit +is complete, delete the regex and the `agentSignalsFilesystem` branch +from `inferLockMode` and remove this section. Target window: post +v52 stabilisation, 2026-05-20. + ## Decision table -``` -task.workspaceId set → exclusive on task.workspaceId -task.requiresFilesystem=1 → exclusive on agent.workspaceId - OR agent role is coding or project primary; throws if - neither exists -everything else → none / workspace=null -``` +| Signal on the (task, agent) pair | Lock mode | Workspace used | +| --- | --- | --- | +| `task.workspaceId` set (explicit opt-in, e.g. PR-review task) | `exclusive` | The task's workspace | +| `task.requiresFilesystem = 1` **OR** agent role matches coding pattern **OR** `agent.department = "engineering"` | `exclusive` | `agent.workspaceId` → project primary → throws `WorkspaceConfigurationError` | +| Everything else (strategy, ops, finance, PM, drafting) | `none` | `null` — no lock claimed | Explicit `task.workspaceId` always wins. This is the escape hatch for "run this PR-review task on *that specific* branch workspace", and diff --git a/src/main/orchestration/resolve-workspace.ts b/src/main/orchestration/resolve-workspace.ts index 4283bd8b..77edb421 100644 --- a/src/main/orchestration/resolve-workspace.ts +++ b/src/main/orchestration/resolve-workspace.ts @@ -62,6 +62,16 @@ 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; /** @@ -143,7 +153,7 @@ function inferLockMode( } throw new WorkspaceConfigurationError( - `Task requires a filesystem-backed run but no workspace is bound to the task, agent, or project (companyId=${companyId}).`, + `Task requires a filesystem-backed run but no workspace is bound to the task, agent, or project (companyId=${companyId}, taskId=${task?.workspaceId === undefined ? "" : (task as { id?: string }).id ?? ""}, agentId=${(agent as { id?: string }).id ?? ""}, projectId=${task?.projectId ?? ""}).`, ); }