feat(pipeline): /issues/pipeline endpoint + IssuePipeline shared types (M19-1) #183

Merged
charles merged 2 commits from boss/174 into main 2026-04-20 19:46:42 +00:00
Collaborator

Summary

Implements M19-1: the backend half of the pipeline monitor. Derives the canonical stage model per issue (Breakdown → Implement → PR → CI → Review ↺ → Approved → Merge → Closed, plus the design track) from existing task_history rows + live Forgejo queries. No new persistent state — every response is a derived view.

What landed

  • Shared types in @claude-hooks/shared (packages/shared/src/pipeline.ts):
    Stage, StageState, StageEntry, IssuePipeline, IssuePipelineResponse,
    PipelineStageEvent. Frozen per the spec so the UI (M19-2+) can consume the
    exact shape the server emits.
  • Derivation in apps/server/src/pipeline.ts:
    • implement / design / design_review / merge / breakdown stages roll up
      task_history rows matching (repo, issue_number) and the appropriate
      agent type.
    • pr stage present iff an open PR body references #<issue> via
      Closes / Fixes / Resolves #N.
    • ci stage folds listWorkflowRuns(head.sha) into worst-of
      pending / success / failure.
    • review stage reads listPullReviews(pr), counts distinct
      REQUEST_CHANGES SHAs for the loop round counter, and flags stalled
      when a pending REQUEST_REVIEW ages past
      pipeline.stall_threshold_ms (default 10 min).
    • approved stage resolves to success iff any APPROVED review at head.
    • closed mirrors issue.state.
  • Endpoint GET /issues/pipeline with ?repo, ?milestone, ?state,
    ?limit (default 100, max 500). 5 s server-side cache keyed by the query
    shape so multiple dashboard tabs share one Forgejo round-trip.
  • SSE: new pipeline_stage event fires alongside the existing
    task_started / task_finished / result events whenever a stage
    transitions. Additive — old SSE consumers unaffected.
  • Config knob: config/agents.json :: pipeline.stall_threshold_ms
    (optional, defaults to 600_000 ms).
  • Forgejo API: adds listRepoIssues + listWorkflowRuns helpers.
  • Tests: pipeline.test.ts (17 cases — derivation across tracks,
    round counter, stall detection, CI worst-of, closed, force-merge,
    cache TTL) + pipeline-sse.test.ts (stage mapping + envelope shape).
  • Docs: README /issues/pipeline section; CLAUDE.md Modules table
    row + API row.

Test plan

  • bun x turbo run typecheck — passes across server / web / shared
  • bun x biome check . — clean
  • bun x biome format . — clean
  • bun test for apps/server — 666 pass, 2 pre-existing web-landing
    failures unrelated to this change (confirmed by stashing all M19-1
    files and re-running)
  • pipeline.test.ts — 17/17 pass in isolation and in the full suite
  • pipeline-sse.test.ts — 11/11 pass in isolation and in the full suite

Closes #174

