Repository navigation
fix(mobile): open Code Reviewer and Security Agent above the tabs - #8509
Conversation
The Home Explore shortcuts for Code Reviewer and Security Agent pushed routes that live inside the Profile tab's stack, so expo-router switched the tab navigator and the bottom bar drew Profile as the selected tab while those screens were on top. Move both route trees out of `(tabs)/(3_profile)` into the root stack, like PR Review: the screens open above the tab bar, no tab is marked selected while they are up, and going back returns to the tab the user came from. URL paths are unchanged because group segments are not part of a URL, and the shared universal-link table now targets the new paths. A last-route record mirrored by an older build is read at the screen's current path instead of an unmatched one. The content clearance follows the same rule: the root stack publishes that no tab bar sits under its screens, so their scroll views reserve the safe-area inset instead of a bar height.
iscekic
added
drain
|
bot: Captures from the affected app on the local dev stack with an owned fixture account. Before — Android, Code Reviewer opened from Home (the bottom bar reported Profile as selected): After — Android, the same entry, no tab bar on the pushed screen: After — Android, Home root with the bar back and Home selected on return: After — iOS simulator, Code Reviewer opened from Home: After — iOS simulator, Home after going back:
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit adds the retention rationale above Assumptions: Files Reviewed (1 file)
Previous Review Summaries (5 snapshots, latest commit d57753c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit d57753c)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up closes the last persisted-navigation gap by routing both the last-route mirror and the pending deep-link mirror through one Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit a247c3c)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up delta keeps the moved Code Reviewer and Security Agent trees, the last-route mirror and the system-search ledger consistent; the one remaining gap is the pending deep-link mirror, which still replays verbatim an href captured by an older build. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 89fa69c)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe follow-up commits close the earlier Security Agent back-navigation defect and repoint every invalid Code Reviewer route at the Code Reviewer entry; the only remaining gap is the pending-deep-link mirror, which still replays an href captured by an older build verbatim. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (14 files)
Fix these issues in Kilo Cloud Previous review (commit 7e6a517)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe follow-up commit closes both previously reported system-search gaps — legacy finding ids now canonicalise to the current route before the allowlist, source lookup, and deeplink are computed, so a stale entry both opens the right screen and can finally be pruned — leaving two persisted-navigation gaps from the route move uncovered. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous review (commit 511a746)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe route move and the new root-stack clearance rule are coherent, but persisted navigation state was only migrated for the last-route mirror — the system-search ledger keeps stale finding ids forever, and two other persisted/history paths still assume Security Agent lives under the Profile tab. Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (75 files)
Reviewed by deepseek-flash-latest · Input: 34.1K · Output: 2.2K · Cached: 126.2K Review guidance: REVIEW.md from base branch |
The phone's search index keeps an entry until the app removes it, and iOS hands the indexed id back when the result is picked. Ids issued before the finding route left the profile tab stack no longer matched the current prefix, so such a result could not open its screen, and the sync could not name the source that authorises removing it, leaving the stale entry in place. Canonicalise an indexed id to the route the app shows today before the allowlist and the source lookup read it, so an older result opens the same screen and the next sync replaces it. Also restore the parse comment that sat above the moved-route table.
iscekic
added
merge:blocked:kilobot-not-ready
…lore-screens # Conflicts: # apps/mobile/src/app/(app)/code-reviewer/[scope]/[platform]/(edit)/instructions.mounted.test.tsx
iscekic
removed
merge:bot
…entry A Code Reviewer route that cannot render, because its scope or review id is missing, now sends the user to the Code Reviewer entry instead of the Profile tab. The routes live in the root stack, so the entry is the screen that opened them and no tab bar is selected underneath.
…lore-screens # Conflicts: # apps/mobile/src/app/(app)/_layout.tsx
A finding opened without local history now returns to its scope's findings list in the root stack. This keeps the return path within the relocated Security Agent surface rather than switching to a tab.
…lore-screens # Conflicts: # apps/mobile/src/lib/security-notification-navigation.mounted.test.tsx
iscekic
added
merge:blocked:kilobot-not-ready
|
bot: Fixed the summary-only pending-deep-link finding in d57753c. Both persisted navigation readers now share currentRouteHref. Four regression cases replayed the old route groups before the fix; all now resolve to the root-stack routes while preserving organization context and single-shot consumption. Six targeted test files pass (167 tests), and a standalone Node smoke exercised restoration and consumption for both relocated surfaces. Mobile typecheck, lint, unused-export checks, and formatting pass after building the required workspace declarations. Earlier Android/iOS screenshots remain the visual evidence; this nonvisual follow-up did not repeat the device run. |
iscekic
removed
merge:bot
iscekic
added
the
merge:blocked:insufficient-e2e
iscekic
added
the
merge:bot
|
bot: Re-ran the navigation flow on the affected Android app at the current head, against the local dev stack and a fixture account. Observed with accessibility snapshots and screenshots:
|
iscekic
removed
the
merge:blocked:insufficient-e2e
Evidence
Evidence: demonstrated — the pre-fix state was recorded on
mainduring this pull request's earlier device run: opening Code Reviewer from the Home Explore row pushed a screen inside the Profile tab stack, so the bottom bar stayed on screen and reported Profile as selected. At the current head the same flow was re-run on the Android app against the local dev stack with a fixture account: the pushed Code Reviewer screen, its GitHub settings and recent reviews children, and Security Agent draw no bottom bar (their accessibility snapshots contain no tab nodes), returning to Home shows the bar again with Home selected, and a push from the Profile tab row still returns to Profile.Change
currentRouteHrefbetween last-route and pending-deep-link restoration so persisted destinations written before the move still resolve to the relocated screens.Verification
Device run on the Android emulator against the local dev stack, fixture account, current head:
Home, tab, 1 of 4reportedselected="true".Home, tab, 1 of 4selected again.Regression and static checks:
Proxy answers
The owner's proxy gave these answers in the owner's place.