Skip to content

Commit 5039404

Browse files
authored
FullCharge: Cancel a session only on a real native settings change (#65)
The session service's native-change observer cancelled without restoring on ANY notification for the adapter's setting URIs. Notifications are dispatched asynchronously, so the session's own override write can arrive after the observer registers, and an OEM provider can notify without a value change — both observed on HyperOS 3 tanzanite (issue #48), where the session ended mid-charge and the protective policy was never restored. NativeChangeGuard now verifies the change where the configuration is synchronously readable: a readback still decoding to the session's override policy is noise and keeps the session; a different, foreign, or unreadable value cancels as before. Adapters without sync readback (Pixel) keep the blanket cancel. A Shizuku readback timeout counts as unreadable instead of silently dropping the notification. Also records the tanzanite beta verification results in the qualification ledger: absent key decodes as Intelligent (confirmed), app-context three-mode control works over direct WSS, and the mode-2 cap is fixed at 80%.
1 parent b29b2ef commit 5039404

6 files changed

Lines changed: 181 additions & 14 deletions

File tree

.claude/rules/architecture.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,12 @@ ordering, session overrides) lives in the **`oem-adapters` skill** — read it b
6969
override), expiry or a backwards clock restores as before. `full` and the 24h safety timeout keep priority. On
7070
every other adapter `replugGraceMillis` is 0 and the decision table is unchanged.
7171
- While active, the service watches the adapter's settings URIs; an unexpected native/system change **cancels without
72-
restoring**, so Amply never overwrites a newer external choice.
72+
restoring**, so Amply never overwrites a newer external choice. Cancellation requires a **real** change
73+
(`NativeChangeGuard`): settings notifications are dispatched asynchronously, so the session's own override write can
74+
arrive after the observer registers, and an OEM provider may notify without any value change (both observed on
75+
HyperOS 3 `tanzanite`, issue #48 — blind cancellation ended the session with the protective policy never restored).
76+
On sync-readback adapters a notification whose readback still decodes to the session override is ignored as noise;
77+
without sync readback (Pixel) any notification still cancels.
7378
- Boot recovery runs the restore *inside the service* with a bounded convergence check (re-write until the HAL
7479
confirms or budget expires), because a boot-time write can race the observer registration. The pending target is
7580
persisted so a killed service resumes.

.claude/skills/device-qualification/SKILL.md

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ only after adding a row here. Detailed run narratives live in each adapter's lan
5151
| Xiaomi | Xiaomi 13T `2306EPN60G` HyperOS 2.0 (`ro.mi.os.version.code=2`) | **Partial** — mapping/readback/session verified; the adaptive 80% hold could not be triggered, so daemon-level hardware enforcement is **not yet demonstrated** | Read matrix, both-direction writes, session at 100%, unknown-value refusal, R8 beta | 2026-07-21 |
5252
| OnePlus (Oplus) | OnePlus Nord CE4 Lite `CPH2621` ColorOS 15 (`ro.build.version.oplusrom=V15.0.0`) | Full — enforcement directly observable (device holds at 80%); external writes stick | Two mutually-exclusive `system` keys (Charging limit / Smart charging), WSS-only write rejected + Shizuku write succeeds for all three policies, WSS-only UX (controls disabled + Shizuku-required banner) | 2026-07-21 |
5353
| GrapheneOS | Pixel 9 Pro XL `komodo`, GrapheneOS 2026080501 / Android 17 — **REMOTE qualification via issue #49** (tester-run protocol, not maintainer hardware) | **Enforcement observed**: held at 80% with shield, `dumpsys battery` status=4/Charging state=4/policy=2 (limit on) vs 2/1/1 (off); shell-UID writes move the Settings UI live, **latch at plug-session start** — mid-session writes have no hardware effect until unplug→replug, replug reliably applies the current value | Key isolation (`settings list` diff → single `global battery_charge_limit` 0/1), write→UI both directions, mid-session no-op both directions, replug latch both directions, hardware signal both states. **NOT run**: app-context access tiers (WSS write from Amply, `app.grapheneos.*` package visibility), sessions/boot recovery, wireless, factory-absent key state, secondary user | 2026-08-12 |
54-
| Xiaomi (HyperOS 3) | Redmi Note 14 `24117RN76G` (`tanzanite`), HyperOS 3.0.302 / Android 16 (`ro.mi.os.version.code=3`) — **REMOTE qualification via issue #48** (contributor-run protocol, not maintainer hardware) | **Both-direction enforcement of EXTERNAL shell-UID writes observed** (the same write path as Amply's Shizuku service): `settings put … 2` below the cap → Battery protection active, Settings UI follows immediately, held at 80% for ~20 min under active use (voltage 4228 mV holding vs 4391 mV charging, charge counter 4283 vs 4341 corroborate; sysfs `current_now` permission-denied, so no current reading); `settings put … 0` mid-hold → charging resumes past 80 immediately. **No hardware hold signal**: `dumpsys battery` reports `status: 2` / `Charging state: 0` / `Charging policy: 0` in both states → read-back-only verification | Key mapping (three modes incl. `2` = Battery protection @80), external write → UI both directions, sustained hold, mid-hold release. **NOT run** (GrapheneOS-precedent landing; verify on the next beta via issue #48): app-context access tiers, sessions/boot recovery, factory-absent key state, wireless, R8 | 2026-08-14 |
54+
| Xiaomi (HyperOS 3) | Redmi Note 14 `24117RN76G` (`tanzanite`), HyperOS 3.0.302 / Android 16 (`ro.mi.os.version.code=3`) — **REMOTE qualification via issue #48** (contributor-run protocol, not maintainer hardware) | **Both-direction enforcement of EXTERNAL shell-UID writes observed** (the same write path as Amply's Shizuku service): `settings put … 2` below the cap → Battery protection active, Settings UI follows immediately, held at 80% for ~20 min under active use (voltage 4228 mV holding vs 4391 mV charging, charge counter 4283 vs 4341 corroborate; sysfs `current_now` permission-denied, so no current reading); `settings put … 0` mid-hold → charging resumes past 80 immediately. **No hardware hold signal**: `dumpsys battery` reports `status: 2` / `Charging state: 0` / `Charging policy: 0` in both states → read-back-only verification | Key mapping (three modes incl. `2` = Battery protection @80, cap fixed — no percent picker), external write → UI both directions, sustained hold, mid-hold release. Beta run 2026-08-14 added: app-context three-mode control (direct WSS), factory-absent key = Intelligent (confirmed). Session restore FAILED in that run (observer noise cancel — app bug, fixed; see Known gaps). **NOT run**: Shizuku tier, boot recovery, wireless, R8, session re-verify post-fix | 2026-08-14 |
5555
| GrapheneOS (follow-up) | Same device, **0.3.2-beta0 on-device report via issue #49** | **Package detection VERIFIED from app context** (`is_grapheneos=true` with the FLAG_SYSTEM check); **unprivileged key read DENIED**`has_battery_charge_limit=false` while the very same report showed `battery_charging_status=4` (limit enforcing). Root cause in GrapheneOS source: the key is `@Protected(read = SYSTEM_UI, readWrite = SETTINGS)` (frameworks_base `c30c6393`); SettingsProvider throws SecurityException for all other packages **including WSS holders**, with the shell UID explicitly exempt ("ADB is used for testing", `e87c93a2`) — so the tester's earlier adb runs ARE the Shizuku-path evidence. Factory-absent semantics resolved from source: `BoolSetting(..., default false)` → absent = off | Detection + fail-closed probe verified live; adapter re-gated to Shizuku-only in response | 2026-08-13 |
5656

5757
## Known gaps
@@ -190,15 +190,29 @@ only after adding a row here. Detailed run narratives live in each adapter's lan
190190
contributor was asked whether the settings screen shows any third mode / 80% option.
191191
- **LANDED 2026-08-14: `xiaomi-hyperos3-v1`, gated to a qualified-codename allowlist (`tanzanite` only) —
192192
GrapheneOS-precedent landing** (remote enforcement qualification via issue #48, see the Verified devices
193-
row; app-level items open, all failing closed, to be verified by the contributor on the next beta):
194-
- **Absent-key decode unverified**: absent decodes as Intelligent/Adaptive, mirroring HyperOS 2's
195-
factory-state assumption; HyperOS 3 factory semantics are unknown. Worst case if wrong: a session
196-
restore writes `1` onto a factory state that was not Intelligent — bounded, no battery hazard.
197-
Next-beta ask: `settings delete secure security_pc_secure_protect_mode_key`, then `settings get` +
198-
observe which mode the native UI shows.
199-
- **Sessions / boot recovery / access tiers (WSS-only vs Shizuku) / R8**: NOT RUN on HyperOS 3;
200-
the mechanism is the shared session engine + the same `secure`-namespace write path qualified on
201-
HyperOS 2, but on-device confirmation is pending the next beta.
193+
row). **First on-device verification run (2026-08-14, issue #48; contributor-run, presumably on
194+
v0.3.3-beta0 — the first release carrying the adapter):**
195+
- **Absent-key decode CONFIRMED**: `settings delete``settings get` returns `null`, and both the native
196+
battery settings and Amply fall back to Intelligent charging. Absent = Intelligent is the real factory
197+
semantic; the shipped decode is correct.
198+
- **App-context writes + three-mode switching CONFIRMED**: switching all three modes from inside Amply is
199+
mirrored by the system settings immediately and vice versa, with the key value following each change
200+
(contributor polled `settings get` per switch). Access tier in the run: direct WSS (dashboard showed
201+
"Read back through direct wss"); the Shizuku tier remains unexercised on this device.
202+
- **Temporary session FAILED to restore — real app bug, adapter-independent, FIXED post-run**: the
203+
session's native-change observer received a settings notification carrying no value change (the
204+
session's own override write delivered late by async dispatch, or a HyperOS spurious notification) and
205+
cancelled the session without restoring; the device charged to 100% with the protective policy never
206+
re-written. Root-caused from the run's artifacts — the contributor's 22:22 screenshot shows the
207+
dashboard already idle at 96% while their 2s key monitor shows no value change after the 21:59:08
208+
override write. Fixed by `NativeChangeGuard` (readback-verified cancellation, see
209+
`rules/architecture.md`); **session-lifecycle re-verification is the headline ask for the next beta**.
210+
Boot recovery and R8 smoke remain NOT RUN.
211+
- **Cap is fixed at 80%** (no percent picker in the Battery protection screen), and mode `2`'s own
212+
description says the device will "charge fully only when scheduled" — HyperOS reserves an OEM-side
213+
scheduled full charge while in Battery protection. No code impact (verification is settings readback;
214+
sessions override with `0`), but a future "charged to 100% while protected" report may be this OEM
215+
behavior rather than a bug.
202216
- **Adaptive (mode `1`) enforcement undemonstrated** — identical provisional status as HyperOS 2 (the
203217
top-level Xiaomi gap above); only mode `2` has demonstrated hardware enforcement.
204218
- The gate cannot widen past the codename allowlist: record any new HyperOS 3 device here plus a

app/src/main/java/eu/darken/amply/charging/core/adapter/XiaomiChargingAdapter.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,8 @@ class XiaomiHyperOs3ChargingAdapter @Inject constructor() : ChargingAdapter {
166166
)
167167
}
168168
return when (mode.value) {
169-
// Absent = intelligent mirrors the HyperOS 2 factory-state assumption but is
170-
// UNVERIFIED on HyperOS 3 — pending the issue-#48 qualification run's
171-
// factory/absent-key check; adjust if the evidence contradicts it.
169+
// Absent = intelligent, VERIFIED on tanzanite (issue #48, 2026-08-14): deleting the
170+
// key made the native battery settings fall back to Intelligent charging.
172171
null, XiaomiChargingAdapter.VALUE_INTELLIGENT ->
173172
ChargeObservation.Verified(ChargePolicy.Adaptive, backend.kind)
174173

app/src/main/java/eu/darken/amply/fullcharge/core/ChargeSessionService.kt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import kotlinx.coroutines.CoroutineScope
3131
import kotlinx.coroutines.Dispatchers
3232
import kotlinx.coroutines.Job
3333
import kotlinx.coroutines.SupervisorJob
34+
import kotlinx.coroutines.TimeoutCancellationException
3435
import kotlinx.coroutines.cancel
3536
import kotlinx.coroutines.delay
3637
import kotlinx.coroutines.launch
@@ -93,6 +94,51 @@ class ChargeSessionService : Service() {
9394
// fast-path check above but before we acquire the lock, so its own write must not be
9495
// mistaken for a native change and cancelled.
9596
if (restoring) return@withExclusive
97+
// Only cancel on a REAL native change. Notifications also arrive for the
98+
// session's own override write (async dispatch can outrun the observer
99+
// registration in beginOrResume) and, on some OEM providers, without any value
100+
// change at all — both fatal here, because cancelling ends the session with the
101+
// protective policy never restored (observed on HyperOS 3, issue #48). Where
102+
// the configuration is synchronously readable, a readback still matching the
103+
// override is that noise; without readback (Pixel) cancel as before.
104+
if (fullChargeStore.currentSession() != null) {
105+
val overridePolicy = repository.currentAdapter()?.sessionOverridePolicy
106+
?: ChargePolicy.Unrestricted
107+
// Holding the dispatch lock across the readback is bounded by the same
108+
// backend a restore would need anyway (worst case one cold Shizuku bind on
109+
// GrapheneOS; every other adapter reads direct).
110+
val readback = try {
111+
repository.syncReadback()
112+
} catch (e: TimeoutCancellationException) {
113+
// A Shizuku bind/command timeout is a failed READBACK, not this
114+
// observer being cancelled — rethrowing would silently drop the
115+
// notification and keep a session alive past a genuine native change.
116+
log(TAG, Logging.Priority.WARN) {
117+
"Readback for the native-change check timed out"
118+
}
119+
null
120+
} catch (e: CancellationException) {
121+
throw e
122+
} catch (e: Exception) {
123+
// Unverifiable counts as a change: same conservative end state as the
124+
// pre-guard blanket cancel.
125+
log(TAG, Logging.Priority.WARN) {
126+
"Readback for the native-change check failed: ${e.message}"
127+
}
128+
null
129+
}
130+
if (!NativeChangeGuard.shouldCancel(readback, overridePolicy)) {
131+
log(TAG) {
132+
"Ignoring settings notification; readback still matches the " +
133+
"session override: $readback"
134+
}
135+
return@withExclusive
136+
}
137+
log(TAG, Logging.Priority.INFO) {
138+
"Native settings change during session (readback=$readback); " +
139+
"cancelling without restore"
140+
}
141+
}
96142
// Respect a native Settings change instead of restoring over the user's choice.
97143
manager.cancelWithoutRestore()
98144
unregisterSettingObserver()

app/src/main/java/eu/darken/amply/fullcharge/core/SessionDecision.kt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package eu.darken.amply.fullcharge.core
22

3+
import eu.darken.amply.charging.core.ChargeObservation
34
import eu.darken.amply.charging.core.ChargePolicy
45
import eu.darken.amply.fullcharge.core.ChargeSessionRecord
56

@@ -50,6 +51,32 @@ object SessionStartDecider {
5051
}
5152
}
5253

54+
/**
55+
* Decides whether a settings-change notification observed during an active session is a genuine
56+
* native change (cancel the session without restoring) or noise (ignore, keep the session).
57+
*
58+
* Android dispatches settings notifications asynchronously, so the session's OWN override write can
59+
* be delivered after the observer registers, and an OEM provider may notify without any value
60+
* change at all — both observed on HyperOS 3 `tanzanite`, where blind cancellation ended the
61+
* session mid-charge and the protective policy was never restored (issue #48). Where the adapter's
62+
* configuration is synchronously readable, a notification whose readback still decodes to the
63+
* session's override policy is therefore treated as noise. Deliberate trade-off: a native change
64+
* that still decodes to the override policy — re-selecting the same value, or on multi-key
65+
* adapters editing an auxiliary key the decoded policy ignores (e.g. Samsung's threshold while the
66+
* PauseAtFull override is active) — is indistinguishable from that noise and keeps the session
67+
* running, and the later restore can overwrite such an auxiliary edit with pre-session values.
68+
* Accepted: only reachable by editing protection settings mid-session, and never a charge-safety
69+
* regression (the protective policy still comes back).
70+
*
71+
* Everything else cancels, preserving the previous blanket behavior: a different verified policy
72+
* is a real native change; an unrecognized or unreadable value cannot be attributed to the
73+
* session; and without sync readback (Pixel, [readback] null) nothing can be verified.
74+
*/
75+
object NativeChangeGuard {
76+
fun shouldCancel(readback: ChargeObservation?, overridePolicy: ChargePolicy): Boolean =
77+
!(readback is ChargeObservation.Verified && readback.policy == overridePolicy)
78+
}
79+
5380
enum class SessionDecision {
5481
CONTINUE,
5582
MARK_CONNECTED,
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package eu.darken.amply.fullcharge.core
2+
3+
import eu.darken.amply.charging.core.BackendKind
4+
import eu.darken.amply.charging.core.ChargeObservation
5+
import eu.darken.amply.charging.core.ChargePolicy
6+
import eu.darken.amply.common.ca.toCaString
7+
import io.kotest.matchers.shouldBe
8+
import org.junit.jupiter.api.Test
9+
10+
class NativeChangeGuardTest {
11+
12+
private val override = ChargePolicy.Unrestricted
13+
14+
private fun verified(policy: ChargePolicy, backend: BackendKind = BackendKind.DIRECT_WSS) =
15+
ChargeObservation.Verified(policy, backend)
16+
17+
@Test
18+
fun `readback matching the override is noise`() {
19+
NativeChangeGuard.shouldCancel(verified(ChargePolicy.Unrestricted), override) shouldBe false
20+
// The backend that produced the readback is irrelevant to the comparison.
21+
NativeChangeGuard.shouldCancel(
22+
verified(ChargePolicy.Unrestricted, BackendKind.SHIZUKU),
23+
override,
24+
) shouldBe false
25+
}
26+
27+
@Test
28+
fun `non-Unrestricted overrides compare by policy equality`() {
29+
// Samsung modern overrides with PauseAtFull instead of Unrestricted.
30+
NativeChangeGuard.shouldCancel(
31+
verified(ChargePolicy.PauseAtFull),
32+
ChargePolicy.PauseAtFull,
33+
) shouldBe false
34+
NativeChangeGuard.shouldCancel(
35+
verified(ChargePolicy.FixedLimit(80)),
36+
ChargePolicy.FixedLimit(80),
37+
) shouldBe false
38+
NativeChangeGuard.shouldCancel(
39+
verified(ChargePolicy.FixedLimit(85)),
40+
ChargePolicy.FixedLimit(80),
41+
) shouldBe true
42+
}
43+
44+
@Test
45+
fun `a different verified policy is a real native change`() {
46+
NativeChangeGuard.shouldCancel(verified(ChargePolicy.FixedLimit(80)), override) shouldBe true
47+
NativeChangeGuard.shouldCancel(verified(ChargePolicy.Adaptive), override) shouldBe true
48+
}
49+
50+
@Test
51+
fun `unrecognized and unreadable values cancel`() {
52+
NativeChangeGuard.shouldCancel(
53+
ChargeObservation.Unknown("foreign value".toCaString(), unrecognizedValue = true),
54+
override,
55+
) shouldBe true
56+
NativeChangeGuard.shouldCancel(
57+
ChargeObservation.Unknown("unreadable".toCaString()),
58+
override,
59+
) shouldBe true
60+
}
61+
62+
@Test
63+
fun `no sync readback cancels like before`() {
64+
// Async adapters (Pixel) return null — the pre-guard blanket behavior stays.
65+
NativeChangeGuard.shouldCancel(null, override) shouldBe true
66+
}
67+
68+
@Test
69+
fun `only verified readback can rescue the session`() {
70+
// A last-requested claim is Amply's own journal, not an observation of the key.
71+
NativeChangeGuard.shouldCancel(
72+
ChargeObservation.LastRequested(ChargePolicy.Unrestricted),
73+
override,
74+
) shouldBe true
75+
}
76+
}

0 commit comments

Comments
 (0)