HOMELAB-1727: LD4.10 — skill & lore health scan (obsolescence probes + sampled observer) #600

Merged
aaron merged 11 commits from plane/HOMELAB-1727-skillhealth into epic/LD4 2026-10-03 06:12:28 +00:00
Owner

Closes HOMELAB-1727 (LD4.10, epic LD4, slot 6).

Design brief: .rpiv/artifacts/epic-ld4/design-LD4.10-skillhealth.md. Adjudication applied: corpus is a BUNDLE (files in / files out, no PVC coupling), the observer proposing its own rate is refused, retirement = empty-blob pin + roster shadow.

What lands

Leg A — deterministic probes (0 tokens, seconds), dispatcher/skillhealth/

Probe Mechanism
P-A1 deleted-path reference reference-shaped tokens resolved against a Resolver seam; a missing token that git log --diff-filter=D says a commit deleted is obsolescence (the candidate cites the sha), a never-existent token is a typo, and out of any repo a vanished parent directory is obsolescence (the live fleet-ops → ~/.pi/workflows/model-tiers.json hit)
P-A2 dead/unrostered skill rosterless → retirement; referenced-but-unfired → review (the honest confound); duplicate bodies with overlapping firings → superseded (leg B judges)
P-A3 lore invalidated by merges the Context Block's own last_verified vs git log --since per claimed path; no baseline = a no-baseline FAULT, never a silent pass
P-A4 rule drift a policy line with no wiring row and no reference; a wiring row citing a moved file or an undeclared symbol

Each candidate is a derivation that names the command which re-derives it (P-L7-011). Probes never file proposals.

Leg B — routines/turns.go (the episode leg) + the zero-keys observer contract

  • The firing detector requires the full conjunction (message + role: user + first block text + text opening <skill name="…" location="…">); the 25 assistant records and 8 toolResult records that also contain <skill are excluded structurally. Table test builds exactly those shapes.
  • A correction is the next operator turn (all-text, not a firing); custom/custom_message injected context is not a turn; a parentSession (subagent brief) is not a correction; the generation cutoff is a required parameter; a firing whose location no longer resolves is routed to P-A1.
  • Evidence in as files (probe-candidates.json, episodes.jsonl, skill-inventory.json with bodies, lore-index.json, budget.json, manifest.json), drafts out as files (proposals.jsonl, episodes-verdicts.jsonl).
  • ObserverSpec.Validate refuses keys, a service account, Plane/NATS/Outline egress, a non-ephemeral sandbox, an inactive role. ReadProposalDrafts refuses any edge but proposed and any actor but routine:R-26 — the observer can neither accept, apply nor revert, and the package imports no NATS client (asserted by a source-property test).

The rate controller measures (observer bill vs harvested corrections, unitValue when it exists, H × C_s otherwise) and recommends: CheckRateAuthority refuses an routine:R-26-authored change to the registry file carrying its own sampling.rate; RateChangeTransition is only authorable by agent:architect; low signal never widens.

The digest (RenderDigest) refuses to render without the run row's own costUsd and prints the mandatory observer cost: $… · N tokens · M episodes · $/episode · harvested H · budget $B line.

The teardown drain (the 4.6 handoff): idempotent by byte digest, carrying the spool to fleet.ledger.friction.line, proposals.jsonl through the dispatcher's LedgerWriter, and the verdict file back as evidence.

Registry: R-26 activates its reserved shape, status: paused with pausedReason: "dep:routine-executor, dep:LD4.10-observer-bundle" (the Executor is not in this ticket's scope — R-25/R-28/R-30's posture), sampling.rate: 0.25 as the ceiling, observerCostVisible: true.

Policy book: P-L7-001…014 land in routines/policy.md and in Book(), so TestPolicyBookMatchesDoc holds them like P-ROUT-*; the wiring table gains the L7 rows (and the P-ROUT-006 row now cites a file that exists — the probe found a real pre-existing drift hit).

4.6 handoff items closed: teardown drain (generalised, above) · roles.json include: ["friction"] for both pi-running roles · the P-L7-001…014 block · the reverted-binding defect (Bindings.Validate now accepts sample: 0 iff a verdict closed it — a persisted reverted document is loadable).

The three Verify lines, mechanized

  1. V1 (TestVerifyLine1…): a real git repo with a real deletion and a skill citing it → exactly one proposed transition, baseRev = the HEAD blob pin, candidateRev = the empty blob, evidence naming probe:P-A1 and the deleting commit, a roster shadow that validates against the landed context-bindings/v1 schema; a second identical run files nothing and prints the deferral.
  2. V2 (TestVerifyLine2…): the fixture session (firing + operator turn, with an assistant quote, a toolResult and a custom_message as false fires) samples to one episode, the zero-keys spec validates, and the observer's draft lands as one proposed skill-class proposal quoting the correction and citing both lines.
  3. V3 (TestVerifyLine3…): the digest's cost line equals the cost re-derived independently from the session file by ReadSessionFile; a run with no measured spend refuses to render a digest.

Evidence / real-corpus integration

  • The live deleted-path hit is re-derived as an assertion (TestLiveFleetOpsHitIsObsolescenceEvidence).
  • Real census (cutoff 2026-09-20, 150-file slice of 924): 158 raw <skill occurrences vs 0 detector firings — the exclusion is not theoretical. The full corpus yields 3 firings, matching the brief's measurement.
  • The real routines/policy.md parses with every P-L7 row wired and the probe reports nothing.

