HOMELAB-1679: feat(plane,forgejo): verification closure — plane ticket/epic boundaries + forgejo landing lock #51

Merged

aaron merged 5 commits from plane/HOMELAB-1679-closure into main 2026-09-30 21:55:43 +00:00

Conversation 2 Commits 5 Files changed 17 +14816 -21

aaron commented 2026-09-30 21:29:13 +00:00

Owner

Copy link

Converts three prose rules into fail-closed tool-layer boundaries (Lab Director Contract §5: boundaries, not prompts). Design: .rpiv/artifacts/research/homelab-1679/closure-boundaries-design.md. No override parameter exists on any boundary; humans keep the Forgejo UI merge path.

Boundary 1 — ticket closure (plane_update_item)

A transition to a state in the completed group (Done and any future completed-group state) is refused unless the ticket's newest comment carries the verification marker: a ## Verification heading line (/^#{1,3}\s*verification\b/i after HTML→text per-line), an <h1-3>Verification</h1-3> element, or data-ld-verification. Newest-comment rule: re-opened work appends newer comments, so a stale verification cannot satisfy a later Done. The refusal names the ticket and prints the exact plane_comment call to fix it. The check runs before the PATCH, so a refused transition changes nothing.

Boundary 1 requires the states cache to be warm, and it is now warmed lazily (ensureStates) instead of throwing "Project cache not loaded" — a cold cache can no longer turn a completion transition into an unchecked one.

Boundary 2 — epic-Done census (plane_update_item)

A ticket with parent_id children cannot go Done unless every child is (a) in a completed-group state and (b) carries the Boundary-1 marker as its newest comment. Refusals cite each blocking child: HOMELAB-1800 [In Progress] not in a completed-group state / HOMELAB-1801 [Done] completed but unverified — its newest comment has no verification marker, plus a count-N-of-M header. Success returns the census it verified (epic census passed: 2 children, 2 closed and verified (…) + details.ldClosure.census) so the Done is attributable.

Epic-shaped tickets ((epic) in title/description) with no parented children are refused loudly ("no machine-readable children; re-parent …") — no auto-pass. listChildren filters client-side because Plane ignores parent_id as a list filter (verified read-only against the homelab workspace: a bogus parent_id returned all 1441 items); the child field is parent.

Boundary 3 — landing lock (forgejo_merge_pr)

1. CI leg — the PR head sha's combined commit status must be success, read through the HOMELAB-1736 endpoint-fallback pattern (/commits/{sha}/status primary, /statuses list fallback with the degradation surfaced in the result). pending → wait-for-CI refusal; failure/error/warning → refusal naming the non-green contexts; no status at all → refusal (fail closed; the message points repos without CI at the UI merge path).
2. Verdict leg — a review verdict (## Review / ## Landing review … heading, or a verdict: line) must post-date the head commit (commit date from /commits/{sha}, fallback the PR commit list); an earlier round's verdict is stale.
3. Blocker leg — an open blocker finding refuses the merge and is cited with its id.

The design left the blocker grammar open, so the lock defines one and the refusal message teaches it: blocker <id> findings, resolved <id> / fixed-in <id> resolutions, negated statements ("no blockers", "blockers: none") ignored. Every leg fails closed when its evidence cannot be read.

HOMELAB-1799

createRelation now writes issues (201) instead of the read-shape related_issues (400s). Covered by a test that would fail against the old payload.

Tests / CI

New scaffolding for plane/ and forgejo/ (package.json, tsconfig.json, lockfiles — mirroring the other gated packages), plus two CI jobs. All API traffic is a route-table fake; no network, no Plane/Forgejo writes.

• plane/tests/closure.test.ts — 19 tests: marker acceptance/rejection, stale-verification refusal, newest-comment ordering, census refusal citing children, cancelled child, epic-shaped no-children, non-completion transitions untouched, createRelation payload.
• forgejo/tests/landing.test.ts — 23 tests: red/pending/absent CI, fallback + double-failure, missing/stale verdict, review-body verdict, open blocker + resolved blocker, wrong-id resolution, negated blockers, pure state derivation.
• npx tsc --noEmit clean for both packages; two dormant type errors in plane/index.ts and three in forgejo were fixed so the packages can be gated at all.

Deviations / interpretations (design is silent, recorded here)

• Marker matching is HTML-tolerant: the design's regex is matched per line of the HTML→text conversion and an <h1-3>Verification</h1-3> element counts, because Plane stores comments as HTML.
• Verdict markers are widened to ## Landing review … (the shape the landing pipeline actually posts, verified against PR #48) alongside the design's ## Review / verdict: forms.
• Both boundaries apply to an epic: the epic's own newest comment must be a verification and the census must pass. The design defines Boundary 1 unconditionally; this is the composition, not a loosening.
• cancelled children block the census (the design says completed-group; "every child dispositioned" would have been the looser reading). The refusal's remedy line names re-parenting, so this cannot dead-end silently.
• Verdict staleness uses verdict.at >= headCommitAt (second-resolution Forgejo timestamps); a verdict from an earlier round is strictly older than a later commit, so >= cannot resurrect one.

Honest limits

• Children linked only by naming convention (legacy LD1 epics) are invisible to the census; that case fails loud rather than auto-passing, but it is a weaker check, not a complete one.
• TOCTOU: the lock reads the head sha it saw; Forgejo's merge endpoint takes no head-sha guard, so a push landing between check and merge is not detected. Follow-up candidate.
• Blocker parsing is a documented heuristic over free-form review prose; a verdict that encodes blockers only as 🔴 (or a blockers_count field) is not caught. Canonical blocker <id> + resolved <id> markers are always caught.
• Dangerous-class detection (RBAC/secrets/network-policy/Terraform) is deliberately not in this boundary — file-path classification stays in the landing pipeline's dispatch discipline until a classifier exists.
• The design's "attempted, blocked, logged" evidence: refusals surface as tool errors in the session transcript; no separate audit log was added. Live scratch-ticket dry-run was skipped in favour of the fixture matrix (the ticket's verify list marks the live run optional).
• The plane census fetches the project's item list (~15 requests at 1441 items) per Done transition; acceptable for a rare, load-bearing check.

