UN-4523 [MISC] CLA check action to replace cla-assistant.io by ritwik-g · Pull Request #1 · Zipstack/cla-action

ritwik-g · 2026-10-27T06:34:04Z

What

A GitHub Action that enforces the Zipstack CLA on pull requests. It replaces cla-assistant.io, which has had repeated outages that left the required license/cla check stuck and blocked Zipstack/unstract PRs. Jira: UN-4523.

Checks every commit author on the PR, plus the PR author. Authors who have signed, bots and *[bot] accounts, and Zipstack org members pass. A PR with more commits than GitHub lists (250) fails rather than passing on a partial list.
Contributors sign by posting a fixed phrase as a PR comment. Every run reads all the PR's comments for signatures, so a sign comment whose run the concurrency group cancelled is picked up by the next run. Edited or hidden comments never sign. The signature is recorded by GitHub user id in signatures.json in the private Zipstack/cla-signatures repo. Writes carry the blob sha and retry on conflict, so two signatures at once can't be lost.
Reports the outcome as the license/cla commit status. The unstract ruleset already requires that name without pinning it to the cla-assistant app, so branch protection needs no change.
Keeps one PR comment per PR, edited as things change.
Commits made with Claude Code are authored by the claude account. They're attributed to the PR author rather than allowlisted, because allowlisting would let anyone skip the CLA by having the agent make every commit.
scripts/import-cla-assistant.js converts cla-assistant.io's JSON export into this format.
Uses pull_request_target so PRs from forks get the signatures token. It never checks out PR code.
No dependencies: built-in fetch and node:test.

Testing

npm test: 30 tests. Mutation-checked guards (each mutant, applied to a throwaway copy, fails at least one test): only a commit author or the PR author can sign; the PR author is always checked; a PR over the 250-commit listing limit fails; signatures are read from all PR comments, not just the triggering one; an edited or hidden comment never signs, and does not stop a later fresh sign comment from signing; signed_at is the sign comment's time; a PR whose commit count matches its listed commits is not treated as truncated; a retried write re-reads the file so a concurrent signature survives; a 422 on a first-file race is retried; pagination follows every page; the write retry is bounded; agent commits are attributed to the PR author.
Known limitations accepted for now are listed in the README.
Read-only dry run against live Zipstack/unstract PRs using real GitHub reads, with writes logged instead of sent:
- #2373 and #2332 (outside contributors) fail and ask the author to sign.
- #2378 (all commits by claude, PR by an org member) passes. cla-assistant leaves that PR at "not signed".

Not in this PR

The GitHub App, the org variable and secret, and the import of existing signers.
Adding the workflow to unstract and unlinking cla-assistant.io (cutover).

🤖 Generated with Claude Code

Checks every PR commit author against a signatures file in a private repo, records signatures posted as a PR comment, and reports the result as the license/cla commit status so existing branch protection keeps working. Includes an importer for the cla-assistant.io JSON export. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 1 (review-deep, INITIAL)

