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 }); + } + }, +);