Skip to content

Commit 8e179cf

Browse files
committed
[Admin Governance] Settle repair side effects
1 parent 1d5c909 commit 8e179cf

2 files changed

Lines changed: 138 additions & 15 deletions

File tree

front/migrations/20260922_restore_editorless_agent_authors.test.ts

Lines changed: 97 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@ import { GroupPermissionResource } from "@app/lib/resources/group_permission_res
33
import { withTransaction } from "@app/lib/utils/sql_utils";
44
import baseLogger from "@app/logger/logger";
55
import { restoreEditorlessAgentAuthors } from "@app/migrations/20260922_restore_editorless_agent_authors";
6+
import * as searchIndexationClient from "@app/temporal/es_indexation/client";
67
import { AgentConfigurationFactory } from "@app/tests/utils/AgentConfigurationFactory";
78
import { createResourceTest } from "@app/tests/utils/generic_resource_tests";
89
import { MembershipFactory } from "@app/tests/utils/MembershipFactory";
910
import { UserFactory } from "@app/tests/utils/UserFactory";
1011
import type { AgentConfigurationType } from "@app/types/assistant/agent";
12+
import { Err, Ok } from "@app/types/shared/result";
1113
import type { UserType } from "@app/types/user";
1214
import assert from "assert";
1315
import { afterEach, describe, expect, it, vi } from "vitest";
@@ -50,8 +52,8 @@ async function editorIds(
5052
describe("restoreEditorlessAgentAuthors", () => {
5153
it("restores only editorless agents, with a dry run and idempotent rerun", async () => {
5254
const launchSearchIndexation = vi
53-
.spyOn(AgentResource, "launchSearchIndexation")
54-
.mockResolvedValue();
55+
.spyOn(searchIndexationClient, "launchIndexAgentSearchWorkflow")
56+
.mockResolvedValue(new Ok(undefined));
5557
const {
5658
authenticator: auth,
5759
user,
@@ -117,10 +119,10 @@ describe("restoreEditorlessAgentAuthors", () => {
117119
otherEditor.sId,
118120
]);
119121
expect(await editorIds(auth, resourceWithGlobalEditor)).toEqual([]);
120-
expect(launchSearchIndexation).toHaveBeenCalledExactlyOnceWith(
121-
expect.anything(),
122-
[editorlessAgent.sId]
123-
);
122+
expect(launchSearchIndexation).toHaveBeenCalledExactlyOnceWith({
123+
workspaceId: workspace.sId,
124+
agentId: editorlessAgent.sId,
125+
});
124126

125127
await expect(
126128
restoreEditorlessAgentAuthors({
@@ -135,6 +137,95 @@ describe("restoreEditorlessAgentAuthors", () => {
135137
});
136138
});
137139

140+
it("retries failed search workflow launches", async () => {
141+
const launchSearchIndexation = vi
142+
.spyOn(searchIndexationClient, "launchIndexAgentSearchWorkflow")
143+
.mockResolvedValue(new Ok(undefined));
144+
const { authenticator: auth, workspace } = await createResourceTest({
145+
role: "admin",
146+
});
147+
const agent = await AgentConfigurationFactory.createTestAgent(auth, {
148+
name: "Editorless agent",
149+
});
150+
await replaceEditors(auth, agent, []);
151+
launchSearchIndexation.mockReset();
152+
launchSearchIndexation
153+
.mockResolvedValueOnce(new Err(new Error("Temporal unavailable")))
154+
.mockResolvedValueOnce(new Err(new Error("Temporal unavailable")))
155+
.mockResolvedValueOnce(new Ok(undefined));
156+
157+
await restoreEditorlessAgentAuthors({
158+
execute: true,
159+
logger,
160+
wId: workspace.sId,
161+
});
162+
163+
expect(launchSearchIndexation).toHaveBeenCalledTimes(3);
164+
});
165+
166+
it("waits for every in-flight repair before propagating a failure", async () => {
167+
vi.spyOn(
168+
searchIndexationClient,
169+
"launchIndexAgentSearchWorkflow"
170+
).mockResolvedValue(new Ok(undefined));
171+
const {
172+
authenticator: auth,
173+
user,
174+
workspace,
175+
} = await createResourceTest({ role: "admin" });
176+
const firstAgent = await AgentConfigurationFactory.createTestAgent(auth, {
177+
name: "First editorless agent",
178+
});
179+
const secondAgent = await AgentConfigurationFactory.createTestAgent(auth, {
180+
name: "Second editorless agent",
181+
});
182+
const firstResource = await replaceEditors(auth, firstAgent, []);
183+
const secondResource = await replaceEditors(auth, secondAgent, []);
184+
185+
const originalGrantToUser = GroupPermissionResource.grantToUser;
186+
let releaseSecondGrant: () => void = () => undefined;
187+
const secondGrantGate = new Promise<void>((resolve) => {
188+
releaseSecondGrant = resolve;
189+
});
190+
let markSecondGrantStarted: () => void = () => undefined;
191+
const secondGrantStarted = new Promise<void>((resolve) => {
192+
markSecondGrantStarted = resolve;
193+
});
194+
vi.spyOn(GroupPermissionResource, "grantToUser").mockImplementation(
195+
async (grantAuth, grant) => {
196+
if (grant.resourceId === firstResource.id) {
197+
throw new Error("Failed first repair");
198+
}
199+
if (grant.resourceId === secondResource.id) {
200+
markSecondGrantStarted();
201+
await secondGrantGate;
202+
}
203+
return originalGrantToUser.call(
204+
GroupPermissionResource,
205+
grantAuth,
206+
grant
207+
);
208+
}
209+
);
210+
211+
let settled = false;
212+
const run = restoreEditorlessAgentAuthors({
213+
execute: true,
214+
logger,
215+
wId: workspace.sId,
216+
}).finally(() => {
217+
settled = true;
218+
});
219+
await secondGrantStarted;
220+
await new Promise((resolve) => setTimeout(resolve, 0));
221+
expect(settled).toBe(false);
222+
223+
releaseSecondGrant();
224+
await expect(run).rejects.toThrow("Failed first repair");
225+
expect(await editorIds(auth, firstResource)).toEqual([]);
226+
expect(await editorIds(auth, secondResource)).toEqual([user.sId]);
227+
});
228+
138229
it("rejects an unknown workspace scope", async () => {
139230
await expect(
140231
restoreEditorlessAgentAuthors({

front/migrations/20260922_restore_editorless_agent_authors.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
emitAuditLogEventDirect,
44
} from "@app/lib/api/audit/workos_audit";
55
import { Authenticator } from "@app/lib/auth";
6-
import { AgentResource } from "@app/lib/resources/agent_resource";
76
import { GroupPermissionResource } from "@app/lib/resources/group_permission_resource";
87
import { frontSequelize } from "@app/lib/resources/storage";
98
import { UserResource } from "@app/lib/resources/user_resource";
@@ -12,8 +11,11 @@ import { concurrentExecutor, withRetry } from "@app/lib/utils/async_utils";
1211
import { withTransaction } from "@app/lib/utils/sql_utils";
1312
import type { Logger } from "@app/logger/logger";
1413
import { makeScript } from "@app/scripts/helpers";
14+
import { launchIndexAgentSearchWorkflow } from "@app/temporal/es_indexation/client";
1515
import type { AgentConfigurationScope } from "@app/types/assistant/agent";
1616
import type { ModelId } from "@app/types/shared/model_id";
17+
import { Err, Ok } from "@app/types/shared/result";
18+
import { normalizeError } from "@app/types/shared/utils/error_utils";
1719
import assert from "assert";
1820
import type { Transaction } from "sequelize";
1921
import { QueryTypes } from "sequelize";
@@ -220,11 +222,17 @@ async function restoreAuthor(
220222
},
221223
});
222224

223-
// Keep indexing outside the write transaction so a Temporal connection failure cannot skip the
224-
// audit event after the repair has committed. Retry connection failures, then log and continue.
225-
const indexation = await withRetry(() =>
226-
AgentResource.launchSearchIndexation(auth, [restored.agentId])
227-
);
225+
// Keep indexing outside the write transaction so a Temporal failure cannot skip the audit event
226+
// after the repair has committed. Retry launch failures, then log and continue.
227+
const indexation = await withRetry(async () => {
228+
const result = await launchIndexAgentSearchWorkflow({
229+
workspaceId: restored.workspaceId,
230+
agentId: restored.agentId,
231+
});
232+
if (result.isErr()) {
233+
throw result.error;
234+
}
235+
});
228236
if (indexation.isErr()) {
229237
logger.error(
230238
{
@@ -329,14 +337,35 @@ export async function restoreEditorlessAgentAuthors({
329337
const auth = authsByWorkspaceId.get(agent.workspaceId);
330338
assert(auth);
331339
// Each agent needs its own transaction and advisory lock to serialize with live editor
332-
// changes. Concurrency is capped at four to bound database pressure.
333-
return restoreAuthor(auth, agent, logger);
340+
// changes. Catch failures here so every in-flight repair finishes its post-commit audit
341+
// attempt before the batch propagates an error to makeScript.
342+
try {
343+
return new Ok(await restoreAuthor(auth, agent, logger));
344+
} catch (error) {
345+
return new Err(normalizeError(error));
346+
}
334347
},
335348
{ concurrency: CONCURRENCY }
336349
);
337350

338-
for (const [index, restored] of results.entries()) {
351+
let firstError: Error | undefined;
352+
for (const [index, result] of results.entries()) {
339353
const discovered = agents[index];
354+
if (result.isErr()) {
355+
firstError ??= result.error;
356+
logger.error(
357+
{
358+
error: result.error,
359+
workspaceId: discovered.workspaceId,
360+
agentId: discovered.agentId,
361+
agentModelId: discovered.agentModelId,
362+
},
363+
"Failed to restore author as agent editor"
364+
);
365+
continue;
366+
}
367+
368+
const restored = result.value;
340369
if (!restored) {
341370
stats.agentsSkipped += 1;
342371
logger.info(
@@ -361,6 +390,9 @@ export async function restoreEditorlessAgentAuthors({
361390
"Restored author as agent editor"
362391
);
363392
}
393+
if (firstError) {
394+
throw firstError;
395+
}
364396
}
365397

366398
cursor = agents[agents.length - 1].agentModelId;

0 commit comments

Comments
 (0)