ci: restrict Develocity cache writes to trusted events - #6550
Merged
Conversation
github-actions
Bot
added
bugfix
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds firmware-gated LoRa regions and presets, updates regional frequency calculations, filters unsupported settings options, adds model tests, and restricts remote build-cache writes to selected CI events. ChangesLoRa capability and frequency support
CI cache write restrictions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsUI as LoRaConfigItemList
participant Capabilities
participant RegionInfo
SettingsUI->>Capabilities: Check region and preset support
Capabilities->>RegionInfo: Read minimum firmware metadata
RegionInfo-->>Capabilities: Return support status
Capabilities-->>SettingsUI: Return filtered options
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt (1)
111-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the duplicated
"2.8.0"firmware-version constant.
ChannelOption.ktandCapabilities.kteach declare a separateDeviceVersion("2.8.0")constant for the same firmware release. Keeping two independent literals for the same version risks drift if the value is ever corrected or the release is renumbered.
core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt#L111-L116: keepFIRMWARE_2_8as the single source of truth, or move a shared constant to a common location.core/model/src/commonMain/kotlin/org/meshtastic/core/model/Capabilities.kt#L112-L119: replace the privateV2_8_0declaration with a reference to the shared constant instead of a secondDeviceVersion("2.8.0")literal.🤖 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 `@core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt` around lines 111 - 116, Consolidate the duplicated 2.8.0 firmware version by keeping ChannelOption.kt’s FIRMWARE_2_8 as the shared source of truth and removing the separate V2_8_0 DeviceVersion declaration in Capabilities.kt; update Capabilities’ references to use FIRMWARE_2_8, with no direct change required in ChannelOption.kt.feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/LoRaConfigItemList.kt (1)
104-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark the retained-but-unsupported region as disabled, matching
buildPresetItems.
buildPresetItemsexplicitly appends the current illegal preset withenabled = falseso the UI communicates it cannot be freely re-selected.buildRegionItemsretains the current unsupported region (Line 110) but returns a plainPair<RegionCode, String>with no way to convey it is disabled, so it appears as a normal, fully selectable dropdown entry. This can mislead the user into thinking the region is valid for the connected firmware.Align the two patterns, for example by having the region dropdown consume
DropDownItemas the preset dropdown does, and disabling the retained-but-unsupported entry.🤖 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 `@feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/LoRaConfigItemList.kt` around lines 104 - 112, Update buildRegionItems to return the same DropDownItem-based type used by buildPresetItems, marking the selected region disabled when capabilities.supportsRegion returns false while leaving supported regions enabled. Update the region dropdown consumer to use this richer item type so the retained unsupported selection is visibly non-selectable.
🤖 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 `@core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt`:
- Around line 84-98: Update LoRaConfig.channelNum() so the overrideSlot branch
bounds or coerces the regional override to the current numChannels before
returning it. Preserve the existing channel_num priority, zero-channel behavior,
and hashed fallback, while ensuring the returned override-based channel is valid
for the selected preset.
---
Nitpick comments:
In `@core/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.kt`:
- Around line 111-116: Consolidate the duplicated 2.8.0 firmware version by
keeping ChannelOption.kt’s FIRMWARE_2_8 as the shared source of truth and
removing the separate V2_8_0 DeviceVersion declaration in Capabilities.kt;
update Capabilities’ references to use FIRMWARE_2_8, with no direct change
required in ChannelOption.kt.
In
`@feature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/LoRaConfigItemList.kt`:
- Around line 104-112: Update buildRegionItems to return the same
DropDownItem-based type used by buildPresetItems, marking the selected region
disabled when capabilities.supportsRegion returns false while leaving supported
regions enabled. Update the region dropdown consumer to use this richer item
type so the retained unsupported selection is visibly non-selectable.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a23fb99-5c9f-4779-89be-b7bb8769e75e
📒 Files selected for processing (6)
build-logic/settings-plugin/src/main/kotlin/MeshtasticDevelocitySettingsPlugin.ktcore/model/src/commonMain/kotlin/org/meshtastic/core/model/Capabilities.ktcore/model/src/commonMain/kotlin/org/meshtastic/core/model/ChannelOption.ktcore/model/src/commonTest/kotlin/org/meshtastic/core/model/CapabilitiesTest.ktcore/model/src/commonTest/kotlin/org/meshtastic/core/model/RegionInfoTest.ktfeature/settings/src/commonMain/kotlin/org/meshtastic/feature/settings/radio/component/LoRaConfigItemList.kt
jamesarich
enabled auto-merge
August 3, 2026 00:52
jamesarich
disabled auto-merge
August 3, 2026 01:07
Remote cache writes were gated on CI plus a non-empty DEVELOCITY_ACCESS_KEY.
A same-repository pull request DOES receive repository secrets, so PR builds
were writing entries into the shared cache — unmerged code could serve results
to main.
Require GITHUB_EVENT_NAME to be push or merge_group as well. Cache population is
unaffected: main-check.yml (push to main) and merge-queue.yml (merge_group) are
both still trusted writers, and they are the runs whose outputs correspond to
code that actually landed. pull-request.yml becomes pull-only, which is what it
should have been.
Verified against a CI-shaped environment:
CI=true GITHUB_EVENT_NAME=pull_request -> pull-only
CI=true GITHUB_EVENT_NAME=push -> writes enabled
CI=true GITHUB_EVENT_NAME=merge_group -> writes enabled
Fork PRs have no key and are excluded twice over; local builds are excluded by
isCI.
This restores the protection the self-hosted HttpBuildCache had before #6531 —
it gated on GITHUB_EVENT_NAME and excluded pull_request. The same gap was found
and fixed across the six repos being onboarded now (meshtastic/kzstd#36 and
siblings); this brings android back in line.
jamesarich
force-pushed
the
fix/develocity-cache-push-gate
branch
from
August 3, 2026 01:08
6673cd1 to
7d3a6af
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull-request builds are currently writing to the shared Develocity remote build
cache. They should not be.
The gap
MeshtasticDevelocitySettingsPlugingates cache writes on CI plus a non-emptyaccess key:
A same-repository pull request does receive repository secrets, so
pull-request.ymlruns satisfy both conditions and push entries into the cachethat
mainand the merge queue then read. Unmerged code can serve build outputsto trusted builds.
This is a regression introduced in #6531. The self-hosted
HttpBuildCachethatpreceded it gated on the event and excluded pull requests:
That protection was lost when the cache moved to
remote(develocity.buildCache).The fix
Require
GITHUB_EVENT_NAMEto bepushormerge_groupas well.Cache population is unaffected.
main-check.yml(push tomain) andmerge-queue.yml(merge_group) remain trusted writers — and they are the runswhose outputs correspond to code that actually landed, which is exactly what the
cache should contain.
pull-request.ymlbecomes pull-only, so PRs still get thefull read benefit.
Verification
Run against a CI-shaped environment rather than reasoned about:
./gradlew spotlessCheck detektgreen (185 tasks). Settings-plugin-only change,so no app code is touched.
Context
Found while onboarding the rest of the org's Gradle repos to the same
configuration — I copied this plugin's shape into six repos and CodeRabbit
caught the missing gate there. Fixed in all six
(meshtastic/kzstd#36, meshtastic/gradle-flatpak-sources#28,
meshtastic/MQTTastic-Client-KMP#118, meshtastic/meshtastic-sdk#91,
meshtastic/protobufs#1027, meshtastic/TAKPacket-SDK#124); this brings android
back in line with them.
Summary by CodeRabbit