fix: bootstrap PostgreSQL without a working service manager (#823)

[GG-O-BP]

What & why

Closes #823.

mxcli run --ensure-db only knew service and a non-working Debian pg_ctlcluster placeholder. On Arch neither produced a server, so bootstrap failed even though the portable initdb, pg_ctl, and psql tools were available.

Fix

• Try service postgresql start with a bounded readiness probe, then fall back to a user-owned cluster under ~/.mxcli/postgres when no service becomes ready. The unusable pg_ctlcluster -- start placeholder is removed.
• Before fallback, check whether the requested TCP port is already owned. A service-started PostgreSQL still in recovery is left to the authoritative 20-second readiness wait instead of racing a second cluster against it.
• Initialize TCP authentication with SCRAM and keep password-free superuser provisioning only on the private Unix socket. Existing socket directories are explicitly tightened to 0700.
• Persist listen_addresses, port, unix_socket_directories, and unix_socket_permissions = 0700 in postgresql.conf; plain later pg_ctl -D ... start therefore preserves the safe endpoint without mxcli-only runtime flags.
• Reuse an initialized/running cluster only when both its live port and socket directory match. Refuse pre-review clusters that still contain host-trust authentication and provide a stop/remove/recreate path.
• Normalize and validate ports and empty hosts, surface service diagnostics and the server-log path, retain sudo -u postgres psql for system clusters, and keep remote hosts check-only.

Tests

Focused command-stub regressions cover service readiness, the occupied-port race, the removed placeholder, first/repeated/running starts, port and socket mismatches, existing-directory permissions, persistent and idempotent configuration, legacy host trust, authentication arguments, superuser resolution, validation, and actionable failures.

Validation completed on the final tree:

• make build
• make vet
• make test
• go test ./cmd/mxcli/docker -count=1
• targeted go test -race for the changed package paths
• real PostgreSQL 18.4 bootstrap, app login, repeated provisioning, socket/file permission checks, stop, plain pg_ctl restart without -o, reconnect, and legacy-host-trust refusal

Docs and scope

Updated only the implementation, its focused tests, the run-local skill/docs, CHANGELOG, and the final fix-issue table row: six files, one concern, one commit on current main. The docs now explain persistence, the server log, and safe stop/removal. No MDL fixture is applicable because this change has no MDL surface.

[ako]

Review

Sound fix, no blockers. Three things I'd want changed before merge, all localized to startLocalPostgres/startUserCluster.

What I verified — applied the two Go files to current main in a throwaway worktree (they apply cleanly):

• go vet ./cmd/mxcli/docker clean; all 12 new subtests pass in 0.5 s.
• The reported Arch path is genuinely fixed: neither service nor pg_ctlcluster is on PATH, both LookPath calls skip, and the fallback runs immediately.

---

Moderate

1. The fallback multiplies the timeout by the number of service attempts — confirmed, not theoretical.

Moving waitPGReady inside the attempts loop means every attempt pays a full 20 s. Instrumented with a scaled-down serviceReadyTimeout, counting probe invocations:

service → isready → pg_ctlcluster → isready → initdb → pg_ctl_start
ready-waits paid = 2   (at the real timeout: 40 s)

Worst case is now 20 + 20 (services) + 30 (pg_ctl -w -t 30) + 20 (EnsureDatabase's own waitPGReady) ≈ 90 s before an error surfaces, against ~20 s before. Worse, 20 s of that is spent on {"pg_ctlcluster", "--", "start"} — the entry whose own comment admits the args are a placeholder and cannot work.

Fix: drop the pg_ctlcluster placeholder, and probe inside the loop with a short timeout (2–3 s), keeping the single long waitPGReady where it already is in EnsureDatabase.

2. --auth-host=trust disables authentication on 127.0.0.1:5432.

Confirmed via the initdb args: -U postgres --auth-local=trust --auth-host=trust. The comment justifies it with "the server binds only the local interface", but loopback is not an access control on a multi-user host: any local account becomes the postgres superuser, and PostgreSQL superuser gets COPY … PROGRAM, i.e. shell as the developer's OS user.

Fix: the socket directory is already 0700 (verified — drwx------), so trust over the socket is genuinely safe. Keep --auth-local=trust, set --auth-host=scram-sha-256, and point superuser.psql's non-sudo branch at -h <sockDir> instead of -h 127.0.0.1. The app role already has a password, so canConnectDB over TCP still works unchanged.

3. The fix-issue.md row lands mid-table.

The hunk inserts at line ~322; on current main that table ends at line 429. .gitattributes sets merge=union on that file — which keeps both sides of a conflicting hunk, so a mid-table insert against a moved table interleaves and merges "cleanly" while being wrong. The branch predates the append-at-the-end rule.

Fix: rebase and move the row to the end of the table.

Minor

• --db-host ":5432" produces a malformed server option. isLocalHost("") is true, so an empty host reaches -o "-h -p 5432 -k …"; pg_ctl splits on whitespace and postmaster reads -p as the value of -h. Confirmed in a probe. Obscure (the default fills in 127.0.0.1:5432), but a one-line guard in startUserCluster would close it.
• pg_ctl status == 0 returns early without checking the port. A cluster left running from an earlier --ensure-db --db-port 5433 satisfies status, then EnsureDatabase burns 20 s on waitPGReady against the wrong port and reports a confusing failure. Reading the port from line 4 of postmaster.pid would make it precise.
• The server log path isn't in the error. startUserCluster writes -l ~/.mxcli/postgres/server.log, but the failure message carries only pg_ctl's stdout. Naming the log file makes the error self-service.
• Service-manager output is now fully discarded (_ = exec.Command(…).Run()). If service postgresql start explains itself, the user instead sees an initdb error from two steps later.

Checklist

Bug-fix requirements met: symptom row added, tests accompany the fix, .claude/skills/mendix/run-local.md (the sync source, not the regenerated cmd/mxcli/skills/ embed dir — correct choice), the docs-site page, and the CHANGELOG all updated. Single concern, single commit.

The mdl-examples/bug-tests/ requirement is the only apparent checklist gap and is correctly N/A — there's no MDL surface here and nothing for Studio Pro to validate. The backend-abstraction, grammar, and syntax/features_*.go sections don't apply either.

Thanks for the thorough stub-test coverage — the ready/non-ready service split and the idempotency cases are exactly the right seams to pin down.

🤖 Generated with Claude Code

[GG-O-BP]

Thanks for the thorough review, @ako — all seven points are addressed. Rebased onto main (the branch was CONFLICTING; it's now a clean single commit) and everything stays localized to startLocalPostgres/startUserCluster/resolveSuperuser.

Moderate

1. Timeout multiplication + dead pg_ctlcluster. Dropped the pg_ctlcluster placeholder entirely, and each service attempt now pays only a short serviceReadyTimeout (3 s, down from 20 s) — the single authoritative 20 s wait stays in EnsureDatabase. New TestStartLocalPostgres_NeverRunsPgCtlCluster asserts the entry can never run even when it's on PATH.
2. --auth-host=trust on loopback TCP. initdb now uses --auth-local=trust --auth-host=scram-sha-256, and the direct superuser connects over the 0700 socket dir (-h <sockDir>) instead of -h 127.0.0.1. So the passwordless superuser can only reach itself over the private socket; loopback TCP is no longer an authentication bypass. canConnectDB (app role, has a password) still works over TCP unchanged. Covered by TestStartUserCluster_InitdbAuthArgs and TestResolveSuperuser_PrefersClusterSocket.
3. fix-issue.md row mid-table. Rebased; the row is now appended at the very end of the symptom table (one-line diff).

Minor

• Empty --db-host (":5432"). Normalised to 127.0.0.1 in both EnsureDatabase and startUserCluster, so the server option is well-formed. TestStartUserCluster_EmptyHostNormalised reproduces the old malformed -h -p 5432 and asserts the fix.
• pg_ctl status==0 ignoring the port. New clusterStatus reads the port from line 4 of postmaster.pid; a running cluster is reused only when its port matches, otherwise it fails fast with an actionable message instead of burning the readiness wait on the wrong port. TestStartUserCluster_RunningPortGuard covers both branches.
• Server log path missing from the error. The start-failure error now names ~/.mxcli/postgres/server.log (TestStartUserCluster_StartErrorNamesLog).
• Discarded service-manager output. Captured and appended to the fallback error when the fallback then fails (TestStartLocalPostgres_SurfacesServiceOutput).

Each new test was proven to fail on the un-fixed code (reverted each change in a throwaway worktree and watched the matching test fail with the reported symptom). make vet, go build ./..., and go test ./cmd/mxcli/docker -count=1 are green.

[ako]

Review (round 2)

Thanks — I re-verified all seven points from the previous round and they are all addressed. Three new issues, one of which is a regression introduced by the fix for point 1.

What I verified — merged the branch into current main in a throwaway worktree:

• go vet ./cmd/mxcli/docker clean; all 20 subtests pass, on the branch and merged onto main.
• The fix-issue.md row is genuinely last now: 229 rows on main → 230 merged, no interleave.

Prior point	Status
Timeout multiplication	Fixed — serviceReadyTimeout = 3s inside the loop, the 20 s wait stays in EnsureDatabase. Worst case ~53 s, down from ~90 s
pg_ctlcluster placeholder	Removed, with TestStartLocalPostgres_NeverRunsPgCtlCluster pinning it
--auth-host=trust	Now scram-sha-256; superuser goes over the private socket. Test asserts both
fix-issue.md mid-table	Verified last, merges cleanly against current main
--db-host ":5432"	Normalised in both EnsureDatabase and startUserCluster
pg_ctl status port	postmaster.pid line 4 compared; mismatch errors immediately
Log path / service diagnostics	Both in the error now

---

Blocker

1. The short probe turns "slow but working" into a hard failure.

startLocalPostgres falls through to startUserCluster unconditionally once the 3 s probe expires. The old code returned nil after running the service manager and let EnsureDatabase's 20 s wait decide, so this is a regression. Confirmed with a stub whose pg_isready succeeds on the 9th probe (~4 s), i.e. a service-started server that comes up just after the short probe gives up:

err=<nil> calls="service\ninitdb\npg_ctl_start\n"

Against real PostgreSQL that pg_ctl start binds an already-occupied 5432 → could not bind IPv4 address … Address already in use → EnsureDatabase fails with a confusing error, and a stray ~40 MB cluster is left in ~/.mxcli/postgres.

The likely trigger is not exotic: crash recovery after a reaped container. PostgreSQL refuses connections while replaying WAL (the database system is starting up, pg_isready → not ready), easily past 3 s — and that is exactly the path mxcli run --local --setup --ensure-db takes from the SessionStart hook after a devcontainer is killed.

Fix — in startLocalPostgres, before the fallback:

// Something already owns the port (e.g. a service-started server still in
// crash recovery). Never initdb a competitor — EnsureDatabase's authoritative
// 20s wait decides.
if pingTCP(net.JoinHostPort(host, port), time.Second) == nil {
    return nil
}

A short probe should answer "should I look further?", never "should I build a competitor?".

Moderate

2. os.MkdirAll(sockDir, 0o700) does not tighten an already-existing directory.

MkdirAll is a no-op on an existing path — the mode argument is ignored. Pre-created the dir at 0755 and ran startUserCluster:

sock dir mode after startUserCluster: -rwxr-xr-x

That directory mode is the entire access control behind --auth-local=trust; at 0755 any local account connects as the postgres superuser, which is the vulnerability the previous round closed, reached by a different route.

Fix: os.Chmod(sockDir, 0o700) after MkdirAll (or Stat-and-refuse). The test should pre-create the dir at 0755 and assert it was tightened, rather than only exercising the fresh-dir path.

3. The 0700-socket control lives only in a command-line flag, not in the cluster.

-k <sockDir> is passed through pg_ctl -o, so the hardening holds only for a server mxcli starts. Any other start reverts to the build's compiled unix_socket_directories (/tmp on many builds, world-writable), and --auth-local=trust applies to that socket too. The code nudges users down exactly that path — the mismatched-port error says stop it with pg_ctl -D … stop, and the natural next step is a plain pg_ctl … start.

Fix: persist it in the cluster's own config — write unix_socket_directories, listen_addresses, and port into postgresql.conf after initdb (or initdb -c unix_socket_directories=… on PG 15+), so the property survives a start mxcli did not perform.

Minor

• No upgrade path off the earlier revision's cluster. A data directory initialized by the earlier --auth-host=trust commit is reused silently — PG_VERSION is the only check — so the hardening does not apply retroactively. That includes anyone who tested the earlier build. A CHANGELOG line ("delete ~/.mxcli/postgres if you ran the pre-review build") or a pg_hba.conf check on reuse would close it.
• _ = logPath in TestStartLocalPostgres_SurfacesServiceOutput is a leftover; dir, _ := newStubPATH(t) reads better.
• The docs don't mention that the cluster persists across sessions, where server.log lives, or how to stop/remove it. pg_ctl -D ~/.mxcli/postgres/data stop currently appears only inside an error string.
• The branch is ~24 commits behind main and CHANGELOG.md conflicts on merge (fix-issue.md merges cleanly). Worth a rebase before merging.

Checklist

Bug-fix requirements met: symptom row appended at the end of the table, tests accompany the fix, the sync source skill (.claude/skills/mendix/run-local.md, not the regenerated embed dir), the docs-site page, and the CHANGELOG all updated. Single concern, single commit. mdl-examples/bug-tests/ remains correctly N/A — no MDL surface. The --ensure-db flag help needed no change (it never mentioned sudo).

The stub-test coverage is a real strength here — the port-guard and idempotency cases in particular made the two remaining defects cheap to demonstrate rather than argue about.

🤖 Generated with Claude Code

[GG-O-BP]

Thanks, @ako — round 2 is fully addressed in 8efbd329.

Blocker

1. Slow service startup no longer creates a competitor. After the short readiness probe, startLocalPostgres now checks the requested endpoint with pingTCP(net.JoinHostPort(host, port), time.Second). If anything already owns it, control returns to the authoritative 20-second wait in EnsureDatabase; initdb and pg_ctl start are not called. TestStartLocalPostgres_OccupiedPortSkipsFallback uses a real TCP listener plus a failing pg_isready stub and proved the old branch took the fallback before the guard was added.

Moderate

2. An existing socket directory is tightened. MkdirAll is followed by explicit os.Chmod(sockDir, 0o700). The regression pre-creates it as 0755 and asserts 0700 afterward.

3. The endpoint is now cluster configuration, not a one-shot pg_ctl -o flag. A single idempotent managed block at the end of postgresql.conf persists listen_addresses, port, unix_socket_directories, and unix_socket_permissions = 0700; pg_ctl start no longer passes -o. The test asserts the settings, mode 0600, one managed block, and byte-stable repeat application. I also verified this against PostgreSQL 18.4 by stopping the bootstrapped cluster, restarting it with plain pg_ctl -D ... -w -l ... start, and reconnecting as the app role.

Minor and follow-through

• Pre-review host-trust clusters are detected from pg_hba.conf and refused with an explicit stop/remove/rerun path; CHANGELOG also calls out that cleanup.
• Removed the _ = logPath leftover.
• Both run-local docs now explain persistence, server.log, stopping, and destructive removal.
• Rebuilt the branch as exactly one commit on current main; the PR is mergeable with no CHANGELOG conflict.
• Also validate/canonicalize the port before writing PostgreSQL configuration and require both the live port and private socket directory to match before reusing a running cluster.

The final tree passed make build, make vet, make test, package tests, targeted race tests, git diff --check, and the PostgreSQL 18.4 end-to-end bootstrap/restart checks above.

[GG-O-BP]

Final self-audit amended the single commit to 01cd6c59 (the round-2 fixes described above are unchanged). Two narrow edge-case corrections were included before handoff: endpoint validation now precedes the persisted-config write so a rejected running-cluster request cannot alter its next restart, and fallback tests allocate an unused loopback port instead of assuming CI port 5432 is free. The exact final commit then passed make build, make vet, make test, targeted race tests, and git diff --check.