From 1ee240389729ed48f7fcb91fbf63698bd5c6d730 Mon Sep 17 00:00:00 2001 From: CC#1 Kora Substrate Date: Sat, 23 May 2026 15:21:42 -0700 Subject: [PATCH] =?UTF-8?q?chore(kora):=20KR-TEST-STABILITY-SWEEP=20?= =?UTF-8?q?=E2=80=94=20triage=20+=20fix=20pre-existing=20test=20failures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inventory captured at HEAD 8603404 via ``pytest tests/ --tb=line --continue-on-collection-errors``. Total residual failures: 309 across 86 files. Per the spec STOP-ASK condition (>20 genuine bugs), STOP-ASKing to PM on the bulk of the residual (~294 failures). This commit ships the SUBSET of fixes where the classification is unambiguously SAFE — test-only edits + zero risk of misclassifying a real production-code bug. # Fixes shipped (15 failures + 1 collection error → 0) ## 1. blake3 ImportError at collection (1 collection error → 0) ``tests/plugins/memory/test_scratchpad.py`` imports ``blake3`` at module top. The production module ``plugins/memory/isokron/scratchpad.py`` already imports it inside a try/except since blake3 is in the ``isokron`` extra (not in the default ``[dev] + [all]`` test invocation). Test-side mirrors that with ``pytest.importorskip("blake3")`` so the suite collects cleanly without the isokron extra. Without this fix the entire collection halts at this one file — masks everything downstream. ## 2. Anthropic adapter keychain isolation (14 failures → 0) ``tests/agent/test_anthropic_adapter.py``: ``TestResolveAnthropicToken``, ``TestRefreshOauthToken``, etc. indirectly call ``read_claude_code_credentials()`` which on macOS reads from the system keychain via ``_read_claude_code_credentials_from_keychain``. Individual tests monkeypatched the env vars + filesystem credential paths but NOT the keychain reader — developer's real OAuth token bled in. ``TestReadClaudeCodeCredentials`` already had a per-class autouse fixture stubbing the keychain reader. Promoted to MODULE scope so every Test* class in the file gets the same isolation by default — same shape, broader reach. Verified: 152/152 pass (was 138/152). ## 3. DingTalk display_name identity rebrand (1 → 0) ``tests/gateway/test_dingtalk.py::TestSend::test_send_posts_to_webhook`` asserted ``payload["markdown"]["title"] == "Hermes"``. Production ``PlatformConfig.display_name`` defaults to "Kora" at ``gateway/config.py:304`` since the Hermes→Kora fork. Updated the literal + added an inline comment pointing at the production default so future readers can trace the invariant. # Triage of residual ~294 failures (DEFERRED — see PR body) Categorization summary (full per-file breakdown in PR body): * **Real production drift** (~50 tests across acp / gateway): APIs changed shape (e.g., test_edit_approval asserts return type that's now ``None``; test_identity_strings expects slash commands that were removed). NOT safe to silently fix; each one is a real semantic question for the relevant feature bucket owner. * **Test-suite-wide state pollution** (~50 tests in test_hermes_* + test_subprocess_home_isolation + test_kanban_db): these files PASS individually but FAIL when run with the broader suite. CC#3's #152 fixed the xdist-specific case; these are serial-mode pollution from shared singletons (HERMES_HOME, kora_constants, profile state). Worth a dedicated bucket — too rabbit-hole for safe blanket fix. * **MagicMock setup gaps** (~70 tests across gateway/discord*, tools/test_skill_*): MagicMock attributes return Mock objects when the test expects scalar values. Tests need deeper rewiring; not a stale-assertion 1-line fix. * **Stale identity / env / version literals** (~30 tests): similar shape to the dingtalk fix above but each needs individual verification (is the new literal the right one OR is the production change the bug?). Bulk regex-replace would risk classifying real drift as cosmetic. * **Missing test deps / environment** (~20 tests): tests requiring docker/wsl/network/credentials not present in the default test env. Belongs in a separate test-infra-hardening bucket. * **Genuinely flaky / asyncio coroutine warnings** (~20 tests): tests using AsyncMock incorrectly or with cross-test event- loop state. Hard to fix without per-test diagnosis. * **Other / unclassified** (~50 tests): need individual inspection. Per spec STOP-ASK condition (>20 genuine bugs), surfacing this scope back to PM for follow-on bucket dispatch. # Approach rationale Spec §1 (Phase B) lists SAFE categories explicitly: stale assertions, stale env defaults, missing fixture isolation, dep drift, missing test deps. This commit ships exactly those three where the SAFETY is unambiguous + reversible: * blake3 → optional-dep importorskip (test-only) * keychain bleed → autouse fixture (test-only) * Hermes→Kora literal → matches verified-current production default (gateway/config.py:304) Spec HARD NON-SCOPE is "no production code changes." All three fixes touch ONLY tests/ files. Zero risk to production behavior. Spec STOP-ASK criteria #1 ("triage reveals >20 genuine bugs") applies to the residual. Surfacing for PM dispatch rather than risking false-positive "stale assertion" rewrites of real bugs. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/agent/test_anthropic_adapter.py | 28 +++++++++++++++++++++++++ tests/gateway/test_dingtalk.py | 4 +++- tests/plugins/memory/test_scratchpad.py | 10 ++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/tests/agent/test_anthropic_adapter.py b/tests/agent/test_anthropic_adapter.py index 10f82ca95e08..df4d10aa95f0 100644 --- a/tests/agent/test_anthropic_adapter.py +++ b/tests/agent/test_anthropic_adapter.py @@ -28,6 +28,34 @@ from agent.transports import get_transport +# --------------------------------------------------------------------------- +# Module-level isolation — KR-TEST-STABILITY-SWEEP +# --------------------------------------------------------------------------- +# +# ``resolve_anthropic_token`` + ``read_claude_code_credentials`` read +# from the OS keychain on macOS (via +# ``_read_claude_code_credentials_from_keychain``) in addition to the +# env vars + filesystem credential files individual tests +# monkeypatch. Without an autouse keychain-stub a developer's real +# OAuth token bleeds into TestResolveAnthropicToken assertions +# (observed during pre-existing-failure sweep: keychain returned a +# live ``sk-ant-oat01-...`` token regardless of the test's env setup). +# +# ``TestReadClaudeCodeCredentials`` already had its own per-class +# autouse fixture stubbing the keychain reader. Promoted to module +# scope so every Test* class in the file gets the same isolation by +# default — same shape, broader reach. The per-class fixture in +# TestReadClaudeCodeCredentials is now redundant but kept verbatim +# for self-documentation; both fire on those tests with no side +# effects (both patch to the same lambda value). +@pytest.fixture(autouse=True) +def _module_no_keychain(monkeypatch): + monkeypatch.setattr( + "agent.anthropic_adapter._read_claude_code_credentials_from_keychain", + lambda: None, + ) + + # --------------------------------------------------------------------------- # Auth helpers # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_dingtalk.py b/tests/gateway/test_dingtalk.py index 6b2db13299dd..91aeee909ac0 100644 --- a/tests/gateway/test_dingtalk.py +++ b/tests/gateway/test_dingtalk.py @@ -250,7 +250,9 @@ async def test_send_posts_to_webhook(self): assert call_args[0][0] == "https://dingtalk.example/webhook" payload = call_args[1]["json"] assert payload["msgtype"] == "markdown" - assert payload["markdown"]["title"] == "Hermes" + # Identity rebrand: PlatformConfig.display_name defaults to + # "Kora" since the Hermes→Kora fork (see gateway/config.py:304). + assert payload["markdown"]["title"] == "Kora" assert payload["markdown"]["text"] == "Hello!" @pytest.mark.asyncio diff --git a/tests/plugins/memory/test_scratchpad.py b/tests/plugins/memory/test_scratchpad.py index c2aac6a9c823..ae42c8372788 100644 --- a/tests/plugins/memory/test_scratchpad.py +++ b/tests/plugins/memory/test_scratchpad.py @@ -21,9 +21,17 @@ import logging from typing import Any, List, Optional -import blake3 import pytest +# blake3 is an optional plugin dep (declared in the ``isokron`` extra +# in pyproject.toml + the plugin.yaml). The production module +# ``plugins.memory.isokron.scratchpad`` already imports it inside a +# try/except — test side mirrors that with importorskip so the suite +# collects cleanly in environments where the isokron extra isn't +# installed (e.g. the default ``--extra dev --extra all`` test +# invocation). Collection-time skip > collection-time ImportError. +blake3 = pytest.importorskip("blake3") + from plugins.memory.isokron.scratchpad import ( DEFAULT_SCRATCHPAD_READ_LIMIT, ScratchpadEntry,