Repository navigation
feat(smoke): fail the deploy when a container is unhealthy; record the skills decision - #196
Conversation
…e skills decision
Two of the three follow-ups you asked me to decide. The third (TH/DE) is
yours — the 10 strings per locale are listed for your reviewers.
## Assert container health in smoke.sh
Nothing in the repo ever asserted this, and the HTTP checks cannot stand
in for it: the worker has no HTTP surface, so a running-but-broken worker
passed every check smoke made.
Not hypothetical. v2.8.0 shipped a worker healthcheck that failed on
EVERY interval — `node -e "require('pg')…"`, which cannot resolve a
transitive dep under pnpm's isolated node_modules. Deploy reported
success, smoke passed, and the container sat heading for `unhealthy`
until I found it by hand with `docker inspect`.
Two details do more work than the check itself:
- "Nothing is unhealthy", NOT "everything is healthy". caddy declares no
healthcheck; a gate that failed on services which never declared one
would cry wolf every run — and a gate that cries wolf gets switched
off, which is how you end up with no gate at all.
- Print the failing container's last probe output. The original bug was
undiagnosable from the deploy log; a gate that reports THAT something
failed without WHY only relocates the work.
`starting` warns without failing — smoke runs immediately after deploy
and start_period is up to 40s.
Chose the deploy over a CI compose boot deliberately. A healthcheck
describes a running container; CI would boot a reconstruction of one,
cost minutes per PR, and duplicate what deploy already does. Its only
marginal benefit is catching the fault before MERGE rather than before
TRAFFIC — and with autonomous-deploy-on-green, before-traffic is the
boundary that protects the instance.
## Decision: Skill gets no project dimension
Declining the migration, and the argument isn't cost.
There is no correct backfill. A skill is distilled from work that may
span several projects, so existing rows would be either NULLed — hiding
every existing skill from scoped tokens — or assigned arbitrarily, which
is fabricating data to satisfy a schema. The (skillId, ownerUserId)
unique key would also have to either gain the project (duplicating the
same skill per project, where it then drifts) or leave the column
advisory. When a backfill has no truthful value, that is usually the
schema saying the column doesn't belong on that table.
It also fights what a Skill is: Knowledge is atomic and project-bound,
but a Skill is a portable recipe — the exporter writes them to
.claude/skills/ and .cursor/rules/, and BLUEPRINT §11.2 plans to
distribute them as packs. Skill already carries `scope` and
`ownerTeamId`; the missing project axis is a statement, not an oversight.
If a scoped token must be kept away from skills, the right primitive is a
token capability (read:knowledge without read:skills) — one column, no
backfill, generalises to every future surface. Not built until asked.
Test plan:
- [x] `bash -n` clean.
- [x] Runs green against the live stack (7 services, none unhealthy).
- [x] VERIFIED IN BOTH DIRECTIONS — detects a container deliberately
broken with the SAME `require('pg')` probe that caused the original
incident, and prints its output; and treats a container with no
healthcheck defined as a pass rather than a failure. A gate that
has only ever passed has not been tested, it has been observed.
- [ ] CI.
Docs: KNOWN_ISSUES §0q (both), GUIDELINES (healthcheck rules + why no CI
gate caught it), KNOWLEDGE §12.21 (skills decision), APPROACH §5bi
(declining a schema change is a result; put the gate where the artifact
is).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDtsHQ5kq3HHxswr2o2s2Q
📝 WalkthroughWalkthroughThe PR documents the decision to keep Skills unpartitioned by project and adds Docker Compose health validation to ChangesSkill scope decision
Compose health gate
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant smoke.sh
participant DockerCompose as Docker Compose
participant ServiceContainer as Service container
smoke.sh->>DockerCompose: inspect service health states
DockerCompose->>ServiceContainer: read health status and probe output
ServiceContainer-->>DockerCompose: return health status
DockerCompose-->>smoke.sh: return health details
smoke.sh-->>smoke.sh: fail, warn, or skip based on status
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/GUIDELINES.md`:
- Around line 209-214: Update the probe guidance in GUIDELINES.md to prohibit
resolving application or transitive dependencies, while explicitly allowing Node
built-in APIs such as require('http'). Ensure the wording remains consistent
with the existing webapp and MCP Compose healthchecks without requiring those
probes to switch to fetch.
In `@scripts/smoke.sh`:
- Around line 71-78: Update the health-check loop in scripts/smoke.sh to
preserve failures from docker compose and docker inspect instead of converting
them to an empty state with 2>/dev/null || true. Capture the Compose container
list and each container’s inspection result once, validate lookup and inspection
success before evaluating .State.Health.Status, and return a failure when
discovery fails rather than reporting no unhealthy containers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d7f1690d-8b55-4e6f-8917-12737317c87c
📒 Files selected for processing (5)
docs/APPROACH.mddocs/GUIDELINES.mddocs/KNOWLEDGE.mddocs/KNOWN_ISSUES.mdscripts/smoke.sh
| - **Never resolve a module in a probe.** `node -e "require('pg')…"` fails with | ||
| `Cannot find module 'pg'` under pnpm's isolated `node_modules` — a transitive | ||
| dependency is not resolvable from the app directory even when the app itself | ||
| depends on it. Have the service expose a liveness endpoint and probe it with | ||
| Node's built-in `fetch`, which needs nothing from `node_modules` at all. That | ||
| removes the resolution question instead of working around it. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | rg '(^|/)docs/GUIDELINES\.md$|(^|/)deploy/docker-compose\.yml$|(^|/)scripts/smoke\.sh$|(^|/.github/)' || true
printf '\n--- guidelines around probe rule ---\n'
if [ -f docs/GUIDELINES.md ]; then
nl -ba docs/GUIDELINES.md | sed -n '190,235p'
fi
printf '\n--- docker compose healthchecks/require examples ---\n'
if [ -f deploy/docker-compose.yml ]; then
rg -n "healthcheck|require\\(|fetch\\(|cmd|test:" deploy/docker-compose.yml -C 3
fi
printf '\n--- node require/fetch probe-related usage ---\n'
rg -n "node -e|require\\(|fetch\\(" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -120Repository: bejranonda/ExternalBrain
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg '(^|/)docs/GUIDELINES\.md$|(^|/)deploy/docker-compose\.yml$|(^|/)scripts/smoke\.sh$|(^|/\.github/)' || true
printf '%s\n' ''
printf '%s\n' '--- guidelines around probe rule ---'
if [ -f docs/GUIDELINES.md ]; then
nl -ba docs/GUIDELINES.md | sed -n '190,235p'
fi
printf '%s\n' ''
printf '%s\n' '--- docker compose healthchecks/require examples ---'
if [ -f deploy/docker-compose.yml ]; then
rg -n "healthcheck|require\(|fetch\(|cmd:|test:" deploy/docker-compose.yml -C 3 || true
fi
printf '%s\n' ''
printf '%s\n' '--- node require/fetch probe-related usage ---'
rg -n "node -e|require\(|fetch\(" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -120 || trueRepository: bejranonda/ExternalBrain
Length of output: 24973
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- guidelines around probe rule ---'
if [ -f docs/GUIDELINES.md ]; then
sed -n '190,235p' docs/GUIDELINES.md
fi
printf '%s\n' ''
printf '%s\n' '--- docker compose healthchecks/require examples ---'
if [ -f deploy/docker-compose.yml ]; then
rg -n "healthcheck|require\(|fetch\(|cmd:|test:" deploy/docker-compose.yml -C 3 || true
fi
printf '%s\n' ''
printf '%s\n' '--- nodemon/probe scripts smoke docs ---'
if [ -f scripts/smoke.sh ]; then
sed -n '1,180p' scripts/smoke.sh
fi
printf '%s\n' ''
printf '%s\n' '--- node http/fetch probe-related usage ---'
rg -n "node -e|require\(|fetch\(" scripts docs deploy apps --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -200 || trueRepository: bejranonda/ExternalBrain
Length of output: 30900
🌐 Web query:
Node.js LTS built-in fetch global fetch without require documentation
💡 Result:
In Node.js, the Fetch API is available as a global object by default in all maintained Long Term Support (LTS) versions, including Node.js 22 and 24 [1]. You do not need to install any packages, use require, or import it to use fetch in your code [1][2]. Key details regarding native global fetch in Node.js: 1. Availability: Global fetch was introduced experimentally in Node.js 18 [3] and became fully stable in Node.js 21 [4]. As of August 2026, all currently supported LTS versions include this feature [1]. 2. Usage: Because it is a global, you can simply call fetch directly in your scripts [1][2]. example: const response = await fetch('https://api.example.com/data'); const data = await response.json; console.log(data); 3. Implementation: Node.js's native implementation is powered by Undici, a high-performance HTTP client [4][5]. It aims to be compatible with the browser-standard Fetch API [1][3]. 4. No Installation Required: External libraries like node-fetch or axios are no longer necessary for basic HTTP requests [1][2]. You can remove these dependencies if they were only used for standard fetch functionality [4]. 5. Advanced Configuration: If you require features not supported by the native fetch (such as specific mTLS/client certificate configurations or custom agents), you may need to use the undici library directly or use a lower-level Node.js HTTP module, as the native fetch API's RequestInit object has limited support for these advanced options [6][7]. For official technical documentation on global APIs in Node.js, you can refer to the Node.js API reference for globals [6].
Citations:
- 1: https://thunderbit.com/blog/nodejs-fetch-api-guide
- 2: https://www.hlibco.com/posts/2022/nodejs-18-native-fetch-api-experimental-test-runner/
- 3: https://nodejs.org/en/blog/announcements/v18-release-announce
- 4: https://blog.logrocket.com/fetch-api-node-js/
- 5: https://nodejs.org/learn/getting-started/fetch
- 6: Native fetch client certificate support nodejs/node#48977
- 7: Make it possible to use Fetch with proxies or other agents nodejs/node#42814
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Node Engine requirements ---'
for f in package.json apps/web/package.json apps/mcp-server/package.json apps/worker/package.json packages/*/package.json; do
[ -f "$f" ] || continue
printf '%s: ' "$f"
node -e "const p=require('./${f#./}'); console.log(JSON.stringify({engines:p.engines,packageManager:p.packageManager,scripts:p.scripts && Object.keys(p.scripts).slice(0,8)}).slice(0,400))" 2>/dev/null || cat "$f" | sed -n '1,80p'
done
printf '%s\n' ''
printf '%s\n' '--- Node version declarations ---'
rg -n '"node"|engines|node-version|setup-node|node:' .github package.json apps packages deploy --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -80 || trueRepository: bejranonda/ExternalBrain
Length of output: 8450
🌐 Web query:
Node.js 20 global fetch version release channel required for global fetch experimental stable
💡 Result:
In Node.js 20, the global fetch API is classified as experimental [1][2]. While it does not require a command-line flag to be enabled—as the flag was removed in Node.js 18.0.0—it remains marked as "1- Experimental" in the Node.js documentation throughout the entire Node.js 20 release line [1][3][4][5]. The Fetch API was promoted to stable status in Node.js 21 [5]. Because Node.js 20 is a Long Term Support (LTS) release, it retains the experimental designation for fetch for the duration of its support lifecycle [1][6][5]. There is no channel or configuration setting within the Node.js 20 release line that changes the status of global fetch from experimental to stable [1][2]. Users requiring a stable fetch implementation are advised to use newer versions of Node.js (v21 or later) or to use a library like undici [7][5].
Citations:
- 1: https://nodejs.org/docs/latest-v20.x/api/globals.html
- 2: https://nodejs.org/dist/latest-v20.x/docs/api/globals.html
- 3: https://nodejs.org/download/release/v20.19.0/docs/api/globals.html
- 4: https://nodejs.org/download/release/v20.8.0/docs/api/globals.html
- 5: https://dev.to/appsignal/whats-new-in-nodejs-21-137b
- 6: https://nodejs.org/en/blog/release/v20.20.1
- 7: https://undici.nodejs.org/best-practices/undici-vs-builtin-fetch
Clarify the probe rule for built-in Node APIs.
require('http') is not a transitive dependency, but the current wording conflicts with the webapp and MCP Compose healthchecks. Narrow it to prohibit resolving application or transitive dependencies, or update those probes to fetch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/GUIDELINES.md` around lines 209 - 214, Update the probe guidance in
GUIDELINES.md to prohibit resolving application or transitive dependencies,
while explicitly allowing Node built-in APIs such as require('http'). Ensure the
wording remains consistent with the existing webapp and MCP Compose healthchecks
without requiring those probes to switch to fetch.
| if command -v docker >/dev/null 2>&1 && docker compose -f deploy/docker-compose.yml --env-file .env ps -q >/dev/null 2>&1; then | ||
| UNHEALTHY="" | ||
| STARTING="" | ||
| for cid in $(docker compose -f deploy/docker-compose.yml --env-file .env ps -q 2>/dev/null); do | ||
| name=$(docker inspect --format '{{.Name}}' "$cid" 2>/dev/null | sed 's#^/##') | ||
| # `.State.Health` is absent when the service declares no healthcheck — | ||
| # `{{if}}` keeps that case as the empty string rather than "<no value>". | ||
| state=$(docker inspect --format '{{if .State.Health}}{{.State.Health.Status}}{{end}}' "$cid" 2>/dev/null || true) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
compose=(docker compose -f deploy/docker-compose.yml --env-file .env)
set +e
result=$("${compose[@]}" ps -q 2>&1)
status=$?
set -e
printf 'exit=%s\n%s\n' "$status" "$result"Repository: bejranonda/ExternalBrain
Length of output: 211
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the smoke script region and nearby behavior without running repository code.
if [ -f scripts/smoke.sh ]; then
wc -l scripts/smoke.sh
sed -n '60,110p' scripts/smoke.sh | cat -n
else
echo "scripts/smoke.sh not found"
fd -a 'smoke\.sh$' .
fi
# Find Docker/Compose usage and similar patterns across tracked files.
rg -n "docker compose|docker inspect|\\|\\| true|no unhealthy containers|no unhealthy" . -g '!node_modules' -g '!dist' -g '!build' || trueRepository: bejranonda/ExternalBrain
Length of output: 37893
🌐 Web query:
Docker Compose v2 ps exit code no containers unhealthy containers --format json
💡 Result:
The docker compose ps command in Docker Compose v2 has the following characteristics regarding exit codes, unhealthy containers, and JSON output: Exit Codes The docker compose ps command generally returns an exit code of 0 even if no containers are found or if the specified service does not yield running containers [1][2]. It is designed to report the status of containers without necessarily treating an empty result as a failure condition [2]. Unhealthy Containers By default, docker compose ps only displays running containers [3][4]. Unhealthy containers that are still running will appear in the output, but containers that have exited (whether unhealthy or exited gracefully) are hidden unless you explicitly include the --all flag [3][5][4]. When listed (via --all), the status column or JSON output will indicate their state (e.g., exited, unhealthy) [5][4]. JSON Format (--format json) The --format json output produces Newline-Delimited JSON (NDJSON / JSON Lines) rather than a single valid JSON array [6][7]. Each line represents a separate JSON object corresponding to a container [7]. To process this output with tools like jq, you may need to use streaming options (e.g., jq -s) to convert the input into a proper JSON array [8][7]. Summary of Key Options Use --all (-a) to include stopped, exited, or unhealthy containers that are not currently running [3][4]. Use --status=[filter] to restrict output to specific states (e.g., --status=exited), though note that this filter is typically applied after retrieving the container list [3][9]. Use --format json for machine-readable output, keeping in mind the NDJSON structure [7].
Citations:
- 1: docker compose ps xxx error behavior changed between v1 and v2 docker-archive/compose-cli#1893
- 2: docker/compose@1b15a2a
- 3: https://docs.docker.com/reference/cli/docker/compose/ps/
- 4: https://github.com/docker/compose/blob/main/docs/reference/compose_ps.md
- 5:
docker compose psdoes not list stopped containers and their exit status docker-archive/compose-cli#1671 - 6: [BUG] docker-compose ps --format json docker/compose#11784
- 7: docs: ps --format json outputs JSON Lines, not a JSON array docker/compose#13868
- 8: https://stackoverflow.com/questions/77142618/incorrect-json-format-from-docker-compose-ps
- 9: [BUG]
docker compose ps --statusbehavior differs from documentation docker/compose#11464
🌐 Web query:
site:github.com/docker/compose docker compose ps exit code 1 no containers --format json
💡 Result:
The docker compose ps command does not return an exit code of 1 when no containers are found [1][2]. Historically, older versions of Docker Compose could return an error or exit with a non-zero code if no containers matched the request [1][2]. However, this behavior was intentionally changed to ensure the command exits successfully (exit code 0) even when the result set is empty [1][2]. Regarding the --format json output: 1. Format Change: As of version 2.21.0, the output format of docker compose ps --format json was updated to align with the Docker CLI [3]. 2. JSON Lines: It no longer returns a single JSON array (e.g., [...]) [3][4][5]. Instead, it outputs "JSON Lines" format, where each container is represented as an individual JSON object on its own line [6][7][8]. 3. Parsing: Because the output is a stream of objects rather than a single array, tools like jq can process the output by piping it to jq -s . if you need to convert it into a valid JSON array for further processing, or by using jq . to process each line individually [6][7][8]. If you are experiencing an unexpected exit code of 1 in a modern version of Docker Compose, it is likely due to a different issue, such as an invalid command argument or a configuration error, rather than the absence of containers [9].
Citations:
- 1: docker/compose@e1267cd
- 2: docker/compose@1b15a2a
- 3: [BUG]
docker compose ps --format jsonno longer returns an array in 2.21.0 docker/compose#10958 - 4: https://github.com/docker/compose/blob/d91fc638/docs/reference/compose_ps.md
- 5: https://github.com/docker/compose/blob/e5c6b9c3/docs/reference/docker_compose_ps.yaml
- 6: https://github.com/docker/compose/blob/main/docs/reference/compose_ps.md
- 7: https://github.com/docker/compose/blob/e19f3c63/docs/reference/compose_ps.md
- 8: https://github.com/docker/compose/blob/e19f3c63/docs/reference/docker_compose_ps.yaml
- 9: [BUG]
pserrors out when any of the arguments is a non created but valid container name docker/compose#9951
🌐 Web query:
Docker Compose v2 ps exit code unhealthy container no healthcheck
💡 Result:
In Docker Compose v2, if a container has no healthcheck defined, it does not have an "unhealthy" state [1][2]. Health status is only calculated if a healthcheck block is explicitly configured for the service [1][3][4]. If you see a container listed as "unhealthy," it means a health check has been configured but is failing [1][2]. If you have not defined a healthcheck in your docker-compose.yml but believe your container is reporting an "unhealthy" status, consider the following: 1. Base Image Overrides: Some official Docker images define a HEALTHCHECK instruction in their Dockerfile [5]. This baked-in health check will execute automatically unless you explicitly disable it in your docker-compose.yml [5]. You can disable it by setting healthcheck: disable in your service definition. 2. Troubleshooting the Failure: To determine why a container is marked unhealthy, use the following commands: - Run docker inspect --format='{{json .State.Health}}' <container_name> to see the health status, consecutive failure count (FailingStreak), and the output/exit codes of the most recent checks [1][4][6]. - Use docker ps or docker compose ps to verify the container's status [2][7]. - Remember that health check exit codes have specific meanings: 0 (success/healthy), 1 (failed/unhealthy), and 127 (command not found) [2][8]. If you are using depends_on with condition: service_healthy, Docker Compose will wait for the service to report a "healthy" status before starting dependent services [1][3][5]. If no health check is defined (or if it is disabled), a service will never transition to "healthy," which may cause your stack to hang if it is waiting for that condition [2]. If you only need to ensure the container has started, use the default condition: service_started instead [2][5].
Citations:
- 1: https://blog.path-finder.jp/troubleshooting/how-to-fix-dependency-failed-to-start-container-se/
- 2: https://fixdevs.com/blog/docker-compose-healthcheck-not-working/
- 3: https://systeminternals.dev/docker/docker-compose/
- 4: https://oneuptime.com/blog/post/2026-02-08-how-to-use-docker-compose-healthcheck-configuration/view
- 5: https://reponotes.com/blog/docker-compose-healthcheck/
- 6: https://gist.github.com/matrafisco/76375b93cba6f7f004d822e9ca3fcb2a
- 7: https://docs.docker.com/reference/cli/docker/compose/ps/
- 8: Compose healthcheck example might not work as expected docker/docs#14113
🌐 Web query:
GitHub docker/compose ps exited unhealthy default running containers
💡 Result:
When you see the status exited (unhealthy) in docker compose ps [1][2], it indicates that a container has stopped (exited) and was in an unhealthy state at the time it stopped [1]. It is important to understand the following key points about this status: 1. Meaning: A container is marked unhealthy when its defined HEALTHCHECK command returns a non-zero exit code or times out consecutively for a specified number of retries [3]. When a container is in this state and then stops (due to a crash, docker compose stop, or other reasons), its status reflects both its stopped state and its last recorded health status [1]. 2. Visibility: By default, docker compose ps only lists running containers [4][5]. If your container has exited, it will not appear in the default output [4][5]. You must use the --all (or -a) flag to see exited or stopped containers [4][5][2]. 3. Diagnosis: To investigate why a container became unhealthy before exiting, you can use docker inspect to examine its health status and logs [3]. The command docker inspect --format='{{json .State.Health}}' <container_name> will show the current status, the FailingStreak, and a log of recent health check attempts, including their exit codes and output [3]. 4. Automation: Because unhealthy containers do not automatically restart by default, some developers use automation to monitor and restart them. A common approach is using a script or a systemd service that periodically runs docker compose ps (filtering for the "unhealthy" string) and executes docker compose restart on those services [6]. If you are encountering unexpected behavior where exited containers are not showing up even with flags, note that there have been historical bugs in docker compose related to how --status filters interact with the --all flag; using docker compose ps --all is generally the most reliable way to list all containers in a project [7].
Citations:
- 1:
docker compose psdoes not list stopped containers and their exit status docker-archive/compose-cli#1671 - 2:
docker compose psdoes not list stopped containers and their exit status docker-archive/compose-cli#1671 - 3: https://www.netdata.cloud/guides/docker/docker-container-running-but-unhealthy/
- 4: https://docs.docker.com/reference/cli/docker/compose/ps/
- 5: https://github.com/docker/compose/blob/main/docs/reference/compose_ps.md
- 6: https://mateusznowak.dev/articles/restarting-unhealthy-compose-containers-oneliner/
- 7: [BUG]
docker compose ps --statusbehavior differs from documentation docker/compose#11464
🌐 Web query:
Docker Compose v2 ps --include-recreating --format json
💡 Result:
The command 'docker compose ps --include-recreating --format json' contains a flag that is not supported by the 'docker compose ps' command [1][2][3]. 1. --include-recreating: This flag does not exist for 'docker compose ps'. The command only supports a specific set of flags, primarily including '--all', '--filter', '--format', '--no-trunc', '--orphans', '--quiet', '--services', and '--status' [1][2]. 2. --format json: This flag is supported [1][4]. As of Docker Compose v2.21.0, this outputs container information in a newline-delimited JSON stream (JSON Lines format), where each container is represented by a single JSON object on its own line [5][6]. This change was made to align with the output format of the 'docker ps --format json' command [6]. Note that some documentation sources may still incorrectly describe this as a single JSON array [6]. If you intended to filter containers, you can use the '--status' or '--filter' flags to narrow down the output based on container status (e.g., 'paused', 'restarting', 'removing', 'running', 'dead', 'created', 'exited') [1][2].
Citations:
- 1: https://docs.docker.com/reference/cli/docker/compose/ps/
- 2: https://github.com/docker/compose/blob/main/docs/reference/compose_ps.md
- 3: https://manpages.debian.org/trixie/docker-compose/docker-compose-ps.1.en.html
- 4: https://manpages.debian.org/unstable/docker-compose/docker-compose-ps.1.en.html
- 5: [BUG] docker-compose ps --format json docker/compose#11784
- 6: [BUG]
docker compose ps --format jsonno longer returns an array in 2.21.0 docker/compose#10958
🌐 Web query:
Docker Compose v2 ps --format json Health Status
💡 Result:
When running docker compose ps --format json, the output includes a Health field as part of each JSON object [1][2]. Key details regarding this command's output format and content: 1. Health Status Field: The JSON object returned for each container contains a "Health" key [1][2]. This field reflects the container's health status (e.g., "healthy", "unhealthy", "starting", or an empty string if no healthcheck is configured) [2][3]. 2. Output Format (JSON Lines): Since Docker Compose v2.21.0, the command outputs data in JSON Lines format (newline-delimited JSON), meaning it returns one individual JSON object per line rather than a single JSON array [4][5]. If your tooling expects a standard JSON array, you can convert the output using a tool like jq by piping the command to jq -s . [6][5]. 3. Documentation Note: Older documentation or man pages may incorrectly describe the output as a JSON array [4][7][5]. Official current documentation now accurately reflects the JSON Lines (NDJSON) behavior [1][2].
Citations:
- 1: https://github.com/docker/compose/blob/main/docs/reference/compose_ps.md
- 2: https://docs.docker.com/reference/cli/docker/compose/ps/
- 3: Can docker-compose ps show health status? docker/compose#5525
- 4: [BUG]
docker compose ps --format jsonno longer returns an array in 2.21.0 docker/compose#10958 - 5: docs: ps --format json outputs JSON Lines, not a JSON array docker/compose#13868
- 6: https://stackoverflow.com/questions/77142618/incorrect-json-format-from-docker-compose-ps
- 7: https://manpages.debian.org/testing/docker-compose/docker-compose-ps.1.en.html
Preserve health-discovery failures.
docker inspect failures currently fall through as an empty state because the command uses 2>/dev/null || true; an unhealthy container can then report no unhealthy containers. Capture a container once and check for Compose config/lookup/inspection errors before reporting success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/smoke.sh` around lines 71 - 78, Update the health-check loop in
scripts/smoke.sh to preserve failures from docker compose and docker inspect
instead of converting them to an empty state with 2>/dev/null || true. Capture
the Compose container list and each container’s inspection result once, validate
lookup and inspection success before evaluating .State.Health.Status, and return
a failure when discovery fails rather than reporting no unhealthy containers.
Two of the three follow-ups you asked me to decide. TH/DE is yours — the 10 strings per locale are listed for your reviewers.
Assert container health in
smoke.shNothing ever asserted this, and the HTTP checks can't stand in: the worker has no HTTP surface, so a running-but-broken worker passed every check.
Not hypothetical — v2.8.0 shipped a healthcheck that failed on every interval (
node -e "require('pg')…", unresolvable under pnpm's isolatednode_modules). Deploy reported success, smoke passed, and I found it by hand withdocker inspect.Two details do more work than the check itself:
caddydeclares no healthcheck; a gate that failed on services which never declared one would cry wolf every run — and a gate that cries wolf gets switched off, which is how you end up with no gate.Deploy, not CI, on purpose. A healthcheck describes a running container; CI would boot a reconstruction, cost minutes per PR, and duplicate what deploy does. Its only marginal benefit is catching the fault before merge rather than before traffic — and with autonomous-deploy-on-green, before-traffic is the boundary that protects the instance.
Decision:
Skillgets no project dimensionDeclining the migration, and the argument isn't cost.
There is no correct backfill. A skill is distilled from work spanning several projects — existing rows would be either NULLed (hiding every existing skill from scoped tokens) or assigned arbitrarily, which is fabricating data to satisfy a schema. The
(skillId, ownerUserId)unique key would also have to either gain the project (duplicating skills per project, where they drift) or leave the column advisory. When a backfill has no truthful value, that's usually the schema saying the column doesn't belong on that table.It also fights what a Skill is:
Knowledgeis atomic and project-bound, but a Skill is a portable recipe — exported to.claude/skills/, andBLUEPRINT §11.2plans to distribute them as packs.Skillalready hasscopeandownerTeamId; the missing project axis is a statement, not an oversight.If a scoped token must be kept from skills, the right primitive is a token capability (
read:knowledgewithoutread:skills) — one column, no backfill, generalises. Not built until asked.Test plan
bash -ncleanrequire('pg')probe that caused the original incident and prints its output; treats a no-healthcheck container as a pass. A gate that has only ever passed has not been tested, it has been observed.🤖 Generated with Claude Code
https://claude.ai/code/session_01GDtsHQ5kq3HHxswr2o2s2Q
Summary by CodeRabbit