ci: add Android PR checks and increase test coverage#5
Conversation
- Add ktlint plugin (org.jlleitschuh.gradle.ktlint) to all subprojects - Add Android Lint config with checkDependencies for full coverage - Add JUnit 5 dependencies to app, core/domain, core/data modules - Create .editorconfig for ktlint rules (indent 4, max line 120) - Create android-pr.yml workflow with parallel jobs: ktlint, lint-termux, lint-embedded, test-termux, test-embedded - Add gate jobs to android-build.yml and pr.yml - Add golangci-lint job to pr.yml - Replace build.yml with go-build.yml Co-Authored-By: Claude Opus 4.6 <[email protected]>
Address Go linter (errcheck, printf) and Android lint (ktlint, deprecated APIs) warnings without changing any runtime behavior. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add unit tests for Go packages (agent, bus, channels, config, constants, cron, skills, utils) and Android modules (app, backend, core, feature). Fix stripFrontmatter bug where multi-line YAML frontmatter was not stripped due to missing (?s) flag in regex. Fix .gitignore pattern 'clawdroid' matching paths containing 'io/clawdroid/' by scoping to root-only '/clawdroid'. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、Android開発ワークフローと全体的なコード品質を大幅に向上させることを目的としています。堅牢なCI/CDチェックの導入、広範なユニットテストの追加、およびいくつかの重要なバグとコード衛生の問題への対処を通じて、プロジェクトの安定性と保守性が強化されました。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
このPRは、AndroidのCI/CDパイプラインの追加、GoとAndroid両方でのテストカバレッジの大幅な向上、そして多数のバグ修正とリファクタリングを含んでおり、素晴らしい改善です。特に、40件近くの新しいテストファイルを追加した点は、プロジェクトの品質と安定性を大きく向上させるものであり、高く評価します。
コード全体として、ktlintの適用や未使用コードの削除、AccessibilityNodeInfoのメモリリーク修正など、多くの改善が見られます。
いくつか、エラーハンドリングに関する改善点を提案します。pkg/agent/loop.goでのセッション保存、pkg/tools/cron.goでのcronジョブ更新、pkg/tools/exit.goでの終了シグナル送信において、エラーが無視されています。これらのエラーを適切に処理することで、アプリケーションの堅牢性がさらに向上すると考えられます。
詳細は各コメントをご確認ください。
golangci-lint v1.x is built with Go 1.24 which is incompatible with the project's Go 1.25.6. golangci-lint v2.4.0+ supports Go 1.25. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
golangci-lint-action@v6 only supports golangci-lint v1. v7+ is required for golangci-lint v2, and v9 is the latest. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The lint job needs go generate to copy the workspace/ directory required by //go:embed, matching the vet and test jobs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add explicit error return handling for Close, Remove, Write, Setenv - Fix capitalized error strings (ST1005) - Fix trailing newline in error string (ST1005) - Remove duplicate import of telegohandler (ST1019) - Replace WriteString(Sprintf) with Fprintf (QF1012) Co-Authored-By: Claude Opus 4.6 <[email protected]>
Fix additional errcheck violations in loop_test, heartbeat_test, skills_test, media_test, edit, web, and skills tools. Replace remaining WriteString(Sprintf) with Fprintf in mcp and skills. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Merge duplicate parameter descriptions in buildParameters (#1) - Use map for DisabledActions lookup in isActionEnabled (#2) - Add URL scheme whitelist (http/https only) for open_url (#3) - Escape SQL LIKE wildcards in ContactsActionHandler (#4) - Fix JSON tag inconsistency: web_actions -> web (#5) - Add comprehensive tests for android tool (#6) Co-Authored-By: Claude Opus 4.6 <[email protected]>
* feat: extend android tool with category-based action system Split monolithic android tool into category-based modules (alarm, calendar, contacts, communication, media, navigation, device control, settings, web, clipboard) with per-category enable/disable config. Privacy-sensitive categories (contacts, communication) default to off. Closes #24 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review feedback - Merge duplicate parameter descriptions in buildParameters (#1) - Use map for DisabledActions lookup in isActionEnabled (#2) - Add URL scheme whitelist (http/https only) for open_url (#3) - Escape SQL LIKE wildcards in ContactsActionHandler (#4) - Fix JSON tag inconsistency: web_actions -> web (#5) - Add comprehensive tests for android tool (#6) Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: replace DisabledActions with struct-based per-action toggles Replace the DisabledActions string slice with typed action structs (AlarmActions, CalendarActions, etc.) grouped under AndroidActions. Each action is now a bool field with a label tag, so the schema system auto-generates individual toggles in the settings UI. - Add 10 category action structs + AndroidActions wrapper to config - Update migrateV2ToV3 to convert old DisabledActions to new struct - Add depth field to SchemaField for hierarchical UI rendering - Hide DisabledActions from UI via empty label tag - Update Android app to use depth-based indentation in config screen Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: unify categories and actions into per-category structs Merge the separate Categories/Actions structure into unified category wrappers (e.g. AlarmCategory with Enabled + Actions). This makes each category toggle and its actions appear together in the settings UI with proper depth-based hierarchy. Android UI now disables child action toggles when the parent category is turned off, providing clear visual feedback. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: propagate section-level enabled toggle to all child fields When the top-level "Enabled" toggle (e.g. Android tool) is turned off, all child categories and actions are now greyed out in the settings UI. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: use depth instead of group to detect section-level toggle The section-level "Enabled" field (e.g. android.enabled) has a non-empty group ("Android"), so the previous group.isEmpty() check never matched. Use depth <= 1 to identify section toggles and depth > 1 for category toggles. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add runtime permission requests before tool action execution Add permission checking/requesting before ActionHandler execution so users see permission dialogs instead of SecurityException errors. Runtime permissions (calendar, contacts, location) use PermissionRequester with suspendCancellableCoroutine to await the dialog result. Special permissions (DND, write settings) open the settings screen with an error message since they cannot be awaited programmatically. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: use ContentResolver.insert for create_event instead of Intent Replace the Intent-based calendar event creation (which opens the calendar app's add screen) with direct ContentResolver.insert() for consistency with update_event, delete_event, and add_reminder. Automatically selects the primary or first writable calendar as default. WRITE_CALENDAR permission is now required and requested via the permission dialog. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add calendar picker for selecting target calendar account Allow users to choose which calendar to use for write actions (create_event, update_event, add_reminder) instead of always auto-detecting the primary calendar. On first use without a configured calendar, a picker dialog is shown and the selection is saved to config. The setting is also editable from the config UI. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: remove liveCfg from AndroidTool and redesign calendar picker with Compose Remove runtime config mutation (saveCalendarIdToConfig) from CalendarActionHandler — calendar_id is now a value copy from config, changed via settings screen and reflected on restart. Replace android.app.AlertDialog in CalendarPickerActivity with Compose AlertDialog using ClawDroidTheme colors. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: change calendar setting label from auto-detect to pick each time When calendar_id is empty, the picker launches every write action. The label should reflect this behavior instead of implying auto-detection. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add input validation and race condition guards for security hardening - Add phone number and email validation (Go + Kotlin) to reject injection attempts - Validate intent extras to only allow primitive types (reject nested maps/arrays) - Return error for invalid settings sections instead of silent fallback - Add URL scheme validation (http/https only) in WebActionHandler - Fix BroadcastReceiver double-unregister race with AtomicBoolean guard - Add comprehensive tests for all new validations Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
📝 Description
Android CI/CDパイプライン追加、テストカバレッジ大幅向上、および複数のバグ修正を含む統合PR。
主な変更:
stripFrontmatterの正規表現).gitignoreのパターン修正(Androidテストファイルが誤って除外されていた問題)🗣️ Type of Change
🔗 Linked Issue
N/A
📚 Technical Context (Skip for Docs)
pkg/skills/loader.goのstripFrontmatter正規表現バグ((?s)フラグ欠落)を修正。🧪 Test Environment & Hardware
📸 Proof of Work (Optional for Docs)
Click to view Logs/Screenshots
Go tests:
go test ./...- all passingAndroid tests:
./gradlew testTermuxDebugUnitTest- all passing☑️ Checklist