oficina / meet Public

the camera watch keeps its budget; the canonical test earns its name

Two small ones from Codex round 2.

S4-10 (P3). `framesEncoded !== cameraFrames` read every change as
"encoding recovered", including the drop to zero a stats or encoder
restart produces. So [100,100,100,0,0,0,0,0] gave repair, repair, reset
instead of one repair and then the reset, and a camera that kept
restarting could be repaired for ever without a single new frame. A
different number is not progress: the baseline moves either way,
because the old number means nothing after a reset, but the repair
budget is only forgiven by frames that actually arrived. Two tests, the
first seen failing first; the second holds the other side — real growth
after a restart does clear the budget.

S4-11 (P3). Every case in the canonicalisation regression sent a
payload that was already canonical, so an implementation forwarding the
request's raw bytes would have passed it. Two of the new cases send
real JSON escapes in a Go raw string and require the decoded letters on
the recipient's wire; putting the raw bytes back on the wire fails
those two and nothing else, which is how they were checked. `share` had
no output coverage at all and now has two cases: `on` is rebuilt from
the bool, so the spelling a sender chose never reaches anybody.

node --check, 200 node tests, make ci.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
Paul Campbell pushed by paul@keymail.dev 716e0f6162227518de64863af758379cf337f722 parent bb97a15
3 files changed, +72 −11
  • internal/meet/static/room.js +10 −3
  • internal/meettest/chat_test.go +32 −8
  • test/browser/room-lane.test.mjs +30 −0
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js
index 6401e56..1eda7a8 100644
--- a/internal/meet/static/room.js
+++ b/internal/meet/static/room.js
@@ -4046,11 +4046,18 @@
return '';
}
if (report.framesEncoded !== entry.cameraFrames) {
- // Encoding. Whatever was wrong is not wrong now, and a repair that
- // worked is forgotten rather than counted against the next stall.
+ // A different number is not the same thing as progress (Codex
+ // S4-10). A stats or encoder restart takes the counter *back*,
+ // often to zero, and reading that as "encoding recovered" handed
+ // the repair allowance back without a single new frame — so a
+ // camera that kept restarting could be repaired for ever and
+ // never reset. The baseline moves either way, because the old
+ // number means nothing now; the budget is only forgiven by frames
+ // that actually arrived.
+ const grew = report.framesEncoded > entry.cameraFrames;
entry.cameraFrames = report.framesEncoded;
entry.cameraStalls = 0;
- entry.cameraRepairs = 0;
+ if (grew) entry.cameraRepairs = 0;
return '';
}
entry.cameraStalls += 1;
diff --git a/internal/meettest/chat_test.go b/internal/meettest/chat_test.go
index c0f25d4..e4628be 100644
--- a/internal/meettest/chat_test.go
+++ b/internal/meettest/chat_test.go
@@ -339,25 +339,49 @@ func TestChatDeliversTheCanonicalValidatedPayload(t *testing.T) {
cases := []struct {
name string
+ typ string
sent string
want string
}{
- {"angle brackets and ampersands go out as themselves",
+ {"angle brackets and ampersands go out as themselves", "chat",
`{"text":"a<b>c&d"}`, `"payload":{"text":"a<b>c&d"}`},
- {"an escape is delivered decoded, not re-escaped",
- `{"text":"A<"}`, `"payload":{"text":"A<"}`},
- {"an ordinary line is untouched",
+ // Codex S4-11: every case here used to be its own answer — a
+ // canonical payload in and the same bytes out, which an
+ // implementation that forwarded the request's raw bytes would
+ // have passed just as well. The two escape cases are the ones
+ // that cannot: they can only come out this way if the payload
+ // was rebuilt from the value that was validated. Verified by
+ // putting the raw bytes back on the wire, where these two fail
+ // and nothing else does. (Whitespace is not a discriminator —
+ // the SSE encoder compacts a RawMessage on its way out — but
+ // the shape is worth pinning anyway.) The backslashes are real:
+ // a Go raw string sends the six characters \u0041, not the
+ // letter A.
+ {"an escape really is delivered decoded", "chat",
+ `{"text":"\u0041\u003c"}`, `"payload":{"text":"A<"}`},
+ {"a tab escape survives as the escape JSON gives it", "chat",
+ `{"text":"a\u0009b"}`, `"payload":{"text":"a\tb"}`},
+ {"whitespace inside the payload is not the payload", "chat",
+ `{ "text" : "spaced" }`, `"payload":{"text":"spaced"}`},
+ {"an ordinary line is untouched", "chat",
`{"text":"plain"}`, `"payload":{"text":"plain"}`},
+ // And the other fan-out type, which had no output coverage at
+ // all: `on` is rebuilt from the bool, so the spelling a sender
+ // chose for it never reaches anybody.
+ {"share on is canonical too", "share",
+ `{"on":true}`, `"payload":{"on":true}`},
+ {"share off is canonical too", "share",
+ `{ "on" :false }`, `"payload":{"on":false}`},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
- body := `{"peer":"` + pa + `","type":"chat","payload":` + c.sent + `}`
+ body := `{"peer":"` + pa + `","type":"` + c.typ + `","payload":` + c.sent + `}`
if res, respBody := a.signalRaw(sqid, body); res.StatusCode != http.StatusOK {
t.Fatalf("%s: %d %s", c.name, res.StatusCode, respBody)
}
- ev, data, ok := sb.nextOf("chat", 2*time.Second)
- if !ok || ev != "chat" {
- t.Fatalf("%s: no chat arrived (%q)", c.name, ev)
+ ev, data, ok := sb.nextOf(c.typ, 2*time.Second)
+ if !ok || ev != c.typ {
+ t.Fatalf("%s: no %s arrived (%q)", c.name, c.typ, ev)
}
if !strings.Contains(string(data), c.want) {
t.Fatalf("%s: wire %s, want it to carry %s", c.name, data, c.want)
diff --git a/test/browser/room-lane.test.mjs b/test/browser/room-lane.test.mjs
index da819b4..b9d0f57 100644
--- a/test/browser/room-lane.test.mjs
+++ b/test/browser/room-lane.test.mjs
@@ -1048,6 +1048,36 @@ test('a camera that comes back forgets the repair it needed', () => {
'the second stall was reset rather than repaired: ' + JSON.stringify(said));
});
+test('a counter that goes backwards is re-baselined, not forgiven', () => {
+ // Codex S4-10. `framesEncoded !== cameraFrames` read every change as
+ // "encoding recovered", including the drop to zero that a stats or
+ // encoder restart produces — so [100,100,100,0,0,0,0,0] gave repair,
+ // repair, reset instead of one repair and then the reset, and every
+ // further reset could hand the allowance back again without a single
+ // new frame being encoded.
+ const c = cameraTx();
+ const entry = watching();
+ const said = verdicts(entry, c.tx, [100, 100, 100, 0, 0, 0, 0, 0]);
+ assert.equal(said.filter((v) => v === 'repair').length, 1,
+ 'the allowance was replenished by a counter going backwards: ' + JSON.stringify(said));
+ assert.equal(said.filter((v) => v === 'reset').length, 1, JSON.stringify(said));
+ assert.ok(said.indexOf('repair') < said.indexOf('reset'));
+});
+
+test('growth after a counter reset does forgive the repair', () => {
+ // The other half: a restart that really did come back is a camera
+ // that is encoding again, and the next stall — an hour later — gets a
+ // repair of its own rather than the reset this one earned.
+ const c = cameraTx();
+ const entry = watching();
+ verdicts(entry, c.tx, [100, 100, 100]);
+ assert.equal(entry.cameraRepairs, 1);
+ verdicts(entry, c.tx, [0, 5, 10]);
+ assert.equal(entry.cameraRepairs, 0, 'real growth after the reset must clear the budget');
+ const said = verdicts(entry, c.tx, [10, 10, 10]);
+ assert.ok(said.includes('repair') && !said.includes('reset'), JSON.stringify(said));
+});
+
test('nothing is repaired on a connection that is not sending a camera', () => {
for (const opts of [{ direction: 'recvonly' }, { direction: 'inactive' }, { readyState: 'ended' }]) {
const c = cameraTx(opts);