[sylvesterkaczmarek]

Summary

Canceling a scan during PDF page lookup could still start text extraction. If cancellation happened around cleanup, the caller could receive a PDF error or a “no extractable text” error instead of its original abort reason. Fixes #531.

Changes

Check the signal after document and page lookup. Always close the PDF, then honor cancellation whether cleanup succeeds or rejects. When the caller hasn’t canceled, cleanup failures still surface with their original cause.

Testing

• Reproduced three failures on main at 28a18608 using a controlled PDF loader. The affected cases were cancellation during page lookup, cleanup rejecting after cancellation, and cancellation while successfully closing a blank PDF.
• All 72 knowledge-base tests passed on Linux, including all seven PDF cancellation and control cases. Each case exercises both snapshot extraction and knowledge-base preparation. The suite also reads a real searchable PDF.
• SDK and MCP type checks, SDK formatting, and the SDK build passed.

Risk and rollout

An in-flight PDF operation still has time to finish and release its resources. This change affects which operation starts next and which error reaches the caller.

Public disclosure review

• No customer, partner, prospect, or user identities, data, or identifying details are included.
• No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
• I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

[sylvesterkaczmarek]

Rebased onto current main and corrected the cancellation regression's stale assertion: the third cancellation check aborts inside PDF extraction, and the catch intentionally performs one final throwIfAborted call to preserve the original reason. Current head is GitHub Verified; TypeScript build:ci and the focused PDF cancellation regression pass, Prettier is clean, and git diff --check passes. Could a maintainer review the current head?

[chatgpt-codex-connector]

Note

You have reached your Codex usage limits for security reviews. Please try again later.

[sylvesterkaczmarek]

Refreshed this PR onto current main (9cfdc336) as signed commit 9f2af70, retaining the existing PDF cancellation handling and regression suite. The previous merge conflict is resolved; GitHub now reports the PR as mergeable. On macOS, all 51 knowledge-base tests pass (including 6 PDF-cancellation boundary tests), along with the SDK TypeScript CI check, scoped Prettier and git diff --check. Awaiting hosted CI and maintainer review.

[sylvesterkaczmarek]

Resolved the conflict in 27727fb while retaining current upstream behavior. All 52 knowledge-base tests and type/schema checks pass; the two remaining cancellation regressions fail on unchanged main. Could a maintainer review and approve CI?

[sylvesterkaczmarek]

Following up on the PDF cancellation fix now that the refreshed head aeac2deb is mergeable and the hosted CI matrix has completed without failures (76 check entries, including skipped matrix placeholders). The change remains limited to cancellation checks after PDF loading/page lookup and preserving the caller's abort reason if cleanup also fails; the focused extraction regressions and prior knowledge-base validation are documented above.

Could a maintainer review this head when convenient? Happy to address any specific feedback.

[sylvesterkaczmarek]

Thanks for checking the blank-page cleanup case too, Michael. The extra cancellation check after PDF cleanup addresses that gap while leaving ordinary cleanup errors visible. Appreciate you tightening the tests around both entry points.