feat(lint): add bun:test expect-expect oxlint plugin

.archgate/adrs/ARCH-005-testing-standards.md

@@ -44,6 +44,10 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes
 - **When a test creates a temp git repo and needs to call `git commit`, configure local user identity first** — CI runners have no global git config, so commits fail without explicit local identity. Set it with `await Bun.$\`git config user.email "test@test.com"\`.cwd(tempDir).quiet()`and`await Bun.$\`git config user.name "Test"\`.cwd(tempDir).quiet()`immediately after`git init`.
 - Test public module interfaces, not private implementation details
 - Use descriptive test names that explain the expected behavior
+- **Every test MUST contain at least one `expect()` assertion** — enforced by the custom `bun-test/expect-expect` oxlint plugin at `lint/expect-expect.ts` (registered via `jsPlugins` in `.oxlintrc.json`, enabled for `tests/**/*.test.ts`). `bun run lint` fails on any runnable `test()`/`it()` whose body contains no `expect()`. oxlint's built-in `jest/expect-expect` does not cover `bun:test`, which is why this plugin exists.
+- **Make implicit "does not throw" contracts explicit** — for a smoke test that merely invokes a function, assert the contract: `expect(() => fn()).not.toThrow()` for synchronous calls, or `await expect(promise).resolves.toBeUndefined()` for async calls. Calling a function with no assertion provides false confidence and is blocked by the assertion rule.
+- **Use `test.skip` or `test.todo` for intentionally empty or disabled tests** — the `bun-test/expect-expect` rule deliberately ignores `.skip` and `.todo` so placeholders remain explicit and self-documenting.
+- **When adding the first `expect()` to a previously assertion-less test file, add `expect` to the `bun:test` import** — older smoke-test files (e.g., `sentry.test.ts`) historically omitted it because their bodies never asserted.
 - **When mocking `fetch` in tests, assign directly to `globalThis.fetch`** — use `globalThis.fetch = mockFn as unknown as typeof fetch`. Restore in `afterEach` via `mock.restore()` (from `bun:test`) or by reassigning the original reference before the test.
 - **Wrap `spyOn` / `mockImplementation` calls in `try/finally` to guarantee `mockRestore()` runs** — when `expect()` assertions fail, they throw immediately, skipping any `mockRestore()` that follows. The un-restored spy leaks into subsequent tests, causing false positives or false negatives. Pattern: `const spy = spyOn(...).mockImplementation(() => {}); try { /* assertions */ } finally { spy.mockRestore(); }`. Alternatively, create and restore spies in `beforeEach`/`afterEach` hooks instead of inline.
 - **Make large production thresholds injectable so tests can use a small value** — When production code only acts past a large numeric threshold (e.g., `SCOPE_FILE_WARN_THRESHOLD = 1000` in `src/engine/git-files.ts`), add an optional parameter that defaults to the module constant (e.g., `resolveScopedFiles(root, globs, { fileWarnThreshold })`). Tests inject a tiny value such as `5` and create only a handful of files to exercise the same code path. This keeps the test fast and deterministic on every platform instead of materializing thousands of fixture files.
@@ -58,6 +62,8 @@ Use Bun's built-in test runner (`bun test`) for all tests. Test files go in `tes
 - **Don't rely on globally-configured git identity in temp git repos** — always set `user.email` and `user.name` locally in any repo that makes commits. Omitting this works locally (where developers have global git config) but fails silently in CI, producing a cryptic `ShellPromise` error with no indication that git identity is the cause.
 - **Don't let tests send real events to Sentry** — set `Bun.env.NODE_ENV = "test"` in `beforeEach` (and restore in `afterEach`) for any test that initializes Sentry. The Sentry SDK is configured with `enabled: Bun.env.NODE_ENV !== "test"` to prevent test noise from polluting production error tracking.
 - Don't skip tests without a tracking issue
