From 253ac05defe916791b300c351bfdb321936f8135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Canouil?= <8896044+mcanouil@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:36:08 +0200 Subject: [PATCH] fix: Apply extension-contributed project metadata in quarto preview Project metadata contributed by an extension through contributes.metadata.project, such as brand, was applied only on the render path. The preview command builds the project context before the render services exist, so the contribution never reached the project configuration and the brand was ignored. The merge now runs in every project context, with a fresh extension context when the render options are absent. Invalid contributed metadata stays fatal for a render, and becomes a warning that names the extension elsewhere, so preview, inspect, publish and serve keep working. Fixes #14783 --- news/changelog-1.11.md | 6 + src/project/project-context.ts | 88 ++++- src/project/types/single-file/single-file.ts | 64 ++-- tests/unit/preview-extension-brand.test.ts | 381 +++++++++++++++++++ 4 files changed, 503 insertions(+), 36 deletions(-) create mode 100644 tests/unit/preview-extension-brand.test.ts diff --git a/news/changelog-1.11.md b/news/changelog-1.11.md index 35f6ee3a280..111741c6e63 100644 --- a/news/changelog-1.11.md +++ b/news/changelog-1.11.md @@ -1,5 +1,11 @@ All changes included in 1.11: +## Commands + +### `quarto preview` + +- ([#14783](https://github.com/quarto-dev/quarto-cli/issues/14783)): Fix `quarto preview` ignoring project metadata contributed by an extension, such as `brand`. (author: @mcanouil) + ## Engines ### `knitr` diff --git a/src/project/project-context.ts b/src/project/project-context.ts index f68be8bb46c..fd842d4c905 100644 --- a/src/project/project-context.ts +++ b/src/project/project-context.ts @@ -107,28 +107,67 @@ import { createProjectCache } from "../core/cache/cache.ts"; import { createTempContext } from "../core/temp.ts"; import { onCleanup } from "../core/cleanup.ts"; +import { warning } from "../deno_ral/log.ts"; +import { ZodError } from "zod"; +import { extensionIdString } from "../extension/extension-shared.ts"; +import { Extension } from "../extension/types.ts"; import { Zod } from "../resources/types/zod/schema-types.ts"; -import { ExternalEngine } from "../resources/types/schema-types.ts"; +import { + ExternalEngine, + ProjectConfig as ContributedProjectConfig, +} from "../resources/types/schema-types.ts"; + +// Turn a validation failure into one line that names the offending keys, so +// the caller can report it without dumping the raw validation error +const projectMetadataError = (extension: Extension, err: unknown) => { + const reason = err instanceof ZodError + ? err.issues.map((issue) => + issue.path.length + ? `${issue.path.join(".")}: ${issue.message}` + : issue.message + ).join("; ") + : err instanceof Error + ? err.message + : String(err); + return new Error( + `The extension ${ + extensionIdString(extension.id) + } contributes invalid project metadata (${reason}).`, + ); +}; -export const mergeExtensionMetadata = async ( +const mergeExtensionMetadata = async ( context: ProjectContext, - pOptions: RenderOptions, + extensionContext: ExtensionContext, + // called with the extension that failed validation; it either throws to + // make the failure fatal, or returns to drop that one contribution + onInvalid: (err: Error) => void, ) => { // this will mutate context.config.project to merge // in any project metadata from extensions if (context.config) { - const extensions = await pOptions.services.extension.extensions( + const extensions = await extensionContext.extensions( undefined, context.config, context.dir, { builtIn: false }, ); - // Handle project metadata extensions - const projectMetadata = extensions.filter((extension) => - extension.contributes.metadata?.project - ).map((extension) => { - return Zod.ProjectConfig.parse(extension.contributes.metadata!.project); - }); + // Handle project metadata extensions, one by one, so that a single + // invalid extension does not discard what the others contribute + const projectMetadata: ContributedProjectConfig[] = []; + for ( + const extension of extensions.filter((extension) => + extension.contributes.metadata?.project + ) + ) { + try { + projectMetadata.push( + Zod.ProjectConfig.parse(extension.contributes.metadata!.project), + ); + } catch (err) { + onInvalid(projectMetadataError(extension, err)); + } + } context.config.project = mergeProjectMetadata( context.config.project, ...projectMetadata, @@ -136,6 +175,23 @@ export const mergeExtensionMetadata = async ( } }; +// Extension metadata is validated against a strict schema, so an invalid +// extension makes the merge fail. That is fatal for a render, as it always +// was, and elsewhere the contribution of that one extension is dropped with +// a warning, so commands such as preview and inspect keep working (#14783). +export const mergeExtensionMetadataForContext = async ( + context: ProjectContext, + extensionContext: ExtensionContext, + fatal: boolean, +) => { + await mergeExtensionMetadata(context, extensionContext, (err) => { + if (fatal) { + throw err; + } + warning(`Ignoring the invalid project metadata. ${err.message}`); + }); +}; + export async function projectContext( path: string, notebookContext: NotebookContext, @@ -176,10 +232,16 @@ export async function projectContext( const returnResult = async ( context: ProjectContext, ) => { - if (renderOptions) { - await mergeExtensionMetadata(context, renderOptions); - } + // Register cleanup before merging: the merge validates extension metadata + // and can throw, and the context already holds an open disk cache onCleanup(context.cleanup); + // Always merge extension metadata so contributions such as `brand` are + // seen even when called without renderOptions (e.g. from preview, #14783) + await mergeExtensionMetadataForContext( + context, + extensionContext, + renderOptions !== undefined, + ); return context; }; diff --git a/src/project/types/single-file/single-file.ts b/src/project/types/single-file/single-file.ts index 22efa8e8d59..823779afb05 100644 --- a/src/project/types/single-file/single-file.ts +++ b/src/project/types/single-file/single-file.ts @@ -12,6 +12,7 @@ import { dirname } from "../../../deno_ral/path.ts"; import { warning } from "../../../deno_ral/log.ts"; +import { warnOnce } from "../../../core/log.ts"; import { normalizePath } from "../../../core/path.ts"; import { NotebookContext } from "../../../render/notebook/notebook-types.ts"; import { makeProjectEnvironmentMemoizer } from "../../project-environment.ts"; @@ -32,7 +33,7 @@ import { createProjectCache } from "../../../core/cache/cache.ts"; import { globalTempContext } from "../../../core/temp.ts"; import { once } from "../../../core/once.ts"; import { - mergeExtensionMetadata, + mergeExtensionMetadataForContext, resolveEngineExtensions, } from "../../project-context.ts"; import { createExtensionContext } from "../../../extension/extension.ts"; @@ -105,32 +106,49 @@ export async function singleFileProjectContext( result.dir, ); - if (renderOptions) { - // Merge extension metadata (requires renderOptions for full services) - await mergeExtensionMetadata(result, renderOptions); - - // Check if extensions contributed output-dir metadata - // If so, set forceClean as if --output-dir specified on command line, - // to ensure proper cleanup - const outputDir = result.config?.project?.["output-dir"]; - if (outputDir) { - const willForceClean = renderOptions.flags?.clean !== false; - warning( - `An extension contributed 'output-dir: ${outputDir}' metadata for single-file render.\n` + - `Output will go to that directory. The temporary .quarto directory will ${ - willForceClean - ? "be cleaned up" - : "NOT be cleaned up (--no-clean specified)" - } after rendering.\n` + - "To suppress this warning, use --output-dir flag instead of extension metadata.", - ); - renderOptions.forceClean = willForceClean; - } - } // because the single-file project is cleaned up with // the global text context, we don't need to register it // in the same way that we need to register the multi-file // projects. + // This is registered before the merge below, because the merge validates + // extension metadata and can throw once the disk cache is already open. temp.onCleanup(result.cleanup); + + // Always merge extension metadata so contributions such as `brand` are + // seen even when called without renderOptions (e.g. from preview, #14783) + await mergeExtensionMetadataForContext( + result, + extensionContext, + renderOptions !== undefined, + ); + + // Warn whenever an extension contributed output-dir metadata, whichever + // command built this context, and on the render path also set forceClean + // as if --output-dir was given on the command line, to ensure proper cleanup + const outputDir = result.config?.project?.["output-dir"]; + if (outputDir) { + const willForceClean = renderOptions + ? renderOptions.flags?.clean !== false + : undefined; + const outcome = willForceClean === undefined + ? "Output will go to that directory when the file is rendered." + : `Output will go to that directory. The temporary .quarto directory will ${ + willForceClean + ? "be cleaned up" + : "NOT be cleaned up (--no-clean specified)" + } after rendering.`; + const message = + `An extension contributed 'output-dir: ${outputDir}' metadata for a single file.\n` + + `${outcome}\n` + + "To suppress this warning, use --output-dir flag instead of extension metadata."; + if (renderOptions && willForceClean !== undefined) { + warning(message); + renderOptions.forceClean = willForceClean; + } else { + // inspect, publish and serve build a context repeatedly, and the user + // cannot act on this at that point, so say it once per process + warnOnce(message); + } + } return result; } diff --git a/tests/unit/preview-extension-brand.test.ts b/tests/unit/preview-extension-brand.test.ts new file mode 100644 index 00000000000..41eb416d59f --- /dev/null +++ b/tests/unit/preview-extension-brand.test.ts @@ -0,0 +1,381 @@ +/* + * preview-extension-brand.test.ts + * + * Tests that project metadata contributed by an extension through + * `contributes.metadata.project` is merged on a ProjectContext built without + * RenderOptions, which is how `quarto preview` builds it, and that the render + * path keeps its own behaviour. + * + * The bug: mergeExtensionMetadata() only ran when renderOptions was present, + * so the contributed `project.brand` never reached project.config.project and + * the brand was silently ignored during preview (#14783). + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { unitTest } from "../test.ts"; +import { assert, assertEquals, assertRejects } from "testing/asserts"; +import { join } from "../../src/deno_ral/path.ts"; +import { LightDarkBrandDarkFlag } from "../../src/core/brand/brand.ts"; +import { singleFileProjectContext } from "../../src/project/types/single-file/single-file.ts"; +import { projectContext } from "../../src/project/project-context.ts"; +import { notebookContext } from "../../src/render/notebook/notebook-context.ts"; +import { renderServices } from "../../src/command/render/render-services.ts"; +import { RenderOptions } from "../../src/command/render/types.ts"; +import { initYamlIntelligenceResourcesFromFilesystem } from "../../src/core/schema/utils.ts"; +import { safeRemoveSync } from "../../src/core/path.ts"; + +const BRAND_YML = `color: + palette: + imperial-red: "#BC1E22" + primary: imperial-red +`; + +const DOCUMENT = "---\ntitle: test\nformat: typst\n---\n"; + +function extensionYml(projectMetadata: string) { + return `title: Test Extension +author: Test Author +version: 1.0.0 +quarto-required: ">=1.4.0" +contributes: + metadata: + project: +${projectMetadata}`; +} + +// Write an extension contributing the given project metadata, plus the brand +// file that metadata may point at, next to the document under test. +function writeExtension( + dir: string, + projectMetadata: string, + name = "test-extension", +) { + const extensionDir = join(dir, "_extensions", name); + Deno.mkdirSync(extensionDir, { recursive: true }); + Deno.writeTextFileSync( + join(extensionDir, "_extension.yml"), + extensionYml(projectMetadata), + ); + Deno.writeTextFileSync(join(extensionDir, "brand.yml"), BRAND_YML); +} + +// The palette entry is unique to the extension's brand.yml, so asserting it +// proves that this file, and not some fallback, was the one resolved. +function assertContributedBrand( + brand: LightDarkBrandDarkFlag | undefined, + context: string, +) { + assert( + brand !== undefined, + `brand contributed by the extension must resolve ${context}`, + ); + assertEquals( + brand.light?.data.color?.palette?.["imperial-red"], + "#BC1E22", + `the resolved brand must be the one contributed by the extension ${context}`, + ); +} + +unitTest( + "projectResolveBrand - single-file: extension-contributed brand resolves without renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, " brand: brand.yml\n"); + + // preview builds the context this way: no renderOptions. + project = await singleFileProjectContext(file, notebookContext()); + + assertContributedBrand(await project.resolveBrand(), "for a single file"); + } finally { + // Release the project's disk cache before removing the dir, otherwise + // Windows holds a lock on the temp directory. + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - project (_quarto.yml present): extension-contributed brand resolves without renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + let project; + + try { + Deno.writeTextFileSync( + join(tmpDir, "_quarto.yml"), + "project:\n type: default\n", + ); + Deno.writeTextFileSync(join(tmpDir, "index.qmd"), DOCUMENT); + writeExtension(tmpDir, " brand: brand.yml\n"); + + // preview builds the context this way: no renderOptions. + project = await projectContext(tmpDir, notebookContext()); + assert( + project !== undefined, + "projectContext must resolve for a _quarto.yml dir", + ); + + assertContributedBrand(await project.resolveBrand(), "for a project"); + } finally { + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - single-file: extension-contributed brand still resolves with renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + const nbContext = notebookContext(); + const services = renderServices(nbContext); + const renderOptions = { services, flags: {} } as RenderOptions; + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, " brand: brand.yml\n"); + + // render builds the context this way: renderOptions present. + project = await singleFileProjectContext(file, nbContext, renderOptions); + + assertContributedBrand(await project.resolveBrand(), "for a render"); + } finally { + project?.cleanup?.(); + services.cleanup(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "singleFileProjectContext: extension-contributed output-dir forces clean on the render path (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + const nbContext = notebookContext(); + const services = renderServices(nbContext); + const renderOptions = { services, flags: {} } as RenderOptions; + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, ' output-dir: "_out"\n'); + + project = await singleFileProjectContext(file, nbContext, renderOptions); + + assertEquals( + project.config?.project?.["output-dir"], + "_out", + "the contributed output-dir must be merged", + ); + assertEquals( + renderOptions.forceClean, + true, + "the render path must force the clean that --output-dir implies", + ); + } finally { + project?.cleanup?.(); + services.cleanup(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "singleFileProjectContext: extension-contributed output-dir merges without renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, ' output-dir: "_out"\n'); + + project = await singleFileProjectContext(file, notebookContext()); + + assertEquals( + project.config?.project?.["output-dir"], + "_out", + "the contributed output-dir must be merged without renderOptions too", + ); + } finally { + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +// One valid key and one key the strict project schema rejects, so a test can +// tell "the whole contribution was dropped" from "it was partially applied". +const MALFORMED_METADATA = ' output-dir: "_out"\n' + + " not-a-project-key: true\n"; + +unitTest( + "singleFileProjectContext: a malformed extension does not break a context built without renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, MALFORMED_METADATA); + + // preview and inspect must degrade to "metadata not applied" here. + project = await singleFileProjectContext(file, notebookContext()); + + assertEquals( + project.config?.project?.["output-dir"], + undefined, + "the whole contribution of a malformed extension must be dropped", + ); + } finally { + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectContext: a malformed extension does not break a context built without renderOptions (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + let project; + + try { + Deno.writeTextFileSync( + join(tmpDir, "_quarto.yml"), + "project:\n type: default\n", + ); + Deno.writeTextFileSync(join(tmpDir, "index.qmd"), DOCUMENT); + writeExtension(tmpDir, MALFORMED_METADATA); + + project = await projectContext(tmpDir, notebookContext()); + assert( + project !== undefined, + "projectContext must still resolve with a malformed extension", + ); + assertEquals( + project.config?.project?.["output-dir"], + undefined, + "the whole contribution of a malformed extension must be dropped", + ); + } finally { + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "singleFileProjectContext: a malformed extension is fatal on the render path (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + const nbContext = notebookContext(); + const services = renderServices(nbContext); + const renderOptions = { services, flags: {} } as RenderOptions; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, MALFORMED_METADATA); + + await assertRejects( + () => singleFileProjectContext(file, nbContext, renderOptions), + Error, + "contributes invalid project metadata", + ); + } finally { + services.cleanup(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectContext: a malformed extension is fatal on the render path (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const nbContext = notebookContext(); + const services = renderServices(nbContext); + const renderOptions = { services, flags: {} } as RenderOptions; + + try { + Deno.writeTextFileSync( + join(tmpDir, "_quarto.yml"), + "project:\n type: default\n", + ); + Deno.writeTextFileSync(join(tmpDir, "index.qmd"), DOCUMENT); + writeExtension(tmpDir, MALFORMED_METADATA); + + await assertRejects( + () => projectContext(tmpDir, nbContext, renderOptions), + Error, + "contributes invalid project metadata", + ); + } finally { + services.cleanup(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +); + +unitTest( + "projectResolveBrand - single-file: one malformed extension does not drop what a valid one contributes (#14783)", + async () => { + await initYamlIntelligenceResourcesFromFilesystem(); + + const tmpDir = Deno.makeTempDirSync({ prefix: "quarto-test" }); + const file = join(tmpDir, "test.qmd"); + let project; + + try { + Deno.writeTextFileSync(file, DOCUMENT); + writeExtension(tmpDir, MALFORMED_METADATA, "broken-extension"); + writeExtension(tmpDir, " brand: brand.yml\n", "brand-extension"); + + project = await singleFileProjectContext(file, notebookContext()); + + assertContributedBrand( + await project.resolveBrand(), + "when a sibling extension is malformed", + ); + assertEquals( + project.config?.project?.["output-dir"], + undefined, + "the contribution of the malformed extension must still be dropped", + ); + } finally { + project?.cleanup?.(); + safeRemoveSync(tmpDir, { recursive: true }); + } + }, +);