charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

refactor(server): migrate pipeline view to ForgePort (first shape-leak fix) #275

Merged
charles merged 2 commits from refactor/migrate-pipeline-to-forgeport into main 2026-04-23 21:06:05 +00:00
Conversation 0 Commits 2 Files changed 6 +452 -174
claude-desktop commented 2026-04-23 20:52:24 +00:00
Collaborator
Copy link

First of the six per-consumer Forgejo migrations called out in PR #274's follow-up backlog. Pipeline was flagged by review as the highest-value target because it's where raw Forgejo shapes (IssueListEntry, PullRequestSummary, PullReview, WorkflowRunSummary) leaked furthest into view derivation.

What changes

  • domain/views/pipeline.ts switches from forgejo-api imports to ForgePort shapes via ForgejoAdapter
  • PipelineDeps DI signatures take port types; PROD_DEPS builds an adapter per call
  • Internal logic reads domain fields (r.reviewer, r.state: "approved", r.commitSha, r.submittedAt) instead of raw Forgejo strings
  • Review-state comparisons use the lowercase enum ("approved", "changes_requested", "request_review")
  • Workflow run status folding simplified — the port's status enum already encodes what conclusion used to

Port extensions surfaced by this migration

Review's concern #8 on PR #274 called out that shipping the port without consumers made the contract unverified. This migration uncovered three real gaps:

  • ForgeReview.stale — needed by the webhook-ci bounce path (future migration); already surfaced via listPullReviews
  • ForgeWorkflowRun.htmlUrl — pipeline picks the newest run's URL for the "click to see CI" stage link
  • ReviewState adds request_review — distinct from submitted reviews with state "pending"; the review-stage stall detection needs this state to count pending reviewer requests separately from other pending verdicts

Tests

  • pipeline.test.ts / pipeline-stall.test.ts fixture helpers now construct ForgeIssue / ForgePullRequest / ForgeReview / ForgeWorkflowRun
  • New review() and workflowRun() fixture builders to keep per-test fixtures terse
  • CI fixtures drop {status: "completed", conclusion: "success"} in favour of workflowRun({status: "success"}) — the port's resolved status eliminates the two-field dance

Checks

  • bunx tsc --noEmit -p apps/server/tsconfig.json — EXIT=0
  • bun --cwd apps/server test — 990 pass / 4 pre-existing fails (3 sweeper JSONL pruning + 1 foreman listSessions flaky — all present on main)

