fix(compaction): review round 11 — settle-helper null-guard to the chokepoint

The settleSendResponse extraction left the two panes' call sites diverging on
the null-guard: interactive passed bare `data`, the coordinator passed
`data || {}` — reintroducing the copy-paste variation the shared helper existed
to erase. If a /send 2xx body were ever non-object JSON, the unknown/"ok"
fall-through would deref `data.attached_ids` and paint an already-delivered
message as a connection error; the endpoint always returns an object, so this
is a latent divergence, not a live bug.

Normalize the body once at the helper entry (`data = data || {}`) so both call
sites pass bare `data` and stay byte-identical, and every internal deref plus
any future caller is covered by the single chokepoint. The node settle-harness
gains a null-body case — red without the fix, since the call-arg evaluation
throws before the stub runs.
This commit is contained in:
Patrick Buckley
2026-07-17 10:08:25 -07:00
parent 1e86f068cd
commit abe053f507
3 changed files with 20 additions and 2 deletions
+10
View File
@@ -830,6 +830,16 @@ if (c.includes("promote"))
c = run(makeEl({ hasAttribute: (a) => a === "aria-busy" }), false, queued);
if (c.includes("promote"))
throw new Error("dismiss-in-flight chip must be left to its DELETE verdict: " + c);
// Null / non-object 2xx body (a misbehaving proxy answering `200 null`): the
// helper normalizes it to {} so neither call site guards — it must fall through
// to the unknown/"ok" arm and SETTLE the optimistic chip (promote), never throw
// and strand a delivered message as a connection error. (The no-op
// consumeAttachments stub cannot prevent this: data.attached_ids is evaluated to
// build the :639 call args, so against unfixed code this line throws and crashes
// the harness.)
c = run(makeEl(), false, null);
if (!c.includes("promote"))
throw new Error("null body must settle via unknown-ok, not throw: " + c);
console.log("settle matrix OK");
""",
encoding="utf-8",
@@ -2068,7 +2068,7 @@ function createCoordinatorPane(root, wsId, opts) {
// queue_full, attachments_busy, cross_user, unknown-ok) lives in
// the shared helper — ONE settle matrix for both panes; see
// settleSendResponse's contract for the arm semantics.
settleSendResponse(queue, data || {}, {
settleSendResponse(queue, data, {
queuedEl,
optimisticEl,
isBusy,
+9 -1
View File
@@ -544,7 +544,15 @@ export function parsePriority(text) {
// busy / attachments_busy / cross_user_interjection / unknown-ok —
// the panes' historical shapes, verbatim.
export function settleSendResponse(queue, data, ctx) {
var status = data && data.status;
// Normalize a null / non-object 2xx body once, here at the shared
// chokepoint, so neither pane's call site has to guard it (interactive
// passed bare `data`, the coordinator passed `data || {}` — the divergence
// this helper exists to erase). In-tree /send always returns an object, so
// this only hardens against a misbehaving proxy answering e.g. `200 null`;
// without it the unknown/"ok" fall-through below would deref
// data.attached_ids and surface a delivered message as a connection error.
data = data || {};
var status = data.status;
if (status === "queued" && data.msg_id) {
var queuedEl = ctx.queuedEl;
if (!queuedEl && data.deferred) {