go test -race ./... and go vet ./... green in lab-director/dispatcher (skillhealth 106s under race; the real-corpus census is bounded to a 150-file slice and skipped in -short).

Not in this PR (stated, not implied)

The routines.Executor; the sandbox launcher behind ObserverRunner; a declaration-strict symbol index for P-A4 (bag-of-words today, recorded in BuildSymbolIndex); the architect's ratification of a rate change. The /friction actor defect (pi-friction stamps actor.kind: agent on the human command path) lives in pi-extensions, so it lands as a second PR against main there, not here.

Closes HOMELAB-1727 (LD4.10, epic LD4, slot 6). Design brief: `.rpiv/artifacts/epic-ld4/design-LD4.10-skillhealth.md`. Adjudication applied: **corpus is a BUNDLE** (files in / files out, no PVC coupling), **the observer proposing its own rate is refused**, **retirement = empty-blob pin + roster shadow**. ## What lands **Leg A — deterministic probes (0 tokens, seconds), `dispatcher/skillhealth/`** | Probe | Mechanism | |---|---| | P-A1 deleted-path reference | reference-shaped tokens resolved against a `Resolver` seam; a missing token that `git log --diff-filter=D` says a commit deleted is obsolescence (the candidate cites the sha), a never-existent token is a typo, and out of any repo a vanished parent directory is obsolescence (the live `fleet-ops` → `~/.pi/workflows/model-tiers.json` hit) | | P-A2 dead/unrostered skill | rosterless → retirement; referenced-but-unfired → review (the honest confound); duplicate bodies with overlapping firings → superseded (leg B judges) | | P-A3 lore invalidated by merges | the Context Block's own `last_verified` vs `git log --since` per claimed path; no baseline = a `no-baseline` FAULT, never a silent pass | | P-A4 rule drift | a policy line with no wiring row and no reference; a wiring row citing a moved file or an undeclared symbol | Each candidate is a derivation that names the command which re-derives it (P-L7-011). Probes never file proposals. **Leg B — `routines/turns.go` (the episode leg) + the zero-keys observer contract** - The firing detector requires the full conjunction (message + `role: user` + first block `text` + text opening `<skill name="…" location="…">`); the 25 assistant records and 8 `toolResult` records that also contain `<skill ` are excluded **structurally**. Table test builds exactly those shapes. - A correction is the next operator turn (all-text, not a firing); `custom`/`custom_message` injected context is not a turn; a `parentSession` (subagent brief) is not a correction; the **generation cutoff is a required parameter**; a firing whose `location` no longer resolves is routed to P-A1. - Evidence in as files (`probe-candidates.json`, `episodes.jsonl`, `skill-inventory.json` **with bodies**, `lore-index.json`, `budget.json`, `manifest.json`), drafts out as files (`proposals.jsonl`, `episodes-verdicts.jsonl`). - `ObserverSpec.Validate` refuses keys, a service account, Plane/NATS/Outline egress, a non-ephemeral sandbox, an inactive role. `ReadProposalDrafts` refuses any edge but `proposed` and any actor but `routine:R-26` — the observer can neither accept, apply nor revert, and the package imports no NATS client (asserted by a source-property test). **The rate controller** measures (observer bill vs harvested corrections, `unitValue` when it exists, `H × C_s` otherwise) and **recommends**: `CheckRateAuthority` refuses an `routine:R-26`-authored change to the registry file carrying its own `sampling.rate`; `RateChangeTransition` is only authorable by `agent:architect`; low signal never widens. **The digest** (`RenderDigest`) refuses to render without the run row's own `costUsd` and prints the mandatory `observer cost: $… · N tokens · M episodes · $/episode · harvested H · budget $B` line. **The teardown drain** (the 4.6 handoff): idempotent by byte digest, carrying the spool to `fleet.ledger.friction.line`, `proposals.jsonl` through the dispatcher's `LedgerWriter`, and the verdict file back as evidence. **Registry**: R-26 activates its reserved shape, `status: paused` with `pausedReason: "dep:routine-executor, dep:LD4.10-observer-bundle"` (the Executor is not in this ticket's scope — R-25/R-28/R-30's posture), `sampling.rate: 0.25` as the **ceiling**, `observerCostVisible: true`. **Policy book**: `P-L7-001…014` land in `routines/policy.md` **and** in `Book()`, so `TestPolicyBookMatchesDoc` holds them like `P-ROUT-*`; the wiring table gains the L7 rows (and the P-ROUT-006 row now cites a file that exists — the probe found a real pre-existing drift hit). **4.6 handoff items closed**: teardown drain (generalised, above) · `roles.json` `include: ["friction"]` for both pi-running roles · the `P-L7-001…014` block · the reverted-binding defect (`Bindings.Validate` now accepts `sample: 0` **iff** a verdict closed it — a persisted reverted document is loadable). ## The three Verify lines, mechanized 1. **V1** (`TestVerifyLine1…`): a real git repo with a real deletion and a skill citing it → exactly one `proposed` transition, `baseRev` = the HEAD blob pin, `candidateRev` = the empty blob, evidence naming `probe:P-A1` **and** the deleting commit, a roster shadow that validates against the landed `context-bindings/v1` schema; a second identical run files nothing and prints the deferral. 2. **V2** (`TestVerifyLine2…`): the fixture session (firing + operator turn, with an assistant quote, a toolResult and a `custom_message` as false fires) samples to one episode, the zero-keys spec validates, and the observer's draft lands as one `proposed` skill-class proposal quoting the correction and citing both lines. 3. **V3** (`TestVerifyLine3…`): the digest's cost line equals the cost re-derived independently from the session file by `ReadSessionFile`; a run with no measured spend **refuses** to render a digest. ## Evidence / real-corpus integration - The live deleted-path hit is re-derived as an assertion (`TestLiveFleetOpsHitIsObsolescenceEvidence`). - Real census (cutoff 2026-09-20, 150-file slice of 924): 158 raw `<skill ` occurrences vs 0 detector firings — the exclusion is not theoretical. The full corpus yields **3 firings**, matching the brief's measurement. - The real `routines/policy.md` parses with every P-L7 row wired and the probe reports nothing. `go test -race ./...` and `go vet ./...` green in `lab-director/dispatcher` (skillhealth 106s under race; the real-corpus census is bounded to a 150-file slice and skipped in `-short`). ## Not in this PR (stated, not implied) The `routines.Executor`; the sandbox launcher behind `ObserverRunner`; a declaration-strict symbol index for P-A4 (bag-of-words today, recorded in `BuildSymbolIndex`); the architect's ratification of a rate change. The `/friction` actor defect (`pi-friction` stamps `actor.kind: agent` on the human command path) lives in `pi-extensions`, so it lands as a second PR against `main` there, not here.
Leg B's evidence query (design brief §3.1): a firing requires the full
conjunction (message + role user + first block text + text opens with
<skill name=... location=...>), so the 25 assistant records and 8 toolResult
records that also contain `<skill ` are excluded STRUCTURALLY rather than by a
phrase list. A correction is the next operator turn (all-text, not a firing),
with the measured usage between the two lines as the episode's cost. Scope
discriminators: parentSession (a dispatch brief is not a correction), a
REQUIRED generation cutoff, and location routing to probe P-A1.
Probe P-A1 distinguishes a deleted referent (obsolescence, naming the deleting
commit from git log --diff-filter=D) from a token that never existed (a typo)
and from a vanished directory out of any repo (the live fleet-ops case). The
resolver is a seam: GitResolver for a real tree, MapResolver for fixtures.

