FE-1222, FE-1226: Classify optimization transport errors, auto-reconnect runs by id + cursor, and remove the legacy optimization path - #9066
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
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). |
PR SummaryHigh Risk Overview HASH frontend bridge: Host iframe messages split into Optimizations UI: The provider creates runs, tracks Optimizer service: Clients & docs: Website demo and optimizer client use detached-run APIs; docs describe reconnect vs reload/cancel behavior. Reviewed by Cursor Bugbot for commit bb41b6c. Bugbot is set up for automated code reviews on this repo. Configure here. |
b54c931 to
cf08515
Compare
198a2ce to
469d5b1
Compare
633185f to
74f438d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated no new comments.
Suppressed comments (3)
libs/@hashintel/petrinaut/src/react/optimizations/provider.tsx:198
aftercounts only successful trials. A run with 5 completed, 3 pruned, and 2 failed trials therefore reports “after 5 … trials” even though 10 trial events were applied (the UI progress elsewhere sums all three states). Include pruned and failed counts so the progress-aware transport error reflects the actual resume cursor/progress.
const after = `after ${progress.completedTrials} of ${progress.requestedTrials} trials`;
libs/@hashintel/petrinaut-core/src/optimization.ts:501
- This accepts
seq: 0, but the consumer initializes its cursor to 0 and skips every event withseq <= lastSeq(provider.tsx:496,525-529), so a schema-valid first event numbered 0 is silently discarded. The detached-run producer actually starts sequences at 1 (apps/petrinaut-opt/src/optimization_runs.py:102-104), and cursor 0 means “replay everything”; encode that contract by requiring a positive sequence number.
const optimizationEventSeqSchema = z.number().int().nonnegative().optional();
libs/@hashintel/petrinaut/docs/optimization.md:99
- This says every reload loses the run view, but the new provider restores stored runs from
sessionStorage(provider.tsx:730-776) in same-origin hosts; only the known opaque-origin HASH iframe degrades to this behavior. As written, the user guide contradicts the reload-recovery feature and advises same-origin users to cancel unnecessarily. Describe the conditional behavior and distinguish reload from closing the tab.
Reloading or closing the page is different: the page loses its view of a
still-running optimization. The run itself continues on the server until it
finishes or is cleaned up, and it can block you from starting a new
optimization until then — so use **Cancel** first if you intend to reload and
run something else. **Cancel** is also what actually stops a run: it ends the
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 74f438d. Configure here.
…onnect detached runs - Classify Petrinaut optimization transport failures in the UI so the user sees whether a run was rejected, dropped, or is retrying, instead of one opaque error. - Auto-reconnect detached optimization runs by run id and event `seq` cursor, re-attaching after connection drops and (where storage allows) page reloads; report an honest connection state from the new `onAttached` signal. - Make the detached contract the only one: the host capability is now `createOptimizationRun`/`attachOptimizationRun`/`cancelOptimizationRun`, and the legacy single-connection `optimize` method is removed from petrinaut-core. - Remove the legacy optimization path end to end: the optimizer's `/optimize/all` and `/optimize/best` routes and their `stream_all`/`stream_best` generators, the client's `openPetrinautOptimizationStream`, and the provider's legacy fork. - Migrate the website optimization demo and the SimulateView stories to the detached-run API, and update the Petrinaut optimization user docs.
74f438d to
af8d841
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
libs/@hashintel/petrinaut/src/react/optimizations/provider.tsx:199
- The displayed progress counts only successful trials. Pruned and failed trials are also received and retained, so a drop after 10 complete + 63 pruned trials is reported as “after 10 of 100” instead of 73. Include all three terminal trial counters when building the progress message.
progress: { completedTrials: number; requestedTrials: number },
): string {
const after = `after ${progress.completedTrials} of ${progress.requestedTrials} trials`;
apps/petrinaut-website/src/main/app/optimization-demo/petrinaut-opt-optimization.ts:70
- This labels every non-HTTP request error as
network, butattachPetrinautOptimizationRunStreamalso throws a plainErrorwhen a successful response has no body (attach-optimization-run.ts:88-89). That protocol violation is therefore retried and eventually shown as a connection interruption. As inclassifyStreamError, onlyTypeErrorshould be classified as network and other non-abort errors as protocol.
const classifyRequestError = (error: unknown): unknown =>
error instanceof PetrinautOptimizerHttpError
? classifyHttpError(error)
: error instanceof Error && error.name !== "AbortError"
? Object.assign(error, { category: "network" })
: error;
libs/@hashintel/petrinaut-core/src/optimization.ts:501
- The detached capability now depends on
seqfor both resume cursors and replay de-duplication, but this schema still makes it optional. A host can therefore satisfyPetrinautOptimization.attachOptimizationRunwhile yielding unsequenced trials;runAttachLoopthen keeps cursor 0 and appends the full replay again after every reconnect. Since the legacyoptimizecapability is removed, expose a detached-run event type/schema that requiresseqfor replayable run events (while separately allowing attachment-scoped timeout/superseded errors without one).
* replayed event at or below that cursor. Optional so streams from hosts that
* predate detached runs keep validating.
*/
const optimizationEventSeqSchema = z.number().int().nonnegative().optional();

🌟 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. This is a breaking change to that interface, released as apatch— the convention for these pre-v1 packages, which have no external consumers yet.@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).