From 0f6e97e94e24f456b4cd537c4f04883f828568c1 Mon Sep 17 00:00:00 2001 From: theprogrammersingh Date: Fri, 4 Sep 2026 12:05:08 +0530 Subject: [PATCH] feat(webmcp): let agents drive the app instead of posting behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every tool posted straight to /api/*, so nothing on screen moved: an agent approving an expense left the user looking at a row that still read "Submitted". ToolRegistry.observe() refreshes only Session.pendingApprovals, whose sole consumer is ToolSession gating a tool, and every page's resource() params is signal-free, so nothing could re-trigger a load. A human cannot add an expense without going to /add, or change a budget without going to /budgets. Now neither can an agent. - page-actions.ts is the rendezvous: a page publishes an action while mounted, a tool waits for it. No API fallback on timeout, deliberately — a fallback would restore the invisible path this removes, and only when something went wrong. Unregister clears only the handler it installed, since Angular builds the incoming component before destroying the outgoing one. - page-driven-tools.ts holds every write and injects no ApiClient. Each one navigates to the owning page, waits, and hands over the arguments; the page does the work through the same method its own buttons call, so the row patching, form messages and reloads all apply unchanged. - set_budget is new. POST/PATCH /budgets and the form already existed; only the tool was missing, so an agent could read a budget and never change one. - navigate_to moves the browser between the seven authenticated pages, with enum descriptions that double as the map of the app an agent reads off getTools(). Pinned against the real router config in both directions. - The Cambiaro converter is now a card on the dashboard, open on arrival: a cross-origin tool lives only as long as the document that registered it, so a collapsed frame meant the Copilot could not convert until someone clicked. The /add handler does not use form.requestSubmit(): onSubmit stamps the audit row actor: agentInvoked ? 'agent' : 'human', and a synthetic submit carries no agentInvoked, so it would file the agent's work as a person's. That flag is the only thing that can produce actor: 'human'. The fill is paced and the Copilot collapses to the orb below sm, where the panel is fixed inset-0 and would hide the page it is driving. --- CLAUDE.md | 86 +++++++++- Progress.md | 21 ++- frontend/src/app/ai/gemini-schema.spec.ts | 2 + frontend/src/app/app.ts | 11 +- frontend/src/app/copilot/copilot.ts | 38 +++++ frontend/src/app/core/agent/fill-pacing.ts | 50 ++++++ .../app/pages/add-expense/add-expense.spec.ts | 139 +++++++++++++++ .../src/app/pages/add-expense/add-expense.ts | 114 ++++++++++++- .../src/app/pages/budgets/budgets.spec.ts | 137 ++++++++++++++- frontend/src/app/pages/budgets/budgets.ts | 91 ++++++++++ .../src/app/pages/dashboard/dashboard.spec.ts | 73 +++++--- frontend/src/app/pages/dashboard/dashboard.ts | 94 ++++++---- .../src/app/pages/expenses/expenses.spec.ts | 93 ++++++++++ frontend/src/app/pages/expenses/expenses.ts | 83 ++++++++- frontend/src/app/tools/expense-tools.spec.ts | 74 ++------ frontend/src/app/tools/expense-tools.ts | 58 ------- .../navigate-destinations-contract.spec.ts | 71 ++++++++ .../src/app/tools/navigation-tools.spec.ts | 126 ++++++++++++++ frontend/src/app/tools/navigation-tools.ts | 84 +++++++++ .../src/app/tools/page-driven-tools.spec.ts | 150 ++++++++++++++++ frontend/src/app/tools/page-driven-tools.ts | 95 +++++++++++ frontend/src/app/webmcp/page-actions.spec.ts | 101 +++++++++++ frontend/src/app/webmcp/page-actions.ts | 132 +++++++++++++++ frontend/src/app/webmcp/tool-session.spec.ts | 2 + frontend/src/app/webmcp/tool-session.ts | 12 +- shared/src/tools.ts | 160 +++++++++++++++++- 26 files changed, 1906 insertions(+), 191 deletions(-) create mode 100644 frontend/src/app/core/agent/fill-pacing.ts create mode 100644 frontend/src/app/tools/navigate-destinations-contract.spec.ts create mode 100644 frontend/src/app/tools/navigation-tools.spec.ts create mode 100644 frontend/src/app/tools/navigation-tools.ts create mode 100644 frontend/src/app/tools/page-driven-tools.spec.ts create mode 100644 frontend/src/app/tools/page-driven-tools.ts create mode 100644 frontend/src/app/webmcp/page-actions.spec.ts create mode 100644 frontend/src/app/webmcp/page-actions.ts diff --git a/CLAUDE.md b/CLAUDE.md index db6e76e..149b049 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -466,6 +466,66 @@ It is set in the runtime stage of the Dockerfile, and deliberately NOT at build time — a production-flagged install drops devDependencies, and the build is almost entirely devDependencies. +## Writes go through the page, never behind it + +**What a human can do, the agent can do; and it does it where a human would.** +A person cannot add an expense without going to `/add`, or change a budget +without going to `/budgets`. Neither can an agent. + +Every mutating tool — `submit_expense`, `approve_expense`, `set_budget` — lives +in `frontend/src/app/tools/page-driven-tools.ts`, which **injects no +`ApiClient`**. Its `execute()` does three things: navigate to the page that owns +the action, wait for that page to mount, and hand it the arguments. The page +performs the work through the same method its own buttons call. + +That last part is what makes it worth the machinery. The tool gets the row +patching, the form messages, the validation and the reloads that already exist, +because it is running the same code. `ExpensesPage.run()` merges the decision +into `patched` exactly as the Approve button does, so the visible row changes +and no `Load more` page is thrown away; `BudgetsPage.commit()` is what the Save +button calls, so the bars reload and a 403 is worded the way the form already +words it. + +These tools used to POST directly, and the effect was that **nothing on screen +moved**. `ToolRegistry.observe()` refreshes only `Session.pendingApprovals`, and +the sole consumer of that signal is `ToolSession` gating a tool — no page reads +it. Every page's `resource()` `params` is signal-free, so no signal change can +re-trigger a load either. An agent approving an expense left the user looking at +a row that still said "Submitted". + +Four things here are load-bearing: + +- **`PageActions` has no API fallback when no page answers.** A fallback would + restore the invisible path this exists to remove, and would do it only when + something went wrong — a slow chunk, a guard redirect. A timeout is an error + the model reports. +- **Unregistering only clears the handler it installed.** During a route change + Angular builds the incoming component before destroying the outgoing one, so + an unconditional delete lets a page being torn down wipe the registration the + new page just made. Same ordering `ConverterSession` documents. +- **The `/add` handler does not use `form.requestSubmit()`.** The form's own + `onSubmit` stamps the audit row `actor: agentInvoked ? 'agent' : 'human'`, and + a synthetic submit carries no `agentInvoked` — so it would file the agent's + work as a person's. That flag is the only thing in the app that can produce + `actor: 'human'`, and the audit viewer's contrast is built on it. + `ToolRegistry.log()` already records the call. +- **The fill is paced, and the pause is not decoration.** `core/agent/fill-pacing.ts` + staggers fields and waits before saving. Filling and submitting in one frame + leaves no frame in which the filled form is on screen — indistinguishable from + the invisible POST it replaced. Specs drive the stagger at 0. + +The Copilot collapses to the orb below the `sm` breakpoint before a page-driven +tool runs (`Copilot.collapseForPageAction()`), because the panel is +`fixed inset-0` there — a full-screen sheet the driven page would be hidden +behind. It collapses for the *turn*, not per call, so a turn that files an +expense and then approves it does not flicker. From `sm:` up the panel is +already non-blocking and nothing happens. + +`submit_expense` still means create **and** submit for approval, as it always +has. The declarative `add_expense_form` creates a draft only, so the `/add` +handler performs the transition after the save rather than quietly narrowing the +contract. + ## The expense workflow is one table, shared Which action is legal, who may perform it, and on whose row — all of it lives in @@ -523,7 +583,11 @@ request; the access token deliberately carries no role claim. `App` subscribes once and fans out to the audit write and the pending-approval re-poll. Keep HTTP and session dependencies out of the registry itself — that is what keeps its spec free of fakes. -- `tools/` — the five tool `execute()` implementations over `/api/*`. +- `tools/` — the tool `execute()` implementations, split by what they touch so + each spec needs only its own fakes. `expense-tools.ts` is the six reads over + `/api/*`; `navigation-tools.ts` is `navigate_to`, which touches the `Router` + and nothing else; `page-driven-tools.ts` is every write, and touches **no + `ApiClient` at all** — see "Writes go through the page" below. - `copilot/` — `Copilot` (the agent loop) and `CopilotPanel` (orb + panel). - `core/api/` — `ApiClient`. `core/theme/` — `ThemeService`. - `pages/add-expense/` — the declarative WebMCP form. It is the only tool call a @@ -634,8 +698,12 @@ rows were, at hand-written rates nobody published. It lives under `backend/` because pnpm hoists nothing: a root script cannot resolve `@nestjs/core`. **The embedded converter does not change this, and must not.** `converter/` -frames a separate converter app on four surfaces (`/convert`, `/agent`, the -dashboard's excluded-rows notice, and expense rows in another currency). It is +frames a separate converter app on four surfaces (`/convert`, `/agent`, a card +on the dashboard, and expense rows in another currency). The dashboard's and +`/convert`'s open on arrival; the other two are triggers. Opening on the +dashboard is deliberate — a cross-origin tool lives only as long as the document +that registered it, so a collapsed frame on the screen people land on means the +Copilot cannot convert until someone clicks. It is advisory: a rate a person reads off another site *today* is not the historical rate locked at entry, so nothing it shows may reach `converted_amount`, `sumSpend()`, `sumByCategory()`, or the `excludedNotice()` copy. The FX pass @@ -669,7 +737,17 @@ These are the load-bearing constraints — most bugs worth preventing here are v PRD §7 is a checklist every row of which needs a concrete implementation. When adding a tool, know which aspect it demonstrates: - **Declarative** — the Add Expense quick-entry form is annotated HTML with *no* JS tool registration. Keep it plain. -- **Imperative** (`registerTool`) — `submit_expense`, `search_expenses`, `get_budget_status`, `approve_expense`, `generate_report` +- **Imperative** (`registerTool`) — `submit_expense`, `search_expenses`, `get_budget_status`, `approve_expense`, `set_budget`, `generate_report` +- **Navigation** — `navigate_to` moves the browser between the app's authenticated + pages. It exists for agents driving Actuo from outside, which otherwise have to + read the DOM and guess where to click, and its enum descriptions double as the + map of the app an agent reads straight off `getTools()`. The destination table + is `APP_DESTINATIONS` in `shared/src/tools.ts`, pinned against the real router + config in both directions by `tools/navigate-destinations-contract.spec.ts` — + every `authGuard` route is a destination and every destination is one, so a new + gated page cannot ship undescribed. It is **not** `readOnlyHint` (it moves the + page), which is why `app.ts` exempts it by name from the pending-approval + re-poll that every other mutating tool triggers. - **State-gated** — `approve_expense` registers only when the user is `admin`/`owner` AND a pending item exists; emits `toolchange` - **Cancellation** — `generate_report` honors `AbortSignal`; the UI must react within ~100ms - **Cross-origin** — the Copilot must work embedded on an unrelated demo page (iframe + `exposedTo`/`fromOrigins`/`allow="tools"`) with no code changes diff --git a/Progress.md b/Progress.md index 27f7df3..abd8d61 100644 --- a/Progress.md +++ b/Progress.md @@ -151,7 +151,7 @@ their own expense. | Original + converted amounts stored | 1 | ✅ | Columns exist. `core/expense/amount.ts` owns the rule: `sumSpend()` adds only base-currency rows and reports the rest | | Live FX + daily cache | 1 | ✅ | `backend/src/fx/` reads ECB rates from `api.frankfurter.dev` and caches them in `fx_rates` (migration `0003`). Keyed on the date *asked for*, not the date the rate is from, or every weekend lookup would miss forever; the only entry that can go stale is today's while it still stands in for an earlier day. `FxService.rateOn` returns `null` rather than throwing, so an unreachable publisher cannot stop an expense being filed | | Historical rate lock | 1 | ✅ | `expenses.fx_rate` + `fx_rate_date`, written with `converted_amount` at the expense's own date. **`fx_rate_date` is not `expense_date`** — the ECB publishes once per working day, so a Saturday expense locks Friday's rate, and the row prints the rate's own date. An edit re-locks on amount, currency *or* date. Verified live: AWS filed 2026-08-29 (a Saturday) carries the 2026-08-28 rate | -| Embedded converter (advisory) | 1 | ✅ | `converter/currency-converter.ts` frames a separate converter app on `/convert`, `/agent`, the dashboard notice and foreign-currency expense rows. One frame at a time, lazily mounted, `CONVERTER_URL` from `GET /api/config` | +| Embedded converter (advisory) | 1 | ✅ | `converter/currency-converter.ts` frames a separate converter app on `/convert`, `/agent`, a dashboard card and foreign-currency expense rows. `/convert` and the dashboard open on arrival — the dashboard because a cross-origin tool lives only as long as the document that registered it, so a collapsed frame on the landing screen means the Copilot cannot convert until someone clicks. One frame at a time, `CONVERTER_URL` from `GET /api/config` | > **The exclusion rules were what made FX cheap to land.** `sumSpend()` returns > `{total, excluded}` and `sumByCategory()` returns `unconverted` — a row counts @@ -235,7 +235,10 @@ Cross-origin is live as of 2026-08-29; only the standalone-script packaging (Pha | Aspect | Status | Notes | |---|---|---| | Declarative API (annotated form) | ✅ | Add Expense: `toolname`/`tooldescription`/`toolparamdescription`/`toolautosubmit`, `agentInvoked` + `respondWith`, **no JS registration** | -| Imperative `registerTool` | ✅ | Six tools, per-tool `AbortController` lifetime | +| Imperative `registerTool` | ✅ | Eight tools, per-tool `AbortController` lifetime | +| **Writes drive the visible page** | ✅ | `submit_expense`, `approve_expense` and `set_budget` navigate to the page that owns the action and hand the work to it, so the user watches it happen — the form fills field by field, the row's badge changes in place, the budget bar moves. `page-driven-tools.ts` injects no `ApiClient`, so there is no path back to posting behind the page. Rendezvous in `webmcp/page-actions.ts` | +| **Budget editing** | ✅ | `set_budget` creates or updates through the Budgets form. The route (`POST`/`PATCH /budgets`) and the form both already existed; only the tool was missing, so an agent could read a budget and never change one | +| **Agent navigation** | ✅ | `navigate_to` moves the browser between the seven authenticated pages, so an agent driving Actuo from outside does not have to read the DOM and guess where to click. Its enum descriptions are the map of the app, read straight off `getTools()`. `APP_DESTINATIONS` is pinned against the real router config in both directions by `tools/navigate-destinations-contract.spec.ts` | | JSON Schema inputs | ✅ | One definition in `shared/src/tools.ts`, used by client and server | | **Dynamic / state-gated tools** | ✅ | The shell polls on sign-in and after every mutating call. Verified live: `approve_expense` present in `getTools()` as owner with 3 pending, absent as member, and every tool retired on sign-out | | Cancellation (`AbortSignal`) | ✅ | Client aborts, polls stop, server abandons the job mid-fetch and mid-format | @@ -247,6 +250,20 @@ Cross-origin is live as of 2026-08-29; only the standalone-script packaging (Pha > Open question: `generate_report` is annotated `readOnlyHint: true` but creates a > server-side job. Defensible, but decide it deliberately. +Before this, every tool posted straight to `/api/*` and nothing on screen +moved: `ToolRegistry.observe()` refreshes only `Session.pendingApprovals`, whose +only consumer is `ToolSession` gating a tool, and every page's `resource()` +`params` is signal-free so nothing could re-trigger a load. An agent approving +an expense left the user looking at a row that still read "Submitted". + +`navigate_to` is deliberately **not** `readOnlyHint`: it reads and writes no +data, but it changes what the user is looking at, and that is what the flag +tells a client. It is the same category the embedded converter's own UI-moving +tools sit in, which `/agent` already renders as `Mutating`. The one consequence +is that `app.ts` exempts it by name from the pending-approval re-poll every +other mutating tool triggers — navigation cannot change the queue, and an agent +walking the app would otherwise fire a search per hop. + `download_report` is the companion tool that saves a finished report to disk. It exists because a file has no other route out for a client that can only call tools: the download route needs the session bearer header, so no URL — in chat or diff --git a/frontend/src/app/ai/gemini-schema.spec.ts b/frontend/src/app/ai/gemini-schema.spec.ts index b0a9b8a..7cb0e25 100644 --- a/frontend/src/app/ai/gemini-schema.spec.ts +++ b/frontend/src/app/ai/gemini-schema.spec.ts @@ -286,6 +286,8 @@ describe('toGeminiSchema', () => { 'generate_report', 'download_report', 'fetch_categories', + 'navigate_to', + 'set_budget', 'approve_expense', ]); for (const declaration of declarations) { diff --git a/frontend/src/app/app.ts b/frontend/src/app/app.ts index ef4d52c..cac2bef 100644 --- a/frontend/src/app/app.ts +++ b/frontend/src/app/app.ts @@ -6,6 +6,7 @@ import { inject, } from '@angular/core'; import { RouterLink, RouterLinkActive, RouterOutlet } from '@angular/router'; +import { NAVIGATE_TO } from '@actuo/shared'; import { CopilotPanel } from './copilot/copilot-panel.js'; import { Session } from './core/session/session.js'; import { ThemeService } from './core/theme/theme-service.js'; @@ -243,7 +244,15 @@ export class App { // An approval decision changes the queue, which closes the gate. Reads // cannot, and `search_expenses` runs often enough that polling on it // would be a request per question. - if (this.registry.isMutating(invocation.toolName)) { + // + // `navigate_to` is mutating by the `readOnlyHint` test — it moves the + // page — but it cannot touch the queue, and an agent walking the app + // would otherwise fire a search per hop. The network tab is part of what + // this app demonstrates, so the exemption is worth the named check. + if ( + this.registry.isMutating(invocation.toolName) && + invocation.toolName !== NAVIGATE_TO.name + ) { void this.session.refreshPendingApprovals(); } }); diff --git a/frontend/src/app/copilot/copilot.ts b/frontend/src/app/copilot/copilot.ts index 5cbed15..e2182d7 100644 --- a/frontend/src/app/copilot/copilot.ts +++ b/frontend/src/app/copilot/copilot.ts @@ -44,6 +44,9 @@ const SYSTEM_INSTRUCTION = [ 'Amounts are money: state the currency, and never invent figures you did not read from a tool.', "Tools from another origin are advisory: quote a result as that site's answer, say what rate", 'and date it used, and never fold one into an Actuo total or present it as an Actuo figure.', + 'navigate_to only moves the screen: it returns no expense data, so never call it to answer a', + 'question. Use it when the user asks to see a page, or when what they asked for is something', + 'they need to be looking at.', 'Never write download links or file URLs: a link you invent points at a route the browser', 'cannot authenticate. When the user asks to download, save or export a report, call', 'download_report with the jobId generate_report returned.', @@ -71,6 +74,8 @@ export class Copilot { private controller: AbortController | null = null; /** Resolves when the user answers a confirmation card. */ private pendingConfirmation: ((approved: boolean) => void) | null = null; + /** True while the panel is hidden so a page-driven tool stays watchable. */ + private collapsed = false; readonly entries = this.entryList.asReadonly(); readonly isBusy = this.busy.asReadonly(); @@ -148,9 +153,41 @@ export class Copilot { } finally { this.busy.set(false); this.controller = null; + this.restoreAfterPageAction(); } } + /** + * Get out of the way so the user can watch a tool operate the page. + * + * On a phone the panel is `fixed inset-0` — a full-screen opaque sheet — so a + * tool driving the UI underneath it would be completely invisible, which + * would defeat the point of driving the UI at all. Below the `sm` breakpoint + * it therefore drops to the orb for the rest of the turn and comes back when + * the turn ends. From `sm:` up the panel is already non-blocking, so nothing + * happens. + * + * Collapsing for the *turn* rather than per call is deliberate: a turn that + * files an expense and then approves it would otherwise flicker the panel + * shut and open between the two. + */ + collapseForPageAction(): void { + if (!this.open() || this.collapsed) return; + if (typeof window === 'undefined' || window.matchMedia === undefined) return; + // 40rem is Tailwind's `sm`, where the panel stops covering the page. + if (window.matchMedia('(min-width: 40rem)').matches) return; + + this.collapsed = true; + this.open.set(false); + } + + /** Put the panel back after a turn that collapsed it. */ + private restoreAfterPageAction(): void { + if (!this.collapsed) return; + this.collapsed = false; + this.open.set(true); + } + /** Stop everything in flight. §3.2.6 wants this to feel immediate. */ stop(): void { this.controller?.abort(); @@ -163,6 +200,7 @@ export class Copilot { ), ); this.busy.set(false); + this.restoreAfterPageAction(); } /** Called by the Confirm / Cancel buttons on a tool card. */ diff --git a/frontend/src/app/core/agent/fill-pacing.ts b/frontend/src/app/core/agent/fill-pacing.ts new file mode 100644 index 0000000..2453eb8 --- /dev/null +++ b/frontend/src/app/core/agent/fill-pacing.ts @@ -0,0 +1,50 @@ +/** + * Pacing for a form an agent is filling in front of the user. + * + * The reason a tool drives the visible form instead of posting behind it is so + * a person can watch it happen. Setting every field and submitting in the same + * frame produces no frame in which the filled form is on screen — the result + * looks exactly like the invisible POST it replaced. A short pause between + * fields, and one before the save, is what makes it legible. + * + * Specs drive this at 0. + */ +export const AGENT_FILL_STAGGER_MS = 120; + +/** A sleep that gives up promptly when the agent is stopped. */ +export function agentPause(ms: number, signal?: AbortSignal): Promise { + if (ms <= 0) return Promise.resolve(); + return new Promise((resolve, reject) => { + const timer = setTimeout(resolve, ms); + signal?.addEventListener( + 'abort', + () => { + clearTimeout(timer); + reject(signal.reason ?? new Error('Aborted.')); + }, + { once: true }, + ); + }); +} + +/** + * Set one control the way a person would, so the screen actually repaints. + * + * Both `input` and `change` are dispatched: a `` from showing a + * blank while the agent's choice is already set. + */ + private async setFromAgent(args: SetBudgetArgs): Promise { + if (this.existing().length === 0 && this.categories().length === 0) { + await this.loadCategories(); + } + + const categoryId = args.categoryId ?? null; + const existing = this.existing().find((b) => (b.categoryId ?? null) === categoryId); + + // Editing and creating are different requests; `startEdit` is what tells + // the form (and `commit`) which one this is. + if (existing) this.startEdit(existing); + else { + this.editingBudget.set(null); + this.newCategoryId.set(categoryId ?? ''); + } + + this.newAmount.set(String(args.amount)); + this.rollover.set(args.rollover ?? existing?.rollover ?? false); + + /* + * Let the filled form reach the screen before saving it. Without this the + * fill and the save land in the same frame, so there is no frame in which + * the user sees what the agent chose — which is the whole reason this goes + * through the form instead of posting behind it. + */ + await agentPause(this.fillStaggerMs); + + await this.commit(); + + /* + * `commit()` reports two different refusals: the amount check writes + * `amountError` and returns without ever calling the API, while a rejected + * request sets `formFailed`. Both have to reach the agent as errors, or a + * budget that was never saved comes back looking saved. + */ + const refusal = this.amountError() ?? (this.formFailed() ? this.formMessage() : null); + if (refusal) throw new Error(refusal); + + const saved = this.existing().find((b) => (b.categoryId ?? null) === categoryId); + return { + categoryId, + amount: saved?.amount ?? args.amount, + rollover: saved?.rollover ?? args.rollover ?? false, + created: !existing, + }; } private async loadCategories(): Promise { @@ -385,6 +460,16 @@ export class Budgets { protected async saveBudget(event: Event): Promise { event.preventDefault(); + await this.commit(); + } + + /** + * Save whatever the form currently holds. + * + * Split out of `saveBudget` so the agent-driven path and the Save button are + * the same code: the only thing the button adds is `preventDefault()`. + */ + private async commit(): Promise { this.formMessage.set(null); this.amountError.set(null); @@ -570,3 +655,9 @@ function describeBudgetFailure(error: unknown, isUpdate = false): string { ? 'That budget could not be updated. Nothing was changed.' : 'That budget could not be saved. Nothing was changed.'; } + +type SetBudgetArgs = { + categoryId?: string; + amount: number; + rollover?: boolean; +}; diff --git a/frontend/src/app/pages/dashboard/dashboard.spec.ts b/frontend/src/app/pages/dashboard/dashboard.spec.ts index 424f6ec..c800719 100644 --- a/frontend/src/app/pages/dashboard/dashboard.spec.ts +++ b/frontend/src/app/pages/dashboard/dashboard.spec.ts @@ -275,14 +275,19 @@ describe('Dashboard', () => { * "click here to include them". The copy has to carry that, so it is * asserted rather than left to drift. */ - describe('the rate lookup beside it', () => { - it('offers a lookup that says it will not change the total', async () => { + describe('the rate lookup below it', () => { + /** The converter reaches the page a tick after the resource does. */ + async function withConverter(expenses = MIXED): Promise { api.get.mockImplementation( - respond({ expenses: MIXED, converterUrl: 'https://cambiaro.example/' }), + respond({ expenses, converterUrl: 'https://cambiaro.example/' }), ); await create(); await settle(); await settle(); + } + + it('points at the lookup in copy that says it will not change the total', async () => { + await withConverter(); expect(text()).toContain("this won't change the total"); }); @@ -298,31 +303,55 @@ describe('Dashboard', () => { }); /** - * LOAD-BEARING. Opening the lookup must not move a figure on this page. - * See CLAUDE.md, "Money: never add two currencies". + * The frame is what publishes the cross-origin `convertCurrency` tool, + * and a tool lives only as long as the document that registered it — so + * a converter nobody has clicked open is a Copilot that cannot convert. + * This page is where people land, so it opens on arrival. */ - it('leaves the total and the notice exactly as they were', async () => { - api.get.mockImplementation( - respond({ expenses: MIXED, converterUrl: 'https://cambiaro.example/' }), - ); + it('opens the frame on arrival, with no click', async () => { + await withConverter(); + + const frame = find('iframe') as HTMLIFrameElement | null; + expect(frame).not.toBeNull(); + expect(frame!.getAttribute('allow')).toBe('tools'); + }); + + /** One mount only: two frames would publish `convertCurrency` twice. */ + it('mounts exactly one converter', async () => { + await withConverter(); + + expect(findAll('app-currency-converter').length).toBe(1); + expect(findAll('iframe').length).toBe(1); + }); + + it('renders no converter card when none is configured', async () => { + api.get.mockImplementation(respond({ expenses: MIXED })); await create(); await settle(); await settle(); - const tileOf = () => - findAll('ui-stat-card') - .find((card) => card.textContent?.includes('This month')) - ?.textContent?.trim(); - const before = tileOf(); - - const trigger = findAll('button').find((b) => - (b.textContent ?? '').includes("won't change the total"), - ) as HTMLButtonElement | undefined; - expect(trigger).toBeDefined(); - trigger!.click(); - await settle(); + expect(findAll('iframe').length).toBe(0); + expect(text()).not.toContain('Currency converter'); + }); + + /** + * LOAD-BEARING. The lookup must not move a figure on this page. + * See CLAUDE.md, "Money: never add two currencies". + * + * The converter is open on arrival now, so this is asserted directly + * rather than across a click: with the frame on screen, every figure + * reads exactly what it reads with no converter configured at all. + */ + it('leaves the total and the notice exactly as they were', async () => { + await withConverter(); + expect(find('iframe')).not.toBeNull(); - expect(tileOf()).toBe(before); + const tile = findAll('ui-stat-card').find((card) => + card.textContent?.includes('This month'), + ); + // Still 1000, not 1350 — the same figure the converter-less case gives. + expect(tile?.textContent).toContain('1,000'); + expect(tile?.textContent).not.toContain('1,350'); expect(text()).toContain('2 expenses in other currencies'); }); }); diff --git a/frontend/src/app/pages/dashboard/dashboard.ts b/frontend/src/app/pages/dashboard/dashboard.ts index 24f597b..0910978 100644 --- a/frontend/src/app/pages/dashboard/dashboard.ts +++ b/frontend/src/app/pages/dashboard/dashboard.ts @@ -2,6 +2,7 @@ import { isPlatformBrowser } from '@angular/common'; import { ChangeDetectionStrategy, Component, + type OnInit, PLATFORM_ID, computed, inject, @@ -167,33 +168,50 @@ const PACE_LABEL: Record = {

{{ note }}

@if (converter.isAvailable()) { - - -
+

+ The rate lookup below is a reference — this won't change the total. +

+ } + } + + + @if (converter.isAvailable()) { +
+ +
+

Currency converter

+

+ A separate app on its own origin, embedded here — the Copilot can drive it + through the tools it publishes. +

+
+ -
- } + +
}
@@ -268,7 +286,7 @@ const PACE_LABEL: Record = { `, }) -export class Dashboard { +export class Dashboard implements OnInit { private readonly api = inject(ApiClient); private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); @@ -344,12 +362,7 @@ export class Dashboard { () => totalForMonth(this.expenses(), this.window.previousMonth).total, ); - /** - * How many rows on this page could not be totalled, across the whole fetched - * window — which is exactly what the tiles and the trend report on. One - * notice for the screen beats repeating the caveat on every tile. - */ - /** Owns the single converter frame; the trigger below is one of four. */ + /** Owns the single converter frame; this page is one of four surfaces. */ protected readonly converter = inject(ConverterSession); /** This mount point's id. See `ConverterSession.open`. */ @@ -357,16 +370,31 @@ export class Dashboard { constructor() { /* - * Resolve the converter's config up front. The rate-lookup trigger is gated - * on `isAvailable()`, and the frame that would otherwise load that config - * only mounts once the trigger is shown — so without this the gate could - * never open. The session caches it, so this is one request per session - * however many surfaces ask. + * Resolve the converter's config up front. The card is gated on + * `isAvailable()`, and the frame that would otherwise load that config only + * mounts once the card is shown — so without this the gate could never + * open. The session caches it, so this is one request per session however + * many surfaces ask. */ void this.converter.ensureConfig(); } + ngOnInit(): void { + /* + * Open on arrival, the way `/convert` does. This is the screen people land + * on, and a cross-origin tool lives exactly as long as the document that + * registered it — collapsed, the Copilot has no `convertCurrency` until + * someone thinks to click. `ConverterSession` is a radio group, so this + * also closes the frame wherever else it was open. + */ + this.converter.open(this.converterSurface); + } + /** + * How many rows on this page could not be totalled, across the whole fetched + * window — which is exactly what the tiles and the trend report on. One + * notice for the screen beats repeating the caveat on every tile. + */ protected readonly excludedNote = computed(() => excludedNotice(sumSpend(this.expenses()).excluded), ); diff --git a/frontend/src/app/pages/expenses/expenses.spec.ts b/frontend/src/app/pages/expenses/expenses.spec.ts index 7c8bf56..91fa806 100644 --- a/frontend/src/app/pages/expenses/expenses.spec.ts +++ b/frontend/src/app/pages/expenses/expenses.spec.ts @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ApiClient, ApiError } from '../../core/api/api-client.js'; import { Session } from '../../core/session/session.js'; +import { PageActions } from '../../webmcp/page-actions.js'; import { Expenses } from './expenses.js'; function expense(overrides: Partial = {}): Expense { @@ -839,4 +840,96 @@ describe('Expenses workflow actions', () => { expect(text()).not.toContain('Barista'); }); }); + + /** + * `approve_expense` navigates here and hands the decision over, so the + * decision runs through `run()` — the same path the row's own buttons use. + */ + describe('as the page that performs approve_expense', () => { + function handler() { + return TestBed.inject(PageActions).awaitHandler( + 'approve_expense', + new AbortController().signal, + ); + } + + it('offers the action while mounted and withdraws it on destroy', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + const pages = TestBed.inject(PageActions); + expect(pages.has('approve_expense')).toBe(true); + + fixture.destroy(); + + expect(pages.has('approve_expense')).toBe(false); + }); + + it('decides through the same route the row button posts to', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + + const run = await handler(); + await run({ expenseId: 'a', decision: 'approved' } as never, { + signal: new AbortController().signal, + }); + + expect(api.post).toHaveBeenCalledWith('/expenses/a/approve', {}); + }); + + it('passes a rejection comment through', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + api.post.mockResolvedValue(expense({ id: 'a', status: 'rejected' })); + + const run = await handler(); + await run({ expenseId: 'a', decision: 'rejected', comment: 'no receipt' } as never, { + signal: new AbortController().signal, + }); + + expect(api.post).toHaveBeenCalledWith('/expenses/a/reject', { comment: 'no receipt' }); + }); + + /** + * LOAD-BEARING. `run()` patches the row in place; reloading would throw + * away every `Load more` page and the scroll position, which is exactly + * what the button path avoids and why `patched` exists at all. + */ + it('updates the visible row without refetching the list', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + const loadsBefore = api.get.mock.calls.length; + + const run = await handler(); + await run({ expenseId: 'a', decision: 'approved' } as never, { + signal: new AbortController().signal, + }); + fixture.detectChanges(); + + expect(api.get.mock.calls.length).toBe(loadsBefore); + expect(text()).toContain('Approved'); + }); + + it('reports a refusal instead of claiming the decision landed', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + api.post.mockRejectedValue(new ApiError('Forbidden', 403, null)); + + const run = await handler(); + + await expect( + run({ expenseId: 'a', decision: 'approved' } as never, { + signal: new AbortController().signal, + }), + ).rejects.toThrow(); + }); + + /** An agent can name a row from a search that reaches past what is loaded. */ + it('fetches a row that is not on the loaded page', async () => { + await create([expense({ id: 'a', status: 'submitted', userId: OTHER })]); + api.get.mockResolvedValue(expense({ id: 'z', status: 'submitted', userId: OTHER })); + + const run = await handler(); + await run({ expenseId: 'z', decision: 'approved' } as never, { + signal: new AbortController().signal, + }); + + expect(api.get).toHaveBeenCalledWith('/expenses/z', undefined, expect.anything()); + expect(api.post).toHaveBeenCalledWith('/expenses/z/approve', {}); + }); + }); }); diff --git a/frontend/src/app/pages/expenses/expenses.ts b/frontend/src/app/pages/expenses/expenses.ts index e21d7cc..8d4f4cb 100644 --- a/frontend/src/app/pages/expenses/expenses.ts +++ b/frontend/src/app/pages/expenses/expenses.ts @@ -2,13 +2,14 @@ import { isPlatformBrowser } from '@angular/common'; import { ChangeDetectionStrategy, Component, + DestroyRef, PLATFORM_ID, computed, inject, resource, signal, } from '@angular/core'; -import { EXPENSE_PAGE_MAX } from '@actuo/shared'; +import { APPROVE_EXPENSE, EXPENSE_PAGE_MAX } from '@actuo/shared'; import type { Expense, Page, TransitionAction } from '@actuo/shared'; import { ApiClient, ApiError } from '../../core/api/api-client.js'; @@ -18,6 +19,7 @@ import { expenseAmount, expenseCurrency, isConverted } from '../../core/expense/ import { CurrencyConverter } from '../../converter/currency-converter.js'; import { ConverterSession } from '../../converter/converter-session.js'; import { Session } from '../../core/session/session.js'; +import { PageActions } from '../../webmcp/page-actions.js'; import { ACTION_LABEL, actionPath, @@ -187,7 +189,11 @@ const PAGE_SIZE = EXPENSE_PAGE_MAX; @for (row of rows(); track row.id) { - + + {{ formatDate(row.expenseDate) }} @@ -311,7 +317,7 @@ const PAGE_SIZE = EXPENSE_PAGE_MAX;
    @for (row of rows(); track row.id) { -
  • +

  • {{ row.merchant || 'Untitled' }}

    @@ -522,6 +528,9 @@ export class Expenses { /** Owns the single converter frame across every surface. */ protected readonly converter = inject(ConverterSession); + private readonly pages = inject(PageActions); + private readonly destroyRef = inject(DestroyRef); + constructor() { /* * Resolve the converter's config up front. The rate-lookup trigger is gated @@ -531,6 +540,68 @@ export class Expenses { * however many surfaces ask. */ void this.converter.ensureConfig(); + + /* + * This page owns `approve_expense`. The tool navigates here and hands the + * decision over, so it runs through `run()` — the same path the row's own + * buttons use, which patches the visible row rather than reloading. + */ + this.destroyRef.onDestroy( + this.pages.provide(APPROVE_EXPENSE.name, (args: DecisionArgs, { signal }) => + this.decideFromAgent(args, signal), + ), + ); + } + + /** + * Approve or reject on behalf of an agent, through the visible row. + * + * Deliberately no `data.reload()`: `run()` patches the row in place, and a + * reload would discard every `Load more` page and the scroll position for the + * same reason the button path avoids it. + */ + private async decideFromAgent(args: DecisionArgs, signal: AbortSignal): Promise { + const action: TransitionAction = args.decision === 'approved' ? 'approve' : 'reject'; + + /* + * The row is usually already on screen, but an agent can name an expense + * from a search that reaches past the loaded page. Fetching the one row is + * cheaper and less disruptive than paging until it turns up. + */ + const target = + this.rows().find((row) => row.id === args.expenseId) ?? + (await this.api.get(`/expenses/${args.expenseId}`, undefined, signal)); + + this.scrollRowIntoView(target.id); + await this.run(target, action, args.comment ?? ''); + + const rowError = this.rowError(); + if (rowError?.id === target.id) throw new Error(rowError.message); + + const updated = this.patched()[target.id] ?? target; + return { + id: updated.id, + amount: updated.amount, + currency: updated.currency, + merchant: updated.merchant, + status: updated.status, + date: updated.expenseDate, + }; + } + + /** + * Put the row the agent is about to act on where the user can see it. + * + * Feature-detected rather than assumed: scrolling is a courtesy, and a + * decision must not fail because a row is off screen or because the host has + * no layout to scroll (the test environment, and prerendering). + */ + private scrollRowIntoView(id: string): void { + if (!this.isBrowser) return; + const row = document.querySelector(`[data-expense-row="${id}"]`); + if (row && typeof row.scrollIntoView === 'function') { + row.scrollIntoView({ block: 'center', behavior: 'smooth' }); + } } /** Row id with an action in flight, so its buttons can show a busy state. */ @@ -875,3 +946,9 @@ function describeFailure(error: unknown, action: TransitionAction | 'delete'): s } return `Could not ${verb} this expense. Nothing was changed.`; } + +type DecisionArgs = { + expenseId: string; + decision: string; + comment?: string; +}; diff --git a/frontend/src/app/tools/expense-tools.spec.ts b/frontend/src/app/tools/expense-tools.spec.ts index 01f4e59..a769528 100644 --- a/frontend/src/app/tools/expense-tools.spec.ts +++ b/frontend/src/app/tools/expense-tools.spec.ts @@ -62,45 +62,6 @@ describe('ExpenseTools', () => { }); }); - describe('submit_expense', () => { - it('is flagged as requiring confirmation, since it moves money', () => { - const tool = tools.submitExpense(); - expect(tool.contract.requiresConfirmation).toBe(true); - expect(tool.contract.annotations.readOnlyHint).toBe(false); - }); - - it('creates then submits, and defaults the date to today', async () => { - api.post - .mockResolvedValueOnce(expense({ status: 'draft' })) - .mockResolvedValueOnce(expense({ status: 'submitted' })); - - const result = await tools.submitExpense().execute( - { amount: 450, currency: 'INR', merchant: 'Barista' }, - { signal: new AbortController().signal }, - ); - - const [, createBody] = api.post.mock.calls[0]; - expect(createBody.expenseDate).toMatch(/^\d{4}-\d{2}-\d{2}$/); - expect(api.post.mock.calls[1][0]).toBe('/expenses/exp-1/submit'); - expect(result).toMatchObject({ status: 'submitted' }); - }); - - it('passes categoryId to the create call', async () => { - api.post - .mockResolvedValueOnce(expense({ status: 'draft' })) - .mockResolvedValueOnce(expense({ status: 'submitted' })); - - await tools.submitExpense().execute( - { amount: 450, currency: 'INR', categoryId: 'cat-uuid-1' }, - { signal: new AbortController().signal }, - ); - - const [, createBody] = api.post.mock.calls[0]; - expect(createBody.categoryId).toBe('cat-uuid-1'); - expect(createBody).not.toHaveProperty('category'); - }); - }); - describe('get_budget_status', () => { it('formats utilization and flags overspend', async () => { api.get.mockResolvedValue([ @@ -338,31 +299,17 @@ describe('ExpenseTools', () => { }); }); - describe('approve_expense', () => { - it('routes to approve or reject based on the decision', async () => { - api.post.mockResolvedValue(expense({ status: 'approved' })); - - await tools.approveExpense().execute( - { expenseId: 'exp-9', decision: 'approved' }, - { signal: new AbortController().signal }, - ); - expect(api.post.mock.calls[0][0]).toBe('/expenses/exp-9/approve'); - - api.post.mockResolvedValue(expense({ status: 'rejected' })); - await tools.approveExpense().execute( - { expenseId: 'exp-9', decision: 'rejected', comment: 'no receipt' }, - { signal: new AbortController().signal }, - ); - expect(api.post.mock.calls[1][0]).toBe('/expenses/exp-9/reject'); - expect(api.post.mock.calls[1][1]).toEqual({ comment: 'no receipt' }); - }); - - it('is not in the always-on set, because it is state-gated', () => { + describe('what it no longer does', () => { + /** + * `submit_expense` and `approve_expense` moved to `PageDrivenTools`, which + * drives the visible page instead of posting behind it. They are absent + * here on purpose: this service must stay reachable with only an + * `ApiClient` fake, no router and no DOM. + */ + it('publishes only the reads and the report tools', () => { const names = tools.all().map((tool) => tool.contract.name); - expect(names).not.toContain('approve_expense'); expect(names).toEqual([ 'search_expenses', - 'submit_expense', 'get_budget_status', 'get_spend_summary', 'generate_report', @@ -370,5 +317,10 @@ describe('ExpenseTools', () => { 'fetch_categories', ]); }); + + it('no longer offers the tools that change expenses', () => { + expect('submitExpense' in tools).toBe(false); + expect('approveExpense' in tools).toBe(false); + }); }); }); diff --git a/frontend/src/app/tools/expense-tools.ts b/frontend/src/app/tools/expense-tools.ts index 311365b..ca70333 100644 --- a/frontend/src/app/tools/expense-tools.ts +++ b/frontend/src/app/tools/expense-tools.ts @@ -6,8 +6,6 @@ import { GET_BUDGET_STATUS, GET_SPEND_SUMMARY, SEARCH_EXPENSES, - SUBMIT_EXPENSE, - APPROVE_EXPENSE, type AnalyticsSummary, type BudgetStatus, type Category, @@ -53,33 +51,6 @@ export class ExpenseTools { }; } - /** Mutating — the Copilot must confirm in-chat before this runs. */ - submitExpense(): ActuoTool<{ - amount: number; - currency: string; - merchant?: string; - categoryId?: string; - note?: string; - expenseDate?: string; - }> { - return { - contract: SUBMIT_EXPENSE, - execute: async (args, { signal }) => { - const expense = await this.api.post( - '/expenses', - { ...args, expenseDate: args.expenseDate ?? today() }, - signal, - ); - const submitted = await this.api.post( - `/expenses/${expense.id}/submit`, - undefined, - signal, - ); - return summarize(submitted); - }, - }; - } - getBudgetStatus(): ActuoTool<{ category?: string }> { return { contract: GET_BUDGET_STATUS, @@ -180,30 +151,6 @@ export class ExpenseTools { }; } - /** - * State-gated (PRD §7): the registry only publishes this when the signed-in - * user is an admin/owner and something is actually pending. The server still - * re-checks the role — the gate is UX, not security. - */ - approveExpense(): ActuoTool<{ - expenseId: string; - decision: 'approved' | 'rejected'; - comment?: string; - }> { - return { - contract: APPROVE_EXPENSE, - execute: async ({ expenseId, decision, comment }, { signal }) => { - const path = decision === 'approved' ? 'approve' : 'reject'; - const expense = await this.api.post( - `/expenses/${expenseId}/${path}`, - { comment }, - signal, - ); - return summarize(expense); - }, - }; - } - /** Read-only lookup for category IDs (PRD §7). */ fetchCategories(): ActuoTool> { return { @@ -226,7 +173,6 @@ export class ExpenseTools { all(): ActuoTool[] { return [ this.searchExpenses(), - this.submitExpense(), this.getBudgetStatus(), this.getSpendSummary(), this.generateReport(), @@ -288,10 +234,6 @@ function summarize(expense: Expense) { }; } -function today(): string { - return new Date().toISOString().slice(0, 10); -} - /** A sleep that rejects promptly on abort, so cancellation feels instant. */ function delay(ms: number, signal: AbortSignal): Promise { return new Promise((resolve, reject) => { diff --git a/frontend/src/app/tools/navigate-destinations-contract.spec.ts b/frontend/src/app/tools/navigate-destinations-contract.spec.ts new file mode 100644 index 0000000..b6fff80 --- /dev/null +++ b/frontend/src/app/tools/navigate-destinations-contract.spec.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; +import type { Route } from '@angular/router'; +import { APP_DESTINATIONS } from '@actuo/shared'; +import { routes } from '../app.routes.js'; +import { authGuard } from '../core/session/auth-guard.js'; + +/** + * `APP_DESTINATIONS` is what an agent is told the app contains, and it lives in + * `@actuo/shared` while the routes live here — two files that have to agree. + * + * The invariant is exact in both directions: **every `authGuard`-protected + * route is a destination, and every destination is an `authGuard`-protected + * route.** That is stricter than "the paths resolve", and deliberately so — + * a one-directional check lets a new gated page ship undescribed, which is the + * failure that matters. An agent cannot navigate to a page nobody told it about, + * and it is exactly the page a human would have added a nav tab for. + * + * If a future route genuinely should be reachable by a person but not by an + * agent, add it to `NOT_A_DESTINATION` with the reason. The list is empty today + * because every gated page is somewhere an agent may usefully send the user. + * + * Same idea as `shared/src/page-limit-contract.spec.ts` and + * `report-format-contract.spec.ts`: pin the two definitions together rather + * than trusting them to be updated in step. + */ +const NOT_A_DESTINATION: readonly string[] = []; + +function isGated(route: Route): boolean { + return (route.canActivate ?? []).includes(authGuard); +} + +/** `app.routes.ts` declares paths without the leading slash. */ +function pathOf(route: Route): string { + return `/${route.path ?? ''}`; +} + +describe('navigate_to destinations', () => { + const gatedPaths = routes.filter(isGated).map(pathOf); + + it('finds the authenticated routes it is describing', () => { + // A sanity floor: if the route table is ever read wrongly, the two + // assertions below would both pass over an empty list and prove nothing. + expect(gatedPaths.length).toBeGreaterThan(0); + }); + + it('describes every authenticated page', () => { + const described = new Set(APP_DESTINATIONS.map((d) => d.path)); + const undescribed = gatedPaths.filter( + (path) => !described.has(path) && !NOT_A_DESTINATION.includes(path), + ); + + expect(undescribed).toEqual([]); + }); + + it('describes no page that is not an authenticated route', () => { + const gated = new Set(gatedPaths); + const stale = APP_DESTINATIONS.filter((d) => !gated.has(d.path)).map((d) => d.path); + + expect(stale).toEqual([]); + }); + + it('gives every destination a distinct id and a real description', () => { + const ids = APP_DESTINATIONS.map((d) => d.id); + expect(new Set(ids).size).toBe(ids.length); + + for (const destination of APP_DESTINATIONS) { + expect(destination.description.length).toBeGreaterThan(20); + expect(destination.path.startsWith('/')).toBe(true); + } + }); +}); diff --git a/frontend/src/app/tools/navigation-tools.spec.ts b/frontend/src/app/tools/navigation-tools.spec.ts new file mode 100644 index 0000000..40ecb7a --- /dev/null +++ b/frontend/src/app/tools/navigation-tools.spec.ts @@ -0,0 +1,126 @@ +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { APP_DESTINATIONS } from '@actuo/shared'; +import { NavigationTools } from './navigation-tools.js'; + +describe('NavigationTools', () => { + let router: { navigateByUrl: ReturnType; url: string }; + let tools: NavigationTools; + + /** Lands wherever it was sent, which is the happy path. */ + function landsWhereSent() { + router.navigateByUrl.mockImplementation((url: string) => { + router.url = url; + return Promise.resolve(true); + }); + } + + beforeEach(() => { + router = { navigateByUrl: vi.fn(), url: '/dashboard' }; + landsWhereSent(); + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [{ provide: Router, useValue: router }], + }); + tools = TestBed.inject(NavigationTools); + }); + + function run(destination: string) { + return tools.navigateTo().execute({ destination }, { signal: new AbortController().signal }); + } + + describe('the contract', () => { + /** + * It reads and writes nothing, but it changes what the user is looking at, + * and a client deciding whether to announce an action needs to be told. + * Same category as the embedded converter's UI-moving tools. + */ + it('is not read-only, and needs no confirmation', () => { + const { contract } = tools.navigateTo(); + expect(contract.annotations.readOnlyHint).toBe(false); + expect(contract.requiresConfirmation).toBe(false); + }); + + it('offers every destination in the schema enum', () => { + const properties = tools.navigateTo().contract.inputSchema['properties'] as { + destination: { enum: string[]; description: string }; + }; + expect(properties.destination.enum).toEqual(APP_DESTINATIONS.map((d) => d.id)); + }); + + /** + * The descriptions are the feature: an agent reading `getTools()` should + * learn what is on each page without opening any of them. + */ + it('describes each destination to the model', () => { + const properties = tools.navigateTo().contract.inputSchema['properties'] as { + destination: { description: string }; + }; + for (const destination of APP_DESTINATIONS) { + expect(properties.destination.description).toContain(destination.description); + } + }); + }); + + describe('navigating', () => { + it('sends the browser to the destination it was asked for', async () => { + const result = await run('budgets'); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/budgets'); + expect(result).toMatchObject({ destination: 'budgets', path: '/budgets', redirected: false }); + }); + + it('reaches every declared destination', async () => { + for (const destination of APP_DESTINATIONS) { + await run(destination.id); + expect(router.navigateByUrl).toHaveBeenCalledWith(destination.path); + } + }); + + /** + * The enum in a JSON Schema is a hint to the model, not a guarantee about + * what arrives. A value that is not in the table must fail here rather than + * reaching `navigateByUrl`, which would take any path at all. + */ + it('refuses a destination it does not know, and says what it accepts', async () => { + await expect(run('/etc/passwd')).rejects.toThrow(/Unknown destination/); + await expect(run('/etc/passwd')).rejects.toThrow(/budgets/); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + + /** + * LOAD-BEARING. A guard can redirect — an expired session lands on + * `/login` — and a model told it reached the budgets page while the user + * stares at a login form will keep building on that mistake. The router is + * read back afterwards precisely so the answer cannot be wrong. + */ + it('reports where it actually landed when a guard redirects', async () => { + router.navigateByUrl.mockImplementation(() => { + router.url = '/login'; + return Promise.resolve(false); + }); + + const result = await run('budgets'); + + expect(result).toMatchObject({ path: '/login', redirected: true }); + expect(JSON.stringify(result)).toContain('redirected'); + }); + + it('ignores query and fragment when reporting the landing path', async () => { + router.navigateByUrl.mockImplementation(() => { + router.url = '/expenses?status=submitted'; + return Promise.resolve(true); + }); + + const result = await run('expenses'); + + expect(result).toMatchObject({ path: '/expenses', redirected: false }); + }); + }); + + it('publishes exactly the navigation tool', () => { + expect(tools.all().map((tool) => tool.contract.name)).toEqual(['navigate_to']); + }); +}); diff --git a/frontend/src/app/tools/navigation-tools.ts b/frontend/src/app/tools/navigation-tools.ts new file mode 100644 index 0000000..9724ff4 --- /dev/null +++ b/frontend/src/app/tools/navigation-tools.ts @@ -0,0 +1,84 @@ +import { Injectable, PLATFORM_ID, inject } from '@angular/core'; +import { isPlatformBrowser } from '@angular/common'; +import { Router } from '@angular/router'; +import { APP_DESTINATIONS, NAVIGATE_TO, type AppDestination } from '@actuo/shared'; +import type { ActuoTool } from '../webmcp/tool-registry.js'; + +/** + * The navigation tool (PRD §7). + * + * Separate from `ExpenseTools` on purpose: every tool there is an HTTP call to + * `/api/*`, and this one touches the `Router` and nothing else. Mixing them + * would drag routing into the spec of a file that tests cleanly with a fake + * `ApiClient` and no router at all. + */ +@Injectable({ providedIn: 'root' }) +export class NavigationTools { + private readonly router = inject(Router); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + + navigateTo(): ActuoTool<{ destination: string }> { + return { + contract: NAVIGATE_TO, + execute: async ({ destination }) => { + /* + * Resolve through the table; the model's string never reaches the + * router. `navigateByUrl` would happily take an arbitrary path, and an + * enum in a JSON Schema is a hint to the model, not a guarantee about + * what arrives — a mistyped or hallucinated value has to fail here + * rather than navigating somewhere nobody described. + */ + const target = findDestination(destination); + if (!target) { + throw new Error( + `Unknown destination "${destination}". Valid destinations are: ` + + `${APP_DESTINATIONS.map((d) => d.id).join(', ')}.`, + ); + } + + // Defensive: tools only publish once signed in, which is browser-only. + if (!this.isBrowser) { + throw new Error('Navigation is only available in the browser.'); + } + + await this.router.navigateByUrl(target.path); + + /* + * Report where the browser actually is, not where it was told to go. + * A guard can redirect — an expired session lands on `/login` — and the + * boolean `navigateByUrl` resolves does not distinguish "blocked" from + * "redirected somewhere else" reliably. Reading the router back is the + * only answer that cannot be wrong, and a model told it reached the + * budgets page when the user is staring at a login form will keep + * building on that mistake. + */ + const landedOn = stripQuery(this.router.url); + const redirected = landedOn !== target.path; + + return { + destination: target.id, + path: landedOn, + description: redirected ? undefined : target.description, + redirected, + ...(redirected + ? { note: `Navigation to ${target.path} was redirected to ${landedOn}.` } + : {}), + }; + }, + }; + } + + /** Everything this service publishes. Mirrors `ExpenseTools.all()`. */ + all(): ActuoTool[] { + return [this.navigateTo()] as unknown as ActuoTool[]; + } +} + +function findDestination(id: unknown): AppDestination | undefined { + return APP_DESTINATIONS.find((d) => d.id === id); +} + +/** `Router.url` carries query and fragment; destinations are bare paths. */ +function stripQuery(url: string): string { + return url.split(/[?#]/)[0] ?? url; +} diff --git a/frontend/src/app/tools/page-driven-tools.spec.ts b/frontend/src/app/tools/page-driven-tools.spec.ts new file mode 100644 index 0000000..2d4664d --- /dev/null +++ b/frontend/src/app/tools/page-driven-tools.spec.ts @@ -0,0 +1,150 @@ +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ApiClient } from '../core/api/api-client.js'; +import { Copilot } from '../copilot/copilot.js'; +import { PageActions } from '../webmcp/page-actions.js'; +import { PageDrivenTools } from './page-driven-tools.js'; + +describe('PageDrivenTools', () => { + let router: { navigateByUrl: ReturnType; url: string }; + let copilot: { collapseForPageAction: ReturnType }; + let pages: PageActions; + let tools: PageDrivenTools; + let api: Record>; + + beforeEach(() => { + router = { navigateByUrl: vi.fn().mockResolvedValue(true), url: '/dashboard' }; + copilot = { collapseForPageAction: vi.fn() }; + api = { get: vi.fn(), post: vi.fn(), patch: vi.fn(), delete: vi.fn() }; + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: Router, useValue: router }, + { provide: Copilot, useValue: copilot }, + { provide: ApiClient, useValue: api }, + ], + }); + pages = TestBed.inject(PageActions); + tools = TestBed.inject(PageDrivenTools); + }); + + const signal = () => new AbortController().signal; + + /** A page that answers immediately, as a mounted one would. */ + function mount(action: string, result: unknown = { ok: true }) { + const handler = vi.fn().mockResolvedValue(result); + pages.provide(action, handler); + return handler; + } + + describe('every write goes through the page that owns it', () => { + it('opens Add expense and hands it the values', async () => { + const handler = mount('submit_expense', { id: 'exp-1', status: 'submitted' }); + + const result = await tools + .submitExpense() + .execute({ amount: 450, currency: 'INR', merchant: 'Barista' }, { signal: signal() }); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/add'); + expect(handler).toHaveBeenCalledWith( + { amount: 450, currency: 'INR', merchant: 'Barista' }, + expect.anything(), + ); + expect(result).toEqual({ id: 'exp-1', status: 'submitted' }); + }); + + it('opens Expenses to decide on a row', async () => { + const handler = mount('approve_expense'); + + await tools + .approveExpense() + .execute({ expenseId: 'exp-9', decision: 'approved' }, { signal: signal() }); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/expenses'); + expect(handler).toHaveBeenCalledWith( + { expenseId: 'exp-9', decision: 'approved' }, + expect.anything(), + ); + }); + + it('opens Budgets to change a limit', async () => { + const handler = mount('set_budget'); + + await tools.setBudget().execute({ amount: 20000 }, { signal: signal() }); + + expect(router.navigateByUrl).toHaveBeenCalledWith('/budgets'); + expect(handler).toHaveBeenCalledWith({ amount: 20000 }, expect.anything()); + }); + + /** The page has to be there before it can be asked to do anything. */ + it('navigates before asking for the handler', async () => { + const order: string[] = []; + router.navigateByUrl.mockImplementation(() => { + order.push('navigate'); + return Promise.resolve(true); + }); + pages.provide('set_budget', async () => { + order.push('run'); + return null; + }); + + await tools.setBudget().execute({ amount: 1 }, { signal: signal() }); + + expect(order).toEqual(['navigate', 'run']); + }); + }); + + /** + * LOAD-BEARING. The whole point is that an agent cannot change anything the + * user is not looking at. If this service could reach `ApiClient` it would be + * one line away from posting behind the page again. + */ + it('never touches the API itself', async () => { + mount('submit_expense'); + mount('approve_expense'); + mount('set_budget'); + + await tools.submitExpense().execute({ amount: 1, currency: 'INR' }, { signal: signal() }); + await tools + .approveExpense() + .execute({ expenseId: 'e', decision: 'approved' }, { signal: signal() }); + await tools.setBudget().execute({ amount: 1 }, { signal: signal() }); + + for (const call of Object.values(api)) expect(call).not.toHaveBeenCalled(); + }); + + it('reports a page that never opened, rather than acting invisibly', async () => { + vi.useFakeTimers(); + try { + const running = tools.setBudget().execute({ amount: 5 }, { signal: signal() }); + const assertion = expect(running).rejects.toThrow(/did not open in time/); + await vi.advanceTimersByTimeAsync(10_000); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + + /** + * Below `sm` the Copilot is a full-screen sheet, so the page it drives would + * be hidden behind it — which would defeat driving the page at all. + */ + it('gets the Copilot out of the way first', async () => { + mount('submit_expense'); + + await tools + .submitExpense() + .execute({ amount: 10, currency: 'INR' }, { signal: signal() }); + + expect(copilot.collapseForPageAction).toHaveBeenCalled(); + }); + + it('publishes the always-on writes, leaving approve to the state gate', () => { + expect(tools.all().map((tool) => tool.contract.name)).toEqual([ + 'submit_expense', + 'set_budget', + ]); + }); +}); diff --git a/frontend/src/app/tools/page-driven-tools.ts b/frontend/src/app/tools/page-driven-tools.ts new file mode 100644 index 0000000..6a6898f --- /dev/null +++ b/frontend/src/app/tools/page-driven-tools.ts @@ -0,0 +1,95 @@ +import { Injectable, PLATFORM_ID, inject } from '@angular/core'; +import { isPlatformBrowser } from '@angular/common'; +import { Router } from '@angular/router'; +import { APPROVE_EXPENSE, SET_BUDGET, SUBMIT_EXPENSE } from '@actuo/shared'; +import { Copilot } from '../copilot/copilot.js'; +import { PageActions } from '../webmcp/page-actions.js'; +import type { ActuoTool } from '../webmcp/tool-registry.js'; + +/** Which page owns each action. The tool goes there before asking for it. */ +const OWNER_PAGE: Readonly> = { + [SUBMIT_EXPENSE.name]: '/add', + [APPROVE_EXPENSE.name]: '/expenses', + [SET_BUDGET.name]: '/budgets', +}; + +/** + * The tools that change something, and therefore have to be seen changing it. + * + * Every one of these used to POST straight to `/api/*`, which gave the agent a + * back door no person has: the work happened, the screen did not move, and the + * user was left looking at figures that were quietly out of date. A human + * cannot add an expense without going to the Add expense page, or change a + * budget without going to the Budgets page. Neither can an agent now. + * + * Each `execute` is the same three steps — go to the page, wait for it to + * mount, hand it the work — and the page performs the action through the exact + * path its own buttons use. That is what makes the optimistic row patching, the + * form messages and the reloads apply with no new plumbing, and it is why + * **this service injects no `ApiClient`**: there is no way for it to reach the + * API behind the page's back, which is the property `page-driven-tools.spec.ts` + * asserts directly. + * + * Kept apart from `ExpenseTools` for the same reason `navigation-tools.ts` is: + * the reads test cleanly against a fake `ApiClient` with no router and no DOM, + * and these test cleanly against a fake router with no HTTP at all. + */ +@Injectable({ providedIn: 'root' }) +export class PageDrivenTools { + private readonly router = inject(Router); + private readonly pages = inject(PageActions); + private readonly copilot = inject(Copilot); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + + /** Create an expense and submit it for approval, on the Add expense page. */ + submitExpense(): ActuoTool<{ + amount: number; + currency: string; + merchant?: string; + categoryId?: string; + note?: string; + expenseDate?: string; + }> { + return { contract: SUBMIT_EXPENSE, execute: this.onOwnerPage(SUBMIT_EXPENSE.name) }; + } + + /** Approve or reject a row, on the Expenses page. */ + approveExpense(): ActuoTool<{ expenseId: string; decision: string; comment?: string }> { + return { contract: APPROVE_EXPENSE, execute: this.onOwnerPage(APPROVE_EXPENSE.name) }; + } + + /** Create or update a budget, on the Budgets page. */ + setBudget(): ActuoTool<{ categoryId?: string; amount: number; rollover?: boolean }> { + return { contract: SET_BUDGET, execute: this.onOwnerPage(SET_BUDGET.name) }; + } + + /** The always-on ones. `approve_expense` is state-gated by `ToolSession`. */ + all(): ActuoTool[] { + return [this.submitExpense(), this.setBudget()] as unknown as ActuoTool[]; + } + + /** + * Navigate to the page that owns `action`, then run it there. + * + * The navigation is not checked for success: the handler wait is the real + * test, and it reports a page that never arrived in words a model can pass + * on. A guard redirect and a slow chunk both land in the same place. + */ + private onOwnerPage>( + action: string, + ): (args: TArgs, context: { signal: AbortSignal }) => Promise { + return async (args, { signal }) => { + if (!this.isBrowser) { + throw new Error('This action needs the app open in a browser.'); + } + + // Below `sm` the Copilot covers the whole screen; drop it to the orb so + // the user can actually watch what happens next. + this.copilot.collapseForPageAction(); + + await this.router.navigateByUrl(OWNER_PAGE[action] ?? '/dashboard'); + const run = await this.pages.awaitHandler(action, signal); + return run(args as never, { signal }); + }; + } +} diff --git a/frontend/src/app/webmcp/page-actions.spec.ts b/frontend/src/app/webmcp/page-actions.spec.ts new file mode 100644 index 0000000..3f57f29 --- /dev/null +++ b/frontend/src/app/webmcp/page-actions.spec.ts @@ -0,0 +1,101 @@ +import { TestBed } from '@angular/core/testing'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { PageActions } from './page-actions.js'; + +describe('PageActions', () => { + let pages: PageActions; + + beforeEach(() => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({}); + pages = TestBed.inject(PageActions); + }); + + const live = () => new AbortController().signal; + + it('hands a tool the handler a mounted page provided', async () => { + const handler = vi.fn().mockResolvedValue({ ok: true }); + pages.provide('submit_expense', handler); + + const run = await pages.awaitHandler('submit_expense', live()); + await run({ amount: 5 } as never, { signal: live() }); + + expect(handler).toHaveBeenCalledWith({ amount: 5 }, expect.anything()); + }); + + it('waits for a page that has not mounted yet', async () => { + const waiting = pages.awaitHandler('set_budget', live()); + + const handler = vi.fn().mockResolvedValue('saved'); + pages.provide('set_budget', handler); + + await expect(waiting).resolves.toBe(handler); + }); + + it('reports nothing mounted before a page provides the action', () => { + expect(pages.has('set_budget')).toBe(false); + pages.provide('set_budget', vi.fn()); + expect(pages.has('set_budget')).toBe(true); + }); + + /** + * LOAD-BEARING. There is no API fallback, deliberately. Falling back would + * restore the invisible path this whole seam exists to remove, and it would + * do it only when something went wrong — a slow chunk, a guard redirect. + */ + it('fails when no page turns up, rather than doing the work invisibly', async () => { + vi.useFakeTimers(); + try { + const waiting = pages.awaitHandler('submit_expense', live(), 50); + const assertion = expect(waiting).rejects.toThrow(/did not open in time/); + await vi.advanceTimersByTimeAsync(60); + await assertion; + } finally { + vi.useRealTimers(); + } + }); + + it('gives up when the agent is stopped mid-wait', async () => { + const controller = new AbortController(); + const waiting = pages.awaitHandler('submit_expense', controller.signal); + + controller.abort(new Error('Stopped.')); + + await expect(waiting).rejects.toThrow('Stopped.'); + }); + + it('stops offering an action once the page is destroyed', () => { + const release = pages.provide('set_budget', vi.fn()); + release(); + expect(pages.has('set_budget')).toBe(false); + }); + + it('ignores a double release', () => { + const release = pages.provide('set_budget', vi.fn()); + release(); + pages.provide('set_budget', vi.fn()); + release(); + + expect(pages.has('set_budget')).toBe(true); + }); + + /** + * ORDER IS SIGNIFICANT. During a route change Angular constructs the + * incoming component before destroying the outgoing one — the same ordering + * `ConverterSession` documents. If teardown deleted unconditionally, the page + * being destroyed would wipe the registration the new page just made, and the + * action would vanish exactly when a tool navigated to reach it. + */ + it('lets an incoming page keep its registration when the outgoing one tears down', async () => { + const outgoing = vi.fn(); + const releaseOutgoing = pages.provide('submit_expense', outgoing); + + const incoming = vi.fn(); + pages.provide('submit_expense', incoming); + + releaseOutgoing(); + + expect(pages.has('submit_expense')).toBe(true); + await expect(pages.awaitHandler('submit_expense', live())).resolves.toBe(incoming); + }); +}); diff --git a/frontend/src/app/webmcp/page-actions.ts b/frontend/src/app/webmcp/page-actions.ts new file mode 100644 index 0000000..5a9707d --- /dev/null +++ b/frontend/src/app/webmcp/page-actions.ts @@ -0,0 +1,132 @@ +import { Injectable } from '@angular/core'; + +/** What a page does when a tool hands it work. */ +export type PageAction< + TArgs extends Record = Record, + TResult = unknown, +> = (args: TArgs, context: { signal: AbortSignal }) => Promise; + +/** + * How long a tool waits for the page that owns an action to mount. + * + * Generous, because the page is lazily loaded: on a cold navigation the chunk + * has to be fetched before the component exists at all. + */ +export const PAGE_ACTION_TIMEOUT_MS = 8000; + +/** + * The rendezvous between a tool and the page that can perform it. + * + * ## Why tools do not just call the API + * + * A person cannot add an expense without going to the Add expense page, or + * change a budget without going to the Budgets page. When a tool posts straight + * to `/api/*` the agent gets a private back door: the work happens, the screen + * does not move, and the user is left looking at figures that are quietly + * wrong. So a mutating tool navigates to the page that owns the action and + * hands the work here — the page then does it through the exact code path its + * own buttons use, which is why the optimistic row patching, the form messages + * and the reloads all keep working with no new plumbing. + * + * Modelled on {@link ToolSession} and `ConverterSession`: "which page can do + * what right now" is session state, not component state. + * + * **There is deliberately no API fallback when nothing answers.** A fallback + * would restore exactly the invisible path this exists to remove, and it would + * do so only in the cases hardest to notice — a slow chunk, a guard redirect. + * A timeout is an error, which the model reports and the user can see. + */ +@Injectable({ providedIn: 'root' }) +export class PageActions { + private readonly handlers = new Map>(); + private readonly waiting = new Map) => void>>(); + + /** + * Publish an action for as long as the page is mounted. Returns an + * unregister to call on destroy. + */ + provide, TResult>( + name: string, + handler: PageAction, + ): () => void { + const stored = handler as unknown as PageAction; + this.handlers.set(name, stored); + + for (const resolve of this.waiting.get(name) ?? []) resolve(stored); + this.waiting.delete(name); + + let released = false; + return () => { + if (released) return; + released = true; + /* + * ORDER IS SIGNIFICANT. Only clear the handler if it is still ours. + * During a route change Angular constructs the incoming component before + * destroying the outgoing one — the same ordering `ConverterSession` + * documents — so an unconditional delete here would let a page being torn + * down wipe the registration the incoming page has already made. + */ + if (this.handlers.get(name) === stored) this.handlers.delete(name); + }; + } + + /** Whether a mounted page can perform this action right now. */ + has(name: string): boolean { + return this.handlers.has(name); + } + + /** + * Resolve once a page provides `name`. Rejects on abort or timeout rather + * than falling back to anything — see the class comment. + */ + awaitHandler( + name: string, + signal: AbortSignal, + timeoutMs: number = PAGE_ACTION_TIMEOUT_MS, + ): Promise> { + const existing = this.handlers.get(name); + if (existing) return Promise.resolve(existing); + + return new Promise((resolve, reject) => { + let settled = false; + + const waiters = this.waiting.get(name) ?? new Set(); + this.waiting.set(name, waiters); + + const cleanup = () => { + waiters.delete(onProvided); + if (waiters.size === 0) this.waiting.delete(name); + clearTimeout(timer); + signal.removeEventListener('abort', onAbort); + }; + + const onProvided = (handler: PageAction) => { + if (settled) return; + settled = true; + cleanup(); + resolve(handler); + }; + + const onAbort = () => { + if (settled) return; + settled = true; + cleanup(); + reject(signal.reason ?? new Error('Aborted.')); + }; + + const timer = setTimeout(() => { + if (settled) return; + settled = true; + cleanup(); + reject( + new Error( + `The page that performs "${name}" did not open in time, so nothing was changed.`, + ), + ); + }, timeoutMs); + + waiters.add(onProvided); + signal.addEventListener('abort', onAbort, { once: true }); + }); + } +} diff --git a/frontend/src/app/webmcp/tool-session.spec.ts b/frontend/src/app/webmcp/tool-session.spec.ts index 9a9d54e..74b2814 100644 --- a/frontend/src/app/webmcp/tool-session.spec.ts +++ b/frontend/src/app/webmcp/tool-session.spec.ts @@ -34,7 +34,9 @@ describe('ToolSession (state-gated approve_expense)', () => { 'generate_report', 'get_budget_status', 'get_spend_summary', + 'navigate_to', 'search_expenses', + 'set_budget', 'submit_expense', ]); }); diff --git a/frontend/src/app/webmcp/tool-session.ts b/frontend/src/app/webmcp/tool-session.ts index d33e5ec..ad00306 100644 --- a/frontend/src/app/webmcp/tool-session.ts +++ b/frontend/src/app/webmcp/tool-session.ts @@ -1,6 +1,8 @@ import { Injectable, effect, inject, signal } from '@angular/core'; import { APPROVE_EXPENSE, type Role } from '@actuo/shared'; import { ExpenseTools } from '../tools/expense-tools.js'; +import { NavigationTools } from '../tools/navigation-tools.js'; +import { PageDrivenTools } from '../tools/page-driven-tools.js'; import { ToolRegistry } from './tool-registry.js'; /** @@ -19,6 +21,8 @@ import { ToolRegistry } from './tool-registry.js'; export class ToolSession { private readonly registry = inject(ToolRegistry); private readonly tools = inject(ExpenseTools); + private readonly navigation = inject(NavigationTools); + private readonly pageDriven = inject(PageDrivenTools); private readonly role = signal(null); private readonly pendingApprovals = signal(0); @@ -36,7 +40,7 @@ export class ToolSession { const isExposed = this.registry.has(APPROVE_EXPENSE.name); if (shouldExpose && !isExposed) { - void this.registry.register(this.tools.approveExpense()); + void this.registry.register(this.pageDriven.approveExpense()); } else if (!shouldExpose && isExposed) { this.registry.unregister(APPROVE_EXPENSE.name); } @@ -46,7 +50,11 @@ export class ToolSession { /** Publish the always-on tools. Safe to call more than once. */ async start(): Promise { if (this.started()) return; - for (const tool of this.tools.all()) { + for (const tool of [ + ...this.tools.all(), + ...this.navigation.all(), + ...this.pageDriven.all(), + ]) { await this.registry.register(tool); } this.started.set(true); diff --git a/shared/src/tools.ts b/shared/src/tools.ts index cc3d5c7..8b689b8 100644 --- a/shared/src/tools.ts +++ b/shared/src/tools.ts @@ -84,7 +84,7 @@ export const SUBMIT_EXPENSE: ActuoToolContract = { name: 'submit_expense', title: 'Submit an expense', description: - 'Create an expense and submit it for approval. Use when the user describes a purchase they want recorded. If the user mentions a category, call fetch_categories first to get the categoryId.', + 'Create an expense and submit it for approval. Use when the user describes a purchase they want recorded. If the user mentions a category, call fetch_categories first to get the categoryId. This opens the Add expense page and fills the form in front of the user, so tell them where they are going.', inputSchema: { type: 'object', properties: { @@ -219,6 +219,160 @@ export const FETCH_CATEGORIES: ActuoToolContract = { requiresConfirmation: false, }; +/** + * Where an agent may send the browser (PRD §7). + * + * Each `description` is written for a model, not for a person: it says what is + * *on* the page, so an agent reading `getTools()` learns the shape of the app + * without visiting a single route. That is the point of the tool — an agent + * driving Actuo from outside otherwise has to read the DOM and guess where to + * click, which is slow and breaks on any markup change. + * + * `frontend/src/app/tools/navigate-destinations-contract.spec.ts` pins this + * list against the real router config in both directions, so a new gated page + * cannot ship undescribed and an entry here cannot outlive its route. + */ +export interface AppDestination { + /** The value the model passes as `destination`. */ + id: string; + /** Router path, leading slash included. */ + path: string; + /** What is on that page. */ + description: string; +} + +export const APP_DESTINATIONS: readonly AppDestination[] = [ + { + id: 'dashboard', + path: '/dashboard', + description: + "This month's spend, pace against budget, the 14-day trend, the pending-approval count, and recent activity.", + }, + { + id: 'expenses', + path: '/expenses', + description: + 'The full expense list with search and status filters, and the per-row submit, approve and reject actions.', + }, + { + id: 'add', + path: '/add', + description: 'The quick-entry form for filing a new expense.', + }, + { + id: 'budgets', + path: '/budgets', + description: 'Per-category budgets and how much of each is used this period.', + }, + { + id: 'convert', + path: '/convert', + description: + 'The embedded currency converter, for looking up a European Central Bank rate. Advisory only: it changes no Actuo figure.', + }, + { + id: 'agent', + path: '/agent', + description: + 'The WebMCP surface: the tools this page publishes, the tools it discovered on other origins, and a log of every tool call.', + }, + { + id: 'settings', + path: '/settings', + description: + "Profile, the organization's base currency, theme, and the user's own Gemini API key.", + }, +] as const; + +/** The destination list, flattened into one line the model reads in the schema. */ +const DESTINATION_GUIDE = APP_DESTINATIONS.map((d) => `${d.id} — ${d.description}`).join(' '); + +/** + * Move the browser (PRD §7). + * + * The one tool here whose entire effect is on the viewport. It exists for + * agents that drive the app from outside: without it, "show me my budgets" + * means reading the DOM and guessing which element to click. + * + * **Not `readOnlyHint`.** It reads and writes no data, but it changes what the + * user is looking at, and a client deciding whether to announce an action + * should be told that. It is the same category the embedded converter's own + * UI-moving tools sit in, which `/agent` renders as `Mutating`. + * + * No confirmation: navigation is trivially reversible and touches no money. + * `requiresConfirmation` is for things that move money or change approval state. + */ +export const NAVIGATE_TO: ActuoToolContract = { + name: 'navigate_to', + title: 'Go to a page', + description: + "Move the browser to one of Actuo's pages. Use it when the user asks to see or go to a screen, " + + 'or when what they want is something they need to be looking at — a form to fill in, a chart to ' + + 'read. It only changes what is displayed and returns no expense data, so never call it to answer ' + + 'a question; use the read tools for that. Returns the path actually landed on, which can differ ' + + 'from the one asked for if the session has expired.', + inputSchema: { + type: 'object', + properties: { + destination: { + type: 'string', + enum: APP_DESTINATIONS.map((d) => d.id), + description: `Which page to open. ${DESTINATION_GUIDE}`, + }, + }, + required: ['destination'], + additionalProperties: false, + }, + annotations: { readOnlyHint: false }, + requiresConfirmation: false, +}; + +/** + * Set or change a category's budget (PRD §6.3). + * + * A person cannot change a budget without going to the Budgets page and using + * the form there, and neither can an agent: this drives that form. The route + * (`POST /budgets`, `PATCH /budgets/:id`) and the form both already existed — + * only the tool was missing, which meant an agent could *read* a budget with + * `get_budget_status` and never touch one. + * + * Mutating and confirmed: it changes a spending limit, which is the same bar + * `submit_expense` clears. Owners and admins only, enforced server-side. + */ +export const SET_BUDGET: ActuoToolContract = { + name: 'set_budget', + title: 'Set a budget', + description: + 'Set or change the monthly budget for a category, or the organization-wide budget. ' + + 'Call fetch_categories first to get the categoryId. Creates the budget if none exists ' + + 'and updates it otherwise. Owners and admins only. This opens the Budgets page and fills ' + + 'the form in front of the user, so tell them where they are going.', + inputSchema: { + type: 'object', + properties: { + categoryId: { + type: 'string', + description: + 'Category UUID from fetch_categories. Omit for the organization-wide budget.', + }, + amount: { + type: 'number', + minimum: 0, + description: 'The monthly limit, in the organization base currency.', + }, + rollover: { + type: 'boolean', + default: false, + description: 'Whether unused budget carries into the next month.', + }, + }, + required: ['amount'], + additionalProperties: false, + }, + annotations: { readOnlyHint: false }, + requiresConfirmation: true, +}; + /** * State-gated (PRD §7): only registered when the signed-in user is an * admin/owner AND at least one expense is awaiting approval. Registration and @@ -228,7 +382,7 @@ export const APPROVE_EXPENSE: ActuoToolContract = { name: 'approve_expense', title: 'Approve or reject an expense', description: - 'Approve or reject a submitted expense. Only available to admins and owners while items are pending.', + 'Approve or reject a submitted expense. Only available to admins and owners while items are pending. This opens the Expenses page and acts on the row in front of the user.', inputSchema: { type: 'object', properties: { @@ -261,6 +415,8 @@ export const ALWAYS_ON_TOOLS: readonly ActuoToolContract[] = [ GENERATE_REPORT, DOWNLOAD_REPORT, FETCH_CATEGORIES, + NAVIGATE_TO, + SET_BUDGET, ] as const; export const ALL_TOOL_CONTRACTS: readonly ActuoToolContract[] = [