From cdceede33db3148495cd9df285f1a37d785dd2e4 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 24 Aug 2026 18:00:52 +0200 Subject: [PATCH] fix(deepseek): atomic shim write, honest attribution comment, name-fallback profile classifier The three smaller review nits, plus the first real test coverage for the status shim (it had none: it is emitted as a STRING, so tsc never sees it). 1. The shim was written with a plain writeFileSync. The TUI can be exec'ing that exact path while an upgraded Codeman refreshes it, and a reader catching a half-written file gets a syntax error, exits non-zero, and is retried four times per state change for a file that will never parse. Now temp + rename (atomic within the directory), with the temp chmod'ed before the rename since writeFileSync's mode only applies on create, and removed if the write throws. SHIM_VERSION bumped to 2, because SHIM_SOURCE changed and an existing v1 shim would otherwise keep matching the embedded marker and never be refreshed. 2. The pane-id comment claimed the ambient env "cannot be spoofed by an argument the agent itself could influence". The agent runs IN that pane and can invoke the shim with CODEMAN_SESSION_ID unset and any argv it likes. It buys nothing it did not already have (the hook-secret file is readable from the same pane, so it can POST /api/hook-event directly), but the comment read like a security boundary. Rewritten to say what the preference actually buys: correct attribution when a TUI mangles or re-uses the pane argument. Accidents, not adversaries. 3. classifyProfile() folded the directory name into the same haystack as the bundles, but only the TUI arm could match a bare name, so a stock profile whose package.json has no dsh.profile.bundles (hand-edited, older layout, mid-install) classified as `unknown` -> launchable -> eligible as the DEFAULT pick, which is exactly the pane-dies-on-arrival failure the two-part availability gate exists to prevent. The stock names are now a LAST-resort fallback consulted after the bundle patterns, so real bundle evidence still wins over a name the user chose. The loose `tui` arm gained word boundaries: it decides which profile boots by default, and matching the middle of `intuition` is not a rule anyone could predict. New test/deepseek-status-shim.test.ts runs the generated script the way the harness does -- real node process, real argv, real env, real listener -- and covers the exit-code contract that makes the retry behaviour safe: mapped states post and exit 0, an unknown verb or unmapped state exits 0 WITHOUT posting (a non-zero there would be four HTTP requests per state change forever), a rejecting server or an unreachable one exits non-zero so the caller retries, the hook secret is read at execution time, and `node --check` parses the file (a template-literal typo in SHIM_SOURCE is invisible to tsc). Trap worth recording, hit while writing it: the tests must spawn the shim ASYNCHRONOUSLY. The listener lives in the test process, so spawnSync blocks the event loop that has to accept the connection, the shim waits out its own 1500ms socket timeout and exits 1, and it reads exactly like a broken shim (measured: Socket._onTimeout in its --trace-exit output, server logging nothing). Verified: full gate green (6142 passed, +10), typecheck/lint/format clean. --- docs/architecture-invariants.md | 2 +- src/deepseek-status-shim.ts | 34 ++++- src/utils/deepseek-cli-resolver.ts | 32 ++++- test/deepseek-cli-resolver.test.ts | 45 +++++++ test/deepseek-status-shim.test.ts | 207 +++++++++++++++++++++++++++++ 5 files changed, 312 insertions(+), 8 deletions(-) create mode 100644 test/deepseek-status-shim.test.ts diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index fcebbbcc..6fbb5dbe 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -34,7 +34,7 @@ Implementation detail extracted from `CLAUDE.md` so that file stays small enough ⚠️ **The resolver needs the strictest identity probe of any CLI**, because `dsh` is not merely a squattable npm name: Debian ships an unrelated `dsh` (dancer's shell, `apt install dsh`) that would answer a version probe convincingly. `probeDeepSeekVersion()` therefore checks `dsh --help` against `DEEPSEEK_IDENTITY_REGEX` (`DeepSeek Harness`) FIRST and only then reads a version, and `test/deepseek-cli-resolver.test.ts` pins both the rejection and the VITEST hermeticity gate with a real executable fixture. `DEEPSEEK_VERSION_REGEX` keeps the prerelease tail (`0.1.1-rc.2`), since truncating it would report an rc as a release; it is shared with the `dsh` dependency-registry entry so doctor and run mode agree about the version even though the resolver is stricter about identity. -Model is NOT a session field: it is a composition entry in the profile's config tree (`agent-default-model`), configured in `~/.dsh/settings.yaml` + `cordis.patch.yml`, so both create paths deliberately resolve no model for this mode. Env allowlist: `DSH_*` + `DEEPSEEK_*`; provider keys named by a settings-file `apiKeyEnv` stay OUT, which is pi's 34-provider-key problem in a new shape and gets the same answer. Docker seeds `~/.dsh` per-file (`.env`, `settings.yaml`, `cordis.patch.yml`) and the image installs its OWN profile, because `profiles/` is a per-profile `node_modules` tree — host-arch-specific and far too large to copy per container start. Stays OUT of `isAltScreenStripMode()` (third-party fullscreen TUI — the opencode case). Availability via `GET /api/deepseek/status`, the widest per-CLI status shape (`available`/`runnable`/`path`/`version`/`dshHome`/`defaultProfile`/`profiles`); `POST /api/deepseek/install-profile` bootstraps a profile and is the only endpoint in Codeman that installs third-party code — regex-confined specifier, argv-array spawn, privileged grant required in multi-user mode, and the held-open request is bounded by a HAND-ROLLED timeout over a `detached: true` process group (negative-pid SIGTERM→SIGKILL, as `runGit()` does in git-clone.ts). ⚠️ Node's own `spawn` `timeout` is NOT enough: a plugin install fans out into package-manager children, the built-in timeout signals only the direct child, and the survivors hold the inherited stdio pipes open so `close` never fires and the request leaks forever. User guide: `docs/deepseek-integration.md`. Tests: `test/deepseek-mode.test.ts`, `test/deepseek-cli-resolver.test.ts`. +Model is NOT a session field: it is a composition entry in the profile's config tree (`agent-default-model`), configured in `~/.dsh/settings.yaml` + `cordis.patch.yml`, so both create paths deliberately resolve no model for this mode. Env allowlist: `DSH_*` + `DEEPSEEK_*`; provider keys named by a settings-file `apiKeyEnv` stay OUT, which is pi's 34-provider-key problem in a new shape and gets the same answer. Docker seeds `~/.dsh` per-file (`.env`, `settings.yaml`, `cordis.patch.yml`) and the image installs its OWN profile, because `profiles/` is a per-profile `node_modules` tree — host-arch-specific and far too large to copy per container start. Stays OUT of `isAltScreenStripMode()` (third-party fullscreen TUI — the opencode case). ⚠️ `classifyProfile()` reads the profile's BUNDLES, and "unknown means launchable" is deliberate (anyone can publish an app bundle), but it has one knowably-wrong case: `readProfile()` returns an empty bundle list for a `package.json` with no `dsh.profile.bundles`, which made the SHIPPED `web`/`headless` profiles look third-party and launchable. The directory name is therefore consulted as a LAST resort (`STOCK_NON_INTERACTIVE_PROFILES`), after the bundle patterns, so real bundle evidence always wins over a name the user chose. The loose `tui` arm carries word boundaries for the same reason: it decides which profile boots by default, and matching the middle of `intuition` is not a rule anyone could predict. ⚠️ The generated shim is written **temp + rename**, not in place: the TUI can be exec'ing that exact path while an upgraded Codeman refreshes it, and a half-written file is a syntax error the caller then retries four times per state change forever. Bump `SHIM_VERSION` whenever `SHIM_SOURCE` changes, or an existing shim keeps matching the embedded marker and is never refreshed. Availability via `GET /api/deepseek/status`, the widest per-CLI status shape (`available`/`runnable`/`path`/`version`/`dshHome`/`defaultProfile`/`profiles`); `POST /api/deepseek/install-profile` bootstraps a profile and is the only endpoint in Codeman that installs third-party code — regex-confined specifier, argv-array spawn, privileged grant required in multi-user mode, and the held-open request is bounded by a HAND-ROLLED timeout over a `detached: true` process group (negative-pid SIGTERM→SIGKILL, as `runGit()` does in git-clone.ts). ⚠️ Node's own `spawn` `timeout` is NOT enough: a plugin install fans out into package-manager children, the built-in timeout signals only the direct child, and the survivors hold the inherited stdio pipes open so `close` never fires and the request leaks forever. User guide: `docs/deepseek-integration.md`. Tests: `test/deepseek-mode.test.ts`, `test/deepseek-cli-resolver.test.ts`. **Pi specifics** (#206, `docs/pi-integration.md`): command built by `buildPiCommand()` (`--model` — the only builder whose model regex admits `:` and `/`, for `sonnet:high` and `openai/gpt-4o` — plus `--provider`, `--thinking`, `--session ` / `-c`, and the TRI-STATE `--approve`/`--no-approve`). ⚠️ **Pi has no permission prompts and no sandbox**, so there is no `--dangerously-skip-permissions` analog and Codeman must not invent one; the privilege-shaped knob is `approveProjectTrust`, which makes pi LOAD AND EXECUTE repo-local `.pi/extensions` TypeScript and npm-install missing project packages. It therefore joins `clampExternalCliBypassForOwner()`'s **materialize** branch (gemini's, not codex/antigravity's only-if-sent one): an absent config still yields `--no-approve` for a non-granted owner, because pi's own default is an interactive prompt the session user could answer themselves. ⚠️ `--api-key` is NEVER wired — it would put a provider secret on the spawn command line. ⚠️ Pi stays **out** of `isAltScreenStripMode()`: its default TUI renders into the main screen with terminal-owned scrollback (nothing to strip), and since 0.84.0 the user can flip to a fullscreen TUI at runtime via `/settings`, where the alt screen is load-bearing — being out of the list is exactly what makes that switch safe. ⚠️ Only the `PI_*` env prefix was added; pi's ~34 provider keys share no prefix and `ALLOWED_ENV_PREFIXES` is a single GLOBAL list with no mode context, so admitting them would widen the allowlist for every mode at once (a mode-aware allowlist is the tracked follow-up). ⚠️ `pi` is a short, GENERIC binary name, so unlike the sibling resolvers `pi-cli-resolver.ts` sanity-probes `pi --version` (cached, vitest-skipped) and requires semver-shaped output; `GET /api/pi/status` carries `version` on top of the sibling `{available, path}` shape so a misresolution is diagnosable. Local echo: pi lands on the `'buffer'` overlay via the fallthrough in `_updateLocalEchoState` (pinned in `test/local-echo-codex-gating.test.ts`); if pi's live composer turns out to fight it the way codex's did, the fallback is one `'off'` branch. Tests: `test/pi-mode.test.ts`, `test/routes/external-cli-bypass-clamp.test.ts` (first-ever coverage of the clamp). diff --git a/src/deepseek-status-shim.ts b/src/deepseek-status-shim.ts index 19dbfe9a..fa3b385e 100644 --- a/src/deepseek-status-shim.ts +++ b/src/deepseek-status-shim.ts @@ -44,7 +44,7 @@ * @module deepseek-status-shim */ -import { chmodSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { chmodSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; import { dirname } from 'node:path'; import { dataPath } from './config/instance.js'; @@ -54,7 +54,7 @@ import { dataPath } from './config/instance.js'; * by an older Codeman and rewrite only when needed (rather than rewriting on * every session create, or — worse — leaving a stale one in place forever). */ -const SHIM_VERSION = 1; +const SHIM_VERSION = 2; const SHIM_MARKER = `codeman-dsh-status-shim v${SHIM_VERSION}`; /** @@ -125,8 +125,13 @@ const event = STATE_TO_EVENT[String(flag('--state') ?? '')] if (!event) process.exit(0) // The pane id we hand the TUI IS the Codeman session id, but prefer the ambient -// env: it is set by the same code that set HERDR_PANE_ID and cannot be spoofed -// by an argument the agent itself could influence. +// env: it is set by the same code that set HERDR_PANE_ID, so a TUI that mangles, +// truncates or re-uses the pane argument still reports against the right session. +// NOT a security boundary, and do not read it as one: the agent runs IN this pane +// and can invoke the shim with CODEMAN_SESSION_ID unset and any argv it likes. +// That buys it nothing it did not already have, since the hook-secret file is +// readable from the same pane and any process there can POST /api/hook-event +// directly. Attribution here is about accidents, not adversaries. const sessionId = process.env.CODEMAN_SESSION_ID || argv[2] const apiUrl = process.env.CODEMAN_API_URL if (!sessionId || !apiUrl) process.exit(1) @@ -213,7 +218,26 @@ export function ensureDeepSeekStatusShim(): string | null { } if (!current.includes(SHIM_MARKER)) { mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, SHIM_SOURCE, { mode: 0o700 }); + // Temp + rename, not a plain write: the TUI can be executing this exact + // path at the moment an upgraded Codeman refreshes it (every state change + // runs it, and session create is when the rewrite happens), and a reader + // that catches a half-written file gets a syntax error, exits non-zero, + // and is retried four times per state change for a file that will never + // parse. rename(2) is atomic within the directory, so a concurrent exec + // sees either the old shim or the new one, never a truncated one. + // Same reasoning as the state-store writes; pid-suffixed so two instances + // sharing a data dir cannot collide on the temp name. + const tempPath = `${path}.${process.pid}.tmp`; + try { + writeFileSync(tempPath, SHIM_SOURCE, { mode: 0o700 }); + // The mode argument only applies when writeFileSync CREATES the file, so + // a leftover temp from a crashed run would keep its old permissions. + chmodSync(tempPath, 0o700); + renameSync(tempPath, path); + } catch (err) { + rmSync(tempPath, { force: true }); + throw err; + } } // Re-assert the mode even when the content matched: a shim that lost its // executable bit (a restored backup, a copied data dir) would make every diff --git a/src/utils/deepseek-cli-resolver.ts b/src/utils/deepseek-cli-resolver.ts index c30604b3..9868d01f 100644 --- a/src/utils/deepseek-cli-resolver.ts +++ b/src/utils/deepseek-cli-resolver.ts @@ -120,8 +120,36 @@ const HEADLESS_BUNDLE_PATTERN = /dsh-headless/i; * scoped `dsh-tui` packages from a dozen different authors compete. Anything * matching is a TUI; anything unmatched is `unknown`, which still counts as * launchable. + * + * `tui` carries word boundaries so the loose arm stays a TOKEN match: `-` and + * `/` are non-word characters, so `@someone/tui-app` and `dsh-tui` both match + * while `intuition` and `gratuitous` do not. Being wrong here is cheap (an + * unmatched profile is `unknown`, which is launchable too) but it decides which + * profile a session boots by DEFAULT, and "the one whose name happens to contain + * t-u-i" is not a rule anyone could predict. */ -const TUI_BUNDLE_PATTERN = /dsh-tui|dsh-terminal-app|tui/i; +const TUI_BUNDLE_PATTERN = /dsh-tui|dsh-terminal-app|\btui\b/i; + +/** + * The profile names DeepSeek itself ships for its non-interactive surfaces. + * + * Consulted only AFTER the bundle patterns have found nothing, and only against + * the directory name. `readProfile()` yields an empty bundle list for any + * `package.json` without a `dsh.profile.bundles` array — a hand-edited file, an + * older layout, a profile mid-install — and with no bundles to read, the stock + * `web` and `headless` profiles look exactly like an unrecognized third-party + * one and inherit its launchable-by-default treatment. That is the single + * "unknown" that is knowably wrong, and it produces precisely the + * pane-dies-on-arrival failure the two-part availability gate exists to prevent. + * + * Deliberately a fallback rather than a first check: a third-party profile that + * legitimately composes a terminal app is identified by its BUNDLES, and its + * directory name (which the user chose) must never override that evidence. + */ +const STOCK_NON_INTERACTIVE_PROFILES = new Map([ + ['web', 'web'], + ['headless', 'headless'], +]); /** Profile directory names that are not profiles. */ const NON_PROFILE_DIRS = new Set(['node_modules', '.bin', '.pnpm']); @@ -134,7 +162,7 @@ function classifyProfile(name: string, bundles: string[]): DeepSeekProfileKind { if (WEB_BUNDLE_PATTERN.test(haystack)) return 'web'; if (HEADLESS_BUNDLE_PATTERN.test(haystack)) return 'headless'; if (TUI_BUNDLE_PATTERN.test(haystack)) return 'interactive'; - return 'unknown'; + return STOCK_NON_INTERACTIVE_PROFILES.get(name.toLowerCase()) ?? 'unknown'; } /** diff --git a/test/deepseek-cli-resolver.test.ts b/test/deepseek-cli-resolver.test.ts index c458110d..83397c7c 100644 --- a/test/deepseek-cli-resolver.test.ts +++ b/test/deepseek-cli-resolver.test.ts @@ -224,6 +224,51 @@ describe('DeepSeek profile inventory', () => { expect(resolveDefaultDeepSeekProfile()).toBe('custom'); }); + it('does not treat a bundle-less stock profile as launchable', () => { + // readProfile() yields an empty bundle list for any package.json without a + // `dsh.profile.bundles` array (hand-edited, older layout, mid-install), and + // with no bundles to read the shipped web/headless profiles used to look + // exactly like an unrecognized third-party one — inheriting its + // launchable-by-default treatment and producing the pane-dies-on-arrival + // failure the two-part availability gate exists to prevent. + const bare = (name: string) => { + const dir = join(home, 'profiles', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name: `dsh-profile-${name}` })); + }; + bare('web'); + bare('headless'); + + const profiles = listDeepSeekProfiles(); + expect(profiles.map((p) => `${p.name}:${p.kind}`).sort()).toEqual(['headless:headless', 'web:web']); + expect(profiles.every((p) => !isLaunchableProfile(p))).toBe(true); + expect(resolveDefaultDeepSeekProfile()).toBeNull(); + }); + + it('lets bundle evidence beat the name fallback', () => { + // The name check is a LAST resort, so a profile the user happened to call + // `web` that really composes a terminal app is still interactive. Otherwise + // a directory name would override what the profile actually contains. + writeProfile('web', ['@deepseek-ai/dsh-base', '@someone/dsh-tui']); + const profile = listDeepSeekProfiles().find((p) => p.name === 'web')!; + expect(profile.kind).toBe('interactive'); + expect(resolveDefaultDeepSeekProfile()).toBe('web'); + }); + + it('does not read `tui` out of the middle of an unrelated word', () => { + // The loose arm is a TOKEN match: `@someone/tui-app` is a TUI, `intuition` + // is a word. Being wrong is cheap (unknown is launchable too) but it decides + // which profile boots by DEFAULT, and "its name contains t-u-i" is not a + // rule anyone could predict. + writeProfile('intuition', ['@someone/gratuitous-surface']); + expect(listDeepSeekProfiles().find((p) => p.name === 'intuition')!.kind).toBe('unknown'); + + writeProfile('mine', ['@someone/tui-app']); + expect(listDeepSeekProfiles().find((p) => p.name === 'mine')!.kind).toBe('interactive'); + // Preferred over the merely-unknown one, which is the whole point of ranking. + expect(resolveDefaultDeepSeekProfile()).toBe('mine'); + }); + it('survives a stray directory under profiles/', () => { mkdirSync(join(home, 'profiles', 'not-a-profile'), { recursive: true }); writeProfile('dsh-tui', ['@deepseek-harness-tui/dsh-tui']); diff --git a/test/deepseek-status-shim.test.ts b/test/deepseek-status-shim.test.ts new file mode 100644 index 00000000..15b51c81 --- /dev/null +++ b/test/deepseek-status-shim.test.ts @@ -0,0 +1,207 @@ +/** + * The generated DeepSeek Harness status shim. + * + * This file is the one piece of DeepSeek's wiring that is neither TypeScript we + * typecheck nor a route we can `inject()` into: it is a script emitted as a + * string, dropped in the data dir, and executed by a third-party TUI as a + * SUBPROCESS. So the assertions here run it the way the harness does — a real + * `node` process, real argv, real env, against a real listener — rather than + * inspecting the source text. + * + * The exit codes are the contract's load-bearing half: the caller retries with + * backoff on any non-zero, so "cannot ever succeed" (unknown verb, unmapped + * state) must exit 0 or one typo becomes four HTTP requests per state change, + * forever. + */ +import { describe, expect, it, beforeEach, beforeAll, afterAll } from 'vitest'; +import { execFileSync, spawn } from 'node:child_process'; +import { createServer, type Server } from 'node:http'; +import { existsSync, readdirSync, readFileSync, statSync, writeFileSync, chmodSync } from 'node:fs'; +import { dirname } from 'node:path'; +import { + ensureDeepSeekStatusShim, + deepSeekStatusShimPath, + resetDeepSeekStatusShimForTest, + DEEPSEEK_STATE_TO_HOOK_EVENT, +} from '../src/deepseek-status-shim.js'; + +const PORT = 3251; + +describe('DeepSeek status shim: provisioning', () => { + beforeEach(() => { + resetDeepSeekStatusShimForTest(); + }); + + it('writes an executable shim that node can actually parse', () => { + const path = ensureDeepSeekStatusShim(); + expect(path).toBeTruthy(); + expect(existsSync(path!)).toBe(true); + // 0700: the TUI execs it directly, so a lost exec bit means every report + // fails and is retried four times per state change. + expect(statSync(path!).mode & 0o777).toBe(0o700); + // `node --check` on the real file, because a template-literal typo in + // SHIM_SOURCE is invisible to tsc — the shim is a STRING as far as the + // compiler is concerned. + expect(() => execFileSync(process.execPath, ['--check', path!], { stdio: 'pipe' })).not.toThrow(); + }); + + it('refreshes a shim written by an older Codeman, and leaves no temp file behind', () => { + const path = deepSeekStatusShimPath(); + ensureDeepSeekStatusShim(); + const current = readFileSync(path, 'utf-8'); + + // A v1 shim from an older install: right path, stale content. + writeFileSync(path, '#!/usr/bin/env node\n// codeman-dsh-status-shim v1\nprocess.exit(0)\n', { mode: 0o700 }); + resetDeepSeekStatusShimForTest(); + ensureDeepSeekStatusShim(); + + expect(readFileSync(path, 'utf-8')).toBe(current); + // The rewrite goes through a temp + rename so a TUI exec'ing this path mid + // refresh can never read a half-written file. The temp must not survive it. + const strays = readdirSync(dirname(path)).filter((f) => f.startsWith('dsh-status-shim') && f.endsWith('.tmp')); + expect(strays).toEqual([]); + }); + + it('re-asserts the exec bit even when the content already matches', () => { + const path = ensureDeepSeekStatusShim()!; + chmodSync(path, 0o600); // a restored backup / copied data dir + resetDeepSeekStatusShimForTest(); + ensureDeepSeekStatusShim(); + expect(statSync(path).mode & 0o777).toBe(0o700); + }); +}); + +describe('DeepSeek status shim: the supervisor contract', () => { + let server: Server | undefined; + const received: Array<{ body: unknown; secret: string | undefined }> = []; + let status = 200; + + const listen = () => + new Promise((resolve) => { + server = createServer((req, res) => { + let raw = ''; + req.on('data', (c) => (raw += c)); + req.on('end', () => { + received.push({ + body: (() => { + try { + return JSON.parse(raw); + } catch { + return raw; + } + })(), + secret: req.headers['x-codeman-hook-secret'] as string | undefined, + }); + res.writeHead(status, { 'Content-Type': 'application/json' }); + res.end('{}'); + }); + }); + server.listen(PORT, '127.0.0.1', resolve); + }); + + beforeAll(() => listen()); + + afterAll(() => { + server?.close(); + }); + + /** + * Run the shim the way the TUI does, and ASYNCHRONOUSLY. + * + * Never spawnSync here: the listener above lives in this same process, so a + * synchronous spawn blocks the event loop that has to accept the connection. + * The shim then waits out its own 1500ms socket timeout and exits 1, which + * reads exactly like a broken shim (measured: `Socket._onTimeout` in its exit + * trace, and the server logging nothing). + */ + const run = (args: string[], env: Record = {}) => + new Promise<{ status: number | null; stderr: string }>((resolve) => { + const path = ensureDeepSeekStatusShim()!; + const child = spawn(process.execPath, [path, ...args], { + env: { + ...process.env, + CODEMAN_API_URL: `http://127.0.0.1:${PORT}`, + CODEMAN_SESSION_ID: 'sess-from-env', + ...env, + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stderr = ''; + child.stderr.on('data', (c: Buffer) => (stderr += c.toString('utf-8'))); + child.on('close', (status) => resolve({ status, stderr })); + }); + + // The exact command line the harness TUI runs, from the Herdr contract. + const report = (state: string, extra: string[] = []) => [ + 'pane', + 'report-agent', + 'pane-arg-id', + '--source', + 'custom:dsh-tui', + '--agent', + 'dsh-tui', + '--state', + state, + ...extra, + '--seq', + '7', + ]; + + it('forwards each harness state as its mapped hook event, and exits 0 on delivery', async () => { + resetDeepSeekStatusShimForTest(); + for (const [state, event] of Object.entries(DEEPSEEK_STATE_TO_HOOK_EVENT)) { + received.length = 0; + const out = await run(report(state, ['--message', 'needs a decision'])); + expect(out.status, `${state}: ${out.stderr}`).toBe(0); + expect(received).toHaveLength(1); + const body = received[0].body as { event: string; sessionId: string; data: Record }; + expect(body.event).toBe(event); + // The ambient env wins over the pane argument: same code set both, and the + // argument is whatever the TUI chose to pass. + expect(body.sessionId).toBe('sess-from-env'); + expect(body.data.agent).toBe('dsh-tui'); + expect(body.data.message).toBe('needs a decision'); + } + }); + + it('sends the hook secret read at EXECUTION time, so rotation needs no respawn', async () => { + resetDeepSeekStatusShimForTest(); + const secretFile = `${deepSeekStatusShimPath()}.secret-fixture`; + writeFileSync(secretFile, 'rotated-secret\n', { mode: 0o600 }); + received.length = 0; + const out = await run(report('idle'), { CODEMAN_HOOK_SECRET_FILE: secretFile }); + expect(out.status).toBe(0); + expect(received[0].secret).toBe('rotated-secret'); + }); + + it('exits 0 without posting for anything a retry could never fix', async () => { + resetDeepSeekStatusShimForTest(); + for (const args of [ + ['pane', 'list'], // unknown verb + ['something-else', 'report-agent', 'id', '--state', 'idle'], // unknown noun + [...report('rebooting')], // a state this bridge does not map + ['pane', 'report-agent', 'id'], // no --state at all + ]) { + received.length = 0; + const out = await run(args); + expect(out.status, `args ${args.join(' ')}`).toBe(0); + expect(received).toEqual([]); + } + }); + + it('exits non-zero when the post genuinely fails, so the caller retries', async () => { + resetDeepSeekStatusShimForTest(); + + // A rejecting server: transport worked, Codeman said no. + status = 500; + received.length = 0; + expect((await run(report('idle'))).status).not.toBe(0); + expect(received).toHaveLength(1); + status = 200; + + // Nothing listening at all. + expect((await run(report('idle'), { CODEMAN_API_URL: 'http://127.0.0.1:1' })).status).not.toBe(0); + // No API url to post to. + expect((await run(report('idle'), { CODEMAN_API_URL: '' })).status).not.toBe(0); + }); +});