Homelab/pi-extensions
6
0
Fork 0
Code Issues Pull requests 4 Projects Releases Packages Wiki Activity Actions

HOMELAB-1679: fix(forgejo): the reviewer identity set is a fixed per-repo map in code; the env lever is removed (D6.1(d)) #134

Open
pi-landing wants to merge 5 commits from reviewers-by-repo-f1 into main
pull from: reviewers-by-repo-f1
merge into: Homelab:main
Conversation 0 Commits 5 Files changed 22 +867 -138
pi-landing commented 2026-11-12 14:01:34 +00:00
Collaborator
Copy link

What this changes

The set of logins whose verdict is approving evidence becomes a fixed map in code, per repo — DEFAULT_LANDING_POLICY.reviewerIdentitiesByRepo in forgejo/landing.ts — and the environment override FORGEJO_LANDING_REVIEWERS is removed completely (it is not read anywhere: not on GO-gated repos, not on any other).

Why: whose prose may approve a diff is a property of the repo, and a value a session can set is not a gate (the class review B3 / the N15 pin already closed on GO-gated repos). Until now any lane could widen its own reviewer set by exporting one login.

The map preserves today's behaviour (30-day census, read-only)

Window 2026-10-13T13:57Z → 2026-11-12, 214 merged PRs, each read individually for merged_by (the list endpoint does not carry it):

repo merged PRs in the window (by merger) reviewer set today reviewer set after
Homelab/ghostty-in-the-shell 41 pi-landing ld-reviewer gits-seat-comment
Homelab/infra-core 28 pi-landing ld-reviewer ld-reviewer
Homelab/lab-director 23 pi-landing + 12 ld-landing ld-reviewer ld-reviewer
aaron/lab-director 18 aaron + 1 pi-landing ld-reviewer ld-reviewer
Homelab/pi-extensions 42 pi-landing + 2 aaron ld-reviewer ld-reviewer
Homelab/prod 31 pi-landing + 9 aaron + 4 jesus + 1 ld-landing ld-reviewer ld-reviewer
Homelab/throwaway-ld-test-20261108 2 pi-landing ld-reviewer unmapped → refused
  • pi-landing / ld-landing are the merger identities, never reviewers. No repo's effective reviewer set was anything other than ld-reviewer: no sbx-* pod sets FORGEJO_LANDING_REVIEWERS (pod specs read by the supervisor, 14:3xZ), and the only other merger identity is ld-landing.
  • The only behaviour change is Homelab/ghostty-in-the-shell → gits-seat-comment (D6.1(d)): the account it now seats with, replacing the revoked one.
  • Homelab/throwaway-ld-test-20261108 is deliberately NOT mapped: it is a throwaway test repo, and it stays rejected. That is the fail-closed answer, not an oversight.
  • Sendrast: a9 confirms that no Sendrast repo lands through the landing tool — sendrast-bot merges by API, and the release PRs land in Homelab/prod (→ ld-reviewer). No Sendrast row is needed in the map.

Rule

  1. A repo's set is its entry in the map (owner/repo in repoKey form, so any spelling the forge or a caller produces resolves).
  2. The environment is not read: naming another reviewer, or adding pi-landing, is ignored on every repo, gated or not.
  3. A repo with no entry is refused explicitly (no trusted reviewer identity is configured for <repo>), and a malformed entry (empty list, blank login) is refused the same way — never "anyone".

Tests

