Skip to content

fix: speed up firewall shutdown by ~10s#1150

Merged
Mossaka merged 2 commits intomainfrom
fix/fast-shutdown
Mar 13, 2026
Merged

fix: speed up firewall shutdown by ~10s#1150
Mossaka merged 2 commits intomainfrom
fix/fast-shutdown

Conversation

@Mossaka
Copy link
Collaborator

@Mossaka Mossaka commented Mar 5, 2026

Summary

Fixes #1103. The awf-api-proxy container takes ~10s to stop because its Dockerfile uses shell form CMD, making /bin/sh PID 1 instead of Node.js. The shell doesn't forward SIGTERM, so Docker waits its 10s grace period before SIGKILL.

  • Fix root cause: Change api-proxy Dockerfile from shell form to exec form so Node.js is PID 1 and handles SIGTERM directly
  • Add --skip-cleanup flag: Skip all cleanup (containers, iptables, work dir) in CI where the runner terminates anyway, saving additional ~10s

Test plan

  • npm run build compiles successfully
  • npm test — all 821 tests pass
  • npm run lint — 0 errors
  • Local test: sudo awf --build-local --allow-domains github.com -- curl https://api.github.com — verify fast shutdown
  • Local test: sudo awf --skip-cleanup --build-local --allow-domains github.com -- curl https://api.github.com — verify no cleanup happens

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 5, 2026 18:24
@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

⚠️ Coverage Regression Detected

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

Overall Coverage

Metric Base PR Delta
Lines 82.03% 82.06% 📈 +0.03%
Statements 82.01% 82.03% 📈 +0.02%
Functions 82.50% 82.50% ➡️ +0.00%
Branches 74.20% 74.15% 📉 -0.05%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 46.6% → 46.3% (-0.36%) 47.0% → 46.7% (-0.36%)
src/docker-manager.ts 83.1% → 83.7% (+0.56%) 82.4% → 83.0% (+0.54%)

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Node.js Build Test Results ✅

Project Install Tests Status
clsx PASS PASS
execa PASS PASS
p-limit PASS PASS

Overall: PASS

Generated by Build Test Node.js for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Bun Build Test Results

Project Install Tests Status
elysia 1/1 PASS
hono 1/1 PASS

Overall: ✅ PASS

All tests passed with Bun v1.3.10.

Generated by Build Test Bun for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Smoke Test Results

  • ✅ GitHub MCP: #1003 chore(deps): bump the all-npm-dependencies group | #1078 fix: add explicit execute directive to smoke-codex
  • ✅ Playwright: GitHub page title verified ("GitHub · Change is constant...")
  • ✅ File Write: /tmp/gh-aw/agent/smoke-test-claude-22730613836.txt created
  • ✅ Bash: File content verified

Overall: PASS

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

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

🦀 Rust Build Test Results

Project Build Tests Status
fd 1/1 PASS
zoxide 1/1 PASS

Overall: ✅ PASS

Generated by Build Test Rust for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

🦕 Deno Build Test Results

Project Tests Status
oak 1/1 ✅ PASS
std 1/1 ✅ PASS

Overall: ✅ PASS

Test output details

oak:

running 1 test from ./test.ts
oak test ... ok (0ms)
ok | 1 passed | 0 failed (2ms)
```

**std:**
```
running 1 test from ./test.ts
std test ... ok (0ms)
ok | 1 passed | 0 failed (2ms)

Generated by Build Test Deno for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Smoke test results (run 22730613841)

✅ GitHub MCP — Last 2 merged PRs: #1078 fix: add explicit execute directive to smoke-codex to prevent noop, #1069 fix(deps): resolve high-severity rollup vulnerability in docs-site
✅ Playwright — github.com title contains "GitHub"
✅ File write — /tmp/gh-aw/agent/smoke-test-copilot-22730613841.txt created and verified
✅ Bash — cat confirmed file content

Overall: PASS | PR by @Mossaka, no assignees

📰 BREAKING: Report filed by Smoke Copilot for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

.NET Build Test Results

Project Restore Build Run Status
hello-world PASS
json-parse PASS

Overall: PASS

Run output

hello-world: Hello, World!

json-parse:

{
  "Name": "AWF Test",
  "Version": 1,
  "Success": true
}
Name: AWF Test, Success: True

Generated by Build Test .NET for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Merged PRs: fix: add explicit execute directive to smoke-codex to prevent noop | fix(deps): resolve high-severity rollup vulnerability in docs-site
GitHub MCP last-2 merged PRs: ✅
safeinputs-gh pr list: ✅
Playwright title contains GitHub: ✅
Tavily search: ❌ (tool unavailable)
File write: ✅
Cat verify: ✅
Discussion comment: ✅
Build npm ci && npm run build: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Result: FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

C++ Build Test Results

Project CMake Build Status
fmt PASS
json PASS

Overall: PASS 🎉

Generated by Build Test C++ for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Go Build Test Results

Project Download Tests Status
color PASS ✅ PASS
env PASS ✅ PASS
uuid PASS ✅ PASS

