From fd580d36fe263a9a25e386f62d201d74b1af7fbb Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Wed, 30 Sep 2026 15:22:18 +0200 Subject: [PATCH 1/3] Tell the buddy which page it is on, and open its app links in place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backend's new get_app_guide tool answers "where do I…" questions about the app itself. Two things make its answers usable here: - streamMessage sends the pathname the dock is floating over as currentPage, so "what can I do on this page?" has a "this page". - Root-relative links in a buddy reply navigate in place instead of reloading the app in a new tab (same change as on the 311 branch, so the two merge cleanly). Plus a status label for the tool while it runs. Co-Authored-By: Claude Opus 5.5 --- .../buddy/components/BuddyMarkdown.tsx | 32 ++++++++++++++--- .../buddy/hooks/useBuddyConversation.ts | 5 +++ src/features/buddy/toolLabel.ts | 2 ++ src/services/buddyService.ts | 9 ++++- .../features/buddy/BuddyMarkdown.test.tsx | 35 +++++++++++++++++++ tests/unit/features/buddy/toolLabel.test.ts | 1 + tests/unit/services/buddyService.test.ts | 26 ++++++++++++++ 7 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 tests/unit/features/buddy/BuddyMarkdown.test.tsx diff --git a/src/features/buddy/components/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index ca7256c01..95f2e4d1d 100644 --- a/src/features/buddy/components/BuddyMarkdown.tsx +++ b/src/features/buddy/components/BuddyMarkdown.tsx @@ -1,7 +1,23 @@ import { memo } from "react"; import ReactMarkdown from "react-markdown"; +import { Link } from "react-router-dom"; import remarkGfm from "remark-gfm"; +/** + * Whether a link the buddy wrote points inside the app. + * + * The mentor is given the paths of the things it talks about — a step, a question, a phase — so that + * "want to take the check?" can arrive as something clickable. Those are app paths, and opening one + * in a new tab would reload the whole SPA and lose the conversation the hire was having. + * + * Root-relative only, and deliberately: a protocol-relative `//evil.example` is also "relative" to a + * careless check, and the model's output is not a place to be careless. `/\evil.example` is the same + * thing in disguise -- browsers read the backslash as a slash. + */ +function isInAppPath(href: string | undefined): href is string { + return href !== undefined && href.startsWith("/") && href[1] !== "/" && href[1] !== "\\"; +} + /** * Renders the buddy's reply as Markdown (GitHub-flavoured), so lists, bold, headings, links, * code and tables come through instead of raw asterisks and backticks. Shared by both buddy @@ -34,11 +50,17 @@ function BuddyMarkdownImpl({ content }: { content: string }) { ( - - {children} - - ), + // An app path navigates in place; anything else is still somebody else's site in a new + // tab. The dock stays mounted across a route change, so a hire who follows a link into + // their path keeps the conversation that sent them there. + a: ({ children, href }) => + isInAppPath(href) ? ( + {children} + ) : ( + + {children} + + ), table: ({ children }) => (
diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index a38440876..efce291a3 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -728,6 +728,11 @@ export function useBuddyConversation( }, // Read at call time: a turn speaks to whichever conversation is current when it starts. teamProjectIdRef.current ?? undefined, + // The page the dock is floating over, for the buddy's app guide. Read from the window at + // send time rather than through `useLocation`, which would re-render the one app-wide + // conversation on every navigation to learn something only a send needs. The pathname + // is all the guide matches on, so the query string stays on this side. + window.location.pathname, ); // The turn is over — the first moment a card placed mid-answer is certainly on the board. diff --git a/src/features/buddy/toolLabel.ts b/src/features/buddy/toolLabel.ts index 37c0fdb14..e3bded4c7 100644 --- a/src/features/buddy/toolLabel.ts +++ b/src/features/buddy/toolLabel.ts @@ -7,6 +7,8 @@ const TOOL_LABELS: Record = { get_my_metrics: "Checking your progress…", get_my_competencies: "Looking at where you stand…", get_suggested_tasks: "Finding good tasks for you…", + // Mounted in both modes: everybody asks where things are in the app. + get_app_guide: "Checking where that is in the app…", // Team-mode reads: the manager is asking about their *people*, and the labels say so. get_team_attention: "Seeing who needs attention…", diff --git a/src/services/buddyService.ts b/src/services/buddyService.ts index 7457583af..479590ab1 100644 --- a/src/services/buddyService.ts +++ b/src/services/buddyService.ts @@ -392,17 +392,24 @@ async function readBuddyStream( * @param teamProjectId Pass to speak in *team mode* about a managed project instead of the hire's * own conversation. Sent in the body, not the query string — the backend's contract puts the * team target on the POST body and leaves the hire's own conversation the body-less default. + * @param currentPage The app path the sender is on (`/team-management`), so the buddy's app + * guide can answer "where is this here?". Omitted when unknown, like `teamProjectId`. */ export async function streamMessage( content: string, handlers: BuddyStreamHandlers, teamProjectId?: string, + currentPage?: string, ): Promise { const outcome = await readBuddyStream( `/api/v1/onboarding/me/buddy/messages`, // Omitted, never null: the backend treats an absent field as the hire's own conversation, // and carrying `teamProjectId: null` would send a field the contract does not have. - teamProjectId ? { content, teamProjectId } : { content }, + { + content, + ...(teamProjectId ? { teamProjectId } : {}), + ...(currentPage ? { currentPage } : {}), + }, (event) => { switch (event.type) { case "tool_use": diff --git a/tests/unit/features/buddy/BuddyMarkdown.test.tsx b/tests/unit/features/buddy/BuddyMarkdown.test.tsx new file mode 100644 index 000000000..30f7d94ff --- /dev/null +++ b/tests/unit/features/buddy/BuddyMarkdown.test.tsx @@ -0,0 +1,35 @@ +import { render, screen } from "@testing-library/react"; +import { MemoryRouter } from "react-router-dom"; +import { describe, expect, it } from "vitest"; +import { BuddyMarkdown } from "../../../../src/features/buddy/components/BuddyMarkdown"; + +function renderLink(href: string) { + render( + + + , + ); + return screen.getByRole("link", { name: "the link" }); +} + +describe("BuddyMarkdown links", () => { + it("navigates in place for an app path", () => { + const link = renderLink("/onboarding?step=s1"); + + expect(link).toHaveAttribute("href", "/onboarding?step=s1"); + expect(link).not.toHaveAttribute("target"); + }); + + it.each(["//evil.example", "https://example.com"])( + "treats %s as somebody else's site", + (href) => { + expect(renderLink(href)).toHaveAttribute("target", "_blank"); + }, + ); + + it("never hands a backslash path on as one that could leave the app", () => { + // Browsers read `/\host` as `//host`. Markdown drops the backslash before it reaches the link, + // and `isInAppPath` would refuse it if it ever did not. + expect(renderLink("/\\evil.example").getAttribute("href")).not.toMatch(/^\/[\\/]/); + }); +}); diff --git a/tests/unit/features/buddy/toolLabel.test.ts b/tests/unit/features/buddy/toolLabel.test.ts index bc1a4cadf..41f545b6c 100644 --- a/tests/unit/features/buddy/toolLabel.test.ts +++ b/tests/unit/features/buddy/toolLabel.test.ts @@ -6,6 +6,7 @@ describe("toolLabel", () => { expect(toolLabel("get_my_metrics")).toBe("Checking your progress…"); expect(toolLabel("get_my_competencies")).toBe("Looking at where you stand…"); expect(toolLabel("get_suggested_tasks")).toBe("Finding good tasks for you…"); + expect(toolLabel("get_app_guide")).toBe("Checking where that is in the app…"); }); it("falls back to a generic label for an unknown tool", () => { diff --git a/tests/unit/services/buddyService.test.ts b/tests/unit/services/buddyService.test.ts index fb80a0d18..37bdaa2d2 100644 --- a/tests/unit/services/buddyService.test.ts +++ b/tests/unit/services/buddyService.test.ts @@ -633,6 +633,32 @@ describe("buddyService", () => { expect(capturedBody).toEqual({ content: "who is behind?", teamProjectId: "p-123" }); }); + it("sends the page the hire is on, and omits it when unknown", async () => { + const bodies: Record[] = []; + const encoder = new TextEncoder(); + server.use( + http.post("/api/v1/onboarding/me/buddy/messages", async ({ request }) => { + bodies.push((await request.json()) as Record); + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode('data: {"type":"done"}\n\n')); + controller.close(); + }, + }); + return new HttpResponse(stream, { headers: { "Content-Type": "text/event-stream" } }); + }), + ); + const handlers = { onToken: vi.fn(), onCitation: vi.fn(), onDone: vi.fn() }; + + await streamMessage("where are roles?", handlers, undefined, "/team-management"); + await streamMessage("hi", handlers); + + expect(bodies).toEqual([ + { content: "where are roles?", currentPage: "/team-management" }, + { content: "hi" }, + ]); + }); + it("opens the team greeting under the teamProjectId query param", async () => { let capturedUrl = ""; const encoder = new TextEncoder(); From 88514be0dda49ca1a2e4acd5fe6b9e3c6cd37da7 Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sat, 3 Oct 2026 15:50:34 +0200 Subject: [PATCH 2/3] Fail CI when a page is added without a decision about the buddy's guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The buddy's map of the app (get_app_guide) lives in the backend's AppGuide.kt. appGuideRoutes.ts mirrors its route list, and a test fails when accessPolicy.ts has a route that is neither described there nor excluded on purpose — so a new page trips CI in the repo it was added in. accessPolicy.ts exports APP_ROUTES for it; AGENTS.md says to keep the guide in step. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 1 + src/auth/accessPolicy.ts | 8 +++- src/features/buddy/appGuideRoutes.ts | 42 +++++++++++++++++++ .../features/buddy/appGuideRoutes.test.ts | 34 +++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 src/features/buddy/appGuideRoutes.ts create mode 100644 tests/unit/features/buddy/appGuideRoutes.test.ts diff --git a/AGENTS.md b/AGENTS.md index 7eac8e5d9..88069f4bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -180,6 +180,7 @@ Everything else — `ui/Button`, `ui/Input`, `SpotlightCard`, `AuroraBackground` - **Keycloak** (`keycloak-js`) for IAM; dev requires a Keycloak user with a role (`USER` / `PM` / `HR` / `ADMIN`) and redirect-based login. - Route access is centralized in `auth/accessPolicy.ts` (`AppRoute` union + `canAccessRoute`) and enforced by `router/AuthGuard.tsx`. **New protected routes must be added to `AppRoute` + the permission map**, or they won't type-check / won't be access-controlled. +- **The buddy has a hand-written map of these pages** (`AppGuide.kt` in the backend, served by the `get_app_guide` tool). A new, renamed or removed page, tab or tab URL parameter, or a changed sidebar/tab/button label it quotes, means updating that file in the same change. `appGuideRoutes.test.ts` fails when `accessPolicy.ts` gains a route the guide neither describes nor excludes on purpose (`src/features/buddy/appGuideRoutes.ts`). - See [docs/FRONTEND_ARCHITECTURE.md §4](./docs/FRONTEND_ARCHITECTURE.md#4-routing--access-control) for the full routing model (declarative `` API, not data-router loaders). --- diff --git a/src/auth/accessPolicy.ts b/src/auth/accessPolicy.ts index b78663a58..1a459744d 100644 --- a/src/auth/accessPolicy.ts +++ b/src/auth/accessPolicy.ts @@ -158,8 +158,14 @@ export function getDefaultRoute(profile: UserProfile | null): AppRoute { return "/"; } +/** + * Every route the policy knows, at runtime. `AppRoute` is a type and vanishes on build, so anything + * that has to enumerate the routes — the buddy's app-guide coverage test, say — reads this. + */ +export const APP_ROUTES = Object.keys(routePermissions) as AppRoute[]; + export function getMatchingProtectedRoute(pathname: string): AppRoute | null { - const routes = Object.keys(routePermissions) as AppRoute[]; + const routes = APP_ROUTES; const exactMatch = routes.find((route) => route === pathname); diff --git a/src/features/buddy/appGuideRoutes.ts b/src/features/buddy/appGuideRoutes.ts new file mode 100644 index 000000000..a9c42241a --- /dev/null +++ b/src/features/buddy/appGuideRoutes.ts @@ -0,0 +1,42 @@ +import type { AppRoute } from "../../auth/accessPolicy"; + +/** + * The routes the buddy's app guide describes — a mirror of the page list in the backend's + * `onboarding/service/AppGuide.kt`, which is what the `get_app_guide` tool reads from. + * + * The guide lives in the backend because it is filtered by the caller's role there, but it is a + * description of *this* app, and it goes stale the moment a page changes here. This list is how a + * change here notices: `appGuideRoutes.test.ts` fails when `accessPolicy.ts` has a route that is + * neither in this list nor in {@link APP_GUIDE_EXCLUDED_ROUTES}. Making it pass means deciding — + * describe the page in `AppGuide.kt` (and list it here), or exclude it here with a reason. + * + * Not checked automatically across the repos: adding a route here does not prove the backend + * describes it. The two lists are kept in the same order as `AppGuide.kt` to make that review a + * glance. Member profiles (`/team/:userId`) are described there too, under Team. + */ +export const APP_GUIDE_ROUTES: readonly AppRoute[] = [ + "/", + "/board", + "/chat", + "/buddy", + "/knowledge-base", + "/onboarding", + "/settings", + "/pm-dashboard", + "/team-management", + "/insights/onboarding", + "/insights/faq", + "/insights/knowledge-gaps", + "/insights/knowledge-requests", + "/data-ingestion", + "/blueprints", + "/hire-setup", + "/admin", +]; + +/** Routes the guide leaves out on purpose, and why — each one a decision, not an oversight. */ +export const APP_GUIDE_EXCLUDED_ROUTES: Partial> = { + "/arrival-steps": "Redirects to /hire-setup?tab=arrival.", + "/starter-work": "Redirects to /hire-setup?tab=starter.", + "/profile": "Redirects to /settings.", +}; diff --git a/tests/unit/features/buddy/appGuideRoutes.test.ts b/tests/unit/features/buddy/appGuideRoutes.test.ts new file mode 100644 index 000000000..3399482d1 --- /dev/null +++ b/tests/unit/features/buddy/appGuideRoutes.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { APP_ROUTES } from "../../../../src/auth/accessPolicy"; +import { + APP_GUIDE_EXCLUDED_ROUTES, + APP_GUIDE_ROUTES, +} from "../../../../src/features/buddy/appGuideRoutes"; + +/** + * The buddy's map of the app lives in the backend (`AppGuide.kt`). This is the tripwire on this + * side: a page added to the access policy without a decision about the guide fails here, in the + * repo the page was added in. + */ +describe("the buddy's app guide coverage", () => { + it("covers or deliberately excludes every route in the access policy", () => { + const decided = new Set([ + ...APP_GUIDE_ROUTES, + ...Object.keys(APP_GUIDE_EXCLUDED_ROUTES), + ]); + + // When this fails: describe the new page in the backend's onboarding/service/AppGuide.kt and + // add it to APP_GUIDE_ROUTES, or add it to APP_GUIDE_EXCLUDED_ROUTES with the reason. + expect(APP_ROUTES.filter((route) => !decided.has(route))).toEqual([]); + }); + + it("never lists a route both as described and as excluded", () => { + const excluded = Object.keys(APP_GUIDE_EXCLUDED_ROUTES); + + expect(APP_GUIDE_ROUTES.filter((route) => excluded.includes(route))).toEqual([]); + }); + + it("names only routes the access policy knows", () => { + expect(APP_GUIDE_ROUTES.filter((route) => !APP_ROUTES.includes(route))).toEqual([]); + }); +}); From 6649180d5437a7375f8fc88257a5b326681faa4f Mon Sep 17 00:00:00 2001 From: DavidLeuter Date: Sat, 3 Oct 2026 17:17:00 +0200 Subject: [PATCH 3/3] Follow a ?tab= link on Access Management while the page is already open The buddy links to /admin?tab=projects in place. The tab parameter was only read on mount, so somebody already on the page kept their tab while the parameter sat in the URL. Co-Authored-By: Claude Opus 5.5 --- src/pages/AdminPage.tsx | 22 +++++++++++++--------- tests/unit/pages/AdminPage.test.tsx | 26 +++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/src/pages/AdminPage.tsx b/src/pages/AdminPage.tsx index 694a9ac47..577ee94c6 100644 --- a/src/pages/AdminPage.tsx +++ b/src/pages/AdminPage.tsx @@ -60,19 +60,23 @@ export function AdminPage() { A hand-off, not a permanent part of the URL: it seeds the tab once and is then stripped, so the tab bar keeps behaving exactly as before and the back button does not turn into a step-through of sections. + + Consumed whenever it arrives, not only on mount: the buddy links to `/admin?tab=projects` + in place, and somebody already on this page would otherwise keep their tab while the + parameter sat in the URL. */ const [searchParams, setSearchParams] = useSearchParams(); - const [activeTab, setActiveTab] = useState(() => { - const requested = searchParams.get("tab"); - return ADMIN_TAB_ORDER.find((tab) => tab === requested) ?? "users"; - }); - - const tabParamConsumed = useRef(false); + const requestedTab = ADMIN_TAB_ORDER.find((tab) => tab === searchParams.get("tab")); + const [activeTab, setActiveTab] = useState(requestedTab ?? "users"); + // Adjusted during render rather than in the effect below: the tab follows a new request in + // the same paint, and forgetting the request once stripped lets the same link work twice. + const [handledTab, setHandledTab] = useState(requestedTab); + if (requestedTab !== handledTab) { + setHandledTab(requestedTab); + if (requestedTab) setActiveTab(requestedTab); + } useEffect(() => { - if (tabParamConsumed.current) return; - tabParamConsumed.current = true; - if (!searchParams.has("tab")) return; const next = new URLSearchParams(searchParams); diff --git a/tests/unit/pages/AdminPage.test.tsx b/tests/unit/pages/AdminPage.test.tsx index cbe2c5cdd..432bce476 100644 --- a/tests/unit/pages/AdminPage.test.tsx +++ b/tests/unit/pages/AdminPage.test.tsx @@ -1,7 +1,7 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { MemoryRouter } from "react-router-dom"; +import { MemoryRouter, useNavigate } from "react-router-dom"; import { AdminPage } from "../../../src/pages/AdminPage"; import type { AdminUser, ProjectSummary } from "../../../src/services/adminUserService"; import type { @@ -209,6 +209,30 @@ describe("AdminPage", () => { expect(screen.queryByTestId("users-tab")).not.toBeInTheDocument(); }); + // The buddy links to `/admin?tab=projects` in place, so the reader may already be here. + it("follows a tab named in the URL while the page is already open", async () => { + function BuddyLink() { + const navigate = useNavigate(); + return ; + } + const user = userEvent.setup(); + render( + + + + , + ); + await waitFor(() => { + expect(screen.getByTestId("users-tab")).toBeInTheDocument(); + }); + + await user.click(screen.getByRole("button", { name: "buddy link" })); + + await waitFor(() => { + expect(screen.getByTestId("projects-tab")).toBeInTheDocument(); + }); + }); + it("ignores a tab in the URL that does not exist", async () => { render(