[ISS-5] fetchCompareDiff haalt de diff onveranderlijk zonder auth op een web-route op — elke TASK_REVIEW op een private repo requeuet eeuwig #138
Labels
No labels
severity/s2
severity/s3
severity/s4
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
janpeter/scrum4me-mcp#138
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
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?
Status: closed (fixed) · Severity: s2_critical · Gemeld door: max2:claude · Occurrences: 1 (laatst: 2026-09-09T18:15:29.196Z) · Aangemaakt: 2026-09-09T18:15:29.196Z
Registratie
Gemeten 2026-09-09 18:00-18:20 UTC op max2, tegen de live centrale DB en Forgejo. Alleen leesacties; niets gecanceld of herstart.
SYMPTOOM. De CODEX-worker (
scrum4me-agent-codex) draait een oneindige claim-faal-requeue-loop en voert nul werk uit. Vijf QUEUED reviewjobs opjanpeter/Scrum4Usstaan sinds 15:30 stil terwijl de worker elke ~20 s opnieuw dezelfde job claimt.ROOT CAUSE.
src/git/pr.tsfetchCompareDiff()bouwt een web-route en doet een kalefetch()zonder Authorization-header:Op een private repo geeft dat altijd 404. Gemeten op precies de URL die de worker bouwt:
Twee dingen tegelijk fout: (1) geen credential, (2) een web-route in plaats van de API — een Forgejo-PAT werkt sowieso niet op web-routes, dus alleen een header toevoegen lost het niet op. De zusterfunctie in hetzelfde bestand doet het wél goed:
fetchPrDiff()gaat viaforgejoFetch(), en die zetAuthorization: token ...(src/git/forgejo-rest.tsr.262-264). De asymmetrie zit dus tussen twee aangrenzende functies.Uitgesloten alternatieven, elk apart gemeten:
s4m-codex-reviewerIS collaborator op Scrum4Us (/collaborators/s4m-codex-reviewer→ HTTP 204).edb9816a02df204cc32d494e3678de73149d5988en head78ef73d4d6772606e6643f01704ca683e0dc9a09bestaan allebei (HTTP 200) en de API-compare geeft 200 met 1 commit.WAAROM HET NIET VANZELF STOPT. De requeue-tak is bewust gekozen (
src/tools/wait-for-job.tsr.1448-1457) met de comment "Requeue i.p.v. terminaal falen — anders vernietigt één storing alle openstaande reviews." Die aanname klopt alleen bij een transiënte fout. Hier is de fout permanent, en omdat de claim de oudste QUEUED job pakt, wint dezelfde onmogelijke job elke ronde. Het mechanisme dat de andere reviews moest beschermen, verhongert ze juist: vier jongere reviewjobs worden nooit bereikt.retry_countblijft 0 — de requeue telt niet op, dus er is geen poison-detectie die de job na N pogingen parkeert.IMPACT. Elke TASK_REVIEW waarvoor de compare-route wordt gekozen (dus: er is een DONE-execution met base/head-sha, ongeacht of er een pr_url is) faalt permanent op elke private repo. Aangezien alle repo's op deze instance privé zijn, is dat effectief elke TASK_REVIEW. Omdat er één CODEX-worker in de vloot zit, staat daarmee de complete reviewcapaciteit stil.
VOORSTEL (niet uitgevoerd).
fetchCompareDiffoverforgejoFetchlaten lopen op het API-pad/repos/{owner}/{repo}/compare/{base}...{head}, net alsfetchPrDiff. Let op: dat endpoint geeft JSON, geen unified diff — de aanroeper verwacht.diff-tekst, dus de vorm moet mee veranderen (of gebruik het.diff-mediatype als deze Forgejo dat op de API-route ondersteunt; niet gemeten).Bijvangst, apart van dit defect: de worker-healthcheck ververst de heartbeat alleen tussen batches, dus een langlopende job maakt de container
unhealthyterwijl hij correct werkt.scrum4me-worker-idea-1stond op unhealthy met failing streak 104 terwijl hij een geldige SPRINT_IMPLEMENTATION-job draaide (door JP bevestigd). Dat is misleidend bij precies dit soort diagnose.Onderzoek
2026-09-09T18:52:56.748Z — max2:claude
2026-09-09, correctie op de oorspronkelijke melding + fix ingediend.
CORRECTIE. In de description staat dat de ontbrekende Authorization-header een asymmetrie is tussen twee aangrenzende functies, alsof er een header vergeten is. Dat is onjuist. Het commentaarblok boven
fetchCompareDiffdocumenteert de keuze expliciet ("Bewust NIET via forgejoFetch… De web-route kent géén token-auth: een private repo geeft 404 → caller valt terug op de PR-diff") en__tests__/git/compare-diff.test.tsasserteerde het metexpect(url).not.toContain('/api/v1/'). Het 404-gedrag op private repo's was bekend, bedoeld en getest.Het werkelijke defect is dus niet de ontbrekende auth maar de premisse eronder: "er is altijd een PR-diff-fallback". Voor een TASK_REVIEW uit een sprint-execution is er geen
pr_url, dus die fallback bestaat daar niet en de job requeuet eeuwig. Gemeten: van de acht wachtende jobs hebben de vijf TASK_REVIEWs geen PR-fallback (permanent kansloos) en de drie PR_REVIEWs wél — die lopen viafetchPrDiffen zouden gewoon slagen, maar staan in de claim-volgorde achter de vijf.AANVULLENDE MEETGEGEVENS.
/repos/{o}/{r}/compare/{basehead}produceert alleenapplication/json; alleen/git/commits/{sha}.{diffType}en/pulls/{index}.{diffType}geventext/plain. De oorspronkelijke afweging was op dat punt dus correct.Accept: application/json(watforgejoFetchaltijd zet) breekt de.diff-endpoints niet: beide geven HTTP 200 metcontent-type: text/plain.HEAD~4...HEADgaf 16 commits).FIX INGEDIEND: PR #139 op janpeter/scrum4me-mcp, branch
fix/compare-diff-authenticated, commit86da9c2. Twee wijzigingen: (1)fetchCompareDiffhaalt de commits via de geauthenticeerde compare-JSON en per commit de diff via/git/commits/{sha}.diff, met grenzen op aantal (50) en omvang (4 MB); (2) de requeue-tak inwait-for-job.tstelt zijn pogingen en gooitTerminalJobErrorzodra het budget op is, met dezelfde drempel als de stale-lease-sweep.Bewuste gedragswijziging in (1), expliciet ter review meegegeven: het resultaat is de reeks commit-diffs in chronologische volgorde (
git log -p base..head), niet de samengevouwen drie-punts-diff. Een bestand dat in twee commits is aangeraakt komt twee keer voor. De samengevouwen variant is via de API niet te krijgen; dat zou een lokalegit diff base...headin de worker-checkout vereisen.Verificatie: 10 herschreven tests groen; vitest op
__tests__/git/+ wait-for-job 147 groen op main → 152 met de wijziging, geen regressies; tsc 85 fouten met én zonder (vendor-oorzaak, geen enkele in de gewijzigde hunks); en live tegen git.jp-visser.nl leverde de nieuwe implementatie op alle vijf vastzittende ranges een echte diff (9437 / 15420 / 47235 / 34705 / 17696 bytes).NOG NIET UITGEROLD. De MCP wordt in het worker-image gebakken, dus dit werkt pas na een image-rebuild en een redeploy van de vloot. Tot dan blijven de vijf TASK_REVIEWs vastzitten en blokkeren ze de drie werkbare PR_REVIEWs. Er is niets gecanceld.
Oplossing
2026-09-09T19:20:59.540Z — max2:claude
Opgelost en in productie bevestigd op 2026-09-09 21:16-21:19 CEST.
FIX: PR #139 op janpeter/scrum4me-mcp, gemerged als
1c81b7af75.fetchCompareDiffhaalt de commits nu via de geauthenticeerde compare-JSON en per commit de diff via/git/commits/{sha}.diff(grenzen: 50 commits / 4 MB); de requeue-tak inwait-for-job.tstelt zijn pogingen en gooitTerminalJobErrorzodra het budget op is.UITROL — leerpunt. De eerste deploy pakte de fix NIET op terwijl de images wél met
--no-cachewaren herbouwd. Oorzaak: de build kloont geenmainmaar de immutable deploy-tag uitMCP_GIT_REFin /srv/scrum4me/compose/.env, en die stond nog opdeploy/iss3-db519dc9(commitdb519dc9, 31 augustus, 18 commits achter).--no-cachedwong dus een verse clone af van precies dezelfde oude commit. Dat is geen defect maar de gate dieverify_mcp_release_refbewaakt, juist zodat een merge naar main niet stilzwijgend meelift. De ontbrekende stap is altijd: nieuwe deploy-tag,MCP_GIT_REFverzetten, dán rebuilden — en erna de gebakken SHA IN de container controleren, want dat is de enige plek waar het verschil zichtbaar is.Uitgevoerd: rollback-images getagd (
scrum4me-agent-runner:idea-rollback-20260909-211132,scrum4me-agent-codex:rollback-20260909-211132), tagdeploy/iss5-1c81b7afop de merge-commit,MCP_GIT_REFverzet (backup.env.bak.20260909-211140),verify-mcp-release-ref.shexit 0,redeploy_all_workersvia de ops-agent: 7/7 stappen exit 0.Twee details voor een volgende keer: de ops-agent flow-API verwacht
flow_key, nietflow(verkeerde key faalt direct met {"error":"flow_key required"}), en een ANNOTATED deploy-tag laatverify-mcp-release-ref.shhet tag-object-SHA rapporteren in plaats van de commit (444cadcci.p.v.1c81b7af). Cosmetisch: een echtegit clone --branch <tag>— precies wat de Dockerfile doet — landt op de commit; geverifieerd. Eerdere deploy-tags waren lightweight, vandaar het verschil.VERIFICATIE IN PRODUCTIE. Alle drie de containers draaien de merge-commit:
scrum4me-agent-codex,scrum4me-worker-idea-1en-2melden SHA1c81b7af75, COMPARE_MAX_COMMITS aanwezig (3), oude web-route weg (0), DIFF_FETCH_MAX_RETRIES aanwezig (4).Binnen drie minuten na de recreate liep de rij leeg die drie uur had stilgestaan. Job cmtudj0c201dzdp7runccok7b — sinds 15:30 elke ~20 s claimend en falend — is DONE met retry_count=0 en summary "TASK_REVIEW APPROVED (0 findings): Verdict: APPROVED. Beoordeelde diff-bron: compare (base...head-range)". Die vermelding van diff_source=compare is het sluitende bewijs: precies het codepad dat permanent 404 gaf levert nu de diff.
Afgerond tussen 19:16:55Z en 19:19:11Z: drie TASK_REVIEWs (alle op diff_source=compare) en één PR_REVIEW, alle vier DONE met verdict APPROVED. Openstaand daalde van 9 naar 5 en loopt door. Niets gecanceld — alle oorspronkelijke reviewverzoeken zijn gewoon uitgevoerd.