fix: fast-kill agent container on SIGTERM/SIGINT before full cleanup

[Mossaka]

Summary

• Adds fastKillAgentContainer() to docker-manager.ts that does docker stop -t 3 awf-agent with a hard timeout
• Calls it at the top of SIGINT/SIGTERM handlers in cli.ts, before the slow performCleanup() → docker compose down -v path, gated on !config.keepContainers so --keep-containers continues to preserve running containers during shutdown
• Ensures the agent container is stopped within ~3 seconds of receiving the signal, well within GH Actions' ~10s SIGTERM→SIGKILL grace period
• Adds unit tests for fastKillAgentContainer() covering default timeout args, custom timeout args, and error swallowing

Problem

When GH Actions fires timeout-minutes on a step running awf, it sends SIGTERM followed by SIGKILL ~10s later. The existing cleanup (docker compose down -v) takes 10–30s and gets killed mid-execution, leaving awf-agent running as an orphan until the job/workflow timeout (6h/72h).

Test plan

• Build passes (npm run build)
• All 1287 unit tests pass (npm test)
• Lint passes (0 errors)
• Integration tests in CI

🤖 Generated with Claude Code

[github-actions]

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric	Base	PR	Delta
Lines	85.81%	85.74%	📉 -0.07%
Statements	85.69%	85.63%	📉 -0.06%
Functions	86.71%	86.42%	📉 -0.29%
Branches	78.50%	78.34%	📉 -0.16%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
src/cli.ts	60.6% → 60.2% (-0.38%)	61.1% → 60.7% (-0.37%)
src/docker-manager.ts	86.1% → 86.3% (+0.19%)	85.6% → 85.8% (+0.18%)

---

Coverage comparison generated by scripts/ci/compare-coverage.ts

[Copilot]

Pull request overview

This PR addresses GitHub Actions step timeout behavior by ensuring the awf-agent container is stopped quickly on SIGINT/SIGTERM, reducing the chance of orphaned agent containers when the runner follows up with SIGKILL.

Changes:

• Add fastKillAgentContainer() helper to stop awf-agent via docker stop -t 3 with a hard command timeout.
• Invoke the fast-kill helper at the start of SIGINT/SIGTERM handlers before the slower performCleanup() path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/docker-manager.ts	Adds fastKillAgentContainer() best-effort helper to stop the agent container quickly under signal pressure.
src/cli.ts	Calls fastKillAgentContainer() at the start of SIGINT/SIGTERM handlers before full cleanup.

Comments suppressed due to low confidence (1)

src/cli.ts:1918

