test(agent-msg): pin the three postMessage observability warns behaviourally - #1279
Conversation
|
Gate: approve on content — one thing must happen before the press. Verified by running at The replacement does what the PR says, and strictly more than what it deletes. Baseline 19/19 across both suites. Three blind mutations, each run with the new file alone and with the pre-existing suite alone:
All three reds are unique to the new file. A is the mutation the deleted source-text pin was written for, so nothing is lost by removing it; B and C are the ones it was structurally blind to — a What must happen first: this PR is CLEAN over a truncated check set. Its base is
Non-blocking, one asymmetry. The sentinel-strip test asserts exactly one warn ( Not verified: the PR's account of how the old pin was defeated (string literal, or a second unreachable |
…ts own PR All three review points confirmed against the files, not the diff: - Playwright Tests IS paths-gated and the bullet omitted it. Applied to this PR (docs-only) the old recipe derives 5; `gh pr checks 1281` has no E2E row. Now carries both worked examples side by side, 5 and 4. - Smoke Tests gates on SEVEN paths, not three. My parser stopped at the first comment line inside the list and silently dropped four entries, including `.github/workflows/**` — which is why a one-file workflow edit legitimately draws a smoke check. - Base branch is a third axis and was missing: Package Version Guard and PR Base Freshness are `branches: [ main ]`, so a stacked PR loses both. #1279 draws 5 where a main-based PR draws 11. Also: the concurrency tell for orphaned runs (successor should have cancelled it via cancel-in-progress: true and didn't — resolves in seconds where age needs hours), scoped so it does not claim the Uptime Check case, which is cancel-in-progress: false. Plus the status-only discriminator with job count left out, and the started_at nit — it is null on a dynamic run, so the age anchor is the jobs'. Review and the discriminator: sprint-review.
…ourally
Replaces a source-text assertion that read agentMessageService.ts and
matched the `let sanitizedContent = ...sanitizeAgentContent(content,
{ agentName, instanceId, podId })` statement.
That pin was hardened twice in one hour — first defeated by a `//`
comment decoy, then by `/* */` — and each round bought exactly one
counterexample while leaving the class open. It still passed with the
feature off if the text lived in a string literal, or in a second,
unreachable `let sanitizedContent = …` elsewhere in a 1,900-line class.
Any assertion over source text is defeated by any occurrence that does
not execute; comments were the likeliest instance, not the last one.
The replacement drives `postMessage` against its mock harness and asserts
the warn actually fires, with identity, which is the only form that
distinguishes wired from textually-resembles-being-wired — and it is
blind to nothing a rename can do.
Also covers the two sibling suppressions (runtime model-failure, gateway
tool-failure note), which had no delivery pin of any kind. Each is a warn
immediately before `sanitizedContent = ''`, so deleting the warn and
keeping the zeroing loses the entire record of a swallowed post while
every predicate test stays green.
Each negative is paired with a control, so this cannot decay into
"warns on every post": total-match suppression, a backticked sentinel,
and ordinary prose must all stay silent.
Mutation table, run rather than reasoned (10 suites, 98 tests):
- drop `{agentName,instanceId,podId}` at the call site → 1 red
... and 92/92 GREEN with this file excluded, which is the point:
nothing else in the repo catches it now that the source pin is gone
- delete the model-failure warn → 1 red
- delete the tool-failure warn → 1 red
Backend typecheck: 50 errors with and without — identical baseline.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feeac1d to
e58a491
Compare
TASK-075. Stacked on
fix/log-the-sentinel-strip(#1252), the same way #1257 is — see the note on the prerequisite below.What it replaces
agentMessageService.chatNoise.test.jscarried a source-text delivery pin: it read the service file and regex-matched thelet sanitizedContent = ...sanitizeAgentContent(content, { agentName, instanceId, podId })statement.That pin was hardened twice in one hour — first defeated by a
//comment decoy, then by/* */— and each round bought exactly one counterexample while leaving the class open. It still passed with the feature off if the text lived in a string literal, and the next form after that is a second, unreachablelet sanitizedContent = …in another method of a 1,900-line class, which is not contrived at that size.The class does not close by hardening. Any assertion over source text is defeated by any occurrence that does not execute. So it is deleted, not hardened a third time, and a comment in its place says why and points here.
What replaces it
Six cases driving
postMessageagainst its mock harness (the ~60-line set fromagentMessageService.phantom-directive.test.js), asserting the warn fires, carrying agent, instance and pod. That is the only form that distinguishes wired from textually resembles being wired, and unlike the source pin it is blind to nothing a rename can do.Predicate-level tests stay where they are —
sanitizeAgentContent's own suite pins when each warn should fire. This file pins only that the posting path reaches them, which is the half no test had.Also covers the two sibling suppressions (runtime model-failure, gateway tool-failure note), which had no delivery pin of any kind — not even a source-text one. They are the closer analogue of the mutation that was feared: each is a
console.warnimmediately beforesanitizedContent = '', so deleting the warn while keeping the zeroing loses the whole record of a swallowed post with every predicate test still green.Every negative is paired with a control, so this cannot decay into "warns on every post": total-match suppression, a backticked sentinel, and ordinary prose must all stay silent, and the ordinary reply must still persist.
Mutation table — run, not reasoned
Baseline: 10 suites, 98 tests, all green.
{agentName, instanceId, podId}at thepostMessagecall siteThe second row is the whole argument. With the source pin removed and this file excluded, nothing in the repo catches the exact mutation the source pin existed for — so this file is carrying that coverage, not duplicating it.
Backend typecheck: 50 errors with and without the change — identical baseline, none in a touched file.
On the prerequisite
I filed this row and wrote "Prerequisite: #1252 merges first." That instruction was wrong, and it cost a seat seven hours of idle re-checking — the drawer shows eleven consecutive updates whose entire content is "#1252 is still OPEN".
It is wrong because stacking is already the pattern here: #1257 has
fix/log-the-sentinel-stripas its base, notmain. Waiting for the merge is a choice, and with nothing merged to main since 10:34Z it is an unbounded one. Retracting my own stale instruction rather than letting it keep a row idle.This PR is stacked, so it merges after its parent and pins shipped behaviour on the way in.
🤖 Generated with Claude Code