feat(runtime): add javascript facade and skill creator agent#1971
Merged
senamakel merged 3 commits intoMay 17, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR renames core bootstrap routines, exposes a JavaScript language surface backed by a runtime_node implementation (types/ops/rpc/schemas), re-exports Node runtime utilities under ChangesJavaScript Runtime and Agent System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@gitbooks/developing/architecture.md`:
- Line 166: The doc currently mixes "Node.js Runtime" terminology with leftover
"QuickJS" descriptions; update the architecture narrative to be consistent by
replacing all QuickJS-specific wording (e.g., "QuickJS", "QuickJS-per-skill
execution", and any sections describing per-skill QuickJS VMs, execution flow or
security assumptions tied to QuickJS) with Node.js-oriented equivalents or a
clear note that QuickJS is deprecated/removed; ensure the "Node.js Runtime"
header and paragraphs about resolving/ installing node, runtime bridge, and tool
execution reflect a single model, and audit the other QuickJS occurrences (the
other sections flagged) to unify wording and adjust any flow/security statements
to the Node.js model.
In `@src/openhuman/agent/agents/skill_creator/prompt.rs`:
- Around line 10-38: Add tracing to the prompt assembly in function build: emit
a trace/debug at entry (include ctx id or summary if available), log each render
call invocation (render_user_files, render_tools, render_safety,
render_workspace) and capture their Result outcome and the produced string
length (or error) and the boolean decision whether the section was appended
(e.g., user_files_included, tools_included, workspace_included), log branch
decisions when skipping empty sections, and log on any early errors returned
from render_* with the error info; finally emit a trace/debug at exit with the
final out.len() and success status. Use the crate's tracing/log macros (trace!
or debug!) and keep logs limited to lengths/flags only, not the prompt content.
In `@src/openhuman/runtime_node/ops.rs`:
- Around line 32-113: Add structured debug/trace instrumentation to the runtime
tool flow: in build_runtime_tools log at trace entry with the incoming Config
summary, trace before/after the call to
crate::openhuman::memory::create_memory_with_local_ai (include success or mapped
error), and trace the call/return of tools::all_tools_with_runtime; in
list_tools trace entry, number of tools returned and trace exit after sorting;
in execute_tool trace entry with tool_name and prefer_markdown, trace the result
of build_runtime_tools, trace the branch that finds or fails to find the tool
(include tool_name), emit debug when publishing
DomainEvent::ToolExecutionStarted, trace before/after tool.execute_with_options
(include errors), compute and debug-log elapsed_ms and success, and trace before
returning ExecuteToolOutcome; use the crate's tracing/log macros (debug/trace)
and include identifiers like build_runtime_tools, list_tools, execute_tool,
create_memory_with_local_ai, tools::all_tools_with_runtime, summarize_tool, and
publish_global to make logs easy to correlate.
In `@src/openhuman/runtime_node/schemas.rs`:
- Around line 116-159: The two functions handle_list_tools and
handle_execute_tool contain operational runtime logic and should be converted
into thin delegators that forward work to implementations in rpc.rs; extract the
orchestration (config loading, calling javascript::list_tools /
javascript::execute_tool, building payload/log and RpcOutcome) into new async
functions in the runtime rpc module (e.g. list_tools_handler and
execute_tool_handler) and update handle_list_tools and handle_execute_tool to
only deserialize params to the appropriate types (ListToolsParams /
ExecuteToolParams) and call the corresponding rpc.rs handler, returning its
result unchanged so schemas.rs remains declarative and only responsible for
registration/delegation.
🪄 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: 15809d28-9397-46de-b3d5-9173bd4edbd1
📒 Files selected for processing (44)
app/test/e2e/helpers/skill-e2e-runtime.tsgitbooks/developing/architecture.mdsrc/core/all.rssrc/core/event_bus/events_tests.rssrc/core/jsonrpc.rssrc/core/jsonrpc_tests.rssrc/openhuman/agent/agents/loader.rssrc/openhuman/agent/agents/mod.rssrc/openhuman/agent/agents/orchestrator/agent.tomlsrc/openhuman/agent/agents/skill_creator/agent.tomlsrc/openhuman/agent/agents/skill_creator/mod.rssrc/openhuman/agent/agents/skill_creator/prompt.mdsrc/openhuman/agent/agents/skill_creator/prompt.rssrc/openhuman/agent/harness/builtin_definitions.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/composio/periodic.rssrc/openhuman/composio/providers/registry.rssrc/openhuman/config/schema/node.rssrc/openhuman/javascript/mod.rssrc/openhuman/memory/global.rssrc/openhuman/mod.rssrc/openhuman/runtime_node/bootstrap.rssrc/openhuman/runtime_node/downloader.rssrc/openhuman/runtime_node/extractor.rssrc/openhuman/runtime_node/mod.rssrc/openhuman/runtime_node/ops.rssrc/openhuman/runtime_node/resolver.rssrc/openhuman/runtime_node/schemas.rssrc/openhuman/runtime_node/types.rssrc/openhuman/scheduler_gate/gate.rssrc/openhuman/screen_intelligence/cli/mod.rssrc/openhuman/skills/README.mdsrc/openhuman/skills/mod.rssrc/openhuman/skills/types.rssrc/openhuman/tool_timeout/mod.rssrc/openhuman/tools/impl/system/node_exec.rssrc/openhuman/tools/impl/system/npm_exec.rssrc/openhuman/tools/impl/system/shell.rssrc/openhuman/tools/ops.rssrc/openhuman/webhooks/bus.rssrc/openhuman/webhooks/ops.rssrc/openhuman/webhooks/router.rssrc/openhuman/webhooks/schemas.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
gitbooks/developing/architecture.md (1)
166-177:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRuntime narrative is still mixed with QuickJS-era sections.
This update describes Node.js/runtime_node, but the same document still states QuickJS-based execution in several places (Line 60, Line 81, Line 92, Line 226, Line 263, Line 277, Line 308, Line 311, Line 320), leaving contradictory architecture guidance.
Also applies to: 189-200
🤖 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 `@gitbooks/developing/architecture.md` around lines 166 - 177, The document mixes Node.js/runtime_node details with leftover QuickJS descriptions causing contradictions; search for and replace or update all QuickJS-era references (e.g., mentions around "QuickJS", "QuickJS-based execution", and related lines referenced in the comment) to reflect the current Node.js runtime model, ensuring consistency in sections that describe "Node.js Runtime", "Current JS backend: runtime_node", the tool bridge behavior (SKILL.md and runtime bridge), and any integrity/installation notes so that every reference describes Node-backed execution rather than QuickJS-based VMs; keep terminology consistent (use "Node.js", "runtime_node", "managed Node" where applicable) and remove or rework any QuickJS-specific instructions or examples.
🤖 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/runtime_node/rpc.rs`:
- Around line 20-23: The RPC handlers (e.g., list_tools_handler) currently
return Result<serde_json::Value, String>; change their signatures to return
RpcOutcome<serde_json::Value> and update all return points to construct
RpcOutcome::Ok(...) or RpcOutcome::Err(...) (propagating errors via the
RpcOutcome type) instead of using Result<T, String>, ensuring any CLI/JSON
conversion remains at the schema/delegation boundary rather than inside these
handlers; apply the same change to the other handlers noted (the ones around
lines referenced in the review) so the module consistently uses RpcOutcome<T> as
the RPC contract.
- Around line 23-25: Add tracing calls around the RPC handler flow: at function
entry and exit, and before/after each key call such as
config_rpc::load_config_with_timeout() and javascript::list_tools(), using
tracing::debug or tracing::trace; log the input context, success/failure of each
call (including error details on Err) and include the final payload and the
returned log vector in the exit trace so runtime diagnostics are available.
Locate the handler in rpc.rs and insert tracing::trace!("enter <handler_name>");
before work begins, trace the result of load_config_with_timeout() and
list_tools() with context, and trace!("exit <handler_name> payload={:?}
logs={:?}", payload, log_vec) on success (and corresponding traces on error
paths). Ensure you import tracing and use appropriate debug/trace levels per
guideline.
---
Duplicate comments:
In `@gitbooks/developing/architecture.md`:
- Around line 166-177: The document mixes Node.js/runtime_node details with
leftover QuickJS descriptions causing contradictions; search for and replace or
update all QuickJS-era references (e.g., mentions around "QuickJS",
"QuickJS-based execution", and related lines referenced in the comment) to
reflect the current Node.js runtime model, ensuring consistency in sections that
describe "Node.js Runtime", "Current JS backend: runtime_node", the tool bridge
behavior (SKILL.md and runtime bridge), and any integrity/installation notes so
that every reference describes Node-backed execution rather than QuickJS-based
VMs; keep terminology consistent (use "Node.js", "runtime_node", "managed Node"
where applicable) and remove or rework any QuickJS-specific instructions or
examples.
🪄 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: aeec22ae-214f-4afd-ade3-33efdfbf0f53
📒 Files selected for processing (6)
gitbooks/developing/architecture.mdsrc/openhuman/agent/agents/skill_creator/prompt.rssrc/openhuman/runtime_node/mod.rssrc/openhuman/runtime_node/ops.rssrc/openhuman/runtime_node/rpc.rssrc/openhuman/runtime_node/schemas.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/runtime_node/mod.rs
- src/openhuman/agent/agents/skill_creator/prompt.rs
Comment on lines
+20
to
+23
| pub(crate) async fn list_tools_handler( | ||
| _params: ListToolsParams, | ||
| ) -> Result<serde_json::Value, String> { | ||
| let config = config_rpc::load_config_with_timeout().await?; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use RpcOutcome<T> as the RPC handler return contract.
These handlers convert to CLI JSON inside rpc.rs and return Result<Value, String>. Keep rpc.rs returning RpcOutcome<T> and do CLI conversion at the schema/delegation boundary.
Proposed contract-aligned patch
--- a/src/openhuman/runtime_node/rpc.rs
+++ b/src/openhuman/runtime_node/rpc.rs
@@
pub(crate) async fn list_tools_handler(
_params: ListToolsParams,
-) -> Result<serde_json::Value, String> {
+) -> Result<RpcOutcome<serde_json::Value>, String> {
@@
- RpcOutcome::new(payload, log).into_cli_compatible_json()
+ Ok(RpcOutcome::new(payload, log))
}
@@
pub(crate) async fn execute_tool_handler(
params: ExecuteToolParams,
-) -> Result<serde_json::Value, String> {
+) -> Result<RpcOutcome<serde_json::Value>, String> {
@@
- RpcOutcome::new(payload, log).into_cli_compatible_json()
+ Ok(RpcOutcome::new(payload, log))
}--- a/src/openhuman/runtime_node/schemas.rs
+++ b/src/openhuman/runtime_node/schemas.rs
@@
- list_tools_handler(params).await
+ list_tools_handler(params).await?.into_cli_compatible_json()
@@
- execute_tool_handler(params).await
+ execute_tool_handler(params).await?.into_cli_compatible_json()As per coding guidelines, "src/openhuman/*/rpc.rs: Use RpcOutcome<T> as the return type for RPC controller handlers in src/openhuman/<domain>/rpc.rs".
Also applies to: 36-39, 33-34, 57-58
🤖 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/runtime_node/rpc.rs` around lines 20 - 23, The RPC handlers
(e.g., list_tools_handler) currently return Result<serde_json::Value, String>;
change their signatures to return RpcOutcome<serde_json::Value> and update all
return points to construct RpcOutcome::Ok(...) or RpcOutcome::Err(...)
(propagating errors via the RpcOutcome type) instead of using Result<T, String>,
ensuring any CLI/JSON conversion remains at the schema/delegation boundary
rather than inside these handlers; apply the same change to the other handlers
noted (the ones around lines referenced in the review) so the module
consistently uses RpcOutcome<T> as the RPC contract.
Comment on lines
+23
to
+25
| let config = config_rpc::load_config_with_timeout().await?; | ||
| let tools = javascript::list_tools(&config)?; | ||
| let payload = json!({ "tools": tools }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add debug/trace instrumentation in RPC handlers.
The new RPC entrypoints currently have no debug/trace logs for entry/exit and call boundaries; the returned log vector is not a substitute for runtime diagnostics.
Minimal tracing patch sketch
use serde::Deserialize;
use serde_json::{json, Value};
+use tracing::{debug, trace};
@@
pub(crate) async fn list_tools_handler(
_params: ListToolsParams,
) -> Result<serde_json::Value, String> {
+ debug!("[runtime_node::rpc] list_tools_handler: start");
let config = config_rpc::load_config_with_timeout().await?;
+ trace!("[runtime_node::rpc] list_tools_handler: javascript::list_tools");
let tools = javascript::list_tools(&config)?;
@@
- RpcOutcome::new(payload, log).into_cli_compatible_json()
+ debug!(tool_count = payload["tools"].as_array().map(|v| v.len()).unwrap_or(0), "[runtime_node::rpc] list_tools_handler: done");
+ RpcOutcome::new(payload, log).into_cli_compatible_json()
}
@@
) -> Result<serde_json::Value, String> {
+ debug!(tool_name = %params.tool_name, prefer_markdown = params.prefer_markdown, "[runtime_node::rpc] execute_tool_handler: start");
let config = config_rpc::load_config_with_timeout().await?;
@@
+ trace!(tool_name = %params.tool_name, "[runtime_node::rpc] execute_tool_handler: javascript::execute_tool");
let outcome =
javascript::execute_tool(&config, ¶ms.tool_name, args, params.prefer_markdown).await?;
@@
+ debug!(tool_name = %payload["tool_name"].as_str().unwrap_or("<unknown>"), elapsed_ms = payload["elapsed_ms"].as_u64().unwrap_or(0), "[runtime_node::rpc] execute_tool_handler: done");
RpcOutcome::new(payload, log).into_cli_compatible_json()
}As per coding guidelines, "src/**/*.rs: Rust core code must use log / tracing at debug / trace levels for verbose diagnostics on new/changed flows...".
Also applies to: 39-47, 52-57
🤖 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/runtime_node/rpc.rs` around lines 23 - 25, Add tracing calls
around the RPC handler flow: at function entry and exit, and before/after each
key call such as config_rpc::load_config_with_timeout() and
javascript::list_tools(), using tracing::debug or tracing::trace; log the input
context, success/failure of each call (including error details on Err) and
include the final payload and the returned log vector in the exit trace so
runtime diagnostics are available. Locate the handler in rpc.rs and insert
tracing::trace!("enter <handler_name>"); before work begins, trace the result of
load_config_with_timeout() and list_tools() with context, and trace!("exit
<handler_name> payload={:?} logs={:?}", payload, log_vec) on success (and
corresponding traces on error paths). Ensure you import tracing and use
appropriate debug/trace levels per guideline.
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
javascriptruntime surface in the Rust core for tool listing and dispatchruntime_nodemoduleskill_creatoragent and expose it to the orchestrator ascreate_skillProblem
Solution
openhuman::javascriptas the first-class public language module while keeping the existing Node bootstrap as the current backend implementationruntime_nodeand update tool/runtime imports and controller wiring accordinglyskill_creatorbuilt-in agent with Node-capable tools and wire it into the orchestrator subagent inventoryjavascriptSubmission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge. N/A: not run locally here; rely on CI for gate validation.docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change) N/A: behaviour/module refactor only, no matrix row changes.## RelatedN/A: no matrix feature IDs were updated.docs/RELEASE-MANUAL-SMOKE.md) N/A: no release manual smoke rows changed.Closes #NNNin the## Relatedsection N/A: no linked GitHub issue was provided.Impact
javascript, with the current Node implementation preserved behindruntime_nodejavascriptwithout forcing Node-specific naming into the top-level APIRelated
node_runtime::*toruntime_node::*/javascript::*if desiredAI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/javascript-runtime-skill-creator326970eeValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckGGML_NATIVE=OFF cargo test --lib openhuman::runtime_node --manifest-path Cargo.tomlGGML_NATIVE=OFF cargo test --lib openhuman::agent::agents::loader --manifest-path Cargo.tomlcargo fmt --allGGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlpnpm --filter openhuman-app rust:checkValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
javascript, move the current Node implementation behindruntime_node, and add askill_creatoragent for orchestrator delegationcreate_skillspecialistParity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Documentation
Refactor