fix(core): fail the QA contains assertion on a non-evaluable actual instead of silently passing (#7256)

.changeset/qa-contains-non-evaluable-fails-loud.md

@@ -0,0 +1,60 @@
+---
+"@objectstack/core": minor
+---
+
+fix(core): the QA `contains` assertion fails loudly instead of silently passing on a non-array/non-string actual (#7256)
+
+`TestRunner.assert`'s `case 'contains':` handled the two shapes it can evaluate —
+an array (membership) and a string (substring) — and had **no `else`**. Every
+other shape fell straight out of the switch throwing nothing, so the assertion
+reported **PASSED**. A scenario asserting
+`{ field: "body.data.items", operator: "contains", expectedValue: "acme" }`
+against a response that has no `body.data.items` at all reported ✅. The
+overwhelmingly common way to reach that branch is the one that matters most: a
+typo'd `field` path, or a response shape that moved under a suite nobody
+re-read. The assertion that was supposed to *be* the test is the thing that
+silently disappears, and CI believes the green.
+
+`contains` was the only path in this engine that could decide "no comparison
+applies here" and report success. Every other unhandled shape already fails
+loud — an operator with no branch throws `Unknown assertion operator`, an action
+type with no adapter branch throws `Unsupported action type in HttpAdapter`,
+and `equals`/`not_equals`/`is_null`/`not_null` all compare unconditionally. This
+closes the asymmetry rather than adding a new posture: an assertion the engine
+**cannot evaluate** is a **failed** assertion.
+
+The message is written for the author who has to act on it, so it names the
+field, the operator and the runtime type of what the path actually resolved to
+(`null` and arrays get their own names, not `typeof`'s `object`), and then says
+which of the two things is wrong:
+
+```
+Assertion failed: body.data.items cannot be evaluated by 'contains' — expected an
+array or a string at that path, got undefined. The path resolved to nothing — the
+field is absent from the result, or the path is misspelled. Use 'is_null' if
+asserting absence is what you meant.
+```
+
+`undefined`/`null` point at the **fixture** (the path did not resolve, so the
+field path or the response shape it was written against is the suspect);
+a number, boolean or object points at the **assertion** (the path resolved
+fine and `contains` is the wrong operator for what it found).
+
+**Behaviour change, and its measured blast radius.** Suites that today pass a
+`contains` against a non-array/non-string will start failing — which is the
+point; each such assertion was asserting nothing. The in-tree radius was
+measured on the loud build and is **zero**: `os test` is the runner's only
+consumer, and the repository contains no Quality Protocol suite documents at
+all (no `qa/*.test.json` anywhere; the three example apps run `vitest`, and
+`packages/qa/*` are vitest suites that never touch `TestRunner`). No CI workflow
+invokes `os test`. So no in-repo case was passing vacuously and none needed
+repair. Downstream suites are the ones that will see red, and every case they
+see is a test that was never running.
+
+The two evaluable shapes are untouched in both directions: a matching array or
+string still passes, a non-matching one still fails with its existing message.
+`not_contains`, `gt`, `gte`, `lt`, `lte` and `error` are declared in
+`TestAssertionTypeSchema` and still have no branch in the runner — they were
+already refused loudly at `default:` rather than silently passed, so they do not
+carry this defect; that gap is recorded separately and is pinned here so a later
+implementation is a deliberate change rather than an accident.

content/docs/deployment/cli.mdx

@@ -1026,6 +1026,15 @@ and counts as one failed suite — the rest of the glob still runs. This is what
 stops a malformed suite from reporting success: a misspelled `steps` key used to
 produce a scenario that passed having executed nothing.
 
+An assertion the runner **cannot evaluate** fails, it does not pass. `contains`
+is defined over an array (membership) and a string (substring); point it at
+anything else — most often a `field` path the response does not carry, because it
+was misspelled or the shape moved — and it fails, naming the field, the operator
+and the runtime type it actually found. Until #7256 that case fell out of the
+switch and reported ✅, so a `contains` against a missing path was a test that
+silently deleted itself. Assert absence with `is_null`; compare a scalar with
+`equals`.
+
 #### `os doctor`
 
 Checks your development environment and reports issues:

packages/core/src/qa/runner.test.ts

@@ -0,0 +1,221 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// #7256 — the `contains` assertion used to SILENTLY PASS when the actual value was
+// neither an array nor a string. The `case 'contains':` block handled the two
+// evaluable shapes and had no `else`, so `undefined` (a typo'd `field` path, or a
+// response shape that moved), `null`, a number or an object fell out of the switch
+// throwing nothing, and the scenario reported ✅. A suite asserting `contains`
+// against a field the result does not have was asserting NOTHING, and CI believed it.
+//
+// These pin both halves of the fix: the three non-evaluable shapes now fail LOUD with
+// a message that names the runtime type and says which of the fixture or the assertion
+// is the suspect, and the two evaluable shapes keep their pre-existing behaviour in
+// BOTH directions (a match still passes, a miss still fails).
+
+import { describe, it, expect } from 'vitest';
+import * as QA from '@objectstack/spec/qa';
+import { TestRunner } from './runner.js';
+import type { TestExecutionAdapter } from './adapter.js';
+
+/** An adapter that hands the runner one fixed result — the assertion is the unit under test. */
+class StubAdapter implements TestExecutionAdapter {
+  constructor(private result: unknown) {}
+  async execute(): Promise<unknown> {
+    return this.result;
+  }
+}
+
+/** Run one assertion against one canned adapter result, through the public runner surface. */
+async function runAssertion(
+  result: unknown,
+  assertion: QA.TestAssertion,
+): Promise<{ passed: boolean; error: string }> {
+  const runner = new TestRunner(new StubAdapter(result));
+  const [outcome] = await runner.runSuite({
+    name: 'contains-pins',
+    scenarios: [
+      {
+        id: 'scenario-1',
+        name: 'single assertion',
+        steps: [
+          {
+            name: 'step-1',
+            action: { type: 'api_call', target: '/api/v1/accounts' },
+            assertions: [assertion],
+          },
+        ],
+      },
+    ],
+  });
+  const error = outcome.error;
+  return {
+    passed: outcome.passed,
+    error: error instanceof Error ? error.message : String(error ?? ''),
+  };
+}
+
+const containsAcme: QA.TestAssertion = {
+  field: 'body.data.items',
+  operator: 'contains',
+  expectedValue: 'acme',
+};
+
+describe("TestRunner — `contains` against a non-array/non-string actual fails loud (#7256)", () => {
+  it('fails when the field path resolves to nothing (the filed case: a missing path)', async () => {
+    const { passed, error } = await runAssertion({ body: { data: {} } }, containsAcme);
+
+    expect(passed).toBe(false);
+    // Names the field, the operator and the runtime type...
+    expect(error).toContain('body.data.items');
+    expect(error).toContain("'contains'");
+    expect(error).toContain('got undefined');
+    // ...and points at the FIXTURE, because the path is what did not resolve.
+    expect(error).toContain('absent from the result');
+  });
+
+  it('fails when the whole response shape is missing, not just the leaf', async () => {
+    const { passed, error } = await runAssertion(undefined, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('got undefined');
+  });
+
+  it('fails when the field path resolves to null', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: null } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('got null');
+    // `null` is not reported as `object` — the author needs to see which one it is.
+    expect(error).not.toContain('got object');
+    expect(error).toContain('resolved to null');
+  });
+
+  it('fails when the field path resolves to a number', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: 42 } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('got number');
+    // The path resolved fine here, so the ASSERTION is the suspect, not the fixture.
+    expect(error).toContain('array membership and string substrings only');
+  });
+
+  it('fails when the field path resolves to an object', async () => {
+    const { passed, error } = await runAssertion(
+      { body: { data: { items: { acme: true } } } },
+      containsAcme,
+    );
+
+    expect(passed).toBe(false);
+    expect(error).toContain('got object');
+    expect(error).toContain('array membership and string substrings only');
+  });
+
+  it('fails when the field path resolves to a boolean', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: false } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('got boolean');
+  });
+
+  it('every non-evaluable shape reports the same failure, not a pass', async () => {
+    const nonEvaluable: unknown[] = [undefined, null, 0, 42, false, true, { acme: true }];
+
+    for (const value of nonEvaluable) {
+      const { passed, error } = await runAssertion({ body: { data: { items: value } } }, containsAcme);
+      expect(passed, `contains against ${String(value)} must not pass`).toBe(false);
+      expect(error).toContain("cannot be evaluated by 'contains'");
+    }
+  });
+});
+
+describe('TestRunner — `contains` keeps its behaviour on the two evaluable shapes (#7256)', () => {
+  it('passes when the array contains the expected member', async () => {
+    const { passed } = await runAssertion({ body: { data: { items: ['acme', 'globex'] } } }, containsAcme);
+
+    expect(passed).toBe(true);
+  });
+
+  it('fails when the array does not contain the expected member', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: ['globex'] } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('array does not contain acme');
+    // Still the membership failure, NOT the new inapplicable-shape failure.
+    expect(error).not.toContain("cannot be evaluated by 'contains'");
+  });
+
+  it('passes when the string contains the expected substring', async () => {
+    const { passed } = await runAssertion({ body: { data: { items: 'acme corp' } } }, containsAcme);
+
+    expect(passed).toBe(true);
+  });
+
+  it('fails when the string does not contain the expected substring', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: 'globex corp' } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('string does not contain acme');
+    expect(error).not.toContain("cannot be evaluated by 'contains'");
+  });
+
+  it('an empty array is evaluable — it simply does not contain the member', async () => {
+    const { passed, error } = await runAssertion({ body: { data: { items: [] } } }, containsAcme);
+
+    expect(passed).toBe(false);
+    expect(error).toContain('array does not contain acme');
+  });
+
+  it('an empty string is evaluable — every string contains the empty substring', async () => {
+    const { passed } = await runAssertion(
+      { body: { data: { items: '' } } },
+      { field: 'body.data.items', operator: 'contains', expectedValue: '' },
+    );
+
+    expect(passed).toBe(true);
+  });
+});
+
+describe('TestRunner — the sibling operators are unchanged by #7256', () => {
+  it('`equals` still compares unconditionally, including against a missing path', async () => {
+    const { passed, error } = await runAssertion(
+      { body: {} },
+      { field: 'body.status', operator: 'equals', expectedValue: 'active' },
+    );
+
+    expect(passed).toBe(false);
+    expect(error).toContain('expected active');
+  });
+
+  it('`is_null` still passes on a missing path — absence is what it asserts', async () => {
+    const { passed } = await runAssertion(
+      { body: {} },
+      { field: 'body.status', operator: 'is_null', expectedValue: null },
+    );
+
+    expect(passed).toBe(true);
+  });
+
+  it('`not_null` still fails on a missing path', async () => {
+    const { passed } = await runAssertion(
+      { body: {} },
+      { field: 'body.status', operator: 'not_null', expectedValue: null },
+    );
+
+    expect(passed).toBe(false);
+  });
+
+  // `not_contains` / `gt` / `gte` / `lt` / `lte` / `error` are declared in
+  // `TestAssertionTypeSchema` and have no branch in the runner. That is a DIFFERENT
+  // defect from #7256 (a declared operator the engine refuses is annoying but honest,
+  // where a silent pass is a lie), and it is pinned here so a later implementation is
+  // a deliberate change rather than an accident.
+  it('a declared-but-unimplemented operator is refused, not silently passed', async () => {
+    const { passed, error } = await runAssertion(
+      { body: { data: { items: ['globex'] } } },
+      { field: 'body.data.items', operator: 'not_contains', expectedValue: 'acme' },
+    );
+
+    expect(passed).toBe(false);
+    expect(error).toContain('Unknown assertion operator: not_contains');
+  });
+});

