feat(runtime): preflight native PDF inputs against provider document constraints

[jhaabhijeet864]

Summary

Part of #3284. Introduces a deterministic, bounded, zero-dependency PDF preflight validator in @maka/runtime to inspect native PDF inputs against provider document constraints before dispatch.

Problem

When users provide PDF documents to PDF-capable providers (such as Anthropic Claude 3.5 Sonnet), submitting invalid, encrypted, or oversized documents leads to late provider-side 400 errors and burned turn latency. Providers enforce strict constraints:

1. Unencrypted / unpassworded standard PDFs only.
2. Request-level page bounds (e.g., 100 pages maximum for Anthropic document inputs).
3. Legitimate PDF binary headers, regardless of declared file extension or MIME type.

Solution

• Implemented validatePdfBytes(bytes, limits) in packages/runtime/src/pdf-preflight.ts:

• Magic Byte Verification: Verifies presence of %PDF- signature within PDF_HEADER_SCAN_BYTES (1024 bytes) aligned with @maka/core/attachments.

• Encryption Detection: Scans for /Encrypt dictionaries as a fast-path heuristic without executing or evaluating PDF objects.

• Bounded Page Count Extraction: Best-effort scan of /Type /Pages and /Count entries using a bounded window (capped at 5MB or head/tail 1MB slice for large files) to prevent memory ballooning and ReDoS.

• Strict Bounds & Zero Dependency: Implemented as pure TypeScript buffer/string operations with no external parsing engines, native binaries, or text extraction/rendering overhead.

• Exported ./pdf-preflight from packages/runtime/package.json as a staged foundational boundary for provider dispatch and CLI prompt content integration.

• Added comprehensive unit tests in packages/runtime/src/__tests__/pdf-preflight.test.ts.

Part of #3284

Verification

Automated Checks Ran Locally

1. Unit Tests (node:test):

▶ pdf-preflight
  ✔ rejects files without %PDF- magic bytes (1.1ms)
  ✔ accepts files with %PDF- magic bytes (0.3ms)
  ✔ rejects encrypted PDFs (0.1ms)
  ✔ extracts page count and enforces limits (0.2ms)
✔ pdf-preflight (2.5ms)
4 pass, 0 fail

2. ASF License Headers Check:

> maka@0.2.0 check:asf-headers
Audited 4072 covered and 251 excluded file(s) in checkout mode at E:\Open Source\apache_maka
Every source file carries the ASF header or a reviewed exclusion.

3. Biome Formatting & Lint:

> @biomejs/biome check packages/runtime/src/pdf-preflight.ts packages/runtime/src/__tests__/pdf-preflight.test.ts packages/runtime/package.json
Checked 3 files in 14ms. No fixes applied. (0 errors, 0 warnings)

Review focus

