Skip to content

(MOT-4310) feat(pdf,console): read PDFs locally and route the pages that need OCR - #682

Merged
rohitg00 merged 6 commits into
mainfrom
feat/pdf-worker
Aug 4, 2026
Merged

(MOT-4310) feat(pdf,console): read PDFs locally and route the pages that need OCR#682
rohitg00 merged 6 commits into
mainfrom
feat/pdf-worker

Conversation

@rohitg00

@rohitg00 rohitg00 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Agents cannot read PDFs. A PDF handed to a conversation either never reaches the model or arrives as binary noise, and nothing says whether a document holds real text or is a photograph of a page. That second question is the expensive one: sending a text PDF to an OCR service costs seconds and money for a result the machine could produce in milliseconds.

This adds a pdf worker that parses documents locally, and makes the console actually hand it the file a person attaches.

What it does

pdf::classify samples a document in about twenty milliseconds and answers whether the pages hold real characters, plus which individual pages cannot be read without a vision model and why. That verdict decides whether anything else is worth doing, and it is what separates "this document is empty" from "this document is a scan".

pdf::to-markdown converts a text based document, keeping headings, lists, links and tables. pdf::extract-text returns plain text for search and embedding. pdf::extract-items returns every positioned run of characters with its box, font and styling. pdf::extract-regions returns the real text inside given boxes on given pages, for the case where a vision model has located a region and the exact characters should come from the document rather than from a transcription.

Nothing leaves the machine and no key is required. The parser is the MIT licensed pdf-inspector crate: pure Rust, no C libraries, no subprocess, no network.

Console

Attaching a PDF in the composer used to send nothing. The paperclip builds a preview only for small text and image files, and the only thing the send path forwards is text blocks, so the document never left the browser. The agent then answered as though it had been given nothing.

At send time each attached PDF now goes to the worker and comes back as an <attached-file …> block, the same envelope #file(<path>) mentions already use, so the transcript, the chip renderer and the model all see a shape they understand. No harness change and no new wire shape.

Classification runs first, so a scan produces a block saying the document was read and found unreadable rather than an empty one. A long document is capped, and the block says how much it withheld and how to get the rest. A missing worker is reported as the one thing a person can act on. Nothing can block a send: every failure becomes a placeholder block plus a notice.

The worker also ships an injected console page (drop a document, see the verdict, the per page OCR decision, the timings and the markdown) and a renderer so pdf::* calls read as decisions in chat rather than as raw JSON.

Things that were easy to get wrong

Page numbering. The parser is not internally consistent: one classification entry point counts pages from one and another counts from zero, its per page extraction takes zero indexed input, and its whole document page filter takes one indexed. Every number crossing this worker's wire is 1-indexed, the conversions live in one place, and tests cover both directions.

Coordinates. pdf::extract-items reports PDF points from the bottom left, the PDF convention. pdf::extract-regions takes boxes from the top left, which is what a layout model produces. They disagree deliberately and each response states which it used, because assuming the wrong one returns text from the wrong end of the page with no error.

CJK CMaps. The parser resolves its CJK CMap payload against the manifest directory recorded when the parser crate itself was compiled. As a cross compiled dependency that is a path inside the build machine's cargo registry, which does not exist on the machine running a released binary. The lookup then finds nothing and CID fonts with no ToUnicode table decode to empty text, with no crash and no log. The payload is staged into the build output, embedded, and materialized at boot.

Response size. Every text bearing response is capped by default and reports what it withheld, so a fragment cannot be mistaken for a document. max_chars: 0 lifts the cap for worker to worker moves. The page filter is the cheaper lever: a four hundred page report classifies in 118 ms but takes 39.7 s to convert whole.

The guidance hook binds fail_open. Pre generate hooks default to fail closed, so a hook that errored would abort generation. A missing paragraph of advice must never kill a turn.

The chat renderer. A function result arrives wrapped by the harness as { content, details }, not as the response itself. Reading the raw value looked like it worked right up until every field was undefined and the card fell through to empty chrome.

Scope note

The parser's structured cell table API (rows, columns, spans, header flags) is not usable standalone: every entry point for it requires structure tokens and cell boxes produced by an external table structure recognition model running on a rendered page crop, which nothing in this stack produces. Tables reach callers the two ways that do work, rendered as markdown tables in pdf::to-markdown output and per page through pages_with_tables.

Rasterizing pages is deliberately out of scope. Scanned documents are classified and routed, not read; adding a rasterizer means pdfium or mupdf, which means system libraries and a broken clean cross compile.

Verification

Worker: cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, 105 tests. Ten of those drive the whole surface over a real booted engine and assert at the wire, where serde silently drops a field a unit test would never notice.

Console: typecheck, biome, 1102 tests across 87 files, production build.

Live against a running rig: both console assets serve with empty warnings and hashes move on re-registration; an eight page document classifies in 15 ms and converts in 72 ms; an encrypted document opens with a password; a four hundred page document behaves as described above. An agent given a document path calls pdf::classify and then pdf::to-markdown on its own.

Two things not verified and worth naming. The CJK failure mode cannot be reproduced on a development machine, because the cargo registry path the parser was compiled against exists there; it is reasoned from the parser's source and guarded by tests that the payload is embedded and materialized. And the console attachment path is tested and built but has not been exercised against a running console, which needs this branch's console worker deployed rather than a hot reload.

Fixes MOT-4310
Fixes MOT-4328
Refs MOT-4311, MOT-4312, MOT-4313, MOT-4314, MOT-4315, MOT-4316, MOT-4317, MOT-4318, MOT-4319, MOT-4320, MOT-4321

Summary by CodeRabbit

  • New Features

    • Added PDF inspection with classification, OCR detection, Markdown conversion, text extraction, positioned items, and region/table extraction.
    • Added a console page for uploading PDFs and reviewing results, warnings, metadata, and page previews.
    • PDF attachments in chat are automatically expanded into readable content when sending or editing messages.
    • Added support for scanned, mixed, encrypted, truncated, and oversized documents with clear status reporting.
  • Documentation

    • Added PDF worker usage, configuration, capabilities, and limitations documentation.

Agents cannot read PDFs. A PDF handed to a conversation either never
reaches the model or arrives as binary noise, and nothing says whether a
given document holds real text or is a photograph of a page. That second
question is the expensive one: sending a text PDF to an OCR service costs
seconds and money for a result the machine could produce in milliseconds.

