Preserve surrogate pairs when truncating imported file names

[charan-rathore]

What changed

Current main already fixes the original split-surrogate defect: merged #148 makes sanitizeFileName in apps/server/src/files.ts slice whole code points before joining, so truncation no longer creates a lone surrogate, and this branch builds on that code with the same 180-code-point cap.

This PR adds one narrow line on top: after slicing, strip a trailing high surrogate, so a name that arrives already broken — an unpaired high surrogate landing exactly at the 180-code-point cut — still yields an encodable stored name. On current main, sanitizeFileName("x".repeat(179) + String.fromCharCode(0xD800) + ".pdf") returns a name ending in a lone high surrogate whose encodeURIComponent throws URIError; with the strip the result is 179 characters and encodable.

Scope is deliberately narrow: mid-name lone surrogates and a boundary low surrogate still throw on both versions, and no live input path delivering a source-broken name on current main has been demonstrated — the Gmail attachment path is guarded by #121, and UTF-8-decoded upload names cannot carry lone surrogates. This is edge-case hardening with regression tests, not a demonstrated defect. Existing ASCII names and complete surrogate pairs keep their behavior. Previously stored malformed names are not migrated.

Verification

• The strip regression (sanitizeFileName strips a trailing unpaired surrogate and keeps its fallback) fails against main's files.ts and passes with the change.
• tests/file-name-unicode.test.ts and tests/resource-ids.test.ts: 7/7 pass, including the end-to-end HTTP regression (local session, multipart PDF upload with a boundary-crossing emoji name, signed content URL fetch: status 200, Content-Disposition, PDF MIME type, byte-identical content).
• Branch integrated with current main (31bdd92). No full-suite pass claimed beyond the two files above.

Integration limits

All HTTP tests use the real Hono route in-process in sample mode with a disposable local store and generated PDF. No live provider, personal files or deployment used. Native/web exports, browser-worker lifecycle and Docker checks were not run.

[jerelvelarde]

The Unicode truncation fix is useful and the Unicode/PDF suites pass (9 tests); the description fits the template. Please rebase onto current main and apply the trailing-surrogate removal inside sanitizeFileName(), after slice(0, 180), retaining its document.pdf fallback and the file-ID/path guards from #47. The current Files.import hunk conflicts with that extraction. Rerun the Unicode HTTP regression plus file security coverage and CI on the integrated implementation.

[charan-rathore]

@jerelvelarde Rebased onto current main in 4c59b9f. The trailing-surrogate removal now lives inside sanitizeFileName(), right after slice(0, 180), with the document.pdf fallback and the file-ID/path guards from #47 untouched. The import hunk is gone; the behavior is exercised through the extracted function.

Also added a small direct test for sanitizeFileName covering the trailing-surrogate cut, the empty-name fallback, and the path-strip case.

Reruns on the integrated implementation:

• tests/file-name-unicode.test.ts (unit + HTTP content-route regression): 3 passed
• tests/resource-ids.test.ts and tests/api.test.ts (file security coverage): 11 passed
• tsc --noEmit clean, biome clean on the two changed files

[jerelvelarde]

The original split-surrogate defect is already fixed on current main by merged #148: sanitization truncates code points before joining, preserving encoding, document.pdf fallback and basename handling. Direct boundary checks confirm both reported inputs remain encodable. This head's production conflict concerns a different cap policy (180 UTF-16 units versus main's 180 code points), so replacing main is not a routine integration fix. Clarify the incremental benefit/policy, update the stale description claiming the defect remains, and integrate/test the chosen behavior. No new security defect demonstrated. A fresh HTTP regression was initially limited by missing local dependencies; the direct sanitizer checks do not establish full HTTP verification.

[charan-rathore]

@jerelvelarde Done in 9c80884. Rebased onto current main (2a68bb6) and applied the trailing-surrogate removal inside sanitizeFileName() after slice(0, 180), keeping its document.pdf fallback and the file-ID/path guards from #47 untouched.

One judgment call to flag: main's extracted version slices code points (Array.from(...).slice(0, 180)), which already keeps pairs intact, and resource-ids.test.ts pins that behavior. So I kept the code-point slicing and added only the strip of a trailing unpaired high surrogate after the join - a trailing high surrogate is always unpaired, and dropping it keeps the name encodable for the content-disposition header. My test expectations were updated to code-point semantics accordingly, including a valid-pair-at-the-end guard.

Ran: Unicode HTTP regression 3/3 (red-checked: the lone-surrogate case fails without the strip), file security coverage (resource-ids) 4/4. The full local suite SIGKILLed 3 browser test files (browser, conversation-browser, conversation-calendar) under load in my environment - resource exhaustion, not assertion failures. CI is definitive for the full run.

[charan-rathore]

Honest reassessment on top of #148, as asked:

You're right on the original defect. The split-surrogate bug this PR was opened for is fixed on current main by #148's slice-before-join; my own pair-at-the-boundary checks ("x"*179 + "😀" + ".pdf") pass on main exactly as they do here. The description still claims the defect remains — that's stale and I'm correcting it.

The cap-policy conflict is also stale. After the earlier rebase, this branch sits on top of #148 and keeps main's 180-code-point cap untouched. The PR diff against current main is exactly one line of behavior — the trailing strip — plus the test file. No 180-units-vs-180-code-points replacement remains.

What the strip still does: it covers one residual edge #148 doesn't — a name that arrives already broken, with a lone high surrogate landing exactly at the 180-code-point cut. Direct check on current main: sanitizeFileName("x".repeat(179) + String.fromCharCode(0xD800) + ".pdf") returns the 179 x's with the lone high surrogate still attached, and encodeURIComponent of it throws URIError, so the content-disposition path would hit an unencodable name. With the strip the result is 179 chars and encodable. The committed test strips a trailing unpaired surrogate and keeps its fallback shows it: red against main's files.ts, green here.

The honest limits: mid-name lone surrogates and a boundary low surrogate still throw on both versions, and I have not demonstrated a live input path that delivers a source-broken name on current main — the Gmail attachment path is guarded by #121, and UTF-8-decoded upload names can't carry lone surrogates. So this is narrow edge-case hardening, not a demonstrated defect.

Merged current main (31bdd92); file-name-unicode and resource-ids suites pass 7/7, typecheck clean.

Given that, your call: keep the PR as the one-line boundary strip with its regression tests, or close it as superseded by #148? I'm fine with either — I'd rather the PR earn its diff than coast on a stale one.