## Summary Implements M19-1: the backend half of the pipeline monitor. Derives the canonical stage model per issue (`Breakdown → Implement → PR → CI → Review ↺ → Approved → Merge → Closed`, plus the design track) from existing `task_history` rows + live Forgejo queries. No new persistent state — every response is a derived view. ## What landed - **Shared types** in `@claude-hooks/shared` (`packages/shared/src/pipeline.ts`): `Stage`, `StageState`, `StageEntry`, `IssuePipeline`, `IssuePipelineResponse`, `PipelineStageEvent`. Frozen per the spec so the UI (M19-2+) can consume the exact shape the server emits. - **Derivation** in `apps/server/src/pipeline.ts`: - `implement / design / design_review / merge / breakdown` stages roll up `task_history` rows matching `(repo, issue_number)` and the appropriate agent type. - `pr` stage present iff an open PR body references `#<issue>` via `Closes / Fixes / Resolves #N`. - `ci` stage folds `listWorkflowRuns(head.sha)` into worst-of `pending / success / failure`. - `review` stage reads `listPullReviews(pr)`, counts distinct `REQUEST_CHANGES` SHAs for the loop round counter, and flags `stalled` when a pending `REQUEST_REVIEW` ages past `pipeline.stall_threshold_ms` (default 10 min). - `approved` stage resolves to `success` iff any `APPROVED` review at head. - `closed` mirrors `issue.state`. - **Endpoint** `GET /issues/pipeline` with `?repo`, `?milestone`, `?state`, `?limit` (default 100, max 500). 5 s server-side cache keyed by the query shape so multiple dashboard tabs share one Forgejo round-trip. - **SSE**: new `pipeline_stage` event fires alongside the existing `task_started` / `task_finished` / `result` events whenever a stage transitions. Additive — old SSE consumers unaffected. - **Config knob**: `config/agents.json :: pipeline.stall_threshold_ms` (optional, defaults to 600_000 ms). - **Forgejo API**: adds `listRepoIssues` + `listWorkflowRuns` helpers. - **Tests**: `pipeline.test.ts` (17 cases — derivation across tracks, round counter, stall detection, CI worst-of, closed, force-merge, cache TTL) + `pipeline-sse.test.ts` (stage mapping + envelope shape). - **Docs**: README `/issues/pipeline` section; CLAUDE.md Modules table row + API row. ## Test plan - [x] `bun x turbo run typecheck` — passes across server / web / shared - [x] `bun x biome check .` — clean - [x] `bun x biome format .` — clean - [x] `bun test` for `apps/server` — 666 pass, 2 pre-existing web-landing failures unrelated to this change (confirmed by stashing all M19-1 files and re-running) - [x] `pipeline.test.ts` — 17/17 pass in isolation and in the full suite - [x] `pipeline-sse.test.ts` — 11/11 pass in isolation and in the full suite Closes #174
feat(pipeline): /issues/pipeline endpoint + IssuePipeline shared types (M19-1)
Some checks failed
qa / qa (pull_request) Has been cancelled
qa / dockerfile (pull_request) Has been cancelled
a68ac4b41e
Derives the canonical stage model (Breakdown → Implement → PR → CI →
Review ↺ → Approved → Merge → Closed, plus design track) per issue
from existing task_history rows + live Forgejo queries. No new
persistent state. 5-second server-side cache keyed by (repo, milestone,
state, limit) shares one Forgejo round-trip across dashboard tabs.

- `packages/shared/src/pipeline.ts`: Stage / StageState / StageEntry /
  IssuePipeline / IssuePipelineResponse / PipelineStageEvent types.
- `apps/server/src/pipeline.ts`: derivation + cache + SSE helpers.
- `apps/server/src/forgejo-api.ts`: adds `listRepoIssues` +
  `listWorkflowRuns`.
- `apps/server/src/task-store.ts`: adds `listTasksForIssue` to roll up
  task_history rows per (repo, issue).
- `apps/server/src/webhook-config.ts`: new `pipeline.stall_threshold_ms`
  knob (default 600_000 ms).
- `apps/server/src/main.ts`: adds `GET /issues/pipeline`, wires
  `pipeline_stage` SSE emission alongside existing task lifecycle events,
  exports `addSSEListener` for in-process subscribers.
- Tests: pipeline.test.ts covers derivation (implement / design / round
  counter / stall / CI worst-of / closed / force-merge) + cache TTL;
  pipeline-sse.test.ts covers stageForTask + pipelineStageEvent shape.
- Docs: README `/issues/pipeline` section with response shape + stall
  threshold config; CLAUDE.md Modules table + API row.

Closes #174

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
test(pipeline): listRepoIssues + listWorkflowRuns coverage; vite proxy /issues
All checks were successful
qa / qa (pull_request) Successful in 3m52s
qa / dockerfile (pull_request) Successful in 8s
88b3cb16d5
Adds fetch-spy tests for the two new forgejo-api helpers landed in the
/issues/pipeline commit, covering:

- listRepoIssues query-param encoding (state, milestones, limit) and
  default state=open
- listWorkflowRuns parsing both the legacy array shape and the modern
  `{ total_count, workflow_runs: [] }` envelope
- listWorkflowRuns head_sha URL encoding

Also threads `/issues` through the Vite dev proxy alongside /stats,
/usage, … so the upcoming M19 dashboard fetches hit the Bun service
on 4500 without CORS during local dev.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reviewer requested changes 2026-04-20 19:29:33 +00:00
Dismissed
reviewer left a comment

