Skip to content

docs(spec): EasyCLA → Self Serve program spec + M1–M6 milestone docs - #5132

Open
mlehotskylf wants to merge 5 commits into
devfrom
docs/easycla-ss-m2-m6
Open

docs(spec): EasyCLA → Self Serve program spec + M1–M6 milestone docs#5132
mlehotskylf wants to merge 5 commits into
devfrom
docs/easycla-ss-m2-m6

Conversation

@mlehotskylf

Copy link
Copy Markdown
Collaborator

What

Adds the program-level spec and the M2–M6 milestone design docs for the EasyCLA → LFX Self Serve migration. This completes the roadmap context around the M1 read-only "My CLAs" slice already covered by #5124.

Contents

specs/001-easycla-ss-integration-fable/:

  • spec.md — program-level feature spec (all six milestones, user stories, review outcomes)
  • 00-overview-fable.md — program overview, current-state architecture, cross-cutting risks
  • 0106-milestone-*-fable.md — per-milestone design docs (M1 read-only Me lens → M6 K8s V2 API + DynamoDB→Postgres eval)
  • checklists/requirements.md

Scope decisions

Related

🤖 Generated with Claude Code

Adds the program-level spec and the M2–M6 milestone design docs for the
EasyCLA → LFX Self Serve migration, completing the roadmap context around
the M1 read-only "My CLAs" slice already covered by #5124.

