feat(engine): let a host name a run, and key approval reviews on it

[senamakel]

Stacked on #52 (base: hitl-approval). Merge that first.

Why

Review feedback on #52 established that deriving an approval node's
request_id from the trigger payload is unsafe — the trigger is caller-supplied,
so on a webhook an attacker could name a review colliding with an earlier run's
and inherit its cached decision. #52 removed that source. But that left the node
with no usable run identity: the engine's own run id is a process-local
counter minted inside every run call, and engine::resume re-executes the
workflow, so it differs between a run and its own resume. Feeding it to
request_id would open a second review on every resume — exactly what
create-or-fetch exists to prevent. And there was no public API to seed one, so
config.request_id was de facto mandatory while documented as optional.

Only the host knows an id that is both unique per run and survives a pause,
because only the host owns run persistence. So it supplies one.

What

RunInput::with_run_id(...) — seeds a durable, host-generated id into run
state as run.id:

let input = || RunInput::new(trigger.clone()).with_run_id("run-7f3a");
let paused = run(&compiled, input(), &caps).await?;      // review opens
let done   = resume(&compiled, input(), vec![], &caps).await?;  // SAME review

• Seeded outside run.trigger, so the de-duplication key is a slot the host
  fills deliberately rather than one a request can reach.
• Carried through merge_approvals, so a resume is the same run continuing and
  does not re-key everything derived from it.
• None (every caller predating this) leaves run.id unset, unchanged.

With it seeded, request_id goes back to genuinely optional: the default
"<run id>:<node id>" is unique per run and stable across resume.

E2E: a review that does not stop the graph

tests/approval_async_e2e.rs covers the case where the verdict is awaited
alongside other work rather than in front of it. A review in
wait_mode: "poll" sits on its own branch while a sibling
enrich → score → summarize chain advances through its own super-steps; a
merge absorbs both when the decision lands.

The assertion is about ordering, not just completion — a RunObserver
records finish order, and the test proves the review was already outstanding
when the work started and settled only after the work finished:

review, enrich, review, score, review, summarize, review, combine

It also asserts every poll addressed one review id (a polling loop that
re-registered would notify a person once per poll) and that the run never
paused. A second test covers a late rejection routing to its own branch without
disturbing the work branch.

Testing

cargo test --all-features = 1353 passed / 0 failed, cargo clippy --all-targets --all-features silent, cargo fmt clean. The hitl_review
example now uses with_run_id instead of a hardcoded request_id, and its
output shows one review row (run-7f3a:review) across both the run and the
resume.

[coderabbitai]

Warning

Review limit reached

@senamakel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bb1d09ab-cb59-45ed-9b30-073edf9ac2af

📥 Commits

Reviewing files that changed from the base of the PR and between 6294356 and 85153a0.

📒 Files selected for processing (8)

