fix: record every output file when a converter splits its input

[tizius79]

The problem

Not every converter writes the single file it is asked for. ImageMagick turns a multi-page PDF into one numbered file per page:

name-0.jpg
name-1.jpg
name-2.jpg

handleConvert stored the expected name (name.jpg) in file_names regardless of what was actually written, so the results page linked to a file that does not exist and every download failed:

ENOENT: no such file or directory, open './data/output/1/4/name .jpg'

The conversion itself succeeded — the log even says Converted ... successfully using imagemagick — but the output was unreachable from the UI. The Tar button still worked, since /archive/:jobId packs the whole output directory and ignores the recorded names. Probably the same root cause as #321.

The change

Each conversion now runs in a directory of its own, and its results are moved into the job's output directory afterwards. Whatever ends up in that directory was written by that conversion and by nothing else, so the files it produced are known rather than inferred from names in a shared directory — an input called report-1.pdf can no longer be credited with a page of report.pdf, and an input whose expected name was already written by an earlier chunk is no longer handed that earlier file.

One row is recorded per produced file. jobs.num_files is raised to match, which matters: results.tsx compares it against the number of rows to decide a job is finished, so without it the progress bar never completes and the Delete and Tar buttons stay disabled.

Output names are handed out once per job. Two inputs can genuinely produce the same name — report.pdf split into pages and report-1.pdf both write report-1.jpg — and moving the second over the first would lose a page and leave a result row pointing at another conversion's content, so the later arrival is given a name of its own.

Conversions that write the single expected file, and conversions that fail, record exactly the row they did before.

Smaller details in the same change: the move falls back to a copy when rename reports EXDEV, directories a converter leaves behind are not mistaken for output files, and the scratch directory name carries a random component so two conversions of one job cannot share it.

Testing

Built both images and converted the same 3-page PDF through each, following the download link the results page actually renders.

	rows shown	download link
v0.18.0	name.jpg (never written)	HTTP 500
this branch	name-0.jpg, name-1.jpg, name-2.jpg	HTTP 200, 200, 200

Also checked, with MAX_CONVERT_PROCESS=1 to force separate chunks:

• report.pdf (3 pages) and report-1.pdf (1 page) in one job — four distinct files, four working links, no page lost; the collision lands as report-1 (2).jpg.
• report.png and report.pdf in one job, both expecting report.jpg — the PDF records its own three pages rather than the PNG's output.
• 1-page PDF to JPEG and to PNG — one row and a working link, unchanged from before.
• No scratch directories left behind in any run.

prettier --check, tsc --noEmit and eslint all pass.

[cubic-dev-ai]

All reported issues were addressed across 1 file

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

All reported issues were addressed across 1 file (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/converters/main.ts">

<violation number="1" location="src/converters/main.ts:272">
P2: The move loop at line 273 moves every file produced in the scratch dir, but only `outputNames` (the filtered result of `pickOutputNames`) are recorded as rows. Any produced file that `pickOutputNames` rejects — auxiliary files (e.g. LaTeX `name.aux`/`name.log`) or numbered siblings when the expected name was also written (the `produced.includes(expectedName)` early-return ignores them) — is still moved into the served output directory, where it becomes an unlisted orphan that the `/archive` route still tars. Only move the names in `outputNames` so the written files and the recorded/downloadable rows stay consistent.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The move loop at line 273 moves every file produced in the scratch dir, but only outputNames (the filtered result of pickOutputNames) are recorded as rows. Any produced file that pickOutputNames rejects — auxiliary files (e.g. LaTeX name.aux/name.log) or numbered siblings when the expected name was also written (the produced.includes(expectedName) early-return ignores them) — is still moved into the served output directory, where it becomes an unlisted orphan that the /archive route still tars. Only move the names in outputNames so the written files and the recorded/downloadable rows stay consistent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/converters/main.ts, line 272:

<comment>The move loop at line 273 moves every file produced in the scratch dir, but only `outputNames` (the filtered result of `pickOutputNames`) are recorded as rows. Any produced file that `pickOutputNames` rejects — auxiliary files (e.g. LaTeX `name.aux`/`name.log`) or numbered siblings when the expected name was also written (the `produced.includes(expectedName)` early-return ignores them) — is still moved into the served output directory, where it becomes an unlisted orphan that the `/archive` route still tars. Only move the names in `outputNames` so the written files and the recorded/downloadable rows stay consistent.</comment>

<file context>
@@ -230,53 +232,71 @@ export async function handleConvert(
+
+          const outputNames = pickOutputNames(produced, newFileName);
+
+          for (const name of produced) {
+            await moveInto(scratchDir, userOutputDir, name);
+          }
</file context>

Suggested change

	for (const name of produced) {
	await moveInto(scratchDir, userOutputDir, name);
	}
	for (const name of outputNames) {
	await moveInto(scratchDir, userOutputDir, name);
	}

[tizius79]

Deliberate, and I would rather keep it.

Before this change converters wrote straight into the served output directory, so auxiliary files already landed there, unlisted, and /archive/:jobId tarred them along with everything else. Moving only outputNames would now delete them with the scratch directory, taking away files users previously received in the tar.

Moving everything preserves the existing behaviour exactly; the only thing that changed is which directory the files are written from. Happy to switch if you would rather the output directory hold only listed files, but that felt like a separate decision from fixing the broken downloads.

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/converters/main.ts">

<violation number="1" location="src/converters/main.ts:272">
P1: When two conversions for the same job overlap or a job is retried, each `handleConvert` call has its own `claimed` set, so the later rename can overwrite the earlier conversion's output and both rows point to the same content. Share destination claims across calls or reserve names atomically in the output directory.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When two conversions for the same job overlap or a job is retried, each handleConvert call has its own claimed set, so the later rename can overwrite the earlier conversion's output and both rows point to the same content. Share destination claims across calls or reserve names atomically in the output directory.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/converters/main.ts, line 272:

<comment>When two conversions for the same job overlap or a job is retried, each `handleConvert` call has its own `claimed` set, so the later rename can overwrite the earlier conversion's output and both rows point to the same content. Share destination claims across calls or reserve names atomically in the output directory.</comment>

<file context>
@@ -241,6 +268,9 @@ export async function handleConvert(
   });
 
+  // Output names handed out so far, so conversions never overwrite each other.
+  const claimed = new Set<string>();
+
   for (const chunk of chunks(conversions, MAX_CONVERT_PROCESS)) {
</file context>

[tizius79]

Real, but it predates this PR and this change does not widen it. Converters used to write straight into the output directory, so a job converted a second time overwrote its earlier files in exactly the same way, with the older rows already pointing at the newer content.

Reserving names on disk across calls would change that behaviour rather than restore it: a job converted twice would start accumulating name (2).jpg, name (3).jpg, which seems worse than the problem it solves and well outside a fix for broken downloads.

For genuinely concurrent conversions this branch is already an improvement. Each conversion writes into a directory of its own, so two overlapping runs can no longer interleave writes to the same path; the loser of a rename is a whole file rather than a half-written one.

The comment above claimed did promise more than the code delivers, though, so I have corrected it to say the guarantee holds for the conversions started by that call.