Skip to content

Commit 904c871

Browse files
martins-ossmartins-oss
authored
fix(gateway): handle exec approvals persistence errors (#79861)
1 parent a1d1381 commit 904c871

2 files changed

Lines changed: 110 additions & 24 deletions

File tree

‎src/gateway/server-methods/exec-approvals.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import type { ExecApprovalsFile } from "../../infra/exec-approvals.js";
3+
4+
const ensureExecApprovalsMock = vi.hoisted(() => vi.fn());
5+
const readExecApprovalsSnapshotMock = vi.hoisted(() => vi.fn());
6+
const saveExecApprovalsMock = vi.hoisted(() => vi.fn());
7+
8+
vi.mock("../../infra/exec-approvals.js", async (importOriginal) => {
9+
const actual = await importOriginal<typeof import("../../infra/exec-approvals.js")>();
10+
return {
11+
...actual,
12+
ensureExecApprovals: ensureExecApprovalsMock,
13+
readExecApprovalsSnapshot: readExecApprovalsSnapshotMock,
14+
saveExecApprovals: saveExecApprovalsMock,
15+
};
16+
});
17+
18+
const { execApprovalsHandlers } = await import("./exec-approvals.js");
19+
20+
function makeSnapshot(file: ExecApprovalsFile = { version: 1, agents: {} }) {
21+
return {
22+
path: "/tmp/exec-approvals.json",
23+
exists: true,
24+
raw: JSON.stringify(file),
25+
file,
26+
hash: "base-hash",
27+
};
28+
}
29+
30+
describe("exec approvals gateway methods", () => {
31+
it("returns a structured unavailable error when local approvals get cannot read state", async () => {
32+
ensureExecApprovalsMock.mockImplementationOnce(() => {
33+
throw new Error("permission denied while ensuring approvals");
34+
});
35+
const respond = vi.fn();
36+
37+
await execApprovalsHandlers["exec.approvals.get"]({
38+
req: { type: "req", id: "req-1", method: "exec.approvals.get", params: {} },
39+
params: {},
40+
client: null,
41+
isWebchatConnect: () => false,
42+
respond,
43+
context: {} as never,
44+
});
45+
46+
expect(respond).toHaveBeenCalledWith(
47+
false,
48+
undefined,
49+
expect.objectContaining({
50+
code: "UNAVAILABLE",
51+
message: expect.stringContaining("permission denied while ensuring approvals"),
52+
}),
53+
);
54+
});
55+
56+
it("returns a structured unavailable error when local approvals set cannot persist", async () => {
57+
ensureExecApprovalsMock.mockReturnValue({ version: 1, agents: {} });
58+
readExecApprovalsSnapshotMock.mockReturnValue(makeSnapshot());
59+
saveExecApprovalsMock.mockImplementationOnce(() => {
60+
throw new Error("disk full while saving approvals");
61+
});
62+
const respond = vi.fn();
63+
64+
await execApprovalsHandlers["exec.approvals.set"]({
65+
req: { type: "req", id: "req-2", method: "exec.approvals.set", params: {} },
66+
params: { baseHash: "base-hash", file: { version: 1, agents: {} } },
67+
client: null,
68+
isWebchatConnect: () => false,
69+
respond,
70+
context: {} as never,
71+
});
72+
73+
expect(respond).toHaveBeenCalledWith(
74+
false,
75+
undefined,
76+
expect.objectContaining({
77+
code: "UNAVAILABLE",
78+
message: expect.stringContaining("disk full while saving approvals"),
79+
}),
80+
);
81+
});
82+
});

‎src/gateway/server-methods/exec-approvals.ts‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -126,37 +126,41 @@ async function respondWithExecApprovalsNodePayload<TParams extends { nodeId: str
126126
}
127127

128128
export const execApprovalsHandlers: GatewayRequestHandlers = {
129-
"exec.approvals.get": ({ params, respond }) => {
129+
"exec.approvals.get": async ({ params, respond }) => {
130130
if (!assertValidParams(params, validateExecApprovalsGetParams, "exec.approvals.get", respond)) {
131131
return;
132132
}
133-
ensureExecApprovals();
134-
const snapshot = readExecApprovalsSnapshot();
135-
respond(true, toExecApprovalsPayload(snapshot), undefined);
133+
await respondUnavailableOnThrow(respond, async () => {
134+
ensureExecApprovals();
135+
const snapshot = readExecApprovalsSnapshot();
136+
respond(true, toExecApprovalsPayload(snapshot), undefined);
137+
});
136138
},
137-
"exec.approvals.set": ({ params, respond }) => {
139+
"exec.approvals.set": async ({ params, respond }) => {
138140
if (!assertValidParams(params, validateExecApprovalsSetParams, "exec.approvals.set", respond)) {
139141
return;
140142
}
141-
ensureExecApprovals();
142-
const snapshot = readExecApprovalsSnapshot();
143-
if (!requireApprovalsBaseHash(params, snapshot, respond)) {
144-
return;
145-
}
146-
const incoming = (params as { file?: unknown }).file;
147-
if (!incoming || typeof incoming !== "object") {
148-
respond(
149-
false,
150-
undefined,
151-
errorShape(ErrorCodes.INVALID_REQUEST, "exec approvals file is required"),
152-
);
153-
return;
154-
}
155-
const normalized = normalizeExecApprovals(incoming as ExecApprovalsFile);
156-
const next = mergeExecApprovalsSocketDefaults({ normalized, current: snapshot.file });
157-
saveExecApprovals(next);
158-
const nextSnapshot = readExecApprovalsSnapshot();
159-
respond(true, toExecApprovalsPayload(nextSnapshot), undefined);
143+
await respondUnavailableOnThrow(respond, async () => {
144+
ensureExecApprovals();
145+
const snapshot = readExecApprovalsSnapshot();
146+
if (!requireApprovalsBaseHash(params, snapshot, respond)) {
147+
return;
148+
}
149+
const incoming = (params as { file?: unknown }).file;
150+
if (!incoming || typeof incoming !== "object") {
151+
respond(
152+
false,
153+
undefined,
154+
errorShape(ErrorCodes.INVALID_REQUEST, "exec approvals file is required"),
155+
);
156+
return;
157+
}
158+
const normalized = normalizeExecApprovals(incoming as ExecApprovalsFile);
159+
const next = mergeExecApprovalsSocketDefaults({ normalized, current: snapshot.file });
160+
saveExecApprovals(next);
161+
const nextSnapshot = readExecApprovalsSnapshot();
162+
respond(true, toExecApprovalsPayload(nextSnapshot), undefined);
163+
});
160164
},
161165
"exec.approvals.node.get": async ({ params, respond, context }) => {
162166
await respondWithExecApprovalsNodePayload({

0 commit comments

Comments
 (0)