Skip to content
This repository was archived by the owner on Sep 17, 2026. It is now read-only.

Make pg_isready probe TCP, which is what dependents connect to - #27

Merged
Peeja merged 2 commits into
mainfrom
claude/pg-healthcheck-tcp
Sep 17, 2026
Merged

Peeja merged 2 commits into
mainfrom
claude/pg-healthcheck-tcp

Conversation

@Peeja

@Peeja Peeja commented Sep 17, 2026

Copy link
Copy Markdown

⏭️ Safe to merge without waiting for CI — no. Watch e2e, and ci's guards.

Refreshed on every push. Checks are not required yet, so this is advice, not a gate.

check verdict
e2e Load-bearing. This changes when containers in the stack are considered healthy. It is the suite the bug was found in.
ci → guards Load-bearing. Runs the new check-pg-healthchecks.sh for the first time in CI.
ci → 13 unit jobs unit smelt is load-bearing (smelt/pkg/generate is changed). The other 12 are skippable — go list -deps reaches no other module, and no other module's packages are touched.
itest Skippable. itest hilt/ingot boot their own stacks, but neither hilt's nor ingot's compose postgres is on their critical path in a way this changes — it only makes a healthcheck stricter, so the worst case is a slower boot, not a failure. The weakest claim in this block; if itest goes red, believe it over me.
images ×8 Skippable. No Dockerfile changed.

What this costs if it's wrong: the checks still run; we just don't wait. If an ignored check goes red after merge, main is red.


TestUploadAndRetrieve/filesystem has failed 2 of the last 40 e2e runs, naming a different container each time — upload-1 on run 160 (main ff2f794d), plc-1 on run 182. I called that a generic startup race this morning. It isn't. It's one bug with a precise mechanism, and plc-postgres's own log against plc's crash shows it:

16:55:27.593  temp server: listening on Unix socket ONLY
16:55:27.633  temp server: ready to accept connections   ← healthcheck goes GREEN
16:55:29.441  temp server: shut down
16:55:29.887  real server: listening on IPv4 0.0.0.0:5432 ← TCP finally exists
16:55:30.531  plc exits: ECONNREFUSED 172.18.0.6:5432

The healthcheck was green 2.25 seconds before the port existed.

Why

test: ["CMD-SHELL", "pg_isready -U plc -d plc"]      # no -h → Unix socket

Without -h, pg_isready probes the Unix socket. The official postgres image runs initdb against a temporary server started with listen_addresses='' — socket up, TCP refused by design — so the healthcheck passes during initialisation. A dependent with condition: service_healthy then starts into a window where the port it connects to doesn't exist.

plc already had depends_on: plc-postgres: {condition: service_healthy}. The ordering was never wrong; the readiness signal was. That's also why the victim differs run to run — whichever dependent happens to boot inside the window.

The fix, and its scope

-h 127.0.0.1 on all six sites — five compose files plus smelt/pkg/generate/compose.go, whose output generated/compose/piri.yml is gitignored and regenerates. I verified the generator actually emits it, not merely that the source changed:

generated/compose/piri.yml:105:  - pg_isready -U piri -d postgres -h 127.0.0.1

Postgres was the only member of this class, checked rather than assumed — every other healthcheck in the tree is HTTP over localhost/127.0.0.1 or redis-cli ping, all TCP by construction, so none can have the socket-versus-port gap.

The guard is the part that lasts

check-pg-healthchecks.sh, wired into the existing guards job. No list in it: every pg_isready either names a host or is a bug, so nothing needs updating when a service is added. It covers *.go as well as *.yml because the generator builds one of these strings.

It also fails when it finds nothing at all — a check that silently guards an empty set is the exact failure mode it exists to prevent.

Verified in both directions, not just the green one: 6 FAILs against the tree before the fix, 6 oks after.

⚠️ What I have not verified

That e2e now passes reliably. A 5% flake cannot be shown fixed by one green run, and there's no Docker here to exercise it. The mechanism is proven from the logs; the frequency is not. If it recurs, the log to check is whether the failing dependent's postgres shows the temp-server/real-server split again — if it does, this fix didn't take and the next suspect is start_period rather than the probe.

smelt/pkg/generate tests pass. The three pkg/stack failures in my sandbox are no such file or directory: /var/run/docker.sock and reproduce identically on unmodified main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt


Generated by Claude Code

TestUploadAndRetrieve/filesystem failed 2 of 40 e2e runs, naming a different
container each time -- upload-1 on run 160 (main, ff2f794), plc-1 on run 182.
That looked like a generic startup race. It is not; it is one bug with a precise
mechanism, and plc-postgres's own log against plc's crash shows it:

    16:55:27.593  temp server: listening on Unix socket ONLY
    16:55:27.633  temp server: ready to accept connections   <- healthcheck green
    16:55:29.441  temp server: shut down
    16:55:29.887  real server: listening on IPv4 0.0.0.0:5432
    16:55:30.531  plc exits: ECONNREFUSED 172.18.0.6:5432

