sandbox file-diff: make failures observable + correct (follow-up to #892)

[tombeckenham]

Follow-up to #892 (typed custom events + sandbox file-diff hooks). A post-merge review surfaced a set of correctness and observability gaps in the new sandbox file-diff subsystem. They share a single root cause — buildFileHookEvent and the watcher are built on a "never throws, fall back to ''/empty-map, don't log" contract — so a git/exec/fs failure silently becomes an empty value with no signal. Grouping them here since a single fix pass addresses the theme.

The typed-events half of #892 is sound and is intentionally excluded from this issue (type-hardening items will be filed separately). Severity overall: medium — the feature is new and opt-in (no prod consumers yet), no security/data-loss angle, but it silently delivers wrong/empty data, which is the worst failure mode for an observability feature.

Line references are against main post-merge.

1. New (untracked) files produce an empty diff — packages/ai-sandbox/src/file-diff.ts:44-77

For a create event in a git-backed workspace (baseSha !== ''), diff() runs git diff <baseSha> -- <path>. Git does not show untracked files, so it exits 0 with empty stdout and diff() returns ''. The synthesizeAddPatch fallback that would render the new file's contents only runs when baseSha === '' (non-git). Net effect: with fileEvents: { diff: true }, every file the agent creates — the most common agent action — emits a sandbox.file.diff chunk carrying diff: '', and the accessors become internally inconsistent (after() has content, diff() is empty).

Fix idea: when git diff yields empty for a create, fall back to synthesizeAddPatch (or git add -N / git diff --no-index /dev/null <path>).

2. A failed find poll fabricates a delete→create event storm — packages/ai-sandbox/src/watch.ts:204-206

return result.exitCode === 0
  ? parseFindOutput(result.stdout, root)
  : new Map<string, string>()

A non-zero find exit collapses the snapshot to empty; the next diffSnapshots(previous, next) then emits a delete for every tracked file, and a create for each when the poll recovers. One transient blip (slow container, find not on PATH, permissions) fans phantom deletes/creates out to user hooks and the stream. Note the asymmetry: the thrown-exec path in tick() (watch.ts:220) correctly preserves previous — the non-zero-exit path must do the same, and log stderr.

3. Systemic swallow-to-empty with zero logging — file-diff.ts (before/after/diff), middleware.ts:133 (baseSha capture), middleware.ts:104 (hook-dispatch catch)

Every git/exec/fs failure is converted to an empty value with no log, even though res.stderr is captured right there. Two aggravating specifics:

• The empty catch in dispatchDefinitionHooks (middleware.ts:104) contradicts the docs this PR shipped — packages/ai/skills/ai-core/middleware/SKILL.md states hook errors are "caught and logged under the sandbox debug category." They are not. The sibling run-scoped path already does it right: packages/ai/src/activities/chat/index.ts:727 logs via this.logger.errors('sandbox file hook failed', ...).
• A silent baseSha-capture failure (middleware.ts:133-140) degrades a real git repo to "every file is brand new" (full-file add-patches for every change) with nothing to grep for.

Fix idea: log the swallowed error / non-zero res.stderr (via the existing sandbox/errors logger categories) before falling back; align the doc claim with actual behavior.

4. The pendingDiffs teardown-drain is untested for abort/error — packages/ai-sandbox/src/middleware.ts:213,248,265

pendingDiffs exists so an in-flight diff() isn't dropped when a run ends. But the only test that fires a diff awaits a flush before onFinish, so the promise is already settled; the onAbort (:248) and onError (:265) drains are entirely untested. Deleting await Promise.allSettled(state.pendingDiffs) from any of the three paths would pass the whole suite while silently dropping the final file's diff under real timing. A single deferred-promise test (fire a change, call onFinish/onAbort/onError without flushing, assert the drain awaits the pending diff) closes it.

---

Observed live

A real claude-code × docker run with fileEvents: { diff: true } confirmed the feature streams sandbox.file + sandbox.file.diff end-to-end with real unified-diff payloads (and the /workspace relativization fix works). Because that example uses a non-git workspace (source: { type: 'none' }), every change came through as a full-file --- /dev/null add-patch — the flip side of #1/#3's baseline handling.

Out of scope (separate issue)

Type-hardening: the producer side is untyped (createCustomEventChunk(name: string, value: Record<string, any>) as CustomEvent at index.ts:2603 — no compile-time link to KnownCustomEvent), and SessionIdEvent's \${string}.session-id`` name is broader than the 4 real adapter literals.

Raised from a post-merge review of #892.