Scope: the -fable variant only (consistent with the m1-my-cla naming in
#5124). The -opus/-sonnet draft variants and Spec-Kit tooling from the
original 001-easycla-ss-integration working branch are intentionally left
out. The architecture-review deck lives as a Google Slides link in
docs/easycla-ss-migration/README.md (added in #5124).

Docs only — no code changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Copilot AI balanced review requested due to automatic review settings July 30, 2026 16:08
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR adds six EasyCLA-to-LFX Self Serve milestone specifications. It defines contributor signing, corporate and organization workflows, project administration, migration requirements, role boundaries, parity criteria, and legacy API consolidation criteria.

Changes

EasyCLA Self Serve migration

Layer / File(s) Summary
Program requirements and architecture
specs/001-easycla-ss-integration-fable/00-overview-fable.md, specs/001-easycla-ss-integration-fable/spec.md, specs/001-easycla-ss-integration-fable/checklists/requirements.md
Defines the six milestones, architecture, migration strategy, entities, requirements, acceptance criteria, and M6 implementation-detail boundaries.
Contributor CLA flows
specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md, specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md
Defines signature aggregation by document identifier and proactive Self Serve signing handoff to the Contributor Console.
Corporate CLA flows
specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md, specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md
Defines corporate ECLA and Organization-lens CCLA workflows, role boundaries, parity requirements, and console retirement.
Project CLA administration
specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md
Defines migration of project administration from PCC into Self Serve, including authorization mapping, enrollment parity, invalidation behavior, and removal criteria.
Legacy API consolidation
specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md
Clarifies the shared cla-backend deployment for legacy /v1 and /v2 Go APIs and refines M6 Lambda teardown criteria.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the documentation change and the EasyCLA to Self Serve migration scope.
Description check ✅ Passed The description directly explains the program specification, milestone documents, scope decisions, and docs-only changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/easycla-ss-m2-m6

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds the program roadmap and milestone designs for migrating EasyCLA functionality into LFX Self Serve.

Changes:

  • Defines program requirements, scenarios, and success criteria.
  • Documents M1–M6 architecture, scope, risks, and exit criteria.
  • Adds a specification quality checklist.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
spec.md Defines the six-milestone program specification.
checklists/requirements.md Records specification readiness checks.
00-overview-fable.md Summarizes architecture and migration strategy.
01-milestone-read-only-me-lens-fable.md Documents the M1 read-only CLA view.
02-milestone-sign-icla-fable.md Designs ICLA signing in Self Serve.
03-milestone-sign-ecla-fable.md Designs ECLA signing and console retirement.
04-milestone-ccla-org-lens-fable.md Designs organization-level CCLA management.
05-milestone-project-lens-pcc-fable.md Designs project-level EasyCLA administration.
06-milestone-k8s-v2-api-fable.md Evaluates Kubernetes and PostgreSQL migration.
Comments suppressed due to low confidence (2)

specs/001-easycla-ss-integration-fable/spec.md:236

  • M2 and M3 explicitly continue to call contributor-critical /v1//v2 endpoints, so stating that all M1–M5 work builds only against v4 contradicts the milestone designs and the assumption at line 217. Describe the full existing API surface to avoid dropping legacy endpoint reachability from the plan.
- **Q3 — Sequencing of the platform rewrite**: RESOLVED — UI-first: M1–M5 build against the existing v4 APIs with deliberately thin adapters (single SS server module); M6 remains a separately gated decision. Noted alternative worth revisiting at the M6 go/no-go: the hybrid strangler (stand up a CLA read/query V2 service after M2 for M4/M5 to consume) spreads M6 risk across the program and reduces adapter rework if M6 is committed early.

specs/001-easycla-ss-integration-fable/checklists/requirements.md:30

  • This repeats the inaccurate claim at line 9. Because the program spec intentionally includes concrete migration targets and API constraints, the readiness checklist should verify that those details are appropriately bounded, not claim that they are absent.
- [x] No implementation details leak into specification (spec.md)

Comment thread specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/checklists/requirements.md
Comment thread specs/001-easycla-ss-integration-fable/spec.md

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 14

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/001-easycla-ss-integration-fable/00-overview-fable.md`:
- Around line 94-99: Resolve the conflicting easycla-landing-page disposition:
in specs/001-easycla-ss-integration-fable/00-overview-fable.md lines 94-99,
explicitly state whether it remains operational or is retired with M3, including
the required redirect, documentation, support ownership, and exit-criteria
implications; then update specs/001-easycla-ss-integration-fable/spec.md lines
222-228 so the M3 decommission scope matches that decision.
- Around line 61-64: Scope the “cutover per milestone is a config flip” and
SC-007 rollback claims in 00-overview-fable.md to UI milestones, explicitly
excluding stateful M6. In 06-milestone-k8s-v2-api-fable.md at lines 20-22,
define event ownership and single-writer, idempotency, and replay rules for
gradual cutover; at line 28, clarify whether dual-run is shadow-only or permits
duplicate writes, and document an explicit M6 rollback state before claiming
rollback guarantees.
- Around line 14-21: Mark M5 as conditional in
specs/001-easycla-ss-integration-fable/00-overview-fable.md:14-21; make the
decision status authoritative and define the resulting scope in
specs/001-easycla-ss-integration-fable/spec.md:222-227; and remove the
unconditional “all pass/ready” claim from
specs/001-easycla-ss-integration-fable/checklists/requirements.md:34-35,
requiring separate plan branches or resolution of the PCC migration decision
before declaring the specification ready.
- Around line 33-34: Normalize lowercase prose references to “Approved List” in
specs/001-easycla-ss-integration-fable/00-overview-fable.md lines 33-34 and
specs/001-easycla-ss-integration-fable/spec.md lines 86, 160-163, 169, and
192-193; preserve exact API, module, and field names where required.

In
`@specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md`:
- Line 47: Update the historical signature merge/deduplication guidance to use
each signature or document’s stable identifier as the deduplication key, rather
than the current (type, project, company) tuple. Preserve distinct ICLA versions
for the same project or company while still removing duplicate identity matches.
- Around line 28-29: Update the PDF download requirements to define controls for
the 15-minute presigned URL: require Cache-Control: no-store, prevent the URL
from entering logs or analytics, and specify an appropriate restrictive referrer
policy. Apply the same controls to the corresponding requirement referenced at
the later location.

In `@specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md`:
- Around line 33-35: Update the SS entry route and Auth0 redirect handling to
derive the authenticated user from the server session, reject any
client-supplied user mismatch, and persist CLA group and return URL context in a
signed, expiring state value. Validate return URLs against approved
platform/project destinations before redirecting, preserving context through
login and preventing open redirects or account confusion.
- Line 57: Update the SS signing request path described in the embargo/OFAC
attestation section to enforce signature_embargo_acked=true server-side before
creating a signature, rejecting missing or false acknowledgements. Add
negative-path tests covering both omitted and false values, and ensure this
validation occurs before any signature creation or persistence.

In `@specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md`:
- Around line 45-46: Update the M4 entry-point continuity requirements to keep
email signatory acknowledgement outside the authenticated Organization lens:
define a separate public, single-use, expiring acknowledgement/DocuSign handoff
route for send-by-email signatories, and ensure the decommissioned Corporate
Console path does not handle this flow.
- Line 58: Update the M4 migration requirements to make the v4 API reject
deletion of the final CLA Manager, verify this behavior against the live API
contract, and add confirmation of the invariant to the console/BFF decommission
exit checklist. Remove the unresolved “decide where the guard belongs” wording
while preserving the requirement that at least one manager always remains.

In
`@specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md`:
- Line 51: Add an exit-criteria acceptance test for the Project-lens signature
invalidation path, extending SC-005 to verify preservation of
signature.invalidated, the invalidation actor and context, and the invalidated
count in the emitted audit event. Align the expected fields and behavior with
the existing event and project-handler contracts.

In `@specs/001-easycla-ss-integration-fable/checklists/requirements.md`:
- Line 9: Correct the “No implementation details” checklist item in
requirements.md and its corresponding occurrence in 30-30: either narrow the
wording to permit explicitly scoped architecture decisions documented in
Milestone 6, or uncheck the item so the checklist no longer claims all criteria
pass inaccurately.

In `@specs/001-easycla-ss-integration-fable/spec.md`:
- Around line 50-53: Constrain the preserved PR return URL in the remediation
flow to normalized URLs matching the approved supported Git-host pull-request
shapes, and reject all other values. Bind the accepted redirect state to the
originating session or request so it cannot be reused or altered across flows;
update the requirements around the unauthorized-contributor and signing return
paths while preserving valid PR redirection.
- Around line 142-147: Define the M1 identity-aggregation contract for FR-005
before considering it complete: update the user-resolution flow around
/v4/user-from-token and its implementation in server.go to deterministically
gather all linked EasyCLA records across emails and GitHub identities, specify
collision handling, and aggregate their agreements without attributing another
user’s data.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a1fb4829-8744-49f2-9d0c-a25b4c70d319

📥 Commits

Reviewing files that changed from the base of the PR and between 22867ae and 751b753.

📒 Files selected for processing (9)
  • specs/001-easycla-ss-integration-fable/00-overview-fable.md
  • specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md
  • specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md
  • specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md
  • specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md
  • specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md
  • specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md
  • specs/001-easycla-ss-integration-fable/checklists/requirements.md
  • specs/001-easycla-ss-integration-fable/spec.md

Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/checklists/requirements.md
Comment thread specs/001-easycla-ss-integration-fable/spec.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/spec.md
@mlehotskylf mlehotskylf changed the title docs(spec): EasyCLA → Self Serve program spec + M2–M6 milestone docs docs(spec): EasyCLA → Self Serve program spec + M1–M6 milestone docs Jul 30, 2026
Address review comments from copilot-pull-request-reviewer[bot], coderabbitai[bot]:

- 00-overview-fable.md: remove stale claim that CLAUDE.md describes cla-backend-legacy as Python (it doesn't) (per copilot-pull-request-reviewer[bot])
- 00-overview-fable.md: fix internal inconsistency on easycla-landing-page disposition — now matches spec.md's M3 decommission decision (per copilot-pull-request-reviewer[bot], coderabbitai[bot])
- 00-overview-fable.md, spec.md: normalize lowercase "approval list"/"approval-list" prose to "Approved List" per repo terminology convention (per coderabbitai[bot])
- 01-milestone-read-only-me-lens-fable.md: fix dedup-key guidance that contradicted the adjacent "versions shown, not hidden" note (per copilot-pull-request-reviewer[bot])
- 06-milestone-k8s-v2-api-fable.md: correct "separate legacy stack" wording — legacy-api-lambda deploys from the same cla-backend serverless stack (per copilot-pull-request-reviewer[bot])
- 06-milestone-k8s-v2-api-fable.md: remove obsolete "Python retirement" phrase (Python is already removed) (per copilot-pull-request-reviewer[bot])
- 06-milestone-k8s-v2-api-fable.md: soften exit criterion that contradicted the doc's own note that the Streams consumer Lambda may remain (per copilot-pull-request-reviewer[bot])
- checklists/requirements.md: soften "no implementation details" claim to acknowledge M6's explicitly scoped Kubernetes/Postgres decisions (per coderabbitai[bot])

Resolves 13 review threads; 13 additional threads addressed via reply only
(flagged as legitimate but premature implementation-level concerns for a
planning-stage doc, or already correctly marked as open decisions in spec.md).

Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 30, 2026 16:52
@mlehotskylf

Copy link
Copy Markdown
Collaborator Author

Review Feedback Addressed

Commit: f555c4e

Changes Made

  • 00-overview-fable.md: removed stale claim that CLAUDE.md still describes cla-backend-legacy as Python (verified CLAUDE.md already correctly says Python was removed) (per copilot-pull-request-reviewer[bot])
  • 00-overview-fable.md: fixed internal inconsistency on easycla-landing-page disposition — now matches spec.md's M3 decommission decision (per copilot-pull-request-reviewer[bot], coderabbitai[bot])
  • 00-overview-fable.md, spec.md: normalized lowercase "approval list"/"approval-list" prose to "Approved List" per the repo's documented terminology convention (field/API/route names left untouched) (per coderabbitai[bot])
  • 01-milestone-read-only-me-lens-fable.md: fixed dedup-key guidance that contradicted the adjacent "superseded versions shown, not hidden" note (per copilot-pull-request-reviewer[bot], coderabbitai[bot])
  • 06-milestone-k8s-v2-api-fable.md: corrected "separate legacy stack" wording — the legacy /v1//v2 Go surface deploys from the same cla-backend serverless stack (per copilot-pull-request-reviewer[bot])
  • 06-milestone-k8s-v2-api-fable.md: removed obsolete "Python retirement" phrase since Python is already gone (per copilot-pull-request-reviewer[bot])
  • 06-milestone-k8s-v2-api-fable.md: softened the exit criterion that contradicted the doc's own note that the Streams-consumer Lambda may remain (per copilot-pull-request-reviewer[bot])
  • checklists/requirements.md: softened the "no implementation details" claim to acknowledge spec.md's explicitly scoped M6 architecture decisions (Kubernetes, Postgres) (per copilot-pull-request-reviewer[bot], coderabbitai[bot])

Declined

  • Several security/enforcement suggestions (PR-return-URL allowlisting, session binding on the M2 entry route, presigned-PDF-URL Cache-Control, embargo/OFAC server-side enforcement, last-CLA-manager server-side invariant, SC-005 audit-event test coverage) are legitimate concerns, but they're implementation-level requirements for milestones that haven't started build-out yet, not spec-doc defects. Carried forward as future engineering requirements rather than fully specified now (flagged by coderabbitai[bot]).
  • Several findings were checked against current doc content and found to already be correct/already flagged as open decisions (M1 API references, M1-M5 legacy-API-surface coverage, M2 redirect sequencing, M5 decision-gating, M6 rollback framing, M4's email-signatory/last-manager gaps, FR-005 identity-aggregation scope) — no doc inconsistency found (flagged by copilot-pull-request-reviewer[bot], coderabbitai[bot]).

Threads Resolved

26 of 26 unresolved threads addressed in this iteration.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/001-easycla-ss-integration-fable/00-overview-fable.md`:
- Around line 96-99: Update the overview’s M3 retirement count to include
easycla-landing-page, and revise the M3 milestone table’s “Retires” entry to
list both Contributor Console and easycla-landing-page. Keep the scope
consistent with the existing landing-page retirement statement.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe91700b-1243-484b-9545-fbb1c57ee00a

📥 Commits

Reviewing files that changed from the base of the PR and between 751b753 and f555c4e.

📒 Files selected for processing (5)
  • specs/001-easycla-ss-integration-fable/00-overview-fable.md
  • specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md
  • specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md
  • specs/001-easycla-ss-integration-fable/checklists/requirements.md
  • specs/001-easycla-ss-integration-fable/spec.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • specs/001-easycla-ss-integration-fable/checklists/requirements.md
  • specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md

Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

specs/001-easycla-ss-integration-fable/00-overview-fable.md:64

  • This overstates the rollback guarantee. The M2 document itself says the GitLab/Gerrit switches do not yet exist or remain unverified, and the current server passes the same CLAContributorv2Base value into the GitLab services (cla-backend-go/cmd/server.go:507-509). Therefore a per-milestone/per-platform config flip is not yet available; qualify SC-007 until independent switches are implemented and rehearsed.
4. **Cutover per milestone is a config flip** (the SSM redirect base for contributor flows; lens feature flags for org/project), giving SC-007's rollback guarantee.

specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md:27

  • This relative link is broken in both this branch and dev: the referenced file is only being added by PR #5124, which is still open. Make #5124 a merge prerequisite and rebase this PR, or link to content that exists in the current tree, so implementers can access the required spike instructions.
- **A. Map to project lens authority (recommended)**: SS gates the CLA admin module on the same project-admin relation used by the lens's other admin features; EasyCLA v4 continues its own server-side checks. v4 accepts SS-authenticated calls with api-gw-audience access tokens (verified in the role-mapping feasibility analysis — authorization keys on the username, not the client/audience); confirm the project-scoped ACS policies admit these operations via spikes 1–2 in [docs/easycla-ss-migration/role-mapping-feasibility.md](../../docs/easycla-ss-migration/role-mapping-feasibility.md), or fall back to the M2M + subject pattern.

specs/001-easycla-ss-integration-fable/spec.md:112

  • Remove the duplicated “with”.
**Why this priority**: Pure re-platforming; enormous scope; only worth committing after UI migrations prove the integration surface. Explicitly presented to the architecture board as a decision with with/without-database options.

Comment thread specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md Outdated
Comment thread specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md Outdated
Address review comments from copilot[bot], coderabbitai:

- 00-overview-fable.md: correct the retirement scope — the program retires all
  four UIs (not three), and M3's "Retires" cell now includes
  easycla-landing-page, matching the committed decision in spec.md (per coderabbitai)
- 02-milestone-sign-icla-fable.md: the embargo/OFAC attestation is a client-only
  gate on a public endpoint (signature_embargo_acked written unconditionally at
  v2/sign/service.go:1546); M2 must require + validate it server-side, not just
  reimplement the checkbox (per copilot[bot])
- 04-milestone-ccla-org-lens-fable.md: same client-only bypass on the CCLA path
  (service.go:2731); M4 must enforce the attestation server-side (per copilot[bot])

Resolves 3 review threads.

Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 31, 2026 07:41
@mlehotskylf

Copy link
Copy Markdown
Collaborator Author

Review Feedback Addressed

Commit: 1570787a4

Changes Made

  • 00-overview-fable.md: corrected the retirement scope — the program retires all four UIs (was "three"), and M3's "Retires" cell now includes easycla-landing-page, matching the committed decision in spec.md (Program review outcomes) (per coderabbitai)
  • 02-milestone-sign-icla-fable.md: the embargo/OFAC attestation is a client-only gate on a public endpoint (signature_embargo_acked written unconditionally at v2/sign/service.go:1546); the parity item now states M2 must require + validate the attestation server-side before issuing the signing URL, not just reimplement the checkbox (per copilot[bot])
  • 04-milestone-ccla-org-lens-fable.md: same client-only bypass on the CCLA path (service.go:2731); the parity item now states M4 must enforce the attestation server-side (per copilot[bot])

Threads Resolved

All 3 previously-unresolved threads addressed in this iteration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (5)

specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md:3

  • The PR context states M1 has already shipped, but this milestone is still marked Planned; the linked M1 spec is also marked planned. Update the M1 status documentation to reflect the shipped implementation so M2 planning does not treat its dependency as unfinished.
**Status**: Planned — implementation artifacts in [m1-my-cla/](m1-my-cla/) | **Depends on**: nothing | **Retires**: nothing | **Effort**: S

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:51

  • Q3 is explicitly resolved as UI-first in spec.md:236 and this document's status line. Calling it an open decision and asking the board to choose again contradicts the approved sequencing; frame these as alternatives that may be revisited only at the later M6 go/no-go.
## Scope decision for the review (Open Decision Q3)

Because M3–M5 build SS↔v4 adapters and an ACS role bridge that M6 then replaces, the board should choose explicitly:

specs/001-easycla-ss-integration-fable/checklists/requirements.md:9

  • This checked claim still conflicts with spec.md, which directly specifies Kubernetes/Postgres plus concrete API and backend constraints; line 30 already acknowledges the intentional M6 exception. Reword this item so the checklist does not certify that implementation details are absent.
- [x] No implementation details (languages, frameworks, APIs) — spec.md is behavior-level; verified current-state technical facts are deliberately isolated in the milestone/overview docs, which the architecture board explicitly requested

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:18

  • This still says M6 retires a “separate stack,” although the same sentence correctly says the legacy API is deployed from the shared cla-backend stack, which also contains workers that Track A may retain. Describe retirement of the legacy API functions/routes instead of a nonexistent standalone stack.

This issue also appears on line 49 of the same file.

2. **Legacy Go backend** (`cla-backend-legacy`) — the `/v1`/`/v2` surface is already ported to Go (Python fully removed) and deployed from the `cla-backend` stack on the original `api.*` domains. This shrinks M6 meaningfully versus the old Python-port assumption, but it remains a second API codebase and deployment: M6 folds its endpoints into the V2 service (or consciously containerizes it alongside) and retires the separate stack.

specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md:27

  • This relative link is broken on the PR's dev base: docs/easycla-ss-migration/role-mapping-feasibility.md exists only in the still-open, unmerged #5124. Merge/rebase on #5124 first or include the referenced document here so this PR does not land with a dead dependency.
- **A. Map to project lens authority (recommended)**: SS gates the CLA admin module on the same project-admin relation used by the lens's other admin features; EasyCLA v4 continues its own server-side checks. v4 accepts SS-authenticated calls with api-gw-audience access tokens (verified in the role-mapping feasibility analysis — authorization keys on the username, not the client/audience); confirm the project-scoped ACS policies admit these operations via spikes 1–2 in [docs/easycla-ss-migration/role-mapping-feasibility.md](../../docs/easycla-ss-migration/role-mapping-feasibility.md), or fall back to the M2M + subject pattern.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
specs/001-easycla-ss-integration-fable/00-overview-fable.md (1)

118-118: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the M6 database decision wording.

The phrase “go/no-go with the with/without-database analysis” is difficult to parse. Rewrite it as “go/no-go informed by the database-option analysis in doc 06.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/00-overview-fable.md` at line 118,
Update the M6 clause in the milestone progression sentence to replace “go/no-go
with the with/without-database analysis in doc 06” with “go/no-go informed by
the database-option analysis in doc 06,” leaving the rest of the milestone
description unchanged.
specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md (1)

13-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use Approved List in descriptive text.

Line 13 uses approval-list CRUD, and Line 71 uses Approval-list writes. Use Approved List in prose. Keep PUT …/approval-list unchanged because it is an API path.

As per coding guidelines, use the terminology Approved List for this concept.

Proposed wording
-CCLA signing initiation ..., approval-list CRUD ...
+CCLA signing initiation ..., Approved List CRUD ...

-Auto-create-ECLA side effects | Approval-list writes can create signatures
+Auto-create-ECLA side effects | Approved List writes can create signatures

Also applies to: 71-71

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md`
at line 13, Update the descriptive text in the feature inventory and the
corresponding writes reference to use the exact terminology “Approved List”
instead of “approval-list” or “Approval-list”; preserve `PUT …/approval-list`
unchanged as the API path.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/001-easycla-ss-integration-fable/00-overview-fable.md`:
- Line 10: Update the migration-scope statement near the overview’s description
of the four EasyCLA UIs to clarify that those UIs are retired while emailed CCLA
signing remains available as an external, LF SSO-independent path through M4.
Keep the broader migration into LFX Self Serve unchanged for other user-facing
functionality.

---

Outside diff comments:
In `@specs/001-easycla-ss-integration-fable/00-overview-fable.md`:
- Line 118: Update the M6 clause in the milestone progression sentence to
replace “go/no-go with the with/without-database analysis in doc 06” with
“go/no-go informed by the database-option analysis in doc 06,” leaving the rest
of the milestone description unchanged.

In `@specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md`:
- Line 13: Update the descriptive text in the feature inventory and the
corresponding writes reference to use the exact terminology “Approved List”
instead of “approval-list” or “Approval-list”; preserve `PUT …/approval-list`
unchanged as the API path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b5d9668e-60c3-468f-937f-6aa0c45eb8b3

📥 Commits

Reviewing files that changed from the base of the PR and between f555c4e and 1570787.

📒 Files selected for processing (3)
  • specs/001-easycla-ss-integration-fable/00-overview-fable.md
  • specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md
  • specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md

Comment thread specs/001-easycla-ss-integration-fable/00-overview-fable.md Outdated
Address a follow-up review comment from coderabbitai: the intro said the
program migrates "all" user-facing functionality into SS, which overstated the
scope — emailed CCLA signatory signing deliberately stays an external, LF
SSO-independent DocuSign path through M4 (already documented in §5). Added a
one-clause carve-out on line 10 so the intro matches §5.

Resolves 1 review thread.

Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Copilot AI review requested due to automatic review settings July 31, 2026 10:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (6)

specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md:17

  • This current-state API description is stale: the implemented M1 contract is /v4/my-clas, while the existing /v4/signatures/user/{userID} route filters out ECLAs. Pointing readers at the nonexistent /users/{userID}/signatures shape would make the documented design unable to satisfy the milestone.
- The v4 API already has a per-user endpoint: `GET /v2 (v4 path) /users/{userID}/signatures` returning the user's ICLAs and ECLAs, plus per-signature PDF endpoints (`/signatures/{signatureID}/signed-document`, ICLA PDF routes).

specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md:47

  • These design decisions were superseded by the merged M1 API: EasyCLA now resolves identities, aggregates and deduplicates signatures, evaluates validity, and authorizes PDF access. Keeping those responsibilities in the Self Serve adapter conflicts with this directory's own implementation-update notes and would lead later milestones to build on the abandoned contract.
- **API sufficiency**: `GET /users/{userID}/signatures` requires the EasyCLA `userID`. SS must first resolve LF identity → EasyCLA user(s). If no efficient lookup-by-LF-username/email endpoint exists in v4, add one small read endpoint to EasyCLA (`GET /v4/users/by-identity?...` or reuse existing user search) rather than scanning client-side. This is the only backend change anticipated.
- **Authorization**: user-scoped data; the SS server must enforce "only the logged-in user's records" (derive `userID` server-side from the session, never trust a client-passed ID). Token model: user's bearer through the gateway if the EasyCLA v4 auth accepts it, else SS M2M with server-side subject binding — decide in design; crowdfunding token-exchange is the fallback pattern.
- **Validity semantics**: define "valid ECLA" precisely from the signature flags (`signature_approved`, `signature_signed`, not revoked/invalidated) and current-employer semantics; superseded ICLA document versions shown with status, not hidden — matches what CLA enforcement would actually honor.
- **Multiple EasyCLA users per person**: merge results by signature/document identifier (not by (type, project, company) alone, which would hide superseded ICLA versions — see validity semantics above), dedupe; show all — an incomplete list here erodes trust in every later milestone.

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:18

  • This sentence still says M6 retires a separate legacy stack, but /v1//v2, /v3, and several workers are deployed from the shared cla-backend/serverless.yml stack. The distinction matters because removing the legacy routes requires updating or partially dismantling that shared stack rather than deleting an independent deployment.
2. **Legacy Go backend** (`cla-backend-legacy`) — the `/v1`/`/v2` surface is already ported to Go (Python fully removed) and deployed from the `cla-backend` stack on the original `api.*` domains. This shrinks M6 meaningfully versus the old Python-port assumption, but it remains a second API codebase and deployment: M6 folds its endpoints into the V2 service (or consciously containerizes it alongside) and retires the separate stack.

specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md:3

  • The M3 scope and program overview both retire easycla-landing-page, but this milestone header lists only the Contributor Console. Include both so the at-a-glance decommission inventory matches the actual exit scope.
**Status**: Draft | **Depends on**: M2 | **Retires**: Contributor Console | **Effort**: L

specs/001-easycla-ss-integration-fable/00-overview-fable.md:10

  • This overview presents retirement of all four UIs as committed, but the reviewed program outcome says M5 may keep EasyCLA administration in PCC. Qualify the PCC portion here so the program goal does not contradict its decision gate.
EasyCLA today spans four UIs (Contributor Console, Corporate CLA Console, PCC EasyCLA module, plus the landing page) and a backend running as AWS Lambdas behind API Gateway / lfx-gateway. The program migrates EasyCLA user-facing functionality into **LFX Self Serve (LFX One)** under its Me / Organization / Project lenses, retiring all four UIs — the one deliberate exception is emailed CCLA signatory signing, which stays an external, LF SSO-independent DocuSign path through M4 (see §5) — and — as a separately gated decision — re-platforms the EasyCLA API as an LFX V2 Kubernetes service, optionally replacing DynamoDB with Postgres.

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:3

  • M6 cannot depend unconditionally on completing M5 because M5 may resolve to keeping EasyCLA in PCC. Express this as dependency on the M5 placement decision (or completed UI migrations) so the backend modernization remains schedulable under either approved outcome.
**Status**: Draft — **decision milestone**, not committed scope | **Depends on**: M1–M5 (UI-first sequencing confirmed; hybrid-strangler alternative noted below) | **Retires**: Lambda/API-GW deployment (including the legacy `/v1`/`/v2` Go surface, deployed from the same `cla-backend` serverless stack) | **Effort**: XL without DB migration; XXL with

Per Heather (PM) on Slack: Contributor Console is not cut over/retired
in M2. M2 instead adds a proactive entry point in Self Serve where a
contributor picks a CLA Group (and possibly GitHub org/repo) and
whether to sign ICLA or ECLA, then hands off to the existing
Contributor Console to complete signing for both paths. Update
02-milestone-sign-icla-fable.md accordingly, and flag the
now-uncertain downstream impact on M3, spec.md, and the program
overview with revision notes rather than resolving them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Copilot AI review requested due to automatic review settings August 4, 2026 11:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (10)

specs/001-easycla-ss-integration-fable/00-overview-fable.md:109

  • M2 is now PR-independent and leaves PR remediation/signing behavior entirely in the Contributor Console, so there is no M2 Self Serve UX in which to preserve or explain this multi-PR behavior. Assign this requirement to a future native PR-signing migration instead.
  - **Multi-PR behavior**: signing updates only one PR — verified mechanism: a single `active_signature:{userID}` KV record holds one return context; other PRs re-check via the `/easycla` comment command (verified handler) — preserve and state in SS UX copy (M2).

specs/001-easycla-ss-integration-fable/00-overview-fable.md:113

  • M2c was removed by the 2026-08-04 scope revision, so retaining it here contradicts the revised M2 document and the sequencing summary on line 118.
  - **Gerrit constraints**: instances are LF-hosted and added via support ticket (not self-service), CLA enablement is all-or-nothing per instance, and contributors must sign out/in of Gerrit after signing — M2c/M3c/M5 scope is narrower than GitHub/GitLab.

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:49

  • Q3 is not open anymore: the program spec marks UI-first sequencing resolved and approved, and this document's status line says sequencing is confirmed. Reframe these options as alternatives that may be revisited at M6's go/no-go rather than asking the board to decide Q3 again.
## Scope decision for the review (Open Decision Q3)

specs/001-easycla-ss-integration-fable/checklists/requirements.md:17

  • This checklist was not updated after the 2026-08-04 revision. M2 now has three explicit open design questions and no M2a–c split, while M3 says its scope and exit criteria cannot yet be trusted. Mark the affected checks incomplete and reassess the later “all items pass / ready for plan” declarations.
- [x] No [NEEDS CLARIFICATION] markers remain — Q1–Q3 resolved after review feedback on 2026-07-11 (Q1: LF login already mandatory in the console, no decision needed; Q2: all three platforms via sub-milestones M2a–c/M3a–c; Q3: UI-first, hybrid strangler noted as the alternative if M6 is committed early)
- [x] Requirements are testable and unambiguous

specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md:43

  • M2 no longer provides this pattern: it hands users to the Contributor Console and Self Serve never receives a sign_url. Describe M4's Organization-lens initiation as new Self Serve behavior rather than implying it reuses an implemented M2 flow.
3. **CCLA signing**: signatory flow reusing the M2 pattern (v4 returns DocuSign `sign_url`; send-by-email variant supported). No DocuSign integration in SS.

specs/001-easycla-ss-integration-fable/01-milestone-read-only-me-lens-fable.md:3

  • M1 is no longer planned: the related #5124 describes it as shipped, and #5125 is merged. The status and present/future-tense statements later in this document (for example, “Contributors currently have no self-service view”) should be converted to a historical design record so the roadmap does not present completed work as pending.
**Status**: Planned — implementation artifacts in [m1-my-cla/](m1-my-cla/) | **Depends on**: nothing | **Retires**: nothing | **Effort**: S

specs/001-easycla-ss-integration-fable/05-milestone-project-lens-pcc-fable.md:27

  • This relative link is broken on the current branch: docs/easycla-ss-migration/role-mapping-feasibility.md is only proposed by #5124, which is still open and unmerged. Merge/rebase that dependency first or include a reachable target so this milestone does not ship a dead reference.
- **A. Map to project lens authority (recommended)**: SS gates the CLA admin module on the same project-admin relation used by the lens's other admin features; EasyCLA v4 continues its own server-side checks. v4 accepts SS-authenticated calls with api-gw-audience access tokens (verified in the role-mapping feasibility analysis — authorization keys on the username, not the client/audience); confirm the project-scoped ACS policies admit these operations via spikes 1–2 in [docs/easycla-ss-migration/role-mapping-feasibility.md](../../docs/easycla-ss-migration/role-mapping-feasibility.md), or fall back to the M2M + subject pattern.

specs/001-easycla-ss-integration-fable/spec.md:214

  • This assumption contradicts M4's required Organization-lens CCLA initiation (FR-030 and milestone 04): in M4, Self Serve must request the corporate signing URL rather than the Contributor Console doing so. Keep backend ownership of DocuSign, but distinguish which UI calls EasyCLA in M2 versus M4.
- E-signature (DocuSign) integration stays inside the EasyCLA backend, unchanged, through M4: the Contributor Console (not Self Serve) requests the signing URL and hands the browser to the ceremony, for both the PR-redirect path and M2's new proactive-picker path; envelope creation, webhooks, and PDF storage do not move. *(Revised 2026-08-04: in the original framing SS itself requested the signing URL from M2 onward — that is no longer the plan; SS hands off to the Console before reaching that step. No new "DocuSign bridge" service is needed either way — analysis in milestone 02 doc.)*

specs/001-easycla-ss-integration-fable/00-overview-fable.md:107

  • The revised M2 has no signing UI—it redirects to the unchanged Contributor Console before signing. Assigning this attestation UI to M2 would send implementers to build behavior that M2 explicitly places out of scope.

This issue also appears in the following locations of the same file:

  • line 109
  • line 113
  - **Embargo/OFAC checkbox**: mandatory attestation gating the Sign button on ICLA/CCLA — required in M2/M4 signing UIs.

specs/001-easycla-ss-integration-fable/06-milestone-k8s-v2-api-fable.md:18

  • The legacy API is deployed from the shared cla-backend stack, as this same line and line 3 acknowledge, so there is no separate legacy stack to retire. Describe removal of the legacy deployment artifact from the shared stack instead; auxiliary Lambdas may remain there under Track A.

This issue also appears on line 49 of the same file.

2. **Legacy Go backend** (`cla-backend-legacy`) — the `/v1`/`/v2` surface is already ported to Go (Python fully removed) and deployed from the `cla-backend` stack on the original `api.*` domains. This shrinks M6 meaningfully versus the old Python-port assumption, but it remains a second API codebase and deployment: M6 folds its endpoints into the V2 service (or consciously containerizes it alongside) and retires the separate stack.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md (3)

51-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the required Approved List terminology.

Change approval-list management to Approved List management.

As per coding guidelines, use the terminology Approved List.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md` at
line 51, Update the milestone text by replacing “approval-list management” with
“Approved List management,” preserving the surrounding CCLA signing and
milestone wording.

Source: Coding guidelines


44-45: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Correct the API-version scope.

Line 30 lists Gerrit POST /v1/user/gerrit. Line 44 limits the M3 integration to v2/v3/v4 endpoints. Include the Gerrit v1 contract, or define the adapter that hides it. Otherwise M3c can omit the Gerrit integration.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md`
around lines 44 - 45, Update the M3 corporate-path scope to include the Gerrit
POST /v1/user/gerrit contract alongside the existing v2/v3/v4 endpoints, or
explicitly identify the adapter that encapsulates this v1 integration. Ensure
M3c retains Gerrit coverage and cannot omit the Gerrit integration.

74-75: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Align console retirement with the actual hand-off path.

Lines 27 and 37 keep the no-CCLA designee hand-off to the Corporate Console while M4 is incomplete. Line 66 also allows that temporary hop. Line 74 then requires approximately zero console traffic and console decommissioning as an M3 exit condition. Gate SC-003 on M4 completion, or allow the documented residual console path and define a measurable retirement threshold.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md`
around lines 74 - 75, Update SC-003 to align console retirement with the
documented hand-off flow: either gate the zero-console-traffic and
decommissioning requirements on M4 completion, or explicitly permit the
temporary Corporate Console path while defining a measurable threshold for
retirement. Preserve the existing M3a–M3c signed-off parity checklist
requirement.
🧹 Nitpick comments (1)
specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md (1)

60-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Define the retry and timeout budget.

Replace sensible timeouts with a per-attempt timeout, a total orchestration deadline, and terminal error behavior. The document gives retry counts for company creation and designee polling, but no time budget. A measurable contract prevents request hangs and inconsistent implementations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md` at
line 60, Update the company-add retry choreography requirement to define
explicit per-attempt and total orchestration timeouts, alongside the existing
retry counts for Clearbit, company creation, and Salesforce designee polling.
Specify terminal error behavior when either the per-attempt timeout or overall
deadline is exceeded, ensuring the server-side orchestration returns a
consistent failure rather than continuing or hanging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md`:
- Line 10: Update the revision note’s Contributor Console statement to use the
formal wording “remains unchanged” instead of “stays as-is,” while preserving
the rest of the scope and SSM cutover details.
- Line 34: Clarify the M2 scope in the picker flow described in “Picker UI” and
the corporate selection scope near the referenced out-of-scope statement:
explicitly identify whether org/repo selection applies to CLA Group picking,
corporate selection, or both. If corporate selection remains in Contributor
Console, update the In and Exit criteria to state that ownership and boundary
clearly.
- Line 19: Update the M2 specification around the Self Serve signing flow to
explicitly define the ICLA, CCLA, and ECLA sign-type contract: describe picker
options, map ICLA and CCLA to project_icla_enabled and project_ccla_enabled,
state the required corporate preconditions for CCLA/ECLA, and specify the
Contributor Console hand-off route. Use consistent terminology across the goal,
scope, hand-off, and exit criteria, while preserving the existing PR-check
remediation path.

In `@specs/001-easycla-ss-integration-fable/spec.md`:
- Around line 40-53: Clarify the M2 handoff contract in the User Story 2
acceptance criteria: explicitly define whether the picker’s ECLA option maps to
ECLA acknowledgement, CCLA, or both, and document the corresponding Contributor
Console flow for ICLA and corporate signing. Define the handoff payload’s
platform scope for GitHub, GitLab, and Gerrit, including equivalent
organization/repository inputs or stating that the Console resolves platform
context after handoff.

---

Outside diff comments:
In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md`:
- Line 51: Update the milestone text by replacing “approval-list management”
with “Approved List management,” preserving the surrounding CCLA signing and
milestone wording.
- Around line 44-45: Update the M3 corporate-path scope to include the Gerrit
POST /v1/user/gerrit contract alongside the existing v2/v3/v4 endpoints, or
explicitly identify the adapter that encapsulates this v1 integration. Ensure
M3c retains Gerrit coverage and cannot omit the Gerrit integration.
- Around line 74-75: Update SC-003 to align console retirement with the
documented hand-off flow: either gate the zero-console-traffic and
decommissioning requirements on M4 completion, or explicitly permit the
temporary Corporate Console path while defining a measurable threshold for
retirement. Preserve the existing M3a–M3c signed-off parity checklist
requirement.

---

Nitpick comments:
In `@specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md`:
- Line 60: Update the company-add retry choreography requirement to define
explicit per-attempt and total orchestration timeouts, alongside the existing
retry counts for Clearbit, company creation, and Salesforce designee polling.
Specify terminal error behavior when either the per-attempt timeout or overall
deadline is exceeded, ensuring the server-side orchestration returns a
consistent failure rather than continuing or hanging.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 400d3bab-a6f0-4b9f-9b90-1f70a261a804

📥 Commits

Reviewing files that changed from the base of the PR and between ef71238 and 436acf3.

📒 Files selected for processing (4)
  • specs/001-easycla-ss-integration-fable/00-overview-fable.md
  • specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md
  • specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md
  • specs/001-easycla-ss-integration-fable/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • specs/001-easycla-ss-integration-fable/00-overview-fable.md


Prior drafts of this milestone had SS take over the PR-redirect flow end-to-end (native DocuSign signing in SS, Contributor Console cut over via the `CLAContributorv2Base` SSM flip). Per Heather (PM) on Slack, 2026-08-04, that is **not** the M2 scope:

- **Contributor Console stays as-is.** The PR-check remediation link keeps pointing at the Console; there is no SSM cutover in M2.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use formal wording in the revision note.

Replace stays as-is with remains unchanged. This keeps the scope statement precise.

🧰 Tools
🪛 LanguageTool

[style] ~10-~10: To elevate your writing, try using more formal phrasing here.
Context: ... as-is.** The PR-check remediation link keeps pointing at the Console; there is no SSM cutover...

(CONTINUE_TO_VB)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md` at
line 10, Update the revision note’s Contributor Console statement to use the
formal wording “remains unchanged” instead of “stays as-is,” while preserving
the rest of the scope and SSM cutover details.

Source: Linters/SAST tools


## Goal

A contributor who wants to sign a CLA — without first hitting a failing PR check — can go to Self Serve, pick which CLA Group (and, where relevant, GitHub org/repo) they need to sign for, choose ICLA or ECLA, and be hopped off to the Contributor Console to complete the actual signing there. The PR-check remediation path is untouched: it still points at the Console today, same as before M2.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 \
  'ICLA|ECLA|CCLA|project_(icla|ccla)_enabled|request-(individual|employee)-signature' \
  specs/001-easycla-ss-integration-fable/spec.md \
  specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md \
  specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md \
  specs/001-easycla-ss-integration-fable/04-milestone-ccla-org-lens-fable.md

Repository: linuxfoundation/easycla

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in \
  specs/001-easycla-ss-integration-fable/00-overview-fable.md \
  specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md \
  specs/001-easycla-ss-integration-fable/03-milestone-sign-ecla-fable.md; do
  echo "===== $f ====="
  wc -l "$f"
  rg -n -C 3 'ICLA|ECLA|CCLA|project_(icla|ccla)_enabled|individual|corporate|employee|request-(individual|employee|corporate)-signature' "$f" \
    | sed -n '1,260p'
done

echo "===== exact M2 vocabulary occurrences ====="
python3 - <<'PY'
from pathlib import Path
p = Path("specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md")
terms = ("ICLA", "ECLA", "CCLA", "project_icla_enabled", "project_ccla_enabled")
for i, line in enumerate(p.read_text().splitlines(), 1):
    if any(term in line for term in terms):
        print(f"{i}: {line}")
PY

Repository: linuxfoundation/easycla

Length of output: 23723


Define the M2 sign-type contract for ICLA, CCLA, and ECLA.

ICLA is an individual agreement, CCLA is a corporate agreement, and ECLA is an employee acknowledgement of a CCLA. Define the picker options, the project_icla_enabled and project_ccla_enabled mappings, the required corporate preconditions, and the Contributor Console route. Use the same terms in the goal, scope, hand-off, and exit criteria.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md` at
line 19, Update the M2 specification around the Self Serve signing flow to
explicitly define the ICLA, CCLA, and ECLA sign-type contract: describe picker
options, map ICLA and CCLA to project_icla_enabled and project_ccla_enabled,
state the required corporate preconditions for CCLA/ECLA, and specify the
Contributor Console hand-off route. Use consistent terminology across the goal,
scope, hand-off, and exit criteria, while preserving the existing PR-check
remediation path.

### In

1. **New SS entry point** (e.g. surfaced from the Me lens — "Sign a CLA"), reachable without any PR context. Not a deep-link/redirect route; a normal authenticated SS page.
2. **Picker UI**: user selects a CLA Group. Where a CLA Group spans multiple GitHub orgs/repos, the picker additionally lets the user narrow to a specific org/repo (exact scope TBD in design — see Open questions).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Resolve the org/repo picker boundary.

Line 34 makes org/repo narrowing an in-scope picker requirement for multi-org CLA Groups. Line 44 places corporate org-selection refinement out of scope. State which flow uses org/repo selection in M2. If corporate selection remains in Contributor Console, state that in the In and Exit criteria.

Also applies to: 44-44

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/02-milestone-sign-icla-fable.md` at
line 34, Clarify the M2 scope in the picker flow described in “Picker UI” and
the corporate selection scope near the referenced out-of-scope statement:
explicitly identify whether org/repo selection applies to CLA Group picking,
corporate selection, or both. If corporate selection remains in Contributor
Console, update the In and Exit criteria to state that ownership and boundary
clearly.

Comment on lines +40 to +53
### User Story 2 (Milestone 2) - Contributor proactively starts CLA signing from Self Serve, hands off to Contributor Console (Priority: P2)

**Revised 2026-08-04** (per Heather, PM, Slack): the Contributor Console is **not** cut over or retired in M2, and the PR-check remediation link keeps pointing at the Console unchanged. Instead, M2 adds a new, additive entry point: a contributor can go to Self Serve without any PR context, pick a CLA Group (and possibly a GitHub org/repo) from a dropdown, choose whether they want to sign an ICLA or acknowledge an ECLA, and be handed off to the existing Contributor Console to complete the signing there — for both the ICLA and CCLA paths. SS does not run the signing ceremony itself in M2.

**Why this priority**: Delivers a self-initiated signing path (no failing PR required) using the Console's existing, already-working signing flow — lower-risk than moving DocuSign/ECLA logic natively into SS, while still giving contributors a reason to start using Self Serve.

**Independent Test**: As a logged-in Self Serve user with no open PR, use the new "Sign a CLA" entry to pick a CLA Group and ICLA/ECLA, and verify landing in the Contributor Console pre-scoped to that CLA Group/sign type, ready to complete signing there.

**Acceptance Scenarios**:

1. **Given** a logged-in Self Serve user with no PR context, **When** they open the new proactive signing entry, **Then** they can pick from CLA Groups available to them (narrowing by GitHub org/repo where applicable).
2. **Given** a user who picked a CLA Group and ICLA, **When** they proceed, **Then** they are handed off to the Contributor Console's individual-signing flow, scoped to that CLA Group.
3. **Given** a user who picked a CLA Group and ECLA/CCLA, **When** they proceed, **Then** they are handed off to the Contributor Console's corporate flow, scoped to that CLA Group.
4. **Given** the existing PR-check status link, **When** a contributor clicks it, **Then** they still land directly in the Contributor Console as today — unaffected by the new SS entry point.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Define the M2 handoff contract for agreement type and platform scope.

Line 154 lets the user choose ICLA or ECLA, but Line 155 requires both ICLA and CCLA paths. Line 52 also uses ECLA/CCLA. State whether the picker selects an ECLA acknowledgement, a CCLA flow, or both. Define the mapping to the Contributor Console.

Line 156 says the handoff applies to GitHub, GitLab, and Gerrit, but Lines 42 and 50 define only GitHub organization/repository narrowing. Specify equivalent GitLab/Gerrit inputs, or state that the Console resolves platform context after handoff. Without this, the M2 acceptance criteria do not define one implementable handoff payload.

Also applies to: 151-157

🧰 Tools
🪛 LanguageTool

[style] ~42-~42: To elevate your writing, try using more formal phrasing here.
Context: ...n M2, and the PR-check remediation link keeps pointing at the Console unchanged. Instead, M2 a...

(CONTINUE_TO_VB)


[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ng flow, scoped to that CLA Group. 3. Given a user who picked a CLA Group and ECL...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...te flow, scoped to that CLA Group. 4. Given the existing PR-check status link, **...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-easycla-ss-integration-fable/spec.md` around lines 40 - 53, Clarify
the M2 handoff contract in the User Story 2 acceptance criteria: explicitly
define whether the picker’s ECLA option maps to ECLA acknowledgement, CCLA, or
both, and document the corresponding Contributor Console flow for ICLA and
corporate signing. Define the handoff payload’s platform scope for GitHub,
GitLab, and Gerrit, including equivalent organization/repository inputs or
stating that the Console resolves platform context after handoff.

mlehotskylf added a commit that referenced this pull request Aug 4, 2026
…d-off)

Extract Milestone 2 into its own implementable Spec Kit folder
(m2-sign-cla-handoff/), mirroring the existing m1-my-cla/ layout, based on
main so it is self-contained and independent of the program-level design PR
(#5132).

Reflects the 2026-08-04 revision (per Heather/PM): M2 is an additive,
PR-independent "Sign a CLA" picker in the Me lens that hands off to the
existing Contributor Console for signing — for both ICLA and CCLA paths. SS
runs no signing ceremony, makes no signing-initiation calls, and does not cut
over or retire the Console; the PR-check remediation link is unchanged.

Scaffolds spec.md + plan.md only. The hand-off contract, org/repo picker
scope, and CLA-Group discovery endpoint are left as explicit open questions
for the Spec Kit planning flow to resolve — research.md/data-model.md/
contracts/ are intentionally not hand-written.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
mlehotskylf added a commit that referenced this pull request Aug 4, 2026
…d-off)

Extract Milestone 2 into its own implementable Spec Kit folder
(m2-sign-cla-handoff/), mirroring the existing m1-my-cla/ layout, based on
main so it is self-contained and independent of the program-level design PR
(#5132).

Reflects the 2026-08-04 revision (per Heather/PM): M2 is an additive,
PR-independent "Sign a CLA" picker in the Me lens that hands off to the
existing Contributor Console for signing — for both ICLA and CCLA paths. SS
runs no signing ceremony, makes no signing-initiation calls, and does not cut
over or retire the Console; the PR-check remediation link is unchanged.

Scaffolds spec.md + plan.md only. The hand-off contract, org/repo picker
scope, and CLA-Group discovery endpoint are left as explicit open questions
for the Spec Kit planning flow to resolve — research.md/data-model.md/
contracts/ are intentionally not hand-written.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
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.

2 participants