fix(cli): fail host metrics setup when the reload fails or the stack is too old

Follow-up to !436 (closed). It fixes the issues from the review note on that MR, and targets !436 (closed)'s branch. Relates to https://gitlab.com/postgres-ai/internal/-/issues/354

Round 2 (2026-09-30) is at the end: onboarding through mon local-install, key handling, stopped-stack bugs, mon update and PGAI_TAG, and a real ClickHouse Cloud run.

1. A failed reload now fails the command. mon targets add and mon targets remove now check the exit code of the SIGHUP to sink-prometheus. If the reload fails, they exit 1 and print "Reloading sink-prometheus failed. Run 'postgresai mon restart' to load the host metrics change."

2. An older stack no longer gets a false success. Before the reload, add runs a check inside the running sink-prometheus container. It confirms that the mounted prometheus.yml has scrape_config_files and that the new /etc/pgai/host-metrics/clickhouse-<name>.yml is visible, which proves the ./host-metrics mount. If either is missing, or the container is not running, the command exits 1 and tells the user to run postgresai mon update, mon stop and mon start (round 2: mon update now moves PGAI_TAG itself). The scrape files are kept. We point to stop and start rather than mon restart because docker compose restart keeps the old config-init image and the old mounts.

3. A reload that VictoriaMetrics receives but does not apply now fails the command. VictoriaMetrics applies a SIGHUP asynchronously. When it cannot parse a file under scrape_config_files, it logs the error, skips the file, and still reports the reload as successful: we saw vm_promscrape_config_last_reload_successful stay at 1 and vm_promscrape_config_reloads_errors_total stay at 0 on v1.140.0. So the reload metrics cannot catch this, and only /api/v1/targets shows the result. After the SIGHUP, the CLI runs a short script inside sink-prometheus that polls /api/v1/targets on 127.0.0.1:9090. It sends basic auth when VM_AUTH_USERNAME is set. Each request times out after 2 s, and the poll stops after 10 s.

• add waits for the exact new file. The scrape file now has a __pgai_rev target label, a hash of the rendered config. VictoriaMetrics drops labels that start with __ from stored series, so no series changes. The label still shows in the targets list, which means a re-add cannot pass by matching the job that was loaded before the reload. If the revision does not appear, the command exits 1 with "sink-prometheus did not load the scrape job 'clickhouse-' after the reload. Check 'docker logs sink-prometheus' for the error. The scrape files are saved."
• remove waits until the clickhouse-<name> scrape pool is gone from a successful targets response. Otherwise it exits 1 with "sink-prometheus still scrapes 'clickhouse-' after the reload. Check 'docker logs sink-prometheus' for the error."

The line "Host metrics: ClickHouse Cloud ..." now prints only after the check, the reload and the verification all succeed.

Tests

• cli/test/clickhouse-reload.test.ts spawns the real CLI with a temporary HOME, XDG_CONFIG_HOME and project dir. A fake docker placed first on PATH records every compose call, and the exec/kill sequence is compared against a golden list. It covers a failed reload, an older stack, a rejected add, a remove that still scrapes, and success for both add and remove.
• cli/test/clickhouse-verify.test.ts runs the exact verification script under /bin/sh, with a fake wget in place of the HTTP call. It covers the request arguments (timeout, auth header with @:/ in the password), waiting for the new revision instead of accepting the old one, clickhouse-a vs clickhouse-ab, an error response, no answer, and requests that time out. The failure cases finish between 9 and 14 s.
• cli/test/clickhouse.test.ts: the scrape-config golden now includes __pgai_rev. The revision stays the same for the same input and changes when the input changes, and it is read from the parsed label, not from revision-like text in another label.
• Each fix has a failing test commit first.
• Full suite locally: 1985 pass, 1 fail. The one failure, checkup-api CHECKUP_ALLOW_HTTP=1, is a network timeout that happens without this change too. tsc reports the same 23 errors in unrelated test files as before this change.
• Manual check against a real victoriametrics/victoria-metrics:v1.140.0 container with basic auth on: add and a changed re-add pass right away, a rejected file fails after 10 s, remove passes. Busybox wget -T 2 in that image times out at 2 s on a listener that never answers. The existing real-VM integration test, clickhouse-scrape.integration.test.ts, passes with the new label.

