[codex] Harden adapter layer: dedup helpers, reuse isolation, coverage gate - #24
Merged
Merged
Conversation
Consolidate the helpers that had been copy-pasted across the three vendor adapters into adapters/_common.py (field_value, optional_int, optional_str, fingerprint_value/fingerprint_item, and a single close_vendor_resource ladder), removing the per-adapter drift that previously produced three identical bugs. Also harden the reuse path: - Claude: scope the reuse cache key by conversation identity so tasks with distinct sessions never share one client; expose sdk_process_reuse_scope. - Antigravity: only chmod the runtime dir when we own it, and tolerate (log, not crash) a chmod failure on a shared/non-owned data dir. - All adapters: log a warning when closing a vendor resource fails after a startup failure instead of silently suppressing it.
Add tests/test_adapter_protocol.py parametrizing all three adapters through one AgentRuntime conformance suite (protocol membership, async lifecycle methods, availability diagnostic, ordered task lifecycle events). Regression coverage for the hardening: - Claude reuse keyed by session (shared vs conversation scope) and a close- failure-after-connect-failure warning path. - Antigravity runtime dir survives a chmod PermissionError; close-failure warning after enter failure. - Codex close-failure warning after enter failure. - safe_emit: an exploding event sink must not abort a run.
- Add pytest-cov dev dependency, [tool.coverage] config, and a --cov-fail-under=85 gate in CI (current coverage ~89%). Mark the real-SDK import bodies in each _load_sdk with pragma: no cover (unreachable in the SDK-free CI lanes). - docs/providers.md: document Claude's session-scoped reuse key and sdk_process_reuse_scope, and that reuse_process=True serializes runs on a runtime instance.
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
1 participant
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.
Stacked on #23 (
codex/codex-process-reuse). Addresses the adapter-layer review findings on top of the SDK process-reuse work.What this does
Refactor — kill the cross-adapter duplication
The three vendor adapters had copy-pasted helpers that drifted (and previously produced three identical bugs). Consolidated into
adapters/_common.py:field_value,optional_int,optional_strfingerprint_value/fingerprint_item(single superset incl. the pydanticmodel_dumpbranch)close_vendor_resource(primary, *, fallback=, try_disconnect=)— one close ladder for all threeFixes
resume_from/session_id), so two tasks with distinct sessions never share one client even with identical options. No-session tasks still share the process (isolated by per-query session id). NewAgentResult.metadata["sdk_process_reuse_scope"]("conversation"/"shared")._runtime_dironly enforces0o700on dirs we own and tolerates (logs, does not crash) achmodfailure on a shared / non-owneddata_dir.logging.warning) instead of being silently suppressed. The original startup error still propagates.Tests
tests/test_adapter_protocol.py: one parametrizedAgentRuntimeconformance suite across all three adapters (protocol membership, async lifecycle, availability diagnostic, ordered task lifecycle events).safe_emitresilience (exploding sink must not abort a run).Coverage gate + docs
pytest-cov+--cov-fail-under=85in CI (current ~89%); real-SDK_load_sdkimports marked# pragma: no cover(unreachable in the SDK-free CI lanes).docs/providers.md: documents the session-scoped reuse key,sdk_process_reuse_scope, and thatreuse_process=Trueserializes runs on a runtime instance.Verification
uv run ruff check .— cleanuv run mypy— clean (14 files)uv run pytest -q --cov=agent_runtime_kit --cov-fail-under=85— 154 passed, 3 skipped, coverage 88.94%Default per-call isolation behavior is unchanged.