Skip to content

fix(OFJAVA-033): CU-86akeeczr 41 review findings across 30 files - #2172

Draft
flamingo[bot] wants to merge 30 commits into
mainfrom
ai-fix/ofjava-033-001b9e48-3bcc34c5
Draft

flamingo[bot] wants to merge 30 commits into
mainfrom
ai-fix/ofjava-033-001b9e48-3bcc34c5

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Closes 41 review findings across 30 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

# Fix confidence Finding Location
1 🟢 95 high Java record used for CredentialHeader instead of Lombok class openframe-external-api-service-core/src/main/java/com/openframe/external/service/RestProxyService.java:186
2 🔴 40 low — review closely Response body logged in full at INFO level, potential PII/secret leakage in proxied tool responses openframe-external-api-service-core/src/main/java/com/openframe/external/service/RestProxyService.java:102
3 🔴 20 low — review closely RestProxyService.proxyApiRequest performs unscoped tool lookup by key without tenant filtering openframe-external-api-service-core/src/main/java/com/openframe/external/service/RestProxyService.java:61
4 🔴 15 low — review closely RestProxyService reads directly from IntegratedToolRepository, mixing data-access with proxying logic openframe-external-api-service-core/src/main/java/com/openframe/external/service/RestProxyService.java:61
5 🟡 85 medium Verbose per-request INFO logging of tool internals in RestProxyService openframe-external-api-service-core/src/main/java/com/openframe/external/service/RestProxyService.java:76
6 🟢 92 high Java record used for RetryState instead of a Lombok-annotated class openframe-client-core/src/main/java/com/openframe/client/service/rmm/ScriptDeliveryRetryStore.java:74
7 🟢 90 high Exception swallowed with only e.getMessage() logged, losing stack trace openframe-client-core/src/main/java/com/openframe/client/service/rmm/ScriptDeliveryRetryStore.java:26
8 🟢 90 high Exception swallowed with only e.getMessage() logged in write() openframe-client-core/src/main/java/com/openframe/client/service/rmm/ScriptDeliveryRetryStore.java:49
9 🟢 90 high AppleNativeRegisterRequest declared as a Java record instead of a Lombok class openframe-security-oauth/src/main/java/com/openframe/security/oauth/controller/OAuthBffController.java:236
10 🟡 85 medium Exception message logged without the exception object in OAuth callback error handler openframe-security-oauth/src/main/java/com/openframe/security/oauth/controller/OAuthBffController.java:197
11 🟡 85 medium Exception swallowed without full stack trace in appleNativeRegister error handler openframe-security-oauth/src/main/java/com/openframe/security/oauth/controller/OAuthBffController.java:230
12 🟢 90 high JoinPendingResponse and record usage in SsoJoinController violate the no-record rule openframe-authorization-service-core/src/main/java/com/openframe/authz/controller/SsoJoinController.java:63
13 🔴 30 low — review closely Business validation logic embedded directly in controller method rather than delegated to service or Bean Validation openframe-authorization-service-core/src/main/java/com/openframe/authz/controller/SsoJoinController.java:108
14 🟡 85 medium SignupTicketPayload declared as a Java record, forbidden by org convention openframe-authorization-service-core/src/main/java/com/openframe/authz/service/sso/SignupTicketService.java:35
15 🟡 70 medium IllegalStateException used for a business-flow condition (expired signup session) rather than a dedicated exception openframe-authorization-service-core/src/main/java/com/openframe/authz/service/sso/SignupTicketService.java:67
16 🔴 15 low — review closely CreateOrganizationRequest / UpdateOrganizationRequest appear to be Java records based on accessor-style calls openframe-api-lib/src/main/java/com/openframe/api/mapper/OrganizationMapper.java:26
17 🔴 40 low — review closely OrganizationMapper methods return null instead of throwing or using Optional openframe-api-lib/src/main/java/com/openframe/api/mapper/OrganizationMapper.java:21
18 🔴 30 low — review closely Nested EntityCount/CategoryCount likely record types referenced but not shown; verify no java record usage — flagged pending confirmation openframe-notification-core/src/main/java/com/openframe/notification/readstate/NotificationReadStateService.java:145
19 🔴 55 low — review closely dismissForAllRecipients computes flipped count independently of the recipients actually notified, risking drift between DB state and published events openframe-notification-core/src/main/java/com/openframe/notification/readstate/NotificationReadStateService.java:111
20 🟢 90 high TicketNoteRequest declared as a Java record openframe-external-api-service-core/src/main/java/com/openframe/external/dto/ticket/TicketNoteRequest.java:7
21 🟢 90 high CustomerFilterResponse declared as a Java record, violating the Lombok-class convention openframe-external-api-service-core/src/main/java/com/openframe/external/dto/audit/CustomerFilterResponse.java:5
22 🟢 90 high CreateCustomerRequest declared as a Java record, forbidden by OFJAVA-033 openframe-external-api-service-core/src/main/java/com/openframe/external/dto/customer/CreateCustomerRequest.java:13
23 🟢 90 high TicketOwnerResponse declared as a Java record, violating the no-records convention openframe-external-api-service-core/src/main/java/com/openframe/external/dto/ticket/TicketOwnerResponse.java:6
24 🟡 85 medium UpdateTicketRequest declared as a Java record instead of a Lombok class openframe-external-api-service-core/src/main/java/com/openframe/external/dto/ticket/UpdateTicketRequest.java:9
25 🟢 90 high ToolUrlResponse declared as a Java record, forbidden by org convention openframe-external-api-service-core/src/main/java/com/openframe/external/dto/tool/ToolUrlResponse.java:6
26 🟡 85 medium Java record used for SsoLoginCookiePayload violates no-record convention openframe-authorization-service-core/src/main/java/com/openframe/authz/security/SsoLoginCookiePayload.java:7
27 🟢 90 high EntityCount declared as a Java record, violating the no-record convention openframe-data-mongo-sync/src/main/java/com/openframe/data/repository/notification/EntityCount.java:6
28 🟢 90 high MachineField declared as a Java record instead of a Lombok-annotated class openframe-data-mongo-sync/src/main/java/com/openframe/data/service/machine/MachineField.java:7
29 🟢 90 high CreateTicketRequest declared as a Java record instead of a Lombok class openframe-external-api-service-core/src/main/java/com/openframe/external/dto/ticket/CreateTicketRequest.java:10
30 🟡 60 medium UtilityClass ScheduleRecurrence returns null instead of Optional — combined with rule violation risk openframe-client-core/src/main/java/com/openframe/client/service/rmm/ScheduleRecurrence.java:11
31 🟢 95 high LogProjection dataclass uses public fields instead of Lombok-generated private fields with accessors openframe-data-pinot/src/main/java/com/openframe/data/pinot/model/LogProjection.java:15
32 🟡 70 medium NotificationSettingsView omits @builder and @NoArgsConstructor required by the Lombok quartet convention openframe-api-service-core/src/main/java/com/openframe/api/dto/NotificationSettingsView.java:9
33 🟢 90 high MetricsMessage lacks Lombok quartet required for model classes openframe-client-core/src/main/java/com/openframe/client/dto/metrics/MetricsMessage.java:1
34 🟡 75 medium ChocoEntry omits @NoArgsConstructor, breaking the required Lombok quartet for a data-carrier class openframe-api-service-core/src/main/java/com/openframe/api/service/packagesearch/ChocoEntry.java:9
35 🟢 90 high Verify AgentTokenResponse/AgentRegistrationResponse DTOs use Lombok class, not record — confirmed compliant but check builder absence openframe-client-core/src/main/java/com/openframe/client/dto/AgentTokenResponse.java:7
36 🟡 60 medium SSOPerTenantConfig missing @builder despite Lombok quartet expectations for model classes openframe-data-mongo-common/src/main/java/com/openframe/data/document/tenant/SSOPerTenantConfig.java:17
37 🔴 25 low — review closely Duplicate ToolResponse DTOs across modules risk drift instead of shared type openframe-test-service-core/src/main/java/com/openframe/test/data/dto/external/tool/ToolResponse.java:1
38 🟢 90 high SetupResponse model does not use full Lombok quartet required for MAJORLEA/OPENFRAM data-carrier classes sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/model/SetupResponse.java:7
39 🟡 85 medium Java record used instead of Lombok class for ExecutionOwnerScope openframe-data-mongo-common/src/main/java/com/openframe/data/document/rmm/filter/ExecutionOwnerScope.java:6
40 🟢 90 high ForceToolAgentUpdateResponse missing Lombok quartet (only @DaTa) openframe-api-service-core/src/main/java/com/openframe/api/dto/force/response/ForceToolAgentUpdateResponse.java:7
41 🟢 90 high ForceToolAgentUpdateResponseItem missing Lombok quartet (only @DaTa) openframe-api-service-core/src/main/java/com/openframe/api/dto/force/response/ForceToolAgentUpdateResponseItem.java:5

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.


Run: https://product-hub.flamingo.so/admin/code-review
Run id: 3bcc34c5-a1d3-4527-ae23-0142cbfa3178

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akeeczr OpenFrame lib batch review findings sweep 2 (13 PRs)

flamingo Bot added 30 commits September 14, 2026 05:42
@flamingo flamingo Bot changed the title fix(OFJAVA-033): 41 review findings across 30 files fix(OFJAVA-033): CU-86akeeczr 41 review findings across 30 files Sep 14, 2026
@michaelassraf

Copy link
Copy Markdown
Contributor

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.

1 participant