Remaining migrations (per PR #274 follow-up)

  • board.ts — same Forgejo shapes as pipeline (next)
  • webhook-handlers.ts, webhook-ci.ts, deps.ts, janitor.ts — mutating paths

agent-runner.ts + foreman.ts → Claude SDK (Phase 5, requires agent-runner split).

First of the six per-consumer Forgejo migrations called out in PR #274's follow-up backlog. Pipeline was flagged by review as the highest-value target because it's where raw Forgejo shapes (`IssueListEntry`, `PullRequestSummary`, `PullReview`, `WorkflowRunSummary`) leaked furthest into view derivation. ## What changes - `domain/views/pipeline.ts` switches from `forgejo-api` imports to `ForgePort` shapes via `ForgejoAdapter` - `PipelineDeps` DI signatures take port types; `PROD_DEPS` builds an adapter per call - Internal logic reads domain fields (`r.reviewer`, `r.state: "approved"`, `r.commitSha`, `r.submittedAt`) instead of raw Forgejo strings - Review-state comparisons use the lowercase enum (`"approved"`, `"changes_requested"`, `"request_review"`) - Workflow run status folding simplified — the port's status enum already encodes what `conclusion` used to ## Port extensions surfaced by this migration Review's concern #8 on PR #274 called out that shipping the port without consumers made the contract unverified. This migration uncovered three real gaps: - `ForgeReview.stale` — needed by the webhook-ci bounce path (future migration); already surfaced via `listPullReviews` - `ForgeWorkflowRun.htmlUrl` — pipeline picks the newest run's URL for the "click to see CI" stage link - `ReviewState` adds `request_review` — distinct from submitted reviews with state `"pending"`; the review-stage stall detection needs this state to count pending reviewer requests separately from other pending verdicts ## Tests - `pipeline.test.ts` / `pipeline-stall.test.ts` fixture helpers now construct `ForgeIssue` / `ForgePullRequest` / `ForgeReview` / `ForgeWorkflowRun` - New `review()` and `workflowRun()` fixture builders to keep per-test fixtures terse - CI fixtures drop `{status: "completed", conclusion: "success"}` in favour of `workflowRun({status: "success"})` — the port's resolved status eliminates the two-field dance ## Checks - `bunx tsc --noEmit -p apps/server/tsconfig.json` — EXIT=0 - `bun --cwd apps/server test` — 990 pass / 4 pre-existing fails (3 sweeper JSONL pruning + 1 foreman listSessions flaky — all present on main) ## Remaining migrations (per PR #274 follow-up) - `board.ts` — same Forgejo shapes as pipeline (next) - `webhook-handlers.ts`, `webhook-ci.ts`, `deps.ts`, `janitor.ts` — mutating paths `agent-runner.ts` + `foreman.ts` → Claude SDK (Phase 5, requires agent-runner split).
refactor(server): migrate pipeline to ForgePort; port now covers pipeline surface
All checks were successful
qa / qa (pull_request) Successful in 4m8s
Details
qa / dockerfile (pull_request) Successful in 10s
Details
4d2ce71a5e
 
Pipeline was the biggest shape-leak site — IssueListEntry, PullRequestSummary,
PullReview, WorkflowRunSummary all bled into the view derivation. This
commit switches pipeline.ts and its 3 test files to the domain shapes from
ForgePort.

### Port extensions surfaced by pipeline usage

- `ForgeReview.stale` — Forgejo flags stranded pending requests stale;
  webhook-ci bounce path (future migration) needs this
- `ForgeWorkflowRun.htmlUrl` — pipeline picks the newest run's URL as
  the stage's "click to see CI" link
- `ReviewState` now includes `request_review` — distinct from submitted
  reviews with state "pending"; the review-stage stall detection needs
  this to count pending reviewer requests separately

### pipeline.ts changes

- `PipelineDeps` signatures take ForgePort shapes; `PROD_DEPS` builds a
  `ForgejoAdapter` per call and delegates
- Adapter's `listIssues` filters out PR-backed rows, so pipeline no
  longer needs the `!i.pull_request` guard
- Derivation logic reads domain fields: `r.reviewer` / `r.state` /
  `r.commitSha` / `r.submittedAt` instead of `r.user.login` /
  raw Forgejo strings / `r.commit_id` / `r.submitted_at`
- Review-state comparisons use the lowercase enum: `"approved"`,
  `"changes_requested"`, `"request_review"`
- Workflow run status folding rewritten — the port's status field is
  already the terminal resolution (no separate `conclusion`), so the
  aggregate logic is simpler
- `foldWorkflowRuns` no longer reads `conclusion`; the port's status
  enum ("success" | "failure" | "in_progress" | …) encodes what
  conclusion used to

### Tests

- `pipeline.test.ts` / `pipeline-stall.test.ts` fixture helpers now
  construct ForgeIssue / ForgePullRequest / ForgeReview / ForgeWorkflowRun
- Added `review()` and `workflowRun()` fixture builders to keep per-test
  fixtures terse
- CI fixtures drop `{status: "completed", conclusion: "success"}` in
  favour of `workflowRun({status: "success"})` — the port's resolved
  status eliminates the two-field dance

Full server suite: 990 pass / 4 pre-existing fails (3 sweeper JSONL
pruning + 1 foreman listSessions flaky). Zero regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(server): address PR #275 review — milestone forwarding, CI conclusion folding, assignee merge
All checks were successful
qa / qa (pull_request) Successful in 4m10s
Details
qa / dockerfile (pull_request) Successful in 6s
Details
860f46a2ca
 
Three correctness fixes plus dedicated adapter tests. No runtime behavior
change outside the fixed paths.

### Milestone filter now forwards to Forgejo (critical)

`PROD_DEPS.listRepoIssues` in `pipeline.ts` accepted a numeric milestone
id but never forwarded it, while the pipeline cache key still included
milestone — so different milestones produced identical cache entries
and no server-side filtering. Fix:

- `ForgePort.listIssues` opts now accept `milestone: number | string`
- Numeric id → forwarded to `api.listRepoIssues` as `milestones=<id>`
  (matches Forgejo's server-side query param)
- String title → client-side filter on `milestone.title` (unchanged
  behavior for that branch)
- `PROD_DEPS.listRepoIssues` forwards the milestone when present

### CI `timed_out` / `action_required` fold to failure (critical)

`toForgeWorkflowRun` had no case for these two terminal-failure
conclusions; both fell to `"unknown"`, which pipeline treated as
pending. Users saw "CI running" on timed-out workflows until the stall
threshold fired. Fix: fold both to `"failure"` in the mapper — matches
the pre-migration `foldWorkflowRuns` semantics.

### Singular `assignee` merged into `assignees` array

Forgejo populates both the deprecated singular `assignee` and the plural
`assignees` field; on older rows only the singular is set. `toForgeIssue`
now merges so consumers never have to check both. Pipeline's
`issue.assignees[0] ?? null` is now equivalent to the old
`issue.assignee?.login ?? issue.assignees?.find(...)` two-step.

### New test file: forgejo-adapter.test.ts

17 tests covering the three fixed paths plus `parseRepo` guard
regressions from PR #274 review. Uses a minimal fetch spy — no
`mock.module`, no leak risk to sibling suites.

Server suite: 1006 pass / 4 pre-existing fails.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
charles deleted branch refactor/migrate-pipeline-to-forgeport 2026-04-23 21:06:05 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
area:agents
Agent types, pool scheduling, per-instance config

  
area:dashboard
Dashboard UI and observability surfaces

  
area:database
DB layer — schema, migrations, ORM, raw SQL

  
area:design
UI/UX mockup work — routes to designer agent

  
area:design-review
Design review dispatch — routes to design-reviewer agent

  
area:flows
Flow runner — YAML loader, executor, op registry, expression eval

  
area:infra
Deployment, isolation, containers, systemd units

  
area:meta
Tracking, scaffolding, project setup

  
area:security
Security — routes to reviewer-security (opus)

  
area:sessions
Session-id store, Claude SDK resume logic

  
area:webhook
Forgejo webhook routing and handlers

  
area:workdir
Clone cache, worktrees, git identity

  
security
Security-sensitive issue

  
type:bug
Bug

  
type:chore
Chore

  
type:meta
Tracking or decisions, not implementation work

  
type:user-story
User story

No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
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!275
No description provided.