Carlos Florêncio
@@ -88,6 +88,31 @@ Grok ACP currently exposes neither per-turn cost nor subscription quota/reset va
|
||||
|
||||
JSON-RPC 2.0 over stdin/stdout between agentctl and agent process. Requests: `initialize`, `session/new`, `session/load`, `session/prompt`, `session/cancel`, `session/close`. Notifications: `session/update` with types `message_chunk`, `tool_call`, `tool_update`, `complete`, `error`, `permission_request`, `context_window`.
|
||||
|
||||
### ACP permission identity and injected MCP approval
|
||||
|
||||
The ACP client preserves `ToolCall.Name` and `ToolCall.Meta` on the internal
|
||||
permission request. The Claude dialect reads `_meta.claudeCode.toolName` only
|
||||
for Claude frames and uses an exact qualified title fallback only when both
|
||||
identity fields are absent and the ACP kind is `other`. Malformed or
|
||||
conflicting identity data stays in the normal permission path. Other provider
|
||||
dialects must not inherit this title fallback without a tested wire contract.
|
||||
|
||||
The process manager may automatically select an offered allow-once, then
|
||||
allow-always option for any qualified tool on the host-injected Kandev MCP
|
||||
server when blanket approval is off. It requires the internal construction
|
||||
provenance marker and the exact current-port HTTP or SSE server entry. This
|
||||
server-wide provider-layer rule includes destructive Kandev tools; MCP
|
||||
authentication, task/session authorization, questions, and workflow gates stay
|
||||
separate. Shell, file, third-party, ambiguous, malformed, and unqualified
|
||||
requests keep the pending permission flow. Internal identity fields are not
|
||||
part of permission snapshots or stream events.
|
||||
|
||||
The flattened `mcp__server__tool` parser accepts only one unambiguous `__`
|
||||
separator. It rejects delimiter collisions, adjacent underscores at the
|
||||
boundary, and additional `__` sequences in the tool suffix. This prevents a
|
||||
server such as `kandev__external` from being read as the reserved `kandev`
|
||||
server. Tool names with these ambiguous forms keep the pending flow.
|
||||
|
||||
### ACP frame debug logging (`adapter/transport/shared/acplog.go`)
|
||||
|
||||
When `KANDEV_DEBUG_AGENT_MESSAGES=true` (on by default in the **dev** profile), the ACP adapter dumps every raw + normalized frame to **per-session** JSONL files:
|
||||
|
||||
@@ -202,10 +202,18 @@ func (c *Client) forwardPermissionRequest(ctx context.Context, handler Permissio
|
||||
actionDetails["description"] = description
|
||||
}
|
||||
|
||||
var toolName *string
|
||||
if p.ToolCall.Name != nil {
|
||||
name := *p.ToolCall.Name
|
||||
toolName = &name
|
||||
}
|
||||
|
||||
req := &types.PermissionRequest{
|
||||
SessionID: string(p.SessionId),
|
||||
ToolCallID: string(p.ToolCall.ToolCallId),
|
||||
Title: title,
|
||||
ToolName: toolName,
|
||||
ToolMeta: p.ToolCall.Meta,
|
||||
ActionType: actionType,
|
||||
ActionDetails: actionDetails,
|
||||
Options: options,
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/types"
|
||||
)
|
||||
|
||||
func TestPermissionProgrammaticName(t *testing.T) {
|
||||
programmaticName := "mcp__kandev__update_task_plan_kandev"
|
||||
meta := map[string]any{
|
||||
"claudeCode": map[string]any{"toolName": "update_task_plan_kandev"},
|
||||
}
|
||||
var captured *types.PermissionRequest
|
||||
handler := func(_ context.Context, req *types.PermissionRequest) (*types.PermissionResponse, error) {
|
||||
captured = req
|
||||
return &types.PermissionResponse{OptionID: "allow-once"}, nil
|
||||
}
|
||||
|
||||
client := NewClient()
|
||||
_, err := client.forwardPermissionRequest(context.Background(), handler, acpsdk.RequestPermissionRequest{
|
||||
SessionId: "session-1",
|
||||
ToolCall: acpsdk.ToolCallUpdate{
|
||||
ToolCallId: "tool-1",
|
||||
Title: stringPtr("Update task plan"),
|
||||
Kind: toolKindPtr(acpsdk.ToolKindOther),
|
||||
Name: &programmaticName,
|
||||
Meta: meta,
|
||||
},
|
||||
Options: []acpsdk.PermissionOption{{
|
||||
OptionId: "allow-once",
|
||||
Kind: acpsdk.PermissionOptionKindAllowOnce,
|
||||
}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("forwardPermissionRequest returned error: %v", err)
|
||||
}
|
||||
if captured == nil {
|
||||
t.Fatal("permission handler was not called")
|
||||
}
|
||||
if captured.ToolName == nil || *captured.ToolName != programmaticName {
|
||||
t.Fatalf("ToolName = %v, want %q", captured.ToolName, programmaticName)
|
||||
}
|
||||
// claudeCode.toolName uses the short form intentionally. The client passes
|
||||
// both fields through as-is, and dialect normalization happens later in the
|
||||
// adapter. The mismatch proves that the client is not synthesizing a name.
|
||||
if captured.ToolMeta["claudeCode"].(map[string]any)["toolName"] != "update_task_plan_kandev" {
|
||||
t.Fatalf("ToolMeta = %#v, want Claude metadata", captured.ToolMeta)
|
||||
}
|
||||
|
||||
encoded, err := json.Marshal(captured)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal permission request: %v", err)
|
||||
}
|
||||
if strings.Contains(string(encoded), programmaticName) || strings.Contains(string(encoded), "claudeCode") {
|
||||
t.Fatalf("internal identity fields leaked into serialized request: %s", encoded)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequestPermissionWithoutOptionsCancels(t *testing.T) {
|
||||
client := NewClient()
|
||||
response, err := client.RequestPermission(context.Background(), acpsdk.RequestPermissionRequest{
|
||||
SessionId: "session-1",
|
||||
ToolCall: acpsdk.ToolCallUpdate{ToolCallId: "tool-1"},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("RequestPermission returned error: %v", err)
|
||||
}
|
||||
if response.Outcome.Cancelled == nil || response.Outcome.Selected != nil {
|
||||
t.Fatalf("empty permission response = %+v, want cancelled", response.Outcome)
|
||||
}
|
||||
}
|
||||
|
||||
func stringPtr(value string) *string { return &value }
|
||||
|
||||
func toolKindPtr(value acpsdk.ToolKind) *acpsdk.ToolKind { return &value }
|
||||
@@ -11,6 +11,8 @@ import (
|
||||
// Since both acpclient and adapter now use the shared types package,
|
||||
// no conversion is needed - we just forward to the handler.
|
||||
func (a *Adapter) handlePermissionRequest(ctx context.Context, req *PermissionRequest) (*PermissionResponse, error) {
|
||||
req.ToolName = a.dialect.normalizePermissionToolName(req.ToolName, req.ToolMeta, req.Title, req.ActionType)
|
||||
|
||||
a.mu.RLock()
|
||||
handler := a.permissionHandler
|
||||
fallbackSessionID := a.sessionID
|
||||
|
||||
@@ -28,6 +28,7 @@ type acpDialect struct {
|
||||
mcpToolCall func(map[string]any, any) (mcpToolCallFrame, bool)
|
||||
mcpToolResult func(any) (any, bool)
|
||||
responseAttemptReset func(map[string]any) bool
|
||||
permissionToolName func(*string, map[string]any, string, string) *string
|
||||
}
|
||||
|
||||
type mcpToolCallFrame struct {
|
||||
@@ -59,6 +60,8 @@ func newACPDialect(agentID string) acpDialect {
|
||||
return newGrokACPDialect()
|
||||
case codexAgentID:
|
||||
return newCodexACPDialect()
|
||||
case claudeAgentID:
|
||||
return newClaudeACPDialect()
|
||||
case mockAgentID:
|
||||
return newMockACPDialect()
|
||||
}
|
||||
@@ -155,3 +158,15 @@ func (d acpDialect) promptUsage(
|
||||
func (d acpDialect) resetsResponseAttempt(meta map[string]any) bool {
|
||||
return d.responseAttemptReset != nil && d.responseAttemptReset(meta)
|
||||
}
|
||||
|
||||
func (d acpDialect) normalizePermissionToolName(
|
||||
name *string,
|
||||
meta map[string]any,
|
||||
title string,
|
||||
actionType string,
|
||||
) *string {
|
||||
if d.permissionToolName == nil {
|
||||
return name
|
||||
}
|
||||
return d.permissionToolName(name, meta, title, actionType)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"strings"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/types"
|
||||
)
|
||||
|
||||
const claudePermissionOtherActionType = "other"
|
||||
|
||||
// newClaudeACPDialect enables only the compatibility translations verified
|
||||
// against Claude ACP frames. Other providers retain their normal identity path.
|
||||
func newClaudeACPDialect() acpDialect {
|
||||
return acpDialect{permissionToolName: normalizeClaudePermissionToolName}
|
||||
}
|
||||
|
||||
func normalizeClaudePermissionToolName(
|
||||
programmaticName *string,
|
||||
meta map[string]any,
|
||||
title string,
|
||||
actionType string,
|
||||
) *string {
|
||||
metadataName, metadataPresent, metadataValid := claudePermissionMetadataName(meta)
|
||||
if !metadataValid {
|
||||
return emptyPermissionToolName()
|
||||
}
|
||||
if metadataPresent {
|
||||
if programmaticName != nil && *programmaticName != metadataName {
|
||||
return emptyPermissionToolName()
|
||||
}
|
||||
return &metadataName
|
||||
}
|
||||
|
||||
// A provider-supplied programmatic name is authoritative when no Claude
|
||||
// metadata identity is present. In particular, a nonmatching name must not
|
||||
// be replaced with a matching display title.
|
||||
if programmaticName != nil {
|
||||
return programmaticName
|
||||
}
|
||||
|
||||
if actionType != claudePermissionOtherActionType {
|
||||
return nil
|
||||
}
|
||||
candidate := strings.TrimSpace(title)
|
||||
server, _, ok := types.ParseQualifiedMCPToolName(candidate)
|
||||
if !ok || server != kandevMCPServerName {
|
||||
return nil
|
||||
}
|
||||
return &candidate
|
||||
}
|
||||
|
||||
func claudePermissionMetadataName(meta map[string]any) (name string, present, valid bool) {
|
||||
claudeMeta, ok := meta["claudeCode"]
|
||||
if !ok {
|
||||
return "", false, true
|
||||
}
|
||||
fields, ok := claudeMeta.(map[string]any)
|
||||
if !ok {
|
||||
return "", false, false
|
||||
}
|
||||
rawName, ok := fields["toolName"]
|
||||
if !ok {
|
||||
return "", false, true
|
||||
}
|
||||
name, ok = rawName.(string)
|
||||
if !ok {
|
||||
return "", false, false
|
||||
}
|
||||
return name, true, true
|
||||
}
|
||||
|
||||
func emptyPermissionToolName() *string {
|
||||
name := ""
|
||||
return &name
|
||||
}
|
||||
@@ -0,0 +1,148 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
func TestClaudePermissionIdentity(t *testing.T) {
|
||||
a := newTestAdapterForAgent(claudeAgentID)
|
||||
a.activeToolCalls["tool-1"] = &streams.NormalizedPayload{}
|
||||
var captured *PermissionRequest
|
||||
a.permissionHandler = func(_ context.Context, req *PermissionRequest) (*PermissionResponse, error) {
|
||||
captured = req
|
||||
return &PermissionResponse{OptionID: "allow-once"}, nil
|
||||
}
|
||||
|
||||
_, err := a.handlePermissionRequest(t.Context(), &PermissionRequest{
|
||||
ToolCallID: "tool-1",
|
||||
Title: "mcp__kandev__update_task_plan_kandev",
|
||||
ActionType: string(streams.ActionTypeOther),
|
||||
ToolMeta: map[string]any{
|
||||
"claudeCode": map[string]any{"toolName": "mcp__kandev__update_task_plan_kandev"},
|
||||
},
|
||||
Options: []PermissionOption{{
|
||||
OptionID: "allow-once",
|
||||
Kind: streams.PermissionOptionKindAllowOnce,
|
||||
}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("handlePermissionRequest returned error: %v", err)
|
||||
}
|
||||
if captured == nil || captured.ToolName == nil || *captured.ToolName != "mcp__kandev__update_task_plan_kandev" {
|
||||
t.Fatalf("normalized ToolName = %#v, want mcp__kandev__update_task_plan_kandev", captured)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClaudePermissionIdentityNormalization(t *testing.T) {
|
||||
qualified := "mcp__kandev__update_task_plan_kandev"
|
||||
other := "mcp__other__tool"
|
||||
empty := ""
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
programmatic *string
|
||||
meta map[string]any
|
||||
title string
|
||||
actionType string
|
||||
want *string
|
||||
}{
|
||||
{
|
||||
name: "programmatic name is authoritative when metadata agrees",
|
||||
programmatic: &qualified,
|
||||
meta: map[string]any{"claudeCode": map[string]any{"toolName": qualified}},
|
||||
title: qualified,
|
||||
actionType: claudePermissionOtherActionType,
|
||||
want: &qualified,
|
||||
},
|
||||
{
|
||||
name: "conflicting metadata blocks approval identity",
|
||||
programmatic: &qualified,
|
||||
meta: map[string]any{"claudeCode": map[string]any{"toolName": other}},
|
||||
want: &empty,
|
||||
},
|
||||
{
|
||||
name: "Claude metadata supplies qualified identity",
|
||||
meta: map[string]any{"claudeCode": map[string]any{"toolName": qualified}},
|
||||
want: &qualified,
|
||||
},
|
||||
{
|
||||
name: "legacy exact title is accepted only for other kind",
|
||||
title: qualified,
|
||||
actionType: claudePermissionOtherActionType,
|
||||
want: &qualified,
|
||||
},
|
||||
{
|
||||
name: "legacy ambiguous title has no identity",
|
||||
title: "mcp__kandev__external__execute",
|
||||
actionType: claudePermissionOtherActionType,
|
||||
},
|
||||
{
|
||||
name: "programmatic ambiguous name remains authoritative",
|
||||
programmatic: stringPtr("mcp__kandev__external__execute"),
|
||||
want: stringPtr("mcp__kandev__external__execute"),
|
||||
},
|
||||
{
|
||||
name: "legacy title with execute kind is not an identity",
|
||||
title: qualified,
|
||||
actionType: "execute",
|
||||
},
|
||||
{
|
||||
name: "malformed Claude metadata blocks title fallback",
|
||||
meta: map[string]any{"claudeCode": "invalid"},
|
||||
title: qualified,
|
||||
actionType: claudePermissionOtherActionType,
|
||||
want: &empty,
|
||||
},
|
||||
{
|
||||
name: "malformed Claude metadata blocks programmatic identity",
|
||||
programmatic: &qualified,
|
||||
meta: map[string]any{"claudeCode": "invalid"},
|
||||
want: &empty,
|
||||
},
|
||||
{
|
||||
name: "malformed tool name blocks title fallback",
|
||||
meta: map[string]any{"claudeCode": map[string]any{"toolName": 42}},
|
||||
title: qualified,
|
||||
actionType: claudePermissionOtherActionType,
|
||||
want: &empty,
|
||||
},
|
||||
{
|
||||
name: "unqualified metadata remains ineligible",
|
||||
meta: map[string]any{"claudeCode": map[string]any{"toolName": "update_task_plan_kandev"}},
|
||||
want: stringPtr("update_task_plan_kandev"),
|
||||
},
|
||||
{
|
||||
name: "explicit empty name remains authoritative",
|
||||
programmatic: &empty,
|
||||
title: qualified,
|
||||
actionType: claudePermissionOtherActionType,
|
||||
want: &empty,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
got := newACPDialect(claudeAgentID).normalizePermissionToolName(
|
||||
test.programmatic, test.meta, test.title, test.actionType)
|
||||
if (got == nil) != (test.want == nil) {
|
||||
t.Fatalf("normalized name = %v, want %v", got, test.want)
|
||||
}
|
||||
if got != nil && *got != *test.want {
|
||||
t.Fatalf("normalized name = %q, want %q", *got, *test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNonClaudePermissionIdentityDoesNotUseClaudeTitleFallback(t *testing.T) {
|
||||
qualified := "mcp__kandev__update_task_plan_kandev"
|
||||
got := newACPDialect("other-agent").normalizePermissionToolName(nil, nil, qualified, claudePermissionOtherActionType)
|
||||
if got != nil {
|
||||
t.Fatalf("non-Claude normalized name = %q, want nil", *got)
|
||||
}
|
||||
}
|
||||
|
||||
func stringPtr(value string) *string { return &value }
|
||||
@@ -278,6 +278,10 @@ type InstanceConfig struct {
|
||||
// McpServers is a list of MCP servers to configure for the agent
|
||||
McpServers []McpServerConfig
|
||||
|
||||
// InjectedKandevMCP records that this instance configuration was created
|
||||
// with the host-owned Kandev MCP server injected into McpServers.
|
||||
InjectedKandevMCP bool `json:"-"`
|
||||
|
||||
// ProcessBufferMaxBytes caps per-process output buffer size
|
||||
ProcessBufferMaxBytes int64
|
||||
|
||||
@@ -618,6 +622,7 @@ func (c *Config) NewInstanceConfig(port int, overrides *InstanceOverrides) *Inst
|
||||
// to forward tool calls to the backend.
|
||||
if port > 0 {
|
||||
cfg.McpServers = injectKandevMcpServer(cfg.McpServers, port)
|
||||
cfg.InjectedKandevMCP = true
|
||||
}
|
||||
|
||||
// Parse agent command into args
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -11,6 +12,7 @@ import (
|
||||
"time"
|
||||
|
||||
commonconfig "github.com/kandev/kandev/internal/common/config"
|
||||
"github.com/kandev/kandev/pkg/agent"
|
||||
)
|
||||
|
||||
func TestLoadWithStartupUsesExplicitManagedValues(t *testing.T) {
|
||||
@@ -106,6 +108,59 @@ func TestNewInstanceConfig_PropagatesMCPToolNamePresentationCapability(t *testin
|
||||
}
|
||||
}
|
||||
|
||||
func TestInjectedKandevMCPProvenance(t *testing.T) {
|
||||
workDir := t.TempDir()
|
||||
base := &Config{Defaults: InstanceDefaults{
|
||||
Protocol: agent.ProtocolACP,
|
||||
AgentCommand: "agent --acp",
|
||||
WorkDir: workDir,
|
||||
}}
|
||||
input := []McpServerConfig{
|
||||
{Name: "kandev", Type: "stdio", Command: "spoofed-kandev"},
|
||||
{Name: "third-party", Type: "http", URL: "https://mcp.example.test/mcp"},
|
||||
}
|
||||
|
||||
cfg := base.NewInstanceConfig(43210, &InstanceOverrides{
|
||||
Env: []string{},
|
||||
McpServers: input,
|
||||
})
|
||||
if !cfg.InjectedKandevMCP {
|
||||
t.Fatal("positive-port instance must retain injected Kandev provenance")
|
||||
}
|
||||
if len(cfg.McpServers) != 3 {
|
||||
t.Fatalf("McpServers = %+v, want injected HTTP/SSE plus third-party", cfg.McpServers)
|
||||
}
|
||||
if cfg.McpServers[0].Name != "kandev" || cfg.McpServers[0].Type != "http" || cfg.McpServers[0].URL != "http://localhost:43210/mcp" {
|
||||
t.Fatalf("HTTP injection = %+v", cfg.McpServers[0])
|
||||
}
|
||||
if cfg.McpServers[1].Name != "kandev" || cfg.McpServers[1].Type != "sse" || cfg.McpServers[1].URL != "http://localhost:43210/sse" {
|
||||
t.Fatalf("SSE injection = %+v", cfg.McpServers[1])
|
||||
}
|
||||
if cfg.McpServers[2].Name != "third-party" {
|
||||
t.Fatalf("unrelated MCP server was not preserved: %+v", cfg.McpServers)
|
||||
}
|
||||
|
||||
encoded, err := json.Marshal(cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal InstanceConfig: %v", err)
|
||||
}
|
||||
var serialized map[string]any
|
||||
if err := json.Unmarshal(encoded, &serialized); err != nil {
|
||||
t.Fatalf("unmarshal InstanceConfig: %v", err)
|
||||
}
|
||||
if _, present := serialized["InjectedKandevMCP"]; present {
|
||||
t.Fatalf("provenance marker leaked into serialized config: %s", encoded)
|
||||
}
|
||||
|
||||
withoutPort := base.NewInstanceConfig(0, &InstanceOverrides{
|
||||
Env: []string{},
|
||||
McpServers: input,
|
||||
})
|
||||
if withoutPort.InjectedKandevMCP {
|
||||
t.Fatal("zero-port instance must not claim injected Kandev provenance")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCollectAgentEnvKeepsGitHubCLIShimAheadOfProfilePath(t *testing.T) {
|
||||
t.Setenv("KANDEV_GITHUB_CREDENTIAL_BROKER_URL", "https://kandev.example/api/github/credentials/resolve")
|
||||
t.Setenv("KANDEV_GITHUB_CLI_SHIM_DIR", "/kandev/shims")
|
||||
|
||||
@@ -2762,6 +2762,9 @@ func (m *Manager) handlePermissionRequest(ctx context.Context, req *adapter.Perm
|
||||
if m.cfg.AutoApprovePermissions {
|
||||
return m.autoApprovePermission(req)
|
||||
}
|
||||
if response, approved := m.autoApproveInjectedKandevPermission(req); approved {
|
||||
return response, nil
|
||||
}
|
||||
|
||||
// Create pending permission with response channel
|
||||
createdAt := time.Now().UTC()
|
||||
|
||||
@@ -0,0 +1,498 @@
|
||||
package process
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/config"
|
||||
"github.com/kandev/kandev/internal/agentctl/types"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/pkg/agent"
|
||||
)
|
||||
|
||||
const (
|
||||
permissionACPPort = 43210
|
||||
injectedKandevToolName = "mcp__kandev__update_task_plan_kandev"
|
||||
thirdPartyToolName = "mcp__other__search"
|
||||
permissionFlowToolCall = "tool-1"
|
||||
permissionFlowSessionID = "session-acp"
|
||||
)
|
||||
|
||||
type permissionACPCase struct {
|
||||
name string
|
||||
taskID string
|
||||
resume bool
|
||||
toolName string
|
||||
title string
|
||||
toolKind acpsdk.ToolKind
|
||||
toolMeta map[string]any
|
||||
autoSelect string
|
||||
wantEvent bool
|
||||
}
|
||||
|
||||
func TestInjectedKandevPermissionACPFlow(t *testing.T) {
|
||||
tests := []permissionACPCase{
|
||||
{
|
||||
name: "fresh task auto approves injected tool",
|
||||
taskID: "task-1",
|
||||
toolName: injectedKandevToolName,
|
||||
title: "Update task plan",
|
||||
toolKind: acpsdk.ToolKindOther,
|
||||
toolMeta: claudePermissionMeta(injectedKandevToolName),
|
||||
autoSelect: "allow-once",
|
||||
},
|
||||
{
|
||||
name: "resumed quick chat auto approves injected tool",
|
||||
resume: true,
|
||||
toolName: injectedKandevToolName,
|
||||
title: "Update task plan",
|
||||
toolKind: acpsdk.ToolKindOther,
|
||||
toolMeta: claudePermissionMeta(injectedKandevToolName),
|
||||
autoSelect: "allow-once",
|
||||
},
|
||||
{
|
||||
name: "fresh task keeps Bash pending",
|
||||
taskID: "task-2",
|
||||
toolName: "bash",
|
||||
title: "Run bash command",
|
||||
toolKind: acpsdk.ToolKind("execute"),
|
||||
wantEvent: true,
|
||||
},
|
||||
{
|
||||
name: "resumed quick chat keeps third party MCP pending",
|
||||
resume: true,
|
||||
toolName: thirdPartyToolName,
|
||||
title: "Search third-party service",
|
||||
toolKind: acpsdk.ToolKindOther,
|
||||
wantEvent: true,
|
||||
},
|
||||
{
|
||||
name: "programmatic colliding server stays pending",
|
||||
taskID: "task-3",
|
||||
toolName: "mcp__kandev__external__execute",
|
||||
title: "Execute external MCP tool",
|
||||
toolKind: acpsdk.ToolKindOther,
|
||||
wantEvent: true,
|
||||
},
|
||||
{
|
||||
name: "legacy Claude colliding server stays pending",
|
||||
resume: true,
|
||||
title: "mcp__kandev__external__execute",
|
||||
toolKind: acpsdk.ToolKindOther,
|
||||
wantEvent: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) { runPermissionACPCase(t, test) })
|
||||
}
|
||||
}
|
||||
|
||||
func runPermissionACPCase(t *testing.T, test permissionACPCase) {
|
||||
t.Helper()
|
||||
manager, fake, cleanup := newPermissionACPFlow(t, test)
|
||||
defer cleanup()
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
preparePermissionACPTestSession(t, manager, ctx, test.resume)
|
||||
drainAdapterUpdates(manager.adapter)
|
||||
|
||||
promptDone := make(chan error, 1)
|
||||
go func() {
|
||||
promptDone <- manager.adapter.Prompt(ctx, "continue", nil, 1)
|
||||
}()
|
||||
|
||||
if test.wantEvent {
|
||||
resolvePendingPermissionACP(t, manager, test)
|
||||
} else {
|
||||
awaitAutoApprovedPermission(t, manager, fake, test.autoSelect)
|
||||
}
|
||||
waitForPermissionPrompt(t, ctx, promptDone)
|
||||
|
||||
if test.wantEvent {
|
||||
if got := fake.selectedOption(); got != "reject-once" {
|
||||
t.Fatalf("explicitly resolved permission option = %q, want reject-once", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
events := collectPromptEvents(manager.adapter.Updates())
|
||||
assertPromptEvents(t, events)
|
||||
select {
|
||||
case event := <-manager.GetUpdates():
|
||||
t.Fatalf("injected Kandev request emitted delayed manager event %q", event.Type)
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func preparePermissionACPTestSession(t *testing.T, manager *Manager, ctx context.Context, resume bool) {
|
||||
t.Helper()
|
||||
if resume {
|
||||
if err := manager.adapter.LoadSession(ctx, permissionFlowSessionID, nil); err != nil {
|
||||
t.Fatalf("LoadSession: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if _, err := manager.adapter.NewSession(ctx, permissionFlowMCPServers()); err != nil {
|
||||
t.Fatalf("NewSession: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func resolvePendingPermissionACP(t *testing.T, manager *Manager, test permissionACPCase) {
|
||||
t.Helper()
|
||||
event := waitForPermissionEvent(t, manager.GetUpdates())
|
||||
if event.Type != streams.EventTypePermissionRequest {
|
||||
t.Fatalf("manager event type = %q, want permission_request", event.Type)
|
||||
}
|
||||
if event.PermissionTitle != test.title {
|
||||
t.Fatalf("permission title = %q, want %q", event.PermissionTitle, test.title)
|
||||
}
|
||||
if _, err := manager.ResolvePermission(event.RequestID, event.PendingID, "reject-once"); err != nil {
|
||||
t.Fatalf("ResolvePermission: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func awaitAutoApprovedPermission(t *testing.T, manager *Manager, fake *permissionFlowAgent, wantOption string) {
|
||||
t.Helper()
|
||||
deadline := time.NewTimer(2 * time.Second)
|
||||
defer deadline.Stop()
|
||||
select {
|
||||
case event := <-manager.GetUpdates():
|
||||
t.Fatalf("injected Kandev request emitted manager event %q", event.Type)
|
||||
case <-fake.responseReady:
|
||||
if fake.selectedOption() != wantOption {
|
||||
t.Fatalf("selected permission option = %q, want %q", fake.selectedOption(), wantOption)
|
||||
}
|
||||
case <-deadline.C:
|
||||
t.Fatal("injected Kandev permission did not complete")
|
||||
}
|
||||
}
|
||||
|
||||
func waitForPermissionPrompt(t *testing.T, ctx context.Context, promptDone <-chan error) {
|
||||
t.Helper()
|
||||
select {
|
||||
case err := <-promptDone:
|
||||
if err != nil {
|
||||
t.Fatalf("Prompt: %v", err)
|
||||
}
|
||||
case <-ctx.Done():
|
||||
t.Fatalf("Prompt did not complete: %v", ctx.Err())
|
||||
}
|
||||
}
|
||||
|
||||
type permissionFlowAgent struct {
|
||||
burstPermissionAgent
|
||||
conn *acpsdk.AgentSideConnection
|
||||
toolName string
|
||||
title string
|
||||
toolKind acpsdk.ToolKind
|
||||
toolMeta map[string]any
|
||||
responseReady chan struct{}
|
||||
|
||||
selectedMu sync.Mutex
|
||||
selected string
|
||||
}
|
||||
|
||||
// The ACP server sends requests to this fake agent only through the methods
|
||||
// below. All agent-to-client traffic in the test goes through conn, matching
|
||||
// the production JSON-RPC boundary.
|
||||
type burstPermissionAgent struct{}
|
||||
|
||||
func (burstPermissionAgent) Authenticate(context.Context, acpsdk.AuthenticateRequest) (acpsdk.AuthenticateResponse, error) {
|
||||
return acpsdk.AuthenticateResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) Initialize(_ context.Context, req acpsdk.InitializeRequest) (acpsdk.InitializeResponse, error) {
|
||||
return acpsdk.InitializeResponse{
|
||||
ProtocolVersion: req.ProtocolVersion,
|
||||
AgentCapabilities: acpsdk.AgentCapabilities{
|
||||
LoadSession: true,
|
||||
McpCapabilities: acpsdk.McpCapabilities{
|
||||
Http: true,
|
||||
Sse: true,
|
||||
},
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) Logout(context.Context, acpsdk.LogoutRequest) (acpsdk.LogoutResponse, error) {
|
||||
return acpsdk.LogoutResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) Cancel(context.Context, acpsdk.CancelNotification) error { return nil }
|
||||
|
||||
func (burstPermissionAgent) CloseSession(context.Context, acpsdk.CloseSessionRequest) (acpsdk.CloseSessionResponse, error) {
|
||||
return acpsdk.CloseSessionResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) DeleteSession(context.Context, acpsdk.DeleteSessionRequest) (acpsdk.DeleteSessionResponse, error) {
|
||||
return acpsdk.DeleteSessionResponse{}, errors.New("not implemented")
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) ListSessions(context.Context, acpsdk.ListSessionsRequest) (acpsdk.ListSessionsResponse, error) {
|
||||
return acpsdk.ListSessionsResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) NewSession(context.Context, acpsdk.NewSessionRequest) (acpsdk.NewSessionResponse, error) {
|
||||
return acpsdk.NewSessionResponse{SessionId: permissionFlowSessionID}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) ResumeSession(context.Context, acpsdk.ResumeSessionRequest) (acpsdk.ResumeSessionResponse, error) {
|
||||
return acpsdk.ResumeSessionResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) SetSessionConfigOption(context.Context, acpsdk.SetSessionConfigOptionRequest) (acpsdk.SetSessionConfigOptionResponse, error) {
|
||||
return acpsdk.SetSessionConfigOptionResponse{}, nil
|
||||
}
|
||||
|
||||
func (burstPermissionAgent) SetSessionMode(context.Context, acpsdk.SetSessionModeRequest) (acpsdk.SetSessionModeResponse, error) {
|
||||
return acpsdk.SetSessionModeResponse{}, nil
|
||||
}
|
||||
|
||||
func (a *permissionFlowAgent) LoadSession(context.Context, acpsdk.LoadSessionRequest) (acpsdk.LoadSessionResponse, error) {
|
||||
return acpsdk.LoadSessionResponse{}, nil
|
||||
}
|
||||
|
||||
func (a *permissionFlowAgent) Prompt(ctx context.Context, req acpsdk.PromptRequest) (acpsdk.PromptResponse, error) {
|
||||
title := a.title
|
||||
kind := a.toolKind
|
||||
status := acpsdk.ToolCallStatus("pending")
|
||||
if err := a.conn.SessionUpdate(ctx, acpsdk.SessionNotification{
|
||||
SessionId: req.SessionId,
|
||||
Update: acpsdk.SessionUpdate{ToolCall: &acpsdk.SessionUpdateToolCall{
|
||||
ToolCallId: permissionFlowToolCall,
|
||||
Title: title,
|
||||
Kind: kind,
|
||||
Name: a.toolNamePtr(),
|
||||
Meta: a.toolMeta,
|
||||
Status: status,
|
||||
}},
|
||||
}); err != nil {
|
||||
return acpsdk.PromptResponse{}, err
|
||||
}
|
||||
|
||||
response, err := a.conn.RequestPermission(ctx, acpsdk.RequestPermissionRequest{
|
||||
SessionId: req.SessionId,
|
||||
ToolCall: acpsdk.ToolCallUpdate{
|
||||
ToolCallId: permissionFlowToolCall,
|
||||
Title: &title,
|
||||
Kind: &kind,
|
||||
Name: a.toolNamePtr(),
|
||||
Meta: a.toolMeta,
|
||||
},
|
||||
Options: []acpsdk.PermissionOption{
|
||||
{OptionId: "reject-once", Kind: acpsdk.PermissionOptionKindRejectOnce, Name: "Reject"},
|
||||
{OptionId: "allow-once", Kind: acpsdk.PermissionOptionKindAllowOnce, Name: "Allow once"},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
return acpsdk.PromptResponse{}, err
|
||||
}
|
||||
if response.Outcome.Selected != nil {
|
||||
a.selectedMu.Lock()
|
||||
a.selected = string(response.Outcome.Selected.OptionId)
|
||||
a.selectedMu.Unlock()
|
||||
}
|
||||
close(a.responseReady)
|
||||
|
||||
if response.Outcome.Cancelled != nil {
|
||||
return acpsdk.PromptResponse{StopReason: acpsdk.StopReasonEndTurn}, nil
|
||||
}
|
||||
|
||||
completed := acpsdk.ToolCallStatus("completed")
|
||||
if err := a.conn.SessionUpdate(ctx, acpsdk.SessionNotification{
|
||||
SessionId: req.SessionId,
|
||||
Update: acpsdk.SessionUpdate{ToolCallUpdate: &acpsdk.SessionToolCallUpdate{
|
||||
ToolCallId: permissionFlowToolCall,
|
||||
Status: &completed,
|
||||
RawOutput: "permission handled",
|
||||
}},
|
||||
}); err != nil {
|
||||
return acpsdk.PromptResponse{}, err
|
||||
}
|
||||
if err := a.conn.SessionUpdate(ctx, acpsdk.SessionNotification{
|
||||
SessionId: req.SessionId,
|
||||
Update: acpsdk.SessionUpdate{AgentMessageChunk: &acpsdk.SessionUpdateAgentMessageChunk{
|
||||
Content: acpsdk.TextBlock("permission handled"),
|
||||
}},
|
||||
}); err != nil {
|
||||
return acpsdk.PromptResponse{}, err
|
||||
}
|
||||
return acpsdk.PromptResponse{StopReason: acpsdk.StopReasonEndTurn}, nil
|
||||
}
|
||||
|
||||
func (a *permissionFlowAgent) selectedOption() string {
|
||||
a.selectedMu.Lock()
|
||||
defer a.selectedMu.Unlock()
|
||||
return a.selected
|
||||
}
|
||||
|
||||
func newPermissionACPFlow(t *testing.T, test permissionACPCase) (*Manager, *permissionFlowAgent, func()) {
|
||||
t.Helper()
|
||||
workDir := t.TempDir()
|
||||
cfg := (&config.Config{
|
||||
Defaults: config.InstanceDefaults{
|
||||
Protocol: agent.ProtocolACP,
|
||||
WorkDir: workDir,
|
||||
},
|
||||
}).NewInstanceConfig(permissionACPPort, &config.InstanceOverrides{
|
||||
AgentType: "claude-acp",
|
||||
WorkDir: workDir,
|
||||
TaskID: test.taskID,
|
||||
SessionID: permissionFlowSessionID,
|
||||
McpServers: permissionFlowConfigMCPServers(),
|
||||
})
|
||||
if !cfg.InjectedKandevMCP {
|
||||
t.Fatal("NewInstanceConfig did not record injected Kandev MCP provenance")
|
||||
}
|
||||
if !injectedKandevMCPConfigured(cfg) {
|
||||
t.Fatal("NewInstanceConfig did not retain the exact injected Kandev MCP entry")
|
||||
}
|
||||
var hasCollidingServer bool
|
||||
for _, server := range cfg.McpServers {
|
||||
if server.Name == "kandev__external" {
|
||||
hasCollidingServer = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !hasCollidingServer {
|
||||
t.Fatal("NewInstanceConfig did not retain the kandev__external test server")
|
||||
}
|
||||
manager := NewManager(cfg, newTestLogger(t))
|
||||
if err := manager.buildAdapterConfig(); err != nil {
|
||||
t.Fatalf("buildAdapterConfig: %v", err)
|
||||
}
|
||||
|
||||
clientToAgentReader, clientToAgentWriter := io.Pipe()
|
||||
agentToClientReader, agentToClientWriter := io.Pipe()
|
||||
if err := manager.adapter.Connect(clientToAgentWriter, agentToClientReader); err != nil {
|
||||
t.Fatalf("adapter Connect: %v", err)
|
||||
}
|
||||
fake := &permissionFlowAgent{
|
||||
toolName: test.toolName,
|
||||
title: test.title,
|
||||
toolKind: test.toolKind,
|
||||
toolMeta: test.toolMeta,
|
||||
responseReady: make(chan struct{}),
|
||||
}
|
||||
fake.conn = acpsdk.NewAgentSideConnection(fake, agentToClientWriter, clientToAgentReader)
|
||||
|
||||
cleanup := func() {
|
||||
_ = manager.adapter.Close()
|
||||
_ = clientToAgentReader.Close()
|
||||
_ = clientToAgentWriter.Close()
|
||||
_ = agentToClientReader.Close()
|
||||
_ = agentToClientWriter.Close()
|
||||
manager.lifetimeCancel()
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
if err := manager.adapter.Initialize(ctx); err != nil {
|
||||
cleanup()
|
||||
t.Fatalf("adapter Initialize: %v", err)
|
||||
}
|
||||
return manager, fake, cleanup
|
||||
}
|
||||
|
||||
func (a *permissionFlowAgent) toolNamePtr() *string {
|
||||
if a.toolName == "" {
|
||||
return nil
|
||||
}
|
||||
return &a.toolName
|
||||
}
|
||||
|
||||
func claudePermissionMeta(toolName string) map[string]any {
|
||||
return map[string]any{
|
||||
"claudeCode": map[string]any{"toolName": toolName},
|
||||
}
|
||||
}
|
||||
|
||||
func permissionFlowMCPServers() []types.McpServer {
|
||||
return []types.McpServer{
|
||||
{Name: "kandev", Type: "http", URL: "http://localhost:43210/mcp"},
|
||||
{Name: "kandev__external", Type: "http", URL: "https://mcp.example.test/external"},
|
||||
}
|
||||
}
|
||||
|
||||
func permissionFlowConfigMCPServers() []config.McpServerConfig {
|
||||
return []config.McpServerConfig{
|
||||
{Name: "kandev__external", Type: "http", URL: "https://mcp.example.test/external"},
|
||||
}
|
||||
}
|
||||
|
||||
func drainAdapterUpdates(agentAdapter adapter.AgentAdapter) {
|
||||
for {
|
||||
select {
|
||||
case <-agentAdapter.Updates():
|
||||
default:
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func waitForPermissionEvent(t *testing.T, events <-chan adapter.AgentEvent) adapter.AgentEvent {
|
||||
t.Helper()
|
||||
deadline := time.NewTimer(2 * time.Second)
|
||||
defer deadline.Stop()
|
||||
for {
|
||||
select {
|
||||
case event := <-events:
|
||||
if event.Type == streams.EventTypePermissionRequest {
|
||||
return event
|
||||
}
|
||||
case <-deadline.C:
|
||||
t.Fatal("timed out waiting for permission_request event")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func collectPromptEvents(events <-chan adapter.AgentEvent) []adapter.AgentEvent {
|
||||
var collected []adapter.AgentEvent
|
||||
deadline := time.NewTimer(2 * time.Second)
|
||||
defer deadline.Stop()
|
||||
for {
|
||||
select {
|
||||
case event := <-events:
|
||||
collected = append(collected, event)
|
||||
if event.Type == streams.EventTypeComplete {
|
||||
return collected
|
||||
}
|
||||
case <-deadline.C:
|
||||
return collected
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func assertPromptEvents(t *testing.T, events []adapter.AgentEvent) {
|
||||
t.Helper()
|
||||
want := []string{
|
||||
streams.EventTypeToolCall,
|
||||
streams.EventTypeToolUpdate,
|
||||
streams.EventTypeMessageChunk,
|
||||
streams.EventTypeComplete,
|
||||
}
|
||||
var got []string
|
||||
for _, event := range events {
|
||||
for _, wantType := range want {
|
||||
if event.Type == wantType {
|
||||
got = append(got, event.Type)
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("prompt event types = %v, want at least %v", got, want)
|
||||
}
|
||||
for i, wantType := range want {
|
||||
if got[i] != wantType {
|
||||
t.Fatalf("prompt event types = %v, want %v", got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,76 @@
|
||||
package process
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/config"
|
||||
"github.com/kandev/kandev/internal/agentctl/types"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// injectedKandevMCPServerName is the reserved server identifier used by the
|
||||
// host-injected Kandev MCP entry. This must match kandevMCPServerName in
|
||||
// adapter/transport/acp/adapter_session.go and kandevMcpServerName in
|
||||
// server/config/config.go.
|
||||
const injectedKandevMCPServerName = "kandev"
|
||||
|
||||
func (m *Manager) autoApproveInjectedKandevPermission(req *adapter.PermissionRequest) (*adapter.PermissionResponse, bool) {
|
||||
if req == nil || m.cfg == nil || !m.cfg.InjectedKandevMCP || m.cfg.Port <= 0 {
|
||||
return nil, false
|
||||
}
|
||||
if !injectedKandevMCPConfigured(m.cfg) {
|
||||
return nil, false
|
||||
}
|
||||
if req.ToolName == nil {
|
||||
return nil, false
|
||||
}
|
||||
server, tool, ok := types.ParseQualifiedMCPToolName(*req.ToolName)
|
||||
if !ok || server != injectedKandevMCPServerName {
|
||||
return nil, false
|
||||
}
|
||||
|
||||
option, ok := injectedKandevPermissionOption(req.Options)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
m.logger.Info("auto-approving injected Kandev MCP permission",
|
||||
zap.String("reason", "injected_kandev_mcp"),
|
||||
zap.String("tool", tool),
|
||||
zap.String("option_kind", string(option.Kind)))
|
||||
return &adapter.PermissionResponse{OptionID: option.OptionID}, true
|
||||
}
|
||||
|
||||
func injectedKandevMCPConfigured(cfg *config.InstanceConfig) bool {
|
||||
for _, server := range cfg.McpServers {
|
||||
if server.Name != injectedKandevMCPServerName || server.Command != "" || len(server.Args) != 0 || len(server.Env) != 0 || len(server.Headers) != 0 {
|
||||
continue
|
||||
}
|
||||
switch server.Type {
|
||||
case "http":
|
||||
if server.URL == fmt.Sprintf("http://localhost:%d/mcp", cfg.Port) {
|
||||
return true
|
||||
}
|
||||
case "sse":
|
||||
if server.URL == fmt.Sprintf("http://localhost:%d/sse", cfg.Port) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func injectedKandevPermissionOption(options []adapter.PermissionOption) (adapter.PermissionOption, bool) {
|
||||
for _, allowedKind := range []streams.PermissionOptionKind{
|
||||
streams.PermissionOptionKindAllowOnce,
|
||||
streams.PermissionOptionKindAllowAlways,
|
||||
} {
|
||||
for _, option := range options {
|
||||
if option.Kind == allowedKind {
|
||||
return option, true
|
||||
}
|
||||
}
|
||||
}
|
||||
return adapter.PermissionOption{}, false
|
||||
}
|
||||
@@ -0,0 +1,255 @@
|
||||
package process
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/config"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
func TestInjectedKandevPermissionPolicy(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
toolName := "mcp__kandev__update_task_plan_kandev"
|
||||
|
||||
m := &Manager{
|
||||
cfg: &config.InstanceConfig{
|
||||
Port: 43210,
|
||||
InjectedKandevMCP: true,
|
||||
McpServers: []config.McpServerConfig{
|
||||
{Name: "kandev", Type: "http", URL: "http://localhost:43210/mcp"},
|
||||
{Name: "kandev", Type: "sse", URL: "http://localhost:43210/sse"},
|
||||
{Name: "third-party", Type: "http", URL: "https://mcp.example.test/mcp"},
|
||||
},
|
||||
},
|
||||
logger: newTestLogger(t),
|
||||
updatesCh: make(chan adapter.AgentEvent, 2),
|
||||
pendingPermissions: make(map[string]*PendingPermission),
|
||||
}
|
||||
|
||||
resultCh := make(chan struct {
|
||||
response *adapter.PermissionResponse
|
||||
err error
|
||||
}, 1)
|
||||
go func() {
|
||||
response, err := m.handlePermissionRequest(ctx, &adapter.PermissionRequest{
|
||||
SessionID: "session-1",
|
||||
ToolCallID: "tool-1",
|
||||
Title: "Update task plan",
|
||||
ToolName: &toolName,
|
||||
ActionType: string(streams.ActionTypeOther),
|
||||
Options: []adapter.PermissionOption{
|
||||
{OptionID: "reject", Kind: streams.PermissionOptionKindRejectOnce},
|
||||
{OptionID: "allow-once", Kind: streams.PermissionOptionKindAllowOnce},
|
||||
},
|
||||
})
|
||||
resultCh <- struct {
|
||||
response *adapter.PermissionResponse
|
||||
err error
|
||||
}{response: response, err: err}
|
||||
}()
|
||||
|
||||
select {
|
||||
case result := <-resultCh:
|
||||
if result.err != nil {
|
||||
t.Fatalf("handlePermissionRequest returned error: %v", result.err)
|
||||
}
|
||||
if result.response == nil || result.response.Cancelled || result.response.OptionID != "allow-once" {
|
||||
t.Fatalf("response = %+v, want allow-once without cancellation", result.response)
|
||||
}
|
||||
case event := <-m.updatesCh:
|
||||
cancel()
|
||||
select {
|
||||
case <-resultCh:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("permission request did not stop after cancellation")
|
||||
}
|
||||
t.Fatalf("injected Kandev request emitted %q instead of being approved", event.Type)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInjectedKandevPermissionOptions(t *testing.T) {
|
||||
m := injectedKandevPermissionManager(t, injectedKandevMCPServers(43210))
|
||||
toolName := "mcp__kandev__update_task_plan_kandev"
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
options []adapter.PermissionOption
|
||||
wantOption string
|
||||
wantOK bool
|
||||
}{
|
||||
{
|
||||
name: "allow once wins regardless of order",
|
||||
options: []adapter.PermissionOption{
|
||||
{OptionID: "reject", Kind: streams.PermissionOptionKindRejectOnce},
|
||||
{OptionID: "allow-always", Kind: streams.PermissionOptionKindAllowAlways},
|
||||
{OptionID: "allow-once", Kind: streams.PermissionOptionKindAllowOnce},
|
||||
},
|
||||
wantOption: "allow-once",
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "allow always is fallback",
|
||||
options: []adapter.PermissionOption{
|
||||
{OptionID: "reject", Kind: streams.PermissionOptionKindRejectOnce},
|
||||
{OptionID: "allow-always", Kind: streams.PermissionOptionKindAllowAlways},
|
||||
},
|
||||
wantOption: "allow-always",
|
||||
wantOK: true,
|
||||
},
|
||||
{
|
||||
name: "reject only retains normal flow",
|
||||
options: []adapter.PermissionOption{
|
||||
{OptionID: "reject", Kind: streams.PermissionOptionKindRejectOnce},
|
||||
},
|
||||
wantOK: false,
|
||||
},
|
||||
{
|
||||
name: "unknown option kind retains normal flow",
|
||||
options: []adapter.PermissionOption{
|
||||
{OptionID: "unknown", Kind: streams.PermissionOptionKind("maybe")},
|
||||
},
|
||||
wantOK: false,
|
||||
},
|
||||
{
|
||||
name: "empty options retain normal flow",
|
||||
options: nil,
|
||||
wantOK: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
response, ok := m.autoApproveInjectedKandevPermission(&adapter.PermissionRequest{
|
||||
ToolName: &toolName,
|
||||
Options: test.options,
|
||||
})
|
||||
if ok != test.wantOK {
|
||||
t.Fatalf("approved = %v, want %v; response=%+v", ok, test.wantOK, response)
|
||||
}
|
||||
if !test.wantOK {
|
||||
return
|
||||
}
|
||||
if response == nil || response.Cancelled || response.OptionID != test.wantOption {
|
||||
t.Fatalf("response = %+v, want option %q", response, test.wantOption)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestInjectedKandevPermissionPolicyCoversServerTools(t *testing.T) {
|
||||
m := injectedKandevPermissionManager(t, injectedKandevMCPServers(43210))
|
||||
for _, toolName := range []string{
|
||||
"mcp__kandev__create_task_kandev",
|
||||
"mcp__kandev__update_task_plan_kandev",
|
||||
"mcp__kandev__delete_task_kandev",
|
||||
} {
|
||||
t.Run(toolName, func(t *testing.T) {
|
||||
response, approved := m.autoApproveInjectedKandevPermission(&adapter.PermissionRequest{
|
||||
ToolName: permissionStringPtr(toolName),
|
||||
Options: []adapter.PermissionOption{{
|
||||
OptionID: "allow-once",
|
||||
Kind: streams.PermissionOptionKindAllowOnce,
|
||||
}},
|
||||
})
|
||||
if !approved || response == nil || response.OptionID != "allow-once" {
|
||||
t.Fatalf("tool %q response = %+v, approved = %v", toolName, response, approved)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestInjectedKandevPolicyRunsAfterBlanketApproval(t *testing.T) {
|
||||
m := injectedKandevPermissionManager(t, injectedKandevMCPServers(43210))
|
||||
m.cfg.AutoApprovePermissions = true
|
||||
toolName := "mcp__kandev__update_task_plan_kandev"
|
||||
response, err := m.handlePermissionRequest(context.Background(), &adapter.PermissionRequest{
|
||||
ToolName: &toolName,
|
||||
Options: []adapter.PermissionOption{
|
||||
{OptionID: "allow-always", Kind: streams.PermissionOptionKindAllowAlways},
|
||||
{OptionID: "allow-once", Kind: streams.PermissionOptionKindAllowOnce},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("handlePermissionRequest returned error: %v", err)
|
||||
}
|
||||
if response == nil || response.OptionID != "allow-always" {
|
||||
t.Fatalf("blanket response = %+v, want first offered allow-always", response)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInjectedKandevPermissionPolicyRejectsUntrustedRequests(t *testing.T) {
|
||||
matchingTitle := "mcp__kandev__update_task_plan_kandev"
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
toolName *string
|
||||
title string
|
||||
servers []config.McpServerConfig
|
||||
injectedProvenance bool
|
||||
wantApproved bool
|
||||
}{
|
||||
{name: "every injected tool is eligible", toolName: stringPtr("mcp__kandev__delete_task_kandev"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: true},
|
||||
{name: "matching title cannot establish identity", toolName: nil, servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "nonmatching programmatic name wins over title", toolName: permissionStringPtr("mcp__other__tool"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "Bash title resembling MCP name is not identity", toolName: permissionStringPtr("bash"), title: "Run mcp__kandev__update_task_plan_kandev", servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "suffix alone is not qualified", toolName: permissionStringPtr("update_task_plan_kandev"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "empty qualified suffix is rejected", toolName: permissionStringPtr("mcp__kandev__"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "invalid characters are rejected", toolName: permissionStringPtr("mcp__kandev__delete/task"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
{name: "missing provenance is rejected", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: injectedKandevMCPServers(43210), wantApproved: false},
|
||||
{name: "wrong port is rejected", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: injectedKandevMCPServers(43211), injectedProvenance: true, wantApproved: false},
|
||||
{name: "stdio server is rejected", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: []config.McpServerConfig{{Name: "kandev", Type: "stdio", Command: "agent"}}, injectedProvenance: true, wantApproved: false},
|
||||
{name: "streamable HTTP is not an injected transport", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: []config.McpServerConfig{{Name: "kandev", Type: "streamable_http", URL: "http://localhost:43210/mcp"}}, injectedProvenance: true, wantApproved: false},
|
||||
{name: "configured command invalidates HTTP entry", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: []config.McpServerConfig{{Name: "kandev", Type: "http", URL: "http://localhost:43210/mcp", Command: "spoof"}}, injectedProvenance: true, wantApproved: false},
|
||||
{name: "reserved-name spoof with headers is rejected", toolName: permissionStringPtr("mcp__kandev__update_task_plan_kandev"), servers: []config.McpServerConfig{{Name: "kandev", Type: "http", URL: "http://localhost:43210/mcp", Headers: map[string]string{"Authorization": "spoof"}}}, injectedProvenance: true, wantApproved: false},
|
||||
{name: "unrelated server does not become eligible", toolName: permissionStringPtr("mcp__third-party__tool"), servers: injectedKandevMCPServers(43210), injectedProvenance: true, wantApproved: false},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
m := injectedKandevPermissionManager(t, test.servers)
|
||||
m.cfg.InjectedKandevMCP = test.injectedProvenance
|
||||
title := test.title
|
||||
if title == "" {
|
||||
title = matchingTitle
|
||||
}
|
||||
response, approved := m.autoApproveInjectedKandevPermission(&adapter.PermissionRequest{
|
||||
Title: title,
|
||||
ToolName: test.toolName,
|
||||
Options: []adapter.PermissionOption{{
|
||||
OptionID: "allow-once",
|
||||
Kind: streams.PermissionOptionKindAllowOnce,
|
||||
}},
|
||||
})
|
||||
if approved != test.wantApproved {
|
||||
t.Fatalf("approved = %v, response = %+v, want %v", approved, response, test.wantApproved)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func injectedKandevPermissionManager(t *testing.T, servers []config.McpServerConfig) *Manager {
|
||||
t.Helper()
|
||||
return &Manager{
|
||||
cfg: &config.InstanceConfig{
|
||||
Port: 43210,
|
||||
InjectedKandevMCP: true,
|
||||
McpServers: servers,
|
||||
},
|
||||
logger: newTestLogger(t),
|
||||
}
|
||||
}
|
||||
|
||||
func injectedKandevMCPServers(port int) []config.McpServerConfig {
|
||||
return []config.McpServerConfig{
|
||||
{Name: "kandev", Type: "http", URL: "http://localhost:" + strconv.Itoa(port) + "/mcp"},
|
||||
{Name: "kandev", Type: "sse", URL: "http://localhost:" + strconv.Itoa(port) + "/sse"},
|
||||
{Name: "third-party", Type: "http", URL: "https://mcp.example.test/mcp"},
|
||||
}
|
||||
}
|
||||
|
||||
func permissionStringPtr(value string) *string { return &value }
|
||||
@@ -0,0 +1,49 @@
|
||||
package types
|
||||
|
||||
import "strings"
|
||||
|
||||
// ParseQualifiedMCPToolName parses an unambiguous protocol-qualified MCP tool
|
||||
// name used by ACP clients, such as mcp__kandev__update_task_plan_kandev.
|
||||
// The flattened format cannot identify a boundary when either side contains
|
||||
// a delimiter-like underscore sequence, so those names fail closed.
|
||||
func ParseQualifiedMCPToolName(name string) (server, tool string, ok bool) {
|
||||
if !strings.HasPrefix(name, "mcp__") {
|
||||
return "", "", false
|
||||
}
|
||||
|
||||
qualified := strings.TrimPrefix(name, "mcp__")
|
||||
separator := strings.Index(qualified, "__")
|
||||
if !unambiguousQualifiedMCPSeparator(qualified, separator) {
|
||||
return "", "", false
|
||||
}
|
||||
server, tool = qualified[:separator], qualified[separator+2:]
|
||||
if !permissionIdentifier(server) || !permissionIdentifier(tool) {
|
||||
return "", "", false
|
||||
}
|
||||
return server, tool, true
|
||||
}
|
||||
|
||||
func unambiguousQualifiedMCPSeparator(qualified string, separator int) bool {
|
||||
if separator <= 0 || separator+2 >= len(qualified) {
|
||||
return false
|
||||
}
|
||||
if qualified[separator-1] == '_' || qualified[separator+2] == '_' {
|
||||
return false
|
||||
}
|
||||
return !strings.Contains(qualified[separator+2:], "__")
|
||||
}
|
||||
|
||||
func permissionIdentifier(value string) bool {
|
||||
if value == "" {
|
||||
return false
|
||||
}
|
||||
for _, char := range value {
|
||||
if (char < 'a' || char > 'z') &&
|
||||
(char < 'A' || char > 'Z') &&
|
||||
(char < '0' || char > '9') &&
|
||||
char != '_' && char != '-' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
@@ -0,0 +1,36 @@
|
||||
package types
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestParseQualifiedMCPToolName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
wantServer string
|
||||
wantTool string
|
||||
wantOK bool
|
||||
}{
|
||||
{name: "qualified tool", input: "mcp__kandev__update_task_plan_kandev", wantServer: "kandev", wantTool: "update_task_plan_kandev", wantOK: true},
|
||||
{name: "hyphenated identifiers", input: "mcp__third-party__tool-name", wantServer: "third-party", wantTool: "tool-name", wantOK: true},
|
||||
{name: "single underscores in tool", input: "mcp__kandev__tool_with_underscores", wantServer: "kandev", wantTool: "tool_with_underscores", wantOK: true},
|
||||
{name: "delimiter in tool suffix is ambiguous", input: "mcp__kandev__tool__with__underscores"},
|
||||
{name: "delimiter in server name is ambiguous", input: "mcp__kandev__external__execute"},
|
||||
{name: "trailing underscore server is ambiguous", input: "mcp__kandev___execute"},
|
||||
{name: "missing prefix", input: "kandev__update_task_plan_kandev"},
|
||||
{name: "missing server", input: "mcp____update_task_plan_kandev"},
|
||||
{name: "missing tool", input: "mcp__kandev__"},
|
||||
{name: "whitespace", input: "mcp__kandev__tool name"},
|
||||
{name: "slash", input: "mcp__kandev__tool/name"},
|
||||
{name: "unicode identifier", input: "mcp__kandev__工具"},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
server, tool, ok := ParseQualifiedMCPToolName(test.input)
|
||||
if server != test.wantServer || tool != test.wantTool || ok != test.wantOK {
|
||||
t.Fatalf("ParseQualifiedMCPToolName(%q) = (%q, %q, %v), want (%q, %q, %v)",
|
||||
test.input, server, tool, ok, test.wantServer, test.wantTool, test.wantOK)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -121,6 +121,14 @@ type PermissionRequest struct {
|
||||
Title string `json:"title"`
|
||||
Options []PermissionOption `json:"options"`
|
||||
|
||||
// ToolName is the optional programmatic identity supplied by ACP. It is
|
||||
// retained for internal permission policy decisions and is never serialized
|
||||
// into public permission snapshots.
|
||||
ToolName *string `json:"-"`
|
||||
// ToolMeta contains provider-specific ACP metadata used by adapter dialects.
|
||||
// It is internal and must not be exposed through permission snapshots.
|
||||
ToolMeta map[string]any `json:"-"`
|
||||
|
||||
// PendingID is the unique identifier for this permission request.
|
||||
// If set by the adapter (e.g., OpenCode's "per_xxx" ID or Claude Code's requestID),
|
||||
// the process manager will use this ID instead of generating a new one.
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
# ADR-2026-09-16-injected-mcp-approval: Approve the injected Kandev server through ACP
|
||||
|
||||
**Status:** accepted
|
||||
**Date:** 2026-09-16
|
||||
**Area:** backend, protocol
|
||||
|
||||
## Context
|
||||
|
||||
Issue #3729 reports repeated Claude ACP permission prompts for Kandev plan and task tools.
|
||||
Codex already receives server-wide approval through its launch configuration.
|
||||
The issue explicitly offers server-wide Claude parity as its minimum fix.
|
||||
|
||||
## Decision
|
||||
|
||||
The proposed repair adds a server-specific decision before ACP creates a pending permission request.
|
||||
It requires a provider tool identity and host-owned injection provenance.
|
||||
It approves every tool on the injected Kandev server, not only bookkeeping tools.
|
||||
It does not enable blanket approval or change task authorization.
|
||||
|
||||
Provider naming compatibility belongs in pure adapter dialect hooks.
|
||||
The process manager owns policy and choice selection.
|
||||
Unknown identities retain normal permission handling.
|
||||
The flattened `mcp__server__tool` form is accepted only when it has one
|
||||
unambiguous separator. Delimiter collisions and adjacent underscores remain
|
||||
pending because the server/tool boundary cannot be trusted.
|
||||
|
||||
The [requirements](../specs/agents/requirements/injected-mcp-approval.md)
|
||||
and [design](../specs/agents/system-design/injected-mcp-approval.md) define the implemented boundary.
|
||||
|
||||
## Consequences
|
||||
|
||||
Claude can update plans and task titles without repeated prompts.
|
||||
The same policy also approves task creation, deletion, and session-management calls on the injected server.
|
||||
Existing server-side authorization and user-input barriers still apply.
|
||||
No approval history, new profile field, or database migration is necessary.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- Blanket profile approval also permits shell and third-party operations. It exceeds the requested boundary.
|
||||
- A selective allowlist and a restore-prompting setting need a different policy across Claude and Codex. They are outside this minimum package.
|
||||
- CLI-only allow flags depend on provider behavior and do not resolve the confirmed host-side pending-request path.
|
||||
- A generic title-prefix match cannot distinguish provider presentation from programmatic identity. The design restricts legacy title interpretation to Claude.
|
||||
@@ -0,0 +1,107 @@
|
||||
---
|
||||
created: 2026-09-16
|
||||
status: complete
|
||||
requirements:
|
||||
- REQ-AGENTS-INJECTED-MCP-APPROVAL-001
|
||||
system_design:
|
||||
- ../../specs/agents/system-design/injected-mcp-approval.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Fix plan: Claude injected MCP approval
|
||||
|
||||
## Overview
|
||||
|
||||
Resolve [issue #3729](https://github.com/kdlbs/kandev/issues/3729) through its minimum server-wide parity option.
|
||||
The issue is assigned to `carlosflorencio`.
|
||||
One sequential work order implements the complete permission path and its regression coverage.
|
||||
Implementation is complete through the single work order below.
|
||||
|
||||
## Evidence and root cause
|
||||
|
||||
Investigated commit: `4c6e80091d82b4302e9dfc0a17b80965f1d4c62d`.
|
||||
The canonical issue has no comments or image attachments at investigation time.
|
||||
|
||||
The process manager checks only blanket `AutoApprovePermissions` before it stores a pending request.
|
||||
There is no injected-server exception. The ACP client also discards the programmatic tool name.
|
||||
Codex's separate passthrough strategy already emits a server-wide approval override.
|
||||
|
||||
A temporary `TestIssue3729Repro` used `Config.NewInstanceConfig(43210, nil)` and the real manager permission handler.
|
||||
It submitted `mcp__kandev__update_task_plan_kandev`, kind `other`, with allow-once and reject-once choices.
|
||||
Blanket approval remained false. The handler emitted `permission_request` and waited until context cancellation.
|
||||
|
||||
Command run:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test ./internal/agentctl/server/process -run '^TestIssue3729Repro$' -count=1 -v)
|
||||
```
|
||||
|
||||
Result: PASS, with the unwanted permission event observed. The temporary test was removed after execution.
|
||||
This proves the host permission path, not a live Claude or desktop session.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Preserve ACP programmatic tool identity and normalize legacy Claude identity.
|
||||
- Approve the host-injected server before pending-request creation.
|
||||
- Preserve unrelated permissions and normal tool events.
|
||||
- Document the server-wide policy and cover fresh and resumed configurations.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Per-tool allowlists, new settings, persistence, and Claude terminal passthrough.
|
||||
- Frontend changes, provider upgrades, and unverified provider dialects.
|
||||
- Changes to Codex policy or server-side authorization.
|
||||
|
||||
## Technical approach
|
||||
|
||||
Follow the [system design](../../specs/agents/system-design/injected-mcp-approval.md).
|
||||
The implementation crosses the existing ACP client, adapter, instance configuration, and process manager.
|
||||
Use new focused helper and test files instead of expanding large manager files.
|
||||
|
||||
## Tests
|
||||
|
||||
| Criteria | Permanent regression evidence |
|
||||
| --- | --- |
|
||||
| 001.1, 001.3, 001.8 | `client_permission_identity_test.go:TestPermissionProgrammaticName` and `dialect_claude_test.go:TestClaudePermissionIdentity` |
|
||||
| 001.2, 001.4, 001.7 | `config_test.go:TestInjectedKandevMCPProvenance` and `manager_permission_policy_test.go:TestInjectedKandevPermissionPolicy` |
|
||||
| 001.5, 001.6 | `manager_permission_policy_test.go:TestInjectedKandevPermissionOptions` |
|
||||
| 001.1, 001.3, 001.7, 001.8 | `manager_permission_acp_test.go:TestInjectedKandevPermissionACPFlow` |
|
||||
|
||||
The first policy regression must fail on current code because the manager emits a pending request.
|
||||
Tests must include mixed server lists, spoofed names, a Bash title that resembles an MCP name, and reversed option ordering.
|
||||
|
||||
## End-to-end evidence
|
||||
|
||||
The Go protocol integration test connects the real ACP conversion, adapter, and process manager through an in-memory ACP peer.
|
||||
It emits a Kandev permission request and verifies the selected wire response without a pending permission event.
|
||||
It then emits Bash and third-party requests and verifies that both remain pending until explicit resolution.
|
||||
Run the same sequence with fresh and resumed session identities and with task and Quick Chat instance contexts.
|
||||
Verify normal tool-call/result events remain present.
|
||||
This protocol boundary is the deterministic end-to-end evidence. No artificial browser test or UI redraw is required.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Implement injected server approval](task-01-injected-server-approval.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
Investigation reproduction passed as described above. Implementation and verification passed.
|
||||
|
||||
- `(cd apps/backend && go test -race ./internal/agentctl/server/acp ./internal/agentctl/server/adapter/transport/acp ./internal/agentctl/server/config ./internal/agentctl/server/process ./internal/agent/mcpconfig -count=1)`: passed.
|
||||
- `node --test scripts/validate-public-docs.test.mjs`: passed.
|
||||
- `node scripts/validate-public-docs.mjs`: passed.
|
||||
- `python3 scripts/list-docs.py validate`: passed.
|
||||
- `python3 scripts/lint-spec-files.test.py`: passed.
|
||||
- `python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Catalog discovery found both new agent specification files.
|
||||
- GitHub assignment readback confirmed `carlosflorencio`.
|
||||
|
||||
## Risks
|
||||
|
||||
- The minimum policy approves all injected Kandev tools, including destructive and work-spawning tools.
|
||||
- Legacy Claude title interpretation requires exact provider and kind checks. It must not become a generic title heuristic.
|
||||
- Installed provider formats can differ. Unsupported shapes retain prompts, and compatibility claims require fixtures.
|
||||
- No live provider session was run during investigation.
|
||||
@@ -0,0 +1,151 @@
|
||||
---
|
||||
id: "01-injected-server-approval"
|
||||
title: "Implement injected server approval"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-AGENTS-INJECTED-MCP-APPROVAL-001
|
||||
acceptance_criteria:
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.1
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.2
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.3
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.4
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.5
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.6
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.7
|
||||
- AC-AGENTS-INJECTED-MCP-APPROVAL-001.8
|
||||
system_design:
|
||||
- ../../specs/agents/system-design/injected-mcp-approval.md
|
||||
---
|
||||
|
||||
# Task 01: Implement injected server approval
|
||||
|
||||
## Summary
|
||||
|
||||
Add a permission decision for the host-injected Kandev server without enabling blanket approval.
|
||||
Preserve identity through the ACP conversion and use strict provider normalization.
|
||||
|
||||
## In scope
|
||||
|
||||
- Start with a failing manager regression for an injected Kandev plan update with blanket approval disabled.
|
||||
- Add identity fields, Claude dialect normalization, injection provenance, and offered-option selection through the documented path.
|
||||
- Add the unit and protocol integration tests named in the plan.
|
||||
- Update the permission sections in `docs/public/agents-and-profiles.md` and scoped agentctl guidance.
|
||||
- Mark the ADR accepted only after explicit implementation authorization for this package.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- New permission settings, storage changes, UI controls, and unrelated approval cleanup.
|
||||
- Changes to blanket approval's existing first-option fallback.
|
||||
- Removing Codex's existing launch override.
|
||||
|
||||
## Acceptance
|
||||
|
||||
1. All listed criteria pass through real production seams, including the protocol integration test.
|
||||
2. Unknown identity, forged title, unrelated server, missing provenance, and missing allow choices never receive approval through the new policy.
|
||||
3. Public reference text explains the server-wide exception, its destructive-tool scope, and its independence from blanket approval.
|
||||
|
||||
## Test matrix
|
||||
|
||||
- Names: programmatic qualified name, Claude metadata, legacy Claude title, empty name, malformed metadata, conflicting fields, wrong server, suffix-only name.
|
||||
- Operations: plan creation/update, title update, task deletion/creation, Bash, file edit, third-party MCP.
|
||||
- Injection: actual constructor, zero port, user-supplied reserved name, wrong port/path/transport, mixed server inventory, recreated instance.
|
||||
- Options: reject first, allow-always first with allow-once later, allow-always only, reject only, empty, unknown kinds.
|
||||
- Lifecycle: fresh/resumed session, task/Quick Chat context, cancellation of a remaining prompt, blanket approval on/off.
|
||||
- Streams: normal tool-call/result events, no automatic-path permission card, normal explicit resolution for unrelated tools.
|
||||
|
||||
Use channel synchronization and cleanup registration before assertions.
|
||||
Do not use arbitrary sleeps or a real user instance.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/agentctl/server/acp ./internal/agentctl/server/adapter/transport/acp ./internal/agentctl/server/config ./internal/agentctl/server/process ./internal/agent/mcpconfig -count=1)
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
python3 scripts/list-docs.py validate
|
||||
python3 scripts/lint-spec-files.test.py
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Record the initial failing regression and each final command result.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
Paths below are repository-relative. New files are marked `(new)`.
|
||||
|
||||
- `apps/backend/internal/agentctl/types/types.go`
|
||||
- `apps/backend/internal/agentctl/types/permission_identity.go` (new)
|
||||
- `apps/backend/internal/agentctl/types/permission_identity_test.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/acp/client.go`
|
||||
- `apps/backend/internal/agentctl/server/acp/client_permission_identity_test.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_claude.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_claude_test.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_permissions.go`
|
||||
- `apps/backend/internal/agentctl/server/config/config.go`
|
||||
- `apps/backend/internal/agentctl/server/config/config_test.go`
|
||||
- `apps/backend/internal/agentctl/server/process/manager.go`
|
||||
- `apps/backend/internal/agentctl/server/process/manager_permission_policy.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/process/manager_permission_policy_test.go` (new)
|
||||
- `apps/backend/internal/agentctl/server/process/manager_permission_acp_test.go` (new)
|
||||
- `apps/backend/internal/agentctl/AGENTS.md`
|
||||
- `docs/public/agents-and-profiles.md`
|
||||
- `docs/decisions/2026-09-16-injected-mcp-approval.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
Server-wide approval includes task deletion and creation.
|
||||
The implementation must not infer trust from raw input or a display title from an unknown provider.
|
||||
Public permission snapshots must not expose the newly preserved provider metadata.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/agents/requirements/injected-mcp-approval.md)
|
||||
- [System design](../../specs/agents/system-design/injected-mcp-approval.md)
|
||||
- [Plan and evidence](plan.md)
|
||||
- Existing `client_test.go` permission conversion tests.
|
||||
- Existing `manager_permission_test.go` pending-request and cancellation tests.
|
||||
- Existing `config_test.go` injected HTTP/SSE ordering tests.
|
||||
- Repository `/tdd` skill and its backend test reference.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented the Claude ACP injected-server permission path. ACP conversion now
|
||||
preserves internal programmatic identity and metadata, the Claude dialect
|
||||
normalizes only verified identity forms, and instance construction records
|
||||
host-injection provenance. The process manager approves every qualified tool
|
||||
on the exact injected Kandev HTTP or SSE server, preferring allow-once and then
|
||||
allow-always, while blanket approval and all unrelated pending flows remain
|
||||
unchanged. Flattened names with delimiter collisions fail closed to the
|
||||
pending flow. Permanent unit and in-memory ACP protocol tests cover fresh and
|
||||
resumed task and Quick Chat contexts, destructive and planning tools, spoofed
|
||||
identities, malformed metadata, option ordering, normal tool/result events, and
|
||||
explicit unrelated-tool resolution.
|
||||
|
||||
The public permissions reference and scoped agentctl guidance document the
|
||||
server-wide exception and its independent authorization boundaries. The ADR is
|
||||
accepted, and the paired requirement and system design are active/current.
|
||||
|
||||
Verification passed:
|
||||
|
||||
- `(cd apps/backend && go test -race ./internal/agentctl/server/acp ./internal/agentctl/server/adapter/transport/acp ./internal/agentctl/server/config ./internal/agentctl/server/process ./internal/agent/mcpconfig -count=1)`
|
||||
- `node --test scripts/validate-public-docs.test.mjs`
|
||||
- `node scripts/validate-public-docs.mjs`
|
||||
- `python3 scripts/list-docs.py validate`
|
||||
- `python3 scripts/lint-spec-files.test.py`
|
||||
- `python3 scripts/lint-spec-files.py --all`
|
||||
- `git diff --check`
|
||||
@@ -383,6 +383,8 @@ Literal values remain in profile configuration. A secret reference avoids copyin
|
||||
|
||||
In a structured ACP session, the agent can present a permission request and its available responses. With **Auto-approve all permissions** disabled, a person chooses a response in the session. With it enabled, the runtime selects the first allow-once or allow-always response without waiting. If the agent supplies no allow response, Kandev selects its first response even when that response is not approval; with no responses, it cancels.
|
||||
|
||||
Claude ACP also has a narrower built-in rule. When the agent uses the host-injected `kandev` MCP server, Kandev automatically selects an offered allow-once response, or allow-always when allow-once is not offered, even when **Auto-approve all permissions** is disabled. This covers every tool on that server, including task creation, task deletion, and plan changes. It is not a saved approval decision and is recreated for each task or Quick Chat session. Kandev still enforces MCP authentication, task and session authorization, user-question barriers, and workflow gates. Shell commands, file operations, third-party tools, and ambiguous requests keep the normal permission flow.
|
||||
|
||||
An external MCP client can also list live pending requests for an authorized task and submit one
|
||||
exact option originally offered by the agent. The request-generation ID prevents an old approval
|
||||
from targeting a replacement prompt that reused a provider pending ID. Kandev records who selected
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
---
|
||||
status: active
|
||||
system: agents
|
||||
created: 2026-09-16
|
||||
owners:
|
||||
- kandev
|
||||
---
|
||||
|
||||
# Injected MCP approval requirements
|
||||
|
||||
## Overview
|
||||
|
||||
Claude ACP sessions require repeated approval for Kandev task and plan tools.
|
||||
Codex already receives automatic approval for the injected Kandev server.
|
||||
The agent system owns this provider permission contract.
|
||||
|
||||
This package adopts the minimum server-wide parity option in
|
||||
[issue #3729](https://github.com/kdlbs/kandev/issues/3729).
|
||||
It does not adopt the issue's separate proposal for selective approval and a new setting.
|
||||
|
||||
## Requirements
|
||||
|
||||
### REQ-AGENTS-INJECTED-MCP-APPROVAL-001: Approval for the injected Kandev server
|
||||
|
||||
**Intent:** Users can use Kandev tools through Claude ACP without enabling blanket approval for the agent.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.1:** With blanket approval disabled, identifiable calls to the injected Kandev server shall proceed without permission prompts in Claude ACP sessions.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.2:** This policy shall apply to every tool on that server, including task creation and deletion. Tool authorization shall remain enforced.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.3:** Shell commands, file operations, third-party tools, and ambiguous requests shall retain their existing permission behavior.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.4:** A user-configured server with a similar or identical name shall not receive approval through this policy.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.5:** Automatic approval shall select an offered allow-once choice. If none exists, it shall use an offered allow-always choice.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.6:** Without an offered allow choice, the request shall retain its normal permission behavior. A request without choices shall cancel.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.7:** Fresh and resumed task sessions, including Quick Chat, shall use the same policy without a saved approval decision.
|
||||
- **AC-AGENTS-INJECTED-MCP-APPROVAL-001.8:** Existing Codex approval and blanket approval behavior shall remain unchanged. Unknown provider naming formats shall retain normal permission behavior.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Selective tool lists, persisted approval decisions, and new profile or workspace settings.
|
||||
- Claude terminal passthrough, which does not use ACP permission frames.
|
||||
- Automatic support claims for unverified provider naming formats.
|
||||
- Changes to question barriers, workflow gates, or server-side task authorization.
|
||||
|
||||
## Implementation plans
|
||||
|
||||
- [Issue 3729 fix package](../../../plans/claude-injected-mcp-approval/plan.md)
|
||||
@@ -0,0 +1,123 @@
|
||||
---
|
||||
status: current
|
||||
system: agents
|
||||
requirements:
|
||||
- REQ-AGENTS-INJECTED-MCP-APPROVAL-001
|
||||
---
|
||||
|
||||
# Injected MCP approval design
|
||||
|
||||
## Purpose and boundaries
|
||||
|
||||
The process manager owns the automatic permission decision.
|
||||
The ACP client preserves provider identity fields, and the adapter normalizes provider-specific names.
|
||||
The MCP server continues to enforce task and session authorization.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Acceptance criteria | Design section |
|
||||
| --- | --- |
|
||||
| 001.1, 001.3, 001.8 | Identity normalization |
|
||||
| 001.2, 001.4 | Injection provenance |
|
||||
| 001.5, 001.6 | Permission decision |
|
||||
| 001.7 | Lifecycle and observability |
|
||||
|
||||
All references belong to `REQ-AGENTS-INJECTED-MCP-APPROVAL-001`.
|
||||
|
||||
## Current path and defect
|
||||
|
||||
`internal/agent/mcpconfig/passthrough.go:codexServerArgs` adds
|
||||
`default_tools_approval_mode=approve` for the `kandev` server.
|
||||
ACP uses a different path:
|
||||
|
||||
1. `server/acp/client.go:forwardPermissionRequest` converts the permission frame.
|
||||
2. `transport/acp/adapter_permissions.go:handlePermissionRequest` forwards it.
|
||||
3. `server/process/manager.go:handlePermissionRequest` checks only `AutoApprovePermissions`.
|
||||
4. Without blanket approval, the manager creates a pending request and waits.
|
||||
|
||||
The ACP conversion currently drops the programmatic `ToolCall.Name` and its metadata.
|
||||
The pinned Go SDK already supports both fields. No SDK upgrade is necessary.
|
||||
The shared adapter's `AutoApprove` field does not implement the permission decision.
|
||||
|
||||
## Identity normalization
|
||||
|
||||
Extend the internal `types.PermissionRequest` with optional `ToolName` and `ToolMeta` fields.
|
||||
Copy `ToolCall.Name` and `ToolCall.Meta` in `forwardPermissionRequest`.
|
||||
These fields remain internal. Do not expose metadata in public permission snapshots or logs.
|
||||
|
||||
Use a strict qualified-name parser for `mcp__kandev__<tool>`.
|
||||
Because this flattened format uses `__` as its boundary, accept only one
|
||||
unambiguous separator. Reject a separator next to another underscore and
|
||||
reject another `__` in the remaining name. This keeps names such as
|
||||
`mcp__kandev__external__execute` and `mcp__kandev___execute` out of the
|
||||
automatic path, even when the tool identifier itself could contain `__`.
|
||||
Require the exact server segment and a nonempty tool identifier with letters,
|
||||
digits, underscores, or hyphens.
|
||||
Do not match substrings, descriptions, raw arguments, or the `_kandev` suffix alone.
|
||||
|
||||
A supplied programmatic name is authoritative, including a nonmatching name.
|
||||
Never replace a nonmatching name with a matching display title.
|
||||
For `claude-acp` frames without a name, use a pure compatibility hook in `dialect_claude.go`.
|
||||
The hook first reads `_meta.claudeCode.toolName` when present.
|
||||
Malformed or conflicting metadata produces no eligible identity.
|
||||
Only when both identity fields are absent can the hook use an exact qualified title with kind `other`.
|
||||
This fallback is specific to Claude's observed MCP presentation, not a generic ACP rule.
|
||||
Other provider formats retain the normal prompt path until a tested dialect supports them.
|
||||
|
||||
The upstream [Claude tool formatter](https://github.com/agentclientprotocol/claude-agent-acp/blob/v0.75.1/src/tools.ts)
|
||||
uses the tool name as the default MCP title.
|
||||
Its [permission presentation](https://github.com/agentclientprotocol/claude-agent-acp/blob/v0.75.1/src/permissions/presentation.ts)
|
||||
also supplies a programmatic name.
|
||||
Implementation fixtures must cover both shapes because installed versions can differ.
|
||||
|
||||
## Injection provenance
|
||||
|
||||
Add an internal `InjectedKandevMCP` boolean with `json:"-"` to `config.InstanceConfig`.
|
||||
Only `Config.NewInstanceConfig` sets it, immediately after local server injection with a positive instance port.
|
||||
Do not add it to `InstanceOverrides`, public DTOs, persisted profiles, or incoming JSON.
|
||||
The marker is false for configurations without host injection.
|
||||
|
||||
The permission predicate requires this marker and the exact injected server configuration.
|
||||
Require a URL for the current instance port at `http://localhost:<port>/mcp` or `/sse`.
|
||||
Require no stdio command and the corresponding supported HTTP/SSE transport.
|
||||
The current injector replaces user entries named `kandev`, so those entries cannot retain the reserved slot.
|
||||
Cover a mixed list containing both the injected server and unrelated servers.
|
||||
The existence of unrelated entries must not grant them approval or disable the injected entry.
|
||||
|
||||
## Permission decision
|
||||
|
||||
Keep the existing blanket approval branch first.
|
||||
Before pending-request allocation, evaluate the injected-server predicate.
|
||||
Select an offered `allow_once`, then an offered `allow_always`.
|
||||
Return its exact option ID. Never invent a choice or fall back to a reject option.
|
||||
If no allow choice exists, continue through the existing pending-request path.
|
||||
The ACP client's existing empty-options cancellation remains intact.
|
||||
|
||||
Use a small helper in `server/process/manager_permission_policy.go`.
|
||||
Do not reuse `autoApprovePermission` for this branch: that helper falls back to the first option, including rejection.
|
||||
Keep all existing pending-request identity, cancellation, and exactly-once resolution rules unchanged.
|
||||
|
||||
## Lifecycle and observability
|
||||
|
||||
The policy has no persistence or migration. Instance construction recreates provenance after restart or resume.
|
||||
The adapter retains normal tool-call and result events on the automatic path.
|
||||
Automatic approval does not create a pending permission card on desktop or phone.
|
||||
Log the policy reason, tool identifier, and selected option kind without arguments, headers, or credentials.
|
||||
No frontend layout or copy changes are required.
|
||||
|
||||
## Security and compatibility
|
||||
|
||||
This policy trusts the configured agent's protocol identity, as the existing permission flow does.
|
||||
It does not defend against a malicious agent process that fabricates protocol frames.
|
||||
Raw MCP arguments cannot establish identity or injection provenance.
|
||||
Unknown and malformed names fail closed to the normal permission flow.
|
||||
|
||||
Server-wide parity also approves destructive Kandev tools at the provider permission layer.
|
||||
Task reachability, ownership, explicit user-question answers, and workflow completion rules remain separate enforcement points.
|
||||
The external permission-resolution specification remains authoritative for requests that become pending.
|
||||
Its unchanged-auto-approval statement describes that feature's scope, not this proposed policy addition.
|
||||
|
||||
## Related decisions
|
||||
|
||||
- [Injected server approval policy](../../../decisions/2026-09-16-injected-mcp-approval.md)
|
||||
- [Live permission authority](../../../decisions/2026-08-11-live-agent-permission-authority.md)
|
||||
Reference in New Issue
Block a user