test(plugin-email): assert the over-limit attachment by identity, not deep equality

[claude]

Part of #16506

Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ

What changed

One assertion, in packages/plugins/plugin-email/src/email-service.queue-delivery.test.ts, in the
test still refuses the queue for attachments OVER the limit, and stores nothing (#5177):

// was
expect(transport.send).toHaveBeenCalledWith(expect.objectContaining({
  attachments: [{ filename: 'big.bin', content: huge }],
}));

// now
const [delivered] = transport.send.mock.calls[0]!;
const files = delivered.attachments!;
expect(files).toHaveLength(1);
expect(files[0]!.filename).toBe('big.bin');
expect(files[0]!.content).toBe(huge);   // identity, not deep equality

(The committed file inlines the array instead of binding files. It is bound here only to keep
the non-null operator away from a following bracket: GitHub's renderer treats the byte pair
! + [ as the start of image syntax and eats the ! — measured on the first version of this
description, INSIDE a fenced code block. Read the file, not this excerpt, for the exact text.)

Why — identity is the STRONGER assertion. Speed is the consequence, not the reason.

The comment directly above the assertion states its subject: "Pre-#5177 behaviour, unchanged:
delivered inline and delivered WHOLE." Deep equality is the weaker reading of "whole" — it
passes for a copy too, so it cannot tell an untouched buffer from one the service re-encoded,
sliced and rebuilt to the same bytes. toBe proves the exact instance the caller allocated
travelled through the over-limit path untouched.

This is measured, not argued. A reverse mutation made normalizeMessage hand the transport a
byte-identical copy (content: Buffer.from(a.content)), and the two assertions were run against
that same mutated tree:

assertion under the copy mutation	result
old — toHaveBeenCalledWith(objectContaining(...))	PASSES, 656 ms — blind to the copy
new — toBe(huge)	FAILS: expected Buffer[...] to be Buffer[...] // Object.is equality / Received: serializes to the same string

That last line is vitest saying the two buffers are deep-equal. The old assertion could not have
seen the defect; the new one does. Four further mutations at the delivery seam, each with its
mutation proven on disk and its restore proven byte-exact against the HEAD blob:

mutation	new assertion
copy the buffer (bytes identical)	RED — Object.is equality on content
drop the attachment at transport.send	RED — Target cannot be null or undefined on toHaveLength(1)
truncate at transport.send (subarray(0, 10))	RED — Object.is on content
replace at transport.send (same length, 0x42)	RED — Object.is on content, and it is the ONLY failure
truncate inside normalizeMessage	RED — caught earlier, at res.status, because truncating under the limit changes the ROUTING

What it cost the repo

That single deep-equality walk over a 256 KiB + 1 buffer ejected two independent PRs from the
merge queue inside 24 hours (#16369 and #16442), and the queue rebuilds everything behind an ejected
PR. The failure reason on both was Error: Test timed out in 5000ms. — a timeout, not an
AssertionError. At ~670 ms on an idle machine the test needs only a ~7.5x slowdown to cross the
5000 ms default, which is ordinary contention for one of six parallel shards.

Timings — re-derived in this container, under the shared verify lock, --reporter=verbose

reading	before	after
the target test	674 ms	1-2 ms
its file, tests total (22 tests)	800 ms	125 ms
whole plugin-email package, tests total (468 tests)	2287 ms	1537 ms
the target's rank in the package	#1, 2.1x the runner-up	not in the top 5

Its 21 siblings total 126 ms between them. 22 passed on both legs; 468 passed on both package legs.
Absolutes here are shared-box seconds, so the ratio is the durable claim.

What was deliberately NOT done

• ⛔ no testTimeout raised and no per-test timeout added — the point is that the cost is
  removable; raising the ceiling would convert a visible ejection into a slow suite nobody looks at;
• ⛔ the fixture is NOT shrunk — the over-limit boundary IS the test's subject;
• ⛔ nothing skipped, quarantined or marked .todo;
• ⛔ no neighbouring test touched, no label graded, nothing else in plugin-email widened.

One incidental change inside the same test: its transport fake now declares its parameter
(vi.fn(async (_message: NormalizedEmailMessage) => ...)). vi.fn(async () => ...) has an empty
parameter tuple, so reading mock.calls[0][0] is a type error under this package's test-layer
typecheck (TS2493 + three TS18048). Declaring the parameter is what makes the captured call typed
rather than cast.

Census — is anything else this shape?

Two instruments, both with their own positive control:

1. A multiline grep for a deep-equality assertion carrying a large-buffer identifier across all 25
   plugin-email/src/*.test.ts: exactly one hit, the one this PR changes (so the census fires on
   its own target).
2. The behavioural census — per-test durations across all 468 tests in the package. After this
   change the top entries are 172 ms (smtp.wire), 163 ms (sys-email-payload.wire), 122 ms and
   101 ms. The former runner-up at 317 ms is produces a byte-identical MIME message, inline vs row round trip, whose cost is a real MIME round trip through onTheWire, not an assertion — a
   different shape, and it already asserts with toBe.

Nothing else in plugin-email carries the defect. Nothing else was touched.

Clause-② — no, measured rather than predicted

The diff is confined to one *.test.ts. Measured on @objectstack/plugin-email, whose files[] is
dist:

• Ablation, both legs rebuilt: built at head, swapped the one changed file back to the merge base
  c383352cb, rebuilt. All 6 published files byte-identical (index.d.ts, index.d.mts,
  index.js, index.mjs, and both .map). The rebuild is proven live by dist/index.js mtime
  moving 1788762853 to 1788762860. Restore proven: blob back to the HEAD blob and git diff HEAD
  empty.
• Sourcemap source census: index.js.map lists 18 sources and index.mjs.map 17; 0 are
  .test.ts. Firing control: src/email-service.ts IS among the sources of both.
• Untouched-text controls taken from the edited test file (independent of this change) appear in
  0 of the 6 published files; so does this diff's own new text. Firing controls from a non-test
  source appear in 4 and 6 of them.

Changeset — the skip-changeset label, derived

.github/workflows/pr-automation.yml routes a diff that "releases nothing (... tests-only, and the
like)" to the label, which that file marks as the PREFERRED route, and reserves the WHICH LEVEL question for route 1, a PR
that releases something. The measurement above is exactly the evidence for route 2: no published
byte moves. scripts/check-changeset-no-major.mjs adds the cost of getting this wrong — every
publishable package is in the Changesets fixed group, so a patch here would version the whole
lockstep group for a diff no consumer can observe. The label is applied to this PR.

Verification

7d8103600, the head this PR points at.

• pnpm --filter '@objectstack/plugin-email^...' build — dependency closure, green.
• pnpm --filter @objectstack/plugin-email run typecheck — green: tsc --noEmit plus
  check:test-typecheck, the latter reporting 0 file(s) / 0 error(s). The edited test file is
  proven inside the swept population (tsc --listFiles on both tsconfig.json and
  tsconfig.test.json lists it).
• pnpm --filter @objectstack/plugin-email exec vitest run — 30 files, 468 passed.
• 56 gate families, derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
  from the real change set (the script reads the merge base itself). 53 exit 0. The other three
  are NOT MEASURED rather than red, and two say so in their own words:
  check:dual-build-cjs-loads and check:published-readme-exports both exit 3, PREREQUISITE NOT MET -- this gate reads built output, and some package has no dist/, naming 51 packages this
  round never built (none of them touched here); check:react-declaration-parity needs a
  browser-produced SDUI manifest. All three read built output or a browser artefact, and the
  ablation above shows this diff's built output is byte-identical — so their verdicts are invariant
  under it. Declared narrowing; CI runs the farm.
• Line-number anchors: this diff adds lines and moves line numbers inside the test file. git grep
  finds zero references to email-service.queue-delivery outside its own package; the same grep
  shape on registry.ts: returns hits in 5+ files, so the zero is a reading. The only
  plugin-email/src/*.ts:NNN anchors in the repo name email-plugin.ts:117 and
  email-template-provenance.ts:54 — neither is the edited file.
• pnpm check:nul-bytes green, plus a direct control-character scan of the edited file: no hits.

Notes for triage

Part of, not a closing keyword: this PR removes the measured cause of the two recorded ejections,
but #16506 is a standing anchor that the merge-queue-triage workflow refreshes on every further
ejection, and its own text asks a human to decide whether it closes. That decision is triage's, not
this PR's.

#16434 is a separate card about a different cause — packages running expensive suites under
vitest's default 5000 ms budget — and it stays open on its own terms. The two are not the same
defect: that one is a budget question, this one was a single accidental O(n) assertion.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see

• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json a83482c5d0f689d9b855cd7c7fab23cf760c09d3 → packageMentionDocs.

[claude]

check:partof-closing-keyword is red, and the repair is at merge — the gate says so itself

My defect, stated plainly: commit 7d8103600 carries a Part of #16506 trailer in its message.
RULE 2 forbids a card-relation trailer in any commit on a PR, because this repo squash-merges and
the landed message is assembled by concatenating commit messages. The relation belongs in the body
only, and this PR's body does carry it exactly once.

I am not amending or force-pushing to remove it, and that is the gate's own instruction, not a
preference. Verbatim from the gate's error output, run locally against the real posted body and
the real commit list:

Remedy: move this relation into the PR body, where it is stated once, and take the squash
message from that body at merge. Do NOT amend, rebase or force-push to remove it — rewriting
pushed history is forbidden here, and it is not what fixes this.

Its docblock says the same: "a red on an already-pushed branch is repaired the way this repo's
merges already repair it, in the body and at the merge." AGENTS.md's multi-agent discipline and
the os-dev agent rules forbid force-push independently of this gate.

Measured, so nobody has to re-derive it

Run with the real inputs — PR_NUMBER, PR_BODY from the posted body, PR_COMMITS_FILE from
/pulls/16521/commits — with the exit code captured by redirect-then-read, never through a pipe:

run	inputs	exit
actual	real body + real commit list	1 — one finding, the commit trailer
control	same body, same commit, trailer line deleted	0 — green

The control is what makes this precise: the trailer line is the sole cause. The body passes
RULE 1 and RULE 3 as posted, so the prose mentions of other card numbers in it are not implicated,
and no body edit can clear this red.

What the merger needs to do

Take the squash message from this body, which declares the relation once as Part of #16506.
The trailer in the commit message must not reach the landed message.

If a green check is required rather than a documented red, the only route that does not rewrite
pushed history is a replacement branch and a replacement PR carrying the identical tree with a
clean commit message. That trades this PR's identity for a green, so it is the PM's call and not
mine — say the word and I will open it.

---

Generated by Claude Code

[claude]

Superseded by #16522, which carries the
identical tree (both trees hash to fb2e339a1904da01469ce1981e1a6303e82a64ad) with a commit
message that carries no card-relation trailer.

Why this PR could not simply be repaired: its single commit message carried the card relation as a
trailer, which check:partof-closing-keyword RULE 2 refuses -- this repo squash-merges, so every
commit message on a PR is concatenated into the message that lands, and the relation must be
declared once, in the body, only. Removing a trailer from an already-pushed commit needs a history
rewrite, and that gate's own emitted remedy forbids exactly that: "Do NOT amend, rebase or
force-push to remove it -- rewriting pushed history is forbidden here, and it is not what fixes
this." AGENTS.md and the os-dev agent rules forbid force-push independently of the gate. A
replacement branch is the only route that removes the trailer without rewriting anything.

Measured, so the diagnosis is not a guess: the gate run against this PR's real body and real commit
list exits 1 naming only the trailer; the same body with only that trailer line deleted exits 0. The
trailer was the sole cause, and no edit to this body could have cleared it.

This PR's branch is left in place, untouched.

---

Generated by Claude Code