callstack / callstack/agent-device
fix(host-kit): a killed command settles on exit, background exec cannot take a timeout
- Dominant language
- TypeScript
- Stars
- 4.6k
- Forks
- 299
- Avg merge
- 10h 17m
- Merged PRs (30d)
- 515
Description
## Defects in `packages/host-kit/src/internal/exec.ts` (821 lines, verified at `eefe37b51e`)
1. **A killed command settles on `close`, not `exit`.** `runSpawnedCommand` resolves from `child.on('close')` (`:196`); `killProcessTree` (`:795-802`) sends a process-group kill only when `detached` was set, otherwise `child.kill('SIGKILL')` on the direct child. A descendant holding the inherited stdio pipe keeps `close` from firing and wedges the request and its device lock. `execHostAdb` (`src/platform-runtime-android-adb-host.ts:105-108`) does not pass `detached` while `execSerialAdb` (`:89-92`) does; adb's fork-server is the concrete grandchild.
2. **`runCmdBackground` ignores `timeoutMs`.** `ExecBackgroundOptions = ExecOptions` (`:59`) but the implementation (`:380-443`) never reads it. No production caller sets a real value today, but `spawnSerialAdb` spreads `AndroidAdbSpawnOptions` (which includes `timeoutMs`) straight into it (`src/platform-runtime-android-adb-host.ts:97-101`), and the persistent snapshot-helper session rides that path. Honoring the field would be one plumbing change away from killing the helper session. Make it unrepresentable: `ExecBackgroundOptions = Omit` and drop the dead forwarding in `src/platform-runtime-app-log-process.ts:177` and `src/platform-runtime-app-log-android-transport.ts:32`.
3. **Background output buffers unbounded by default** (`captureOutput ?? true`, `:399`). The keep-hot `xcodebuild` runner (`runner-process-launch.ts:27-56`) retains it for the session lifetime while `runner-io.ts` already tees the same bytes to `runner.log`. **Do not add a default tail cap**: `buildRunnerEarlyExitError` (`runner-contract.ts:402-429`) and `classifyBootFailure` (`boot-diagnostics.ts:42-66`) substring-scan the *early* output for signatures like `0xe8008012`, and `stopAppleXctraceProcess` (`perf-xctrace.ts:368-390`) reads output after SIGKILL. If a cap is added it is opt-in per caller and keeps head plus tail.
## Fix, in this order
- Settle-on-`exit` after a kill we issued, for both `runSpawnedCommand` and the background kill path; keep `close` for children we did not kill. Pass `detached` in `execHostAdb`.
- Narrow the background options type (item 2).
- Opt-in `maxCaptureBytes` (head+tail) for the runner spawn only, if at all.
Scope: `runSpawnedCommand` and `runCmdBackground`. `runCmdSync` (spawnSync) and `runCmdDetached*` (unref, no settle) stay out. `withCommandExecutorOverride` (`:79`) is a production seam (`adb-provider-scope.ts:159-163`) consulted only by `runCmd`/`runCmdStreaming`; do not widen it. No net-LOC claim. exec.ts has 179 lines of headroom before the 1,000-line ratchet.
## Characterization tests to add before changing behavior
- timeout-killed `runCmd` whose grandchild holds the stdio pipe still settles;
- `runCmdBackground` resolves full stdout for a child that writes >1 MB;
- `buildRunnerEarlyExitError` still yields `IOS_RUNNER_DEVICE_NOT_PROVISIONED` when `0xe8008012` is in the first KB of large output;
- `withCommandExecutorOverride` is not consulted by sync/background/detached;
- `spawnSerialAdb` with `timeoutMs` in options does not kill the child.
Contributor guide
Assessment
This issue has not been assessed yet.