+- **Don't write assertion-less tests** — a `test()`/`it()` body with no `expect()` call silently passes and gives false confidence. The `bun-test/expect-expect` oxlint plugin blocks these at lint time.
+- **Don't use a bare early `return` or an empty callback body to conditionally skip a test** — use `test.skipIf(condition)`, `test.skip`, or `test.todo` so the skip is explicit and the assertion rule can recognize it. Bare returns and empty bodies are exactly how assertion-less tests accumulated silently before the rule existed.
 - Don't import test utilities from `node:test` — use Bun's built-in `bun:test` module
 - **Don't use `mock.module("node:fetch", ...)` to intercept HTTP fetch calls** — in Bun, the runtime fetch is `globalThis.fetch` and `mock.module` targeting `node:fetch` does not intercept it. The mock silently has no effect: the real network is hit, making tests non-deterministic and dependent on external services. Assign `globalThis.fetch` directly instead (see Do's above).
 - **Don't place `mockRestore()` after assertions without `try/finally` protection** — if the assertion throws, the spy is never restored and contaminates subsequent tests. This is especially dangerous when spying on globals like `console.warn` or `globalThis.fetch`, where a leaked spy silently suppresses or redirects output for every test that follows.
@@ -202,6 +208,7 @@ globalThis.fetch = (() =>
 ### Automated Enforcement
 
 - **Archgate rule** `ARCH-005/test-mirrors-src`: Scans all source files in `src/` and verifies a corresponding `.test.ts` file exists in `tests/`. Severity: `error`.
+- **oxlint plugin** `bun-test/expect-expect` (`lint/expect-expect.ts`): A custom oxlint JS plugin enabled for `tests/**/*.test.ts` that fails the build for any runnable `test()`/`it()` (including `test.skipIf(...)()`, `test.each(...)()`) whose body contains no `expect()` call. It ignores `test.skip` and `test.todo`. Runs as part of `bun run lint` (and therefore `bun run validate` and CI). oxlint's built-in `jest/expect-expect` only recognizes `jest`/`vitest` imports, so it does not cover `bun:test` — this plugin fills that gap.
 - **CI pipeline**: `bun test --timeout 60000` runs on every pull request. Test failures and per-test timeouts block merge. All workflow jobs have `timeout-minutes` set to prevent indefinite hangs.
 - **Coverage threshold**: The `Coverage Report` job enforces a 90% minimum line coverage. If total coverage drops below 90%, the job fails and the `Validate Code` gate blocks the PR.
 
@@ -216,7 +223,8 @@ Code reviewers MUST verify:
 5. Tests that call `git commit` on a temp repo configure `user.email` and `user.name` locally before committing
 6. Tests that mock HTTP fetch assign `globalThis.fetch` directly — no `mock.module("node:fetch", ...)` usage
 7. Tests that use `spyOn` or `mockImplementation` inline (not in `beforeEach`/`afterEach`) wrap the spy lifecycle in `try/finally` to guarantee `mockRestore()` runs even when assertions fail
-8. Tests that exercise a numeric production threshold inject a small threshold value rather than generating fixtures large enough to trip the production default, and no per-test timeout override is shorter than the global `--timeout 60000`
+8. Every test asserts something with `expect()` — no smoke tests that merely call a function; "does not throw" contracts are made explicit via `expect(() => fn()).not.toThrow()` or `await expect(promise).resolves.toBeUndefined()`
+9. Tests that exercise a numeric production threshold inject a small threshold value rather than generating fixtures large enough to trip the production default, and no per-test timeout override is shorter than the global `--timeout 60000`
 
 ## References
 

.claude/agent-memory/archgate-developer/MEMORY.md

@@ -30,6 +30,10 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv
 
 ## Patterns & Fixes
 
+- **Custom oxlint JS plugins live in `lint/*.ts`, registered via `jsPlugins` in `.oxlintrc.json`** — The repo has a custom plugin `lint/expect-expect.ts` (rule id `bun-test/expect-expect`) that fails the build for any runnable `bun:test` `test()`/`it()` (incl. `test.skipIf(...)()`, `test.each(...)()`) whose body has no `expect()` call; it ignores `test.skip`/`test.todo`. It exists because oxlint's built-in `jest/expect-expect` only recognizes `jest`/`vitest` imports and silently skips `bun:test`. The plugin uses the ESLint-compatible default-export shape (`{ meta:{name}, rules:{ "x": { create(ctx){ return { CallExpression(node){...} } } } } }`) and runs as native TS under Bun (no build step). Important scoping: `lint/` is NOT in `tsconfig.json` `include` (so `tsc` does not typecheck it) and NOT in `knip.json` `project` (so knip ignores it) — but `oxlint --deny-warnings .` DOES lint it, so the plugin file must itself pass all oxlint rules. To enable a jsPlugin rule only for tests, list it under an `overrides` entry for `tests/**/*.test.ts` (the plugin is loaded top-level via `jsPlugins`, the rule is turned on in the override). The convention is now documented in ARCH-005.
+- **Converting assertion-less smoke tests: assert the "does not throw" contract explicitly** — `expect(() => fn()).not.toThrow()` for sync void calls, `await expect(promise).resolves.toBeUndefined()` for async void calls. Watch for two gotchas: (1) older smoke-test files may NOT import `expect` from `bun:test` (their bodies never asserted) — add it or you get `ReferenceError: expect is not defined` at runtime, not at lint time; (2) a Windows-only `test.skipIf(process.platform !== "win32")` test that was a no-op placeholder can often be made real by stubbing `Bun.spawn`/`Bun.which` in a `try/finally` (see `tests/helpers/git.test.ts` forcing the `wsl which git` fallback to miss).
+- **oxlint `unicorn/no-array-callback-reference`** — Don't pass a bare function reference to `.map()`/`.find()`/`.filter()` etc. (e.g. `args.map(asNode)`); the rule wants the element-only contract made explicit. Wrap in an arrow: `args.map((x) => asNode(x)).find((x) => isFn(x))`.
+- **oxlint `require-unicode-regexp`** — Regex literals need the `u` flag: `/Git is not installed/u`, not `/Git is not installed/`. Applies in tests too (e.g. `expect(...).rejects.toThrow(/.../u)`).
 - **`oxfmt` formats markdown too — run `bun run format` after editing any `.md`, including ADRs** — `format:check` (part of `bun run validate` and the CI "Lint, Test & Check" job) runs `oxfmt --check .` over ALL files, not just `.ts`. It normalizes markdown (e.g., emphasis `*word*` → `_word_`). The `adr-author` skill writes ADR markdown but does NOT auto-format it, so ADR edits frequently fail CI `format:check` even when the TS changes are clean. Always run `bun run format` before committing ADR/markdown edits. Tripped CI on PR #372 (ARCH-005 edit).
 - **Make large production thresholds injectable rather than building giant test fixtures** — Tests that must cross a big numeric threshold in production code (e.g., the 1000-file `SCOPE_FILE_WARN_THRESHOLD` in `src/engine/git-files.ts`) should NOT create 1000+ real files + `git add .` to trip it. On Windows CI runners that filesystem work is slow enough to exceed the per-test timeout — `git add .` gets SIGTERM'd mid-run (`git add . failed (exit 143)`, 143 = 128+SIGTERM) and the whole suite fails flakily. Fix: add an optional override (e.g., `fileWarnThreshold?: number` defaulting to the module constant) to the function under test, and have the test inject a tiny value (5) so it only needs ~6 files. Caused the `git-files > resolveScopedFiles > warns when file scope exceeds threshold` flake on PR #366's Windows smoke test. Also captured in ARCH-005.
 - **Never set a per-test timeout override SHORTER than the global `--timeout`** — CI runs `bun test --timeout 60000` (60s). A per-test `}, 30_000` override makes that test MORE likely to time out than the default, the opposite of the usual intent. Only override the per-test timeout to raise it above the global, never to lower it. The 30s override on the git-files threshold test was the proximate cause of its Windows flake.

.oxlintrc.json

@@ -1,4 +1,5 @@
 {
+  "jsPlugins": ["./lint/expect-expect.ts"],
   "categories": {
     "correctness": "error",
     "pedantic": "error",
@@ -15,6 +16,10 @@
     "typescript/no-deprecated": "error"
   },
   "overrides": [
+    {
+      "files": ["tests/**/*.test.ts"],
+      "rules": { "bun-test/expect-expect": "error" }
+    },
     {
       "files": [".archgate/adrs/*.rules.ts"],
       "rules": { "typescript/triple-slash-reference": "off" }

lint/expect-expect.ts

@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright 2026 Archgate
+
+// Custom oxlint JS plugin: enforce that every runnable bun:test test/it call
+// contains at least one `expect()` assertion.
+//
+// Why this exists: oxlint ships `jest/expect-expect`, but its Rust
+// implementation only recognizes `jest` and `vitest` imports — it silently
+// ignores `bun:test`. This plugin reimplements the rule for the bun:test API
+// using oxlint's JS plugins API (https://oxc.rs/docs/guide/usage/linter/js-plugins.html).
+//
+// The plugin runs natively as TypeScript under Bun, so there is no build step.
+
+/** Minimal ESTree-ish node shape. The oxlint AST is ESLint-compatible. */
+type AstNode = { type: string } & Record<string, unknown>;
+
+/** Narrow an unknown value to an AST node (an object with a string `type`). */
+function asNode(value: unknown): AstNode | undefined {
+  if (
+    value !== null &&
+    typeof value === "object" &&
+    typeof (value as { type?: unknown }).type === "string"
+  ) {
+    return value as AstNode;
+  }
+  return undefined;
+}
+
+/** True when the node is a function expression that can serve as a test body. */
+function isFunctionNode(node: AstNode | undefined): boolean {
+  return (
+    node?.type === "ArrowFunctionExpression" ||
+    node?.type === "FunctionExpression"
+  );
+}
+
+/**
+ * Resolve the leftmost identifier name of a callee chain.
+ *
+ * Examples (callee -> result):
+ *   test                       -> "test"
+ *   test.skip                  -> "test"
+ *   test.skipIf(cond)          -> "test"
+ *   expect(x).toBe             -> "expect"
+ */
+function leftmostName(node: AstNode | undefined): string | undefined {
+  let current = node;
+  while (current) {
+    switch (current.type) {
+      case "Identifier": {
+        return typeof current.name === "string" ? current.name : undefined;
+      }
+      case "MemberExpression": {
+        current = asNode(current.object);
+        break;
+      }
+      case "CallExpression": {
+        current = asNode(current.callee);
+        break;
+      }
+      default: {
+        return undefined;
+      }
+    }
+  }
+  return undefined;
+}
+
+/**
+ * Collect the member method names used in a callee chain.
+ *
+ * Examples (callee -> result):
+ *   test.skip                  -> ["skip"]
+ *   test.skipIf(cond)          -> ["skipIf"]
+ *   test.each(rows)            -> ["each"]
+ *   test                       -> []
+ */
+function memberMethods(node: AstNode | undefined): string[] {
+  const methods: string[] = [];
+  let current = node;
+  while (current) {
+    if (current.type === "MemberExpression") {
+      const property = asNode(current.property);
+      if (
+        property?.type === "Identifier" &&
+        typeof property.name === "string"
+      ) {
+        methods.push(property.name);
+      }
+      current = asNode(current.object);
+    } else if (current.type === "CallExpression") {
+      current = asNode(current.callee);
+    } else {
+      break;
+    }
+  }
+  return methods;
+}
+
+/** Depth-first walk over an AST subtree, skipping back-references and locations. */
+function walk(node: AstNode, visit: (node: AstNode) => void): void {
+  visit(node);
+  for (const key of Object.keys(node)) {
+    if (key === "parent" || key === "loc" || key === "range") continue;
+    const value = node[key];
+    if (Array.isArray(value)) {
+      for (const item of value) {
+        const child = asNode(item);
+        if (child) walk(child, visit);
+      }
+    } else {
+      const child = asNode(value);
+      if (child) walk(child, visit);
+    }
+  }
+}
+
+/** True when the subtree contains a call whose callee resolves to `expect`. */
+function containsExpect(root: AstNode): boolean {
+  let found = false;
+  walk(root, (node) => {
+    if (found || node.type !== "CallExpression") return;
+    if (leftmostName(asNode(node.callee)) === "expect") found = true;
+  });
+  return found;
+}
+
+interface ReportDescriptor {
+  node: AstNode;
+  message: string;
+}
+
+interface RuleContext {
+  report(descriptor: ReportDescriptor): void;
+}
+
+const MESSAGE =
+  "Test has no assertions. Add at least one `expect()` call, or use `test.skip` / `test.todo` for an intentionally empty test.";
+
+const expectExpect = {
+  create(context: RuleContext) {
+    return {
+      CallExpression(node: AstNode) {
+        const callee = asNode(node.callee);
+        const base = leftmostName(callee);
+        if (base !== "test" && base !== "it") return;
+
+        // Skip the inner `test.skipIf(cond)` call of `test.skipIf(cond)(...)`:
+        // only the outermost invocation carries the test callback.
+        const parent = asNode(node.parent);
+        if (parent?.type === "CallExpression" && asNode(parent.callee) === node)
+          return;
+
+        // Disabled/placeholder tests are not expected to assert.
+        const methods = memberMethods(callee);
+        if (methods.includes("skip") || methods.includes("todo")) return;
+
+        const args = Array.isArray(node.arguments) ? node.arguments : [];
+        const callback = args
+          .map((arg) => asNode(arg))
+          .find((arg) => isFunctionNode(arg));
+        if (!callback) return;
+
+        if (!containsExpect(callback)) {
+          context.report({ node, message: MESSAGE });
+        }
+      },
+    };
+  },
+};
+
+const plugin = {
+  meta: { name: "bun-test" },
+  rules: { "expect-expect": expectExpect },
+};
+
+export default plugin;

tests/helpers/auth.test.ts

@@ -101,8 +101,7 @@ describe("auth", () => {
       const { clearCredentials } =
         await import("../../src/helpers/credential-store");
 
-      // Should not throw
-      await clearCredentials();
+      await expect(clearCredentials()).resolves.toBeUndefined();
     });
   });
 

tests/helpers/credential-store-impl.test.ts

@@ -144,8 +144,7 @@ describe("credential-store", () => {
 
   describe("clearCredentials", () => {
     test("does not throw when no credentials exist", async () => {
-      // Should not throw
-      await clearCredentials();
+      await expect(clearCredentials()).resolves.toBeUndefined();
     });
 
     test("cleans up legacy metadata file", async () => {

tests/helpers/git.test.ts

@@ -61,14 +61,23 @@ describe("installGit", () => {
   test.skipIf(process.platform !== "win32")(
     "throws when git is not found on Windows",
     async () => {
-      // On other platforms, installGit would attempt brew/apt install instead of throwing.
-      await withBunWhich(
-        () => null,
-        async () => {
-          // Even on Windows, git is typically available so this won't reach
-          // the throw. This test documents the expected error for the path.
-        }
-      );
+      // On other platforms, installGit would attempt brew/apt install instead
+      // of throwing. Force the WSL fallback (`wsl which git`) to report a miss
+      // so resolveCommand returns null and installGit reaches the Windows throw.
+      const originalSpawn = Bun.spawn;
+      Bun.spawn = (() => ({
+        exited: Promise.resolve(1),
+      })) as unknown as typeof Bun.spawn;
+      try {
+        await withBunWhich(
+          () => null,
+          async () => {
+            await expect(installGit()).rejects.toThrow(/Git is not installed/u);
+          }
+        );
+      } finally {
+        Bun.spawn = originalSpawn;
+      }
     }
   );
 });