Repository navigation
docs(cla): the recheck comment is inert here, and the write fails after a 200 - #2792
Conversation
Coverage Gate✅ No Python changes — nothing to gate (bar: 95% on changed lines). |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved exact head 305109e40794d2c7354a4795c8ec94f0384196a7.
This is a focused docs-only correction. The revised text distinguishes webhook delivery acknowledgement from the later SHA-bound status write, makes the repository-specific reason the @cla-assistant check comment is inert explicit, preserves close+reopen as the practical recovery, and correctly leaves removal/rework of .github/workflows/cla-recheck-on-push.yml to #2791 rather than bundling it here.
Validation:
- full one-file diff plus the current
cla-recheck-on-push.ymlcaller inspected - hardcoded-path review: examples remain portable (
<owner>/<repo>/<id>), with no host-specific path added - all completed exact-head hosted gates pass, including Coverage and the docs/path/static lints; CI was still running
I could not independently query the private repository webhook-delivery log from this environment, so the 12-delivery population is accepted as author-supplied evidence rather than re-verified here. The patch does not overstate a root cause for the missing statuses; it limits the claim to receipt/acknowledgement followed by an absent status.
Worst-case disruption: stale operator guidance could encourage repeated inert comments; this change ends that loop and keeps the existing recoverable close+reopen path. No runtime or state-format impact.
Merge readiness: approved; code-review ready, with final merge still gated on CI, CLA, and the repository’s two-maintainer approval requirement.
Reviewed by Qingyun's Personal Codex.
…er a 200 Two statements in the CLA section are contradicted by the webhook delivery log. `@cla-assistant check` is described as unreliable. It cannot work at all: the CLA webhook subscribes to `merge_group, pull_request`, so `issue_comment` never reaches the service. "Unreliable" invites a retry; "inert" ends it. Records that `reopened` acks at 200 while `closed` returns 204, so the reopen is the half that works, and points at the open issue for the workflow rather than presuming its fix. The delivery model said the event's delivery can fail. Measured: 12 `pull_request/synchronize` deliveries, all HTTP 200 `'OK - Will be working on it'`, 9 with a status written and 3 without (#2342 #2785 #2790). Delivery succeeds and is acknowledged; the write fails afterward. That also explains why author email, committer identity, push recency and button-vs-local push all die against the full population — the discriminator is not in the request. Adds the reproduce command, including that `&page=N` is silently ignored on the deliveries endpoint (cursor-paginated) and returns an empty list that reads as an empty log. Stand: Echo Act IV Pro
305109e to
a0f35fc
Compare
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved exact head a0f35fc9752e4a4e9681d2d4b7c08b54511d28c0.
No blocking findings. The new head expands the same docs-only CLA guidance reviewed earlier: it now states the narrower observed model (delivery is acknowledged, the status write can fail afterward), explains why @cla-assistant check is inert for this repo because the hook is not subscribed to issue_comment, preserves close+reopen as the practical retry, and keeps the workflow removal/rework decision scoped to #2791. I still cannot independently query the private webhook delivery log here, so the 12-delivery population remains accepted as author-supplied evidence; the doc is careful not to claim an internal CLA-Assistant root cause.
Focused validation in an isolated /private/tmp checkout:
git diff --check origin/main...HEADpython3 scripts/gen-src-map.py --checkgit diff origin/main...HEAD | bash scripts/review-checks.sh
Hosted docs/static/coverage/CLA checks visible on the new head were green; tsc + tests (clean install) was still running at metadata fetch, so final merge remains gated on that completing green and the repository approval policy.
Reviewed by Qingyun's Personal Codex.
bassilkhilo-ag2
left a comment
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Docs-only diff to CONTRIBUTING.md revising two claims in the CLA-recovery section (@cla-assistant check is inert vs merely unreliable; delivery acks 200 but the write can silently fail after). Confirmed the removed text matches the pre-PR file, and open issue #2791 matches the PR's stated scope-limiting cross-reference. All fenced code blocks balanced, no hardcoded host paths in added lines. CI shows one red job (tsc + tests) but it's an unrelated pre-existing flake — the failing assertion is in a different unrelated test file, and a concurrent unrelated PR is failing the same CI window on a different test, which a docs-only markdown diff can't cause. One caveat: I couldn't independently re-derive the webhook event list myself (no admin:repo_hook scope on this token), so the core empirical claim is corroborated via the linked issue and internal consistency rather than directly re-verified — noting that rather than claiming full verification I didn't do.
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approval signal for refreshed head 52b1af4a755e5993b57c143dc8242768f95e9b79.
No blocking findings. This remains a docs-only CLA recovery correction after merging current main: it distinguishes webhook delivery acknowledgement from the later status-write failure, explains why @cla-assistant check is inert in this repo, and keeps the workflow-removal decision scoped elsewhere.
Validation:
git diff --check origin/main...HEADpython3 scripts/gen-src-map.py --check- Hosted checks are green, including
tsc + tests (clean install).
Reviewed by Qingyun's Personal Codex.
|
@cla-assistant check |
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved exact head cda54f9.
This is a merge-only refresh from 52b1af4 onto current main e7db805. CONTRIBUTING.md is byte-for-byte unchanged from the previously approved topic head, and the merge delta does not touch it. I re-read the full CLA guidance and the referenced recheck workflow; review threads are clear.
Current-head source-map, diff-hygiene, and REVIEW path checks pass. Fast hosted checks and CLA are green; clean-install CI was still running. Worst-case disruption is misleading recovery guidance, and the unchanged text still distinguishes absent versus failing status and directs operators to the effective reopen path. Code-ready; normal merge gates remain.
What
Two statements in
CONTRIBUTING.md's CLA section are contradicted by the webhook delivery log. Docs only.Why — evidence, not description
1.
@cla-assistant checkis inert here, not unreliableThe section said "do not rely on" it, which is right about the behaviour and silent about the cause. The cause is that it cannot arrive:
issue_commentis not subscribed, and that is the only CLA hook on the repo. The distinction is operational: "unreliable" invites a retry, "inert" ends it. Three separate retries this week — the recheck workflow's automatic comments, a manual one from a human account on #2763, and one on #2785 — could not have reached the service at all.2. Delivery succeeds; the write fails after the acknowledgement
The section's model was that delivery can fail. Measured 2026-08-11 — 12
pull_request/synchronizedeliveries, each mapped to its PR through the delivery payload, cross-referenced against whether that exact head carries a status:Identical request, identical acknowledgement, different outcome. That also explains why every request-side hypothesis dies against the full population — author email, committer identity, push recency, update-branch-button vs local push were each tested and each has PRs on both sides. The discriminator is not in the request.
Reproduce:
&page=Nis silently ignored on that endpoint (cursor-paginated) and returns an empty list that reads exactly like an empty log — that produced a false "0 deliveries" before I probedper_pageon its own, so the note is in the doc.Scope
Deliberately not touching
:213's description ofcla-recheck-on-push.yml. That workflow is inert for the same reason, but remove-versus-rework is a maintainer call tracked in #2791, and the doc line should follow whichever is picked. This PR only adds a parenthetical pointing at it.What is NOT claimed
:207's close+reopen was already correct. This adds why it works (reopenedacks at 200;closedreturns 204 and is discarded).🤖 Generated with Claude Code