From 0813e691cd44b16c5288844d463698b690d47f68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 19 Apr 2026 16:56:22 -0400 Subject: [PATCH] Retire work_products whose files the run deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #16 kept deletions from registering, but rows created by earlier runs stayed `status = "active"` forever even after the agent `rm`'d the file. The DocumentCenter kept showing zombies and `openWorkProduct` resolved to an empty path. `ChangeSet` now carries both `changed` and `deleted` paths, populated from the two detectors: - git porcelain: the `D` status entries that were previously dropped are now captured into `deleted` instead; - baseline diff: paths present in the baseline but no longer on disk at diff time are `deleted`. A new `retireDeletedArtifacts` pass runs alongside `projectChangedPaths` and flips the matching `work_products.status` to `archived`, bumping `last_touched_by_run_id` to the retiring run so the provenance timeline records the deletion. `upsertFilesystemArtifact` sets `status = "active"` on update so a subsequent run that recreates the path brings the row back into the visible set. A new `ProjectionOutcome` variant — `retired` — lets the hook and reconciliation logs report both sides of the fan-out. --- src/main/agent-runtime/artifact-projector.ts | 146 ++++++++++++++----- src/main/agent-runtime/hook-handlers.ts | 5 +- src/main/agent-runtime/runtime-bridge.ts | 5 +- 3 files changed, 116 insertions(+), 40 deletions(-) diff --git a/src/main/agent-runtime/artifact-projector.ts b/src/main/agent-runtime/artifact-projector.ts index 29e10dd9..36acf350 100644 --- a/src/main/agent-runtime/artifact-projector.ts +++ b/src/main/agent-runtime/artifact-projector.ts @@ -217,7 +217,8 @@ function isRegistrablePath(absPath: string, workspacePath: string): boolean { export type ProjectionOutcome = | { kind: "skipped"; reason: string } | { kind: "inserted"; workProductId: string; relativePath: string } - | { kind: "updated"; workProductId: string; relativePath: string }; + | { kind: "updated"; workProductId: string; relativePath: string } + | { kind: "retired"; workProductId: string; relativePath: string }; /** * Upsert work_products rows for a successful destructive tool call. @@ -266,9 +267,13 @@ export function reconcileWorkspaceArtifacts( if (change.source === "none") { return [{ kind: "skipped", reason: change.skipReason ?? "no_changes" }]; } - const outcomes = change.paths.length === 0 - ? [{ kind: "skipped", reason: "no_changes" } satisfies ProjectionOutcome] - : projectChangedPaths(db, ctx, change.paths); + const outcomes: ProjectionOutcome[] = []; + if (change.changed.length === 0 && change.deleted.length === 0) { + outcomes.push({ kind: "skipped", reason: "no_changes" }); + } else { + outcomes.push(...projectChangedPaths(db, ctx, change.changed)); + outcomes.push(...retireDeletedArtifacts(db, ctx, change.deleted)); + } if (change.coverageTruncated) { outcomes.push({ kind: "skipped", reason: "baseline_truncated" }); } @@ -298,6 +303,42 @@ function projectChangedPaths( return outcomes; } +/** + * Archive `work_products` rows whose filesystem sources the change + * detector reported as removed during this run. The row stays in the + * table as a tombstone (operators can audit who produced what) but + * drops to `status = "archived"` so the DocumentCenter stops + * surfacing it and `openWorkProduct` no longer leads to an empty + * path. `last_touched_by_run_id` carries the retiring run so the + * provenance timeline records the deletion alongside the earlier + * edits. + */ +function retireDeletedArtifacts( + db: AppDatabase, + ctx: ArtifactProjectorRunContext, + deleted: string[], +): ProjectionOutcome[] { + if (deleted.length === 0) return []; + const outcomes: ProjectionOutcome[] = []; + const budget = deleted.slice(0, MAX_BASH_PROJECTED_FILES); + for (const rel of budget) { + const existing = db.findWorkProductByExternalId( + ctx.companyId, + "filesystem", + rel, + ctx.workspaceId, + ); + if (!existing) continue; + db.updateWorkProduct(existing.id, ctx.companyId, { + status: "archived", + lastTouchedByRunId: ctx.runId, + workspaceId: ctx.workspaceId, + }); + outcomes.push({ kind: "retired", workProductId: existing.id, relativePath: rel }); + } + return outcomes; +} + /** * Compare current workspace mtimes against the run-start baseline. * Skipped-dir filtering mirrors the baseline walker so the diff and @@ -309,16 +350,22 @@ function projectChangedPaths( * - it was not in the baseline AND the baseline saw the whole tree * (`baseline.truncated === false`). * + * A file is reported as `deleted` when it was in the baseline at + * capture time and is no longer a regular file on disk — the + * retirement path archives the matching `work_products` row so the + * DocumentCenter stops showing a removed file. + * * A truncated baseline deliberately stops classifying unsampled files * as new: the walker gave up before reaching them, so we cannot * distinguish a file that was always there from a file the run just * wrote. Falsely registering every one of them as a deliverable is * worse than missing a few that live past the sample cap. Callers - * emit a `baseline_truncated` outcome so operators can see the - * degraded signal without the bogus rows landing in `work_products`. + * emit a `baseline_truncated` outcome so operators see the degraded + * signal. */ -function diffAgainstBaseline(workspacePath: string, baseline: WorkspaceBaseline): string[] { +function diffAgainstBaseline(workspacePath: string, baseline: WorkspaceBaseline): GitChangeSet { const changed: string[] = []; + const seenRels = new Set(); const stack: string[] = [workspacePath]; while (stack.length > 0) { const dir = stack.pop(); @@ -347,6 +394,7 @@ function diffAgainstBaseline(workspacePath: string, baseline: WorkspaceBaseline) } const rel = relative(workspacePath, abs); if (rel.length === 0) continue; + seenRels.add(rel); const previous = baseline.mtimeByRelPath.get(rel); if (previous === undefined) { if (!baseline.truncated) changed.push(rel); @@ -355,7 +403,11 @@ function diffAgainstBaseline(workspacePath: string, baseline: WorkspaceBaseline) if (mtimeMs > previous) changed.push(rel); } } - return changed; + const deleted: string[] = []; + for (const rel of baseline.mtimeByRelPath.keys()) { + if (!seenRels.has(rel)) deleted.push(rel); + } + return { changed, deleted }; } function projectSinglePathArtifact( @@ -379,8 +431,10 @@ type ChangeSource = "git" | "baseline" | "none"; interface ChangeSet { source: ChangeSource; - /** Paths relative to the workspace root. */ - paths: string[]; + /** Paths, relative to the workspace root, that currently exist on disk with a new or advanced mtime. */ + changed: string[]; + /** Paths, relative to the workspace root, that the detector reports as removed during the run. */ + deleted: string[]; /** Reason the source chose to skip (only meaningful when source === "none"). */ skipReason?: string; /** @@ -394,22 +448,27 @@ interface ChangeSet { } function collectChangedPaths(ctx: ArtifactProjectorRunContext): ChangeSet { - if (!ctx.workspacePath) return { source: "none", paths: [], skipReason: "no_workspace" }; + if (!ctx.workspacePath) { + return { source: "none", changed: [], deleted: [], skipReason: "no_workspace" }; + } if (isGitRepo(ctx.workspacePath)) { try { - return { source: "git", paths: listChangedRelPaths(ctx.workspacePath) }; + const gitSet = listChangedRelPaths(ctx.workspacePath); + return { source: "git", changed: gitSet.changed, deleted: gitSet.deleted }; } catch { - return { source: "none", paths: [], skipReason: "git_status_failed" }; + return { source: "none", changed: [], deleted: [], skipReason: "git_status_failed" }; } } if (ctx.workspaceBaseline) { + const diff = diffAgainstBaseline(ctx.workspacePath, ctx.workspaceBaseline); return { source: "baseline", - paths: diffAgainstBaseline(ctx.workspacePath, ctx.workspaceBaseline), + changed: diff.changed, + deleted: diff.deleted, coverageTruncated: ctx.workspaceBaseline.truncated, }; } - return { source: "none", paths: [], skipReason: "no_baseline" }; + return { source: "none", changed: [], deleted: [], skipReason: "no_baseline" }; } function projectBashArtifacts( @@ -420,9 +479,13 @@ function projectBashArtifacts( if (change.source === "none") { return [{ kind: "skipped", reason: change.skipReason ?? "no_changes" }]; } - const outcomes = change.paths.length === 0 - ? [{ kind: "skipped", reason: "no_changes" } satisfies ProjectionOutcome] - : projectChangedPaths(db, ctx, change.paths); + const outcomes: ProjectionOutcome[] = []; + if (change.changed.length === 0 && change.deleted.length === 0) { + outcomes.push({ kind: "skipped", reason: "no_changes" }); + } else { + outcomes.push(...projectChangedPaths(db, ctx, change.changed)); + outcomes.push(...retireDeletedArtifacts(db, ctx, change.deleted)); + } if (change.coverageTruncated) { outcomes.push({ kind: "skipped", reason: "baseline_truncated" }); } @@ -456,30 +519,34 @@ function isGitRepo(workspacePath: string): boolean { } } +interface GitChangeSet { + changed: string[]; + deleted: string[]; +} + /** - * Enumerate tracked-modified + untracked files via `git status --porcelain=v1 -z`. + * Enumerate porcelain status entries and split them into "currently on + * disk" (`changed`) and "removed during the run" (`deleted`). The + * retirement path consumes `deleted` to archive the matching + * `work_products` row so the DocumentCenter stops showing a file that + * no longer exists. * - * Output format per entry: `XY path`. NUL terminates each entry. For - * renames the new path is in the entry with the `R` status code and - * the old path follows as its own NUL-terminated entry — we advance - * past that old-path entry without emitting it. - * - * Deleted paths are excluded at the source. A worktree `D` status (the - * file is gone on disk) or a staged `D` with an unchanged worktree - * both represent a removal, not a produced artifact; registering them - * would plant ghost rows in `work_products` and send the DocumentCenter - * "open" link to an empty path. + * Porcelain v1 format per entry: `XY path`, NUL-terminated. For + * renames the new path is the entry and the old path follows as its + * own NUL-terminated trailer — we advance past the trailer without + * emitting it. */ -function listChangedRelPaths(workspacePath: string): string[] { +function listChangedRelPaths(workspacePath: string): GitChangeSet { const raw = execFileSync("git", ["status", "--porcelain=v1", "-z", "--untracked-files=all"], { cwd: workspacePath, timeout: 5_000, encoding: "utf8", maxBuffer: 1024 * 1024, }); - if (!raw) return []; + if (!raw) return { changed: [], deleted: [] }; const entries = raw.split("\0").filter(Boolean); - const paths: string[] = []; + const changed: string[] = []; + const deleted: string[] = []; for (let i = 0; i < entries.length; i++) { const entry = entries[i]; if (entry.length < 3) continue; @@ -489,21 +556,23 @@ function listChangedRelPaths(workspacePath: string): string[] { if (indexStatus === "!" && worktreeStatus === "!") continue; const isRename = indexStatus === "R" || worktreeStatus === "R"; if (representsDeletion(indexStatus, worktreeStatus)) { + deleted.push(rest); if (isRename) i += 1; continue; } - paths.push(rest); + changed.push(rest); if (isRename) i += 1; } - return paths; + return { changed, deleted }; } function representsDeletion(indexStatus: string, worktreeStatus: string): boolean { // Worktree missing (`Y = D`) always means the file is not on disk. if (worktreeStatus === "D") return true; // Staged deletion whose worktree matches (`XD` + Y space) means the - // file was removed from the tree and the worktree is in sync; nothing - // to register either. + // file was removed from the tree and the worktree is in sync; the + // retirement path needs to hear about it so the corresponding + // `work_products` row can be archived. if (indexStatus === "D" && worktreeStatus === " ") return true; return false; } @@ -543,8 +612,13 @@ function upsertFilesystemArtifact( // all see the latest run that wrote the file. const existing = db.findWorkProductByExternalId(ctx.companyId, provider, relativePath, ctx.workspaceId); 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.updateWorkProduct(existing.id, ctx.companyId, { title, + status: "active", lastTouchedByRunId: ctx.runId, workspaceId: ctx.workspaceId, }); diff --git a/src/main/agent-runtime/hook-handlers.ts b/src/main/agent-runtime/hook-handlers.ts index da161a68..66c06399 100644 --- a/src/main/agent-runtime/hook-handlers.ts +++ b/src/main/agent-runtime/hook-handlers.ts @@ -194,9 +194,10 @@ export function handleAfterToolCall( }, ); const registered = outcomes.filter((o) => o.kind === "inserted" || o.kind === "updated").length; - if (registered > 0) { + const retired = outcomes.filter((o) => o.kind === "retired").length; + if (registered > 0 || retired > 0) { deps.logger.info( - `[hook-handlers] projection registered ${registered} artifact(s) for run ${ctx.runId} via ${payload.toolName}`, + `[hook-handlers] projection for run ${ctx.runId} via ${payload.toolName}: ${registered} registered, ${retired} retired`, ); } } catch (error) { diff --git a/src/main/agent-runtime/runtime-bridge.ts b/src/main/agent-runtime/runtime-bridge.ts index 08ec78f3..6399d1e7 100644 --- a/src/main/agent-runtime/runtime-bridge.ts +++ b/src/main/agent-runtime/runtime-bridge.ts @@ -133,9 +133,10 @@ export class RuntimeBridge { workspaceBaseline: ctx.workspaceBaseline, }); const inserted = outcomes.filter((o) => o.kind === "inserted").length; - if (inserted > 0) { + const retired = outcomes.filter((o) => o.kind === "retired").length; + if (inserted > 0 || retired > 0) { this.deps.logger.info( - `[runtime-bridge] reconciliation registered ${inserted} new artifact(s) for run ${ctx.runId}`, + `[runtime-bridge] reconciliation for run ${ctx.runId}: ${inserted} registered, ${retired} retired`, ); } } catch (error) {