From 670ad3c78617f057fb524b9e1be4c4c5d4e88f51 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Tue, 29 Sep 2026 18:01:03 +0900 Subject: [PATCH 1/3] fix(convert): fail --strict on Asciidoctor errors The playbook's runtime.log.failure_level never took effect: the pipeline drives Antora as a library and never configured @antora/logger, so an unresolved include was published with exit 0. convert.ts now configures the logger from the playbook and fails --strict on any ERROR except xrefs into the external components the converter rewrites, and the generated appendix the Boot synthesized eras declare as lost (ADR-0004, ADR-0006). Closes #1044 --- bun.lock | 1 + package.json | 1 + scripts/convert.ts | 186 +++++++++++++++++++++++++++- scripts/lib/antora.d.ts | 20 +++ scripts/lib/upstream-sources.ts | 79 ++++++++++++ tests/integration/pipeline.test.ts | 67 +++++++++- tests/unit/convert.test.ts | 41 +++++- tests/unit/fetch-upstream.test.ts | 1 + tests/unit/upstream-sources.test.ts | 31 +++++ 9 files changed, 423 insertions(+), 4 deletions(-) diff --git a/bun.lock b/bun.lock index 720aa4791..95c466165 100644 --- a/bun.lock +++ b/bun.lock @@ -8,6 +8,7 @@ "@antora/asciidoc-loader": "3.2.0", "@antora/content-aggregator": "3.2.0", "@antora/content-classifier": "3.2.0", + "@antora/logger": "3.2.0", "@antora/playbook-builder": "3.2.0", "@asciidoctor/core": "2.2.8", "@springio/asciidoctor-extensions": "1.0.0-alpha.18", diff --git a/package.json b/package.json index 6a0668906..7a1fa21d7 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "@antora/asciidoc-loader": "3.2.0", "@antora/content-aggregator": "3.2.0", "@antora/content-classifier": "3.2.0", + "@antora/logger": "3.2.0", "@antora/playbook-builder": "3.2.0", "@asciidoctor/core": "2.2.8", "@springio/asciidoctor-extensions": "1.0.0-alpha.18" diff --git a/scripts/convert.ts b/scripts/convert.ts index 811d53ddb..7e0ab7fda 100644 --- a/scripts/convert.ts +++ b/scripts/convert.ts @@ -14,17 +14,20 @@ * * Exit codes: * 0 — every page converted - * 1 — conversion failed, a page produced an error, or `--strict` and a page - * reported a conversion warning + * 1 — conversion failed, a page produced an error, or `--strict` and either a + * page reported a conversion warning or Antora logged a message at the + * playbook's `failure_level` * 2 — bad arguments */ +import type { AcceptedMissing } from './lib/upstream-sources.ts' import { mkdir, rm, writeFile } from 'node:fs/promises' import { dirname, join, resolve } from 'node:path' import process from 'node:process' import loadAsciiDoc from '@antora/asciidoc-loader' import aggregateContent from '@antora/content-aggregator' import classifyContent from '@antora/content-classifier' +import antoraLogger from '@antora/logger' import buildPlaybook from '@antora/playbook-builder' import { componentNameOf } from './lib/component-descriptor.ts' import { convertDocument } from './lib/markdown-converter.ts' @@ -145,9 +148,13 @@ export function playbookFor(source: string, javadocLocation: string, hasCompanio // No `antora.extensions` block: those are loaded by the site generator, which // this pipeline never runs. Declaring them here would be silently ignored. 'runtime:', + // Applied by {@link configureLogger}. `format` is pinned because Antora's + // `auto` picks JSON on stdout whenever stdout is not a TTY, which would + // change the log a local run prints depending on how it is piped. ' log:', ' level: warn', ' failure_level: error', + ' format: pretty', 'urls:', ' latest_version_segment: \'\'', // Required by the playbook schema but never fetched: this pipeline runs the @@ -159,6 +166,152 @@ export function playbookFor(source: string, javadocLocation: string, hasCompanio return `${lines.join('\n')}\n` } +/** The part of a built playbook {@link configureLogger} reads. */ +interface LoggedPlaybook { + readonly dir?: string + readonly runtime: { readonly log: { readonly failureLevel: string } } +} + +/** What Antora logged at or above the playbook's `failure_level`. */ +interface LoggedFailures { + /** Messages that fail a `--strict` build. */ + readonly failures: number + /** Unresolved xrefs into an external component; see {@link externalXrefComponent}. */ + readonly externalXrefs: number + /** Unresolved targets the era declares as accepted losses; see {@link isAcceptedLoss}. */ + readonly acceptedLosses: number +} + +const XREF_NOT_FOUND = 'target of xref not found: ' +const INCLUDE_NOT_FOUND = 'target of include not found: ' + +/** + * The external component an unresolved-xref log message points into, if any. + * + * Antora logs every xref into a component absent from the content catalog as + * `target of xref not found: `, where the id is the author's own + * spec, `[version@][component:][module:][family$]relative[#fragment]`. For a + * component this build deliberately does not aggregate — `externalComponents` + * in upstream-sources.ts — that dangling link is expected: the converter + * rewrites it to the component's published docs.spring.io URL (see + * `rewriteXrefTarget` in inline-html.ts), so the page loses nothing. + * + * The component is taken as the id's first colon-separated segment, which is + * the same segment the converter keys its rewrite on. An id with no colon + * (`attachment$api/java/index.html`) or whose first segment is not external + * (`appendix:…`) names no external component, and stays a failure. So does a + * versioned id (`4.1.1@maven-plugin:…`): the converter's rewrite does not + * recognize the `version@` form, so that link would publish dangling. + */ +export function externalXrefComponent(message: unknown, external: ReadonlySet): string | undefined { + if (typeof message !== 'string' || !message.startsWith(XREF_NOT_FOUND)) + return undefined + let id = message.slice(XREF_NOT_FOUND.length) + const hash = id.indexOf('#') + if (hash !== -1) + id = id.slice(0, hash) + if (id.includes('@')) + return undefined + const colon = id.indexOf(':') + if (colon === -1) + return undefined + const component = id.slice(0, colon).toLowerCase() + return external.has(component) ? component : undefined +} + +/** + * Whether a log message is an unresolved target the era accepts as lost. + * + * A synthesized Boot era cannot rebuild the generated appendix, and ADR-0004 + * and ADR-0006 accept shipping without it; the era declares exactly which + * targets that leaves unresolved (`acceptedMissing` in upstream-sources.ts). + * Only `target of include not found: ` and `target of xref not found: ` + * qualify, and only when `` equals a declared entry or starts with a + * declared entry ending in `/` — so a new missing target, even one beside a + * declared file, still fails the build. + */ +export function isAcceptedLoss(message: unknown, accepted: AcceptedMissing): boolean { + if (typeof message !== 'string') + return false + const [id, entries] = message.startsWith(INCLUDE_NOT_FOUND) + ? [message.slice(INCLUDE_NOT_FOUND.length), accepted.includes] + : message.startsWith(XREF_NOT_FOUND) + ? [message.slice(XREF_NOT_FOUND.length), accepted.xrefs] + : [undefined, []] + if (id === undefined) + return false + return entries.some(entry => entry.endsWith('/') ? id.startsWith(entry) : id === entry) +} + +/** + * Apply the playbook's `runtime.log` to Antora's logger. + * + * The site generator normally does this, and this pipeline never runs it + * (ADR-0002). Without this call the first message Antora logs creates a default + * logger whose failure level is `silent`, so the playbook's `failure_level` + * would be declared and never enforced. + * + * The verdict is this function's own count rather than `finalize()`'s + * `failOnExit`, for two reasons. Antora sets `failOnExit` for every message at + * the failure level, including the external-component xrefs + * {@link externalXrefComponent} exempts, so its boolean cannot tell them apart. + * And it only says *whether*, never *how many*. So `setFailOnExit` is disabled + * and the root logger's own methods at or above the failure level are wrapped + * instead: every Antora component logs through a child of the root, and each + * child's method delegates to its parent's exactly once, so the root's method + * sees each message once however deep the child. (Antora's hook is no + * substitute for a counter anyway: every child re-decorates the inherited + * method, so it fires once per nesting level.) Exempt messages — external + * xrefs and the era's {@link isAcceptedLoss accepted losses} — are still + * logged, only not counted as failures. + * + * Returns the finalizer, which flushes the log and resolves to the counts. + */ +function configureLogger( + playbook: LoggedPlaybook, + externalComponents: Readonly>, + acceptedMissing: AcceptedMissing, +): () => Promise { + antoraLogger.configure(playbook.runtime.log, playbook.dir) + const root = antoraLogger.get(null) + if (!root) + throw new Error('@antora/logger returned no root logger after configure()') + // A message below the log level never reaches the wrappers — Asciidoctor's + // adapter calls `setFailOnExit` for it directly — so such a playbook would + // fail nothing. + if (root.levelVal > root.failureLevelVal) + throw new Error('runtime.log.level must not be above runtime.log.failure_level') + + const external = new Set(Object.keys(externalComponents).map(name => name.toLowerCase())) + let failures = 0 + let externalXrefs = 0 + let acceptedLosses = 0 + root.setFailOnExit = () => {} + const methods = root as unknown as Record void> + for (const [level, value] of Object.entries(root.levels.values)) { + if (value < root.failureLevelVal) + continue + const log = methods[level] + if (typeof log !== 'function') + continue + methods[level] = function (this: unknown, ...args: unknown[]) { + // pino's call shape: `(message)` or `(mergingObject, message)`. + const message = typeof args[0] === 'string' ? args[0] : args[1] + if (externalXrefComponent(message, external) !== undefined) + externalXrefs++ + else if (isAcceptedLoss(message, acceptedMissing)) + acceptedLosses++ + else + failures++ + log.apply(this, args) + } + } + return async () => { + await antoraLogger.finalize() + return { failures, externalXrefs, acceptedLosses } + } +} + async function main(): Promise { let args: Args try { @@ -181,6 +334,14 @@ async function main(): Promise { ) const playbook = buildPlaybook(['--playbook', playbookPath], {}) + const loggedPlaybook = playbook as unknown as LoggedPlaybook + // Before aggregation: Antora's component loggers bind to whatever root + // logger exists when they first log. + const finalizeLogger = configureLogger( + loggedPlaybook, + upstream.externalComponents, + upstream.acceptedMissing, + ) const asciidocConfig = loadAsciiDoc.resolveConfig(playbook) const catalog = classifyContent(playbook, await aggregateContent(playbook), asciidocConfig) // Only the component at the source root is published. A companion's pages @@ -222,6 +383,7 @@ async function main(): Promise { await writeFile(join(outDir, INDEX_FILENAME), buildIndex(args.project, args.version, written)) await rm(playbookPath, { force: true }) + const logged = await finalizeLogger() if (warnings.length > 0) { console.error(`${warnings.length} conversion warning(s):`) @@ -235,6 +397,26 @@ async function main(): Promise { } } + // The messages themselves are already in the log above; these only name + // how many there were, so a long log is not the sole record of them. + const level = loggedPlaybook.runtime.log.failureLevel.toUpperCase() + if (logged.externalXrefs > 0) { + console.error( + `${logged.externalXrefs} ${level} xref(s) to external components, rewritten by the converter; not counted as failures`, + ) + } + if (logged.acceptedLosses > 0) { + console.error( + `${logged.acceptedLosses} ${level}(s) the era declares as accepted losses (ADR-0004); not counted as failures`, + ) + } + if (logged.failures > 0) { + const summary = `${logged.failures} Antora log message(s) at ${level} or above` + if (args.strict) + throw new Error(`${summary} with --strict; see the ${level} lines above`) + console.error(summary) + } + console.log(`Converted ${written.length} files to ${outDir}/`) } catch (error) { diff --git a/scripts/lib/antora.d.ts b/scripts/lib/antora.d.ts index dbd723800..511ddc25d 100644 --- a/scripts/lib/antora.d.ts +++ b/scripts/lib/antora.d.ts @@ -43,6 +43,26 @@ declare module '@antora/asciidoc-loader' { export = loadAsciiDoc } +declare module '@antora/logger' { + /** The root (pino) logger, reduced to what convert.ts counts failures with. */ + interface RootLogger { + /** Numeric value of the configured `level`. */ + readonly levelVal: number + /** Numeric value of the configured `failure_level`; `Infinity` when none. */ + readonly failureLevelVal: number + /** Antora's hook, called by every message at or above the failure level. */ + setFailOnExit: () => void + readonly levels: { readonly values: Readonly> } + } + const logger: { + configure: (options: unknown, baseDir?: string) => unknown + /** Resolves to whether a message reached the configured failure level. */ + finalize: () => Promise + get: (name: null) => RootLogger | undefined + } + export = logger +} + declare module '@asciidoctor/core' { /** Factory returning an Asciidoctor instance. Used by tests to build real AST nodes. */ const Asciidoctor: () => { diff --git a/scripts/lib/upstream-sources.ts b/scripts/lib/upstream-sources.ts index 0ccf4d1a4..b26d74d22 100644 --- a/scripts/lib/upstream-sources.ts +++ b/scripts/lib/upstream-sources.ts @@ -131,6 +131,13 @@ export interface UpstreamCoordinates { * the link useful instead of emitting a fragment that resolves nowhere. */ readonly externalComponents: Readonly> + /** + * Unresolved references this version's era accepts as known losses. + * + * The synthesized era's {@link SynthesisSources.acceptedMissing}; empty for + * every other era, whose content is complete. + */ + readonly acceptedMissing: AcceptedMissing /** * Base URL of this version's published `_images/` directory. * @@ -142,6 +149,22 @@ export interface UpstreamCoordinates { readonly imageBase: string } +/** + * Include and xref targets an era knows it cannot supply, and ships without. + * + * Each entry is an Antora resource id exactly as Asciidoctor logs it in + * `target of include not found: ` / `target of xref not found: `: + * either one exact id, or a prefix ending in `/` that covers a directory of + * generated partials. + */ +export interface AcceptedMissing { + readonly includes: readonly string[] + readonly xrefs: readonly string[] +} + +/** The declaration of an era that loses nothing. */ +const NOTHING_MISSING: AcceptedMissing = { includes: [], xrefs: [] } + /** Where an era's component descriptor and generated content come from. */ export type DescriptorSource = 'archive' | 'synthesized' | 'overlay' | 'template' @@ -179,6 +202,17 @@ export interface SynthesisSources { * dropped under `modules/ROOT/partials//`. */ readonly metadataArtifacts: readonly string[] + /** + * The generated content this reconstruction does not rebuild. + * + * ADR-0004 (3.x) and ADR-0006 (4.0.0-4.0.7) accept that the generated + * appendix — auto-configuration listings, configuration-property tables and + * the other Gradle task outputs — is lost in a synthesized era. Declaring it + * here makes that decision data: `convert.ts --strict` tolerates exactly + * these unresolved targets and still fails on any other, so a loss the ADRs + * did not accept cannot slip in beside them. + */ + readonly acceptedMissing: AcceptedMissing } /** @@ -395,6 +429,35 @@ interface ProjectDefinition { readonly javadocLocationFor: (version: string) => string } +/** + * Generated partials neither synthesized era rebuilds (ADR-0004, ADR-0006). + * + * Measured, not guessed: the exact ids every 3.3.0-4.0.7 release run logged as + * unresolved (2026-09-28 scan of the release logs). A directory prefix stands + * for a family of more than one generated file; a single generated file is + * named exactly. + */ +const BOOT_SYNTHESIS_MISSING_INCLUDES = [ + 'partial$configuration-properties/', + 'partial$deprecated-configuration-properties/', + 'partial$dependency-versions/', + 'partial$slices/documented-slices.adoc', + 'ROOT:example$remote-spring-application.txt', + 'ROOT:partial$application/spring-application.txt', + 'ROOT:partial$logging/logging-format.txt', + 'ROOT:partial$propertydefaults/devtools-property-defaults.adoc', + 'ROOT:partial$starters/', +] as const + +/** + * The generated auto-configuration pages `redirect.adoc` links to, which no + * synthesized era produces (ADR-0004's "4 dangling `xref:appendix:` targets"). + */ +const BOOT_SYNTHESIS_MISSING_XREFS = [ + 'appendix:auto-configuration-classes/spring-boot-actuator-autoconfigure.adoc#appendix.auto-configuration-classes.spring-boot-actuator-autoconfigure', + 'appendix:auto-configuration-classes/spring-boot-autoconfigure.adoc#appendix.auto-configuration-classes.spring-boot-autoconfigure', +] as const + /** * Modules whose jar carries configuration-property metadata in the 3.3-3.5 line. * @@ -753,6 +816,15 @@ const PROJECTS: Readonly> = { gradlePropertiesPath: 'gradle.properties', managedVersionAttributes: BOOT_3_MANAGED_VERSIONS, metadataArtifacts: BOOT_3_METADATA_ARTIFACTS, + acceptedMissing: { + includes: [ + ...BOOT_SYNTHESIS_MISSING_INCLUDES, + // 3.x only: its auto-configuration index includes the generated + // listings through this odd `partial$/` spelling; 4.0.x does not. + 'partial$/auto-configuration-classes/', + ], + xrefs: BOOT_SYNTHESIS_MISSING_XREFS, + }, }, }, }, @@ -783,6 +855,10 @@ const PROJECTS: Readonly> = { gradlePropertiesPath: 'gradle.properties', managedVersionAttributes: BOOT_4_MANAGED_VERSIONS, metadataArtifacts: BOOT_4_METADATA_ARTIFACTS, + acceptedMissing: { + includes: BOOT_SYNTHESIS_MISSING_INCLUDES, + xrefs: BOOT_SYNTHESIS_MISSING_XREFS, + }, }, }, }, @@ -1132,6 +1208,9 @@ export function resolveUpstream(project: string, version: string): UpstreamCoord checkoutPaths: checkoutPathsFor(era), javadocLocation: definition.javadocLocationFor(version), externalComponents: definition.externalComponentsFor(version), + acceptedMissing: era.assembly.descriptor === 'synthesized' + ? era.assembly.synthesis.acceptedMissing + : NOTHING_MISSING, imageBase: definition.imageBaseFor(version), } } diff --git a/tests/integration/pipeline.test.ts b/tests/integration/pipeline.test.ts index 3ab54a527..fb498cb0b 100644 --- a/tests/integration/pipeline.test.ts +++ b/tests/integration/pipeline.test.ts @@ -59,7 +59,12 @@ async function initRepo(dir: string) { ) } -async function convert(outDir: string, source = join(work, 'src'), project = PROJECT) { +async function convert( + outDir: string, + source = join(work, 'src'), + project = PROJECT, + flags: readonly string[] = [], +) { return run( [ 'bun', @@ -72,6 +77,7 @@ async function convert(outDir: string, source = join(work, 'src'), project = PRO VERSION, '--out', outDir, + ...flags, ], REPO_ROOT, ) @@ -244,6 +250,65 @@ describe('convert.ts over a store with a companion component', () => { }) }) +describe('convert.ts --strict over Asciidoctor ERRORs', () => { + // Asciidoctor logs a missing include at ERROR and renders an "Unresolved + // include directive" line in its place, so the page still converts. Only the + // playbook's `failure_level`, once applied, turns that lost content into a + // failed build (#1044). The one ERROR it tolerates is an xref into a + // component `boot` declares external, which the converter rewrites. + + /** A single-page `boot` component whose page body is `body`. */ + async function fixture(name: string, body: string): Promise { + const source = join(work, `${name}-src`) + await mkdir(join(source, 'modules', 'ROOT', 'pages'), { recursive: true }) + await writeFile(join(source, 'antora.yml'), `name: ${PROJECT}\nversion: ${VERSION}\ntitle: Fixture\n`) + await writeFile(join(source, 'modules', 'ROOT', 'pages', 'index.adoc'), `= Fixture\n\n${body}\n`) + await initRepo(source) + return source + } + + let missingInclude: string + + beforeAll(async () => { + missingInclude = await fixture('missing-include', 'include::partial$absent.adoc[]') + }, TIMEOUT) + + test('--strict exits 1 on a missing include and names the ERROR', async () => { + const result = await convert(join(work, 'missing-include-strict'), missingInclude, PROJECT, ['--strict']) + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain('target of include not found') + expect(result.stderr).toContain('1 Antora log message(s) at ERROR or above with --strict') + expect(result.stderr).not.toContain('logger not configured') + }, TIMEOUT) + + test('without --strict the page converts and the ERROR is still reported', async () => { + const result = await convert(join(work, 'missing-include-lenient'), missingInclude) + expect(result.exitCode).toBe(0) + expect(result.stderr).toContain('target of include not found') + expect(result.stderr).toContain('1 Antora log message(s) at ERROR or above') + }, TIMEOUT) + + test('--strict tolerates an xref into an external component, which the converter rewrites', async () => { + const source = await fixture('external-xref', 'See xref:maven-plugin:index.adoc[the Maven plugin].') + const out = join(work, 'external-xref-out') + const result = await convert(out, source, PROJECT, ['--strict']) + expect(result.exitCode).toBe(0) + expect(result.stderr).toContain('target of xref not found: maven-plugin:index.adoc') + expect(result.stderr).toContain('1 ERROR xref(s) to external components, rewritten by the converter') + expect(await readFile(join(out, NAME, 'index.md'), 'utf8')).toContain( + `[the Maven plugin](https://docs.spring.io/spring-boot/${VERSION}/maven-plugin/index.html)`, + ) + }, TIMEOUT) + + test('--strict exits 1 on an xref into a component that is not external', async () => { + const source = await fixture('missing-xref', 'See xref:appendix:absent.adoc[a missing page].') + const result = await convert(join(work, 'missing-xref-out'), source, PROJECT, ['--strict']) + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain('target of xref not found: appendix:absent.adoc') + expect(result.stderr).toContain('1 Antora log message(s) at ERROR or above with --strict') + }, TIMEOUT) +}) + describe('package-release.ts over a converted tree', () => { let out: string let archive: string diff --git a/tests/unit/convert.test.ts b/tests/unit/convert.test.ts index a72874032..8b982782f 100644 --- a/tests/unit/convert.test.ts +++ b/tests/unit/convert.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test' -import { parseArgs, playbookFor } from '../../scripts/convert.ts' +import { externalXrefComponent, isAcceptedLoss, parseArgs, playbookFor } from '../../scripts/convert.ts' describe('parseArgs', () => { test('parses the required options with a separate-value form and defaults strict to false', () => { @@ -58,3 +58,42 @@ describe('playbookFor', () => { expect(yaml.content.sources).toEqual([{ url: '/src', branches: 'HEAD' }]) }) }) + +describe('externalXrefComponent', () => { + const external = new Set(['maven-plugin', 'api']) + + test('names the external component an unresolved xref points into', () => { + expect(externalXrefComponent('target of xref not found: maven-plugin:index.adoc', external)).toBe('maven-plugin') + expect(externalXrefComponent('target of xref not found: api::rest/index.adoc#x', external)).toBe('api') + }) + + test('is undefined for a component that is not external, or no component at all', () => { + expect(externalXrefComponent('target of xref not found: appendix:absent.adoc', external)).toBeUndefined() + expect(externalXrefComponent('target of xref not found: attachment$api/java/index.html', external)).toBeUndefined() + expect(externalXrefComponent('target of include not found: maven-plugin:index.adoc', external)).toBeUndefined() + }) + + test('does not exempt a versioned id, which the converter leaves dangling', () => { + expect(externalXrefComponent('target of xref not found: 4.1.1@maven-plugin:index.adoc', external)).toBeUndefined() + }) +}) + +describe('isAcceptedLoss', () => { + const accepted = { + includes: ['partial$configuration-properties/', 'ROOT:partial$logging/logging-format.txt'], + xrefs: ['appendix:auto.adoc#auto'], + } + + test('accepts an id under a declared directory prefix or equal to a declared id', () => { + expect(isAcceptedLoss('target of include not found: partial$configuration-properties/web.adoc', accepted)).toBe(true) + expect(isAcceptedLoss('target of include not found: ROOT:partial$logging/logging-format.txt', accepted)).toBe(true) + expect(isAcceptedLoss('target of xref not found: appendix:auto.adoc#auto', accepted)).toBe(true) + }) + + test('rejects a sibling of a declared file, a lookalike prefix, and the wrong message kind', () => { + expect(isAcceptedLoss('target of include not found: ROOT:partial$logging/other.txt', accepted)).toBe(false) + expect(isAcceptedLoss('target of include not found: partial$configuration-properties-extra.adoc', accepted)).toBe(false) + expect(isAcceptedLoss('target of xref not found: partial$configuration-properties/web.adoc', accepted)).toBe(false) + expect(isAcceptedLoss('target of include not found: appendix:auto.adoc#auto', accepted)).toBe(false) + }) +}) diff --git a/tests/unit/fetch-upstream.test.ts b/tests/unit/fetch-upstream.test.ts index f23d3f16d..5fda84bf1 100644 --- a/tests/unit/fetch-upstream.test.ts +++ b/tests/unit/fetch-upstream.test.ts @@ -81,6 +81,7 @@ describe('copyExamples', () => { gradlePropertiesPath: 'gradle.properties', managedVersionAttributes: [], metadataArtifacts: [], + acceptedMissing: { includes: [], xrefs: [] }, } test('copies the examples tree in under modules/ROOT/examples', async () => { diff --git a/tests/unit/upstream-sources.test.ts b/tests/unit/upstream-sources.test.ts index 040e2d10c..a4ad93c51 100644 --- a/tests/unit/upstream-sources.test.ts +++ b/tests/unit/upstream-sources.test.ts @@ -272,6 +272,37 @@ describe('resolveUpstream layout eras', () => { // An archive era needs nothing beyond the component root. expect(upstream.checkoutPaths).toEqual(['documentation/spring-boot-docs/src/docs/antora']) }) + test('both synthesized eras declare the generated appendix as an accepted loss', () => { + // ADR-0004 and ADR-0006: the reconstruction cannot rebuild the appendix, so + // its unresolved includes and xrefs are declared rather than tolerated blind. + for (const version of ['3.3.0', '3.5.16', '4.0.0', '4.0.7']) { + const { acceptedMissing } = resolveUpstream('boot', version) + expect(acceptedMissing.includes).toContain('partial$configuration-properties/') + expect(acceptedMissing.includes).toContain('ROOT:partial$starters/') + expect(acceptedMissing.xrefs).toContain( + 'appendix:auto-configuration-classes/spring-boot-autoconfigure.adoc#appendix.auto-configuration-classes.spring-boot-autoconfigure', + ) + } + // Only 3.x logs this include; 4.0.x must not inherit the exemption. + expect(resolveUpstream('boot', '3.5.16').acceptedMissing.includes) + .toContain('partial$/auto-configuration-classes/') + expect(resolveUpstream('boot', '4.0.7').acceptedMissing.includes) + .not + .toContain('partial$/auto-configuration-classes/') + }) + + test('every non-synthesized era declares no accepted loss', () => { + const complete = [ + ['boot', '4.0.8'], + ['boot', '4.1.1'], + ['framework', '6.2.0'], + ['security', '7.1.1'], + ['ai', '1.0.0'], + ['data-relational', '3.2.9'], + ] as const + for (const [project, version] of complete) + expect(resolveUpstream(project, version).acceptedMissing).toEqual({ includes: [], xrefs: [] }) + }) }) describe('metadataJars', () => { From aa103b793487f3eea659b01f1bb6835b50b05623 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Wed, 30 Sep 2026 10:47:53 +0900 Subject: [PATCH 2/3] fix(convert): flush the logger on failure and report counts before throwing --- scripts/convert.ts | 39 +++++++++++++++++++++++++++----------- tests/unit/convert.test.ts | 5 +++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/scripts/convert.ts b/scripts/convert.ts index 7e0ab7fda..2e98d340d 100644 --- a/scripts/convert.ts +++ b/scripts/convert.ts @@ -228,7 +228,8 @@ export function externalXrefComponent(message: unknown, external: ReadonlySet` and `target of xref not found: ` * qualify, and only when `` equals a declared entry or starts with a * declared entry ending in `/` — so a new missing target, even one beside a - * declared file, still fails the build. + * declared file, still fails the build. A prefix match must also stay inside + * the declared directory: a remainder with a `..` segment could name any file. */ export function isAcceptedLoss(message: unknown, accepted: AcceptedMissing): boolean { if (typeof message !== 'string') @@ -240,7 +241,9 @@ export function isAcceptedLoss(message: unknown, accepted: AcceptedMissing): boo : [undefined, []] if (id === undefined) return false - return entries.some(entry => entry.endsWith('/') ? id.startsWith(entry) : id === entry) + return entries.some(entry => entry.endsWith('/') + ? id.startsWith(entry) && !id.slice(entry.length).split('/').includes('..') + : id === entry) } /** @@ -265,7 +268,9 @@ export function isAcceptedLoss(message: unknown, accepted: AcceptedMissing): boo * xrefs and the era's {@link isAcceptedLoss accepted losses} — are still * logged, only not counted as failures. * - * Returns the finalizer, which flushes the log and resolves to the counts. + * Returns the finalizer, which flushes the log and resolves to the counts. It + * is safe to call from both the success and the failure path: every call after + * the first returns the first call's promise, so the logger is finalized once. */ function configureLogger( playbook: LoggedPlaybook, @@ -306,10 +311,9 @@ function configureLogger( log.apply(this, args) } } - return async () => { - await antoraLogger.finalize() - return { failures, externalXrefs, acceptedLosses } - } + let finalized: Promise | undefined + return () => (finalized ??= antoraLogger.finalize() + .then(() => ({ failures, externalXrefs, acceptedLosses }))) } async function main(): Promise { @@ -324,6 +328,10 @@ async function main(): Promise { const source = resolve(process.cwd(), args.source) const outDir = resolve(process.cwd(), args.out, `${args.project}-${args.version}`) + // Outside the `try` so the failure path can flush it too: the pretty log + // format writes through an async stream, and exiting without `finalize()` + // can drop the very ERROR lines that explain the failure. + let finalizeLogger: (() => Promise) | undefined try { const upstream = resolveUpstream(args.project, args.version) @@ -337,7 +345,7 @@ async function main(): Promise { const loggedPlaybook = playbook as unknown as LoggedPlaybook // Before aggregation: Antora's component loggers bind to whatever root // logger exists when they first log. - const finalizeLogger = configureLogger( + finalizeLogger = configureLogger( loggedPlaybook, upstream.externalComponents, upstream.acceptedMissing, @@ -385,13 +393,16 @@ async function main(): Promise { await rm(playbookPath, { force: true }) const logged = await finalizeLogger() + // Every summary is printed before either `--strict` condition throws, so a + // run that fails on one still reports the counts of the other. + const strictFailures: string[] = [] if (warnings.length > 0) { console.error(`${warnings.length} conversion warning(s):`) for (const warning of warnings.slice(0, 20)) console.error(` - ${warning}`) if (warnings.length > 20) console.error(` … and ${warnings.length - 20} more`) if (args.strict) { - throw new Error( + strictFailures.push( `${warnings.length} conversion warning(s) with --strict; add a conversion rule for each construct above`, ) } @@ -413,13 +424,19 @@ async function main(): Promise { if (logged.failures > 0) { const summary = `${logged.failures} Antora log message(s) at ${level} or above` if (args.strict) - throw new Error(`${summary} with --strict; see the ${level} lines above`) - console.error(summary) + strictFailures.push(`${summary} with --strict; see the ${level} lines above`) + else + console.error(summary) } + if (strictFailures.length > 0) + throw new Error(strictFailures.join('; and ')) console.log(`Converted ${written.length} files to ${outDir}/`) } catch (error) { + // A no-op when the success path already finalized; a flush failure must + // not mask the error being reported. + await finalizeLogger?.().catch(() => undefined) console.error(`✗ convert failed: ${error instanceof Error ? error.message : String(error)}`) process.exit(1) } diff --git a/tests/unit/convert.test.ts b/tests/unit/convert.test.ts index 8b982782f..164c59f8d 100644 --- a/tests/unit/convert.test.ts +++ b/tests/unit/convert.test.ts @@ -96,4 +96,9 @@ describe('isAcceptedLoss', () => { expect(isAcceptedLoss('target of xref not found: partial$configuration-properties/web.adoc', accepted)).toBe(false) expect(isAcceptedLoss('target of include not found: appendix:auto.adoc#auto', accepted)).toBe(false) }) + + test('rejects a prefix match that climbs out of the declared directory', () => { + expect(isAcceptedLoss('target of include not found: partial$configuration-properties/../other.adoc', accepted)).toBe(false) + expect(isAcceptedLoss('target of include not found: partial$configuration-properties/a/../../b.adoc', accepted)).toBe(false) + }) }) From a34867c12ab8d4c704d570722dc159e58bd4bdec Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Wed, 30 Sep 2026 10:56:55 +0900 Subject: [PATCH 3/3] fix(convert): exempt only external xrefs the converter rewrites --- scripts/convert.ts | 55 +++++++++++++++++++++++------- scripts/lib/inline-html.ts | 24 +++++++++++-- scripts/lib/markdown-converter.ts | 14 +++++++- tests/integration/pipeline.test.ts | 15 ++++++++ tests/unit/convert.test.ts | 5 +++ tests/unit/inline-html.test.ts | 14 ++++++++ 6 files changed, 110 insertions(+), 17 deletions(-) diff --git a/scripts/convert.ts b/scripts/convert.ts index 2e98d340d..f1a9caa35 100644 --- a/scripts/convert.ts +++ b/scripts/convert.ts @@ -176,8 +176,13 @@ interface LoggedPlaybook { interface LoggedFailures { /** Messages that fail a `--strict` build. */ readonly failures: number - /** Unresolved xrefs into an external component; see {@link externalXrefComponent}. */ - readonly externalXrefs: number + /** + * Ids of unresolved xrefs into an external component, one per message; see + * {@link externalXrefComponent}. Only candidates: whether each is exempt + * depends on whether the converter rewrote it, known once every page is + * converted. + */ + readonly externalXrefIds: readonly string[] /** Unresolved targets the era declares as accepted losses; see {@link isAcceptedLoss}. */ readonly acceptedLosses: number } @@ -200,8 +205,15 @@ const INCLUDE_NOT_FOUND = 'target of include not found: ' * the same segment the converter keys its rewrite on. An id with no colon * (`attachment$api/java/index.html`) or whose first segment is not external * (`appendix:…`) names no external component, and stays a failure. So does a - * versioned id (`4.1.1@maven-plugin:…`): the converter's rewrite does not - * recognize the `version@` form, so that link would publish dangling. + * versioned id (`4.1.1@maven-plugin:…`, an `@` before the first colon): the + * converter's rewrite does not recognize the `version@` form, so that link + * would publish dangling. An `@` after the colon is part of the page path + * (`maven-plugin:page@2x.adoc`), which the converter does rewrite. + * + * Naming an external component makes a message a candidate only: `convert.ts` + * exempts it once the converter has actually rewritten that reference, since a + * reference emitted verbatim (a `[literal]` block with `subs=+macros`) is + * logged the same way but never reaches the rewrite. */ export function externalXrefComponent(message: unknown, external: ReadonlySet): string | undefined { if (typeof message !== 'string' || !message.startsWith(XREF_NOT_FOUND)) @@ -210,11 +222,11 @@ export function externalXrefComponent(message: unknown, external: ReadonlySet name.toLowerCase())) let failures = 0 - let externalXrefs = 0 + const externalXrefIds: string[] = [] let acceptedLosses = 0 root.setFailOnExit = () => {} const methods = root as unknown as Record void> @@ -303,7 +315,7 @@ function configureLogger( // pino's call shape: `(message)` or `(mergingObject, message)`. const message = typeof args[0] === 'string' ? args[0] : args[1] if (externalXrefComponent(message, external) !== undefined) - externalXrefs++ + externalXrefIds.push((message as string).slice(XREF_NOT_FOUND.length)) else if (isAcceptedLoss(message, acceptedMissing)) acceptedLosses++ else @@ -313,7 +325,7 @@ function configureLogger( } let finalized: Promise | undefined return () => (finalized ??= antoraLogger.finalize() - .then(() => ({ failures, externalXrefs, acceptedLosses }))) + .then(() => ({ failures, externalXrefIds, acceptedLosses }))) } async function main(): Promise { @@ -369,6 +381,12 @@ async function main(): Promise { const warnings: string[] = [] const written: string[] = [] + // Across the run, not per page: a deliberate simplification. A reference + // rewritten on one page and emitted verbatim on another is exempt on both, + // since its target is reachable from the page that rewrote it. Matching per + // page would mean mapping each log record's `file` — the included partial, + // with the page somewhere up its `stack` — back to the page converted. + const rewrittenXrefs = new Set() for (const page of pages) { const componentVersion = catalog.getComponentVersion(page.src.component, page.src.version) @@ -381,6 +399,7 @@ async function main(): Promise { }) for (const warning of result.warnings) warnings.push(`${sourcePath}: ${warning}`) + for (const id of result.externalXrefs) rewrittenXrefs.add(id) const relativeOut = outputPathFor(page) const target = join(outDir, relativeOut) @@ -411,9 +430,18 @@ async function main(): Promise { // The messages themselves are already in the log above; these only name // how many there were, so a long log is not the sole record of them. const level = loggedPlaybook.runtime.log.failureLevel.toUpperCase() - if (logged.externalXrefs > 0) { + const rewritten = logged.externalXrefIds.filter(id => rewrittenXrefs.has(id)) + const unrewritten = logged.externalXrefIds.filter(id => !rewrittenXrefs.has(id)) + if (rewritten.length > 0) { + console.error( + `${rewritten.length} ${level} xref(s) to external components, rewritten by the converter; not counted as failures`, + ) + } + if (unrewritten.length > 0) { + const named = [...new Set(unrewritten)] console.error( - `${logged.externalXrefs} ${level} xref(s) to external components, rewritten by the converter; not counted as failures`, + `${unrewritten.length} ${level} xref(s) to external components the converter never rewrote, so they ` + + `publish dangling: ${named.slice(0, 5).join(', ')}${named.length > 5 ? `, … and ${named.length - 5} more` : ''}`, ) } if (logged.acceptedLosses > 0) { @@ -421,8 +449,9 @@ async function main(): Promise { `${logged.acceptedLosses} ${level}(s) the era declares as accepted losses (ADR-0004); not counted as failures`, ) } - if (logged.failures > 0) { - const summary = `${logged.failures} Antora log message(s) at ${level} or above` + const failures = logged.failures + unrewritten.length + if (failures > 0) { + const summary = `${failures} Antora log message(s) at ${level} or above` if (args.strict) strictFailures.push(`${summary} with --strict; see the ${level} lines above`) else diff --git a/scripts/lib/inline-html.ts b/scripts/lib/inline-html.ts index d2ef9f697..cfd3359c2 100644 --- a/scripts/lib/inline-html.ts +++ b/scripts/lib/inline-html.ts @@ -406,14 +406,23 @@ function escapeText( * Rewrite a resolved Antora xref target for the Markdown tree. * * Antora resolves `xref:` to a `.html` URL that is already relative to the - * current page, so only the extension changes — never the path. + * current page, so only the extension changes — never the path. A dangling + * reference into an external component is rewritten to its published URL, and + * `onExternalXref` is told which one. */ -function rewriteXrefTarget(href: string, externalComponents: Readonly>): string { +function rewriteXrefTarget( + href: string, + externalComponents: Readonly>, + onExternalXref?: (id: string) => void, +): string { const dangling = DANGLING_COMPONENT.exec(href) if (dangling !== null) { const base = externalComponents[(dangling[1] ?? '').toLowerCase()] if (base === undefined) return href + // Antora renders an unresolved xref as `'#' + refSpec`, and logs that same + // refSpec, so this is the id exactly as the log line names it. + onExternalXref?.(href.slice(1)) const path = (dangling[2] ?? '').replace(ADOC_EXTENSION, '.html') return `${base}/${path}${dangling[3] ?? ''}` } @@ -454,6 +463,14 @@ export interface InlineOptions { * guessing a pairing instead would ship a silently wrong one. */ readonly onUnpairedStem?: (run: string) => void + /** + * Called with the Antora resource id of each dangling reference rewritten to + * an {@link externalComponents external component}'s published URL. + * + * `convert.ts` exempts an external xref's `target of xref not found` ERROR + * only when the converter really rewrote that reference; this is its record. + */ + readonly onExternalXref?: (id: string) => void } /** @@ -489,6 +506,7 @@ export function inlineHtmlToMarkdown(html: string, options: InlineOptions = {}): imageBase, onImageWithoutBase, onUnpairedStem, + onExternalXref, } = options let out = '' const emit = (text: string): void => { @@ -525,7 +543,7 @@ export function inlineHtmlToMarkdown(html: string, options: InlineOptions = {}): } const classes = (node.attrs.get('class') ?? '').split(CLASS_SEPARATOR) const target = classes.includes('xref') - ? rewriteXrefTarget(href, externalComponents) + ? rewriteXrefTarget(href, externalComponents, onExternalXref) : href emit('[') walk(node.children) diff --git a/scripts/lib/markdown-converter.ts b/scripts/lib/markdown-converter.ts index b176c32aa..7a9a09a44 100644 --- a/scripts/lib/markdown-converter.ts +++ b/scripts/lib/markdown-converter.ts @@ -70,6 +70,12 @@ export interface ConvertResult { readonly markdown: string /** Unhandled constructs, deduplicated. Never empty when something was dropped. */ readonly warnings: readonly string[] + /** + * Antora resource ids of the dangling references rewritten to an external + * component's published URL, deduplicated. A reference the converter emits + * verbatim — inside a literal block, say — is not among them. + */ + readonly externalXrefs: readonly string[] } /** AsciiDoc admonition styles mapped to their GFM alert keyword. */ @@ -176,6 +182,7 @@ function flattenForCell(text: string): string { */ export function convertDocument(doc: AsciidoctorNode, options: ConvertOptions): ConvertResult { const warnings = new Set() + const externalXrefs = new Set() /** * A one-line, bounded excerpt of a text run, for naming it in a warning. @@ -210,6 +217,7 @@ export function convertDocument(doc: AsciidoctorNode, options: ConvertOptions): onImageWithoutBase: src => warnings.add(`inline image "${src}" dropped: no published image base for this project`), onUnknownTag: tag => warnings.add(`unknown inline tag <${tag}>`), + onExternalXref: id => externalXrefs.add(id), onUnpairedStem: run => warnings.add( `stem:[…] delimiters do not pair up, so the run was left escaped: "${excerpt(run)}"`, @@ -572,5 +580,9 @@ export function convertDocument(doc: AsciidoctorNode, options: ConvertOptions): ...renderBlocks(doc.getBlocks()), ] - return { markdown: `${chunks.join('\n\n')}\n`, warnings: [...warnings] } + return { + markdown: `${chunks.join('\n\n')}\n`, + warnings: [...warnings], + externalXrefs: [...externalXrefs], + } } diff --git a/tests/integration/pipeline.test.ts b/tests/integration/pipeline.test.ts index fb498cb0b..3954a5525 100644 --- a/tests/integration/pipeline.test.ts +++ b/tests/integration/pipeline.test.ts @@ -300,6 +300,21 @@ describe('convert.ts --strict over Asciidoctor ERRORs', () => { ) }, TIMEOUT) + test('--strict exits 1 on an external xref the converter emits verbatim rather than rewrites', async () => { + // `subs=+macros` makes Asciidoctor resolve the xref, so Antora logs it like + // any other, but a literal block is fenced as-is and never rewritten. + const source = await fixture( + 'literal-xref', + '[literal,subs="+macros"]\n....\nxref:maven-plugin:index.adoc[x]\n....', + ) + const result = await convert(join(work, 'literal-xref-out'), source, PROJECT, ['--strict']) + expect(result.exitCode).toBe(1) + expect(result.stderr).toContain('1 ERROR xref(s) to external components the converter never rewrote') + expect(result.stderr).toContain('maven-plugin:index.adoc') + expect(result.stderr).toContain('1 Antora log message(s) at ERROR or above with --strict') + expect(result.stderr).not.toContain('rewritten by the converter; not counted') + }, TIMEOUT) + test('--strict exits 1 on an xref into a component that is not external', async () => { const source = await fixture('missing-xref', 'See xref:appendix:absent.adoc[a missing page].') const result = await convert(join(work, 'missing-xref-out'), source, PROJECT, ['--strict']) diff --git a/tests/unit/convert.test.ts b/tests/unit/convert.test.ts index 164c59f8d..b2a742111 100644 --- a/tests/unit/convert.test.ts +++ b/tests/unit/convert.test.ts @@ -75,6 +75,11 @@ describe('externalXrefComponent', () => { test('does not exempt a versioned id, which the converter leaves dangling', () => { expect(externalXrefComponent('target of xref not found: 4.1.1@maven-plugin:index.adoc', external)).toBeUndefined() + expect(externalXrefComponent('target of xref not found: 4.1.1@maven-plugin:x.adoc', external)).toBeUndefined() + }) + + test('reads an @ after the first colon as part of the page path, not a version', () => { + expect(externalXrefComponent('target of xref not found: maven-plugin:page@2x.adoc', external)).toBe('maven-plugin') }) }) diff --git a/tests/unit/inline-html.test.ts b/tests/unit/inline-html.test.ts index 1d1e37861..b51f8ae9a 100644 --- a/tests/unit/inline-html.test.ts +++ b/tests/unit/inline-html.test.ts @@ -66,6 +66,20 @@ describe('inlineHtmlToMarkdown', () => { expect(result).toBe('[build](https://docs.example/maven-plugin/build-image.html#build-image)') }) + test('reports each external reference it rewrites by its Antora resource id', () => { + const seen: string[] = [] + inlineHtmlToMarkdown( + 'build ' + + 'x', + { + externalComponents: { 'maven-plugin': 'https://docs.example/maven-plugin' }, + onExternalXref: id => seen.push(id), + }, + ) + + expect(seen).toEqual(['maven-plugin:build-image.adoc#build-image']) + }) + test('leaves a dangling component reference alone when it has no mapping', () => { expect( inlineHtmlToMarkdown('x'),