Review — M19-1 /issues/pipeline endpoint

CI: green (run #1777, sha 88b3cb1)

All acceptance criteria from issue #174 are met: shared types match the spec exactly, derivation logic is correct for all 10 stages (including design track, force-merge, round counter, stall detection), the 5 s cache keys on (repo, milestone, state, limit), SSE events are additive, and test coverage is solid (17 derivation cases + 11 SSE cases). The implementation is clean and well-structured.

One functional bug to fix before merge:


Bug — wrong query param name in listRepoIssues silently breaks milestone filtering

File: apps/server/src/forgejo-api.ts

// current — wrong
if (typeof opts.milestone === "number") params.set("milestones", String(opts.milestone));

Forgejo's /repos/{owner}/{repo}/issues endpoint uses milestone (singular) to filter by a specific milestone ID. The plural milestones is not a recognised parameter and is silently ignored, so GET /issues/pipeline?milestone=N returns all issues rather than filtering.

Fix:

if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone));

This matters because the milestone filter is a first-class documented query param for the endpoint, and pipeline views scoped to a milestone (e.g. "show me all M19 issues") will silently return everything instead. The tests don't exercise the milestone path with a real Forgejo response, so the typo slipped through.


Everything else looks good — no other issues.

## Review — M19-1 `/issues/pipeline` endpoint **CI:** ✅ green (run #1777, sha `88b3cb1`) All acceptance criteria from issue #174 are met: shared types match the spec exactly, derivation logic is correct for all 10 stages (including design track, force-merge, round counter, stall detection), the 5 s cache keys on `(repo, milestone, state, limit)`, SSE events are additive, and test coverage is solid (17 derivation cases + 11 SSE cases). The implementation is clean and well-structured. One functional bug to fix before merge: --- ### Bug — wrong query param name in `listRepoIssues` silently breaks milestone filtering **File:** `apps/server/src/forgejo-api.ts` ```typescript // current — wrong if (typeof opts.milestone === "number") params.set("milestones", String(opts.milestone)); ``` Forgejo's `/repos/{owner}/{repo}/issues` endpoint uses `milestone` (singular) to filter by a specific milestone ID. The plural `milestones` is not a recognised parameter and is silently ignored, so `GET /issues/pipeline?milestone=N` returns all issues rather than filtering. Fix: ```typescript if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone)); ``` This matters because the milestone filter is a first-class documented query param for the endpoint, and pipeline views scoped to a milestone (e.g. "show me all M19 issues") will silently return everything instead. The tests don't exercise the milestone path with a real Forgejo response, so the typo slipped through. --- Everything else looks good — no other issues.
Collaborator

Wrong query param name: Forgejo's /repos/{owner}/{repo}/issues endpoint filters by milestone using milestone (singular), not milestones (plural). Change to:

if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone));

The plural is silently ignored by Forgejo, so ?milestone=N on the pipeline endpoint currently returns all issues unfiltered.

Wrong query param name: Forgejo's `/repos/{owner}/{repo}/issues` endpoint filters by milestone using `milestone` (singular), not `milestones` (plural). Change to: ```typescript if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone)); ``` The plural is silently ignored by Forgejo, so `?milestone=N` on the pipeline endpoint currently returns all issues unfiltered.
Author
Collaborator

Respectfully disagreeing on this one — the call is backwards.

Forgejo's /repos/{owner}/{repo}/issues uses milestones (plural). The /pulls endpoint is the one that uses singular milestone, which I suspect is where the confusion came from. Straight from the live swagger at forge.jacquin.app:

GET /repos/issues/search       -> milestones
GET /repos/{owner}/{repo}/issues -> milestones   ← our code path
GET /repos/{owner}/{repo}/pulls  -> milestone

Documented as: "comma separated list of milestone names or ids. It uses names and fall back to ids."

Empirical test against the live instance (repo has 7 milestones, M19 is id=19, 5 issues assigned to it):

?milestones=19   → 5 issues, all milestone.id=19      ✅ filter applied
?milestone=19    → 5 issues including PR #183/184 with no milestone  ❌ silently ignored
no filter        → same unfiltered result as `milestone=`  (control)

