Skip to content

[spark-compete] fix(runcommand): src/lib/server/command-runner - #855

Open
4gjnbzb4zf-sudo wants to merge 1 commit into
vibeforge1111:mainfrom
4gjnbzb4zf-sudo:spark-compete/runcommand-sigkill-grace
Open

[spark-compete] fix(runcommand): src/lib/server/command-runner#855
4gjnbzb4zf-sudo wants to merge 1 commit into
vibeforge1111:mainfrom
4gjnbzb4zf-sudo:spark-compete/runcommand-sigkill-grace

Conversation

@4gjnbzb4zf-sudo

@4gjnbzb4zf-sudo 4gjnbzb4zf-sudo commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

{
"schema": "spark-compete-hotfix-v1",
"event": "spark-compete-first-event",
"submission_mode": "public_repo_pr",
"submission_target_url": "#855",
"team": {
"name": "SparkThisUp",
"members": [
"ValHallaBuilder",
"Baz707",
"DanFireDash"
],
"github_accounts": [
"4gjnbzb4zf-sudo"
],
"llm_device_holder": "ValHallaBuilder",
"device_holder_github": "4gjnbzb4zf-sudo"
},
"target_repo": {
"id": "vibeforge1111/vibeship-spawner-ui",
"source": "https://github.com/vibeforge1111/vibeship-spawner-ui",
"owner_surface": "spawner-ui"
},
"issue": {
"type": "usage_friction",
"severity": "medium",
"title": "src/lib/server/command-runner",
"actual_behavior": "When a child ignores SIGTERM, runCommand never resolves; mission-board verify/scan stays pending forever.",
"expected_behavior": "runCommand escalates to SIGKILL after a 5s SIGTERM grace, so the promise always settles within timeoutMs + 5s and the operator gets a deterministic failure.",
"repro_steps": [
"1. Invoke a verify/scan flow whose target subprocess installs a SIGTERM handler that delays exit (or hangs in a native I/O wait).",
"2. Wait for SPAWNER_COMMAND_TIMEOUT_MS (default 30 minutes) \u2014 Node fires the spawn timeout and sends SIGTERM.",
"3. Observed: the child swallows SIGTERM and stays alive; runCommand promise never resolves; the operator sees an indefinite spinner.",
"4. Expected: a follow-up SIGKILL forces the child down after a short grace window so the operator sees a clean failure."
],
"affected_workflow": "Operator-facing flow in spawner-ui."
},
"evidence": {
"safe_links_only": true,
"before_after_proof": "Before: When a child ignores SIGTERM, runCommand never resolves; mission-board verify/scan stays pending forever.\nAfter: runCommand escalates to SIGKILL after a 5s SIGTERM grace, so the promise always settles within timeoutMs + 5s and the operator gets a deterministic failure.",
"links": [
"https://github.com//pull/855",
"https://github.com//pull/855/files"
],
"forbidden": [
"raw secrets",
"raw logs",
"raw conversations",
"private chat IDs",
"session tokens",
"cookies",
"private repo maps",
"raw memory dumps",
"full compile JSON",
"scoring details"
]
},
"proposed_fix": {
"approach": "Add a SIGTERM_GRACE_MS=5000 constant and schedule a setTimeout SIGKILL at timeoutMs + grace; clear it in both 'close' and 'error' handlers. Same shape as the spark-telegram-bot llm.ts runProcess sigkill-escalation we landed earlier.",
"files_expected": [
"src/lib/server/command-runner.ts"
],
"tests_or_smoke": "Smoke: run the affected code path in the repo and confirm before\u2192after behavior change. Build-clean: python3 -m py_compile src/lib/server/command-runner.ts or npx tsc --noEmit --skipLibCheck src/lib/server/command-runner.ts."
},
"pr": {
"url": "#855",
"branch": "spark-compete/runcommand-sigkill-grace",
"title_prefix": "[spark-compete]",
"author_github": "4gjnbzb4zf-sudo",
"body_must_include": [
"packet",
"team",
"pr_author",
"repo",
"actual_behavior",
"expected_behavior",
"repro_steps",
"before_after_proof",
"tests_or_smoke",
"duplicate_notes",
"risk_notes",
"review_claim"
]
},
"review_claim": {
"impact_claim": "medium",
"evidence_types": [
"redacted_terminal_excerpt"
],
"duplicate_notes": "Searched open PRs and issues for the same defect; this fix is targeted to src/lib/server/command-runner.ts.",
"risk_notes": "No new packages, CI workflows, or secrets-adjacent paths changed. Diff is bounded to src/lib/server/command-runner.ts. Same code paths execute on same inputs; only the documented behavior in expected_behavior changes.",
"review_state_requested": "pr_review"
}
}

