git:20260711.5972d3e to git:20260827.cc701b9

---

name: subprocess-safety

audience: swarm-plugin

description: Guidelines for safe subprocess calls in opencode-swarm. Load before adding, modifying, or reviewing any file that calls spawn, spawnSync, bunSpawn, or child_process. Covers the six required properties, Windows portability, _internals DI seam pattern, and verification grep.

---

# Subprocess Safety

Read, in order:

1. `AGENTS.md` (Invariant 3: subprocesses)

2. `docs/engineering-invariants.md` (subsection 3)

3. `.agents/skills/writing-tests/SKILL.md` if tests are touched

4. `.opencode/skills/generated/mock-to-internals-migration/SKILL.md` if converting mock.module to _internals

Codex-specific execution notes:

- This skill consolidates AGENTS.md Invariant 3 into an actionable checklist.

- The canonical spawn shape and six required properties are non-negotiable per AGENTS.md.

- Violations are advisory in CI but blocking in code review.

## When to use this skill

- You are adding, modifying, or reviewing a subprocess call (`bunSpawn`, `spawn`,

`spawnSync`, `child_process.execFile`, etc.)

- You are writing or updating tests that exercise subprocess-dependent code

- A PR review flags a subprocess call missing timeout, cwd, or cleanup

## Scope

This skill applies to all files that spawn child processes:

- `src/utils/git*.ts`

- `src/hooks/*.ts`

- `src/tools/*.ts`

- `src/services/*.ts`

- `src/plugins/*.ts`

- `src/index.ts` (init-path subprocesses)

- Any test file (`tests/**`) that stubs or exercises subprocess code

## Canonical spawn shape

Every subprocess call MUST follow this pattern:

```typescript

const PER_CALL_TIMEOUT_MS = 10_000; // module-level constant (choose an appropriate value)

const proc = bunSpawn(['git', '-C', dir, 'rev-parse', '--show-toplevel'], {

stdin: 'ignore',

cwd: dir,

timeout: PER_CALL_TIMEOUT_MS,

// stdout/stderr: piped, bounded, or ignored

});

try {

const result = await proc;

// process result

} finally {

proc.kill(); // best-effort cleanup

}

```

## Six required properties

| Property | Required | Rationale |

|----------|----------|-----------|

| Array-form args | Yes | No shell-string commands (injection risk, quoting hell) |

| `cwd` or `git -C` | Yes | Never rely on inherited process.cwd() |

| `stdin: 'ignore'` | Yes | A never-closed stdin pipe under Bun/Windows can block child exit (v7.3.3) |

| `timeout: <ms>` | Yes | No subprocess is "always fast" on every platform |

| stdout/stderr bounded | Yes | Never leave piped stream unattended on long-running child |

| `proc.kill()` in `finally` | Yes | Outer withTimeout lets awaiter proceed but doesn't abort child |

## execFile callback vs execFileSync distinction

`child_process.execFile` (callback form) and `child_process.execFileSync` have different

default stdio behavior:

| API | Default stdin | Risk |

|-----|---------------|------|

| `execFileSync` | `'inherit'` | Child inherits parent stdin — **v7.3.3 vector** on Windows/Bun if stdin is never closed |

| `execFile` (callback) | `'pipe'` | Child gets an internal pipe — lower risk but still not ideal for defense-in-depth |

**Key differences from the canonical spawn pattern:**

1. **`proc.kill()` in `finally` (line 69)**: **Applicable to callback-form `execFile`.**

The function returns a `ChildProcess` reference (matching the canonical spawn

pattern per Node.js docs). The child reference enables `kill()` before that

point for timeout safety, and failing to call `proc.kill()` in `finally` can

leave orphaned children when combined with an outer `withTimeout`. The

`timeout` option triggers internal `SIGTERM`, but is not a substitute for

explicit kill in `finally` — always kill the child in `finally`.

2. **`stdin: 'ignore'` (line 66)**: Technically default-safe for callback `execFile`

(stdin is piped, not inherited). However, always add `stdio: ['ignore', 'pipe', 'pipe']`

for defense-in-depth and consistency with `execFileSync` calls. Note: Bun's

TypeScript definitions do not include `stdio` in `ExecFileOptions` — use

`execOpts as any` when passing stdio to callback-form `execFile`.

3. **`execFileSync` should always use `stdio: ['ignore', 'pipe', 'pipe']`** to

prevent the stdin-inheritance hang on Windows/Bun (v7.3.3).

## Windows-specific notes

- `.cmd` extensions: npm/bun binaries on Windows are `.cmd` wrappers. Resolve

the executable path explicitly using `which`/`where` or the project's

cross-platform helper. Do NOT enable `shell: true` or shell-mediated

execution to work around PATH resolution.

- PATH differences: `cmd.exe` and PowerShell resolve PATH differently. Test on

Windows, not just macOS/Linux.

- `child_process.spawn('bin', ...)` does not behave identically to running

under `cmd.exe`. Use array-form args and explicit `cwd`.

