docs(testing): new tests extend existing cases and stay at one layer

.coderabbit.config.ts

@@ -27,7 +27,7 @@ export default defineConfig((ctx) => ({
         instructions: "Hold changed code to the rules in docs/internals/web-ui.md.",
       },
       {
-        path: "**/*.test.{ts,tsx}",
+        path: "**/*.test.*",
         instructions:
           "Hold changed tests to the rules in docs/internals/testing.md. Request tests only as those rules allow.",
       },

.macroscope/check-run-agents/test-signal.md

@@ -0,0 +1,27 @@
+---
+title: Test Signal
+model: gpt-6-1-terra
+reasoning: low
+input: incremental
+tools:
+  - browse_code
+  - modify_pr
+# An explicit include overrides ignore.md, which skips test files for every other check.
+include:
+  - "**/*.test.*"
+exclude:
+  - ".repos/**"
+requires:
+  - Check
+maxBudgetPerRun: 1
+maxBudgetPerPR: 5
+conclusion: neutral
+---
+
+# Test signal review
+
+Review only the tests this pull request adds or grows. Tests it moves, renames, or deletes are not findings. The rules are in `docs/internals/testing.md` under `## What to test`; read that section first and hold new tests to it.
+
+Post each finding as a brief inline comment on the new test with one fix. Ask only for fewer or smaller tests, never for more coverage.
+
+When there are no findings, make the entire final response exactly `All clear` on one line with nothing else.

AGENTS.md

@@ -102,11 +102,13 @@ An empty database is a bad test. Seed your worktree's `.t3` with a copy of real
 - While you iterate, run `vp test run <files>`. For big files, add `-t "<name>"`.
 - Use `vp run typecheck`, not `npx tsc`; it caches per package. Both run tsc-rs (`typescript` is the `@tsc-rs/typescript` drop-in).
 - Run `node -e` snippets from `apps/server` or the owning package. `effect` does not resolve from the repo root.
-- Test meaningful logic or observable behavior. Do not render components to static markup to assert props or attributes, or add tests that merely assert callback wiring or mirror the implementation.
+- Test behavior that can break, such as orchestration and event-sourcing, provider protocol translation, auth and permission guards, persistence and migrations, external-format parsing, race fixes, and the logic a bug fix changed. Leave out tests for what types already guarantee, trivial logic, copy text, smoke runs, mock-call mirrors, callback wiring, and props rendered to static markup.
+- Extend before you add. When an existing test or table covers the behavior, add a row or an assertion there. Write a new `it` only when no case fits.
+- Test at one layer: the lowest one that fails on `main`. Do not repeat the case in a wrapper test, at each call site, or in web and mobile copies of the same logic.
 - [Testing](docs/internals/testing.md) carries these same test rules, plus review guidance, for review bots. Change both together.
 - **Do not run repo-wide checks.** No `vp check`, no `vp run -r test`, no `vp run -r typecheck` unless I ask. CI owns the full suite.
 - Backend behavior changes ship with focused tests for that behavior.
-- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in the module's existing test file), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript.
+- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in an existing case of the module's test file where one fits), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript.
 - The server is event-sourced, and side effects run after the command commits. In tests, drain the effect worker (`OrchestrationEffectWorkerV2.drain`) or await the specific persisted event or `Deferred` that marks the milestone. Never wait on sleeps or polling. A test that needs a timeout to pass is wrong. Concretely:
   - `it.effect` runs on a `TestClock`; `Effect.sleep` only advances through `TestClock.adjust`. `it.live` is wall-clock and only for subprocess or network fixtures.
   - No `yieldNow` or `sleep` polling loops. Await the `Deferred`, take from the `Queue`, or drain the worker.

docs/internals/testing.md

@@ -4,10 +4,12 @@ Rules for writing tests, and for asking for them in review. AGENTS.md carries th
 
 ## What to test
 
-- Test meaningful logic or observable behavior. Do not render components to static markup to assert props or attributes, or add tests that merely assert callback wiring or mirror the implementation.
-- In review, ask for a missing test only when it would catch a real regression. Do not ask for one assertion per constant, label, or `switch` case.
+- Test behavior that can break, such as orchestration and event-sourcing, provider protocol translation, auth and permission guards, persistence and migrations, external-format parsing, race fixes, and the logic a bug fix changed. Leave out tests for what types already guarantee, trivial logic, copy text, smoke runs, mock-call mirrors, callback wiring, and props rendered to static markup.
+- Extend before you add. When an existing test or table covers the behavior, add a row or an assertion there. Write a new `it` only when no case fits.
+- Test at one layer: the lowest one that fails on `main`. Do not repeat the case in a wrapper test, at each call site, or in web and mobile copies of the same logic.
+- In review, ask for a missing test only when it would catch a real regression. Do not ask for one assertion per constant, label, or `switch` case. Flag a new test that breaks the rules above, and name the fix: fold it into the existing case or table, delete it, or keep one layer.
 - Backend behavior changes ship with focused tests for that behavior.
-- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in the module's existing test file), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript.
+- Proof for a bug fix depends on the bug. Logic, data, state, and protocol bugs get a failing test first (the fewest that fail on `main` for that reason, in an existing case of the module's test file where one fits), then the fix. Purely visual bugs (spacing, colour, alignment, a layout shift) get before/after captures as their proof and no test: do not add UI tests for nitpicks, and do not ask for one in review when a capture already shows it. A server-only fix with no visible surface ships the test plus a short runtime transcript.
 
 ## Waiting for work
 

evals/README.md

@@ -4,10 +4,11 @@ An eval runs a real model against T3 Code's real text or code and scores what th
 
 Write or run an eval when a change affects what a model sees or does: agent instructions, MCP tool descriptions and schemas, tool results, generated titles, or the guidance an agent gets for a feature such as Device Manager.
 
-| Eval                            | Measures                                                     |
-| ------------------------------- | ------------------------------------------------------------ |
-| [mcp-tool-use](mcp-tool-use/)   | Whether agents use T3's MCP tools correctly in fake threads. |
-| [thread-titles](thread-titles/) | Whether generated thread titles name the right subject.      |
+| Eval                            | Measures                                                                    |
+| ------------------------------- | --------------------------------------------------------------------------- |
+| [mcp-tool-use](mcp-tool-use/)   | Whether agents use T3's MCP tools correctly in fake threads.                |
+| [thread-titles](thread-titles/) | Whether generated thread titles name the right subject.                     |
+| [test-signal](test-signal/)     | Whether a cheap model running the test-signal check flags low-signal tests. |
 
 ## Run an eval
 

evals/mcp-tool-use/run.ts

@@ -50,6 +50,7 @@ import {
   CLAUDE_PROVIDER,
   makeClaudeQueryOptions,
 } from "../../apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts";
+import { proxyConfig } from "../shared/proxy.ts";
 import { changeRequestsCreatedBy } from "../../apps/server/src/orchestration-v2/PullRequestSyncReactor.ts";
 /** Agent models by id, with the display name a harness gives them. */
 const MODELS = {
@@ -1342,34 +1343,6 @@ const callCodex = Effect.fn("evaluateMcpToolUse.callCodex")(function* (
   };
 });
 
-// Read credentials into memory only. Python's TOML parser avoids another runner dependency.
-const decodeProxyConfig = Schema.decodeUnknownSync(
-  Schema.Struct({ url: Schema.String, headers: Schema.Record(Schema.String, Schema.String) }),
-);
-const proxyConfig = () =>
-  decodeProxyConfig(
-    JSON.parse(
-      NodeChildProcess.execFileSync(
-        "python3",
-        [
-          "-c",
-          `
-import json, os, pathlib, tomllib
-home = pathlib.Path.home() / '.codex'
-p = tomllib.loads((home / 'config.toml').read_text())['model_providers']['vibeproxy']
-h = p.get('http_headers', {}).copy()
-h.update({k: os.environ[v] for k,v in p.get('env_http_headers', {}).items()})
-key = os.environ[p['env_key']] if p.get('env_key') else p.get('experimental_bearer_token')
-if not key and p.get('requires_openai_auth'): key = json.loads((home / 'auth.json').read_text()).get('OPENAI_API_KEY')
-if key: h['Authorization'] = 'Bearer ' + key
-print(json.dumps({'url': p['base_url'].rstrip('/'), 'headers': h}))
-`,
-        ],
-        { encoding: "utf8" },
-      ),
-    ),
-  );
-
 const postModel = Effect.fn("evaluateMcpToolUse.postModel")(function* (
   apiKey: string,
   body: unknown,

evals/package.json

@@ -6,6 +6,7 @@
     "mcp-tool-use": "node --env-file-if-exists=../.env mcp-tool-use/run.ts",
     "mcp-tool-use:trim-capture": "node mcp-tool-use/trim-capture.ts",
     "thread-titles": "node --env-file-if-exists=../.env thread-titles/run.ts",
+    "test-signal": "node --env-file-if-exists=../.env test-signal/run.ts",
     "typecheck": "tsc --noEmit",
     "test": "vp test run --config ../vite.config.ts --dir ."
   },

evals/shared/proxy.ts

@@ -0,0 +1,33 @@
+// Eval-only helper: it reads the local Codex config through python3 at startup.
+// @effect-diagnostics nodeBuiltinImport:off
+import * as NodeChildProcess from "node:child_process";
+import * as Schema from "effect/Schema";
+
+// Read credentials into memory only. Python's TOML parser avoids another runner dependency.
+const decodeProxyConfig = Schema.decodeUnknownSync(
+  Schema.Struct({ url: Schema.String, headers: Schema.Record(Schema.String, Schema.String) }),
+);
+/** Reads the `vibeproxy` CLIProxyAPI provider from `~/.codex/config.toml`. */
+export const proxyConfig = () =>
+  decodeProxyConfig(
+    JSON.parse(
+      NodeChildProcess.execFileSync(
+        "python3",
+        [
+          "-c",
+          `
+import json, os, pathlib, tomllib
+home = pathlib.Path.home() / '.codex'
+p = tomllib.loads((home / 'config.toml').read_text())['model_providers']['vibeproxy']
+h = p.get('http_headers', {}).copy()
+h.update({k: os.environ[v] for k,v in p.get('env_http_headers', {}).items()})
+key = os.environ[p['env_key']] if p.get('env_key') else p.get('experimental_bearer_token')
+if not key and p.get('requires_openai_auth'): key = json.loads((home / 'auth.json').read_text()).get('OPENAI_API_KEY')
+if key: h['Authorization'] = 'Bearer ' + key
+print(json.dumps({'url': p['base_url'].rstrip('/'), 'headers': h}))
+`,
+        ],
+        { encoding: "utf8" },
+      ),
+    ),
+  );

evals/test-signal/README.md

@@ -0,0 +1,23 @@
+# Test signal
+
+Runs the Macroscope `test-signal` check ([`.macroscope/check-run-agents/test-signal.md`](../../.macroscope/check-run-agents/test-signal.md)) on real and synthetic test diffs, and checks that a cheap model flags low-signal tests and leaves good ones alone. Use it to cut the check to the fewest words that still work.
+
+The model gets the check body, the PR's test diff, `read_file` and `grep` at the PR head, and `post_comment`. The wrapper prompt around the body is a stand-in for Macroscope's, which is not public.
+
+## Run
+
+```sh
+cd evals
+pnpm test-signal --out runs/baseline
+pnpm test-signal --out runs/cut --prompt /path/to/variant.md --case new-table-row
+```
+
+- Models go through the `vibeproxy` CLIProxyAPI provider in `~/.codex/config.toml`. Default models: `gpt-6.1-terra` and `gpt-6-luna`. Pass `--model` once per model to change them.
+- Flags: `--prompt <file>` (a variant of the check, default the real one), `--case <id>` (repeatable), `--samples <n>` (default 2, and a 3rd when the first 2 disagree), `--concurrency <n>` (default 6), `--reasoning <level>` (default `low`, as the check runs).
+- A bad case passes when a comment lands inside a target test. A guard case passes when the check posts nothing and replies exactly `All clear`.
+- `summary.md` gives the prompt hash and word count, then per model: bad cases caught, guards clean, errors, and tokens.
+- Cost: proxy quota only, about 0.5M tokens for a full run on both models.
+
+## Add a case
+
+Add an entry to `cases.ts`. A `real` case names a merged commit and its test paths. A `synthetic` case inserts one test after an anchor string in a real file. Bad cases list the titles of the tests to flag.

evals/test-signal/cases.ts

@@ -0,0 +1,177 @@
+/**
+ * Pull request test diffs for the test-signal check. A `real` case replays test files from a
+ * merged PR. A `synthetic` case inserts one test into a real file at `rev`, for a pattern with no
+ * clean real example.
+ */
+export type Case = {
+  readonly id: string;
+  readonly kind: "bad" | "guard";
+  readonly source: string;
+  readonly rev: string;
+  /** Titles of the tests the check must flag. Guard cases expect no comment and `All clear`. */
+  readonly targets: ReadonlyArray<{ readonly path: string; readonly title: string }>;
+} & (
+  | { readonly type: "real"; readonly paths: ReadonlyArray<string> }
+  | {
+      readonly type: "synthetic";
+      readonly path: string;
+      readonly after: string;
+      readonly insert: string;
+    }
+);
+
+const base = "cb7e3814d71c8fcdc5b803c660c29b7d7e39be96";
+
+export const cases: ReadonlyArray<Case> = [
+  {
+    id: "copy-text-at-every-call-site",
+    kind: "bad",
+    source: "#23999 checks the PR or MR label in 11 files",
+    type: "real",
+    rev: "218e5c09df",
+    paths: [
+      "packages/client-runtime/src/state/gitActions.test.ts",
+      "apps/web/src/lib/composerContextRecords.test.ts",
+      "apps/mobile/src/lib/composerContext.test.ts",
+    ],
+    targets: [
+      {
+        path: "packages/client-runtime/src/state/gitActions.test.ts",
+        title: "says PR for a GitHub ref",
+      },
+      {
+        path: "packages/client-runtime/src/state/gitActions.test.ts",
+        title: "says MR for a GitLab ref",
+      },
+      {
+        path: "packages/client-runtime/src/state/gitActions.test.ts",
+        title: "names the host's change request in the default-branch dialog",
+      },
+      {
+        path: "apps/web/src/lib/composerContextRecords.test.ts",
+        title: "names the change request the way its host does",
+      },
+      {
+        path: "apps/mobile/src/lib/composerContext.test.ts",
+        title: "describes the change request with its host's term",
+      },
+    ],
+  },
+  {
+    id: "same-case-in-wrapper-test",
+    kind: "bad",
+    source: "#22452 added the same two cases to threadAccess and OrchestratorMcpService",
+    type: "real",
+    rev: "b260a7cf69",
+    paths: [
+      "apps/server/src/mcp/threadAccess.test.ts",
+      "apps/server/src/mcp/OrchestratorMcpService.test.ts",
+    ],
+    // Either copy counts: the check must notice the duplicate. Which copy it keeps is secondary.
+    targets: [
+      {
+        path: "apps/server/src/mcp/threadAccess.test.ts",
+        title: "points a bare uuid at the mcp: thread it belongs to",
+      },
+      {
+        path: "apps/server/src/mcp/threadAccess.test.ts",
+        title: "keeps the plain message for an id that matches nothing",
+      },
+      {
+        path: "apps/server/src/mcp/OrchestratorMcpService.test.ts",
+        title: "points a bare uuid at the mcp: thread it belongs to",
+      },
+      {
+        path: "apps/server/src/mcp/OrchestratorMcpService.test.ts",
+        title: "keeps the plain message for an id that matches nothing",
+      },
+    ],
+  },
+  {
+    id: "new-it-for-a-table-row",
+    kind: "bad",
+    source: "Synthetic: a new `it` next to the table that already covers it",
+    type: "synthetic",
+    rev: base,
+    path: "packages/shared/src/markdownLinks.test.ts",
+    after: "    expect(parseMarkdownFileLink(href)?.path).toBe(path);\n  });\n",
+    insert:
+      '\n  it("recognizes a Windows path with forward slashes as a file", () => {\n    expect(parseMarkdownFileLink("C:/repo/src/app.ts")?.path).toBe("C:/repo/src/app.ts");\n  });\n',
+    targets: [
+      {
+        path: "packages/shared/src/markdownLinks.test.ts",
+        title: "recognizes a Windows path with forward slashes as a file",
+      },
+    ],
+  },
+  {
+    id: "mock-call-mirror",
+    kind: "bad",
+    source: "Synthetic: asserts the mock call the code makes, already covered above it",
+    type: "synthetic",
+    rev: base,
+    path: "packages/client-runtime/src/state/threadRequests.test.ts",
+    after: "    expect(send).toHaveBeenCalledOnce();\n  });\n",
+    insert:
+      '\n  it("calls respond with decline", async () => {\n    const respond = vi.fn<ApprovalResponder>(async () => AsyncResult.success(undefined));\n    await declinePendingApprovals([approval], respond);\n    expect(respond).toHaveBeenCalledWith(requestId, "decline");\n    expect(respond).toHaveBeenCalledTimes(1);\n  });\n',
+    targets: [
+      {
+        path: "packages/client-runtime/src/state/threadRequests.test.ts",
+        title: "calls respond with decline",
+      },
+    ],
+  },
+  {
+    id: "type-guaranteed",
+    kind: "bad",
+    source: "Synthetic: checks a return type the signature already declares",
+    type: "synthetic",
+    rev: base,
+    path: "packages/shared/src/markdownLinks.test.ts",
+    after: "    expect(isMarkdownFileLinkLabel(label, href)).toBe(expected);\n  });\n",
+    insert:
+      '\n  it("returns a boolean", () => {\n    expect(typeof isMarkdownFileLinkLabel("a.ts", "/repo/a.ts")).toBe("boolean");\n  });\n',
+    targets: [{ path: "packages/shared/src/markdownLinks.test.ts", title: "returns a boolean" }],
+  },
+  {
+    id: "deadlock-race-fix",
+    kind: "guard",
+    source: "#22094 run metrics read lineage inside the transaction so a writer cannot deadlock",
+    type: "real",
+    rev: "f8e4834861",
+    paths: ["apps/server/src/orchestration-v2/RunMetrics.test.ts"],
+    targets: [],
+  },
+  {
+    id: "clone-retry-fix",
+    kind: "guard",
+    source: "#23170 clones retry over HTTPS when the SSH key is refused",
+    type: "real",
+    rev: "a49d18cf3b",
+    paths: ["apps/server/src/sourceControl/SourceControlRepositoryService.test.ts"],
+    targets: [],
+  },
+  {
+    id: "deleted-tests",
+    kind: "guard",
+    source: "#22592 pruned low-value tests",
+    type: "real",
+    rev: "a2f4df0412",
+    paths: [
+      "apps/server/src/workspace/WorkspaceEntries.test.ts",
+      "apps/server/src/workspace/WorkspacePaths.test.ts",
+    ],
+    targets: [],
+  },
+  {
+    id: "new-table-row",
+    kind: "guard",
+    source: "Synthetic: the right way to add the Windows path case",
+    type: "synthetic",
+    rev: base,
+    path: "packages/shared/src/markdownLinks.test.ts",
+    after: '    ["file://localhost/home/me/notes.md", "/home/me/notes.md"],\n',
+    insert: '    ["C:/repo/src/app.ts", "C:/repo/src/app.ts"],\n',
+    targets: [],
+  },
+];