-
Notifications
You must be signed in to change notification settings - Fork 0
feat: agent engine with orchestrator, parallel workers, and workspace intelligence #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
36a8ed8
docs: add jam agent engine design spec
sunilp 34568e9
docs: add jam agent engine implementation plan
sunilp 03944fe
feat(agent): add shared types with DAG validation and topological sort
sunilp 68cf7ce
feat(agent): add agent-specific error codes and hints
sunilp db26499
feat(agent): add agent config schema with permissions, sandbox, and w…
sunilp cea0cab
feat(agent): add tiered permission classifier (safe/moderate/dangerous)
sunilp c80d3dc
feat(agent): add OS-level command sandbox (macOS/Linux with fallback)
sunilp 4e4251f
feat(agent): add multimodal image input with provider fallback
sunilp 3e4412f
feat(agent): add file-lock manager with deadlock detection
sunilp be14473
feat(agent): add provider pool with semaphore-based concurrency control
sunilp d8cc6d5
feat(intel): add conventions analyzer for code style and patterns det…
sunilp c1f8fc0
feat(agent): add workspace intelligence with cached profiling
sunilp 45757e0
feat(agent): add task planner with DAG validation and token cost esti…
sunilp c7d9939
feat(agent): add worker execution loop with guardrails and round budget
sunilp 8fa3630
feat(agent): add orchestrator with parallel dispatch, file-lock, and …
sunilp 928d852
feat(agent): add progress reporter with multiplexed worker output
sunilp 923dc8c
feat(agent): upgrade jam run to use orchestrator with parallel workers
sunilp 09d72f9
feat(agent): rewrite jam go as interactive agent console with session…
sunilp b38bfb0
feat(agent): add barrel export and end-to-end integration test
sunilp 1d52d8d
fix: resolve type errors in planner, progress, defaults, and conventions
sunilp aa0f577
fix: resolve all ESLint errors in agent module
sunilp 2ae312b
fix: resolve lint errors in go.ts and run.ts
sunilp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { JamConfigSchema } from '../config/schema.js'; | ||
|
|
||
| describe('agent config schema', () => { | ||
| it('provides defaults when agent section is omitted', () => { | ||
| const result = JamConfigSchema.parse({}); | ||
| expect(result.agent).toBeDefined(); | ||
| expect(result.agent.maxWorkers).toBe(3); | ||
| expect(result.agent.defaultMode).toBe('supervised'); | ||
| expect(result.agent.maxRoundsPerWorker).toBe(20); | ||
| expect(result.agent.sandbox.filesystem).toBe('workspace-only'); | ||
| expect(result.agent.sandbox.network).toBe('allowed'); | ||
| expect(result.agent.sandbox.timeout).toBe(60000); | ||
| expect(result.agent.permissions.safe).toEqual([]); | ||
| expect(result.agent.permissions.dangerous).toEqual([]); | ||
| }); | ||
|
|
||
| it('validates custom agent config', () => { | ||
| const result = JamConfigSchema.parse({ | ||
| agent: { | ||
| maxWorkers: 5, | ||
| defaultMode: 'auto', | ||
| permissions: { safe: ['npm test'], dangerous: ['docker rm'] }, | ||
| sandbox: { filesystem: 'unrestricted', network: 'blocked', timeout: 30000 }, | ||
| }, | ||
| }); | ||
| expect(result.agent.maxWorkers).toBe(5); | ||
| expect(result.agent.defaultMode).toBe('auto'); | ||
| expect(result.agent.permissions.safe).toEqual(['npm test']); | ||
| expect(result.agent.sandbox.network).toBe('blocked'); | ||
| }); | ||
|
|
||
| it('rejects invalid mode', () => { | ||
| expect(() => JamConfigSchema.parse({ agent: { defaultMode: 'yolo' } })).toThrow(); | ||
| }); | ||
|
|
||
| it('rejects maxWorkers < 1', () => { | ||
| expect(() => JamConfigSchema.parse({ agent: { maxWorkers: 0 } })).toThrow(); | ||
| }); | ||
|
|
||
| it('rejects maxRoundsPerWorker out of bounds', () => { | ||
| expect(() => JamConfigSchema.parse({ agent: { maxRoundsPerWorker: 0 } })).toThrow(); | ||
| expect(() => JamConfigSchema.parse({ agent: { maxRoundsPerWorker: 51 } })).toThrow(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { JamError } from '../utils/errors.js'; | ||
|
|
||
| const AGENT_CODES = [ | ||
| 'AGENT_PLAN_FAILED', 'AGENT_PLAN_CYCLE', 'AGENT_WORKER_TIMEOUT', | ||
| 'AGENT_WORKER_CANCELLED', 'AGENT_FILE_LOCK_CONFLICT', 'AGENT_FILE_LOCK_TIMEOUT', | ||
| 'AGENT_BUDGET_EXCEEDED', 'AGENT_SANDBOX_UNAVAILABLE', 'AGENT_RATE_LIMITED', | ||
| 'AGENT_MERGE_CONFLICT', | ||
| ] as const; | ||
|
|
||
| describe('agent error codes', () => { | ||
| for (const code of AGENT_CODES) { | ||
| it(`creates JamError with code ${code}`, () => { | ||
| const err = new JamError(`test ${code}`, code); | ||
| expect(err.code).toBe(code); | ||
| expect(err.hint).toBeDefined(); | ||
| }); | ||
| } | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { FileLockManager } from './file-lock.js'; | ||
|
|
||
| describe('FileLockManager', () => { | ||
| it('assigns ownership from plan', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [ | ||
| { path: 'src/a.ts', mode: 'create' }, | ||
| { path: 'src/b.ts', mode: 'modify' }, | ||
| ]); | ||
| expect(mgr.getOwner('src/a.ts')).toBe('w1'); | ||
| expect(mgr.getOwner('src/b.ts')).toBe('w1'); | ||
| }); | ||
|
|
||
| it('grants request for unowned file', () => { | ||
| const mgr = new FileLockManager(); | ||
| const resp = mgr.requestFile({ workerId: 'w1', path: 'src/c.ts', reason: 'need it' }); | ||
| expect(resp.granted).toBe(true); | ||
| expect(mgr.getOwner('src/c.ts')).toBe('w1'); | ||
| }); | ||
|
|
||
| it('grants request for own file', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [{ path: 'src/a.ts', mode: 'modify' }]); | ||
| const resp = mgr.requestFile({ workerId: 'w1', path: 'src/a.ts', reason: 'already mine' }); | ||
| expect(resp.granted).toBe(true); | ||
| }); | ||
|
|
||
| it('denies request for file owned by another worker', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [{ path: 'src/a.ts', mode: 'modify' }]); | ||
| const resp = mgr.requestFile({ workerId: 'w2', path: 'src/a.ts', reason: 'need it' }); | ||
| expect(resp.granted).toBe(false); | ||
| expect(resp.waitForWorker).toBe('w1'); | ||
| }); | ||
|
|
||
| it('releases all locks for a worker', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [ | ||
| { path: 'src/a.ts', mode: 'create' }, | ||
| { path: 'src/b.ts', mode: 'modify' }, | ||
| ]); | ||
| mgr.releaseAll('w1'); | ||
| expect(mgr.getOwner('src/a.ts')).toBeUndefined(); | ||
| expect(mgr.getOwner('src/b.ts')).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('grants file after previous owner releases', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [{ path: 'src/a.ts', mode: 'modify' }]); | ||
| mgr.releaseAll('w1'); | ||
| const resp = mgr.requestFile({ workerId: 'w2', path: 'src/a.ts', reason: 'now free' }); | ||
| expect(resp.granted).toBe(true); | ||
| }); | ||
|
|
||
| it('detects deadlock (cycle in wait graph)', () => { | ||
| const mgr = new FileLockManager(); | ||
| mgr.assignOwnership('w1', [{ path: 'src/a.ts', mode: 'modify' }]); | ||
| mgr.assignOwnership('w2', [{ path: 'src/b.ts', mode: 'modify' }]); | ||
| // w1 waits for w2's file | ||
| mgr.requestFile({ workerId: 'w1', path: 'src/b.ts', reason: 'need b' }); | ||
| // Now w2 wants w1's file — this would create a deadlock | ||
| const resp = mgr.requestFile({ workerId: 'w2', path: 'src/a.ts', reason: 'need a' }); | ||
| expect(resp.granted).toBe(false); | ||
| // detectDeadlock should return true internally | ||
| }); | ||
|
|
||
| it('returns undefined owner for unknown path', () => { | ||
| const mgr = new FileLockManager(); | ||
| expect(mgr.getOwner('nonexistent')).toBeUndefined(); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import type { FileOwnership, FileLockRequest, FileLockResponse } from './types.js'; | ||
|
|
||
| export class FileLockManager { | ||
| // Maps file path → owner worker ID | ||
| private owners = new Map<string, string>(); | ||
| // Maps worker ID → set of file paths they own | ||
| private workerFiles = new Map<string, Set<string>>(); | ||
| // Wait graph: worker ID → worker ID they're waiting on (for deadlock detection) | ||
| private waitGraph = new Map<string, string>(); | ||
|
|
||
| /** Bulk assign ownership from plan */ | ||
| assignOwnership(workerId: string, files: FileOwnership[]): void { | ||
| for (const file of files) { | ||
| this.owners.set(file.path, workerId); | ||
| if (!this.workerFiles.has(workerId)) { | ||
| this.workerFiles.set(workerId, new Set()); | ||
| } | ||
| this.workerFiles.get(workerId)!.add(file.path); | ||
| } | ||
| } | ||
|
|
||
| /** Request access to a file not originally owned */ | ||
| requestFile(request: FileLockRequest): FileLockResponse { | ||
| const owner = this.owners.get(request.path); | ||
|
|
||
| // No owner → grant immediately | ||
| if (!owner) { | ||
| this.owners.set(request.path, request.workerId); | ||
| if (!this.workerFiles.has(request.workerId)) { | ||
| this.workerFiles.set(request.workerId, new Set()); | ||
| } | ||
| this.workerFiles.get(request.workerId)!.add(request.path); | ||
| return { granted: true }; | ||
| } | ||
|
|
||
| // Already own it | ||
| if (owner === request.workerId) return { granted: true }; | ||
|
|
||
| // Check for deadlock before adding to wait graph | ||
| if (this.detectDeadlock(request.workerId, owner)) { | ||
| return { granted: false, waitForWorker: owner }; | ||
| // Caller (orchestrator) handles the deadlock | ||
| } | ||
|
|
||
| // Not available — caller must wait | ||
| this.waitGraph.set(request.workerId, owner); | ||
| return { granted: false, waitForWorker: owner }; | ||
| } | ||
|
|
||
| /** Release all locks held by a worker */ | ||
| releaseAll(workerId: string): void { | ||
| const files = this.workerFiles.get(workerId); | ||
| if (files) { | ||
| for (const path of files) { | ||
| this.owners.delete(path); | ||
| } | ||
| this.workerFiles.delete(workerId); | ||
| } | ||
| this.waitGraph.delete(workerId); | ||
| } | ||
|
|
||
| /** Get owner of a file */ | ||
| getOwner(path: string): string | undefined { | ||
| return this.owners.get(path); | ||
| } | ||
|
|
||
| /** Check if granting would create a deadlock (cycle in wait graph) */ | ||
| detectDeadlock(requestingWorker: string, waitForWorker: string): boolean { | ||
| // DFS from waitForWorker through wait graph | ||
| // If we reach requestingWorker, it's a cycle (deadlock) | ||
| const visited = new Set<string>(); | ||
| let current: string | undefined = waitForWorker; | ||
| while (current) { | ||
| if (current === requestingWorker) return true; | ||
| if (visited.has(current)) break; | ||
| visited.add(current); | ||
| current = this.waitGraph.get(current); | ||
| } | ||
| return false; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| export * from './types.js'; | ||
| export { PermissionClassifier, classifyCommand, isHardBlocked, ApprovalTracker } from './permissions.js'; | ||
| export { detectSandboxStrategy, buildSandboxArgs, executeSandboxed } from './sandbox.js'; | ||
| export type { SandboxStrategy } from './sandbox.js'; | ||
| export { getTextContent, hasImages, flattenForProvider, loadImage } from './multimodal.js'; | ||
| export { FileLockManager } from './file-lock.js'; | ||
| export { ProviderPool } from './provider-pool.js'; | ||
| export type { ProviderLease } from './provider-pool.js'; | ||
| export { buildWorkspaceProfile, formatProfileForPrompt, computeProfileHash, loadCachedProfile } from './workspace-intel.js'; | ||
| export { generateTaskPlan, estimateTokenCost } from './planner.js'; | ||
| export { executeWorker } from './worker.js'; | ||
| export type { WorkerDeps } from './worker.js'; | ||
| export { Orchestrator } from './orchestrator.js'; | ||
| export type { OrchestratorDeps, OrchestratorOptions, OrchestratorResult, ProgressEvent } from './orchestrator.js'; | ||
| export { ProgressReporter, createProgressReporter } from './progress.js'; | ||
| export type { OutputMode } from './progress.js'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; | ||
| import { Orchestrator } from './orchestrator.js'; | ||
| import type { ProgressEvent } from './orchestrator.js'; | ||
| import type { ProviderAdapter } from '../providers/base.js'; | ||
| import type { Subtask, SubtaskContext } from './types.js'; | ||
|
|
||
| // Mock workspace-intel to avoid filesystem dependency | ||
| vi.mock('./workspace-intel.js', () => ({ | ||
| buildWorkspaceProfile: vi.fn().mockResolvedValue({ | ||
| language: 'typescript', monorepo: false, srcLayout: 'src/', | ||
| entryPoints: ['src/index.ts'], codeStyle: { | ||
| indent: 'spaces', indentSize: 2, quotes: 'single', | ||
| semicolons: true, trailingCommas: true, namingConvention: 'camelCase', | ||
| }, | ||
| fileNaming: 'kebab-case.ts', exportStyle: 'barrel', importStyle: 'relative', | ||
| errorHandling: 'JamError', logging: 'Logger', configPattern: 'cosmiconfig', | ||
| testFramework: 'vitest', testLocation: 'co-located', testNaming: '*.test.ts', | ||
| testStyle: 'describe/it', testCommand: 'npm test', commitConvention: 'conventional', | ||
| branchPattern: 'feat/*', packageManager: 'npm', typeChecker: 'tsc', | ||
| }), | ||
| formatProfileForPrompt: vi.fn().mockReturnValue('TypeScript project'), | ||
| })); | ||
|
|
||
| // Mock planner to return a 2-subtask plan with dependency | ||
| vi.mock('./planner.js', () => ({ | ||
| generateTaskPlan: vi.fn().mockResolvedValue({ | ||
| goal: 'Add greeting feature', | ||
| subtasks: [ | ||
| { | ||
| id: '1', description: 'Create greeting module', | ||
| files: [{ path: 'src/greeting.ts', mode: 'create' }], | ||
| estimatedRounds: 5, | ||
| }, | ||
| { | ||
| id: '2', description: 'Add tests for greeting', | ||
| files: [{ path: 'src/greeting.test.ts', mode: 'create' }], | ||
| estimatedRounds: 5, | ||
| validationCommand: 'npm test', | ||
| }, | ||
| ], | ||
| dependencyGraph: new Map([['1', []], ['2', ['1']]]), | ||
| }), | ||
| estimateTokenCost: vi.fn().mockReturnValue(10000), | ||
| })); | ||
|
|
||
| // Mock worker to simulate completing subtasks | ||
| let _workerCallCount = 0; | ||
| vi.mock('./worker.js', () => ({ | ||
| executeWorker: vi.fn().mockImplementation((subtask: Subtask) => { | ||
| _workerCallCount++; | ||
| return Promise.resolve({ | ||
| subtaskId: subtask.id, | ||
| status: 'completed', | ||
| filesChanged: [{ path: subtask.files[0]?.path ?? 'unknown', action: 'created', diff: '' }], | ||
| summary: `Completed subtask ${subtask.id}: ${subtask.description}`, | ||
| tokensUsed: { promptTokens: 200, completionTokens: 100, totalTokens: 300 }, | ||
| }); | ||
| }), | ||
| })); | ||
|
|
||
| const mockAdapter = { | ||
| info: { name: 'mock', supportsStreaming: true, supportsTools: true }, | ||
| validateCredentials: vi.fn(), | ||
| streamCompletion: vi.fn(), | ||
| listModels: vi.fn(), | ||
| chatWithTools: vi.fn(), | ||
| } as unknown as ProviderAdapter; | ||
|
|
||
| describe('Agent Engine Integration', () => { | ||
| beforeEach(() => { | ||
| _workerCallCount = 0; | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('orchestrates a 2-subtask plan end-to-end', async () => { | ||
| const orch = new Orchestrator({ | ||
| adapter: mockAdapter, | ||
| workspaceRoot: '/workspace', | ||
| toolSchemas: [], | ||
| executeTool: vi.fn().mockResolvedValue('ok'), | ||
| }); | ||
|
|
||
| const events: ProgressEvent[] = []; | ||
| const result = await orch.execute('add a greeting feature', { | ||
| mode: 'auto', | ||
| maxWorkers: 2, | ||
| onProgress: (e) => events.push(e), | ||
| }); | ||
|
|
||
| // Plan was generated | ||
| expect(result.plan.goal).toBe('Add greeting feature'); | ||
| expect(result.plan.subtasks).toHaveLength(2); | ||
|
|
||
| // Both subtasks completed | ||
| expect(result.results).toHaveLength(2); | ||
| expect(result.results.every(r => r.status === 'completed')).toBe(true); | ||
|
|
||
| // Dependency order: subtask 1 before subtask 2 | ||
| expect(result.results[0].subtaskId).toBe('1'); | ||
| expect(result.results[1].subtaskId).toBe('2'); | ||
|
|
||
| // Files tracked | ||
| expect(result.filesChanged).toContain('src/greeting.ts'); | ||
| expect(result.filesChanged).toContain('src/greeting.test.ts'); | ||
|
|
||
| // Token usage aggregated | ||
| expect(result.totalTokens.totalTokens).toBe(600); // 300 * 2 | ||
|
|
||
| // Progress events fired | ||
| expect(events.some(e => e.type === 'plan-ready')).toBe(true); | ||
| expect(events.filter(e => e.type === 'worker-started')).toHaveLength(2); | ||
| expect(events.filter(e => e.type === 'worker-completed')).toHaveLength(2); | ||
| expect(events.some(e => e.type === 'all-done')).toBe(true); | ||
|
|
||
| // Summary contains both subtask results | ||
| expect(result.summary).toContain('1:'); | ||
| expect(result.summary).toContain('2:'); | ||
| }); | ||
|
|
||
| it('worker receives prior context from dependency', async () => { | ||
| const { executeWorker } = await import('./worker.js'); | ||
|
|
||
| const orch = new Orchestrator({ | ||
| adapter: mockAdapter, | ||
| workspaceRoot: '/workspace', | ||
| toolSchemas: [], | ||
| executeTool: vi.fn().mockResolvedValue('ok'), | ||
| }); | ||
|
|
||
| await orch.execute('test', { mode: 'auto', maxWorkers: 1 }); | ||
|
|
||
| // Second worker call should have received context from first | ||
| const mockFn = executeWorker as unknown as Mock; | ||
| const calls = mockFn.mock.calls as Array<[Subtask, SubtaskContext, ...unknown[]]>; | ||
| expect(calls).toHaveLength(2); | ||
|
|
||
| // Second call's context should reference subtask 1's output | ||
| const secondCallContext = calls[1][1]; // context parameter | ||
| expect(secondCallContext.priorSummary).toContain('subtask 1'); | ||
| expect(secondCallContext.filesAvailable).toContain('src/greeting.ts'); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expose deadlocks separately from ordinary lock contention.
In
src/agent/file-lock.ts, the deadlock branch returns the same{ granted: false, waitForWorker }payload as a normal wait, so the caller cannot react differently and this test cannot prove a cycle was detected. Add a deadlock-specific signal toFileLockResponseand assert it here.🤖 Prompt for AI Agents