FE-1222, FE-1226: Classify optimization transport errors, auto-reconnect runs by id + cursor, and remove the legacy optimization path - #9066
Open
kube wants to merge 27 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
area/apps > hash*
|
Semgrep found 1
No validation of origin is done by the addEventListener API. It may be possible to exploit this flaw to perform Cross Origin attacks such as Cross-Site Scripting(XSS). |
kube
marked this pull request as ready for review
July 19, 2026 20:20
PR SummaryHigh Risk Overview HASH frontend splits the iframe bridge into create (201 +
Reviewed by Cursor Bugbot for commit 79c0b6f. 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}
YannisZa
previously approved these changes
Jul 19, 2026
Replace the raw `network error` the host forwarded from a caught
fetch/reader exception with a classified, actionable error, and thread the
NodeAPI correlation ids to the UI so a failed run is traceable to the
NodeAPI and optimizer logs.
- messages: optimizationResponseStart carries the x-hash-request-id /
X-Optimization-Run-ID response headers; optimizationError gains a
category (network/http/protocol/aborted) plus optional correlation ids,
http status, and error name.
- host (process-editor): captures the correlation headers, forwards them,
and classifies its own transport failures as `network` with a safe
message.
- bridge: a PetrinautOptimizationTransportError carries the structured
fields; HTTP (non-ok) failures are classified `http` with status +
correlation ids, protocol violations `protocol`, and the response-start
timeout `network`; correlation ids captured at response-start backfill a
later stream error.
- provider: builds a safe, progress-aware message ("...interrupted after N
of M trials. Retry the optimization. (diagnostic id: ...)"), keeps the
received trials, stores the category/diagnostics on the record, and adds
retryOptimization to start a fresh run from the failed one's input.
- view drawer: a Retry action on a failed run.
Tests cover network/http/protocol classification, correlation-id
threading through the bridge, the composed message + preserved trials, and
retry.
…cursor Decouple a Petrinaut optimization from the single browser connection that started it. A run is now created by id, its NDJSON event stream is (re-)attached with a seq cursor, and it is cancelled explicitly — so a dropped connection resumes instead of failing the run, and a reload can re-attach where storage allows. - petrinaut-core: every optimization event variant gains an optional, server-authoritative `seq`; the host capability gains optional createOptimizationRun / attachOptimizationRun / cancelOptimizationRun methods, so hosts that only implement `optimize` keep type-checking and working. - messages: new iframe->host optimizationCreate / optimizationAttach / optimizationCancel kinds and a host->iframe optimizationCreateResult reply; attachment streams reuse the existing responseStart/chunk/end/ error family keyed by request id. - host (process-editor): the shared NDJSON relay is factored out and used by both the legacy POST flow and the new cursor GET flow; create POSTs and replies with runId or a classified failure (status, Retry-After, safe message); cancel DELETEs fire-and-forget; the two legacy error replies that predate FE-1222 now carry their `protocol` category. - bridge: implements the three capability methods over those messages, rejecting creation failures with a classified PetrinautOptimizationTransportError that now also carries retryAfter. - provider: detached runs store runId/lastSeq/connectionState on the record, apply events idempotently (skipping replayed seq <= cursor), and re-attach with exponential backoff (1s doubling, capped 30s, reset on any received event) for network/protocol failures until 8 consecutive attachments fail, then surface the FE-1222 classified error; cancel aborts locally and stops the run server-side; active runs are recorded in sessionStorage and re-attached (cursor 0) on mount, silently dropping ids the service no longer knows; the legacy optimize() path is unchanged when the new methods are absent. - drawer: shows "(reconnecting…)" next to the status while a dropped stream is being re-established. - docs: optimization.md now describes automatic reconnection, reload re-attachment, and Cancel as the way to stop the server-side run.
Match the final FE-1225 browser-facing contract: - URLs: detached runs live under POST/GET/DELETE /api/petrinaut-optimizer/optimize/runs[/{runId}[/events]]; the legacy streaming POST keeps /optimize. The attach cursor is clamped to NodeAPI's ^\d{1,15}$ validation range. - Attachments emit no `started` event: the record turns `running` on the first applied event, and the test fakes now start with a trial line. - Attachment trials and summaries carry `best: null`, so the provider now computes the running best itself from every applied completed trial (using the manifest's objective direction), preferring `event.best` when the legacy stream provides it. The best therefore survives reconnects and reload replays instead of freezing at its pre-disconnect value. - Attachment `complete` counts are since-cursor, not totals: the detached flow keeps the record's accumulated per-trial counters and running best, taking only `requestedTrials` (the true total) and the status from the event. The legacy flow still trusts its summary. - A NodeAPI-authored terminal error event with `retryable: true` (its per-attachment window timed out while the run continues) now feeds the same reconnect-with-cursor path as a dropped connection, sharing the failure cap and its reset-on-progress; only when reconnecting is exhausted does the event's own message become the run's outcome. `retryable: false` events stay terminal, as does any error event in the legacy flow. - A mid-run reconnect that 404s (NodeAPI dropped the run) surfaces the classified http error immediately without further retries; the reload path still drops such records silently.
…emantics - Orphaned runs / account busy-lock: every give-up path in the attach loop (retryable-window exhaustion, non-reconnectable classified failure, unclassified throw) now fire-and-forget cancels the possibly-live run before surfacing the terminal error, releasing NodeAPI's per-account ownership slot so a follow-up run (e.g. Retry) isn't rejected as busy until the ownership TTL. The sessionStorage entry is only forgotten once that cancel resolves, so a reload during the same outage can still recover the run; and explicit Cancel/Remove fall back to the record's own runId once the attach loop has ended, so a user can still DELETE a run after a surfaced terminal error. - Docs: stop promising reload re-attachment the shipped sandboxed iframe cannot deliver (opaque origin, no sessionStorage). The page now states that reloading loses the view of a still-running optimization, that the run continues server-side and can block new optimizations, and to Cancel first when intending to run something else. A host-side storage bridge remains a follow-up feature. - Reconnect predicate: gateway statuses (502/503/504) on re-attach — NodeAPI restarting or deploying — now reconnect within the same failure cap; 404 and all other http failures stay immediately terminal. - Busy message: a create rejected 429 now explains that another optimization may already be running for the account, including the Retry-After delay when NodeAPI sent one. - Bridge create races: a late optimizationCreateResult for a create that already aborted/timed out locally (or an unknown request id) posts optimizationCancel for the orphaned runId; accepted creates are tombstoned briefly so duplicated replies never cancel a live run. - StrictMode: the reload re-attach effect now cleans up after itself (aborts its loops and drops its records) instead of a provider- lifetime ref guard, so double-invoked effects rebuild cleanly rather than duplicating restored runs. - Honest connection state: attachOptimizationRun gains an optional onAttached callback (fired by the bridge when the attachment response arrives OK) which the provider uses to clear "(reconnecting…)" on quiet-but-healthy reattachments. Deliberate trade-off, documented in code: only received events reset the failure counter, so attachment windows that keep dying without progress still exhaust the cap.
kube
force-pushed
the
cf/fe-1222-classify-petrinaut-optimization-transport-errors
branch
from
July 23, 2026 16:23
b54c931 to
cf08515
Compare
kube
force-pushed
the
cf/fe-1226-petrinaut-optimization-reconnect
branch
from
July 23, 2026 16:24
198a2ce to
469d5b1
Compare
kube
changed the base branch from
cf/fe-1222-classify-petrinaut-optimization-transport-errors
to
cf/fe-1225-nodeapi-detached-run-proxy
July 23, 2026 18:10
kube
marked this pull request as draft
July 23, 2026 18:10
# Conflicts: # apps/petrinaut-opt/src/petrinaut_optimizer.py
…rden the bridge - The website demo classifies mid-stream decode failures as `protocol` (and HTTP failures with their status), so the provider's reconnect-with-cursor path works there too instead of failing the run on the first drop. - The iframe bridge installs its message listener at creation rather than lazily on the first optimization call: a late `optimizationCreateResult` arriving right after an iframe reload is now heard, so its orphaned run is cancelled instead of lingering until the optimizer's reaper collects it. - The never-consumed `optimizationError.errorName` field is gone. - The client decoder's superseded-attachment mapping (from the backend branch) is folded into the single-mode decoder.
kube
marked this pull request as ready for review
July 23, 2026 19:46
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.
Adopts the optimizer-owned run-state redesign; the website demo drives the detached-run API through the generated openapi-fetch client (its local requested-trials bookkeeping is replaced by the X-Requested-Trials header).
Cancel is how an account frees its own single-flight slot: with cancel on the 10/min admission bucket, an account that burned its budget on rejected creates could not cancel the very run blocking those creates until the window rolled. Cancel joins attachment in the looser non-admission bucket. Flagged by Cursor Bugbot on 1848a8c.
Creation only resolves once the study is running server-side, and attachments emit no started event — a quiet run stayed on "initializing" until its first trial despite already streaming. Flagged by Cursor Bugbot on 469d5b1.
…runs past give-up - The website demo classifies request-time fetch failures as `network` (HTTP errors keep their status semantics; aborts pass through), so an attach that dies before responding reconnects with backoff exactly like a mid-stream drop instead of definitively failing a possibly-live run. - The provider's reconnect give-up path no longer forgets the stored run when the cancel promise resolves: some hosts' cancel is fire-and-forget, so resolution proves nothing. The entry survives and the next reload's re-attach settles the run's true fate — a delivered cancel replays the cancelled terminal, a reaped run 404s and is silently dropped, and a run the cancel never reached is recovered live. Both flagged by Cursor Bugbot.
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 79c0b6f. Configure here.
github-actions
Bot
added
area/deps
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌟 What is the purpose of this PR?
Cuts the frontend over to the detached-run optimization contract from #9067 — and makes it the only contract. A dropped connection no longer fails an optimization: transport failures are classified and the UI auto-reconnects by run id and cursor, resuming exactly where it stopped. The legacy single-connection path is removed end to end (frontend, NodeAPI surface, optimizer routes, client, demo hosts).
Consolidates FE-1222 (transport-error classification, previously #9063) and FE-1226 (auto-reconnect). Stacked on #9067.
🔗 Related links
🔍 What does this change?
Classified transport errors (FE-1222)
network/http/protocol/aborted) and carries thex-hash-request-id/X-Optimization-Run-IDcorrelation ids; the provider builds safe, progress-aware messages ("…after 73 of 100 trials. Retry the optimization.") instead of surfacing raw exception strings. Already-received trials are preserved, and Retry remains available from the error state.Auto-reconnect (FE-1226)
runId+ last appliedseq. Classifiednetwork/protocolfailures, NodeAPI'sretryable: trueattachment-window errors, and transient gateway statuses (502/503/504) re-attach with exponential backoff (1s → 30s, capped at 8 consecutive failures) from the cursor; replayed events are de-duplicated byseq, so trials are never double-counted.sessionStorageso a reload can re-attach and rebuild from a full replay; runs the service no longer knows are dropped silently. On every give-up path the possibly-live run is cancelled so the account's single-flight frees up.Legacy cutover
@hashintel/petrinaut-core:PetrinautOptimizationdropsoptimize;createOptimizationRun/attachOptimizationRun/cancelOptimizationRunare required (minor bump via changeset).@hashintel/petrinaut: the provider's legacy stream fork is gone; the Storybook fake implements the detached contract.apps/hash-frontend: theoptimizationRequestbridge flow and its host handler are removed.apps/petrinaut-opt: the legacyPOST /optimize/all+POST /optimize/bestroutes and theirstream_all/stream_best/run_streamgenerators are deleted; OpenAPI + generated client types re-synced.@local/petrinaut-optimizer-client:openPetrinautOptimizationStreamis deleted; the decoder loses its study mode (syntheticstarted, best-so-far aggregation — the consumer's running best was already authoritative across cursors).apps/petrinaut-website: the optimization demo drives the detached-run API through the sharedopenapi-fetchclient via its dev proxy.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:
mainonce FE-1224, FE-1225: Add detached, reconnectable optimization runs to the optimizer and proxy them through NodeAPI #9067 squash-merges.sessionStorage); in-page reconnection is unaffected. It works in same-origin hosts (the website demo, non-sandboxed embedders of the published package), which is why the machinery is kept.🛡 What tests cover this?
@hashintel/petrinaut: 16 provider tests over the reconnect state machine (seq de-duplication across overlapping replays, backoff exhaustion + orphan cancel, retryable attachment-window errors, gateway retries, definitive mid-run 404s, stored-run recovery, StrictMode double-mounting).apps/hash-frontend: bridge unit + browser tests (create/attach/cancel round-trips, tombstoned late create replies self-cancelling orphaned runs, abort relay, mid-stream error classification with correlation ids).apps/petrinaut-opt/@local/petrinaut-optimizer-client: updated suites for the single-mode decoder and the removed legacy routes.❓ How to test this?
yarn devwith the optimizer running; start an optimization with ~100 trials.docker stats).