fix(scripts): only publish images the repository owns, and keep their suffix

Review found four real gaps in the image placement this PR introduced.

Link rewriting only needs a target to exist, but publication copies its bytes
onto the site: a reference reaching out of the tree through `../..` or a
symlink would put a build-machine file on a published page. Only a regular
file whose real path stays inside the repository is copied now, and anything
else fails the projection naming the page and the target.

A placed reference kept none of its `?query` or `#fragment`, which the GitHub
branch has always carried and which decides what an SVG view fragment or a
Vite query means. The suffix rides along again, and the file name is
percent-encoded because the destination is a Markdown inline target.

Page outputs and placed images now claim projected paths from one map, so the
"fail loud rather than overwrite" invariant covers a page and an image landing
on one path, not only two images. `docsSourceFiles()` reports placed images, so
replacing a screenshot re-projects under `docs:dev` instead of serving the
previous copy until something touches the page.

The guide said to set `agent-loop`'s `agents` to change the default model,
which does nothing for `dsh web`: that default is `api-gateway`'s, and the
shipped composition leaves `agents` empty. It also promised that a catalog
provider needs only an API key, which is false for Bedrock, Vertex, Azure, and
Codex. Both are corrected.

The projection note and the doc-site skill carried the superseded "a
repository image becomes a raw GitHub URL" rule; both now describe what ships.
This commit is contained in:
Yichen Jiang
2026-08-06 21:14:39 +08:00
parent c826966181
commit a48b84c001
12 changed files with 178 additions and 50 deletions

View File

@@ -1,12 +1,14 @@
/** Tests for the documentation website projection adapter. */
import { execFileSync } from 'node:child_process'
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join, resolve } from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
import { docsPages, type DocsPage } from '../website/docs.ts'
import { addProjectionFrontmatter, projectedPageContent, rewriteMarkdown } from './project-doc-site.ts'
import {
addProjectionFrontmatter, projectedPageContent, publishableImage, rewriteMarkdown,
} from './project-doc-site.ts'
const roots: string[] = []
const repositoryRoot = resolve(import.meta.dirname, '..')
@@ -63,6 +65,32 @@ describe('website source layout', () => {
})
})
describe('publishableImage', () => {
it('accepts a regular file inside the repository', () => {
const { root } = fixture()
const real = realpathSync(join(root, 'packages/logo.svg'))
expect(publishableImage(join(root, 'packages/logo.svg'), realpathSync(root))).toBe(real)
})
it('refuses a target whose real path escapes the repository', () => {
// Publication copies the bytes onto the site, so a reference reaching a
// build-machine file must not be treated as an image the repository owns.
const { root } = fixture()
const outside = mkdtempSync(join(tmpdir(), 'dsh-doc-site-outside-'))
roots.push(outside)
writeFileSync(join(outside, 'secret.png'), 'not really a png\n')
symlinkSync(join(outside, 'secret.png'), join(root, 'packages/linked.png'))
expect(publishableImage(join(root, 'packages/linked.png'), realpathSync(root))).toBeUndefined()
expect(publishableImage(join(outside, 'secret.png'), realpathSync(root))).toBeUndefined()
})
it('refuses a directory', () => {
const { root } = fixture()
expect(publishableImage(join(root, 'packages'), realpathSync(root))).toBeUndefined()
})
})
describe('rewriteMarkdown', () => {
it('maps published pages and pins unpublished source links', () => {
const { root, pages } = fixture()
@@ -107,7 +135,9 @@ describe('rewriteMarkdown', () => {
it('hands an image to the placer and uses the URL it returns', () => {
// A raw GitHub URL cannot serve a private repository, so the site build
// carries images itself; the placer is what puts them there.
// carries images itself; the placer is what puts them there. The stand-in
// derives its URL the way the real one does, so a placer that stopped
// returning the basename would fail here rather than pass on a constant.
const { root, pages } = fixture()
const placed: string[] = []
expect(rewriteMarkdown('![logo](../packages/logo.svg)\n', {
@@ -118,13 +148,29 @@ describe('rewriteMarkdown', () => {
repoRoot: root,
repositoryRef: 'abc123',
placeImage: (absPath) => {
placed.push(absPath.split('/').pop() ?? '')
return './logo.svg'
const name = absPath.split('/').pop() ?? ''
placed.push(name)
return `./${name}`
},
})).toBe('![logo](./logo.svg)\n')
expect(placed).toEqual(['logo.svg'])
})
it('keeps a placed image\u2019s query or fragment', () => {
// An SVG view fragment and a Vite query both change what the reference
// means, and the GitHub branch has always carried them.
const { root, pages } = fixture()
expect(rewriteMarkdown('![logo](../packages/logo.svg#view)\n', {
locale: 'en',
sourcePath: 'docs/a.md',
route: 'en/a.md',
pages,
repoRoot: root,
repositoryRef: 'abc123',
placeImage: absPath => `./${absPath.split('/').pop() ?? ''}`,
})).toBe('![logo](./logo.svg#view)\n')
})
it('leaves a published page link to the route even when a placer exists', () => {
const { root, pages } = fixture()
expect(rewriteMarkdown('[B](b.md)\n', {

View File

@@ -5,7 +5,9 @@
* tier, while this adapter rewrites cross-source links for the public site.
*/
import { copyFileSync, existsSync, lstatSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import {
copyFileSync, existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync,
} from 'node:fs'
import { basename, dirname, extname, posix, relative, resolve, sep } from 'node:path'
import { fromMarkdown } from 'mdast-util-from-markdown'
import { gfmFromMarkdown } from 'mdast-util-gfm'
@@ -234,7 +236,9 @@ export function rewriteMarkdown(source: string, options: RewriteMarkdownOptions)
const nextUrl = page !== undefined
? routeTarget(options.route, page.route, suffix)
: node.type === 'image' && options.placeImage !== undefined
? options.placeImage(absPath)
// The suffix rides along exactly as the GitHub branch keeps it: an SVG
// view fragment or a Vite query changes what the reference means.
? `${options.placeImage(absPath)}${suffix}`
: githubTarget(absPath, line, suffix, options.repositoryRef, options.repoRoot, node.type === 'image')
const start = node.position?.start.offset
@@ -302,19 +306,78 @@ export function projectedPageContent(markdown: string, page: DocsPage): string {
return markdown.slice(0, closing + closingDelimiter.length)
}
/** Canonical Markdown files watched by the local VitePress dev server. */
/**
* The repository file one image reference resolves to, or `undefined` when the
* target is not a local file this build may publish.
* @param absPath - resolved image target.
* @param repoRoot - repository root every published image must stay inside.
* @returns the file's real path, or `undefined` when it must not be copied.
*
* Only a regular file whose real path stays inside the repository qualifies.
* Publication copies the bytes into the site, so a reference escaping the
* repository — `../../.ssh/id_rsa`, or a symlink pointing out of the tree —
* would put a build-machine file on the site; `existsSync` alone, which is all
* link resolution needs, does not answer that.
*/
export function publishableImage(absPath: string, repoRoot: string): string | undefined {
const real = realpathSync(absPath)
const inside = real === repoRoot || real.startsWith(`${repoRoot}${sep}`)
return inside && statSync(real).isFile() ? real : undefined
}
/** Every local image a published page references, resolved to its repository file. */
function referencedImages(): string[] {
const found = new Set<string>()
for (const page of docsPages) {
const sourceAbs = resolve(root, page.source)
if (!existsSync(sourceAbs)) continue
rewriteMarkdown(readFileSync(sourceAbs, 'utf8'), {
sourcePath: page.source,
locale: page.locale,
route: page.route,
pages: docsPages,
repoRoot: root,
repositoryRef: 'master',
placeImage: (absPath) => {
const real = publishableImage(absPath, root)
if (real !== undefined) found.add(real)
return ''
},
})
}
return [...found]
}
/**
* Files watched by the local VitePress dev server: every canonical Markdown
* source, plus the images they publish. Without the images, replacing a
* screenshot leaves the previous copy in the generated tree until something
* touches the Markdown beside it.
*/
export function docsSourceFiles(): string[] {
return [...new Set(docsPages.map(page => resolve(root, page.source)))]
return [...new Set([...docsPages.map(page => resolve(root, page.source)), ...referencedImages()])]
}
/** Rebuild the disposable VitePress source tree from the publication manifest. */
export function projectDocs(): void {
const routes = new Set<string>()
/** Projected asset path to the source it came from, for collision detection. */
const assets = new Map<string, string>()
/** Projected path to the repository file that claimed it, pages and images alike. */
const claimed = new Map<string, string>()
const repositoryRef = process.env.GITHUB_SHA ?? 'master'
rmSync(generatedRoot, { recursive: true, force: true })
/** Reserve one projected path, refusing a second source for it. */
const claim = (target: string, sourceAbs: string): void => {
const holder = claimed.get(target)
if (holder !== undefined && holder !== sourceAbs) {
throw new Error(
`project-doc-site: ${repoPath(sourceAbs, root)} and ${repoPath(holder, root)}`
+ ` both project to ${relative(generatedRoot, target).split(sep).join('/')}.`,
)
}
claimed.set(target, sourceAbs)
}
for (const page of docsPages) {
if (routes.has(page.route)) throw new Error(`project-doc-site: duplicate route ${JSON.stringify(page.route)}.`)
routes.add(page.route)
@@ -323,6 +386,9 @@ export function projectDocs(): void {
throw new Error(`project-doc-site: source ${JSON.stringify(page.source)} does not exist or is not a file.`)
}
const output = resolve(generatedRoot, page.route)
// Claimed before the images are placed: a page and an image landing on one
// path would otherwise overwrite each other in whichever order they ran.
claim(output, sourceAbs)
mkdirSync(dirname(output), { recursive: true })
const markdown = readFileSync(sourceAbs, 'utf8')
const projected = rewriteMarkdown(markdown, {
@@ -333,22 +399,23 @@ export function projectDocs(): void {
repoRoot: root,
repositoryRef,
placeImage: (absPath) => {
// Beside the page that references it, under its own basename: each
// locale's route tree gets its own copy, so one relative URL is correct
// from both. Two sources that would land on one name are a collision
// rather than a silent overwrite of whichever copied last.
const name = basename(absPath)
const target = resolve(dirname(output), name)
const claimed = assets.get(target)
if (claimed !== undefined && claimed !== absPath) {
const real = publishableImage(absPath, root)
if (real === undefined) {
throw new Error(
`project-doc-site: ${repoPath(absPath, root)} and ${repoPath(claimed, root)}`
+ ` both project to ${relative(generatedRoot, target).split(sep).join('/')}.`,
`project-doc-site: ${page.source} references image ${repoPath(absPath, root)},`
+ ' which is not a regular file inside the repository.',
)
}
assets.set(target, absPath)
copyFileSync(absPath, target)
return `./${name}`
// Beside the page that references it, under its own basename: each
// locale's route tree gets its own copy, so one relative URL is correct
// from both.
const name = basename(real)
const target = resolve(dirname(output), name)
claim(target, real)
copyFileSync(real, target)
// Encoded because the destination is a Markdown inline target, where an
// unescaped space would end it early.
return `./${encodeURI(name)}`
},
})
writeFileSync(output, addProjectionFrontmatter(projectedPageContent(projected, page), page))