From 4a06e7ba237d265657d2edc9a7f5264d1ee7d352 Mon Sep 17 00:00:00 2001 From: lixiang <1014027506@qq.com> Date: Tue, 8 Sep 2026 16:22:11 +0800 Subject: [PATCH] fix(adapters): report why a turn produced nothing instead of nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rounds of the GUI matrix have ended with every agent reaching the LLM call and then reporting `No agent reply within 300s`, with no other information anywhere. The gateway and the credentials are both verified now — the same key and base URL drive the claude CLI to a correct answer from a shell on the same box — so what is left is that a failing turn has nothing to say for itself. Two reasons for that, both here. claude.js collected the child's stderr into `pp.stderrBuf` and read it back in exactly no place. When the CLI died before emitting its first JSON event there was no `result` to quote, so the channel got "No response generated. Please try again." and the daemon log got an exit code — while the actual account of what happened sat in a string nobody looked at. When it hung instead, the watchdog killed it at 300s and said "became unresponsive", again dropping the buffer. codex, gemini and opencode all log their stderr on exit; claude now does too, and also uses it on the two paths where it is the only thing there is: the watchdog kill and the silent exit. The "no response" fallback carries the exit code rather than nothing. Secrets are stripped on the way out — a CLI failing on auth tends to echo the key it was handed. The redaction rule already existed as identical private statics on codex and opencode; a third copy of a security- relevant rule is one too many, so it moves to adapters/utils.js and both delegate. Redaction runs over the whole buffer before the tail is cut, so a key cannot survive by being split across the boundary. The other reason is a dead heat. Every adapter gives up at exactly 300s — claude's stdout watchdog (20 x 15s), codex's direct-LLM request timeout, opencode's TIMEOUT_MS, gemini's idle monitor — and respond.spec polled for exactly 300s. The adapter posts its diagnosis to the channel at the moment the test stops listening, which is why every failure has read as the contentless timeout while the real reason was a second away. The spec now waits past it, so the adapter's own message becomes the assertion's failure text. --- .../agent-connector/src/adapters/claude.js | 41 +++++++++++++-- .../agent-connector/src/adapters/codex.js | 14 +----- .../agent-connector/src/adapters/opencode.js | 15 +----- .../agent-connector/src/adapters/utils.js | 27 ++++++++++ .../test/claude-stderr-surfacing.test.js | 50 +++++++++++++++++++ packages/launcher/e2e/respond.spec.ts | 12 ++++- 6 files changed, 127 insertions(+), 32 deletions(-) create mode 100644 packages/agent-connector/test/claude-stderr-surfacing.test.js diff --git a/packages/agent-connector/src/adapters/claude.js b/packages/agent-connector/src/adapters/claude.js index 52292bdaa..eb70f944c 100644 --- a/packages/agent-connector/src/adapters/claude.js +++ b/packages/agent-connector/src/adapters/claude.js @@ -17,7 +17,7 @@ const path = require('path'); const { execSync, spawn } = require('child_process'); const BaseAdapter = require('./base'); -const { formatAttachmentsForPrompt, SESSION_DEFAULT_RE, generateSessionTitle } = require('./utils'); +const { formatAttachmentsForPrompt, SESSION_DEFAULT_RE, generateSessionTitle, redactSecrets } = require('./utils'); const { buildClaudeSystemPrompt, buildClaudeSkillMd, workspaceSkillName } = require('./workspace-prompt'); const { pinnedFingerprint, sampleRecap } = require('./decision-log'); const { defaultAgentWorkdir, whichBinary, whereBinary } = require('../paths'); @@ -737,9 +737,17 @@ class ClaudeAdapter extends BaseAdapter { } if (consecutiveTimeouts >= this._WATCHDOG_MAX_TIMEOUTS) { + const tail = this._stderrTail(pp); this._log(`Watchdog: process unresponsive for ${consecutiveTimeouts * 15}s on ${pp.msgChannel} — killing`); + if (tail) this._log(`stderr: ${tail}`); this._stopWatchdog(pp); - try { await this.sendError(pp.msgChannel, 'Agent process became unresponsive and was restarted.'); } catch {} + try { + await this.sendError( + pp.msgChannel, + 'Agent process became unresponsive and was restarted.' + + (tail ? `\n\n\`\`\`\n${tail}\n\`\`\`` : ''), + ); + } catch {} if (pp.messageResolve) { const resolve = pp.messageResolve; pp.messageResolve = null; @@ -926,6 +934,8 @@ class ClaudeAdapter extends BaseAdapter { proc.on('exit', (code) => { this._log(`Persistent process exited: channel=${channel} code=${code}`); + const tail = this._stderrTail(pp); + if (tail) this._log(`stderr: ${tail}`); pp.alive = false; if (pp.idleTimer) clearTimeout(pp.idleTimer); this._stopWatchdog(pp); @@ -992,6 +1002,22 @@ class ClaudeAdapter extends BaseAdapter { }); } + /** + * What the CLI wrote to stderr this turn, trimmed and redacted, or ''. + * + * `stderrBuf` was collected and then read by nothing at all: when the process + * died before emitting a single JSON event, or hung without emitting one, the + * only account of why sat in a string no code path ever looked at. The channel + * got "No response generated" and the daemon log got an exit code, so a run + * that had the reason in hand reported none. codex, gemini and opencode all + * log their stderr on exit; this is that, plus the two paths — watchdog kill + * and silent exit — where it is the ONLY thing there is to report. + */ + _stderrTail(pp) { + const tail = String((pp && pp.stderrBuf) || '').trim(); + return tail ? redactSecrets(tail).slice(-800) : ''; + } + /** * Turn a raw Claude `result` error into a user-facing message. Auth failures * (401 / invalid token) are the most common real-world cause and are otherwise @@ -1291,10 +1317,15 @@ class ClaudeAdapter extends BaseAdapter { continue; } if (!pp.everPostedAnything) { - if (pp.lastErrorText) { - try { await this.sendError(msgChannel, this._formatClaudeError(pp.lastErrorText)); } catch {} + // stderr is the fallback, not nothing: a CLI that dies before its + // first JSON event leaves no `result` to read, and "No response + // generated. Please try again." is what turned those into an + // unreportable failure. + const detail = pp.lastErrorText || this._stderrTail(pp); + if (detail) { + try { await this.sendError(msgChannel, this._formatClaudeError(detail)); } catch {} } else { - try { await this.sendResponse(msgChannel, 'No response generated. Please try again.'); } catch {} + try { await this.sendResponse(msgChannel, `No response generated (exit code ${result.code === undefined ? 'unknown' : result.code}). Please try again.`); } catch {} } } break; diff --git a/packages/agent-connector/src/adapters/codex.js b/packages/agent-connector/src/adapters/codex.js index 033d1af6f..96df2ddf9 100644 --- a/packages/agent-connector/src/adapters/codex.js +++ b/packages/agent-connector/src/adapters/codex.js @@ -21,6 +21,7 @@ const https = require('https'); const { whereBinary } = require('../paths'); const BaseAdapter = require('./base'); +const { redactSecrets } = require('./utils'); const { buildOpenclawSystemPrompt } = require('./workspace-prompt'); const IS_WINDOWS = process.platform === 'win32'; @@ -535,18 +536,7 @@ class CodexAdapter extends BaseAdapter { /** Redact secrets (keys, tokens, bearer/authorization, query secrets) from diagnostics. */ static _redact(s) { - let out = String(s == null ? '' : s); - out = out - .replace(/\bsk-[A-Za-z0-9_-]{6,}/g, 'sk-[REDACTED]') - .replace(/\b(?:github_pat|gh[pousr])_[A-Za-z0-9_]{10,}/g, '[REDACTED_TOKEN]') - .replace(/\bxox[baprs]-[A-Za-z0-9-]{8,}/g, '[REDACTED_TOKEN]') - .replace(/\bAKIA[0-9A-Z]{12,}/g, '[REDACTED_KEY]') - .replace(/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, '[REDACTED_JWT]') - .replace(/(authorization|api[_-]?key|x-api-key|token|bearer|secret|password|passwd)(["'\s:=]+)([^\s"',}]+)/gi, - (m, k, sep) => `${k}${sep}[REDACTED]`) - .replace(/([?&](?:api[_-]?key|key|token|access_token)=)[^&\s"']+/gi, '$1[REDACTED]') - .replace(/\b[A-Za-z0-9_-]{40,}\b/g, '[REDACTED]'); - return out; + return redactSecrets(s); } // ------------------------------------------------------------------ diff --git a/packages/agent-connector/src/adapters/opencode.js b/packages/agent-connector/src/adapters/opencode.js index c3c9ddff8..d80360f05 100644 --- a/packages/agent-connector/src/adapters/opencode.js +++ b/packages/agent-connector/src/adapters/opencode.js @@ -17,7 +17,7 @@ const crypto = require('crypto'); const { execSync, spawn } = require('child_process'); const BaseAdapter = require('./base'); -const { formatAttachmentsForPrompt } = require('./utils'); +const { formatAttachmentsForPrompt, redactSecrets } = require('./utils'); const { buildOpenCodeSkillMd, buildOpenCodeSystemPrompt, workspaceSkillName } = require('./workspace-prompt'); const { whichBinary, whereBinary, getEnhancedEnv } = require('../paths'); @@ -1224,18 +1224,7 @@ class OpenCodeAdapter extends BaseAdapter { /** Redact secrets (keys, tokens, bearer/authorization, query secrets) from diagnostics. */ static _redact(s) { - let out = String(s == null ? '' : s); - out = out - .replace(/\bsk-[A-Za-z0-9_-]{6,}/g, 'sk-[REDACTED]') - .replace(/\b(?:github_pat|gh[pousr])_[A-Za-z0-9_]{10,}/g, '[REDACTED_TOKEN]') - .replace(/\bxox[baprs]-[A-Za-z0-9-]{8,}/g, '[REDACTED_TOKEN]') - .replace(/\bAKIA[0-9A-Z]{12,}/g, '[REDACTED_KEY]') - .replace(/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, '[REDACTED_JWT]') - .replace(/(authorization|api[_-]?key|x-api-key|token|bearer|secret|password|passwd)(["'\s:=]+)([^\s"',}]+)/gi, - (m, k, sep) => `${k}${sep}[REDACTED]`) - .replace(/([?&](?:api[_-]?key|key|token|access_token)=)[^&\s"']+/gi, '$1[REDACTED]') - .replace(/\b[A-Za-z0-9_-]{40,}\b/g, '[REDACTED]'); - return out; + return redactSecrets(s); } } diff --git a/packages/agent-connector/src/adapters/utils.js b/packages/agent-connector/src/adapters/utils.js index d657ea691..32c846231 100644 --- a/packages/agent-connector/src/adapters/utils.js +++ b/packages/agent-connector/src/adapters/utils.js @@ -120,8 +120,35 @@ function formatAttachmentsForPrompt( return lines.join('\n'); } +/** + * Strip secrets out of anything on its way to a log line or a channel message. + * + * Adapter diagnostics quote raw CLI output, and a CLI that fails on auth tends + * to echo the credential it was handed. The shapes here are the ones that + * actually turn up in that output; the closing catch-all takes any long opaque + * token the named patterns missed. + * + * Lived as a private static on two adapters before claude needed it as well — + * a third identical copy is one copy too many for a security-relevant rule. + */ +function redactSecrets(s) { + let out = String(s == null ? '' : s); + out = out + .replace(/\bsk-[A-Za-z0-9_-]{6,}/g, 'sk-[REDACTED]') + .replace(/\b(?:github_pat|gh[pousr])_[A-Za-z0-9_]{10,}/g, '[REDACTED_TOKEN]') + .replace(/\bxox[baprs]-[A-Za-z0-9-]{8,}/g, '[REDACTED_TOKEN]') + .replace(/\bAKIA[0-9A-Z]{12,}/g, '[REDACTED_KEY]') + .replace(/\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, '[REDACTED_JWT]') + .replace(/(authorization|api[_-]?key|x-api-key|token|bearer|secret|password|passwd)(["'\s:=]+)([^\s"',}]+)/gi, + (m, k, sep) => `${k}${sep}[REDACTED]`) + .replace(/([?&](?:api[_-]?key|key|token|access_token)=)[^&\s"']+/gi, '$1[REDACTED]') + .replace(/\b[A-Za-z0-9_-]{40,}\b/g, '[REDACTED]'); + return out; +} + module.exports = { SESSION_DEFAULT_RE, generateSessionTitle, formatAttachmentsForPrompt, + redactSecrets, }; diff --git a/packages/agent-connector/test/claude-stderr-surfacing.test.js b/packages/agent-connector/test/claude-stderr-surfacing.test.js new file mode 100644 index 000000000..660701911 --- /dev/null +++ b/packages/agent-connector/test/claude-stderr-surfacing.test.js @@ -0,0 +1,50 @@ +const { test, describe } = require('node:test'); +const assert = require('node:assert'); + +const ClaudeAdapter = require('../src/adapters/claude'); + +// `stderrBuf` used to be collected and never read: a CLI that died — or hung — +// before emitting its first JSON event left the only account of why in a string +// no code path looked at, and the channel got "No response generated". These +// pin the two properties that make it reportable: it comes back at all, and it +// comes back with credentials stripped, since a CLI failing on auth tends to +// echo the key it was handed. + +const tail = (stderrBuf) => + ClaudeAdapter.prototype._stderrTail.call(null, { stderrBuf }); + +describe('ClaudeAdapter._stderrTail', () => { + test('returns the process stderr so a silent failure has something to report', () => { + assert.strictEqual( + tail(' error: --input-format requires --output-format stream-json\n'), + 'error: --input-format requires --output-format stream-json', + ); + }); + + test('redacts credentials the CLI echoed back', () => { + const out = tail('Invalid API key: sk-Mzax7abcdef123456'); + assert.ok(!out.includes('sk-Mzax7abcdef123456'), out); + assert.match(out, /sk-\[REDACTED\]/); + }); + + test('reads as empty when the process said nothing, rather than as a blank error', () => { + assert.strictEqual(tail(''), ''); + assert.strictEqual(tail(' \n '), ''); + assert.strictEqual(tail(undefined), ''); + assert.strictEqual(ClaudeAdapter.prototype._stderrTail.call(null, null), ''); + }); + + test('keeps the END of a long stream — the failure is the last thing said', () => { + const noise = Array.from({ length: 200 }, (_, i) => `warn ${i}: deprecated flag`).join('\n'); + const out = tail(`${noise}\nfatal: could not connect to the model endpoint`); + assert.ok(out.endsWith('fatal: could not connect to the model endpoint'), out.slice(-60)); + assert.ok(out.length <= 800, String(out.length)); + }); + + // Redaction runs over the WHOLE buffer before the tail is cut: slicing first + // could split a key so the pattern no longer matches and a fragment survives. + test('redacts across the part that gets cut away, not just the tail', () => { + const out = tail(`Authorization: Bearer sk-Mzax7abcdef123456\n${'filler line\n'.repeat(200)}done`); + assert.ok(!out.includes('Mzax7abcdef'), out); + }); +}); diff --git a/packages/launcher/e2e/respond.spec.ts b/packages/launcher/e2e/respond.spec.ts index 0e2f8201f..2262c0567 100644 --- a/packages/launcher/e2e/respond.spec.ts +++ b/packages/launcher/e2e/respond.spec.ts @@ -24,6 +24,14 @@ const spec = agentBySlug(SLUG) const INSTALL_TIMEOUT = 15 * 60 * 1000 const START_TIMEOUT = 90_000 +// Longer than any adapter's own give-up timer, which is 300s across the board: +// claude's stdout watchdog (20 x 15s), codex's direct-LLM request timeout, +// opencode's TIMEOUT_MS, gemini's idle monitor. Polling for the same 300s was a +// dead heat — the adapter posts its diagnosis to the channel at the exact +// moment this stops listening, so every failure reported as the contentless +// "No agent reply within 300s" while the real reason was one second away. +// Waiting past it makes that message the assertion's own failure text. +const REPLY_TIMEOUT = 360_000 // Per-agent credentials (keys from E2E_* secrets; all via one gateway). claude // speaks Anthropic (own base); the rest are OpenAI-compatible on E2E_OPENAI_BASE. @@ -94,7 +102,7 @@ test.describe("launcher full flow", () => { // keys can't drive it, so its keyed flow stays install-smoke-only for now. test.skip(SLUG === "cursor", "cursor: needs a real cursor.com API key (no gateway support)") test.skip(!haveAgentKey(), `no provider API key for ${SLUG}`) - test.setTimeout(INSTALL_TIMEOUT + 12 * 60 * 1000) + test.setTimeout(INSTALL_TIMEOUT + 13 * 60 * 1000) const runId = process.env.GITHUB_RUN_ID || String(Date.now()) const osTag = @@ -371,7 +379,7 @@ test.describe("launcher full flow", () => { const cursor = await baselineCursor() await sendMessage(name, name, "What is 2+2? Reply with just the number.") try { - const reply = await pollForReply(name, name, cursor, 300_000) + const reply = await pollForReply(name, name, cursor, REPLY_TIMEOUT) expect(reply).toContain("4") } catch (e) { // Attach the daemon log/status so a non-reply is diagnosable (why the