Without -h, pg_isready uses the Unix socket. The official postgres image runs
initdb against a temporary server started with listen_addresses='', so the
socket is up while TCP is refused, and the healthcheck passes during init. A
dependent with condition: service_healthy then starts into a window where the
port it connects to does not exist yet -- here 2.25 seconds wide, which is why
it is intermittent and why the victim differs run to run.

Six sites, all of them: five compose files plus smelt/pkg/generate/compose.go,
whose output generated/compose/piri.yml is gitignored and regenerates. Verified
the generator actually emits it, not just that the source changed.

Postgres was the only class member: every other healthcheck in the tree is HTTP
over localhost/127.0.0.1 or redis-cli ping, all TCP by construction, so none can
have the socket-versus-port gap. Checked rather than assumed.

check-pg-healthchecks.sh keeps it fixed. The rule has no list in it -- every
pg_isready either names a host or is a bug -- so nothing needs updating when a
service is added, and it covers Go as well as YAML because the generator builds
one of these strings. It also fails when it finds nothing at all, since a check
that silently guards an empty set is the failure it exists to prevent. Verified
in both directions: 6 FAILs before the fix, 6 oks after.

Not verified here: that e2e now passes reliably. A 5% flake cannot be shown
fixed by one green run, and CI has no Docker-less way to exercise it. The
mechanism is proven from the logs; the frequency is not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Chasing #25's e2e red to the bottom found a real bug. pg_isready without -h
probes the Unix socket; the official postgres image runs initdb against a
temporary server started with listen_addresses='', so the probe passes while TCP
is still refused. plc-postgres's log against plc's crash shows the healthcheck
going green 2.25 seconds before the port existed, and plc booting into the gap.

That explains the shape that made it look like a generic race: plc already
declared condition: service_healthy, so the ordering was never wrong -- the
readiness signal was, and whichever dependent happened to boot inside the window
was the one that failed. Hence a different container each run.

#27 fixes six sites and adds a list-free guard that also covers Go, because the
generator builds one of these strings, and that fails if it ever finds nothing
to check. Postgres was the only class member; every other healthcheck is HTTP
over localhost or redis-cli ping, TCP by construction.

Recorded with what is still unproven: the mechanism, yes; the frequency, no. A
5% flake cannot be shown fixed by one green run.

Also: main is 95e8366, #25 and #26 merged, the layer cache is live.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…heck

The previous commit updated Current State but its companion edit failed before
writing, so this page still claimed five open PRs and #25 unmerged. Now: #27
only, with #25 and #26 merged and the cache numbers carried across.

Names the one thing on that list the agent genuinely cannot do rather than has
not done: checking the cache against GitHub's 10 GB per-repository limit needs
gh cache list or the Actions cache API, and neither is reachable from here. It
matters because eviction is LRU and an overflowing cache degrades quietly back
to cold builds, which is the silent regression shape this repo keeps deleting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
The guard failed CI on its own step name. ci.yml:59 reads "every pg_isready
healthcheck probes TCP", the grep matched the bare word, and the check reported
a workflow file as an unhosted healthcheck. A guard that cannot tell running a
thing from naming it is not guarding the thing.

Anchored to a preceding quote, so the name has to start a quoted command
string. That keeps .github/ in scope rather than excluding it, which matters
because an Actions services: block can carry --health-cmd "pg_isready ..." and
excluding the directory would have made a real blind spot to dodge a false
positive.

The gap it does leave, named in the header rather than papered over: an
unquoted invocation like `--health-cmd pg_isready`. Nothing here writes one,
and the alternative -- matching every mention -- is exactly what just misfired.

How I missed it: I verified the guard in both directions, then wired it into
ci.yml, and did not run it again. The verification was of a tree that no longer
existed by the time I pushed. Re-verified against the final tree this time, and
on three cases rather than two: it still FAILs when -h is dropped from plc, it
does not trip on an added prose mention, and the tree as pushed is clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
#27's check-pg-healthchecks.sh went red in CI on ci.yml:59 -- the step name
"every pg_isready healthcheck probes TCP" contains the word, and the grep
matched a mention rather than an invocation. Anchored to a preceding quote,
which keeps .github/ in scope: an Actions services: block can carry
--health-cmd "pg_isready ...", so excluding the directory would have bought a
blind spot to dodge a false positive.

The durable part is not the regex. The guard was verified in both directions and
then wired into ci.yml, with no re-run after -- so the verification was of a tree
that did not exist by push time. Rule 5 already says check both directions; it
now also has to say check the final state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

Peeja commented Sep 17, 2026

Copy link
Copy Markdown
Author

[From Claude:] guards went red, and it was the guard failing on its own step name. Fixed in a8a6040b.

FAIL ./.github/workflows/ci.yml:59 pg_isready with no -h probes the Unix socket

ci.yml:59 is - name: every pg_isready healthcheck probes TCP. The grep matched the bare word, so the check reported a workflow file as an unhosted postgres healthcheck. A guard that cannot tell running a thing from naming it is not guarding the thing.

The fix

Anchored to a preceding quote, so the name has to start a quoted command string. Deliberately not done by excluding .github/: an Actions services: block can legitimately carry --health-cmd "pg_isready …", so excluding the directory would have bought a real blind spot in exchange for dodging a false positive.

