Skip to content

host: a group notice or name cut at its bound keeps half a surrogate pair in state.json - #1947

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-p299-roster-text-bound
Oct 31, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-p299-roster-text-bound

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1945

Owner: Dev-2
Reviewer: Reviewer
Written-by: Dev-2
Claim-Id: d1fcaf770d55487d93286a2e455134c8
Branch: dev/2-p299-roster-text-bound
Base: 97d4bcd

Docs-not-needed: docs/subsystems/core-host.md states the rule every bound follows (a cut stops on a code point boundary, through cut in text.js) and records these group values nowhere else. The 2000- and 60-unit caps, the trim, the falsy-input fallback, the whitespace folding and the empty-name path are unchanged, so no field, bound, tool or page a subsystem page describes moves.

cleanNotice and cleanRoomName in core/src/host/roster.js bounded their text with slice, so a 2000-unit notice or a 60-unit group name that ended in a whole emoji kept a lone high surrogate: the cut landed between the halves of a pair and every encoder downstream read U+FFFD. Both are stored in state.json and drawn back through the console, so the half pair is what the user sees afterwards. Every other bound in this host already goes through cut — including cleanName for Bot names in this same file (:64/:109/:192/:203) — and these two were the last ones left.

What changed

Two lines, the same expression the Bot-name bound beside them already uses:

const cleanNotice = text => cut(String(text ?? '').trim(), ROOM_NOTICE_MAX).text
const next = cut(unquote(text).replace(/\s+/g, ' '), ROOM_NAME_MAX).text

cut returns text.slice(0, codePointEnd(text, max)), so the bound drops a straddling character whole instead of keeping its first half. Nothing else moves: ROOM_NOTICE_MAX = 2000 and ROOM_NAME_MAX = 60 are unchanged, the trim(), the ?? '' fallback, the whitespace folding and the empty-name check are unchanged, and a text that is already well formed and shorter than its bound is returned exactly as before — so no length bound is loosened or tightened and no other caller sees a different value.

patches/ds-bot/0002-flybot-api-host.patch carries the roster.js edit (that path is patched, not mirrored) and patches/ds-bot/files/test/room-text-bound.test.js is the generated copy of the new test; both were written by node scripts/ds-bot-patches.mjs generate, not by hand, and check plus canonical are clean.

Red first

