Skip to content

Commit e1e1f88

Browse files
fix: improve rollback atomicity and validate AI service responses
Track successful dependency additions during persist so rollback removes partial adds before restoring the snapshot. Add validation guard in BridgedStructuredGenerator to throw descriptive errors when AI service returns null/undefined instead of failing silently.
1 parent ed04a48 commit e1e1f88

4 files changed

Lines changed: 198 additions & 3 deletions

File tree

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
---
2-
"@tm/core": minor
3-
"@tm/cli": minor
2+
"task-master-ai": minor
43
---
54

65
Add `tm clusters generate` command that uses AI to analyze tags in parallel, suggest inter-tag dependencies, and present an interactive terminal UI to review and re-order the cluster layout before persisting. Supports `--auto` for non-interactive mode and `--json` for machine-readable output.

apps/cli/src/commands/cluster-generate.command.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,27 @@ export async function persistClusterDependencies(
122122
}
123123
}
124124

125+
// Track successful additions for rollback
126+
const succeededAdds: Array<{ from: string; to: string }> = [];
127+
125128
try {
126129
for (const dep of dependencies) {
127130
await tmCore.tasks.addTagDependency(dep.from, dep.to);
131+
succeededAdds.push({ from: dep.from, to: dep.to });
128132
}
129133
} catch (error) {
134+
// Remove partially-added dependencies before restoring snapshot
135+
for (const { from, to } of succeededAdds) {
136+
await tmCore.tasks.removeTagDependency(from, to);
137+
}
138+
130139
// Restore original dependencies from snapshot
131140
for (const [tagName, deps] of snapshot) {
132141
for (const dep of deps) {
133142
await tmCore.tasks.addTagDependency(tagName, dep);
134143
}
135144
}
145+
136146
throw error;
137147
}
138148
}
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import { z } from 'zod';
3+
import {
4+
BridgedStructuredGenerator,
5+
type GenerateObjectServiceFn
6+
} from './structured-generator.js';
7+
import type { AIPrimitiveOptions } from '../types/primitives.types.js';
8+
9+
describe('BridgedStructuredGenerator', () => {
10+
describe('generate', () => {
11+
it('should throw error when result is null', async () => {
12+
const mockService: GenerateObjectServiceFn = vi
13+
.fn()
14+
.mockResolvedValue(null);
15+
const generator = new BridgedStructuredGenerator(mockService);
16+
17+
const schema = z.object({ value: z.string() });
18+
const options: AIPrimitiveOptions & { objectName?: string } = {
19+
commandName: 'test-command',
20+
objectName: 'test-object'
21+
};
22+
23+
await expect(
24+
generator.generate('test prompt', schema, options)
25+
).rejects.toThrow(
26+
'AI service returned null or undefined result (objectName: test-object, commandName: test-command)'
27+
);
28+
});
29+
30+
it('should throw error when result.mainResult is null', async () => {
31+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
32+
mainResult: null,
33+
modelId: 'test-model',
34+
providerName: 'test-provider'
35+
});
36+
const generator = new BridgedStructuredGenerator(mockService);
37+
38+
const schema = z.object({ value: z.string() });
39+
const options: AIPrimitiveOptions & { objectName?: string } = {
40+
commandName: 'test-command',
41+
objectName: 'test-object'
42+
};
43+
44+
await expect(
45+
generator.generate('test prompt', schema, options)
46+
).rejects.toThrow(
47+
'AI service returned null or undefined result (objectName: test-object, commandName: test-command, modelId: test-model, providerName: test-provider)'
48+
);
49+
});
50+
51+
it('should throw error when result.mainResult is undefined', async () => {
52+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
53+
mainResult: undefined,
54+
modelId: 'test-model',
55+
providerName: 'test-provider'
56+
});
57+
const generator = new BridgedStructuredGenerator(mockService);
58+
59+
const schema = z.object({ value: z.string() });
60+
const options: AIPrimitiveOptions & { objectName?: string } = {
61+
commandName: 'test-command',
62+
objectName: 'test-object'
63+
};
64+
65+
await expect(
66+
generator.generate('test prompt', schema, options)
67+
).rejects.toThrow(
68+
'AI service returned null or undefined result (objectName: test-object, commandName: test-command, modelId: test-model, providerName: test-provider)'
69+
);
70+
});
71+
72+
it('should return valid result when mainResult is present', async () => {
73+
const mockResult = { value: 'test-value' };
74+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
75+
mainResult: mockResult,
76+
modelId: 'test-model',
77+
providerName: 'test-provider',
78+
telemetryData: {
79+
inputTokens: 10,
80+
outputTokens: 20
81+
}
82+
});
83+
const generator = new BridgedStructuredGenerator(mockService);
84+
85+
const schema = z.object({ value: z.string() });
86+
const options: AIPrimitiveOptions & { objectName?: string } = {
87+
commandName: 'test-command',
88+
objectName: 'test-object'
89+
};
90+
91+
const result = await generator.generate('test prompt', schema, options);
92+
93+
expect(result.data).toEqual(mockResult);
94+
expect(result.usage).toEqual({
95+
inputTokens: 10,
96+
outputTokens: 20,
97+
model: 'test-model',
98+
provider: 'test-provider',
99+
duration: expect.any(Number)
100+
});
101+
});
102+
103+
it('should use default objectName when not provided', async () => {
104+
const mockResult = { value: 'test-value' };
105+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
106+
mainResult: mockResult,
107+
modelId: 'test-model',
108+
providerName: 'test-provider'
109+
});
110+
const generator = new BridgedStructuredGenerator(mockService);
111+
112+
const schema = z.object({ value: z.string() });
113+
const options: AIPrimitiveOptions = {
114+
commandName: 'test-command'
115+
};
116+
117+
await generator.generate('test prompt', schema, options);
118+
119+
expect(mockService).toHaveBeenCalledWith(
120+
expect.objectContaining({
121+
objectName: 'generated_object'
122+
})
123+
);
124+
});
125+
126+
it('should include error context without model/provider when not available', async () => {
127+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
128+
mainResult: null
129+
});
130+
const generator = new BridgedStructuredGenerator(mockService);
131+
132+
const schema = z.object({ value: z.string() });
133+
const options: AIPrimitiveOptions & { objectName?: string } = {
134+
commandName: 'test-command',
135+
objectName: 'test-object'
136+
};
137+
138+
await expect(
139+
generator.generate('test prompt', schema, options)
140+
).rejects.toThrow(
141+
'AI service returned null or undefined result (objectName: test-object, commandName: test-command)'
142+
);
143+
});
144+
145+
it('should handle zero tokens and missing telemetry gracefully', async () => {
146+
const mockResult = { value: 'test-value' };
147+
const mockService: GenerateObjectServiceFn = vi.fn().mockResolvedValue({
148+
mainResult: mockResult,
149+
telemetryData: null
150+
});
151+
const generator = new BridgedStructuredGenerator(mockService);
152+
153+
const schema = z.object({ value: z.string() });
154+
const options: AIPrimitiveOptions = {
155+
commandName: 'test-command'
156+
};
157+
158+
const result = await generator.generate('test prompt', schema, options);
159+
160+
expect(result.usage).toEqual({
161+
inputTokens: 0,
162+
outputTokens: 0,
163+
model: 'unknown',
164+
provider: 'unknown',
165+
duration: expect.any(Number)
166+
});
167+
});
168+
});
169+
});

