Skip to content

FE-1224: Add detached, reconnectable optimization runs to the Petrinaut optimizer#9064

Open
kube wants to merge 4 commits into
mainfrom
cf/fe-1224-petrinaut-detached-runs
Open

FE-1224: Add detached, reconnectable optimization runs to the Petrinaut optimizer#9064
kube wants to merge 4 commits into
mainfrom
cf/fe-1224-petrinaut-detached-runs

Conversation

@kube

@kube kube commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

🌟 What is the purpose of this PR?

Decouples a Petrinaut optimization study's lifetime from the HTTP connection consuming it. A run is created detached and identified by a run_id; consumers attach, detach, and re-attach to its event stream with a cursor, and killing the socket no longer kills the study. This is the optimizer half of the fix for the intermittent staging network error on long optimization streams (a mid-run transport reset currently destroys the study); cancellation becomes an explicit DELETE instead of being implied by disconnect.

🔗 Related links

  • FE-1224 — this issue
  • FE-1223 — parent issue holding the full contract (NodeAPI and frontend halves follow)
  • SRE-831 — the suspected transport-reset root cause this architecture makes survivable

🚫 Blocked by

🔍 What does this change?

  • POST /optimize/runs — admits (existing 4-slot cap, 429 + Retry-After), starts the CLI + Optuna study in a background task owned by the run, returns 201 {"run_id"} immediately.
  • GET /optimize/runs/{run_id}/events?cursor=N (or Last-Event-ID) — SSE that replays buffered events with sequence > cursor (id: field carries the sequence, starting at 1), then live-tails with heartbeats. Disconnect does not affect the run; a new attachment supersedes a stale one; a terminal run replays fully then closes; 404 for unknown/expired runs.
  • DELETE /optimize/runs/{run_id} — idempotent explicit cancel: prompt CLI close, slot release, dedicated event: cancelled terminal frame (also used by the reaper and shutdown).
  • Slot lifetime = run lifetime; the per-run event log is bounded by the existing ≤1,000-trials manifest cap.
  • Orphan reaper: a run unattached for HASH_PETRINAUT_OPT_DETACH_GRACE_SECONDS (default 300; ≤0 disables) is cancelled; terminal run logs are retained for the same window for replay.
  • Legacy /optimize/all and /optimize/best keep their exact behavior (stream from start, cancel-on-disconnect) — their shared worker scaffolding was extracted and reused by the new engine rather than rewritten, and all 83 pre-existing tests pass unmodified.
  • README "Detached runs" section; OpenAPI spec regenerated with the three new routes.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • require changes to docs which are made as part of this PR

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

  • A run lives inside one optimizer process (the CLI is its child), so re-attachment must reach the same instance — fine at today's single replica; a persistent job architecture is the durable successor (out of scope, noted in FE-1223).
  • NodeAPI does not use these routes yet — that is FE-1225, stacked on this branch.

🐾 Next steps

🛡 What tests cover this?

apps/petrinaut-opt: 83 → 112 pytest. New coverage: create → 201 with the slot held until terminal and released exactly once; replay + live tail with strictly increasing id:s; cursor and Last-Event-ID re-attach (query wins); post-terminal full replay then clean close; disconnect not cancelling the run; attachment supersession; prompt idempotent DELETE (graceful=False close, terminal frame); reaper reaping unattached runs while sparing attached ones (and disabled at ≤0); retention expiry; pump-crash backstop with redacted logging; lifespan shutdown cancelling pumps; 404/429+Retry-After/413/500-with-run-id; OpenAPI shape for the three routes. The OpenAPI drift gate (yarn test:unit) is clean.

❓ How to test this?

  1. cd apps/petrinaut-opt && uv run pytest
  2. With a local optimizer running (see README), POST /optimize/runs with the checked-in supply-chain manifest, note the run_id.
  3. curl -N .../optimize/runs/<run_id>/events, kill the curl mid-run, re-run it with ?cursor=<last id seen> — confirm the study kept running and the stream resumes after the cursor.
  4. DELETE .../optimize/runs/<run_id> — confirm a prompt event: cancelled terminal frame and slot release.

🤖 Generated with Claude Code

@kube kube self-assigned this Jul 19, 2026
@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hash Ready Ready Preview, Comment Jul 19, 2026 9:17pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
hashdotdesign-tokens Ignored Ignored Preview Jul 19, 2026 9:17pm
petrinaut Skipped Skipped Jul 19, 2026 9:17pm

@github-actions github-actions Bot added area/apps > hash* Affects HASH (a `hash-*` app) area/infra Relates to version control, CI, CD or IaC (area) area/apps > hash-api Affects the HASH API (app) area/libs Relates to first-party libraries/crates/packages (area) type/eng > frontend Owned by the @frontend team type/eng > backend Owned by the @backend team area/tests New or updated tests area/apps labels Jul 19, 2026
if (response.destroyed || response.writableEnded) {
throw new Error("The optimization client disconnected");
}
if (response.write(`${JSON.stringify(event)}\n`)) {

@semgrep-code-hashintel semgrep-code-hashintel Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detected directly writing to a Response object from user-defined input. This bypasses any HTML escaping and may expose your application to a Cross-Site-scripting (XSS) vulnerability. Instead, use 'resp.render()' to render safely escaped HTML.

Fixed in commit 760a6f0

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.12500% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.24%. Comparing base (5c7a2d9) to head (760a6f0).

Files with missing lines Patch % Lines
...optimizer/create-petrinaut-optimization-handler.ts 78.12% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9064   +/-   ##
=======================================
  Coverage   59.23%   59.24%           
=======================================
  Files        1391     1391           
  Lines      135181   135208   +27     
  Branches     6235     6242    +7     
=======================================
+ Hits        80073    80100   +27     
+ Misses      54145    54143    -2     
- Partials      963      965    +2     
Flag Coverage Δ
apps.hash-api 10.43% <78.12%> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vercel
vercel Bot temporarily deployed to Preview – petrinaut July 19, 2026 19:46 Inactive
@kube
kube marked this pull request as ready for review July 19, 2026 20:07
@kube
kube requested a review from YannisZa July 19, 2026 20:08
@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches admission slots, long-running background studies, and optimization proxy error paths; changes are well-tested but affect core optimizer lifecycle and operational debugging contracts.

Overview
Adds detached optimization runs in petrinaut-opt: POST /optimize/runs starts a background study and returns run_id; GET .../events replays and live-tails SSE with id: sequences and cursor/Last-Event-ID; DELETE cancels with an event: cancelled frame. Studies outlive consumer disconnects; an orphan reaper and optional wall-clock ceiling bound unattended runs. Streaming /optimize/all and /optimize/best keep cancel-on-disconnect behavior but share refactored study workers via a new pump_events path.

Introduces JSON structured logging (logging_config.py) with request_id, trace_id, and run_id, plus bounded CLI stderr forwarding and PETRINAUT_CORRELATION_ID on spawned CLIs—manifests and raw user code stay out of logs.

hash-api gains request-scoped optimization logs (accept, stream open, outcomes, capacity/validation rejects), forwards x-hash-request-id upstream, surfaces X-Optimization-Run-ID to clients and failure responses, and caps validation issue path segments without logging validation message bodies.

Reviewed by Cursor Bugbot for commit 760a6f0. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fec71b8. Configure here.

Comment thread apps/petrinaut-opt/src/petrinaut_optimizer.py
YannisZa
YannisZa previously approved these changes Jul 19, 2026
kube added 4 commits July 19, 2026 23:02
…ization pipeline

One optimization is now traceable end to end: NodeAPI's x-hash-request-id
is forwarded upstream, the optimizer's run_id comes back as
X-Optimization-Run-ID and is logged on both sides, and the CLI receives
the run id at spawn as PETRINAUT_CORRELATION_ID.

- petrinaut-optimizer-client: openPetrinautOptimizationStream accepts a
  requestId (sent as x-hash-request-id) and returns { events,
  optimizationRunId } read from the X-Optimization-Run-ID header.
- hash-api: request-scoped child logger (Apollo pattern); logs request
  acceptance, validation failures (issue counts and schema paths only),
  capacity rejections, stream opening, and a terminal outcome with
  durationMs (completed / timeout kind / upstream-error /
  client-disconnected — disconnects no longer return silently).
- petrinaut-opt: stdlib JSON-lines logging to stdout (logging_config.py,
  HASH_PETRINAUT_OPT_LOG_LEVEL, idempotent setup); inbound
  x-hash-request-id and traceparent headers threaded into every study
  lifecycle log (admission, capacity rejection, initialization,
  study start/completion, per-trial outcomes, wall-clock ceiling,
  disconnect, worker join timeout, CLI termination path, slot release).
  CLI stderr is no longer discarded: each line is forwarded to a pn_cli
  logger, truncated with a bounded pending buffer.
- petrinaut-cli: minimal JSON stderr diagnostics after the unchanged
  ready handshake (bootstrap_completed, per-request id/method/durationMs/
  outcome, stdin_eof), correlation id injected via cli.ts.

Logs never include manifests, user-authored code, or raw request bodies;
docs updated (README logging section, CLI stderr contract, threat model).
- Cap forwarded CLI stderr at 1000 lines/run (drain the rest) so a chatty
  or hostile CLI cannot flood the log pipeline; seed the drain thread with
  bytes read past the bootstrap handshake so an early CLI diagnostic is
  forwarded rather than dropped.
- Truncate the per-trial pruned-error field and stop interpolating the raw
  CLI error into the log message; correct the docstring.
- Log an error type and coarse category instead of the raw initialization
  error string, which can embed a user identifier or expression error; the
  full message still reaches the requester in the 500 detail.
- Cap validation issue-path segments before they reach the 400 response or
  the log, since manifest keys are user-controlled and unbounded.
- Capture the optimizer run id from a failed response so a pre-stream
  upstream failure still correlates end to end.
- Charset-validate the traceparent trace id instead of int(..., 16), which
  accepted 0x-prefixed, underscored, and signed values.
- Document the aggregate stderr cap and header validation in the threat
  model.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/apps area/infra Relates to version control, CI, CD or IaC (area) area/libs Relates to first-party libraries/crates/packages (area) area/tests New or updated tests type/eng > backend Owned by the @backend team type/eng > frontend Owned by the @frontend team

Development

Successfully merging this pull request may close these issues.

3 participants