Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ test('creates an explore Session through the Host-owned default model route', as
{
sessionId: 'bot-session-1',
workspace: { kind: 'project', projectId: 'project-1' },
mode: 'bot_chat',
name: 'Telegram conversation',
labels: ['bot', 'telegram'],
modelTarget: { kind: 'default' },
Expand Down
19 changes: 18 additions & 1 deletion apps/desktop/src/main/bot-incoming-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
nonTextMessageAck,
plaintextHelpReply,
} from '@maka/core/bot-events';
import { generalizedErrorMessage } from '@maka/core/redaction';
import { generalizedErrorMessage, redactSecrets } from '@maka/core/redaction';
import type { BotIncomingMessage, BotRegistry, BotReplyStream } from '@maka/runtime/bots';
import type { BotSessionAdapter, BotSessionTurnResult } from './bot-session-adapter.js';
import { isBotSessionUnavailableError } from './bot-session-adapter.js';
Expand Down Expand Up @@ -205,6 +205,7 @@ export function createBotIncomingMainService(deps: BotIncomingMainServiceDeps):
): Promise<void> {
if (closed) return;
let replyStream: BotReplyStream | null = null;
let processingStage = 'command-routing';
// PR-BOT-EPHEMERAL-REPLY-0: TTL for system notices (help / reset ack /
// fallback errors). Five minutes is long enough for the user to read
// and process the notice on mobile; short enough that bot DMs do not
Expand Down Expand Up @@ -248,13 +249,15 @@ export function createBotIncomingMainService(deps: BotIncomingMainServiceDeps):
let sessionId = botConversationSessions.get(conversationKey);
try {
if (!sessionId) {
processingStage = 'session-create';
sessionId = await createBotConversationSession(
conversationKey,
message,
SYSTEM_NOTICE_TTL_MS,
);
if (!sessionId) return;
} else {
processingStage = 'session-prepare';
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
let rebound = false;
try {
const permissionModeOk = await ensureBotSessionExploreMode(
Expand Down Expand Up @@ -285,6 +288,7 @@ export function createBotIncomingMainService(deps: BotIncomingMainServiceDeps):
}

const turnId = randomUUID();
processingStage = 'turn-start';
const replyOptions = message.sourceMessageId
? { replyToMessageId: message.sourceMessageId }
: undefined;
Expand Down Expand Up @@ -344,6 +348,7 @@ export function createBotIncomingMainService(deps: BotIncomingMainServiceDeps):
// bridge layer drops the field for non-Telegram platforms / multi-chunk
// continuation pieces.
if (reply.trim()) {
processingStage = 'reply-send';
// Actual agent reply: NO ephemeral TTL. The answer must stay
// visible — auto-deleting it would defeat the bot's purpose.
const sent = replyStream
Expand All @@ -370,6 +375,12 @@ export function createBotIncomingMainService(deps: BotIncomingMainServiceDeps):
} catch (error) {
await replyStream?.abort().catch(() => {});
if (closed) return;
const errorName = error instanceof Error ? error.name : typeof error;
const errorCode = readErrorCode(error);
const errorDetail = redactSecrets(error instanceof Error ? error.message : String(error));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] This new log line reaches for redactSecrets, but that function does not cover Telegram credentials.

const errorDetail = redactSecrets(error instanceof Error ? error.message : String(error));
console.error(`[bot-incoming] conversation processing failed: stage=${processingStage} … detail=${errorDetail}`);

processingStage includes reply-send — the stage that calls the Telegram API — and Telegram puts the credential in the URL path: https://api.telegram.org/bot<TOKEN>/<method>.

Measured against SECRET_PATTERNS (packages/core/src/redaction.ts:56-62), using a placeholder rather than a real token:

in  : fetch failed for https://api.telegram.org/bot111111111:AAAA…AAA/getUpdates
out : fetch failed for https://api.telegram.org/bot111111111:AAAA…AAA/getUpdates   ← unchanged
ref : "error with sk-ant-abcdefghijklmnop"  →  "error with [redacted]"             ← this one matches

The patterns cover sk-, AIza, gh*_, xox*-, and 40+ hex runs. Telegram's <digits>:<35 mixed alphanumerics> matches none of them, and because the secret sits in the path rather than the query, redactUrlQuerySecrets does not reach it either.

What was not demonstrated, stated plainly: undici transport failures were exercised directly — error.message is fetch failed and cause carries TLS/socket text, neither containing the URL. So no live leak path was proven. This is a lid that does not cover what it is placed over, not a leak that is currently happening.

Why it is still worth fixing:

  1. String(error) applies to any non-Error throw, whose shape is not constrained.
  2. Any later wrapper that adds context — failed to POST ${url} is the obvious one — passes straight through unredacted.
  3. A redactSecrets call sitting right there tells the next reviewer this is already covered. That is the part that makes it P2 rather than a note: the guard's presence discourages the check that would catch it.

Either add a Telegram-shaped pattern to SECRET_PATTERNS, or guarantee at the telegramApi layer that the URL never enters error text. The second is stronger — it removes the need for the redactor to know about this provider at all.

Independent untrusted-input & credential line, bound to 2da8dfbd. Verified at the gate 2026-08-23 13:20 UTC. All tokens above are placeholders; the probe requests targeted 192.0.2.1 (TEST-NET-1), so nothing left the machine.

console.error(
`[bot-incoming] conversation processing failed: stage=${processingStage} name=${errorName} code=${errorCode ?? 'unknown'} detail=${errorDetail}`,
);
const detail = isSessionWorkspaceUnavailableError(error)
? '工作目录不可用,请在桌面端选择有效目录后重试'
: generalizedErrorMessage(error, '机器人对话处理失败');
Expand Down Expand Up @@ -412,3 +423,9 @@ function botReply(result: BotSessionTurnResult): string {
if (result.kind === 'errored') return `Maka 处理失败:${result.reason}`;
return result.text;
}

function readErrorCode(error: unknown): string | undefined {
if (!error || typeof error !== 'object' || !('code' in error)) return undefined;
const code = (error as { code?: unknown }).code;
return typeof code === 'string' || typeof code === 'number' ? String(code) : undefined;
}
1 change: 1 addition & 0 deletions apps/desktop/src/main/runtime-host-bot-session-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export function createRuntimeHostBotSessionAdapter(
session = await deps.client.createSession({
sessionId,
workspace: target.workspace,
mode: 'bot_chat',
name: input.name,
labels: [...input.labels],
modelTarget: { kind: 'default' },
Expand Down
25 changes: 17 additions & 8 deletions apps/desktop/src/main/settings-bots-ipc-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,25 @@ export function registerSettingsBotsIpc(
});
return toSettingsTestResult(provider, result);
});
deps.ipcMain.handle('settings:bots:listStatuses', () =>
tryResult(async () => deps.botRegistry.allStatuses(), 'BOTS_STATUS_FAILED'),
);
deps.ipcMain.handle('settings:bots:listStatuses', () => deps.botRegistry.allStatuses());
deps.ipcMain.handle('settings:bots:restart', (_event, provider: BotProvider) =>
tryResult(async () => {
await deps.botRegistry.applySettings(
(await deps.settingsStore.get()).botChat,
(async () => {
const settings = await deps.settingsStore.get();
const status = await deps.botRegistry.restart(
provider,
settings.botChat.channels[provider],
);
return deps.botRegistry.getStatus(provider);
}, 'BOTS_RESTART_FAILED'),
if (!status.running) {
const reason = status.reason ?? 'unknown';
console.error(
`[BotRegistry] ${provider} restart did not enter listening: ${reason}`,
Comment on lines +98 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Restart discards failure status 🐞 Bug ≡ Correctness

Disposition: fix-now. The restart IPC handler rejects normal non-running BotStatus results, so the
renderer’s declared Promise<BotStatus> contract and its explicit status.running === false UI
path are bypassed; it retains stale status and shows only the generic start-failed toast.
Agent Prompt
## Issue description
`settings:bots:restart` turns a normal, non-running `BotStatus` into an IPC rejection. This violates the preload contract and prevents the renderer from storing and presenting the returned failed status.

## Issue Context
`BotRegistry.restart()` deliberately returns `getStatus()` after reconciliation; bridge start failures are caught inside reconciliation, so a stopped/failed status is an expected result rather than an exceptional IPC failure. The renderer already has a non-running-status branch.

## Fix Focus Areas
- apps/desktop/src/main/settings-bots-ipc-main.ts[91-108]
- apps/desktop/src/renderer/settings/bot-chat-settings-page.tsx[245-275]
- packages/runtime/src/bots/bot-registry.ts[52-69]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

);
// The renderer intentionally hides unmapped raw status reasons. Reject
// here so diagnostics retain the actionable, already-generalized cause.
throw new Error(`${provider} listener startup failed: ${reason}`);
}
return status;
})(),
);
deps.ipcMain.handle(
'settings:bots:onboarding:start',
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/__tests__/bot-events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe('bot event contract', () => {
test('recognizes only exact plaintext commands in direct messages', () => {
assert.equal(isPlaintextResetCommand({ isGroup: false, text: ' RESET ' }), true);
assert.equal(isPlaintextHelpCommand({ isGroup: false, text: '帮助' }), true);
assert.equal(isPlaintextHelpCommand({ isGroup: false, text: ' /START ' }), true);
assert.equal(isPlaintextHelpCommand({ isGroup: true, text: 'help' }), false);
assert.equal(isPlaintextHelpCommand({ isGroup: false, text: 'please help' }), false);
assert.equal(isPlaintextHelpCommand({ isGroup: false, text: ' ' }), false);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/bot-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export function isPlaintextResetCommand(
}

export const BOT_PLAINTEXT_HELP_COMMANDS: ReadonlyArray<string> = Object.freeze([
'/start',
'help',
'/help',
'?',
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/explore-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export interface SessionStartModeSpec {
}

export const SESSION_START_MODE_SPECS = {
bot_chat: {
name: 'Bot conversation',
labels: ['mode:bot_chat'],
permissionMode: 'explore',
},
deep_research: {
name: 'Deep Research',
labels: ['mode:deep_research'],
Expand Down
4 changes: 4 additions & 0 deletions packages/runtime/src/bots/__tests__/base-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ describe('BaseBotAdapter', () => {
botSettingsRequireRestart(base, { ...base, domain: 'https://bot.example.test' }),
true,
);
assert.equal(
botSettingsRequireRestart(base, { ...base, proxyUrl: 'http://127.0.0.1:7897' }),
true,
);

const adapter = new TestAdapter('telegram', { ...base, enabled: true, token: 'old-token' });
assert.deepEqual(adapter.updateSettings({ ...base, enabled: true, token: 'old-token' }), {
Expand Down
27 changes: 27 additions & 0 deletions packages/runtime/src/bots/__tests__/bot-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,33 @@ describe('BotRegistry', () => {
assert.equal(registry.getStatus('wecom').readiness, 'scaffolded');
});

test('restart force-starts a stopped bridge even when settings are unchanged', async () => {
const statuses: BotStatus[] = [];
const registry = new BotRegistry({
onIncomingMessage: () => {},
onStatusChange: (status) => statuses.push(status),
});
const channel = {
...createDefaultBotChannel('wecom'),
enabled: true,
token: '',
appId: undefined,
appSecret: undefined,
};

await registry.applySettings(settingsWith({ wecom: channel }));
const beforeRestart = statuses.length;
const status = await registry.restart('wecom', channel);

assert.equal(status.running, false);
assert.equal(status.reason, 'no-credentials');
assert.equal(statuses.length > beforeRestart, true);
assert.equal(
statuses.slice(beforeRestart).some((entry) => entry.platform === 'wecom'),
true,
);
});

test('stopAll waits behind any pending applySettings call and clears bridges', async () => {
const registry = new BotRegistry({
onIncomingMessage: () => {},
Expand Down
25 changes: 25 additions & 0 deletions packages/runtime/src/bots/__tests__/proxied-fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,31 @@ function startStreamingProxy(): Promise<{
}

describe('proxiedFetch', () => {
test('uses a per-request proxy URL when no global proxy is active', async () => {
const proxy = await startStreamingProxy();
setActiveProxy(null);
try {
const response = await withTimeout(
proxiedFetch('http://example.test/channel-proxy', {
timeoutMs: 0,
proxyUrl: `http://127.0.0.1:${proxy.port}`,
}),
5_000,
'proxiedFetch did not use the per-request proxy URL',
);
assert.equal(response.status, 200);
assert.ok(response.body);
const reader = response.body.getReader();
const first = await withTimeout(reader.read(), 5_000, 'proxy body was not readable');
assert.equal(Buffer.from(first.value ?? []).toString(), 'hello world');
proxy.sendTail();
await reader.cancel();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
} finally {
setActiveProxy(null);
await proxy.close();
}
});

test('returns a streaming proxied response at headers, before body EOF', async () => {
const proxy = await startStreamingProxy();
setActiveProxy({
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/src/bots/base-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export function botSettingsRequireRestart(
previous.appId !== next.appId ||
previous.appSecret !== next.appSecret ||
previous.domain !== next.domain ||
previous.webhookUrl !== next.webhookUrl
previous.webhookUrl !== next.webhookUrl ||
previous.proxyUrl !== next.proxyUrl

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2 · category ①] proxyUrl is now a restart trigger for every BotProvider, but this PR only passes the channel override into Telegram requests. Discord REST, gateway lookup, typing, and WebSocket connection still use proxiedFetch/WebSocket without settings.proxyUrl, while the shared Settings detail still exposes that field for Discord. Changing a Discord proxy therefore tears down and reconnects the gateway—creating a recoverable message window—yet leaves routing unchanged, so the saved setting is ineffective. Either scope this restart predicate/field to Telegram, or plumb the override through the complete Discord REST and gateway path with coverage.

);
}
46 changes: 34 additions & 12 deletions packages/runtime/src/bots/bot-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,27 @@ export class BotRegistry extends EventEmitter {
return next;
}

/** Force a provider restart even when its persisted settings are unchanged. */
async restart(platform: BotPlatform, settings: BotChannelSettings): Promise<BotStatus> {
const restart = async (): Promise<BotStatus> => {
const existing = this.bridges.get(platform);
if (existing) {
await existing.stop().catch(() => {});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Cancel or invalidate in-flight startup before replacing the bridge. stop() cannot cancel the pending Telegram getMe because the poll AbortController does not exist yet. If restart lands during that await, this method detaches/deletes the old bridge, but its startup later sets running = true and launches an untracked getUpdates loop. The orphan competes with the replacement and silently consumes updates after its listeners were removed. Please make startup generation/cancellation registry-owned (or pass a start signal and check it immediately after every await), then test restart() while getMe is gated and assert the old bridge never polls.

this.detach(existing);
this.bridges.delete(platform);
}
this.statuses.delete(platform);
await this.reconcileOne(platform, settings);
return this.getStatus(platform);
};
const next = this.applyQueue.then(restart, restart);
this.applyQueue = next.then(
() => undefined,
() => undefined,
);
return next;
}

getStatus(platform: BotPlatform): BotStatus {
return (
this.bridges.get(platform)?.getStatus() ??
Expand Down Expand Up @@ -134,18 +155,7 @@ export class BotRegistry extends EventEmitter {
).updateSettings;
if (update && !update.call(existing, settings).needsRestart) return;
await existing.stop().catch(() => {});
// Drop our 'message' / 'statusChange' listeners on the old
// bridge before dereferencing it. Some bridges (Discord
// gateway, WeChat poll) hold async tasks that can still emit
// after `stop()` returns; without removeAllListeners those
// emissions would race-call onIncomingMessage / onStatusChange
// against a bridge the registry already considers gone.
try {
(existing as BotBridge & EventEmitter).removeAllListeners('message');
(existing as BotBridge & EventEmitter).removeAllListeners('statusChange');
} catch {
// best-effort — non-EventEmitter bridges don't have listeners to clear.
}
this.detach(existing);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
this.statuses.delete(platform);

Expand Down Expand Up @@ -174,6 +184,18 @@ export class BotRegistry extends EventEmitter {
);
}

private detach(bridge: BotBridge): void {
// Some bridges retain async tasks after stop(); detach registry listeners
// before dereferencing them so stale emissions cannot race the replacement.
try {
const emitter = bridge as BotBridge & EventEmitter;
emitter.removeAllListeners('message');
emitter.removeAllListeners('statusChange');
} catch {
// Non-EventEmitter bridges have no listeners to clear.
}
}

private wire(bridge: BotBridge): void {
const emitter = bridge as BotBridge & EventEmitter;
emitter.on('message', (message: BotIncomingMessage) => this.deps.onIncomingMessage(message));
Expand Down
6 changes: 5 additions & 1 deletion packages/runtime/src/bots/bot-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ async function testTelegram(channel: BotChannelSettings): Promise<BotTestResult>
const base = `https://api.telegram.org/bot${channel.token}`;
try {
const me = await (
await proxiedFetch(`${base}/getMe`, { method: 'GET', timeoutMs: BOT_TEST_TIMEOUT_MS })
await proxiedFetch(`${base}/getMe`, {
method: 'GET',
timeoutMs: BOT_TEST_TIMEOUT_MS,
proxyUrl: channel.proxyUrl,
})
).json();
if (!me.ok) return { ok: false, error: me.description ?? 'Invalid bot token' };
return {
Expand Down
33 changes: 31 additions & 2 deletions packages/runtime/src/bots/proxied-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { ProxySettings } from '@maka/core/settings/network-settings';
import { fetch, type Dispatcher, type RequestInit as UndiciRequestInit } from 'undici';
import { matchesBypassList } from '../network/bypass-matcher.js';
import { buildProxyDispatcher } from '../network/proxy-dispatcher.js';
Expand All @@ -12,6 +13,8 @@ export type ProxiedFetchInit = Omit<
> & {
signal?: AbortSignal | null;
timeoutMs?: number;
/** Per-channel proxy override (for example Telegram's saved proxy URL). */
proxyUrl?: string;
};

export async function proxiedFetch(
Expand All @@ -20,12 +23,12 @@ export async function proxiedFetch(
): Promise<Response> {
const url =
typeof input === 'string' ? input : input instanceof URL ? input.toString() : input.url;
const proxy = resolveActiveProxy();
const { timeoutMs = DEFAULT_TIMEOUT_MS, signal, proxyUrl, ...fetchInit } = init;
const proxy = proxyUrl?.trim() ? proxySettingsFromUrl(proxyUrl) : resolveActiveProxy();
Comment on lines +26 to +27

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Default proxy hijacks telegram 🐞 Bug ≡ Correctness

proxiedFetch now treats every nonblank channel proxyUrl as an explicit override, while each
default Telegram channel is pre-populated with http://127.0.0.1:7890, causing fresh and previously
defaulted installations without that local proxy to bypass direct/global networking and fail even
with valid credentials. This affects connection tests, startup, polling, sends, typing, deletion,
and streaming drafts, contradicting the intended behavior of applying a per-channel override only
when configured by the user.
Agent Prompt
## Issue description
The new per-request proxy selection treats Telegram's historically non-empty default proxy URL (`http://127.0.0.1:7890`) as an explicit mandatory per-channel override. Because that local proxy service is often absent, valid Telegram bots on default settings can no longer connect or poll, and all Bot API traffic is forced away from the previously working direct/global route.

## Issue Context
The proxy field is pre-populated by the default channel constructor rather than supplied only after a user explicitly configures a proxy. Remove the unnecessary persisted default while retaining the existing UI placeholder as a suggestion and preserving blank-value fallback to the active/global transport; also account for already-persisted untouched defaults, such as through a settings migration, because changing only new-profile defaults would leave existing installations broken.

## Fix Focus Areas
- packages/core/src/bot-chat-settings.ts[97-106]
- packages/runtime/src/bots/proxied-fetch.ts[24-31]
- packages/runtime/src/bots/simple-bridge.ts[441-449]
- apps/desktop/src/renderer/settings/bot-chat-detail.tsx[442-451]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

let dispatcher: Dispatcher | undefined;
if (proxy && !matchesBypassList(new URL(url).hostname, proxy.bypassList)) {
dispatcher = buildProxyDispatcher(proxy) as Dispatcher;
}
const { timeoutMs = DEFAULT_TIMEOUT_MS, signal, ...fetchInit } = init;
const timeoutEnabled = timeoutMs > 0;
const controller = new AbortController();
const requestSignal = signal ? AbortSignal.any([signal, controller.signal]) : controller.signal;
Expand Down Expand Up @@ -91,6 +94,32 @@ export async function proxiedFetch(
return response;
}

function proxySettingsFromUrl(value: string): ProxySettings {
const url = new URL(value);
const type =
url.protocol === 'socks5:'
? 'socks5'
: url.protocol === 'https:'
? 'https'
: url.protocol === 'http:'
? 'http'
: undefined;
if (!type) throw new Error(`Unsupported proxy protocol: ${url.protocol}`);
const port = Number(url.port || (type === 'https' ? 443 : type === 'socks5' ? 1080 : 80));
if (!url.hostname || !Number.isInteger(port) || port < 1 || port > 65_535) {
throw new Error('Invalid proxy URL');
}
return {
enabled: true,
type,
host: url.hostname,
port,
...(url.username ? { username: decodeURIComponent(url.username) } : {}),
...(url.password ? { password: decodeURIComponent(url.password) } : {}),
bypassList: [],
};
}

Object.defineProperty(proxiedFetch, FETCH_PROXY_SNAPSHOT, {
get: resolveActiveProxy,
enumerable: false,
Expand Down
Loading
Loading