From 06ec4bc8ef2f76932193db13820f26c6a7376da9 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Sat, 5 Sep 2026 16:49:49 +0200 Subject: [PATCH] fix(e2e): the chrome destinations are a named gap, not a mystery failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LaunchPad declares nine pages in its manifest and serves ONE. /store, /reports and /flows each redirect to /dashboard, and /reports/dashboards renders the workspace grid. That is not a bug in these tests. It is the app: LaunchPad has no vue-router at all — createRouter appears nowhere in src/ — because navigation is Pinia state that never touches the URL, and it is the only app in the fleet that does not root on CnAppRoot. main.js has called this Tier 1 since manifest adoption landed and named the change that would fix it. That change was never filed, so it is filed here: openspec/changes/launchpad-manifest-tier-3/. The four route-dependent assertions are marked test.fail() against it. Not skipped: a skipped test proves nothing and stops being read, while Playwright FAILS the run when a test.fail() test passes — so landing routing turns these red until the markers come off, in the change that earns it. What stays a live assertion is the half that is true today and worth guarding: the manifest declares four footer destinations in ADR-114's order, each with an icon, Documentation as an external href, and each of the other three naming a page the manifest hosts. The admin settings section splits out of the old foldout test and keeps passing, because it is a Nextcloud settings route rather than one of this app's own. Co-Authored-By: Claude Opus 5 (1M context) --- .../launchpad-manifest-tier-3/proposal.md | 88 +++++++++++++++++++ .../launchpad-manifest-tier-3/tasks.md | 31 +++++++ tests/e2e/app-chrome.spec.ts | 87 +++++++++++++----- 3 files changed, 185 insertions(+), 21 deletions(-) create mode 100644 openspec/changes/launchpad-manifest-tier-3/proposal.md create mode 100644 openspec/changes/launchpad-manifest-tier-3/tasks.md diff --git a/openspec/changes/launchpad-manifest-tier-3/proposal.md b/openspec/changes/launchpad-manifest-tier-3/proposal.md new file mode 100644 index 00000000..8e62c6cb --- /dev/null +++ b/openspec/changes/launchpad-manifest-tier-3/proposal.md @@ -0,0 +1,88 @@ +# LaunchPad manifest tier 3 — serve the pages the manifest declares + +`src/manifest.json` declares nine pages and eight menu entries. LaunchPad +serves one. `/store`, `/reports` and `/flows` each redirect to `/dashboard`, +and `/reports/dashboards` renders the workspace grid. + +This change gives LaunchPad the routing its manifest already assumes, so that a +declared destination is a reachable one. + +## Why now + +`main.js` has named this change since Tier 1 adoption landed: + +> Tier 1 manifest adoption (ADR-024): register the bundled manifest with nc-vue +> so the shared shell can read menu/page declarations. The vue-router definition +> below remains hand-wired (Tier 1 — not yet manifest-driven). +> **Tier 3 (launchpad-manifest-tier-3) will replace hand-wired routes.** + +It was never filed, and in the meantime three things arrived that assume it: + +- **ADR-114 app chrome.** `feat(chrome): give launchpad a Store` (2026-09-04) + added the fourth footer destination and took gate-107 from 4 of 5 to 5 of 5. + gate-107 reads the manifest. It cannot see that nothing routes what the + manifest declares. +- **An E2E spec that has never passed.** `tests/e2e/app-chrome.spec.ts` landed + 2026-09-03 and that is the run in which LaunchPad first went red. It has not + been green since. It asserted `[data-testid="cn-nav"]`, which this app does + not render; retargeting it to `.workspace-shell` (#547) only moved the + failure onto the true cause — the destinations do not resolve. +- **Four cards in the setup wizard's own chrome.** Documentation is an external + href and works. Store, Reports and Features & roadmap are pages this app + claims to host. + +## The shape of the problem + +LaunchPad is the only app in the fleet that does not root on `CnAppRoot`, and +it has **no vue-router at all** — `createRouter` appears nowhere in `src/`. +Navigation is Pinia state: `dashboard.js` and `orgNavigation.js` switch what +the grid shows, in place, without touching the URL. + +That is not an oversight. `App.vue` records the decision where it writes its own +skip link, because not rooting on `NcContent`/`CnAppRoot` means not inheriting +Nextcloud's. `WorkspaceApp.vue` owns five regions — sidebar backdrop, hamburger +strip, grid surface, branded `DashboardFooter` — and `#launchpad-main-content` +carries both the WCAG 2.2 bypass target and the quick-search Esc contract. + +So this is not "add a router". It is "give a single-surface app a second +surface without breaking the first". + +## Affected code units + +- `src/main.js` — create a `vue-router` instance in path mode and mount it; + replace the `useAppManifest` Tier 1 registration with the manifest-driven + route table nc-vue derives from `manifest.pages`. +- `src/App.vue` — root on `CnAppRoot` so the shared chrome renders the four + footer destinations, with `WorkspaceApp` as the dashboard route's component + rather than the app's only child. The skip link moves to whichever shell ends + up owning it; it must not be dropped. +- `src/views/WorkspaceApp.vue` — becomes the `/dashboard` route's view. Its + five regions stay; `#launchpad-main-content` keeps `tabindex="-1"`. +- `src/manifest.json` — no new declarations. This change makes the existing + ones true. +- `tests/e2e/app-chrome.spec.ts` — the four `test.fail()` markers this change + is named in come off, and the assertions become ordinary ones. + +## Decisions needed + +- **Does `CnAppRoot` host the workspace, or the other way round?** The org + navigation rail and the slide-in sidebar are LaunchPad's own and have no + equivalent in `CnAppNav`. Nesting the workspace inside `CnAppRoot`'s content + area gives two navigation systems on one page unless the rail is reconciled + with the nav. +- **The skip link.** `CnAppRoot` brings Nextcloud's; LaunchPad writes its own + against `#launchpad-main-content`. Two bypass links is worse than one. +- **Whether the dashboard keeps its URL.** Dashboards switch in Pinia today, so + a dashboard has no address. `dashboard-deeplinking` (an existing spec) wants + one. Routing is the natural place to settle that, and it widens this change. + +## Alternative considered + +**Make LaunchPad a documented ADR-114 exception**: drop the pages it cannot +serve from the manifest, move the chrome into `DashboardFooter`, and record the +exception in ADR-114 so gate-107 stops reporting a chrome that is not there. + +Rejected as the default because every other app in the fleet roots on +`CnAppRoot`, and the manifest already declares the pages — the cheaper change +is to make the declaration true rather than to withdraw it. Worth revisiting if +the decisions above turn out to cost more than the chrome is worth. diff --git a/openspec/changes/launchpad-manifest-tier-3/tasks.md b/openspec/changes/launchpad-manifest-tier-3/tasks.md new file mode 100644 index 00000000..2c181fdd --- /dev/null +++ b/openspec/changes/launchpad-manifest-tier-3/tasks.md @@ -0,0 +1,31 @@ +# Tasks: LaunchPad manifest tier 3 + +## Decide first + +- [ ] Settle whether `CnAppRoot` hosts `WorkspaceApp` or the workspace shell keeps the page and only borrows the footer chrome. +- [ ] Settle the skip link: one bypass target, not two. +- [ ] Settle whether a dashboard gets a URL, and whether that is this change or `dashboard-deeplinking`. + +## Routing + +- [ ] Add `vue-router` and create the router in `src/main.js` in **path** mode (`createWebHistory`), matching the fleet: a hash route would be ignored and land silently on the dashboard. +- [ ] Derive the route table from `manifest.pages` rather than hand-wiring it, so a declared page is a routed page by construction. +- [ ] `/dashboard` (and `/`) render `WorkspaceApp`; `#launchpad-main-content` keeps `tabindex="-1"` and stays the bypass target. +- [ ] `/store`, `/reports`, `/reports/dashboards` and `/features-roadmap` render their declared page types. + +## Shell + +- [ ] Root on `CnAppRoot` per the decision above, keeping the org navigation rail and slide-in sidebar working. +- [ ] One skip link. +- [ ] `.workspace-shell` still renders on the dashboard route — the chrome spec's tripwire test asserts the shell, and it should keep passing. + +## Tests + +- [ ] Remove the four `test.fail()` markers in `tests/e2e/app-chrome.spec.ts`. They are expected-to-fail today; Playwright fails the run if a `test.fail()` test passes, so landing routing turns them red until the markers come off. That is the intended signal. +- [ ] The tripwire test `the shell is LaunchPad's own, not the shared CnAppNav one` asserts `cn-nav` has count 0. If `CnAppRoot` is adopted it must be rewritten, not deleted. +- [ ] A route test per declared page, so a page added to the manifest without a component fails. + +## Verify + +- [ ] gate-107 still reports 5 of 5, and now truthfully. +- [ ] The E2E leg is green on the `development` push run — it has not been since 2026-09-03. diff --git a/tests/e2e/app-chrome.spec.ts b/tests/e2e/app-chrome.spec.ts index 6d34d703..5366b244 100644 --- a/tests/e2e/app-chrome.spec.ts +++ b/tests/e2e/app-chrome.spec.ts @@ -92,9 +92,7 @@ test.describe('app chrome (ADR-114)', () => { ).toHaveCount(0) }) - test('the chrome declares Documentation, Store, Reports and Features & roadmap, and each destination resolves', async ({ - page, - }) => { + test('the chrome declares Documentation, Store, Reports and Features & roadmap', async () => { // The manifest is read here rather than restated, so a renamed or // dropped entry is a failure instead of a silently stale literal. @@ -131,21 +129,48 @@ test.describe('app chrome (ADR-114)', () => { (manifest.pages ?? []).map((p: any) => [p.id, p.route]), ) for (const entry of footer.slice(1)) { - const route = pages.get(entry.route) expect( - route, + pages.get(entry.route), `${entry.label} names page "${entry.route}", which this app does not host`, ).toBeTruthy() + } + }) + + test('each declared chrome destination opens', async ({ page }) => { + // 🔴 EXPECTED TO FAIL, AND FILED. LaunchPad declares nine pages and + // serves ONE: `/store`, `/reports` and `/flows` each redirect to + // `/dashboard`. It has no vue-router at all — `createRouter` appears + // nowhere in src/ — because navigation is Pinia state that never + // touches the URL. `main.js` calls this Tier 1 and names the change + // that fixes it; that change is now filed at + // `openspec/changes/launchpad-manifest-tier-3/`. + // + // `test.fail()` rather than a skip, deliberately: a skipped test proves + // nothing and quietly stops being read, while this one FAILS THE RUN + // the moment routing lands. That is the notification we want — the + // marker comes off in the same change that makes it pass. + test.fail() + + const manifest = JSON.parse( + // eslint-disable-next-line @typescript-eslint/no-require-imports + require('fs').readFileSync( + // eslint-disable-next-line @typescript-eslint/no-require-imports + require('path').resolve(__dirname, '../../src/manifest.json'), + 'utf-8', + ), + ) + const footer = (manifest.menu ?? []) + .filter((e: any) => e.section === 'footer' && e.route) + .sort((a: any, b: any) => (a.order ?? 0) - (b.order ?? 0)) + const pages = new Map( + (manifest.pages ?? []).map((p: any) => [p.id, p.route]), + ) - // AND IT RENDERS. A row that goes nowhere is the failure mode a - // manifest gate cannot see, so each destination is opened. + for (const entry of footer) { + const route = pages.get(entry.route) await page.goto(`${APP_BASE}${route}`, { waitUntil: 'domcontentloaded', }) - await expect( - page.locator('.workspace-shell'), - `${entry.label} (${route}) did not render the app shell`, - ).toBeVisible({ timeout: 30_000 }) await expect(page).toHaveURL(new RegExp(`${route}(\\?|$)`), { timeout: 15_000, }) @@ -155,12 +180,16 @@ test.describe('app chrome (ADR-114)', () => { test('Reports lists the one report this app can honestly offer', async ({ page, }) => { + // 🔴 EXPECTED TO FAIL UNTIL ROUTING LANDS — see + // `openspec/changes/launchpad-manifest-tier-3/`. This app serves one + // page; the route below redirects to `/dashboard`. `test.fail()` rather + // than a skip, so the run goes red the moment it starts passing. + test.fail() + // One card, deliberately. LaunchPad's register holds a single schema — // dashboard — so a second report would either repeat this one or invent // a reading the data cannot support. If a schema is added later and no // report follows, this count is what notices. - // By route: the Reports destination is declared in the manifest's footer - // section, and this app renders no nav entry to click (see the top). await page.goto(`${APP_BASE}/reports`, { waitUntil: 'domcontentloaded' }) await expect(page).toHaveURL(/\/apps\/launchpad\/reports(\?|$)/, { timeout: 15_000, @@ -177,6 +206,12 @@ test.describe('app chrome (ADR-114)', () => { test('the dashboards report renders real numbers, not empty cards', async ({ page, }) => { + // 🔴 EXPECTED TO FAIL UNTIL ROUTING LANDS — see + // `openspec/changes/launchpad-manifest-tier-3/`. This app serves one + // page; the route below redirects to `/dashboard`. `test.fail()` rather + // than a skip, so the run goes red the moment it starts passing. + test.fail() + await page.goto(`${APP_BASE}/reports/dashboards`) await expect(page.locator('.workspace-shell')).toBeVisible({ timeout: 30_000, @@ -193,6 +228,12 @@ test.describe('app chrome (ADR-114)', () => { test('Store opens the hosted store surface, which this app writes no backend for', async ({ page, }) => { + // 🔴 EXPECTED TO FAIL UNTIL ROUTING LANDS — see + // `openspec/changes/launchpad-manifest-tier-3/`. This app serves one + // page; the route below redirects to `/dashboard`. `test.fail()` rather + // than a skip, so the run goes red the moment it starts passing. + test.fail() + await page.goto(`${APP_BASE}/store`, { waitUntil: 'domcontentloaded' }) await expect(page).toHaveURL(/\/apps\/launchpad\/store(\?|$)/, { @@ -208,15 +249,14 @@ test.describe('app chrome (ADR-114)', () => { }) }) - test('admin settings and Flows are reachable, which is what the foldout was for', async ({ - page, - }) => { + test('the admin settings section renders', async ({ page }) => { // ⚠️ NOT A FOLDOUT TEST ANY MORE, and it cannot be. The settings // foldout is `CnAppNav`'s, and this app renders no CnAppNav — the // personal-settings entry in particular is a nav widget with no // equivalent in `.workspace-shell`, so there is nothing here to assert - // about it. What survives is the part that is about LaunchPad rather - // than about the nav component: the two destinations exist and open. + // about it. What survives is the half that is about LaunchPad rather + // than about the nav component, and it is a Nextcloud settings route + // rather than one of this app's own, which is why it still passes. await page.goto('/settings/admin/launchpad', { waitUntil: 'domcontentloaded', }) @@ -224,11 +264,16 @@ test.describe('app chrome (ADR-114)', () => { page.locator('#app-content, main').first(), 'the admin settings section did not render', ).toBeVisible({ timeout: 30_000 }) + }) + + test('the Flows page opens', async ({ page }) => { + // 🔴 EXPECTED TO FAIL UNTIL ROUTING LANDS — see + // `openspec/changes/launchpad-manifest-tier-3/`. This app serves one + // page; the route below redirects to `/dashboard`. `test.fail()` rather + // than a skip, so the run goes red the moment it starts passing. + test.fail() await page.goto(`${APP_BASE}/flows`, { waitUntil: 'domcontentloaded' }) - await expect(page.locator('.workspace-shell')).toBeVisible({ - timeout: 30_000, - }) await expect(page).toHaveURL(/\/apps\/launchpad\/flows(\?|$)/, { timeout: 15_000, })