fix(coding): close PI-100 review findings

This commit is contained in:
2026-08-23 20:55:49 +08:00
parent 22b4a9f9c4
commit 52b2467d5d
16 changed files with 679 additions and 122 deletions

View File

@@ -27,7 +27,7 @@ const ATTACHMENT_ID_PATTERN = /^[A-Za-z0-9-]{1,64}$/;
export class CodingConversationServiceError extends Error {
constructor(
readonly status: 400 | 404 | 409 | 503,
readonly status: 400 | 404 | 409 | 500 | 503,
readonly code: string,
message: string,
) {
@@ -41,6 +41,14 @@ export type CodingPromptAcceptance = PromptAcceptance;
interface AcceptanceRecord {
fingerprint: string;
flight: Promise<CodingPromptAcceptance>;
state: { value: 'protected' | 'settled' };
}
export interface CodingConversationServiceOptions {
archiveSession?(input: {
projectId: string;
sessionKey: string;
}): Promise<void>;
}
export type CodingConversationStreamEvent =
@@ -78,8 +86,17 @@ function runtimeError(error: unknown): never {
? error.publicError as { code?: unknown; message?: unknown }
: null;
if (publicError && typeof publicError.code === 'string') {
const status = publicError.code === 'CODING_CONVERSATION_NOT_FOUND'
? 404
: publicError.code === 'CODING_STORAGE_WRITE_FAILED'
? 500
: publicError.code === 'CODING_RUNTIME_START_FAILED'
|| publicError.code === 'CODING_RUNTIME_READY_TIMEOUT'
|| publicError.code === 'CODING_RUNTIME_PROTOCOL_ERROR'
? 503
: 409;
throw new CodingConversationServiceError(
publicError.code === 'CODING_CONVERSATION_NOT_FOUND' ? 404 : 409,
status,
publicError.code,
typeof publicError.message === 'string' ? publicError.message : 'Coding runtime request failed',
);
@@ -87,6 +104,21 @@ function runtimeError(error: unknown): never {
throw new CodingConversationServiceError(503, 'CODING_RUNTIME_UNAVAILABLE', 'The local coding runtime is unavailable');
}
async function persist<T>(operation: () => Promise<T>): Promise<T> {
try {
return await operation();
} catch (error) {
if (error instanceof CodingConversationServiceError || error instanceof CodingProjectServiceError) {
throw error;
}
throw new CodingConversationServiceError(
500,
'CODING_STORAGE_WRITE_FAILED',
'Coding data could not be persisted',
);
}
}
function publicConversation(conversation: CodingConversationV2): CodingConversationV2 {
const { piSessionId: _piSessionId, sessionKey: _sessionKey, ...safe } = conversation;
return safe;
@@ -135,6 +167,7 @@ export class CodingConversationService {
constructor(
readonly projects: CodingProjectService,
readonly runtime: CodingConversationRuntime,
private readonly options: CodingConversationServiceOptions = {},
) {}
async listConversations(projectId?: string): Promise<CodingConversationV2[]> {
@@ -160,12 +193,12 @@ export class CodingConversationService {
if (!agent) {
throw new CodingConversationServiceError(404, 'CODING_AGENT_NOT_FOUND', 'Coding project Agent does not exist');
}
const created = await this.projects.conversationStore(project.path).create({
const created = await persist(() => this.projects.conversationStore(project.path).create({
agentId,
title: requiredString(input.title, 'Conversation title', MAX_TITLE_LENGTH),
model: agent.model,
modelResolution: agent.modelResolution,
});
}));
return publicConversation(created);
}
@@ -189,7 +222,7 @@ export class CodingConversationService {
if (patch.unread !== undefined && typeof patch.unread !== 'boolean') {
throw new CodingConversationServiceError(400, 'CODING_CONVERSATION_REQUEST_INVALID', 'Conversation unread state is invalid');
}
const updated = await this.projects.conversationStore(project.path).patchMetadata(conversationId, {
const updated = await persist(() => this.projects.conversationStore(project.path).patchMetadata(conversationId, {
...(patch.title !== undefined
? { title: requiredString(patch.title, 'Conversation title', MAX_TITLE_LENGTH) }
: {}),
@@ -197,18 +230,19 @@ export class CodingConversationService {
? { archivedAt: patch.archived ? new Date().toISOString() : null }
: {}),
...(typeof patch.unread === 'boolean' ? { unread: patch.unread } : {}),
});
}));
return publicConversation(updated);
}
async deleteConversation(conversationId: string): Promise<void> {
const { project } = await this.projects.findActiveConversation(conversationId);
const { project, conversation } = await this.projects.findActiveConversation(conversationId);
try {
await this.runtime.dispose(conversationId);
} catch (error) {
runtimeError(error);
}
await this.projects.conversationStore(project.path).delete(conversationId);
await this.archiveSession(project.id, conversation.sessionKey);
await persist(() => this.projects.conversationStore(project.path).delete(conversationId));
this.prepareFlights.delete(conversationId);
for (const key of [...this.acceptances.keys()]) {
if (key.startsWith(`${conversationId}\u0000`)) this.acceptances.delete(key);
@@ -263,18 +297,26 @@ export class CodingConversationService {
if (prior.fingerprint !== fingerprint) {
throw new CodingConversationServiceError(409, 'CODING_REQUEST_ID_CONFLICT', 'Client request id was already used');
}
if (prior.state.value === 'settled') {
this.acceptances.delete(key);
this.acceptances.set(key, prior);
}
return await prior.flight;
}
this.reserveAcceptanceSlot();
const acceptanceState: AcceptanceRecord['state'] = { value: 'protected' };
const flight = (async (): Promise<CodingPromptAcceptance> => {
try {
await this.ensurePrepared(conversationId);
return await this.runtime.prompt({
const acceptance = await this.runtime.prompt({
conversationId,
clientRequestId,
mode,
text: input.text as string,
attachments: normalizedAttachments,
});
acceptanceState.value = 'settled';
return acceptance;
} catch (error) {
const publicCode = error && typeof error === 'object'
&& 'publicError' in error
@@ -287,8 +329,7 @@ export class CodingConversationService {
runtimeError(error);
}
})();
this.acceptances.set(key, { fingerprint, flight });
this.trimAcceptances();
this.acceptances.set(key, { fingerprint, flight, state: acceptanceState });
return await flight;
}
@@ -298,20 +339,23 @@ export class CodingConversationService {
}
async setModel(conversationId: string, model: ProductModelRef): Promise<ConversationModelState> {
await this.ensurePrepared(conversationId);
const { project } = await this.projects.findActiveConversation(conversationId);
let selected: ProductModelRef;
try {
const state = await this.runtime.setModel({
conversationId,
accountId: model.accountId,
modelId: model.modelId,
});
const withThinking = state.model
? { model: { ...state.model, thinkingLevel: model.thinkingLevel }, modelResolution: 'resolved' as const }
: state;
if (withThinking.model) await this.runtime.setThinking({ conversationId, thinkingLevel: model.thinkingLevel });
await this.projects.conversationStore(project.path).setModelState(conversationId, withThinking);
return withThinking;
selected = await this.runtime.validateModel(model);
} catch (error) { runtimeError(error); }
const state: ConversationModelState = {
model: selected,
modelResolution: 'resolved',
};
await persist(() => this.projects.conversationStore(project.path).setModelState(conversationId, state));
const preparing = this.prepareFlights.get(conversationId);
if (preparing) await preparing.catch(() => undefined);
try {
await this.runtime.dispose(conversationId);
this.prepareFlights.delete(conversationId);
await this.ensurePrepared(conversationId);
return state;
} catch (error) { runtimeError(error); }
}
@@ -338,12 +382,13 @@ export class CodingConversationService {
async fork(sourceConversationId: string, sourceEntryId?: string): Promise<CodingConversationV2> {
const prepared = await this.ensurePrepared(sourceConversationId);
const source = await this.projects.findActiveConversation(sourceConversationId);
const created = await this.projects.conversationStore(source.project.path).create({
const store = this.projects.conversationStore(source.project.path);
const created = await persist(() => store.create({
agentId: source.conversation.agentId,
title: `${source.conversation.title} (fork)`,
model: source.conversation.model,
modelResolution: source.conversation.modelResolution,
});
}));
try {
await this.runtime.fork({
sourceConversationId,
@@ -360,7 +405,15 @@ export class CodingConversationService {
});
return publicConversation(created);
} catch (error) {
await this.projects.conversationStore(source.project.path).delete(created.id).catch(() => undefined);
try {
await this.runtime.dispose(created.id);
const latest = await store.get(created.id);
await this.archiveSession(source.project.id, latest?.sessionKey);
await persist(() => store.delete(created.id));
} catch {
// Keep product metadata when cleanup cannot complete so the partial
// runtime/session remains owned and can be diagnosed or deleted later.
}
runtimeError(error);
}
}
@@ -493,11 +546,30 @@ export class CodingConversationService {
return flight;
}
private trimAcceptances(): void {
while (this.acceptances.size > MAX_ACCEPTANCES) {
const key = this.acceptances.keys().next().value as string | undefined;
if (!key) return;
this.acceptances.delete(key);
private reserveAcceptanceSlot(): void {
while (this.acceptances.size >= MAX_ACCEPTANCES) {
const settled = [...this.acceptances.entries()]
.find(([, record]) => record.state.value === 'settled');
if (!settled) {
throw new CodingConversationServiceError(
503,
'CODING_REQUEST_CAPACITY_EXCEEDED',
'The local request registry is full',
);
}
this.acceptances.delete(settled[0]);
}
}
private async archiveSession(projectId: string, sessionKey: string | undefined): Promise<void> {
if (!sessionKey) return;
if (!this.options.archiveSession) {
throw new CodingConversationServiceError(
500,
'CODING_STORAGE_WRITE_FAILED',
'Coding session archival is unavailable',
);
}
await persist(() => this.options.archiveSession!({ projectId, sessionKey }));
}
}