Skip to content

Commit 9f98b6e

Browse files
BsnizNDPeter Steinberger
BsnizND
and
Peter Steinberger
authored
fix(gateway): emit stale exec approval followup diagnostics (#98293)
* fix(gateway): emit stale exec approval followup diagnostics * fix(gateway): cover approval suppression diagnostics in ci * fix(logging): preserve approval ids in stability bundles * docs(exec): document suppression diagnostics --------- Co-authored-by: BSnizND <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
1 parent 5a5913a commit 9f98b6e

14 files changed

Lines changed: 172 additions & 5 deletions

‎docs/cli/gateway.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ openclaw gateway stability --json
229229

230230
<AccordionGroup>
231231
<Accordion title="Privacy and bundle behavior">
232-
- Records keep operational metadata: event names, counts, byte sizes, memory readings, queue/session state, channel/plugin names, and redacted session summaries. They do not keep chat text, webhook bodies, tool outputs, raw request or response bodies, tokens, cookies, secret values, hostnames, or raw session ids. Set `diagnostics.enabled: false` to disable the recorder entirely.
232+
- Records keep operational metadata: event names, counts, byte sizes, memory readings, queue/session state, approval ids, channel/plugin names, and redacted session summaries. They do not keep chat text, webhook bodies, tool outputs, raw request or response bodies, tokens, cookies, secret values, hostnames, or raw session ids. Set `diagnostics.enabled: false` to disable the recorder entirely.
233233
- On fatal Gateway exits, shutdown timeouts, and restart startup failures, OpenClaw writes the same diagnostic snapshot to `~/.openclaw/logs/stability/openclaw-stability-*.json` when the recorder has events. Inspect the newest bundle with `openclaw gateway stability --bundle latest`; `--limit`, `--type`, and `--since-seq` also apply to bundle output.
234234

235235
</Accordion>

‎docs/gateway/opentelemetry.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,10 @@ to them directly without OTLP export.
412412
- `exec.process.completed` - terminal outcome, duration, target, mode, exit
413413
code, and failure kind. Command text and working directories are not
414414
included.
415+
- `exec.approval.followup_suppressed` - stale approval follow-up dropped after
416+
a session rebound. Includes `approvalId`, `reason` (`session_rebound`),
417+
`phase` (`direct_delivery` or `gateway_preflight`), and the dispatcher
418+
timestamp. Session keys, routes, and command text are not included.
415419

416420
## Without an exporter
417421

‎extensions/diagnostics-otel/src/service.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3823,6 +3823,8 @@ export function createDiagnosticsOtelService(): OpenClawPluginService {
38233823
case "exec.process.completed":
38243824
recordExecProcessCompleted(evt);
38253825
return;
3826+
case "exec.approval.followup_suppressed":
3827+
return;
38263828
case "log.record":
38273829
recordLogRecord?.(evt, metadata);
38283830
return;

‎scripts/plugin-sdk-surface-report.mjs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ const defaultPublicDeprecatedExportsByEntrypointBudget = Object.freeze({
127127
"outbound-send-deps": 4,
128128
"outbound-runtime": 16,
129129
"file-access-runtime": 2,
130-
"infra-runtime": 584,
130+
"infra-runtime": 585,
131131
"ssrf-policy": 1,
132132
"ssrf-runtime": 1,
133133
"media-runtime": 2,
@@ -202,11 +202,11 @@ let publicDeprecatedExportsByEntrypointBudget;
202202
try {
203203
budgets = {
204204
publicEntrypoints: readBudgetEnv("OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_ENTRYPOINTS", 322),
205-
publicExports: readBudgetEnv("OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_EXPORTS", 10401),
205+
publicExports: readBudgetEnv("OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_EXPORTS", 10402),
206206
publicFunctionExports: readBudgetEnv("OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_FUNCTION_EXPORTS", 5220),
207207
publicDeprecatedExports: readBudgetEnv(
208208
"OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_DEPRECATED_EXPORTS",
209-
3260,
209+
3261,
210210
),
211211
publicWildcardReexports: readBudgetEnv(
212212
"OPENCLAW_PLUGIN_SDK_MAX_PUBLIC_WILDCARD_REEXPORTS",

‎src/agents/bash-tools.exec-approval-followup.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import os from "node:os";
1818
import path from "node:path";
1919
import { clearSessionStoreCacheForTest } from "../config/sessions/store.js";
2020
import { writeSessionStoreForTest } from "../config/sessions/test-helpers.js";
21+
import {
22+
onDiagnosticEvent,
23+
resetDiagnosticEventsForTest,
24+
waitForDiagnosticEventsDrained,
25+
type DiagnosticEventPayload,
26+
} from "../infra/diagnostic-events.js";
2127
import { sendMessage } from "../infra/outbound/message.js";
2228
import {
2329
buildExecApprovalFollowupPrompt,
@@ -39,6 +45,7 @@ function writeTempSessionStore(entries: Record<string, { sessionId: string }>):
3945

4046
afterEach(() => {
4147
vi.resetAllMocks();
48+
resetDiagnosticEventsForTest();
4249
clearSessionStoreCacheForTest();
4350
while (tempStoreDirs.length > 0) {
4451
const dir = tempStoreDirs.pop();
@@ -171,6 +178,10 @@ describe("exec approval followup", () => {
171178
const sessionStore = writeTempSessionStore({
172179
"agent:main:main": { sessionId: "session-after-reset" },
173180
});
181+
const diagnostics: DiagnosticEventPayload[] = [];
182+
onDiagnosticEvent((event) => {
183+
diagnostics.push(event);
184+
});
174185

175186
const result = await sendExecApprovalFollowup({
176187
approvalId: "req-denied-rebound",
@@ -184,6 +195,15 @@ describe("exec approval followup", () => {
184195
});
185196

186197
expect(result).toBe(false);
198+
await waitForDiagnosticEventsDrained();
199+
expect(diagnostics).toContainEqual(
200+
expect.objectContaining({
201+
type: "exec.approval.followup_suppressed",
202+
approvalId: "req-denied-rebound",
203+
reason: "session_rebound",
204+
phase: "direct_delivery",
205+
}),
206+
);
187207
expect(sendMessage).not.toHaveBeenCalled();
188208
expect(callGatewayTool).not.toHaveBeenCalled();
189209
});
@@ -212,6 +232,10 @@ describe("exec approval followup", () => {
212232
const sessionStore = writeTempSessionStore({
213233
"agent:main:main": { sessionId: "session-after-reset" },
214234
});
235+
const diagnostics: DiagnosticEventPayload[] = [];
236+
onDiagnosticEvent((event) => {
237+
diagnostics.push(event);
238+
});
215239

216240
const result = await sendExecApprovalFollowup({
217241
approvalId: "req-finished-rebound",
@@ -225,6 +249,15 @@ describe("exec approval followup", () => {
225249
});
226250

227251
expect(result).toBe(false);
252+
await waitForDiagnosticEventsDrained();
253+
expect(diagnostics).toContainEqual(
254+
expect.objectContaining({
255+
type: "exec.approval.followup_suppressed",
256+
approvalId: "req-finished-rebound",
257+
reason: "session_rebound",
258+
phase: "direct_delivery",
259+
}),
260+
);
228261
expect(sendMessage).not.toHaveBeenCalled();
229262
expect(callGatewayTool).not.toHaveBeenCalled();
230263
});

‎src/agents/bash-tools.exec-approval-followup.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from "@openclaw/normalization-core/string-coerce";
1010
import { resolveStorePath } from "../config/sessions/paths.js";
1111
import { loadSessionStore } from "../config/sessions/store-load.js";
12+
import { emitDiagnosticEvent } from "../infra/diagnostic-events.js";
1213
import {
1314
resolveExternalBestEffortDeliveryTarget,
1415
type ExternalBestEffortDeliveryTarget,
@@ -394,6 +395,12 @@ export async function sendExecApprovalFollowup(
394395
sessionStore: params.sessionStore,
395396
})
396397
) {
398+
emitDiagnosticEvent({
399+
type: "exec.approval.followup_suppressed",
400+
approvalId: params.approvalId,
401+
reason: "session_rebound",
402+
phase: "direct_delivery",
403+
});
397404
log.info(
398405
`Dropping stale denied exec approval followup ${params.approvalId}: session ${sessionKey ?? ""} was rebound before the approval resolved`,
399406
);
@@ -423,6 +430,12 @@ export async function sendExecApprovalFollowup(
423430
sessionStore: params.sessionStore,
424431
})
425432
) {
433+
emitDiagnosticEvent({
434+
type: "exec.approval.followup_suppressed",
435+
approvalId: params.approvalId,
436+
reason: "session_rebound",
437+
phase: "direct_delivery",
438+
});
426439
log.info(
427440
`Dropping stale exec approval followup ${params.approvalId} direct fallback: session ${sessionKey ?? ""} was rebound before the approval resolved`,
428441
);

‎src/gateway/server-methods/agent.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ import fs from "node:fs/promises";
44
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
55
import { ErrorCodes } from "../../../packages/gateway-protocol/src/index.js";
66
import type { readAcpSessionMeta } from "../../acp/runtime/session-meta.js";
7+
import {
8+
onDiagnosticEvent,
9+
resetDiagnosticEventsForTest,
10+
waitForDiagnosticEventsDrained,
11+
type DiagnosticEventPayload,
12+
} from "../../infra/diagnostic-events.js";
713
import {
814
registerExecApprovalFollowupRuntimeHandoff,
915
resetExecApprovalFollowupRuntimeHandoffsForTests,
@@ -600,6 +606,7 @@ describe("gateway agent handler", () => {
600606
afterEach(() => {
601607
envSnapshot.restore();
602608
resetDetachedTaskLifecycleRuntimeForTests();
609+
resetDiagnosticEventsForTest();
603610
resetTaskRegistryForTests();
604611
resetSubagentRegistryForTests({ persist: false });
605612
subagentRegistryTesting.setDepsForTest();
@@ -3212,6 +3219,10 @@ describe("gateway agent handler", () => {
32123219
lastTo: "123",
32133220
});
32143221
const context = makeContext();
3222+
const diagnostics: DiagnosticEventPayload[] = [];
3223+
onDiagnosticEvent((event) => {
3224+
diagnostics.push(event);
3225+
});
32153226
const updateSessionStoreCallsBefore = mocks.updateSessionStore.mock.calls.length;
32163227
const agentCommandCallsBefore = mocks.agentCommand.mock.calls.length;
32173228

@@ -3237,6 +3248,15 @@ describe("gateway agent handler", () => {
32373248
status: "ok",
32383249
summary: expect.stringContaining("exec approval followup dropped"),
32393250
});
3251+
await waitForDiagnosticEventsDrained();
3252+
expect(diagnostics).toContainEqual(
3253+
expect.objectContaining({
3254+
type: "exec.approval.followup_suppressed",
3255+
approvalId: "req-rebound-followup",
3256+
reason: "session_rebound",
3257+
phase: "gateway_preflight",
3258+
}),
3259+
);
32403260
expect(mocks.updateSessionStore.mock.calls.length).toBe(updateSessionStoreCallsBefore);
32413261
expect(mocks.agentCommand.mock.calls.length).toBe(agentCommandCallsBefore);
32423262
const dedupeEntry = context.dedupe.get("agent:exec-approval-followup:req-rebound-followup");

‎src/gateway/server-methods/agent.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ import {
7777
import { hasProviderOwnedSession } from "../../config/sessions/entry-freshness.js";
7878
import { resolveMaintenanceConfigFromInput } from "../../config/sessions/store-maintenance.js";
7979
import type { OpenClawConfig } from "../../config/types.openclaw.js";
80+
import { emitDiagnosticEvent } from "../../infra/diagnostic-events.js";
8081
import {
8182
assertAgentRunLifecycleGenerationCurrent,
8283
claimAgentRunContext,
@@ -1516,6 +1517,12 @@ export const agentHandlers: GatewayRequestHandlers = {
15161517
resolvedSessionId: currentSessionId,
15171518
})
15181519
) {
1520+
emitDiagnosticEvent({
1521+
type: "exec.approval.followup_suppressed",
1522+
approvalId: execApprovalFollowupApprovalId,
1523+
reason: "session_rebound",
1524+
phase: "gateway_preflight",
1525+
});
15191526
context.logGateway.info(
15201527
`Dropping stale exec approval followup ${execApprovalFollowupApprovalId}: session ${requestedSessionKeyRaw} rebound (expected ${expectedSessionId}, current ${currentSessionId}) before the approval resolved`,
15211528
);

‎src/infra/diagnostic-events.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,32 @@ describe("diagnostic-events", () => {
801801
expect(internalEvents).toEqual(["log.record"]);
802802
});
803803

804+
it("emits exec approval followup suppression events on the public stream", async () => {
805+
const events: DiagnosticEventPayload[] = [];
806+
onDiagnosticEvent((event) => {
807+
events.push(event);
808+
});
809+
810+
emitDiagnosticEvent({
811+
type: "exec.approval.followup_suppressed",
812+
approvalId: "approval-123",
813+
reason: "session_rebound",
814+
phase: "gateway_preflight",
815+
});
816+
817+
await waitForDiagnosticEventsDrained();
818+
819+
expect(events).toContainEqual(
820+
expect.objectContaining({
821+
type: "exec.approval.followup_suppressed",
822+
approvalId: "approval-123",
823+
reason: "session_rebound",
824+
phase: "gateway_preflight",
825+
ts: expect.any(Number),
826+
}),
827+
);
828+
});
829+
804830
it("keeps trusted private data off shared internal diagnostic listeners", async () => {
805831
const internalEvents: DiagnosticEventPayload[] = [];
806832
const trustedEvents: Array<{

‎src/infra/diagnostic-events.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,13 @@ export type DiagnosticExecProcessCompletedEvent = DiagnosticBaseEvent & {
514514
| "runtime-error";
515515
};
516516

517+
export type DiagnosticExecApprovalFollowupSuppressedEvent = DiagnosticBaseEvent & {
518+
type: "exec.approval.followup_suppressed";
519+
approvalId: string;
520+
reason: "session_rebound";
521+
phase: "direct_delivery" | "gateway_preflight";
522+
};
523+
517524
type DiagnosticRunBaseEvent = DiagnosticBaseEvent & {
518525
runId: string;
519526
sessionKey?: string;
@@ -769,6 +776,7 @@ export type DiagnosticEventPayload =
769776
| DiagnosticToolExecutionBlockedEvent
770777
| DiagnosticSkillUsedEvent
771778
| DiagnosticExecProcessCompletedEvent
779+
| DiagnosticExecApprovalFollowupSuppressedEvent
772780
| DiagnosticRunStartedEvent
773781
| DiagnosticRunCompletedEvent
774782
| DiagnosticHarnessRunStartedEvent
@@ -863,6 +871,7 @@ const ASYNC_DIAGNOSTIC_EVENT_TYPES = new Set<DiagnosticEventPayload["type"]>([
863871
"tool.execution.blocked",
864872
"skill.used",
865873
"exec.process.completed",
874+
"exec.approval.followup_suppressed",
866875
"message.delivery.started",
867876
"message.delivery.completed",
868877
"message.delivery.error",

0 commit comments

Comments
 (0)