diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a824367..19c5e45c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,41 @@ # Changelog +## 2026-08-30 + +### Changes + +- [Fisherman] In replicate mode, API requests now authenticate with the current browser session: + cookies are taken from the live jar filtered to the API origin, the CSRF token is read from the + page, and auth headers are never reused from previous sessions' captured requests. Achieve mode + authenticates solely through `api.headers` config. +- [Fisherman] A turn without a tool call now ends the run as a finish instead of erroring: tool + choice is no longer forced, and when writes already succeeded the model's text becomes the + summary while created items still come from the request ledger. Models that close with prose + instead of the `finish` tool no longer trigger retries that re-create the same data. + ## 2026-08-29 ### Changes +- [Fisherman] No longer treats its own past API calls as captured browser traffic. Replicate mode + used to reload every request it had ever made — rejected ones included — and hand the next run a + failed request body as "the example", sending it into long guessing loops. Only real browser + requests are read back now; a project with no captured browser traffic reports data preparation + as unavailable instead of replaying its own mistakes. +- [Fisherman] Picks the best captured example for an endpoint instead of the first one found: an + exact endpoint match beats a deeper sub-path, a successful request beats a rejected one, and + newer beats older. +- [Fisherman] The endpoint list shown to the AI is scoped to the page being tested by matching the + page URL against captured request paths, so endpoints from other projects no longer appear. When + scoping isn't possible the prompt says so explicitly instead of silently listing everything. +- [Fisherman] `finish` no longer reports success on faith. Reporting success requires at least one + API write that actually succeeded in this run, and every claimed created item is checked against + the ids the API really returned — unverifiable claims are dropped, and each confirmed item shows + which request created it. A run that ends without finishing now reports how many requests were + made, how many succeeded, and the last failure, instead of an empty result. +- [Fisherman] Stops after four failures in a row against the same endpoint instead of spending the + whole iteration budget guessing request bodies. +- [Pilot] Precondition steps now record which API request created each item. - [Provider] Groq prompt cache hits are counted again. Groq reports how much of a prompt it served from cache, but the pinned `@ai-sdk/groq` build read that number out of the response and then dropped it, so every Groq request was recorded as a full-price miss and the cache hit rate showed diff --git a/docs/superpowers/plans/2026-08-29-fisherman-reliability.md b/docs/superpowers/plans/2026-08-29-fisherman-reliability.md new file mode 100644 index 00000000..550053fa --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-fisherman-reliability.md @@ -0,0 +1,953 @@ +# Fisherman Reliability Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make Fisherman (the API test-data preparation agent) produce trustworthy results in replicate mode: real request examples, scope-correct endpoint lists, and a `finish` report derived from what was actually executed. + +**Architecture:** All fixes are deterministic-tier changes in the data layer (`RequestStore`, `RequestResult`) and the tool glue (`fisherman-tools.ts`), following the escalation ladder in CLAUDE.md: deterministic filters gate what the model sees, and a ledger of actually-made requests verifies what the model claims (generate-then-verify). One prompt line and one loop guard land in `fisherman.ts`. No new agents, no new envelope keys, no new files except one integration test. + +**Tech Stack:** Bun, TypeScript, Vercel AI SDK tools, `@copilotkit/aimock` for integration tests. + +**Spec:** `docs/superpowers/specs/2026-08-29-fisherman-reliability-design.md` (trace evidence, root causes, and design decisions; the Findings section below is the working summary). + +## Global Constraints + +- Bun only, never Node.js; run `bun run format` after each code change +- No code comments; avoid ternaries; private methods after public; premature exit over if/else +- Prompts and rules stay general — never encode a specific failing input from the trace review into a prompt, rule, or validator +- Never start the regression CI run (`regression` label / `gh workflow run`) — only the user applies the label +- `bun test tests/unit tests/integration` must pass before every commit +- Work on a fresh branch off `main` (project convention: `bunosh worktree:create fisherman-reliability`); do not build on `fix/skip-planning-on-error-pages` + +--- + +## Findings (why each task exists) + +From Langfuse traces of the only real Fisherman episodes (2026-04-30, 2026-06-01), verified against the current code: + +1. **Self-poisoning store.** `addMadeRequest()` saves every request Fisherman itself makes — 400s included — into `output/requests/`. `loadFromDisk()` reads that whole directory back as `capturedRequests`, indistinguishable from browser XHR. A run is handed its predecessor's rejected bodies as "the captured example". Provenance is *already* encoded in the file id prefix: browser captures are `xhr_*` (`xhr-capture.ts:72`), Fisherman's own calls are unprefixed (`api-client.ts:58`), and `fail_*` records are never saved to disk (`addFailedRequest` doesn't call `save()`). +2. **First-match spec lookup.** `findCapturedRequest()` is `find(method && path.startsWith(prefix))` — no status ranking, no exactness: a stale 400 displaces a good 200, and `/suites` matches `/suites/123/move`. +3. **Scope filter never matches.** `getWriteRequestsForScope()` prefix-matches a page URL (`/projects/…`) against API paths (`/api/…`), always falls through to `'/'` — every captured write from every project, which produced a cross-project endpoint in the Apr 30 prompt. Degradation is silent. +4. **`finish` is unconditional success.** It writes the model's own `created` array through verbatim (`fisherman-tools.ts:150`). A run that created nothing — or created a *test* when a *milestone* was asked for — reports success, and Pilot passes it on as a satisfied precondition. +5. **Silent exhaustion & clobbered status.** Hitting max iterations without `finish` returns `summary: ''`, so Pilot logs nothing and the vision check is told the reason is "unknown". In the `request` tool, `...extractKeyFields()` spreads *after* the `status` key, so a body field named `status` overwrites the HTTP status, and the depth-5 first-id scan surfaces ids the run never created. + +Design decisions that differ from the earlier proposed plan: + +- **No new `source:` envelope key.** The `xhr_` id prefix already has a single writer (`generateRequestId`) and a closed vocabulary; filtering on it in `loadFromDisk()` is the whole fix and migrates poisoned directories for free. +- **Reuse `isDynamicSegment()`** from `src/utils/url-matcher.ts` for id-shaped path segments instead of a new classifier. +- **Scope = most selective shared segment**, not max-shared-segment count (which would drop same-project endpoints that don't match the deepest page path, and a shared literal like `projects` must not win over a project slug). +- **`finish` is gated, not replaced.** The model still names types and summarizes; the ledger verifies ids and vetoes success with zero successful writes. + +Behavioral change to state in the CHANGELOG: a `output/requests/` directory containing only Fisherman-made files now yields zero captures, so replicate mode reports itself disabled — correct, since there was never real browser traffic to replicate. + +Blast radius (verified by grep, 2026-08-29): `loadFromDisk`, `getCapturedRequests`, `toEndpointList`, `findCapturedRequest`, and `getWriteRequestsForScope` are consumed only by `fisherman.ts`, `fisherman-tools.ts`, and tests — nothing in `boat/` or `bin/` touches them, so the behavior changes in Tasks 1–3 affect Fisherman alone. + +--- + +### Task 1: `loadFromDisk` admits only browser captures + +**Files:** +- Modify: `src/api/request-store.ts:120-136` +- Test: `tests/unit/request-store.test.ts` + +**Interfaces:** +- Consumes: existing id prefixes — `xhr_` from `xhr-capture.ts`, unprefixed from `api-client.ts` +- Produces: `loadFromDisk(): void` unchanged signature; `capturedRequests` now contains only browser-captured requests. Task 2/3 ranking and scoping rely on this. + +- [ ] **Step 1: Write the failing test** + +Extend the `makeRequest` helper in `tests/unit/request-store.test.ts` with an optional id: + +```ts +function makeRequest(method: string, path: string, status: number, id?: string): RequestResult { + counter++; + return new RequestResult({ + id: id || `req_${counter}`, + method, + path, + fullUrl: path, + requestHeaders: {}, + status, + statusText: String(status), + responseHeaders: {}, + timing: 0, + timestamp: new Date(), + }); +} +``` + +Add a new describe block (reuses the existing `outputDir`/`store` beforeEach): + +```ts +describe('RequestStore loadFromDisk', () => { + it('loads only browser-captured requests from disk', () => { + makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites').save(outputDir); + makeRequest('POST', '/api/suites', 400, '001_POST_api_suites').save(outputDir); + + const fresh = new RequestStore(outputDir); + fresh.loadFromDisk(); + + expect(fresh.getCapturedRequests()).toHaveLength(1); + expect(fresh.getCapturedRequests()[0].id).toBe('xhr_001_POST_api_suites'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/unit/request-store.test.ts` +Expected: FAIL — `getCapturedRequests()` has length 2. + +- [ ] **Step 3: Implement the filter** + +In `src/api/request-store.ts` `loadFromDisk()`, change the file filter line to: + +```ts +const files = readdirSync(requestsDir).filter((f) => f.startsWith('xhr_') && f.endsWith('.request.yaml')); +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/unit/request-store.test.ts` +Expected: PASS (all, including the pre-existing failure-listener tests). + +- [ ] **Step 5: Format and commit** + +```bash +bun run format +git add src/api/request-store.ts tests/unit/request-store.test.ts +git commit -m "fix(fisherman): stop replicate mode reading its own past requests as captures" +``` + +--- + +### Task 2: Rank captured examples in `findCapturedRequest` + +**Files:** +- Modify: `src/api/request-store.ts:111-114` and `normalizePathPattern` at `src/api/request-store.ts:150-152` +- Test: `tests/unit/request-store.test.ts` + +**Interfaces:** +- Consumes: `isDynamicSegment(segment: string): boolean` from `src/utils/url-matcher.ts` +- Produces: `findCapturedRequest(method: string, searchPath: string): RequestResult | undefined` — same signature, now ranked. The search path may contain literal `{id}` segments (as printed by Task 3's endpoint list) or concrete ids; both match stored requests. Contract for Task 3: endpoint lists print `normalizePathPattern` output, and this method accepts exactly those strings. + +Ranking: candidates share the method and their normalized path starts with the normalized search path at segment boundaries. Exact segment-count match beats a deeper sub-path; within a tier, `status < 400` beats a rejection; within that, newest `timestamp` wins. An exact-path rejection deliberately beats a sub-path success — `getEndpointSpec`'s existing `usable: false` branch then explains it to the model. + +Accepted over-generalization: `isDynamicSegment` also fires on short mixed-alphanumeric segments like API version prefixes, so `/api/v2/posts` lists as `POST /api/{id}/posts`. That is recoverable by design — `getEndpointSpec` returns the stored request's *concrete* `path`, and the WORKFLOW already mandates a spec lookup before first use — so do not "fix" it by narrowing the normalization; that would reintroduce the weak dedup. + +- [ ] **Step 1: Write the failing tests** + +```ts +describe('findCapturedRequest ranking', () => { + it('prefers a successful capture over a rejected one for the same endpoint', () => { + store.addCapturedRequest(makeRequest('POST', '/api/suites', 400)); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201)); + + expect(store.findCapturedRequest('POST', '/api/suites')?.status).toBe(201); + }); + + it('prefers the exact endpoint over a deeper sub-path', () => { + store.addCapturedRequest(makeRequest('POST', '/api/suites/42/move', 200)); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 400)); + + expect(store.findCapturedRequest('POST', '/api/suites')?.path).toBe('/api/suites'); + }); + + it('matches {id} patterns and concrete ids against stored ids', () => { + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/1a2b3c4d', 200)); + + expect(store.findCapturedRequest('PATCH', '/api/suites/{id}')?.status).toBe(200); + expect(store.findCapturedRequest('PATCH', '/api/suites/9f8e7d6c')?.status).toBe(200); + }); + + it('prefers the newest among otherwise equal candidates', () => { + const older = makeRequest('POST', '/api/suites', 201); + older.timestamp = new Date('2026-01-01'); + const newer = makeRequest('POST', '/api/suites', 201); + newer.timestamp = new Date('2026-02-01'); + store.addCapturedRequest(older); + store.addCapturedRequest(newer); + + expect(store.findCapturedRequest('POST', '/api/suites')?.id).toBe(newer.id); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/unit/request-store.test.ts` +Expected: FAIL — first-match behavior returns the 400 / the sub-path / the older entry. + +- [ ] **Step 3: Implement normalization and ranking** + +In `src/api/request-store.ts`, add the import and rewrite `normalizePathPattern` on top of the existing shared classifier: + +```ts +import { isDynamicSegment } from '../utils/url-matcher.ts'; +``` + +```ts +function normalizePathPattern(urlPath: string): string { + return urlPath + .split('/') + .map((segment) => (segment && isDynamicSegment(segment) ? '{id}' : segment)) + .join('/'); +} +``` + +Replace `findCapturedRequest`: + +```ts +findCapturedRequest(method: string, searchPath: string): RequestResult | undefined { + const upper = method.toUpperCase(); + const search = normalizePathPattern(searchPath).split('/').filter(Boolean); + + let best: RequestResult | undefined; + let bestScore = -1; + + for (const req of this.capturedRequests) { + if (req.method !== upper) continue; + const segments = normalizePathPattern(req.path).split('/').filter(Boolean); + if (segments.length < search.length) continue; + if (!search.every((segment, i) => segment === segments[i])) continue; + + let score = 0; + if (segments.length === search.length) score += 4; + if (req.status < 400) score += 2; + if (score < bestScore) continue; + if (score === bestScore && best && req.timestamp <= best.timestamp) continue; + best = req; + bestScore = score; + } + + return best; +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/unit/request-store.test.ts tests/unit/fisherman-tools.test.ts` +Expected: PASS. (`fisherman-tools.test.ts` duck-types `findCapturedRequest`, so it is unaffected; running it confirms.) + +- [ ] **Step 5: Format and commit** + +```bash +bun run format +git add src/api/request-store.ts tests/unit/request-store.test.ts +git commit -m "fix(fisherman): rank captured examples — exact path, then success, then recency" +``` + +--- + +### Task 3: Scope the endpoint list by shared path segment, own it in RequestStore + +**Files:** +- Modify: `src/api/request-store.ts:80-93` (`toEndpointList`), `src/api/request-store.ts:138-141` (`getWriteRequestsForScope`) +- Modify: `src/ai/fisherman.ts:163-185` (`buildEndpointList`), `src/ai/fisherman.ts:187-217` (`buildSystemPrompt`) +- Test: `tests/unit/request-store.test.ts` + +**Interfaces:** +- Consumes: Task 2's `normalizePathPattern` +- Produces: `getWriteRequestsForScope(scopePath: string): RequestResult[]` — same signature; returns `[]` (not everything) when nothing matches the scope. `toEndpointList(scopePath?: string): string` — optional scope parameter; lines are `METHOD normalized-path`, deduplicated. Fisherman's `buildEndpointList` delegates to it and no longer builds lines itself. + +Scope rule (spec sentence): score each scope-URL path segment by how many captured writes contain it in their path; the scope key is the non-zero segment with the fewest matches, leftmost on ties; a request is in scope when its path contains that segment. A root or empty scope returns all writes. This matches a page URL's project/tenant slug to API paths without any site-specific knowledge, and a shared generic literal (matching everything) loses to the selective slug. + +- [ ] **Step 1: Write the failing tests** + +```ts +describe('RequestStore scope filtering', () => { + it('scopes writes by the most selective segment shared with the page URL', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('PATCH', '/api/other-shop/suites/5', 200)); + + const scoped = store.getWriteRequestsForScope('/projects/alpha-shop/suites'); + + expect(scoped).toHaveLength(1); + expect(scoped[0].path).toBe('/api/alpha-shop/suites'); + }); + + it('returns nothing when the scope shares no segment with any write', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + + expect(store.getWriteRequestsForScope('/dashboard')).toHaveLength(0); + }); + + it('returns all writes for the root scope', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('POST', '/api/other-shop/labels', 201)); + + expect(store.getWriteRequestsForScope('/')).toHaveLength(2); + }); + + it('deduplicates endpoint list lines by id pattern', () => { + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/1a2b3c4d', 200)); + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/9f8e7d6c', 200)); + + expect(store.toEndpointList()).toBe('PATCH /api/suites/{id}'); + }); + + it('scopes the endpoint list when a scope path is given', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('POST', '/api/other-shop/suites', 201)); + + expect(store.toEndpointList('/projects/alpha-shop')).toBe('POST /api/alpha-shop/suites'); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/unit/request-store.test.ts` +Expected: FAIL — prefix matching returns 0 for page URLs, and dedup keeps both id-bearing paths. + +- [ ] **Step 3: Implement scope + scoped list in RequestStore** + +Replace `getWriteRequestsForScope` and `toEndpointList` in `src/api/request-store.ts`: + +```ts +getWriteRequestsForScope(scopePath: string): RequestResult[] { + const writeMethods = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); + const writes = this.capturedRequests.filter((r) => writeMethods.has(r.method)); + const scopeSegments = scopePath.split('/').filter(Boolean); + if (scopeSegments.length === 0) return writes; + + let scopeKey = ''; + let fewest = Number.POSITIVE_INFINITY; + for (const segment of scopeSegments) { + const matches = writes.filter((r) => r.path.split('/').includes(segment)).length; + if (matches === 0 || matches >= fewest) continue; + scopeKey = segment; + fewest = matches; + } + if (!scopeKey) return []; + + return writes.filter((r) => r.path.split('/').includes(scopeKey)); +} +``` + +```ts +toEndpointList(scopePath?: string): string { + let requests = this.capturedRequests; + if (scopePath) requests = this.getWriteRequestsForScope(scopePath); + + const seen = new Set(); + const lines: string[] = []; + + for (const req of requests) { + const key = `${req.method} ${normalizePathPattern(req.path)}`; + if (seen.has(key)) continue; + seen.add(key); + lines.push(key); + } + + return lines.join('\n'); +} +``` + +- [ ] **Step 4: Delegate from Fisherman and surface degradation** + +In `src/ai/fisherman.ts`, add a field next to the other privates: + +```ts +private scopeDegraded = false; +``` + +Replace `buildEndpointList`: + +```ts +private buildEndpointList(scopeUrl?: string): string { + this.scopeDegraded = false; + if (this.mode === 'achieve' && this.spec) { + const specEndpoints = listAllEndpoints(this.spec, this.baseEndpoint); + if (specEndpoints) return specEndpoints; + } + + const scoped = this.requestStore.toEndpointList(scopeUrl || '/'); + if (scoped) return scoped; + + this.scopeDegraded = true; + return this.requestStore.toEndpointList(); +} +``` + +In `buildSystemPrompt`, replace the `scopeBlock` line with: + +```ts +let scopeBlock = ''; +if (scopeUrl) { + scopeBlock = `\n\nSCOPE: You are operating within ${scopeUrl}.\nAll created items must belong to this scope.`; + if (this.scopeDegraded) scopeBlock += '\nThe endpoint list could not be narrowed to this scope and may include endpoints belonging to other scopes. Before writing, confirm the target belongs to this scope.'; +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `bun test tests/unit/ tests/integration/` +Expected: PASS. + +- [ ] **Step 6: Format and commit** + +```bash +bun run format +git add src/api/request-store.ts src/ai/fisherman.ts tests/unit/request-store.test.ts +git commit -m "fix(fisherman): scope endpoint list by shared URL segment, announce degradation" +``` + +--- + +### Task 4: Keep the HTTP status authoritative in the `request` tool + +**Files:** +- Modify: `src/ai/fisherman-tools.ts:76-125` (`request` tool) +- Test: `tests/unit/fisherman-tools.test.ts` + +**Interfaces:** +- Consumes: existing `extractKeyFields` +- Produces: successful `request` tool results are `{ success: true, status: number, extracted: Record }` — extraction is namespaced so a response-body field can never clobber the HTTP `status`, and the model sees clearly which part is the transport verdict and which is body data. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/unit/fisherman-tools.test.ts`: + +```ts +it('keeps the HTTP status authoritative over response body fields', async () => { + const apiClient = { + request: async () => ({ status: 201, statusText: 'Created', rawResponseBody: '', responseBody: { id: 7, status: 'draft' } }), + }; + const { tools } = createFishermanTools(apiClient as any, store(), {}); + + const result: any = await tools.request.execute({ method: 'POST', path: '/items' }, {} as any); + + expect(result.status).toBe(201); + expect(result.extracted).toEqual({ id: 7, status: 'draft' }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `bun test tests/unit/fisherman-tools.test.ts` +Expected: FAIL — `result.status` is `'draft'` and `result.extracted` is undefined. + +- [ ] **Step 3: Namespace the extraction** + +In the `request` tool's success return, replace the spread: + +```ts +const extracted = extractKeyFields(reqResult.responseBody); +tag('success').log(`Fisherman: ${input.method} ${input.path} > ${statusLine}`); +return { + success: true, + status: reqResult.status, + extracted, +}; +``` + +Update the tool description's second line to: `Returns status, plus IDs and names auto-extracted from the response under 'extracted'.` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `bun test tests/unit/fisherman-tools.test.ts` +Expected: PASS. + +- [ ] **Step 5: Format and commit** + +```bash +bun run format +git add src/ai/fisherman-tools.ts tests/unit/fisherman-tools.test.ts +git commit -m "fix(fisherman): namespace extracted response fields so body data cannot clobber HTTP status" +``` + +--- + +### Task 5: Derive the result from the request ledger + +**Files:** +- Modify: `src/ai/fisherman-tools.ts` (`createFishermanTools`, `finish` tool, `FishermanResult` type) +- Modify: `src/api/request-result.ts` (add `isWrite` getter) +- Modify: `src/ai/pilot.ts:764-768` (show `via` in the precondition step text) +- Test: `tests/unit/fisherman-tools.test.ts` + +**Interfaces:** +- Consumes: `RequestStore.getMadeRequests(): RequestResult[]`, `RequestResult.extractIdAndTitle(): { id?, title? }`, `RequestResult.toSummary(): string` +- Produces: `RequestResult` gains `get isWrite(): boolean` (method is POST/PUT/PATCH/DELETE). `FishermanResult.created` items gain optional `via?: string` (`"POST /api/suites"`). `createFishermanTools` snapshots the made-request count at creation; everything after that index is "this run". `finish` with zero successful writes in this run returns `{ finished: false, error }` and does not end the loop. `getResult()` without a `finish`/`stop` synthesizes an honest summary and ledger-derived `created` instead of `summary: ''`. + +This is the generate-then-verify ladder from CLAUDE.md: the model still names types and writes the summary (judgment), while success and ids are checked against what HTTP actually returned (deterministic, loud). + +- [ ] **Step 1: Write the failing tests** + +Replace the `store()` helper at the bottom of `tests/unit/fisherman-tools.test.ts` and add a made-request factory: + +```ts +function store(captured?: any, made: any[] = []): any { + return { + findCapturedRequest: () => captured, + addMadeRequest: (r: any) => made.push(r), + getMadeRequests: () => made, + }; +} + +function madeWrite(method: string, path: string, status: number, body: Record = {}): any { + return { + method, + path, + status, + error: undefined, + isWrite: true, + extractIdAndTitle: () => body, + toSummary: () => `${method} ${path} → ${status} (0ms)`, + }; +} +``` + +Add the tests: + +```ts +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 result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: '1' }] }, {} as any); + + expect(result.finished).toBe(false); + expect(result.error).toContain('No successful write'); + expect(isFinished()).toBe(false); + }); + + 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 result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: 's1' }] }, {} as any); + + expect(result.finished).toBe(false); + expect(isFinished()).toBe(false); + }); + + it('drops created items whose id no write response returned, keeps verified ones with via', async () => { + const made: any[] = []; + const { tools, getResult } = createFishermanTools({} as any, store(undefined, made), {}); + made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); + + await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: 's1' }, { type: 'milestone', id: 'm9' }] }, {} as any); + + const result = getResult(); + expect(result.success).toBe(true); + expect(result.created).toEqual([{ type: 'suite', id: 's1', via: 'POST /api/suites' }]); + }); + + it('synthesizes an honest summary when the loop ends without finish', async () => { + const made: any[] = []; + const { getResult } = createFishermanTools({} as any, store(undefined, made), {}); + made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); + made.push(madeWrite('POST', '/api/tests', 400)); + + const result = getResult(); + expect(result.success).toBe(true); + expect(result.summary).toContain('1 successful write'); + expect(result.summary).toContain('POST /api/tests → 400'); + expect(result.created[0].id).toBe('s1'); + }); + + 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), {}); + made.push(madeWrite('POST', '/api/tests', 400)); + + const result = getResult(); + expect(result.success).toBe(false); + expect(result.summary).not.toBe(''); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `bun test tests/unit/fisherman-tools.test.ts` +Expected: FAIL — `finish` currently always succeeds and `getResult()` returns `summary: ''`. (The four pre-existing tests must still pass — the updated `store()` helper keeps their behavior.) + +- [ ] **Step 3: Add the `isWrite` getter** + +In `src/api/request-result.ts`, after the `responseBody` getter: + +```ts +get isWrite(): boolean { + return ['POST', 'PUT', 'PATCH', 'DELETE'].includes(this.method); +} +``` + +- [ ] **Step 4: Implement the ledger in `createFishermanTools`** + +In `src/ai/fisherman-tools.ts`, replace the head of `createFishermanTools` and the `finish` tool: + +```ts +export function createFishermanTools(apiClient: ApiClient, requestStore: RequestStore, 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()); + const isFinished = () => finished; +``` + +Replace the `finish` tool's `execute`: + +```ts +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 viaById = new Map(); + for (const write of writes) { + const { id } = write.extractIdAndTitle(); + if (id === undefined) continue; + viaById.set(String(id), `${write.method} ${write.path}`); + } + + const verified: FishermanResult['created'] = []; + for (const item of created) { + if (item.id === undefined) { + verified.push(item); + continue; + } + const via = viaById.get(String(item.id)); + if (!via) { + tag('warning').log(`Fisherman: dropped unverified created item ${item.type} (id: ${item.id})`); + continue; + } + verified.push({ ...item, via }); + } + if (verified.length === 0) verified.push(...writes.map(toCreatedItem)); + + tag('success').log(`Fisherman done: ${summary}`); + finished = true; + result = { success: true, summary, created: verified, failed: failed || [] }; + return { finished: true }; +}, +``` + +Add the module-private helpers after the exported function, and extend the type at the end of the file: + +```ts +function synthesizeResult(made: RequestResult[], writes: RequestResult[]): FishermanResult { + const failures = made.filter((r) => r.status >= 400 || r.error); + 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()}`; + return { success: writes.length > 0, summary, created: writes.map(toCreatedItem), failed: [] }; +} + +function toCreatedItem(write: RequestResult): FishermanResult['created'][number] { + const { id, title } = write.extractIdAndTitle(); + const segments = write.path.split('/').filter((s) => s && !isDynamicSegment(s)); + return { type: segments[segments.length - 1] || 'item', id, title, via: `${write.method} ${write.path}` }; +} +``` + +```ts +export interface FishermanResult { + success: boolean; + summary: string; + created: Array<{ type: string; id?: string | number; title?: string; via?: string }>; + failed: Array<{ type: string; reason: string }>; +} +``` + +Add the imports at the top: `import type { RequestResult } from '../api/request-result.ts';` and `import { isDynamicSegment } from '../utils/url-matcher.ts';` + +- [ ] **Step 5: Show `via` in Pilot's precondition step** + +In `src/ai/pilot.ts` `buildPreconditionTool`, extend the item formatting (currently lines 764-768): + +```ts +const parts = [c.type]; +if (c.title) parts.push(`"${c.title}"`); +if (c.id) parts.push(`(id: ${c.id})`); +if (c.via) parts.push(`via ${c.via}`); +``` + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `bun test tests/unit/ tests/integration/` +Expected: PASS. + +- [ ] **Step 7: Format and commit** + +```bash +bun run format +git add src/ai/fisherman-tools.ts src/api/request-result.ts src/ai/pilot.ts tests/unit/fisherman-tools.test.ts +git commit -m "fix(fisherman): verify finish against the request ledger, synthesize result on exhaustion" +``` + +--- + +### Task 6: Repeated-failure guard and no-substitution rule + +**Files:** +- Modify: `src/ai/fisherman.ts` (`prepareData` loop, system prompt RULES, new private method) + +**Interfaces:** +- Consumes: `requestStore.getMadeRequests()`, Task 5's honest `getResult()` (which turns an early stop into an accurate report) +- Produces: the run ends after `REPEATED_FAILURE_LIMIT` consecutive failures against one endpoint instead of burning 15 iterations × 5 roundtrips on body guesses; one general prompt rule against creating substitute resource types. + +This mirrors the deterministic dead-loop detection StateManager does for navigation: identical repeats are a structural signal, not a judgment call. + +- [ ] **Step 1: Add the constant and the guard** + +In `src/ai/fisherman.ts` next to `MAX_ITERATIONS`: + +```ts +const REPEATED_FAILURE_LIMIT = 4; +``` + +In `prepareData`, capture the ledger start before the loop (after `createFishermanTools`): + +```ts +const ledgerStart = this.requestStore.getMadeRequests().length; +``` + +In the loop callback, after the `isFinished()` check: + +```ts +if (this.isStuckOnEndpoint(ledgerStart)) { + tag('warning').log('Fisherman: repeated failures on the same endpoint — stopping'); + stop(); + return; +} +``` + +Add the private method after the other private methods: + +```ts +private isStuckOnEndpoint(ledgerStart: number): boolean { + const made = this.requestStore.getMadeRequests().slice(ledgerStart); + 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); +} +``` + +- [ ] **Step 2: Add the prompt rule** + +In `buildSystemPrompt`'s RULES block, add one line after the retry rule: + +``` +- Create only the resource types that were requested. If no endpoint creates a requested type, call stop — never create a different type as a substitute +``` + +- [ ] **Step 3: Run tests** + +Run: `bun test tests/unit/ tests/integration/` +Expected: PASS. + +- [ ] **Step 4: Format and commit** + +```bash +bun run format +git add src/ai/fisherman.ts +git commit -m "fix(fisherman): stop after repeated failures on one endpoint, forbid substitute types" +``` + +--- + +### Task 7: Integration test — the full replicate-mode loop + +**Files:** +- Create: `tests/integration/fisherman.test.ts` + +**Interfaces:** +- Consumes: everything above — scoped prompt (Task 3), ledger-gated `finish` (Task 5); the aimock pattern from `tests/integration/prima-do.test.ts` (tool-call fixtures) and `tests/integration/planner.test.ts` (Provider setup, `extractPromptText`) +- Produces: end-to-end proof that a run's system prompt is scope-filtered and that an empty-handed `finish` is rejected and converted to an honest failure. + +- [ ] **Step 1: Write the integration test** + +```ts +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { createOpenAI } from '@ai-sdk/openai'; +import { LLMock } from '@copilotkit/aimock'; +import { Fisherman } from '../../src/ai/fisherman.ts'; +import { Provider } from '../../src/ai/provider.ts'; +import { RequestResult } from '../../src/api/request-result.ts'; +import { RequestStore } from '../../src/api/request-store.ts'; +import { ConfigParser } from '../../src/config.ts'; + +let counter = 0; +function requestResult(id: string, method: string, urlPath: string, status: number, body?: any): RequestResult { + counter++; + const result = new RequestResult({ + id, + method, + path: urlPath, + fullUrl: urlPath, + requestHeaders: {}, + requestBody: body, + status, + statusText: String(status), + responseHeaders: {}, + timing: 0, + timestamp: new Date(), + }); + return result; +} + +function toolCall(id: string, name: string, args: Record) { + return { id, name, arguments: JSON.stringify(args) }; +} + +function extractPromptText(entry: any): string { + if (!entry?.body?.messages) return ''; + return entry.body.messages + .map((message: any) => { + if (typeof message.content === 'string') return message.content; + if (Array.isArray(message.content)) { + return message.content + .filter((part: any) => part.type === 'text') + .map((part: any) => part.text || '') + .join('\n'); + } + return ''; + }) + .join('\n'); +} + +describe('Fisherman with aimock', () => { + let mock: LLMock; + let provider: Provider; + let outputDir: string; + let requestStore: RequestStore; + let apiResponses: RequestResult[]; + + beforeAll(async () => { + mock = new LLMock({ port: 0, logLevel: 'silent' }); + await mock.start(); + + const openai = createOpenAI({ baseURL: `${mock.url}/v1`, apiKey: 'test-key', compatibility: 'compatible' }); + ConfigParser.resetForTesting(); + ConfigParser.setupTestConfig(); + provider = new Provider({ model: openai.chat('test-model'), config: {} }); + }); + + beforeEach(() => { + mock.clearRequests(); + mock.resetMatchCounts(); + mock.clearFixtures(); + + outputDir = mkdtempSync(path.join(tmpdir(), 'fisherman-')); + requestStore = new RequestStore(outputDir); + requestStore.addCapturedRequest(requestResult('xhr_001_POST_api_alpha-shop_suites', 'POST', '/api/alpha-shop/suites', 201, { title: 'Suite' })); + requestStore.addCapturedRequest(requestResult('xhr_002_POST_api_other-shop_suites', 'POST', '/api/other-shop/suites', 201, { title: 'Suite' })); + apiResponses = []; + }); + + afterAll(async () => { + await mock.stop(); + ConfigParser.cleanupAllTestDirectories(); + }); + + function createFisherman(): Fisherman { + const apiClient = { + request: async () => apiResponses.shift(), + setHeaders: () => {}, + getHeaders: () => ({}), + }; + return new Fisherman(provider, apiClient as any, requestStore, async () => null, 'https://example.test/api', async () => ({})); + } + + it('scopes the prompt and reports verified created items with via', async () => { + const created = requestResult('made_1', 'POST', '/api/alpha-shop/suites', 201); + created.rawResponseBodyValue = JSON.stringify({ data: { id: 's1', title: 'Suite A' } }); + apiResponses.push(created); + + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'request', { method: 'POST', path: '/api/alpha-shop/suites', body: { title: 'Suite A' } })] }); + mock.on({ sequenceIndex: 1 }, { toolCalls: [toolCall('c2', 'finish', { summary: '1 suite created', created: [{ type: 'suite', id: 's1', title: 'Suite A' }] })] }); + mock.on({}, { content: 'done' }); + + const result = await createFisherman().prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(result.success).toBe(true); + expect(result.created).toEqual([{ type: 'suite', id: 's1', title: 'Suite A', via: 'POST /api/alpha-shop/suites' }]); + + const systemPrompt = extractPromptText(mock.getRequests()[0]); + expect(systemPrompt).toContain('POST /api/alpha-shop/suites'); + expect(systemPrompt).not.toContain('other-shop'); + }); + + it('rejects an empty-handed finish and returns an honest failure', async () => { + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'finish', { summary: 'all done', created: [{ type: 'suite', id: '99' }] })] }); + mock.on({}, { toolCalls: [toolCall('c2', 'stop', { reason: 'The data cannot be created' })] }); + + const result = await createFisherman().prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(result.success).toBe(false); + expect(result.created).toHaveLength(0); + expect(result.summary).toBe('The data cannot be created'); + }); +}); +``` + +Note: `addCapturedRequest` saves into the temp `outputDir`; `detectMode`'s `loadFromDisk` dedups by id, so replicate mode activates from the seeded captures. The `beforeAll` mirrors `tests/integration/prima-do.test.ts`. + +- [ ] **Step 2: Run the test** + +Run: `bun test tests/integration/fisherman.test.ts` +Expected: PASS. If the first fixture assertion fails on prompt content, print `extractPromptText(mock.getRequests()[0])` to see the actual endpoint list — the scoping from Task 3 must have filtered it. + +- [ ] **Step 3: Run the full suite, format, and commit** + +```bash +bun test tests/unit/ tests/integration/ +bun run format +git add tests/integration/fisherman.test.ts +git commit -m "test(fisherman): integration coverage for scoped prompts and ledger-gated finish" +``` + +--- + +### Task 8: Rollout — regression fixture and changelog + +**Files:** +- Modify: `tests/regression/fixture/explorbot.config.js:50-52` +- Modify: `CHANGELOG.md` (via the `/changelog` skill) + +**Interfaces:** +- Consumes: `explorbot.ts:329` — replicate mode requires `fisherman: { enabled: true }` when no `api` config block exists +- Produces: the next user-triggered regression run exercises Fisherman in replicate mode for the first time since the fixes. + +- [ ] **Step 1: Verify the fixture can feed replicate mode** + +Replicate mode needs browser-captured JSON write XHRs before any `precondition()` fires. Inspect the Trackly fixture scenarios and knowledge (`tests/regression/fixture/`, `tests/regression/seeds/`) and confirm at least one scenario performs a create/edit through the UI. If no scenario produces a write before preconditions are wanted, report that to the user in the PR description instead of silently flipping the flag — the flip would prove nothing. + +- [ ] **Step 2: Enable Fisherman in the fixture** + +In `tests/regression/fixture/explorbot.config.js`: + +```js +fisherman: { + enabled: true, +}, +``` + +- [ ] **Step 3: Update the changelog** + +Invoke the `/changelog` skill. The entry must mention the behavioral change: request directories containing only Fisherman-made files (no `xhr_*` captures) no longer activate replicate mode, and `finish` now fails when no successful write request was made. + +- [ ] **Step 4: Final check and commit** + +```bash +bun test tests/unit/ tests/integration/ +bun run format +git add tests/regression/fixture/explorbot.config.js CHANGELOG.md +git commit -m "chore(fisherman): enable replicate mode in the regression fixture" +``` + +- [ ] **Step 5: Hand regression to the user** + +Do not start the regression workflow. Tell the user the branch is ready for a regression run and that they can apply the `regression` label when they want it. Acceptance criterion for the whole plan: one trace where `precondition()` returns created ids and those ids are visible on the page the Tester then acts on. diff --git a/docs/superpowers/plans/2026-08-30-fisherman-live-session-auth.md b/docs/superpowers/plans/2026-08-30-fisherman-live-session-auth.md new file mode 100644 index 00000000..b10019b5 --- /dev/null +++ b/docs/superpowers/plans/2026-08-30-fisherman-live-session-auth.md @@ -0,0 +1,457 @@ +# Fisherman Live-Session Auth Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** In replicate mode, Fisherman API requests carry the current browser session's credentials — cookies filtered to the API origin and a live CSRF token — and never a credential scraped from a previous session's captures. Achieve mode keeps authenticating solely through `api.headers` config. + +**Architecture:** Three layers change. `RequestStore.extractAuthHeaders` gains a session gate (only captures made during this run, newest first) and stops scraping cookies. `Fisherman.refreshAuth` reorders precedence to captured < live browser < config. The DI-glue provider in `explorbot.ts` filters the cookie jar by the API origin and additionally reads the page's `meta[name="csrf-token"]`. + +**Tech Stack:** Bun, TypeScript, Playwright (via Explorer.withPage), bun:test, @copilotkit/aimock for the integration test. + +**Spec:** `docs/superpowers/specs/2026-08-30-fisherman-live-session-auth-design.md` + +## Global Constraints + +- Bun only — never Node.js. Run tests with `bun test`. +- Work in the worktree `/home/davert/projects/explorbot-fisherman-reliability`, branch `fisherman-reliability` (continues PR #160 — do NOT open a new PR). +- No code comments. No ternary operators. Premature exit over if/else. +- Run `bun run format` after each code change, before each commit. +- NEVER trigger the regression CI workflow (no `regression` label, no `gh workflow run regression.yml`). Only the user does that. +- Task 4 is severable: if the user cuts it, Tasks 1–3 stand alone and the provider keeps its `cookieProvider` name. + +--- + +### Task 1: Session-gated auth header extraction (RequestStore) + +**Files:** +- Modify: `src/api/request-store.ts:6` (AUTH_HEADERS), `src/api/request-store.ts:98-112` (extractAuthHeaders), field near `src/api/request-store.ts:9-16` +- Modify: `src/api/request-result.ts:176` (timestamp fallback in `RequestResult.load`) +- Test: `tests/unit/request-store.test.ts` + +**Interfaces:** +- Consumes: existing `RequestResult` (`timestamp: Date`, `requestHeaders: Record`), `RequestStore.addCapturedRequest`, `RequestStore.loadFromDisk`. +- Produces: `extractAuthHeaders(): Record` — same signature, but returns only `authorization` / `x-api-key` / `x-csrf-token` headers from captures made during this session, newest value first. Task 2 relies on this being safe to apply before live browser headers. + +- [ ] **Step 1: Write the failing tests** + +Extend the `makeRequest` helper in `tests/unit/request-store.test.ts` with an optional headers argument: + +```ts +let counter = 0; +function makeRequest(method: string, path: string, status: number, id?: string, headers: Record = {}): RequestResult { + counter++; + return new RequestResult({ + id: id || `req_${counter}`, + method, + path, + fullUrl: path, + requestHeaders: headers, + status, + statusText: String(status), + responseHeaders: {}, + timing: 0, + timestamp: new Date(), + }); +} +``` + +Add a new describe block at the end of the file (before the final `loadFromDisk` block is fine too — position does not matter): + +```ts +describe('extractAuthHeaders session gating', () => { + let outputDir: string; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('ignores auth headers from captures of previous sessions', () => { + const stale = makeRequest('DELETE', '/api/old-project/suites', 200, 'xhr_033_DELETE_api_old', { 'x-csrf-token': 'stale-token' }); + stale.timestamp = new Date('2026-07-07'); + stale.save(outputDir); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + + expect(store.extractAuthHeaders()).toEqual({}); + }); + + it('returns auth headers from captures made during this session', () => { + const store = new RequestStore(outputDir); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, undefined, { authorization: 'Bearer live', 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ authorization: 'Bearer live', 'x-csrf-token': 'live-token' }); + }); + + it('never returns cookie headers from captures', () => { + const store = new RequestStore(outputDir); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, undefined, { cookie: 'session=captured', 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'live-token' }); + }); + + it('prefers the newest session capture when values differ', () => { + const store = new RequestStore(outputDir); + const older = makeRequest('POST', '/api/suites', 201, undefined, { 'x-csrf-token': 'first' }); + older.timestamp = new Date(Date.now() + 1000); + const newer = makeRequest('POST', '/api/tests', 201, undefined, { 'x-csrf-token': 'second' }); + newer.timestamp = new Date(Date.now() + 2000); + store.addCapturedRequest(older); + store.addCapturedRequest(newer); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'second' }); + }); + + it('resolves a same-id collision between a stale disk file and a live capture', () => { + const stale = makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites', { 'x-csrf-token': 'stale-token' }); + stale.timestamp = new Date('2026-07-07'); + stale.save(outputDir); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites', { 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'live-token' }); + }); + + it('treats a capture file without a timestamp as stale', () => { + const requestsDir = join(outputDir, 'requests'); + mkdirSync(requestsDir, { recursive: true }); + writeFileSync(join(requestsDir, 'xhr_002_POST_api_x.request.yaml'), '---\nmethod: POST\nurl: /api/x\nfullUrl: /api/x\nheaders:\n x-csrf-token: orphan\nstatus: 200\nstatusText: OK\nresponseHeaders:\n---\n', 'utf8'); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + + expect(store.extractAuthHeaders()).toEqual({}); + }); +}); +``` + +Update the file's imports: add `mkdirSync, writeFileSync` to the `node:fs` import. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd /home/davert/projects/explorbot-fisherman-reliability && bun test tests/unit/request-store.test.ts` +Expected: the first, third, and sixth new tests FAIL (the stale, cookie, and no-timestamp values are currently returned). The second, fourth, and fifth pass by accident under the current code — they stay in as pins on the new sort-based behavior. + +- [ ] **Step 3: Implement** + +In `src/api/request-store.ts`, change the constant: + +```ts +const AUTH_HEADERS = ['authorization', 'x-api-key', 'x-csrf-token']; +``` + +Add a private field to `RequestStore` (private fields live after public methods is a rule for methods; fields stay at the top with the others): + +```ts + private sessionStartedAt = new Date(); +``` + +Replace `extractAuthHeaders`: + +```ts + extractAuthHeaders(): Record { + const headers: Record = {}; + const sessionCaptures = this.capturedRequests.filter((r) => r.timestamp >= this.sessionStartedAt).sort((a, b) => b.timestamp.getTime() - a.timestamp.getTime()); + + for (const req of sessionCaptures) { + for (const [key, value] of Object.entries(req.requestHeaders)) { + if (AUTH_HEADERS.includes(key.toLowerCase()) && !headers[key]) { + headers[key] = value; + } + } + } + + return headers; + } +``` + +In `src/api/request-result.ts` line 176, change the load fallback so an absent timestamp reads as stale: + +```ts + timestamp: new Date(meta.timestamp || 0), +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `bun test tests/unit/request-store.test.ts && bun test tests/unit/` +Expected: all PASS (the second command catches any other unit test that relied on the old fallback). + +- [ ] **Step 5: Format and commit** + +```bash +cd /home/davert/projects/explorbot-fisherman-reliability +bun run format +git add src/api/request-store.ts src/api/request-result.ts tests/unit/request-store.test.ts +git commit -m "Extract auth headers only from current-session captures" +``` + +--- + +### Task 2: Live browser headers replace captured ones, replicate mode only (Fisherman.refreshAuth) + +**Files:** +- Modify: `src/ai/fisherman.ts:156-170` (refreshAuth) +- Test: `tests/integration/fisherman.test.ts` + +**Interfaces:** +- Consumes: `extractAuthHeaders()` from Task 1; the Fisherman constructor's 6th argument `cookieProvider: () => Promise>` (renamed in Task 4, unchanged here); `ApiClient.setHeaders(headers)` which Object.assigns into defaults; `this.mode`, set by `ensureReady()` before `refreshAuth()` runs (`prepareData` calls them in that order). +- Produces: `refreshAuth` application order captured → browser → config, with the captured and browser layers applied only when `this.mode === 'replicate'`. Task 4's provider relies on its returned headers overriding same-named captured headers. + +- [ ] **Step 1: Write the failing integration test** + +In `tests/integration/fisherman.test.ts`: + +1. Add a module-scope variable next to `apiResponses` and reset it in `beforeEach`: + +```ts + let apiHeaders: Record; +``` + +```ts + apiHeaders = {}; +``` + +2. Change `createFisherman` to record headers, accept the browser-provided ones, and allow achieve-mode construction (existing call sites stay `createFisherman()`): + +```ts + function createFisherman(browserHeaders: Record = {}, configHeaders: Record = {}, hasApiConfig = false): Fisherman { + const apiClient = { + request: async () => apiResponses.shift(), + setHeaders: (h: Record) => Object.assign(apiHeaders, h), + getHeaders: () => ({ ...apiHeaders }), + }; + return new Fisherman( + provider, + apiClient as any, + requestStore, + async () => null, + 'https://example.test/api', + async () => browserHeaders, + configHeaders, + hasApiConfig + ); + } +``` + +3. Add the tests: + +```ts + it('sends the current browser session credentials, replacing captured ones', async () => { + const captured = requestResult('xhr_010_POST_api_alpha-shop_tests', 'POST', '/api/alpha-shop/tests', 201); + captured.requestHeaders = { 'x-csrf-token': 'captured-token', cookie: 'session=captured' }; + requestStore.addCapturedRequest(captured); + + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'stop', { reason: 'nothing to do' })] }); + mock.on({}, { content: 'done' }); + + await createFisherman({ Cookie: 'session=live', 'x-csrf-token': 'live-token' }).prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(apiHeaders.Cookie).toBe('session=live'); + expect(apiHeaders['x-csrf-token']).toBe('live-token'); + expect(apiHeaders.cookie).toBeUndefined(); + }); + + it('achieve mode authenticates only through config headers, never the browser session', async () => { + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'stop', { reason: 'nothing to do' })] }); + mock.on({}, { content: 'done' }); + + await createFisherman({ Cookie: 'session=live' }, { 'x-api-key': 'from-config' }, true).prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(apiHeaders['x-api-key']).toBe('from-config'); + expect(apiHeaders.Cookie).toBeUndefined(); + }); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `bun test tests/integration/fisherman.test.ts` +Expected: both new tests FAIL — in the first, the current order applies browser headers before captured ones, so the captured `x-csrf-token` (`captured-token`, a live-session capture that passes Task 1's gate) overwrites `live-token`; in the second, the current unconditional `refreshAuth` sends the browser Cookie in achieve mode. + +- [ ] **Step 3: Reorder refreshAuth and gate it to replicate mode** + +In `src/ai/fisherman.ts`, replace `refreshAuth`: + +```ts + private async refreshAuth(): Promise { + if (this.mode === 'replicate') { + const xhrHeaders = this.requestStore.extractAuthHeaders(); + if (Object.keys(xhrHeaders).length > 0) { + this.apiClient.setHeaders(xhrHeaders); + } + + const cookies = await this.cookieProvider(); + if (Object.keys(cookies).length > 0) { + this.apiClient.setHeaders(cookies); + } + } + + if (Object.keys(this.configHeaders).length > 0) { + this.apiClient.setHeaders(this.configHeaders); + } + } +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `bun test tests/integration/fisherman.test.ts && bun test tests/unit/fisherman-tools.test.ts` +Expected: all PASS, including the two pre-existing integration tests. + +- [ ] **Step 5: Format and commit** + +```bash +bun run format +git add src/ai/fisherman.ts tests/integration/fisherman.test.ts +git commit -m "Apply live browser session headers over captured ones" +``` + +--- + +### Task 3: Filter the cookie jar by the API origin + +**Files:** +- Modify: `src/explorbot.ts:349-353` (cookieProvider inside agentFisherman) + +**Interfaces:** +- Consumes: `baseEndpoint` local (`apiConfig?.baseEndpoint || this.config.playwright.url`), `Explorer.withPage`. +- Produces: unchanged provider signature; the returned `Cookie` header now contains only cookies Playwright would send to `baseEndpoint`. + +- [ ] **Step 1: Pass the target URL to the jar** + +This is DI glue over a Playwright API (`BrowserContext.cookies(urls)` filters by domain/path the way a real browser does) — there is no unit seam to test without mocking Playwright itself, so this task is verified by types and the full suite. Change line 350: + +```ts + const cookies = await this.explorer.withPage((page) => page.context().cookies(baseEndpoint)).catch(() => []); +``` + +- [ ] **Step 2: Run the suite** + +Run: `bun test tests/unit/ && bun test tests/integration/` +Expected: all PASS (no behavior change reachable from tests). + +- [ ] **Step 3: Format and commit** + +```bash +bun run format +git add src/explorbot.ts +git commit -m "Send only cookies scoped to the API origin" +``` + +--- + +### Task 4 (severable): Live CSRF token from the page, provider renamed + +If the user cuts this task, stop after Task 3 — nothing below is required by Tasks 1–3. + +**Files:** +- Modify: `src/explorbot.ts:349-356` (provider + Fisherman construction) +- Modify: `src/ai/fisherman.ts:24,33,39` (field/param rename) and the `refreshAuth` body from Task 2 +- Modify: `tests/integration/fisherman.test.ts` (argument name only) + +**Interfaces:** +- Consumes: Task 2's refreshAuth ordering; Task 3's URL-filtered jar. +- Produces: `browserHeaderProvider: () => Promise>` as the Fisherman constructor's 6th argument — same type, new name — returning `{ Cookie?, 'x-csrf-token'? }`. + +- [ ] **Step 1: Extend and rename the provider in explorbot.ts** + +Replace the `cookieProvider` block (which after Task 3 reads `cookies(baseEndpoint)`) with: + +```ts + const browserHeaderProvider = async (): Promise> => { + const session = await this.explorer + .withPage(async (page) => ({ + cookies: await page.context().cookies(baseEndpoint), + csrf: await page.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.getAttribute('content') || ''), + })) + .catch(() => ({ cookies: [] as any[], csrf: '' })); + + const headers: Record = {}; + if (session.cookies.length) headers.Cookie = session.cookies.map((c: any) => `${c.name}=${c.value}`).join('; '); + if (session.csrf) headers['x-csrf-token'] = session.csrf; + return headers; + }; +``` + +Update the construction call on the line that reads `new Fisherman(ai, apiClient, requestStore, specLoader, baseEndpoint, cookieProvider, configHeaders, hasApiConfig)` to pass `browserHeaderProvider`. + +`meta[name="csrf-token"]` is a cross-framework convention (Rails, Laravel) — structural knowledge like ARIA roles, not a site-specific locator (see spec D5). + +- [ ] **Step 2: Rename inside Fisherman** + +In `src/ai/fisherman.ts`, rename the private field `cookieProvider` to `browserHeaderProvider` (declaration line 24, constructor parameter and assignment lines 33/39) and update the browser block inside `refreshAuth`'s replicate-mode gate accordingly: + +```ts + const browserHeaders = await this.browserHeaderProvider(); + if (Object.keys(browserHeaders).length > 0) { + this.apiClient.setHeaders(browserHeaders); + } +``` + +- [ ] **Step 3: Rename the test argument** + +In `tests/integration/fisherman.test.ts`, `createFisherman(browserHeaders …)` already uses the right name from Task 2 — verify no remaining `cookieProvider` identifier exists in the repo: + +Run: `grep -rn cookieProvider src/ tests/` +Expected: no matches. + +- [ ] **Step 4: Run the suite** + +Run: `bun test tests/unit/ && bun test tests/integration/` +Expected: all PASS — the Task 2 test already proves a provider-supplied `x-csrf-token` reaches the client and overrides the captured one. + +- [ ] **Step 5: Format and commit** + +```bash +bun run format +git add src/explorbot.ts src/ai/fisherman.ts tests/integration/fisherman.test.ts +git commit -m "Read the live CSRF token from the page meta tag" +``` + +--- + +### Task 5: Housekeeping and PR update + +**Files:** +- Modify: `CHANGELOG.md` (extend the existing 2026-08-29 `[Fisherman]` entry area with a 2026-08-30 entry) + +**Interfaces:** +- Consumes: all previous tasks committed. +- Produces: branch pushed to PR #160 with an updated description. + +- [ ] **Step 1: Merge main and verify** + +```bash +cd /home/davert/projects/explorbot-fisherman-reliability +git fetch origin && git merge origin/main +bun test tests/unit/ && bun test tests/integration/ +``` + +Expected: clean merge (resolve conflicts if any, rerun tests), all tests PASS. + +- [ ] **Step 2: Update CHANGELOG** + +Add under a `## 2026-08-30` heading, following the existing entry style: + +```markdown +- [Fisherman] In replicate mode, API requests now authenticate with the current browser session: cookies are taken from the live jar filtered to the API origin, the CSRF token is read from the page, and auth headers are never reused from previous sessions' captured requests. Achieve mode authenticates solely through `api.headers` config. +``` + +- [ ] **Step 3: Format, commit, push** + +```bash +bun run format +git add CHANGELOG.md +git commit -m "Changelog for live-session auth" +git push origin fisherman-reliability +``` + +- [ ] **Step 4: Update the PR #160 description** + +Append a section to the PR body via `gh pr edit 160 --body-file` (fetch the current body with `gh pr view 160 --json body -q .body` first, never overwrite blindly): one paragraph stating that trace `de95bd1cffce09169599d99d1bee56cd` exposed stale-credential assembly (unfiltered cookie jar + auth headers scraped from months-old captures) and that Fisherman now authenticates with the live browser session per `docs/superpowers/specs/2026-08-30-fisherman-live-session-auth-design.md`. + +Do NOT touch the `regression` label or workflow. diff --git a/docs/superpowers/specs/2026-08-29-fisherman-reliability-design.md b/docs/superpowers/specs/2026-08-29-fisherman-reliability-design.md new file mode 100644 index 00000000..58002f3b --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-fisherman-reliability-design.md @@ -0,0 +1,45 @@ +# Fisherman Reliability — Design + +Fisherman is the API test-data preparation agent. Pilot calls it through the `precondition()` tool before a test runs; in **replicate mode** (no `api` config block) it learns endpoints from browser XHR traffic captured into `output/requests/`, in **achieve mode** it reads an OpenAPI spec. This design fixes replicate mode, which has never worked reliably. + +## Evidence + +Langfuse traces contain only two real Fisherman episodes (2026-04-30 and 2026-06-01); every August export has zero Fisherman spans, so the August fixes (#133) have no behavioral data behind them. + +- **Jun 01 01:28 — the one clean success.** 12 tool calls: a 400 on a missing field, retried correctly, parent suite created, test created inside it, `finish` with both real ids. Error-driven recovery worked. +- **Jun 01 01:30–01:32 — poisoned by the success.** The 01:28 run's own *rejected* bodies were handed back to the next runs as "the captured request example". They spent 20–49 requests guessing body shapes. One "succeeded" on the third invocation after ~155 model calls, with weak evidence: the reported created id was one an earlier run had *read* out of a GET list. +- **Apr 30 — false success.** Asked for a milestone; no milestone endpoint existed, so it POSTed to `/tests` and reported a created milestone. Pilot passed that to the Tester as a satisfied precondition. The prompt's endpoint list also contained a write endpoint from a different project. `stop` was never called in any episode; runs that hit max iterations reported nothing at all. + +## Root causes (verified in current code) + +1. **Self-poisoning store.** `addMadeRequest()` saves every request Fisherman itself makes — 400s included — into `output/requests/`. `loadFromDisk()` reads the whole directory back as `capturedRequests`, indistinguishable from browser XHR. Replicate-only: achieve mode never calls `loadFromDisk`. +2. **First-match spec lookup.** `findCapturedRequest()` is `find(method && path.startsWith(prefix))` — a stale 400 displaces a good 200, and `/suites` matches `/suites/123/move`. #133 only labels rejected captures `usable: false`; it does not rank. +3. **Scope filter never matches.** `getWriteRequestsForScope()` prefix-matches a page URL (`/projects/…`) against API paths (`/api/…`) — always falls through to `'/'`, silently, giving every captured write from every project. +4. **`finish` is unconditional success.** It writes the model's `created` array through verbatim; nothing checks it against what HTTP actually returned. +5. **Silent exhaustion & clobbered status.** No `finish` → `summary: ''` → Pilot logs nothing and the vision fallback is told the reason is "unknown". In the `request` tool, `...extractKeyFields()` spreads after the `status` key, so a body field named `status` overwrites the HTTP status, and the depth-5 first-id scan surfaces ids the run never created. + +## Design decisions + +1. **Provenance by id prefix, not a new envelope key.** File ids already encode the writer: `xhr_*` from `xhr-capture.ts`, unprefixed from `api-client.ts`, and `fail_*` records never reach disk. `loadFromDisk()` admits only `xhr_*` files. Poisoned directories migrate for free; a directory with only Fisherman-made files now correctly yields replicate mode disabled. +2. **Ranked lookup.** `findCapturedRequest` ranks candidates: exact segment-count match > deeper sub-path, then `status < 400` > rejection, then newest timestamp. An exact-path rejection deliberately beats a sub-path success — the `usable: false` branch explains it to the model. +3. **Shared path normalization.** Id-shaped segments are detected by the existing `isDynamicSegment()` (`src/utils/url-matcher.ts`) and printed as `{id}` in endpoint lists; lookup normalizes both sides, so patterns and concrete ids both match. Over-generalization (e.g. `/api/v2/…` → `/api/{id}/…`) is accepted: `getEndpointSpec` returns the concrete stored path, and the workflow mandates a spec lookup before first use. +4. **Scope = most selective shared segment.** Score each page-URL path segment by how many captured writes contain it; the scope key is the non-zero segment with the fewest matches, leftmost on ties. The project slug beats generic literals like `projects` structurally, with no site-specific knowledge. When nothing matches, the list degrades to global — and the system prompt says so. +5. **`finish` gated by the request ledger, not replaced.** `createFishermanTools` snapshots the made-request count; "this run" is everything after it. `finish` with zero successful writes is rejected back to the model (it can keep working or `stop`). Claimed ids are verified against actual 2xx write responses; verified items carry `via: "POST /api/…"` so Pilot sees what ran; unverifiable ids are dropped. When the loop ends without `finish`, the result is synthesized from the ledger — honest summary, ledger-derived created items — never an empty string. +6. **Deterministic loop guard.** Four consecutive failures (HTTP ≥ 400 or network error) against one endpoint end the run — the API-side analog of StateManager's dead-loop detection. +7. **One general prompt rule** against substituting resource types: if no endpoint creates a requested type, `stop` — never create a different type. (Ledger id-verification alone cannot catch the milestone→test case, since that POST genuinely succeeded.) + +All checks are deterministic-tier (structural matching, closed vocabularies); the model keeps judgment over what to create and how to describe it — the generate-then-verify ladder from CLAUDE.md. + +## Out of scope + +- Achieve mode (OpenAPI-driven) behavior. +- Cross-session reuse of captures beyond what the `xhr_` filter admits. +- New agents, tools, or envelope keys. + +## Acceptance criterion + +One regression trace where `precondition()` returns created ids and those ids are visible on the page the Tester then acts on. The regression run is triggered only by the user via the `regression` label. + +## Implementation + +`docs/superpowers/plans/2026-08-29-fisherman-reliability.md` diff --git a/docs/superpowers/specs/2026-08-30-fisherman-live-session-auth-design.md b/docs/superpowers/specs/2026-08-30-fisherman-live-session-auth-design.md new file mode 100644 index 00000000..f7c2eaf4 --- /dev/null +++ b/docs/superpowers/specs/2026-08-30-fisherman-live-session-auth-design.md @@ -0,0 +1,37 @@ +# Fisherman Live-Session Auth — Design + +## Problem + +Langfuse trace `de95bd1cffce09169599d99d1bee56cd` (2026-08-30, beta.testomat.io, project `zyntra-don-t-touch-cloned`): every Fisherman write returned `403 {"error":"Unauthorized"}` in ~8ms while the same browser session performed successful writes to the same project minutes earlier. The requests were rejected at the auth layer because Fisherman assembled stale credentials: + +1. `cookieProvider` (`src/explorbot.ts`) serialized `page.context().cookies()` — the **entire jar, unfiltered** — producing a doubled `Cookie` header with a dead `localhost` session pair ahead of the valid one. A real browser never sends localhost cookies to another host. +2. `extractAuthHeaders` (`src/api/request-store.ts`) scraped `x-csrf-token` from a **2026-07-07 capture of another project** (`imr_manual12`). The store iterates `capturedRequests` from the end, but `loadFromDisk` fills it in alphabetical filename order, so "last" means alphabetically-late, not newest. `output/requests/` is a graveyard spanning months and many projects, so any stale credential can win. +3. `refreshAuth` (`src/ai/fisherman.ts`) applies live cookies **before** capture-scraped headers, so a captured `cookie` header (in `AUTH_HEADERS`) could even clobber the fresh jar. + +## Principle + +**Captures are a source of API shape — endpoints, body examples — which is durable across sessions. They are never a source of credentials, which are ephemeral.** Credentials come from the live browser session or from explicit `api.headers` config. This rules out any future re-accretion of header scraping from old captures. + +## Decisions + +- **D1 — Cookies from the live jar, filtered by the API origin.** `page.context().cookies(baseEndpoint)`: Playwright applies the same domain/path matching a browser applies when sending to that URL. No manual dedup — same-name cookies on parent/child domains are legitimate browser behavior and Playwright's filter already yields exactly what the browser would send. +- **D2 — Never scrape cookies from captures.** `'cookie'` leaves `AUTH_HEADERS`. The jar is the single, always-current source of cookies. +- **D3 — Auth headers only from current-session captures, newest first.** `RequestStore` records `sessionStartedAt` at construction; `extractAuthHeaders` considers only captures with `timestamp >= sessionStartedAt`, sorted newest-first. Live captures (added by `XhrCapture` during this run) pass; the disk graveyard never does. This also resolves same-id collisions (an old `xhr_001_…` disk file vs a live capture reusing that counter id): the gate keeps only the live one. +- **D4 — Precedence: captured < live browser < config.** `refreshAuth` applies session-capture headers first, live browser headers second (current session replaces old), explicit `configHeaders` last (user intent stays authoritative). Still refreshed once per episode — the browser is idle while Fisherman runs. +- **D4a — Replicate mode only.** Browser-derived credentials (jar cookies, page CSRF token, session-capture headers) apply only in replicate mode, where Fisherman replays what the browser does. In achieve mode the API contract is explicit and authentication comes solely from `api.headers` config — injecting browser cookies there would be surprising and can leak a UI session into a separately-authenticated API. +- **D5 — Live CSRF token from the page (severable).** The provider also reads `meta[name="csrf-token"]` from the current page and sends it as `x-csrf-token`. This is a cross-framework web convention (Rails, Laravel), the same class of structural knowledge as ARIA attributes or URL anatomy — not a site-specific locator. `cookieProvider` is renamed `browserHeaderProvider` since it now supplies all live-browser-derived headers. Cutting this decision cuts only Task 4 of the plan; Tasks 1–3 stand alone. +- **D6 — A capture without a timestamp is stale.** `RequestResult.load` currently stamps load-time for a file missing `timestamp`, which would slip past the session gate; absent timestamp now parses as epoch. + +## Declared behavior change + +`Authorization` / `x-api-key` values scraped from **previous-session** captures are no longer sent. Cookie-authenticated apps are unaffected (the jar is live). An app that was only ever authenticated through a stale captured token now fails honestly instead of sending dead credentials — the remedy is `api.headers` in config. + +## Out of scope + +- Prompt changes to make the model stop faster on `authorization` failures (the 4-failure guard already bounds it). +- Pilot's final verdict misattributing the failure to later tester errors instead of the failed precondition. +- Mirroring the `XSRF-TOKEN` cookie into an `X-XSRF-TOKEN` header (Angular/Laravel convention) — add only if a real trace shows it's needed. + +## Acceptance + +A run against a cookie-authenticated app where Fisherman's writes carry only the cookies the current browser session would send to the API origin, plus a current CSRF token — verified by the request ledger in `output/requests/` showing a single-valued `Cookie` header matching the live session and no header value originating from a previous session's captures. diff --git a/src/ai/fisherman-tools.ts b/src/ai/fisherman-tools.ts index 9e0c8abc..bcecc977 100644 --- a/src/ai/fisherman-tools.ts +++ b/src/ai/fisherman-tools.ts @@ -2,16 +2,28 @@ import { tool } from 'ai'; import dedent from 'dedent'; import { z } from 'zod'; import type { ApiClient } from '../api/api-client.ts'; +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'; export function createFishermanTools(apiClient: ApiClient, requestStore: RequestStore, opts: { spec?: any; baseEndpoint?: string }) { let finished = false; - let result: FishermanResult = { success: false, summary: '', created: [], failed: [] }; + let result: FishermanResult | null = null; + const ledgerStart = requestStore.getMadeRequests().length; - const getResult = () => result; + 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()); const isFinished = () => finished; + const finishFromText = (text?: string) => { + finished = true; + const synthesized = synthesizeResult(runRequests(), successfulWrites()); + if (text && synthesized.success) synthesized.summary = text; + result = synthesized; + }; const tools = { getEndpointSpec: tool({ @@ -76,7 +88,7 @@ export function createFishermanTools(apiClient: ApiClient, requestStore: Request request: tool({ description: dedent` Make an HTTP request to the API. - Returns status, timing, and auto-extracted IDs and names from the response. + Returns status, plus IDs and names auto-extracted from the response under 'extracted'. `, inputSchema: z.object({ method: z.enum(['GET', 'POST', 'PUT', 'PATCH', 'DELETE']).describe('HTTP method'), @@ -119,7 +131,7 @@ export function createFishermanTools(apiClient: ApiClient, requestStore: Request return { success: true, status: reqResult.status, - ...extracted, + extracted, }; }, }), @@ -148,9 +160,32 @@ 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)); + tag('success').log(`Fisherman done: ${summary}`); finished = true; - result = { success: true, summary, created, failed: failed || [] }; + result = { success: true, summary, created: verified, failed: failed || [] }; return { finished: true }; }, }), @@ -169,7 +204,21 @@ export function createFishermanTools(apiClient: ApiClient, requestStore: Request }), }; - return { tools, getResult, isFinished }; + return { tools, getResult, isFinished, finishFromText }; +} + +function synthesizeResult(made: RequestResult[], writes: RequestResult[]): FishermanResult { + const failures = made.filter((r) => r.status >= 400 || r.error); + 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()}`; + return { success: writes.length > 0, summary, created: writes.map(toCreatedItem), failed: [] }; +} + +function toCreatedItem(write: RequestResult): FishermanResult['created'][number] { + const { id, title } = write.extractIdAndTitle(); + const segments = write.path.split('/').filter((s) => s && !isDynamicSegment(s)); + return { type: segments[segments.length - 1] || 'item', id, title, request: write.toEndpoint() }; } function responseCategory(status: number): ResponseCategory { @@ -209,7 +258,7 @@ function extractKeyFields(body: any, result: Record = {}, depth = 0 export interface FishermanResult { success: boolean; summary: string; - created: Array<{ type: string; id?: string | number; title?: string }>; + created: Array<{ type: string; id?: string | number; title?: string; request?: string }>; failed: Array<{ type: string; reason: string }>; } diff --git a/src/ai/fisherman.ts b/src/ai/fisherman.ts index 28670488..c6dd6e4c 100644 --- a/src/ai/fisherman.ts +++ b/src/ai/fisherman.ts @@ -13,6 +13,7 @@ import { dataProtectionRules } from './rules.ts'; const MAX_ITERATIONS = 15; const MAX_TOOL_ROUNDTRIPS = 5; +const REPEATED_FAILURE_LIMIT = 4; export class Fisherman implements Agent { emoji = '🎣'; @@ -20,21 +21,22 @@ export class Fisherman implements Agent { private apiClient: ApiClient; private requestStore: RequestStore; private specLoader: () => Promise; - private cookieProvider: () => Promise>; + private browserHeaderProvider: () => Promise>; private configHeaders: Record; private sessionName?: string; private baseEndpoint: string; private spec: any | null = null; private mode: 'replicate' | 'achieve' | 'disabled' = 'disabled'; private hasApiConfig: boolean; + private scopeDegraded = false; - constructor(provider: Provider, apiClient: ApiClient, requestStore: RequestStore, specLoader: () => Promise, baseEndpoint: string, cookieProvider: () => Promise>, configHeaders: Record = {}, hasApiConfig = false) { + constructor(provider: Provider, apiClient: ApiClient, requestStore: RequestStore, specLoader: () => Promise, baseEndpoint: string, browserHeaderProvider: () => Promise>, configHeaders: Record = {}, hasApiConfig = false) { this.provider = provider; this.apiClient = apiClient; this.requestStore = requestStore; this.specLoader = specLoader; this.baseEndpoint = baseEndpoint; - this.cookieProvider = cookieProvider; + this.browserHeaderProvider = browserHeaderProvider; this.configHeaders = configHeaders; this.hasApiConfig = hasApiConfig; this.mode = hasApiConfig ? 'achieve' : 'replicate'; @@ -77,10 +79,11 @@ export class Fisherman implements Agent { await this.refreshAuth(); debugLog(`auth headers: ${Object.keys(this.apiClient.getHeaders()).join(', ')}`); - const { tools, getResult, isFinished } = createFishermanTools(this.apiClient, this.requestStore, { + const { tools, getResult, isFinished, finishFromText } = createFishermanTools(this.apiClient, this.requestStore, { 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)); @@ -90,7 +93,6 @@ export class Fisherman implements Agent { debugLog(`iteration ${iteration}`); const invokeResult = await this.provider.invokeConversation(conversation, tools, { maxToolRoundtrips: MAX_TOOL_ROUNDTRIPS, - toolChoice: 'required', agentName: 'fisherman', }); debugLog(`iteration ${iteration} done, text: ${invokeResult?.response?.text?.slice(0, 200) || '(none)'}`); @@ -100,6 +102,19 @@ export class Fisherman implements Agent { return; } + if (!invokeResult?.toolExecutions?.length) { + debugLog('no tool call in this turn — treating as finish'); + finishFromText(invokeResult?.response?.text); + stop(); + return; + } + + if (this.isStuckOnEndpoint(ledgerStart)) { + tag('warning').log('Fisherman: repeated failures on the same endpoint — stopping'); + stop(); + return; + } + if (iteration >= MAX_ITERATIONS) { tag('warning').log('Fisherman: max iterations reached'); stop(); @@ -145,14 +160,16 @@ export class Fisherman implements Agent { } private async refreshAuth(): Promise { - const cookies = await this.cookieProvider(); - if (Object.keys(cookies).length > 0) { - this.apiClient.setHeaders(cookies); - } + if (this.mode === 'replicate') { + const xhrHeaders = this.requestStore.extractAuthHeaders(); + if (Object.keys(xhrHeaders).length > 0) { + this.apiClient.setHeaders(xhrHeaders); + } - const xhrHeaders = this.requestStore.extractAuthHeaders(); - if (Object.keys(xhrHeaders).length > 0) { - this.apiClient.setHeaders(xhrHeaders); + const browserHeaders = await this.browserHeaderProvider(); + if (Object.keys(browserHeaders).length > 0) { + this.apiClient.setHeaders(browserHeaders); + } } if (Object.keys(this.configHeaders).length > 0) { @@ -161,31 +178,25 @@ export class Fisherman implements Agent { } private buildEndpointList(scopeUrl?: string): string { + this.scopeDegraded = false; if (this.mode === 'achieve' && this.spec) { const specEndpoints = listAllEndpoints(this.spec, this.baseEndpoint); if (specEndpoints) return specEndpoints; } - let writeRequests = this.requestStore.getWriteRequestsForScope(scopeUrl || '/'); - if (writeRequests.length === 0) { - writeRequests = this.requestStore.getWriteRequestsForScope('/'); - } + const scoped = this.requestStore.toEndpointList(scopeUrl || '/'); + if (scoped) return scoped; - const seen = new Set(); - const lines: string[] = []; - - for (const req of writeRequests) { - const key = `${req.method} ${req.path}`; - if (seen.has(key)) continue; - seen.add(key); - lines.push(key); - } - - return lines.join('\n'); + this.scopeDegraded = true; + return this.requestStore.toEndpointList(); } private buildSystemPrompt(endpointList: string, toolNames: string[], scopeUrl?: string): string { - const scopeBlock = scopeUrl ? `\n\nSCOPE: You are operating within ${scopeUrl}.\nAll created items must belong to this scope.` : ''; + let scopeBlock = ''; + if (scopeUrl) { + scopeBlock = `\n\nSCOPE: You are operating within ${scopeUrl}.\nAll created items must belong to this scope.`; + if (this.scopeDegraded) scopeBlock += '\nThe endpoint list could not be narrowed to this scope and may include endpoints belonging to other scopes. Before writing, confirm the target belongs to this scope.'; + } return dedent` You are Fisherman — a data preparation agent. You create test data by making API requests. @@ -210,12 +221,21 @@ export class Fisherman implements Agent { - Chain requests logically — create parent resources before children - Use the response category and error text to decide what failed: validation requires corrected data, authorization requires valid access, not_found requires a valid path or parent, and conflict requires resolving the conflicting state - Retry temporary or server failures once. Retry other failures only when the specification or error text gives a concrete correction + - Create only the resource types that were requested. If no endpoint creates a requested type, call stop — never create a different type as a substitute - Use realistic but unique data for each item (vary names, titles) ${dataProtectionRules} `; } + private isStuckOnEndpoint(ledgerStart: number): boolean { + const made = this.requestStore.getMadeRequests().slice(ledgerStart); + 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); + } + private buildTaskPrompt(instructions: string): string { return dedent` Prepare the following test data: diff --git a/src/ai/pilot.ts b/src/ai/pilot.ts index 7256ad98..ebae181c 100644 --- a/src/ai/pilot.ts +++ b/src/ai/pilot.ts @@ -765,6 +765,7 @@ export class Pilot implements Agent { const parts = [c.type]; if (c.title) parts.push(`"${c.title}"`); if (c.id) parts.push(`(id: ${c.id})`); + if (c.request) parts.push(`via ${c.request}`); return parts.join(' '); }); const stepText = `Precondition: created ${items.join(', ')}`; diff --git a/src/api/request-result.ts b/src/api/request-result.ts index 5fc6fbb7..2e069817 100644 --- a/src/api/request-result.ts +++ b/src/api/request-result.ts @@ -85,6 +85,10 @@ export class RequestResult { } } + get isWrite(): boolean { + return ['POST', 'PUT', 'PATCH', 'DELETE'].includes(this.method); + } + save(outputDir: string): void { const requestsDir = path.join(outputDir, 'requests'); if (!existsSync(requestsDir)) { @@ -169,7 +173,7 @@ export class RequestResult { statusText: meta.statusText || '', responseHeaders: meta.responseHeaders || {}, timing: Number.parseInt(meta.timing) || 0, - timestamp: new Date(meta.timestamp || Date.now()), + timestamp: new Date(meta.timestamp || 0), }); result.requestFile = requestFile; @@ -178,8 +182,12 @@ export class RequestResult { return result; } + toEndpoint(): string { + return `${this.method} ${this.path}`; + } + toSummary(): string { - return `${this.method} ${this.path} → ${this.status} (${this.timing}ms)`; + return `${this.toEndpoint()} → ${this.status} (${this.timing}ms)`; } extractIdAndTitle(): { id?: string | number; title?: string } { diff --git a/src/api/request-store.ts b/src/api/request-store.ts index 7cfabebf..459a86ef 100644 --- a/src/api/request-store.ts +++ b/src/api/request-store.ts @@ -1,8 +1,9 @@ import { existsSync, readdirSync } from 'node:fs'; import path from 'node:path'; +import { isDynamicSegment } from '../utils/url-matcher.ts'; import { RequestResult } from './request-result.ts'; -const AUTH_HEADERS = ['authorization', 'cookie', 'x-api-key', 'x-csrf-token']; +const AUTH_HEADERS = ['authorization', 'x-api-key', 'x-csrf-token']; export class RequestStore { private capturedRequests: RequestResult[] = []; @@ -10,6 +11,7 @@ export class RequestStore { private failedRequests: RequestResult[] = []; private onFailedListeners: Array<(r: RequestResult) => void> = []; private outputDir: string; + private sessionStartedAt = new Date(); constructor(outputDir: string) { this.outputDir = outputDir; @@ -77,13 +79,15 @@ export class RequestStore { return this.madeRequests.filter((r) => r.status === status); } - toEndpointList(): string { + toEndpointList(scopePath?: string): string { + let requests = this.capturedRequests; + if (scopePath) requests = this.getWriteRequestsForScope(scopePath); + const seen = new Set(); const lines: string[] = []; - for (const req of this.capturedRequests) { - const normalized = normalizePathPattern(req.path); - const key = `${req.method} ${normalized}`; + for (const req of requests) { + const key = `${req.method} ${normalizePathPattern(req.path)}`; if (seen.has(key)) continue; seen.add(key); lines.push(key); @@ -94,23 +98,42 @@ export class RequestStore { extractAuthHeaders(): Record { const headers: Record = {}; + const sessionCaptures = this.capturedRequests.filter((r) => r.timestamp >= this.sessionStartedAt).sort((a, b) => b.timestamp.getTime() - a.timestamp.getTime()); - for (let i = this.capturedRequests.length - 1; i >= 0; i--) { - const req = this.capturedRequests[i]; + for (const req of sessionCaptures) { for (const [key, value] of Object.entries(req.requestHeaders)) { if (AUTH_HEADERS.includes(key.toLowerCase()) && !headers[key]) { headers[key] = value; } } - if (AUTH_HEADERS.every((h) => Object.keys(headers).some((k) => k.toLowerCase() === h))) break; } return headers; } - findCapturedRequest(method: string, pathPrefix: string): RequestResult | undefined { + findCapturedRequest(method: string, searchPath: string): RequestResult | undefined { const upper = method.toUpperCase(); - return this.capturedRequests.find((r) => r.method === upper && r.path.startsWith(pathPrefix)); + const search = normalizePathPattern(searchPath).split('/').filter(Boolean); + + let best: RequestResult | undefined; + let bestScore = -1; + + for (const req of this.capturedRequests) { + if (req.method !== upper) continue; + const segments = normalizePathPattern(req.path).split('/').filter(Boolean); + if (segments.length < search.length) continue; + if (!search.every((segment, i) => segment === segments[i])) continue; + + let score = 0; + if (segments.length === search.length) score += 4; + if (req.status < 400) score += 2; + if (score < bestScore) continue; + if (score === bestScore && best && req.timestamp <= best.timestamp) continue; + best = req; + bestScore = score; + } + + return best; } toLog(): string { @@ -122,7 +145,7 @@ export class RequestStore { if (!existsSync(requestsDir)) return; const existingIds = new Set(this.capturedRequests.map((r) => r.id)); - const files = readdirSync(requestsDir).filter((f) => f.endsWith('.request.yaml')); + const files = readdirSync(requestsDir).filter((f) => f.startsWith('xhr_') && f.endsWith('.request.yaml')); for (const file of files) { try { @@ -137,7 +160,22 @@ export class RequestStore { getWriteRequestsForScope(scopePath: string): RequestResult[] { const writeMethods = new Set(['POST', 'PUT', 'PATCH', 'DELETE']); - return this.capturedRequests.filter((r) => writeMethods.has(r.method) && r.path.startsWith(scopePath)); + const writes = this.capturedRequests.filter((r) => writeMethods.has(r.method)); + const scopeSegments = scopePath.split('/').filter(Boolean); + if (scopeSegments.length === 0) return writes; + + let scopeKey = ''; + let fewest = Number.POSITIVE_INFINITY; + for (const segment of scopeSegments) { + if (isDynamicSegment(segment)) continue; + const matches = writes.filter((r) => r.path.split('/').includes(segment)).length; + if (matches === 0 || matches >= fewest) continue; + scopeKey = segment; + fewest = matches; + } + if (!scopeKey) return []; + + return writes.filter((r) => r.path.split('/').includes(scopeKey)); } clear(): void { @@ -148,5 +186,8 @@ export class RequestStore { } function normalizePathPattern(urlPath: string): string { - return urlPath.replace(/\/[0-9a-f]{24}\b/g, '/{id}').replace(/\/\d+\b/g, '/{id}'); + return urlPath + .split('/') + .map((segment) => (segment && isDynamicSegment(segment) ? '{id}' : segment)) + .join('/'); } diff --git a/src/explorbot.ts b/src/explorbot.ts index 5421ce7f..c921ebc0 100644 --- a/src/explorbot.ts +++ b/src/explorbot.ts @@ -346,14 +346,22 @@ export class ExplorBot { } }; - const cookieProvider = async (): Promise> => { - const cookies = await this.explorer.withPage((page) => page.context().cookies()).catch(() => []); - if (!cookies.length) return {}; - return { Cookie: cookies.map((c: any) => `${c.name}=${c.value}`).join('; ') }; + const browserHeaderProvider = async (): Promise> => { + const session = await this.explorer + .withPage(async (page) => ({ + cookies: await page.context().cookies(baseEndpoint), + csrf: await page.evaluate(() => document.querySelector('meta[name="csrf-token"]')?.getAttribute('content') || '').catch(() => ''), + })) + .catch(() => ({ cookies: [] as any[], csrf: '' })); + + const headers: Record = {}; + if (session.cookies.length) headers.Cookie = session.cookies.map((c: any) => `${c.name}=${c.value}`).join('; '); + if (session.csrf) headers['x-csrf-token'] = session.csrf; + return headers; }; this.agents.fisherman = this.createAgent(({ ai }) => { - return new Fisherman(ai, apiClient, requestStore, specLoader, baseEndpoint, cookieProvider, configHeaders, hasApiConfig); + return new Fisherman(ai, apiClient, requestStore, specLoader, baseEndpoint, browserHeaderProvider, configHeaders, hasApiConfig); }); } return this.agents.fisherman; diff --git a/src/utils/request-map.ts b/src/utils/request-map.ts new file mode 100644 index 00000000..0c320392 --- /dev/null +++ b/src/utils/request-map.ts @@ -0,0 +1,19 @@ +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/src/utils/url-matcher.ts b/src/utils/url-matcher.ts index 13edd2b1..3cac31b2 100644 --- a/src/utils/url-matcher.ts +++ b/src/utils/url-matcher.ts @@ -9,6 +9,7 @@ export function isDynamicSegment(segment: string): boolean { /* config not loaded yet */ } + if (/^v\d+$/i.test(segment)) return false; // numeric: /users/123 if (/^\d+$/.test(segment)) return true; // UUID: /items/550e8400-e29b-41d4-a716-446655440000 diff --git a/tests/integration/fisherman.test.ts b/tests/integration/fisherman.test.ts new file mode 100644 index 00000000..7cf3ffab --- /dev/null +++ b/tests/integration/fisherman.test.ts @@ -0,0 +1,176 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'bun:test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { createOpenAI } from '@ai-sdk/openai'; +import { LLMock } from '@copilotkit/aimock'; +import { Fisherman } from '../../src/ai/fisherman.ts'; +import { Provider } from '../../src/ai/provider.ts'; +import { RequestResult } from '../../src/api/request-result.ts'; +import { RequestStore } from '../../src/api/request-store.ts'; +import { ConfigParser } from '../../src/config.ts'; + +function requestResult(id: string, method: string, urlPath: string, status: number, body?: any): RequestResult { + return new RequestResult({ + id, + method, + path: urlPath, + fullUrl: urlPath, + requestHeaders: {}, + requestBody: body, + status, + statusText: String(status), + responseHeaders: {}, + timing: 0, + timestamp: new Date(), + }); +} + +function toolCall(id: string, name: string, args: Record) { + return { id, name, arguments: JSON.stringify(args) }; +} + +function extractPromptText(entry: any): string { + if (!entry?.body?.messages) return ''; + return entry.body.messages + .map((message: any) => { + if (typeof message.content === 'string') return message.content; + if (Array.isArray(message.content)) { + return message.content + .filter((part: any) => part.type === 'text') + .map((part: any) => part.text || '') + .join('\n'); + } + return ''; + }) + .join('\n'); +} + +describe('Fisherman with aimock', () => { + let mock: LLMock; + let provider: Provider; + let outputDir: string; + let requestStore: RequestStore; + let apiResponses: RequestResult[]; + let apiHeaders: Record; + + beforeAll(async () => { + mock = new LLMock({ port: 0, logLevel: 'silent' }); + await mock.start(); + + const openai = createOpenAI({ baseURL: `${mock.url}/v1`, apiKey: 'test-key', compatibility: 'compatible' }); + ConfigParser.resetForTesting(); + ConfigParser.setupTestConfig(); + provider = new Provider({ model: openai.chat('test-model'), config: {} }); + }); + + beforeEach(() => { + mock.clearRequests(); + mock.resetMatchCounts(); + mock.clearFixtures(); + + outputDir = mkdtempSync(path.join(tmpdir(), 'fisherman-')); + requestStore = new RequestStore(outputDir); + requestStore.addCapturedRequest(requestResult('xhr_001_POST_api_alpha-shop_suites', 'POST', '/api/alpha-shop/suites', 201, { title: 'Suite' })); + requestStore.addCapturedRequest(requestResult('xhr_002_POST_api_other-shop_suites', 'POST', '/api/other-shop/suites', 201, { title: 'Suite' })); + apiResponses = []; + apiHeaders = {}; + }); + + afterEach(() => { + rmSync(outputDir, { recursive: true, force: true }); + }); + + afterAll(async () => { + await mock.stop(); + ConfigParser.cleanupAllTestDirectories(); + }); + + function createFisherman(browserHeaders: Record = {}, configHeaders: Record = {}, hasApiConfig = false): Fisherman { + const apiClient = { + request: async () => apiResponses.shift(), + setHeaders: (h: Record) => Object.assign(apiHeaders, h), + getHeaders: () => ({ ...apiHeaders }), + }; + return new Fisherman( + provider, + apiClient as any, + requestStore, + async () => null, + 'https://example.test/api', + async () => browserHeaders, + configHeaders, + hasApiConfig + ); + } + + it('scopes the prompt and reports verified created items with their request', async () => { + const created = requestResult('made_1', 'POST', '/api/alpha-shop/suites', 201); + created.rawResponseBodyValue = JSON.stringify({ data: { id: 's1', title: 'Suite A' } }); + apiResponses.push(created); + + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'request', { method: 'POST', path: '/api/alpha-shop/suites', body: { title: 'Suite A' } })] }); + mock.on({ sequenceIndex: 1 }, { toolCalls: [toolCall('c2', 'finish', { summary: '1 suite created', created: [{ type: 'suite', id: 's1', title: 'Suite A' }] })] }); + mock.on({}, { content: 'done' }); + + const result = await createFisherman().prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(result.success).toBe(true); + expect(result.created).toEqual([{ type: 'suite', id: 's1', title: 'Suite A', request: 'POST /api/alpha-shop/suites' }]); + + const systemPrompt = extractPromptText(mock.getRequests()[0]); + expect(systemPrompt).toContain('POST /api/alpha-shop/suites'); + expect(systemPrompt).not.toContain('other-shop'); + }); + + it('rejects an empty-handed finish and returns an honest failure', async () => { + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'finish', { summary: 'all done', created: [{ type: 'suite', id: '99' }] })] }); + mock.on({}, { toolCalls: [toolCall('c2', 'stop', { reason: 'The data cannot be created' })] }); + + const result = await createFisherman().prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(result.success).toBe(false); + expect(result.created).toHaveLength(0); + expect(result.summary).toBe('The data cannot be created'); + }); + + it('sends the current browser session credentials, replacing captured ones', async () => { + const captured = requestResult('xhr_010_POST_api_alpha-shop_tests', 'POST', '/api/alpha-shop/tests', 201); + captured.requestHeaders = { 'x-csrf-token': 'captured-token', cookie: 'session=captured' }; + requestStore.addCapturedRequest(captured); + + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'stop', { reason: 'nothing to do' })] }); + mock.on({}, { content: 'done' }); + + await createFisherman({ Cookie: 'session=live', 'x-csrf-token': 'live-token' }).prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(apiHeaders.Cookie).toBe('session=live'); + expect(apiHeaders['x-csrf-token']).toBe('live-token'); + expect(apiHeaders.cookie).toBeUndefined(); + }); + + it('achieve mode authenticates only through config headers, never the browser session', async () => { + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'stop', { reason: 'nothing to do' })] }); + mock.on({}, { content: 'done' }); + + await createFisherman({ Cookie: 'session=live' }, { 'x-api-key': 'from-config' }, true).prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(apiHeaders['x-api-key']).toBe('from-config'); + expect(apiHeaders.Cookie).toBeUndefined(); + }); + + it('treats a no-tool-call response as finish with the text as summary', async () => { + const created = requestResult('made_2', 'POST', '/api/alpha-shop/suites', 201); + created.rawResponseBodyValue = JSON.stringify({ data: { id: 's2', title: 'Suite B' } }); + apiResponses.push(created); + + mock.on({ sequenceIndex: 0 }, { toolCalls: [toolCall('c1', 'request', { method: 'POST', path: '/api/alpha-shop/suites', body: { title: 'Suite B' } })] }); + mock.on({}, { content: 'Created one suite' }); + + const result = await createFisherman().prepareData('1 suite', '/projects/alpha-shop/suites'); + + expect(result.success).toBe(true); + expect(result.summary).toBe('Created one suite'); + expect(result.created).toEqual([{ type: 'suites', id: 's2', title: 'Suite B', request: 'POST /api/alpha-shop/suites' }]); + }); +}); diff --git a/tests/unit/fisherman-tools.test.ts b/tests/unit/fisherman-tools.test.ts index c23f7e4b..38437bbb 100644 --- a/tests/unit/fisherman-tools.test.ts +++ b/tests/unit/fisherman-tools.test.ts @@ -55,11 +55,129 @@ describe('Fisherman tools', () => { expect(result.category).toBe(category); } }); + + it('keeps the HTTP status authoritative over response body fields', async () => { + const apiClient = { + request: async () => ({ status: 201, statusText: 'Created', rawResponseBody: '', responseBody: { id: 7, status: 'draft' } }), + }; + const { tools } = createFishermanTools(apiClient as any, store(), {}); + + const result: any = await tools.request.execute({ method: 'POST', path: '/items' }, {} as any); + + expect(result.status).toBe(201); + expect(result.extracted).toEqual({ id: 7, status: 'draft' }); + }); }); -function store(captured?: any): any { +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 result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: '1' }] }, {} as any); + + expect(result.finished).toBe(false); + expect(result.error).toContain('No successful write'); + expect(isFinished()).toBe(false); + }); + + 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 result: any = await tools.finish.execute({ summary: 'done', created: [{ type: 'suite', id: 's1' }] }, {} as any); + + expect(result.finished).toBe(false); + expect(isFinished()).toBe(false); + }); + + 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), {}); + made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); + + await tools.finish.execute( + { + summary: 'done', + created: [ + { type: 'suite', id: 's1' }, + { type: 'milestone', id: 'm9' }, + ], + }, + {} as any + ); + + const result = getResult(); + expect(result.success).toBe(true); + expect(result.created).toEqual([{ type: 'suite', id: 's1', request: 'POST /api/suites' }]); + }); + + it('synthesizes an honest summary when the loop ends without finish', async () => { + const made: any[] = []; + const { getResult } = createFishermanTools({} as any, store(undefined, made), {}); + made.push(madeWrite('POST', '/api/suites', 201, { id: 's1', title: 'Suite A' })); + made.push(madeWrite('POST', '/api/tests', 400)); + + const result = getResult(); + expect(result.success).toBe(true); + expect(result.summary).toContain('1 successful write'); + expect(result.summary).toContain('POST /api/tests → 400'); + expect(result.created[0].id).toBe('s1'); + }); + + 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), {}); + made.push(madeWrite('POST', '/api/tests', 400)); + + const result = getResult(); + expect(result.success).toBe(false); + expect(result.summary).not.toBe(''); + }); + + 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), {}); + made.push(madeWrite('POST', '/api/suites', 201, { id: 's1' })); + + finishFromText('Created the suite'); + + expect(isFinished()).toBe(true); + const result = getResult(); + expect(result.success).toBe(true); + expect(result.summary).toBe('Created the suite'); + expect(result.created[0].id).toBe('s1'); + }); + + 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), {}); + made.push(madeWrite('POST', '/api/tests', 400)); + + finishFromText('All done successfully'); + + const result = getResult(); + expect(result.success).toBe(false); + expect(result.summary).not.toBe('All done successfully'); + }); +}); + +function store(captured?: any, made: any[] = []): any { return { findCapturedRequest: () => captured, - addMadeRequest: () => {}, + addMadeRequest: (r: any) => made.push(r), + getMadeRequests: () => made, + }; +} + +function madeWrite(method: string, path: string, status: number, body: Record = {}): any { + return { + method, + path, + status, + error: undefined, + isWrite: true, + extractIdAndTitle: () => body, + toEndpoint: () => `${method} ${path}`, + toSummary: () => `${method} ${path} → ${status} (0ms)`, }; } diff --git a/tests/unit/request-store.test.ts b/tests/unit/request-store.test.ts index 92e67cf4..bea70166 100644 --- a/tests/unit/request-store.test.ts +++ b/tests/unit/request-store.test.ts @@ -1,19 +1,19 @@ import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; -import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { RequestResult } from '../../src/api/request-result.js'; import { RequestStore } from '../../src/api/request-store.js'; let counter = 0; -function makeRequest(method: string, path: string, status: number): RequestResult { +function makeRequest(method: string, path: string, status: number, id?: string, headers: Record = {}): RequestResult { counter++; return new RequestResult({ - id: `req_${counter}`, + id: id || `req_${counter}`, method, path, fullUrl: path, - requestHeaders: {}, + requestHeaders: headers, status, statusText: String(status), responseHeaders: {}, @@ -94,3 +94,210 @@ describe('RequestStore failures', () => { expect(received).toEqual([503]); }); }); + +describe('findCapturedRequest ranking', () => { + let outputDir: string; + let store: RequestStore; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + store = new RequestStore(outputDir); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('prefers a successful capture over a rejected one for the same endpoint', () => { + store.addCapturedRequest(makeRequest('POST', '/api/suites', 400)); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201)); + + expect(store.findCapturedRequest('POST', '/api/suites')?.status).toBe(201); + }); + + it('prefers the exact endpoint over a deeper sub-path', () => { + store.addCapturedRequest(makeRequest('POST', '/api/suites/42/move', 200)); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 400)); + + expect(store.findCapturedRequest('POST', '/api/suites')?.path).toBe('/api/suites'); + }); + + it('matches {id} patterns and concrete ids against stored ids', () => { + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/1a2b3c4d', 200)); + + expect(store.findCapturedRequest('PATCH', '/api/suites/{id}')?.status).toBe(200); + expect(store.findCapturedRequest('PATCH', '/api/suites/9f8e7d6c')?.status).toBe(200); + }); + + it('prefers the newest among otherwise equal candidates', () => { + const older = makeRequest('POST', '/api/suites', 201); + older.timestamp = new Date('2026-01-01'); + const newer = makeRequest('POST', '/api/suites', 201); + newer.timestamp = new Date('2026-02-01'); + store.addCapturedRequest(older); + store.addCapturedRequest(newer); + + expect(store.findCapturedRequest('POST', '/api/suites')?.id).toBe(newer.id); + }); +}); + +describe('RequestStore scope filtering', () => { + let outputDir: string; + let store: RequestStore; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + store = new RequestStore(outputDir); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('scopes writes by the most selective segment shared with the page URL', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('PATCH', '/api/other-shop/suites/5', 200)); + + const scoped = store.getWriteRequestsForScope('/projects/alpha-shop/suites'); + + expect(scoped).toHaveLength(1); + expect(scoped[0].path).toBe('/api/alpha-shop/suites'); + }); + + it('ignores id-shaped page segments when choosing the scope', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('PATCH', '/api/alpha-shop/runs/8471', 200)); + + const scoped = store.getWriteRequestsForScope('/projects/alpha-shop/tests/8471'); + + expect(scoped.map((r) => r.path)).toContain('/api/alpha-shop/suites'); + }); + + it('returns nothing when the scope shares no segment with any write', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + + expect(store.getWriteRequestsForScope('/dashboard')).toHaveLength(0); + }); + + it('returns all writes for the root scope', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('POST', '/api/other-shop/labels', 201)); + + expect(store.getWriteRequestsForScope('/')).toHaveLength(2); + }); + + it('deduplicates endpoint list lines by id pattern', () => { + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/1a2b3c4d', 200)); + store.addCapturedRequest(makeRequest('PATCH', '/api/suites/9f8e7d6c', 200)); + + expect(store.toEndpointList()).toBe('PATCH /api/suites/{id}'); + }); + + it('scopes the endpoint list when a scope path is given', () => { + store.addCapturedRequest(makeRequest('POST', '/api/alpha-shop/suites', 201)); + store.addCapturedRequest(makeRequest('POST', '/api/other-shop/suites', 201)); + + expect(store.toEndpointList('/projects/alpha-shop')).toBe('POST /api/alpha-shop/suites'); + }); + + it('keeps version segments literal in the endpoint list', () => { + store.addCapturedRequest(makeRequest('POST', '/api/v1/suites', 201)); + + expect(store.toEndpointList()).toBe('POST /api/v1/suites'); + }); +}); + +describe('RequestStore loadFromDisk', () => { + let outputDir: string; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('loads only browser-captured requests from disk', () => { + makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites').save(outputDir); + makeRequest('POST', '/api/suites', 400, '001_POST_api_suites').save(outputDir); + + const fresh = new RequestStore(outputDir); + fresh.loadFromDisk(); + + expect(fresh.getCapturedRequests()).toHaveLength(1); + expect(fresh.getCapturedRequests()[0].id).toBe('xhr_001_POST_api_suites'); + }); +}); + +describe('extractAuthHeaders session gating', () => { + let outputDir: string; + + beforeEach(() => { + outputDir = mkdtempSync(join(tmpdir(), 'reqstore-')); + }); + + afterEach(() => { + if (existsSync(outputDir)) rmSync(outputDir, { recursive: true, force: true }); + }); + + it('ignores auth headers from captures of previous sessions', () => { + const stale = makeRequest('DELETE', '/api/old-project/suites', 200, 'xhr_033_DELETE_api_old', { 'x-csrf-token': 'stale-token' }); + stale.timestamp = new Date('2026-07-07'); + stale.save(outputDir); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + + expect(store.extractAuthHeaders()).toEqual({}); + }); + + it('returns auth headers from captures made during this session', () => { + const store = new RequestStore(outputDir); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, undefined, { authorization: 'Bearer live', 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ authorization: 'Bearer live', 'x-csrf-token': 'live-token' }); + }); + + it('never returns cookie headers from captures', () => { + const store = new RequestStore(outputDir); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, undefined, { cookie: 'session=captured', 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'live-token' }); + }); + + it('prefers the newest session capture when values differ', () => { + const store = new RequestStore(outputDir); + const older = makeRequest('POST', '/api/suites', 201, undefined, { 'x-csrf-token': 'first' }); + older.timestamp = new Date(Date.now() + 1000); + const newer = makeRequest('POST', '/api/tests', 201, undefined, { 'x-csrf-token': 'second' }); + newer.timestamp = new Date(Date.now() + 2000); + store.addCapturedRequest(older); + store.addCapturedRequest(newer); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'second' }); + }); + + it('resolves a same-id collision between a stale disk file and a live capture', () => { + const stale = makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites', { 'x-csrf-token': 'stale-token' }); + stale.timestamp = new Date('2026-07-07'); + stale.save(outputDir); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + store.addCapturedRequest(makeRequest('POST', '/api/suites', 201, 'xhr_001_POST_api_suites', { 'x-csrf-token': 'live-token' })); + + expect(store.extractAuthHeaders()).toEqual({ 'x-csrf-token': 'live-token' }); + }); + + it('treats a capture file without a timestamp as stale', () => { + const requestsDir = join(outputDir, 'requests'); + mkdirSync(requestsDir, { recursive: true }); + writeFileSync(join(requestsDir, 'xhr_002_POST_api_x.request.yaml'), '---\nmethod: POST\nurl: /api/x\nfullUrl: /api/x\nheaders:\n x-csrf-token: orphan\nstatus: 200\nstatusText: OK\nresponseHeaders:\n---\n', 'utf8'); + + const store = new RequestStore(outputDir); + store.loadFromDisk(); + + expect(store.extractAuthHeaders()).toEqual({}); + }); +}); diff --git a/tests/unit/url-matcher.test.ts b/tests/unit/url-matcher.test.ts index 36c6a1ef..39513378 100644 --- a/tests/unit/url-matcher.test.ts +++ b/tests/unit/url-matcher.test.ts @@ -50,6 +50,17 @@ describe('url-matcher', () => { expect(isDynamicSegment('new-test')).toBe(false); }); + it('rejects version segments', () => { + expect(isDynamicSegment('v1')).toBe(false); + expect(isDynamicSegment('v2')).toBe(false); + expect(isDynamicSegment('V3')).toBe(false); + }); + + it('still detects real ids alongside version segments', () => { + expect(isDynamicSegment('1a2b3c4d')).toBe(true); + expect(isDynamicSegment('8471')).toBe(true); + }); + it('respects user-provided dynamicPageRegex override', () => { const instance = ConfigParser.getInstance(); (instance as any).config = { ...(instance as any).config, dynamicPageRegex: '^custom-\\d+$' };