The pdf worker parses documents locally. It classifies in about twenty
milliseconds, converts text-based documents to markdown that keeps their
headings, lists, links and tables, and reports exactly which pages still
need OCR and why. Nothing leaves the machine and no key is required.

Surface:

- pdf::classify        routing verdict plus a per-page OCR reason
- pdf::to-markdown     structure-preserving conversion, page filter, caps
- pdf::extract-text    plain text, for search and embedding
- pdf::extract-items   positioned runs with font, size and styling
- pdf::extract-regions the real characters inside a box on a page

It also ships an injected console page (drop a PDF, see the verdict, the
per-page decision and the markdown) and a renderer so pdf::* calls read as
decisions in chat rather than raw JSON.

Notes on the parts that are easy to get wrong:

- Page numbers are 1-indexed everywhere on the wire. The parser is not
  internally consistent about this, so the conversions live in one place
  and are covered by tests in both directions.
- The two coordinate conventions disagree on purpose: extract-items reports
  bottom-left, extract-regions takes top-left. Each response states which
  it used, because assuming wrong returns text from the wrong end of the
  page with no error.
- The parser resolves its CJK CMap payload against its own compile-time
  manifest directory, which for a cross-compiled dependency does not exist
  on the machine running the binary. The payload is staged into the build
  output, embedded, and materialized at boot; without this, CID fonts with
  no ToUnicode table decode to empty text silently.
- Responses are capped by default and report what they withheld, so a
  fragment cannot be mistaken for a document. max_chars 0 lifts the cap for
  worker-to-worker moves.
- The guidance hook binds fail_open, because pre-generate defaults to
  fail-closed and a missing paragraph of advice must never kill a turn.

Configuration is Path B with every field hot-reloading. 104 tests, ten of
them driving the surface over a real engine.
…editor

The speed is the reason to parse locally rather than pay an OCR service, and
one combined number hid it. The page now reports the two stages separately —
classifying, then extracting — plus the characters-a-second rate underneath, so
an eight page document reading in tens of milliseconds is visible rather than
implied. Both timing tiles carry a tooltip saying what that stage actually did.

The markdown source tab was a plain block of preformatted text. It now uses the
console's shared Monaco editor, read-only, which is the contract for every code
and long-text surface.

Also: stop retrying a missing configuration entry. A not-found is the normal
state on a clean install, not a transient failure, so retrying it spent the
whole backoff and logged two warnings on every first boot.

`pages_sampled` and its siblings are optional on the wire (absent for an
encrypted document), so the page renders "all" rather than "undefined of 8".
Attaching a PDF in the composer sent nothing. The paperclip builds a preview
only for small text and image files, and the only thing the send path forwards
is text blocks, so the document never left the browser. The agent then answered
as though it had been given nothing, which is what a person sees as the
assistant ignoring the file they just attached.

At send time each attached PDF now goes to the `pdf` worker on the machine and
comes back as an `<attached-file …>` block — the same envelope `#file(<path>)`
mentions already use, so the transcript, the chip renderer and the model all see
a shape they understand. No harness change and no new wire shape.

Classification runs first because it decides whether extraction is worth doing.
A scan produces a block saying the document was read and found unreadable, which
the model needs in order to tell that apart from being handed nothing. A long
document is capped, and the block says how much it withheld and how to get the
rest. A missing worker is reported as the one thing a person can act on rather
than as a bus error. Nothing here can block a send: every failure becomes a
placeholder block plus a notice.

Also fixes the worker's own chat renderer, which was rendering empty cards. A
function result arrives wrapped by the harness as `{ content, details }`, not as
the response itself, so reading the raw value found undefined everywhere and
fell through to blank chrome. The console has its own unwrap helper, but an
injected asset can only import from the shared package, so the same rule lives
in the renderer.

The page's drop zone now collapses to a bar once a document is loaded, with the
file name and a "read another" action: after the first read, the results are
what the page is for.
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
workers Ready Ready Preview Aug 3, 2026 4:55pm
workers-tech-spec Ready Ready Preview Aug 3, 2026 4:55pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@rohitg00, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9f55d4e7-269c-4cd9-8e2f-cc5d7162843f

📥 Commits

Reviewing files that changed from the base of the PR and between 3c53e0d and 44d265d.

📒 Files selected for processing (24)
  • console/web/src/components/chat/ChatView.tsx
  • console/web/src/lib/pdf-attachments.test.ts
  • console/web/src/lib/pdf-attachments.ts
  • iii-permissions.yaml
  • pdf/README.md
  • pdf/build.rs
  • pdf/examples/probe.rs
  • pdf/skills/SKILL.md
  • pdf/src/config.rs
  • pdf/src/functions/classify.rs
  • pdf/src/functions/markdown.rs
  • pdf/src/functions/text.rs
  • pdf/src/lib.rs
  • pdf/src/main.rs
  • pdf/src/source.rs
  • pdf/tests/fixtures.rs
  • pdf/tests/fixtures/README.md
  • pdf/tests/golden/schemas/pdf.classify.json
  • pdf/tests/golden/schemas/pdf.extract-items.json
  • pdf/tests/golden/schemas/pdf.extract-regions.json
  • pdf/tests/golden/schemas/pdf.extract-text.json
  • pdf/tests/golden/schemas/pdf.to-markdown.json
  • pdf/tests/integration.rs
  • pdf/tests/support/engine.rs
📝 Walkthrough

Walkthrough

Adds a new Rust PDF worker with classification, Markdown, text, item, and region extraction. Adds configuration, OCR diagnostics, a console inspection UI, chat PDF attachment expansion, permissions, tests, documentation, and release integration.

Changes

PDF worker foundation and processing

