Skip to content

Commit 6bb7480

Browse files
authored
fix(mobile): keep the current screen and draft when a deep link opens nothing (#9372)
* fix(mobile): leave the current screen alone when a deep link opens nothing The session row of the universal-link table accepts any capture, so `/cloud/sessions/<anything>` reached the session route, which answers an id it cannot resolve with the New-session composer. A path with no table route was handed to the resolver, which selected a tab. Either way the visible route was replaced and the screen beneath it, including an open form and its unsaved input, was unmounted, with nothing shown to the user. `redirectSystemPath` now follows a link only when it names something the app can open. A session capture must be `new` or a real session id, and a path without a table route must address the app's own route namespace: a route group (`/(app)`, `/(auth)`), one of the families the phone's search indexes, or a root-level route. Everything else returns the falsy handled value, so the resolver never runs. Every published universal link, app-action URL, share intent, remote-MCP return, fixture and dev app-route link is unchanged. * fix(mobile): share one destination rule with the cold capture and action URLs Review follow-ups on the same defect class: - The synchronous cold-launch capture stashed a session link whose capture cannot open, and the slot is the layout's only navigation input, so a cold launch still opened the composer. It now captures only a destination that opens. - The group-stripped `agent-chat/` family link (how Android delivers a tap on a search result) reached the session route unchecked; it takes the same session rule. - `open-session` action URLs took the same unchecked session capture. `src/lib/deep-link-destination.ts` owns the answers, and the session row's target prefix is imported from `system-search-families.ts` instead of declared a second time. * fix(mobile): apply the session rule to the group-prefixed link form too Review follow-ups: - `opensAppRoutePath` checked the session param only on the group-stripped form, so the group-prefixed form the design gate and the mobile docs use still resolved to the composer fallback for an unusable id. Both forms now reduce to the session row's href and take the same rule. - The committed seed fixtures recorded deep links outside the form the app forwards (`kiloapp:///code-reviewer/...`, `kiloapp://agent-chat/...`, `kiloapp://agent-briefing/...`), which `dev/seed/AGENTS.md` asks them to record as the link that opens the fixture. They now use the group-prefixed form, and a test keeps every seed link on that form (or on a `dev/` fixture link, which `+native-intent` answers before the mapper). * fix(mobile): keep the slash-command seed link on the form the app opens The seed for the profile slash-command surface landed on main while this branch was open, with the same group-stripped link form the other seeds had. Fold it into the group-prefixed form, and format the seed files the earlier edits pushed past the line width. * fix(mobile): line the secret switch up with the row's icons The native switch draws its track inside a box that reserves the thumb's overhang, so the visible track ended about 12dp inside the box and the row's padding left it 23dp from the row's edge against the 13dp the lock glyph keeps from its opposite edge. Pull the box out by the row's padding with a logical end margin, so the track's end matches the glyph's inset in both writing directions.
1 parent e933d30 commit 6bb7480

20 files changed

Lines changed: 428 additions & 65 deletions

‎apps/mobile/src/app/+native-intent.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ import { modelPickerFixtureRoute } from '@/lib/model-picker-fixture';
1010
// Composes the native-intent concerns in order: the share-extension check
1111
// returns early before web-path mapping (a share URL is never a web route), then
1212
// an app-action URL is handled by its own contract, and only what neither owns
13-
// reaches the universal-link mapper. Both handled branches return falsy
14-
// (`deep-link-handler.ts`); a truthy return re-dispatches the linking resolver.
13+
// reaches the universal-link mapper. Every handled or ignored link returns
14+
// falsy (`deep-link-handler.ts`); a truthy return re-dispatches the linking
15+
// resolver.
1516
export function redirectSystemPath({ path, initial }: { path: string; initial: boolean }) {
1617
let shareKey: string | null = null;
1718
try {

‎apps/mobile/src/components/profiles/profile-variables-rows.tsx‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -330,11 +330,21 @@ export function VariableEditForm({
330330
<Text className="flex-1 text-sm font-medium text-foreground">
331331
{t('profiles.secrets.markSecret')}
332332
</Text>
333-
<Switch
334-
value={isSecret}
335-
accessibilityLabel={t('profiles.secrets.markSecret')}
336-
onValueChange={setIsSecret}
337-
/>
333+
{/* The native switch draws its 32dp track inside a box that reserves the
334+
thumb's overhang, so the track ends about 12dp before the box does
335+
and the row's 10.5dp padding leaves it 23dp from the row's edge,
336+
twice the 13dp the lock glyph keeps from its opposite edge. Pulling
337+
the box out by the padding itself lines the track's end up with the
338+
glyph on both edges. The pull is on the END edge, so it must stay
339+
logical: `marginInlineEnd` follows the writing direction, while a
340+
hand-picked `-mr-3` lands on the title's side under RTL. */}
341+
<View className="-me-3">
342+
<Switch
343+
value={isSecret}
344+
accessibilityLabel={t('profiles.secrets.markSecret')}
345+
onValueChange={setIsSecret}
346+
/>
347+
</View>
338348
</View>
339349
<View className="flex-row gap-2">
340350
<Button variant="outline" className="flex-1" onPress={onCancel}>

‎apps/mobile/src/lib/app-actions/action-url-handler.test.ts‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import {
1111
import { handleAppActionPath } from './action-url-handler';
1212
import { getPendingAppAction, takePendingAppAction } from './pending-app-action';
1313

14+
const SESSION_ID = 'ses_0123456789abcdefghijklmnop';
15+
1416
vi.mock('@sentry/react-native', () => ({ captureException: vi.fn() }));
1517

1618
beforeEach(() => {
@@ -39,11 +41,11 @@ describe('handleAppActionPath', () => {
3941
it('stashes a session URL on the universal-link rails', () => {
4042
expect(
4143
handleAppActionPath({
42-
path: 'kiloapp:///actions/open-session?sessionId=ses_1',
44+
path: `kiloapp:///actions/open-session?sessionId=${SESSION_ID}`,
4345
initial: false,
4446
})
4547
).toBe(true);
46-
expect(getPendingDeepLink()).toBe('/(app)/agent-chat/ses_1');
48+
expect(getPendingDeepLink()).toBe(`/(app)/agent-chat/${SESSION_ID}`);
4749
expect(getPendingAppAction()).toBeNull();
4850
});
4951

@@ -57,11 +59,24 @@ describe('handleAppActionPath', () => {
5759
expect(getPendingDeepLink()).toBe('/(app)/pr-review/o/r/7');
5860
});
5961

62+
it('opens nothing for a session id the route cannot resolve', () => {
63+
// Still the contract's URL, but there is no destination to open: stashing
64+
// the composer fallback would replace the current screen with it.
65+
expect(
66+
handleAppActionPath({
67+
path: 'kiloapp:///actions/open-session?sessionId=not-a-session',
68+
initial: false,
69+
})
70+
).toBe(true);
71+
expect(getPendingDeepLink()).toBeNull();
72+
expect(getPendingAppAction()).toBeNull();
73+
});
74+
6075
it('accepts the scheme-less path expo-router hands redirectSystemPath', () => {
6176
expect(
62-
handleAppActionPath({ path: '/actions/open-session?sessionId=ses_1', initial: true })
77+
handleAppActionPath({ path: `/actions/open-session?sessionId=${SESSION_ID}`, initial: true })
6378
).toBe(true);
64-
expect(getPendingDeepLink()).toBe('/(app)/agent-chat/ses_1');
79+
expect(getPendingDeepLink()).toBe(`/(app)/agent-chat/${SESSION_ID}`);
6580
});
6681

6782
it('leaves anything that is not an action URL to the caller', () => {
@@ -108,23 +123,23 @@ describe('handleAppActionPath', () => {
108123
});
109124

110125
it('does not re-stash a cold action when the launch capture already stashed', () => {
111-
_setGetLinkingURLForTests(() => 'https://app.kilo.ai/cloud/sessions/ses_9');
126+
_setGetLinkingURLForTests(() => `https://app.kilo.ai/cloud/sessions/${SESSION_ID}`);
112127
captureLaunchDeepLink();
113-
expect(getPendingDeepLink()).toBe('/(app)/agent-chat/ses_9');
128+
expect(getPendingDeepLink()).toBe(`/(app)/agent-chat/${SESSION_ID}`);
114129
expect(
115130
handleAppActionPath({
116-
path: 'kiloapp:///actions/open-session?sessionId=ses_1',
131+
path: `kiloapp:///actions/open-session?sessionId=${SESSION_ID}`,
117132
initial: true,
118133
})
119134
).toBe(true);
120135
expect(getPendingDeepLink()).toBeNull();
121136
// A warm arrival has no launch capture to guard against, so it stashes.
122137
expect(
123138
handleAppActionPath({
124-
path: 'kiloapp:///actions/open-session?sessionId=ses_1',
139+
path: `kiloapp:///actions/open-session?sessionId=${SESSION_ID}`,
125140
initial: false,
126141
})
127142
).toBe(true);
128-
expect(getPendingDeepLink()).toBe('/(app)/agent-chat/ses_1');
143+
expect(getPendingDeepLink()).toBe(`/(app)/agent-chat/${SESSION_ID}`);
129144
});
130145
});

‎apps/mobile/src/lib/app-actions/action-url-handler.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
// cannot turn into a destination parks too: nothing opens, and the tabs layout
1010
// reports the contract's refusal instead of the app arriving unexplained.
1111

12+
import { opensDestination } from '@/lib/deep-link-destination';
1213
import { setPendingDeepLink, wasLaunchLinkHandled } from '@/lib/deep-link-launch';
1314

1415
import { appActionHref, parseAppActionUrl } from './app-action-contract';
@@ -20,9 +21,10 @@ import { setPendingAppAction } from './pending-app-action';
2021
* `false` means the path is not an action URL and the caller keeps going — the
2122
* universal-link mapper resolves nothing for these paths and must own the rest.
2223
* `true` means it belongs to this contract, including a link the provider
23-
* resolvers cannot turn into a destination: nothing is opened — the request
24-
* parks for the tabs consumer, which reports the refusal — and the path still
25-
* must not be mapped as a web route.
24+
* resolvers cannot turn into a destination (parked for the tabs consumer, which
25+
* reports the refusal) and a session destination the route cannot open (nothing
26+
* opens, and the current screen keeps its state): the path still must not be
27+
* mapped as a web route.
2628
*
2729
* The stash is the identical call `deep-link-handler.ts:71,76` makes, cold-start
2830
* guard included: the synchronous launch capture may already have stashed this
@@ -57,6 +59,12 @@ export function handleAppActionPath({
5759
setPendingAppAction(request);
5860
return true;
5961
}
62+
if (!opensDestination(resolved)) {
63+
// A session destination the route cannot open is not a destination: the
64+
// same rule `deep-link-handler.ts` applies, so a hand-built action URL
65+
// cannot replace the current screen with the composer. Nothing opens.
66+
return true;
67+
}
6068
if (initial ? !wasLaunchLinkHandled() : true) {
6169
setPendingDeepLink(resolved, 'universal-link');
6270
}
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// What an incoming link may open. The two shapes that used to replace the
2+
// visible screen — a session capture that cannot name a session, and a path
3+
// outside the app's route namespace — are rejected here, so the navigator never
4+
// resolves them and the screen beneath keeps its unsaved state.
5+
6+
// eslint-disable-next-line import/no-nodejs-modules -- vitest-only check, runs in node, never bundled into the app
7+
import { readdirSync, readFileSync } from 'node:fs';
8+
// eslint-disable-next-line import/no-nodejs-modules -- vitest-only check, runs in node, never bundled into the app
9+
import { fileURLToPath } from 'node:url';
10+
11+
import { describe, expect, it } from 'vitest';
12+
13+
import { APP_ROUTE_ROOTS, opensAppRoutePath, opensDestination } from './deep-link-destination';
14+
15+
const SESSION_ID = 'ses_0123456789abcdefghijklmnop';
16+
17+
describe('opensDestination', () => {
18+
it.each([
19+
'/(app)/(tabs)/(0_home)',
20+
'/(app)/(tabs)/(3_profile)',
21+
'/(app)/security-agent/personal/findings',
22+
'/(app)/pr-review/Kilo-Org/cloud/6234',
23+
'/(app)/agent-chat/new',
24+
`/(app)/agent-chat/${SESSION_ID}`,
25+
])('opens the table destination %s', href => {
26+
expect(opensDestination(href)).toBe(true);
27+
});
28+
29+
it.each([
30+
'/(app)/agent-chat/not-a-session',
31+
'/(app)/agent-chat/00000000-0000-0000-0000-000000000000',
32+
'/(app)/agent-chat/ses_short',
33+
])('refuses the session destination %s', href => {
34+
expect(opensDestination(href)).toBe(false);
35+
});
36+
});
37+
38+
describe('opensAppRoutePath', () => {
39+
it.each([
40+
'kiloapp:///(app)/profile-management/34d743e6-7bb8-4dd1-880c-580bd8d907d5/variables',
41+
'kiloapp:///(auth)/session-row-fixture',
42+
'kiloapp:///force-update',
43+
'kiloapp:///button-label-fixture',
44+
'/(app)/agent-chat/new?mode=code',
45+
'/(auth)/login',
46+
`kiloapp:///(app)/agent-chat/${SESSION_ID}`,
47+
'kiloapp:///(app)/agent-chat/new',
48+
`kiloapp://agent-chat/${SESSION_ID}`,
49+
'kiloapp://pr-review/Kilo-Org/cloud/6234',
50+
'kiloapp://security-agent/personal',
51+
])('opens the app route %s', path => {
52+
expect(opensAppRoutePath(path)).toBe(true);
53+
});
54+
55+
it.each([
56+
'kiloapp:///',
57+
'kiloapp://',
58+
'kiloapp:///definitely-not-a-route',
59+
'kiloapp:///dev/unknown-fixture',
60+
'https://app.kilo.ai/admin',
61+
'https://example.com/profile',
62+
'not a url',
63+
'kiloapp://agent-chat/not-a-session',
64+
'kiloapp:///(app)/agent-chat/not-a-session',
65+
'/(app)/agent-chat/ses_short',
66+
])('refuses %s', path => {
67+
expect(opensAppRoutePath(path)).toBe(false);
68+
});
69+
});
70+
71+
describe('route namespace', () => {
72+
it('names every root entry of the route directory', () => {
73+
// A new root-level route must be opened deliberately: an incoming link
74+
// reaches the navigator only through one of these names (or the indexed
75+
// families), so a route missing here is unreachable by link.
76+
const entries = readdirSync(fileURLToPath(new URL('../app', import.meta.url)), {
77+
withFileTypes: true,
78+
})
79+
.map(entry => entry.name)
80+
// `_layout` and `+`-prefixed files are navigator plumbing, not routes.
81+
.filter(name => !name.startsWith('_') && !name.startsWith('+'))
82+
.filter(name => !/\.test\.tsx?$/.test(name))
83+
.map(name => name.replace(/\.(tsx|ts|jsx|js)$/, ''));
84+
85+
expect([...new Set(entries)].sort()).toEqual([...APP_ROUTE_ROOTS].sort());
86+
});
87+
});
88+
89+
describe('seed fixture links', () => {
90+
it('records the link form the app opens', () => {
91+
// `dev/seed/AGENTS.md` requires a fixture to record the deep link that opens
92+
// it, and `opensAppRoutePath` forwards only the group-prefixed form (or a
93+
// `dev/` fixture link, which `+native-intent` answers before the mapper).
94+
const files = seedSources(fileURLToPath(new URL('../../../../dev/seed', import.meta.url)));
95+
const links = files.flatMap(file =>
96+
[...readFileSync(file, 'utf8').matchAll(/kiloapp:\/\/[^\s'"`]+/g)].map(match => match[0])
97+
);
98+
99+
expect(links.length).toBeGreaterThan(0);
100+
for (const link of links) {
101+
expect(
102+
link.startsWith('kiloapp:///(app)/') || link.startsWith('kiloapp:///dev/'),
103+
`${link} does not follow the recorded link form`
104+
).toBe(true);
105+
}
106+
});
107+
});
108+
109+
function seedSources(directory: string): string[] {
110+
return readdirSync(directory, { withFileTypes: true }).flatMap(entry => {
111+
const path = `${directory}/${entry.name}`;
112+
if (entry.isDirectory()) {
113+
return seedSources(path);
114+
}
115+
return entry.name.endsWith('.ts') ? [path] : [];
116+
});
117+
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* What an incoming link may open.
3+
*
4+
* A link that names no screen the app can show must not reach the navigator:
5+
* resolving a link replaces the visible route and unmounts the screen beneath
6+
* it, so an open form and its unsaved input are lost even though the link
7+
* opened nothing anyone asked for. The two consumers are `redirectSystemPath`
8+
* (a warm arrival, which also forwards the app's own routes) and the
9+
* synchronous cold-launch capture in `deep-link-launch.ts` (which only needs the
10+
* table destination). One module owns both answers so they cannot disagree.
11+
*/
12+
13+
import { parseKiloWebPath } from '@kilocode/app-shared/universal-links';
14+
15+
import { isPlausibleKiloSessionId } from './session-id-shape';
16+
import {
17+
deeplinkPathFromHref,
18+
INDEXED_LINK_PREFIXES,
19+
SESSION_HREF_PREFIX,
20+
} from './system-search-families';
21+
22+
/**
23+
* Root entries of `src/app` a link may address directly: the two route groups
24+
* and the root-level screens. The universal-link table routes the app's
25+
* published links; these are the in-app routes a dev or E2E link opens
26+
* (`kiloapp:///(app)/…`, `kiloapp:///force-update`).
27+
*
28+
* `deep-link-destination.test.ts` keeps the list equal to the route
29+
* directory's entries.
30+
*/
31+
export const APP_ROUTE_ROOTS: readonly string[] = [
32+
'(app)',
33+
'(auth)',
34+
'button-label-fixture',
35+
'force-update',
36+
];
37+
38+
/** Route param that opens the New-session composer instead of a session. */
39+
const NEW_SESSION_CAPTURE = 'new';
40+
41+
/** The session family's group-stripped prefix, as an indexed link carries it. */
42+
const SESSION_FAMILY_PREFIX = deeplinkPathFromHref(SESSION_HREF_PREFIX);
43+
44+
/**
45+
* Whether a universal-link destination opens something the app can show. Every
46+
* row but the session row names a fixed screen; the session row accepts any
47+
* capture, so its capture has to be the composer's `new` or an id the route can
48+
* resolve. `isPlausibleKiloSessionId` is the same shape guard the route uses
49+
* before it decides an id can never be a session.
50+
*/
51+
export function opensDestination(href: string): boolean {
52+
if (!href.startsWith(SESSION_HREF_PREFIX)) {
53+
return true;
54+
}
55+
const capture = href.slice(SESSION_HREF_PREFIX.length);
56+
return capture === NEW_SESSION_CAPTURE || isPlausibleKiloSessionId(capture);
57+
}
58+
59+
/**
60+
* The pathname of a raw link: the path of one of our URL forms, or the
61+
* scheme-less path expo-router hands `redirectSystemPath`. Null when the link
62+
* carries no path of ours at all (a bare `kiloapp://`, or another host).
63+
*/
64+
function pathnameOf(path: string): string | null {
65+
const webPath = parseKiloWebPath(path);
66+
if (webPath !== null) {
67+
return webPath;
68+
}
69+
if (!path.startsWith('/')) {
70+
return null;
71+
}
72+
const end = path.search(/[?#]/);
73+
return end === -1 ? path : path.slice(0, end);
74+
}
75+
76+
/**
77+
* Whether a link addresses the app's own routes — the paths this app, not the
78+
* web table, owns. A dev or E2E link carries the route with its group segment
79+
* (`kiloapp:///(app)/…`, the form the design gate and the mobile docs use); a
80+
* system-search tap is delivered as the group-stripped family link the app
81+
* itself indexed (`kiloapp://agent-chat/…`). The session route appears in both
82+
* forms and its param must name a screen either way.
83+
*/
84+
export function opensAppRoutePath(path: string): boolean {
85+
const pathname = pathnameOf(path);
86+
if (pathname === null) {
87+
return false;
88+
}
89+
const withoutLeadingSlash = pathname.replace(/^\/+/, '');
90+
// One href for the session route, whichever form the link carries: with the
91+
// group segment (dev and E2E links) or without it (a search tap).
92+
const sessionHref = withoutLeadingSlash.startsWith(SESSION_FAMILY_PREFIX)
93+
? `${SESSION_HREF_PREFIX}${withoutLeadingSlash.slice(SESSION_FAMILY_PREFIX.length)}`
94+
: `/${withoutLeadingSlash}`;
95+
if (sessionHref.startsWith(SESSION_HREF_PREFIX)) {
96+
return opensDestination(sessionHref);
97+
}
98+
if (INDEXED_LINK_PREFIXES.some(prefix => withoutLeadingSlash.startsWith(prefix))) {
99+
return true;
100+
}
101+
const root = withoutLeadingSlash.split('/')[0];
102+
return root !== undefined && APP_ROUTE_ROOTS.includes(root);
103+
}

0 commit comments

Comments
 (0)