Version the debug bundle, and move the system_server code into hook/ - #307
Merged
Conversation
`state.json` carried two schema numbers (one on the state, one on the report), both pinned at 1 since the format existed, and a KDoc saying compatibility was not a concern. A number that never moves tells a triager nothing about the file in front of them, and nothing stopped the shape from drifting between releases. One version for the whole bundle, and a golden file so it cannot go stale: the serialized shape is pinned in a test, which fails on any change and points at the refresh command and the bump rule. - give LsposedState and ProtectionCheck explicit @SerialName values. Without them kotlinx emits the fully-qualified class name as the `kind` discriminator, so those two blocks read "dev.okhsunrog.vpnhide.LsposedState.Active" while every other sealed type reads "active" — and the value would silently change if the class ever moved package - drop DiagnosticReport.schema; the top-level one is the bundle's version - bump it to 2 and document the history + bump rules in docs/debug-bundle.md §2.1, with the current schema in the triage list - BundleSchemaGoldenTest pins the encoded shape (including the dashboard block, the only path those two sealed types reach) and asserts no fully-qualified name reaches the wire
96 files sat flat in one package, which hid the only boundary in this module with a real failure mode: `hook/` is loaded by LSPosed into system_server, everything else runs in the app process. Nothing enforced it — `internal` is module-wide, so a hook file referencing a Compose screen compiles fine and fails as a system_server crash on a device. The seam was already clean (no Compose in any Xposed-importing file), so this is a pure move: 7 files, no logic touched, 12 imports added. - hook/: HookEntry, PackageVisibilityHooks, SystemServerConfigCache, HookLog, HookReflectionProbe, ConnectivityAttachDiagnostics and the shared /data/system watcher the three of them use - shared vocabulary stays at the root package (HookRegistry, LsposedStats, canonical-config parsing, LogTags) — it is used by ~15 app-side files and belongs to neither process - assets/xposed_init and the R8 keep rule now name the new class; verified against the built APK, which carries dev/okhsunrog/vpnhide/hook/HookEntry after minification - HookPackageBoundaryTest enforces both directions: no UI in hook/, no Xposed imports outside it, and xposed_init matching the real package
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.
state.jsoncarried two schema numbers, both pinned at 1 since the format existed, next to a comment saying compatibility was not a concern. A number that never moves tells a triager nothing about the file in front of them, and nothing stopped the shape from drifting between releases. This makes it a real version and then uses that safety net for the first package move:hook/is loaded by LSPosed into system_server, which is the one boundary in this module with a real failure mode, and the flat 96-file package hid it.DiagnosticReport.schemais gone), bumped to 2, with a history table and bump rules in docs/debug-bundle.md §2.1LsposedStateandProtectionCheckget explicit@SerialNamevalues; without them kotlinx emits the fully-qualified class name as thekinddiscriminator, so those two blocks readdev.okhsunrog.vpnhide.LsposedState.Activewhile every other sealed type readsactive— and the value would have changed silently the moment the class moved packageBundleSchemaGoldenTestpins the encoded shape against a checked-in golden file and asserts no fully-qualified name reaches the wire, so a shape change cannot ship unnoticedhook/package: HookEntry, PackageVisibilityHooks, SystemServerConfigCache, HookLog, HookReflectionProbe, ConnectivityAttachDiagnostics and the shared /data/system watcher. Pure move, 12 imports added, no logic touched; the shared vocabulary (HookRegistry, LsposedStats, canonical-config parsing, LogTags) stays at the root packageassets/xposed_initand the R8 keep rule name the new class — verified against the built APK, which carriesdev/okhsunrog/vpnhide/hook/HookEntryafter minificationHookPackageBoundaryTestenforces both directions: no Compose/Activity imports inhook/, no Xposed imports outside it, and xposed_init matching the real packageNote for whichever of this and #308 merges second: both touch the serialized report, so the golden file needs refreshing (
UPDATE_GOLDEN=1 ./gradlew :app:testDebugUnitTest --tests '*BundleSchemaGoldenTest*'). Adding a field needs no schema bump.