Skip to content

feat: expose explicit CLA pass result - #9

Merged
lawrencecchen merged 2 commits into
masterfrom
feat/pass-output
Sep 1, 2026
Merged

feat: expose explicit CLA pass result#9
lawrencecchen merged 2 commits into
masterfrom
feat/pass-output

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Sep 1, 2026

Copy link
Copy Markdown

Summary

  • add the cla_passed action output for the final policy result
  • keep it false for preflight, unsigned, closed, and error runs
  • set it true only after all-signed validation and the final bot comment write succeed, including the empty-ledger allowlisted bootstrap path
  • preserve signature_recorded semantics and document the distinction

Verification

  • regression commit 465b881 adds behavior tests before the implementation
  • implementation commit 8e1d05b adds source, metadata, docs, and bundled dist/index.js
  • npm test -- --runInBand (18 suites, 236 tests)
  • npm run lint
  • npx tsc --noEmit
  • npm run build
  • git diff --check

Summary by cubic

Adds a cla_passed action output so consumers no longer need to infer the final CLA result from signature_recorded. It is true only when all required contributors are signed and the final bot comment write succeeds; it remains false for preflight, unsigned, closed, and error runs.

  • The allowlisted empty-ledger bootstrap path reports a pass only after its all-signed comment is applied.
  • signature_recorded can remain true while another contributor is still unsigned.
  • Consumers must also require the writer job to succeed because cla_passed is a per-run result, not durable authorization.
  • Documents the output, updates the bundled dist/index.js, and adds regression coverage for success and failure paths.

Written for commit 8e1d05b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added a cla_passed workflow output indicating whether all required contributors signed and the final confirmation was applied successfully.
    • The output is true only after successful completion; otherwise, it remains false for preflight, unsigned, closed, and error runs.
    • Documented the new output and its relationship to signature_recorded.
  • Bug Fixes

    • Ensured failed final confirmation updates report cla_passed=false even when signatures were recorded.

@lawrence703 lawrence703 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 8e1d05b against base b4d3c4f. The new cla_passed output is false by default and for preflight, unsigned, closed, and error paths; it becomes true only after final all-signed validation and successful bot-comment application, including empty-ledger bootstrap. signature_recorded semantics stay unchanged. Tests cover partial signing, final-comment failure, preflight, closed/error paths, and bundled dist smoke. Isolated review helper is unavailable on this host (sandbox-exec status 71); manual exact-diff review and the reported full suite/lint/typecheck/build passed.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 6eb59b61-26cc-4fd2-b142-6652e87ecb39

📥 Commits

Reviewing files that changed from the base of the PR and between b4d3c4f and 8e1d05b.

⛔ Files ignored due to path filters (1)
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (12)
  • CHANGELOG.md
  • README.md
  • __tests__/integration/bugfixes.test.ts
  • __tests__/integration/distSmoke.test.ts
  • __tests__/integration/errorPaths.test.ts
  • __tests__/integration/preflight.test.ts
  • __tests__/integration/scenarios.test.ts
  • __tests__/main.test.ts
  • action.yml
  • src/main.ts
  • src/setupClaCheck.ts
  • src/signerPreflight.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The action adds a cla_passed output. It defaults to false and becomes true only after all required contributors sign and the final bot comment succeeds. Documentation and tests cover success, failure, preflight, closed, and partial-signature runs.

Changes

CLA passed output

Layer / File(s) Summary
Output contract and documentation
action.yml, README.md, CHANGELOG.md
Defines the cla_passed output and documents its final all-signed semantics separately from signature_recorded.
Runtime result assignment
src/main.ts, src/setupClaCheck.ts, src/signerPreflight.ts
Initializes cla_passed to false. Sets it to true only after successful all-signed completion and final comment application.
Behavior validation
__tests__/integration/*, __tests__/main.test.ts
Checks cla_passed across successful, unsigned, preflight, closed, error, and partial-signature paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 8e1d0

This PR adds an explicit per-run CLA result while preserving existing signature behavior; it is merge-ready after normal checks and review, with no actionable merge-blocking risk remaining.

Sequence Diagram(s)

sequenceDiagram
  participant Action
  participant SignerPreflight
  participant CLA_Check
  participant GitHub_API
  Action->>Action: Initialize cla_passed=false
  Action->>SignerPreflight: Run admission checks
  SignerPreflight-->>Action: Keep cla_passed=false
  Action->>CLA_Check: Validate required signatures
  CLA_Check->>GitHub_API: Apply final all-signed comment
  GitHub_API-->>CLA_Check: Comment apply succeeds
  CLA_Check-->>Action: Set cla_passed=true
Loading

Suggested reviewers: iainmcgin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: exposing an explicit CLA pass result through the new cla_passed output.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 9 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pass-output

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cmux-founders cmux-founders left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent maintainer review: exact head 8e1d05b passed the action test, lint, typecheck, build, bundled smoke, and CodeQL checks.

@lawrencecchen
lawrencecchen merged commit 212a0f2 into master Sep 1, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants