Skip to content

Commit a2eccff

Browse files
HweinstockHweinstock
committed
refactor: unify AddResult and RemovalResult into Result<T>
- Introduce Result<T> discriminated union with data field (Rust-inspired) - Error fields now carry Error objects instead of raw strings - AddResult<T> and RemovalResult preserved as deprecated aliases - data is required on success branch when T is not void - No data field when T is void (removals) 44 files changed, all tests passing (3901 tests)
1 parent a91d888 commit a2eccff

44 files changed

Lines changed: 325 additions & 237 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/cli/commands/create/action.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ export async function createProjectWithAgent(options: CreateWithAgentOptions): P
207207
});
208208
if (!importResult.success) {
209209
onProgress?.('Import agent from Bedrock', 'error');
210-
return { success: false, error: importResult.error, warnings: depWarnings };
210+
return { success: false, error: importResult.error.message, warnings: depWarnings };
211211
}
212212
onProgress?.('Import agent from Bedrock', 'done');
213213
return {

‎src/cli/operations/agent/import/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ export async function executeImportAgent(
118118
// 6. Set up Python environment
119119
await setupPythonProject({ projectDir: agentPath });
120120

121-
return { success: true, agentName: name, agentPath };
121+
return { success: true, data: { agentName: name, agentPath } };
122122
} catch (err) {
123-
return { success: false, error: getErrorMessage(err) };
123+
return { success: false, error: err instanceof Error ? err : new Error(getErrorMessage(err)) };
124124
}
125125
}

‎src/cli/operations/identity/__tests__/credential-ops.test.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ describe('createCredential', () => {
5454
apiKey: 'key123',
5555
});
5656

57-
expect(result).toEqual(expect.objectContaining({ success: true, credentialName: 'NewCred' }));
57+
expect(result).toEqual(
58+
expect.objectContaining({ success: true, data: expect.objectContaining({ credentialName: 'NewCred' }) })
59+
);
5860
expect(mockWriteProjectSpec).toHaveBeenCalled();
5961
expect(mockSetEnvVar).toHaveBeenCalledWith('AGENTCORE_CREDENTIAL_NEWCRED', 'key123');
6062
});
@@ -70,7 +72,9 @@ describe('createCredential', () => {
7072
apiKey: 'newkey',
7173
});
7274

73-
expect(result).toEqual(expect.objectContaining({ success: true, credentialName: 'ExistCred' }));
75+
expect(result).toEqual(
76+
expect.objectContaining({ success: true, data: expect.objectContaining({ credentialName: 'ExistCred' }) })
77+
);
7478
expect(mockWriteProjectSpec).not.toHaveBeenCalled();
7579
expect(mockSetEnvVar).toHaveBeenCalledWith('AGENTCORE_CREDENTIAL_EXISTCRED', 'newkey');
7680
});

‎src/cli/operations/mcp/__tests__/create-mcp-utils.test.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ describe('GatewayPrimitive.add (createGateway)', () => {
142142
authorizerType: 'NONE',
143143
});
144144

145-
expect(result).toEqual(expect.objectContaining({ success: true, gatewayName: 'new-gw' }));
145+
expect(result).toEqual(
146+
expect.objectContaining({ success: true, data: expect.objectContaining({ gatewayName: 'new-gw' }) })
147+
);
146148
expect(mockWriteProjectSpec).toHaveBeenCalledWith(
147149
expect.objectContaining({
148150
agentCoreGateways: [
@@ -176,7 +178,9 @@ describe('GatewayPrimitive.add (createGateway)', () => {
176178
authorizerType: 'NONE',
177179
});
178180

179-
expect(result).toEqual(expect.objectContaining({ success: true, gatewayName: 'new-gw' }));
181+
expect(result).toEqual(
182+
expect.objectContaining({ success: true, data: expect.objectContaining({ gatewayName: 'new-gw' }) })
183+
);
180184
expect(mockWriteProjectSpec.mock.calls[0]![0].agentCoreGateways).toHaveLength(2);
181185
});
182186

@@ -200,7 +204,10 @@ describe('GatewayPrimitive.add (createGateway)', () => {
200204
});
201205

202206
expect(result).toEqual(
203-
expect.objectContaining({ success: false, error: expect.stringContaining('Gateway "dup-gw" already exists') })
207+
expect.objectContaining({
208+
success: false,
209+
error: expect.objectContaining({ message: expect.stringContaining('Gateway "dup-gw" already exists') }),
210+
})
204211
);
205212
});
206213

‎src/cli/operations/memory/__tests__/create-memory.test.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ describe('add', () => {
6363
expiry: 60,
6464
});
6565

66-
expect(result).toEqual(expect.objectContaining({ success: true, memoryName: 'NewMem' }));
66+
expect(result).toEqual(
67+
expect.objectContaining({ success: true, data: expect.objectContaining({ memoryName: 'NewMem' }) })
68+
);
6769
expect(mockWriteProjectSpec).toHaveBeenCalled();
6870

6971
// Verify the written spec contains the correct memory
@@ -86,7 +88,7 @@ describe('add', () => {
8688
expiry: 30,
8789
});
8890

