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.
This commit is contained in:
ZacharyZhang-NY committed 2026-04-20 15:40:46 -04:00
1 parent 15e3863cba
commit e15bf67c9f
2 files changed
+27 -8

No files matched your search

+16 -7
View File
@@ -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
+11 -1
View File
@@ -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 ? "<null-task>" : (task as { id?: string }).id ?? "<unknown>"}, agentId=${(agent as { id?: string }).id ?? "<unknown>"}, projectId=${task?.projectId ?? "<null>"}).`,
);
}