HOMELAB-2634: fix(herdr): recognise relieved sessions, fail closed on everything else #93

Merged

pi-landing merged 3 commits from plane/HOMELAB-2634-janitor-relief into main 2026-11-04 19:03:49 +00:00

Conversation 4 Commits 3 Files changed 5 +1777 -131

aaron commented 2026-11-04 18:30:59 +00:00

Owner

Copy link

HOMELAB-2634 — recognise relieved sessions, fail closed on everything else

Recovered work (the previous orchestrator was killed mid-run at 2026-11-04T15:27Z with no handoff) plus one follow-up commit. Branch was local-only until now; the ticket's "janitor ready" report is on HOMELAB-2634.

What this changes

scripts/herdr_janitor/janitor.py already closed successor/retire-marker panes, but it trusted agent_status. Measured on this fleet that is wrong: sup-gits-e1-5 read done while generating (1 shell still running, ← 1 agent), infra-mem-overseer read done with 4 shells still running, and pi panes running subagents read idle. The close authority is now evidence the pane cannot fake:

• Live work — the pane's own footer on both agents (N running agents, N/N agents, N shells still running, ← N agent); any non-zero count refuses.
• Screen stability — 600 s of byte-identical screen, for the pi-mid-turn-with-no-subagents case no footer can distinguish.
• The operator's roster (working-files/ldv0/directorio-claude.md), cross-checked on every close path; an unreadable roster refuses everything.
• Relief evidence — the successor's posted start or the old pane's HANDOFF READY line.
• Role grammar — only the two families the operator named (sup-<rest>-<N>, <rest>-orchestrator-<N>); everything else is report-only, forever.
• Never wS/wH, never ld-hypervisor/el-director, never a focused pane, never an unnamed or duplicate name.

