mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-27 09:02:15 +00:00
docs(mattermost): document user-first ID resolution and explicit prefix guidance
- mattermost.md: replace 'Bare IDs are treated as channels' with accurate description of user-first resolution behavior. - cron-jobs.md: add note that bare 26-char Mattermost IDs use user-first resolution; recommend explicit prefixes for deterministic routing.
This commit is contained in:
committed by
Muhammed Mukhthar CM
parent
f03a1ede1b
commit
b8b640ecef
@@ -262,6 +262,7 @@ If `delivery.channel` or `delivery.to` is omitted, cron can fall back to the mai
|
||||
Target format reminders:
|
||||
|
||||
- Slack/Discord/Mattermost (plugin) targets should use explicit prefixes (e.g. `channel:<id>`, `user:<id>`) to avoid ambiguity.
|
||||
Mattermost bare 26-char IDs are resolved **user-first** (DM if user exists, channel otherwise) — use `user:<id>` or `channel:<id>` for deterministic routing.
|
||||
- Telegram topics should use the `:topic:` form (see below).
|
||||
|
||||
#### Telegram delivery targets (topics / forum threads)
|
||||
|
||||
@@ -153,7 +153,14 @@ Use these target formats with `openclaw message send` or cron/webhooks:
|
||||
- `user:<id>` for a DM
|
||||
- `@username` for a DM (resolved via the Mattermost API)
|
||||
|
||||
Bare IDs are treated as channels.
|
||||
Bare opaque IDs (like `64ifufp...`) are **ambiguous** in Mattermost (user ID vs channel ID).
|
||||
|
||||
OpenClaw resolves them **user-first**:
|
||||
|
||||
- If the ID exists as a user (`GET /api/v4/users/<id>` succeeds), OpenClaw sends a **DM** by resolving the direct channel via `/api/v4/channels/direct`.
|
||||
- Otherwise the ID is treated as a **channel ID**.
|
||||
|
||||
If you need deterministic behavior, always use the explicit prefixes (`user:<id>` / `channel:<id>`).
|
||||
|
||||
## Reactions (message tool)
|
||||
|
||||
|
||||
@@ -270,29 +270,33 @@ describe("parseMattermostTarget", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Valid 26-char Mattermost IDs for user-first resolution tests
|
||||
// These IDs match /^[a-z0-9]{26}$/ and trigger ambiguous resolution
|
||||
const FAKE_USER_ID = "abcdef1234567890abcdef1234"; // 26 chars
|
||||
const FAKE_CHANNEL_ID = "zzzzzz0987654321zzzzzz9876"; // 26 chars
|
||||
|
||||
// Each test uses a unique (token, id) pair to avoid module-level cache collisions.
|
||||
// userIdResolutionCache and dmChannelCache are module singletons that survive across tests.
|
||||
// Using unique cache keys per test ensures full isolation without needing a cache reset API.
|
||||
describe("sendMessageMattermost user-first resolution", () => {
|
||||
function makeAccount(token: string) {
|
||||
return {
|
||||
accountId: "default",
|
||||
botToken: token,
|
||||
baseUrl: "https://mattermost.example.com",
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockState.createMattermostClient.mockReturnValue({});
|
||||
mockState.createMattermostPost.mockResolvedValue({ id: "post-id" });
|
||||
mockState.createMattermostDirectChannel.mockResolvedValue({ id: "dm-channel-id" });
|
||||
mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-id" });
|
||||
mockState.resolveMattermostAccount.mockReturnValue({
|
||||
accountId: "default",
|
||||
botToken: "bot-token",
|
||||
baseUrl: "https://mattermost.example.com",
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves unprefixed 26-char id as user and sends via DM channel", async () => {
|
||||
mockState.fetchMattermostUser.mockResolvedValueOnce({ id: FAKE_USER_ID });
|
||||
// Unique token + id to avoid cache pollution from other tests
|
||||
const userId = "aaaaaa1111111111aaaaaa1111"; // 26 chars
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-user-dm-t1"));
|
||||
mockState.fetchMattermostUser.mockResolvedValueOnce({ id: userId });
|
||||
|
||||
const res = await sendMessageMattermost(FAKE_USER_ID, "hello");
|
||||
const res = await sendMessageMattermost(userId, "hello");
|
||||
|
||||
expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
|
||||
expect(mockState.createMattermostDirectChannel).toHaveBeenCalledTimes(1);
|
||||
@@ -303,62 +307,72 @@ describe("sendMessageMattermost user-first resolution", () => {
|
||||
});
|
||||
|
||||
it("falls back to channel id when user lookup returns 404", async () => {
|
||||
// Unique token + id for this test
|
||||
const channelId = "bbbbbb2222222222bbbbbb2222"; // 26 chars
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-404-t2"));
|
||||
const err = new Error("Mattermost API 404: user not found");
|
||||
mockState.fetchMattermostUser.mockRejectedValueOnce(err);
|
||||
|
||||
const res = await sendMessageMattermost(FAKE_CHANNEL_ID, "hello");
|
||||
const res = await sendMessageMattermost(channelId, "hello");
|
||||
|
||||
expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
|
||||
expect(mockState.createMattermostDirectChannel).not.toHaveBeenCalled();
|
||||
const params = mockState.createMattermostPost.mock.calls[0]?.[1];
|
||||
expect(params.channelId).toBe(FAKE_CHANNEL_ID);
|
||||
expect(res.channelId).toBe(FAKE_CHANNEL_ID);
|
||||
expect(params.channelId).toBe(channelId);
|
||||
expect(res.channelId).toBe(channelId);
|
||||
});
|
||||
|
||||
it("falls back to channel id without caching negative result on transient error", async () => {
|
||||
// Two unique tokens so each call has its own cache namespace
|
||||
const userId = "cccccc3333333333cccccc3333"; // 26 chars
|
||||
const tokenA = "token-transient-t3a";
|
||||
const tokenB = "token-transient-t3b";
|
||||
const transientErr = new Error("Mattermost API 503: service unavailable");
|
||||
mockState.fetchMattermostUser
|
||||
.mockRejectedValueOnce(transientErr)
|
||||
.mockResolvedValueOnce({ id: FAKE_USER_ID });
|
||||
|
||||
// First call: transient error → fall back to channel, do NOT cache negative
|
||||
const res1 = await sendMessageMattermost(FAKE_USER_ID, "first");
|
||||
expect(res1.channelId).toBe(FAKE_USER_ID);
|
||||
// First call: transient error → fall back to channel id, do NOT cache negative
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount(tokenA));
|
||||
mockState.fetchMattermostUser.mockRejectedValueOnce(transientErr);
|
||||
|
||||
// Second call with same id: should retry user lookup (not cached)
|
||||
const res1 = await sendMessageMattermost(userId, "first");
|
||||
expect(res1.channelId).toBe(userId);
|
||||
|
||||
// Second call with a different token (new cache key) → retries user lookup
|
||||
vi.clearAllMocks();
|
||||
mockState.createMattermostClient.mockReturnValue({});
|
||||
mockState.createMattermostPost.mockResolvedValue({ id: "post-id-2" });
|
||||
mockState.createMattermostDirectChannel.mockResolvedValue({ id: "dm-channel-id" });
|
||||
mockState.fetchMattermostMe.mockResolvedValue({ id: "bot-id" });
|
||||
mockState.resolveMattermostAccount.mockReturnValue({
|
||||
accountId: "default",
|
||||
botToken: "bot-token-2", // different token to bypass cache
|
||||
baseUrl: "https://mattermost.example.com",
|
||||
});
|
||||
mockState.fetchMattermostUser.mockResolvedValueOnce({ id: FAKE_USER_ID });
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount(tokenB));
|
||||
mockState.fetchMattermostUser.mockResolvedValueOnce({ id: userId });
|
||||
|
||||
const res2 = await sendMessageMattermost(FAKE_USER_ID, "second");
|
||||
const res2 = await sendMessageMattermost(userId, "second");
|
||||
expect(mockState.fetchMattermostUser).toHaveBeenCalledTimes(1);
|
||||
expect(res2.channelId).toBe("dm-channel-id");
|
||||
});
|
||||
|
||||
it("does not apply user-first resolution for explicit user: prefix", async () => {
|
||||
const res = await sendMessageMattermost(`user:${FAKE_USER_ID}`, "hello");
|
||||
// Unique token + id — explicit user: prefix bypasses probe, goes straight to DM
|
||||
const userId = "dddddd4444444444dddddd4444"; // 26 chars
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-explicit-user-t4"));
|
||||
|
||||
const res = await sendMessageMattermost(`user:${userId}`, "hello");
|
||||
|
||||
// explicit user: prefix → no fetchMattermostUser probe, goes straight to DM
|
||||
expect(mockState.fetchMattermostUser).not.toHaveBeenCalled();
|
||||
expect(mockState.createMattermostDirectChannel).toHaveBeenCalledTimes(1);
|
||||
expect(res.channelId).toBe("dm-channel-id");
|
||||
});
|
||||
|
||||
it("does not apply user-first resolution for explicit channel: prefix", async () => {
|
||||
const res = await sendMessageMattermost(`channel:${FAKE_USER_ID}`, "hello");
|
||||
// Unique token + id — explicit channel: prefix, no probe, no DM
|
||||
const chanId = "eeeeee5555555555eeeeee5555"; // 26 chars
|
||||
mockState.resolveMattermostAccount.mockReturnValue(makeAccount("token-explicit-chan-t5"));
|
||||
|
||||
const res = await sendMessageMattermost(`channel:${chanId}`, "hello");
|
||||
|
||||
expect(mockState.fetchMattermostUser).not.toHaveBeenCalled();
|
||||
expect(mockState.createMattermostDirectChannel).not.toHaveBeenCalled();
|
||||
const params = mockState.createMattermostPost.mock.calls[0]?.[1];
|
||||
expect(params.channelId).toBe(FAKE_USER_ID);
|
||||
expect(res.channelId).toBe(FAKE_USER_ID);
|
||||
expect(params.channelId).toBe(chanId);
|
||||
expect(res.channelId).toBe(chanId);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user