diff --git a/README.md b/README.md index 0c247e9ea..5b2ab6e5d 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ Common slash commands: |---|---| | `/model`, `/provider` | switch the active model/provider | | `/spec`, `/plan` | draft and review a plan before building | -| `/image` | attach an image for vision-capable models | +| `/image` | attach an image for vision-capable models, or PDF text with Poppler's `pdftotext` installed | | `/resume`, `/rewind` | continue or roll back local sessions | | `/new` | start a fresh session in place (previous session stays on disk) | | `/btw [question]` | ask in an isolated fork without adding the side conversation to the main session | @@ -221,6 +221,12 @@ Common slash commands: | `/add-dir` | allow an extra write directory for this session | | `/theme`, `/doctor`, `/config` | adjust appearance and inspect setup | +PDF text attachments use Poppler's `pdftotext` executable. Install Poppler with +your platform's package manager and ensure `pdftotext` is on `PATH`; optional +PDF page images for vision models also require Poppler's `pdftoppm`. A vision +model can still receive those rendered page images when a PDF has no usable text +layer or `pdftotext` cannot extract one. + ### Headless `exec` ```bash diff --git a/go.mod b/go.mod index 47e9c2894..7a49dd68f 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/charmbracelet/x/ansi v0.11.7 github.com/charmbracelet/x/term v0.2.2 github.com/coder/websocket v1.8.15 - github.com/ledongthuc/pdf v0.0.0-20250511090121-5959a4027728 golang.org/x/image v0.45.0 golang.org/x/sys v0.47.0 mvdan.cc/sh/v3 v3.13.1 diff --git a/go.sum b/go.sum index 7af86ff65..88f18b425 100644 --- a/go.sum +++ b/go.sum @@ -48,8 +48,6 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/ledongthuc/pdf v0.0.0-20250511090121-5959a4027728 h1:QwWKgMY28TAXaDl+ExRDqGQltzXqN/xypdKP86niVn8= -github.com/ledongthuc/pdf v0.0.0-20250511090121-5959a4027728/go.mod h1:1fEHWurg7pvf5SG6XNE5Q8UZmOwex51Mkx3SLhrW5B4= github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= github.com/lucasb-eyer/go-colorful v1.4.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-runewidth v0.0.24 h1:cpokDiIn0MGnhdHwuWnJBITySJ20QyNGnY2kR/ay2DU= @@ -64,8 +62,6 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= golang.org/x/exp v0.0.0-20260611194520-c48552f49976 h1:X8Hz2ImujgbmetVuW+w2YkyZChE3cBpZi2P158rTG9M= golang.org/x/exp v0.0.0-20260611194520-c48552f49976/go.mod h1:vnf4pv9iKZXY58sQE1L86zmNWJ4159e1RkcWiLCkeEY= -golang.org/x/image v0.44.0 h1:+tDekMZED9+LrtB3G5xzRggpVh9CARjZqROla3R3R+I= -golang.org/x/image v0.44.0/go.mod h1:V8K3KE9KKKE+pLpQDOeN18w9oacNSvy1tDOirTu4xtY= golang.org/x/image v0.45.0 h1:FMb1nTbH5H9vF55SriQHgFw5GnNL9Jg6L25BwXKzhB0= golang.org/x/image v0.45.0/go.mod h1:n62x/7RqlwXDvGsSU4u6IUTUf6KghUZ9Bt7cG/T9Fx4= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= diff --git a/internal/imageinput/pdf.go b/internal/imageinput/pdf.go index 93ac81023..019f52c41 100644 --- a/internal/imageinput/pdf.go +++ b/internal/imageinput/pdf.go @@ -11,21 +11,22 @@ import ( "path/filepath" "sort" "strings" + "sync" "time" "github.com/Gitlawb/zero/internal/zeroruntime" - "github.com/ledongthuc/pdf" ) -// Dependency posture (see stage 12): the DEFAULT build extracts a PDF's text -// layer in pure Go via github.com/ledongthuc/pdf (BSD-licensed, no CGO, no -// transitive deps), so ZERO stays a single static cross-compilable binary with -// no runtime dependencies. Rasterizing pages to images for vision models needs -// real font/graphics rendering, which no maintained pure-Go library does well; -// that path is OPTIONAL and uses the poppler tools (pdftotext / pdftoppm) only -// when they are already on PATH -- the same "external tool the user may have" -// posture as the LSP language servers. When poppler is absent, extraction -// silently degrades to the pure-Go text layer; absence is never an error. +// Dependency posture: PDF text extraction uses Poppler's pdftotext when it is +// on PATH and disableExternalTools is false. We intentionally do not retain an +// in-process parser fallback: the previously used parser materialized all +// decompressed page text in Zero's own process before exposing a reader. Poppler +// runs in a separately cancellable process with a capped captured output and a +// fixed deadline. Rasterizing pages to images for vision models needs +// real font/graphics rendering and uses pdftoppm only when it is already on +// PATH -- the same "external tool the user may have" posture as the LSP +// language servers. When Poppler is unavailable, text extraction fails clearly +// instead of processing an untrusted document without enforceable limits. // MaxDocumentBytes is the per-document raw-file cap (32 MiB). PDFs are routinely // larger than the image cap, but we still bound the file before it is read into @@ -38,6 +39,11 @@ const MaxDocumentBytes = 32 << 20 // usable instead of refused outright. const MaxDocumentTextBytes = 256 << 10 +// maxPDFInfoOutputBytes bounds the small metadata response consumed from +// pdfinfo. It is intentionally separate from the text cap because page-count +// output is not exposed to the model. +const maxPDFInfoOutputBytes = 64 << 10 + // documentTruncatedMarker is appended to capped text so the agent (and the user) // can tell extraction was cut short rather than the document simply ending. const documentTruncatedMarker = "\n\n[... document text truncated at the size limit ...]" @@ -47,17 +53,28 @@ const documentTruncatedMarker = "\n\n[... document text truncated at the size li // DocumentOptions.MaxPages. const defaultMaxRasterPages = 10 -// popplerTimeout bounds each external poppler invocation so a wedged or -// pathological binary cannot hang the CLI/TUI. +// maxRasterDimension caps both dimensions passed to pdftoppm. The resulting +// bitmap is below the per-image byte cap even before PNG compression, preventing +// a tiny PDF with an enormous media box from filling temporary storage. +const maxRasterDimension = 1536 + +// popplerTimeout bounds the whole Poppler phase of one PDF attachment so a +// wedged or pathological document cannot multiply the synchronous CLI/TUI wait +// across rasterization, text extraction, and page counting. const popplerTimeout = 30 * time.Second +// rasterTimeout bounds optional page rendering independently. It lets a vision +// attachment retain useful diagrams/layout without letting rendering outlive the +// user-facing extraction deadline or multiply it serially. +var rasterOperationTimeout = 10 * time.Second + // pdfMagic is the leading signature of every PDF stream. Detection keys on these // bytes, never on the file extension alone. var pdfMagic = []byte("%PDF-") -// Document is the result of ingesting a PDF: the extracted text layer (always -// populated when a text layer exists) plus, on the optional vision path, one -// ImageBlock per rendered page. Pages is the page count the parser reported; +// Document is the result of ingesting a PDF: its extracted text layer when the +// bounded extractor succeeds plus, on the optional vision path, one ImageBlock +// per rendered page. Pages is best-effort external metadata and may be zero; // Truncated is set when Text was capped at MaxDocumentTextBytes. type Document struct { Text string @@ -76,9 +93,8 @@ type DocumentOptions struct { // defaultMaxRasterPages. MaxPages int - // disableExternalTools forces the pure-Go path even if poppler is installed. - // It exists so tests are deterministic on any host; it is intentionally - // unexported and not part of the public surface. + // disableExternalTools simulates an unavailable Poppler installation for + // deterministic tests. It is intentionally unexported and not public API. disableExternalTools bool } @@ -131,9 +147,8 @@ func LooksLikeDocumentFile(path string, workspaceRoot string) bool { // With opts.Vision and an available rasterizer it also renders the first N pages // to ImageBlocks. The file is identified by magic bytes, not its extension, so a // ".pdf"-named non-PDF is rejected with a clear error. A PDF with no text layer -// and no rasterization/OCR available returns an explicit "no extractable text" -// error rather than a silent empty success. Errors are plain (callers wrap them -// into surface-specific notice text). +// and no rasterization/OCR available returns an explicit error rather than a +// silent empty success. Errors are plain (callers wrap them into surface-specific notice text). func LoadDocument(path string, workspaceRoot string, opts DocumentOptions) (Document, error) { data, err := readDocumentBytes(path, workspaceRoot) if err != nil { @@ -145,49 +160,81 @@ func LoadDocument(path string, workspaceRoot string, opts DocumentOptions) (Docu useExternal := !opts.disableExternalTools - // Vision path (optional): render pages to images via poppler when available. - // Failures here are non-fatal -- we still return the text layer below. + // Start the independent Poppler operations under one deadline. LoadDocument + // runs on the synchronous /image path; running these serially would let one + // hostile PDF spend a separate timeout in each process. var images []zeroruntime.ImageBlock - if opts.Vision && useExternal { - if rendered, rerr := rasterizeWithPoppler(data, opts.maxPages()); rerr == nil { - images = rendered - } - } - - // Text path. Prefer poppler's pdftotext when present (it handles more font - // encodings); otherwise use the pure-Go extractor. Either way, absence of the - // external tool is not an error. - text, pages := "", 0 + textResult := popplerTextResult{status: popplerTextUnavailable} + pages := 0 if useExternal { - if t, ok := extractTextWithPoppler(data); ok { - text = t - // pdftotext does not report a page count, so derive it from the pure-Go - // reader (cheap structural read, no text extraction) to keep - // Document.Pages correct regardless of which text path wins. - pages = pdfPageCount(data) + ctx, cancel := context.WithTimeout(context.Background(), popplerOperationTimeout) + var work sync.WaitGroup + textDone := make(chan struct{}) + var rasterDone <-chan struct{} + work.Add(2) + go func() { + defer work.Done() + defer close(textDone) + textResult = popplerTextExtractor(ctx, data) + }() + go func() { + defer work.Done() + pages = popplerPageCounter(ctx, data) + }() + if opts.Vision { + done := make(chan struct{}) + rasterDone = done + work.Add(1) + go func() { + defer work.Done() + defer close(done) + rasterCtx, rasterCancel := context.WithTimeout(ctx, rasterOperationTimeout) + defer rasterCancel() + // Rendering is optional: text remains usable if it fails or times out. + if rendered, rerr := popplerRasterizer(rasterCtx, data, opts.maxPages()); rerr == nil { + images = rendered + } + }() } - } - if strings.TrimSpace(text) == "" { - t, p, terr := extractTextPureGo(data) - if terr != nil { - // Only surface the pure-Go error when we have nothing else (no poppler - // text and no rasterized pages) to offer. - if len(images) == 0 { - return Document{}, terr - } - } else { - text, pages = t, p + <-textDone + // Page count is informational. Rendering is optional but, when requested, + // contributes usable vision input and has its own shorter deadline. + if rasterDone != nil { + <-rasterDone } + cancel() + work.Wait() } - text, truncated := capDocumentText(text) + // Text path. Poppler output is retained through a bounded writer. There is no + // in-process fallback because its parser cannot enforce this boundary before + // decompression and text aggregation. + text, textOverflow := "", false + textStatus := textResult.status + if textStatus == popplerTextExtracted { + text, textOverflow = textResult.text, textResult.overflow + } + + // Decide whether any usable text exists before adding a truncation marker. + // Otherwise whitespace-only overflow could turn into a marker-only document + // that bypasses the no-text guard below. + hasText := strings.TrimSpace(text) != "" + if !hasText { + text, textOverflow = "", false + } + text, truncated := capDocumentTextWithOverflow(text, textOverflow) // Scanned-PDF guard: no text layer AND no rendered pages means we have nothing // the model can use. Say so explicitly instead of returning empty success. - if strings.TrimSpace(text) == "" && len(images) == 0 { - return Document{}, fmt.Errorf("%s has no extractable text; OCR is not available (install poppler's pdftotext/pdftoppm for image-only PDFs)", path) + if !hasText && len(images) == 0 { + if textStatus == popplerTextFailed { + return Document{}, fmt.Errorf("%s could not extract PDF text with pdftotext", path) + } + if textStatus == popplerTextUnavailable { + return Document{}, fmt.Errorf("%s has no extractable text; install Poppler's pdftotext for PDF text extraction (and pdftoppm for image-only PDFs)", path) + } + return Document{}, fmt.Errorf("%s has no extractable text; PDF OCR is not available", path) } - return Document{Text: text, Images: images, Pages: pages, Truncated: truncated}, nil } @@ -232,59 +279,19 @@ func readDocumentBytes(path string, workspaceRoot string) ([]byte, error) { return data, nil } -// extractTextPureGo extracts the full text layer with the pure-Go parser. The -// ledongthuc/pdf parser panics (not errors) on some malformed structures, so the -// whole call is wrapped in a recover: a bad PDF becomes a clean error, never a -// crash that escapes the package. It returns the joined text and the page count. -func extractTextPureGo(data []byte) (text string, pages int, err error) { - defer func() { - if rec := recover(); rec != nil { - text, pages = "", 0 - err = fmt.Errorf("could not parse PDF (malformed or unsupported): %v", rec) - } - }() - - reader, rerr := pdf.NewReader(bytes.NewReader(data), int64(len(data))) - if rerr != nil { - return "", 0, fmt.Errorf("could not parse PDF: %w", rerr) - } - pages = reader.NumPage() - - var buf strings.Builder - plain, perr := reader.GetPlainText() - if perr != nil { - return "", pages, fmt.Errorf("could not extract PDF text: %w", perr) - } - if _, cerr := io.Copy(&buf, plain); cerr != nil { - return "", pages, fmt.Errorf("could not read PDF text: %w", cerr) - } - return strings.TrimSpace(buf.String()), pages, nil -} - -// pdfPageCount returns the page count via the pure-Go reader without extracting -// any text. It backs Document.Pages on the poppler text path (pdftotext does not -// report a count). Like extractTextPureGo it recovers from the parser's panics on -// malformed input and reports 0 rather than crashing -- the page count is -// informational, so an unreadable structure simply yields 0. -func pdfPageCount(data []byte) (pages int) { - defer func() { - if recover() != nil { - pages = 0 - } - }() - reader, err := pdf.NewReader(bytes.NewReader(data), int64(len(data))) - if err != nil { - return 0 - } - return reader.NumPage() -} - // capDocumentText truncates text to MaxDocumentTextBytes on a UTF-8 rune // boundary and appends documentTruncatedMarker when it had to cut. The second // return reports whether truncation happened. The marker is counted against the // cap so the returned string never exceeds MaxDocumentTextBytes. func capDocumentText(text string) (string, bool) { - if len(text) <= MaxDocumentTextBytes { + return capDocumentTextWithOverflow(text, false) +} + +// capDocumentTextWithOverflow applies the model text cap and preserves a +// truncation signal from a bounded upstream reader. That signal is necessary +// when trimming whitespace makes the retained string appear to fit the cap. +func capDocumentTextWithOverflow(text string, overflow bool) (string, bool) { + if !overflow && len(text) <= MaxDocumentTextBytes { return text, false } // Reserve room for the marker so the final payload (text + marker) stays at or @@ -294,8 +301,11 @@ func capDocumentText(text string) (string, bool) { if cut < 0 { cut = 0 } + if cut > len(text) { + cut = len(text) + } // Back up to a rune boundary so we never split a multi-byte character. - for cut > 0 && !utf8RuneStart(text[cut]) { + for cut > 0 && cut < len(text) && !utf8RuneStart(text[cut]) { cut-- } return text[:cut] + documentTruncatedMarker, true @@ -308,8 +318,31 @@ func utf8RuneStart(b byte) bool { return b&0xC0 != 0x80 } +type popplerTextStatus uint8 + +const ( + popplerTextUnavailable popplerTextStatus = iota + popplerTextFailed + popplerTextExtracted +) + +type popplerTextResult struct { + text string + overflow bool + status popplerTextStatus +} + +var ( + popplerTextExtractor = extractTextWithPoppler + popplerPageCounter = pdfPageCountWithPoppler + popplerRasterizer = rasterizeWithPoppler + popplerLookup = popplerAvailable + popplerCommandWithContext = exec.CommandContext + popplerOperationTimeout = popplerTimeout +) + func (o DocumentOptions) maxPages() int { - if o.MaxPages > 0 { + if o.MaxPages > 0 && o.MaxPages < defaultMaxRasterPages { return o.MaxPages } return defaultMaxRasterPages @@ -323,35 +356,99 @@ func popplerAvailable(name string) bool { return err == nil } -// extractTextWithPoppler runs `pdftotext - -` (read stdin, write stdout) when -// pdftotext is on PATH. The bool is false when the tool is absent or failed, so -// the caller can fall back to the pure-Go extractor. Absence is never an error. -func extractTextWithPoppler(data []byte) (string, bool) { - if !popplerAvailable("pdftotext") { - return "", false +// extractTextWithPoppler runs `pdftotext - -` (read stdin, write stdout). It +// keeps executable discovery distinct from execution failure so callers can +// provide accurate, non-sensitive remediation without exposing tool stderr. +func extractTextWithPoppler(ctx context.Context, data []byte) popplerTextResult { + if !popplerLookup("pdftotext") { + return popplerTextResult{status: popplerTextUnavailable} } - ctx, cancel := context.WithTimeout(context.Background(), popplerTimeout) + ctx, cancel := context.WithCancel(ctx) defer cancel() - // "-layout" keeps the visual column layout; the trailing "- -" reads the PDF // from stdin and writes UTF-8 text to stdout. - cmd := exec.CommandContext(ctx, "pdftotext", "-layout", "-enc", "UTF-8", "-", "-") + cmd := popplerCommandWithContext(ctx, "pdftotext", "-layout", "-enc", "UTF-8", "-", "-") cmd.Stdin = bytes.NewReader(data) - var stdout, stderr bytes.Buffer + stdout := newBoundedBuffer(MaxDocumentTextBytes) + stdout.onOverflow = cancel cmd.Stdout = &stdout - cmd.Stderr = &stderr + cmd.Stderr = io.Discard if err := cmd.Run(); err != nil { - return "", false + if stdout.overflow { + return popplerTextResult{text: strings.TrimSpace(stdout.String()), overflow: true, status: popplerTextExtracted} + } + return popplerTextResult{status: popplerTextFailed} + } + return popplerTextResult{text: strings.TrimSpace(stdout.String()), overflow: stdout.overflow, status: popplerTextExtracted} +} + +func pdfPageCountWithPoppler(ctx context.Context, data []byte) int { + if !popplerLookup("pdfinfo") { + return 0 + } + cmd := popplerCommandWithContext(ctx, "pdfinfo", "-") + cmd.Stdin = bytes.NewReader(data) + var out boundedBuffer + out.limit = maxPDFInfoOutputBytes + cmd.Stdout = &out + cmd.Stderr = io.Discard + if err := cmd.Run(); err != nil || out.overflow { + return 0 + } + for _, line := range strings.Split(out.String(), "\n") { + if value, ok := strings.CutPrefix(strings.TrimSpace(line), "Pages:"); ok { + var pages int + if _, err := fmt.Sscan(value, &pages); err == nil { + return pages + } + } + } + return 0 +} + +// boundedBuffer retains at most limit+1 bytes while accepting the complete +// write. The extra byte distinguishes exact-limit output from overflow without +// allowing a subprocess or parser to grow memory without bound. +type boundedBuffer struct { + buffer bytes.Buffer + limit int + overflow bool + onOverflow func() +} + +func newBoundedBuffer(limit int) boundedBuffer { + return boundedBuffer{limit: limit} +} + +func (buffer *boundedBuffer) Write(data []byte) (int, error) { + remaining := buffer.limit + 1 - buffer.buffer.Len() + if remaining > 0 { + if remaining > len(data) { + remaining = len(data) + } + _, _ = buffer.buffer.Write(data[:remaining]) + } + if buffer.buffer.Len() > buffer.limit { + if !buffer.overflow { + buffer.overflow = true + if buffer.onOverflow != nil { + buffer.onOverflow() + } + } } - return strings.TrimSpace(stdout.String()), true + return len(data), nil } +func (buffer *boundedBuffer) Len() int { return buffer.buffer.Len() } + +func (buffer *boundedBuffer) String() string { return buffer.buffer.String() } + // rasterizeWithPoppler renders the first maxPages pages to PNG via pdftoppm and // returns them as normalized ImageBlocks (reusing the image allow-list, sniff, // and per-image cap). It returns an error when pdftoppm is absent or rendering // produced nothing; the caller treats that as "no rasterization available" and // keeps the text layer. -func rasterizeWithPoppler(data []byte, maxPages int) ([]zeroruntime.ImageBlock, error) { +func rasterizeWithPoppler(ctx context.Context, data []byte, maxPages int) ([]zeroruntime.ImageBlock, error) { if !popplerAvailable("pdftoppm") { return nil, fmt.Errorf("pdftoppm not available") } @@ -365,16 +462,14 @@ func rasterizeWithPoppler(data []byte, maxPages int) ([]zeroruntime.ImageBlock, } defer os.RemoveAll(dir) - ctx, cancel := context.WithTimeout(context.Background(), popplerTimeout) - defer cancel() - prefix := filepath.Join(dir, "page") - // -png: PNG output; -r 150: 150 DPI (legible without huge files); - // -f 1 / -l N: render only the first N pages so context can't blow up. - cmd := exec.CommandContext(ctx, "pdftoppm", "-png", "-r", "150", "-f", "1", "-l", fmt.Sprintf("%d", maxPages), "-", prefix) + // -png: PNG output; -r 150: legible default resolution; -scale-to limits + // each output bitmap's largest dimension; -f 1 / -l N limits page count. + cmd := exec.CommandContext(ctx, "pdftoppm", "-png", "-r", "150", "-scale-to", fmt.Sprintf("%d", maxRasterDimension), "-f", "1", "-l", fmt.Sprintf("%d", maxPages), "-", prefix) cmd.Stdin = bytes.NewReader(data) - var stderr bytes.Buffer - cmd.Stderr = &stderr + // Renderer diagnostics are not surfaced to callers; retaining hostile tool + // output would bypass the attachment's bounded-output contract. + cmd.Stderr = io.Discard if err := cmd.Run(); err != nil { return nil, fmt.Errorf("pdftoppm failed: %w", err) } diff --git a/internal/imageinput/pdf_test.go b/internal/imageinput/pdf_test.go index 614250ab3..54ca161af 100644 --- a/internal/imageinput/pdf_test.go +++ b/internal/imageinput/pdf_test.go @@ -2,21 +2,25 @@ package imageinput import ( "bytes" + "context" "fmt" + "io" "os" + "os/exec" "path/filepath" "strconv" "strings" "testing" + "time" + + "github.com/Gitlawb/zero/internal/zeroruntime" ) const minimalPDFTextChunkSize = 80 // buildMinimalPDF assembles a tiny, single-page PDF whose content stream draws -// the given text. It computes a real cross-reference table and trailer so a -// pure-Go PDF parser (ledongthuc/pdf) accepts it. Generating the fixture in-test -// keeps the repo free of opaque binary blobs while still exercising the real -// text-extraction path on real PDF bytes. +// the given text. It computes a real cross-reference table and trailer, keeping +// the repo free of opaque binary blobs for PDF routing tests. func buildMinimalPDF(text string) []byte { var buf bytes.Buffer offsets := make([]int, 0, 8) @@ -129,6 +133,7 @@ func TestLoadDocumentTextExtraction(t *testing.T) { if err := os.WriteFile(filepath.Join(root, "doc.pdf"), buildMinimalPDF(want), 0o644); err != nil { t.Fatalf("write pdf: %v", err) } + stubPDFTools(t, want, false, 1) doc, err := LoadDocument("doc.pdf", root, DocumentOptions{}) if err != nil { @@ -145,6 +150,50 @@ func TestLoadDocumentTextExtraction(t *testing.T) { } } +func TestExtractTextWithPoppler(t *testing.T) { + originalLookup, originalCommand := popplerLookup, popplerCommandWithContext + popplerLookup = func(name string) bool { return name == "pdftotext" } + popplerCommandWithContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=TestPDFCommandHelper") + cmd.Env = append(os.Environ(), "ZERO_PDF_HELPER_MODE=fail") + return cmd + } + t.Cleanup(func() { + popplerLookup, popplerCommandWithContext = originalLookup, originalCommand + }) + + result := extractTextWithPoppler(t.Context(), buildMinimalPDF("ignored by helper")) + if result.status != popplerTextFailed { + t.Fatalf("status = %d, want execution failure", result.status) + } +} + +func TestPDFCommandHelper(t *testing.T) { + switch os.Getenv("ZERO_PDF_HELPER_MODE") { + case "fail": + os.Exit(1) + case "flood": + _, _ = os.Stdout.WriteString(strings.Repeat("x", MaxDocumentTextBytes+1024)) + os.Exit(0) + } +} + +func TestExtractTextWithPopplerCancelsOnOverflow(t *testing.T) { + originalLookup, originalCommand := popplerLookup, popplerCommandWithContext + popplerLookup = func(name string) bool { return name == "pdftotext" } + popplerCommandWithContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=TestPDFCommandHelper") + cmd.Env = append(os.Environ(), "ZERO_PDF_HELPER_MODE=flood") + return cmd + } + t.Cleanup(func() { popplerLookup, popplerCommandWithContext = originalLookup, originalCommand }) + + result := extractTextWithPoppler(t.Context(), buildMinimalPDF("ignored")) + if result.status != popplerTextExtracted || !result.overflow { + t.Fatalf("result = %#v, want extracted overflow", result) + } +} + // A .pdf-named file that is not actually a PDF must be rejected with a clear // error rather than silently treated as a document (extension is never trusted // over magic bytes). @@ -218,6 +267,7 @@ func TestLoadDocumentTruncatesLongText(t *testing.T) { if err := os.WriteFile(filepath.Join(root, "long.pdf"), buildMinimalPDF(body.String()), 0o644); err != nil { t.Fatalf("write long: %v", err) } + stubPDFTools(t, body.String(), true, 1) doc, err := LoadDocument("long.pdf", root, DocumentOptions{}) if err != nil { t.Fatalf("LoadDocument: %v", err) @@ -233,25 +283,20 @@ func TestLoadDocumentTruncatesLongText(t *testing.T) { } } -// A PDF with no extractable text layer and no rasterization/OCR available must -// surface the explicit "no extractable text" message, never a silent empty -// success. +// A PDF with no extractable text layer and no rasterization available must +// surface an explicit error, never a silent empty success. func TestLoadDocumentNoTextNoRaster(t *testing.T) { root := t.TempDir() if err := os.WriteFile(filepath.Join(root, "scan.pdf"), buildEmptyTextPDF(), 0o644); err != nil { t.Fatalf("write scan: %v", err) } - // Force the pure-Go path with no external rasterizer so the no-text branch is - // deterministic regardless of what is installed on the test host. + // Simulate a host without Poppler so the no-text branch is deterministic. _, err := LoadDocument("scan.pdf", root, DocumentOptions{disableExternalTools: true}) if err == nil { t.Fatal("expected an error for a PDF with no extractable text and no raster") } if !strings.Contains(err.Error(), "no extractable text") { - t.Fatalf("error %q should explain there is no extractable text", err.Error()) - } - if !strings.Contains(err.Error(), "OCR") { - t.Fatalf("error %q should mention OCR is unavailable", err.Error()) + t.Fatalf("error %q should explain that no text is available", err.Error()) } } @@ -284,8 +329,8 @@ func buildEmptyTextPDF() []byte { return buf.Bytes() } -// Malformed PDF bytes that pass the header check but break the parser must be -// turned into a clean error, never a panic that escapes the package. +// Malformed PDF bytes that pass the header check must produce a clean error +// when no safe extractor is available. func TestLoadDocumentMalformedDoesNotPanic(t *testing.T) { root := t.TempDir() bad := []byte("%PDF-1.4\nthis header is valid but the body and xref are garbage\nstartxref\n9\n%%EOF\n") @@ -298,41 +343,126 @@ func TestLoadDocumentMalformedDoesNotPanic(t *testing.T) { } } -// When the external poppler tools are absent (or disabled), extraction falls -// back to the pure-Go text path and still succeeds; absence is never an error. -func TestLoadDocumentFallsBackToPureGo(t *testing.T) { +func TestLoadDocumentRequiresBoundedExtractor(t *testing.T) { root := t.TempDir() - want := "Pure Go fallback text" - if err := os.WriteFile(filepath.Join(root, "doc.pdf"), buildMinimalPDF(want), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(root, "doc.pdf"), buildMinimalPDF("text"), 0o644); err != nil { t.Fatalf("write pdf: %v", err) } - doc, err := LoadDocument("doc.pdf", root, DocumentOptions{disableExternalTools: true}) + _, err := LoadDocument("doc.pdf", root, DocumentOptions{disableExternalTools: true}) + if err == nil || !strings.Contains(err.Error(), "pdftotext") { + t.Fatalf("LoadDocument error = %v, want extractor guidance", err) + } +} + +func TestLoadDocumentDoesNotMisreportInstalledPopplerFailure(t *testing.T) { + root := t.TempDir() + bad := []byte("%PDF-1.4\nthis header is valid but the body and xref are garbage\nstartxref\n9\n%%EOF\n") + if err := os.WriteFile(filepath.Join(root, "bad.pdf"), bad, 0o644); err != nil { + t.Fatalf("write pdf: %v", err) + } + original := popplerTextExtractor + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { return popplerTextResult{status: popplerTextFailed} } + t.Cleanup(func() { popplerTextExtractor = original }) + + _, err := LoadDocument("bad.pdf", root, DocumentOptions{}) + if err == nil || !strings.Contains(err.Error(), "could not extract PDF text") { + t.Fatalf("LoadDocument error = %v, want extraction failure", err) + } + if strings.Contains(err.Error(), "install Poppler") { + t.Fatalf("LoadDocument error = %q must not claim Poppler is absent", err) + } +} + +func TestLoadDocumentDoesNotMisreportTextlessPDFAsMissingPoppler(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "scan.pdf"), buildEmptyTextPDF(), 0o644); err != nil { + t.Fatalf("write scan: %v", err) + } + original := popplerTextExtractor + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{status: popplerTextExtracted} + } + t.Cleanup(func() { popplerTextExtractor = original }) + + _, err := LoadDocument("scan.pdf", root, DocumentOptions{}) + if err == nil || !strings.Contains(err.Error(), "no extractable text") { + t.Fatalf("LoadDocument error = %v, want textless-PDF guidance", err) + } + if strings.Contains(err.Error(), "install Poppler") { + t.Fatalf("LoadDocument error = %q must not claim Poppler is absent", err) + } +} + +func TestLoadDocumentRejectsWhitespaceOnlyOverflow(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "blank.pdf"), buildEmptyTextPDF(), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + original := popplerTextExtractor + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{text: strings.Repeat(" ", MaxDocumentTextBytes), overflow: true, status: popplerTextExtracted} + } + t.Cleanup(func() { popplerTextExtractor = original }) + + _, err := LoadDocument("blank.pdf", root, DocumentOptions{}) + if err == nil || !strings.Contains(err.Error(), "no extractable text") { + t.Fatalf("LoadDocument error = %v, want textless-PDF guidance", err) + } +} + +func TestLoadDocumentVisionUsesRenderedPagesWhenTextExtractionFails(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "scan.pdf"), buildEmptyTextPDF(), 0o644); err != nil { + t.Fatalf("write scan: %v", err) + } + originalText, originalPages, originalRaster := popplerTextExtractor, popplerPageCounter, popplerRasterizer + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{status: popplerTextUnavailable} + } + popplerPageCounter = func(context.Context, []byte) int { return 1 } + popplerRasterizer = func(context.Context, []byte, int) ([]zeroruntime.ImageBlock, error) { + return []zeroruntime.ImageBlock{{MediaType: "image/png", Data: []byte("png")}}, nil + } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter, popplerRasterizer = originalText, originalPages, originalRaster + }) + + doc, err := LoadDocument("scan.pdf", root, DocumentOptions{Vision: true}) if err != nil { - t.Fatalf("LoadDocument (pure-Go): %v", err) + t.Fatalf("LoadDocument: %v", err) } - if !strings.Contains(doc.Text, want) { - t.Fatalf("pure-Go text %q should contain %q", doc.Text, want) + if doc.Text != "" || len(doc.Images) != 1 || doc.Pages != 1 { + t.Fatalf("Document = %#v, want rendered page with no text", doc) } } -// Vision-mode extraction without an available rasterizer must not error: it -// degrades to the text layer (a vision model can still read the text block). -func TestLoadDocumentVisionWithoutRasterizerUsesText(t *testing.T) { +func TestLoadDocumentVisionUsesText(t *testing.T) { root := t.TempDir() want := "Vision degrade to text" if err := os.WriteFile(filepath.Join(root, "doc.pdf"), buildMinimalPDF(want), 0o644); err != nil { t.Fatalf("write pdf: %v", err) } - doc, err := LoadDocument("doc.pdf", root, DocumentOptions{Vision: true, disableExternalTools: true}) + stubPDFTools(t, want, false, 1) + + doc, err := LoadDocument("doc.pdf", root, DocumentOptions{Vision: true}) if err != nil { - t.Fatalf("LoadDocument (vision, no raster): %v", err) - } - if len(doc.Images) != 0 { - t.Fatalf("no rasterizer available, expected 0 images, got %d", len(doc.Images)) + t.Fatalf("LoadDocument: %v", err) } if !strings.Contains(doc.Text, want) { - t.Fatalf("vision-without-raster should keep text, got %q", doc.Text) + t.Fatalf("vision input should keep text, got %q", doc.Text) + } +} + +func stubPDFTools(t *testing.T, text string, overflow bool, pages int) { + t.Helper() + originalTextExtractor, originalPageCounter := popplerTextExtractor, popplerPageCounter + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{text: text, overflow: overflow, status: popplerTextExtracted} } + popplerPageCounter = func(context.Context, []byte) int { return pages } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter = originalTextExtractor, originalPageCounter + }) } // capDocumentText must keep the final payload (text + marker) at or under the @@ -361,17 +491,210 @@ func TestCapDocumentTextRespectsCap(t *testing.T) { if got != under { t.Fatal("at-cap text must be returned unchanged") } + + got, truncated = capDocumentTextWithOverflow(under, true) + if !truncated { + t.Fatal("upstream overflow must preserve truncation after whitespace trimming") + } + if !strings.HasSuffix(got, documentTruncatedMarker) { + t.Fatal("upstream overflow should add the truncation marker") + } + + got, truncated = capDocumentTextWithOverflow("x", true) + if !truncated || got != "x"+documentTruncatedMarker { + t.Fatalf("short overflow = (%q, %v), want text plus marker without a panic", got, truncated) + } +} + +func TestPDFOutputReadersAreBounded(t *testing.T) { + buffer := newBoundedBuffer(16) + if _, err := buffer.Write([]byte(strings.Repeat("y", 1024))); err != nil { + t.Fatalf("boundedBuffer.Write: %v", err) + } + if !buffer.overflow { + t.Fatal("boundedBuffer should report overflow") + } + if buffer.Len() != 17 { + t.Fatalf("boundedBuffer retained %d bytes, want 17", buffer.Len()) + } + + buffer = newBoundedBuffer(16) + _, _ = buffer.Write([]byte(strings.Repeat("z", 17))) + if !buffer.overflow { + t.Fatal("boundedBuffer must report exactly limit+1 bytes as overflow") + } + + buffer = newBoundedBuffer(16) + overflowed := false + buffer.onOverflow = func() { overflowed = true } + if _, err := io.Copy(&buffer, strings.NewReader(strings.Repeat("q", 1024))); err != nil { + t.Fatalf("io.Copy into boundedBuffer: %v", err) + } + if !buffer.overflow || !overflowed || buffer.Len() != 17 { + t.Fatalf("io.Copy bypassed bound: overflow=%v len=%d", buffer.overflow, buffer.Len()) + } + + buffer = newBoundedBuffer(16) + calls := 0 + buffer.onOverflow = func() { calls++ } + for range 4 { + _, _ = buffer.Write([]byte(strings.Repeat("m", 8))) + } + if !buffer.overflow || calls != 1 || buffer.Len() != 17 { + t.Fatalf("incremental writes: overflow=%v calls=%d len=%d", buffer.overflow, calls, buffer.Len()) + } +} + +func TestLoadDocumentUsesOnePopplerDeadline(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "slow.pdf"), buildMinimalPDF("text"), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + originalText, originalPages, originalTimeout := popplerTextExtractor, popplerPageCounter, popplerOperationTimeout + popplerOperationTimeout = 50 * time.Millisecond + textStarted, pagesStarted := make(chan struct{}), make(chan struct{}) + popplerTextExtractor = func(ctx context.Context, _ []byte) popplerTextResult { + close(textStarted) + <-ctx.Done() + return popplerTextResult{status: popplerTextFailed} + } + popplerPageCounter = func(ctx context.Context, _ []byte) int { + close(pagesStarted) + <-ctx.Done() + return 0 + } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter, popplerOperationTimeout = originalText, originalPages, originalTimeout + }) + + started := time.Now() + _, err := LoadDocument("slow.pdf", root, DocumentOptions{}) + if err == nil || !strings.Contains(err.Error(), "could not extract PDF text") { + t.Fatalf("LoadDocument error = %v, want timed-out extraction failure", err) + } + if elapsed := time.Since(started); elapsed > 250*time.Millisecond { + t.Fatalf("LoadDocument took %s; independent Poppler operations must share one deadline", elapsed) + } + select { + case <-textStarted: + default: + t.Fatal("text extraction did not start") + } + select { + case <-pagesStarted: + default: + t.Fatal("page counting did not start") + } +} + +func TestLoadDocumentDoesNotWaitForInformationalPageCount(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "fast.pdf"), buildMinimalPDF("text"), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + originalText, originalPages, originalTimeout := popplerTextExtractor, popplerPageCounter, popplerOperationTimeout + popplerOperationTimeout = time.Second + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{text: "text", status: popplerTextExtracted} + } + popplerPageCounter = func(ctx context.Context, _ []byte) int { + <-ctx.Done() + return 0 + } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter, popplerOperationTimeout = originalText, originalPages, originalTimeout + }) + + started := time.Now() + doc, err := LoadDocument("fast.pdf", root, DocumentOptions{}) + if err != nil { + t.Fatalf("LoadDocument: %v", err) + } + if doc.Text != "text" || doc.Pages != 0 { + t.Fatalf("Document = %#v, want attached text with no delayed page count", doc) + } + if elapsed := time.Since(started); elapsed > 250*time.Millisecond { + t.Fatalf("LoadDocument took %s; informational page count must not delay attachment", elapsed) + } +} + +func TestLoadDocumentVisionRetainsRasterWhenTextSucceeds(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "fast.pdf"), buildMinimalPDF("text"), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + originalText, originalPages, originalRaster, originalTimeout := popplerTextExtractor, popplerPageCounter, popplerRasterizer, popplerOperationTimeout + popplerOperationTimeout = time.Second + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{text: "text", status: popplerTextExtracted} + } + popplerPageCounter = func(ctx context.Context, _ []byte) int { + <-ctx.Done() + return 0 + } + popplerRasterizer = func(context.Context, []byte, int) ([]zeroruntime.ImageBlock, error) { + return []zeroruntime.ImageBlock{{MediaType: "image/png", Data: []byte("png")}}, nil + } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter, popplerRasterizer, popplerOperationTimeout = originalText, originalPages, originalRaster, originalTimeout + }) + + doc, err := LoadDocument("fast.pdf", root, DocumentOptions{Vision: true}) + if err != nil { + t.Fatalf("LoadDocument: %v", err) + } + if doc.Text != "text" || len(doc.Images) != 1 { + t.Fatalf("Document = %#v, want text and rendered page", doc) + } } -// pdfPageCount must report the real page count from PDF bytes (this is what -// backs Document.Pages on the poppler text path, where pdftotext gives no count) -// and must return 0 -- not panic -- on garbage. -func TestPDFPageCount(t *testing.T) { - if got := pdfPageCount(buildMinimalPDF("one page")); got != 1 { - t.Fatalf("pdfPageCount = %d, want 1", got) +func TestLoadDocumentVisionRasterDeadline(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "slow.pdf"), buildMinimalPDF("text"), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + originalText, originalPages, originalRaster, originalTimeout := popplerTextExtractor, popplerPageCounter, popplerRasterizer, rasterOperationTimeout + rasterOperationTimeout = 50 * time.Millisecond + popplerTextExtractor = func(context.Context, []byte) popplerTextResult { + return popplerTextResult{text: "text", status: popplerTextExtracted} + } + popplerPageCounter = func(context.Context, []byte) int { return 1 } + popplerRasterizer = func(ctx context.Context, _ []byte, _ int) ([]zeroruntime.ImageBlock, error) { + <-ctx.Done() + return nil, ctx.Err() + } + t.Cleanup(func() { + popplerTextExtractor, popplerPageCounter, popplerRasterizer, rasterOperationTimeout = originalText, originalPages, originalRaster, originalTimeout + }) + + started := time.Now() + doc, err := LoadDocument("slow.pdf", root, DocumentOptions{Vision: true}) + if err != nil { + t.Fatalf("LoadDocument: %v", err) } - if got := pdfPageCount([]byte("not a pdf at all")); got != 0 { - t.Fatalf("pdfPageCount on garbage = %d, want 0", got) + if doc.Text != "text" || len(doc.Images) != 0 { + t.Fatalf("Document = %#v, want text after raster deadline", doc) + } + if elapsed := time.Since(started); elapsed > 250*time.Millisecond { + t.Fatalf("LoadDocument took %s; raster deadline was not enforced", elapsed) + } +} + +func TestLoadDocumentHostilePDFDoesNotUseInProcessParser(t *testing.T) { + root := t.TempDir() + cases := map[string][]byte{ + "cycle.pdf": []byte("%PDF-1.4\n1 0 obj\n<< /Type /Catalog /Pages 1 0 R /Parent 1 0 R /Kids [1 0 R] /Count 999999999 /First 1 0 R /Next 1 0 R >>\nendobj\ntrailer\n<< /Root 1 0 R /Size 999999999 >>\nstartxref\n9\n%%EOF\n"), + "hex.pdf": []byte("%PDF-1.4\n1 0 obj\n<< /Type /Catalog /Pages 2 0 R >>\nendobj\nstream\n<" + strings.Repeat("A", 4096) + "\nendstream\n%%EOF\n"), + } + for name, body := range cases { + path := filepath.Join(root, name) + if err := os.WriteFile(path, body, 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } + _, err := LoadDocument(name, root, DocumentOptions{disableExternalTools: true}) + if err == nil || !strings.Contains(err.Error(), "pdftotext") { + t.Fatalf("LoadDocument(%s) error = %v, want extractor guidance", name, err) + } } } @@ -422,3 +745,15 @@ func TestIsProbablyDocumentPath(t *testing.T) { } } } + +func TestDocumentOptionsMaxPagesIsHardCapped(t *testing.T) { + if got := (DocumentOptions{}).maxPages(); got != defaultMaxRasterPages { + t.Fatalf("default max pages = %d, want %d", got, defaultMaxRasterPages) + } + if got := (DocumentOptions{MaxPages: 3}).maxPages(); got != 3 { + t.Fatalf("requested max pages = %d, want 3", got) + } + if got := (DocumentOptions{MaxPages: defaultMaxRasterPages + 1}).maxPages(); got != defaultMaxRasterPages { + t.Fatalf("oversized max pages = %d, want hard cap %d", got, defaultMaxRasterPages) + } +} diff --git a/internal/tui/commands.go b/internal/tui/commands.go index e5ad05af4..91e988144 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -172,7 +172,7 @@ var commandDefinitions = []commandDefinition{ name: "/image", usage: "/image | clear", group: commandGroupSession, - description: "Attach a local image (vision models) or PDF (text layer for any model) to the next message. /image clear removes pending attachments.", + description: "Attach a local image (vision models) or PDF text layer (requires Poppler's pdftotext); vision models can use PDF page images via pdftoppm. /image clear removes pending attachments.", kind: commandImage, }, { diff --git a/internal/tui/image_attach.go b/internal/tui/image_attach.go index afc02c05b..d9dad3abd 100644 --- a/internal/tui/image_attach.go +++ b/internal/tui/image_attach.go @@ -163,8 +163,9 @@ func (m model) attachClipboardImage(data []byte, mediaType string) model { } // handleImageCommand processes "/image " and "/image clear". A bare -// "/image" prints usage. PDFs are routed to the document path (text layer always -// attaches; pages rasterize to images only for vision models with a rasterizer). +// "/image" prints usage. PDFs are routed to the document path (their text layer +// attaches when pdftotext can extract it; pages rasterize to images only for +// vision models with a rasterizer). // Image files attach only to vision models. Attachment failures (missing file, // unsupported type, oversize) surface as an inline notice and attach nothing. func (m model) handleImageCommand(arg string) model { @@ -217,12 +218,10 @@ type pendingDocument struct { text string } -// handleDocumentAttach loads a PDF through imageinput.LoadDocument. The text +// handleDocumentAttach loads a PDF through imageinput.LoadDocument. Its text // layer is staged for every model; when the active model supports vision and a -// rasterizer is available, the rendered pages are staged through the existing -// pending-image pipeline too. A scanned PDF with no text (and no rasterizer) -// surfaces LoadDocument's explicit "no extractable text" notice and attaches -// nothing. +// rasterizer is available, rendered pages are staged through the existing +// pending-image pipeline too. A load error prevents either result from staging. func (m model) handleDocumentAttach(path string) model { doc, err := imageinput.LoadDocument(path, m.cwd, imageinput.DocumentOptions{ Vision: m.modelSupportsVisionTUI(), diff --git a/internal/tui/image_attach_test.go b/internal/tui/image_attach_test.go index 84ba3e69f..9ad289d64 100644 --- a/internal/tui/image_attach_test.go +++ b/internal/tui/image_attach_test.go @@ -4,7 +4,9 @@ import ( "bytes" "context" "fmt" + "io" "os" + "os/exec" "path/filepath" "strconv" "strings" @@ -317,12 +319,36 @@ func writeTestPDF(t *testing.T, dir, name, text string) string { return path } +func requirePopplerText(t *testing.T, path string) { + t.Helper() + executable, err := exec.LookPath("pdftotext") + if err != nil { + t.Skip("pdftotext is not installed") + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read PDF fixture: %v", err) + } + cmd := exec.Command(executable, "-layout", "-enc", "UTF-8", "-", "-") + cmd.Stdin = bytes.NewReader(data) + cmd.Stdout = io.Discard + cmd.Stderr = io.Discard + if err := cmd.Run(); err != nil { + // This is an optional host integration. Some supported Poppler builds + // reject this deliberately minimal test fixture even though the loader's + // command shape is correct; imageinput's helper-process tests cover that + // production path without depending on a host parser build. + t.Skipf("pdftotext cannot process this fixture on this host: %v", err) + } +} + // A PDF carries a text layer every model can read, so /image stages a // pending document even on a non-vision model -- unlike a raw image, which is // refused. No page images are staged without a rasterizer. func TestImageCommandAttachesPDFTextOnNonVisionModel(t *testing.T) { root := t.TempDir() - writeTestPDF(t, root, "spec.pdf", "Design spec body text") + path := writeTestPDF(t, root, "spec.pdf", "Design spec body text") + requirePopplerText(t, path) m := newModel(context.Background(), Options{Cwd: root, ModelName: "totally-unknown-custom"}) m.input.SetValue("/image spec.pdf") @@ -350,7 +376,8 @@ func TestImageCommandAttachesPDFTextOnNonVisionModel(t *testing.T) { // a non-vision model instead of being refused as a non-image. func TestImageCommandAttachesExtensionlessPDFByContent(t *testing.T) { root := t.TempDir() - writeTestPDF(t, root, "spec", "Extensionless PDF body text") + path := writeTestPDF(t, root, "spec", "Extensionless PDF body text") + requirePopplerText(t, path) m := newModel(context.Background(), Options{Cwd: root, ModelName: "totally-unknown-custom"}) m.input.SetValue("/image spec") @@ -388,10 +415,45 @@ func TestImageCommandRejectsFakePDF(t *testing.T) { } } +func TestImageCommandRejectsMalformedPDF(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "broken.pdf"), []byte("%PDF-1.4\nbroken"), 0o644); err != nil { + t.Fatalf("write PDF: %v", err) + } + m := newModel(context.Background(), Options{Cwd: root, ModelName: "gpt-4.1"}) + m.input.SetValue("/image broken.pdf") + updated, _ := m.handleSubmit() + next := updated.(model) + if len(next.pendingDocuments) != 0 || len(next.pendingImages) != 0 { + t.Fatal("malformed PDF must not stage attachments") + } + if notice := lastTranscriptText(next); !strings.Contains(notice, "PDF") { + t.Fatalf("expected PDF extraction notice, got %q", notice) + } +} + +func TestImageCommandExplainsWhenBoundedPDFExtractorIsUnavailable(t *testing.T) { + root := t.TempDir() + writeTestPDF(t, root, "spec.pdf", "text") + t.Setenv("PATH", "") + + m := newModel(context.Background(), Options{Cwd: root, ModelName: "gpt-4.1"}) + m.input.SetValue("/image spec.pdf") + updated, _ := m.handleSubmit() + next := updated.(model) + if len(next.pendingDocuments) != 0 || len(next.pendingImages) != 0 { + t.Fatal("an unavailable bounded extractor must not stage a document") + } + if notice := lastTranscriptText(next); !strings.Contains(notice, "pdftotext") { + t.Fatalf("expected installation guidance, got %q", notice) + } +} + // /image clear removes staged documents as well as images. func TestImageCommandClearAlsoClearsDocuments(t *testing.T) { root := t.TempDir() - writeTestPDF(t, root, "spec.pdf", "some text") + path := writeTestPDF(t, root, "spec.pdf", "some text") + requirePopplerText(t, path) m := newModel(context.Background(), Options{Cwd: root, ModelName: "gpt-4.1"}) m.input.SetValue("/image spec.pdf") @@ -426,7 +488,8 @@ func TestTranscriptViewShowsDocumentChips(t *testing.T) { // receives (so the model can read it), and the pending documents are cleared. func TestSubmitPrependsDocumentTextThenClears(t *testing.T) { root := t.TempDir() - writeTestPDF(t, root, "spec.pdf", "Top secret design notes") + path := writeTestPDF(t, root, "spec.pdf", "Top secret design notes") + requirePopplerText(t, path) provider := &fakeProvider{events: []zeroruntime.StreamEvent{ {Type: zeroruntime.StreamEventText, Content: "ok"},