diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index 2b0c887c0..576e36f28 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -687,6 +687,12 @@ export default function Page() { // file-op-notify message. We track the rename so the tool-metadata diffs // (which still have the old path) are displayed at the new location. const [fileRenames, setFileRenames] = createSignal(new Map()) + // --- External file status tracking (#844) --- + // When the FileWatcherBridge in the extension host detects that a file has + // been deleted (or recreated), it posts fs-diff-invalidate. We track the + // status here so mergeServerAndToolDiffs can override the original status + // for both in-project and cross-project files. + const [externalFileStatus, setExternalFileStatus] = createSignal(new Map()) const onFileOpNotify = (e: MessageEvent) => { const d = e.data as { source?: string; kind?: string; op?: string; oldPath?: string; newPath?: string; home?: string } | undefined if (d?.source !== "amicode" || d?.kind !== "file-op-notify") return @@ -722,6 +728,41 @@ export default function Page() { window.addEventListener("message", onFileOpNotify) onCleanup(() => window.removeEventListener("message", onFileOpNotify)) + // #844: fs-diff-invalidate — the FileWatcherBridge detected a change to a + // file we're watching. Update externalFileStatus for deletions and bump + // diff_version to trigger a server refetch. + const onFsDiffInvalidate = (e: MessageEvent) => { + const d = e.data as { source?: string; kind?: string; file?: string; changeType?: string } | undefined + if (d?.source !== "amicode" || d?.kind !== "fs-diff-invalidate") return + if (!d.file) return + const sessionID = params.id + if (!sessionID) return + // Normalize the file path the same way reviewDiffs does + const dir = sdk().directory + const home = typeof globalThis.process !== "undefined" ? globalThis.process.env?.HOME : undefined + const prefix = home && dir.startsWith(home) ? "~" + dir.slice(home.length) : dir + const normFile = toHomePath(d.file, home, prefix) + if (d.changeType === "deleted") { + setExternalFileStatus((prev) => { + const next = new Map(prev) + next.set(normFile, "deleted") + return next + }) + } else { + // File was recreated or modified — clear any stale deletion status + setExternalFileStatus((prev) => { + if (!prev.has(normFile)) return prev + const next = new Map(prev) + next.delete(normFile) + return next + }) + } + // Bump diff_version to trigger a server refetch + sync().set("diff_version", sessionID, (v: number | undefined) => (v ?? 0) + 1) + } + window.addEventListener("message", onFsDiffInvalidate) + onCleanup(() => window.removeEventListener("message", onFsDiffInvalidate)) + // Refetch when the session transitions to idle (assistant finished, snapshot taken) // or when a file-editing tool completes mid-turn (diff_version bumps) const sessionDiffVersion = () => { @@ -796,9 +837,27 @@ export default function Page() { serverResponded, directory: dir, home, + externalFileStatus: externalFileStatus(), }) }) + // #844: Send watch-files to the extension host whenever the file list changes. + // The FileWatcherBridge watches these paths for external create/change/delete. + // We resolve ~/... paths back to absolute for the OS-level watcher. + createEffect(() => { + const diffs = reviewDiffs() + const home = typeof globalThis.process !== "undefined" ? globalThis.process.env?.HOME : undefined + const files = diffs + .map((d) => { + if (!d.file) return "" + // Resolve ~/... back to absolute + if (home && d.file.startsWith("~/")) return home + d.file.slice(1) + return d.file + }) + .filter((f) => f.length > 0) + window.parent.postMessage({ source: "amicode", kind: "watch-files", files }, "*") + }) + // All files touched by edit tools in this session — fetched from the server // which scans ALL messages regardless of client-side pagination. const touchedFilesQuery = createQuery(() => { diff --git a/packages/app/src/pages/session/v2/accumulate-diffs.test.ts b/packages/app/src/pages/session/v2/accumulate-diffs.test.ts index 7e0500088..1089437ca 100644 --- a/packages/app/src/pages/session/v2/accumulate-diffs.test.ts +++ b/packages/app/src/pages/session/v2/accumulate-diffs.test.ts @@ -204,6 +204,84 @@ describe("mergeServerAndToolDiffs", () => { }) expect(result).toHaveLength(1) }) + + // --- externalFileStatus: filesystem watcher deletion overrides --- + + test("externalFileStatus overrides status to 'deleted' for cross-project files", () => { + const externalFileStatus = new Map([["~/other-project/bar.ts", "deleted" as const]]) + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs: [diff("~/other-project/bar.ts", "stale-patch", 3, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + externalFileStatus, + }) + expect(result).toHaveLength(1) + expect(result[0].file).toBe("~/other-project/bar.ts") + expect(result[0].status).toBe("deleted") + }) + + test("externalFileStatus does not affect files NOT in the status map", () => { + const externalFileStatus = new Map([["~/other-project/bar.ts", "deleted" as const]]) + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("src/foo.ts")], + toolDiffs: [diff("~/other-project/baz.ts", "patch", 1, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + externalFileStatus, + }) + const baz = result.find((d) => d.file === "~/other-project/baz.ts") + expect(baz).toBeDefined() + expect(baz!.status).toBe("added") + }) + + test("externalFileStatus with empty map has no effect", () => { + const externalFileStatus = new Map() + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs: [diff("~/other-project/bar.ts", "patch", 1, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + externalFileStatus, + }) + expect(result).toHaveLength(1) + expect(result[0].status).toBe("added") + }) + + test("externalFileStatus overrides status to 'deleted' for in-project server diffs", () => { + // Server still returns the file (snapshot-based diff shows "added"), + // but the watcher detected an external deletion. Override the status. + const externalFileStatus = new Map([["~/harmoniqs/amicode/test.md", "deleted" as const]]) + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("test.md", "patch", 3, 0, "added")], + toolDiffs: [diff("~/harmoniqs/amicode/test.md", "tool-patch", 3, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + externalFileStatus, + }) + const entry = result.find((d) => d.file === "~/harmoniqs/amicode/test.md") + expect(entry).toBeDefined() + expect(entry!.status).toBe("deleted") + }) + + test("externalFileStatus does not affect in-project server diffs NOT in the map", () => { + const externalFileStatus = new Map([["~/harmoniqs/amicode/other.md", "deleted" as const]]) + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("test.md", "patch", 3, 0, "added")], + toolDiffs: [], + serverResponded: true, + directory: DIR, + home: HOME, + externalFileStatus, + }) + const entry = result.find((d) => d.file === "~/harmoniqs/amicode/test.md") + expect(entry).toBeDefined() + expect(entry!.status).toBe("added") + }) }) // --- applyRenames --- diff --git a/packages/app/src/pages/session/v2/accumulate-diffs.ts b/packages/app/src/pages/session/v2/accumulate-diffs.ts index 22d4e1ef8..0a6c52e6d 100644 --- a/packages/app/src/pages/session/v2/accumulate-diffs.ts +++ b/packages/app/src/pages/session/v2/accumulate-diffs.ts @@ -24,6 +24,11 @@ export interface MergeOpts { serverResponded: boolean directory: string home: string | undefined + /** External status overrides from the filesystem watcher. When a file is + * marked "deleted" here, its status in the merged output is overridden + * regardless of the server's or tool-metadata's original status — applies + * to both in-project and cross-project files. */ + externalFileStatus?: Map } /** @@ -39,23 +44,30 @@ export interface MergeOpts { * are returned as a fallback. */ export function mergeServerAndToolDiffs(opts: MergeOpts): Array { - const { serverDiffs, toolDiffs, serverResponded, directory, home } = opts + const { serverDiffs, toolDiffs, serverResponded, directory, home, externalFileStatus } = opts const prefix = home && directory.startsWith(home) ? "~" + directory.slice(home.length) : directory const projectPrefix = prefix + "/" if (serverDiffs.length > 0 || serverResponded) { const normalizedServerDiffs = serverDiffs .filter((d): d is SnapshotFileDiff & { file: string } => !!d.file) - .map((d) => ({ ...d, file: toHomePath(d.file, home, prefix) })) + .map((d) => { + const normed = { ...d, file: toHomePath(d.file, home, prefix) } + const override = externalFileStatus?.get(normed.file) + return override ? { ...normed, status: override } : normed + }) const serverFiles = new Set(normalizedServerDiffs.map((d) => d.file)) // Only pass through tool-metadata diffs that are BOTH absent from the // server set AND outside the project directory. In-project files trust // the server's authority — if the server excluded them (created + deleted, // or reverted), they should not leak through as phantom entries. - const crossProjectDiffs = toolDiffs.filter( - (d) => !serverFiles.has(d.file) && !d.file.startsWith(projectPrefix), - ) + const crossProjectDiffs = toolDiffs + .filter((d) => !serverFiles.has(d.file) && !d.file.startsWith(projectPrefix)) + .map((d) => { + const override = externalFileStatus?.get(d.file) + return override ? { ...d, status: override } : d + }) return [...normalizedServerDiffs, ...crossProjectDiffs] }