Layer / File(s) Summary
Worker foundation and runtime configuration
pdf/Cargo.toml, pdf/src/*, pdf/build.rs
Adds the worker package, startup flow, CMap caching, configuration parsing and reload, source validation, response truncation, and manifest generation.
PDF processing functions and schemas
pdf/src/functions/*, pdf/tests/golden/schemas/*
Adds five typed PDF functions for classification, Markdown conversion, text extraction, positioned items, and regions.
Validation and integration tests
pdf/tests/*
Adds fixture, schema, manifest, engine-backed, and wire-level tests for PDF processing and guidance behavior.

Console and chat integration

Layer / File(s) Summary
PDF console UI
pdf/ui/*, pdf/src/ui.rs, pdf/src/guidance.rs
Adds PDF upload and inspection views, Markdown and OCR results, function-trigger cards, scoped styles, embedded assets, and prompt guidance.
Chat PDF attachments
console/web/src/components/chat/*, console/web/src/lib/pdf-attachments.ts, console/web/src/types/chat.ts
Retains browser File objects, expands readable PDFs through pdf::classify and pdf::to-markdown, and preserves sends when conversion fails.
Attachment expansion tests
console/web/src/lib/pdf-attachments.test.ts
Tests PDF detection, ordering, OCR handling, truncation, failures, limits, restored attachments, and filename escaping.

Release and documentation

Layer / File(s) Summary
Worker metadata and release wiring
.github/workflows/*, README.md, pdf/README.md, pdf/skills/SKILL.md, pdf/iii.worker.yaml, iii-permissions.yaml
Adds PDF worker documentation, skill metadata, permissions, worker deployment metadata, and tag-based release workflow entries.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

  • iii-hq/workers#623: Adds another worker through the same workflow and root README integration points.
  • iii-hq/workers#579: Adds a worker-specific console UI with embedded assets and function-trigger rendering.
  • iii-hq/workers#673: Follows a similar new Rust worker pattern with configuration, permissions, schemas, tests, and UI integration.

Poem

A rabbit reads pages, both narrow and wide,
Finds OCR clues hiding inside.
Markdown hops from each PDF byte,
While console cards make results bright.
Failed files leave warnings in view—
Thump, thump, the worker is ready too!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: local PDF processing and OCR routing in the PDF worker and console.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pdf-worker

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

skill-check — worker

0 verified, 53 skipped (no docs/).

Layer Result
structure
vale
ai
render

Four for four. Nicely done.

@rohitg00
rohitg00 marked this pull request as ready for review August 3, 2026 16:04
…hat was read

Two things a person could not see, and one they should not have been paying for.

The guidance hook fired on every generation, appending two and a half kilobytes
of PDF advice to conversations that would never touch a document. It now reads
the turn's messages and stays silent unless a document is actually in play: a
file name, the MIME type, the console's own attachment block, or a conversation
already using these functions. A turn that starts talking about a document gets
the guidance on that turn. The bare word "pdf" is deliberately not a marker, or
this would inject on nearly every turn again.

The attachment chip read "report.pdf 32kb" whether the document had been parsed,
skipped or failed. The expansion runs at send time, before the model is called,
so it never appears as a function call in the transcript, which left no way to
tell the document had reached the agent at all. The chip now reads
"report.pdf · 8 pages · 5,932 chars · 87 ms", or
"scan.pdf · 3 pages · no readable text · 9 ms" when there was nothing to
extract. A truncated extract marks its count so the number is not read as the
whole document.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (13)
pdf/ui/build.mjs (1)

15-36: 🚀 Performance & Scalability | 🔵 Trivial

Consider minifying the production build.

The build options at Lines 15-29 do not set minify. These assets are embedded into the worker binary and served to the console on every page load, so an unminified bundle increases transfer size and parse time for no benefit in the non-watch path.

♻️ Proposed change
 if (process.argv.includes('--watch')) {
   const ctx = await esbuild.context(options)
   await ctx.watch()
 } else {
-  await esbuild.build(options)
+  await esbuild.build({ ...options, minify: true })
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/ui/build.mjs` around lines 15 - 36, Update the esbuild options object
used by the production build to enable minification, while keeping watch-mode
behavior functional and unchanged apart from using the shared options.
console/web/src/lib/pdf-attachments.ts (1)

90-98: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

No client-side size guard before encoding a PDF to base64.

fileToBase64 reads the whole file into a Uint8Array, then chunks it into a binary string and calls btoa. There is no check on attachment.file.size before this runs. A very large PDF (tens or hundreds of MB) can block the main thread for a noticeable time and produce a very large RPC payload to the local worker, independent of the worker's own response-size caps (those bound the extracted markdown, not the input encoding step).

Add an upper bound on PDF file size before calling fileToBase64, and surface oversized files through the existing failure-block path (similar to how oversized/scanned documents already get an explanatory placeholder).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@console/web/src/lib/pdf-attachments.ts` around lines 90 - 98, Update the PDF
attachment processing flow around fileToBase64 to validate attachment.file.size
against an explicit maximum before reading or encoding the file. Route oversized
PDFs through the existing failure-block path and provide an explanatory
placeholder consistent with oversized or scanned document handling; only call
fileToBase64 for files within the limit.
pdf/src/main.rs (1)

123-124: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle SIGTERM as well as Ctrl+C.

Process supervisors and container runtimes stop a worker with SIGTERM. This code awaits ctrl_c only, so a normal stop skips shutdown_async() and the worker deregisters only after the supervisor kill timeout.

♻️ Proposed shutdown handling
-    tokio::signal::ctrl_c().await?;
+    #[cfg(unix)]
+    {
+        let mut term = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())?;
+        tokio::select! {
+            _ = tokio::signal::ctrl_c() => {}
+            _ = term.recv() => {}
+        }
+    }
+    #[cfg(not(unix))]
+    tokio::signal::ctrl_c().await?;
     iii.shutdown_async().await;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/main.rs` around lines 123 - 124, Update the shutdown wait around
tokio::signal::ctrl_c() so it also completes when SIGTERM is received, then
continue through iii.shutdown_async() for either signal. Preserve the existing
error propagation and ensure both Ctrl+C and SIGTERM trigger the same graceful
shutdown path.
pdf/src/config.rs (1)

50-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Validate text_page_ratio_threshold on both parse paths.

The field accepts any f32. A live value above 1.0 makes every document classify as scanned. A negative value or NaN makes every document classify as text based. deny_unknown_fields catches a typo'd key, but nothing catches an out-of-range value, so the failure appears as wrong classification instead of a config error.

Add a validation step and call it from from_yaml and from_json, so the seed file and the live snapshot get the same check.

♻️ Proposed validation
     pub fn from_yaml(yaml: &str) -> Result<Self, String> {
         let expanded = expand_env(yaml);
-        serde_yaml::from_str(&expanded).map_err(|e| format!("yaml parse: {e}"))
+        let cfg: Self = serde_yaml::from_str(&expanded).map_err(|e| format!("yaml parse: {e}"))?;
+        cfg.validate()?;
+        Ok(cfg)
     }
     pub fn from_json(value: &Value) -> Result<Self, String> {
-        serde_json::from_value(value.clone()).map_err(|e| format!("json parse: {e}"))
+        let cfg: Self =
+            serde_json::from_value(value.clone()).map_err(|e| format!("json parse: {e}"))?;
+        cfg.validate()?;
+        Ok(cfg)
     }
+
+    /// Reject values that would silently invert classification.
+    fn validate(&self) -> Result<(), String> {
+        if !(0.0..=1.0).contains(&self.text_page_ratio_threshold) {
+            return Err(format!(
+                "text_page_ratio_threshold must be between 0.0 and 1.0, got {}",
+                self.text_page_ratio_threshold
+            ));
+        }
+        Ok(())
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/config.rs` around lines 50 - 53, Validate text_page_ratio_threshold
after deserializing configuration in both from_yaml and from_json, requiring a
finite value within the inclusive 0.0–1.0 range and returning a configuration
error otherwise. Centralize this check in a helper near the configuration
implementation, then invoke it on both parse paths before returning their
results.
pdf/src/cmaps.rs (1)

34-46: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Set the CMap directory before #[tokio::main] starts the runtime.

cmaps::materialize() currently runs inside the tokio async body, when worker threads can already be accessing the global process environment. std::env::set_var is unsafe in multi-threaded programs on non-Windows platforms, so move this mutation into a synchronous startup path before the runtime starts.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/cmaps.rs` around lines 34 - 46, Move the cmaps::materialize() call
out of the #[tokio::main] async body and invoke it from a synchronous startup
function before the Tokio runtime is created. Preserve the existing materialize
behavior and ensure CMAP_DIR_ENV is set before any runtime worker threads can
access the process environment.
pdf/tests/integration.rs (1)

10-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fail the PDF integration suite when the engine is missing.

with_stack currently returns early with a skip notice when boot() cannot locate iii, so pdf/tests/integration.rs can pass without executing assertions in local or CI environments that do not provide the engine. Keep soft-skipping for developer convenience, but add an opt-in strict mode for the CI job that runs these tests (for example, III_REQUIRE_ENGINE=1 makes missing-engine runs panic).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/tests/integration.rs` around lines 10 - 17, Update
support::engine::with_stack to preserve its default soft-skip behavior while
panicking when boot() cannot locate iii and the opt-in III_REQUIRE_ENGINE=1
environment variable is set. Ensure the PDF integration test
classify_answers_over_the_bus runs under this strict mode in the CI job so
missing-engine runs fail instead of passing without assertions.
pdf/tests/fixtures/README.md (1)

15-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a language to the fenced code block.

markdownlint reports MD040 here.

🧹 Proposed fix
-```
+```bash
 python3 tests/fixtures/make_fixtures.py
 ```
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/tests/fixtures/README.md` around lines 15 - 17, Update the fenced code
block containing the make_fixtures.py command by adding the bash language
identifier to its opening fence, while leaving the command unchanged.

Source: Linters/SAST tools

pdf/build.rs (2)

129-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Declare the environment variables that change build behavior.

SKIP_UI_BUILD and PNPM alter the output of this build script. Without rerun-if-env-changed, cargo does not re-run the script when either value changes, so a developer who unsets SKIP_UI_BUILD keeps the previously embedded assets.

♻️ Proposed addition
 fn build_ui() {
+    println!("cargo:rerun-if-env-changed=SKIP_UI_BUILD");
+    println!("cargo:rerun-if-env-changed=PNPM");
     // `dist/` itself is not listed: include_str! reads it directly, and
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/build.rs` around lines 129 - 141, Update build_ui to declare
rerun-if-env-changed directives for both SKIP_UI_BUILD and PNPM, so Cargo reruns
the script whenever either build-affecting environment variable changes.

49-51: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Compare more than the file count when deciding to skip staging.

A dependency upgrade can change CMap file contents while keeping the same file count. The staged copy in OUT_DIR then stays stale, and CID fonts decode to nothing — the failure this script exists to prevent. Compare size and mtime per file, or copy unconditionally when any source file is newer than its staged counterpart.

♻️ Proposed stricter freshness check
-    if dest.is_dir() && dir_file_count(&dest) == dir_file_count(&src) {
+    if dest.is_dir() && staged_matches(&src, &dest) {
         return;
     }
/// `true` when every source file is staged with the same length and is not
/// newer than the staged copy.
fn staged_matches(src: &Path, dest: &Path) -> bool {
    let Ok(entries) = std::fs::read_dir(src) else {
        return false;
    };
    let mut seen = 0usize;
    for entry in entries.flatten() {
        let path = entry.path();
        if !path.is_file() {
            continue;
        }
        seen += 1;
        let Some(name) = path.file_name() else {
            return false;
        };
        let (Ok(s), Ok(d)) = (path.metadata(), dest.join(name).metadata()) else {
            return false;
        };
        if s.len() != d.len() {
            return false;
        }
        match (s.modified(), d.modified()) {
            (Ok(sm), Ok(dm)) if sm <= dm => {}
            _ => return false,
        }
    }
    seen > 0 && seen == dir_file_count(dest)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/build.rs` around lines 49 - 51, Replace the file-count-only skip
condition in the staging logic with a freshness check such as staged_matches,
comparing each source file with its destination by filename, size, and
modification time. Return early only when every source file exists in the staged
directory, matches in size, is not newer, and the file counts agree; otherwise
perform the existing copy.
pdf/src/functions/markdown.rs (1)

163-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider reusing items::page_filter for this validation.

items::page_filter performs the same three checks (empty list, page 0, collect into a HashSet<u32>) with the same error strings. Sharing one helper keeps the two messages from drifting apart.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/functions/markdown.rs` around lines 163 - 178, Replace the local
`req.pages` validation and `HashSet` construction in the markdown conversion
flow with the existing `items::page_filter` helper. Preserve the current
`Option` behavior for omitted pages and propagate the helper’s identical
validation errors and collected page filter.
pdf/src/functions/mod.rs (1)

67-85: 🚀 Performance & Scalability | 🔵 Trivial

Consider bounding concurrent blocking extractions.

Each invocation moves an owned buffer onto the tokio blocking pool. The module doc states a 200-page document takes hundreds of milliseconds. Concurrent calls therefore occupy the blocking pool and hold one full document buffer each, so memory scales with in-flight requests. A semaphore around the spawn_blocking call, sized from configuration, gives back-pressure instead of pool saturation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/functions/mod.rs` around lines 67 - 85, Update the register_blocking!
macro to acquire a configuration-sized semaphore permit before calling
tokio::task::spawn_blocking, retaining the permit for the extraction’s full
duration and propagating acquisition errors appropriately. Reuse the existing
configuration or concurrency-limit symbol exposed by the surrounding code, and
preserve the current panic and handler error mapping.
pdf/tests/support/engine.rs (1)

179-186: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Replace the fixed registration delay with a poll.

The 500 ms sleep assumes registration completes in that window. On a loaded runner it can be too short, which produces a function-not-found failure that looks like a worker bug. It also adds 500 ms to every test that calls boot().

This file already polls for engine and configuration readiness. Apply the same approach here: retry one cheap call against a registered function until it resolves, with the existing deadline pattern.

♻️ Proposed poll instead of a fixed sleep
-    // Let the registrations land before the first call.
-    tokio::time::sleep(Duration::from_millis(500)).await;
-
-    Some(Stack {
+    let stack = Stack {
         iii,
         _engine: engine,
-    })
+    };
+
+    // Poll until a registered function resolves, rather than assuming a fixed
+    // window is enough on a loaded runner.
+    let deadline = Instant::now() + Duration::from_secs(10);
+    loop {
+        if stack
+            .call("pdf::classify", json!({ "bytes_base64": "" }))
+            .await
+            .is_ok_and(|_| true)
+        {
+            break;
+        }
+        if Instant::now() > deadline {
+            break;
+        }
+        tokio::time::sleep(Duration::from_millis(100)).await;
+    }
+
+    Some(stack)

Note: pdf::classify with empty bytes returns an error result, not a transport error. Match on the error text to tell "not registered" apart from "registered and rejected the input", or add a trivially valid payload.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/tests/support/engine.rs` around lines 179 - 186, Replace the fixed 500 ms
sleep in boot with the file’s existing deadline-based polling pattern,
repeatedly making a cheap call to a registered function such as pdf::classify.
Continue polling only while the result indicates the function is not registered;
treat the expected invalid-input error as readiness, and preserve the existing
deadline/timeout behavior before returning Stack.
pdf/tests/fixtures.rs (1)

34-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: add a request helper to cut the repetition.

Eight tests in this file build the same classify::Request literal with password: None and sample_pages: None. A helper keeps the tests focused on the assertion. It also localizes the edit when a field is added to classify::Request.

♻️ Proposed helper
 fn cfg() -> WorkerConfig {
     WorkerConfig::default()
 }
+
+fn classify_request(name: &str) -> classify::Request {
+    classify::Request {
+        source: fixture(name),
+        password: None,
+        sample_pages: None,
+    }
+}

Call sites then read:

let result = classify::handle(classify_request("text-two-page.pdf"), &cfg()).expect("classify");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/tests/fixtures.rs` around lines 34 - 51, Optionally add a shared
request-construction helper in the test module, such as classify_request, that
accepts the fixture name and initializes classify::Request with the fixture
source, password: None, and sample_pages: None. Update the repeated test setup
blocks, including a_text_document_classifies_as_text_based_and_needs_no_ocr, to
call the helper while preserving each test’s existing assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@console/web/src/components/chat/ChatView.tsx`:
- Around line 504-515: Update the queued-message PDF expansion block in ChatView
to iterate over expanded.failures after expandPdfAttachments, matching the
live-send path near makeSystemNotice. Append a warn makeSystemNotice for each
failure so editing a queued message visibly reports every PDF conversion failure
while preserving the existing block appending behavior.

In `@console/web/src/lib/pdf-attachments.ts`:
- Around line 252-254: Update escapeAttr to encode > as &gt; alongside the
existing attribute escapes, and update unescapeAttr to decode &gt; back to > so
attachment paths round-trip correctly through parseAttachedFileHeader.

In `@console/web/src/types/chat.ts`:
- Around line 52-58: Ensure the attachment file payload is removed before user
messages enter any client-persisted transcript backup or reload path, while
preserving attachment metadata and the visible chip. Update the relevant
transcript persistence flow around userMsg.attachments to explicitly strip file,
and document that session::message-added already receives server-side metadata
with file omitted.

In `@iii-permissions.yaml`:
- Around line 317-326: Update the PDF permissions entries for pdf::classify,
pdf::to-markdown, pdf::extract-text, pdf::extract-items, and
pdf::extract-regions so agent-facing requests cannot bypass output limits when
max_chars or max_items is 0. Enforce a non-overridable cap through the relevant
worker configuration, or remove these functions from the default-allow list to
retain approval gating.
- Around line 317-326: Remove the default-allow entries for pdf::classify,
pdf::to-markdown, pdf::extract-text, pdf::extract-items, and
pdf::extract-regions, and update PdfSource::load to enforce the same
shell::fs::read worker/jail scope, grant checks, canonicalization, and size
limits before metadata or file reads.

In `@pdf/src/functions/classify.rs`:
- Around line 132-145: Update detection_config so the nonzero sample value
passed to ScanStrategy::Sample is saturated at u32::MAX rather than truncated by
the usize-to-u32 cast. Preserve the existing sample == 0 behavior of
ScanStrategy::Full and ensure oversized sample_pages values cannot become
Sample(0).

In `@pdf/src/functions/markdown.rs`:
- Around line 203-207: Update the per_page handling in handle and the related
extract_per_page path to reject the request when req.per_page is enabled and
req.password.is_some(), returning an explicit error before extraction. Preserve
the existing unencrypted extraction behavior and ensure both referenced per_page
handling locations apply the same validation.
- Around line 198-201: Update the pages_converted calculation near page_filter
to cap the requested filter length at result.page_count, so out-of-range page
numbers are excluded from the reported converted count while preserving the
existing result.page_count fallback.

In `@pdf/src/source.rs`:
- Around line 304-308: Update the test encryption_errors_never_echo_the_password
and its describe_error input so the error text includes the password-like token
“secret” while retaining the encrypted-PDF context, then assert the resulting
message does not contain that token. Ensure the test exercises whether
describe_error removes or suppresses password content rather than passing an
unrelated error string.

In `@pdf/tests/golden/schemas/pdf.extract-text.json`:
- Around line 16-33: Update the shared Body.truncated description used by
pdf::extract-text so it does not instruct callers to retry with a pages filter,
since this function only accepts source and max_chars. Make that guidance
conditional on functions exposing a pages request property, or add pages support
to pdf::extract-text before retaining the shared description.

In `@pdf/tests/support/engine.rs`:
- Around line 85-104: Update the generated YAML in the configuration setup
around config_path so the interpolated directory value in the adapter’s
directory field is quoted and remains valid when paths contain spaces, colons,
or comment characters. Also preserve the engine’s stderr when startup fails so
boot() or with_stack can report the actual failure instead of silently treating
it as a missing engine.

---

Nitpick comments:
In `@console/web/src/lib/pdf-attachments.ts`:
- Around line 90-98: Update the PDF attachment processing flow around
fileToBase64 to validate attachment.file.size against an explicit maximum before
reading or encoding the file. Route oversized PDFs through the existing
failure-block path and provide an explanatory placeholder consistent with
oversized or scanned document handling; only call fileToBase64 for files within
the limit.

In `@pdf/build.rs`:
- Around line 129-141: Update build_ui to declare rerun-if-env-changed
directives for both SKIP_UI_BUILD and PNPM, so Cargo reruns the script whenever
either build-affecting environment variable changes.
- Around line 49-51: Replace the file-count-only skip condition in the staging
logic with a freshness check such as staged_matches, comparing each source file
with its destination by filename, size, and modification time. Return early only
when every source file exists in the staged directory, matches in size, is not
newer, and the file counts agree; otherwise perform the existing copy.

In `@pdf/src/cmaps.rs`:
- Around line 34-46: Move the cmaps::materialize() call out of the
#[tokio::main] async body and invoke it from a synchronous startup function
before the Tokio runtime is created. Preserve the existing materialize behavior
and ensure CMAP_DIR_ENV is set before any runtime worker threads can access the
process environment.

In `@pdf/src/config.rs`:
- Around line 50-53: Validate text_page_ratio_threshold after deserializing
configuration in both from_yaml and from_json, requiring a finite value within
the inclusive 0.0–1.0 range and returning a configuration error otherwise.
Centralize this check in a helper near the configuration implementation, then
invoke it on both parse paths before returning their results.

In `@pdf/src/functions/markdown.rs`:
- Around line 163-178: Replace the local `req.pages` validation and `HashSet`
construction in the markdown conversion flow with the existing
`items::page_filter` helper. Preserve the current `Option` behavior for omitted
pages and propagate the helper’s identical validation errors and collected page
filter.

In `@pdf/src/functions/mod.rs`:
- Around line 67-85: Update the register_blocking! macro to acquire a
configuration-sized semaphore permit before calling tokio::task::spawn_blocking,
retaining the permit for the extraction’s full duration and propagating
acquisition errors appropriately. Reuse the existing configuration or
concurrency-limit symbol exposed by the surrounding code, and preserve the
current panic and handler error mapping.

In `@pdf/src/main.rs`:
- Around line 123-124: Update the shutdown wait around tokio::signal::ctrl_c()
so it also completes when SIGTERM is received, then continue through
iii.shutdown_async() for either signal. Preserve the existing error propagation
and ensure both Ctrl+C and SIGTERM trigger the same graceful shutdown path.

In `@pdf/tests/fixtures.rs`:
- Around line 34-51: Optionally add a shared request-construction helper in the
test module, such as classify_request, that accepts the fixture name and
initializes classify::Request with the fixture source, password: None, and
sample_pages: None. Update the repeated test setup blocks, including
a_text_document_classifies_as_text_based_and_needs_no_ocr, to call the helper
while preserving each test’s existing assertions.

In `@pdf/tests/fixtures/README.md`:
- Around line 15-17: Update the fenced code block containing the
make_fixtures.py command by adding the bash language identifier to its opening
fence, while leaving the command unchanged.

In `@pdf/tests/integration.rs`:
- Around line 10-17: Update support::engine::with_stack to preserve its default
soft-skip behavior while panicking when boot() cannot locate iii and the opt-in
III_REQUIRE_ENGINE=1 environment variable is set. Ensure the PDF integration
test classify_answers_over_the_bus runs under this strict mode in the CI job so
missing-engine runs fail instead of passing without assertions.

In `@pdf/tests/support/engine.rs`:
- Around line 179-186: Replace the fixed 500 ms sleep in boot with the file’s
existing deadline-based polling pattern, repeatedly making a cheap call to a
registered function such as pdf::classify. Continue polling only while the
result indicates the function is not registered; treat the expected
invalid-input error as readiness, and preserve the existing deadline/timeout
behavior before returning Stack.

In `@pdf/ui/build.mjs`:
- Around line 15-36: Update the esbuild options object used by the production
build to enable minification, while keeping watch-mode behavior functional and
unchanged apart from using the shared options.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ecb72f69-5845-4104-9b6f-2cf35fc4224c

📥 Commits

Reviewing files that changed from the base of the PR and between 5eaa26f and 3c53e0d.

⛔ Files ignored due to path filters (4)
  • pdf/Cargo.lock is excluded by !**/*.lock
  • pdf/tests/fixtures/no-text.pdf is excluded by !**/*.pdf
  • pdf/tests/fixtures/text-two-page.pdf is excluded by !**/*.pdf
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (52)
  • .github/workflows/create-tag.yml
  • .github/workflows/release.yml
  • README.md
  • console/web/src/components/chat/AttachmentButton.tsx
  • console/web/src/components/chat/ChatView.tsx
  • console/web/src/lib/pdf-attachments.test.ts
  • console/web/src/lib/pdf-attachments.ts
  • console/web/src/types/chat.ts
  • iii-permissions.yaml
  • pdf/Cargo.toml
  • pdf/README.md
  • pdf/build.rs
  • pdf/examples/probe.rs
  • pdf/iii.worker.yaml
  • pdf/skills/SKILL.md
  • pdf/src/cmaps.rs
  • pdf/src/config.rs
  • pdf/src/configuration.rs
  • pdf/src/functions/classify.rs
  • pdf/src/functions/items.rs
  • pdf/src/functions/markdown.rs
  • pdf/src/functions/mod.rs
  • pdf/src/functions/regions.rs
  • pdf/src/functions/text.rs
  • pdf/src/guidance.rs
  • pdf/src/lib.rs
  • pdf/src/main.rs
  • pdf/src/manifest.rs
  • pdf/src/source.rs
  • pdf/src/ui.rs
  • pdf/tests/fixtures.rs
  • pdf/tests/fixtures/README.md
  • pdf/tests/fixtures/make_fixtures.py
  • pdf/tests/golden/schemas/pdf.classify.json
  • pdf/tests/golden/schemas/pdf.extract-items.json
  • pdf/tests/golden/schemas/pdf.extract-regions.json
  • pdf/tests/golden/schemas/pdf.extract-text.json
  • pdf/tests/golden/schemas/pdf.to-markdown.json
  • pdf/tests/integration.rs
  • pdf/tests/manifest.rs
  • pdf/tests/schemas.rs
  • pdf/tests/support/engine.rs
  • pdf/tests/support/mod.rs
  • pdf/ui/build.mjs
  • pdf/ui/package.json
  • pdf/ui/page.tsx
  • pdf/ui/src/function-trigger-message/index.tsx
  • pdf/ui/src/lib/api.ts
  • pdf/ui/src/page/index.tsx
  • pdf/ui/styles.css
  • pdf/ui/tsconfig.json
  • pnpm-workspace.yaml

