Harden api-proxy startup healthcheck to reduce early unhealthy flaps#2155
Harden api-proxy startup healthcheck to reduce early unhealthy flaps#2155
Conversation
There was a problem hiding this comment.
Pull request overview
Hardens the api-proxy container startup healthcheck in generated Docker Compose to reduce early “unhealthy” flaps that can block the agent via depends_on: service_healthy.
Changes:
- Increased
api-proxyhealthcheck tolerance by updatingtimeout,retries, andstart_period. - Updated unit tests to assert the new healthcheck parameters.
Show a summary per file
| File | Description |
|---|---|
src/docker-manager.ts |
Adjusts api-proxy healthcheck parameters (timeout/retries/start_period) to be more tolerant during bootstrap. |
src/docker-manager.test.ts |
Extends the api-proxy healthcheck test assertions to match the updated compose output. |
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: 1
| const proxy = result.services['api-proxy'] as any; | ||
| expect(proxy.healthcheck).toBeDefined(); | ||
| expect((proxy.healthcheck as any).test).toEqual(['CMD', 'curl', '-f', 'http://localhost:10000/health']); | ||
| expect(proxy.healthcheck.test).toEqual(['CMD', 'curl', '-f', 'http://localhost:10000/health']); | ||
| expect(proxy.healthcheck.timeout).toBe('2s'); |
There was a problem hiding this comment.
In this test, casting the entire proxy service to any is unnecessary and reduces type safety; DockerService already includes a typed healthcheck field. Prefer keeping proxy typed and using the existing pattern used elsewhere in this file (e.g., proxy.healthcheck!.... after toBeDefined()) or assigning const healthcheck = proxy.healthcheck! for assertions.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot integrate this review feedback #2155 (review) |
Integrated the review feedback by removing the unnecessary |
This comment has been minimized.
This comment has been minimized.
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PASS — PR by
|
|
Smoke Test Results
PASS
|
This comment has been minimized.
This comment has been minimized.
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environment.
|
Smoke Test Results: GitHub Actions Services Connectivity
All 3 checks failed. The service containers at
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL — 2 of 3 tests could not be verified due to unsubstituted workflow template variables. PR author:
|
|
Smoke test matrix
Warning The following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
api-proxystartup was intermittently marked unhealthy before the Node server fully stabilized, which could blockagentstartup viadepends_on: service_healthy. This change increases healthcheck tolerance during bootstrap while preserving the same readiness endpoint.Healthcheck startup tolerance (api-proxy)
start_periodfrom2s→10sretriesfrom5→10timeoutfrom1s→2scurl -f http://localhost:10000/health,interval: 1s)Unit test contract update
generateDockerComposeapi-proxy healthcheck assertions to lock the newtimeout,retries, andstart_periodvalues.