Merge pull request #4553 from code-yeongyu/ulw/codex-hephaestus-dup
fix: skip claude hook injection for internal prompts
This commit is contained in:
@@ -4,6 +4,27 @@ import { unsafeTestValue } from "../../../test-support/unsafe-test-value"
|
|||||||
|
|
||||||
describe("buildBackgroundTaskNotificationText", () => {
|
describe("buildBackgroundTaskNotificationText", () => {
|
||||||
describe("#given one task still running after a completed task notification", () => {
|
describe("#given one task still running after a completed task notification", () => {
|
||||||
|
test("#when building the partial notification #then it does not use the final completed heading", () => {
|
||||||
|
// given
|
||||||
|
const notification = buildBackgroundTaskNotificationText({
|
||||||
|
task: {
|
||||||
|
id: "task-1",
|
||||||
|
description: "Index repo",
|
||||||
|
status: "completed",
|
||||||
|
},
|
||||||
|
duration: "42s",
|
||||||
|
statusText: "COMPLETED",
|
||||||
|
allComplete: false,
|
||||||
|
remainingCount: 1,
|
||||||
|
completedTasks: [],
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(notification).not.toContain("[BACKGROUND TASK COMPLETED]")
|
||||||
|
expect(notification).toContain("[BACKGROUND TASK RESULT READY]")
|
||||||
|
expect(notification).toContain("You WILL be notified when ALL complete.")
|
||||||
|
})
|
||||||
|
|
||||||
test("#when building the partial notification #then it preserves the existing completed-task format", () => {
|
test("#when building the partial notification #then it preserves the existing completed-task format", () => {
|
||||||
// given
|
// given
|
||||||
const notification = buildBackgroundTaskNotificationText({
|
const notification = buildBackgroundTaskNotificationText({
|
||||||
@@ -21,7 +42,7 @@ describe("buildBackgroundTaskNotificationText", () => {
|
|||||||
|
|
||||||
// when
|
// when
|
||||||
const expectedNotification = `<system-reminder>
|
const expectedNotification = `<system-reminder>
|
||||||
[BACKGROUND TASK COMPLETED]
|
[BACKGROUND TASK RESULT READY]
|
||||||
**ID:** \`task-1\`
|
**ID:** \`task-1\`
|
||||||
**Description:** Index repo
|
**Description:** Index repo
|
||||||
**Duration:** 42s
|
**Duration:** 42s
|
||||||
@@ -156,6 +177,32 @@ Use \`background_output(task_id="<id>")\` to retrieve each result.
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("#given a completed task with retry attempt history", () => {
|
describe("#given a completed task with retry attempt history", () => {
|
||||||
|
test("#when building the final notification #then it includes the final completed heading", () => {
|
||||||
|
// given
|
||||||
|
const notification = buildBackgroundTaskNotificationText({
|
||||||
|
task: {
|
||||||
|
id: "task-3",
|
||||||
|
description: "Fallback task",
|
||||||
|
status: "completed",
|
||||||
|
},
|
||||||
|
duration: "10s",
|
||||||
|
statusText: "COMPLETED",
|
||||||
|
allComplete: true,
|
||||||
|
remainingCount: 0,
|
||||||
|
completedTasks: [
|
||||||
|
{
|
||||||
|
id: "task-3",
|
||||||
|
description: "Fallback task",
|
||||||
|
status: "completed",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(notification).toContain("[BACKGROUND TASK COMPLETED]")
|
||||||
|
expect(notification).toContain("[ALL BACKGROUND TASKS COMPLETE]")
|
||||||
|
})
|
||||||
|
|
||||||
test("#when building the final notification #then it renders the spec-aligned balanced attempt timeline", () => {
|
test("#when building the final notification #then it renders the spec-aligned balanced attempt timeline", () => {
|
||||||
// given
|
// given
|
||||||
const notification = buildBackgroundTaskNotificationText({
|
const notification = buildBackgroundTaskNotificationText({
|
||||||
|
|||||||
@@ -85,7 +85,7 @@ export function buildBackgroundTaskNotificationText(input: {
|
|||||||
const hasFailures = failedTasks.length > 0
|
const hasFailures = failedTasks.length > 0
|
||||||
const header = hasFailures
|
const header = hasFailures
|
||||||
? `[ALL BACKGROUND TASKS FINISHED - ${failedTasks.length} FAILED]`
|
? `[ALL BACKGROUND TASKS FINISHED - ${failedTasks.length} FAILED]`
|
||||||
: "[ALL BACKGROUND TASKS COMPLETE]"
|
: "[BACKGROUND TASK COMPLETED]\n[ALL BACKGROUND TASKS COMPLETE]"
|
||||||
|
|
||||||
let body = ""
|
let body = ""
|
||||||
if (succeededText) {
|
if (succeededText) {
|
||||||
@@ -108,9 +108,10 @@ Use \`background_output(task_id="<id>")\` to retrieve each result.${hasFailures
|
|||||||
}
|
}
|
||||||
|
|
||||||
const isFailure = statusText !== "COMPLETED"
|
const isFailure = statusText !== "COMPLETED"
|
||||||
|
const header = isFailure ? `[BACKGROUND TASK ${statusText}]` : "[BACKGROUND TASK RESULT READY]"
|
||||||
|
|
||||||
return `<system-reminder>
|
return `<system-reminder>
|
||||||
[BACKGROUND TASK ${statusText}]
|
${header}
|
||||||
**ID:** \`${task.id}\`
|
**ID:** \`${task.id}\`
|
||||||
**Description:** ${safeDescription(task)}
|
**Description:** ${safeDescription(task)}
|
||||||
**Duration:** ${duration}${errorInfo}
|
**Duration:** ${duration}${errorInfo}
|
||||||
|
|||||||
@@ -390,7 +390,7 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => {
|
|||||||
expect(promptAsyncCalls).toHaveLength(1)
|
expect(promptAsyncCalls).toHaveLength(1)
|
||||||
expect(promptAsyncCalls[0]?.body.noReply).toBe(true)
|
expect(promptAsyncCalls[0]?.body.noReply).toBe(true)
|
||||||
const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts)
|
const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts)
|
||||||
expect(notificationPayload).toContain("BACKGROUND TASK COMPLETED")
|
expect(notificationPayload).toContain("BACKGROUND TASK RESULT READY")
|
||||||
expect(notificationPayload).not.toContain("ALL BACKGROUND TASKS COMPLETE")
|
expect(notificationPayload).not.toContain("ALL BACKGROUND TASKS COMPLETE")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -50,6 +50,7 @@ export function createChatMessageHandler(
|
|||||||
})
|
})
|
||||||
|
|
||||||
const messageParts: MessagePart[] = textParts.map((p) => ({
|
const messageParts: MessagePart[] = textParts.map((p) => ({
|
||||||
|
...p,
|
||||||
type: "text",
|
type: "text",
|
||||||
text: p.text,
|
text: p.text,
|
||||||
}))
|
}))
|
||||||
|
|||||||
@@ -1,10 +1,16 @@
|
|||||||
import { describe, it, expect } from "bun:test"
|
import { afterEach, describe, expect, it, mock, spyOn } from "bun:test"
|
||||||
|
import { OMO_INTERNAL_INITIATOR_MARKER } from "../../shared/internal-initiator-marker"
|
||||||
|
import * as dispatchHookModule from "./dispatch-hook"
|
||||||
import {
|
import {
|
||||||
executeUserPromptSubmitHooks,
|
executeUserPromptSubmitHooks,
|
||||||
type UserPromptSubmitContext,
|
type UserPromptSubmitContext,
|
||||||
} from "./user-prompt-submit"
|
} from "./user-prompt-submit"
|
||||||
|
|
||||||
describe("executeUserPromptSubmitHooks", () => {
|
describe("executeUserPromptSubmitHooks", () => {
|
||||||
|
afterEach(() => {
|
||||||
|
mock.restore()
|
||||||
|
})
|
||||||
|
|
||||||
it("returns early when no config provided", async () => {
|
it("returns early when no config provided", async () => {
|
||||||
// given
|
// given
|
||||||
const ctx: UserPromptSubmitContext = {
|
const ctx: UserPromptSubmitContext = {
|
||||||
@@ -104,4 +110,65 @@ describe("executeUserPromptSubmitHooks", () => {
|
|||||||
expect(result1.block).toBe(false)
|
expect(result1.block).toBe(false)
|
||||||
expect(result2.block).toBe(false)
|
expect(result2.block).toBe(false)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("#given synthetic hook context only #when prompt submit runs #then hook command is not dispatched", async () => {
|
||||||
|
// given
|
||||||
|
const dispatchSpy = spyOn(dispatchHookModule, "dispatchHook").mockResolvedValue({
|
||||||
|
exitCode: 0,
|
||||||
|
stdout: "hook output",
|
||||||
|
stderr: "",
|
||||||
|
})
|
||||||
|
const ctx: UserPromptSubmitContext = {
|
||||||
|
sessionId: "test-session-synthetic",
|
||||||
|
prompt: "synthetic hook message",
|
||||||
|
parts: [{ type: "text", text: "synthetic hook message", synthetic: true }],
|
||||||
|
cwd: "/tmp",
|
||||||
|
}
|
||||||
|
const config = {
|
||||||
|
UserPromptSubmit: [
|
||||||
|
{ matcher: "*", hooks: [{ type: "command" as const, command: "echo hook" }] },
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = await executeUserPromptSubmitHooks(ctx, config)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.block).toBe(false)
|
||||||
|
expect(result.messages).toEqual([])
|
||||||
|
expect(dispatchSpy).toHaveBeenCalledTimes(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#given internal prompt marker only #when prompt submit runs #then hook command is not dispatched", async () => {
|
||||||
|
// given
|
||||||
|
const dispatchSpy = spyOn(dispatchHookModule, "dispatchHook").mockResolvedValue({
|
||||||
|
exitCode: 0,
|
||||||
|
stdout: "hook output",
|
||||||
|
stderr: "",
|
||||||
|
})
|
||||||
|
const ctx: UserPromptSubmitContext = {
|
||||||
|
sessionId: "test-session-internal",
|
||||||
|
prompt: `internal hook message\n${OMO_INTERNAL_INITIATOR_MARKER}`,
|
||||||
|
parts: [
|
||||||
|
{
|
||||||
|
type: "text",
|
||||||
|
text: `internal hook message\n${OMO_INTERNAL_INITIATOR_MARKER}`,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
cwd: "/tmp",
|
||||||
|
}
|
||||||
|
const config = {
|
||||||
|
UserPromptSubmit: [
|
||||||
|
{ matcher: "*", hooks: [{ type: "command" as const, command: "echo hook" }] },
|
||||||
|
],
|
||||||
|
}
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = await executeUserPromptSubmitHooks(ctx, config)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result.block).toBe(false)
|
||||||
|
expect(result.messages).toEqual([])
|
||||||
|
expect(dispatchSpy).toHaveBeenCalledTimes(0)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import type {
|
|||||||
ClaudeHooksConfig,
|
ClaudeHooksConfig,
|
||||||
} from "./types"
|
} from "./types"
|
||||||
import { findMatchingHooks, log } from "../../shared"
|
import { findMatchingHooks, log } from "../../shared"
|
||||||
|
import { isRealUserTextPart } from "../../shared/internal-initiator-marker"
|
||||||
import { dispatchHook, getHookIdentifier } from "./dispatch-hook"
|
import { dispatchHook, getHookIdentifier } from "./dispatch-hook"
|
||||||
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
import { isHookCommandDisabled, type PluginExtendedConfig } from "./config-loader"
|
||||||
|
|
||||||
@@ -44,10 +45,14 @@ export async function executeUserPromptSubmitHooks(
|
|||||||
return { block: false, modifiedParts, messages }
|
return { block: false, modifiedParts, messages }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const realUserTextParts = ctx.parts.filter(isRealUserTextPart)
|
||||||
|
if (realUserTextParts.length === 0) {
|
||||||
|
return { block: false, modifiedParts, messages }
|
||||||
|
}
|
||||||
|
|
||||||
// Check if hook tags are in the current user input only (not in injected context)
|
// Check if hook tags are in the current user input only (not in injected context)
|
||||||
// by checking only the text parts that were provided in this message
|
// by checking only the text parts that were provided in this message
|
||||||
const userInputText = ctx.parts
|
const userInputText = realUserTextParts
|
||||||
.filter((p) => p.type === "text" && p.text)
|
|
||||||
.map((p) => p.text ?? "")
|
.map((p) => p.text ?? "")
|
||||||
.join("\n")
|
.join("\n")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user