From 75e7b97830d07feb005d8f076f5f6fc63af2dccd Mon Sep 17 00:00:00 2001 From: jenkins Date: Wed, 12 Aug 2026 06:22:29 -0300 Subject: [PATCH] hermes: recover incomplete routed tool calls --- services/hermes/agent-deployment.yaml | 2 +- services/hermes/chat-configmap.yaml | 15 ++- services/hermes/chat-statefulset.yaml | 2 +- .../plugins/image-gen-broker/__init__.py | 126 +++++++++++++++++- services/hermes/scripts/codex_broker.py | 57 ++++++-- testing/tests/test_hermes_chat_quality.py | 72 +++++++++- 6 files changed, 254 insertions(+), 20 deletions(-) diff --git a/services/hermes/agent-deployment.yaml b/services/hermes/agent-deployment.yaml index 343255079..578fde802 100644 --- a/services/hermes/agent-deployment.yaml +++ b/services/hermes/agent-deployment.yaml @@ -25,7 +25,7 @@ spec: ai.bstein.dev/execution: Hermes Kanban with durable direct Codex and Claude Code CLI workers ai.bstein.dev/model-policy: Jetson-assisted AUTO routing, low through xhigh, cross-provider fallback ai.bstein.dev/placement: rpi5 preferred; Jetson deferred until state storage is available - ai.bstein.dev/config-rev: "20260812-provider-status-refreshable" + ai.bstein.dev/config-rev: "20260812-codex-incomplete-failover" vault.hashicorp.com/agent-inject: "true" vault.hashicorp.com/role: hermes-agent vault.hashicorp.com/agent-inject-secret-anthropic-token: kv/data/atlas/hermes/agent-tokens diff --git a/services/hermes/chat-configmap.yaml b/services/hermes/chat-configmap.yaml index a644f64e9..e96ed2edf 100644 --- a/services/hermes/chat-configmap.yaml +++ b/services/hermes/chat-configmap.yaml @@ -128,10 +128,12 @@ data: When a user asks to create or edit an image, use an image generation tool. Treat natural follow-ups such as "edit this", "turn it into", "make this image", or a reference to the subject in the most recent generated image - as image-edit requests. For those turns, reuse the newest `MEDIA:` image - path in the conversation as `image_url`; do not answer with instructions, - route the request as ordinary text, or require the user to upload the image - again. Preserve the most recently selected image lane for an edit unless + as image-edit requests. For those turns use `image_edit_latest`, + `image_edit_latest_local`, or `image_edit_latest_hosted`; these compact tools + resolve the newest private image on the server, so never reproduce its long + `MEDIA:` path in tool arguments. Do not answer with instructions, route the + request as ordinary text, or require the user to upload the image again. + Preserve the most recently selected image lane for an edit unless the user explicitly requests local, OpenAI/hosted, or AUTO instead. Use `image_generate_local` when the request says local, private, on my hardware, or FLUX. Use `image_generate_hosted` when the request says @@ -163,7 +165,8 @@ data: present a single final answer. Use the image generation tool for natural-language image creation and editing requests; generated images remain in this tenant's private cache. - Natural follow-ups that refer to the latest generated image must edit its - newest `MEDIA:` path rather than starting an unrelated text-only answer. + Natural follow-ups that refer to the latest generated image must use a + compact `image_edit_latest*` tool rather than starting an unrelated + text-only answer or copying a long `MEDIA:` path into tool arguments. Do not claim access to Kubernetes, Vault, Gitea, Brad's projects, other users, the agent coordinator, or automated triage. diff --git a/services/hermes/chat-statefulset.yaml b/services/hermes/chat-statefulset.yaml index 290cea04c..699cffb48 100644 --- a/services/hermes/chat-statefulset.yaml +++ b/services/hermes/chat-statefulset.yaml @@ -29,7 +29,7 @@ spec: ai.bstein.dev/router-wire-contract: ollama-numeric-keepalive ai.bstein.dev/isolation: one Hermes process and PVC per Keycloak subject ai.bstein.dev/model-policy: uniform automatic policy with per-user overrides - ai.bstein.dev/config-rev: "20260812-image-reference-normalization" + ai.bstein.dev/config-rev: "20260812-compact-image-edits" vault.hashicorp.com/agent-inject: "true" vault.hashicorp.com/role: hermes-chat vault.hashicorp.com/agent-inject-secret-anthropic-token: kv/data/atlas/hermes/agent-tokens diff --git a/services/hermes/plugins/image-gen-broker/__init__.py b/services/hermes/plugins/image-gen-broker/__init__.py index e8bbc2420..66122bced 100644 --- a/services/hermes/plugins/image-gen-broker/__init__.py +++ b/services/hermes/plugins/image-gen-broker/__init__.py @@ -74,6 +74,23 @@ IMAGE_GENERATE_PARAMETERS = { "required": ["prompt"], } +IMAGE_EDIT_PARAMETERS = { + "type": "object", + "properties": { + "prompt": { + "type": "string", + "description": "Detailed description of the requested edit.", + }, + "aspect_ratio": { + "type": "string", + "enum": ["landscape", "square", "portrait"], + "default": DEFAULT_ASPECT_RATIO, + "description": "Requested output aspect ratio.", + }, + }, + "required": ["prompt"], +} + LOCAL_IMAGE_SCHEMA = { "name": "image_generate_local", "description": ( @@ -104,6 +121,42 @@ HOSTED_IMAGE_SCHEMA = { "parameters": IMAGE_GENERATE_PARAMETERS, } +AUTO_EDIT_SCHEMA = { + "name": "image_edit_latest", + "description": ( + "Edit the newest generated image in this private conversation using " + "the automatic image route. Use this compact tool for natural " + "follow-ups such as turn this cat into a clown, change it, edit the " + "image, or make the pictured subject different when the user does not " + "name a provider. The server resolves the source image; do not copy a " + "MEDIA path into the tool call. Hosted quality is tried first and local " + "FLUX is the fallback." + ), + "parameters": IMAGE_EDIT_PARAMETERS, +} + +LOCAL_EDIT_SCHEMA = { + "name": "image_edit_latest_local", + "description": ( + "Edit the newest generated image using only private local FLUX. Use " + "when an edit follow-up says local, private, FLUX, or on my hardware. " + "The server resolves the source image; pass only the edit prompt and " + "optional aspect ratio." + ), + "parameters": IMAGE_EDIT_PARAMETERS, +} + +HOSTED_EDIT_SCHEMA = { + "name": "image_edit_latest_hosted", + "description": ( + "Edit the newest generated image using only hosted OpenAI GPT Image at " + "highest quality. Use when an edit follow-up says OpenAI, GPT Image, " + "hosted, or highest hosted quality. The server resolves the source " + "image; pass only the edit prompt and optional aspect ratio." + ), + "parameters": IMAGE_EDIT_PARAMETERS, +} + def _broker_key() -> str: """Load the internal relay key from process env or the private .env.""" @@ -184,6 +237,29 @@ def _local_image_data_url(value: str) -> str: return f"data:{mime};base64,{base64.b64encode(raw).decode('ascii')}" +def _latest_generated_image() -> str: + """Return the newest generated artifact from this tenant's private cache.""" + home = Path(os.environ.get("HERMES_HOME", "/opt/data")).resolve() + cache = (home / "cache" / "images").resolve() + if not cache.is_dir(): + raise ValueError("no generated image is available to edit") + candidates: list[Path] = [] + for path in cache.iterdir(): + try: + resolved = path.resolve(strict=True) + resolved.relative_to(cache) + except (OSError, ValueError): + continue + if ( + resolved.is_file() + and resolved.suffix.lower() in {".png", ".jpg", ".jpeg", ".gif", ".webp"} + ): + candidates.append(resolved) + if not candidates: + raise ValueError("no generated image is available to edit") + return str(max(candidates, key=lambda item: item.stat().st_mtime_ns)) + + class AtlasBrokerImageProvider(ImageGenProvider): """High-quality GPT Image generation through the owner-only broker.""" @@ -343,13 +419,45 @@ def _handle_hosted_image(args: dict[str, Any], **_kwargs: Any) -> str: return _handle_image_generate(args, "hosted") +def _handle_latest_edit(args: dict[str, Any], route: str) -> str: + """Edit the newest tenant artifact without model-generated path arguments.""" + try: + image_url = _latest_generated_image() + except ValueError as exc: + return json.dumps( + { + "success": False, + "image": None, + "error": str(exc), + "error_type": "missing_reference", + "requested_route": route, + } + ) + return _handle_image_generate({**args, "image_url": image_url}, route) + + +def _handle_auto_edit(args: dict[str, Any], **_kwargs: Any) -> str: + """Edit the newest image with hosted-first automatic failover.""" + return _handle_latest_edit(args, "auto") + + +def _handle_local_edit(args: dict[str, Any], **_kwargs: Any) -> str: + """Edit the newest image without hosted provider substitution.""" + return _handle_latest_edit(args, "local") + + +def _handle_hosted_edit(args: dict[str, Any], **_kwargs: Any) -> str: + """Edit the newest image only with hosted GPT Image.""" + return _handle_latest_edit(args, "hosted") + + def _image_tool_available() -> bool: """Expose the tool when at least one broker route is healthy.""" return AtlasBrokerImageProvider().is_available() def register(ctx: Any) -> None: - """Register the broker and explicit local/hosted image tools.""" + """Register generation routes and compact latest-image edit tools.""" ctx.register_image_gen_provider(AtlasBrokerImageProvider()) ctx.register_tool( name="image_generate_local", @@ -373,3 +481,19 @@ def register(ctx: Any) -> None: description=HOSTED_IMAGE_SCHEMA["description"], emoji="🎨", ) + for name, schema, handler in ( + ("image_edit_latest", AUTO_EDIT_SCHEMA, _handle_auto_edit), + ("image_edit_latest_local", LOCAL_EDIT_SCHEMA, _handle_local_edit), + ("image_edit_latest_hosted", HOSTED_EDIT_SCHEMA, _handle_hosted_edit), + ): + ctx.register_tool( + name=name, + toolset="image_gen", + schema=schema, + handler=handler, + check_fn=_image_tool_available, + requires_env=[], + is_async=False, + description=schema["description"], + emoji="🎨", + ) diff --git a/services/hermes/scripts/codex_broker.py b/services/hermes/scripts/codex_broker.py index d2e0108e5..970f3caf9 100644 --- a/services/hermes/scripts/codex_broker.py +++ b/services/hermes/scripts/codex_broker.py @@ -26,6 +26,9 @@ UPSTREAM = os.environ.get( "https://chatgpt.com/backend-api/codex", ).rstrip("/") MAX_BODY_BYTES = int(os.environ.get("HERMES_CODEX_BROKER_MAX_BODY", str(64 << 20))) +MAX_RESPONSE_BYTES = int( + os.environ.get("HERMES_CODEX_BROKER_MAX_RESPONSE", str(64 << 20)) +) READ_TIMEOUT_SECONDS = float(os.environ.get("HERMES_CODEX_BROKER_READ_TIMEOUT", "900")) FALLBACK_ALLOWED_MODELS = { value.strip() @@ -129,12 +132,24 @@ def _validate_payload(payload: Any) -> dict[str, Any]: # selected provider request. The first-party subscription Codex endpoint # does not accept any of the public API token-budget aliases. Let Codex use # its own response budget instead of turning a healthy fallback into a 400. - for token_budget_key in ( + for unsupported_key in ( "max_output_tokens", "max_completion_tokens", "max_tokens", + # The subscription Codex backend owns sampling. OpenAI-compatible + # clients may add these public API fields during a retry; forwarding + # them turns an otherwise healthy fallback into HTTP 400. + "temperature", + "top_p", + "frequency_penalty", + "presence_penalty", + "logprobs", + "top_logprobs", + "seed", + "n", + "stop", ): - payload.pop(token_budget_key, None) + payload.pop(unsupported_key, None) # Tenant conversations must not enter the owner's server-side history. payload["store"] = False payload["stream"] = True @@ -178,6 +193,18 @@ def _completed_response(lines: Iterable[str]) -> dict[str, Any]: else: upstream_error = str(error or "") if terminal_response is not None: + status = str(terminal_response.get("status") or "").lower() + if status != "completed": + details = terminal_response.get("incomplete_details") or {} + reason = ( + str(details.get("reason") or "") + if isinstance(details, dict) + else "" + ) + raise RuntimeError( + "Codex returned a retryable incomplete response" + + (f": {reason}" if reason else "") + ) # The subscription Codex endpoint streams complete output items but # currently leaves the terminal response's output array empty. Public # Responses clients, including Switchyard, expect those items there. @@ -273,22 +300,34 @@ class Handler(BaseHTTPRequestHandler): self.wfile.write(body) return + # Validate the terminal Responses event before committing + # HTTP 200 downstream. The Codex subscription endpoint can + # end a tool call with ``response.incomplete`` while still + # returning HTTP 200. If that is streamed through, Hermes + # sees malformed JSON and eventually exposes "Response + # truncated" to the user. Buffering one model boundary + # lets Switchyard receive a retryable 502 and choose another + # provider/model instead. Tool execution remains streamed + # by Hermes after this short model boundary completes. + body = response.read() + if len(body) > MAX_RESPONSE_BYTES: + raise RuntimeError("Codex response exceeded broker limit") + completed = _completed_response( + body.decode("utf-8", errors="replace").splitlines() + ) if not requested_stream: - self._json(200, _completed_response(response.iter_lines())) + self._json(200, completed) return - # HTTP/1.0 close-delimited streaming avoids buffering a - # potentially long tool-calling turn in the broker. self.send_response(200) self.send_header("Content-Type", "text/event-stream") + self.send_header("Content-Length", str(len(body))) self.send_header("Cache-Control", "no-store") self.send_header("X-Accel-Buffering", "no") self.end_headers() response_started = True - for chunk in response.iter_raw(): - if chunk: - self.wfile.write(chunk) - self.wfile.flush() + self.wfile.write(body) + self.wfile.flush() except ValueError as exc: self._json(400, {"error": {"message": str(exc), "type": "invalid_request_error"}}) except (BrokenPipeError, ConnectionResetError): diff --git a/testing/tests/test_hermes_chat_quality.py b/testing/tests/test_hermes_chat_quality.py index 58c5d771f..0acdcf495 100644 --- a/testing/tests/test_hermes_chat_quality.py +++ b/testing/tests/test_hermes_chat_quality.py @@ -352,7 +352,8 @@ def test_chat_image_generation_uses_private_owner_broker(): assert "Use `image_generate_local`" in configmap["data"]["SOUL.md"] assert "Use `image_generate_hosted`" in configmap["data"]["SOUL.md"] assert "ComfyUI endpoint" in configmap["data"]["SOUL.md"] - assert "newest `MEDIA:` image" in configmap["data"]["SOUL.md"] + assert "`MEDIA:` path" in configmap["data"]["SOUL.md"] + assert "`image_edit_latest`" in configmap["data"]["SOUL.md"] assert "Preserve the most recently selected image lane" in configmap["data"]["SOUL.md"] config = yaml.safe_load(configmap["data"]["config.yaml"]) assert config["image_gen"] == { @@ -378,6 +379,10 @@ def test_chat_image_generation_uses_private_owner_broker(): assert '"hosted": "gpt-image-2-high"' in plugin assert 'name="image_generate_local"' in plugin assert 'name="image_generate_hosted"' in plugin + assert '"name": "image_edit_latest"' in plugin + assert '"name": "image_edit_latest_local"' in plugin + assert '"name": "image_edit_latest_hosted"' in plugin + assert "def _latest_generated_image" in plugin assert "newest MEDIA: path from the conversation" in plugin assert 'candidate.upper().startswith("MEDIA:")' in plugin assert "override=True" not in plugin @@ -421,6 +426,56 @@ def test_chat_image_generation_uses_private_owner_broker(): ) +def test_compact_image_edit_resolves_latest_tenant_artifact(tmp_path, monkeypatch): + """Follow-up edits resolve the source server-side and keep tool JSON small.""" + provider_module = SimpleNamespace( + DEFAULT_ASPECT_RATIO="square", + ImageGenProvider=object, + error_response=lambda **value: value, + normalize_reference_images=lambda value: value, + resolve_aspect_ratio=lambda value: value, + save_b64_image=lambda *_args, **_kwargs: tmp_path / "saved.png", + success_response=lambda **value: value, + ) + monkeypatch.setitem(sys.modules, "agent", SimpleNamespace()) + monkeypatch.setitem(sys.modules, "agent.image_gen_provider", provider_module) + spec = importlib.util.spec_from_file_location( + "hermes_image_plugin", + HERMES / "plugins" / "image-gen-broker" / "__init__.py", + ) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + image_dir = tmp_path / "cache" / "images" + image_dir.mkdir(parents=True) + older = image_dir / "atlas_flux-old.png" + newest = image_dir / "atlas_gpt-image-new.png" + older.write_bytes(b"older") + newest.write_bytes(b"newest") + older.touch() + time.sleep(0.001) + newest.touch() + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + calls = [] + monkeypatch.setattr( + module, + "_handle_image_generate", + lambda args, route: calls.append((args, route)) or "ok", + ) + assert module._handle_hosted_edit({"prompt": "make it a clown"}) == "ok" + assert calls == [ + ( + { + "prompt": "make it a clown", + "image_url": str(newest.resolve()), + }, + "hosted", + ) + ] + + def test_chat_reasoning_uses_switchyard_without_owner_credentials(): """Family pods use AUTO/manual routes without mounting owner credentials.""" configmap = _documents(HERMES / "chat-configmap.yaml")[0] @@ -485,7 +540,7 @@ def test_chat_reasoning_uses_switchyard_without_owner_credentials(): statefulset = _documents(HERMES / "chat-statefulset.yaml")[0] assert statefulset["spec"]["template"]["metadata"]["annotations"][ "ai.bstein.dev/config-rev" - ] == "20260812-image-reference-normalization" + ] == "20260812-compact-image-edits" hermes = next( item for item in statefulset["spec"]["template"]["spec"]["containers"] @@ -529,6 +584,8 @@ def test_codex_broker_auth_and_request_contract(tmp_path: Path, monkeypatch): "max_output_tokens": 96, "max_completion_tokens": 96, "max_tokens": 96, + "temperature": 0.7, + "top_p": 0.9, } ) assert payload["store"] is False @@ -536,6 +593,8 @@ def test_codex_broker_auth_and_request_contract(tmp_path: Path, monkeypatch): assert "max_output_tokens" not in payload assert "max_completion_tokens" not in payload assert "max_tokens" not in payload + assert "temperature" not in payload + assert "top_p" not in payload assert payload["input"] == [ { "type": "message", @@ -601,6 +660,15 @@ def test_codex_broker_auth_and_request_contract(tmp_path: Path, monkeypatch): "data: [DONE]", ] )["output"] == [completed_item] + with pytest.raises(RuntimeError, match="retryable incomplete response"): + module._completed_response( + [ + "event: response.incomplete", + 'data: {"type":"response.incomplete","response":' + '{"status":"incomplete","incomplete_details":' + '{"reason":"max_output_tokens"}}}', + ] + ) with pytest.raises(RuntimeError, match="provider unavailable"): module._completed_response( [