Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/fix-web-question-timeout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Stop auto-dismissing questions in the web UI after 60 seconds so they wait for the user's answer.
1 change: 0 additions & 1 deletion apps/kimi-web/src/api/daemon/agentEventProjector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,6 @@ const PROTOCOL_EVENT_NAMES = new Set([
'question.requested',
'question.answered',
'question.dismissed',
'question.expired',
// Background tasks (projected)
'task.created',
'task.progress',
Expand Down
3 changes: 1 addition & 2 deletions apps/kimi-web/src/api/daemon/eventReducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,7 @@ export function reduceAppEvent(

// -------------------------------------------------------------------------
case 'questionAnswered':
case 'questionDismissed':
case 'questionExpired': {
case 'questionDismissed': {
const sid = event.sessionId;
const qid = event.questionId;
const list = next.questionsBySession[sid] ?? [];
Expand Down
8 changes: 0 additions & 8 deletions apps/kimi-web/src/api/daemon/mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ export function toAppQuestionRequest(wire: WireQuestionRequest): AppQuestionRequ
turnId: wire.turn_id,
toolCallId: wire.tool_call_id,
questions: wire.questions.map(toAppQuestionItem),
expiresAt: wire.expires_at,
createdAt: wire.created_at,
};
}
Expand Down Expand Up @@ -645,13 +644,6 @@ export function toAppEvent(wire: WireEvent): AppEvent {
dismissedAt: w.payload.dismissed_at,
};

case 'event.question.expired':
return {
type: 'questionExpired',
sessionId: w.session_id,
questionId: w.payload.question_id,
};

// ----- Background tasks -----
case 'event.task.created':
return {
Expand Down
4 changes: 0 additions & 4 deletions apps/kimi-web/src/api/daemon/wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ export interface WireQuestionRequest {
turn_id?: number;
tool_call_id?: string;
questions: WireQuestionItem[];
expires_at: string;
created_at: string;
}

Expand Down Expand Up @@ -728,8 +727,6 @@ type WireEventQuestionDismissed = WireEventBase<'event.question.dismissed', {
dismissed_by: string;
dismissed_at: string;
}>;
type WireEventQuestionExpired = WireEventBase<'event.question.expired', { question_id: string }>;

// Background tasks
type WireEventTaskCreated = WireEventBase<'event.task.created', { task: WireBackgroundTask }>;
type WireEventTaskProgress = WireEventBase<'event.task.progress', {
Expand Down Expand Up @@ -791,7 +788,6 @@ export type WireEvent =
| WireEventQuestionRequested
| WireEventQuestionAnswered
| WireEventQuestionDismissed
| WireEventQuestionExpired
// Background tasks
| WireEventTaskCreated
| WireEventTaskProgress
Expand Down
2 changes: 0 additions & 2 deletions apps/kimi-web/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ export interface AppQuestionRequest {
turnId?: number;
toolCallId?: string;
questions: QuestionItem[];
expiresAt: string;
createdAt: string;
}

Expand Down Expand Up @@ -415,7 +414,6 @@ export type AppEvent =
| { type: 'questionRequested'; sessionId: string; question: AppQuestionRequest }
| { type: 'questionAnswered'; sessionId: string; questionId: string; resolvedAt: string }
| { type: 'questionDismissed'; sessionId: string; questionId: string; dismissedAt: string }
| { type: 'questionExpired'; sessionId: string; questionId: string }
| { type: 'taskCreated'; sessionId: string; task: AppTask }
| { type: 'taskProgress'; sessionId: string; taskId: string; outputChunk: string; stream: 'stdout' | 'stderr' }
| { type: 'taskCompleted'; sessionId: string; taskId: string; status: AppTaskStatus; outputPreview?: string; outputBytes?: number }
Expand Down
4 changes: 3 additions & 1 deletion packages/agent-core/src/rpc/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ export function createRPC<Left extends Record<string, any>, Right extends Record
signal?.throwIfAborted();
let response: RpcResponse;
try {
const value = await abortableRpc(Promise.resolve(fn(rpcPayload)), signal);
const handlerResult =
signal === undefined ? fn(rpcPayload) : fn(rpcPayload, { signal });
const value = await abortableRpc(Promise.resolve(handlerResult), signal);
response = { ok: true, value };
} catch (error) {
signal?.throwIfAborted();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ export class BridgeClientAPI implements SDKAPI {

async requestQuestion(
request: QuestionRequest & { sessionId: string; agentId: string },
options?: { signal?: AbortSignal },
): Promise<QuestionResult> {
return this.deps.questionService.request(request);
return this.deps.questionService.request(request, options);
}

async toolCall(
Expand Down
8 changes: 4 additions & 4 deletions packages/agent-core/src/services/question/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ export interface IQuestionService {
* Resolves with the in-process `QuestionResult` (null = no handler / fully
* dismissed). Concrete impls own timeout policy.
*/
request(req: InProcessQuestionRequest & { sessionId: string; agentId: string }): Promise<QuestionResult>;
request(
req: InProcessQuestionRequest & { sessionId: string; agentId: string },
options?: { signal?: AbortSignal },
): Promise<QuestionResult>;

/**
* Called by the answer-side (REST handler / TUI / mock) to settle a pending
Expand Down Expand Up @@ -104,8 +107,6 @@ export interface QuestionToBrokerRequestParams {
readonly sessionId: string;
/** `createdAt` ISO string; broker passes `new Date().toISOString()`. */
readonly createdAt: string;
/** `expiresAt` ISO string; broker computes `createdAt + 60s`. */
readonly expiresAt: string;
}

/**
Expand Down Expand Up @@ -161,7 +162,6 @@ export function toBrokerRequest(
session_id: params.sessionId,
questions: req.questions.map((q, i) => buildItem(q, i)),
created_at: params.createdAt,
expires_at: params.expiresAt,
};
if (req.turnId !== undefined) out.turn_id = req.turnId;
if (req.toolCallId !== undefined) out.tool_call_id = req.toolCallId;
Expand Down
3 changes: 1 addition & 2 deletions packages/agent-core/src/services/session/sessionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ export class SessionService extends Disposable implements ISessionService {
case 'event.approval.expired':
case 'event.question.requested':
case 'event.question.answered':
case 'event.question.dismissed':
case 'event.question.expired': {
case 'event.question.dismissed': {
this._emitStatusChanged(sessionId);
break;
}
Expand Down
2 changes: 0 additions & 2 deletions packages/agent-core/test/services/question-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('question-adapter · toBrokerRequest (in-process → protocol)', () =>
questionId: '01J_QUESTION',
sessionId: 'sess_x',
createdAt: '2026-06-04T10:30:00.000Z',
expiresAt: '2026-06-04T10:31:00.000Z',
});

expect(protoReq.question_id).toBe('01J_QUESTION');
Expand Down Expand Up @@ -91,7 +90,6 @@ describe('question-adapter · toBrokerRequest (in-process → protocol)', () =>
questionId: 'q',
sessionId: 's',
createdAt: '2026-06-04T10:30:00.000Z',
expiresAt: '2026-06-04T10:31:00.000Z',
});
expect(protoReq.turn_id).toBeUndefined();
expect(protoReq.tool_call_id).toBeUndefined();
Expand Down
2 changes: 0 additions & 2 deletions packages/protocol/src/__tests__/question.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ describe('questionRequestSchema (SCHEMAS §6.2)', () => {
},
],
created_at: '2026-06-04T10:30:00Z',
expires_at: '2026-06-04T10:31:00Z',
};

it('accepts a 1-question request', () => {
Expand Down Expand Up @@ -262,7 +261,6 @@ describe('listPendingQuestionsResponseSchema (REST pending recovery)', () => {
},
],
created_at: '2026-06-04T10:30:00Z',
expires_at: '2026-06-04T10:31:00Z',
};

it('accepts status=pending query', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/protocol/src/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const questionRequestSchema = z.object({
tool_call_id: z.string().min(1).optional(),
questions: z.array(questionItemSchema).min(1).max(4),
created_at: isoDateTimeSchema,
expires_at: isoDateTimeSchema,
});
export type QuestionRequest = z.infer<typeof questionRequestSchema>;

Expand Down
1 change: 0 additions & 1 deletion packages/server-e2e/scenarios/08-pending-recovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ interface QuestionRequestedPayload {
options: Array<{ id: string; label: string }>;
}>;
created_at: string;
expires_at: string;
}

async function main() {
Expand Down
76 changes: 37 additions & 39 deletions packages/server/src/services/approval/approvalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ export const APPROVAL_DEFAULT_TIMEOUT_MS = 60_000;

export const APPROVAL_RECENTLY_RESOLVED_CAP = 1024;

export class ApprovalExpiredError extends Error {
constructor(public readonly approvalId: string, timeoutMs: number) {
super(`approval ${approvalId} expired after ${timeoutMs}ms`);
this.name = 'ApprovalExpiredError';
}
}

class PendingApproval implements IDisposable {
private _settled = false;

Expand All @@ -35,13 +28,11 @@ class PendingApproval implements IDisposable {
readonly protocolRequest: ProtocolApprovalRequest,
private readonly _resolveFn: (r: ApprovalResponse) => void,
private readonly _rejectFn: (e: Error) => void,
private readonly _timer: NodeJS.Timeout,
) {}

markSettled(): void {
if (this._settled) return;
this._settled = true;
clearTimeout(this._timer);
}

resolve(r: ApprovalResponse): void {
Expand All @@ -55,7 +46,6 @@ class PendingApproval implements IDisposable {
dispose(): void {
if (this._settled) return;
this._settled = true;
clearTimeout(this._timer);
try {
this._rejectFn(new Error('server shutting down'));
} catch {
Expand All @@ -72,7 +62,6 @@ export class ApprovalService extends Disposable implements IApprovalService {
private readonly _byToolCallId = new Map<string, string>();

private readonly _recentlyResolved = new Set<string>();
private _timeoutMs = APPROVAL_DEFAULT_TIMEOUT_MS;
private readonly _recentlyResolvedCap = APPROVAL_RECENTLY_RESOLVED_CAP;

constructor(
Expand All @@ -81,6 +70,39 @@ export class ApprovalService extends Disposable implements IApprovalService {
) {
super();
this._pending = this._register(new DisposableMap<string, PendingApproval>());

// The turn's abort signal never reaches this broker: agent-core's
// `BridgeClientAPI.requestApproval` drops the `{ signal }` option, so an
// aborted turn would otherwise leave the approval in `_pending` forever
// (pinning the session in `awaiting_approval` and keeping the web panel
// open). Settle stale approvals when the in-process bus reports the turn
// ended for a cancellation reason. This is intentionally session-scoped: a
// turn has at most one pending approval, and on normal completion the
// approval is already resolved so this is a no-op.
this._register(
this.eventService.onDidPublish((event) => {
if ((event as { type?: string }).type !== 'turn.ended') return;
const reason = (event as { reason?: string }).reason;
if (reason !== 'cancelled' && reason !== 'failed' && reason !== 'filtered') return;
const sessionId = (event as { sessionId?: string }).sessionId;
if (sessionId === undefined || sessionId === '') return;
this.dismissForSession(sessionId);
}),
);
}

private dismissForSession(sessionId: string): void {
const ids: string[] = [];
for (const p of this._pending.values()) {
if (p.sessionId === sessionId) ids.push(p.approvalId);
}
for (const id of ids) {
// Reuse resolve(): clears `_pending` / `_byToolCallId` and publishes
// `event.approval.resolved` (decision: 'cancelled') so the web panel
// closes. The agent-core caller's promise is already rejected by the
// abort, so the resolved value is only observed by tests.
this.resolve(id, { decision: 'cancelled' });
}
}

async request(
Expand All @@ -92,7 +114,10 @@ export class ApprovalService extends Disposable implements IApprovalService {

const approvalId = ulid();
const createdAt = new Date().toISOString();
const expiresAt = new Date(Date.now() + this._timeoutMs).toISOString();
// `expires_at` is still populated for the protocol/web contract, but the
// broker no longer enforces it — approvals wait until the user resolves
// them or the server shuts down.
const expiresAt = new Date(Date.now() + APPROVAL_DEFAULT_TIMEOUT_MS).toISOString();

const protocolRequest = approvalToBrokerRequest(req, {
approvalId,
Expand Down Expand Up @@ -121,8 +146,6 @@ export class ApprovalService extends Disposable implements IApprovalService {
);

return new Promise<ApprovalResponse>((resolve, reject) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Clear approvals when the turn aborts

When a tool permission approval is pending and the user aborts the turn, requestApproval is called with the turn signal (packages/agent-core/src/agent/permission/index.ts:149-158) and the RPC layer now rejects the caller-side wait, but this broker never receives or listens to that signal. Since this change also removed the only timeout cleanup, the approval remains in _pending indefinitely after cancellation, so listPending() keeps the session in awaiting_approval and the stale approval can survive until the user manually resolves it or the server shuts down.

Useful? React with 👍 / 👎.

const timer = setTimeout(() => this._expire(approvalId), this._timeoutMs);
timer.unref?.();
this._pending.set(
approvalId,
new PendingApproval(
Expand All @@ -134,7 +157,6 @@ export class ApprovalService extends Disposable implements IApprovalService {
protocolRequest,
resolve,
reject,
timer,
),
);
this._byToolCallId.set(req.toolCallId, approvalId);
Expand Down Expand Up @@ -199,30 +221,6 @@ export class ApprovalService extends Disposable implements IApprovalService {
return { sessionId: p.sessionId, toolCallId: p.toolCallId };
}

_setTimeoutMsForTests(ms: number): void {
this._timeoutMs = ms;
}

private _expire(approvalId: string): void {
const p = this._pending.get(approvalId);
if (!p) return;
p.markSettled();
this._pending.deleteAndLeak(approvalId);
this._byToolCallId.delete(p.toolCallId);

this.markResolved(p.approvalId);

const expiredEvent: Event = {
type: 'event.approval.expired',
sessionId: p.sessionId,
agentId: 'main',
approval_id: p.approvalId,
} as unknown as Event;
this.eventService.publish(expiredEvent);

p.reject(new ApprovalExpiredError(p.approvalId, this._timeoutMs));
}

override dispose(): void {
if (this._store.isDisposed) return;
this._byToolCallId.clear();
Expand Down
1 change: 0 additions & 1 deletion packages/server/src/services/approval/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export {
ApprovalService,
ApprovalExpiredError,
APPROVAL_DEFAULT_TIMEOUT_MS,
APPROVAL_RECENTLY_RESOLVED_CAP,
} from './approvalService';
2 changes: 0 additions & 2 deletions packages/server/src/services/question/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
export {
QuestionService,
QuestionExpiredError,
QUESTION_DEFAULT_TIMEOUT_MS,
QUESTION_RECENTLY_RESOLVED_CAP,
} from './questionService';
Loading
Loading