fix(openhuman): give an embedded turn a trust label and the run's checkout - #251
Conversation
Adds the openhuman library as a vendored dependency to support upcoming functionality that relies on its APIs. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The OpenHuman provider now treats a missing run id as an error instead of silently proceeding, which prevents downstream failures when the provider returns an incomplete response. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The OpenHuman provider now treats a missing run id as an error instead of silently proceeding, preventing downstream failures when the provider returns an incomplete response. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The OpenHuman provider now treats a missing run id as an error instead of silently proceeding, which prevents downstream failures caused by an invalid or incomplete provider response. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The OpenHuman provider now treats a missing run id as an error instead of silently proceeding, which prevents downstream failures when the provider returns an incomplete response. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Refresh the vendored openhuman crate to its latest upstream revision, incorporating upstream fixes and improvements. No local code changes were required beyond the vendor update. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The OpenHuman provider now treats a missing run id as an error instead of panicking, ensuring graceful failure when the API returns an unexpected payload. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add tests for the turn workspace root helper, covering unset, missing, and non-directory paths to ensure they grant no root, and verifying that existing checkouts resolve to canonical absolute paths for containment checks. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
Next review available in: 41 minutes 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (41)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffa68b785e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # src/sdk/src/daemon/providers/openhuman/run.rs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f11259b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33eaf49eec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # src/sdk/src/daemon/providers/openhuman/run.rs # src/sdk/src/daemon/providers/openhuman/tests.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70da5749c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The runtime previously failed to signal the daemon to shut down when the last handle was dropped, leaving the daemon running indefinitely. This change restores the shutdown signal so the daemon exits cleanly once all runtime handles are released. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The task loop now checks for a shutdown signal before processing each iteration, ensuring the daemon exits promptly when a termination request is received rather than continuing to run until the next natural break point. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The task loop run module is no longer needed as its functionality has been superseded by the updated task scheduling logic. This change removes the dead code to keep the codebase clean and reduce maintenance overhead. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the task loop's receiver channel is closed, the loop now exits cleanly instead of panicking. This ensures graceful shutdown when the sender is dropped, preventing unnecessary error logs and improving daemon stability during normal termination. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the daemon binary path is not set, the embedded daemon now returns an error instead of panicking. This improves robustness by allowing callers to handle the missing configuration explicitly rather than crashing unexpectedly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the daemon starts, it now checks for the presence of a required entry field and logs a clear error if it is missing, preventing a silent failure or panic later in the startup sequence. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The daemon entry point previously dropped the shutdown signal handler, causing the process to exit immediately instead of waiting for a termination signal. This change retains the handler so the daemon runs until explicitly stopped. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The worker loop now gracefully handles the case where a file being watched is deleted before the watcher processes it, preventing a panic from an unwrap on a missing entry. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds test coverage for the origin module in the SDK daemon, verifying that origin-related functionality behaves as expected. This ensures the daemon's origin handling is properly validated and guarded against regressions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The origin tests were previously removed during a refactor, and this change restores them to ensure the daemon's origin handling behavior is properly verified. The tests cover the expected origin validation and rejection paths. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test module declaration in the daemon tests file was updated to use the correct module path, ensuring that the tests are properly discovered and executed by the test runner. This fixes a broken test configuration that previously prevented the daemon tests from running. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The origin tests were previously removed during a refactor, and this change restores them to ensure the origin resolution logic remains verified. The tests cover the expected behavior of origin handling in the daemon. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The origin test that verifies the daemon's absence was previously removed, and this change restores it to ensure the expected behavior is covered again. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted several multi-line function calls and import statements to fit on single lines, and fixed a missing trailing newline in a test file. These changes are purely cosmetic and do not affect any runtime behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.0099 · 148,204 in / 27,514 out · 115,077 cached (78%) · z-ai/glm-5.2
critique: $0.0029 · 33,401 in / 6,509 out · 14,598 cached (44%) · z-ai/glm-5.2
security: $0.0044 · 79,277 in / 12,628 out · 69,568 cached (88%) · z-ai/glm-5.2
tests: $0.0014 · 17,536 in / 4,815 out · 15,080 cached (86%) · z-ai/glm-5.2
description: $0.0011 · 17,990 in / 3,562 out · 15,831 cached (88%) · z-ai/glm-5.2
tinysweeper
Bot
added
priority: p1
The test that verifies provider creation fails when the daemon is absent was previously removed. This change restores it to ensure the error path is covered again. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Corrected a misspelled word in a comment within the hooks test file to improve readability and maintain consistency with the codebase's documentation standards. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
The previously-blocking findings are resolved. Clearing the changes request.
$0.0030 · 55,967 in / 8,704 out · 49,611 cached (89%) · z-ai/glm-5.2
critique: $0.0007 · 17,542 in / 1,608 out · 15,787 cached (90%) · z-ai/glm-5.2
security: $0.0003 · 2,129 in / 1,200 out · 1,894 cached (89%) · z-ai/glm-5.2
tests: $0.0008 · 17,921 in / 2,016 out · 16,128 cached (90%) · z-ai/glm-5.2
description: $0.0012 · 18,375 in / 3,880 out · 15,802 cached (86%) · z-ai/glm-5.2
1 participant
Makes a workflow
agentnode dispatched to the embedded OpenHuman harness able todo real work in the checkout the run names. Before this, a probe workflow proved the
turn was useless for coding: every write into the run's checkout was refused, and every
external-effect tool a delegated coding sub-agent tried was refused as an unlabelled
call site.
Pairs with tinyhumansai/openhuman#5452, which this PR's
vendor/openhumangitlinkmoves to. That PR carries the core half and the full root-cause writeup; land it first.
Behaviour changes
run_openhuman_tasknow scopes two things around itsinvoke("openhuman.inference_agent_chat", …)— the core'sCoreRuntime::invokepolls thehandler on the calling task, so the whole turn runs inside them:
TrustedAutomation { source: Workflow { require_approval: false }, job_id: <thread id> }. Previously the core stamped its own blanketClilabel onthis RPC.
RunTaskOptions::cwdwas destructured away and never reached the turn. Itis now resolved to a canonical directory and scoped as the turn's workspace, which makes it
both the default cwd for acting tools and a read/write root for the core's path policy —
so
file_writeinto the run's checkout is no longer refused withResolved parent path escapes workspace: …. An empty, missing, or non-directorycwdscopes nothing and leaves the turn on the core's own workspace, as today.
Also corrects stale docs: the module and
run.rsclaimed Medulla hooks are absent forOpenHuman and logged a warning saying so. That has been untrue since #246 added
core_host::hooks, which registersPreToolUse/PostToolUse/Stopon the core at boot —auto-commitreally does fire during an embedded turn. The warning is removed and the moduledoc now states what is actually true, including the one real difference (hooks are
process-global here, not per-dispatch).
How workflow authority reaches the turn
The
originabove is keyed off a newworkflowNode: truefield on the task-frameprotocol, which the daemon's workflow host sets when it dispatches a graph's
agentnodeover its in-process loopback bridge (
TaskRunnerDispatch→EmbeddedDaemon). The fieldalone is caller-controlled JSON, so the daemon never trusts it from a remote peer:
each transport that delivers a
taskframe computes its ownis_device_local(from)verdict, and that verdict is what may upgrade the frame to
RunTaskOrigin::Workflow—A forged
workflowNode: truefrom any authenticated peer over the host link is thereforeread as ordinary delegated work, so the
TrustedAutomation::Workflowlabel — and therequire_approval: falseit carries — is only ever minted by daemon-local dispatch theoperator's own workflow store drove. The loopback workflow host is the sole legitimate
producer and it resolves device-local, so unattended workflow nodes keep their authority.
Policy / security implication
Labelling these turns
TrustedAutomation::Workflow { require_approval: false }means theapproval gate auto-allows external-effect tools for them instead of parking for a human
decision. This is the same trust root a user-authored cron job's prompt carries, and the same
one a saved tinyflows workflow already gets: the graph was authored and saved by the operator
before it ran, so the action is pre-declared. Parking instead would strand an unattended run
on a prompt nobody is watching. The turn's own id rides on
job_id, so an audit row or aparked approval names the dispatch it came from. Because the label can only be minted by
device-local dispatch (see above), this trust root is not reachable by any peer that can
authenticate to a remote host.
The workspace grant is deliberately no stronger than a user-configured
TrustedRoot { access: ReadWrite }— credential stores, workspace-internal state, and thecross-profile write guard are all unaffected. See the openhuman PR for the detail.
Two origins are explicitly not workflow-labelled:
task_loop/workflow/handle.rs). An auto-on-failurereview turn runs proposal tools against a failed run; it is not an authored workflow
node, so its
RuntimeDispatchis pinned toRunTaskOrigin::DelegatedTaskand neverinherits the trust label.
sender_device_localgate above mayproduce
Workflow, and a host-link peer is never device-local.Hook execution semantics
While correcting the docs, the hook runner also changed one behaviour: a hook whose
stdin payload cannot be written (the command closed its stdin, or exited first) is now
captured in the wait result and only surfaced when the hook is enforced. Before, a
write error short-circuited the async block and failed the call even for observational
stop hooks. A pre-hook (
enforce_status = true) that never received its payload stillvetoes the tool call; a stop hook (
enforce_status = false) absorbs the write failure,because that race — the hook closing stdin before the payload lands — must not fail the
turn.
Validation
From this worktree:
cargo fmt --check— cleancargo clippy --all-targets -- -D warnings— cleancargo test— all suites pass (2620 + 1062 + … , 0 failed)And in
vendor/openhuman(see #5452 for the full list):cargo fmt --check,cargo clippy --lib -- -D warnings, and theturn_origin/turn_workspace/turn_root/effective_origin/security::policy/orchestration/session::buildersuites.New unit tests cover: the
cwdresolution branches (unset, missing, a file, and anon-canonical path that must resolve to the canonical root); the
workflowNodeorigingate (a forged marker from a remote sender is demoted to
DelegatedTask, a device-localsender keeps
Workflow, and a device-local sender without the marker staysDelegatedTask); and the hook stdin-write race (an enforced hook whose payload cannotbe written vetoes, while an observational stop hook absorbs the write failure).