refactor(server): migrate janitor to ForgePort — impl seam wraps adapter #279

Merged
charles merged 1 commit from refactor/janitor-forge-port into main 2026-04-23 21:51:10 +00:00
Collaborator

Fifth per-consumer Forgejo migration. Janitor uses an impl seam pattern for test isolation (not mock.module, which is process-global and leaks across suites); this PR rewrites the seam entries to wrap ForgejoAdapter calls while preserving the seam's swap-for-tests contract.

Scope

background/janitor.ts + background/janitor.test.ts. One additional port extension surfaced by three rules.

Port extension

ForgeIssueRef ({number, state}) — added. Three janitor rules (closes_keyword_drift, dependent_unblocked, stale_idle_assigned) need blocker state to decide whether to act. The port's getBlockers / getBlocked previously returned number[] and dropped the state field from the dependency payload.

  • getBlockers(repo, number): Promise<ForgeIssueRef[]>
  • getBlocked(repo, number): Promise<ForgeIssueRef[]>

Adapter projects api.IssueDependencyEntry.state (default "open" when absent on older Forgejo builds) into the new ref shape.

Downstream consumer impact: only domain/workflow/deps.ts uses these methods via the port, and per #277's report it kept a raw forgejo-api.getIssueBlocks import for its cross-repo filter — so the port change doesn't affect the deps PR.

impl seam rewrite

Each entry now wraps the adapter, e.g.:

getIssue: async (repo: string, n: number, token: string) =>
  new ForgejoAdapter(token).getIssue(parseRepo(repo), n),

Rule bodies read domain fields (issue.labels.includes("area:design") instead of issue.labels.some(l => l.name === "area:design"), r.reviewer instead of r.user.login, etc.).

Label-mutation simplification

label_dispatch_miss previously pre-loaded listRepoLabels to resolve name → id. The port's addLabels/removeLabel take names; the adapter resolves IDs internally. Rule dropped the listRepoLabels step; test asserts now compare against the name string.

Tests

  • janitor.test.ts — new fixture builders issue(), pr(), review(), comment(), blocker() mirror the pipeline/board pattern
  • All 42/42 janitor tests pass
  • Full server suite: 1006 pass / 4 pre-existing fails

Overlap with sibling PRs

Opened alongside #277 (deps) and earlier webhook-ci PR. If deps merges first this one auto-rebases cleanly; if this merges first, deps needs a trivial .number extraction fixup.

