From 1f2f9770a6886e97b5ae19579f9cb5c409531118 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 16:22:40 -0400 Subject: [PATCH] =?UTF-8?q?Hotfix:=20separate=20workspace=20observation=20?= =?UTF-8?q?from=20lock=20claim=20=E2=80=94=20restore=20artifact=20projecto?= =?UTF-8?q?r=20for=20cognitive=20runs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production regression of T1 (7829acc..15e3863). User reports "明明生成 了很多文件,但只有一个" — the filesystem shows 19 markdown deliverables in ~/Documents/Github/test/{business-plan,operations,ceo-coordination}/ but the DocumentCenter surfaces only one row. Root cause is structural, predicted in spirit by Karpathy's Round 1 take ("把'认知'和'执行'两件事一起锁是数据结构错"). The T1 rewrite collapsed two concerns into one nullable field on the run context: workspacePath = workspace?.localPath ?? null ^^^^^^^^^ ^^^^ lock target observation root For lockMode='none' (every cognitive C-suite agent post-T1) the resolver returns workspace=null, so workspacePath was null, so artifact-projector.ts:242 short-circuited every projection with `{ kind: "skipped", reason: "no_workspace" }`. The agents kept writing files via bash (cwd falls back to the company default workspace at worker/runtime/index.ts:142), but nothing observed them — the DocumentCenter only saw the one file that was projected back when the agent still held an exclusive workspace. Fix: split the two concerns. WorkspaceResolution gains a non-lock `observationPath` field; the resolver populates it from the company primary workspace (new `getCompanyObservationWorkspace` query — primary first, oldest second, null only when company has zero workspace rows) for lockMode='none' runs, and from `workspace.localPath` itself for lockMode='exclusive' runs. RunDispatchParams threads the field through; turn-context's `workspacePath` reads it first. artifact-projector.ts is unchanged — it just sees a non-null path again and resumes baselining, diffing, and upserting work_products rows the way it did pre-T1. Three new unit tests pin the regression: cognitive role with workspace ⇒ observationPath set; cognitive role with no workspace ⇒ observationPath null; exclusive ⇒ observationPath equals workspace.localPath. typecheck green, 543/543 unit tests, 10/10 eval. No fallback, no mock, no silent degradation. --- src/main/database/workspaces.ts | 22 ++++++++++ src/main/orchestration/heartbeat.ts | 1 + src/main/orchestration/resolve-workspace.ts | 46 ++++++++++++++++++++- src/main/orchestration/task-runner.ts | 2 + src/main/orchestration/turn-context.ts | 20 ++++++++- tests/unit/resolve-workspace.test.ts | 45 ++++++++++++++++++++ 6 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/main/database/workspaces.ts b/src/main/database/workspaces.ts index 57f59687..72c60cce 100644 --- a/src/main/database/workspaces.ts +++ b/src/main/database/workspaces.ts @@ -61,6 +61,28 @@ export function buildWorkspaceMethods( return row ? { ...row, isPrimary: toBoolean(row.isPrimary) } : null; }, + /** + * Pick a single workspace per company for *observation only* (no + * lock claim). Used by the artifact projector to know where on + * disk a `lockMode='none'` cognitive run might write files. The + * tie-break order is `is_primary` first, then earliest `created_at` + * — i.e. the workspace the operator marked as canonical wins, and + * if no one is marked, the oldest row wins (which historically is + * the company's default scratch directory). + * + * Returns `null` when the company has no workspace at all; the + * caller treats that as "agent has no observation surface", which + * is correct — there is genuinely nowhere to look for files. + */ + getCompanyObservationWorkspace(companyId: string): WorkspaceRecord | null { + const row = getDb() + .prepare( + "select id, company_id as companyId, name, local_path as localPath, repo_url as repoUrl, repo_ref as repoRef, project_id as projectId, state, is_primary as isPrimary, created_at as createdAt, updated_at as updatedAt from workspaces where company_id = ? order by is_primary desc, created_at asc limit 1", + ) + .get(companyId) as WorkspaceRecord | undefined; + return row ? { ...row, isPrimary: toBoolean(row.isPrimary) } : null; + }, + listWorkspaceLocalPaths(companyId: string): string[] { const rows = getDb() .prepare("select local_path as localPath from workspaces where company_id = ? and local_path is not null") diff --git a/src/main/orchestration/heartbeat.ts b/src/main/orchestration/heartbeat.ts index 7d8017ba..dbcb4133 100644 --- a/src/main/orchestration/heartbeat.ts +++ b/src/main/orchestration/heartbeat.ts @@ -255,6 +255,7 @@ export function createHeartbeat( connector, workspace, workspaceMode: workspaceResolution.lockMode, + observationPath: workspaceResolution.observationPath, companyName: companyBasic?.name ?? "Unknown", companyDescription: companyBasic?.description ?? "", snapshot: db.snap.listSnapshot(), diff --git a/src/main/orchestration/resolve-workspace.ts b/src/main/orchestration/resolve-workspace.ts index 77edb421..5d608b57 100644 --- a/src/main/orchestration/resolve-workspace.ts +++ b/src/main/orchestration/resolve-workspace.ts @@ -49,9 +49,33 @@ export type WorkspaceResolutionSource = | "none"; export interface WorkspaceResolution { + /** + * The workspace whose lock the dispatcher will claim. Non-null iff + * `lockMode === 'exclusive'` — the type system enforces the pair. + */ workspace: WorkspaceRecord | null; lockMode: WorkspaceLockMode; source: WorkspaceResolutionSource; + /** + * 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`, + * 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). + * + * 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*. + * + * Decoupling observation from locking is what fixes the regression + * where cognitive agents wrote 19 files but the DocumentCenter + * surfaced only 1: previously `workspacePath` shared the lock + * decision, so any non-exclusive run lost its projector. + */ + observationPath: string | null; } export class WorkspaceConfigurationError extends Error { @@ -177,7 +201,19 @@ export function resolveWorkspaceWithLockMode( const decision = inferLockMode(db, task, agent, companyId); if (decision.lockMode === "none") { - return { workspace: null, lockMode: "none", source: "none" }; + // No lock claim, but the cognitive run can still touch the + // filesystem via `bash`. Pick a stable observation root so the + // artifact projector can baseline + diff the run's footprint. + // `getCompanyObservationWorkspace` returns the primary or oldest + // workspace; null only when the company has no workspace row at + // all, in which case there is genuinely nothing to observe. + const observation = db.workspacesMod.getCompanyObservationWorkspace(companyId); + return { + workspace: null, + lockMode: "none", + source: "none", + observationPath: observation?.localPath ?? null, + }; } if (!decision.workspaceId) { @@ -198,5 +234,11 @@ export function resolveWorkspaceWithLockMode( ); } - return { workspace, lockMode: "exclusive", source: decision.source }; + return { + workspace, + lockMode: "exclusive", + source: decision.source, + // Exclusive runs observe the same path they lock — single source. + observationPath: workspace.localPath, + }; } diff --git a/src/main/orchestration/task-runner.ts b/src/main/orchestration/task-runner.ts index cc733ee9..cd10056a 100644 --- a/src/main/orchestration/task-runner.ts +++ b/src/main/orchestration/task-runner.ts @@ -130,6 +130,7 @@ export function createTaskRunner( connector, workspace, workspaceMode: workspaceResolution.lockMode, + observationPath: workspaceResolution.observationPath, companyName: companyBasic?.name ?? "Unknown", companyDescription: companyBasic?.description ?? "", snapshot: db.snap.listSnapshot(), @@ -230,6 +231,7 @@ export function createTaskRunner( connector, workspace: workspaceResolution.workspace, workspaceMode: workspaceResolution.lockMode, + observationPath: workspaceResolution.observationPath, companyName: company.name, companyDescription: company.description, snapshot, diff --git a/src/main/orchestration/turn-context.ts b/src/main/orchestration/turn-context.ts index a62c1f3a..009c2778 100644 --- a/src/main/orchestration/turn-context.ts +++ b/src/main/orchestration/turn-context.ts @@ -28,6 +28,16 @@ export interface RunDispatchParams { connector: ReturnType; workspace: ReturnType; workspaceMode: WorkspaceLockMode; + /** + * Filesystem root the artifact projector observes for files this run + * may produce, independent of `workspaceMode`. Set whenever the + * resolver found a workspace row in the company (including for + * `lockMode='none'` cognitive runs that do not claim the lock but + * still write deliverables via `bash`). Optional so legacy callers + * that bypass the resolver keep typechecking; the new resolver + * always sets it. + */ + observationPath?: string | null; companyName: string; companyDescription: string; snapshot: ReturnType; @@ -286,7 +296,7 @@ function buildConnectorCatalog( * This is the core composition step that was previously inlined in dispatchRunToWorker. */ export function buildRunPayload(params: RunDispatchParams, deps: TurnContextDeps): EnqueueRunPayload { - const { runId, companyId, agent, task, connector, workspace, workspaceMode, companyName, companyDescription, snapshot, trigger } = params; + const { runId, companyId, agent, task, connector, workspace, workspaceMode, observationPath, companyName, companyDescription, snapshot, trigger } = params; const { db } = deps; const { env, apiUrl, apiKey } = assembleApiEnv(db, connector, companyId, agent.id, runId, trigger, deps); @@ -375,7 +385,13 @@ export function buildRunPayload(params: RunDispatchParams, deps: TurnContextDeps agentName: agent.name, connectorId: connector.id, workspaceId: workspaceMode === "exclusive" ? (workspace?.id ?? null) : null, - workspacePath: workspace?.localPath ?? null, + // observationPath, not lock target. Cognitive runs (lockMode='none') + // still write deliverables via bash; the artifact projector needs an + // observation root regardless of whether we hold the lock. Falls back + // to workspace?.localPath for old call sites that did not yet thread + // the observationPath through (the resolver always sets it now; + // callers that bypass the resolver remain on the legacy field). + workspacePath: observationPath ?? workspace?.localPath ?? null, workspaceMode, model: modelSelection.effectiveModel, thinkingEffort, diff --git a/tests/unit/resolve-workspace.test.ts b/tests/unit/resolve-workspace.test.ts index 2a920a87..3e88519c 100644 --- a/tests/unit/resolve-workspace.test.ts +++ b/tests/unit/resolve-workspace.test.ts @@ -217,4 +217,49 @@ describe("resolveWorkspaceWithLockMode", () => { expect(m.workspace).toBeNull(); } }); + + it("lockMode=none still returns observationPath when company has a workspace (artifact projector regression)", () => { + const { companyId } = seedCompany(db); + const result = resolveWorkspaceWithLockMode( + db, + null, + { workspaceId: null, role: "CMO", department: "marketing" }, + companyId, + ); + expect(result.lockMode).toBe("none"); + expect(result.workspace).toBeNull(); + // Observation path is the company's primary workspace localPath + // even though no lock is claimed — the artifact projector needs + // somewhere to baseline + diff so files written by cognitive + // runs surface in the DocumentCenter. + expect(result.observationPath).toBe("/tmp/fixture"); + }); + + it("lockMode=none observationPath is null when company has no workspace at all", () => { + const companyId = db.company.saveCompany({ + name: "No-workspace Co", + description: "", + status: "active", + }); + const result = resolveWorkspaceWithLockMode( + db, + null, + { workspaceId: null, role: "CFO", department: "finance" }, + companyId, + ); + expect(result.lockMode).toBe("none"); + expect(result.observationPath).toBeNull(); + }); + + it("lockMode=exclusive observationPath equals workspace.localPath (single source)", () => { + const { companyId, workspaceId } = seedCompany(db); + const result = resolveWorkspaceWithLockMode( + db, + null, + { workspaceId, role: "CTO", department: "engineering" }, + companyId, + ); + expect(result.lockMode).toBe("exclusive"); + expect(result.observationPath).toBe(result.workspace?.localPath); + }); });