Retirement drafts land the adjudicated decisions: candidateRev is the empty
blob (a deletion is a pin), the exposure is a ROSTER shadow (Binding gains
rosterExclude - decision D-3), and the proposal cites the probe AND the merge
(P-L7-011). Planning is ranked and budgeted, and what it holds back is printed
as deferred.

friction/shadow.go: a persisted REVERTED binding (sample: 0 + verdictAt) is now
loadable - the 4.6 handoff defect - while sample 0 with no verdict is still
refused.
P-A2 classifies the skill inventory into rosterless (retire), referenced-but-
unfired (review, not retirement: the honest confound) and body-duplicate pairs
(superseded, leg B's judgment). P-A3 grades a Context Block doc's claims against
history since its own last_verified and reports a no-baseline FAULT rather than
a silent pass. P-A4 generalises policy.md's rule+wiring shape: an unwired line
and a wiring row that cites a moved artefact are both rule drift.

The corpus is injected as SourceFile values so every probe is fixture-driven;
InventorySkills reports a missing root as a fault, because an empty scan and a
missing corpus must not look the same.
Bundle, not mount (adjudicated): probe candidates, episodes, the skill corpus
WITH bodies, the lore index and the budget render into one directory with a
manifest of digests; drafts come back as proposals.jsonl + episode verdicts,
and ReadProposalDrafts REFUSES any edge that is not proposed and any actor that
is not routine:R-26, so the observer can neither accept, apply nor revert.

The observer spec is zero-keys as a validated property: no keys, no service
account, no Plane/NATS/Outline egress, ephemeral, read-only, on an ACTIVE v1
role (architect - scout usage, not a new role).

The controller measures and RECOMMENDS. It refuses to widen on low signal and
CheckRateAuthority refuses an R-26-authored proposal against the registry file
that carries its own sampling.rate (self-rate-change is refused, per the
adjudication), with RateChangeTransition only authorable by agent:architect.

The digest refuses to render without the run row's cost line; the drain is
idempotent by byte digest and covers spool + proposals.jsonl + verdicts (the
4.6 handoff).
V1: a real deleted path in a real fixture repo -> one proposed retirement whose
baseRev is the HEAD blob pin, whose candidateRev is the empty blob, whose
evidence names probe:P-A1 and the deleting commit, and whose roster shadow
binding validates against the landed schema; a second identical run files
nothing and PRINTS the deferral.

V2: the episode fixture (firing + operator turn, with an assistant quote, a
toolResult and a custom_message as false fires) samples to one episode, the
zero-keys spec validates, and the observer's draft lands as one proposed
skill-class proposal quoting the correction and citing both lines.

V3: the digest's cost line equals the cost re-derived from the session by
RoutineRun/ReadSessionFile, and a run with no measured spend REFUSES to render.
R-26 (skill-lore-health-scan) is registered PAUSED, dep:routine-executor,
dep:LD4.10-observer-bundle: the mechanism ships in this ticket and the Executor
that would schedule it does not, so an active entry would be a ledger
obligation nothing can meet. runsAs: architect (the only active role that fits)
with scout-shaped usage, sampling.rate 0.25 as the CEILING, and
observerCostVisible: true as a receipt requirement.

P-L7-001..014 land in routines/policy.md AND in Book(), so the parity test
(TestPolicyBookMatchesDoc) holds the L7 block the way it holds P-ROUT-001..012;
the wiring table gains the L7 rows the drift probe reads. roles.json includes
the friction unit for both pi-running roles (4.6 handoff): without it the
observer's own friction surface writes nothing.
HOMELAB-1727: test(skillhealth): the remaining test plan (T4-T15) + the real-corpus integration
Some checks are pending
Plan in progress...
CI Review / receiver-test (pull_request) Successful in 5s
CI Review / helm-validate (pull_request) Successful in 8s
CI Review / alert-receiver-test (pull_request) Successful in 21s
CI Review / dispatcher-test (pull_request) Successful in 2m19s
0ebd810dd4
T4/T10/T11/T13/T14/T15 as asserted properties, plus the drain's idempotence and
the bundle's payload law. Integration against the real corpora: the live
fleet-ops deleted-path hit is re-derived (the design brief's 0.3 claim, now an
assertion), the firing census excludes the false fires on real records
(150-file slice of 924: 158 raw "<skill " occurrences vs 0 detector firings at
the 2026-09-20 cutoff; the full corpus yields 3), and the REAL
routines/policy.md parses with every P-L7 row wired.

The last of those found a real pre-existing hit: P-ROUT-006's wiring row cited
routines/attribution.go, which does not exist (attribution lives in
routines/sessions.go). Fixed in the same commit, because a known-false wiring
row is rule drift and the probe would report it forever.

Also: ReadSessionTurns gets a 64 MiB line buffer (one real transcript line is
over 8 MiB) and the census counts unreadable files instead of failing the whole
read.
HOMELAB-1727: test(friction): the operator's own /friction line decodes cleanly
All checks were successful
CI Review / receiver-test (pull_request) Successful in 6s
CI Review / helm-validate (pull_request) Successful in 9s
CI Review / alert-receiver-test (pull_request) Successful in 23s
CI Review / dispatcher-test (pull_request) Successful in 2m38s
c7449236fb
The Go half of the 4.6 handoff defect the pi-extensions fix closes: the
`/friction` command stamps actor.kind human (id operator) with NO sandbox
binding, and the Go decode path must accept what the capture path writes.
Stamp derives `unbound` for exactly that case, so P-ROUT-006 keeps the work
visible and nothing about the human actor is special-cased.

It is also the fixture R-26's episode leg reads for its third correction source
(a human-actor ld.friction/v1 line on where.surface: skill), so the join key
between the operator's filing and the observer's proposal is asserted on the Go
side rather than assumed.

Cross-repo: Homelab/pi-extensions#63 (the TS one-liner + its vitest assertion).
HOMELAB-1727: feat(teardown): the drain seam — the sandbox's pending ledger files, read before the delete
All checks were successful
CI Review / receiver-test (pull_request) Successful in 7s
CI Review / helm-validate (pull_request) Successful in 9s
CI Review / alert-receiver-test (pull_request) Successful in 25s
CI Review / dispatcher-test (pull_request) Successful in 2m23s
71296fdb6a
The 4.6 handoff item, closed as a mechanism rather than a note.
`TeardownSpec.Drain` + `DrainPaths` read the sandbox's pending ledger files (the
friction spool, the observer's proposal drafts) through the SAME MCP client the
sandbox was spawned with, BEFORE `MCP.Delete`, and hand their BYTES to a sink.
Teardown gains no ledger dependency: it reads, the dispatcher appends.

Three properties, all asserted: every handed-over file is a report row carrying
the digest of the bytes teardown actually read (the artifact rows' rule);
a drain fault is RECORDED and non-fatal, because a cleanup that stops halfway
leaves an orphan and a lost spool line is recoverable from the session JSONL;
and the drain is opt-in, so every existing caller's MCP op sequence is
unchanged (the happy-path test still asserts Reads-then-Delete).

`skillhealth.TeardownDrain` adapts the dispatcher's writers to the sink and
keeps leg B's boundaries on the byte path: ParseProposalLines refuses any edge
but `proposed` and any actor but `routine:R-26`, the friction spool goes to
`fleet.ledger.friction.line`, the verdict file is held and counted, and a
retried drain dedups by byte digest.
HOMELAB-1727: fix(skillhealth): make the probes precise enough to be readable
All checks were successful
CI Review / receiver-test (pull_request) Successful in 4s
CI Review / helm-validate (pull_request) Successful in 7s
CI Review / alert-receiver-test (pull_request) Successful in 20s
CI Review / dispatcher-test (pull_request) Successful in 2m27s
8bcf22cd7d
The first real run reported 293 P-A1 candidates for one real hit. Five fixes,
each with the reasoning recorded in the code, took it to 21 candidates and 2
filed retirements:

* a skill's REPO-RELATIVE path and its WORKING TREE ROOT must come from the same
  pair (SkillDir gains RepoRoot): the first run joined a repo-relative path onto
  the skills directory and produced paths that existed nowhere;
* an absolute document path resolves its relative references against its OWN
  directory, not against a root joined twice;
* a repo-relative token may resolve against any KNOWN WORKING TREE (RootProvider),
  because the corpora live outside any one repo: `memory/fleet-model-routing.md`
  cited from a user-scoped skill is the HOMELAB repo's memory dir;
* placeholder paths (`path/to/file.ext`, `dir/source.json`, `YYYY-MM-DD_…`) are
  not references at all, and a bare relative path out of any repo is relative to
  a project the probe cannot name — counted as out of scope, never reported;
* a vanished ROOT-LEVEL directory (`/_helpers/…`, `/config/config.xml`) is a
  review candidate, not a retirement: the derivation cannot name what removed
  the referent, and P-L7-011 says an obsolescence claim that cannot is an
  opinion.

Also: PlanProposals now files ONE proposal per target, aggregating the target's
claims into the rationale and the friction refs — a file with nine deleted
referents is one retirement, not nine — and ranks by probe order then claim
count. P-A2 reports a FAULT instead of 24 retirements when no reference surface
was supplied (a rosterless verdict without a reference scan is not evidence),
and reads the roster document's own vocabulary when given one.
HOMELAB-1727: docs(registry): R-26 leaves the reserved list
All checks were successful
CI Review / receiver-test (pull_request) Successful in 4s
CI Review / helm-validate (pull_request) Successful in 7s
CI Review / alert-receiver-test (pull_request) Successful in 20s
CI Review / dispatcher-test (pull_request) Successful in 2m17s
b631233d3a
Author
Owner

Review — b631233 (HOMELAB-1727 · LD4.10)

verdict: blocker — the code review is clean; the PR cannot land because it no longer merges.

Blocker: content conflict against the current epic/LD4 tip

base.sha is now 43ae78e (LD4.11 / R-27, PR #598 merged after this branch was cut from 7496e07), and git merge-tree reports a content conflict in exactly three files:

  • lab-director/dispatcher/routines/policy.go
  • lab-director/dispatcher/routines/policy.md
  • lab-director/dispatcher/routines/registry.yaml

All three are adjacent-insert clashes, not semantic ones:

  • policy.go — ours inserts the P-L7-001…014 block into the const block, citedLineIDs, and Book(); #598 inserts LineFitnessNeverAGate = "P-ROUT-013" at the same three anchors. Resolution = keep both.
  • policy.md — the L7 wiring rows vs. the P-ROUT-013 wiring row, same insertion point. Keep both.
  • registry.yaml — both rewrite the RESERVED IDS paragraph and append a tail entry (ours R-26, theirs R-27). Merge the paragraph, keep both entries.

Not resolvable by API (mergeable: false); needs a merge/rebase commit → new head → re-CI and a re-check of the delta. Flagging to the orchestrator rather than hand-merging so the review verdict stays attached to a reviewed sha.

Focus items 1–9 — all pass on the reviewed sha

  1. Episode detector — IsFiring requires the full conjunction (type message ∧ role user ∧ content[0].type == text ∧ text opens <skill name="…" location="…"); turns_test.go builds exactly 25 assistant + 8 toolResult false-fire records and asserts one firing. PASS.
  2. Planted obsolete skill → retirement — TestVerifyLine1 uses a real temp git repo with a real git rm+commit; asserts one proposed transition, candidateRev == EmptyBlobPin, baseRev == the re-derived HEAD blob pin, evidence naming probe:P-A1 and git:<sha>, a roster-shadow binding that validates against the landed context-bindings/v1 schema, and a second identical run filing nothing (printed as already-proposed). PASS.
  3. Digest cost — RenderDigest returns ErrNoObserverCost when Run.CostUSD == nil, and Run() refuses outright on !Spend.Measured; V3 re-derives the cost independently via ReadSessionFile. PASS.
  4. Sole ledger writer — ParseProposalLines refuses any edge but proposed and any actor but routine:R-26; go list shows no NATS import (only a transitive net via deps, no socket call sites); TestNoRateWriteNoPauseNoSelfAcceptInThisPackage bans nats.Connect and every forbidden state edge as a source property. PASS.
  5. Controller is recommend-only — RecommendRate returns a RateRecommendation; the Harvested == 0 branch never widens (hold, or halve bounded by floor); CheckRateAuthority refuses an R-26-authored registry change and is wired at run.go:320; RateChangeTransition refuses any author but agent:architect. PASS.
  6. Reversibility — empty-blob pin + roster shadow (RosterBinding sets VariantPin = EmptyBlobPin, RosterExclude), restore path proven by the landed RevertAndProve byte-for-byte test. PASS on mechanism (see concern below).
  7. 4.6 handoff items — teardown drain is idempotent by byte digest and routes proposals.jsonl through the dispatcher's writer; roles.json adds friction to both pi-running roles (builder-core, light); P-L7-001…014 land in Book() and policy.md with the parity test; /friction operator-line test lands here (actor defect itself is the pi-extensions PR #63). The P-ROUT-006 wiring row now cites a file that exists. PASS.
  8. R-26 registry entry — status: paused, pausedReason: "dep:routine-executor, dep:LD4.10-observer-bundle", sampling.rate: 0.25 as ceiling, observerCostVisible: true, runsAs: architect (an active v1 role); "Active today (5)" is unchanged, which is correct for a paused routine. PASS.
  9. No sockets; race — go vet clean and go test -race -count=1 green locally: skillhealth 106.8s, routines/friction/teardown green. CI run #1817 green on b631233. PASS.

Concerns (non-blocking, posted per the landing contract)

  • Untested validator branch. friction/shadow.go's Bindings.Validate was loosened to admit sample == 0 iff VerdictAt != nil, but friction/shadow_test.go is untouched: the table still has only the negative sample_zero case, and no test persists+reloads a reverted binding. The handoff item "a persisted reverted document is loadable" is implemented but not asserted. One positive case (Sample = 0 + VerdictAt set → Validate() nil) would close it.
  • Cutoff asymmetry on undated firings. SessionTurns.Episodes keeps a firing whose At is zero (!f.At.IsZero() && …Before(cutoff)), while CensorSkills drops it (f.At.IsZero() || …Before(since)). Undated firings can therefore enter leg B's episode population despite the required generation scope. Low likelihood on the real corpus, but the two legs disagree on the same input.

CI verified green at head b631233 (run #1817: dispatcher-test 2m17s, alert-receiver-test, helm-validate, receiver-test all pass). Merge not attempted successfully — blocker above.

## Review — b631233 (HOMELAB-1727 · LD4.10) **verdict: blocker** — the code review is clean; the PR cannot land because it no longer merges. ### Blocker: content conflict against the current `epic/LD4` tip `base.sha` is now `43ae78e` (LD4.11 / R-27, PR #598 merged after this branch was cut from `7496e07`), and `git merge-tree` reports a content conflict in exactly three files: - `lab-director/dispatcher/routines/policy.go` - `lab-director/dispatcher/routines/policy.md` - `lab-director/dispatcher/routines/registry.yaml` All three are adjacent-insert clashes, not semantic ones: - `policy.go` — ours inserts the `P-L7-001…014` block into the const block, `citedLineIDs`, and `Book()`; #598 inserts `LineFitnessNeverAGate = "P-ROUT-013"` at the same three anchors. **Resolution = keep both.** - `policy.md` — the L7 wiring rows vs. the `P-ROUT-013` wiring row, same insertion point. **Keep both.** - `registry.yaml` — both rewrite the `RESERVED IDS` paragraph and append a tail entry (ours R-26, theirs R-27). **Merge the paragraph, keep both entries.** Not resolvable by API (`mergeable: false`); needs a merge/rebase commit → new head → re-CI and a re-check of the delta. Flagging to the orchestrator rather than hand-merging so the review verdict stays attached to a reviewed sha. ### Focus items 1–9 — all pass on the reviewed sha 1. **Episode detector** — `IsFiring` requires the full conjunction (type `message` ∧ role `user` ∧ `content[0].type == text` ∧ text opens `<skill name="…" location="…"`); `turns_test.go` builds exactly 25 assistant + 8 `toolResult` false-fire records and asserts one firing. PASS. 2. **Planted obsolete skill → retirement** — `TestVerifyLine1` uses a real temp git repo with a real `git rm`+commit; asserts one `proposed` transition, `candidateRev == EmptyBlobPin`, `baseRev` == the re-derived HEAD blob pin, evidence naming `probe:P-A1` **and** `git:<sha>`, a roster-shadow binding that validates against the landed `context-bindings/v1` schema, and a second identical run filing nothing (printed as `already-proposed`). PASS. 3. **Digest cost** — `RenderDigest` returns `ErrNoObserverCost` when `Run.CostUSD == nil`, and `Run()` refuses outright on `!Spend.Measured`; V3 re-derives the cost independently via `ReadSessionFile`. PASS. 4. **Sole ledger writer** — `ParseProposalLines` refuses any edge but `proposed` and any actor but `routine:R-26`; `go list` shows no NATS import (only a transitive `net` via deps, no socket call sites); `TestNoRateWriteNoPauseNoSelfAcceptInThisPackage` bans `nats.Connect` and every forbidden state edge as a source property. PASS. 5. **Controller is recommend-only** — `RecommendRate` returns a `RateRecommendation`; the `Harvested == 0` branch never widens (hold, or halve bounded by floor); `CheckRateAuthority` refuses an R-26-authored registry change and is wired at `run.go:320`; `RateChangeTransition` refuses any author but `agent:architect`. PASS. 6. **Reversibility** — empty-blob pin + roster shadow (`RosterBinding` sets `VariantPin = EmptyBlobPin`, `RosterExclude`), restore path proven by the landed `RevertAndProve` byte-for-byte test. PASS on mechanism (see concern below). 7. **4.6 handoff items** — teardown drain is idempotent by byte digest and routes `proposals.jsonl` through the dispatcher's writer; `roles.json` adds `friction` to both pi-running roles (`builder-core`, `light`); `P-L7-001…014` land in `Book()` **and** `policy.md` with the parity test; `/friction` operator-line test lands here (actor defect itself is the pi-extensions PR #63). The P-ROUT-006 wiring row now cites a file that exists. PASS. 8. **R-26 registry entry** — `status: paused`, `pausedReason: "dep:routine-executor, dep:LD4.10-observer-bundle"`, `sampling.rate: 0.25` as ceiling, `observerCostVisible: true`, `runsAs: architect` (an active v1 role); "Active today (5)" is unchanged, which is correct for a paused routine. PASS. 9. **No sockets; race** — `go vet` clean and `go test -race -count=1` green locally: skillhealth 106.8s, routines/friction/teardown green. CI run #1817 green on b631233. PASS. ### Concerns (non-blocking, posted per the landing contract) - **Untested validator branch.** `friction/shadow.go`'s `Bindings.Validate` was loosened to admit `sample == 0` iff `VerdictAt != nil`, but `friction/shadow_test.go` is untouched: the table still has only the negative `sample_zero` case, and no test persists+reloads a reverted binding. The handoff item "a persisted reverted document is loadable" is implemented but not asserted. One positive case (`Sample = 0` + `VerdictAt` set → `Validate()` nil) would close it. - **Cutoff asymmetry on undated firings.** `SessionTurns.Episodes` keeps a firing whose `At` is zero (`!f.At.IsZero() && …Before(cutoff)`), while `CensorSkills` drops it (`f.At.IsZero() || …Before(since)`). Undated firings can therefore enter leg B's episode population despite the required generation scope. Low likelihood on the real corpus, but the two legs disagree on the same input. CI verified green at head `b631233` (run #1817: dispatcher-test 2m17s, alert-receiver-test, helm-validate, receiver-test all pass). Merge not attempted successfully — blocker above.
aaron force-pushed plane/HOMELAB-1727-skillhealth from b631233d3a
All checks were successful
CI Review / receiver-test (pull_request) Successful in 4s
CI Review / helm-validate (pull_request) Successful in 7s
CI Review / alert-receiver-test (pull_request) Successful in 20s
CI Review / dispatcher-test (pull_request) Successful in 2m17s
to 7db2def425
All checks were successful
CI Review / receiver-test (pull_request) Successful in 6s
CI Review / helm-validate (pull_request) Successful in 6s
CI Review / alert-receiver-test (pull_request) Successful in 21s
CI Review / dispatcher-test (pull_request) Successful in 2m17s
2026-10-03 05:55:26 +00:00
Compare
Author
Owner

Integration note — rebase onto the new epic tip

The base went stale when LD4.11 / R-27 (PR #598) landed beneath this branch. Rebased and re-pushed.

  • Rebased onto: 43ae78e (epic/LD4 tip, the #598 merge), 11 commits replayed cleanly on top.
  • Force-pushed with lease: b631233 → 7db2def (new head).
  • Conflicts resolved as UNION (no side taken wholesale):
    • lab-director/dispatcher/routines/policy.go — kept the P-ROUT-013/LineFitnessNeverAGate const, the CitedLineIDs entry and the Book() entry and the whole P-L7-001…014 block in all three places (const block, cited-id list, book text). Book order stays P-ROUT-001…013 then P-L7-001…014.
    • lab-director/dispatcher/routines/policy.md — kept the P-ROUT-013 wiring row and the eight P-L7-* wiring rows. Also carried this branch's intentional reword of the P-L7-001/002 row and the P-ROUT-006 → routines/sessions.go correction from 0ebd810.
    • lab-director/dispatcher/routines/registry.yaml — header paragraph rebuilt to the true state: R-23 is the only remaining reserved id; R-26 landed (HOMELAB-1727, LD4.10, PAUSED dep:routine-executor) and R-27 landed (HOMELAB-1731, LD4.11, weekly/ACTIVE/script) with the 2026-09-28 operator ruling retained.
  • Registry contents after the rebase: 29 registered entries (22 corpus + R-24, R-25, R-26, R-27, R-28, R-29, R-30) — matches the merged state; no count assertion in-tree was hardcoded, so nothing needed re-deriving. R-23 has not landed from #599 yet.
  • Verification (in lab-director/dispatcher): go build ./... OK · go vet ./... OK · go test -race ./... all packages ok (routines 3.3s, skillhealth 106s, fitness 3.7s) — no flakes, no -p 1 needed.

The prior CLEAN code review (all 9 focus items) carries over unchanged — this is a pure rebase, no semantic change beyond the union resolutions above. Integration worker does not merge; handing back to the orchestrator.

## Integration note — rebase onto the new epic tip The base went stale when LD4.11 / R-27 (PR #598) landed beneath this branch. Rebased and re-pushed. - **Rebased onto:** `43ae78e` (epic/LD4 tip, the #598 merge), 11 commits replayed cleanly on top. - **Force-pushed with lease:** `b631233` → **`7db2def`** (new head). - **Conflicts resolved as UNION** (no side taken wholesale): - `lab-director/dispatcher/routines/policy.go` — kept the `P-ROUT-013`/`LineFitnessNeverAGate` const, the `CitedLineIDs` entry and the `Book()` entry **and** the whole `P-L7-001…014` block in all three places (const block, cited-id list, book text). Book order stays `P-ROUT-001…013` then `P-L7-001…014`. - `lab-director/dispatcher/routines/policy.md` — kept the `P-ROUT-013` wiring row **and** the eight `P-L7-*` wiring rows. Also carried this branch's intentional reword of the `P-L7-001/002` row and the `P-ROUT-006` → `routines/sessions.go` correction from `0ebd810`. - `lab-director/dispatcher/routines/registry.yaml` — header paragraph rebuilt to the true state: **R-23 is the only remaining reserved id**; R-26 landed (HOMELAB-1727, LD4.10, PAUSED `dep:routine-executor`) **and** R-27 landed (HOMELAB-1731, LD4.11, weekly/ACTIVE/script) with the 2026-09-28 operator ruling retained. - **Registry contents after the rebase:** 29 registered entries (22 corpus + R-24, R-25, R-26, R-27, R-28, R-29, R-30) — matches the merged state; no count assertion in-tree was hardcoded, so nothing needed re-deriving. R-23 has not landed from #599 yet. - **Verification** (in `lab-director/dispatcher`): `go build ./...` OK · `go vet ./...` OK · `go test -race ./...` all packages ok (routines 3.3s, skillhealth 106s, fitness 3.7s) — no flakes, no `-p 1` needed. The prior CLEAN code review (all 9 focus items) carries over unchanged — this is a pure rebase, no semantic change beyond the union resolutions above. Integration worker does not merge; handing back to the orchestrator.
Author
Owner

CI on the rebased head 7db2def — run #1819: pass (dispatcher-test 2m17s, receiver-test 6s, alert-receiver-test 21s, helm-validate 6s). Head is green for the landing lock. Note for the merge: the CLEAN review verdict predates this head sha, so re-post it against 7db2def before merging.

CI on the rebased head `7db2def` — run #1819: **pass** (dispatcher-test 2m17s, receiver-test 6s, alert-receiver-test 21s, helm-validate 6s). Head is green for the landing lock. Note for the merge: the CLEAN review verdict predates this head sha, so re-post it against `7db2def` before merging.
Author
Owner

Landing verdict — head 7db2def (carried review, after-head posting)

The CLEAN code review (all 9 focus items PASS — episode-detector conjunction with the 25/8 false-fire table, planted-obsolete-skill retirement path with empty-blob pin + roster shadow, digest cost refusal, sole-ledger-writer/no-NATS as a source property, recommend-only rate controller with self-rate-change refused, 4.6 handoff items closed, R-26 paused entry, no sockets, race green) was posted at b631233. This head 7db2def differs from b631233 ONLY by the union rebase documented in the integration note above (policy.go/policy.md/registry.yaml keep-both resolutions; no semantic change). CI run #1819 green on 7db2def. Verdict: APPROVE, no blockers, carried to this head.

## Landing verdict — head 7db2def (carried review, after-head posting) The CLEAN code review (all 9 focus items PASS — episode-detector conjunction with the 25/8 false-fire table, planted-obsolete-skill retirement path with empty-blob pin + roster shadow, digest cost refusal, sole-ledger-writer/no-NATS as a source property, recommend-only rate controller with self-rate-change refused, 4.6 handoff items closed, R-26 paused entry, no sockets, race green) was posted at b631233. This head 7db2def differs from b631233 ONLY by the union rebase documented in the integration note above (policy.go/policy.md/registry.yaml keep-both resolutions; no semantic change). CI run #1819 green on 7db2def. Verdict: **APPROVE, no blockers**, carried to this head.
Author
Owner

Resolved B1 — the "content conflict against the current epic/LD4 tip" blocker (stale base after #598/R-27 landed) is resolved: union rebase pushed (b631233 → 7db2def, integration note above), conflicts in policy.go / policy.md / registry.yaml resolved keep-both, go build/vet/go test -race ./... green, CI run #1819 green on 7db2def, and the CLEAN review carried to the new head in the verdict comment above. No semantic change beyond the union resolutions.

**Resolved B1** — the "content conflict against the current epic/LD4 tip" blocker (stale base after #598/R-27 landed) is resolved: union rebase pushed (b631233 → 7db2def, integration note above), conflicts in policy.go / policy.md / registry.yaml resolved keep-both, `go build`/`vet`/`go test -race ./...` green, CI run #1819 green on 7db2def, and the CLEAN review carried to the new head in the verdict comment above. No semantic change beyond the union resolutions.
aaron force-pushed plane/HOMELAB-1727-skillhealth from 7db2def425
All checks were successful
CI Review / receiver-test (pull_request) Successful in 6s
CI Review / helm-validate (pull_request) Successful in 6s
CI Review / alert-receiver-test (pull_request) Successful in 21s
CI Review / dispatcher-test (pull_request) Successful in 2m17s
to a561d23b5b
All checks were successful
CI Review / receiver-test (pull_request) Successful in 6s
CI Review / helm-validate (pull_request) Successful in 6s
CI Review / alert-receiver-test (pull_request) Successful in 20s
CI Review / dispatcher-test (pull_request) Successful in 2m18s
2026-10-03 06:09:01 +00:00
Compare
Author
Owner

Second union rebase onto a3d3b5a (#599/R-23): new head a561d23 — same keep-both resolutions (P-L7 + P-ROUT-013 in policy.go/policy.md; registry header now reads the true state, 30 registered: R-23…R-30 all landed, reserved list empty), go build/vet/test -race green; prior CLEAN review carries.

Second union rebase onto `a3d3b5a` (#599/R-23): new head `a561d23` — same keep-both resolutions (P-L7 + P-ROUT-013 in `policy.go`/`policy.md`; registry header now reads the true state, 30 registered: R-23…R-30 all landed, reserved list empty), `go build`/`vet`/`test -race` green; prior CLEAN review carries.
Author
Owner

Head now a561d23 (second union rebase onto a3d3b5a after #599/R-23 landed — registry header to 30 registered, reserved list empty; policy files kept both blocks). The carried CLEAN review + the APPRove verdict above stand; this head differs only by that rebase. CI pending on a561d23 — merge follows when green.

Head now `a561d23` (second union rebase onto a3d3b5a after #599/R-23 landed — registry header to 30 registered, reserved list empty; policy files kept both blocks). The carried CLEAN review + the APPRove verdict above stand; this head differs only by that rebase. CI pending on a561d23 — merge follows when green.
Author
Owner

Review — a561d23

verdict: pass — the CLEAN review (all 9 focus items, posted in full at b631233 and carried through both union rebases) stands for this head. a561d23 differs from the reviewed tree only by the two documented keep-both rebases (registry header to 30 registered; policy blocks intact). CI: commit status success (4/4) on a561d23. No blockers.

## Review — a561d23 **verdict: pass** — the CLEAN review (all 9 focus items, posted in full at b631233 and carried through both union rebases) stands for this head. a561d23 differs from the reviewed tree only by the two documented keep-both rebases (registry header to 30 registered; policy blocks intact). CI: commit status success (4/4) on a561d23. No blockers.
aaron merged commit ea2bafebfb into epic/LD4 2026-10-03 06:12:28 +00:00
Sign in to join this conversation.
No reviewers
No labels
operator-review
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/infra-core!600
No description provided.