What we did not check

• End-to-end runs on a full local compose stack, including an upgrade from a 0.16.0 stack, are in the comments.
• The basic auth header is passed to wget on its command line inside sink-prometheus. It is visible there only to processes in that container, which already has the password in its environment.
• If a re-add changes only the file and VictoriaMetrics still has the old revision loaded, we wait up to 10 s. We did not measure reload latency on a busy instance, where a slow reload could cause a false failure.
• runCompose treats a process killed by a signal as exit 0. This was here before this change, and we did not change it.

Round 2 (2026-09-30)

Rebased onto !436 (closed)'s head (the only conflict was the import line in cli/bin/postgres-ai.ts) and merged !436 (closed), which now carries !434 (closed)'s table_describe hardening.

What changed

1. mon local-install --db-url sets up ClickHouse host metrics. It and the interactive prompt now add the target through addTarget, the same path as mon targets add, so the main onboarding path writes the scrape job and the key file. The stack starts afterwards and loads the files, so nothing is reloaded at that point. The target name drops the URL query: ClickHouse Cloud URLs end in ?sslmode=require&channel_binding=require, and that used to become part of the name. local-install now stops with exit 1 when the target was not saved, instead of starting a stack without it.
2. The org API key. host-metrics/ is created 0700, and tightened to 0700 when an older CLI created it 0755. The key file stays 0600. A host-metrics symlink is refused. The key never appears in output or errors, and tests cover add, the failure paths, and local-install. We checked the real API for the least privilege that works: a key with only the Basic Service API Reader role gets 200 on the service list, the service details and /prometheus (49 PostgresServer_ lines), and 403 on PATCH. An Organization API Reader key gets 403 on the details and on /prometheus. The 403 error, targets add --help, cli/README (new "ClickHouse Cloud host metrics" section) and CHANGELOG now recommend that role and say where the key is kept and how to rotate it.
3. Read-only commands in a directory the CLI cannot write. The pre-compose mkdir host-metrics in resolveOrInitPaths is now best effort. Before, mon targets list in a read-only project dir died with an uncaught EACCES.
4. remove on a stopped stack now reads the sink-prometheus container state (compose ps -a -q plus docker inspect, which docker-compose v1 also supports). A created, exited or dead container, or no container, means there is nothing to reload, so the command exits 0 with "sink-prometheus is not running; it will not load 'clickhouse-' when it starts." A paused container, a running one, or an unknown state still goes through reload and verification, and those fail loudly.
5. A failed add leaves the stack alone. It returns before up -d pgwatch-*, which started a stopped sink-prometheus as a dependency, and prints "The Postgres target is saved but not applied. Fix the error above and re-run this command."
6. mon update moves PGAI_TAG. It sets every PGAI_TAG assignment in .env to the CLI version, as local-install does, and keeps export, quotes, comments and CRLF. It never downgrades a newer tag, including prereleases. It does not move anything from a non-release CLI build, or when an assignment is not a plain value (interpolation, PGAI_TAG: x); in both cases it prints the exact line to set. It carries the move into process.env, because Bun loads .env into the environment and compose prefers the environment. It now always suggests mon stop && mon start: mon restart keeps the old containers. The older-stack hint and the README upgrade steps no longer ask for a manual .env edit.

Evidence

