Skip to content

ci: fail Android PRs on ktlint formatting drift#101275

Merged
steipete merged 4 commits into
mainfrom
claude/funny-grothendieck-b7533a
Jul 7, 2026
Merged

ci: fail Android PRs on ktlint formatting drift#101275
steipete merged 4 commits into
mainfrom
claude/funny-grothendieck-b7533a

Conversation

@steipete

@steipete steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Android ktlint formatting drift keeps landing on main because CI never runs ktlintCheck: the android CI job only covers test-play, test-third-party, and build-play. This forced a manual sweep commit (#100304, "style(android): apply ktlint to landed changes"), and more unformatted Android PRs landed the same day. While preparing this PR, main had already drifted again: #101161 landed 4 ktlint violations in ShellScreen.kt.

Why This Change Was Made

Adds an android-ktlint lane to the existing android matrix job that runs ./gradlew --no-daemon --build-cache :app:ktlintCheck :benchmark:ktlintCheck, mirroring pnpm android:lint. It reuses the job's existing Java/Android SDK setup — ktlint itself is JVM-only, but the AGP modules need the SDK to configure, so a slimmer SDK-less job would not work. The first commit fixes the current ShellScreen.kt drift (via ./gradlew :app:ktlintFormat, formatting-only) so the lane is green from the start; the formatting shifted tracked line numbers, so the native i18n inventory is resynced in a follow-up commit (pnpm native:i18n:sync, line numbers only).

User Impact

No user-facing behavior change. PRs touching apps/android now fail CI on formatting drift instead of silently landing it, so maintainers no longer need periodic manual ktlint sweeps.

Evidence

Verified against main at ceb7a4a (Android tree identical to the ktlint-checked b2cecd8):

  • ./gradlew :app:ktlintCheck :benchmark:ktlintCheck on unmodified main FAILS with 4 violations in app/src/main/java/ai/openclaw/app/ui/ShellScreen.kt (introduced by fix(android): stabilize recent sessions overview #101161).
  • After the ktlintFormat-generated fix in this PR: BUILD SUCCESSFUL, both :app and :benchmark clean; ShellScreen.kt is the only file the formatter touched.
  • The new android-ktlint check ran end-to-end on this PR's first push and completed success — live proof the lane triggers for PRs touching apps/android (this PR edits a Kotlin file) and passes on formatted code.
  • pnpm native:i18n:check passes after the inventory resync (entries=2868 changed=false).
  • actionlint and YAML parse clean on the modified .github/workflows/ci.yml; the lane runs under the same run_android_job scope gate as the existing Android lanes.

Known unrelated failures from current main (present at ceb7a4a in files this PR does not touch): check-lint fails on src/skills/workshop/curator.ts:521 (oxc no-map-spread, from #101214) and on literal merge-conflict markers in ui/src/components/app-sidebar.ts (landed via #101191); checks-fast-bundled-protocol fails because the generated protocol clients were not regenerated for the new SkillsCuratorStatus* schema (#101214). Being fixed separately.

@openclaw-barnacle openclaw-barnacle Bot added app: android App: android size: XS maintainer Maintainer-authored PR labels Jul 7, 2026
@steipete
steipete force-pushed the claude/funny-grothendieck-b7533a branch from 6cd756a to 83ba328 Compare July 7, 2026 02:37
@clawsweeper

clawsweeper Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 6, 2026, 10:52 PM ET / 02:52 UTC.

Summary
The branch adds an android-ktlint row to the Android CI matrix, formats ShellScreen.kt, and resyncs native i18n line metadata.

PR surface: Config +5, Other -4. Total +1 across 3 files.

Reproducibility: yes. Current main's Android matrix omits ktlint while package.json and the Android README define ktlintCheck as the Android lint path, and live PR status shows the new row ran successfully.

Review metrics: 1 noteworthy metric.

  • Android CI matrix rows: 1 added, 0 removed; 3 -> 4 rows. The change adds one Blacksmith-backed Android row while keeping Android max-parallel at 2, which is the relevant CI capacity fact before merge.

Stored data model
Persistent data-model change detected: persistent cache schema: .github/workflows/ci.yml. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Add docs/ci.md coverage and a ci-workflow-guards assertion for the android-ktlint matrix row.
  • Refresh exact-head checks after bringing the branch up to current main.

Risk before merge

  • [P1] CI docs and guard tests still do not cover android-ktlint, so a future workflow refactor could silently drop the format gate.
  • [P1] The live PR is behind current main, so exact-head checks should refresh before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the ktlint lane after adding docs/ci.md coverage and a focused ci-workflow-guards assertion, while preserving Android max-parallel: 2 and the existing Gradle lint command.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The remaining gap is a narrow CI docs/test guard repair that an automated worker can attempt if maintainers promote it; protected labels still leave merge and close decisions with maintainers.

Security
Cleared: The diff adds one Gradle ktlint task in the existing Android CI job without changing permissions, action refs, secrets, dependencies, or download sources.

Review findings

  • [P3] Document and guard the Android ktlint lane — .github/workflows/ci.yml:407
Review details

Best possible solution:

Land the ktlint lane after adding docs/ci.md coverage and a focused ci-workflow-guards assertion, while preserving Android max-parallel: 2 and the existing Gradle lint command.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main's Android matrix omits ktlint while package.json and the Android README define ktlintCheck as the Android lint path, and live PR status shows the new row ran successfully.

Is this the best way to solve the issue?

Yes, with one completion gap. Reusing the existing Android CI job is the best owner-boundary fix because it already has the SDK/JDK setup; docs and a guard assertion should be added to make it durable.

Full review comments:

  • [P3] Document and guard the Android ktlint lane — .github/workflows/ci.yml:407
    Still unfixed from the prior review: this adds a fourth Android CI row, but docs/ci.md still says Android CI only runs the two unit-test lanes plus the Play debug build, and ci-workflow-guards only asserts max-parallel. Please document and assert android-ktlint so future matrix edits cannot silently drop the formatting gate.
    Confidence: 0.83

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 084cf2a43ddf.

Label changes

Label justifications:

  • P3: This is low-risk CI formatting enforcement with no product behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and live exact-head checks show android-ktlint completed successfully, with native i18n also passing after the metadata resync.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and live exact-head checks show android-ktlint completed successfully, with native i18n also passing after the metadata resync.
Evidence reviewed

PR surface:

Config +5, Other -4. Total +1 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 1 5 0 +5
Generated 0 0 0 0
Other 2 35 39 -4
Total 3 40 39 +1

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs test/scripts/ci-workflow-guards.test.ts.
  • [P1] node scripts/check-workflows.mjs.
  • [P1] git diff --check.

What I checked:

Likely related people:

  • steipete: Blame ties the current Android CI matrix to commit 5157446, and this PR's CI lane and formatting commits are also by this author. (role: recent area contributor and likely follow-up owner; confidence: high; commits: 5157446a2c35, 08af44b35e06, dd5f814c4564; files: .github/workflows/ci.yml, apps/android/app/src/main/java/ai/openclaw/app/ui/ShellScreen.kt, apps/.i18n/native-source.json)
  • Solvely-Colin: Blame shows the overviewRecentSessions helper being formatted by this PR came from merged recent-sessions stabilization commit 82106a1. (role: introduced formatted Android UI behavior; confidence: medium; commits: 82106a18b3be; files: apps/android/app/src/main/java/ai/openclaw/app/ui/ShellScreen.kt, apps/.i18n/native-source.json)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-07T02:46:29.122Z sha 83ba328 :: found issues before merge. :: [P3] Document and guard the Android ktlint lane

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 7, 2026
steipete added 4 commits July 6, 2026 20:51
Formatting drift landed via #101161; :app:ktlintCheck fails on main
without this. Generated with ./gradlew :app:ktlintFormat.
Android formatting drift kept landing on main because CI never ran
ktlintCheck (see the #100304 manual sweep). Adds an android-ktlint
matrix lane mirroring pnpm android:lint.
The ktlint format fix shifted line numbers tracked in
apps/.i18n/native-source.json; regenerated via pnpm native:i18n:sync.
…eyTest

Import-order/blank-line drift landed via the hardware-Enter changes
(#101321 and follow-up) while this PR was open — the exact recurrence
the new android-ktlint lane prevents. Generated with ktlintFormat;
native i18n inventory resynced for the shifted line numbers.
@steipete
steipete force-pushed the claude/funny-grothendieck-b7533a branch from 83ba328 to e3d17f2 Compare July 7, 2026 03:51
@steipete
steipete merged commit b337164 into main Jul 7, 2026
92 checks passed
@steipete
steipete deleted the claude/funny-grothendieck-b7533a branch July 7, 2026 04:01
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
* style(android): fix ktlint drift in ShellScreen.kt

Formatting drift landed via openclaw#101161; :app:ktlintCheck fails on main
without this. Generated with ./gradlew :app:ktlintFormat.

* ci(android): add ktlint lane to the Android CI job

Android formatting drift kept landing on main because CI never ran
ktlintCheck (see the openclaw#100304 manual sweep). Adds an android-ktlint
matrix lane mirroring pnpm android:lint.

* chore(i18n): sync native inventory after ShellScreen.kt format

The ktlint format fix shifted line numbers tracked in
apps/.i18n/native-source.json; regenerated via pnpm native:i18n:sync.

* style(android): fix new ktlint drift in NodeRuntime and ChatHardwareKeyTest

Import-order/blank-line drift landed via the hardware-Enter changes
(openclaw#101321 and follow-up) while this PR was open — the exact recurrence
the new android-ktlint lane prevents. Generated with ktlintFormat;
native i18n inventory resynced for the shifted line numbers.
sheyanmin pushed a commit to sheyanmin/openclaw that referenced this pull request Jul 8, 2026
* style(android): fix ktlint drift in ShellScreen.kt

Formatting drift landed via openclaw#101161; :app:ktlintCheck fails on main
without this. Generated with ./gradlew :app:ktlintFormat.

* ci(android): add ktlint lane to the Android CI job

Android formatting drift kept landing on main because CI never ran
ktlintCheck (see the openclaw#100304 manual sweep). Adds an android-ktlint
matrix lane mirroring pnpm android:lint.

* chore(i18n): sync native inventory after ShellScreen.kt format

The ktlint format fix shifted line numbers tracked in
apps/.i18n/native-source.json; regenerated via pnpm native:i18n:sync.

* style(android): fix new ktlint drift in NodeRuntime and ChatHardwareKeyTest

Import-order/blank-line drift landed via the hardware-Enter changes
(openclaw#101321 and follow-up) while this PR was open — the exact recurrence
the new android-ktlint lane prevents. Generated with ktlintFormat;
native i18n inventory resynced for the shifted line numbers.
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
* style(android): fix ktlint drift in ShellScreen.kt

Formatting drift landed via openclaw#101161; :app:ktlintCheck fails on main
without this. Generated with ./gradlew :app:ktlintFormat.

* ci(android): add ktlint lane to the Android CI job

Android formatting drift kept landing on main because CI never ran
ktlintCheck (see the openclaw#100304 manual sweep). Adds an android-ktlint
matrix lane mirroring pnpm android:lint.

* chore(i18n): sync native inventory after ShellScreen.kt format

The ktlint format fix shifted line numbers tracked in
apps/.i18n/native-source.json; regenerated via pnpm native:i18n:sync.

* style(android): fix new ktlint drift in NodeRuntime and ChatHardwareKeyTest

Import-order/blank-line drift landed via the hardware-Enter changes
(openclaw#101321 and follow-up) while this PR was open — the exact recurrence
the new android-ktlint lane prevents. Generated with ktlintFormat;
native i18n inventory resynced for the shifted line numbers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: android App: android maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant