Skip to content

Commit 00174eb

Browse files
committed
fix(webapp): keep a task, agent or prompt page on an environment switch
Truncating every resource page to its list is right for a resource an environment issued an id for, but the last segment of a task, agent, prompt, playground or model page is a name that comes from the user's code or the model catalog, so it names the same thing in every environment of the project. An environment switch now keeps that name, as swapping the slug used to; a project or organization switch still truncates, since the name need not exist over there. The leak assertion is split rather than relaxed: pages addressed by an id must still resolve with the id gone, for all three switches, and the slug-addressed ones must come back whole from an environment switch. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e0719bb commit 00174eb

2 files changed

Lines changed: 112 additions & 7 deletions

File tree

apps/webapp/app/utils/pageSwitching.test.ts

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
projectPortablePage,
2121
requestedOrganizationPortablePage,
2222
requestedProjectPortablePage,
23+
SLUG_ADDRESSED_PAGES,
2324
} from "./pageSwitching";
2425

2526
const APP_DIR = join(__dirname, "..");
@@ -94,6 +95,15 @@ const environmentPages = [
9495
const idFreePages = environmentPages.filter((page) => !page.includes(":") && page !== "");
9596
const idPages = environmentPages.filter((page) => page.includes(":"));
9697

98+
const probes = idPages.map((page) => page.replace(/:[^/]+/g, PROBE));
99+
100+
function listAbove(page: string): string {
101+
return page.slice(0, page.lastIndexOf("/"));
102+
}
103+
104+
const slugAddressedProbes = probes.filter((page) => SLUG_ADDRESSED_PAGES.includes(listAbove(page)));
105+
const idAddressedProbes = probes.filter((page) => !SLUG_ADDRESSED_PAGES.includes(listAbove(page)));
106+
97107
function matchesARoute(page: string): boolean {
98108
const wanted = page === "" ? [] : page.split("/");
99109

@@ -307,18 +317,19 @@ describe("pages an organization switch cannot carry", () => {
307317

308318
describe("pages named after a resource", () => {
309319
it("truncate to a list page, id and all, for every one of them", () => {
310-
const leaks = (resolve: (page: string) => string, pages: ReadonlySet<string>) =>
311-
idPages
312-
.map((page) => page.replace(/:[^/]+/g, PROBE))
320+
const leaks = (resolve: (page: string) => string, pages: ReadonlySet<string>, from: string[]) =>
321+
from
313322
.filter((page) => {
314323
const resolved = resolve(page);
315324
return resolved.includes(PROBE) || !(resolved === "" || pages.has(resolved));
316325
})
317326
.sort();
318327

319-
expect(leaks(environmentPortablePage, ENVIRONMENT_PORTABLE_PAGES)).toEqual([]);
320-
expect(leaks(projectPortablePage, PROJECT_PORTABLE_PAGES)).toEqual([]);
321-
expect(leaks(organizationPortablePage, ORGANIZATION_PORTABLE_PAGES)).toEqual([]);
328+
expect(leaks(environmentPortablePage, ENVIRONMENT_PORTABLE_PAGES, idAddressedProbes)).toEqual(
329+
[]
330+
);
331+
expect(leaks(projectPortablePage, PROJECT_PORTABLE_PAGES, probes)).toEqual([]);
332+
expect(leaks(organizationPortablePage, ORGANIZATION_PORTABLE_PAGES, probes)).toEqual([]);
322333
});
323334

324335
it("truncate to the list they were reached from", () => {
@@ -353,6 +364,64 @@ describe("pages named after a resource", () => {
353364
});
354365
});
355366

367+
describe("pages named after something the environment did not issue", () => {
368+
it("are the ones a route below them takes a code or catalog name for", () => {
369+
expect([...new Set(slugAddressedProbes.map(listAbove))].sort()).toEqual(
370+
[...SLUG_ADDRESSED_PAGES].sort()
371+
);
372+
373+
for (const page of slugAddressedProbes) {
374+
expect(environmentPortablePage(page)).toBe(page);
375+
}
376+
});
377+
378+
it("keep their name when only the environment changes, since it names the same thing there", () => {
379+
const switched = (page: string, search = "") =>
380+
pathForEnvironmentSwitch({
381+
location: locationOn(page, search),
382+
environmentPathname: environmentLocation.pathname,
383+
environmentSlug: "prod",
384+
});
385+
386+
expect(switched("tasks/standard/my-task", "?period=1d")).toBe(
387+
"/orgs/acme/projects/api/env/prod/tasks/standard/my-task?period=1d"
388+
);
389+
expect(switched("tasks/scheduled/my-task")).toBe(
390+
"/orgs/acme/projects/api/env/prod/tasks/scheduled/my-task"
391+
);
392+
expect(switched("test/tasks/my-task")).toBe(
393+
"/orgs/acme/projects/api/env/prod/test/tasks/my-task"
394+
);
395+
expect(switched("agents/my-agent")).toBe("/orgs/acme/projects/api/env/prod/agents/my-agent");
396+
expect(switched("playground/my-agent")).toBe(
397+
"/orgs/acme/projects/api/env/prod/playground/my-agent"
398+
);
399+
expect(switched("prompts/my-prompt")).toBe(
400+
"/orgs/acme/projects/api/env/prod/prompts/my-prompt"
401+
);
402+
expect(switched("models/gpt-5")).toBe("/orgs/acme/projects/api/env/prod/models/gpt-5");
403+
});
404+
405+
it("fall back to their list page when the project or organization changes, which may not have the name", () => {
406+
expect(projectPortablePage("tasks/standard/my-task")).toBe("");
407+
expect(projectPortablePage("test/tasks/my-task")).toBe("test");
408+
expect(projectPortablePage("agents/my-agent")).toBe("agents");
409+
expect(organizationPortablePage("prompts/my-prompt")).toBe("prompts");
410+
expect(organizationPortablePage("models/gpt-5")).toBe("models");
411+
});
412+
413+
it("keep nothing but a single plain name in that last segment", () => {
414+
expect(environmentPortablePage("tasks/standard/..%2f..%2flogin")).toBe("");
415+
expect(environmentPortablePage("tasks/standard/../../login")).toBe("");
416+
expect(environmentPortablePage("agents/%2e%2e")).toBe("agents");
417+
expect(environmentPortablePage("agents/..")).toBe("agents");
418+
expect(environmentPortablePage("agents/%zz")).toBe("agents");
419+
expect(environmentPortablePage("agents/")).toBe("agents");
420+
expect(environmentPortablePage("models/my%2Fmodel")).toBe("models");
421+
expect(environmentPortablePage("prompts/my-prompt/extra")).toBe("prompts");
422+
});
423+
});
424+
356425
describe("a page suffix that is not a plain relative page", () => {
357426
it("falls back to the environment root rather than being sanitised into one", () => {
358427
expect(projectPortablePage("/apikeys")).toBe("");
@@ -386,6 +455,9 @@ describe("a page suffix that is not a plain relative page", () => {
386455
"settings/general/../../..",
387456
"/branches",
388457
"..%2fbranches",
458+
"tasks/standard/../../login",
459+
"agents/..%2f..%2flogin",
460+
"models/%2f%2fevil.example.com",
389461
];
390462

391463
for (const attempt of attempts) {

apps/webapp/app/utils/pageSwitching.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ export const PROJECT_SPECIFIC_PAGES = ["branches", "dev-branches"];
2626
/** Gated by an organization feature flag, so their loaders turn you away in an organization without it. */
2727
export const ORGANIZATION_SPECIFIC_PAGES = ["logs", "query", "dashboards/queues"];
2828

29+
/**
30+
* Pages whose last segment is a name the user's code or the model catalog decides, rather than an
31+
* id one environment issued, so the same address names the same thing in every environment of the
32+
* project. Another project need not have that name, so only an environment switch carries it.
33+
*/
34+
export const SLUG_ADDRESSED_PAGES = [
35+
"agents",
36+
"models",
37+
"playground",
38+
"prompts",
39+
"tasks/scheduled",
40+
"tasks/standard",
41+
"test/tasks",
42+
];
43+
2944
/** Every page below an environment that names no resource, so any environment can render it. */
3045
export const ENVIRONMENT_PORTABLE_PAGES: ReadonlySet<string> = new Set(
3146
[
@@ -60,9 +75,27 @@ function nearestPage(suffix: string, pages: ReadonlySet<string>): string {
6075
return "";
6176
}
6277

78+
/**
79+
* `suffix` itself when it is a slug-addressed page, as long as the slug is a single plain segment —
80+
* a traversal or an encoded path in its place falls through to the list page above it.
81+
*/
82+
function slugAddressedPage(suffix: string): string | undefined {
83+
const boundary = suffix.lastIndexOf("/");
84+
if (boundary < 1 || !SLUG_ADDRESSED_PAGES.includes(suffix.slice(0, boundary))) return undefined;
85+
86+
let slug: string;
87+
try {
88+
slug = decodeURIComponent(suffix.slice(boundary + 1));
89+
} catch {
90+
return undefined;
91+
}
92+
93+
return slug !== "" && !/^\.+$/.test(slug) && !/[/\\]/.test(slug) ? suffix : undefined;
94+
}
95+
6396
/** The page to keep when only the environment changes. */
6497
export function environmentPortablePage(suffix: string): string {
65-
return nearestPage(suffix, ENVIRONMENT_PORTABLE_PAGES);
98+
return slugAddressedPage(suffix) ?? nearestPage(suffix, ENVIRONMENT_PORTABLE_PAGES);
6699
}
67100

68101
/** The page to keep when the project changes. */

0 commit comments

Comments
 (0)