Round tip: a1b59f25b7baca6a0e934015d16167d313829914 · delta b5d8188..a1b59f2 (whole PR: merge base → tip) · 16 files / 1088 lines, count verified against GitHub · base fetch: ok.
Why review-deep: the PR carries an auth/token and attack surface (pull_request_target, signatures token, the CLA authorization decision) — a review-shallow deferral signal; shallow was not attempted.
Signals: lenses 4, 6, 11, 13, 14, 16 raised (action.yml/examples/cla.yml tokens and concurrency, package.json, test/*, README.md).
Agents: code-reviewer (1,2,3,9,10,15), silent-failure-hunter (3), pr-test-analyzer (13), comment-analyzer (16), type-design-analyzer (7), code-reviewer whole-diff cold read, code-reviewer infra group (5,6,8,11,19); security deep-dive (1 find + 5 filter sub-tasks).

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 3 · Medium: 11 · Low: 8 · Lenses run: 19/19 (12, 18, 19 N/A)

Findings

[High] [Lens 6, 3, 11] — A comment arriving while a run is in progress cancels the pending run, losing a signature or a new head's status
- Location: examples/cla.yml:L18-L19 (with :L11-L12, :L24), src/main.js:L20-L28
- Failure mode: GitHub keeps at most one pending run per concurrency group; a newly queued run cancels the pending one. Every PR comment (including Greptile/CodeRabbit) joins cla-<n>. A pending sign-phrase run cancelled by any later comment is replaced by a run that returns "Nothing to do": the signature is never recorded and nobody is told. Same for a pending synchronize run: the new head never gets license/cla.
- Evidence: documented "any existing pending job or workflow in the same concurrency group will be canceled"; resolveTrigger returns null for other comments.
- Suggested fix: every run does the full evaluation, and signatures are recorded by scanning the PR's comments for the phrase rather than only from the triggering comment, so whichever run replaces a cancelled one does its work.
- Confidence: High (four agents independently).
- Verification: VERIFIED against documented concurrency semantics and the code path; not reproduced on live Actions.
[High] [Lens 4] — PR author is never checked, so spoofing a commit author email passes the CLA without signing
- Location: src/main.js:L56-L58, src/cla.js:L42-L57, src/cla.js:L75-L89
- Failure mode: commit.author is linked by unverified git author email. An outside fork contributor commits as 41898282+github-actions[bot]@users.noreply.github.com (or an org member's / prior signer's noreply address); every commit resolves to an exempt or signed identity and license/cla goes green. The PR author (the actual contributor) is only considered via attributeToPrAuthor for agent commits — the same bypass class the agent-attribution design closes, left open for every other identity.
- Suggested fix: always classify pr.user as a required party alongside commit authors.
- Confidence: High (security filter 7/10; found independently by the orchestrator).
- Verification: VERIFIED by reading the code: pr.user reaches classification only via attributeToPrAuthor.
[High] [Lens 3, 4] — PRs with more than 250 commits pass on their first 250 only
- Location: src/main.js:L56, src/github.js:L33-L41
- Failure mode: GET /pulls/{n}/commits returns at most 250 commits (documented); paginate stops on the short page without error. Authors only after commit 250 are never classified, so the status can be success — fails open.
- Suggested fix: when commits.length < pr.commits, set failure with a "too large to verify" description.
- Confidence: High. Verification: VERIFIED in code; the 250 cap is documented API behaviour.
[Medium] [Lens 13] — The concurrency test cannot detect a lost concurrent signature
- Location: test/fake-github.js:L36-L39, test/main.test.js:L153-L159
- Failure mode: the fake's injected 409 leaves the file unchanged, so a mutant that reuses its first-read data and only refreshes the sha (the overwrite this guard exists to prevent) passes 22/22. The PR body's "a concurrent signature isn't rewritten" mutation-check claim does not hold.
- Suggested fix: on conflict the fake adds a concurrent signature and bumps the sha; assert both survive.
- Verification: VERIFIED — mutant run by pr-test-analyzer.
[Medium] [Lens 13] — Fake returns 409 where GitHub returns 422 (PUT without sha to an existing file), so the 422 retry is untested — test/fake-github.js:L40-L42, src/store.js:L48. VERIFIED: removing || error.status === 422 passes 22/22.
[Medium] [Lens 13] — The real paginate loop is never exercised — src/github.js:L33-L41; the fake replaces it, and a first-page-only mutant passes 22/22.
[Medium] [Lens 13] — gh and signaturesGh are the same fake in tests, so a call routed through the wrong token is not caught — test/main.test.js:L34-L44.
[Medium] [Lens 13] — src/index.js input wiring is untested; inverting the exempt-org-members check passes — src/index.js:L9-L28.
[Medium] [Lens 13] — scripts/import-cla-assistant.js is untested and its export field names (revoked_at etc.) are unconfirmed against a real export — scripts/import-cla-assistant.js:L32-L63.
[Medium] [Lens 3, 8] — A run that throws posts no status and nothing on the PR; fetch has no timeout or retry — src/index.js:L45-L48, src/github.js:L23-L29. Fails closed (required check stays "Expected"), but a failed sign attempt in an issue_comment run is invisible on the PR.
[Medium] [Lens 3] — A 404 on the signatures file or org-membership lookup is read as "empty" / "not a member", so misconfiguration looks like a request to sign — src/store.js:L21, src/main.js:L92. Fails closed.
[Medium] [Lens 3] — A rejected sign attempt (commenter not a commit author) gets no feedback on the PR — src/main.js:L80-L82.
[Medium] [Lens 7] — Signature record carries no CLA version; importer and action write different record shapes — src/main.js:L70-L78, scripts/import-cla-assistant.js:L53-L61, src/store.js:L26-L28. Policy question: does a signature for document A count for B?
[Medium] [Lens 16] — README overstates how Claude Code commits are authored — README.md:L78, src/cla.js:L59-L60. A local Claude Code session commits as the human with noreply@anthropic.com only in a Co-Authored-By trailer (not checked); agent-logins applies only when the agent is the git author.
[Low] [Lens 3, 4] — Marker comment matched without checking its author — src/main.js:L33-L34. Fails closed; security filter 2/10.
[Low] [Lens 4] — Unlinked commit author name rendered unescaped as Markdown in the bot comment — src/cla.js:L119. Security filter 2/10.
[Low] [Lens 5, 9] — Contents API stops returning content once signatures.json passes 1 MB; every run then fails — src/store.js:L17-L18.
[Low] [Lens 3] — exempt-org-members only turns off for the exact string false — src/index.js:L25.
[Low] [Lens 7, 16] — Status description counts unlinked commits as "committers" — src/main.js:L119-L125.
[Low] [Lens 7] — signatures.json version written but never checked on read — src/store.js:L15-L19.
[Low] [Lens 16] — Minor prose mismatches: action.yml:L22 "Exact comment" (matching is normalised); src/store.js:L3-L6 omits 422 and the 5-attempt cap; README.md:L7 says every comment re-runs the check; importer message scripts/import-cla-assistant.js:L48 calls a renamed account "not found".
[Low] [Lens 13] — Agent-login case folding untested — src/cla.js:L64-L67.

Security deep-dive: done — 5 candidates, none at confidence >= 8 (email-spoof bypass 7/10, carried as #2 on the rubric's own severity; the other four 2–3/10).
boundary-sweep: 6 sinks traced, clean (run log: logins and counts; PR comment: logins and unlinked author name, never email; status description; signatures-file record: login/id/comment URL; ::error:: message: method, path, API body — no token).

Lens checklist

Spec & intent — Clean (PR-body mutation claim contradicted, see #4; title UN-4523 [MISC] …, repo states no convention)
Architectural fit — Clean
Correctness & edge cases — See findings #1, #3, #10, #11, #12, #15, #18
Security — See findings #2, #3, #15, #16 (orchestrator-assessed + security deep-dive)
Data integrity — See findings #17 (orchestrator + infra agent)
Concurrency — See findings #1 (orchestrator + infra agent)
API & contract — See findings #13, #19, #20 (consumer search by orchestrator/agent: license/cla consumer is the unstract ruleset only)
Reliability — See findings #10 (infra agent)
Performance & cost — Clean (a run per PR comment is a few API calls)
Observability — Clean
Operational safety — See findings #1 (infra agent; rollback fine, consumers pin @v1)
LLM/agent — N/A — no model calls, prompts or tools
Testing — See findings #4, #5, #6, #7, #8, #9, #22
Dependencies & build — Clean (zero dependencies, engines matches runs.using node24; orchestrator-assessed)
Code quality — Clean (no formatter/linter pinned in the repo, so no pinned gate to run)
Doc & comment accuracy — See findings #14, #21
Cross-document consistency — Clean (lens-17 merge sweep: only same-branch definitions in a new repo; orchestrator-assessed)
On-prem — N/A — no chart, on-prem values or on-prem build surface
Scaling & lifecycle — N/A — no worker pool, autoscaling or grace periods

Open questions for the author

Should a signature for one cla-document-url count for a repo configured with another (#13)?

Assumptions made

GitHub's documented one-pending-run-per-group concurrency semantics hold for issue_comment and pull_request_target alike.

cost: agents=13 wall=09:37 lenses-examined=16 of 19 files=16 lines=1088

ritwik-g · 2026-10-27T06:44:32Z

+    description: Branch of the signatures repository to read and write.
+    default: main
+  sign-phrase:
+    description: Exact comment a contributor posts to sign.


[Low] [Lens 16] — Minor prose mismatches: action.yml:L22 "Exact comment" (matching is normalised); src/store.js:L3-L6 omits 422 and the 5-attempt cap; README.md:L7 says every comment re-runs the check; importer message scripts/import-cla-assistant.js:L48 calls a renamed account "not found".

PARTIALLY RESOLVED in 392164d — README "How it works" rewritten; action.yml "Exact comment", the store.js header and the importer message are unchanged (wording only, below the fix bar).

ritwik-g · 2026-10-27T06:44:32Z

+  const agents = new Set(agentLogins.map((login) => login.toLowerCase()))
+  const result = new Map()
+  for (const user of users) {
+    const owner = agents.has(user.login.toLowerCase())


[Low] [Lens 13] — Agent-login case folding untested — src/cla.js:L64-L67.

…nd 1 - Check the PR author as well as commit authors. Commit authors are matched by email, which the committer sets, so attributing every commit to an exempt or signed account no longer passes the check. - Fail a PR whose commit count exceeds what GitHub lists (250) instead of passing on a partial list. - Run the full check on every PR comment and record signatures from all sign comments on the PR, so a sign comment whose run the concurrency group cancelled is recorded by the next run. - Make the fake GitHub behave like the real one on write conflicts (a concurrent writer adds a signature and moves the sha; a missing sha on an existing file gets 422), and test the real pagination loop. - README: describe the above, correct when Claude Code commits as `claude`, and list the known limitations accepted for now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 2 (review-deep, FOLLOWUP)

Round tip: 392164dfc567e448bac3c5280065e4d60bfdf518 · round delta a1b59f2..392164d (round-1 fixes) · 17 files in PR scope, base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which still carries the auth/token surface; shallow was not attempted.
Agents: code-reviewer (1,2,3,9,10,15), silent-failure-hunter (3), pr-test-analyzer (13), comment-analyzer (16), whole-diff cold read, code-reviewer infra group (5,6,8,11).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA a1b59f25b7baca6a0e934015d16167d313829914.

Scope change — NO

Verdict — REQUEST CHANGES

Summary (open, including unresolved prior findings) — Critical: 0 · High: 2 · Medium: 6 · Low: 6 · Lenses run: 19/19 (12, 18, 19 N/A)

Prior findings status

#1 [High] concurrency cancels pending runs → RESOLVED — every run reads all PR comments, src/main.js:L71-L97; test "a sign comment whose own run was cancelled…"
#2 [High] PR author never checked → RESOLVED — src/cla.js addPrAuthor, src/main.js:L63
#3 [High] 250-commit cap fails open → RESOLVED — src/main.js:L58-L61, status/comment on truncation (see new finding 24 on its test)
#4 [Medium] concurrency test vacuous → RESOLVED — fake now simulates a concurrent writer, test/fake-github.js
#5 [Medium] 422 untested → RESOLVED — test/main.test.js "two first signatures at once…"
#6 [Medium] paginate untested → RESOLVED — test/github.test.js
#7 [Medium] shared fake for both tokens → NOT RESOLVED — test gap, below floor; left open
#8 [Medium] src/index.js untested → NOT RESOLVED — test gap, below floor; left open
#9 [Medium] importer untested → NOT RESOLVED — test gap, below floor; left open
#10 [Medium] failed run posts no status → WAIVED — accepted in README.md "Known limitations"
#11 [Medium] 404 read as empty / non-member → WAIVED — README.md "Known limitations"
#12 [Medium] rejected sign gets no reply → WAIVED — README.md "Known limitations"
#13 [Medium] no CLA version → WAIVED — README.md "Known limitations" (policy question open)
#14 [Medium] Claude Code authorship overstated → RESOLVED — README.md coding-agents section, src/cla.js:L55-L58
#15–#20 [Low] → WAIVED — each listed in README.md "Known limitations"
#21 [Low] prose mismatches → PARTIALLY RESOLVED — README.md:L7 area rewritten; action.yml:L22 "Exact", src/store.js header and the importer message unchanged (wording, below floor)
#22 [Low] agent case folding untested → NOT RESOLVED — below floor

New findings

[High] #23 [Lens 4, 3, 13] — Edited comments now sign, and "edited comments do not sign" no longer tests that
- Location: src/main.js:L78-L95, test/main.test.js:L210-L213
- Failure mode: every run now reads each PR comment's current body. A comment edited into the sign phrase — by its author, or by anyone with edit rights on others' comments (repo writers, other workflows' tokens, installed apps) — records a CLA signature for the comment's author on the next run, with comment_url pointing at text that author may never have written. The test only asserts resolveTrigger ignores the edited event, so it passes while an edited comment signs.
- Suggested fix: only count comments never edited (updated_at === created_at); test it through handle.
- Confidence: High (three agents; reproduced by pr-test-analyzer in a scratch test).
- Verification: VERIFIED — scratch test against HEAD wrote a signature from an edited comment.
[High] #24 [Lens 13] — The fake PR has no commits field, so a truncation guard that blocks every PR passes the suite
- Location: test/main.test.js:L29-L31, guarding src/main.js:L61
- Failure mode: pr.commits is undefined in every test but one, so commits.length < pr.commits is false for a reason production never has. Mutating < to <= (which would fail every real PR with "Split it into smaller PRs") passes 29/29.
- Suggested fix: the fake's PR GET reports commits as the length of the commit list it serves, as GitHub does.
- Verification: VERIFIED — mutant run by pr-test-analyzer.
[Medium] #25 [Lens 3, 5] — signed_at is the recording run's time, not the sign comment's
- Location: src/main.js:L90
- Failure mode: a sign comment can now be recorded days later (its run was cancelled, or the commenter became required later); signed_at then misstates when they agreed, while comment_url points at the earlier comment.
- Suggested fix: signed_at: comment.created_at.
- Verification: VERIFIED by reading.
[Medium] #26 [Lens 3] — A PR opened by an agent-logins account (type User) must now sign, which it never can
- Location: src/main.js:L63, src/cla.js:L75-L78
- Failure mode: fails closed — the check stays failing. Whether agents ever open PRs as the plain claude user is unverified.
- Suggested fix: policy decision; at minimum document it.
- Verification: UNVERIFIED — can a PR here be authored by the claude user account?
[Medium] #27 [Lens 16] — Known-limitations intro says each item fails closed; the exempt-org-members item fails open — README.md:L88, README.md:L96.
[Low] #28 [Lens 16] — "committer" wording now also covers a PR author with no commits — src/cla.js:L98, src/main.js:L143, action.yml:L2.
[Low] #29 [Lens 16] — Truncation-only comment still suggests recheck — src/cla.js:L134.
[Low] #30 [Lens 16] — "no … retry beyond the signatures write" overstates it (only sha conflicts retry) — README.md:L90.
[Low] #31 [Lens 8] — Every PR comment now posts a status; GitHub caps 1000 statuses per sha and context — src/main.js:L136-L145. Needs ~1000 comments without a push.

Security deep-dive: done — re-run on this round (1 find + 3 filters; two repeat candidates carry round-1 filter scores): no findings at confidence >= 8. Edited-comment signing scored 3/10 as a vulnerability (needs a repo collaborator) and is carried as 23 on the rubric's own severity, since it regresses what the 'edited comments do not sign' test was written to guarantee.
boundary-sweep: 3 sinks traced in the delta, clean (new log line for rejected signers: login and PR number; status description: commit counts; PR comment: fixed truncation text).

Lens checklist

Spec & intent — Clean (PR body matches the three round-1 fixes)
Architectural fit — Clean
Correctness & edge cases — See findings #23, #25, #26
Security — See findings #23 (orchestrator-assessed)
Data integrity — See findings #25
Concurrency — Clean (infra agent: per-PR serialisation, replacement runs now do full work)
API & contract — Clean (status/comment text only; orchestrator-assessed)
Reliability — See findings #31
Performance & cost — Clean
Observability — Clean
Operational safety — Clean
LLM/agent — N/A — no model calls
Testing — See findings #23, #24
Dependencies & build — Clean (none added; orchestrator-assessed)
Code quality — Clean (no formatter or linter pinned in the repo)
Doc & comment accuracy — See findings #27, #28, #29, #30
Cross-document consistency — Clean (README, PR body and code comments agree on the new rules; orchestrator-assessed)
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

Open questions / assumptions

Can a PR on the target repos be opened by the plain claude user account (26)?

cost: agents=10 wall=03:44 lenses-examined=16 of 19 files=17 lines=0

ritwik-g · 2026-10-27T06:56:42Z

+
+function renderComment({ unsigned, unlinked, truncated = false, claUrl, phrase }) {
+  if (unsigned.length === 0 && unlinked.length === 0 && !truncated) {
+    return `${COMMENT_MARKER}\n✅ All committers have signed the [CLA](${claUrl}). Thank you!`


[Low] #28 [Lens 16] — "committer" wording now also covers a PR author with no commits — src/cla.js:L98, src/main.js:L143, action.yml:L2.

ritwik-g · 2026-10-27T06:56:42Z

+      '',
+    )
+  }
+  lines.push('<sub>Already signed? Comment `recheck` to run the check again.</sub>')


[Low] #29 [Lens 16] — Truncation-only comment still suggests recheck — src/cla.js:L134.

ritwik-g · 2026-10-27T06:56:42Z

+  await gh.request('POST', `/repos/${repository}/statuses/${pr.head.sha}`, {
+    state: passed ? 'success' : 'failure',
+    context: inputs.statusContext,
+    description: passed
+      ? 'All committers have signed the CLA.'
+      : truncated
+        ? `PR has ${pr.commits} commits; only ${commits.length} can be checked. Split it into smaller PRs.`
+        : `${pending} committer${pending === 1 ? '' : 's'} still need${pending === 1 ? 's' : ''} to sign the CLA.`,
+    target_url: comment ? comment.html_url : inputs.claDocumentUrl,
+  })


[Low] #31 [Lens 8] — Every PR comment now posts a status; GitHub caps 1000 statuses per sha and context — src/main.js:L136-L145. Needs ~1000 comments without a push.

Self-review round 2: - Reading every PR comment let a comment edited into the sign phrase record a signature, and anyone with write access can edit others' comments. Edited comments no longer sign; the test now drives that through a later run instead of only checking resolveTrigger. - signed_at is the sign comment's created_at rather than the time of whichever run recorded it. - The fake PR reports its commit count as GitHub does, so the truncation guard is exercised at the boundary. - README known limitations: drop the "each one fails closed" framing, which the exempt-org-members item contradicts; note that a PR opened by an agent-logins account can't pass and that edited comments don't sign; correct which write is retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 3 (review-deep, FOLLOWUP)

Round tip: f336521204be7d48b732c04b1f81951e0c64db58 · round delta 392164d..f336521 (round-2 fixes) · base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which carries the auth/token surface; shallow was not attempted.
Agents: code-reviewer (1,2,3,9,10,15), pr-test-analyzer (13), comment-analyzer (16), code-reviewer (5); security deep-dive (1 find + 4 filters).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA 392164dfc567e448bac3c5280065e4d60bfdf518.

Scope change — NO

Verdict — REQUEST CHANGES

Summary (open, including unresolved prior findings) — Critical: 0 · High: 0 · Medium: 7 · Low: 8 · Lenses run: 19/19 (12, 18, 19 N/A)

Prior findings status

#23 [High] edited comments sign → RESOLVED — src/main.js:L82, test "edited comments do not sign" now runs a later comment through handle
#24 [High] fake PR lacks commits → RESOLVED — test/fake-github.js:L29-L32; new boundary test; <→<= mutant fails 8 tests
#25 [Medium] signed_at is run time → RESOLVED — src/main.js:L93 (see new #36 on its format)
#26 [Medium] agent-login PR author must sign → WAIVED — README.md "Known limitations" (see new #35 on its wording)
#27 [Medium] "fails closed" framing → RESOLVED — framing sentence removed
#28 [Low] "committer" wording → NOT RESOLVED — below floor
#29 [Low] truncation footer suggests recheck → NOT RESOLVED — below floor
#30 [Low] retry wording → PARTIALLY RESOLVED — reworded; still narrower than the code (new #37)
#31 [Low] 1000-status cap → NOT RESOLVED — below floor
#7 [Medium] shared fake for both tokens → NOT RESOLVED — below floor, unchanged
#8 [Medium] src/index.js untested → NOT RESOLVED — below floor, unchanged
#9 [Medium] importer untested → NOT RESOLVED — below floor, unchanged
#21 [Low] prose mismatches → PARTIALLY RESOLVED — unchanged since round 2
#22 [Low] agent case folding untested → NOT RESOLVED — unchanged

New findings

[Medium] #32 [Lens 3, 16] — A hidden (minimized) sign comment is treated as edited and never signs
- Location: src/main.js:L73-L82, README.md:L97
- Failure mode: hiding a comment moves updated_at without an edit, so a sign comment a maintainer hid before any run recorded it is skipped for good. Fails closed; the prose says "edited" only.
- Suggested fix: say "edited or hidden" in the comment and README.
- Verification: VERIFIED — GraphQL over cli/cli PR comments: 4 of 26 had updatedAt != createdAt with lastEditedAt: null, all isMinimized: true.
[Medium] #33 [Lens 10] — A sign comment skipped as edited leaves no log line
- Location: src/main.js:L82-L83
- Failure mode: README says rejected signatures are explained in the workflow log; this skip is silent, unlike the not-required skip.
- Suggested fix: test the phrase first, then log and skip an edited one without consuming the signer's slot.
- Verification: VERIFIED by reading.
[Medium] #34 [Lens 13] — Re-signing after an edited comment is untested
- Location: test/main.test.js:L223-L232, src/main.js:L82-L84, README.md:L96
- Failure mode: moving handled.add before the edited check locks a signer out of the PR for good; 30/30 still pass.
- Suggested fix: post a fresh phrase after the edited one and assert it signs.
- Verification: VERIFIED — mutant run by pr-test-analyzer.
[Medium] #35 [Lens 16] — README says a PR opened by an agent-logins account "can't pass"; the code would pass it if that account posted the phrase
- Location: README.md:L95
- Suggested fix: state what the code does (the account is the PR author and would have to sign itself) without implying enforcement. Security filter rated the account-signs scenario 2/10 (sessions cannot comment as claude).
[Low] #36 [Lens 5] — signed_at now mixes …Z (GitHub) and ….000Z (importer) formats — src/main.js:L93, scripts/import-cla-assistant.js:L56.
[Low] #37 [Lens 16] — "only a signatures write that loses a race is retried" is narrower than the code (any 409/422) — README.md:L88.

Security deep-dive: done — re-run this round: 5 candidates (agent account signing 2/10, app-posted sign comment 2/10, importer rename squat 2/10, marker comment owner — already filtered 2/10 in rounds 1–2, same-second edit 2/10); no findings at confidence >= 8.
boundary-sweep: 1 sink in the delta traced, clean (signature record signed_at: GitHub timestamp).

Lens checklist

Spec & intent — Clean
Architectural fit — Clean
Correctness & edge cases — See findings #32
Security — Clean (deep-dive re-run found nothing at the bar; orchestrator-assessed)
Data integrity — See findings #36
Concurrency — Clean (no concurrency change in this delta; orchestrator-assessed)
API & contract — Clean (record shape unchanged; orchestrator-assessed)
Reliability — Clean (no change; orchestrator-assessed)
Performance & cost — Clean
Observability — See findings #33
Operational safety — Clean (no workflow change; orchestrator-assessed)
LLM/agent — N/A — no model calls
Testing — See findings #34
Dependencies & build — Clean (none added)
Code quality — Clean (no formatter or linter pinned)
Doc & comment accuracy — See findings #35, #37
Cross-document consistency — Clean (README, PR body and code comments agree; orchestrator-assessed)
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

cost: agents=9 wall=05:54 lenses-examined=16 of 19 files=17 lines=0

Self-review round 3: - An edited (or hidden) sign comment is now logged when skipped, and does not use up the signer's slot, so posting the phrase again signs. Tested, including the reordering that would lock a signer out. - signed_at is normalised with toISOString, so records match the importer's format; the test fixture now uses GitHub's timestamp format. - Code comment and README say hidden comments are skipped too, which the updated_at check implies. - README: the agent-logins PR limitation no longer claims the code stops an agent account from signing; the retry note says which responses are retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 4 (review-deep, FOLLOWUP)

Round tip: a253251b7ab877e4f050364a710ad0debf7a21a5 · round delta f336521..a253251 (round-3 fixes) · base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which carries the auth/token surface; shallow was not attempted.
Agents: code-reviewer (1,2,3,9,10,15), pr-test-analyzer (13), comment-analyzer (16), code-reviewer (5); security deep-dive (1 find + 1 filter).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA f336521204be7d48b732c04b1f81951e0c64db58.

Scope change — NO

Verdict — APPROVE WITH NITS

Summary (open, including unresolved prior findings) — Critical: 0 · High: 0 · Medium: 5 · Low: 8 · Lenses run: 19/19 (12, 18, 19 N/A)

Prior findings status

#32 [Medium] hidden comment treated as edited → RESOLVED — src/main.js:L73-L76, README.md:L96 say "edited or hidden" (hiding verified to bump updated_at on live comments)
#33 [Medium] skipped edited comment not logged → RESOLVED — src/main.js:L84-L88 (see new #40)
#34 [Medium] re-sign after edit untested → RESOLVED — test/main.test.js:L239-L244; reorder mutant fails it
#35 [Medium] agent-logins "can't pass" claim → RESOLVED — README.md:L95 reworded (see new #41)
#36 [Low] mixed signed_at formats → RESOLVED — src/main.js:L98; fixture now uses GitHub's format
#37 [Low] retry wording → RESOLVED — README.md:L88
#30 [Low] retry wording (round 2) → RESOLVED — superseded by #37's fix
#7 [Medium] shared fake for both tokens → NOT RESOLVED — below floor
#8 [Medium] src/index.js untested → NOT RESOLVED — below floor
#9 [Medium] importer untested → NOT RESOLVED — below floor
#21 [Low] prose mismatches → PARTIALLY RESOLVED — unchanged
#22 [Low] agent case folding untested → NOT RESOLVED — below floor
#28 [Low] "committer" wording → NOT RESOLVED — below floor
#29 [Low] truncation footer suggests recheck → NOT RESOLVED — below floor
#31 [Low] 1000-status cap → NOT RESOLVED — below floor

New findings

[Medium] #38 [Lens 16] — README says a hidden comment "never signs", but hiding after the signature was recorded does not remove it
- Location: README.md:L96
- Failure mode: a maintainer who hides a sign comment intending to refuse it leaves the signature (recorded seconds after posting) in place.
- Suggested fix: say that hiding or editing after the signature is recorded does not remove it.
- Verification: VERIFIED — no revocation path in src/.
[Medium] #39 [Lens 13] — The "ignored edited comment" log line, which the README promises, is untested
- Location: src/main.js:L84-L88, test/main.test.js:L43
- Verification: VERIFIED — deleting the log line passes 30/30.
[Low] #40 [Lens 10, 16] — The ignored-comment log tells people to post again even when they have already signed or cannot sign — src/main.js:L86; the edit check runs before the already-signed and required checks.
[Low] #41 [Lens 16] — The agent-logins limitation omits that an exempt (org member or allowlisted) agent account passes — README.md:L95.
[Low] #42 [Lens 13] — The edited-comment fixture's editedAt still has milliseconds — test/main.test.js:L234.

Security deep-dive: done — re-run this round: 5 candidates (importer login re-registration, head-sha race, same-second edit, marker comment owner — each filtered at 2/10 in earlier rounds; Bot-type PR author exempt 2/10 this round); no findings at confidence >= 8.
boundary-sweep: 1 sink in the delta traced, clean (new log line: public comment URL and login).

Lens checklist

Spec & intent — Clean
Architectural fit — Clean
Correctness & edge cases — Clean
Security — Clean (deep-dive re-run found nothing at the bar; orchestrator-assessed)
Data integrity — Clean (agent: signed_at format now matches the importer)
Concurrency — Clean (no concurrency change in this delta; orchestrator-assessed)
API & contract — Clean (record shape unchanged; orchestrator-assessed)
Reliability — Clean (no change; orchestrator-assessed)
Performance & cost — Clean
Observability — See findings #40
Operational safety — Clean (no workflow change; orchestrator-assessed)
LLM/agent — N/A — no model calls
Testing — See findings #39, #42
Dependencies & build — Clean (none added)
Code quality — Clean (no formatter or linter pinned)
Doc & comment accuracy — See findings #38, #41
Cross-document consistency — Clean (README, PR body and code comments agree; orchestrator-assessed)
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

cost: agents=6 wall=03:46 lenses-examined=16 of 19 files=17 lines=0

Self-review round 4: - The signature loop checks "already signed" and "may sign" before the edited-comment check, so the ignored-comment log no longer tells someone who has signed, or cannot sign, to post the phrase again. The log line is now asserted, including that it stops once the person has signed. - README: hiding or editing a comment after its signature was recorded does not remove the signature; an exempt agent account passes as PR author. - Edited-comment fixture uses GitHub's timestamp format. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 5 (review-deep, FOLLOWUP)

Round tip: 01097588391c54720d02392161c6958d8aaf1c7a · round delta a253251..0109758 (round-4 fixes) · base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which carries the auth/token surface; shallow was not attempted.
Agents: code-reviewer (1,2,3,9,10,15), pr-test-analyzer (13), comment-analyzer (16); security deep-dive (1 find; no new candidate needed filtering).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA a253251b7ab877e4f050364a710ad0debf7a21a5.

Scope change — NO

Verdict — APPROVE WITH NITS

Summary (open, including unresolved prior findings) — Critical: 0 · High: 0 · Medium: 4 · Low: 7 · Lenses run: 19/19 (12, 18, 19 N/A)

Prior findings status

#38 [Medium] hiding after recording does not revoke → RESOLVED — README.md:L96
#39 [Medium] ignored-comment log untested → RESOLVED — test/main.test.js:L231-L240; removing the log line fails it
#40 [Low] repost advice after signing → RESOLVED — src/main.js:L84-L98 reordered; asserted at test/main.test.js:L249-L252 (residual for exempt users: new #43)
#41 [Low] agent-logins exemption wording → RESOLVED — README.md:L95 (new #44 on its allowlist mention)
#42 [Low] fixture milliseconds → RESOLVED — test/main.test.js:L234
#7 [Medium] shared fake for both tokens → NOT RESOLVED — below floor
#8 [Medium] src/index.js untested → NOT RESOLVED — below floor
#9 [Medium] importer untested → NOT RESOLVED — below floor
#21 [Low] prose mismatches → PARTIALLY RESOLVED — unchanged
#22 [Low] agent case folding untested → NOT RESOLVED — below floor
#28 [Low] "committer" wording → NOT RESOLVED — below floor
#29 [Low] truncation footer suggests recheck → NOT RESOLVED — below floor
#31 [Low] 1000-status cap → NOT RESOLVED — below floor

New findings

[Medium] #45 [Lens 13] — Moving the not-required check ahead of the edited check is untested
- Location: src/main.js:L88-L97, test/main.test.js:L228-L253
- Failure mode: order signed → edited → required survives 30/30; a non-required user whose edited comment matches the phrase would be told to post again. Log output only.
- Suggested fix: a non-required user's edited sign comment logs "not a commit author" and not the repost advice.
- Verification: VERIFIED — mutant run by pr-test-analyzer.
[Low] #43 [Lens 3, 10] — The repost advice still reaches exempt users (org members, allowlisted), for whom signing changes nothing — src/main.js:L94-L98. Log only; exemption is decided after the loop.
[Low] #44 [Lens 16] — The agent-logins limitation lists the allowlist as an exemption, which the README elsewhere warns against for agent accounts — README.md:L95, against README.md:L84.

Security deep-dive: done — re-run this round: 5 candidates, each a repeat of one filtered at 2–3/10 in earlier rounds (Bot PR author, head-sha race, email attribution, marker owner, same-second edit); no findings at confidence >= 8.
boundary-sweep: no new sinks in the delta (log text reordered only), clean.

Lens checklist

Spec & intent — Clean
Architectural fit — Clean
Correctness & edge cases — See findings #43
Security — Clean (deep-dive re-run found nothing at the bar; orchestrator-assessed)
Data integrity — Clean (no record change in this delta; orchestrator-assessed)
Concurrency — Clean (no change; orchestrator-assessed)
API & contract — Clean (no change; orchestrator-assessed)
Reliability — Clean (no change; orchestrator-assessed)
Performance & cost — Clean
Observability — See findings #43
Operational safety — Clean (no change; orchestrator-assessed)
LLM/agent — N/A — no model calls
Testing — See findings #45
Dependencies & build — Clean (none added)
Code quality — Clean (no formatter or linter pinned)
Doc & comment accuracy — See findings #44
Cross-document consistency — See findings #44
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

cost: agents=4 wall=03:16 lenses-examined=16 of 19 files=17 lines=0

Self-review round 5: - Test that someone who may not sign is told so, rather than told to post the phrase again, which pins the order of the checks in the signature loop. - The ignored-comment log advises posting again only "if a signature is still needed", since exempt users reach it too. - README no longer suggests the allowlist for an agent account, which the coding-agents section warns against. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 6 (review-deep, FOLLOWUP)

Round tip: 816742149eeb42b64ff113e071edf718b717fba8 · round delta 0109758..8167421 (round-5 fixes) · base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which carries the auth/token surface; shallow was not attempted.
Agents: code-reviewer (1,2,3,9,10,15), pr-test-analyzer (13), comment-analyzer (16); security deep-dive (1 find; no new candidate needed filtering).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA 01097588391c54720d02392161c6958d8aaf1c7a.

Scope change — NO

Verdict — APPROVE WITH NITS

Summary (open, including unresolved prior findings) — Critical: 0 · High: 0 · Medium: 4 · Low: 5 · Lenses run: 19/19 (12, 18, 19 N/A)

Prior findings status

#45 [Medium] check order untested → RESOLVED — test/main.test.js:L249-L254; the signed → edited → required mutant now fails it
#43 [Low] repost advice reaches exempt users → RESOLVED — log says "if a signature is still needed", src/main.js:L95
#44 [Low] allowlist suggested for agent accounts → RESOLVED — README.md:L95 (but see new #46)
#7 [Medium] shared fake for both tokens → NOT RESOLVED — below floor
#8 [Medium] src/index.js untested → NOT RESOLVED — below floor
#9 [Medium] importer untested → NOT RESOLVED — below floor
#21 [Low] prose mismatches → PARTIALLY RESOLVED — unchanged
#22 [Low] agent case folding untested → NOT RESOLVED — below floor
#28 [Low] "committer" wording → NOT RESOLVED — below floor
#29 [Low] truncation footer suggests recheck → NOT RESOLVED — below floor
#31 [Low] 1000-status cap → NOT RESOLVED — below floor

New findings

[Medium] #46 [Lens 16, 17] — The agent-logins limitation now states the wrong exemption conditions
- Location: README.md:L95
- Failure mode: "unless that account itself signs or is an org member" is false both ways — an allowlisted agent account passes (src/cla.js:L85), and an org-member one fails when exempt-org-members is false (src/main.js:L112). It also disagrees with "How it works" step 2.
- Suggested fix: "signs or is exempt (see How it works)", leaving the don't-allowlist-agents advice where it is.
- Verification: VERIFIED by reading src/cla.js:L80-L95 and src/main.js:L112-L130 (two agents independently).

Security deep-dive: done — re-run this round: 6 candidates, each a repeat of one filtered at 2/10 in earlier rounds (importer re-registration, shared head sha, author-name markdown, marker owner, rejected-then-required signer, head-sha race); no findings at confidence >= 8.
boundary-sweep: no new sinks in the delta (log wording only), clean.

Lens checklist

Spec & intent — Clean
Architectural fit — Clean
Correctness & edge cases — Clean
Security — Clean (deep-dive re-run found nothing at the bar; orchestrator-assessed)
Data integrity — Clean (no change; orchestrator-assessed)
Concurrency — Clean (no change; orchestrator-assessed)
API & contract — Clean (no change; orchestrator-assessed)
Reliability — Clean (no change; orchestrator-assessed)
Performance & cost — Clean
Observability — Clean
Operational safety — Clean (no change; orchestrator-assessed)
LLM/agent — N/A — no model calls
Testing — Clean
Dependencies & build — Clean (none added)
Code quality — Clean (no formatter or linter pinned)
Doc & comment accuracy — See findings #46
Cross-document consistency — See findings #46
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

cost: agents=4 wall=03:58 lenses-examined=16 of 19 files=17 lines=0

Self-review round 6: the note listed only org membership, which is wrong for an allowlisted agent account and for exempt-org-members: false. It now refers to "How it works" step 2 instead of restating the rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g

Self-review loop — round 7 (review-deep, FOLLOWUP)

Round tip: 5a5ded5f7fd1a7bb356ffe5763cb58f2d7fd8c3a · round delta 8167421..5a5ded5 (round-6 fix: one README sentence) · base fetch ok.
Why review-deep: review-shallow's gate reads the whole PR from the merge base, which carries the auth/token surface; shallow was not attempted.
Agents: comment-analyzer (16) — the delta is one README sentence, so no code lens had a surface to dispatch on; security deep-dive (1 find + 2 filters).

Prior context fetched — followup-fetch.sh exited 0; previous-review SHA 816742149eeb42b64ff113e071edf718b717fba8.

Scope change — NO

Verdict — APPROVE WITH NITS

Summary (open, including unresolved prior findings) — Critical: 0 · High: 0 · Medium: 3 · Low: 5 · Lenses run: 19/19 (12, 18, 19 N/A)

Round 7 found nothing on its delta: the comment analyzer verified the new sentence against classifyAuthors, addPrAuthor and "How it works" step 2, and the security deep-dive found nothing at the bar. Every open item below is below the severity floor (test gaps that change no behaviour, wording, or a speculative limit), unchanged since earlier rounds.

Prior findings status

#46 [Medium] agent-logins note lists wrong exemptions → RESOLVED — README.md:L95 now points to "How it works" step 2
#7 [Medium] shared fake for both tokens → NOT RESOLVED — below floor (test gap)
#8 [Medium] src/index.js untested → NOT RESOLVED — below floor (test gap)
#9 [Medium] importer untested → NOT RESOLVED — below floor (test gap; export field names unconfirmed without a real export)
#21 [Low] prose mismatches → PARTIALLY RESOLVED — wording only
#22 [Low] agent case folding untested → NOT RESOLVED — below floor
#28 [Low] "committer" wording → NOT RESOLVED — wording only
#29 [Low] truncation footer suggests recheck → NOT RESOLVED — wording only
#31 [Low] 1000-status cap → NOT RESOLVED — speculative (needs ~1000 comments without a push)

New findings

None.

Security deep-dive: done — re-run this round: 5 candidates; shared head sha, marker owner and author-name markdown repeat ones filtered at 2/10 earlier; bot PR author absorbing agent commits 2/10 and org-wide App key scope 2/10 filtered this round; no findings at confidence >= 8.
boundary-sweep: no sinks in the delta (README only), clean.

Lens checklist

Spec & intent — Clean
Architectural fit — Clean
Correctness & edge cases — Clean
Security — Clean (deep-dive re-run found nothing at the bar; orchestrator-assessed)
Data integrity — Clean (no code change; orchestrator-assessed)
Concurrency — Clean (no code change; orchestrator-assessed)
API & contract — Clean (no code change; orchestrator-assessed)
Reliability — Clean (no code change; orchestrator-assessed)
Performance & cost — Clean (no code change; orchestrator-assessed)
Observability — Clean (no code change; orchestrator-assessed)
Operational safety — Clean (no code change; orchestrator-assessed)
LLM/agent — N/A — no model calls
Testing — Clean (no test or code change; suite 30/30; orchestrator-assessed)
Dependencies & build — Clean (none added)
Code quality — Clean (no formatter or linter pinned)
Doc & comment accuracy — Clean
Cross-document consistency — Clean (the sentence now defers to step 2 instead of restating it; orchestrator-assessed)
On-prem — N/A — no on-prem surface
Scaling & lifecycle — N/A — no workers or autoscaling

cost: agents=4 wall=03:50 lenses-examined=16 of 19 files=17 lines=0

ritwik-g and others added 2 commits October 27, 2026 11:57

chore: add MIT licence

a1b59f2

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>