- `fs.renameSync` cannot overwrite existing directories on Windows. Use a

remove-then-rename pattern or `fs.rename` with error handling.

## gh CLI Subprocess Patterns

The `gh` CLI is a common subprocess in this repo (`scripts/release-notes-fragments.mjs`, CI workflows). It follows the same six required properties as all subprocesses, plus several `gh`-specific patterns.

### `gh api --paginate` requires `--slurp`

**Bug pattern (PR #1762 F-002):** `gh api --paginate` without `--slurp` produces concatenated JSON arrays on stdout. `JSON.parse()` can only parse the first array — subsequent arrays cause a parse error or are silently lost.

**Correct pattern:**

```javascript

const raw = execFileSync('gh', ['api', '--paginate', '--slurp', 'repos/.../pulls', ...], {

encoding: 'utf8',

timeout: 30_000,

maxBuffer: 16 * 1024 * 1024,

stdio: ['ignore', 'pipe', 'pipe'], // required for execFileSync (AGENTS.md §3)

});

// --slurp wraps paginated results as [[page1], [page2], ...]

const pages = JSON.parse(raw);

const allItems = pages.flat(); // flatten to single array

```

**Without `--slurp`:** stdout is `[item1, item2][item3, item4]` — invalid JSON after the first array. This is a silent data loss bug that only manifests when results span multiple pages (>30 items by default).

### `stdin: 'ignore'` for `gh` calls

`gh` subprocess calls must include `stdin: 'ignore'` (or `stdio: ['ignore', 'pipe', 'pipe']` for `execFileSync`). This is the same invariant as all subprocesses (AGENTS.md §3). For example, `scripts/release-notes-fragments.mjs` defines `ghJson()` and `ghText()` helpers using `execFileSync` — these must include `stdio: ['ignore', 'pipe', 'pipe']` per the six required properties. A PR review (pre-merge) identified this gap.

### `Number.isInteger()` for API response validation

When validating integer IDs from API responses (PR numbers, issue numbers, run IDs), use `Number.isInteger()`, not `Number.isFinite()`. `Number.isFinite()` accepts floats like `1.5`, which are never valid IDs.

```javascript

// Correct

function isValidPrNumber(n) {

return Number.isInteger(n) && n > 0;

}

// Wrong — accepts 1.5, NaN, Infinity

function isValidPrNumber(n) {

return Number.isFinite(n) && n > 0;

}

```

> **Note:** This is a stricter pattern. Some existing code uses `Number.isFinite()` after `parseInt()` — while technically safe for parsed integers, `Number.isInteger()` is the correct guard for all ID validation going forward.

### `maxBuffer` for large API responses

`gh api` can return large payloads. Set `maxBuffer: 16 * 1024 * 1024` (16 MiB) to prevent silent truncation. This is especially important for `--paginate` calls that aggregate multiple pages.

> **Note:** `maxBuffer` is specific to Node.js `child_process.execFile`/`execFileSync`. For Bun's `bunSpawn`, use the equivalent output bounding option.

## Testing pattern: `_internals` DI seam, NOT `mock.module`

`mock.module(...)` leaks across test files in Bun's shared test-runner process.

Use dependency injection instead:

```typescript

// --- source file (e.g. src/utils/gitignore-warning.ts) ---

import { bunSpawn } from './bun-compat';

export const _internals: { bunSpawn: typeof bunSpawn } = { bunSpawn };

// In production code, call _internals.bunSpawn(...) instead of bunSpawn(...)

// --- test file ---

import { _internals } from '../../src/utils/gitignore-warning';

const real = _internals.bunSpawn;

beforeEach(() => { _internals.bunSpawn = stub; });

afterEach(() => { _internals.bunSpawn = real; });

```

For the full migration protocol, load the `mock-to-internals-migration` skill.

## Verification grep

After changing any file with subprocess calls, run:

```bash

grep -n "bunSpawn\|spawn(\|spawnSync(" src/<changed>/*.ts

```

Every match MUST have all of:

1. `timeout` set to a concrete millisecond value

2. `stdin: 'ignore'` (unless intentionally interactive; note: callback-form `execFile` uses `stdio: ['ignore', 'pipe', 'pipe']` instead)

3. `cwd` or `git -C <directory>` for explicit working directory

4. `proc.kill()` in a `finally` block or equivalent cleanup path (exception: callback-form `execFile` manages cleanup internally via `timeout` option)

## Historical failures

- **v7.0.3 (#704)**: repo-graph Desktop hang -- unbounded filesystem scan on

plugin init. No timeout, no kill path. Result: "no agents in TUI/GUI" with

no error message.

- **v7.3.3 (#732)**: Git-hygiene startup regression -- `ensureSwarmGitExcluded`

called git without timeout, stdin, or kill. Result: same silent failure on

Windows.

Both caused OpenCode to silently drop the plugin manifest. Users saw no agents

and no error. Every subprocess call is a potential repeat of these failures

unless all six properties are enforced.