@4gjnbzb4zf-sudo

Copy link
Copy Markdown
Contributor Author

TL;DR

runCommand in spawner-ui's verify/scan path leans on Node's built-in spawn timeout, which only sends SIGTERM. If the child process ignores SIGTERM (npm/test wrappers, hung worker pools), the close event never fires and the operator's mission-board card spins forever. This patch schedules a SIGKILL escalation at timeoutMs + 5s so runCommand always settles on a deterministic failure.

What I noticed

I was tracing why a verify run on the mission board can sit on the spinner long past its declared timeout. The path is /api/verify -> runCommand in src/lib/server/command-runner.ts. The spawn options pass timeout: timeoutMs, which Node implements by sending SIGTERM to the child when the timer fires. There's no follow-up SIGKILL — so if the child has a SIGTERM handler that delays or hangs (very common with npm test, vitest with hung workers, mid-compile tsc), the parent stays parked on 'close' indefinitely. The /api/verify request thread doesn't return, the mission card never flips off "running", and downstream Spark Agent Site polling never sees a terminal state.

The bug

file: vibeship-spawner-ui/src/lib/server/command-runner.ts:107
spawn(..., { timeout: timeoutMs }) only delivers SIGTERM. Children that ignore SIGTERM keep the parent's promise un-resolved past timeoutMs, with no upper bound.

The fix

Add SIGTERM_GRACE_MS = 5000. Right after the spawn(...) call, schedule a setTimeout(... timeoutMs + SIGTERM_GRACE_MS) that sends SIGKILL. Clear the timer inside both 'close' and 'error' handlers so the cleanup never lingers when the child does exit normally. ~18 line diff, single file.

Reproduction

  1. Configure a verify target that runs a script with a SIGTERM trap (e.g. trap '' TERM; sleep 99999).
  2. Lower SPAWNER_COMMAND_TIMEOUT_MS for repro (SPAWNER_COMMAND_TIMEOUT_MS=2000).
  3. Observed today: the verify request never returns; the mission card stays on the spinner past 2 s.
  4. Expected after fix: SIGKILL fires at 2s + 5s = 7s; runCommand resolves with exitCode 1; mission card flips to failed.

Verification

  • npx tsc --noEmit --skipLibCheck src/lib/server/command-runner.ts clean (matches baseline).
  • Hand-test: run a SIGTERM-ignoring child with a low SPAWNER_COMMAND_TIMEOUT_MS; before the patch the promise never resolves; after, it resolves within 5s of the SIGTERM.

Sister precedent

PR #113 in this repo accepted the same shape ("fix: add provider execution timeout") for the provider-client path. Same grace-window pattern landed in spark-telegram-bot via runprocess-sigkill-escalation for src/llm.ts.

@4gjnbzb4zf-sudo
4gjnbzb4zf-sudo force-pushed the spark-compete/runcommand-sigkill-grace branch from e54d597 to ea4637b Compare June 7, 2026 20:51
ifeoluwaaj pushed a commit to ifeoluwaaj/vibeship-spawner-ui that referenced this pull request Jun 27, 2026
Independent single-file hardening fixes:
- scheduler: in-flight Set so _tick cannot relaunch a record whose
  previous fire is still running (vibeforge1111#858)
- command-runner: SIGKILL escalation timer at timeoutMs+5s, cleared on
  close and error, so a SIGTERM-ignoring child can't hang the caller (vibeforge1111#855)
- retry-after: cap honoured Retry-After at 60s so a hostile/quota-exhausted
  upstream can't stall a mission for hours (vibeforge1111#853)
- sync-client: cap reconnect backoff at 30s and add +/-25% jitter so a
  fleet of tabs doesn't reconnect in lockstep (vibeforge1111#824)
- spark-harness-client: tolerate up to 3 transient status-poll failures
  before failing the mission (vibeforge1111#823)
- events POST: dedup caller-supplied event ids within a 5m window so a
  retried POST doesn't fan out duplicate events (vibeforge1111#851)
- brief-enricher: validate positive-numeric env overrides (vibeforge1111#852)
- h70-skill-matcher: precompute multi-word phrase keys once at module
  load instead of per task (vibeforge1111#872)
- canvas store: mirror sibling-tab writes via storage events, skipping
  while local edits are pending (vibeforge1111#859)
- MissionBoard: guard NaN dates in relative-time formatting (vibeforge1111#842)

harness_core interim_until_migration for scheduler: re-home into Governor
on migration.

Co-authored-by: 4gjnbzb4zf-sudo <4gjnbzb4zf-sudo@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant