fix(pi): retain ownership for uncertain mutations
This commit is contained in:
@@ -24,7 +24,7 @@ import type {
|
||||
PiRpcResponse,
|
||||
} from '../../electron/coding-runtime/pi/rpc-client';
|
||||
import type { PiWorkerStopReason } from '../../electron/coding-runtime/pi/worker-process';
|
||||
import type { PiProcessError } from '../../electron/coding-runtime/pi/process-errors';
|
||||
import { PiProcessError } from '../../electron/coding-runtime/pi/process-errors';
|
||||
import { BackgroundLifecycleController } from '../../electron/main/background-lifecycle';
|
||||
|
||||
const roots: string[] = [];
|
||||
@@ -35,14 +35,28 @@ class LifecycleFakeWorker implements PiConversationWorker {
|
||||
readonly stopReasons: PiWorkerStopReason[] = [];
|
||||
private readonly events = new Set<(event: PiRpcEvent) => void>();
|
||||
private readonly invalidations = new Set<(error: PiProcessError) => void>();
|
||||
private timeoutType: string | null = null;
|
||||
private compacted = false;
|
||||
private lateResult: PiRpcRequestOptions['onLateResult'];
|
||||
|
||||
constructor(readonly id: string, readonly generation: number) {}
|
||||
|
||||
async request<T = unknown>(
|
||||
command: PiRpcCommand,
|
||||
_options?: PiRpcRequestOptions,
|
||||
options?: PiRpcRequestOptions,
|
||||
): Promise<PiRpcResponse<T>> {
|
||||
this.requests.push(structuredClone(command));
|
||||
if (this.timeoutType === command.type) {
|
||||
this.timeoutType = null;
|
||||
this.lateResult = options?.onLateResult;
|
||||
await new Promise<void>((_resolve, reject) => {
|
||||
setTimeout(() => reject(new PiProcessError(
|
||||
'PI_RPC_TIMEOUT',
|
||||
`fake ${command.type} confirmation timeout`,
|
||||
{ generation: this.generation },
|
||||
)), 10_000);
|
||||
});
|
||||
}
|
||||
const data = command.type === 'get_state'
|
||||
? {
|
||||
sessionId: `session-${this.id}`,
|
||||
@@ -52,7 +66,29 @@ class LifecycleFakeWorker implements PiConversationWorker {
|
||||
pendingMessageCount: 0,
|
||||
}
|
||||
: command.type === 'get_entries'
|
||||
? { entries: [], leafId: null }
|
||||
? this.compacted
|
||||
? {
|
||||
entries: [
|
||||
{
|
||||
type: 'message',
|
||||
id: 'entry-kept',
|
||||
parentId: null,
|
||||
timestamp: NOW,
|
||||
message: { role: 'user', content: 'retained', timestamp: 1 },
|
||||
},
|
||||
{
|
||||
type: 'compaction',
|
||||
id: 'entry-compaction-proof',
|
||||
parentId: 'entry-kept',
|
||||
timestamp: NOW,
|
||||
summary: 'not exposed',
|
||||
firstKeptEntryId: 'entry-kept',
|
||||
tokensBefore: 5_000,
|
||||
},
|
||||
],
|
||||
leafId: 'entry-compaction-proof',
|
||||
}
|
||||
: { entries: [], leafId: null }
|
||||
: command.type === 'get_session_stats'
|
||||
? {
|
||||
contextUsage: { tokens: 0, contextWindow: 100_000, percent: 0 },
|
||||
@@ -86,9 +122,23 @@ class LifecycleFakeWorker implements PiConversationWorker {
|
||||
}
|
||||
|
||||
emit(event: PiRpcEvent): void {
|
||||
if (event.type === 'compaction_end' && event.aborted !== true && typeof event.errorMessage !== 'string') {
|
||||
this.compacted = true;
|
||||
}
|
||||
for (const listener of this.events) listener(event);
|
||||
}
|
||||
|
||||
timeoutNext(type: string): void {
|
||||
this.timeoutType = type;
|
||||
}
|
||||
|
||||
completeLateSuccess(): void {
|
||||
this.lateResult?.({
|
||||
response: { type: 'response', id: 'late-proof', success: true },
|
||||
});
|
||||
this.lateResult = undefined;
|
||||
}
|
||||
|
||||
async stop(reason: PiWorkerStopReason) {
|
||||
this.stopReasons.push(reason);
|
||||
return { mode: 'stdin-close' as const, code: 0, signal: null };
|
||||
@@ -101,7 +151,7 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe('Pi run background lifecycle lease', () => {
|
||||
it('keeps accepted running and queued work alive while hidden, then evicts idle workers', async () => {
|
||||
it('keeps confirmation-uncertain and queued work alive while hidden, then evicts idle workers', async () => {
|
||||
vi.useFakeTimers();
|
||||
const projectPath = await mkdtemp(path.join(tmpdir(), 'makelore-pi-background-'));
|
||||
roots.push(projectPath);
|
||||
@@ -203,13 +253,39 @@ describe('Pi run background lifecycle lease', () => {
|
||||
});
|
||||
|
||||
await Promise.all(inputs.map(async (input) => await runtime.prepare(input)));
|
||||
const accepted = await runtime.prompt({
|
||||
workersByConversation.get(runningConversation.id)!.timeoutNext('prompt');
|
||||
const uncertain = runtime.prompt({
|
||||
clientRequestId: 'request-1',
|
||||
conversationId: runningConversation.id,
|
||||
mode: 'prompt',
|
||||
text: 'Keep working while hidden',
|
||||
attachments: [],
|
||||
});
|
||||
void uncertain.catch(() => undefined);
|
||||
await vi.advanceTimersByTimeAsync(10_000);
|
||||
await expect(uncertain).rejects.toMatchObject({
|
||||
publicError: { code: 'CODING_REQUEST_UNCERTAIN' },
|
||||
});
|
||||
expect(runtime.getResilienceProofDiagnostics()).toMatchObject({
|
||||
backgroundLeases: { active: 1 },
|
||||
pool: { runs: { active: 1, waiting: 0 } },
|
||||
});
|
||||
expect((await runtime.getSnapshot(runningConversation.id)).run).toMatchObject({
|
||||
status: 'running',
|
||||
error: { code: 'CODING_REQUEST_UNCERTAIN' },
|
||||
});
|
||||
await expect(runtime.compact(runningConversation.id)).rejects.toMatchObject({
|
||||
publicError: { code: 'CODING_REQUEST_UNCERTAIN' },
|
||||
});
|
||||
expect(workersByConversation.get(runningConversation.id)!.requests.filter(
|
||||
({ type }) => type === 'compact',
|
||||
)).toHaveLength(0);
|
||||
workersByConversation.get(runningConversation.id)!.completeLateSuccess();
|
||||
await expect.poll(async () => (
|
||||
await runtime.getSnapshot(runningConversation.id)
|
||||
).run.error).toBeUndefined();
|
||||
expect(controller.getLeaseCount()).toBe(1);
|
||||
|
||||
const queued = await runtime.prompt({
|
||||
clientRequestId: 'request-2',
|
||||
conversationId: queuedConversation.id,
|
||||
@@ -217,7 +293,6 @@ describe('Pi run background lifecycle lease', () => {
|
||||
text: 'Wait safely while hidden',
|
||||
attachments: [],
|
||||
});
|
||||
expect(accepted.accepted).toBe(true);
|
||||
expect(queued).toMatchObject({ accepted: true, queuePosition: 1 });
|
||||
expect(controller.getLeaseCount()).toBe(2);
|
||||
const releaseChild = pool.trackGenerationResource({
|
||||
@@ -249,6 +324,45 @@ describe('Pi run background lifecycle lease', () => {
|
||||
expect(releasedLeaseIds.sort()).toEqual(acquiredLeaseIds.sort());
|
||||
expect(new Set(releasedLeaseIds).size).toBe(releasedLeaseIds.length);
|
||||
|
||||
workersByConversation.get(runningConversation.id)!.timeoutNext('compact');
|
||||
const compact = runtime.compact(runningConversation.id);
|
||||
void compact.catch(() => undefined);
|
||||
await vi.advanceTimersByTimeAsync(10_000);
|
||||
await expect(compact).rejects.toMatchObject({
|
||||
publicError: { code: 'CODING_REQUEST_UNCERTAIN' },
|
||||
});
|
||||
expect(controller.getLeaseCount()).toBe(1);
|
||||
expect(onStopRuntime).not.toHaveBeenCalled();
|
||||
expect((await runtime.getSnapshot(runningConversation.id)).run).toMatchObject({
|
||||
status: 'compacting',
|
||||
error: { code: 'CODING_REQUEST_UNCERTAIN' },
|
||||
});
|
||||
|
||||
workersByConversation.get(runningConversation.id)!.emit({
|
||||
type: 'compaction_start',
|
||||
reason: 'manual',
|
||||
});
|
||||
workersByConversation.get(runningConversation.id)!.emit({
|
||||
type: 'compaction_end',
|
||||
reason: 'manual',
|
||||
result: {
|
||||
summary: 'controlled summary',
|
||||
firstKeptEntryId: 'entry-kept',
|
||||
tokensBefore: 5_000,
|
||||
},
|
||||
aborted: false,
|
||||
willRetry: false,
|
||||
});
|
||||
expect((await runtime.getSnapshot(runningConversation.id)).context.compaction).toBe('idle');
|
||||
workersByConversation.get(runningConversation.id)!.emit({ type: 'agent_settled' });
|
||||
await expect.poll(async () => (
|
||||
await runtime.getSnapshot(runningConversation.id)
|
||||
).run.status).toBe('idle');
|
||||
expect((await runtime.getSnapshot(runningConversation.id)).nodes).toContainEqual(
|
||||
expect.objectContaining({ kind: 'compaction', status: 'complete' }),
|
||||
);
|
||||
expect(controller.getLeaseCount()).toBe(0);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
expect(onSleep).toHaveBeenCalledTimes(1);
|
||||
expect(onStopRuntime).toHaveBeenCalledTimes(1);
|
||||
|
||||
Reference in New Issue
Block a user