Skip to content

fix(test): kill process group on teardown to prevent stale web-client leak (#918 fix) - #1231

Closed
bassilkhilo-ag2 wants to merge 1 commit into
sonichi:mainfrom
bassilkhilo-ag2:fix/agent-state-test-stale-server-918
Closed

bassilkhilo-ag2 wants to merge 1 commit into
sonichi:mainfrom
bassilkhilo-ag2:fix/agent-state-test-stale-server-918

Conversation

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator

Root cause

npx tsx spawns a multi-process chain: npm → tsx → node. The previous teardown sent SIGTERM/SIGKILL only to the direct child (npm exec), leaving the actual tsx/node web-client processes alive — still holding the test port (18081).

On the next test run:

  • before() polls the port → gets 200 from the stale server instead of the newly spawned one
  • Stale server has a deleted temp workspace → core-status.json ENOENT → stale: true → tmux scrape sees sutando-core active → effectiveAgentState() returns 'working' for every request
  • All state-checking tests fail with actual: 'working', regardless of track resets in beforeEach

This explains the 6 consistent failures that appeared to be unrelated to recent changes — they only fail when a previous test run's teardown leaked the server process.

Fix

  • Spawn with detached: true → creates a new process group for the entire npm→tsx→node subtree
  • Teardown uses process.kill(-child.pid!, signal) (negative PID = send to process group) so all descendants die together
  • Guard updated to child?.pid for null safety

Test

  • All 9 subtests pass
  • Port 18081 is verified CLOSED after each run (no leak)
  • Two consecutive runs confirmed clean (previously: second run always failed)

See also

  • Stale process symptom: {"state":"working","label":"Bash"} regardless of /mute-state?state=idle resets
  • Root signal: tmux scrape returning working when workspace temp dir is deleted

🤖 Generated with Claude Code

@VasiliyRad VasiliyRad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eighth PR this session with the same 4-file bundle (build_log.md, health-check.py, health-check-bridge-multi-pid.test.py, sync-memory-memory-dir.test.sh). Real change is the 16/-6 in tests/agent-state-endpoint.test.ts. Strip the bundle; the base branch your PRs are forked from still carries those un-pushed pass-207 changes. Pattern flagged on #1196/#1194/#1225/#1226/#1227/#1228/#1229; #1230 was closed appropriately after you saw the dup-src feedback — please apply the same fix here.

@bassilkhilo-ag2
bassilkhilo-ag2 force-pushed the fix/agent-state-test-stale-server-918 branch from 380d1de to d7e42fe Compare May 27, 2026 01:40
@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main — PR now contains exactly 1 commit touching only tests/agent-state-endpoint.test.ts (+16/-6). The 4 base-branch files (build_log.md, health-check.py, etc.) are gone.

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Branch is now clean — only tests/agent-state-endpoint.test.ts in the diff. The 4-file bundle has been rebased out. Could you take another look?

…ver leaks

`npx tsx` spawns a multi-process chain: npm → tsx → node. The previous
teardown sent SIGTERM/SIGKILL only to the direct child (npm), leaving
the actual tsx/node web-client processes alive with the port bound.

On the next test run, the `before()` hook polls the port and immediately
gets 200 from the stale server instead of the newly spawned one. The stale
server has a deleted temp workspace → core-status.json ENOENT → stale:true
→ tmux scrape sees sutando-core active → effectiveAgentState() returns
'working' for every test regardless of track state.

Fix: spawn with `detached: true` (new process group) and kill `-child.pid`
(the group) instead of `child.pid` in teardown, so all descendants die
together. Also updates the `after()` guard to `child?.pid` for safety.

Resolves the 6 consistent test failures that were introduced when
the test's previous run left stale PID processes holding port 18081.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bassilkhilo-ag2
bassilkhilo-ag2 force-pushed the fix/agent-state-test-stale-server-918 branch from d7e42fe to 05fc05c Compare May 27, 2026 03:39
@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Rebased on clean origin/main — bundled files stripped, single-commit diff. Re-requesting review.

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator Author

Closing per owner instruction — autonomous PR creation halted.

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.

2 participants