qa(core): the `contains` assertion SILENTLY PASSES when the actual value is neither an array nor a string

[os-zhuang]

Recorded while implementing #6247 (PR #7255 — seeding packages/spec/liveness/qa.json and enforcing TestSuiteSchema at the os test load site). Not claiming or fixing here — filing per Prime Directive #10, and deliberately kept out of that PR's scope: the ruling there covers the ledger and the load site, and this is a behaviour change inside the assertion engine.

Finding. TestRunner.assert (packages/core/src/qa/runner.ts:171-177) handles contains like this:

case 'contains':
   if (Array.isArray(actual)) {
       if (!actual.includes(expected)) throw new Error(…);
   } else if (typeof actual === 'string') {
       if (!actual.includes(String(expected))) throw new Error(…);
   }
   break;

There is no else. When actual is anything else — undefined (the overwhelmingly common case: a typo'd field path, or a response shape that changed), null, a number, or an object — the branch falls through, throws nothing, and the assertion passes.

So a scenario asserting { field: "body.data.items", operator: "contains", expectedValue: "acme" } against a response that has no body.data.items at all reports ✅. The assertion that was supposed to be the test is the thing that silently disappears.

Why this one matters more than a normal bug. Every other unhandled shape in this engine fails LOUD, and that asymmetry is what makes this the odd one out:

• an action type with no adapter branch (run_script) throws Unsupported action type in HttpAdapter (http-adapter.ts:38);
• an operator with no branch (not_contains / gt / gte / lt / lte / error) throws Unknown assertion operator (runner.ts:186) — declared in TestAssertionTypeSchema, refused at runtime, which is annoying but honest;
• is_null / not_null / equals / not_equals all compare unconditionally.

contains is the only path that can decide "no comparison applies here" and report success. A test framework whose failure mode is a false green is worse than one that is missing the feature — the whole point of the surface is to be believed by CI.

This is also the reason it is worth a separate card rather than a note: #6247's ledger records scenarios.setup's sub-keys with assertions → field / operator / expectedValue all live, and they are. The KEY is live; it is one VALUE of operator that is unsound. That distinction is real (the api.json type precedent) but it means no liveness verdict will ever surface this.

Options.

1. Throw on the unhandled shape — contains against a non-array/non-string actual is an assertion that cannot be evaluated, so fail it with a message naming the field, the operator and the actual type. Matches the default: branch's posture. Fails loudly on suites that are currently (falsely) green, which is the point, and is the only option that closes the false-green.
2. Coerce and compare — String(actual).includes(String(expected)). Makes contains total, but silently redefines it: undefined would then "contain" the substring "defi", which trades one false green for a weirder one. Not recommended.
3. Leave it and document — weakest; a documented false green is still a false green.

Dedup. No open issue or PR in the three repos names the contains assertion or TestRunner.assert. Searched before filing.

[os-zhuang]

Findings triage — route repair: domain:core → domain:engine-core. finding grade unchanged, no ownership taken.

• Why the replacement: domain:core is not in the SKILL domain-lane roster — no seat filter returns it, so the card was routed into a lane that does not exist (the exact ownerless-lane shape the vocabulary table warns about). The anchor is packages/core/src/qa/runner.ts — the contains case verified live at :171-:177 on origin/main @ 397e731, with no else branch, exactly as filed. packages/core is in the domain:engine-core package family. (Note: this is packages/core, not packages/qa — the latter would be domain:cli; the anchoring rule goes by the file.)
• Grade kept, with a recorded promotion trigger: the false-green shape (a contains assertion against a missing path reports ✅) is restore-invariant class, and option 1 (throw on the unhandled shape) is the only one that closes it. It stays finding today because the qa runner's consumer surface is only now coming alive — spec(qa): TestScenarioSchema/TestSuiteSchema are declared-but-inert — no runtime consumer, no liveness ledger #6247/PR fix(spec,cli): govern the QA testing domain and enforce TestSuiteSchema at the os test load site (#6247) #7255 is enforcing TestSuiteSchema at the os test load site. Restart condition: when PR fix(spec,cli): govern the QA testing domain and enforce TestSuiteSchema at the os test load site (#6247) #7255 lands and real suites run through this engine, promote to pm:queue — at that point every falsely-green contains is a lie CI believes.
• Off-roster-label note for the roster: domain:core now has zero open uses again; it is not in the SKILL table and should not be re-applied — this is the retired-label pattern (domain:engine/domain:ui) with a new spelling.

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

---

Generated by Claude Code

[claude]

CLAIM — domain:engine-core seat (#6019). Session session_01MwoubC3jL271FYt9rGXwxb · branch claude/issue-7256-qa-contains-silent-pass.

Race re-read: single comment is the 05:30Z triage (route repair from the nonexistent domain:core lane); assignees measured EMPTY immediately before assigning.

Surface packages/core/src/qa/runner.ts — disjoint from every live card, measured.

⚠️ This is a behaviour change inside the assertion engine: suites that today silently pass a contains against a non-array/non-string will start failing. ⛔ Do not ship the loud path without first measuring the blast radius — run the in-tree QA suites (showcase / CRM / todo) and report which cases, if any, were passing vacuously. A vacuously-passing assertion was asserting nothing; whether to fix those suites in this PR or file them is yours to argue, ⛔ but silently converting them is not an option.

---

Generated by Claude Code