Fifth per-consumer Forgejo migration. Janitor uses an `impl` seam pattern for test isolation (not `mock.module`, which is process-global and leaks across suites); this PR rewrites the seam entries to wrap `ForgejoAdapter` calls while preserving the seam's swap-for-tests contract. ## Scope `background/janitor.ts` + `background/janitor.test.ts`. One additional port extension surfaced by three rules. ## Port extension **`ForgeIssueRef` (`{number, state}`)** — added. Three janitor rules (`closes_keyword_drift`, `dependent_unblocked`, `stale_idle_assigned`) need blocker state to decide whether to act. The port's `getBlockers` / `getBlocked` previously returned `number[]` and dropped the `state` field from the dependency payload. - `getBlockers(repo, number): Promise<ForgeIssueRef[]>` - `getBlocked(repo, number): Promise<ForgeIssueRef[]>` Adapter projects `api.IssueDependencyEntry.state` (default `"open"` when absent on older Forgejo builds) into the new ref shape. **Downstream consumer impact:** only `domain/workflow/deps.ts` uses these methods via the port, and per #277's report it kept a raw `forgejo-api.getIssueBlocks` import for its cross-repo filter — so the port change doesn't affect the deps PR. ## `impl` seam rewrite Each entry now wraps the adapter, e.g.: ```ts getIssue: async (repo: string, n: number, token: string) => new ForgejoAdapter(token).getIssue(parseRepo(repo), n), ``` Rule bodies read domain fields (`issue.labels.includes("area:design")` instead of `issue.labels.some(l => l.name === "area:design")`, `r.reviewer` instead of `r.user.login`, etc.). ## Label-mutation simplification `label_dispatch_miss` previously pre-loaded `listRepoLabels` to resolve `name → id`. The port's `addLabels`/`removeLabel` take names; the adapter resolves IDs internally. Rule dropped the `listRepoLabels` step; test asserts now compare against the name string. ## Tests - `janitor.test.ts` — new fixture builders `issue()`, `pr()`, `review()`, `comment()`, `blocker()` mirror the pipeline/board pattern - All 42/42 janitor tests pass - Full server suite: 1006 pass / 4 pre-existing fails ## Overlap with sibling PRs Opened alongside #277 (deps) and earlier webhook-ci PR. If deps merges first this one auto-rebases cleanly; if this merges first, deps needs a trivial `.number` extraction fixup.
refactor(server): migrate janitor to ForgePort — impl seam wraps adapter
All checks were successful
qa / qa (pull_request) Successful in 4m14s
qa / dockerfile (pull_request) Successful in 7s
8b06979713
Third per-consumer Forgejo migration after pipeline (#275) and board.
Janitor consumed the widest surface of the raw `forgejo-api.ts` (15
functions), so this also exercises the parts of the port that were
untouched by the view migrations: `ForgeComment`, `addLabels` +
`removeLabel` (name-indexed, adapter resolves IDs), `patchIssue`.

### Design: impl seam stays, shape underneath flips

Janitor keeps its `impl` test seam — Bun's `mock.module` is
process-global and the seam is the only mechanism that prevents
per-file stubs from leaking into sibling suites. Each `impl.*` entry
is now a port-backed closure: `new ForgejoAdapter(token).<method>
(parseRepo(repo), ...)` returning domain shapes (`ForgeIssue`,
`ForgePullRequest`, `ForgeReview`, `ForgeComment`, `ForgeIssueRef`).

The seam's call signatures stay `(repo, ..., token)` so `main.ts`'s
wiring is unchanged; only the return types flip from raw Forgejo
JSON to port shapes.

### Rule-body simplifications

- `issue.labels?.some((l) => l.name === "x")` → `issue.labels.includes("x")`
- `issue.assignees?.map((a) => a.login).filter(Boolean)` → `issue.assignees`
- `review.state === "APPROVED"` → `review.state === "approved"`
- `review.state === "REQUEST_CHANGES"` → `review.state === "changes_requested"`
- `pr.merged_at` / `pr.head.sha` / `pr.head.ref` → `pr.mergedAt` / `pr.headSha` / `pr.headRef`
- `comment.user?.login` → `comment.author`
- Dropped `issue.pull_request` skip — `listIssues` already sets `type=issues` server-side
- `label_dispatch_miss` now passes label *names* to `addLabels`/`removeLabel`;
  the adapter resolves names to Forgejo IDs internally, so the rule no
  longer fetches `listRepoLabels` to build an `id-by-name` map

### Port extension: `ForgeIssueRef`

`getBlockers` / `getBlocked` on the port returned `number[]`, discarding
the `state` field three janitor rules need (`closes_keyword_drift`,
`dependent_unblocked`, `stale_idle_assigned`). Added a lightweight
`ForgeIssueRef` shape (`{ number, state }`) and changed both port
methods to return it. Only caller (deps.ts) stayed on raw `forgejo-api`
so no consumer update needed beyond the janitor.

### Tests

- `janitor.test.ts` fixture builders — `issue()`, `pr()`, `review()`,
  `comment()`, `blocker()` — construct port shapes directly, mirroring
  the pipeline / board test conventions.
- Mock stubs return `ForgeIssue[]` / `ForgePullRequest[]` / `ForgeReview[]`
  / `ForgeComment[]` / `ForgeIssueRef[]` instead of hand-rolled raw JSON.
- `label_dispatch_miss` assertions updated for name-indexed mutation:
  `removeIssueLabel.mock.calls[0][2]` is now `"area:design"`, not `7`.
- 42 tests — all pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
charles deleted branch refactor/janitor-forge-port 2026-04-23 21:51:10 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!279
No description provided.