Overall: ✅ PASS

Generated by Build Test Go for issue #1150

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Java Build Test Results ☕

Project Compile Tests Status
gson 1/1 PASS
caffeine 1/1 PASS

Overall: PASS 🎉

Both Java projects compiled and all tests passed successfully through the AWF firewall proxy.

Generated by Build Test Java for issue #1150

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

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

Overall Coverage

Metric Base PR Delta
Lines 82.37% 82.39% 📈 +0.02%
Statements 82.27% 82.30% 📈 +0.03%
Functions 82.60% 82.60% ➡️ +0.00%
Branches 74.21% 74.17% 📉 -0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 46.6% → 46.3% (-0.36%) 47.0% → 46.7% (-0.36%)
src/docker-manager.ts 83.4% → 84.0% (+0.54%) 82.8% → 83.3% (+0.52%)

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

@github-actions
Copy link
Contributor

Go Build Test Results

Project Download Tests Status
color PASS ✅ PASS
env PASS ✅ PASS
uuid PASS ✅ PASS

Overall: ✅ PASS

Generated by Build Test Go for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results

✅ GitHub MCP: #1151 fix(ci): resolve integration test suite failures on main, #1159 fix(security): eliminate TOCTOU race conditions in ssl-bump.ts
✅ Playwright: GitHub page title verified
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-22929876672.txt created
✅ Bash verify: File contents confirmed

Overall: PASS

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

@github-actions
Copy link
Contributor

Smoke test results:
Merged PRs: fix(ci): resolve integration test suite failures on main | fix(security): eliminate TOCTOU race conditions in ssl-bump.ts
Safeinputs PRs: chore(deps): bump the all-docs-site-dependencies group across 1 directory with 5 updates | [Deps] Safe dependency updates 2026-03-10
Playwright title contains GitHub: ✅
Tavily search: ❌ (missing tool)
File write+cat: ✅
Build (npm ci && npm run build): ✅
Discussion comment: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results — PASS ✅

Test Result
GitHub MCP (last 2 merged PRs) #1160 fix(squid): block direct IP connections... by @Mossaka; #1157 feat: combine all build-test workflows... by @Copilot
Playwright (github.com title contains "GitHub")
File writing
Bash tool

Overall: PASS | PR author: @Mossaka

📰 BREAKING: Report filed by Smoke Copilot for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results

Overall: PASS

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

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1150

@github-actions

This comment has been minimized.

@Mossaka Mossaka force-pushed the fix/fast-shutdown branch from 9835ca5 to 94eb3d9 Compare March 12, 2026 20:08
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

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

Overall Coverage

Metric Base PR Delta
Lines 83.46% 83.49% 📈 +0.03%
Statements 83.45% 83.47% 📈 +0.02%
Functions 83.64% 83.64% ➡️ +0.00%
Branches 76.15% 76.11% 📉 -0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 54.3% → 54.0% (-0.35%) 54.8% → 54.4% (-0.35%)
src/docker-manager.ts 85.1% → 85.6% (+0.53%) 84.4% → 84.9% (+0.52%)

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

@github-actions
Copy link
Contributor

Smoke Test Results

GitHub MCP: Last 2 merged PRs: #1262 "test: add logger/aggregator tests for blocked domain detection" · #1261 "ci: skip CI when only release.yml changes"
Playwright: github.com title contains "GitHub"
File Write: /tmp/gh-aw/agent/smoke-test-copilot-23021568001.txt created
Bash Tool: File verified via cat

Overall: PASS | PR by @Mossaka

📰 BREAKING: Report filed by Smoke Copilot for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results — PASS

✅ GitHub MCP: #1262 test: add logger/aggregator tests for blocked domain detection, #1256 chore(deps): bump devalue from 5.6.3 to 5.6.4 in /docs-site
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-23021567980.txt created
✅ Bash verify: file contents confirmed

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

@github-actions
Copy link
Contributor

Smoke test results:
Merged PRs: test: add logger/aggregator tests for blocked domain detection | feat(cli): organize help text with logical option groups

  1. GitHub MCP merged PRs: ✅
  2. safeinputs-gh pr list: ✅
  3. Playwright title: ✅
  4. Tavily search: ❌
  5. File write: ✅
  6. Bash read: ✅
  7. Discussion query+comment: ✅
  8. Build: ✅
    Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1150

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ 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 #1150

@github-actions

This comment has been minimized.

The api-proxy container used shell form CMD, making /bin/sh PID 1 instead
of Node.js. The shell doesn't forward SIGTERM, forcing Docker to wait its
10s grace period before SIGKILL. Switch to exec form so Node.js handles
SIGTERM directly.

Also add --skip-cleanup flag to skip all cleanup in CI environments where
the runner terminates anyway, saving additional shutdown time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Mossaka Mossaka force-pushed the fix/fast-shutdown branch from 94eb3d9 to be47e43 Compare March 12, 2026 23:15
@github-actions
Copy link
Contributor

⚠️ Coverage Regression Detected

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

Overall Coverage

