From bf173347414f7591d053c38ed511e5214565eb06 Mon Sep 17 00:00:00 2001 From: Jie Shen <185767017+Sj295@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:19:04 +0800 Subject: [PATCH] fix: address all Copilot review issues across PR #1, #2, #3 --- .../java/com/ccj/app/AppConfiguration.java | 15 +++++++++++--- .../com/ccj/app/invoker/AgentInvoker.java | 1 - .../com/ccj/core/tool/ToolUseContext.java | 5 +++++ .../main/java/com/ccj/cron/CronScheduler.java | 18 +++++++++-------- .../src/main/java/com/ccj/cron/CronTools.java | 4 +++- .../ccj/hooks/watcher/FileChangedWatcher.java | 8 ++++---- .../com/ccj/models/ForkedAgentRunner.java | 4 +++- .../com/ccj/repl/dialog/PermissionDialog.java | 20 ++++++++++++++++--- .../tools/adapter/ToolCallbackAdapter.java | 17 +++++++++------- 9 files changed, 64 insertions(+), 28 deletions(-) diff --git a/ccc-app/src/main/java/com/ccj/app/AppConfiguration.java b/ccc-app/src/main/java/com/ccj/app/AppConfiguration.java index 3baec24..19476db 100644 --- a/ccc-app/src/main/java/com/ccj/app/AppConfiguration.java +++ b/ccc-app/src/main/java/com/ccj/app/AppConfiguration.java @@ -100,15 +100,24 @@ public ChatClient chatClient(AppSettings settings, Credentials credentials, if (result instanceof com.ccj.core.tool.PermissionResult.Deny d) { return d.message(); } - // Allow 和 Ask 都放行（Ask 在 REPL 层处理，此处简化为允许） - return null; + // 修复 Copilot 审查：Ask 不应被当作 allow 放行，否则绕过用户确认流程 + if (result instanceof com.ccj.core.tool.PermissionResult.Ask a) { + return "Permission required: " + a.message(); + } + return null; // Allow } catch (Exception e) { - log.warn("Permission check failed for {}: {}", tool.name(), e.getMessage()); + log.warn("Permission check failed for {}: {}", tool.name(), e.getMessage(), e); return null; // 出错时允许（fail-open） } }; + // 修复 Copilot 审查：REPL 模式下隐藏原始工具，只暴露 ReplTool 给模型 + boolean replMode = com.ccj.tools.builtin.ReplPrimitiveTools.isReplModeEnabled(); + java.util.Set hiddenTools = replMode + ? com.ccj.tools.builtin.ReplPrimitiveTools.REPL_ONLY_TOOLS + : java.util.Set.of(); java.util.List toolCallbacks = toolRegistry.all().stream() .filter(Tool::isEnabled) + .filter(t -> !hiddenTools.contains(t.name())) .map(t -> (org.springframework.ai.tool.ToolCallback) new com.ccj.tools.adapter.ToolCallbackAdapter(t, ctxTemplate, permChecker)) .toList(); log.info("Registered {} tool callbacks on ChatClient", toolCallbacks.size()); diff --git a/ccc-app/src/main/java/com/ccj/app/invoker/AgentInvoker.java b/ccc-app/src/main/java/com/ccj/app/invoker/AgentInvoker.java index e1dacdb..f9126f6 100644 --- a/ccc-app/src/main/java/com/ccj/app/invoker/AgentInvoker.java +++ b/ccc-app/src/main/java/com/ccj/app/invoker/AgentInvoker.java @@ -9,7 +9,6 @@ import org.springframework.ai.chat.prompt.Prompt; import java.util.Optional; -import java.util.function.Consumer; /** * Agent 调用器。替换原 ClaudeCodeJavaApplication 中的 Function<String,String> 裸 lambda。 diff --git a/ccc-core/src/main/java/com/ccj/core/tool/ToolUseContext.java b/ccc-core/src/main/java/com/ccj/core/tool/ToolUseContext.java index fd67b36..27009dc 100644 --- a/ccc-core/src/main/java/com/ccj/core/tool/ToolUseContext.java +++ b/ccc-core/src/main/java/com/ccj/core/tool/ToolUseContext.java @@ -36,6 +36,11 @@ public record ToolUseContext( session = session == null ? Session.inMemory() : session; } + /** 默认上下文（所有字段使用默认值）。 */ + public static ToolUseContext defaultContext() { + return new ToolUseContext(null, null, null, null, null, null); + } + /** 中止能力抽象（对应 abortController）。 */ @FunctionalInterface public interface Cancellable { diff --git a/ccc-cron/src/main/java/com/ccj/cron/CronScheduler.java b/ccc-cron/src/main/java/com/ccj/cron/CronScheduler.java index a27cced..3840513 100644 --- a/ccc-cron/src/main/java/com/ccj/cron/CronScheduler.java +++ b/ccc-cron/src/main/java/com/ccj/cron/CronScheduler.java @@ -113,16 +113,18 @@ private void check() { log.info("Cron task {} fired: {}", task.id(), task.prompt().substring(0, Math.min(50, task.prompt().length()))); // Phase 2.2: 按 agentId 路由 - if (onFireTask != null) { - onFireTask.accept(task); - } else { - onFire.accept(task.prompt()); - } - // 同时通知 lead 队列（teammate 任务也通知 lead，便于感知） + // 修复 Copilot 审查：原实现对非 teammate 任务调用了两次 onFire if (task.isTeammateTask()) { - // teammate 任务：onFireTask 负责路由到 teammate 队列 - // 此处不再调 onFire（避免重复入队） + // teammate 任务：通过 onFireTask 路由到 teammate 队列 + if (onFireTask != null) { + onFireTask.accept(task); + } + // 不再调 onFire（避免 lead 队列重复入队） } else { + // lead 任务：通知 lead 队列 + if (onFireTask != null) { + onFireTask.accept(task); // onFireTask 可做额外处理（如日志） + } onFire.accept(task.prompt()); } diff --git a/ccc-cron/src/main/java/com/ccj/cron/CronTools.java b/ccc-cron/src/main/java/com/ccj/cron/CronTools.java index 1cd35de..a904c46 100644 --- a/ccc-cron/src/main/java/com/ccj/cron/CronTools.java +++ b/ccc-cron/src/main/java/com/ccj/cron/CronTools.java @@ -73,13 +73,15 @@ public CronCreateTool(CronTaskStore store, CronScheduler scheduler) { @Override public Map inputSchema() { + // 修复 Copilot 审查：agentId 在 call() 中被读取但未在 inputSchema 声明 return Map.of( "type", "object", "properties", Map.of( "cron", Map.of("type", "string", "description", "5-field cron expression, e.g. '*/5 * * * *'"), "prompt", Map.of("type", "string", "description", "Prompt to run when triggered"), "recurring", Map.of("type", "boolean", "description", "true=recurring, false=one-shot", "default", true), - "durable", Map.of("type", "boolean", "description", "true=persist across restarts", "default", false) + "durable", Map.of("type", "boolean", "description", "true=persist across restarts", "default", false), + "agentId", Map.of("type", "string", "description", "Teammate agent ID to route the task to (optional, null for lead tasks)") ), "required", List.of("cron", "prompt") ); diff --git a/ccc-hooks/src/main/java/com/ccj/hooks/watcher/FileChangedWatcher.java b/ccc-hooks/src/main/java/com/ccj/hooks/watcher/FileChangedWatcher.java index e6ff700..ea0f5d0 100644 --- a/ccc-hooks/src/main/java/com/ccj/hooks/watcher/FileChangedWatcher.java +++ b/ccc-hooks/src/main/java/com/ccj/hooks/watcher/FileChangedWatcher.java @@ -132,11 +132,11 @@ private void watchLoop() { /** 处理文件事件。 */ private void handleFileEvent(Path filePath, String eventKind) { - // 检查是否匹配任一监听路径 + // 修复 Copilot 审查：原实现仅按文件名匹配，忽略目录，可能导致跨目录误触发。 + // 现在按完整路径匹配（watchedPaths 存储的是完整解析路径）。 + Path normalized = filePath.toAbsolutePath().normalize(); boolean matched = watchedPaths.stream() - .anyMatch(wp -> wp.getFileName() != null - && wp.getFileName().toString().equals( - filePath.getFileName() != null ? filePath.getFileName().toString() : "")); + .anyMatch(wp -> wp.toAbsolutePath().normalize().equals(normalized)); if (!matched) return; String eventName = switch (eventKind) { diff --git a/ccc-models/src/main/java/com/ccj/models/ForkedAgentRunner.java b/ccc-models/src/main/java/com/ccj/models/ForkedAgentRunner.java index 8ba8fd1..3368f39 100644 --- a/ccc-models/src/main/java/com/ccj/models/ForkedAgentRunner.java +++ b/ccc-models/src/main/java/com/ccj/models/ForkedAgentRunner.java @@ -76,7 +76,9 @@ public CompletableFuture runAsync(String taskPrompt, List paren * @return ForkedAgentResult（text + usage） */ public ForkedAgentResult runForked(String taskPrompt, List parentHistory) { - log.debug("ForkedAgent running task ({} parent messages)", parentHistory.size()); + // 修复 Copilot 审查：原实现在 null 检查前访问 parentHistory.size()，导致 NPE + int historySize = parentHistory != null ? parentHistory.size() : 0; + log.debug("ForkedAgent running task ({} parent messages)", historySize); try { var spec = parentClient.prompt(); if (systemPrompt != null && !systemPrompt.isBlank()) { diff --git a/ccc-repl/src/main/java/com/ccj/repl/dialog/PermissionDialog.java b/ccc-repl/src/main/java/com/ccj/repl/dialog/PermissionDialog.java index 74a4a97..dc6bd53 100644 --- a/ccc-repl/src/main/java/com/ccj/repl/dialog/PermissionDialog.java +++ b/ccc-repl/src/main/java/com/ccj/repl/dialog/PermissionDialog.java @@ -32,6 +32,7 @@ public class PermissionDialog implements StreamingToolExecutor.PermissionCallbac private static final Logger log = LoggerFactory.getLogger(PermissionDialog.class); private final Terminal terminal; + // 修复 Copilot 审查：并发工具调用可能同时请求权限，用 synchronized 串行化权限提示 private final BlockingQueue responseQueue = new LinkedBlockingQueue<>(1); public PermissionDialog(Terminal terminal) { @@ -42,10 +43,15 @@ public PermissionDialog(Terminal terminal) { * 渲染权限请求并等待用户响应。 * 由 agent 线程调用（通过 StreamingToolExecutor.PermissionCallback.resolve）。 * 会阻塞直到用户响应。 + * + * 修复 Copilot 审查：synchronized 防止并发工具调用同时渲染提示和竞争 responseQueue。 */ @Override - public PermissionResult resolve(ContentBlock.ToolUseBlock block, Tool tool, + public synchronized PermissionResult resolve(ContentBlock.ToolUseBlock block, Tool tool, PermissionResult.Ask askResult) { + // 清除可能残留的旧响应（防止上一次 offer 的响应被本次消费） + responseQueue.clear(); + // 渲染提示 renderPermissionRequest(block, tool, askResult); @@ -82,9 +88,17 @@ private void renderPermissionRequest(ContentBlock.ToolUseBlock block, Tool tool, w.flush(); } - /** 用户响应（由 REPL 主线程读取按键后调用）。 */ + /** + * 用户响应（由 REPL 主线程读取按键后调用）。 + * 修复 Copilot 审查：先 clear 再 put，防止用户多次按键时响应被 offer 静默丢弃。 + */ public void provideResponse(PermissionResponse response) { - responseQueue.offer(response); + responseQueue.clear(); + try { + responseQueue.put(response); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } /** 将用户响应转为 PermissionResult。 */ diff --git a/ccc-tools/src/main/java/com/ccj/tools/adapter/ToolCallbackAdapter.java b/ccc-tools/src/main/java/com/ccj/tools/adapter/ToolCallbackAdapter.java index 44b44bc..b6bf52c 100644 --- a/ccc-tools/src/main/java/com/ccj/tools/adapter/ToolCallbackAdapter.java +++ b/ccc-tools/src/main/java/com/ccj/tools/adapter/ToolCallbackAdapter.java @@ -19,12 +19,13 @@ * 让 Spring AI 的 ToolCallingAdvisor 工具调用循环能驱动我们的 Tool 实现。 * Spring AI 调用 call(toolInputJson, toolContext) 时： * 1. 反序列化 JSON 为 input Map - * 2. 构造 ToolUseContext（从 toolContext 取工作目录等） - * 3. 调用 tool.call(input, context) - * 4. 序列化结果为 JSON 字符串返回 + * 2. 如果配置了 PermissionChecker，先检查权限（deny 则返回错误，不执行工具） + * 3. 构造 ToolUseContext（从 toolContext 取工作目录等） + * 4. 调用 tool.call(input, context) + * 5. 序列化结果为 JSON 字符串返回 * - * 注意：权限检查和 Hook 在此适配器之外（由 ToolExecutionPipeline 或 - * PermissionAwareToolCallingAdvisor 处理）。此适配器只做纯执行委托。 + * 权限检查在适配器内部通过 PermissionChecker 执行。Hook 由 ToolExecutionPipeline + * 在非 Spring AI 路径处理。 */ public class ToolCallbackAdapter implements ToolCallback { @@ -51,7 +52,8 @@ public ToolCallbackAdapter(Tool tool, ToolUseContext contextTemplate) { public ToolCallbackAdapter(Tool tool, ToolUseContext contextTemplate, PermissionChecker permissionChecker) { this.tool = tool; - this.contextTemplate = contextTemplate; + // 修复 Copilot 审查：null contextTemplate 会导致 tool.call 收到 null，引发 NPE + this.contextTemplate = contextTemplate != null ? contextTemplate : ToolUseContext.defaultContext(); this.permissionChecker = permissionChecker; } @@ -89,7 +91,8 @@ public String call(String toolInput, ToolContext toolContext) { } } catch (Exception e) { // 权限检查器异常：fail-open（记录但不阻断） - log.warn("Permission checker exception for {}: {}", tool.name(), e.getMessage()); + // 修复 Copilot 审查：传入异常对象本身以记录完整堆栈 + log.warn("Permission checker exception for {}: {}", tool.name(), e.getMessage(), e); } }