Skip to content

fix: eliminate 10s container shutdown delay#1373

Merged
Mossaka merged 1 commit intomainfrom
fix/eliminate-shutdown-delay
Mar 19, 2026
Merged

fix: eliminate 10s container shutdown delay#1373
Mossaka merged 1 commit intomainfrom
fix/eliminate-shutdown-delay

Conversation

@Mossaka
Copy link
Collaborator

@Mossaka Mossaka commented Mar 19, 2026

Summary

Fixes #1371

Three root causes contributed to a ~10-second delay during docker compose down:

  • api-proxy shell-form CMD: The Dockerfile used CMD node server.js 2>&1 | tee ... (shell form), so node ran under /bin/sh as a child process. /bin/sh does not forward SIGTERM to children, causing Docker to wait the full stop timeout before sending SIGKILL. Switched to exec form CMD ["node", "server.js"] so node is PID 1 and receives SIGTERM directly.

  • Squid shutdown_lifetime: Squid's default shutdown_lifetime (30s) causes it to wait for active connections to drain on SIGTERM. Added shutdown_lifetime 0 seconds since this is an ephemeral per-session proxy with no need for connection draining.

  • Docker Compose stop timeout: Docker Compose defaults to a 10-second stop timeout before sending SIGKILL. Added stop_grace_period: 2s to squid and api-proxy services, which is sufficient now that both containers shut down promptly on SIGTERM.

Test plan

  • npm run build compiles successfully
  • npm test — all 1119 tests pass, including new tests for shutdown_lifetime and stop_grace_period
  • npm run lint — no errors (only pre-existing warnings)
  • Integration: verify docker compose down completes in ~2s instead of ~10s

🤖 Generated with Claude Code

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

github-actions bot commented Mar 19, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 86.11% 86.23% 📈 +0.12%
Statements 86.03% 86.15% 📈 +0.12%
Functions 86.13% 86.13% ➡️ +0.00%
Branches 79.31% 79.38% 📈 +0.07%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 87.7% → 88.1% (+0.48%) 87.0% → 87.5% (+0.47%)

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

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.

Pull request overview

Reduces docker compose down latency by ensuring the api-proxy and squid containers terminate promptly on SIGTERM, avoiding the default 10s Compose stop timeout behavior.

Changes:

  • Switch containers/api-proxy to an exec-form CMD so Node receives SIGTERM directly.
  • Configure Squid with shutdown_lifetime 0 seconds to avoid waiting on connection draining during shutdown.
  • Set stop_grace_period: 2s for squid and api-proxy in the generated Compose config, and add tests for both settings.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/squid-config.ts Adds shutdown_lifetime 0 seconds to the generated squid.conf to speed termination.
src/squid-config.test.ts Asserts the generated config includes the new shutdown directive.
src/docker-manager.ts Adds stop_grace_period: '2s' to squid and api-proxy service definitions.
src/docker-manager.test.ts Verifies stop_grace_period is set on both services in generated compose output.
containers/api-proxy/Dockerfile Replaces shell-form pipeline CMD with exec-form Node CMD to fix signal handling.

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

Comment on lines +33 to +34
# Use exec form so node is PID 1 and receives SIGTERM directly
CMD ["node", "server.js"]
@github-actions github-actions bot mentioned this pull request Mar 19, 2026
@github-actions
Copy link
Contributor

Smoke Test Results

GitHub MCP: PR #1370 "[WIP] Fix the failing GitHub Actions workflow for test coverage report", PR #1334 "[Test Coverage] test: improve pid-tracker branch coverage for edge cases"
Playwright: Page title contains "GitHub"
File Write: /tmp/gh-aw/agent/smoke-test-claude-23311291737.txt created
Bash: File contents verified

Overall: PASS

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

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

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

@github-actions
Copy link
Contributor

Smoke Test Results — run #23311291777

✅ GitHub MCP — Last 2 merged PRs: #1370 "[WIP] Fix the failing GitHub Actions workflow for test coverage report", #1340 "docs: update architecture docs with three-component overview"
✅ Playwright — github.com title contains "GitHub"
✅ File write — /tmp/gh-aw/agent/smoke-test-copilot-23311291777.txt created and verified
✅ Bash — file content confirmed via cat

Overall: PASS

PR author: @Mossaka | No assignees

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

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Three root causes of the 10-second shutdown delay:

1. api-proxy Dockerfile used shell-form CMD (node runs under /bin/sh,
   which doesn't forward SIGTERM to the child process). Switched to
   exec form so node is PID 1 and handles signals directly.

2. Squid's default shutdown_lifetime (30s) causes it to wait for active
   connections to drain. Added shutdown_lifetime 0 since this is an
   ephemeral proxy with no need for connection draining.

3. Docker Compose default stop timeout is 10s. Added stop_grace_period
   of 2s to squid and api-proxy services since they now shut down
   promptly on SIGTERM.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Mossaka Mossaka force-pushed the fix/eliminate-shutdown-delay branch from c645bb0 to 4a42a0f Compare March 19, 2026 23:07
@github-actions
Copy link
Contributor

Smoke test results for run 23321225306@Mossaka

Test Result
GitHub MCP (last 2 merged PRs: #1374, #1372)
Playwright (github.com title check)
File write + read
Bash tool

Overall: PASS

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

@github-actions
Copy link
Contributor

Smoke Test Results — PASS

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

@github-actions
Copy link
Contributor

🔮 Oracle smoke trace for PR #1373

  • PR titles: "fix: update vulnerable dependencies (flatted, markdownlint-cli2)", "feat: support base path prefix for OpenAI and Anthropic API targets"
  • GitHub MCP (last 2 merged PRs): ✅
  • safeinputs-gh PR query: ❌ (safeinputs-gh not found)
  • Playwright github.com title contains "GitHub": ✅
  • Tavily search: ❌ (Tavily tool unavailable)
  • File write + cat check: ✅
  • Discussion query/comment test: ❌ (github-discussion-query unavailable; single safe-output-call policy)
  • npm ci && npm run build: ✅
  • Overall status: FAIL

🔮 The oracle has spoken through Smoke Codex

Warning

⚠️ Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • ab.chatgpt.com
  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"
    - "registry.npmjs.org"

See Network Configuration for more information.

@github-actions
Copy link
Contributor

Chroot Version Comparison Results

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

Result: 1/3 runtimes match — Go matches, but Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot for issue #1373

@github-actions
Copy link
Contributor

🏗️ 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 passed ✅ PASS
Go env passed ✅ PASS
Go uuid 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 #1373 ·

@Mossaka Mossaka merged commit cc21c11 into main Mar 19, 2026
60 checks passed
@Mossaka Mossaka deleted the fix/eliminate-shutdown-delay branch March 19, 2026 23:41
@github-actions
Copy link
Contributor

Smoke Test Results — Claude (run 23342821518)

✅ GitHub MCP: PR #1374 "fix: update vulnerable dependencies (flatted, markdownlint-cli2)" / PR #1373 "fix: eliminate 10s container shutdown delay"
✅ Playwright: github.com title contains "GitHub"
✅ File write: /tmp/gh-aw/agent/smoke-test-claude-23342821518.txt created
✅ Bash verify: file contents confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude

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.

fix: eliminate 10s container shutdown delay (api-proxy shell-form CMD + squid shutdown_lifetime)

2 participants