So the proposed "fix" would actually introduce the exact bug the comment describes — the existing milestones call is correct and does filter. Leaving the code as-is.

Happy to add a follow-up test that hits a fake server with milestones=N in the URL to lock this in if you'd like — the gap the reviewer correctly identified is that pipeline.test.ts stubs listRepoIssues directly rather than exercising the URL-construction path.

Respectfully disagreeing on this one — the call is backwards. **Forgejo's `/repos/{owner}/{repo}/issues` uses `milestones` (plural).** The `/pulls` endpoint is the one that uses singular `milestone`, which I suspect is where the confusion came from. Straight from the live swagger at `forge.jacquin.app`: ``` GET /repos/issues/search -> milestones GET /repos/{owner}/{repo}/issues -> milestones ← our code path GET /repos/{owner}/{repo}/pulls -> milestone ``` Documented as: *"comma separated list of milestone names or ids. It uses names and fall back to ids."* Empirical test against the live instance (repo has 7 milestones, M19 is id=19, 5 issues assigned to it): ``` ?milestones=19 → 5 issues, all milestone.id=19 ✅ filter applied ?milestone=19 → 5 issues including PR #183/184 with no milestone ❌ silently ignored no filter → same unfiltered result as `milestone=` (control) ``` So the proposed "fix" would actually introduce the exact bug the comment describes — the existing `milestones` call is correct and does filter. Leaving the code as-is. Happy to add a follow-up test that hits a fake server with `milestones=N` in the URL to lock this in if you'd like — the gap the reviewer correctly identified is that `pipeline.test.ts` stubs `listRepoIssues` directly rather than exercising the URL-construction path.
Author
Collaborator

Correction on my last comment: the URL-construction test already exists and already locks this in. apps/server/src/forgejo-api.test.ts:217-225:

test("listRepoIssues encodes state + milestone + limit as query params", async () => {
    registerFetchHandler("/issues?", async () => Response.json([{ number: 1, title: "t", state: "open", labels: [] }]));
    await listRepoIssues(REPO, TOKEN, { state: "closed", milestone: 19, limit: 50 });
    const url = fetchSpy[0]?.url ?? "";
    expect(url).toContain("type=issues");
    expect(url).toContain("state=closed");
    expect(url).toContain("milestones=19");   // ← asserts plural
    expect(url).toContain("limit=50");
});

Applying the reviewer's proposed change would fail this test. Nothing to add, closing out.

Correction on my last comment: the URL-construction test already exists and already locks this in. `apps/server/src/forgejo-api.test.ts:217-225`: ```typescript test("listRepoIssues encodes state + milestone + limit as query params", async () => { registerFetchHandler("/issues?", async () => Response.json([{ number: 1, title: "t", state: "open", labels: [] }])); await listRepoIssues(REPO, TOKEN, { state: "closed", milestone: 19, limit: 50 }); const url = fetchSpy[0]?.url ?? ""; expect(url).toContain("type=issues"); expect(url).toContain("state=closed"); expect(url).toContain("milestones=19"); // ← asserts plural expect(url).toContain("limit=50"); }); ``` Applying the reviewer's proposed change would fail this test. Nothing to add, closing out.
code-lead force-pushed boss/174 from 88b3cb16d5
All checks were successful
qa / qa (pull_request) Successful in 3m52s
qa / dockerfile (pull_request) Successful in 8s
to 88d66d73e4
All checks were successful
qa / qa (pull_request) Successful in 3m57s
qa / dockerfile (pull_request) Successful in 8s
2026-04-20 19:41:41 +00:00
Compare
charles deleted branch boss/174 2026-04-20 19:46:43 +00:00
reviewer left a comment

Review — Round 2

CI: green (run #1781, sha 88d66d7, 4m6s)

The new commit (test(pipeline): listRepoIssues + listWorkflowRuns coverage; vite proxy /issues) added test coverage but did not fix the bug flagged in my previous review. Re-raising the single blocking finding:


Bug still present — milestones (plural) silently ignored by Forgejo

File: apps/server/src/forgejo-api.ts, listRepoIssues

// current — still wrong
if (typeof opts.milestone === "number") params.set("milestones", String(opts.milestone));

Forgejo's GET /repos/{repo}/issues endpoint uses milestone (singular) to filter by a specific milestone ID. The plural milestones is not a recognised query parameter and is silently dropped, so GET /issues/pipeline?milestone=N returns all issues instead of just the ones in that milestone.

Fix:

if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone));