The gap that remains is named in the script's header rather than papered over: an unquoted invocation, e.g. --health-cmd pg_isready. Nothing in this repository writes one, and the alternative — matching every mention — is exactly what just misfired.

How I missed it, which is the part worth keeping

I verified the guard in both directions, then wired it into ci.yml, and did not run it again. So what I verified was a tree that no longer existed by the time I pushed — and the thing that broke it was the very line that wires it in.

Rule 5 already says a guard proves nothing until you have seen it fail on the defect it is for. It now also has to say: verify it against the tree you are actually pushing. That is on the wiki.

Re-verified on three cases this time, against the final tree:

-h dropped from plc FAIL … compose.yml:51, exit 1
a prose mention added to ci.yml exit 0 — no longer trips
the tree as pushed exit 0, 6 oks

Unchanged

The postgres fix itself is untouched — six sites, -h 127.0.0.1, generator output checked. And still unverified: that the flake is gone. One green e2e cannot show a ~5% failure rate fixed. The mechanism is proven from plc-postgres's log; the frequency is not.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 17, 2026
22/22 on a8a6040, guards included and e2e passing. Recorded with what that is
actually worth: at a ~5% base rate a single green e2e run had a ~95% chance of
happening anyway, so it is close to no evidence. The mechanism from
plc-postgres's log is what justifies the fix; the run is not.

Also a third cache data point, and the first from a real branch push rather than
a same-commit re-run: 8 images in 2m27s, seven of them 3-7s and
indexing-service alone 1m55s. That is squarely the "minutes rather than seconds"
the entry predicted, so the caveat was calibrated rather than lucky.

The one cold image has a cause worth recording: the previous push's run was
cancelled mid-build by cancel-in-progress, so that scope's cache export never
finished. A rapid re-push can leave a partially warmed cache, which means no
single job's timing is a clean measurement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
@Peeja
Peeja merged commit 24b18ee into main Sep 17, 2026
22 checks passed
@Peeja
Peeja deleted the claude/pg-healthcheck-tcp branch September 17, 2026 19:02
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…st clears

Petra approved the lot on 2026-09-17. Rather than let the reasoning go with the
list -- Recently cleared gets pruned -- the durable parts moved to Current State:

Rule 2 gains its worked example. Pinning an image the tree already pins means
reusing that digest rather than resolving a fresh one, because a fresh resolve
puts two builds of one tag in one repository, which is the agreement failure
rather than the reproducibility one.

Known debt gains two entries. Image pins inside subtree prefixes are a standing
cost every future subtree pull carries, accepted because unlike a lint config an
image pin has no root-level alternative -- worth meeting at the next resync
rather than rediscovering as a conflict. And the four guard scripts are recorded
as the agent's own initiative and approved, named rather than assumed welcome
because an earlier squashed-subtree guard was declined.

The measured case against a Go image guard was already on Current State, so it
needed no migration.

Also: no check-name change is outstanding now. #16's is resolved, #21 is closed,
and #27 adds a step to the existing guards job rather than a new check. That
line is worth re-reading before required checks are turned on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
main is 24b18ee. The postgres healthcheck fix and check-pg-healthchecks.sh are
live, and e2e has passed twice on the fix -- once on a8a6040, once on main
post-merge.

Stated as what it is rather than as vindication: at a ~5% base rate two
consecutive passes had a ~90% chance of happening even unfixed, so they rule out
very little. The mechanism is what justifies the fix. The page now says to
record passes as they accumulate and to treat a recurrence as informative rather
than as noise, since that is the observation that would actually falsify this.

Also a fourth cache data point: e2e on main post-merge in 6m18s against a
13m10s baseline. The first run after a merge was expected to be partly cold and
was not, which is the cache working across the PR-to-main scope boundary.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
This page said every subtree was resynced to its upstream head. True when
written, false now -- sprue, libforge and ucantone all committed today.
Measured by comparing each git-subtree-split recorded in main's history against
that repository's live origin/main: ingot 25 behind, hilt 6, smelt 4, sprue 4,
delegator 4, piri 2, and swarf, indexing-service, piri-signing-service and
forgectl at the import point.

Two rows matter beyond the count.

smelt 96fc212 is the same bug #27 just fixed, found upstream two days earlier --
its message names pg_isready going green against the temporary initdb server.
#27 derived it independently from the plc-postgres log, so the account holds,
but it was already known in the polyrepo and nobody here had looked. Recorded as
not-novel rather than left reading as a discovery. The fixes are complementary:
upstream makes the consumer wait and also fixes an openbao raft-leader race we
have not hit; #27 fixes the signal. They do not conflict, line 150 versus
163-183.

ingot #166 is CI itest sharding -- the same work as closed #21 -- and arrives
with the final pull regardless.

Petra's policy, recorded: no regular pulls, one final pull at the end, pull
early where upstream fixes something we hit. smelt is the one row meeting that
bar now, flagged as a recommendation rather than started, since an
agent-initiated subtree pull is a rule 7 operation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants