Skip to content

Refactor container startup-failure detection into shared helpers#4336

Merged
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-startup-failure-detection
Jun 5, 2026
Merged

Refactor container startup-failure detection into shared helpers#4336
lpcox merged 2 commits into
mainfrom
copilot/fix-duplicate-code-startup-failure-detection

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 4, 2026

container-lifecycle.ts had two duplicated startup-failure detection paths (api-proxy and squid) with near-identical error parsing and docker inspect logic. This made retry-path behavior changes easy to drift across containers.

  • What changed

    • Consolidated duplicate checks into:
      • isContainerStartupFailureError(errorMsg, containerName)
      • didContainerFailStartup(errorMsg, containerName)
    • Replaced api-proxy/squid-specific call sites in startContainers retry handling with the shared helper, preserving existing container-specific behavior and logging context via containerName.
  • Test coverage update

    • Added a focused branch test for squid’s inspect fallback (running|unhealthy) to ensure shared helper behavior applies symmetrically across both startup-critical containers.
  • Resulting call-site shape

    const firstAttemptApiProxyStartupFailure =
      await didContainerFailStartup(firstErrorMsg, API_PROXY_CONTAINER_NAME);
    
    const firstAttemptSquidStartupFailure =
      !firstAttemptApiProxyStartupFailure
      && await didContainerFailStartup(firstErrorMsg, SQUID_CONTAINER_NAME);

Copilot AI changed the title [WIP] Refactor duplicated startup-failure detection functions Refactor container startup-failure detection into shared helpers Jun 4, 2026
Copilot finished work on behalf of lpcox June 4, 2026 23:20
Copilot AI requested a review from lpcox June 4, 2026 23:20
@lpcox lpcox marked this pull request as ready for review June 4, 2026 23:46
Copilot AI review requested due to automatic review settings June 4, 2026 23:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.59% 96.64% 📈 +0.05%
Statements 96.50% 96.55% 📈 +0.05%
Functions 98.71% 98.70% ➡️ -0.01%
Branches 90.86% 91.00% 📈 +0.14%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/container-lifecycle.ts 95.5% → 95.8% (+0.25%) 95.7% → 96.0% (+0.26%)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)

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

Copy link
Copy Markdown
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

This PR refactors duplicated “container failed to start” detection logic in container-lifecycle.ts into shared helpers, ensuring api-proxy and squid startup-failure handling stays consistent (including the docker inspect fallback used on generic compose errors).

Changes:

  • Introduced shared helpers isContainerStartupFailureError(errorMsg, containerName) and didContainerFailStartup(errorMsg, containerName) to consolidate startup-failure detection and inspect-based fallback.
  • Updated startContainers retry logic to use the shared helper for both api-proxy and squid while preserving the existing “only inspect squid if api-proxy didn’t already claim failure” behavior.
  • Added a focused branch-coverage test ensuring squid’s running|unhealthy inspect output triggers the same retry behavior as api-proxy.
Show a summary per file
File Description
src/container-lifecycle.ts Replaces container-specific startup-failure detection with shared helper functions and updates retry handling call sites.
src/container-lifecycle-branches.test.ts Adds branch coverage for squid’s docker inspect unhealthy-health fallback to validate symmetric behavior.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Smoke Test: Claude Engine ✅

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read

PR: Refactor container startup-failure detection into shared helpers
Author: @Copilot · Assignees: @lpcox, @Copilot

Overall: PASS 🟢

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Smoke Test: Copilot BYOK (Direct) — PASS

Test Result
GitHub MCP connectivity
GitHub.com connectivity
File write/read /tmp/gh-aw/agent/smoke-test-copilot-byok-26985448849.txt
BYOK inference (api-proxy → api.githubcopilot.com)

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com.

Overall: PASS@Copilot (author), @lpcox @Copilot (assignees)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

🧪 Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.16.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ NOT all tests passed.

Python and Node.js versions differ between host and chroot. Go versions match.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Refactor container startup-failure detection into shared helpers
refactor(test): deduplicate TLS tunnel mock setup in websocket tests
Move tool cache mount detection into AWF and add stdin config schema support
GitHub PR query: ✅
Browser title: ✅
File write: ✅
Build: ✅
Discussion lookup/comment: ✅
Overall status: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

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

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

🏗️ 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 #4336 · sonnet46 1.3M ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Smoke Test Results

Check Result
Redis PING ❌ Timeout (no response)
PostgreSQL pg_isready ❌ No response on port 5432
PostgreSQL SELECT 1 ❌ No response (pg_isready failed)

host.docker.internal resolves to 172.17.0.1 but neither service is reachable.

Overall: FAIL — service containers appear unreachable from this runner environment.

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Smoke Test Results

  • GitHub MCP Testing: ❌ (mcpscripts command not found)
  • GitHub.com Connectivity: ❌ (SSL error 35: wrong version number)
  • File Writing Testing: ✅
  • Bash Tool Testing: ✅

Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

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

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@lpcox lpcox merged commit df79ac5 into main Jun 5, 2026
64 of 66 checks passed
@lpcox lpcox deleted the copilot/fix-duplicate-code-startup-failure-detection branch June 5, 2026 00:24
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.

[Duplicate Code] startup-failure detection functions duplicated twice in container-lifecycle.ts

3 participants