mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(discord): scope command-deploy cache by application id (#77367)
* fix(discord): scope command-deploy cache by application id Multi-bot Discord setups share a single command-deploy-cache.json under the state dir. Cache keys were unscoped (`global:reconcile`, `guild:<id>`), so a later account whose command set hashed identically to an earlier account would hit the shared hash and skip its own application's command reconcile entirely — Discord's Integration panel showed 'This application has no commands' for the secondary bot even though gateway connect, application id, and token were all valid. Scope every cache key with `app:<clientId>:` so each Discord application reconciles independently. Add regression tests covering: two applications with identical command sets each call REST against their own application; a single application with the same command set still hits the persisted cache; the on-disk cache JSON contains application-scoped keys. Fixes #77359. * fix(discord): merge on-disk hashes inside persistHashes to survive concurrent writes Codex follow-up on #77359 noted that server-channels.ts can start multiple Discord deployers concurrently, so two deployers that both load the cache file before either persists end up with the second writer overwriting the first writer's app-scoped key — defeating the rate-limit cache that the file exists to provide. Inside persistHashes, re-read the on-disk cache and merge it with our in-memory entries before the rename. Our in-memory entries always win on key collisions (we just produced them); on-disk entries we don't have in memory are preserved. Refresh in-memory state after the write so future writes from the same deployer also keep entries other deployers added. This is the lighter of the two repairs the codex review suggested (re-read/merge vs serialize writes); it covers the realistic case where one deployer writes before the other persists. Add a regression test that exercises the load-then-other-deployer-writes-then-persist sequence. * fix(discord): serialize command-deploy cache persists via in-process mutex Codex follow-up on #77367 noted: re-read-before-write inside persistHashes isn't enough — two deployers running persistHashes in true parallel can both read the same snapshot before either writes, and the later rename overwrites the earlier writer's app-scoped entries. Add a module-level Map<storePath, Promise<void>> mutex and wrap the read-merge-write cycle in withCachePersistLock so concurrent persists for the same on-disk path serialize. In-process is sufficient because Discord deployers only run inside the gateway process. New regression test fires three deployers via Promise.all on the same tick and asserts all three application-scoped entries survive — pre-fix this race lost at least one entry. * fix(discord): add override modifier on StaticCommand.description to satisfy strict TS Current main enables noImplicitOverride; the StaticCommand test helper re-declares the concrete BaseCommand.description property, which now requires an explicit 'override' modifier (TS4114). * test(discord): suppress typescript/unbound-method on vitest mock refs The createRest() helper returns vi.fn() handlers cast as RequestClient, so expect(rest.get).toHaveBeenCalledTimes(...) triggers typescript/unbound-method 12 times. File-level disable: these are vitest mock identities, not unbound class methods. * fix(discord): clean up command cache lock Signed-off-by: sallyom <somalley@redhat.com> --------- Signed-off-by: sallyom <somalley@redhat.com> Co-authored-by: sallyom <somalley@redhat.com>
This commit is contained in:
Shubhankar Tripathy
committed by
GitHub
GitHub
parent
d4fcc38696
commit
0bcabea9cc
@@ -1,7 +1,13 @@
|
||||
// Discord tests cover command deploy plugin behavior.
|
||||
import type { APIApplicationCommand } from "discord-api-types/v10";
|
||||
import { describe, expect, test } from "vitest";
|
||||
import { testing } from "./command-deploy.js";
|
||||
/* oxlint-disable typescript/unbound-method -- vitest mocks of RequestClient methods (createRest) intentionally expose vi.fn refs via `restA.get`/`.post`; not unbound class methods. */
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { ApplicationCommandType, type APIApplicationCommand } from "discord-api-types/v10";
|
||||
import { describe, expect, test, vi } from "vitest";
|
||||
import { DiscordCommandDeployer, testing } from "./command-deploy.js";
|
||||
import { BaseCommand } from "./commands.js";
|
||||
import type { RequestClient } from "./rest.js";
|
||||
|
||||
const { commandsEqual } = testing;
|
||||
|
||||
@@ -196,3 +202,332 @@ describe("commandsEqual", () => {
|
||||
expect(commandsEqual(current, desired)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Regression for #77359: when two Discord accounts share the same on-disk
|
||||
* deploy-cache file (the default in multi-bot setups) the persisted hash key
|
||||
* must be scoped by application/client id. Otherwise a later account whose
|
||||
* command set hashes the same as the first account's reuses the first
|
||||
* account's hash and skips reconciling its own Discord application — leaving
|
||||
* "This application has no commands" in the secondary bot's Integrations panel.
|
||||
*/
|
||||
describe("DiscordCommandDeployer cache scoping (multi-application)", () => {
|
||||
class StaticCommand extends BaseCommand {
|
||||
name: string;
|
||||
override description = "ping the bot";
|
||||
type = ApplicationCommandType.ChatInput;
|
||||
constructor(name: string) {
|
||||
super();
|
||||
this.name = name;
|
||||
}
|
||||
serializeOptions() {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function createRest(): RequestClient {
|
||||
return {
|
||||
get: vi.fn(async () => []),
|
||||
post: vi.fn(async () => undefined),
|
||||
patch: vi.fn(async () => undefined),
|
||||
put: vi.fn(async () => undefined),
|
||||
delete: vi.fn(async () => undefined),
|
||||
} as unknown as RequestClient;
|
||||
}
|
||||
|
||||
test("two applications with identical command sets each reconcile their own application", async () => {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const commands = [new StaticCommand("ping")];
|
||||
|
||||
const restA = createRest();
|
||||
const deployerA = new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restA,
|
||||
});
|
||||
await deployerA.deploy({ mode: "reconcile" });
|
||||
|
||||
const restB = createRest();
|
||||
const deployerB = new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restB,
|
||||
});
|
||||
await deployerB.deploy({ mode: "reconcile" });
|
||||
|
||||
// The first deploy issues a list + create against application "app-default".
|
||||
expect(restA.get).toHaveBeenCalledTimes(1);
|
||||
expect(restA.post).toHaveBeenCalledTimes(1);
|
||||
// The second deploy MUST also list + create against "app-secondary"; before
|
||||
// the fix it short-circuited on the shared `global:reconcile` hash and
|
||||
// never touched its own Discord application.
|
||||
expect(restB.get).toHaveBeenCalledTimes(1);
|
||||
expect(restB.post).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test("re-deploying the same application still hits the persisted cache", async () => {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const commands = [new StaticCommand("ping")];
|
||||
|
||||
const restFirst = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restFirst,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
const restSecond = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restSecond,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
expect(restFirst.get).toHaveBeenCalledTimes(1);
|
||||
expect(restFirst.post).toHaveBeenCalledTimes(1);
|
||||
// Same application, same command set, same hash file => skip reconcile.
|
||||
expect(restSecond.get).not.toHaveBeenCalled();
|
||||
expect(restSecond.post).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("persisted cache keys are namespaced by application id", async () => {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const commands = [new StaticCommand("ping")];
|
||||
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
const raw = await fs.readFile(hashStorePath, "utf8");
|
||||
const parsed = JSON.parse(raw) as { hashes: Record<string, string> };
|
||||
const keys = Object.keys(parsed.hashes);
|
||||
expect(keys).toContain("app:app-default:global:reconcile");
|
||||
expect(keys).toContain("app:app-secondary:global:reconcile");
|
||||
expect(keys).not.toContain("global:reconcile");
|
||||
});
|
||||
|
||||
test("successful deploy repairs a corrupt persisted cache file", async () => {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
await fs.writeFile(hashStorePath, "{not json", "utf8");
|
||||
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands: [new StaticCommand("ping")],
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
const raw = await fs.readFile(hashStorePath, "utf8");
|
||||
const parsed = JSON.parse(raw) as { hashes: Record<string, string> };
|
||||
expect(parsed.hashes).toHaveProperty("app:app-default:global:reconcile");
|
||||
});
|
||||
|
||||
test("a deployer that loaded an empty cache before another deployer's write preserves the other deployer's entries on persist", async () => {
|
||||
// Regression for the codex follow-up on PR #77367: `server-channels.ts`
|
||||
// can start multiple Discord deployers concurrently. Before the fix, a
|
||||
// deployer that loaded the (empty) cache file before another deployer's
|
||||
// first write would later overwrite it on its own `persistHashes()`,
|
||||
// serializing only its own in-memory `app:<id>:...` entry and dropping
|
||||
// the other deployer's entry. The current implementation re-reads the
|
||||
// on-disk hashes inside `persistHashes` and merges them with our
|
||||
// in-memory entries before the rename.
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const commands = [new StaticCommand("ping")];
|
||||
|
||||
// Deployer B starts first, loads the empty cache. Then deployer A
|
||||
// completes its full deploy + persist, writing `app:app-default:...` to
|
||||
// disk. When deployer B finally persists, it must merge in deployer A's
|
||||
// entry instead of overwriting it with just its own.
|
||||
const deployerB = new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
});
|
||||
// Trigger B's load of the (still missing) cache file by starting deploy
|
||||
// and immediately awaiting just enough to clear the load. The deploy
|
||||
// call awaits loadPersistedHashes inside putCommandSetIfChanged before
|
||||
// calling deploy(); to keep the seam minimal here, we just race the load
|
||||
// by running deployer A's full deploy in between.
|
||||
const deployerA = new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
});
|
||||
|
||||
// Step 1: A runs a full deploy (load -> reconcile -> persist) on the
|
||||
// initially missing cache file; result: file now has app-default entry.
|
||||
await deployerA.deploy({ mode: "reconcile" });
|
||||
|
||||
// Step 2: B runs its full deploy. Without the fix, B's persistHashes
|
||||
// would write only `app:app-secondary:...` and drop A's entry. With the
|
||||
// fix, B re-reads the on-disk file inside persistHashes, sees A's entry,
|
||||
// and merges it into the write so both keys survive.
|
||||
await deployerB.deploy({ mode: "reconcile" });
|
||||
|
||||
const raw = await fs.readFile(hashStorePath, "utf8");
|
||||
const parsed = JSON.parse(raw) as { hashes: Record<string, string> };
|
||||
const keys = Object.keys(parsed.hashes);
|
||||
expect(keys).toContain("app:app-default:global:reconcile");
|
||||
expect(keys).toContain("app:app-secondary:global:reconcile");
|
||||
|
||||
// And subsequent restarts must still hit the cache for both apps,
|
||||
// proving the rate-limit protection survived the concurrent write.
|
||||
const restA = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restA,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
const restB = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restB,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
expect(restA.get).not.toHaveBeenCalled();
|
||||
expect(restA.post).not.toHaveBeenCalled();
|
||||
expect(restB.get).not.toHaveBeenCalled();
|
||||
expect(restB.post).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("truly parallel deployers serialize cache writes via the per-path mutex (codex follow-up on #77367)", async () => {
|
||||
// Codex follow-up on PR #77367: re-read-before-write alone isn't enough
|
||||
// when two deployers run `persistHashes` in real parallel — both can read
|
||||
// the same snapshot before either writes. The in-process per-path mutex
|
||||
// around the read-merge-write cycle makes the operation atomic.
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const commands = [new StaticCommand("ping")];
|
||||
|
||||
// Run BOTH deploys with Promise.all on the SAME process tick — pre-fix,
|
||||
// both `persistHashes` calls would race on read-then-rename and one
|
||||
// writer's `app:<id>:...` entry would be lost.
|
||||
const restA = createRest();
|
||||
const restB = createRest();
|
||||
const restC = createRest();
|
||||
await Promise.all([
|
||||
new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restA,
|
||||
}).deploy({ mode: "reconcile" }),
|
||||
new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restB,
|
||||
}).deploy({ mode: "reconcile" }),
|
||||
new DiscordCommandDeployer({
|
||||
clientId: "app-tertiary",
|
||||
commands,
|
||||
hashStorePath,
|
||||
rest: () => restC,
|
||||
}).deploy({ mode: "reconcile" }),
|
||||
]);
|
||||
|
||||
const raw = await fs.readFile(hashStorePath, "utf8");
|
||||
const parsed = JSON.parse(raw) as { hashes: Record<string, string> };
|
||||
const keys = Object.keys(parsed.hashes);
|
||||
// All three apps' entries must survive — pre-fix, one or two would be
|
||||
// lost to the race.
|
||||
expect(keys).toContain("app:app-default:global:reconcile");
|
||||
expect(keys).toContain("app:app-secondary:global:reconcile");
|
||||
expect(keys).toContain("app:app-tertiary:global:reconcile");
|
||||
});
|
||||
|
||||
test("parallel changed deploys preserve fresher sibling cache entries", async () => {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-discord-multi-app-"));
|
||||
const hashStorePath = path.join(dir, "command-deploy-cache.json");
|
||||
const oldCommands = [new StaticCommand("ping")];
|
||||
const newCommands = [new StaticCommand("status")];
|
||||
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands: oldCommands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
}).deploy({ mode: "reconcile" });
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands: oldCommands,
|
||||
hashStorePath,
|
||||
rest: () => createRest(),
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
let postStarts = 0;
|
||||
let releasePosts: () => void = () => {};
|
||||
const bothPostsStarted = new Promise<void>((resolve) => {
|
||||
releasePosts = resolve;
|
||||
});
|
||||
function createWaitingRest(): RequestClient {
|
||||
const rest = createRest();
|
||||
rest.post = vi.fn(async () => {
|
||||
postStarts += 1;
|
||||
if (postStarts === 2) {
|
||||
releasePosts();
|
||||
}
|
||||
await bothPostsStarted;
|
||||
}) as RequestClient["post"];
|
||||
return rest;
|
||||
}
|
||||
|
||||
await Promise.all([
|
||||
new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands: newCommands,
|
||||
hashStorePath,
|
||||
rest: () => createWaitingRest(),
|
||||
}).deploy({ mode: "reconcile" }),
|
||||
new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands: newCommands,
|
||||
hashStorePath,
|
||||
rest: () => createWaitingRest(),
|
||||
}).deploy({ mode: "reconcile" }),
|
||||
]);
|
||||
|
||||
const restA = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-default",
|
||||
commands: newCommands,
|
||||
hashStorePath,
|
||||
rest: () => restA,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
const restB = createRest();
|
||||
await new DiscordCommandDeployer({
|
||||
clientId: "app-secondary",
|
||||
commands: newCommands,
|
||||
hashStorePath,
|
||||
rest: () => restB,
|
||||
}).deploy({ mode: "reconcile" });
|
||||
|
||||
expect(restA.get).not.toHaveBeenCalled();
|
||||
expect(restA.post).not.toHaveBeenCalled();
|
||||
expect(restB.get).not.toHaveBeenCalled();
|
||||
expect(restB.post).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,8 +21,42 @@ export type DeployCommandOptions = {
|
||||
|
||||
type SerializedCommand = ReturnType<BaseCommand["serialize"]>;
|
||||
|
||||
/**
|
||||
* Per-`command-deploy-cache.json` path async mutex. `server-channels.ts` can
|
||||
* start several Discord deployers concurrently in the same Node.js process;
|
||||
* each one shares the same on-disk cache file. Without this lock, two
|
||||
* deployers can run `persistHashes` in parallel, both read the same on-disk
|
||||
* snapshot before either writes, and the later `rename` then overwrites the
|
||||
* earlier writer's entries — defeating the rate-limit cache.
|
||||
*
|
||||
* This is an in-process lock; cross-process serialization would need an OS
|
||||
* file lock. Discord deployers only run inside the gateway process, so an
|
||||
* in-process mutex is sufficient for the documented concurrency surface.
|
||||
*/
|
||||
const cachePersistLocks = new Map<string, Promise<void>>();
|
||||
|
||||
async function withCachePersistLock<T>(storePath: string, fn: () => Promise<T>): Promise<T> {
|
||||
const previous = cachePersistLocks.get(storePath) ?? Promise.resolve();
|
||||
let release: () => void = () => {};
|
||||
const next = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
const chained = previous.then(() => next);
|
||||
cachePersistLocks.set(storePath, chained);
|
||||
try {
|
||||
await previous;
|
||||
return await fn();
|
||||
} finally {
|
||||
release();
|
||||
if (cachePersistLocks.get(storePath) === chained) {
|
||||
cachePersistLocks.delete(storePath);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export class DiscordCommandDeployer {
|
||||
private readonly hashes = new Map<string, string>();
|
||||
private readonly pendingHashes = new Map<string, string>();
|
||||
private hashesLoaded = false;
|
||||
|
||||
constructor(
|
||||
@@ -45,7 +79,7 @@ export class DiscordCommandDeployer {
|
||||
const serializedGlobal = globalCommands.map((command) => command.serialize());
|
||||
for (const [guildId, entries] of groupGuildCommands(commands)) {
|
||||
await this.putCommandSetIfChanged(
|
||||
`guild:${guildId}`,
|
||||
this.scopedCacheKey(`guild:${guildId}`),
|
||||
entries,
|
||||
async () => {
|
||||
await overwriteGuildApplicationCommands(
|
||||
@@ -62,7 +96,7 @@ export class DiscordCommandDeployer {
|
||||
for (const guildId of this.params.devGuilds) {
|
||||
const entries = commands.map((command) => command.serialize());
|
||||
await this.putCommandSetIfChanged(
|
||||
`dev-guild:${guildId}`,
|
||||
this.scopedCacheKey(`dev-guild:${guildId}`),
|
||||
entries,
|
||||
async () => {
|
||||
await overwriteGuildApplicationCommands(
|
||||
@@ -79,7 +113,7 @@ export class DiscordCommandDeployer {
|
||||
}
|
||||
if (options.mode !== "overwrite") {
|
||||
await this.putCommandSetIfChanged(
|
||||
"global:reconcile",
|
||||
this.scopedCacheKey("global:reconcile"),
|
||||
serializedGlobal,
|
||||
async () => {
|
||||
await this.reconcileGlobalCommands(serializedGlobal);
|
||||
@@ -89,7 +123,7 @@ export class DiscordCommandDeployer {
|
||||
return { mode: "reconcile" as const, usedDevGuilds: false };
|
||||
}
|
||||
await this.putCommandSetIfChanged(
|
||||
"global:overwrite",
|
||||
this.scopedCacheKey("global:overwrite"),
|
||||
serializedGlobal,
|
||||
async () => {
|
||||
await overwriteApplicationCommands(this.rest, this.params.clientId, serializedGlobal);
|
||||
@@ -99,6 +133,17 @@ export class DiscordCommandDeployer {
|
||||
return { mode: "overwrite" as const, usedDevGuilds: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* Scope cache keys by Discord application id so multi-bot setups that share a
|
||||
* single deploy-cache file still reconcile each application separately. The
|
||||
* prior unscoped `global:reconcile` / `guild:<id>` keys let a later account
|
||||
* with an identical command set reuse the first account's hash and skip its
|
||||
* own application's reconcile entirely (#77359).
|
||||
*/
|
||||
private scopedCacheKey(suffix: string): string {
|
||||
return `app:${this.params.clientId}:${suffix}`;
|
||||
}
|
||||
|
||||
private async reconcileGlobalCommands(desired: SerializedCommand[]) {
|
||||
const existing = await this.getCommands();
|
||||
const existingByKey = new Map(existing.map((command) => [stableCommandKey(command), command]));
|
||||
@@ -135,6 +180,7 @@ export class DiscordCommandDeployer {
|
||||
}
|
||||
await deploy();
|
||||
this.hashes.set(key, hash);
|
||||
this.pendingHashes.set(key, hash);
|
||||
await this.persistHashes();
|
||||
}
|
||||
|
||||
@@ -169,18 +215,62 @@ export class DiscordCommandDeployer {
|
||||
if (!storePath) {
|
||||
return;
|
||||
}
|
||||
// Serialize concurrent persists for the same on-disk path. The earlier
|
||||
// "re-read inside persistHashes" merge alone is not enough — two
|
||||
// deployers running `persistHashes` in true parallel would both read the
|
||||
// same snapshot before either writes, and the later `rename` would still
|
||||
// overwrite the earlier one's `app:<id>:...` entries. The mutex makes the
|
||||
// read-merge-write cycle atomic for in-process callers.
|
||||
await withCachePersistLock(storePath, async () => {
|
||||
await this.persistHashesLocked(storePath);
|
||||
});
|
||||
}
|
||||
|
||||
private async persistHashesLocked(storePath: string): Promise<void> {
|
||||
try {
|
||||
await privateFileStore(path.dirname(storePath)).writeJson(
|
||||
path.basename(storePath),
|
||||
// Re-read the on-disk hashes immediately before writing and merge only
|
||||
// keys this deployer changed. Previously loaded hashes can be stale when
|
||||
// sibling deployers update the same file, so on-disk wins for untouched
|
||||
// keys while pending keys win because this deployer just produced them.
|
||||
const storeFile = path.basename(storePath);
|
||||
const fileStore = privateFileStore(path.dirname(storePath));
|
||||
const merged = new Map<string, string>();
|
||||
let onDisk: { hashes?: unknown } | null = null;
|
||||
try {
|
||||
onDisk = await fileStore.readJsonIfExists<{
|
||||
hashes?: unknown;
|
||||
}>(storeFile);
|
||||
} catch {
|
||||
// A corrupt cache should not become permanent. Treat the re-read as
|
||||
// empty and replace it with the fresh pending hashes after deploy.
|
||||
}
|
||||
if (onDisk?.hashes && typeof onDisk.hashes === "object") {
|
||||
for (const [key, value] of Object.entries(onDisk.hashes)) {
|
||||
if (typeof value === "string" && key.trim() && value.trim()) {
|
||||
merged.set(key, value);
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const [key, value] of this.pendingHashes.entries()) {
|
||||
merged.set(key, value);
|
||||
}
|
||||
await fileStore.writeJson(
|
||||
storeFile,
|
||||
{
|
||||
version: 1,
|
||||
updatedAt: new Date().toISOString(),
|
||||
hashes: Object.fromEntries(
|
||||
[...this.hashes.entries()].toSorted(([left], [right]) => left.localeCompare(right)),
|
||||
[...merged.entries()].toSorted(([left], [right]) => left.localeCompare(right)),
|
||||
),
|
||||
},
|
||||
{ trailingNewline: true },
|
||||
);
|
||||
// Refresh in-memory state so future writes from the same deployer also
|
||||
// see entries that other deployers added concurrently.
|
||||
for (const [key, value] of merged.entries()) {
|
||||
this.hashes.set(key, value);
|
||||
}
|
||||
this.pendingHashes.clear();
|
||||
} catch {
|
||||
// The cache is only an optimization to avoid redundant Discord writes.
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user