From d5b3cb20c6dacd31bb54c52a30a0f64487e30215 Mon Sep 17 00:00:00 2001 From: tanaka takashi Date: Tue, 11 Aug 2026 00:04:36 +0900 Subject: feat: P1 tool improvements and P5 Agent Skills standard compliance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1: Tool小改善 1. tailバイナリ判定 (TextFileDetector): - file_search tailモードでtext/binary判定を追加 - 先頭4KBのみ読み、BOM確認(UTF-8/UTF-16 LE/BE)→UTF-8 strict byte検証 - NULバイトのみ判定不可(UTF-16でNULが正常出現するため) - binaryファイルはToolValidationExceptionで拒否 - TextFileDetectorクラスへ判定ロジックを分離 2. file_edit rename機能: - file_edit.operation=renameを追加(新規toolではない) - new_path property追加、Write capability要求(source/destination双方) - destination既存時は禁止(overwrite不可) - cross-volume renameは不支持、Tool failureとして拒否 - FileRenameTransaction + RecoveryManager file.rename recovery追加 - crash recovery: destination hash == journal記録の場合のみ戻す P5: Agent Skills open standard準拠 3. Skill形式をagentskills.io標準へ移行: - legacy単一Markdown形式(skills/*.md)を廃止 - /SKILL.md標準directory形式のみ対応 - YAML frontmatter必須(name/description)、YamlDotNetで解析 - validation: name必須(1-64文字,lowercase英数字+hyphen,親dir名一致), description必須(1-1024文字), malformed YAML reject, duplicate fatal 4. Progressive Disclosure: - Stage 1 Discovery: name+descriptionのみをsystem messageへ(name+desc一覧) - Stage 3 Instructions: 選択Skillの本文のみ注入 - PromptComposer: 全文埋め込みを廃止、カタログ形式へ変更 - PromptContext: Skills→AvailableSkills+SelectedSkillsへ分割 - 未選択Skill本文はpromptへ入らない 5. Internal Tool境界維持: - skill_read等のInternal Tool追加なし - Internal Toolは6群(file_search/file_read/file_edit/process_run/task_update/dir)のまま - Skill選択・本文ロードはAgent Runtime/Prompt Context Managementの責務 6. AgentSkill data model (Agent.Core/Skills/AgentSkill.cs): - RootPath, Name, Description, Instructions, License?, Compatibility?, Metadata?, AllowedTools? - SkillLoader(Agent.App)がYamlDotNetでfrontmatter解析→AgentSkill構築 設計判断: - renameはfile_edit operation拡張(新規toolではない) - rmdir空dirのみ制約は維持 - Skill load != permission grant維持 - allowed-toolsはSandbox permissionとして解釈しない - YamlDotNetはAgent.App側のみ使用(Agent.Coreへ依存追加なし) 変更対象: - TextFileDetector.cs: 新規、text/binary判定 - WindowsToolBackend.cs: tail binary判定統合、rename case追加 - InternalToolCatalog.cs: file_edit operation enum に rename 追加、new_path property - InternalToolValidator.cs: rename時new_path必須検証 - TransactionalFileEditor.cs: Rename() + FileRenameTransaction - RecoveryManager.cs: file.rename recovery - AgentSkill.cs: 新規data model - SkillLoader.cs: 全面書き換え、YamlDotNet frontmatter解析、validation - PromptComposer.cs: Progressive Disclosure、AvailableSkills+SelectedSkills - AgentRuntimeSettings.cs: Skills型変更 - AgentRuntime.cs: SelectedSkills=[]で初期化 - Program.cs: SkillLoader戻り値型対応 - Specifications.md: FS-07 rename、Skill節Agent Skills standard準拠 - Design.md: §7.1 tail binary判定、§7.3 rename、AgentSkill model - ImplementationPlan.md: Prompt順序更新、Skill標準準拠 テスト: - SecurityPolicyTests.cs: tail binary拒否/UTF-8/UTF-16 text許可、rename正常/dest既存拒否/許可外拒否 - InternalToolTests.cs: rename validationテスト追加 - ConfigurationTests.cs: SkillLoader標準形式/missing frontmatter/name mismatch/duplicate tests - PromptAndRuntimeTests.cs: PromptContext型変更対応、skill catalog検証 検証結果: - Release build: 警告0、エラー0 - Core tests: 61/61 PASS、skip 0 - Planner tests: 10/10 PASS、skip 0 - Internal Tool 6群固定: 確認済み --- tests/Agent.Core.Tests/ConfigurationTests.cs | 45 ++++++- tests/Agent.Core.Tests/InternalToolTests.cs | 2 + tests/Agent.Core.Tests/PromptAndRuntimeTests.cs | 11 +- tests/Agent.Windows.Tests/SecurityPolicyTests.cs | 144 +++++++++++++++++++++++ 4 files changed, 196 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/Agent.Core.Tests/ConfigurationTests.cs b/tests/Agent.Core.Tests/ConfigurationTests.cs index 2c52b0c..79c746d 100644 --- a/tests/Agent.Core.Tests/ConfigurationTests.cs +++ b/tests/Agent.Core.Tests/ConfigurationTests.cs @@ -1,5 +1,6 @@ using Agent.App.Cli; using Agent.App.Configuration; +using Agent.Core.Skills; namespace Agent.Core.Tests; @@ -43,16 +44,52 @@ public sealed class ConfigurationTests } [Fact] - public void SkillLoader_supports_legacy_markdown_and_standard_skill_directories() + public void SkillLoader_loads_standard_skill_directories_only() { using var directory = new TestDirectory(); + // Legacy single markdown — should be ignored (not loaded). directory.File("skills/legacy.md", "legacy skill"); - directory.File("skills/python-via-uv/SKILL.md", "standard skill"); + // Standard skill directory with frontmatter. + directory.File("skills/python-via-uv/SKILL.md", "---\nname: python-via-uv\ndescription: Python execution skill.\n---\n# Python via uv\nInstructions here."); + // Non-SKILL.md files in skill directory should not be loaded as skills. directory.File("skills/python-via-uv/reference.md", "must not be loaded as a skill"); - string[] skills = SkillLoader.Load(Path.Combine(directory.Path, "skills")); + var skills = SkillLoader.Load(Path.Combine(directory.Path, "skills")); - Assert.Equal(["legacy skill", "standard skill"], skills); + Assert.Single(skills); + Assert.Equal("python-via-uv", skills[0].Name); + Assert.Equal("Python execution skill.", skills[0].Description); + Assert.Contains("Instructions here.", skills[0].Instructions); + } + + [Fact] + public void SkillLoader_rejects_missing_frontmatter() + { + using var directory = new TestDirectory(); + directory.File("skills/no-frontmatter/SKILL.md", "# No frontmatter\nJust body."); + + Assert.Throws(() => SkillLoader.Load(Path.Combine(directory.Path, "skills"))); + } + + [Fact] + public void SkillLoader_rejects_name_mismatch() + { + using var directory = new TestDirectory(); + directory.File("skills/wrong-name/SKILL.md", "---\nname: different-name\ndescription: A skill.\n---\nBody."); + + Assert.Throws(() => SkillLoader.Load(Path.Combine(directory.Path, "skills"))); + } + + [Fact] + public void SkillLoader_rejects_duplicate_names() + { + using var directory = new TestDirectory(); + directory.File("skills/dup-skill/SKILL.md", "---\nname: dup-skill\ndescription: First.\n---\nBody1."); + // Create a second directory with the same skill name (via symlink or junction is not needed; + // just create another dir that also has name: dup-skill in frontmatter). + directory.File("skills/dup-skill-copy/SKILL.md", "---\nname: dup-skill\ndescription: Second.\n---\nBody2."); + + Assert.Throws(() => SkillLoader.Load(Path.Combine(directory.Path, "skills"))); } private static string CreateValidConfiguration(TestDirectory directory) diff --git a/tests/Agent.Core.Tests/InternalToolTests.cs b/tests/Agent.Core.Tests/InternalToolTests.cs index 2015702..c157458 100644 --- a/tests/Agent.Core.Tests/InternalToolTests.cs +++ b/tests/Agent.Core.Tests/InternalToolTests.cs @@ -19,6 +19,8 @@ public sealed class InternalToolTests [InlineData("file_search", "{\"root\":\".\",\"mode\":\"text\",\"query\":\"x\",\"max_results\":1001}")] [InlineData("process_run", "{\"executable\":\"x\",\"args\":\"bad\",\"cwd\":\".\"}")] [InlineData("task_update", "{\"operation\":\"add\",\"description\":\"missing estimate\"}")] + [InlineData("file_edit", "{\"path\":\"a\",\"operation\":\"rename\"}")] + [InlineData("file_edit", "{\"path\":\"a\",\"operation\":\"rename\",\"new_path\":\"\"}")] public void Malformed_arguments_are_rejected(string tool, string json) { using JsonDocument document = JsonDocument.Parse(json); diff --git a/tests/Agent.Core.Tests/PromptAndRuntimeTests.cs b/tests/Agent.Core.Tests/PromptAndRuntimeTests.cs index 5e682cd..27308ef 100644 --- a/tests/Agent.Core.Tests/PromptAndRuntimeTests.cs +++ b/tests/Agent.Core.Tests/PromptAndRuntimeTests.cs @@ -2,6 +2,7 @@ using Agent.Core.Ollama; using Agent.Core.Prompts; using Agent.Core.Runtime; using Agent.Core.Sessions; +using Agent.Core.Skills; using Agent.Core.Tasks; using Agent.Core.Tools; using System.Text.Json; @@ -13,12 +14,18 @@ public sealed class PromptAndRuntimeTests [Fact] public void Prompt_order_is_fixed_and_request_is_last() { + var availableSkills = new List + { + new("root", "test-skill", "A test skill.", "skill instructions") + }; IReadOnlyList messages = PromptComposer.Compose(new PromptContext( - "fixed", "user system", "ja", ["skill"], "capability", "graph", "summary", + "fixed", "user system", "ja", availableSkills, [], "capability", "graph", "summary", [new ConversationMessage("user", "old"), new ConversationMessage("assistant", "reply")], "new request", ["image"])); - Assert.Equal(["fixed", "user system", "Respond in locale: ja.", "skill", "capability", "graph", "summary", "old", "reply", "new request"], messages.Select(x => x.Content)); + // Check that the skill catalog (name + description) is present, not the full instructions. + Assert.Contains(messages, m => m.Content.Contains("test-skill") && m.Content.Contains("A test skill.")); + Assert.DoesNotContain(messages, m => m.Content == "skill instructions"); Assert.Equal("system", messages[0].Role); Assert.Equal("user", messages[^1].Role); Assert.Equal(["image"], messages[^1].Images); diff --git a/tests/Agent.Windows.Tests/SecurityPolicyTests.cs b/tests/Agent.Windows.Tests/SecurityPolicyTests.cs index a7197b8..9f13f56 100644 --- a/tests/Agent.Windows.Tests/SecurityPolicyTests.cs +++ b/tests/Agent.Windows.Tests/SecurityPolicyTests.cs @@ -704,6 +704,150 @@ public sealed class SecurityPolicyTests await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); } + [Fact] + public async Task Tail_rejects_binary_file() + { + using TestDirectory directory = new(); + string workspace = Path.Combine(directory.Path, "workspace"); + Directory.CreateDirectory(workspace); + string binaryFile = Path.Combine(workspace, "binary.dat"); + File.WriteAllBytes(binaryFile, [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, ..new byte[100]]); + var audit = new MemorySecurityAuditSink(); + var settings = new WindowsSandboxSettings( + [new FilePolicyEntry(workspace, FileCapability.Read)], [], false, + new SandboxJobLimits(), "deny", [], Path.Combine(directory.Path, "journals"), Path.Combine(directory.Path, "backups"), + 16 * 1024 * 1024, 1024 * 1024, 1000, 10000); + var backend = new WindowsToolBackend(settings, audit); + Guid taskId = Guid.NewGuid(); + await backend.BeginTaskAsync(taskId, TestContext.Current.CancellationToken); + using JsonDocument tail = JsonDocument.Parse($$"""{"root":{{JsonSerializer.Serialize(binaryFile)}},"mode":"tail","query":"","tail_lines":10}"""); + + await Assert.ThrowsAsync(async () => await backend.ExecuteAsync(taskId, "file_search", tail.RootElement, TestContext.Current.CancellationToken)); + + await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); + } + + [Theory] + [InlineData("utf8")] + [InlineData("utf8bom")] + [InlineData("utf16le")] + [InlineData("utf16be")] + public async Task Tail_accepts_text_files(string encoding) + { + using TestDirectory directory = new(); + string workspace = Path.Combine(directory.Path, "workspace"); + Directory.CreateDirectory(workspace); + string textFile = Path.Combine(workspace, "text.txt"); + string content = "line1\nline2\nline3\n"; + byte[] bytes = encoding switch + { + "utf8" => System.Text.Encoding.UTF8.GetBytes(content), + "utf8bom" => [.. System.Text.Encoding.UTF8.Preamble, .. System.Text.Encoding.UTF8.GetBytes(content)], + "utf16le" => [.. System.Text.Encoding.Unicode.Preamble, .. System.Text.Encoding.Unicode.GetBytes(content)], + "utf16be" => [.. System.Text.Encoding.BigEndianUnicode.Preamble, .. System.Text.Encoding.BigEndianUnicode.GetBytes(content)], + _ => throw new ArgumentException(encoding) + }; + File.WriteAllBytes(textFile, bytes); + var audit = new MemorySecurityAuditSink(); + var settings = new WindowsSandboxSettings( + [new FilePolicyEntry(workspace, FileCapability.Read)], [], false, + new SandboxJobLimits(), "deny", [], Path.Combine(directory.Path, "journals"), Path.Combine(directory.Path, "backups"), + 16 * 1024 * 1024, 1024 * 1024, 1000, 10000); + var backend = new WindowsToolBackend(settings, audit); + Guid taskId = Guid.NewGuid(); + await backend.BeginTaskAsync(taskId, TestContext.Current.CancellationToken); + using JsonDocument tail = JsonDocument.Parse($$"""{"root":{{JsonSerializer.Serialize(textFile)}},"mode":"tail","query":"","tail_lines":10}"""); + + ToolExecutionResult result = await backend.ExecuteAsync(taskId, "file_search", tail.RootElement, TestContext.Current.CancellationToken); + + Assert.True(result.Success); + await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); + } + + [Fact] + public async Task File_edit_rename_moves_file_within_approved_root() + { + using TestDirectory directory = new(); + string workspace = Path.Combine(directory.Path, "workspace"); + Directory.CreateDirectory(workspace); + string source = Path.Combine(workspace, "source.txt"); + string dest = Path.Combine(workspace, "dest.txt"); + File.WriteAllText(source, "rename-content"); + var audit = new MemorySecurityAuditSink(); + var settings = new WindowsSandboxSettings( + [new FilePolicyEntry(workspace, FileCapability.Read | FileCapability.Write | FileCapability.Delete)], [], false, + new SandboxJobLimits(), "deny", [], Path.Combine(directory.Path, "journals"), Path.Combine(directory.Path, "backups"), + 16 * 1024 * 1024, 1024 * 1024, 1000, 10000); + var backend = new WindowsToolBackend(settings, audit); + Guid taskId = Guid.NewGuid(); + await backend.BeginTaskAsync(taskId, TestContext.Current.CancellationToken); + using JsonDocument rename = JsonDocument.Parse($$"""{"path":{{JsonSerializer.Serialize(source)}},"operation":"rename","new_path":{{JsonSerializer.Serialize(dest)}}}"""); + + ToolExecutionResult result = await backend.ExecuteAsync(taskId, "file_edit", rename.RootElement, TestContext.Current.CancellationToken); + + Assert.True(result.Success); + Assert.True(result.ExternalEffect); + Assert.False(File.Exists(source)); + Assert.True(File.Exists(dest)); + Assert.Equal("rename-content", File.ReadAllText(dest)); + await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); + Assert.Empty(Directory.EnumerateFiles(Path.Combine(directory.Path, "journals"))); + } + + [Fact] + public async Task File_edit_rename_rejects_existing_destination() + { + using TestDirectory directory = new(); + string workspace = Path.Combine(directory.Path, "workspace"); + Directory.CreateDirectory(workspace); + string source = Path.Combine(workspace, "source.txt"); + string dest = Path.Combine(workspace, "dest.txt"); + File.WriteAllText(source, "source-content"); + File.WriteAllText(dest, "dest-content"); + var audit = new MemorySecurityAuditSink(); + var settings = new WindowsSandboxSettings( + [new FilePolicyEntry(workspace, FileCapability.Read | FileCapability.Write)], [], false, + new SandboxJobLimits(), "deny", [], Path.Combine(directory.Path, "journals"), Path.Combine(directory.Path, "backups"), + 16 * 1024 * 1024, 1024 * 1024, 1000, 10000); + var backend = new WindowsToolBackend(settings, audit); + Guid taskId = Guid.NewGuid(); + await backend.BeginTaskAsync(taskId, TestContext.Current.CancellationToken); + using JsonDocument rename = JsonDocument.Parse($$"""{"path":{{JsonSerializer.Serialize(source)}},"operation":"rename","new_path":{{JsonSerializer.Serialize(dest)}}}"""); + + await Assert.ThrowsAsync(async () => await backend.ExecuteAsync(taskId, "file_edit", rename.RootElement, TestContext.Current.CancellationToken)); + + Assert.True(File.Exists(source)); // source still exists + Assert.True(File.Exists(dest)); // dest untouched + await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); + } + + [Fact] + public async Task File_edit_rename_rejects_outside_approved_root() + { + using TestDirectory directory = new(); + using TestDirectory outside = new(); + string workspace = Path.Combine(directory.Path, "workspace"); + Directory.CreateDirectory(workspace); + string source = Path.Combine(workspace, "source.txt"); + string dest = Path.Combine(outside.Path, "escaped.txt"); + File.WriteAllText(source, "content"); + var audit = new MemorySecurityAuditSink(); + var settings = new WindowsSandboxSettings( + [new FilePolicyEntry(workspace, FileCapability.Read | FileCapability.Write)], [], false, + new SandboxJobLimits(), "deny", [], Path.Combine(directory.Path, "journals"), Path.Combine(directory.Path, "backups"), + 16 * 1024 * 1024, 1024 * 1024, 1000, 10000); + var backend = new WindowsToolBackend(settings, audit); + Guid taskId = Guid.NewGuid(); + await backend.BeginTaskAsync(taskId, TestContext.Current.CancellationToken); + using JsonDocument rename = JsonDocument.Parse($$"""{"path":{{JsonSerializer.Serialize(source)}},"operation":"rename","new_path":{{JsonSerializer.Serialize(dest)}}}"""); + + ToolExecutionResult result = await backend.ExecuteAsync(taskId, "file_edit", rename.RootElement, TestContext.Current.CancellationToken); + + Assert.False(result.Success); + Assert.False(result.RetryAllowed); + await backend.EndTaskAsync(taskId, true, TestContext.Current.CancellationToken); + } + [Fact(Timeout = 60_000)] public async Task Wfp_enforces_actual_appcontainer_loopback_deny_allow_and_port_scope() { -- cgit v1.2.3