Comment thread console/web/src/components/chat/ChatView.tsx
Comment on lines +252 to +254
function escapeAttr(value: string): string {
return value.replaceAll('&', '&amp;').replaceAll('"', '&quot;')
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the "chip renderer" referenced in the file's top comment, and check
# how it parses the <attached-file ...> envelope (regex vs. attribute-aware).
rg -n "attached-file" console/web/src -C3
rg -n "escapeAttr" console/web/src -C3

Repository: iii-hq/workers

Length of output: 18056


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- file-mentions.ts relevant functions ---\n'
sed -n '130,210p' console/web/src/lib/file-mentions.ts | cat -n

printf '\n--- pdf-attachments.ts relevant functions ---\n'
sed -n '160,260p' console/web/src/lib/pdf-attachments.ts | cat -n

printf '\n--- references to file-mentions.ts / AttachedFileHeader ---\n'
rg -n "parseAttachedFileHeader|isAttachedFileBlock|AttachedFileHeader|attached-file" . -g '!node_modules' -g '!dist' -g '!build' -C2 | sed -n '1,220p'

Repository: iii-hq/workers

Length of output: 243


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- file-mentions.ts relevant functions ---'
sed -n '130,210p' console/web/src/lib/file-mentions.ts | cat -n

echo
echo '--- pdf-attachments.ts relevant functions ---'
sed -n '160,260p' console/web/src/lib/pdf-attachments.ts | cat -n

echo
echo '--- references to file-mentions.ts / AttachedFileHeader ---'
rg -n "parseAttachedFileHeader|isAttachedFileBlock|AttachedFileHeader|attached-file" . -g '!node_modules' -g '!dist' -g '!build' -C2 | sed -n '1,220p'

Repository: iii-hq/workers

Length of output: 20527


Escape < in escapeAttr as the header terminator.

parseAttachedFileHeader closes the header with the first > and reads double-quoted attributes with "([^"]*)". A path value containing > becomes an unescaped raw terminator, so normal characters can split the block into header/body content. Add &gt; escaping for this sender, and add the matching round-trip in unescapeAttr.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] 252-252: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: value.replaceAll('&', '&').replaceAll('"', '"')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(manual-sanitization-typescript)


