FE-1224, FE-1225: Add detached, reconnectable optimization runs to the optimizer and proxy them through NodeAPI#9067
FE-1224, FE-1225: Add detached, reconnectable optimization runs to the optimizer and proxy them through NodeAPI#9067kube wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
area/apps > hash*
PR SummaryHigh Risk Overview The optimizer gains Breaking: the legacy streaming optimize endpoint is removed; deploy must pair with the frontend cutover PR. Reviewed by Cursor Bugbot for commit 1848a8c. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
fec71b8 to
760a6f0
Compare
8763d1f to
a39600b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9067 +/- ##
==========================================
+ Coverage 59.32% 59.35% +0.02%
==========================================
Files 1397 1404 +7
Lines 135621 135765 +144
Branches 6295 6315 +20
==========================================
+ Hits 80462 80582 +120
- Misses 54187 54200 +13
- Partials 972 983 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
760a6f0 to
a23b8b0
Compare
a39600b to
c79703e
Compare
The detached-run routes are the only optimization surface NodeAPI exposes: the attached-study `POST /api/petrinaut-optimizer/optimize` route is deleted rather than reworked on top of the run engine, and the account occupancy tracker shrinks to the pending-create set that remains its only job. The frontend cutover to the detached flow lands in the follow-up PR — merge the two together, since a deployed frontend older than that PR can no longer start optimizations once this one is live.
`GET /optimize/runs/{run_id}/events` live-tails for as long as a consumer
stays attached, so its auto-instrumented SERVER span measured the tail
(minutes) rather than the attach, pinning the latency SLI at worst-case and
only exporting on disconnect. The route is now excluded from ASGI
auto-instrumentation and instead opens a short manual SERVER span — with
the extracted upstream trace context — covering run lookup, cursor
resolution, and attachment registration, ending when the tail starts. The
service-graph edge and the 404 error signal are preserved.
Also re-syncs the optimizer OpenAPI schema and the generated client types,
which had drifted from the route metadata, and folds the richer
events/delete response descriptions into the route definitions so the
schema stays reproducible.
kube
changed the title
… and harden the run engine
- A superseded attachment now ends with an attachment-scoped
`event: superseded` sentinel, decoded as a terminal, non-retryable
`attachment_superseded` error. Without it the consumer saw a truncated
stream, received a retryable error, and re-attached — superseding the
newer attachment right back, ping-ponging forever (e.g. two tabs on the
same run). NodeAPI forwards the event without releasing run ownership:
the run lives on under the newer consumer.
- Re-attaches get their own, looser rate-limit bucket (60/min): reconnect
backoff behind a flaky connection must not 429 an account off the event
stream of its own live run.
- The study wall-clock timeout no longer misreports a completion whose
sentinel is still in flight (the worker has already exited) as a failure.
- One bad run can no longer kill the orphan reaper: per-run reap work is
exception-guarded (an unreaped orphan would hold its slot until the next
create recreated the loop).
- The manual attach span no longer records expected 404s as exceptions,
and /status/{run_id} liveness probes are excluded from HTTP
instrumentation like the /status snapshot already was.
- Dead code: pump_events' unused local callback, the impossible
non-tuple worker guard, and the always-true study-span check are gone.
Every optimization route repeated the same preamble — the correlated request logger, the 401 auth check, the 503 unconfigured-optimizer check — and two of them repeated the ownership-or-404 guard and the 502/504 upstream-failure response verbatim. They now live in shared/optimization-route-context.ts, so each handler body is only its own logic. No behavior change: the handler test suites pass unchanged.
…API to a proxy The optimizer is the single source of truth for run state, including who owns a run: - Run creation carries the account's opaque `x-hash-account-id` tag (stamped by NodeAPI behind its authentication). The optimizer enforces per-account single-flight inside its existing admission lock — a pending set covers the CLI-initialization window — and attach/cancel answer 404 for a foreign or absent tag, identical to an unknown run. Requests without the tag (local development, the website demo) create ownerless, openly attachable runs. Attach responses report `X-Requested-Trials` so consumers can size synthesized summaries without remembering manifests. - NodeAPI keeps exactly the basics: authentication, rate limits, manifest validation, NDJSON conversion, and error mapping. Its shadow copy of run state is gone — the ownership map and TTL sweep, the pending-create occupancy set, the create-time liveness probe, and the release-ownership-on-terminal-delivery rules were all machinery for keeping that copy honest, and the staleness bugs they guarded against cannot exist anymore. The optimizer's 429 detail is forwarded so the browser still distinguishes account-busy from service capacity. - The handwritten create/cancel/status client functions are replaced by an `openapi-fetch` client over the already-generated schema types (the create response is now a typed model instead of a plain dict). The SSE attachment adapter stays handwritten — the frame protocol is not expressible in OpenAPI — and now reads the requested-trial count from the response header.
github-actions
Bot
added
the
area/deps
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1848a8c. Configure here.
| PETRINAUT_OPTIMIZER_OPTIMIZE_RUN_PATH, | ||
| optimizationRateLimiter, | ||
| createPetrinautOptimizationHandler({ fetchImpl, logger, origin }), | ||
| createPetrinautOptimizationRunCancelHandler({ client, logger, origin }), |
There was a problem hiding this comment.
Cancel shares create rate limit
Medium Severity
DELETE …/optimize/runs/:runId reuses the same optimizationRateLimiter instance as create (10/min). Attachments were given a separate bucket specifically so a busy account is not 429’d off its own live run, but cancel was left on the create bucket. Failed or retried creates still consume that budget, so an account can be temporarily unable to cancel the run that is holding its single-flight slot.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1848a8c. Configure here.
2 participants
🌟 What is the purpose of this PR?
Today an optimization study lives exactly as long as one browser HTTP connection: any transport reset kills the study (reproduced at trials 69/73 of 100 in SRE-831), and cancellation is implied by disconnect. This PR decouples them: a run is created detached, identified by a
run_id, and any number of sequential consumers can attach, detach, and re-attach to its event stream with a cursor — the run itself is only ever stopped by an explicit cancel, a wall-clock ceiling, or the orphan reaper.The optimizer is the single source of truth for run state, including who owns a run: NodeAPI shrinks to a thin, stateless proxy (authentication, rate limits, manifest validation, SSE→NDJSON conversion, error mapping).
Consolidates FE-1224 (optimizer engine, previously #9064) and FE-1225 (NodeAPI proxy) into one backend PR. The frontend cutover is stacked on top in #9066.
🔗 Related links
🔍 What does this change?
Optimizer (
apps/petrinaut-opt)POST /optimize/runsadmits against the existing 4-slot cap (429 +Retry-After), starts CLI + study as a background task owned by the run, and returns201 {"run_id"}immediately.GET /optimize/runs/{run_id}/events?cursor=N(SSE) replays buffered frames with seq > cursor (id:carries the seq), then live-tails with heartbeats. Disconnecting never affects the run; a newer attachment supersedes a stale one, which ends with anevent: supersededsentinel so its consumer stops cleanly instead of reconnecting into a supersede war.DELETE /optimize/runs/{run_id}cancels explicitly and idempotently, with a prompt process-group close.HASH_PETRINAUT_OPT_DETACH_GRACE_SECONDS(default 300).x-hash-account-idtag (stamped by NodeAPI behind its auth). Per-account single-flight is enforced inside the admission lock, and attach/cancel answer 404 for a foreign or absent tag — identical to an unknown run, so run ids cannot be probed. Requests without the tag (local development, the website demo) create ownerless, openly attachable runs.optimization.studyspan hanging off the creating request.NodeAPI (
apps/hash-api)POST …/optimizeroute is deleted rather than re-implemented on top of the engine.Client (
@local/petrinaut-optimizer-client)openapi-fetchclient over the already-generated schema types; only the SSE attachment adapter (cursor replay, canonical-event decoding, terminal semantics) stays handwritten — its frame protocol is not expressible in OpenAPI.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🛡 What tests cover this?
apps/petrinaut-opt: run-engine unit tests (event log, attachment epochs/supersede, reaper, shutdown), API tests over the real endpoints (replay/cursor semantics, capacity + account single-flight, ownership visibility, orphan reaping), pump lifecycle tests (cancellation, wall-clock ceiling, correlation logging).apps/hash-api: handler tests for create/events/cancel (validation without logging user content, account-tag forwarding, NDJSON replay, retryable timeout lines, superseded forwarding, 404/429 passthrough, disconnect compensation).@local/petrinaut-optimizer-client: decoder and attach tests (seq mapping, terminal rules, oversized events), openapi-fetch adapter tests.❓ How to test this?
yarn devwith the optimizer running (infra/composeincludes it).run_idin the network tab (POST /api/petrinaut-optimizer/optimize/runs).GET …/events?cursor=Nresumes from where you stopped without duplicating trials.DELETE …/optimize/runs/:runIdstops it promptly; a second DELETE still answers 204.