Browse Source

Merge pull request #988 from pxmpsdev/fix/image-gitignore-and-smoke-cleanup

fix(image-hook): scope .gitignore to images directory (plus smoke-script cleanup)
Alvin 1 week ago
parent
commit
9f5b850504

+ 8 - 5
scripts/verify-opencode-host-smoke.ts

@@ -293,17 +293,20 @@ async function verifyHostSmoke(tarballPath: string) {
         ),
         exitPromise,
       ]);
+
+      await new Promise((resolve) => setTimeout(resolve, 1500));
+      assertNoPluginLoadErrors(`${stdout}\n${stderr}`);
     } catch (error) {
       const message = error instanceof Error ? error.message : String(error);
       fail(
         `${message}\nCaptured OpenCode logs:\n${formatCapturedLogs(stdout, stderr)}`,
       );
+    } finally {
+      // Always terminate the spawned server, including on the health-check
+      // failure path where stopProcess would otherwise never be reached and
+      // the child process would leak away after the temp dir is removed.
+      await stopProcess(child);
     }
-
-    await new Promise((resolve) => setTimeout(resolve, 1500));
-    assertNoPluginLoadErrors(`${stdout}\n${stderr}`);
-
-    await stopProcess(child);
   } finally {
     rmSync(tempRoot, { recursive: true, force: true });
   }

+ 467 - 0
src/hooks/image-hook.test.ts

@@ -1,8 +1,16 @@
 import { afterAll, describe, expect, it } from 'bun:test';
+import { spawnSync } from 'node:child_process';
+import { createHash } from 'node:crypto';
 import {
   chmodSync,
+  existsSync,
+  linkSync,
+  lstatSync,
   mkdirSync,
+  readdirSync,
+  readFileSync,
   rmSync,
+  symlinkSync,
   utimesSync,
   writeFileSync,
 } from 'node:fs';
@@ -14,6 +22,14 @@ import type { MessageWithParts } from './types';
 
 const TEST_DIR = path.join(os.tmpdir(), `image-hook-test-${process.pid}`);
 const IMG = { type: 'image', url: 'data:image/png;base64,AAAA' };
+const IMG_BYTES = Buffer.from('AAAA', 'base64');
+const IMG_HASH = createHash('sha1').update(IMG_BYTES).digest('hex').slice(0, 8);
+const IMG_CONTENT_NAME = `image-${IMG_HASH}.png`;
+const LEGACY_GITIGNORE = '*\n';
+const LEGACY_GITIGNORE_BYTES = Buffer.from(LEGACY_GITIGNORE);
+const LEGACY_GITIGNORE_BACKUP = '.gitignore.oh-my-opencode-slim-legacy';
+const IMAGES_GITIGNORE = 'images/\n';
+const IMAGES_GITIGNORE_BYTES = Buffer.from(IMAGES_GITIGNORE);
 
 function makeTestDir(name: string): { workDir: string; saveDir: string } {
   const workDir = path.join(TEST_DIR, name);
@@ -22,6 +38,25 @@ function makeTestDir(name: string): { workDir: string; saveDir: string } {
   return { workDir, saveDir };
 }
 
+function gitignorePath(workDir: string): string {
+  return path.join(workDir, '.opencode', '.gitignore');
+}
+
+function legacyGitignoreBackupPath(workDir: string): string {
+  return path.join(workDir, '.opencode', LEGACY_GITIGNORE_BACKUP);
+}
+
+function writeOpencodeGitignore(
+  workDir: string,
+  content: string | Buffer,
+): string {
+  const opencodeDir = path.join(workDir, '.opencode');
+  mkdirSync(opencodeDir, { recursive: true });
+  const gi = gitignorePath(workDir);
+  writeFileSync(gi, content);
+  return gi;
+}
+
 function makeOldFile(dir: string, name: string): string {
   const filePath = path.join(dir, name);
   writeFileSync(filePath, 'data');
@@ -116,6 +151,438 @@ describe('processImageAttachments image routing', () => {
     expect(textParts[0]?.text).toContain('@observer');
   });
 
+  it('writes a .gitignore covering only the images directory in a fresh workspace', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-fresh');
+    mkdirSync(workDir, { recursive: true });
+    expect(existsSync(gitignorePath(workDir))).toBe(false);
+    processImageAttachments({
+      messages: [makeUserMsg([IMG])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      IMAGES_GITIGNORE_BYTES,
+    );
+  });
+
+  it('migrates exact legacy * gitignore before early returns (direct/text-only)', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-legacy-direct');
+    writeOpencodeGitignore(workDir, LEGACY_GITIGNORE_BYTES);
+    const logs: string[] = [];
+
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'no images' }])],
+      workDir,
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: (message) => logs.push(message),
+    });
+
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      IMAGES_GITIGNORE_BYTES,
+    );
+    expect(readFileSync(legacyGitignoreBackupPath(workDir))).toEqual(
+      LEGACY_GITIGNORE_BYTES,
+    );
+    expect(logs.some((message) => message.includes('backup created at'))).toBe(
+      true,
+    );
+  });
+
+  it('legacy gitignore migration is idempotent', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-legacy-idempotent');
+    writeOpencodeGitignore(workDir, LEGACY_GITIGNORE_BYTES);
+
+    const run = () =>
+      processImageAttachments({
+        messages: [makeUserMsg([{ type: 'text', text: 'hello' }])],
+        workDir,
+        imageRouting: 'direct',
+        disabledAgents: new Set<string>(),
+        log: () => {},
+      });
+
+    run();
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      IMAGES_GITIGNORE_BYTES,
+    );
+    const backupAfterFirst = readFileSync(legacyGitignoreBackupPath(workDir));
+    run();
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      IMAGES_GITIGNORE_BYTES,
+    );
+    expect(readFileSync(legacyGitignoreBackupPath(workDir))).toEqual(
+      backupAfterFirst,
+    );
+  });
+
+  it('uses an exact existing legacy gitignore backup unchanged', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-existing-backup');
+    const existingBackup = LEGACY_GITIGNORE_BYTES;
+    writeOpencodeGitignore(workDir, LEGACY_GITIGNORE_BYTES);
+    writeFileSync(legacyGitignoreBackupPath(workDir), existingBackup);
+
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'hello' }])],
+      workDir,
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      IMAGES_GITIGNORE_BYTES,
+    );
+    expect(readFileSync(legacyGitignoreBackupPath(workDir))).toEqual(
+      existingBackup,
+    );
+  });
+
+  it('keeps an invalid existing backup and legacy gitignore unchanged', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-invalid-backup');
+    const invalidBackup = Buffer.from('# unrelated backup\n');
+    writeOpencodeGitignore(workDir, LEGACY_GITIGNORE_BYTES);
+    writeFileSync(legacyGitignoreBackupPath(workDir), invalidBackup);
+    const logs: string[] = [];
+
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'hello' }])],
+      workDir,
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: (message) => logs.push(message),
+    });
+
+    expect(readFileSync(gitignorePath(workDir))).toEqual(
+      LEGACY_GITIGNORE_BYTES,
+    );
+    expect(readFileSync(legacyGitignoreBackupPath(workDir))).toEqual(
+      invalidBackup,
+    );
+    expect(
+      logs.some((message) => message.includes('backup is not an exact')),
+    ).toBe(true);
+  });
+
+  it('keeps a hard-linked backup and legacy gitignore unchanged', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-hardlink-backup');
+    const gitignore = writeOpencodeGitignore(workDir, LEGACY_GITIGNORE_BYTES);
+    const backup = legacyGitignoreBackupPath(workDir);
+    linkSync(gitignore, backup);
+
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'hello' }])],
+      workDir,
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    expect(readFileSync(gitignore)).toEqual(LEGACY_GITIGNORE_BYTES);
+    expect(readFileSync(backup)).toEqual(LEGACY_GITIGNORE_BYTES);
+  });
+
+  it('preserves custom gitignore with wildcard/comment byte-for-byte on migration', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-custom-preserve');
+    // Contains `*` but is not the exact legacy plugin content.
+    const custom = Buffer.from(
+      '# keep local secrets\n*.local\n!important.local\n',
+    );
+    writeOpencodeGitignore(workDir, custom);
+
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'no images' }])],
+      workDir,
+      imageRouting: 'direct',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    expect(readFileSync(gitignorePath(workDir))).toEqual(custom);
+    expect(existsSync(legacyGitignoreBackupPath(workDir))).toBe(false);
+  });
+
+  it('appends images/ exactly once to custom gitignore when saving images', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-custom-append');
+    const custom = Buffer.from('# project rules\n*.tmp');
+    writeOpencodeGitignore(workDir, custom);
+
+    const run = () =>
+      processImageAttachments({
+        messages: [makeUserMsg([IMG])],
+        workDir,
+        imageRouting: 'auto',
+        disabledAgents: new Set<string>(),
+        log: () => {},
+      });
+
+    run();
+    const afterFirst = readFileSync(gitignorePath(workDir));
+    expect(afterFirst).toEqual(
+      Buffer.concat([custom, Buffer.from('\n'), IMAGES_GITIGNORE_BYTES]),
+    );
+
+    run();
+    const afterSecond = readFileSync(gitignorePath(workDir));
+    expect(afterSecond).toEqual(afterFirst);
+    expect(
+      afterSecond
+        .toString('utf8')
+        .split(/\r?\n/)
+        .filter((l) => l === 'images/'),
+    ).toEqual(['images/']);
+  });
+
+  it('preserves non-UTF-8 prefix bytes when appending images/', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-binary-prefix');
+    // Invalid UTF-8 lead bytes + a comment line; must survive append intact.
+    const prefix = Buffer.from([
+      0xff, 0xfe, 0x00, 0x23, 0x20, 0x62, 0x69, 0x6e, 0x0a,
+    ]);
+    writeOpencodeGitignore(workDir, prefix);
+
+    processImageAttachments({
+      messages: [makeUserMsg([IMG])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    const after = readFileSync(gitignorePath(workDir));
+    expect(after.subarray(0, prefix.length)).toEqual(prefix);
+    expect(after.subarray(prefix.length)).toEqual(IMAGES_GITIGNORE_BYTES);
+  });
+
+  it('does not mutate external target through symlinked .gitignore', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-symlink-file');
+    const external = path.join(TEST_DIR, 'gitignore-symlink-file-external');
+    writeFileSync(external, LEGACY_GITIGNORE_BYTES);
+    mkdirSync(path.join(workDir, '.opencode'), { recursive: true });
+    symlinkSync(external, gitignorePath(workDir));
+
+    const logs: string[] = [];
+    processImageAttachments({
+      messages: [makeUserMsg([IMG])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: (msg) => logs.push(msg),
+    });
+
+    expect(readFileSync(external)).toEqual(LEGACY_GITIGNORE_BYTES);
+    expect(logs.some((m) => m.includes('symlinked'))).toBe(true);
+  });
+
+  it('symlinked .opencode refuses gitignore mutation, external images dir, and keeps attachments', () => {
+    const workDir = path.join(TEST_DIR, 'symlink-opencode-dir');
+    const externalDir = path.join(TEST_DIR, 'symlink-opencode-dir-external');
+    mkdirSync(workDir, { recursive: true });
+    mkdirSync(externalDir, { recursive: true });
+    const externalGi = path.join(externalDir, '.gitignore');
+    const externalContent = Buffer.from('# external custom\n*.bak\n');
+    writeFileSync(externalGi, externalContent);
+    symlinkSync(externalDir, path.join(workDir, '.opencode'));
+
+    const message = makeUserMsg([IMG]);
+    const logs: string[] = [];
+    const result = processImageAttachments({
+      messages: [message],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: (msg) => logs.push(msg),
+    });
+
+    expect(result).toBe(false);
+    expect(imagePartCount(message)).toBe(1);
+    expect(readFileSync(externalGi)).toEqual(externalContent);
+    expect(existsSync(path.join(externalDir, 'images'))).toBe(false);
+    expect(logs.some((m) => m.includes('symlinked'))).toBe(true);
+  });
+
+  it('symlinked images dir does not delete external expired files or write images', () => {
+    const workDir = path.join(TEST_DIR, 'symlink-images-dir');
+    const externalImages = path.join(
+      TEST_DIR,
+      'symlink-images-dir-external-images',
+    );
+    mkdirSync(path.join(workDir, '.opencode'), { recursive: true });
+    mkdirSync(externalImages, { recursive: true });
+
+    const expired = path.join(externalImages, 'old-external.png');
+    writeFileSync(expired, 'external-old');
+    const past = new Date(Date.now() - 2 * 60 * 60 * 1000);
+    utimesSync(expired, past, past);
+
+    symlinkSync(externalImages, path.join(workDir, '.opencode', 'images'));
+
+    const message = makeUserMsg([IMG]);
+    const logs: string[] = [];
+    const result = processImageAttachments({
+      messages: [message],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: (msg) => logs.push(msg),
+    });
+
+    expect(result).toBe(false);
+    expect(imagePartCount(message)).toBe(1);
+    expect(existsSync(expired)).toBe(true);
+    expect(readFileSync(expired, 'utf8')).toBe('external-old');
+    // No session subdirectory or new image written into the external target.
+    expect(readdirSync(externalImages)).toEqual(['old-external.png']);
+    expect(logs.some((m) => m.includes('symlinked'))).toBe(true);
+
+    // Text-only path must also skip cleanup through the symlink.
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'later' }])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(existsSync(expired)).toBe(true);
+    expect(readdirSync(externalImages)).toEqual(['old-external.png']);
+  });
+
+  it('symlinked session directory keeps attachments and does not touch external target', () => {
+    const workDir = path.join(TEST_DIR, 'symlink-session-dir');
+    const imagesDir = path.join(workDir, '.opencode', 'images');
+    const externalSession = path.join(
+      TEST_DIR,
+      'symlink-session-dir-external-session',
+    );
+    mkdirSync(imagesDir, { recursive: true });
+    mkdirSync(externalSession, { recursive: true });
+
+    const externalMarker = path.join(externalSession, 'marker.txt');
+    writeFileSync(externalMarker, 'session-external');
+    // Session id from makeUserMsg is 's1'
+    symlinkSync(externalSession, path.join(imagesDir, 's1'));
+
+    const message = makeUserMsg([IMG]);
+    const logs: string[] = [];
+    const result = processImageAttachments({
+      messages: [message],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: (msg) => logs.push(msg),
+    });
+
+    expect(result).toBe(false);
+    expect(imagePartCount(message)).toBe(1);
+    expect(readFileSync(externalMarker, 'utf8')).toBe('session-external');
+    expect(readdirSync(externalSession)).toEqual(['marker.txt']);
+    expect(lstatSync(path.join(imagesDir, 's1')).isSymbolicLink()).toBe(true);
+    expect(logs.some((m) => m.includes('symlinked session'))).toBe(true);
+
+    // Cleanup must not traverse the session symlink either.
+    processImageAttachments({
+      messages: [makeUserMsg([{ type: 'text', text: 'later' }])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+    expect(readdirSync(externalSession)).toEqual(['marker.txt']);
+  });
+
+  it('symlinked candidate filename advances to local suffix without writing through', () => {
+    const workDir = path.join(TEST_DIR, 'symlink-candidate-file');
+    const sessionDir = path.join(workDir, '.opencode', 'images', 's1');
+    const externalFile = path.join(
+      TEST_DIR,
+      'symlink-candidate-file-external.bin',
+    );
+    mkdirSync(sessionDir, { recursive: true });
+    writeFileSync(externalFile, 'do-not-overwrite');
+    const candidateLink = path.join(sessionDir, IMG_CONTENT_NAME);
+    symlinkSync(externalFile, candidateLink);
+
+    const message = makeUserMsg([IMG]);
+    processImageAttachments({
+      messages: [message],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    // External target bytes must remain unchanged.
+    expect(readFileSync(externalFile, 'utf8')).toBe('do-not-overwrite');
+    expect(lstatSync(candidateLink).isSymbolicLink()).toBe(true);
+
+    // Saved to a non-symlink collision name under the real session dir.
+    const suffixed = path.join(sessionDir, `image-${IMG_HASH}-1.png`);
+    expect(existsSync(suffixed)).toBe(true);
+    expect(lstatSync(suffixed).isSymbolicLink()).toBe(false);
+    expect(readFileSync(suffixed)).toEqual(IMG_BYTES);
+    // Attachment replaced with observer nudge pointing at the local path.
+    expect(imagePartCount(message)).toBe(0);
+    const text = message.parts.find((p) => p.type === 'text')?.text ?? '';
+    expect(text).toContain(suffixed);
+  });
+
+  it('git check-ignore treats images/ as ignored under .opencode', () => {
+    const workDir = path.join(TEST_DIR, 'gitignore-check-ignore');
+    mkdirSync(workDir, { recursive: true });
+
+    const init = spawnSync('git', ['init'], {
+      cwd: workDir,
+      encoding: 'utf8',
+    });
+    if (init.status !== 0) {
+      throw new Error(
+        `git init failed (status=${init.status}): ${init.stderr}`,
+      );
+    }
+
+    processImageAttachments({
+      messages: [makeUserMsg([IMG])],
+      workDir,
+      imageRouting: 'auto',
+      disabledAgents: new Set<string>(),
+      log: () => {},
+    });
+
+    const nestedImage = path.join(
+      workDir,
+      '.opencode',
+      'images',
+      's1',
+      'probe.png',
+    );
+    mkdirSync(path.dirname(nestedImage), { recursive: true });
+    writeFileSync(nestedImage, 'x');
+
+    const configPath = path.join(
+      workDir,
+      '.opencode',
+      'oh-my-opencode-slim.json',
+    );
+    writeFileSync(configPath, '{}');
+
+    const ignored = spawnSync(
+      'git',
+      ['check-ignore', '-q', path.relative(workDir, nestedImage)],
+      { cwd: workDir, encoding: 'utf8' },
+    );
+    expect(ignored.status).toBe(0);
+
+    const configIgnored = spawnSync(
+      'git',
+      ['check-ignore', '-q', path.relative(workDir, configPath)],
+      { cwd: workDir, encoding: 'utf8' },
+    );
+    // config must NOT be ignored (exit 1 = not ignored)
+    expect(configIgnored.status).toBe(1);
+  });
+
   it('resolves omitted image routing to auto and intercepts for Observer', () => {
     const message = makeUserMsg([IMG]);
     processImageAttachments({

+ 222 - 7
src/hooks/image-hook.ts

@@ -1,8 +1,11 @@
 import { createHash } from 'node:crypto';
 import {
+  appendFileSync,
   existsSync,
+  lstatSync,
   mkdirSync,
   readdirSync,
+  readFileSync,
   rmdirSync,
   statSync,
   unlinkSync,
@@ -16,6 +19,175 @@ import { isUserMessageWithParts, type MessageWithParts } from './types';
 const lastCleanupByDir = new Map<string, number>();
 const CLEANUP_INTERVAL = 10 * 60 * 1000; // 10 minutes
 
+/** Exact bytes previously written by this plugin for `.opencode/.gitignore`. */
+const LEGACY_OPENCODE_GITIGNORE_BYTES = Buffer.from('*\n');
+const LEGACY_OPENCODE_GITIGNORE_BACKUP =
+  '.gitignore.oh-my-opencode-slim-legacy';
+/** Correct scoped rule: ignore only the images directory under `.opencode/`. */
+const IMAGES_GITIGNORE_RULE = 'images/';
+const IMAGES_GITIGNORE_BYTES = Buffer.from(`${IMAGES_GITIGNORE_RULE}\n`);
+
+function opencodeDirPath(workDir: string): string {
+  return join(workDir, '.opencode');
+}
+
+function opencodeGitignorePath(workDir: string): string {
+  return join(opencodeDirPath(workDir), '.gitignore');
+}
+
+function legacyOpencodeGitignoreBackupPath(workDir: string): string {
+  return join(opencodeDirPath(workDir), LEGACY_OPENCODE_GITIGNORE_BACKUP);
+}
+
+function hasExactLegacyOpencodeGitignoreBackup(
+  gitignorePath: string,
+  backupPath: string,
+  raw: Buffer,
+): boolean {
+  try {
+    const source = statSync(gitignorePath);
+    const backup = lstatSync(backupPath);
+    return (
+      backup.isFile() &&
+      (backup.dev !== source.dev || backup.ino !== source.ino) &&
+      readFileSync(backupPath).equals(raw)
+    );
+  } catch {
+    return false;
+  }
+}
+
+function pathIsSymlink(target: string): boolean {
+  try {
+    return lstatSync(target).isSymbolicLink();
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Refuse to create/overwrite/append `.opencode/.gitignore` when the ignore file
+ * or its `.opencode` parent is a symlink (would mutate an external target).
+ */
+function isUnsafeOpencodeGitignorePath(workDir: string): boolean {
+  return (
+    pathIsSymlink(opencodeDirPath(workDir)) ||
+    pathIsSymlink(opencodeGitignorePath(workDir))
+  );
+}
+
+function imagesDirPath(workDir: string): string {
+  return join(opencodeDirPath(workDir), 'images');
+}
+
+/**
+ * Refuse mkdir/cleanup/writes when `.opencode` or `.opencode/images` is a
+ * symlink (would create, delete, or write through an external target).
+ */
+function isUnsafeImageSavePath(workDir: string): boolean {
+  return (
+    pathIsSymlink(opencodeDirPath(workDir)) ||
+    pathIsSymlink(imagesDirPath(workDir))
+  );
+}
+
+function gitignoreHasExactRule(content: string, rule: string): boolean {
+  return content.split(/\r?\n/).includes(rule);
+}
+
+/**
+ * Migrate only the exact legacy plugin-generated `.opencode/.gitignore` (`*\n`)
+ * to `images/\n`. Custom contents (comments, other rules, even ones containing
+ * `*`) are left untouched. Symlinked targets are refused.
+ */
+function migrateLegacyOpencodeGitignore(
+  workDir: string,
+  logFn: (msg: string) => void,
+): void {
+  const gitignorePath = opencodeGitignorePath(workDir);
+  const backupPath = legacyOpencodeGitignoreBackupPath(workDir);
+  try {
+    if (!existsSync(gitignorePath) && !pathIsSymlink(gitignorePath)) return;
+    if (isUnsafeOpencodeGitignorePath(workDir)) {
+      logFn('[image-hook] refusing to migrate symlinked .opencode/.gitignore');
+      return;
+    }
+    const raw = readFileSync(gitignorePath);
+    if (!raw.equals(LEGACY_OPENCODE_GITIGNORE_BYTES)) return;
+
+    let createdBackup = false;
+    if (!existsSync(backupPath)) {
+      try {
+        writeFileSync(backupPath, raw, { flag: 'wx' });
+        createdBackup = true;
+      } catch (e) {
+        if (
+          !(e instanceof Error) ||
+          (e as NodeJS.ErrnoException).code !== 'EEXIST'
+        ) {
+          logFn(`[image-hook] failed to back up legacy .gitignore: ${e}`);
+          return;
+        }
+      }
+    }
+
+    if (
+      !createdBackup &&
+      !hasExactLegacyOpencodeGitignoreBackup(gitignorePath, backupPath, raw)
+    ) {
+      logFn(
+        '[image-hook] refusing to migrate legacy .gitignore: backup is not an exact regular file',
+      );
+      return;
+    }
+
+    writeFileSync(gitignorePath, IMAGES_GITIGNORE_BYTES);
+    logFn(
+      `[image-hook] migrated legacy .gitignore; ${
+        createdBackup ? 'backup created at' : 'using existing backup at'
+      } ${backupPath}`,
+    );
+  } catch (e) {
+    logFn(`[image-hook] failed to migrate .gitignore: ${e}`);
+  }
+}
+
+/**
+ * Ensure `.opencode/.gitignore` ignores the images directory when auto mode
+ * actually saves images. Creates the file when absent; appends `images/` once
+ * to custom contents that lack that exact rule, preserving existing bytes.
+ * Symlinked targets are refused.
+ */
+function ensureImagesGitignore(
+  workDir: string,
+  logFn: (msg: string) => void,
+): void {
+  const gitignorePath = opencodeGitignorePath(workDir);
+  try {
+    if (isUnsafeOpencodeGitignorePath(workDir)) {
+      logFn('[image-hook] refusing to update symlinked .opencode/.gitignore');
+      return;
+    }
+
+    if (!existsSync(gitignorePath)) {
+      writeFileSync(gitignorePath, IMAGES_GITIGNORE_BYTES);
+      return;
+    }
+
+    const raw = readFileSync(gitignorePath);
+    const text = raw.toString('utf8');
+    if (gitignoreHasExactRule(text, IMAGES_GITIGNORE_RULE)) return;
+
+    const needsNewline = raw.length > 0 && raw[raw.length - 1] !== 0x0a;
+    const suffix = needsNewline
+      ? Buffer.from(`\n${IMAGES_GITIGNORE_RULE}\n`)
+      : IMAGES_GITIGNORE_BYTES;
+    appendFileSync(gitignorePath, suffix);
+  } catch (e) {
+    logFn(`[image-hook] failed to update .gitignore: ${e}`);
+  }
+}
+
 // Track how many user messages we've already checked for images per directory.
 // Without this, the observer-disabled guard re-checks ALL messages on every
 // transform. Once an image is sent, it stays in the messages array forever,
@@ -85,6 +257,8 @@ function cleanupAllSessions(saveDir: string): void {
   try {
     for (const entry of readdirSync(saveDir, { withFileTypes: true })) {
       const fp = join(saveDir, entry.name);
+      // Never traverse or delete through symlinks (session dirs or files).
+      if (entry.isSymbolicLink() || pathIsSymlink(fp)) continue;
       if (entry.isDirectory()) {
         dirsToScan.push(fp);
       } else {
@@ -100,12 +274,17 @@ function cleanupAllSessions(saveDir: string): void {
   }
 
   for (const dir of dirsToScan) {
+    if (pathIsSymlink(dir)) continue;
     try {
       let isEmpty = true;
       let allRemoved = true;
       for (const f of readdirSync(dir)) {
         isEmpty = false;
         const fp = join(dir, f);
+        if (pathIsSymlink(fp)) {
+          allRemoved = false;
+          continue;
+        }
         try {
           if (now - statSync(fp).mtimeMs > maxAge) {
             unlinkSync(fp);
@@ -140,13 +319,23 @@ function writeUniqueFile(
   const ext = extname(name);
   const base = basename(name, ext) || name;
   let candidate = join(dir, name);
-  if (existsSync(candidate)) {
-    return candidate;
-  }
   let counter = 0;
 
   const MAX_ATTEMPTS = 1000;
   for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
+    // Never treat a symlink as an already-saved image and never write through it.
+    // Advance to the next collision name instead.
+    if (pathIsSymlink(candidate)) {
+      counter += 1;
+      candidate = join(dir, `${base}-${counter}${ext}`);
+      continue;
+    }
+
+    // Existing regular file at this content-addressed name: reuse path.
+    if (existsSync(candidate)) {
+      return candidate;
+    }
+
     try {
       writeFileSync(candidate, data, { flag: 'wx' });
       return candidate;
@@ -180,6 +369,10 @@ export function processImageAttachments(args: {
 }): boolean {
   const { messages, workDir, imageRouting, disabledAgents, log } = args;
 
+  // Repair legacy plugin-generated ignore rules before any early return so
+  // direct/disabled/text-only paths still upgrade existing workspaces.
+  migrateLegacyOpencodeGitignore(workDir, log);
+
   // direct mode: never intercept attachments; the orchestrator handles them
   // inline. @observer remains available for manual delegation.
   if (imageRouting === 'direct') {
@@ -244,21 +437,34 @@ export function processImageAttachments(args: {
 
   // Save images inside the project's .opencode/images/ directory.
   // This is within the workspace so the read tool won't require extra permissions.
-  const saveDir = join(workDir, '.opencode', 'images');
+  const saveDir = imagesDirPath(workDir);
 
   if (messagesWithImages.length === 0) {
-    if (existsSync(saveDir)) cleanupAllSessions(saveDir);
+    // Never walk/delete through a symlinked .opencode or images path.
+    if (!isUnsafeImageSavePath(workDir) && existsSync(saveDir)) {
+      cleanupAllSessions(saveDir);
+    }
+    return false;
+  }
+
+  if (isUnsafeImageSavePath(workDir)) {
+    log(
+      '[image-hook] refusing to create/cleanup/write via symlinked .opencode/images path',
+    );
     return false;
   }
 
-  const gitignorePath = join(workDir, '.opencode', '.gitignore');
   try {
     mkdirSync(saveDir, { recursive: true });
-    if (!existsSync(gitignorePath)) writeFileSync(gitignorePath, '*\n');
   } catch (e) {
     log(`[image-hook] failed to create image directory: ${e}`);
   }
 
+  // Only the images directory is ignored. A bare '*' (legacy plugin output)
+  // ignores all of `.opencode/` — including project config
+  // (.opencode/oh-my-opencode-slim.json) and prompt overrides.
+  ensureImagesGitignore(workDir, log);
+
   cleanupAllSessions(saveDir);
 
   for (const { msg, imageParts } of messagesWithImages) {
@@ -266,6 +472,15 @@ export function processImageAttachments(args: {
       ? sanitizeFilename(msg.info.sessionID)
       : undefined;
     const targetDir = sessionSubdir ? join(saveDir, sessionSubdir) : saveDir;
+
+    // Refuse per-session target when it is already a symlink (external write).
+    if (pathIsSymlink(targetDir)) {
+      log(
+        `[image-hook] refusing to write via symlinked session image directory: ${targetDir}`,
+      );
+      continue;
+    }
+
     try {
       mkdirSync(targetDir, { recursive: true });
     } catch (e) {

+ 57 - 0
src/utils/compat.test.ts

@@ -0,0 +1,57 @@
+import { afterAll, describe, expect, it } from 'bun:test';
+import { mkdirSync, readFileSync, rmSync } from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { runInNewContext } from 'node:vm';
+import { crossWrite } from './compat';
+
+const TEST_DIR = path.join(os.tmpdir(), `compat-test-${process.pid}`);
+
+afterAll(() => {
+  rmSync(TEST_DIR, { recursive: true, force: true });
+});
+
+function testFile(name: string): string {
+  const dir = path.join(TEST_DIR, name);
+  mkdirSync(dir, { recursive: true });
+  return path.join(dir, 'out.bin');
+}
+
+describe('crossWrite', () => {
+  it('writes string data as utf-8 bytes', async () => {
+    const filePath = testFile('string');
+    await crossWrite(filePath, 'hello');
+    expect(readFileSync(filePath)).toEqual(Buffer.from('hello'));
+  });
+
+  it('writes Buffer slices without parent-buffer bytes', async () => {
+    const filePath = testFile('buffer-slice');
+    const parent = Buffer.from([0xaa, 0x01, 0x02, 0x03, 0xbb]);
+    const slice = parent.subarray(1, 4);
+    expect(Buffer.isBuffer(slice)).toBe(true);
+    await crossWrite(filePath, slice);
+    expect(readFileSync(filePath)).toEqual(Buffer.from([0x01, 0x02, 0x03]));
+  });
+
+  it('writes same-realm ArrayBuffer contents', async () => {
+    const filePath = testFile('arraybuffer');
+    const ab = new ArrayBuffer(3);
+    new Uint8Array(ab).set([0x10, 0x20, 0x30]);
+    await crossWrite(filePath, ab);
+    expect(readFileSync(filePath)).toEqual(Buffer.from([0x10, 0x20, 0x30]));
+  });
+
+  it('writes cross-realm ArrayBuffer that fails instanceof ArrayBuffer', async () => {
+    const filePath = testFile('cross-realm-ab');
+
+    const crossRealm = runInNewContext(
+      'const b = new ArrayBuffer(2); new Uint8Array(b).set([0x7e, 0x7f]); b',
+    ) as ArrayBuffer;
+
+    // Prerequisite: node:vm must yield a true cross-realm buffer.
+    expect(crossRealm instanceof ArrayBuffer).toBe(false);
+
+    await crossWrite(filePath, crossRealm);
+    expect(readFileSync(filePath)).toEqual(Buffer.from([0x7e, 0x7f]));
+  });
+});

+ 14 - 1
src/utils/compat.ts

@@ -80,10 +80,23 @@ export function crossSpawn(
 
 /**
  * Cross-runtime file write that works in both Bun and Node.js.
+ *
+ * Order matters: Buffer is checked before treating the remainder as
+ * ArrayBuffer so Buffer slices are written as-is (no parent-buffer copy).
+ * Remaining union member is treated as ArrayBuffer without `instanceof`,
+ * which fails for cross-realm ArrayBuffers.
  */
 export async function crossWrite(
   path: string,
   data: ArrayBuffer | Buffer | string,
 ): Promise<void> {
-  await fsWriteFile(path, Buffer.from(data as ArrayBuffer));
+  if (typeof data === 'string') {
+    await fsWriteFile(path, Buffer.from(data));
+    return;
+  }
+  if (Buffer.isBuffer(data)) {
+    await fsWriteFile(path, data);
+    return;
+  }
+  await fsWriteFile(path, Buffer.from(data));
 }