Skip to content
This repository was archived by the owner on Sep 7, 2026. It is now read-only.
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions src/__tests__/integration/deterministic-api.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
deleteWorkspaceFile,
readWorkspaceFile,
sleep,
uniqueDirName,
uniqueFileName,
workspaceFileExists,
} from './test-utils';
Expand Down Expand Up @@ -132,6 +133,18 @@ describe('Deterministic API Integration Tests', () => {
conversation.switchProfile('__profile_that_should_not_exist__')
).rejects.toBeInstanceOf(HttpError);

// switchLlm swaps the LLM in place, keyed by usage_id (first-write-wins
// in the registry). Reusing the agent's existing usage_id would be a
// silent no-op, so use a fresh one and read the model back off the
// agent to prove the swap actually took effect.
await conversation.switchLlm({
model: 'dummy/switched-model',
api_key: 'dummy-key',
usage_id: 'switched',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: 'switched' is a static string — if the test server ever accumulates state across runs, this usage_id would already be registered and switchLlm would be a silent no-op (the exact trap the comment above warns about). A unique value (e.g. uniqueFileName('usage') or uniqueDirName('usage')) would make this unconditionally safe regardless of server state. Non-blocking in CI where a fresh container runs per job, but cheap to harden.

});
const switchedAgent = await conversation.state.getAgent();
expect(switchedAgent.llm.model).toBe('dummy/switched-model');

const acpConversation = await manager.acp.createConversation(
{
kind: 'Agent',
Expand Down Expand Up @@ -215,4 +228,61 @@ describe('Deterministic API Integration Tests', () => {
},
config.testTimeout
);

it(
'round-trips a profile through save, get, list, activate, rename, and delete',
async () => {
const profileName = uniqueDirName('it-profile');
const renamedProfile = `${profileName}-renamed`;
const llm = { model: 'dummy/model', api_key: 'dummy-key' };

try {
const saved = await manager.profiles.saveProfile(profileName, { llm });
expect(saved.name).toBe(profileName);

const detail = await manager.profiles.getProfile(profileName);
expect(detail.name).toBe(profileName);
// The saved LLM config round-trips...
expect(detail.config.model).toBe('dummy/model');
// ...but the default (no X-Expose-Secrets) response nulls api_key and
// reports the presence of the saved key via api_key_set instead.
expect(detail.config.api_key).toBeNull();
expect(detail.api_key_set).toBe(true);

const list = await manager.profiles.listProfiles();
expect(list.profiles.some((profile) => profile.name === profileName)).toBe(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: This only checks that the profile name appears in the list. ProfileInfo also surfaces model and api_key_set — verifying those here would confirm the list projection is consistent with the detail endpoint and would catch any field-mapping drift between the two representations:

Suggested change
expect(list.profiles.some((profile) => profile.name === profileName)).toBe(true);
const found = list.profiles.find((p) => p.name === profileName);
expect(found).toBeDefined();
expect(found?.model).toBe('dummy/model');
expect(found?.api_key_set).toBe(true);


const activated = await manager.profiles.activateProfile(profileName);
expect(activated.name).toBe(profileName);
expect(activated.llm_applied).toBe(true);

const afterActivate = await manager.profiles.listProfiles();
expect(afterActivate.active_profile).toBe(profileName);

const renamed = await manager.profiles.renameProfile(profileName, renamedProfile);
expect(renamed.name).toBe(renamedProfile);

const renamedDetail = await manager.profiles.getProfile(renamedProfile);
expect(renamedDetail.name).toBe(renamedProfile);
// The config survives the (atomic) rename...
expect(renamedDetail.config.model).toBe('dummy/model');
// ...and because the renamed profile was the active one, the
// active_profile pointer follows it to the new name.
const afterRename = await manager.profiles.listProfiles();
expect(afterRename.active_profile).toBe(renamedProfile);
// The original name stops resolving once the profile is renamed.
await expect(manager.profiles.getProfile(profileName)).rejects.toBeInstanceOf(HttpError);

const deleted = await manager.profiles.deleteProfile(renamedProfile);
expect(deleted.name).toBe(renamedProfile);

const finalList = await manager.profiles.listProfiles();
expect(finalList.profiles.some((profile) => profile.name === renamedProfile)).toBe(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: renamedProfile was the active profile when it was deleted, so active_profile should revert to null after the deletion. Adding that assertion would complete the lifecycle story this test is telling — currently the "profile is gone from the list" check and the active_profile cleanup are separate behaviors that aren't both verified:

Suggested change
expect(finalList.profiles.some((profile) => profile.name === renamedProfile)).toBe(false);
expect(finalList.profiles.some((profile) => profile.name === renamedProfile)).toBe(false);
expect(finalList.active_profile).toBeNull();

} finally {
await manager.profiles.deleteProfile(profileName).catch(() => undefined);
await manager.profiles.deleteProfile(renamedProfile).catch(() => undefined);
}
},
config.testTimeout
);
});
Loading