Skip to content

Commit a8aa84d

Browse files
committed
fix: complete shutdown and ellipsis cleanup
Guard cron delivery continuations after runtime disposal, cover the awaited injection race, and use Unicode ellipses in the remaining tool output. Remove unrelated sorting and test-timeout changes from the PR. Refs #248
1 parent ab8a14d commit a8aa84d

7 files changed

Lines changed: 110 additions & 42 deletions

File tree

apps/pythinker-code/src/tui/components/messages/tool-call.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ export class ToolCallComponent extends Container {
813813
if (this.result !== undefined || text.length === 0) return;
814814
this.liveOutput += text;
815815
if (this.liveOutput.length > MAX_LIVE_OUTPUT_CHARS) {
816-
this.liveOutput = `[...truncated]\n${this.liveOutput.slice(
816+
this.liveOutput = `[truncated]\n${this.liveOutput.slice(
817817
this.liveOutput.length - MAX_LIVE_OUTPUT_CHARS,
818818
)}`;
819819
}
@@ -2130,7 +2130,7 @@ export class ToolCallComponent extends Container {
21302130
const elapsedSeconds =
21312131
startedAtMs === undefined ? 0 : Math.max(0, Math.floor((Date.now() - startedAtMs) / 1000));
21322132
const target = filePath.length > 0 ? ` for ${filePath}` : '';
2133-
const progress = `Preparing changes${target}... ${formatByteSize(bytes)} · ${formatElapsed(
2133+
const progress = `Preparing changes${target} ${formatByteSize(bytes)} · ${formatElapsed(
21342134
elapsedSeconds,
21352135
)} elapsed`;
21362136
this.addChild(new Text(currentTheme.dim(progress), 2, 0));

apps/pythinker-code/test/tui/components/messages/tool-call.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,24 @@ describe('ToolCallComponent', () => {
254254
expect(out).toContain('line2');
255255
});
256256

257+
it('uses a Unicode ellipsis when truncating live Bash output', () => {
258+
const component = new ToolCallComponent(
259+
{
260+
id: 'call_shell_live_truncated',
261+
name: 'Bash',
262+
args: { command: 'printf output' },
263+
},
264+
undefined,
265+
);
266+
267+
component.setExpanded(true);
268+
component.appendLiveOutput('x'.repeat(50_001));
269+
270+
const out = strip(component.render(1000).join('\n'));
271+
expect(out).toContain('[…truncated]');
272+
expect(out).not.toContain('[...truncated]');
273+
});
274+
257275
it('clears live Bash output when the final result arrives', () => {
258276
const component = new ToolCallComponent(
259277
{
@@ -1751,7 +1769,7 @@ describe('ToolCallComponent', () => {
17511769
const out = strip(component.render(100).join('\n'));
17521770
expect(out).toContain('Using Edit');
17531771
expect(out).toContain('foo.ts');
1754-
expect(out).toContain('Preparing changes for foo.ts...');
1772+
expect(out).toContain('Preparing changes for foo.ts');
17551773
expect(out).toContain('4s elapsed');
17561774
expect(out).toMatch(/\d+(?:\.\d+)? (?:B|KB|MB)/);
17571775
expect(out).not.toContain('old20');

apps/vscode/webview-ui/src/components/SessionList.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ export function SessionList({ onClose }: SessionListProps) {
132132
const groupedSessions = useMemo(() => {
133133
const q = searchQuery.trim().toLowerCase();
134134
const filtered = q ? sessions.filter((s) => cleanSystemTags(s.brief).toLowerCase().includes(q)) : sessions;
135-
const sorted = [...filtered].toSorted((a, b) => (sortOrder === "recent" ? b.updatedAt - a.updatedAt : a.updatedAt - b.updatedAt));
135+
// oxlint-disable-next-line eslint-plugin-unicorn/no-array-sort -- The copied array is safe to sort in place.
136+
const sorted = [...filtered].sort((a, b) => (sortOrder === "recent" ? b.updatedAt - a.updatedAt : a.updatedAt - b.updatedAt));
136137

137138
const now = new Date();
138139
// Calendar-day arithmetic, not fixed 24h offsets — a DST change makes a

packages/agent-core-v2/src/features/cron/cronAgentRuntime.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ function deliverFire(
161161
runtime: AgentRuntimeContext<CronModelState>,
162162
task: CronTask,
163163
context: { readonly coalescedCount: number; readonly firedAt: number },
164+
isDisposed: () => boolean,
164165
): Promise<boolean> {
165166
const origin: CronJobOrigin = {
166167
kind: 'cron_job',
@@ -181,11 +182,14 @@ function deliverFire(
181182
try {
182183
launched = runtime.get(IAgentPromptService).inject(message);
183184
} catch (error) {
184-
debugLog(runtime, `steer threw for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
185+
if (!isDisposed()) {
186+
debugLog(runtime, `steer threw for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
187+
}
185188
return Promise.resolve(false);
186189
}
187190
return launched.then(
188191
() => {
192+
if (isDisposed()) return false;
189193
void runtime.dispatch(new CronFired({ origin, prompt: task.prompt }));
190194
telemetryOf(runtime).track2(CRON_FIRED, {
191195
recurring: task.recurring !== false,
@@ -196,7 +200,9 @@ function deliverFire(
196200
return true;
197201
},
198202
(error: unknown) => {
199-
debugLog(runtime, `steer launch rejected for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
203+
if (!isDisposed()) {
204+
debugLog(runtime, `steer launch rejected for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
205+
}
200206
return false;
201207
},
202208
);
@@ -207,6 +213,7 @@ async function processDue(
207213
state: CronEffectState,
208214
task: CronTask,
209215
now: number,
216+
isDisposed: () => boolean,
210217
): Promise<void> {
211218
if (state.inFlight.has(task.id)) return;
212219
let parsed: ParsedCronExpression;
@@ -242,13 +249,15 @@ async function processDue(
242249
const firedAt = state.clocks.wallNow();
243250
let delivered = false;
244251
try {
245-
delivered = await deliverFire(runtime, task, { coalescedCount, firedAt });
252+
delivered = await deliverFire(runtime, task, { coalescedCount, firedAt }, isDisposed);
246253
} catch (error) {
247-
debugLog(runtime, `deliverDue threw for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
254+
if (!isDisposed()) {
255+
debugLog(runtime, `deliverDue threw for task ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
256+
}
248257
} finally {
249-
state.inFlight.delete(task.id);
258+
if (!isDisposed()) state.inFlight.delete(task.id);
250259
}
251-
if (!delivered) return;
260+
if (isDisposed() || !delivered) return;
252261
if (task.recurring === false || isStaleAt(runtime, task, firedAt)) {
253262
const removed = removeTasks(runtime, [task.id]);
254263
state.lastSeenAt.delete(task.id);
@@ -277,7 +286,9 @@ async function tickCron(
277286
if (readCronConfig(config).disabled || runtime.getState().size === 0) return;
278287
if (runtime.get(IAgentLoopService).status().state === 'running') return;
279288
const now = state.clocks.wallNow();
280-
await Promise.all([...runtime.getState().values()].map((task) => processDue(runtime, state, task, now)));
289+
await Promise.all(
290+
[...runtime.getState().values()].map((task) => processDue(runtime, state, task, now, isDisposed)),
291+
);
281292
}
282293

283294
const cronEffects = fromCallback(({

packages/agent-core-v2/test/features/cron/sessionCron.test.ts

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@ import { describe, expect, it } from 'vitest';
22

33
import { DisposableStore } from '#/_base/di/lifecycle';
44
import { createServices } from '#/_base/di/test';
5-
import type { AgentContext } from '#/agent/agentContext/agentContext';
5+
import { IAgentLoopService } from '#/agent/loop/loop';
6+
import { IAgentPromptService } from '#/agent/prompt/prompt';
7+
import type { DurableAgentRuntimeParticipant } from '#/agent/runtime/agentRuntime';
68
import { AgentRuntimeSet } from '#/agent/runtime/agentRuntimeSet';
79
import { IAgentToolRegistryService } from '#/agent/toolRegistry/toolRegistry';
810
import { IConfigService } from '#/app/config/config';
11+
import { ITelemetryService } from '#/app/telemetry/telemetry';
912
import { DEFAULT_CRON_CONFIG } from '#/features/cron/configSection';
1013
import { AgentCron, cronAgentRuntimeProvider } from '#/features/cron/cronAgentRuntime';
11-
import { CronCursor } from '#/features/cron/cronOps';
14+
import { CronCursor, type CronModelState } from '#/features/cron/cronOps';
15+
import { IEventDispatcher } from '#/state/eventDispatcher';
1216

17+
import { stubAgentContext } from '../../agent/agentContext/stubs';
18+
import { stubLoopWithHooks } from '../../agent/loop/stubs';
1319
import {
1420
createTestAgent,
1521
InMemoryWireRecordPersistence,
@@ -28,48 +34,80 @@ async function bootCronContext(options: TestAgentOptions = {}): Promise<TestAgen
2834

2935
describe('session cron wire persistence', () => {
3036
it('settles an in-flight tick without reading services after close', async () => {
31-
let releaseReady!: () => void;
32-
const ready = new Promise<void>((resolve) => { releaseReady = resolve; });
33-
let markTickStarted!: () => void;
34-
const tickStarted = new Promise<void>((resolve) => { markTickStarted = resolve; });
35-
let readyReads = 0;
37+
let releaseInject!: () => void;
38+
const injection = new Promise<undefined>((resolve) => { releaseInject = () => { resolve(undefined); }; });
39+
let markInjectStarted!: () => void;
40+
const injectStarted = new Promise<void>((resolve) => { markInjectStarted = resolve; });
3641
let closed = false;
42+
let postCloseReads = 0;
43+
const recordRead = (): void => {
44+
if (closed) postCloseReads += 1;
45+
};
3746
const disposables = new DisposableStore();
3847
const services = createServices(disposables, {
3948
additionalServices: (reg) => {
4049
reg.definePartialInstance(IConfigService, {
41-
get ready() {
42-
readyReads += 1;
43-
if (readyReads === 2) markTickStarted();
44-
return ready;
45-
},
50+
ready: Promise.resolve(),
4651
get: <T>() => {
47-
if (closed) throw new Error('config read after close');
48-
return DEFAULT_CRON_CONFIG as T;
52+
recordRead();
53+
return { ...DEFAULT_CRON_CONFIG, noJitter: true, manualTick: true } as T;
54+
},
55+
});
56+
reg.defineInstance(IAgentLoopService, stubLoopWithHooks());
57+
reg.definePartialInstance(IAgentPromptService, {
58+
inject: () => {
59+
markInjectStarted();
60+
return injection;
4961
},
5062
});
63+
reg.definePartialInstance(IEventDispatcher, {
64+
dispatch: async () => { recordRead(); },
65+
});
66+
reg.definePartialInstance(ITelemetryService, {
67+
track2: () => { recordRead(); },
68+
});
5169
},
5270
});
53-
const agent = { agentId: 'main', generation: 1, space: {} } as AgentContext;
71+
const agent = stubAgentContext('main');
5472
const runtimes = new AgentRuntimeSet(agent, services);
5573
runtimes.apply({
5674
definition: AgentCron,
5775
provider: cronAgentRuntimeProvider,
5876
generation: 1,
5977
active: true,
6078
});
61-
runtimes.attachDurable({ attach: () => ({ dispose: () => {} }) });
62-
63-
const restoring = runtimes.restore();
64-
const ticking = runtimes.resolve(AgentCron).tick();
65-
await tickStarted;
66-
await runtimes.close();
67-
closed = true;
68-
disposables.dispose();
69-
releaseReady();
70-
71-
await expect(ticking).resolves.toBeUndefined();
72-
await expect(restoring).resolves.toBeUndefined();
79+
let participant: DurableAgentRuntimeParticipant<CronModelState> | undefined;
80+
runtimes.attachDurable({
81+
attach: (attached) => {
82+
participant = attached;
83+
return { dispose: () => {} };
84+
},
85+
});
86+
87+
try {
88+
await runtimes.restore();
89+
if (participant === undefined) throw new Error('Cron runtime was not attached');
90+
const now = Date.now();
91+
participant.commit(new Map([['deadbeef', {
92+
id: 'deadbeef',
93+
cron: '* * * * *',
94+
prompt: 'fire after wait',
95+
recurring: true,
96+
createdAt: now - 120_000,
97+
}]]));
98+
99+
const ticking = runtimes.resolve(AgentCron).tick();
100+
await injectStarted;
101+
await runtimes.close();
102+
closed = true;
103+
releaseInject();
104+
105+
await expect(ticking).resolves.toBeUndefined();
106+
expect(postCloseReads).toBe(0);
107+
} finally {
108+
await runtimes.close();
109+
disposables.dispose();
110+
}
73111
});
74112

75113
it('writes cron ops as durable wire records and rebuilds the task table on replay', async () => {

packages/agent-core-v2/test/features/tower/store.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ describe('merge gate', () => {
587587
await store.merge(consumer.branch);
588588
const state = await store.load();
589589
expect(state.missions.find((m) => m.id === consumer.id)?.status).toBe('merged');
590-
}, 15_000);
590+
});
591591

592592
it('refuses files outside the mission scope until the tower widens it', async () => {
593593
const mission = await setupMission({
@@ -651,7 +651,7 @@ describe('merge gate', () => {
651651
expect(conflictsWith).toEqual([
652652
{ branch: second.branch, files: ['src/a/shared.ts'] },
653653
]);
654-
}, 15_000);
654+
});
655655

656656
it('closes a zero-diff survey with a noop merge — no review, no git ceremony', async () => {
657657
const [mission] = await store.plan([
@@ -754,7 +754,7 @@ describe('merge gate', () => {
754754

755755
const after = await store.merge(third!.branch);
756756
expect(after.conflictsWith.map((c) => c.branch)).not.toContain(second!.branch);
757-
}, 15_000);
757+
});
758758
});
759759

760760
describe('updateMission', () => {

packages/agent-gateway/test/fs-watch.e2e.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe('WS fs watch (agent-gateway)', () => {
210210

211211
writeFileSync(join(workspace, 'src', 'instant.ts'), 'export const i = 1;\n');
212212

213-
const ev = await receiveType(conn, 'event.fs.changed', 10_000);
213+
const ev = await receiveType(conn, 'event.fs.changed', 3000);
214214
expect(ev.session_id).toBe(sid);
215215
const payload = ev.payload as { changes: Array<{ path: string }> };
216216
expect(payload.changes.some((c) => c.path === 'src/instant.ts' || c.path === 'src')).toBe(true);

0 commit comments

Comments
 (0)