Skip to content

fix(split): write split pages without cwd-relative outputFolder (#75) - #79

Merged
haywolf merged 3 commits into
masterfrom
fix/75-win-cross-drive-split
Nov 4, 2026
Merged

haywolf merged 3 commits into
masterfrom
fix/75-win-cross-drive-split

Conversation

@haywolf

@haywolf haywolf commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #75 (also addresses #20)

Root cause: path.relative(process.cwd(), taskDir) returns an absolute path when cwd and taskDir are on different drives. pdf-to-png-converter then joins it onto cwd, producing an invalid path like D:\...\C:\..., which fails with ENOENT. wrapError then rewrites every ENOENT as "PDF file not found".

Changes

  • PDFSplitter no longer passes outputFolder to the library. It renders pages to buffers and writes them itself to {userData}/files/{taskId}/split/, so nothing depends on cwd any more. This also fixes UNC userData paths, which were writing into cwd and then failing with EXDEV.
  • Rendering and writing retry separately.
  • A failed write now reports "Failed to write split images to {taskDir}" with the original error.
  • "Moved or deleted" is only reported when fs.access confirms the source file is actually missing.
  • Tests: new PDFSplitter.win32.test.ts (cross-drive, UNC and same-drive cases via path.win32), a new integration test using the real library and filesystem, and 7 new unit cases.

Tests: test:unit 959 passed, typecheck and build pass. With the fix reverted, 14 of 29 splitter tests fail.

Not verified: a real Windows install on a non-C drive, and macOS.

Follow-up (not in this PR): ImageSplitter.wrapError has the same misleading ENOENT message.

Merge order: #72 → #75 → #77 (none of the three PRs touch the same files), then run a release candidate to validate the release pipeline.

…20)

path.relative(cwd, taskDir) yields an absolute path across Windows drives,
which pdf-to-png-converter then joins onto cwd (D:\...\C:\...). Render to
buffers and write to the task dir ourselves; only report 'source file not
found' when the source really is missing. Adds win32 cross-drive/UNC tests.

Fixes #75
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

AI Code Review Summary

PR: #79 (fix(split): write split pages without cwd-relative outputFolder (#75))
Preferred language: English

Overall Assessment

No blocking issue was detected in the reviewed diff; keep focused regression testing before merge.

Major Findings by Severity

No major issues identified from the reviewed diff.

Actionable Suggestions

  • Address the highest severity findings first and add targeted tests for changed logic.

Potential Risks

  • Potential hidden risks remain in edge cases not covered by the current diff context.

Test Suggestions

  • Add happy-path + boundary + failure-path tests for touched modules.

File-Level Coverage Notes

  • src/core/infrastructure/adapters/split/PDFSplitter.ts: File was in scope but not fully reviewed before budget limits. (coverage_incomplete)
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.integration.test.ts: File was in scope but not fully reviewed before budget limits. (coverage_incomplete)
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.test.ts: File was in scope but not fully reviewed before budget limits. (coverage_incomplete)
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.win32.test.ts: File was in scope but not fully reviewed before budget limits. (coverage_incomplete)

Inline Downgraded Items (processed but not inline)

  • None

Coverage Status

  • Target files: 4
  • Covered files: 0
  • Uncovered files: 4
  • No-patch/binary covered as file-level: 0
  • Findings with unknown confidence (N/A): 0

Uncovered list:

  • src/core/infrastructure/adapters/split/PDFSplitter.ts: coverage_incomplete
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.integration.test.ts: coverage_incomplete
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.test.ts: coverage_incomplete
  • src/core/infrastructure/adapters/split/tests/PDFSplitter.win32.test.ts: coverage_incomplete

No-patch covered list:

  • None

Runtime/Budget

  • Rounds used: 1/4
  • Planned batches: 1
  • Executed batches: 1
  • Sub-agent runs: 1
  • Planner calls: 2
  • Reviewer calls: 2
  • Model calls: 4/64
  • Structured-output summary-only degradation: YES

Reasons:

  • planner_structured_output_failed_round_1: Structured output failed after repair: You have no permission to access this resource (api_key_source: payg) (request_id: 954471ca50444dfbbba842eb82e4fb6f)
  • reviewer_structured_output_failed_round_1_general: Structured output failed after repair: You have no permission to access this resource (api_key_source: payg) (request_id: ca8918766e204c76a2956fffe847d539)

@haywolf
haywolf merged commit fa6e832 into master Nov 4, 2026
3 checks passed
@haywolf
haywolf deleted the fix/75-win-cross-drive-split branch November 4, 2026 04:31
haywolf added a commit that referenced this pull request Nov 4, 2026
Write retries reused the PDF keyword check, so an output path containing
'password' or 'encrypted' made a transient EBUSY fail at once. withRetry
now takes an optional isRetryable (default: the PDF check); page writes
retry on any error. Also cover the 'page has no image' branch.

Follow-up to #79.
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.

解决了PDF file not found: test2.pdf. The file may have been moved or deleted.的问题

1 participant