Skip to content

Commit 24998c0

Browse files
fix: address code review findings for URL routing (#180)
- Decode URI-encoded session IDs in parsePath (copilot:abc → works) - Use router.navigateToSession directly in callers to avoid race with route-change effect when navigating from non-session pages - Respect basePath in SubagentInline href via buildSessionHref helper - Preserve filter params when deselecting session (navigateFromSession) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c818772 commit 24998c0

File tree

6 files changed

+38
-18
lines changed

6 files changed

+38
-18
lines changed

frontend/src/App.svelte

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,25 @@
236236
});
237237
});
238238
239+
// Build URL params from current session filters.
240+
function buildFilterParams(): Record<string, string> {
241+
const f = sessions.filters;
242+
const p: Record<string, string> = {};
243+
if (f.project) p.project = f.project;
244+
if (f.machine) p.machine = f.machine;
245+
if (f.agent) p.agent = f.agent;
246+
if (f.date) p.date = f.date;
247+
if (f.dateFrom) p.date_from = f.dateFrom;
248+
if (f.dateTo) p.date_to = f.dateTo;
249+
if (f.recentlyActive) p.active_since = "true";
250+
if (f.hideUnknownProject) p.exclude_project = "unknown";
251+
if (f.minMessages > 0) p.min_messages = String(f.minMessages);
252+
if (f.maxMessages > 0) p.max_messages = String(f.maxMessages);
253+
if (f.minUserMessages > 0) p.min_user_messages = String(f.minUserMessages);
254+
if (f.includeOneShot) p.include_one_shot = "true";
255+
return p;
256+
}
257+
239258
// Sync active session to URL.
240259
$effect(() => {
241260
const activeId = sessions.activeSessionId;
@@ -247,7 +266,7 @@
247266
if (activeId) {
248267
router.navigateToSession(activeId);
249268
} else {
250-
router.navigateFromSession();
269+
router.navigateFromSession(buildFilterParams());
251270
}
252271
});
253272
});

frontend/src/lib/components/analytics/TopSessions.svelte

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,7 @@
1717
}
1818
1919
function handleSessionClick(id: string) {
20-
if (router.route !== "sessions") {
21-
const params: Record<string, string> = {};
22-
if (analytics.includeOneShot) {
23-
params["include_one_shot"] = "true";
24-
}
25-
router.navigate("sessions", params);
26-
}
20+
router.navigateToSession(id);
2721
sessions.selectSession(id);
2822
}
2923
</script>

frontend/src/lib/components/content/SubagentInline.svelte

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { formatTokenCount } from "../../utils/format.js";
77
import { computeMainModel } from "../../utils/model.js";
88
import { sessions } from "../../stores/sessions.svelte.js";
9-
import { router } from "../../stores/router.svelte.js";
9+
import { router, buildSessionHref } from "../../stores/router.svelte.js";
1010
import MessageContent from "./MessageContent.svelte";
1111
1212
interface Props {
@@ -45,9 +45,7 @@
4545
async function openAsSession(e: MouseEvent) {
4646
e.preventDefault();
4747
e.stopPropagation();
48-
if (router.route !== "sessions") {
49-
router.navigate("sessions");
50-
}
48+
router.navigateToSession(sessionId);
5149
await sessions.navigateToSession(sessionId);
5250
}
5351
@@ -86,7 +84,7 @@
8684
{/if}
8785
</button>
8886
<a
89-
href="/sessions/{sessionId}"
87+
href={buildSessionHref(sessionId)}
9088
class="open-session-link"
9189
onclick={openAsSession}
9290
title="Open as full session"

frontend/src/lib/components/pinned/PinnedPage.svelte

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@
2424
2525
function navigateToPin(sessionId: string, ordinal: number) {
2626
ui.scrollToOrdinal(ordinal, sessionId);
27-
if (router.route !== "sessions") {
28-
router.navigate("sessions");
29-
}
27+
router.navigateToSession(sessionId);
3028
sessions.navigateToSession(sessionId);
3129
}
3230

frontend/src/lib/stores/router.svelte.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,18 @@ const VALID_ROUTES: ReadonlySet<string> = new Set<Route>([
1515

1616
const DEFAULT_ROUTE: Route = "sessions";
1717

18-
function getBasePath(): string {
18+
export function getBasePath(): string {
1919
const base = document.querySelector("base");
2020
if (!base) return "";
2121
const href = base.getAttribute("href") ?? "";
2222
return href.replace(/\/+$/, "");
2323
}
2424

25+
/** Build a full URL path for a session, respecting basePath. */
26+
export function buildSessionHref(id: string): string {
27+
return `${getBasePath()}/sessions/${encodeURIComponent(id)}`;
28+
}
29+
2530
export function parsePath(): {
2631
route: Route;
2732
sessionId: string | null;
@@ -44,7 +49,7 @@ export function parsePath(): {
4449

4550
const sessionId =
4651
route === "sessions" && segments.length >= 2
47-
? segments[1]!
52+
? decodeURIComponent(segments[1]!)
4853
: null;
4954

5055
const params = Object.fromEntries(

frontend/src/lib/stores/router.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ describe("parsePath", () => {
8282
expect(result.sessionId).toBeNull();
8383
});
8484

85+
it("decodes encoded session IDs", () => {
86+
setURL("/sessions/copilot%3Aabc123");
87+
const result = parsePath();
88+
expect(result.sessionId).toBe("copilot:abc123");
89+
});
90+
8591
it("strips basePath from pathname", () => {
8692
const base = document.createElement("base");
8793
base.href = "/agentsview/";

0 commit comments

Comments
 (0)