Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1986,15 +1986,44 @@ export function createApp() {
// longer prunes itself (see its own doc comment) precisely so a 500-installation fan-out below can't turn
// into 500 redundant global TTL-prune scans/deletes against the shared orb_relay_pending table.
await pruneRelayPending(c.env);
await Promise.all(installationIds.map((installationId) => enqueueConfigPushRelay(c.env, installationId, payload)));
// #8880: isolate each per-installation enqueue. A bare Promise.all let one target's throw abort the whole
// fan-out -- the audit event AND the response for the other ~499 installations were silently dropped, and
// there was no route-level try/catch to salvage them. Catch each target's failure inside its own fan-out
// task (mirroring pollPendingAprRepoTransfers' per-item isolation in src/orb/apr-repo-transfer.ts) so one
// bad row can't sink the batch, audit each failure so it is never silently swallowed, and still report the
// successful targets plus the partial failure to the caller.
const settled = await Promise.all(
installationIds.map(async (installationId): Promise<{ installationId: number; error?: string }> => {
try {
await enqueueConfigPushRelay(c.env, installationId, payload);
return { installationId };
} catch (error) {
return { installationId, error: errorMessage(error) };
}
}),
);
const failedInstallationIds: number[] = [];
for (const result of settled) {
if (result.error !== undefined) {
failedInstallationIds.push(result.installationId);
await recordAuditEvent(c.env, {
eventType: "operator.config_push_target_failed",
actor: identity.actor,
targetKey: `config_push#${parsed.data.pushId}#${result.installationId}`,
outcome: "error",
metadata: { installationId: result.installationId, pushId: parsed.data.pushId, message: result.error },
});
}
}
const succeededCount = installationIds.length - failedInstallationIds.length;
await recordAuditEvent(c.env, {
eventType: "operator.config_push_enqueued",
actor: identity.actor,
targetKey: `config_push#${parsed.data.pushId}`,
outcome: "completed",
metadata: { installationCount: installationIds.length, capability: payload.capability ?? null },
outcome: failedInstallationIds.length > 0 ? "error" : "completed",
metadata: { installationCount: installationIds.length, succeededCount, failedCount: failedInstallationIds.length, capability: payload.capability ?? null },
});
return c.json({ ok: true, pushId: parsed.data.pushId, installationCount: installationIds.length });
return c.json({ ok: true, pushId: parsed.data.pushId, installationCount: installationIds.length, succeededCount, failedInstallationIds });
});

// #5672 post-merge incident report, internal-operator side: same reporting path as the repo-scoped customer
Expand Down
60 changes: 56 additions & 4 deletions test/unit/routes-config-push.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { describe, expect, it, vi } from "vitest";

vi.mock("../../src/orb/relay", async (importOriginal) => {
const actual = await importOriginal<typeof import("../../src/orb/relay")>();
return { ...actual, pruneRelayPending: vi.fn(actual.pruneRelayPending) };
return { ...actual, pruneRelayPending: vi.fn(actual.pruneRelayPending), enqueueConfigPushRelay: vi.fn(actual.enqueueConfigPushRelay) };
});

import { createApp } from "../../src/api/routes";
import { createSessionForGitHubUser } from "../../src/auth/security";
import { pruneRelayPending } from "../../src/orb/relay";
import { enqueueConfigPushRelay, pruneRelayPending } from "../../src/orb/relay";
import { createTestEnv } from "../helpers/d1";

// #7522 (piece 1 of #4902's 3-piece design): the config-push write path. Mirrors routes-kill-switch.test.ts's
Expand Down Expand Up @@ -63,7 +63,7 @@ describe("config-push operator route (#7522)", () => {
env,
);
expect(res.status).toBe(200);
await expect(res.json()).resolves.toEqual({ ok: true, pushId: "push-1", installationCount: 2 });
await expect(res.json()).resolves.toEqual({ ok: true, pushId: "push-1", installationCount: 2, succeededCount: 2, failedInstallationIds: [] });

const rows = await relayRows(env);
expect(rows).toHaveLength(2);
Expand Down Expand Up @@ -164,6 +164,58 @@ describe("config-push operator route (#7522)", () => {
.first()) as { actor: string; outcome: string; metadata_json: string } | null;
expect(audit?.actor).toBe("jsonbored");
expect(audit?.outcome).toBe("completed");
expect(JSON.parse(audit?.metadata_json ?? "{}")).toEqual({ installationCount: 1, capability: null });
expect(JSON.parse(audit?.metadata_json ?? "{}")).toEqual({ installationCount: 1, succeededCount: 1, failedCount: 0, capability: null });
});

it("isolates a per-installation failure (#8880): the other targets still enqueue and the response + audit report the partial failure", async () => {
const app = createApp();
const env = createTestEnv();
const relay = await vi.importActual<typeof import("../../src/orb/relay")>("../../src/orb/relay");
// One target's DB write throws; every other target must still land, and the failure must be audited, not
// swallowed. Restore the real implementation afterwards so later tests keep exercising the genuine enqueue.
vi.mocked(enqueueConfigPushRelay).mockImplementation(async (e, installationId, p) => {
if (installationId === 222) throw new Error("simulated D1 write failure for 222");
await relay.enqueueConfigPushRelay(e, installationId, p);
});
try {
const res = await app.request(
"/v1/app/fleet/config-push",
{
method: "POST",
headers: apiHeaders(env),
body: JSON.stringify({ installationIds: [111, 222, 333], pushId: "push-fail", message: "x", capability: "y" }),
},
env,
);
expect(res.status).toBe(200);
await expect(res.json()).resolves.toEqual({
ok: true,
pushId: "push-fail",
installationCount: 3,
succeededCount: 2,
failedInstallationIds: [222],
});

// The two healthy targets still landed their rows -- one bad installation didn't sink the batch.
const rows = await relayRows(env);
expect(rows.map((r) => r.installation_id)).toEqual([111, 333]);

// The failing target was audited per-installation rather than silently dropped.
const failAudit = (await env.DB
.prepare("select target_key, outcome, metadata_json from audit_events where event_type = 'operator.config_push_target_failed'")
.first()) as { target_key: string; outcome: string; metadata_json: string } | null;
expect(failAudit?.outcome).toBe("error");
expect(failAudit?.target_key).toBe("config_push#push-fail#222");
expect(JSON.parse(failAudit?.metadata_json ?? "{}")).toEqual({ installationId: 222, pushId: "push-fail", message: "simulated D1 write failure for 222" });

// The whole-push summary audit still fired and reports the partial failure (outcome 'error', counts split).
const summary = (await env.DB
.prepare("select outcome, metadata_json from audit_events where event_type = 'operator.config_push_enqueued'")
.first()) as { outcome: string; metadata_json: string } | null;
expect(summary?.outcome).toBe("error");
expect(JSON.parse(summary?.metadata_json ?? "{}")).toEqual({ installationCount: 3, succeededCount: 2, failedCount: 1, capability: "y" });
} finally {
vi.mocked(enqueueConfigPushRelay).mockImplementation(relay.enqueueConfigPushRelay);
}
});
});