[andrii-flamingo]

ClickUp task: CU-17tkuw5zc77 BE-7: Final state. Polishing. Integration with FE and Agent

• Removed package: when the agent answers NOT_INSTALLED to an auto-update order for the current version, that device's policy is turned off. Before, every release ordered the package again and failed. Updating the package again with Auto-update on turns the policy back on. Admin orders, other failure reasons and events for an older version leave the policy on.
• Brew type on schedules: creating or updating a software schedule now rejects a brew package without a type, as bundles already do. Without a type, the catalog could return the version of either a formula or a cask with the same name.
• One GraphQL error per field: GraphQLExceptionHandler now sets the error path. Without it, graphql-java added a second "non null type" error on non-null fields. Every GraphQL error from this API now carries path.

Not yet checked on a device: whether the agent's NOT_INSTALLED is reliable. BE-7 covers it.

Change-Set: fix-software-update-leftovers

Change set flamingo-stack/fix-software-update-leftovers: this pull request is the only one in it so far. Another pull request joins by naming this one in a Depends-On line, or by carrying the same Change-Set line.

• flamingo-stack/openframe-oss-lib#2929 (this pull request): fix(software-update): turn off auto-update for a removed package, require a brew type on schedules, answer one GraphQL error per field CU-17tkuw5zc77 · landed

Linked work

• …and 1 linked item not shown here

Linked by the Depends-On / Change-Set lines in these descriptions; this block is maintained by the hub.

[andrii-flamingo]

@flaimngo-review full

[github-actions]

🦩 Flamingo Code Review

✅ Tests and checks passed on this commit (2 lanes).

1 finding: 🔴 0 action required · 🟠 1 recommended · 🔵 0 informational

Advisory: findings do not block the merge.

1 defect outside any rule

Files	Lines	Languages	Tests	🔴 Action required	🟠 Recommended	🔵 Informational
13	+188 / -10	Java (1 Maven lane)	✅ 2994 / 2994 green (270 skipped)	0	1	0

🟠 Recommended (1)

#	Where	Finding	Flagged by
1	PackageAutoUpdateRetry.java:58	Auto-update turns off silently on a single NOT_INSTALLED answer	Tests and mutation

✅ What this change does well

• PackageAutoUpdateRetryTest.java adds thorough parameterized and edge-case coverage (older version, already-off policy, null request, wrong source) for the new turnOffIfPackageRemoved behavior.
• GraphQLExceptionHandlerDgsTest.java and the schema addition verify end-to-end that a non-null field failure now produces exactly one error at the correct path, which directly targets the bug the diff fixes.
• SoftwareScheduleServiceTest.java covers both create and update paths for the new brew-package-type validation, asserting the offending package name appears in the error message.
• PackageAutoUpdateRetry.turnOffIfPackageRemoved reuses currentPolicy, so an admin order, a stale event for an older version and an already disabled policy all leave the policy alone.
• GraphQLExceptionHandler fixes the duplicate non-null error at its cause by setting the error path, with GraphQLExceptionHandlerDgsTest proving one error through real GraphQL execution.

Inline comments: 1 new

5 possible problems checked and ruled out

• openframe-client-core/src/main/java/com/openframe/client/listener/packageupdate/PackageUpdateStatusProcessor.java:71 scheduleIfDue and turnOffIfPackageRemoved each independently look up the current policy: scheduleIfDue's isDue() requires reason in RETRIED_REASONS (MANAGER_ERROR, TIMEOUT, CLOSE_FAILED, NOT_DELIVERED) while turnOffIfPackageRemoved requires NOT_INSTALLED, which is mutually exclusive, so in practice only one of the two calls ever reaches the repository lookup for a given event.
• openframe-client-core/src/main/java/com/openframe/client/listener/packageupdate/PackageAutoUpdateRetry.java:64 Method calls passed inline as arguments to policyRepository.disable: policy.getId() and Instant.now() are trivial accessor/factory calls passed as arguments, the same pattern used throughout this file's log.info calls; this is the kind of pedantic nitpick a senior engineer would not raise despite the literal rule text, and Instant.now() is comparable to a factory call commonly exempted in practice.
• openframe-api-lib/src/main/java/com/openframe/api/service/rmm/software/SoftwareScheduleService.java:280 Compound filter predicate in toPackages() is inlined instead of named: A simple filter predicate with one && inside a stream is a minor stylistic nit; not a functional problem and extraction into a predicate adds no real clarity here.
• openframe-client-core/src/main/java/com/openframe/client/listener/packageupdate/PackageAutoUpdateRetry.java:58 Compound guard condition in turnOffIfPackageRemoved() is not extracted to a named predicate: Standard guard-clause with || is idiomatic and readable; extracting to a named predicate is a pure style nitpick that a senior engineer would not block on.
• openframe-client-core/src/main/java/com/openframe/client/listener/packageupdate/PackageAutoUpdateRetry.java:63 Multi-line lambda body in ifPresent is not extracted to a named method: The ifPresent lambda is two short, directly related statements (disable + log) with no real logic; extracting it to a separate method would not improve stack traces or clarity meaningfully.

🧪 Tests and mutation

Tests per lane

