Give Fisherman's run one RequestHaul and thin the finish tool

[DavertMik]

Implements plans/028-fisherman-haul.md. Pure refactoring — no behavior change.

The problem

PR #160 made Fisherman's success verdict depend on the requests made during a run. But "this run's requests" and "a failed request" were each defined in several places:

• createFishermanTools captured its own ledgerStart mark
• Fisherman.prepareData captured a second one on the next line
• the failure predicate (r.status >= 400 || r.error) was written twice
• the successful-write predicate once more

The two marks agreed only because the two statements happened to be adjacent. Any reordering would make the stuck-detector and the result-synthesizer disagree about which requests belong to the run — reporting "success" on an abandoned run, exactly the bug #160 set out to kill.

The change

One RequestHaul owns the run — what one outing brings in. It lives at src/ai/fisherman/request-haul.ts, beside the only agent that uses it, the way other agents keep their utility classes (ai/researcher/*). Constructed once when the run starts (immediately after refreshAuth(), so auth probes never count), it exposes everything made after that point — requests(), failed(), successfulWrites(), byId() — so the stuck-detector, the result-synthesizer and finish all read the same definitions.

The failure predicate lives once, as isFailedRequest in the data tier (request-store.ts). RequestHaul.failed() and isStuckOnEndpoint both call it; there is no second textual copy.

finish is thin again. Its execute body held Fisherman's most important policy — what counts as created — as inline judgment, violating the repo's glue-tier rule (CLAUDE.md: "Tools = schema + result parsing only, delegating every real operation to an agent or data module in one call"). That verification moved verbatim into an exported verifyFinish(haul, input); the tool is now one call plus result parsing.

src/utils/request-map.ts is deleted. A 19-line class with exactly one consumer, and the only file in src/utils/ importing from ../api/. RequestHaul.byId() replaces it.

Verification

• bun test tests/unit — 1244 pass / 0 fail
• bun test tests/integration/fisherman.test.ts — 5 pass / 0 fail
• bun run format / bun run lint — exit 0
• grep -rn 'getMadeRequests().length' src/ — one match, the RequestHaul constructor (the run's start mark exists nowhere else)
• grep -rn 'RequestMap\|request-map' src/ boat/ bin/ tests/ — no matches

Notes for the reviewer

• The plan's done-criterion grep 'status >= 400' … | wc -l → 1 reports 3. The other two hits are pre-existing per-response status checks inside getEndpointSpec and the request tool — both explicitly out of scope, and neither is the run-failure predicate the criterion means. They were left alone rather than rewritten to force the count. The predicate itself now exists in exactly one place, request-store.ts.
• RequestHaul's unit test stays in tests/unit/request-store.test.ts — it drives a real RequestStore and reuses that file's makeRequest fixture, which is defined nowhere else.
• No CHANGELOG.md entry: the plan scopes it out and gates on "no out-of-scope files modified", and there is no user-visible behavior change to describe. Happy to add one.
• Out of scope and untouched, per the plan: RequestStore's dedup/alias cleanup (that is plan 030, which also edits request-store.ts — land this first), src/api/request-result.ts, and every prompt string in fisherman.ts.

🤖 Generated with Claude Code

https://claude.ai/code/session_01B46vQvvc47LJeCmqQxAhme