fix(pm-agent): log the CLI's stdout envelope when triage fails; add PM_DECISION_MAX_TURNS
`claude --print --output-format json` reports its own failures (API error, usage limit, error_max_turns) as the result envelope on stdout and leaves stderr empty, so ask_pm()'s stderr-only message logged "claude exited 1: " with nothing after it, twice on 2026-10-31. describe_cli_failure() now logs the envelope's diagnostic fields plus stdout/stderr tails, redacted (sk-ant-, Bearer/token, URL userinfo). PM_DECISION_MAX_TURNS (default 1, unchanged) makes the max-turns remedy a ConfigMap change if the envelope turns out to say error_max_turns. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
95e08da2f0
commit
664ee82b9a
3 files changed
+178
-4
No files matched your search
@@ -214,6 +214,12 @@ class Config:
|
||||
# ConfigMap without rebuilding the image.
|
||||
self.push_on_red = env("PM_PUSH_ON_RED_GATE", "false")
|
||||
self.pm_timeout = env_int("PM_DECISION_TIMEOUT_SECONDS", 900)
|
||||
# The PM holds no tools, so the only thing a second turn can be spent on is the CLI's
|
||||
# structured-output step re-asking the model after an answer that failed the schema.
|
||||
# 1 is the long-standing value; it is a knob so that, if the failure envelope now
|
||||
# logged by describe_cli_failure() says `error_max_turns`, the remedy is a ConfigMap
|
||||
# change rather than another image build.
|
||||
self.pm_max_turns = env_int("PM_DECISION_MAX_TURNS", 1)
|
||||
# Opt-in gate. Empty (the default) means every open issue is in scope, which is the
|
||||
# behaviour the service was asked for. Setting it to a label name — `agent` is the
|
||||
# conventional one — narrows the agent to issues a human has explicitly handed it,
|
||||
@@ -231,6 +237,8 @@ class Config:
|
||||
|
||||
if self.max_active_workers < 1:
|
||||
die("PM_MAX_ACTIVE_WORKERS must be >= 1")
|
||||
if not 1 <= self.pm_max_turns <= 5:
|
||||
die("PM_DECISION_MAX_TURNS must be between 1 and 5")
|
||||
if "@sha256:" not in self.worker_image:
|
||||
# The admission policy requires a digest too, so a tag here would produce a Job
|
||||
# that is created and then DENIED — a failure visible only in the dispatcher's
|
||||
@@ -437,7 +445,7 @@ def ask_pm(cfg: Config, thread: str) -> dict:
|
||||
"--system-prompt-file", cfg.persona_file,
|
||||
"--tools", "",
|
||||
"--permission-prompts", "none",
|
||||
"--max-turns", "1",
|
||||
"--max-turns", str(cfg.pm_max_turns),
|
||||
"--output-format", "json",
|
||||
"--json-schema", json.dumps(DECISION_SCHEMA),
|
||||
"--no-session-persistence",
|
||||
@@ -445,10 +453,76 @@ def ask_pm(cfg: Config, thread: str) -> dict:
|
||||
proc = subprocess.run(cmd, input=thread, capture_output=True, text=True,
|
||||
timeout=cfg.pm_timeout)
|
||||
if proc.returncode != 0:
|
||||
raise RuntimeError(f"claude exited {proc.returncode}: {proc.stderr.strip()[:800]}")
|
||||
raise RuntimeError(describe_cli_failure(proc.returncode, proc.stdout, proc.stderr))
|
||||
return extract_decision(proc.stdout)
|
||||
|
||||
|
||||
# Anything that could be a credential in the CLI's output. The CLI does not normally echo its
|
||||
# token, but an API error body or a proxy's HTML page is not something to bet a leak on. Both
|
||||
# schemes are matched (the http one is the lesson of a scheme-specific redaction that leaked
|
||||
# ~/.git-credentials), and `token ...` is Gitea's own header scheme.
|
||||
_SECRET_PATTERNS = [
|
||||
(re.compile(r"sk-ant-[A-Za-z0-9_\-]+"), "sk-ant-[REDACTED]"),
|
||||
(re.compile(r"(?i)\b(bearer|token)(\s+|=|:\s*)[A-Za-z0-9._~+/=\-]{8,}"), r"\1\2[REDACTED]"),
|
||||
(re.compile(r"(?i)(https?://)[^/\s:@]+:[^/\s@]+@"), r"\1[REDACTED]@"),
|
||||
]
|
||||
|
||||
|
||||
def redact(text: str) -> str:
|
||||
for pat, repl in _SECRET_PATTERNS:
|
||||
text = pat.sub(repl, text)
|
||||
return text
|
||||
|
||||
|
||||
def describe_cli_failure(rc: int, stdout: str, stderr: str, limit: int = 800) -> str:
|
||||
"""Say WHY `claude --print` failed, in the dispatcher log, redacted.
|
||||
|
||||
WHY THIS EXISTS: with `--output-format json` the CLI reports its own failures — an API
|
||||
error, a usage limit, `error_max_turns`, a schema the model could not satisfy — as the
|
||||
JSON result envelope on STDOUT (`is_error`, `subtype`, `result`), and writes nothing to
|
||||
stderr. Logging only stderr therefore produced `claude exited 1: ` with an empty message
|
||||
on 2026-10-31, twice, with the cause sitting unread in stdout. So: the envelope's
|
||||
diagnostic fields first, then the raw tails of both streams.
|
||||
"""
|
||||
parts = [f"claude exited {rc}"]
|
||||
out = (stdout or "").strip()
|
||||
env = None
|
||||
if out:
|
||||
try:
|
||||
env = json.loads(out)
|
||||
except ValueError:
|
||||
# A stream of JSON lines, or a log line ahead of the envelope: the last line that
|
||||
# parses as an object is the result.
|
||||
for line in reversed(out.splitlines()):
|
||||
try:
|
||||
cand = json.loads(line)
|
||||
except ValueError:
|
||||
continue
|
||||
if isinstance(cand, dict):
|
||||
env = cand
|
||||
break
|
||||
if isinstance(env, dict):
|
||||
fields = []
|
||||
for key in ("subtype", "is_error", "api_error_status", "num_turns", "duration_ms",
|
||||
"duration_api_ms", "total_cost_usd", "stop_reason"):
|
||||
if key in env:
|
||||
fields.append(f"{key}={env[key]!r}")
|
||||
if fields:
|
||||
parts.append("envelope: " + " ".join(fields))
|
||||
for key in ("result", "error", "errors", "message"):
|
||||
if env.get(key):
|
||||
val = env[key] if isinstance(env[key], str) else json.dumps(env[key])
|
||||
parts.append(f"{key}: {val[:limit]}")
|
||||
if out and not isinstance(env, dict):
|
||||
parts.append(f"stdout tail: {out[-limit:]}")
|
||||
err_text = (stderr or "").strip()
|
||||
if err_text:
|
||||
parts.append(f"stderr tail: {err_text[-limit:]}")
|
||||
if len(parts) == 1:
|
||||
parts.append("(no output on stdout or stderr)")
|
||||
return redact(" | ".join(parts))
|
||||
|
||||
|
||||
def extract_decision(stdout: str) -> dict:
|
||||
"""Pull the decision object out of whatever `claude --print` wrapped it in.
|
||||
|
||||
@@ -501,7 +575,7 @@ def extract_decision(stdout: str) -> dict:
|
||||
obj = as_obj(c)
|
||||
if obj and isinstance(obj.get("action"), str):
|
||||
return obj
|
||||
raise RuntimeError(f"no decision object in claude output: {stdout[:400]!r}")
|
||||
raise RuntimeError(redact(f"no decision object in claude output: {stdout[-800:]!r}"))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- thread
|
||||
@@ -988,8 +1062,25 @@ def selftest() -> int:
|
||||
except RuntimeError:
|
||||
check(True, f"bad output refused: {label}")
|
||||
|
||||
# -- a failed CLI run must say why, and must not leak ---------------------
|
||||
# The 2026-10-31 failure: rc 1, empty stderr, the cause in the stdout envelope.
|
||||
env_err = json.dumps({"type": "result", "subtype": "error_max_turns", "is_error": True,
|
||||
"num_turns": 2, "result": "API Error: 429 rate_limit_error"})
|
||||
msg = describe_cli_failure(1, env_err, "")
|
||||
check("error_max_turns" in msg and "429" in msg,
|
||||
"CLI failure: stdout envelope surfaced when stderr is empty")
|
||||
msg = describe_cli_failure(1, "", "boom on stderr")
|
||||
check("boom on stderr" in msg, "CLI failure: stderr tail surfaced")
|
||||
msg = describe_cli_failure(1, "", "")
|
||||
check("no output" in msg, "CLI failure: silence is named, not an empty message")
|
||||
msg = describe_cli_failure(1, "plain text sk-ant-oat01-abcdefABCDEF_123 here",
|
||||
"Authorization: Bearer abcdefghijklmnop "
|
||||
"http://u:[email protected]/x")
|
||||
check("sk-ant-oat01" not in msg and "abcdefghijklmnop" not in msg and ":pw@" not in msg,
|
||||
"CLI failure: token, bearer and URL credential redacted")
|
||||
|
||||
# -- the committed Job template -----------------------------------------
|
||||
tmpl = os.environ.get("PM_WORKER_JOB_TEMPLATE", "/etc/pm-agent/worker-job-template.yaml")
|
||||
tmpl =os.environ.get("PM_WORKER_JOB_TEMPLATE", "/etc/pm-agent/worker-job-template.yaml")
|
||||
try:
|
||||
selftest_template(tmpl)
|
||||
check(True, f"worker Job template satisfies its contract ({tmpl})")
|
||||
|
||||
Reference in new issue
Block a user