HOMELAB-1688: feat(routines): registry, claim lock, receipt, watchdog, R-06 rollup #567
No reviewers
Labels
No labels
operator-review
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Homelab/infra-core!567
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-1688-routines"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
LD4.4 (HOMELAB-1688), slot 6. Built to
.rpiv/artifacts/epic-ld4/design-LD4.4-routines.md; the orchestrator rulings in the ticket's 2026-10-02 comment supersede the brief where they differ.What this PR is
The routines layer, in three homes (contract §11, no new store): the registry is GitOps YAML, the claim lock is a Plane execution issue, the receipt is a JetStream payload behind a port.
routines/registry.yaml,registry.go,validate.go,types.go,schedule.go): all 22 corpus routines registered; 3 active (R-03 claim-reaper, R-06 cost-rollup-daily, R-19 plane-cycle-flip — each names its mechanism and itsfirstPeriodSeconds), 19pausedwith machine-checkabledep:/registry:reasons. The seven refusal classes of brief §2.1 plus the structural faults, each with a line number and a teaching message.claimlock.go,runner.go,ledger.go):Forbid= "no second execution issue while one is unclosed" (Plane state is the arbiter); a run with no receipt ismissed, neversuccess; the escalation ladder reads the ledger.watchdog.go,cmd/routines): P-ROUT-001/002/010, fail-closed — an unreadable ledger pages and never passes.policy.go↔policy.md, parsed back by a test).rollup.go,sessions.go): leg A attribution key (sandbox_env,unboundis a value and a finding), leg B read-only/redacted transcript reader with nested spend, leg C the ticket→PR→run denominator join key + a sink for teardown's existing telemetry map; the budget circuit-breaker emits the finding that makes custodian'sP-CUST-007real.cmd/routines:-check/-watchdog, exit0/1/2.Territory
Mine (new):
lab-director/dispatcher/routines/**,lab-director/dispatcher/cmd/routines/**.Shared hunk (localized, listed as required):
lab-director/dispatcher/Dockerfile— builds./cmd/routines, bakesroutines/registry.yaml, and runsroutines -checkinside the build so a refused registry fails the image build. Noconfig/,identity/,cmd/dispatcher/,claims/, orteardown/file is touched.Not mine (untouched):
direction/,notifier/,intake/,context_plane.go(1685);persona/,director/,comms/(1687); phase tags + span-parent fixes (4.5d); cockpit rendering (3.4).Boundary with 4.5d, written into both tickets: 4.4 owns the
sandbox_envattribute key + the R-06 rollup; 4.5d owns the phase key + span-parent fixes. This package copies a phase tag and never invents one (an untagged sample stays untagged).Acceptance lines
registry_test.goTestRefusalClasses(each class as a mutation of one valid entry; asserts fault class + line)parity_test.go+testdata/corpus-table.md(the R-01…R-22 table transcribed from the corpus)Forbidvia the Plane issuerunner_test.goTestA3ForbidConcurrencyTestCatchUpPolicyDefaults+ the loader's refusalTestA5ReceiptContract,TestA5EscalationLadderwatchdog_test.go(finding → episode; page on an unreadable and on a log-only ledger)TestRealRegistryLoads+routines -checkcost_test.go(per-env tokens/$, reconcile tolerance, unbound visible, budget kill citing P-CUST-007, read-only + redacted reader, in-process pin freshness)TestRegistryIsReadOnlyData+ErrSecretInFilecasego test -race ./...andgo vet ./...green inlab-director/dispatcher. Not one test opens a socket: every seam (ExecutionPlane,Ledger,Publisher,MetricSource,SessionSource) has a fixture.Deviations from the brief (each deliberate, each reviewable)
activeentries only. A paused routine may have a mechanism/identity that does not exist yet — that is whatpausedmeans — but a paused entry may name a missing role only when itspausedReasoncontainsdep:role-<role>.pausedReasonaccepts a comma-separated token list (a routine can wait on a role and a dependency; a single-token schema pushes the second fact into prose).analyze-sessionsis NOT vendored. The five-script tool does not exist on this machine (the brief records it absent; a filesystem search by this builder found no copy — nothing under~/.pi/agent/skills,pi-extensions/pi-skills, or anywhere in~/repos). Rather than vendor a reimplementation and call it upstream, leg B is an in-package Go implementation of the same contract (read-only, redacted, nested spend included, deterministic JSON). C4's "5 scripts at a pinned revision" line is NOT satisfied and is recorded as such inroutines/README.md.interpreter: go:in-process,scriptRef.path+ sha256), re-derived byTestInProcessPinsMatchTheCode, which fails until the pin is updated. That is why R-03 pinscustodian/reclaim.go(untouched here) and R-06 pinsroutines/rollup.go.gatedandauthorityare orthogonal: R-02 is bothgated: true(its kill/redispatch waits) andauthority: care-dial(its output never gates work). The brief's §2.1 table reads them as one axis; they are not, and R-02 is the case that proves it.Honest limits (also in
routines/README.md)nats.goadapter is not written:Ledgeris the port,Publisher/Readerthe seams,NewStreamLedger(nil)the documented log-only mode whoseListrefuses rather than reporting an empty ledger. v1 read path isFileLedger(JSONL). This is the remaining wiring step for the heartbeat CronJob (whose prod manifest shipssuspend: truefor exactly this reason).python3in the workload — named in the absorption runbook as the blocking precondition.cmd/routinesreads a file ledger only (same reason as 1).Note on this PR's base
epic/LD4existed only locally; this PR's base is the branch pushed to origin for review (cd6db02, which is where the local epic branch pointed). If the orchestrator prefers a different base, retargeting is a one-line change.For the landing worker
Dockerfile(additive, build-time-check) and the registry data itself (parity test is the guard).VERDICT: CHANGES REQUESTED
Reviewed at head
82e5021cb0936a867cac14cc513c05c2a2e57550(baseepic/LD4@cd6db02, true merge-base, no rebase needed) in a fresh context, primary sources only (diff, HOMELAB-1688 + its comments incl. the 2026-10-02 adjudication, design briefdesign-LD4.4-routines.md, repo READMEs, contract §11).Independently reproduced:
go test -count=1 ./routines/... ./cmd/routines/...andgo vet ./routines/... ./cmd/routines/...inlab-director/dispatcher→ green. Commit status on82e5021: 5/5 success (atlantis, CI Review/{helm-validate, receiver-test, alert-receiver-test, dispatcher-test 2m0s}).I read all 10 source files, all 7 test files,
registry.yaml,policy.md,testdata/, and the Dockerfile hunk; recomputed every pin by hand; and reproduced the blocker below with the shipped binary.BLOCKER 1 — the watchdog's never-ran arm (P-ROUT-002 / A6 / the verify line) is unreachable in the only shipped invocation
cmd/routines/main.go:141passesRegisteredAt: time.Now().UTC().watchdog.go:195-197uses that as the registration anchor, andwatchdog.go:218-231suppressesFindingNeverRanwhilenow - registered < firstPeriod()— permanently true whenregistered == now. The binary therefore can never emitroutine-never-ran.Reproduced against the committed registry:
The second run is the sharper evidence: three active routines, two of them with zero receipts, and zero never-ran findings. The fleet's own documented failure — the hermes cron ticker dead on 2026-07-25 and unnoticed for ten weeks (brief §0.1/§3.1.4, the stated reason this watchdog exists) — is exactly the state that reads
0 findings / exit 0 / unreadable: falsehere, indefinitely.Why this is a blocker rather than a nit:
watchdog.go:42,policy.go:25-27andpolicy.md:16all make P-ROUT-002 the mechanism that turns that line mechanical. In the deployed path it is inert; the only thing that produces the finding is a unit test injecting its own anchor.watchdog_test.go:TestA6FirstPeriodNeverRaninjectsRegisteredAt: clk.at;cmd/routines/main_test.go:TestWatchdogExitsnever exercises the CLI's anchor. So the suite is green while the production path is blind — claim-honesty as well as correctness.WatchdogConfig.RegisteredAt's own doc comment (watchdog.go:118) says "Zero means 'use the oldest receipt, or now'"; the implementation only ever does "now". The comment describes an unimplemented half.Direction for the fix worker (either is acceptable; pick one and add a regression test that drives
run(...), not the library, so the CLI anchor is covered):-registered-atflag, orregisteredAtin the registry / a ConfigMap annotation defaulting tometadata.creationTimestamp; orwatchdog.go:118already promises.Plus a backstop that does not depend on the anchor at all: an
activeroutine with zero rows, more thanfirstPeriodSecondsafter its registry revision landed, must not read as healthy.CONCERN 1 — a missing ledger is silently invented, so "the ledger is gone" is not fail-closed
ledger.go:464-476(NewFileLedger) opens withO_CREATE, andcmd/routinesusesFileLedger(main.go:132). A wipedemptyDir/PVC therefore becomes an empty readable ledger →0 findings, exit 0. It never reaches theErrLedgerUnreadablepage (P-ROUT-010) that the PR body, README andpolicy.md:22all rest on. Verified:-ledger-file <nonexistent-but-creatable>→ exit 0; only a non-creatable path pages (thet.TempDir()case inmain_test.go:TestWatchdogExits). Filed as a concern rather than a second blocker because a correct anchor (Blocker 1) would make the never-ran rule break the silence; suggested hardening is to require the file to exist, or to page when the ledger has zero rows while the registry declares active routines.CONCERN 2 — D1's denylist has a dead entry
registry_test.go:355-356:for _, forbidden := range []string{"openbao:", ...} { if strings.Contains(...) && forbidden != "openbao:" {— the first pattern is excluded from its own loop, so four listed patterns assert three. Harmless today (the real protection isscanForSecrets+ErrSecretInFile,validate.go:348-372, genuinely asserted atregistry_test.go:300-309), but the test's stated intent ("asserts the registry carries references only") is broader than what it checks. Either drop"openbao:"from the list or assert it with its real intent (a value after the prefix is the leak, not the prefix).CONCERN 3 — A1's fault-class assertion is an OR
registry_test.go:96:if !err.Has(FaultUnpinnedScript) && !err.Has(FaultMissingField) && !err.Has(FaultInvalidValue). The "branch pin" and "missing pin" mutations must produceunpinned_scriptspecifically; under this assertion a regression toinvalid_valuestill passes. The line number is asserted (registry_test.go:99-101), so only the class half is loose — meaning the PR body's "asserts fault class + line" is slightly stronger than the code. Tighten those two sub-cases toHas(FaultUnpinnedScript).CONCERN 4 — factual correction: the image build did not run on this PR
dispatcher-image-build.yaml:42-43ispull_request: branches: [live], whileci.yamlisbranches: [live, 'epic/**']. This PR targetsepic/LD4, so onlyci.yamlran — the head status list is exactly 5 entries (atlantis + the four CI Review jobs), and that workflow's entire run history islive-targeted (runs #1583…#1616, all HOMELAB-1711). HOMELAB-1688's 2026-10-02 comment records "CI: dispatcher-test -race + image build green": thedispatcher-testhalf is correct, the image-build half is not. Consequence: the Dockerfile's load-bearing guarantee ("a registry the loader refuses fails the IMAGE BUILD",Dockerfile:15-18,79-80) has not been exercised in CI. Not a blocker — the registry itself is covered byTestRealRegistryLoads/TestCheckRealRegistryinside the greendispatcher-testjob, the hunk is additive, andworkflow_dispatchis enabled. It also self-corrects at the epic→live PR, wherepull_request: branches: [live]will build + smoke (the push step stays gated byif: github.event_name != 'pull_request'). Recommend a manualworkflow_dispatchnow, or correct the ticket record.SUGGESTIONS (non-blocking)
registry.go:65andcmd/routines/main.go:80referenceRegistry.RoleCheck, which does not exist. The observable difference is the"roleSource": roles != nilJSON key (main.go:114). Say that, or a reader goes hunting for a field that is not there — and note that the Dockerfile's build-time-check -identity ""therefore does not enforce refusal class 6 (documented, but the comment implies it is recorded somewhere).policy.md:35maps P-ROUT-006 toroutines/attribution.go; the file isroutines/sessions.go.testdata/corpus-table.md:11citesTestDangerousRoutinesAreGated; the dangerous-table assertions live inTestRegistryMatchesCorpus(parity_test.go:60-72).main.go:40DefaultRegistryPath = "/etc/lab-director/routines.yaml"matches neither what the image bakes (Dockerfile:101→/etc/lab-director-routines/registry.yaml) nor prod's mount. Latent only because the CronJob passes-registryexplicitly (prod #868:111), but a flag-less run of the shipped image points at nothing.What I checked and accept
Deviations 1/2/5 (active-only refusals; comma-separated
pausedReason; gated × authority orthogonal) — accepted, and enforced as refusals rather than prose.validate.go:192-209exempts a paused entry only when the reason carriesdep:role-<role>, andregistry_test.go:167-190proves the exemption is not a hole (a paused entry naming the role with an unrelated reason is still refused).ReasonTokensvalidates every token's prefix and payload (validate.go:277-296), withregistry_test.go:216-236refusing both the prose"waiting on LD4.1"and the empty"dep: ".validate.go:151treats care-dial ×gated: trueas legal, which is exactly R-02 in the registry — the case the deviation exists for.Deviation 4 (in-process pins) — accepted, independently verified.
TestInProcessPinsMatchTheCodeis genuine (it fails until re-pinned). I recomputed both:routines/rollup.go=6fcec5b4…839b00andcustodian/reclaim.go=61743546…f86982, both matchingregistry.yaml. I also verified the cross-repo pin by hand — R-19'shermes-automation/scripts/plane/cycles/flip_cycle.py=014a85c9…6546da✓. Note R-19's pin is not re-derived by any test (the path is outside the repo), so it can rot silently; a follow-up, not a blocker.Deviation 3 / the C4 question (
analyze-sessions) — I ratify the substitution; not a blocker. The brief's leg-B intent is read-only, redacted, deterministic, nested spend included. Delivered:sessions.go:ReadSessionFileopens read-only andTestSessionReaderIsReadOnlyAndRedactedbyte-compares the transcript before/after;Redactis applied to every string the reader emits (path, parent, ticket, model) and is asserted on a plantedghp_…; the reader sorts by path and takes no clock/randomness (deterministic); nested spend is satisfied structurally — every*.jsonlunder the root is read, each child carries its ownparentSession+ attribution header, and providertotalTokensis preferred over the sum of parts (asserted: 1175 incl. cache reads). What is genuinely unmet is C4's literal "5 scripts at a pinned revision", and that is disclosed inroutines/README.mdhonest-limit (4) and routed here by the orchestrator's adjudication. Vendoring a tool absent from this machine would have meant shipping a reimplementation under the upstream tool's name — the substitution is the honest option and the unmet line is recorded rather than glossed. Residual risk (follow-up): leg B's absolute numbers carry no cross-check against the upstream tool, so nothing would catch a systematic offset in the Go reader.Security. Registry is data with references only: no credentials in
registry.yaml(pins aresha256:), andscanForSecrets+ErrSecretInFileis a real refusal, asserted on a planted token. The Dockerfile hunk is additive — a secondgo build, twoCOPYs, one build-time-check; no change to base-image digests,USER,ENTRYPOINT, or the existing dispatcher build. The baked registry is shadowed by the ConfigMap mount at the same directory in prod, as the comment claims. Contract §11 holds: no new store (git + a plain-JSON ledger port behindPublisher/Reader,ld.routine.run/v1, subjectld.routines.run.<id>+ Plane state as the claim lock via theExecutionPlaneseam); the workload is prod #868's, outside this diff.No sockets.
grep -rn 'net/http|net.Dial|httptest|ListenAndServe' routines/ cmd/routines/is empty; every seam (ExecutionPlane,Ledger,Publisher,MetricSource,SessionSource) has a fixture.A1–A7 / C1–C5 / D1 map to tests that assert behaviour, not implementation. A1
TestRefusalClasses(class + line, one mutation per case); A2TestRegistryMatchesCorpusagainsttestdata/corpus-table.md— I spot-checked the transcription against the brief §2.2 and it is faithful (R-19 weekly, R-22 monthly, R-16 gated / R-14 not, per the corpus's own table); A3TestA3ForbidConcurrency(executor never ran twice, the skip is a ledger row, an unreadable lock refuses and recordsrefused); A4TestCatchUpPolicyDefaults; A5TestA5ReceiptContract(empty receipt →missed, neversuccess) +TestA5EscalationLadder(a success resets the ladder); A6 as detailed above; A7TestRealRegistryLoads+routines -check(22 registered / 3 active — I confirmed the census); C1/C3/C5cost_test.go(tokens+$ per sandbox env, unbound visible and never folded, budget breach is an episode citing P-CUST-007); C2TestC2AttributionCannotBeSpoofed(the child'sReportedSandboxEnvis deliberately ignored); C4 as above; D1 as above.Not merging. The lock requires no open blocker, and Blocker 1 is small and localized — a fix commit plus a CLI-level regression test should close it. Concerns 1-3 are mergeable once recorded; Concern 4 is a record correction.
Review —
998e825f81(delta re-review)verdict: pass
Scope: the delta this head introduces over the CHANGES REQUESTED round at
82e5021, plus mergeability against the retargeted base (epic/LD4@a9c7287). Reproduced in a fresh context, primary sources only — the fix commit's diff, the ticket's acceptance criteria, the committedroutines/registry.yaml, and a binary built from this head. (The fix worker left no PR comment; its evidence is the commit message, which I did not take on trust.)The prior blocking finding is closed.
cmd/routinesno longer injectstime.Now()asRegisteredAt. The anchor is now data:-registered-at/ROUTINES_REGISTERED_AT; unparsable => exit 64; zero => the ledger's oldest receipt; neither => no first period is granted that cannot be sourced. Reproduced with the shipped binary over the committed registry:routine-never-ran(R-03, R-06, R-19) citingP-ROUT-002, exit 1 (was0 findings/ exit 0)routine-overdueepisode plus R-06/R-19routine-never-ranepisodes, exit 1 — zero-row routines no longer read as healthy-registered-atfresh =>0 findingsexit 0; 72h old => R-03 episode, R-06 finding, R-19 silent (14-day first period);yesterday=> exit 64The regression tests drive the CLI, not the library.
TestWatchdogNeverRanFromTheCLIcallsrun()with no anchor over the committed registry and asserts oneP-ROUT-002finding per active routine;TestWatchdogCreditedAnchorIsRealDataandTestWatchdogMissingLedgerIsAPagelikewise driverun(). Library coverage keeps both halves honest:TestWatchdogNoAnchorIsNeverAPass,TestWatchdogRegistryRevisionIsTheBackstop,TestFileLedgerReaderDoesNotCreateTheLedger,TestRegistryRecordsWhenTheRevisionLanded.Independently run:
go test -race -count=1 ./routines/... ./cmd/routines/...green;go vet ./routines/... ./cmd/routines/...clean; and on the merge withepic/LD4@a9c7287the fullgo test -race -count=1 ./...andgo vet ./...inlab-director/dispatcherare green. Census unchanged on the merged tree: 22 registered / 3 active / 19 paused.Mergeability: clean, no conflicts.
git diff --name-only cd6db02..epic/LD4shows the other merged PR touchedcmd/dispatcher/*andconfig/*only — no overlap withroutines/,cmd/routines/, or the Dockerfile — so there is no additive arithmetic to recompute; the full race suite on the merged tree is the evidence that no assertion drifted.CI on this head: run #1663 —
dispatcher-test2m12s pass,helm-validatepass,receiver-testpass,alert-receiver-testpass; combined commit statussuccess.No finding of blocking severity is open. Remaining non-gating items are recorded here: the two original concerns that were mergeable-when-recorded (dead
openbao:denylist entry; A1's class-1 assertion is an OR), adetail-wording imprecision in the backstop branch (probe J: the sentence says nothing in band dates the entry whilemissedPeriods=360was computed from the mtime), and prod #868's CronJob needing-registered-atwired at unsuspend time (already inside that PR's held-operator path). CONCERN 1 (missing ledger silently invented) is closed: the reader never creates the file;NewFileLedgerremains the writer's opener.Cross-repo check:
sha256sumofroutines/registry.yamlat this head =a0b28b23917728417507e04f06c10d2a263e7c5ba3802895bd79b3b5e927b8af, still matching prod #868's ConfigMap annotation — the documented drift check stays valid.