diff --git a/README.md b/README.md index e812218a..f4e66909 100644 --- a/README.md +++ b/README.md @@ -11,8 +11,15 @@ Note: the custom LLM backend has moved to https://github.com/AIToolsLab/writing- - Open a Microsoft word document on web - Navigate to insert tab - Click add-ins button, upload my add-in -- Drag add-in/manifest.xml into the upload bar -- Click upload +- Drag the manifest into the upload bar, then click upload + +Which manifest depends on which deployment you want. `npm run build` in +`frontend/` renders all of them into `frontend/dist/`: `manifest.xml` (prod), +`manifest-staging.xml` (staging, for beta testers) and `manifest-dev.xml` (your +local dev server). They have separate add-in ids, so you can install more than +one at a time — the Add-ins menu tells them apart by the first word of the name +("Thoughtful", "Beta Thoughtful", "Dev Thoughtful"). See +[frontend/manifest/README.md](frontend/manifest/README.md). If you can't find the Add-ins tab, look instead on the File menu for "Get Add-ins" or something like that, then click Manage Add-ins. diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 8cbd40a0..3a9c29c6 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -17,9 +17,14 @@ TypeScript/React Microsoft Office Add-in for Word + standalone editor - `taskpane.html` - Word task pane - `editor.html` - Standalone demo editor - `logs.html`, `commands.html`, `index.html` (landing page) -- **Static assets**: `public/` (copied to `dist/` root; includes `manifest.xml` - and `public/assets/`). Images imported in code live in `src/assets/`. -- **Manifest**: `frontend/public/manifest.xml` for Office Add-in configuration +- **Static assets**: `public/` (copied to `dist/` root; includes + `public/assets/`). Images imported in code live in `src/assets/`. +- **Manifests**: `frontend/manifest/` — one template plus a table of what differs + per environment, rendered by `vite build` into `dist/manifest.xml` (prod), + `dist/manifest-staging.xml` and `dist/manifest-dev.xml`. Every build emits all + three; they're install-time artifacts, so nothing serves them. See + [manifest/README.md](manifest/README.md) before editing either file — in + particular, never hardcode an origin in the template. ### Pages and the navbar @@ -112,7 +117,9 @@ Two runners own two disjoint directories — never mix them: - **Vitest** (unit/integration) — `src/`, files named `*.test.ts(x)`, colocated in `__tests__/`. Scoped via `include` in `vitest.config.ts`. Run with `npm test` - (or `npm run test:watch`). + (or `npm run test:watch`). `manifest/` is the one addition outside `src/`: it's + build-time config rather than app code, so it doesn't belong under `src/`, but + its test is a plain unit test. - LLM calls are tested by passing a `MockLanguageModelV3` (from `ai/test`) as the `model` arg to `streamTextDeltas`/`generateFullText` — see `src/api/__tests__/generate.test.ts`, including how to stream an `error` part. diff --git a/frontend/manifest/README.md b/frontend/manifest/README.md new file mode 100644 index 00000000..dd491fcd --- /dev/null +++ b/frontend/manifest/README.md @@ -0,0 +1,72 @@ +# Word add-in manifests + +Each deploy target needs its own manifest. The task pane loads whatever origin +`SourceLocation` names, and Office won't render a different origin inside the +pane unless the manifest declares it in `AppDomains` — so unlike the Google Docs +sidebar, which picks its source at runtime (`google-docs-addon/sidebar.html`), +Word decides at install time. + +Two files: + +- **`template.xml`** — the structure, once. Placeholders: `{{APP_ID}}`, + `{{APP_NAME}}`, `{{BASE_URL}}`. +- **`environments.ts`** — the table of what differs, once. + +`vite build` renders one manifest per environment into `frontend/dist/`: + +| Environment | File | Origin | `DisplayName` | +| --- | --- | --- | --- | +| prod | `manifest.xml` | `app.thoughtful-ai.com` | Thoughtful | +| staging | `manifest-staging.xml` | `staging.thoughtful-ai.com` | Beta Thoughtful | +| dev | `manifest-dev.xml` | `localhost:3000` | Dev Thoughtful | + +Every build emits all three, deliberately: which manifest you need depends on who +is installing it, not on the flags that produced the build. (This is also a bug +fix. The manifest used to be rendered according to the build's `--mode`, so the +image the Dockerfile builds carried a manifest naming the prod origin no matter +which environment it was deployed to.) + +Manifests are sideloaded or uploaded to an add-in store, never fetched from a +running server, so nothing serves them and the dev server doesn't render them. + +## Installing one + +Word (web): Insert → Add-ins → Upload My Add-in → the file from `dist/`. Word +keys installs by `` and each environment has its own, so prod, staging and +dev can be installed side by side. + +## Adding or changing an environment + +Edit `environments.ts`. Two constraints `environments.test.ts` enforces, both +learned the hard way: + +- **Ids must be distinct.** Two manifests sharing an id are one add-in to Word: + they can't coexist, and installing one replaces the other. +- **The distinguishing word goes first in `name`.** The Add-ins menu truncates + `DisplayName` to about ten characters, so "Thoughtful-dev" and "Thoughtful" + both show up as "Thoughtful" — which is what made a dev install + indistinguishable from a real one. The task pane header shows the full string, + so the menu is the only place this bites. + +Never hardcode an origin in `template.xml`; build it from `{{BASE_URL}}`. The +test fails the build if a rendered manifest mentions any origin other than its +own (plus Microsoft's `go.microsoft.com` "learn more" link). + +## Validating + +`office-addin-manifest` validates against a Microsoft web service, so these need +network access: + +```bash +npm run build +npm run validate # dev +npm run validate:staging # staging, with AppSource's stricter rules +npm run validate:prod # prod, likewise +``` + +## Known issue + +`Commands.Url` points at `{{BASE_URL}}/commands/commands.html`, but the build +emits `commands.html` at the root of `dist/`, so the `FunctionFile` 404s. This +predates the template (it was the same in the checked-in manifest) and is +untouched here rather than folded silently into a refactor. diff --git a/frontend/manifest/environments.test.ts b/frontend/manifest/environments.test.ts new file mode 100644 index 00000000..919bb2dd --- /dev/null +++ b/frontend/manifest/environments.test.ts @@ -0,0 +1,106 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { + MANIFEST_ENV_NAMES, + MANIFEST_ENVS, + renderManifest, + type ManifestEnv, +} from './environments'; + +// `import.meta.url` rather than `__dirname`: Vitest processes this file as ESM, +// where `__dirname` doesn't exist. (vite.config.ts is the mirror image — Vite +// bundles it as CJS, so it uses `__dirname` and can't use `import.meta`. That's +// why environments.ts stays pure and each caller reads the template itself.) +const template = readFileSync( + fileURLToPath(new URL('./template.xml', import.meta.url)), + 'utf-8', +); + +const rendered = Object.fromEntries( + MANIFEST_ENV_NAMES.map((name) => [name, renderManifest(template, MANIFEST_ENVS[name])]), +) as Record; + +describe('manifest environments', () => { + it('gives every environment a distinct add-in id', () => { + // Office keys installs by id, so a duplicate would silently make two + // environments the same add-in — uninstallable side by side. + const ids = MANIFEST_ENV_NAMES.map((name) => MANIFEST_ENVS[name].id); + expect(new Set(ids).size).toBe(ids.length); + }); + + it('gives every environment a distinct origin and filename', () => { + const origins = MANIFEST_ENV_NAMES.map((name) => MANIFEST_ENVS[name].baseUrl); + const files = MANIFEST_ENV_NAMES.map((name) => MANIFEST_ENVS[name].fileName); + expect(new Set(origins).size).toBe(origins.length); + expect(new Set(files).size).toBe(files.length); + }); + + it('distinguishes environments within the first 10 characters of the name', () => { + // Word's Add-ins menu truncates DisplayName to about this much, which is + // why the marker leads rather than trails. If two environments become + // indistinguishable there, a developer can't tell which add-in they're + // opening — the bug that prompted all of this. + const prefixes = MANIFEST_ENV_NAMES.map((name) => + MANIFEST_ENVS[name].name.slice(0, 10), + ); + expect(new Set(prefixes).size).toBe(prefixes.length); + }); + + it('never lets a baseUrl end in a slash', () => { + // The template appends "/taskpane.html" and friends directly. + for (const name of MANIFEST_ENV_NAMES) { + expect(MANIFEST_ENVS[name].baseUrl.endsWith('/')).toBe(false); + } + }); + + it('throws on an unknown placeholder rather than emitting it', () => { + expect(() => renderManifest('{{NOPE}}', MANIFEST_ENVS.prod)).toThrow( + /\{\{NOPE\}\}/, + ); + }); +}); + +describe.each(MANIFEST_ENV_NAMES)('rendered %s manifest', (envName) => { + const env: ManifestEnv = MANIFEST_ENVS[envName]; + const xml = rendered[envName]; + + it('substitutes every placeholder', () => { + expect(xml).not.toMatch(/\{\{/); + }); + + it('carries its own id, name and origin', () => { + expect(xml).toContain(`${env.id}`); + expect(xml).toContain(``); + expect(xml).toContain(``); + }); + + it('mentions no other environment', () => { + // The guard the old regex transform couldn't provide: it rewrote the + // origins it knew about, so a URL hardcoded into the manifest later would + // have shipped one environment's host inside another's manifest, silently. + for (const otherName of MANIFEST_ENV_NAMES) { + if (otherName === envName) continue; + const other = MANIFEST_ENVS[otherName]; + expect(xml).not.toContain(other.baseUrl); + expect(xml).not.toContain(other.id); + } + }); + + it('points every add-in URL at its own origin', () => { + // Catches a hardcoded origin the check above would miss because it belongs + // to no environment at all (a typo'd host, a leftover ngrok tunnel). + const urls = [...xml.matchAll(/DefaultValue="(https?:\/\/[^"]+)"/g)].map( + (match) => match[1], + ); + const foreign = urls.filter( + (url) => + !url.startsWith(env.baseUrl) && + // The only legitimate third-party URL: Office's own "learn more" link. + !url.startsWith('https://go.microsoft.com/'), + ); + expect(foreign).toEqual([]); + // Guards the regex above against silently matching nothing. + expect(urls.length).toBeGreaterThan(5); + }); +}); diff --git a/frontend/manifest/environments.ts b/frontend/manifest/environments.ts new file mode 100644 index 00000000..eadb168d --- /dev/null +++ b/frontend/manifest/environments.ts @@ -0,0 +1,121 @@ +/** + * The one place the Word add-in's environments differ. + * + * Word has no equivalent of the Google Docs sidebar's runtime source picker: the + * task pane loads whatever origin `SourceLocation` names, so each deploy target + * needs its own manifest. That used to be produced by rewriting a checked-in dev + * manifest with regexes at build time (`.replace(/-dev/g, '')` and friends), + * which had two problems worth remembering, since they're what this file exists + * to prevent: + * + * 1. It could only express two environments. A third has no spelling in a + * transform whose whole vocabulary is "strip the `-dev` suffix". + * 2. It only rewrote strings someone remembered to write in the `-dev` form. + * `CommandsGroup.Label` was plain "Thoughtful", so the ribbon group looked + * identical in dev and prod — drift that nothing detected, in the exact file + * whose purpose is to tell the environments apart. + * + * Now `manifest/template.xml` holds the structure once, this table holds the + * differences once, and `environments.test.ts` asserts a rendered manifest can + * never mention another environment's origin. + */ + +export type ManifestEnvName = 'dev' | 'staging' | 'prod'; + +export interface ManifestEnv { + /** + * Office keys an installed add-in by this GUID: two manifests sharing one id + * are the same add-in as far as Word is concerned, so they can't be installed + * side by side and an update to one replaces the other. Every environment + * therefore needs its own. + */ + id: string; + /** + * Origin serving `taskpane.html`, with no trailing slash. Every URL in the + * rendered manifest is built from this. + */ + baseUrl: string; + /** + * `DisplayName`, and the base for every user-visible string derived from it. + * + * The distinguishing word goes FIRST. Word's Add-ins menu truncates this to + * roughly ten characters, so a trailing marker is invisible exactly where you + * need it — "Thoughtful-dev" and "Thoughtful" both render as "Thoughtful" + * there, which is what made a dev install indistinguishable from a real one. + * The task pane header shows the full string, so only the menu is affected. + */ + name: string; + /** Filename emitted into `dist/`. */ + fileName: string; +} + +export const MANIFEST_ENVS: Record = { + dev: { + id: '46d2493d-60db-4522-b2aa-e6f2c08d2507', + baseUrl: 'https://localhost:3000', + name: 'Dev Thoughtful', + fileName: 'manifest-dev.xml', + }, + staging: { + id: '46d2493d-60db-4522-b2aa-e6f2c08d2509', + baseUrl: 'https://staging.thoughtful-ai.com', + name: 'Beta Thoughtful', + fileName: 'manifest-staging.xml', + }, + prod: { + id: '46d2493d-60db-4522-b2aa-e6f2c08d2508', + baseUrl: 'https://app.thoughtful-ai.com', + // Keeps the bare filename: this is the manifest already submitted to + // AppSource and handed out for sideloading, so it stays where it was. + name: 'Thoughtful', + fileName: 'manifest.xml', + }, +}; + +export const MANIFEST_ENV_NAMES = Object.keys(MANIFEST_ENVS) as ManifestEnvName[]; + +/** Path of the template, relative to the frontend root. */ +export const TEMPLATE_RELATIVE_PATH = 'manifest/template.xml'; + +const SUBSTITUTIONS: Record = { + APP_ID: 'id', + APP_NAME: 'name', + BASE_URL: 'baseUrl', +}; + +/** Comments addressed to whoever edits the template, dropped when rendering. */ +const TEMPLATE_DOC_COMMENT = /\n?/g; + +/** Replaces it, so a rendered manifest says where to make changes instead. */ +function banner(env: ManifestEnv): string { + return ``; +} + +/** + * Substitutes `{{PLACEHOLDER}}` tokens for one environment's values, and swaps + * the template's authoring notes for a generated-file banner. + * + * Throws on an unknown placeholder rather than leaving it in place: a manifest + * containing a literal `{{TYPO}}` is one Office rejects at install time, and a + * build failure is a much cheaper way to find that out. + */ +export function renderManifest(template: string, env: ManifestEnv): string { + // The XML declaration has to stay on the first line, so splice the banner in + // after it rather than prepending. + const withBanner = template + .replace(TEMPLATE_DOC_COMMENT, '') + .replace(/^(<\?xml[^?]*\?>\n)/, `$1${banner(env)}\n`); + + return withBanner.replace(/\{\{(\w+)\}\}/g, (_match, token: string) => { + const field = SUBSTITUTIONS[token]; + if (!field) { + throw new Error( + `Unknown placeholder {{${token}}} in ${TEMPLATE_RELATIVE_PATH}. ` + + `Known placeholders: ${Object.keys(SUBSTITUTIONS) + .map((name) => `{{${name}}}`) + .join(', ')}.`, + ); + } + return env[field]; + }); +} diff --git a/frontend/public/manifest.xml b/frontend/manifest/template.xml similarity index 70% rename from frontend/public/manifest.xml rename to frontend/manifest/template.xml index 2b8e3654..658b8462 100644 --- a/frontend/public/manifest.xml +++ b/frontend/manifest/template.xml @@ -1,20 +1,34 @@ + - 46d2493d-60db-4522-b2aa-e6f2c08d2507 + {{APP_ID}} 1.0.0.1 Calvin University Computer Science Department en-US - + - - - + + + - https://localhost:3000 + {{BASE_URL}} @@ -25,7 +39,7 @@ - + ReadWriteDocument + DefaultValue="{{BASE_URL}}/assets/logo_16.png" /> + DefaultValue="{{BASE_URL}}/assets/logo.png" /> + DefaultValue="{{BASE_URL}}/assets/logo_80.png" /> - + DefaultValue="{{BASE_URL}}/commands/commands.html" /> + - - + DefaultValue="Get started with {{APP_NAME}}!" /> + + - + DefaultValue="{{APP_NAME}} loaded successfully. Go to the HOME tab and click the 'Show {{APP_NAME}}' button to get started." /> + - \ No newline at end of file + diff --git a/frontend/package.json b/frontend/package.json index 2588e8d9..58db9a33 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -18,11 +18,12 @@ "build:google-docs:dev": "BUILD_TARGET=google-docs vite build --mode development", "dev-server": "vite", "prod-server": "http-server ./dist -p 3000", - "start": "office-addin-debugging start manifest.xml", - "start:desktop": "office-addin-debugging start manifest.xml desktop", - "start:web": "office-addin-debugging start manifest.xml web", - "stop": "office-addin-debugging stop manifest.xml", - "validate": "office-addin-manifest validate manifest.xml", + "start": "office-addin-debugging start dist/manifest-dev.xml", + "start:desktop": "office-addin-debugging start dist/manifest-dev.xml desktop", + "start:web": "office-addin-debugging start dist/manifest-dev.xml web", + "stop": "office-addin-debugging stop dist/manifest-dev.xml", + "validate": "office-addin-manifest validate dist/manifest-dev.xml", + "validate:staging": "office-addin-manifest validate -p dist/manifest-staging.xml", "validate:prod": "office-addin-manifest validate -p dist/manifest.xml", "watch": "vite build --watch --mode development", "typecheck": "tsc --noEmit", diff --git a/frontend/test-build-output.mjs b/frontend/test-build-output.mjs index 09d5cd2c..908da550 100644 --- a/frontend/test-build-output.mjs +++ b/frontend/test-build-output.mjs @@ -119,22 +119,28 @@ expectedStaticFiles.forEach((file) => { } }); -// Test 5: manifest.xml should exist and look transformed. -console.log('\nTest 5: Manifest.xml'); -const manifestPath = path.join(distDir, 'manifest.xml'); -if (fs.existsSync(manifestPath)) { - success('manifest.xml exists in dist'); +// Test 5: every environment's manifest should be rendered, in every build — +// they're picked by who's installing, not by the build's flags. Which origin +// each one names is manifest/environments.test.ts's job; this only checks the +// build emitted them. +console.log('\nTest 5: Manifests'); +['manifest.xml', 'manifest-staging.xml', 'manifest-dev.xml'].forEach((file) => { + const manifestPath = path.join(distDir, file); + if (!fs.existsSync(manifestPath)) { + error(`${file} NOT FOUND in dist`); + return; + } + success(`${file} exists in dist`); const content = fs.readFileSync(manifestPath, 'utf-8'); // Structural check that it's a real Office manifest (avoids URL substring // matching, which is brittle and flagged as unsafe sanitization). if (content.includes(' => { return { plugins: [react()], // Runs after the main `vite build` into the same dist/ (emptyOutDir is - // false). Disable publicDir copying so it does NOT re-copy public/ over the - // main build's output — in particular the prod-transformed dist/manifest.xml, - // which would otherwise be clobbered with the raw dev manifest. + // false). Disable publicDir copying so it does NOT re-copy public/ over + // the main build's output. (The manifests are rendered by + // manifestPlugin rather than copied from public/, so they're no longer + // among the files at risk here — but re-copying the rest is still waste.) publicDir: false, resolve, define, diff --git a/frontend/vitest.config.ts b/frontend/vitest.config.ts index c4c5ccde..bca902a3 100644 --- a/frontend/vitest.config.ts +++ b/frontend/vitest.config.ts @@ -26,9 +26,11 @@ export default defineConfig({ alias: { '@': resolve(__dirname, './src') }, }, test: { - // Only our unit tests under src/. Playwright owns tests/*.spec.ts and has - // its own runner, so keep Vitest out of that directory. - include: ['src/**/*.{test,spec}.{ts,tsx}'], + // Our unit tests under src/, plus manifest/ (build-time config that isn't + // app code and so doesn't belong under src/). Playwright owns + // tests/*.spec.ts and has its own runner, so keep Vitest out of that + // directory. + include: ['src/**/*.{test,spec}.{ts,tsx}', 'manifest/**/*.test.ts'], // Logic-layer tests run in node. Switch specific files to jsdom (via a // `// @vitest-environment jsdom` docblock) once we add component tests. environment: 'node',