You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
room-share: keep a cleared notice on a join whose roster had to shrink - #1948
Base: 97d4bcd
Tip SHA: b1babf3 (the pushed head; its parent is that base and its tree is d785ba3aaa08c0e03f711884da467609d803c4c7, so the run below covers it)
Verified commit: a50f9b9d128426506004e07b396bfac6003055d7 (tree d785ba3aaa08c0e03f711884da467609d803c4c7; the pushed head is that same tree, rebuilt onto live main by the push path — the branch carries one replay commit, git log upstream/main..HEAD shows only it and merge-base is 97d4bcd)
Diff: 6 files, +254/−9
What changed
fitJoin in core/src/shared/room-share.js lets the roster go a row at a time first, but it measured the envelope through overWithoutNotice() — the notice deleted, the JSON serialized, the notice restored. That measure leaves out the 12 units a present notice costs on the wire even when it is empty (,"notice":""). So in a room whose roster is near ROSTER_MAX, the roster stopped as soon as the notice-less envelope fitted; restoring the notice pushed the join back over ROOM_SHARE_TEXT_MAX (4,000); the notice branch then had was === '', so the "cut down to nothing" rule (body.notice.length === 0 && was.length > 0) did not apply and the fallback delete body.notice did. A missing notice reads on the guest as "change nothing" (room-membership.jsnoticeOf), so the host's explicit clear never arrived.
The roster's yield now counts the notice field's own cost — noticeKeyCost = ',"notice":""'.length (12) whenever a notice is present — so while (roster.length > 0 && wire-without-notice + 12 > 4,000) stops at a length where the notice field still fits. The roster gives exactly the row that was missing, and the field survives. This is deliberately the field's cost, not the notice text's: the text still pays what is left over in the notice branch below, so #1932/#1934's cut-not-drop case for a long non-empty notice is unchanged (the existing #1932 test, roster kept and notice cut, is green in the run below).
core/test/remote-room.test.js gains two tests:
#1946: a cleared notice rides a near-full join at both ends of the band it used to be dropped — four groups built from ordinary sizes (32 rows, handle 40 / home 20 / name 40 / ownerName 40, five history lines of 1,880, notice: ''). Their place in the band is the length the old loop measured, witnessed independently by the same join encoded with no notice field. 3,988 keeps the notice (3,988 + 12 = 4,000 exactly) with the roster at 20 rows; 3,989, 3,999 and 4,000 are the band that lost it on the base, and each now carries notice: '', guest parseRoomShare(...).notice === '', history dropped and the roster at 19 rows (one more row given, not the list). It also pins both non-empty controls: a long notice is still cut rather than dropped, and a non-empty notice clipped to nothing is still dropped.
#1946: no join in the near-full grid loses the cleared notice — the plan's grid, rows {32, 40, 64} × handle {40, 60, 80, 120} × name {40, 60} × ownerName {40, 60} × history {0, 1, 5, 20} × first-row name 1..40 (7,680 shapes): none loses the notice and each fits the transport.
4,001 is not claimed to be impossible. On the base the grid's widest shape is 4,000 units with the notice withheld and none reaches 4,001; we suspect that is because the loop only runs while the envelope is over the cap and an emptied roster leaves the envelope entirely — a hypothesis, not a proof. The test therefore pins the rule (no shape in the grid loses the notice) and the adjacent pair 3,988/3,989 instead of a 4,001 length.
docs/subsystems/core-host.md gains the sentence that owns this behaviour and changelog.d/1946-empty-notice-join.fixed.md is the user-facing note. patches/ds-bot/files/src/shared/room-share.js and patches/ds-bot/files/test/remote-room.test.js are the byte-identical mirrors (generate rewrote them; check and canonical are green).
Red first
Both reds below were run on this rebased tree (base 97d4bcdd), with only the implementation reverted by git checkout 97d4bcdd -- core/src/shared/room-share.js and the tests unchanged.
The boundary test stops at the first failing group (3,988 purposefully comes first and is green on the base too):
not ok 1 - \#1946: a cleared notice rides a near-full join at both ends of the band it used to be dropped
error: |-
3989: the wire still carries the cleared notice
false !== true
code: 'ERR_ASSERTION'
# tests 2
# pass 0
# fail 2
The grid test on the same tree:
not ok 2 - \#1946: no join in the near-full grid loses the cleared notice
error: |-
936 of the grid's joins lost the cleared notice
+ actual - expected
+ 'rows 32 handle 40 name 40 owner 40 history 1 first 1'
...
# tests 2
# pass 0
# fail 2
The two upper boundary groups were also run one at a time with the probe the issue cites (show() on the 3,999 and 4,000 groups): both printed "notice" on the wire: ABSENT, guest parseRoomShare().notice: ABSENT -> "change nothing", and the assertion failed with the join still carries the notice the host cleared / undefined !== ''. remote-room.test.js alone with the fix is # tests 86 / # pass 86 / # fail 0.
Ran
TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verify on the committed tree, based on live main 97d4bcdd: exit 0. core 2347, social 541, tree 127, timeline 135, workflow 336, .github/scripts 563, interop 37, live-e2e 24 — 0 failures; arch-docs: OK (6 changed file(s)) against base 97d4bcdd; lint 0 errors; the patch legs: ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/, ok patches/ds-bot/ is what generate writes (generate is a no-op). generate was re-run on this base first and wrote the two mirrored files; check and canonical are green standalone.
Not run: no live host run and no manual two-instance check — the tests drive the real encoder/parser and the real applyInboundRoomShare, but the join is encoded in-process rather than sent through Bot Social.
Checkpoint
{
"pr": 1948,
"issue": 1946,
"claim": "Dev-1",
"summary": "fitJoin's roster yield measured the envelope with the notice taken out, so a present empty notice's 12-unit field cost was invisible: the roster stopped too early, restoring the notice pushed the join back over 4,000, and the notice branch deleted the field, which the guest reads as change nothing. The roster now gives while the envelope overflows with the notice field in place, so the host's clear arrives as an empty notice; the notice text still pays its remaining overflow, so a long non-empty notice is still cut. Two tests pin the 3,988/3,989/3,999/4,000 groups and the whole near-full grid.",
"files": ["core/src/shared/room-share.js", "core/test/remote-room.test.js", "docs/subsystems/core-host.md", "changelog.d/1946-empty-notice-join.fixed.md", "patches/ds-bot/files/src/shared/room-share.js", "patches/ds-bot/files/test/remote-room.test.js"],
"checks": {
"ran": "TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verify, plus both reds on this tree and remote-room.test.js alone",
"result": "green",
"at": "b1babf36832cacce69a0f417ee71d40c68d7dba7"
},
"next": "none — ready for review"
}
Red first (hard requirement): with only core/src/shared/room-share.js swapped back to the base blob 97d4bcdd and the tests unchanged, the two new tests fail —
not ok 79 - #1946: a cleared notice rides a near-full join at both ends of the band it used to be dropped
not ok 80 - #1946: no join in the near-full grid loses the cleared notice
and the rest of the suite stays green (84/86), so the red is the change's own, not a broken tree. With the tip impl restored: 86/86.
Scope: tip is the pushed head b1babf36 whose parent is base 97d4bcdd; the diff is the 6 files the body names (room-share.js +11/−3, remote-room.test.js +115/−1, core-host.md 1/1, the changelog fragment, and the two patches/ds-bot/files/ mirrors) — nothing outside the issue. Both mirrors are byte-identical to their core/ originals (cmp clean), so generate/check consistency holds.
Fix reads correctly: noticeKeyCost = ',"notice":""'.length (12) when a notice is present, and the roster now yields while wire-without-notice + 12 > 4000, so the empty-notice field always survives the yield. Counting only the field's cost (not the text) keeps the non-empty path intact — #1932's cut-not-drop case is green at this tip.
Checks at the tip: 16 check-runs, 15 success; the one failure is enqueue-guard at 15:53:45Z, which fired before any conclusion existed at this tip and lists no PASS as its reason — no other failure. Labels: agent-pr added. Body: Fixes #1946, Owner: Dev-1, Reviewer line present; ## Checkpoint block is valid (1,276 chars, summary 600, 6 files, all in this diff, next 23).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1946
Owner: Dev-1
Written-by: Dev-1
Reviewer: Birfy
Base / tip
Base: 97d4bcd
Tip SHA: b1babf3 (the pushed head; its parent is that base and its tree is d785ba3aaa08c0e03f711884da467609d803c4c7, so the run below covers it)
Verified commit: a50f9b9d128426506004e07b396bfac6003055d7 (tree d785ba3aaa08c0e03f711884da467609d803c4c7; the pushed head is that same tree, rebuilt onto live main by the push path — the branch carries one replay commit,
git log upstream/main..HEADshows only it andmerge-baseis 97d4bcd)Diff: 6 files, +254/−9
What changed
fitJoinincore/src/shared/room-share.jslets the roster go a row at a time first, but it measured the envelope throughoverWithoutNotice()— the notice deleted, the JSON serialized, the notice restored. That measure leaves out the 12 units a present notice costs on the wire even when it is empty (,"notice":""). So in a room whose roster is nearROSTER_MAX, the roster stopped as soon as the notice-less envelope fitted; restoring the notice pushed the join back overROOM_SHARE_TEXT_MAX(4,000); the notice branch then hadwas === '', so the "cut down to nothing" rule (body.notice.length === 0 && was.length > 0) did not apply and the fallbackdelete body.noticedid. A missingnoticereads on the guest as "change nothing" (room-membership.jsnoticeOf), so the host's explicit clear never arrived.The roster's yield now counts the notice field's own cost —
noticeKeyCost = ',"notice":""'.length(12) whenever a notice is present — sowhile (roster.length > 0 && wire-without-notice + 12 > 4,000)stops at a length where the notice field still fits. The roster gives exactly the row that was missing, and the field survives. This is deliberately the field's cost, not the notice text's: the text still pays what is left over in the notice branch below, so #1932/#1934's cut-not-drop case for a long non-empty notice is unchanged (the existing#1932test, roster kept and notice cut, is green in the run below).core/test/remote-room.test.jsgains two tests:#1946: a cleared notice rides a near-full join at both ends of the band it used to be dropped— four groups built from ordinary sizes (32 rows, handle 40 / home 20 / name 40 / ownerName 40, five history lines of 1,880,notice: ''). Their place in the band is the length the old loop measured, witnessed independently by the same join encoded with no notice field. 3,988 keeps the notice (3,988 + 12 = 4,000 exactly) with the roster at 20 rows; 3,989, 3,999 and 4,000 are the band that lost it on the base, and each now carriesnotice: '', guestparseRoomShare(...).notice === '', history dropped and the roster at 19 rows (one more row given, not the list). It also pins both non-empty controls: a long notice is still cut rather than dropped, and a non-empty notice clipped to nothing is still dropped.#1946: no join in the near-full grid loses the cleared notice— the plan's grid, rows {32, 40, 64} × handle {40, 60, 80, 120} × name {40, 60} × ownerName {40, 60} × history {0, 1, 5, 20} × first-row name 1..40 (7,680 shapes): none loses the notice and each fits the transport.4,001 is not claimed to be impossible. On the base the grid's widest shape is 4,000 units with the notice withheld and none reaches 4,001; we suspect that is because the loop only runs while the envelope is over the cap and an emptied roster leaves the envelope entirely — a hypothesis, not a proof. The test therefore pins the rule (no shape in the grid loses the notice) and the adjacent pair 3,988/3,989 instead of a 4,001 length.
docs/subsystems/core-host.mdgains the sentence that owns this behaviour andchangelog.d/1946-empty-notice-join.fixed.mdis the user-facing note.patches/ds-bot/files/src/shared/room-share.jsandpatches/ds-bot/files/test/remote-room.test.jsare the byte-identical mirrors (generaterewrote them;checkandcanonicalare green).Red first
Both reds below were run on this rebased tree (base
97d4bcdd), with only the implementation reverted bygit checkout 97d4bcdd -- core/src/shared/room-share.jsand the tests unchanged.The boundary test stops at the first failing group (3,988 purposefully comes first and is green on the base too):
The grid test on the same tree:
The two upper boundary groups were also run one at a time with the probe the issue cites (
show()on the 3,999 and 4,000 groups): both printed"notice" on the wire: ABSENT,guest parseRoomShare().notice: ABSENT -> "change nothing", and the assertion failed withthe join still carries the notice the host cleared / undefined !== ''.remote-room.test.jsalone with the fix is# tests 86 / # pass 86 / # fail 0.Ran
TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verifyon the committed tree, based on live main97d4bcdd: exit 0. core 2347, social 541, tree 127, timeline 135, workflow 336,.github/scripts563, interop 37, live-e2e 24 — 0 failures;arch-docs: OK (6 changed file(s))against base97d4bcdd; lint 0 errors; the patch legs:ok 10 patches apply cleanly in order,ok core/ equals ds-bot/ + patches/ds-bot/,ok patches/ds-bot/ is what generate writes (generate is a no-op).generatewas re-run on this base first and wrote the two mirrored files;checkandcanonicalare green standalone.Not run: no live host run and no manual two-instance check — the tests drive the real encoder/parser and the real
applyInboundRoomShare, but the join is encoded in-process rather than sent through Bot Social.Checkpoint
{ "pr": 1948, "issue": 1946, "claim": "Dev-1", "summary": "fitJoin's roster yield measured the envelope with the notice taken out, so a present empty notice's 12-unit field cost was invisible: the roster stopped too early, restoring the notice pushed the join back over 4,000, and the notice branch deleted the field, which the guest reads as change nothing. The roster now gives while the envelope overflows with the notice field in place, so the host's clear arrives as an empty notice; the notice text still pays its remaining overflow, so a long non-empty notice is still cut. Two tests pin the 3,988/3,989/3,999/4,000 groups and the whole near-full grid.", "files": ["core/src/shared/room-share.js", "core/test/remote-room.test.js", "docs/subsystems/core-host.md", "changelog.d/1946-empty-notice-join.fixed.md", "patches/ds-bot/files/src/shared/room-share.js", "patches/ds-bot/files/test/remote-room.test.js"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=97d4bcdd5aee168705040df2266963bbd23314fa npm run verify, plus both reds on this tree and remote-room.test.js alone", "result": "green", "at": "b1babf36832cacce69a0f417ee71d40c68d7dba7" }, "next": "none — ready for review" }