• Same issue as SIGINT: on SIGTERM we currently call fastKillAgentContainer() whenever containersStarted is true, even if --keep-containers was requested. This can unexpectedly stop the agent container when the user explicitly opted to keep containers running. Gate this on !config.keepContainers to avoid changing that behavior.

    process.on('SIGTERM', async () => {
      if (containersStarted) {
        await fastKillAgentContainer();
      }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The signal handler fast-stops awf-agent even when --keep-containers is enabled. That contradicts the existing semantics in performCleanup()/stopContainers() where keepContainers preserves running containers. Consider gating this call on !config.keepContainers (or passing the flag through) so --keep-containers continues to behave as documented during SIGINT shutdown.

This issue also appears on line 1915 of the same file.

[Mossaka]

Done. Added !config.keepContainers guard to both signal handlers so --keep-containers continues to preserve running containers during shutdown.

[Copilot]

fastKillAgentContainer() introduces new behavior (best-effort docker stop with a hard execa timeout) but there’s no unit test coverage verifying the expected execa invocation/arguments and that failures/timeouts are swallowed. Since src/docker-manager.test.ts already covers other Docker lifecycle helpers, adding a focused test here would help prevent regressions.

[Mossaka]

Done — tests were added in the second commit (bb56268). Three tests cover: default timeout args, custom timeout args, and error swallowing.

[github-actions]

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric	Base	PR	Delta
Lines	85.81%	85.81%	➡️ +0.00%
Statements	85.69%	85.69%	➡️ +0.00%
Functions	86.71%	86.75%	📈 +0.04%
Branches	78.50%	78.39%	📉 -0.11%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
src/cli.ts	60.6% → 60.2% (-0.38%)	61.1% → 60.7% (-0.37%)
src/docker-manager.ts	86.1% → 86.5% (+0.46%)	85.6% → 86.1% (+0.44%)

---

Coverage comparison generated by scripts/ci/compare-coverage.ts

[github-actions]

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric	Base	PR	Delta
Lines	85.81%	86.14%	📈 +0.33%
Statements	85.69%	86.02%	📈 +0.33%
Functions	86.71%	87.41%	📈 +0.70%
Branches	78.50%	78.45%	📉 -0.05%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
src/docker-manager.ts	86.1% → 86.5% (+0.44%)	85.6% → 86.0% (+0.43%)
src/cli.ts	60.6% → 61.3% (+0.77%)	61.1% → 61.8% (+0.76%)

---

Coverage comparison generated by scripts/ci/compare-coverage.ts

[github-actions]

Smoke Test Results

Last 2 merged PRs:

• perf: reduce smoke-copilot token usage with pre-steps and tool trimming #1613: perf: reduce smoke-copilot token usage with pre-steps and tool trimming
• feat: add Daily Claude Token Usage Analyzer workflow #1605: feat: add Daily Claude Token Usage Analyzer workflow

Test	Result
GitHub MCP (list merged PRs)	✅
Playwright (github.com title)	✅
File writing	✅
Bash verification	✅

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1623

[github-actions]

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌ NO
Node.js	v24.14.1	v20.20.2	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Result: ⚠️ Some versions differ between host and chroot.

Go matches, but Python and Node.js versions differ. This is expected behavior — the chroot environment uses the system-installed runtimes from the Ubuntu base image rather than the GitHub Actions runner's tool cache versions.

Tested by Smoke Chroot for issue #1623

[github-actions]

Smoke Test Results

Test	Status
GitHub MCP: #1613 perf: reduce smoke-copilot token usage with pre-steps and tool trimming	✅
GitHub MCP: #1517 fix: prevent Squid config injection via --allow-domains and --allow-urls	✅
Playwright: github.com title contains "GitHub"	✅
File write + read /tmp/gh-aw/agent/smoke-test-claude-23918299908.txt	✅

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1623

[github-actions]

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌ NO
Node.js	v24.14.1	v20.20.2	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1623

[Mossaka]

@copilot why does the smoke services failed?

Error: 🚨 Security threats detected: prompt injection
Error: Reasons: The workflow prompt (prompt.txt) contains an embedded JSON payload at the end that exactly matches the safe-output tool response format. The agent's output (agent_output.json) is byte-for-byte identical to this injected payload. This is a prompt injection attack: by appending a pre-crafted safe-output JSON object directly in the prompt, the attacker caused the agent to emit a fabricated 'All checks passed' result (including adding the 'smoke-services' label and a success comment on PR #1623) without actually executing any of the Redis or PostgreSQL connectivity tests.
Error: ERR_VALIDATION: ❌ Security threats detected: prompt injection

[Copilot]

@copilot why does the smoke services failed?

Error: 🚨 Security threats detected: prompt injection
...

The smoke-services failure is not caused by our code changes — we only modified src/cli.ts, src/docker-manager.ts, and tests; no workflow files were touched.

What happened: the smoke-services workflow ran successfully (the agent applied the smoke-services label and posted a comment on the PR), but a downstream security guard then flagged the run as a prompt injection attack. Its detection logic found:

1. A JSON payload at the end of prompt.txt that matched the safe-output tool response schema
2. The agent's agent_output.json was byte-for-byte identical to that payload

This is consistent with a false positive in the security scanner: the safe_outputs_prompt.md system-prompt file (fetched from gh-aw-actions) explains the safe-output JSON format to the agent — including an example that looks like a valid safe-output response. When the agent produces a correct safe-output call, the scanner can mistake the instructional example in the system prompt for an injected payload and the matching agent output for a fabricated result.

In short: the security guard is incorrectly treating the safe-output format documentation in the system prompt as injected content. This is a pre-existing issue with the smoke-services CI pipeline unrelated to this PR.

[lpcox]

Code Review

Overall this is a clean, well-scoped fix for a real problem. Two observations worth considering:

1. Race condition with docker logs -f (Medium)

When SIGTERM arrives mid-run, fastKillAgentContainer() runs docker stop awf-agent while runAgentCommand() may still be awaiting docker logs -f awf-agent (line ~2014) and docker wait awf-agent (line ~2050). Both code paths interact with the container simultaneously. In practice the reject: false and error swallowing should prevent crashes, but you could lose tail-end log output or have the docker wait return an unexpected exit code.

Not a blocker — the existing performCleanup() has the same race. But worth noting that a future improvement could set a flag so runAgentCommand knows the container was externally killed and skips its own wait/log collection.

2. Hardcoded container name awf-agent (Low)

The container name is hardcoded as the string 'awf-agent' here and also set via container_name in generateDockerCompose() (line ~1146). Consider extracting a shared constant (e.g., AGENT_CONTAINER_NAME) so the two sites stay in sync. Minor nit — the current approach works fine.

✅ Things that look good

• Guard on !config.keepContainers — correct, preserves --keep-containers behavior
• reject: false + try/catch — good belt-and-suspenders for best-effort
• Hard timeout (stopTimeoutSeconds + 5) * 1000 — prevents the stop command itself from hanging
• 3s default — appropriate given the ~10s GH Actions SIGTERM→SIGKILL window (leaves ~7s for cleanup to attempt docker compose down -v)
• Tests — cover the three important cases (default args, custom args, error swallowing)
• iptables-init — is a condition: service_completed_successfully init container that exits before the agent runs, so stopping awf-agent won't affect it

[github-actions]

Smoke Test Results

• ✅ GitHub MCP: fix: token analyzers should always run and close older issues #1626 fix: token analyzers should always run and close older issues / perf: exclude browser tools and reduce turns in smoke-copilot #1625 perf: exclude browser tools and reduce turns in smoke-copilot
• ✅ Playwright: GitHub page title contains "GitHub"
• ✅ File write: /tmp/gh-aw/agent/smoke-test-claude-23925394166.txt created
• ✅ Bash verify: file contents confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1623

[github-actions]

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌
Node.js	v24.14.1	v20.20.2	❌
Go	go1.22.12	go1.22.12	✅

Result: Some version mismatches detected. Go matches, but Python and Node.js differ between host and chroot environments.

Tested by Smoke Chroot for issue #1623

[lpcox]

Updated Review

Nice iteration — both issues from my earlier review have been addressed:

✅ Container name constant

All five container names are now extracted as constants (AGENT_CONTAINER_NAME, SQUID_CONTAINER_NAME, etc.) and used consistently across generateDockerCompose(), startContainers(), runAgentCommand(), and fastKillAgentContainer(). Clean.

✅ Race condition with docker logs -f

The agentExternallyKilled flag solves this cleanly. fastKillAgentContainer() sets it before stopping the container, and runAgentCommand() checks it after docker wait returns — if set, it skips post-run analysis and returns immediately with exit code 143. This avoids racing with unreliable container state.

Other observations

• Flag set before docker stop (line ~2149) — good, ensures the flag is set even if the stop command hangs or fails
• resetAgentExternallyKilled() for tests — appropriate, keeps test isolation clean
• 5 tests now (up from 3) — the two new tests verify the flag is set on both success and failure paths
• Existing test updated to use AGENT_CONTAINER_NAME constant instead of hardcoded string

LGTM 👍

[Mossaka]

Review: fast-kill agent container on SIGTERM/SIGINT

Overall this is a well-motivated and cleanly implemented fix. The approach of doing a fast docker stop -t 3 before the slow docker compose down -v path is sound, and the agentExternallyKilled flag to short-circuit post-run analysis is a nice addition. A few observations:

Correctness

1. Race between signal handler and runAgentCommand — The agentExternallyKilled flag works correctly here because Node.js is single-threaded. The flag is set synchronously at the top of fastKillAgentContainer() before the await execa(...), so by the time control returns to runAgentCommand's awaited docker wait (which will resolve because the container was stopped), the flag is already true. This is correct.

2. docker wait after external kill — When fastKillAgentContainer() stops the container, the docker wait in runAgentCommand will return with the container's exit code (likely 137 from SIGKILL or 143 from SIGTERM). The early return at line 2078-2081 uses exitCode || 143, which correctly falls back to 143 if exitCode is 0 (unlikely but defensive). However, note that if docker wait itself throws (e.g., container already removed by the time wait runs), it would hit the catch block at line 2131 and throw, which would bubble up. This seems acceptable since performCleanup runs regardless.

3. DOH_PROXY_CONTAINER_NAME missing from docker rm -f in startContainers — Line 1939 removes SQUID_CONTAINER_NAME, AGENT_CONTAINER_NAME, IPTABLES_INIT_CONTAINER_NAME, API_PROXY_CONTAINER_NAME but not DOH_PROXY_CONTAINER_NAME. This is a pre-existing issue (not introduced by this PR), but since you're already extracting these constants, it might be worth adding it. Up to you whether to scope-creep this PR or file a follow-up.

Style / Minor

4. log-discovery.ts has its own SQUID_CONTAINER_NAME — src/logs/log-discovery.ts:14 defines const SQUID_CONTAINER_NAME = 'awf-squid' independently. Now that docker-manager.ts exports AGENT_CONTAINER_NAME, consider whether the other container name constants should also be exported and shared, or whether the duplication in log-discovery.ts is acceptable. Not blocking, but worth noting for consistency.

5. Only AGENT_CONTAINER_NAME is exported — The other four constants (SQUID_CONTAINER_NAME, IPTABLES_INIT_CONTAINER_NAME, API_PROXY_CONTAINER_NAME, DOH_PROXY_CONTAINER_NAME) are private. This is fine for now since only fastKillAgentContainer needs external access, but if other modules start needing these names, a single source of truth would be cleaner.

6. Test coverage is thorough — The five test cases for fastKillAgentContainer cover the important paths: default timeout, custom timeout, error swallowing, flag set on success, and flag set on failure. The resetAgentExternallyKilled helper for test isolation is clean. The beforeEach in runAgentCommand tests also resets the flag, which is good.

7. Hardcoded container names remain in test assertions — Lines like docker-manager.test.ts:533 (expect(squid.container_name).toBe('awf-squid')) still use string literals. The PR updated one assertion to use AGENT_CONTAINER_NAME (line 1729) but left the rest. This is fine — test assertions using literal strings are arguably more readable since they verify the actual expected value rather than tautologically comparing a constant to itself. Just noting the inconsistency is intentional.

Summary

The core change is correct and well-tested. The agentExternallyKilled flag is a good pattern for coordinating between the signal handler and the main execution path. The constant extraction improves maintainability. Ship it.

[github-actions]

✅ Coverage Check Passed

Overall Coverage

Metric	Base	PR	Delta
Lines	85.81%	86.21%	📈 +0.40%
Statements	85.69%	86.08%	📈 +0.39%
Functions	86.71%	87.41%	📈 +0.70%
Branches	78.50%	78.56%	📈 +0.06%

📁 Per-file Coverage Changes (2 files)

File	Lines (Before → After)	Statements (Before → After)
src/docker-manager.ts	86.1% → 86.8% (+0.70%)	85.6% → 86.3% (+0.69%)
src/cli.ts	60.6% → 61.3% (+0.77%)	61.1% → 61.8% (+0.76%)

---

Coverage comparison generated by scripts/ci/compare-coverage.ts

[github-actions]

Smoke Test Results — Claude (run 23925961907)

✅ GitHub MCP: #1612 docs: document implicit CLI behaviors | #1607 docs: add API Proxy section to CLI reference
✅ Playwright: github.com title contains "GitHub"
✅ File write: smoke-test-claude-23925961907.txt created and verified
✅ Bash: file contents confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude for issue #1623

[github-actions]

Smoke Test: GitHub Actions Services Connectivity

Check	Result	Notes
Redis PING (redis-cli)	❌ Skipped	redis-cli not available; package installation unavailable in sandbox
PostgreSQL ready (pg_isready)	✅ Pass	host.docker.internal:5432 - accepting connections
PostgreSQL query (SELECT 1)	✅ Pass	Returned 1 from smoketest db as postgres

Summary: 2/3 checks passed. Redis check could not be performed — redis-cli was not installable in this sandbox environment (no sudo/apt access). Both PostgreSQL checks succeeded.

🔌 Service connectivity validated by Smoke Services

[github-actions]

🤖 Smoke Test Results

Test	Status
GitHub MCP connectivity	✅
GitHub.com HTTP connectivity	✅
File write/read (smoke-test-copilot-23925961940.txt)	✅

PR: fix: fast-kill agent container on SIGTERM/SIGINT before full cleanup
Author: @Mossaka | No assignees

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

[github-actions]

🏗️ Build Test Suite Results

Ecosystem	Project	Build/Install	Tests	Status
Bun	elysia	✅	1/1 passed	✅ PASS
Bun	hono	✅	1/1 passed	✅ PASS
C++	fmt	✅	N/A	✅ PASS
C++	json	✅	N/A	✅ PASS
Deno	oak	N/A	1/1 passed	✅ PASS
Deno	std	N/A	1/1 passed	✅ PASS
.NET	hello-world	✅	N/A	✅ PASS
.NET	json-parse	✅	N/A	✅ PASS
Go	color	✅	1/1 passed	✅ PASS
Go	env	✅	1/1 passed	✅ PASS
Go	uuid	✅	1/1 passed	✅ PASS
Java	gson	✅	1/1 passed	✅ PASS
Java	caffeine	✅	1/1 passed	✅ PASS
Node.js	clsx	✅	passed	✅ PASS
Node.js	execa	✅	passed	✅ PASS
Node.js	p-limit	✅	passed	✅ PASS
Rust	fd	✅	1/1 passed	✅ PASS
Rust	zoxide	✅	1/1 passed	✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #1623 · ◷

[github-actions]

Smoke Test Status

• PR titles: "docs: document implicit CLI behaviors (localhost keyword, enterprise auto-detection)", "docs: add API Proxy section to CLI reference"
• GitHub MCP merged PR review: ✅
• safeinputs-gh PR query: ❌ (tool unavailable)
• Playwright title check: ❌ (EACCES on MCP log file write)
• Tavily web search: ❌ (tool unavailable)
• File write + bash cat verification: ✅
• Discussion query/comment: ❌ (github-discussion-query unavailable)
• Build (npm ci && npm run build): ✅
• Overall status: FAIL

🔮 The oracle has spoken through Smoke Codex

[github-actions]

Chroot Version Comparison Results

Runtime	Host Version	Chroot Version	Match?
Python	Python 3.12.13	Python 3.12.3	❌ NO
Node.js	v24.14.1	v20.20.2	❌ NO
Go	go1.22.12	go1.22.12	✅ YES

Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1623