From ddf0c4b0ff10cf1a043428a787a051b391480533 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Tue, 30 Jun 2026 13:10:51 +0200 Subject: [PATCH 1/3] feat(audit): add Navigation 3 detection and audit patterns Add section 2b to the jetpack-compose-audit search-playbook covering: - Nav3 detection (rememberNavBackStack, NavDisplay, entryProvider, NavKey) - Backstack ownership violation (feature VM holds backStack) - entryDecorators without rememberSaveableStateHolderNavEntryDecorator - Navigation triggered in composition body - Missing dropUnlessResumed click guard - Anonymous/non-top-level destination keys - NavController/NavHost mixed with Nav3 code - String routes in Nav3 code - @Composable inside destination data class - ResultEventBus result assumed to survive process death - Positive signals and scoring notes Also adds Nav3 + nav3-recipes to canonical-sources.md. --- .../references/canonical-sources.md | 6 +- .../references/search-playbook.md | 161 ++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) diff --git a/skills/jetpack-compose-audit/references/canonical-sources.md b/skills/jetpack-compose-audit/references/canonical-sources.md index f1ec71d..b6e7675 100644 --- a/skills/jetpack-compose-audit/references/canonical-sources.md +++ b/skills/jetpack-compose-audit/references/canonical-sources.md @@ -124,8 +124,12 @@ This grounds: `https://developer.android.com/develop/ui/compose/custom-modifiers` - Android Developers: `Locally scoped data with CompositionLocal` `https://developer.android.com/develop/ui/compose/compositionlocal` -- Android Developers: `Navigation with Compose` +- Android Developers: `Navigation with Compose` (Nav2) `https://developer.android.com/develop/ui/compose/navigation` +- Android Developers: `Navigation 3` + `https://developer.android.com/guide/navigation/navigation-3` +- nav3-recipes (official sample repo, verified API shapes) + `https://github.com/android/nav3-recipes` These ground: diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index a7cdd0d..0cff3bb 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -121,6 +121,167 @@ Report format: Do not subtract from Performance, State, Side Effects, or Composable API Quality. It may still belong in `Critical Findings` or `Prioritized Fixes` because it is user-visible and usually cheap to fix. +## 2b. Navigation 3 Detection & Audit + +Run this section **only when Nav3 is present**. Nav3 ships a different model from Nav2 — wrong patterns are silent at compile time but break backstack restore, process death recovery, and recomposition safety. + +### Detect Nav3 Usage + +```bash +rg -l 'rememberNavBackStack|NavDisplay|entryProvider|NavKey|androidx\.navigation3' -g '*.kt' -g '*.kts' +``` + +If any file matches, Nav3 is in scope. Continue below. If none match, skip this section. + +### Map Nav3 Entry Points + +```bash +# Back stack instantiation +rg 'rememberNavBackStack' -g '*.kt' -n + +# NavDisplay host(s) +rg 'NavDisplay\s*\(' -g '*.kt' -n + +# Destination key types +rg '@Serializable' -g '*.kt' -n | grep -i 'NavKey\|: NavKey' + +# Custom entry decorators +rg 'entryDecorators\s*=' -g '*.kt' -n + +# Result bus usage +rg 'rememberResultEventBusNavEntryDecorator|LocalResultEventBus|ResultEffect|conflateAsState' -g '*.kt' -n +``` + +### Red Flags To Verify + +**Backstack ownership violation — feature ViewModel holds or mutates the back stack** + +```bash +rg 'NavBackStack|backStack' -g '*.kt' -l | xargs rg -l 'ViewModel\|viewModel\(\)' 2>/dev/null +``` + +A feature/screen ViewModel must never own or directly mutate the back stack. Navigation signals should flow from the ViewModel as state/events that the *route* observes and acts on via `backStack.add` / `removeLastOrNull`. A dedicated app-level nav holder that *owns* the stack is a legitimate pattern; a feature ViewModel that receives `backStack` as a parameter for navigation is not. + +Severity: **Blocker** — this couples navigation lifecycle to the ViewModel and defeats predictive back and scene strategies. + +--- + +**`entryDecorators` supplied without re-adding `rememberSaveableStateHolderNavEntryDecorator`** + +```bash +rg 'entryDecorators\s*=' -g '*.kt' -n -A 6 +``` + +For each hit: check whether the decorator list includes `rememberSaveableStateHolderNavEntryDecorator()`. Supplying a custom list *replaces all defaults*, so `rememberSaveable` inside entries silently stops working if the default saveable-state decorator is omitted. + +Severity: **Blocker** — `rememberSaveable` inside affected entries persists nothing; crash or data loss on config change. + +--- + +**Navigation triggered in composition body (not from event handler or `LaunchedEffect`)** + +```bash +rg 'backStack\.(add|removeLastOrNull|clear)\b' -g '*.kt' -n +``` + +For each hit: confirm it is inside a lambda passed as an event handler (e.g. `onClick`, `onBack`, `onOpenProfile`) or inside a `LaunchedEffect`. A call directly in the composition body triggers navigation on every recomposition. + +Severity: **Blocker** — navigation fires on every recomposition, duplicates back-stack entries. + +--- + +**Missing `dropUnlessResumed` click guard** + +```bash +rg 'onClick\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n +# also check onXxx lambdas +rg 'on\w+\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n +``` + +For each navigation-triggering tap handler: check it is wrapped in `dropUnlessResumed { backStack.add(...) }`. Without this guard, a queued tap can navigate from a screen that has already left `RESUMED`, pushing a duplicate entry during the exit animation. + +Severity: **Should-fix** — rare in practice but reproducible with fast double-taps and on slow devices. + +--- + +**Anonymous or non-top-level destination keys** + +```bash +rg 'object\s*:\s*NavKey' -g '*.kt' -n # anonymous object +rg 'NavKey\b' -g '*.kt' -n | grep -v '^.*data\s\(class\|object\)' | grep 'NavKey' +``` + +All `NavKey` destinations must be **top-level `@Serializable` data classes or objects**. Anonymous inline keys (anonymous objects, local classes) break `rememberSaveable` and cannot be restored after process death. + +Severity: **Blocker** — process-death restore fails; `rememberSaveable` inside that entry silently stops working. + +--- + +**Nav2 `NavController` / `NavHost` used alongside Nav3 code** + +```bash +rg 'rememberNavController|NavHost\s*\(' -g '*.kt' -n +``` + +Check whether any hit coexists in the same navigation graph with `NavDisplay`. Mixing Nav2 and Nav3 in the same back-stack flow is not supported; they must own separate, non-overlapping regions of the UI. + +Severity: **Blocker** if mixed in the same flow; **Nit** if clearly isolated sub-graphs. + +--- + +**String routes still used in Nav3 code** + +```bash +rg 'backStack\.add\s*\(\s*"' -g '*.kt' -n +rg 'navigate\s*\(\s*"' -g '*.kt' -n +``` + +Nav3 destinations are `@Serializable` typed keys — not strings. Any string passed to `backStack.add` is a code smell indicating a Nav2 migration that was only partially completed. + +Severity: **Blocker** — no type safety, no compile-time verification, breaks process-death serialization. + +--- + +**`@Composable` or lambda captured inside a destination data class** + +```bash +rg -A 10 'data class \w+ *\(' -g '*.kt' | grep -E '@Composable|\(\) ->' +``` + +Destination data classes must be serializable plain data. Composable references, lambdas, or any non-serializable type as a field will crash at serialization time. + +Severity: **Blocker** — runtime crash on process death or `rememberSaveable`. + +--- + +**`ResultEventBus` result assumed to survive process death** + +```bash +rg 'conflateAsState\|ResultEffect' -g '*.kt' -n +``` + +`conflateAsState` delivers the latest result within the current navigation lifetime — it is **not** persisted across process death. If a result must survive process death, it belongs in a persisted state holder (e.g. `SavedStateHandle`), not just in `conflateAsState`. + +Severity: **Should-fix** when the result represents user data that must not be lost on backgrounding. + +### Positive Signals + +- All destinations are top-level `@Serializable data class` / `object` implementing `NavKey` +- `dropUnlessResumed { backStack.add(...) }` on every tap-driven navigation call +- `rememberSaveableStateHolderNavEntryDecorator()` present in every custom `entryDecorators` list +- App-level nav holder owns the back stack; feature ViewModels expose navigation state as events/state, not by holding `backStack` +- `rememberViewModelStoreNavEntryDecorator()` added when per-entry `viewModel()` scoping is needed (and placed after the saveable-state decorator) +- `ResultEventBus` used for screen-to-screen results; outcomes that must survive process death persisted in `SavedStateHandle` + +### Nav3 Scoring Note + +Nav3 findings map to the **Side Effects** and **State Management** score categories: +- Backstack ownership violation, composition-body navigation → Side Effects +- `entryDecorators` missing saveable-state decorator, anonymous keys, string routes → State Management +- Missing `dropUnlessResumed` → Side Effects (Nit/Should-fix unless double-nav is observed) + +Do not open a new score category for Nav3. If the project has no Nav2 code at all, note in the report that Nav2 navigation patterns are out of scope and Nav3 is the only navigation layer audited. + ## 3. Performance Checks ### Search For From 66b6f68f41b0746b8ac2f6590c2ea05e17354261 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Tue, 30 Jun 2026 13:22:49 +0200 Subject: [PATCH 2/3] fix(audit): address Codex + Cursor review findings on Nav3 section - Fix broken rg alternation: remove backslash-escaping from | in two patterns (ViewModel|viewModel and conflateAsState|ResultEffect were matching literal |) - Fix destination-key scan: use rg -B1 ': NavKey' instead of same-line @Serializable grep - Broaden lambda check in NavKey destinations: cover (T) -> Unit, suspend, not just () -> and scope search to NavKey files to reduce noise - Add high-false-positive warning to ViewModel+backStack heuristic - Downgrade string-route Blocker to Should-fix: NavBackStack makes strings a compile error in practice; scope navigate() check to Nav3 files only - Add Nav3 scoring entries to scoring.md: State (backstack state) and Side Effects (nav side effects) sections with canonical source links --- .../references/scoring.md | 18 +++++++++++++ .../references/search-playbook.md | 25 +++++++++++-------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/skills/jetpack-compose-audit/references/scoring.md b/skills/jetpack-compose-audit/references/scoring.md index 25e6c5b..e64efdb 100644 --- a/skills/jetpack-compose-audit/references/scoring.md +++ b/skills/jetpack-compose-audit/references/scoring.md @@ -223,6 +223,16 @@ Deduct for: When any of these paging rules affects the State management score, name it in the State section as **Paging load-state handling** rather than folding it into a generic state note. +**Navigation 3 deductions (State management)** + +When Nav3 is detected (see playbook section 2b), also deduct for: +- `entryDecorators` list supplied without re-adding `rememberSaveableStateHolderNavEntryDecorator()` — `rememberSaveable` inside entries silently stops persisting state → [Nav3 save state](https://developer.android.com/guide/navigation/navigation-3/save-state) +- Anonymous or non-top-level `NavKey` destination types — breaks process-death restore and `rememberSaveable` for that entry → [Nav3](https://developer.android.com/guide/navigation/navigation-3) +- Feature/screen ViewModel owns or mutates the back stack directly — navigation lifecycle coupled to ViewModel, defeats predictive back and scene strategies → [Nav3](https://developer.android.com/guide/navigation/navigation-3) +- `ResultEventBus` / `conflateAsState` result treated as surviving process death — result is only available within the current navigation lifetime; persist in `SavedStateHandle` if survival is required → [Nav3 save state](https://developer.android.com/guide/navigation/navigation-3/save-state) + +Name Nav3 state findings as **Nav3 backstack state** in the State section. + Suggested interpretation: - `9-10`: strong UDF, clear ownership, minimal ambiguity @@ -271,6 +281,14 @@ Deduct for: When this paging rule affects the Side Effects score, name it in the Side Effects section as **Paging side-effect signals** rather than folding it into a generic effect note — mirroring **Paging list correctness** (Performance) and **Paging load-state handling** (State). +**Navigation 3 deductions (Side Effects)** + +When Nav3 is detected (see playbook section 2b), also deduct for: +- `backStack.add` / `backStack.removeLastOrNull` called directly in the composition body instead of inside an event handler or `LaunchedEffect` — navigation fires on every recomposition → [Nav3](https://developer.android.com/guide/navigation/navigation-3) +- Tap handler missing `dropUnlessResumed { backStack.add(...) }` — a queued tap can navigate from a screen already in exit animation, duplicating the back-stack entry → [Nav3](https://developer.android.com/guide/navigation/navigation-3) + +Name Nav3 side-effect findings as **Nav3 navigation side effects** in the Side Effects section. + Suggested interpretation: - `9-10`: deliberate, lifecycle-aware effect usage diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index 0cff3bb..194378e 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -142,8 +142,8 @@ rg 'rememberNavBackStack' -g '*.kt' -n # NavDisplay host(s) rg 'NavDisplay\s*\(' -g '*.kt' -n -# Destination key types -rg '@Serializable' -g '*.kt' -n | grep -i 'NavKey\|: NavKey' +# Destination key types (multiline: @Serializable often on separate line from : NavKey) +rg -B1 ': NavKey' -g '*.kt' -n # Custom entry decorators rg 'entryDecorators\s*=' -g '*.kt' -n @@ -157,11 +157,13 @@ rg 'rememberResultEventBusNavEntryDecorator|LocalResultEventBus|ResultEffect|con **Backstack ownership violation — feature ViewModel holds or mutates the back stack** ```bash -rg 'NavBackStack|backStack' -g '*.kt' -l | xargs rg -l 'ViewModel\|viewModel\(\)' 2>/dev/null +rg 'NavBackStack|backStack' -g '*.kt' -l | xargs rg -l 'ViewModel|viewModel\(\)' 2>/dev/null ``` A feature/screen ViewModel must never own or directly mutate the back stack. Navigation signals should flow from the ViewModel as state/events that the *route* observes and acts on via `backStack.add` / `removeLastOrNull`. A dedicated app-level nav holder that *owns* the stack is a legitimate pattern; a feature ViewModel that receives `backStack` as a parameter for navigation is not. +**High false-positive rate:** any file with both `backStack` and `viewModel()` matches — including `App.kt` where the nav holder and ViewModel co-exist legitimately. Always read the hit to confirm the ViewModel actually holds or mutates the stack, not merely that both symbols appear in the same file. + Severity: **Blocker** — this couples navigation lifecycle to the ViewModel and defeats predictive back and scene strategies. --- @@ -232,23 +234,26 @@ Severity: **Blocker** if mixed in the same flow; **Nit** if clearly isolated sub **String routes still used in Nav3 code** ```bash +# Scope to Nav3 files only (from the detection query above) to avoid flagging Nav2 sub-graphs rg 'backStack\.add\s*\(\s*"' -g '*.kt' -n -rg 'navigate\s*\(\s*"' -g '*.kt' -n +# navigate("…") is a Nav2 API; flag only if found in a file that also uses NavDisplay/rememberNavBackStack +rg 'navigate\s*\(\s*"' -g '*.kt' -l | xargs rg -l 'NavDisplay|rememberNavBackStack' 2>/dev/null ``` -Nav3 destinations are `@Serializable` typed keys — not strings. Any string passed to `backStack.add` is a code smell indicating a Nav2 migration that was only partially completed. +Nav3 destinations are `@Serializable` typed keys — not strings. `backStack.add("…")` won't compile with `NavBackStack`, so a hit almost always indicates dead/migrated code or a type mismatch. `navigate("…")` in a file that also uses `NavDisplay` signals an incomplete Nav2→Nav3 migration. -Severity: **Blocker** — no type safety, no compile-time verification, breaks process-death serialization. +Severity: **Should-fix** (migration smell / likely dead code) rather than Blocker — Nav3's type system makes the string-route pattern a compile error in practice. --- -**`@Composable` or lambda captured inside a destination data class** +**`@Composable` or non-serializable type inside a NavKey destination** ```bash -rg -A 10 'data class \w+ *\(' -g '*.kt' | grep -E '@Composable|\(\) ->' +# Find NavKey destination types first +rg -l ': NavKey' -g '*.kt' | xargs rg -n '@Composable|\(\s*\)\s*->\s*\w+|\([^)]+\)\s*->\s*\w+|suspend\s*\(' 2>/dev/null ``` -Destination data classes must be serializable plain data. Composable references, lambdas, or any non-serializable type as a field will crash at serialization time. +Destination data classes must be serializable plain data. Composable references, lambdas (`() -> Unit`, `(String) -> Unit`, suspend lambdas), or any non-serializable type as a field will crash at serialization time. Scope the search to NavKey files to avoid noise from non-navigation data classes. Severity: **Blocker** — runtime crash on process death or `rememberSaveable`. @@ -257,7 +262,7 @@ Severity: **Blocker** — runtime crash on process death or `rememberSaveable`. **`ResultEventBus` result assumed to survive process death** ```bash -rg 'conflateAsState\|ResultEffect' -g '*.kt' -n +rg 'conflateAsState|ResultEffect' -g '*.kt' -n ``` `conflateAsState` delivers the latest result within the current navigation lifetime — it is **not** persisted across process death. If a result must survive process death, it belongs in a persisted state holder (e.g. `SavedStateHandle`), not just in `conflateAsState`. From f04546420e97ad50c465b4ddd036809c5c7f96b3 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Tue, 30 Jun 2026 13:32:08 +0200 Subject: [PATCH 3/3] fix(audit): address round-2 review findings on Nav3 section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Align State vs Side Effects categorization: ViewModel/backstack ownership → State Management in both playbook and scoring.md (was Side Effects in playbook) - Add xargs -r to all three pipeline commands (empty input was searching whole repo) - Fix anonymous-key grep: filter sealed/interface base types, note valid patterns - Fix intro: soften 'silent at compile time' — string routes are often compile errors - Add -U (multiline) to dropUnlessResumed rg commands + fallback -B3 context scan - Add read-the-hit note to @Composable/lambda check (whole-file scan, not field scan) - Add Nav3 pointer to SKILL.md step 5 so auditors don't skip section 2b - Clarify string-route search scope comment (not actually file-scoped, just method-scoped) --- skills/jetpack-compose-audit/SKILL.md | 2 + .../references/scoring.md | 2 + .../references/search-playbook.md | 46 ++++++++++++------- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/skills/jetpack-compose-audit/SKILL.md b/skills/jetpack-compose-audit/SKILL.md index c9acde1..60abc34 100644 --- a/skills/jetpack-compose-audit/SKILL.md +++ b/skills/jetpack-compose-audit/SKILL.md @@ -194,6 +194,8 @@ Stability deductions from step 5 are measured evidence and should be weighted no Use the scoring rubric in `references/scoring.md` and the heuristics in `references/search-playbook.md`. +> **Navigation 3:** if the repo uses `rememberNavBackStack`, `NavDisplay`, or `NavKey`, run **playbook section 2b** (Nav3 Detection & Audit) before scoring. Nav3 findings map to State Management and Side Effects — see the scoring note in section 2b. + #### Performance Focus on: diff --git a/skills/jetpack-compose-audit/references/scoring.md b/skills/jetpack-compose-audit/references/scoring.md index e64efdb..d63d414 100644 --- a/skills/jetpack-compose-audit/references/scoring.md +++ b/skills/jetpack-compose-audit/references/scoring.md @@ -287,6 +287,8 @@ When Nav3 is detected (see playbook section 2b), also deduct for: - `backStack.add` / `backStack.removeLastOrNull` called directly in the composition body instead of inside an event handler or `LaunchedEffect` — navigation fires on every recomposition → [Nav3](https://developer.android.com/guide/navigation/navigation-3) - Tap handler missing `dropUnlessResumed { backStack.add(...) }` — a queued tap can navigate from a screen already in exit animation, duplicating the back-stack entry → [Nav3](https://developer.android.com/guide/navigation/navigation-3) +Note: "feature ViewModel owns/mutates the back stack" is a **State Management** finding (see above), not Side Effects — do not score it here. + Name Nav3 side-effect findings as **Nav3 navigation side effects** in the Side Effects section. Suggested interpretation: diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index 194378e..b8a8c99 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -123,7 +123,7 @@ Do not subtract from Performance, State, Side Effects, or Composable API Quality ## 2b. Navigation 3 Detection & Audit -Run this section **only when Nav3 is present**. Nav3 ships a different model from Nav2 — wrong patterns are silent at compile time but break backstack restore, process death recovery, and recomposition safety. +Run this section **only when Nav3 is present**. Nav3 ships a different model from Nav2 — many wrong patterns compile fine but break backstack restore, process death recovery, or recomposition safety at runtime. (Note: string routes and typed-key mismatches are often compile errors in Nav3; the runtime risks are primarily state/save-state related.) ### Detect Nav3 Usage @@ -157,7 +157,7 @@ rg 'rememberResultEventBusNavEntryDecorator|LocalResultEventBus|ResultEffect|con **Backstack ownership violation — feature ViewModel holds or mutates the back stack** ```bash -rg 'NavBackStack|backStack' -g '*.kt' -l | xargs rg -l 'ViewModel|viewModel\(\)' 2>/dev/null +rg 'NavBackStack|backStack' -g '*.kt' -l | xargs -r rg -l 'ViewModel|viewModel\(\)' 2>/dev/null ``` A feature/screen ViewModel must never own or directly mutate the back stack. Navigation signals should flow from the ViewModel as state/events that the *route* observes and acts on via `backStack.add` / `removeLastOrNull`. A dedicated app-level nav holder that *owns* the stack is a legitimate pattern; a feature ViewModel that receives `backStack` as a parameter for navigation is not. @@ -195,12 +195,14 @@ Severity: **Blocker** — navigation fires on every recomposition, duplicates ba **Missing `dropUnlessResumed` click guard** ```bash -rg 'onClick\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n -# also check onXxx lambdas -rg 'on\w+\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n +# -U enables multiline matching so backStack.add can be on a different line than onClick +rg -U 'onClick\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n +rg -U 'on\w+\s*=\s*\{[^}]*backStack\.add' -g '*.kt' -n +# Simpler alternative: find all backStack.add call sites and read surrounding context +rg 'backStack\.add\(' -g '*.kt' -n -B3 ``` -For each navigation-triggering tap handler: check it is wrapped in `dropUnlessResumed { backStack.add(...) }`. Without this guard, a queued tap can navigate from a screen that has already left `RESUMED`, pushing a duplicate entry during the exit animation. +For each navigation-triggering tap handler: check it is wrapped in `dropUnlessResumed { backStack.add(...) }`. Regex matches are hints — multiline lambdas and nested braces require reading the actual code around each hit. Without this guard, a queued tap can navigate from a screen that has already left `RESUMED`, pushing a duplicate entry during the exit animation. Severity: **Should-fix** — rare in practice but reproducible with fast double-taps and on slow devices. @@ -209,13 +211,17 @@ Severity: **Should-fix** — rare in practice but reproducible with fast double- **Anonymous or non-top-level destination keys** ```bash -rg 'object\s*:\s*NavKey' -g '*.kt' -n # anonymous object -rg 'NavKey\b' -g '*.kt' -n | grep -v '^.*data\s\(class\|object\)' | grep 'NavKey' +# Catch anonymous objects (object expression, not object declaration) +rg 'object\s*:\s*NavKey' -g '*.kt' -n +# Catch NavKey usages that are not top-level data class/object or sealed interface/class base types +rg ': NavKey' -g '*.kt' -n | grep -Ev '(data (class|object)|sealed (class|interface)|^.*object\s+\w+\s*:)' ``` -All `NavKey` destinations must be **top-level `@Serializable` data classes or objects**. Anonymous inline keys (anonymous objects, local classes) break `rememberSaveable` and cannot be restored after process death. +All `NavKey` **destination** types must be top-level `@Serializable` data classes or objects. Anonymous inline keys (object expressions) and local classes break `rememberSaveable` and cannot be restored after process death. -Severity: **Blocker** — process-death restore fails; `rememberSaveable` inside that entry silently stops working. +Valid patterns that must **not** be flagged: `sealed interface AppNavKey : NavKey`, `object HomeKey : NavKey` (named top-level object), base sealed hierarchies. Read each hit before filing — sealed/interface base types are always valid. + +Severity: **Blocker** for anonymous objects and local classes — process-death restore fails; `rememberSaveable` inside that entry silently stops working. --- @@ -237,7 +243,7 @@ Severity: **Blocker** if mixed in the same flow; **Nit** if clearly isolated sub # Scope to Nav3 files only (from the detection query above) to avoid flagging Nav2 sub-graphs rg 'backStack\.add\s*\(\s*"' -g '*.kt' -n # navigate("…") is a Nav2 API; flag only if found in a file that also uses NavDisplay/rememberNavBackStack -rg 'navigate\s*\(\s*"' -g '*.kt' -l | xargs rg -l 'NavDisplay|rememberNavBackStack' 2>/dev/null +rg 'navigate\s*\(\s*"' -g '*.kt' -l | xargs -r rg -l 'NavDisplay|rememberNavBackStack' 2>/dev/null ``` Nav3 destinations are `@Serializable` typed keys — not strings. `backStack.add("…")` won't compile with `NavBackStack`, so a hit almost always indicates dead/migrated code or a type mismatch. `navigate("…")` in a file that also uses `NavDisplay` signals an incomplete Nav2→Nav3 migration. @@ -250,7 +256,7 @@ Severity: **Should-fix** (migration smell / likely dead code) rather than Blocke ```bash # Find NavKey destination types first -rg -l ': NavKey' -g '*.kt' | xargs rg -n '@Composable|\(\s*\)\s*->\s*\w+|\([^)]+\)\s*->\s*\w+|suspend\s*\(' 2>/dev/null +rg -l ': NavKey' -g '*.kt' | xargs -r rg -n '@Composable|\(\s*\)\s*->\s*\w+|\([^)]+\)\s*->\s*\w+|suspend\s*\(' 2>/dev/null ``` Destination data classes must be serializable plain data. Composable references, lambdas (`() -> Unit`, `(String) -> Unit`, suspend lambdas), or any non-serializable type as a field will crash at serialization time. Scope the search to NavKey files to avoid noise from non-navigation data classes. @@ -280,10 +286,18 @@ Severity: **Should-fix** when the result represents user data that must not be l ### Nav3 Scoring Note -Nav3 findings map to the **Side Effects** and **State Management** score categories: -- Backstack ownership violation, composition-body navigation → Side Effects -- `entryDecorators` missing saveable-state decorator, anonymous keys, string routes → State Management -- Missing `dropUnlessResumed` → Side Effects (Nit/Should-fix unless double-nav is observed) +Nav3 findings map to existing score categories — do not open a new one: + +| Finding | Category | +|---------|----------| +| Feature ViewModel owns/mutates back stack | **State Management** (ownership of state) | +| `entryDecorators` missing saveable-state decorator | **State Management** | +| Anonymous / non-top-level `NavKey` types | **State Management** | +| `ResultEventBus` result assumed to survive process death | **State Management** | +| Navigation triggered in composition body | **Side Effects** | +| Missing `dropUnlessResumed` click guard | **Side Effects** (Should-fix) | + +Report State findings as **Nav3 backstack state**; report Side Effects findings as **Nav3 navigation side effects**. Do not open a new score category for Nav3. If the project has no Nav2 code at all, note in the report that Nav2 navigation patterns are out of scope and Nav3 is the only navigation layer audited.