chore(sonar): clear the remaining orbit-api CODE_SMELLs

[thomasluizon]

Drives the last open SonarCloud CODE_SMELL issues on thomasluizon_orbit-api to zero. All changes are behavior-preserving.

Rules cleared

Rule	Count	Fix
external_roslyn:CA1861	6	Hoist the constant column arrays in SyncChangesIndexConfigurationTests into deduped static readonly string[] fields.
csharpsquid:S107	3	(a) SendWithToolsAsync's 9 args → a cohesive AiToolRequest record (destructured at entry; interface, impl, production caller, and all test call-sites/mocks updated). (b) The 11-param CreateAuditContext factory is deleted — callers construct the existing AuditContext record directly (record primary ctors are exempt from S107). (c) LogRateLimitRejected is a [LoggerMessage] source-gen false positive (each param binds to a {placeholder}); suppressed with #pragma warning disable S107 + a URL-linked WHY note.
external_roslyn:xUnit1045	1	ScheduledJobRegistryTests theory data switched from the non-serializable IScheduledJob to the xUnit-serializable Type discriminator (documented fix #1).
csharpsquid:S3267	1	Select(part => part.Text) projection on the streaming content loop in AiIntentService.
csharpsquid:S1905	1	Drop the redundant List<PushSubscription> cast/local in PushNotificationService (the parameter is already a List).

Verification (foreground, local)

• dotnet build Orbit.slnx — 0 errors, 0 new warnings.
• dotnet test Orbit.slnx — 5297 passed, 0 failed, 0 skipped.
• dotnet ef migrations has-pending-model-changes — false (no model touched).
• src/Orbit.Api/openapi.json — unchanged (no HTTP contract altered).
• No bare // narration added (the one new // is the URL-linked S107 WHY note).

Refs thomasluizon/orbit-ui-mobile#243

[claude]

Code Review: PR #397 — chore(sonar): clear the remaining orbit-api CODE_SMELLs

Scope: PR #397 in thomasluizon/orbit-api
Recommendation: APPROVE

Summary

This is a behavior-preserving Sonar CODE_SMELL cleanup: SendWithToolsAsyncs 9 params bundled into a new AiToolRequest record, the 11-param CreateAuditContext factory deleted in favor of constructing the AuditContext record directly (with defaults moved onto the record declaration), a scoped #pragma warning disable/restore S107 around a [LoggerMessage] false positive, a Select-based projection replacing a tuple-field access in a streaming loop, a redundant List<PushSubscription> cast removed, and two test-only Sonar fixes (deduped static readonly string[] column arrays; TheoryData<IScheduledJob> swapped for a serializable Type discriminator). All production call sites and test call-sites were updated in lockstep — verified there are no stale usages of the old SendWithToolsAsync/CreateAuditContext signatures anywhere in src/ or tests/.

Findings

Critical: None.

High: None.

Medium: None.

Low / Info:

• The new AiToolRequest records XML-doc comment, and the URL-linked WHY note on the S107 pragma in DistributedRateLimitAttribute.cs, both conform to the comment policy (dimension 4) — XML-doc on an exported type and a WHY-with-URL note are the two allowed comment forms.
• AuditContexts five optional fields (TargetId, TargetName, Error, ShadowPolicyDecision, ShadowReason) moved their defaults from the deleted factory onto the record declaration itself; every one of the 6 call sites in AgentOperationExecutor.cs still supplies the same values (by name or position) it did pre-refactor — confirmed by diffing each call site against its previous CreateAuditContext(...) invocation.
• ScheduledJobRegistryTests.AllScheduledJobTypes()/Job_HasNameAndCronExpression correctly fixes xUnit1045 (non-serializable theory data): BuildAll() constructs all 11 jobs as distinct concrete types with no duplicates, so the Single(j => j.GetType() == jobType) re-lookup per theory case is unambiguous.

Subagents

Agent	Verdict
security-reviewer	PASS — no findings across all 7 changed src/ files (AiToolRequest never logged, all 6 AuditContext call sites still redact via RedactArguments, S107 pragma scoped only to the false-positive LoggerMessage, no auth/CORS/JWT surface touched)
contract-aligner	N/A — no DTO, Controller route, or packages/shared surface changed

Validation

Build (dotnet): N/A in CI — separate required check.
Tests (dotnet): N/A in CI — separate required check.

Deferred — N/A dimensions & files not verdicted

• Dimension 8 (DESIGN.md/AI-slop): N/A — no apps/* UI files in this diff (backend-only repo).
• Dimension 9 (Parity) / 10 (i18n): N/A — frontend-only dimensions, not applicable to an orbit-api diff.
• Dimension 11 (Contract drift/backward-compat): N/A — no DTO or Controller route changed; contract-aligners sibling-repo half (packages/shared) is not checked out in this CI job and is not verifiable here regardless.
• Dimension 14 (FEATURES.md parity): N/A — pure refactor, no user-facing feature/behavior change.
• All 11 changed files (7 src/, 4 tests/) were read in full and given a verdict above; nothing deferred within scope.

What is good

• Every one of the 9 old SendWithToolsAsync positional args and every CreateAuditContext call site was migrated in the same PR, including all NSubstitute Arg.Any()/Arg.Is() matcher call-sites in ProcessUserChatCommandHandlerTests.cs and AiIntentServiceStreamingTests.cs — no orphaned old-signature usage anywhere.
• The S107 pragma is scoped tightly to just the one false-positive method, not broadened to the neighboring LogRateLimitEvaluationFailed (also 6 params) which stays subject to the real rule.
• PR description is unusually precise: a rule→count→fix table plus a pre-verified build/test/migration/openapi diff — matches what is actually in the diff.

Recommendation

Approve as-is. No action required.

[claude]

PR Review — #397 (chore/api-sonar-smell-tail)

Recommendation: APPROVE

Provenance

• Current head: 862a26cbf1872ca67e214eb0bc2a55083928a787
• This exact commit already carries an APPROVED review from this bot; no commits have been pushed since. Re-running the /pr-review rubric found no new diff surface to evaluate.
• SonarCloud Quality Gate: passed — 0 new issues, 100% coverage on new code, 0 duplication.

Findings

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	0

Summary (carried forward from prior review)

• AiToolRequest parameter-object migration for SendWithToolsAsync, with every call site and test updated accordingly.
• CreateAuditContext factory removed in favor of the existing AuditContext record, defaults preserved at all 6 call sites.
• A tightly-scoped #pragma warning disable/restore S107 with a URL-linked WHY note, addressing a [LoggerMessage] false positive.
• Three smaller Sonar-only fixes, no behavior change.
• No DTO/route/contract surface touched — cross-repo consumer parity not applicable.

No Critical/High findings. Clean, behavior-preserving Sonar cleanup — approving.

[claude]

Review Complete

Scope: PR #397 in thomasluizon/orbit-api ("chore(sonar): clear the remaining orbit-api CODE_SMELLs")
Recommendation: APPROVE

Provenance check

Current head 862a26cbf1872ca67e214eb0bc2a55083928a787 already carries two prior APPROVED reviews from this bot, the second one confirming "no commits pushed since" the first. No new commits exist since the last review — I independently re-walked the full 598-line diff and ran the security-reviewer subagent fresh rather than merely trusting the prior verdict, and reached the same conclusion.

What changed (11 files, all confirmed behavior-preserving)

• AiToolRequest record replaces SendWithToolsAsync's 7 loose params (S107) — single production caller (ProcessUserChatCommand.Ai.cs) and all Infrastructure/test call sites updated; destructuring order in AiIntentService.cs matches the record's declared order exactly (verified no field transposition, e.g. ImageData/ImageMimeType not swapped).
• AgentOperationExecutor.CreateAuditContext (11-param factory, S107) deleted; all 6 call sites construct AuditContext directly with named args; trailing params gained = null defaults with no behavior change (verified Error/ShadowPolicyDecision/ShadowReason/RedactedArguments still flow through unchanged at every call site).
• #pragma warning disable/restore S107 around LogRateLimitRejected in DistributedRateLimitAttribute.cs, confirmed to bracket only that one method (lines 226-241), with a URL-linked WHY comment (compliant with the narration-comment / ORBIT0001 policy).
• AiIntentService.cs: Select(part => part.Text) projection (S3267), no value change.
• PushNotificationService.cs: dropped a redundant List<PushSubscription> cast (S1905) - subs param is already typed List<...>.
• Test-only fixes: deduped static readonly string[] column arrays (CA1861), TheoryData<Type> swap for xUnit1045 serializability.

Findings

None - Critical / High / Medium / Low: all 0.

Subagents

Agent	Verdict
security-reviewer	PASS - no auth/CORS/webhook/rate-limit logic altered; pragma scoped correctly; no field dropped/reordered in the AiToolRequest/AuditContext refactors
contract-aligner	N/A - no DTO, Controller route, or packages/shared surface touched; sibling repo not checked out (would be "not verifiable in CI" if the gate had fired)

Deferred

• Parity / i18n / DESIGN.md / FEATURES.md dimensions: N/A - no frontend, UI, or user-facing feature surface changed (purely internal Sonar cleanup).
• Contract drift: gate not met - no DTO/route/shared-type changed.
• Build/test: skipped here - Build/Unit Tests/SonarCloud run as separate required CI checks on this PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Review Complete

Scope: PR #397 in thomasluizon/orbit-api (chore/api-sonar-smell-tail → main)
Recommendation: APPROVE (carried forward — no new diff to review)

Provenance check

This PR already has prior APPROVED reviews from this bot at the current head commit f3d3096d7efbd60771557797a521fcbe3e934826. That commit's message is explicit: "The Analyze (actions) CodeQL init step failed with a transient network error (read ECONNRESET); this empty commit forces a clean re-run. No code change." It is a CI-retrigger commit only — no files changed since the prior reviews.

The substance of the PR (bundling SendWithToolsAsync's params into AiToolRequest, deleting CreateAuditContext in favor of AuditContext with defaults on the record, the scoped #pragma S107 with URL-linked WHY note on the [LoggerMessage] false positive, the Select-projection S3267 fix, the redundant-cast S1905 fix, and the two test-only theory-data fixes) was already reviewed clean across prior passes, with security-reviewer PASS and no Critical/High findings.

Current state

• Build: SUCCESS. OpenAPI Breaking-Change Gate, Guard Migrations, Guard Conventions, Dependency Review/Scan, GitGuardian: all SUCCESS.
• SonarCloud: Quality Gate passed, 0 new issues, 100% coverage on new code, 0 duplication.
• Cross-repo dimension (orbit-ui-mobile contract-aligner): not verifiable in this CI job — sibling repo not checked out. No API surface changes in this diff that would require consumer updates.

Findings

None.

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	0