diff --git a/CLAUDE.md b/CLAUDE.md index 14ff629..4124753 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +89,7 @@ declarations (see "Why pnpm changes things" below). ```bash pnpm install # `pnpm install --frozen-lockfile` is the CI equivalent of `npm ci` -pnpm run dev # shared, then backend (:3000) + frontend (:4200) + partner demo (:4201) +pnpm run dev # shared, then backend (:3000) + frontend (:4200) pnpm run build # shared -> backend -> frontend, in that order pnpm test # backend + frontend unit tests pnpm run test:e2e # backend e2e @@ -140,7 +140,7 @@ and `pnpm run build` handle the ordering. ``` `frontend/` and `backend/` are separate codebases (own package.json, tsconfig, tests) -that ship as **one** Firebase App Hosting deploy: a single Node process routes +that ship as **one** deploy — a Docker image on Render: a single Node process routes `/api/*` to Nest and everything else to Angular's SSR handler. That process is `server.mjs` at the repo root — see "The deploy" below. @@ -215,36 +215,79 @@ the PRD assumes that do **not** hold: Two live compatibility traps for any `getTools()` consumer: 1. **`inputSchema` type varies by Chrome version.** Chrome 149–153 (most of the origin-trial - population, including the Chrome 152 on this machine) return it as a **serialized JSON + population, including the Chrome 151 on this machine) return it as a **serialized JSON string**; 154+ returns an object. Branch on `typeof` and guard the parse. 2. **`title` may be an empty string**, not absent — `tool.title ?? tool.name` silently yields `""`. Use `tool.title || tool.name`. `annotations` genuinely may be absent. -**Enabling native WebMCP** (Chrome 152): `chrome://flags/#enable-webmcp-testing` +**Enabling native WebMCP** (Chrome 151 here): `chrome://flags/#enable-webmcp-testing` ("Enables the WebMCP API") and `chrome://flags/#devtools-webmcp-support`, or launch with `--enable-blink-features=WebMCP`. -**Both traps are confirmed on this machine, not theoretical.** Driving -`/partner-demo/` in the local Chrome 152 with WebMCP enabled: -`getTools()` returned `typeof inputSchema === 'string'` for every tool, and -`executeTool()` resolved to a JSON **string** rather than an object. `registerTool` -with `exposedTo`, the `readOnlyHint` annotation, and `executeTool()` all work -end to end. `parseInputSchema()` and `parseToolResult()` are what keep that -working — do not "simplify" them away. +**Both traps are confirmed on this machine, not theoretical.** Re-measured on +2026-09-03 against the deployed converter (`https://cambiaro.programmersingh.dev`) +framed from `localhost:4200` in Chrome **151** with WebMCP enabled — a genuine +origin boundary, not a page this repo serves. `getTools({fromOrigins})` returned +all seven of its tools with `typeof inputSchema === 'string'`, and +`executeTool()` resolved to a JSON **string** rather than an object. So +`registerTool` with `exposedTo`, the `readOnlyHint` annotation, `fromOrigins` +discovery and `executeTool()` all work end to end across origins. +`parseInputSchema()` and `parseToolResult()` are what keep that working — do not +"simplify" them away. + +The same run proved the loop closes: `executeTool(convertCurrency, {amount:200, +from:'EUR', to:'INR'})` returned `200 EUR = 22,018.00 INR (1 EUR = 110.09 INR, +2 Sep 2026)` **and the embedded widget moved to that conversion** — which is what +the other side marking those tools as changing its own UI buys. Annotations +survive the boundary intact: its four answering tools rendered `Read-only` in +`/agent` and its three UI-moving ones `Mutating`. **Cross-origin requires native Chrome.** `@mcp-b/webmcp-polyfill` rejects non-empty `fromOrigins`/`exposedTo` with `NotSupportedError`, so the polyfill is a same-origin fallback only. -**Cross-origin also requires an actual second origin.** The partner page lives in -`frontend/public/partner-demo/`, so it is *also* served by the app itself — and from -there `normalizeRegisteredTool()` marks its tools `isCrossOrigin: false`, which is -exactly the set the Copilot filters out. `scripts/partner-server.mjs` (zero -dependencies, `node:http`) serves `frontend/public` on **:4201** so the same -`/partner-demo/` path exists on a different origin; `pnpm run dev` starts it as a -third pane. The origin the app embeds is `PARTNER_DEMO_ORIGIN`, served to the browser -by `GET /api/config` — so a deploy changes it without a rebuild. When it equals the -app's own origin, `/agent` says so instead of showing an empty list. +**Cross-origin also requires an actual second origin, and it must not be one we +serve.** A page served by the app is marked `isCrossOrigin: false` by +`normalizeRegisteredTool()`, which is exactly the set the Copilot filters out. +This repo used to ship a synthetic partner page (`frontend/public/partner-demo/`) +plus a static server on :4201 to give it a second origin; both are gone. They +were only ever true on localhost, and a stand-in exercised in dev but never in +production is how a path stays broken in one of them unnoticed. + +What the app frames is **`CONVERTER_URL`** (it replaced `PARTNER_DEMO_ORIGIN`), +served to the browser by `GET /api/config` — so a deploy changes it without a +rebuild. **Development and production now use the same real converter**; the dev +default is the public one, which costs a network dependency and buys one code +path instead of two. + +It is a **full URL rather than a bare origin** because a converter need not sit +at the root of its host — a GitHub Pages *project* site is +`.github.io//`, and only a custom domain puts it at `/` — and the +`?actuo=` handshake is appended to it either way. Consumers derive the origin +with `new URL(value).origin`; a second "path" variable that had to stay in step +would be one too many. Non-http(s) values are rejected before the sanitizer +bypass. When it equals the app's own origin, every surface says so instead of +showing an empty list. It is deliberately **not** defaulted in production: a +deploy names the converter it trusts rather than inheriting one. + +**The `?actuo=` handshake is what makes any of it work.** A WebMCP tool is +visible only to its own document unless registration names an origin in +`exposedTo`, so the framed page has to be *told* which origin to expose to. +`ConverterSession.frameUrl` appends `?actuo=`, and the converter +reads it and passes it to `registerTool`'s `exposedTo`. +Sending it at runtime rather than hardcoding our hostname there means a deploy +URL can change without a release on the other side. + +**`ConverterSession` owns the discovery lifecycle, not any page.** It used to +belong to `/agent`, which was fine while one page framed one other origin. With four +surfaces, page-owned teardown cleared the Copilot's remote tools while a frame +was still mounted elsewhere. Two rules live there now: **only one frame at a +time** (`getTools()` returns a descriptor per *window*, so two live frames +publish two tools called `convertCurrency`), and **reference-counted discovery** +(during a route change Angular builds the incoming component before destroying +the outgoing one, so clear-on-destroy would wipe what the new surface just +found). `Copilot.discoverRemoteTools()` also dedupes by name now, which fixes +the same duplicate-window bug wherever a second window publishes the same tools. ## The deploy @@ -315,20 +358,36 @@ those into build args; on any other host it needs an explicit `--build-arg`. The URLs — `` in the sitemap, `og:image`, `canonical` — must be decided before the build finishes. `index.html`, `sitemap.xml` and `robots.txt` carry a `__PUBLIC_ORIGIN__` sentinel that survives prerendering into every generated -file, and `scripts/stamp-seo.mjs` replaces it across `dist/frontend/browser` as -the last step of `pnpm run build`. Unset, it substitutes `''` and everything -stays root-relative and valid. It must carry the scheme: the value is -substituted verbatim, so a bare hostname yields a `` that is not a URL. +file, and `scripts/stamp-seo.mjs` replaces it across **the whole +`dist/frontend` tree** as the last step of `pnpm run build`. + +**It must not narrow to `browser/` again.** It did, and the deployed site served +a literal `__PUBLIC_ORIGIN__` in its `canonical` and `og:image`: Angular keeps +its own copies of the page HTML under `server/` — `index.server.html` and the +`assets-chunks/*.mjs` templates, `index_csr_html.mjs` among them — and those are +what the SSR handler serves. `sitemap.xml` and `robots.txt` looked right the +whole time because they come from `browser/`, which is what made it hard to see. +`.mjs` is in the stampable extension set for exactly this reason. + +Unset, `PUBLIC_ORIGIN` substitutes `''` and everything stays root-relative and +valid. It must carry the scheme: the value is substituted verbatim, so a bare +hostname yields a `` that is not a URL. + +**`pnpm run verify:deploy `** checks the deployed result of all of this — +`ng-server-context` present, no sentinel surviving, and the converter configured +on another origin. Local green does not mean deployed correct: SSR fell back to +CSR in production while every test passed and the page looked fine. **The service worker must never cache `/api`.** `ngsw-config.json` has no `dataGroups` at all, deliberately: a cached response would show stale money and would undercut the promise that every read goes through an authenticated route. -`navigationUrls` also excludes `/partner-demo/**`, which is a separate site that -re-registers WebMCP tools on each load. +`navigationUrls` no longer needs a `/partner-demo/**` exclusion — the converter +is a different origin, which the service worker never sees. -**`NODE_ENV=production` changes one behaviour on purpose**: `EnvService.partnerOrigin` -drops its `http://localhost:4201` default, because serving that from a deployed -instance makes `/agent` embed an iframe pointing at each visitor's own machine. +**`NODE_ENV=production` changes one behaviour on purpose**: `EnvService.converterUrl` +drops its development default, because a deploy should name the converter it +trusts through `CONVERTER_URL` rather than silently framing a third party nobody +chose. Unset, the converter surfaces say so. 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. @@ -395,7 +454,7 @@ request; the access token deliberately carries no role claim. *human* can make, so it logs itself with the actor `agentInvoked` reports; everything else through the registry is an agent. - `pages/agent/` — `/agent`, the WebMCP surface made visible: browser support, - the cross-origin partner iframe and what it exposed, and the live invocation + the cross-origin converter iframe and what it exposed, and the live invocation log. The only consumer of `discoveredTools()` and `invocationLog()`. ## Thought signatures (Gemini 3 function calling) @@ -461,6 +520,24 @@ wrong number beat a bar reading zero. It was not slightly wrong: a $200 charge was counted as ₹200. When a real FX pass starts filling `converted_amount`, those rows re-enter every total with no code change. +**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 +advisory: a rate a person reads off another site is not the historical rate +locked at entry, so nothing it shows may reach `converted_amount`, `sumSpend()`, +`sumByCategory()`, or the `excludedNotice()` copy. + +That is enforced structurally rather than by good intentions: +`CurrencyConverter` has **no `output()`, no `postMessage` listener, and never +reads a value back out of the frame**, so no converted figure exists anywhere in +Actuo's component tree to be wired in. Adding one would mean first inventing a +return channel — a visible, reviewable act rather than a one-line slip. +`currency-converter.spec.ts` asserts the component's inputs and outputs +directly, and the dashboard and expenses specs assert that opening the lookup +moves no figure. **`core/expense/amount.ts` and its spec were not touched by +that work**; if a change to the converter needs to edit them, the change is +wrong. + ## Architectural rules that must not be violated These are the load-bearing constraints — most bugs worth preventing here are violations of one of them. diff --git a/Dockerfile b/Dockerfile index 9f61e86..936fe88 100644 --- a/Dockerfile +++ b/Dockerfile @@ -74,9 +74,10 @@ FROM node:22-bookworm-slim AS runner WORKDIR /app -# NODE_ENV=production is read by EnvService.partnerOrigin, which drops its -# localhost:4201 default here — otherwise /agent would embed an iframe pointing -# at each visitor's own machine. +# NODE_ENV=production is read by EnvService.converterUrl, which drops its +# development default here. A deploy names the converter it trusts through +# CONVERTER_URL rather than inheriting one; unset, the converter surfaces say +# so instead of framing a third party nobody chose. ENV NODE_ENV=production ENV PORT=8080 diff --git a/Progress.md b/Progress.md index 6fc3cf5..f373d4d 100644 --- a/Progress.md +++ b/Progress.md @@ -2,7 +2,7 @@ Tracks every feature in the PRD against what is actually in the codebase. -**Last audited:** 2026-08-29 · **Baseline:** 9 shared · 64 backend unit · 34 backend e2e · 742 frontend +**Last audited:** 2026-09-03 · **Baseline:** 9 shared · 65 backend unit · 34 backend e2e · 786 frontend Status is evidence-based, not aspirational. A row is `DONE` only when the code exists, is reachable from the running app, and has a test. A file existing is not @@ -68,6 +68,7 @@ Open the feature and use it. Tests did not catch the aurora-scarcity violation | **WebMCP** | Works in flag-enabled Chrome (`chrome://flags/#enable-webmcp-testing`) **and** still works with the flag off | | **Money / totals** | The number is right on a dataset larger than one page (100 rows) — truncation shows a wrong figure, not an obvious gap | | **UI** | Both themes, phone and desktop widths, keyboard reachable | +| **Anything deployed** | `pnpm run verify:deploy `. Local green does not mean deployed correct — SSR fell back to CSR in production while every test passed, and the page looked fine | ### 6. Update `CLAUDE.md` Only if the change alters a rule, a command, or a non-obvious constraint that @@ -147,8 +148,9 @@ their own expense. | Item | Phase | Status | Notes | |---|---|---|---| | 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 | ⬜ | No FX client, no cache, no rates table | +| Live FX + daily cache | 1 | ⬜ | No FX client, no cache, no rates table. The embedded converter does **not** count — see below | | Historical rate lock | 1 | ⬜ | No rate column | +| 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` | > **Totals are now honest about what they exclude.** `convertedAmount` is still > only set when the currency already equals the base currency, so foreign rows @@ -164,6 +166,16 @@ their own expense. > This is the honest interim, not the feature: real FX (live rates, daily cache, > historical lock) is still ⬜, and the moment `converted_amount` starts being > filled, those rows re-enter every total with no code change. +> +> **The embedded converter does not change any of that, deliberately.** It is a +> reference a person reads, framed from a separate origin; it writes nothing, +> and `CurrencyConverter` has no `output()` and no `postMessage` listener, so +> there is no channel a converted figure could travel back through. That +> absence is the enforcement, and `currency-converter.spec.ts` asserts it +> directly — along with the two surface specs proving that opening the lookup +> moves neither the row's amount nor the dashboard total. `amount.spec.ts` is +> untouched by this work, which is the point: if a change here needed to edit +> it, the change would be wrong. **Verify:** file expenses in two currencies and confirm the dashboard total is not a naive sum, and that it says how many rows it left out. @@ -199,7 +211,7 @@ Cross-origin is live as of 2026-08-29; only the standalone-script packaging (Pha | Confirmation before mutating tools | 0 | ✅ | In-chat card. PRD says "native dialog"; in-chat was chosen deliberately | | Key-setup flow when no key | 0 | ✅ | Opens into setup rather than failing silently | | Embeddable via one ` - - diff --git a/frontend/public/robots.txt b/frontend/public/robots.txt index a19b68c..7e7edde 100644 --- a/frontend/public/robots.txt +++ b/frontend/public/robots.txt @@ -1,11 +1,11 @@ # Actuo — only the public surface is indexable (PRD §8.5). User-agent: * Allow: /$ -Allow: /partner-demo/ Disallow: /dashboard Disallow: /expenses Disallow: /add Disallow: /budgets +Disallow: /convert Disallow: /agent Disallow: /settings Disallow: /login diff --git a/frontend/src/app/app.routes.ts b/frontend/src/app/app.routes.ts index f8839fb..86e4662 100644 --- a/frontend/src/app/app.routes.ts +++ b/frontend/src/app/app.routes.ts @@ -64,6 +64,16 @@ export const routes: Routes = [ title: 'Budgets · Actuo', data: { robots: NOT_INDEXABLE }, }, + { + // The converter as a page of its own (PRD §6.5). Deliberately not a nav + // tab: the bottom bar is measured tight at six on a phone, and this is + // linked from the places the question actually comes up. + path: 'convert', + canActivate: [authGuard], + loadComponent: () => import('./pages/convert/convert.js').then((m) => m.Convert), + title: 'Currency converter · Actuo', + data: { robots: NOT_INDEXABLE }, + }, { // The WebMCP surface, made visible: what this page publishes, what it can // reach on other origins, and every tool call that has run (PRD §7). diff --git a/frontend/src/app/converter/converter-session.spec.ts b/frontend/src/app/converter/converter-session.spec.ts new file mode 100644 index 0000000..81e700c --- /dev/null +++ b/frontend/src/app/converter/converter-session.spec.ts @@ -0,0 +1,280 @@ +import { TestBed } from '@angular/core/testing'; +import { signal } from '@angular/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ApiClient } from '../core/api/api-client.js'; +import { Copilot } from '../copilot/copilot.js'; +import { ToolRegistry } from '../webmcp/tool-registry.js'; +import type { NormalizedTool } from '../webmcp/webmcp.types.js'; +import { ConverterSession } from './converter-session.js'; + +const SELF_ORIGIN = globalThis.location.origin; +const CONVERTER_URL = 'https://cambiaro.example/'; +const CONVERTER_ORIGIN = 'https://cambiaro.example'; + +describe('ConverterSession', () => { + let api: { get: ReturnType }; + let copilot: { + crossOriginTools: ReturnType>; + discoverRemoteTools: ReturnType; + clearRemoteTools: ReturnType; + }; + let registry: { onToolChange: ReturnType }; + let unsubscribe: ReturnType; + + function create(converterUrl: string | null = CONVERTER_URL): ConverterSession { + api.get.mockImplementation((path: string) => + path === '/config' + ? Promise.resolve(converterUrl === null ? {} : { converterUrl }) + : Promise.reject(new Error(`unexpected ${path}`)), + ); + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: ApiClient, useValue: api }, + { provide: Copilot, useValue: copilot }, + { provide: ToolRegistry, useValue: registry }, + ], + }); + return TestBed.inject(ConverterSession); + } + + /** Let the cached `/config` promise and the discovery it chains resolve. */ + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + + beforeEach(() => { + api = { get: vi.fn() }; + copilot = { + crossOriginTools: signal([]), + discoverRemoteTools: vi.fn().mockResolvedValue(undefined), + clearRemoteTools: vi.fn(), + }; + unsubscribe = vi.fn(); + registry = { onToolChange: vi.fn(() => unsubscribe) }; + }); + + describe('where the converter is', () => { + it('derives the origin from a URL that carries a path', async () => { + // A converter need not sit at the root of its host — a GitHub Pages + // project site is `.github.io//`. One variable carries both; + // two that had to agree would be one too many. + const session = create('https://theprogrammersingh.github.io/cambiaro/'); + await session.ensureConfig(); + + expect(session.converterOrigin()).toBe('https://theprogrammersingh.github.io'); + expect(session.isAvailable()).toBe(true); + }); + + it('reports nothing available when none is configured', async () => { + const session = create(null); + await session.ensureConfig(); + + expect(session.converterOrigin()).toBe(''); + expect(session.isAvailable()).toBe(false); + expect(session.frameUrl()).toBeNull(); + }); + + /** + * A misconfigured variable must not become a `javascript:` frame src. This + * runs *before* the sanitizer bypass in `CurrencyConverter`, which is what + * makes that bypass defensible. + */ + it('refuses a URL whose scheme is not http or https', async () => { + for (const bad of ['javascript:alert(1)', 'data:text/html,x', 'file:///etc/passwd']) { + const session = create(bad); + await session.ensureConfig(); + expect(session.converterOrigin()).toBe(''); + expect(session.frameUrl()).toBeNull(); + } + }); + + it('survives a malformed URL rather than taking the surfaces down', async () => { + const session = create('not a url at all'); + await session.ensureConfig(); + expect(session.converterOrigin()).toBe(''); + }); + + it('says so when the converter is on this app own origin', async () => { + const session = create(`${SELF_ORIGIN}/converter/`); + await session.ensureConfig(); + + // getTools() returns same-origin descriptors too, and the Copilot filters + // them out — so this would discover nothing and explain nothing. + expect(session.isSameOrigin()).toBe(true); + expect(session.isAvailable()).toBe(false); + }); + + it('passes this origin along so the converter can expose its tools back', async () => { + const session = create(); + await session.ensureConfig(); + + expect(session.frameUrl()).toBe( + `${CONVERTER_URL}?actuo=${encodeURIComponent(SELF_ORIGIN)}`, + ); + }); + + it('appends the handshake to a URL that already has a query', async () => { + const session = create('https://cambiaro.example/?theme=dark'); + await session.ensureConfig(); + expect(session.frameUrl()).toContain('&actuo='); + }); + + it('asks the backend once however many surfaces mount', async () => { + const session = create(); + const releases = [session.acquire(), session.acquire(), session.acquire()]; + await settle(); + + expect(api.get).toHaveBeenCalledTimes(1); + releases.forEach((release) => release()); + }); + }); + + describe('one frame at a time', () => { + /** + * `getTools()` returns a descriptor per *window*, so two live frames on the + * same origin publish two tools called `convertCurrency` — and the Copilot + * would hand Gemini both. + */ + it('opening one surface closes any other', () => { + const session = create(); + + session.open('dashboard'); + expect(session.isOpen('dashboard')).toBe(true); + + session.open('expense:e1'); + expect(session.isOpen('dashboard')).toBe(false); + expect(session.isOpen('expense:e1')).toBe(true); + }); + + it('toggles a surface off without opening another', () => { + const session = create(); + + session.toggle('convert'); + expect(session.isOpen('convert')).toBe(true); + session.toggle('convert'); + expect(session.openedSurface()).toBeNull(); + }); + + it('closing a surface that is not open leaves the open one alone', () => { + const session = create(); + session.open('agent'); + session.close('dashboard'); + expect(session.isOpen('agent')).toBe(true); + }); + }); + + describe('discovery lifecycle', () => { + it('discovers for the first mount', async () => { + const session = create(); + const release = session.acquire(); + await settle(); + + expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([CONVERTER_ORIGIN]); + release(); + }); + + /** + * The bug this pins: during a route change Angular constructs the incoming + * component before destroying the outgoing one. A naive clear-on-destroy + * would wipe the tools the new surface had just discovered. + */ + it('keeps the tools while any mount remains', async () => { + const session = create(); + const first = session.acquire(); + const second = session.acquire(); + await settle(); + + first(); + expect(copilot.clearRemoteTools).not.toHaveBeenCalled(); + + second(); + expect(copilot.clearRemoteTools).toHaveBeenCalled(); + }); + + it('ignores a release called twice', async () => { + const session = create(); + const first = session.acquire(); + const second = session.acquire(); + await settle(); + + first(); + first(); + expect(copilot.clearRemoteTools).not.toHaveBeenCalled(); + second(); + expect(copilot.clearRemoteTools).toHaveBeenCalledTimes(1); + }); + + /** + * The converter registers asynchronously, after its own load and (being a + * separate app) after it boots — so the iframe's `load` can fire before + * there is anything to find. + */ + it('re-discovers when the browser reports a toolchange', async () => { + const session = create(); + const release = session.acquire(); + await settle(); + copilot.discoverRemoteTools.mockClear(); + + const listener = registry.onToolChange.mock.calls[0][0] as () => void; + listener(); + await settle(); + + expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([CONVERTER_ORIGIN]); + release(); + }); + + it('unsubscribes from toolchange once nothing is mounted', async () => { + const session = create(); + const release = session.acquire(); + await settle(); + + release(); + expect(unsubscribe).toHaveBeenCalled(); + }); + + it('subscribes to toolchange once, not once per mount', async () => { + const session = create(); + const releases = [session.acquire(), session.acquire()]; + await settle(); + + expect(registry.onToolChange).toHaveBeenCalledTimes(1); + releases.forEach((release) => release()); + }); + + it('does not discover when nothing is mounted', async () => { + const session = create(); + await session.ensureConfig(); + await session.rediscover(); + + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); + }); + + it('does not discover a same-origin converter', async () => { + const session = create(`${SELF_ORIGIN}/converter/`); + const release = session.acquire(); + await settle(); + + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); + release(); + }); + + it('leaves the surfaces working when the backend is down', async () => { + api.get.mockRejectedValue(new Error('offline')); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: ApiClient, useValue: api }, + { provide: Copilot, useValue: copilot }, + { provide: ToolRegistry, useValue: registry }, + ], + }); + const session = TestBed.inject(ConverterSession); + + await session.ensureConfig(); + expect(session.converterUrl()).toBe(''); + expect(session.isResolved()).toBe(true); + expect(session.isAvailable()).toBe(false); + }); + }); +}); diff --git a/frontend/src/app/converter/converter-session.ts b/frontend/src/app/converter/converter-session.ts new file mode 100644 index 0000000..fc53ef1 --- /dev/null +++ b/frontend/src/app/converter/converter-session.ts @@ -0,0 +1,238 @@ +import { Injectable, PLATFORM_ID, computed, inject, signal } from '@angular/core'; +import { isPlatformBrowser } from '@angular/common'; +import { ApiClient } from '../core/api/api-client.js'; +import { Copilot } from '../copilot/copilot.js'; +import { ToolRegistry } from '../webmcp/tool-registry.js'; + +/** The subset of `GET /api/config` this service needs. */ +interface ConverterConfig { + converterUrl: string; +} + +/** Schemes a framable converter can be served over. */ +const FRAMABLE_SCHEMES = new Set(['https:', 'http:']); + +/** + * Owns the embedded converter: where it lives, which surface may show it, and + * the cross-origin tool discovery that follows it around. + * + * Discovery used to belong to `/agent`, which was right while exactly one page + * framed exactly one other origin. The converter appears on four surfaces now, + * and page-owned discovery breaks the moment two overlap: navigating away + * from `/agent` called `clearRemoteTools()` and stripped the Copilot's + * converter tools while a converter was still mounted and visible elsewhere. + * + * Two responsibilities, both of which exist because a cross-origin tool lives + * exactly as long as the document that registered it: + * + * 1. **Only one converter frame at a time.** `getTools()` returns one + * descriptor per *window*, so two live frames on the same origin publish two + * tools called `convertCurrency`. `Copilot.toolDeclarations()` would hand + * Gemini both, and `runTool()` resolves a call with `.find()` — first wins, + * arbitrarily. Opening one surface therefore closes any other. + * 2. **Reference-counted discovery.** The first mount discovers, the last one + * to leave clears. The count is not always 0 or 1 even with rule 1: during a + * route change Angular constructs the incoming component before destroying + * the outgoing one, so a naive "clear on destroy" would wipe the tools the + * new surface just discovered. + * + * Modelled on {@link ToolSession}, which owns the state-gated `approve_expense` + * lifecycle for the same reason: "which tools are published right now" is + * session state, not component state. + */ +@Injectable({ providedIn: 'root' }) +export class ConverterSession { + private readonly api = inject(ApiClient); + private readonly copilot = inject(Copilot); + private readonly registry = inject(ToolRegistry); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + + /** `null` until `/api/config` answers; `''` there means none configured. */ + private readonly url = signal(null); + private readonly mounts = signal(0); + private readonly openSurface = signal(null); + private configLoaded: Promise | null = null; + private unsubscribeToolChange: (() => void) | null = null; + + /** The converter's base URL: `null` while unknown, `''` when unconfigured. */ + readonly converterUrl = this.url.asReadonly(); + + /** True once `/api/config` has answered, however it answered. */ + readonly isResolved = computed(() => this.url() !== null); + + /** + * The origin to hand `getTools({fromOrigins})`. + * + * Derived rather than configured separately: `CONVERTER_URL` may carry a path + * — a converter need not sit at the root of its host — and two variables that + * have to agree is one more than necessary. + */ + readonly converterOrigin = computed(() => { + const value = this.url(); + if (!value) return ''; + try { + const parsed = new URL(value); + // A misconfigured variable must not become a `javascript:` frame src. + if (!FRAMABLE_SCHEMES.has(parsed.protocol)) return ''; + return parsed.origin; + } catch { + // A malformed CONVERTER_URL must not take down the surfaces that read it. + return ''; + } + }); + + /** + * True when the converter is configured on this app's own origin. + * + * Worth saying out loud rather than showing a confident empty tool list: + * `getTools()` returns this document's own tools too, and `Copilot` + * deliberately filters same-origin descriptors out, so nothing would be + * discovered and nothing would explain why. + */ + readonly isSameOrigin = computed(() => { + const origin = this.converterOrigin(); + return origin !== '' && origin === this.selfOrigin(); + }); + + /** True once we know there is a framable converter on another origin. */ + readonly isAvailable = computed(() => this.converterOrigin() !== '' && !this.isSameOrigin()); + + /** + * The URL to frame, with this origin passed along so the converter can expose + * its tools back to us. + * + * The `?actuo=` handshake is what the converter reads to decide who may call + * its tools: a WebMCP tool is same-origin unless registration names an origin + * in `exposedTo`, and the embedded page cannot know ours without being told. + * Sending it at runtime rather than hardcoding it there means our hostname + * can change without a release on the other side. + */ + readonly frameUrl = computed(() => { + const base = this.url(); + if (!base || !this.isAvailable()) return null; + const separator = base.includes('?') ? '&' : '?'; + return `${base}${separator}actuo=${encodeURIComponent(this.selfOrigin())}`; + }); + + /** Which surface currently owns the single converter frame. */ + readonly openedSurface = this.openSurface.asReadonly(); + + /** Whether this particular surface is the one showing the converter. */ + isOpen(surface: string): boolean { + return this.openSurface() === surface; + } + + /** + * Show the converter on `surface`, closing it wherever else it was. + * + * A radio group rather than a set, for rule 1 above. + */ + open(surface: string): void { + this.openSurface.set(surface); + } + + /** Hide the converter if `surface` is the one showing it. */ + close(surface: string): void { + if (this.isOpen(surface)) this.openSurface.set(null); + } + + toggle(surface: string): void { + if (this.isOpen(surface)) this.close(surface); + else this.open(surface); + } + + /** + * Register a mounted converter frame. Returns a release function. + * + * Pairing acquire and release in one value is what stops a missed release + * from pinning the tool list open forever. + */ + acquire(): () => void { + this.mounts.update((n) => n + 1); + void this.ensureConfig().then(() => this.rediscover()); + + if (!this.unsubscribeToolChange) { + /* + * The converter registers its tools asynchronously, after its own load + * event and (being a React app) after hydration, so the iframe's `load` + * can fire before there is anything to find. `toolchange` is what says + * "now there is". + */ + this.unsubscribeToolChange = this.registry.onToolChange(() => void this.rediscover()); + } + + let released = false; + return () => { + if (released) return; + released = true; + this.mounts.update((n) => Math.max(0, n - 1)); + if (this.mounts() === 0) this.teardown(); + }; + } + + /** + * Re-run discovery. Safe to call at any time; a no-op when nothing is mounted + * or no converter is reachable. + * + * Called on the iframe's `load`, on `toolchange`, and by `/agent`'s + * Rediscover button. + */ + async rediscover(): Promise { + const origin = this.converterOrigin(); + if (!this.isBrowser || this.mounts() === 0 || !origin || this.isSameOrigin()) return; + await this.copilot.discoverRemoteTools([origin]); + } + + /** + * Load `/api/config` once per session. + * + * The promise is cached rather than the result, so several converters + * mounting in the same tick make one request between them, not several. + */ + ensureConfig(): Promise { + this.configLoaded ??= this.loadConfig(); + return this.configLoaded; + } + + private async loadConfig(): Promise { + // Never during prerender: the origin is a runtime value, and an iframe in + // the prerendered HTML would point somewhere the build cannot know. + if (!this.isBrowser) { + this.url.set(''); + return; + } + try { + const config = await this.api.get('/config'); + const value = typeof config?.converterUrl === 'string' ? config.converterUrl : ''; + this.url.set(value.replace(/\s+/g, '')); + } catch { + // No converter configured or the backend is down: the surfaces render + // their unavailable state rather than the page failing. Nothing else + // depends on this. + this.url.set(''); + } + } + + private teardown(): void { + this.unsubscribeToolChange?.(); + this.unsubscribeToolChange = null; + /* + * `openSurface` is deliberately NOT cleared here. It is the user's intent — + * "show me the converter on this surface" — while the mount count is a + * resource. Coupling them broke going offline: the frame is released, this + * ran, the surface closed, and the panel vanished instead of rendering the + * "live rates need a connection" state. Coming back online then left it + * closed, because nothing had asked for it any more. + */ + /* + * Without this the Copilot keeps offering `convertCurrency` to the model + * after the document implementing it is gone, and every call fails with a + * confusing error instead of the tool simply not being on the menu. + */ + this.copilot.clearRemoteTools(); + } + + private selfOrigin(): string { + return this.isBrowser ? (document.location?.origin ?? '') : ''; + } +} diff --git a/frontend/src/app/converter/currency-converter.spec.ts b/frontend/src/app/converter/currency-converter.spec.ts new file mode 100644 index 0000000..ada6b20 --- /dev/null +++ b/frontend/src/app/converter/currency-converter.spec.ts @@ -0,0 +1,244 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { signal } from '@angular/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { ApiClient } from '../core/api/api-client.js'; +import { PwaService } from '../core/pwa/pwa-service.js'; +import { Copilot } from '../copilot/copilot.js'; +import { ToolRegistry } from '../webmcp/tool-registry.js'; +import type { NormalizedTool } from '../webmcp/webmcp.types.js'; +import { ConverterSession } from './converter-session.js'; +import { CurrencyConverter } from './currency-converter.js'; + +const SELF_ORIGIN = globalThis.location.origin; +const CONVERTER_URL = 'https://cambiaro.example/'; + +describe('CurrencyConverter', () => { + let fixture: ComponentFixture; + let api: { get: ReturnType }; + let copilot: { + crossOriginTools: ReturnType>; + discoverRemoteTools: ReturnType; + clearRemoteTools: ReturnType; + }; + let registry: { + isSupported: ReturnType>; + canExecuteCrossOrigin: ReturnType>; + onToolChange: ReturnType; + }; + let offline: ReturnType>; + let session: ConverterSession; + + const host = () => fixture.nativeElement as HTMLElement; + const text = () => host().textContent ?? ''; + const iframe = () => host().querySelector('iframe'); + + async function create( + options: { converterUrl?: string | null; open?: boolean } = {}, + ): Promise { + const { converterUrl = CONVERTER_URL, open = true } = options; + + api.get.mockImplementation((path: string) => + path === '/config' + ? Promise.resolve(converterUrl === null ? {} : { converterUrl }) + : Promise.reject(new Error(`unexpected ${path}`)), + ); + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: ApiClient, useValue: api }, + { provide: Copilot, useValue: copilot }, + { provide: ToolRegistry, useValue: registry }, + { provide: PwaService, useValue: { isOffline: offline } }, + ], + }); + + session = TestBed.inject(ConverterSession); + fixture = TestBed.createComponent(CurrencyConverter); + fixture.componentRef.setInput('surface', 'test'); + if (open) session.open('test'); + + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + } + + beforeEach(() => { + api = { get: vi.fn() }; + copilot = { + crossOriginTools: signal([]), + discoverRemoteTools: vi.fn().mockResolvedValue(undefined), + clearRemoteTools: vi.fn(), + }; + registry = { + isSupported: signal(true), + canExecuteCrossOrigin: signal(true), + onToolChange: vi.fn(() => vi.fn()), + }; + offline = signal(false); + }); + + describe('mounting', () => { + /** + * The iframe is a whole separate app. Three of the four surfaces render + * this collapsed, and a closed converter must cost nothing — hence `@if` + * rather than hiding it with a class. + */ + it('renders nothing at all until its surface is opened', async () => { + await create({ open: false }); + + expect(iframe()).toBeNull(); + expect(text().trim()).toBe(''); + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); + }); + + it('frames the converter once opened', async () => { + await create(); + + const frame = iframe(); + expect(frame).not.toBeNull(); + expect(frame!.getAttribute('allow')).toBe('tools'); + expect(frame!.getAttribute('referrerpolicy')).toBe('no-referrer'); + expect(frame!.getAttribute('src')).toContain(CONVERTER_URL); + }); + + it('tells the converter which origin to expose its tools to', async () => { + await create(); + expect(iframe()!.getAttribute('src')).toContain( + `actuo=${encodeURIComponent(SELF_ORIGIN)}`, + ); + }); + + it('gives the frame an accessible name', async () => { + await create(); + fixture.componentRef.setInput('title', 'Currency converter for Kaffee Berlin'); + fixture.detectChanges(); + + expect(iframe()!.getAttribute('title')).toBe('Currency converter for Kaffee Berlin'); + }); + + it('claims discovery only while it is actually showing the frame', async () => { + await create({ open: false }); + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); + + session.open('test'); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(copilot.discoverRemoteTools).toHaveBeenCalledWith(['https://cambiaro.example']); + }); + + it('releases its claim when destroyed', async () => { + await create(); + fixture.destroy(); + + expect(copilot.clearRemoteTools).toHaveBeenCalled(); + }); + }); + + describe('degrading honestly', () => { + /** + * A cross-origin frame has no usable error: no `onerror`, and `load` fires + * for the browser's own error page. So this is gated on what we can know. + */ + it('does not frame anything while offline, and says why', async () => { + offline.set(true); + await create(); + + expect(iframe()).toBeNull(); + expect(text()).toContain('Live rates need a connection'); + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); + }); + + /** + * The transition, not just the initial state. Creating the component while + * already offline was covered; *going* offline was not, and it was broken: + * releasing the frame tore the session down, which cleared the open surface, + * so the panel vanished rather than explaining itself — and coming back + * online left it closed because nothing was asking for it any more. + */ + it('keeps the panel and explains itself when the connection drops', async () => { + await create(); + expect(iframe()).not.toBeNull(); + + offline.set(true); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(iframe()).toBeNull(); + expect(text()).toContain('Live rates need a connection'); + expect(session.isOpen('test')).toBe(true); + }); + + it('frames it again when the connection comes back', async () => { + await create(); + offline.set(true); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + expect(iframe()).toBeNull(); + + offline.set(false); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + + expect(iframe()).not.toBeNull(); + }); + + it('names the flag when this browser has no WebMCP, and still frames it', async () => { + registry.isSupported.set(false); + await create(); + + // The point of the fallback: the converter is fully usable by hand. + expect(iframe()).not.toBeNull(); + expect(text()).toContain('chrome://flags/#enable-webmcp-testing'); + }); + + it('says what is unset rather than showing an empty box', async () => { + await create({ converterUrl: null }); + + expect(iframe()).toBeNull(); + expect(text()).toContain('CONVERTER_URL'); + }); + + it('explains a same-origin converter instead of discovering nothing', async () => { + await create({ converterUrl: `${SELF_ORIGIN}/converter/` }); + + expect(iframe()).toBeNull(); + expect(text()).toContain('cross-origin'); + }); + }); + + describe('the advisory boundary', () => { + /** + * LOAD-BEARING. This component has no return channel by construction: no + * `output()`, no `postMessage` listener, nothing that reads a value back + * out of the frame. That absence is what stops a converted figure ever + * reaching `sumSpend()` or `converted_amount` — wiring one in would mean + * first inventing the channel, which is a visible, reviewable act. + * + * If this test fails because an output was added, the question to answer is + * not "how do I fix the test" but "what is that number going to be used + * for". See the class doc, and CLAUDE.md's "never add two currencies". + */ + it('exposes no output for a converted value', () => { + const def = (CurrencyConverter as unknown as { ɵcmp: { outputs: Record } }) + .ɵcmp; + expect(Object.keys(def.outputs)).toEqual([]); + }); + + it('takes only scalars in, never an expense', () => { + const def = (CurrencyConverter as unknown as { ɵcmp: { inputs: Record } }) + .ɵcmp; + expect(Object.keys(def.inputs).sort()).toEqual(['height', 'surface', 'title']); + }); + + it('says on screen that the rate changes no Actuo figure', async () => { + await create(); + expect(text()).toContain('do not change any Actuo figure'); + }); + }); +}); diff --git a/frontend/src/app/converter/currency-converter.ts b/frontend/src/app/converter/currency-converter.ts new file mode 100644 index 0000000..cad823c --- /dev/null +++ b/frontend/src/app/converter/currency-converter.ts @@ -0,0 +1,176 @@ +import { + ChangeDetectionStrategy, + Component, + DestroyRef, + computed, + effect, + inject, + input, +} from '@angular/core'; +import { DomSanitizer, type SafeResourceUrl } from '@angular/platform-browser'; +import { PwaService } from '../core/pwa/pwa-service.js'; +import { ToolRegistry } from '../webmcp/tool-registry.js'; +import { ConverterSession } from './converter-session.js'; + +/** + * The embedded currency converter. + * + * ## LOAD-BEARING: this component has no return channel, deliberately. + * + * It takes no `Expense`, emits no `output()`, listens for no `postMessage`, and + * never reads a value back out of the frame. Rates go in one direction — onto + * the screen, for a person to read. + * + * That is not an oversight, it is the enforcement. Actuo has no FX pass + * (PRD §6.5): `converted_amount` is null for every foreign row, so totals count + * base-currency rows only and *state what they left out* — `sumSpend()` returns + * `{total, excluded}`, and `excludedNotice()` says so in words. The earlier + * code added the raw `amount` instead, and a $200 charge was counted as ₹200 in + * a rupee total. + * + * A live converter sitting next to those figures is exactly what tempts someone + * to close that gap with the number on screen. Because no converted value ever + * enters Actuo's component tree, there is nothing to wire in: doing it would + * mean first inventing a return channel, which is a visible, reviewable act + * rather than a one-line slip. **Do not add one.** A rate a person read off a + * third-party site is not a locked historical rate, and only the backend + * writing `converted_amount` at write time may make a row count. + * + * ## Why an iframe and not an API call + * + * The converter is a separate origin running its own WebMCP tool surface. The + * frame is what makes those tools discoverable (PRD §7's cross-origin row): + * `getTools({fromOrigins})` returns nothing unless a document from that origin + * is alive, so the visible widget and the Copilot's ability to convert are the + * same fact. It is also why this must not be a hidden frame — the tools it + * exposes drive a UI, and driving a UI nobody can see is not a feature. + */ +@Component({ + selector: 'app-currency-converter', + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` + @if (session.isOpen(surface())) { + @if (offline()) { + +

