fix(sleep): reset ClaudeCliBackend.last_call_error at each call start
Follow-up to #126. ClaudeCliBackend recorded last_call_error on failure but never reset it, so a failure followed by a successful call still reported the stale error in persisted diagnostics — making a healthy run look failed. Both _call() and attempt_with_tools() now clear it at the start, matching CodexCliBackend. Note: the "TimeoutExpired leaks the prompt into logs" concern raised in review does NOT apply on current main — the prompt is passed via stdin (input=), not argv (fixed by #98), so TimeoutExpired's str() only contains the argv (no prompt). Verified empirically. Adds a regression test asserting a prior failure is cleared on the next success. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -615,6 +615,9 @@ class ClaudeCliBackend(CliBackend):
|
|||||||
# --exclude-dynamic-... drop per-machine cwd/env/memory/git sections
|
# --exclude-dynamic-... drop per-machine cwd/env/memory/git sections
|
||||||
# cwd=<clean temp> no project CLAUDE.md
|
# cwd=<clean temp> no project CLAUDE.md
|
||||||
import tempfile
|
import tempfile
|
||||||
|
# Reset per call so a prior failure does not make a later successful
|
||||||
|
# call look failed in persisted diagnostics (matches CodexCliBackend).
|
||||||
|
self.last_call_error = ""
|
||||||
cmd = [self.claude_path, "-p", "--output-format", "text"]
|
cmd = [self.claude_path, "-p", "--output-format", "text"]
|
||||||
if os.environ.get("ANTHROPIC_API_KEY"):
|
if os.environ.get("ANTHROPIC_API_KEY"):
|
||||||
cmd.append("--bare")
|
cmd.append("--bare")
|
||||||
@@ -657,6 +660,7 @@ class ClaudeCliBackend(CliBackend):
|
|||||||
# validated honestly: we detect the call from the shim's log, not from
|
# validated honestly: we detect the call from the shim's log, not from
|
||||||
# a self-reported marker. Other tools are stubbed the same way.
|
# a self-reported marker. Other tools are stubbed the same way.
|
||||||
import tempfile, shutil, stat
|
import tempfile, shutil, stat
|
||||||
|
self.last_call_error = ""
|
||||||
work = tempfile.mkdtemp(prefix="skillopt_sleep_tools_")
|
work = tempfile.mkdtemp(prefix="skillopt_sleep_tools_")
|
||||||
calllog = os.path.join(work, "_tool_calls.log")
|
calllog = os.path.join(work, "_tool_calls.log")
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -1148,6 +1148,26 @@ class TestClaudeCliBackendBare(unittest.TestCase):
|
|||||||
self.assertEqual(resp, "")
|
self.assertEqual(resp, "")
|
||||||
self.assertIn("Claude CLI spawn failed", be.last_call_error)
|
self.assertIn("Claude CLI spawn failed", be.last_call_error)
|
||||||
|
|
||||||
|
def test_last_call_error_cleared_on_next_success(self):
|
||||||
|
"""A prior failure must not make a later successful call look failed:
|
||||||
|
_call resets last_call_error at the start of each call."""
|
||||||
|
from unittest import mock
|
||||||
|
from skillopt_sleep.backend import ClaudeCliBackend
|
||||||
|
be = ClaudeCliBackend(claude_path="claude", timeout=3)
|
||||||
|
# First call fails to spawn -> error recorded.
|
||||||
|
with mock.patch("skillopt_sleep.backend.subprocess.run",
|
||||||
|
side_effect=FileNotFoundError("no claude")):
|
||||||
|
self.assertEqual(be._call("p1"), "")
|
||||||
|
self.assertIn("Claude CLI spawn failed", be.last_call_error)
|
||||||
|
# Next call succeeds -> stale error must be cleared.
|
||||||
|
|
||||||
|
class _OK:
|
||||||
|
stdout = "answer"
|
||||||
|
stderr = ""
|
||||||
|
with mock.patch("skillopt_sleep.backend.subprocess.run", return_value=_OK()):
|
||||||
|
self.assertEqual(be._call("p2"), "answer")
|
||||||
|
self.assertEqual(be.last_call_error, "")
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user