From 74f3052273f150f20996b7dbfaa503683b6eb085 Mon Sep 17 00:00:00 2001 From: Sam Julien Date: Wed, 28 Oct 2026 10:55:53 -0700 Subject: [PATCH 1/2] test: remove ineffective and non-user-facing coverage Audit 811 test-bearing files and remove 169 declarations that only check test infrastructure or source spellings, cannot distinguish relevant regressions, or do not exercise their stated behavior. Preserve runtime and configuration coverage, including the named broker matrix cases. Document five surviving mutation probes, removal reasons and verification limits in test-audit.md. --- agent-adk/tests/test_model_spec.py | 5 - agent-ag2/tests/test_model_spec.py | 5 - agent-agno/tests/test_model_spec.py | 5 - agent-antigravity/tests/test_model_spec.py | 20 - agent-computer/tests/aria-snapshot.test.ts | 10 - agent-computer/tests/viewer.test.ts | 25 - agent-crewai/tests/test_model_spec.py | 5 - agent-crewai/tests/test_token_restoration.py | 175 --- agent-langgraph-agui/tests/test_model_spec.py | 5 - agent-langroid/tests/test_model_spec.py | 5 - agent-llamaindex/tests/test_model_spec.py | 5 - agent-microsoft/tests/test_model_spec.py | 5 - agent-pydantic-ai/tests/test_model_spec.py | 5 - agent-strands/tests/test_model_spec.py | 5 - .../admin-plugins-vocabulary-wiring.test.ts | 144 -- app/tests/arcade-no-browser-calls.test.ts | 63 - app/tests/composer-queue-attachments.test.tsx | 40 +- app/tests/form-submit-controls.test.ts | 105 -- app/tests/plugin-mutation-refresh.test.ts | 153 --- app/tests/search-text.test.ts | 34 +- app/tests/theme-preference.test.ts | 25 - app/tests/transcript-attachments.test.tsx | 14 +- bunfig.toml | 2 +- desktop/src-tauri/src/engine.rs | 17 - desktop/src-tauri/src/main.rs | 38 - desktop/src-tauri/src/plan.rs | 13 - desktop/src-tauri/src/stack.rs | 11 - desktop/src-tauri/src/test_support.rs | 25 - scripts/mutation-register.test.ts | 94 -- scripts/test-ci.test.ts | 101 -- server/tests/agent-escalation.test.ts | 19 - server/tests/agent-handoff.test.ts | 11 - server/tests/agent-profile-policy.test.ts | 4 - server/tests/arcade-attempt-schema.test.ts | 92 -- server/tests/arcade-live.test.ts | 8 - server/tests/arcade-pagination-engine.test.ts | 10 - .../tests/arcade-transport-registry.test.ts | 18 +- .../tests/arcade-vendor-read-contract.test.ts | 44 - server/tests/arcade-verifier-notice.test.ts | 25 - server/tests/assertion-shape.test.ts | 1164 ----------------- server/tests/attachment-routes.test.ts | 34 +- server/tests/audit.test.ts | 75 -- .../broker-authorization-pairing.test.ts | 254 ---- server/tests/broker-stub-ratchet.test.ts | 262 ---- server/tests/brokered-provenance.test.ts | 133 -- server/tests/bunfig-preload.test.ts | 47 - .../channel-activity.integration.test.ts | 11 - .../tests/component-store.integration.test.ts | 13 - server/tests/composio-adapter.test.ts | 31 - server/tests/composio-broker-seam.test.ts | 43 - .../tests/composio-connection-kinds.test.ts | 104 +- server/tests/composio-connections.test.ts | 85 +- server/tests/composio-grants-for.test.ts | 37 +- server/tests/computer-client.test.ts | 46 - ...nector-tokens-never-reach-computer.test.ts | 43 - .../tests/control-refusal-end-to-end.test.ts | 16 - server/tests/cull-sweep-wiring.test.ts | 40 - server/tests/database.test.ts | 12 - .../tests/fixture-identity-distinct.test.ts | 425 ------ .../tests/jsonb-encoding.integration.test.ts | 11 - server/tests/mastra-roster.test.ts | 31 - server/tests/migration-journal.test.ts | 69 - ...pending-authorization-audit-ledger.test.ts | 794 ----------- .../tests/people-paging.integration.test.ts | 20 - server/tests/plugin-oauth.test.ts | 11 - server/tests/plugin-store.integration.test.ts | 61 - server/tests/shared-use-arcade-urls.test.ts | 8 - server/tests/sole-definition.test.ts | 553 -------- server/tests/store-scheme-sites.test.ts | 26 +- server/tests/support/database.test.ts | 34 - server/tests/support/source-scan.test.ts | 145 -- server/tests/support/source-scan.ts | 168 --- server/tests/tenant-package.test.ts | 15 - server/tests/test-network-guard.test.ts | 450 ------- server/tests/ts-expect-error-coverage.test.ts | 74 -- shared/harnesses.test.ts | 2 +- test-audit.md | 134 ++ tests/clean-checkout.test.ts | 53 +- tests/compose.test.ts | 17 - tests/workspace.test.ts | 10 - 80 files changed, 160 insertions(+), 6791 deletions(-) delete mode 100644 agent-crewai/tests/test_token_restoration.py delete mode 100644 app/tests/admin-plugins-vocabulary-wiring.test.ts delete mode 100644 app/tests/arcade-no-browser-calls.test.ts delete mode 100644 app/tests/form-submit-controls.test.ts delete mode 100644 scripts/mutation-register.test.ts delete mode 100644 scripts/test-ci.test.ts delete mode 100644 server/tests/arcade-attempt-schema.test.ts delete mode 100644 server/tests/assertion-shape.test.ts delete mode 100644 server/tests/broker-authorization-pairing.test.ts delete mode 100644 server/tests/broker-stub-ratchet.test.ts delete mode 100644 server/tests/brokered-provenance.test.ts delete mode 100644 server/tests/bunfig-preload.test.ts delete mode 100644 server/tests/cull-sweep-wiring.test.ts delete mode 100644 server/tests/database.test.ts delete mode 100644 server/tests/fixture-identity-distinct.test.ts delete mode 100644 server/tests/pending-authorization-audit-ledger.test.ts delete mode 100644 server/tests/sole-definition.test.ts delete mode 100644 server/tests/support/database.test.ts delete mode 100644 server/tests/support/source-scan.test.ts delete mode 100644 server/tests/support/source-scan.ts delete mode 100644 server/tests/test-network-guard.test.ts delete mode 100644 server/tests/ts-expect-error-coverage.test.ts create mode 100644 test-audit.md diff --git a/agent-adk/tests/test_model_spec.py b/agent-adk/tests/test_model_spec.py index 0a54e5cc5b..c001224035 100644 --- a/agent-adk/tests/test_model_spec.py +++ b/agent-adk/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-adk")' in source diff --git a/agent-ag2/tests/test_model_spec.py b/agent-ag2/tests/test_model_spec.py index 00aa3a0d02..b8d280c997 100644 --- a/agent-ag2/tests/test_model_spec.py +++ b/agent-ag2/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-ag2")' in source diff --git a/agent-agno/tests/test_model_spec.py b/agent-agno/tests/test_model_spec.py index df4ce3615a..1ef9da4d4c 100644 --- a/agent-agno/tests/test_model_spec.py +++ b/agent-agno/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-5.5") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-agno")' in source diff --git a/agent-antigravity/tests/test_model_spec.py b/agent-antigravity/tests/test_model_spec.py index 0313507d5e..e4efc2ea59 100644 --- a/agent-antigravity/tests/test_model_spec.py +++ b/agent-antigravity/tests/test_model_spec.py @@ -40,23 +40,3 @@ def test_the_model_is_still_the_deployments_to_choose(monkeypatch): assert bot_settings(BOT_ID, pinned_provider=PINNED) == BotSettings( provider="google", model="gemini-3.8-flash" ) - - -def test_this_bots_source_reads_the_spec_with_the_pin(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert 'bot_settings("agent-antigravity", pinned_provider="google")' in source - - -def test_the_key_and_base_url_come_from_the_provider_row(): - """Not hardcoded here. - - `shared/model-providers.json` names Google's key and base-url variables, and a Bot that - spelled them out itself would drift from the file the moment either changed. The base-url - variable is what points this Bot at a Gemini-compatible endpoint — a gateway, or aimock — - instead of Google. - """ - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert '"key_variable"' in source - assert '"base_url_variable"' in source - assert "GOOGLE_API_KEY" not in source - assert "GOOGLE_GENERATIVE_AI_BASE_URL" not in source diff --git a/agent-computer/tests/aria-snapshot.test.ts b/agent-computer/tests/aria-snapshot.test.ts index 79d93d5037..5a06e3e375 100644 --- a/agent-computer/tests/aria-snapshot.test.ts +++ b/agent-computer/tests/aria-snapshot.test.ts @@ -46,12 +46,6 @@ const CAPTURED = `- generic [ref=e2]: - button "Submit order" [ref=e44]`; describe("parseAriaSnapshot, against captured output", () => { - test("the fixture is genuinely valid YAML", () => { - // The guard against the mistake that made this rewrite necessary: an invented fixture will most - // likely fail here first. - expect(() => Bun.YAML.parse(CAPTURED)).not.toThrow(); - }); - test("keeps the controls and drops the scaffolding", () => { const { elements } = parseAriaSnapshot(CAPTURED); // `generic`, `paragraph`, `group` and `text` all carry refs but are not things a Bot can act on, @@ -132,10 +126,6 @@ describe("parseAriaSnapshot, against captured output", () => { - checkbox "Extra Cheese" [ref=e9] - text: Extra Cheese`; - test("the mixed fixture is genuinely valid YAML", () => { - expect(() => Bun.YAML.parse(MIXED)).not.toThrow(); - }); - test("a half-ticked box is not reported as ticked", () => { const byName = new Map( parseAriaSnapshot(MIXED).elements.map((e) => [e.name, e]), diff --git a/agent-computer/tests/viewer.test.ts b/agent-computer/tests/viewer.test.ts index 9e26f134c7..adf2697f62 100644 --- a/agent-computer/tests/viewer.test.ts +++ b/agent-computer/tests/viewer.test.ts @@ -161,19 +161,6 @@ describe("a close that lands while the browser is still starting", () => { expect(claim.setFollow(() => {})).toBe(false); }); - test("the claim is released even when the launch threw", async () => { - // `open`'s catch closes the socket after sending an error frame. If nothing released the claim - // there, the slot would stay occupied forever and `forgetIdle` could never sweep the - // session, which is the unbounded growth it exists to stop. - const slot = createViewerSlot(); - const socket = { id: "a" }; - slot.claim(socket, () => {}); - - await slot.release(socket); - - expect(slot.occupied()).toBe(false); - }); - test("releasing the same socket twice is harmless", async () => { // The catch path closes the socket, so `close` runs release a second time for the same socket. const slot = createViewerSlot(); @@ -448,18 +435,6 @@ describe("the session sweep asking whether anybody is watching", () => { expect(slot.occupied()).toBe(true); }); - test("a Bot stays watched until teardown has finished", async () => { - const slot = createViewerSlot(); - const socket = { id: "a" }; - const claim = slot.claim(socket, () => {}); - await claim.install(fakeCast()); - - const releasing = slot.release(socket); - await releasing; - - expect(slot.occupied()).toBe(false); - }); - test("a Bot is still watched while its cast is stopping", async () => { // Occupancy has to count the teardown itself, not just whether somebody holds the slot. The // sweep drops sessions with nobody watching and no live browser, and a session dropped while its diff --git a/agent-crewai/tests/test_model_spec.py b/agent-crewai/tests/test_model_spec.py index 74c43fe9eb..af49e69a78 100644 --- a/agent-crewai/tests/test_model_spec.py +++ b/agent-crewai/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-5.5") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-crewai")' in source diff --git a/agent-crewai/tests/test_token_restoration.py b/agent-crewai/tests/test_token_restoration.py deleted file mode 100644 index 6500bce3c3..0000000000 --- a/agent-crewai/tests/test_token_restoration.py +++ /dev/null @@ -1,175 +0,0 @@ -"""Exercise the role test's real pytest teardown in an isolated process.""" - -import argparse -import ipaddress -import json -import os -import socket -import subprocess -import sys -import threading -import time -from pathlib import Path - -import httpx -import pytest -import uvicorn - - -ROLE_TEST = "test_crewai_endpoint_preserves_leading_bot_role_for_provider" - - -def isolated_environment(directory): - directory.mkdir(parents=True, exist_ok=True) - inherited = ( - "HOME", "USERPROFILE", "HOMEDRIVE", "HOMEPATH", "CODEX_HOME", - "PATH", "LANG", "SYSTEMROOT", "WINDIR", - ) - environment = {name: os.environ[name] for name in inherited if name in os.environ} - environment.update( - OTEL_SDK_DISABLED="true", - CREWAI_TELEMETRY_DISABLED="true", - CREWAI_STORAGE_DIR=str(directory / "crewai"), - LITELLM_LOCAL_MODEL_COST_MAP="True", - PYTEST_DISABLE_PLUGIN_AUTOLOAD="1", - PYTHONPYCACHEPREFIX=str(directory / "pycache"), - TMPDIR=str(directory), - ) - return environment - - -def prohibit_external_connections(event, arguments): - if event == "socket.getaddrinfo": - host = arguments[0] - elif event in ("socket.connect", "socket.sendto"): - address = arguments[1] - if not isinstance(address, tuple): - raise RuntimeError("Only loopback TCP/IP is allowed in this probe") - host = address[0] - else: - return - if host != "localhost" and not ipaddress.ip_address(host).is_loopback: - raise RuntimeError("External network access is prohibited in this probe") - - -class RoleTestLifecycle: - def __init__(self, failure): - self.failure = failure - self.role_assertion_passed = False - self.phases = {} - - def pytest_collection_modifyitems(self, items): - assert len(items) == 1 and items[0].name == ROLE_TEST - original = items[0].obj - - def exercise_role_test(monkeypatch): - original(monkeypatch) - self.role_assertion_passed = True - if self.failure == "assertion": - assert False, "injected assertion after role test" - if self.failure == "exception": - raise RuntimeError("injected exception after role test") - - items[0].obj = exercise_role_test - - def pytest_runtest_logreport(self, report): - self.phases[report.when] = report.outcome - - -def auth_statuses(app, header): - """Use real loopback HTTP after pytest has returned in this same process.""" - with socket.socket() as listener: - listener.bind(("127.0.0.1", 0)) - port = listener.getsockname()[1] - server = uvicorn.Server(uvicorn.Config(app, log_level="error", lifespan="off")) - thread = threading.Thread(target=server.run, kwargs={"sockets": [listener]}) - thread.start() - try: - deadline = time.monotonic() + 10 - while not server.started and thread.is_alive() and time.monotonic() < deadline: - time.sleep(0.01) - assert server.started, "loopback app server did not start" - with httpx.Client(base_url=f"http://127.0.0.1:{port}", timeout=5, trust_env=False) as client: - return { - "health": client.get("/health").status_code, - "previous": client.post("/", headers={header: "synthetic-original-token"}, json={}).status_code, - "test": client.post("/", headers={header: "test-token"}, json={}).status_code, - "missing": client.post("/", json={}).status_code, - } - finally: - server.should_exit = True - thread.join(timeout=10) - assert not thread.is_alive(), "loopback app server did not stop" - - -def run_probe(source_root, output, initial_token, failure): - sys.addaudithook(prohibit_external_connections) - expected = "synthetic-original-token" if initial_token == "present" else None - if expected is None: - os.environ.pop("MANAGED_AGENT_TOKEN", None) - else: - os.environ["MANAGED_AGENT_TOKEN"] = expected - lifecycle = RoleTestLifecycle(failure) - target = source_root / "agent-crewai/tests/test_main.py" - code = pytest.main( - [f"{target}::{ROLE_TEST}", "-q", "-p", "no:cacheprovider"], - plugins=[lifecycle], - ) - main = sys.modules["src.main"] - result = { - "sourceRoot": str(source_root), - "initialToken": initial_token, - "failure": failure, - "pytestExit": int(code), - "phases": lifecycle.phases, - "roleAssertionPassed": lifecycle.role_assertion_passed, - "restored": os.environ.get("MANAGED_AGENT_TOKEN") == expected, - "leakedTestToken": os.environ.get("MANAGED_AGENT_TOKEN") == "test-token", - "http": auth_statuses(main.app, main.TOKEN_HEADER), - } - output.write_text(json.dumps(result, indent=2) + "\n") - assert code == (0 if failure == "none" else 1), result - assert lifecycle.role_assertion_passed, result - assert lifecycle.phases == { - "setup": "passed", - "call": "passed" if failure == "none" else "failed", - "teardown": "passed", - }, result - assert result["restored"], "MANAGED_AGENT_TOKEN was not restored after pytest teardown" - assert result["http"] == { - "health": 200, - "previous": 422 if initial_token == "present" else 401, - "test": 401, - "missing": 401, - }, result - - -@pytest.mark.parametrize("initial_token", ["present", "absent"]) -@pytest.mark.parametrize("failure", ["none", "assertion", "exception"]) -def test_role_test_restores_token_after_pytest_teardown(tmp_path, initial_token, failure): - output = tmp_path / "result.json" - result = subprocess.run( - [ - sys.executable, str(Path(__file__).resolve()), - "--source-root", str(Path(__file__).resolve().parents[2]), - "--output", str(output), - "--initial-token", initial_token, - "--failure", failure, - ], - cwd=tmp_path, - env=isolated_environment(tmp_path), - text=True, - capture_output=True, - timeout=90, - ) - assert result.returncode == 0, result.stdout + result.stderr - - -if __name__ == "__main__": - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--source-root", type=Path, required=True) - parser.add_argument("--output", type=Path, required=True) - parser.add_argument("--initial-token", choices=["present", "absent"], required=True) - parser.add_argument("--failure", choices=["none", "assertion", "exception"], default="none") - arguments = parser.parse_args() - run_probe(arguments.source_root.resolve(), arguments.output, arguments.initial_token, arguments.failure) diff --git a/agent-langgraph-agui/tests/test_model_spec.py b/agent-langgraph-agui/tests/test_model_spec.py index a6e9d3c1df..4c858c9ab3 100644 --- a/agent-langgraph-agui/tests/test_model_spec.py +++ b/agent-langgraph-agui/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-langgraph-agui")' in source diff --git a/agent-langroid/tests/test_model_spec.py b/agent-langroid/tests/test_model_spec.py index 6a2d6ce1cc..50a6bb5b8a 100644 --- a/agent-langroid/tests/test_model_spec.py +++ b/agent-langroid/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-langroid")' in source diff --git a/agent-llamaindex/tests/test_model_spec.py b/agent-llamaindex/tests/test_model_spec.py index b3ecc171d6..70364f232e 100644 --- a/agent-llamaindex/tests/test_model_spec.py +++ b/agent-llamaindex/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-5.5") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-llamaindex")' in source diff --git a/agent-microsoft/tests/test_model_spec.py b/agent-microsoft/tests/test_model_spec.py index 2e775bc471..d593b0a870 100644 --- a/agent-microsoft/tests/test_model_spec.py +++ b/agent-microsoft/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-microsoft")' in source diff --git a/agent-pydantic-ai/tests/test_model_spec.py b/agent-pydantic-ai/tests/test_model_spec.py index 5e6f161b96..7657d44fa0 100644 --- a/agent-pydantic-ai/tests/test_model_spec.py +++ b/agent-pydantic-ai/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-pydantic-ai")' in source diff --git a/agent-strands/tests/test_model_spec.py b/agent-strands/tests/test_model_spec.py index 6961425f3a..71853ec8c8 100644 --- a/agent-strands/tests/test_model_spec.py +++ b/agent-strands/tests/test_model_spec.py @@ -30,8 +30,3 @@ def test_blank_environment_falls_through_to_the_file(monkeypatch): monkeypatch.setenv("BOT_PROVIDER", "") monkeypatch.setenv("BOT_MODEL", " ") assert bot_settings(BOT_ID) == BotSettings(provider="openai", model="gpt-4o-mini") - - -def test_this_bots_source_reads_the_spec(): - source = (Path(__file__).resolve().parents[1] / "src" / "main.py").read_text(encoding="utf-8") - assert f'bot_settings("agent-strands")' in source diff --git a/app/tests/admin-plugins-vocabulary-wiring.test.ts b/app/tests/admin-plugins-vocabulary-wiring.test.ts deleted file mode 100644 index f242a52aeb..0000000000 --- a/app/tests/admin-plugins-vocabulary-wiring.test.ts +++ /dev/null @@ -1,144 +0,0 @@ -// app/tests/admin-plugins-vocabulary-wiring.test.ts -/** - * Guard: the admin plugins pages CALL the provider vocabulary; they do not - * re-hardcode a vendor name. - * - * WHY: arcade-admin-plugins.test.tsx pins every string the vocabulary - * functions return, but nothing pinned that `index.tsx` and `$key.tsx` actually - * call them. A guard on return values without one on call sites reads as - * coverage while protecting nothing: a later edit can paste "Composio" straight - * back into the JSX, every vocabulary test stays green, and an Arcade - * deployment shows a Composio-named screen (spec D11: screens show the ACTIVE - * provider only; this PR changes no Composio behaviour). It is the same - * argument the server-side vendor-name guard makes about catching one spelling - * of a mistake and not the other. - * - * Reads the route sources as TEXT (like arcade-no-browser-calls.test.ts) and - * imports nothing from them. Comments are stripped first, so docblocks may - * still explain the vendor. Then: - * 1. NO route file under the plugins directory contains a capitalised - * "Composio" NOR the all-caps "COMPOSIO" (string literal, JSX text or test - * id). `composio.tsx` USED to be exempt, on the argument that the route is - * Composio-gated by construction. The screen it held has moved to - * `components/plugins/broker-directory.tsx`, so the file holds no vendor - * literal and the exemption has no reason left — and an exemption kept - * after its reason has gone is an escape hatch waiting to be used a second - * time. - * The rule scans EVERY route file (`ALL_ROUTES`, enumerated from the - * directory), not a hand-kept list: an un-scanned sibling is the same - * escape hatch by omission, and `arcade.tsx` — the likeliest place for - * a pasted "Composio" — would have been one. - * 2. NO route file contains a hard-coded `/admin/plugins/composio` or - * `/admin/plugins/arcade` navigation target. THIS IS THE LOWERCASE - * SPELLING RULE (1) CANNOT SEE, and it is the one that stranded the - * enablement notes: `index.tsx` linked every deployment, Arcade ones - * included, at the Composio directory, which redirected them straight - * back — so two measured warnings rendered on no deployment that exists. - * The path belongs in `providerVocabulary().directoryPath`, whose literal - * union is checked against the generated route tree. Each file's own - * `createFileRoute("/_authed/admin/plugins/…")` id is not a navigation - * target and is excluded by the `_authed` prefix. - * 3. each vocabulary function appears in the file that should call it. - * - * If this goes red, route the string through provider-vocabulary.ts; do not - * loosen the guard. The scanned-count and file-length assertions stop it - * passing vacuously. - */ -import { expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; -import { - logicalLines, - sourceFiles, - stripComments, -} from "../../server/tests/support/source-scan"; - -const DIR = join(import.meta.dir, "../src/routes/_authed/admin/plugins"); - -function load(name: string): string { - return stripComments(readFileSync(join(DIR, name), "utf8")); -} - -/** - * Every route file under the directory, so a new one is scanned the day it lands — and a new one - * in a SUBdirectory too, which a `readdirSync` of the top level would have walked straight past. - * Names are relative to `DIR`, so a top-level route still reads as `arcade.tsx`. - */ -const ALL_ROUTES = sourceFiles(DIR, [".tsx"]).map((p) => - p.slice(DIR.length + 1), -); - -const VOCAB_CALLERS: Record = { - "index.tsx": [ - "moreAppsRow", - "moreAppsSectionDescription", - "providerVocabulary", - ], - "$key.tsx": [ - "brokeredConnectionDescription", - "brokeredReach", - "brokeredDisconnectNote", - "brokeredProvenance", - ], - "composio.tsx": ["directoryRouteDecision"], - "arcade.tsx": ["directoryRouteDecision"], -}; - -// Self-extending, like rule 2: a hand-kept list is an un-scanned sibling waiting to happen, and -// the Arcade route is the likeliest place for a "Composio" literal to be pasted into. -const NO_VENDOR_LITERAL = ALL_ROUTES; - -test("the plugins routes carry no Composio literal", () => { - // Every route file in the directory is scanned (no exemptions; arcade.tsx and the $key_.* - // children included). The floor and the two named files keep the scan from passing on an - // empty or truncated list. - expect(NO_VENDOR_LITERAL.length).toBeGreaterThanOrEqual(6); - expect(NO_VENDOR_LITERAL).toContain("arcade.tsx"); - expect(NO_VENDOR_LITERAL).toContain("composio.tsx"); - const offenders: string[] = []; - for (const name of NO_VENDOR_LITERAL) { - const text = load(name); - expect(text.length).toBeGreaterThan(500); - for (const { line: at, text: line } of logicalLines(text)) { - // Capitalised AND all-caps. `COMPOSIO_API_KEY` is a user-visible string 1.8 moved - // into the vocabulary (`envVar`), rendered as "Set COMPOSIO_API_KEY on this - // deployment." — it must read ARCADE_API_KEY on an Arcade deployment, and an - // env-var name is exactly the string an editor inlines as a constant. NOT - // case-insensitive: `composioConfigured` and the `/_authed/admin/plugins/composio` - // route id are legitimate and must stay legal. - if (/Composio|COMPOSIO/.test(line)) - offenders.push(`${name}:${at}: ${line.trim()}`); - } - } - expect(offenders).toEqual([]); -}); - -test("no plugins route hard-codes a directory path", () => { - expect(ALL_ROUTES.length).toBeGreaterThanOrEqual(6); - expect(ALL_ROUTES).toContain("arcade.tsx"); - expect(ALL_ROUTES).toContain("composio.tsx"); - const offenders: string[] = []; - for (const name of ALL_ROUTES) { - const text = load(name); - expect(text.length).toBeGreaterThan(500); - for (const { line: at, text: line } of logicalLines(text)) { - // The route's own file id carries `_authed` and is not somewhere anybody navigates. - if (/(? { - expect(Object.keys(VOCAB_CALLERS).length).toBe(4); - const missing: string[] = []; - for (const [name, fns] of Object.entries(VOCAB_CALLERS)) { - const text = load(name); - for (const fn of fns) { - if (!new RegExp(`\\b${fn}\\(`).test(text)) - missing.push(`${name} -> ${fn}`); - } - } - expect(missing).toEqual([]); -}); diff --git a/app/tests/arcade-no-browser-calls.test.ts b/app/tests/arcade-no-browser-calls.test.ts deleted file mode 100644 index c1f2df5719..0000000000 --- a/app/tests/arcade-no-browser-calls.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -// app/tests/arcade-no-browser-calls.test.ts -/** - * Guard: no browser code under `app/src` ever dials Arcade. - * - * WHY: the vendor key never leaves the server and the browser never talks to - * Arcade (spec D25). The grant-status route exists precisely so a browser asks - * OpenBot rather than the vendor. A direct dial from `app/` would either need - * the deployment's key in the bundle or would leak which identities have flows - * outstanding. - * - * Flags any line that imports `@arcadeai/arcadejs` (static, dynamic `import(`, - * or `require(`), names an `arcade.dev` host, or names an Arcade API path. - * - * NOT AN OFFENCE: an `arcade://` url string. That is a brokered row's url, as - * the server hands it to the browser, and is not a network target. It is - * excluded explicitly below (and does not match the host pattern anyway); do - * not widen the host pattern to bare "arcade" or this guard starts crying wolf - * and gets weakened. - * - * EXPECTED GREEN. Unlike brokered-provenance.test.ts (committed RED as a - * checklist), no browser Arcade code exists, so this passes from the start. - * If it goes red, remove the offending browser code; do not loosen the guard. - * The scan-count assertion stops it passing vacuously against an empty walk. - */ -import { expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; -import { - logicalLines, - sourceFiles, -} from "../../server/tests/support/source-scan"; - -const SRC_DIR = join(import.meta.dir, "../src"); - -const PATTERNS: RegExp[] = [ - /@arcadeai\/arcadejs/, - /\barcade\.dev\b/, - /\/v1\/tools\/execute/, - /\/v1\/tools\/authorize/, - /oauth\/confirm_user/, -]; - -// `arcade://` is a row url, not a dial. Blanked before matching. -const ROW_URL = /arcade:\/\/\S*/g; - -test("no browser code under app/src dials Arcade", () => { - // Enumeration and line-splitting come from the shared scanner, so the walk is recursive (a - // future `src//` is scanned the day it lands) and a dial prettier wrapped across two - // physical lines is one line to the patterns. Comments are deliberately NOT stripped: a - // docblock naming `arcade.dev` is still this guard's business. - const files = sourceFiles(SRC_DIR, [".ts", ".tsx"]); - expect(files.length).toBeGreaterThan(50); - const offenders: string[] = []; - for (const file of files) { - for (const { line, text } of logicalLines(readFileSync(file, "utf8"))) { - const scanned = text.replace(ROW_URL, ""); - if (PATTERNS.some((re) => re.test(scanned))) { - offenders.push(`${file.slice(SRC_DIR.length + 1)}:${line}`); - } - } - } - expect(offenders).toEqual([]); -}); diff --git a/app/tests/composer-queue-attachments.test.tsx b/app/tests/composer-queue-attachments.test.tsx index 93343834fe..291a9e6c82 100644 --- a/app/tests/composer-queue-attachments.test.tsx +++ b/app/tests/composer-queue-attachments.test.tsx @@ -7,19 +7,19 @@ import { spyOn, test, } from "bun:test"; +import * as ReactCoreV2 from "@copilotkit/react-core/v2"; import { GlobalRegistrator } from "@happy-dom/global-registrator"; import { cleanup, fireEvent, - render, type RenderResult, + render, waitFor, } from "@testing-library/react"; -import * as ReactCoreV2 from "@copilotkit/react-core/v2"; import { useCallback } from "react"; import { Composer } from "@/components/channels/composer/composer"; -import { ConversationView } from "@/components/channels/conversation-view"; import type { ComposerDraft } from "@/components/channels/composer/draft"; +import { ConversationView } from "@/components/channels/conversation-view"; import { attachmentUrl, MAX_ATTACHMENTS_PER_MESSAGE, @@ -77,15 +77,6 @@ afterAll(async () => { let removeAttachmentCalls: string[]; let consumeAttachmentsCallCount: number; -/** - * Every distinct function identity the spy has handed a rendering composer, so the test below can - * say that it handed out exactly one of each. See the note on stability under the spy itself. - */ -const handedOut = { - removeAttachment: new Set(), - consumeAttachments: new Set(), -}; - /** * A THIN SPY AROUND THE REAL HOOK, NOT A REPLACEMENT FOR IT. Every other field — `attachments`, * `processFiles`, the drag handlers — passes straight through to the real `useAttachments`, so @@ -127,9 +118,6 @@ beforeAll(() => { return hook.consumeAttachments(); }, [hook.consumeAttachments]); - handedOut.removeAttachment.add(removeAttachment); - handedOut.consumeAttachments.add(consumeAttachments); - return { ...hook, removeAttachment, consumeAttachments }; }, ); @@ -161,8 +149,6 @@ let uploads: string[]; beforeEach(() => { removeAttachmentCalls = []; consumeAttachmentsCallCount = 0; - handedOut.removeAttachment.clear(); - handedOut.consumeAttachments.clear(); deletes = []; uploads = []; global.fetch = (async (path: string, init: RequestInit) => { @@ -471,26 +457,6 @@ test("a full queue drains whole, with nothing bumped and nothing to release", as expect(uploads).toHaveLength(MAX_ATTACHMENTS_PER_MESSAGE); }); -test("the spy hands the composer one stable pair of callbacks, not a fresh pair per render", async () => { - // THE HARNESS MEASURING ITSELF, and worth the test. The SDK builds `removeAttachment` and - // `consumeAttachments` as `useCallback(…, [])` — stable for the life of the hook, deliberately — - // and `submitDraft` lists `removeAttachment` in its dependency array. A spy that mints a new - // wrapper on every render silently rebuilds that memo on every render too, so the thing under - // test would be reacting to the instrument rather than to the code. - const view = render( - {}} />, - ); - - // Several renders: the placeholder going on the strip, the upload resolving, the strip - // re-rendering without it. - await dropAndWait(view, [notes()]); - fireEvent.click(view.getByLabelText("Remove notes.txt")); - await waitFor(() => expect(chips(view)).toEqual([])); - - expect(handedOut.removeAttachment.size).toBe(1); - expect(handedOut.consumeAttachments.size).toBe(1); -}); - /** * Registered after the `afterAll` that restores, so it runs after it: the real function is back on * the module and no file bun walks into this process after this one is holding a spy. This is the diff --git a/app/tests/form-submit-controls.test.ts b/app/tests/form-submit-controls.test.ts deleted file mode 100644 index 09f05138b4..0000000000 --- a/app/tests/form-submit-controls.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { expect, test } from "bun:test"; -import { Glob } from "bun"; -import ts from "typescript"; - -/** - * Every `
` has something that submits it. - * - * Base UI's `Button` renders `type="button"` unless told otherwise, so a `, -})); +export function SidebarToggleStub() { + return ; +} // Exercise the recipient picker's creation handoff without running the separate creation flow. -mock.module("@/components/agents/new-bot-sheet", () => ({ - NewBotSheet: ({ - open, - onClose, - onCreated, - }: { - open: boolean; - onClose: () => void; - onCreated: (id: string) => void; - }) => - open ? ( -
- - -
- ) : null, -})); +export function NewBotCreationStub({ + open, + onClose, + onCreated, +}: Parameters[0]) { + return ( + <> + {open ? ( +
+ + +
+ ) : null} + + ); +} diff --git a/app/tests/channel-new-error-state.test.tsx b/app/tests/channel-new-error-state.test.tsx index 37df841b8a..d9112d6b72 100644 --- a/app/tests/channel-new-error-state.test.tsx +++ b/app/tests/channel-new-error-state.test.tsx @@ -1,6 +1,12 @@ -import "./channel-new-error-state.fixture"; - -import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + expect, + spyOn, + test, +} from "bun:test"; import { GlobalRegistrator } from "@happy-dom/global-registrator"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { @@ -13,13 +19,48 @@ import { } from "@tanstack/react-router"; import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import * as NewBotSheetModule from "@/components/agents/new-bot-sheet"; +import * as SidebarToggleModule from "@/components/layout/sidebar-toggle"; import { type AgentProfile, agentKeys } from "@/lib/agents/queries"; import { Route as ChannelNewRoute } from "@/routes/_authed/_app/channel/new"; +import { + NewBotCreationStub, + SidebarToggleStub, +} from "./channel-new-error-state.fixture"; import { settleReactWork } from "./settle-react-work"; beforeAll(() => GlobalRegistrator.register()); -afterEach(() => cleanup()); +// The shared preload supplies the Vite-only marks module before importing the real sheet. +// Namespace spies can be restored; mock.module would leak these handoff stubs into later suites. +const realNewBotSheet = NewBotSheetModule.NewBotSheet; +const realSidebarToggle = SidebarToggleModule.SidebarToggle; +let restoreComponents = () => {}; + +beforeEach(() => { + const newBotSheetSpy = spyOn( + NewBotSheetModule, + "NewBotSheet", + ).mockImplementation(NewBotCreationStub); + const sidebarToggleSpy = spyOn( + SidebarToggleModule, + "SidebarToggle", + ).mockImplementation(SidebarToggleStub); + restoreComponents = () => { + newBotSheetSpy.mockRestore(); + sidebarToggleSpy.mockRestore(); + }; +}); + +afterEach(() => { + try { + cleanup(); + } finally { + restoreComponents(); + expect(NewBotSheetModule.NewBotSheet).toBe(realNewBotSheet); + expect(SidebarToggleModule.SidebarToggle).toBe(realSidebarToggle); + } +}); afterAll(async () => { await settleReactWork(); diff --git a/app/tests/transcript-attachments.test.tsx b/app/tests/transcript-attachments.test.tsx index a7b1210de1..7474210736 100644 --- a/app/tests/transcript-attachments.test.tsx +++ b/app/tests/transcript-attachments.test.tsx @@ -14,6 +14,7 @@ import { ChatTranscript, LightboxPicture, sameAttachmentRow, + TranscriptAttachments, } from "@/components/channels/chat-transcript"; import type { QueuedMessage } from "@/components/channels/composer"; import { attachmentUrl } from "@/lib/channels/attachments"; @@ -804,6 +805,13 @@ test("a row that actually changed does not compare equal", () => { ).toBe(false); }); +// A correct comparator only protects attachment identity if the rendered row uses it. +test("the row of tiles is memoised with that comparison", () => { + expect( + (TranscriptAttachments as unknown as { compare?: unknown }).compare, + ).toBe(sameAttachmentRow); +}); + /* * A PARKED FILE IS THE ONE SURFACE IN THIS BROWSER THAT KNOWS WHAT IT IS HOLDING. * diff --git a/scripts/mutation-register.test.ts b/scripts/mutation-register.test.ts new file mode 100644 index 0000000000..3eeb0226c0 --- /dev/null +++ b/scripts/mutation-register.test.ts @@ -0,0 +1,94 @@ +// The register's STATIC half, cheap enough for every test run: every entry still finds the exact +// expression it guards, its mutant still parses, and its named suites exist. Running the mutants +// themselves is `bun run test:mutations` (scripts/mutation-register.ts) — that is the half that +// proves the suites go red, and it spawns one `bun test` per entry, so it is not run from here. +// +// Why bother with this half: an entry whose expression was refactored away fails `test:mutations` +// loudly, but only when somebody runs it. This makes the same drift fail the ordinary suite, on the +// change that caused it, while the person who moved the code still knows what it was. + +import { describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { + ANCHOR_WINDOW, + locate, + type MutationEntry, + mutate, + REGISTER, + REPO_ROOT, +} from "./mutation-register"; + +const transpilers = { + ts: new Bun.Transpiler({ loader: "ts" }), + tsx: new Bun.Transpiler({ loader: "tsx" }), +}; + +describe("every register entry", () => { + test("has a unique id", () => { + const ids = REGISTER.map((e) => e.id); + expect(ids.filter((id, i) => ids.indexOf(id) !== i)).toEqual([]); + }); + + for (const entry of REGISTER) { + test(`${entry.id}: locates its quoted expression, and its mutant parses`, () => { + const source = readFileSync(join(REPO_ROOT, entry.path), "utf8"); + expect(locate(source, entry)).not.toHaveProperty("error"); + const mutant = mutate(source, entry); + expect(mutant).not.toBe(source); + // A syntax error would make every named test "fail" without telling any distinction apart. + const transpiler = entry.path.endsWith(".tsx") + ? transpilers.tsx + : transpilers.ts; + expect(() => transpiler.transformSync(source)).not.toThrow(); + expect(() => transpiler.transformSync(mutant)).not.toThrow(); + }); + + test(`${entry.id}: names 1-3 test files that exist`, () => { + expect(entry.tests.length).toBeGreaterThanOrEqual(1); + expect(entry.tests.length).toBeLessThanOrEqual(3); + for (const file of entry.tests) { + expect(existsSync(join(REPO_ROOT, file))).toBe(true); + } + }); + } +}); + +describe("locate", () => { + const entry = (over: Partial): MutationEntry => ({ + id: "x", + kind: "identity", + path: "f.ts", + find: "a.vendorUserId", + replace: "a.userId", + tests: ["t.test.ts"], + why: "", + ...over, + }); + + test("refuses a bare expression that occurs twice, rather than picking one", () => { + const source = "f(a.vendorUserId);\ng(a.vendorUserId);"; + expect(locate(source, entry({}))).toHaveProperty("error"); + }); + + test("an anchor picks the occurrence after it", () => { + const source = "f(a.vendorUserId);\nfunction g() { h(a.vendorUserId); }"; + const found = locate(source, entry({ anchor: "function g() {" })); + expect(found).toEqual({ index: source.lastIndexOf("a.vendorUserId") }); + }); + + test("does not re-aim at a sibling past the window when the guarded line is deleted", () => { + // The guarded expression after `function g() {` is gone; the next occurrence belongs to `k`. + const source = `function g() { h(); }\n${" ".repeat(ANCHOR_WINDOW)}\nfunction k() { h(a.vendorUserId); }`; + expect(locate(source, entry({ anchor: "function g() {" }))).toHaveProperty( + "error", + ); + }); + + test("refuses an anchor that is not unique", () => { + const source = "function g() { a.vendorUserId }\nfunction g() {}"; + expect(locate(source, entry({ anchor: "function g() {" }))).toHaveProperty( + "error", + ); + }); +}); diff --git a/scripts/mutation-register.ts b/scripts/mutation-register.ts index 15283e31d0..5c866f75e1 100644 --- a/scripts/mutation-register.ts +++ b/scripts/mutation-register.ts @@ -38,8 +38,7 @@ // // KEYED BY QUOTED EXPRESSION, NEVER BY `path:line`. A line-keyed list goes red when someone rewraps // a comment three functions above the guarded code, and people learn to re-baseline it without -// reading (`assertion-shape.test.ts`'s ALLOW list collided in three consecutive waves for exactly -// this reason; `sole-definition.test.ts`, keyed by path and quoted expression, never has). An entry +// reading. An entry // names the expression itself. When the expression appears more than once in its file, `anchor` // names a second quoted expression that occurs EXACTLY ONCE, and the mutated expression is its first // occurrence after the anchor, within {@link ANCHOR_WINDOW} characters — so deleting the guarded diff --git a/scripts/test-ci.test.ts b/scripts/test-ci.test.ts new file mode 100644 index 0000000000..15d7d94886 --- /dev/null +++ b/scripts/test-ci.test.ts @@ -0,0 +1,101 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, expect, test } from "bun:test"; + +const createdDirectories: string[] = []; +const expectedTail = + "SYNTHETIC_EXPECTED_TAIL: preserve the child failure details"; + +async function createProjectWithFailingTest() { + const directory = await mkdtemp(join(tmpdir(), "openbot-test-ci-")); + createdDirectories.push(directory); + await writeFile( + join(directory, "package.json"), + JSON.stringify({ scripts: { test: "bun synthetic-child.ts" } }), + ); + await writeFile( + join(directory, "synthetic-child.ts"), + String.raw` +export {}; + +const stderr = [ + "start of child stderr", + ...Array.from( + { length: 5000 }, + (_, index) => "filler-" + String(index).padStart(5, "0") + ": " + "x".repeat(200), + ), + "SYNTHETIC_EXPECTED_TAIL: preserve the child failure details", +].join("\n") + "\n"; + +if (!process.stderr.write(stderr)) { + await new Promise((resolve) => process.stderr.once("drain", resolve)); +} +process.exitCode = 7; +`, + ); + return directory; +} + +async function runBun(directory: string, script: string) { + const proc = Bun.spawn(["bun", script], { + cwd: directory, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + return { exitCode, stderr, stdout }; +} + +async function runBunThroughLogPipe(directory: string, script: string) { + const proc = Bun.spawn( + ["bash", "-o", "pipefail", "-c", 'bun "$1" 2>&1 | cat', "bash", script], + { + cwd: directory, + stdout: "pipe", + stderr: "pipe", + }, + ); + + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + return { exitCode, stderr, stdout }; +} + +afterEach(async () => { + await Promise.all( + createdDirectories + .splice(0) + .map((directory) => rm(directory, { force: true, recursive: true })), + ); +}); + +test("preserves the child stderr tail before returning the child status", async () => { + const directory = await createProjectWithFailingTest(); + + const child = await runBun(directory, "synthetic-child.ts"); + expect(child.exitCode).toBe(7); + expect(child.stdout).toBe(""); + expect(child.stderr).toContain(expectedTail); + + const fixedWrapper = await runBunThroughLogPipe( + directory, + join(import.meta.dir, "test-ci.ts"), + ); + expect(fixedWrapper.exitCode).toBe(7); + expect(fixedWrapper.stderr).toBe(""); + expect(fixedWrapper.stdout).toContain(expectedTail); + expect(fixedWrapper.stdout).toContain( + 'error: script "test" exited with code 7', + ); +}); diff --git a/server/bunfig.toml b/server/bunfig.toml index 94e82786fd..c306cbc3ec 100644 --- a/server/bunfig.toml +++ b/server/bunfig.toml @@ -8,5 +8,5 @@ # the race described in scripts/test-preload.ts. # # Both files name the same scripts, so their contents cannot drift. What could drift is this list, and -# tests/bunfig-preload.test.ts fails if the two stop agreeing. +# server/tests/bunfig-preload.test.ts fails if the two stop agreeing. preload = ["./scripts/test-preload.ts", "../app/tests/preload.ts"] diff --git a/server/src/plugins/broker.ts b/server/src/plugins/broker.ts index d3382bc580..01f970597a 100644 --- a/server/src/plugins/broker.ts +++ b/server/src/plugins/broker.ts @@ -657,8 +657,7 @@ export type ConnectedAppBroker = { * IT PAIRS WITH {@link ConnectedAppBroker.authorizations}: a broker declaring `null` there * answers `null` here, and a broker declaring a flow answers a non-null id on every successful * mint. A broker that grew a flow and forgot the id would leave D24 with no source while every - * type still checked, so `broker-authorization-pairing.test.ts` asserts the pair over every - * broker literal in the tree. + * type still checked. A broker's authorization tests must verify this pairing. */ authorize(request: { account: AccountRef; diff --git a/server/src/plugins/plugin-store/context.ts b/server/src/plugins/plugin-store/context.ts index 6f42bf0e6d..cec7bf727c 100644 --- a/server/src/plugins/plugin-store/context.ts +++ b/server/src/plugins/plugin-store/context.ts @@ -1891,8 +1891,7 @@ export function createStoreContext(options: PluginStoreOptions) { * and that order is kept rather than moved behind the revoke for the helper's convenience. * * Answers the payload fragment rather than the boolean so that no writer computes the field - * itself: a writer spreads this, and the guard in `sole-definition.test.ts` holds that to one - * home. + * itself: a writer spreads this to keep the claim's definition in one place. */ async function vendorRevocationClaim( app: string, diff --git a/server/src/plugins/route-groups/connections.ts b/server/src/plugins/route-groups/connections.ts index ceff39ef01..72fd2f323e 100644 --- a/server/src/plugins/route-groups/connections.ts +++ b/server/src/plugins/route-groups/connections.ts @@ -1621,7 +1621,7 @@ export function mountConnectionRoutes({ * construction, the adapter's own refusals are ours, and {@link VendorTimeoutError} is ours. * * NOT AN AUDIT ROW. Nothing here writes to `pending_authorizations`, and the attempt's - * missing trail rows are already debt in `pending-authorization-audit-ledger.test.ts`, under + * missing trail rows are tracked in https://github.com/CopilotKit/OpenBot/issues/876, under * the claim and the expiry overwrite (B5e). Filing an event for this one branch while the * claim it follows files none would put half a lifecycle on the trail. */ diff --git a/server/tests/assertion-shape.test.ts b/server/tests/assertion-shape.test.ts index 948f60947e..66b74bfa8f 100644 --- a/server/tests/assertion-shape.test.ts +++ b/server/tests/assertion-shape.test.ts @@ -78,6 +78,8 @@ export type Offence = { line: number; shape: Shape; text: string; + expression: string; + context: string[]; }; function blankLiterals(text: string): string { @@ -211,10 +213,48 @@ const UNDEFINED_PASSES = /^(?:toBeUndefined|toBeFalsy|toBeNil|toBeNullish)$/; type Range = { start: number; end: number }; +/** Ignore formatting outside literals; literal contents remain part of the identity. */ +function expressionIdentity(source: string): string { + return source.replace( + /("(?:\\.|[^"\\])*"|'(?:\\.|[^'\\])*'|`(?:\\.|[^`\\])*`)|\s+/g, + (_match, literal: string | undefined) => literal ?? "", + ); +} + export function offencesIn(file: string, source: string): Offence[] { const stripped = stripComments(source); const code = blankLiterals(stripped); const found: Offence[] = []; + // Include the enclosing test/describe names so identical assertions in sibling tests + // remain distinct. Neither the name nor the expression depends on its source line. + const contexts: { start: number; end: number; name: string }[] = []; + for (const match of code.matchAll( + /(? { @@ -225,9 +265,20 @@ export function offencesIn(file: string, source: string): Offence[] { } return best.trim().replace(/\s+/g, " ").slice(0, 110); }; - const at = (index: number, shape: Shape) => { + const at = (index: number, shape: Shape, expressionStart = index) => { const line = lineOf(code, index); - found.push({ file, line, shape, text: quote(line) }); + found.push({ + file, + line, + shape, + text: quote(line), + expression: expressionIdentity( + stripped.slice(expressionStart, statementEnd(code, expressionStart)), + ), + context: contexts + .filter((c) => c.start <= index && c.end >= index) + .map((c) => c.name), + }); }; const expects: { index: number; open: number; close: number }[] = []; @@ -248,7 +299,7 @@ export function offencesIn(file: string, source: string): Offence[] { const prefix = head.slice(0, own); if (/\b(?:await|return|yield)\b/.test(prefix)) continue; if (/=>\s*$/.test(prefix)) continue; - at(start + (head.length - head.trimStart().length), "a"); + at(start + (head.length - head.trimStart().length), "a", start + own); } // (b) `?.` in the subject, under a matcher `undefined` satisfies @@ -279,8 +330,14 @@ export function offencesIn(file: string, source: string): Offence[] { const open = (m.index as number) + m[0].length - 1; const close = closerOf(code, open); if (close < 0) continue; - if (/:\s*undefined\s*[,}]/.test(code.slice(open + 1, close))) - at(m.index as number, "c"); + if (/:\s*undefined\s*[,}]/.test(code.slice(open + 1, close))) { + const subject = expects.findLast( + (e) => + e.close < m.index && + /^[\s.\w]*$/.test(code.slice(e.close + 1, m.index)), + ); + at(m.index, "c", subject?.index ?? m.index); + } } // guarded regions: the statement an `if` controls @@ -418,19 +475,20 @@ export function scanTree(root: string, from = root): Offence[] { * * One ground covers every entry below and no other ground is accepted: the offence predates this * guard, so it is fix-cycle work rather than a licence. `finding` names the review that found it, - * or marks it as one this scan turned up that the review rounds did not. `why` quotes the - * expression, so an entry can be read without opening the file. + * or marks it as one this scan turned up that the review rounds did not. `expression` quotes the full + * expression, and `context` names its enclosing tests. Formatting and line shifts do not change + * that identity; an assertion in a different test is never grandfathered by it. */ type Allowed = { file: string; - line: number; + expression: string; + context: string[]; shape: Shape; finding: string; - why: string; }; -const key = (o: { file: string; line: number; shape: Shape }) => - `${o.file}:${o.line}:${o.shape}`; +const key = (o: Pick) => + JSON.stringify([o.file, o.shape, o.context, o.expression]); /** * THE STANDING OFFENDERS, as of the commit that added this file. @@ -450,505 +508,686 @@ const key = (o: { file: string; line: number; shape: Shape }) => const ALLOW: Allowed[] = [ { file: "app/tests/auth-client.test.ts", - line: 83, shape: "a", + context: ['"signInWith"', '"throws what the client said when it refuses"'], + expression: + 'expect(signInWith("okta",refuse)).rejects.toThrow("That provider is not configured.",)', finding: "round 2 — auth-client.test.ts, six sites", - why: 'expect(signInWith("okta", refuse)).rejects.toThrow( "That provider is not configured.", );', }, { file: "app/tests/auth-client.test.ts", - line: 97, shape: "a", + context: [ + '"signInWith"', + '"names the provider when the client says nothing"', + ], + expression: + 'expect(signInWith("microsoft",refuse)).rejects.toThrow("Microsoft")', finding: "round 2 — auth-client.test.ts, six sites", - why: 'test("names the provider when the client says nothing", async () => { const refuse = async () => ({ error: {} ', }, { file: "app/tests/auth-client.test.ts", - line: 101, shape: "a", + context: [ + '"signInWith"', + '"resolves quietly when the redirect is under way"', + ], + expression: + 'expect(signInWith("google",async()=>({error:null})),).resolves.toBeUndefined()', finding: "round 2 — auth-client.test.ts, six sites", - why: 'test("resolves quietly when the redirect is under way", async () => { expect( signInWith("google", async () =>', }, { file: "app/tests/auth-client.test.ts", - line: 162, shape: "a", + context: [ + '"signInWithEmailDomain"', + '"throws what the client said when it refuses"', + ], + expression: + 'expect(signInWithEmailDomain("someone@company.com",refuse),).rejects.toThrow("That domain is registered to another tenant.")', finding: "round 2 — auth-client.test.ts, six sites", - why: 'expect( signInWithEmailDomain("someone@company.com", refuse), ).rejects.toThrow("That domain is registered to ', }, { file: "app/tests/auth-client.test.ts", - line: 170, shape: "a", + context: [ + '"signInWithEmailDomain"', + '"says no provider is registered when the client says nothing"', + ], + expression: + 'expect(signInWithEmailDomain("someone@company.com",refuse),).rejects.toThrow("No identity provider is registered for that address.")', finding: "round 2 — auth-client.test.ts, six sites", - why: 'test("says no provider is registered when the client says nothing", async () => { const refuse = async () => (', }, { file: "app/tests/auth-client.test.ts", - line: 176, shape: "a", + context: [ + '"signInWithEmailDomain"', + '"resolves quietly when the redirect is under way"', + ], + expression: + 'expect(signInWithEmailDomain("someone@company.com",async()=>({error:null,})),).resolves.toBeUndefined()', finding: "round 2 — auth-client.test.ts, six sites", - why: 'test("resolves quietly when the redirect is under way", async () => { expect( signInWithEmailDomain("someone@c', }, { file: "server/tests/audit-initiator.integration.test.ts", - line: 291, shape: "a", + context: [ + '"what started a run"', + '"the trail stays append-only, so a row cannot be re-attributed later"', + ], + expression: "expect(reattribute()).rejects.toThrow()", finding: "NEW (round 2 did not find this)", - why: "expect(reattribute()).rejects.toThrow();", }, { file: "server/tests/component-store.integration.test.ts", - line: 149, shape: "a", + context: [ + '"withholding"', + '"withholding a component that does not exist is an error, not a silent row"', + ], + expression: + // biome-ignore lint/suspicious/noTemplateCurlyInString: This is the literal source expression being identified. + 'expect(store.revoke(`nothing_${suite}`,botA,"x@example.test"),).rejects.toThrow(ComponentNotFoundError)', finding: "NEW (round 2 did not find this)", - why: 'test("withholding a component that does not exist is an error, not a silent row", async () => { expect( store.', }, { file: "server/tests/component-store.integration.test.ts", - line: 372, shape: "a", + context: [ + '"what a component may read"', + '"revoking from a component that does not exist is an error, not a silent no-op"', + ], + expression: + // biome-ignore lint/suspicious/noTemplateCurlyInString: This is the literal source expression being identified. + "expect(store.revokeFunction(`nothing_${suite}`,fn)).rejects.toThrow(ComponentNotFoundError,)", finding: "NEW (round 2 did not find this)", - why: 'test("revoking from a component that does not exist is an error, not a silent no-op", async () => { expect(sto', }, { file: "server/tests/component-store.integration.test.ts", - line: 404, shape: "a", + context: [ + '"what a component may read"', + '"a function this build does not ship cannot be run"', + ], + expression: + // biome-ignore lint/suspicious/noTemplateCurlyInString: This is the literal source expression being identified. + "expect(store.callFunction(`nothing_${suite}`,{})).rejects.toThrow()", finding: "NEW (round 2 did not find this)", - why: 'test("a function this build does not ship cannot be run", async () => { expect(store.callFunction(`nothing_${s', }, { file: "server/tests/computer-client.test.ts", - line: 263, shape: "a", + context: [ + '"acting on an element that is not there"', + '"is its own condition, not an unavailable computer"', + ], + expression: + 'expect(timingOut().click({ref:"e5",snapshotId:1}),).rejects.toBeInstanceOf(ElementNotFoundError)', finding: "NEW (round 2 did not find this)", - why: 'test("is its own condition, not an unavailable computer", async () => { expect( timingOut().click({ ref: "e5",', }, { file: "server/tests/computer-client.test.ts", - line: 305, shape: "a", + context: [ + '"a 409 from the computer"', + '"is a person holding the wheel when the computer says so"', + ], + expression: + 'expect(refusing({error:"A person has taken control of this computer.",humanHasControl:true,}).click({ref:"e1",snapshotId:1}),).rejects.toBeInstanceOf(HumanHasControlError)', finding: "NEW (round 2 did not find this)", - why: 'test("is a person holding the wheel when the computer says so", async () => { expect( refusing({ error: "A per', }, { file: "server/tests/computer-client.test.ts", - line: 316, shape: "a", + context: [ + '"a 409 from the computer"', + '"is a stale snapshot when it does not"', + ], + expression: + 'expect(refusing({error:"Snapshot 3 is not the current one."}).click({ref:"e1",snapshotId:1,}),).rejects.toBeInstanceOf(StaleSnapshotError)', finding: "NEW (round 2 did not find this)", - why: 'test("is a stale snapshot when it does not", async () => { expect( refusing({ error: "Snapshot 3 is not the cu', }, { file: "server/tests/computer-client.test.ts", - line: 334, shape: "a", + context: [ + '"a 409 from the computer"', + '"is a stale snapshot for every shape that is not the flag"', + ], + expression: + 'expect(refusing(body).click({ref:"e1",snapshotId:1}),).rejects.toBeInstanceOf(StaleSnapshotError)', finding: "NEW (round 2 did not find this)", - why: 'expect( refusing(body).click({ ref: "e1", snapshotId: 1 }), ).rejects.toBeInstanceOf(StaleSnapshotError);', }, { file: "server/tests/computer-client.test.ts", - line: 342, shape: "a", + context: [ + '"a 409 from the computer"', + '"a 409 with no body at all is still a refusal, not a crash"', + ], + expression: + 'expect(bodyless.click({ref:"e1",snapshotId:1})).rejects.toBeInstanceOf(StaleSnapshotError,)', finding: "NEW (round 2 did not find this)", - why: 'test("a 409 with no body at all is still a refusal, not a crash", async () => { const bodyless = clientWith(()', }, { file: "server/tests/computer-supervisor.test.ts", - line: 65, shape: "a", + context: [ + '"locating a Bot\'s computer"', + '"a computer with no address at all is an error, not a fallback"', + ], + expression: + 'expect(client.locate("sales")).rejects.toThrow(SupervisorError)', finding: "NEW (round 2 did not find this)", - why: 'expect(client.locate("sales")).rejects.toThrow(SupervisorError);', }, { file: "server/tests/computer-supervisor.test.ts", - line: 75, shape: "a", + context: [ + '"locating a Bot\'s computer"', + '"a refusal from the supervisor is reported in its own words"', + ], + expression: + 'expect(client.locate("bad id")).rejects.toThrow("A bot id may contain only letters.",)', finding: "NEW (round 2 did not find this)", - why: 'expect(client.locate("bad id")).rejects.toThrow( "A bot id may contain only letters.", );', }, { file: "server/tests/computer-supervisor.test.ts", - line: 89, shape: "a", + context: [ + '"locating a Bot\'s computer"', + '"an unreachable supervisor says so, rather than looking like a broken computer"', + ], + expression: + 'expect(client.locate("sales")).rejects.toThrow(/couldnotbereached/)', finding: "NEW (round 2 did not find this)", - why: 'expect(client.locate("sales")).rejects.toThrow(/could not be reached/);', }, { file: "server/tests/people-last-sign-in.integration.test.ts", - line: 127, shape: "a", + context: [ + '"when somebody was last here"', + '"a stamp that cannot be written does not refuse the sign-in"', + ], + expression: + 'expect(stampSignIn(broken,"anybody",SIGNED_IN_AT),).resolves.toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect( stampSignIn(broken, "anybody", SIGNED_IN_AT), ).resolves.toBeUndefined();', }, { file: "server/tests/plugin-user-credential.integration.test.ts", - line: 369, shape: "a", + context: [ + '"a person who has not connected"', + '"is refused, and told to connect rather than told it broke"', + ], + expression: + "expect(store.callTool({ref,args:{},botId,actorId:askerId}),).rejects.toThrow(PluginRefusedError)", finding: "round 2 — plugin-user-credential.integration.test.ts:361", - why: 'describe("a person who has not connected", () => { test("is refused, and told to connect rather than told it b', }, { file: "app/tests/bot-setup.test.tsx", - line: 178, shape: "b", + context: [ + '"a built-in coworker can be renamed on a deployment whose own Bot lives on localhost"', + ], + expression: "expect(updates[0]?.endpoint).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(updates[0]?.endpoint).toBeUndefined();", }, { file: "app/tests/voice-archive.test.ts", - line: 98, shape: "b", + context: [ + '"a busy summary response retains the server\'s durable transcript instead of marking it unsaved"', + ], + expression: "expect(records[0]?.saveState).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(records[0]?.saveState).toBeUndefined();", }, { file: "app/tests/voice-archive.test.ts", - line: 149, shape: "b", + context: [ + '"save failure remains retryable with the same session id and original transcript"', + ], + expression: + 'expect((awaitloadVoiceArchive("channel-1"))[0]?.saveState).toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect((await loadVoiceArchive("channel-1"))[0]?.saveState).toBeUndefined();', }, { file: "server/tests/agent-endpoint.test.ts", - line: 498, - shape: "b", - finding: "NEW (round 2 did not find this)", - why: "expect(forwarded.forwardedProps?.openbotRun).toBeUndefined();", - }, - { - file: "server/tests/agent-escalation.test.ts", - line: 78, - shape: "b", - finding: "NEW (round 2 did not find this)", - why: "expect(written[0]?.initiator).toBe(undefined);", - }, - { - file: "server/tests/agent-handoff.test.ts", - // Moved from 366 by the work-list change, which added tests above it; the offence is the same one and - // is re-keyed rather than re-found. - line: 409, shape: "b", + context: [ + '"dialling a stored agent endpoint"', + '"a hop to another host does not take the signed run with it"', + ], + expression: "expect(forwarded.forwardedProps?.openbotRun).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(plain.events[0]?.initiator).toBe(undefined);", }, { file: "server/tests/approvals-interactive.integration.test.ts", - line: 229, shape: "b", + context: [ + '"a click from a person\'s own chat waits, runs once on allow once, and is never repeated"', + ], + expression: "expect(continued[0]?.error).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(continued[0]?.error).toBeUndefined();", }, { file: "server/tests/approvals-policy.test.ts", - line: 779, shape: "b", + context: [ + '"delegation is gated"', + '"an approved hand-off is re-checked, sent once, and its answer continues the conversation"', + ], + expression: "expect(continued[0]?.error).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(continued[0]?.error).toBeUndefined();", }, { file: "server/tests/arcade-adapter-errors.test.ts", - line: 63, shape: "b", + context: [ + '"normalizeArcadeError"', + '"a connection failure keeps status undefined"', + ], + expression: + 'expect(normalizeArcadeError(newAPIConnectionError({message:"down"}))?.status,).toBeUndefined()', finding: "round 2 — arcade-adapter-errors.test.ts:62-66", - why: 'test("a connection failure keeps status undefined", () => { expect( normalizeArcadeError(new APIConnectionErro', }, { file: "server/tests/arcade-sibling-collateral.test.ts", - line: 417, shape: "b", + context: [ + '"offboarding a person"', + '"records a sibling app as revoked, naming the app whose revoke ended it"', + ], + expression: "expect(row(t.gmail)?.payload.endedWith).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(row(t.gmail)?.payload.endedWith).toBeUndefined();", }, { file: "server/tests/arcade-sibling-collateral.test.ts", - line: 421, shape: "b", + context: [ + '"offboarding a person"', + '"records a sibling app as revoked, naming the app whose revoke ended it"', + ], + expression: "expect(row(t.slack)?.payload.endedWith).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(row(t.slack)?.payload.endedWith).toBeUndefined();", }, { file: "server/tests/audit-export.test.ts", - line: 181, shape: "b", + context: [ + '"readAuditExportRows"', + '"walks every matching page and drops the list\'s own paging"', + ], + expression: "expect(seen[0]?.cursor).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(seen[0]?.cursor).toBeUndefined();", }, { file: "server/tests/computer-gateway.test.ts", - line: 381, shape: "b", + context: [ + '"the computer gateway"', + '"the local development actor is kept out of the audit foreign key"', + ], + expression: "expect(rows[0]?.actorUserId).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(rows[0]?.actorUserId).toBeUndefined();", }, { file: "server/tests/computer-gateway.test.ts", - line: 443, shape: "b", + context: [ + '"the computer gateway"', + '"a permitted command runs and is recorded by command, never by output"', + ], + expression: "expect(rows[0]?.payload.element).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(rows[0]?.payload.element).toBeUndefined();", }, { file: "server/tests/computer-gateway.test.ts", - line: 646, shape: "b", + context: [ + '"the computer gateway"', + '"an action that never named an element records no element"', + ], + expression: "expect(rows.at(-1)?.payload.element).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(rows.at(-1)?.payload.element).toBeUndefined();", }, { file: "server/tests/computer-provider.test.ts", - line: 356, shape: "b", + context: [ + '"shared computer provider"', + '"answers undefined when the computer cannot say, rather than throwing"', + ], + expression: 'expect(awaitprovider.sessionOf?.("sales")).toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect(await provider.sessionOf?.("sales")).toBeUndefined();', }, { file: "server/tests/computer-provider.test.ts", - line: 365, shape: "b", + context: [ + '"shared computer provider"', + '"answers undefined when the computer answers without one"', + ], + expression: 'expect(awaitprovider.sessionOf?.("sales")).toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect(await provider.sessionOf?.("sales")).toBeUndefined();', }, { file: "server/tests/computer-sandbox.test.ts", - line: 79, shape: "b", + context: [ + '"telling one run of a Bot\'s computer from the next"', + '"a suspended computer has no run to name"', + ], + expression: + 'expect(awaitproviderWith(suspended).sessionOf?.("knowledge"),).toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect( await providerWith(suspended).sessionOf?.("knowledge"), ).toBeUndefined();', }, { file: "server/tests/headless-components.integration.test.ts", - line: 350, shape: "b", + context: [ + '"an unattended routine turn answering with a governed chart"', + '"a grant revoked after the turn started is refused at call time"', + ], + expression: "expect(bar?.hidden).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(bar?.hidden).toBeUndefined();", }, { file: "server/tests/headless-components.test.ts", - line: 202, shape: "b", + context: [ + '"headless component tools"', + '"a held chart is offered with its published description and records a confirmation"', + ], + expression: "expect(bar?.hidden).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(bar?.hidden).toBeUndefined();", }, { file: "server/tests/identity-retire.integration.test.ts", - line: 124, shape: "b", + context: [ + '"retiring a removed person\'s links revokes their tokens and keeps the rows"', + ], + expression: "expect(audit.rows[0]?.actorUserId).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(audit.rows[0]?.actorUserId).toBeUndefined();", }, { file: "server/tests/identity-retire.integration.test.ts", - line: 247, shape: "b", + context: [ + '"a directory remover is recorded in the payload only, never as the acting user"', + ], + expression: "expect(audit.rows[0]?.actorUserId).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(audit.rows[0]?.actorUserId).toBeUndefined();", }, { file: "server/tests/mcp-listing.test.ts", - line: 168, shape: "b", + context: [ + '"what an MCP listing tells the classifier"', + '"a vendor\'s read-only claim records nothing at all"', + ], + expression: "expect(fetch?.effect).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(fetch?.effect).toBeUndefined();", }, { file: "server/tests/mcp-listing.test.ts", - line: 179, shape: "b", + context: [ + '"what an MCP listing tells the classifier"', + '"a tool with no annotations is unchanged in every respect"', + ], + expression: "expect(search?.effect).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(search?.effect).toBeUndefined();", }, { file: "server/tests/mcp-listing.test.ts", - line: 180, shape: "b", + context: [ + '"what an MCP listing tells the classifier"', + '"a tool with no annotations is unchanged in every respect"', + ], + expression: "expect(search?.destructive).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(search?.destructive).toBeUndefined();", }, { file: "server/tests/picked-harness-auth.test.ts", - line: 290, shape: "b", + context: [ + '"a plan\'s installed picked harness authenticates without advertising a bundled Bot"', + ], + expression: "expect(result.config.managedAgent?.endpoint).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: 'test("a plan\'s installed picked harness authenticates without advertising a bundled Bot", async () => { const ', }, { file: "server/tests/picked-harness-auth.test.ts", - line: 355, shape: "b", + context: [ + '"the default package\'s case-only picked URL authenticates through the real HTTP client"', + ], + expression: "expect(result.config.managedAgent?.endpoint).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(result.config.managedAgent?.endpoint).toBeUndefined();", }, { file: "server/tests/plugin-catalogue.test.ts", - line: 210, shape: "b", + context: ['"Notion"', '"is in the catalogue with the MCP transport"'], + expression: "expect(entry?.transport).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(entry?.transport).toBeUndefined();", }, { file: "server/tests/shared-computer-run.integration.test.ts", - line: 264, shape: "b", + context: [ + '"a Bot on the one shared computer, against the table"', + '"a computer that cannot say which run it is on leaves the row unordered, not refused"', + ], + expression: + 'expect((awaitsnapshots.load("bot-1"))?.session).toBeUndefined()', finding: "NEW (round 2 did not find this)", - why: 'expect((await snapshots.load("bot-1"))?.session).toBeUndefined();', }, { file: "server/tests/tenant-package.test.ts", - line: 372, shape: "b", + context: [ + '"tenant YAML validation"', + '"parses an explicit avatar seed and leaves an omitted seed undefined"', + ], + expression: "expect(tenantPackage.agents[1]?.avatarSeed).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(tenantPackage.agents[1]?.avatarSeed).toBeUndefined();", }, { file: "server/tests/tool-selection.integration.test.ts", - line: 837, shape: "b", + context: [ + '"a remote Mastra Bot"', + '"keeps only authoritative OpenBot governance through the runtime clone and native Mastra request"', + ], + expression: "expect(run?.forwardedProps?.openbotRun).toBeUndefined()", finding: "NEW (round 2 did not find this)", - why: "expect(run?.forwardedProps?.openbotRun).toBeUndefined();", }, { file: "app/tests/user-preferences.test.tsx", - line: 75, shape: "c", + context: [ + '"loads preferences from the account and renders the saved emphasis"', + ], + expression: + 'expect(requests).toEqual([{path:"/api/settings/preferences",method:"GET",body:undefined},])', finding: "NEW (round 2 did not find this)", - why: 'test("loads preferences from the account and renders the saved emphasis", async () => { const { view } = setup', }, { file: "server/tests/agent-handoff.test.ts", - // Moved from 620 by the work-list change, which added tests above it; the offence is the same one and - // is re-keyed rather than re-found. - line: 663, shape: "c", + context: [ + '"the same ask a second time"', + '"is refused plainly rather than reported as handed over"', + ], + expression: + 'expect(twice.events.map((event)=>({eventType:event.eventType,reason:(event.payloadas{reason?:string}).reason,})),).toEqual([{eventType:"agent.handoff_offered",reason:undefined},{eventType:"agent.handoff_refused",reason:"duplicate"},])', finding: "NEW (round 2 did not find this)", - why: ').toEqual([ { eventType: "agent.handoff_offered", reason: undefined }, { eventType: "agent.handoff_refused", r', }, { file: "server/tests/agent-test-connection-headers.test.ts", - line: 12, shape: "c", + context: ['"parseConnectionHeaders"', '"absent headers stay absent"'], + expression: + "expect(parseConnectionHeaders(undefined)).toEqual({ok:true,value:undefined,})", finding: "NEW (round 2 did not find this)", - why: 'describe("parseConnectionHeaders", () => { test("absent headers stay absent", () => { expect(parseConnectionHe', }, { file: "server/tests/arcade-adapter-errors.test.ts", - line: 55, shape: "c", + context: [ + '"normalizeArcadeError"', + '"an error with no body leaves name and message undefined"', + ], + expression: + "expect(normalizeArcadeError(newAPIError(500,undefined,undefined,newHeaders()),),).toEqual({status:500,name:undefined,message:undefined,})", finding: "round 2 — arcade-adapter-errors.test.ts:50-60", - why: ").toEqual({", }, { file: "server/tests/composio-adapter.test.ts", - // Moved from 5412 by OB-05, which added fifteen lines of tests above it, then back three by the - // Plugins screen, which shortened a test above it; the offence is the same one and is re-keyed - // rather than re-found. - line: 5424, shape: "c", + context: [ + '"an action that publishes no description, schema or version is still listed"', + ], + expression: + 'expect(awaitactions.listActions("gmail",{limit:WHOLE_LISTING}),).toEqual([{slug:"GMAIL_FETCH_EMAILS",description:undefined,inputParameters:undefined,tags:undefined,version:undefined,},])', finding: "NEW (round 2 did not find this)", - why: 'expect( await actions.listActions("gmail", { limit: WHOLE_LISTING }), ).toEqual([ { slug: "GMAIL_FETCH_EMAILS"', }, { file: "server/tests/composio-adapter.test.ts", - // Moved from 5458 by OB-05, which added fifteen lines of tests above it, then back three by the - // Plugins screen, which shortened a test above it; the offence is the same one and is re-keyed - // rather than re-found. - line: 5470, shape: "c", + context: [ + '"an action whose description, schema or version is null is still listed"', + ], + expression: + 'expect(awaitactions.listActions("gmail",{limit:WHOLE_LISTING}),).toEqual([{slug:"GMAIL_FETCH_EMAILS",description:undefined,inputParameters:undefined,tags:undefined,version:undefined,},])', finding: "NEW (round 2 did not find this)", - why: 'expect( await actions.listActions("gmail", { limit: WHOLE_LISTING }), ).toEqual([ { slug: "GMAIL_FETCH_EMAILS"', }, { file: "server/tests/computer-gateway.test.ts", - line: 808, shape: "c", + context: [ + '"the computer gateway"', + "\"computers maps provider status 'running' and 'stopped' directly and preserves egress distinctions\"", + ], + expression: + 'expect(result).toEqual({isolation:"per-bot",computers:[{botId:"bot-proxied",running:true,startedAt:"2026-08-20T12:00:00.000Z",egress:"198.51.100.42",},{botId:"bot-direct",running:false,startedAt:"2026-08-20T11:00:00.000Z",egress:null,},{botId:"bot-unknown-egress",running:false,startedAt:"2026-08-20T10:00:00.000Z",egress:undefined,},],})', finding: "NEW (round 2 did not find this)", - why: 'expect(result).toEqual({ isolation: "per-bot", computers: [ { botId: "bot-proxied",', }, { file: "server/tests/demonstrations.test.ts", - line: 294, shape: "c", + context: [ + '"scheduling creates a routine for the recording\'s own Bot through the routine store"', + ], + expression: + 'expect(created).toEqual([{ownerUserId:"owner",agentId:"bot",instruction:demonstrationRoutineInstruction({title:"Find invoice",slug:"find-invoice",}),cron:"0 9 * * 1-5",timezone:undefined,channelId:undefined,},])', finding: "NEW (round 2 did not find this)", - why: 'expect(created).toEqual([ { ownerUserId: "owner", agentId: "bot", instruction: demonstrationRoutineInstruction', }, { file: "server/tests/routine-runner.test.ts", - // Moved from 169 by the work-list change, which added tests above it; the offence is the same one and - // is re-keyed rather than re-found. - line: 185, shape: "c", + context: [ + '"createRoutineRunner"', + '"runs the owner\'s thread and records the reply exactly once"', + ], + expression: + 'expect(recorded.finished).toEqual([{runId:RUN_ID,status:"succeeded",error:undefined},])', finding: "NEW (round 2 did not find this)", - why: 'expect(recorded.finished).toEqual([ { runId: RUN_ID, status: "succeeded", error: undefined }, ]);', }, { file: "server/tests/routine-runner.test.ts", - // Moved from 272 by the work-list change, which added tests above it; the offence is the same one and - // is re-keyed rather than re-found. - line: 289, shape: "c", + context: [ + '"createRoutineRunner"', + '"keeps a succeeded run recorded when the channel cannot be written to"', + ], + expression: + 'expect(recorded.finished).toEqual([{runId:RUN_ID,status:"succeeded",error:undefined},])', finding: "NEW (round 2 did not find this)", - why: 'expect(recorded.finished).toEqual([ { runId: RUN_ID, status: "succeeded", error: undefined }, ]);', }, { file: "app/tests/connected-accounts-authorized-notice.test.tsx", - line: 104, shape: "d", + context: [ + '"what ?authorized=%s answers is a sentence or nothing, never a function or an object"', + ], + expression: + 'test.each(["no-attempt","toString","constructor","__proto__","something-somebody-made-up",])("what ?authorized=%s answers is a sentence or nothing, never a function or an object",(value)=>{constnotice=authorizationNoticeFor(value);if(notice!==null)expect(typeofnotice).toBe("string");},)', finding: "round 2 — connected-accounts-authorized-notice.test.tsx:103-115", - why: 'test.each([ "no-attempt", "toString", "constructor", "__proto__",', }, { file: "app/tests/connected-account-grants.test.tsx", - line: 126, shape: "e", + context: [], + expression: "constsiblingReads:string[]=[]", finding: "NEW (round 2 did not find this)", - why: "const siblingReads: string[] = [];", }, - /* - * `server/tests/arcade-attempt-store.integration.test.ts:81` — `const actors: string[] = []`, - * written and never read — is GONE rather than renumbered. The list it collects is now read by - * the `removePeople` in that file's `afterAll`: `pending_authorizations.actor_id` keys to `users` - * and cascades, so the ids the suite invents are people it has to create and then take away, and - * the collection that was decorative is now what the cleanup runs on. - */ { file: "server/tests/arcade-disconnect-cascade.test.ts", - // Moved from 36 by round 2's A5 fix, which added three tests above it; the offence is the - // same one and is re-keyed rather than re-found. - line: 39, shape: "e", + context: [], + expression: "constrevoked:string[]=[]", finding: "round 2 — arcade-disconnect-cascade.test.ts:36,60", - why: "const revoked: string[] = [];", }, { file: "server/tests/arcade-grants-route.test.ts", - line: 139, shape: "e", + context: [], + expression: "consttoolkits:string[]=[]", finding: "NEW (round 2 did not find this)", - why: "const toolkits: string[] = [];", }, { file: "server/tests/arcade-grants-route.test.ts", - line: 140, shape: "e", + context: [], + expression: "constviewers:string[]=[]", finding: "NEW (round 2 did not find this)", - why: "const viewers: string[] = [];", }, { file: "server/tests/enterprise-controls.integration.test.ts", - line: 59, shape: "e", + context: [], + expression: "conststopped:string[]=[]", finding: "NEW (round 2 did not find this)", - why: "const stopped: string[] = []; — pushed to at :65, cleared at :718, read nowhere; the `.stopped` at :704 and :720 is the response's own field", }, { file: "server/tests/arcade-vendor-error-engine.test.ts", - line: 186, shape: "f", + context: [ + '"a refusal from Arcade is an Error this deployment wrote"', + // biome-ignore lint/suspicious/noTemplateCurlyInString: This is the literal source expression being identified. + "`${door} throws an Error carrying no vendor words`", + ], + expression: "JSON.stringify(thrown,Object.keys(thrownasobject)),", finding: "round 2 — arcade-vendor-error-engine.test.ts:174", - why: 'expect( JSON.stringify(thrown, Object.keys(thrown as object)), ).not.toContain("SECRET-BEARER");', }, ]; +/** Consume each allowance once: a second copy in the same test is still a new offence. */ +function unmatched< + T extends Pick, +>( + candidates: T[], + baseline: Pick[], +): T[] { + const remaining = new Map(); + for (const entry of baseline) + remaining.set(key(entry), (remaining.get(key(entry)) ?? 0) + 1); + return candidates.filter((entry) => { + const identity = key(entry); + const count = remaining.get(identity) ?? 0; + if (count === 0) return true; + remaining.set(identity, count - 1); + return false; + }); +} + /** One offence per line, the way a reader wants to see it. */ const report = ( offences: { shape: Shape; file: string; line: number; text: string }[], @@ -958,9 +1197,8 @@ const report = ( for (const root of ROOTS) { test(`${root} grows no assertion shape the fix cycle does not already own`, () => { - const allowed = new Set(ALLOW.map(key)); const scanned = scanTree(join(REPO, root), REPO); - expect(report(scanned.filter((o) => !allowed.has(key(o))))).toEqual([]); + expect(report(unmatched(scanned, ALLOW))).toEqual([]); // And the walk reached the tree at all: a scan of nothing reports nothing. expect(sourceFiles(join(REPO, root), EXTENSIONS).length).toBeGreaterThan( 20, @@ -969,14 +1207,69 @@ for (const root of ROOTS) { } test("the allow-list holds no offence the fix cycle has already cleared", () => { - const found = new Set( - ROOTS.flatMap((root) => scanTree(join(REPO, root), REPO)).map(key), - ); - const stale = ALLOW.filter((entry) => !found.has(key(entry))); + const found = ROOTS.flatMap((root) => scanTree(join(REPO, root), REPO)); + const stale = unmatched(ALLOW, found); // A fixed offence must leave the list with its fix, or the list stops meaning anything. expect(stale.map((entry) => `${key(entry)} ${entry.finding}`)).toEqual([]); }); +test("allowances survive comments, line shifts and expression wrapping", () => { + const before = offencesIn( + "fixture.ts", + 'test("existing", () => { expect(row?.value).toBeUndefined(); });', + ); + const wrapped = offencesIn( + "fixture.ts", + `// New introduction. + + +test("existing", () => { + expect( + row?.value + ).toBeUndefined(); +});`, + ); + expect(before).toHaveLength(1); + expect(wrapped[0].line).not.toBe(before[0].line); + expect(unmatched(wrapped, before)).toEqual([]); + expect(unmatched(before, wrapped)).toEqual([]); +}); + +test("an allowance cannot cover the same expression in another test or a second copy", () => { + const original = + 'test("existing", () => { expect(row?.value).toBeUndefined(); });'; + const allowed = offencesIn("fixture.ts", original); + const addedTest = offencesIn( + "fixture.ts", + original + + '\ntest("new case", () => { expect(row?.value).toBeUndefined(); });', + ); + const duplicate = offencesIn( + "fixture.ts", + 'test("existing", () => { expect(row?.value).toBeUndefined(); expect(row?.value).toBeUndefined(); });', + ); + expect(unmatched(addedTest, allowed).map((o) => o.context)).toEqual([ + ['"new case"'], + ]); + expect(unmatched(duplicate, allowed)).toHaveLength(1); + expect(unmatched(allowed, [])).toEqual(allowed); +}); + +test("the full expression identity keeps literal contents and changes beyond diagnostic truncation", () => { + const prefix = "x".repeat(150); + const original = offencesIn( + "fixture.ts", + `test("existing", () => { expect(row?.["${prefix} a"]).toBeUndefined(); });`, + ); + const changed = offencesIn( + "fixture.ts", + `test("existing", () => { expect(row?.["${prefix} a"]).toBeUndefined(); });`, + ); + expect(original).toHaveLength(1); + expect(original[0].text).toBe(changed[0].text); + expect(unmatched(changed, original)).toHaveLength(1); +}); + /* POSITIVE CONTROLS. * * Each plants one shape in a file on disk, at a nesting and an extension the real trees have, and diff --git a/server/tests/brokered-provenance.test.ts b/server/tests/brokered-provenance.test.ts new file mode 100644 index 0000000000..3cf8f05339 --- /dev/null +++ b/server/tests/brokered-provenance.test.ts @@ -0,0 +1,133 @@ +// server/tests/brokered-provenance.test.ts +/** + * Guard: no plugin module outside the three VOCABULARY modules may spell a brokered vendor's name — + * in a provenance or provider comparison, or in a url scheme it builds. + * + * `access.ts`, `composio.ts` and `arcade.ts` are the exempt files. `access.ts` is where the shared + * vocabulary lives — {@link BrokerProvider}, {@link brokeredProvenance}, `TOOLKIT_OF` and + * `BROKERED_PROFILE`, which is why the scheme literals are there and not in `store.ts`. The other + * two are each one broker's own module, and a module whose whole job is to speak one vendor's + * protocol has to be able to spell that vendor's scheme and define that vendor's parser. Neither + * trips the provenance patterns today, and neither is exempt from anything it does not need: + * widening the list is what makes the scheme pattern absolute everywhere else, instead of a pattern + * with a per-line escape hatch — and an escape hatch in a guard is the thing that gets used the + * second time without the argument being made again. + * + * COMMENT LINES ARE SKIPPED, DELIBERATELY AND NARROWLY. A line whose trimmed start is `*`, `//` or + * `/*` is not scanned. `store.ts` alone carries about a dozen `composio://` mentions inside + * docblocks — "a row called `gmail` at `composio://slack`" and the like — which are explanation of + * why a rule exists, not an instance of breaking it. A SCHEME NAMED IN PROSE IS DOCUMENTATION; A + * SCHEME NAMED IN AN EXPRESSION IS A DECISION. The skip is on the line's syntactic position and + * nothing else, so it cannot grow: the alternative — loosening the pattern until the docblocks stop + * matching — would also stop it matching the thing it is for, which is a string carrying a scheme. + * + * The offenders are collected into a list and compared against `[]` rather than asserted one file at + * a time, so a new site has to be argued for here rather than quietly allowed. + * + * THE FOURTH PATTERN FAMILY IS THE PARSER CALL, AND IT IS ARMED. A direct call to a vendor module's + * own parser — `toolkitOf(url)` rather than `toolkitOfFor(provider, url)` — is the same mistake as a + * spelled scheme, reached one step further in: it does not name a vendor, it IS one, and it answers + * null for every url the other broker mints rather than failing. What that bought on an Arcade + * deployment was four different wrong answers from one wrong parser, none of which read as a parser + * problem — an app refused as "not reached through a broker", a directory offering to enable what + * was already enabled, a connect route falling past its own brokered branch, and an authorize, + * grants and disconnect that all refused a connected app. + * + * WHAT THE ARMED PATTERN NOW GUARANTEES: outside the three vocabulary modules, no plugin module + * calls `toolkitOf` at all. Every brokered url in `src/plugins` is parsed through + * `toolkitOfFor(provider, url)`, with the provider handed in rather than sniffed — so adding a + * broker changes the dispatch table in `access.ts` and nothing else, and a module that reaches past + * the dispatch for one vendor's parser is named here before it ships. The only lines the pattern + * still matches are the definitions themselves, in the three exempt modules it does not scan. + */ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { basename, join } from "node:path"; +import { logicalLines, sourceFiles } from "./support/source-scan"; + +const PLUGINS_DIR = join(import.meta.dir, "../src/plugins"); + +/** The modules allowed to speak a broker's vocabulary. Every other plugin module asks them. */ +const VOCABULARY = new Set(["access.ts", "composio.ts", "arcade.ts"]); + +const PATTERNS: RegExp[] = [ + /provenance\s*[!=]==\s*"(composio|arcade)"/, + /brokeredConnections\.provider\s*,\s*"(composio|arcade)"/, + /provider:\s*"(composio|arcade)"/, + // `eq(mcpServers.provenance, "composio")`: the drizzle spelling of the same mistake as the + // `provenance === "composio"` pattern above. A guard catching one spelling of a mistake and not + // the other is worse than none, because it is read as coverage. + /(mcpServers|\w+)\.provenance\s*,\s*"(composio|arcade)"/, + // A scheme spelled into a string — the construction half. `composio://${toolkit}` in `store.ts` + // is what dropped every Arcade connection off the settings page and what sent an Arcade + // deployment's probe to Composio; the scheme comes out of `BROKERED_PROFILE` now. + /["'`](composio|arcade):\/\//, + // A vendor module's own parser called directly, not the dispatch. `toolkitOf(url)` reads one + // broker's scheme and answers null for the other's, so a module holding the active provider must + // ask `toolkitOfFor(provider, url)` instead. + /\btoolkitOf\s*\(/, +]; + +/** A line of prose rather than a line of code. See the docblock: the skip is narrow on purpose. */ +function isComment(line: string): boolean { + const start = line.trimStart(); + return ( + start.startsWith("*") || start.startsWith("//") || start.startsWith("/*") + ); +} + +test("no plugin module hard-codes a brokered vendor name outside the vocabulary modules", () => { + const offenders: string[] = []; + // The shared scanner's walk and line-splitting. RECURSIVE: a `src/plugins//x.ts` was + // unscanned before, and the exemption is by FILE NAME, so a `plugins//access.ts` is not + // quietly exempt either. LOGICAL lines: a scheme prettier wrapped off its own `=` is one + // decision, and reading it per physical line saw half of it. The comment skip is unchanged and + // still narrow — only a line that STARTS as prose is spared. + const files = sourceFiles(PLUGINS_DIR, [".ts"]).filter( + (f) => !VOCABULARY.has(basename(f)), + ); + for (const file of files) { + for (const { line, text } of logicalLines(readFileSync(file, "utf8"))) { + if (isComment(text)) continue; + if (PATTERNS.some((re) => re.test(text))) { + offenders.push(`${file.slice(PLUGINS_DIR.length + 1)}:${line}`); + } + } + } + expect(offenders).toEqual([]); +}); + +/** + * And the comment skip does not also skip code, which is the one way this guard could go quietly + * useless. Each line below is an offender the scan must still name, including the ones that carry a + * comment — a trailing `//` does not make a line prose, and only a line that STARTS as prose is + * skipped. + */ +test("the comment skip takes prose and leaves every expression", () => { + const code = [ + // Interpolation is spelled by concatenation here only so the linter does not read a template + // placeholder inside a plain string as a mistake; what the pattern sees is the same line. + `const url = \`composio://$\{toolkit}\`;`, + `const url = \`arcade://$\{toolkit}\`; // the scheme`, + ' return { url: "composio://gmail" };', + '\tif (row.provenance === "arcade") return null;', + 'const u = "composio://" + toolkit; /* trailing block */', + " const toolkit = row ? toolkitOf(row.url) : null;", + ]; + const prose = [ + " * a row called `gmail` at `composio://slack` would decide a Slack confirm", + "// `arcade://Gmail` and `arcade://gmail` are distinct apps.", + "/* `composio://` is the scheme the rows are minted at. */", + ' * `provenance === "composio"` was the comparison this replaced.', + " * `enabled` comes off `toolkitOf(url)` and never off the row's id.", + ]; + for (const line of code) { + expect(isComment(line)).toBe(false); + expect(PATTERNS.some((re) => re.test(line))).toBe(true); + } + for (const line of prose) { + // Prose that WOULD match: the skip is what spares it, not a gap in the patterns. + expect(PATTERNS.some((re) => re.test(line))).toBe(true); + expect(isComment(line)).toBe(true); + } +}); diff --git a/server/tests/bunfig-preload.test.ts b/server/tests/bunfig-preload.test.ts new file mode 100644 index 0000000000..9b81765b92 --- /dev/null +++ b/server/tests/bunfig-preload.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, test } from "bun:test"; +import { existsSync, readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; + +/** + * The two bunfig.toml files, held against each other. + * + * Bun reads bunfig.toml from the current working directory and nowhere else, so the root one does + * not cover `bun test` run from inside `server`, and there has to be a second one here. A second + * file is a second thing to forget: a preload added to one and not the other reintroduces exactly + * the failure the preload exists to prevent, and reintroduces it silently, because a test file that + * throws while being imported reports nothing at all. + * + * So the drift is asserted away rather than documented away. Both lists are resolved against their + * own file's directory and compared as absolute paths, which is the comparison that matters: the two + * are meant to name the same scripts, not to contain the same strings. + */ + +const repositoryRoot = resolve(import.meta.dir, "..", ".."); + +function preloadedScripts(bunfigPath: string): string[] { + const parsed = Bun.TOML.parse(readFileSync(bunfigPath, "utf8")) as { + test?: { preload?: string[] }; + }; + const declared = parsed.test?.preload ?? []; + return declared.map((entry) => resolve(dirname(bunfigPath), entry)).sort(); +} + +describe("bunfig preload", () => { + const rootBunfig = resolve(repositoryRoot, "bunfig.toml"); + const serverBunfig = resolve(repositoryRoot, "server", "bunfig.toml"); + + test("the root and server configs preload the same scripts", () => { + expect(preloadedScripts(serverBunfig)).toEqual( + preloadedScripts(rootBunfig), + ); + }); + + test("every preloaded script exists", () => { + const scripts = preloadedScripts(rootBunfig); + + expect(scripts.length).toBeGreaterThan(0); + for (const script of scripts) { + expect(existsSync(script)).toBe(true); + } + }); +}); diff --git a/server/tests/component-store.integration.test.ts b/server/tests/component-store.integration.test.ts index 9ea1153892..8f165219ef 100644 --- a/server/tests/component-store.integration.test.ts +++ b/server/tests/component-store.integration.test.ts @@ -1,6 +1,13 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + test, +} from "bun:test"; import { randomUUID } from "node:crypto"; -import { eq, inArray } from "drizzle-orm"; +import { eq, inArray, sql } from "drizzle-orm"; import { ComponentNotFoundError, type ComponentStore, @@ -9,6 +16,7 @@ import { import { createDatabase } from "../src/db/client"; import { agents, + auditEvents, componentExclusions, componentFunctions, components, @@ -58,6 +66,173 @@ beforeAll(async () => { await makeComponent(otherName, false); }); +describe("component data functions against PostgreSQL", () => { + // One connection keeps this temporary table visible to every query. Copy the migrated table + // shape so the production SQL runs unchanged, without depending on other suites' audit rows. + const reportsDatabase = createDatabase(testDatabaseUrl(), { max: 1 }); + const reports = createComponentStore(reportsDatabase); + const day = 86_400_000; + + beforeAll(async () => { + await reportsDatabase.execute( + sql`create temporary table audit_events (like public.audit_events including all)`, + ); + }); + + beforeEach(async () => { + await reportsDatabase.execute(sql`truncate table pg_temp.audit_events`); + }); + + afterAll(async () => { + await reportsDatabase.$client.close(); + }); + + function event(overrides: Partial) { + return { + eventType: "component.rendered", + targetType: "component", + payload: { bot: "active-bot" }, + createdAt: new Date(Date.now() - day), + ...overrides, + }; + } + + test("bot activity aggregates recent actions and excludes older and unattributed rows", async () => { + await reportsDatabase + .insert(auditEvents) + .values([ + event({}), + event({}), + event({ payload: { bot: "other-bot" } }), + event({ createdAt: new Date(Date.now() - 8 * day) }), + event({ payload: {} }), + ]); + + expect(await reports.callFunction("botActivity", {})).toEqual({ + days: 7, + rows: [ + { bot: "active-bot", actions: 2 }, + { bot: "other-bot", actions: 1 }, + ], + }); + }); + + test("bot activity caps the window at 90 days and returns only the 12 busiest bots", async () => { + const ranked = Array.from({ length: 13 }, (_, index) => ({ + bot: `ranked-bot-${index}`, + actions: 13 - index, + })); + await reportsDatabase.insert(auditEvents).values([ + ...ranked.flatMap(({ bot, actions }) => + Array.from({ length: actions }, () => + event({ + payload: { bot }, + createdAt: new Date(Date.now() - 30 * day), + }), + ), + ), + // If the time bound is ignored, this changes the highest-ranked bot's count. + event({ + payload: { bot: "ranked-bot-0" }, + createdAt: new Date(Date.now() - 91 * day), + }), + ]); + + expect( + await reports.callFunction("botActivity", { days: 100_000 }), + ).toEqual({ + days: 90, + rows: ranked.slice(0, 12), + }); + }); + + test("recent refusals filters event types, orders newest first and reads both reason formats", async () => { + const now = Date.now(); + const fixtures = [ + { + eventType: "computer.action_refused", + payload: { bot: "computer-bot", reason: "Permission denied" }, + }, + { + eventType: "component.refused", + payload: { + bot: "component-bot", + decision: { rule: "Component withheld" }, + }, + }, + { + eventType: "component.function_refused", + payload: { + bot: "function-bot", + reason: "Explicit reason", + decision: { rule: "Fallback reason" }, + }, + }, + { eventType: "bot.declined", payload: {} }, + ]; + await reportsDatabase + .insert(auditEvents) + .values([ + ...fixtures.map((fixture, index) => + event({ ...fixture, createdAt: new Date(now - (index + 1) * 1000) }), + ), + event({ eventType: "component.rendered", createdAt: new Date(now) }), + ]); + + expect(await reports.callFunction("recentRefusals", {})).toEqual({ + rows: [ + expect.objectContaining({ + bot: "computer-bot", + what: "computer.action_refused", + reason: "Permission denied", + }), + expect.objectContaining({ + bot: "component-bot", + what: "component.refused", + reason: "Component withheld", + }), + expect.objectContaining({ + bot: "function-bot", + what: "component.function_refused", + reason: "Explicit reason", + }), + expect.objectContaining({ + bot: null, + what: "bot.declined", + reason: null, + }), + ], + }); + }); + + test("recent refusals defaults to 10 and clamps the requested limit to 1 through 50", async () => { + const now = Date.now(); + const fixtures = Array.from({ length: 51 }, (_, index) => + event({ + eventType: "bot.declined", + payload: { bot: `refusal-bot-${index}`, reason: `Reason ${index}` }, + createdAt: new Date(now - index * 1000), + }), + ); + await reportsDatabase.insert(auditEvents).values(fixtures); + for (const { args, count } of [ + { args: {}, count: 10 }, + { args: { limit: 100_000 }, count: 50 }, + { args: { limit: 0 }, count: 1 }, + ]) { + expect(await reports.callFunction("recentRefusals", args)).toEqual({ + rows: fixtures.slice(0, count).map((_, index) => + expect.objectContaining({ + bot: `refusal-bot-${index}`, + what: "bot.declined", + reason: `Reason ${index}`, + }), + ), + }); + } + }); +}); + afterAll(async () => { // Register before writes so a failed assertion or partially completed setup still has teardown. const names = [...fixtureComponentNames]; diff --git a/server/tests/composio-connect-adopts-held-account.test.ts b/server/tests/composio-connect-adopts-held-account.test.ts index ba408a6285..ca29c1a3c9 100644 --- a/server/tests/composio-connect-adopts-held-account.test.ts +++ b/server/tests/composio-connect-adopts-held-account.test.ts @@ -306,8 +306,7 @@ describe("Connect, where Composio already holds this person's account and OpenBo // The row confirm writes for a consent app the vendor says yes for: this person's own. The // store records `connected_by` only for a deployment account, whose connector is not its holder. // The two ids are equal because production writes them equal for a person (`personAccount`); - // this expects the row the store wrote, so the collapse is recorded in - // `fixture-identity-distinct.test.ts` rather than separated. + // this expects that production equality rather than independent fixture identities. const rows = await rowsFor(toolkit); expect(rows).toHaveLength(1); expect(rows[0]).toMatchObject({ diff --git a/server/tests/people-paging.integration.test.ts b/server/tests/people-paging.integration.test.ts index 01009c8224..95ea6695e5 100644 --- a/server/tests/people-paging.integration.test.ts +++ b/server/tests/people-paging.integration.test.ts @@ -154,6 +154,47 @@ describe("reading the people in a deployment", () => { expect(result.people.length).toBeGreaterThan(0); }); + test("search treats underscores and percent signs as literal characters", async () => { + const underscore = await person(0, new Date(), "Ninety_ Percent"); + await person(1, new Date(), "NinetyX Percent"); + const percent = await person(2, new Date(), "Ninety% Percent"); + + for (const [search, wanted] of [ + ["Ninety_ Percent", underscore], + ["Ninety% Percent", percent], + ]) { + const result = await store.list({ search, limit: 10 }); + expect(result.people.map((one) => one.id)).toEqual([wanted]); + } + }); + + test("caps an oversized page at 200 and leaves the rest reachable", async () => { + const base = Date.now(); + const people = Array.from({ length: 201 }, (_, index) => ({ + id: `${PREFIX}cap-${String(index).padStart(3, "0")}`, + email: `${PREFIX}cap-${index}@openbot.test`, + name: `Page cap fixture ${index}`, + emailVerified: true, + lastSignedInAt: new Date(base - index * 1000), + })); + ids.push(...people.map((one) => one.id)); + await database.insert(users).values(people); + + const first = await store.list({ search: `${PREFIX}cap-`, limit: 100_000 }); + expect(first.people.map((one) => one.id)).toEqual( + people.slice(0, 200).map((one) => one.id), + ); + if (!first.nextCursor) + throw new Error("Expected a cursor for the 201st person."); + const last = await store.list({ + search: `${PREFIX}cap-`, + limit: 100_000, + cursor: first.nextCursor, + }); + expect(last.people.map((one) => one.id)).toEqual([people[200]?.id]); + expect(last.nextCursor).toBeNull(); + }); + test("finding one person does not read the deployment", async () => { const base = Date.now(); const wanted = await person(0, new Date(base)); diff --git a/server/tests/plugin-oauth.test.ts b/server/tests/plugin-oauth.test.ts index 6741e8f566..7b052737b4 100644 --- a/server/tests/plugin-oauth.test.ts +++ b/server/tests/plugin-oauth.test.ts @@ -3,6 +3,7 @@ import { seal } from "../src/auth/signed-value"; import type { CatalogueAuth } from "../src/plugins/catalogue"; import { authorizationUrlFor, + challengeFor, connectedAccountsUrlFor, createVerifier, readConnectState, @@ -219,6 +220,15 @@ describe("PKCE", () => { test("two verifiers are not the same", () => { expect(createVerifier()).not.toBe(createVerifier()); }); + + test("derives the S256 challenge from the verifier", () => { + // RFC 7636 Appendix B: https://www.rfc-editor.org/rfc/rfc7636#appendix-B + const verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"; + const challenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"; + expect(challengeFor(verifier)).toBe(challenge); + // Also reject an implementation that always returns the published example. + expect(challengeFor("a".repeat(43))).not.toBe(challenge); + }); }); describe("the address the person is sent to", () => { diff --git a/server/tests/support/database.test.ts b/server/tests/support/database.test.ts new file mode 100644 index 0000000000..ceec633da3 --- /dev/null +++ b/server/tests/support/database.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, test } from "bun:test"; +import { testDatabaseUrlFrom } from "./database"; + +describe("TEST_DATABASE_URL", () => { + test("requires an explicit test database URL", () => { + expect(() => testDatabaseUrlFrom({})).toThrow(/TEST_DATABASE_URL/); + }); + + test("refuses the live development database even when DATABASE_URL names it", () => { + expect(() => + testDatabaseUrlFrom({ + TEST_DATABASE_URL: "postgres://openbot:openbot@localhost:5432/openbot", + DATABASE_URL: "postgres://openbot:openbot@localhost:5432/openbot_test", + }), + ).toThrow(/live openbot database/); + }); + + test("does not read DATABASE_URL as a fallback", () => { + expect(() => + testDatabaseUrlFrom({ + DATABASE_URL: "postgres://openbot:openbot@localhost:5432/openbot_test", + }), + ).toThrow(/TEST_DATABASE_URL/); + }); + + test("accepts a dedicated test database", () => { + expect( + testDatabaseUrlFrom({ + TEST_DATABASE_URL: + "postgres://openbot:openbot@localhost:5432/openbot_test", + }), + ).toBe("postgres://openbot:openbot@localhost:5432/openbot_test"); + }); +}); diff --git a/server/tests/support/source-scan.test.ts b/server/tests/support/source-scan.test.ts new file mode 100644 index 0000000000..db402af9bc --- /dev/null +++ b/server/tests/support/source-scan.test.ts @@ -0,0 +1,145 @@ +// server/tests/support/source-scan.test.ts +/** + * Contract test for the shared scanner, written against the ENUMERATOR rather than the matcher. + * + * Every guard that grew its own copy of these primitives also grew a positive control, and every + * one of those controls plants its offender in a file the enumerator already visits. A control + * sited that way can only ever prove the pattern matches; it is structurally incapable of noticing + * that the walk never reached the file, that the extension filter excluded it, that the line number + * it reported was off by the height of a block comment, or that the offending expression was + * wrapped across two physical lines. So the cases below are the ones no guard's own control can + * reach: they build a fixture tree on disk, with nesting, extensions and formatting the real + * sources may grow at any time, and ask the util what it sees. + */ +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { logicalLines, sourceFiles, stripComments } from "./source-scan"; + +let root = ""; + +/** Write `text` at `rel` under the fixture root, creating the directories it needs. */ +function fixture(rel: string, text: string): string { + const full = join(root, rel); + mkdirSync(join(full, ".."), { recursive: true }); + writeFileSync(full, text, "utf8"); + return full; +} + +/** Paths relative to the fixture root, so an assertion reads as the tree it describes. */ +function scan(exts: string[]): string[] { + return sourceFiles(root, exts).map((p) => p.slice(root.length + 1)); +} + +const ALL = [".ts", ".tsx", ".js", ".jsx", ".mjs"]; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), "source-scan-")); + fixture("top.ts", "export const a = 1;\n"); + fixture("plain.js", "export const b = 2;\n"); + fixture("widget.jsx", "export const C = () => null;\n"); + fixture("module.mjs", "export const d = 4;\n"); + fixture("notes.md", "not source\n"); + fixture("nested/deep/buried.ts", "export const e = 5;\n"); + fixture("nested/index.ts", "export const f = 6;\n"); + fixture("other/index.ts", "export const g = 7;\n"); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +/* 1-4: WHAT THE WALK REACHES. Each of these is a file a non-recursive, .ts-only enumerator drops + * on the floor while its guard reports green. */ + +test("a .js file is enumerated", () => { + expect(scan(ALL)).toContain("plain.js"); + // And the extension list is honoured, so a guard that asks for .ts only still gets .ts only. + expect(scan([".ts"])).not.toContain("plain.js"); +}); + +test("a .jsx file is enumerated", () => { + expect(scan(ALL)).toContain("widget.jsx"); + expect(scan([".tsx"])).not.toContain("widget.jsx"); +}); + +test("an .mjs file is enumerated", () => { + expect(scan(ALL)).toContain("module.mjs"); + expect(scan(ALL)).not.toContain("notes.md"); +}); + +test("a file in a nested subdirectory is enumerated", () => { + expect(scan(ALL)).toContain(join("nested", "deep", "buried.ts")); + // The whole point: the directory the old enumerators stopped at holds more than it shows. + expect(scan(ALL).length).toBeGreaterThan(5); +}); + +/* 5: WHAT A LINE IS. */ + +test("a prettier-wrapped expression is one logical line", () => { + const src = [ + "const keep =", + " typeof search.x ===", + ' "string";', + "const other = 1;", + ].join("\n"); + const lines = logicalLines(src); + const wrapped = lines[0]; + expect(wrapped.line).toBe(1); + expect(wrapped.text).toMatch(/typeof\s+search\.x\s*===\s*"string"/); + // And it did not swallow the statement after it. + expect(lines[1]).toEqual({ line: 4, text: "const other = 1;" }); +}); + +/* 6-8: WHAT THE STRIPPER LEAVES BEHIND. */ + +test("a trailing comment does not remove its own code line", () => { + const src = 'const url = "composio://gmail"; // the scheme\n'; + const stripped = stripComments(src); + expect(stripped).toContain('const url = "composio://gmail";'); + expect(stripped).not.toContain("the scheme"); + // A stripper that drops the whole line would have failed its guard on a line holding no offence. + expect(logicalLines(stripped)[0].text.trim()).toBe( + 'const url = "composio://gmail";', + ); +}); + +test("a block comment spanning lines preserves subsequent line numbers", () => { + const src = [ + "/*", + " * three lines of prose", + " */", + 'const url = "arcade://gmail";', + ].join("\n"); + const stripped = stripComments(src); + expect(stripped.split("\n").length).toBe(4); + const offender = logicalLines(stripped).find((l) => + /arcade:\/\//.test(l.text), + ); + // 4, as the file reads it — not 1, which is what blanking a block comment to "" reports. + expect(offender?.line).toBe(4); +}); + +test("a same-line offence after whitespace is still visible", () => { + const src = ' /* why */ const url = "arcade://gmail"; /* trailing */\n'; + const stripped = stripComments(src); + expect(stripped).toContain('const url = "arcade://gmail";'); + expect(stripped).not.toContain("why"); + expect(stripped).not.toContain("trailing"); + // Columns are preserved too, so the code sits where it sat. + expect(stripped.split("\n")[0].length).toBe(src.length - 1); + expect(logicalLines(stripped)[0].line).toBe(1); +}); + +/* 9: WHAT A FILE IS CALLED. */ + +test("a duplicate-name decoy does not collide", () => { + const found = scan(ALL).filter((p) => p.endsWith("index.ts")); + expect(found.sort()).toEqual([ + join("nested", "index.ts"), + join("other", "index.ts"), + ]); + // Two distinct files with one basename: an enumerator keyed on the name would report one. + expect(new Set(found).size).toBe(2); +}); diff --git a/server/tests/support/source-scan.ts b/server/tests/support/source-scan.ts new file mode 100644 index 0000000000..ad776a671d --- /dev/null +++ b/server/tests/support/source-scan.ts @@ -0,0 +1,168 @@ +// server/tests/support/source-scan.ts +/** + * The four primitives every text-scanning guard needs, in one place: enumerate the files, strip the + * comments, split into lines a pattern can be matched against. + * + * WHY THIS EXISTS. Five guards each grew their own copy of these, and every copy had the same shape + * of blind spot — not in the pattern it matches, but in the text it ever gets to see: + * + * - a NON-RECURSIVE `readdirSync`, so a rule stated over a directory silently stops at the first + * subdirectory somebody adds. {@link sourceFiles} walks. + * - a comment stripper that replaces a block comment with `""`, NEWLINES AND ALL, so every line + * after it is reported at the wrong number — or one that drops whole lines, which turns a + * trailing `// note` into the removal of the code it trails and can fail a guard on a line that + * holds no offence. {@link stripComments} blanks a comment in place: same bytes out as in, minus + * the comment, with every newline where it was. + * - a per-PHYSICAL-line match, so a prettier-wrapped `typeof search.x ===\n"string"` is two lines + * to the guard and one expression to the compiler, and the guard sees neither half. + * {@link logicalLines} joins a wrapped expression back up and reports the line it STARTED on. + * + * Each guard's positive control plants its offender in a file the enumerator already visits, which + * is exactly why none of these were caught by the controls: a control sited that way proves the + * matcher and says nothing about the enumerator. `source-scan.test.ts` is the contract test for the + * enumerator half, over fixtures it writes itself. + */ +import { readdirSync } from "node:fs"; +import { join } from "node:path"; + +/** + * Every file under `root` whose name ends in one of `exts`, RECURSIVELY, sorted. + * + * Sorted so an offender list is stable, and recursive so `src/plugins//x.ts` is scanned the day + * it lands rather than the day somebody notices. Symlinks are not followed (a `node_modules` link is + * not source). + */ +export function sourceFiles(root: string, exts: string[]): string[] { + const out: string[] = []; + for (const entry of readdirSync(root, { withFileTypes: true })) { + const full = join(root, entry.name); + if (entry.isDirectory()) out.push(...sourceFiles(full, exts)); + else if (exts.some((ext) => entry.name.endsWith(ext))) out.push(full); + } + return out.sort(); +} + +/** + * Line and block comments removed, EVERY OTHER BYTE POSITION LEFT ALONE. + * + * A comment is overwritten with spaces and its newlines are kept, so line numbers — and columns — + * are those of the real file. String literals are copied through untouched, so a `"// not a + * comment"` or an `"https://host"` inside a string survives; a single- or double-quoted string is + * taken to end at a newline, as it does in the language. Regex literals are not parsed, which is the + * known limit: a regex holding `//` or `/*` can still be read as a comment. + */ +export function stripComments(text: string): string { + let out = ""; + let i = 0; + while (i < text.length) { + const c = text[i]; + if (c === '"' || c === "'" || c === "`") { + let j = i + 1; + while (j < text.length) { + if (text[j] === "\\") { + j += 2; + continue; + } + if (text[j] === c) { + j++; + break; + } + if (c !== "`" && text[j] === "\n") break; + j++; + } + out += text.slice(i, j); + i = j; + } else if (c === "/" && text[i + 1] === "/") { + while (i < text.length && text[i] !== "\n") { + out += " "; + i++; + } + } else if (c === "/" && text[i + 1] === "*") { + const end = text.indexOf("*/", i + 2); + const stop = end === -1 ? text.length : end + 2; + for (; i < stop; i++) out += text[i] === "\n" ? "\n" : " "; + } else { + out += c; + i++; + } + } + return out; +} + +/** A logical line, and the 1-based PHYSICAL line it started on. */ +export type LogicalLine = { line: number; text: string }; + +/** Brackets opened and not closed on this line, strings skipped. */ +function bracketDelta(line: string): number { + let delta = 0; + let i = 0; + while (i < line.length) { + const c = line[i]; + if (c === '"' || c === "'" || c === "`") { + i++; + while (i < line.length && line[i] !== c) i += line[i] === "\\" ? 2 : 1; + i++; + continue; + } + if (c === "(" || c === "[") delta++; + else if (c === ")" || c === "]") delta--; + i++; + } + return delta; +} + +/** Ends on something that cannot end a statement, so the expression continues on the next line. */ +const DANGLING = /(?:=>|[-+*%<>=!&|?:.,^~])$/; + +/** + * The most physical lines one logical line may absorb. + * + * A wrapped EXPRESSION is short — prettier breaks at 80 columns, so the two or three lines of a + * `typeof x ===` / `"string"` or an `eq(` / arg / arg / `)` are the whole population this exists + * for. A bracket, by contrast, can stay open for the length of a route handler, and joining a + * hundred-line `routes.get("/", …)` into one line would report every offence inside it at the + * handler's first line and splice its docblocks onto its code. So the join stops here and the + * remaining lines start a new logical line: a long call is read as the many lines it is written on, + * which is what the old per-physical-line scan did anyway. + */ +const MAX_JOIN = 5; + +/** + * Physical lines joined back into the expressions they were wrapped out of. + * + * Prettier breaks a long expression at an operator or inside a bracket, so `typeof search.x ===` + * and `"string"` are one decision written on two lines. A guard matching per physical line sees + * neither half and reports green. Lines are joined with a single space while a bracket is open or + * the line ends dangling, for at most {@link MAX_JOIN} physical lines; `{` and `}` are NOT counted, + * so a block body stays many logical lines rather than collapsing into one. Comments do not get a + * vote on where a logical line ends — their brackets and their trailing operators are discounted, + * though their text is kept in the joined line, since whether prose is an offence is the caller's + * rule and not this function's. The reported `line` is where the logical line STARTED, which is the + * line a reader is sent to. + */ +export function logicalLines(text: string): LogicalLine[] { + const out: LogicalLine[] = []; + const physical = text.split("\n"); + let start = 0; + let buffer = ""; + let depth = 0; + let open = false; + for (let i = 0; i < physical.length; i++) { + const raw = physical[i]; + if (!open) { + start = i; + buffer = raw; + depth = 0; + } else { + buffer += ` ${raw.trim()}`; + } + const code = stripComments(raw); + depth += bracketDelta(code); + open = depth > 0 || DANGLING.test(code.trimEnd()); + if (open && i - start + 1 >= MAX_JOIN) open = false; + if (open && i < physical.length - 1) continue; + out.push({ line: start + 1, text: buffer }); + open = false; + } + return out; +} diff --git a/server/tests/support/stub-broker.ts b/server/tests/support/stub-broker.ts index 16aadc51e9..feb86c8230 100644 --- a/server/tests/support/stub-broker.ts +++ b/server/tests/support/stub-broker.ts @@ -8,7 +8,7 @@ * - CAST PAST THE TYPE. `{ … } as unknown as ConnectedAppBroker` or `{} as ConnectedAppBroker` * makes a stub that omits `revoke` and `revokeAccount` compile. The compiler's guarantee is * gone exactly as it was when a broken `import type` degraded the type to `any` (round-2 C8). - * `broker-stub-ratchet.test.ts` stops new casts; this factory is what to write instead. + * Use this factory instead so missing methods remain type errors. * - A PLAUSIBLE DEFAULT. `revokeAccount: async () => {}` type-checks and answers "done" for a * revoke that never happened. A test can then pass on a fiction, and the revoke seam is the * one this PR's Claim 2 is about. diff --git a/server/tests/tenant-package.test.ts b/server/tests/tenant-package.test.ts index 091c01b758..daa747acbe 100644 --- a/server/tests/tenant-package.test.ts +++ b/server/tests/tenant-package.test.ts @@ -1,5 +1,8 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; import { randomUUID } from "node:crypto"; +import { cp, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { fileURLToPath } from "node:url"; import { and, eq } from "drizzle-orm"; import { createAgentProfileStore } from "../src/agents/profile-store"; @@ -1100,6 +1103,42 @@ describe("expanding a package file against the environment", () => { * cases below are the ones that decide whether that is safe to seed. */ describe("skills a package ships", () => { + test("editing only skills.yaml changes the loaded package checksum", async () => { + const directory = await mkdtemp(join(tmpdir(), "openbot-skills-checksum-")); + try { + await cp( + fileURLToPath(new URL("../../examples/fintech", import.meta.url)), + directory, + { recursive: true }, + ); + const skillsPath = join(directory, "skills.yaml"); + const original = await readFile(skillsPath, "utf8"); + const before = await loadTenantPackage(directory); + expect((await loadTenantPackage(directory)).checksum).toBe( + before.checksum, + ); + + await writeFile( + skillsPath, + `${original}\n - slug: checksum-fixture\n title: Checksum fixture\n summary: Added instruction.\n instructions: Read the revised policy.\n`, + "utf8", + ); + const after = await loadTenantPackage(directory); + expect(after.skills).toHaveLength(before.skills.length + 1); + expect(after.skills.at(-1)?.instructions).toBe( + "Read the revised policy.", + ); + expect(after.checksum).not.toBe(before.checksum); + + await writeFile(skillsPath, original, "utf8"); + expect((await loadTenantPackage(directory)).checksum).toBe( + before.checksum, + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + const base = { brand: "tenant: { id: fintech, product_name: Ledgerline }", agents: diff --git a/server/tests/test-network-guard.test.ts b/server/tests/test-network-guard.test.ts new file mode 100644 index 0000000000..9fe0567972 --- /dev/null +++ b/server/tests/test-network-guard.test.ts @@ -0,0 +1,446 @@ +import { describe, expect, test } from "bun:test"; +import dgram from "node:dgram"; +import http, { request as namedHttpRequest } from "node:http"; +import http2 from "node:http2"; +import https, { get as namedHttpsGet } from "node:https"; +import net from "node:net"; +import tls from "node:tls"; +import { createComposioClient } from "../src/plugins/composio-adapter"; + +/** + * The guard that makes a live vendor call impossible, asserted rather than assumed — ONE ENTRY + * POINT AT A TIME. + * + * `server/scripts/test-network-guard.ts` runs from the preload, before any test file is imported. + * Its first version patched `globalThis.fetch` and `http(s).request` only, and this file asserted + * it through `fetch` only, so it stayed green while `https.get` returned 200 from example.com and + * `Bun.fetch` was not guarded at all. A guard is exactly as wide as the entry points its test + * plants a call through, so every entry point a test file could open a connection with is planted + * below, separately, and each one must end in a refusal the GUARD recorded during THAT call. + * + * Why the record rather than the error: a refusal does not always reach the caller as itself (the + * Composio SDK retries and says "Connection error.", and a native `Bun.fetch` sees the refusing + * proxy's 403), and an error alone cannot tell a guarded call from a dialled one that failed. Why + * only the entries recorded during the call: the record is process-wide, so a check against the + * whole of it would be satisfied by whichever earlier test happened to be refused. + * + * example.com is the far side throughout: the assertion is that NOTHING is dialled, so the address + * should be one no product code would ever have reason to reach. + */ + +const FAR = "example.com"; + +/** Runs one planted call and returns what it came to and what the guard recorded while it ran. */ +async function plant( + call: () => unknown, +): Promise<{ outcome: string; refused: string[] }> { + const record = globalThis.openbotTestNetworkRefusals; + if (!record) throw new Error("the network guard did not install its record"); + const before = record.length; + let outcome: string; + try { + const value = await call(); + outcome = + value instanceof Response + ? `response ${value.status} ${value.headers.get("x-openbot-test-network-guard") ?? "unguarded"}` + : "returned"; + } catch (error) { + outcome = `threw: ${error instanceof Error ? error.message : String(error)}`; + } + return { outcome, refused: record.slice(before) }; +} + +const refusedCall = "Tests do not use the network"; + +/** + * What a caller of a NATIVE fetch sees when the refusing proxy turns it away. That text is not the + * guard's: over https the runtime reports the proxy's 403 itself, and how it words it differs by + * platform and version (macOS bun 1.4.2 hands back the guard's own 403 response; CI's Linux runner + * throws "Proxy refused to open a tunnel: 403 Forbidden"). So the outcome is pinned only as far as + * the guard owns it: EITHER the guard's own 403, carrying its header, OR a throw — never a response + * the guard did not write. A call that had really been dialled ends in such a response (the red at + * the C7 base was "response 404 unguarded"). That the GUARD stopped it is then asserted on the + * guard's record, which its proxy writes only when the request was handed to it. + */ +const refusedByProxy = /^(response 403 refused|threw: .+)$/s; + +/** Each entry point, the call it plants, the outcome the caller sees and the target recorded. */ +const entryPoints: Array<[string, () => unknown, string | RegExp, string]> = [ + [ + "globalThis.fetch", + () => fetch(`https://${FAR}/x`), + refusedCall, + `https://${FAR}/x`, + ], + [ + "Bun.fetch", + () => Bun.fetch(`https://${FAR}/x`), + refusedByProxy, + `https://${FAR}`, + ], + [ + "Bun.fetch over http", + () => Bun.fetch(`http://${FAR}/x`), + refusedByProxy, + `http://${FAR}/x`, + ], + [ + "undici fetch", + () => + (require("undici") as { fetch: typeof fetch }).fetch(`https://${FAR}/x`), + refusedByProxy, + `https://${FAR}`, + ], + [ + "undici request", + async () => { + const { statusCode, headers } = await ( + require("undici") as { + request: (url: string) => Promise<{ + statusCode: number; + headers: Record; + }>; + } + ).request(`https://${FAR}/x`); + return new Response(null, { status: statusCode, headers }); + }, + refusedByProxy, + `https://${FAR}`, + ], + ["http.request", () => http.request(`http://${FAR}/x`), refusedCall, FAR], + [ + "https.request", + () => https.request({ hostname: FAR, path: "/x" }), + refusedCall, + FAR, + ], + ["http.get", () => http.get(`http://${FAR}/x`), refusedCall, FAR], + ["https.get", () => https.get(`https://${FAR}/x`), refusedCall, FAR], + [ + "named import of https.get", + () => namedHttpsGet(`https://${FAR}/x`), + refusedCall, + FAR, + ], + [ + "http.ClientRequest", + () => new http.ClientRequest(`http://${FAR}/x`), + refusedCall, + FAR, + ], + ["net.connect", () => net.connect(80, FAR), refusedCall, `socket ${FAR}`], + [ + "net.Socket#connect", + () => new net.Socket().connect({ host: FAR, port: 80 }), + refusedCall, + `socket ${FAR}`, + ], + [ + "tls.connect", + () => tls.connect(443, FAR, { servername: FAR }), + refusedCall, + `socket ${FAR}`, + ], + [ + "Bun.connect", + () => Bun.connect({ hostname: FAR, port: 80, socket: { data() {} } }), + refusedCall, + `socket ${FAR}`, + ], + [ + "http2.connect", + () => http2.connect(`https://${FAR}`), + refusedCall, + `http2 ${FAR}`, + ], + [ + "WebSocket", + () => new WebSocket(`wss://${FAR}/x`), + refusedCall, + `wss://${FAR}/x`, + ], + [ + "ws WebSocket", + () => + new (require("ws") as { WebSocket: typeof WebSocket }).WebSocket( + `wss://${FAR}/x`, + ), + refusedCall, + `wss://${FAR}/x`, + ], + [ + "dgram send", + () => { + const socket = dgram.createSocket("udp4"); + try { + socket.send("x", 9, FAR); + } finally { + socket.close(); + } + }, + refusedCall, + `udp ${FAR}`, + ], + [ + "Bun.udpSocket send", + async () => { + const socket = await Bun.udpSocket({}); + try { + socket.send("x", 9, FAR); + } finally { + socket.close(); + } + }, + refusedCall, + `udp ${FAR}`, + ], +]; + +describe("the test network guard refuses a host that is not loopback, through", () => { + test.each(entryPoints)("%s", async (_name, call, seen, target) => { + const { outcome, refused } = await plant(call); + + // A string is matched as a substring, a pattern as itself. + expect(outcome).toMatch(seen); + expect(refused).toEqual([target]); + }); +}); + +describe("the test network guard lets the suite reach what the suite owns", () => { + test("a loopback HTTP server, through fetch, Bun.fetch and node:http", async () => { + const server = Bun.serve({ port: 0, fetch: () => new Response("local") }); + try { + const { outcome, refused } = await plant(async () => { + const answers = [ + await (await fetch(`http://127.0.0.1:${server.port}/`)).text(), + await (await Bun.fetch(`http://localhost:${server.port}/`)).text(), + await new Promise((resolve, reject) => { + http + .get(`http://127.0.0.1:${server.port}/`, (response) => { + let body = ""; + response.on("data", (chunk) => (body += chunk)); + response.on("end", () => resolve(body)); + }) + .on("error", reject); + }), + ]; + return answers.join(","); + }); + + expect(outcome).toBe("returned"); + expect(refused).toEqual([]); + } finally { + await server.stop(true); + } + }); + + test("a loopback socket, through net.connect", async () => { + const listener = net.createServer((socket) => socket.end("local")); + await new Promise((resolve) => + listener.listen(0, "127.0.0.1", resolve), + ); + const { port } = listener.address() as net.AddressInfo; + try { + const { outcome, refused } = await plant( + () => + new Promise((resolve, reject) => { + const socket = net.connect(port, "127.0.0.1"); + let body = ""; + socket.on("data", (chunk) => (body += chunk)); + socket.on("end", () => resolve(body)); + socket.on("error", reject); + }), + ); + + expect(outcome).toBe("returned"); + expect(refused).toEqual([]); + } finally { + listener.close(); + } + }); + + /** + * The database is the one far NAME a raw socket may reach without its lookup being checked, + * because it is deployment configuration rather than a vendor. `home.arpa` (RFC 8375) never + * resolves in public DNS, so the connection fails in DNS — and the point is that it got as far as + * DNS instead of being refused. Not `.invalid`: that is a special-use name the guard lets through + * on its own terms, so a case aimed at one could not tell whether the database allowance exists. + */ + test("the configured database host, through a raw socket", async () => { + const configured = process.env.TEST_DATABASE_URL; + process.env.TEST_DATABASE_URL = + "postgres://u:p@db.openbot.home.arpa:5432/x"; + try { + const { outcome, refused } = await plant( + () => + new Promise((_resolve, reject) => { + net.connect(5432, "db.openbot.home.arpa").on("error", reject); + }), + ); + + expect(outcome).not.toContain(refusedCall); + expect(refused).toEqual([]); + } finally { + if (configured === undefined) delete process.env.TEST_DATABASE_URL; + else process.env.TEST_DATABASE_URL = configured; + } + }); +}); + +/** + * A suite whose SUBJECT is the socket layer — `agent-computer`'s egress filter, which proves it + * pins the address it checked, falls back, and gives up on one that never answers — has to open + * sockets at addresses that are not loopback. The guard lets through only what cannot be a vendor: + * a literal address the public internet does not route, and a reserved special-use name (RFC 6761) + * ONLY when what it resolves to is such an address. Each allowance is planted next to the far + * address it must still refuse, so widening it into "anything goes" turns these red. + */ +describe("the test network guard judges a socket by the address it reaches", () => { + /** A `lookup` that answers one address, in both shapes the runtime asks for (Bun asks `all`). */ + function answering(address: string): net.LookupFunction { + return (_host, options, callback) => + options.all + ? callback(null, [{ address, family: 4 }]) + : callback(null, address, 4); + } + + /** Settles on connect, on a short wait (an unroutable address may hang), or rejects on error. */ + function dial(open: () => net.Socket): Promise { + return new Promise((resolve, reject) => { + const socket = open(); + const timer = setTimeout(() => { + socket.destroy(); + resolve("pending"); + }, 200); + socket.once("connect", () => { + clearTimeout(timer); + const reached = `connected ${socket.remoteAddress}`; + socket.destroy(); + resolve(reached); + }); + socket.once("error", (error) => { + clearTimeout(timer); + socket.destroy(); + reject(error); + }); + }); + } + + test("an unroutable private address is let through; a public one and the metadata service are not", async () => { + const privately = await plant(() => + dial(() => net.connect({ host: "10.255.255.1", port: 9 })), + ); + expect(privately.outcome).not.toContain(refusedCall); + expect(privately.refused).toEqual([]); + + const publicly = await plant(() => + dial(() => net.connect({ host: "93.184.215.14", port: 80 })), + ); + expect(publicly.outcome).toContain(refusedCall); + expect(publicly.refused).toEqual(["socket 93.184.215.14"]); + + const metadata = await plant(() => + dial(() => net.connect({ host: "169.254.169.254", port: 80 })), + ); + expect(metadata.outcome).toContain(refusedCall); + expect(metadata.refused).toEqual(["socket 169.254.169.254"]); + }); + + test("a .test name is let through when its lookup answers loopback", async () => { + const listener = net.createServer((socket) => socket.end()); + await new Promise((resolve) => + listener.listen(0, "127.0.0.1", resolve), + ); + const { port } = listener.address() as net.AddressInfo; + try { + let reached = ""; + const { refused } = await plant(async () => { + reached = await dial(() => + net.connect({ + host: "pinned.example.test", + port, + lookup: answering("127.0.0.1"), + }), + ); + }); + + expect(reached).toBe("connected 127.0.0.1"); + expect(refused).toEqual([]); + } finally { + listener.close(); + } + }); + + /* + * A reserved name is only reserved in PUBLIC DNS: a `lookup` option (or /etc/hosts) can map it to + * any address at all, which is exactly how a test could smuggle a vendor's address past a + * name-based allowance. Planted through the socket and through a named `node:http` import, which + * the request-layer patch does not reach and which therefore meets only the socket guard. + */ + const smuggle = answering("93.184.215.14"); + + test.each<[string, () => Promise]>([ + [ + "net.connect", + () => + dial(() => + net.connect({ + host: "smuggled.example.test", + port: 80, + lookup: smuggle, + }), + ), + ], + [ + "a named import of http.request", + () => + new Promise((resolve, reject) => { + namedHttpRequest({ + host: "smuggled.example.test", + port: 80, + lookup: smuggle, + }) + .on("response", () => resolve("answered")) + .on("error", reject) + .end(); + }), + ], + ])( + "a .test name whose lookup answers a public address is refused, through %s", + async (_name, call) => { + const { outcome, refused } = await plant(call); + + expect(outcome).toContain(refusedCall); + expect(refused).toEqual([ + "socket smuggled.example.test resolved to 93.184.215.14", + ]); + }, + ); +}); + +/** + * The case the guard was written for, in the shape that defeated the per-file stub: the vendor + * client is constructed with no stub in sight, and its own bound transport is the guard. + */ +test("the test network guard is in force for a vendor SDK that binds its own transport", async () => { + const { broker } = createComposioClient("never-dialled"); + + const { outcome, refused } = await plant(() => + broker.authorize({ + account: { holder: "person", userId: "u", vendorUserId: "u" }, + toolkit: "linear", + returnUrl: "https://openbot.test/connections/x", + }), + ); + + expect(outcome).not.toBe("returned"); + /* + * ASSERTED ON THE GUARD'S OWN RECORD RATHER THAN THE MESSAGE, because the SDK retries the + * rejected transport and reports "Connection error." — the same sentence a real outage gives, + * so a test that read it could not tell a guarded call from a dialled one that failed. + */ + expect(refused.length).toBeGreaterThan(0); + expect( + refused.every((target) => target.includes("backend.composio.dev")), + ).toBe(true); +}); diff --git a/test-audit.md b/test-audit.md deleted file mode 100644 index a2e3aff61f..0000000000 --- a/test-audit.md +++ /dev/null @@ -1,134 +0,0 @@ -# OpenBot test audit - -Base: `origin/main` at `c19d0653`. Branch: `codex/openbot-test-audit`. - -## Criteria and scope - -Reviewed the original 811 test-bearing files: 740 TypeScript/TSX files, 39 Python files, and 32 Rust modules. The review checked whether each case exercises a meaningful OpenBot user behavior, whether its assertions establish its stated claim, and whether a relevant defect can make it fail. Larger suites were read as compact callback bodies, with shared fixtures, helpers and implementation expanded where needed. Compile-time type contracts, executable configuration contracts, dependency-boundary mocks and opt-in live tests were retained where they protect product behavior. - -Removed source-spelling and test-roster guards, tests of test-only infrastructure, tautologies, and claims unsupported by the fixtures or assertions. Infrastructure tests were removed for the requested end-user scope; they were not all incapable of failing. No production behavior was changed. - -## Confirmed false positives - -Each temporary production mutation below left the selected original test passing. All mutations were restored before editing tests. - -| Claimed protection | Mutation that survived | -| --- | --- | -| PKCE S256 challenge | Return the constant `"x"` instead of the SHA-256 digest. | -| People page-size cap | Remove the `MAX_PAGE` clamp. The fixture had only three people. | -| Literal wildcard search | Remove `escapeLike`. The fixture had no competing wildcard match. | -| Component data-query bounds | Return the parsed argument without min/max clamping. Assertions checked only that a result existed. | -| Complete Arcade connection listing | Stop after the first page. The test asked only about the first connection. | - -The desktop stop-profile test also searched its own file for a string already present inside its assertion, so removing the production command could not make it fail. - -## Removals - -Removed 169 source test declarations across 76 test files, including two unreachable placeholder declarations. Deleted 21 whole test files and one unused test helper. Retained the CrewAI deployment-configuration test with a corrected title. - -Counts below are source test declarations, before table/loop expansion. The deleted source-scanner helper has no test declarations. - -| File | Removed declarations | Reason | -| --- | ---: | --- | -| `agent-adk/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-ag2/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-agno/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-antigravity/tests/test_model_spec.py` | 2 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-computer/tests/aria-snapshot.test.ts` | 2 | Fixture-only YAML parsing never calls production parser | -| `agent-computer/tests/viewer.test.ts` | 2 | No launch exception or during-teardown observation; adjacent real lifecycle tests retained | -| `agent-crewai/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-crewai/tests/test_token_restoration.py` | 1 | Exercises teardown of another pytest test, not shipped bot behavior (six parameter combinations). | -| `agent-langgraph-agui/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-langroid/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-llamaindex/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-microsoft/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-pydantic-ai/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `agent-strands/tests/test_model_spec.py` | 1 | Source spelling checks do not execute bot configuration; dead references satisfy assertions. | -| `app/tests/admin-plugins-vocabulary-wiring.test.ts` | 3 | Only scans route-source spellings. A function name in dead code satisfies the positive check; indirect vendor text/navigation avoids the negative regex. Does not render either provider or exercise routing. Real provider UI coverage remains in admin-brokered-row-provider, arcade-directory-screen and connected-account tests. | -| `app/tests/arcade-no-browser-calls.test.ts` | 1 | Only regex-scans source strings (including comments); cannot observe browser network calls and can pass when host/path is assembled at runtime. No actual browser request or import graph is exercised. | -| `app/tests/composer-queue-attachments.test.tsx` | 1 | Retain attachment queue, failure restoration, cap, removal, and lifecycle behaviors. Selected test asserts identities collected by its own useCallback wrappers (handedOut sets), testing the measuring harness rather than a user-visible application outcome. | -| `app/tests/form-submit-controls.test.ts` | 1 | Source AST presence scan never renders/clicks controls. Any JSX component with type="submit" counts even if it ignores the prop or is disabled; handlers and actual submitted payloads are never exercised. Claim can pass for inert forms. | -| `app/tests/plugin-mutation-refresh.test.ts` | 2 | Retain mutation execution tests observing refused-write invalidation, actual hostile-ID URLs and consent return targets. Remove URL source regex and exported-factory test-roster bookkeeping; neither executes the behavior it is intended to guarantee. | -| `app/tests/search-text.test.ts` | 1 | Retain real searchText, searchTextEntry and parseSearchText URL round trips. Remove final source regex guard; aliasing search or using another conditional spelling evades it without preserving the URL value. | -| `app/tests/theme-preference.test.ts` | 4 | Retain parseStoredDarkTheme/applyDarkTheme behavior and actual stylesheet color-scheme declarations. Four pre-paint tests only find text/script tag attributes: an inert inline script plus comments or unreachable theme code passes all checks without applying a theme. | -| `app/tests/transcript-attachments.test.tsx` | 1 | Retain rendering, missing-attachment, lightbox, queue, MIME and comparator behavior tests. Selected assertion checks React.memo internal .compare identity without a rerender or observable preservation/performance outcome; it couples to implementation wiring. | -| `bunfig.toml` | 0 | Removed the stale comment reference to the deleted test-preload guard; preload configuration is unchanged. | -| `desktop/src-tauri/src/engine.rs` | 1 | Constructs EngineStatus locally with engine=None and responding=false, then asserts those exact fields. No detect or transformation invocation; setup forces success. | -| `desktop/src-tauri/src/main.rs` | 2 | Source-text self-scans: one checks another test source for call spelling; the other splits the first handler macro source and searches names. Neither invokes production behavior or IPC registration. Source-scans another test body for ask_the_bot( and never executes production transport or vault access. Tests test-authoring style, not end-user behavior. Source-splits text following generate_handler! and searches for two command-name spellings. Never attempts IPC or inspects compiled handler behavior, so syntactic registration refactors or another dispatch path evade it. | -| `desktop/src-tauri/src/plan.rs` | 1 | Only scans CHATGPT_LOGIN source for json.dumps(raw and absence of the retired marker. Does not execute the login program or inspect printed credentials; commented/dead print satisfies the claimed behavior. | -| `desktop/src-tauri/src/stack.rs` | 1 | Reads its own entire source and searches for .args([...harness, down]); that literal occurs in the test needle itself. Production command can disappear without failure, and Stop is never invoked. Reads its own entire source with include_str!("stack.rs") and searches for a literal .args([..."harness", "down"]) which already occurs inside that assertion’s raw-string needle. Even deleting the production stop command leaves the test green. It never invokes Stop. | -| `desktop/src-tauri/src/test_support.rs` | 2 | Executes only test-support utilities: scratch path uniqueness and compilation/execution of a synthetic fixture. No shipped end-user behavior. Helper implementations are preserved. Only tests test-only support utilities: scratch path uniqueness and compiling/running a synthetic fixture. Neither exercises shipped application behavior. Preserve helper functions and actual application tests using them. | -| `scripts/mutation-register.test.ts` | 7 | Only checks the mutation-runner register, source anchors, fixture transformations and test-file existence; does not run product mutations or user behavior. | -| `scripts/test-ci.test.ts` | 1 | Exercises test-runner log plumbing using a synthetic failed child; no shipped OpenBot use case. | -| `server/tests/agent-escalation.test.ts` | 2 | Default-person claim checks only undefined optional initiator on fake audit recorder. No row existence assertion and no real audit defaulting runs. Also remove tool-name equality: constructs escalationTool and compares name to its exported constant, without offering a tool to any run or evaluating grants. | -| `server/tests/agent-handoff.test.ts` | 1 | Default-person claim checks only undefined optional initiator on fake audit recorder. No row existence assertion and no real audit defaulting runs. | -| `server/tests/agent-profile-policy.test.ts` | 1 | Keep actual access/run/manage permission scenarios; function identity equality merely pins alias implementation already behaviorally covered. | -| `server/tests/arcade-attempt-schema.test.ts` | 3 | Three scenarios execute test-local claim SQL duplicating the application upsert, expiration, and completed-at logic; they do not call production claimAuthorizationAttempt. Production store integration covers these cases. | -| `server/tests/arcade-live.test.ts` | 1 | Preserve opt-in vendor scenarios. Diagnostic print test calls local say and only checks test PORT is integer, proving neither output nor ordering and exercising no production. | -| `server/tests/arcade-pagination-engine.test.ts` | 1 | Pagination and truncation behavior exercised through real SDK/server, except one first-page positive test that claims a whole-list read and survives stopping at first page. | -| `server/tests/arcade-transport-registry.test.ts` | 1 | Keep transport lookup and top-level compile-time contracts; runtime literal test merely compares test-local assignments with the same literals. | -| `server/tests/arcade-vendor-read-contract.test.ts` | 4 | Keep vendor-field behavioral matrix via real broker/transport. Binding-name source regex only checks code presence; two debt-list checks operate solely on the file-local empty KNOWN_FAIL_OPEN object and always pass for current setup. Local ROWS key uniqueness is test-fixture bookkeeping without production behavior. | -| `server/tests/arcade-verifier-notice.test.ts` | 2 | Keep notice function behavior; remove source-string checks posing as mounted-route/startup execution coverage. | -| `server/tests/assertion-shape.test.ts` | 9 | Only enforces test-authoring/source-spelling conventions and tests the local scanner using synthetic source snippets; no executed end-user product behavior. These are lint/architecture guards, outside the requested user-use-case test criterion. | -| `server/tests/attachment-routes.test.ts` | 1 | Retain real HTTP/database upload, download, membership, draft privacy, concurrency, size, header and send-stamp behaviors. Remove local test-helper diagnostic: it calls only waitForBlockedSession defined in this same test file and compares its testing timeout constants, with no shipped behavior exercised. | -| `server/tests/audit.test.ts` | 2 | Keep redaction and admin API behavior. String occurrence does not prove event emission; SQL-chain text does not prove effective database trigger behavior (covered by real retention integration). | -| `server/tests/broker-authorization-pairing.test.ts` | 3 | Source regex parses broker/stub object literals and source coverage counts; never executes production authorization or callback pairing. | -| `server/tests/broker-stub-ratchet.test.ts` | 6 | Only enforces test-authoring/source-spelling conventions and tests the local scanner using synthetic source snippets; no executed end-user product behavior. These are lint/architecture guards, outside the requested user-use-case test criterion. | -| `server/tests/brokered-provenance.test.ts` | 2 | Only enforces test-authoring/source-spelling conventions and tests the local scanner using synthetic source snippets; no executed end-user product behavior. These are lint/architecture guards, outside the requested user-use-case test criterion. | -| `server/tests/bunfig-preload.test.ts` | 2 | Only validates the test runner preload configuration and script existence, with no shipped end-user behavior. Root final consistency review: verifies Bun preload settings for test execution only; no end-user product behavior. Configuration implementation remains unchanged. | -| `server/tests/channel-activity.integration.test.ts` | 1 | Retain real persistent pagination, ties, channel integrity, membership, recency, pins and direct-conversation races. Remove maximum-page test: seeds only one channel, requests100000, asserts<=200, so uncapped production query still satisfies it. | -| `server/tests/component-store.integration.test.ts` | 1 | Only asserts defined results with no row-content or limit assertions; removing bounded argument caps still passes (mutation verified). Actual access/grant HTTP flows, database state, publication decisions and component permissions. | -| `server/tests/composio-adapter.test.ts` | 1 | Enumerates adapter method names and compares them with the test table, then checks non-method member names. It asserts test coverage bookkeeping and API shape without invoking any adapter behavior. Runtime vendor-failure table cases are retained. | -| `server/tests/composio-broker-seam.test.ts` | 1 | Keep real broker return-URL/error formatting and declared authorization capabilities. Remove action-ignored claim: empty auth-config listing forces independent refusal, all errors become strings, and the sole assertion accepts any message lacking action; a successful authorization path is never exercised. | -| `server/tests/composio-connection-kinds.test.ts` | 5 | Actual scheme classification and all36 named consumer/probe callbacks are meaningful. Remove only3test-table bookkeeping checks and2unreachable declared-only callbacks, plus dead scaffolding. | -| `server/tests/composio-connections.test.ts` | 1 | Actual store/database connection lifecycle, probes, gates, audit, idempotency, duplicate rows and failures. Remove local cleanup-helper test only. | -| `server/tests/composio-grants-for.test.ts` | 1 | Keep real broker grant tests and required-field compile-time contract; flow test only constructs local typed object and calls hardcoded local functions. | -| `server/tests/computer-client.test.ts` | 2 | Real request/download/error/abort behavior; two signal-presence-only claims removed. Both claim request timeout behavior but only assert presence of a signal: first uses toBeDefined; second converts signals to 1/0 and expects [1,1]. An AbortController signal that never aborts passes both. Neither tests timeout expiration or the 45s versus 615s deadline distinction. Retain neighboring delayed-fetch test that actually verifies default timeout and override. | -| `server/tests/connector-tokens-never-reach-computer.test.ts` | 1 | Keep executed supervisor environmentFor secret filtering; import-pattern scan is a source architecture rule and does not test token delivery. | -| `server/tests/control-refusal-end-to-end.test.ts` | 1 | Keep real HTTP refusal propagation tests. Remove audit count-only test: gateway writes an allowed decision before executing; rows.length>0 still passes with failure/refusal audit removed. | -| `server/tests/cull-sweep-wiring.test.ts` | 2 | Both tests grep script source for literal option strings. Neither invokes the sweep or checks purge arguments or retention. | -| `server/tests/database.test.ts` | 1 | Only asserts database.query.users property exists; performs no database operation and never proves the claimed absence of a query. Shallow shape smoke without end-user behavior. | -| `server/tests/fixture-identity-distinct.test.ts` | 6 | Only enforces test-authoring/source-spelling conventions and tests the local scanner using synthetic source snippets; no executed end-user product behavior. These are lint/architecture guards, outside the requested user-use-case test criterion. | -| `server/tests/jsonb-encoding.integration.test.ts` | 1 | Retain actual schema and audit-store writes read through SQL JSON operators. Remove ambient whole-trail count test: no malformed historical fixture is seeded and no migration/repair is invoked, so an empty/clean database passes independently of the alleged repair behavior. | -| `server/tests/mastra-roster.test.ts` | 1 | Keep production roster selection and duplication cases. Token test only builds test-local managedAgent/ourEndpoints and compares strings, never invokes isManagedEndpoint or createRuntimeAgentLoader and never checks a token header. | -| `server/tests/migration-journal.test.ts` | 5 | Keep actual journal/snapshot artifact consistency checks; remove five self-tests of test-local SQL/journal scanners on synthetic fixtures. | -| `server/tests/pending-authorization-audit-ledger.test.ts` | 12 | Entire file inventories source syntax, hardcoded dispositions and waiver/debt metadata. Event proximity/name checks do not execute or verify audit firing; dead/unreachable event code satisfies the check. | -| `server/tests/people-paging.integration.test.ts` | 2 | Wildcard fixture has no alternate-character match: unescaped _ Percent still matches exactly one seeded person. Page-ceiling fixture seeds only three rows then asserts <=200, so removing the 200-row cap passes. | -| `server/tests/plugin-oauth.test.ts` | 1 | Claims S256 but asserts only differs from input, URL-safe characters and repeatability. A constant URL-safe string passes; no known SHA256 vector or independently computed expected digest. | -| `server/tests/plugin-store.integration.test.ts` | 1 | Reads plugin store sources, rejects literal error.message and requires sanitizer name somewhere. Renaming caught variable or inserting unused sanitizer reference bypasses claimed confidentiality protection. Behavioral sanitization tests in the same file are retained. | -| `server/tests/shared-use-arcade-urls.test.ts` | 1 | Neither seed nor test files an approval. Both server-row lookups return null, so equality passes even if approval canonicalization is broken. | -| `server/tests/sole-definition.test.ts` | 5 | Entire file counts source definitions/literal spellings against hardcoded known-debt entries and tests its scanner. It does not exercise end-user behavior; even recorded duplicate definitions are accepted. | -| `server/tests/store-scheme-sites.test.ts` | 1 | Reads shared-use-store.ts as text, requires BROKERED_PROFILE[provider].scheme and forbids a template spelling. Unused or wrong-branch reference passes without testing provider dispatch. | -| `server/tests/support/database.test.ts` | 4 | Tests testDatabaseUrlFrom test-only safety helper; no product use case or production code is exercised. | -| `server/tests/support/source-scan.test.ts` | 9 | All nine cases exercise only test-support source scanning and fixture enumeration/comment stripping. Real helper coverage, but no shipped end-user behavior. | -| `server/tests/support/source-scan.ts` | 0 | Unused test-only scanner after removing every consumer. | -| `server/tests/tenant-package.test.ts` | 1 | Calls validateTenantPackage and compares parsed skills arrays only. Never invokes loadTenantPackage, whose checksum code is separate; omitting skills from the checksum leaves this green. | -| `server/tests/test-network-guard.test.ts` | 8 | Exercises only the test-only network guard and its loopback/SDK allowances; the network guard implementation remains installed, but its own tests do not exercise shipped OpenBot user behavior. | -| `server/tests/ts-expect-error-coverage.test.ts` | 2 | Only checks test authoring/compiler inclusion convention and validates its own directive regex. No shipped application behavior. | -| `shared/harnesses.test.ts` | 0 | Preserved meaningful CrewAI deployment configuration assertions; narrowed the test title to the image, health endpoint and port it actually checks. | -| `tests/clean-checkout.test.ts` | 2 | Dependency whitelist never resolves imports; ignore-source text does not prove effective Git ignore rules | -| `tests/compose.test.ts` | 1 | Global Compose string occurrences do not verify migration dependency health ordering | -| `tests/workspace.test.ts` | 1 | Only greps shell source; never starts the worker | - -## Validation - -| Check | Result | -| --- | --- | -| Server, 20 sequential shards against a migrated disposable PostgreSQL database | 5,657 passed; 0 failed; 18 opt-in skipped | -| App tests, including colocated source tests | 1,668 passed; 0 failed | -| Five desktop TypeScript suites | 171 passed; 0 failed | -| Other Bun suites, 94 files in sequential batches of at most 20 | 857 passed; 0 failed; 61 opt-in skipped | -| Restored shared harness configuration suite, checked after final review | 9 passed; 0 failed; includes one restored case absent from the preceding batched run | -| Eleven edited Python model-spec suites, isolated pytest invocations | 33 passed; 0 failed | -| All 38 remaining Python test files | AST parsing passed | -| Forty edited TypeScript/TSX files | Biome lint and formatting passed | -| Git diff whitespace validation | Passed | -| Server test typecheck | Passed against the existing 240-error baseline; no new errors | - -Server command: `TEST_DATABASE_URL=... DATABASE_URL=... SHARD_SIZE=20 bun run test:server:sharded`. App command: `bun test ./app/tests ./app/src`. Test types: `bun run typecheck:tests`. - -Memory was monitored throughout. Resource-heavy test runs were serialized, Bun harness tests were batched, and the disposable database was limited to 512 MiB and one CPU. Reported system-wide free memory stayed around 35–43% during final verification. - -## Limits - -This was a static semantic audit plus selected mutation checks, not exhaustive mutation testing of every retained case. Live vendor/browser/deployment suites remain opt-in and were not exercised. The unchanged Python framework integration suites were reviewed but not executed; the edited model-spec suites were executed. Rust tests were reviewed but could not be compiled or run because Cargo is not installed. Local Bun is 1.4.2; the repository pins 1.4.3. The server test typecheck uses its existing baseline of 240 known errors, rather than asserting the entire test tree is type-error-free. diff --git a/tests/compose.test.ts b/tests/compose.test.ts index 7aa4d033a8..231ca57566 100644 --- a/tests/compose.test.ts +++ b/tests/compose.test.ts @@ -489,6 +489,14 @@ test("enables pgvector before creating vector columns", () => { expect(firstVectorColumn).toBeGreaterThan(extension); }); +test("runs the server migrator after PostgreSQL becomes healthy", () => { + const services = composeServices(); + expect(services.migrate.command).toEqual(["bun", "scripts/migrate.ts"]); + expect(services.migrate.depends_on?.postgres).toEqual({ + condition: "service_healthy", + }); +}); + test("builds the deployment image with Playwright's Chromium payload only", () => { for (const dockerfile of [rootDockerfile(), agentComputerDockerfile()]) { expect(dockerfile).toContain(