• Staged Helper vs Call Site: This PR currently establishes the core bounded inspector and test matrix. As noted in review, full runtime invocation can either land in the CLI prompt attachment ingestion path (packages/cli/src/acp/prompt-content.ts) or alongside provider dispatch materialization (feat(runtime): materialize native PDF inputs for verified PDF-capable providers #3164 / feat(runtime): materialize verified native PDF inputs #3266).
• Heuristic Boundaries: /Encrypt scan and /Count matching are fast-path heuristics designed to fail early on obvious incompatibilities without paying the CPU and memory cost of full PDF object graph parsing.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope:

• Pair-programmed bounded byte inspection logic and test harness with Antigravity / Gemini.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No (Staged foundational helper)

[Astro-Han]

Reviewed 1dcb20812acfcb9dc358a33c33e4f424400ce927 (3 files, +141/−0). It adds validatePdfBytes (new module, 88 lines), a four-case unit test, and a ./pdf-preflight subpath export from @maka/runtime.

One P1 and two P3 notes below. The P1 is about reachability, not about the validator's internals — I read those and they are bounded and allocation-light as claimed.

P1 — nothing calls this, so no PDF is preflighted. The only reference to pdf-preflight in the whole tree outside the module and its own test is the export line in packages/runtime/package.json:19:

$ grep -rn "pdf-preflight" --include="*" . | grep -v node_modules | grep -v src/pdf-preflight.ts | grep -v pdf-preflight.test.ts
./packages/runtime/package.json:19:    "./pdf-preflight": "./dist/pdf-preflight.js",

The description says the export exists "for deterministic consumption by provider dispatch and message projection layers" and ends with Fixes #3284, but neither layer imports it, and the existing PDF seams do not call it either — for example packages/cli/src/acp/prompt-content.ts:222 already scans PDF_HEADER_SCAN_BYTES for %PDF- on its own path. As it stands the change cannot alter any user-visible behaviour: an encrypted, oversized or non-PDF document still reaches the provider exactly as before, which is the late 400 the issue describes. Merging this with Fixes #3284 would close that issue without the fix existing. Either wire it where the description says (or at the CLI attachment path that already sniffs PDFs) and add a test proving a real submission path rejects such a document, or keep it as a staged helper and say so — "Part of #3284", with the missing call site named.

P3 — the magic-byte rule is now in two places. pdf-preflight.ts:29-33 hardcodes a 1024-byte window and re-implements the ASCII scan. @maka/core/attachments already owns that rule: PDF_HEADER_SCAN_BYTES = 1024 (packages/core/src/attachments.ts:118), matched with containsAscii(bytes.subarray(0, PDF_HEADER_SCAN_BYTES), '%PDF-') (:143), and the CLI consumes it (packages/cli/src/acp/prompt-content.ts:222). Two copies of "where a PDF header may appear" will drift; import the constant and reuse the matcher.

P3 — what a caller should know once this is wired (from reading the checks). /Encrypt is matched anywhere in the file (:37-40), so an unencrypted document that merely contains that ASCII literal — a paper about PDF encryption, say — is reported as encrypted. And the page-count regex can capture a child page-tree node's /Count, so maxPages can pass a document that is over the limit. Both are consistent with the module's "best effort" wording, but pages is then a hint rather than a gate, and the doc comment's "strict" oversells the encryption check.

Two notes for whoever decides the merge (not code defects)

• The commit and the description carry no Generated-by: trailer and no AI-use statement. CONTRIBUTING asks for that declaration, and a reader currently cannot tell whether generative tooling contributed.
• CI on this revision is action_required — no check has ever run on it, so this head has no gate evidence; the test output in the description is the author's local run.

What I checked

• The module's internals line by line: the bounded %PDF- window, the /Encrypt scan, the whole-buffer versus head/tail 1 MB scan split at 5 MB, both page-count regex variants, parseInt plus NaN handling, and the zero-copy Buffer.from(bytes.buffer, byteOffset, byteLength) view. It is bounded, dependency-free, and returns not_a_pdf rather than throwing on arbitrary bytes.
• Reachability: every reference to the new export, across TypeScript, JSON and scripts.

What I could not judge

• I did not run the suite, and this revision has no CI result to lean on.
• I did not build real-world PDFs (an encrypted one with object streams, a page tree inside an object stream, a PDF 2.0 file), so how the heuristics behave on those is reasoned, not measured.
• Whether the intended consumer layers can call this synchronously on the submission path is something I could not settle without the call site.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[jhaabhijeet864]

@Astro-Han Thanks for the thorough review. I've updated the PR description to clarify this is a staged helper ("Part of #3284") and filled out the AI-use declaration. I also pushed a new commit to deduplicate the magic byte rule by importing PDF_HEADER_SCAN_BYTES from @maka/core/attachments and added the Generated-by trailer.

[Astro-Han]

Re-reviewed at d43dca7bd4a7bf24ac7df969c1469f96f2d6d87f. This supersedes my earlier comment on 1dcb2081; the author reworked the module and its tests, and nothing from that comment should be read against this revision except where I say it still applies.

Outcome of my previous findings

• P1 — still stands, and it is the reason this re-review matters. The revision still contains only three files (the module, its test, and the ./pdf-preflight export in packages/runtime/package.json), and the module is still imported by nothing except its own test, so no PDF is preflighted anywhere and the late provider 400 the issue describes still happens. The description still ends with Fixes #3284. Either wire it (the description names provider dispatch and message projection; the CLI path that already sniffs PDFs would be an equally natural first consumer) or state that it is staged — Part of #3284 — and name the missing call site.
• P3 (duplicated header rule) — fixed. The 1024-byte window is no longer hardcoded: the module imports PDF_HEADER_SCAN_BYTES from @maka/core/attachments and derives its scan window from it (pdf-preflight.ts:20, :57), so there is one authority again.
• P3 (encryption false positive) — fixed. /Encrypt is now matched as an active dictionary definition, /\/Encrypt\s*(?:<<|\d+\s+\d+\s+R)/ (:81), instead of a bare substring, so a document that merely discusses PDF encryption is no longer reported as encrypted.
• P3 (page count as a gate) — addressed in wording, with one residual accuracy nit below.

P3 — the page-count description says "upper-bound", which the regex cannot promise. The scan takes the first /Type /Pages … /Count N in file order, and on a nested page tree that can be a child node's count — smaller than the total, so it is not an upper bound. The sentence is otherwise well-aimed (calling pages a hint rather than a gate is the right mitigation); saying "may be any page-tree node's count" would be accurate and would point a future caller at the same caution.

Notes for the record

• The AI-use declaration is now complete: the box for a substantive generative contribution is ticked and the scope names Antigravity/Gemini pair-programming. My earlier observation that the declaration was missing no longer applies.
• Checks on this revision, as of writing: audit and Dependency audit both success; test was still running when I wrote this, so I am not claiming a green test for this head. I will confirm it separately rather than leave it implied.
• Structural observation unchanged: the validator itself is still bounded, dependency-free, and never throws on arbitrary bytes.

What I could not judge

• I did not run the suite locally, and (see above) the test check had not finished when this was written.
• I did not build real-world PDFs — an encrypted one using object streams, a nested page tree, or a PDF 2.0 file — so the new heuristics are verified by reading, not by measurement.
• Whether the intended consumer layers can call this synchronously on the submission path is still unknown without the call site.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[me2seeks]

Automated review notice: This comment was posted by an automated review agent operated by me2seeks make. It is not an independent human review and does not replace one.

Summary

Adds a validatePdfBytes PDF preflight validator plus tests, and publishes it as a new @maka/runtime/pdf-preflight subpath export. CI is green. The heuristic core is sane (reuses the canonical header-scan window, bounded scans, deliberately no full PDF parse). The problem is scope: this ships a new public module contract with zero production callers.

Findings

1. Dead contract: no caller, yet a public export is added

packages/runtime/package.json gains "./pdf-preflight": "./dist/pdf-preflight.js", but the only references to validatePdfBytes in the tree are its own test file. Nothing in the attachment intake or provider dispatch path calls it, so the behavior #3284 asks for is unchanged. Per Occam's razor, don't publish a new export surface before a consumer exists — either wire this into the actual PDF attachment dispatch in this PR, or keep the module unexported until the integration lands. As-is the repo gains a public contract plus ~180 LOC and tests that guard unused code.

2. pages is enforced as a hard gate but is not reliably an upper bound

The doc comment tells callers to treat pages as an "upper-bound estimate", but two regex paths undermine that: the first /Type /Pages … /Count N match can be a page-tree subtree node whose count is strictly less than the total, and the alternate-layout fallback (/Count N … /Type /Pages) can pick up an unrelated /Count (e.g. an outlines dictionary) sitting nearby. Since page_limit_exceeded hard-rejects the user's document, a wrong count becomes a wrong user-facing rejection. Either only enforce when the count is known to come from the root /Pages node, or return pages as advisory metadata and let the caller decide.

3. Minor: seam artifacts in the >5 MB head+tail scan

For large files the scan string is head(1 MB) + tail(1 MB) concatenated, so a /Type /Pages at the end of the head and a /Count at the start of the tail can synthesize a match across the seam. Low probability, and free to avoid by scanning head and tail separately.

4. Minor: Buffer used without import

The test imports node:buffer explicitly; the implementation relies on the global. Prefer the explicit import for consistency.

Verdict

Sound heuristic, wrong scope. Please land this together with (or after) the integration that actually calls it, drop the premature package export, and make the page limit advisory unless the count source is trustworthy.

[hqhq1025]

Reviewed the current head 7f9f4a3b (five changed files). The prior no-caller finding is resolved for ACP external-file uploads: publishAcpPromptAttachments now invokes the preflight before Artifact ingest. I found two new issues that prevent this revision from being ready. The first breaks CLI typechecking and produces a malformed ACP error; the second rejects a valid one-page PDF. The runtime and ACP prompt-content tests pass (17/17), but tsc -p packages/cli/tsconfig.json --noEmit reports TS2345 at prompt-content.ts:131. The current head has no hosted check runs or commit statuses. Against current main (c838e1fa), the static merge tree and diff whitespace check are clean. I did not exercise a live ACP client/provider, packaged CLI, or native Windows/macOS.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed head b4bcfb85 (merges cleanly with origin/main). This revision adds validatePdfBytes (@maka/runtime/pdf-preflight), which checks for the %PDF- header and encryption and reports a page count as a hint only. It is called from publishAcpPromptAttachments before Artifact ingest.

Findings from the previous review (5372105219 @ 7f9f4a3b)

• P1 (RequestError arguments reversed): fixed. The call now goes through invalidPrompt(), which uses RequestError.invalidParams and returns -32602. The maka-agent tsc build passes.
• P2 (heuristic page count used as a hard gate): fixed. pages is advisory only and no caller reads it. A regression test for the /Count 101 comment case was added.
• The requested ACP test that checks the error code on an upload rejection was not added. acp-prompt-content.test.ts has no PDF case.

New findings

• P2: On the ACP upload path, every PDF with an /Encrypt entry is now rejected, including PDFs that have only an owner password and an empty user password, which are common. On main, no provider receives PDFs natively: appendImageParts sends only images, and PDFs travel as attachment refs that host tools can process. So this rejection is not tied to any provider constraint, and the PR implements no provider limits despite its title. The user-facing error is the generic "Invalid ACP prompt content".
• P3: The encryption regex also matches inside uncompressed content streams. A valid, unencrypted one-page PDF whose text contains (… /Encrypt 5 0 R …) Tj is rejected as encrypted, which contradicts the docstring.
• P3: package-lock.json drops all 42 "libc" (glibc/musl) constraints. This is unrelated churn, likely from regenerating the lockfile with a different npm version. Without the constraints, npm installs both the glibc and musl optional binaries on Linux. Please restore the lockfile from main. This is a body-only note because those hunks contain only deletions.
• P3 (body-only): The not_a_pdf branch can't be reached from the caller, because mimeType === 'application/pdf' only when the same %PDF- sniff succeeds. The desktop ingest path (apps/desktop/src/main/attachment-ingest.ts) is not preflighted, so the surfaces behave differently.

Checks run: runtime pdf-preflight tests 7/7, CLI acp-prompt-content tests 10/10, and the full workspace build through maka-agent. I also ran a Node probe against the built validator for the false-positive case.

Not exercised: real encrypted PDFs (no qpdf/pdfinfo in this environment), the desktop path, and live provider dispatch.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

[Astro-Han]

Re-review at 0fbf03c, covering the increment since b4bcfb8. That increment is one commit: the ACP upload path now skips encrypted preflight results, pdf-preflight.ts gets comment-only changes (the logic is the same), and there are two new ACP tests. I built core/runtime/cli and ran the targeted tests: runtime pdf-preflight passes 7/7, and cli acp-prompt-content passes 11/12. It merges cleanly with origin/main.

Earlier findings:

• P2, encrypted PDFs rejected on ACP upload: fixed.
• P3, the /Encrypt regex matching content-stream text: not fixed. The code is unchanged. Only the comments changed, and they now say it scans only the trailer/xref area, which it does not. A re-probe still returns encrypted for an unencrypted PDF.
• P3, package-lock dropping 42 libc constraints: still present.
• P3, not_a_pdf unreachable from the ACP caller: confirmed by the new test, which fails (see below).

New:

• P1: the new test rejects a non-PDF file with application/pdf mimeType fails at this head, so the test job will be red.
• P2: with encrypted skipped, the ACP preflight cannot reject anything that reaches it, and it is the only caller. The PR therefore has no runtime effect.

CI note: the audit failure comes from axios/brace-expansion/fast-uri advisories in versions that match main. The lockfile diff only touches libc entries, so this PR did not cause it.

Not exercised: real encrypted PDFs made with qpdf, the desktop attachment path, and provider dispatch.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed fcdba11655156c8fb223959a319210e2548f6310 (5 files, +207/−0).

All three findings from my earlier pass are resolved, and no new P0–P3.

The P1 is gone, via the route I offered. The description now reads Part of #3284 rather than Fixes #3284, and the module is published as a package subpath ("./pdf-preflight" in packages/runtime/package.json), so it is a declared building block for the remaining work rather than an internal module with no caller behind a "fixes" claim. That is exactly the second exit I described, and it is the honest one for a foundation piece.

Both P3s are fixed.

• The duplicated header-scan rule is gone: pdf-preflight.ts now imports PDF_HEADER_SCAN_BYTES from @maka/core/attachments and uses that single source for its window.
• The page-count documentation problem is fixed better than I suggested. Rather than merely re-wording, the count is now described as an advisory text-level scan that "cannot authoritatively distinguish a root page-tree /Count from an intermediate node or a content-stream comment", and the validator does not reject on page count at all. That removes the possibility of the mislabel misleading a reader, which was the substance of the finding, and it is the stronger resolution of the two.

The head also adds a small CLI-side integration (packages/cli/src/acp/prompt-content.ts with its test), so the validator now has a real consumer path in addition to its published export.

Gate on this head: there are no check runs on fcdba116 — the check-runs list is empty — so I cannot report a CI result for this revision. The PR is mergeable with state blocked.

What I could not judge

• No CI result on this exact head, as above; I verified the three resolutions by reading the code and the description.
• No live provider dispatch, so whether a preflight rejection ever fires end-to-end is not exercised here.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed 2dfaadced6ba1e19e162112898b7260d3ea15bea (5 files, +208/−1, 8 commits). This head has no check runs, so nothing below rests on CI — it rests on the code, which I state per item.

The two findings this card asked about are resolved, both by removal rather than by softening.

• The P1 (a test failing at 0fbf03cb) is gone. The failing assertion was "Must reject before upload", and it no longer exists in acp-prompt-content.test.ts; the remaining rejects cases in that file are about other things (a FIFO, unadvertised content kinds, non-local resources).
• The P2 (the ACP gate could not reject anything) is resolved by deleting the gate from that path. There is no longer any reference to pdf-preflight, validatePdfBytes or pdf_encrypted anywhere under packages/cli/src. So the decorative half is not merely made advisory, it is gone: the preflight now exists only as the published @maka/runtime/pdf-preflight module, which is the "foundation for the remaining work" framing I checked last round. Worth noting explicitly that this removes the CLI consumer path I mentioned there, so the module is again export-only — with Part of #3284 in the description, that is consistent rather than a regression.

The contested P3 still stands, and the code is what decides it. My line first raised that the /Encrypt scan reaches stream content; the author replied that detection was "refined … the validator now prioritizes scanning the trailer and xref-stream areas". At this head the buffer selection is unchanged for the small files that matter: scanBuffers still takes the whole buffer when the file is 5 MB or less, and the regex is byte-for-byte the same (/\/Encrypt\s*(?:<<|\d+\s+\d+\s+R)/), applied with .some(...) across every buffer. What did change is the comment above it, which now asserts it searches "only within the trailer or xref-stream contexts" — a narrowing the code does not implement. So the reply overstates the fix, and the docstring now claims a guarantee the implementation lacks; that mismatch is the finding.

Gate on this head: no check runs and no commit statuses are reported, so I make no CI claim. mergeable is true; every commit carries a Generated-by: trailer with no Grok involvement.

What I could not judge

• I did not build and run the CLI test suite, so the P1's resolution is verified by the removed assertion rather than by a green run.
• No live PDF probe against the built module; the P3's evidence is the buffer selection and the regex as written.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed f63f07f1c718792183649366b7a69a0aa007a6e1 (2 files, +4/−11).

The P3 I left open is resolved — by making the claim truthful rather than by narrowing the scan, which is the safer of the two possible fixes.

My finding was that the comment claimed a trailer/xref-only search while the code still scanned the whole buffer for files up to 5 MB, with the /Encrypt regex unchanged. At this head the comment says the opposite of the old claim and matches the code:

"Note: This is a best-effort text-level scan. Because it searches the entire extracted text buffer(s) rather than parsing the PDF object graph, it may falsely match uncompressed stream content containing the exact /Encrypt sequence."

That resolves the mismatch I raised, and it does so without introducing a miss risk — narrowing the implementation to the trailer/xref regions would have risked failing to detect genuinely encrypted files, so correcting the documentation is the better resolution. The buffer-selection comment above it is now accurate too (whole buffer at ≤5 MB; first and last 1 MB beyond that), and so is the page-count note (advisory, never a rejection gate, and the captured value may belong to any page-tree node rather than the root).

One fix came with it that I had not asked for. The head and tail buffers are now scanned separately, with the stated reason "to avoid synthetic matches across a concatenated seam". That removes a real false-match path I had not flagged — a pattern straddling the join of two concatenated slices.

Gate on this head: CI is green on the exact head — test, windows_acp and audit all completed successfully, and the merge state is blocked (review pending) with mergeable true. Commits carry Generated-by: trailers with no Grok involvement.

What I could not judge

• I did not rebuild or run the suites locally; the verdict rests on reading the changed comments against the unchanged code, plus the three green runs on this head.
• No live probe against the built module, so the remaining false-positive path for uncompressed stream content is acknowledged rather than measured.

I did not approve, request changes, or merge.

---

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.