Converts three prose rules into fail-closed tool-layer boundaries (Lab Director Contract §5: boundaries, not prompts). Design: `.rpiv/artifacts/research/homelab-1679/closure-boundaries-design.md`. No override parameter exists on any boundary; humans keep the Forgejo UI merge path. ## Boundary 1 — ticket closure (`plane_update_item`) A transition to a state in the `completed` group (Done and any future completed-group state) is refused unless the ticket's **newest comment** carries the verification marker: a `## Verification` heading line (`/^#{1,3}\s*verification\b/i` after HTML→text per-line), an `<h1-3>Verification</h1-3>` element, or `data-ld-verification`. Newest-comment rule: re-opened work appends newer comments, so a stale verification cannot satisfy a later Done. The refusal names the ticket and prints the exact `plane_comment` call to fix it. The check runs **before** the PATCH, so a refused transition changes nothing. Boundary 1 requires the states cache to be warm, and it is now warmed lazily (`ensureStates`) instead of throwing "Project cache not loaded" — a cold cache can no longer turn a completion transition into an unchecked one. ## Boundary 2 — epic-Done census (`plane_update_item`) A ticket with `parent_id` children cannot go Done unless every child is (a) in a completed-group state and (b) carries the Boundary-1 marker as its newest comment. Refusals cite each blocking child: ` HOMELAB-1800 [In Progress] not in a completed-group state` / ` HOMELAB-1801 [Done] completed but unverified — its newest comment has no verification marker`, plus a count-N-of-M header. Success returns the census it verified (`epic census passed: 2 children, 2 closed and verified (…)` + `details.ldClosure.census`) so the Done is attributable. Epic-shaped tickets (`(epic)` in title/description) with **no** parented children are refused loudly ("no machine-readable children; re-parent …") — no auto-pass. `listChildren` filters client-side because Plane ignores `parent_id` as a list filter (verified read-only against the homelab workspace: a bogus `parent_id` returned all 1441 items); the child field is `parent`. ## Boundary 3 — landing lock (`forgejo_merge_pr`) 1. **CI leg** — the PR head sha's combined commit status must be `success`, read through the HOMELAB-1736 endpoint-fallback pattern (`/commits/{sha}/status` primary, `/statuses` list fallback with the degradation surfaced in the result). `pending` → wait-for-CI refusal; `failure`/`error`/`warning` → refusal naming the non-green contexts; no status at all → refusal (fail closed; the message points repos without CI at the UI merge path). 2. **Verdict leg** — a review verdict (`## Review` / `## Landing review …` heading, or a `verdict:` line) must post-date the head commit (commit date from `/commits/{sha}`, fallback the PR commit list); an earlier round's verdict is stale. 3. **Blocker leg** — an open `blocker` finding refuses the merge and is cited with its id. The design left the blocker grammar open, so the lock defines one and the refusal message teaches it: `blocker <id>` findings, `resolved <id>` / `fixed-in <id>` resolutions, negated statements ("no blockers", "blockers: none") ignored. Every leg fails closed when its evidence cannot be read. ## HOMELAB-1799 `createRelation` now writes `issues` (201) instead of the read-shape `related_issues` (400s). Covered by a test that would fail against the old payload. ## Tests / CI New scaffolding for `plane/` and `forgejo/` (`package.json`, `tsconfig.json`, lockfiles — mirroring the other gated packages), plus two CI jobs. All API traffic is a route-table fake; no network, no Plane/Forgejo writes. - `plane/tests/closure.test.ts` — 19 tests: marker acceptance/rejection, stale-verification refusal, newest-comment ordering, census refusal citing children, cancelled child, epic-shaped no-children, non-completion transitions untouched, createRelation payload. - `forgejo/tests/landing.test.ts` — 23 tests: red/pending/absent CI, fallback + double-failure, missing/stale verdict, review-body verdict, open blocker + resolved blocker, wrong-id resolution, negated blockers, pure state derivation. - `npx tsc --noEmit` clean for both packages; two dormant type errors in `plane/index.ts` and three in `forgejo` were fixed so the packages can be gated at all. ## Deviations / interpretations (design is silent, recorded here) - Marker matching is HTML-tolerant: the design's regex is matched per line of the HTML→text conversion and an `<h1-3>Verification</h1-3>` element counts, because Plane stores comments as HTML. - Verdict markers are widened to `## Landing review …` (the shape the landing pipeline actually posts, verified against PR #48) alongside the design's `## Review` / `verdict:` forms. - Both boundaries apply to an epic: the epic's own newest comment must be a verification **and** the census must pass. The design defines Boundary 1 unconditionally; this is the composition, not a loosening. - `cancelled` children block the census (the design says completed-group; "every child dispositioned" would have been the looser reading). The refusal's remedy line names re-parenting, so this cannot dead-end silently. - Verdict staleness uses `verdict.at >= headCommitAt` (second-resolution Forgejo timestamps); a verdict from an earlier round is strictly older than a later commit, so `>=` cannot resurrect one. ## Honest limits - Children linked only by naming convention (legacy LD1 epics) are invisible to the census; that case fails loud rather than auto-passing, but it is a weaker check, not a complete one. - TOCTOU: the lock reads the head sha it saw; Forgejo's merge endpoint takes no head-sha guard, so a push landing between check and merge is not detected. Follow-up candidate. - Blocker parsing is a documented heuristic over free-form review prose; a verdict that encodes blockers only as 🔴 (or a `blockers_count` field) is not caught. Canonical `blocker <id>` + `resolved <id>` markers are always caught. - Dangerous-class detection (RBAC/secrets/network-policy/Terraform) is deliberately **not** in this boundary — file-path classification stays in the landing pipeline's dispatch discipline until a classifier exists. - The design's "attempted, blocked, logged" evidence: refusals surface as tool errors in the session transcript; no separate audit log was added. Live scratch-ticket dry-run was skipped in favour of the fixture matrix (the ticket's verify list marks the live run optional). - The plane census fetches the project's item list (~15 requests at 1441 items) per Done transition; acceptable for a rare, load-bearing check.