+ Live rates need a connection. Actuo's own figures are all local, so the rest of this + page is unaffected. +

+ } @else if (session.isSameOrigin()) { +

+ The converter is configured on this app's own origin, so nothing it exposes is + cross-origin — getTools() returns its tools marked + same-origin and the Copilot filters them out. Point + CONVERTER_URL at a different host. +

+ } @else if (frameUrl(); as url) { +

+ Embedded from {{ session.converterOrigin() }} + with allow="tools". Rates are indicative, from the + European Central Bank — they do not change any Actuo figure. +

+ + + + @if (!registry.isSupported()) { +

+ Use it directly above. This browser has no WebMCP, so the Copilot cannot drive it — + that needs Chrome with + chrome://flags/#enable-webmcp-testing. +

+ } + } @else if (session.isResolved()) { +

+ No converter is configured. Set CONVERTER_URL to the + base URL of one — it has to be a different origin than this app, or the tools it + publishes come back same-origin and are filtered out. +

+ } + } + `, +}) +export class CurrencyConverter { + private readonly sanitizer = inject(DomSanitizer); + private readonly pwa = inject(PwaService); + private readonly destroyRef = inject(DestroyRef); + + protected readonly session = inject(ConverterSession); + protected readonly registry = inject(ToolRegistry); + + /** + * Stable id for this mount point — `'agent'`, `'convert'`, `'dashboard'`, or + * `expense:`. The session uses it to keep exactly one frame open. + */ + readonly surface = input.required(); + + /** Accessible name for the frame. Every surface should say where it is. */ + readonly title = input('Currency converter'); + + /** Taller on a page that is mostly this; compact in an inline panel. */ + readonly height = input<'compact' | 'full'>('compact'); + + protected readonly offline = this.pwa.isOffline; + protected readonly heightClass = computed(() => + this.height() === 'full' ? 'h-[32rem]' : 'h-80', + ); + + /** + * `[src]` on an iframe is a RESOURCE_URL context, so Angular refuses an + * interpolated string outright. Bypassing is the deliberate call here: the + * URL comes from our own `GET /api/config`, never from user input or anything + * an agent can influence, and `ConverterSession` has already rejected any + * scheme that is not http(s). If it ever becomes user-supplied, this must + * stop being a bypass. + */ + protected readonly frameUrl = computed(() => { + const url = this.session.frameUrl(); + return url ? this.sanitizer.bypassSecurityTrustResourceUrl(url) : null; + }); + + constructor() { + let release: (() => void) | null = null; + + /* + * Claim the session only while this surface is actually showing the frame. + * Mounting the component is not enough — three of the four surfaces render + * it collapsed, and a claim then would discover tools for a document that + * does not exist. + */ + effect(() => { + const shouldHold = this.session.isOpen(this.surface()) && !this.offline(); + if (shouldHold && !release) release = this.session.acquire(); + else if (!shouldHold && release) { + release(); + release = null; + } + }); + + // Resolve the URL even while collapsed, so a surface knows whether to offer + // the trigger at all rather than opening onto an explanation. + void this.session.ensureConfig(); + + this.destroyRef.onDestroy(() => release?.()); + } + + /** + * The frame's tools are registered after its own load — it is a separate app + * that has to boot first — so this is a first attempt, not the only one. + * `toolchange` is what catches the rest. + */ + protected onFrameLoad(): void { + void this.session.rediscover(); + } +} diff --git a/frontend/src/app/copilot/copilot.spec.ts b/frontend/src/app/copilot/copilot.spec.ts index 3989465..71686b7 100644 --- a/frontend/src/app/copilot/copilot.spec.ts +++ b/frontend/src/app/copilot/copilot.spec.ts @@ -206,7 +206,7 @@ describe('Copilot', () => { /** * Cross-origin tools live only as long as the document that registered them. - * Keeping them on the menu after the partner iframe is gone means the model + * Keeping them on the menu after the converter iframe is gone means the model * keeps calling a document that no longer exists, and every call fails with * a confusing error instead of the tool simply not being offered. */ @@ -227,11 +227,11 @@ describe('Copilot', () => { registerTool: vi.fn().mockResolvedValue(undefined), getTools: vi.fn().mockResolvedValue([ { - name: 'get_book_price', - title: 'Get book price', - description: 'Price of one book.', + name: 'convertCurrency', + title: 'Convert currency', + description: 'Convert an amount between two currencies.', inputSchema: { type: 'object', properties: {} }, - origin: 'https://pageturner.example', + origin: 'https://cambiaro.example', annotations: { readOnlyHint: true }, }, // Same-origin tools come back too and must be ignored: the @@ -255,19 +255,74 @@ describe('Copilot', () => { it('keeps only genuinely cross-origin tools', async () => { const copilot = setupWithRemote(); - await copilot.discoverRemoteTools(['https://pageturner.example']); + await copilot.discoverRemoteTools(['https://cambiaro.example']); - expect(copilot.crossOriginTools().map((t) => t.name)).toEqual(['get_book_price']); + expect(copilot.crossOriginTools().map((t) => t.name)).toEqual(['convertCurrency']); }); it('forgets them when asked', async () => { const copilot = setupWithRemote(); - await copilot.discoverRemoteTools(['https://pageturner.example']); + await copilot.discoverRemoteTools(['https://cambiaro.example']); copilot.clearRemoteTools(); expect(copilot.crossOriginTools()).toEqual([]); }); + + /** + * `getTools()` returns a descriptor per *window*, not per tool. The same + * page framed here and also open in another tab publishes the same tool + * twice, and nothing in the app can stop a user opening that tab. + * + * Left alone, `toolDeclarations()` hands Gemini two function declarations + * with the same name — which is a malformed request, not a redundant one — + * while `runTool()` resolves the call with `.find()` and picks whichever + * arrived first anyway. + */ + it('publishes one declaration per name when a page is open twice', async () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: GeminiClient, useValue: scriptedGemini() }, + { provide: KeyStore, useValue: { hasKey: () => true } }, + { provide: ApiClient, useValue: { get: vi.fn(), post: vi.fn() } }, + { + provide: DOCUMENT, + useValue: { + location: { origin: 'https://actuo.app' }, + modelContext: Object.assign(new EventTarget(), { + registerTool: vi.fn().mockResolvedValue(undefined), + getTools: vi.fn().mockResolvedValue([ + { + name: 'convertCurrency', + title: 'Convert currency', + description: 'From the framed window.', + inputSchema: { type: 'object', properties: {} }, + origin: 'https://cambiaro.example', + annotations: { readOnlyHint: true }, + }, + { + name: 'convertCurrency', + title: 'Convert currency', + description: 'From the same page in another tab.', + inputSchema: { type: 'object', properties: {} }, + origin: 'https://cambiaro.example', + annotations: { readOnlyHint: true }, + }, + ]), + }), + }, + }, + ], + }); + const copilot = TestBed.inject(Copilot); + await copilot.discoverRemoteTools(['https://cambiaro.example']); + + const tools = copilot.crossOriginTools(); + expect(tools).toHaveLength(1); + // First wins is fine; sending both is not. + expect(tools[0].description).toContain('framed window'); + }); }); it('clears the conversation on reset', async () => { diff --git a/frontend/src/app/copilot/copilot.ts b/frontend/src/app/copilot/copilot.ts index 203a942..ede46af 100644 --- a/frontend/src/app/copilot/copilot.ts +++ b/frontend/src/app/copilot/copilot.ts @@ -42,6 +42,8 @@ const SYSTEM_INSTRUCTION = [ 'When a tool fails, say specifically what went wrong and suggest the nearest fix', '(for example: "I couldn\'t find a category called \'Food\' — did you mean \'Dining\'?").', '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.', "Today's date is " + new Date().toISOString().slice(0, 10) + '.', ].join(' '); @@ -90,7 +92,7 @@ export class Copilot { } /** - * Pull in tools exposed by another origin (the partner-demo page). + * Pull in tools exposed by another origin (the embedded converter). * * Only genuinely cross-origin descriptors are kept: `getTools()` returns this * document's own tools too, and those are already in the registry with their @@ -100,16 +102,16 @@ export class Copilot { */ async discoverRemoteTools(origins: string[]): Promise { const tools = await this.registry.discover({ fromOrigins: origins }); - this.remoteTools.set(tools.filter((tool) => tool.isCrossOrigin)); + this.remoteTools.set(dedupeByName(tools.filter((tool) => tool.isCrossOrigin))); } /** * Forget the other origin's tools. * - * Called when the page hosting the partner iframe goes away. Without it the - * Copilot keeps offering `search_books` to the model after the document that - * implements it is gone, and every call fails with a confusing error instead - * of the tool simply not being on the menu. + * Called when the last surface framing the other origin goes away. Without it + * the Copilot keeps offering `convertCurrency` to the model after the document + * that implements it is gone, and every call fails with a confusing error + * instead of the tool simply not being on the menu. */ clearRemoteTools(): void { this.remoteTools.set([]); @@ -342,6 +344,27 @@ function format(value: unknown): string { return JSON.stringify(value); } +/** + * One descriptor per tool name. + * + * `getTools()` returns a descriptor per *window*, not per tool, so the same + * page open twice — framed here and also in another tab, which + * `ConverterSession` cannot prevent — publishes two tools called + * `convertCurrency`. Left alone, `toolDeclarations()` hands Gemini two function + * declarations with the same name, and `runTool()` resolves the call with + * `.find()` and picks whichever came back first. + * + * First wins is fine; sending both is not. They are the same tool with the same + * schema, so any of them can serve the call. + */ +function dedupeByName(tools: readonly NormalizedTool[]): readonly NormalizedTool[] { + const seen = new Map(); + for (const tool of tools) { + if (!seen.has(tool.name)) seen.set(tool.name, tool); + } + return [...seen.values()]; +} + function hostOf(origin: string): string { try { return new URL(origin).host; diff --git a/frontend/src/app/pages/agent/agent.spec.ts b/frontend/src/app/pages/agent/agent.spec.ts index 6535402..9070d0c 100644 --- a/frontend/src/app/pages/agent/agent.spec.ts +++ b/frontend/src/app/pages/agent/agent.spec.ts @@ -1,5 +1,6 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { signal } from '@angular/core'; +import { provideRouter } from '@angular/router'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ApiClient } from '../../core/api/api-client.js'; @@ -10,15 +11,17 @@ import type { ToolInvocation } from '../../webmcp/tool-registry.js'; import { Agent } from './agent.js'; const SELF_ORIGIN = globalThis.location.origin; -const PARTNER = 'http://localhost:4201'; +/** CONVERTER_URL may carry a path, so the origin is derived from it. */ +const CONVERTER_URL = 'https://cambiaro.example/'; +const CONVERTER_ORIGIN = 'https://cambiaro.example'; function remoteTool(overrides: Partial = {}): NormalizedTool { return { - name: 'get_book_price', - title: 'Get book price', - description: 'Return the price of one book by its id.', + name: 'convertCurrency', + title: 'Convert currency', + description: 'Convert an amount from one currency to another.', inputSchema: { type: 'object', properties: {} }, - origin: PARTNER, + origin: CONVERTER_ORIGIN, annotations: { readOnlyHint: true }, isCrossOrigin: true, raw: {} as NormalizedTool['raw'], @@ -59,17 +62,20 @@ describe('Agent tools page', () => { const text = () => host().textContent ?? ''; const iframe = () => host().querySelector('iframe'); - /** `null` means "the backend reported no partner origin", not "use the default". */ - async function create(partnerOrigin: string | null = PARTNER): Promise { + /** `null` means "the backend reported no converter", not "use the default". */ + async function create(converterUrl: string | null = CONVERTER_URL): Promise { api.get.mockImplementation((path: string) => path === '/config' - ? Promise.resolve(partnerOrigin === null ? {} : { partnerOrigin }) + ? Promise.resolve(converterUrl === null ? {} : { converterUrl }) : Promise.reject(new Error(`unexpected ${path}`)), ); TestBed.resetTestingModule(); TestBed.configureTestingModule({ providers: [ + // The page links to /convert, so RouterLink needs a router to resolve + // against. No routes are exercised here; the link just has to render. + provideRouter([]), { provide: ApiClient, useValue: api }, { provide: Copilot, useValue: copilot }, { provide: ToolRegistry, useValue: registry }, @@ -103,21 +109,22 @@ describe('Agent tools page', () => { * The dead feature this page revives: `discoverRemoteTools()` existed, * worked and was tested, and nothing in the app ever called it. */ - it('asks the configured partner origin for its tools', async () => { + it('asks the configured converter origin for its tools', async () => { await create(); - expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([PARTNER]); + expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([CONVERTER_ORIGIN]); }); - it('embeds the partner page with the tools permission the spec requires', async () => { + it('embeds the converter with the tools permission the spec requires', async () => { await create(); const frame = iframe(); expect(frame).not.toBeNull(); expect(frame!.getAttribute('allow')).toBe('tools'); - expect(frame!.getAttribute('src')).toContain(`${PARTNER}/partner-demo/`); + expect(frame!.getAttribute('referrerpolicy')).toBe('no-referrer'); + expect(frame!.getAttribute('src')).toContain(CONVERTER_URL); }); - it('tells the partner page which origin to expose its tools to', async () => { + it('tells the embedded page which origin to expose its tools to', async () => { await create(); expect(iframe()!.getAttribute('src')).toContain( `actuo=${encodeURIComponent(SELF_ORIGIN)}`, @@ -132,17 +139,17 @@ describe('Agent tools page', () => { listener(); await fixture.whenStable(); - // The partner page registers asynchronously after its own load, so the + // The converter registers asynchronously after its own load, so the // iframe's `load` event can fire before there is anything to find. - expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([PARTNER]); + expect(copilot.discoverRemoteTools).toHaveBeenCalledWith([CONVERTER_ORIGIN]); }); it('lists what it discovered, with the origin it came from', async () => { copilot.crossOriginTools.set([remoteTool()]); await create(); - expect(text()).toContain('get_book_price'); - expect(text()).toContain('localhost:4201'); + expect(text()).toContain('convertCurrency'); + expect(text()).toContain('cambiaro.example'); expect(text()).toContain('Read-only'); }); @@ -151,29 +158,38 @@ describe('Agent tools page', () => { * the Copilot filters every one of them out. Showing an empty list would * read as a bug; saying so is the honest failure. */ - it('explains itself instead of pretending, when the partner is same-origin', async () => { - await create(SELF_ORIGIN); + it('explains itself instead of pretending, when the converter is same-origin', async () => { + await create(`${SELF_ORIGIN}/converter/`); expect(iframe()).toBeNull(); - expect(text()).toContain('PARTNER_DEMO_ORIGIN'); + expect(text()).toContain('CONVERTER_URL'); expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); }); /** - * The deployed default: `PARTNER_DEMO_ORIGIN` is unset in production, on - * purpose, because a localhost fallback there would embed an iframe - * pointing at each visitor's own machine. An empty card would read as a - * bug, so the page has to say what is missing. + * The deployed default: `CONVERTER_URL` is unset in production, on purpose, + * because a localhost fallback there would embed an iframe pointing at each + * visitor's own machine. An empty card would read as a bug, so the page has + * to say what is missing. */ - it('explains itself when no partner origin is configured', async () => { + it('explains itself when no converter is configured', async () => { await create(null); expect(iframe()).toBeNull(); expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); - expect(text()).toContain('No second origin is configured'); - // Still points at what does exist, and at how to get the real demo. - expect(host().querySelector('a[href="/partner-demo/"]')).not.toBeNull(); - expect(text()).toContain('PARTNER_DEMO_ORIGIN'); + expect(text()).toContain('No converter is configured'); + expect(text()).toContain('CONVERTER_URL'); + }); + + /** + * A misconfigured variable must not become a `javascript:` frame src. The + * scheme check lives in ConverterSession, ahead of the sanitizer bypass. + */ + it('refuses a converter URL that is not http(s)', async () => { + await create('javascript:alert(1)'); + + expect(iframe()).toBeNull(); + expect(copilot.discoverRemoteTools).not.toHaveBeenCalled(); }); /** @@ -249,7 +265,7 @@ describe('Agent tools page', () => { it('marks a cross-origin call as one', async () => { registry.invocationLog.set([ - invocation({ toolName: 'get_book_price', origin: 'cross-origin' }), + invocation({ toolName: 'convertCurrency', origin: 'cross-origin' }), ]); await create(); diff --git a/frontend/src/app/pages/agent/agent.ts b/frontend/src/app/pages/agent/agent.ts index f2b7833..6409bfa 100644 --- a/frontend/src/app/pages/agent/agent.ts +++ b/frontend/src/app/pages/agent/agent.ts @@ -2,24 +2,19 @@ import { isPlatformBrowser } from '@angular/common'; import { ChangeDetectionStrategy, Component, - DestroyRef, + OnInit, PLATFORM_ID, computed, inject, - signal, } from '@angular/core'; -import { DomSanitizer, type SafeResourceUrl } from '@angular/platform-browser'; +import { RouterLink } from '@angular/router'; -import { ApiClient } from '../../core/api/api-client.js'; import { Badge, Button, Card, EmptyState } from '../../ui'; import { Copilot } from '../../copilot/copilot.js'; +import { CurrencyConverter } from '../../converter/currency-converter.js'; +import { ConverterSession } from '../../converter/converter-session.js'; import { ToolRegistry } from '../../webmcp/tool-registry.js'; -/** The subset of `GET /api/config` this screen needs. */ -interface AgentConfig { - partnerOrigin: string; -} - /** * Agent tools — the WebMCP surface, made visible (PRD §7). * @@ -27,11 +22,17 @@ interface AgentConfig { * so neither did anything in the running app: * * 1. **Cross-origin tool use.** `Copilot.discoverRemoteTools()` was never - * called, and the partner page was served from Actuo's own origin — so even - * if it had been, every descriptor would have come back same-origin and been - * filtered out. This screen embeds the partner page from its own origin - * (`PARTNER_DEMO_ORIGIN`, :4201 in dev) with `allow="tools"`, then asks - * `getTools({fromOrigins})` for what it exposes. + * called, and the page it would have queried was served from Actuo's own + * origin — so even if it had been, every descriptor would have come back + * same-origin and been filtered out. This screen embeds the currency + * converter from `CONVERTER_URL` with `allow="tools"`, then asks + * `getTools({fromOrigins})` for what it exposes. That is a separately built, + * independently deployed app, in development as well as on a deploy. + * + * The frame and the discovery lifecycle belong to `ConverterSession`, not to + * this page: the converter also appears on `/convert`, the dashboard and + * foreign-currency expense rows, and page-owned teardown would clear the + * Copilot's remote tools while a frame was still mounted somewhere else. * 2. **The manual debug panel.** `ToolRegistry.discoveredTools()` and * `invocationLog()` had no consumer. They are the two panels below, and * they are what makes "the agent did exactly this" inspectable rather than @@ -47,7 +48,7 @@ interface AgentConfig { @Component({ selector: 'app-agent', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [Badge, Button, Card, EmptyState], + imports: [Badge, Button, Card, CurrencyConverter, EmptyState, RouterLink], host: { class: 'block' }, template: `
@@ -116,9 +117,10 @@ interface AgentConfig {

Another site's tools

- Pageturner Books is an unrelated page that knows nothing about Actuo. It - registers its own tools and exposes them to this origin, and the Copilot picks - them up — the same widget, driving a site it was not built for. + The currency converter below is a separate app on its own origin that knows + nothing about Actuo. It registers its own tools, exposes them to this origin, + and the Copilot picks them up — the same widget, driving a site it was not + built for.

`, }) -export class Agent { - private readonly api = inject(ApiClient); - private readonly sanitizer = inject(DomSanitizer); - private readonly copilot = inject(Copilot); +export class Agent implements OnInit { private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + private readonly copilot = inject(Copilot); protected readonly registry = inject(ToolRegistry); - private readonly destroyRef = inject(DestroyRef); + protected readonly session = inject(ConverterSession); - /** `null` until `/api/config` answers; `''` there means no second origin. */ - protected readonly partnerOrigin = signal(null); + /** The surface id this page claims. See `ConverterSession.open`. */ + protected readonly surface = 'agent'; protected readonly remoteTools = this.copilot.crossOriginTools; protected readonly registeredNames = computed(() => this.registry.registeredNames().join(', ')); @@ -273,51 +242,25 @@ export class Agent { /** Newest first — the call you just made is the one you are looking for. */ protected readonly invocations = computed(() => [...this.registry.invocationLog()].reverse()); - protected readonly sameOrigin = computed(() => { - const origin = this.partnerOrigin(); - return origin !== null && origin === this.selfOrigin(); - }); + protected readonly converterHost = computed(() => hostOf(this.session.converterOrigin())); - protected readonly partnerHost = computed(() => hostOf(this.partnerOrigin() ?? '')); - - /** - * `[src]` on an iframe is a RESOURCE_URL context, so Angular refuses an - * interpolated string outright. Bypassing is the deliberate call here: the - * origin comes from our own `GET /api/config`, never from user input or - * anything an agent can influence, and the `?actuo=` value is this document's - * own origin. If that ever becomes user-supplied, this must stop being a - * bypass. - */ - protected readonly partnerUrl = computed(() => { - const origin = this.partnerOrigin(); - if (!origin || this.sameOrigin()) return null; - const url = `${origin}/partner-demo/?actuo=${encodeURIComponent(this.selfOrigin())}`; - return this.sanitizer.bypassSecurityTrustResourceUrl(url); - }); - - constructor() { + ngOnInit(): void { if (!this.isBrowser) return; - - void this.loadConfig(); - /* - * Re-discover on `toolchange`. The partner page registers its tools - * asynchronously after its own load, so the iframe's `load` event can fire - * before there is anything to find; this is what catches the second beat. + * This page's job is to make the cross-origin surface visible, so it opens + * the frame rather than offering a trigger. `ConverterSession` owns the + * discovery lifecycle from here — including the `toolchange` subscription + * and the teardown that used to live in this constructor. It has to, now + * that the converter also appears on the dashboard and on expense rows: + * page-owned teardown would clear the Copilot's tools while a frame was + * still mounted and visible somewhere else. */ - const stop = this.registry.onToolChange(() => void this.rediscover()); - this.destroyRef.onDestroy(() => { - stop(); - // The iframe is about to be torn down. Leaving its tools on the Copilot's - // menu means the model keeps trying to call a document that is gone. - this.copilot.clearRemoteTools(); - }); + this.session.open(this.surface); } - protected async rediscover(): Promise { - const origin = this.partnerOrigin(); - if (!origin || this.sameOrigin()) return; - await this.copilot.discoverRemoteTools([origin]); + /** The Rediscover button. */ + protected rediscover(): void { + void this.session.rediscover(); } protected hostOf(origin: string): string { @@ -337,28 +280,6 @@ export class Agent { return text.length > 160 ? `${text.slice(0, 159)}…` : text; } - private selfOrigin(): string { - return this.isBrowser ? globalThis.location.origin : ''; - } - - private async loadConfig(): Promise { - try { - const config = await this.api.get('/config'); - if (typeof config?.partnerOrigin === 'string' && config.partnerOrigin) { - this.partnerOrigin.set(config.partnerOrigin.replace(/\/+$/, '')); - /* - * Discover straight away rather than waiting for the iframe's `load` - * or a `toolchange`. The partner page may already be open in another - * tab, in which case its tools are exposed to this origin right now - * and neither of those events will ever fire. - */ - await this.rediscover(); - } - } catch { - // No partner origin configured or the backend is down: the section stays - // empty rather than the page failing. Nothing else here depends on it. - } - } } function hostOf(origin: string): string { diff --git a/frontend/src/app/pages/convert/convert.ts b/frontend/src/app/pages/convert/convert.ts new file mode 100644 index 0000000..4262538 --- /dev/null +++ b/frontend/src/app/pages/convert/convert.ts @@ -0,0 +1,78 @@ +import { ChangeDetectionStrategy, Component, OnInit, inject } from '@angular/core'; +import { RouterLink } from '@angular/router'; +import { Card } from '../../ui'; +import { CurrencyConverter } from '../../converter/currency-converter.js'; +import { ConverterSession } from '../../converter/converter-session.js'; + +/** The surface id this page claims. See `ConverterSession.open`. */ +const SURFACE = 'convert'; + +/** + * `/convert` — the converter as a page of its own. + * + * The other three placements are contextual: an expense row, the dashboard's + * excluded-rows notice, and `/agent`. This one is the plain answer to "I just + * want to convert something", and the only surface that opens the frame on + * arrival, since it is the entire reason to be here. + * + * Deliberately **not** a seventh nav tab. The bottom bar is measured tight at + * six on a 390px phone (Progress.md), and `/showcase` is the existing precedent + * for a route reachable without one. It is linked from the places the question + * actually comes up. + */ +@Component({ + selector: 'app-convert', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [Card, CurrencyConverter, RouterLink], + template: ` +
+
+

Currency converter

+

+ Live and historical European Central Bank rates, from a separate app embedded here. + It is a reference: nothing you do on this page changes an Actuo figure. +

+
+ + + + + + +
+

Why totals still exclude it

+
+

+ Actuo stores every expense in the currency it was filed in, and converts one to your + base currency only when a rate was locked at the time it was recorded. A rate looked + up today is not that rate, so a conversion here is never folded into a total — the + dashboard and budgets say how many rows they left out instead of quietly adding + dollars to rupees. +

+

+ The converter is also a live WebMCP surface. With the Copilot open you can ask it to + convert, and it drives this same embedded app — + see what it exposes. +

+ +
+ `, +}) +export class Convert implements OnInit { + private readonly session = inject(ConverterSession); + + protected readonly surface = SURFACE; + + ngOnInit(): void { + // The page's whole purpose, so it opens rather than offering a trigger. + this.session.open(SURFACE); + } +} diff --git a/frontend/src/app/pages/dashboard/dashboard.spec.ts b/frontend/src/app/pages/dashboard/dashboard.spec.ts index 72c6a29..96bd6b5 100644 --- a/frontend/src/app/pages/dashboard/dashboard.spec.ts +++ b/frontend/src/app/pages/dashboard/dashboard.spec.ts @@ -84,11 +84,19 @@ describe('Dashboard', () => { const findAll = (selector: string) => Array.from(host().querySelectorAll(selector)); /** Routes each parallel call by path, so ordering inside Promise.all cannot matter. */ - function respond(overrides: { expenses?: Expense[]; budgets?: BudgetStatus[] } = {}) { + function respond( + overrides: { + expenses?: Expense[]; + budgets?: BudgetStatus[]; + /** Absent means no converter is configured, which is the deployed default. */ + converterUrl?: string; + } = {}, + ) { return (path: string) => { if (path === '/expenses') return Promise.resolve(page(overrides.expenses ?? EXPENSES)); if (path === '/budgets/status') return Promise.resolve(overrides.budgets ?? BUDGETS); if (path === '/orgs/current') return Promise.resolve(ORG); + if (path === '/config') return Promise.resolve({ converterUrl: overrides.converterUrl ?? '' }); return Promise.reject(new Error(`unexpected path ${path}`)); }; } @@ -256,6 +264,65 @@ describe('Dashboard', () => { expect(text()).not.toContain('in other currencies'); }); + /** + * The trigger sits beside the exclusion notice because that is where the + * question forms — but a converter cannot make an excluded row count, and + * "Convert currencies" next to "2 expenses aren't included" would read as + * "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 () => { + api.get.mockImplementation( + respond({ expenses: MIXED, converterUrl: 'https://cambiaro.example/' }), + ); + await create(); + await settle(); + await settle(); + + expect(text()).toContain("this won't change the total"); + }); + + it('is not offered when no converter is configured', async () => { + api.get.mockImplementation(respond({ expenses: MIXED })); + await create(); + await settle(); + + expect(text()).not.toContain("this won't change the total"); + // The notice itself is unaffected either way. + expect(text()).toContain('2 expenses in other currencies'); + }); + + /** + * LOAD-BEARING. Opening the lookup must not move a figure on this page. + * See CLAUDE.md, "Money: never add two currencies". + */ + it('leaves the total and the notice exactly as they were', async () => { + api.get.mockImplementation( + respond({ expenses: MIXED, converterUrl: 'https://cambiaro.example/' }), + ); + 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(tileOf()).toBe(before); + expect(text()).toContain('2 expenses in other currencies'); + }); + }); + it('prints an unconverted row under its own currency in the activity feed', async () => { api.get.mockImplementation(respond({ expenses: MIXED })); await create(); diff --git a/frontend/src/app/pages/dashboard/dashboard.ts b/frontend/src/app/pages/dashboard/dashboard.ts index 2a5a25f..97bd687 100644 --- a/frontend/src/app/pages/dashboard/dashboard.ts +++ b/frontend/src/app/pages/dashboard/dashboard.ts @@ -12,6 +12,8 @@ import type { BudgetStatus, Expense, Organization, Page } from '@actuo/shared'; import { ApiClient } from '../../core/api/api-client.js'; import { Card, EmptyState, ErrorState, Skeleton, StatCard } from '../../ui'; +import { CurrencyConverter } from '../../converter/currency-converter.js'; +import { ConverterSession } from '../../converter/converter-session.js'; import { formatDay, formatMoney } from '../../core/format/money.js'; import { excludedNotice, expenseCurrency } from '../../core/expense/amount.js'; import { @@ -70,7 +72,7 @@ const PACE_LABEL: Record = { @Component({ selector: 'app-dashboard', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [Card, EmptyState, ErrorState, Skeleton, StatCard], + imports: [Card, CurrencyConverter, EmptyState, ErrorState, Skeleton, StatCard], host: { class: 'block' }, template: `
@@ -154,6 +156,35 @@ const PACE_LABEL: Record = { --> @if (excludedNote(); as note) {

{{ note }}

+ + + @if (converter.isAvailable()) { + + +
+ +
+ } }
@@ -309,6 +340,24 @@ export class Dashboard { * 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. */ + protected readonly converter = inject(ConverterSession); + + /** This mount point's id. See `ConverterSession.open`. */ + protected readonly converterSurface = '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. + */ + void this.converter.ensureConfig(); + } + + 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 2480c13..32a054a 100644 --- a/frontend/src/app/pages/expenses/expenses.spec.ts +++ b/frontend/src/app/pages/expenses/expenses.spec.ts @@ -193,6 +193,84 @@ describe('Expenses', () => { expect(text()).not.toContain('₹'); }); + /** + * The rate lookup is offered on exactly the rows the totals leave out — + * `isConverted()` is the same predicate `sumSpend()` excludes on — so the + * trigger appears where the question forms and nowhere else. + */ + describe('the rate lookup', () => { + function withConverter(rows: Expense[]): void { + api.get.mockImplementation((path: string) => + path === '/config' + ? Promise.resolve({ converterUrl: 'https://cambiaro.example/' }) + : Promise.resolve(page(rows)), + ); + create(); + } + + it('is offered on a row in another currency', async () => { + withConverter([ + expense({ id: 'usd', merchant: 'AWS', amount: 50, currency: 'USD', convertedAmount: null }), + ]); + await settle(); + await settle(); + + expect(text()).toContain('What is this in INR?'); + }); + + it('is not offered on a base-currency row', async () => { + withConverter([expense({ id: 'inr', merchant: 'Barista', amount: 250 })]); + await settle(); + await settle(); + + expect(text()).not.toContain('What is this in'); + }); + + it('is not offered on a row that already has a converted value', async () => { + withConverter([ + expense({ + id: 'usd', + merchant: 'AWS', + amount: 50, + currency: 'USD', + convertedAmount: 4200, + }), + ]); + await settle(); + await settle(); + + expect(text()).not.toContain('What is this in'); + }); + + /** + * LOAD-BEARING. Opening the lookup must not change what the row says it + * cost. The converter is advisory: a rate read off another site is not + * the locked historical rate `converted_amount` would hold, so the money + * on screen is the money that was filed. See CLAUDE.md, "Money: never add + * two currencies". + */ + it('does not change the amount on the row it is opened from', async () => { + withConverter([ + expense({ id: 'usd', merchant: 'AWS', amount: 50, currency: 'USD', convertedAmount: null }), + ]); + await settle(); + await settle(); + + const before = find('[data-money]')?.textContent?.trim(); + expect(before).toContain('$'); + + const trigger = findAll('button').find((b) => + (b.textContent ?? '').includes('What is this in'), + ) as HTMLButtonElement | undefined; + expect(trigger).toBeDefined(); + trigger!.click(); + await settle(); + + expect(find('[data-money]')?.textContent?.trim()).toBe(before); + expect(text()).not.toContain('₹'); + }); + }); + it('labels a converted row with the org base currency', async () => { api.get.mockResolvedValue( page([ @@ -382,6 +460,25 @@ describe('Expenses paging', () => { fixture.detectChanges(); }; + /** + * Queue page responses by PATH, not by call order. + * + * `mockResolvedValueOnce` is consumed by whichever request happens to fire + * first, and this screen also asks `/api/config` (to decide whether to offer + * the per-row rate lookup). Routing by path is what keeps these assertions + * about paging rather than about request ordering — the dashboard spec makes + * the same move, for the same reason. + */ + function queuePages(...responses: unknown[]): void { + const queue = [...responses]; + api.get.mockImplementation((path: string) => { + if (path === '/config') return Promise.resolve({ converterUrl: '' }); + const next = queue.shift(); + if (next === undefined) return Promise.reject(new Error('no queued page')); + return next instanceof Error ? Promise.reject(next) : Promise.resolve(next); + }); + } + beforeEach(() => { api = { get: vi.fn() }; }); @@ -405,15 +502,19 @@ describe('Expenses paging', () => { it('requests the next page by offset and appends the rows', async () => { const more: Expense[] = [{ ...ROWS[0], id: 'later-1', merchant: 'Later Cafe' }]; - api.get.mockResolvedValueOnce(page(ROWS, ROWS.length + 1)); - api.get.mockResolvedValueOnce({ items: more, total: ROWS.length + 1, limit: 100, offset: ROWS.length }); + queuePages(page(ROWS, ROWS.length + 1), { + items: more, + total: ROWS.length + 1, + limit: 100, + offset: ROWS.length, + }); create(); await settle(); loadMore()!.click(); await settle(); - expect(api.get).toHaveBeenLastCalledWith('/expenses/search', { + expect(api.get).toHaveBeenCalledWith('/expenses/search', { limit: 100, offset: ROWS.length, }); @@ -424,8 +525,12 @@ describe('Expenses paging', () => { it('hides Load more once every row is in', async () => { const more: Expense[] = [{ ...ROWS[0], id: 'later-1', merchant: 'Later Cafe' }]; - api.get.mockResolvedValueOnce(page(ROWS, ROWS.length + 1)); - api.get.mockResolvedValueOnce({ items: more, total: ROWS.length + 1, limit: 100, offset: ROWS.length }); + queuePages(page(ROWS, ROWS.length + 1), { + items: more, + total: ROWS.length + 1, + limit: 100, + offset: ROWS.length, + }); create(); await settle(); @@ -440,8 +545,7 @@ describe('Expenses paging', () => { * a worse outcome than not paging at all. */ it('keeps the loaded rows when a later page fails, and says so', async () => { - api.get.mockResolvedValueOnce(page(ROWS, 250)); - api.get.mockRejectedValueOnce(new Error('Network unavailable')); + queuePages(page(ROWS, 250), new Error('Network unavailable')); create(); await settle(); @@ -460,7 +564,9 @@ describe('Expenses paging', () => { loadMore()!.click(); await settle(); - for (const [, params] of api.get.mock.calls) { + const pageCalls = api.get.mock.calls.filter(([path]) => path !== '/config'); + expect(pageCalls.length).toBeGreaterThan(0); + for (const [, params] of pageCalls) { expect((params as { limit: number }).limit).toBeLessThanOrEqual(EXPENSE_PAGE_MAX); } }); diff --git a/frontend/src/app/pages/expenses/expenses.ts b/frontend/src/app/pages/expenses/expenses.ts index ed243fd..bf135dc 100644 --- a/frontend/src/app/pages/expenses/expenses.ts +++ b/frontend/src/app/pages/expenses/expenses.ts @@ -14,7 +14,9 @@ import type { Expense, Page, TransitionAction } from '@actuo/shared'; import { ApiClient, ApiError } from '../../core/api/api-client.js'; import { Badge, Button, EmptyState, ErrorState, Input, Skeleton } from '../../ui'; import { formatDate, formatMoney } from '../../core/format/money.js'; -import { expenseAmount, expenseCurrency } from '../../core/expense/amount.js'; +import { expenseAmount, expenseCurrency, isConverted } from '../../core/expense/amount.js'; +import { CurrencyConverter } from '../../converter/currency-converter.js'; +import { ConverterSession } from '../../converter/converter-session.js'; import { Session } from '../../core/session/session.js'; import { ACTION_LABEL, @@ -60,7 +62,7 @@ const PAGE_SIZE = EXPENSE_PAGE_MAX; @Component({ selector: 'app-expenses', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [Badge, Button, EmptyState, ErrorState, Input, Skeleton], + imports: [Badge, Button, CurrencyConverter, EmptyState, ErrorState, Input, Skeleton], host: { class: 'block' }, template: `
@@ -279,6 +281,23 @@ const PAGE_SIZE = EXPENSE_PAGE_MAX; @if (errorFor(row.id); as message) {

{{ message }}

} + + @if (canConvert(row)) { +
+ +
+ } } @@ -384,11 +403,54 @@ const PAGE_SIZE = EXPENSE_PAGE_MAX; @if (errorFor(row.id); as message) {

{{ message }}

} + + @if (canConvert(row)) { +
+ +
+ } } } + + @if (openConverterFor(); as openRow) { +
+

+ Rate lookup for + {{ openRow.merchant || 'this expense' }} — + {{ amountText(openRow) }} filed in {{ openRow.currency }}. This does not change what + the expense is recorded as. +

+ +
+ } + +