[warning] 252-252: Avoid hand-rolled HTML escaping (replacing characters with HTML entities); use a vetted encoder/sanitizer such as DOMPurify or sanitize-html.
Context: value.replaceAll('&', '&')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').

(manual-sanitization-typescript)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@console/web/src/lib/pdf-attachments.ts` around lines 252 - 254, Update
escapeAttr to encode > as &gt; alongside the existing attribute escapes, and
update unescapeAttr to decode &gt; back to > so attachment paths round-trip
correctly through parseAttachedFileHeader.

Source: Linters/SAST tools

Comment thread console/web/src/types/chat.ts
Comment thread iii-permissions.yaml
Comment on lines +317 to +326
# pdf: every function is a pure read of a document the agent could already
# reach through the filesystem scope, and reaching it any other way returns
# binary noise. Nothing here writes, spends, or leaves the machine, and each
# response is capped by the worker's own configuration. Gating these would
# mean an approval prompt to read a file the agent is already allowed to open.
- pdf::classify
- pdf::to-markdown
- pdf::extract-text
- pdf::extract-items
- pdf::extract-regions

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not default-allow uncapped PDF requests.

The comment says that every response is capped by worker configuration. However, max_chars: 0 disables the text cap, and max_items: 0 disables item truncation. These functions are now callable by agents without approval. A single call can therefore produce document-sized output and exhaust worker or harness resources.

Enforce a non-overridable agent-facing cap, or keep these functions approval-gated.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iii-permissions.yaml` around lines 317 - 326, Update the PDF permissions
entries for pdf::classify, pdf::to-markdown, pdf::extract-text,
pdf::extract-items, and pdf::extract-regions so agent-facing requests cannot
bypass output limits when max_chars or max_items is 0. Enforce a non-overridable
cap through the relevant worker configuration, or remove these functions from
the default-allow list to retain approval gating.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  'struct PdfSource|impl PdfSource|fn load|std::fs::read|fs::read|canonicalize|scope|jail|workspace' \
  pdf/src/source.rs pdf/src pdf/tests --glob '*.rs'

