fix: require Android contact and calendar write permissions in onboarding#99158
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 2:35 PM ET / 18:35 UTC. Summary PR surface: Other +45. Total +45 across 2 files. Reproducibility: yes. by source inspection: current main can mark Contacts or Calendar onboarding rows granted from READ_* even though those rows launch both read and write permissions and add handlers require write permission. I did not run a failing emulator repro in this read-only review. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the focused onboarding permission-state fix and keep Contacts/Calendar rows tied to shared read/write permission groups once normal maintainer merge gates pass. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main can mark Contacts or Calendar onboarding rows granted from READ_* even though those rows launch both read and write permissions and add handlers require write permission. I did not run a failing emulator repro in this read-only review. Is this the best way to solve the issue? Yes, this is the best fix: it updates the onboarding state owner that filters launch permissions and shares the same required permission groups for initial state, callback merge, and request launch. Changing Settings or handlers would not fix onboarding's stale row state. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 97ad0b7c211c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Other +45. Total +45 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🧂 unranked krab
c4bdc0e to
cf50b9f
Compare
|
Maintainer verification Rebased onto landed #99147 and removed the duplicate read/write helper. Contacts and Calendar now share one canonical permission group each across initial state, partial callback merging, and request launch. Validation:
Proof gap: reaching the onboarding permission screen requires a paired Gateway. I did not bypass onboarding for a cosmetic screenshot; the split system grants and shared callback boundary are the relevant proof. |
clawsweeper
Bot
added
proof: sufficient
|
Merged via squash.
|
…ding (openclaw#99158) * fix: require Android contact and calendar write permissions in onboarding * refactor(android): share required onboarding permission groups --------- Co-authored-by: NianJiuZst <180004567+users.noreply.github.com> Co-authored-by: Peter Steinberger <[email protected]>
2 participants
What Problem This Solves
Android onboarding could show Contacts or Calendar as authorized after only the read permission was granted, even though the app requests both read and write permissions from that row.
That left onboarding out of sync with the rest of the Android app: Settings already treats Contacts and Calendar as granted only when both read and write permissions are present, and the actual Contacts/Calendar add handlers require the write permission before they can create records.
This also fixes the filtered callback case. The onboarding request helper filters out permissions that are already granted before launching
RequestMultiplePermissions; if READ was granted first and WRITE was granted later, the callback map can contain only WRITE. The state merge now uses the callback result plus the current system grant state for the omitted permission.AI-assisted: yes.
Why This Change Was Made
The previous onboarding state used only
READ_CONTACTSandREAD_CALENDARfor initial grant status, and the ActivityResult callback also only looked at those read permissions.This change adds a focused read/write permission merge helper and uses it for Contacts and Calendar onboarding state. Initial state now requires both read and write permissions, and callback state now preserves already-granted permissions that were filtered out before the request was launched.
User Impact
Users will no longer see Contacts or Calendar marked as ready in Android onboarding until the permissions needed by both read and add workflows are actually available. Users who grant the two permissions in separate prompts should now see the row update correctly once the missing permission is granted.
Evidence
cd apps/android && ./gradlew :app:testThirdPartyDebugUnitTest --tests ai.openclaw.app.ui.OnboardingFlowLogicTestcd apps/android && ./gradlew :app:testPlayDebugUnitTest --tests ai.openclaw.app.ui.OnboardingFlowLogicTestgit diff --check.agents/skills/autoreview/scripts/autoreview --mode localautoreview clean: no accepted/actionable findings reported