fix(supervisor): attachments land in session working dir, not %TEMP%

[finedesignz]

Summary

term.attach_file was writing uploaded files to tmpdir()/remo-attachments/<sessionId> — outside the session's actual working directory — so a typed attachment path wasn't resolvable relative to the repo the CLI is operating on.

Locked design (panel decision):

1. Write to <repoPath>/.remo/attachments/<sessionId>/<ts-nonce>-<safeName> instead of a host temp dir. sanitizeAttachmentName() is unchanged; a short nonce prefix (Date.now()-<rand>) makes same-name re-uploads within one session non-colliding.
2. mkdirSync(..., { recursive: true, mode: 0o700 }).
3. Idempotent .gitignore guard: when <repoPath>/.git exists, ensure a literal .remo/ line is present (append if missing, create the file if absent). Skipped entirely when .git is absent — rootless/ambient/orchestrator dirs (process-manager.ts already skips the .git requirement for spec.orchestrator).
4. Best-effort cleanup: SessionBridge.stop() now rmSyncs the session's attachments dir, fail-open (never throws out of stop()).
5. Quoting bug — investigated, no fix needed. pty.write(abs + ' ') types the path as raw PTY input bytes. Traced pty_host.rs's session_input → it writes straight to the PTY master (writer.write_all(bytes)), with no shell in between. The receiving end is the CLI's own interactive prompt textbox, which reads the whole typed line as literal prompt text — there's no argv/shell tokenization to defeat, so a space in the path is just a character, not a boundary. Quoting it would instead inject literal quote characters into the prompt. Left unquoted; documented inline in session-bridge.ts so this isn't re-litigated later.

Extracted the write logic into two small exported helpers — attachmentsDirFor() and writeAttachmentFile() — so the path/collision/gitignore logic is unit-testable without spawning a real PTY runner (attach_file's PTY spawn path isn't mockable today; this was the smallest seam that gives real coverage).

Files changed

• supervisor/src/runners/session-bridge.ts
• supervisor/test/attachment-project-dir.test.ts (new)
• CLAUDE.md (documented the new attachment destination + gitignore/cleanup behavior in the PTY terminal surface docs-map row)

Test plan

• New tests: repoPath placement, not the old temp-dir scheme, filename collision (two same-name uploads don't clobber), a repoPath containing a space, gitignore appended once and only once (idempotent), gitignore skipped when .git absent, SessionBridge.stop() cleanup (including when nothing was ever written).
• bun test supervisor/test/attachment-project-dir.test.ts supervisor/test/attachment-filename-sanitize.test.ts — 12 pass, 0 fail.
• bun run check-baseline — OK, within tolerance (baseline pass=2010/skip=247/fail=0 → actual pass=2063/skip=248/fail=0; this worktree needed a fresh bun install, no drift beyond that).

Could not verify

• Full supervisor/test suite has 5 pre-existing failures unrelated to this change: bridge-permission-returnpath.test.ts (question_response assertion) reproduces identically on main before my changes; pty-byte-relay.test.ts / pty-orphan-teardown.test.ts fail in this sandbox because node-pty isn't installed / no real PTY host is available in the CI-less dev container. None touch attachment code.
• Did not exercise the live end-to-end path (browser upload → hub → supervisor → real claude/codex TUI reading the file) — no running supervisor/hub in this sandbox; covered instead by the unit tests above plus the code trace for the quoting question.

🤖 Generated with Claude Code

[finedesignz]

AI Review Gate

Gate verdict: FAILURE — blocking finding from Codex
Head SHA: e9d0b1b90b52

Claude Code (QC): pass

• [info] supervisor/src/runners/session-bridge.ts — Attachment dir git-repo detection is non-recursive: ensureAttachmentsGitignored only checks existsSync(join(repoPath, '.git')); if repoPath is a subdirectory of a larger git worktree (git root above repoPath, e.g. .git is a file pointing elsewhere, or repoPath is nested inside a checkout), this returns false and the .gitignore guard is skipped even though the directory is actually tracked by git, so uploaded attachments in .remo/attachments/ could end up as untracked-but-committable files. Likely intentional per the documented rootless/orchestrator-dir skip, but worth confirming repoPath is always expected to be a literal git root.
• [info] supervisor/src/runners/session-bridge.ts — Attachment bytes buffered twice: writeAttachmentFile() already does Buffer.from(dataB64, 'base64') internally to write the file; the caller in the term.attach_file handler recomputes the same Buffer.from(...) again just to log bytes.length. Minor redundant work, not a correctness issue.

Codex: BLOCK

• [blocking] supervisor/src/runners/session-bridge.ts — Session ID path traversal can write/delete outside attachment root: attachmentsDirFor joins the raw sessionId into a filesystem path, and stop() recursively deletes that derived path. If sessionId contains path separators or .. segments, writeAttachmentFile can place uploaded data outside /.remo/attachments and stop() can rmSync an attacker-influenced directory. Sanitize or constrain sessionId and verify the resolved path remains under the intended attachments root before writing or deleting.

Policy: both reviewers are blocking — a genuine blocking finding from either fails the gate. An infrastructure failure (quota exhausted, timeout, auth failure, no parseable output) is ADVISORY and never blocks: it means the reviewer never saw the code, which is not a verdict about the code.