aaron added 3 commits 2026-09-30 21:29:14 +00:00

HOMELAB-1679: feat(plane): verification closure boundaries (LD2.5) ... 2b71482baa

Turns the "ALWAYS post a verification comment before Done" prose rule into a
fail-closed tool-layer boundary, and fixes the relation payload (HOMELAB-1799).

Boundary 1 — plane_update_item refuses a transition to a completed-group
state unless the ticket's NEWEST comment carries the verification marker
(`## Verification` heading / <h1-3>Verification</h1-3> / data-ld-verification).
Newest-comment rule: a stale verification from an earlier round cannot satisfy
a re-opened ticket. No override parameter exists.

Boundary 2 — epic census: a ticket with parent_id children is refused unless
every child is in a completed-group state AND carries the Boundary-1 marker as
its newest comment. Refusals cite each blocking child as
"IDENTIFIER  [State]  open|completed but unverified"; the success response
names the census it verified. An epic-shaped ticket (`(epic)`) with no
machine-readable children fails loud instead of auto-passing.

Also:
- plane/api.ts: createRelation writes `issues` (201); the read-shape
  `related_issues` 400s on this endpoint (HOMELAB-1799).
- promptGuidelines now teach the `## Verification` marker convention.
- plane/ gains typecheck + vitest scaffolding (package.json, tsconfig,
  lockfile) with mocked-fetch tests; a few dormant type errors in
  plane/index.ts are fixed so the package can be gated in CI.

