chore: back-merge main into dev (ds-shots publisher hotfix, TASK-22044) - #2921
Merged
Merged
Conversation
The API stopped selecting the SendLink `events` relation post-ledger-collapse, so `claimLinkData.events` is now always undefined. Claim.tsx dereferenced `events[0]` with the optional chain on the wrong side of the index, throwing "Cannot read properties of undefined (reading '0')" inside a render-phase useMemo — which Next.js surfaces as "Application error: a client-side exception has occurred" (Sentry PEANUT-UI-SJ0). Guard the array itself and mark `events` optional on the SendLink type so the compiler catches the next orphaned access.
…ts-crash fix(claim): stop claim page crashing on links with no events relation
workflow_run workflows fire only from the default branch, so the publisher that peanut-ui#2897 landed on dev is inert until a release. This carries the three publisher files to main unchanged — the workflow, the render/validate script it runs from the main checkout, and its test — which turns the sticky visual-diff comment on for every PR immediately. The ds-shots job side already runs on dev-based PRs from dev's tests.yml and is already uploading report artifacts. Files are byte-identical to dev (merged #2897, ten Chip review rounds, final round clean), so the next dev -> main release merges over this with no conflict and no back-merge debt. TASK-22044
CodeQL js/file-system-race on the size gate: the file could change between statSync and readFileSync. A single readSync capped at MAX_BYTES+1 removes the race and also stops a hostile oversized file from ever being fully read into memory. Behavior unchanged; oversized still rejects with exit 1. TASK-22044
Chip main-mode MAJOR: SHA-keyed concurrency groups let an older-head publisher stall past its bind check and PATCH stale output over a newer head's comment — different SHAs, different groups, no mutual exclusion. Keyed by head branch instead: every writer of a given PR's comment shares the group (report-path runs come from the PR's branch; the stale path only touches the run's branch), and execution order does not matter because a late older-head run fails the head binding. Also from review: persist-credentials false on the checkout (no git after it), distinct extraction output name (SC2094 audit nag), deploy note updated now the file is live on main, and the non-finite-percent test now actually reaches Number.isFinite (JSON.stringify turned NaN into null; JSON's real path to Infinity is a 1e999 literal). TASK-22044
Chip's filed finding: the SHA-only bind accepted a writer from another branch parked on the same commit — harmless content (same tree) but outside the branch concurrency group that serializes comment updates. The bind now requires head.sha AND head.ref to match the run, so every accepted writer is in the group. Also from CodeRabbit: the concurrency group is repo-qualified (a fork branch sharing a name never queues in an origin branch's group) and the queue comment states the real cap (100 pending, overflow cancelled) instead of claiming 'every'. TASK-22044
Chip main-mode MAJOR: sha+ref binding still accepted a sibling PR on the same branch and commit as the artifact-named target. When the workflow_run payload supplies pull_requests, the artifact's number now has to be in it; an empty payload falls back to the sha+ref bind, kept because the array is documented-unreliable and a silent publisher is worse than a same-tree sibling misdirection (same author, validated content, same serialization group). TASK-22044
Chip main-mode MAJOR with data: 618 of 700 recent pull_request Tests runs had an empty workflow_run.pull_requests, so the membership check almost never fired and the sha+ref fallback still accepted a sibling PR on the same branch+commit. The empty-list path now resolves open PRs for the run's branch+commit itself and proceeds only when exactly one exists and equals the artifact's number. A branch holding two sibling PRs posts nothing while the event omits the list — ambiguity is indistinguishable from misdirection, and the job summary remains the visible diff for that rare configuration. TASK-22044
CodeRabbit major, and sharper than it looks: commits/<sha>/pulls lists every PR whose branch CONTAINS the commit — a dev-tip commit sits in dozens of rebased branches — and the jq head filter runs client-side after pagination. Page one alone can miss the real head match, making the exactly-one bind see zero candidates and silently never post, and the stale path skip legit clears. Both calls now --paginate with per_page=100. TASK-22044
Chip main-mode MAJOR: with the event list empty, an outsider could point a same-named fork branch at a public commit, open a PR, and force the exactly-one check to count 2 — muting the legitimate publisher. All three PR lookups now require head.repo.full_name to be this repo (null-safe for deleted fork repos): the fallback candidates, the stale-path candidates, and — one gap beyond the finding — the direct pulls/<n> bind, which only compared sha+ref and would have let a fork PR with a copied branch name pass outside the empty-list path. TASK-22044
…main-22044 hotfix: activate the ds-shots PR comment publisher (TASK-22044)
Brings the merged ds-shots comment-publisher hotfix (#2912) — the review-hardened workflow, renderer, and tests — plus the claim-crash hotfix (#2889) back to dev. The add/add conflicts on the three publisher files resolve to main's versions, which supersede the originals #2897 left on dev; the closed mirror PR #2914 carried the same bytes and is no longer needed. TASK-22044
kushagrasarathe
requested review from
0xkkonrad,
Hugo0,
abalinda,
innolope-dev and
jjramirezn
as code owners
September 2, 2026 08:58
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
Code-analysis diffPainscore total: 7081.84 → 7081.84 (0) |
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean review. The back-merge restores the review-hardened ds-shots publisher on dev without introducing an actionable correctness, security, adversarial, or maintainability defect.
Checked clean
- Confirmed the detached worktree HEAD, supplied base SHA, and merge base; the three-file payload is byte-identical to the version merged on main in #2912.
- Reviewed the workflow write-token boundary, trusted default-branch checkout, same-repository gate, SHA/branch/PR binding, fork exclusion, pagination, and ambiguous sibling-PR handling.
- Exercised concrete stale-run and concurrent-writer scenarios against the branch-scoped queue, bind recheck, marker guard, fallback, and sticky-comment update paths.
- Reviewed bounded artifact extraction and JSON reading, schema and finite-number validation, markdown injection constraints, and output size caps.
- Exact-head CI passed unit, typecheck, eslint, format, analyze, ds-lint, provenance, and ci-success checks; ds-shots was skipped because this diff cannot alter rendered fixtures. A local targeted Jest run was unavailable because dependencies are not installed in the detached worktree.
- No product or API contract changed, so no product, provider, production, or sibling-repository context was needed.
Second opinion: did not run — the model did not answer in time. This review is one reviewer short.
Third opinion: did not run — claude-failed(1): Warning: no stdin data received in 3s, proceeding without it. If piping from a slow command, redirect stdin explicitly: < /dev/null to skip, or wait longer.. This review is one reviewer short.
Exact head: 6ae445a507dc · Context: repo, ci · Took 17m
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standard main → dev back-merge after the #2912 hotfix.
Brings to dev:
Gates run locally on the merged tree: typecheck, full jest suite, prettier — all green.
Task: TASK-22044
Screenshots: N/A (back-merge, no new visible change).