Repository: iii-hq/workers

Length of output: 12141


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== pdf source file =="
wc -l pdf/src/source.rs
sed -n '1,120p' pdf/src/source.rs

echo "== candidate function signatures =="
rg -n -C 4 'pub (use|mod)|path: PdfSource|PdfSource::load|parse_request|Request::|handle\(' pdf/src pdf/tests --glob '*.rs'

echo "== filesystem read implementations =="
rg -n -C 8 'fn .*read|read_file|filesystem|scope|workspace|Canonical|canonicalize|base_path|jail|std::fs::read|fs::read' --glob '*.rs' | head -n 240

Repository: iii-hq/workers

Length of output: 23436


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== list repository files around shell/fs/pdf =="
git ls-files | rg '(^|/)shell|fs|iii-permissions\.yaml|permissions|pdf/' | sed -n '1,220p'

echo "== search Permission/Function declarations for pdf/shell/fs =="
rg -n -C 3 "\bpdf::|shell::fs::read|struct .*Permission|Permission|read_path|scope|filesystem|jail|workspace|canonicaliz" . \
  --glob '!target/**' --glob '!node_modules/**' | sed -n '1,260p'

echo "== locate candidate fs implementations by filename =="
fd -e rs . | xargs rg -n -l 'std::fs::read|fs::read|canonicalize|scope|jail|workspace|read_path' | sed -n '1,120p'