HOMELAB-1679: feat(forgejo): landing lock on forgejo_merge_pr (LD2.5) ... fa7ce642e5

Boundary 3 turns the AGENTS.md landing prose ("merge on green CI + clean
review verdict") into a fail-closed precondition on the agent tool.

Before the merge executes:
1. CI leg — the PR head sha's commit status must be `success`. The combined
   /status endpoint is primary; if it is unavailable the /statuses list
   answers instead and the degradation is surfaced in the result (HOMELAB-1736
   endpoint-fallback pattern). `pending` -> wait-for-CI refusal; `failure`/
   `error`/`warning` -> refusal naming the non-green contexts; no status at all
   -> refusal (fail closed, with the UI-merge path for repos without CI).
2. Review-verdict leg — a verdict (`## Review`/`## Landing review …` heading or
   a `verdict:` line) must post-date the head commit; a verdict from an earlier
   round does not cover the current diff.
3. Blocker leg — an open `blocker` finding refuses the merge and is cited, with
   its id, in the refusal.

The blocker grammar the design left open is defined in forgejo/api.ts and
taught by the refusal message: `blocker <id>` findings, `resolved <id>` /
`fixed-in <id>` resolutions, negated statements ("no blockers") ignored.
Humans keep the Forgejo UI merge path — the lock governs autonomy, never the
operator. There is no override parameter.

forgejo/ gains typecheck + vitest scaffolding with mocked-fetch tests for every
refusal leg; two dormant type errors are fixed so the package can be gated.
CI: plane + forgejo jobs added (additive one-line job copies).

HOMELAB-1679: fix(plane): accurate census wording for non-completed children ... ef57a26eb7

A child that is not in a completed-group state was cited as "open", which
reads wrong for a cancelled child. The line is now group-accurate and the hint
names re-parenting as the remedy for a child that must not block the epic.

aaron changed title from HOMELAB-1679: verification closure — plane ticket/epic boundaries + forgejo landing lock to HOMELAB-1679: feat(plane,forgejo): verification closure — plane ticket/epic boundaries + forgejo landing lock 2026-09-30 21:29:45 +00:00

aaron added 1 commit 2026-09-30 21:34:27 +00:00

HOMELAB-1679: merge main (HOMELAB-1769 mcp-bridge) into the closure branch ... dd33919969

Resolves the ci.yaml comment-block conflict by keeping both notes (mcp-bridge
typecheck-only job + the plane/forgejo gated jobs) and drops the now-stale
"forgejo, plane -> no package.json at all" line from the coverage note.

aaron added 1 commit 2026-09-30 21:38:08 +00:00

HOMELAB-1679: fix(forgejo): page PR comments before judging the verdict ... ea299f1b85

Forgejo serves issues/{n}/comments oldest-first with a 50-item clamp, so a
single page would miss a verdict on a long-running PR and refuse a merge that
should pass. listAllPRComments pages (6-page cap) and a test pins it with 50
filler comments ahead of the verdict.

aaron commented 2026-09-30 21:43:06 +00:00

Author

Owner

Copy link

Builder handoff — CI green, PR ready for landing review

Head sha: ea299f1b85 · CI run #96: 10/10 jobs green (incl. the two new legs plane, forgejo).

Read-only live check of the lock's own data source (the CI leg of Boundary 3):

GET /repos/Homelab/pi-extensions/commits/ea299f1b85/status
combined state: "success"  |  10 contexts
  CI / pr-title, skill-args, ask-user-question, forgejo-ci, pi-otel-telemetry,
  todo, plane, mcp-bridge, forgejo, pi-swarm-suite  -> all success

So forgejo_merge_pr on this PR will see success for the head sha — the lock is satisfiable on this repo, not just on fixtures.

What the landing reviewer should check

• plane/api.ts + plane/index.ts: boundaries 1 and 2 run before the PATCH (a refusal cannot half-write a transition); no override parameter; listChildren filters client-side because Plane ignores parent_id as a list filter (verified read-only against this workspace — a bogus parent_id returned all 1441 items); the child field is parent.
• forgejo/api.ts + forgejo/index.ts: three legs, each fail-closed when its evidence is unreadable; the /statuses fallback is HOMELAB-1736-shaped and the degradation is surfaced in the result rather than hidden.
• Review focus for the marker/grammar matching: the verdict and finding matchers are regex-heuristics over free-form review prose (documented in-file with the canonical forms). If a real-world verdict shape is missed, the failure mode is a refusal (fixable by posting the canonical marker), not a false pass.

Notes for the landing worker

• This PR supplies the lock that will gate its own merge: the head sha needs the CI leg green (it is), a review verdict comment posted after ea299f1's commit date, and no unresolved finding on a blocker <id> line.
• Interpreting "verdict after the head commit" is where a squashed/final review usually lands; the verdict comment should carry a ## Review/## Landing review … heading or a verdict: line, which is what the pipeline already posts (checked against PR #48's real verdict comments).
• Design deviations and honest limits are enumerated in the PR description — notably cancelled children blocking the census (design says completed-group), and the TOCTOU window between the head-sha check and the merge call (Forgejo's merge endpoint takes no sha guard) which is left as a follow-up.

## Builder handoff — CI green, PR ready for landing review **Head sha:** `ea299f1b85` · **CI run #96: 10/10 jobs green** (incl. the two new legs `plane`, `forgejo`). Read-only live check of the lock's own data source (the CI leg of Boundary 3): ``` GET /repos/Homelab/pi-extensions/commits/ea299f1b85/status combined state: "success" | 10 contexts CI / pr-title, skill-args, ask-user-question, forgejo-ci, pi-otel-telemetry, todo, plane, mcp-bridge, forgejo, pi-swarm-suite -> all success ``` So `forgejo_merge_pr` on this PR will see `success` for the head sha — the lock is satisfiable on this repo, not just on fixtures. ### What the landing reviewer should check - `plane/api.ts` + `plane/index.ts`: boundaries 1 and 2 run **before** the PATCH (a refusal cannot half-write a transition); no override parameter; `listChildren` filters client-side because Plane ignores `parent_id` as a list filter (verified read-only against this workspace — a bogus `parent_id` returned all 1441 items); the child field is `parent`. - `forgejo/api.ts` + `forgejo/index.ts`: three legs, each fail-closed when its evidence is unreadable; the `/statuses` fallback is HOMELAB-1736-shaped and the degradation is surfaced in the result rather than hidden. - Review focus for the marker/grammar matching: the verdict and finding matchers are regex-heuristics over free-form review prose (documented in-file with the canonical forms). If a real-world verdict shape is missed, the failure mode is a refusal (fixable by posting the canonical marker), not a false pass. ### Notes for the landing worker - This PR supplies the lock that will gate its own merge: the head sha needs the CI leg green (it is), a review verdict comment posted **after** `ea299f1`'s commit date, and no unresolved finding on a `blocker <id>` line. - Interpreting "verdict after the head commit" is where a squashed/final review usually lands; the verdict comment should carry a `## Review`/`## Landing review …` heading or a `verdict:` line, which is what the pipeline already posts (checked against PR #48's real verdict comments). - Design deviations and honest limits are enumerated in the PR description — notably `cancelled` children blocking the census (design says completed-group), and the TOCTOU window between the head-sha check and the merge call (Forgejo's merge endpoint takes no sha guard) which is left as a follow-up.

aaron commented 2026-09-30 21:55:22 +00:00

Author

Owner

Copy link

Landing review (fresh-context)

Fresh process, primary sources only: git diff origin/main...ea299f1f (head ea299f1b85dad60f32897c7d24fb885acd0f8da9), the frozen design .rpiv/artifacts/research/homelab-1679/closure-boundaries-design.md, HOMELAB-1679 / HOMELAB-1799, plus the live Plane + Forgejo APIs as a read-only oracle. Implementation summary not used.

Verdict: merge. Findings tagged blocker / concern / suggestion — 0 / 4 / 5.

Traced as fail-closed (the three boundaries)

• B1 refusal precedes the write. plane/index.ts:717-740 runs enforceClosureBoundaries before updateWorkItem; refusal is a thrown error, no PATCH. plane/tests/closure.test.ts:169,182 assert zero PATCH calls on refusal. No override parameter exists on the tool or the predicate (plane/api.ts:494-534).
• B1 marker is sound and cannot be satisfied by prose. VERIFICATION_HEADING_RE is ^#{1,3}\s*verification\b applied per line of htmlToLines() (plane/api.ts:419-463), so "packing verification" / "pending verification of the fix" do not match (tests at closure.test.ts:103). Stale-verification refusal is by newest comment (closure.test.ts:109).
• Unpaginated comment read is not a fail-open. Plane's comments endpoint returns newest-first (live check, HOMELAB-1679: 21:45 comment before 21:10), so the single-page read in enforceClosureBoundaries cannot miss the newest comment; the code also sorts defensively.
• B2 census cannot pass with an open child. evaluateEpicCensus blocks on !completed || !verified (plane/api.ts:544-576); empty census refuses; a child state absent from the cache yields null !== "completed" → blocked; every child's project is warmed via ensureStates, which throws rather than judging blind (plane/index.ts:151-160,224-228).
• listChildren is correct against live Plane. I re-verified the claim independently: the project list response does carry parent (30 parented items on page 1 of 1000), and listAllWorkItems cursor-pagination returns all 1445 items over 15 pages. All 9 (epic)-hinted tickets in HOMELAB have parented children (4-11 each), so the fail-loud childless-epic path has zero false positives today.
• B3 ordering and CI leg. assertLandingLock runs before mergePR (forgejo/index.ts:475-488); refusal = no POST to /merge (tests landing.test.ts:246,258,272). Primary status endpoint is live-correct: /commits/ea299f1b85…/status → {"state":"success","total_count":10}; head sha resolved live (pulls/51 → head.sha).
• No test writes to live state, no network. Both suites stub global fetch with route-table fakes; npm ci-able scaffolding mirrors todo / skill-args. Local re-run on the PR worktree: plane 19/19 pass, forgejo 24/24 pass, tsc --noEmit clean for both.

Concerns (may merge)

1. forgejo/api.ts:467 — getCommit calls GET /repos/{o}/{r}/commits/{sha}, which does not exist on Forgejo (live: 404 page not found for both the short and full head sha). The real route is /repos/{o}/{r}/git/commits/{sha} (live 200). So the "primary" branch of the staleness leg is dead code and every merge eats a wasted 404 before falling through to listPRCommits (live 200, works). The docstring's "Primary: /commits/{sha}" is false, and forgejo/tests/fake.ts:207 matches /commits/{anything}$, so no test can ever catch this.
2. forgejo/api.ts:518,525 — the /statuses fallback derives state from all status entries, not the newest per context. Live for the head sha: /statuses returns 30 entries across 10 duplicated contexts, 20 of them pending (superseded jobs from runs #95/#93) → the fallback derives pending and would refuse a merge the combined endpoint correctly calls success. Fail-closed direction, so not a blocker, but the documented "degraded read" is effectively unusable for this repo; group by context and keep the newest entry per context.
3. plane/index.ts:635 — plane_create_item still writes any state, including a completed-group one, with no B1 check. The design scopes B1 to transitions, so not a deviation, but it is the only remaining path into a completed-group state that skips the check.
4. docs/plane.md:18 ("No npm dependencies") and docs/forgejo.md are now stale relative to this diff: the extensions have package.json + package-lock.json and CI gates, and neither the tools tables nor the Gotchas sections mention the closure boundaries or the landing lock — the most behavior-changing edits in the PR.

Suggestions

5. forgejo/api.ts:565 — when the head sha is not found in the PR commit list, the fallback picks commits[commits.length - 1], which live is the oldest commit (the list is newest-first). commits[0] is the safe default; the current one would weaken the staleness leg if it ever triggered.
6. forgejo/index.ts:98 — the diff joined the fmtRun doc comment onto the function line and dropped the blank line; unintended cosmetic churn.
7. Marker false-positive surface: VERIFICATION_TEMPLATE and the refusal text embed literal <h2>Verification</h2> / data-ld-verification (plane/api.ts:428-436), so a comment quoting the refusal would itself satisfy B1, and a comment that merely documents the verdict grammar satisfies B3. Inherent to a marker-based boundary the design ratified — noted, not blocking.
8. The verdict leg checks freshness, not authorship: an implementer's own ## Review / verdict: comment satisfies it. Consistent with the design (independence lives in dispatch discipline); worth stating in the docstring so the lock is not read as stronger than it is.
9. PR body says forgejo/tests/landing.test.ts has 23 tests; it has 24 (19 + 24 = 43 total). Trivial, and the body's own total is right.

Operational note for the fleet (not a defect)

Sampling live: 0 of the 12 most recently completed HOMELAB tickets have a newest comment matching the marker (real shapes: CLOSING VERIFICATION — …, VERIFIED — PROVEN …, Closing as delivered …). So from the moment this lands, every Done transition — and every LD0-LD8 epic census (LD0 has 4 already-Done children that still need re-derived markers) — will be refused until a conforming ## Verification comment is posted. That is exactly the ratified design's intent, and the refusal self-teaches the remedy; announcing it to the fleet will avoid a wave of "the tool is broken" reports.

Merging on green CI + no blockers.

## Landing review (fresh-context) Fresh process, primary sources only: `git diff origin/main...ea299f1f` (head `ea299f1b85dad60f32897c7d24fb885acd0f8da9`), the frozen design `.rpiv/artifacts/research/homelab-1679/closure-boundaries-design.md`, HOMELAB-1679 / HOMELAB-1799, plus the live Plane + Forgejo APIs as a read-only oracle. Implementation summary not used. **Verdict: merge.** Findings tagged `blocker` / `concern` / `suggestion` — **0 / 4 / 5**. ### Traced as fail-closed (the three boundaries) - **B1 refusal precedes the write.** `plane/index.ts:717-740` runs `enforceClosureBoundaries` before `updateWorkItem`; refusal is a thrown error, no PATCH. `plane/tests/closure.test.ts:169,182` assert zero PATCH calls on refusal. No override parameter exists on the tool or the predicate (`plane/api.ts:494-534`). - **B1 marker is sound and cannot be satisfied by prose.** `VERIFICATION_HEADING_RE` is `^#{1,3}\s*verification\b` applied per line of `htmlToLines()` (`plane/api.ts:419-463`), so `"packing verification"` / `"pending verification of the fix"` do not match (tests at `closure.test.ts:103`). Stale-verification refusal is by *newest comment* (`closure.test.ts:109`). - **Unpaginated comment read is not a fail-open.** Plane's comments endpoint returns **newest-first** (live check, HOMELAB-1679: 21:45 comment before 21:10), so the single-page read in `enforceClosureBoundaries` cannot miss the newest comment; the code also sorts defensively. - **B2 census cannot pass with an open child.** `evaluateEpicCensus` blocks on `!completed || !verified` (`plane/api.ts:544-576`); empty census refuses; a child state absent from the cache yields `null !== "completed"` → blocked; every child's project is warmed via `ensureStates`, which throws rather than judging blind (`plane/index.ts:151-160,224-228`). - **`listChildren` is correct against live Plane.** I re-verified the claim independently: the project list response *does* carry `parent` (30 parented items on page 1 of 1000), and `listAllWorkItems` cursor-pagination returns all **1445 items over 15 pages**. All **9** `(epic)`-hinted tickets in HOMELAB have parented children (4-11 each), so the fail-loud childless-epic path has zero false positives today. - **B3 ordering and CI leg.** `assertLandingLock` runs before `mergePR` (`forgejo/index.ts:475-488`); refusal = no POST to `/merge` (tests `landing.test.ts:246,258,272`). Primary status endpoint is live-correct: `/commits/ea299f1b85…/status` → `{"state":"success","total_count":10}`; head sha resolved live (`pulls/51` → `head.sha`). - **No test writes to live state, no network.** Both suites stub global `fetch` with route-table fakes; `npm ci`-able scaffolding mirrors `todo` / `skill-args`. Local re-run on the PR worktree: `plane` 19/19 pass, `forgejo` 24/24 pass, `tsc --noEmit` clean for both. ### Concerns (may merge) 1. `forgejo/api.ts:467` — `getCommit` calls `GET /repos/{o}/{r}/commits/{sha}`, **which does not exist on Forgejo** (live: 404 `page not found` for both the short and full head sha). The real route is `/repos/{o}/{r}/git/commits/{sha}` (live 200). So the "primary" branch of the staleness leg is dead code and every merge eats a wasted 404 before falling through to `listPRCommits` (live 200, works). The docstring's "Primary: `/commits/{sha}`" is false, and `forgejo/tests/fake.ts:207` matches `/commits/{anything}$`, so no test can ever catch this. 2. `forgejo/api.ts:518,525` — the `/statuses` fallback derives state from **all** status entries, not the newest per context. Live for the head sha: `/statuses` returns **30 entries across 10 duplicated contexts, 20 of them `pending`** (superseded jobs from runs #95/#93) → the fallback derives `pending` and would refuse a merge the combined endpoint correctly calls `success`. Fail-closed direction, so not a blocker, but the documented "degraded read" is effectively unusable for this repo; group by `context` and keep the newest entry per context. 3. `plane/index.ts:635` — `plane_create_item` still writes any `state`, including a completed-group one, with no B1 check. The design scopes B1 to transitions, so not a deviation, but it is the only remaining path into a completed-group state that skips the check. 4. `docs/plane.md:18` ("No npm dependencies") and `docs/forgejo.md` are now stale relative to this diff: the extensions have `package.json` + `package-lock.json` and CI gates, and neither the tools tables nor the Gotchas sections mention the closure boundaries or the landing lock — the most behavior-changing edits in the PR. ### Suggestions 5. `forgejo/api.ts:565` — when the head sha is not found in the PR commit list, the fallback picks `commits[commits.length - 1]`, which live is the **oldest** commit (the list is newest-first). `commits[0]` is the safe default; the current one would weaken the staleness leg if it ever triggered. 6. `forgejo/index.ts:98` — the diff joined the `fmtRun` doc comment onto the function line and dropped the blank line; unintended cosmetic churn. 7. Marker false-positive surface: `VERIFICATION_TEMPLATE` and the refusal text embed literal `<h2>Verification</h2>` / `data-ld-verification` (`plane/api.ts:428-436`), so a comment *quoting the refusal* would itself satisfy B1, and a comment that merely documents the verdict grammar satisfies B3. Inherent to a marker-based boundary the design ratified — noted, not blocking. 8. The verdict leg checks freshness, not authorship: an implementer's own `## Review` / `verdict:` comment satisfies it. Consistent with the design (independence lives in dispatch discipline); worth stating in the docstring so the lock is not read as stronger than it is. 9. PR body says `forgejo/tests/landing.test.ts` has 23 tests; it has 24 (19 + 24 = 43 total). Trivial, and the body's own total is right. ### Operational note for the fleet (not a defect) Sampling live: **0 of the 12 most recently completed HOMELAB tickets** have a newest comment matching the marker (real shapes: `CLOSING VERIFICATION — …`, `VERIFIED — PROVEN …`, `Closing as delivered …`). So from the moment this lands, every Done transition — and every LD0-LD8 epic census (LD0 has 4 already-Done children that still need re-derived markers) — will be refused until a conforming `## Verification` comment is posted. That is exactly the ratified design's intent, and the refusal self-teaches the remedy; announcing it to the fleet will avoid a wave of "the tool is broken" reports. Merging on green CI + no blockers.

aaron merged commit 45f18d863e into main 2026-09-30 21:55:43 +00:00

aaron referenced this pull request from a commit 2026-09-30 21:55:44 +00:00

Merge pull request 'HOMELAB-1679: feat(plane,forgejo): verification closure — plane ticket/epic boundaries + forgejo landing lock' (#51) from plane/HOMELAB-1679-closure into main

Sign in to join this conversation.

Reviewers

No reviewers

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

1 participant

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!51

No description provided.