Conversation
The watcher now covers the static deploy runners beside the CI pools; describe the check names, the failure shapes each reads, the path override, and the runbook the alert carries.
DJRHails
left a comment
There was a problem hiding this comment.
Automated review — findings posted inline. This file is glassine-encrypted, so the diff GitHub shows is ciphertext; the comments below anchor on the data line and describe the plaintext section they refer to without quoting it. P2 fixed in a follow-up commit; P3s fixed alongside since each is a one-line edit in the same file.
- F1 (P2): list the deployment-runner checkout (or the compose path override) among the runner-pool-host prerequisites; without it the new conf check alerts on every run. - F2 (P3): the layout intro now counts four kinds of check. - F3 (P3): name the never-registered example as the since-retired service it was. - F4 (P3): the testing paragraph names the deploy-runner scenarios the regression test now covers.
Review SummaryDocs-only change to a glassine-encrypted skill file; the plaintext was reviewed from the checkout and its claims checked against the watcher change it documents (touchstone#4581, still open) and the live runner registrations. Check names, the path override, the failure shapes, the sweep-ending behaviour on an API failure, the runbook, and the 16-offline count all match the script and the current API state.
Envelope: sops metadata intact, 7 age recipients matching the repo-wide rule in Verified: Ordering note: the PR body already says to merge after the watcher change lands; touchstone#4581 is still open, so hold this until it merges or the skill describes checks that do not exist yet. Fix commit: 335be53 Verdict: approve |
Documents the deploy-runner checks the uptime watcher gains in touchstone#4581: the check names, the two failure shapes each reads, the path override, and the runbook the alert carries. Skill content only; merge after the watcher change lands.
via gantry