vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-07-14 03:26:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(slack): warn on disabled channel drops (#105790)

Browse Source
This commit is contained in:
Peter Steinberger
2026-07-12 17:50:14 -07:00
committed by GitHub
parent 462edb3373
commit fe0a6db2b4
5 changed files with 273 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/concepts/qa-e2e-automation.md
View File

@@ -612,6 +612,8 @@ Imperative Slack scenarios (`extensions/qa-lab/src/live-transports/slack/slack-l
- `slack-canary`
- `slack-mention-gating`
- `slack-allowlist-block`
- `slack-channel-disabled-warning` - opt-in real-Slack probe that confirms a
configured disabled channel emits a structured warning without replying.
- `slack-top-level-reply-shape`
- `slack-restart-resume`
- `slack-progress-commentary-true`, `slack-progress-commentary-false`,

59
extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts
View File

@@ -112,9 +112,11 @@ describe("Slack live QA runtime helpers", () => {
"slack-approval-plugin-native",
"slack-codex-approval-exec-native",
"slack-codex-approval-plugin-native",
"slack-channel-disabled-warning",
])
.map((scenario) => scenario.id),
).toEqual([
"slack-channel-disabled-warning",
"slack-progress-commentary-true",
"slack-progress-commentary-false",
"slack-progress-commentary-omitted",
@@ -144,6 +146,9 @@ describe("Slack live QA runtime helpers", () => {
expect(testing.findScenario().map((scenario) => scenario.id)).not.toContain(
"slack-progress-commentary-true",
);
expect(testing.findScenario().map((scenario) => scenario.id)).not.toContain(
"slack-channel-disabled-warning",
);
expect(testing.findScenario(["slack-codex-approval-exec-native"])[0]?.forcedRuntime).toBe(
"codex",
);
@@ -269,6 +274,7 @@ describe("Slack live QA runtime helpers", () => {
driverBotUserId: "U999999999",
overrides: {
allowFrom: ["U_NEVER_ALLOWED"],
channelEnabled: false,
users: ["U_NEVER_ALLOWED"],
},
sutAccountId: "sut",
@@ -279,9 +285,62 @@ describe("Slack live QA runtime helpers", () => {
const account = cfg.channels?.slack?.accounts?.sut;
expect(account?.allowFrom).toEqual(["U_NEVER_ALLOWED"]);
expect(account?.channels?.C123456789?.enabled).toBe(false);
expect(account?.channels?.C123456789?.users).toEqual(["U_NEVER_ALLOWED"]);
});
it("configures and verifies the disabled-channel warning scenario", async () => {
const scenario = testing.findScenario(["slack-channel-disabled-warning"])[0];
expect(scenario?.configOverrides?.channelEnabled).toBe(false);
const run = scenario?.buildRun("U999999999");
const beforeRun = run && "beforeRun" in run ? run.beforeRun : undefined;
const afterNoReply = run && "afterNoReply" in run ? run.afterNoReply : undefined;
expect(beforeRun).toBeTypeOf("function");
expect(afterNoReply).toBeTypeOf("function");
const call = vi
.fn()
.mockResolvedValueOnce({ cursor: 12 })
.mockResolvedValueOnce({
lines: ["Slack channel denied by configuration channel_not_allowed channel_disabled"],
});
await beforeRun?.({
gateway: {
call,
},
} as never);
await expect(
afterNoReply?.({
gateway: {
call,
},
} as never),
).resolves.toBe("structured disabled-channel warning observed");
expect(call).toHaveBeenNthCalledWith(
1,
"logs.tail",
{ limit: 1, maxBytes: 32_000 },
{ timeoutMs: 20_000 },
);
expect(call).toHaveBeenCalledWith(
"logs.tail",
{ cursor: 12, limit: 200, maxBytes: 256_000 },
{ timeoutMs: 20_000 },
);
await expect(
afterNoReply?.({
gateway: {
call: vi.fn(async () => ({
lines: [
"Slack channel denied by configuration channel_not_allowed",
"channel_disabled",
],
})),
},
} as never),
).rejects.toThrow("did not emit the structured warning");
});
it("builds the Slack progress commentary true, false, omitted, and dedupe configs", () => {
const buildScenarioConfig = (scenarioId: string) => {
const scenario = testing.findScenario([scenarioId])[0];

70
extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts
View File

@@ -78,6 +78,7 @@ const SLACK_QA_APPROVAL_CHECKPOINT_DEFAULT_TIMEOUT_MS = 120_000;
const SLACK_QA_REACTION_VERIFY_TIMEOUT_MS = 15_000;
const SLACK_QA_NATIVE_DATA_VERIFY_TIMEOUT_MS = 15_000;
const SLACK_QA_INVALID_TABLE_DATA_ROW_COUNT = 101;
const SLACK_QA_LOG_TAIL_TIMEOUT_MS = 20_000;
const SLACK_QA_INVALID_TABLE_CAPTION = "QA invalid_blocks fallback";
const SLACK_QA_INVALID_TABLE_HEADERS = ["Row", "Value"] as const;
const SLACK_QA_CHART_TITLE = "QA latency trend";
@@ -139,6 +140,7 @@ type SlackQaScenarioId =
| "slack-codex-approval-exec-native"
| "slack-codex-approval-plugin-native"
| "slack-chart-presentation-native"
| "slack-channel-disabled-warning"
| "slack-mention-gating"
| "slack-progress-commentary-false"
| "slack-progress-commentary-omitted"
@@ -178,10 +180,12 @@ function resolveSlackQaSutAccountId(value?: string) {
}
type SlackQaMessageScenarioRun = {
afterNoReply?: (context: SlackQaScenarioContext) => Promise<string | void>;
kind?: "message";
expectReply: boolean;
input: string;
matchText: string;
preserveGatewayDebug?: boolean;
settleObservedMs?: number;
verify?: (message: SlackMessage, context: { requestThreadTs: string; sentTs: string }) => void;
verifyObserved?: (params: {
@@ -245,6 +249,7 @@ type SlackQaBeforeRunResult =
type SlackQaConfigOverrides = {
allowFrom?: string[];
channelEnabled?: boolean;
approvals?: {
exec?: boolean;
plugin?: boolean;
@@ -666,6 +671,52 @@ const SLACK_QA_SCENARIOS: SlackQaScenarioDefinition[] = [
};
},
},
{
id: "slack-channel-disabled-warning",
title: "Slack disabled channel warns and does not trigger",
timeoutMs: 8_000,
defaultEnabled: false,
configOverrides: { channelEnabled: false },
buildRun: (sutUserId) => {
const marker = `SLACK_QA_DISABLED_${randomUUID().slice(0, 8).toUpperCase()}`;
let logCursor = 0;
return {
expectReply: false,
input: `<@${sutUserId}> reply with only this exact marker: ${marker}`,
matchText: marker,
preserveGatewayDebug: true,
beforeRun: async ({ gateway }) => {
const gatewayLogTail = (await gateway.call(
"logs.tail",
{ limit: 1, maxBytes: 32_000 },
{ timeoutMs: SLACK_QA_LOG_TAIL_TIMEOUT_MS },
)) as { cursor?: unknown };
logCursor = typeof gatewayLogTail.cursor === "number" ? gatewayLogTail.cursor : 0;
},
afterNoReply: async ({ gateway }) => {
const gatewayLogTail = (await gateway.call(
"logs.tail",
{ cursor: logCursor, limit: 200, maxBytes: 256_000 },
{ timeoutMs: SLACK_QA_LOG_TAIL_TIMEOUT_MS },
)) as { lines?: unknown };
const gatewayLogLines = Array.isArray(gatewayLogTail.lines)
? gatewayLogTail.lines.filter((line): line is string => typeof line === "string")
: [];
const expectedFields = [
"Slack channel denied by configuration",
"channel_not_allowed",
"channel_disabled",
];
if (
!gatewayLogLines.some((line) => expectedFields.every((field) => line.includes(field)))
) {
throw new Error("disabled Slack channel did not emit the structured warning");
}
return "structured disabled-channel warning observed";
},
};
},
},
{
id: "slack-top-level-reply-shape",
standardId: "top-level-reply-shape",
@@ -1246,7 +1297,7 @@ function buildSlackQaConfig(
...(execApprovalsConfig ? { execApprovals: execApprovalsConfig } : {}),
channels: {
[params.channelId]: {
enabled: true,
enabled: params.overrides?.channelEnabled ?? true,
requireMention: true,
allowBots: true,
users: params.overrides?.users ?? [params.driverBotUserId],
@@ -3486,13 +3537,26 @@ export async function runSlackQaLive(params: {
sutIdentity,
timeoutMs: scenario.timeoutMs,
});
const afterNoReplyDetails = await scenarioRun.afterNoReply?.({
...baseScenarioContext,
sentTs: sent.ts,
});
if (scenarioRun.preserveGatewayDebug) {
preserveAttemptGatewayDebug = true;
preservedGatewayDebugArtifacts = true;
}
scenarioResults.push({
id: scenario.id,
title: scenario.title,
standardId: scenario.standardId,
status: "pass",
details:
scenarioAttempt > 1 ? `no reply; retried ${scenarioAttempt - 1}x` : "no reply",
details: [
"no reply",
afterNoReplyDetails,
scenarioAttempt > 1 ? `retried ${scenarioAttempt - 1}x` : undefined,
]
.filter(Boolean)
.join("; "),
});
}
break;

54
extensions/slack/src/monitor/context.ts
View File

@@ -64,6 +64,8 @@ type SlackChannelCacheEntry = {
const SLACK_ASSISTANT_THREAD_CONTEXT_METADATA_EVENT = "assistant_thread_context";
const SLACK_CHANNEL_CACHE_MAX_ENTRIES = 1024;
const SLACK_CHANNEL_DENIAL_WARNING_TTL_MS = 5 * 60_000;
const SLACK_CHANNEL_DENIAL_WARNING_MAX_ENTRIES = 1024;
export function buildSlackAssistantThreadMetadata(
context: Omit<SlackAssistantThreadContext, "updatedAt">,
@@ -266,6 +268,11 @@ export function createSlackMonitorContext(params: {
const channelCache = new Map<string, SlackChannelCacheEntry>();
const userCache = new Map<string, { name?: string }>();
const seenMessages = createDedupeCache({ ttlMs: 60_000, maxSize: 500 });
// Rate-limit active denials while retaining periodic evidence; bound keys against config churn.
const channelDenialWarnings = createDedupeCache({
ttlMs: SLACK_CHANNEL_DENIAL_WARNING_TTL_MS,
maxSize: SLACK_CHANNEL_DENIAL_WARNING_MAX_ENTRIES,
});
const assistantThreadContexts = new Map<string, SlackAssistantThreadContext>();
let lastAssistantContextCleanupAt = Date.now();
@@ -660,26 +667,43 @@ export function createSlackMonitorContext(params: {
const channelMatchMeta = formatAllowlistMatchMeta(channelConfig);
const channelAllowed = channelConfig?.allowed !== false;
const channelAllowlistConfigured = hasChannelAllowlistConfig;
if (
!isSlackChannelAllowedByPolicy({
groupPolicy: params.groupPolicy,
channelAllowlistConfigured,
channelAllowed,
})
) {
const allowedByPolicy = isSlackChannelAllowedByPolicy({
groupPolicy: params.groupPolicy,
channelAllowlistConfigured,
channelAllowed,
});
const explicitlyDisabled =
params.groupPolicy !== "disabled" &&
channelConfig?.allowed === false &&
channelConfig.matchSource !== undefined;
// Open policy still honors an explicit room disable; unlisted rooms remain open.
const shouldDrop = !allowedByPolicy || (params.groupPolicy === "open" && explicitlyDisabled);
if (shouldDrop) {
if (explicitlyDisabled) {
const reason = "channel_not_allowed";
const warningKey = `${params.accountId}:${p.channelId}:${reason}`;
if (!channelDenialWarnings.peek(warningKey)) {
channelDenialWarnings.check(warningKey);
logger.warn(
{
provider: "slack",
accountId: params.accountId,
channelId: p.channelId,
reason,
cause: "channel_disabled",
groupPolicy: params.groupPolicy,
matchSource: channelConfig.matchSource,
matchKey: channelConfig.matchKey,
},
"Slack channel denied by configuration",
);
}
}
logVerbose(
`slack: drop channel ${p.channelId} (groupPolicy=${params.groupPolicy}, ${channelMatchMeta})`,
);
return false;
}
// When groupPolicy is "open", only block channels that are EXPLICITLY denied
// (i.e., have a matching config entry with allow:false). Channels not in the
// config (matchSource undefined) should be allowed under open policy.
const hasExplicitConfig = Boolean(channelConfig?.matchSource);
if (!channelAllowed && (params.groupPolicy !== "open" || hasExplicitConfig)) {
logVerbose(`slack: drop channel ${p.channelId} (${channelMatchMeta})`);
return false;
}
logVerbose(`slack: allow channel ${p.channelId} (${channelMatchMeta})`);
}

107
extensions/slack/src/monitor/monitor.test.ts
View File

@@ -2,7 +2,7 @@
import type { App } from "@slack/bolt";
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import { resolveSlackChannelConfig } from "./channel-config.js";
import { createSlackMonitorContext, normalizeSlackChannelType } from "./context.js";
@@ -375,6 +375,111 @@ describe("isChannelAllowed with groupPolicy and channelsConfig", () => {
expect(ctx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(true);
});
it("warns once per explicitly disabled direct channel", () => {
const ctx = createSlackMonitorContext({
...baseParams(),
accountId: "work",
groupPolicy: "open",
channelsConfig: {
C_DENIED: { enabled: false },
C_OTHER: { enabled: false },
},
});
const warnSpy = vi.spyOn(ctx.logger, "warn").mockImplementation(() => undefined);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
expect(ctx.isChannelAllowed({ channelId: "C_OTHER", channelType: "channel" })).toBe(false);
expect(warnSpy).toHaveBeenCalledTimes(2);
expect(warnSpy).toHaveBeenNthCalledWith(
1,
{
provider: "slack",
accountId: "work",
channelId: "C_DENIED",
reason: "channel_not_allowed",
cause: "channel_disabled",
groupPolicy: "open",
matchSource: "direct",
matchKey: "C_DENIED",
},
"Slack channel denied by configuration",
);
});
it("repeats disabled-channel warnings on a fixed interval despite steady traffic", () => {
vi.useFakeTimers();
try {
const ctx = createSlackMonitorContext({
...baseParams(),
groupPolicy: "open",
channelsConfig: { C_DENIED: { enabled: false } },
});
const warnSpy = vi.spyOn(ctx.logger, "warn").mockImplementation(() => undefined);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
vi.advanceTimersByTime(4 * 60_000);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
expect(warnSpy).toHaveBeenCalledTimes(1);
vi.advanceTimersByTime(60_000);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
expect(warnSpy).toHaveBeenCalledTimes(2);
} finally {
vi.useRealTimers();
}
});
it("warns for wildcard disablement under allowlist policy", () => {
const ctx = createSlackMonitorContext({
...baseParams(),
groupPolicy: "allowlist",
channelsConfig: { "*": { enabled: false } },
});
const warnSpy = vi.spyOn(ctx.logger, "warn").mockImplementation(() => undefined);
expect(ctx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(false);
expect(warnSpy).toHaveBeenCalledExactlyOnceWith(
{
provider: "slack",
accountId: "default",
channelId: "C_DENIED",
reason: "channel_not_allowed",
cause: "channel_disabled",
groupPolicy: "allowlist",
matchSource: "wildcard",
matchKey: "*",
},
"Slack channel denied by configuration",
);
});
it("does not warn for allowlist misses or globally disabled groups", () => {
const allowlistCtx = createListedChannelsContext("allowlist");
const allowlistWarnSpy = vi
.spyOn(allowlistCtx.logger, "warn")
.mockImplementation(() => undefined);
expect(allowlistCtx.isChannelAllowed({ channelId: "C_UNLISTED", channelType: "channel" })).toBe(
false,
);
expect(allowlistWarnSpy).not.toHaveBeenCalled();
const disabledCtx = createSlackMonitorContext({
...baseParams(),
groupPolicy: "disabled",
channelsConfig: { C_DENIED: { enabled: false } },
});
const disabledWarnSpy = vi
.spyOn(disabledCtx.logger, "warn")
.mockImplementation(() => undefined);
expect(disabledCtx.isChannelAllowed({ channelId: "C_DENIED", channelType: "channel" })).toBe(
false,
);
expect(disabledWarnSpy).not.toHaveBeenCalled();
});
it("allows all channels when groupPolicy is open and channelsConfig is empty", () => {
const ctx = createSlackMonitorContext({
...baseParams(),