diff --git a/CHANGELOG.md b/CHANGELOG.md index 798a0cfc..adc448a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to this project will be documented in this file. ### Fixed +- **runtime-node**: `formatRunError` no longer rewrites any error containing "not found" or "404" into an LLM provider misconfiguration. It classified by substring, so `Cannot apply patch: file not found in context: src/hooks/useDebounce.ts` — a message from the `apply_patch` skill, with no LLM call involved — was reported as a 404 against a `provider`/`model`/`baseURL` triple read from config, making a fabricated story look well-evidenced. Real LLM request failures are now tagged at the source in `llm-service` (in the message text, since errors are flattened with `error.message` before reaching any formatter) and `formatRunError` switches on that tag. The original message is preserved alongside the hint in every branch rather than replaced. (#408) - **executor**: Write steps are validated *before* the content reaches disk, and a failed write is rolled back. Previously `validateAfterExecution` ran only after the tool had written, and rollback was gated on `step.validation.some(v => v.required)` — a field LLM-generated plans leave empty — so a bad file simply stayed on disk. The pre-write veto is deliberately narrow: only the deterministic markdown-fence criterion can stop a write, because the guard's `syntax_validity` check is a per-line quote-parity heuristic that blocks `"it's fine"`, multi-line template literals and JSX apostrophes (#413). `apply_patch` is covered too — codegen emits whole-file replaces, whose final content is fully known before the write. Rollback failure is reported instead of leaving callers at "rollback started", and `validation_failed` now carries a `stage` (`pre_execution` / `pre_write` / `post_write`) so interception can actually be counted (#388). On `apply_patch` — and only there — `syntax_validity` and `import_validity` are recorded but do not decide step outcome. That is not a loss of interception: before this change the patch path never reached `validateCode` at all, so those verdicts never constrained it; making the content validatable and simultaneously giving them veto power would fail legitimate modify steps over `@/alias` imports and modules a later step creates. `create_file` keeps blocking on both, unchanged. Demoted verdicts stay in `validation.results` and still emit `validation_failed`, so demotion does not also switch off the telemetry — consumers must read `result.pass` to tell a block from a record. (#387, #388, #413) - **guard**: `hallucinationGuard.enabled: false` now disables every `HallucinationGuard` check on the agent path while preserving project-root containment. It does not reach the executor's own filesystem-facts grounding, which blocks an `apply_patch` against a known-nonexistent path regardless of guard config — so an ablation arm that sets this flag is not running with *all* interception off. Disabling `fileExistence` alone now also keeps the containment result on `validate()`, where that path previously produced none. (#400) - **tooling**: `pnpm lint` works again from inside a worktree under `.claude/worktrees/`. The exclusion added for #439 was `**/`-prefixed, and Biome matches the traversal root by absolute path — so when Biome ran *from* such a worktree the root matched its own exclusion and `biome check .` reported "Checked 0 files" and exited non-zero, taking the pre-commit and pre-push hooks with it. The pattern is now anchored to the project root, which still keeps Biome out of a nested checkout at the root (the #439 failure). `.gitignore` deliberately keeps `**/`: git has no equivalent reverse failure, so it can afford the wider match. The two are therefore not equivalent protection — a worktree under a subdirectory's `.claude/` is git-ignored but *not* excluded from Biome, so a root `biome check .` still hits the #439 abort there; move such a worktree to the repo-root `.claude/worktrees/`. Both directions of the root case are pinned by a test that runs the real binary against a synthetic project root. (#444) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 578ba43e..294ed6e9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -30,7 +30,10 @@ export { type GeneratedCode, type GeneratedPatch, type GeneratedPlan, + isLLMRequestFailure, LLMService, + stripLLMRequestTag, + tagLLMRequestFailure, } from './llm/index.js'; export type { MemoryConfig, diff --git a/packages/core/src/llm/index.ts b/packages/core/src/llm/index.ts index 090da76c..3660eeb4 100644 --- a/packages/core/src/llm/index.ts +++ b/packages/core/src/llm/index.ts @@ -1,3 +1,8 @@ export { createLLMService } from './factory.js'; +export { + isLLMRequestFailure, + stripLLMRequestTag, + tagLLMRequestFailure, +} from './llm-request-error.js'; export { LLMService, normalizeProviderBaseURL } from './llm-service.js'; export type { GeneratedCode, GeneratedPatch, GeneratedPlan } from './schemas.js'; diff --git a/packages/core/src/llm/llm-request-error.ts b/packages/core/src/llm/llm-request-error.ts new file mode 100644 index 00000000..5afd0fc8 --- /dev/null +++ b/packages/core/src/llm/llm-request-error.ts @@ -0,0 +1,40 @@ +/** + * 给「LLM 请求真的失败了」这件事打一个可机读的标记。 + * + * 为什么标记在**消息文本**里，而不是用 Error 子类：错误在到达任何格式化器之前 + * 就已经被 `error instanceof Error ? error.message : String(error)` 拍平成字符串 + * （`agent.ts:572` 等四处），子类身份到不了下游。消息是唯一能穿过那道边界的通道。 + * + * 这条标记存在的原因是 #408：`formatRunError` 曾用 `/not found|404/` 去猜错误来源， + * 于是 `Cannot apply patch: file not found in context: src/hooks/useDebounce.ts` + * 被改写成一次「404 Not Found」的 LLM 供应商配置错误，还附上从配置里取来的 + * provider / model / baseURL——一次没发生过的失败，被伪造成了证据充分的样子。 + * 判据必须是来源，不是子串。 + */ +const LLM_REQUEST_FAILED_TAG = '[llm-request-failed]'; + +/** 给一次真实的 LLM 请求失败打标；已带标记的原样返回，避免重复包裹。 */ +export function tagLLMRequestFailure(error: unknown): unknown { + const message = error instanceof Error ? error.message : String(error); + if (message.startsWith(LLM_REQUEST_FAILED_TAG)) return error; + + const tagged = new Error(`${LLM_REQUEST_FAILED_TAG} ${message}`); + // 保留原始堆栈：包裹是为了标注来源，不是为了换一个新的失败点。 + if (error instanceof Error) { + tagged.stack = error.stack; + tagged.cause = error; + } + return tagged; +} + +/** 这条错误是否来自一次真实的 LLM 请求。 */ +export function isLLMRequestFailure(message: string): boolean { + return message.startsWith(LLM_REQUEST_FAILED_TAG); +} + +/** 去掉标记，还原给人看的原文。 */ +export function stripLLMRequestTag(message: string): string { + return isLLMRequestFailure(message) + ? message.slice(LLM_REQUEST_FAILED_TAG.length).trimStart() + : message; +} diff --git a/packages/core/src/llm/llm-service.ts b/packages/core/src/llm/llm-service.ts index d16e6783..418411ff 100644 --- a/packages/core/src/llm/llm-service.ts +++ b/packages/core/src/llm/llm-service.ts @@ -16,6 +16,7 @@ import { generateCodeForFile as generateCodeForFileImpl, generateModifiedCode as generateModifiedCodeImpl, } from './code-generation.js'; +import { tagLLMRequestFailure } from './llm-request-error.js'; import { tryFixGeneratedObject } from './object-repair.js'; import { generatePlan as generatePlanImpl, @@ -192,14 +193,18 @@ export class LLMService { if (!this.model) { throw new Error('No LLM model is configured'); } - const result = await generateText({ - model: this.model, - messages: this.convertMessages(options.messages), - system: options.system, - ...this.buildCallSettings(options), - }); - - return result.text; + try { + const result = await generateText({ + model: this.model, + messages: this.convertMessages(options.messages), + system: options.system, + ...this.buildCallSettings(options), + }); + + return result.text; + } catch (error) { + throw tagLLMRequestFailure(error); + } } async *streamText(options: { @@ -221,15 +226,19 @@ export class LLMService { if (!this.model) { throw new Error('No LLM model is configured'); } - const result = streamText({ - model: this.model, - messages: this.convertMessages(options.messages), - system: options.system, - ...this.buildCallSettings(options), - }); - - for await (const chunk of result.textStream) { - yield chunk; + try { + const result = streamText({ + model: this.model, + messages: this.convertMessages(options.messages), + system: options.system, + ...this.buildCallSettings(options), + }); + + for await (const chunk of result.textStream) { + yield chunk; + } + } catch (error) { + throw tagLLMRequestFailure(error); } } @@ -320,7 +329,7 @@ export class LLMService { LLMService.errorStats.unfixedErrors++; this.debugError('[LLMService] ❌ All fix attempts and retries failed'); this.debugLog('[LLMService] Error Stats:', LLMService.getErrorStats()); - throw error; + throw tagLLMRequestFailure(error); } } diff --git a/packages/runtime-node/src/run-error-format.test.ts b/packages/runtime-node/src/run-error-format.test.ts new file mode 100644 index 00000000..80b15f17 --- /dev/null +++ b/packages/runtime-node/src/run-error-format.test.ts @@ -0,0 +1,86 @@ +import { tagLLMRequestFailure } from '@frontagent/core'; +import { describe, expect, it } from 'vitest'; +import { formatRunError } from './run.js'; + +const input = { + provider: 'anthropic', + model: 'claude-3-5-sonnet-20241022', + baseURL: undefined, + debug: false, +}; + +/** 一次真实 LLM 请求失败在到达这里时的形态：标记由 llm-service 打上，消息已被拍平。 */ +function fromLLM(message: string): string { + const tagged = tagLLMRequestFailure(new Error(message)); + return tagged instanceof Error ? tagged.message : String(tagged); +} + +describe('formatRunError classifies by provenance, not by substring', () => { + // #408 的原始复现。这条错误来自 apply_patch 技能自己 + // （executor-skills.ts: `Cannot apply patch: file not found in context: ${filePath}`）， + // 没有任何 LLM 调用失败——而它曾被改写成一次 404 供应商配置错误，还附上 + // 从配置里读来的 provider / model / baseURL。伪造的三元组让错误的故事看起来证据充分。 + it('leaves a tool error that merely contains "not found" alone', () => { + const error = 'Cannot apply patch: file not found in context: src/hooks/useDebounce.ts'; + + const formatted = formatRunError(error, input); + + expect(formatted).toBe(error); + expect(formatted).not.toContain('404'); + expect(formatted).not.toContain('claude-3-5-sonnet-20241022'); + expect(formatted).not.toContain('provider='); + }); + + it.each([ + ['Command failed: vitest run — 1 test not found', 'a failing test command'], + ['ENOENT: no such file or directory, open "src/a.ts"', 'a filesystem error'], + ['File src/a.ts does not exist (404 in the docs)', 'prose that mentions 404'], + ])('leaves %s alone (%s)', (error) => { + expect(formatRunError(error, input)).toBe(error); + }); + + it('formats a real LLM 404 and keeps the original message', () => { + const formatted = formatRunError(fromLLM('404 Not Found: model does not exist'), input); + + expect(formatted).toContain('404 Not Found'); + expect(formatted).toContain('provider=anthropic'); + // 原文必须保留：它是唯一准确的那部分，此前被整条替换掉 + expect(formatted).toContain('model does not exist'); + expect(formatted).toContain('Anthropic Messages API'); + }); + + it('formats a real LLM auth failure and keeps the original message', () => { + const formatted = formatRunError(fromLLM('401 Unauthorized: invalid api key'), input); + + expect(formatted).toContain('ANTHROPIC_API_KEY'); + expect(formatted).toContain('invalid api key'); + }); + + // 打了标记但不属于已知的两类：仍然要说清是 LLM 失败，而不是默默退回首行—— + // 否则「来源已知」这条信息就白拿了。 + it('still reports an unclassified LLM failure as an LLM failure', () => { + const formatted = formatRunError(fromLLM('socket hang up'), input); + + expect(formatted).toContain('LLM 请求失败'); + expect(formatted).toContain('socket hang up'); + }); + + it('never leaks the machine tag into user-facing text', () => { + for (const error of [fromLLM('404 Not Found'), fromLLM('401'), fromLLM('boom')]) { + expect(formatRunError(error, input)).not.toContain('[llm-request-failed]'); + } + }); + + it('passes everything through untouched in debug mode', () => { + const error = fromLLM('404 Not Found'); + + expect(formatRunError(error, { ...input, debug: true })).toBe(error); + }); + + it('does not double-tag an already-tagged failure', () => { + const once = fromLLM('404 Not Found'); + const twice = tagLLMRequestFailure(new Error(once)); + + expect(twice instanceof Error ? twice.message : String(twice)).toBe(once); + }); +}); diff --git a/packages/runtime-node/src/run.test.ts b/packages/runtime-node/src/run.test.ts index 199720be..84a7d1f5 100644 --- a/packages/runtime-node/src/run.test.ts +++ b/packages/runtime-node/src/run.test.ts @@ -68,7 +68,12 @@ const createAgent = vi.fn((config: AgentConfig) => { }; }); -vi.mock('@frontagent/core', () => ({ +// createAgent is the seam this file needs faked; the LLM-failure tag helpers are +// pure string functions with no collaborators, so they come through from the real +// module — mocking them would make `formatRunError`'s provenance check untestable +// here for no benefit. +vi.mock('@frontagent/core', async (importOriginal) => ({ + ...(await importOriginal()), createAgent: (config: AgentConfig) => createAgent(config), })); diff --git a/packages/runtime-node/src/run.ts b/packages/runtime-node/src/run.ts index baf166a6..f3239d71 100644 --- a/packages/runtime-node/src/run.ts +++ b/packages/runtime-node/src/run.ts @@ -9,7 +9,9 @@ import { createAgent, type ExecutorStepTrace, type HallucinationGuardConfig, + isLLMRequestFailure, type LLMBackend, + stripLLMRequestTag, } from '@frontagent/core'; import { createShellMCPClient } from '@frontagent/mcp-shell'; import type { ApprovalRequest, SecurityApprovalResponse, TaskType } from '@frontagent/shared'; @@ -87,21 +89,35 @@ export function formatRunError( ): string | undefined { if (!error || input.debug) return error; - if (/not found|404/i.test(error)) { + // 按**来源**分类，不按子串。此前这里用 `/not found|404/` 猜，于是 + // `Cannot apply patch: file not found in context: src/hooks/useDebounce.ts` + // 被改写成一次 404 供应商配置错误，还附上从配置里读来的 provider / model / + // baseURL——一次没发生过的 LLM 失败，被伪造得证据充分（#408）。标记由 + // llm-service 在真实请求失败处打上，是唯一能穿过 `error.message` 拍平的通道。 + if (!isLLMRequestFailure(error)) { + return error.split('\n')[0]; + } + + const original = stripLLMRequestTag(error).split('\n')[0]; + const endpoint = `provider=${input.provider}, model=${input.model}, baseURL=${input.baseURL ?? '(default)'}`; + + // 原文一律保留。它是唯一准确的那部分信息，此前被整条替换掉——而 debug 模式 + // 早就把它原样打出来了，说明它一直存在，只是在默认路径上被丢了。 + if (/not found|404/i.test(original)) { return [ - 'LLM 请求失败：404 Not Found。', - `请检查 provider/model/base-url：provider=${input.provider}, model=${input.model}, baseURL=${input.baseURL ?? '(default)'}`, + `LLM 请求失败：404 Not Found（${original}）`, + `请检查 provider/model/base-url：${endpoint}`, input.provider === 'anthropic' ? 'Anthropic provider 会请求 baseURL + /messages；请确认供应商支持 Anthropic Messages API。' : '如 baseURL 包含 /chat/completions，CLI 会自动裁剪；仍失败时请确认供应商的 OpenAI-compatible 地址。', ].join('\n'); } - if (/api key|apikey|unauthorized|401/i.test(error)) { - return `LLM 鉴权失败。请检查 ${input.provider.toUpperCase()}_API_KEY 或 --api-key。`; + if (/api key|apikey|unauthorized|401/i.test(original)) { + return `LLM 鉴权失败（${original}）。请检查 ${input.provider.toUpperCase()}_API_KEY 或 --api-key。`; } - return error.split('\n')[0]; + return `LLM 请求失败：${original}`; } export async function runFrontAgentTask(