Language	Project	Command	Status	Counts	Time
Java (Maven)	.	mvn -B test -pl openframe-api-lib,openframe-api-service-core,openframe-client-core,openframe-data-mongo-sync -am	✅ passed	openframe-agent-key 4 · openframe-api-lib 603 · openframe-api-service-core 760 (42 skipped) · openframe-client-core 383 (3 skipped) · openframe-core 150 · openframe-core-crypto 8 · openframe-data-cassandra 20 · openframe-data-kafka 35 · openframe-data-loki 96 · openframe-data-mongo-sync 468 (199 skipped) · openframe-data-nats 163 (4 skipped) · openframe-data-pinot 69 · openframe-exception 4 · openframe-graphql-node 76 · openframe-graphql-relay 69 · openframe-machine-delivery 147 · openframe-notification-core 103 (22 skipped) · openframe-notification-mail 5 · openframe-presence 12 · openframe-security-core 38 · fleetmdm 51 · 0 failures	170.6 s

Java tests: 3264 run, 0 failed, 270 skipped; openframe-api-lib mutation ran without the tests in openframe-api-service-core, so its 4 no-coverage mutants are a module boundary, not missing tests.

PackageAutoUpdatePolicyRepositoryIT, which covers the new disable query, needs integration.tests=true: it passed locally on the earlier commit of this branch, 7 of 7.

Mutation testing

This build has no PIT plugin, so each module is mutated from the command line with pit-adhoc.sh, changing nothing in the repository.

Module	Classes	Whole classes	New lines only	Time
api-lib	SoftwareScheduleService	83 mutants, 0 killed (0%), 83 no coverage, line coverage 2%	4 mutants: 4 no coverage	13 s
api-service-core	GraphQLExceptionHandler	20 mutants, 19 killed (95%), 1 survived, test strength 95%, line coverage 100%	3 mutants: 3 killed	17 s
client-core	PackageAutoUpdateRetry, PackageUpdateStatusProcessor	78 mutants, 69 killed (88%), 3 survived, 6 no coverage, test strength 96%, line coverage 98%	5 mutants: 5 killed	15 s
data-mongo-sync	PackageAutoUpdatePolicyService	4 mutants, 4 killed (100%), test strength 100%, line coverage 100%	none on added lines	12 s

New code, its covering tests and the result

New code	Covering tests	Result
lambda$toPackages$0 in SoftwareScheduleService, line 281	none	no coverage (line 281)
toPackages in SoftwareScheduleService, line 284	none	no coverage (line 284)
handleException in GraphQLExceptionHandler, lines 40-78	GraphQLExceptionHandlerTest (3), 10 more	killed
turnOffIfPackageRemoved in PackageAutoUpdateRetry, lines 59-63	PackageAutoUpdateRetryTest (3), 3 more	killed
process in PackageUpdateStatusProcessor, line 72	PackageUpdateStatusProcessorTest (3), 8 more	killed

The covering tests by name (3 methods)

• GraphQLExceptionHandler.handleException: GraphQLExceptionHandlerTest.handleException_anonymousCallerRefused_reportsUnauthorizedWithAFixedMessage, GraphQLExceptionHandlerTest.handleException_signedInCallerRefused_reportsForbiddenWithAFixedMessage, GraphQLExceptionHandlerTest.handleException_signedInCallerRefused_logsTheReasonAsAWarningOnly, and 10 more
• PackageAutoUpdateRetry.turnOffIfPackageRemoved: PackageAutoUpdateRetryTest.noRequest_policyStaysOn, PackageAutoUpdateRetryTest.packageNotOnTheDevice_policyTurnedOff, PackageAutoUpdateRetryTest.otherFailure_policyStaysOn, and 3 more
• PackageUpdateStatusProcessor.process: PackageUpdateStatusProcessorTest.failedStatus_policyDecidedBeforeTheStateIsSaved, PackageUpdateStatusProcessorTest.historyRowWriteFails_stateIsNotSaved_soTheRedeliveryIsNotTakenForADuplicate, PackageUpdateStatusProcessorTest.statusArrivingAfterUndelivered_stillApplies, and 8 more

What the surviving mutations leave unverified

• SoftwareScheduleService.java:280-287: Mutation reports no coverage only because the class is in openframe-api-lib and its tests are in openframe-api-service-core: SoftwareScheduleServiceTest.createBrewWithoutTypeRejected and updateBrewWithoutTypeRejected reach these lines, but the mutants were not run against them, and a brew package with a type next to one without is not asserted separately.

4 mutants on lines this change added are not caught (0 survived, 4 with no test reaching them): each is annotated on its line in the Flamingo Code Review / tests and mutation check.

1 gap (not tested here):

• Java (Maven) .: PackageAutoUpdatePolicyRepository.java changed but not mutation-tested (outside the mutation targets openframe-data-mongo-sync configures)

🤖 Prompt for AI agents: fix every finding

Address these code-review findings on pull request #2929 of flamingo-stack/openframe-oss-lib, most important first.
For each one, make the minimal change that resolves it and do not refactor unrelated code.
If you can show a finding is wrong, skip it and say why.

1. [Recommended] openframe-client-core/src/main/java/com/openframe/client/listener/packageupdate/PackageAutoUpdateRetry.java:58 Auto-update turns off silently on a single NOT_INSTALLED answer
   One NOT_INSTALLED answer to an AUTO order disables the device's policy, and the only trace is a log line and updatedBy. Whether the agent's probe can report NOT_INSTALLED for a package that is installed (for example for another user) needs verification on a device.

---

Next: commit a suggestion, or hand a comment's prompt to your coding agent. React 👍 on a finding that helped and 👎 on one that is wrong: the reviewer learns from both.

Review again. New commits are not reviewed until you ask:

• Review the new commits: only what was pushed since this review
• Review the whole diff again: everything, including what was already reviewed

Or comment @flamingo-review (new commits) or @flamingo-review full (everything). Add the flamingo-review-always label to review every push.

Updated 2026-11-02 18:28 UTC · reviewed from Andrii Koropets's machine