968bbf3 adds el-director to PROTECTED_NAMES (the top Claude session's new herdr name), keeping ld-hypervisor.

Safety contract, unchanged

Dry-run by default; real closes need both --apply and a non-symlink activation file containing exactly HOMELAB-2634. Every close is re-decided from fresh herdr state immediately before pane close. The only subcommands ever invoked are pane list, agent list, agent read and pane close — no server stop, no prompt, no send-text.

Evidence

• 80 unit tests OK (32 pre-existing + 48 new).
• Independent check: 70/70 assertions, driving the real decide() against footers captured verbatim from the live fleet.
• End-to-end CLI proof: 47/47, including the activation gate refusing with zero herdr calls, a captured live footer holding the close, and the same pair closing in one cycle with a quiet footer.
• Live dry-run at the time of writing: 9 panes, REPORT=9, zero CLOSE_CANDIDATE.

Not proven

Screen stability across two real cycles on this VM — proved on fixtures and captured footers only. HOMELAB-2634's Verify is the first real close of a relieved session, which cannot happen until the timer is armed.

## HOMELAB-2634 — recognise relieved sessions, fail closed on everything else Recovered work (the previous orchestrator was killed mid-run at 2026-11-04T15:27Z with no handoff) plus one follow-up commit. Branch was local-only until now; the ticket's "janitor ready" report is on HOMELAB-2634. ### What this changes `scripts/herdr_janitor/janitor.py` already closed successor/retire-marker panes, but it trusted `agent_status`. Measured on this fleet that is wrong: `sup-gits-e1-5` read `done` while generating (`1 shell still running`, `← 1 agent`), `infra-mem-overseer` read `done` with `4 shells still running`, and pi panes running subagents read `idle`. The close authority is now evidence the pane cannot fake: - **Live work** — the pane's own footer on both agents (`N running agents`, `N/N agents`, `N shells still running`, `← N agent`); any non-zero count refuses. - **Screen stability** — 600 s of byte-identical screen, for the pi-mid-turn-with-no-subagents case no footer can distinguish. - **The operator's roster** (`working-files/ldv0/directorio-claude.md`), cross-checked on every close path; an unreadable roster refuses everything. - **Relief evidence** — the successor's posted start or the old pane's `HANDOFF READY` line. - **Role grammar** — only the two families the operator named (`sup-<rest>-<N>`, `<rest>-orchestrator-<N>`); everything else is report-only, forever. - **Never** `wS`/`wH`, never `ld-hypervisor`/`el-director`, never a focused pane, never an unnamed or duplicate name. `968bbf3` adds `el-director` to `PROTECTED_NAMES` (the top Claude session's new herdr name), keeping `ld-hypervisor`. ### Safety contract, unchanged Dry-run by default; real closes need **both** `--apply` and a non-symlink activation file containing exactly `HOMELAB-2634`. Every close is re-decided from fresh herdr state immediately before `pane close`. The only subcommands ever invoked are `pane list`, `agent list`, `agent read` and `pane close` — no `server stop`, no prompt, no send-text. ### Evidence - 80 unit tests OK (32 pre-existing + 48 new). - Independent check: 70/70 assertions, driving the real `decide()` against footers captured verbatim from the live fleet. - End-to-end CLI proof: 47/47, including the activation gate refusing with **zero** herdr calls, a captured live footer holding the close, and the same pair closing in one cycle with a quiet footer. - Live dry-run at the time of writing: 9 panes, `REPORT=9`, **zero** CLOSE_CANDIDATE. ### Not proven Screen stability across two **real** cycles on this VM — proved on fixtures and captured footers only. HOMELAB-2634's Verify is the first real close of a relieved session, which cannot happen until the timer is armed.

aaron added 2 commits 2026-11-04 18:31:00 +00:00

HOMELAB-2634: fix(herdr): recognise relieved sessions, fail closed on everything else ... 101eeba2f7

Five live-measured defects, fixed together because they share one decision
table:

D1 handoff evidence was looked for on the wrong side. A close now accepts
evidence from either side and names it: the successor's posted start
(successor-start), or the old pane's HANDOFF READY / TICKET-APPROVED
SUCCESSOR line. Successor text is now read for every relievable pane.

D2 the role grammar only matched <base>-<N>. It now covers exactly the two
families the operator named, sup-<rest>-<N> and <rest>-orchestrator-<N>, and a
bare name in either family is generation 1. Everything else
(infra-mem-overseer, ld-hypervisor, unnamed) is report-only.

D3 the roster ldv0/directorio-claude.md can veto a name-only guess:
directory-current and directory-unreadable block every close path,
directory-successor-not-live and directory-disagrees block the successor path.
An unreadable roster refuses EVERY close and says so in the report header and
in the summary count, so a fail-closed stop is never silent.

D4 agent_status lies in both directions (measured 2026-11-04): a generating
Claude pane reports done, a pi pane running subagents reports idle, and a quiet
pi pane prints no count at all. The live-work patterns are kept and pinned by
the verbatim footers, and a new screen-stability rule (SHA-256 of the last ~40
lines plus hash_since in seen.json) closes the residual hole: the old pane's
screen must be identical to the previous run's and must already have been
watched for the full 600 s floor. One 15-minute cycle after first sight is
enough; no extra warm-up run.

D5 a focused pane is never yanked: reasons=focused-pane, regardless of name.

Process discipline: HerdrCLI refuses any argv outside pane list, agent list,
agent read and pane close, so herdr server stop / upgrades / kills are
unreachable by construction. Revalidation before each close now also re-checks
the roster state and the marker. Dry-run stays the default and still writes
only report.txt and janitor.log.

The unit runs the deployed copy (%h/.pi/agent/extensions) instead of this
checkout, so the timer executes the code the operator installs and reviews.

Tests: 80 in total (31 pre-existing, 49 new), all green; the new file was run
against the unmodified module first (66 red) and its output is in the report.

HOMELAB-2634: also protect el-director, the voice's new herdr name ... 968bbf32c4

The top Claude session is being relaunched as el-director. Keep ld-hypervisor
protected and add the new name to PROTECTED_NAMES, so a rename cannot make the
voice closable. Report-only either way: el-director matches neither role family.

ld-reviewer approved these changes 2026-11-04 18:42:27 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 968bbf32c4
session: homelab-2634-review-1

Scope and method

I reviewed PR #93 at its exact head 968bbf32c40238815e810f150837e6f8b97a9b27 (base main@48c5000) in a clean worktree: git rev-parse HEAD returned that sha and git status --porcelain was empty, so what I read is what is being merged. I read decide() in full (all five branches), _base_protections, live_work, text_hash, screen_stable, successor_age, directory_gate, directory_successor_state, role_parts, interesting_pane_ids, Janitor.require_activation, Janitor._execute, allowed_argv, and main(), and I diffed decide() against 48c5000 to know which guarantees this PR adds from which it inherits. I also drove decide() directly with hand-built panes/agents/seen/markers rather than trusting the test names, and I ran the CLI for real.

Executed, not described: python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py' → 80 tests, all pass (32 in test_herdr_janitor.py, 48 in test_herdr_janitor_relief.py), 0.10 s. Live read-only dry-run into an isolated state dir (--state-dir "$(mktemp -d)" --dry-run): exit 0, summary REPORT=10 total=10 directory_unreadable_refusals=0, zero CLOSE_CANDIDATE and zero WOULD_CLOSE in the output. CI on this exact sha is green: Actions run #254 / PR #93 — all 15 jobs success, and run #255 python success.

Taken on trust, and named so it is not mistaken for proof: that /home/aaron/repos/homelab/working-files/ldv0/directorio-claude.md is the roster the operator means; that herdr 0.9.1 genuinely exposes no pane creation time (which is why first-seen is the only age available); that ~/.pi/agent/extensions/scripts/herdr_janitor/janitor.py will receive exactly this reviewed code, since the unit runs the deployed copy and not this branch.

The six contract points, checked against the code

1. Close only when relieved or retired, and never with live work. Every one of the five branches in decide() appends at least one reason, and CLOSE_CANDIDATE is only ever set inside a branch that then appends its authorising reason, so no decision can reach a close with an empty reasons list — I checked each branch for the "appends nothing" shape rather than assuming it. The successor branch requires: same role base, same workspace, strictly higher generation, successor status in {idle, done, working}, successor_age proven and >= min_successor_seconds, non-empty old-pane text with live_work() empty, screen_stable() true, relief evidence from either side, and no directory objection. The marker branch requires the same base protections, non-empty text and empty live_work(). Fail-closed on the successor path is belt-and-braces: if decision.reasons: path_reasons.append("successor-close-held") refuses even a fully green successor pair when a base protection or directory objection fired.

2. wS/wH are never closed, even when named explicitly. Config.allowed_workspaces is workspaces - NEVER_WORKSPACES, and _base_protections tests workspace in NEVER_WORKSPACES before the allowlist. I ran it: --workspaces wS,wH yields workspaces= (empty allowlist) and summary REPORT=10 total=10. There is no expression of workspace policy outside those two places, so the allowlist cannot be widened from the command line or the environment.

3. Fail closed when unsure. Unreadable/row-less roster → directory_gate returns directory-unreadable for every pane, which lands in decision.reasons and therefore holds both the marker branch (if marker and decision.reasons) and the successor branch (successor-close-held); render_report also emits a loud header line and counts them. Empty or failed agent read → status-read-failed on both close paths. Unproven successor age → successor-age-unproven; first_seen > now is refused rather than accepted. Unstable screen → screen-unstable (hash must equal the previous run's hash and now - hash_since >= 600). Missing handoff → handoff-evidence-missing. Duplicates → duplicate-name, counted across all workspaces, so the stricter answer wins. Unnamed → unnamed plus role-not-relievable. Focused → focused-pane by focused: true regardless of name.

4. Only the four permitted herdr calls. allowed_argv is an exact-sequence allowlist (pane list, agent list, pane close <id>, agent read <id> --source recent-unwrapped --lines 240 --format text) and HerdrCLI._run raises before subprocess.run on anything else. grep for subprocess./os.system/exec( finds a single subprocess.run inside _run, and the test suite asserts refusal of server stop, pane kill, pane send-text, agent prompt, upgrade, bare pane close, and a read with extra flags.

5. Dry-run default; a real close needs --apply and the activation file. apply is False unless --apply is passed, and run() calls require_activation() before collect(), so an unactivated --apply makes zero herdr calls — I confirmed: --apply with an isolated state dir exits 1 with real mode is not activated, printing no pane data. require_activation refuses a symlink and requires the stripped content to equal HOMELAB-2634 exactly. main() refuses --min-successor-seconds 599 with exit 2, and I ran it: the 600 s floor cannot be lowered from the CLI.

6. Protections. PROTECTED_NAMES = frozenset({"ld-hypervisor", "el-director"}), checked in _base_protections, which runs before and independently of every close path, so neither a marker nor a successor can bypass it. No name-based bypass exists either: role_parts only parses sup-<rest>[-N] and <rest>-orchestrator[-N], so anything else — including ld-hypervisor and el-director — is role-not-relievable forever.

I also probed the two shapes the diff could most plausibly get wrong. No fallback to an older generation: with sup-a-5, a healthy sup-a-6 and a blocked sup-a-7 present, the old pane reports successor=wT:p7 reasons=successor-status-blocked and is not closed on sup-a-6. And a successor in wS never relieves a wT pane (reasons=no-successor), while the wS pane itself is never-workspace.

Net assessment versus main@48c5000: this is a strict safety improvement on every axis. Main had no screen-stability check, no roster cross-check, no focused protection, no el-director protection, and no successor revalidation before the destructive call.

Findings

F1 — medium, not blocking this PR (pre-existing on main). The retire-marker branch never consults seen at all: no previous observation, no hash_since, no stillness floor. A pane seen for the first time, carrying a marker, whose last 40 lines print no live-work count, goes straight to CLOSE_CANDIDATE. I reproduced it: panes/agents with one idle pane named for a marker and text "Thinking... (no counts printed at all)" → ('CLOSE_CANDIDATE', ['retire-marker']) with seen hash None and hash_since never read. Contract point 1(b) asks for "no live workers"; on the marker path that is currently "no live-workers count was printed in one instantaneous read", which is exactly the false negative the docs identify for a pi pane mid-turn with zero subagents — the successor path answers it with 600 s of byte-identical screen, the marker path answers it with nothing. This is identical in main@48c5000, so I am not blocking the merge on it, but it is the one close path whose only evidence of "no live work" has no corroboration, and a marker close is destructive. Suggested before the timer is armed (a separate, operator-gated step): require the same screen_stable/hash_since >= min_successor_seconds condition on the marker branch, or state the residual risk explicitly in docs/herdr-janitor.md under "What this cannot prove" so a supervisor knows a marker can end an in-flight turn.

F2 — low, apply-mode reporting only. _execute builds by_pane = {d.pane_id: d for d in decisions} and returns [by_pane[key] for key in by_pane]. The absent-marker rows synthesised in decide() all use pane_id == "-", so two or more absent markers collapse to one row and the count drops (I confirmed: 2 rows in, 1 out). No close risk — those rows are REPORT/absent — but an operator reading an apply-mode report can under-count stale markers.

F3 — informational. require_activation checks is_symlink() and then calls read_text(), which is a TOCTOU window in principle. Exploiting it needs write access to the 0700 state directory, which already implies the ability to write the activation file directly, so I do not consider it a hole.

What this PR does not prove

Nothing here has ever closed a relieved session. The ticket's Verify — the first real close of a relieved session — cannot happen until the timer is armed, and this PR deliberately does not arm it: no unit is installed, no timer enabled, no activation file created. "Armed, not proven" is the correct state, and the live dry-run I pasted above is the strongest evidence available today: it reports ten panes, refusals, and zero close candidates. Two further limits are honest ones the docs already state: a pi pane that prints nothing for ten minutes is indistinguishable from a finished one, so stillness is evidence of quiet, never of idleness; and SUCCESSOR_STARTED is self-declared text, held in check only by the numeric rule, the workspace rule, the age floor and the roster.

I am approving the code as written. F1 is a recommendation for the arming step, not a request to change this diff, and I have said so explicitly so nobody reads this approval as evidence that a marker close has been shown safe on a busy pane.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 968bbf32c40238815e810f150837e6f8b97a9b27 session: homelab-2634-review-1 ## Scope and method I reviewed PR #93 at its exact head `968bbf32c40238815e810f150837e6f8b97a9b27` (base `main@48c5000`) in a clean worktree: `git rev-parse HEAD` returned that sha and `git status --porcelain` was empty, so what I read is what is being merged. I read `decide()` in full (all five branches), `_base_protections`, `live_work`, `text_hash`, `screen_stable`, `successor_age`, `directory_gate`, `directory_successor_state`, `role_parts`, `interesting_pane_ids`, `Janitor.require_activation`, `Janitor._execute`, `allowed_argv`, and `main()`, and I diffed `decide()` against `48c5000` to know which guarantees this PR adds from which it inherits. I also drove `decide()` directly with hand-built panes/agents/seen/markers rather than trusting the test names, and I ran the CLI for real. Executed, not described: `python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py'` → **80 tests, all pass** (32 in `test_herdr_janitor.py`, 48 in `test_herdr_janitor_relief.py`), 0.10 s. Live read-only dry-run into an isolated state dir (`--state-dir "$(mktemp -d)" --dry-run`): exit 0, **`summary REPORT=10 total=10 directory_unreadable_refusals=0`**, zero `CLOSE_CANDIDATE` and zero `WOULD_CLOSE` in the output. CI on this exact sha is green: Actions run #254 / PR #93 — all 15 jobs success, and run #255 `python` success. Taken on trust, and named so it is not mistaken for proof: that `/home/aaron/repos/homelab/working-files/ldv0/directorio-claude.md` is the roster the operator means; that herdr 0.9.1 genuinely exposes no pane creation time (which is why first-seen is the only age available); that `~/.pi/agent/extensions/scripts/herdr_janitor/janitor.py` will receive exactly this reviewed code, since the unit runs the deployed copy and not this branch. ## The six contract points, checked against the code **1. Close only when relieved or retired, and never with live work.** Every one of the five branches in `decide()` appends at least one reason, and `CLOSE_CANDIDATE` is only ever set inside a branch that then appends its authorising reason, so no decision can reach a close with an empty `reasons` list — I checked each branch for the "appends nothing" shape rather than assuming it. The successor branch requires: same role base, same workspace, strictly higher generation, successor status in `{idle, done, working}`, `successor_age` proven and `>= min_successor_seconds`, non-empty old-pane text with `live_work()` empty, `screen_stable()` true, relief evidence from either side, and no directory objection. The marker branch requires the same base protections, non-empty text and empty `live_work()`. Fail-closed on the successor path is belt-and-braces: `if decision.reasons: path_reasons.append("successor-close-held")` refuses even a fully green successor pair when a base protection or directory objection fired. **2. `wS`/`wH` are never closed, even when named explicitly.** `Config.allowed_workspaces` is `workspaces - NEVER_WORKSPACES`, and `_base_protections` tests `workspace in NEVER_WORKSPACES` before the allowlist. I ran it: `--workspaces wS,wH` yields `workspaces=` (empty allowlist) and `summary REPORT=10 total=10`. There is no expression of workspace policy outside those two places, so the allowlist cannot be widened from the command line or the environment. **3. Fail closed when unsure.** Unreadable/row-less roster → `directory_gate` returns `directory-unreadable` for every pane, which lands in `decision.reasons` and therefore holds both the marker branch (`if marker and decision.reasons`) and the successor branch (`successor-close-held`); `render_report` also emits a loud header line and counts them. Empty or failed `agent read` → `status-read-failed` on both close paths. Unproven successor age → `successor-age-unproven`; `first_seen > now` is refused rather than accepted. Unstable screen → `screen-unstable` (hash must equal the previous run's hash **and** `now - hash_since >= 600`). Missing handoff → `handoff-evidence-missing`. Duplicates → `duplicate-name`, counted across all workspaces, so the stricter answer wins. Unnamed → `unnamed` plus `role-not-relievable`. Focused → `focused-pane` by `focused: true` regardless of name. **4. Only the four permitted herdr calls.** `allowed_argv` is an exact-sequence allowlist (`pane list`, `agent list`, `pane close <id>`, `agent read <id> --source recent-unwrapped --lines 240 --format text`) and `HerdrCLI._run` raises before `subprocess.run` on anything else. `grep` for `subprocess.`/`os.system`/`exec(` finds a single `subprocess.run` inside `_run`, and the test suite asserts refusal of `server stop`, `pane kill`, `pane send-text`, `agent prompt`, `upgrade`, bare `pane close`, and a read with extra flags. **5. Dry-run default; a real close needs `--apply` and the activation file.** `apply` is False unless `--apply` is passed, and `run()` calls `require_activation()` before `collect()`, so an unactivated `--apply` makes zero herdr calls — I confirmed: `--apply` with an isolated state dir exits 1 with `real mode is not activated`, printing no pane data. `require_activation` refuses a symlink and requires the stripped content to equal `HOMELAB-2634` exactly. `main()` refuses `--min-successor-seconds 599` with exit 2, and I ran it: the 600 s floor cannot be lowered from the CLI. **6. Protections.** `PROTECTED_NAMES = frozenset({"ld-hypervisor", "el-director"})`, checked in `_base_protections`, which runs before and independently of every close path, so neither a marker nor a successor can bypass it. No name-based bypass exists either: `role_parts` only parses `sup-<rest>[-N]` and `<rest>-orchestrator[-N]`, so anything else — including `ld-hypervisor` and `el-director` — is `role-not-relievable` forever. I also probed the two shapes the diff could most plausibly get wrong. No fallback to an older generation: with `sup-a-5`, a healthy `sup-a-6` and a `blocked` `sup-a-7` present, the old pane reports `successor=wT:p7 reasons=successor-status-blocked` and is not closed on `sup-a-6`. And a successor in `wS` never relieves a `wT` pane (`reasons=no-successor`), while the `wS` pane itself is `never-workspace`. Net assessment versus `main@48c5000`: this is a strict safety improvement on every axis. Main had no screen-stability check, no roster cross-check, no `focused` protection, no `el-director` protection, and no successor revalidation before the destructive call. ## Findings **F1 — medium, not blocking this PR (pre-existing on `main`).** The retire-marker branch never consults `seen` at all: no previous observation, no `hash_since`, no stillness floor. A pane seen for the first time, carrying a marker, whose last 40 lines print no live-work count, goes straight to `CLOSE_CANDIDATE`. I reproduced it: `panes/agents` with one `idle` pane named for a marker and text `"Thinking... (no counts printed at all)"` → `('CLOSE_CANDIDATE', ['retire-marker'])` with `seen` hash `None` and `hash_since` never read. Contract point 1(b) asks for "no live workers"; on the marker path that is currently "no live-workers count was printed in one instantaneous read", which is exactly the false negative the docs identify for a pi pane mid-turn with zero subagents — the successor path answers it with 600 s of byte-identical screen, the marker path answers it with nothing. This is identical in `main@48c5000`, so I am not blocking the merge on it, but it is the one close path whose only evidence of "no live work" has no corroboration, and a marker close is destructive. Suggested before the timer is armed (a separate, operator-gated step): require the same `screen_stable`/`hash_since >= min_successor_seconds` condition on the marker branch, or state the residual risk explicitly in `docs/herdr-janitor.md` under "What this cannot prove" so a supervisor knows a marker can end an in-flight turn. **F2 — low, apply-mode reporting only.** `_execute` builds `by_pane = {d.pane_id: d for d in decisions}` and returns `[by_pane[key] for key in by_pane]`. The absent-marker rows synthesised in `decide()` all use `pane_id == "-"`, so two or more absent markers collapse to one row and the count drops (I confirmed: 2 rows in, 1 out). No close risk — those rows are `REPORT`/`absent` — but an operator reading an apply-mode report can under-count stale markers. **F3 — informational.** `require_activation` checks `is_symlink()` and then calls `read_text()`, which is a TOCTOU window in principle. Exploiting it needs write access to the 0700 state directory, which already implies the ability to write the activation file directly, so I do not consider it a hole. ## What this PR does not prove Nothing here has ever closed a relieved session. The ticket's Verify — the first real close of a relieved session — cannot happen until the timer is armed, and this PR deliberately does not arm it: no unit is installed, no timer enabled, no activation file created. "Armed, not proven" is the correct state, and the live dry-run I pasted above is the strongest evidence available today: it reports ten panes, refusals, and zero close candidates. Two further limits are honest ones the docs already state: a pi pane that prints nothing for ten minutes is indistinguishable from a finished one, so stillness is evidence of quiet, never of idleness; and `SUCCESSOR_STARTED` is self-declared text, held in check only by the numeric rule, the workspace rule, the age floor and the roster. I am approving the code as written. F1 is a recommendation for the arming step, not a request to change this diff, and I have said so explicitly so nobody reads this approval as evidence that a marker close has been shown safe on a busy pane.

ld-reviewer requested changes 2026-11-04 18:48:19 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: REQUEST_CHANGES

model: omniroute/deepseek-flash
head: 968bbf32c4
session: homelab-2634-review-2

What I did

Independent second review of head 968bbf32c4 (base 48c5000). I verified git -C .worktrees/HOMELAB-2634-janitor-relief rev-parse HEAD = that sha and git status --porcelain empty before trusting any file, ran the suite myself, ran a live read-only dry-run into an isolated state dir, and probed decide()/_execute directly from Python instead of only reading them. I did not rely on the first review. I have not taken the deployment state on trust: I did not verify that the timer or the activation file exist, and this review covers only the code at this head.

Contract points, with code evidence

1. Close only when relieved or retired, and with no live workers. decide() has exactly two branches that can set CLOSE_CANDIDATE. The successor branch requires a higher-generation pane of the same role base in the same workspace whose status is in NEW_SAFE_STATUSES and whose successor_age is at least min_successor_seconds, plus screen_stable, plus a readable non-empty screen with live_work(text) empty, plus relief evidence. The retire-marker branch requires the marker name, an empty decision.reasons, a readable non-empty screen, and empty live_work(text) (finding M1 below). Only sup-<rest>-<N> and <rest>-orchestrator-<N> parse into a relievable role; everything else is role-not-relievable. A blocked or unknown successor refuses (successor-status-blocked, successor-status-unknown) and there is no fallback: with gen1 idle, gen2 idle and gen3 blocked, both gen1 and gen2 stayed REPORT, because only newer[0] is ever examined.
2. wS/wH are unreachable. Config.allowed_workspaces is workspaces - NEVER_WORKSPACES, and _base_protections checks workspace in NEVER_WORKSPACES before the allowlist. --workspaces wS,wH yields workspaces= empty in the report header and every row refused; --workspaces wT,wS leaves only wT; a pane with a marker in wS or wH reports never-workspace|retire-marker-held. An empty allowlist fails closed on everything.
3. Fail closed. Unreadable/empty screen gives status-read-failed; unreadable roster blocks every path including markers (directory-unreadable); unproven age gives successor-too-new/successor-age-unproven; unstable screen gives screen-unstable; missing handoff evidence gives handoff-evidence-missing; duplicates refuse on both panes; unnamed panes and focused panes refuse. focused is not dead code: the live dry-run flagged the real focused pane wT:p17 with focused-pane.
4. Only four argv shapes. allowed_argv admits pane list, agent list, agent read <pane> --source recent-unwrapped --lines 240 --format text, and pane close <pane-id>, and HerdrCLI._run refuses anything else. No stop, prompt, send-text or upgrade argv can be constructed.
5. Dry-run default; real close needs --apply and activation. --dry-run/--apply are mutually exclusive and dry-run is the default; require_activation() runs before any herdr call and refuses a missing file, an empty one, wrong content, and a symlink (all four exercised). The dry-run created only report.txt and janitor.log under the isolated state dir — no seen.json, no close, no marker deletion.
6. PROTECTED_NAMES is {ld-hypervisor, el-director} and _base_protections applies it in every branch, so no marker path bypasses it at exact spelling. Case variants are not covered (finding L1).

Tests and dry-run

python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py' → 80 tests, OK (32 in test_herdr_janitor.py, 48 in test_herdr_janitor_relief.py, no skips).

Live dry-run: summary REPORT=9 total=9 directory_unreadable_refusals=0, exit 0, zero CLOSE_CANDIDATE and zero WOULD_CLOSE, over 9 real panes (wT/wR allowed, wH/wS refused as never-workspace). CI on the exact head is green: runs #254 and #255 on 968bbf32c4 all pass.

Findings

M1 (blocks) — the retire-marker branch has no minimum-observation floor and relies on text it has already declared untrustworthy. Code path: in decide(), the elif marker: branch checks only text readability and live_work(text), then sets CLOSE_CANDIDATE with reason retire-marker. It never calls screen_stable(), never consults seen, and never looks at min_successor_seconds; safe_markers() does not stat the marker's mtime either. So the first run that sees a marker closes on a single glance, with no prior observation of any kind. I confirmed three cases that close with seen={} (empty state dir, i.e. a first run): a marker plus a quiet screen, a marker plus a screen whose hash changed one second ago (hash_since = now - 1, which the successor branch calls screen-unstable), and a marker plus a pane mid-turn carrying no footer count. That last case is the exact hazard this module documents at screen_stable(): "a pi pane mid-turn with zero subagents prints no count at all, so 'no live work' cannot be distinguished from 'busy' by text alone". The successor branch spends 600 seconds and two observations defending against it; the marker branch defends against it with nothing, so contract point 1's "no live workers" and point 3's "unstable screen" are unproven on that path, not satisfied. The docs at docs/herdr-janitor.md ("does not need a successor or handoff", "uses the same protected/status/no-live-work/directory checks") match the code, so the defect is shared by the documentation rather than contradicted by it. Fix is small: require screen_stable() on the marker branch as well, or require the marker file's mtime to be at least min_successor_seconds old — either gives the operator's intent time to be re-checked against a quiescent pane. A deliberate marker is real authority, but authority to retire is not evidence that nothing is running, and a closed pane takes in-flight work with it.

L1 (does not block) — PROTECTED_NAMES is matched case-sensitively while every other name comparison in the file folds case through normalise_cell(). A pane named LD-Hypervisor or El-Director with a marker of the same spelling reaches CLOSE_CANDIDATE; I confirmed both. It needs two deliberate acts (a case-variant pane name and a matching case-variant marker), and in the live deployment the case-folded roster usually catches the same pane as directory-current — but el-director is not a roster row today, so for that name the protection rests solely on the exact-set match. Casefold the set and compare folded, then the "no path can bypass it" claim in contract point 6 holds for casing too.

L2 (does not block) — _execute's result mapping does not preserve one row per input row when several retire markers name absent panes. by_pane is keyed by decision.pane_id and every absent-marker row is synthesized with pane_id="-", so N absent markers collapse to one row: decide() returned 2 rows and _execute() returned 1 (beta won, alpha vanished). Real panes have unique pane ids, so no close decision can be lost and no close is affected; the damage is an undercount in the apply-mode report. Key the synthetic rows by marker name or build the result from an index instead of a pane-id map.

L3 (not a defect, noted) — the 600-second floor lives in main() (--min-successor-seconds 599 → "refusing successor observation below 600 seconds", exit 2, before any herdr call), not in Config, so a library caller can construct Config(min_successor_seconds=0). The tests do exactly that, which is fine; the CLI is the contract and it holds.

No other way to CLOSE_CANDIDATE occurred to me and survived probing. _execute cannot be bypassed: it runs only under --apply behind require_activation, it re-collects and re-decides immediately before each destructive call, and in a state-flip test (successor idle at decide time, blocked at revalidation time) the decision became SKIP_CHANGED with revalidation-failed and close_pane was never called.

What this does not prove

Armed, not proven. No real close has happened against this code — not in my session and, as far as the artefact shows, in any session. Nothing here demonstrates that herdr's pane close behaves as the janitor assumes, that a relief sequence resolves correctly on a real fleet, or that the marker deletion after a successful close leaves the right state. The dry-run is a read-only snapshot with an empty observation state, so it cannot exercise screen_stable at all. Above 600 seconds of identical-screen observation on a real successor pair remains untested, and it is the whole mechanism.

Verdict

REQUEST_CHANGES on M1 only. M1 is a fail-open on the one branch that needs no successor, no age, no anchor and no second observation — the cheapest path to closing a pane that may be working, and the one branch whose safety rests on the text this module itself says it cannot read. L1 and L2 are real but do not block; I would merge a fix for M1 with L1 and L2 folded in, and I will re-review promptly.

## Verdict: REQUEST_CHANGES model: omniroute/deepseek-flash head: 968bbf32c40238815e810f150837e6f8b97a9b27 session: homelab-2634-review-2 ## What I did Independent second review of head 968bbf32c40238815e810f150837e6f8b97a9b27 (base 48c5000). I verified `git -C .worktrees/HOMELAB-2634-janitor-relief rev-parse HEAD` = that sha and `git status --porcelain` empty before trusting any file, ran the suite myself, ran a live read-only dry-run into an isolated state dir, and probed `decide()`/`_execute` directly from Python instead of only reading them. I did not rely on the first review. I have not taken the deployment state on trust: I did not verify that the timer or the activation file exist, and this review covers only the code at this head. ## Contract points, with code evidence 1. Close only when relieved or retired, and with no live workers. `decide()` has exactly two branches that can set `CLOSE_CANDIDATE`. The successor branch requires a higher-generation pane of the same role base in the same workspace whose status is in `NEW_SAFE_STATUSES` and whose `successor_age` is at least `min_successor_seconds`, plus `screen_stable`, plus a readable non-empty screen with `live_work(text)` empty, plus relief evidence. The retire-marker branch requires the marker name, an empty `decision.reasons`, a readable non-empty screen, and empty `live_work(text)` (finding M1 below). Only `sup-<rest>-<N>` and `<rest>-orchestrator-<N>` parse into a relievable role; everything else is `role-not-relievable`. A blocked or `unknown` successor refuses (`successor-status-blocked`, `successor-status-unknown`) and there is no fallback: with gen1 idle, gen2 idle and gen3 blocked, both gen1 and gen2 stayed `REPORT`, because only `newer[0]` is ever examined. 2. wS/wH are unreachable. `Config.allowed_workspaces` is `workspaces - NEVER_WORKSPACES`, and `_base_protections` checks `workspace in NEVER_WORKSPACES` before the allowlist. `--workspaces wS,wH` yields `workspaces=` empty in the report header and every row refused; `--workspaces wT,wS` leaves only wT; a pane with a marker in wS or wH reports `never-workspace|retire-marker-held`. An empty allowlist fails closed on everything. 3. Fail closed. Unreadable/empty screen gives `status-read-failed`; unreadable roster blocks every path including markers (`directory-unreadable`); unproven age gives `successor-too-new`/`successor-age-unproven`; unstable screen gives `screen-unstable`; missing handoff evidence gives `handoff-evidence-missing`; duplicates refuse on both panes; unnamed panes and `focused` panes refuse. `focused` is not dead code: the live dry-run flagged the real focused pane `wT:p17` with `focused-pane`. 4. Only four argv shapes. `allowed_argv` admits `pane list`, `agent list`, `agent read <pane> --source recent-unwrapped --lines 240 --format text`, and `pane close <pane-id>`, and `HerdrCLI._run` refuses anything else. No stop, prompt, send-text or upgrade argv can be constructed. 5. Dry-run default; real close needs `--apply` and activation. `--dry-run`/`--apply` are mutually exclusive and dry-run is the default; `require_activation()` runs before any herdr call and refuses a missing file, an empty one, wrong content, and a symlink (all four exercised). The dry-run created only `report.txt` and `janitor.log` under the isolated state dir — no `seen.json`, no close, no marker deletion. 6. `PROTECTED_NAMES` is `{ld-hypervisor, el-director}` and `_base_protections` applies it in every branch, so no marker path bypasses it at exact spelling. Case variants are not covered (finding L1). ## Tests and dry-run `python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py'` → 80 tests, OK (32 in test_herdr_janitor.py, 48 in test_herdr_janitor_relief.py, no skips). Live dry-run: `summary REPORT=9 total=9 directory_unreadable_refusals=0`, exit 0, zero `CLOSE_CANDIDATE` and zero `WOULD_CLOSE`, over 9 real panes (wT/wR allowed, wH/wS refused as `never-workspace`). CI on the exact head is green: runs #254 and #255 on 968bbf32c4 all pass. ## Findings M1 (blocks) — the retire-marker branch has no minimum-observation floor and relies on text it has already declared untrustworthy. Code path: in `decide()`, the `elif marker:` branch checks only `text` readability and `live_work(text)`, then sets `CLOSE_CANDIDATE` with reason `retire-marker`. It never calls `screen_stable()`, never consults `seen`, and never looks at `min_successor_seconds`; `safe_markers()` does not stat the marker's mtime either. So the first run that sees a marker closes on a single glance, with no prior observation of any kind. I confirmed three cases that close with `seen={}` (empty state dir, i.e. a first run): a marker plus a quiet screen, a marker plus a screen whose hash changed one second ago (`hash_since = now - 1`, which the successor branch calls `screen-unstable`), and a marker plus a pane mid-turn carrying no footer count. That last case is the exact hazard this module documents at `screen_stable()`: "a pi pane mid-turn with zero subagents prints no count at all, so 'no live work' cannot be distinguished from 'busy' by text alone". The successor branch spends 600 seconds and two observations defending against it; the marker branch defends against it with nothing, so contract point 1's "no live workers" and point 3's "unstable screen" are unproven on that path, not satisfied. The docs at docs/herdr-janitor.md ("does not need a successor or handoff", "uses the same protected/status/no-live-work/directory checks") match the code, so the defect is shared by the documentation rather than contradicted by it. Fix is small: require `screen_stable()` on the marker branch as well, or require the marker file's mtime to be at least `min_successor_seconds` old — either gives the operator's intent time to be re-checked against a quiescent pane. A deliberate marker is real authority, but authority to retire is not evidence that nothing is running, and a closed pane takes in-flight work with it. L1 (does not block) — `PROTECTED_NAMES` is matched case-sensitively while every other name comparison in the file folds case through `normalise_cell()`. A pane named `LD-Hypervisor` or `El-Director` with a marker of the same spelling reaches `CLOSE_CANDIDATE`; I confirmed both. It needs two deliberate acts (a case-variant pane name and a matching case-variant marker), and in the live deployment the case-folded roster usually catches the same pane as `directory-current` — but el-director is not a roster row today, so for that name the protection rests solely on the exact-set match. Casefold the set and compare folded, then the "no path can bypass it" claim in contract point 6 holds for casing too. L2 (does not block) — `_execute`'s result mapping does not preserve one row per input row when several retire markers name absent panes. `by_pane` is keyed by `decision.pane_id` and every absent-marker row is synthesized with `pane_id="-"`, so N absent markers collapse to one row: `decide()` returned 2 rows and `_execute()` returned 1 (`beta` won, `alpha` vanished). Real panes have unique pane ids, so no close decision can be lost and no close is affected; the damage is an undercount in the apply-mode report. Key the synthetic rows by marker name or build the result from an index instead of a pane-id map. L3 (not a defect, noted) — the 600-second floor lives in `main()` (`--min-successor-seconds 599` → "refusing successor observation below 600 seconds", exit 2, before any herdr call), not in `Config`, so a library caller can construct `Config(min_successor_seconds=0)`. The tests do exactly that, which is fine; the CLI is the contract and it holds. No other way to `CLOSE_CANDIDATE` occurred to me and survived probing. `_execute` cannot be bypassed: it runs only under `--apply` behind `require_activation`, it re-collects and re-decides immediately before each destructive call, and in a state-flip test (successor idle at decide time, blocked at revalidation time) the decision became `SKIP_CHANGED` with `revalidation-failed` and `close_pane` was never called. ## What this does not prove Armed, not proven. No real close has happened against this code — not in my session and, as far as the artefact shows, in any session. Nothing here demonstrates that herdr's `pane close` behaves as the janitor assumes, that a relief sequence resolves correctly on a real fleet, or that the marker deletion after a successful close leaves the right state. The dry-run is a read-only snapshot with an empty observation state, so it cannot exercise `screen_stable` at all. Above 600 seconds of identical-screen observation on a real successor pair remains untested, and it is the whole mechanism. ## Verdict REQUEST_CHANGES on M1 only. M1 is a fail-open on the one branch that needs no successor, no age, no anchor and no second observation — the cheapest path to closing a pane that may be working, and the one branch whose safety rests on the text this module itself says it cannot read. L1 and L2 are real but do not block; I would merge a fix for M1 with L1 and L2 folded in, and I will re-review promptly.

aaron added 1 commit 2026-11-04 18:54:25 +00:00

HOMELAB-2634: require screen stillness before a retire-marker close (review M1); fold protected names (L1) ... 649897a9cf

M1: the retire-marker branch of decide() reached CLOSE_CANDIDATE after only a
readable screen and an empty live_work(text), so a pane could be closed on the
very first observation -- including a mid-turn pi pane whose footer prints no
agent counter, the exact hazard screen_stable documents. The branch now demands
the same continuous-observation floor as the successor branch (same call, same
screen-unstable reason). A marker appearing now closes nothing this cycle; the
close lands on a later cycle once the screen has been byte-identical for
min_successor_seconds. Marker closes still need no successor and no handoff, and
live work / unreadable screens still refuse first.

L1: _base_protections compared the pane name against PROTECTED_NAMES case
sensitively while every other name comparison folds through normalise_cell, so
El-Director or LD-Hypervisor were unprotected. The check now folds through
normalise_cell against PROTECTED_NAME_KEYS, derived from PROTECTED_NAMES, which
stays the single source of truth. No other name check changed.

Existing tests that encoded close-on-first-sight now supply the prior
byte-identical observation the new contract requires; none were deleted or
weakened. New MarkerStillnessTests and ProtectedNameFoldingTests cover the floor,
the change and rename resets, the preserved live-*/status-read-failed refusals,
and all four protected spellings. docs/herdr-janitor.md states the stillness
requirement for the marker path.

ld-reviewer approved these changes 2026-11-04 19:01:37 +00:00

Dismissed

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 649897a9cf
session: homelab-2634-review-4

I approve this head. I reviewed it from its own detached worktree created with git worktree add under a temporary directory, so no checkout was shared with another session. Everything below was executed against 649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6 and nothing earlier. I am the second independent approval for a dangerous-class change.

The M1 fix is real and it is complete

I did not take the commit message's word for it. I imported decide() from the reviewed file and drove the retire-marker branch directly, holding the pane, the marker set and the text fixed so that only the previous observation varied. With an empty seen the marker path now returns REPORT reasons=screen-unstable. A seen row whose name differs gives screen-unstable. A row whose text_hash differs gives screen-unstable. A hash_since one second old gives screen-unstable. Exactly 600 seconds old gives CLOSE_CANDIDATE and 601 seconds gives CLOSE_CANDIDATE, which is the inclusive floor the successor branch already had and the boundary I would expect. hash_since set to NaN, to +Inf, to True, to False, to the string "400", to None, and to a value in the future all refuse. The boolean and non-finite cases are refused because screen_stable validates through valid_first_seen, so a JSON true cannot be coerced into an epoch.

I then tried to reach a close on a first sight through every other branch. The successor path on an empty seen refuses twice over, with successor-age-unproven and screen-unstable, which is belt and braces and correct. A marker plus a live higher-generation successor also refuses, because the marker branch is evaluated before the successor branch and carries the stillness floor with it. A pane with no marker and a name outside the two named role families gets role-not-relievable and stays report only. An unnamed pane gets unnamed. I found no path to CLOSE_CANDIDATE on a first observation.

The L1 fix is also real. PROTECTED_NAME_KEYS is derived from PROTECTED_NAMES through normalise_cell, so the set stays the single source of truth rather than becoming a second hand-maintained list. ld-hypervisor, LD-Hypervisor, El-Director, el-director, padded " EL-DIRECTOR ", LD-HYPERVISOR and even the accented ĺd-hypervisor all now carry protected-name. The folding is strictly additive to protection; there is no spelling that matched before and fails now. Names like ld_hypervisor or ld hypervisor are not folded to a protected key, but neither can they be closed except through a marker file the operator must touch under that exact name, and they are outside both role families, so this is not a hole I can turn into a close.

Never-workspaces and the allowlist

The wS/wH guarantee holds by construction rather than by blacklist luck. Config.allowed_workspaces subtracts NEVER_WORKSPACES and every close path then requires membership in the remainder. So a pane in wS or wH is refused even when the operator passes --workspaces wS,wH,wT,wR: the subtraction still leaves wT,wR and the pane gets never-workspace. Spellings ws, WS, Wh, "wS ", " wS", "wS,wH" and the empty workspace all get workspace-not-allowed, because the allowlist is positive and the default is wT,wR. Duplicate and empty names are refused as duplicate-name and unnamed. A focused pane in an allowed workspace with a stable marker is refused as focused-pane. I cannot get wS or wH into the allowlist by any spelling I tried.

The per-close revalidation

_execute re-collects from herdr and re-runs decide against the same loaded seen immediately before each close, and it compares successor, evidence, directory state and marker before calling pane close. I confirmed by construction that this is strict in the direction that matters: because the previous seen is not refreshed between candidates, a screen that changed between the report and the close fails screen_stable in the fresh decision and the close degrades to SKIP_CHANGED. A pane that became focused, that moved into wS, or that changed status all gain base reasons and therefore degrade as well. A failing close yields CLOSE_FAILED and retains the marker; the activation gate refuses a symlink, a directory, a missing file and wrong text.

The pane_id="-" row collapse reported against the previous head is still present. _execute builds by_pane = {decision.pane_id: decision ...} and returns [by_pane[key] for key in by_pane], so every absent-retire-marker row collapses onto the single key "-". Two ghost markers returned two rows of which one was the ghost and the report printed one retire-marker-pane-absent line for two markers. I checked whether this can produce a wrong close and it cannot: the destructive loop iterates the original list by identity and the dict is only used to build the returned list, so a candidate is never dropped or rerouted. It is a report undercount, and in a contrived case where herdr itself returned a pane whose id is literally -, it could hide a CLOSE_FAILED from the summary and from main's exit code. I would still like both of these cleaned up.

Tests, CI, dry-run

89 unit tests pass under python3 -m unittest discover -s tests -v. I also ran the previous head's suite for comparison: 80 tests. So this commit adds nine, which is exactly the seven MarkerStillnessTests plus two ProtectedNameFoldingTests, and no test method was deleted (git diff reports zero removed def test_). The two tests that previously asserted a marker close on empty seen.json now seed the prior byte-identical observation and then still assert that dry-run writes no seen.json and makes no herdr call, so they are strengthened rather than weakened: they now exercise a real candidate instead of a vacuous one.

CI on the exact head: the python job in run 257 is green (7s, the workflow that actually runs this suite). The second run on the same commit, run 256, is green on fourteen jobs and still running pi-swarm-suite, so the aggregate suite has not concluded at the time of this review; nothing on this head is red.

Read-only dry-run against the live fleet into an isolated state directory: 9 panes, REPORT=9, zero CLOSE_CANDIDATE and zero WOULD_CLOSE, directory_unreadable_refusals=0. The three wS/wH panes were reported and refused, el-director was refused as protected-name|focused-pane, and the only files created were report.txt and janitor.log. Worth stating plainly: because a dry-run never writes seen.json, a dry-run into a fresh state directory can never show a candidate at all, so dry-run is a safety check and not a preview of the next apply cycle.

Findings

• Low severity, does not touch any close decision, pre-existing: the pane_id="-" collapse in _execute undercounts absent-marker rows in the report and could in a contrived case mask a failed close from the exit code. No wrong close is reachable through it.
• Low severity, hardening: load_seen validates first_seen for finiteness but never validates hash_since. A seen.json carrying hash_since: -1e9 yields instant stillness and a CLOSE_CANDIDATE. Reproduced. This requires write access to the state directory, which already contains the activate file, so it grants no authority the attacker did not already hold; still, a hash_since that is not finite, not a number, or negative should be discarded, and hash_since > last_seen likewise.
• Informational: the CLI enforces the 600 s floor (--min-successor-seconds 599 exits 2) and decide uses config.min_successor_seconds, but nothing stops a programmatic caller from constructing Config(min_successor_seconds=1), which I confirmed closes at one second. The deployed unit runs the CLI, so this is a library-boundary note.
• Informational: marker-name matching and duplicate-name counting stay case-sensitive while protected names and the roster fold. Both directions fail closed, so I would not change them.
• Informational: text_hash covers the last 40 lines only, so a change above the tail does not restart the floor. The documentation names this window and a working pane's growth lands in the tail, so I accept it.

What I took on trust

That herdr 0.9.1's agent read --source recent-unwrapped --lines 240 --format text returns the operator's visible screen and that the last 40 lines are the part that moves while a turn is in flight; I saw those 240-line reads succeed live but I did not correlate a mid-turn pane's tail over two real cycles. allowed_argv I verified only by direct calls: pane close with no argument, with an extra argument, with an empty id, pane kill, agent send-text, server stop and pane list extra are all refused, and only the exact four allowed argv shapes pass. I did not run --apply against the live fleet and I have not: this review closes nothing. The change is armed, not proven; no real close has happened, the timer is not installed, and the first genuine relief close remains HOMELAB-2634's own verification.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6 session: homelab-2634-review-4 I approve this head. I reviewed it from its own detached worktree created with `git worktree add` under a temporary directory, so no checkout was shared with another session. Everything below was executed against `649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6` and nothing earlier. I am the second independent approval for a dangerous-class change. ### The M1 fix is real and it is complete I did not take the commit message's word for it. I imported `decide()` from the reviewed file and drove the retire-marker branch directly, holding the pane, the marker set and the text fixed so that only the previous observation varied. With an empty `seen` the marker path now returns `REPORT reasons=screen-unstable`. A `seen` row whose `name` differs gives `screen-unstable`. A row whose `text_hash` differs gives `screen-unstable`. A `hash_since` one second old gives `screen-unstable`. Exactly 600 seconds old gives `CLOSE_CANDIDATE` and 601 seconds gives `CLOSE_CANDIDATE`, which is the inclusive floor the successor branch already had and the boundary I would expect. `hash_since` set to NaN, to +Inf, to `True`, to `False`, to the string `"400"`, to `None`, and to a value in the future all refuse. The boolean and non-finite cases are refused because `screen_stable` validates through `valid_first_seen`, so a JSON `true` cannot be coerced into an epoch. I then tried to reach a close on a first sight through every other branch. The successor path on an empty `seen` refuses twice over, with `successor-age-unproven` and `screen-unstable`, which is belt and braces and correct. A marker plus a live higher-generation successor also refuses, because the marker branch is evaluated before the successor branch and carries the stillness floor with it. A pane with no marker and a name outside the two named role families gets `role-not-relievable` and stays report only. An unnamed pane gets `unnamed`. I found no path to `CLOSE_CANDIDATE` on a first observation. The L1 fix is also real. `PROTECTED_NAME_KEYS` is derived from `PROTECTED_NAMES` through `normalise_cell`, so the set stays the single source of truth rather than becoming a second hand-maintained list. `ld-hypervisor`, `LD-Hypervisor`, `El-Director`, `el-director`, padded `" EL-DIRECTOR "`, `LD-HYPERVISOR` and even the accented `ĺd-hypervisor` all now carry `protected-name`. The folding is strictly additive to protection; there is no spelling that matched before and fails now. Names like `ld_hypervisor` or `ld hypervisor` are not folded to a protected key, but neither can they be closed except through a marker file the operator must touch under that exact name, and they are outside both role families, so this is not a hole I can turn into a close. ### Never-workspaces and the allowlist The `wS`/`wH` guarantee holds by construction rather than by blacklist luck. `Config.allowed_workspaces` subtracts `NEVER_WORKSPACES` and every close path then requires membership in the remainder. So a pane in `wS` or `wH` is refused even when the operator passes `--workspaces wS,wH,wT,wR`: the subtraction still leaves `wT,wR` and the pane gets `never-workspace`. Spellings `ws`, `WS`, `Wh`, `"wS "`, `" wS"`, `"wS,wH"` and the empty workspace all get `workspace-not-allowed`, because the allowlist is positive and the default is `wT,wR`. Duplicate and empty names are refused as `duplicate-name` and `unnamed`. A focused pane in an allowed workspace with a stable marker is refused as `focused-pane`. I cannot get `wS` or `wH` into the allowlist by any spelling I tried. ### The per-close revalidation `_execute` re-collects from herdr and re-runs `decide` against the same loaded `seen` immediately before each close, and it compares successor, evidence, directory state and marker before calling `pane close`. I confirmed by construction that this is strict in the direction that matters: because the previous `seen` is not refreshed between candidates, a screen that changed between the report and the close fails `screen_stable` in the fresh decision and the close degrades to `SKIP_CHANGED`. A pane that became focused, that moved into `wS`, or that changed status all gain base reasons and therefore degrade as well. A failing close yields `CLOSE_FAILED` and retains the marker; the activation gate refuses a symlink, a directory, a missing file and wrong text. The `pane_id="-"` row collapse reported against the previous head is still present. `_execute` builds `by_pane = {decision.pane_id: decision ...}` and returns `[by_pane[key] for key in by_pane]`, so every absent-retire-marker row collapses onto the single key `"-"`. Two ghost markers returned two rows of which one was the ghost and the report printed one `retire-marker-pane-absent` line for two markers. I checked whether this can produce a wrong close and it cannot: the destructive loop iterates the original list by identity and the dict is only used to build the returned list, so a candidate is never dropped or rerouted. It is a report undercount, and in a contrived case where herdr itself returned a pane whose id is literally `-`, it could hide a `CLOSE_FAILED` from the summary and from `main`'s exit code. I would still like both of these cleaned up. ### Tests, CI, dry-run 89 unit tests pass under `python3 -m unittest discover -s tests -v`. I also ran the previous head's suite for comparison: 80 tests. So this commit adds nine, which is exactly the seven `MarkerStillnessTests` plus two `ProtectedNameFoldingTests`, and no test method was deleted (`git diff` reports zero removed `def test_`). The two tests that previously asserted a marker close on empty `seen.json` now seed the prior byte-identical observation and then still assert that dry-run writes no `seen.json` and makes no herdr call, so they are strengthened rather than weakened: they now exercise a real candidate instead of a vacuous one. CI on the exact head: the `python` job in run 257 is green (7s, the workflow that actually runs this suite). The second run on the same commit, run 256, is green on fourteen jobs and still running `pi-swarm-suite`, so the aggregate suite has not concluded at the time of this review; nothing on this head is red. Read-only dry-run against the live fleet into an isolated state directory: 9 panes, `REPORT=9`, zero `CLOSE_CANDIDATE` and zero `WOULD_CLOSE`, `directory_unreadable_refusals=0`. The three `wS`/`wH` panes were reported and refused, `el-director` was refused as `protected-name|focused-pane`, and the only files created were `report.txt` and `janitor.log`. Worth stating plainly: because a dry-run never writes `seen.json`, a dry-run into a fresh state directory can never show a candidate at all, so dry-run is a safety check and not a preview of the next apply cycle. ### Findings - Low severity, does not touch any close decision, pre-existing: the `pane_id="-"` collapse in `_execute` undercounts absent-marker rows in the report and could in a contrived case mask a failed close from the exit code. No wrong close is reachable through it. - Low severity, hardening: `load_seen` validates `first_seen` for finiteness but never validates `hash_since`. A `seen.json` carrying `hash_since: -1e9` yields instant stillness and a `CLOSE_CANDIDATE`. Reproduced. This requires write access to the state directory, which already contains the `activate` file, so it grants no authority the attacker did not already hold; still, a `hash_since` that is not finite, not a number, or negative should be discarded, and `hash_since > last_seen` likewise. - Informational: the CLI enforces the 600 s floor (`--min-successor-seconds 599` exits 2) and `decide` uses `config.min_successor_seconds`, but nothing stops a programmatic caller from constructing `Config(min_successor_seconds=1)`, which I confirmed closes at one second. The deployed unit runs the CLI, so this is a library-boundary note. - Informational: marker-name matching and duplicate-name counting stay case-sensitive while protected names and the roster fold. Both directions fail closed, so I would not change them. - Informational: `text_hash` covers the last 40 lines only, so a change above the tail does not restart the floor. The documentation names this window and a working pane's growth lands in the tail, so I accept it. ### What I took on trust That herdr 0.9.1's `agent read --source recent-unwrapped --lines 240 --format text` returns the operator's visible screen and that the last 40 lines are the part that moves while a turn is in flight; I saw those 240-line reads succeed live but I did not correlate a mid-turn pane's tail over two real cycles. `allowed_argv` I verified only by direct calls: `pane close` with no argument, with an extra argument, with an empty id, `pane kill`, `agent send-text`, `server stop` and `pane list extra` are all refused, and only the exact four allowed argv shapes pass. I did not run `--apply` against the live fleet and I have not: this review closes nothing. The change is armed, not proven; no real close has happened, the timer is not installed, and the first genuine relief close remains HOMELAB-2634's own verification.

ld-reviewer approved these changes 2026-11-04 19:01:56 +00:00

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: 649897a9cf
session: homelab-2634-review-3

Reviewed the exact head 649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6 (base main@48c5000) in the worktree plane/HOMELAB-2634-janitor-relief. git rev-parse HEAD matched the head and git status --porcelain was empty before I read anything, so the files I probed are the files under review.

This is the third review session on HOMELAB-2634 and my second pass over the M1 fix. I did not take either finding on trust: I drove decide() from Python directly, importing scripts/herdr_janitor and handing it a default-constructed J.Directory() (readable, zero rows) so no roster objection could mask the branch behaviour I was trying to observe — every refusal I report below is the branch's own, not the roster gate's.

Probes, with what I actually saw

1. Marker + quiet screen + empty seen ⇒ REPORT, reasons=screen-unstable. Not a candidate, as required. This is M1 itself: a marker appearing in this cycle closes nothing in this cycle.
2. Marker + quiet screen + seen row {name, text_hash(quiet), hash_since=now-700} ⇒ CLOSE_CANDIDATE, evidence=retire-marker, successor=None, reasons=retire-marker. Three things at once: the floor is satisfiable, the marker is still independent authority, and the close path still needs neither a successor nor handoff evidence.
3. Same but hash_since=now-5 ⇒ REPORT, screen-unstable. The floor is a real duration test, not a presence test.
4. Same but the stored text_hash differs from the current text ⇒ REPORT, screen-unstable. A changed screen restarts the clock rather than reusing it.
5. Marker + live footer 3 running agents ⇒ REPORT, reasons=live-3-agent. The live-work refusal still fires first, before any stillness consideration.
6. Marker + None read ⇒ REPORT, status-read-failed; marker + empty-string read ⇒ REPORT, status-read-failed. An unreadable screen is never mistaken for a quiet one.
7. el-director, El-Director, ld-hypervisor, LD-Hypervisor — all four ⇒ REPORT with protected-name. L1 is genuinely closed: the check now folds through the file's existing normalise_cell, so a capitalisation change can no longer silently unprotect the voice. I confirmed folding only ever widens protection and that normalise_cell("") cannot match, so an unnamed pane is not accidentally swept into the protected set.
8. --workspaces wS,wH ⇒ effective allowlist []; --workspaces wT,wS ⇒ ['wT']; --workspaces wT,wH,wR ⇒ ['wR','wT']. wS/wH are subtracted, never merely defaulted. Note that the report header prints the effective set (workspaces=wR,wT) alongside never_workspaces=wH,wS, so an operator reading the artefact can see the subtraction happened.
9. --min-successor-seconds 599 ⇒ stderr refusing successor observation below 600 seconds, exit 2. The CLI floor is still enforced independently of Config.
10. --apply with no activation file (against a logging fake herdr) ⇒ exit 1, message real mode is not activated: .../state/activate, and the fake herdr recorded zero calls. Activation is checked before any read or close.

Diff check: nothing the fix broke

The two changed hunks are exactly the two findings. PROTECTED_NAMES remains the single source of truth and PROTECTED_NAME_KEYS is derived from it; the successor branch's text is byte-identical to the previous head. I verified empirically that the marker branch still requires no successor and no handoff evidence (probe 2: successor=None, evidence=retire-marker), which is the correct contract — the marker is authority for what to close, and the succession rule is authority for a different class of close. The marker branch only borrows screen_stable, which is authority for when. Those are three separate questions and the code keeps them separate.

I also confirmed the ordering is fail-closed: status-read-failed is checked before the stillness call, so a missing read never reaches screen_stable and never produces a candidate; live-* is appended before the stillness call too, so a busy pane refuses on work rather than on quietness. Both refusals were preserved, not replaced.

One consequence worth stating plainly for the record: a marker close now needs at least two cycles — the cycle that first sees the marker and the cycle after the screen has been byte-identical for the floor. That is intended and is documented in docs/herdr-janitor.md, which now says "Every close — numbered successor or retire marker".

Tests

python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py' in the worktree: 89 tests, OK, run in 0.118s. No skips, no failures.

On whether the fix weakened existing tests: it did not. No test function was deleted (git show of the fix commit contains zero removed def test_ or assert lines across tests/). Counting definitions, test_herdr_janitor.py is flat at 32 → 32 and test_herdr_janitor_relief.py grows 48 → 57, so nine new tests were added and none removed. Each of the six updated bodies is a stronger assertion, not a relaxed one:

• test_read_error_and_valid_no_live_footer_controls (marker subTest): the quiet case now supplies a prior byte-identical observation and still asserts CLOSE_CANDIDATE; the None case still supplies {} and still asserts REPORT/status-read-failed. It now proves the close needs a prior observation rather than assuming one.
• test_retire_marker_does_not_need_successor_or_handoff: supplies the seen row and still asserts CLOSE_CANDIDATE + evidence=retire-marker. The point of the test — independence from succession — is preserved and now demonstrated under the floor.
• test_dry_run_has_no_pane_marker_or_seen_mutations: previously reached "candidate" partly because of the bug; now it is a legitimately eligible candidate, and the assertion that a dry run mutates neither the marker nor seen.json bytes is therefore stronger, not weaker.
• test_real_retire_close_deletes_marker_only_after_success and test_failed_close_retains_marker: both now seed a stable observation, so the close is attempted through the real contract instead of around it.
• test_dry_run_with_a_candidate_closes_nothing_and_writes_no_seen (relief): same shape — a genuine candidate, same no-mutation and WOULD_CLOSE assertions.

The nine new tests cover what the fix claims: first sight never closes, an observation started this cycle never closes, a screen that changed since the last look never closes, a renamed pane restarts the floor, the floor does eventually permit a close, live-1-agent still refuses, unreadable/empty screens still refuse, all four protected spellings are protected, and the protected set stays the single source of truth.

Dry run

python3 scripts/herdr_janitor/janitor.py --state-dir "$(mktemp -d)" --dry-run, read-only against the live fleet with the real roster readable:

summary REPORT=9 total=9 directory_unreadable_refusals=0

Zero CLOSE_CANDIDATE / WOULD_CLOSE lines, exit 0. Nine panes reported, all refused, and the roster parsed (directory_unreadable_refusals=0), so the quiet result is real observation and not a silent unreadable-directory stop. The refusals are what I would expect from the current fleet: never-workspace on the wH/wS panes, protected-name|focused-pane on el-director, no-successor on infra-stab-orchestrator-5, role-not-relievable on the unnamed and out-of-family panes.

CI

On the exact head I read two Actions runs, and both pass. Run #256 (the full matrix) is green: ask-user-question, forgejo, forgejo-ci, mcp-bridge, nats-intercom, pi-friction, pi-otel-telemetry, pi-swarm-suite, plane, pr-title, redact, refocus, skill-args and todo all succeeded. Run #257 (python) also succeeded. So CI on 649897a is green, and I am calling it green on that evidence rather than on the earlier reading — when I first polled, run #256 was still in progress with pi-swarm-suite running, and I did not call it green then. The change touches scripts/herdr_janitor/, its two test files, and docs/herdr-janitor.md.

What this PR has not proven

No real close has happened. Not one, by me or by the janitor. Every close I exercised was a CLOSE_CANDIDATE inside a decide() call or a WOULD_CLOSE in a dry-run report, and the one --apply invocation I ran refused at the activation gate before issuing a single herdr call. This is armed, not proven: the code is a defensible close authority on paper and in probes, and the first real close will be the first actual evidence that pane close on a relieved or retired pane does what the operator wants and nothing more. I would want the first live cycle run with the marker list empty, watching report.txt and janitor.log, before trusting it with a marker at all.

I found no blocker and no lesser finding worth filing. Approving.

## Verdict: APPROVED model: omniroute/deepseek-flash head: 649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6 session: homelab-2634-review-3 Reviewed the exact head `649897a9cf88ddb68fdf21bdea7e2b42d8fe85e6` (base `main@48c5000`) in the worktree `plane/HOMELAB-2634-janitor-relief`. `git rev-parse HEAD` matched the head and `git status --porcelain` was empty before I read anything, so the files I probed are the files under review. This is the third review session on HOMELAB-2634 and my second pass over the M1 fix. I did not take either finding on trust: I drove `decide()` from Python directly, importing `scripts/herdr_janitor` and handing it a default-constructed `J.Directory()` (readable, zero rows) so no roster objection could mask the branch behaviour I was trying to observe — every refusal I report below is the branch's own, not the roster gate's. ### Probes, with what I actually saw 1. Marker + quiet screen + **empty `seen`** ⇒ `REPORT`, `reasons=screen-unstable`. Not a candidate, as required. This is M1 itself: a marker appearing in this cycle closes nothing in this cycle. 2. Marker + quiet screen + `seen` row `{name, text_hash(quiet), hash_since=now-700}` ⇒ `CLOSE_CANDIDATE`, `evidence=retire-marker`, `successor=None`, `reasons=retire-marker`. Three things at once: the floor is satisfiable, the marker is still independent authority, and the close path still needs neither a successor nor handoff evidence. 3. Same but `hash_since=now-5` ⇒ `REPORT`, `screen-unstable`. The floor is a real duration test, not a presence test. 4. Same but the stored `text_hash` differs from the current text ⇒ `REPORT`, `screen-unstable`. A changed screen restarts the clock rather than reusing it. 5. Marker + live footer `3 running agents` ⇒ `REPORT`, `reasons=live-3-agent`. The live-work refusal still fires first, before any stillness consideration. 6. Marker + `None` read ⇒ `REPORT`, `status-read-failed`; marker + empty-string read ⇒ `REPORT`, `status-read-failed`. An unreadable screen is never mistaken for a quiet one. 7. `el-director`, `El-Director`, `ld-hypervisor`, `LD-Hypervisor` — all four ⇒ `REPORT` with `protected-name`. L1 is genuinely closed: the check now folds through the file's existing `normalise_cell`, so a capitalisation change can no longer silently unprotect the voice. I confirmed folding only ever widens protection and that `normalise_cell("")` cannot match, so an unnamed pane is not accidentally swept into the protected set. 8. `--workspaces wS,wH` ⇒ effective allowlist `[]`; `--workspaces wT,wS` ⇒ `['wT']`; `--workspaces wT,wH,wR` ⇒ `['wR','wT']`. `wS`/`wH` are subtracted, never merely defaulted. Note that the report header prints the effective set (`workspaces=wR,wT`) alongside `never_workspaces=wH,wS`, so an operator reading the artefact can see the subtraction happened. 9. `--min-successor-seconds 599` ⇒ stderr `refusing successor observation below 600 seconds`, exit **2**. The CLI floor is still enforced independently of `Config`. 10. `--apply` with no activation file (against a logging fake `herdr`) ⇒ exit **1**, message `real mode is not activated: .../state/activate`, and the fake herdr recorded **zero** calls. Activation is checked before any read or close. ### Diff check: nothing the fix broke The two changed hunks are exactly the two findings. `PROTECTED_NAMES` remains the single source of truth and `PROTECTED_NAME_KEYS` is derived from it; the successor branch's text is byte-identical to the previous head. I verified empirically that the marker branch still requires **no successor and no handoff evidence** (probe 2: `successor=None`, `evidence=retire-marker`), which is the correct contract — the marker is authority for *what* to close, and the succession rule is authority for a *different* class of close. The marker branch only borrows `screen_stable`, which is authority for *when*. Those are three separate questions and the code keeps them separate. I also confirmed the ordering is fail-closed: `status-read-failed` is checked before the stillness call, so a missing read never reaches `screen_stable` and never produces a candidate; `live-*` is appended before the stillness call too, so a busy pane refuses on work rather than on quietness. Both refusals were preserved, not replaced. One consequence worth stating plainly for the record: a marker close now needs at least two cycles — the cycle that first sees the marker and the cycle after the screen has been byte-identical for the floor. That is intended and is documented in `docs/herdr-janitor.md`, which now says "Every close — numbered successor or retire marker". ### Tests `python3 -m unittest discover -s tests -p 'test_herdr_janitor*.py'` in the worktree: **89 tests, OK**, run in 0.118s. No skips, no failures. On whether the fix weakened existing tests: it did not. No test function was deleted (`git show` of the fix commit contains zero removed `def test_` or `assert` lines across `tests/`). Counting definitions, `test_herdr_janitor.py` is flat at 32 → 32 and `test_herdr_janitor_relief.py` grows 48 → 57, so nine new tests were added and none removed. Each of the six updated bodies is a **stronger** assertion, not a relaxed one: - `test_read_error_and_valid_no_live_footer_controls` (marker subTest): the quiet case now supplies a prior byte-identical observation and still asserts `CLOSE_CANDIDATE`; the `None` case still supplies `{}` and still asserts `REPORT`/`status-read-failed`. It now proves the close needs a prior observation rather than assuming one. - `test_retire_marker_does_not_need_successor_or_handoff`: supplies the `seen` row and still asserts `CLOSE_CANDIDATE` + `evidence=retire-marker`. The point of the test — independence from succession — is preserved and now demonstrated under the floor. - `test_dry_run_has_no_pane_marker_or_seen_mutations`: previously reached "candidate" partly because of the bug; now it is a *legitimately* eligible candidate, and the assertion that a dry run mutates neither the marker nor `seen.json` bytes is therefore stronger, not weaker. - `test_real_retire_close_deletes_marker_only_after_success` and `test_failed_close_retains_marker`: both now seed a stable observation, so the close is attempted through the real contract instead of around it. - `test_dry_run_with_a_candidate_closes_nothing_and_writes_no_seen` (relief): same shape — a genuine candidate, same no-mutation and `WOULD_CLOSE` assertions. The nine new tests cover what the fix claims: first sight never closes, an observation started this cycle never closes, a screen that changed since the last look never closes, a renamed pane restarts the floor, the floor does eventually permit a close, `live-1-agent` still refuses, unreadable/empty screens still refuse, all four protected spellings are protected, and the protected set stays the single source of truth. ### Dry run `python3 scripts/herdr_janitor/janitor.py --state-dir "$(mktemp -d)" --dry-run`, read-only against the live fleet with the real roster readable: ``` summary REPORT=9 total=9 directory_unreadable_refusals=0 ``` **Zero** `CLOSE_CANDIDATE` / `WOULD_CLOSE` lines, exit 0. Nine panes reported, all refused, and the roster parsed (`directory_unreadable_refusals=0`), so the quiet result is real observation and not a silent unreadable-directory stop. The refusals are what I would expect from the current fleet: `never-workspace` on the `wH`/`wS` panes, `protected-name|focused-pane` on `el-director`, `no-successor` on `infra-stab-orchestrator-5`, `role-not-relievable` on the unnamed and out-of-family panes. ### CI On the exact head I read two Actions runs, and **both pass**. Run #256 (the full matrix) is green: `ask-user-question`, `forgejo`, `forgejo-ci`, `mcp-bridge`, `nats-intercom`, `pi-friction`, `pi-otel-telemetry`, `pi-swarm-suite`, `plane`, `pr-title`, `redact`, `refocus`, `skill-args` and `todo` all succeeded. Run #257 (`python`) also succeeded. So CI on `649897a` is green, and I am calling it green on that evidence rather than on the earlier reading — when I first polled, run #256 was still in progress with `pi-swarm-suite` running, and I did not call it green then. The change touches `scripts/herdr_janitor/`, its two test files, and `docs/herdr-janitor.md`. ### What this PR has not proven No real close has happened. Not one, by me or by the janitor. Every close I exercised was a `CLOSE_CANDIDATE` inside a `decide()` call or a `WOULD_CLOSE` in a dry-run report, and the one `--apply` invocation I ran refused at the activation gate before issuing a single herdr call. This is **armed, not proven**: the code is a defensible close authority on paper and in probes, and the first real close will be the first actual evidence that `pane close` on a relieved or retired pane does what the operator wants and nothing more. I would want the first live cycle run with the marker list empty, watching `report.txt` and `janitor.log`, before trusting it with a marker at all. I found no blocker and no lesser finding worth filing. Approving.

pi-landing merged commit 945d97c786 into main 2026-11-04 19:03:49 +00:00

pi-landing referenced this pull request from a commit 2026-11-04 19:03:51 +00:00

Merge pull request 'HOMELAB-2634: fix(herdr): recognise relieved sessions, fail closed on everything else' (#93) from plane/HOMELAB-2634-janitor-relief into main

aaron referenced this pull request 2026-11-04 19:14:47 +00:00

HOMELAB-2634: feat(herdr): clean up background sessions, count sessions, notice only failures #94

Sign in to join this conversation.

Reviewers

No reviewers

ld-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

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/pi-extensions!93

No description provided.