The new tests mock the HTTP layer and don't assert on the constructed URL, so CI stays green despite the typo. One line change — please apply and push.

## Review — Round 2 **CI:** ✅ green (run #1781, sha `88d66d7`, 4m6s) The new commit (`test(pipeline): listRepoIssues + listWorkflowRuns coverage; vite proxy /issues`) added test coverage but **did not fix the bug flagged in my previous review**. Re-raising the single blocking finding: --- ### Bug still present — `milestones` (plural) silently ignored by Forgejo **File:** `apps/server/src/forgejo-api.ts`, `listRepoIssues` ```typescript // current — still wrong if (typeof opts.milestone === "number") params.set("milestones", String(opts.milestone)); ``` Forgejo's `GET /repos/{repo}/issues` endpoint uses `milestone` (singular) to filter by a specific milestone ID. The plural `milestones` is not a recognised query parameter and is silently dropped, so `GET /issues/pipeline?milestone=N` returns all issues instead of just the ones in that milestone. Fix: ```typescript if (typeof opts.milestone === "number") params.set("milestone", String(opts.milestone)); ``` The new tests mock the HTTP layer and don't assert on the constructed URL, so CI stays green despite the typo. One line change — please apply and push.
Author
Collaborator

Disagreeing with this finding — the current code is correct, and the proposed fix would introduce the bug.

Forgejo swagger (https://forge.jacquin.app/swagger.v1.json, GET /repos/{owner}/{repo}/issues) declares the filter parameter as:

{
  "name": "milestones",
  "in": "query",
  "type": "string",
  "description": "comma separated list of milestone names or ids. It uses names and fall back to ids. Fetch only issues that have any of this milestones."
}

Plural — that's the authoritative shape.

Empirical confirmation against this instance (milestone id 18 = "Next-gen UI + architect agent"):

GET /api/v1/repos/charles/claude-hooks/issues?type=issues&state=all&milestones=18&limit=5
→ #170, #169, #168, #167, #166 — all carry "Next-gen UI + architect agent" ✓

GET /api/v1/repos/charles/claude-hooks/issues?type=issues&state=all&milestone=18&limit=5
→ #182 (no milestone), #181/179/178/177 (Pipeline monitor) — param ignored ✗

Singular milestone is the one Forgejo silently drops; plural milestones is what the endpoint actually honours. The endpoint path the reviewer may be thinking of is GET /repos/issues/search, which takes a different parameter shape.

Resolving as won't-fix. PR already merged as 5c63abd.

Disagreeing with this finding — the current code is correct, and the proposed fix would introduce the bug. **Forgejo swagger** (`https://forge.jacquin.app/swagger.v1.json`, `GET /repos/{owner}/{repo}/issues`) declares the filter parameter as: ```json { "name": "milestones", "in": "query", "type": "string", "description": "comma separated list of milestone names or ids. It uses names and fall back to ids. Fetch only issues that have any of this milestones." } ``` Plural — that's the authoritative shape. **Empirical confirmation** against this instance (milestone id 18 = "Next-gen UI + architect agent"): ``` GET /api/v1/repos/charles/claude-hooks/issues?type=issues&state=all&milestones=18&limit=5 → #170, #169, #168, #167, #166 — all carry "Next-gen UI + architect agent" ✓ GET /api/v1/repos/charles/claude-hooks/issues?type=issues&state=all&milestone=18&limit=5 → #182 (no milestone), #181/179/178/177 (Pipeline monitor) — param ignored ✗ ``` Singular `milestone` is the one Forgejo silently drops; plural `milestones` is what the endpoint actually honours. The endpoint path the reviewer may be thinking of is `GET /repos/issues/search`, which takes a different parameter shape. Resolving as won't-fix. PR already merged as `5c63abd`.
Sign in to join this conversation.
No reviewers
No milestone
No project
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!183
No description provided.