Repository navigation
test: fix dev test-suite drift + harness bugs (no product changes) - #1520
Merged
Merged
Conversation
A full-suite run on `dev` surfaced 38 failures; all root-caused to stale tests or test-harness issues — zero product regressions found. - platform_prompt_unit: 5 stub lambdas now accept the `runtime=` kwarg the production compose path forwards (#1187 runtime-aware platform prompt). - cb_probe_execution_close: add `get_setting_value` to `_CanaryTempDB`. The canary temp-DB stub leaks onto the module-level `from database import db` reference, so under full-suite ordering it services task_execution_service.get_platform_default_model and AttributeError'd. - fork_to_own: defensive `list_all_agents_fast` backfill for a leaked bare `services.docker_service` stub (same sys.modules cross-file leak class; both pass in isolation, only error under full-suite order). - files_guardrail_bypass / mcp_validator_endpoint: assert the current "Disallowed credential file path" wording (#1305 curated credential file-type injection). - event_subscriptions: nonexistent source now expects 403, not 400 — intentional #186 enumeration-safety collapse (commit 34ddf71). - activities: derive the valid `triggered_by` set from the canonical db/schedules._TRIGGER_BUCKETS map so it stops drifting each time a trigger/channel value is added (webhook, telegram/slack/whatsapp, ...). - whatsapp_integration: read the pending-login Redis key via the backend's authenticated client instead of a bare `redis-cli GET`. Redis ACL auth is mandatory since #589; `redis-cli` exits 0 on NOAUTH, so the helper's returncode guard never tripped and the value never matched. Product code is correct (verified by writing/reading the key via the real adapter). Not in this diff (local environment, not code): - tests/.venv had drifted (bcrypt 5.0.0, no Pillow), causing 9 unit failures (bcrypt 72-byte error, ModuleNotFoundError: PIL). requirements-test.txt already pins `bcrypt>=4.2.0,<5` and `Pillow>=11.1.0`, so a clean CI venv is unaffected — resync the local venv with `pip install -r requirements-test.txt`. Verified: 74 unit + 162 integration = 236 passed on the touched files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Review: ✅ Approve — safe to merge Test-files-only, zero production lines (verified: diff touches only
CI green across all 3 seeds × base/head. One note (already called out in the PR): clusters 1–3 are band-aided per-file against |
dolho
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A full-suite run on
dev(Docker, local) surfaced 38 failures + 2 errors. Every one was root-caused to a stale test or a test-harness issue — zero product regressions. This PR is test-files-only (0 lines of production code).Verified after fixes: 74 unit + 162 integration = 236 passed across the touched files.
Fixes (what drifted, why)
test_platform_prompt_unit(5)**_kwargsruntime=(#1187)test_cb_probe_execution_close(7)get_setting_valueto_CanaryTempDBfrom database import db; under full-suite order it servicedget_platform_default_modelandAttributeError'dtest_fork_to_own(2 errors)list_all_agents_fastbackfillservices.docker_servicestub (samesys.modulescross-file leak class)test_files_guardrail_bypass,test_mcp_validator_endpoint(2)"Disallowed credential file path"test_event_subscriptions(1)34ddf71a)test_activities(1)_TRIGGER_BUCKETSwebhook/ channel triggers; now tied to source of truthtest_whatsapp_integration(4)redis-cli GETfailsNOAUTH(Redis ACL mandatory since #589) but exits 0, defeating thereturncodeguard. Product code verified correct.Clusters 1–3 pass in isolation and only failed under full-suite ordering (
sys.modulesleakage). A durable follow-up would harden thetests/unit/conftest.pysys.modules baseline restore — out of scope here.Running the full suite locally still shows a handful of failures/skips that are not fixed by this PR because they are environmental — a clean CI run is unaffected:
bcrypt72-byte error ×3,ModuleNotFoundError: PIL×6) — a drifted localtests/.venv(bcrypt 5.0.0, no Pillow).requirements-test.txtalready pinsbcrypt>=4.2.0,<5andPillow>=11.1.0. Resync your local venv:cd tests && .venv/bin/pip install -r requirements-test.txt.TEST_AGENT_NAMEunset, notestfixfixture). Needs a persistent test agent.test_dynamic_thinking_statusasync ×3, one 429) — the real Claude subscription hit its usage cap during the 50-min run.workspace_available/auto_switchdefaults,agent_renamename collision) — pass in a clean CI env / fresh DB; code defaults are correct.Test plan
pyteston all touched files (leak-order preserved for canary→cb_probe): 236 passedsrc/backend/production code🤖 Generated with Claude Code