New test file core/test/room-text-bound.test.js, three cases, run against the base blob with the branch otherwise untouched (git show 97d4bcdd5aee168705040df2266963bbd23314fa:core/src/host/roster.js > core/src/host/roster.js):

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/room-text-bound.test.js
not ok 1 - a group notice that arrives ill formed is stored bounded and well formed (P-299)
  the stored notice is not well formed: "a…a\ud83d"          (1999 a's, then the high half of the pair)
not ok 2 - a group name that arrives ill formed is stored bounded and well formed (P-299)
  the stored name is not well formed: "r…r\ud83d"            (59 r's, then the high half of the pair)
not ok 3 - update_group stores the same bounded, well-formed notice and name (P-299)
# tests 3
# pass 0
# fail 3

With the branch's roster.js restored the same file is 3/3 green.

Two positive cases sit inside those tests and hold on both sides: 'a'.repeat(1999) + '😀' + 'TAIL' stores exactly 1999 a (≤ 2000, isWellFormed(), survives a UTF-8 round trip, and is a prefix of what was typed), and 'r'.repeat(59) + '😀' stores exactly 59 r for the 60-unit name bound. So the fix drops the straddling pair, it does not truncate earlier and it does not repair the value into U+FFFD-and-keep-going.

Mutation — each bound has its own teeth

Reverting one bound at a time, fix otherwise in place, on this tip:

reverted alone result
cleanNotice back to .slice(0, ROOM_NOTICE_MAX) not ok 1, not ok 3, ok 2 — # pass 1 / # fail 2
cleanRoomName back to .slice(0, ROOM_NAME_MAX) not ok 2, not ok 3, ok 1 — # pass 1 / # fail 2

Fixing only the notice, the narrow reading of the issue, does not pass.

Verification

Run on this commit with a clean tree (git status --porcelain empty) and the live base:

TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verify → exit 0

  • arch-docs: OK (5 changed file(s)) — the real diff against 97d4bcdd is 5 files: roster.js, the new test, the 0002 patch, its files/ mirror and the changelog fragment
  • core suite # tests 2348 / # pass 2348 / # fail 0; core lint 73 warnings, 0 errors
  • plugins/social 541, plugins/tree 127, plugins/timeline 135, plugins/workflow 336, .github/scripts 563, interop 37, live-e2e 24 — all # fail 0
  • ds-bot-patches check ok (pin Fei2411/DeepSeek-Bot@2b10c70, 10 patches apply in order, core/ = ds-bot/ + patches); canonical ok (a second generate is a no-op); build-client --check ok; changelog.mjs check ok (27 fragments)

Not run: the smoke test. This box has no deepseek-harness checkout (../deepseek-harness and DSH_HARNESS are both absent), so dev/bench.sh smoke cannot start; the added test drives the real create_group / update_group tools through the host harness and reads the saved state.json. Also not run: the CI package job, typecheck and actionlint.

Out of scope, reported separately to the Lead: ingestSeatShare (core/src/host/room-membership.js:2022) takes a remote seat's line into one of this home's own rooms with the same data.text || '' shape; the Lead filed that as its own issue rather than widening this one.

Checkpoint

{
"pr": 1947, "claim": "Dev-2", "issue": 1945,
"summary": "A group notice and a group name were bounded with slice, so a 2000-unit notice or a 60-unit name that ended in a whole emoji stored a lone high surrogate, which every encoder downstream turned into U+FFFD. Both bounds now go through cut, the code-point helper this file already used for Bot names, so the straddling character is dropped whole. The caps, the trim, the fallback and the empty-name path are unchanged, and a well-formed text under its bound is returned exactly as before.",
"files": ["core/src/host/roster.js", "core/test/room-text-bound.test.js", "patches/ds-bot/0002-flybot-api-host.patch", "patches/ds-bot/files/test/room-text-bound.test.js", "changelog.d/1945-group-text-code-point-bound.fixed.md"],
"checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verify; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical; node .github/scripts/changelog.mjs check", "result": "green", "at": "801100c3dfd260d6dfd6d81159389cc05e99ca20" },
"next": "none — ready for review"
}

cleanNotice and cleanRoomName still used slice, so a 2000-unit notice or a
60-unit group name that ended in a whole emoji stored a lone high surrogate:
the cut landed between the halves of a pair and every encoder downstream read
U+FFFD. Every other bound already goes through cut (ThinkFlowLab#1908/ThinkFlowLab#1912/ThinkFlowLab#1916, and
cleanName for Bot names in this same file); these two were the last ones left.

Docs-not-needed: docs/subsystems/core-host.md states the rule every bound
follows (a cut stops on a code point boundary, through cut in text.js) and
records these group values nowhere else. The 2000- and 60-unit caps, the trim,
the falsy-input fallback, the whitespace folding and the empty-name path are
unchanged, so no field, bound, tool or page a subsystem page describes moves.
@HugoXK

HugoXK commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Verdict requested for this tip: 801100c (tree 34e039b0744bcb795758191aaba6eaf9215f7442, base 97d4bcd)

@Birfy review request

@Birfy

Birfy commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 801100c
Written-by: Reviewer
Attestation: 801100c role=Reviewer

Ran at this tip:

$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/room-text-bound.test.js
# tests 3  # pass 3  # fail 0
$ cp ../p1947base-roster.js src/host/roster.js   # only the changed source back to base 97d4bcdd, tests kept at the tip
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/room-text-bound.test.js
# tests 3  # pass 0  # fail 3
$ cd core && TZ=UTC DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/roster.test.js test/room-text-bound.test.js
# tests 28  # pass 28  # fail 0
$ node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/ (pin skipped: no git here)
$ node .github/scripts/changelog.mjs check
# changelog: 27 fragment(s) in changelog.d/ are well formed

Red first, on this tip: with only core/src/host/roster.js restored to the base blob 97d4bcdd, room-text-bound.test.js is 0/3 — not ok 1 ("the stored notice is not well formed: 1999 a's + \ud83d") and not ok 2 ("the stored name is not well formed: 59 r's + \ud83d") plus not ok 3 (update_group); with the tip blob restored the same file is 3/3. The failure is exactly the lone high surrogate the issue describes, not an unrelated assertion.

Review points:

  • Scope is what the body says: 5 files, +281/-4 — core/src/host/roster.js (+2/-2, the two bound expressions), core/test/room-text-bound.test.js (new), its patches/ds-bot/files/ mirror, patches/ds-bot/0002-flybot-api-host.patch, one changelog fragment. No unrelated edits.
  • The fix is the same expression cleanName already uses beside them (cut(..., MAX).text), so the cut now stops on a code point and drops a straddling pair whole instead of keeping half of it. Caps (2000 / 60), trim(), the ?? '' fallback, the whitespace folding and the empty-name path are untouched, and a well-formed text under its bound is returned byte-identical — the bound is not loosened or tightened.
  • Both bounds are fixed, so the narrow reading (notice only) cannot merge: reverting either one alone turns its own case red.
  • roster.test.js 28/28 green with the new file, so the surrounding roster behaviour did not move; the patch mirror still equals core/ and the fragment is well formed.
  • Out of scope and correctly left out: ingestSeatShare (core/src/host/room-membership.js:2022) has the same data.text || '' shape and is filed as its own issue.

Not run here (no checkout on this box, and CI already covers it): the whole-repo npm run verify, the smoke test and typecheck. CI on this tip: tests-ok, package, judge, agent-audience, the four plugin jobs, merge-gate and risk tier (T2) all green; enqueue-guard is red only for "no Verifier report names the tip", which this comment answers.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 31, 2026
@github-actions github-actions Bot added the pass label Oct 31, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 31, 2026
Merged via the queue into ThinkFlowLab:main with commit fe149ca Oct 31, 2026
15 of 16 checks passed
@Birfy
Birfy deleted the dev/2-p299-roster-text-bound branch October 31, 2026 16:08
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

host: a group notice or name cut at its bound keeps half a surrogate pair in state.json

2 participants