From 9fba8dd6c3e7fc6420070b6ed9bd1344b10a08d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 21 Apr 2026 14:52:38 -0400 Subject: [PATCH] fix(projector): stop writing null workspace_id on work_products MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sites in upsertFilesystemArtifact leaked null workspace_ids into the work_products table and produced the orphan rows the read path now has to heal: 1. Create path unconditionally forwarded ctx.workspaceId, which was null whenever a run was created without a workspace binding (historically cognitive runs, or runs that lost their link through the observationPath regression fixed in 1f2f977). 2. Update path passed {workspaceId: ctx.workspaceId} as part of the patch. updateWorkProduct treats any key present in patch as "write this value", so a run touching a previously-well-linked row with ctx.workspaceId=null would clobber the existing workspace_id back to null — actively destroying data. Resolve both at the projector layer: - Add workspacesMod.findWorkspaceIdByLocalPath(companyId, localPath) as a reverse-lookup. workspacePath is the argument the projector is already walking; it came from some workspace row's local_path, so finding that row by path is deterministic. - Compute resolvedWorkspaceId = ctx.workspaceId ?? reverseLookup(...) once, and use it for both the identity lookup (findWorkProductByExternalId) and the subsequent write. - On the update path, only include workspaceId in the patch when resolvedWorkspaceId is non-null. Null now means "no info — leave the existing column alone" instead of "clobber to null". Together with the read-path heal in cfb8ba5 this closes the loop: new files never enter the table with a null workspace_id if we can possibly resolve it, old files keep their links, and truly unlinkable rows still fall back to the observation workspace on open. Typecheck and build:verify clean. --- src/main/agent-runtime/artifact-projector.ts | 22 +++++++++++++++----- src/main/database/workspaces.ts | 18 ++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/main/agent-runtime/artifact-projector.ts b/src/main/agent-runtime/artifact-projector.ts index 232879f7..32f8f1d7 100644 --- a/src/main/agent-runtime/artifact-projector.ts +++ b/src/main/agent-runtime/artifact-projector.ts @@ -608,29 +608,41 @@ function upsertFilesystemArtifact( const title = lastSlash >= 0 ? relativePath.slice(lastSlash + 1) : relativePath; const provider = "filesystem"; + // Reverse-resolve workspaceId from workspacePath when ctx didn't + // carry it. The run may have been created before workspace_id was + // tracked end-to-end, but the projector only ever walks a workspace + // whose local_path is known — that path uniquely identifies the + // workspace row, so it is safe to persist the foreign key instead + // of writing another orphan. + const resolvedWorkspaceId = + ctx.workspaceId ?? db.workspacesMod.findWorkspaceIdByLocalPath(ctx.companyId, workspacePath); // Identity is scoped to (company, workspace, provider, externalId) so two // workspaces with the same relative path do not collapse onto one row. // Every touch refreshes `last_touched_by_run_id` so the deliverable // signal, the DocumentCenter preview, and the run-provenance readers // all see the latest run that wrote the file. - const existing = db.documentsMod.findWorkProductByExternalId(ctx.companyId, provider, relativePath, ctx.workspaceId); + const existing = db.documentsMod.findWorkProductByExternalId(ctx.companyId, provider, relativePath, resolvedWorkspaceId); if (existing) { // `status: "active"` flips a previously-archived row (earlier run // retired the path, this run recreated it) back into the visible // DocumentCenter set. `lastTouchedByRunId` keeps the provenance // timeline pointed at the run that just wrote the bytes. - db.documentsMod.updateWorkProduct(existing.id, ctx.companyId, { + // Only include workspaceId in the patch when we actually have one — + // otherwise the UPDATE would clobber a previously-resolved + // workspace_id back to null (the original source of orphan rows). + const patch: Parameters[2] = { title, status: "active", lastTouchedByRunId: ctx.runId, - workspaceId: ctx.workspaceId, - }); + }; + if (resolvedWorkspaceId) patch.workspaceId = resolvedWorkspaceId; + db.documentsMod.updateWorkProduct(existing.id, ctx.companyId, patch); return { kind: "updated", workProductId: existing.id, relativePath }; } const workProductId = db.documentsMod.createWorkProduct({ companyId: ctx.companyId, taskId: ctx.taskId, - workspaceId: ctx.workspaceId, + workspaceId: resolvedWorkspaceId, type: "file", provider, externalId: relativePath, diff --git a/src/main/database/workspaces.ts b/src/main/database/workspaces.ts index 72c60cce..d106ead9 100644 --- a/src/main/database/workspaces.ts +++ b/src/main/database/workspaces.ts @@ -51,6 +51,24 @@ export function buildWorkspaceMethods( return row ? { ...row, isPrimary: toBoolean(row.isPrimary) } : null; }, + /** + * Reverse-lookup a workspace's id by its local_path within a + * company. Used by the artifact projector to heal a run context + * whose workspaceId was never bound but whose workspacePath is + * still known — we look up the row that owns that path so the + * resulting work_product carries a real foreign key instead of + * another null that downstream readers would have to heal again. + * Returns null when the path does not match any workspace (the + * caller proceeds without a workspace_id, which is then handled + * defensively by updateWorkProduct). + */ + findWorkspaceIdByLocalPath(companyId: string, localPath: string): string | null { + const row = getDb() + .prepare("select id from workspaces where company_id = ? and local_path = ? limit 1") + .get(companyId, localPath) as { id: string } | undefined; + return row?.id ?? null; + }, + getProjectPrimaryWorkspace(projectId: string | null | undefined, companyId: string): WorkspaceRecord | null { if (!projectId) return null; const row = getDb()