fix: surface hook block response without live user rewrite

This commit is contained in:
jesse-merhi
2026-05-06 12:46:31 +10:00
committed by clawsweeper
parent ae922c96f9
commit 5f10007b7a
5 changed files with 26 additions and 69 deletions

View File

@@ -700,7 +700,7 @@ describe("runWithModelFallback", () => {
it("keeps before_agent_run hook blocks out of empty-result fallback", () => {
const runResult: EmbeddedPiRunResult = {
payloads: [],
payloads: [{ text: "Blocked by before-run policy.", isError: true }],
meta: {
durationMs: 1,
livenessState: "blocked",

View File

@@ -49,7 +49,7 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
mockedGlobalHookRunner.hasHooks.mockImplementation(() => false);
});
it("does not emit a duplicate agent payload when before_agent_run blocks", async () => {
it("emits the before_agent_run hook block message as the agent payload", async () => {
mockedRunEmbeddedAttempt.mockResolvedValueOnce(
makeAttemptResult({
assistantTexts: [],
@@ -64,8 +64,11 @@ describe("runEmbeddedPiAgent incomplete-turn safety", () => {
});
expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1);
expect(result.payloads).toEqual([]);
expect(result.payloads).toEqual([{ text: "Blocked by before-run policy.", isError: true }]);
expect(result.meta).toMatchObject({
finalAssistantVisibleText: "Blocked by before-run policy.",
finalAssistantRawText: "Blocked by before-run policy.",
finalPromptText: undefined,
livenessState: "blocked",
error: { kind: "hook_block", message: "Blocked by before-run policy." },
});

View File

@@ -1834,7 +1834,7 @@ export async function runEmbeddedPiAgent(
livenessState: "blocked",
});
return {
payloads: [],
payloads: [{ text: errorText, isError: true }],
meta: {
durationMs: Date.now() - started,
agentMeta: buildErrorAgentMeta({
@@ -1848,7 +1848,9 @@ export async function runEmbeddedPiAgent(
lastTurnTotal,
}),
systemPromptReport: attempt.systemPromptReport,
finalPromptText: attempt.finalPromptText,
finalAssistantVisibleText: errorText,
finalAssistantRawText: errorText,
finalPromptText: undefined,
replayInvalid,
livenessState: "blocked",
error: { kind: "hook_block", message: errorText },

View File

@@ -2129,21 +2129,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
});
expect(userUpdateCountAtAgentStart).toBe(0);
const userUpdate = mockState.emittedTranscriptUpdates.find(
const userUpdates = mockState.emittedTranscriptUpdates.filter(
(update) =>
typeof update.message === "object" &&
update.message !== null &&
(update.message as { role?: unknown }).role === "user",
);
expect(userUpdate).toMatchObject({
sessionFile: expect.stringMatching(/sess\.jsonl$/),
sessionKey: "main",
message: {
role: "user",
content: "secret prompt that may be blocked",
timestamp: expect.any(Number),
},
});
expect(userUpdates).toHaveLength(0);
});
it("does not emit raw user transcript content when before_agent_run blocks without a persisted marker", async () => {
@@ -2172,7 +2164,7 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
expect(userUpdates).toHaveLength(0);
});
it("emits raw user transcript content when before_agent_run passes but the agent fails", async () => {
it("does not emit live user transcript content when before_agent_run hooks are present and the agent fails", async () => {
createTranscriptFixture("openclaw-chat-send-user-transcript-gate-pass-error-");
mockState.triggerAgentRunStart = true;
mockState.hasBeforeAgentRunHooks = true;
@@ -2188,23 +2180,13 @@ describe("chat directive tag stripping for non-streaming final payloads", () =>
expectBroadcast: false,
});
await waitForAssertion(() => {
const userUpdate = mockState.emittedTranscriptUpdates.find(
(update) =>
typeof update.message === "object" &&
update.message !== null &&
(update.message as { role?: unknown }).role === "user",
);
expect(userUpdate).toMatchObject({
sessionFile: expect.stringMatching(/sess\.jsonl$/),
sessionKey: "main",
message: {
role: "user",
content: "prompt allowed before model error",
timestamp: expect.any(Number),
},
});
});
const userUpdates = mockState.emittedTranscriptUpdates.filter(
(update) =>
typeof update.message === "object" &&
update.message !== null &&
(update.message as { role?: unknown }).role === "user",
);
expect(userUpdates).toHaveLength(0);
});
it("adds persisted media paths to the user transcript update", async () => {

View File

@@ -2260,29 +2260,7 @@ export const chatHandlers: GatewayRequestHandlers = {
let appendedWebchatAgentMedia = false;
let userTranscriptUpdatePromise: Promise<void> | null = null;
let agentRunStarted = false;
let beforeAgentRunBlocked = false;
const hasBeforeAgentRunGate = getGlobalHookRunner()?.hasHooks("before_agent_run") === true;
const beforeAgentRunBlockIdempotencyKey = `hook-block:before_agent_run:user:${clientRunId}`;
const hasPersistedBeforeAgentRunBlock = async () => {
if (!hasBeforeAgentRunGate) {
return false;
}
const { storePath: latestStorePath, entry: latestEntry } = loadSessionEntry(sessionKey);
const resolvedSessionId = latestEntry?.sessionId ?? backingSessionId;
if (!resolvedSessionId) {
return false;
}
const transcriptPath = resolveTranscriptPath({
sessionId: resolvedSessionId,
storePath: latestStorePath,
sessionFile: latestEntry?.sessionFile ?? entry?.sessionFile,
agentId,
});
if (!transcriptPath) {
return false;
}
return await transcriptHasIdempotencyKey(transcriptPath, beforeAgentRunBlockIdempotencyKey);
};
const emitUserTranscriptUpdate = async () => {
if (userTranscriptUpdatePromise) {
await userTranscriptUpdatePromise;
@@ -2316,12 +2294,6 @@ export const chatHandlers: GatewayRequestHandlers = {
})();
await userTranscriptUpdatePromise;
};
const emitUserTranscriptUpdateUnlessBeforeAgentRunBlocked = async () => {
if (beforeAgentRunBlocked || (await hasPersistedBeforeAgentRunBlock())) {
return;
}
await emitUserTranscriptUpdate();
};
let transcriptMediaRewriteDone = false;
const rewriteUserTranscriptMedia = async () => {
if (transcriptMediaRewriteDone) {
@@ -2497,8 +2469,7 @@ export const chatHandlers: GatewayRequestHandlers = {
onModelSelected,
},
})
.then(async (dispatchResult) => {
beforeAgentRunBlocked = dispatchResult.beforeAgentRunBlocked === true;
.then(async () => {
await rewriteUserTranscriptMedia();
// WebChat persistence has two owners. Agent runs persist model-visible turns
// through Pi's SessionManager; this dispatcher only owns live delivery payloads.
@@ -2678,8 +2649,8 @@ export const chatHandlers: GatewayRequestHandlers = {
message,
});
}
} else {
await emitUserTranscriptUpdateUnlessBeforeAgentRunBlocked().catch((transcriptErr) => {
} else if (!hasBeforeAgentRunGate) {
await emitUserTranscriptUpdate().catch((transcriptErr) => {
context.logGateway.warn(
`webchat user transcript update failed after agent run: ${formatForLog(transcriptErr)}`,
);
@@ -2703,10 +2674,9 @@ export const chatHandlers: GatewayRequestHandlers = {
`webchat transcript media rewrite failed after error: ${formatForLog(rewriteErr)}`,
);
});
const emitAfterError = !agentRunStarted
? emitUserTranscriptUpdate()
: hasBeforeAgentRunGate
? emitUserTranscriptUpdateUnlessBeforeAgentRunBlocked()
const emitAfterError =
agentRunStarted && hasBeforeAgentRunGate
? Promise.resolve()
: emitUserTranscriptUpdate();
await emitAfterError.catch((transcriptErr) => {
context.logGateway.warn(