fix(review): compare-diff via de geauthenticeerde API + begrensde requeue #139

Merged

janpeter merged 1 commit from fix/compare-diff-authenticated into main 2026-09-09 20:54:38 +02:00

Conversation 0 Commits 1 Files changed 3 +212 -42

janpeter commented 2026-09-09 20:52:29 +02:00

Owner

Copy link

Waarom

De CODEX-worker voerde vanmiddag drie uur lang nul werk uit. Vijf reviewjobs op janpeter/Scrum4Us stonden QUEUED terwijl de worker elke ~20 s dezelfde oudste job claimde, faalde en requeuede:

[run-one-job] claimed job_id=cmtudj0c201dzdp7runccok7b
getFullJobContext: TASK_REVIEW cmtudj0c2… diff-fetch mislukt, requeue — compare: Forgejo compare-diff failed: 404
[run-one-job] exit code=1

Twee onafhankelijke oorzaken. Beide zitten in deze PR.

1. fetchCompareDiff kon op een private repo niet slagen

Hij gebruikte de web-route /{owner}/{repo}/compare/{base}...{head}.diff met een kale fetch() zonder credential.

Dat was geen slordigheid. Het stond zo in het commentaarblok, en de test asserteerde het expliciet: expect(url).not.toContain('/api/v1/'). De redenering was: de API kent geen raw diff voor een range, dus de web-route, en bij een private repo valt de caller terug op de PR-diff. Die premisse klopt alleen niet — een TASK_REVIEW uit een sprint-execution heeft geen pr_url, dus daar bestaat de fallback niet. Ik draai hier dus een gedocumenteerd en getest besluit om, niet een vergissing.

Gemeten op precies de URL die de worker bouwt:

zonder auth (wat de worker doet) : HTTP 404
met token-header op de web-route : HTTP 404   ← web-routes kennen geen PAT
API-compare met token            : HTTP 200
janpeter/Scrum4Us private        : true

De constatering over de API klopt wél, en ik heb hem bevestigd tegen de swagger van de instance zelf:

endpoint	produces
/repos/{o}/{r}/compare/{basehead}	application/json — geen diff
/repos/{o}/{r}/git/commits/{sha}.{diffType}	text/plain
/repos/{o}/{r}/pulls/{index}.{diffType}	text/plain

Daarom nu: de compare-JSON levert de commits van de range, en per commit haalt /git/commits/{sha}.diff de diff op — allebei via forgejoFetch, dus mét token. Precies wat de zusterfunctie fetchPrDiff al deed.

Bewuste gedragswijziging — graag hierop reviewen

Het resultaat is de reeks commit-diffs in chronologische volgorde (git log -p base..head), niet de samengevouwen drie-punts-diff die de web-route gaf. Een bestand dat in twee commits is aangeraakt komt dus twee keer voor. Voor een review is dat een andere, eerder rijkere vorm — maar het is een verschil, en er bestaat geen API-endpoint dat de samengevouwen variant kan leveren. De enige manier om die exact te behouden zou een lokale git diff base...head in de worker-checkout zijn; dat is een grotere ingreep (signatuur + afhankelijkheid van een gefetchte clone) en heb ik hier bewust niet gedaan.

Twee details die daaruit volgen: de API geeft commits nieuwste-eerst, dus de volgorde wordt omgedraaid. En een drie-punts-range trekt merge-historie mee — gemeten gaf HEAD~4...HEAD er 16 — dus er zijn harde grenzen op aantal (50) en omvang (4 MB). Overschrijding is een expliciete fout, geen stil afgekapte diff: een half aangeleverde review is erger dan een geweigerde.

2. De requeue was onbegrensd

Bij een permanente fout requeuet dezelfde job eeuwig, en omdat de claim de oudste QUEUED rij pakt wint hij elke ronde. Eén onmogelijke job legt zo de hele reviewrij stil; de vier jongere reviews werden nooit bereikt. retry_count werd op dit pad niet opgehoogd, dus er was geen poison-detectie.

Nu telt het pad zijn pogingen en gooit TerminalJobError zodra het budget op is. Dat is dezelfde afweging die het !job.task_id-pad twintig regels hoger al maakt ("de poison-loop die de worker na 5 pogingen UNHEALTHY maakt"), en hetzelfde getal als de stale-lease-sweep hanteert (retry_count >= 2).

