fix(daemon): correct the job trace accounting between the event stream and the flush

Closes #115 (closed). Closes #116 (closed).

Two tracker issues in the same code, plus two more defects the review of that code turned up. One fix per commit.

Commits

1. fix(daemon): stop FlushTrace duplicating bytes into the persisted trace — #115 (closed)

FlushTrace read its delivered-byte count under eventMu, dropped the lock across a blocking GET /internal/trace, then sliced full[delivered:] with the pre-request value. The event stream keeps running throughout, so frames arriving during the request were written by the dispatcher and then written again by the flush — into the trace file, where the duplicate is permanent and re-served to every later reader.

Each job now has its own lock, held across both the write and the position update, and the position is read under it after the fetch. Per job rather than the client-wide eventMu because a trace writer is a user-facing tee that may block on disk, and eventMu gates every executor-side call for every job.

2. fix(daemon): make RemoveTraceWriter wait for in-flight trace writes — #116 (closed)

Removal only deleted the map entry. A dispatch already past the lookup could write after the executor closed the writer: the write fails on the closed trace file, but history.TeeWriter still publishes the line to the event bus, so it reached live viewers and never reached disk — invisibly, since both sides discard the write error. Removal now takes the job's lock and clears the writer, so it doubles as a barrier.

RemoveTraceWriter's own doc had the ordering backwards, telling callers to remove after closing. The executor has always removed first.

3. fix(daemon): track the trace stream by offset, not by bytes counted — found in review

Frames carry their absolute offset in the job's trace, in the same coordinate space GET /internal/trace serves. The client parsed that field out of the header and discarded it, reconstructing position by summing the lengths of frames it received.

A sum cannot express a hole, and the mock's bus drops whichever frame is publishing when a subscriber's buffer fills — not necessarily the last. Reproduced: frames 1 and 3 of "aaa\nbbb\nccc\n" delivered, frame 2 dropped, gave "aaa\nccc\nccc\n" — bbb lost permanently, ccc duplicated. So #115 (closed)'s symptom survived via a different trigger.

Two positions now, both offsets: contiguous (first byte the writer is missing) and delivered (end of the highest range written). They differ only after a drop. Recovery resumes from contiguous, filling the gap and repeating what follows — the trade this path already makes, a duplicate over a missing line — and the no-drop case is now exact. Re-delivered frames are dropped and overlapping ones trimmed, so a reconnect replay no longer duplicates.

4. fix(daemon): retire the trace writer that a re-registration replaces — found in review

SetJobTraceWriter swapped the map entry and returned, so the barrier from commit 2 could be walked around through the other door. Both paths now go through one retireTrace helper. No production caller replaces a live registration today, so this closes the gap rather than fixing an observed failure.

Tests

Regression tests for #115 (closed), #116 (closed) and the offset misalignment are mutation-checked — each verified to fail against the code it replaces, e.g. the offset test reproduces "aaa\nccc\nccc\n" exactly. Also covered: recovery of the tail and of frames predating registration, removal during a fetch, 404/5xx responses, reconnect replay dedup, a frame spanning a hole, a hole-filling redelivery below the high-water mark, failed writes, and the FlushTrace-before-RemoveTraceWriter ordering the executor depends on (swapping them in cleanupTrace would otherwise make tail recovery a silent no-op with every test still passing).

A new mock-server test pins the offset contract itself — frames index the byte array the flush endpoint serves, including after a Content-Range de-duplicated append — since a divergence there would surface only as duplicated or missing lines in a job log, far from its cause.

go build, go vet, make test, and -race on pkg/daemon are green. The unit CI job was run locally with glci before pushing: pkg/daemon and pkg/mockserver pass in-container. The two pkg/variables failures it reports are #117 (closed) — they fail identically on an unmodified v0.7.0 tree whenever glci runs from a non-main branch.

Out of scope

Found in review, left alone deliberately: the retry path never flushes the failed attempt's trace before discarding it; the timeout/cancel path skips the flush entirely, which is where a dropped tail matters most; the resume path is bus-only and never persists (#114 (closed) — a comment there now records why it must not naively flush); the mock's event bus can publish on a channel unsubscribe has closed; PATCH /jobs/:id/trace never returns 416 and its Range header is off by one against real GitLab.