diff --git a/.changeset/curly-llamas-jump.md b/.changeset/curly-llamas-jump.md new file mode 100644 index 000000000..c0a571f74 --- /dev/null +++ b/.changeset/curly-llamas-jump.md @@ -0,0 +1,7 @@ +--- +"openwiki": patch +--- + +fix: emit relative markdown links so every renderer resolves them + +Generated page bodies used root-relative links (`/openwiki/page.md`, `/src/foo.ts`) that no renderer resolves. Page bodies are now required to use paths relative to the linking file, the validator stamps any root-relative destination so the next update repairs it, and the visualizer graph resolves leftover root-relative links instead of dropping their edges. diff --git a/src/agent/prompt.ts b/src/agent/prompt.ts index 36d6e9d6a..481b2acd0 100644 --- a/src/agent/prompt.ts +++ b/src/agent/prompt.ts @@ -115,8 +115,9 @@ Output language: export function createLinkIntegrityInstructions(): string { return ` Link integrity: -- Prefer relative Markdown links to existing wiki pages and stable heading anchors. Do not invent destinations that are not written in the same run. -- OpenWiki validates relative internal links and heading anchors after the run. Broken links are left in place and marked with an HTML comment starting with "openwiki: broken internal link", so the run completes and a later update can self-correct. If you find such a comment, repair the href or restore the target page using the reason in the comment, then delete the comment. +- Write Markdown link destinations relative to the linking file, never rooted at /. The /-rooted virtual paths in the filesystem-tool instructions are only for tool calls. Use the relative form the generated index.md files use, for example ./architecture/overview.md, ../agent/workflow.md, or ../../src/main.ts; a root-relative destination such as /openwiki/page.md or /src/main.ts resolves in no renderer (GitHub, editor preview, or the OpenWiki visualizer graph) and is treated as broken. +- Prefer links to existing wiki pages and stable heading anchors. Do not invent destinations that are not written in the same run. +- OpenWiki validates internal links and heading anchors after the run. Broken links — root-relative ones included — are left in place and marked with an HTML comment starting with "openwiki: broken internal link", so the run completes and a later update can self-correct. If you find such a comment, repair the href or restore the target page using the reason in the comment, then delete the comment. `; } diff --git a/src/agent/wiki-link-validator.ts b/src/agent/wiki-link-validator.ts index 10a248044..3a3041091 100644 --- a/src/agent/wiki-link-validator.ts +++ b/src/agent/wiki-link-validator.ts @@ -124,7 +124,7 @@ export async function validateWikiInternalLinks( report.issuesFound += issues.length; const stamped = stampBrokenLinks(cleaned, issues); - if (stamped === original) { + if (stamped === normalizeLineEndings(original)) { continue; } @@ -170,6 +170,16 @@ export function stripBrokenLinkStamps(content: string): string { .join("\n"); } +/** + * Normalizes CRLF and lone-CR line endings to LF, so a rewrite comparison is + * immune to the host platform's line-ending convention. `stripBrokenLinkStamps` + * already returns LF, so a file whose only difference from the clean content is + * line endings must not be rewritten (or reported as stamped). + */ +function normalizeLineEndings(content: string): string { + return content.replace(/\r\n?/gu, "\n"); +} + /** * Inserts broken-link stamps above each failing link line (bottom-up). */ @@ -221,6 +231,19 @@ async function validateLink( return null; } + // A leading-slash destination is root-relative: no renderer (GitHub, editor + // preview, or the visualizer graph) resolves it, so it cannot be relied on. + // Flag it so a later update run rewrites it as a path relative to this file, + // matching the relative form the deterministic index.md files already use. + if (path.posix.isAbsolute(href)) { + return { + href, + line, + message: `root-relative link "${href}" resolves in no renderer; rewrite it as a path relative to ${sourcePath}`, + sourcePath, + }; + } + const { anchor, path: linkPath } = parseLinkDestination(href); if (!linkPath) { if (!anchor) { @@ -252,7 +275,11 @@ async function validateLink( ? resolvedPath.replace(/\/+$/u, "") : resolvedPath; - if (!(await pathExists(backend, targetPath, isDirectory))) { + const exists = + (await pathExists(backend, targetPath, isDirectory)) || + (!isDirectory && (await pathExists(backend, targetPath, true))); + + if (!exists) { return { href, line, @@ -428,13 +455,12 @@ function parseLinkDestination(rawHref: string): { } /** - * Resolves a link path to a normalized repo-absolute path, or undefined when it - * cannot be contained within the repo root. + * Resolves a relative link path against its source file's directory into a + * normalized repo-absolute path, or undefined when it cannot be contained + * within the repo root. * - * A leading-slash link is absolute from the virtual filesystem root (the repo - * root in `repository` mode, the wiki dir in `local-wiki` mode) — the same - * convention the generation prompt teaches and GitHub renders. A relative link - * resolves against its source file's directory. + * Root-relative destinations are rejected by `validateLink` before this runs, + * so only relative paths reach this resolver. * * The result is not constrained to the wiki subtree: wiki pages may link out to * other repo files, so containment is enforced at the repo root instead. @@ -449,9 +475,7 @@ function resolveRepoLinkPath( linkPath: string, ): string | null { const candidate = path.posix.normalize( - linkPath.startsWith("/") - ? linkPath - : path.posix.join(path.posix.dirname(sourcePath), linkPath), + path.posix.join(path.posix.dirname(sourcePath), linkPath), ); return path.posix.isAbsolute(candidate) ? candidate : null; @@ -468,6 +492,12 @@ function isExternalHref(href: string): boolean { /** * True when a wiki-absolute path resolves to an existing file or directory on * the backend. Any read error is treated as "does not exist". + * + * Directory existence cannot be inferred from `ls` alone, because the backend + * lists a missing directory as an empty result rather than an error. Instead + * the parent directory is listed and the target's entry is matched by name, so + * a genuinely missing directory (e.g. a clamps-at-root `../` escape) still + * resolves to false. */ async function pathExists( backend: BackendProtocolV2, @@ -475,13 +505,26 @@ async function pathExists( isDirectory: boolean, ): Promise { try { - if (isDirectory) { - const result = await backend.ls(targetPath); + if (!isDirectory) { + const result = await backend.readRaw(targetPath); return !result.error; } - const result = await backend.readRaw(targetPath); - return !result.error; + const normalized = targetPath.replace(/\/+$/u, ""); + if (!normalized || normalized === "/") { + return true; + } + + const parent = path.posix.dirname(normalized); + const name = path.posix.basename(normalized); + const result = await backend.ls(parent); + if (result.error) { + return false; + } + + return (result.files ?? []).some( + (entry) => entry.is_dir === true && entryName(entry) === name, + ); } catch { return false; } diff --git a/src/visualize/graph.ts b/src/visualize/graph.ts index e68fb5964..baf6b19f6 100644 --- a/src/visualize/graph.ts +++ b/src/visualize/graph.ts @@ -294,6 +294,35 @@ async function readNode(file: string, wikiRoot: string): Promise { }; } +/** + * Resolve one markdown link target from its source file into a node id, or + * undefined when it matches no node. A relative link resolves against the + * source file's directory. A leading-slash (root-relative) link — which older + * generated wikis carry — is tried against the wiki root first, then against + * the repo root (the wiki root's parent), so both code-mode + * `/openwiki/foo.md` and local-mode `/foo.md` links resolve without the + * builder knowing the mode. Only candidates naming an existing node are + * accepted, so no edge is ever fabricated. + */ +function resolveNodeId( + wikiRoot: string, + fileDir: string, + link: string, + byId: Map, +): string | undefined { + if (!link.startsWith("/")) { + const id = toId(wikiRoot, path.resolve(fileDir, link)); + return byId.has(id) ? id : undefined; + } + + const repoRelative = link.replace(/^\/+/, ""); + const candidates = [ + toId(wikiRoot, path.join(wikiRoot, repoRelative)), + toId(wikiRoot, path.join(path.dirname(wikiRoot), repoRelative)), + ]; + return candidates.find((id) => byId.has(id)); +} + /** * Resolve each node's markdown links into directed edges between existing nodes, * recording them on the nodes' `links`/`backlinks` in place. Self-links, links to @@ -306,7 +335,8 @@ function linkNodes(nodes: WikiNode[], wikiRoot: string): WikiEdge[] { for (const node of nodes) { const fileDir = path.dirname(path.join(wikiRoot, `${node.id}.md`)); for (const link of markdownLinks(node.body)) { - const target = toId(wikiRoot, path.resolve(fileDir, link)); + const target = resolveNodeId(wikiRoot, fileDir, link, byId); + if (!target) continue; const targetNode = byId.get(target); const key = `${node.id}\n${target}`; if (!targetNode || target === node.id || seen.has(key)) continue; diff --git a/test/visualize-graph.test.ts b/test/visualize-graph.test.ts index c8bcbd478..d14628cfc 100644 --- a/test/visualize-graph.test.ts +++ b/test/visualize-graph.test.ts @@ -109,6 +109,51 @@ describe("buildGraph", () => { expect(graph.edges).toEqual([]); }); + test("resolves root-relative links against the wiki root", async () => { + const root = await makeWiki({ + "quickstart.md": "# Home\nSee [workflow](/agent/workflow.md).\n", + "agent/workflow.md": "# Workflow\n", + }); + + const graph = await buildGraph(root); + + expect(graph.edges).toContainEqual({ + source: "quickstart", + target: "agent/workflow", + }); + expect( + graph.nodes.find((n) => n.id === "agent/workflow")?.backlinks, + ).toContain("quickstart"); + }); + + test("resolves root-relative links under an openwiki/ subdir", async () => { + const root = await makeWiki({ + "openwiki/quickstart.md": + "# Home\nSee [Viewer Runtime](/openwiki/rendering/viewer.md).\n", + "openwiki/rendering/viewer.md": "# Viewer\n", + }); + + const graph = await buildGraph(path.join(root, "openwiki")); + + expect(graph.edges).toContainEqual({ + source: "quickstart", + target: "rendering/viewer", + }); + expect( + graph.nodes.find((n) => n.id === "rendering/viewer")?.backlinks, + ).toContain("quickstart"); + }); + + test("does not create edges from root-relative links to unknown pages", async () => { + const root = await makeWiki({ + "quickstart.md": "# Home\nSee [nope](/openwiki/missing.md).\n", + }); + + const graph = await buildGraph(root); + + expect(graph.edges).toEqual([]); + }); + test("does not follow a symlink that escapes the wiki root", async () => { const secret = await mkdtemp(path.join(tmpdir(), "openwiki-secret-")); tempDirs.push(secret); diff --git a/test/wiki-link-validator.test.ts b/test/wiki-link-validator.test.ts index 4d855c9ee..dba19154b 100644 --- a/test/wiki-link-validator.test.ts +++ b/test/wiki-link-validator.test.ts @@ -51,8 +51,36 @@ describe("validateWikiInternalLinks", () => { ).resolves.toBe(before); }); - test("accepts repo-root-absolute links carrying the /openwiki prefix", async () => { - const { backend } = await setupWiki(); + test("does not rewrite CRLF files whose only difference is line endings", async () => { + const { backend, rootDir } = await setupWiki(); + await backend.write( + "/openwiki/quickstart.md", + "# Quickstart\r\n\r\nSee [architecture](./architecture/overview.md).\r\n", + ); + await backend.write( + "/openwiki/architecture/overview.md", + "# Overview\r\n", + ); + const before = await readFile( + path.join(rootDir, "openwiki/quickstart.md"), + "utf8", + ); + const edit = vi.spyOn(backend, "edit"); + + const report = await validateWikiInternalLinks(backend, "repository"); + + expect(report).toMatchObject({ + issuesFound: 0, + stampedFiles: [], + }); + expect(edit).not.toHaveBeenCalled(); + await expect( + readFile(path.join(rootDir, "openwiki/quickstart.md"), "utf8"), + ).resolves.toBe(before); + }); + + test("stamps root-relative links so the next run rewrites them relative", async () => { + const { backend, rootDir } = await setupWiki(); await backend.write( "/openwiki/integrations/connectors.md", "See [CLI usage](/openwiki/cli/usage.md).\n", @@ -61,15 +89,23 @@ describe("validateWikiInternalLinks", () => { const report = await validateWikiInternalLinks(backend, "repository"); - expect(report.issuesFound).toBe(0); - expect(report.stampedFiles).toEqual([]); + expect(report.issuesFound).toBe(1); + expect(report.stampedFiles).toEqual(["integrations/connectors.md"]); + const after = await readFile( + path.join(rootDir, "openwiki/integrations/connectors.md"), + "utf8", + ); + expect(after).toContain( + "openwiki: broken internal link [/openwiki/cli/usage.md]", + ); + expect(after).toContain("root-relative"); }); - test("accepts repo-root-absolute links with heading anchors", async () => { + test("accepts relative links with heading anchors", async () => { const { backend } = await setupWiki(); await backend.write( "/openwiki/architecture/agents.md", - "See [Shared Browser Tooling](/openwiki/architecture/shared-tools.md#one-browser-tab-per-run).\n", + "See [Shared Browser Tooling](./shared-tools.md#one-browser-tab-per-run).\n", ); await backend.write( "/openwiki/architecture/shared-tools.md", @@ -82,7 +118,7 @@ describe("validateWikiInternalLinks", () => { expect(report.stampedFiles).toEqual([]); }); - test("stamps repo-root-absolute links to missing files", async () => { + test("stamps root-relative links to missing files", async () => { const { backend } = await setupWiki(); await backend.write( "/openwiki/quickstart.md", @@ -95,21 +131,27 @@ describe("validateWikiInternalLinks", () => { expect(report.stampedFiles).toEqual(["quickstart.md"]); }); - test("stamps repo-root-absolute links with missing anchors", async () => { - const { backend } = await setupWiki(); + test("stamps root-relative links to repo source files", async () => { + const { backend, rootDir } = await setupWiki(); + await mkdir(path.join(rootDir, "src"), { recursive: true }); + await writeFile(path.join(rootDir, "src/index.ts"), "export {};\n", "utf8"); await backend.write( "/openwiki/quickstart.md", - "See [section](/openwiki/overview.md#missing-anchor).\n", + "See [src](/src/index.ts).\n", ); - await backend.write("/openwiki/overview.md", "# Overview\n"); const report = await validateWikiInternalLinks(backend, "repository"); expect(report.issuesFound).toBe(1); expect(report.stampedFiles).toEqual(["quickstart.md"]); + const after = await readFile( + path.join(rootDir, "openwiki/quickstart.md"), + "utf8", + ); + expect(after).toContain("root-relative link \"/src/index.ts\""); }); - test("accepts links to existing repo files outside the wiki dir", async () => { + test("accepts relative links to existing repo files outside the wiki dir", async () => { const { backend, rootDir } = await setupWiki(); await mkdir(path.join(rootDir, "docs"), { recursive: true }); await writeFile( @@ -119,7 +161,7 @@ describe("validateWikiInternalLinks", () => { ); await backend.write( "/openwiki/architecture/ui-components.md", - "See [decision](/docs/decision.md) and [src](/src/index.ts).\n", + "See [decision](../../docs/decision.md) and [src](../../src/index.ts).\n", ); await mkdir(path.join(rootDir, "src"), { recursive: true }); await writeFile(path.join(rootDir, "src/index.ts"), "export {};\n", "utf8"); @@ -130,11 +172,11 @@ describe("validateWikiInternalLinks", () => { expect(report.stampedFiles).toEqual([]); }); - test("stamps links to missing repo files outside the wiki dir", async () => { + test("stamps relative links to missing repo files outside the wiki dir", async () => { const { backend, rootDir } = await setupWiki(); await backend.write( "/openwiki/architecture/ui-components.md", - "See [decision](/docs/missing-decision.md).\n", + "See [decision](../../docs/missing-decision.md).\n", ); const report = await validateWikiInternalLinks(backend, "repository"); @@ -145,7 +187,9 @@ describe("validateWikiInternalLinks", () => { path.join(rootDir, "openwiki/architecture/ui-components.md"), "utf8", ); - expect(after).toContain('file "/docs/missing-decision.md" does not exist'); + expect(after).toContain( + 'file "../../docs/missing-decision.md" does not exist', + ); }); test("does not validate anchors on non-markdown targets", async () => { @@ -155,7 +199,7 @@ describe("validateWikiInternalLinks", () => { // GitHub line anchors (#L10) on source files must not be treated as broken. await backend.write( "/openwiki/quickstart.md", - "See [line](/src/index.ts#L10).\n", + "See [line](../../src/index.ts#L10).\n", ); const report = await validateWikiInternalLinks(backend, "repository"); @@ -164,6 +208,20 @@ describe("validateWikiInternalLinks", () => { expect(report.stampedFiles).toEqual([]); }); + test("accepts slash-less directory links", async () => { + const { backend, rootDir } = await setupWiki(); + await mkdir(path.join(rootDir, "openwiki", "agent"), { recursive: true }); + await writeFile( + path.join(rootDir, "openwiki", "page.md"), + "- [agent](agent)\n", + "utf8", + ); + + const report = await validateWikiInternalLinks(backend, "repository"); + + expect(report.issuesFound).toBe(0); + }); + test("stamps missing target files without throwing", async () => { const { backend, rootDir } = await setupWiki(); await backend.write( @@ -257,9 +315,9 @@ describe("validateWikiInternalLinks", () => { await backend.write( "/openwiki/architecture/agents.md", [ - "See [tokens](/openwiki/architecture/overview.md#layout-primitives--design-tokens).", - "See [store](/openwiki/architecture/overview.md#state--store).", - "See [ab](/openwiki/architecture/overview.md#a--b).", + "See [tokens](./overview.md#layout-primitives--design-tokens).", + "See [store](./overview.md#state--store).", + "See [ab](./overview.md#a--b).", ].join("\n"), ); await backend.write(