packages/core/src/qa/runner.ts

@@ -19,6 +19,40 @@ export interface StepResult {
   duration: number;
 }
 
+/**
+ * Name the runtime shape of a value the way a suite author sees it in their fixture.
+ * `typeof` answers `object` for both `null` and an array, which are the two shapes a
+ * `contains` author most needs told apart from a plain record.
+ */
+function describeActualType(value: unknown): string {
+  if (value === null) return 'null';
+  if (Array.isArray(value)) return 'array';
+  return typeof value;
+}
+
+/**
+ * Say WHICH of the two things is wrong, because the message is the only thing the
+ * author has: `undefined`/`null` mean the path did not resolve to a value, so the
+ * FIXTURE (the field path, or the response shape it was written against) is the
+ * suspect; anything else means the path resolved fine and the ASSERTION picked an
+ * operator that does not apply to what it found.
+ */
+function containsInapplicableHint(actual: unknown): string {
+  if (actual === undefined) {
+    return (
+      'The path resolved to nothing — the field is absent from the result, or the path is misspelled. ' +
+      "Use 'is_null' if asserting absence is what you meant."
+    );
+  }
+  if (actual === null) {
+    return "The path resolved to null. Use 'is_null' if asserting absence is what you meant.";
+  }
+  return (
+    "'contains' tests array membership and string substrings only. " +
+    "Use 'equals' to compare a scalar, or point the field at the array or string you meant to look inside."
+  );
+}
+
 export class TestRunner {
   constructor(private adapter: TestExecutionAdapter) {}
 
@@ -173,6 +207,20 @@ export class TestRunner {
              if (!actual.includes(expected)) throw new Error(`Assertion failed: ${assertion.field} array does not contain ${expected}`);
          } else if (typeof actual === 'string') {
              if (!actual.includes(String(expected))) throw new Error(`Assertion failed: ${assertion.field} string does not contain ${expected}`);
+         } else {
+             // `contains` is defined over arrays (membership) and strings (substring), and
+             // over nothing else. This branch used to be absent, so every other shape fell
+             // out of the switch and the assertion reported PASSED (#7256) — a `contains`
+             // written against a path the result does not carry was the test silently
+             // deleting itself, and CI believed the green. An assertion the engine cannot
+             // evaluate is a FAILED assertion, which is the posture every other unhandled
+             // shape in this engine already takes (`default:` below; the HTTP adapter's
+             // unknown action type).
+             throw new Error(
+                 `Assertion failed: ${assertion.field} cannot be evaluated by 'contains' — ` +
+                 `expected an array or a string at that path, got ${describeActualType(actual)}. ` +
+                 containsInapplicableHint(actual)
+             );
          }
          break;
       case 'not_null':