packages/tm-core/src/modules/ai/structured-generation/structured-generator.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,33 @@ export class BridgedStructuredGenerator implements IStructuredGenerator {
4646
): Promise<AIPrimitiveResult<T>> {
4747
const startTime = Date.now();
4848

49+
const objectName = options.objectName ?? 'generated_object';
4950
const result = await this.generateObjectService({
5051
role: 'main',
5152
systemPrompt: options.systemPrompt ?? 'You are a helpful assistant.',
5253
prompt,
5354
schema,
54-
objectName: options.objectName ?? 'generated_object',
55+
objectName,
5556
commandName: options.commandName,
5657
outputType: 'cli'
5758
});
5859

60+
// Validate that the AI service returned a valid result
61+
if (!result || result.mainResult === null || result.mainResult === undefined) {
62+
const errorContext = [
63+
`objectName: ${objectName}`,
64+
`commandName: ${options.commandName}`,
65+
result?.modelId && `modelId: ${result.modelId}`,
66+
result?.providerName && `providerName: ${result.providerName}`
67+
]
68+
.filter(Boolean)
69+
.join(', ');
70+
71+
throw new Error(
72+
`AI service returned null or undefined result (${errorContext})`
73+
);
74+
}
75+
5976
const duration = Date.now() - startTime;
6077

6178
return {

0 commit comments

Comments
 (0)