feat(pipeline): /issues/pipeline endpoint + IssuePipeline shared types (M19-1) #183
No reviewers
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks!183
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "boss/174"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 existingtask_historyrows + live Forgejo queries. No new persistent state — every response is a derived view.What landed
@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 theexact shape the server emits.
apps/server/src/pipeline.ts:implement / design / design_review / merge / breakdownstages roll uptask_historyrows matching(repo, issue_number)and the appropriateagent type.
prstage present iff an open PR body references#<issue>viaCloses / Fixes / Resolves #N.cistage foldslistWorkflowRuns(head.sha)into worst-ofpending / success / failure.reviewstage readslistPullReviews(pr), counts distinctREQUEST_CHANGESSHAs for the loop round counter, and flagsstalledwhen a pending
REQUEST_REVIEWages pastpipeline.stall_threshold_ms(default 10 min).approvedstage resolves tosuccessiff anyAPPROVEDreview at head.closedmirrorsissue.state.GET /issues/pipelinewith?repo,?milestone,?state,?limit(default 100, max 500). 5 s server-side cache keyed by the queryshape so multiple dashboard tabs share one Forgejo round-trip.
pipeline_stageevent fires alongside the existingtask_started/task_finished/resultevents whenever a stagetransitions. Additive — old SSE consumers unaffected.
config/agents.json :: pipeline.stall_threshold_ms(optional, defaults to 600_000 ms).
listRepoIssues+listWorkflowRunshelpers.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)./issues/pipelinesection; CLAUDE.md Modules tablerow + API row.
Test plan
bun x turbo run typecheck— passes across server / web / sharedbun x biome check .— cleanbun x biome format .— cleanbun testforapps/server— 666 pass, 2 pre-existing web-landingfailures 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 suitepipeline-sse.test.ts— 11/11 pass in isolation and in the full suiteCloses #174
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>Review — M19-1
/issues/pipelineendpointCI: ✅ 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
listRepoIssuessilently breaks milestone filteringFile:
apps/server/src/forgejo-api.tsForgejo's
/repos/{owner}/{repo}/issuesendpoint usesmilestone(singular) to filter by a specific milestone ID. The pluralmilestonesis not a recognised parameter and is silently ignored, soGET /issues/pipeline?milestone=Nreturns all issues rather than filtering.Fix:
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.
Wrong query param name: Forgejo's
/repos/{owner}/{repo}/issuesendpoint filters by milestone usingmilestone(singular), notmilestones(plural). Change to:The plural is silently ignored by Forgejo, so
?milestone=Non the pipeline endpoint currently returns all issues unfiltered.Respectfully disagreeing on this one — the call is backwards.
Forgejo's
/repos/{owner}/{repo}/issuesusesmilestones(plural). The/pullsendpoint is the one that uses singularmilestone, which I suspect is where the confusion came from. Straight from the live swagger atforge.jacquin.app: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):
So the proposed "fix" would actually introduce the exact bug the comment describes — the existing
milestonescall is correct and does filter. Leaving the code as-is.Happy to add a follow-up test that hits a fake server with
milestones=Nin the URL to lock this in if you'd like — the gap the reviewer correctly identified is thatpipeline.test.tsstubslistRepoIssuesdirectly rather than exercising the URL-construction path.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:Applying the reviewer's proposed change would fail this test. Nothing to add, closing out.
88b3cb16d588d66d73e4Review — 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 ForgejoFile:
apps/server/src/forgejo-api.ts,listRepoIssuesForgejo's
GET /repos/{repo}/issuesendpoint usesmilestone(singular) to filter by a specific milestone ID. The pluralmilestonesis not a recognised query parameter and is silently dropped, soGET /issues/pipeline?milestone=Nreturns all issues instead of just the ones in that milestone.Fix:
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.
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:Plural — that's the authoritative shape.
Empirical confirmation against this instance (milestone id 18 = "Next-gen UI + architect agent"):
Singular
milestoneis the one Forgejo silently drops; pluralmilestonesis what the endpoint actually honours. The endpoint path the reviewer may be thinking of isGET /repos/issues/search, which takes a different parameter shape.Resolving as won't-fix. PR already merged as
5c63abd.