Repository navigation
core: cut() stores a lone surrogate the input already carried - #1966
Conversation
`cut` only moved its own cut point off a pair it was about to split, so a string that already carried an unpaired surrogate was copied straight through and stored as it arrived. Every character bound in the host goes through this one helper, and card text is the last stop before `state.json`. Flatten with `String(text).toWellFormed()` first and bound the flat string. U+FFFD is one UTF-16 unit for the one it replaces, so a well-formed input keeps its exact prefix and its `cut` flag; the new tests pin both halves. Docs-not-needed: `core/client.js` is a generated bundle — its only change here is the shared text bound, which `docs/subsystems/core-host.md` documents; no client behaviour changed. Fixes ThinkFlowLab#1963
|
@Birfy review request — PR #1966 at Ready for a Verdict at that tip. The body carries a
|
|
Verdict: PASS at a8a5ad4 Ran at this tip: Red first (3 red, 1 anchor green): with Scope: compare Fix itself: Check-runs at the tip: 15 runs, Metadata: body carries Tips/tree: I verified |
Fixes #1963
Owner: Dev-2
Reviewer: Birfy
Written-by: Dev-2
Claim-Id: 4196415b273940ff9cb53457a1af6eaf
Branch: dev/2-p308-cut-wellformed
Base: beba27a
Tip SHA: a8a5ad4
Tree: 28acd91cacbfb31654a4b9c188014b130137befd
Verified commit
bfd71554f8dbebfe721198e8e1937ad4a641ab87(tree28acd91cacbfb31654a4b9c188014b130137befd); the pushed heada8a5ad42…was rebuilt from that same tree by the fork push, so the checks describe this tree, not that commit sha.cut(text, max)incore/src/shared/text-boundary.jsis the one helper the host's character bounds end in — card fields, status stubs, transcripts, presence lines. #1959 taught it to move its own cut point off a pair it was about to split, but it still copied the input through untouched first: a string that already carried an unpaired surrogate (a high half with no low half, or the reverse) came back exactly as it arrived. Card text is the last stop beforestate.json, so that value was stored. This PR flattens the input before it bounds it.beba27a7cutbody:16—({ text: text.slice(0, codePointEnd(text, max)), cut: text.length > max }):19—const flat = String(text).toWellFormed()at:20, thenflat.slice(0, codePointEnd(flat, max))andflat.length > maxThis tree adds three lines above
cut(the body is now a block), socutreads:19against:16on the base; nothing below it in the file moved.#1959and#1964left this file alone, so both numbers are exact.Behaviour change
Well-formed input is unchanged, byte for byte.
toWellFormed()is the identity on a string with no unpaired surrogate, so the prefix and thecutflag are whatslice+codePointEndproduced before. That is pinned twice: the new test'swell-formed input keeps its exact prefix and flagcase (eight literal pairs —''@5,abcdef@3,abc@3,abc@0,a😀@3,aa😀@3 →'aa',👨👩👧@4 →'👨','a'×2000@2000), and a 20,000-case fuzz that ran the old and newcutover random well-formed strings (ASCII,é, CJK, emoji, ZWJ family, flag pairs) and compared both thetextand thecutflag: 20,000 / 20,000 identical.Already-broken input changes, one unit for one unit. An unpaired surrogate before or at the bound is replaced by U+FFFD before the bound is computed, so the repair is in the prefix rather than after it. U+FFFD is one UTF-16 unit for the unit it replaces, so
.lengthand.cutare unchanged — the fix cannot move a budget. On a separate 5,000-case run of strings that carried a lone surrogate, all 5,000 came back well formed; the base implementation repaired none of them.state.json, before and after (cut('a\uD800b', 300), the string a card field would carry):The base file holds the escaped lone surrogate;
JSON.stringifywrites it as"a\ud800b"andJSON.parsehands the same lone surrogate back, so the bad value survived every reload and only became U+FFFD when some UTF-8 writer touched it — between those two points it was a wrong string in memory. This tree stores the replacement character instead. The well-formed controlcut('ok😀', 3)is"ok",cut=true, on both sides.Nothing else moves. No signature, constant, budget or record shape changes;
String(text)does widen the non-string case: on basecut(123, 2)throwsTypeError: text.charCodeAt is not a function, while this tree coerces ('12',cut: true). All 54cut(call sites in the tree pass a string — the two that read state (task.note || 'Completed'andtask.summary) can only hold strings — so nothing reachable changes; a future caller that forgot would store"undefined"or[object Object]instead of failing.String.prototype.toWellFormed(ES2024) is not new to the client bundle: basecore/client.js:1730already calls it incleanPeerName(core/src/shared/remote-label.js:6). This tree does not raise the browser floor — it only widens where the floor applies, becausecutnow reaches card fields, status stubs and presence lines too. The floor stays Chromium 111+ / Electron 24+ / Safari 16.4 / Firefox 119.Base and rebase
The branch was cut from
4accf1e2, rebased ontobeba27a7(the merge of#1964), not merged. The two pull requests touchdocs/subsystems/core-host.mdin different sections — this one changes thecore/src/host/text.jsrow of the## Key filestable (:41on both sides, in place),#1964adds a sentence inside the### Inbound shared roomsbullet (:329) — so the rebase applied with no conflict.npm run buildon the new tree produced no diff: the client's own sources did not change under#1964, so thecore/client.jsin this tree is what the current sources build to. The newchangelog.d/1963-cut-wellformed.fixed.mddoes not collide with any fragment#1964left behind.Red first
Four cases in
core/test/text-boundary.test.js; three of them red on the base blob, the fourth (well-formed input keeps its exact prefix and flag) green on both sides by design — it is the regression anchor for "nothing that was fine may move". Run against the base file (git show beba27a7:core/src/shared/text-boundary.js > core/src/shared/text-boundary.js, this branch's test file):Case 3 is the one that pins where the repair lands rather than that it happens: the input is
ab\uD800cd(five units) with a bound of four, so the unpaired high surrogate sits inside the prefix that is kept, next to the boundary. The base answersab\ud800c— a four-unit prefix that still carries the lone half — and this tree answersab\uFFFDc: the same four units, with the replacement character in the slot the bad unit occupied. Both are four units and both answercut=true, so the case is about the content of the kept prefix, not about a length change (U+FFFD is 1:1, and a budget must not move). With the fixed helper all four cases pass.Mutation
One edit — drop
.toWellFormed(), keep the new block form — and rerun the same file:const flat = String(text)(no flatten)# pass 1 / # fail 3, and the three failures are exactly cases 1–3; the anchor stays greenSo the two obligations are held by different cases: flattening is what cases 1–3 need, and not disturbing good input is what the anchor needs. Neither can be satisfied by the other.
Why
core/client.jsandpatches/ds-bot/0008-client-bundle.patchare in the 8 filescore/src/shared/text-boundary.jsis bundled into the single-file client (core/client.js), and the mirror serie carries that bundle aspatches/ds-bot/0008-client-bundle.patch. Both are generated, not hand-written:npm --prefix core run buildrewritescore/client.js(its only change here is the four-linecutbody) andnode scripts/ds-bot-patches.mjs generaterewrites the patch. The twopatches/ds-bot/files/entries are the byte-identical mirrors of the twocore/files this PR changes —cmpclean on both.Docs-not-needed:
core/client.jsis a generated bundle — its only change here is the shared text bound, whichdocs/subsystems/core-host.mddocuments; no client behaviour changed.Verification
TZ=UTC ARCH_DOCS_BASE=beba27a77e9ca453a1c357fff497c6a500d4be0e npm run verify→ exit 0, on a clean tree (git status --porcelainempty) at this tip's tree28acd91c…, run once after the rebase. Counts, this tree / basebeba27a7:beba27a7.github/scriptsThe base core count is measured, not inferred: a clean worktree at
beba27a7with the same harness runs# tests 2362 / # pass 2362 / # fail 0, so this PR adds exactly the four cases above. The three plugin suites,.github/scripts, interop and live-e2e have no file in this diff and are the same commits on both sides; on this machine each of social/tree/timeline reports one skipped case because that plugin is not installed here (540/1, 126/1, 134/1, measured), while CI runs them installed and reported 541/541, 127/127, 135/135 with 0 skipped on the previous tips.arch-docs: OK (8 changed file(s))againstARCH_DOCS_BASE=beba27a7…— the same eight files as the diff: twocore/src+testchanges, their threepatches/ds-bot/mirrors, the doc and the changelog fragmentbuild-client --checkok (the bundle matches the sources)ds-bot-patches.mjs check: pinFei2411/DeepSeek-Bot@2b10c70, 10 patches apply cleanly in order,core/=ds-bot/+ patches;canonical: whatgeneratewrites (a no-op);generatethis round wrote nothing (tree unchanged);cmpbyte-identical onpatches/ds-bot/files/src/shared/text-boundary.jsandpatches/ds-bot/files/test/text-boundary.test.jsnode .github/scripts/changelog.mjs check→6 fragment(s) in changelog.d/ are well formedNot run: the smoke test — this checkout has no
dev/bench.shand no running FlyBot instance to drive. Not run: the CIpackagejob, typecheck and actionlint. Also not asserted here: that the Electron/Chromium build a client ships on meets the floor that API needs (Chromium 111+ / Electron 24+ / Safari 16.4 / Firefox 119) — that floor is already on base throughcleanPeerName, and this checkout cannot drive a browser; the bundle is generated fromcore/src/shared/text-boundary.js, so it ships exactly what the node tests exercise.Checkpoint
{
"pr": 1966, "claim": "Dev-2", "issue": 1963,
"summary": "cut() in core/src/shared/text-boundary.js now flattens its input with String(text).toWellFormed() before it bounds, so an unpaired surrogate the caller already carried is stored as U+FFFD instead of passing through to state.json. Well-formed input is byte-identical: eight literal pairs and a 20,000-case fuzz against the old implementation agree on both text and flag. U+FFFD is one unit per unit, so length and the cut flag do not move and no budget changes. The new tests are three red-first cases plus one regression anchor, and dropping the flatten call reds exactly the three.",
"files": ["core/src/shared/text-boundary.js", "core/test/text-boundary.test.js", "core/client.js", "docs/subsystems/core-host.md", "changelog.d/1963-cut-wellformed.fixed.md", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/shared/text-boundary.js", "patches/ds-bot/files/test/text-boundary.test.js"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=beba27a77e9ca453a1c357fff497c6a500d4be0e npm run verify; node scripts/ds-bot-patches.mjs check; node .github/scripts/changelog.mjs check", "result": "green", "at": "a8a5ad426f35c9ebe6093f23ad2fcda47db4549a" },
"next": "none — ready for review"
}