forgejo/tests/landing-reviewers-by-repo.test.ts (new):

  • GITS counts gits-seat-comment; an exact-head ld-reviewer verdict — which used to approve — is no longer evidence (the row that fails with the old code);
  • every other mapped repo keeps ld-reviewer;
  • env naming another reviewer / adding pi-landing: ignored on GITS, on a mapped non-gated repo and on a GO-gated repo; an ld-reviewer verdict on GITS stays untrusted even when the env names it;
  • unmapped repo → explicit refusal, approvals empty; throwaway repo → refused;
  • malformed entry → refused; map key spellings (https://…/X.git, upper case, trailing slash) resolve.

Updated for the removed lever: landing-gated-reviewers-n6, landing-gated-env-pins-n15, landing-census-gated-policy-017, landing-record, landing-server-repo-name-b1, landing-corpus, docs/forgejo.md.

Local run: the runnable subset passes (584 tests). 19 test files in this checkout do not load because @mariozechner/pi-ai is not installed in this pod — pre-existing, unrelated to the change (they fail the same way on main); CI installs it and runs the whole suite.

Do not merge

Gate PR: needs a clean pair (SEAT-LINE + SEATSID), homelab-f1's review, and El Director's GO. No merge from this branch.

## What this changes The set of logins whose verdict is **approving evidence** becomes a **fixed map in code, per repo** — `DEFAULT_LANDING_POLICY.reviewerIdentitiesByRepo` in `forgejo/landing.ts` — and the environment override `FORGEJO_LANDING_REVIEWERS` is **removed completely** (it is not read anywhere: not on GO-gated repos, not on any other). Why: whose prose may approve a diff is a property of the repo, and a value a session can set is not a gate (the class review B3 / the N15 pin already closed on GO-gated repos). Until now any lane could widen its own reviewer set by exporting one login. ## The map preserves today's behaviour (30-day census, read-only) Window 2026-10-13T13:57Z → 2026-11-12, **214 merged PRs**, each read individually for `merged_by` (the list endpoint does not carry it): | repo | merged PRs in the window (by merger) | reviewer set today | reviewer set after | |---|---|---|---| | Homelab/ghostty-in-the-shell | 41 pi-landing | `ld-reviewer` | **`gits-seat-comment`** | | Homelab/infra-core | 28 pi-landing | `ld-reviewer` | `ld-reviewer` | | Homelab/lab-director | 23 pi-landing + 12 ld-landing | `ld-reviewer` | `ld-reviewer` | | aaron/lab-director | 18 aaron + 1 pi-landing | `ld-reviewer` | `ld-reviewer` | | Homelab/pi-extensions | 42 pi-landing + 2 aaron | `ld-reviewer` | `ld-reviewer` | | Homelab/prod | 31 pi-landing + 9 aaron + 4 jesus + 1 ld-landing | `ld-reviewer` | `ld-reviewer` | | Homelab/throwaway-ld-test-20261108 | 2 pi-landing | `ld-reviewer` | **unmapped → refused** | - `pi-landing` / `ld-landing` are the **merger** identities, never reviewers. No repo's effective reviewer set was anything other than `ld-reviewer`: no sbx-* pod sets `FORGEJO_LANDING_REVIEWERS` (pod specs read by the supervisor, 14:3xZ), and the only other merger identity is `ld-landing`. - **The only behaviour change is `Homelab/ghostty-in-the-shell` → `gits-seat-comment`** (D6.1(d)): the account it now seats with, replacing the revoked one. - **`Homelab/throwaway-ld-test-20261108` is deliberately NOT mapped**: it is a throwaway test repo, and it stays rejected. That is the fail-closed answer, not an oversight. - **Sendrast**: a9 confirms that no Sendrast repo lands through the landing tool — `sendrast-bot` merges by API, and the release PRs land in `Homelab/prod` (→ `ld-reviewer`). No Sendrast row is needed in the map. ## Rule 1. A repo's set is its entry in the map (`owner/repo` in `repoKey` form, so any spelling the forge or a caller produces resolves). 2. The environment is not read: naming another reviewer, or adding `pi-landing`, is ignored on every repo, gated or not. 3. A repo with **no entry** is refused explicitly (`no trusted reviewer identity is configured for <repo>`), and a malformed entry (empty list, blank login) is refused the same way — never "anyone". ## Tests `forgejo/tests/landing-reviewers-by-repo.test.ts` (new): - GITS counts `gits-seat-comment`; an exact-head `ld-reviewer` verdict — which **used to approve** — is no longer evidence (the row that fails with the old code); - every other mapped repo keeps `ld-reviewer`; - env naming another reviewer / adding `pi-landing`: ignored on GITS, on a mapped non-gated repo and on a GO-gated repo; an `ld-reviewer` verdict on GITS stays untrusted even when the env names it; - unmapped repo → explicit refusal, approvals empty; throwaway repo → refused; - malformed entry → refused; map key spellings (`https://…/X.git`, upper case, trailing slash) resolve. Updated for the removed lever: `landing-gated-reviewers-n6`, `landing-gated-env-pins-n15`, `landing-census-gated-policy-017`, `landing-record`, `landing-server-repo-name-b1`, `landing-corpus`, `docs/forgejo.md`. Local run: the runnable subset passes (584 tests). 19 test files in this checkout do not load because `@mariozechner/pi-ai` is not installed in this pod — pre-existing, unrelated to the change (they fail the same way on `main`); CI installs it and runs the whole suite. ## Do not merge Gate PR: needs a clean pair (SEAT-LINE + SEATSID), homelab-f1's review, and El Director's GO. **No merge from this branch.**
HOMELAB-1679: fix(forgejo): the reviewer identity set is a fixed map in code, per repo (D6.1(d))
Some checks failed
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 36s
Details
CI / pi-otel-telemetry (pull_request) Successful in 42s
Details
CI / plane (pull_request) Successful in 51s
Details
CI / mcp-bridge (pull_request) Successful in 14s
Details
CI / ask-user-question (pull_request) Successful in 29s
Details
CI / nats-intercom (pull_request) Successful in 19s
Details
CI / skill-args (pull_request) Successful in 33s
Details
CI / todo (pull_request) Successful in 39s
Details
tests / python (pull_request) Successful in 6s
Details
CI / refocus (pull_request) Successful in 33s
Details
CI / pi-friction (pull_request) Successful in 28s
Details
CI / redact (pull_request) Successful in 27s
Details
CI / forgejo (pull_request) Failing after 1m45s
Details
CI / pi-swarm-suite (pull_request) Successful in 4m8s
Details
1328c9f327 
The set of logins whose verdict is approving evidence was one fleet-wide default
plus FORGEJO_LANDING_REVIEWERS, an environment override. A value a session can
set is not a gate: any lane could widen its own reviewer set by exporting one
login (the class the N15 pin already closed on GO-gated repos).

Now reviewerIdentitiesByRepo in DEFAULT_LANDING_POLICY names each repo that
lands through the tool, mapped to the set it effectively had in the 30 days up
to 2026-11-12, so removing the env lever moves no repo's behaviour. The one
intended move: Homelab/ghostty-in-the-shell counts gits-seat-comment instead of
ld-reviewer (D6.1(d)). Homelab/throwaway-ld-test-20261108 stays unmapped and is
refused; a repo with no entry (or a malformed one) is refused explicitly by the
lock, never treated as 'anyone'.

The environment is not read for this at all, on gated repos or anywhere else.
HOMELAB-1679: fix(forgejo): review nits for the per-repo reviewer map (fail-closed fallback, frozen map, e2e)
Some checks failed
CI / pr-title (pull_request) Successful in 1s
Details
CI / forgejo-ci (pull_request) Successful in 1m3s
Details
CI / pi-otel-telemetry (pull_request) Successful in 1m16s
Details
CI / skill-args (pull_request) Successful in 1m22s
Details
CI / todo (pull_request) Successful in 1m23s
Details
CI / forgejo (pull_request) Failing after 1m27s
Details
CI / plane (pull_request) Successful in 1m33s
Details
CI / mcp-bridge (pull_request) Successful in 22s
Details
CI / ask-user-question (pull_request) Successful in 42s
Details
CI / nats-intercom (pull_request) Successful in 21s
Details
tests / python (pull_request) Successful in 12s
Details
CI / refocus (pull_request) Successful in 46s
Details
CI / pi-friction (pull_request) Successful in 40s
Details
CI / redact (pull_request) Successful in 46s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m58s
Details
b7fa8c3716 
- fail-closed: no policy inherits NOTHING (the set is the map's answer for the
  repo the input names; an unmapped repo or no repo name resolves to no reviewer
  and the lock refuses), and isTrustedReviewer() without a policy says no. Two
  test rows.
- the map is DEEP-frozen (record + every list) and reviewerIdentitiesForRepo
  hands out a copy, so a caller cannot widen the gate in place.
- e2e: forgejo_merge_pr on Homelab/ghostty-in-the-shell lands an exact-head
  approval from gits-seat-comment and refuses the same approval from ld-reviewer.
- tests updated for the removed lever (n6, server-repo-name-b1, census, record)
  and each grammar test states the reviewer set it means.
HOMELAB-1679: test(forgejo): hermetic landing env + cases for the fail-closed reviewer set
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m49s
Details
CI / pi-otel-telemetry (pull_request) Successful in 51s
Details
CI / forgejo-ci (pull_request) Successful in 37s
Details
CI / plane (pull_request) Successful in 53s
Details
CI / skill-args (pull_request) Successful in 29s
Details
CI / todo (pull_request) Successful in 33s
Details
CI / forgejo (pull_request) Successful in 1m34s
Details
CI / ask-user-question (pull_request) Successful in 29s
Details
CI / mcp-bridge (pull_request) Successful in 24s
Details
CI / refocus (pull_request) Successful in 46s
Details
CI / nats-intercom (pull_request) Successful in 21s
Details
tests / python (pull_request) Successful in 5s
Details
CI / pi-friction (pull_request) Successful in 36s
Details
CI / redact (pull_request) Successful in 25s
Details
bebfd83c16 
The per-repo reviewer map is in code and nothing reads
FORGEJO_LANDING_REVIEWERS any more, so four cases still asserted the old
"is set and ignored" line and two called resolveLandingPolicy() with no
repo name and expected the fleet default.

- tests/setup-env-isolation.ts (new): the suite drops ambient
  FORGEJO_* / PI_LANDING_* before each file. Measured in the GITS sandbox
  2026-11-12, where the pod exports FORGEJO_LANDING_TOKEN_FILE and
  FORGEJO_LEGACY_PANEL_OWNERS: merge-identity merged with the MOUNTED token
  instead of the one the case staged, and the census printed an ignored-lever
  line for a knob the case never set. Green in CI, red here.
- b1 / n6 / census-017: the resolved set is the proof now (mapped repo ->
  ["ld-reviewer"]; unmapped or unreadable name -> [] and the lock refuses),
  and no case asserts an empty stderr for levers it does not own.
- b1 (unreadable server name): the verdict helper now REFUSES, fail-closed,
  instead of posting under the empty set — asserted with zero writes.
- landing-sessions: the legacy-owner override case resolves the policy FOR a
  repo, because with no name the set is empty and the lock refuses first.
- landing.ts: the field type follows the deep freeze
  (Readonly<Record<string, readonly string[]>>) — tsc reported the mismatch
  between REVIEWER_IDENTITIES_BY_REPO and the declared mutable type.

Suite: 1365 passed / 1365 (67 files), vitest 2.1.9, run in the sandbox with
the alias node_modules (pi-swarm-suite node_modules + @earendil-works/pi-ai
linked as @mariozechner/pi-ai), NOT with installed deps.
HOMELAB-1679: fix(forgejo): freeze the default policy in depth; the finding scan reads the repo's own reviewer set
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m25s
Details
CI / forgejo-ci (pull_request) Successful in 31s
Details
CI / pi-otel-telemetry (pull_request) Successful in 35s
Details
CI / todo (pull_request) Successful in 32s
Details
CI / skill-args (pull_request) Successful in 34s
Details
CI / plane (pull_request) Successful in 51s
Details
CI / ask-user-question (pull_request) Successful in 29s
Details
CI / mcp-bridge (pull_request) Successful in 16s
Details
CI / refocus (pull_request) Successful in 34s
Details
tests / python (pull_request) Successful in 5s
Details
CI / nats-intercom (pull_request) Successful in 17s
Details
CI / pi-friction (pull_request) Successful in 27s
Details
CI / redact (pull_request) Successful in 28s
Details
CI / forgejo (pull_request) Successful in 1m44s
Details
8f8c9ef91f 
homelab-f1 r3, the two r2 requests that were not in the code.

B1 — `DEFAULT_LANDING_POLICY` was only half a gate: the reviewer map was
deep-frozen, but the policy object and its four lists were not, so one
`DEFAULT_LANDING_POLICY.dangerousPaths.push(…)` moved the dangerous class for
every caller and one `reviewerIdentities.push(…)` moved the reviewer set for the
fleet. The object and every list are now frozen (a `frozenList` helper takes a
copy first), and the three reads that resolved through it
(`resolveLandingPolicy` ×2, `evaluateLandingLock`'s no-policy fallback) use the
frozen map constant directly, so there is no reachable mutation path.

B2 — `verdict.ts` called `findOpenBlockers(items)` with no options, so the scan
fell back to the default policy and answered for `ld-reviewer` on every repo. In
Homelab/ghostty-in-the-shell that inverted the rule the map exists to enforce:
`gits-seat-comment`, the identity that repo trusts, got "your resolution does
not clear" for `--resolve-review`, and `ld-reviewer`, whose verdicts are no
longer evidence there, closed findings. It now passes `{ policy, headSha }` —
the same policy the rest of the report uses, and the head, so a resolution that
pins another head closes nothing (the lock's own rule).

N2 — `landing-record.test.ts` no longer exercised HOMELAB-2522's citation
binding: the map gives each repo ONE identity, so the two-trusted-author shape
the rule is about cannot arise end to end any more, and the old case was passing
on the untrusted-identity leg instead. It is restored where the rule lives (the
lock input) with the set stated explicitly, and it asserts both directions:
another author's tagged verdict supersedes nothing (`#1601` named, "do not
declare"), the same author's does (the note names the supersession and the
panel passes).

Tests: 4 new/restored cases, each red on the previous commit (B2 both
directions verified red by reverting the one-line call). Suite 1369 passed /
1369 (68 files) with the alias node_modules in the sandbox.
All checks were successful
CI / pr-title (pull_request) Successful in 1s
Details
CI / pi-swarm-suite (pull_request) Successful in 3m25s
Details
CI / forgejo-ci (pull_request) Successful in 31s
Details
CI / pi-otel-telemetry (pull_request) Successful in 35s
Details
CI / todo (pull_request) Successful in 32s
Details
CI / skill-args (pull_request) Successful in 34s
Details
CI / plane (pull_request) Successful in 51s
Details
CI / ask-user-question (pull_request) Successful in 29s
Details
CI / mcp-bridge (pull_request) Successful in 16s
Details
CI / refocus (pull_request) Successful in 34s
Details
tests / python (pull_request) Successful in 5s
Details
CI / nats-intercom (pull_request) Successful in 17s
Details
CI / pi-friction (pull_request) Successful in 27s
Details
CI / redact (pull_request) Successful in 28s
Details
CI / forgejo (pull_request) Successful in 1m44s
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin reviewers-by-repo-f1:reviewers-by-repo-f1
git switch reviewers-by-repo-f1

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff reviewers-by-repo-f1
git switch reviewers-by-repo-f1
git rebase main
git switch main
git merge --ff-only reviewers-by-repo-f1
git switch reviewers-by-repo-f1
git rebase main
git switch main
git merge --no-ff reviewers-by-repo-f1
git switch main
git merge --squash reviewers-by-repo-f1
git switch main
git merge --ff-only reviewers-by-repo-f1
git switch main
git merge reviewers-by-repo-f1
git push origin 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
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!134
No description provided.