• Red/green: each fix has a failing test commit first (ffda04a, 5d258f5, 05e1f1d, ce4d13d, b85ab15, 79fadb1, 61fc78e, 62befe0), then the fix. New and extended tests: clickhouse-local-install.test.ts (runs the real mon local-install against a fake ClickHouse API, including the interactive prompt), mon-update-tag.test.ts, clickhouse-reload.test.ts, clickhouse-targets.test.ts.
• bun test clickhouse local-install mon-update-tag upgrade: 214 pass, 0 fail (156/156 on the final commit for clickhouse, local-install and clickhouse-reload). tsc: the same 23 errors as the base, none in changed files. The full suite locally: 8 failures, none from this change. 7 are init.test.ts local-install tests that stop at "already running" because another project's monitoring containers were running on this machine. The eighth is the known CHECKUP_ALLOW_HTTP network timeout.
• Cross-model security review of each diff. Round 1 found 5 blocking issues, round 2 found 4, and round 3 found 1 (tag parsing, local-install after a save failure, a symlinked dir, a failed exec counted as stopped, a retry hint, docker-compose v1 ps --format, the interactive prompt, a v1 one-off container listed before the real one). All of them are fixed, with tests, and the last round found nothing blocking.
• Real ClickHouse Cloud, 2026-09-30. One r6gd.medium PG 17 service in us-east-1 was created at 00:04:13 UTC; its GET returned 404 at 00:16:36 UTC, and the org's service list is empty. We ran prepare-db as the service superuser, then mon local-install --db-url <postgres_ai_mon URL> --tag fix-clickhouse-host-metrics-followups -y with only CLICKHOUSE_ORG_ID and a Basic Service API Reader key in the environment. It used a separate compose project with a temporary HOME, XDG_CONFIG_HOME and project dir. Exit 0: "Host metrics: ClickHouse Cloud Prometheus endpoint for service pgai-acc-726652". host-metrics/ was drwx------ and the key file -rw-------. The scrape job was up in about 60 s. VictoriaMetrics stored 39 series, with 21 PostgresServer_* names and PostgresServiceInfo, and no __pgai_rev label. pgwatch stored 192 metric names for the target. The reporter, run from monitoring data, built A002 (17.11), A003 (426 settings), H001 and K001 (empty on a fresh database). mon targets remove on the running stack exited 0, and the job and files were gone. A re-add exited 0 after a verified reload. remove with sink-prometheus stopped exited 0 with the not-running message. Both test keys were deleted.
• Also on that service: table_describe from !434 (closed) has search_path=pg_catalog, pg_temp, and it did not run a planted public.format(text, text, text).

What we did not check

• The changes made after the real-cloud run (the state probe through compose ps -q/inspect, tag parsing, the local-install save guard) were tested against a fake docker, not against a real stack.
• A mon update that actually moves the tag, end to end: this build reports 0.0.0-dev.0, so the move is covered by unit tests of planUpdateTag/writeEnvTag only.
• docker-compose v1, Linux hosts and rootless Docker. We ran on Docker Desktop, arm64. VictoriaMetrics runs as root there, so it can read the 0600 key file. A non-root VictoriaMetrics could not.
• mon local-install still prints the --db-url with its password ("Adding PostgreSQL instance from: ..."). That is old behaviour, and we did not change it.
• Dashboards for PostgresServer_*: another change owns the host dashboard row, and metric names and labels are unchanged here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GRNbWXUQJebE3vqWjAgTFh

Round 3 (review of 2026-09-29, round 2)

• mon update moving PGAI_TAG is now its own MR, !444, stacked on !436 (closed)'s branch like this one. Until it lands, the older-stack hint tells the user to set PGAI_TAG before mon update, mon stop, mon start.
• composeServiceState (ps -a -q plus docker inspect for compose v1 one-off containers) is replaced by one docker compose ps --status running --status paused call. Checked on Docker Compose 5.0.2: running and paused list the container, stopped and absent list nothing. When compose cannot answer (docker-compose v1 has no --status), targets remove reloads and verifies as for a running service instead of failing.
• maskConnectionString falls back to redactTextSecrets instead of its own regexes.
• local-install no longer parses the URL itself; addTarget derives the same <host>-<db> name.
• targets remove of a name like prod.db no longer fails (from !436 (closed)).
• Red b240726, d8e1cb0; green 9621e29, 47e20b5. ClickHouse CLI tests: 139 pass.
• Real ClickHouse Managed Postgres, 2026-09-30: clickhouse.cloud.test.ts passed from this branch (provider detected, scope line, --verify, the /prometheus endpoint serves every documented series); the test deleted its service.
• Own delta: code +167/−65 (was +241/−37), tests +549 (was +660).

🤖 Generated with Claude Code

https://claude.ai/code/session_01GRNbWXUQJebE3vqWjAgTFh