From 390f76fa7608c2a96835ca8f775c9fff121c91a3 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:45:59 -0400 Subject: [PATCH] fix(docs): split work-product open errors into actionable reasons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous handler folded every null target into one error code ("WORK_PRODUCT_NOT_FOUND: Work product not found or its workspace is unresolved"). When a user couldn't open a file we had no way to tell whether the row was archived, the provider/external_id was empty, the workspace had no local_path, or the external_id escaped the root. Widen resolveWorkProductTarget's return from (target | null) to (target | error) where error carries one of five reasons: not_found row missing for id+company archived status == "archived" tombstone no_target provider != filesystem and no url workspace_unresolved all three workspace_id / task_id / run_id lookups returned no local_path path_escape external_id is absolute or steps outside root Each reason includes the concrete detail (ids checked, paths tried) so the toast on the renderer side tells the user exactly which slot is null without having to open the DB. The IPC handler maps each reason to its own failure code and passes the detail through, so WorkProductRow's toast now reads e.g. "workspace_id=null task_id=null run_id=null — none led to a workspace with a local_path" instead of a generic unresolved. Typecheck and build:verify clean. --- src/main/database/documents.ts | 32 ++++++++++++++++++++---------- src/main/ipc-core-read-handlers.ts | 11 +++++++++- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/main/database/documents.ts b/src/main/database/documents.ts index ac4e13d4..b7fa2e2b 100644 --- a/src/main/database/documents.ts +++ b/src/main/database/documents.ts @@ -351,24 +351,26 @@ export function buildDocumentMethods( * artifact) or a concrete absolute path that downstream * `canOpenPath` validates. */ - resolveWorkProductTarget(workProductId: string, companyId: string): { kind: "url" | "path"; value: string } | null { + resolveWorkProductTarget( + workProductId: string, + companyId: string, + ): + | { kind: "url" | "path"; value: string } + | { kind: "error"; reason: "not_found" | "archived" | "no_target" | "workspace_unresolved" | "path_escape"; detail: string } { const row = getDb().prepare( "SELECT provider, external_id, url, status, task_id, created_by_run_id, last_touched_by_run_id, workspace_id FROM work_products WHERE id = ? AND company_id = ?", ).get(workProductId, companyId) as | { provider: string; external_id: string | null; url: string | null; status: string; task_id: string | null; created_by_run_id: string | null; last_touched_by_run_id: string | null; workspace_id: string | null } | undefined; - if (!row) return null; - // Archived rows are retirement tombstones — the file no longer - // exists (or was explicitly retired) and the row is kept for - // audit only. Refuse to resolve a target so `openWorkProduct` - // can fail cleanly instead of pointing the shell at a path that - // leads nowhere. - if (row.status === "archived") return null; + if (!row) return { kind: "error", reason: "not_found", detail: `No work_product row with id=${workProductId}` }; + if (row.status === "archived") return { kind: "error", reason: "archived", detail: "Work product is archived (retirement tombstone)" }; if (row.url && row.url.length > 0) { return { kind: "url", value: row.url }; } - if (row.provider !== "filesystem" || !row.external_id) return null; + if (row.provider !== "filesystem" || !row.external_id) { + return { kind: "error", reason: "no_target", detail: `provider=${row.provider} external_id=${row.external_id ?? "null"} url=null` }; + } const workspaceRoot = resolveWorkspaceRoot( getDb, @@ -377,10 +379,18 @@ export function buildDocumentMethods( row.task_id, row.last_touched_by_run_id ?? row.created_by_run_id, ); - if (!workspaceRoot) return null; + if (!workspaceRoot) { + return { + kind: "error", + reason: "workspace_unresolved", + detail: `workspace_id=${row.workspace_id ?? "null"} task_id=${row.task_id ?? "null"} run_id=${row.last_touched_by_run_id ?? row.created_by_run_id ?? "null"} — none led to a workspace with a local_path`, + }; + } const absolute = joinWorkspaceRelative(workspaceRoot, row.external_id); - if (!absolute) return null; + if (!absolute) { + return { kind: "error", reason: "path_escape", detail: `external_id=${row.external_id} escapes workspace root ${workspaceRoot}` }; + } return { kind: "path", value: absolute }; }, diff --git a/src/main/ipc-core-read-handlers.ts b/src/main/ipc-core-read-handlers.ts index e57a44ab..990f6e13 100644 --- a/src/main/ipc-core-read-handlers.ts +++ b/src/main/ipc-core-read-handlers.ts @@ -69,7 +69,16 @@ export function registerCoreReadHandlers({ const companyError = ensureCompanyExists(parsed.data.companyId); if (companyError) return companyError; const target = db.documentsMod.resolveWorkProductTarget(parsed.data.id, parsed.data.companyId); - if (!target) return fail("WORK_PRODUCT_NOT_FOUND", "Work product not found or its workspace is unresolved."); + if (target.kind === "error") { + const codeByReason = { + not_found: "WORK_PRODUCT_NOT_FOUND", + archived: "WORK_PRODUCT_ARCHIVED", + no_target: "WORK_PRODUCT_NO_TARGET", + workspace_unresolved: "WORK_PRODUCT_WORKSPACE_UNRESOLVED", + path_escape: "WORK_PRODUCT_PATH_ESCAPE", + } as const; + return fail(codeByReason[target.reason], target.detail); + } if (target.kind === "url") { const error = await shell.openExternal(target.value).then(() => "").catch((err: unknown) => err instanceof Error ? err.message : String(err)); return error ? fail("OPEN_PATH_FAILED", error) : ok(true);