Metric Base PR Delta
Lines 84.23% 84.25% 📈 +0.02%
Statements 84.21% 84.23% 📈 +0.02%
Functions 84.37% 84.37% ➡️ +0.00%
Branches 77.09% 77.04% 📉 -0.05%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 56.0% → 55.7% (-0.34%) 56.6% → 56.2% (-0.34%)
src/docker-manager.ts 86.8% → 87.3% (+0.52%) 86.1% → 86.6% (+0.50%)

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

@github-actions
Copy link
Contributor

Smoke Test Results

  • ✅ GitHub MCP: Last 2 merged PRs retrieved (feat(proxy): add GitHub Enterprise Cloud/Server support / feat(cli): add predownload command)
  • ✅ Playwright: GitHub page title contains "GitHub"
  • ✅ File Write: /tmp/gh-aw/agent/smoke-test-claude-23028383732.txt created
  • ✅ Bash: File content verified

Overall: PASS

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

@github-actions
Copy link
Contributor

Smoke test results for @Mossaka's PR (no assignees)

✅ GitHub MCP — Last 2 merged PRs: fix: drop -f from curl to avoid GitHub API rate-limit flakiness (#1267), fix: add missing formatItem and program imports in cli.test.ts (#1265)
✅ Playwright — github.com title contains "GitHub"
✅ File write — /tmp/gh-aw/agent/smoke-test-copilot-23028383762.txt created and verified
✅ Bash — cat confirmed file content

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results (2026-03-12)
GitHub MCP merged PRs: feat(proxy): add GitHub Enterprise Cloud/Server support with automatic endpoint detection; fix: drop -f from curl to avoid GitHub API rate-limit flakiness
Safeinputs GH PR list: fix: add missing formatItem and program imports in cli.test.ts; fix(cli): fix secure_getenv() bypass of one-shot token protection
Playwright title contains GitHub: ✅
Tavily search: ❌ (tool unavailable)
File write: ✅
Bash cat: ✅
Discussion comment: ✅
Build npm ci && npm run build: ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1150

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.14.0 v20.20.1
Go go1.22.12 go1.22.12

Result: FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1150

@Mossaka Mossaka enabled auto-merge (squash) March 13, 2026 00:12
Cover the --skip-cleanup CLI option parsing and add tests for the
previously untested hasRateLimitOptions function to maintain coverage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 84.23% 84.40% 📈 +0.17%
Statements 84.21% 84.38% 📈 +0.17%
Functions 84.37% 84.82% 📈 +0.45%
Branches 77.09% 77.49% 📈 +0.40%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/cli.ts 56.0% → 56.3% (+0.21%) 56.6% → 56.8% (+0.20%)
src/docker-manager.ts 86.8% → 87.3% (+0.52%) 86.1% → 86.6% (+0.50%)

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

@github-actions
Copy link
Contributor

🏗️ Build Test Suite Results

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

Overall: 0/8 ecosystems passed — ❌ FAIL

❌ Error Details

All 8 ecosystems failed at the clone step. The gh CLI is not authenticated — GH_TOKEN environment variable is not set in this workflow run.

Error for all clones:

gh: To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable. Example:
  env:
    GH_TOKEN: ${{ github.token }}

All repositories (gh-aw-firewall-test-bun, gh-aw-firewall-test-cpp, gh-aw-firewall-test-deno, gh-aw-firewall-test-dotnet, gh-aw-firewall-test-go, gh-aw-firewall-test-java, gh-aw-firewall-test-node, gh-aw-firewall-test-rust) could not be cloned.

Generated by Build Test Suite for issue #1150 ·

@github-actions
Copy link
Contributor

🤖 Smoke test results for run 23030123204:

Test Result
GitHub MCP (last 2 merged PRs) #1267 "fix: drop -f from curl to avoid GitHub API rate-limit flakiness", #1265 "fix: add missing formatItem and program imports in cli.test.ts"
Playwright (github.com title) ✅ Title contains "GitHub"
File writing /tmp/gh-aw/agent/smoke-test-copilot-23030123204.txt created
Bash tool ✅ File verified via cat

Overall: PASS@Mossaka

📰 BREAKING: Report filed by Smoke Copilot for issue #1150

@github-actions
Copy link
Contributor

Smoke Test Results

Overall: PASS

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

@github-actions
Copy link
Contributor

Merged PRs: feat(proxy): add GitHub Enterprise Cloud/Server support with automatic endpoint detection | fix: drop -f from curl to avoid GitHub API rate-limit flakiness
GitHub MCP ✅; safeinputs-gh ✅; Playwright ✅; Tavily ❌
File write ✅; bash cat ✅; discussion comment ✅; build ✅
Overall: FAIL

🔮 The oracle has spoken through Smoke Codex for issue #1150

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.12 Python 3.12.3 ❌ NO
Node.js v24.14.0 v20.20.0 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1150

@Mossaka Mossaka merged commit 9b55519 into main Mar 13, 2026
57 checks passed
@Mossaka Mossaka deleted the fix/fast-shutdown branch March 13, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

firewall process takes 10sec to shutdown

2 participants