From 6df5a285f558a2836e49383fe9fef200b29b79a9 Mon Sep 17 00:00:00 2001 From: Cascade Bot Date: Sat, 4 Apr 2026 14:36:14 +0000 Subject: [PATCH] refactor(agents): remove LegacyCapabilities shim and migrate callers to capability strings --- src/agents/shared/builderFactory.ts | 4 +- src/agents/shared/capabilities.ts | 83 ------------------- tests/unit/agents/definitions/loader.test.ts | 43 ++++------ .../unit/agents/shared/builderFactory.test.ts | 22 ++--- 4 files changed, 25 insertions(+), 127 deletions(-) delete mode 100644 src/agents/shared/capabilities.ts diff --git a/src/agents/shared/builderFactory.ts b/src/agents/shared/builderFactory.ts index a214c39c..ddfb88a7 100644 --- a/src/agents/shared/builderFactory.ts +++ b/src/agents/shared/builderFactory.ts @@ -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'; @@ -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); } } diff --git a/src/agents/shared/capabilities.ts b/src/agents/shared/capabilities.ts deleted file mode 100644 index f1cab599..00000000 --- a/src/agents/shared/capabilities.ts +++ /dev/null @@ -1,83 +0,0 @@ -/** - * Agent Capabilities - * - * Re-exports capability types and functions from the new capability registry. - * - * This file is kept for backward compatibility. New code should import from: - * - '../capabilities/index.js' for full capability system - * - '../definitions/schema.js' for AgentCapabilities type - */ - -// Re-export capability functions -export { - buildGadgetsFromCapabilities, - CAPABILITIES, - CAPABILITY_REGISTRY, - deriveIntegrations, - deriveRequiredIntegrations, - filterToolManifests, - generateUnavailableCapabilitiesNote, - getCapabilitiesByIntegration, - getCapabilityIntegration, - getGadgetNamesFromCapabilities, - getSdkToolsFromCapabilities, - getUnavailableOptionalCapabilities, - isBuiltInCapability, - isValidCapability, - resolveEffectiveCapabilities, -} from '../capabilities/index.js'; -// Re-export capability types -export type { AgentCapabilities, Capability } from '../definitions/schema.js'; - -import { resolveAgentDefinition } from '../definitions/index.js'; - -/** - * Legacy interface for derived capability flags. - * Used by code that needs boolean capability checks. - */ -export interface LegacyCapabilities { - canEditFiles: boolean; - canCreatePR: boolean; - canUpdateChecklists: boolean; - isReadOnly: boolean; -} - -/** - * Get legacy capability flags for an agent type. - * - * Derives boolean capability flags from the new capability array format: - * - canEditFiles = has 'fs:write' - * - canCreatePR = has 'scm:pr' - * - canUpdateChecklists = has 'pm:checklist' - * - isReadOnly = does not have 'fs:write' - * - * For unknown agent types, returns full-access defaults to maintain - * backward compatibility. - */ -export async function getAgentCapabilities(agentType: string): Promise { - try { - const def = await resolveAgentDefinition(agentType); - const allCaps = [...def.capabilities.required, ...def.capabilities.optional]; - - return { - canEditFiles: allCaps.includes('fs:write'), - canCreatePR: allCaps.includes('scm:pr'), - canUpdateChecklists: allCaps.includes('pm:checklist'), - isReadOnly: !allCaps.includes('fs:write'), - }; - } catch (error) { - // Only fall back to full access for "agent not found" errors. - // Re-throw unexpected errors to avoid masking bugs with elevated privileges. - const message = error instanceof Error ? error.message : String(error); - if (message.includes('not found')) { - // Unknown agent type - return full-access defaults for backward compatibility - return { - canEditFiles: true, - canCreatePR: true, - canUpdateChecklists: true, - isReadOnly: false, - }; - } - throw error; - } -} diff --git a/tests/unit/agents/definitions/loader.test.ts b/tests/unit/agents/definitions/loader.test.ts index fb4631ed..c194c906 100644 --- a/tests/unit/agents/definitions/loader.test.ts +++ b/tests/unit/agents/definitions/loader.test.ts @@ -3,6 +3,7 @@ import { deriveIntegrations, getSdkToolsFromCapabilities, } from '../../../../src/agents/capabilities/resolver.js'; +import { getAgentCapabilities } from '../../../../src/agents/definitions/index.js'; import { clearDefinitionCache, getBuiltinAgentTypes, @@ -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', @@ -302,11 +302,11 @@ 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'); }); @@ -314,9 +314,9 @@ describe('YAML agent definitions loader', () => { 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'); }); @@ -324,8 +324,9 @@ describe('YAML agent definitions loader', () => { 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'); }); @@ -333,32 +334,18 @@ describe('YAML agent definitions loader', () => { 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(); }); }); diff --git a/tests/unit/agents/shared/builderFactory.test.ts b/tests/unit/agents/shared/builderFactory.test.ts index 91942653..07a122cb 100644 --- a/tests/unit/agents/shared/builderFactory.test.ts +++ b/tests/unit/agents/shared/builderFactory.test.ts @@ -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: [], }), })); @@ -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'; @@ -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); @@ -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);