diff --git a/src/ai/fisherman-tools.ts b/src/ai/fisherman-tools.ts index 19cc0969..912af3a0 100644 --- a/src/ai/fisherman-tools.ts +++ b/src/ai/fisherman-tools.ts @@ -6,21 +6,18 @@ import type { RequestResult } from '../api/request-result.ts'; import type { RequestStore } from '../api/request-store.ts'; import { extractEndpointDefinition } from '../api/spec-reader.ts'; import { tag } from '../utils/logger.ts'; -import { RequestMap } from '../utils/request-map.ts'; import { isDynamicSegment } from '../utils/url-matcher.ts'; +import type { RequestHaul } from './fisherman/request-haul.ts'; -export function createFishermanTools(apiClient: ApiClient, requestStore: RequestStore, opts: { spec?: any; baseEndpoint?: string }) { +export function createFishermanTools(apiClient: ApiClient, requestStore: RequestStore, haul: RequestHaul, opts: { spec?: any; baseEndpoint?: string }) { let finished = false; let result: FishermanResult | null = null; - const ledgerStart = requestStore.getMadeRequests().length; - const runRequests = () => requestStore.getMadeRequests().slice(ledgerStart); - const successfulWrites = () => runRequests().filter((r) => r.isWrite && !r.error && r.status >= 200 && r.status < 400); - const getResult = () => result ?? synthesizeResult(runRequests(), successfulWrites(), false); + const getResult = () => result ?? synthesizeResult(haul, false); const isFinished = () => finished; const finishFromText = (text?: string) => { finished = true; - const synthesized = synthesizeResult(runRequests(), successfulWrites(), true); + const synthesized = synthesizeResult(haul, true); if (text && synthesized.success) synthesized.summary = text; result = synthesized; }; @@ -160,32 +157,12 @@ export function createFishermanTools(apiClient: ApiClient, requestStore: Request .describe('List of items that could not be created'), }), execute: async ({ summary, created, failed }) => { - const writes = successfulWrites(); - if (writes.length === 0) { - tag('warning').log('Fisherman: finish rejected — no successful write request in this run'); - return { finished: false, error: 'No successful write request was made in this run, so nothing was created. Keep working, or call stop if the data cannot be prepared.' }; - } - - const createdRequests = new RequestMap(writes); - - const verified: FishermanResult['created'] = []; - for (const item of created) { - if (item.id === undefined) { - verified.push(item); - continue; - } - const request = createdRequests.get(item.id); - if (!request) { - tag('warning').log(`Fisherman: dropped unverified created item ${item.type} (id: ${item.id})`); - continue; - } - verified.push({ ...item, request: request.toEndpoint() }); - } - if (verified.length === 0) verified.push(...writes.map(toCreatedItem)); + const { result: verified, error } = verifyFinish(haul, { summary, created, failed }); + if (!verified) return { finished: false, error }; tag('success').log(`Fisherman done: ${summary}`); finished = true; - result = { success: true, summary, created: verified, failed: failed || [] }; + result = verified; return { finished: true }; }, }), @@ -207,8 +184,37 @@ export function createFishermanTools(apiClient: ApiClient, requestStore: Request return { tools, getResult, isFinished, finishFromText }; } -function synthesizeResult(made: RequestResult[], writes: RequestResult[], declaredDone: boolean): FishermanResult { - const failures = made.filter((r) => r.status >= 400 || r.error); +export function verifyFinish(haul: RequestHaul, input: { summary: string; created: FishermanResult['created']; failed?: FishermanResult['failed'] }): { result: FishermanResult | null; error?: string } { + const writes = haul.successfulWrites(); + if (writes.length === 0) { + tag('warning').log('Fisherman: finish rejected — no successful write request in this run'); + return { result: null, error: 'No successful write request was made in this run, so nothing was created. Keep working, or call stop if the data cannot be prepared.' }; + } + + const createdRequests = haul.byId(); + + const verified: FishermanResult['created'] = []; + for (const item of input.created) { + if (item.id === undefined) { + verified.push(item); + continue; + } + const request = createdRequests.get(String(item.id)); + if (!request) { + tag('warning').log(`Fisherman: dropped unverified created item ${item.type} (id: ${item.id})`); + continue; + } + verified.push({ ...item, request: request.toEndpoint() }); + } + if (verified.length === 0) verified.push(...writes.map(toCreatedItem)); + + return { result: { success: true, summary: input.summary, created: verified, failed: input.failed || [] } }; +} + +function synthesizeResult(haul: RequestHaul, declaredDone: boolean): FishermanResult { + const made = haul.requests(); + const writes = haul.successfulWrites(); + const failures = haul.failed(); let summary = `Stopped before finishing: ${made.length} requests, ${writes.length} successful writes, ${failures.length} failed`; const lastFailure = failures[failures.length - 1]; if (lastFailure) summary += `; last failure: ${lastFailure.toSummary()}`; diff --git a/src/ai/fisherman.ts b/src/ai/fisherman.ts index c6dd6e4c..4d6f9d6b 100644 --- a/src/ai/fisherman.ts +++ b/src/ai/fisherman.ts @@ -1,6 +1,6 @@ import dedent from 'dedent'; import type { ApiClient } from '../api/api-client.ts'; -import type { RequestStore } from '../api/request-store.ts'; +import { type RequestStore, isFailedRequest } from '../api/request-store.ts'; import { listAllEndpoints } from '../api/spec-reader.ts'; import { createDebug, tag } from '../utils/logger.ts'; @@ -8,6 +8,7 @@ const debugLog = createDebug('explorbot:fisherman'); import { loop } from '../utils/loop.ts'; import type { Agent } from './agent.ts'; import { type FishermanResult, createFishermanTools } from './fisherman-tools.ts'; +import { RequestHaul } from './fisherman/request-haul.ts'; import type { Provider } from './provider.ts'; import { dataProtectionRules } from './rules.ts'; @@ -79,11 +80,11 @@ export class Fisherman implements Agent { await this.refreshAuth(); debugLog(`auth headers: ${Object.keys(this.apiClient.getHeaders()).join(', ')}`); - const { tools, getResult, isFinished, finishFromText } = createFishermanTools(this.apiClient, this.requestStore, { + const haul = new RequestHaul(this.requestStore); + const { tools, getResult, isFinished, finishFromText } = createFishermanTools(this.apiClient, this.requestStore, haul, { spec: this.spec, baseEndpoint: this.baseEndpoint, }); - const ledgerStart = this.requestStore.getMadeRequests().length; const conversation = this.provider.startConversation(this.buildSystemPrompt(endpointList, Object.keys(tools), scopeUrl), 'fisherman'); conversation.addUserText(this.buildTaskPrompt(instructions)); @@ -109,7 +110,7 @@ export class Fisherman implements Agent { return; } - if (this.isStuckOnEndpoint(ledgerStart)) { + if (this.isStuckOnEndpoint(haul)) { tag('warning').log('Fisherman: repeated failures on the same endpoint — stopping'); stop(); return; @@ -228,12 +229,12 @@ export class Fisherman implements Agent { `; } - private isStuckOnEndpoint(ledgerStart: number): boolean { - const made = this.requestStore.getMadeRequests().slice(ledgerStart); + private isStuckOnEndpoint(haul: RequestHaul): boolean { + const made = haul.requests(); if (made.length < REPEATED_FAILURE_LIMIT) return false; const recent = made.slice(-REPEATED_FAILURE_LIMIT); const first = recent[0]; - return recent.every((r) => (r.status >= 400 || r.error) && r.method === first.method && r.path === first.path); + return recent.every((r) => isFailedRequest(r) && r.method === first.method && r.path === first.path); } private buildTaskPrompt(instructions: string): string { diff --git a/src/ai/fisherman/request-haul.ts b/src/ai/fisherman/request-haul.ts new file mode 100644 index 00000000..c3213fc4 --- /dev/null +++ b/src/ai/fisherman/request-haul.ts @@ -0,0 +1,32 @@ +import type { RequestResult } from '../../api/request-result.ts'; +import { type RequestStore, isFailedRequest } from '../../api/request-store.ts'; + +export class RequestHaul { + private start: number; + + constructor(private store: RequestStore) { + this.start = store.getMadeRequests().length; + } + + requests(): RequestResult[] { + return this.store.getMadeRequests().slice(this.start); + } + + failed(): RequestResult[] { + return this.requests().filter(isFailedRequest); + } + + successfulWrites(): RequestResult[] { + return this.requests().filter((r) => r.isWrite && !r.error && r.status >= 200 && r.status < 400); + } + + byId(): Map { + const map = new Map(); + for (const request of this.successfulWrites()) { + const { id } = request.extractIdAndTitle(); + if (id === undefined) continue; + map.set(String(id), request); + } + return map; + } +} diff --git a/src/api/request-store.ts b/src/api/request-store.ts index 9217921c..e5cf8903 100644 --- a/src/api/request-store.ts +++ b/src/api/request-store.ts @@ -191,6 +191,10 @@ export class RequestStore { } } +export function isFailedRequest(request: RequestResult): boolean { + return request.status >= 400 || Boolean(request.error); +} + function normalizePathPattern(urlPath: string): string { return urlPath .split('/') diff --git a/src/utils/request-map.ts b/src/utils/request-map.ts deleted file mode 100644 index 0c320392..00000000 --- a/src/utils/request-map.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { RequestResult } from '../api/request-result.ts'; - -export class RequestMap { - private requests = new Map(); - - constructor(requests: RequestResult[] = []) { - for (const request of requests) this.add(request); - } - - add(request: RequestResult): void { - const { id } = request.extractIdAndTitle(); - if (id === undefined) return; - this.requests.set(String(id), request); - } - - get(id: string | number): RequestResult | undefined { - return this.requests.get(String(id)); - } -} diff --git a/tests/unit/fisherman-tools.test.ts b/tests/unit/fisherman-tools.test.ts index 752a572f..308755a9 100644 --- a/tests/unit/fisherman-tools.test.ts +++ b/tests/unit/fisherman-tools.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it } from 'bun:test'; import { createFishermanTools } from '../../src/ai/fisherman-tools.ts'; +import { RequestHaul } from '../../src/ai/fisherman/request-haul.ts'; describe('Fisherman tools', () => { it('does not present a rejected capture as a request example', async () => { const captured = { method: 'POST', path: '/plans', status: 400, requestBody: { plan: 'wrong' } }; - const { tools } = createFishermanTools({} as any, store(captured), {}); + const { tools } = fishermanTools({} as any, store(captured), {}); const result: any = await tools.getEndpointSpec.execute({ method: 'POST', path: '/plans' }, {} as any); @@ -16,7 +17,7 @@ describe('Fisherman tools', () => { it('prefers the specification when the captured request was rejected', async () => { const captured = { method: 'POST', path: '/plans', status: 422, requestBody: { plan: 'wrong' } }; const spec = { paths: { '/plans': { post: { requestBody: { required: true } } } } }; - const { tools } = createFishermanTools({} as any, store(captured), { spec }); + const { tools } = fishermanTools({} as any, store(captured), { spec }); const result: any = await tools.getEndpointSpec.execute({ method: 'POST', path: '/plans' }, {} as any); @@ -27,7 +28,7 @@ describe('Fisherman tools', () => { it('keeps a successful captured request as a usable example', async () => { const captured = { method: 'POST', path: '/plans', status: 201, requestBody: { title: 'Plan' } }; - const { tools } = createFishermanTools({} as any, store(captured), {}); + const { tools } = fishermanTools({} as any, store(captured), {}); const result: any = await tools.getEndpointSpec.execute({ method: 'POST', path: '/plans' }, {} as any); @@ -48,7 +49,7 @@ describe('Fisherman tools', () => { const apiClient = { request: async () => ({ status, statusText: String(status), rawResponseBody: '{}', responseBody: null }), }; - const { tools } = createFishermanTools(apiClient as any, store(), {}); + const { tools } = fishermanTools(apiClient as any, store(), {}); const result: any = await tools.request.execute({ method: 'POST', path: '/plans' }, {} as any); @@ -60,7 +61,7 @@ describe('Fisherman tools', () => { const apiClient = { request: async () => ({ status: 201, statusText: 'Created', rawResponseBody: '', responseBody: { id: 7, status: 'draft' } }), }; - const { tools } = createFishermanTools(apiClient as any, store(), {}); + const { tools } = fishermanTools(apiClient as any, store(), {}); const result: any = await tools.request.execute({ method: 'POST', path: '/items' }, {} as any); @@ -71,7 +72,7 @@ describe('Fisherman tools', () => { describe('ledger-derived results', () => { it('rejects finish when no successful write was made in this run', async () => { - const { tools, isFinished } = createFishermanTools({} as any, store(), {}); + const { tools, isFinished } = fishermanTools({} as any, store(), {}); const result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: '1' }] }, {} as any); @@ -82,7 +83,7 @@ describe('ledger-derived results', () => { it('ignores writes made before this run started', async () => { const made = [madeWrite('POST', '/api/suites', 201, { id: 's1' })]; - const { tools, isFinished } = createFishermanTools({} as any, store(undefined, made), {}); + const { tools, isFinished } = fishermanTools({} as any, store(undefined, made), {}); const result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: 's1' }] }, {} as any); @@ -92,7 +93,7 @@ describe('ledger-derived results', () => { it('drops created items whose id no write response returned, keeps verified ones with their request', async () => { const made: any[] = []; - const { tools, getResult } = createFishermanTools({} as any, store(undefined, made), {}); + const { tools, getResult } = fishermanTools({} as any, store(undefined, made), {}); made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); await tools.finish.execute( @@ -113,7 +114,7 @@ describe('ledger-derived results', () => { it('reports failure when the loop ends without finish, even after a successful write', async () => { const made: any[] = []; - const { getResult } = createFishermanTools({} as any, store(undefined, made), {}); + const { getResult } = fishermanTools({} as any, store(undefined, made), {}); made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); made.push(madeWrite('POST', '/api/tests', 400)); @@ -126,7 +127,7 @@ describe('ledger-derived results', () => { it('reports failure with a reason when the loop ends with no successful writes', async () => { const made: any[] = []; - const { getResult } = createFishermanTools({} as any, store(undefined, made), {}); + const { getResult } = fishermanTools({} as any, store(undefined, made), {}); made.push(madeWrite('POST', '/api/tests', 400)); const result = getResult(); @@ -136,7 +137,7 @@ describe('ledger-derived results', () => { it('treats a text-only turn as finish, using the text as summary when writes succeeded', () => { const made: any[] = []; - const { finishFromText, getResult, isFinished } = createFishermanTools({} as any, store(undefined, made), {}); + const { finishFromText, getResult, isFinished } = fishermanTools({} as any, store(undefined, made), {}); made.push(madeWrite('POST', '/api/suites', 201, { id: 's1' })); finishFromText('Created the suite'); @@ -150,7 +151,7 @@ describe('ledger-derived results', () => { it('keeps the honest failure summary when a text-only turn ends a run with no successful writes', () => { const made: any[] = []; - const { finishFromText, getResult } = createFishermanTools({} as any, store(undefined, made), {}); + const { finishFromText, getResult } = fishermanTools({} as any, store(undefined, made), {}); made.push(madeWrite('POST', '/api/tests', 400)); finishFromText('All done successfully'); @@ -161,6 +162,10 @@ describe('ledger-derived results', () => { }); }); +function fishermanTools(apiClient: any, requestStore: any, opts: any): any { + return createFishermanTools(apiClient, requestStore, new RequestHaul(requestStore), opts); +} + function store(captured?: any, made: any[] = []): any { return { findCapturedRequest: () => captured, diff --git a/tests/unit/request-store.test.ts b/tests/unit/request-store.test.ts index bfc27802..e92f95ae 100644 --- a/tests/unit/request-store.test.ts +++ b/tests/unit/request-store.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { RequestHaul } from '../../src/ai/fisherman/request-haul.js'; import { RequestResult } from '../../src/api/request-result.js'; import { RequestStore } from '../../src/api/request-store.js'; @@ -317,3 +318,32 @@ describe('extractAuthHeaders session gating', () => { expect(store.extractAuthHeaders()).toEqual({}); }); }); + +describe('RequestHaul', () => { + let outputDir: string; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('sees only the requests made after it was created', () => { + const store = new RequestStore(outputDir); + store.addMadeRequest(makeRequest('GET', '/api/suites', 200)); + store.addMadeRequest(makeRequest('POST', '/api/suites', 201)); + + const haul = new RequestHaul(store); + store.addMadeRequest(makeRequest('POST', '/api/tests', 422)); + const created = makeRequest('POST', '/api/tests', 201); + created.rawResponseBodyValue = JSON.stringify({ id: 42, title: 'Test A' }); + store.addMadeRequest(created); + + expect(haul.requests()).toHaveLength(2); + expect(haul.failed()).toHaveLength(1); + expect(haul.successfulWrites()).toHaveLength(1); + expect(haul.byId().get('42')?.path).toBe('/api/tests'); + }); +});