Skip to content

Commit ae7ea46

Browse files
authored
Cut the five text bounds in turns.js on code points, not UTF-16 units (#1985)
Fixes #1984 Owner: Dev-3 Written-by: Dev-3 Reviewer: Birfy Claim-Id: ac124b1661744f348c1a09cabe4a35ca Tip: 3837d5c Base: 5b72400 Tree: 22f824b ## What changed Five places in `core/src/host/turns.js` bounded text with a bare `String.prototype.slice`, which cuts on UTF-16 units: `turnFailure` (:36, 500), `failureLine` (:45, 200), the owner-visible step line (:303, 79 + `…`), an approval card's description and command (:363, 300 / 400) and the reason a failed turn leaves in the inbox (:405, 160). A bound that landed between the halves of a surrogate pair stored `U+D83D` at the tail — `U+FFFD` once encoded — in state, in the line the owner reads, and in the reason the next turn sees. Each now goes through `cut(text, max)` from `core/src/shared/text-boundary.js` (`host/text.js` already re-exports it, so the existing `textOf` import gained a name). No budget moved: 500, 200, 79, 300, 400, 160 are unchanged. The step line still renders at 80 columns; its `…` is now appended only when `cut` actually dropped something (`cut().cut`) instead of being guessed from `line.length > 80`, so a line that measures 80 but loses a pair no longer claims to be whole. `:630` is the same field as `:45` cut a second time and is untouched, as is everything else in the file. Three tests, one per reading path: `diagnostics.test.js` drives the exported `turnFailure` / `failureLine`; `group-status.test.js` drives the real route (`openTurn` + `tool/call` + `askApproval`, then reads `state.work.bots.b.step` and `state.work.approvals[0].detail`); `dead-turn.test.js` drives `turn/end` and reads `state.work.ended[0].failed`. Each asserts the same three things — `isWellFormed()`, a code-point prefix of the input, and the unchanged length budget — and each carries a control case. `docs/subsystems/core-host.md` (owned by `core/src/host/`) gains one sentence naming the five bounds; `changelog.d/1984-turns-code-point-bounds.fixed.md` is the fixed entry. ## Red first (base `5b72400c`, source untouched, tests applied only) Inputs put the cut exactly on the high surrogate (`'a'.repeat(N-1) + '😀' + 'tail'`, N = the site's budget), driven through the real functions and routes, never a private helper: ``` diagnostics.test.js # tests 18 # pass 16 # fail 2 not ok 12 - a failure message cut at its 500-unit bound keeps the pair whole the message is not well formed: "aaa\ud83d" not ok 13 - a failure line cut at its 200-unit message bound keeps the pair whole the line is not well formed: "aaa\ud83d" group-status.test.js # tests 25 # pass 23 # fail 2 not ok 2 - an approval card cuts its description and command at their bounds without splitting a pair the description is not well formed: "aaa\ud83d" not ok 20 - a step line cut at its bound keeps the pair whole, and the ellipsis only shows when it cut the step is not well formed: "aa\ud83d…" dead-turn.test.js # tests 6 # pass 5 # fail 1 not ok 3 - a failed turn records its reason cut at 160 units without splitting a pair the stored reason is not well formed: "aaa\ud83d" ``` Five failures, five sites. With the source fix applied and nothing else changed the same three files are `18/18`, `25/25`, `6/6`. ## Reverting one fix at a time (base `5b72400c`, fix applied except the site shown) | site | what was reverted | red | | --- | --- | --- | | 1 | `:36` → `error.message.trim().slice(0, 500)` | `diagnostics` not ok 12 only | | 2 | `:45` → `failure.message.slice(0, 200)` | `diagnostics` not ok 13 only | | 3 | `:303` → `{ text: line.slice(0, 79), cut: line.length > 79 }` | `group-status` not ok 20 only | | 4 | `:363` → `text.trim().slice(0, 300)` | `group-status` not ok 2 only | | 5 | `:405` → `(failure.message \|\| failure.code).slice(0, 160)` | `dead-turn` not ok 3 only | Each revert reddens exactly its own case and nothing else, so no case passes on another site's fix. ## Verification at the tip Head SHA: `3837d5c72c60f11a323a946ffa2376a933e4b2f8` — tree `22f824b4343ac2526c7c31d1bb8e13c6b689d5f9`. The run below happened on that same tree at `d8c7d6d85cbf0e3b89414d3f91d179cefcc55051` (the pre-push commit, clean working tree); the push helper rebuilt the commit on base `5b72400c` and proved the tree identical (`tree ok 22f824b (9 paths)`), so the pushed head carries exactly what was verified. Ran: ``` node scripts/ds-bot-patches.mjs generate # rewrote the series from core/ (0001, 0007, files/test/dead-turn.test.js) node scripts/ds-bot-patches.mjs check # 3 lines ok: pin, 10 patches apply in order, core = ds-bot + patches node scripts/ds-bot-patches.mjs canonical # ok — generate is a no-op node .github/scripts/changelog.mjs check # 13 fragments well formed TZ=UTC ARCH_DOCS_BASE=5b72400c6fef51c9a2145b78de9530c338853b74 npm run verify # exit 0 ``` `verify` at that tip: core **2387/2387** (`base 5b72400` measures **2382**, so this branch is exactly +5 tests), social 541, tree 127, timeline 135, workflow 336, `.github/scripts` 563, interop 37, live-e2e 24 — 0 failures anywhere. Core lint 73 warnings / 0 errors; plugin lint 36 / 9 / 2 / 16 warnings, 0 errors; `arch-docs: OK (9 changed file(s))`. Not run (and why): - the smoke test: it needs a `../deepseek-harness` checkout or `DSH_HARNESS`, and this machine has none. The change is confined to bounds inside `src/host/turns.js`; nothing in the module list or prompt path changes shape. - CI's `package` job (pack check, typecheck, actionlint): not part of `npm run verify`. - Windows: this branch touches no path handling. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1985, "claim": "Dev-3", "issue": 1984, "summary": "The five text bounds in core/src/host/turns.js now cut through the shared cut helper instead of a bare slice. A bound that landed inside a surrogate pair used to leave a lone high surrogate in state, in the owner-visible step line and in a failed turn's stored reason. Each site keeps its original budget and now yields a well-formed code-point prefix of its input. The step line's ellipsis follows the helper's cut flag rather than a length guess.", "files": ["core/src/host/turns.js", "core/test/group-status.test.js", "core/test/diagnostics.test.js", "core/test/dead-turn.test.js", "changelog.d/1984-turns-code-point-bounds.fixed.md"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=5b72400c npm run verify (exit 0, core 2387/2387)", "result": "green", "at": "3837d5c72c60f11a323a946ffa2376a933e4b2f8" }, "next": "none — ready for review at this tip" } ``` <!-- /flybot:checkpoint -->
1 parent 5b72400 commit ae7ea46

9 files changed

Lines changed: 211 additions & 21 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
The host's turn lines no longer split a surrogate pair at their character bounds ([#1984](https://github.com/ThinkFlowLab/FlyBot/issues/1984)): a failed turn's 500-unit message, the 200-unit message of its one-line reason, the 79-unit step a running call shows, an approval card's 300-unit description and 400-unit command, and the 160-unit reason a failed turn leaves for the inbox now cut on a code point boundary, so a line that ends on an emoji keeps the whole emoji instead of an unpaired half the encoder turns into U+FFFD.

‎core/src/host/turns.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// Turn tracking: which turns were triggered by another Bot, and what they said.
55
import { REFERENCE_KIND, REPLY_KIND, SOURCE_KIND } from './constants.js'
66
import { unwrapUntrusted } from './delivery.js'
7-
import { textOf } from './text.js'
7+
import { cut, textOf } from './text.js'
88
import { reminderText } from './schedules.js'
99
import { OTHER, combine, originOfSource, settled } from './turn-origin.js'
1010

@@ -33,7 +33,7 @@ export function isRemoteFailureNotice(text) {
3333
// error row, an AUTH failure loses its message: providers may echo part of the key.
3434
export function turnFailure(error) {
3535
const code = typeof error?.code === 'string' && error.code !== '' ? error.code : 'UNKNOWN'
36-
const message = code === 'AUTH' || typeof error?.message !== 'string' ? '' : error.message.trim().slice(0, 500)
36+
const message = code === 'AUTH' || typeof error?.message !== 'string' ? '' : cut(error.message.trim(), 500).text
3737
return {
3838
code,
3939
message,
@@ -42,7 +42,7 @@ export function turnFailure(error) {
4242
}
4343
}
4444

45-
export const failureLine = failure => `${failure.code}${failure.status ? ` ${failure.status}` : ''}${failure.message ? `: ${failure.message.slice(0, 200)}` : ''}`
45+
export const failureLine = failure => `${failure.code}${failure.status ? ` ${failure.status}` : ''}${failure.message ? `: ${cut(failure.message, 200).text}` : ''}`
4646

4747
// A group's own Session never reaches a model, so the snapshot DSH prepared there
4848
// rides with the user's queued message to the members who answer it.
@@ -300,7 +300,9 @@ export function install(rt) {
300300
const value = ['command', 'cmd', 'pattern', 'query', 'url', 'file_path', 'path', 'description']
301301
.map(key => args?.[key]).find(item => typeof item === 'string' && item.trim() !== '')
302302
const line = value?.replace(/\s+/g, ' ').trim()
303-
return line && line.length > 80 ? `${line.slice(0, 79)}…` : line
303+
if (!line) return line
304+
const bound = cut(line, 79)
305+
return bound.cut ? `${bound.text}…` : line
304306
}
305307

306308
// Work a group member hands a Bot with message_bot runs in a hidden turn of the Bot's own chat,
@@ -360,7 +362,7 @@ export function install(rt) {
360362
const first = keys => keys.map(key => args?.[key]).find(value => typeof value === 'string' && value.trim() !== '')
361363
const text = first(['description', 'justification'])
362364
const code = first(['command', 'cmd', 'file_path', 'path', 'url', 'query'])
363-
return { ...(text ? { text: text.trim().slice(0, 300) } : {}), ...(code ? { code: code.trim().slice(0, 400) } : {}) }
365+
return { ...(text ? { text: cut(text.trim(), 300).text } : {}), ...(code ? { code: cut(code.trim(), 400).text } : {}) }
364366
}
365367

366368
// The latest turns of the user's own Bots that finished or failed, for a system notification
@@ -402,7 +404,7 @@ export function install(rt) {
402404
...(owner ? { roomId: owner.roomId } : { sessionId: id }),
403405
at,
404406
...(calls > 0 ? { calls } : {}),
405-
...(failure ? { failed: (failure.message || failure.code).slice(0, 160) } : {}),
407+
...(failure ? { failed: cut(failure.message || failure.code, 160).text } : {}),
406408
...(rateLimited !== undefined ? { rateLimited } : {}),
407409
...(silent ? { silent: true, streak } : {}),
408410
}], at)

‎core/test/dead-turn.test.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ test('#1044: a turn that wrote text then failed adds no line', async (t) => {
101101
assert.equal(failureLines({ ended: await host.ended() }, 'b', key => key).length, 0)
102102
})
103103

104+
// The reason a failed turn leaves for the inbox is cut at 160 units: a bound inside a surrogate pair
105+
// leaves a lone high half, which the stored record keeps and every client draws as U+FFFD.
106+
test('a failed turn records its reason cut at 160 units without splitting a pair', async (t) => {
107+
const host = await start()
108+
t.after(host.stop)
109+
const message = `${'a'.repeat(159)}😀tail`
110+
die(host.emit, 'b', 1, { code: 'SERVER', message })
111+
const [entry] = await host.ended()
112+
assert.equal(entry.failed.isWellFormed(), true, `the stored reason is not well formed: ${JSON.stringify(entry.failed.slice(-4))}`)
113+
assert.equal(entry.failed, message.slice(0, 159))
114+
assert.ok(entry.failed.length <= 160, `the 160-unit budget moved: ${entry.failed.length}`)
115+
})
116+
104117
test('#1044: three dead turns in a row are three lines and the streak is visible', async (t) => {
105118
const host = await start()
106119
t.after(host.stop)

‎core/test/diagnostics.test.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,29 @@ test('turnFailure drops AUTH messages and keeps the facts', () => {
234234
assert.equal(failureLine({ code: 'SERVER', status: 503, message: 'busy' }), 'SERVER 503: busy')
235235
})
236236

237+
// A bound that lands between the halves of a surrogate pair keeps the high half alone, and that lone
238+
// surrogate is what the record carries and every renderer draws as U+FFFD. The five bounds in
239+
// turns.js go through `cut()`; each has its own case, in the file that reads its value.
240+
test('a failure message cut at its 500-unit bound keeps the pair whole', () => {
241+
const message = `${'a'.repeat(499)}😀tail`
242+
const kept = turnFailure({ code: 'SERVER', message }).message
243+
assert.equal(kept.isWellFormed(), true, `the message is not well formed: ${JSON.stringify(kept.slice(-4))}`)
244+
// The emoji needs two units and only one is left inside the bound, so the prefix stops before it.
245+
assert.equal(kept, message.slice(0, 499))
246+
assert.ok(kept.length <= 500, `the 500-unit budget moved: ${kept.length}`)
247+
// A pair that ends exactly on the bound stays whole.
248+
const exact = `${'a'.repeat(498)}😀`
249+
assert.deepEqual(turnFailure({ code: 'SERVER', message: exact }).message, exact)
250+
})
251+
252+
test('a failure line cut at its 200-unit message bound keeps the pair whole', () => {
253+
const line = failureLine({ code: 'SERVER', status: 503, message: `${'a'.repeat(199)}😀tail` })
254+
assert.equal(line.isWellFormed(), true, `the line is not well formed: ${JSON.stringify(line.slice(-4))}`)
255+
assert.equal(line, `SERVER 503: ${'a'.repeat(199)}`)
256+
assert.ok(line.length <= 'SERVER 503: '.length + 200, `the 200-unit budget moved: ${line.length}`)
257+
assert.equal(failureLine({ code: 'SERVER', status: 503, message: 'busy' }), 'SERVER 503: busy')
258+
})
259+
237260
function turnsHost() {
238261
let listener
239262
const delivered = []

‎core/test/group-status.test.js‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,29 @@ test('a group Session\'s approval waits in the team view, frees the round, and t
132132
assert.equal(text(post), '[Group post from Writer]\nCopied the doc to E:/docs.')
133133
})
134134

135+
// Both fields of a card go through a bound of their own: 300 for the description, 400 for the
136+
// command. A cut that splits a surrogate pair leaves a lone high half the card draws as U+FFFD.
137+
test('an approval card cuts its description and command at their bounds without splitting a pair', async (t) => {
138+
const host = await start()
139+
t.after(host.stop)
140+
const result = await openTurn(t, host)
141+
const description = `${'a'.repeat(299)}😀tail`
142+
const command = `${'b'.repeat(399)}😀tail`
143+
host.emit('gb', 'tool/call', { callId: 'k1', name: 'pwsh', arguments: JSON.stringify({ command, description }) })
144+
const outcome = host.askApproval('gb', { callId: 'k1' })
145+
146+
await settle(() => result() !== undefined)
147+
const [approval] = (await host.call('state', {})).value.work.approvals
148+
assert.equal(approval.detail.text.isWellFormed(), true, `the description is not well formed: ${JSON.stringify(approval.detail.text.slice(-4))}`)
149+
assert.equal(approval.detail.text, description.slice(0, 299))
150+
assert.equal(approval.detail.code.isWellFormed(), true, `the command is not well formed: ${JSON.stringify(approval.detail.code.slice(-4))}`)
151+
assert.equal(approval.detail.code, command.slice(0, 399))
152+
assert.ok(approval.detail.text.length <= 300 && approval.detail.code.length <= 400, `a budget moved: ${approval.detail.text.length}/${approval.detail.code.length}`)
153+
154+
assert.equal((await host.call('approval-answer', { id: approval.id, choice: 'allow' })).ok, true)
155+
assert.equal(await outcome, 'allowed-once')
156+
})
157+
135158
test('an own chat\'s approval stays with DSH and is listed while it waits', async (t) => {
136159
const host = await start()
137160
t.after(host.stop)
@@ -447,6 +470,24 @@ test('a running Bot\'s work names its current step, in one short line, until the
447470
assert.equal((await bots()).b, undefined)
448471
})
449472

473+
// The step is 79 units plus an ellipsis, and the cut is decided by whether anything was dropped.
474+
test('a step line cut at its bound keeps the pair whole, and the ellipsis only shows when it cut', async (t) => {
475+
const host = await start()
476+
t.after(host.stop)
477+
await openTurn(t, host)
478+
const bots = async () => (await host.call('state', {})).value.work.bots
479+
host.emit('gb', 'tool/call', { callId: 'k1', name: 'pwsh', arguments: JSON.stringify({ command: `${'a'.repeat(78)}😀tail` }) })
480+
const { step } = (await bots()).b
481+
assert.equal(step.isWellFormed(), true, `the step is not well formed: ${JSON.stringify(step.slice(-4))}`)
482+
// The emoji needs two units and only one is left inside the 79, so the prefix stops before it.
483+
assert.equal(step, `${'a'.repeat(78)}…`)
484+
assert.ok(step.length <= 80, `the 79 + ellipsis budget moved: ${step.length}`)
485+
host.emit('gb', 'tool/result', { callId: 'k1' })
486+
host.emit('gb', 'tool/call', { callId: 'k2', name: 'pwsh', arguments: JSON.stringify({ command: 'npm test' }) })
487+
assert.equal((await bots()).b.step, 'npm test')
488+
host.emit('gb', 'tool/result', { callId: 'k2' })
489+
})
490+
450491
// #626: the shape a Bot actually hits — the owner asks for work, the turn calls `subagent`, and the
451492
// child settles while that same turn is still running. The notice is an account of work the owner
452493
// already asked for, not a new request, so the connector tools stay.

0 commit comments

Comments
 (0)