Repository: iii-hq/workers

Length of output: 30969


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== shell fs/mod.rs =="
sed -n '1,220p' shell/src/fs/mod.rs

echo "== shell fs/host.rs and sandbox.rs relevant sections =="
sed -n '1,260p' shell/src/fs/host.rs
sed -n '1,260p' shell/src/fs/sandbox.rs

echo "== shell path/mod.rs =="
sed -n '1,220p' shell/src/path/mod.rs

echo "== shell fs_dispatch.rs and fs_read.rs =="
sed -n '1,220p' shell/src/functions/fs_dispatch.rs
sed -n '1,180p' shell/src/functions/fs_read.rs

echo "== pdf worker permissions in iii-permissions.yaml =="
sed -n '290,340p' iii-permissions.yaml

Repository: iii-hq/workers

Length of output: 34878


Enforce shell::fs::read guardrails in PdfSource::load.

PdfSource::load resolves the caller-supplied path with std::fs::metadata and reads it with std::fs::read, so it bypasses shell::fs::read's worker/jail scope and canonicalization path. Default-allowing these PDF reads exposes arbitrary-host-file reads for callers with shell::fs::read; only allow them when PdfSource::load applies the same scope, grant, canonicalization, and size protections before reading.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@iii-permissions.yaml` around lines 317 - 326, Remove the default-allow
entries for pdf::classify, pdf::to-markdown, pdf::extract-text,
pdf::extract-items, and pdf::extract-regions, and update PdfSource::load to
enforce the same shell::fs::read worker/jail scope, grant checks,
canonicalization, and size limits before metadata or file reads.

