From dba32a9a9de39ef5c1fd4c5f005ad049268aa6cd Mon Sep 17 00:00:00 2001 From: Oleg Date: Mon, 1 Jun 2026 11:03:32 -0700 Subject: [PATCH] fix(#151): refresh effort from registry on context_restart; fix PUT /effort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two #151 follow-ups surfaced while dogfooding native ultracode: 1. restart_streaming_session reused the live in-memory _config and never re-read thinking_effort from the registry. An out-of-band default change (direct DB write or PUT /agents/{name}/effort) was invisible to the running session until a full daemon restart — so native ultracode arming in _build_claude_cmd evaluated a stale effort and silently skipped. Refresh _config.thinking_effort from agents.get() before reconnect. A session-level _effort_override still wins in effective_effort, so this only makes the stored default authoritative. 2. PUT /agents/{name}/effort called a nonexistent agents.update() → AttributeError → 500. AgentRegistry uses register() as its upsert and already handles thinking_effort on existing agents. Tests: regression coverage for both (PUT persists default; restart refreshes effort from registry). Sibling restart tests stay green. Co-Authored-By: Claude Opus 4.8 --- src/pinky_daemon/api.py | 16 ++++++++++++- tests/test_api.py | 52 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/pinky_daemon/api.py b/src/pinky_daemon/api.py index f03c0e1a..08521859 100644 --- a/src/pinky_daemon/api.py +++ b/src/pinky_daemon/api.py @@ -5432,7 +5432,10 @@ async def set_agent_effort(name: str, req: dict): agent = agents.get(name) if not agent: raise HTTPException(404, f"Agent '{name}' not found") - agents.update(name, thinking_effort=level) + # AgentRegistry has no update(); register() is the upsert and handles + # the thinking_effort field on existing agents (#151 follow-up — the + # prior agents.update() call raised AttributeError → 500). + agents.register(name, thinking_effort=level) return {"agent": name, "default": level} @app.post("/agents/{name}/sessions/{session_label}/effort") @@ -7008,6 +7011,17 @@ async def restart_streaming_session(name: str): ss._config.wake_context = _build_streaming_wake_context(name, commit=False) ss._config.resume_handle = "" ss._config.restart_reason = "context_restart" + # #151: refresh the persistent effort from the registry before relaunch. + # The live session reuses its boot-time _config, so an out-of-band + # default change (direct DB write, PUT /agents/{name}/effort) is + # otherwise invisible until a full daemon restart — which meant native + # ultracode arming in _build_claude_cmd evaluated a stale effort and + # silently skipped. A session-level override (_effort_override, set via + # set_thinking_effort) still wins in effective_effort, so this only + # makes the stored default authoritative, never clobbers an override. + fresh = agents.get(name) + if fresh: + ss._config.thinking_effort = fresh.thinking_effort or "medium" # PR for #543: explicit launch-behavior contract — the next # transport spawn must NOT resume the prior conversation. For # SDK this is implicit (resume_handle="" already accomplishes diff --git a/tests/test_api.py b/tests/test_api.py index 09c2484a..3591a038 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -995,6 +995,58 @@ def test_streaming_restart_clears_codex_session_id_on_codex_sessions(self): ) assert fake.resume_handle == "" + def test_put_agent_effort_persists_default(self): + """PUT /agents/{name}/effort must persist the default via the + registry. Regression for #151: the route called a nonexistent + agents.update() → AttributeError → 500.""" + with tempfile.TemporaryDirectory() as tmpdir: + db_path = os.path.join(tmpdir, "test.db") + app = self._make_app(db_path) + with TestClient(app) as client: + client.post("/agents", json={"name": "test-agent", "model": "sonnet"}) + + resp = client.put("/agents/test-agent/effort", json={"effort": "ultracode"}) + assert resp.status_code == 200, resp.text + assert resp.json()["default"] == "ultracode" + + got = client.get("/agents/test-agent/effort") + assert got.status_code == 200 + assert got.json()["default"] == "ultracode" + + def test_streaming_restart_refreshes_thinking_effort_from_registry(self): + """A live session reuses its boot-time _config, so an out-of-band + default change is invisible until the daemon restarts — which left + native ultracode arming evaluating a stale effort. Restart must + refresh _config.thinking_effort from the registry (#151).""" + with tempfile.TemporaryDirectory() as tmpdir: + db_path = os.path.join(tmpdir, "test.db") + app = self._make_app(db_path) + with TestClient(app) as client: + client.post("/agents", json={"name": "test-agent", "model": "sonnet"}) + fake = self._FakeStreamingSession("test-agent", "main") + # Boot-time config still on the stale default. + fake._config.thinking_effort = "medium" + app.state.broker.register_streaming("test-agent", fake, label="main") + + # Out-of-band default bump (mirrors a DB write / PUT /effort). + app.state.agents.register("test-agent", thinking_effort="ultracode") + + app.state.agents.set_context( + "test-agent", + task="Testing effort refresh on restart", + metadata={"source": "save_my_context"}, + updated_by=fake.resume_handle, + ) + fake.last_active = time.time() + + resp = client.post("/agents/test-agent/streaming/restart") + assert resp.status_code == 200, resp.text + assert resp.json()["restarted"] is True + assert fake._config.thinking_effort == "ultracode", ( + "restart must refresh the persistent effort from the " + "registry so native ultracode arming sees the live default" + ) + # ────────────────────────────────────────────────────────────────── # Task #103 — /admin/force-restart-agent/{name} # Wedged-agent recovery escape hatch. Bumps agent_contexts.updated_at