Page both /statuses/{sha} reads in review-verdict.yml — limit clamps to 50, so the post-write race check can miss a raced verdict #763
Closed
opened 2026-08-10 19:28:49 +02:00 by timothy
·
3 comments
No Branch/Tag Specified
main
renovate/meziantou.analyzer-3.x
release/v26.15.0-notes
fix/830-add-items-error-surface
renovate/lucene.net
renovate/cliwrap-3.x
issue-806-guard-populations
renovate/dotnet-monorepo
scratch/767b-poisoned
scratch/767b-control
release/v26.14.0-notes
release/v26.14.0
renovate/sqlitepclraw.bundle_e_sqlite3-3.x
docs/510-skill-logo-bug-policy
fix/510-watermark-resolution-policy
fix/629-verdict-classifier-falseopens
fix/609-decisions-edit-token-scope
issue-135-clear-to-none
release/v26.12.0-notes
fix/409b-lastscan-api-parity
fix/401-updatechannel-mirror-422
fix/327-playlist-rename-validation
fix/410-scancancel-log-level
fix/409-447-librariesscreen-neverscanned
fix/338-zap-exit-code
fix/367-plex-budget-message
fix/310-debom-legacy-cs
ci/604-lane-rebalance
feat/388-design-mirror
feat/247-test-ownership
feat/247-primary-action
feat/357-player-owned-playback
feat/357-jellyfin-plugin-poc
fix/289-mcp-hardening
issue58-mcp
feat/244-channels-extract
ci/auto-bump-prod-compose
feat/multi-rerun-collections-api
feat/collections-api
feat/quick-wins
feat/185-docs-part2
feat/140-collections-screen
feat/146-channel-edit
feat/147-classic-ui-link
issue22-renovate-dashboard
feat/91-cutover
feat/63-composite-create
feat/65-library-browse
feat/85-epg
feat/86-schedule-editor
feat/109-dashboard-data
feat/99-session-tracking
fix/dockerfile-node-tag
feat/59-spa-foundation
docs/59-ui-redesign-brief
feat/102-json-guide
feat/111-schedule-durations
feat/104-artwork-upload
feat/103-media-sources-api
feat/playouts-read-api
feat/108-health-api
feat/105-picker-list-endpoints
issue-97-channel-state-api
issue42-jellyfin-musicvideos
issue46-rest-api-error-contract
dependabot/nuget/ErsatzTV.FFmpeg.Tests/multi-d307a2e06f
qsv-improvements
hdr-vulkan-cuda-test
v26.15.0
v26.14.0
v26.13.0
v26.12.0
v26.11.0
v26.10.0
v26.9.0
v26.8.0
v26.7.0
blazor-final
v26.6.0
v26.5.0
v26.4.0
v26.3.1
v26.3.0
v26.2.0
v26.1.1
v26.1.0
v25.9.0
v25.8.0
v25.7.1
v25.7.0
v25.6.0
v25.5.0
v25.4.0
v25.3.1
v25.3.0
v25.2.0
v25.1.0
v0.8.8-beta
v0.8.7-beta
v0.8.6-beta
v0.8.5-beta
v0.8.4-beta
v0.8.3-beta
v0.8.2-beta
v0.8.1-beta
v0.8.0-beta
v0.7.9-beta
v0.7.8-beta
v0.7.7-beta
v0.7.6-beta
v0.7.5-beta
v0.7.4-beta
v0.7.3-beta
v0.7.2-beta
v0.7.1-beta
v0.7.0-beta
v0.6.9-beta
v0.6.8-beta
v0.6.7-beta
v0.6.6-beta
v0.6.5-beta
v0.6.4-beta
v0.6.3-beta
v0.6.2-beta
v0.6.1-beta
v0.6.0-beta
v0.5.8-beta
v0.5.7-beta
v0.5.6-beta
v0.5.5-beta
v0.5.4-beta
v0.5.3-beta
v0.5.2-beta
v0.5.1-beta
v0.5.0-beta
v0.4.5-alpha
v0.4.4-alpha
v0.4.3-alpha
v0.4.2-alpha
v0.4.1-alpha
v0.4.0-alpha
v0.3.8-alpha
v0.3.7-alpha
develop
v0.3.6-alpha
v0.3.5-alpha
v0.3.4-alpha
v0.3.3-alpha
v0.3.2-alpha
v0.3.1-alpha
v0.3.0-alpha
v0.2.5-alpha
v0.2.4-alpha
v0.2.3-alpha
v0.2.2-alpha
v0.2.1-alpha
v0.2.0-alpha
v0.1.5-alpha
v0.1.4-alpha
v0.1.3-alpha
v0.1.2-alpha
v0.1.1-alpha
v0.1.0-alpha
v0.0.62-alpha
v0.0.61-alpha
v0.0.60-alpha
v0.0.59-alpha
v0.0.58-alpha
v0.0.57-alpha
v0.0.56-alpha
v0.0.55-alpha
v0.0.54-alpha
v0.0.53-alpha
v0.0.52-alpha
v0.0.51-alpha
v0.0.50-alpha
v0.0.49-prealpha
v0.0.48-prealpha
v0.0.47-prealpha
v0.0.46-prealpha
v0.0.45-prealpha
v0.0.44-prealpha
v0.0.43-prealpha
v0.0.42-prealpha
v0.0.41-prealpha
v0.0.40-prealpha
v0.0.39-prealpha
v0.0.38-prealpha
v0.0.37-prealpha
v0.0.36-prealpha
v0.0.35-prealpha
v0.0.34-prealpha
v0.0.33-prealpha
v0.0.32-prealpha
v0.0.31-prealpha
v0.0.30-prealpha
v0.0.29-prealpha
v0.0.28-prealpha
v0.0.27-prealpha
v0.0.26-prealpha
v0.0.25-prealpha
v0.0.24-prealpha
v0.0.23-prealpha
v0.0.22-prealpha
v0.0.21-prealpha
v0.0.20-prealpha
v0.0.19-prealpha
v0.0.18-prealpha
v0.0.17-prealpha
v0.0.16-prealpha
v0.0.15-prealpha
v0.0.14-prealpha
v0.0.13-prealpha
v0.0.12-prealpha
v0.0.11-prealpha
v0.0.10-prealpha
v0.0.9-prealpha
v0.0.8-prealpha
v0.0.7-prealpha
v0.0.6-prealpha
v0.0.5-prealpha
v0.0.4-prealpha
v0.0.3-prealpha
v0.0.2-prealpha
v0.0.1-prealpha
Labels
Clear labels
ad-hoc
api
bug
ci-cd
content
dependencies
enhancement
frontend
in-progress
jellyfin
parked
priority: high
priority: low
priority: medium
review
security
One-off / ad-hoc work not tracked by a dedicated issue
REST API / HTTP endpoints
Something isn't working
Build, test, deploy pipeline
Channel content / schedules / playlists
Dependency updates (Renovate)
New feature or improvement
ChicoryTV React SPA frontend
Claimed by an active session — do not pick up
Jellyfin tuner / IPTV integration
Excluded from automatic queue pickup; work only when explicitly selected
Adversarial review finding
Security / vulnerability fix
Milestone
No items
No Milestone
Projects
Clear projects
No projects
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#763
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Split out of #751, raised by its fourth cold review round.
The residual
review-verdict.ymlreads the per-POST status historyGET /statuses/{sha}?limit=100twice — once for the high-water mark (hist_before) and once for the post-write race check (post_hist). Neither pages.limit=100clamps to the server-wideMAX_RESPONSE_ITEMS, measured at 50 on this instance. So on a head with more than 50 status rows, both reads see a partial list:post_hist— a raced humanReview-verdict: BLOCKEDcan sit on a page the job never reads, soraced=0, no repair fires, and the exemptionsuccessstands over a human rejection. This is the one path in the design whose failure direction is toward SUCCESS.hist_before— the high-water mark may not be the true maximum id, which makes pre-existing rows look newer than the mark. That direction is safe (a false repair topending), but it is a stall nobody would understand.#751 added a conservative mitigation for the first: if page 1 shows no raced row, the job reads page 2 and treats any rows there — or an unreadable page 2 — as "assume raced", repairing to
pending. That closes the fail-open direction without paging, but it is a blunt instrument: on a head that genuinely runs past one page, every exemption gets repaired topendingand needs a human verdict.Why it is not urgent
Reachable but not currently reached: a live probe head carried 33 rows after ~5 workflow runs (measured 2026-08-10), against a cap of 50. A PR with a few more CI reruns gets there. Ordering is only coarsely newest-first (ids came back
33,32,31,30,28,29,27,…), so the raced row being on page 1 is not something to rely on — andreview-verdict.ymlexplicitly disclaims relying on order.Proposed
Page both reads to a validated terminator, the way
count_retargetsdoes, and drop the blunt page-2 mitigation once real paging exists.Notes from #751 that apply directly:
/statuses/{sha}past the end returns[];/commits/{sha}/statusreturns{"statuses": null};/issues/{n}/timelinereturns barenull;/issues/{n}/commentsreturns[]. Measure the one you are paging — guessing has been wrong twice.limit=100that was dead code because the cap is 50, and the repo had already documented that cap in three places.total_counton the combined endpoint is per page, not per commit, so it cannot detect truncation.ci.paged-endpoint-completenessis cited in the workflow but has never existed as a record — #751 repointed that citation).appears-on-read:N/human-after-postcount how many times the job LOOKED, and a completeness probe is part of the same look. Getting this wrong presents as three unrelated mid-run-race tests going red, and the tempting fix (bumping the expected counts) destroys what they measure.Done-when
/statuses/{sha}reads page to a validated terminator, with a page cap and an explicit "could not establish" pathdocs/ci-cd.md+ci.verdict-write-retarget-fenceupdatedLive instance observed 2026-08-26 (while working #708)
This issue's predicted cost — "on a head that genuinely runs past one page, every exemption gets
repaired to
pendingand needs a human verdict" — fired on a real Renovate PR, not a probe head.Renovate PR #761 (
renovate/meziantou.analyzer-3.x, head8798a1d2, changed set exactlyDirectory.Packages.props). The gate classified it correctly and posted its exemption, then repairedit away in the same job:
No human verdict existed on that head. The
::error::is a false positive: page 2 merely had rows,which the conservative rule treats as "assume raced".
Three things this pins down that the issue body left open:
long-lived dependency PR accumulating re-runs, not an artificial case.
MAX_RESPONSE_ITEMSisconfirmed at 50 on this instance.
REPAIR_DESCis on the sha, theex_repairbranch refuses to re-exempt it on every later run — by design (it must be a fixedpoint), but it means a head that crosses the page boundary is permanently un-exempt until a human
posts a verdict. Re-triggering does not clear it; a later run posted the same sentinel again.
types: [… edited]and thatwas sufficient. No push, no retarget, no actual concurrency required.
So the failure direction is toward a stall that nobody can diagnose from the status alone (the
description asserts a human verdict was overwritten, which is false), and it lands on exactly the PRs
the exemption exists to keep moving. That argues for the paging fix this issue proposes rather than
leaving the blunt mitigation in place.
Evidence: run 2288 / job 9690, and the
review-verdict/h10history on8798a1d2(4 rows, ids 63/64posted in the same second, then 98 and 102).
Side effect to be aware of: PR #761's
h10is currentlypendingfor this reason and needs are-posted verdict (or will re-exempt itself on Renovate's next rebase to a fresh sha). It carries no
human rejection — the sentinel is the false positive described above.
Claiming (Claude Code session, 2026-08-28, worktree
main-2).Pre-claim checks per
process.parallel-session-claim, all four run: no open PR referencing #763 (the five open PRs are all Renovate dependency bumps), no remote branch naming it, no prior claiming comment, and a freshgit fetch origin main.Parallel-session note (sessions are indistinguishable in Gitea, so stating intent): a concurrent session claimed #781 + #799 at 14:34 today — plugin/MCP config hygiene under
.mcp.json/.claude/settings*.json. That is disjoint from this issue's file set (.gitea/workflows/review-verdict.yml,scripts/tests/,docs/). I also skipped the higher-ranked #747 deliberately rather than on size: its remaining scope mutates shared live infra — the owner-level Actions token mode andmain's branch protection — which the concurrent session depends on to merge.Not bundling #803, though it is the nearest sibling (same label, same subsystem, both about paged reads of remote state in the gate). #803 was split out of #778 precisely because stacking pre-existing-contract corrections onto an already multi-round gate PR "is how a scoped change stops being reviewable", and the fix here rewrites this workflow's paging semantics. Folding them together would reproduce the exact failure #803 documents as its own reason for existing.
Starting from the 2026-08-26 live-fire comment above (Renovate PR #761, head
8798a1d2) as the reproduction target: the conservative page-2 mitigation posted a false::error::asserting a human verdict was overwritten when none existed, and the repair is sticky. Note that PR #761 is still open withh10stuckpendingfor this reason — I will not clear that by hand, since it is the live evidence.Closing record
Outcome: Shipped — PR #868, merged as
609fd852c. Both/statuses/{sha}reads page to a validated empty page (page cap 20, one retry per page, explicit "could not establish" path), the high-water mark is computed over the paged list, and #751's page-2 "assume raced" probe is deleted. All seven## Done-whenboxes satisfied.Root cause:
GET /statuses/{sha}?limit=100clamps to the server-wideMAX_RESPONSE_ITEMS(50), so a single read returned a partial list. But this issue's own framing of the consequence was wrong, and that correction is the most reusable thing here. Under the server default (created_unix DESC) page 1 already held the true maximum id and every row newer than the mark — the only rows the post-write check selects on. A single-page read could miss a raced verdict only if more than 50 rows were created inside the write window, not merely on "a head with more than 50 rows". What actually caused PR #761's stall was the page-2 probe treating "there are rows I did not read" as "assume raced"; retiring it is the fix. The walk earns its place for a different reason: the gate's one fail-toward-SUCCESS path no longer rests on an undocumented ordering the server honours only coarsely.Decisions/conventions changed: none added.
ci.verdict-write-retarget-fenceupdated (rule, mechanics, body) — it already owned this workflow's paging discipline, so a second record would have split one rule across two.Reusable knowledge:
.id > $sincehad the identical string-vs-number flaw as the high-watermax— jq orders strings above every number, so a pre-existing row with"id": "3"read as newer than any mark, was counted as raced, and earned the sticky sentinel plus a false "was overwritten" on every later run. Live onmain, found only because a reviewer swept the twin.sort=highestindex(closes a mid-walk-insert gap, but ASC puts the oldest rows on page 1, inverting the partial-mark fallback into the very #761 stall). Both withdrawn, both documented as rejected alternatives so they are not re-adopted — the second with a behavioural test, not just a structural one.max()over mixedstr/int— so a mutation stayed green because the double crashed, not because the code was right.git checkout -- <file>restores from the INDEX. Used it to revert a mutation and it silently discarded unstaged edits. Restore from an explicit backup copy instead.Verification:
scripts/tests1097 passed / 2 skipped. Eighteen executed mutations, each reddening its named test; one arm is unreachable by any fixture and is annotated as such rather than claimed as proved.page_statusesexecuted in isolation underset -euo pipefailacross 13–15 hostile inputs. Terminator shape,limitclamp, sort order and id monotonicity all measured live on Gitea 1.27.1 (2026-08-28) rather than taken from prose. CI green on11287a54b.Review: six independent cold passes (one cross-family Codex, five isolated Opus agents). They found a Blocker, a live-on-
maintwin defect, four documentation overclaims, and five "test passes for the wrong reason" instances. A seventh cross-family pass was attempted and could not complete — the Codex account hit its usage limit — so the re-reviews of the fix rounds were same-family; recorded rather than glossed.Deferred:
Docs updated:
docs/ci-cd.md,docs/decisions/records/ci/verdict-write-retarget-fence.md(+ regenerateddocs/decisions/README.md). Both carry the corrected reachability, the DESC dependency of the partial-mark fallback, both withdrawals, and the accepted residual.