diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index d8349e530..f408b5403 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -174,6 +174,15 @@ Done when: external-path or symlink checks. - [x] The policy normalizes a variable `git ls-tree` tree operand to the reviewed read-only verb. Other Git subcommands keep exact parser output. +- [x] Tool schemas and always-loaded guidance distinguish a persistent project + root from one-command `WorkingDirectory` scope, prevent redundant project + switches, and preserve `cd` when directory mutation is the requested shell + behavior. +- [x] Sanitized behavioral eval cases cover early project declaration, + one-command typed scope, failed-path recovery, and deliberate inline `cd`. +- [ ] Run the new behavioral eval cases against a configured model provider. + The local eval provider type, endpoint, and model ID were unset for this + slice; syntax and ShellCheck validation passed. - [ ] A constrained executable grammar proves any future safe `sed` form. The `-n` option alone is not proof because a `sed` program can write files or execute commands. diff --git a/evals/run-evals.sh b/evals/run-evals.sh index bd3179cda..9da857c1a 100755 --- a/evals/run-evals.sh +++ b/evals/run-evals.sh @@ -107,6 +107,11 @@ check_prerequisites() { exit 1 fi + if ! command -v jq >/dev/null 2>&1; then + echo "ERROR: 'jq' not found. Install jq to run the eval suite." >&2 + exit 1 + fi + # Identity files are rendered from repo templates into an isolated eval # home; the host does not need a pre-initialized ~/.netclaw tree. if [[ ! -f "$REPO_ROOT/src/Netclaw.Cli/Resources/identity/SOUL.template.md" ]]; then @@ -601,7 +606,7 @@ store_result() { VALUES ('$RUN_ID', '$esc_category', '$case_name', $run_number, '$esc_prompt', $passed, '$esc_details');" } -## Parses a [usage] line and stores performance metrics. +## Parses text or structured JSON usage output and stores performance metrics. ## Args: case_name, run_number, [turn_number (default 1)], [usage_line (default: last [usage] in STDOUT_FILE)] ## Called after each run_prompt / run_prompt_resume. store_metrics() { @@ -612,18 +617,30 @@ store_metrics() { local turn_number="${3:-1}" local usage_line="${4:-}" - # When no explicit usage line is passed, read the last one in STDOUT_FILE. + local input_tokens output_tokens cached_tokens prompt_ms tok_s + + # Structured cases keep tool calls separate from model text so assertions + # can prove provenance. Preserve their performance metrics as well. if [[ -z "$usage_line" ]]; then - usage_line=$(grep -ao '\[usage\].*' "$STDOUT_FILE" 2>/dev/null | tail -1) || return 0 + if jq -e '.usage != null' "$STDOUT_FILE" >/dev/null 2>&1; then + input_tokens=$(jq -r '.usage.inputTokens // empty' "$STDOUT_FILE") + output_tokens=$(jq -r '.usage.outputTokens // empty' "$STDOUT_FILE") + cached_tokens=$(jq -r '.usage.cachedInputTokens // empty' "$STDOUT_FILE") + prompt_ms=$(jq -r '.usage.promptMs // empty' "$STDOUT_FILE") + tok_s=$(jq -r '.usage.predictedPerSecond // empty' "$STDOUT_FILE") + else + usage_line=$(grep -ao '\[usage\].*' "$STDOUT_FILE" 2>/dev/null | tail -1) || return 0 + fi fi # Parse fields from: [usage] in=X out=Y total=Z cached=C prompt_ms=P tok_s=T - local input_tokens output_tokens cached_tokens prompt_ms tok_s - input_tokens=$(echo "$usage_line" | grep -aoP 'in=\K[0-9]+' || echo "") - output_tokens=$(echo "$usage_line" | grep -aoP 'out=\K[0-9]+' || echo "") - cached_tokens=$(echo "$usage_line" | grep -aoP 'cached=\K[0-9]+' || echo "") - prompt_ms=$(echo "$usage_line" | grep -aoP 'prompt_ms=\K[0-9.]+' || echo "") - tok_s=$(echo "$usage_line" | grep -aoP 'tok_s=\K[0-9.]+' || echo "") + if [[ -n "$usage_line" ]]; then + input_tokens=$(echo "$usage_line" | grep -aoP 'in=\K[0-9]+' || echo "") + output_tokens=$(echo "$usage_line" | grep -aoP 'out=\K[0-9]+' || echo "") + cached_tokens=$(echo "$usage_line" | grep -aoP 'cached=\K[0-9]+' || echo "") + prompt_ms=$(echo "$usage_line" | grep -aoP 'prompt_ms=\K[0-9.]+' || echo "") + tok_s=$(echo "$usage_line" | grep -aoP 'tok_s=\K[0-9.]+' || echo "") + fi # Skip if no metrics found [[ -z "$input_tokens" && -z "$cached_tokens" && -z "$prompt_ms" ]] && return 0 @@ -744,6 +761,7 @@ check_daemon_alive() { run_prompt() { local prompt="$1" + local output_format="${2:-text}" STDOUT_FILE="$TMPDIR_EVAL/stdout_$(date +%s%N).txt" # Record daemon log position before the prompt (the daemon writes to a @@ -757,9 +775,14 @@ run_prompt() { # Run prompt via the host CLI, but redirect it at the eval container's # daemon and keep CLI-side path resolution inside the eval sandbox. + local -a output_args=() + if [[ "$output_format" == "json" ]]; then + output_args+=(--json) + fi + NETCLAW_DAEMON_ENDPOINT="http://127.0.0.1:$EVAL_PORT" \ NETCLAW_HOME="$EVAL_HOME" \ - timeout "$PROMPT_TIMEOUT" "$NETCLAW_BIN" chat -p "$prompt" \ + timeout "$PROMPT_TIMEOUT" "$NETCLAW_BIN" chat -p "${output_args[@]}" "$prompt" \ > "$STDOUT_FILE" 2>&1 || true # Brief pause for daemon log flush @@ -950,6 +973,29 @@ stdout_tool_called() { grep -qaE "\\[tool:call\\] $1\\(" "$STDOUT_FILE" 2>/dev/null } +stdout_json_envelope_valid() { + jq -e ' + type == "object" + and (.sessionId | type == "string" and length > 0) + and (.response | type == "string") + and (.toolCalls == null or (.toolCalls | type == "array")) + ' "$STDOUT_FILE" >/dev/null 2>&1 +} + +stdout_json_tool_called() { + local tool_name="$1" + jq -e --arg tool_name "$tool_name" \ + 'any(.toolCalls[]?; .toolName == $tool_name)' \ + "$STDOUT_FILE" >/dev/null 2>&1 +} + +stdout_json_tool_call_arguments() { + local tool_name="$1" + jq -ce --arg tool_name "$tool_name" \ + '.toolCalls[]? | select(.toolName == $tool_name) | .argumentsJson | fromjson' \ + "$STDOUT_FILE" 2>/dev/null +} + stdout_skill_file_read_called() { grep -aiE '^\[tool:call\] file_read\(' "$STDOUT_FILE" 2>/dev/null \ | grep -qi 'SKILL\.md' @@ -1489,21 +1535,29 @@ assert_multi_turn_conflicting_speakers() { # because calling it after the first shell prompt has already burned the # user's attention is the regression we're guarding against. assert_approval_set_working_directory_positive() { - stdout_tool_called 'set_working_directory' || return 1 + local set_call + stdout_json_envelope_valid || return 1 + set_call=$(stdout_json_tool_call_arguments 'set_working_directory' | head -1) + jq -e '.Path == "/tmp"' <<<"$set_call" >/dev/null || return 1 # If shell_execute also happened, ensure set_working_directory came first. - if stdout_tool_called 'shell_execute'; then - local swd_line shell_line - swd_line=$(grep -anE '\[tool:call\] set_working_directory' "$STDOUT_FILE" | head -1 | cut -d: -f1) - shell_line=$(grep -anE '\[tool:call\] shell_execute' "$STDOUT_FILE" | head -1 | cut -d: -f1) - [[ -n "$swd_line" && -n "$shell_line" && "$swd_line" -lt "$shell_line" ]] + if stdout_json_tool_called 'shell_execute'; then + local shell_call command + shell_call=$(stdout_json_tool_call_arguments 'shell_execute' | head -1) + command=$(jq -r '.Command // empty' <<<"$shell_call") + jq -e ' + [.toolCalls[]?.toolName] as $names + | ($names | index("set_working_directory")) < ($names | index("shell_execute")) + ' "$STDOUT_FILE" >/dev/null && \ + [[ ! "$command" =~ ^[[:space:]]*cd[[:space:]] ]] fi } # Negative: no project signal. Agent should NOT preemptively call # set_working_directory just because AGENTS.md mentions it. assert_approval_set_working_directory_negative() { - ! stdout_tool_called 'set_working_directory' + stdout_json_envelope_valid || return 1 + ! stdout_json_tool_called 'set_working_directory' } # Recovery: T1 agent issues a shell call that gets denied for cwd-outside- @@ -1517,7 +1571,50 @@ assert_approval_set_working_directory_negative() { # triggers the prompt path. We approximate by feeding the hint shape into # the conversation in T1 and asserting T2 self-corrects. assert_approval_recovery_hint() { - stdout_tool_called 'set_working_directory' + local set_call + stdout_json_envelope_valid || return 1 + set_call=$(stdout_json_tool_call_arguments 'set_working_directory' | head -1) + jq -e '.Path == "/tmp"' <<<"$set_call" >/dev/null +} + +# One command in another directory should use the typed shell argument. +assert_approval_shell_working_directory_argument() { + local shell_call + stdout_json_envelope_valid || return 1 + shell_call=$(stdout_json_tool_call_arguments 'shell_execute' | head -1) + + jq -e '.WorkingDirectory == "/tmp" and .Command == "pwd"' \ + <<<"$shell_call" >/dev/null +} + +# Preserve inline cd when directory mutation is the behavior under test. +assert_approval_inline_cd_semantics() { + local shell_call + stdout_json_envelope_valid || return 1 + shell_call=$(stdout_json_tool_call_arguments 'shell_execute' | head -1) + + jq -e '.Command == "cd /tmp && pwd" and (.WorkingDirectory? == null)' \ + <<<"$shell_call" >/dev/null +} + +# A failed project switch must be corrected before shell work continues. +assert_approval_set_working_directory_retry() { + local shell_call + local -a swd_calls + stdout_json_envelope_valid || return 1 + mapfile -t swd_calls < <(stdout_json_tool_call_arguments 'set_working_directory') + shell_call=$(stdout_json_tool_call_arguments 'shell_execute' | head -1) + + [[ "${#swd_calls[@]}" -ge 2 ]] && \ + jq -e '.Path == "/tmp/missing-project"' <<<"${swd_calls[0]}" >/dev/null && \ + jq -e '.Path == "/tmp"' <<<"${swd_calls[1]}" >/dev/null && \ + jq -e ' + [.toolCalls[]?.toolName] as $names + | [$names[] | select(. == "set_working_directory")] | length >= 2 + and ($names | index("shell_execute")) > ($names | index("set_working_directory")) + and ($names | index("shell_execute")) > ($names | rindex("set_working_directory")) + ' "$STDOUT_FILE" >/dev/null && \ + jq -e '.Command == "pwd"' <<<"$shell_call" >/dev/null } # Schedule pre-approval: user asks to schedule an unattended task that @@ -1575,6 +1672,11 @@ end_category() { } run_case() { + local output_format="text" + if [[ "${1:-}" == "--json" ]]; then + output_format="json" + shift + fi local case_name="$1"; shift local description="$1"; shift local -a prompts=("$@") @@ -1599,7 +1701,7 @@ run_case() { local prompt prompt=$(pick_variant "${prompts[@]}") - run_prompt "$prompt" + run_prompt "$prompt" "$output_format" local passed=0 local details="fail" @@ -1972,17 +2074,26 @@ run_all() { # rather than waiting for the user to do it manually. print_category "Approval Policy v2" - run_case approval_set_working_directory_positive "calls set_working_directory before shell tool when project mentioned" \ + run_case --json approval_set_working_directory_positive "calls set_working_directory before shell tool when project mentioned" \ "I'm starting a debugging session on the project checked out at /tmp. Get oriented in that codebase — look at the layout, identify build files, and figure out what kind of project it is. We'll be running multiple shell commands across the tree." \ "I want to start working on the Netclaw checkout at /tmp. Plan to run several commands across that tree — start by getting yourself oriented." - run_case approval_set_working_directory_negative "does NOT call set_working_directory for unrelated prompts" \ + run_case --json approval_set_working_directory_negative "does NOT call set_working_directory for unrelated prompts" \ "What's two plus two? Just give me the number." \ "Explain what a hash table is in one sentence." - run_case approval_recovery_hint "recovers from cwd-outside-safe-spaces denial by calling set_working_directory" \ + run_case --json approval_recovery_hint "recovers from cwd-outside-safe-spaces denial by calling set_working_directory" \ "I just tried to run a shell command in /tmp and the daemon returned: 'Tool access denied: approval_denied_by_user. Hint: \"/tmp\" is outside the session'\\''s trusted scope. Call set_working_directory \"/tmp\" first, then retry — that brings the directory into your trusted scope so the approval policy can reason about it.' How should I unblock this so the next shell call works?" + run_case --json approval_shell_working_directory_argument "uses the typed WorkingDirectory argument instead of inline cd" \ + "Run pwd from /tmp with one shell_execute call. Do not change the session project directory." + + run_case --json approval_inline_cd_semantics "keeps inline cd when directory change is the requested shell behavior" \ + "Run a Bash control-flow experiment in one shell_execute call: execute 'cd /tmp && pwd' exactly as a compound command. Changing directory is the behavior being tested, so do not replace it with a WorkingDirectory argument." + + run_case --json approval_set_working_directory_retry "corrects a failed project switch before shell work" \ + "Test project-directory recovery: first call set_working_directory with /tmp/missing-project and observe the rejection. Then correct it by calling set_working_directory with /tmp, and only after that run pwd in the shell." + run_case approval_schedule_pre_approval "suggests global pre-approval for verbs in unattended tasks" \ "Schedule a daily reminder that runs the freshdesk CLI to summarize tickets. The reminder fires unattended and won't be able to answer approval prompts, so the verb needs to be globally pre-approved before the schedule fires. Call netclaw approvals trust-verb freshdesk via shell_execute as part of the setup." diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index 6ea20671a..84c30b213 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.46.0" + version: "2.47.0" --- # Netclaw Operations @@ -44,6 +44,13 @@ allowed roots); the project's identity file (`.netclaw/AGENTS.md`, `CLAUDE.md`, `AGENTS.md`, or `CONTEXT.md`) then loads into the prompt. Full rules: `skill_read_resource('netclaw-operations', 'references/projects.md')`. +Use the `shell_execute` `WorkingDirectory` argument for one command in another +directory. Do not add an inline `cd` unless changing directory is itself the +behavior the user asked you to run or test. Use +`set_working_directory` when later commands and subagents need the same project +root. Do not repeat it when `[working-context]` already names that project. If +the tool rejects a path, correct the path and retry it before work continues. + For Team and Personal sessions, `[working-context]` is refreshed at the start of each new turn. In a Git project it includes the active worktree, branch, HEAD, upstream divergence, and dirty counts. Treat this as turn-start diff --git a/feeds/skills/.system/files/netclaw-operations/references/projects.md b/feeds/skills/.system/files/netclaw-operations/references/projects.md index f590767c0..f3d8e6978 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/projects.md +++ b/feeds/skills/.system/files/netclaw-operations/references/projects.md @@ -13,7 +13,7 @@ SOUL/AGENTS/TOOLING layers. Use `set_working_directory` to set or change the project directory: ``` -set_working_directory(path: "/home/user/workspaces/akadonic") +set_working_directory(path: "/workspace/service") ``` Rules: @@ -27,6 +27,12 @@ Rules: - The project directory persists across crash/restart via `WorkingContext` - The `[working-context]` block includes `project_dir:` so you always know which project is active +- Do not call the tool again when `project_dir` already names the right project +- A failed call does not change the project directory. Correct the path and + retry the tool before you continue. +- For one shell call in another directory, use the `shell_execute` + `WorkingDirectory` argument. Do not add an inline `cd` unless changing + directory is itself the behavior the user asked you to run or test. The project directory is distinct from the session directory (`~/.netclaw/sessions/{id}/`). The session directory is immutable and used for diff --git a/src/Netclaw.Actors.Tests/Tools/SetWorkingDirectoryAudienceTests.cs b/src/Netclaw.Actors.Tests/Tools/SetWorkingDirectoryAudienceTests.cs index 813159a5a..a5e5b365d 100644 --- a/src/Netclaw.Actors.Tests/Tools/SetWorkingDirectoryAudienceTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/SetWorkingDirectoryAudienceTests.cs @@ -20,6 +20,23 @@ public sealed class SetWorkingDirectoryAudienceTests ShellExecutionMode.HostAllowed, UsedStrictFallback: false); + [Fact] + public void Path_schema_describes_persistent_multi_command_scope() + { + var tool = new SetWorkingDirectoryTool(new ToolConfig(), new NetclawPaths()); + Assert.Contains("before multi-command work", tool.Description, StringComparison.Ordinal); + Assert.Contains("Do not call it again", tool.Description, StringComparison.Ordinal); + + var description = tool.ParameterSchema + .GetProperty("properties") + .GetProperty("Path") + .GetProperty("description") + .GetString(); + + Assert.Contains("project root", description, StringComparison.Ordinal); + Assert.Contains("multi-command task", description, StringComparison.Ordinal); + } + [Fact] public void SetWorkingDirectory_BlockedForPublicAudience_ByDefault() { diff --git a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs index 33c921e99..4f9cfe989 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs @@ -42,6 +42,25 @@ public void Constructor_rejects_policies_from_different_shell_environments() Assert.Contains("same shell environment", exception.Message); } + [Fact] + public void Working_directory_schema_prefers_the_typed_argument_to_inline_cd() + { + var commandDescription = _tool.ParameterSchema + .GetProperty("properties") + .GetProperty("Command") + .GetProperty("description") + .GetString(); + var description = _tool.ParameterSchema + .GetProperty("properties") + .GetProperty("WorkingDirectory") + .GetProperty("description") + .GetString(); + + Assert.Equal("The shell command to execute.", commandDescription); + Assert.Contains("Prefer this argument", description, StringComparison.Ordinal); + Assert.Contains("inline cd", description, StringComparison.Ordinal); + } + [Fact] public async Task Missing_selected_executable_fails_without_fallback() { diff --git a/src/Netclaw.Actors/Tools/SetWorkingDirectoryTool.cs b/src/Netclaw.Actors/Tools/SetWorkingDirectoryTool.cs index 651a35b7d..45c6f43e7 100644 --- a/src/Netclaw.Actors/Tools/SetWorkingDirectoryTool.cs +++ b/src/Netclaw.Actors/Tools/SetWorkingDirectoryTool.cs @@ -16,7 +16,8 @@ namespace Netclaw.Actors.Tools; /// re-assemble the system prompt with project-scoped identity files. /// [NetclawTool(ToolName, - "Declare your project root and expand your trusted scope. " + + "Call this once before multi-command work in a named project. Do not call it again when the current project already matches. " + + "It declares the project root and expands your trusted scope. " + "Once set, read-only verbs (ls, grep, cat, git status, git log, ...) inside that tree " + "auto-run without prompting — the safe-verb short-circuit treats the directory as a safe space. " + "Mutating commands still prompt, but the prompt shows the right cwd so persisted approvals are " + @@ -33,7 +34,7 @@ public sealed partial class SetWorkingDirectoryTool : NetclawTool private readonly ShellExecutionEnvironment _environment; public record Params( - [property: Description("The shell command to execute")] string Command, - [property: Description("Working directory to run the command in (optional)")] string? WorkingDirectory = null); + [param: Description("The shell command to execute.")] string Command, + [param: Description( + "Run the command in this directory. Prefer this argument to an inline cd. Omit it to use the session project or scratch directory.")] + string? WorkingDirectory = null); public ShellTool(ToolConfig config, ToolPathPolicy pathPolicy, ShellCommandPolicy commandPolicy) { diff --git a/src/Netclaw.Configuration.Tests/FileSystemPromptProviderAudienceTests.cs b/src/Netclaw.Configuration.Tests/FileSystemPromptProviderAudienceTests.cs index 9f816a86d..cbf5e6b03 100644 --- a/src/Netclaw.Configuration.Tests/FileSystemPromptProviderAudienceTests.cs +++ b/src/Netclaw.Configuration.Tests/FileSystemPromptProviderAudienceTests.cs @@ -70,6 +70,20 @@ public void Team_and_Personal_audience_get_full_agents_with_all_sections(TrustAu Assert.Contains("Skill Loading", prompt); } + [Fact] + public void Personal_rules_prefer_typed_shell_working_directory_and_retry_failed_project_scope() + { + var prompt = _provider.GetSystemPrompt(TrustAudience.Personal); + + Assert.Contains("`WorkingDirectory` argument", prompt); + Assert.Contains("Do not prefix the command with an inline `cd`", prompt); + Assert.Contains("before the first shell", prompt); + Assert.Contains("Do not repeat it when", prompt); + Assert.Contains("changing directory is itself behavior", prompt); + Assert.Contains("correct the path and retry the tool", prompt); + Assert.Contains("Do not continue with a stale directory", prompt); + } + [Fact] public void Public_audience_does_not_include_tooling() { diff --git a/src/Netclaw.Configuration/Resources/AGENTS.md b/src/Netclaw.Configuration/Resources/AGENTS.md index cda99e4d0..a74fb61e0 100644 --- a/src/Netclaw.Configuration/Resources/AGENTS.md +++ b/src/Netclaw.Configuration/Resources/AGENTS.md @@ -30,8 +30,10 @@ deeper paths, so a future `find /home/user/repo/.netclaw` is auto-allowed. where the agent will run multiple commands without explicit path arguments — typical interactive REPL work, `git status` followed by `git diff` followed by edits, or `make build` and similar tools that -hide their target behind flags (`make -C`, `git -C`). In those cases -call `set_working_directory ` so the safe-verb short-circuit +hide their target behind flags (`make -C`, `git -C`). When the user names +that project, call `set_working_directory ` before the first shell +command. Do not repeat it when `[working-context]` already names the right +`project_dir`. The safe-verb short-circuit then treats that tree as a safe space; the agent's read-only verbs auto-run with no prompt. @@ -51,12 +53,22 @@ one-shot lookups against external APIs. Calling `set_working_directory` preemptively without a project signal is its own kind of noise. +For one shell call in a named directory, set the `shell_execute` +`WorkingDirectory` argument. Do not prefix the command with an inline `cd`. +Inline `cd` changes control flow. In `cd && A; B`, command `B` can run +after a failed `cd`, so approval analysis cannot use the requested directory. +Keep inline `cd` only when changing directory is itself behavior that the user +asked you to run or test. + **Recovery from a denied shell call.** If `shell_execute` fails with a denial that mentions cwd being outside the safe spaces, the result includes a hint pointing at `set_working_directory `. Read the hint, call the tool with the directory the user is asking about, then retry the original shell call — do not re-prompt the user. +If `set_working_directory` rejects a path, correct the path and retry the tool. +Do not continue with a stale directory or use an inline `cd` as a workaround. + ## Native Shell Syntax The `[working-context]` block names the exact shell executable, grammar, and