fix(e2e): install Docudesk in CI, repair 2 lying skips, turn traces back on

.github/workflows/code-quality.yml

@@ -110,7 +110,37 @@ jobs:
       # Register on application-type imports — OR PR #1464) and the lifecycle
       # engine (ObjectTransitionedEvent) that OpenBuild's repair steps + the
       # Newman publish step need; `main` predates both. See openbuild#29.
-      additional-apps: '[{"repo":"ConductionNL/openregister","app":"openregister","ref":"development"}]'
+      #
+      # DOCUDESK IS HERE BECAUSE THREE E2E SPECS ASSERT AGAINST ITS API AND
+      # THERE WAS NO WAY FOR THEM TO PASS WITHOUT IT.
+      #
+      # `tests/e2e/spec-coverage/docudesk-document-templates.spec.ts` drives the
+      # builder's Documents section: attach a template, preview it, and warn
+      # when the attached template was deleted. All three go through Docudesk's
+      # own REST surface (`GET/POST /apps/docudesk/api/templates`,
+      # `GET /apps/docudesk/api/templates/{id}`,
+      # `POST /apps/docudesk/api/templates/{id}/preview`). With Docudesk absent
+      # every one of those is a Nextcloud router 404, and the run said so
+      # plainly: `[globalSetup] docudesk not installed — template fixtures
+      # skipped`, then three failures reading `Expected: 200 / Received: 404`.
+      # That is not an app defect and no code change in this repository could
+      # have fixed it — the dependency was simply not installed.
+      #
+      # `tests/e2e/global-setup.ts` already knows what to do once it IS: it
+      # configures Docudesk's `template_register`/`template_schema` through
+      # Docudesk's own `POST /api/settings` and seeds the two template fixtures
+      # the specs attach to, and it degrades to a log line when Docudesk is
+      # missing. So this input is the whole fix.
+      #
+      # `ref: main` — Docudesk has no `development` branch (verified by listing
+      # the repo's branches; it publishes `main` and `beta` only), and `main`
+      # carries every route above.
+      #
+      # Only the API is exercised. `additional-apps` clones and runs `composer
+      # install` but never builds an app's frontend, so Docudesk's own JS bundle
+      # is absent here — which is fine, because no spec in this suite opens a
+      # Docudesk page.
+      additional-apps: '[{"repo":"ConductionNL/openregister","app":"openregister","ref":"development"},{"repo":"ConductionNL/docudesk","app":"docudesk","ref":"main"}]'
       enable-sbom: true
 
       # Integration Tests (Newman) stays OFF here, deliberately, and unlike the

playwright.config.ts

