Merge pull request #4133 from code-yeongyu/fix/3816-frozen-output-args
fix: replace direct output.args mutation with replaceToolArgs helper (fixes #3816)
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { log } from "../../shared/logger"
|
||||
import { replaceToolArgs } from "../../shared/replace-tool-args"
|
||||
import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive"
|
||||
import { isCallerOrchestrator } from "../../shared/session-utils"
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
@@ -179,7 +180,7 @@ export function createToolExecuteBeforeHandler(input: {
|
||||
|
||||
const prompt = toolOutput.args.prompt as string | undefined
|
||||
if (prompt && !prompt.includes(SYSTEM_DIRECTIVE_PREFIX)) {
|
||||
toolOutput.args.prompt = `<system-reminder>${SINGLE_TASK_DIRECTIVE}</system-reminder>\n` + prompt
|
||||
replaceToolArgs(toolOutput, { prompt: `<system-reminder>${SINGLE_TASK_DIRECTIVE}</system-reminder>\n` + prompt })
|
||||
log(`[${HOOK_NAME}] Injected single-task directive to task`, {
|
||||
sessionID: toolInput.sessionID,
|
||||
})
|
||||
|
||||
@@ -8,7 +8,7 @@ import {
|
||||
import { appendTranscriptEntry } from "../transcript"
|
||||
import { cacheToolInput } from "../tool-input-cache"
|
||||
import type { PluginConfig } from "../types"
|
||||
import { isHookDisabled, log } from "../../../shared"
|
||||
import { isHookDisabled, log, replaceToolArgs } from "../../../shared"
|
||||
|
||||
export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginConfig) {
|
||||
return async (
|
||||
@@ -39,7 +39,7 @@ export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginC
|
||||
)
|
||||
}
|
||||
|
||||
output.args.todos = parsed
|
||||
replaceToolArgs(output, { todos: parsed })
|
||||
log("todowrite: parsed todos string to array", { sessionID: input.sessionID })
|
||||
}
|
||||
|
||||
@@ -87,7 +87,7 @@ export function createToolExecuteBeforeHandler(ctx: PluginInput, config: PluginC
|
||||
}
|
||||
|
||||
if (result.modifiedInput) {
|
||||
Object.assign(output.args, result.modifiedInput)
|
||||
replaceToolArgs(output, result.modifiedInput as Record<string, unknown>)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { resolveSessionEventID } from "../../shared/event-session-id"
|
||||
import { log } from "../../shared/logger"
|
||||
import { replaceToolArgs } from "../../shared/replace-tool-args"
|
||||
|
||||
interface TodoSnapshot {
|
||||
id?: string
|
||||
@@ -233,7 +234,7 @@ export function createCompactionTodoPreserverHook(
|
||||
return
|
||||
}
|
||||
|
||||
output.args.todos = snapshot
|
||||
replaceToolArgs(output, { todos: snapshot })
|
||||
log(`[${HOOK_NAME}] Replaced late Atlas bootstrap todowrite with restored snapshot`, {
|
||||
sessionID: input.sessionID,
|
||||
count: snapshot.length,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { HOOK_NAME, NON_INTERACTIVE_ENV, SHELL_COMMAND_PATTERNS } from "./constants"
|
||||
import { log, buildEnvPrefix } from "../../shared"
|
||||
import { log, buildEnvPrefix, replaceToolArgs } from "../../shared"
|
||||
import { detectShellType, type ShellType } from "../../shared/shell-env"
|
||||
|
||||
export * from "./constants"
|
||||
@@ -97,7 +97,7 @@ export function createNonInteractiveEnvHook(_ctx: PluginInput) {
|
||||
return
|
||||
}
|
||||
|
||||
output.args.command = `${envPrefix} ${command}`
|
||||
replaceToolArgs(output, { command: `${envPrefix} ${command}` })
|
||||
|
||||
log(`[${HOOK_NAME}] Prepended non-interactive env vars to git command`, {
|
||||
sessionID: input.sessionID,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { HOOK_NAME, BLOCKED_TOOLS, PLANNING_CONSULT_WARNING, PROMETHEUS_WORKFLOW_REMINDER } from "./constants"
|
||||
import { log } from "../../shared/logger"
|
||||
import { replaceToolArgs } from "../../shared/replace-tool-args"
|
||||
import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive"
|
||||
import { getAgentDisplayName } from "../../shared/agent-display-names"
|
||||
import { getAgentFromSession } from "./agent-resolution"
|
||||
@@ -27,7 +28,7 @@ export function createPrometheusMdOnlyHook(ctx: PluginInput) {
|
||||
if (TASK_TOOLS.includes(toolName)) {
|
||||
const prompt = output.args.prompt as string | undefined
|
||||
if (prompt && !prompt.includes(SYSTEM_DIRECTIVE_PREFIX)) {
|
||||
output.args.prompt = PLANNING_CONSULT_WARNING + prompt
|
||||
replaceToolArgs(output, { prompt: PLANNING_CONSULT_WARNING + prompt })
|
||||
log(`[${HOOK_NAME}] Injected planning warning to ${toolName}`, {
|
||||
sessionID: input.sessionID,
|
||||
tool: toolName,
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { replaceToolArgs } from "../../shared/replace-tool-args"
|
||||
|
||||
const MAX_LABEL_LENGTH = 30;
|
||||
|
||||
interface QuestionOption {
|
||||
@@ -56,7 +58,7 @@ export function createQuestionLabelTruncatorHook() {
|
||||
if (toolName === "askuserquestion" || toolName === "ask_user_question") {
|
||||
if (hasQuestions(output.args)) {
|
||||
const truncatedArgs = truncateQuestionLabels(output.args);
|
||||
Object.assign(output.args, truncatedArgs);
|
||||
replaceToolArgs(output, { questions: truncatedArgs.questions });
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { isCallerOrchestrator } from "../../shared/session-utils"
|
||||
import { SYSTEM_DIRECTIVE_PREFIX } from "../../shared/system-directive"
|
||||
import { log } from "../../shared/logger"
|
||||
import { replaceToolArgs } from "../../shared/replace-tool-args"
|
||||
import { HOOK_NAME, NOTEPAD_DIRECTIVE } from "./constants"
|
||||
|
||||
export function createSisyphusJuniorNotepadHook(ctx: PluginInput) {
|
||||
@@ -33,7 +34,7 @@ export function createSisyphusJuniorNotepadHook(ctx: PluginInput) {
|
||||
}
|
||||
|
||||
// 5. Prepend directive
|
||||
output.args.prompt = NOTEPAD_DIRECTIVE + prompt
|
||||
replaceToolArgs(output, { prompt: NOTEPAD_DIRECTIVE + prompt })
|
||||
|
||||
// 6. Log injection
|
||||
log(`[${HOOK_NAME}] Injected notepad directive to task`, {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import { log } from "../../shared"
|
||||
import { log, replaceToolArgs } from "../../shared"
|
||||
import {
|
||||
MAX_WEBFETCH_REDIRECTS,
|
||||
WEBFETCH_REDIRECT_ERROR_PATTERNS,
|
||||
@@ -85,7 +85,7 @@ export function createWebFetchRedirectGuardHook(_ctx: PluginInput) {
|
||||
})
|
||||
|
||||
if (resolution.type === "resolved") {
|
||||
output.args.url = resolution.url
|
||||
replaceToolArgs(output, { url: resolution.url })
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"
|
||||
|
||||
import { getMainSessionID } from "../features/claude-code-session-state"
|
||||
import { clearBoulderState } from "../features/boulder-state"
|
||||
import { log } from "../shared"
|
||||
import { log, replaceToolArgs } from "../shared"
|
||||
import { stripInvisibleAgentCharacters } from "../shared/agent-display-names"
|
||||
import { resolveSessionAgent } from "./session-agent-resolver"
|
||||
import { parseRalphLoopArguments } from "../hooks/ralph-loop/command-arguments"
|
||||
@@ -54,7 +54,7 @@ export function createToolExecuteBeforeHandler(args: {
|
||||
return async (input, output): Promise<void> => {
|
||||
if (input.tool.toLowerCase() === "bash" && typeof output.args.command === "string") {
|
||||
if (output.args.command.includes("\x00")) {
|
||||
output.args.command = output.args.command.replace(/\x00/g, "")
|
||||
replaceToolArgs(output, { command: output.args.command.replace(/\x00/g, "") })
|
||||
log("[tool-execute-before] Stripped null bytes from bash command", {
|
||||
sessionID: input.sessionID,
|
||||
callID: input.callID,
|
||||
@@ -100,21 +100,20 @@ export function createToolExecuteBeforeHandler(args: {
|
||||
}
|
||||
|
||||
if (input.tool === "task") {
|
||||
const argsObject = output.args
|
||||
const category = typeof argsObject.category === "string" ? argsObject.category : undefined
|
||||
const subagentType = typeof argsObject.subagent_type === "string" ? argsObject.subagent_type : undefined
|
||||
const taskId = typeof argsObject.task_id === "string" ? argsObject.task_id : undefined
|
||||
const category = typeof output.args.category === "string" ? output.args.category : undefined
|
||||
const subagentType = typeof output.args.subagent_type === "string" ? output.args.subagent_type : undefined
|
||||
const taskId = typeof output.args.task_id === "string" ? output.args.task_id : undefined
|
||||
|
||||
if (category) {
|
||||
argsObject.subagent_type = "sisyphus-junior"
|
||||
replaceToolArgs(output, { subagent_type: "sisyphus-junior" })
|
||||
} else if (!subagentType && taskId) {
|
||||
const resolvedAgent = await resolveSessionAgent(ctx.client, taskId)
|
||||
argsObject.subagent_type = resolvedAgent ?? "continue"
|
||||
replaceToolArgs(output, { subagent_type: resolvedAgent ?? "continue" })
|
||||
}
|
||||
|
||||
const normalizedSubagentType =
|
||||
typeof argsObject.subagent_type === "string" ? stripInvisibleAgentCharacters(argsObject.subagent_type) : undefined
|
||||
const prompt = typeof argsObject.prompt === "string" ? argsObject.prompt : ""
|
||||
typeof output.args.subagent_type === "string" ? stripInvisibleAgentCharacters(output.args.subagent_type) : undefined
|
||||
const prompt = typeof output.args.prompt === "string" ? output.args.prompt : ""
|
||||
const loopState = typeof ctx.directory === "string" ? readState(ctx.directory) : null
|
||||
const shouldInjectOracleVerification =
|
||||
normalizedSubagentType === "oracle"
|
||||
@@ -136,12 +135,14 @@ export function createToolExecuteBeforeHandler(args: {
|
||||
verification_attempt_id: verificationAttemptId,
|
||||
verification_session_id: undefined,
|
||||
})
|
||||
argsObject.run_in_background = false
|
||||
argsObject.prompt = buildUltraworkOracleVerificationPrompt(
|
||||
prompt,
|
||||
loopState.prompt,
|
||||
verificationAttemptId,
|
||||
)
|
||||
replaceToolArgs(output, {
|
||||
run_in_background: false,
|
||||
prompt: buildUltraworkOracleVerificationPrompt(
|
||||
prompt,
|
||||
loopState.prompt,
|
||||
verificationAttemptId,
|
||||
),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -84,3 +84,4 @@ export * from "./task-system-enabled"
|
||||
export * from "./parse-tools-config"
|
||||
export { parseModelString } from "./model-string-parser"
|
||||
export { EXCLUDED_DIRS } from "./excluded-dirs"
|
||||
export * from "./replace-tool-args"
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { describe, it, expect } from "bun:test"
|
||||
import { readdir, readFile } from "node:fs/promises"
|
||||
import { join, relative } from "node:path"
|
||||
|
||||
const SRC_DIR = join(import.meta.dir, "..")
|
||||
|
||||
async function collectTsFiles(dir: string): Promise<string[]> {
|
||||
const results: string[] = []
|
||||
const entries = await readdir(dir, { withFileTypes: true })
|
||||
|
||||
for (const entry of entries) {
|
||||
const fullPath = join(dir, entry.name)
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === "node_modules" || entry.name === "dist") continue
|
||||
results.push(...(await collectTsFiles(fullPath)))
|
||||
} else if (entry.name.endsWith(".ts") && !entry.name.endsWith(".test.ts") && !entry.name.endsWith(".audit.test.ts")) {
|
||||
results.push(fullPath)
|
||||
}
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
|
||||
const HELPER_FILE = "shared/replace-tool-args.ts"
|
||||
|
||||
// Matches direct mutations like `output.args.foo =` or `toolOutput.args.foo =`
|
||||
// but excludes comparisons (===, !==, ==)
|
||||
const DIRECT_MUTATION_PATTERN = /\w*[Oo]utput\.args\.\w+\s*=[^=]/g
|
||||
const OBJECT_ASSIGN_PATTERN = /Object\.assign\(\s*\w*[Oo]utput\.args/g
|
||||
|
||||
describe("replace-tool-args audit", () => {
|
||||
it("#given src/**/*.ts files #when scanning for direct output.args mutation #then no matches found outside the helper", async () => {
|
||||
// given
|
||||
const files = await collectTsFiles(SRC_DIR)
|
||||
const violations: string[] = []
|
||||
|
||||
// when
|
||||
for (const file of files) {
|
||||
const relPath = relative(SRC_DIR, file)
|
||||
if (relPath === HELPER_FILE) continue
|
||||
|
||||
const content = await readFile(file, "utf-8")
|
||||
const lines = content.split("\n")
|
||||
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i]
|
||||
if (DIRECT_MUTATION_PATTERN.test(line)) {
|
||||
violations.push(`${relPath}:${i + 1}: ${line.trim()}`)
|
||||
}
|
||||
DIRECT_MUTATION_PATTERN.lastIndex = 0
|
||||
|
||||
if (OBJECT_ASSIGN_PATTERN.test(line)) {
|
||||
violations.push(`${relPath}:${i + 1}: ${line.trim()}`)
|
||||
}
|
||||
OBJECT_ASSIGN_PATTERN.lastIndex = 0
|
||||
}
|
||||
}
|
||||
|
||||
// then
|
||||
expect(violations).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,148 @@
|
||||
import { describe, it, expect } from "bun:test"
|
||||
import { replaceToolArgs } from "./replace-tool-args"
|
||||
|
||||
describe("replaceToolArgs", () => {
|
||||
describe("#given a mutable output.args object", () => {
|
||||
it("#when patching a single property #then the output.args contains the patched value", () => {
|
||||
// given
|
||||
const output = { args: { command: "git status", timeout: 30 } as Record<string, unknown> }
|
||||
|
||||
// when
|
||||
replaceToolArgs(output, { command: "git log" })
|
||||
|
||||
// then
|
||||
expect(output.args.command).toBe("git log")
|
||||
expect(output.args.timeout).toBe(30)
|
||||
})
|
||||
|
||||
it("#when patching multiple properties #then all patched values are present", () => {
|
||||
// given
|
||||
const output = { args: { url: "http://old.com", format: "text" } as Record<string, unknown> }
|
||||
|
||||
// when
|
||||
replaceToolArgs(output, { url: "http://new.com", format: "markdown" })
|
||||
|
||||
// then
|
||||
expect(output.args.url).toBe("http://new.com")
|
||||
expect(output.args.format).toBe("markdown")
|
||||
})
|
||||
|
||||
it("#when patching #then the original args object is not the same reference", () => {
|
||||
// given
|
||||
const originalArgs = { command: "echo hi" } as Record<string, unknown>
|
||||
const output = { args: originalArgs }
|
||||
|
||||
// when
|
||||
replaceToolArgs(output, { command: "echo bye" })
|
||||
|
||||
// then
|
||||
expect(output.args).not.toBe(originalArgs)
|
||||
expect(originalArgs.command).toBe("echo hi")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a frozen output.args object", () => {
|
||||
it("#when patching a single property #then no TypeError is thrown and the value is updated", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ command: "git status", timeout: 30 }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { command: "git log" })).not.toThrow()
|
||||
expect(output.args.command).toBe("git log")
|
||||
expect(output.args.timeout).toBe(30)
|
||||
})
|
||||
|
||||
it("#when patching with Object-typed value #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ todos: "[]" }) as Record<string, unknown> }
|
||||
const parsed = [{ id: "1", content: "test", status: "pending" }]
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { todos: parsed })).not.toThrow()
|
||||
expect(output.args.todos).toEqual(parsed)
|
||||
})
|
||||
|
||||
it("#when patching url on frozen webfetch args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ url: "http://old.com", format: "markdown" }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { url: "http://redirected.com" })).not.toThrow()
|
||||
expect(output.args.url).toBe("http://redirected.com")
|
||||
expect(output.args.format).toBe("markdown")
|
||||
})
|
||||
|
||||
it("#when patching command with env prefix on frozen bash args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ command: "git rebase --continue" }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { command: "GIT_EDITOR=: git rebase --continue" })).not.toThrow()
|
||||
expect(output.args.command).toBe("GIT_EDITOR=: git rebase --continue")
|
||||
})
|
||||
|
||||
it("#when patching prompt on frozen task args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ prompt: "Do the thing", category: "quick" }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { prompt: "[DIRECTIVE] Do the thing" })).not.toThrow()
|
||||
expect(output.args.prompt).toBe("[DIRECTIVE] Do the thing")
|
||||
expect(output.args.category).toBe("quick")
|
||||
})
|
||||
|
||||
it("#when stripping null bytes from frozen command #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const frozenCommand = "echo \x00hello"
|
||||
const output = { args: Object.freeze({ command: frozenCommand }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { command: "echo hello" })).not.toThrow()
|
||||
expect(output.args.command).toBe("echo hello")
|
||||
})
|
||||
|
||||
it("#when replacing truncated question labels on frozen args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = {
|
||||
args: Object.freeze({
|
||||
questions: [{ question: "Pick", options: [{ label: "A very long label that should be truncated" }] }],
|
||||
}) as Record<string, unknown>,
|
||||
}
|
||||
const truncated = {
|
||||
questions: [{ question: "Pick", options: [{ label: "A very long label that sho..." }] }],
|
||||
}
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, truncated)).not.toThrow()
|
||||
expect((output.args.questions as Array<{ options: Array<{ label: string }> }>)[0].options[0].label).toBe(
|
||||
"A very long label that sho...",
|
||||
)
|
||||
})
|
||||
|
||||
it("#when replacing modifiedInput from PreToolUse hook on frozen args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = { args: Object.freeze({ filePath: "/old/path.ts" }) as Record<string, unknown> }
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { filePath: "/new/path.ts" })).not.toThrow()
|
||||
expect(output.args.filePath).toBe("/new/path.ts")
|
||||
})
|
||||
|
||||
it("#when replacing todo snapshot on frozen args #then no TypeError is thrown", () => {
|
||||
// given
|
||||
const output = {
|
||||
args: Object.freeze({
|
||||
todos: [{ content: "bootstrap", status: "pending" }],
|
||||
}) as Record<string, unknown>,
|
||||
}
|
||||
const snapshot = [
|
||||
{ content: "Real task 1", status: "in_progress" },
|
||||
{ content: "Real task 2", status: "pending" },
|
||||
]
|
||||
|
||||
// when / then
|
||||
expect(() => replaceToolArgs(output, { todos: snapshot })).not.toThrow()
|
||||
expect(output.args.todos).toEqual(snapshot)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,16 @@
|
||||
/**
|
||||
* Safely replace tool arguments without mutating frozen objects.
|
||||
*
|
||||
* opencode >=1.14 may freeze `output.args` via Immer before plugin hooks run.
|
||||
* Direct property assignment (`output.args.key = value`) or `Object.assign(output.args, patch)`
|
||||
* throws `TypeError: Attempted to assign to readonly property` on a frozen object.
|
||||
*
|
||||
* This helper replaces `output.args` with a shallow clone containing the patch,
|
||||
* which works regardless of whether the original args object is frozen.
|
||||
*/
|
||||
export function replaceToolArgs(
|
||||
output: { args: Record<string, unknown> },
|
||||
patch: Record<string, unknown>,
|
||||
): void {
|
||||
output.args = { ...output.args, ...patch }
|
||||
}
|
||||
Reference in New Issue
Block a user