fix(bin): produce conforming PDFs and gate delivery on a real reader - #32
Merged
Conversation
…real reader A browser-printed handover document reached the captain unopenable while looking correct on screen. The cause was the assembly step, not the browser and not the file size: poppler's `pdfunite` wrote a trailer declaring `/Size 2935` for a document whose own cross-reference table declares 39249 entries and whose highest object number is 39248, violating PDF 32000-1 section 7.5.5. A known-good PDF pushed through the same two poppler steps acquires the identical defect, which isolates the cause. `bin/fm-pdf-finish.sh` replaces that assembly with Ghostscript's pdfwrite device, which rebuilds the document and emits a correct cross-reference table, then reads the result back and publishes it only if the check passes. Work happens on a temporary file beside the destination, so a rejected result leaves any previous file untouched and creates no new one. `bin/fm-pdf-verify.sh` is that check. It deliberately does not trust an exit code: Ghostscript exits 0 on a file it has just reported as non-conforming, `-dPDFSTOPONERROR` and `-dPDFSTOPONWARNING` do not change that, and `-q` additionally hides the diagnostics. A gate written as "run Ghostscript, check $?" would pass the exact file this commit is about. The verdict comes from the reader's diagnostics, and the script refuses in both directions - when the reader reports a problem, and equally when the reader is absent, fails, or prints nothing recognizable. A check that passes when it could not actually check reads like an assurance and is not one. `tests/fm-pdf-output.test.sh` proves both directions. It builds its own conforming fixture and reproduces the field defect deterministically by rewriting only the trailer, so it needs no poppler, and additionally asserts against genuine `pdfunite` output when poppler is present. It covers the fail-closed paths, the no-publish-on-reject guarantee, and page preservation. The suite is not vacuous: an always-pass gate fails it at the first assertion. Applied to the file that failed, this reproduces the original repair exactly - 2,448,482 bytes in, 780,372 bytes out, 27 pages, conforming, with text identical to the repaired copy the captain already accepted. docs/pdf-output.md carries the evidence, versions, and exact output.
…ass, and gate skips
…ion, unpin refusal class
…cript prerequisite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Repair the PDF generation path so it stops producing files that violate the PDF specification, instead of repairing each file after the fact.
Background the reviewer cannot see in the diff: on 2026-07-26 a 27-page handover PDF reached the captain and would not open on their phone. The cause was NOT file size. A Ghostscript check reported the file as non-conforming; rewriting it through Ghostscript both repaired it and shrank it from 2.34 MB to 0.74 MB. Both copies still exist under the captain-private data/fleet-handover-pt/ and were the reproduction case for this work.
Root cause found and evidenced, not guessed: the generation path printed HTML from a headless browser, split with poppler pdfseparate, and reassembled with poppler pdfunite. pdfunite wrote a trailer declaring /Size 2935 while the file's own cross-reference table declares 39249 entries and the highest object number is 39248 - a violation of PDF 32000-1 section 7.5.5, which requires /Size to be one greater than the highest object number. Pushing a known-good PDF through the same two poppler steps reproduces the identical defect, which isolates the cause to the assembly step and exonerates the browser.
Deliberate decisions made while doing the work, so they are not mistakes:
Ghostscript post-processing was explicitly authorized by the captain as a pragmatic route, on the condition that it be hardwired rather than a chance discovery. bin/fm-pdf-finish.sh therefore replaces pdfunite with Ghostscript's pdfwrite device, which both merges and normalizes, and it is the mandatory last step of generation rather than an optional cleanup.
The check deliberately does NOT trust the tool's exit code, and this is the single most important property of the change. It was measured on the actual broken file: Ghostscript exits 0 on a file it has just reported as non-conforming; -dPDFSTOPONERROR and -dPDFSTOPONWARNING do not change that; and -q additionally suppresses the diagnostics while still exiting 0. A gate written as 'run Ghostscript, check $?' would pass the exact file this change is about. The verdict is therefore taken from the reader's diagnostic text. If a reviewer proposes simplifying this to an exit-code check, that would reintroduce the bug.
The gate fails closed in both directions by explicit requirement: it refuses when the reader reports a problem, AND equally refuses when the reader is missing, exits non-zero, or prints nothing recognizable. A check that silently passes when the tool itself failed was called out as worse than no check, because it reads like an assurance and is not one. Hence the proof-of-work assertions that the reader announced its page range and interpreted the final page, and the separate exit code 3 meaning 'could not verify' - which callers must treat as failure, not success.
Ordering inside the verifier is deliberate: the reader's own complaint is evaluated BEFORE the proof-of-work check, so a badly truncated file is reported as rejected rather than as merely unverifiable.
Publication is atomic and conditional by design: work happens on a temporary file beside the destination and is moved into place only after the gate passes, so a rejected result leaves any previous file untouched and creates no new one.
The mktemp template deliberately ends at the X's with no .pdf suffix, because BSD/macOS mktemp rejects a suffix after them; the producer and gate go by content, not extension.
The test fixture deliberately builds its own conforming PDF with Ghostscript and reproduces the field defect by rewriting only the trailer, copying the body byte-for-byte with head -c, so the suite needs no poppler and cannot drift. It additionally asserts against genuine pdfunite output when poppler happens to be present, and skips that part otherwise. Both directions were required to be demonstrated: a broken file is rejected, a conforming file passes.
A single line was added to AGENTS.md section 9 rather than a new skill, following this repo's knowledge-placement decision tree and size discipline: the mechanics live in the script headers and docs/pdf-output.md, and only the load trigger is inline. AGENTS.md size discipline is a standing concern in this repo.
Constraint the captain fixed on 2026-07-27 and which must not be reopened: the handover PDF stays at 27 pages in the template's format, including cover, table of contents and divider pages. No shortening to ~15 pages. The 2.34 MB to 0.74 MB reduction came from the repair, not from dropping content - that is the benchmark. Verified: the regenerated document is 27 pages, conforming, 780,372 bytes, with text identical to the repaired copy the captain already accepted, and all five divider pages plus cover and contents present.
One known and accepted consequence, already true of the copy the captain accepted, so it is the established baseline rather than a new change: rebuilding maps fi and fl to their ligature codepoints, so extracted text carries the ligature character. Rendering and word count are unchanged; only text extraction and in-viewer search differ. It is documented.
Verification already done locally: shellcheck clean under the repo's pinned 0.11.0, bin/fm-lint.sh clean, the new suite green through the repo runner, and the 48-test changed-file regression set green with 0 failures.
What Changed
pdfseparate/pdfuniteassembly step, which wrote a trailer whose/Sizedisagreed with the cross-reference table (PDF 32000-1 §7.5.5) and produced documents that failed to open at the recipient, withbin/fm-pdf-finish.sh: it merges and normalizes rendered parts through Ghostscript'spdfwritedevice, then publishes atomically to a temporary file beside the destination and moves it into place only after the gate passes, so a rejected result leaves any previous file untouched.bin/fm-pdf-verify.sh, a conformance gate that reads the finished file back and takes its verdict from the reader's diagnostic text rather than its exit code — Ghostscript exits 0 on a file it has just reported as non-conforming, and-qsuppresses the report while still exiting 0. It fails closed in both directions, distinguishing exit 1 (REJECTED: the reader named a document problem, evaluated before the proof-of-work check) from exit 3 (CANNOT VERIFY: reader missing, broken, or silent), with the shared--pages/--quietparsing andFM_PDF_GSresolution factored intobin/fm-pdf-lib.sh.fm-pdf-finish.sh,tests/fm-pdf-output.test.shbuilds its own conforming fixture and reproduces the field defect by rewriting only the trailer (asserting against realpdfuniteoutput when poppler is present), and the CI serial lane now installs Ghostscript and runs with--fail-on-gate-skipso the proof cannot silently self-skip while CI stays green. Evidence, theFM_PDF_GSprerequisite, and the accepted fi/fl ligature effect on text extraction are documented indocs/pdf-output.md.Risk Assessment
✅ Low: The round-3 change is a small, well-bounded tests-and-docs edit that resolves all three prior findings, and I confirmed with read-only tooling that changed-test selection now exits 0 including this suite, the coverage guard passes, and the suite runs in the one lane that requires Ghostscript.
Testing
Ran the new PDF suite through the repo runner (green, with the real-poppler branch executing rather than skipping), then reproduced the field case end to end: a 27-page browser-printed handover assembled with pdfseparate/pdfunite reproduced the exact trailer defect (/Size 353 against 41798 xref entries, highest object 41797), Ghostscript exited 0 on it even with -dPDFSTOPONERROR/-dPDFSTOPONWARNING and silently with -q, and the new gate rejected it from the reader's diagnostics; fm-pdf-finish.sh then published a conforming 27-page document at 64,245 bytes versus the 1,492,205-byte rejected assembly with identical extracted text, word count, page breaks, cover, contents and all five dividers. Also confirmed atomic conditional publication (forced refusal left the previous deliverable byte-identical, created no destination, left no temp files) and both fail-closed directions (missing reader and a silent exit-0 stub each yield exit 3 with nothing published). Artifacts include the rendered 27-page contact sheet, both PDFs, and the full CLI transcript; worktree left clean.
/home/coditan/.cache/no-mistakes-tmp/no-mistakes-evidence/01KYJ3SXTYB7DYKCH7T14HNBNY/published-pdf-contact-sheet.png)Evidence: End-to-end CLI transcript: defect reproduction, exit-code trap, gate verdicts, atomic publication, fail-closed
--- OLD PATH: poppler pdfseparate + pdfunite (the field defect) --- trailer declares: /Size 353 xref subsection says: 0 41798 entries highest object number: 41797 0 obj -> PDF 32000-1 7.5.5 requires /Size = highest object number + 1. Violated. --- WHY AN EXIT-CODE GATE WOULD HAVE SHIPPED THIS FILE --- **** This file had errors that were repaired or ignored. **** file that it does not conform to Adobe's published PDF gs exit status: 0 (even with -dPDFSTOPONERROR -dPDFSTOPONWARNING) gs -q exit status: 0 (diagnostics suppressed, still 0) --- THE GATE TAKES ITS VERDICT FROM THE READER'S DIAGNOSTICS --- fm-pdf-verify: REJECTED old-path-pdfunite.pdf - does not conform to the PDF specification exit status: 1 --- NEW PATH: the gated generation step --- fm-pdf-finish: published handover-final.pdf (27 pages, conforming) fm-pdf-verify: OK handover-final.pdf (27 pages, conforming) FILE BYTES PAGES VERDICT printed.pdf 268978 27 conforming old-path-pdfunite.pdf 1492205 27 REJECTED handover-final.pdf 64245 27 conforming content check: word count 1693 -> 1693, 27 page breaks, pdftotext output byte-identical; cover + table of contents + all 5 divider pages present. --- PUBLICATION IS ATOMIC AND CONDITIONAL --- fm-pdf-verify: REJECTED .fm-pdf-finish.SfGvb4 - has 27 pages, expected 15 fm-pdf-finish: the finished document was rejected - nothing published to deliverable.pdf previous deliverable byte-identical afterwards: yes temp files left beside destination: 0 --- FAILS CLOSED WHEN THE CHECK CANNOT BE PERFORMED --- fm-pdf-finish: no PDF producer found (looked for 'fm-no-such-reader'; install ghostscript) exit status: 3 (nothing published); destination file created: no fm-pdf-verify: CANNOT VERIFY printed.pdf - the PDF reader produced no recognizable page report exit status: 3 (3, not 0 - silence is never a pass)Evidence: Published deliverable produced by bin/fm-pdf-finish.sh (27 pages, conforming, 64,245 bytes)
Evidence: The rejected old-path assembly (pdfseparate + pdfunite, 1,492,205 bytes, non-conforming)
Evidence: Targeted suite output through the repo runner
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 6 issues found → auto-fixed (2) ✅
bin/fm-pdf-finish.sh:158- On the success path the script re-runs the verifier against the already-published file, and that call is the script's last command, so its status becomes the script's exit status. The header contract (line 33) states "0 published; 1 the gate rejected the result (nothing published); 3 the gate could not run (nothing published)", and docs/pdf-output.md:140 tells callers to treat any non-zero exit as a failure - so a caller would conclude nothing was published while the file is in fact live at <out>. The authoritative gate already ran on the temp file at line 136; this second full interpretation pass also doubles the reader cost on every successful generation (a 27-page document is read twice) just to print one line. Fix: keep the success line but stop it from setting the exit status (e.g. follow the block with an explicitexit 0, or reuse the first run's output).tests/fm-pdf-output.test.sh:25- The whole suite exits 0 withskip: ghostscript not foundwhen gs is absent. bin/fm-test-run.sh classifies this file as family "unclassified" (expected_gate_skip=none) and records the gate skip without failing, and no CI job installs or requires ghostscript - .github/workflows/ci.yml's serial lane, which is where this test lands, only hard-requires tmux and tasks-axi. If the runner image lacks gs (or a future image drops it), the only proof of the conformance gate silently stops running while CI stays green - the same class of "looks like an assurance and is not one" failure this change exists to prevent. Consider acommand -v gs || errorstep in the serial lane, or--fail-on-gate-skip 'ghostscript not found'there.tests/fm-pdf-output.test.sh:69- The assertion requires the exact Ghostscript warning phrase "incorrect xref size". That wording is specific to the version recorded in docs/pdf-output.md (10.02.1); other releases word the damaged-xref warning differently while still emitting the "does not conform" banner that the gate actually matches on. No gs version is pinned anywhere in the repo (unlike ShellCheck via bin/fm-lint.sh), so a runner with a different Ghostscript fails this assertion even though the gate behaved correctly. The assertion one line above ("does not conform") already proves the diagnosis reaches the caller.bin/fm-pdf-verify.sh:94- The reader invocation omits -dSAFER while the producer in bin/fm-pdf-finish.sh:118 passes it. The script header documents calling this directly "to audit a file that already exists", i.e. against files this repo did not produce, and Ghostscript interprets whatever it is handed. This is a no-op on gs >= 9.50 where SAFER is the default, but FM_PDF_GS lets an older or wrapped reader be substituted, and the asymmetry with the producer is unexplained. Adding -dSAFER costs nothing and does not affect the diagnostics the verdict is taken from.bin/fm-pdf-verify.sh:118- The comment immediately above says "A reader that failed to run at all is an unverifiable result, not a pass", but the branch returns 1, so the caller loop prints "REJECTED <file> - the PDF reader failed" and bin/fm-pdf-finish.sh:144 prints "the finished document was rejected". A gs that is installed but broken (missing shared library, OOM, sandbox denial) therefore reads as a condemned document rather than a check that could not run - exactly the confusion the separate exit-3 "CANNOT VERIFY" class exists to avoid. Fail-closed behaviour is preserved either way (nothing is published), so this is diagnostics only; tests/fm-pdf-output.test.sh:113 pins the current exit 1, so if the classification is deliberate the comment should be reworded instead.bin/fm-pdf-finish.sh:58- The --pages/--quiet/-h/--/-* option loop with its integer validation (finish lines 58-75) and the GS_BIN=${FM_PDF_GS:-gs} resolution are duplicated near-verbatim from bin/fm-pdf-verify.sh:52-69 and :73, about 20 lines differing only in the message prefix. The repo already uses bin/fm-*-lib.sh for exactly this kind of sharing. The counter-argument is that both scripts stay standalone with no sourcing dependency, which has value for a gate that must not fail for structural reasons - flagging so the tradeoff is a choice rather than drift.🔧 Fix: fix(pdf): correct exit status, reader-failure class, and gate skips
3 issues (1 error, 2 infos) still open:
bin/fm-pdf-lib.sh:1- The new library breaksbin/fm-test-run.sh --changed. bin/fm-pdf-lib.sh matches no explicit arm in families_for_changed_path, so it falls through tobin/*)(bin/fm-test-run.sh:705) and families_for_test_reference greps the test files for the literal string "fm-pdf-lib.sh" - which appears in no test. The function returns 1, the path is emitted as__unmapped__, and select_changed callsdie(bin/fm-test-run.sh:734). Verified by running the listing mode only (no tests executed):bin/fm-test-run.sh --list --changed --base a669313prints "fm-test-run: no changed-test mapping for source path: bin/fm-pdf-lib.sh" and exits 2 with zero scripts selected. Every changed-file selection on this branch therefore aborts, including the 48-test regression set the change's own verification claims, and it stays broken for any future change that touches this file. The sibling scripts are unaffected because tests/fm-pdf-output.test.sh names them. Fix: either reference bin/fm-pdf-lib.sh from tests/fm-pdf-output.test.sh so the reference lookup resolves, or add abin/fm-pdf-*arm to families_for_changed_path alongside the other explicit library mappings.tests/fm-pdf-output.test.sh:161- With the new rule, a non-zero reader exit is only REJECTED when the output matches one of the four literal phrases in bin/fm-pdf-verify.sh:132-153; otherwise it becomes CANNOT VERIFY. Truncated and non-PDF files are therefore classified by Ghostscript's message wording rather than by its exit status, yet these two tests (lines 161 and 170) assert exit 1 exactly. The CI lane now installs whatever ghostscript the runner image ships and no gs version is pinned anywhere, so a wording change in a future release silently reclassifies these to exit 3 and fails both assertions even though the gate still refuses the file - the same version-coupling class that was just removed from theincorrect xref sizeassertion, and in tension with the claim in docs/pdf-output.md:148 that the suite does not assert on version-specific wording. Asserting "refused" (non-zero, no OK line) for these two real-file cases would keep the captain's requirement that a truncated or non-PDF file is still refused, while the deterministic stub tests at lines 120-144 already lock the REJECTED/CANNOT VERIFY distinction exactly.tests/fm-pdf-output.test.sh:31-[ -n "${CI:-}" ]is a brand-new convention here - no other script or test in this repo reads CI or GITHUB_ACTIONS. CI is exported by many local toolchains and agent harnesses, not just GitHub Actions, so a developer whose shell has it set and who lacks ghostscript gets a hard "not ok" instead of the skip that docs/pdf-output.md promises ("A developer run without Ghostscript may still skip"). The lane-level--fail-on-gate-skip 'ghostscript not found'plus the explicit Require-Ghostscript step already close the CI hole through the repo's own mechanism; if the second lock is wanted, GITHUB_ACTIONS matches the actual CI this guards.🔧 Fix: fix(pdf): map shared lib for changed-test selection, unpin refusal class
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-pdf-output.test.sh— 25 assertions green, no gate skip (Ghostscript 10.02.1 and poppler both present, so the real-pdfunite field-defect branch ran)Manual end-to-end reproduction: generated a 27-page handover HTML (cover, table of contents, 5 dividers), printed it withchrome-headless-shell --print-to-pdf, then assembled it both wayspdfseparate+pdfuniteon the browser output — inspected the result: trailer/Size 353, xref subsection0 41798, highest object41797 0 obj(PDF 32000-1 §7.5.5 violation reproduced)gs -o /dev/null -sDEVICE=nullpage -dNOPAUSE -dBATCH -dPDFSTOPONERROR -dPDFSTOPONWARNING <broken.pdf>— prints 'does not conform' and exits 0;gs -q ...exits 0 with diagnostics suppressedbin/fm-pdf-verify.sh old-path-pdfunite.pdf— exit 1, REJECTED, names the conformance failurebin/fm-pdf-finish.sh --pages 27 handover-final.pdf parts/p-*.pdf— exit 0, published 27 pages conforming, 64,245 bytes vs the 1,492,205-byte rejected assemblybin/fm-pdf-verify.sh --pages 27 handover-final.pdf— independent read-back, OKContent preservation:pdfinfopage counts,pdftotextdiff (identical, 1693 words, 27 page breaks), cover/contents/5 divider pages all presentpdftoppm -pngof the published file, composed into a 27-page contact sheet for visual confirmationAtomicity:bin/fm-pdf-finish.sh --pages 15 deliverable.pdf ...— exit 1, previous deliverable byte-identical (cksum), destination absent when it did not pre-exist, no.fm-pdf-finish.*leftoversFail-closed:FM_PDF_GS=fm-no-such-reader bin/fm-pdf-finish.sh— exit 3, nothing published;FM_PDF_GS=<stub exiting 0 silently> bin/fm-pdf-verify.sh— exit 3 CANNOT VERIFYbin/fm-pdf-verify.shon a truncated PDF — REJECTED (exit 1), not downgraded to unverifiablebin/fm-test-run.sh --list --lane portable-serialand--list --changed --base a669313— the new suite is in the CI lane that installs Ghostscript and passes --fail-on-gate-skip, and is selected by changed-file mappingdocs/configuration.md:341- AGENTS.md now instructs that every captain PDF deliverable goes through bin/fm-pdf-finish.sh, which hard-requires Ghostscript, but Ghostscript is not part of bootstrap's detected toolchain (docs/configuration.md "Toolchain" is the single owner of that list and bin/fm-bootstrap.sh never probes gs). I documented the prerequisite in docs/pdf-output.md, the scripts' owner doc, and deliberately did not add it to the Toolchain list, because that section describes what bootstrap actually detects and offers to install - listing gs there would be false until the code detects it. Decide whether Ghostscript should become a bootstrap-detected tool (code change, out of scope for a docs-only change) or stay an on-demand dependency surfaced only by the scripts' exit 3.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.