89-
expect(result).toEqual(expect.objectContaining({ success: false, error: expect.any(String) }));
91+
expect(result).toEqual(expect.objectContaining({ success: false, error: expect.any(Error) }));
9092
expect(mockWriteProjectSpec).not.toHaveBeenCalled();
9193
});
9294

@@ -96,7 +98,10 @@ describe('add', () => {
9698
const result = await primitive.add({ name: 'Existing', strategies: '', expiry: 30 });
9799

98100
expect(result).toEqual(
99-
expect.objectContaining({ success: false, error: expect.stringContaining('Memory "Existing" already exists') })
101+
expect.objectContaining({
102+
success: false,
103+
error: expect.objectContaining({ message: expect.stringContaining('Memory "Existing" already exists') }),
104+
})
100105
);
101106
});
102107
});

‎src/cli/operations/remove/__tests__/remove-agent-ops.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,17 @@ describe('remove', () => {
8787

8888
const result = await primitive.remove('Missing');
8989

90-
expect(result).toEqual({ success: false, error: 'Agent "Missing" not found.' });
90+
expect(result).toEqual({
91+
success: false,
92+
error: expect.objectContaining({ message: 'Agent "Missing" not found.' }),
93+
});
9194
});
9295

9396
it('returns error on exception', async () => {
9497
mockReadProjectSpec.mockRejectedValue(new Error('read fail'));
9598

9699
const result = await primitive.remove('Agent1');
97100

98-
expect(result).toEqual({ success: false, error: 'read fail' });
101+
expect(result).toEqual({ success: false, error: expect.objectContaining({ message: 'read fail' }) });
99102
});
100103
});

‎src/cli/operations/remove/__tests__/remove-gateway-ops.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,17 @@ describe('remove', () => {
101101

102102
const result = await primitive.remove('missing');
103103

104-
expect(result).toEqual({ success: false, error: 'Gateway "missing" not found.' });
104+
expect(result).toEqual({
105+
success: false,
106+
error: expect.objectContaining({ message: 'Gateway "missing" not found.' }),
107+
});
105108
});
106109

107110
it('returns error on exception', async () => {
108111
mockReadProjectSpec.mockRejectedValue(new Error('read fail'));
109112

110113
const result = await primitive.remove('gw1');
111114

112-
expect(result).toEqual({ success: false, error: 'read fail' });
115+
expect(result).toEqual({ success: false, error: expect.objectContaining({ message: 'read fail' }) });
113116
});
114117
});

‎src/cli/operations/remove/__tests__/remove-identity-ops.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,17 @@ describe('remove', () => {
9393

9494
const result = await primitive.remove('Missing');
9595

96-
expect(result).toEqual({ success: false, error: 'Credential "Missing" not found.' });
96+
expect(result).toEqual({
97+
success: false,
98+
error: expect.objectContaining({ message: 'Credential "Missing" not found.' }),
99+
});
97100
});
98101

99102
it('returns error on exception', async () => {
100103
mockReadProjectSpec.mockRejectedValue(new Error('read fail'));
101104

102105
const result = await primitive.remove('Cred1');
103106

104-
expect(result).toEqual({ success: false, error: 'read fail' });
107+
expect(result).toEqual({ success: false, error: expect.objectContaining({ message: 'read fail' }) });
105108
});
106109
});

‎src/cli/operations/remove/__tests__/remove-memory-ops.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,17 @@ describe('remove', () => {
8484

8585
const result = await primitive.remove('Missing');
8686

87-
expect(result).toEqual({ success: false, error: 'Memory "Missing" not found.' });
87+
expect(result).toEqual({
88+
success: false,
89+
error: expect.objectContaining({ message: 'Memory "Missing" not found.' }),
90+
});
8891
});
8992

9093
it('returns error on exception', async () => {
9194
mockReadProjectSpec.mockRejectedValue(new Error('read fail'));
9295

9396
const result = await primitive.remove('Mem1');
9497

95-
expect(result).toEqual({ success: false, error: 'read fail' });
98+
expect(result).toEqual({ success: false, error: expect.objectContaining({ message: 'read fail' }) });
9699
});
97100
});

‎src/cli/operations/remove/remove-gateway-target.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,11 @@ export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise
172172

173173
const gateway = mcpSpec.agentCoreGateways.find(g => g.name === tool.gatewayName);
174174
if (!gateway) {
175-
return { success: false, error: `Gateway "${tool.gatewayName}" not found.` };
175+
return { success: false, error: new Error(`Gateway "${tool.gatewayName}" not found.`) };
176176
}
177177
const target = gateway.targets.find(t => t.name === tool.name);
178178
if (!target) {
179-
return { success: false, error: `Target "${tool.name}" not found in gateway "${tool.gatewayName}".` };
179+
return { success: false, error: new Error(`Target "${tool.name}" not found in gateway "${tool.gatewayName}".`) };
180180
}
181181
if (target.compute?.implementation && 'path' in target.compute.implementation) {
182182
toolPath = target.compute.implementation.path;
@@ -201,6 +201,6 @@ export async function removeGatewayTarget(tool: RemovableGatewayTarget): Promise
201201
return { success: true };
202202
} catch (err) {
203203
const message = err instanceof Error ? err.message : 'Unknown error';
204-
return { success: false, error: message };
204+
return { success: false, error: new Error(message) };
205205
}
206206
}

0 commit comments

Comments
 (0)