• CHANGELOG.md
• examples/hitl_review.rs
• src/catalog/contracts/group_03.rs
• src/engine.rs
• src/engine/run_state.rs
• src/nodes/integration/approval_tests/approval_tests_part_02_tests.rs
• tests/approval_async_e2e.rs
• wiki/Capability-Traits.md

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[tinysweeper]

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.1238 · 192,500 in / 53,500 out · 15,104 cached (8%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-pro-0813 · 710 embedded
critique:    $0.0631 · 90,469 in  / 31,305 out · 8,192 cached (9%)  · deepseek/deepseek-v4-pro-0813
security:    $0.0365 · 72,282 in  / 8,428 out  · 5,376 cached (7%)  · deepseek/deepseek-v4-pro-0813
tests:       $0.0144 · 18,828 in  / 7,471 out  · 768 cached (4%)    · deepseek/deepseek-v4-pro-0813
description: $0.0099 · 10,921 in  / 6,296 out  · 768 cached (7%)    · deepseek/deepseek-v4-pro-0813

[tinysweeper]

How this change flows

4 changed behaviours across 11 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 39 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["main<br/>changed"]:::changed
  n1["RunInput<br/>changed"]:::changed
  n2["resume<br/>changed"]:::changed
  n3["...sume_decision_withdraws_the_provider_card<br/>changed"]:::changed
  n4["Capabilities"]:::impacted
  n5["CompiledWorkflow"]:::impacted
  n6["build_and_run"]:::impacted
  n7["mock_capabilities"]:::impacted
  n8["RunObserver"]:::impacted
  n0 -->|uses| n4
  n2 -->|uses| n1
  n2 -->|uses| n4
  n2 -->|uses| n5
  n3 -->|uses| n4
  n3 -->|calls| n7
  n3 -->|tests| n7
  n6 -->|uses| n1
  n6 -->|uses| n4
  n6 -->|uses| n5
  n6 -->|uses| n8
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

Loading

_{Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.}

[tinysweeper]

Requesting changes: 3 lane(s) blocking, worst finding is critical.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.1397 · 208,735 in / 66,713 out · 21,120 cached (10%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-pro-0813 · 710 embedded
critique:    $0.0698 · 105,200 in / 33,176 out · 11,264 cached (11%) · deepseek/deepseek-v4-pro-0813
security:    $0.0445 · 73,410 in  / 17,641 out · 6,528 cached (9%)   · deepseek/deepseek-v4-pro-0813
tests:       $0.0129 · 19,016 in  / 6,189 out  · 1,664 cached (9%)   · deepseek/deepseek-v4-pro-0813
description: $0.0126 · 11,109 in  / 9,707 out  · 1,664 cached (15%)  · deepseek/deepseek-v4-pro-0813

[tinysweeper]

Requesting changes: 1 lane(s) blocking, worst finding is medium.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0334 · 41,719 in / 19,339 out · 3,584 cached (9%)  · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-pro-0813 · 706 embedded
critique:    $0.0058 · 11,345 in / 1,953 out  · 2,048 cached (18%) · deepseek/deepseek-v4-pro-0813
tests:       $0.0155 · 19,156 in / 8,580 out  · 768 cached (4%)    · deepseek/deepseek-v4-pro-0813
description: $0.0122 · 11,218 in / 8,806 out  · 768 cached (7%)    · deepseek/deepseek-v4-pro-0813

[tinysweeper]

Requesting changes: 2 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.1265 · 209,129 in / 60,144 out · 38,912 cached (19%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-pro-0813 · 706 embedded
critique:    $0.0624 · 104,831 in / 30,578 out · 22,784 cached (22%) · deepseek/deepseek-v4-pro-0813
security:    $0.0396 · 73,962 in  / 14,585 out · 12,288 cached (17%) · deepseek/deepseek-v4-pro-0813
tests:       $0.0156 · 19,137 in  / 9,367 out  · 1,920 cached (10%)  · deepseek/deepseek-v4-pro-0813
description: $0.0089 · 11,199 in  / 5,614 out  · 1,920 cached (17%)  · deepseek/deepseek-v4-pro-0813

[tinysweeper]

The previously-blocking findings are resolved. Clearing the changes request.

             $0.0971 · 142,993 in / 50,652 out · 21,248 cached (15%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-pro-0813 · 715 embedded
critique:    $0.0498 · 73,609 in  / 27,001 out · 13,184 cached (18%) · deepseek/deepseek-v4-pro-0813
security:    $0.0187 · 38,441 in  / 5,478 out  · 6,528 cached (17%)  · deepseek/deepseek-v4-pro-0813
tests:       $0.0185 · 19,236 in  / 11,970 out · 768 cached (4%)     · deepseek/deepseek-v4-pro-0813
description: $0.0102 · 11,707 in  / 6,203 out  · 768 cached (7%)     · deepseek/deepseek-v4-pro-0813

[senamakel]

PR Babysitter status

Head: 85153a0461cbb96b16ffdcdc4e276ac405d15c40 (base main, after merging upstream/main's PR #52 security fixes into this branch's run_id work)

CI: all green — Rust SDK, Chrome Extension, CodeRabbit (skipped, no findings), tinysweeper (tests/critique/security/description/review) all SUCCESS.

Mergeability: CLEAN / MERGEABLE.

Review threads: 0 unresolved. tinysweeper's overall verdict: APPROVED.

Work done this session:

• Merged base-branch change (hitl-approval → main, after feat(nodes): approval node kind + ApprovalProvider capability #52 merged) via the merge-conflict-resolver subagent: integrated main's approval-security fixes (self-approval-bypass fix, approval provenance, resume verdict naming, assignees validation) with this branch's RunInput::run_id/with_run_id work. Result: 1356 tests passing (up from 1352/1353 on either side alone).
• Investigated and rebutted ~14 tinysweeper findings across several review batches — all false positives, traced to two recurring misreadings:
  1. mock_capabilities() was read as having no ApprovalProvider; it actually defaults to MockApprovals::approving() (src/caps/mock_builders.rs), so the cited tests never pause.
  2. The wf() test helper (src/nodes/integration/approval_tests.rs) auto-fills config.request_id when absent, so tests using it were misread as exercising the "no request_id, no run id" refusal path when they don't; build_request (src/nodes/integration/approval_request.rs:61-73) does return an error in that case, confirmed by direct code read.
  3. A finish-order assertion in tests/approval_async_e2e.rs was flagged as a scheduler race; traced to be deterministic under the test's current_thread tokio flavor plus no real suspension point in either concurrent branch's first poll (join_all polls in declared-edge order) — confirmed with 80 consecutive stress runs, 0 flakes.
• Made small, legitimate clarity improvements (not concessions to false findings): reworded the RunInput::with_run_id wiki paragraph twice for precision, added an explicit in-test comment on two_runs_of_one_graph_get_distinct_reviews naming the default approving provider, and added an in-test comment documenting the finish-order determinism mechanism inline in tests/approval_async_e2e.rs.

Unresolved items: none.

Next action: hand off to pr-approval-reviewer.