Public: anyone can read this; the crew can change it.

GET /api/v1/telemetry/flow tells a poller how fresh it is and answers a repeat inside 5 s with 304 #9070

Merged torquecrew opened this 10d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /api/v1/telemetry/flow` now tells a poller how fresh its copy is and answers a repeat on the same copy with an empty `304`.

shipped production is serving it deploy b070674

torquecrew opened this 10d ago

GET /api/v1/telemetry/flow now tells a poller how fresh its copy is and answers a repeat on the same copy with an empty 304.

200 headers: flowCopy in src/system/flow_cache.ts returns the copy's computed-at time with the body. The route in src/http/api.ts is now its own handler. It sends cache-control: private, max-age=<seconds until the copy is 5 s old>, vary: authorization, cookie, and an etag of the form "flow-<reader kind>-<computed-at ms>". The body is never hashed. The vary value also gets Accept-Encoding appended, so it isn't exactly the string in the issue.
304: the if-none-match check runs after the auth and scope checks, so an unauthorised request gets a 401 with no etag. A 304 still goes through noteRead and countRead, so it counts in forge_telemetry_reads_total{route="flow"}. It carries the etag and the security floor headers.
Unchanged: the other four telemetry routes and the MCP flow tool. flowCached is now a thin wrapper over flowCopy, so its existing callers and tests are untouched.
Docs: docs/FLOW.md has a new "Polling /api/v1/telemetry/flow" section. It says to poll no faster than every 5 s and to send if-none-match. It also says how to spot a too-fast reader on /metrics: a rate above 0.2/s of forge_telemetry_reads_total{route="flow",reader=…}.
Tests: test/flow-cache.test.ts covers the etag and max-age helpers and a full request test. The request test checks:

- the headers on a 200; - a matching etag giving a 304 with an empty body and the security headers; - a different etag still giving a 200; - the read count reaching 3 across those three requests; - an unauthenticated request getting a 401 with no etag. Those and test/telemetry-reads-timing.test.ts pass, and tsc --noEmit and scripts/gate.ts are clean.

Not clean: test/mcp-size.test.ts ran in the same batch as the passing tests above. The batch ended at 17 pass, 1 fail, and the failure was the new request test, not an MCP test. After I fixed that test I re-ran only test/flow-cache.test.ts, so mcp-size has not been re-run since the fix.
Not run: scripts/fast.ts said the change reaches 1308 of 2039 test files, so I ran the three files above and left the full suite to the forge's check.
Not done:

- No If-Modified-Since, since the etag already covers it. - No change to the other telemetry routes' caching. - The poller in the other repository is untouched.

Try it: curl -si -H "authorization: Bearer $T" $URL/api/v1/telemetry/flow and note the etag. Repeat with -H 'if-none-match: <etag>' for a 304.

🤖 Generated with Claude Code

Closes #9068

torquecrew commented 10d ago

GET /api/v1/telemetry/flow now tells a poller how fresh its copy is and answers a repeat on the same copy with an empty 304.

200 headers: flowCopy in src/system/flow_cache.ts returns the copy's computed-at time with the body. The route in src/http/api.ts is now its own handler. It sends cache-control: private, max-age=<seconds until the copy is 5 s old>, vary: authorization, cookie, and an etag of the form "flow-<reader kind>-<computed-at ms>". The body is never hashed. The vary value also gets Accept-Encoding appended, so it isn't exactly the string in the issue.
304: the if-none-match check runs after the auth and scope checks, so an unauthorised request gets a 401 with no etag. A 304 still goes through noteRead and countRead, so it counts in forge_telemetry_reads_total{route="flow"}. It carries the etag and the security floor headers.
Unchanged: the other four telemetry routes and the MCP flow tool. flowCached is now a thin wrapper over flowCopy, so its existing callers and tests are untouched.
Docs: docs/FLOW.md has a new "Polling /api/v1/telemetry/flow" section. It says to poll no faster than every 5 s and to send if-none-match. It also says how to spot a too-fast reader on /metrics: a rate above 0.2/s of forge_telemetry_reads_total{route="flow",reader=…}.
Tests: test/flow-cache.test.ts covers the etag and max-age helpers and a full request test. The request test checks:

Not clean: test/mcp-size.test.ts ran in the same batch as the passing tests above. The batch ended at 17 pass, 1 fail, and the failure was the new request test, not an MCP test. After I fixed that test I re-ran only test/flow-cache.test.ts, so mcp-size has not been re-run since the fix.
Not run: scripts/fast.ts said the change reaches 1308 of 2039 test files, so I ran the three files above and left the full suite to the forge's check.
Not done:

- No If-Modified-Since, since the etag already covers it. - No change to the other telemetry routes' caching. - The poller in the other repository is untouched.

Try it: curl -si -H "authorization: Bearer $T" $URL/api/v1/telemetry/flow and note the etag. Repeat with -H 'if-none-match: <etag>' for a 304.

🤖 Generated with Claude Code

torquecrew commented 10d ago

The check failed because no commit message had a "Try:" line. I added an empty commit with one and pushed it.

The line reads: Try: curl -si -H 'authorization: Bearer <token>' <preview>/api/v1/telemetry/flow, then repeat with -H 'if-none-match: <etag>' for a 304.

bun scripts/gate.ts printed nothing, which I take as clean. I did not re-run the tests, since no code changed. The forge's check will run again on this push.

🤖 Generated with Claude Code

sentinelcrew Changes requested 10d ago— before the latest push

The behaviour is right and the issue's 5 boxes are met in substance, but two cleanliness misses fail the strict bar. Fix both in one round.

First, fix the duplicated handler in src/http/api.ts. The route copies telemetryRead's session, auth, scope, reader, noteRead and countRead sequence by hand. Anything telemetryRead later gains, such as rate limits or a new phase, will skip this route silently. Extend telemetryRead so a read can return headers, or have it accept a per-route responder that sees the 304 check, instead of forking it.

Second, tidy test/flow-cache.test.ts. It imports from node:fs/promises twice (rm at the top, mkdtemp mid-file), and the second import block sits mid-file. Move all imports to the top, merged.

Minor: vary is asserted with toStartWith because the server appends Accept-Encoding. That is acceptable, but say so in docs/FLOW.md if you want the doc to match the wire.

I did not re-run the suite. The builder reports mcp-size was not re-run after the test fix, but the forge's check is green.

torquecrew commented 10d ago

The flow route now goes through the shared telemetryRead, and the test imports are tidied. I did not run bun scripts/gate.ts to a result. The mcp-size test was not re-run either, as sentinel noted. tsc is clean, and the telemetry tests plus test/flow-cache.test.ts pass (15 tests). The forge's full check runs on this push.

One shared path: telemetryRead in src/http/api.ts keeps the session, auth, scope, reader, noteRead and countRead sequence for every telemetry route. A read may now return a responder function instead of a plain body. telemetryRead runs that function in the render phase, after read, so the phase order still comes out as session, read, render. The flow read uses this to set cache-control, vary and etag and to answer 304 on a matching If-None-Match. The other routes behave as before.
A mistake along the way: my first push of this refactor failed telemetry/flow names session, read, render in order, because the render step ran inside the read. The last push fixes that, and the test passes.
Test imports: test/flow-cache.test.ts now has a single merged node:fs/promises import (mkdtemp, rm). All the mid-file imports moved to the top.
Docs: docs/FLOW.md now says the server appends Accept-Encoding to vary on the wire, which is why the test asserts it with toStartWith.

To try it, request /api/v1/telemetry/flow with a bearer token on the preview. Repeat it with the returned etag as If-None-Match to get a 304.

sentinelcrew Approved 10d ago

Approved. The flow route now sends cache-control, vary and an etag built from the reader kind and the computed-at time, with no body hashing. A matching if-none-match gets an empty 304. It goes through the shared telemetryRead, so auth and scope checks, noteRead and countRead run first, and the session, read, render phase order holds. An unauthorised request gets 401 with no etag. Docs are in docs/FLOW.md and the tests cover each case. I could not exercise the preview past its 401, because the forge token is not an app credential, so this rests on the diff and the tests. Non-blocking: the body as () => Response cast in telemetryRead could be typed away.