Skip to content

fix(room-share): count braces outside JSON strings only - #2532

Closed
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:delivery1/main-room-share-brace-in-text
Closed

fix(room-share): count braces outside JSON strings only#2532
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:delivery1/main-room-share-brace-in-text

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #2447

Owner: Delivery-1
Reviewer: Birfy
Written-by: Delivery-1
Base: 90a5944
Risk tier: T3

What changed

parseRoomShare found the end of the JSON envelope by counting every { and }, so a brace inside a chat line's text — which JSON.stringify leaves raw — was read as structure. One unbalanced brace closed the object early, JSON.parse failed, and the whole envelope was dropped: the host's ingestRemoteRoomLine/applyInboundRoomShare never saw a line, and a join that copied such a line lost its history.

The scan now skips string literals, honouring backslash escapes, so if (x) { y() and a lone } in a message are data again. An envelope that really is cut short is still refused.

Verification at the tip

Tip SHA: 375b3be

Ran:

  • red first on a clean 90a594471b5886737b433e116f6397aa8ca2b6f2 tree carrying only core/test/room-share-brace-in-text.test.js: # tests 4 / # pass 2 / # fail 2 — the open-brace and close-brace cases both failed with the envelope was dropped; the escaped-quote case and the truncated-envelope control passed on the base by design.
  • after the fix, that suite: # tests 4 / # pass 4 / # fail 0
  • full core suite, TZ=UTC node --import ./test/harness-resolve.mjs --test "test/*.test.js": # tests 2931 / # pass 2931 / # fail 0
  • node scripts/build-client.mjs --check: exit 0
  • node scripts/ds-bot-patches.mjs check: ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/
  • node scripts/ds-bot-patches.mjs canonical: ok patches/ds-bot/ is what generate writes (generate is a no-op)
  • npm run lint: 81 warnings, 0 errors
  • node .github/scripts/changelog.mjs check: 36 fragment(s) in changelog.d/ are well formed
  • ARCH_DOCS_BASE=90a59447 node scripts/check-arch-docs.mjs: OK (5 changed file(s))

The two patches/ds-bot/files/** entries are whole-file mirrors written by generate.

Not run: the smoke test (no index.js or src/host behaviour change) and CI itself.

Docs-not-needed: no owned path, module or contract changes; the parser now reads the envelope the envelope itself describes.

Checkpoint

{
  "pr": 2532,
  "claim": "Delivery-1",
  "issue": 2447,
  "summary": "parseRoomShare found the envelope's end by counting every brace, so a brace in a chat line's text ended the object early, JSON.parse failed and the whole line was dropped. The scan now skips JSON string literals with backslash escapes honoured, so an unbalanced brace is data again; a truncated envelope is still refused. Red first at 90a59447: 2 of 4 new tests failed with 'the envelope was dropped'; the escaped-quote case and the truncated control passed on the base.",
  "files": [
    "core/src/shared/room-share.js",
    "core/test/room-share-brace-in-text.test.js",
    "patches/ds-bot/files/src/shared/room-share.js",
    "patches/ds-bot/files/test/room-share-brace-in-text.test.js",
    "changelog.d/2447-room-share-brace-in-text.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC node --import ./test/harness-resolve.mjs --test \"test/*.test.js\"; node scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical; npm run lint",
    "result": "green",
    "at": "375b3bed22d1481da6be6aceb85cf01e99bd7b7e"
  },
  "next": "ready for the Verdict at 375b3bed22d1481da6be6aceb85cf01e99bd7b7e"
}

@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Written-by: Delivery-1
Owner: Delivery-1
Reviewer: Birfy

Requesting the Verdict at 375b3bed22d1481da6be6aceb85cf01e99bd7b7e — base 90a594471b5886737b433e116f6397aa8ca2b6f2, 1 commit, 5 files +93/−2, maintainer_can_modify: true.

What changed: parseRoomShare counted every { and } to find the envelope's end, so a brace a person typed in a chat line (JSON.stringify leaves it raw) closed the object early, JSON.parse failed, and the host dropped the whole line — ingestRemoteRoomLine / applyInboundRoomShare never saw it, and a join copying such a line lost its history. The scan now skips string literals with backslash escapes honoured, so one unbalanced brace is data again; a truncated envelope is still refused.

Red first, on a clean 90a59447 tree carrying only core/test/room-share-brace-in-text.test.js — # tests 4 / # pass 2 / # fail 2:

  1. an open brace in a line does not end the envelope early — the envelope was dropped
  2. a close brace in a line does not end the envelope early — the envelope was dropped

Controls that passed on the base by design: a brace inside a quoted word with an escaped quote and a backslash, and an envelope that really is cut short (still undefined).

After the fix: that suite 4/4; full core suite # tests 2931 / # pass 2931 / # fail 0.

Gates at this tip, verbatim:

  • node scripts/build-client.mjs --check → exit 0
  • node scripts/ds-bot-patches.mjs check → ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
  • node scripts/ds-bot-patches.mjs canonical → ok patches/ds-bot/ is what generate writes (generate is a no-op)

Also green at this tip: npm run lint 81 warnings / 0 errors; changelog.mjs check 36 fragments; ARCH_DOCS_BASE=90a59447 arch-docs OK (5 changed files). Tier: risk-tier: T3; basis: core/src/shared/room-share.js=T3, …. The two patches/ds-bot/files/** entries are whole-file mirrors written by generate.

@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Written-by: Delivery-1
Owner: Delivery-1

Closing as a duplicate: #2529 is the earlier pull request for the same defect, so this one is withdrawn.

  • same issue: Fixes #2447
  • same place: the bracket scan in core/src/shared/room-share.js and the same pair of mirrors under patches/ds-bot/files/
  • #2529 head 7c07f46cacf6dba512d501d0bf9cfc1ed0289bbf, opened earlier
  • this pull request head 375b3bed22d1481da6be6aceb85cf01e99bd7b7e

The earlier pull request is kept; no replacement pull request will be opened for this defect.

@HugoXK HugoXK closed this Nov 5, 2026
@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #2529.

The work here is not lost or replayed under another number: room-share.js keeps exactly one open PR, #2529. Nothing in this PR is being pushed or reopened elsewhere.

Written-by: FlyBot Lead

@zy95-12
fix(room-share): count braces outside JSON strings only
375b3be 
parseRoomShare found the end of the envelope by counting every `{` and `}`, so
a brace a person typed in a chat line closed the object early, JSON.parse
failed and the whole line was dropped. An open brace in `if (x) { y()`, or a
lone `}` at the end of a dict, lost the message.

Skip string literals while scanning, with backslash escapes honoured, so the
braces JSON.stringify left raw inside `text` stay data.

Fixes ThinkFlowLab#2447

Docs-not-needed: no owned path or contract changes; the parser now reads the
envelope the envelope itself describes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

room-share: one brace in a chat line's text drops the whole envelope

2 participants

@HugoXK @zy95-12