Fix SSS updates in optional mode

[lukaszgryglicki]

Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io

Assisted by OpenAI

Assisted by GitHub Copilot

Assisted by Claude

[coderabbitai]

Walkthrough

Both checkCompanyCompliance functions in cla-backend-go/v2/sign/service.go and cla-backend-legacy/internal/api/handlers.go now insert an early guard after the SSS call that accepts only clean/flagged statuses as actionable. Any other SSS status routes to complianceUnavailable (Go v2) or returns an error/persisted isSanctioned value (legacy), replacing the prior behavior that allowed ambiguous statuses to fall through in optional mode. Tests validate the Go v2 guard behavior across required and optional modes.

Changes

SSS Unexpected-Status Guard in Both Backends

Layer / File(s)	Summary
Early guard for unexpected SSS statuses cla-backend-go/v2/sign/service.go, cla-backend-legacy/internal/api/handlers.go	Both checkCompanyCompliance implementations now accept only Clean and Flagged as actionable SSS result statuses. In the Go v2 service, any other status routes through complianceUnavailable. In the legacy handler, any other status returns an error when sssRequired is true, or returns the currently persisted isSanctioned value when sssRequired is false, without clearing or persisting a sanction on an ambiguous result.
Tests for Go v2 complianceUnavailable behavior cla-backend-go/v2/sign/service_sss_test.go	Three new unit tests verify how complianceUnavailable handles unexpected SSS statuses in both required and optional modes: required mode blocks with an error, optional mode allows through for an unsanctioned company, and optional mode continues blocking when a persisted SSS-origin sanction exists. The standard library errors package is imported to support test construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• linuxfoundation/easycla#5078: Directly part of the same SSS enforcement wiring and checkCompanyCompliance logic, routing non-actionable statuses to complianceUnavailable.
• linuxfoundation/easycla#5087: Broader SSS compliance flow changes in the same checkCompanyCompliance/sanctioning logic, including sanction-origin persistence and gating.
• linuxfoundation/easycla#5091: Also updates checkCompanyCompliance in cla-backend-go/v2/sign/service.go to centralize non-actionable SSS outcomes through complianceUnavailable.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description lacks any substantive information about the changeset and only contains boilerplate attribution to tools and sign-off information.	Add a meaningful description explaining the SSS status handling fix, the bug being resolved, and why the changes are necessary for optional mode compliance.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix SSS updates in optional mode' directly reflects the main changes across both files, which focus on fixing SSS status handling in optional mode by treating ambiguous statuses as unavailable.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch unicron-sss-easycla-fix-2

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR updates the company sanctions screening flow (SSS) to treat non-clean/non-flagged SSS statuses as ambiguous, preventing optional-mode runs from accidentally clearing an existing SSS-origin block when SSS returns an unexpected status.

Changes:

• Treat unexpected SSS statuses as “no actionable live result”: block with an error when SSS is required; otherwise keep the persisted sanction decision.
• Ensure optional mode does not auto-clear an SSS-origin block (and in v2, also avoids caching) when SSS returns an ambiguous status.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
cla-backend-legacy/internal/api/handlers.go	In optional mode, unexpected SSS statuses now return the persisted sanction decision (no auto-clear on ambiguous statuses).
cla-backend-go/v2/sign/service.go	Routes unexpected SSS statuses through complianceUnavailable(...) so optional mode honors persisted sanction state and avoids caching/clearing.

[coderabbitai]

🧹 Nitpick comments (1)

cla-backend-go/v2/sign/service_sss_test.go (1)

89-120: 🏗️ Heavy lift

Add one guard-path test through checkCompanyCompliance for unexpected SSS status.

These tests validate complianceUnavailable itself, but they don’t verify that checkCompanyCompliance actually routes ambiguous statuses into that path. A regression in the guard (service.go, Line 3077) could slip through while Lines 89-120 still pass.

🤖 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 `@cla-backend-go/v2/sign/service_sss_test.go` around lines 89 - 120, Add a new
test function that verifies the integration path through the
checkCompanyCompliance function (the guard at service.go Line 3077) when
handling unexpected SSS status values. This test should call
checkCompanyCompliance directly rather than testing complianceUnavailable in
isolation, and should verify that ambiguous SSS statuses are properly routed to
and handled by the complianceUnavailable path. This ensures that a regression in
the guard routing logic would be caught, since the current tests only validate
complianceUnavailable itself without verifying that checkCompanyCompliance
actually invokes it correctly for unexpected status cases.

🤖 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.

Nitpick comments:
In `@cla-backend-go/v2/sign/service_sss_test.go`:
- Around line 89-120: Add a new test function that verifies the integration path
through the checkCompanyCompliance function (the guard at service.go Line 3077)
when handling unexpected SSS status values. This test should call
checkCompanyCompliance directly rather than testing complianceUnavailable in
isolation, and should verify that ambiguous SSS statuses are properly routed to
and handled by the complianceUnavailable path. This ensures that a regression in
the guard routing logic would be caught, since the current tests only validate
complianceUnavailable itself without verifying that checkCompanyCompliance
actually invokes it correctly for unexpected status cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a281c0d-ce41-48f1-bb37-09d48a4046a7

📥 Commits

Reviewing files that changed from the base of the PR and between 296c6bb and 32e3322.

📒 Files selected for processing (1)

• cla-backend-go/v2/sign/service_sss_test.go

[Copilot]

Pull request overview

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