From 99ec0304ac9cc2ed54875e9bd3559d8e808c334f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:08:12 -0700 Subject: [PATCH] refactor(agent/acp,credits): shared reader pump, dict-comprehension header parse, sentinel sweep --- agent/copilot_acp_client.py | 34 ++++++++++++------------------- agent/credits_tracker.py | 37 +++++++++++----------------------- agent/gemini_native_adapter.py | 7 ++++--- 3 files changed, 29 insertions(+), 49 deletions(-) diff --git a/agent/copilot_acp_client.py b/agent/copilot_acp_client.py index 2038a10536..452a96701a 100644 --- a/agent/copilot_acp_client.py +++ b/agent/copilot_acp_client.py @@ -141,10 +141,6 @@ def _jsonrpc_error(message_id: Any, code: int, message: str) -> dict[str, Any]: return {"jsonrpc": "2.0", "id": message_id, "error": {"code": code, "message": message}} -def _permission_denied(message_id: Any) -> dict[str, Any]: - return _jsonrpc_result(message_id, {"outcome": {"outcome": "cancelled"}}) - - def _enabled_ids(entries: Any, key: str) -> set[str]: """Ids of ``entries`` (dicts) whose ``_meta.copilotEnablement`` is not ``disabled``.""" return { @@ -369,19 +365,18 @@ class CopilotACPClient: inbox: queue.Queue[dict[str, Any]] = queue.Queue() stderr_tail: deque[str] = deque(maxlen=40) - def _stdout_reader() -> None: - for line in proc.stdout: - try: - inbox.put(json.loads(line)) - except Exception: - inbox.put({"raw": line.rstrip("\n")}) + def _decode(line: str) -> dict[str, Any]: + try: + return json.loads(line) + except Exception: + return {"raw": line.rstrip("\n")} - def _stderr_reader() -> None: - for line in proc.stderr or (): - stderr_tail.append(line.rstrip("\n")) + def _pump(stream, sink) -> None: + for line in stream or (): + sink(line) - threading.Thread(target=_stdout_reader, daemon=True).start() - threading.Thread(target=_stderr_reader, daemon=True).start() + threading.Thread(target=_pump, args=(proc.stdout, lambda line: inbox.put(_decode(line))), daemon=True).start() + threading.Thread(target=_pump, args=(proc.stderr, lambda line: stderr_tail.append(line.rstrip("\n"))), daemon=True).start() request_ids = iter(range(1, 1 << 62)) def _request(method: str, params: dict[str, Any], *, text_parts: list[str] | None = None, reasoning_parts: list[str] | None = None) -> Any: @@ -416,18 +411,15 @@ class CopilotACPClient: session_id = str(session.get("sessionId") or "").strip() if not session_id: raise RuntimeError("Copilot ACP did not return a sessionId.") - # Stable ACP v1 session-config API first; session/set_model is the fallback. if requested_model and requested_model != "copilot-acp": try: - selection = _model_selection_request(session, requested_model) - if selection is not None: + if (selection := _model_selection_request(session, requested_model)) is not None: _request(*selection) else: logger.warning("Copilot ACP does not offer model %r; using the session default.", requested_model) except Exception as exc: logger.warning( - "Copilot ACP model selection for %r failed; continuing with the session default: %s", - requested_model, exc, + "Copilot ACP model selection for %r failed; continuing with the session default: %s", requested_model, exc ) text_parts: list[str] = [] reasoning_parts: list[str] = [] @@ -462,7 +454,7 @@ class CopilotACPClient: message_id = msg.get("id") if method == "session/request_permission": - response = _permission_denied(message_id) + response = _jsonrpc_result(message_id, {"outcome": {"outcome": "cancelled"}}) elif method in _FS_HANDLERS: try: response = _jsonrpc_result(message_id, _FS_HANDLERS[method](msg.get("params") or {}, cwd)) diff --git a/agent/credits_tracker.py b/agent/credits_tracker.py index a5b6905ad8..493ea8a9e1 100644 --- a/agent/credits_tracker.py +++ b/agent/credits_tracker.py @@ -290,26 +290,19 @@ def parse_credits_headers(headers: Mapping[str, str], provider: str = "") -> Opt _version_warning_emitted = True logger.warning("credits header version %d unsupported, ignoring — update Hermes", version_val) return None - fields: dict[str, Any] = {} - for name, key, kind, *default in _HEADER_FIELDS: - val = _parse_field(kind, lowered.get(key), *default) - if val is _SENTINEL: - return None - fields[name] = val + fields: dict[str, Any] = { + name: _parse_field(kind, lowered.get(key), *default) for name, key, kind, *default in _HEADER_FIELDS + } # tool_pool_micros is OPTIONAL: absent → 0; present-but-invalid → miss. tp_raw = lowered.get("x-nous-tool-pool-micros") fields["tool_pool_micros"] = 0 if tp_raw is None else _parse_field("micros", tp_raw) - if fields["tool_pool_micros"] is _SENTINEL: - return None lim_micros_raw = lowered.get("x-nous-credits-subscription-limit-micros") lim_usd_raw = lowered.get("x-nous-credits-subscription-limit-usd") if lim_micros_raw is not None and lim_usd_raw is not None: - lm = _parse_field("micros", lim_micros_raw) - if lm is _SENTINEL or not _validate_usd(lim_usd_raw): - return None - fields["subscription_limit_micros"], fields["subscription_limit_usd"] = lm, lim_usd_raw + fields["subscription_limit_micros"] = _parse_field("micros", lim_micros_raw) + fields["subscription_limit_usd"] = _parse_field("usd", lim_usd_raw) denominator_kind = lowered.get("x-nous-credits-denominator-kind", "none") - if denominator_kind not in _VALID_DENOMINATOR_KINDS: + if _SENTINEL in fields.values() or denominator_kind not in _VALID_DENOMINATOR_KINDS: return None return CreditsState( version=version_val, denominator_kind=denominator_kind, @@ -361,10 +354,8 @@ def dev_fixture_credits_state() -> Optional[CreditsState]: """Fixture CreditsState for HERMES_DEV_CREDITS_FIXTURE, or None (unknown name / unset). Prod-leak guard: applies ONLY when HERMES_DEV_CREDITS is also on, so a stray fixture env var can never surface fabricated balances on a real account.""" - if not is_truthy_value(os.environ.get("HERMES_DEV_CREDITS")): - return None name = os.environ.get("HERMES_DEV_CREDITS_FIXTURE", "").strip() - if not name: + if not name or not is_truthy_value(os.environ.get("HERMES_DEV_CREDITS")): return None if os.path.sep in name or "/" in name: # looks like a path → read the name from the file try: @@ -372,12 +363,10 @@ def dev_fixture_credits_state() -> Optional[CreditsState]: name = fh.read().strip() except OSError: return None - spec = _DEV_FIXTURES.get(name.lower()) - if not spec: + if not (spec := _DEV_FIXTURES.get(name.lower())): return None - # Stamp what the REAL parser always guarantees so a fixture is field-identical - # to a parse_credits_headers() result (differential test): version 1, and a - # valid purchased_usd (a zero-top-up account still carries "0.00"). + # Stamp what the REAL parser always guarantees so a fixture is field-identical to a + # parse_credits_headers() result: version 1 and a valid purchased_usd (zero top-up = "0.00"). return CreditsState(**{"version": 1, "purchased_usd": "0.00", **spec}, from_header=True, captured_at=time.time()) @@ -418,11 +407,9 @@ def _hydrate_seed_state(agent, state) -> None: agent._credits_session_start_micros = state.remaining_micros latch = getattr(agent, "_credits_latch", None) if isinstance(latch, dict) and state.used_fraction is not None: - # Prime ONLY seen_below_90. Never prime seen_grant_unspent: a seed - # observing grant-spent is a steady state; priming revives the nag. + # Prime ONLY seen_below_90 — priming seen_grant_unspent would revive the nag (grant-spent is a steady state). latch["seen_below_90"] = True - emit = getattr(agent, "_emit_credits_notices", None) - if callable(emit): + if callable(emit := getattr(agent, "_emit_credits_notices", None)): emit() diff --git a/agent/gemini_native_adapter.py b/agent/gemini_native_adapter.py index ddfb39904c..0bef6d6cdd 100644 --- a/agent/gemini_native_adapter.py +++ b/agent/gemini_native_adapter.py @@ -194,9 +194,10 @@ def _extract_multimodal_parts(content: Any) -> List[Dict[str, Any]]: text = _text_of(item) if text or isinstance(item, str): parts.append({"text": text}) - elif isinstance(item, dict) and item.get("type") == "image_url": - if image := _inline_data_part((item.get("image_url") or {}).get("url") or ""): - parts.append(image) + elif isinstance(item, dict) and item.get("type") == "image_url" and ( + image := _inline_data_part((item.get("image_url") or {}).get("url") or "") + ): + parts.append(image) return parts