feat(audit): add Navigation 3 detection and audit patterns

[hamen]

Summary

Adds section 2b — Navigation 3 Detection & Audit to jetpack-compose-audit/references/search-playbook.md.

Previously, Nav3 support in the suite was limited to compose-agent (guidance/decision table). The jetpack-compose-audit skill had no Nav3 detection or diagnostic patterns — it only covered Nav2 (NavHost, navController.navigate). This PR closes that gap.

Patterns added

Finding	Severity
Feature ViewModel owns/mutates the back stack	Blocker
entryDecorators without rememberSaveableStateHolderNavEntryDecorator	Blocker
Navigation triggered in composition body	Blocker
Anonymous / non-top-level destination keys	Blocker
String routes used in Nav3 code	Blocker
@Composable or lambda inside destination data class	Blocker
Missing dropUnlessResumed click guard	Should-fix
ResultEventBus result assumed to survive process death	Should-fix
Nav2 NavController mixed with Nav3 code	Blocker (if same flow)

Also

• Positive signals for correct Nav3 patterns
• Scoring note mapping Nav3 findings to existing Side Effects / State Management categories
• Nav3 + nav3-recipes added to canonical-sources.md

All patterns verified against the official nav3-recipes sample repo and the compose-agent/references/navigation.md guardrails (already in the suite).

🤖 Generated with Claude Code

[hamen]

🟢 Codex review (automated cross-review)

Blocker
None.

Should-fix

• search-playbook.md / scoring.md: the entryDecorators rule effectively treats every custom decorator list missing rememberSaveableStateHolderNavEntryDecorator() as a Blocker. That conflicts with the local Nav3 guidance that official result-only recipes can omit it when screens do not use rememberSaveable (navigation.md). Require auditors to verify affected entries rely on saveable entry state before filing a Blocker.

• search-playbook.md / search-playbook.md: the NavKey audit only searches direct : NavKey declarations, so it misses the official subtype pattern (sealed interface MyAppNavKey : NavKey, leaves implementing MyAppNavKey) and does not explicitly flag missing @Serializable. The Android save-state docs require keys to implement NavKey and be @Serializable, and show the subtype pattern. Add a step to discover NavKey subtypes and their implementors.

• search-playbook.md / search-playbook.md: two playbook Blockers, Nav2+Nav3 mixed in one flow and non-serializable fields in NavKey destinations, are not mapped in scoring.md or the scoring table. Since this section is run before scoring, auditors need a category/score impact or an explicit “Critical Findings only” instruction.

Nit

• search-playbook.md: the comment says the string-route query is scoped to Nav3 files, but the next rg searches all Kotlin files. Either scope it to detected Nav3 files or soften the comment.

_{via pr-review-relay · author: claude}

[hamen]

🔵 Cursor review (automated cross-review)

PR #9 Review — Nav3 audit patterns

Solid addition: section 2b mirrors 2a’s shape, aligns with compose-agent/references/navigation.md, and the scoring/playbook/SKILL wiring is mostly consistent. No security issues. Below are gaps that could produce wrong or inconsistent audit output.

---

Blocker

None that block merge outright. The nested-key detection gap (Should-fix) is the closest to a correctness bug.

---

Should-fix

1. Non-top-level NavKey grep misses nested data class / sealed types. The filter excludes any line containing data class or sealed, so a nested destination like data class Profile(...) : NavKey inside another type is dropped even though it’s invalid per the guardrails. Only anonymous object : NavKey and non–data-class locals are reliably caught.

2. scoring.md omits several playbook Blockers. Playbook flags these as Blocker, but they’re absent from scoring deductions and the section 2b mapping table:

   • @Composable / lambda / non-serializable fields in NavKey destinations
   • Nav2 + Nav3 mixed in the same flow (playbook: Blocker; no score category guidance)

   Auditors may report them in Critical Findings without deducting, or apply ad hoc penalties.

3. PR description severity table ≠ playbook. The PR body lists “String routes” and “@Composable in NavKey” as Blocker; the playbook correctly marks string routes as Should-fix (compile-time) and doesn’t map composable-in-key into scoring at all. Fix the PR table so agents don’t over-severity.

4. ResultEventBus / conflateAsState heuristic is easy to over-apply. Playbook severity is conditional (“when the result must survive process death”), but “Map Nav3 Entry Points” greps all conflateAsState|ResultEffect hits. scoring.md wording is fine (“treated as surviving process death”), yet add an explicit “only deduct after confirming persistence expectation” note—same style as the ViewModel/backStack false-positive warning.

5. Composable-in-NavKey search is file-scoped, not type-scoped. rg -l ': NavKey' … | xargs rg '@Composable|…' flags composables in the same file as keys (common layout). Playbook says “scope to NavKey files” but agents may still file Blockers on screen composables; tighten to “field inside a NavKey data class” in prose.

6. No version/changelog bump for jetpack-compose-audit. Substantive audit behavior changed; skill is still 4.2.0 with no CHANGELOG entry (contrast paging in 4.2.0). Worth a minor bump for traceability.

---

Nit

• SKILL.md trigger lists three symbols; detection also uses entryProvider and androidx.navigation3—minor mismatch.
• Side Effects §5 still only calls out navController.navigate in composition; a cross-ref to 2b for backStack.add would help if 2b is skipped.
• xargs -r is GNU-specific (same as existing playbook §4 patterns); macOS auditors need a fallback.
• No Nav3 eval cases in evals/evals.json (paging got explicit eval hooks in 4.2.0).
• backStack.clear() is in the composition-body search but not in scoring.md Side Effects bullets (add/remove only).

---

Verdict: Direction and content look good and well grounded in navigation.md / nav3-recipes. Worth merging after fixing the nested-NavKey grep false negative and closing the scoring ↔ playbook gaps so Blocker findings map to deductions consistently.

_{via pr-review-relay · author: claude}

[hamen]

🟠 Antigravity review (automated cross-review)

Here is the review for PR #9, grouped by severity:

Blocker

• Broken heuristic for @Composable inside NavKey (search-playbook.md): The query rg -l ': NavKey' ... | xargs -r rg -n '@Composable|...' will produce a near-100% false-positive rate. In idiomatic Compose, the NavKey route (e.g. data object Home : NavKey) and its @Composable screen function are placed in the exact same file. The file-level grep will flag the valid screen composable itself rather than an invalid field inside the data class.
  • Fix: Narrow the search to look for properties (e.g., rg '(val|var)\s+\w+\s*:\s*(\([^)]*\)\s*->|@Composable|suspend)') or explicitly note that this query just finds files with routes and the auditor must manually check the data class fields instead.

Nit

• Valid NavKey type false-positives (search-playbook.md): The exclusion regex grep -Ev '(data (class|object)...)' will flag perfectly valid @Serializable types like value class and enum class as anonymous/non-top-level violations. Consider adding |value class|enum class to the whitelist to reduce noise.

_{via pr-review-relay · author: claude}