@@ -38,6 +38,15 @@ export default defineConfig({
 	forbidOnly: !!process.env.CI,
 	retries: process.env.CI ? 1 : 0,
 	workers: 1,
+	// Mirror of tests/e2e/playwright.config.ts. CI does not pick this file up
+	// (`playwright-test-path: tests/e2e` makes the workflow's first lookup hit
+	// the config next to the specs), but it IS the documented fallback the
+	// shared workflow uses when that file is absent — and the job it would run
+	// under carries `timeout-minutes: 45`. A cancellation at the job cap is not
+	// a verdict: no tally, and the report is never written so the artifact
+	// upload finds nothing. Stopping ourselves below the cap fails with numbers
+	// attached instead.
+	globalTimeout: 36 * 60 * 1000,
 	globalSetup: './tests/e2e/global-setup.ts',
 	reporter: process.env.CI ? [['github'], ['html', { open: 'never' }]] : 'list',
 	use: {
@@ -56,7 +65,12 @@ export default defineConfig({
 		// (no Location header is emitted, the page stays on /login).
 		// Specs that need OCS-APIRequest set it on their explicit `request`
 		// calls (e.g. versionRouting.spec.ts, applicationDetailOverview.spec.ts).
-		trace: 'on-first-retry',
+		// `retain-on-failure`, not `on-first-retry`: this config's `retries` is
+		// 1 only on CI and 0 locally, so on a developer box the old setting
+		// produced no trace for any failure at all — and CI does not use this
+		// config (see tests/e2e/playwright.config.ts). Keeping the two files in
+		// step so a local reproduction has the same evidence a CI run does.
+		trace: 'retain-on-failure',
 		screenshot: 'only-on-failure',
 		video: 'retain-on-failure',
 		headless: true,

src/dialogs/DocumentTemplateAttachmentDialog.vue

@@ -21,10 +21,30 @@
 		@update:open="$emit('update:open', $event)"
 		@closing="onClose">
 		<div class="ob-document-attach">
+			<!--
+				EXACTLY ONE of these may render. They are mutually exclusive
+				claims about the same thing and they were `v-if`/`v-if`, so both
+				rendered together the moment `docudeskAvailable` flipped false
+				after the dialog had already opened — which is the normal case,
+				because `PageDesignerHost` initialises the flag to `true` and
+				resolves the real value asynchronously (PageDesignerHost.vue
+				`docudeskAvailable: true` → `docudeskStatus.available`). The
+				dialog's `open` watcher therefore ran the snapshot refresh, got a
+				404 from a route that does not exist, set `templateMissing`, and
+				then the late `false` added the second paragraph on top.
+
+				The result told the user two contradictory things at once — "the
+				app is not installed" and "your template was deleted from it" —
+				and the second is not knowable when the first is true: a 404 from
+				an absent app is the router saying the ROUTE is missing, not
+				Docudesk saying the TEMPLATE is. `v-else-if` makes the absence
+				message win, which is both the honest reading and the actionable
+				one.
+			-->
 			<p v-if="!docudeskAvailable" class="ob-document-attach__warn">
 				{{ t('openbuild', 'Docudesk is not installed or enabled on this instance. The template list cannot be loaded.') }}
 			</p>
-			<p v-if="templateMissing" class="ob-document-attach__warn" role="alert">
+			<p v-else-if="templateMissing" class="ob-document-attach__warn" role="alert">
 				{{ t('openbuild', 'The attached template no longer exists in Docudesk. Pick another template or detach.') }}
 			</p>
 

tests/dialogs/DocumentTemplateAttachmentDialog.spec.js

@@ -66,3 +66,53 @@ describe('DocumentTemplateAttachmentDialog — preview sanitization', () => {
 		expect(out).toContain('<li>a</li>')
 	})
 })
+
+/**
+ * The two `.ob-document-attach__warn` paragraphs are mutually exclusive claims
+ * about the same subject: "Docudesk is not installed" and "the template you
+ * attached no longer exists IN Docudesk". The second is not knowable when the
+ * first is true.
+ *
+ * They were authored as two independent `v-if`s, and both rendered together
+ * whenever `docudeskAvailable` arrived late — the normal case, since
+ * PageDesignerHost initialises it to `true` and resolves the real value
+ * asynchronously. The dialog's `open` watcher then ran its snapshot refresh,
+ * took a 404 from a route that does not exist, set `templateMissing`, and the
+ * late `false` stacked the absence message on top.
+ *
+ * Playwright saw it as `strict mode violation: locator('.ob-document-attach__warn')
+ * resolved to 2 elements` in run 31083894467. These assertions pin the
+ * invariant directly so the next regression is caught in milliseconds by the
+ * unit suite instead of in a 19-minute browser run.
+ */
+describe('DocumentTemplateAttachmentDialog — mutually exclusive warnings', () => {
+	/**
+	 * Mount the dialog and drive it into the both-warnings-eligible state.
+	 *
+	 * @param {boolean} docudeskAvailable Value of the capability prop.
+	 * @return {Promise<object>} The mounted wrapper.
+	 */
+	async function warnState(docudeskAvailable) {
+		const wrapper = mount(DocumentTemplateAttachmentDialog, {
+			propsData: { docudeskAvailable },
+			stubs: baseStubs,
+		})
+		// `templateMissing` is what a 404 from the snapshot refresh sets.
+		await wrapper.setData({ templateMissing: true })
+		return wrapper
+	}
+
+	it('renders exactly one warning when Docudesk is absent AND a template 404d', async () => {
+		const wrapper = await warnState(false)
+		const warns = wrapper.findAll('.ob-document-attach__warn')
+		expect(warns).toHaveLength(1)
+		expect(warns[0].text()).toContain('not installed')
+	})
+
+	it('renders the deleted-template warning when Docudesk IS available', async () => {
+		const wrapper = await warnState(true)
+		const warns = wrapper.findAll('.ob-document-attach__warn')
+		expect(warns).toHaveLength(1)
+		expect(warns[0].text()).toContain('no longer exists')
+	})
+})

tests/e2e/automations-rbac.spec.ts

@@ -27,6 +27,11 @@ const EDITOR_USER = process.env.NC_RBAC_EDITOR_USER ?? 'rbac-editor'
 const EDITOR_PASS = process.env.NC_RBAC_EDITOR_PASS ?? 'RbacEditor-1!'
 const OWNER_USER = process.env.NC_RBAC_OWNER_USER ?? 'rbac-owner'
 const OWNER_PASS = process.env.NC_RBAC_OWNER_PASS ?? 'RbacOwner-1!'
+// Admin credentials for the capability probe below ONLY. Same resolution the
+// config uses for `use.httpCredentials`; the tests themselves deliberately run
+// as non-admins.
+const ADMIN_USER = process.env.NC_ADMIN_USER ?? 'admin'
+const ADMIN_PASS = process.env.NC_ADMIN_PASSWORD ?? process.env.NC_ADMIN_PASS ?? 'admin'
 // The seeded `hello-world` fixture only ever carries a single `production`
 // version (see lib/Command/SeedHelloWorldFixture.php) — there is no draft
 // version to author on, so REQ-AUTD-008's "editor authors on draft, gets 403
@@ -46,24 +51,48 @@ const APP_SLUG = process.env.NC_RBAC_TEST_SLUG ?? 'rbac-automations-app'
 const APP_TITLE_PATTERN = new RegExp(APP_SLUG.replace(/-/g, '.?'), 'i')
 
 /**
- * Same schema-slug-collision guard as automations.spec.ts (duplicated here
- * since each e2e spec file is self-contained) — see that file's
- * `automationSchemaIsUsable()` doc comment for the full root-cause writeup.
- * On this shared instance the openbuild `automation` schema slug collides
- * with a pre-existing, unrelated schema of the same slug from another app,
- * so `POST`ing a real automation payload 400s regardless of which
- * Application/Version it targets — this blocks BOTH scenarios in this file
- * (composing one, and toggling a pre-existing one) identically.
+ * Is openbuild's `automation` schema readable and shaped as this suite expects?
+ *
+ * THIS PROBE USED TO REPORT `false` HERE AND `true` EVERYWHERE ELSE, IN THE
+ * SAME RUN, AGAINST THE SAME INSTANCE.
+ *
+ * It is a copy of `automations.spec.ts`'s helper, and it carried that file's
+ * reason with it: "the openbuild `automation` schema slug collides with a
+ * pre-existing schema of the same slug on this shared instance — automation
+ * CREATE/SAVE 400s regardless of app/version". Run 31083894467 disproves that
+ * for CI outright. Seven tests in `automations.spec.ts` sit behind the very
+ * same guard and PASSED, composing and saving real automations end to end
+ * (REQ-AUTD-002 ×3, -003, -005, -006, -007). Both tests in THIS file skipped.
+ *
+ * The discriminator is not the instance, it is the auth context. This describe
+ * declares `test.use({ storageState: { cookies: [], origins: [] } })` so each
+ * test can log in as a non-admin — which also makes the `request` fixture
+ * anonymous. The probe's read of `api/schemas/automation` was therefore
+ * refused, `resp.ok()` was false, and the helper reported the refusal as a
+ * fact about the schema. A guard that returns "the feature is broken" when it
+ * means "I could not look" produces exactly this: a permanent skip with a
+ * confident, wrong explanation attached.
+ *
+ * Two changes. The probe authenticates with the admin credentials the config
+ * already uses for `httpCredentials`, independently of whatever storageState
+ * the test is running under. And a NON-OK response is now a thrown error
+ * rather than a `false`, so "cannot read the schema" fails the run loudly
+ * instead of silently becoming "the schema is unusable" — the failure mode
+ * this helper just spent a release exhibiting.
  *
  * @param request Playwright APIRequestContext (fixture-provided).
- * @return {Promise<boolean>} True when automation CREATE/SAVE is usable.
+ * @return {Promise<boolean>} True when the schema reads back with the expected shape.
  */
 async function automationSchemaIsUsable(request: APIRequestContext): Promise<boolean> {
+	const auth = Buffer.from(`${ADMIN_USER}:${ADMIN_PASS}`).toString('base64')
 	const resp = await request.get(`${NEXTCLOUD_URL}/index.php/apps/openregister/api/schemas/automation`, {
-		headers: { 'OCS-APIRequest': 'true' },
+		headers: { 'OCS-APIRequest': 'true', Authorization: `Basic ${auth}` },
 	})
 	if (resp.ok() === false) {
-		return false
+		throw new Error(
+			`automationSchemaIsUsable: could not read api/schemas/automation — HTTP ${resp.status()}. `
+			+ 'This is a broken probe, not a verdict about the schema; it must not be reported as one.',
+		)
 	}
 	const schema = await resp.json()
 	return schema?.properties?.trigger?.type === 'object'
@@ -93,7 +122,7 @@ test.describe('automation-designer — RBAC (REQ-AUTD-008)', () => {
 	test.use({ storageState: { cookies: [], origins: [] } })
 
 	test('editor authors + enables an automation on a non-production (draft) version', async ({ page, request }) => {
-		test.skip(await automationSchemaIsUsable(request) === false, 'openbuild `automation` schema slug collides with a pre-existing schema of the same slug on this shared instance — automation CREATE/SAVE 400s regardless of app/version; see automationSchemaIsUsable()')
+		test.skip(await automationSchemaIsUsable(request) === false, 'openbuild `automation` schema does not read back with a `trigger` object property — see automationSchemaIsUsable() for why this must be a real verdict and not a failed lookup')
 		await loginAs(page, EDITOR_USER, EDITOR_PASS)
 		await page.goto(`${NEXTCLOUD_URL}/apps/openbuild/automations`)
 		await page.waitForSelector('.automations-page', { timeout: 20_000 })
@@ -121,7 +150,7 @@ test.describe('automation-designer — RBAC (REQ-AUTD-008)', () => {
 	})
 
 	test('editor gets 403 enabling on the production version; owner succeeds', async ({ page, browser, request }) => {
-		test.skip(await automationSchemaIsUsable(request) === false, 'requires a pre-existing automation on the production version, which cannot be created — openbuild `automation` schema slug collides with a pre-existing schema of the same slug on this shared instance; see automationSchemaIsUsable()')
+		test.skip(await automationSchemaIsUsable(request) === false, 'openbuild `automation` schema does not read back with a `trigger` object property — see automationSchemaIsUsable() for why this must be a real verdict and not a failed lookup')
 		await loginAs(page, EDITOR_USER, EDITOR_PASS)
 		await page.goto(`${NEXTCLOUD_URL}/apps/openbuild/automations`)
 		await page.waitForSelector('.automations-page', { timeout: 20_000 })

tests/e2e/bootstrap-openbuild.e2e.spec.ts

@@ -17,14 +17,28 @@ import { test, expect } from '@playwright/test'
  *  - Playwright browsers installed (`npx playwright install --with-deps`).
  */
 test.describe('bootstrap-openbuild hello-world', () => {
-	// QUARANTINED (Conduction/openbuild#41): openbuild admin UI not functional in this build — builder host blank (BuilderHostView unresolved by nc-vue CnPageRenderer) / no detail/editor/version pages. Re-enable when #41 is fixed.
-	test.skip('renders the three seeded hello-message objects on the index page', async ({ page }) => {
+	// UN-QUARANTINED 2026-08-06. The recorded reason — "#41: builder host blank
+	// (BuilderHostView unresolved by nc-vue CnPageRenderer)" — is the SAME
+	// sentence builder-host.spec.ts carries above its own un-quarantine note
+	// saying it "no longer holds". builder-host.spec.ts's first test performs
+	// this identical journey (goto /apps/openbuild/builder/hello-world, then
+	// assert the same three seeded titles) and PASSES in CI — measured in run
+	// 31083894467. One of the two files was simply never revisited.
+	test('renders the three seeded hello-message objects on the index page', async ({ page }) => {
 		await page.goto('/apps/openbuild/builder/hello-world')
 
 		// The SPA needs a moment to fetch the manifest and resolve the index page.
 		// The hello-world manifest's index page lists `hello-message` objects with
 		// the title, body and @self.created columns.
-		await expect(page).toHaveURL(/\/index\.php\/apps\/openbuild\/builder\/hello-world/)
+		//
+		// The `/index.php` prefix is OPTIONAL and is not what this test is about.
+		// Nextcloud emits it only when `htaccess.IgnoreFrontController` is off;
+		// CI turns it ON (tests/e2e/ci-seed.sh gates on the served page reporting
+		// `modRewriteWorking:true`), so the pretty form is what the router
+		// produces there and the old anchored regex could not have matched. The
+		// app path is still asserted in full — only the webroot style, an
+		// instance-configuration artifact, is allowed to vary.
+		await expect(page).toHaveURL(/(\/index\.php)?\/apps\/openbuild\/builder\/hello-world/)
 
 		// Seed bodies — anchored on the canonical strings written by
 		// SeedHelloWorld::buildSampleMessages(). At minimum the page must

tests/e2e/builder-undo-redo.spec.ts

@@ -31,6 +31,7 @@
 
 import { test, expect, type Page } from '@playwright/test'
 import { ensureApp as ensureAppFixture, dismissOverlays, suppressSupportDialog } from './support/appFixture'
+import { ensureVersionChain } from './support/versionChain'
 
 // PLAYWRIGHT_BASE_URL wins — see tests/e2e/support/baseUrl.ts.
 import { E2E_BASE_URL as BASE_URL } from './support/baseUrl'
@@ -244,18 +245,30 @@ test.describe('builder-undo-redo — page designer (REQ-BUR-001..004)', () => {
 	})
 
 	test('REQ-BUR-004: a version switch resets the session history', async ({ page }) => {
-		// Requires a second seeded ApplicationVersion (e.g. "staging") for
-		// pw-undo-redo, same precondition class as versionRouting.spec.ts's
-		// 9.1/9.3 — skip gracefully rather than fail when it isn't seeded.
-		const versionCheck = await page.request.get(
-			`${BASE_URL}/index.php/apps/openbuild/api/applications/${APP_SLUG}/versions/staging`,
-		).catch(() => null)
-		if (!versionCheck || versionCheck.status() !== 200) {
-			test.skip(true, `SKIP: ApplicationVersion "staging" not seeded for ${APP_SLUG} — seed one to exercise this scenario`)
-			return
-		}
-
-		await page.goto(`${BASE_URL}/apps/openbuild/builder/${APP_SLUG}/pages`, { waitUntil: 'domcontentloaded' })
+		// THIS TEST USED TO SKIP ITSELF, AND THE REASON WAS FALSE.
+		//
+		// It probed `GET .../versions/staging`, found nothing, and skipped with
+		// "ApplicationVersion 'staging' not seeded — seed one to exercise this
+		// scenario". Nothing in CI was ever going to seed it, so the skip was
+		// permanent: a guard on a precondition the suite could satisfy for
+		// itself, phrased as a fact about the environment.
+		//
+		// `tests/e2e/support/versionChain.ts::ensureVersionChain()` provisions
+		// development -> staging -> production on demand and is proven working
+		// in this same job — versionRouting.spec.ts drives `?_version=staging`
+		// through it and passes (9.1 / 9.2 / 9.3, run 31083894467). So the
+		// scenario was drivable all along; it simply never asked.
+		//
+		// A DEDICATED SLUG, not `pw-undo-redo`. The other tests in this describe
+		// open `/pages` with no `?_version=`, so their default-version
+		// resolution depends on how many versions the app has. Growing a chain
+		// on the shared slug would leave them running against a different app
+		// shape on every run AFTER the first — a fixture that changes what its
+		// neighbours test is worse than the skip it replaces.
+		const CHAIN_SLUG = 'pw-undo-redo-chain'
+		await ensureVersionChain(page, CHAIN_SLUG, 'PW Undo Redo Chain')
+
+		await page.goto(`${BASE_URL}/apps/openbuild/builder/${CHAIN_SLUG}/pages`, { waitUntil: 'domcontentloaded' })
 		await expect(page.locator('.page-designer__left')).toBeVisible({ timeout: 15_000 })
 
 		await page.locator('.page-list-editor__add').click()
@@ -264,7 +277,7 @@ test.describe('builder-undo-redo — page designer (REQ-BUR-001..004)', () => {
 		await expect(pageDesignerButton(page, 'Undo')).toBeEnabled()
 
 		await page.goto(
-			`${BASE_URL}/apps/openbuild/builder/${APP_SLUG}/pages?_version=staging`,
+			`${BASE_URL}/apps/openbuild/builder/${CHAIN_SLUG}/pages?_version=staging`,
 			{ waitUntil: 'domcontentloaded' },
 		)
 		await expect(page.locator('.page-designer__left')).toBeVisible({ timeout: 15_000 })