Skip to content

fix(mobile): keep a header Back press inside its own stack - #9327

Open
iscekic wants to merge 10 commits into
mainfrom
drain/header-back-burst-guard
Open

iscekic wants to merge 10 commits into
mainfrom
drain/header-back-burst-guard

Conversation

@iscekic

@iscekic iscekic commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — on this VM's Android emulator (dev client on the local dev stack, a seeded fake account with its own Security Agent scope, default font scale and density, head a59d47a after merging main) the reported screen appeared in the earlier round and the same steps no longer reach it:

  • Before this branch: open Security Agent, then Settings; kill the app and relaunch it; press the header Back once — the app landed on the terminal "Page not found" screen (no tab bar, one Back button). Temporary probes, since removed, showed the shape: the relaunch repair reopens the recorded screen with withAnchor, the [scope] layout read scope = "personal", and the anchor-seeded index route beneath it had no params of its own, so that route rendered its invalid state instead of the dashboard.
  • At head a59d47a: the same restore. Personal → Security Agent → Settings, force-stop, relaunch from the launcher; the app reopens the Settings screen (Security Agent eyebrow, Settings title, the repositories/models/automation rows), and one header Back lands on the Security Agent dashboard (Security Agent / Findings / Settings / View audit report, filter row, SLA tiles). No "not found" screen at any step.
  • At head a59d47a: a burst of four header Back presses about 60 ms apart (raw touch events, so the delivery is not spread out by the adb client) from the notifications subpage lands on the Security Agent dashboard — one level. The same burst from Settings ends on the Profile tab with the tab bar present (all four tab nodes, Profile selected), i.e. at the tab the user came from. The presses that arrive after the first pop reach the next screen's own header, which has its own window, so a burst unwinds one level per screen and stops at the tab root; it never reaches an unmatched route.
  • At head a59d47a: two deliberate presses, more than 500 ms apart, still walk two levels — Settings → dashboard → Profile tab (Profile tab node selected). The window does not swallow intentional navigation.
  • Control at the same head: planting origin/main's screen-header.tsx and restarting Metro, the identical burst lands on the same tab, so the burst landings alone do not separate the two builds on this emulator; the guard's own window (one dispatch per 500 ms per header, none without history, the screen's onBack first, a backwards clock step opening a new window) is covered by header-back.test.ts and use-header-back.mounted.test.tsx.

Follow-up (design review at 29c28ea), same emulator and stack, with seeded non-production fixtures for a never-synced personal scope, a synced organization scope and the notification settings:

  • The first-sync state states the status once. The repository row reads "All repositories" alone and the prompt below carries "Not yet synced" with its own "Sync findings" action; an accessibility dump of the base and dark cells holds exactly one "Not yet synced" node, at the prompt.
  • The SLA posture rows code their severity, the same way the risk rows below them do: Critical overdue draws #B0483A in the light cell and #F28B7A in the dark, High overdue #956011 / #F2B05F, Medium & low overdue the neutral #6F6A61 / #8A8680. Measured from the dots' own pixels in the gate's captures.
  • The dark off switch reads as a control: thumb #EDEBE6 on track #8A8680 over the #1F1F24 card (capture pixels); the light cell keeps thumb #FBFAF5 on track #6F6A61.
  • The notification subtitle fits one line in the base, dark and compact cells (14.1 dp text box) instead of leaving "sync." alone.

Two review-2 findings are answered rather than changed, per the owner's decision:

  • The dark-mode "Sync findings" button keeps the primary action fill. In this palette the primary action is #4F5A10 on the light card and the brand yellow on the dark one; DESIGN.md keeps brand yellow for exactly one primary action per surface. The palette has no olive dark variant, and adding one for a single screen would grow the token set the whole app shares.
  • The "Manage repositories" link now uses the same text-primary accent as every other link in the app, Button variant="link" included, so the link and the primary action share one accent in each mode. The blue dark-mode link review 1 asked for is reverted: it left the light cell olive and the dark cell blue, which is the mismatch review 2 reported.

Re-review of the same head (owner-authorized design re-review, same emulator and stack, seeded fake account, default font scale and density):

  • Four header Back presses about 230 ms apart — raw sendevent sequences timed on the device, starts 0 / 229 / 446 / 738 ms apart — from Settings land on the Profile tab with all four tab nodes present and Profile selected. No "Page not found" state, no unmatched route.
  • The same burst from the notification settings subpage lands on the Security Agent dashboard: the header, filter row, SLA area and "Not yet synced" prompt are back.
  • Two deliberate presses more than 500 ms apart still walk two levels, Settings → dashboard → Profile tab, so the window does not swallow intentional navigation.
  • Restore path: with Settings open, force-stop and relaunch; the app reopens Settings and one header Back lands on the Security Agent dashboard.
  • Each state asserted from a uiautomator dump captured in the same command as its screencap; the two screenshots are attached to this pull request.

Method: uiautomator dump and screencap before and after each step, raw sendevent touch sequences for the bursts, the design gate on three routes, pnpm --filter kilocode-app run typecheck, oxlint and oxfmt on the changed files, and the mobile vitest suites for the header, the header hook and the Security Agent screens; CI on the head commits runs the full mobile suite. The back-navigation steps and the design states were re-checked on the final head a59d47a, and the design gate was re-run there together with its merge-base run.

Limit: this branch's own commits keep the administrative steps of the earlier rounds; the merge of main changes only the back control's frame (main's circular press-surface clip), so the mobile behaviour was re-verified rather than re-derived.

Change

  • apps/mobile/src/lib/hooks/header-back.ts and use-header-back.ts: a header's Back control dispatches one back per 500 ms press burst, pops only when the container can go back, otherwise uses the screen's backFallback, and otherwise does nothing. A press that resolves to nothing leaves the burst window untouched, and a backwards wall-clock step opens a new window instead of holding presses out.
  • apps/mobile/src/components/screen-header.tsx: the back control uses that hook, inside the shared circular frame that clips the Android press and focus surface. A screen's own onBack still runs first, so the Security Agent settings unsaved-changes confirmation is unchanged.
  • apps/mobile/src/components/screen-header.mounted.test.tsx: mounts the header and presses its Back four times inside one window — one dispatch, one screen popped — then presses again past the window and walks a second level. The test fails when the window is disabled, so a component that stops wiring the hook cannot pass it.
  • apps/mobile/src/components/security-agent/security-agent-scope-route.tsx and the [scope] layout: the layout publishes the scope it validated, and a route with no scope param of its own renders under it. A param that names no scope still renders the invalid state, so no query runs with a bad owner.
  • apps/mobile/src/components/security-agent/dashboard-screen.tsx: before the first sync the screen offers the sync instead of a grid of zeros, only over an empty result; the repository row drops its status line while that prompt already carries it; the SLA compliance tile uses the app's no-value dash; the action links use the primary accent in both schemes; the repositories row keeps one control; "Manage repositories" stays reachable from the first-sync state.
  • apps/mobile/src/components/security-agent/dashboard-sections.tsx: the SLA posture dots carry each row's severity class (critical red, high amber, medium & low neutral) rather than turning red only when a count is non-zero; the empty "Act first" row is informational — no chevron, no press feedback.
  • apps/mobile/src/components/security-agent/notification-settings-screen.tsx and the English catalog: the personal scope drops the two section headings that restated each row's title, and the new-finding subtitle is shortened so it no longer leaves an orphan. Only the English value changes: the shortened sentence promises the same thing — every sync notifies, the first included — so the other catalogs' wording still states it and stays as it is; they keep their own line lengths, which the gate does not measure outside English.
  • apps/mobile/src/components/security-agent/settings-toggle-row.tsx: the switch draws both states from theme tokens, with the off thumb on the light end of the ladder in either scheme.

Production prerequisites

None.

Design gate

Design gate: 4 cells, 0 errors, 0 warnings.

Three routes in one run (the personal dashboard, its settings screen, its notification settings), 12 screen-cells, exit code 0, at head a59d47a (run uploaded under that head). The same three links ran again on the merge base 1cfecdb, with Metro serving that checkout, into the run's base/ folder: 4 cells, 12 screen-cells, 5 errors, 3 warnings there — five right-to-left clipped errors on the settings toggle rows (one on the settings screen, four on the notification rows), two dashboard touch-target warnings and one dashboard rtl-mirror warning. The head run reports none of them on the same routes, so this change introduces no design finding; the base findings are pre-existing defects this branch's own commits already cover. The base checkout was left untouched and the branch was restored before the head-facing work.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Scope: On main (5f28cef) with a signed-in fake account on this VM's emulator I reproduced the defect class but not the exact end screen: a 4-tap header Back burst delivers 1-2 presses to one header, each dispatching its own GO_BACK; two presses pop two levels, and at the stack root the extra GO_BACK is spent on the tab navigator (Profile -> Home) or is unhandled. The reported 'Page not found' landing did not occur in any run (the tab bar stayed). Proceed with the shared-header fix for the reproduced class, and state the end-screen limit in the PR evidence?
    Answer: Proceed with the header guard
    Confidence: 0.85 · Precedent: Apply the reasonable fix when a defect does not reproduce: "Merge fix(mobile): polish quick chat composer edge and session list loading rows #7613 and close this" over "Check on your iPhone"; fix the class: "if anything repeats the same mistake over and over again, fix the class"
  2. Question: Burst window: How wide should the burst guard be?
    Answer: Per-header, 500 ms
    Confidence: 0.60 · Precedent: Choose the simplest design and smaller scope when enough: "Simplest please"
  3. Question: No history: A header Back with no history left and no backFallback declared: what should it do?
    Answer: Do nothing
    Confidence: 0.80 · Precedent: Fix the root cause and the class of the issue: "if anything repeats the same mistake over and over again, fix the class"
  4. Question: The design critic asked the dark-mode dashboard action links to stop using brand yellow and use the app's standard link colour. Which token?
    Answer: Blue info token in dark only (Recommended)
    Confidence: 0.60 · Precedent: UX: change only the property asked for and keep the rest as it was ("you changed the dimensions - don't do that, just change the color"). Also keep elements consistent within the design system.
  5. Question: The critic found two controls for the same sync on the dashboard (header refresh icon and a 'Sync findings' text link). Which control should remain?
    Answer: Keep the refresh icon, move the labelled action into the first-sync prompt (Recommended)
    Confidence: 0.55 · Precedent: UX: remove redundant copy and use icons instead of text where appropriate ("im ok with using icons instead of text where appropriate"). Keep mobile layouts compact.
  6. Question: Dark accent: Design review 2 asks the Security Agent dashboard's controls to keep the light cell's olive in dark mode: the 'Sync findings' primary button ('saturated bright yellow in dark') and the 'Manage repositories' link ('blue in dark, olive in light'), each to use 'the dark-mode variant of the olive accent token'. Facts: the mobile palette has no olive dark variant — global.css pairs light --primary #4F5A10 (olive) with dark --primary #FFDD00 (brand yellow), DESIGN.md says yellow acts, and the only dark:text-info in the app is the line this PR added at review 1's request. The curated row-hue band does pair light #697C1D with dark #D6EB84. How should this change settle it?
    Answer: Keep the system pairing, rebut both
    Confidence: 0.55 · Precedent: Reject changes that reverse a deliberate, documented choice on main without showing a bug (DESIGN.md 'yellow acts'). Also keep the PR diff to files the feature needs and keep similar elements consistent across surfaces. This fix is about back navigation, so revert the review-1 link colour to text-primary and do not grow the palette.
  7. Question: Head review: merging main into the branch left the pull request head as a merge commit. The review bot's check does not run for a merge-commit head — observed on this branch and on another recently pushed merge-commit head, while single-parent heads are reviewed — and the branch history cannot be rewritten. The reachable ways forward were:
    Answer: Push a real follow-up commit (Recommended)
    Confidence: 0.75 · Precedent: Autonomy: blocked is not an acceptable end state; Git: never rebase or force-push, always merge.

Rapid presses on a shared header's Back control each dispatched their own
GO_BACK, so one burst popped screens the reader had not asked to leave, and a
Back with no history left was spent on the tab navigator (the tab switched)
or was unhandled at the root container. The header now claims one dispatch per
500 ms burst and pops only when the container can go back, using the declared
fallback destination otherwise.

The Security Agent [scope] layout also publishes the scope it validated, so a
route beneath it that a navigation seeds without params of its own — the anchor
sibling of a restored screen — renders its screen instead of the terminal
"Page not found" state that a Back press used to reveal.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Oct 30, 2026
@iscekic

iscekic commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator proof for the header Back change on this head (dev client on the local dev stack, seeded test account, default font scale and density).

  • Before: from the Security Agent settings screen, a restart reopened the same screen and the header Back landed on the app's "Page not found" state (no tab bar, one Back button).
  • After: the same step lands on the Security Agent dashboard.
  • Before: four quick Back presses on the dashboard header, opened from the Profile tab, ended on the Home tab.
  • After: the same burst stays on the Profile tab, tab bar present.
  • Design gate contact sheet for the two screens on this head; its findings are pre-existing, as the report on main shows.

Screen identity for each capture was asserted from the uiautomator dump taken in the same command as the screenshot (this VM has no OCR pass), and the PNGs carry no metadata chunks.

Previous state after Back from settings

Back from settings now lands on the dashboard

Previous tab after a quick Back burst

The same burst now stays on the Profile tab

Design gate contact sheet

@kilo-code-bot

kilo-code-bot Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change adds one mounted test asserting that a burst of header Back presses dispatches a single router.back() and that a press after the 500 ms window still walks the stack; the test matches useHeaderBack/claimHeaderBack at HEAD and introduces no new defect.

Assumptions: [INFERENCE] The incremental range e8df600a..HEAD contains a merge of main whose changes are not part of this PR, so the review focused on the branch commit a59d47ab6 (the new test) and a targeted re-verification of the PR's own files at HEAD. No test suite was run in this read-only review; the new test's helpers, mocks (routerState.routes/back, canGoBack) and HEADER_BACK_GUARD_MS export were verified by source reading. The earlier screen-header.tsx:279 suggestion remains answered by the author and withdrawn; it is not re-raised.

Files Reviewed (8 files)

  • apps/mobile/src/components/screen-header.mounted.test.tsx - new burst-guard test, no issues
  • apps/mobile/src/components/screen-header.tsx
  • apps/mobile/src/lib/hooks/header-back.ts
  • apps/mobile/src/lib/hooks/header-back.test.ts
  • apps/mobile/src/lib/hooks/use-header-back.ts
  • apps/mobile/src/lib/hooks/use-header-back.mounted.test.tsx
  • apps/mobile/src/components/security-agent/dashboard-screen.tsx
  • apps/mobile/src/components/security-agent/security-agent-scope-route.tsx

Verification Notes

  • The new test calls the header's onPress four times inside one fake-clock instant and expects one router.back(), then advances exactly HEADER_BACK_GUARD_MS (the out-of-window boundary of insideWindow) and expects the second pop; routerState is reset in beforeEach, and vi.useRealTimers() runs in finally, so the test cannot leak its clock into later cases.
  • useHeaderBack reads Date.now(), which the fake system clock drives, and it imports the exported constant the test advances by, so the assertion cannot drift from the implementation.
  • No new listener or subscription is added in the changed code (useCanGoBack still returns its navigationRef.addListener disposer), so no memory-leak concern is introduced.
Previous Review Summaries (5 snapshots, latest commit e8df600)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e8df600)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (7 files)
  • apps/mobile/src/components/offline-banner.mounted.test.tsx
  • apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx
  • apps/mobile/src/components/security-agent/dashboard-screen.tsx
  • apps/mobile/src/components/security-agent/dashboard-sections.tsx
  • apps/mobile/src/components/security-agent/settings-toggle-row.mounted.test.tsx
  • apps/mobile/src/components/security-agent/settings-toggle-row.tsx
  • apps/mobile/src/i18n/locales/en.json

Previous review (commit 29c28ea)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change gates the first-sync prompt on an empty stats grid and lifts the repository-setup link out of the post-sync branch, resolving both earlier findings with matching mounted tests; no new issues found.

Assumptions: [INFERENCE] The device evidence in the PR body was not re-run in this read-only review; verification is limited to source and test reading. analysis.total was confirmed to count every open finding (apps/web/src/lib/security-agent/db/dashboard-stats.ts:298-310), so the new neverSynced gate cannot hide analyzing/not-analyzed or SLA counts.

Files Reviewed (2 files)
  • apps/mobile/src/components/security-agent/dashboard-screen.tsx
  • apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx

Previous review (commit d0bb81d)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental dashboard rework gates the KPI grid and posture sections on a null sync time, which also hides real findings left behind by a partial sync failure.

Assumptions: [INFERENCE] The owner-level sync-time read was traced through apps/web/src/lib/security-agent/db/security-findings.ts:781-794, which deliberately returns null when agent_configs.runtime_state is missing instead of falling back to finding timestamps; no mobile test run was performed here. The prior screen-header.tsx:279 suggestion is not re-raised: it has an author reply on the thread and the previous review withdrew it after re-checking the navigation structure.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/components/security-agent/dashboard-screen.tsx 233 neverSynced treats a null sync time as proof of no prior sync, but a partial sync failure can leave findings with no runtime-state timestamp, so the counts and sections are replaced by the first-sync prompt.

SUGGESTION

File Line Issue
apps/mobile/src/components/security-agent/dashboard-screen.tsx 558 The "Manage repositories" row now renders only after a sync, so a never-synced scope with no repositories configured is offered only a Sync action that returns PRECONDITION_FAILED.
Files Reviewed (12 files)
  • apps/mobile/src/components/security-agent/dashboard-screen.tsx - 2 issues
  • apps/mobile/src/components/security-agent/dashboard-sections.tsx
  • apps/mobile/src/components/security-agent/notification-settings-screen.tsx
  • apps/mobile/src/components/security-agent/settings-toggle-row.tsx
  • apps/mobile/src/lib/hooks/header-back.ts
  • apps/mobile/src/lib/hooks/use-header-back.ts
  • apps/mobile/src/components/security-agent/dashboard-screen.mounted.test.tsx
  • apps/mobile/src/components/security-agent/notification-settings-screen.headers.mounted.test.tsx
  • apps/mobile/src/components/security-agent/scope-entry-screen.test.ts
  • apps/mobile/src/components/security-agent/settings-toggle-row.mounted.test.tsx
  • apps/mobile/src/lib/hooks/header-back.test.ts
  • apps/mobile/src/lib/hooks/use-header-back.mounted.test.tsx

Fix these issues in Kilo Cloud

Previous review (commit 1d4bb60)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change (one class swap in the settings toggle row) is a direction-agnostic replacement for the physical pr-3, and the earlier Back-guard suggestion is withdrawn after re-checking the navigation structure.

Assumptions: [INFERENCE] The previous suggestion on screen-header.tsx:279 is not carried forward: [scope]/_layout.tsx:33 makes index (the dashboard) the section entry, so one container-wide router.back() at that root is the intended "leave Security Agent" pop, and the burst guard covers the double-dispatch that actually changed tabs. No mobile test run was performed here; the PR reports typecheck, lint, and the full mobile vitest suite passing.

Files Reviewed (4 files)
  • apps/mobile/src/components/security-agent/settings-toggle-row.tsx (changed line 62/66) - no issues
  • apps/mobile/src/components/screen-header.tsx (targeted re-check of the prior finding) - no issues
  • apps/mobile/src/lib/hooks/use-header-back.ts (targeted re-check) - no issues
  • apps/mobile/src/app/(app)/security-agent/[scope]/_layout.tsx (targeted re-check) - no issues

Previous review (commit 1cc80e3)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The burst guard and the scope-context fallback are sound; the new Back decision still treats container-wide history as local, so one press can leave a nested stack.

Assumptions: [INFERENCE] The residual single-press case (nested stack at its own root with parent history) was accepted by the PR's proxy answer that scoped the fix to a 500 ms burst guard; only the stated "keep the press inside its own stack" goal is affected, not a crash or data path. No mobile test run was performed here; the PR reports typecheck, lint, and the full mobile vitest suite passing.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/components/screen-header.tsx 279 canGoBack comes from router.canGoBack(), which reports parent navigator history (finding-detail-back.ts:11); a press from a nested stack at its local root still dispatches router.back() and pops the parent (the tab switch described in the PR evidence). Only bursts are contained.
Files Reviewed (7 files)
  • apps/mobile/src/app/(app)/security-agent/[scope]/_layout.tsx - no issues
  • apps/mobile/src/components/screen-header.tsx - 1 suggestion
  • apps/mobile/src/components/security-agent/security-agent-scope-route.tsx - no issues
  • apps/mobile/src/components/security-agent/security-agent-scope-route.mounted.test.tsx - no issues
  • apps/mobile/src/lib/hooks/header-back.ts - no issues
  • apps/mobile/src/lib/hooks/header-back.test.ts - no issues
  • apps/mobile/src/lib/hooks/use-header-back.ts - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 71.7K · Output: 10.8K · Cached: 1M

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:human Substantive functional risk requires human review. labels Oct 30, 2026
The title column reserved its gap with a physical right padding. Under RTL
layout the title text painted into that reserved space: the design gate's
rtl cell measured the title paragraph 10.9 dp beyond its parent's content
box on the Security Agent settings screen. A row gap keeps the same 12 dp
between the copy and the switch in both directions and leaves the text
inside its column.
@iscekic iscekic removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:human Substantive functional risk requires human review. labels Oct 30, 2026
@iscekic

iscekic commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate rerun on the new head, with the shared toggle row's spacing made direction-agnostic.

  • The earlier rtl error (clipped on the settings toggle row's title) is gone: the rtl settings and notifications screens now report no findings, and the run ends with 0 errors and 6 warnings.
  • The remaining warnings are on the dashboard screen (touch target on a SectionCard row, two rtl layout warnings, one late-settling capture) and measure the same on main with this change absent.
  • Navigation behaviour rechecked on this head: Back from the restored Settings screen lands on the Security Agent dashboard, and four quick Back presses on the dashboard header, opened from the Profile tab, stay on the Profile tab.

Screen identity for each capture was asserted from the uiautomator dump taken in the same command as the screenshot.

Design gate contact sheet after the spacing change

Back from the restored settings screen lands on the dashboard

A quick Back burst on the dashboard stays on the Profile tab

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:human Substantive functional risk requires human review. merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 30, 2026

@eshurakov eshurakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot - Light review: no critical issues found.

Minor findings (non-blocking):

  1. The guard suppresses onBack handlers too. useHeaderBack claims the window before resolving, so a screen whose onBack opens a confirmation (e.g. Security Agent settings unsaved-changes) ignores a second header Back press within 500 ms of the first. That is fine for a burst, but a user who dismisses the confirmation and immediately re-taps is dropped with no feedback. Consider claiming only for the history/fallback paths, or resetting the window when onBack returns without navigating.

  2. claimHeaderBack uses Date.now(). A backwards wall-clock step (NTP/manual change) makes now - lastBackAt negative and can suppress presses for much longer than 500 ms. performance.now() or clamping a negative delta keeps the window bounded.

  3. The decision is computed after the claim, so a press resolving to 'none' still consumes the window. Harmless today, but resolving first keeps the guard state meaningful and makes the invariant easier to reason about.

  4. use-header-back.ts has no test of its own; only the pure helpers in header-back.ts are covered. A small test that the callback pops once per window (and never calls router.back() when canGoBack is false) would lock in the behavior the PR is fixing.

Claim the burst window only for a press that runs something, so a press
with no destination does not hold the next one out, and read a backwards
wall-clock step as a new burst instead of a long suppression. Cover the
hook itself: one pop per window, no pop without history, the declared
fallback, and the handler taking precedence.
Before the first sync the dashboard printed zero counts that read as a
clean result; it now offers the first sync instead of the grid and the
posture sections. An unmeasured SLA compliance tile shows the shared
no-value dash rather than a sentence in the figure slot, the action links
use the link colour in dark mode, and the repositories row keeps one sync
control. The empty "Act first" row no longer promises a destination, the
personal notification screen drops headings that restated each row title,
and the toggle row's switch draws both states from theme tokens so the
off state stays visible on either card surface.
@iscekic iscekic removed the merge:human Substantive functional risk requires human review. label Oct 30, 2026
@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. labels Oct 30, 2026
A missing owner-level sync stamp can accompany findings left by a partial
run, so the prompt now stands in for an empty result rather than for the
stamp, and it waits for the stats read. The repository-setup link also
renders before that prompt, because a sync without a configured
repository is refused.
@iscekic iscekic removed merge:blocked:insufficient-e2e Verification does not cover the change merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 30, 2026
@iscekic iscekic removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change labels Oct 30, 2026
@iscekic

iscekic commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design review fixes exercised on this VM's Android emulator (dev client on the local dev stack, seeded non-production fixture data, default font scale and density). Screen identity for each capture comes from the uiautomator dump taken in the same command as the screenshot.

  • Before the first sync the dashboard shows one prompt with the labelled sync action, and the repository-setup link stays reachable; the zero counts and the posture sections no longer render.
  • A scope whose owner-level sync stamp is missing but which holds findings keeps its real counts and sections instead of the prompt.
  • The SLA compliance tile shows the no-value dash, with the explanation in its caption.
  • Dark mode draws the action link in the link colour (capture pixel #60A5FA) instead of brand yellow; the light cell keeps its olive (#4F5A10).
  • The repositories row carries one link: the sync action exists only in the header control and in the first-sync prompt.
  • The notification rows carry no section headings restating their own titles.
  • Both switch states come from theme tokens: off track #6F6A61 on the light card (4.6:1) and #8A8680 on the dark card (4.5:1), thumbs 5.1:1 and 5.3:1 against their tracks.
  • The empty "Act first" row reports no chevron and no pressable ancestor: an accessibility dump of that row holds one icon and no clickable node.

Design gate on this head: 4 cells, 0 errors, 2 warnings. Both are the dashboard scroll row's inset-asymmetry and rtl-mirror in the right-to-left cell and measure the same with this change absent, as reported against main earlier.

The contact sheet below is the gate run for this head; the screenshots are the states it captures.

Design gate contact sheet for this head

First sync prompt with the repository setup link

Synced dashboard with the no-value SLA dash and one repositories link

Notification rows without repeated section headings

Empty act-first row without a chevron

Findings kept visible when the sync stamp is missing

@iscekic

iscekic commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

bot: On the four minor remarks in the review body (they had no threads of their own):

  1. The guard suppresses onBack too — no change, and the suggestion cannot be applied safely as written. "Claim only for the history/fallback paths" would stop guarding the handler path, but a screen's handler is free to navigate itself: apps/mobile/src/components/pr-review/pr-review-file-navigator-screen.tsx:67 and apps/mobile/src/components/profiles/profile-commands-screen.tsx:301 both pass onBack={() => { router.back(); }}, and PrReviewGateHeader passes the inbox dismissTo helper. Leaving that path unclaimed re-opens exactly the defect this change fixes: one router.back() per press, one burst popping the screens beneath. "Reset the window when onBack returns without navigating" needs the hook to know whether the handler navigated; the handler is a () => void and the router applies a pop on a later transition, so there is no synchronous answer to read — a timer or heuristic reset would either clear the window for handlers that did navigate or keep suppressing the ones that did not. The window models one gesture: the header Back of a screen that confirms is still one control, dismissing that confirmation happens on the dialog's own controls with the header covered, and 500 ms outlasts a single press.
  2. Date.now() and a backwards clock step — fixed in 1064773. The claim now reads a negative delta as a new burst, so a step cannot hold presses out for longer than the window. Unit case added.
  3. Decision computed after the claim — fixed in 1064773. The decision resolves first, and a press that resolves to nothing returns before claiming, so the window only ever records a dispatch. Unit case added.
  4. No test for use-header-back — added in 1064773: one pop per window, a second pop after the window, no router.back() without history, the declared fallback, the handler taking precedence inside one burst, and the unclaimed no-op press.

The dashboard states its sync status once before the first sync, the SLA
posture dots carry their severity, the dark off switch keeps a light thumb,
and the new-finding subtitle no longer leaves an orphan.
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 31, 2026
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design review 2 fixes exercised on this VM's Android emulator (dev client on the local dev stack, seeded non-production fixture data, default font scale and density). Screen identity for each capture comes from the accessibility dump the design gate took with it.

  • First-sync state: the repository row reads "All repositories" alone and the prompt below carries the status and its own sync action; one status statement per screen, in the base and dark cells.
  • SLA posture dots: Critical overdue red, High overdue amber, Medium & low overdue neutral, measured from the dots' own pixels (#B0483A / #956011 / #6F6A61 in the light cell, #F28B7A / #F2B05F / #8A8680 in the dark).
  • Notification settings in dark mode: the off switch now reads as a control — light thumb #EDEBE6 on track #8A8680 over the card.
  • The new-finding subtitle fits one line in the base, dark and compact cells.

Two findings are answered instead of changed, per the owner's decision: the primary action keeps the palette's primary fill in each scheme, and the setup link now uses the same primary accent as every other link, so the link and the action share one accent per mode.

Design gate on this head: 4 cells, 0 errors, 2 warnings — the dashboard scroll row's inset-asymmetry and right-to-left mirror warnings, which measure the same with this change absent.

Design gate contact sheet for the three screens

Notification settings in dark mode after the fix

Synced dashboard with severity-coded posture dots

First-sync dashboard stating the status once

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 31, 2026
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate re-run on the final head (three links: the never-synced personal dashboard, its notification settings, and a synced organization dashboard).

  • 4 cells, 0 errors, 3 warnings; all three warnings are right-to-left-cell measurements on surfaces this change does not touch (the dashboard scroll row's inset symmetry and row mirroring, and the scope entry's loading skeleton, whose four placeholder tiles are identical).
  • The entry screen re-checked on this head: the repository row reads "All repositories" alone and one "Not yet synced" node carries the state at the prompt.

Design gate contact sheet for the final head

@iscekic

iscekic commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator Author

bot: Design gate artifact of record for this head, re-run with the same target names the review uses (app-security-agent-personal, app-security-agent-personal-settings-notifications, app-security-agent-synced) so the sheets line up with the findings.

  • 4 cells, 0 errors, 3 warnings, exit code 0. The dashboard scroll row's inset symmetry and row mirroring in the right-to-left cell appear in every repeat; one more row-mirroring warning appears when the capture catches the scope entry's loading skeleton before the reads settle, which is why the count reads 2 or 3 across repeats of this head.
  • The skeleton's four placeholder tiles are identical, so mirroring them changes nothing visible.

Design gate contact sheet, three targets with the review's target names

@iscekic iscekic added the merge:blocked:human-design The owner reviews the design label Oct 31, 2026
…st-guard

# Conflicts:
#	apps/mobile/src/components/screen-header.tsx
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 31, 2026
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Re-verified the back navigation on the merge head on this VM's Android emulator (dev client on the local dev stack, owned fake account, default font scale and density).

  • Restored Settings screen, one header Back: lands on the Security Agent dashboard, no unmatched route.
  • Four header Back presses about 60 ms apart from the notifications subpage: lands on the Security Agent dashboard (one level); the same burst from Settings ends on the Profile tab with the tab bar present.
  • Two deliberate presses more than 500 ms apart: Settings → dashboard → Profile tab, so intentional navigation still walks the stack.

The attached design gate run covers the personal dashboard, its notification settings and its settings screen across the base, dark, compact and right-to-left cells: 4 cells, 0 errors, 2 warnings (both right-to-left measurements of the dashboard's own pull-to-refresh row, which this change does not touch).

Design gate contact sheet for the base, dark, compact and right-to-left cells

Security Agent dashboard after one header Back from the restored settings screen

Profile tab after a burst of four header Back presses

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Oct 31, 2026
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Same two back-navigation checks re-run on the final head (a59d47a), same emulator, stack and fixture, plus the header-level regression test.

  • Restored Settings screen, one header Back: Security Agent dashboard, no unmatched route.
  • Four header Back presses about 60 ms apart from Settings: Profile tab with the tab bar present.

The new mounted test presses the header's Back four times inside one window and asserts a single dispatch, then presses again past the window and walks one more level; it fails when the window is disabled.

Security Agent dashboard after one header Back from the restored settings screen

Profile tab after four quick header Back presses

@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate re-run after the last change, on the final head (a59d47a), same emulator and dev stack.

Three routes: the personal dashboard, its settings screen and its notification settings. Matrix standard, 4 cells (base, dark, compact, right-to-left), 12 screen-cells, exit code 0: 0 errors, 2 warnings.

Both warnings are right-to-left measurements of the dashboard's own pull-to-refresh scroll row, which this change does not touch — inset-asymmetry (top 10.3 dp vs bottom 29.7 dp) and rtl-mirror (the row's children keep their left-to-right order under RTL). They read the same in the previous run at the merge head and were recorded for that row in the earlier rounds.

Design gate contact sheet for the base, dark, compact and right-to-left cells

@iscekic iscekic removed the merge:blocked:human-design The owner reviews the design label Oct 31, 2026
@iscekic

iscekic commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator Author

bot: Visual proof re-run at head a59d47a, on this work VM's Android emulator with the dev client on the local dev stack and seeded fake-account fixtures at the default font scale and density.

Observed from uiautomator dump captured with each screenshot:

  • Header Back pressed four times rapidly — raw sendevent taps about 230 ms apart — from Security Agent settings: the Profile tab comes back with the tab bar and all four tab nodes present; there is no unmatched-route state.
  • The same rapid presses from the notification settings subpage: the Security Agent dashboard comes back, with its header, filter row and first-sync prompt.
  • Four presses inside one 500 ms window — the single-window case this guard is built for — are covered by the mounted tests in screen-header.mounted.test.tsx and use-header-back.mounted.test.tsx; on this device the fourth tap of the run above starts a new window, so the sequence shows one pop per window rather than four taps in one window.
  • Design gate: 4 cells, 12 screen-cells, 0 errors, 0 warnings on the three changed Security Agent routes, its merge-base run uploaded alongside it under the same head.

Design gate contact sheet for the changed screens

Profile tab with tab bar after a quick header Back burst

Security Agent dashboard after a quick header Back burst from its subpage

@iscekic iscekic added the merge:blocked:human-design The owner reviews the design label Oct 31, 2026
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 31, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:blocked:design-review Design review has not cleared this change merge:blocked:human-design The owner reviews the design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants