kyleandClaude Opus 5.5
103ec3d05a
Task 02 (the whole runner) ended at 21:08 with no file and no commit: Ornith wrote the
implementation out in its reply (~700 lines), OpenCode compacted the session, and the model
then emitted a malformed tool call. It had also looked for the given test at a wrong path
("_files/…" relative to the plan: my ambiguity).
- 02-runner → 02-runner-core (types, Submit, Status, Touch, Cancel; no goroutines) and
03-runner-workers; the given test split to match (runner_core_test.go gains three tests:
queued storage, cancel while queued, Status never moves the lease); later tasks renumbered
04-09 and every cross-reference checked.
- Copy paths spelled from the repo root.
- guard.PreCheck (+ its test) is committed here by the owner: the old task 05 told the
implementer to edit internal/guard/, which AGENTS.md forbids. My contradiction.
- AGENTS.md rule 6: write code into files early, in small steps.
Both runner test files compile against a panic-only skeleton (the core file also alone) and
fail there on panic: todo.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
87 lines
4.1 KiB
Markdown
87 lines
4.1 KiB
Markdown
# P6 task 05: extract `v1fetch`'s pipeline into `fetchTarget`
|
|
|
|
**Branch:** `p6` (`git switch p6`; `git status --short` must be empty, otherwise stop)
|
|
**Commit subject:** `api: v1fetch's per-target pipeline becomes fetchTarget (no behaviour change)`
|
|
|
|
> This is the one task in P6 that edits working code. It is a **move, not a rewrite**: the
|
|
> reviewer will diff the old `v1fetch` body against `fetchTarget` line by line.
|
|
|
|
## Goal
|
|
|
|
`v1fetch` today inlines the whole per-target pipeline: fetch → recover → PDF render or clean →
|
|
question limit → digest → context → log line → envelope. The batch worker (task 06) needs the
|
|
same pipeline for one target. Move it into one method that both call:
|
|
|
|
```go
|
|
// internal/api/target.go
|
|
func (s *Server) fetchTarget(ctx context.Context, c *auth.Caller, t batch.Target) (batch.Result, error)
|
|
```
|
|
|
|
`v1fetch` keeps its exact responses. **Every existing api test must pass unchanged.**
|
|
|
|
## Files
|
|
|
|
- Copy: `docs/plans/p6/_files/api/fetchtarget_test.go` → `internal/api/fetchtarget_test.go`
|
|
- Create: `internal/api/target.go` (`fetchTarget`)
|
|
- Modify: `internal/api/api.go` (`v1fetch` shrinks; `doFetch` takes a `context.Context` in place of
|
|
the `*http.Request`, and its call sites pass `r.Context()`), `docs/implementer-log.md`
|
|
|
|
## What already exists
|
|
|
|
```go
|
|
// internal/batch (task 02)
|
|
type Options struct{ Mode, Question, Pages, Grep, JQ string; Context *bool; Recover bool }
|
|
type Target struct{ URL string; Options Options }
|
|
type Result struct{ Content string; Meta map[string]any }
|
|
```
|
|
|
|
`fetch.Error{Status int; Msg, Hint string; RetryAfter int}` is the error type `writeFetchErr`
|
|
already turns into a response.
|
|
|
|
## Rules
|
|
|
|
1. **What stays in `v1fetch`:** `s.reqN.Add(1)`, the caller check, decoding `v1Req`, and the
|
|
`400 invalid url` for a bad body or an empty URL. Then it builds a `batch.Target` from the
|
|
request (`URL`, and `Mode`, `Question`, `Pages`, `Grep`, `JQ`, `Context`, `Recover` into
|
|
`Options`), calls `s.fetchTarget(r.Context(), c, t)`, and:
|
|
- on an error, `s.writeFetchErr(w, c, req.URL, err)`;
|
|
- on success, `writeJSON(w, 200, env)` where `env` is `res.Meta` plus `"content": res.Content`.
|
|
2. **What moves into `fetchTarget`, in the same order and unchanged:** the question truncation
|
|
to 500 runes, the mode default and check, `wantCtx`, `doFetch`, the recover path
|
|
(`recover.Try(ctx, …)`), the PDF branch (render, `pages`/`grep` errors, the question-chunk
|
|
limit), the clean/raw branch, the digest, the context search, the `fetch` log line, and the
|
|
envelope.
|
|
3. **Errors that `v1fetch` wrote directly with `writeErr` become `*fetch.Error` values with the same
|
|
status, message and hint:** `400 invalid url` for a bad mode, `400 raw mode does not apply to
|
|
PDFs`. `writeFetchErr` then writes them exactly as before. The only visible difference is its
|
|
`fetch failed` log line, which is fine.
|
|
4. **The envelope:** `fetchTarget` returns `Result{Content: content, Meta: m}` where `m` holds every
|
|
other key of today's `out` map (`mode`, `untrusted`, `source`, `warnings`, `sizes`, `timings`,
|
|
and `pdf`, `digest`, `context` when present). Never put `content` in `Meta`.
|
|
5. **`doFetch(ctx, c, rawURL, jq, onlyMain)`:** `r.Context()` becomes `ctx`. That is the only change
|
|
in `doFetch`. `scrape` (the other caller) passes `r.Context()`.
|
|
6. No new behaviour, no new log fields, no renamed JSON keys. If something cannot move without
|
|
changing behaviour, stop and report it.
|
|
|
|
## Steps
|
|
|
|
- [ ] **1.** `git switch p6`; copy the given test. **2. See it fail** (`fetchTarget` undefined).
|
|
- [ ] **3.** `doFetch(ctx, …)` and its call sites. **4.** `target.go`. **5.** Shrink `v1fetch`.
|
|
- [ ] **6.** `gofmt -w`; `go test -race -count=3 ./internal/api/` → `ok`, every existing test
|
|
included. **7.** `make gate`.
|
|
- [ ] **8.** Row `p6/05-fetchtarget`; commit.
|
|
|
|
```sh
|
|
git add internal/api docs/implementer-log.md
|
|
git commit -F .state/commit-msg.txt
|
|
```
|
|
|
|
## Done when
|
|
|
|
- The two given tests and **every existing api test** pass under `-race -count=3`, with no
|
|
existing test file modified; `make gate` ok; the given file is byte-identical.
|
|
|
|
## Stop and report if
|
|
|
|
- An existing api test fails and fixing it would need a change to that test.
|