diff --git a/src/commands/code.ts b/src/commands/code.ts index ee5ef5e..c28c26d 100644 --- a/src/commands/code.ts +++ b/src/commands/code.ts @@ -1023,6 +1023,13 @@ export async function cmdCode( process.stderr.write("\n " + runSummary(summaryStatus, remaining, touched.size, secs) + "\n"); } if (log) process.stderr.write(` ⤷ log: ${log.dir}\n`); + // A viewer sees only Git's measured checkout snapshot after the run settles. + // RC is optional; an unavailable broker must not change the code result. + try { + if (!repoSpec) await rcObserver?.publishDiff(cwd); + } catch { + // The coding verdict remains authoritative if observation fails. + } if (process.env["AETHER_PROJECT_MEMORY_RECEIPTS_ENABLED"] === "1") { const memory = await completeMemory(memoryContext, pinnedMemory, outcome.state === "succeeded"); if (ctx.flags.json) process.stdout.write(JSON.stringify({ type: "project_memory_status", text: memory }) + "\n"); diff --git a/src/commands/rc.ts b/src/commands/rc.ts index ba87db0..9fc6044 100644 --- a/src/commands/rc.ts +++ b/src/commands/rc.ts @@ -27,6 +27,7 @@ import { spawnSync } from "node:child_process"; import { join, resolve } from "node:path"; import { configDir } from "../core/config.js"; +import { checkoutDiffSummary } from "../core/rc/diff_summary.js"; import type { CommandFlags } from "../core/command_dispatch.js"; import type { AppContext } from "../core/context.js"; import { digestOf } from "../core/device_runtime/canonical_json.js"; @@ -380,6 +381,12 @@ async function start( enqueueEvent(record, opened.event_type, opened.payload); const presence = hostPresenceEvent(enrolled.device_id, "live"); enqueueEvent(record, presence.event_type, presence.payload); + try { + const diff = await checkoutDiffSummary(deps.cwd); + if (diff) enqueueEvent(record, diff.event_type, diff.payload); + } catch { + // Diff observation is optional; session opening still succeeds. + } saveOutbox(hostDeps.outboxPath, record); const flushed = await flushOutbox(hostDeps, record); diff --git a/src/commands/rc_observation.ts b/src/commands/rc_observation.ts index 00c6274..b61d0f6 100644 --- a/src/commands/rc_observation.ts +++ b/src/commands/rc_observation.ts @@ -5,12 +5,14 @@ import type { BrainEvent } from "../core/brain_protocol.js"; import { resolve } from "node:path"; import type { ApiClient } from "../core/transport.js"; import { flushOutbox, type RcHostDeps } from "../core/rc/host.js"; +import { checkoutDiffSummary } from "../core/rc/diff_summary.js"; import { enqueueEvent, loadOutbox, saveOutbox } from "../core/rc/outbox.js"; import { mapBrainEventToRc } from "../core/rc/producers.js"; import { projectRefFor, rcOutboxPath } from "./rc.js"; export interface RcCodingObserver { feed(event: BrainEvent): void; + publishDiff(checkoutRoot: string): Promise; /** Lets integration tests wait for a started upload; the coding run never does. */ drain(): Promise; } @@ -70,6 +72,23 @@ export function openRcCodingObserver( // A broken or unwritable outbox is an RC failure, not a coding failure. } }, + async publishDiff(checkoutRoot): Promise { + if (stopped) return; + try { + const event = await checkoutDiffSummary(checkoutRoot); + if (!event) return; + const current = loadOutbox(path, root); + if (current.session_id !== record.session_id || current.revoke_pending) { + stopped = true; + return; + } + if (!enqueueEvent(record, event.event_type, event.payload)) return; + saveOutbox(path, record); + flush(); + } catch { + // A diff observation cannot change the local coding result. + } + }, drain: () => pending, }; } catch { diff --git a/src/core/diff_counts.ts b/src/core/diff_counts.ts index e6ab9a1..1936c76 100644 --- a/src/core/diff_counts.ts +++ b/src/core/diff_counts.ts @@ -114,7 +114,7 @@ export function numstatArgs(staged: boolean): string[] { * repository, and running them in series doubles the latency of the headline * number for no benefit. Neither writes anything. */ -export async function readDiffCounts(run: AsyncRunner, root: string): Promise> { +export async function readDiffCountSnapshot(run: AsyncRunner, root: string): Promise<{ counts: Map; complete: boolean }> { const [stagedRun, unstagedRun] = await Promise.all([ run("git", ["-C", root, ...numstatArgs(true)], root), run("git", ["-C", root, ...numstatArgs(false)], root), @@ -133,7 +133,11 @@ export async function readDiffCounts(run: AsyncRunner, root: string): Promise> { + return (await readDiffCountSnapshot(run, root)).counts; } export interface CountTotal { diff --git a/src/core/rc/diff_summary.ts b/src/core/rc/diff_summary.ts new file mode 100644 index 0000000..147f632 --- /dev/null +++ b/src/core/rc/diff_summary.ts @@ -0,0 +1,37 @@ +// A checkout snapshot for RC. Paths come from Git status; line counts come +// only from Git numstat. Neither model output nor a human-readable diff is read. +import { resolve } from "node:path"; +import { defaultAsyncRunner, readDiffCountSnapshot, totalCounts, type AsyncRunner } from "../diff_counts.js"; +import { parseStatusV2, STATUS_V2_ARGS } from "../review_state.js"; +import { defaultRunner, type Runner } from "../worktree.js"; +import { diffSummaryEvent, type RcProducedEvent } from "./producers.js"; +import { isSafeRelativePath } from "./redaction.js"; + +export async function checkoutDiffSummary( + projectRoot: string, + run: Runner = defaultRunner(), + runAsync: AsyncRunner = defaultAsyncRunner(), +): Promise { + const root = run("git", ["--no-optional-locks", "-C", projectRoot, "rev-parse", "--show-toplevel"], projectRoot); + if (root.status !== 0 || resolve(root.stdout.trim()) !== resolve(projectRoot)) return null; + + const status = run("git", ["--no-optional-locks", "-C", projectRoot, ...STATUS_V2_ARGS], projectRoot); + if (status.status !== 0) return null; + const paths = [...new Set(parseStatusV2(status.stdout).files.map((file) => file.path))].sort(); + // Refuse the whole snapshot: publishing counts for one set of paths and a + // filtered list for another would give the viewer a misleading summary. + if (paths.some((path) => !isSafeRelativePath(path)) || paths.length > 100_000) return null; + + const snapshot = await readDiffCountSnapshot(runAsync, projectRoot); + const total = totalCounts(snapshot.counts, paths); + const event = diffSummaryEvent(total, paths.slice(0, 64)); + event.payload["files_changed"] = paths.length; + // A failed side, binary or untracked path has no complete line count. The + // schema has optional counts, so omission is the honest unknown state. + if (!snapshot.complete || total.uncounted.length || paths.some((path) => snapshot.counts.get(path)?.binary) || + !Number.isSafeInteger(total.additions) || !Number.isSafeInteger(total.deletions)) { + delete event.payload["insertions"]; + delete event.payload["deletions"]; + } + return event; +} diff --git a/src/core/rc/redaction.ts b/src/core/rc/redaction.ts index d5bbc31..008cafb 100644 --- a/src/core/rc/redaction.ts +++ b/src/core/rc/redaction.ts @@ -76,6 +76,13 @@ const MAX_STRING_LENGTH = 1024; const MAX_LIST_ITEMS = 64; const ABSOLUTE_PATH = /^(?:[A-Za-z]:[\\/]|\\\\|\/|~[\\/])/; +/** Git's project-relative path form, with traversal and machine paths refused. */ +export function isSafeRelativePath(value: string): boolean { + return value.length > 0 && value.length <= 512 && + !ABSOLUTE_PATH.test(value) && !value.includes(":") && + !/[\\\u0000-\u001f\u007f]/.test(value) && + value.split("/").every((part) => part !== "" && part !== "." && part !== ".."); +} // C0 controls and DEL, built without literal control characters in the source. const CONTROL_CHARS = new RegExp( `[${String.fromCharCode(0)}-${String.fromCharCode(31)}${String.fromCharCode(127)}]`, @@ -145,6 +152,15 @@ export function sanitizeRemotePayload( options: SanitizeOptions, ): Record | null { if (!isViewerEventType(eventType)) return null; + if (eventType === "diff_summary") { + const files = payload["files"]; + if (files !== undefined && (!Array.isArray(files) || files.some((path: unknown) => + typeof path !== "string" || !isSafeRelativePath(path)))) return null; + for (const key of ["files_changed", "insertions", "deletions"]) { + const count = payload[key]; + if (count !== undefined && (typeof count !== "number" || !Number.isSafeInteger(count) || count < 0)) return null; + } + } const allowed = RC_ALLOWED_KEYS[eventType]; const env = options.env ?? process.env; const out: Record = {}; diff --git a/test/rc_coding_observation.test.ts b/test/rc_coding_observation.test.ts index cda489d..0c68c18 100644 --- a/test/rc_coding_observation.test.ts +++ b/test/rc_coding_observation.test.ts @@ -1,6 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync } from "node:fs"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -102,6 +103,35 @@ test("a safe error is queued when the broker is disconnected without delaying th assert.doesNotMatch(JSON.stringify(saved), /private-prompt-and-model-output/); }); +test("a settled coding worktree queues a measured diff in the launch project's RC session", async () => { + const root = mkdtempSync(join(tmpdir(), "rc-code-repo-")); + const checkout = `${root}-checkout`; + const outbox = join(mkdtempSync(join(tmpdir(), "rc-code-outbox-")), "outbox.json"); + const git = (...args: string[]): void => { + const result = spawnSync("git", ["-C", root, ...args], { encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + }; + git("init", "-q", "-b", "main"); + git("config", "user.email", "t@t.t"); + git("config", "user.name", "t"); + git("config", "commit.gpgsign", "false"); + git("config", "core.autocrlf", "false"); + writeFileSync(join(root, "a.txt"), "one\n"); + git("add", "a.txt"); + git("commit", "-q", "-m", "base"); + git("worktree", "add", "-q", "--detach", checkout); + writeFileSync(join(checkout, "a.txt"), "one\ntwo\n"); + seeded(root, outbox); + + const api = { postJson: async () => { throw new Error("offline"); } } as unknown as ApiClient; + const observer = openRcCodingObserver(root, api, outbox); + assert.ok(observer); + await observer.publishDiff(checkout); + await observer.drain(); + const diff = loadOutbox(outbox, root).events.find((event) => event.event_type === "diff_summary"); + assert.deepEqual(diff?.payload, { projection_version: "1", files_changed: 1, insertions: 1, deletions: 0, files: ["a.txt"] }); +}); + test("without rc start there is no observer and no account or upload call", async () => { const root = mkdtempSync(join(tmpdir(), "rc-code-")); let calls = 0; diff --git a/test/rc_diff_summary.test.ts b/test/rc_diff_summary.test.ts new file mode 100644 index 0000000..7cf6053 --- /dev/null +++ b/test/rc_diff_summary.test.ts @@ -0,0 +1,77 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { checkoutDiffSummary } from "../src/core/rc/diff_summary.js"; +import { createOutbox, enqueueEvent } from "../src/core/rc/outbox.js"; +import { diffSummaryEvent } from "../src/core/rc/producers.js"; +import type { Runner } from "../src/core/worktree.js"; +import { tmpWorkspace } from "./tmp_workspace.js"; + +const haveGit = !spawnSync("git", ["--version"], { encoding: "utf8" }).error; + +test("RC publishes measured changed, clean, and binary checkout snapshots", async (t) => { + if (!haveGit) return t.skip("git not available"); + const dir = tmpWorkspace("aether-rc-diff-"); + const git = (...args: string[]): void => { + const result = spawnSync("git", ["-C", dir, ...args], { encoding: "utf8" }); + assert.equal(result.status, 0, result.stderr); + }; + git("init", "-q", "-b", "main"); + git("config", "user.email", "t@t.t"); + git("config", "user.name", "t"); + git("config", "commit.gpgsign", "false"); + git("config", "core.autocrlf", "false"); + writeFileSync(join(dir, "a.txt"), "one\n"); + writeFileSync(join(dir, "image.bin"), Buffer.from([0, 1, 2])); + git("add", "-A"); + git("commit", "-q", "-m", "first"); + + const clean = await checkoutDiffSummary(dir); + assert.deepEqual(clean?.payload, { projection_version: "1", files_changed: 0, insertions: 0, deletions: 0, files: [] }); + + writeFileSync(join(dir, "a.txt"), "one\ntwo\n"); + const changed = await checkoutDiffSummary(dir); + assert.deepEqual(changed?.payload, { projection_version: "1", files_changed: 1, insertions: 1, deletions: 0, files: ["a.txt"] }); + + writeFileSync(join(dir, "image.bin"), Buffer.from([0, 1, 9])); + const binary = await checkoutDiffSummary(dir); + assert.equal(binary?.payload["files_changed"], 2); + assert.deepEqual(binary?.payload["files"], ["a.txt", "image.bin"]); + assert.equal(binary?.payload["insertions"], undefined, "binary line counts are unknown"); + assert.equal(binary?.payload["deletions"], undefined); + + const record = createOutbox({ session_id: "s", project_ref: "p", device_id: "d", epoch: 1, project_root: dir }); + assert.equal(enqueueEvent(record, binary!.event_type, binary!.payload), true); + assert.deepEqual(record.events[0]?.payload["files"], ["a.txt", "image.bin"]); + + git("restore", "image.bin"); + git("add", "a.txt"); + writeFileSync(join(dir, "a.txt"), Buffer.from([0, 1, 9])); + const mixed = await checkoutDiffSummary(dir); + assert.deepEqual(mixed?.payload["files"], ["a.txt"]); + assert.equal(mixed?.payload["insertions"], undefined, "a binary side makes the aggregate unknown"); + assert.equal(mixed?.payload["deletions"], undefined); +}); + +test("external paths are refused before durable enqueue", async () => { + const root = "C:/checkout"; + const run: Runner = (_cmd, args) => args.includes("rev-parse") + ? { status: 0, stdout: `${root}\n`, stderr: "" } + : { status: 0, stdout: "? /outside/secret.txt\0", stderr: "" }; + assert.equal(await checkoutDiffSummary(root, run), null); + const record = createOutbox({ session_id: "s", project_ref: "p", device_id: "d", epoch: 1, project_root: root }); + const unsafe = diffSummaryEvent({ additions: 1, deletions: 0, uncounted: [] }, ["/outside/secret.txt"]); + assert.equal(enqueueEvent(record, unsafe.event_type, unsafe.payload), false); + assert.equal(record.events.length, 0); +}); + +test("a failed numstat read never becomes a measured zero", async () => { + const root = "C:/checkout"; + const run: Runner = (_cmd, args) => args.includes("rev-parse") + ? { status: 0, stdout: `${root}\n`, stderr: "" } + : { status: 0, stdout: "? new.txt\0", stderr: "" }; + const event = await checkoutDiffSummary(root, run, async () => ({ status: 1, stdout: "", stderr: "unavailable" })); + assert.deepEqual(event?.payload, { projection_version: "1", files_changed: 1, files: ["new.txt"] }); +}); diff --git a/test/rc_producers.test.ts b/test/rc_producers.test.ts index 5c7e2f1..3b7d055 100644 --- a/test/rc_producers.test.ts +++ b/test/rc_producers.test.ts @@ -497,11 +497,11 @@ test("preview publishes a public URL but never a loopback one", () => { assert.equal(persisted(previewEvent({ ...PREVIEW, url: "https://preview.example/app?token=private" }, () => false))?.["url"], undefined); }); -test("path traversal and URL targets are reduced to safe identifiers", () => { +test("unsafe diff paths are refused and URL targets are reduced to safe identifiers", () => { const diff = persisted(diffSummaryEvent({ additions: 1, deletions: 0, uncounted: [] }, ["../private.txt"])); - assert.deepEqual(diff?.["files"], ["[external-path]"]); + assert.equal(diff, null); const unnamed = persisted(diffSummaryEvent({ additions: 0, deletions: 0, uncounted: [] }, [""])); - assert.deepEqual(unnamed?.["files"], ["[unnamed-file]"]); + assert.equal(unnamed, null); const tool = mapBrainEventToRc({ type: "tool_call", id: "1", name: "fetch", args: { target: "https://user:password@example.test/path?token=private", } });