diff --git a/.github/workflows/demo_smoke.yml b/.github/workflows/demo_smoke.yml index 430e183ef..09ac25307 100644 --- a/.github/workflows/demo_smoke.yml +++ b/.github/workflows/demo_smoke.yml @@ -41,7 +41,7 @@ jobs: - uses: actions/checkout@v7 - name: Setup .NET - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: global.json diff --git a/.github/workflows/pr_validation.yml b/.github/workflows/pr_validation.yml index e7edd4c41..666ac34e7 100644 --- a/.github/workflows/pr_validation.yml +++ b/.github/workflows/pr_validation.yml @@ -47,7 +47,7 @@ jobs: fetch-depth: 0 - name: "Install .NET SDK" - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: "./global.json" @@ -158,7 +158,7 @@ jobs: fetch-depth: 0 - name: "Install .NET SDK" - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: "./global.json" @@ -184,6 +184,10 @@ jobs: with: fetch-depth: 1 + - name: "Install shell integration test dependencies" + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install --yes fish zsh + - name: "install.sh smoke test" if: runner.os != 'Windows' shell: bash @@ -194,6 +198,11 @@ jobs: shell: pwsh run: ./scripts/smoke/install-smoke.ps1 + - name: "install.ps1 Windows PowerShell 5.1 smoke test" + if: runner.os == 'Windows' + shell: powershell + run: ./scripts/smoke/install-smoke.ps1 + semver-conformance: name: SemVer Conformance runs-on: ubuntu-latest diff --git a/.github/workflows/publish_release_binaries.yml b/.github/workflows/publish_release_binaries.yml index 085d329f8..edfc903db 100644 --- a/.github/workflows/publish_release_binaries.yml +++ b/.github/workflows/publish_release_binaries.yml @@ -126,7 +126,7 @@ jobs: fetch-depth: 0 - name: Install .NET SDK - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: "./global.json" diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index a4cd18b4c..618934ddd 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -62,7 +62,7 @@ jobs: uses: actions/checkout@v7 - name: Install .NET SDK - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: ./global.json diff --git a/.github/workflows/validate_docker_image.yml b/.github/workflows/validate_docker_image.yml index 59b0049a2..7459c9b24 100644 --- a/.github/workflows/validate_docker_image.yml +++ b/.github/workflows/validate_docker_image.yml @@ -57,7 +57,7 @@ jobs: fetch-depth: 1 - name: "Install .NET SDK" - uses: actions/setup-dotnet@v5 + uses: actions/setup-dotnet@v6 with: global-json-file: "./global.json" diff --git a/Directory.Build.props b/Directory.Build.props index 4027b2aba..332adb7db 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -9,7 +9,7 @@ enable true 0.25.0 - alpha.onnx.7 + alpha.onnx.8 - Preserve Git working context across sessions and subagents — A bounded, audience-aware Git working-context snapshot stays current in the system prompt, and coding subagents inherit recent-file/project context (#1630) - User-written AGENTS.md for application-specific agent guidance — Operators can author ~/.netclaw/identity/AGENTS.md, layered after Netclaw's embedded operating core and inherited by sub-agents (#1622) - Memory curation no longer overwrites existing documents on collision — Create-decision collisions now append new content instead of silently overwriting the existing document (#1637) diff --git a/README.md b/README.md index fdff5673b..b98f73e20 100644 --- a/README.md +++ b/README.md @@ -100,8 +100,14 @@ curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- --channel beta # Pin a specific version (e.g. a prerelease) NETCLAW_VERSION=0.17.1 curl -sSL https://releases.netclaw.dev/install.sh | bash + +# Install without modifying your shell profile +curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- --skip-shell ``` +By default, the Unix installer updates the detected Bash, zsh, or fish startup +configuration so new shells include Netclaw on `PATH`. + **macOS** (Apple Silicon — M1 or later — installs CLI + daemon to `~/.netclaw/bin`): ```bash @@ -119,8 +125,9 @@ available on macOS ([#1015](https://github.com/netclaw-dev/netclaw/issues/1015)) iwr -useb https://releases.netclaw.dev/install.ps1 | iex ``` -The `-Component cli|daemon`, `-Channel beta`, and `-Version` options work the same -way as their Linux counterparts (download the script and run it with the flag). +The installer adds Netclaw to your User PATH. The `-Component cli|daemon`, +`-Channel beta`, `-Version`, and `-SkipShell` options work the same way as their +Linux counterparts (download the script and run it with the flag). **Docker** (multi-arch: amd64/arm64): diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index bea954dcf..a432dec1f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,125 @@ # NetClaw Release Notes +## 0.25.0-alpha.onnx.8 (2026-07-18) + +> Experimental ONNX local-embeddings build. Syncs `feature/memory-embeddings` with `dev` @ +> **0.25.0 stable**. Everything still rides `Memory.Embeddings.Enabled`, off by default; +> install only by exact pin (`NETCLAW_VERSION=0.25.0-alpha.onnx.8`). + +### Synced from dev +- **Daemon shutdown fixes** — `netclaw daemon stop` now bounds the session-drain timeout, adds a CLI force-kill grace window, and keeps the generated systemd unit's `TimeoutStopSec=` in lockstep with the daemon's own graceful-shutdown budget ([#1673](https://github.com/netclaw-dev/netclaw/pull/1673)). **Operators: re-run `netclaw daemon install` after upgrading so the regenerated unit picks up `TimeoutStopSec=230`.** +- **Automated shell PATH integration for installers** — Unix and Windows installers now automatically update the shell profile or PATH registry on install ([#1687](https://github.com/netclaw-dev/netclaw/pull/1687)) +- **MCP arguments shown in approval prompts** — approval prompts now display full MCP tool arguments for better operator context ([#1689](https://github.com/netclaw-dev/netclaw/pull/1689)) +- **CLI reliability fixes** — unresolved model references and model migration validation errors are now reported cleanly instead of crashing the CLI ([#1680](https://github.com/netclaw-dev/netclaw/pull/1680), [#1678](https://github.com/netclaw-dev/netclaw/pull/1678)) +- **Security** — Pinned `Microsoft.OpenApi` to 2.7.5, addressing CVE-2026-49451 ([#1543](https://github.com/netclaw-dev/netclaw/pull/1543)) +- Routine dependency bumps synced from dev (OpenAI SDK 2.12, Microsoft.Extensions.AI 10.8.0, Akka 1.5.70, others) + +### This onnx build only +- **Curation LLM timeout raised 10s → 60s** ([#1679](https://github.com/netclaw-dev/netclaw/pull/1679)) — first onnx build carrying this + +## 0.25.0 (2026-07-18) + +This stable release concludes the 0.25.0 beta cycle (five beta releases from 0.25.0-beta.1 through beta.5) and adds a round of final polish focused on installation, CLI reliability, and daemon shutdown robustness. + +### Features +- **Automated shell PATH integration for installers** — Unix and Windows installers now automatically update the user's shell profile or PATH registry on install. Unix installers detect the active shell (bash, zsh, fish) and source a self-guarding `~/.netclaw/env` script from the correct RC file with duplicate prevention. Windows installers modify the User-scope PATH and broadcast `WM_SETTINGCHANGE`. Use `--skip-shell` (`-SkipShell` on Windows) to opt out ([#1687](https://github.com/netclaw-dev/netclaw/pull/1687)) +- **Timestamped HMAC verification for webhooks** — Webhook routes now verify request timestamps alongside HMAC signatures to prevent replay attacks, with configurable HMAC algorithm support ([#1660](https://github.com/netclaw-dev/netclaw/pull/1660)) +- **TSV support in content scanner** — Added `text/tab-separated-values` as a recognized MIME type, allowing TSV files to be scanned and processed by the content pipeline ([#1645](https://github.com/netclaw-dev/netclaw/pull/1645)) +- **Preserve Git working context across sessions and subagents** — A bounded, audience-aware Git working-context snapshot (branch, worktree, repository, upstream, changed files) now stays current in the system prompt, and coding subagents inherit recent-file/project context so parent sessions merge back only confirmed successful child edits. Measured 20% → 100% success rate on a linked-worktree coding eval ([#1630](https://github.com/netclaw-dev/netclaw/pull/1630)) +- **User-written `AGENTS.md` for application-specific agent guidance** — Operators can now author `~/.netclaw/identity/AGENTS.md`, layered after NetClaw's embedded operating core and inherited by sub-agents, to give the running agent deployment-specific mission and workflow guidance. Seeded with a minimal scaffold during init without overwriting existing guidance ([#1622](https://github.com/netclaw-dev/netclaw/pull/1622)) +- **Discord DM reminder delivery** — Reminders can now be delivered to Discord DMs via improved `DiscordReminderTargetResolver` ([#1609](https://github.com/netclaw-dev/netclaw/pull/1609)) +- **Named model configuration & provider runtime validation** — New `NamedModelConfiguration` and `ProviderRuntimeValidation` types, config schema updates, and CLI wizard improvements for provider/model setup ([#1610](https://github.com/netclaw-dev/netclaw/pull/1610)) +- **SkillServer native sub-agent sync** — Optional native manifest sidecar sync for server-managed sub-agents, keeping RFC skill sync primary while downloading verified native artifacts when available. Local sub-agent files load before server-feed files so user-authored definitions always win ([#1539](https://github.com/netclaw-dev/netclaw/pull/1539)) +- **GitHub Enterprise Copilot support** — Authenticate GitHub Copilot tokens against GHE instances and route requests to the correct data residency endpoint ([#1509](https://github.com/netclaw-dev/netclaw/pull/1509), [#1512](https://github.com/netclaw-dev/netclaw/pull/1512), [#1555](https://github.com/netclaw-dev/netclaw/pull/1555)) +- **Slack native processing status** — Real-time processing indicators in Slack instead of generic "working" messages ([#1524](https://github.com/netclaw-dev/netclaw/pull/1524)) +- **Reminder failure visibility** — Operators can now see when reminders fail or are skipped ([#1503](https://github.com/netclaw-dev/netclaw/pull/1503)) +- **Degraded startup mode** — Daemon starts with a "no valid model" banner when no provider is configured, instead of failing host startup ([#1540](https://github.com/netclaw-dev/netclaw/pull/1540)) +- **Synced skill resources via shell** — Skill resources synced from the cloud can now execute via shell commands ([#1551](https://github.com/netclaw-dev/netclaw/pull/1551)) +- **DwarfStar (ds4) provider support** — New openai-compatible backend strategy supporting DwarfStar models ([#1349](https://github.com/netclaw-dev/netclaw/pull/1349)) +- **Inherit embedded AGENTS.md for sub-agents** — Sub-agents now inherit the parent session's embedded operating rules from AGENTS.md ([#1490](https://github.com/netclaw-dev/netclaw/pull/1490)) +- **Show advertised skill count for remote skill servers** — The Skill Sources config screen now displays how many skills each remote server advertises ([#1452](https://github.com/netclaw-dev/netclaw/pull/1452)) + +### Bug Fixes +- **MCP arguments shown in approval prompts** — Fixed: approval prompts now display full MCP tool arguments for better operator context ([#1689](https://github.com/netclaw-dev/netclaw/pull/1689)) +- **CLI reports unresolved model references cleanly** — Fixed: CLI no longer crashes with opaque errors when a model reference cannot be resolved ([#1680](https://github.com/netclaw-dev/netclaw/pull/1680)) +- **CLI handles model migration errors without crashing** — Fixed: model migration validation errors are now reported gracefully instead of crashing the CLI ([#1678](https://github.com/netclaw-dev/netclaw/pull/1678)) +- **CLI rejects numeric model modalities** — Fixed: model modality values are now validated as strings, rejecting numeric input ([#1677](https://github.com/netclaw-dev/netclaw/pull/1677)) +- **Daemon-stop session drain bounded** — Fixed: `netclaw daemon stop` now properly bounds the session drain timeout, preventing hangs on interactive-tool-paused sessions and giving the CLI adequate headroom ([#1673](https://github.com/netclaw-dev/netclaw/pull/1673)) +- **Reminders rescan definitions before startup alerts** — Fixed: startup alerts now fire only after reminder definitions are fully loaded, preventing missed or duplicate reminders on restart ([#1653](https://github.com/netclaw-dev/netclaw/pull/1653)) +- **OpenAI client 2.12 compatibility** — Fixed: updated OpenAI provider plugin to work with OpenAI client 2.12+ changes ([#1654](https://github.com/netclaw-dev/netclaw/pull/1654)) +- **Attachment path guidance** — Fixed: authoritative attachment path guidance now points to the correct session media directory ([#1686](https://github.com/netclaw-dev/netclaw/pull/1686)) +- **Windows actor checks made deterministic** — Fixed: Windows actor lifecycle tests were non-deterministic ([#1661](https://github.com/netclaw-dev/netclaw/pull/1661)) +- **Ollama smoke test pinned** — Fixed: pinned Ollama installer to a specific release for smoke test stability ([#1659](https://github.com/netclaw-dev/netclaw/pull/1659)) +- **VHS process group timeout** — Fixed: full VHS process group now times out properly during smoke tests ([#1655](https://github.com/netclaw-dev/netclaw/pull/1655)) +- **Screenshot regression determinism** — Fixed: screenshot regression suite now uses pixel comparison with blank/partial retry and Termina seam fixes ([#1451](https://github.com/netclaw-dev/netclaw/pull/1451)) +- **Legacy model environment overrides serialized** — Fixed: legacy model environment overrides are now properly serialized ([#1656](https://github.com/netclaw-dev/netclaw/pull/1656)) +- **Subagent token usage tracked** — Sub-agent token usage is now properly tracked and reported ([#1597](https://github.com/netclaw-dev/netclaw/pull/1597)) +- **Subagent fail-closed for unattended approvals** — Fixed: subagents fail safely when approvals are required but no human is present ([#1616](https://github.com/netclaw-dev/netclaw/pull/1616)) +- **Memory core: curation documents stable** — Fixed: memory curation documents were unstable across writes, causing cross-session memory recall issues ([#1575](https://github.com/netclaw-dev/netclaw/pull/1575)) +- **Model set/picker preserves modalities** — Fixed: model selection UI now preserves model modalities correctly ([#1610](https://github.com/netclaw-dev/netclaw/pull/1610)) +- **Slack processing status serialization** — Fixed: Slack processing status now serializes correctly for all states ([#1556](https://github.com/netclaw-dev/netclaw/pull/1556)) +- **Autonomous sessions can write workspace directory** — Fixed: autonomous sessions could not write to the workspace directory ([#1498](https://github.com/netclaw-dev/netclaw/pull/1498)) +- **Reminder duplicate-execution guard** — Fixed: Mode A reminder session wedge prevented the duplicate guard from releasing ([#1500](https://github.com/netclaw-dev/netclaw/pull/1500)) +- **Reset stops daemon first** — Fixed: `netclaw reset` now stops the daemon before proceeding and shows a progress screen ([#1494](https://github.com/netclaw-dev/netclaw/pull/1494)) +- **MCP tool errors display cleanly** — Fixed: MCP errors now show as attributed messages instead of raw JSON dumps ([#1510](https://github.com/netclaw-dev/netclaw/pull/1510)) +- **TUI: auto-advance the add-skill-server flow** — Fixed: skill server probe flow now auto-advances on successful connection ([#1458](https://github.com/netclaw-dev/netclaw/pull/1458)) +- **TUI: auto-start init health checks** — Fixed: init health checks now start automatically ([#1454](https://github.com/netclaw-dev/netclaw/pull/1454)) +- **TUI: discoverable Done rows** — Added "Done" back-out rows across Security & Access menus and config screens ([#1448](https://github.com/netclaw-dev/netclaw/pull/1448), [#1441](https://github.com/netclaw-dev/netclaw/pull/1441)) +- **TUI: session browser selection highlight** — Fixed: session browser now shows selection highlight ([#1531](https://github.com/netclaw-dev/netclaw/pull/1531)) +- **TUI: identity redo timezone loop** — Fixed: identity redo would loop on timezone; also fixed session browser regression ([#1518](https://github.com/netclaw-dev/netclaw/pull/1518)) +- **TUI: flaky host crash during reset** — Fixed: thread-unsafe ReactiveProperty access during reset caused crashes ([#1525](https://github.com/netclaw-dev/netclaw/pull/1525)) +- **Subagent terminal result summaries** — Fixed: subagent terminal results now display properly ([#1519](https://github.com/netclaw-dev/netclaw/pull/1519)) +- **UTF-8 BOM in skill frontmatter** — Fixed: skill scanner now strips UTF-8 BOM before parsing YAML frontmatter ([#1583](https://github.com/netclaw-dev/netclaw/pull/1583)) +- **Block system and external skill mutations** — Fixed: skill system now blocks mutations to system and externally-synced skills ([#1457](https://github.com/netclaw-dev/netclaw/pull/1457)) +- **Self-monitoring spawn_agent liveness respected** — Fixed: tool pipeline now respects self-monitoring spawn_agent liveness ([#1456](https://github.com/netclaw-dev/netclaw/pull/1456)) + +### Breaking Changes +- **Removed silent local-ollama fallback** — Users without any provider configured will now see a "no valid model" banner instead of an automatic fallback to local Ollama. Explicit provider configuration is now required for full functionality ([#1540](https://github.com/netclaw-dev/netclaw/pull/1540)) + +### Improvements +- **Tool execution pipeline refactoring** — Restructured the session tool execution pipeline with isolated invocation scope, composed execution pipeline, typed sub-agent context isolation, and proper lifecycle cleanup ([#1641](https://github.com/netclaw-dev/netclaw/pull/1641), [#1643](https://github.com/netclaw-dev/netclaw/pull/1643), [#1644](https://github.com/netclaw-dev/netclaw/pull/1644), [#1646](https://github.com/netclaw-dev/netclaw/pull/1646)) +- **Logical skill access and authoritative inventory refresh** — Skill loading now resolves through logical `skill_load`/`skill_read_resource` access with native > managed-feed > external precedence. Startup, sync, watcher, and `skill_manage` inventory rebuilds centralized through one live-source refresher ([#1634](https://github.com/netclaw-dev/netclaw/pull/1634)) +- **Session actor decomposed into transient-state handlers** — `LlmSessionActor` decomposed into smaller, focused handlers for improved maintainability and testability ([#1496](https://github.com/netclaw-dev/netclaw/pull/1496)) +- **Log stream partitioned by session** — Each session now gets its own `session.log` while `daemon.log` remains sparse. Cleaner logs and better observability with OTEL union support ([#1499](https://github.com/netclaw-dev/netclaw/pull/1499)) +- **Memory core redesign** — Shared curation evaluator unified across both memory write pipelines so they can never diverge. July 2026 audit: revived curation LLM, balanced prompt, and recall precision re-tune ([#1575](https://github.com/netclaw-dev/netclaw/pull/1575), [#1568](https://github.com/netclaw-dev/netclaw/pull/1568)) + +### Security +- **Pinned Microsoft.OpenApi to 2.7.5** — Addresses CVE-2026-49451 ([#1543](https://github.com/netclaw-dev/netclaw/pull/1543)) +- **Suppressed GHSA-2m69-gcr7-jv3q (SQLitePCLRaw CVE-2025-6965)** — Temporarily suppressed until upstream patches available. Tracking: dotnet/efcore#38257 ([#1444](https://github.com/netclaw-dev/netclaw/pull/1444)) + +### Dependency Updates +- **OpenAI SDK** — Updated to 2.12 (required for API compatibility) ([#1654](https://github.com/netclaw-dev/netclaw/pull/1654)) +- **Microsoft.Extensions.AI** — 10.6.0 → 10.8.0 ([#1650](https://github.com/netclaw-dev/netclaw/pull/1650)) +- **Microsoft.AspNetCore.DataProtection** — 10.0.9 → 10.0.10 ([#1657](https://github.com/netclaw-dev/netclaw/pull/1657)) +- **Microsoft.NET.Test.Sdk** — 18.7.0 → 18.8.1 ([#1651](https://github.com/netclaw-dev/netclaw/pull/1651)) +- **Mattermost.NET** — 5.0.0 → 5.0.3 ([#1626](https://github.com/netclaw-dev/netclaw/pull/1626)) +- **SkillServer** — `Netclaw.SkillClient` 0.4.0-beta.4 → 0.4.0 (stable) ([#1638](https://github.com/netclaw-dev/netclaw/pull/1638)) +- **Anthropic SDK** — 12.30.0 → 12.35.1 ([#1488](https://github.com/netclaw-dev/netclaw/pull/1488), [#1561](https://github.com/netclaw-dev/netclaw/pull/1561)) +- **Akka** — Akka.Cluster.Sharding and Akka.Persistence 1.5.69 → 1.5.70 ([#1560](https://github.com/netclaw-dev/netclaw/pull/1560)) +- **Testcontainers** — 4.12.0 → 4.13.0 ([#1563](https://github.com/netclaw-dev/netclaw/pull/1563)) +- **YamlDotNet** — 18.0.0 → 18.1.0 ([#1516](https://github.com/netclaw-dev/netclaw/pull/1516)) +- **Verify.XunitV3** — 31.19.1 → 31.20.0 ([#1446](https://github.com/netclaw-dev/netclaw/pull/1446)) + +## 0.25.0-beta.5 (2026-07-16) + +### Features +- **Timestamped HMAC verification for webhooks** — Webhook routes now verify request timestamps alongside HMAC signatures to prevent replay attacks, with configurable HMAC algorithm support ([#1660](https://github.com/netclaw-dev/netclaw/pull/1660)) +- **TSV support in content scanner** — Added `text/tab-separated-values` as a recognized MIME type, allowing TSV files to be scanned and processed by the content pipeline ([#1645](https://github.com/netclaw-dev/netclaw/pull/1645)) + +### Bug Fixes +- **Reminders rescan definitions before startup alerts** — Fixed: startup alerts could fire before reminder definitions were fully loaded, causing missed or duplicate reminders on restart ([#1653](https://github.com/netclaw-dev/netclaw/pull/1653)) +- **OpenAI client 2.12 compatibility** — Updated OpenAI provider plugin to work with the breaking API change in OpenAI SDK 2.12 (`OpenAIClientOptions` → `ResponsesClientOptions`) ([#1654](https://github.com/netclaw-dev/netclaw/pull/1654)) + +### Improvements +- **Tool execution pipeline refactoring** — Restructured the session tool execution pipeline with isolated invocation scope, composed execution pipeline, typed subagent context isolation, and proper lifecycle cleanup. Fixes subagent fatal spawn failures ([#1641](https://github.com/netclaw-dev/netclaw/pull/1641), [#1643](https://github.com/netclaw-dev/netclaw/pull/1643), [#1644](https://github.com/netclaw-dev/netclaw/pull/1644), [#1646](https://github.com/netclaw-dev/netclaw/pull/1646)) + +### Dependency Updates +- **Bump OpenAI client to 2.12** — Required for compatibility with the updated SDK API ([#1654](https://github.com/netclaw-dev/netclaw/pull/1654)) +- **Bump Microsoft.Extensions.AI** — 10.6.0 → 10.8.0 ([#1650](https://github.com/netclaw-dev/netclaw/pull/1650)) +- **Bump Microsoft.AspNetCore.DataProtection** — 10.0.9 → 10.0.10 ([#1657](https://github.com/netclaw-dev/netclaw/pull/1657)) +- **Bump Microsoft.NET.Test.Sdk** — 18.7.0 → 18.8.1 ([#1651](https://github.com/netclaw-dev/netclaw/pull/1651)) +- **Bump Mattermost.NET** — 5.0.0 → 5.0.3 ([#1626](https://github.com/netclaw-dev/netclaw/pull/1626)) + ## 0.25.0-alpha.onnx.7 (2026-07-16) > Experimental ONNX local-embeddings build. Syncs `feature/memory-embeddings` with `dev` diff --git a/docs/spec/SPEC-004-cli-contract.md b/docs/spec/SPEC-004-cli-contract.md index 259d6b219..819e04866 100644 --- a/docs/spec/SPEC-004-cli-contract.md +++ b/docs/spec/SPEC-004-cli-contract.md @@ -100,6 +100,9 @@ Behavior: - optional output: JSON for automation - exit code `0` for success - exit code `1` for validation, policy, or runtime failures +- expected model-configuration failures, including migration and named-role resolution errors, + are validation failures: print actionable output, return exit code `1`, and do not create crash + logs or emit stack traces - exit code `2` for usage and argument errors ## Safety Rules diff --git a/docs/spec/SPEC-011-daemon-architecture.md b/docs/spec/SPEC-011-daemon-architecture.md index 3ca232b78..4218d7773 100644 --- a/docs/spec/SPEC-011-daemon-architecture.md +++ b/docs/spec/SPEC-011-daemon-architecture.md @@ -225,7 +225,8 @@ The daemon installs process-level exception handlers at startup for: - `AppDomain.CurrentDomain.UnhandledException` - `TaskScheduler.UnobservedTaskException` -On either path, Netclaw writes a crash log under `~/.netclaw/logs/crash-*.log` +On either path, Netclaw writes a crash log under `/logs/crash-*.log`; +`NETCLAW_HOME` defaults to `~/.netclaw` with process diagnostics and the latest known session/turn context. When DI is available, the daemon also emits an operational alert with type `daemon.crashing` (category `DaemonCrashed`) so configured webhook targets can diff --git a/evals/README.md b/evals/README.md index 5cd5fce02..8f5707419 100644 --- a/evals/README.md +++ b/evals/README.md @@ -72,7 +72,7 @@ log patterns** (skill loading, memory recall, checkpoint formation). | Skill Auto-Loading | 4 | Keyword matching triggers correct skills | | Memory Pipeline | 4 | Memory recall is active, identity-vs-memory routing is correct, explicit saves use memory tools, and automatic checkpointing still fires | | Tool Discovery & Use | 9 | Progressive tool discovery and invocation, including timestamped webhook configuration | -| Grounding & Alignment | 3 | Uses tools to verify facts, admits uncertainty | +| Grounding & Alignment | 4 | Uses tools to verify facts, admits uncertainty, and resolves announced attachment paths from the authoritative session root | | Autonomy & Execution | 2 | Executes tasks rather than describing them | | Deployment Mission | 1 | Applies the disk mission playbook, loads its required skill, and returns reviewed sales email | | Subagents | 2 | Delegates through `spawn_agent`, completes ambiguous work, and gives specialized subagent guidance precedence over a conflicting deployment playbook | diff --git a/evals/run-evals.sh b/evals/run-evals.sh index 7bde30b02..94e011e35 100755 --- a/evals/run-evals.sh +++ b/evals/run-evals.sh @@ -1231,6 +1231,19 @@ assert_grounding_action_verification() { stdout_contains '\[tool:call\] set_reminder' } +assert_grounding_attachment_path() { + stdout_response_contains '/home/netclaw/\.netclaw/sessions/.*/inbox/image_1\.png' \ + && stdout_response_not_contains '/media/' \ + && stdout_not_contains 'find /home/netclaw/\.netclaw/sessions' +} + +setup_grounding_attachment_path() { + local run="$1" + local session_dir="/home/netclaw/.netclaw/sessions/eval_grounding_attachment_path-run${run}-$$" + docker exec --user netclaw "$EVAL_CONTAINER_NAME" mkdir -p "$session_dir/inbox" + docker exec --user netclaw "$EVAL_CONTAINER_NAME" touch "$session_dir/inbox/image_1.png" +} + # Category 6: Autonomy & Execution assert_autonomy_execute() { stdout_contains '\[tool:call\] shell_execute' @@ -1817,6 +1830,9 @@ run_all() { run_case grounding_action_verification "set_reminder called" \ "Schedule a reminder to check email in 10 minutes" + run_multi_turn_case grounding_attachment_path "resolves the announced inbox path without searching other sessions" \ + "An uploaded image was announced as [attachment] name=\"image.png\" path=\"inbox/image_1.png\". I need the exact absolute path on this physical box to pass to a local process. Reply with only that path." + end_category # ── Category 6: Autonomy & Execution ── diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index 1a52a360f..5c24859c3 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.32.0" + version: "2.36.0" --- # Netclaw Operations @@ -115,6 +115,19 @@ or MCP tools by capability before concluding a tool doesn't exist. Full guidance ## Approval Prompts +MCP approval prompts show a bounded, redacted preview of the call arguments. +Actual path- and URL-shaped values appear first and receive a larger preview so +the operator can verify location context without guessing from argument names. +URL credentials, query values, and fragments are redacted. +Large strings, binary data, and nested collections are summarized by size; +secret-like fields and token-shaped values are always redacted. Argument names +and values are escaped before display so server-controlled schema text cannot +break or spoof the approval prompt. MCP grants are tool-wide rather than +directory-scoped, so these prompts omit the misleading `Always here` option and +label the persistent choice `Always allow this tool` rather than the +shell-oriented `Always anywhere`. Other non-shell tools also omit `Always here` +because their approval matchers do not consume directory scope. + Approvals are typed `(verb, directory)` pairs in `tool-approvals.json`: - **verb** — the command head plus subcommand chain only (e.g. `git push`, diff --git a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md index 440856bb3..cca166e3c 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md +++ b/feeds/skills/.system/files/netclaw-operations/references/diagnostics.md @@ -11,8 +11,8 @@ When something seems wrong with Netclaw itself: 2. If doctor reports fixable issues, run `netclaw doctor --fix --dry-run` to preview auto-repairs (schema-driven: stale properties, enum coercion, missing defaults) 3. Run `netclaw status` via `shell_execute` — live runtime state from daemon -3. Check daemon logs at `~/.netclaw/logs/daemon-{yyyy-MM-dd}.log` -4. Check session logs at `~/.netclaw/logs/sessions/{sanitized-session-id}/session.log` +3. Check daemon logs at `/logs/daemon-{yyyy-MM-dd}.log` (`NETCLAW_HOME` defaults to `~/.netclaw`) +4. Check session logs at `/logs/sessions/{sanitized-session-id}/session.log` If `netclaw status` or `netclaw chat` prints `daemon not configured - please run netclaw init`, do not troubleshoot daemon reachability or model defaults. The @@ -78,7 +78,7 @@ debugging a daemon-wide problem → read `daemon.log`. | Missing tools | `netclaw mcp list`; check MCP connection state | | Memory recall degraded | `netclaw status` memory section | | Memory embedding/relevance model unavailable | Fires a `memory.embedding_model.unavailable` / `memory.relevance_model.unavailable` operational alert (once per model per daemon run) naming the model, failure reason, and consequence when `Memory.Embeddings.Enabled=true` and either ONNX model fails to provision or load; see `netclaw-memory`'s Embeddings section and `netclaw doctor`'s Memory Embeddings / Memory Relevance Gate checks | -| Daemon won't start | crash logs at `~/.netclaw/logs/crash-*.log` | +| Daemon won't start | crash logs at `/logs/crash-*.log` (`NETCLAW_HOME` defaults to `~/.netclaw`) | | Docker daemon cannot create `/home/netclaw/.netclaw/*` | Official image entrypoint repairs writable bind mounts to UID/GID `1654:1654`; if bypassed or read-only, run `sudo chown -R 1654:1654 ` or use a Docker named volume | | Discord/Slack channel offline | `netclaw status` shows the channel `disconnected` with a reason. Discord may also report `degraded` when Discord.Net says the socket is connected but the gateway is not ready, such as after a resumed session that Netclaw is replacing with a clean reconnect. A misconfigured channel (bad token, missing Discord Message Content intent) degrades only that channel — the daemon keeps running and other channels are unaffected. A transient network failure retries automatically; a config/permission failure stays offline until the operator fixes the config and restarts the daemon. | | `command not found` for `netclaw`/`dotnet`/a user tool from the shell tool when the daemon runs as a systemd service | The systemd `--user` service does not inherit your login-shell `PATH`; `netclaw daemon install` captures it into `~/.netclaw/config/daemon.env`. Run `netclaw doctor` (the **Systemd Unit PATH** check flags a missing/stale/legacy env file), then `netclaw doctor --fix` to rehydrate `PATH` from your current shell (or re-run `netclaw daemon install`), and finally `systemctl --user restart netclaw`. Installed a new tool after install? Its dir won't be seen until you re-run one of those and restart. Per-directory managers (`mise`/`asdf`/`direnv`) are not captured. | diff --git a/feeds/skills/.system/files/netclaw-operations/references/providers.md b/feeds/skills/.system/files/netclaw-operations/references/providers.md index 8930557b3..ca3976ba6 100644 --- a/feeds/skills/.system/files/netclaw-operations/references/providers.md +++ b/feeds/skills/.system/files/netclaw-operations/references/providers.md @@ -35,18 +35,22 @@ options instead of adding provider-specific properties to `ProviderEntry`. ### Degraded mode: No-Op chat client -When Netclaw starts without an explicitly configured main model/provider -(no `Models.Roles.Main`, an unresolved definition, no `Providers`, or the selected definition -points to a provider that is not configured), the daemon launches in -**degraded mode** with a No-Op chat client. Bound defaults such as -`local-ollama/qwen3:30b` do not count as operator configuration unless those -fields are actually present in config. Every chat turn returns a fixed -configuration banner beginning with `"No valid model configuration detected."` -and listing recovery steps. If no provider is configured, send the operator -through `netclaw init`; it configures both a provider and main model. If a -provider already exists but the main model is missing or points to the wrong -provider name, use `netclaw model`. Manual repair means editing `netclaw.json` -/ `secrets.json` and restarting the daemon. +When Netclaw starts without an explicitly configured main model/provider (no +`Models` configuration, no `Providers`, or the selected definition points to a +provider that is not configured), the daemon launches in **degraded mode** with +a No-Op chat client. Bound defaults such as `local-ollama/qwen3:30b` do not +count as operator configuration unless those fields are actually present in +config. Every chat turn returns a fixed configuration banner beginning with +`"No valid model configuration detected."` and listing recovery steps. If no +provider is configured, send the operator through `netclaw init`; it configures +both a provider and main model. If a provider already exists but the main model +is missing or points to the wrong provider name, use `netclaw model`. Manual +repair means editing `netclaw.json` / `secrets.json` and restarting the daemon. + +A role that names a definition absent from `Models.Definitions` is malformed +configuration, not degraded mode. `netclaw model list` and `netclaw doctor` +report the exact role and missing definition. Repair the role manually or use +`netclaw model set`; `netclaw doctor --fix` does not guess a replacement. If the operator reports seeing that banner, do not troubleshoot model behavior; the daemon has no working provider. Direct them through the recovery steps and @@ -103,9 +107,10 @@ definition, including adding a property the definition deliberately omits. To change a preserved value you must pass the corresponding flag (a plain re-set will not touch it). A legacy or hand-edited entry with an unreadable value does not block a re-set — `model set` migrates legacy inline roles to named -definitions and repairs the selected entry while keeping the fields -it can still read; `model list` reports an unparseable config instead of -crashing, and `netclaw doctor --fix` repairs it. +definitions and repairs the selected entry while keeping the fields it can +still read. `model list` reports an unparseable config instead of crashing. +`netclaw doctor --fix` applies only repairs it can derive safely; it does not +invent missing named definitions or role assignments. ### Adding GitHub Copilot diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 5db509d7f..3633a3d86 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -7,6 +7,7 @@ # .\install.ps1 -InstallDir C:\tools\netclaw # .\install.ps1 -Channel beta # Opt into prereleases # .\install.ps1 -DryRun +# .\install.ps1 -SkipShell # Don't modify PATH # # -Channel beta installs the newest prerelease (or latest stable if no prerelease # exists). -Version pins an exact version and overrides -Channel (e.g. 0.19.0-beta.1). @@ -24,11 +25,37 @@ param( [string]$Channel = "stable", # Resolve and report what would be installed, but install nothing. - [switch]$DryRun + [switch]$DryRun, + + # Skip automatic PATH modification. + [switch]$SkipShell ) $ErrorActionPreference = "Stop" +function Remove-TrailingDirectorySeparators { + param([string]$Path) + + if ([string]::IsNullOrEmpty($Path)) { + return $Path + } + + $root = [System.IO.Path]::GetPathRoot($Path) + if ($Path -eq $root) { + return $Path + } + + $separators = [char[]]@( + [System.IO.Path]::DirectorySeparatorChar, + [System.IO.Path]::AltDirectorySeparatorChar) + $trimmed = $Path.TrimEnd($separators) + if ([string]::IsNullOrEmpty($trimmed) -and -not [string]::IsNullOrEmpty($root)) { + return $root + } + + return $trimmed +} + function Invoke-DownloadWithProgress { param( [string]$Uri, @@ -94,6 +121,11 @@ if (-not $InstallDir) { $InstallDir = $DefaultInstallDir } +$InstallDir = [System.IO.Path]::GetFullPath($InstallDir) +if ($InstallDir.Contains(';') -or $InstallDir.Contains("`r") -or $InstallDir.Contains("`n")) { + throw "InstallDir cannot contain semicolons, carriage returns, or newlines when used on PATH." +} + Write-Host "Netclaw installer" Write-Host " Platform: win-x64" Write-Host " Install dir: $InstallDir" @@ -256,21 +288,126 @@ try { } } - # Check PATH and offer to add if missing + # ── Add to PATH ── Write-Host "" - $userPath = [Environment]::GetEnvironmentVariable("PATH", "User") - $pathEntries = $userPath -split ';' | ForEach-Object { $_.TrimEnd('\') } - $installDirNormalized = $InstallDir.TrimEnd('\') - if ($pathEntries -contains $installDirNormalized) { - Write-Host "Installation complete! netclaw is already on your PATH." + if (-not $SkipShell) { + $installDirNormalized = Remove-TrailingDirectorySeparators $InstallDir + + # Read the raw registry value so an existing REG_EXPAND_SZ PATH keeps both + # its %VAR% references and its registry type when we prepend Netclaw. + # HKCU is writable by the current user and does not require elevation. + # CreateSubKey opens the normal existing key and also supports minimal + # profiles where the per-user Environment key has not been created yet. + $userEnvironmentKey = [Microsoft.Win32.Registry]::CurrentUser.CreateSubKey("Environment") + if ($null -eq $userEnvironmentKey) { + throw "Cannot create or open the current user's Environment registry key for PATH update." + } + + try { + $pathValueExists = $userEnvironmentKey.GetValueNames() -contains "Path" + $userPath = if ($pathValueExists) { + $userEnvironmentKey.GetValue( + "Path", + $null, + [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames) + } else { + "" + } + + if ($null -ne $userPath -and $userPath -isnot [string]) { + throw "The current user's PATH registry value is not a string." + } + + $userPathKind = if ($pathValueExists) { + $userEnvironmentKey.GetValueKind("Path") + } else { + [Microsoft.Win32.RegistryValueKind]::String + } + if ($userPathKind -notin @( + [Microsoft.Win32.RegistryValueKind]::String, + [Microsoft.Win32.RegistryValueKind]::ExpandString)) { + throw "The current user's PATH registry value has unsupported type $userPathKind." + } + + if ($userPathKind -eq [Microsoft.Win32.RegistryValueKind]::ExpandString ` + -and $installDirNormalized.Contains('%')) { + throw "InstallDir containing '%' cannot be safely added to an expandable User PATH. Choose a directory without '%' or rerun with -SkipShell." + } + + $userPathEntries = if ([string]::IsNullOrEmpty($userPath)) { @() } else { + $userPath -split ';' | + Where-Object { -not [string]::IsNullOrEmpty($_) } | + ForEach-Object { + $expandedEntry = [Environment]::ExpandEnvironmentVariables($_) + Remove-TrailingDirectorySeparators $expandedEntry + } + } + + $userPathChanged = $false + if ($userPathEntries -notcontains $installDirNormalized) { + $newUserPath = if ([string]::IsNullOrEmpty($userPath)) { + $installDirNormalized + } else { + "$installDirNormalized;$userPath" + } + + if ($newUserPath.Length -gt 32700) { + Write-Warning "User PATH is near its 32,767 character limit ($($newUserPath.Length) chars)." + Write-Host "Please manually add $InstallDir to your User PATH." + } else { + $userEnvironmentKey.SetValue("Path", $newUserPath, $userPathKind) + $userPathChanged = $true + } + } + } finally { + $userEnvironmentKey.Dispose() + } + + $processPath = $env:PATH + $processPathEntries = if ([string]::IsNullOrEmpty($processPath)) { @() } else { + $processPath -split ';' | + Where-Object { -not [string]::IsNullOrEmpty($_) } | + ForEach-Object { Remove-TrailingDirectorySeparators $_ } + } + if ($processPathEntries -notcontains $installDirNormalized) { + $env:PATH = if ([string]::IsNullOrEmpty($processPath)) { + $installDirNormalized + } else { + "$installDirNormalized;$processPath" + } + } + + if ($userPathChanged) { + if (-not ("NetclawInstaller.NativeMethods" -as [type])) { + Add-Type -Namespace NetclawInstaller -Name NativeMethods -MemberDefinition @' + [DllImport("user32.dll", SetLastError=true, CharSet=CharSet.Auto)] + public static extern IntPtr SendMessageTimeout(IntPtr hWnd, uint msg, + UIntPtr wParam, string lParam, uint fuFlags, uint uTimeout, out UIntPtr lpdwResult); +'@ + } + + $broadcastOutput = [UIntPtr]::Zero + $broadcastResult = [NetclawInstaller.NativeMethods]::SendMessageTimeout( + [IntPtr]0xFFFF, 0x001A, [UIntPtr]::Zero, "Environment", 2, 1000, [ref]$broadcastOutput) + if ($broadcastResult -eq [IntPtr]::Zero) { + Write-Warning "User PATH was updated, but Windows did not acknowledge the environment-change notification. New terminals may require sign-out or restart." + } + } + + if ($userPathEntries -contains $installDirNormalized) { + Write-Host "Installation complete! netclaw is already on your User PATH." + } elseif ($userPathChanged) { + Write-Host "Installation complete! netclaw was added to your User PATH." + } else { + Write-Host "Installation complete! netclaw is on PATH for this terminal only." + } } else { - Write-Host "Installation complete!" + Write-Host "Installation complete! (PATH modification skipped)" Write-Host "" - Write-Host "Add Netclaw to your PATH by running:" + Write-Host "Add this directory to your User PATH using Windows Environment Variables settings:" Write-Host "" - Write-Host " `$userPath = [Environment]::GetEnvironmentVariable('PATH', 'User')" - Write-Host " [Environment]::SetEnvironmentVariable('PATH', `"$InstallDir;`$userPath`", 'User')" + Write-Host " $InstallDir" Write-Host "" Write-Host "Then restart your terminal." } diff --git a/scripts/install.sh b/scripts/install.sh index bff6252a6..7f9b4ca74 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -6,6 +6,7 @@ # curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- cli # CLI only # curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- daemon # Daemon only # curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- --channel beta # Opt into prereleases +# curl -sSL https://releases.netclaw.dev/install.sh | bash -s -- --skip-shell # Don't modify shell profile # INSTALL_DIR=/opt/netclaw curl -sSL https://releases.netclaw.dev/install.sh | bash # # Arguments: @@ -13,6 +14,7 @@ # --channel stable|beta — Release channel (default: stable). 'beta' installs the # newest prerelease (or latest stable if no prerelease exists). # --dry-run — Resolve and report what would happen; install nothing. +# --skip-shell — Skip automatic shell profile modification. # # Environment variables: # INSTALL_DIR — Install directory (default: ~/.netclaw/bin) @@ -36,9 +38,11 @@ COMPONENT="all" # "all", "cli", or "daemon" DRY_RUN=false # --dry-run: resolve and report what would happen, install nothing CHANNEL="stable" # release channel: "stable" (default) or "beta" (opt into prereleases) CHANNEL_EXPLICIT=false # true when --channel was explicitly passed +SKIP_SHELL=false # --skip-shell: don't modify shell profile while [ $# -gt 0 ]; do case "$1" in --dry-run) DRY_RUN=true; shift ;; + --skip-shell) SKIP_SHELL=true; shift ;; --channel) if [ $# -lt 2 ]; then echo "Error: --channel requires a value (stable|beta)" >&2; exit 1 @@ -46,7 +50,7 @@ while [ $# -gt 0 ]; do CHANNEL="$2"; CHANNEL_EXPLICIT=true; shift 2 ;; --channel=*) CHANNEL="${1#*=}"; CHANNEL_EXPLICIT=true; shift ;; all|cli|daemon) COMPONENT="$1"; shift ;; - *) echo "Usage: install.sh [all|cli|daemon] [--channel stable|beta] [--dry-run]" >&2; exit 1 ;; + *) echo "Usage: install.sh [all|cli|daemon] [--channel stable|beta] [--dry-run] [--skip-shell]" >&2; exit 1 ;; esac done @@ -133,11 +137,61 @@ json_field() { fi } +json_asset_field() { + local json="$1" version="$2" component="$3" rid="$4" field="$5" + + # Release manifests contain flat release and asset objects. Splitting object + # boundaries lets POSIX awk select an asset without depending on property order + # or GNU-only regular-expression extensions. + printf '%s\n' "$json" | tr '{' '\n' | tr '}' '\n' | awk \ + -v wanted_version="$version" \ + -v wanted_component="$component" \ + -v wanted_rid="$rid" \ + -v wanted_field="$field" ' + function string_field(record, name, value) { + if (index(record, "\"" name "\"") == 0) { + return "" + } + + value = record + sub(".*\"" name "\"[[:space:]]*:[[:space:]]*\"", "", value) + sub("\".*", "", value) + return value + } + + { + release_version = string_field($0, "version") + if (release_version != "") { + current_version = release_version + } + + if (current_version == wanted_version && + string_field($0, "component") == wanted_component && + string_field($0, "rid") == wanted_rid) { + print string_field($0, wanted_field) + exit + } + } + ' +} + +validate_install_dir_for_path() { + local install_dir="$1" + + # PATH uses ':' as its entry separator, and startup files are line-oriented. + # These names cannot be represented without changing their meaning. + if [[ "$install_dir" == *:* || "$install_dir" == *$'\n'* || "$install_dir" == *$'\r'* ]]; then + echo "Error: INSTALL_DIR cannot contain ':', carriage returns, or newlines when used on PATH." >&2 + return 1 + fi +} + # ── Main ── check_deps RID=$(detect_platform) INSTALL_DIR="${INSTALL_DIR:-$HOME/.netclaw/bin}" +validate_install_dir_for_path "$INSTALL_DIR" echo "Netclaw installer" echo " Platform: $RID" @@ -191,16 +245,8 @@ download_component() { url=$(echo "$MANIFEST" | jq -r ".releases[] | select(.version==\"$VERSION\") | .assets[] | select(.component==\"$component\" and .rid==\"$RID\") | .url") sha256=$(echo "$MANIFEST" | jq -r ".releases[] | select(.version==\"$VERSION\") | .assets[] | select(.component==\"$component\" and .rid==\"$RID\") | .sha256") else - # Fallback: extract URL and sha256 using grep (fragile but works for well-formed JSON) - # Find the block for this component+rid - local block - block=$(echo "$MANIFEST" | tr '\n' ' ' | grep -oP "\"component\"\\s*:\\s*\"${component}\"[^}]*\"rid\"\\s*:\\s*\"${RID}\"[^}]*}" | head -1) - if [ -z "$block" ]; then - # Try reversed order - block=$(echo "$MANIFEST" | tr '\n' ' ' | grep -oP "\"rid\"\\s*:\\s*\"${RID}\"[^}]*\"component\"\\s*:\\s*\"${component}\"[^}]*}" | head -1) - fi - url=$(echo "$block" | grep -oP '"url"\s*:\s*"\K[^"]+') - sha256=$(echo "$block" | grep -oP '"sha256"\s*:\s*"\K[^"]+') + url=$(json_asset_field "$MANIFEST" "$VERSION" "$component" "$RID" "url") + sha256=$(json_asset_field "$MANIFEST" "$VERSION" "$component" "$RID" "sha256") fi if [ -z "$url" ] || [ "$url" = "null" ]; then @@ -246,12 +292,19 @@ download_component() { return 1 fi - mkdir -p "$INSTALL_DIR" cp "$binary_path" "$INSTALL_DIR/$binary_name" chmod +x "$INSTALL_DIR/$binary_name" echo " Installed $binary_name to $INSTALL_DIR/" } +if [ "$DRY_RUN" = false ]; then + # Resolve symlinks before installing so the exact path persisted into shell + # startup files is the same path that passed delimiter validation. + mkdir -p "$INSTALL_DIR" + INSTALL_DIR="$(cd "$INSTALL_DIR" && pwd -P)" + validate_install_dir_for_path "$INSTALL_DIR" +fi + # Download requested components SUCCESS=true if [[ "$COMPONENT" == "all" || "$COMPONENT" == "cli" ]]; then @@ -304,20 +357,178 @@ if [ "$CHANNEL_EXPLICIT" = true ]; then fi fi -# PATH instructions -echo "" -if echo "$PATH" | tr ':' '\n' | grep -qx "$INSTALL_DIR"; then - echo "Installation complete! netclaw is already on your PATH." -else - echo "Installation complete!" +# ── Shell integration ───────────────────────────────────────────────────── +# Bash and zsh source a small POSIX env file. Fish gets native syntax in its +# dedicated conf.d file; fish cannot source POSIX `case ... esac` syntax. +ENV_SCRIPT="$HOME/.netclaw/env" + +shell_quote() { + printf "'" + printf '%s' "$1" | sed "s/'/'\\\\''/g" + printf "'" +} + +INSTALL_DIR_QUOTED="$(shell_quote "$INSTALL_DIR")" +ENV_SCRIPT_QUOTED="$(shell_quote "$ENV_SCRIPT")" +SOURCE_LINE=". $ENV_SCRIPT_QUOTED" +MANUAL_PATH_LINE="export PATH=$INSTALL_DIR_QUOTED\${PATH:+:\"\$PATH\"}" + +detect_shell() { + # $SHELL is inherited from the parent login shell — it reflects the user's + # configured shell even when this script is piped via `curl | bash`. + local shell_name + shell_name="$(basename "${SHELL:-/bin/sh}")" + echo "$shell_name" +} + +get_rc_file() { + local shell_name="$1" shell_path="$2" + local os + os="$(uname -s | tr '[:upper:]' '[:lower:]')" + + case "$shell_name" in + zsh) + local effective_zdotdir + # ZDOTDIR is often assigned without export in ~/.zshenv, so ask zsh + # for the value it actually uses rather than relying on Bash's env. + effective_zdotdir="$("$shell_path" -c "printf '%s' \"\${ZDOTDIR:-\$HOME}\"")" || return 1 + if [[ -z "$effective_zdotdir" || "$effective_zdotdir" != /* || \ + "$effective_zdotdir" == *$'\n'* || "$effective_zdotdir" == *$'\r'* ]]; then + return 1 + fi + echo "$effective_zdotdir/.zshrc" + ;; + bash) + if [ "$os" = "darwin" ]; then + # A login shell reads only the first existing file in this list. + if [ -f "$HOME/.bash_profile" ]; then + echo "$HOME/.bash_profile" + elif [ -f "$HOME/.bash_login" ]; then + echo "$HOME/.bash_login" + else + echo "$HOME/.profile" + fi + else + echo "$HOME/.bashrc" + fi + ;; + *) + echo "" + ;; + esac +} + +write_posix_env_script() { + mkdir -p "$(dirname "$ENV_SCRIPT")" + cat > "$ENV_SCRIPT" </dev/null; then + echo " Shell profile '$rc_file' already sources netclaw." + return 0 + fi + + if [ -s "$rc_file" ] && [ "$(tail -c1 "$rc_file" | wc -l)" -eq 0 ]; then + echo "" >> "$rc_file" + fi + + { + echo "# netclaw shell setup" + echo "$SOURCE_LINE" + } >> "$rc_file" + + echo " Modified '$rc_file' to add netclaw to PATH." +} + +write_fish_config() { + local fish_config_dir="${XDG_CONFIG_HOME:-$HOME/.config}/fish/conf.d" + local fish_config="$fish_config_dir/netclaw.fish" + mkdir -p "$fish_config_dir" + cat > "$fish_config" < checksum -> extract -> install path, plus -DryRun. # # Usage: pwsh -File scripts/smoke/install-smoke.ps1 -# Requires: PowerShell 7+, python (for the local HTTP server). +# powershell.exe -File scripts/smoke/install-smoke.ps1 +# Requires: PowerShell 5.1+ and python (for the local HTTP server). Set-StrictMode -Version Latest $ErrorActionPreference = "Stop" -$RepoRoot = (Resolve-Path (Join-Path $PSScriptRoot ".." "..")).Path -$InstallPs1 = Join-Path $RepoRoot "scripts" "install.ps1" +$PowerShellExecutable = if ($PSVersionTable.PSEdition -eq "Desktop") { + (Get-Command powershell.exe).Source +} else { + (Get-Command pwsh).Source +} + +$RepoRoot = (Resolve-Path (Join-Path (Join-Path $PSScriptRoot "..") "..")).Path +$InstallPs1 = Join-Path (Join-Path $RepoRoot "scripts") "install.ps1" $Version = "0.0.0" # stable -> manifest.latest $BetaVersion = "0.0.1-beta1" # prerelease -> manifest.latestPrerelease $Rid = "win-x64" @@ -22,9 +29,68 @@ $script:Fail = 0 function Pass([string]$m) { Write-Host "PASS: $m"; $script:Pass++ } function Fail([string]$m) { Write-Host "FAIL: $m"; $script:Fail++ } +function Invoke-CapturedPowerShell { + param([string[]]$Arguments) + + # Windows PowerShell 5.1 promotes a native child's stderr to an error record. + # Rejection tests need to inspect that output and exit code without aborting. + $previousErrorActionPreference = $ErrorActionPreference + try { + $ErrorActionPreference = "Continue" + $output = & $PowerShellExecutable @Arguments 2>&1 | Out-String + [PSCustomObject]@{ Output = $output; ExitCode = $LASTEXITCODE } + } finally { + $ErrorActionPreference = $previousErrorActionPreference + } +} + +function Get-UserPathRegistryState { + $key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey("Environment", $false) + if ($null -eq $key) { throw "Cannot open the current user's Environment registry key." } + try { + $exists = $key.GetValueNames() -contains "Path" + [PSCustomObject]@{ + Exists = $exists + Value = if ($exists) { + $key.GetValue( + "Path", + $null, + [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames) + } else { + $null + } + Kind = if ($exists) { $key.GetValueKind("Path") } else { $null } + } + } finally { + $key.Dispose() + } +} + +function Set-UserPathRegistryState { + param( + [bool]$Exists, + [AllowNull()][string]$Value, + [AllowNull()][Microsoft.Win32.RegistryValueKind]$Kind + ) + + $key = [Microsoft.Win32.Registry]::CurrentUser.CreateSubKey("Environment") + if ($null -eq $key) { throw "Cannot create or open the current user's Environment registry key for update." } + try { + if ($Exists) { + $key.SetValue("Path", $Value, $Kind) + } else { + $key.DeleteValue("Path", $false) + } + } finally { + $key.Dispose() + } +} + $Work = Join-Path ([System.IO.Path]::GetTempPath()) ("netclaw-install-smoke-" + [Guid]::NewGuid().ToString('N')) $Serve = Join-Path $Work "serve" $BinDir = Join-Path $Work "bin" +$OriginalUserPath = Get-UserPathRegistryState +$OriginalProcessPath = $env:PATH New-Item -ItemType Directory -Path $Serve, $BinDir -Force | Out-Null $ServerProc = $null @@ -74,7 +140,9 @@ try { latestPrerelease = $BetaVersion releases = @($betaEntry, $stableEntry) } - $manifest | ConvertTo-Json -Depth 8 | Set-Content -Path (Join-Path $Serve "manifest.json") -Encoding utf8 + # The fixture is ASCII-only. Windows PowerShell 5.1 writes a BOM for + # -Encoding UTF8 while PowerShell 7 does not, so use an identical encoding. + $manifest | ConvertTo-Json -Depth 8 | Set-Content -Path (Join-Path $Serve "manifest.json") -Encoding ascii # 4. Serve the manifest + archives from localhost $python = Get-Command python3 -ErrorAction SilentlyContinue @@ -103,7 +171,7 @@ try { # 5. Dry-run check - resolves assets, installs nothing Write-Host "=== dry run ===" $dryDir = Join-Path $Work "dryrun-none" - $dryOut = & pwsh -NoProfile -File $InstallPs1 -InstallDir $dryDir -DryRun 2>&1 | Out-String + $dryOut = & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $dryDir -DryRun 2>&1 | Out-String Write-Host ($dryOut.TrimEnd()) if ($LASTEXITCODE -eq 0 ` -and $dryOut -match 'DRY RUN: would install netclaw ' ` @@ -121,14 +189,21 @@ try { # 6. Real install of the stand-in archives Write-Host "" Write-Host "=== real install ===" - $installDir = Join-Path $Work "installed" - $installOut = & pwsh -NoProfile -File $InstallPs1 -InstallDir $installDir 2>&1 | Out-String - Write-Host ($installOut.TrimEnd()) - if ($LASTEXITCODE -eq 0) { - Pass "install: exited 0" + + $invalidResult = Invoke-CapturedPowerShell -Arguments @( + "-NoProfile", "-File", $InstallPs1, + "-InstallDir", (Join-Path $Work "invalid;path"), "-DryRun") + if ($invalidResult.ExitCode -ne 0 -and $invalidResult.Output -match "cannot contain semicolons") { + Pass "PATH: unrepresentable Windows install directory rejected" } else { - Fail "install: exited $LASTEXITCODE" + Fail "PATH: Windows install directory containing ';' was accepted" } + + $installDir = Join-Path $Work "installed" + $existingUserEntry = Join-Path $Work "existing-user-bin" + Set-UserPathRegistryState $true $existingUserEntry ([Microsoft.Win32.RegistryValueKind]::String) + $installOut = & $InstallPs1 -InstallDir $installDir *>&1 | Out-String + Write-Host ($installOut.TrimEnd()) foreach ($name in @("netclaw", "netclawd")) { $exe = Join-Path $installDir "$name.exe" if ((Test-Path $exe) -and ((Get-Item $exe).Length -gt 0)) { @@ -138,20 +213,125 @@ try { } } - # 7. Verify PATH instruction uses User scope correctly (issue #1072) - # The printed instruction must NOT use $env:PATH (which merges Machine+User - # and corrupts the User PATH when written back). It must read User scope. + # 7. Verify the real installer changed User PATH without replacing the + # current process's inherited Machine PATH entries. + Write-Host "" + Write-Host "=== PATH automation ===" + $persistedPath = Get-UserPathRegistryState + $persistedEntries = @($persistedPath.Value -split ';') + if ($persistedEntries[0] -eq $installDir ` + -and $persistedEntries -contains $existingUserEntry ` + -and @($persistedEntries | Where-Object { $_ -eq $installDir }).Count -eq 1) { + Pass "PATH: install directory prepended once and existing User PATH preserved" + } else { + Fail "PATH: persisted User PATH has unexpected contents" + } + + $originalProcessEntries = @($OriginalProcessPath -split ';' | Where-Object { $_ }) + $currentProcessEntries = @($env:PATH -split ';' | Where-Object { $_ }) + $missingProcessEntries = @($originalProcessEntries | Where-Object { $currentProcessEntries -notcontains $_ }) + if ($currentProcessEntries[0] -eq $installDir ` + -and @($currentProcessEntries | Where-Object { $_ -eq $installDir }).Count -eq 1 ` + -and $missingProcessEntries.Count -eq 0) { + Pass "PATH: current process prepended once and inherited entries preserved" + } else { + Fail "PATH: current process lost inherited entries or contains duplicates" + } + + # A persisted entry must still repair a stale current process, and a + # trailing separator must not create a duplicate User PATH entry. + $env:PATH = $OriginalProcessPath + $userPathBeforeRepeat = Get-UserPathRegistryState + & $InstallPs1 -InstallDir "$installDir\" *>&1 | Out-Null + $userPathAfterRepeat = Get-UserPathRegistryState + $repeatProcessEntries = @($env:PATH -split ';' | Where-Object { $_ }) + if ($userPathAfterRepeat.Value -eq $userPathBeforeRepeat.Value ` + -and $userPathAfterRepeat.Kind -eq $userPathBeforeRepeat.Kind ` + -and $repeatProcessEntries[0] -eq $installDir ` + -and @($repeatProcessEntries | Where-Object { $_ -eq $installDir }).Count -eq 1) { + Pass "PATH: repeat install is idempotent and repairs current process" + } else { + Fail "PATH: repeat install changed User PATH or duplicated process entry" + } + + $env:NETCLAW_SMOKE_INSTALL_DIR = $installDir + $expandedUserPath = "%NETCLAW_SMOKE_INSTALL_DIR%;$existingUserEntry" + Set-UserPathRegistryState $true $expandedUserPath ([Microsoft.Win32.RegistryValueKind]::ExpandString) + $env:PATH = $OriginalProcessPath + & $InstallPs1 -InstallDir $installDir *>&1 | Out-Null + $expandedUserPathAfter = Get-UserPathRegistryState + if ($expandedUserPathAfter.Value -eq $expandedUserPath ` + -and $expandedUserPathAfter.Kind -eq [Microsoft.Win32.RegistryValueKind]::ExpandString) { + Pass "PATH: expandable User entry keeps its raw text and REG_EXPAND_SZ type" + } else { + Fail "PATH: expandable User entry or its registry type was rewritten" + } + + $literalPercentInstall = Join-Path $Work "%NETCLAW_LITERAL%\bin" + Set-UserPathRegistryState $true $existingUserEntry ([Microsoft.Win32.RegistryValueKind]::String) + $literalPercentOut = & $PowerShellExecutable -NoProfile -File $InstallPs1 ` + -InstallDir $literalPercentInstall 2>&1 | Out-String + $literalPercentState = Get-UserPathRegistryState + if ($LASTEXITCODE -eq 0 ` + -and $literalPercentState.Kind -eq [Microsoft.Win32.RegistryValueKind]::String ` + -and @($literalPercentState.Value -split ';')[0] -eq $literalPercentInstall) { + Pass "PATH: literal percent is preserved in a non-expanding User PATH" + } else { + Fail "PATH: literal percent was not preserved in a non-expanding User PATH" + Write-Host ($literalPercentOut.TrimEnd()) + } + + $expandablePath = "%SystemRoot%\System32" + $unsafePercentInstall = Join-Path $Work "%TEMP%\netclaw" + Set-UserPathRegistryState $true $expandablePath ([Microsoft.Win32.RegistryValueKind]::ExpandString) + $expandableStateBefore = Get-UserPathRegistryState + $unsafePercentResult = Invoke-CapturedPowerShell -Arguments @( + "-NoProfile", "-File", $InstallPs1, "-InstallDir", $unsafePercentInstall) + $expandableStateAfter = Get-UserPathRegistryState + if ($unsafePercentResult.ExitCode -ne 0 ` + -and $unsafePercentResult.Output -match "cannot be safely added to an expandable User PATH" ` + -and $expandableStateAfter.Value -eq $expandableStateBefore.Value ` + -and $expandableStateAfter.Kind -eq $expandableStateBefore.Kind) { + Pass "PATH: literal percent is rejected before mutating an expandable User PATH" + } else { + Fail "PATH: literal percent corrupted or changed an expandable User PATH" + } + + $env:PATH = "" + & $InstallPs1 -InstallDir $installDir *>&1 | Out-Null + if ($env:PATH -eq $installDir) { + Pass "PATH: empty process PATH does not create an empty entry" + } else { + Fail "PATH: empty process PATH produced unexpected contents" + } + $env:PATH = $OriginalProcessPath + + # 7c. Test -SkipShell flag Write-Host "" - Write-Host "=== PATH instruction check ===" - if ($installOut -match '\$env:PATH') { - Fail "PATH instruction: uses `$env:PATH (corrupts User PATH by merging Machine entries)" + Write-Host "=== -SkipShell flag ===" + $skipDir = Join-Path $Work "skip-install's" + $skipUserPathBefore = Get-UserPathRegistryState + $skipProcessPathBefore = $env:PATH + $skipOut = & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $skipDir -SkipShell 2>&1 | Out-String + if ($LASTEXITCODE -eq 0) { + Pass "-SkipShell: install completes without error" + } else { + Fail "-SkipShell: install failed (exit=$LASTEXITCODE)" + } + if ($skipOut -match "PATH modification skipped") { + Pass "-SkipShell: output mentions PATH modification skipped" } else { - Pass "PATH instruction: does not use `$env:PATH" + Fail "-SkipShell: missing 'skipped' message" } - if ($installOut -match "GetEnvironmentVariable\('PATH',\s*'User'\)") { - Pass "PATH instruction: reads from User scope" + $skipUserPathAfter = Get-UserPathRegistryState + if ($skipUserPathAfter.Value -eq $skipUserPathBefore.Value ` + -and $skipUserPathAfter.Kind -eq $skipUserPathBefore.Kind ` + -and $env:PATH -eq $skipProcessPathBefore ` + -and $skipOut -match [regex]::Escape("Add this directory to your User PATH") ` + -and $skipOut -match [regex]::Escape($skipDir)) { + Pass "-SkipShell: PATH is unchanged and manual guidance handles a literal install directory" } else { - Fail "PATH instruction: should read from User scope with GetEnvironmentVariable('PATH', 'User')" + Fail "-SkipShell: changed PATH or printed an unusable manual command" } # 8. Release channel resolution (dry-run) @@ -160,7 +340,7 @@ try { $shouldNotExist = Join-Path $Work "should-not-exist" function Assert-Resolves([string]$desc, [string]$want, [string[]]$extraArgs) { - $out = & pwsh -NoProfile -File $InstallPs1 -InstallDir $shouldNotExist -DryRun @extraArgs 2>&1 | Out-String + $out = & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $shouldNotExist -DryRun @extraArgs 2>&1 | Out-String $pattern = "(?m)^\s+Version:\s+$([regex]::Escape($want))\s*$" if ($LASTEXITCODE -eq 0 -and $out -match $pattern) { Pass "channel: $desc -> $want" @@ -176,8 +356,10 @@ try { Assert-Resolves "-Version pin overrides -Channel" $BetaVersion @("-Channel", "stable", "-Version", $BetaVersion) # An unknown channel must be rejected by the ValidateSet, not silently default. - & pwsh -NoProfile -File $InstallPs1 -InstallDir $shouldNotExist -DryRun -Channel bogus 2>&1 | Out-Null - if ($LASTEXITCODE -ne 0) { + $invalidChannelResult = Invoke-CapturedPowerShell -Arguments @( + "-NoProfile", "-File", $InstallPs1, "-InstallDir", $shouldNotExist, + "-DryRun", "-Channel", "bogus") + if ($invalidChannelResult.ExitCode -ne 0) { Pass "channel: unknown value rejected" } else { Fail "channel: unknown value should fail (exit=$LASTEXITCODE)" @@ -190,7 +372,7 @@ try { $freshDir = Join-Path $Work "fresh-beta" $freshConfigDir = Join-Path $Work "fresh-beta-config" $env:NETCLAW_CONFIG_DIR = $freshConfigDir - & pwsh -NoProfile -File $InstallPs1 -InstallDir $freshDir -Channel beta 2>&1 | Out-Null + & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $freshDir -Channel beta -SkipShell 2>&1 | Out-Null $freshConfig = Join-Path $freshConfigDir "netclaw.json" if ((Test-Path $freshConfig)) { $c = Get-Content -Raw $freshConfig | ConvertFrom-Json @@ -209,7 +391,7 @@ try { New-Item -ItemType Directory -Path $existConfigDir -Force | Out-Null '{"configVersion":1,"Daemon":{"ExposureMode":"local"}}' | Set-Content -Path (Join-Path $existConfigDir "netclaw.json") -Encoding UTF8 $env:NETCLAW_CONFIG_DIR = $existConfigDir - & pwsh -NoProfile -File $InstallPs1 -InstallDir $existDir -Channel beta 2>&1 | Out-Null + & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $existDir -Channel beta -SkipShell 2>&1 | Out-Null $c = Get-Content -Raw (Join-Path $existConfigDir "netclaw.json") | ConvertFrom-Json if ($c.Daemon.UpdateChannel -eq "beta" -and $c.Daemon.ExposureMode -eq "local") { Pass "config: -Channel beta patches existing config, preserves other Daemon keys" @@ -223,7 +405,7 @@ try { New-Item -ItemType Directory -Path $noflagConfigDir -Force | Out-Null '{"configVersion":1,"Daemon":{"UpdateChannel":"beta"}}' | Set-Content -Path (Join-Path $noflagConfigDir "netclaw.json") -Encoding UTF8 $env:NETCLAW_CONFIG_DIR = $noflagConfigDir - & pwsh -NoProfile -File $InstallPs1 -InstallDir $noflagDir 2>&1 | Out-Null + & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $noflagDir -SkipShell 2>&1 | Out-Null $c = Get-Content -Raw (Join-Path $noflagConfigDir "netclaw.json") | ConvertFrom-Json if ($c.Daemon.UpdateChannel -eq "beta") { Pass "config: plain upgrade preserves existing beta channel" @@ -237,7 +419,7 @@ try { New-Item -ItemType Directory -Path $downConfigDir -Force | Out-Null '{"configVersion":1,"Daemon":{"UpdateChannel":"beta"}}' | Set-Content -Path (Join-Path $downConfigDir "netclaw.json") -Encoding UTF8 $env:NETCLAW_CONFIG_DIR = $downConfigDir - & pwsh -NoProfile -File $InstallPs1 -InstallDir $downDir -Channel stable 2>&1 | Out-Null + & $PowerShellExecutable -NoProfile -File $InstallPs1 -InstallDir $downDir -Channel stable -SkipShell 2>&1 | Out-Null $c = Get-Content -Raw (Join-Path $downConfigDir "netclaw.json") | ConvertFrom-Json if ($c.Daemon.UpdateChannel -eq "stable") { Pass "config: -Channel stable overwrites existing beta" @@ -247,8 +429,11 @@ try { } finally { if ($ServerProc -and -not $ServerProc.HasExited) { $ServerProc.Kill() } + Set-UserPathRegistryState $OriginalUserPath.Exists $OriginalUserPath.Value $OriginalUserPath.Kind + $env:PATH = $OriginalProcessPath $env:MANIFEST_URL = $null $env:NETCLAW_CONFIG_DIR = $null + $env:NETCLAW_SMOKE_INSTALL_DIR = $null Remove-Item -Path $Work -Recurse -Force -ErrorAction SilentlyContinue } @@ -259,7 +444,6 @@ if ($script:Fail -gt 0) { exit 1 } Write-Host "install smoke (ps1): PASSED" -# Exit explicitly on the result, not on $LASTEXITCODE — the channel checks above run -# `pwsh -Channel bogus` (which exits non-zero by design), and without this the script -# would fall off the end and inherit that non-zero code despite all assertions passing. +# Deliberate rejection checks run child processes that exit nonzero, so return +# the assertion result explicitly instead of inheriting a child's exit code. exit 0 diff --git a/scripts/smoke/install-smoke.sh b/scripts/smoke/install-smoke.sh index d11c39176..6cee3ee14 100755 --- a/scripts/smoke/install-smoke.sh +++ b/scripts/smoke/install-smoke.sh @@ -111,6 +111,26 @@ rm -f "$MANIFEST_DEST" bash "$MANIFEST_GEN" "$VERSION" "$WORK/checksums-$VERSION" "$BASE_URL" >/dev/null bash "$MANIFEST_GEN" "$BETA_VERSION" "$WORK/checksums-$BETA_VERSION" "$BASE_URL" >/dev/null cp "$MANIFEST_DEST" "$SERVE/manifest.json" +python3 - "$SERVE/manifest.json" "$SERVE/manifest-rid-first.json" <<'PY' +import json +import sys + +with open(sys.argv[1], encoding="utf-8") as source: + manifest = json.load(source) +for release in manifest["releases"]: + release["assets"] = [ + { + "rid": asset["rid"], + "component": asset["component"], + "url": asset["url"], + "sha256": asset["sha256"], + "sizeBytes": asset["sizeBytes"], + } + for asset in release["assets"] + ] +with open(sys.argv[2], "w", encoding="utf-8") as destination: + json.dump(manifest, destination) +PY # ── 4. Serve the manifest + archives from localhost ────────────────────────── python3 -m http.server "$PORT" --bind 127.0.0.1 --directory "$SERVE" >/dev/null 2>&1 & @@ -169,6 +189,66 @@ check_detect "macOS x86_64 + Rosetta -> osx-arm64" Darwin x86_64 1 'DRY RUN: wou check_detect "Intel Mac rejected" Darwin x86_64 0 'Apple Silicon' 1 check_detect "unsupported OS rejected" freebsd x86_64 0 'Unsupported OS' 1 +set +e +invalid_path_out=$(INSTALL_DIR="$WORK/invalid:path" \ + MANIFEST_URL="$BASE_URL/manifest.json" \ + bash "$INSTALL_SH" --dry-run 2>&1) +invalid_path_rc=$? +set -e +if [ "$invalid_path_rc" -ne 0 ] && echo "$invalid_path_out" | grep -q "cannot contain ':'"; then + pass "PATH: unrepresentable Unix install directory rejected" +else + fail "PATH: Unix install directory containing ':' was accepted" +fi + +PHYSICAL_INSTALL="$WORK/physical:install" +SYMLINK_INSTALL="$WORK/safe-install-link" +SYMLINK_HOME="$WORK/symlink-home" +mkdir -p "$PHYSICAL_INSTALL" "$SYMLINK_HOME" +ln -s "$PHYSICAL_INSTALL" "$SYMLINK_INSTALL" +set +e +symlink_path_out=$(HOME="$SYMLINK_HOME" SHELL="$(command -v bash)" \ + INSTALL_DIR="$SYMLINK_INSTALL" MANIFEST_URL="$BASE_URL/manifest.json" \ + bash "$INSTALL_SH" cli 2>&1) +symlink_path_rc=$? +set -e +if [ "$symlink_path_rc" -ne 0 ] \ + && echo "$symlink_path_out" | grep -q "cannot contain ':'" \ + && [ ! -e "$SYMLINK_HOME/.netclaw/env" ] \ + && [ ! -e "$SYMLINK_HOME/.bashrc" ] \ + && [ ! -e "$PHYSICAL_INSTALL/netclaw" ]; then + pass "PATH: physical install path is validated before install or shell mutation" +else + fail "PATH: unrepresentable symlink target was installed or persisted" + echo "$symlink_path_out" | indent +fi + +# Exercise the dependency-free parser with a valid manifest whose asset fields +# are in reverse order. A private mirror need not preserve JSON property order. +NO_JQ_BIN="$WORK/no-jq-bin" +mkdir -p "$NO_JQ_BIN" +for cmd in awk bash curl cut grep head mktemp rm sed sha256sum shasum tar tr uname; do + command_path=$(command -v "$cmd" 2>/dev/null || true) + if [ -n "$command_path" ]; then + ln -s "$command_path" "$NO_JQ_BIN/$cmd" + fi +done +set +e +no_jq_out=$(PATH="$NO_JQ_BIN" \ + MANIFEST_URL="$BASE_URL/manifest-rid-first.json" \ + INSTALL_DIR="$WORK/no-jq-install" \ + /bin/bash "$INSTALL_SH" --dry-run 2>&1) +no_jq_rc=$? +set -e +if [ "$no_jq_rc" -eq 0 ] \ + && echo "$no_jq_out" | grep -q "DRY RUN: would install netclaw .*/$VERSION/" \ + && echo "$no_jq_out" | grep -q "DRY RUN: would install netclawd .*/$VERSION/"; then + pass "manifest: jq-less parser selects stable assets with reordered fields" +else + fail "manifest: jq-less reordered-field parse failed (exit=$no_jq_rc)" + echo "$no_jq_out" | indent +fi + # Dry run must not create the install directory. if [ -d "$WORK/should-not-exist" ]; then fail "dry-run: created an install directory (should install nothing)" @@ -177,11 +257,16 @@ else fi # ── 6. Mechanical check: a real install on the host's native RID ───────────── +# Uses a temp HOME so shell integration writes to the temp dir, not the +# CI runner's real profile — and we can verify the RC was modified. echo "" echo "=== real install (host RID, stand-in archive) ===" -INSTALL_DIR="$WORK/installed" +INSTALL_HOME="$WORK/installed-home" +INSTALL_DIR="$INSTALL_HOME/.netclaw/bin" +mkdir -p "$INSTALL_HOME" set +e -install_out=$(MANIFEST_URL="$BASE_URL/manifest.json" INSTALL_DIR="$INSTALL_DIR" \ +install_out=$(HOME="$INSTALL_HOME" \ + MANIFEST_URL="$BASE_URL/manifest.json" INSTALL_DIR="$INSTALL_DIR" \ bash "$INSTALL_SH" 2>&1) install_rc=$? set -e @@ -201,6 +286,26 @@ for name in netclaw netclawd; do fi done +# Verify shell integration actually ran +INSTALL_ENV="$INSTALL_HOME/.netclaw/env" +if [ -f "$INSTALL_ENV" ]; then + pass "real install: env script created" +else + fail "real install: env script not found at $INSTALL_ENV" +fi + +# The RC file depends on $SHELL — check whichever one was created +RC_MODIFIED=false +for rc in "$INSTALL_HOME/.bashrc" "$INSTALL_HOME/.zshrc" "$INSTALL_HOME/.profile" "$INSTALL_HOME/.config/fish/conf.d/netclaw.fish"; do + if [ -f "$rc" ] && grep -qxF ". '$INSTALL_ENV'" "$rc" 2>/dev/null; then + pass "real install: $(basename "$rc") sources env script" + RC_MODIFIED=true + fi +done +if [ "$RC_MODIFIED" = false ]; then + fail "real install: no RC file sources env script (SHELL=$SHELL)" +fi + # ── 7. Release channel resolution (dry-run) ────────────────────────────────── echo "" echo "=== release channel resolution ===" @@ -256,7 +361,7 @@ set +e fresh_out=$(MANIFEST_URL="$BASE_URL/manifest.json" \ INSTALL_DIR="$FRESH_DIR" \ CONFIG_DIR="$FRESH_CONFIG_DIR" \ - bash "$INSTALL_SH" --channel beta 2>&1) + bash "$INSTALL_SH" --channel beta --skip-shell 2>&1) fresh_rc=$? set -e if [ "$fresh_rc" -eq 0 ] && [ -f "$FRESH_CONFIG_DIR/netclaw.json" ]; then @@ -284,7 +389,7 @@ set +e exist_out=$(MANIFEST_URL="$BASE_URL/manifest.json" \ INSTALL_DIR="$EXIST_DIR" \ CONFIG_DIR="$EXIST_CONFIG_DIR" \ - bash "$INSTALL_SH" --channel beta 2>&1) + bash "$INSTALL_SH" --channel beta --skip-shell 2>&1) exist_rc=$? set -e if [ "$exist_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then @@ -297,6 +402,7 @@ if [ "$exist_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then fi else fail "config: --channel beta on existing config (exit=$exist_rc)" + echo "$exist_out" | indent fi # 8c. Plain upgrade (no --channel) leaves existing beta config alone @@ -308,7 +414,7 @@ set +e noflag_out=$(MANIFEST_URL="$BASE_URL/manifest.json" \ INSTALL_DIR="$NOFLAG_DIR" \ CONFIG_DIR="$NOFLAG_CONFIG_DIR" \ - bash "$INSTALL_SH" 2>&1) + bash "$INSTALL_SH" --skip-shell 2>&1) noflag_rc=$? set -e if [ "$noflag_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then @@ -320,6 +426,7 @@ if [ "$noflag_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then fi else fail "config: plain upgrade (exit=$noflag_rc)" + echo "$noflag_out" | indent fi # 8d. --channel stable on existing beta overwrites to stable @@ -331,7 +438,7 @@ set +e down_out=$(MANIFEST_URL="$BASE_URL/manifest.json" \ INSTALL_DIR="$DOWNGRADE_DIR" \ CONFIG_DIR="$DOWNGRADE_CONFIG_DIR" \ - bash "$INSTALL_SH" --channel stable 2>&1) + bash "$INSTALL_SH" --channel stable --skip-shell 2>&1) down_rc=$? set -e if [ "$down_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then @@ -343,6 +450,153 @@ if [ "$down_rc" -eq 0 ] && command -v jq >/dev/null 2>&1; then fi else fail "config: --channel stable on existing beta (exit=$down_rc)" + echo "$down_out" | indent +fi + +# ── 9. Shell integration (PATH automation) ─────────────────────────────────── +echo "" +echo "=== shell integration ===" + +assert_path_once() { + local desc="$1" observed_path="$2" install_dir="$3" + local count + count=$(printf '%s' "$observed_path" | tr ':' '\n' | grep -cxF "$install_dir" || true) + if [ "$count" -eq 1 ]; then + pass "$desc: install directory appears exactly once on PATH" + else + fail "$desc: install directory appears $count times on PATH" + fi +} + +run_unix_installer() { + local shell_path="$1" home="$2" install_dir="$3" + shift 3 + SHELL="$shell_path" HOME="$home" \ + MANIFEST_URL="$BASE_URL/manifest.json" INSTALL_DIR="$install_dir" \ + CONFIG_DIR="$home/.netclaw/config" \ + bash "$INSTALL_SH" "$@" +} + +# Bash: run the generated startup path through Bash itself, then repeat the +# install to prove both profile mutation and PATH evaluation are idempotent. +BASH_HOME="$WORK/shell-bash" +BASH_INSTALL="$BASH_HOME/netclaw install's/bin" +mkdir -p "$BASH_HOME" +if [ "$(uname -s)" = "Darwin" ]; then + BASH_RC="$BASH_HOME/.bash_profile" + printf '# existing bash profile' > "$BASH_RC" + printf '# profile must remain untouched\n' > "$BASH_HOME/.profile" +else + BASH_RC="$BASH_HOME/.bashrc" + printf '# existing bash rc' > "$BASH_RC" +fi + +if run_unix_installer "$(command -v bash)" "$BASH_HOME" "$BASH_INSTALL" >/dev/null \ + && run_unix_installer "$(command -v bash)" "$BASH_HOME" "$BASH_INSTALL" >/dev/null; then + BASH_INSTALL_PHYSICAL=$(cd "$BASH_INSTALL" && pwd -P) + bash_path=$(PATH="/usr/bin:/bin" HOME="$BASH_HOME" \ + bash --noprofile --rcfile "$BASH_RC" -i -c 'printf "%s" "$PATH"' 2>/dev/null) + assert_path_once "bash" "$bash_path" "$BASH_INSTALL_PHYSICAL" + bash_empty_path=$(PATH="" HOME="$BASH_HOME" \ + /bin/bash --noprofile --rcfile "$BASH_RC" -i -c 'printf "%s" "$PATH"' 2>/dev/null) + if [ "$bash_empty_path" = "$BASH_INSTALL_PHYSICAL" ]; then + pass "bash: empty PATH does not introduce a current-directory entry" + else + fail "bash: empty PATH produced '$bash_empty_path'" + fi + source_count=$(grep -cF "$BASH_HOME/.netclaw/env" "$BASH_RC" || true) + if [ "$source_count" -eq 1 ]; then + pass "bash: profile source line is idempotent" + else + fail "bash: profile contains $source_count netclaw source lines" + fi + if [ "$(uname -s)" = "Darwin" ] && ! grep -qF netclaw "$BASH_HOME/.profile"; then + pass "bash-macos: existing .bash_profile wins over .profile" + fi +else + fail "bash: installer failed" +fi + +# Zsh: resolve a non-exported ZDOTDIR from .zshenv, then execute the selected +# startup file under zsh so a Bash-compatible false positive cannot pass. +if command -v zsh >/dev/null 2>&1; then + ZSH_HOME="$WORK/shell-zsh" + ZDOT_DIR="$ZSH_HOME/custom-zdotdir" + ZSH_INSTALL="$ZSH_HOME/netclaw install's/bin" + mkdir -p "$ZDOT_DIR" + printf "ZDOTDIR='%s'\n" "$ZDOT_DIR" > "$ZSH_HOME/.zshenv" + printf '# existing zsh config\n' > "$ZDOT_DIR/.zshrc" + if (unset ZDOTDIR; run_unix_installer "$(command -v zsh)" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null) \ + && (unset ZDOTDIR; run_unix_installer "$(command -v zsh)" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null); then + ZSH_INSTALL_PHYSICAL=$(cd "$ZSH_INSTALL" && pwd -P) + zsh_path=$(PATH="/usr/bin:/bin" ZDOTDIR="$ZDOT_DIR" \ + zsh -f -c 'source "$ZDOTDIR/.zshrc"; print -rn -- "$PATH"') + assert_path_once "zsh" "$zsh_path" "$ZSH_INSTALL_PHYSICAL" + if [ ! -e "$ZSH_HOME/.zshrc" ]; then + pass "zsh: non-exported ZDOTDIR is authoritative" + else + fail "zsh: installer touched ~/.zshrc despite ZDOTDIR" + fi + else + fail "zsh: installer failed" + fi +else + echo "SKIP: zsh executable not available" +fi + +# Fish owns a native conf.d file. Execute that file with fish, not Bash. +if command -v fish >/dev/null 2>&1; then + FISH_HOME="$WORK/shell-fish" + FISH_INSTALL="$FISH_HOME/netclaw install's/bin" + FISH_RC="$FISH_HOME/.config/fish/conf.d/netclaw.fish" + if XDG_CONFIG_HOME="$FISH_HOME/.config" \ + run_unix_installer "$(command -v fish)" "$FISH_HOME" "$FISH_INSTALL" >/dev/null \ + && XDG_CONFIG_HOME="$FISH_HOME/.config" \ + run_unix_installer "$(command -v fish)" "$FISH_HOME" "$FISH_INSTALL" >/dev/null; then + FISH_INSTALL_PHYSICAL=$(cd "$FISH_INSTALL" && pwd -P) + fish_path=$(PATH="/usr/bin:/bin" fish --no-config -c \ + "source '$FISH_RC'; string join : -- \$PATH") + assert_path_once "fish" "$fish_path" "$FISH_INSTALL_PHYSICAL" + else + fail "fish: installer failed" + fi +else + echo "SKIP: fish executable not available" +fi + +# Opt-out under a supported shell must print a self-contained command instead +# of referring to an env file that was not made. +MANUAL_HOME="$WORK/shell-skip" +MANUAL_INSTALL="$MANUAL_HOME/netclaw install's/bin" +mkdir -p "$MANUAL_HOME" +manual_out=$(run_unix_installer "$(command -v bash)" "$MANUAL_HOME" "$MANUAL_INSTALL" --skip-shell) +manual_command=$(printf '%s\n' "$manual_out" | sed -n 's/^ \{0,4\}\(export PATH=.*\)$/\1/p' | head -1) +if [ -n "$manual_command" ] && [ ! -e "$MANUAL_HOME/.netclaw/env" ]; then + MANUAL_INSTALL_PHYSICAL=$(cd "$MANUAL_INSTALL" && pwd -P) + manual_path=$(PATH="/usr/bin:/bin" bash -c "$manual_command; printf '%s' \"\$PATH\"") + assert_path_once "skip" "$manual_path" "$MANUAL_INSTALL_PHYSICAL" + manual_empty_path=$(PATH="" /bin/bash -c "$manual_command; printf '%s' \"\$PATH\"") + if [ "$manual_empty_path" = "$MANUAL_INSTALL_PHYSICAL" ]; then + pass "skip: manual command preserves an empty PATH without adding current directory" + else + fail "skip: manual command produced '$manual_empty_path' from an empty PATH" + fi +else + fail "skip: missing usable manual PATH command or created shell files" +fi + +# Unsupported shells get shell-neutral guidance; emitting Bash syntax for an +# arbitrary shell would make the suggested command actively misleading. +UNKNOWN_HOME="$WORK/shell-unknown" +UNKNOWN_INSTALL="$UNKNOWN_HOME/netclaw install's/bin" +unknown_out=$(run_unix_installer /bin/unknownshell "$UNKNOWN_HOME" "$UNKNOWN_INSTALL") +UNKNOWN_INSTALL_PHYSICAL=$(cd "$UNKNOWN_INSTALL" && pwd -P) +if echo "$unknown_out" | grep -qF "$UNKNOWN_INSTALL_PHYSICAL" \ + && ! echo "$unknown_out" | grep -q 'export PATH=' \ + && [ ! -e "$UNKNOWN_HOME/.netclaw/env" ]; then + pass "unknown: guidance is shell-neutral and no shell files are created" +else + fail "unknown: guidance is shell-specific or shell files were created" fi # ── Summary ────────────────────────────────────────────────────────────────── diff --git a/src/Netclaw.Actors.Tests/Channels/DiscordApprovalPromptBuilderTests.cs b/src/Netclaw.Actors.Tests/Channels/DiscordApprovalPromptBuilderTests.cs index ac819290b..5ca89df6f 100644 --- a/src/Netclaw.Actors.Tests/Channels/DiscordApprovalPromptBuilderTests.cs +++ b/src/Netclaw.Actors.Tests/Channels/DiscordApprovalPromptBuilderTests.cs @@ -279,6 +279,55 @@ private static ToolInteractionRequest V2Request( Options = options }; + [Fact] + public void Mcp_prompt_renders_invocation_without_shell_scope_chrome() + { + var request = V2Request( + "Dropbox/upload(destination_directory=\"/Finance/Q3\", contents=(90000 chars, 2000 lines))", + ["Dropbox/upload"], + cwd: null, + options: + [ + new ToolInteractionOption(ApprovalOptionKeys.ApproveOnceKey, ApprovalOptionKeys.ApproveOnceLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveSessionKey, ApprovalOptionKeys.ApproveSessionLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveEverywhereKey, ApprovalOptionKeys.ApproveMcpToolLabel), + new ToolInteractionOption(ApprovalOptionKeys.DenyKey, ApprovalOptionKeys.DenyLabel) + ]) with + { + ToolName = new Netclaw.Tools.ToolName("Dropbox/upload") + }; + + var text = DiscordApprovalPromptBuilder.BuildTextPrompt(request); + var (buttonText, buttons) = DiscordApprovalPromptBuilder.BuildButtonPrompt(request); + + Assert.Contains("MCP tool approval required", text); + Assert.Contains("Invocation:", text); + Assert.Contains("Allow this MCP tool invocation?", text); + Assert.Contains("**Invocation:**", buttonText); + Assert.Contains(buttons, button => button.Label == ApprovalOptionKeys.ApproveMcpToolLabel); + Assert.DoesNotContain("no working directory", text); + Assert.DoesNotContain("Always anywhere", text); + Assert.DoesNotContain("• Dropbox/upload", text); + } + + [Fact] + public void Mcp_resolution_describes_tool_scope_not_shell_location() + { + var request = V2Request("Dropbox/upload(path=\"/Finance/Q3\")", ["Dropbox/upload"], null, FullButtonRow()) with + { + ToolName = new Netclaw.Tools.ToolName("Dropbox/upload") + }; + + var text = DiscordApprovalPromptBuilder.BuildResolvedPromptText( + request, + ApprovalOptionKeys.ApproveEverywhere, + "user-1"); + + Assert.Contains("MCP tool approval resolved", text); + Assert.Contains("Always allowed: Dropbox/upload", text); + Assert.DoesNotContain("anywhere", text); + } + [Fact] public void V2_single_verb_collapses_into_header() { diff --git a/src/Netclaw.Actors.Tests/Channels/MattermostApprovalPromptBuilderTests.cs b/src/Netclaw.Actors.Tests/Channels/MattermostApprovalPromptBuilderTests.cs index 9c325df5d..2a2c15512 100644 --- a/src/Netclaw.Actors.Tests/Channels/MattermostApprovalPromptBuilderTests.cs +++ b/src/Netclaw.Actors.Tests/Channels/MattermostApprovalPromptBuilderTests.cs @@ -70,15 +70,29 @@ public void BuildTextPrompt_omits_pattern_when_empty() [Fact] public void BuildDecisionStatus_formats_known_keys() { - Assert.Contains(ApprovalOptionKeys.ApproveOnceLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.ApproveOnce)); - Assert.Contains(ApprovalOptionKeys.ApproveAlwaysLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.ApproveAlways)); - Assert.Contains(ApprovalOptionKeys.DenyLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.Deny)); + var toolName = new Netclaw.Tools.ToolName("shell_execute"); + Assert.Contains(ApprovalOptionKeys.ApproveOnceLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.ApproveOnce, toolName)); + Assert.Contains(ApprovalOptionKeys.ApproveAlwaysLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.ApproveAlways, toolName)); + Assert.Contains(ApprovalOptionKeys.DenyLabel, MattermostApprovalPromptBuilder.BuildDecisionStatus(ApprovalOptionKeys.Deny, toolName)); + } + + [Fact] + public void BuildDecisionStatus_uses_MCP_persistent_label() + { + var status = MattermostApprovalPromptBuilder.BuildDecisionStatus( + ApprovalOptionKeys.ApproveEverywhere, + new Netclaw.Tools.ToolName("Dropbox/upload")); + + Assert.Contains(ApprovalOptionKeys.ApproveMcpToolLabel, status); + Assert.DoesNotContain(ApprovalOptionKeys.ApproveEverywhereLabel, status); } [Fact] public void BuildDecisionStatus_passes_through_unknown_key() { - var status = MattermostApprovalPromptBuilder.BuildDecisionStatus("custom_key"); + var status = MattermostApprovalPromptBuilder.BuildDecisionStatus( + "custom_key", + new Netclaw.Tools.ToolName("shell_execute")); Assert.Contains("custom_key", status); } @@ -355,6 +369,53 @@ private static ToolInteractionRequest CreateStandardRequest() ] }; + [Fact] + public void Mcp_prompt_renders_invocation_without_shell_patterns() + { + var request = CreateStandardRequest() with + { + ToolName = new Netclaw.Tools.ToolName("Dropbox/upload"), + DisplayText = "Dropbox/upload(destination_directory=\"/Finance/Q3\", contents=(90000 chars, 2000 lines))", + Patterns = ["Dropbox/upload"], + Options = + [ + new ToolInteractionOption(ApprovalOptionKeys.ApproveOnceKey, ApprovalOptionKeys.ApproveOnceLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveSessionKey, ApprovalOptionKeys.ApproveSessionLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveEverywhereKey, ApprovalOptionKeys.ApproveMcpToolLabel), + new ToolInteractionOption(ApprovalOptionKeys.DenyKey, ApprovalOptionKeys.DenyLabel) + ] + }; + + var text = MattermostApprovalPromptBuilder.BuildTextPrompt(request); + + Assert.Contains("MCP tool approval required", text); + Assert.Contains("**Invocation:**", text); + Assert.Contains("Allow this MCP tool invocation?", text); + Assert.Contains(ApprovalOptionKeys.ApproveMcpToolLabel, text); + Assert.DoesNotContain("**Pattern", text); + Assert.DoesNotContain("Always anywhere", text); + } + + [Fact] + public void Mcp_resolution_uses_contextual_persistent_grant_label() + { + var request = CreateStandardRequest() with + { + ToolName = new Netclaw.Tools.ToolName("Dropbox/upload"), + DisplayText = "Dropbox/upload(path=\"/Finance/Q3\")" + }; + + var text = MattermostApprovalPromptBuilder.BuildResolvedPromptText( + request, + ApprovalOptionKeys.ApproveEverywhere, + "user-1"); + + Assert.Contains("MCP tool approval resolved", text); + Assert.Contains($"**Decision:** {ApprovalOptionKeys.ApproveMcpToolLabel}", text); + Assert.DoesNotContain("Allow this MCP tool invocation?", text); + Assert.DoesNotContain("Always anywhere", text); + } + [Fact] public void Oversized_command_keeps_prompt_under_Mattermost_cap() { diff --git a/src/Netclaw.Actors.Tests/Channels/SlackApprovalBlockBuilderTests.cs b/src/Netclaw.Actors.Tests/Channels/SlackApprovalBlockBuilderTests.cs index 90739034a..1d45d37aa 100644 --- a/src/Netclaw.Actors.Tests/Channels/SlackApprovalBlockBuilderTests.cs +++ b/src/Netclaw.Actors.Tests/Channels/SlackApprovalBlockBuilderTests.cs @@ -78,6 +78,55 @@ public void Multi_verb_uses_generic_header_with_bulleted_verbs() Assert.Contains("• `git status`", text); } + [Fact] + public void Mcp_prompt_renders_invocation_without_shell_scope_chrome() + { + var request = Request( + "Dropbox/upload(destination_directory=\"/Finance/Q3\", contents=(90000 chars, 2000 lines))", + ["Dropbox/upload"], + cwd: null, + options: + [ + new ToolInteractionOption(ApprovalOptionKeys.ApproveOnceKey, ApprovalOptionKeys.ApproveOnceLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveSessionKey, ApprovalOptionKeys.ApproveSessionLabel), + new ToolInteractionOption(ApprovalOptionKeys.ApproveEverywhereKey, ApprovalOptionKeys.ApproveMcpToolLabel), + new ToolInteractionOption(ApprovalOptionKeys.DenyKey, ApprovalOptionKeys.DenyLabel) + ]) with + { + ToolName = new ToolName("Dropbox/upload") + }; + + var text = SlackApprovalBlockBuilder.BuildApprovalText(request); + var blocks = string.Join('\n', SlackApprovalBlockBuilder.BuildApprovalBlocks(request) + .OfType() + .Select(block => block.Text is Markdown markdown ? markdown.Text : string.Empty)); + + Assert.Contains("MCP tool approval required", text); + Assert.Contains("Allow this MCP tool invocation?", text); + Assert.Contains("*Invocation:*", blocks); + Assert.DoesNotContain("no working directory", text); + Assert.DoesNotContain("Always anywhere", text); + Assert.DoesNotContain("• `Dropbox/upload`", text); + } + + [Fact] + public void Mcp_resolution_describes_tool_scope_not_shell_location() + { + var request = Request("Dropbox/upload(path=\"/Finance/Q3\")", ["Dropbox/upload"], null, FullButtonRow()) with + { + ToolName = new ToolName("Dropbox/upload") + }; + + var text = SlackApprovalBlockBuilder.BuildResolvedApprovalText( + request, + ApprovalOptionKeys.ApproveEverywhere, + "U123"); + + Assert.Contains("MCP tool approval resolved", text); + Assert.Contains("Always allowed: Dropbox/upload", text); + Assert.DoesNotContain("anywhere", text); + } + [Fact] public void Messy_command_emits_complex_command_hint() { diff --git a/src/Netclaw.Actors.Tests/Channels/SlackAttachmentLineTests.cs b/src/Netclaw.Actors.Tests/Channels/SlackAttachmentLineTests.cs index facc40aac..fc8062acd 100644 --- a/src/Netclaw.Actors.Tests/Channels/SlackAttachmentLineTests.cs +++ b/src/Netclaw.Actors.Tests/Channels/SlackAttachmentLineTests.cs @@ -3,7 +3,9 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Netclaw.Actors.Protocol; using Netclaw.Channels; +using Netclaw.Media; using Xunit; namespace Netclaw.Actors.Tests.Channels; @@ -59,4 +61,71 @@ public void BuildAttachmentLine_with_hostile_metadata_produces_single_parseable_ Assert.DoesNotContain("\r", line, StringComparison.Ordinal); Assert.StartsWith("[attachment]", line, StringComparison.Ordinal); } + + [Fact] + public async Task BuildAcceptedProjection_uses_final_collision_safe_live_inbox_path() + { + var sessionDir = Path.Combine(Path.GetTempPath(), $"netclaw-attachment-line-{Guid.NewGuid():N}"); + var inboxDir = Path.Combine(sessionDir, SessionDirectoryHelper.InboxSubdirectory); + Directory.CreateDirectory(inboxDir); + + try + { + await InboxWriter.SanitizeReserveAndWriteAsync( + inboxDir, "image.png", new byte[] { 1 }, TestContext.Current.CancellationToken); + var renamedPath = await InboxWriter.SanitizeReserveAndWriteAsync( + inboxDir, "image.png", new byte[] { 2 }, TestContext.Current.CancellationToken); + + var projection = await AttachmentIngressFormatting.BuildAcceptedProjectionAsync( + renamedPath, + "image.png", + "image/png", + AttachmentCategory.Image, + inlineImages: true, + size: 1, + TestContext.Current.CancellationToken); + + Assert.EndsWith("image_1.png", renamedPath, StringComparison.Ordinal); + Assert.Contains("path=\"inbox/image_1.png\"", projection.Line, StringComparison.Ordinal); + Assert.True(File.Exists(Path.Combine(sessionDir, "inbox", "image_1.png"))); + Assert.NotNull(projection.InlineContent); + } + finally + { + Directory.Delete(sessionDir, recursive: true); + } + } + + [Fact] + public async Task BuildAcceptedProjection_uses_final_stable_historical_inbox_path() + { + var sessionDir = Path.Combine(Path.GetTempPath(), $"netclaw-historical-line-{Guid.NewGuid():N}"); + var inboxDir = Path.Combine(sessionDir, SessionDirectoryHelper.InboxSubdirectory); + Directory.CreateDirectory(inboxDir); + var stagedPath = Path.Combine(sessionDir, "stage.tmp"); + await File.WriteAllBytesAsync(stagedPath, [1, 2, 3], TestContext.Current.CancellationToken); + + try + { + var historicalPath = HistoricalAttachmentInbox.PromoteOrReuse( + inboxDir, "image.png", "slack:F123", stagedPath); + var projection = await AttachmentIngressFormatting.BuildAcceptedProjectionAsync( + historicalPath, + "image.png", + "image/png", + AttachmentCategory.Image, + inlineImages: true, + size: 3, + TestContext.Current.CancellationToken); + + var finalName = Path.GetFileName(historicalPath); + Assert.Matches("^image_hist_[0-9a-f]{16}\\.png$", finalName); + Assert.Contains($"path=\"inbox/{finalName}\"", projection.Line, StringComparison.Ordinal); + Assert.True(File.Exists(Path.Combine(sessionDir, "inbox", finalName))); + } + finally + { + Directory.Delete(sessionDir, recursive: true); + } + } } diff --git a/src/Netclaw.Actors.Tests/Protocol/ChatMessageConverterTests.cs b/src/Netclaw.Actors.Tests/Protocol/ChatMessageConverterTests.cs index e4594dd94..82c1f7578 100644 --- a/src/Netclaw.Actors.Tests/Protocol/ChatMessageConverterTests.cs +++ b/src/Netclaw.Actors.Tests/Protocol/ChatMessageConverterTests.cs @@ -279,16 +279,17 @@ public void FromAiMessage_writes_DataContent_to_session_dir_and_produces_media_r using var tempDir = new TempSessionDir(); // Real PNG, small enough to pass through the egress normalizer unchanged. var imageBytes = SmallPng(); + const string announcedPath = "inbox/image_hist_0123456789abcdef.png"; var contents = new List { - new TextContent("Check this image"), + new TextContent($"[attachment] name=\"image.png\" mime=\"image/png\" size={imageBytes.Length} path=\"{announcedPath}\" inlined=\"true\""), new DataContent(imageBytes, "image/png") }; var ai = new AiChatMessage(AiChatRole.User, contents); var msg = ChatMessageConverter.FromAiMessage(ai, sessionDir: tempDir.Path); - Assert.Equal("Check this image", msg.Content); + Assert.Contains($"path=\"{announcedPath}\"", msg.Content, StringComparison.Ordinal); Assert.Single(msg.MediaReferences); Assert.Equal("image/png", msg.MediaReferences[0].MimeType.Value); Assert.Equal((int)MediaModality.Image, msg.MediaReferences[0].Modality); @@ -302,6 +303,7 @@ public void FromAiMessage_writes_DataContent_to_session_dir_and_produces_media_r var filePath = Path.Combine(tempDir.Path, "media", msg.MediaReferences[0].RelativePath); Assert.True(File.Exists(filePath)); Assert.Equal(imageBytes, File.ReadAllBytes(filePath)); + Assert.DoesNotContain(msg.MediaReferences[0].RelativePath, msg.Content, StringComparison.Ordinal); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Sessions/AttachmentContextHintTests.cs b/src/Netclaw.Actors.Tests/Sessions/AttachmentContextHintTests.cs index 0e276c253..e0fa32d26 100644 --- a/src/Netclaw.Actors.Tests/Sessions/AttachmentContextHintTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/AttachmentContextHintTests.cs @@ -24,6 +24,15 @@ public void Hint_names_the_inbox_subdirectory() Assert.Contains("inbox/", SessionMessageAssembler.AttachmentContextHint, System.StringComparison.Ordinal); } + [Fact] + public void Hint_defines_the_announced_path_as_authoritative_and_session_relative() + { + Assert.Contains("path` is authoritative", SessionMessageAssembler.AttachmentContextHint, System.StringComparison.Ordinal); + Assert.Contains("relative to `session_dir`", SessionMessageAssembler.AttachmentContextHint, System.StringComparison.Ordinal); + Assert.Contains("collision-safe filename change", SessionMessageAssembler.AttachmentContextHint, System.StringComparison.Ordinal); + Assert.Contains("`{session_dir}/{path}`", SessionMessageAssembler.AttachmentContextHint, System.StringComparison.Ordinal); + } + [Fact] public void Hint_documents_the_inlined_field_and_both_values() { diff --git a/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs b/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs index 444bfb934..89c4a39f4 100644 --- a/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs +++ b/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs @@ -8,6 +8,7 @@ using Akka.Persistence.Hosting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Netclaw.Actors.Channels; using Netclaw.Actors.Hosting; using Netclaw.Actors.Jobs; using Netclaw.Actors.Reminders; @@ -86,6 +87,8 @@ protected sealed override void ConfigureServices(HostBuilderContext context, ISe services.AddSingleton(); services.AddSingleton(NullNotificationSink.Instance); services.AddSingleton(NullReminderChannelNotifier.Instance); + services.AddSingleton(); + services.AddSingleton(sp => sp.GetRequiredService()); ConfigureSessionServices(services); services.AddLlmSessionCompositeRecords(); } diff --git a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs index 7dc950234..f43eba3ed 100644 --- a/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/SessionMessageAssemblerTests.cs @@ -363,18 +363,19 @@ public void Public_audience_static_block_contains_session_id_only() Assert.DoesNotContain("media_dir:", text); } - [Fact] - public void Personal_audience_static_block_contains_filesystem_paths() + [Theory] + [InlineData(TrustAudience.Team)] + [InlineData(TrustAudience.Personal)] + public void Trusted_audience_static_block_contains_only_authoritative_session_root(TrustAudience audience) { - // Personal audience gets the full session block with directories. - var input = MakeInput(SeedHistory("hi"), activeRecall: null, audience: TrustAudience.Personal); + var input = MakeInput(SeedHistory("hi"), activeRecall: null, audience: audience); var messages = SessionMessageAssembler.Assemble(input); var staticBlock = messages[1]; var text = staticBlock.Text ?? string.Empty; Assert.Contains("session_dir:", text); - Assert.Contains("media_dir:", text); + Assert.DoesNotContain("media_dir:", text); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs index f0816cb77..941d24990 100644 --- a/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ShellToolTests.cs @@ -61,22 +61,6 @@ public async Task Timeout_kills_long_running_process() Assert.Contains("timed out", result); } - [Fact] - public async Task Requested_timeout_overrides_default_timeout() - { - var tool = new ShellTool(new ToolConfig(), new ToolPathPolicy([]), new ShellCommandPolicy()); - var args = ToolInput.Create("Command", "sleep 1"); - var context = TestToolExecutionContext.CreateBound("test/thread", Path.GetTempPath(), new TestToolExecutionContextOptions - { - Audience = TrustAudience.Personal, - ExecutionTimeout = new ToolExecutionTimeout(TimeSpan.FromSeconds(5)) - }); - - var result = await tool.ExecuteAsync(args, context, CancellationToken.None); - - Assert.Contains("Exit code: 0", result); - } - [Fact] public async Task Caller_cancellation_kills_child_process_tree_and_returns_gracefully() { diff --git a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs index bde7a159e..e49302f4d 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolApprovalGateTests.cs @@ -367,6 +367,74 @@ public void mcp_server_default_applies_when_no_exact_override() Assert.Equal("notion/create-pages", decision.ApprovalContext!.ToolName); } + [Fact] + public void mcp_approval_context_displays_arguments_without_netclaw_meta_fields() + { + var approvalPolicy = new ToolApprovalConfig + { + DefaultMode = ToolApprovalMode.Auto, + McpServerDefaults = new Dictionary(StringComparer.Ordinal) + { + ["notion"] = ToolApprovalMode.Approval + } + }; + var policy = CreateMcpApprovalPolicy(approvalPolicy); + var content = string.Join('\n', Enumerable.Repeat("large memory payload", 1_000)); + + var decision = policy.AuthorizeInvocation( + McpTool("notion", "create-pages"), + PersonalContext(), + new Dictionary + { + ["destination_path"] = "/Board/Quarterly Results", + ["content"] = content, + ["_rationale"] = "Create the requested report" + }); + + Assert.True(decision.NeedsApproval); + Assert.Contains("destination_path=\"/Board/Quarterly Results\"", decision.ApprovalContext!.DisplayText); + Assert.Contains($"content=({content.Length} chars, 1000 lines)", decision.ApprovalContext.DisplayText); + Assert.DoesNotContain("_rationale", decision.ApprovalContext.DisplayText); + Assert.DoesNotContain("Create the requested report", decision.ApprovalContext.DisplayText); + Assert.DoesNotContain( + decision.ApprovalContext.Options, + option => option.Key.Value == ApprovalOptionKeys.ApproveAlways); + Assert.Contains( + decision.ApprovalContext.Options, + option => option.Key.Value == ApprovalOptionKeys.ApproveEverywhere + && option.Label == ApprovalOptionKeys.ApproveMcpToolLabel); + Assert.DoesNotContain( + decision.ApprovalContext.Options, + option => option.Label == ApprovalOptionKeys.ApproveEverywhereLabel); + } + + [Fact] + public void mcp_declared_parameter_that_resembles_meta_remains_visible() + { + var approvalPolicy = new ToolApprovalConfig + { + DefaultMode = ToolApprovalMode.Auto, + McpServerDefaults = new Dictionary(StringComparer.Ordinal) + { + ["notion"] = ToolApprovalMode.Approval + } + }; + var policy = CreateMcpApprovalPolicy(approvalPolicy); + var function = AIFunctionFactory.Create( + (string rationale) => rationale, + "create-pages", + "create-pages"); + var tool = new McpToolAdapter(function, "notion", "create-pages"); + + var decision = policy.AuthorizeInvocation( + tool, + PersonalContext(), + new Dictionary { ["Rationale"] = "Customer-visible reason" }); + + Assert.True(decision.NeedsApproval); + Assert.Contains("Rationale=\"Customer-visible reason\"", decision.ApprovalContext!.DisplayText); + } + [Fact] public void mcp_exact_override_beats_server_default() { @@ -521,6 +589,35 @@ public void Non_interactive_tool_requires_approval_when_policy_requires_approval Assert.NotNull(decision.ApprovalContext); } + [Fact] + public void Non_shell_tool_omits_directory_scoped_persistence_option() + { + var config = new ToolConfig(); + config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig + { + DefaultMode = ToolApprovalMode.Approval + }; + var policy = new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false)); + + var tool = new Netclaw.Actors.Tests.Memory.FakeNetclawTool("file_read", "content"); + var decision = policy.AuthorizeInvocation(tool, PersonalContext()); + + Assert.True(decision.NeedsApproval); + Assert.DoesNotContain( + decision.ApprovalContext!.Options, + option => option.Key.Value == ApprovalOptionKeys.ApproveAlways); + Assert.Contains( + decision.ApprovalContext.Options, + option => option.Key.Value == ApprovalOptionKeys.ApproveEverywhere + && option.Label == ApprovalOptionKeys.ApproveEverywhereLabel); + } + [Fact] public void Non_safe_list_tool_returns_requires_approval_when_interactive_unsupported() { diff --git a/src/Netclaw.Actors/Protocol/ApprovalOptionKeys.cs b/src/Netclaw.Actors/Protocol/ApprovalOptionKeys.cs index 32a9ec3ec..695cb211c 100644 --- a/src/Netclaw.Actors/Protocol/ApprovalOptionKeys.cs +++ b/src/Netclaw.Actors/Protocol/ApprovalOptionKeys.cs @@ -43,6 +43,7 @@ public static class ApprovalOptionKeys public const string ApproveSessionLabel = "This chat"; public const string ApproveAlwaysLabel = "Always here"; public const string ApproveEverywhereLabel = "Always anywhere"; + public const string ApproveMcpToolLabel = "Always allow this tool"; public const string DenyLabel = "Deny"; /// @@ -67,12 +68,19 @@ public static bool IsDangerStyled(string optionKey) /// Maps an approval option key to its short human-readable label. Returns /// the key unchanged when it is not a recognized option. /// - public static string LabelFor(string optionKey) => optionKey switch + public static string LabelFor(string optionKey) => LabelFor(optionKey, isMcpTool: false); + + /// + /// Maps an approval option key to its context-aware label. MCP grants are + /// tool-scoped rather than directory-scoped, so the global persistence key + /// is rendered as "Always allow this tool" instead of "Always anywhere". + /// + public static string LabelFor(string optionKey, bool isMcpTool) => optionKey switch { ApproveOnce => ApproveOnceLabel, ApproveSession => ApproveSessionLabel, ApproveAlways => ApproveAlwaysLabel, - ApproveEverywhere => ApproveEverywhereLabel, + ApproveEverywhere => isMcpTool ? ApproveMcpToolLabel : ApproveEverywhereLabel, Deny => DenyLabel, _ => optionKey }; diff --git a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs index 9a3c1ec28..fb52acd85 100644 --- a/src/Netclaw.Actors/Sessions/LlmSessionActor.cs +++ b/src/Netclaw.Actors/Sessions/LlmSessionActor.cs @@ -3907,7 +3907,9 @@ private bool TryResolveTextApprovalResponse( ]; var options = optionKeys - .Select(key => new ToolInteractionOption(new ApprovalOptionKey(key), ApprovalOptionKeys.LabelFor(key))) + .Select(key => new ToolInteractionOption( + new ApprovalOptionKey(key), + ApprovalOptionKeys.LabelFor(key, new ToolName(pending.ToolName).IsMcp))) .ToArray(); if (!ToolInteractionResponseParser.TryParseApprovalResponse(msg.Text, options, out var selectedKey) diff --git a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs index cbfea2aab..749411716 100644 --- a/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs +++ b/src/Netclaw.Actors/Sessions/SessionMessageAssembler.cs @@ -97,6 +97,8 @@ public static class SessionMessageAssembler "Your session working directory contains an `inbox/` subdirectory where user-uploaded files are placed.\n" + "Each attachment is announced in the inbound message as a single line of the form:\n" + " [attachment] name=\"...\" mime=\"...\" size=... path=\"inbox/...\" inlined=\"true|false\" [note=\"...\"]\n" + + "The announced `path` is authoritative, relative to `session_dir`, and already includes any collision-safe filename change. " + + "Use `{session_dir}/{path}` when you need the absolute path on the host; do not search other session subdirectories for another copy.\n" + "When `inlined=\"true\"` you can see the file content natively in this turn.\n" + "When `inlined=\"false\"`:\n" + " - If `note` begins with \"current model has no\": the file exists on disk but you cannot render it natively. " + @@ -169,9 +171,7 @@ private static string BuildStaticContextBlock(ContextAssemblyInput input, string } else { - var sessionBlock = $"[session]\nid: {input.SessionId.Value}" - + $"\nsession_dir: {sessionDir}" - + $"\nmedia_dir: {Path.Combine(sessionDir, SessionDirectoryHelper.MediaSubdirectory)}"; + var sessionBlock = $"[session]\nid: {input.SessionId.Value}" + $"\nsession_dir: {sessionDir}"; parts.Add(sessionBlock); } diff --git a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs index 05405f379..28952072f 100644 --- a/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs +++ b/src/Netclaw.Actors/Tools/ToolAccessPolicy.cs @@ -109,7 +109,10 @@ public ToolAccessDecision AuthorizeInvocation( return ToolAccessDecision.Deny("mcp_tool_not_allowed_for_audience_profile"); var mcpToolName = new ToolName(tool.Name); - return CheckApprovalGate(mcpToolName, context, arguments, DefaultApprovalMatcher.Instance); + var (_, approvalArguments) = ToolCallMeta.ExtractFrom( + arguments, + key => ToolArgumentValidator.ResolveMetaField(mcp, key)); + return CheckApprovalGate(mcpToolName, context, approvalArguments, McpApprovalMatcher.Instance); } var toolName = new ToolName(tool.Name); @@ -344,7 +347,9 @@ private ToolAccessDecision CheckApprovalGate( isMessy, isCwdShallow: IsCwdTooShallow(context.Approval.Cwd), allEffectiveDirsAreSessionScratch: AllCandidatesResolveToSessionScratch( - candidates, context.Approval.Cwd, context.SessionDirectory)); + candidates, context.Approval.Cwd, context.SessionDirectory), + supportsDirectoryScope: matcher is ShellApprovalMatcher, + isMcpTool: toolName.IsMcp); var approvalContext = new ToolApprovalContext( toolName.Value, @@ -408,12 +413,18 @@ private static bool AllCandidatesResolveToSessionScratch( /// to a directory that won't recur. This chat already provides /// the equivalent in-session semantics without polluting the persistent /// store. + /// No directory scope (all non-shell tools) — Always + /// here is omitted because these matchers grant independently of cwd. + /// For MCP tools, the remaining persistent choice is labeled Always + /// allow this tool because it persists a canonical-tool grant. /// /// private static IReadOnlyList BuildApprovalOptions( bool isMessy, bool isCwdShallow, - bool allEffectiveDirsAreSessionScratch) + bool allEffectiveDirsAreSessionScratch, + bool supportsDirectoryScope, + bool isMcpTool) { if (isMessy) { @@ -430,12 +441,14 @@ private static IReadOnlyList BuildApprovalOptions( new ToolApprovalOption(ApprovalOptionKeys.ApproveSessionKey, ApprovalOptionKeys.ApproveSessionLabel) }; - if (!isCwdShallow && !allEffectiveDirsAreSessionScratch) + if (supportsDirectoryScope && !isCwdShallow && !allEffectiveDirsAreSessionScratch) { options.Add(new ToolApprovalOption(ApprovalOptionKeys.ApproveAlwaysKey, ApprovalOptionKeys.ApproveAlwaysLabel)); } - options.Add(new ToolApprovalOption(ApprovalOptionKeys.ApproveEverywhereKey, ApprovalOptionKeys.ApproveEverywhereLabel)); + options.Add(new ToolApprovalOption( + ApprovalOptionKeys.ApproveEverywhereKey, + ApprovalOptionKeys.LabelFor(ApprovalOptionKeys.ApproveEverywhere, isMcpTool))); options.Add(new ToolApprovalOption(ApprovalOptionKeys.DenyKey, ApprovalOptionKeys.DenyLabel)); return options; diff --git a/src/Netclaw.Channels.Discord/DiscordApprovalPromptBuilder.cs b/src/Netclaw.Channels.Discord/DiscordApprovalPromptBuilder.cs index f05e1d305..34db8e458 100644 --- a/src/Netclaw.Channels.Discord/DiscordApprovalPromptBuilder.cs +++ b/src/Netclaw.Channels.Discord/DiscordApprovalPromptBuilder.cs @@ -6,6 +6,7 @@ using System.Text; using Netclaw.Actors.Protocol; using Netclaw.Channels; +using Netclaw.Tools; using static Netclaw.Actors.Sessions.SessionProtocol; namespace Netclaw.Channels.Discord; @@ -25,13 +26,14 @@ internal static class DiscordApprovalPromptBuilder public static string BuildTextPrompt(ToolInteractionRequest request) { var sb = new StringBuilder(); - sb.AppendLine("Netclaw approval required:"); + sb.AppendLine(request.ToolName.IsMcp ? "MCP tool approval required:" : "Netclaw approval required:"); sb.Append("Tool: ").AppendLine(request.ToolName.Value); - sb.Append("Action: ").AppendLine(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)); + sb.Append(request.ToolName.IsMcp ? "Invocation: " : "Action: ") + .AppendLine(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)); sb.AppendLine(BuildApproveHeader(request)); var verbs = ResolveDisplayVerbs(request); - if (verbs.Count > 1) + if (!request.ToolName.IsMcp && verbs.Count > 1) { foreach (var verb in verbs) sb.Append(" • ").AppendLine(verb); @@ -52,7 +54,9 @@ public static (string Text, IReadOnlyList Buttons) BuildButto ToolInteractionRequest request) { var sb = new StringBuilder(); - sb.AppendLine(":lock: **Tool approval required**"); + sb.Append(":lock: **") + .Append(request.ToolName.IsMcp ? "MCP tool approval required" : "Tool approval required") + .AppendLine("**"); AppendToolSummary(sb, request); sb.AppendLine(); @@ -87,9 +91,12 @@ public static string BuildResolvedPromptText( : ":white_check_mark:"; var sb = new StringBuilder(); - sb.Append(statusEmoji).AppendLine(" **Tool approval resolved**"); + sb.Append(statusEmoji) + .Append(request.ToolName.IsMcp ? " **MCP tool approval resolved**" : " **Tool approval resolved**") + .AppendLine(); sb.Append("**Tool:** `").Append(request.ToolName).AppendLine("`"); - sb.Append("**Action:** `").Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); + sb.Append(request.ToolName.IsMcp ? "**Invocation:** `" : "**Action:** `") + .Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); sb.Append("**").Append(BuildResolutionLine(request, selectedKey)).Append("**"); sb.Append(" (by <@").Append(senderId).Append(">)"); @@ -126,24 +133,36 @@ public static string BuildResolvedPromptTextWithoutRequest( : ":white_check_mark:"; var sb = new StringBuilder(); - sb.Append(statusEmoji).AppendLine(" **Tool approval resolved**"); + var isMcpTool = !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; + sb.Append(statusEmoji) + .Append(isMcpTool ? " **MCP tool approval resolved**" : " **Tool approval resolved**") + .AppendLine(); if (!string.IsNullOrEmpty(toolName) && !string.IsNullOrEmpty(displayText)) { sb.Append("**Tool:** `").Append(toolName).AppendLine("`"); - sb.Append("**Action:** `") + sb.Append(isMcpTool ? "**Invocation:** `" : "**Action:** `") .Append(ApprovalDisplayTextFormatter.Truncate(displayText, MaxDisplayTextChars)) .AppendLine("`"); } - sb.Append("**").Append(BuildGenericResolutionLine(selectedKey)).Append("**"); + sb.Append("**").Append(BuildGenericResolutionLine(selectedKey, isMcpTool)).Append("**"); sb.Append(" (by <@").Append(senderId).Append(">)"); return sb.ToString(); } - private static string BuildGenericResolutionLine(string selectedKey) - => selectedKey switch + private static string BuildGenericResolutionLine(string selectedKey, bool isMcpTool) + => isMcpTool + ? selectedKey switch + { + ApprovalOptionKeys.ApproveAlways or ApprovalOptionKeys.ApproveEverywhere => "Always allowed this MCP tool", + ApprovalOptionKeys.ApproveSession => "Allowed this MCP tool for this chat", + ApprovalOptionKeys.ApproveOnce => "Allowed once", + ApprovalOptionKeys.Deny => "Denied", + _ => "Resolved" + } + : selectedKey switch { ApprovalOptionKeys.ApproveAlways => "Saved: always here", ApprovalOptionKeys.ApproveEverywhere => "Saved: always anywhere", @@ -156,11 +175,12 @@ private static string BuildGenericResolutionLine(string selectedKey) private static void AppendToolSummary(StringBuilder sb, ToolInteractionRequest request) { sb.Append("**Tool:** `").Append(request.ToolName).AppendLine("`"); - sb.Append("**Action:** `").Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); + sb.Append(request.ToolName.IsMcp ? "**Invocation:** `" : "**Action:** `") + .Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); sb.Append("**").Append(BuildApproveHeader(request)).AppendLine("**"); var verbs = ResolveDisplayVerbs(request); - if (verbs.Count > 1) + if (!request.ToolName.IsMcp && verbs.Count > 1) { foreach (var verb in verbs) sb.Append(" • `").Append(verb).AppendLine("`"); @@ -180,6 +200,9 @@ private static void AppendToolSummary(StringBuilder sb, ToolInteractionRequest r /// private static string BuildApproveHeader(ToolInteractionRequest request) { + if (request.ToolName.IsMcp) + return "Allow this MCP tool invocation?"; + var verbs = ResolveDisplayVerbs(request); var location = ResolveHeaderLocation(request); @@ -195,6 +218,18 @@ private static string BuildApproveHeader(ToolInteractionRequest request) /// private static string BuildResolutionLine(ToolInteractionRequest request, string selectedKey) { + if (request.ToolName.IsMcp) + { + return selectedKey switch + { + ApprovalOptionKeys.ApproveAlways or ApprovalOptionKeys.ApproveEverywhere => $"Always allowed: {request.ToolName}", + ApprovalOptionKeys.ApproveSession => $"Allowed for this chat: {request.ToolName}", + ApprovalOptionKeys.ApproveOnce => "Allowed once", + ApprovalOptionKeys.Deny => "Denied", + _ => "Resolved" + }; + } + var verbs = string.Join(", ", ResolveDisplayVerbs(request)); var location = ResolveHeaderLocation(request); diff --git a/src/Netclaw.Channels.Discord/DiscordIngressMessages.cs b/src/Netclaw.Channels.Discord/DiscordIngressMessages.cs index a3841e228..f8b45d6b2 100644 --- a/src/Netclaw.Channels.Discord/DiscordIngressMessages.cs +++ b/src/Netclaw.Channels.Discord/DiscordIngressMessages.cs @@ -92,8 +92,11 @@ public PendingApprovalRequest( RequesterSenderId = requesterSenderId; RequesterPrincipal = requesterPrincipal; OptionKeys = [.. optionKeys]; + var isMcpTool = !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; Options = OptionKeys - .Select(key => new ToolInteractionOption(new ApprovalOptionKey(key), ApprovalOptionKeys.LabelFor(key))) + .Select(key => new ToolInteractionOption( + new ApprovalOptionKey(key), + ApprovalOptionKeys.LabelFor(key, isMcpTool))) .ToArray(); PromptMessageId = promptMessageId; ToolName = toolName; diff --git a/src/Netclaw.Channels.Mattermost/MattermostApprovalPromptBuilder.cs b/src/Netclaw.Channels.Mattermost/MattermostApprovalPromptBuilder.cs index 2fd466307..847888199 100644 --- a/src/Netclaw.Channels.Mattermost/MattermostApprovalPromptBuilder.cs +++ b/src/Netclaw.Channels.Mattermost/MattermostApprovalPromptBuilder.cs @@ -6,6 +6,7 @@ using System.Text; using Netclaw.Actors.Protocol; using Netclaw.Channels; +using Netclaw.Tools; using static Netclaw.Actors.Sessions.SessionProtocol; namespace Netclaw.Channels.Mattermost; @@ -28,8 +29,8 @@ public static (string Text, IReadOnlyList Attachments) Bui MattermostCallbackActionStore? actionStore = null) { var sb = new StringBuilder(); - sb.AppendLine(":lock: **Tool approval required**"); - AppendToolSummary(sb, request); + AppendApprovalTitle(sb, request); + AppendToolSummary(sb, request, includeApprovalQuestion: true); sb.AppendLine(); sb.Append("You can also reply with ").Append(FormatReplyLetters(request.Options)).Append(" in this thread."); @@ -62,7 +63,7 @@ public static (string Text, IReadOnlyList Attachments) Bui .ToList(); var attachment = new MattermostAttachment( - Fallback: $"Tool approval required — reply with {string.Join(", ", Enumerable.Range(0, actions.Count).Select(GetReplyLetter))}", + Fallback: $"{(request.ToolName.IsMcp ? "MCP tool approval required" : "Tool approval required")} — reply with {string.Join(", ", Enumerable.Range(0, actions.Count).Select(GetReplyLetter))}", Color: "#3AA3E3", Actions: actions); @@ -72,8 +73,8 @@ public static (string Text, IReadOnlyList Attachments) Bui public static string BuildTextPrompt(ToolInteractionRequest request) { var sb = new StringBuilder(); - sb.AppendLine(":lock: **Tool approval required**"); - AppendToolSummary(sb, request); + AppendApprovalTitle(sb, request); + AppendToolSummary(sb, request, includeApprovalQuestion: true); sb.AppendLine(); sb.AppendLine("Reply with:"); @@ -81,9 +82,9 @@ public static string BuildTextPrompt(ToolInteractionRequest request) return sb.ToString().TrimEnd(); } - public static string BuildDecisionStatus(string selectedKey) + public static string BuildDecisionStatus(string selectedKey, ToolName toolName) { - var label = ApprovalOptionKeys.LabelFor(selectedKey); + var label = ApprovalOptionKeys.LabelFor(selectedKey, toolName.IsMcp); return $"Recorded approval decision: {label}."; } @@ -95,11 +96,13 @@ public static string BuildResolvedPromptText( var statusEmoji = selectedKey == ApprovalOptionKeys.Deny ? ":no_entry:" : ":white_check_mark:"; - var decisionLabel = ApprovalOptionKeys.LabelFor(selectedKey); + var decisionLabel = ApprovalOptionKeys.LabelFor(selectedKey, request.ToolName.IsMcp); var sb = new StringBuilder(); - sb.Append(statusEmoji).AppendLine(" **Tool approval resolved**"); - AppendToolSummary(sb, request); + sb.Append(statusEmoji) + .Append(request.ToolName.IsMcp ? " **MCP tool approval resolved**" : " **Tool approval resolved**") + .AppendLine(); + AppendToolSummary(sb, request, includeApprovalQuestion: false); sb.Append("**Decision:** ").Append(decisionLabel); sb.Append(" (by @").Append(senderId).Append(')'); @@ -141,15 +144,18 @@ public static MattermostAttachment BuildResolvedAttachmentWithoutRequest( var statusEmoji = selectedKey == ApprovalOptionKeys.Deny ? ":no_entry:" : ":white_check_mark:"; - var decisionLabel = ApprovalOptionKeys.LabelFor(selectedKey); + var isMcpTool = !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; + var decisionLabel = ApprovalOptionKeys.LabelFor(selectedKey, isMcpTool); var sb = new StringBuilder(); - sb.Append(statusEmoji).AppendLine(" **Tool approval resolved**"); + sb.Append(statusEmoji) + .Append(isMcpTool ? " **MCP tool approval resolved**" : " **Tool approval resolved**") + .AppendLine(); if (!string.IsNullOrEmpty(toolName) && !string.IsNullOrEmpty(displayText)) { sb.Append("**Tool:** `").Append(toolName).AppendLine("`"); - sb.Append("**Action:** `") + sb.Append(isMcpTool ? "**Invocation:** `" : "**Action:** `") .Append(ApprovalDisplayTextFormatter.Truncate(displayText, MaxDisplayTextChars)) .AppendLine("`"); } @@ -166,12 +172,20 @@ public static MattermostAttachment BuildResolvedAttachmentWithoutRequest( Text: resolvedText); } - private static void AppendToolSummary(StringBuilder sb, ToolInteractionRequest request) + private static void AppendToolSummary( + StringBuilder sb, + ToolInteractionRequest request, + bool includeApprovalQuestion) { sb.Append("**Tool:** `").Append(request.ToolName).AppendLine("`"); - sb.Append("**Action:** `").Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); + sb.Append(request.ToolName.IsMcp ? "**Invocation:** `" : "**Action:** `") + .Append(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)).AppendLine("`"); - if (request.Patterns.Count > 0) + if (request.ToolName.IsMcp && includeApprovalQuestion) + { + sb.AppendLine("**Allow this MCP tool invocation?**"); + } + else if (request.Patterns.Count > 0) { if (request.Patterns.Count == 1) { @@ -188,6 +202,11 @@ private static void AppendToolSummary(StringBuilder sb, ToolInteractionRequest r AppendAdoptedContextSummary(sb, request); } + private static void AppendApprovalTitle(StringBuilder sb, ToolInteractionRequest request) + => sb.Append(":lock: **") + .Append(request.ToolName.IsMcp ? "MCP tool approval required" : "Tool approval required") + .AppendLine("**"); + private static void AppendAdoptedContextSummary(StringBuilder sb, ToolInteractionRequest request) { if (!request.HasAdoptedContext) diff --git a/src/Netclaw.Channels.Mattermost/MattermostIngressMessages.cs b/src/Netclaw.Channels.Mattermost/MattermostIngressMessages.cs index b0fd2183b..e306cb80e 100644 --- a/src/Netclaw.Channels.Mattermost/MattermostIngressMessages.cs +++ b/src/Netclaw.Channels.Mattermost/MattermostIngressMessages.cs @@ -85,8 +85,11 @@ public PendingApprovalRequest( RequesterSenderId = requesterSenderId; RequesterPrincipal = requesterPrincipal; OptionKeys = [.. optionKeys]; + var isMcpTool = !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; Options = OptionKeys - .Select(key => new ToolInteractionOption(new ApprovalOptionKey(key), ApprovalOptionKeys.LabelFor(key))) + .Select(key => new ToolInteractionOption( + new ApprovalOptionKey(key), + ApprovalOptionKeys.LabelFor(key, isMcpTool))) .ToArray(); PromptPostId = promptPostId; ToolName = toolName; diff --git a/src/Netclaw.Channels.Slack/SlackApprovalBlockBuilder.cs b/src/Netclaw.Channels.Slack/SlackApprovalBlockBuilder.cs index bb0d19f5f..99f6f9cfe 100644 --- a/src/Netclaw.Channels.Slack/SlackApprovalBlockBuilder.cs +++ b/src/Netclaw.Channels.Slack/SlackApprovalBlockBuilder.cs @@ -4,6 +4,7 @@ // // ----------------------------------------------------------------------- using Netclaw.Actors.Protocol; +using Netclaw.Tools; using SlackNet.Blocks; using static Netclaw.Actors.Sessions.SessionProtocol; @@ -28,13 +29,13 @@ public static string BuildApprovalText(ToolInteractionRequest request) { var lines = new List { - ":lock: *Tool approval required*", + $":lock: *{ApprovalTitle(request.ToolName)}*", $"> `{request.ToolName}`: `{ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)}`", BuildApproveHeader(request) }; var verbs = ResolveDisplayVerbs(request); - if (verbs.Count > 1) + if (!request.ToolName.IsMcp && verbs.Count > 1) { foreach (var verb in verbs) lines.Add($" • `{verb}`"); @@ -59,11 +60,13 @@ public static IReadOnlyList BuildApprovalBlocks(ToolInteractionRequest re { new SectionBlock { - Text = new Markdown(":lock: *Tool approval required*") + Text = new Markdown($":lock: *{ApprovalTitle(request.ToolName)}*") }, new SectionBlock { - Text = new Markdown($"*Tool:* `{EscapeMarkdown(request.ToolName.Value)}`\n*Request:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars))}`"), + Text = new Markdown( + $"*Tool:* `{EscapeMarkdown(request.ToolName.Value)}`\n" + + $"*{RequestLabel(request.ToolName)}:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars))}`"), Expand = true }, new SectionBlock @@ -73,7 +76,7 @@ public static IReadOnlyList BuildApprovalBlocks(ToolInteractionRequest re }; var verbs = ResolveDisplayVerbs(request); - if (verbs.Count > 1) + if (!request.ToolName.IsMcp && verbs.Count > 1) { var verbLines = verbs.Select(v => $"• `{EscapeMarkdown(v)}`"); blocks.Add(new SectionBlock @@ -134,7 +137,7 @@ public static string BuildResolvedApprovalText( return string.Join("\n", new[] { - $"{statusPrefix} *Tool approval resolved* by <@{EscapeMarkdown(senderId)}>", + $"{statusPrefix} *{ApprovalResolvedTitle(request.ToolName)}* by <@{EscapeMarkdown(senderId)}>", $"> `{request.ToolName}`: `{ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars)}`", BuildResolutionLine(request, selectedKey) }); @@ -154,13 +157,13 @@ public static IReadOnlyList BuildResolvedApprovalBlocks( { new SectionBlock { - Text = new Markdown($"{statusPrefix} *Tool approval resolved* by <@{EscapeMarkdown(senderId)}>") + Text = new Markdown($"{statusPrefix} *{ApprovalResolvedTitle(request.ToolName)}* by <@{EscapeMarkdown(senderId)}>") }, new SectionBlock { Text = new Markdown( $"*Tool:* `{EscapeMarkdown(request.ToolName.Value)}`\n" - + $"*Request:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars))}`\n" + + $"*{RequestLabel(request.ToolName)}:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(request.DisplayText, MaxDisplayTextChars))}`\n" + $"*{EscapeMarkdown(resolutionLine)}*"), Expand = true } @@ -208,7 +211,7 @@ public static string BuildResolvedApprovalTextWithoutRequest( var lines = new List(3) { - $"{statusPrefix} *Tool approval resolved* by <@{EscapeMarkdown(senderId)}>" + $"{statusPrefix} *{ApprovalResolvedTitle(toolName)}* by <@{EscapeMarkdown(senderId)}>" }; if (!string.IsNullOrEmpty(toolName) && !string.IsNullOrEmpty(displayText)) @@ -217,7 +220,7 @@ public static string BuildResolvedApprovalTextWithoutRequest( $"> `{toolName}`: `{ApprovalDisplayTextFormatter.Truncate(displayText, MaxDisplayTextChars)}`"); } - lines.Add(BuildGenericResolutionLine(selectedKey)); + lines.Add(BuildGenericResolutionLine(selectedKey, IsMcpToolName(toolName))); return string.Join("\n", lines); } @@ -241,7 +244,7 @@ public static IReadOnlyList BuildResolvedApprovalBlocksWithoutRequest( { new SectionBlock { - Text = new Markdown($"{statusPrefix} *Tool approval resolved* by <@{EscapeMarkdown(senderId)}>") + Text = new Markdown($"{statusPrefix} *{ApprovalResolvedTitle(toolName)}* by <@{EscapeMarkdown(senderId)}>") } }; @@ -251,8 +254,8 @@ public static IReadOnlyList BuildResolvedApprovalBlocksWithoutRequest( { Text = new Markdown( $"*Tool:* `{EscapeMarkdown(toolName)}`\n" - + $"*Request:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(displayText, MaxDisplayTextChars))}`\n" - + $"*{EscapeMarkdown(BuildGenericResolutionLine(selectedKey))}*"), + + $"*{RequestLabel(toolName)}:* `{EscapeMarkdown(ApprovalDisplayTextFormatter.Truncate(displayText, MaxDisplayTextChars))}`\n" + + $"*{EscapeMarkdown(BuildGenericResolutionLine(selectedKey, IsMcpToolName(toolName)))}*"), Expand = true }); } @@ -260,15 +263,24 @@ public static IReadOnlyList BuildResolvedApprovalBlocksWithoutRequest( { blocks.Add(new SectionBlock { - Text = new Markdown($"*{EscapeMarkdown(BuildGenericResolutionLine(selectedKey))}*") + Text = new Markdown($"*{EscapeMarkdown(BuildGenericResolutionLine(selectedKey, isMcpTool: false))}*") }); } return blocks; } - private static string BuildGenericResolutionLine(string selectedKey) - => selectedKey switch + private static string BuildGenericResolutionLine(string selectedKey, bool isMcpTool) + => isMcpTool + ? selectedKey switch + { + ApprovalOptionKeys.ApproveAlways or ApprovalOptionKeys.ApproveEverywhere => "Always allowed this MCP tool", + ApprovalOptionKeys.ApproveSession => "Allowed this MCP tool for this chat", + ApprovalOptionKeys.ApproveOnce => "Allowed once", + ApprovalOptionKeys.Deny => "Denied", + _ => "Resolved" + } + : selectedKey switch { ApprovalOptionKeys.ApproveAlways => "Saved: always here", ApprovalOptionKeys.ApproveEverywhere => "Saved: always anywhere", @@ -299,6 +311,9 @@ private static string BuildGenericResolutionLine(string selectedKey) /// private static string BuildApproveHeader(ToolInteractionRequest request) { + if (request.ToolName.IsMcp) + return "Allow this MCP tool invocation?"; + var verbs = ResolveDisplayVerbs(request); var location = ResolveHeaderLocation(request); @@ -354,6 +369,18 @@ private static bool IsSessionScratchPath(string cwd) /// private static string BuildResolutionLine(ToolInteractionRequest request, string selectedKey) { + if (request.ToolName.IsMcp) + { + return selectedKey switch + { + ApprovalOptionKeys.ApproveAlways or ApprovalOptionKeys.ApproveEverywhere => $"Always allowed: {request.ToolName}", + ApprovalOptionKeys.ApproveSession => $"Allowed for this chat: {request.ToolName}", + ApprovalOptionKeys.ApproveOnce => "Allowed once", + ApprovalOptionKeys.Deny => "Denied", + _ => "Resolved" + }; + } + var verbs = string.Join(", ", ResolveDisplayVerbs(request)); var location = ResolveHeaderLocation(request); @@ -380,6 +407,24 @@ private static IReadOnlyList ResolveDisplayVerbs(ToolInteractionRequest ? request.CandidateVerbs : request.Patterns; + private static string ApprovalTitle(ToolName toolName) + => toolName.IsMcp ? "MCP tool approval required" : "Tool approval required"; + + private static string ApprovalResolvedTitle(ToolName toolName) + => toolName.IsMcp ? "MCP tool approval resolved" : "Tool approval resolved"; + + private static string ApprovalResolvedTitle(string? toolName) + => IsMcpToolName(toolName) ? "MCP tool approval resolved" : "Tool approval resolved"; + + private static string RequestLabel(ToolName toolName) + => toolName.IsMcp ? "Invocation" : "Request"; + + private static string RequestLabel(string toolName) + => new ToolName(toolName).IsMcp ? "Invocation" : "Request"; + + private static bool IsMcpToolName(string? toolName) + => !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; + private static void AppendAdoptedContextSummary(List lines, ToolInteractionRequest request) { if (!request.HasAdoptedContext) diff --git a/src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs b/src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs index b58891f38..678fbf2d1 100644 --- a/src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs +++ b/src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs @@ -1835,8 +1835,11 @@ public PendingApprovalRequest( RequesterSenderId = requesterSenderId; RequesterPrincipal = requesterPrincipal; OptionKeys = [.. optionKeys]; + var isMcpTool = !string.IsNullOrEmpty(toolName) && new ToolName(toolName).IsMcp; Options = OptionKeys - .Select(key => new ToolInteractionOption(new ApprovalOptionKey(key), ApprovalOptionKeys.LabelFor(key))) + .Select(key => new ToolInteractionOption( + new ApprovalOptionKey(key), + ApprovalOptionKeys.LabelFor(key, isMcpTool))) .ToArray(); PromptMessageTs = promptMessageTs; ToolName = toolName; diff --git a/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs index db130cdc5..78ff21425 100644 --- a/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs +++ b/src/Netclaw.Cli.Tests/Daemon/DaemonPathEnvironmentFileTests.cs @@ -108,6 +108,15 @@ public void BuildDaemonUnitContent_WiresEnvironmentFile_AndOmitsInlinePath() Assert.DoesNotContain("Environment=PATH=", unit, StringComparison.Ordinal); Assert.Contains("ExecStart=/opt/netclaw/netclawd", unit, StringComparison.Ordinal); Assert.Contains("ExecStop=/opt/netclaw/netclaw daemon stop", unit, StringComparison.Ordinal); + + // Producer/consumer contract (netclaw-dev/netclaw#1665): the generated unit's + // TimeoutStopSec= must track DaemonConfig.SystemdTimeoutStopSec, not a stale literal, + // so systemd never SIGKILLs the cgroup out from under a still-legitimately-waiting + // `netclaw daemon stop` (ExecStop=). + Assert.Contains( + $"TimeoutStopSec={(int)DaemonConfig.SystemdTimeoutStopSec.TotalSeconds}", + unit, + StringComparison.Ordinal); } [Fact] diff --git a/src/Netclaw.Cli.Tests/Doctor/ContextWindowDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/ContextWindowDoctorCheckTests.cs index ef9b0b29a..0604b9729 100644 --- a/src/Netclaw.Cli.Tests/Doctor/ContextWindowDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/ContextWindowDoctorCheckTests.cs @@ -52,6 +52,38 @@ public async Task NoModelsMainSection_ReturnsWarning() Assert.DoesNotContain("32,768", result.Message); } + [Fact] + public async Task MissingNamedDefinition_ReturnsActionableError() + { + WriteConfig(new + { + configVersion = 1, + Models = new + { + Definitions = new Dictionary + { + ["known"] = new + { + Provider = "my-ollama", + ModelId = "qwen3:30b" + } + }, + Roles = new + { + Main = "missing" + } + } + }); + var check = CreateCheck(CreateOfflineDaemonApi()); + + var result = await check.RunAsync(TestContext.Current.CancellationToken); + + Assert.Equal(DoctorSeverity.Error, result.Severity); + Assert.Contains("Models:Roles:Main references unknown definition 'missing'.", result.Message); + Assert.Contains("Fix the Models configuration", result.Remediation); + Assert.DoesNotContain("doctor --fix", result.Remediation); + } + [Fact] public async Task MainModelWithoutProvider_ReturnsUnavailableWithoutDefaultProvider() { diff --git a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs index 245bb3d22..98a3a96a0 100644 --- a/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/DoctorFixServiceTests.cs @@ -5,12 +5,14 @@ // ----------------------------------------------------------------------- using Netclaw.Cli.Daemon; using System.Text.Json; +using Netclaw.Cli.Config; using Netclaw.Cli.Doctor; using Netclaw.Configuration; using Xunit; namespace Netclaw.Cli.Tests.Doctor; +[Collection(Netclaw.Cli.Tests.LegacyModelEnvironmentCollection.Name)] public sealed class DoctorFixServiceTests { // POSIX install dir: systemd units are always POSIX-style regardless of the host OS @@ -283,6 +285,84 @@ public async Task DoesNotThrow_WhenUnitEnvironmentFilePathIsMalformed() Assert.DoesNotContain(plan.Fixes, f => f.FilePath == paths.DaemonEnvironmentFilePath); } + [Fact] + public async Task LegacyEnvironmentOverride_BlocksMigrationWithoutChangingConfig() + { + var paths = NewPaths(); + const string config = + """ + { + "configVersion": 1, + "Models": { + "Main": { + "Provider": "vllm", + "ModelId": "qwen-vl" + } + } + } + """; + await File.WriteAllTextAsync(paths.NetclawConfigPath, config, TestContext.Current.CancellationToken); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + var service = ConfigOnlyService(paths); + + var exception = await Assert.ThrowsAsync( + () => service.BuildPlanAsync(TestContext.Current.CancellationToken)); + + Assert.Contains(envVar, exception.Message, StringComparison.Ordinal); + Assert.Equal(config, await File.ReadAllTextAsync( + paths.NetclawConfigPath, TestContext.Current.CancellationToken)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + + [Fact] + public async Task LegacyEnvironmentOverride_DoesNotBlockAlreadyNamedModels() + { + var paths = NewPaths(); + const string config = + """ + { + "configVersion": 1, + "Models": { + "Definitions": { + "main": { + "Provider": "vllm", + "ModelId": "qwen-vl" + } + }, + "Roles": { + "Main": "main" + } + } + } + """; + await File.WriteAllTextAsync(paths.NetclawConfigPath, config, TestContext.Current.CancellationToken); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + var service = ConfigOnlyService(paths); + + var plan = await service.BuildPlanAsync(TestContext.Current.CancellationToken); + + Assert.DoesNotContain(plan.Fixes, fix => fix.FilePath == paths.NetclawConfigPath); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + private static NetclawPaths NewPaths() { var paths = new NetclawPaths(CreateTempBasePath()); diff --git a/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs b/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs index b541261d7..7917fb3fc 100644 --- a/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs +++ b/src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs @@ -12,6 +12,7 @@ namespace Netclaw.Cli.Tests.Model; +[Collection(Netclaw.Cli.Tests.LegacyModelEnvironmentCollection.Name)] public sealed class ModelCommandTests : IDisposable { private readonly DisposableTempDir _dir = new(); @@ -439,6 +440,11 @@ public async Task Set_ClearModalities_RemovesExistingOverride() [InlineData("--input-modalities requires a value", "--input-modalities")] [InlineData("unknown argument '--input-modalites'", "--input-modalites", "Text")] [InlineData("invalid modalities", "--input-modalities", "3")] + [InlineData("invalid modalities", "--input-modalities", "1")] + [InlineData("invalid modalities", "--input-modalities", "2")] + [InlineData("invalid modalities", "--input-modalities", "4")] + [InlineData("invalid modalities", "--input-modalities", "8")] + [InlineData("invalid modalities", "--output-modalities", "1")] [InlineData("cannot be combined", "--context-window", "32768", "--clear-context-window")] public async Task Set_InvalidOptions_ReturnErrorWithoutWriting( string expectedError, @@ -589,6 +595,39 @@ public async Task List_CorruptModalityConfig_ReturnsErrorWithoutCrashing() Assert.Contains("could not be parsed", _output.ToString()); } + [Fact] + public async Task List_MissingNamedDefinition_SurfacesResolverErrorWithoutAutofixGuidance() + { + WriteConfig(new Dictionary + { + ["configVersion"] = 1, + ["Models"] = new Dictionary + { + ["Definitions"] = new Dictionary + { + ["known"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b" + } + }, + ["Roles"] = new Dictionary + { + ["Main"] = "missing" + } + } + }); + + var exitCode = await ModelCommand.RunAsync(["model", "list"], _paths, output: _output); + + var output = _output.ToString(); + Assert.Equal(1, exitCode); + Assert.Contains("Models:Roles:Main references unknown definition 'missing'.", output); + Assert.Contains("Fix the Models section", output); + Assert.DoesNotContain("could not be parsed", output); + Assert.DoesNotContain("doctor --fix", output); + } + [Fact] public async Task Set_CorruptModalityButValidWindow_PreservesWindowEndToEnd() { @@ -613,6 +652,115 @@ public async Task Set_CorruptModalityButValidWindow_PreservesWindowEndToEnd() Assert.False(main.TryGetProperty("InputModalities", out _)); // corrupt override dropped } + [Fact] + public async Task Set_LegacyEnvironmentOverride_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b" + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + const string envVar = "NETCLAW_Models__Main__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + + var exitCode = await ModelCommand.RunAsync( + ["model", "set", "main", "my-ollama", "qwen3:8b"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + $"Error: Cannot migrate Models while legacy environment override '{envVar}' is set.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + + [Fact] + public async Task Set_ConflictingLegacyRoles_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b", + ["ContextWindow"] = 32768 + }, + ["Fallback"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b", + ["ContextWindow"] = 65536 + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + + var exitCode = await ModelCommand.RunAsync( + ["model", "set", "compaction", "my-ollama", "qwen3:30b"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + "Error: Legacy model roles conflict for my-ollama/qwen3:30b; align their metadata before migration.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + + [Fact] + public async Task Clear_LegacyEnvironmentOverride_ReturnsErrorWithoutChangingConfig() + { + var config = ProvidersOnly(); + config["Models"] = new Dictionary + { + ["Main"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:30b" + }, + ["Fallback"] = new Dictionary + { + ["Provider"] = "my-ollama", + ["ModelId"] = "qwen3:8b" + } + }; + WriteConfig(config); + var original = File.ReadAllText(_paths.NetclawConfigPath); + const string envVar = "NETCLAW_Models__Fallback__ContextWindow"; + var previous = Environment.GetEnvironmentVariable(envVar); + + try + { + Environment.SetEnvironmentVariable(envVar, "65536"); + + var exitCode = await ModelCommand.RunAsync( + ["model", "clear", "fallback"], _paths, output: _output); + + Assert.Equal(1, exitCode); + Assert.Contains( + $"Error: Cannot migrate Models while legacy environment override '{envVar}' is set.", + _output.ToString(), StringComparison.Ordinal); + Assert.Equal(original, File.ReadAllText(_paths.NetclawConfigPath)); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + private static Dictionary WithMainEntry(Dictionary main) { var config = ProvidersOnly(); diff --git a/src/Netclaw.Cli.Tests/Tui/ChatPageTests.cs b/src/Netclaw.Cli.Tests/Tui/ChatPageTests.cs index 501941f3e..0f071ef8b 100644 --- a/src/Netclaw.Cli.Tests/Tui/ChatPageTests.cs +++ b/src/Netclaw.Cli.Tests/Tui/ChatPageTests.cs @@ -4,10 +4,14 @@ // // ----------------------------------------------------------------------- using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.AI; using Netclaw.Actors.Protocol; +using Netclaw.Actors.Tools; using Netclaw.Cli.Daemon; using Netclaw.Cli.Tui; using Netclaw.Configuration; +using Netclaw.Security; +using Netclaw.Tools; using Termina; using Termina.Hosting; using Termina.Input; @@ -325,9 +329,89 @@ public async Task LongApprovalBody_RenderedFrameSnapshot() WriteSnapshot("chat-approval-expanded.txt", expanded); } + [Fact] + public async Task LargeMcpApproval_RenderedFrameSnapshot() + { + var content = string.Join('\n', Enumerable.Repeat("large memorizer payload that must not render", 2_000)); + var function = AIFunctionFactory.Create( + (string contents, string source_path, string destination_directory, string access_token) => "uploaded", + "upload", + "Upload a file to Dropbox"); + var tool = new McpToolAdapter(function, "Dropbox", "upload"); + var config = new ToolConfig { ShellMode = ShellExecutionMode.HostAllowed }; + config.AudienceProfiles.Personal.AllowedMcpServers.Add("Dropbox"); + config.AudienceProfiles.Personal.ApprovalPolicy = new ToolApprovalConfig + { + McpServerDefaults = new Dictionary(StringComparer.Ordinal) + { + ["Dropbox"] = ToolApprovalMode.Approval + } + }; + var policy = new ToolAccessPolicy( + config, + new EffectivePolicyDefaults( + DeploymentPosture.Personal, + TrustAudience.Personal, + ShellExecutionMode.HostAllowed, + UsedStrictFallback: false)); + var executionContext = new ToolExecutionContext( + new ToolRunScope + { + Session = new ToolSessionScope.Bound("test-session", null), + Audience = TrustAudience.Personal, + InlineOutputBudget = InlineOutputBudget.Default, + InteractiveApproval = new InteractiveApprovalCapability.Unavailable() + }, + ToolExecutionTimeout.Default); + var decision = policy.AuthorizeInvocation( + tool, + executionContext, + new Dictionary + { + ["contents"] = content, + ["source_path"] = "/home/operator/reports/2026/Q3/quarterly-results-final.pdf", + ["destination_directory"] = "/Finance/Board Pack/2026/Q3", + ["access_token"] = "must-never-render", + ["_rationale"] = "Upload the requested board report" + }); + var approvalContext = Assert.IsType(decision.ApprovalContext); + var approval = BuildApproval(approvalContext.DisplayText, approvalContext.ToolName) with + { + Patterns = approvalContext.Patterns, + CandidateVerbs = approvalContext.CandidateVerbs, + Options = approvalContext.Options + .Select(option => new ToolInteractionOption(option.Key, option.Label)) + .ToArray() + }; + + var collapsed = await CaptureFrameAsync(approval, expand: false); + var expanded = await CaptureFrameAsync(approval, expand: true); + + WriteSnapshot("chat-mcp-approval-large-collapsed.txt", collapsed); + WriteSnapshot("chat-mcp-approval-large-expanded.txt", expanded); + + Assert.Contains("/Finance/Board Pack/2026/Q3", expanded); + Assert.Contains("quarterly-results-final.pdf", expanded); + Assert.Contains(content.Length.ToString(), expanded); + Assert.Contains("chars, 2000 lines", expanded); + Assert.Contains("MCP tool approval required", expanded); + Assert.Contains("Invocation:", expanded); + Assert.Contains(ApprovalOptionKeys.ApproveMcpToolLabel, expanded); + Assert.DoesNotContain("Patterns:", expanded); + Assert.DoesNotContain(ApprovalOptionKeys.ApproveEverywhereLabel, expanded); + Assert.DoesNotContain("large memorizer payload", expanded); + Assert.DoesNotContain("must-never-render", collapsed); + Assert.DoesNotContain("must-never-render", expanded); + Assert.DoesNotContain("_rationale", expanded); + Assert.DoesNotContain("Upload the requested board report", expanded); + } + private async Task CaptureFrameAsync(bool expand) + => await CaptureFrameAsync(BuildApproval(LongShellBody), expand); + + private async Task CaptureFrameAsync(ToolInteractionRequest approval, bool expand) { - var (terminal, app, _) = CreateHeadlessApp(BuildApproval(LongShellBody), out var input); + var (terminal, app, _) = CreateHeadlessApp(approval, out var input); if (expand) input.EnqueueKey(ConsoleKey.O, false, false, true); input.EnqueueKey(ConsoleKey.Q, false, false, true); @@ -363,7 +447,7 @@ private static void WriteSnapshot(string filename, string content) File.WriteAllText(Path.Combine(targetDir, filename), content); } - private static ToolInteractionRequest BuildApproval(string displayText) + private static ToolInteractionRequest BuildApproval(string displayText, string toolName = "shell_execute") { return new ToolInteractionRequest { @@ -371,7 +455,7 @@ private static ToolInteractionRequest BuildApproval(string displayText) TimestampMs = 0, Kind = "approval", CallId = new Netclaw.Tools.ToolCallId("test-call"), - ToolName = new Netclaw.Tools.ToolName("shell_execute"), + ToolName = new Netclaw.Tools.ToolName(toolName), DisplayText = displayText, Patterns = ["cd"], CandidateVerbs = ["cd"], diff --git a/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-collapsed.txt b/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-collapsed.txt new file mode 100644 index 000000000..4ab3447de --- /dev/null +++ b/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-collapsed.txt @@ -0,0 +1,40 @@ +╭─Netclaw Chat─────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│System: MCP tool approval required │ +│ Tool: Dropbox/upload │ +│Invocation: Dropbox/upload(source_path="/home/operator/reports/2026/Q3/quarterly-results-final.pdf", │ +│destination_directory="/Finance/Board Pack/2026/Q3", contents=(89999 chars, 2000 lines), access_token=***REDACTED***) │ +│ Options: Once, This chat, Always allow this tool, Deny │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─Input────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│MCP tool approval required: Dropbox/upload. │ +│Invocation: Dropbox/upload(source_path="/home/operator/reports/2026/Q3/quarterly-results-fina… [Ctrl+O to view full] │ +│Select an option and press Enter. │ +│1. Once │ +│2. This chat │ +│3. Always allow this tool │ +│4. Deny │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +[Up/Down] Select [Enter] Confirm [Ctrl+O] View full [PgUp/PgDn] Scroll [Ctrl+Q] Quit | Approval required | test-model \ No newline at end of file diff --git a/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-expanded.txt b/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-expanded.txt new file mode 100644 index 000000000..107d32af1 --- /dev/null +++ b/src/Netclaw.Cli.Tests/Tui/__snapshots__/chat-mcp-approval-large-expanded.txt @@ -0,0 +1,40 @@ +╭─Netclaw Chat─────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│System: MCP tool approval required │ +│ Tool: Dropbox/upload │ +│Invocation: Dropbox/upload(source_path="/home/operator/reports/2026/Q3/quarterly-results-final.pdf", │ +│destination_directory="/Finance/Board Pack/2026/Q3", contents=(89999 chars, 2000 lines), access_token=***REDACTED***) │ +│ Options: Once, This chat, Always allow this tool, Deny │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +╭─Input────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│MCP tool approval required: Dropbox/upload. │ +│Invocation: Dropbox/upload(source_path="/home/operator/reports/2026/Q3/quarterly-results-final.pdf", │ +│destination_directory="/Finance/Board Pack/2026/Q3", contents=(89999 chars, 2000 lines), access_token=***REDACTED***) │ +│Select an option and press Enter. │ +│1. Once │ +│2. This chat │ +│3. Always allow this tool │ +│4. Deny │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +[Up/Down] Select [Enter] Confirm [Ctrl+O] Collapse [PgUp/PgDn] Scroll [Ctrl+Q] Quit | Approval required | test-model \ No newline at end of file diff --git a/src/Netclaw.Cli/Config/ConfigFileHelper.cs b/src/Netclaw.Cli/Config/ConfigFileHelper.cs index 2fd3f3fed..5094f53ce 100644 --- a/src/Netclaw.Cli/Config/ConfigFileHelper.cs +++ b/src/Netclaw.Cli/Config/ConfigFileHelper.cs @@ -177,14 +177,7 @@ private static void PreserveLegacyModelsBackup(string path, Dictionary__* and " + - "NETCLAW_Models__Roles__* first."); - } + ModelEntryWriter.ThrowIfLegacyEnvironmentOverride(); var backupPath = path + ".legacy-models.bak"; if (!File.Exists(backupPath)) diff --git a/src/Netclaw.Cli/Config/ModelEntryWriter.cs b/src/Netclaw.Cli/Config/ModelEntryWriter.cs index 0c3f7e697..4df3ef243 100644 --- a/src/Netclaw.Cli/Config/ModelEntryWriter.cs +++ b/src/Netclaw.Cli/Config/ModelEntryWriter.cs @@ -37,10 +37,24 @@ internal static bool MigrateLegacy(Dictionary modelsSection) return false; if (!modelsSection.Keys.Any(key => key is "Main" or "Fallback" or "Compaction")) return false; + + ThrowIfLegacyEnvironmentOverride(); EnsureNamedShape(modelsSection); return true; } + internal static void ThrowIfLegacyEnvironmentOverride() + { + var legacyEnvironmentOverride = FindLegacyEnvironmentOverride(); + if (legacyEnvironmentOverride is null) + return; + + throw new ModelConfigurationException( + $"Cannot migrate Models while legacy environment override '{legacyEnvironmentOverride}' is set. " + + "Move model overrides to NETCLAW_Models__Definitions____* and " + + "NETCLAW_Models__Roles__* first."); + } + internal static bool ClearRole(Dictionary modelsSection, string roleKey) { var (_, roles) = EnsureNamedShape(modelsSection); @@ -123,12 +137,12 @@ private static (Dictionary Definitions, Dictionary Definitions, Dictionary Definitions, Dictionary(raw) - ?? throw new InvalidOperationException($"Models:{role} could not be parsed."); + ?? throw new ModelConfigurationException($"Models:{role} could not be parsed."); } catch (JsonException) { model = ReadLegacyIdentity(raw) - ?? throw new InvalidOperationException($"Models:{role} could not be repaired."); + ?? throw new ModelConfigurationException($"Models:{role} could not be repaired."); } var existingName = FindDefinition(definitions, model.Provider, model.ModelId); if (existingName is not null) @@ -167,7 +181,7 @@ private static (Dictionary Definitions, Dictionary(definitions[existingName])!; if (!Equivalent(existing, model)) { - throw new InvalidOperationException( + throw new ModelConfigurationException( $"Legacy model roles conflict for {model.Provider}/{model.ModelId}; " + $"align their metadata before migration."); } @@ -219,7 +233,7 @@ private static Dictionary GetDictionary(Dictionary @@ -368,17 +388,6 @@ internal void RemoveDaemonEnvironmentFile() File.Delete(envFilePath); } - /// - /// systemd's TimeoutStopSec= for the generated unit: comfortably longer than - /// 's own graceful-shutdown wait - /// () so systemd never SIGKILLs the whole - /// cgroup out from under ExecStop= (netclaw daemon stop) while that command is - /// still legitimately waiting on the daemon's own CoordinatedShutdown drain. Existing - /// installs only pick this up after re-running netclaw daemon install. - /// - private static readonly int TimeoutStopSecValue = - (int)(DaemonConfig.GracefulShutdownBudget + TimeSpan.FromSeconds(30)).TotalSeconds; - /// /// Builds the systemd --user unit content. The daemon's shell-tool PATH is /// supplied out-of-band via EnvironmentFile= (see @@ -388,6 +397,13 @@ internal void RemoveDaemonEnvironmentFile() /// The - prefix makes systemd tolerant of a missing env file: a deleted PATH /// file degrades tool resolution (which SystemdUnitPathDoctorCheck flags) /// rather than preventing the entire daemon from starting. + /// + /// TimeoutStopSec= is — comfortably + /// longer than this 's own graceful-wait-plus-grace budget + /// () so systemd never SIGKILLs the whole + /// cgroup out from under ExecStop= (netclaw daemon stop) while that command is + /// still legitimately waiting on the daemon's own shutdown (netclaw-dev/netclaw#1665). + /// Existing installs only pick this up after re-running netclaw daemon install. /// internal static string BuildDaemonUnitContent(string binaryPath, string cliBinaryPath, string environmentFilePath) => $""" [Unit] @@ -398,7 +414,7 @@ internal static string BuildDaemonUnitContent(string binaryPath, string cliBinar Type=simple ExecStart={binaryPath} ExecStop={cliBinaryPath} daemon stop - TimeoutStopSec={TimeoutStopSecValue} + TimeoutStopSec={(int)DaemonConfig.SystemdTimeoutStopSec.TotalSeconds} Restart=always RestartSec=5 Environment=DOTNET_ENVIRONMENT=Production diff --git a/src/Netclaw.Cli/Doctor/ContextWindowDoctorCheck.cs b/src/Netclaw.Cli/Doctor/ContextWindowDoctorCheck.cs index d68bfe68c..69ed8d8aa 100644 --- a/src/Netclaw.Cli/Doctor/ContextWindowDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/ContextWindowDoctorCheck.cs @@ -43,7 +43,19 @@ public async Task RunAsync(CancellationToken cancellationToke if (root is null) return DoctorCheckResult.Pass("Context Window", "No config file to check."); - var resolvedModels = ModelConfigurationResolver.Resolve(_configuration).Selection; + ModelSelection resolvedModels; + try + { + resolvedModels = ModelConfigurationResolver.Resolve(_configuration).Selection; + } + catch (ModelConfigurationException ex) + { + return DoctorCheckResult.Error( + "Context Window", + $"Invalid model configuration: {ex.Message}", + "Fix the Models configuration in netclaw.json, then rerun `netclaw doctor`."); + } + var main = resolvedModels.Main; var runtimeValidation = ValidateRuntimeConfiguration(root); diff --git a/src/Netclaw.Cli/Doctor/DoctorFixService.cs b/src/Netclaw.Cli/Doctor/DoctorFixService.cs index 57e5e19ce..d5cf22b5f 100644 --- a/src/Netclaw.Cli/Doctor/DoctorFixService.cs +++ b/src/Netclaw.Cli/Doctor/DoctorFixService.cs @@ -67,15 +67,6 @@ public Task BuildPlanAsync(CancellationToken cancellationToken = if (obj["Models"] is JsonObject modelsNode) { - var legacyEnvironmentOverride = ModelEntryWriter.FindLegacyEnvironmentOverride(); - if (legacyEnvironmentOverride is not null) - { - throw new InvalidOperationException( - $"Cannot migrate Models while legacy environment override '{legacyEnvironmentOverride}' is set. " + - "Move model overrides to NETCLAW_Models__Definitions____* and " + - "NETCLAW_Models__Roles__* first."); - } - var models = JsonSerializer.Deserialize>(modelsNode.ToJsonString())!; if (ModelEntryWriter.MigrateLegacy(models)) { diff --git a/src/Netclaw.Cli/Model/ModelCommand.cs b/src/Netclaw.Cli/Model/ModelCommand.cs index 44021cc61..c811789c2 100644 --- a/src/Netclaw.Cli/Model/ModelCommand.cs +++ b/src/Netclaw.Cli/Model/ModelCommand.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Globalization; using System.Text.Json; using Microsoft.Extensions.Configuration; using Netclaw.Cli.Config; @@ -25,25 +26,31 @@ public static async Task RunAsync( var writer = output ?? Console.Out; var subcommand = args.Length > 1 ? args[1] : "help"; - return subcommand switch + try { - "list" => RunList(paths, writer), - "set" => await RunSetAsync(args, paths, probe, writer), - "discover" => await RunDiscoverAsync(args, paths, probe, writer), - "clear" => RunClear(args, paths, writer), - "help" or "-h" or "--help" => WriteHelp(writer), - _ => WriteHelp(writer) - }; + return subcommand switch + { + "list" => RunList(paths, writer), + "set" => await RunSetAsync(args, paths, probe, writer), + "discover" => await RunDiscoverAsync(args, paths, probe, writer), + "clear" => RunClear(args, paths, writer), + "help" or "-h" or "--help" => WriteHelp(writer), + _ => WriteHelp(writer) + }; + } + catch (ModelConfigurationException ex) + { + writer.WriteLine($"Error: {ex.Message}"); + return 1; + } } private static int RunList(NetclawPaths paths, TextWriter writer) { - if (!TryLoadModelSelection(paths, out var models)) + if (!TryLoadModelSelection(paths, out var models, out var error)) { - // Config present but unparseable — surface it rather than showing a corrupt config as - // "no models configured", which would send the operator down the wrong recovery path. - writer.WriteLine("Error: model configuration could not be parsed."); - writer.WriteLine("Run `netclaw doctor` to diagnose, or `netclaw doctor --fix` to repair it."); + writer.WriteLine($"Error: {error}"); + writer.WriteLine("Fix the Models section in netclaw.json, then rerun `netclaw model list`."); return 1; } @@ -306,11 +313,11 @@ private static bool TryParseModalities(string value, out ModelModality modalitie var result = ModelModality.None; foreach (var token in value.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) { - // Enum.TryParse also accepts the underlying integer ("3" → Text|Image), but the contract - // advertised in the help text and error message is named flags only. Enum.IsDefined - // rejects a numeric token because its parsed value is not a single declared member, so a - // mistyped or scripted number is not silently coerced into a modality set. - if (!Enum.TryParse(token, ignoreCase: true, out ModelModality parsed) + // Enum.TryParse accepts underlying integers, including values that happen to be declared + // members ("1" → Text). The CLI contract is named flags only, so reject numeric tokens + // before parsing rather than relying on Enum.IsDefined to distinguish composites. + if (int.TryParse(token, NumberStyles.Integer, CultureInfo.InvariantCulture, out _) + || !Enum.TryParse(token, ignoreCase: true, out ModelModality parsed) || !Enum.IsDefined(parsed) || parsed == ModelModality.None) return false; @@ -498,7 +505,7 @@ private static int RunClear(string[] args, NetclawPaths paths, TextWriter writer /// . /// internal static ModelSelection? LoadModelSelection(NetclawPaths paths) - => TryLoadModelSelection(paths, out var models) ? models : null; + => TryLoadModelSelection(paths, out var models, out _) ? models : null; /// /// Attempts to load the model selection. Returns false when the config file is present but its @@ -506,9 +513,13 @@ private static int RunClear(string[] args, NetclawPaths paths, TextWriter writer /// treating it as "no models". Returns true (with null ) when no config /// file or Models section exists. /// - internal static bool TryLoadModelSelection(NetclawPaths paths, out ModelSelection? models) + internal static bool TryLoadModelSelection( + NetclawPaths paths, + out ModelSelection? models, + out string? error) { models = null; + error = null; if (!File.Exists(paths.NetclawConfigPath)) return true; @@ -523,8 +534,14 @@ internal static bool TryLoadModelSelection(NetclawPaths paths, out ModelSelectio models = ModelConfigurationResolver.Resolve(configuration).Selection; return true; } + catch (ModelConfigurationException ex) + { + error = ex.Message; + return false; + } catch (Exception ex) when (ex is JsonException or InvalidOperationException) { + error = "model configuration could not be parsed."; return false; } } diff --git a/src/Netclaw.Cli/Program.cs b/src/Netclaw.Cli/Program.cs index 13d708408..1a777cf22 100644 --- a/src/Netclaw.Cli/Program.cs +++ b/src/Netclaw.Cli/Program.cs @@ -233,40 +233,56 @@ static async Task RunAsync(string[] args) var runner = scope.ServiceProvider.GetRequiredService(); var fixService = scope.ServiceProvider.GetRequiredService(); - DoctorFixPlan? fixPlan = null; - if (doctorOptions!.Fix) + try { - fixPlan = await fixService.BuildPlanAsync(); - if (doctorOptions.Format is DoctorOutputFormat.Text) - WriteDoctorFixPlan(fixPlan, doctorOptions.DryRun); - - if (fixPlan.HasChanges && !doctorOptions.DryRun) + DoctorFixPlan? fixPlan = null; + if (doctorOptions!.Fix) { - var shouldApply = doctorOptions.Yes || PromptForDoctorFixApply(); - if (shouldApply) - await fixService.ApplyAsync(fixPlan); + fixPlan = await fixService.BuildPlanAsync(); + if (doctorOptions.Format is DoctorOutputFormat.Text) + WriteDoctorFixPlan(fixPlan, doctorOptions.DryRun); + + if (fixPlan.HasChanges && !doctorOptions.DryRun) + { + var shouldApply = doctorOptions.Yes || PromptForDoctorFixApply(); + if (shouldApply) + await fixService.ApplyAsync(fixPlan); + } } - } - var result = await runner.RunAsync(); + var result = await runner.RunAsync(); - if (doctorOptions.Format is DoctorOutputFormat.Json) - WriteDoctorJsonResult(result, fixPlan, doctorOptions); - else - WriteDoctorResult(result); + if (doctorOptions.Format is DoctorOutputFormat.Json) + WriteDoctorJsonResult(result, fixPlan, doctorOptions); + else + WriteDoctorResult(result); - // Hint about --fix when there are issues and fix wasn't requested - if (!doctorOptions.Fix - && result.ExitCode != 0 - && doctorOptions.Format is DoctorOutputFormat.Text) - { - fixPlan ??= await fixService.BuildPlanAsync(); - if (fixPlan.HasChanges) - Console.WriteLine("hint: Some issues may be auto-fixable. Run `netclaw doctor --fix --dry-run` to preview."); + // Hint about --fix when there are issues and fix wasn't requested + if (!doctorOptions.Fix + && result.ExitCode != 0 + && doctorOptions.Format is DoctorOutputFormat.Text) + { + fixPlan ??= await fixService.BuildPlanAsync(); + if (fixPlan.HasChanges) + Console.WriteLine("hint: Some issues may be auto-fixable. Run `netclaw doctor --fix --dry-run` to preview."); + } + + Environment.ExitCode = result.ExitCode; + return; } + catch (ModelConfigurationException ex) + { + var failure = new DoctorRunResult( + [DoctorCheckResult.Error("model-configuration", ex.Message)], + ExitCode: 1); + if (doctorOptions!.Format is DoctorOutputFormat.Json) + WriteDoctorJsonResult(failure, fixPlan: null, doctorOptions); + else + Console.WriteLine($"Error: {ex.Message}"); - Environment.ExitCode = result.ExitCode; - return; + Environment.ExitCode = 1; + return; + } } if (mode is "status") diff --git a/src/Netclaw.Cli/Tui/ChatPage.cs b/src/Netclaw.Cli/Tui/ChatPage.cs index e4b88902a..8f4231170 100644 --- a/src/Netclaw.Cli/Tui/ChatPage.cs +++ b/src/Netclaw.Cli/Tui/ChatPage.cs @@ -462,9 +462,17 @@ private void HandleOutput(SessionOutput output) case ToolInteractionRequest msg: RemoveThinkingSpinner(); - _chatHistory.AppendLine($"System: Approval required for {msg.ToolName}", Color.Yellow); - _chatHistory.AppendLine($" {msg.DisplayText}", Color.White); - if (msg.Patterns.Count > 0) + _chatHistory.AppendLine( + msg.ToolName.IsMcp + ? "System: MCP tool approval required" + : $"System: Approval required for {msg.ToolName}", + Color.Yellow); + if (msg.ToolName.IsMcp) + _chatHistory.AppendLine($" Tool: {msg.ToolName}", Color.BrightBlack); + _chatHistory.AppendLine( + msg.ToolName.IsMcp ? $" Invocation: {msg.DisplayText}" : $" {msg.DisplayText}", + Color.White); + if (!msg.ToolName.IsMcp && msg.Patterns.Count > 0) _chatHistory.AppendLine($" Patterns: {string.Join(", ", msg.Patterns)}", Color.BrightBlack); _chatHistory.AppendLine($" Options: {string.Join(", ", msg.Options.Select(o => o.Label))}", Color.Yellow); _chatHistory.ScrollToBottom(); diff --git a/src/Netclaw.Cli/Tui/ChatViewModel.cs b/src/Netclaw.Cli/Tui/ChatViewModel.cs index 428e58199..6f463fb44 100644 --- a/src/Netclaw.Cli/Tui/ChatViewModel.cs +++ b/src/Netclaw.Cli/Tui/ChatViewModel.cs @@ -288,7 +288,9 @@ public string GetApprovalSummary() if (CurrentInteraction is not { } interaction) return "Approval required"; - return $"Approval required for {interaction.ToolName}."; + return interaction.ToolName.IsMcp + ? $"MCP tool approval required: {interaction.ToolName}." + : $"Approval required for {interaction.ToolName}."; } /// @@ -305,11 +307,12 @@ public string GetApprovalBody(int maxLineWidth) if (CurrentInteraction is not { } interaction) return string.Empty; - var patterns = interaction.Patterns.Count > 0 + var patterns = !interaction.ToolName.IsMcp && interaction.Patterns.Count > 0 ? $" Patterns: {string.Join(", ", interaction.Patterns)}" : string.Empty; - var fullBody = $"{interaction.DisplayText}{patterns}"; + var prefix = interaction.ToolName.IsMcp ? "Invocation: " : string.Empty; + var fullBody = $"{prefix}{interaction.DisplayText}{patterns}"; if (IsApprovalDetailVisible.Value) return Netclaw.Channels.ApprovalDisplayTextFormatter.Truncate(fullBody, MaxExpandedApprovalBodyChars); diff --git a/src/Netclaw.Cli/Tui/ModelManagerViewModel.cs b/src/Netclaw.Cli/Tui/ModelManagerViewModel.cs index 4a5410b8e..efdbf3447 100644 --- a/src/Netclaw.Cli/Tui/ModelManagerViewModel.cs +++ b/src/Netclaw.Cli/Tui/ModelManagerViewModel.cs @@ -95,7 +95,7 @@ public override void OnActivated() public void Refresh() { - if (!Model.ModelCommand.TryLoadModelSelection(_paths, out var models)) + if (!Model.ModelCommand.TryLoadModelSelection(_paths, out var models, out _)) { Models = null; StatusMessage.Value = "Model configuration is invalid. Run `netclaw doctor` for details."; diff --git a/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs new file mode 100644 index 000000000..fae069346 --- /dev/null +++ b/src/Netclaw.Configuration.Tests/CrashLogWriterTests.cs @@ -0,0 +1,66 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using Xunit; + +namespace Netclaw.Configuration.Tests; + +[Collection(nameof(NetclawHomeEnvCollection))] +public sealed class CrashLogWriterTests : IDisposable +{ + private const string EnvVar = "NETCLAW_HOME"; + private readonly string? _originalValue = Environment.GetEnvironmentVariable(EnvVar); + private readonly List _tempDirectories = []; + + public void Dispose() + { + Environment.SetEnvironmentVariable(EnvVar, _originalValue); + foreach (var directory in _tempDirectories) + Directory.Delete(directory, recursive: true); + } + + [Fact] + public void TryWrite_DefaultDirectory_HonorsNetclawHome() + { + var home = NewTempDirectory(); + Environment.SetEnvironmentVariable(EnvVar, home); + using var errors = new StringWriter(); + + var crashPath = CrashLogWriter.TryWrite( + new InvalidOperationException("boom"), "CLI", errorWriter: errors); + + Assert.NotNull(crashPath); + Assert.Equal(Path.Join(home, "logs"), Path.GetDirectoryName(crashPath)); + Assert.True(File.Exists(crashPath)); + Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); + } + + [Fact] + public void TryWrite_ExplicitDirectory_TakesPrecedenceOverNetclawHome() + { + var home = NewTempDirectory(); + var explicitLogs = NewTempDirectory(); + Environment.SetEnvironmentVariable(EnvVar, home); + using var errors = new StringWriter(); + + var crashPath = CrashLogWriter.TryWrite( + new InvalidOperationException("boom"), "CLI", + errorWriter: errors, logsDirectory: explicitLogs); + + Assert.NotNull(crashPath); + Assert.Equal(explicitLogs, Path.GetDirectoryName(crashPath)); + Assert.True(File.Exists(crashPath)); + Assert.False(Directory.Exists(Path.Join(home, "logs"))); + Assert.Contains(crashPath, errors.ToString(), StringComparison.Ordinal); + } + + private string NewTempDirectory() + { + var path = Path.Join(Path.GetTempPath(), "netclaw-crash-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(path); + _tempDirectories.Add(path); + return path; + } +} diff --git a/src/Netclaw.Configuration.Tests/DaemonConfigTests.cs b/src/Netclaw.Configuration.Tests/DaemonConfigTests.cs index a31011aab..abc2f063f 100644 --- a/src/Netclaw.Configuration.Tests/DaemonConfigTests.cs +++ b/src/Netclaw.Configuration.Tests/DaemonConfigTests.cs @@ -288,25 +288,49 @@ public void Validator_rejects_invalid_trusted_proxy_entry() Assert.Contains(issues, issue => issue.Message.Contains("not-an-ip", StringComparison.OrdinalIgnoreCase)); } - /// - /// Regression guard for the canary daemon-stop finding: - /// is the single source of truth shared by - /// the daemon's CoordinatedShutdown session-drain phase, its generic-host ShutdownTimeout, - /// the CLI's SIGTERM grace period, and the generated systemd unit's TimeoutStopSec. It only - /// does its job if it comfortably exceeds how long a session can legitimately still be - /// mid-LLM-call at shutdown ('s default) — shrink - /// it below that and the CLI (or systemd) will force-kill the daemon mid-graceful-drain - /// again, exactly the bug this constant exists to prevent. - /// + // ── Shutdown-budget layering (netclaw-dev/netclaw#1664, #1665) ─────────── + // + // Four surfaces derive from DaemonConfig.GracefulShutdownBudget and must stay strictly + // ordered: the daemon-stop drain bound, the Akka before-service-unbind phase timeout, the + // CLI's force-kill budget (graceful wait + grace window), and the generated systemd unit's + // TimeoutStopSec=. If any margin is changed inconsistently in the future — e.g. the CLI's + // grace window grows past the systemd teardown margin — this test fails instead of silently + // reintroducing the guaranteed-SIGKILL race these issues were filed against. + [Fact] - public void GracefulShutdownBudget_exceeds_default_TurnLlmTimeout() + public void ShutdownBudgetLayering_bounded_drain_finishes_before_the_akka_phase_timeout() { - var defaultTurnLlmTimeout = new SessionConfig().TurnLlmTimeout; + Assert.True( + DaemonConfig.BoundedDrainTimeout < DaemonConfig.GracefulShutdownBudget, + "the daemon-stop drain's own deadline must fire before the Akka phase timeout abandons the task outright"); + } + + [Fact] + public void ShutdownBudgetLayering_cli_waits_at_least_as_long_as_the_daemon_phase_timeout() + { + Assert.True( + DaemonConfig.GracefulShutdownBudget <= DaemonConfig.CliForceKillBudget, + "the CLI's graceful-wait budget must be at least the daemon's own Akka phase timeout"); + } + [Fact] + public void ShutdownBudgetLayering_cli_force_kill_budget_stays_under_systemd_timeout_stop_sec() + { Assert.True( - DaemonConfig.GracefulShutdownBudget > defaultTurnLlmTimeout, - $"GracefulShutdownBudget ({DaemonConfig.GracefulShutdownBudget}) must exceed the default " + - $"TurnLlmTimeout ({defaultTurnLlmTimeout}) so an in-flight LLM turn can finish draining " + - "before the daemon's graceful-shutdown budget is exhausted."); + DaemonConfig.CliForceKillBudget < DaemonConfig.SystemdTimeoutStopSec, + "systemd's TimeoutStopSec= must exceed the CLI's own budget+grace, or systemd SIGKILLs " + + "the cgroup out from under a `netclaw daemon stop` (ExecStop=) that is still legitimately waiting"); + } + + [Fact] + public void ShutdownBudgetLayering_matches_the_documented_second_values() + { + // Pins the concrete values referenced in netclaw-dev/netclaw#1665's evidence trail + // (200s phase timeout, 230s TimeoutStopSec) so a change to any constant is a visible, + // deliberate diff rather than a silent drift. + Assert.Equal(TimeSpan.FromSeconds(190), DaemonConfig.BoundedDrainTimeout); + Assert.Equal(TimeSpan.FromSeconds(200), DaemonConfig.GracefulShutdownBudget); + Assert.Equal(TimeSpan.FromSeconds(215), DaemonConfig.CliForceKillBudget); + Assert.Equal(TimeSpan.FromSeconds(230), DaemonConfig.SystemdTimeoutStopSec); } } diff --git a/src/Netclaw.Configuration.Tests/NamedModelConfigurationTests.cs b/src/Netclaw.Configuration.Tests/NamedModelConfigurationTests.cs index 42dbefa0e..50e0fd2f5 100644 --- a/src/Netclaw.Configuration.Tests/NamedModelConfigurationTests.cs +++ b/src/Netclaw.Configuration.Tests/NamedModelConfigurationTests.cs @@ -59,7 +59,7 @@ public void Resolve_MixedShape_FailsLoudly() ["Models:Roles:Main"] = "vision", }); - var exception = Assert.Throws( + var exception = Assert.Throws( () => ModelConfigurationResolver.Resolve(configuration)); Assert.Contains("mixes legacy", exception.Message); @@ -75,7 +75,7 @@ public void Resolve_MissingDefinition_FailsLoudly() ["Models:Roles:Main"] = "missing", }); - var exception = Assert.Throws( + var exception = Assert.Throws( () => ModelConfigurationResolver.Resolve(configuration)); Assert.Contains("unknown definition 'missing'", exception.Message); diff --git a/src/Netclaw.Configuration/CrashLogWriter.cs b/src/Netclaw.Configuration/CrashLogWriter.cs index 44aa94cd5..7bfd60960 100644 --- a/src/Netclaw.Configuration/CrashLogWriter.cs +++ b/src/Netclaw.Configuration/CrashLogWriter.cs @@ -24,10 +24,7 @@ public static class CrashLogWriter try { - var effectiveLogsDirectory = logsDirectory - ?? Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - ".netclaw", "logs"); + var effectiveLogsDirectory = logsDirectory ?? new NetclawPaths().LogsDirectory; Directory.CreateDirectory(effectiveLogsDirectory); diff --git a/src/Netclaw.Configuration/DaemonConfig.cs b/src/Netclaw.Configuration/DaemonConfig.cs index 5625b1b6e..d2d87182a 100644 --- a/src/Netclaw.Configuration/DaemonConfig.cs +++ b/src/Netclaw.Configuration/DaemonConfig.cs @@ -21,33 +21,83 @@ public sealed record DaemonConfig public const int DefaultPort = 5199; /// - /// Worst-case time the daemon's graceful shutdown drain is allotted before something - /// gives up and forces termination. Sized to comfortably exceed - /// 's default (3 minutes) so a session mid-LLM-call - /// during shutdown can finish draining instead of being interrupted. + /// Worst-case time the daemon's graceful shutdown drain is allotted before something gives + /// up and forces termination. Sized to comfortably exceed SessionConfig.TurnLlmTimeout's + /// default (3 minutes) so a session mid-LLM-call during shutdown can finish draining instead + /// of being interrupted. /// - /// Single source of truth shared by four surfaces that must stay in lockstep: + /// Single source of truth shared by every shutdown-timing surface that must stay in + /// lockstep (netclaw-dev/netclaw#1664, #1665): /// - Netclaw.Daemon's Akka coordinated-shutdown.phases.before-service-unbind.timeout - /// HOCON override (where the actual session drain runs) + /// HOCON override (see DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon), + /// where SessionDrainHelper.DrainAsync actually drains sessions + /// - , the deadline the daemon-stop CoordinatedShutdown + /// task gives that same drain call — always strictly under this budget, so the drain + /// finishes (timed out or not) before the phase timeout above abandons it outright /// - Netclaw.Daemon's generic-host HostOptions.ShutdownTimeout - /// - 's SIGTERM grace period in - /// netclaw daemon stop (the systemd unit's ExecStop=) + /// - Netclaw.Cli.Daemon.DaemonManager's graceful-wait before force-kill in + /// netclaw daemon stop, followed by /// - the generated systemd unit's TimeoutStopSec= - /// (see DaemonManager.BuildDaemonUnitContent) + /// (; see DaemonManager.BuildDaemonUnitContent) /// - /// A production canary regression traced to these four being inconsistent: the CLI's - /// SIGTERM wait was hardcoded to 10 seconds — far short of the 200s the daemon's own - /// CoordinatedShutdown phase is deliberately allotted — so a session still mid-LLM-call - /// caused the CLI to give up and force-kill the daemon itself long before the daemon's - /// own graceful drain could finish. The daemon still died, so netclaw daemon stop - /// (ExecStop) reported success, but via SIGKILL mid-shutdown rather than a clean exit — - /// exactly what systemd's failed (Result: signal) was observing. - /// - /// Changing this value requires re-running netclaw daemon install on existing - /// hosts to regenerate the unit file with the new TimeoutStopSec=. + /// #1664: the SIGTERM/daemon-stop drain previously waited unbounded + /// (CancellationToken.None), so a session parked on interactive tool approval — which + /// cannot ack within any budget — hung the call for the full phase timeout, leaking the + /// abandoned drain task. #1665: the CLI's force-kill wait equaled this exact budget with no + /// headroom, so whenever the drain legitimately used the full budget, the CLI guaranteed a + /// SIGKILL race the daemon could not win (evidence: a production daemon force-killed ~100ms + /// from a clean exit). /// public static readonly TimeSpan GracefulShutdownBudget = TimeSpan.FromSeconds(200); + /// + /// Safety margin subtracted from to produce + /// , so the daemon-stop session drain always finishes — + /// timed out or not — strictly before the Akka before-service-unbind phase timeout + /// itself fires and abandons the drain task. + /// + public static readonly TimeSpan DrainSafetyMargin = TimeSpan.FromSeconds(10); + + /// + /// Additional time netclaw daemon stop polls for process exit after + /// elapses, before escalating to SIGKILL. Covers the + /// daemon's own post-drain teardown (actor system termination, PID file cleanup) so a + /// daemon that finishes right at the budget boundary is not killed out from under itself + /// (netclaw-dev/netclaw#1665). + /// + public static readonly TimeSpan CliForceKillGraceWindow = TimeSpan.FromSeconds(15); + + /// + /// Headroom added on top of for the systemd unit's + /// TimeoutStopSec= (), so systemd never SIGKILLs + /// the whole cgroup out from under ExecStop= (netclaw daemon stop) while that + /// command is still legitimately waiting on the daemon's own bounded shutdown. + /// + public static readonly TimeSpan SystemdTeardownMargin = TimeSpan.FromSeconds(30); + + /// + /// The deadline the daemon-stop CoordinatedShutdown task gives + /// SessionDrainHelper.DrainAsync. Always strictly less than + /// so the drain completes — as timed out if + /// necessary — before the Akka phase timeout fires. + /// + public static TimeSpan BoundedDrainTimeout => GracefulShutdownBudget - DrainSafetyMargin; + + /// + /// Total time netclaw daemon stop allows before escalating to SIGKILL: the + /// graceful wait (matching the daemon's own Akka phase timeout) plus + /// . + /// + public static TimeSpan CliForceKillBudget => GracefulShutdownBudget + CliForceKillGraceWindow; + + /// + /// The systemd unit's TimeoutStopSec= value: comfortably longer than + /// so systemd never SIGKILLs the whole cgroup out from + /// under ExecStop= (netclaw daemon stop) while that command is still + /// legitimately waiting on the daemon's own shutdown. + /// + public static TimeSpan SystemdTimeoutStopSec => GracefulShutdownBudget + SystemdTeardownMargin; + /// /// IP address the daemon binds to. Defaults to loopback (127.0.0.1). /// diff --git a/src/Netclaw.Configuration/NamedModelConfiguration.cs b/src/Netclaw.Configuration/NamedModelConfiguration.cs index 5d45997ea..fa0fa8eb9 100644 --- a/src/Netclaw.Configuration/NamedModelConfiguration.cs +++ b/src/Netclaw.Configuration/NamedModelConfiguration.cs @@ -41,7 +41,7 @@ public static ModelConfigurationResolution Resolve(IConfigurationSection modelsS var hasNamed = hasDefinitions || hasRoles; if (hasLegacy && hasNamed) - throw new InvalidOperationException( + throw new ModelConfigurationException( "Models configuration mixes legacy inline roles with named Definitions/Roles. " + "Run `netclaw doctor --fix` after removing one representation."); @@ -53,7 +53,7 @@ public static ModelConfigurationResolution Resolve(IConfigurationSection modelsS } if (!hasDefinitions || !hasRoles) - throw new InvalidOperationException( + throw new ModelConfigurationException( "Named Models configuration requires both Definitions and Roles sections."); var duplicateDefinition = modelsSection.GetSection(nameof(NamedModelConfiguration.Definitions)) @@ -62,13 +62,13 @@ public static ModelConfigurationResolution Resolve(IConfigurationSection modelsS .FirstOrDefault(group => group.Count() > 1); if (duplicateDefinition is not null) { - throw new InvalidOperationException( + throw new ModelConfigurationException( $"Models:Definitions contains duplicate case-insensitive name '{duplicateDefinition.Key}'."); } var named = modelsSection.Get() ?? new NamedModelConfiguration(); if (named.Definitions.Count == 0) - throw new InvalidOperationException("Models:Definitions must contain at least one model definition."); + throw new ModelConfigurationException("Models:Definitions must contain at least one model definition."); var selection = new ModelSelection { @@ -87,7 +87,7 @@ private static ModelReference ResolveRequired( NamedModelConfiguration named, string role, string definitionName) { if (string.IsNullOrWhiteSpace(definitionName)) - throw new InvalidOperationException($"Models:Roles:{role} must reference a model definition."); + throw new ModelConfigurationException($"Models:Roles:{role} must reference a model definition."); return ResolveDefinition(named, role, definitionName); } @@ -105,7 +105,7 @@ private static ModelReference ResolveDefinition( string.Equals(pair.Key, definitionName, StringComparison.OrdinalIgnoreCase)); if (string.IsNullOrEmpty(match.Key)) { - throw new InvalidOperationException( + throw new ModelConfigurationException( $"Models:Roles:{role} references unknown definition '{definitionName}'."); } @@ -124,3 +124,8 @@ private static ModelReference ResolveDefinition( } public sealed record ModelConfigurationResolution(ModelSelection Selection, bool IsLegacy); + +/// +/// Represents an invalid operator-authored model configuration that cannot be resolved safely. +/// +public sealed class ModelConfigurationException(string message) : InvalidOperationException(message); diff --git a/src/Netclaw.Daemon.Tests/DaemonShutdownConfigurationTests.cs b/src/Netclaw.Daemon.Tests/DaemonShutdownConfigurationTests.cs index ac6c3683c..67a9e20b4 100644 --- a/src/Netclaw.Daemon.Tests/DaemonShutdownConfigurationTests.cs +++ b/src/Netclaw.Daemon.Tests/DaemonShutdownConfigurationTests.cs @@ -3,42 +3,45 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Akka.Configuration; using Netclaw.Configuration; -using Netclaw.Daemon; using Xunit; namespace Netclaw.Daemon.Tests; /// -/// Regression coverage for the canary daemon-stop finding (see -/// remarks): the Akka CoordinatedShutdown -/// "before-service-unbind" phase timeout — where session draining actually happens — must -/// track , not a hardcoded literal that can -/// silently drift out of sync with the CLI's SIGTERM wait or the generated systemd unit's -/// TimeoutStopSec. +/// Tests for , which +/// replaced the inline HOCON literal Program.cs used to prepend a hardcoded 200s +/// before-service-unbind phase timeout (netclaw-dev/netclaw#1664, #1665). Proves the emitted +/// HOCON tracks whatever resolves to instead +/// of drifting back to a literal that could disagree with the daemon-stop drain's own bound. /// public sealed class DaemonShutdownConfigurationTests { - [Fact] - public void BuildCoordinatedShutdownHocon_UsesGracefulShutdownBudget() + [Theory] + [InlineData(200, 200)] + [InlineData(90, 90)] + public void BuildCoordinatedShutdownHocon_interpolates_the_given_budget_in_seconds( + int budgetSeconds, int expectedPhaseTimeoutSeconds) { - var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(DaemonConfig.GracefulShutdownBudget); + var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(TimeSpan.FromSeconds(budgetSeconds)); + + var config = ConfigurationFactory.ParseString(hocon); - Assert.Contains( - $"phases.before-service-unbind.timeout = {(int)DaemonConfig.GracefulShutdownBudget.TotalSeconds}s", - hocon, - StringComparison.Ordinal); - Assert.Contains("exit-clr = off", hocon, StringComparison.Ordinal); + Assert.Equal(TimeSpan.FromSeconds(expectedPhaseTimeoutSeconds), + config.GetTimeSpan("akka.coordinated-shutdown.phases.before-service-unbind.timeout")); + Assert.False(config.GetBoolean("akka.coordinated-shutdown.exit-clr")); } - [Theory] - [InlineData(30)] - [InlineData(200)] - [InlineData(600)] - public void BuildCoordinatedShutdownHocon_InterpolatesArbitraryTimeouts(int seconds) + [Fact] + public void BuildCoordinatedShutdownHocon_tracks_DaemonConfig_GracefulShutdownBudget() { - var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(TimeSpan.FromSeconds(seconds)); + var hocon = DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(DaemonConfig.GracefulShutdownBudget); + + var config = ConfigurationFactory.ParseString(hocon); - Assert.Contains($"phases.before-service-unbind.timeout = {seconds}s", hocon, StringComparison.Ordinal); + Assert.Equal( + DaemonConfig.GracefulShutdownBudget, + config.GetTimeSpan("akka.coordinated-shutdown.phases.before-service-unbind.timeout")); } } diff --git a/src/Netclaw.Daemon.Tests/Services/DaemonRestartCoordinatorTests.cs b/src/Netclaw.Daemon.Tests/Services/DaemonRestartCoordinatorTests.cs index e2171c0b0..0e548af4f 100644 --- a/src/Netclaw.Daemon.Tests/Services/DaemonRestartCoordinatorTests.cs +++ b/src/Netclaw.Daemon.Tests/Services/DaemonRestartCoordinatorTests.cs @@ -194,6 +194,42 @@ public async Task SessionDrainHelper_propagates_caller_cancellation() await Assert.ThrowsAnyAsync(() => operation); } + [Fact] + public async Task SessionDrainHelper_daemon_stop_bound_times_out_instead_of_hanging_when_a_session_never_acks() + { + // Mirrors the daemon-stop CoordinatedShutdown drain task wired in Program.cs + // (netclaw-dev/netclaw#1664): a session whose in-flight turn is parked on interactive + // tool approval never acks PrepareForDaemonRestart. Previously this call passed + // CancellationToken.None for the operation token and hung until Akka's own 200s + // before-service-unbind phase timeout abandoned the task. The bounded CTS below — + // sized from DaemonConfig.BoundedDrainTimeout (GracefulShutdownBudget minus + // DrainSafetyMargin) and driven by TimeProvider exactly as Program.cs constructs it — + // must make the drain complete with a timed-out result well before that. + var time = new FakeTimeProvider(); + var activeIds = new[] { "slack/approval-parked" }; + var drain = new DrainControl(activeIds, activeIds); // never acknowledged + var sessionManager = _system.ActorOf(Props.Create(() => new StubSessionManagerActor( + activeIds, + drain, + throwOnEnumeration: false))); + using var deadlineCts = new CancellationTokenSource(DaemonConfig.BoundedDrainTimeout, time); + + var operation = SessionDrainHelper.DrainAsync( + sessionManager, + "daemon-stop", + NullLogger.Instance, + deadlineCts.Token, + CancellationToken.None); + await drain.AllRequestsObserved; + time.Advance(DaemonConfig.BoundedDrainTimeout); + + var result = await operation; + + Assert.Single(result.AllSessionIds); + Assert.Empty(result.DrainedSessionIds); + Assert.Equal("slack/approval-parked", Assert.Single(result.TimedOutSessionIds).Value); + } + public async ValueTask DisposeAsync() { await _system.Terminate(); diff --git a/src/Netclaw.Daemon/DaemonShutdownConfiguration.cs b/src/Netclaw.Daemon/DaemonShutdownConfiguration.cs index eafcc465e..55d897e02 100644 --- a/src/Netclaw.Daemon/DaemonShutdownConfiguration.cs +++ b/src/Netclaw.Daemon/DaemonShutdownConfiguration.cs @@ -12,8 +12,8 @@ namespace Netclaw.Daemon; /// phase. Extracted into its own testable method (rather than an inline string literal in /// Program.cs's top-level statements) so a unit test can assert the interpolated timeout /// tracks instead of drifting back to a -/// hardcoded literal — the exact class of bug behind the canary daemon-stop finding (see -/// remarks for the full story). +/// hardcoded literal (see remarks for why +/// that drift is the exact class of bug behind netclaw-dev/netclaw#1664 and #1665). /// internal static class DaemonShutdownConfiguration { diff --git a/src/Netclaw.Daemon/Program.cs b/src/Netclaw.Daemon/Program.cs index 1419ec9c3..9f4f90bfc 100644 --- a/src/Netclaw.Daemon/Program.cs +++ b/src/Netclaw.Daemon/Program.cs @@ -435,13 +435,15 @@ static void ConfigureDaemonServices( services.Configure(options => { - // Must comfortably exceed DaemonConfig.GracefulShutdownBudget below (the Akka - // `before-service-unbind` CoordinatedShutdown phase timeout, where session draining - // actually happens) plus headroom for the handful of default-timeout phases that run - // after it. Previously a bare 30s — shorter than the 200s session-drain budget, which - // let this host-level timeout diverge from the Akka phase it is supposed to bound - // (canary finding: see DaemonConfig.GracefulShutdownBudget remarks for the full story). - options.ShutdownTimeout = DaemonConfig.GracefulShutdownBudget + TimeSpan.FromSeconds(30); + // Generic-host shutdown ceiling for all hosted services combined. Kept in lockstep + // with DaemonConfig.SystemdTimeoutStopSec (= GracefulShutdownBudget + teardown margin) + // rather than a bare, unrelated literal: a value shorter than the Akka + // before-service-unbind phase timeout below would silently reintroduce the same class + // of mismatch behind netclaw-dev/netclaw#1665 (budgets that look independent but must + // stay ordered). AkkaHostedService.StopAsync does not observe this cancellation token + // and awaits CoordinatedShutdown.Run to its own natural completion, so this value does + // not itself truncate the drain — it only keeps the documented layering consistent. + options.ShutdownTimeout = DaemonConfig.SystemdTimeoutStopSec; }); // Resolve models for session config @@ -1058,9 +1060,11 @@ static void ConfigureDaemonServices( { // Prevent coordinated shutdown from calling Environment.Exit(), // which would kill the process before the restart loop can iterate. - // The before-service-unbind phase needs a generous timeout because sessions - // mid-LLM-call (TurnLlmTimeout defaults to 3 minutes) must finish before - // passivation can begin. + // The before-service-unbind phase needs a generous timeout (DaemonConfig. + // GracefulShutdownBudget) because sessions mid-LLM-call (TurnLlmTimeout defaults to + // 3 minutes) must finish before passivation can begin. See DaemonConfig. + // GracefulShutdownBudget remarks for the full set of surfaces this must stay in + // lockstep with. akkaBuilder.AddHocon( DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon(DaemonConfig.GracefulShutdownBudget), HoconAddMode.Prepend); @@ -1118,8 +1122,8 @@ static void ConfigureDaemonServices( // Runs in an early CoordinatedShutdown phase while actors are still alive. // If DaemonRestartCoordinator already drained sessions (config reload), the ingress // gate will be closed and this task skips its drain to avoid double-draining. - // The phase timeout (200s) is generous because sessions mid-LLM-call must finish - // before passivation can begin. + // The phase timeout (DaemonConfig.GracefulShutdownBudget) is generous because + // sessions mid-LLM-call must finish before passivation can begin. var cs = CoordinatedShutdown.Get(system); var sessionManager = registry.Get(); var ingressGate = sp.GetRequiredService(); @@ -1136,11 +1140,20 @@ static void ConfigureDaemonServices( try { + // Bounded strictly under DaemonConfig.GracefulShutdownBudget (the Akka phase + // timeout above) so the drain always completes -- timed out or not -- before + // the phase timeout itself fires and abandons this task outright. + // netclaw-dev/netclaw#1664: a session parked on interactive tool approval + // never acks PrepareForDaemonRestart, so an unbounded wait here (previously + // CancellationToken.None, CancellationToken.None) hung for the full 200s + // phase timeout with no timeout of its own, leaking the abandoned drain task. + using var drainDeadlineCts = new CancellationTokenSource(DaemonConfig.BoundedDrainTimeout, tp); + var drainResult = await SessionDrainHelper.DrainAsync( sessionManager, "daemon-stop", drainLogger, - CancellationToken.None, + drainDeadlineCts.Token, CancellationToken.None); lifecycleNotifier.NotifyShutdown("daemon-stop", drainResult.ToNotificationContext()); diff --git a/src/Netclaw.Daemon/Services/SessionDrainHelper.cs b/src/Netclaw.Daemon/Services/SessionDrainHelper.cs index 14481c893..b9dbdc6b0 100644 --- a/src/Netclaw.Daemon/Services/SessionDrainHelper.cs +++ b/src/Netclaw.Daemon/Services/SessionDrainHelper.cs @@ -100,9 +100,10 @@ public static async Task DrainAsync( else { logger.LogWarning( - "Drain completed with {DrainedCount} session(s) drained and {TimedOutCount} timed out; timed-out sessions will recover from the last durable checkpoint.", + "Drain completed with {DrainedCount} session(s) drained and {TimedOutCount} timed out ({TimedOutSessionIds}); timed-out sessions will recover from the last durable checkpoint.", drained.Length, - timedOut.Length); + timedOut.Length, + string.Join(", ", timedOut.Select(static id => id.Value))); } return new DrainResult(sessionIds, drained, timedOut); diff --git a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs index 3015d1bc7..3f68c194c 100644 --- a/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs +++ b/src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Text.Json; using Netclaw.Configuration; using Netclaw.Tools; using Xunit; @@ -980,3 +981,199 @@ public void IsApproved_no_match() cwd: null)); } } + +public sealed class McpApprovalMatcherTests +{ + private readonly McpApprovalMatcher _matcher = McpApprovalMatcher.Instance; + + [Fact] + public void FormatForDisplay_without_arguments_returns_tool_name() + { + Assert.Equal( + "Dropbox/upload", + _matcher.FormatForDisplay(new ToolName("Dropbox/upload"), null)); + } + + [Fact] + public void FormatForDisplay_prioritizes_location_values_and_summarizes_large_strings() + { + var content = string.Join('\n', Enumerable.Repeat("memorizer payload line", 2_000)); + var display = _matcher.FormatForDisplay( + new ToolName("Dropbox/upload"), + new Dictionary + { + ["contents"] = content, + ["overwrite"] = true, + ["source_path"] = "/home/operator/reports/quarterly-results.pdf", + ["destination_directory"] = "/Finance/Board/2026/Q3" + }); + + Assert.Contains("source_path=\"/home/operator/reports/quarterly-results.pdf\"", display); + Assert.Contains("destination_directory=\"/Finance/Board/2026/Q3\"", display); + Assert.Contains($"contents=({content.Length} chars, 2000 lines)", display); + Assert.DoesNotContain("memorizer payload line", display); + Assert.True( + display.IndexOf("destination_directory", StringComparison.Ordinal) + < display.IndexOf("contents", StringComparison.Ordinal)); + Assert.True( + display.IndexOf("source_path", StringComparison.Ordinal) + < display.IndexOf("contents", StringComparison.Ordinal)); + } + + [Fact] + public void FormatForDisplay_handles_JsonElement_scalars_and_small_structures() + { + using var json = JsonDocument.Parse( + """{"mode":"replace","options":{"notify":true,"retries":3},"tags":["finance","board"]}"""); + var root = json.RootElement; + + var display = _matcher.FormatForDisplay( + new ToolName("Dropbox/upload"), + new Dictionary + { + ["mode"] = root.GetProperty("mode").Clone(), + ["options"] = root.GetProperty("options").Clone(), + ["tags"] = root.GetProperty("tags").Clone() + }); + + Assert.Contains("mode=\"replace\"", display); + Assert.Contains("options={\"notify\":true,\"retries\":3}", display); + Assert.Contains("tags=[\"finance\",\"board\"]", display); + Assert.DoesNotContain('\n', display); + } + + [Fact] + public void FormatForDisplay_redacts_top_level_nested_and_token_shaped_secrets() + { + using var json = JsonDocument.Parse( + """{"password":"nested-password","safe":"visible"}"""); + const string providerToken = "sk-1234567890abcdef"; + + var display = _matcher.FormatForDisplay( + new ToolName("service/create"), + new Dictionary + { + ["access_token"] = "plain-access-token", + ["options"] = json.RootElement.Clone(), + ["reference"] = providerToken + }); + + Assert.DoesNotContain("plain-access-token", display); + Assert.DoesNotContain("nested-password", display); + Assert.DoesNotContain(providerToken, display); + Assert.Contains("***REDACTED***", display); + Assert.Contains("visible", display); + } + + [Fact] + public void FormatForDisplay_preserves_both_ends_of_long_locator() + { + var path = "/source/" + new string('a', 1_100) + "/quarterly-results.pdf"; + + var display = _matcher.FormatForDisplay( + new ToolName("Dropbox/upload"), + new Dictionary { ["source_path"] = path }); + + Assert.Contains("/source/", display); + Assert.Contains("quarterly-results.pdf", display); + Assert.Contains($"[{path.Length} chars]", display); + } + + [Fact] + public void FormatForDisplay_redacts_uri_credentials_query_and_fragment() + { + const string signedUrl = "https://operator:password@example.com/reports/q3.pdf?signature=opaque-secret&expires=123#access-token"; + + var display = _matcher.FormatForDisplay( + new ToolName("Dropbox/upload"), + new Dictionary { ["callback"] = signedUrl }); + + Assert.Contains("https://example.com/reports/q3.pdf", display); + Assert.DoesNotContain("operator", display); + Assert.DoesNotContain("password", display); + Assert.DoesNotContain("opaque-secret", display); + Assert.DoesNotContain("access-token", display); + Assert.Contains("REDACTED", display); + } + + [Fact] + public void FormatForDisplay_escapes_untrusted_argument_names_and_inline_code_breakouts() + { + var display = _matcher.FormatForDisplay( + new ToolName("service/invoke"), + new Dictionary + { + ["safe\n```\u202E**Approve**"] = "value`spoof" + }); + + Assert.DoesNotContain('\n', display); + Assert.DoesNotContain('`', display); + Assert.DoesNotContain('\u202E', display); + Assert.Contains("\\u000A", display); + Assert.Contains("\\u0060", display); + Assert.Contains("\\u202E", display); + } + + [Fact] + public void FormatForDisplay_does_not_infer_value_sensitivity_from_argument_names() + { + var longValue = new string('x', 2_000); + var display = _matcher.FormatForDisplay( + new ToolName("service/invoke"), + new Dictionary + { + ["file_contents"] = longValue, + ["source_code"] = longValue, + ["target_payload"] = longValue + }); + + Assert.Equal(3, display.Split("2000 chars", StringSplitOptions.None).Length - 1); + Assert.DoesNotContain(new string('x', 50), display); + } + + [Fact] + public void FormatForDisplay_bounds_argument_and_collection_work() + { + using var json = JsonDocument.Parse( + $"[{string.Join(',', Enumerable.Range(0, 1_000))}]"); + var arguments = Enumerable.Range(0, 1_000) + .ToDictionary(i => $"argument_{i:D4}", i => (object?)json.RootElement.Clone()); + + var display = _matcher.FormatForDisplay(new ToolName("service/invoke"), arguments); + + Assert.True(display.Length <= 1_600); + Assert.Contains("12+ items", display); + Assert.Contains("arguments", display); + Assert.DoesNotContain("argument_0024", display); + } + + [Fact] + public void FormatForDisplay_bounds_oversized_names_and_redacts_their_values() + { + var oversizedKey = new string('k', 10_000); + var display = _matcher.FormatForDisplay( + new ToolName($"service/{new string('t', 10_000)}`\nspoof"), + new Dictionary { [oversizedKey] = "must-not-appear" }); + + Assert.True(display.Length <= 1_600); + Assert.DoesNotContain("must-not-appear", display); + Assert.DoesNotContain('`', display); + Assert.DoesNotContain('\n', display); + Assert.Contains("10015 chars", display); + Assert.Contains("10000\\u0020chars", display); + Assert.Contains("REDACTED", display); + } + + [Fact] + public void FormatForDisplay_reports_unserializable_value_without_throwing() + { + var value = new Func(() => 42); + + var display = _matcher.FormatForDisplay( + new ToolName("service/invoke"), + new Dictionary { ["callback"] = value }); + + Assert.Contains("value unavailable", display); + Assert.DoesNotContain("42", display); + } +} diff --git a/src/Netclaw.Security/IToolApprovalMatcher.cs b/src/Netclaw.Security/IToolApprovalMatcher.cs index 3fd9a2c33..e23261422 100644 --- a/src/Netclaw.Security/IToolApprovalMatcher.cs +++ b/src/Netclaw.Security/IToolApprovalMatcher.cs @@ -3,7 +3,9 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using System.Collections; using System.Text; +using System.Text.Json; using Netclaw.Configuration; using Netclaw.Tools; @@ -772,3 +774,365 @@ public bool IsMessy(ToolName toolName, IDictionary? arguments) public string FormatForDisplay(ToolName toolName, IDictionary? arguments) => toolName.Value; } + +/// +/// MCP approval matcher. Grant matching remains at the canonical tool-name +/// level, while the display text includes a bounded, redacted preview of the +/// server arguments so an operator can make an informed decision. +/// +public sealed class McpApprovalMatcher : IToolApprovalMatcher +{ + public static readonly McpApprovalMatcher Instance = new(); + + private const int MaxToolNameChars = 256; + private const int MaxArgumentNameChars = 160; + private const int MaxArguments = 24; + private const int MaxDisplayChars = 1_600; + private const int MaxNestedItems = 12; + private const int MaxNestedDepth = 3; + private const int MaxStructurePreviewChars = 360; + private const int MaxUrlParseChars = 4_096; + private const int MaxLineCountChars = 100_000; + private const int StandardStringPreviewChars = 240; + private const int LocatorStringPreviewChars = 1_000; + private const int LocatorPreviewHeadChars = 600; + private const int LocatorPreviewTailChars = 350; + private const string Redacted = "***REDACTED***"; + + private static DefaultApprovalMatcher Default => DefaultApprovalMatcher.Instance; + + public string GetApprovalModeKey(ToolName toolName, IDictionary? arguments) + => Default.GetApprovalModeKey(toolName, arguments); + + public bool IsFailClosedOnPersonal(ToolName toolName, IDictionary? arguments) + => Default.IsFailClosedOnPersonal(toolName, arguments); + + public IReadOnlyList ExtractPatterns(ToolName toolName, IDictionary? arguments) + => Default.ExtractPatterns(toolName, arguments); + + public IReadOnlyList ExtractCandidateVerbs(ToolName toolName, IDictionary? arguments) + => Default.ExtractCandidateVerbs(toolName, arguments); + + public IReadOnlyList ExtractCandidates( + ToolName toolName, + IDictionary? arguments) + => Default.ExtractCandidates(toolName, arguments); + + public bool IsApproved( + ToolName toolName, + IDictionary? arguments, + IReadOnlyList approvedEntries, + string? cwd) + => Default.IsApproved(toolName, arguments, approvedEntries, cwd); + + public bool IsMessy(ToolName toolName, IDictionary? arguments) + => Default.IsMessy(toolName, arguments); + + public string FormatForDisplay(ToolName toolName, IDictionary? arguments) + { + var displayToolName = FormatToolName(toolName.Value); + if (arguments is null || arguments.Count == 0) + return displayToolName; + + // MCP schemas may contain arbitrary property names and arbitrarily + // large collections. Sample before formatting so an approval prompt + // has a hard work and allocation ceiling even for a hostile server. + var sampled = arguments + .Take(MaxArguments) + .OrderBy(static pair => IsLocationLike(pair.Value) ? 0 : 1) + .ToList(); + + var display = new StringBuilder(Math.Min(MaxDisplayChars, displayToolName.Length + 256)); + display.Append(displayToolName).Append('('); + var rendered = 0; + + foreach (var pair in sampled) + { + var part = $"{FormatArgumentName(pair.Key)}={FormatArgument(pair.Key, pair.Value)}"; + var separatorChars = rendered == 0 ? 0 : 2; + if (display.Length + separatorChars + part.Length + 1 > MaxDisplayChars) + break; + + if (separatorChars > 0) + display.Append(", "); + + display.Append(part); + rendered++; + } + + var omitted = arguments.Count - rendered; + if (omitted > 0) + { + var omission = $"… (+{omitted} arguments)"; + var separator = rendered == 0 ? string.Empty : ", "; + var available = MaxDisplayChars - display.Length - 1; + if (separator.Length + omission.Length <= available) + display.Append(separator).Append(omission); + else if (available > 1) + display.Append('…'); + } + + display.Append(')'); + + return display.ToString(); + } + + private static string FormatArgument(string key, object? value) + { + if (SecretOutputRedactor.IsSecretKey(key) || value is SensitiveString) + return Redacted; + + if (value is byte[] bytes) + return $"({bytes.Length} bytes)"; + + if (value is string text) + return FormatString(text); + + if (value is IDictionary dictionary) + return $"({dictionary.Count} properties)"; + + if (value is ICollection collection) + return $"({collection.Count} items)"; + + if (value is IEnumerable) + return "(collection value)"; + + JsonElement element; + try + { + element = value is JsonElement json + ? json + : JsonSerializer.SerializeToElement(value, value?.GetType() ?? typeof(object)); + } + catch (Exception ex) when (ex is JsonException or NotSupportedException) + { + return $"({value?.GetType().Name ?? "unknown"} value unavailable)"; + } + + return FormatJsonValue(key, element, depth: 0); + } + + private static string FormatJsonValue(string key, JsonElement value, int depth) + { + if (SecretOutputRedactor.IsSecretKey(key)) + return Redacted; + + return value.ValueKind switch + { + JsonValueKind.String => FormatString(value.GetString() ?? string.Empty), + JsonValueKind.Number => value.GetRawText(), + JsonValueKind.True => "true", + JsonValueKind.False => "false", + JsonValueKind.Null => "null", + JsonValueKind.Undefined => "(undefined)", + JsonValueKind.Object => FormatObject(value, depth), + JsonValueKind.Array => FormatArray(value, depth), + _ => "(unsupported value)" + }; + } + + private static string FormatString(string value) + { + if (TrySanitizeAbsoluteUri(value, out var sanitizedUri)) + return SerializeStringForDisplay(sanitizedUri); + + if (IsPathLike(value)) + return FormatPath(value); + + return FormatNonLocatorString(value); + } + + private static string FormatPath(string value) + { + if (value.Length <= LocatorStringPreviewChars) + return SerializeStringForDisplay(SecretOutputRedactor.Redact(value)); + + var preview = value[..LocatorPreviewHeadChars] + + $"…[{value.Length} chars]…" + + value[^LocatorPreviewTailChars..]; + return SerializeStringForDisplay(SecretOutputRedactor.Redact(preview)); + } + + private static string FormatNonLocatorString(string value) + { + if (value.Length <= StandardStringPreviewChars) + return SerializeStringForDisplay(SecretOutputRedactor.Redact(value)); + + return $"({value.Length} chars, {CountLinesForDisplay(value)} lines)"; + } + + private static string FormatObject(JsonElement value, int depth) + { + if (depth >= MaxNestedDepth) + return "(nested object)"; + + var properties = value.EnumerateObject().Take(MaxNestedItems + 1).ToList(); + if (properties.Count > MaxNestedItems) + return $"({MaxNestedItems}+ properties)"; + + var parts = properties + .OrderBy(static property => property.Name, StringComparer.Ordinal) + .Select(property => + $"{SerializeStringForDisplay(BoundArgumentName(property.Name))}:{FormatJsonValue(property.Name, property.Value, depth + 1)}"); + return BoundStructurePreview($"{{{string.Join(",", parts)}}}", properties.Count, "properties"); + } + + private static string FormatArray(JsonElement value, int depth) + { + if (depth >= MaxNestedDepth) + return "(nested array)"; + + var items = value.EnumerateArray().Take(MaxNestedItems + 1).ToList(); + if (items.Count > MaxNestedItems) + return $"({MaxNestedItems}+ items)"; + + var preview = $"[{string.Join(",", items.Select(item => FormatJsonValue(string.Empty, item, depth + 1)))}]"; + return BoundStructurePreview(preview, items.Count, "items"); + } + + private static string BoundStructurePreview(string preview, int count, string unit) + => preview.Length <= MaxStructurePreviewChars ? preview : $"({count} {unit})"; + + private static string FormatArgumentName(string key) + { + if (key.Length is > 0 and <= MaxArgumentNameChars && key.All(IsSafeArgumentNameChar)) + return key; + + // Approval displays are embedded in inline-code markup by the chat + // renderers. Encode every non-identifier code unit, including + // backticks, line breaks, ANSI controls, and bidi overrides, so an MCP + // schema cannot escape the invocation line or spoof prompt chrome. + var bounded = BoundArgumentName(key); + var escaped = new StringBuilder(bounded.Length + 2); + escaped.Append('"'); + foreach (var ch in bounded) + { + if (IsSafeArgumentNameChar(ch)) + escaped.Append(ch); + else + escaped.Append("\\u").Append(((int)ch).ToString("X4")); + } + + return escaped.Append('"').ToString(); + } + + private static string BoundArgumentName(string key) + => key.Length <= MaxArgumentNameChars + ? key + : $"{key[..120]}…[{key.Length} chars]"; + + private static string FormatToolName(string toolName) + { + var bounded = toolName.Length <= MaxToolNameChars + ? toolName + : $"{toolName[..200]}…[{toolName.Length} chars]"; + return EscapePresentationControls(bounded); + } + + private static bool IsSafeArgumentNameChar(char ch) + => ch is >= 'a' and <= 'z' + or >= 'A' and <= 'Z' + or >= '0' and <= '9' + or '_' or '-' or '.'; + + private static string SerializeStringForDisplay(string value) + { + var serialized = JsonSerializer.Serialize(value); + return EscapePresentationControls(serialized); + } + + private static string EscapePresentationControls(string value) + { + if (!value.Any(IsPresentationControl)) + return value; + + var escaped = new StringBuilder(value.Length); + foreach (var ch in value) + { + if (IsPresentationControl(ch)) + escaped.Append("\\u").Append(((int)ch).ToString("X4")); + else + escaped.Append(ch); + } + + return escaped.ToString(); + } + + private static bool IsPresentationControl(char ch) + => ch == '`' || char.IsControl(ch) || IsDirectionalOrLineControl(ch); + + private static bool IsDirectionalOrLineControl(char ch) + => ch is '\u061C' or '\u200E' or '\u200F' + or >= '\u2028' and <= '\u202E' + or >= '\u2066' and <= '\u2069' + or '\uFEFF'; + + private static bool TrySanitizeAbsoluteUri(string value, out string sanitized) + { + sanitized = string.Empty; + if (value.Length > MaxUrlParseChars + || !Uri.TryCreate(value, UriKind.Absolute, out var uri) + || string.IsNullOrEmpty(uri.Host)) + { + return false; + } + + try + { + var builder = new UriBuilder(uri) + { + UserName = string.Empty, + Password = string.Empty, + Query = string.IsNullOrEmpty(uri.Query) ? string.Empty : Redacted, + Fragment = string.IsNullOrEmpty(uri.Fragment) ? string.Empty : Redacted + }; + sanitized = SecretOutputRedactor.Redact(builder.Uri.AbsoluteUri); + return true; + } + catch (UriFormatException) + { + return false; + } + } + + private static bool IsPathLike(string value) + { + if (value.Length == 0 || value.IndexOfAny(['\r', '\n']) >= 0) + return false; + + return Path.IsPathRooted(value) + || value.StartsWith("~/", StringComparison.Ordinal) + || value.StartsWith("./", StringComparison.Ordinal) + || value.StartsWith("../", StringComparison.Ordinal) + || value.StartsWith("\\\\", StringComparison.Ordinal) + || value.Length >= 3 && char.IsAsciiLetter(value[0]) && value[1] == ':' + && value[2] is '\\' or '/'; + } + + private static bool IsLocationLike(object? value) + { + var text = value switch + { + string stringValue => stringValue, + JsonElement { ValueKind: JsonValueKind.String } jsonValue => jsonValue.GetString(), + _ => null + }; + + return text is not null + && (IsPathLike(text) || TrySanitizeAbsoluteUri(text, out _)); + } + + private static string CountLinesForDisplay(string value) + { + var lines = 1; + var charsToInspect = Math.Min(value.Length, MaxLineCountChars); + for (var i = 0; i < charsToInspect; i++) + { + if (value[i] == '\n') + lines++; + } + + return value.Length <= MaxLineCountChars ? lines.ToString() : $"{lines}+"; + } + +} diff --git a/src/Netclaw.Security/SecretOutputRedactor.cs b/src/Netclaw.Security/SecretOutputRedactor.cs index 75c37c89b..79b9a6e1d 100644 --- a/src/Netclaw.Security/SecretOutputRedactor.cs +++ b/src/Netclaw.Security/SecretOutputRedactor.cs @@ -13,8 +13,35 @@ namespace Netclaw.Security; /// public static partial class SecretOutputRedactor { + private const int MaxSecretKeyChars = 512; private const string Redacted = "***REDACTED***"; + private static readonly string[] SecretKeyFragments = + [ + "apikey", + "token", + "secret", + "password", + "authorization", + "credential", + "privatekey", + "signingkey", + "connectionstring" + ]; + + internal static bool IsSecretKey(string key) + { + // A hostile MCP schema can supply arbitrarily large property names. + // Fail closed instead of allocating an equally large normalized key + // just to decide whether its value is safe to display. + if (key.Length > MaxSecretKeyChars) + return true; + + var normalized = Netclaw.Tools.ToolArgumentHelper.NormalizeKey(key); + return SecretKeyFragments.Any(fragment => + normalized.Contains(fragment, StringComparison.OrdinalIgnoreCase)); + } + public static bool ContainsSecretLikeContent(string output) { if (string.IsNullOrEmpty(output)) diff --git a/src/Netclaw.Tools.Abstractions/ToolName.cs b/src/Netclaw.Tools.Abstractions/ToolName.cs index c12080888..c44b16432 100644 --- a/src/Netclaw.Tools.Abstractions/ToolName.cs +++ b/src/Netclaw.Tools.Abstractions/ToolName.cs @@ -12,6 +12,13 @@ namespace Netclaw.Tools; /// public readonly record struct ToolName(string Value) { + /// + /// MCP tools use the canonical operator-facing {server}/{tool} + /// identity. First-party tool names never contain the separator. + /// + public bool IsMcp => !string.IsNullOrEmpty(Value) + && Value.IndexOf('/', StringComparison.Ordinal) > 0; + public static explicit operator ToolName(string value) => new(value); public override string ToString() => Value; diff --git a/tests/smoke/scenarios/doctor.sh b/tests/smoke/scenarios/doctor.sh index 3a1761bee..d7a9dcf0a 100755 --- a/tests/smoke/scenarios/doctor.sh +++ b/tests/smoke/scenarios/doctor.sh @@ -28,5 +28,136 @@ case "$doctor_status" in ;; esac +log "Testing 'doctor --fix' legacy migration guard..." +config_path="${NETCLAW_HOME}/config/netclaw.json" +cat >"$config_path" <<'JSON' +{ + "configVersion": 1, + "Models": { + "Main": { + "Provider": "local", + "ModelId": "qwen3:30b" + } + } +} +JSON +expected_config="${NETCLAW_HOME}/doctor-legacy.expected.json" +cp "$config_path" "$expected_config" +crash_count_before="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +fix_status=0 +fix_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --fix --yes 2>&1 +)" || fix_status=$? +echo "$fix_output" +crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +if [[ "$fix_status" -eq 1 \ + && "$fix_output" == Error:*"Cannot migrate Models"* \ + && "$fix_output" != *"Unhandled exception"* \ + && "$fix_output" != *"Fatal error"* \ + && "$crash_count_after" == "$crash_count_before" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "doctor --fix: migration guard exits 1 without crash artefacts or config changes" +else + fail "doctor --fix: migration guard was not a clean validation failure" +fi + +json_status=0 +json_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --fix --yes --format json 2>&1 +)" || json_status=$? +echo "$json_output" +if [[ "$json_status" -eq 1 \ + && "$json_output" == *'"exitCode": 1'* \ + && "$json_output" == *'"name": "model-configuration"'* \ + && "$json_output" == *"Cannot migrate Models"* \ + && "$json_output" != *"Unhandled exception"* \ + && "$json_output" != *"Fatal error"* ]]; then + pass "doctor --fix --format json: migration guard preserves the JSON error envelope" +else + fail "doctor --fix --format json: migration guard did not return structured validation output" +fi + +log "Testing unresolved named model references fail cleanly..." +cat >"$config_path" <<'JSON' +{ + "configVersion": 1, + "Providers": { + "local": { + "Type": "ollama", + "Endpoint": "http://localhost:11434" + } + }, + "Models": { + "Definitions": { + "known": { + "Provider": "local", + "ModelId": "qwen3:30b" + } + }, + "Roles": { + "Main": "missing" + } + } +} +JSON +expected_config="${NETCLAW_HOME}/doctor-unresolved.expected.json" +cp "$config_path" "$expected_config" +crash_count_before="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" + +model_status=0 +model_output="$(run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" model list 2>&1)" || model_status=$? +echo "$model_output" +if [[ "$model_status" -eq 1 + && "$model_output" == *"Models:Roles:Main references unknown definition 'missing'."* + && "$model_output" != *"could not be parsed"* + && "$model_output" != *"doctor --fix"* + && "$model_output" != *"Unhandled exception"* + && "$model_output" != *"Fatal error"* ]]; then + pass "model list: unresolved role reports the exact reference without autofix guidance" +else + fail "model list: unresolved role was not reported cleanly" +fi + +doctor_status=0 +doctor_output="$(run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor 2>&1)" || doctor_status=$? +echo "$doctor_output" +if [[ "$doctor_status" -eq 1 + && "$doctor_output" == *"Models:Roles:Main references unknown definition 'missing'."* + && "$doctor_output" != *"Unhandled exception"* + && "$doctor_output" != *"Fatal error"* ]]; then + pass "doctor: unresolved role exits 1 without crashing" +else + fail "doctor: unresolved role did not return a clean validation failure" +fi + +json_status=0 +json_output="$(run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --format json 2>&1)" || json_status=$? +echo "$json_output" +if [[ "$json_status" -eq 1 + && "$json_output" == *'"exitCode": 1'* + && "$json_output" == *"Models:Roles:Main references unknown definition"* + && "$json_output" == *"missing"* + && "$json_output" != *"Unhandled exception"* + && "$json_output" != *"Fatal error"* ]]; then + pass "doctor --format json: unresolved role preserves the JSON error envelope" +else + fail "doctor --format json: unresolved role was not structured cleanly" +fi + +fix_status=0 +fix_output="$(run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" doctor --fix --yes 2>&1)" || fix_status=$? +echo "$fix_output" +crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +if [[ "$fix_status" -eq 1 + && "$fix_output" == *"Models:Roles:Main references unknown definition 'missing'."* + && "$fix_output" != *"Unhandled exception"* + && "$fix_output" != *"Fatal error"* + && "$crash_count_after" == "$crash_count_before" ]] && cmp -s "$config_path" "$expected_config"; then + pass "doctor --fix: unresolved role exits 1 without guessing, crashing, or changing config" +else + fail "doctor --fix: unresolved role handling was unsafe or crashed" +fi summarize exit $? diff --git a/tests/smoke/scenarios/provider-model-cli.sh b/tests/smoke/scenarios/provider-model-cli.sh index 2c43196d1..04a061d32 100755 --- a/tests/smoke/scenarios/provider-model-cli.sh +++ b/tests/smoke/scenarios/provider-model-cli.sh @@ -93,5 +93,90 @@ else pass "model clear: $ALT_MODEL cleared from fallback" fi +log "Testing legacy migration failures are clean model errors..." +config_path="${NETCLAW_HOME}/config/netclaw.json" +cat >"$config_path" </dev/null | wc -l | tr -d ' ')" +migration_status=0 +migration_output="$( + NETCLAW_Models__Main__ContextWindow=65536 run_timed \ + "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" model set main local-ollama "$ALT_MODEL" 2>&1 +)" || migration_status=$? +echo "$migration_output" +crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +new_crash_log="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' -newer "$expected_config" -print -quit 2>/dev/null)" +if [[ "$migration_status" -eq 1 \ + && "$migration_output" == Error:*"Cannot migrate Models"* \ + && "$migration_output" != *"Unhandled exception"* \ + && "$migration_output" != *"Fatal error"* \ + && -z "$new_crash_log" \ + && "$crash_count_after" == "$crash_count_before" \ + && ! -e "${config_path}.legacy-models.bak" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "model set: legacy environment guard exits 1 without crash artefacts or config changes" +else + fail "model set: legacy environment guard was not a clean validation failure" +fi + +cat >"$config_path" </dev/null | wc -l | tr -d ' ')" +conflict_status=0 +conflict_output="$( + run_timed "$STEP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" \ + model set compaction local-ollama "$SMOKE_MODEL" 2>&1 +)" || conflict_status=$? +echo "$conflict_output" +conflict_crash_count_after="$(find "${NETCLAW_HOME}/logs" -maxdepth 1 -name 'crash-*.log' 2>/dev/null | wc -l | tr -d ' ')" +if [[ "$conflict_status" -eq 1 \ + && "$conflict_output" == Error:*"Legacy model roles conflict"* \ + && "$conflict_output" != *"Unhandled exception"* \ + && "$conflict_output" != *"Fatal error"* \ + && "$conflict_crash_count_after" == "$conflict_crash_count_before" ]] \ + && cmp -s "$config_path" "$expected_config"; then + pass "model set: conflicting legacy roles exit 1 without changing config" +else + fail "model set: conflicting legacy roles were not a clean validation failure" +fi summarize exit $?