fix: resolve remaining PR test merge conflict

This commit is contained in:
Marc J Saint-jour 2026-03-12 18:57:02 -04:00
parent 30b5cbd181
commit dac4df18ce

View File

@ -171,7 +171,6 @@ vi.mock("../../acp/persistent-bindings.js", async () => {
};
});
import { buildConfiguredAcpSessionKey } from "../../acp/persistent-bindings.js";
import type { HandleCommandsParams } from "./commands-types.js";
import { buildCommandContext, handleCommands } from "./commands.js";
@ -224,7 +223,6 @@ beforeEach(() => {
params.nativeChannelId ?? params.to ?? params.channel ?? "unknown",
);
});
describe("handleCommands gating", () => {
it("blocks gated commands when disabled or not elevated-allowlisted", async () => {
const cases = typedCases<{
@ -269,6 +267,9 @@ describe("handleCommands gating", () => {
commands: { config: false, debug: false, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
}) as OpenClawConfig,
applyParams: (params: ReturnType<typeof buildParams>) => {
params.command.senderIsOwner = true;
},
expectedText: "/config is disabled",
},
{
@ -279,6 +280,9 @@ describe("handleCommands gating", () => {
commands: { config: false, debug: false, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
}) as OpenClawConfig,
applyParams: (params: ReturnType<typeof buildParams>) => {
params.command.senderIsOwner = true;
},
expectedText: "/debug is disabled",
},
{
@ -311,6 +315,9 @@ describe("handleCommands gating", () => {
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
},
applyParams: (params: ReturnType<typeof buildParams>) => {
params.command.senderIsOwner = true;
},
expectedText: "/config is disabled",
},
{
@ -327,6 +334,9 @@ describe("handleCommands gating", () => {
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
},
applyParams: (params: ReturnType<typeof buildParams>) => {
params.command.senderIsOwner = true;
},
expectedText: "/debug is disabled",
},
]);
@ -378,6 +388,122 @@ describe("/approve command", () => {
);
});
it("accepts Telegram command mentions for /approve", async () => {
const cfg = {
commands: { text: true },
channels: {
telegram: {
allowFrom: ["*"],
execApprovals: { enabled: true, approvers: ["123"], target: "dm" },
},
},
} as OpenClawConfig;
const params = buildParams("/approve@bot abc12345 allow-once", cfg, {
BotUsername: "bot",
Provider: "telegram",
Surface: "telegram",
SenderId: "123",
});
callGatewayMock.mockResolvedValue({ ok: true });
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Exec approval allow-once submitted");
expect(callGatewayMock).toHaveBeenCalledWith(
expect.objectContaining({
method: "exec.approval.resolve",
params: { id: "abc12345", decision: "allow-once" },
}),
);
});
it("rejects Telegram /approve mentions targeting a different bot", async () => {
const cfg = {
commands: { text: true },
channels: {
telegram: {
allowFrom: ["*"],
execApprovals: { enabled: true, approvers: ["123"], target: "dm" },
},
},
} as OpenClawConfig;
const params = buildParams("/approve@otherbot abc12345 allow-once", cfg, {
BotUsername: "bot",
Provider: "telegram",
Surface: "telegram",
SenderId: "123",
});
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("targets a different Telegram bot");
expect(callGatewayMock).not.toHaveBeenCalled();
});
it("surfaces unknown or expired approval id errors", async () => {
const cfg = {
commands: { text: true },
channels: {
telegram: {
allowFrom: ["*"],
execApprovals: { enabled: true, approvers: ["123"], target: "dm" },
},
},
} as OpenClawConfig;
const params = buildParams("/approve abc12345 allow-once", cfg, {
Provider: "telegram",
Surface: "telegram",
SenderId: "123",
});
callGatewayMock.mockRejectedValue(new Error("unknown or expired approval id"));
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("unknown or expired approval id");
});
it("rejects Telegram /approve when telegram exec approvals are disabled", async () => {
const cfg = {
commands: { text: true },
channels: { telegram: { allowFrom: ["*"] } },
} as OpenClawConfig;
const params = buildParams("/approve abc12345 allow-once", cfg, {
Provider: "telegram",
Surface: "telegram",
SenderId: "123",
});
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Telegram exec approvals are not enabled");
expect(callGatewayMock).not.toHaveBeenCalled();
});
it("rejects Telegram /approve from non-approvers", async () => {
const cfg = {
commands: { text: true },
channels: {
telegram: {
allowFrom: ["*"],
execApprovals: { enabled: true, approvers: ["999"], target: "dm" },
},
},
} as OpenClawConfig;
const params = buildParams("/approve abc12345 allow-once", cfg, {
Provider: "telegram",
Surface: "telegram",
SenderId: "123",
});
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("not authorized to approve");
expect(callGatewayMock).not.toHaveBeenCalled();
});
it("rejects gateway clients without approvals scope", async () => {
const cfg = {
commands: { text: true },
@ -1004,6 +1130,36 @@ describe("extractMessageText", () => {
});
});
describe("handleCommands /config owner gating", () => {
it("blocks /config show from authorized non-owner senders", async () => {
const cfg = {
commands: { config: true, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
const params = buildParams("/config show", cfg);
params.command.senderIsOwner = false;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply).toBeUndefined();
});
it("keeps /config show working for owners", async () => {
const cfg = {
commands: { config: true, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
readConfigFileSnapshotMock.mockResolvedValueOnce({
valid: true,
parsed: { messages: { ackreaction: ":)" } },
});
const params = buildParams("/config show messages.ackReaction", cfg);
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Config messages.ackreaction");
});
});
describe("handleCommands /config configWrites gating", () => {
it("blocks /config set when channel config writes are disabled", async () => {
const cfg = {
@ -1011,11 +1167,60 @@ describe("handleCommands /config configWrites gating", () => {
channels: { whatsapp: { allowFrom: ["*"], configWrites: false } },
} as OpenClawConfig;
const params = buildParams('/config set messages.ackReaction=":)"', cfg);
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Config writes are disabled");
});
it("blocks /config set when the target account disables writes", async () => {
const previousWriteCount = writeConfigFileMock.mock.calls.length;
const cfg = {
commands: { config: true, text: true },
channels: {
telegram: {
configWrites: true,
accounts: {
work: { configWrites: false, enabled: true },
},
},
},
} as OpenClawConfig;
const params = buildPolicyParams(
"/config set channels.telegram.accounts.work.enabled=false",
cfg,
{
AccountId: "default",
Provider: "telegram",
Surface: "telegram",
},
);
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("channels.telegram.accounts.work.configWrites=true");
expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount);
});
it("blocks ambiguous channel-root /config writes from channel commands", async () => {
const previousWriteCount = writeConfigFileMock.mock.calls.length;
const cfg = {
commands: { config: true, text: true },
channels: { telegram: { configWrites: true } },
} as OpenClawConfig;
const params = buildPolicyParams('/config set channels.telegram={"enabled":false}', cfg, {
Provider: "telegram",
Surface: "telegram",
});
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain(
"cannot replace channels, channel roots, or accounts collections",
);
expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount);
});
it("blocks /config set from gateway clients without operator.admin", async () => {
const cfg = {
commands: { config: true, text: true },
@ -1026,6 +1231,7 @@ describe("handleCommands /config configWrites gating", () => {
GatewayClientScopes: ["operator.write"],
});
params.command.channel = INTERNAL_MESSAGE_CHANNEL;
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("requires operator.admin");
@ -1045,6 +1251,7 @@ describe("handleCommands /config configWrites gating", () => {
GatewayClientScopes: ["operator.write"],
});
params.command.channel = INTERNAL_MESSAGE_CHANNEL;
params.command.senderIsOwner = false;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Config messages.ackreaction");
@ -1068,11 +1275,82 @@ describe("handleCommands /config configWrites gating", () => {
GatewayClientScopes: ["operator.write", "operator.admin"],
});
params.command.channel = INTERNAL_MESSAGE_CHANNEL;
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(writeConfigFileMock).toHaveBeenCalledOnce();
expect(result.reply?.text).toContain("Config updated");
});
it("keeps /config set working for gateway operator.admin on protected account paths", async () => {
readConfigFileSnapshotMock.mockResolvedValueOnce({
valid: true,
parsed: {
channels: {
telegram: {
accounts: {
work: { enabled: true, configWrites: false },
},
},
},
},
});
validateConfigObjectWithPluginsMock.mockImplementation((config: unknown) => ({
ok: true,
config,
}));
const params = buildParams(
"/config set channels.telegram.accounts.work.enabled=false",
{
commands: { config: true, text: true },
channels: {
telegram: {
accounts: {
work: { enabled: true, configWrites: false },
},
},
},
} as OpenClawConfig,
{
Provider: INTERNAL_MESSAGE_CHANNEL,
Surface: INTERNAL_MESSAGE_CHANNEL,
GatewayClientScopes: ["operator.write", "operator.admin"],
},
);
params.command.channel = INTERNAL_MESSAGE_CHANNEL;
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Config updated");
const written = writeConfigFileMock.mock.calls.at(-1)?.[0] as OpenClawConfig;
expect(written.channels?.telegram?.accounts?.work?.enabled).toBe(false);
});
});
describe("handleCommands /debug owner gating", () => {
it("blocks /debug show from authorized non-owner senders", async () => {
const cfg = {
commands: { debug: true, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
const params = buildParams("/debug show", cfg);
params.command.senderIsOwner = false;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply).toBeUndefined();
});
it("keeps /debug show working for owners", async () => {
const cfg = {
commands: { debug: true, text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
const params = buildParams("/debug show", cfg);
params.command.senderIsOwner = true;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("Debug overrides");
});
});
describe("handleCommands bash alias", () => {
@ -1225,6 +1503,35 @@ describe("handleCommands /allowlist", () => {
});
});
it("blocks config-targeted /allowlist edits when the target account disables writes", async () => {
const previousWriteCount = writeConfigFileMock.mock.calls.length;
const cfg = {
commands: { text: true, config: true },
channels: {
telegram: {
configWrites: true,
accounts: {
work: { configWrites: false, allowFrom: ["123"] },
},
},
},
} as OpenClawConfig;
readConfigFileSnapshotMock.mockResolvedValueOnce({
valid: true,
parsed: structuredClone(cfg),
});
const params = buildPolicyParams("/allowlist add dm --account work --config 789", cfg, {
AccountId: "default",
Provider: "telegram",
Surface: "telegram",
});
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("channels.telegram.accounts.work.configWrites=true");
expect(writeConfigFileMock.mock.calls.length).toBe(previousWriteCount);
});
it("removes default-account entries from scoped and legacy pairing stores", async () => {
removeChannelAllowFromStoreEntryMock
.mockResolvedValueOnce({
@ -1571,226 +1878,6 @@ describe("handleCommands hooks", () => {
});
});
describe("handleCommands ACP-bound /new and /reset", () => {
const discordChannelId = "1478836151241412759";
const buildDiscordBoundConfig = (): OpenClawConfig =>
({
commands: { text: true },
bindings: [
{
type: "acp",
agentId: "codex",
match: {
channel: "discord",
accountId: "default",
peer: {
kind: "channel",
id: discordChannelId,
},
},
acp: {
mode: "persistent",
},
},
],
channels: {
discord: {
allowFrom: ["*"],
guilds: { "1459246755253325866": { channels: { [discordChannelId]: {} } } },
},
},
}) as OpenClawConfig;
const buildDiscordBoundParams = (body: string) => {
const params = buildParams(body, buildDiscordBoundConfig(), {
Provider: "discord",
Surface: "discord",
OriginatingChannel: "discord",
AccountId: "default",
SenderId: "12345",
From: "discord:12345",
To: discordChannelId,
OriginatingTo: discordChannelId,
SessionKey: "agent:main:acp:binding:discord:default:feedface",
});
params.sessionKey = "agent:main:acp:binding:discord:default:feedface";
return params;
};
it("handles /new as ACP in-place reset for bound conversations", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const);
const result = await handleCommands(buildDiscordBoundParams("/new"));
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("ACP session reset in place");
expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1);
expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({
reason: "new",
});
});
it("continues with trailing prompt text after successful ACP-bound /new", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const);
const params = buildDiscordBoundParams("/new continue with deployment");
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply).toBeUndefined();
const mutableCtx = params.ctx as Record<string, unknown>;
expect(mutableCtx.BodyStripped).toBe("continue with deployment");
expect(mutableCtx.CommandBody).toBe("continue with deployment");
expect(mutableCtx.AcpDispatchTailAfterReset).toBe(true);
expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1);
});
it("handles /reset failures without falling back to normal session reset flow", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: false, error: "backend unavailable" });
const result = await handleCommands(buildDiscordBoundParams("/reset"));
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("ACP session reset failed");
expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1);
expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({
reason: "reset",
});
});
it("does not emit reset hooks when ACP reset fails", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: false, error: "backend unavailable" });
const spy = vi.spyOn(internalHooks, "triggerInternalHook").mockResolvedValue();
const result = await handleCommands(buildDiscordBoundParams("/reset"));
expect(result.shouldContinue).toBe(false);
expect(spy).not.toHaveBeenCalled();
spy.mockRestore();
});
it("keeps existing /new behavior for non-ACP sessions", async () => {
const cfg = {
commands: { text: true },
channels: { whatsapp: { allowFrom: ["*"] } },
} as OpenClawConfig;
const result = await handleCommands(buildParams("/new", cfg));
expect(result.shouldContinue).toBe(true);
expect(resetAcpSessionInPlaceMock).not.toHaveBeenCalled();
});
it("still targets configured ACP binding when runtime routing falls back to a non-ACP session", async () => {
const fallbackSessionKey = `agent:main:discord:channel:${discordChannelId}`;
const configuredAcpSessionKey = buildConfiguredAcpSessionKey({
channel: "discord",
accountId: "default",
conversationId: discordChannelId,
agentId: "codex",
mode: "persistent",
});
const params = buildDiscordBoundParams("/new");
params.sessionKey = fallbackSessionKey;
params.ctx.SessionKey = fallbackSessionKey;
params.ctx.CommandTargetSessionKey = fallbackSessionKey;
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("ACP session reset unavailable");
expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1);
expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({
sessionKey: configuredAcpSessionKey,
reason: "new",
});
});
it("emits reset hooks for the ACP session key when routing falls back to non-ACP session", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const);
const hookSpy = vi.spyOn(internalHooks, "triggerInternalHook").mockResolvedValue();
const fallbackSessionKey = `agent:main:discord:channel:${discordChannelId}`;
const configuredAcpSessionKey = buildConfiguredAcpSessionKey({
channel: "discord",
accountId: "default",
conversationId: discordChannelId,
agentId: "codex",
mode: "persistent",
});
const fallbackEntry = {
sessionId: "fallback-session-id",
sessionFile: "/tmp/fallback-session.jsonl",
} as SessionEntry;
const configuredEntry = {
sessionId: "configured-acp-session-id",
sessionFile: "/tmp/configured-acp-session.jsonl",
} as SessionEntry;
const params = buildDiscordBoundParams("/new");
params.sessionKey = fallbackSessionKey;
params.ctx.SessionKey = fallbackSessionKey;
params.ctx.CommandTargetSessionKey = fallbackSessionKey;
params.sessionEntry = fallbackEntry;
params.previousSessionEntry = fallbackEntry;
params.sessionStore = {
[fallbackSessionKey]: fallbackEntry,
[configuredAcpSessionKey]: configuredEntry,
};
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("ACP session reset in place");
expect(hookSpy).toHaveBeenCalledWith(
expect.objectContaining({
type: "command",
action: "new",
sessionKey: configuredAcpSessionKey,
context: expect.objectContaining({
sessionEntry: configuredEntry,
previousSessionEntry: configuredEntry,
}),
}),
);
hookSpy.mockRestore();
});
it("uses active ACP command target when conversation binding context is missing", async () => {
resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const);
const activeAcpTarget = "agent:codex:acp:binding:discord:default:feedface";
const params = buildParams(
"/new",
{
commands: { text: true },
channels: {
discord: {
allowFrom: ["*"],
},
},
} as OpenClawConfig,
{
Provider: "discord",
Surface: "discord",
OriginatingChannel: "discord",
AccountId: "default",
SenderId: "12345",
From: "discord:12345",
},
);
params.sessionKey = "discord:slash:12345";
params.ctx.SessionKey = "discord:slash:12345";
params.ctx.CommandSource = "native";
params.ctx.CommandTargetSessionKey = activeAcpTarget;
params.ctx.To = "user:12345";
params.ctx.OriginatingTo = "user:12345";
const result = await handleCommands(params);
expect(result.shouldContinue).toBe(false);
expect(result.reply?.text).toContain("ACP session reset in place");
expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1);
expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({
sessionKey: activeAcpTarget,
reason: "new",
});
});
});
describe("handleCommands context", () => {
it("returns expected details for /context commands", async () => {
const cfg = {