Implement advanced Claude Code skill runtime - #21
Conversation
Co-authored-by: multica-agent <github@multica.ai>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @trotsky1997, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
![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: 9d1ca08be7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .map(|(tool, _)| tool) | ||
| .unwrap_or(decl) | ||
| .trim(); | ||
| head == name |
There was a problem hiding this comment.
When a skill uses Claude Code frontmatter such as allowed-tools: Bash(git status:*) or disallowed-tools: Read, this exact-case comparison does not match Ra's lowercase built-in tool names (bash, read, etc.; see src/tools/core.rs). The parser preserves those Claude-style declarations, so directly invoked skills with an allow list can accidentally hide/deny the intended tool, and deny lists with capitalized tool names will fail to block it. Normalize the declaration head and tool name before comparing.
Useful? React with 👍 / 👎.
| effective_text = render_slash_template(&template, &cmd.args); | ||
| skill_runtime = template.runtime.clone(); | ||
| if let Some(runtime) = &skill_runtime { | ||
| match render_dynamic_shell_context(effective_text, runtime, self.session.cwd()) |
There was a problem hiding this comment.
Because argument substitution runs before render_dynamic_shell_context, any direct skill whose body contains $ARGUMENTS (or no placeholders, triggering the ARGUMENTS: fallback) will execute shell syntax supplied by the user, e.g. /review !\touch /tmp/pwned`. That runs locally before the model/tool policy path and bypasses scoped allowed-tools, disallowed-tools, and hook checks, even if the skill author did not include dynamic context in the skill body. Render only the skill-authored template commands before interpolating arguments, or escape arguments so they cannot introduce !`/```! blocks.
Useful? React with 👍 / 👎.
| let head = decl | ||
| .split_once('(') | ||
| .map(|(tool, _)| tool) | ||
| .unwrap_or(decl) | ||
| .trim(); | ||
| head == name |
There was a problem hiding this comment.
For declarations like allowed-tools: "bash(git status:*)", this strips everything after ( and treats it as permission for the entire bash tool. A skill intended to allow only read-only git status commands will advertise and execute unrestricted bash commands for the invocation, so the scoped policy is much broader than the skill declares. Preserve and validate the parenthesized constraint against the tool input instead of widening it to the tool name.
Useful? React with 👍 / 👎.
| let child_messages = self.snapshot_messages().await; | ||
| self.restore_messages(parent_snapshot).await; | ||
| let outcome = result?; | ||
| if let Some(final_text) = final_assistant_text(&child_messages) { |
There was a problem hiding this comment.
When a forked skill is cancelled or otherwise completes without new assistant text, child_messages still contains the parent snapshot, so final_assistant_text can pick the parent's previous assistant message and append it as the skill result. This duplicates stale parent output even though the fork produced no final answer; select only assistant messages added after parent_snapshot.len() and avoid appending on cancellation/no result.
Useful? React with 👍 / 👎.
| } else if let Some(scope) = scope { | ||
| session.prompt_scoped(effective_text, scope).await |
There was a problem hiding this comment.
For a directly invoked skill that declares a hooks.Stop entry, this scoped prompt path finishes and clears the skill scope before the outer AgentEnd hook runs, and that outer hook only calls self.session.hooks(). As a result, skill-scoped Stop hooks run only on the early UserPromptSubmit block/stop paths above, not on normal completion, so cleanup/audit hooks declared by the skill are silently skipped.
Useful? React with 👍 / 👎.
Co-authored-by: multica-agent <github@multica.ai>
1 participant
Summary
Tests
Closes #20