From fc00aceef3be605d6a349afa62eff499d74a3a4a Mon Sep 17 00:00:00 2001 From: Altair Li <137127498+Altairpaca@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:32:12 +0800 Subject: [PATCH] fix(claude-sdk-oauth): reuse eval summary normalization --- .../session-commit-boundary.ts | 32 ++++++- ...laude-sdk-oauth-eval-summary-clamp.test.ts | 85 +++++++++++++++++++ packages/senpi-codemode/package.json | 4 + .../senpi-codemode/src/tool/eval-request.ts | 18 +--- .../src/tool/eval-summary.d.mts | 3 + .../senpi-codemode/src/tool/eval-summary.mjs | 11 +++ packages/senpi-codemode/src/tool/types.ts | 5 +- 7 files changed, 140 insertions(+), 18 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/1472-claude-sdk-oauth-eval-summary-clamp.test.ts create mode 100644 packages/senpi-codemode/src/tool/eval-summary.d.mts create mode 100644 packages/senpi-codemode/src/tool/eval-summary.mjs diff --git a/packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/session-commit-boundary.ts b/packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/session-commit-boundary.ts index 3c92fa2d44..3ef386301b 100644 --- a/packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/session-commit-boundary.ts +++ b/packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/session-commit-boundary.ts @@ -1,3 +1,4 @@ +import { clampEvalSummary } from "@code-yeongyu/senpi-codemode/eval-summary"; import type { AssistantMessage } from "@earendil-works/pi-ai"; import { CLAUDE_SDK_OAUTH_PROVIDER_ID } from "./account-management.ts"; import { sessionSyncDigest } from "./session-sync.ts"; @@ -6,13 +7,35 @@ export type AssistantCommitOutcome = "clean" | "rewritten" | "not-resident"; type ContentBlock = AssistantMessage["content"][number]; +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +/** + * The eval tool's prepareArguments shim normalizes run summaries before schema + * validation. That harness-owned normalization mutates the committed tool-call + * arguments after the provider-final boundary, so continuity must fingerprint + * the same effective arguments on both sides. Keep other tools fail-closed. + */ +function semanticToolCallArguments(name: string, args: unknown): unknown { + if (name !== "eval" || !isRecord(args) || args.action === "peek" || args.action === "stop") return args; + if (typeof args.summary !== "string") return args; + + const summary = clampEvalSummary(args.summary); + const canonical = { ...args }; + if (summary === undefined) delete canonical.summary; + else canonical.summary = summary; + return canonical; +} + /** * Only the payload the model produced is fingerprinted. Everything the stream * pipeline stamps around it (thinking timing, content-block indices, partial * JSON) can legitimately differ between the last `message_update` and `message_end` * without any extension rewriting the answer; hashing such fields marked plain * turns `assistant_rewritten` and forced a full re-send on the next turn - * (senpi#691, oh-my-openagent#7925). An unknown block shape stays fail-closed. + * (senpi#691, oh-my-openagent#7925). Harness-owned argument normalization is + * canonicalized to the effective tool input; unknown block shapes stay fail-closed. */ function semanticContentBlock(block: ContentBlock): unknown { switch (block.type) { @@ -21,7 +44,12 @@ function semanticContentBlock(block: ContentBlock): unknown { case "thinking": return { type: block.type, thinking: block.thinking, thinkingSignature: block.thinkingSignature }; case "toolCall": - return { type: block.type, id: block.id, name: block.name, arguments: block.arguments }; + return { + type: block.type, + id: block.id, + name: block.name, + arguments: semanticToolCallArguments(block.name, block.arguments), + }; default: return block; } diff --git a/packages/coding-agent/test/suite/regressions/1472-claude-sdk-oauth-eval-summary-clamp.test.ts b/packages/coding-agent/test/suite/regressions/1472-claude-sdk-oauth-eval-summary-clamp.test.ts new file mode 100644 index 0000000000..8580e96af6 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/1472-claude-sdk-oauth-eval-summary-clamp.test.ts @@ -0,0 +1,85 @@ +import type { AssistantMessage } from "@earendil-works/pi-ai"; +import { describe, expect, it } from "vitest"; +import { createEvalTool } from "../../../../senpi-codemode/src/tool/eval-tool.ts"; +import { CLAUDE_SDK_OAUTH_PROVIDER_ID } from "../../../src/core/extensions/builtin/claude-sdk-oauth/account-management.ts"; +import { AssistantCommitBoundary } from "../../../src/core/extensions/builtin/claude-sdk-oauth/session-commit-boundary.ts"; + +const MODEL_ID = "claude-opus-5"; + +const evalTool = createEvalTool({ + enabledLanguages: { js: true, py: false, rb: false, jl: false }, + kernelManager: { + getKernel: () => Promise.reject(new Error("unused in summary preparation regression")), + }, + cellTimeoutSeconds: 30, + executeTool: () => Promise.reject(new Error("unused in summary preparation regression")), +}); + +function preparedEvalSummary(summary: string): string { + const prepare = evalTool.prepareArguments; + if (prepare === undefined) throw new Error("eval tool must define prepareArguments"); + const prepared = prepare({ language: "js", code: "return 1", summary }); + if (typeof prepared !== "object" || prepared === null || !("summary" in prepared)) + throw new Error("prepared eval run must retain a summary"); + if (typeof prepared.summary !== "string") throw new Error("prepared eval summary must be a string"); + return prepared.summary; +} + +function evalAssistant(summary: string, code = "return 1"): AssistantMessage { + return { + role: "assistant", + api: CLAUDE_SDK_OAUTH_PROVIDER_ID, + provider: CLAUDE_SDK_OAUTH_PROVIDER_ID, + model: MODEL_ID, + content: [ + { + type: "toolCall", + id: "call-1", + name: "eval", + arguments: { language: "js", code, summary }, + }, + ], + stopReason: "toolUse", + timestamp: 1, + usage: { + input: 1, + output: 1, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 2, + cost: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + total: 0, + }, + }, + }; +} + +describe("issue #1472: eval summary normalization preserves Claude SDK continuity", () => { + it("treats the harness summary clamp as clean using eval prepareArguments", () => { + const streamedSummary = "s".repeat(81); + const streamed = evalAssistant(streamedSummary); + const committed = evalAssistant(preparedEvalSummary(streamedSummary)); + const boundary = new AssistantCommitBoundary(); + + boundary.captureProviderFinal("eval-clamp", streamed); + expect(boundary.commit("eval-clamp", committed, MODEL_ID)).toBe("clean"); + }); + + it("still detects a semantic summary rewrite within the schema limit", () => { + const boundary = new AssistantCommitBoundary(); + boundary.captureProviderFinal("summary-rewrite", evalAssistant("inspect cache state")); + + expect(boundary.commit("summary-rewrite", evalAssistant("delete cache state"), MODEL_ID)).toBe("rewritten"); + }); + + it("still detects changes to other eval arguments", () => { + const boundary = new AssistantCommitBoundary(); + boundary.captureProviderFinal("code-rewrite", evalAssistant("inspect result", "return 1")); + + expect(boundary.commit("code-rewrite", evalAssistant("inspect result", "return 2"), MODEL_ID)).toBe("rewritten"); + }); +}); diff --git a/packages/senpi-codemode/package.json b/packages/senpi-codemode/package.json index f10db8c53d..35bdbee999 100644 --- a/packages/senpi-codemode/package.json +++ b/packages/senpi-codemode/package.json @@ -8,6 +8,10 @@ "types": "./src/index.ts", "exports": { ".": "./src/index.ts", + "./eval-summary": { + "types": "./src/tool/eval-summary.d.mts", + "import": "./src/tool/eval-summary.mjs" + }, "./package.json": "./package.json" }, "files": [ diff --git a/packages/senpi-codemode/src/tool/eval-request.ts b/packages/senpi-codemode/src/tool/eval-request.ts index f47e43329d..b9929ba7cf 100644 --- a/packages/senpi-codemode/src/tool/eval-request.ts +++ b/packages/senpi-codemode/src/tool/eval-request.ts @@ -1,20 +1,10 @@ import type { ExtensionContext } from "@code-yeongyu/senpi"; -import { EVAL_SUMMARY_MAX_LENGTH, type EvalControlInput, type EvalToolInput, type EvalToolRequest } from "./types.ts"; +import { clampEvalSummary } from "./eval-summary.mjs"; +import { type EvalControlInput, type EvalToolInput, type EvalToolRequest } from "./types.ts"; -const NON_INTERACTIVE_MODES = new Set(["print", "json"]); - -const ELLIPSIS = "..."; +export { clampEvalSummary }; -// Harness-side enforcement of the schema maxLength: the tool advertises the limit, but an -// over-limit value is force-truncated here (prepareArguments runs before schema validation) -// instead of failing the call. -export function clampEvalSummary(value: unknown): string | undefined { - if (typeof value !== "string") return undefined; - const normalized = value.trim().replace(/\s+/gu, " "); - if (normalized.length === 0) return undefined; - if (normalized.length <= EVAL_SUMMARY_MAX_LENGTH) return normalized; - return `${normalized.slice(0, EVAL_SUMMARY_MAX_LENGTH - ELLIPSIS.length)}${ELLIPSIS}`; -} +const NON_INTERACTIVE_MODES = new Set(["print", "json"]); export function parseEvalRequest(params: unknown): EvalToolRequest { if (!isRecord(params)) throw new TypeError("eval parameters must be an object"); diff --git a/packages/senpi-codemode/src/tool/eval-summary.d.mts b/packages/senpi-codemode/src/tool/eval-summary.d.mts new file mode 100644 index 0000000000..085280c26b --- /dev/null +++ b/packages/senpi-codemode/src/tool/eval-summary.d.mts @@ -0,0 +1,3 @@ +export declare const EVAL_SUMMARY_MAX_LENGTH = 80; + +export declare function clampEvalSummary(value: unknown): string | undefined; diff --git a/packages/senpi-codemode/src/tool/eval-summary.mjs b/packages/senpi-codemode/src/tool/eval-summary.mjs new file mode 100644 index 0000000000..9b4b59438b --- /dev/null +++ b/packages/senpi-codemode/src/tool/eval-summary.mjs @@ -0,0 +1,11 @@ +export const EVAL_SUMMARY_MAX_LENGTH = 80; + +const ELLIPSIS = "..."; + +export function clampEvalSummary(value) { + if (typeof value !== "string") return undefined; + const normalized = value.trim().replace(/\s+/gu, " "); + if (normalized.length === 0) return undefined; + if (normalized.length <= EVAL_SUMMARY_MAX_LENGTH) return normalized; + return `${normalized.slice(0, EVAL_SUMMARY_MAX_LENGTH - ELLIPSIS.length)}${ELLIPSIS}`; +} diff --git a/packages/senpi-codemode/src/tool/types.ts b/packages/senpi-codemode/src/tool/types.ts index 2d900e9979..6c06b6cf12 100644 --- a/packages/senpi-codemode/src/tool/types.ts +++ b/packages/senpi-codemode/src/tool/types.ts @@ -8,6 +8,9 @@ import { defaultCodemodeSettings, } from "../config/settings.ts"; import type { TruncationMeta } from "../output/output-meta.ts"; +import { EVAL_SUMMARY_MAX_LENGTH } from "./eval-summary.mjs"; + +export { EVAL_SUMMARY_MAX_LENGTH }; export const evalLanguageOrder = ["js", "py", "rb", "jl"] as const; export type EvalLanguage = (typeof evalLanguageOrder)[number]; @@ -17,8 +20,6 @@ export function enabledLanguageList(enabled: EnabledEvalLanguages): EvalLanguage return evalLanguageOrder.filter((language) => enabled[language]); } -export const EVAL_SUMMARY_MAX_LENGTH = 80; - /** The deadlines the schema teaches the model; every number comes from the resolved settings. */ export interface EvalDeadlineSeconds { readonly runBudgetSeconds: number;