+ Expenses filed in another currency are not converted into this one. + Look up a rate + to see what one is worth today. +

+ @if (roleCopy(); as copy) {

diff --git a/frontend/src/app/ui/badge.spec.ts b/frontend/src/app/ui/badge.spec.ts index f4350e5..9a85c99 100644 --- a/frontend/src/app/ui/badge.spec.ts +++ b/frontend/src/app/ui/badge.spec.ts @@ -66,11 +66,11 @@ describe('Badge', () => { it('falls back to the explicit tone when no status is given', () => { fixture.componentRef.setInput('tone', 'info'); - fixture.componentRef.setInput('label', 'via partner-demo.app'); + fixture.componentRef.setInput('label', 'via cambiaro.programmersingh.dev'); fixture.detectChanges(); expect(fixture.componentInstance.tone()).toBe('info'); - expect(pill().textContent?.trim()).toBe('via partner-demo.app'); + expect(pill().textContent?.trim()).toBe('via cambiaro.programmersingh.dev'); }); it('lets an explicit label override the status label', () => { diff --git a/frontend/src/app/ui/badge.ts b/frontend/src/app/ui/badge.ts index 8b9f730..cbc9823 100644 --- a/frontend/src/app/ui/badge.ts +++ b/frontend/src/app/ui/badge.ts @@ -39,7 +39,7 @@ const STATUS_LABEL: Record = { * * ```html * - * + * * ``` * * The colour is carried by a dot *and* the text, and the text is always present, diff --git a/frontend/src/app/ui/showcase/showcase.ts b/frontend/src/app/ui/showcase/showcase.ts index f5571b3..48c3a9c 100644 --- a/frontend/src/app/ui/showcase/showcase.ts +++ b/frontend/src/app/ui/showcase/showcase.ts @@ -108,7 +108,7 @@ import { StatCard } from '../stat-card'; }

- +
diff --git a/frontend/src/app/ui/tool-call-card.spec.ts b/frontend/src/app/ui/tool-call-card.spec.ts index 0426585..276069c 100644 --- a/frontend/src/app/ui/tool-call-card.spec.ts +++ b/frontend/src/app/ui/tool-call-card.spec.ts @@ -74,8 +74,8 @@ describe('ToolCallCard', () => { create(); expect(text()).not.toContain('via '); - create({ origin: 'partner-demo.app' }); - expect(text()).toContain('via partner-demo.app'); + create({ origin: 'cambiaro.programmersingh.dev' }); + expect(text()).toContain('via cambiaro.programmersingh.dev'); }); // §3.2.4 — a mutating call is never executed silently. diff --git a/frontend/src/app/webmcp/tool-registry.spec.ts b/frontend/src/app/webmcp/tool-registry.spec.ts index 98ccae0..1cd8844 100644 --- a/frontend/src/app/webmcp/tool-registry.spec.ts +++ b/frontend/src/app/webmcp/tool-registry.spec.ts @@ -231,14 +231,14 @@ describe('ToolRegistry', () => { title: '', description: 'List books.', inputSchema: JSON.stringify({ type: 'object' }), - origin: 'https://partner-demo.app', + origin: 'https://cambiaro.programmersingh.dev', window: globalThis.window, }, ]), }); const registry = configure(context); - const tools = await registry.discover({ fromOrigins: ['https://partner-demo.app'] }); + const tools = await registry.discover({ fromOrigins: ['https://cambiaro.programmersingh.dev'] }); expect(tools).toHaveLength(1); expect(tools[0].isCrossOrigin).toBe(true); diff --git a/frontend/src/app/webmcp/webmcp.types.spec.ts b/frontend/src/app/webmcp/webmcp.types.spec.ts index 7c8a6e1..aee77e1 100644 --- a/frontend/src/app/webmcp/webmcp.types.spec.ts +++ b/frontend/src/app/webmcp/webmcp.types.spec.ts @@ -54,7 +54,7 @@ describe('normalizeRegisteredTool', () => { expect(sameOrigin.isCrossOrigin).toBe(false); const crossOrigin = normalizeRegisteredTool( - descriptor({ origin: 'https://partner-demo.app' }), + descriptor({ origin: 'https://cambiaro.programmersingh.dev' }), 'https://actuo.app', ); expect(crossOrigin.isCrossOrigin).toBe(true); diff --git a/frontend/src/server.ts b/frontend/src/server.ts index bc20b89..d131f96 100644 --- a/frontend/src/server.ts +++ b/frontend/src/server.ts @@ -27,29 +27,6 @@ const angularApp = new AngularNodeAppEngine(); * requires, and the source of the handler `server.mjs` mounts. */ -/** - * The WebMCP partner demo — a static sub-site, not an Angular route. - * - * It needs its own mount because the general handler below sets `index: false`, - * deliberately: letting `express.static` answer a directory request would have - * it serve `index.html` for `/` and pre-empt server-side rendering of the - * landing page. Without this mount `/partner-demo/` matches no Angular route, - * falls to the router's `**` redirect and 302s to `/` — which is what it did - * until this was added, silently, since only `/partner-demo/index.html` worked. - * - * `no-store` for the same reason `scripts/partner-server.mjs` uses it: the page - * registers its tools against the `?actuo=` origin on every load, so a cached - * copy would keep re-registering against a stale one. - */ -app.use( - '/partner-demo', - express.static(join(browserDistFolder, 'partner-demo'), { - index: 'index.html', - redirect: true, - setHeaders: (res) => res.setHeader('Cache-Control', 'no-store'), - }), -); - /** * Serve static files from /browser */ diff --git a/package.json b/package.json index fd17976..ad08edf 100644 --- a/package.json +++ b/package.json @@ -8,13 +8,13 @@ "pnpm": ">=9.0.0 <10.0.0" }, "scripts": { - "dev": "pnpm run build:shared && concurrently -n backend,frontend,partner -c blue,magenta,yellow \"pnpm run dev:backend\" \"pnpm run dev:frontend\" \"pnpm run dev:partner\"", + "dev": "pnpm run build:shared && concurrently -n backend,frontend -c blue,magenta \"pnpm run dev:backend\" \"pnpm run dev:frontend\"", "dev:backend": "pnpm --filter backend run start:dev", "dev:frontend": "pnpm --filter frontend run start", - "dev:partner": "node scripts/partner-server.mjs", "build:shared": "pnpm --filter @actuo/shared run build", "build": "pnpm run build:shared && pnpm --filter backend run build && pnpm --filter frontend run build && pnpm run build:seo", "build:seo": "node scripts/stamp-seo.mjs", + "verify:deploy": "node scripts/verify-deploy.mjs", "start": "node server.mjs", "test": "pnpm --filter @actuo/shared run test && pnpm --filter backend run test && pnpm --filter frontend run test", "test:e2e": "pnpm --filter backend run test:e2e" diff --git a/render.yaml b/render.yaml index a4a87ee..6fcc98a 100644 --- a/render.yaml +++ b/render.yaml @@ -77,9 +77,21 @@ services: - key: JWT_REFRESH_SECRET sync: false - # PARTNER_DEMO_ORIGIN is deliberately absent. It must be an origin that is - # NOT this app's, or the cross-origin WebMCP demo proves nothing — /agent - # explains that when it is unset, rather than rendering an empty card. + # The embedded currency converter (PRD §6.5). A full URL, not a bare + # origin, because a converter need not sit at the root of its host — a + # GitHub Pages project site is `.github.io//` — and the + # `?actuo=` handshake is appended to it. Consumers derive the origin with + # `new URL(...).origin` for `getTools({fromOrigins})`. + # + # It must NOT be this app's own origin: `getTools({fromOrigins})` would + # return same-origin tools, which the Copilot filters out, and the + # cross-origin path would prove nothing. Cambiaro is a genuinely separate + # deployment, which is the point. + # + # RUNTIME, unlike PUBLIC_ORIGIN above: nothing about it is prerendered, + # so changing it needs a restart rather than a rebuild. + - key: CONVERTER_URL + value: https://cambiaro.programmersingh.dev/ # PORT is injected by Render and must not be set here. server.mjs reads it and # falls back to 8080, and binds 0.0.0.0 so the container is reachable. diff --git a/scripts/partner-server.mjs b/scripts/partner-server.mjs deleted file mode 100644 index 0f0bdab..0000000 --- a/scripts/partner-server.mjs +++ /dev/null @@ -1,68 +0,0 @@ -/** - * Serves the WebMCP partner-demo page on its own origin. - * - * PRD §7's cross-origin row needs two origins, not two paths. The page lives in - * `frontend/public/partner-demo/`, so in dev it is *also* reachable at - * `localhost:4200/partner-demo/` — but from there it is same-origin, and - * `normalizeRegisteredTool()` marks its tools `isCrossOrigin: false`, which is - * exactly the set the Copilot filters out. Nothing about the demo would be - * cross-origin. - * - * So this serves `frontend/public` on :4201, which puts the page at - * `/partner-demo/` there just as it is on the app's own origin — one URL shape - * in dev and in production, so `PARTNER_DEMO_ORIGIN` is the only thing that - * changes between them. Deliberately dependency-free - * (`node:http` + `node:fs`): a static file server is not worth a package, and - * pnpm blocks lifecycle scripts by default, so every added dependency is a new - * way for `pnpm run dev` to fail on a fresh clone. - * - * node scripts/partner-server.mjs # :4201 - * PORT=5001 node scripts/partner-server.mjs - */ - -import { createServer } from 'node:http'; -import { readFile } from 'node:fs/promises'; -import { extname, join, normalize, resolve, sep } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const ROOT = resolve(fileURLToPath(new URL('..', import.meta.url)), 'frontend/public'); -const PORT = Number(process.env.PORT ?? 4201); - -const CONTENT_TYPES = { - '.html': 'text/html; charset=utf-8', - '.js': 'text/javascript; charset=utf-8', - '.css': 'text/css; charset=utf-8', - '.json': 'application/json; charset=utf-8', - '.svg': 'image/svg+xml', - '.ico': 'image/x-icon', -}; - -const server = createServer(async (req, res) => { - const { pathname } = new URL(req.url ?? '/', `http://localhost:${PORT}`); - - // Resolve inside ROOT and verify it stayed there: `..` in a URL path is a - // directory traversal, and this process can read the whole repo. - const requested = pathname.endsWith('/') ? `${pathname}index.html` : pathname; - const target = join(ROOT, normalize(decodeURIComponent(requested))); - if (target !== ROOT && !target.startsWith(ROOT + sep)) { - res.writeHead(403, { 'content-type': 'text/plain' }).end('Forbidden'); - return; - } - - try { - const body = await readFile(target); - res.writeHead(200, { - 'content-type': CONTENT_TYPES[extname(target)] ?? 'application/octet-stream', - // The page registers tools scoped to Actuo's origin on every load, so a - // cached copy would keep re-registering against a stale `?actuo=` value. - 'cache-control': 'no-store', - }); - res.end(body); - } catch { - res.writeHead(404, { 'content-type': 'text/plain' }).end('Not found'); - } -}); - -server.listen(PORT, () => { - console.log(`Partner demo (WebMCP cross-origin) on http://localhost:${PORT}/partner-demo/`); -}); diff --git a/scripts/stamp-seo.mjs b/scripts/stamp-seo.mjs index ab90a08..309bd84 100644 --- a/scripts/stamp-seo.mjs +++ b/scripts/stamp-seo.mjs @@ -12,6 +12,16 @@ * survives prerendering into every generated HTML file, so one pass covers all * of them rather than each page needing its own handling. * + * **It has to be the whole `dist/frontend` tree, not just `browser/`.** This + * walked only `browser/` once, and the deployed site shipped a literal + * `__PUBLIC_ORIGIN__` in its `canonical` and `og:image`: Angular keeps its own + * copies of the page HTML under `server/` — `index.server.html` and the + * `assets-chunks/*.mjs` templates, `index_csr_html.mjs` among them — and those + * are what the SSR handler serves. `sitemap.xml` and `robots.txt` looked right + * throughout, because they are served from `browser/`, which is exactly what + * made it hard to see. A crawler reading a malformed URL is worse than one + * reading a relative URL, so this must not narrow again. + * * With PUBLIC_ORIGIN unset it substitutes the empty string, leaving every URL * root-relative and still valid — a local build is never broken by an * unconfigured domain, it just loses the absolute forms. @@ -25,10 +35,16 @@ import { extname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; const SENTINEL = '__PUBLIC_ORIGIN__'; -const STAMPABLE = new Set(['.html', '.xml', '.txt', '.webmanifest', '.json']); +/** + * `.mjs` is here for `server/assets-chunks/*.mjs` — Angular inlines each page's + * HTML into a JS module, sentinel and all. Stamping a string constant inside a + * generated module is safe: the sentinel appears nowhere else in the output. + */ +const STAMPABLE = new Set(['.html', '.xml', '.txt', '.webmanifest', '.json', '.mjs']); const ROOT = resolve(fileURLToPath(new URL('..', import.meta.url))); -const BROWSER_DIST = join(ROOT, 'frontend/dist/frontend/browser'); +/** The whole build output: `browser/` is served statically, `server/` is rendered from. */ +const DIST = join(ROOT, 'frontend/dist/frontend'); /** Trailing slashes would double up against the leading slash of every path. */ const origin = (process.env.PUBLIC_ORIGIN ?? '').trim().replace(/\/+$/, ''); @@ -41,14 +57,14 @@ async function* walk(dir) { } } -if (!existsSync(BROWSER_DIST)) { - console.error(`[seo] ${BROWSER_DIST} is missing. Run \`pnpm run build\` first.`); +if (!existsSync(DIST)) { + console.error(`[seo] ${DIST} is missing. Run \`pnpm run build\` first.`); process.exit(1); } let files = 0; let occurrences = 0; -for await (const path of walk(BROWSER_DIST)) { +for await (const path of walk(DIST)) { const before = await readFile(path, 'utf8'); if (!before.includes(SENTINEL)) continue; occurrences += before.split(SENTINEL).length - 1; diff --git a/scripts/verify-deploy.mjs b/scripts/verify-deploy.mjs new file mode 100644 index 0000000..cf35d61 --- /dev/null +++ b/scripts/verify-deploy.mjs @@ -0,0 +1,131 @@ +/** + * Smoke-checks a running deploy. + * + * Every check here exists because the thing it checks **failed silently in + * production** and nobody noticed until someone opened the site by hand: + * + * - SSR fell back to client-side rendering. Angular does not error when the + * `Host` header is off its allowlist — it quietly renders on the client and + * throws away the SSR and structured-data work. The page still looks fine. + * - `__PUBLIC_ORIGIN__` shipped literally in `canonical` and `og:image`, + * because the SEO stamp only walked `browser/` while the SSR handler serves + * HTML from `server/`. A crawler read a malformed URL, which is worse than a + * relative one. `sitemap.xml` looked correct throughout, which is what made + * it hard to see. + * + * Neither is visible to a unit test, and both are invisible to a casual look at + * the page. So they are checked here, against the real origin, over the network. + * + * node scripts/verify-deploy.mjs https://actuo.example + * pnpm run verify:deploy https://actuo.example + * + * Exits non-zero on the first failure, naming what to change. + */ + +const SENTINEL = '__PUBLIC_ORIGIN__'; +const TIMEOUT_MS = 20_000; + +const base = (process.argv[2] ?? process.env.DEPLOY_URL ?? '').trim().replace(/\/+$/, ''); +if (!base) { + console.error('usage: node scripts/verify-deploy.mjs '); + process.exit(2); +} + +/** A deploy that is merely asleep should read as slow, not as broken. */ +async function get(path) { + const url = `${base}${path}`; + const response = await fetch(url, { signal: AbortSignal.timeout(TIMEOUT_MS) }); + return { url, status: response.status, body: await response.text() }; +} + +const failures = []; +const pass = (label, detail) => console.log(` ok ${label}${detail ? ` — ${detail}` : ''}`); +const fail = (label, why, fix) => { + console.log(` FAIL ${label} — ${why}`); + failures.push({ label, why, fix }); +}; + +console.log(`\nVerifying ${base}\n`); + +// --- The API answers at all ------------------------------------------------- +try { + const { status, body } = await get('/api/health'); + const ok = status === 200 && JSON.parse(body)?.status === 'ok'; + if (ok) pass('/api/health', '200 ok'); + else fail('/api/health', `status ${status}, body ${body.slice(0, 120)}`, 'The container is not serving Nest. Check the service logs.'); +} catch (error) { + fail('/api/health', String(error), 'The deploy is unreachable, or still waking up.'); +} + +// --- SSR is actually on ----------------------------------------------------- +let home = ''; +try { + const { status, body } = await get('/'); + home = body; + if (status !== 200) { + fail('/', `status ${status}`, 'The Angular handler is not answering.'); + } else if (body.includes('ng-server-context')) { + pass('/', 'server-rendered (ng-server-context present)'); + } else { + fail( + '/ server-rendered', + 'no ng-server-context — Angular fell back to client-side rendering', + 'Set NG_ALLOWED_HOSTS on the SERVICE (runtime, not just build) to cover this hostname, e.g. "*.onrender.com". Angular does not error when the Host header is off the list; it silently renders on the client.', + ); + } +} catch (error) { + fail('/', String(error), 'The deploy is unreachable.'); +} + +// --- The SEO stamp reached every copy of the HTML --------------------------- +for (const [path, body] of [['/', home], ['/sitemap.xml', null], ['/robots.txt', null]]) { + try { + const text = body ?? (await get(path)).body; + if (!text) continue; + if (text.includes(SENTINEL)) { + fail( + `${path} stamped`, + `still contains ${SENTINEL}`, + 'PUBLIC_ORIGIN is a BUILD arg, so a restart cannot fix it — redeploy. If only some paths are affected, scripts/stamp-seo.mjs is not covering the whole dist tree.', + ); + } else { + pass(`${path} stamped`, 'no sentinel'); + } + } catch (error) { + fail(`${path} stamped`, String(error), 'Could not fetch it.'); + } +} + +// --- The converter is configured, and on another origin --------------------- +try { + const { body } = await get('/api/config'); + const config = JSON.parse(body); + const url = typeof config?.converterUrl === 'string' ? config.converterUrl : ''; + if (!url) { + fail( + '/api/config converterUrl', + 'unset', + 'Set CONVERTER_URL on the service. Unset is a valid state — the converter surfaces say so — but the cross-origin demo will not run.', + ); + } else if (new URL(url).origin === new URL(base).origin) { + fail( + '/api/config converterUrl', + `same origin as the app (${url})`, + 'It must be an origin the app does not serve, or getTools() returns same-origin tools and the Copilot filters them out.', + ); + } else { + pass('/api/config converterUrl', url); + } +} catch (error) { + fail('/api/config', String(error), 'Could not read the client config.'); +} + +// --- Report ----------------------------------------------------------------- +if (failures.length === 0) { + console.log('\nAll checks passed.\n'); + process.exit(0); +} + +console.log(`\n${failures.length} check(s) failed:\n`); +for (const { label, fix } of failures) console.log(` ${label}\n → ${fix}\n`); +process.exit(1);