feat(rlm): language-based workflows — Rhai .ragsh REPL as a first-class rlm tool#4528
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThis PR adds a new "rlm" (language workflows) tool that exposes TinyAgents' Rhai-backed ChangesRLM Language Workflow Feature
Unrelated Test String-Construction Cleanups
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant RlmTool
participant ops as "ops::eval_rlm_cell"
participant RlmSessionManager
participant CapabilityRegistry as "bridge::CapabilityRegistry"
participant ApprovalGate
Orchestrator->>RlmTool: execute(script, session_id, limits)
RlmTool->>ops: eval_rlm_cell(request)
ops->>ops: resolve_policy(autonomy tier)
ops->>CapabilityRegistry: build_capability_registry(parent)
ops->>RlmSessionManager: get_or_create(session key)
RlmSessionManager-->>ops: SlotHandle (session, cancel flag)
ops->>CapabilityRegistry: eval_cell(script) on blocking thread
CapabilityRegistry->>ApprovalGate: gated_execute(tool call)
ApprovalGate-->>CapabilityRegistry: allow/deny outcome
CapabilityRegistry-->>ops: ReplResult (stdout, value, calls)
ops->>RlmSessionManager: finish_cell(result)
RlmSessionManager-->>ops: CellStats
ops-->>RlmTool: RlmEvalResponse
RlmTool-->>Orchestrator: ToolResult (JSON)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 6587d0c9e9
ℹ️ 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".
| }; | ||
| } | ||
| GateOutcome::Allow => { | ||
| let result = execute_openhuman_tool(tool, call, context).await; |
There was a problem hiding this comment.
When a .ragsh cell calls an inner tool, this bridge invokes execute_openhuman_tool directly and only recreates the external-effect approval check. The normal harness also runs ToolPolicyMiddleware::channel_permission_block, which rechecks permission_level_with_args for the actual arguments; visible multi-action tools such as scheduling or accessibility actions can have read-only list/status operations but mutating actions that require Execute. In a limited/read-only channel where such a tool remains visible, an RLM script can call the mutating action and bypass that channel ceiling, so these calls need to go through the same policy path or duplicate the per-call permission check before execution.
Useful? React with 👍 / 👎.
| let handle = manager.get_or_create(&key, move || { | ||
| let capabilities = ReplCapabilities::new(Arc::new(registry)); | ||
| ReplSession::<()>::new() | ||
| .with_capabilities(capabilities) | ||
| .with_policy(policy_for_build) |
There was a problem hiding this comment.
For an existing session_id, get_or_create returns the stored ReplSession and does not run this builder, so the newly resolved CapabilityRegistry and ReplPolicy are ignored after the first cell. If the user's autonomy/channel permissions, visible tools, allowed subagents, model, or limit overrides change and the orchestrator reuses the encouraged same session_id, the session keeps the old tool surface and limits; for example a session opened before permissions were lowered can still call capabilities that are no longer visible. Recreate or update the session when the policy/capability fingerprint differs.
Useful? React with 👍 / 👎.
| tokio::spawn(async move { | ||
| token.cancelled().await; | ||
| tracing::debug!("[rlm] run cancellation observed — tripping cell cancel flag"); | ||
| flag.cancel(); | ||
| }); |
There was a problem hiding this comment.
Each successful RLM cell spawns a task that owns a clone of the run cancellation token and waits on token.cancelled() forever. Normal turns do not cancel this token when they finish, and because the spawned task holds a clone, it is not dropped either, so every non-cancelled RLM cell leaves behind a sleeping task until process exit. Tie the watcher to the cell lifetime, abort it after eval_cell returns, or race it with a completion signal.
Useful? React with 👍 / 👎.
senamakel
force-pushed
the
feat/rlm-language-workflows
branch
from
6587d0c to
eb3d5cf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb3d5cf6b3
ℹ️ 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".
| .unwrap_or(Duration::from_secs(DEFAULT_RLM_TIMEOUT_SECS)) | ||
| + OUTER_TIMEOUT_GRACE; | ||
|
|
||
| let join = tokio::task::spawn_blocking(move || { |
There was a problem hiding this comment.
When a .ragsh cell reaches an effectful tool_call or sub-agent action, it runs from this spawn_blocking closure, but Tokio task-locals such as AgentTurnOrigin and APPROVAL_CHAT_CONTEXT do not propagate across that boundary; the bridge only re-installs PARENT_CONTEXT for subagents. In a normal web-chat turn, ApprovalGate::intercept_audited therefore sees Unknown/no thread for non-auto-approved external-effect tools, so the action is denied or the approval is published without a routable thread card. Capture and re-scope the turn approval/origin contexts for RLM capability calls before evaluating the cell.
Useful? React with 👍 / 👎.
| model: impl Into<String>, | ||
| temperature: f64, | ||
| ) -> Arc<dyn ChatModel<()>> { | ||
| Arc::new(ProviderModel::new(provider, model, temperature)) |
There was a problem hiding this comment.
This factory registers the RLM model_query model with a fresh ProviderModel::new(...), whose default usage_carry is private and has no OpenhumanEventBridge consumer. The normal harness shares a ProviderUsageCarry with the event bridge before registering the model so charged USD/context-window usage is recorded; here, each successful model_query can call the backend while only accumulating usage in an unobserved queue, under-reporting cost/usage for RLM workflows. Thread the parent turn's usage/event bridge through this registration or explicitly record the provider usage from these calls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a77d7b8985
ℹ️ 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".
| let outcome = with_parent_context( | ||
| self.parent.clone(), | ||
| run_subagent(definition, &input.prompt, options), | ||
| ) |
There was a problem hiding this comment.
When an RLM cell is run from an already nested sub-agent, eval_cell is executing across the spawn_blocking boundary, so the CURRENT_SPAWN_DEPTH task-local is no longer in scope. This call only restores ParentExecutionContext before run_subagent; the runner's pre-dispatch check reads current_spawn_depth() and sees 0, allowing a sub-agent to call rlm → agent_query and continue spawning past MAX_SPAWN_DEPTH. Capture the current depth before the blocking eval and re-scope it with with_spawn_depth around this run_subagent call.
Useful? React with 👍 / 👎.
| .min_by_key(|(_, slot)| slot.last_access) | ||
| .map(|(k, _)| k.clone()) | ||
| { | ||
| map.remove(&lru_key); |
There was a problem hiding this comment.
When more than MAX_SESSIONS RLM sessions are opened while a long-running cell is still evaluating, this removes the least-recently-used slot even if that session's mutex is currently held by the in-flight cell. The cell keeps running via its cloned Arc, but finish_cell later cannot find the slot, so the successful response reports default/zero usage and the returned session_id resumes as a fresh namespace instead of preserving bindings. Avoid evicting locked/running sessions or defer eviction until they finish.
Useful? React with 👍 / 👎.
senamakel
force-pushed
the
feat/rlm-language-workflows
branch
from
a77d7b8 to
e8dde00
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8dde00d1f
ℹ️ 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".
|
|
||
| When a task needs **ad-hoc control flow** over delegated work — loops, conditionals, a | ||
| dedup-then-verify pipeline, "spawn N, filter, then verify each survivor with M checks" — that | ||
| the fixed `spawn_parallel_agents` / `delegate_*` primitives can't express, use the `rlm` tool. |
There was a problem hiding this comment.
This prompt teaches the orchestrator to call rlm, but the orchestrator uses a ToolScope::Named allowlist from src/openhuman/agent_registry/agents/orchestrator/agent.toml, and that list does not include rlm. The session builder filters prompt/provider tools through that non-empty allowlist before applying policy, so in normal chat sessions the model is instructed to use a tool that is not actually advertised or callable; add rlm to the orchestrator's named tools (or otherwise surface it through the scope) with this prompt change.
Useful? React with 👍 / 👎.
| crate::openhuman::tool_timeout::MAX_TIMEOUT_SECS, | ||
| ) | ||
| .unwrap_or(DEFAULT_RLM_TIMEOUT_SECS); | ||
| ToolTimeout::Secs(secs) |
There was a problem hiding this comment.
Returning the same timeout_secs here that resolve_policy installs as the REPL policy means the normal tool executor's tokio::time::timeout(tool.timeout_policy(...)) can fire before run_cell reaches its own policy.timeout + 5s backstop and cleanup path. For a cell or capability call that reaches the deadline, the rlm future can be dropped with a generic tool timeout while the spawn_blocking eval continues holding the session, so the fail-closed session drop in the RLM outer timeout never runs; make the tool-level timeout longer than the inner RLM timeout/backstop (or lower the inner timeout) so RLM owns cleanup.
Useful? React with 👍 / 👎.
senamakel
force-pushed
the
feat/rlm-language-workflows
branch
from
e8dde00 to
5181275
Compare
coderabbitai
Bot
added
agent
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5181275f72
ℹ️ 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".
| // No global gate installed (e.g. gate disabled): fall through and | ||
| // execute — the harness-level env kill-switch owns that decision. | ||
| } | ||
| execute_openhuman_tool(tool, call, context).await |
There was a problem hiding this comment.
The normal harness installs CredentialScrubMiddleware as the innermost tool wrapper before transcripts or tool outcomes can see raw output (src/openhuman/tinyagents/mod.rs:1748-1754), but this RLM path returns execute_openhuman_tool directly. In any agent where rlm and read tools are visible, such as the wildcard tools_agent with file_read, a cell can read a .env/API key and print or return it to the model unredacted; run the same credential scrub on bridged TaToolResult values or route these calls through the middleware chain.
Useful? React with 👍 / 👎.
| model: impl Into<String>, | ||
| temperature: f64, | ||
| ) -> Arc<dyn ChatModel<()>> { | ||
| Arc::new(ProviderModel::new(provider, model, temperature)) |
There was a problem hiding this comment.
The normal TinyAgents path applies with_max_tokens(max_output_tokens) before registering the provider model (src/openhuman/tinyagents/mod.rs:1183-1190, whose comment notes uncapped output was a parity bug), but this RLM factory registers a bare ProviderModel::new, leaving ChatRequest.max_tokens as None. A .ragsh cell that calls model_query for a long completion can bypass the per-turn/sub-agent output budget and incur much larger provider responses before the RLM result is handled; thread the parent output cap into this factory.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/openhuman/rlm/sessions.rs (1)
18-20: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftGlobal session cap is shared across every thread, not per-thread/user.
MAX_SESSIONS = 16bounds a single process-wideRlmSessionManager::global()used by all concurrent chat threads (keyed only by<thread_id>:<session_id>). A thread that opens several rlm sessions can LRU-evict another, unrelated thread's live session, silently dropping that thread's persistent Rhai namespace mid-conversation. Worth confirming whether 16 concurrent sessions across the whole app is expected to be sufficient, or whether the bound should be partitioned per thread/workspace instead of shared globally.Also applies to: 63-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/rlm/sessions.rs` around lines 18 - 20, The session limit is currently enforced globally through RlmSessionManager::global and the MAX_SESSIONS constant, so unrelated threads can evict each other’s live sessions. Review the session storage and eviction logic around the global manager and the keying scheme used for <thread_id>:<session_id>, then change the cap to be scoped per thread/workspace or otherwise prevent cross-thread LRU eviction. Update the related logic in the session manager so the limit is applied to the intended ownership boundary, not the whole process.src/openhuman/rlm/policy.rs (1)
31-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTie
RLM_MAX_DEPTHto the harness ceiling instead of hard-coding3. This literal can drift fromcrate::openhuman::agent::harness::MAX_SPAWN_DEPTHand silently change the sub-agent recursion bound.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/rlm/policy.rs` around lines 31 - 34, `RLM_MAX_DEPTH` is hard-coded and can drift from the harness recursion ceiling. Update the `policy.rs` constant so it derives from `crate::openhuman::agent::harness::MAX_SPAWN_DEPTH` instead of a literal, keeping the `RLM_MAX_DEPTH` and `MAX_SPAWN_DEPTH` bounds aligned in one place.src/openhuman/rlm/types.rs (1)
15-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
RlmSessionIdis currently unused
src/openhuman/rlm/types.rs:15-29only defines the newtype; the session/request path still uses plainString, so this looks like dead code. Remove it or wire it intoRlmEvalRequestand the session-handling flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/rlm/types.rs` around lines 15 - 29, RlmSessionId is only defined in types.rs and not actually used, so either remove the dead newtype or wire it through the session/request flow. Update RlmEvalRequest and the related session-handling code to use RlmSessionId instead of plain String, and make sure any parsing, serialization, and Display/as_str usage is consistent across the affected types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/rlm/mod.rs`:
- Around line 1-11: The module doc comment in mod.rs has a broken README link
that currently points to the repo root instead of the module’s README. Update
the markdown link in the top-level documentation comment so the `README.md`
reference resolves to the actual module README, keeping the existing
`RlmTool`/`mod.rs` docs consistent with the parenthetical mention of the module
README.
In `@src/openhuman/rlm/ops.rs`:
- Around line 208-215: The cancellation watcher spawned in run_cell is detached
and can outlive the cell, leaving extra task and cell_flag clones behind. Update
the run_cell flow around current_run_cancellation and tokio::spawn to keep the
JoinHandle or use an abort-on-drop guard, and ensure the watcher is aborted on
every exit path from run_cell so it cannot keep waiting on token.cancelled()
after the cell finishes.
In `@vendor/tinyagents`:
- Line 1: The submodule reference for tinyagents is pinned to an unstable
pull-request head commit, which can break clean checkouts if that ref changes.
Update the vendor/tinyagents submodule to point at a merged or tagged commit
instead of refs/pull/19/head. Make sure the new reference is a stable, permanent
revision so the checkout remains reproducible.
---
Nitpick comments:
In `@src/openhuman/rlm/policy.rs`:
- Around line 31-34: `RLM_MAX_DEPTH` is hard-coded and can drift from the
harness recursion ceiling. Update the `policy.rs` constant so it derives from
`crate::openhuman::agent::harness::MAX_SPAWN_DEPTH` instead of a literal,
keeping the `RLM_MAX_DEPTH` and `MAX_SPAWN_DEPTH` bounds aligned in one place.
In `@src/openhuman/rlm/sessions.rs`:
- Around line 18-20: The session limit is currently enforced globally through
RlmSessionManager::global and the MAX_SESSIONS constant, so unrelated threads
can evict each other’s live sessions. Review the session storage and eviction
logic around the global manager and the keying scheme used for
<thread_id>:<session_id>, then change the cap to be scoped per thread/workspace
or otherwise prevent cross-thread LRU eviction. Update the related logic in the
session manager so the limit is applied to the intended ownership boundary, not
the whole process.
In `@src/openhuman/rlm/types.rs`:
- Around line 15-29: RlmSessionId is only defined in types.rs and not actually
used, so either remove the dead newtype or wire it through the session/request
flow. Update RlmEvalRequest and the related session-handling code to use
RlmSessionId instead of plain String, and make sure any parsing, serialization,
and Display/as_str usage is consistent across the affected types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f6b5ffb-6739-4213-9a4f-19d44a808da2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
Cargo.tomlgitbooks/developing/architecture/agent-harness.mdsrc/openhuman/about_app/catalog_data.rssrc/openhuman/agent_orchestration/tools/spawn_subagent.rssrc/openhuman/agent_registry/agents/orchestrator/prompt.mdsrc/openhuman/mod.rssrc/openhuman/rlm/README.mdsrc/openhuman/rlm/bridge.rssrc/openhuman/rlm/mod.rssrc/openhuman/rlm/ops.rssrc/openhuman/rlm/policy.rssrc/openhuman/rlm/sessions.rssrc/openhuman/rlm/tools.rssrc/openhuman/rlm/types.rssrc/openhuman/tinyagents/mod.rssrc/openhuman/tinyagents/model.rssrc/openhuman/tinyagents/tools.rssrc/openhuman/tokenjuice/reduce_tests.rssrc/openhuman/tokenjuice/text/process.rssrc/openhuman/tools/mod.rssrc/openhuman/tools/ops.rsvendor/tinyagents
| //! `rlm` — language-based workflows over the TinyAgents Rhai `.ragsh` REPL. | ||
| //! | ||
| //! Exposes `tinyagents::ReplSession` (the `repl` feature) as a first-class | ||
| //! [`RlmTool`], so the orchestrator can author and run its own workflow scripts | ||
| //! — fan-out, batched calls, loops, dedup/verify pipelines — bounded and | ||
| //! fail-closed. See [`README.md`](https://github.com/tinyhumansai/openhuman) | ||
| //! (module `README.md`) for the design. | ||
| //! | ||
| //! Module shape (canonical): `mod.rs` exports only; logic lives in the | ||
| //! siblings. No controller schemas in v1 (`scope() = AgentOnly`). | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Broken README link.
The doc-link points to the repo root, not the actual module README.md (src/openhuman/rlm/README.md), which is confusing given the parenthetical "(module README.md)".
📝 Proposed fix
-//! fail-closed. See [`README.md`](https://github.com/tinyhumansai/openhuman)
-//! (module `README.md`) for the design.
+//! fail-closed. See the module's
+//! [`README.md`](https://github.com/tinyhumansai/openhuman/blob/main/src/openhuman/rlm/README.md)
+//! for the design.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! `rlm` — language-based workflows over the TinyAgents Rhai `.ragsh` REPL. | |
| //! | |
| //! Exposes `tinyagents::ReplSession` (the `repl` feature) as a first-class | |
| //! [`RlmTool`], so the orchestrator can author and run its own workflow scripts | |
| //! — fan-out, batched calls, loops, dedup/verify pipelines — bounded and | |
| //! fail-closed. See [`README.md`](https://github.com/tinyhumansai/openhuman) | |
| //! (module `README.md`) for the design. | |
| //! | |
| //! Module shape (canonical): `mod.rs` exports only; logic lives in the | |
| //! siblings. No controller schemas in v1 (`scope() = AgentOnly`). | |
| //! `rlm` — language-based workflows over the TinyAgents Rhai `.ragsh` REPL. | |
| //! | |
| //! Exposes `tinyagents::ReplSession` (the `repl` feature) as a first-class | |
| //! [`RlmTool`], so the orchestrator can author and run its own workflow scripts | |
| //! — fan-out, batched calls, loops, dedup/verify pipelines — bounded and | |
| //! fail-closed. See the module's | |
| //! [`README.md`](https://github.com/tinyhumansai/openhuman/blob/main/src/openhuman/rlm/README.md) | |
| //! for the design. | |
| //! | |
| //! Module shape (canonical): `mod.rs` exports only; logic lives in the | |
| //! siblings. No controller schemas in v1 (`scope() = AgentOnly`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/rlm/mod.rs` around lines 1 - 11, The module doc comment in
mod.rs has a broken README link that currently points to the repo root instead
of the module’s README. Update the markdown link in the top-level documentation
comment so the `README.md` reference resolves to the actual module README,
keeping the existing `RlmTool`/`mod.rs` docs consistent with the parenthetical
mention of the module README.
| if let Some(token) = current_run_cancellation() { | ||
| let flag = cell_flag.clone(); | ||
| tokio::spawn(async move { | ||
| token.cancelled().await; | ||
| tracing::debug!("[rlm] run cancellation observed — tripping cell cancel flag"); | ||
| flag.cancel(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C4 'fn current_run_cancellation' src/openhuman/tinyagents/run_cancellation_context.rs
ast-grep outline src/openhuman/tinyagents/run_cancellation_context.rs --items allRepository: tinyhumansai/openhuman
Length of output: 759
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== run_cancellation_context =="
cat -n src/openhuman/tinyagents/run_cancellation_context.rs
echo
echo "== search for current_run_cancellation usage =="
rg -n "current_run_cancellation|with_run_cancellation|CancellationToken|cancelled\(" src/openhuman -S
echo
echo "== inspect rlm ops around watcher =="
sed -n '170,260p' src/openhuman/rlm/ops.rsRepository: tinyhumansai/openhuman
Length of output: 16618
Cancel the watcher when run_cell exits. The spawned task is detached and will keep waiting on token.cancelled() for the rest of the run, so each completed cell leaves one extra task + cell_flag clone around until the run token is cancelled. Store the JoinHandle and abort it on all exit paths (an abort-on-drop guard works well).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/rlm/ops.rs` around lines 208 - 215, The cancellation watcher
spawned in run_cell is detached and can outlive the cell, leaving extra task and
cell_flag clones behind. Update the run_cell flow around
current_run_cancellation and tokio::spawn to keep the JoinHandle or use an
abort-on-drop guard, and ensure the watcher is aborted on every exit path from
run_cell so it cannot keep waiting on token.cancelled() after the cell finishes.
| @@ -1 +1 @@ | |||
| Subproject commit 357bcc85871b40c03a7dd81b61d1b3253bfb33e6 | |||
| Subproject commit df391c46891cb69ffd39f69023f497a5d95bec0e | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the submodule is pinned to the expected commit and inspect its remote/ref config.
set -e
echo "=== .gitmodules ==="
cat .gitmodules 2>/dev/null || echo "no .gitmodules"
echo "=== recorded submodule commit ==="
git ls-tree HEAD vendor/tinyagents
echo "=== submodule status ==="
git submodule status vendor/tinyagents 2>/dev/null || echo "submodule not initialized"Repository: tinyhumansai/openhuman
Length of output: 1034
🏁 Script executed:
#!/bin/bash
# Resolve the tinyagents remote URL from .gitmodules, then check where the commit lives.
url=$(git config -f .gitmodules --get submodule.vendor/tinyagents.url 2>/dev/null \
|| git config -f .gitmodules --get-regexp 'submodule\..*\.url' | awk '/tinyagents/{print $2; exit}')
echo "remote url: $url"
commit=df391c46891cb69ffd39f69023f497a5d95bec0e
git ls-remote "$url" | grep -i "$commit" || echo "Commit $commit not found on any advertised ref (branch/tag) of $url"Repository: tinyhumansai/openhuman
Length of output: 274
Avoid pinning vendor/tinyagents to a PR head
df391c46891cb69ffd39f69023f497a5d95bec0e is only advertised as refs/pull/19/head, not a stable branch or tag. Pinning the submodule here makes clean checkouts fragile if that PR ref is rebased or removed; use a merged/tagged commit instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/tinyagents` at line 1, The submodule reference for tinyagents is
pinned to an unstable pull-request head commit, which can break clean checkouts
if that ref changes. Update the vendor/tinyagents submodule to point at a merged
or tagged commit instead of refs/pull/19/head. Make sure the new reference is a
stable, permanent revision so the checkout remains reproducible.
Adds the embedded Rhai `.ragsh` session runtime (`tinyagents::ReplSession`) that the new `rlm` language-workflow tool builds on.
…on for RLM Makes the seams the `rlm` capability bridge reuses `pub(crate)`: - `mod tools`/`mod model`/`mod run_cancellation_context` now `pub(crate)`; - `execute_openhuman_tool` + `tool_policy_from_openhuman_tool` `pub(crate)`; - new `provider_chat_model(provider, model, temperature)` factory over the `pub(super)` `ProviderModel::new`. No behavior change to the existing harness path.
New `src/openhuman/rlm/` domain exposing tinyagents' Rhai session as bounded, fail-closed capability-driven workflow cells: - `policy.rs`: autonomy tier + tool-timeout clamps -> `ReplPolicy` (readonly refused; full may raise limits to a 2x ceiling; timeout always bounded). - `bridge.rs`: builds the `CapabilityRegistry` — approval-gated tool adapters (the repl path bypasses the harness `wrap_tool` middleware, so the bridge invokes `ApprovalGate::intercept_audited` itself for external-effect tools), provider model, and a sub-agent capability per `allowed_subagent_ids` that re-installs `PARENT_CONTEXT` (lost across spawn_blocking + block_on). Excludes `rlm`/`spawn_*`/workflow and `CliRpcOnly` tools. - `sessions.rs`: LRU + idle-TTL bounded manager, one cell at a time (typed "session busy" on contention). - `ops.rs`: `eval_rlm_cell` — spawn_blocking + layered `tokio::timeout` backstop, run-cancellation -> per-cell `ReplCancelFlag`, and a model-consumable error taxonomy. Verbose `[rlm]`-prefixed debug logging throughout. Unit tests co-located.
- Register `RlmTool` in `all_tools_with_runtime`, gated off the read-only autonomy tier and behind the `OPENHUMAN_RLM=0` kill switch (default-on for supervised/full). - Orchestrator `prompt.md`: "Language workflows (rlm)" section — when to prefer it over spawn_parallel_agents, one-cell-per-call, session reuse. - `about_app` capability catalog entry; agent-harness gitbook RLM section.
Points `vendor/tinyagents` at the `feat/repl-host-embedding` head (df391c4): external `ReplCancelFlag` cancellation, live `ReplCall` events, and `set_cancel_flag`. Depends on tinyhumansai/tinyagents#19; re-pin to the merged commit once that lands.
…o the graph
Enabling the tinyagents `repl` feature adds `rhai`, which depends on
`smartstring`. smartstring defines `impl<Mode> Add<SmartString<Mode>> for
String`, so `String` now has a second `Add` impl crate-wide. That defeats the
deref-coercion the compiler previously used to resolve `String + &String`
(via `Add<&str>`): with two candidate impls it no longer coerces `&String ->
&str`, and existing `s + &owned_string` expressions fail to type-check
("cannot add `&String` to `String`").
The lib itself has no such site, but five test-code concatenations
(spawn_subagent, tokenjuice) did. Rewrite each `+ &expr` as `+ expr.as_str()`
so the `Add<&str>` impl is selected unambiguously — a no-op at runtime.
NOTE for future contributors: with the `repl` feature on, `String + &String`
no longer compiles anywhere in this crate; prefer `format!(...)`, `push_str`,
or `+ x.as_str()`.
Pure formatting (rustfmt) of the new `src/openhuman/rlm/` files, the tinyagents-bridge exposures, and the smartstring-fix sites. No behavior change.
senamakel
force-pushed
the
feat/rlm-language-workflows
branch
from
5181275 to
26f145e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26f145e520
ℹ️ 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".
| Err(err) => { | ||
| let mapped = map_eval_error(err, &available); | ||
| tracing::info!(session_key = %key, kind = mapped.kind(), "[rlm] cell failed"); | ||
| return Err(mapped); |
There was a problem hiding this comment.
When eval_cell returns an error, this early return skips both response construction and the later req.close_session cleanup. In the advertised fresh-session path where session_id is omitted, run_cell has already inserted a UUID-backed session, but RlmTool::execute only returns the error string, so the caller never learns the id to retry or close it; repeated parse/runtime/limit failures leave inaccessible sessions until TTL/LRU. Include the session id in error results or close generated/close_session sessions before returning.
Useful? React with 👍 / 👎.
| model: impl Into<String>, | ||
| temperature: f64, | ||
| ) -> Arc<dyn ChatModel<()>> { | ||
| Arc::new(ProviderModel::new(provider, model, temperature)) |
There was a problem hiding this comment.
For an RLM cell that calls model_query with a large generated prompt, this registers a bare ProviderModel with no context-window limit. The normal turn harness attaches .with_context_window(window) before registration (src/openhuman/tinyagents/mod.rs:1192-1195) so TinyAgents can reject oversized input before dispatch; RLM model queries bypass that profile limit and can send over-context prompts to the backend, failing late and expensively. Thread the parent context window into this factory before registering the model.
Useful? React with 👍 / 👎.
| if rlm_enabled | ||
| && security.autonomy != crate::openhuman::security::policy::AutonomyLevel::ReadOnly | ||
| { | ||
| tools.push(Box::new(crate::openhuman::rlm::RlmTool::new())); |
There was a problem hiding this comment.
Registering rlm in the global tool list makes it available to wildcard-scope agents on supervised/full tiers; for example tools_agent and morning_briefing use wildcard = {}, and the sub-agent registration strip only removes spawn/delegate tools, not rlm. That leaves the REPL/model-query surface exposed through generic or background agents even though the new docs describe it as an orchestrator-only workflow tool. Add an agent-scope/disallow filter or otherwise restrict registration to the intended orchestrator surface.
Useful? React with 👍 / 👎.
1 participant
Summary
Exposes TinyAgents' Rhai-backed
.ragshREPL (thereplcargo feature) as afirst-class
rlmtool in the OpenHuman core, so the orchestrator can authorand execute its own workflow scripts — fan-out over sub-agents, batched
tool/model calls, loops, dedup/verify pipelines — deterministically and
fail-closed, in the spirit of Claude Code Workflows and Recursive Language
Models.
One orchestrator tool call maps to one
eval_cell: the model writes a Rhaicell that runs against a persistent per-session namespace (
letbindingssurvive across cells via a
session_id), and the structured result flows backas the tool result. Scripts reach the host only through capability functions
(
tool_call,agent_query,model_query, their*_batchedvariants,emit,answer), each policy-bounded.Implements the phased plan in
docs/plans/rlm-workflows/.Architecture
Fail-closed guarantees. Every failure mode returns a model-consumable tool
result — never a panic, never a hung turn:
on_progressdeadline (script loops) →bridge_block_ontimer race (hung capability futures) → outertokio::timeout(policy.timeout + 5s)aroundspawn_blocking→ harnessToolTimeout::Secsbackstop (always above the tool's own timeout).a busy session returns a typed "busy" error, not a deadlock; poisoned sessions
are dropped, never reused.
ReplPolicycaps on ops, output bytes, script bytes, andper-kind call counts.
fulltier may raise call-count limits to a 2× ceilingvia
limits;readonlydoesn't get the tool at all.per-cell
ReplCancelFlag, so a user cancel drops an in-flight cell promptly;the session stays resumable.
Approval & security. The bridge restricts callable tools to the parent's
visible_tool_namesand excludes recursion/duplication hazards (rlm,spawn_*,run_workflow/await_workflow,CliRpcOnly-scoped tools). Approvalis not on the tinyagents repl path (it lives in the harness
wrap_toolmiddleware the REPL bypasses), so the bridge itself invokes
ApprovalGate::intercept_audited(+record_execution) for any tool whoseexternal_effect_with_argsis true, failing closed on denial. Becauseeval_cellruns onspawn_blocking+block_on, theagent_queryadapterre-installs the
PARENT_CONTEXTtask-localrun_subagentresolves.Kill switch. Registered for the orchestrator on
supervised/fulltiersonly; dark on
readonlyand behindOPENHUMAN_RLM=0.Two-repo dependency (⚠️ merge order)
This PR depends on tinyhumansai/tinyagents#19 (the
replhost-embedding:external
ReplCancelFlagcancellation, liveAgentEvent::ReplCallevents,set_cancel_flag, spawn_blocking docs). The submodule pointer here pins thatPR's branch head (
df391c4); re-pin to the merged commit once #19 lands,then this PR's CI full-lane re-runs. (One plan deviation, noted in #19: it
reuses the existing unit
TinyAgentsError::Cancelledrather than the plan'sproposed breaking
Cancelled(String).)Enabling
repladdsrhai→smartstring, whoseimpl Add<SmartString> for Stringputs a secondAddimpl forStringin the whole crate graph. Thatdefeats the deref-coercion the compiler used for
String + &String, sos + &owned_stringno longer compiles anywhere in this crate (useformat!,push_str, or+ x.as_str()). One commit here fixes the five existingtest-code sites this broke (
spawn_subagent,tokenjuice). This is an inherentconsequence of the plan's "enable
replunconditionally" directive; flagged foryour visibility in case you'd prefer a default-off cargo feature instead.
Tests (authored in the final phase, per the brief)
Co-located
#[cfg(test)]matrices across the domain (canonical module shape):policy.rs— tier gating, timeout clamp, override ceilings, concurrency cap.sessions.rs— namespace persistence, thread/session isolation, LRU cap,close, stats accumulation.
ops.rs— the phase-5 failure taxonomy driven end-to-end throughrun_cellwith a hand-built registry (happy path, parse error, unknown capability,
call-count limit, script-loop timeout, oversized output, session busy,
namespace persistence + close), plus full error-variant → kind mapping.
bridge.rs— exclusion predicate.tools.rs— metadata, timeout bounds,display, schema/arg validation.
run_cellwas split out ofeval_rlm_cellso the full evaluation path isunit-testable without constructing a whole
ParentExecutionContext.Commands run
cargo fmt --check·cargo clippy --all-targets --features repl -- -D warnings·cargo test --features repl·cargo test— all green.
GGML_NATIVE=OFF cargo check --lib— clean;cargo test --lib rlm::— 30 passed, 0 failed; full lib + all testtargets type-check clean (smartstring fixes verified).
Rollout / rollback
The surface is dark unless the tool registers (
OPENHUMAN_RLM=0kill switch +not on
readonly). Reverting the registration commit disables it withouttouching the domain. v1 out of scope (follow-ups): graph execution from scripts,
RPC/CLI exposure, durable cross-restart sessions, streaming partial stdout.
https://claude.ai/code/session_01WUHpeKDuT3zeowqrpyN17F
Summary by CodeRabbit
New Features
Documentation