test(#586) + test-suite recovery + doc backfill + CSO 2026-05-17 - #875
Merged
Conversation
test_voice_tools.py and test_fleet_status_resilience.py install incomplete stubs of services.agent_client into sys.modules at module-collection scope. The autouse restore in tests/conftest.py only restored keys whose baseline was non-None, and only ran for the non-unit tier (the unit tier sets norecursedirs = .. in tests/unit/pytest.ini and bypasses the parent conftest entirely). The polluted stubs missed CircuitState, so transitive importers (adapters → task_execution_service:32 → from services.agent_client import CircuitState) raised ImportError, taking down 19 tests in test_file_upload.py and 1 in test_session_persistence_flag.py. Two changes: 1. tests/conftest.py: add services.agent_client to _SYS_MODULES_INVARIANT_KEYS and pre-import it before the baseline is captured, so the baseline is a real module object that gets restored between tests (covers the non-unit tier). 2. tests/unit/conftest.py: mirror the baseline+autouse-restore mechanism for services and services.agent_client. The unit tier has its own rootdir; without this, the parent conftest's defenses never run for the unit suite. Unit tier: 20 failed → 4 failed (the 4 remaining are unrelated SQL schema and KeyError failures in test_extract_photo_largest_size and test_voice_auth, out of scope for this task). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rror Follow-up to #762 — code-review feedback. except Exception: pass would mask non-import errors (e.g. side-effect runtime exceptions at module load) and silently degrade the autouse-restore defense. ImportError is the only expected failure mode for the preload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… probe tests Seven tests in TestCircuitBreakerFastFail and TestCancelledErrorInExecuteTask failed with "object MagicMock can't be used in 'await' expression" — but only when test_validation.py ran first in the same pytest session. The TypeError fires at `await svc.execute_task(...)`, not on a single attribute mock. Root cause: test_validation.py does `sys.modules["services.task_execution_service"] = MagicMock()` at module-collection time. The conftest baseline-restore (#762) cannot undo it because the baseline value is `None` — the real task_execution_service module isn't loadable from conftest preload (it imports `database`, which mkdirs `/data` and fails outside Docker). The autouse restore explicitly preserves None-baseline entries to avoid clobbering deliberate stubs. When test_cb_probe later does `from services.task_execution_service import TaskExecutionService` it gets `MagicMock.TaskExecutionService`, instantiating returns a MagicMock, `svc.execute_task(...)` returns a MagicMock, and `await MagicMock` raises TypeError. Fix: in each affected class's autouse `_patch_env` fixture, pop `services.task_execution_service` from sys.modules before the test body's import — forcing a fresh load of the real module. Also replace the module-level `setdefault` of `utils.credential_sanitizer` with an unconditional install (test_validation.py installs a partial stub that lacks `sanitize_execution_log`, so setdefault was a no-op and the fresh task_execution_service import failed at `from utils.credential_sanitizer import sanitize_execution_log`). Verified: full file passes (10/10) standalone and after test_validation.py pollution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…port
Outcome (a) from the Task 5 plan: when an admin calls
POST /api/agents/{name}/credentials/import on an agent whose container
status is "running" but whose internal FastAPI server hasn't bound to
port 8000 yet, `import_to_agent()` raises `httpx.ConnectError`
("All connection attempts failed"), which is not a `ValueError`. The
existing handler had only `except ValueError → 400` and a bare
`except Exception → 500`, so the transient race surfaced as a 500.
`test_import_credentials_no_enc_file_fails` accepts 400 (file missing)
or 503 (agent not ready) but never 500 — so the regression failed CI.
Fix mirrors the pattern already used by `inject_credentials` (same file,
line 250) and `routers/agent_files.py:82`: catch `httpx.RequestError`
(parent of ConnectError / TimeoutException / ReadError) and map to 503
with a warning log. Applied symmetrically to `export_credentials` since
it has the same shape and would 500 on the same transient condition.
`CredentialsFileNotFoundError(ValueError)` is unaffected — when the
agent server *is* reachable but no `.credentials.enc` exists,
`import_to_agent()` still raises that ValueError subclass and the
existing 400 mapping still fires.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rom .env
Closes the env-friction set surfaced by the May 2026 test-recovery audit:
- tests/setup-env.sh sourced by run-*.sh exports the four vars
pytest needs from project .env.
- tests/conftest.py picks them up the same way for direct pytest
invocation (alias ADMIN_PASSWORD -> TRINITY_TEST_PASSWORD).
- tests/requirements-test.txt installs src/cli editable so
test_cli_admin_login.py and test_cli_profiles.py can collect.
- tests/README.md documents the env matrix, tiers, the rate-limit +
setup-completed recovery commands, and the Conductor-worktree
backend-mount caveat.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The setdefault chain in tests/conftest.py was order-dependent: line 30 unconditionally setdefault'd "test" as a placeholder for backend-config import safety, which made the subsequent setdefault from .env a no-op. tests/security/ ACL tests then ran with the wrong password and failed with NOAUTH unless the caller manually exported REDIS_BACKEND_PASSWORD before pytest. Switch to explicit override: when the .env value is present AND the current env-var is either unset or the "test" placeholder, replace it. An explicit caller export (any value other than "test") still wins. Follow-up to dbdb808. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two changes:
1. lint_sys_modules.py:iter_test_files — skip directories that contain
third-party / generated code (.venv, venv, __pycache__,
.pytest_cache, node_modules). The previous rglob walked into
tests/.venv/lib/python3.11/site-packages and reported 30+ pseudo-
violations in dependency code that vary per machine / dep version.
The baseline already excluded these (zero .venv entries) so the
committed baseline was machine-relative without anyone noticing
until a dep upgrade exposed the drift.
2. tests/lint_sys_modules_baseline.txt — regenerate. Two real changes:
- test_cb_probe_execution_close.py: 5 → 7 (+2 sys.modules.pop calls
added in commit f586532 to evict cross-file stub pollution).
- tests/unit/test_cleanup_unreachable_orphan.py removed (cleaned up
during the dev rebase).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary of the 7 fixes landed in this branch (CircuitState sys.modules, MagicMock-await eviction, credentials 503 mapping, env friction docs + helpers, lint baseline regen + linter .venv exclusion) plus an inventory of out-of-scope failures and Conductor-worktree caveats that need follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds test_setsid_escapee_drained_via_orphan_killer_preserves_result_line to pin the full production path: parent emits result, forks a setsid()'d grandchild that holds stdout open, parent exits. Asserts that drain_reader_threads invokes _kill_orphan_pipe_writers and the buffered "RESULT_LINE" survives — i.e. the drain doesn't fall through to the force-close fallback that discards the kernel pipe buffer. Sibling of the #531 buffered-data test, distinct because the grandchild calls os.setsid() — the case that escapes terminate_process_group(pgid) and is the real-world signature in production (git push → ssh). Also adds a KNOWN_ISSUES.md entry describing the bug class, the platform fix (#620), and operator-side defense-in-depth for stop hooks that spawn network processes. Refs #586 Co-Authored-By: Claude <noreply@anthropic.com>
- agent-lifecycle.md / container-capabilities.md: document Phase 3c capability drop (#602 / PR #830, 2026-05-13) — SYS_PTRACE, MKNOD, NET_RAW, FSETID removed from FULL_CAPABILITIES (now 9 caps, was 13); constants extracted to capabilities.py so test_capability_set.py can import them stdlib-only. - credential-injection.md: document 503 mapping for import/export endpoints (commit 35d4e78, 2026-05-17) — httpx.RequestError now surfaces 503 instead of 500, mirroring inject and agent-files. - session-tab.md: update lock semantics for #779 — cold turns now serialised on session_lock:cold:{session_id} (previously short- circuited, letting two concurrent first POSTs race on update_cached_claude_session_id and orphan one JSONL inside the agent). - feature-flows.md: index entries for the above. Co-Authored-By: Claude <noreply@anthropic.com>
- cso-2026-05-17.{md,json}: daily full-phase audit (Phases 0-14).
Verdict CLEAR at 8/10 confidence gate; one persistent MEDIUM
(Dockerfile USER hardening — tracked since 2026-04-05). No new
CRITICAL or HIGH findings.
- cso-diff-2026-05-13.md: diff-mode report covering the working-tree
changes for the #586 regression test and KNOWN_ISSUES.md entry. No
vulns or secrets — docs + test-only, no production code path.
Co-Authored-By: Claude <noreply@anthropic.com>
Picks up the methodology toolkit head that adds DEVELOPMENT_WORKFLOW.md. Co-Authored-By: Claude <noreply@anthropic.com>
…e-586-plan # Conflicts: # docs/memory/feature-flows.md # docs/memory/feature-flows/session-tab.md
pip resolves relative paths in requirements files against the current working directory, not the requirements file. `-e ../src/cli` (added in dbdb808) only worked when pip was invoked from `tests/`; CI runs from the repo root and got `../src/cli` -> outside workspace -> 6 pytest jobs + schema-parity + regression-diff all red on PR #875. Switch to `-e ./src/cli` and update tests/README.md to instruct invoking from repo root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… reload test_config_accepts_url_with_credentials calls _reload_config() which pops sys.modules["config"] and re-imports. The reloaded module reads SECRET_KEY from os.environ at reload time, picking up "test-secret-key-for-unit-tests" from test_voice_auth.py's os.environ.setdefault — but the *original* config (loaded at conftest pre-import time, before that env was set) had a random SECRET_KEY from secrets.token_hex(32). The test then leaves the new module in sys.modules permanently. Downstream, routers/voice.py's runtime `from config import SECRET_KEY, ALGORITHM` (called from voice_websocket) reads the new SECRET_KEY, while test_voice_auth.py's JWTs were signed with the original key captured at its module-collection time. JWT decode raises JWTError, voice_websocket closes 4001 instead of the expected 4003/accept, and the 3 ownership-gate tests fail — but only under pytest-randomly seeds that schedule test_config_fail_fast before test_voice_auth (notably seed 12345 after PR #875 added test_subprocess_pgroup.py and shifted the random ordering). Snapshot/restore sys.modules["config"] via an autouse fixture so each test's reload is scoped to itself. The 3 voice_auth tests (test_owner_passes_auth_gate, test_other_user_rejected_4003, test_admin_bypasses_ownership) now pass on seed 12345. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_fail_fast Renames _restore_config_module to the lint-recognized _STUBBED_MODULE_NAMES + _restore_sys_modules pair so the helper-exception in tests/lint_sys_modules.py fires (precedent: tests/unit/test_telegram_webhook_backfill.py). The previous attempt (0966243) used a bespoke fixture name + bare sys.modules.pop, which is exactly what the #762 hygiene linter bans outside conftest.py — the lint job went red on push. Same behavior, correct pattern, exempted by the linter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lot_per_slot_ttl.py #871 landed on dev (commit 98574f3) with test_slot_per_slot_ttl.py containing 6 bare sys.modules.{pop,setdefault,assign} calls but no matching baseline entry. Dev's own post-merge lint job is now red on that commit (Issue #802 caught it as intended). Any PR that merges with current dev inherits the failure. This PR is unrelated to the slot file but blocked by the inherited red. Bump the baseline to match what dev actually ships, unblocking CI here. The proper fix — refactor test_slot_per_slot_ttl.py to use the _STUBBED_MODULE_NAMES + _restore_sys_modules helper pattern, or scope its sys.modules mutations via monkeypatch — should land as a follow-up on #871's author or whoever owns the slot file (file is otherwise untouched here). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n test_slot_per_slot_ttl.py" This reverts commit 459b535.
Contributor
PR Validation ReportPR: #875 — Summary
CI Status
Issues FoundCritical (Block Merge)
Warnings (Review Required)
Suggestions
RecommendationREQUEST CHANGES — solely on the red lint check. Substance is strong: real regression test that pins #620's fix for a P1 production bug, disciplined test-suite recovery (7 fixes documented in The blocker is environmental — the sys.modules linter is red because of Two paths forward:
🤖 Generated with Claude Code |
vybe
requested changes
May 20, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Substance is approved — real #586 regression test, narrow credentials 503 fix mirroring existing patterns, test-recovery commits each with clear root-cause analysis, doc backfill aligned with the touched code, clean CSO audit, 6-seed pytest matrix + regression-diff + schema-parity all green.
Blocker before merge:
- Rebase on current
devand resolve the baseline conflict. PR #884 just merged and regeneratedtests/lint_sys_modules_baseline.txt(addstest_slot_per_slot_ttl.py: 6, removestest_cleanup_unreachable_orphan.py: 3). This branch's45419160reverted its own equivalent baseline bump in anticipation of #884 landing, so HEAD now diverges. After rebase, acceptdev's version of the baseline file and the lint job should go green on the next CI run.
Once CI is green I'll re-approve immediately.
4 tasks
…e-586-plan Resolves PR #875 reviewer request: rebase on dev to pick up PR #884's baseline regeneration (adds test_slot_per_slot_ttl.py: 6 via #881's _STUBBED_MODULE_NAMES refactor; removes test_cleanup_unreachable_orphan.py: 3). After merge, sys.modules lint is green: - test_slot_per_slot_ttl.py: now exempt via sanctioned _STUBBED_MODULE_NAMES + _restore_sys_modules pattern (PR #881 on dev) - test_cb_probe_execution_close.py: baseline 7 matches actual (this PR's stub eviction) - test_config_fail_fast.py: actual 0, baseline 1 (reduced — sanctioned) Conflict in docs/memory/feature-flows.md resolved by keeping both this PR's #35d4e78 row and dev's #887/#888 rows.
Contributor
Author
|
Rebased — merged latest Conflict resolution:
Local lint result:
|
…e-586-plan Two new commits on dev since the previous merge (c9008e4): - b892069: feat(observability): emit [METRIC] drain_outcome on slow-path orphan-killer (#586) (#837) - c6bc456: feat(soft-delete): agent_ownership soft-delete + retention purge (#834 Phase 1a) (#838) Conflicts resolved: - tests/unit/test_subprocess_pgroup.py: kept BOTH this PR's test_setsid_escapee_drained_via_orphan_killer_preserves_result_line (#586 regression) AND dev's #837 metric tests (test_emits_metric_on_natural_drain, test_emits_metric_on_force_close_path). Tests cover orthogonal aspects of the slow-path drain: setsid escape + result-line preservation vs. drain_outcome metric emission. - docs/memory/feature-flows.md: kept both this PR's #35d4e78 row and dev's #586/#837 row. Lint still green; sys.modules baseline already current after the previous dev-merge (c9008e4).
Contributor
Author
|
CI fully green on
Second merge resolved two conflicts:
Ready for re-review. |
…e-586-plan # Conflicts: # tests/test_cb_probe_execution_close.py
Contributor
|
Merged current The other 18 touched files merged cleanly. CI re-running. |
The branch's submodule bump (3873d2c) was a parent of dev's current pointer (9650477), so merging would silently revert the "fix(announce): prevent duplicate Slack sends by switching to Python urllib" change in .claude. Resetting the submodule pointer to dev's current state effectively drops the bump from this PR. The DEVELOPMENT_WORKFLOW.md doc added by 3873d2c is still present (9650477 includes it as an ancestor). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Pushed one fix-up commit on your behalf — branch is now What I changed:
Validation pass summary (all green except the submodule, which I just fixed):
Will re-approve and squash-merge once CI re-runs on |
vybe
approved these changes
May 23, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Re-approved after submodule fix. CI green (lint, 6 pytest seeds, schema-parity, regression-diff, CodeQL). Pushed 86d31b4e myself to reset the .claude submodule pointer to dev's current — your bump was to the parent commit which would have silently reverted the announce fix.
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 27, 2026
The first CI run this PR was ever able to produce (it was CONFLICTING until now, so GitHub could not build refs/pull/1827/merge and reported zero checks) failed `lint (sys.modules pollution check)`: 12 bare `sys.modules.pop(...)` calls, 3 per new file. Fixed by satisfying the guard, NOT by widening its baseline (`lint_sys_modules_baseline.txt` is untouched) — this guard has already caught this same class in #783, #606 and #875, and retiring it on a test-rigour PR would be backwards. Both eviction sites are out of monkeypatch's reach, or actively wrong for it: * the `utils*` shadow clear is IMPORT-time, before any fixture exists, so monkeypatch structurally cannot reach it; * the `ops` fixture must evict `db.*` so `from db.schedules import ...` re-imports against the harness-bound engine. `monkeypatch.delitem` records NO undo for a key absent on entry (verified against _pytest.monkeypatch), so the freshly imported harness-bound module would stay resident for later files — strictly worse isolation than the explicit pop it would replace. So both take the documented escape hatch instead: a top-level `_STUBBED_MODULE_NAMES` list plus an autouse `_restore_sys_modules` fixture, matching the shape of tests/unit/test_telegram_webhook_backfill.py. It is a teardown-time guarantee only — the snapshot is taken before the test body, so no in-test behaviour and no assertion changes. Verified: lint reports "203 violation(s) in 60 file(s); baseline allows 240 — no new violations"; the four files still report exactly 129 passed, 1 xfailed (same strict xfail); `git diff origin/dev...HEAD -- src/` still empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks
AndriiPasternak31
added a commit
that referenced
this pull request
Jul 28, 2026
The first CI run this PR was ever able to produce (it was CONFLICTING until now, so GitHub could not build refs/pull/1827/merge and reported zero checks) failed `lint (sys.modules pollution check)`: 12 bare `sys.modules.pop(...)` calls, 3 per new file. Fixed by satisfying the guard, NOT by widening its baseline (`lint_sys_modules_baseline.txt` is untouched) — this guard has already caught this same class in #783, #606 and #875, and retiring it on a test-rigour PR would be backwards. Both eviction sites are out of monkeypatch's reach, or actively wrong for it: * the `utils*` shadow clear is IMPORT-time, before any fixture exists, so monkeypatch structurally cannot reach it; * the `ops` fixture must evict `db.*` so `from db.schedules import ...` re-imports against the harness-bound engine. `monkeypatch.delitem` records NO undo for a key absent on entry (verified against _pytest.monkeypatch), so the freshly imported harness-bound module would stay resident for later files — strictly worse isolation than the explicit pop it would replace. So both take the documented escape hatch instead: a top-level `_STUBBED_MODULE_NAMES` list plus an autouse `_restore_sys_modules` fixture, matching the shape of tests/unit/test_telegram_webhook_backfill.py. It is a teardown-time guarantee only — the snapshot is taken before the test body, so no in-test behaviour and no assertion changes. Verified: lint reports "203 violation(s) in 60 file(s); baseline allows 240 — no new violations"; the four files still report exactly 129 passed, 1 xfailed (same strict xfail); `git diff origin/dev...HEAD -- src/` still empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
pushed a commit
that referenced
this pull request
Jul 29, 2026
…ers and analytics aggregation (#1827) * test(db/schedules): edge cases + Hypothesis properties for CAS status writers (#1771) /edge-cases target 3, sub-area A: the #1082 status-as-projection CAS writers in db/schedules/executions.py and db/schedules/queue.py. - test_1771c_schedules_cas_edges.py: matrix rows A1b/A2/A3/A5/A5b/A6/A7/A8/ A12/A13/A14/A15/A16 as parametrized cases on the db_harness (#300) full production schema — SQLite always, PostgreSQL when TEST_POSTGRES_URL is set. - test_1771c_schedules_cas_properties.py: P-A1 bounded RuleBasedStateMachine ("terminal is absorbing" — canary E-02 at the DB layer) plus a meta-test that injects a phantom reversal and proves the machine reports it; P-A2/P-A3 no-crash + idempotence asserted PER RETURN CLASS (bool / int / Optional[Dict] are three different contracts); P-A4 lexicographic-ISO oracle. - hypothesis==6.161.5 added to tests/requirements-test.txt (exact pin so the three concurrent #1771 slices produce an identical, trivially-mergeable line). One strict xfail: A6 — a clock-skewed started_at persists a NEGATIVE duration_ms. No product-code fix (#1771 AC#4). Test-only. No src/ changes. Refs #1771 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(db/schedules): edge cases + Hypothesis properties for analytics aggregation (#1771) /edge-cases target 3, sub-area B: db/schedules/analytics.py — get_agent_analytics (#1107), get_schedule_analytics (#868), get_agent_schedules_summary (#1115) and the pure leaves _bucket_for_trigger / _schedule_command_label. - test_1771c_schedules_analytics_edges.py: matrix rows B1–B12. Highlights: B1 pins _TRIGGER_BUCKETS.values() subset of _BUCKET_ORDER (the existing literal assertion in test_agent_analytics.py omits "Reminders", so it was never a completeness check); B6b mechanises the locked "headline avg is full-set, never the capped pool" discipline; B10 freezes iso_cutoff so the strict-'>' boundary is deterministic rather than a clock race. - test_1771c_schedules_analytics_properties.py: P-B1 conservation (sum of by_type == total_executions, over arbitrary unicode triggers), P-B2 rate/count bounds incl. the zero-terminal-day-is-None rule, P-B3 contiguous UTC-day timeline over arbitrary windows, P-B4 no-crash-total on the pure leaves. Hypothesis `event()` markers make non-vacuity provable via --hypothesis-show-statistics rather than assumed. Five first-run failures were all WRONG TESTS (my expectations/mechanisms), not product bugs — reflection notes recorded in the docstrings. Test-only. No src/ changes. Refs #1771 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(registry): register the four #1771 edge-case/property test files /update-tests tail step. Appended with ensure_ascii=True so the existing \uXXXX escaping of 24 unrelated entries is preserved — the diff is +54/-0. The companion .claude/agents/test-runner.md catalog sync is deliberately NOT done: that path is inside the pinned private .claude submodule, and this wave must not move the submodule gitlink. Refs #1771 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(db/schedules): make the B9 day-bucketing case date-relative (#1771) Self-review fix. B9 hard-coded a 2026-03-01/02 pair, which forced a 10-year window to keep the rows in range — so the gap-fill loop built ~3650 day-dicts per call, and the test would have aged out of any tighter window. Derive the straddling instant from today instead and use a 168h window. Same assertion, cannot age out; the file's runtime drops 8.3s -> 2.4s. Refs #1771 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(db/schedules): close the get_schedule_analytics gap and de-vacuum P-B2b (#1771) /review findings on the #1771 target-3 edge-case slice, fixed in place. Test-only: `git diff f7aff46..HEAD -- src/` stays empty (AC#4). R1 REQUIREMENTS MISSING — `get_schedule_analytics` was named a subject-under-test by the plan (§3) and by the edges file's own docstring, yet no test invoked it: under a new-tests-only coverage run its whole body (analytics.py:113-259) was unexecuted, and the reported 92% came from bundling nine pre-existing neighbour files. Added rows B13/B13b/B13c/B15. R2 Closed the residual gap the matrix itself flagged: the `tool_calls` JSON-SHAPE guards. The column is agent-written, so valid-JSON-not-a-list, list-of-scalars, dict-without-`name` and non-numeric `duration_ms` are all reachable input, and a regressed guard turns an analytics read into a 500. Neighbours cover only the `json.loads` raise. Now pinned on BOTH surfaces (`get_schedule_analytics` + `get_agent_schedules_summary`, 20 cases). R3 P-B2b was half-vacuous: it asserted `sampled is (eligible > cap)` with the cap at its production 5000 and at most 12 seeded rows, so the `True` arm was unreachable for every example. The cap is now drawn (1-6) and monkeypatched, with over/at/empty `@example` pins. Verified load-bearing — a probe forcing `sampled is False` fails on the explicit example. R7 Renamed the `st` loop variable that shadowed the conventional `hypothesis.strategies as st` alias used in the sibling properties file. Also corrected in `.plan/edge-cases-1771c-matrix.md` (untracked by convention): `analytics.py:234`/`241` were mis-described as tool_calls guards (234 is the empty-day guard, 241 the FAILED timeline arm, now covered); `542->550` is defensive-dead, not malformed-`started_at`-reachable (`'' > cutoff` is false in SQL, so an empty `started_at` never reaches the loop); both coverage numbers are now reported instead of only the bundled one; and the Hypothesis `event()` percentages are labelled as one observed run rather than a stable contract. Declined: ruff F811 on the `db_backend` fixture import (pre-existing repo-wide pattern in every db_harness consumer; no CI workflow runs ruff). Refs #1771 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(deps): harmonize the #1771 hypothesis block across the three slices The three concurrent #1771 slices each added `hypothesis==6.161.5` to tests/requirements-test.txt at a different position with a different comment, so they conflicted with each other rather than merging cleanly. Rebuild the file from dev's copy and insert one byte-identical canonical block at one fixed position (after pytest-cov). All three slices now carry the same bytes in the same place, so they merge in any order and the file keeps exactly ONE hypothesis line. No product code, no test logic change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(#1771): restore sys.modules the four 1771c files evict (#762 lint) The first CI run this PR was ever able to produce (it was CONFLICTING until now, so GitHub could not build refs/pull/1827/merge and reported zero checks) failed `lint (sys.modules pollution check)`: 12 bare `sys.modules.pop(...)` calls, 3 per new file. Fixed by satisfying the guard, NOT by widening its baseline (`lint_sys_modules_baseline.txt` is untouched) — this guard has already caught this same class in #783, #606 and #875, and retiring it on a test-rigour PR would be backwards. Both eviction sites are out of monkeypatch's reach, or actively wrong for it: * the `utils*` shadow clear is IMPORT-time, before any fixture exists, so monkeypatch structurally cannot reach it; * the `ops` fixture must evict `db.*` so `from db.schedules import ...` re-imports against the harness-bound engine. `monkeypatch.delitem` records NO undo for a key absent on entry (verified against _pytest.monkeypatch), so the freshly imported harness-bound module would stay resident for later files — strictly worse isolation than the explicit pop it would replace. So both take the documented escape hatch instead: a top-level `_STUBBED_MODULE_NAMES` list plus an autouse `_restore_sys_modules` fixture, matching the shape of tests/unit/test_telegram_webhook_backfill.py. It is a teardown-time guarantee only — the snapshot is taken before the test body, so no in-test behaviour and no assertion changes. Verified: lint reports "203 violation(s) in 60 file(s); baseline allows 240 — no new violations"; the four files still report exactly 129 passed, 1 xfailed (same strict xfail); `git diff origin/dev...HEAD -- src/` still empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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
test_setsid_escapee_drained_via_orphan_killer_preserves_result_linepins the production path that fix(agent-runtime): kill npx MCP orphans outside claude pgid that hold stdout pipe open (#618) #620 fixed: parent emits result, forks asetsid()'d grandchild that holds stdout open, parent exits. Asserts_kill_orphan_pipe_writersfires insidedrain_reader_threadsand the bufferedRESULT_LINEsurvives — i.e. the drain doesn't fall through to the force-close fallback that discards the kernel pipe buffer. Sibling of the bug: drain_reader_threads closes stdout pipe before reader can drain backlog — silently loses final result line on long agentic tasks #531 buffered-data test, but with the setsid escape that's the real-world signature in production (git push → ssh).services.agent_clientinsys.modulesbaseline (fix(tests): test_audit_log_unit.py — 50 collection ImportErrors when run in full suite #762), narrow exceptions in agent_client preload, evict pollutedtask_execution_servicestub in CB probe tests, auto-loadTRINITY_TEST_PASSWORD+REDIS_BACKEND_PASSWORDfrom.env, override placeholder Redis password, skip.venv/__pycache__in thesys_moduleslinter and regen the baseline. Plus a post-recovery report (docs/memory/tests/post-recovery-report.md).fix(credentials): map agent-server connect errors to 503 onimport_credentials/export_credentials(commit35d4e78), matching the inject endpoint pattern.KNOWN_ISSUES.mdentry for the Stop-hook stdout-pipe-holder bug class with operator-side defense-in-depth recipes (bash/Python/Node) for hooks that spawn network processes.cso-2026-05-17): CLEAR at 8/10 confidence gate, no new CRITICAL/HIGH; one persistent MEDIUM (Dockerfile USER, tracked since 2026-04-05). Plus the bug: stdout reader thread stuck 35s after process exit when hook subprocess escapes process group kill #586 diff report (cso-diff-2026-05-13)..claudeto head withDEVELOPMENT_WORKFLOW.md.Test plan
pytest tests/unit/test_subprocess_pgroup.py::TestDrainReaderThreads::test_setsid_escapee_drained_via_orphan_killer_preserves_result_linepasses on Linux CI (skipped on macOS — uses/proc).sys_moduleslinter baseline change.git pushstop hook executes without theReader thread(s) still busy→force-closing pipes→Execution completed without a result messagefailure pattern.Refs #586
🤖 Generated with Claude Code