Hotfix: separate workspace observation from lock claim — restore artifact projector for cognitive runs
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.
This commit is contained in:
1 parent
e15bf67c9f
commit
1f2f9770a6
6 files changed
+132
-4
No files matched your search
@@ -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")
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -28,6 +28,16 @@ export interface RunDispatchParams {
|
||||
connector: ReturnType<AppDatabase["connectorsCrud"]["getConnector"]>;
|
||||
workspace: ReturnType<AppDatabase["workspacesMod"]["getWorkspace"]>;
|
||||
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<AppDatabase["snap"]["listSnapshot"]>;
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user