Comment thread pdf/src/functions/classify.rs
Comment on lines +198 to +201
let pages_converted = page_filter
.as_ref()
.map(|f| f.len() as u32)
.unwrap_or(result.page_count);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

pages_converted counts requested pages, not converted pages.

The filter length includes page numbers beyond the document. A request for pages: [1, 2, 999] on a two-page document reports pages_converted: 3. Clamp the count to pages that exist.

🐛 Proposed fix
     let pages_converted = page_filter
         .as_ref()
-        .map(|f| f.len() as u32)
+        .map(|f| f.iter().filter(|&&p| p <= result.page_count).count() as u32)
         .unwrap_or(result.page_count);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let pages_converted = page_filter
.as_ref()
.map(|f| f.len() as u32)
.unwrap_or(result.page_count);
let pages_converted = page_filter
.as_ref()
.map(|f| f.iter().filter(|&&p| p <= result.page_count).count() as u32)
.unwrap_or(result.page_count);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pdf/src/functions/markdown.rs` around lines 198 - 201, Update the
pages_converted calculation near page_filter to cap the requested filter length
at result.page_count, so out-of-range page numbers are excluded from the
reported converted count while preserving the existing result.page_count
fallback.

Comment thread pdf/src/functions/markdown.rs
Comment thread pdf/src/source.rs
Comment thread pdf/tests/golden/schemas/pdf.extract-text.json
Comment thread pdf/tests/support/engine.rs
The hook ran on every single generation. Gating what it injected was the wrong
fix: the span still fired on "hi how are you", because the harness invokes a
bound hook whether or not it has anything to say. A worker that reads documents
has no business in the turn loop of a conversation about the weather.

`fp` earns a hook because moving bulk data is a rule that applies to every turn.
Reading a PDF is not; it is an on-demand capability, and the ways to find one
already exist: the function registry an agent searches, the skill that says when
to reach for it, and the console page a person opens. This worker now registers
no harness hook and binds no trigger type, so a conversation that never touches
a document never pays for it and there is no per-turn cost to having it
installed.

The guidance content moves to `skills/SKILL.md`, which is where "when to use
this worker" belongs, and the README states the on-demand contract. The
integration test now asserts the inverse of what it used to: the worker is
discoverable through `engine::functions::list` and registers no hook.

Operational note for anyone reproducing this: a hard-killed worker leaves its
Message-path triggers registered, because SIGKILL skips the SDK's graceful
shutdown. Three stale bindings had to be removed with
`engine::unregister_trigger` rather than waiting for a garbage collection that
only happens on a clean disconnect.
…ew fixes

The important one: a `path` was read without checking the filesystem scope the
harness stamps on every call it dispatches. With these functions on the default
allow list, an agent could name any document on the machine and get its text
back, outside the scope its session was granted. Paths are now canonicalized
first, then checked against the scope's root and grants, so a symlink or a `..`
cannot walk out. The comparison is on resolved paths, so a sibling whose name
merely shares a prefix with the root is not treated as inside it. An unstamped
call is an operator or console call and is unaffected, and inline bytes carry no
path to escape with.

The rest:

- `pdf::classify` cast a `usize` sample count to `u32`. A value above `u32::MAX`
  wrapped, and wrapping to zero means `Sample(0)`, which samples nothing.
  Saturates now.
- `pdf::to-markdown` with both `per_page` and `password` extracted the whole
  document and then failed on the per-page pass, because that entry point cannot
  decrypt. Refused up front, naming which half to drop.
- `pages_converted` reported the size of the requested filter, so naming pages
  past the end overstated what was read. Clamped to the document.
- A `>` in a file name truncated the attachment header, because the header ends
  at the first `>`. Escaped alongside `&` and `"`.
- SIGTERM now takes the same graceful path as ctrl-c. A managed worker is
  stopped with SIGTERM, and dying without `shutdown_async` leaves Message-path
  triggers registered against a function that no longer exists.
- An oversized PDF is refused in the composer before it is encoded, rather than
  freezing the tab and then being rejected by the worker's own ceiling.
- `text_page_ratio_threshold` is a share, so a value outside 0.0 to 1.0 is
  rejected at parse time on both paths instead of silently classifying every
  document the same way.
- The queued-message path now reports conversion failures, matching the live
  send path; staying silent let an edited queued message lose its document.
- The attachment's `File` is dropped once expansion is done, rather than holding
  the whole document in memory for the life of the conversation.
- The shared truncation message told every caller to retry with a page filter,
  which `pdf::extract-text` does not accept.
- The password-redaction test fed in an error containing no password, so it
  asserted nothing. It now uses one that does.
- `build.rs` reruns when `SKIP_UI_BUILD` or `PNPM` changes; the config test
  fixture quotes its interpolated path; the fixtures README fences its command.
@rohitg00
rohitg00 merged commit df0102e into main Aug 4, 2026
20 checks passed
@rohitg00
rohitg00 deleted the feat/pdf-worker branch August 4, 2026 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant