diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdcf4a8..76f1667 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,6 +86,31 @@ jobs: - run: bun install --frozen-lockfile - run: DOCX_LO_ALL=1 bun run test:integration # full fixture sweep in CI + windows-render: + name: Render through LibreOffice (Windows) + runs-on: windows-latest + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@v6 + - uses: oven-sh/setup-bun@v2 + - uses: actions/cache@v5 + with: + path: ~/.bun/install/cache + key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} + restore-keys: bun-${{ runner.os }}- + - name: Install LibreOffice + shell: pwsh + run: choco install libreoffice-fresh -y --no-progress --limit-output + - run: bun install --frozen-lockfile + - run: bun test tests/cli/render.test.ts tests/cli/images.test.ts + - run: bun run build:binary + - name: Render with the compiled binary + run: | + ./dist/docx render tests/fixtures/markdown-import.docx --out smoke --engine libreoffice --pages 1 + test -s smoke/page-001.png + build-binary: name: Smoke-build compiled binary (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b5f2e2a..fce0623 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,6 +118,7 @@ GitHub Actions (`.github/workflows/ci.yml`) runs these jobs on push to `main` an | `check` | `biome check . && knip-bun && tsc --noEmit` | | `unit-tests` | `bun run test:unit` and `bun run test:benchmark` (no model calls) | | `macos-output` | Output contract and real-pipe regressions on macOS with Bun 1.3.10 and latest | +| `windows-render` | Installs LibreOffice on Windows; runs the render and image tests, then renders with the compiled binary | | `integration-tests` | Installs LibreOffice, runs `bun run test:integration` | | `build-binary` | Builds on Linux and native Intel/ARM macOS; signs/verifies macOS binaries; runs version/help, read, and schema-validation smoke checks | diff --git a/skills/docx-cli/references/troubleshooting.md b/skills/docx-cli/references/troubleshooting.md index c4089bc..35e0ac4 100644 --- a/skills/docx-cli/references/troubleshooting.md +++ b/skills/docx-cli/references/troubleshooting.md @@ -89,6 +89,17 @@ HTML), which can transform literal prose. When you need text in **verbatim**, us the parser-free channel: `insert` and `create` take `--text-file PATH` (`-` = stdin). Every character lands as-is; each newline starts a new paragraph. +## Accents or CJK text look garbled in PowerShell output + +`docx` writes UTF-8, and PowerShell decodes captured output using its own console +encoding, which may be an older code page such as IBM437. The document is fine, +but what you see isn't, so don't write that text back. Set this once per session +before running `docx`, or use Git Bash: + +```powershell +[Console]::OutputEncoding = [Text.Encoding]::UTF8 +``` + ## I edited a file and lost some unusual formatting You shouldn't — docx-cli mutates the underlying XML **in place** and leaves diff --git a/src/core/image/source.ts b/src/core/image/source.ts index 0f8c11f..1c7af6f 100644 --- a/src/core/image/source.ts +++ b/src/core/image/source.ts @@ -4,6 +4,7 @@ // (parsed from the file header) used to size the drawing's . import { lookup } from "node:dns/promises"; +import { fileURLToPath } from "node:url"; import convert from "heic-convert"; import { CANONICAL_IMAGE_EXTENSION, @@ -235,9 +236,7 @@ async function readBodyWithCap( } async function loadFile(path: string): Promise { - const cleaned = path.startsWith("file://") - ? path.slice("file://".length) - : path; + const cleaned = path.startsWith("file://") ? pathFromFileUrl(path) : path; const file = Bun.file(cleaned); if (!(await file.exists())) { throw new ImageSourceError(`Image file not found: ${cleaned}`); @@ -322,6 +321,17 @@ async function transcodeHEICToJPEG(bytes: Uint8Array): Promise { } } +/** `file:///C:/x.png` is `C:\x.png` on Windows, not `/C:/x.png`, and a percent-encoded + * space is a space. A URL with a host keeps meaning the path after the prefix, as it + * always has (`file://relative/x.png`): on Windows, converting it would make a UNC path, + * turning an attacker-influenced `--image` into a request to their server. */ +function pathFromFileUrl(url: string): string { + try { + if (!new URL(url).host) return fileURLToPath(url); + } catch {} + return url.slice("file://".length); +} + function extensionFromPath(path: string): string | undefined { const lastDot = path.lastIndexOf("."); const lastSlash = path.lastIndexOf("/"); diff --git a/src/core/render/CLAUDE.md b/src/core/render/CLAUDE.md index f497c77..95a8643 100644 --- a/src/core/render/CLAUDE.md +++ b/src/core/render/CLAUDE.md @@ -80,7 +80,9 @@ Word for Mac is sandboxed: arbitrary file paths trigger a "Grant File Access" pr ## LibreOffice profile isolation -`soffice --headless` on macOS locks the default user profile (`~/Library/Application Support/LibreOffice/4`). Two concurrent invocations (or one stale soffice process) make subsequent spawns exit non-zero. We mint a temp profile per call via `-env:UserInstallation=file://` and `rm -rf` it after. Mirrors [tests/integration/libreoffice-roundtrip.test.ts](../../../tests/integration/libreoffice-roundtrip.test.ts)'s worker-pool pattern; **any new soffice spawn in this codebase must do the same**. +`soffice --headless` on macOS locks the default user profile (`~/Library/Application Support/LibreOffice/4`). Two concurrent invocations (or one stale soffice process) make subsequent spawns exit non-zero. We mint a temp profile per call via `-env:UserInstallation=` and `rm -rf` it after. Mirrors [tests/integration/libreoffice-roundtrip.test.ts](../../../tests/integration/libreoffice-roundtrip.test.ts)'s worker-pool pattern; **any new soffice spawn in this codebase must do the same**. + +**Windows, both verified on a runner:** spawn `soffice.exe`, never `soffice.com` (Bun.spawn fails with "Executable not found", and `where soffice` lists the `.com` first), and build the profile URL with `pathToFileURL` (soffice hangs on `file://C:\…`). The `windows-render` CI job guards both. ## PDF rasterization details diff --git a/src/core/render/engines/libreoffice.ts b/src/core/render/engines/libreoffice.ts index 85c18a8..deceadf 100644 --- a/src/core/render/engines/libreoffice.ts +++ b/src/core/render/engines/libreoffice.ts @@ -1,8 +1,13 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; +import { pathToFileURL } from "node:url"; import { type RenderEngine, RenderEngineError } from "./types"; +/** Long enough for a large document's conversion on a cold start, matching the + * Word for Windows engine's cap. */ +const CONVERT_TIMEOUT_MS = 150_000; + /** Cross-platform LibreOffice engine. Drives `soffice --headless * --convert-to pdf` against the input docx. Used as the fallback when Word * isn't installed; also usable explicitly on a server / CI machine where @@ -35,7 +40,8 @@ export const libreofficeEngine: RenderEngine = { [ soffice, "--headless", - `-env:UserInstallation=file://${profile}`, + // A real file URL: `file://C:\…` isn't one, and soffice hangs on it. + `-env:UserInstallation=${pathToFileURL(profile).href}`, "--convert-to", "pdf", "--outdir", @@ -44,7 +50,23 @@ export const libreofficeEngine: RenderEngine = { ], { stdout: "pipe", stderr: "pipe" }, ); - const exit = await proc.exited; + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + proc.kill(); + }, CONVERT_TIMEOUT_MS); + let exit: number; + try { + exit = await proc.exited; + } finally { + clearTimeout(killTimer); + } + if (timedOut) { + throw new RenderEngineError( + "RENDER_FAILED", + `soffice did not finish within ${CONVERT_TIMEOUT_MS / 1000}s`, + ); + } if (exit !== 0) { const stderr = await new Response(proc.stderr).text(); throw new RenderEngineError( @@ -64,7 +86,11 @@ export const libreofficeEngine: RenderEngine = { rmSync(sofficeOutput, { force: true }); } } finally { - rmSync(profile, { recursive: true, force: true }); + // A killed soffice can still hold its profile open on Windows; a leftover + // temp folder is harmless and must not replace the real error. + try { + rmSync(profile, { recursive: true, force: true }); + } catch {} } }, }; @@ -73,15 +99,19 @@ async function findSoffice(): Promise { // PATH search first (the brew install on macOS, apt on Linux, the LO // installer's program/ dir on Windows when added to PATH). Then check // the canonical install paths in case PATH wasn't set up. - const onPath = await whichEverywhere("soffice"); + // On Windows, ask for soffice.exe by name: `where soffice` lists soffice.com + // first, and Bun.spawn can't launch a .com ("Executable not found"). + const onPath = await whichEverywhere( + process.platform === "win32" ? "soffice.exe" : "soffice", + ); if (onPath) return onPath; const fallbacks = process.platform === "darwin" ? ["/Applications/LibreOffice.app/Contents/MacOS/soffice"] : process.platform === "win32" ? [ - "C:\\Program Files\\LibreOffice\\program\\soffice.com", - "C:\\Program Files (x86)\\LibreOffice\\program\\soffice.com", + "C:\\Program Files\\LibreOffice\\program\\soffice.exe", + "C:\\Program Files (x86)\\LibreOffice\\program\\soffice.exe", ] : ["/usr/bin/soffice", "/snap/bin/libreoffice"]; for (const candidate of fallbacks) { diff --git a/tests/cli/images.test.ts b/tests/cli/images.test.ts index 4c44190..e84b8cd 100644 --- a/tests/cli/images.test.ts +++ b/tests/cli/images.test.ts @@ -1,6 +1,7 @@ import { beforeAll, describe, expect, test } from "bun:test"; import { mkdirSync, readdirSync } from "node:fs"; import { join } from "node:path"; +import { pathToFileURL } from "node:url"; import { Document, XmlNode } from "@core"; import { Pkg } from "@core/ast/document/package"; import { runCli, tempWorkspace } from "./harness"; @@ -235,6 +236,26 @@ async function dataUri(path: string, mime: string): Promise { } describe("docx images add", () => { + // Slicing off `file://` left `/C:/x.png` on Windows and a literal `%20` everywhere. + test("embeds a PNG from a file:// URL with a space in its path", async () => { + const docPath = await newDoc("img-file-url"); + const spaced = join(tempWorkspace("img file url"), "my pic.png"); + await Bun.write(spaced, Bun.file(PNG_PATH)); + const result = await runCli( + "images", + "add", + docPath, + "--after", + "p0", + "--image", + pathToFileURL(spaced).href, + ); + expect(result.exitCode).toBe(0); + + const pkg = await Pkg.open(docPath); + expect(pkg.listParts()).toContain("word/media/image1.png"); + }); + test("embeds a PNG from a file path: media part, relationship, content-type", async () => { const docPath = await newDoc("img-png"); const result = await runCli( diff --git a/tests/cli/render.test.ts b/tests/cli/render.test.ts index b065d44..613cb62 100644 --- a/tests/cli/render.test.ts +++ b/tests/cli/render.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { existsSync } from "node:fs"; import { join } from "node:path"; +import { libreofficeEngine } from "@core/render"; import { parsePagesSpec } from "../../src/cli/render/parse-pages"; import { runCli, tempWorkspace } from "./harness"; @@ -10,7 +11,7 @@ import { runCli, tempWorkspace } from "./harness"; * machines without LibreOffice (matches the LibreOffice round-trip * integration suite's pattern). Pure-logic tests (--pages parsing, * USAGE errors) always run. */ -const LIBREOFFICE_AVAILABLE = await detectLibreOffice(); +const LIBREOFFICE_AVAILABLE = await libreofficeEngine.available(); /** LibreOffice cold-start (the first soffice launch) routinely exceeds bun's 5s * default per-test timeout under load — e.g. right after the weak-agent-test @@ -19,17 +20,6 @@ const LIBREOFFICE_AVAILABLE = await detectLibreOffice(); * a failure. (They still run in ~2-3s when LibreOffice is warm.) */ const RENDER_TIMEOUT = 60_000; -async function detectLibreOffice(): Promise { - const probe = async (cmd: string[]): Promise => { - const proc = Bun.spawn(cmd, { stdout: "pipe", stderr: "pipe" }); - return (await proc.exited) === 0; - }; - return ( - (await probe(["/bin/sh", "-c", "command -v soffice"])) || - existsSync("/Applications/LibreOffice.app/Contents/MacOS/soffice") - ); -} - describe("docx render — argument parsing (no engine needed)", () => { test("missing FILE → USAGE error", async () => { const result = await runCli("render");