Verificatie

• 10 tests in __tests__/git/compare-diff.test.ts, herschreven voor het nieuwe contract: API-paden i.p.v. web-route, chronologische volgorde, merge-commits met lege diff, beide grenzen, alle foutpaden.
• vitest op __tests__/git/ + wait-for-job: 147 groen op main, 152 met deze wijziging, geen regressies. De 17 falende testbestanden falen identiek op main (ontbrekende vendor/scrum4me-shared in een verse clone).
• tsc: 85 fouten met én zonder deze wijziging, geen enkele in de gewijzigde hunks — zelfde vendor-oorzaak.
• Live tegen git.jp-visser.nl met de nieuwe implementatie, op de vijf ranges die nu vastzitten:

cmtudj0c201dzdp7runccok7b  OK      9437 bytes  3 bestanden
cmtudvzyx01e6dp7rxzgtao1a  OK     15420 bytes  2 bestanden
cmtuefc0j01eedp7rj3jtwd6f  OK     47235 bytes  7 bestanden
cmtues2ma01efdp7rd39ni325  OK     34705 bytes  9 bestanden
cmtufhsj301f0dp7r16h7r17n  OK     17696 bytes  9 bestanden

Na de merge

De MCP wordt in het worker-image gebakken, dus dit werkt pas na een image-rebuild en een redeploy van de vloot. Tot dat moment blijven de vijf TASK_REVIEWs vastzitten; de drie PR_REVIEW-jobs in dezelfde rij zouden het al doen (die lopen via fetchPrDiff) maar staan achter de vijf. Er is niets gecanceld.

Context en metingen staan in scrum4me-mcp ISS-5 (max2:scrum4me-mcp:compare-diff-unauthenticated-404).

🤖 Generated with Claude Code

https://claude.ai/code/session_019i7qBVDNJ3ur7Htx3GwSTq

