fix(bin): surface certsync health cannot-run states - #73
Merged
Conversation
added 2 commits
August 4, 2026 22:57
run_bounded's `2>/dev/null || true` discarded the wrapped command's exit status, so certsync_health_reason's docker-permission-denial path (no root/sudo/docker-group by design) collapsed into the exact same silent no-wake outcome as a confirmed-healthy read. The check failed for 6.5 hours while reporting fine the whole time. run_bounded now returns the wrapped command's real exit status (its one other caller, the Bridge fetch in the main loop, never reads it, so it is unaffected). certsync_health_reason uses that to report every inability to read certsync's status - missing docker/jq, a failed status command, empty output, invalid JSON, a missing healthy field - as its own "cannot run" check wake, sharing the existing unhealthy path's resurface-dedup instead of going silent. Only a missing project or compose file (certsync not deployed here) stays quiet, since that is a legitimate N/A, not a failure.
Freudator86
pushed a commit
that referenced
this pull request
Aug 5, 2026
The certsync health check ran `docker compose exec -T certsync certsync status`. On this fleet's own accounts - deliberately not in the docker group - that exec is denied at unix:///var/run/docker.sock, so since the honesty fix (PR #73) the check honestly but permanently reports "cannot run: status command failed", giving no read at all on whether certsync is well. Read certsync's status directly off the host instead. certsync now exposes its heartbeat JSON and sqlite state DB under a readable host bind mount (see the certsync repo's docs/deploy.md, "State host path"); `certsync status` computes healthy/reason purely from those two files plus the daemon-state argument, so certsync_health_reason runs certsync's own build_status via python3 against them (PYTHONPATH=$FM_CERTSYNC_SRC, default $FM_CERTSYNC_PROJECT/src). No docker socket, no exec, no docker-group membership, no socket proxy - the read needs no docker access at all, and reproduces the exact healthy/reason JSON the exec produced. Reading frozen files loses the one liveness signal exec gave for free: exec failed when the container was down. Reinstate it as a heartbeat-freshness bound (FM_CERTSYNC_HEARTBEAT_MAX_AGE, default 7200s = 2x the 3600s max sync interval, 0 disables): a healthy:true reading whose heartbeat has gone stale reads as unhealthy, never quiet, so a stopped container or a run of failing syncs can never read as healthy off stale files. This also closes a pre-existing blind spot where failing syncs read healthy under the exec path. Preflight now checks python3/jq and the certsync source tree; each failure keeps its own distinct "cannot run: ..." reason. Tests rewritten to the file-read model with added coverage for the freshness gate and the source-unavailable preflight. docs/configuration.md updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Stop firstmate's certsync health check (bin/fm-watch.sh: certsync_health_reason) from reporting healthy when it cannot run at all. Reproduced the reported defect: this account has no root/sudo/docker-group by design, so docker compose exec fails with a permission denial (nonzero exit, empty stdout); the shared helper run_bounded discarded exit codes via '... 2>/dev/null || true', so that failure collapsed into the exact same silent no-wake outcome as a confirmed-healthy reading. Fix: run_bounded now returns the wrapped command's real exit status (its only other caller, a fire-and-forget git fetch in the Bridge-inbox loop, never reads that status, so it is provably unaffected - proven with a new dedicated test plus the existing Bridge-inbox test suite passing unchanged). certsync_health_reason now treats every inability to read certsync's status - missing docker/jq, a failed status command, empty output, invalid JSON, a missing healthy boolean - as its own distinct 'cannot run' check wake, reusing the existing hourly resurface-dedup so it neither goes silent nor spams. Only a missing project/compose file (certsync not deployed on this host) stays legitimately quiet. Deliberately chose 'report as its own distinct wake' over inventing a new alarm channel or refusing outright, since the check already rides the watcher's existing heartbeat/check-wake infrastructure and a human should decide what to do about a docker permission gap that has a known bounded operator remediation. Did NOT fix the underlying docker permission denial - the account intentionally has no docker-group access and I was told not to propose adding it - instead documented a docker-socket-proxy runbook for the privileged operator who owns that fix (rejecting a same-shape differently-named raw-socket group as not actually narrower). Updated the existing test that had pinned the OLD buggy silent-absorb behavior (test_heartbeat_certsync_unknown_absorbed) to assert the new distinct cannot-run wake, and added a permission-denied regression test reproducing this exact defect end-to-end through the real watcher subprocess, plus a focused run_bounded contract test. Also updated docs/configuration.md to describe the corrected contract.
What Changed
cannot runcheck wakes for missing tools, failed status commands, empty output, invalid JSON, or missinghealthydata instead of treating unreadable status as quiet.run_boundednow preserves the wrapped command's exit status while still discarding stderr, so callers can distinguish command failure from empty successful output.run_bounded, certsync cannot-run watcher triage, and the docker permission-denied failure path, with docs updated for the corrected health-check contract.Risk Assessment
✅ Low: Captain, the change is focused, preserves the only shared helper call site’s behavior, and makes certsync unreadable states fail visible without broad watcher changes.
Testing
Captain, I inspected the certsync/run_bounded diff, ran the focused helper contract test, the full watcher triage suite, and the Bridge inbox suite; then I manually reproduced the permission-denied docker path through the real watcher and drained queue. One initial manual transcript command was rejected by the local guard due to an
rm -rfcleanup shape, then rerun successfully without deletion. The working tree was clean afterward, and no UI screenshot was needed because this is watcher/CLI behavior.Evidence: Manual certsync permission-denied watcher transcript
watcher stdout: check: certsync health: cannot run: status command failed (exit 1) drained wake queue: 1785884768 1 check certsync-health check: certsync health: cannot run: status command failed (exit 1)Evidence: Watcher triage regression test log
ok - heartbeat surfaces a docker-permission-denied certsync status instead of silently reporting healthyEvidence: run_bounded helper contract log
ok - a caller that never reads run_bounded's exit status (the Bridge fetch call site's shape) is unaffected by the real exit code now propagatingEvidence: Bridge inbox suite log
ok - origin ack clears the check without mutating a stale local working treePipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bin/fm-session-start.shgit diff --stat deb3593458529e439b11d7d0a183be12f3bb3750..37d807208b32eed3116acaa55cadccd3d6cf24d0git diff --name-only deb3593458529e439b11d7d0a183be12f3bb3750..37d807208b32eed3116acaa55cadccd3d6cf24d0rg -n "certsync_health_reason|run_bounded|test_heartbeat_certsync|permission" bin test tests docstests/fm-watch-run-bounded.test.sh | tee /tmp/no-mistakes-evidence/01KZ7G1T44XT852X50BFSCZFJT/fm-watch-run-bounded.logtests/fm-watch-triage.test.sh | tee /tmp/no-mistakes-evidence/01KZ7G1T44XT852X50BFSCZFJT/fm-watch-triage.logtests/fm-watch-bridge-inbox.test.sh | tee /tmp/no-mistakes-evidence/01KZ7G1T44XT852X50BFSCZFJT/fm-watch-bridge-inbox.logManual watcher subprocess check with a fakedocker composepermission denial, followed bybin/fm-wake-drain.sh, recorded at/tmp/no-mistakes-evidence/01KZ7G1T44XT852X50BFSCZFJT/manual-certsync-permission-denied.txtgit status --short✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.