Skip to content
Merged
Show file tree
Hide file tree
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
4 changes: 2 additions & 2 deletions src/agents/shared/builderFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { initSessionState, type SessionHooks, setReadOnlyFs } from '../../gadget
import type { LLMCallLogger } from '../../utils/llmLogging.js';
import { resolveSquintDbPath } from '../../utils/squintDb.js';
import type { IProgressMonitor } from '../contracts/index.js';
import { getAgentCapabilities } from '../shared/capabilities.js';
import { getAgentCapabilities } from '../definitions/index.js';
import { type AccumulatedLlmCall, createObserverHooks } from '../utils/hooks.js';
import type { TrackingContext } from '../utils/tracking.js';

Expand Down Expand Up @@ -104,7 +104,7 @@ export async function createConfiguredBuilder(options: CreateBuilderOptions): Pr

// Mark session as read-only if agent lacks fs:write capability
const caps = await getAgentCapabilities(agentType);
if (caps.isReadOnly) {
if (![...caps.required, ...caps.optional].includes('fs:write')) {
setReadOnlyFs(true);
}
}
Expand Down
83 changes: 0 additions & 83 deletions src/agents/shared/capabilities.ts

This file was deleted.

43 changes: 15 additions & 28 deletions tests/unit/agents/definitions/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
deriveIntegrations,
getSdkToolsFromCapabilities,
} from '../../../../src/agents/capabilities/resolver.js';
import { getAgentCapabilities } from '../../../../src/agents/definitions/index.js';
import {
clearDefinitionCache,
getBuiltinAgentTypes,
Expand All @@ -11,7 +12,6 @@ import {
loadBuiltinDefinition,
} from '../../../../src/agents/definitions/loader.js';
import { CONTEXT_STEP_REGISTRY } from '../../../../src/agents/definitions/strategies.js';
import { getAgentCapabilities } from '../../../../src/agents/shared/capabilities.js';

const ALL_AGENT_TYPES = [
'alerting',
Expand Down Expand Up @@ -302,63 +302,50 @@ describe('YAML agent definitions loader', () => {
it('implementation agent has full capabilities and stop hooks', async () => {
const def = loadBuiltinDefinition('implementation');
const caps = await getAgentCapabilities('implementation');
const allCaps = [...caps.required, ...caps.optional];

expect(caps.canEditFiles).toBe(true);
expect(caps.canCreatePR).toBe(true);
expect(caps.canUpdateChecklists).toBe(true);
expect(caps.isReadOnly).toBe(false);
expect(allCaps.includes('fs:write')).toBe(true);
expect(allCaps.includes('scm:pr')).toBe(true);
expect(allCaps.includes('pm:checklist')).toBe(true);
expect(def.hooks?.finish?.scm?.requiresPR).toBe(true);
expect(def.integrations?.required).toContain('scm');
});

it('review agent is read-only', async () => {
const def = loadBuiltinDefinition('review');
const caps = await getAgentCapabilities('review');
const allCaps = [...caps.required, ...caps.optional];

expect(caps.canEditFiles).toBe(false);
expect(caps.isReadOnly).toBe(true);
expect(allCaps.includes('fs:write')).toBe(false);
expect(def.hooks?.finish?.scm?.requiresReview).toBe(true);
expect(def.integrations?.required).toContain('scm');
});

it('respond-to-ci agent requires scm integration', async () => {
const def = loadBuiltinDefinition('respond-to-ci');
const caps = await getAgentCapabilities('respond-to-ci');
const allCaps = [...caps.required, ...caps.optional];

expect(caps.canEditFiles).toBe(true);
expect(allCaps.includes('fs:write')).toBe(true);
expect(def.integrations?.required).toContain('scm');
});

it('capabilities from getAgentCapabilities are derived correctly for all agents', async () => {
for (const agentType of ALL_AGENT_TYPES) {
const def = loadBuiltinDefinition(agentType);
const caps = await getAgentCapabilities(agentType);
const allCaps = [...def.capabilities.required, ...def.capabilities.optional];

// canEditFiles = has fs:write
expect(caps.canEditFiles).toBe(allCaps.includes('fs:write'));

// canCreatePR = has scm:pr
expect(caps.canCreatePR).toBe(allCaps.includes('scm:pr'));
const allCaps = [...caps.required, ...caps.optional];
const defAllCaps = [...def.capabilities.required, ...def.capabilities.optional];

// canUpdateChecklists = has pm:checklist
expect(caps.canUpdateChecklists).toBe(allCaps.includes('pm:checklist'));

// isReadOnly = no fs:write
expect(caps.isReadOnly).toBe(!allCaps.includes('fs:write'));
// capabilities should match the definition
expect(allCaps).toEqual(defAllCaps);
}
});
});

describe('unknown agent type fallbacks', () => {
it('getAgentCapabilities returns full-access defaults for unknown type', async () => {
const caps = await getAgentCapabilities('nonexistent-agent-type');
expect(caps).toEqual({
canEditFiles: true,
canCreatePR: true,
canUpdateChecklists: true,
isReadOnly: false,
});
it('getAgentCapabilities throws for unknown agent type', async () => {
await expect(getAgentCapabilities('nonexistent-agent-type')).rejects.toThrow();
});
});

Expand Down
22 changes: 8 additions & 14 deletions tests/unit/agents/shared/builderFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,10 @@ vi.mock('../../../../src/gadgets/sessionState.js', async (importOriginal) => {
};
});

vi.mock('../../../../src/agents/shared/capabilities.js', () => ({
vi.mock('../../../../src/agents/definitions/index.js', () => ({
getAgentCapabilities: vi.fn().mockResolvedValue({
canEditFiles: true,
canCreatePR: true,
canUpdateChecklists: true,
isReadOnly: false,
required: ['fs:read', 'fs:write', 'shell:exec', 'session:ctrl'],
optional: [],
}),
}));

Expand Down Expand Up @@ -80,11 +78,11 @@ vi.mock('llmist', () => ({

import { execSync } from 'node:child_process';
import { AgentBuilder, BudgetPricingUnavailableError } from 'llmist';
import { getAgentCapabilities } from '../../../../src/agents/definitions/index.js';
import {
createConfiguredBuilder,
isSquintEnabled,
} from '../../../../src/agents/shared/builderFactory.js';
import { getAgentCapabilities } from '../../../../src/agents/shared/capabilities.js';
import { initSessionState, setReadOnlyFs } from '../../../../src/gadgets/sessionState.js';
import { resolveSquintDbPath } from '../../../../src/utils/squintDb.js';

Expand Down Expand Up @@ -322,10 +320,8 @@ describe('createConfiguredBuilder', () => {

it('calls setReadOnlyFs(true) when agent is read-only', async () => {
mockGetAgentCapabilities.mockResolvedValueOnce({
canEditFiles: false,
canCreatePR: false,
canUpdateChecklists: false,
isReadOnly: true,
required: ['fs:read', 'session:ctrl'],
optional: [],
});
const options = createBaseOptions({ agentType: 'review' });
await createConfiguredBuilder(options);
Expand All @@ -334,10 +330,8 @@ describe('createConfiguredBuilder', () => {

it('does not call setReadOnlyFs when agent has write access', async () => {
mockGetAgentCapabilities.mockResolvedValueOnce({
canEditFiles: true,
canCreatePR: true,
canUpdateChecklists: true,
isReadOnly: false,
required: ['fs:read', 'fs:write', 'shell:exec', 'session:ctrl'],
optional: [],
});
const options = createBaseOptions();
await createConfiguredBuilder(options);
Expand Down
Loading