## Waarom De CODEX-worker voerde vanmiddag drie uur lang **nul** werk uit. Vijf reviewjobs op `janpeter/Scrum4Us` stonden QUEUED terwijl de worker elke ~20 s dezelfde oudste job claimde, faalde en requeuede: ``` [run-one-job] claimed job_id=cmtudj0c201dzdp7runccok7b getFullJobContext: TASK_REVIEW cmtudj0c2… diff-fetch mislukt, requeue — compare: Forgejo compare-diff failed: 404 [run-one-job] exit code=1 ``` Twee onafhankelijke oorzaken. Beide zitten in deze PR. ## 1. `fetchCompareDiff` kon op een private repo niet slagen Hij gebruikte de **web-route** `/{owner}/{repo}/compare/{base}...{head}.diff` met een kale `fetch()` zonder credential. **Dat was geen slordigheid.** Het stond zo in het commentaarblok, en de test asserteerde het expliciet: `expect(url).not.toContain('/api/v1/')`. De redenering was: de API kent geen raw diff voor een range, dus de web-route, en bij een private repo valt de caller terug op de PR-diff. Die premisse klopt alleen niet — een `TASK_REVIEW` uit een sprint-execution heeft **geen `pr_url`**, dus daar bestaat de fallback niet. Ik draai hier dus een gedocumenteerd en getest besluit om, niet een vergissing. Gemeten op precies de URL die de worker bouwt: ``` zonder auth (wat de worker doet) : HTTP 404 met token-header op de web-route : HTTP 404 ← web-routes kennen geen PAT API-compare met token : HTTP 200 janpeter/Scrum4Us private : true ``` De constatering over de API klopt wél, en ik heb hem bevestigd tegen de swagger van de instance zelf: | endpoint | produces | |---|---| | `/repos/{o}/{r}/compare/{basehead}` | `application/json` — geen diff | | `/repos/{o}/{r}/git/commits/{sha}.{diffType}` | `text/plain` | | `/repos/{o}/{r}/pulls/{index}.{diffType}` | `text/plain` | Daarom nu: de compare-JSON levert de commits van de range, en per commit haalt `/git/commits/{sha}.diff` de diff op — allebei via `forgejoFetch`, dus mét token. Precies wat de zusterfunctie `fetchPrDiff` al deed. ### Bewuste gedragswijziging — graag hierop reviewen Het resultaat is de **reeks commit-diffs in chronologische volgorde** (`git log -p base..head`), niet de samengevouwen drie-punts-diff die de web-route gaf. Een bestand dat in twee commits is aangeraakt komt dus twee keer voor. Voor een review is dat een andere, eerder rijkere vorm — maar het is een verschil, en er bestaat geen API-endpoint dat de samengevouwen variant kan leveren. De enige manier om die exact te behouden zou een lokale `git diff base...head` in de worker-checkout zijn; dat is een grotere ingreep (signatuur + afhankelijkheid van een gefetchte clone) en heb ik hier bewust niet gedaan. Twee details die daaruit volgen: de API geeft commits **nieuwste-eerst**, dus de volgorde wordt omgedraaid. En een drie-punts-range trekt merge-historie mee — gemeten gaf `HEAD~4...HEAD` er **16** — dus er zijn harde grenzen op aantal (50) en omvang (4 MB). Overschrijding is een expliciete fout, geen stil afgekapte diff: een half aangeleverde review is erger dan een geweigerde. ## 2. De requeue was onbegrensd Bij een permanente fout requeuet dezelfde job eeuwig, en omdat de claim de **oudste** QUEUED rij pakt wint hij elke ronde. Eén onmogelijke job legt zo de hele reviewrij stil; de vier jongere reviews werden nooit bereikt. `retry_count` werd op dit pad niet opgehoogd, dus er was geen poison-detectie. Nu telt het pad zijn pogingen en gooit `TerminalJobError` zodra het budget op is. Dat is dezelfde afweging die het `!job.task_id`-pad twintig regels hoger al maakt (*"de poison-loop die de worker na 5 pogingen UNHEALTHY maakt"*), en hetzelfde getal als de stale-lease-sweep hanteert (`retry_count >= 2`). ## Verificatie - **10 tests** in `__tests__/git/compare-diff.test.ts`, herschreven voor het nieuwe contract: API-paden i.p.v. web-route, chronologische volgorde, merge-commits met lege diff, beide grenzen, alle foutpaden. - `vitest` op `__tests__/git/` + `wait-for-job`: **147 groen op `main`, 152 met deze wijziging**, geen regressies. De 17 falende test*bestanden* falen identiek op `main` (ontbrekende `vendor/scrum4me-shared` in een verse clone). - `tsc`: **85 fouten met én zonder** deze wijziging, geen enkele in de gewijzigde hunks — zelfde vendor-oorzaak. - **Live tegen `git.jp-visser.nl`** met de nieuwe implementatie, op de vijf ranges die nu vastzitten: ``` cmtudj0c201dzdp7runccok7b OK 9437 bytes 3 bestanden cmtudvzyx01e6dp7rxzgtao1a OK 15420 bytes 2 bestanden cmtuefc0j01eedp7rj3jtwd6f OK 47235 bytes 7 bestanden cmtues2ma01efdp7rd39ni325 OK 34705 bytes 9 bestanden cmtufhsj301f0dp7r16h7r17n OK 17696 bytes 9 bestanden ``` ## Na de merge De MCP wordt in het worker-image gebakken, dus dit werkt pas na een image-rebuild en een redeploy van de vloot. Tot dat moment blijven de vijf TASK_REVIEWs vastzitten; de drie `PR_REVIEW`-jobs in dezelfde rij zouden het al doen (die lopen via `fetchPrDiff`) maar staan achter de vijf. Er is niets gecanceld. Context en metingen staan in scrum4me-mcp ISS-5 (`max2:scrum4me-mcp:compare-diff-unauthenticated-404`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019i7qBVDNJ3ur7Htx3GwSTq

janpeter added 1 commit 2026-09-09 20:52:29 +02:00

fix(review): compare-diff via de geauthenticeerde API + begrensde requeue ... 86da9c2275

De CODEX-worker voerde vanmiddag drie uur lang nul werk uit. Vijf reviewjobs
op janpeter/Scrum4Us stonden QUEUED terwijl de worker elke ~20 s dezelfde
oudste job claimde, faalde en requeuede:

  getFullJobContext: TASK_REVIEW cmtudj0c2… diff-fetch mislukt, requeue —
  compare: Forgejo compare-diff failed: 404

Twee onafhankelijke oorzaken, allebei hier geadresseerd.

1. fetchCompareDiff kon op een private repo niet slagen

Hij gebruikte de WEB-route /{owner}/{repo}/compare/{base}...{head}.diff met
een kale fetch zonder credential. Dat was bewust en gedocumenteerd, met de
PR-diff als bedoelde fallback, en het stond zo ook in de test
(`expect(url).not.toContain('/api/v1/')`). De premisse klopt alleen niet: een
TASK_REVIEW uit een sprint-execution heeft geen pr_url, dus daar bestaat die
fallback niet. Gemeten op precies de URL die de worker bouwt:

  zonder auth (wat de worker doet) : HTTP 404
  met token-header op de web-route : HTTP 404   (web-routes kennen geen PAT)
  API-compare met token            : HTTP 200

De API kent geen raw diff voor een range — blijkens de swagger van de
instance produceert /repos/{o}/{r}/compare/{basehead} alleen JSON, en `.diff`
op dat pad is 404. Alleen /git/commits/{sha}.{diffType} en
/pulls/{index}.{diffType} leveren text/plain. Daarom: de compare-JSON geeft
de commits, en per commit haalt /git/commits/{sha}.diff de diff op, allebei
via forgejoFetch dus mét token.

Bewuste gedragswijziging: het resultaat is de reeks commit-diffs in
chronologische volgorde (`git log -p base..head`), niet de samengevouwen
drie-punts-diff die de web-route gaf. Een bestand dat in twee commits is
aangeraakt komt dus twee keer voor. De API geeft commits nieuwste-eerst; die
volgorde wordt omgedraaid. Een drie-punts-range trekt merge-historie mee
(gemeten: HEAD~4...HEAD gaf 16 commits), dus zijn er harde grenzen op aantal
en omvang — overschrijding is een expliciete fout, geen stil afgekapte diff.

2. De requeue was onbegrensd

Bij een permanente fout requeuet dezelfde job eeuwig, en omdat de claim de
OUDSTE QUEUED rij pakt wint hij elke ronde: één onmogelijke job legt de hele
reviewrij stil. Vier jongere reviews werden nooit bereikt. retry_count werd
op dit pad niet opgehoogd, dus er was geen poison-detectie.

Nu telt het pad zijn pogingen en gooit het TerminalJobError zodra het budget
op is — dezelfde afweging die het `!job.task_id`-pad hierboven al maakt, en
hetzelfde getal als de stale-lease-sweep (`retry_count >= 2`).

Verificatie

- 10 tests in __tests__/git/compare-diff.test.ts, herschreven voor het nieuwe
  contract (volgorde, merge-commits met lege diff, grenzen, foutpaden).
- vitest op __tests__/git/ + wait-for-job: 147 tests groen op main, 152 met
  deze wijziging. Geen regressies. De 17 falende testBESTANDEN falen identiek
  op main (ontbrekende vendor/scrum4me-shared in een verse clone).
- tsc: 85 fouten met én zonder deze wijziging, geen enkele in de gewijzigde
  hunks — zelfde vendor-oorzaak.
- Live tegen git.jp-visser.nl met de nieuwe implementatie, op de vijf ranges
  die nu vastzitten: alle vijf een echte diff (9437 / 15420 / 47235 / 34705 /
  17696 bytes, 3 / 2 / 7 / 9 / 9 bestanden).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i7qBVDNJ3ur7Htx3GwSTq

janpeter merged commit 1c81b7af75 into main 2026-09-09 20:54:38 +02:00

janpeter referenced this pull request from a commit 2026-09-09 20:54:39 +02:00

Merge pull request 'fix(review): compare-diff via de geauthenticeerde API + begrensde requeue' (#139) from fix/compare-diff-authenticated into main

janpeter referenced this pull request 2026-09-09 20:55:33 +02:00

[ISS-5] fetchCompareDiff haalt de diff onveranderlijk zonder auth op een web-route op — elke TASK_REVIEW op een private repo requeuet eeuwig #138

janpeter referenced this pull request from a commit 2026-09-10 07:48:20 +02:00

docs(audit): range-diff via de API + begrensde TASK_REVIEW-requeue (PR #139)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

severity/s2

  

severity/s3

  

severity/s4

No labels

severity/s2

severity/s3

severity/s4

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

janpeter/scrum4me-mcp!139

No description provided.