feat(office): migrate workflow decisions to task-bound CLI (#3484)

This commit is contained in:
Carlos Florêncio
2026-09-09 20:08:48 +01:00
committed by GitHub
parent 1eaa21bf45
commit 30fcbfb901
81 changed files with 2976 additions and 766 deletions
+45 -1
View File
@@ -7,9 +7,14 @@ import (
"strings"
)
const (
decisionApproved = "approved"
decisionRejected = "rejected"
)
func runTaskCmd(args []string) int {
if len(args) == 0 {
cliError("usage: agentctl kandev task <get|update|create> [flags]")
cliError("usage: agentctl kandev task <get|update|create|decision> [flags]")
return 1
}
switch args[0] {
@@ -19,12 +24,51 @@ func runTaskCmd(args []string) int {
return taskUpdate(args[1:])
case subcmdCreate:
return taskCreate(args[1:])
case "decision":
return taskDecision(args[1:])
default:
cliError("unknown task subcommand: %s", args[0])
return 1
}
}
// taskDecision records the caller's workflow-step verdict through the signed
// Office runtime API. The runtime derives every identity field from the token.
func taskDecision(args []string) int {
fs := flag.NewFlagSet("task decision", flag.ContinueOnError)
decision := fs.String("decision", "", fmt.Sprintf("Verdict: %s or %s (required)", decisionApproved, decisionRejected))
reason := fs.String("reason", "", "Reason for the verdict (required)")
if err := fs.Parse(args); err != nil {
cliError("parse flags: %v", err)
return 1
}
if fs.NArg() > 0 {
cliError("unexpected arguments: %s", strings.Join(fs.Args(), " "))
return 1
}
switch *decision {
case decisionApproved, decisionRejected:
default:
cliError("--decision must be %q or %q", decisionApproved, decisionRejected)
return 1
}
if strings.TrimSpace(*reason) == "" {
cliError("--reason is required")
return 1
}
client, err := newKandevClient()
if err != nil {
cliError("%v", err)
return 1
}
body, status, err := client.do(http.MethodPost, "/api/v1/office/runtime/task/decision", map[string]string{
"decision": *decision,
"reason": *reason,
})
return handleResponse(body, status, err)
}
// taskGet fetches a task by ID. Defaults to $KANDEV_TASK_ID when --id is omitted.
func taskGet(args []string) int {
fs := flag.NewFlagSet("task get", flag.ContinueOnError)
@@ -0,0 +1,55 @@
package main
import (
"encoding/json"
"net/http"
"testing"
)
func TestTaskDecision_PostsOnlyDecisionAndReasonToRuntimeEndpoint(t *testing.T) {
captured := setupMockTransport(t, http.StatusOK, `{"decision":"approved"}`)
t.Setenv("KANDEV_API_URL", "http://kandev.test")
t.Setenv("KANDEV_API_KEY", "signed-office-run-token")
t.Setenv("KANDEV_RUN_ID", "run-456")
t.Setenv("KANDEV_TASK_ID", "task-from-environment")
t.Setenv("KANDEV_AGENT_ID", "agent-from-environment")
code := runKandevCLI([]string{
"task", "decision", "--decision", "approved", "--reason", "looks good",
})
if code != 0 {
t.Fatalf("task decision exit = %d, want 0", code)
}
if captured.Method != http.MethodPost || captured.Path != "/api/v1/office/runtime/task/decision" {
t.Fatalf("request = %s %s, want POST /api/v1/office/runtime/task/decision", captured.Method, captured.Path)
}
assertRunIDHeader(t, captured, "run-456")
var body map[string]any
if err := json.Unmarshal([]byte(captured.Body), &body); err != nil {
t.Fatalf("unmarshal request body: %v", err)
}
if len(body) != 2 || body["decision"] != "approved" || body["reason"] != "looks good" {
t.Fatalf("request body = %#v, want only decision and reason", body)
}
}
func TestTaskDecision_RejectsInvalidInputBeforeRequest(t *testing.T) {
captured := setupMockTransport(t, http.StatusOK, `{}`)
t.Setenv("KANDEV_API_URL", "http://kandev.test")
t.Setenv("KANDEV_API_KEY", "signed-office-run-token")
if code := runKandevCLI([]string{"task", "decision", "--decision", "approve", "--reason", "ok"}); code == 0 {
t.Fatal("expected invalid decision to fail")
}
if captured.Method != "" {
t.Fatalf("invalid decision contacted server: %s %s", captured.Method, captured.Path)
}
if code := runKandevCLI([]string{"task", "decision", "--decision", "approved", "--reason", " "}); code == 0 {
t.Fatal("expected blank reason to fail")
}
if captured.Method != "" {
t.Fatalf("blank reason contacted server: %s %s", captured.Method, captured.Path)
}
}
@@ -16,7 +16,6 @@ Available tools:
- list_task_documents_kandev: List documents on an accessible related task. Required: task_id.
- get_task_document_kandev: Read one document on an accessible related task. Required: task_id, document_key.
- write_task_document_kandev: Create or replace a document on the current task or an ancestor. Required: task_id, document_key, content. Optional: title, type.
- record_step_decision_kandev: Record an approve/reject decision for the current workflow step, as a reviewer or approver. Required: decision ("approved" or "rejected"), reason. Not idempotent: a second call supersedes the first.
- step_complete_kandev: Signal that every requirement for the current workflow step is satisfied when the step requires an explicit signal. Required: summary.
{step_complete_instruction}
@@ -259,21 +259,23 @@ const (
MetadataKeyOfficeAgentProfileID = "office_agent_profile_id"
// SSH runtime metadata keys (per-session, except SSHWorkdirRoot which is per-profile).
MetadataKeySSHHostAlias = "ssh_host_alias"
MetadataKeySSHHost = "ssh_host"
MetadataKeySSHPort = "ssh_port"
MetadataKeySSHUser = "ssh_user"
MetadataKeySSHHostFingerprint = "ssh_host_fingerprint"
MetadataKeySSHRemoteTaskDir = "ssh_remote_task_dir"
MetadataKeySSHRemoteSessionDir = "ssh_remote_session_dir"
MetadataKeySSHRemoteAgentctlPort = "ssh_remote_agentctl_port"
MetadataKeySSHRemoteAgentctlPID = "ssh_remote_agentctl_pid"
MetadataKeySSHLocalForwardPort = "ssh_local_forward_port"
MetadataKeySSHRemoteAgentctlURL = "ssh_remote_agentctl_url"
MetadataKeySSHWorkdirRoot = "ssh_workdir_root"
MetadataKeySSHProxyJump = "ssh_proxy_jump"
MetadataKeySSHIdentitySource = "ssh_identity_source"
MetadataKeySSHIdentityFile = "ssh_identity_file"
MetadataKeySSHHostAlias = "ssh_host_alias"
MetadataKeySSHHost = "ssh_host"
MetadataKeySSHPort = "ssh_port"
MetadataKeySSHUser = "ssh_user"
MetadataKeySSHHostFingerprint = "ssh_host_fingerprint"
MetadataKeySSHRemoteTaskDir = "ssh_remote_task_dir"
MetadataKeySSHRemoteSessionDir = "ssh_remote_session_dir"
MetadataKeySSHRemoteAgentctlPort = "ssh_remote_agentctl_port"
MetadataKeySSHRemoteAgentctlPID = "ssh_remote_agentctl_pid"
MetadataKeySSHLocalForwardPort = "ssh_local_forward_port"
MetadataKeySSHRemoteAgentctlURL = "ssh_remote_agentctl_url"
MetadataKeySSHRuntimeAPILocalURL = "ssh_runtime_api_local_url"
MetadataKeySSHRuntimeAPIRemotePort = "ssh_runtime_api_remote_port"
MetadataKeySSHWorkdirRoot = "ssh_workdir_root"
MetadataKeySSHProxyJump = "ssh_proxy_jump"
MetadataKeySSHIdentitySource = "ssh_identity_source"
MetadataKeySSHIdentityFile = "ssh_identity_file"
// MetadataKeySSHShell names the login shell used when running commands
// over SSH on the remote (probe, agentctl launch, install, setup
// scripts). Empty / unset falls back to "bash" at runtime — see
@@ -309,22 +311,24 @@ var persistentMetadataKeys = map[string]bool{
MetadataKeyLocalPort: true,
// SSH runtime
MetadataKeySSHHost: true,
MetadataKeySSHPort: true,
MetadataKeySSHUser: true,
MetadataKeySSHHostFingerprint: true,
MetadataKeySSHRemoteTaskDir: true,
MetadataKeySSHRemoteSessionDir: true,
MetadataKeySSHRemoteAgentctlPort: true,
MetadataKeySSHRemoteAgentctlPID: true,
MetadataKeySSHLocalForwardPort: true,
MetadataKeySSHRemoteAgentctlURL: true,
MetadataKeySSHWorkdirRoot: true,
MetadataKeySSHProxyJump: true,
MetadataKeySSHIdentitySource: true,
MetadataKeySSHIdentityFile: true,
MetadataKeySSHShell: true,
MetadataKeySSHReclaimTaskDir: true,
MetadataKeySSHHost: true,
MetadataKeySSHPort: true,
MetadataKeySSHUser: true,
MetadataKeySSHHostFingerprint: true,
MetadataKeySSHRemoteTaskDir: true,
MetadataKeySSHRemoteSessionDir: true,
MetadataKeySSHRemoteAgentctlPort: true,
MetadataKeySSHRemoteAgentctlPID: true,
MetadataKeySSHLocalForwardPort: true,
MetadataKeySSHRemoteAgentctlURL: true,
MetadataKeySSHRuntimeAPILocalURL: true,
MetadataKeySSHRuntimeAPIRemotePort: true,
MetadataKeySSHWorkdirRoot: true,
MetadataKeySSHProxyJump: true,
MetadataKeySSHIdentitySource: true,
MetadataKeySSHIdentityFile: true,
MetadataKeySSHShell: true,
MetadataKeySSHReclaimTaskDir: true,
// Kubernetes connection and exact resource inventory.
MetadataKeyKubernetesAuthMode: true,
@@ -390,7 +394,8 @@ var persistentMetadataPrefixes = []string{
// runtime resources (process PIDs, allocated ports, session directories on
// the remote). These keys ARE persisted across a SAME-session resume — that's
// how a backend restart reattaches to a still-running remote agent — but they
// MUST NOT be carried across SIBLING sessions on the same task. If they were,
// MUST NOT be carried across SIBLING sessions on the same task. This includes
// the SSH runtime API reverse-tunnel URL and port. If they were,
// the second session would try to attach to the first session's agentctl
// process and end up sharing its ACP session and instance port.
var sessionScopedMetadataKeys = map[string]bool{
@@ -403,6 +408,8 @@ var sessionScopedMetadataKeys = map[string]bool{
MetadataKeySSHRemoteAgentctlPID: true,
MetadataKeySSHLocalForwardPort: true,
MetadataKeySSHRemoteAgentctlURL: true,
MetadataKeySSHRuntimeAPILocalURL: true,
MetadataKeySSHRuntimeAPIRemotePort: true,
MetadataKeyKubernetesAuthMode: true,
MetadataKeyKubernetesKubeconfigPath: true,
MetadataKeyKubernetesKubeContext: true,
@@ -80,6 +80,16 @@ func TestOfficeAgentIdentityMetadataIsSessionScoped(t *testing.T) {
"Office identity must not leak to a sibling session")
}
func TestSSHRuntimeAPIMetadataIsPersistentAndSessionScoped(t *testing.T) {
for _, key := range []string{
MetadataKeySSHRuntimeAPILocalURL,
MetadataKeySSHRuntimeAPIRemotePort,
} {
require.True(t, ShouldPersistMetadataKey(key), "%s must survive same-session restart", key)
require.True(t, IsSessionScopedMetadataKey(key), "%s must not leak to sibling sessions", key)
}
}
func TestFilterPersistentMetadata(t *testing.T) {
t.Run("nil input returns nil", func(t *testing.T) {
require.Nil(t, FilterPersistentMetadata(nil))
@@ -40,18 +40,19 @@ const (
// when many short-lived channels race the long-lived stream channels), and
// the local port forwarder.
type sshSessionState struct {
target *SSHTarget
client *ssh.Client
forwarder *SSHPortForwarder
agentctlClient *agentctl.Client
pid int
remoteDir string
remoteTaskDir string
authToken string
reusingProcess bool
metadata map[string]interface{}
prepareEnv map[string]string
platform SSHRemotePlatform
target *SSHTarget
client *ssh.Client
forwarder *SSHPortForwarder
agentctlClient *agentctl.Client
pid int
remoteDir string
remoteTaskDir string
authToken string
reusingProcess bool
metadata map[string]interface{}
prepareEnv map[string]string
platform SSHRemotePlatform
runtimeAPITunnel *sshRuntimeAPITunnel
}
// SSHExecutor implements ExecutorBackend for SSH-reachable Linux and macOS hosts.
@@ -117,6 +118,7 @@ func (r *SSHExecutor) Close() error {
}
r.mu.Unlock()
for _, s := range states {
_ = s.runtimeAPITunnel.Close()
if s.forwarder != nil {
_ = s.forwarder.Close()
}
@@ -216,9 +218,11 @@ func (r *SSHExecutor) CreateInstance(ctx context.Context, req *ExecutorCreateReq
if err != nil {
return nil, fmt.Errorf("ssh: connect to %s@%s: %w", target.User, target.Host, err)
}
var runtimeAPITunnel *sshRuntimeAPITunnel
released := false
defer func() {
if !released {
_ = runtimeAPITunnel.Close()
_ = client.Close()
}
}()
@@ -231,6 +235,10 @@ func (r *SSHExecutor) CreateInstance(ctx context.Context, req *ExecutorCreateReq
if err != nil {
return nil, err
}
runtimeAPITunnel, _, err = openSSHRuntimeAPITunnelForRequest(client, req)
if err != nil {
return nil, err
}
workdir := r.workdirRoot(req.Metadata)
taskDir := ""
@@ -277,16 +285,17 @@ func (r *SSHExecutor) CreateInstance(ctx context.Context, req *ExecutorCreateReq
r.mu.Lock()
r.sessions[req.InstanceID] = &sshSessionState{
target: target,
client: client,
forwarder: fwd,
pid: pid,
remoteDir: sessionDir,
remoteTaskDir: taskDir,
authToken: authToken,
metadata: cloneSSHMetadata(req.Metadata),
prepareEnv: sshRemoteContributionEnv(req, agentctlBin),
platform: platform,
target: target,
client: client,
forwarder: fwd,
pid: pid,
remoteDir: sessionDir,
remoteTaskDir: taskDir,
authToken: authToken,
metadata: cloneSSHMetadata(req.Metadata),
prepareEnv: sshRemoteContributionEnv(req, agentctlBin),
platform: platform,
runtimeAPITunnel: runtimeAPITunnel,
}
r.mu.Unlock()
released = true // ownership transferred to session state; released on StopInstance
@@ -406,6 +415,9 @@ func (r *SSHExecutor) startAndForwardAgentctl(
req *ExecutorCreateRequest,
platform SSHRemotePlatform,
) (int, int, *SSHPortForwarder, string, error) {
if err := uploadSSHSkillManifest(ctx, client, taskDir, req.Metadata); err != nil {
return 0, 0, nil, "", fmt.Errorf("ssh: upload skill manifest: %w", err)
}
shell := sshShellForRemote(req.Metadata, platform)
controlPort, pid, authToken, err := r.startAgentctlAndHandshake(ctx, client, shell, agentctlBin, taskDir, sessionDir, req)
if err != nil {
@@ -531,6 +543,20 @@ func (r *SSHExecutor) buildInstance(
port, pid int,
workdir, authToken string,
) *ExecutorInstance {
metadata := map[string]interface{}{
MetadataKeySSHHost: target.Host,
MetadataKeySSHPort: strconv.Itoa(target.Port),
MetadataKeySSHUser: target.User,
MetadataKeySSHHostFingerprint: target.PinnedFingerprint,
MetadataKeySSHRemoteTaskDir: taskDir,
MetadataKeySSHRemoteSessionDir: sessionDir,
MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port),
MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(pid),
MetadataKeySSHLocalForwardPort: strconv.Itoa(fwd.LocalPort()),
MetadataKeySSHWorkdirRoot: workdir,
MetadataKeyIsRemote: true,
}
copySSHRuntimeAPIMetadata(metadata, req.Metadata)
return &ExecutorInstance{
InstanceID: req.InstanceID,
TaskID: req.TaskID,
@@ -541,19 +567,7 @@ func (r *SSHExecutor) buildInstance(
agentctl.WithSessionID(req.SessionID), agentctl.WithAuthToken(authToken)),
WorkspacePath: taskDir,
AuthToken: authToken,
Metadata: map[string]interface{}{
MetadataKeySSHHost: target.Host,
MetadataKeySSHPort: strconv.Itoa(target.Port),
MetadataKeySSHUser: target.User,
MetadataKeySSHHostFingerprint: target.PinnedFingerprint,
MetadataKeySSHRemoteTaskDir: taskDir,
MetadataKeySSHRemoteSessionDir: sessionDir,
MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port),
MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(pid),
MetadataKeySSHLocalForwardPort: strconv.Itoa(fwd.LocalPort()),
MetadataKeySSHWorkdirRoot: workdir,
MetadataKeyIsRemote: true,
},
Metadata: metadata,
}
}
@@ -572,6 +586,21 @@ func (r *SSHExecutor) buildResumedInstance(req *ExecutorCreateRequest, state *ss
agentctl.WithExecutionID(resumedSSHAgentctlInstanceID(req)),
agentctl.WithSessionID(req.SessionID), agentctl.WithAuthToken(state.authToken))
}
metadata := map[string]interface{}{
MetadataKeySSHHost: state.target.Host,
MetadataKeySSHPort: strconv.Itoa(state.target.Port),
MetadataKeySSHUser: state.target.User,
MetadataKeySSHHostFingerprint: state.target.PinnedFingerprint,
MetadataKeySSHRemoteTaskDir: taskDir,
MetadataKeySSHRemoteSessionDir: state.remoteDir,
MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port),
MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(state.pid),
MetadataKeySSHLocalForwardPort: strconv.Itoa(state.forwarder.LocalPort()),
MetadataKeySSHWorkdirRoot: workdir,
MetadataKeyIsRemote: true,
"reuse_existing_process": state.reusingProcess,
}
copySSHRuntimeAPIMetadata(metadata, req.Metadata)
return &ExecutorInstance{
InstanceID: req.InstanceID,
TaskID: req.TaskID,
@@ -580,20 +609,18 @@ func (r *SSHExecutor) buildResumedInstance(req *ExecutorCreateRequest, state *ss
Client: client,
WorkspacePath: taskDir,
AuthToken: state.authToken,
Metadata: map[string]interface{}{
MetadataKeySSHHost: state.target.Host,
MetadataKeySSHPort: strconv.Itoa(state.target.Port),
MetadataKeySSHUser: state.target.User,
MetadataKeySSHHostFingerprint: state.target.PinnedFingerprint,
MetadataKeySSHRemoteTaskDir: taskDir,
MetadataKeySSHRemoteSessionDir: state.remoteDir,
MetadataKeySSHRemoteAgentctlPort: strconv.Itoa(port),
MetadataKeySSHRemoteAgentctlPID: strconv.Itoa(state.pid),
MetadataKeySSHLocalForwardPort: strconv.Itoa(state.forwarder.LocalPort()),
MetadataKeySSHWorkdirRoot: workdir,
MetadataKeyIsRemote: true,
"reuse_existing_process": state.reusingProcess,
},
Metadata: metadata,
}
}
func copySSHRuntimeAPIMetadata(dst, src map[string]interface{}) {
for _, key := range []string{
MetadataKeySSHRuntimeAPILocalURL,
MetadataKeySSHRuntimeAPIRemotePort,
} {
if value, ok := src[key]; ok {
dst[key] = value
}
}
}
@@ -617,6 +644,7 @@ func (r *SSHExecutor) StopInstance(ctx context.Context, instance *ExecutorInstan
zap.String("instance_id", instance.InstanceID))
return nil
}
_ = state.runtimeAPITunnel.Close()
if state.forwarder != nil {
_ = state.forwarder.Close()
}
@@ -744,14 +772,21 @@ func (r *SSHExecutor) ResumeRemoteInstance(ctx context.Context, req *ExecutorCre
clearSSHResumeRuntimeMetadata(req.Metadata)
return nil
}
runtimeAPITunnel, _, err := openSSHRuntimeAPITunnelForRequest(client, req)
if err != nil {
_ = client.Close()
return err
}
remotePort, _ := strconv.Atoi(portStr)
fwd, err := StartPortForward(client, remotePort, r.logger)
if err != nil {
_ = runtimeAPITunnel.Close()
_ = client.Close()
return fmt.Errorf("ssh resume: port forward: %w", err)
}
if err := waitAgentctlHealthy(ctx, fwd.LocalPort(), sshAgentctlHealthTimeout); err != nil {
_ = runtimeAPITunnel.Close()
_ = fwd.Close()
_ = client.Close()
return fmt.Errorf("ssh resume: agentctl health: %w", err)
@@ -762,17 +797,18 @@ func (r *SSHExecutor) ResumeRemoteInstance(ctx context.Context, req *ExecutorCre
r.mu.Lock()
r.sessions[req.InstanceID] = &sshSessionState{
target: target,
client: client,
forwarder: fwd,
agentctlClient: instanceClient,
pid: pid,
remoteDir: sessionDir,
remoteTaskDir: taskDir,
authToken: req.AuthToken,
reusingProcess: reusingProcess,
metadata: cloneSSHMetadata(req.Metadata),
prepareEnv: sshRemoteContributionEnv(req, agentctlBin),
target: target,
client: client,
forwarder: fwd,
agentctlClient: instanceClient,
pid: pid,
remoteDir: sessionDir,
remoteTaskDir: taskDir,
authToken: req.AuthToken,
reusingProcess: reusingProcess,
metadata: cloneSSHMetadata(req.Metadata),
prepareEnv: sshRemoteContributionEnv(req, agentctlBin),
runtimeAPITunnel: runtimeAPITunnel,
}
r.mu.Unlock()
@@ -896,6 +932,7 @@ func clearSSHResumeRuntimeMetadata(metadata map[string]interface{}) {
MetadataKeySSHRemoteAgentctlPID,
MetadataKeySSHLocalForwardPort,
MetadataKeySSHRemoteAgentctlURL,
MetadataKeySSHRuntimeAPIRemotePort,
} {
delete(metadata, key)
}
@@ -249,6 +249,9 @@ func TestBuildSSHCreateInstanceRequestMapsEveryField(t *testing.T) {
if got.Env[envKeyOpenAIAPIKey] != "sk-1" {
t.Fatalf("Env = %+v, want the allowlisted key", got.Env)
}
if got.Env[envKeyKandevCLI] != "/remote/agentctl" {
t.Fatalf("KANDEV_CLI = %q, want the remote agentctl path", got.Env[envKeyKandevCLI])
}
if _, leaked := got.Env["UNRELATED"]; leaked {
t.Fatalf("Env leaked a non-allowlisted key: %+v", got.Env)
}
@@ -300,6 +303,23 @@ func TestSSHRemoteAgentEnvForwardsOnlyScopedCredentials(t *testing.T) {
}
})
t.Run("signed runtime contract is forwarded", func(t *testing.T) {
env := sshRemoteAgentEnv(&ExecutorCreateRequest{Env: map[string]string{
envKeyKandevAPIURL: "http://127.0.0.1:38429/api/v1",
envKeyKandevAPIKey: "signed-key",
envKeyKandevRunToken: "signed-token",
envKeyKandevAgentID: "agent-1",
envKeyKandevWorkspaceID: "workspace-1",
envKeyKandevRunID: "run-1",
envKeyKandevTaskID: "task-1",
envKeyKandevCLI: "/host/agentctl",
}})
if env[envKeyKandevAPIURL] == "" || env[envKeyKandevAPIKey] != "signed-key" ||
env[envKeyKandevRunToken] != "signed-token" || env[envKeyKandevTaskID] != "task-1" {
t.Fatalf("runtime contract = %+v", env)
}
})
t.Run("managed broker env and indexed git config ride along", func(t *testing.T) {
env := sshRemoteAgentEnv(&ExecutorCreateRequest{Env: map[string]string{
githubauth.CredentialBrokerURLEnv: "http://127.0.0.1:9/broker",
@@ -8,6 +8,7 @@ import (
"io"
"io/fs"
"os"
"path"
"regexp"
"strings"
@@ -386,6 +387,47 @@ func (u *sshFileUploader) WriteFile(_ context.Context, path string, data []byte,
return nil
}
func (u *sshFileUploader) RemoveAll(ctx context.Context, target string) error {
c, err := newSFTPClientContext(ctx, u.client)
if err != nil {
return fmt.Errorf("sftp: new client: %w", err)
}
defer func() { _ = c.Close() }()
if err := removeSFTPPath(ctx, c, target); err != nil && !isSFTPNotExist(err) && !errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("sftp: remove %s: %w", target, err)
}
return nil
}
func removeSFTPPath(ctx context.Context, client *sftp.Client, target string) error {
if err := ctx.Err(); err != nil {
return err
}
info, err := client.Lstat(target)
if err != nil {
return err
}
if info.Mode()&os.ModeSymlink != 0 {
return client.Remove(target)
}
if !info.IsDir() {
return client.Remove(target)
}
entries, err := client.ReadDirContext(ctx, target)
if err != nil {
return err
}
for _, entry := range entries {
if err := removeSFTPPath(ctx, client, path.Join(target, entry.Name())); err != nil {
if isSFTPNotExist(err) || errors.Is(err, os.ErrNotExist) {
continue
}
return err
}
}
return client.RemoveDirectory(target)
}
// sshMkdirAll mimics `mkdir -p` over SFTP. Walks every prefix of dir and
// creates segments that don't exist; treats "already exists" as success.
func sshMkdirAll(c *sftp.Client, dir string) error {
@@ -929,6 +929,18 @@ const (
envKeyKandevGitLabHost = "KANDEV_GITLAB_HOST"
envKeyMCPTimeout = "MCP_TIMEOUT"
envKeyMCPToolTimeout = "MCP_TOOL_TIMEOUT"
envKeyKandevAPIURL = "KANDEV_API_URL"
envKeyKandevAPIKey = "KANDEV_API_KEY"
envKeyKandevRunToken = "KANDEV_RUN_TOKEN"
envKeyKandevCLI = "KANDEV_CLI"
envKeyKandevAgentID = "KANDEV_AGENT_ID"
envKeyKandevAgentName = "KANDEV_AGENT_NAME"
envKeyKandevWorkspaceID = "KANDEV_WORKSPACE_ID"
envKeyKandevRunID = "KANDEV_RUN_ID"
envKeyKandevTaskID = "KANDEV_TASK_ID"
envKeyKandevWakeReason = "KANDEV_WAKE_REASON"
envKeyKandevWakeCommentID = "KANDEV_WAKE_COMMENT_ID"
envKeyKandevWakePayload = "KANDEV_WAKE_PAYLOAD_JSON"
)
var sshRemoteAgentCredentialEnvKeys = []string{
@@ -944,11 +956,23 @@ var sshRemoteAgentCredentialEnvKeys = []string{
envKeyKandevGitLabHost,
}
// sshRemoteAgentRuntimeEnvKeys are non-secret runtime controls that must reach
// the remote agent process after profile and agent precedence has been resolved.
// sshRemoteAgentRuntimeEnvKeys are resolved runtime contract values that must
// reach the remote agent process after profile and agent precedence is applied.
var sshRemoteAgentRuntimeEnvKeys = []string{
envKeyMCPTimeout,
envKeyMCPToolTimeout,
envKeyKandevAPIURL,
envKeyKandevAPIKey,
envKeyKandevRunToken,
envKeyKandevCLI,
envKeyKandevAgentID,
envKeyKandevAgentName,
envKeyKandevWorkspaceID,
envKeyKandevRunID,
envKeyKandevTaskID,
envKeyKandevWakeReason,
envKeyKandevWakeCommentID,
envKeyKandevWakePayload,
}
// sshRemoteAgentEnv builds the env map sent to the remote agent instance. Each
@@ -66,6 +66,12 @@ func sshRemoteContributionEnv(req *ExecutorCreateRequest, agentctlBin string) ma
if req == nil {
return env
}
if agentctlBin != "" {
env[envKeyKandevCLI] = agentctlBin
}
if agentctlBin != "" {
env["KANDEV_CLI"] = agentctlBin
}
count := sshRemoteGitConfigCount(env)
managedBroker := hasManagedGitHubBrokerEnv(req.Env)
foundManagedHelper := rewriteSSHManagedGitCredentialHelpers(env, count, managedBroker, agentctlBin)
@@ -32,8 +32,10 @@ func TestSSHBuildResumedInstanceRecordsProcessReuse(t *testing.T) {
TaskID: "task-1",
SessionID: "session-1",
Metadata: map[string]interface{}{
MetadataKeySSHRemoteAgentctlPort: "43123",
MetadataKeySSHRemoteTaskDir: "/remote/task",
MetadataKeySSHRemoteAgentctlPort: "43123",
MetadataKeySSHRemoteTaskDir: "/remote/task",
MetadataKeySSHRuntimeAPILocalURL: "http://127.0.0.1:38429/api/v1",
MetadataKeySSHRuntimeAPIRemotePort: "41234",
},
}, &sshSessionState{
target: &SSHTarget{Host: "ssh.example", Port: 22, User: "agent"},
@@ -48,10 +50,36 @@ func TestSSHBuildResumedInstanceRecordsProcessReuse(t *testing.T) {
if !ok || reusingProcess != tc.reusingProcess {
t.Fatalf("reuse_existing_process = (%v, %v), want (%v, true)", reusingProcess, ok, tc.reusingProcess)
}
if got := instance.Metadata[MetadataKeySSHRuntimeAPILocalURL]; got != "http://127.0.0.1:38429/api/v1" {
t.Fatalf("local API URL metadata = %v", got)
}
if got := instance.Metadata[MetadataKeySSHRuntimeAPIRemotePort]; got != "41234" {
t.Fatalf("remote API port metadata = %v", got)
}
})
}
}
func TestSSHBuildInstanceCarriesRuntimeAPITunnelMetadata(t *testing.T) {
executor := &SSHExecutor{logger: newNopLogger(t)}
instance := executor.buildInstance(
&ExecutorCreateRequest{InstanceID: "instance-1", Metadata: map[string]interface{}{
MetadataKeySSHRuntimeAPILocalURL: "http://127.0.0.1:38429/api/v1",
MetadataKeySSHRuntimeAPIRemotePort: "41234",
}},
&SSHTarget{Host: "ssh.example", Port: 22, User: "agent"},
&SSHPortForwarder{localPort: 43124},
"/remote/task", "/remote/session", 43123, 99, "/home/agent/.kandev", "token",
)
if got := instance.Metadata[MetadataKeySSHRuntimeAPILocalURL]; got != "http://127.0.0.1:38429/api/v1" {
t.Fatalf("local API URL metadata = %v", got)
}
if got := instance.Metadata[MetadataKeySSHRuntimeAPIRemotePort]; got != "41234" {
t.Fatalf("remote API port metadata = %v", got)
}
}
func TestSSHResumedAgentctlClientChecksExistingProcess(t *testing.T) {
for _, tc := range []struct {
name string
@@ -0,0 +1,195 @@
package lifecycle
import (
"fmt"
"io"
"net"
"net/url"
"strconv"
"strings"
"sync"
"time"
"golang.org/x/crypto/ssh"
)
const sshRuntimeAPIDialTimeout = 10 * time.Second
const urlSchemeHTTP = "http"
// sshRuntimeAPITunnel exposes a control-plane loopback API through the SSH
// connection. The remote agent can then use the same API URL shape as a local
// launch without assuming that the SSH host can route to the developer's
// machine.
type sshRuntimeAPITunnel struct {
listener net.Listener
localTarget string
remotePort string
once sync.Once
}
func openSSHRuntimeAPITunnelForRequest(
client *ssh.Client,
req *ExecutorCreateRequest,
) (*sshRuntimeAPITunnel, string, error) {
if req == nil {
return nil, "", fmt.Errorf("ssh runtime API tunnel: request is required")
}
rawURL := getMetadataString(req.Metadata, MetadataKeySSHRuntimeAPILocalURL)
if rawURL == "" && req.Env != nil {
rawURL = req.Env[envKeyKandevAPIURL]
}
requestedPort := getMetadataString(req.Metadata, MetadataKeySSHRuntimeAPIRemotePort)
tunnel, runtimeURL, err := openSSHRuntimeAPITunnel(client, rawURL, requestedPort)
if err != nil || tunnel == nil {
return tunnel, runtimeURL, err
}
if req.Env == nil {
req.Env = make(map[string]string)
}
req.Env[envKeyKandevAPIURL] = runtimeURL
if req.Metadata == nil {
req.Metadata = make(map[string]interface{})
}
req.Metadata[MetadataKeySSHRuntimeAPILocalURL] = rawURL
req.Metadata[MetadataKeySSHRuntimeAPIRemotePort] = tunnel.RemotePort()
return tunnel, runtimeURL, nil
}
func openSSHRuntimeAPITunnel(client *ssh.Client, rawURL string, requestedRemotePort ...string) (*sshRuntimeAPITunnel, string, error) {
parsed, err := url.Parse(strings.TrimSpace(rawURL))
if err != nil {
return nil, rawURL, fmt.Errorf("ssh runtime API URL: %w", err)
}
if !isLoopbackURL(parsed) {
return nil, rawURL, nil
}
if client == nil {
return nil, rawURL, fmt.Errorf("ssh runtime API tunnel: SSH client is required")
}
localPort := parsed.Port()
if localPort == "" {
localPort = defaultURLPort(parsed.Scheme)
}
if localPort == "" {
return nil, rawURL, fmt.Errorf("ssh runtime API tunnel: URL must include a port")
}
remotePort := "0"
if len(requestedRemotePort) > 0 && strings.TrimSpace(requestedRemotePort[0]) != "" {
remotePort = strings.TrimSpace(requestedRemotePort[0])
if _, err := parseSSHRuntimeAPIPort(remotePort, true); err != nil {
return nil, rawURL, err
}
}
remoteAddress := net.JoinHostPort("127.0.0.1", remotePort)
listener, err := client.Listen("tcp", remoteAddress)
if err != nil {
return nil, rawURL, fmt.Errorf("ssh runtime API tunnel: listen on remote %s: %w", remoteAddress, err)
}
actualRemotePort, err := sshRuntimeAPIPortFromAddr(listener.Addr())
if err != nil {
_ = listener.Close()
return nil, rawURL, err
}
tunnel := &sshRuntimeAPITunnel{
listener: listener,
localTarget: net.JoinHostPort(parsed.Hostname(), localPort),
remotePort: actualRemotePort,
}
go tunnel.acceptLoop()
parsed.Host = net.JoinHostPort("127.0.0.1", actualRemotePort)
return tunnel, parsed.String(), nil
}
func parseSSHRuntimeAPIPort(raw string, allowZero bool) (int, error) {
port, err := strconv.Atoi(raw)
if err != nil || port < 0 || port > 65535 || (!allowZero && port == 0) {
return 0, fmt.Errorf("ssh runtime API tunnel: invalid remote port %q", raw)
}
return port, nil
}
func sshRuntimeAPIPortFromAddr(addr net.Addr) (string, error) {
if addr == nil {
return "", fmt.Errorf("ssh runtime API tunnel: listener has no address")
}
_, port, err := net.SplitHostPort(addr.String())
if err != nil {
return "", fmt.Errorf("ssh runtime API tunnel: parse listener address %q: %w", addr.String(), err)
}
if _, err := parseSSHRuntimeAPIPort(port, false); err != nil {
return "", err
}
return port, nil
}
func isLoopbackURL(parsed *url.URL) bool {
if parsed == nil || parsed.Hostname() == "" {
return false
}
host := strings.ToLower(parsed.Hostname())
if host == "localhost" || host == "::1" {
return true
}
ip := net.ParseIP(host)
return ip != nil && ip.IsLoopback()
}
func defaultURLPort(scheme string) string {
switch strings.ToLower(scheme) {
case urlSchemeHTTP:
return "80"
case "https":
return "443"
default:
return ""
}
}
func (t *sshRuntimeAPITunnel) acceptLoop() {
for {
remote, err := t.listener.Accept()
if err != nil {
return
}
go t.proxy(remote)
}
}
func (t *sshRuntimeAPITunnel) proxy(remote net.Conn) {
local, err := net.DialTimeout("tcp", t.localTarget, sshRuntimeAPIDialTimeout)
if err != nil {
_ = remote.Close()
return
}
done := make(chan struct{}, 2)
go func() {
_, _ = io.Copy(local, remote)
done <- struct{}{}
}()
go func() {
_, _ = io.Copy(remote, local)
done <- struct{}{}
}()
<-done
_ = local.Close()
_ = remote.Close()
<-done
}
func (t *sshRuntimeAPITunnel) RemotePort() string {
if t == nil {
return ""
}
return t.remotePort
}
func (t *sshRuntimeAPITunnel) Close() error {
if t == nil {
return nil
}
var err error
t.once.Do(func() { err = t.listener.Close() })
return err
}
@@ -0,0 +1,160 @@
package lifecycle
import (
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
"strconv"
"sync"
"testing"
)
func TestOpenSSHRuntimeAPITunnel_LeavesReachableURLsUnchanged(t *testing.T) {
const rawURL = "https://kandev.example.test/api/v1"
tunnel, gotURL, err := openSSHRuntimeAPITunnel(nil, rawURL)
if err != nil {
t.Fatalf("openSSHRuntimeAPITunnel: %v", err)
}
if tunnel != nil {
t.Fatal("reachable URL unexpectedly created a tunnel")
}
if gotURL != rawURL {
t.Fatalf("URL = %q, want %q", gotURL, rawURL)
}
}
func TestOpenSSHRuntimeAPITunnel_RequiresSSHClientForLoopbackURL(t *testing.T) {
_, _, err := openSSHRuntimeAPITunnel(nil, "http://127.0.0.1:38429/api/v1")
if err == nil {
t.Fatal("loopback URL without SSH client unexpectedly succeeded")
}
}
func TestOpenSSHRuntimeAPITunnel_ConcurrentSameHostSessionsGetUniqueRemotePorts(t *testing.T) {
backendListener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("reserve backend port: %v", err)
}
backendPort := backendListener.Addr().(*net.TCPAddr).Port
_ = backendListener.Close()
rawURL := "http://127.0.0.1:" + strconv.Itoa(backendPort) + "/api/v1"
server := newFakeSSHServer(t, nil)
client := server.dial(t)
start := make(chan struct{})
type result struct {
tunnel *sshRuntimeAPITunnel
url string
err error
}
results := make(chan result, 2)
var wg sync.WaitGroup
for range 2 {
wg.Add(1)
go func() {
defer wg.Done()
<-start
tunnel, rewritten, openErr := openSSHRuntimeAPITunnel(client, rawURL)
results <- result{tunnel: tunnel, url: rewritten, err: openErr}
}()
}
close(start)
wg.Wait()
close(results)
seenPorts := make(map[string]struct{}, 2)
for outcome := range results {
if outcome.err != nil {
t.Fatalf("concurrent tunnel open failed: %v", outcome.err)
}
if outcome.tunnel == nil {
t.Fatal("loopback URL did not create a tunnel")
}
parsed, parseErr := url.Parse(outcome.url)
if parseErr != nil {
t.Fatalf("parse rewritten URL %q: %v", outcome.url, parseErr)
}
seenPorts[parsed.Port()] = struct{}{}
if parsed.Port() == strconv.Itoa(backendPort) {
t.Fatalf("rewritten URL still uses backend port %d: %q", backendPort, outcome.url)
}
_ = outcome.tunnel.Close()
}
if len(seenPorts) != 2 {
t.Fatalf("concurrent tunnels used %d remote ports, want 2", len(seenPorts))
}
}
func TestOpenSSHRuntimeAPITunnel_ResumeRebindsPersistedRemotePort(t *testing.T) {
server := newFakeSSHServer(t, nil)
client := server.dial(t)
const rawURL = "http://127.0.0.1:38429/api/v1"
firstReq := &ExecutorCreateRequest{Env: map[string]string{envKeyKandevAPIURL: rawURL}}
firstTunnel, firstURL, err := openSSHRuntimeAPITunnelForRequest(client, firstReq)
if err != nil {
t.Fatalf("first tunnel: %v", err)
}
firstPort := firstTunnel.RemotePort()
if firstReq.Metadata[MetadataKeySSHRuntimeAPILocalURL] != rawURL {
t.Fatalf("local URL metadata = %v, want %q", firstReq.Metadata[MetadataKeySSHRuntimeAPILocalURL], rawURL)
}
if firstReq.Metadata[MetadataKeySSHRuntimeAPIRemotePort] != firstPort {
t.Fatalf("remote port metadata = %v, want %q", firstReq.Metadata[MetadataKeySSHRuntimeAPIRemotePort], firstPort)
}
if err := firstTunnel.Close(); err != nil {
t.Fatalf("close first tunnel: %v", err)
}
resumedReq := &ExecutorCreateRequest{
Env: map[string]string{envKeyKandevAPIURL: firstURL},
Metadata: cloneSSHMetadata(firstReq.Metadata),
}
secondTunnel, secondURL, err := openSSHRuntimeAPITunnelForRequest(client, resumedReq)
if err != nil {
t.Fatalf("resume tunnel: %v", err)
}
defer func() { _ = secondTunnel.Close() }()
if secondTunnel.RemotePort() != firstPort {
t.Fatalf("resume remote port = %q, want persisted %q", secondTunnel.RemotePort(), firstPort)
}
if secondURL != firstURL {
t.Fatalf("resume URL = %q, want the original rewritten URL %q", secondURL, firstURL)
}
if resumedReq.Env[envKeyKandevAPIURL] != secondURL {
t.Fatalf("resume API URL = %q, want %q", resumedReq.Env[envKeyKandevAPIURL], secondURL)
}
}
func TestOpenSSHRuntimeAPITunnel_ProxiesRoundTripAndTearsDown(t *testing.T) {
localServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Connection", "close")
_, _ = io.WriteString(w, "runtime-api-round-trip")
}))
t.Cleanup(localServer.Close)
server := newFakeSSHServer(t, nil)
client := server.dial(t)
tunnel, rewrittenURL, err := openSSHRuntimeAPITunnel(client, localServer.URL)
if err != nil {
t.Fatalf("open tunnel: %v", err)
}
t.Cleanup(func() { _ = tunnel.Close() })
requestClient := &http.Client{Transport: &http.Transport{DisableKeepAlives: true}}
response, err := requestClient.Get(rewrittenURL)
if err != nil {
t.Fatalf("GET through SSH runtime API tunnel: %v", err)
}
t.Cleanup(func() { _ = response.Body.Close() })
body, err := io.ReadAll(response.Body)
if err != nil {
t.Fatalf("read tunneled response: %v", err)
}
if string(body) != "runtime-api-round-trip" {
t.Fatalf("tunneled response = %q", body)
}
}
@@ -0,0 +1,124 @@
package lifecycle
import (
"context"
"encoding/json"
"fmt"
"os"
"path"
"strings"
"golang.org/x/crypto/ssh"
"github.com/kandev/kandev/internal/agent/runtime/lifecycle/skill"
)
type sshSkillWorkspace interface {
WriteFile(context.Context, string, []byte, os.FileMode) error
RemoveAll(context.Context, string) error
}
// uploadSSHSkillManifest materializes launch-scoped skills after the remote
// task workspace exists and before the remote agent starts. The lifecycle
// deployer cannot write the host worktree because SSH agents run elsewhere.
func uploadSSHSkillManifest(
ctx context.Context,
client *ssh.Client,
taskDir string,
metadata map[string]interface{},
) error {
raw := getMetadataString(metadata, MetadataKeySkillManifestJSON)
if raw == "" {
return nil
}
return materializeSSHSkillManifest(ctx, &sshFileUploader{client: client}, taskDir, raw)
}
func materializeSSHSkillManifest(
ctx context.Context,
workspace sshSkillWorkspace,
taskDir string,
raw string,
) error {
if workspace == nil {
return fmt.Errorf("SSH skill workspace is required")
}
taskRoot := path.Clean(strings.TrimSpace(taskDir))
if taskRoot == "." || !path.IsAbs(taskRoot) {
return fmt.Errorf("SSH task directory must be absolute")
}
var manifest skill.Manifest
if err := json.Unmarshal([]byte(raw), &manifest); err != nil {
return fmt.Errorf("unmarshal SSH skill manifest: %w", err)
}
projectDir := manifest.ProjectSkillDir
if projectDir == "" {
projectDir = skill.DefaultProjectSkillDir
}
projectDir, ok := sshSafeRelativePath(projectDir)
if !ok {
return fmt.Errorf("SSH skill manifest project directory is unsafe")
}
projectRoot := path.Join(taskRoot, projectDir)
decisionRoot := path.Join(projectRoot, skill.DirName(skill.ReservedDecisionSkillSlug))
if err := workspace.RemoveAll(ctx, decisionRoot); err != nil {
return fmt.Errorf("remove stale SSH decision skill: %w", err)
}
return materializeSSHManifestSkills(ctx, workspace, projectRoot, manifest.Skills)
}
func materializeSSHManifestSkills(
ctx context.Context,
workspace sshSkillWorkspace,
projectRoot string,
skills []skill.Skill,
) error {
claimed := make(map[string]string, len(skills))
for _, item := range skills {
if !validSlugRe.MatchString(item.Slug) {
continue
}
dirName := skill.DirName(item.Slug)
if _, exists := claimed[dirName]; exists {
continue
}
claimed[dirName] = item.Slug
if err := writeSSHManifestSkill(ctx, workspace, projectRoot, item); err != nil {
return err
}
}
return nil
}
func writeSSHManifestSkill(
ctx context.Context,
workspace sshSkillWorkspace,
projectRoot string,
item skill.Skill,
) error {
root := path.Join(projectRoot, skill.DirName(item.Slug))
if err := workspace.WriteFile(ctx, path.Join(root, "SKILL.md"), []byte(item.Content), 0o644); err != nil {
return fmt.Errorf("write SSH skill %q: %w", item.Slug, err)
}
for _, support := range item.Files {
relative, safe := sshSafeRelativePath(support.Path)
if !safe || relative == "SKILL.md" {
continue
}
if err := workspace.WriteFile(ctx, path.Join(root, relative), []byte(support.Content), 0o644); err != nil {
return fmt.Errorf("write SSH skill support file %q: %w", support.Path, err)
}
}
return nil
}
func sshSafeRelativePath(value string) (string, bool) {
if value == "" || strings.Contains(value, "\\") || strings.ContainsRune(value, '\x00') || path.IsAbs(value) {
return "", false
}
cleaned := path.Clean(value)
if cleaned == "." || cleaned == ".." || strings.HasPrefix(cleaned, "../") {
return "", false
}
return cleaned, true
}
@@ -0,0 +1,109 @@
package lifecycle
import (
"context"
"encoding/json"
"os"
"path/filepath"
"testing"
"github.com/kandev/kandev/internal/agent/runtime/lifecycle/skill"
)
func TestUploadSSHSkillManifest_WritesDecisionSkillToRemoteWorkspace(t *testing.T) {
remoteWorkspace := t.TempDir()
server := newFakeSSHServer(t, nil)
server.enableSFTP()
manifest, err := json.Marshal(skill.Manifest{
ProjectSkillDir: ".claude/skills",
Skills: []skill.Skill{{
Slug: skill.ReservedDecisionSkillSlug,
Content: "---\nname: kandev-step-decision\ndescription: decision\n---\n# decision",
}},
})
if err != nil {
t.Fatalf("marshal manifest: %v", err)
}
if err := uploadSSHSkillManifest(context.Background(), server.dial(t), remoteWorkspace,
map[string]interface{}{MetadataKeySkillManifestJSON: string(manifest)}); err != nil {
t.Fatalf("uploadSSHSkillManifest: %v", err)
}
path := filepath.Join(remoteWorkspace, ".claude", "skills", "kandev-step-decision", "SKILL.md")
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read remote decision skill: %v", err)
}
if string(data) != "---\nname: kandev-step-decision\ndescription: decision\n---\n# decision" {
t.Fatalf("remote decision skill = %q", string(data))
}
}
func TestUploadSSHSkillManifest_NoSeatRemovesStaleDecisionSkill(t *testing.T) {
remoteWorkspace := t.TempDir()
server := newFakeSSHServer(t, nil)
server.enableSFTP()
client := server.dial(t)
seatManifest, err := json.Marshal(skill.Manifest{
ProjectSkillDir: ".claude/skills",
Skills: []skill.Skill{{Slug: skill.ReservedDecisionSkillSlug, Content: "# decision"}},
})
if err != nil {
t.Fatalf("marshal seat manifest: %v", err)
}
metadata := map[string]interface{}{MetadataKeySkillManifestJSON: string(seatManifest)}
if err := uploadSSHSkillManifest(context.Background(), client, remoteWorkspace, metadata); err != nil {
t.Fatalf("seat uploadSSHSkillManifest: %v", err)
}
decisionPath := filepath.Join(remoteWorkspace, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(decisionPath); err != nil {
t.Fatalf("seat decision skill missing: %v", err)
}
noSeatManifest, err := json.Marshal(skill.Manifest{ProjectSkillDir: ".claude/skills"})
if err != nil {
t.Fatalf("marshal no-seat manifest: %v", err)
}
metadata[MetadataKeySkillManifestJSON] = string(noSeatManifest)
if err := uploadSSHSkillManifest(context.Background(), client, remoteWorkspace, metadata); err != nil {
t.Fatalf("no-seat uploadSSHSkillManifest: %v", err)
}
if _, err := os.Stat(decisionPath); !os.IsNotExist(err) {
t.Fatalf("no-seat upload left stale decision skill at %s", decisionPath)
}
}
func TestUploadSSHSkillManifest_NoSeatDoesNotFollowDecisionSkillSymlink(t *testing.T) {
remoteWorkspace := t.TempDir()
outsideWorkspace := t.TempDir()
marker := filepath.Join(outsideWorkspace, "keep.txt")
if err := os.WriteFile(marker, []byte("must survive"), 0o600); err != nil {
t.Fatalf("seed outside marker: %v", err)
}
decisionRoot := filepath.Join(remoteWorkspace, ".claude", "skills", skill.ReservedDecisionSkillSlug)
if err := os.MkdirAll(filepath.Dir(decisionRoot), 0o755); err != nil {
t.Fatalf("create decision skill parent: %v", err)
}
if err := os.Symlink(outsideWorkspace, decisionRoot); err != nil {
t.Skipf("symlinks are unavailable: %v", err)
}
server := newFakeSSHServer(t, nil)
server.enableSFTP()
manifest, err := json.Marshal(skill.Manifest{ProjectSkillDir: ".claude/skills"})
if err != nil {
t.Fatalf("marshal no-seat manifest: %v", err)
}
if err := uploadSSHSkillManifest(context.Background(), server.dial(t), remoteWorkspace,
map[string]interface{}{MetadataKeySkillManifestJSON: string(manifest)}); err != nil {
t.Fatalf("uploadSSHSkillManifest: %v", err)
}
if _, err := os.Stat(marker); err != nil {
t.Fatalf("outside marker was removed: %v", err)
}
if _, err := os.Lstat(decisionRoot); !os.IsNotExist(err) {
t.Fatalf("decision symlink still exists or returned unexpected error: %v", err)
}
}
@@ -522,21 +522,26 @@ func TestSSHTaskDirName(t *testing.T) {
func TestClearSSHResumeRuntimeMetadata(t *testing.T) {
metadata := map[string]interface{}{
MetadataKeySSHRemoteSessionDir: "/remote/session",
MetadataKeySSHRemoteAgentctlPort: "41234",
MetadataKeySSHRemoteAgentctlPID: "4242",
MetadataKeySSHLocalForwardPort: "5000",
MetadataKeySSHRemoteAgentctlURL: "http://127.0.0.1:5000",
MetadataKeySSHRemoteTaskDir: "/remote/task",
MetadataKeySSHHost: "build.example",
MetadataKeySSHRemoteSessionDir: "/remote/session",
MetadataKeySSHRemoteAgentctlPort: "41234",
MetadataKeySSHRemoteAgentctlPID: "4242",
MetadataKeySSHLocalForwardPort: "5000",
MetadataKeySSHRemoteAgentctlURL: "http://127.0.0.1:5000",
MetadataKeySSHRuntimeAPILocalURL: "http://127.0.0.1:3456/api/v1",
MetadataKeySSHRuntimeAPIRemotePort: "45678",
MetadataKeySSHRemoteTaskDir: "/remote/task",
MetadataKeySSHHost: "build.example",
}
clearSSHResumeRuntimeMetadata(metadata)
if len(metadata) != 2 {
t.Fatalf("remaining metadata = %+v, want only the durable keys", metadata)
if len(metadata) != 3 {
t.Fatalf("remaining metadata = %+v, want durable keys plus the local API URL", metadata)
}
if metadata[MetadataKeySSHRemoteTaskDir] != "/remote/task" || metadata[MetadataKeySSHHost] != "build.example" {
t.Fatalf("durable metadata was cleared: %+v", metadata)
}
if metadata[MetadataKeySSHRuntimeAPILocalURL] != "http://127.0.0.1:3456/api/v1" {
t.Fatalf("local API URL was cleared: %+v", metadata)
}
clearSSHResumeRuntimeMetadata(nil) // must not panic
}
@@ -23,7 +23,9 @@ func (d *Deployer) deliver(_ context.Context, manifest *Manifest, executorType,
return d.deliverSprites(manifest)
case "k8s":
return d.deliverKubernetes(manifest)
case "", "local", "local_pc", "worktree", "local_docker", "remote_docker", "ssh", "mock_remote":
case "ssh":
return d.deliverSSH(manifest)
case "", "local", "local_pc", "worktree", "local_docker", "remote_docker", "mock_remote":
// local_pc and local_docker share the same delivery: the
// worktree IS the agent's CWD inside the executor (Docker
// bind-mounts it; local_pc runs the agent in it directly), so
@@ -68,6 +70,16 @@ func (d *Deployer) deliverKubernetes(manifest *Manifest) DeployResult {
return d.deliverRemote(manifest, dir, "k8s")
}
// deliverSSH keeps the manifest in launch metadata. The SSH executor uploads
// its files after the remote task workspace and agentctl instance exist; the
// host-side worktree is not the SSH agent's workspace.
func (d *Deployer) deliverSSH(manifest *Manifest) DeployResult {
result := d.deliverRemote(manifest, "", "ssh")
result.InstructionsDir = instructionsDirHost(d.basePath, manifest.WorkspaceSlug, manifest.AgentID)
d.writeInstructionFiles(manifest, result.InstructionsDir)
return result
}
func (d *Deployer) deliverRemote(manifest *Manifest, dir, executorType string) DeployResult {
rewriteManifestRefs(manifest, dir)
normalizeManifestSkills(manifest)
@@ -80,11 +80,12 @@ func New(cfg Config) (*Deployer, error) {
// lifecycle.SkillDeployRequest but is decoupled from the lifecycle
// package to keep the import boundary one-way.
type Request struct {
Profile *settingsmodels.AgentProfile
WorkspacePath string
ExecutorType string
WorkspaceID string
SessionID string
Profile *settingsmodels.AgentProfile
WorkspacePath string
ExecutorType string
WorkspaceID string
SessionID string
AdditionalSkillSlugs []string
// OfficeRuntime reports whether backend selected Office mode and the
// finalized launch env contains a non-empty KANDEV_CLI. Office launch
// validation checks the remaining runtime variables before this hook runs.
@@ -100,7 +101,7 @@ func (d *Deployer) Deploy(ctx context.Context, req Request) (DeployResult, error
if req.Profile == nil {
return DeployResult{}, errors.New("skill deploy: profile is required")
}
manifest := d.buildManifest(ctx, req.Profile, d.workspaceSlugFn(req.WorkspaceID), req.OfficeRuntime)
manifest := d.buildManifest(ctx, req.Profile, d.workspaceSlugFn(req.WorkspaceID), req.AdditionalSkillSlugs, req.OfficeRuntime)
result := d.deliver(ctx, manifest, req.ExecutorType, req.WorkspacePath)
return result, nil
}
@@ -55,9 +55,10 @@ func TestDeploy_OfficeRuntimeGatesSystemSkills(t *testing.T) {
ID: "routine-agent",
SkillIDs: string(skillIDsJSON),
},
ExecutorType: "worktree",
WorkspacePath: worktree,
OfficeRuntime: officeRuntime,
ExecutorType: "worktree",
WorkspacePath: worktree,
AdditionalSkillSlugs: []string{skill.ReservedDecisionSkillSlug},
OfficeRuntime: officeRuntime,
}); err != nil {
t.Fatalf("Deploy: %v", err)
}
@@ -344,6 +344,122 @@ func TestDeploy_KanbanProfile_NoSkillsNoOp(t *testing.T) {
}
}
func TestDeploy_AdditionalSkillSlugMaterializesForEmptyProfile(t *testing.T) {
base := t.TempDir()
worktree := t.TempDir()
reader := &fakeSkillReader{skills: map[string]*skill.Skill{
"kandev-step-decision": {Slug: "kandev-step-decision", Content: "# decision"},
}}
d := newDeployer(t, base, reader, &fakeInstructionLister{})
_, err := d.Deploy(context.Background(), skill.Request{
Profile: &settingsmodels.AgentProfile{
ID: "office-p1",
AgentID: "claude-acp",
},
AdditionalSkillSlugs: []string{"kandev-step-decision"},
ExecutorType: "local_pc",
WorkspacePath: worktree,
})
if err != nil {
t.Fatalf("Deploy: %v", err)
}
skillPath := filepath.Join(worktree, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(skillPath); err != nil {
t.Fatalf("additional decision skill was not materialized: %v", err)
}
}
func TestDeploy_SSHReturnsManifestWithoutWritingControlPlaneWorktree(t *testing.T) {
base := t.TempDir()
worktree := t.TempDir()
reader := &fakeSkillReader{skills: map[string]*skill.Skill{
"kandev-step-decision": {Slug: "kandev-step-decision", Content: "# decision"},
}}
d := newDeployer(t, base, reader, &fakeInstructionLister{})
res, err := d.Deploy(context.Background(), skill.Request{
Profile: &settingsmodels.AgentProfile{ID: "ssh-p1", AgentID: "claude-acp"},
ExecutorType: "ssh", WorkspacePath: worktree,
AdditionalSkillSlugs: []string{"kandev-step-decision"},
})
if err != nil {
t.Fatalf("Deploy: %v", err)
}
raw, ok := res.Metadata[skill.MetadataKeySkillManifestJSON].(string)
if !ok || raw == "" {
t.Fatalf("SSH delivery missing skill manifest: %#v", res.Metadata)
}
var manifest skill.Manifest
if err := json.Unmarshal([]byte(raw), &manifest); err != nil {
t.Fatalf("unmarshal SSH manifest: %v", err)
}
if len(manifest.Skills) != 1 || manifest.Skills[0].Slug != "kandev-step-decision" {
t.Fatalf("SSH manifest skills = %+v", manifest.Skills)
}
path := filepath.Join(worktree, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Fatalf("SSH delivery wrote control-plane worktree file %s", path)
}
}
func TestDeploy_DurableDecisionSkillIsNotMaterialized(t *testing.T) {
base := t.TempDir()
worktree := t.TempDir()
decision := &skill.Skill{Slug: "kandev-step-decision", Content: "# decision"}
reader := &fakeSkillReader{skills: map[string]*skill.Skill{
"kandev-step-decision": decision,
"decision-id": decision,
}}
d := newDeployer(t, base, reader, &fakeInstructionLister{})
_, err := d.Deploy(context.Background(), skill.Request{
Profile: &settingsmodels.AgentProfile{
ID: "kanban-p1", AgentID: "claude-acp",
SkillIDs: `["decision-id"]`,
DesiredSkills: `["kandev-step-decision"]`,
},
ExecutorType: "local_pc", WorkspacePath: worktree,
})
if err != nil {
t.Fatalf("Deploy: %v", err)
}
path := filepath.Join(worktree, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Fatalf("durably selected decision skill was materialized at %s", path)
}
}
func TestDeploy_SeatThenNonSeatRemovesDecisionSkill(t *testing.T) {
base := t.TempDir()
worktree := t.TempDir()
reader := &fakeSkillReader{skills: map[string]*skill.Skill{
"kandev-step-decision": {Slug: "kandev-step-decision", Content: "# decision"},
"sk-review": {Slug: "sk-review", Content: "# review"},
}}
d := newDeployer(t, base, reader, &fakeInstructionLister{})
profile := &settingsmodels.AgentProfile{ID: "p1", AgentID: "claude-acp"}
if _, err := d.Deploy(context.Background(), skill.Request{
Profile: profile, ExecutorType: "local_pc", WorkspacePath: worktree,
AdditionalSkillSlugs: []string{"kandev-step-decision"},
}); err != nil {
t.Fatalf("seat Deploy: %v", err)
}
decisionPath := filepath.Join(worktree, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(decisionPath); err != nil {
t.Fatalf("seat decision skill missing: %v", err)
}
if _, err := d.Deploy(context.Background(), skill.Request{
Profile: profile, ExecutorType: "local_pc", WorkspacePath: worktree,
}); err != nil {
t.Fatalf("non-seat Deploy: %v", err)
}
if _, err := os.Stat(decisionPath); !os.IsNotExist(err) {
t.Fatalf("non-seat deployment left decision skill at %s", decisionPath)
}
}
// TestDeploy_KanbanProfileWithSkill_DeploysFiles verifies that a
// kanban-flavoured profile (no DesiredSkills, no Role) that the user
// later enriched with SkillIDs gets the same delivery treatment as
@@ -10,6 +10,11 @@ import (
settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
)
// ReservedDecisionSkillSlug is launch-scoped. It is admitted only through
// Request.AdditionalSkillSlugs so a seat can receive decision instructions
// without making them part of a durable agent profile.
const ReservedDecisionSkillSlug = "kandev-step-decision"
// buildManifest resolves a profile's desired skills + instruction files
// into a Manifest the delivery strategies can consume. It is pure data
// — no filesystem side effects.
@@ -17,8 +22,10 @@ import (
// The agent profile is the unified ADR 0005 row. We read both
// SkillIDs (the new merged column) and DesiredSkills (legacy office
// column) and union the two into a single list of slugs / IDs. Empty
// slots are dropped before lookup.
func (d *Deployer) buildManifest(ctx context.Context, profile *settingsmodels.AgentProfile, workspaceSlug string, officeRuntime bool) *Manifest {
// slots and launch-reserved skill slugs are dropped before lookup.
// System skills are included only for Office launches, whose runtime env
// provides the protocol they depend on.
func (d *Deployer) buildManifest(ctx context.Context, profile *settingsmodels.AgentProfile, workspaceSlug string, additionalSkillSlugs []string, officeRuntime bool) *Manifest {
// Profile.AgentID IS the agent type ID after ADR 0005 — the
// agent_profiles row's agent_id column points at the agents
// table (claude-acp, codex-acp, ...). No extra resolver needed.
@@ -29,28 +36,53 @@ func (d *Deployer) buildManifest(ctx context.Context, profile *settingsmodels.Ag
AgentID: profile.ID,
ProjectSkillDir: d.resolveProjectSkillDir(agentTypeID),
}
d.appendSkills(ctx, manifest, profile, officeRuntime)
d.appendSkills(ctx, manifest, profile, additionalSkillSlugs, officeRuntime)
d.appendInstructions(ctx, manifest, profile.ID)
return manifest
}
// appendSkills resolves every desired slug / id on the profile to a
// runtime Skill record. Lookups that fail are logged at debug level
// and dropped — a missing skill must never abort a launch. System
// skills (bundled Office protocol/task-ops skills) are skipped unless
// officeRuntime is true: their instructions depend on Office runtime
// env that only the office scheduler launch path provides.
func (d *Deployer) appendSkills(ctx context.Context, manifest *Manifest, profile *settingsmodels.AgentProfile, officeRuntime bool) {
// and dropped — a missing skill must never abort a launch.
// System skills (bundled Office protocol/task-ops skills) are skipped unless
// officeRuntime is true: their instructions depend on Office runtime env
// that only the Office scheduler launch path provides.
func (d *Deployer) appendSkills(ctx context.Context, manifest *Manifest, profile *settingsmodels.AgentProfile, additionalSkillSlugs []string, officeRuntime bool) {
if d.skillReader == nil {
return
}
for _, key := range mergedSkillKeys(profile) {
keys := mergedSkillKeys(profile)
additionalKeys := make(map[string]struct{}, len(additionalSkillSlugs))
seen := make(map[string]struct{}, len(keys)+len(additionalSkillSlugs))
for _, key := range keys {
seen[key] = struct{}{}
}
for _, key := range additionalSkillSlugs {
key = strings.TrimSpace(key)
if key == "" {
continue
}
additionalKeys[key] = struct{}{}
if _, exists := seen[key]; exists {
continue
}
seen[key] = struct{}{}
keys = append(keys, key)
}
for _, key := range keys {
skill, err := d.skillReader.GetSkillFromConfig(ctx, key)
if err != nil || skill == nil {
d.logger.Debug("skip skill in manifest",
zap.String("key", key), zap.Error(err))
continue
}
if isReservedSkillSlug(skill.Slug) {
if _, launchScoped := additionalKeys[key]; !launchScoped {
d.logger.Debug("skip reserved durable skill",
zap.String("key", key), zap.String("slug", skill.Slug))
continue
}
}
if skill.IsSystem && !officeRuntime {
d.logger.Debug("skip system skill: no office runtime env",
zap.String("key", key))
@@ -96,9 +128,11 @@ func (d *Deployer) resolveProjectSkillDir(agentTypeID string) string {
}
// mergedSkillKeys returns the union of profile.SkillIDs and
// profile.DesiredSkills, with empties trimmed. Duplicates collapse.
// profile.DesiredSkills, with empties and launch-reserved skill slugs
// removed. Duplicates collapse.
// The order is SkillIDs first (the canonical post-ADR field), then
// DesiredSkills (legacy office input that callers still populate).
// Launch-reserved skills are admitted only through AdditionalSkillSlugs.
//
// Both fields are JSON-array TEXT columns; non-empty values are unmarshalled
// into a list of slugs and concatenated. Malformed JSON is treated as an
@@ -114,7 +148,7 @@ func mergedSkillKeys(profile *settingsmodels.AgentProfile) []string {
out := make([]string, 0, len(skillIDs)+len(desired))
for _, k := range skillIDs {
k = strings.TrimSpace(k)
if k == "" || seen[k] {
if k == "" || isReservedSkillSlug(k) || seen[k] {
continue
}
seen[k] = true
@@ -122,7 +156,7 @@ func mergedSkillKeys(profile *settingsmodels.AgentProfile) []string {
}
for _, k := range desired {
k = strings.TrimSpace(k)
if k == "" || seen[k] {
if k == "" || isReservedSkillSlug(k) || seen[k] {
continue
}
seen[k] = true
@@ -131,6 +165,10 @@ func mergedSkillKeys(profile *settingsmodels.AgentProfile) []string {
return out
}
func isReservedSkillSlug(slug string) bool {
return strings.TrimSpace(slug) == ReservedDecisionSkillSlug
}
// decodeSkillSlugs parses a JSON-array string into a []string. Empty input
// or non-array input collapses to nil so callers can treat "no entries"
// uniformly without checking for malformed JSON.
@@ -1,8 +1,8 @@
// Package skill is the runtime-tier implementation of per-profile skill
// and instruction-file deployment introduced in ADR 0005 Wave A. The
// package owns the manifest builder and the per-executor delivery
// strategies (local filesystem symlinks, Docker bind-mount, Sprites
// upload). Wave E moved this code out of internal/office into the
// strategies (local filesystem, Docker bind-mount, Sprites upload,
// SSH/SFTP upload). Wave E moved this code out of internal/office into the
// runtime so every launch (kanban or office) goes through the same
// path.
//
@@ -15,9 +15,10 @@ import (
const MetadataKeyInstructionsDir = "kandev_instructions_dir"
// runSkillDeploy resolves the agent profile (now including the office
// enrichment fields from ADR 0005 Wave A), short-circuits when there's
// nothing to deploy, and otherwise hands off to the configured
// SkillDeployer. Any metadata patches the deployer returns are merged
// enrichment fields from ADR 0005 Wave A) and hands it off to the configured
// SkillDeployer. Empty profiles still reach the deployer so it can reconcile
// launch-scoped files left by a previous seat run. Any metadata patches the
// deployer returns are merged
// onto the prepared LaunchRequest so executor backends see them
// (e.g. Docker bind-mount, Sprites manifest upload). Errors are logged
// and swallowed: an in-flight launch must not be aborted because skill
@@ -36,18 +37,14 @@ func (m *Manager) runSkillDeploy(ctx context.Context, original, prepared *Launch
zap.Error(err))
return
}
if len(profile.SkillIDs) == 0 && len(profile.DesiredSkills) == 0 {
// Fast path: shallow / kanban-flavour profiles with no enrichment to
// deploy. Most launches today land here.
return
}
req := SkillDeployRequest{
Profile: profile,
WorkspacePath: prepared.WorkspacePath,
ExecutorType: prepared.ExecutorType,
WorkspaceID: profile.WorkspaceID,
SessionID: original.SessionID,
OfficeRuntime: prepared.McpMode == mcpmode.Office && strings.TrimSpace(prepared.Env["KANDEV_CLI"]) != "",
Profile: profile,
WorkspacePath: prepared.WorkspacePath,
ExecutorType: prepared.ExecutorType,
WorkspaceID: profile.WorkspaceID,
SessionID: original.SessionID,
AdditionalSkillSlugs: append([]string(nil), original.AdditionalSkillSlugs...),
OfficeRuntime: prepared.McpMode == mcpmode.Office && strings.TrimSpace(prepared.Env["KANDEV_CLI"]) != "",
}
result, err := m.skillDeployer.DeploySkills(ctx, req)
if err != nil {
@@ -3,9 +3,12 @@ package lifecycle
import (
"context"
"errors"
"os"
"path/filepath"
"sync/atomic"
"testing"
runtimeskill "github.com/kandev/kandev/internal/agent/runtime/lifecycle/skill"
settingsmodels "github.com/kandev/kandev/internal/agent/settings/models"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/common/mcpmode"
@@ -31,6 +34,17 @@ type recordingDeployer struct {
err error
}
type lifecycleSkillReader struct {
skill *runtimeskill.Skill
}
func (r *lifecycleSkillReader) GetSkillFromConfig(_ context.Context, key string) (*runtimeskill.Skill, error) {
if key == r.skill.Slug {
return r.skill, nil
}
return nil, nil
}
func (r *recordingDeployer) DeploySkills(_ context.Context, req SkillDeployRequest) (SkillDeployResult, error) {
r.called.Add(1)
r.last = req
@@ -63,10 +77,10 @@ func TestRunSkillDeploy_NoOpWithoutReader(t *testing.T) {
}
}
// TestRunSkillDeploy_FastPathEmptyProfile verifies that a shallow / kanban
// profile (empty skill_ids, empty desired_skills) short-circuits before
// invoking the deployer.
func TestRunSkillDeploy_FastPathEmptyProfile(t *testing.T) {
// TestRunSkillDeploy_EmptyProfileInvokesDeployerForCleanup verifies that a
// shallow / kanban profile still reaches the deployer so it can remove a
// launch-scoped skill left by an earlier seat run.
func TestRunSkillDeploy_EmptyProfileInvokesDeployerForCleanup(t *testing.T) {
mgr := newSkillDeployTestManager(t)
rec := &recordingDeployer{}
mgr.skillDeployer = rec
@@ -78,8 +92,70 @@ func TestRunSkillDeploy_FastPathEmptyProfile(t *testing.T) {
&LaunchRequest{AgentProfileID: "p1"},
&LaunchRequest{WorkspacePath: "/tmp/ws", ExecutorType: "local_pc"})
if rec.called.Load() != 0 {
t.Errorf("shallow profile should fast-path, deployer was called %d times", rec.called.Load())
if rec.called.Load() != 1 {
t.Errorf("empty profile should invoke cleanup, deployer was called %d times", rec.called.Load())
}
}
func TestRunSkillDeploy_AdditionalSkillSlugInvokesDeployerForEmptyProfile(t *testing.T) {
mgr := newSkillDeployTestManager(t)
rec := &recordingDeployer{}
mgr.skillDeployer = rec
mgr.agentProfileReader = &fakeProfileReader{
profile: &settingsmodels.AgentProfile{ID: "p1", AgentID: "a1"},
}
mgr.runSkillDeploy(context.Background(),
&LaunchRequest{
AgentProfileID: "p1",
AdditionalSkillSlugs: []string{"kandev-step-decision"},
},
&LaunchRequest{WorkspacePath: "/tmp/ws", ExecutorType: "local_pc"})
if rec.called.Load() != 1 {
t.Fatalf("expected deployer once, got %d", rec.called.Load())
}
if len(rec.last.AdditionalSkillSlugs) != 1 || rec.last.AdditionalSkillSlugs[0] != "kandev-step-decision" {
t.Fatalf("additional skill slugs = %v", rec.last.AdditionalSkillSlugs)
}
}
func TestRunSkillDeploy_SeatThenNonSeatCleansMaterializedDecisionSkill(t *testing.T) {
base := t.TempDir()
worktree := t.TempDir()
log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"})
deployer, err := runtimeskill.New(runtimeskill.Config{
Logger: log,
BasePath: base,
SkillReader: &lifecycleSkillReader{skill: &runtimeskill.Skill{
Slug: runtimeskill.ReservedDecisionSkillSlug, Content: "# decision",
}},
ProjectSkillDirResolver: func(string) string { return ".claude/skills" },
})
if err != nil {
t.Fatalf("skill.New: %v", err)
}
mgr := newSkillDeployTestManager(t)
mgr.skillDeployer = NewSkillDeployerAdapter(deployer)
mgr.agentProfileReader = &fakeProfileReader{
profile: &settingsmodels.AgentProfile{ID: "seat-profile", AgentID: "claude-acp"},
}
prepared := &LaunchRequest{WorkspacePath: worktree, ExecutorType: "local_pc"}
mgr.runSkillDeploy(context.Background(), &LaunchRequest{
AgentProfileID: "seat-profile",
AdditionalSkillSlugs: []string{
runtimeskill.ReservedDecisionSkillSlug,
},
}, prepared)
decisionPath := filepath.Join(worktree, ".claude", "skills", "kandev-step-decision", "SKILL.md")
if _, err := os.Stat(decisionPath); err != nil {
t.Fatalf("seat launch did not materialize decision skill: %v", err)
}
mgr.runSkillDeploy(context.Background(), &LaunchRequest{AgentProfileID: "seat-profile"}, prepared)
if _, err := os.Stat(decisionPath); !os.IsNotExist(err) {
t.Fatalf("non-seat launch left decision skill at %s", decisionPath)
}
}
@@ -27,11 +27,12 @@ type SkillDeployer interface {
// the agent will run inside; ExecutorType is the executor backend
// (local_pc / local_docker / sprites) so the deployer can pick a strategy.
type SkillDeployRequest struct {
Profile *settingsmodels.AgentProfile
WorkspacePath string
ExecutorType string
WorkspaceID string
SessionID string
Profile *settingsmodels.AgentProfile
WorkspacePath string
ExecutorType string
WorkspaceID string
SessionID string
AdditionalSkillSlugs []string
// OfficeRuntime reports whether backend selected Office mode and the
// finalized launch env contains a non-empty KANDEV_CLI. Office launch
// validation checks the remaining runtime variables before this hook runs.
@@ -35,12 +35,13 @@ func (a *skillDeployerAdapter) DeploySkills(ctx context.Context, req SkillDeploy
return SkillDeployResult{}, nil
}
res, err := a.inner.Deploy(ctx, skill.Request{
Profile: req.Profile,
WorkspacePath: req.WorkspacePath,
ExecutorType: req.ExecutorType,
WorkspaceID: req.WorkspaceID,
SessionID: req.SessionID,
OfficeRuntime: req.OfficeRuntime,
Profile: req.Profile,
WorkspacePath: req.WorkspacePath,
ExecutorType: req.ExecutorType,
WorkspaceID: req.WorkspaceID,
SessionID: req.SessionID,
AdditionalSkillSlugs: append([]string(nil), req.AdditionalSkillSlugs...),
OfficeRuntime: req.OfficeRuntime,
})
if err != nil {
return SkillDeployResult{}, err
@@ -6,6 +6,7 @@ import (
"errors"
"io"
"net"
"strconv"
"strings"
"sync"
"testing"
@@ -55,6 +56,9 @@ type fakeSSHServer struct {
calls []sshExecCall
handler sshExecHandler
reverseMu sync.Mutex
reverseForwards map[string]net.Listener
// sftpEnabled serves the "sftp" subsystem against the real filesystem, so
// tests point remote paths at a t.TempDir() and assert on real bytes.
sftpEnabled bool
@@ -81,7 +85,10 @@ func newFakeSSHServer(t *testing.T, handler sshExecHandler) *fakeSSHServer {
if err != nil {
t.Fatalf("listen: %v", err)
}
s := &fakeSSHServer{t: t, listener: listener, signer: signer, handler: handler}
s := &fakeSSHServer{
t: t, listener: listener, signer: signer, handler: handler,
reverseForwards: make(map[string]net.Listener),
}
t.Cleanup(s.Close)
s.wg.Add(1)
go s.acceptLoop()
@@ -140,6 +147,12 @@ func (s *fakeSSHServer) lastCommandContaining(substr string) (sshExecCall, bool)
func (s *fakeSSHServer) Close() {
s.closeOnce.Do(func() {
_ = s.listener.Close()
s.reverseMu.Lock()
for key, listener := range s.reverseForwards {
_ = listener.Close()
delete(s.reverseForwards, key)
}
s.reverseMu.Unlock()
})
s.wg.Wait()
}
@@ -184,10 +197,7 @@ func (s *fakeSSHServer) serveConn(conn net.Conn) {
defer func() { _ = serverConn.Close() }()
s.wg.Add(1)
go func() {
defer s.wg.Done()
ssh.DiscardRequests(reqs)
}()
go s.serveGlobalRequests(serverConn, reqs)
for newChan := range chans {
switch newChan.ChannelType() {
@@ -203,6 +213,134 @@ func (s *fakeSSHServer) serveConn(conn net.Conn) {
}
}
func (s *fakeSSHServer) serveGlobalRequests(conn *ssh.ServerConn, requests <-chan *ssh.Request) {
defer s.wg.Done()
for req := range requests {
switch req.Type {
case "tcpip-forward":
s.handleTCPIPForward(conn, req)
case "cancel-tcpip-forward":
s.handleCancelTCPIPForward(req)
default:
if req.WantReply {
_ = req.Reply(false, nil)
}
}
}
}
func (s *fakeSSHServer) handleTCPIPForward(conn *ssh.ServerConn, req *ssh.Request) {
var payload struct {
Addr string
Port uint32
}
if err := ssh.Unmarshal(req.Payload, &payload); err != nil {
_ = req.Reply(false, nil)
return
}
address := net.JoinHostPort(payload.Addr, strconv.FormatUint(uint64(payload.Port), 10))
listener, err := net.Listen("tcp", address)
if err != nil {
_ = req.Reply(false, nil)
return
}
actualPort := uint32(listener.Addr().(*net.TCPAddr).Port)
key := net.JoinHostPort(payload.Addr, strconv.FormatUint(uint64(actualPort), 10))
s.reverseMu.Lock()
s.reverseForwards[key] = listener
s.reverseMu.Unlock()
if payload.Port == 0 {
_ = req.Reply(true, ssh.Marshal(struct{ Port uint32 }{Port: actualPort}))
} else {
_ = req.Reply(true, nil)
}
s.wg.Add(1)
go s.serveReverseForward(conn, key, listener)
}
func (s *fakeSSHServer) handleCancelTCPIPForward(req *ssh.Request) {
var payload struct {
Addr string
Port uint32
}
if err := ssh.Unmarshal(req.Payload, &payload); err != nil {
_ = req.Reply(false, nil)
return
}
key := net.JoinHostPort(payload.Addr, strconv.FormatUint(uint64(payload.Port), 10))
s.reverseMu.Lock()
listener, ok := s.reverseForwards[key]
if ok {
delete(s.reverseForwards, key)
}
s.reverseMu.Unlock()
if ok {
_ = listener.Close()
}
_ = req.Reply(true, nil)
}
func (s *fakeSSHServer) serveReverseForward(conn *ssh.ServerConn, key string, listener net.Listener) {
defer s.wg.Done()
defer func() {
s.reverseMu.Lock()
delete(s.reverseForwards, key)
s.reverseMu.Unlock()
}()
for {
incoming, err := listener.Accept()
if err != nil {
return
}
s.serveReverseForwardConnection(conn, incoming)
}
}
func (s *fakeSSHServer) serveReverseForwardConnection(conn *ssh.ServerConn, incoming net.Conn) {
defer func() { _ = incoming.Close() }()
originAddr, originPort := tcpOrigin(incoming.RemoteAddr())
channel, requests, err := conn.OpenChannel("forwarded-tcpip", ssh.Marshal(struct {
Addr string
Port uint32
OriginAddr string
OriginPort uint32
}{
Addr: "127.0.0.1",
Port: uint32(incoming.LocalAddr().(*net.TCPAddr).Port),
OriginAddr: originAddr,
OriginPort: originPort,
}))
if err != nil {
return
}
s.wg.Add(1)
go func() {
defer s.wg.Done()
ssh.DiscardRequests(requests)
}()
var copies sync.WaitGroup
copies.Add(2)
go func() {
defer copies.Done()
_, _ = io.Copy(channel, incoming)
}()
go func() {
defer copies.Done()
_, _ = io.Copy(incoming, channel)
}()
copies.Wait()
_ = channel.Close()
}
func tcpOrigin(addr net.Addr) (string, uint32) {
tcpAddr, ok := addr.(*net.TCPAddr)
if !ok {
return "", 0
}
return tcpAddr.IP.String(), uint32(tcpAddr.Port)
}
func (s *fakeSSHServer) serveSession(newChan ssh.NewChannel) {
defer s.wg.Done()
channel, requests, err := newChan.Accept()
@@ -923,6 +923,9 @@ type LaunchRequest struct {
TaskDescription string // Task description to send via ACP prompt
Attachments []MessageAttachment // Attachments (images/files) for the initial prompt
Env map[string]string // Additional env vars
// AdditionalSkillSlugs are materialized for this launch in addition to the
// durable profile selection.
AdditionalSkillSlugs []string
// ApprovedSecretEnvKeys contains repository binding keys that SSH may
// forward in addition to its managed credential allowlist.
ApprovedSecretEnvKeys []string
@@ -424,6 +424,7 @@ func buildLifecycleLaunchRequest(
TaskDescription: req.TaskDescription,
Attachments: convertToLifecycleAttachments(req.Attachments),
Env: req.Env,
AdditionalSkillSlugs: append([]string(nil), req.AdditionalSkillSlugs...),
ApprovedSecretEnvKeys: append([]string(nil), req.ApprovedSecretEnvKeys...),
EnvironmentDefinitions: append([]runtimeenv.Definition(nil), req.EnvironmentDefinitions...),
EnvironmentResolutionRequired: req.EnvironmentResolutionRequired,
@@ -220,32 +220,32 @@ func stringContains(value, want string) bool {
func newCanvasEditTestDependencies() (*fakeCanvasEditTaskStore, *fakeCanvasEditCanvasStore, *fakeCanvasEditReleaseStore, *fakeCanvasEditArtifactStore) {
return &fakeCanvasEditTaskStore{
workspace: &models.Workspace{
ID: "workspace-a",
DefaultAgentProfileID: stringPointer("profile-default"),
DefaultExecutorID: stringPointer("executor-default"),
},
}, &fakeCanvasEditCanvasStore{canvas: &canvas.Canvas{
ID: "canvas-a",
PluginInstanceID: "instance-a",
WorkspaceID: "workspace-a",
ScopeKind: canvas.ScopeWorkspace,
Status: instances.StatusActive,
ActiveReleaseID: "release-a",
ActiveReleaseStatus: instances.ValidationValid,
EffectiveGrants: []canvas.GrantProjection{{
PermissionKind: "api_read", Resource: "tasks", ScopeCeiling: instances.ScopeWorkspace,
}},
}}, &fakeCanvasEditReleaseStore{release: instances.Release{
ID: "release-a",
InstanceID: "instance-a",
PackageDigest: "digest-a",
ArtifactPath: "releases/digest-a",
ValidationStatus: instances.ValidationValid,
}}, &fakeCanvasEditArtifactStore{files: map[string][]byte{
"manifest.yaml": []byte("manifest"),
"index.html": []byte("index"),
}}
workspace: &models.Workspace{
ID: "workspace-a",
DefaultAgentProfileID: stringPointer("profile-default"),
DefaultExecutorID: stringPointer("executor-default"),
},
}, &fakeCanvasEditCanvasStore{canvas: &canvas.Canvas{
ID: "canvas-a",
PluginInstanceID: "instance-a",
WorkspaceID: "workspace-a",
ScopeKind: canvas.ScopeWorkspace,
Status: instances.StatusActive,
ActiveReleaseID: "release-a",
ActiveReleaseStatus: instances.ValidationValid,
EffectiveGrants: []canvas.GrantProjection{{
PermissionKind: "api_read", Resource: "tasks", ScopeCeiling: instances.ScopeWorkspace,
}},
}}, &fakeCanvasEditReleaseStore{release: instances.Release{
ID: "release-a",
InstanceID: "instance-a",
PackageDigest: "digest-a",
ArtifactPath: "releases/digest-a",
ValidationStatus: instances.ValidationValid,
}}, &fakeCanvasEditArtifactStore{files: map[string][]byte{
"manifest.yaml": []byte("manifest"),
"index.html": []byte("index"),
}}
}
func stringPointer(value string) *string { return &value }
@@ -1878,10 +1878,6 @@ func registerMCPAndDebugRoutes(
if p.services.GitLab != nil {
mcpHandlers.SetTaskMRAutomationService(p.services.GitLab)
}
if p.services.OfficeSvcs != nil && p.services.OfficeSvcs.Dashboard != nil {
mcpHandlers.SetDashboardService(p.services.OfficeSvcs.Dashboard)
}
// Reuse the cross-task handoff service constructed in registerRoutes —
// the same instance backs the MCP path and the HTTP Kanban path so
// workspace-group state stays consistent across both surfaces.
+8 -10
View File
@@ -81,7 +81,6 @@ import (
"github.com/kandev/kandev/internal/office/configloader"
officeservice "github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/orchestrator"
v1 "github.com/kandev/kandev/pkg/api/v1"
// Office feature packages
office "github.com/kandev/kandev/internal/office"
@@ -2056,17 +2055,16 @@ func backfillAgentDefaultSkills(
}
}
// newOfficeTaskStarter wraps orchestratorSvc.StartTaskWithEnv in the
// officeservice.TaskStarterWithEnvFunc adapter. Extracted from
// newOfficeTaskStarter wraps orchestratorSvc.StartTaskWithEnvAndSkills in the
// officeservice.TaskStarterWithLaunchContextFunc adapter. Extracted from
// initOfficeServices to keep that function under the funlen cap.
func newOfficeTaskStarter(orchestratorSvc *orchestrator.Service) officeservice.TaskStarter {
return officeservice.TaskStarterWithEnvFunc(
func(ctx context.Context, taskID, agentProfileID, executorID,
executorProfileID string, priority string, prompt, workflowStepID string,
planMode bool, attachments []v1.MessageAttachment, env map[string]string) error {
_, err := orchestratorSvc.StartTaskWithEnv(ctx, taskID, agentProfileID,
executorID, executorProfileID, priority, prompt,
workflowStepID, planMode, false, attachments, env)
return officeservice.TaskStarterWithLaunchContextFunc(
func(ctx context.Context, taskID, agentProfileID string, launch officeservice.LaunchContext) error {
_, err := orchestratorSvc.StartTaskWithEnvAndSkills(ctx, taskID, agentProfileID,
launch.ExecutorID, launch.ExecutorProfileID, launch.Priority, launch.Prompt,
launch.WorkflowStepID, launch.PlanMode, false, launch.Attachments, launch.Env,
launch.AdditionalSkillSlugs)
return err
},
)
@@ -1,97 +0,0 @@
package handlers
import (
"context"
"encoding/json"
"errors"
"go.uber.org/zap"
"github.com/kandev/kandev/internal/office/dashboard"
"github.com/kandev/kandev/internal/office/shared"
"github.com/kandev/kandev/internal/task/models"
ws "github.com/kandev/kandev/pkg/websocket"
)
// SetDashboardService wires the office dashboard service used to back the
// record_step_decision_kandev MCP tool. Optional — when nil, the handler
// reports a configuration error instead of registering and returning a
// broken endpoint.
func (h *Handlers) SetDashboardService(svc *dashboard.DashboardService) {
h.dashboardSvc = svc
}
// handleRecordStepDecision dispatches mcp.record_step_decision
// (record_step_decision_kandev). Per the tool contract, task/step/role are
// never caller-supplied: the task comes from the calling session (msg
// carries session_id/task_id set by the MCP server from its own bound
// s.sessionID/s.taskID, matching every other Office tool), and role/seat
// are resolved server-side by DashboardService.RecordAgentDecision via the
// engine's AC-4a slate.
func (h *Handlers) handleRecordStepDecision(ctx context.Context, msg *ws.Message) (*ws.Message, error) {
if h.dashboardSvc == nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "dashboard service not configured", nil)
}
var req struct {
TaskID string `json:"task_id"`
SessionID string `json:"session_id"`
Decision string `json:"decision"`
Reason string `json:"reason"`
}
if err := json.Unmarshal(msg.Payload, &req); err != nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
if req.TaskID == "" || req.SessionID == "" {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id and session_id are required", nil)
}
session, err := h.sessionRepo.GetTaskSession(ctx, req.SessionID)
if err != nil {
if errors.Is(err, models.ErrTaskSessionNotFound) {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "session not found", nil)
}
h.logger.Error("record_step_decision: failed to load session",
zap.String("session_id", req.SessionID), zap.Error(err))
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to load session", nil)
}
if session.TaskID != req.TaskID {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "session does not belong to task", nil)
}
result, err := h.dashboardSvc.RecordAgentDecision(ctx, dashboard.RecordAgentDecisionInput{
TaskID: req.TaskID,
AgentProfileID: session.AgentProfileID,
Decision: req.Decision,
Reason: req.Reason,
SessionID: req.SessionID,
})
if err != nil {
if !errors.Is(err, shared.ErrForbidden) && !dashboard.IsAgentDecisionValidationError(err) {
h.logger.Error("record_step_decision: failed to record decision", zap.String("task_id", req.TaskID), zap.Error(err))
}
return mapRecordStepDecisionError(msg, err)
}
return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
"decision": result.Decision,
"role": result.Role,
"step_id": result.StepID,
"decision_id": result.DecisionID,
"decided_at": result.DecidedAt,
"transition_applied": result.TransitionApplied,
"guards": result.Guards,
})
}
// mapRecordStepDecisionError classifies RecordAgentDecision failures into
// WS error codes. Only typed validation failures are safe to expose. Raw
// repository and engine failures become an opaque internal error.
func mapRecordStepDecisionError(msg *ws.Message, err error) (*ws.Message, error) {
if errors.Is(err, shared.ErrForbidden) {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeForbidden, err.Error(), nil)
}
if dashboard.IsAgentDecisionValidationError(err) {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
}
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to record step decision", nil)
}
@@ -1,157 +0,0 @@
package handlers
import (
"context"
"errors"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/office/dashboard"
"github.com/kandev/kandev/internal/office/shared"
"github.com/kandev/kandev/internal/task/models"
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
)
// This file covers the record_step_decision_kandev MCP transport boundary
// (handleRecordStepDecision / mapRecordStepDecisionError). Review round 1
// flagged this file at zero coverage. RecordAgentDecision's own business
// logic (role resolution, verdict/reason validation, engine re-evaluation)
// is already covered at the dashboard-service tier in
// internal/office/dashboard/agent_decisions_test.go — these tests instead
// exercise what's unique to this transport: payload validation, session
// lookup, the session/task ownership check, and error-code mapping.
// seedRecordStepDecisionSession seeds a minimal workspace/task/session chain
// so handleRecordStepDecision's h.sessionRepo.GetTaskSession lookup and the
// session.TaskID ownership check have something real to read.
func seedRecordStepDecisionSession(t *testing.T, repo *sqliterepo.Repository, workspaceID, taskID, sessionID string) {
t.Helper()
ctx := context.Background()
now := time.Now().UTC()
require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{
ID: workspaceID, Name: "Decision WS", CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, repo.CreateTask(ctx, &models.Task{
ID: taskID, WorkspaceID: workspaceID, Title: "Decision Task",
State: v1.TaskStateInProgress, CreatedAt: now, UpdatedAt: now,
}))
require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{
ID: sessionID, TaskID: taskID, State: models.TaskSessionStateRunning,
StartedAt: now, UpdatedAt: now,
}))
}
// newRecordStepDecisionHandler builds a *Handlers whose dashboardSvc is a
// real, minimally-constructed *dashboard.DashboardService. Every test case
// here returns before RecordAgentDecision ever touches its repo/dispatcher
// dependencies, so a nil Repository is safe — dashboardSvc only needs to be
// non-nil to pass handleRecordStepDecision's first guard.
func newRecordStepDecisionHandler(t *testing.T, repo *sqliterepo.Repository, svc *dashboard.DashboardService) *Handlers {
t.Helper()
return &Handlers{
sessionRepo: repo,
dashboardSvc: svc,
logger: testLogger(t).WithFields(),
}
}
func TestHandleRecordStepDecision_NotConfigured(t *testing.T) {
h := &Handlers{logger: testLogger(t).WithFields()}
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{
"task_id": "t1", "session_id": "s1", "decision": "approved", "reason": "lgtm",
})
resp, err := h.handleRecordStepDecision(context.Background(), msg)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeInternalError)
}
func TestHandleRecordStepDecision_InvalidPayload(t *testing.T) {
svc := dashboard.NewDashboardService(nil, testLogger(t), nil, nil, nil)
h := newRecordStepDecisionHandler(t, nil, svc)
msg := &ws.Message{
ID: "test-id", Type: ws.MessageTypeRequest,
Action: ws.ActionMCPRecordStepDecision, Payload: []byte(`{"task_id":`),
}
resp, err := h.handleRecordStepDecision(context.Background(), msg)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeBadRequest)
}
func TestHandleRecordStepDecision_MissingFields(t *testing.T) {
svc := dashboard.NewDashboardService(nil, testLogger(t), nil, nil, nil)
cases := []struct {
name string
payload map[string]interface{}
}{
{
name: "missing task_id",
payload: map[string]interface{}{"session_id": "s1", "decision": "approved", "reason": "lgtm"},
},
{
name: "missing session_id",
payload: map[string]interface{}{"task_id": "t1", "decision": "approved", "reason": "lgtm"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
h := newRecordStepDecisionHandler(t, nil, svc)
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, tc.payload)
resp, err := h.handleRecordStepDecision(context.Background(), msg)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeValidation)
})
}
}
func TestHandleRecordStepDecision_SessionNotFound(t *testing.T) {
svc := dashboard.NewDashboardService(nil, testLogger(t), nil, nil, nil)
_, repo := newTestTaskService(t)
h := newRecordStepDecisionHandler(t, repo, svc)
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{
"task_id": "missing-task", "session_id": "missing-session", "decision": "approved", "reason": "lgtm",
})
resp, err := h.handleRecordStepDecision(context.Background(), msg)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeNotFound)
}
func TestHandleRecordStepDecision_SessionDoesNotBelongToTask(t *testing.T) {
svc := dashboard.NewDashboardService(nil, testLogger(t), nil, nil, nil)
_, repo := newTestTaskService(t)
seedRecordStepDecisionSession(t, repo, "ws-rd1", "task-rd1", "sess-rd1")
h := newRecordStepDecisionHandler(t, repo, svc)
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{
"task_id": "some-other-task", "session_id": "sess-rd1", "decision": "approved", "reason": "lgtm",
})
resp, err := h.handleRecordStepDecision(context.Background(), msg)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeValidation)
}
func TestMapRecordStepDecisionError_ForbiddenMapsTo403(t *testing.T) {
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{})
resp, err := mapRecordStepDecisionError(msg, shared.ErrForbidden)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeForbidden)
}
func TestMapRecordStepDecisionError_UnexpectedFailureMapsToInternal(t *testing.T) {
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{})
resp, err := mapRecordStepDecisionError(msg, errors.New("task has no workflow step bound"))
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeInternalError)
}
func TestMapRecordStepDecisionError_ValidationMapsToValidation(t *testing.T) {
msg := makeWSMessage(t, ws.ActionMCPRecordStepDecision, map[string]interface{}{})
validationErr := &dashboard.AgentDecisionValidationError{Err: errors.New("reason is required")}
resp, err := mapRecordStepDecisionError(msg, validationErr)
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeValidation)
}
@@ -21,7 +21,6 @@ import (
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/events/bus"
mcpscope "github.com/kandev/kandev/internal/mcp/scope"
"github.com/kandev/kandev/internal/office/dashboard"
"github.com/kandev/kandev/internal/orchestrator"
"github.com/kandev/kandev/internal/orchestrator/executor"
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
@@ -285,10 +284,6 @@ type Handlers struct {
// MCP tools introduced in office task handoffs phase 2.
handoffSvc *service.HandoffService
// Office dashboard service (optional, set via SetDashboardService).
// Wires the record_step_decision_kandev MCP tool.
dashboardSvc *dashboard.DashboardService
// Optional PR lister (set via SetTaskPRLister) used to enrich
// task-listing responses with associated pull requests.
taskPRLister TaskPRLister
@@ -545,9 +540,6 @@ func (h *Handlers) registerConfigModeHandlers(d *guardedMCPDispatcher) {
d.RegisterFunc(ws.ActionMCPGetTaskDocument, h.handleGetTaskDocument)
d.RegisterFunc(ws.ActionMCPWriteTaskDocument, h.handleWriteTaskDocument)
}
if h.dashboardSvc != nil {
d.RegisterFunc(ws.ActionMCPRecordStepDecision, h.handleRecordStepDecision)
}
if h.taskSvc != nil {
h.registerTaskConfigMutationHandlers(d)
}
@@ -1,53 +0,0 @@
package mcp
import (
"context"
"encoding/json"
"strings"
"github.com/mark3labs/mcp-go/mcp"
"github.com/mark3labs/mcp-go/server"
ws "github.com/kandev/kandev/pkg/websocket"
)
// registerRecordStepDecisionTool registers record_step_decision_kandev, the
// Office quorum decision-recording tool. Per the tool contract in
// docs/specs/tasks/requirements/workflow-quorum-decision-recording-agent-surface.md, it takes no
// task_id — task/step/role are resolved server-side from the calling
// session, matching every other Office tool.
func (s *Server) registerRecordStepDecisionTool() {
s.mcpServer.AddTool(
mcp.NewTool("record_step_decision_kandev",
mcp.WithDescription(`Record an approve/reject decision for the current workflow step, as a reviewer or approver. Call this once you have finished evaluating the step's output. The task, step, and your role are resolved from your session — you cannot record a decision on another task. Not idempotent: a second call supersedes the first.`),
mcp.WithString("decision", mcp.Required(), mcp.Enum("approved", "rejected"), mcp.Description("The verdict: \"approved\" or \"rejected\".")),
mcp.WithString("reason", mcp.Required(), mcp.Description("Non-empty explanation for the verdict. Recorded in the task timeline.")),
),
s.wrapHandler("record_step_decision_kandev", s.recordStepDecisionHandler()),
)
}
func (s *Server) recordStepDecisionHandler() server.ToolHandlerFunc {
return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
if s.taskID == "" || s.sessionID == "" {
return mcp.NewToolResultError("record_step_decision_kandev requires a bound task and session"), nil
}
decision := strings.TrimSpace(req.GetString("decision", ""))
reason := strings.TrimSpace(req.GetString("reason", ""))
if reason == "" {
return mcp.NewToolResultError("reason is required"), nil
}
payload := map[string]interface{}{
"task_id": s.taskID,
"session_id": s.sessionID,
"decision": decision,
"reason": reason,
}
var result map[string]interface{}
if err := s.backend.RequestPayload(ctx, ws.ActionMCPRecordStepDecision, payload, &result); err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
data, _ := json.MarshalIndent(result, "", " ")
return mcp.NewToolResultText(string(data)), nil
}
}
@@ -1,88 +0,0 @@
package mcp
import (
"testing"
ws "github.com/kandev/kandev/pkg/websocket"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// This file covers the record_step_decision_kandev tool-registration/dispatch
// half of the MCP transport (registerRecordStepDecisionTool /
// recordStepDecisionHandler). Review round 1 flagged this file at zero
// coverage. The WS-handler half (handleRecordStepDecision,
// mapRecordStepDecisionError) is covered separately in
// internal/mcp/handlers/agent_decision_handlers_test.go; these tests instead
// exercise what's unique to this transport boundary: the bound-session guard,
// trimming/validation before dispatch, the exact payload shape sent to
// s.backend.RequestPayload, and surfacing a backend error as a tool error.
func newOfficeModeServer(t *testing.T, backend BackendClient, sessionID, taskID string) *Server {
t.Helper()
return New(backend, sessionID, taskID, 10005, newTestLogger(t), "", false, ModeOffice)
}
func TestRecordStepDecision_RequiresBoundTaskAndSession(t *testing.T) {
backend := &testBackend{}
s := newOfficeModeServer(t, backend, "", "")
result := callTool(t, s, "record_step_decision_kandev", map[string]interface{}{
"decision": "approved",
"reason": "looks good",
})
require.True(t, result.IsError, "unbound task/session must be rejected")
assert.Empty(t, backend.lastAction, "backend must not be called without a bound session")
}
func TestRecordStepDecision_RejectsEmptyReason(t *testing.T) {
backend := &testBackend{}
s := newOfficeModeServer(t, backend, "test-session", "task-current")
result := callTool(t, s, "record_step_decision_kandev", map[string]interface{}{
"decision": "approved",
"reason": " ",
})
require.True(t, result.IsError, "whitespace-only reason must be rejected")
assert.Empty(t, backend.lastAction, "backend must not be called with an empty reason")
}
func TestRecordStepDecision_SendsExpectedPayload(t *testing.T) {
backend := &testBackend{
response: map[string]interface{}{
"decision": "approved",
"role": "reviewer",
"step_id": "step-1",
},
}
s := newOfficeModeServer(t, backend, "test-session", "task-current")
result := callTool(t, s, "record_step_decision_kandev", map[string]interface{}{
"decision": "approved",
"reason": " looks good ",
})
require.False(t, result.IsError)
assert.Equal(t, ws.ActionMCPRecordStepDecision, backend.lastAction)
payload, ok := backend.lastPayload.(map[string]interface{})
require.True(t, ok)
assert.Equal(t, "task-current", payload["task_id"])
assert.Equal(t, "test-session", payload["session_id"])
assert.Equal(t, "approved", payload["decision"])
assert.Equal(t, "looks good", payload["reason"], "reason should be trimmed before dispatch")
}
func TestRecordStepDecision_SurfacesBackendErrorAsToolError(t *testing.T) {
backend := &testBackend{err: assert.AnError}
s := newOfficeModeServer(t, backend, "test-session", "task-current")
result := callTool(t, s, "record_step_decision_kandev", map[string]interface{}{
"decision": "rejected",
"reason": "needs work",
})
require.True(t, result.IsError, "a backend error must surface as a tool error, not panic or succeed")
}
@@ -1094,7 +1094,6 @@ func (s *Server) profileToolGroups() []profileToolGroup {
{name: "review", enabled: kanban, register: func(s *Server) { s.registerReviewTools() }},
{name: "related-tasks", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || office(ctx) }, register: func(s *Server) { s.registerRelatedTasksTool() }},
{name: "office-documents", enabled: office, register: func(s *Server) { s.registerTaskDocumentTools() }},
{name: "office-decisions", enabled: office, register: func(s *Server) { s.registerRecordStepDecisionTool() }},
{name: "task-branch-sources", enabled: kanban, register: func(s *Server) {
s.registerAddBranchToTaskTool()
s.registerAddWorkspaceSourcesTool()
@@ -1051,11 +1051,11 @@ func TestServerModeOffice_ToolCount(t *testing.T) {
s := New(backend, "test-session", "test-task", 10005, log, "", false, ModeOffice)
tools := getRegisteredToolNames(s)
// 4 plan + 1 interaction + 1 related-tasks + 3 task-documents
// + 1 rich-output + 1 decisions + 1 step_complete (ADR 0015) = 12.
// + 1 rich-output + 1 step_complete (ADR 0015) = 11.
// (delegate_task_kandev retired in favour of `agentctl kandev task create …`).
// (list_task_comments_kandev retired in favour of `agentctl kandev comment list …`).
assert.Contains(t, tools, "step_complete_kandev", "office mode must register the ADR 0015 completion signal")
assert.Equal(t, 12, len(tools))
assert.Equal(t, 11, len(tools))
}
func TestServerModeOffice_DisableAskQuestion(t *testing.T) {
@@ -1073,9 +1073,9 @@ func TestServerModeOffice_DisableAskQuestion(t *testing.T) {
// the agentctl CLI as `agentctl kandev task create --parent …`).
assert.NotContains(t, tools, "delegate_task_kandev")
// 4 plan + 1 related-tasks + 3 task-documents + 1 rich-output
// + 1 decisions + 1 step_complete (ADR 0015) = 11
// + 1 step_complete (ADR 0015) = 10
// (no ask_user_question, no delegate, no list_task_comments)
assert.Equal(t, 11, len(tools))
assert.Equal(t, 10, len(tools))
}
func TestServerModeConstants(t *testing.T) {
@@ -274,6 +274,23 @@ func TestSyspromptToolNames_ExactlyMatchMCPOfficeMode(t *testing.T) {
assert.Equal(t, registered, referenced,
"Office first-turn context must advertise exactly the ModeOffice tool inventory")
assert.Len(t, registered, 11, "Office MCP must expose the replacement 11-tool catalog")
}
func TestDecisionMCPTransportIsAbsentFromOfficeAndKanban(t *testing.T) {
log := newTestLogger(t)
backend := NewChannelBackendClient(log)
defer backend.Close()
office := New(backend, "office-session", "office-task", 10005, log, "", false, ModeOffice)
kanban := New(backend, "task-session", "task", 10005, log, "", false, ModeTask)
require.NotNil(t, office)
require.NotNil(t, kanban)
assert.NotContains(t, office.mcpServer.ListTools(), "record_step_decision_kandev")
assert.NotContains(t, sysprompt.OfficeContext(), "record_step_decision_kandev")
assert.NotContains(t, kanban.mcpServer.ListTools(), "record_step_decision_kandev")
assert.NotContains(t, sysprompt.KandevContext(), "record_step_decision")
}
// TestSyspromptToolNames_NoBareToolReferences catches the opposite drift: a
@@ -0,0 +1,43 @@
---
name: kandev-step-decision
description: Record the current workflow step decision through the task-bound Office CLI.
kandev:
system: true
version: "0.42.0"
---
# Workflow step decisions
Use the task-bound CLI command when you hold the current review or approval
seat and the run prompt advertises the decision action.
## Record the decision
The only accepted verdicts are `approved` and `rejected`. Use a non-empty reason
that explains the evidence for the verdict.
```bash
$KANDEV_CLI kandev task decision --decision approved --reason "..."
```
Use `approved` as shown, or replace it with `rejected` when rejecting. Replace
the reason with the evidence for the current workflow step. Make this command
the final action for the turn, then stop. A repeated
decision supersedes the previous decision for that participant and step.
The command returns structured JSON with these fields:
```json
{
"decision": "approved",
"role": "reviewer",
"step_id": "step-1",
"decision_id": "decision-1",
"decided_at": "2026-01-01T00:00:00Z",
"transition_applied": false,
"guards": []
}
```
Posting comments do not count as a workflow step decision. Approval-inbox commands do not count
either. Do not use either operation instead of the task decision command.
@@ -16,7 +16,7 @@ import (
const agentDecisionReasonRequiredErr = "reason is required"
// RecordAgentDecisionInput bundles the agent decision tool's caller-supplied
// RecordAgentDecisionInput bundles the runtime decision endpoint's caller-supplied
// fields. Per the tool contract, task/step/role are never caller-supplied —
// they are resolved server-side from the calling session and the AC-50
// slate.
@@ -28,9 +28,9 @@ type RecordAgentDecisionInput struct {
// SessionID, when features.officeSessionIdentity is on, names the
// decider's own calling session so RecordDecision re-evaluates against
// it instead of the task's most-recently-started ("active") session.
// Populated unconditionally by the MCP handler; gated here because the
// flag decision belongs with the rest of this service's behavior, not
// the transport layer.
// The runtime handler derives it from the signed RunContext and passes it
// unconditionally; gated here because the flag decision belongs with the
// rest of this service's behavior, not the transport layer.
SessionID string
}
@@ -57,7 +57,7 @@ func (e *AgentDecisionValidationError) Unwrap() error {
}
// IsAgentDecisionValidationError reports whether err is safe to expose as a
// validation response at the MCP boundary.
// validation response at the runtime boundary.
func IsAgentDecisionValidationError(err error) bool {
var target *AgentDecisionValidationError
return errors.As(err, &target)
@@ -67,7 +67,7 @@ func agentDecisionValidation(err error) error {
return &AgentDecisionValidationError{Err: err}
}
// RecordAgentDecisionResult is the AC-64 tool-contract return shape.
// RecordAgentDecisionResult is the AC-64 runtime contract return shape.
type RecordAgentDecisionResult struct {
Decision string
Role string
@@ -92,8 +92,8 @@ type quorumEvaluatingDispatcher interface {
EvaluateStepQuorum(ctx context.Context, taskID string) (engine.QuorumSnapshot, error)
}
// RecordAgentDecision is the agent/MCP counterpart to ApproveTask /
// RequestTaskChanges (`record_step_decision_kandev`). Unlike the human
// RecordAgentDecision is the agent-runtime counterpart to ApproveTask /
// RequestTaskChanges. Unlike the human
// path — whose resolveDeciderRole/resolveParticipantID stay unchanged and
// step-scoped per AC-57b — this path resolves role and seat over the
// AC-50 population via the engine's ResolveParticipantRole (AC-4a): a new
@@ -4,7 +4,7 @@ import "github.com/kandev/kandev/internal/workflow/engine"
// GuardStateDTO is the direct JSON projection of a single AC-57d guard
// entry (`engine.QuorumGuardState`), shared verbatim by the AC-24b HTTP
// quorum endpoint and the AC-64 agent decision tool response so the two
// quorum endpoint and the AC-64 runtime decision response so the two
// surfaces can never report different arithmetic for the same step.
type GuardStateDTO struct {
TargetStepID string `json:"target_step_id"`
@@ -0,0 +1,68 @@
package office
import (
"context"
"fmt"
"github.com/kandev/kandev/internal/office/dashboard"
officeruntime "github.com/kandev/kandev/internal/office/runtime"
)
type dashboardDecisionRecorder interface {
RecordAgentDecision(ctx context.Context, input dashboard.RecordAgentDecisionInput) (*dashboard.RecordAgentDecisionResult, error)
}
type runtimeDecisionRecorder struct {
dashboard dashboardDecisionRecorder
}
func newRuntimeDecisionRecorder(svc dashboardDecisionRecorder) officeruntime.DecisionRecorder {
if svc == nil {
return nil
}
return &runtimeDecisionRecorder{dashboard: svc}
}
func (r *runtimeDecisionRecorder) RecordAgentDecision(
ctx context.Context,
input officeruntime.RecordAgentDecisionInput,
) (officeruntime.RecordAgentDecisionResult, error) {
result, err := r.dashboard.RecordAgentDecision(ctx, dashboard.RecordAgentDecisionInput{
TaskID: input.TaskID,
AgentProfileID: input.AgentProfileID,
SessionID: input.SessionID,
Decision: input.Decision,
Reason: input.Reason,
})
if err != nil {
if dashboard.IsAgentDecisionValidationError(err) {
return officeruntime.RecordAgentDecisionResult{}, officeruntime.NewDecisionValidationError(err)
}
return officeruntime.RecordAgentDecisionResult{}, err
}
if result == nil {
return officeruntime.RecordAgentDecisionResult{}, fmt.Errorf("record agent decision returned no result")
}
guards := make([]officeruntime.DecisionGuard, 0, len(result.Guards))
for _, guard := range result.Guards {
guards = append(guards, officeruntime.DecisionGuard{
TargetStepID: guard.TargetStepID,
Role: guard.Role,
Threshold: guard.Threshold,
RequiredCount: guard.RequiredCount,
ReceivedCount: guard.ReceivedCount,
Satisfied: guard.Satisfied,
Reason: guard.Reason,
Error: guard.Error,
})
}
return officeruntime.RecordAgentDecisionResult{
Decision: result.Decision,
Role: result.Role,
StepID: result.StepID,
DecisionID: result.DecisionID,
DecidedAt: result.DecidedAt,
TransitionApplied: result.TransitionApplied,
Guards: guards,
}, nil
}
@@ -0,0 +1,79 @@
package office
import (
"context"
"errors"
"testing"
"time"
"github.com/kandev/kandev/internal/office/dashboard"
officeruntime "github.com/kandev/kandev/internal/office/runtime"
)
type fakeDashboardDecisionRecorder struct {
input dashboard.RecordAgentDecisionInput
result *dashboard.RecordAgentDecisionResult
err error
}
func (f *fakeDashboardDecisionRecorder) RecordAgentDecision(
_ context.Context,
input dashboard.RecordAgentDecisionInput,
) (*dashboard.RecordAgentDecisionResult, error) {
f.input = input
return f.result, f.err
}
func TestRuntimeDecisionRecorderForwardsIdentityAndMapsResult(t *testing.T) {
decidedAt := time.Date(2026, 9, 7, 20, 0, 0, 0, time.UTC)
fake := &fakeDashboardDecisionRecorder{result: &dashboard.RecordAgentDecisionResult{
Decision: "approved",
Role: "approver",
StepID: "step-1",
DecisionID: "decision-1",
DecidedAt: decidedAt,
TransitionApplied: true,
Guards: []dashboard.GuardStateDTO{{
TargetStepID: "step-2",
Role: "approver",
Threshold: "all_approve",
RequiredCount: 1,
ReceivedCount: 1,
Satisfied: true,
}},
}}
recorder := newRuntimeDecisionRecorder(fake)
got, err := recorder.RecordAgentDecision(context.Background(), officeruntime.RecordAgentDecisionInput{
TaskID: "task-1",
AgentProfileID: "agent-1",
SessionID: "session-1",
Decision: "approved",
Reason: "ready",
})
if err != nil {
t.Fatalf("RecordAgentDecision: %v", err)
}
if fake.input.TaskID != "task-1" || fake.input.AgentProfileID != "agent-1" || fake.input.SessionID != "session-1" ||
fake.input.Decision != "approved" || fake.input.Reason != "ready" {
t.Fatalf("dashboard input = %+v", fake.input)
}
if got.Decision != "approved" || got.Role != "approver" || got.StepID != "step-1" ||
got.DecisionID != "decision-1" || !got.DecidedAt.Equal(decidedAt) || !got.TransitionApplied || len(got.Guards) != 1 {
t.Fatalf("runtime result = %+v", got)
}
if got.Guards[0].TargetStepID != "step-2" || got.Guards[0].RequiredCount != 1 || !got.Guards[0].Satisfied {
t.Fatalf("runtime guard = %+v", got.Guards[0])
}
}
func TestRuntimeDecisionRecorderClassifiesDashboardValidation(t *testing.T) {
fake := &fakeDashboardDecisionRecorder{err: &dashboard.AgentDecisionValidationError{Err: errors.New("reason is required")}}
recorder := newRuntimeDecisionRecorder(fake)
_, err := recorder.RecordAgentDecision(context.Background(), officeruntime.RecordAgentDecisionInput{})
var validation *officeruntime.DecisionValidationError
if !errors.As(err, &validation) {
t.Fatalf("error = %v, want runtime validation error", err)
}
}
@@ -55,7 +55,7 @@ type EngineHandle interface {
RecordParticipantDecision(ctx context.Context, sessionID string, in engine.DecisionInfo) (engine.RecordDecisionResult, error)
EvaluateStepQuorum(ctx context.Context, taskID, sessionID string) (engine.QuorumSnapshot, error)
// ResolveParticipantRole is the AC-2/3/4/4a role-and-seat resolution
// entry point the agent decision tool needs. Reached the same way as
// entry point the runtime decision path needs. Reached the same way as
// the two methods above: via Dispatcher.ResolveParticipantRole plus a
// narrow caller-side type assertion.
ResolveParticipantRole(ctx context.Context, taskID, stepID, agentProfileID string) (role, participantID string, err error)
@@ -9,7 +9,7 @@ import (
// HasActiveStepDecision reports whether an active (non-superseded) row exists
// in workflow_step_decisions for the given task, step, and decider. It backs
// the review/approval completion check that flags a reviewer or approver
// turn that ended without recording a verdict via record_step_decision_kandev.
// turn that ended without recording a workflow-step verdict.
func (r *Repository) HasActiveStepDecision(ctx context.Context, taskID, stepID, deciderID string) (bool, error) {
var exists int
err := r.ro.QueryRowxContext(ctx, r.ro.Rebind(`
+2
View File
@@ -42,6 +42,8 @@ func RegisterAllRoutes(router *gin.RouterGroup, svcs *Services, handoff *taskser
}),
svcs.Skills,
svcs.Workspaces,
newRuntimeDecisionRecorder(svcs.Dashboard),
log,
))
skillsHandler := skills.NewHandler(svcs.Skills)
@@ -27,8 +27,9 @@ const (
CapabilityDeleteSkills = "delete_skills"
)
// AvailableActionRecordStepDecision identifies the MCP decision tool that a
// run prompt may advertise when the agent holds the current workflow seat.
// AvailableActionRecordStepDecision identifies the advisory runtime decision
// action that a run prompt may advertise when the agent holds the current
// workflow seat.
const AvailableActionRecordStepDecision = "record_step_decision"
// Allows reports whether the named runtime capability is granted.
@@ -24,7 +24,7 @@ type RunSnapshotStore interface {
// seat (reviewer or approver) at a task's current workflow step. Implemented
// by *service.Service via HoldsDecisionSeat, mirroring the authorization
// RecordAgentDecision itself applies. The result is advisory prompt metadata;
// the decision tool performs live authorization when it is called.
// the runtime decision endpoint performs live authorization when it is called.
type DecisionSeatResolver interface {
HoldsDecisionSeat(ctx context.Context, taskID, agentProfileID string) (bool, error)
}
@@ -0,0 +1,74 @@
package runtime
import (
"context"
"fmt"
"time"
)
// DecisionRecorder is the composition-boundary dependency for the signed
// Office task decision endpoint.
type DecisionRecorder interface {
RecordAgentDecision(ctx context.Context, input RecordAgentDecisionInput) (RecordAgentDecisionResult, error)
}
// RecordAgentDecisionInput contains only identity resolved from the signed run
// and the two caller-controlled decision fields.
type RecordAgentDecisionInput struct {
TaskID string
AgentProfileID string
SessionID string
Decision string
Reason string
}
// DecisionGuard is the JSON shape shared by the runtime decision response and
// the Office quorum diagnostic response.
type DecisionGuard struct {
TargetStepID string `json:"target_step_id"`
Role string `json:"role"`
Threshold string `json:"threshold"`
RequiredCount int `json:"required_count"`
ReceivedCount int `json:"received_count"`
Satisfied bool `json:"satisfied"`
Reason string `json:"reason,omitempty"`
Error string `json:"error,omitempty"`
}
// RecordAgentDecisionResult is the seven-field task decision contract.
type RecordAgentDecisionResult struct {
Decision string `json:"decision"`
Role string `json:"role"`
StepID string `json:"step_id"`
DecisionID string `json:"decision_id"`
DecidedAt time.Time `json:"decided_at"`
TransitionApplied bool `json:"transition_applied"`
Guards []DecisionGuard `json:"guards"`
}
// DecisionValidationError identifies a caller-correctable decision request.
type DecisionValidationError struct {
Err error
}
func (e *DecisionValidationError) Error() string {
if e == nil || e.Err == nil {
return "invalid agent decision"
}
return e.Err.Error()
}
func (e *DecisionValidationError) Unwrap() error {
if e == nil {
return nil
}
return e.Err
}
// NewDecisionValidationError preserves validation classification at the
// runtime/composition boundary without importing the dashboard package here.
func NewDecisionValidationError(err error) error {
return &DecisionValidationError{Err: err}
}
var errDecisionRecorderMissing = fmt.Errorf("%w: decision recorder", ErrRuntimeDependencyMissing)
@@ -4,11 +4,14 @@ import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"strings"
"github.com/gin-gonic/gin"
"go.uber.org/zap"
commonlogger "github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/office/agents"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/shared"
@@ -20,6 +23,8 @@ type Handler struct {
actions *Actions
skillLister SkillLister
runEvents RunEventAppender
decisions DecisionRecorder
logger *commonlogger.Logger
}
// RunEventAppender records runtime behavior against a run.
@@ -33,18 +38,26 @@ func NewHandler(
actions *Actions,
skillLister SkillLister,
runEvents RunEventAppender,
decisions DecisionRecorder,
log *commonlogger.Logger,
) *Handler {
if log == nil {
log = commonlogger.Default()
}
return &Handler{
agentSvc: agentSvc,
actions: actions,
skillLister: skillLister,
runEvents: runEvents,
decisions: decisions,
logger: log,
}
}
// RegisterRoutes mounts runtime syscall routes.
func RegisterRoutes(group *gin.RouterGroup, h *Handler) {
group.POST("/runtime/comments", h.postComment)
group.POST("/runtime/task/decision", h.recordAgentDecision)
group.POST("/runtime/tasks/:id/status", h.updateTaskStatus)
group.POST("/runtime/tasks/:id/subtasks", h.createSubtask)
group.POST("/runtime/tasks", h.createTask)
@@ -60,6 +73,43 @@ func RegisterRoutes(group *gin.RouterGroup, h *Handler) {
group.DELETE("/runtime/skills/:id", h.deleteSkill)
}
type recordAgentDecisionRequest struct {
Decision string `json:"decision"`
Reason string `json:"reason"`
}
func (h *Handler) recordAgentDecision(c *gin.Context) {
runCtx, _, ok := h.contextFromRequest(c)
if !ok {
return
}
if runCtx.TaskID == "" {
c.JSON(http.StatusBadRequest, gin.H{"error": "decision requires a task-bound runtime"})
return
}
var req recordAgentDecisionRequest
if !bindClosedJSON(c, &req) {
return
}
if h.decisions == nil {
h.respondRuntimeError(c, runCtx, "record_agent_decision", "task", runCtx.TaskID, errDecisionRecorderMissing)
return
}
result, err := h.decisions.RecordAgentDecision(c.Request.Context(), RecordAgentDecisionInput{
TaskID: runCtx.TaskID,
AgentProfileID: runCtx.AgentID,
SessionID: runCtx.SessionID,
Decision: req.Decision,
Reason: req.Reason,
})
if err != nil {
h.respondRuntimeError(c, runCtx, "record_agent_decision", "task", runCtx.TaskID, err)
return
}
h.appendActionRunEvent(c.Request.Context(), runCtx, "record_agent_decision", "task", runCtx.TaskID)
c.JSON(http.StatusOK, result)
}
func (h *Handler) createTask(c *gin.Context) {
runCtx, _, ok := h.contextFromRequest(c)
if !ok {
@@ -392,6 +442,24 @@ func bindJSON(c *gin.Context, target any) bool {
return true
}
func bindClosedJSON(c *gin.Context, target any) bool {
decoder := json.NewDecoder(c.Request.Body)
decoder.DisallowUnknownFields()
if err := decoder.Decode(target); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return false
}
var extra any
if err := decoder.Decode(&extra); err != io.EOF {
if err == nil {
err = errors.New("request body must contain one JSON value")
}
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return false
}
return true
}
func bearerToken(header string) string {
const prefix = "Bearer "
if !strings.HasPrefix(header, prefix) {
@@ -413,12 +481,26 @@ func (h *Handler) respondRuntimeError(
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
var decisionValidation *DecisionValidationError
if errors.As(err, &decisionValidation) {
h.appendDeniedRunEvent(c.Request.Context(), runCtx, action, targetType, targetID, err)
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
if errors.Is(err, shared.ErrForbidden) {
h.appendDeniedRunEvent(c.Request.Context(), runCtx, action, targetType, targetID, err)
c.JSON(http.StatusForbidden, gin.H{"error": err.Error()})
return
}
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
h.logger.Error("office runtime action failed",
zap.String("action", action),
zap.String("target_type", targetType),
zap.String("target_id", targetID),
zap.String("run_id", runCtx.RunID),
zap.String("agent_id", runCtx.AgentID),
zap.Error(err),
)
c.JSON(http.StatusInternalServerError, gin.H{"error": "internal runtime error"})
}
func (h *Handler) respondTaskStatusError(c *gin.Context, runCtx RunContext, taskID string, err error) {
@@ -0,0 +1,158 @@
package runtime
import (
"context"
"encoding/json"
"errors"
"net/http"
"strings"
"testing"
"time"
"github.com/kandev/kandev/internal/office/shared"
)
type recordingDecisionRecorder struct {
inputs []RecordAgentDecisionInput
result RecordAgentDecisionResult
err error
}
func (r *recordingDecisionRecorder) RecordAgentDecision(
_ context.Context,
input RecordAgentDecisionInput,
) (RecordAgentDecisionResult, error) {
r.inputs = append(r.inputs, input)
return r.result, r.err
}
func TestRuntimeHandler_RecordAgentDecisionRejectsCallerIdentityFields(t *testing.T) {
h := newRuntimeHandlerHarness(t, Capabilities{})
resp := h.request(t, http.MethodPost, "/runtime/task/decision", map[string]string{
"decision": "approved",
"reason": "looks good",
"task_id": "forged-task",
})
if resp.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d; body=%s", resp.Code, http.StatusBadRequest, resp.Body.String())
}
}
func TestRuntimeHandler_RecordAgentDecisionUsesSignedRunIdentity(t *testing.T) {
h := newRuntimeHandlerHarness(t, Capabilities{})
h.decisions.result = RecordAgentDecisionResult{
Decision: "approved",
Role: "reviewer",
StepID: "step-1",
DecisionID: "decision-1",
DecidedAt: mustDecisionTime(t, "2026-09-07T20:00:00Z"),
TransitionApplied: true,
Guards: []DecisionGuard{},
}
resp := h.request(t, http.MethodPost, "/runtime/task/decision", map[string]string{
"decision": "approved",
"reason": "looks good",
})
if resp.Code != http.StatusOK {
t.Fatalf("status = %d, want %d; body=%s", resp.Code, http.StatusOK, resp.Body.String())
}
if len(h.decisions.inputs) != 1 {
t.Fatalf("decision inputs = %d, want 1", len(h.decisions.inputs))
}
input := h.decisions.inputs[0]
if input.TaskID != "task-1" || input.AgentProfileID != "agent-1" || input.SessionID != "sess-1" ||
input.Decision != "approved" || input.Reason != "looks good" {
t.Fatalf("decision input = %+v", input)
}
var body RecordAgentDecisionResult
if err := json.Unmarshal(resp.Body.Bytes(), &body); err != nil {
t.Fatalf("decode decision response: %v", err)
}
if body.Decision != "approved" || body.Role != "reviewer" || body.StepID != "step-1" ||
body.DecisionID != "decision-1" || !body.TransitionApplied || body.Guards == nil {
t.Fatalf("decision response = %+v", body)
}
assertActionRunEvent(t, h.runEvents, "record_agent_decision", "task", "task-1")
}
func TestRuntimeHandler_RecordAgentDecisionRejectsTasklessRun(t *testing.T) {
h := newRuntimeHandlerHarness(t, Capabilities{})
token, err := h.agentSvc.MintRuntimeJWT("agent-1", "", "ws-1", "run-1", "sess-1", "")
if err != nil {
t.Fatalf("mint taskless runtime token: %v", err)
}
h.token = token
resp := h.request(t, http.MethodPost, "/runtime/task/decision", map[string]string{
"decision": "approved",
"reason": "looks good",
})
if resp.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d; body=%s", resp.Code, http.StatusBadRequest, resp.Body.String())
}
if !strings.Contains(resp.Body.String(), "decision requires a task-bound runtime") {
t.Fatalf("body = %q, want task-bound runtime error", resp.Body.String())
}
if len(h.decisions.inputs) != 0 {
t.Fatalf("decision inputs = %d, want 0", len(h.decisions.inputs))
}
}
func TestRuntimeHandler_RecordAgentDecisionMapsValidationAndPermissionErrors(t *testing.T) {
tests := []struct {
name string
err error
wantStatus int
wantEvent string
}{
{name: "validation", err: NewDecisionValidationError(errors.New("invalid decision")), wantStatus: http.StatusBadRequest, wantEvent: "runtime.denied"},
{name: "permission", err: shared.ErrForbidden, wantStatus: http.StatusForbidden, wantEvent: "runtime.denied"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
h := newRuntimeHandlerHarness(t, Capabilities{})
h.decisions.err = tc.err
resp := h.request(t, http.MethodPost, "/runtime/task/decision", map[string]string{
"decision": "approved",
"reason": "looks good",
})
if resp.Code != tc.wantStatus {
t.Fatalf("status = %d, want %d; body=%s", resp.Code, tc.wantStatus, resp.Body.String())
}
if len(h.runEvents.events) != 1 || h.runEvents.events[0].eventType != tc.wantEvent {
t.Fatalf("run events = %#v, want one %s event", h.runEvents.events, tc.wantEvent)
}
})
}
}
func TestRuntimeHandler_RecordAgentDecisionHidesUnexpectedErrors(t *testing.T) {
h := newRuntimeHandlerHarness(t, Capabilities{})
h.decisions.err = errors.New("database password leaked")
resp := h.request(t, http.MethodPost, "/runtime/task/decision", map[string]string{
"decision": "approved",
"reason": "looks good",
})
if resp.Code != http.StatusInternalServerError {
t.Fatalf("status = %d, want %d; body=%s", resp.Code, http.StatusInternalServerError, resp.Body.String())
}
if strings.Contains(resp.Body.String(), "database password leaked") {
t.Fatalf("body leaked internal error: %q", resp.Body.String())
}
if !strings.Contains(resp.Body.String(), "internal runtime error") {
t.Fatalf("body = %q, want opaque runtime error", resp.Body.String())
}
}
func mustDecisionTime(t *testing.T, value string) time.Time {
t.Helper()
parsed, err := time.Parse(time.RFC3339, value)
if err != nil {
t.Fatalf("parse decision time: %v", err)
}
return parsed
}
@@ -30,6 +30,7 @@ type handlerHarness struct {
projects *recordingProjectManager
tasks *handlerTaskCreator
runEvents *recordingRunEvents
decisions *recordingDecisionRecorder
agentSvc *agents.AgentService
repository *sqlite.Repository
}
@@ -808,6 +809,7 @@ func newRuntimeHandlerHarnessWithProjectManager(
projectManager = projectManagerFactory(repo)
}
runEvents := &recordingRunEvents{}
decisions := &recordingDecisionRecorder{}
router := gin.New()
RegisterRoutes(router.Group(""), NewHandler(
agentSvc,
@@ -821,6 +823,8 @@ func newRuntimeHandlerHarnessWithProjectManager(
}),
nil,
runEvents,
decisions,
logger.Default(),
))
return &handlerHarness{
router: router,
@@ -830,6 +834,7 @@ func newRuntimeHandlerHarnessWithProjectManager(
projects: projects,
tasks: tasks,
runEvents: runEvents,
decisions: decisions,
agentSvc: agentSvc,
repository: repo,
}
@@ -27,7 +27,7 @@ type decisionSeatDispatcher interface {
// HoldsDecisionSeat reports whether agentProfileID currently holds a
// decision seat (reviewer or approver) at the task's current workflow step.
// It resolves the seat the same way RecordAgentDecision authorizes a real
// record_step_decision_kandev call, so runtime.ContextBuilder can grant the
// runtime decision call, so runtime.ContextBuilder can grant the
// matching capability without over- or under-stating who can actually
// decide.
func (s *Service) HoldsDecisionSeat(ctx context.Context, taskID, agentProfileID string) (bool, error) {
@@ -448,7 +448,7 @@ func (s *Service) handleAgentCompleted(ctx context.Context, event *bus.Event) er
}
// warnIfReviewDecisionMissing flags a review or approval run that finished
// without the agent ever calling record_step_decision_kandev. A reviewer can
// without the agent ever recording a workflow decision. A reviewer can
// post a full critique and reject the work in a comment, but if that comment
// never becomes a recorded decision the workflow engine has nothing to act
// on and the task strands in its current step forever. This does not fix
@@ -290,7 +290,9 @@ func buildReviewStagePrompt(pc *PromptContext) string {
}
b.WriteString("\nReview the implementation carefully. Check for correctness, edge cases, and code quality.\n")
b.WriteString("Submit your verdict: approve if the work is satisfactory, or reject with specific feedback on what needs to change.")
writeDecisionContract(&b)
if hasDecisionAction(pc.AllowedActions) {
writeDecisionContract(&b)
}
return b.String()
}
@@ -305,19 +307,19 @@ func buildApprovalStagePrompt(pc *PromptContext) string {
}
b.WriteString("\nConfirm that the approval requirements are met for this workflow.\n")
b.WriteString("Submit your verdict: approve if the requirements are met, or reject with specific feedback on what needs to change.")
writeDecisionContract(&b)
if hasDecisionAction(pc.AllowedActions) {
writeDecisionContract(&b)
}
return b.String()
}
// writeDecisionContract appends the explicit record_step_decision_kandev contract shared by the
// review and approval stage prompts: a verdict must be recorded via the tool call, which the agent
// must treat as its final action for the turn, since posting a comment alone is not a decision and
// leaves the task stranded in review. The tool call itself does not halt the agent mid-turn (it
// returns an ordinary result), so the prompt must not claim otherwise — it instructs the agent to
// stop on its own after calling it.
// writeDecisionContract appends the task-bound CLI contract shared by the
// review and approval stage prompts. The command must be the final action for
// the turn because a comment alone does not advance the workflow.
func writeDecisionContract(b *strings.Builder) {
b.WriteString("\n\nYou must call the record_step_decision_kandev tool with decision (\"approved\" or \"rejected\") and reason to record your verdict. Make this your final tool call for the turn, then stop.")
b.WriteString(" Posting a comment alone is not a decision and will not advance the task.")
b.WriteString("\n\nUse this command as your final action for the turn:\n")
b.WriteString(`$KANDEV_CLI kandev task decision --decision approved --reason "..."`)
b.WriteString("\nUse `approved` as shown, or replace it with `rejected` when rejecting. Both require a non-empty reason, then stop. Posting a comment or using an approval-inbox command alone does not count as a workflow step decision.")
}
func buildShipStagePrompt(pc *PromptContext) string {
@@ -87,14 +87,10 @@ func TestBuildPrompt_LegacyRunReasonsRemainCompatible(t *testing.T) {
{name: "review started", reason: "review_started", contains: []string{
"You are reviewing",
"Legacy workflow task",
"record_step_decision_kandev",
"Posting a comment alone is not a decision",
}},
{name: "approval started", reason: "approval_started", contains: []string{
"You are approving",
"Legacy workflow task",
"record_step_decision_kandev",
"Posting a comment alone is not a decision",
}},
}
@@ -120,13 +116,14 @@ func TestBuildPrompt_TaskReviewRequestedUsesStageType(t *testing.T) {
StageType: "approval",
TaskIdentifier: "KAN-review",
TaskTitle: "Approve release",
AllowedActions: []string{officeruntime.AvailableActionRecordStepDecision},
})
if !strings.HasPrefix(prompt, "You are approving") {
t.Errorf("task_review_requested approver prompt = %q, want approver framing", prompt)
}
if !strings.Contains(prompt, "record_step_decision_kandev") {
t.Errorf("task_review_requested prompt missing decision tool contract: %q", prompt)
if !strings.Contains(prompt, `$KANDEV_CLI kandev task decision --decision approved --reason "..."`) {
t.Errorf("task_review_requested prompt missing decision CLI contract: %q", prompt)
}
}
@@ -433,8 +430,8 @@ func TestBuildPrompt_ReviewStageAllowedActionsIncludeRecordStepDecision(t *testi
}
prompt := service.BuildPrompt(pc)
if !strings.Contains(prompt, "You must call the record_step_decision_kandev tool") {
t.Fatalf("expected the decision contract sentence:\n%s", prompt)
if !strings.Contains(prompt, `$KANDEV_CLI kandev task decision --decision approved --reason "..."`) {
t.Fatalf("expected the decision CLI contract:\n%s", prompt)
}
allowedIdx := strings.Index(prompt, "- Allowed actions:")
if allowedIdx == -1 {
@@ -446,6 +443,54 @@ func TestBuildPrompt_ReviewStageAllowedActionsIncludeRecordStepDecision(t *testi
}
}
func TestBuildPrompt_DecisionContractUsesCLIAsFinalAction(t *testing.T) {
for _, stageType := range []string{"review", "approval"} {
t.Run(stageType, func(t *testing.T) {
prompt := service.BuildPrompt(&service.PromptContext{
Reason: service.RunReasonTaskAssigned,
TaskIdentifier: "KAN-decision",
TaskTitle: "Decision task",
StageType: stageType,
AllowedActions: []string{officeruntime.AvailableActionRecordStepDecision},
})
if !strings.Contains(prompt, `$KANDEV_CLI kandev task decision --decision approved --reason "..."`) {
t.Fatalf("%s prompt missing CLI decision contract:\n%s", stageType, prompt)
}
if !strings.Contains(prompt, "final action") {
t.Fatalf("%s prompt must make the CLI decision the final action:\n%s", stageType, prompt)
}
if strings.Contains(prompt, "record_step_decision_kandev") {
t.Fatalf("%s prompt still names the retired MCP decision tool:\n%s", stageType, prompt)
}
})
}
}
func TestBuildPrompt_WithoutDecisionActionOmitsOfficeDecisionContract(t *testing.T) {
for _, stageType := range []string{"review", "approval"} {
t.Run(stageType, func(t *testing.T) {
prompt := service.BuildPrompt(&service.PromptContext{
Reason: service.RunReasonTaskAssigned,
TaskIdentifier: "KAN-kanban",
TaskTitle: "Kanban task",
StageType: stageType,
AllowedActions: []string{officeruntime.CapabilityPostComment},
})
for _, fragment := range []string{
"$KANDEV_CLI kandev task decision",
"record_step_decision",
"decision_id",
} {
if strings.Contains(prompt, fragment) {
t.Fatalf("%s prompt must not contain Office decision metadata %q:\n%s", stageType, fragment, prompt)
}
}
})
}
}
func TestBuildPrompt_ApprovalStageUsesNeutralLifecycleLanguage(t *testing.T) {
prompt := service.BuildPrompt(&service.PromptContext{
Reason: service.RunReasonTaskAssigned,
@@ -278,15 +278,6 @@ func (si *SchedulerIntegration) prepareAndLaunch(
ctx context.Context, run *models.Run,
agent *models.AgentInstance, taskID string, execCfg *ExecutorConfig,
) {
// ADR 0005 Wave E: skill + instruction file delivery moved into the
// runtime (internal/agent/runtime/lifecycle/skill). We still build
// the manifest here to extract AGENTS.md content for the prompt and
// to compute the deterministic instructionsDir path the runtime
// will write to. No filesystem side effects from this call.
manifest := si.buildSkillManifest(ctx, agent, defaultWorkspaceName)
instructionsDir, agentsMD := si.resolveInstructionsForPrompt(manifest, execCfg.Type)
si.snapshotRunSkills(ctx, run.ID, manifest, instructionsDir)
runCtx, err := (&officeruntime.ContextBuilder{
Agents: si.svc,
Runs: si.svc.repo,
@@ -307,6 +298,14 @@ func (si *SchedulerIntegration) prepareAndLaunch(
"workspace_id": runCtx.WorkspaceID,
"wake_reason": runCtx.Reason,
})
// ADR 0005 Wave E: skill + instruction file delivery moved into the
// runtime (internal/agent/runtime/lifecycle/skill). We still build
// the manifest here to extract AGENTS.md content for the prompt and
// to compute the deterministic instructionsDir path the runtime
// will write to. No filesystem side effects from this call.
manifest := si.buildSkillManifest(ctx, agent, defaultWorkspaceName, runCtx.AvailableActions...)
instructionsDir, agentsMD := si.resolveInstructionsForPrompt(manifest, execCfg.Type)
si.snapshotRunSkills(ctx, run.ID, manifest, instructionsDir)
token, err := si.mintRuntimeToken(run, agent, runCtx)
if err != nil {
@@ -333,9 +332,10 @@ func (si *SchedulerIntegration) prepareAndLaunch(
zap.Int("env_count", len(env)))
launchCtx := LaunchContext{
Prompt: prompt,
Env: env,
ProfileID: profileID,
Prompt: prompt,
Env: env,
ProfileID: profileID,
AdditionalSkillSlugs: decisionSkillSlugs(runCtx.AvailableActions),
}
// launchAgent returns true only when the adapter was actually invoked.
// When it was, leave the run `claimed` and let the AgentCompleted/
@@ -591,7 +591,9 @@ func (si *SchedulerIntegration) launchAgent(
return launched
}
var err error
if starter, ok := si.svc.taskStarter.(TaskStarterWithEnv); ok {
if starter, ok := si.svc.taskStarter.(TaskStarterWithLaunchContext); ok {
err = starter.StartTaskWithLaunchContext(ctx, taskID, launch.ProfileID, launch)
} else if starter, ok := si.svc.taskStarter.(TaskStarterWithEnv); ok {
err = starter.StartTaskWithEnv(ctx, taskID, launch.ProfileID, "", "", "",
launch.Prompt, "", false, nil, launch.Env)
} else {
@@ -11,6 +11,7 @@ import (
"github.com/kandev/kandev/internal/agentctl/types/streams"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/workflow/engine"
)
// captureDispatcher implements service.RoutingDispatcher and records the
@@ -136,13 +137,24 @@ func TestSchedulerIntegration_RoutingReceivesBuiltPromptAndEnv(t *testing.T) {
}
}
func TestSchedulerIntegration_SeatActionFlowsToPromptOnly(t *testing.T) {
func TestSchedulerIntegration_SeatActionFlowsToPromptAndLaunch(t *testing.T) {
mock := &mockTaskStarter{}
svc := newTestService(t, service.ServiceOptions{TaskStarter: mock})
dispatcher := &captureDispatcher{}
svc.SetRoutingDispatcher(dispatcher)
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{})
ctx := context.Background()
if err := svc.CreateSkill(ctx, &models.Skill{
ID: "decision-skill",
WorkspaceID: "ws-1",
Name: "Step decision",
Slug: "kandev-step-decision",
Content: "decision skill",
Version: "0.42.0",
ContentHash: "decision-skill-hash",
}); err != nil {
t.Fatalf("create decision skill: %v", err)
}
agent := &models.AgentInstance{
ID: "decision-agent-1",
@@ -174,6 +186,10 @@ func TestSchedulerIntegration_SeatActionFlowsToPromptOnly(t *testing.T) {
if allowedIdx == -1 || !containsIgnoreCase(prompt[allowedIdx:], "record_step_decision") {
t.Fatalf("prompt must advertise the seat-derived action: %s", dispatcher.lastCall().Prompt)
}
launch := dispatcher.lastCall()
if len(launch.AdditionalSkillSlugs) != 1 || launch.AdditionalSkillSlugs[0] != "kandev-step-decision" {
t.Fatalf("launch skill additions = %v, want decision skill", launch.AdditionalSkillSlugs)
}
runs, err := svc.ListRuns(ctx, "ws-1")
if err != nil {
@@ -203,6 +219,78 @@ func TestSchedulerIntegration_SeatActionFlowsToPromptOnly(t *testing.T) {
if actions, ok := snapshot["available_actions"].([]any); !ok || len(actions) != 1 || actions[0] != "record_step_decision" {
t.Fatalf("persisted snapshot missing advisory action: %#v", snapshot["available_actions"])
}
snapshots, err := svc.ListRunSkillSnapshotsForTest(ctx, run.ID)
if err != nil {
t.Fatalf("list run skill snapshots: %v", err)
}
if len(snapshots) != 1 || snapshots[0].SkillID != "decision-skill" {
t.Fatalf("decision seat run snapshots = %#v, want decision skill", snapshots)
}
}
func TestSchedulerIntegration_NonSeatRunDoesNotReceiveDecisionSkill(t *testing.T) {
mock := &mockTaskStarter{}
svc := newTestService(t, service.ServiceOptions{TaskStarter: mock})
dispatcher := &captureDispatcher{}
svc.SetRoutingDispatcher(dispatcher)
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{err: engine.ErrParticipantNotFound})
ctx := context.Background()
if err := svc.CreateSkill(ctx, &models.Skill{
ID: "decision-skill",
WorkspaceID: "ws-1",
Name: "Step decision",
Slug: "kandev-step-decision",
Content: "decision skill",
Version: "0.42.0",
ContentHash: "decision-skill-hash",
}); err != nil {
t.Fatalf("create decision skill: %v", err)
}
agent := &models.AgentInstance{
ID: "non-seat-decision-agent",
WorkspaceID: "ws-1",
Name: "non-seat-reviewer",
Role: models.AgentRoleWorker,
Status: models.AgentStatusIdle,
ExecutorPreference: `{"type":"worktree"}`,
}
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
svc.ExecSQL(t, `INSERT INTO workflow_steps (id, stage_type) VALUES (?, ?)`, "step-non-seat", "review")
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, workflow_step_id, title, description, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
"task-non-seat", "ws-1", "step-non-seat", "Non-seat task", "Review the change")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-non-seat"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
service.RunSchedulerTick(svc, ctx)
runs, err := svc.ListRuns(ctx, "ws-1")
if err != nil {
t.Fatalf("list runs: %v", err)
}
var run *models.Run
for _, candidate := range runs {
if candidate.AgentProfileID == agent.ID && candidate.Reason == service.RunReasonTaskAssigned {
run = candidate
break
}
}
if run == nil {
t.Fatal("missing non-seat decision run")
}
snapshots, err := svc.ListRunSkillSnapshotsForTest(ctx, run.ID)
if err != nil {
t.Fatalf("list run skill snapshots: %v", err)
}
if len(snapshots) != 0 {
t.Fatalf("non-seat run snapshots = %#v, want no decision skill", snapshots)
}
}
// TestSchedulerIntegration_RoutingFallThrough_FallsBackToLegacy asserts
@@ -45,6 +45,12 @@ type TaskStarterWithEnv interface {
planMode bool, attachments []v1.MessageAttachment, env map[string]string) error
}
// TaskStarterWithLaunchContext optionally carries the complete Office launch
// context into the agent runtime, including per-run skill additions.
type TaskStarterWithLaunchContext interface {
StartTaskWithLaunchContext(ctx context.Context, taskID string, agentProfileID string, launch LaunchContext) error
}
// LaunchContext mirrors scheduler.LaunchContext so the office.service
// package can carry the Office-built launch context (prompt, env,
// workflow step, attachments, plan-mode, profile) into the routing
@@ -53,15 +59,16 @@ type TaskStarterWithEnv interface {
// The scheduler.RoutingDispatcher implementation translates this to
// the scheduler-side LaunchContext when calling StartTaskWithRoute.
type LaunchContext struct {
ExecutorID string
ExecutorProfileID string
Priority string
Prompt string
WorkflowStepID string
PlanMode bool
Attachments []v1.MessageAttachment
Env map[string]string
ProfileID string
ExecutorID string
ExecutorProfileID string
Priority string
Prompt string
WorkflowStepID string
PlanMode bool
Attachments []v1.MessageAttachment
Env map[string]string
ProfileID string
AdditionalSkillSlugs []string
}
// RoutingDispatcher is the seam the office scheduler integration uses to
@@ -169,6 +176,46 @@ func (f TaskStarterWithEnvFunc) StartTaskWithEnv(ctx context.Context, taskID, ag
priority, prompt, workflowStepID, planMode, attachments, env)
}
// TaskStarterWithLaunchContextFunc adapts a complete launch-context function
// to the TaskStarter interfaces used by the Office scheduler.
type TaskStarterWithLaunchContextFunc func(ctx context.Context, taskID, agentProfileID string, launch LaunchContext) error
// StartTask implements TaskStarter.
func (f TaskStarterWithLaunchContextFunc) StartTask(ctx context.Context, taskID, agentProfileID, executorID,
executorProfileID string, priority string, prompt, workflowStepID string,
planMode bool, attachments []v1.MessageAttachment) error {
return f(ctx, taskID, agentProfileID, LaunchContext{
ExecutorID: executorID,
ExecutorProfileID: executorProfileID,
Priority: priority,
Prompt: prompt,
WorkflowStepID: workflowStepID,
PlanMode: planMode,
Attachments: attachments,
})
}
// StartTaskWithEnv implements TaskStarterWithEnv.
func (f TaskStarterWithLaunchContextFunc) StartTaskWithEnv(ctx context.Context, taskID, agentProfileID, executorID,
executorProfileID string, priority string, prompt, workflowStepID string,
planMode bool, attachments []v1.MessageAttachment, env map[string]string) error {
return f(ctx, taskID, agentProfileID, LaunchContext{
ExecutorID: executorID,
ExecutorProfileID: executorProfileID,
Priority: priority,
Prompt: prompt,
WorkflowStepID: workflowStepID,
PlanMode: planMode,
Attachments: attachments,
Env: env,
})
}
// StartTaskWithLaunchContext implements TaskStarterWithLaunchContext.
func (f TaskStarterWithLaunchContextFunc) StartTaskWithLaunchContext(ctx context.Context, taskID, agentProfileID string, launch LaunchContext) error {
return f(ctx, taskID, agentProfileID, launch)
}
// WorkspaceCreator creates a DB workspace row for kanban compatibility.
// Implemented by the task service or its repository.
type WorkspaceCreator interface {
@@ -6,8 +6,11 @@ import (
"go.uber.org/zap"
"github.com/kandev/kandev/internal/office/models"
officeruntime "github.com/kandev/kandev/internal/office/runtime"
)
const decisionSkillSlug = "kandev-step-decision"
// SkillManifest holds the resolved skills and instructions for an agent session.
// It is a pure data structure built before executor selection, so the delivery
// strategy can adapt to the target executor type.
@@ -40,6 +43,7 @@ type ManifestInstruction struct {
// returning a manifest that can be delivered to any executor type.
func (si *SchedulerIntegration) buildSkillManifest(
ctx context.Context, agent *models.AgentInstance, workspaceSlug string,
availableActions ...string,
) *SkillManifest {
// Wave G: AgentInstance.ID == agent_profiles.id under the unified model.
agentTypeID := si.svc.resolveAgentType(agent.ID)
@@ -50,22 +54,18 @@ func (si *SchedulerIntegration) buildSkillManifest(
ProjectSkillDir: si.svc.resolveProjectSkillDir(agentTypeID),
}
decisionAvailable := hasDecisionAction(availableActions)
// Load desired skills.
slugs := ParseDesiredSlugs(agent.DesiredSkills)
for _, slug := range slugs {
skill, err := si.svc.GetSkillFromConfig(ctx, slug)
if err != nil {
si.logger.Debug("skip skill in manifest",
zap.String("slug", slug), zap.Error(err))
if slug == decisionSkillSlug && !decisionAvailable {
continue
}
manifest.Skills = append(manifest.Skills, ManifestSkill{
ID: skill.ID,
Slug: skill.Slug,
Content: skill.Content,
Version: skill.Version,
ContentHash: skill.ContentHash,
})
si.appendConfiguredSkill(ctx, manifest, slug)
}
if decisionAvailable && !manifestHasSkill(manifest, decisionSkillSlug) {
si.appendConfiguredSkill(ctx, manifest, decisionSkillSlug)
}
// Load instruction files from DB.
@@ -85,3 +85,46 @@ func (si *SchedulerIntegration) buildSkillManifest(
return manifest
}
func (si *SchedulerIntegration) appendConfiguredSkill(
ctx context.Context, manifest *SkillManifest, slug string,
) {
skill, err := si.svc.GetSkillFromConfig(ctx, slug)
if err != nil {
si.logger.Debug("skip skill in manifest",
zap.String("slug", slug), zap.Error(err))
return
}
manifest.Skills = append(manifest.Skills, ManifestSkill{
ID: skill.ID,
Slug: skill.Slug,
Content: skill.Content,
Version: skill.Version,
ContentHash: skill.ContentHash,
})
}
func manifestHasSkill(manifest *SkillManifest, slug string) bool {
for _, skill := range manifest.Skills {
if skill.Slug == slug {
return true
}
}
return false
}
func hasDecisionAction(actions []string) bool {
for _, action := range actions {
if action == officeruntime.AvailableActionRecordStepDecision {
return true
}
}
return false
}
func decisionSkillSlugs(actions []string) []string {
if !hasDecisionAction(actions) {
return nil
}
return []string{decisionSkillSlug}
}
@@ -0,0 +1,76 @@
package service_test
import (
"context"
"testing"
"github.com/kandev/kandev/internal/office/models"
officeruntime "github.com/kandev/kandev/internal/office/runtime"
"github.com/kandev/kandev/internal/office/service"
)
func TestBuildSkillManifest_DecisionSkillSeatBranches(t *testing.T) {
tests := []struct {
name string
desiredSkills string
available []string
wantSkillCount int
}{
{
name: "desired without decision seat",
desiredSkills: `["kandev-step-decision"]`,
wantSkillCount: 0,
},
{
name: "decision seat auto injects skill",
available: []string{officeruntime.AvailableActionRecordStepDecision},
wantSkillCount: 1,
},
{
name: "decision seat keeps desired skill once",
desiredSkills: `["kandev-step-decision"]`,
available: []string{officeruntime.AvailableActionRecordStepDecision},
wantSkillCount: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
svc := newTestService(t)
ctx := context.Background()
if err := svc.CreateSkill(ctx, &models.Skill{
ID: "decision-skill",
WorkspaceID: "ws-1",
Name: "Step decision",
Slug: "kandev-step-decision",
Content: "decision skill",
Version: "0.42.0",
ContentHash: "decision-skill-hash",
}); err != nil {
t.Fatalf("create skill: %v", err)
}
manifest := service.BuildSkillManifestForTest(
service.NewSchedulerIntegration(svc, 0),
ctx,
&models.AgentInstance{
ID: "agent-1",
WorkspaceID: "ws-1",
DesiredSkills: tt.desiredSkills,
},
"ws-1",
tt.available...,
)
count := 0
for _, skill := range manifest.Skills {
if skill.Slug == "kandev-step-decision" {
count++
}
}
if count != tt.wantSkillCount {
t.Fatalf("decision skill count = %d, want %d; manifest = %+v", count, tt.wantSkillCount, manifest.Skills)
}
})
}
}
@@ -193,8 +193,9 @@ func BuildSkillManifestForTest(
ctx context.Context,
agent *models.AgentInstance,
workspaceSlug string,
availableActions ...string,
) *SkillManifest {
return si.buildSkillManifest(ctx, agent, workspaceSlug)
return si.buildSkillManifest(ctx, agent, workspaceSlug, availableActions...)
}
// Skill delivery test helpers were removed in ADR 0005 Wave E along
@@ -345,6 +345,44 @@ func TestBundledSkillCLIExamplesAvoidKnownUnsupportedOperations(t *testing.T) {
}
}
func TestBundledDecisionSkillContract(t *testing.T) {
specs, err := skills.LoadBundledSystemSkills()
if err != nil {
t.Fatalf("LoadBundledSystemSkills: %v", err)
}
for _, spec := range specs {
if spec.Slug != "kandev-step-decision" {
continue
}
if len(spec.DefaultForRoles) != 0 {
t.Fatalf("decision skill default roles = %v, want none", spec.DefaultForRoles)
}
for _, fragment := range []string{
`$KANDEV_CLI kandev task decision --decision approved --reason "..."`,
"approved",
"rejected",
"non-empty reason",
"supersedes",
"decision",
"role",
"step_id",
"decision_id",
"decided_at",
"transition_applied",
"guards",
"comments do not count",
"Approval-inbox commands do not count",
} {
if !strings.Contains(spec.Content, fragment) {
t.Errorf("decision skill missing %q:\n%s", fragment, spec.Content)
}
}
return
}
t.Fatal("kandev-step-decision bundled system skill not found")
}
func TestBundledProtocolSkillDocumentsScopedTaskMessages(t *testing.T) {
specs, err := skills.LoadBundledSystemSkills()
if err != nil {
@@ -431,6 +431,8 @@ type LaunchAgentRequest struct {
Priority string
Metadata map[string]interface{}
Env map[string]string
// AdditionalSkillSlugs are launch-scoped skills selected by Office.
AdditionalSkillSlugs []string
// ApprovedSecretEnvKeys contains repository binding keys that SSH may
// forward in addition to its managed credential allowlist. Values are
// still taken only from Env; the key list is the explicit repository grant.
@@ -594,6 +596,9 @@ type LaunchOptions struct {
McpProfile *mcpprofile.Context
Attachments []v1.MessageAttachment
Env map[string]string
// AdditionalSkillSlugs are materialized for this launch in addition to the
// durable profile selection.
AdditionalSkillSlugs []string
// RouteOverride carries a provider-routing override resolved by the
// office scheduler. When nil, launch behavior is identical to today.
RouteOverride *RouteOverride
@@ -617,14 +622,15 @@ type RouteOverride struct {
// preserve the Office-built prompt and configuration that the legacy
// path receives via StartTaskWithEnv.
type LaunchContext struct {
ExecutorID string
ExecutorProfileID string
Priority string
Prompt string
WorkflowStepID string
PlanMode bool
Attachments []v1.MessageAttachment
Env map[string]string
ExecutorID string
ExecutorProfileID string
Priority string
Prompt string
WorkflowStepID string
PlanMode bool
Attachments []v1.MessageAttachment
Env map[string]string
AdditionalSkillSlugs []string
}
// LaunchAgentResponse contains the result of launching an agent
@@ -1068,6 +1068,8 @@ func TestApplyExecutorRunningMetadata_SkipsSessionScopedKeys(t *testing.T) {
lifecycle.MetadataKeySSHRemoteAgentctlPID,
lifecycle.MetadataKeySSHLocalForwardPort,
lifecycle.MetadataKeySSHRemoteAgentctlURL,
lifecycle.MetadataKeySSHRuntimeAPILocalURL,
lifecycle.MetadataKeySSHRuntimeAPIRemotePort,
lifecycle.MetadataKeyOfficeAgentProfileID,
}
for _, k := range sessionScoped {
@@ -1305,6 +1305,7 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses
}
req.StartAgent = startAgent
mergeEnv(req, opts.Env)
req.AdditionalSkillSlugs = append([]string(nil), opts.AdditionalSkillSlugs...)
if opts.RouteOverride != nil {
req.RouteOverride = opts.RouteOverride
if opts.RouteOverride.ExecutionProfileID == "" {
@@ -1029,6 +1029,15 @@ func (s *Service) StartTaskWithEnv(ctx context.Context, taskID string, agentProf
return s.startTask(ctx, taskID, agentProfileID, executorID, executorProfileID, priority, prompt, workflowStepID, planMode, autoStart, attachments, startTaskOptions{Env: env})
}
// StartTaskWithEnvAndSkills starts a task with launch-scoped environment and
// skills selected by the Office run rather than by the durable profile.
func (s *Service) StartTaskWithEnvAndSkills(ctx context.Context, taskID string, agentProfileID string, executorID string, executorProfileID string, priority string, prompt string, workflowStepID string, planMode, autoStart bool, attachments []v1.MessageAttachment, env map[string]string, additionalSkillSlugs []string) (*executor.TaskExecution, error) {
return s.startTask(ctx, taskID, agentProfileID, executorID, executorProfileID, priority, prompt, workflowStepID, planMode, autoStart, attachments, startTaskOptions{
Env: env,
AdditionalSkillSlugs: append([]string(nil), additionalSkillSlugs...),
})
}
// startTaskOptions carries the optional, server-only launch inputs that only
// some callers supply. Keeping them in one struct avoids growing startTask's
// already long positional parameter list for every new orthogonal concern.
@@ -1039,6 +1048,9 @@ type startTaskOptions struct {
ProfileExplicit bool
// Env holds launch-scoped environment variables for the agent runtime.
Env map[string]string
// AdditionalSkillSlugs are materialized for this launch in addition to the
// profile's durable skill selection.
AdditionalSkillSlugs []string
// Route pins a concrete execution profile chosen by Office provider routing.
Route *executor.RouteOverride
// SpawnOrigin is set when another agent session spawned this launch; it
@@ -1064,7 +1076,11 @@ func (s *Service) StartTaskWithRoute(
_, err := s.startTask(ctx, taskID, agentProfileID,
launch.ExecutorID, launch.ExecutorProfileID, launch.Priority,
launch.Prompt, launch.WorkflowStepID, launch.PlanMode, false,
launch.Attachments, startTaskOptions{Env: launch.Env, Route: &route})
launch.Attachments, startTaskOptions{
Env: launch.Env,
AdditionalSkillSlugs: append([]string(nil), launch.AdditionalSkillSlugs...),
Route: &route,
})
return err
}
@@ -1354,6 +1370,7 @@ func (s *Service) startTask(ctx context.Context, taskID string, agentProfileID s
McpMode: mcpMode,
Attachments: attachments,
Env: env,
AdditionalSkillSlugs: append([]string(nil), opts.AdditionalSkillSlugs...),
RouteOverride: route,
})
if err != nil {
-3
View File
@@ -472,9 +472,6 @@ const (
ActionMCPGetCanvasState = "mcp.get_canvas_state"
ActionMCPSetCanvasState = "mcp.set_canvas_state"
// Office quorum decision recording.
ActionMCPRecordStepDecision = "mcp.record_step_decision"
// Config-mode MCP actions (agent-native configuration)
ActionMCPCreateWorkflow = "mcp.create_workflow"
ActionMCPUpdateWorkflow = "mcp.update_workflow"
@@ -1,4 +1,4 @@
# ADR-2026-09-07-separate-runtime-permissions-and-advisory-actions: Keep MCP availability separate from runtime permissions
# ADR-2026-09-07-separate-runtime-permissions-and-advisory-actions: Keep live advisory actions separate from runtime permissions
**Status:** accepted
**Date:** 2026-09-07
@@ -9,22 +9,33 @@
Office runtime capabilities are enforced permissions for the runtime HTTP
action surface. They are persisted on a run and copied into the agent JWT.
The `record_step_decision_kandev` MCP tool is different. It is always exposed
to the bound session, and the handler performs live workflow-seat
authorization when the agent calls it. A run-launch seat lookup can therefore
describe prompt availability, but it cannot grant or revoke the MCP action.
Workflow-step decision recording is different. It is a task-bound Office CLI
command backed by the runtime HTTP surface, and the handler performs live
workflow-seat authorization when the agent calls it. A run-launch seat lookup
can therefore select prompt and skill guidance, but it cannot grant or revoke
the decision action.
Putting the MCP action in `runtime.Capabilities` makes `Allows` and JWT data
look like authorization even though the MCP handler does not enforce them.
Putting the decision action in `runtime.Capabilities` makes `Allows` and JWT
data look like authorization even though the runtime handler must resolve the
current reviewer or approver seat on every call.
## Decision
Keep `runtime.Capabilities` limited to permissions enforced by the runtime HTTP
handler. Store seat-derived MCP affordances in `RunContext.AvailableActions`.
Keep `runtime.Capabilities` limited to static permissions enforced directly by
the runtime HTTP handlers. Store the seat-derived decision affordance in
`RunContext.AvailableActions`.
The scheduler may merge `AvailableActions` with runtime capability keys when it
renders the prompt. It must not serialize those actions into runtime JWT
capability claims. The MCP handler remains the live authorization boundary.
renders the prompt and may use it to inject a narrow Kandev skill. It must not
serialize those actions into runtime JWT capability claims. The task, session,
and agent come only from the signed run context; the runtime handler resolves
the current workflow step and participant seat live before it delegates to
`DashboardService.RecordAgentDecision`.
The Office MCP registry does not expose `record_step_decision_kandev`. Decision
recording uses `$KANDEV_CLI kandev task decision`, which calls the narrow Office
runtime endpoint. The CLI and endpoint accept only `decision` and `reason`; they
do not accept task, step, role, participant, session, or agent identifiers.
If the seat lookup fails, context construction fails and the scheduler uses its
existing retry path. A missing resolver, missing step, or absent seat produces
@@ -32,23 +43,30 @@ no advertised action.
## Consequences
Prompts can advertise the decision tool without changing the meaning of the
runtime permission model. Run input snapshots retain the launch-time advisory
state for inspection, while JWT claims remain limited to enforced permissions.
Prompts and injected skills can advertise the decision command without changing
the meaning of the runtime permission model. Run input snapshots retain the
launch-time advisory state for inspection, while JWT claims remain limited to
static enforced permissions.
Seat changes after launch can make the prompt stale. This is safe because the
decision handler resolves the current step and participant seat again.
decision handler resolves the current step and participant seat again. Moving
the action out of MCP also removes its tool schema from every Office turn.
Transient seat lookup failures delay a run through the normal retry policy
instead of launching a prompt that requires an unavailable action.
## Alternatives Considered
- Keep `record_step_decision_kandev` in the Office MCP registry: rejected
because every Office turn pays for a decision schema even when the caller
does not occupy a decision seat, while the established Office CLI already
provides a task-bound authenticated transport.
- Keep `record_step_decision` in `Capabilities`: rejected because a false flag
does not block the MCP call and a true flag does not guarantee live authority.
- Enforce the MCP action with the runtime JWT capability: rejected because it
would duplicate workflow-seat authorization and couple the generic runtime
permission model to one tool transport.
would not represent the live participant slate and a true flag would not
guarantee current authority.
- Enforce the CLI action only with the runtime JWT capability: rejected because
it would duplicate workflow-seat authorization and turn a launch-time
snapshot into an authority source.
- Continue on lookup errors and omit the action: rejected because review and
approval prompts still require the decision call, which would create a
contradictory launch state.
@@ -0,0 +1,104 @@
---
created: 2026-09-07
status: in_progress
requirements:
- REQ-TASKS-QUORUM-RECORDING-001
system_design:
- ../../specs/tasks/system-design/workflow-quorum-decision-recording.md
legacy_specs: []
---
# Implementation Plan: Office Decision CLI Transport
## Overview
Move Office workflow-step decisions from the Office MCP catalog to the signed,
task-bound Office runtime CLI. Delivery is sequential: establish and verify the
CLI/API path, teach eligible seat holders through an injected skill and prompt,
then remove the old MCP transport and update its inventories.
## Scope
### In scope
- Add `$KANDEV_CLI kandev task decision` and a closed Office runtime endpoint.
- Derive task, session, and agent identity from the signed run context.
- Delegate through `DashboardService.RecordAgentDecision` and the existing
workflow-engine decision path.
- Inject decision guidance for reviewer and approver seat holders.
- Remove `record_step_decision_kandev` from Office MCP registration, context,
websocket transport, and transport-only tests.
- Update internal and public Office MCP/CLI inventories.
### Out of scope
- Changes to quorum thresholds, seat canonicalization, persistence, reactivity,
or transition semantics.
- Changes to the separate Office approval-inbox command.
- A Kanban decision command or any Kanban MCP profile change.
## Technical approach
### Signed runtime API and CLI
Add `task decision` to `apps/backend/cmd/agentctl/kandev_task.go`. It accepts
only `--decision` and `--reason` and posts to
`/api/v1/office/runtime/task/decision` using the existing runtime client.
The runtime handler derives the task, session, and agent from the validated JWT.
Its request body is closed over `decision` and `reason`, so identity-shaped or
unknown fields fail instead of being ignored. A small adapter at the Office
composition boundary translates runtime DTOs to
`dashboard.RecordAgentDecisionInput` and maps the existing structured result
back to JSON. Validation, live role/seat resolution, persistence, events,
reactivity, and quorum advancement remain in `DashboardService` and the engine.
### Seat-specific skill and prompt
Add the bundled `kandev-step-decision` system skill without assigning it as a
static role default. When `RunContext.AvailableActions` contains the
launch-time decision affordance, the scheduler adds that skill to the run's
manifest. Review and approval prompts name the CLI command as the final action.
The endpoint still performs live authorization, so a stale launch-time seat
snapshot cannot grant access.
### MCP retirement
After the first two work orders pass, remove the Office decision tool group,
tool implementation, websocket action and handler wiring, and transport-only
tests. Keep the service and engine regression suites. Reduce the documented
Office tool count from 12 to 11 and remove the decision tool from first-turn and
public inventories. Add negative assertions for both Office MCP schema absence
and Kanban instruction/metadata isolation.
## Tests
| Acceptance criterion | Evidence |
| --- | --- |
| `AC-TASKS-QUORUM-RECORDING-001.1` | CLI request and seat-specific skill/prompt tests |
| `AC-TASKS-QUORUM-RECORDING-001.2`–`.5` | Runtime identity forwarding plus existing dashboard seat/persistence/quorum tests |
| `AC-TASKS-QUORUM-RECORDING-001.6`–`.8` | CLI/API validation and existing no-write dashboard tests |
| `AC-TASKS-QUORUM-RECORDING-001.9` | Kanban prompt/context isolation regression |
| `AC-TASKS-QUORUM-RECORDING-001.10`–`.11` | Runtime adapter and existing dashboard structured-result tests |
| `AC-TASKS-QUORUM-RECORDING-001.12` | Office MCP inventory and exact first-turn context tests |
## Work orders
- [done] [Task 01: Establish the task-bound decision transport](task-01-task-bound-decision-transport.md)
- [done] [Task 02: Inject the decision skill](task-02-inject-decision-skill.md)
- [done] [Task 03: Retire the decision MCP transport](task-03-retire-decision-mcp.md)
## Verification results
- Task 01: `cd apps/backend && rtk go test ./cmd/agentctl ./internal/office/runtime ./internal/office/dashboard -run 'Test(TaskDecision|RuntimeHandler_RecordAgentDecision|RecordAgentDecision)' -count=1` — passed (18 tests).
- Task 02: `cd apps/backend && rtk go test ./internal/office/configloader ./internal/office/skills ./internal/office/service -run 'Test.*(DecisionSkill|DecisionContract|ReviewStage|ApprovalStage|SkillManifest|SystemSkills)' -count=1` — passed (36 tests); full affected packages passed (564 tests).
- Task 03: `cd apps/backend && rtk go test ./internal/mcp/server ./internal/mcp/handlers ./internal/backendapp -run 'Test.*(ModeOffice|ModeTask|Sysprompt|Decision|HandlerRegistration)' -count=1` — passed (22 tests); public docs tests and validator passed (61 tests, 46 pages).
## Risks
- Importing `office/dashboard` from `office/runtime` would create a package
cycle; the composition adapter must keep DTO translation at `internal/office`.
- A permissive JSON binder could silently ignore a forged `task_id`; the new
endpoint needs a closed request decoder.
- Static role-based skill assignment cannot represent live participant seats;
injection must follow the seat-derived advisory action and retain live checks.
@@ -0,0 +1,96 @@
---
id: "01-task-bound-decision-transport"
title: "Establish the task-bound decision transport"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-QUORUM-RECORDING-001
acceptance_criteria:
- AC-TASKS-QUORUM-RECORDING-001.2
- AC-TASKS-QUORUM-RECORDING-001.3
- AC-TASKS-QUORUM-RECORDING-001.4
- AC-TASKS-QUORUM-RECORDING-001.5
- AC-TASKS-QUORUM-RECORDING-001.6
- AC-TASKS-QUORUM-RECORDING-001.7
- AC-TASKS-QUORUM-RECORDING-001.8
- AC-TASKS-QUORUM-RECORDING-001.10
- AC-TASKS-QUORUM-RECORDING-001.11
system_design:
- ../../specs/tasks/system-design/workflow-quorum-decision-recording.md
---
# Task 01: Establish the Task-Bound Decision Transport
## Summary
Add the singular `task decision` CLI command and its signed Office runtime API.
Keep the handler thin and route the trusted run identity into the existing
dashboard and workflow-engine decision path.
## In scope
- Add CLI flag validation and JSON request handling.
- Add the closed runtime endpoint with no caller-selectable identity fields.
- Add an acyclic Office composition adapter for `DashboardService`.
- Cover trusted identity forwarding, cross-task input rejection,
non-participant denial, structured results, and no-write validation failures.
## Out of scope
- Prompt, skill, or MCP registry changes.
- Any change to dashboard or engine quorum semantics.
## Acceptance
- The CLI accepts only `approved|rejected` plus a non-empty reason and posts no
task, step, role, participant, session, or agent identifier.
- The API derives all identity from the signed run and rejects forged identity
fields and callers without a live decision seat.
- Reviewer and approver decisions still persist under the canonical live seat,
return all seven fields, and can advance quorum through the existing service.
## Verification
```bash
cd apps/backend && rtk go test ./cmd/agentctl ./internal/office/runtime ./internal/office/dashboard -run 'Test(TaskDecision|RuntimeHandler_RecordAgentDecision|RecordAgentDecision)' -count=1
```
## Files likely touched
- `apps/backend/cmd/agentctl/kandev_task.go`
- `apps/backend/cmd/agentctl/kandev_test.go`
- `apps/backend/internal/office/runtime/handler.go`
- `apps/backend/internal/office/runtime/handler_test.go`
- `apps/backend/internal/office/runtime/errors.go`
- `apps/backend/internal/office/routes.go`
- `apps/backend/internal/office/decision_adapter.go`
- `apps/backend/internal/office/dashboard/agent_decisions_test.go`
## Dependencies
None.
## Risks
- The runtime/dashboard package cycle requires DTO translation at the Office
composition root.
- Unknown JSON fields must fail closed for cross-task regression coverage.
## Parallelism
`sequential`
## Inputs
- `REQ-TASKS-QUORUM-RECORDING-001` and the agent command contract.
- Existing runtime handler, CLI client, and dashboard decision tests.
## Results
- Added the signed `kandev task decision` CLI command with local verdict/reason validation.
- Added the closed `POST /api/v1/office/runtime/task/decision` endpoint.
- Added the Office composition adapter to `DashboardService.RecordAgentDecision`.
- Verified signed task/session/agent identity forwarding, forged identity rejection, structured results, validation and permission errors, and dashboard decision regressions.
- Verification passed: `cd apps/backend && rtk go test ./cmd/agentctl ./internal/office/runtime ./internal/office/dashboard -run 'Test(TaskDecision|RuntimeHandler_RecordAgentDecision|RecordAgentDecision)' -count=1` (18 tests).
@@ -0,0 +1,101 @@
---
id: "02-inject-decision-skill"
title: "Inject the decision skill"
status: done
wave: 2
depends_on:
- "01-task-bound-decision-transport"
plan: "plan.md"
requirements:
- REQ-TASKS-QUORUM-RECORDING-001
acceptance_criteria:
- AC-TASKS-QUORUM-RECORDING-001.1
- AC-TASKS-QUORUM-RECORDING-001.3
- AC-TASKS-QUORUM-RECORDING-001.9
system_design:
- ../../specs/tasks/system-design/workflow-quorum-decision-recording.md
---
# Task 02: Inject the Decision Skill
## Summary
Teach decision-seat holders the CLI contract through a bundled system skill.
Select that skill from the seat-derived advisory action and make the review and
approval prompts require the command as their final action.
## In scope
- Add `kandev-step-decision/SKILL.md` with no static role default.
- Append it to the run manifest only when the launch-time seat lookup grants
`record_step_decision` as an advisory action.
- Update review and approval prompts from the MCP tool to the CLI command.
- Prove normal Kanban sessions receive no decision CLI skill or instruction.
## Out of scope
- Treating launch-time `AvailableActions` or JWT claims as authorization.
- Removing the MCP transport before the replacement guidance is green.
## Acceptance
- A reviewer or approver seat run receives the decision skill and final-action
CLI prompt; a non-seat Office run does not receive the skill.
- The skill explains accepted verdicts, required reason, superseding repeats,
structured output, and that comments or approval-inbox commands do not count.
- Kanban context and prompts contain no Office decision CLI instruction or
decision metadata.
## Verification
```bash
cd apps/backend && rtk go test ./internal/office/configloader ./internal/office/skills ./internal/office/service -run 'Test.*(DecisionSkill|DecisionContract|ReviewStage|ApprovalStage|SkillManifest|SystemSkills)' -count=1
```
## Files likely touched
- `apps/backend/internal/office/configloader/skills/kandev-step-decision/SKILL.md`
- `apps/backend/internal/office/skills/system_sync_test.go`
- `apps/backend/internal/office/service/skill_manifest.go`
- `apps/backend/internal/office/service/scheduler_integration.go`
- `apps/backend/internal/office/service/prompt_builder.go`
- `apps/backend/internal/office/service/prompt_builder_test.go`
- `apps/backend/internal/office/service/scheduler_integration_routing_test.go`
- `apps/backend/internal/mcp/server/sysprompt_sync_test.go`
## Dependencies
- Task 01 provides the command that the skill teaches.
## Risks
- Static role defaults do not match workflow participant seats; selection must
use the existing live-seat advisory lookup at launch.
- Prompt wording must not imply that launch-time skill availability grants
authority after the participant slate changes.
## Parallelism
`sequential`
## Inputs
- The accepted advisory-actions ADR.
- Existing bundled skill synchronization and scheduler manifest pipeline.
## Results
Implemented the bundled `kandev-step-decision` skill without static role
defaults. The scheduler now builds the signed run context before resolving the
manifest, injects the skill only when the launch-time advisory decision action
is present, and filters the skill from non-seat runs. Review and approval
prompts use the task-bound CLI command as the final action and omit the Office
decision contract when the action is absent.
Verification:
```text
cd apps/backend && rtk go test ./internal/office/configloader ./internal/office/skills ./internal/office/service -run 'Test.*(DecisionSkill|DecisionContract|ReviewStage|ApprovalStage|SkillManifest|SystemSkills)' -count=1
```
Passed: 36 tests. The full affected package run also passed 564 tests.
@@ -0,0 +1,107 @@
---
id: "03-retire-decision-mcp"
title: "Retire the decision MCP transport"
status: done
wave: 3
depends_on:
- "02-inject-decision-skill"
plan: "plan.md"
requirements:
- REQ-TASKS-QUORUM-RECORDING-001
acceptance_criteria:
- AC-TASKS-QUORUM-RECORDING-001.9
- AC-TASKS-QUORUM-RECORDING-001.12
system_design:
- ../../specs/tasks/system-design/workflow-quorum-decision-recording.md
---
# Task 03: Retire the Decision MCP Transport
## Summary
Remove the now-replaced Office MCP schema and websocket transport. Preserve the
normal Kanban catalog and update internal and public Office inventories to the
11-tool surface plus the task-bound CLI command.
## In scope
- Remove the Office decision tool group and implementation.
- Remove its websocket action, handler dependency/wiring, and transport-only
test files.
- Update exact Office context/tool-count tests and add negative schema checks.
- Update the public Office MCP/runtime CLI reference and coverage inventory.
## Out of scope
- Removing `DashboardService.RecordAgentDecision` or its engine tests.
- Changing `step_complete_kandev` or any normal Kanban MCP registration.
- Reusing or changing `agentctl kandev approvals decide`.
## Acceptance
- Office `tools/list` and first-turn context omit
`record_step_decision_kandev`; the Office tool count is 11.
- Normal Kanban tool registration is byte-for-byte unchanged in intent and has
no Office decision metadata or instructions.
- No dead websocket decision action, handler, dashboard dependency, or
transport-only test remains.
## Verification
```bash
cd apps/backend && rtk go test ./internal/mcp/server ./internal/mcp/handlers ./internal/backendapp -run 'Test.*(ModeOffice|ModeTask|Sysprompt|Decision|HandlerRegistration)' -count=1
cd ../.. && rtk node --test scripts/validate-public-docs.test.mjs && rtk node scripts/validate-public-docs.mjs
```
## Files likely touched
- `apps/backend/config/prompts/office-context.md`
- `apps/backend/internal/mcp/server/server.go`
- `apps/backend/internal/mcp/server/server_test.go`
- `apps/backend/internal/mcp/server/sysprompt_sync_test.go`
- `apps/backend/internal/mcp/server/agent_decision_tool.go`
- `apps/backend/internal/mcp/server/agent_decision_tool_test.go`
- `apps/backend/internal/mcp/handlers/handlers.go`
- `apps/backend/internal/mcp/handlers/agent_decision_handlers.go`
- `apps/backend/internal/mcp/handlers/agent_decision_handlers_test.go`
- `apps/backend/internal/backendapp/helpers.go`
- `apps/backend/pkg/websocket/actions.go`
- `docs/public/automation-and-mcp.md`
- `docs/public/coverage.json`
## Dependencies
- Task 02 verifies the replacement skill and prompt path before deletion.
## Risks
- Exact context-inventory tests must change with the registry so documentation
cannot drift from `tools/list`.
- Removing shared handler wiring must not affect other Office or Kanban tools.
## Parallelism
`sequential`
## Inputs
- The verified outputs of Tasks 01 and 02.
- Current Office MCP registry, prompt inventory, and public CLI reference.
## Results
Removed the Office decision MCP tool, websocket action, handler dependency and
composition wiring, plus their transport-only tests. The Office context and
public reference now describe the 11-tool catalog and the task-bound CLI
decision command. Kanban registration remains unchanged and negative tests
cover the absence of Office decision metadata from both surfaces.
Verification:
```text
cd apps/backend && rtk go test ./internal/mcp/server ./internal/mcp/handlers ./internal/backendapp -run 'Test.*(ModeOffice|ModeTask|Sysprompt|Decision|HandlerRegistration)' -count=1
cd ../.. && rtk node --test scripts/validate-public-docs.test.mjs && rtk node scripts/validate-public-docs.mjs
```
Passed: 22 backend tests, 61 public-doc tests, and validation of 46 published
docs pages.
+15 -3
View File
@@ -582,12 +582,11 @@ Office runs use a smaller MCP surface than regular task-mode sessions. The built
- `ask_user_question_kandev`;
- `create_task_plan_kandev`, `get_task_plan_kandev`, `update_task_plan_kandev`, and `delete_task_plan_kandev`;
- `list_related_tasks_kandev`;
- `list_task_documents_kandev`, `get_task_document_kandev`, and `write_task_document_kandev`.
- `list_task_documents_kandev`, `get_task_document_kandev`, and `write_task_document_kandev`;
- `show_rich_output_kandev`;
- `record_step_decision_kandev` records an `approved` or `rejected` verdict for the current workflow step. It requires a non-empty reason, and a later verdict supersedes the earlier one.
- `step_complete_kandev`, per ADR 0015: Kandev includes its completion instruction, and acts on its signal, only on Office steps whose auto-advance action explicitly requires that signal (office-default's `work` step is one such step).
These tools cover human questions, the current task plan, related-task discovery, task documents, quorum decisions, and the step-completion signal. Office state changes use the injected `$KANDEV_CLI kandev ...` commands instead. An Office agent should not search for additional Kandev MCP tools: Kanban/configuration tools are task-mode only and are not registered in Office mode.
These tools cover human questions, the current task plan, related-task discovery, task documents, and the step-completion signal. Office state changes use the injected `$KANDEV_CLI kandev ...` commands instead. An Office agent should not search for additional Kandev MCP tools: Kanban/configuration tools are task-mode only and are not registered in Office mode.
### Runtime credentials
@@ -603,6 +602,19 @@ missing, do not set either variable yourself. A regular task session should use
its injected Kandev MCP tools. An Office-owned task must be started or woken
through Office so the scheduler can supply its signed runtime context.
Reviewers and approvers record a workflow-step verdict through the task-bound
runtime CLI. The command accepts only `approved` or `rejected` and requires a
non-empty reason:
```bash
$KANDEV_CLI kandev task decision --decision approved --reason "..."
```
The runtime derives the task, session, and agent identity from the signed run
context. A repeated decision supersedes the earlier decision for that
participant and step. Comments and approval-inbox commands do not record a
workflow-step verdict.
An Office run can inspect the projects in its current workspace:
```bash
+6 -5
View File
@@ -130,16 +130,17 @@
"tasks-and-workflows"
],
"sources": [
"apps/backend/internal/mcp/server/agent_decision_tool.go",
"apps/backend/cmd/agentctl/kandev_task.go",
"apps/backend/internal/office/configloader/skills/kandev-step-decision/SKILL.md",
"apps/backend/internal/office/runtime/handler.go",
"apps/backend/internal/office/dashboard/agent_decisions.go"
],
"tests": [
"apps/backend/internal/mcp/server/agent_decision_tool_test.go",
"apps/backend/cmd/agentctl/kandev_task_decision_test.go",
"apps/backend/internal/office/runtime/handler_decision_test.go",
"apps/backend/internal/office/dashboard/agent_decisions_test.go"
],
"mcpTools": [
"record_step_decision_kandev"
]
"mcpTools": []
},
{
"id": "task-lifecycle",
+8 -4
View File
@@ -246,7 +246,7 @@ Three MCP modes coexist:
|---|---|---|---|
| `ModeTask` | 27 (kanban + plans + walkthroughs + coordination + completion) | ~3-5K | Interactive kanban sessions |
| `ModeConfig` | 29 (workflows + agents + executors) | ~8-10K | Config setup sessions |
| `ModeOffice` | 12 (plans + tasks + rich output + decisions + completion) | ~1-2K | Office agent sessions |
| `ModeOffice` | 11 (plans + tasks + rich output + completion) | ~1-2K, without the decision schema | Office agent sessions |
Agent routing and Office ownership are independent. Workflow-level defaults, per-step agent profiles, and `runner` participants select the execution identity only; they never make a Kanban task Office-owned. A task is Office-owned only when it is linked to an Office project or its workflow matches the workspace's Office workflow.
@@ -255,13 +255,17 @@ Agent routing and Office ownership are independent. Workflow-level defaults, per
- `ask_user_question_kandev` (only meaningful when the user opens the task in advanced mode).
- `list_related_tasks_kandev`.
- 3 task-document tools (`list_task_documents_kandev`, `get_task_document_kandev`, `write_task_document_kandev`).
- `show_rich_output_kandev`, `record_step_decision_kandev`, and gated `step_complete_kandev`.
- `show_rich_output_kandev` and gated `step_complete_kandev`.
`ModeOffice` excludes kanban/config tools and workspace/workflow listing tools. Its first-turn context lists only registered tools, and Office mutations use `$KANDEV_CLI`.
`ModeOffice` excludes kanban/config tools, workspace/workflow listing tools, and
the workflow-step decision action. Its first-turn context lists only registered
tools. Reviewer and approver runs receive an injected Kandev skill for
`$KANDEV_CLI kandev task decision`; other Office mutations also use
`$KANDEV_CLI`.
### Skills are preferred over MCP tools
Skills are the preferred pattern for teaching agents office capabilities. A skill provides instructions in `SKILL.md` and the agent calls API endpoints via `$KANDEV_CLI`. This is cheaper than MCP tools: instructions read once per session, shell calls thereafter; MCP tool definitions add per-call overhead (tool schemas in context, structured I/O parsing on every invocation). The `kandev-protocol` system skill teaches CLI usage and replaces the earlier curl-based version. New office capabilities expose API endpoints, ship a skill that teaches the agent how to call them, and assign the skill to agents that need it.
Skills are the preferred pattern for teaching agents office capabilities. A skill provides instructions in `SKILL.md` and the agent calls API endpoints via `$KANDEV_CLI`. This is cheaper than MCP tools: instructions read once per session, shell calls thereafter; MCP tool definitions add per-turn schema cost and structured I/O parsing on every invocation. The `kandev-protocol` system skill teaches general CLI usage and replaces the earlier curl-based version. The narrow `kandev-step-decision` system skill is injected when the launch-time seat lookup finds a reviewer or approver seat; the runtime endpoint still authorizes the seat live on every call. New office capabilities expose API endpoints, ship a skill that teaches the agent how to call them, and assign or inject the skill only where it is useful.
## UI
@@ -261,7 +261,7 @@ Recovery and runtime-action authorization splits across three actors: human user
- **Mark fixed (inbox dismiss + retry)** and **Resume session**: any workspace user. There is no agent-role gating on these affordances; they are explicit human-driven recovery actions surfaced in the inbox and per-task chat.
- **Reassignment** (`PATCH /tasks/:id` with new `assignee_agent_instance_id`): workspace user. Agent JWTs cannot reassign tasks via the runtime action surface in v1; there is no `CapabilityReassignTask`.
- **Manual unpause** (clearing `pause_reason` from the agent detail page): workspace admin / CEO role only, enforced by `isAdminRole` in `internal/office/agents/handler.go`.
- **Runtime action surface capabilities**: derived from `agent_instances.role` / `permissions` via `runtime.FromAgent`. The agent JWT carries the serialized `Capabilities` snapshot taken at run-claim time; subsequent permission revocations on the agent profile do not affect a JWT already in flight (it expires after `DefaultTokenDuration = 4h`). The seat-derived `record_step_decision` MCP action is not a runtime capability. `runtime.ContextBuilder.Build` stores it in `RunContext.AvailableActions` when the agent holds the current workflow participant seat. The scheduler adds this advisory action to the prompt, but not to the JWT capability claims. `handleRecordStepDecision` performs live authorization with `ResolveParticipantRole`.
- **Runtime action surface capabilities**: derived from `agent_instances.role` / `permissions` via `runtime.FromAgent`. The agent JWT carries the serialized `Capabilities` snapshot taken at run-claim time; subsequent permission revocations on the agent profile do not affect a JWT already in flight (it expires after `DefaultTokenDuration = 4h`). The seat-derived `record_step_decision` CLI affordance is not a runtime capability. `runtime.ContextBuilder.Build` stores it in `RunContext.AvailableActions` when the agent holds the current workflow participant seat. The scheduler uses this advisory action to inject `kandev-step-decision` and add the command to the prompt, but does not add it to JWT capability claims. The task-bound runtime decision handler derives task, session, and agent identity from the signed run context and performs live authorization through `DashboardService.RecordAgentDecision` and `ResolveParticipantRole`.
- **Task scope**: `RunContext.CanMutateTask(taskID)` returns true only when `taskID == runCtx.TaskID` or `taskID` (or wildcard `*`) appears in `Capabilities.AllowedTaskIDs`. Out-of-scope mutations return `ErrTaskOutOfScope`.
- **Workspace scope**: every action that touches another entity (target agent, target skill, target task) is rejected with `ErrWorkspaceOutOfScope` when the target's `workspace_id` does not match the run's.
- **Memory namespaces**: `CanAccessMemory` enforces workspace match plus, for `kind=agent`, that the namespace ID matches `runCtx.AgentID`. Agents cannot read or write another agent's memory.
@@ -32,9 +32,11 @@ An Office participant can submit a validated verdict with a reason, and the resu
- **AC-TASKS-QUORUM-RECORDING-001.1:** WHEN an Office agent session is attached to the task's current step
with role `reviewer` or `approver` and `decision_required = 1`, THE SYSTEM
SHALL expose an MCP tool that records a decision, accepting a verdict of
`approved` or `rejected` and a required non-empty reason.
- **AC-TASKS-QUORUM-RECORDING-001.2:** WHEN that tool is called with a valid verdict, THE SYSTEM SHALL write
SHALL inject a Kandev skill and stage prompt that require the task-bound
`$KANDEV_CLI kandev task decision` command as the agent's final action. The
command SHALL accept a verdict of `approved` or `rejected` and a required
non-empty reason.
- **AC-TASKS-QUORUM-RECORDING-001.2:** WHEN that command is called with a valid verdict, THE SYSTEM SHALL write
one row to `workflow_step_decisions` with `task_id` = the calling session's
task, `step_id` = that task's current `workflow_step_id`, `role` = the
caller's resolved role, `decider_type` = `agent`, `decider_id` = the caller's
@@ -71,16 +73,18 @@ An Office participant can submit a validated verdict with a reason, and the resu
an approver attached at an earlier step is currently unresolvable at the step
where the approval is due; role resolution and slate membership SHALL NOT
disagree about who participates.
- **AC-TASKS-QUORUM-RECORDING-001.6:** WHEN the verdict is any value other than `approved` or `rejected`,
- **AC-TASKS-QUORUM-RECORDING-001.6:** WHEN the command or its Office API receives a verdict other than `approved` or `rejected`,
THE SYSTEM SHALL reject the call with a validation error and write no row.
- **AC-TASKS-QUORUM-RECORDING-001.7:** WHEN the reason is empty or whitespace-only, THE SYSTEM SHALL reject
- **AC-TASKS-QUORUM-RECORDING-001.7:** WHEN the command or its Office API receives an empty or whitespace-only reason, THE SYSTEM SHALL reject
the call with a validation error and write no row.
- **AC-TASKS-QUORUM-RECORDING-001.8:** WHEN the task has no `workflow_step_id` bound, THE SYSTEM SHALL
reject the call with an error naming the unbound step and write no row.
- **AC-TASKS-QUORUM-RECORDING-001.9:** THE SYSTEM SHALL NOT expose the decision tool to sessions whose
profile does not enable `SurfaceOfficeTask`.
- **AC-TASKS-QUORUM-RECORDING-001.9:** THE SYSTEM SHALL NOT expose decision MCP
metadata, the Office decision skill, or Office decision instructions to a
normal Kanban session. The task-bound command SHALL be available only inside
a scheduler-prepared, signed Office run context.
- **AC-TASKS-QUORUM-RECORDING-001.10:** THE SYSTEM SHALL validate the decision tool's preconditions in this
- **AC-TASKS-QUORUM-RECORDING-001.10:** THE SYSTEM SHALL validate the Office decision API's preconditions in this
order: (1) the task has a bound `workflow_step_id` (AC-TASKS-QUORUM-RECORDING-001.8), (2) the caller
resolves to a participant with `decision_required = 1` for `reviewer` or
`approver` (AC-TASKS-QUORUM-RECORDING-001.3), (3) the verdict is `approved` or `rejected` (AC-TASKS-QUORUM-RECORDING-001.6), (4) the
@@ -93,8 +97,8 @@ An Office participant can submit a validated verdict with a reason, and the resu
both preconditions are false at once in that case and the two errors differ in
diagnostic value.
- **AC-TASKS-QUORUM-RECORDING-001.11:** WHEN the decision tool returns successfully, THE SYSTEM SHALL return
the seven fields enumerated in `## Tool contract` — `decision`, `role`,
- **AC-TASKS-QUORUM-RECORDING-001.11:** WHEN the decision command returns successfully, THE SYSTEM SHALL return
the seven fields enumerated in the system design's agent command contract: `decision`, `role`,
`step_id`, `decision_id`, `decided_at`, `transition_applied` and `guards` —
and SHALL NOT return a scalar `required_count` or `received_count`.
`guards` SHALL equal the AC-TASKS-QUORUM-REEVALUATION-001.14 snapshot's `Guards` for the validated step,
@@ -111,6 +115,11 @@ An Office participant can submit a validated verdict with a reason, and the resu
and `transition_applied`, via AC-TASKS-QUORUM-BINDING-001.2 and AC-TASKS-QUORUM-CONCURRENCY-001.4 — were observable by any
acceptance criterion, so a wrong `role` or a wrong count could ship untested.
- **AC-TASKS-QUORUM-RECORDING-001.12:** WHEN an Office session lists its MCP
tools after the CLI migration, THE SYSTEM SHALL NOT include
`record_step_decision_kandev` or its JSON schema. Normal Kanban MCP profiles
SHALL remain unchanged by this migration.
## Out of scope
@@ -22,7 +22,7 @@ requirements:
The task system owns quorum semantics, participant canonicalization, decision persistence, guarded transition application, and diagnostic projections. Office and MCP surfaces are transports over that engine-owned contract.
The task system owns quorum semantics, participant canonicalization, decision persistence, guarded transition application, and diagnostic projections. The task-bound Office CLI and runtime API are transports over that engine-owned contract.
@@ -70,18 +70,19 @@ The engine's evaluation is therefore correct **given its inputs**. With zero
decision rows, `all_approve` and `any_reject` both correctly evaluate false and
the card correctly stays put. The defects are all on the input side.
### Defect 1 — an agent has no tool to record a decision
### Transport follow-up — remove the per-turn decision schema
The MCP tool registry (`apps/backend/internal/mcp/server/server.go:892-923`)
enables exactly four groups for `SurfaceOfficeTask`: `plan` (4 tools),
`related-tasks` (1), `office-documents` (3), and the capability-gated
`user-question` (1). Nine tools, none of which records a decision.
The first agent surface added `record_step_decision_kandev` to
`SurfaceOfficeTask`. That proved the server-owned decision and quorum path, but
it also makes every Office turn carry a decision JSON schema, including turns
whose agents do not occupy a reviewer or approver seat.
`POST /api/v1/office/tasks/:id/approve` and `.../request-changes` exist
(`office/dashboard/handler.go:91-92`) and are reachable from the human UI
(`apps/web/components/task/simple/components/approval-action-bar.tsx:122-123`),
but no agent can reach either. A reviewer agent's only means of expressing a
verdict is a free-text comment, which nothing parses.
Office already uses a signed, task-bound runtime CLI for its mutation surface.
The final transport moves decision recording to that established path while
retaining `DashboardService.RecordAgentDecision` and the workflow engine as the
only business and transition path. The MCP registration, websocket transport,
and transport-only tests are removed only after the CLI, runtime endpoint, and
injected skill are covered.
### Defect 2 — the human path writes a verdict the evaluator cannot read
@@ -183,14 +184,19 @@ which `ConfigTransitionGuard` accepts (`workflow/engine/types.go:533`):
Approval is the same shape with `role: approver`.
## Tool contract
## Agent command contract
Pinned here because Build would otherwise invent it and Review would relitigate it.
**Name.** `record_step_decision_kandev`, registered under a new
`office-decisions` group in the profile registry
(`apps/backend/internal/mcp/server/server.go:892-923`), enabled for
`SurfaceOfficeTask` only.
**Command.** `$KANDEV_CLI kandev task decision --decision approved|rejected
--reason "..."`. The command is taught through the injected
`kandev-step-decision` system skill and repeated in the review and approval
stage prompts as the required final action.
**HTTP endpoint.** `POST /api/v1/office/runtime/task/decision` with a closed JSON
body containing only `decision` and `reason`. The route uses the existing
Office runtime bearer-token middleware and does not expose a task id in its
path. Identity-shaped body fields are rejected rather than ignored.
**Arguments.**
@@ -200,16 +206,27 @@ Pinned here because Build would otherwise invent it and Review would relitigate
| `reason` | yes | non-empty string |
No idempotency argument. A retry is a new verdict that supersedes the previous
one under AC-TASKS-QUORUM-CONCURRENCY-001.2; the tool is deliberately NOT idempotent across calls, and
one under AC-TASKS-QUORUM-CONCURRENCY-001.2; the command is deliberately NOT idempotent across calls, and
AC-TASKS-QUORUM-CONCURRENCY-001.7 states this as contract so a builder does not invent a client-supplied
key. Idempotency in this feature is confined to transition application
(AC-TASKS-QUORUM-CONCURRENCY-001.6, AC-TASKS-QUORUM-CONCURRENCY-001.4).
**Scoping.** The tool takes no `task_id`. It resolves the task and step from the
calling session, matching every other Office tool. An agent cannot record a
decision on a task other than the one its session is bound to.
**Scoping.** The command and endpoint accept no task, step, role, participant,
session, or agent identifier. The runtime handler derives `task_id`,
`session_id`, and `agent_profile_id` from the validated signed Office run
context. `DashboardService.RecordAgentDecision` resolves the task's current
workflow step and the caller's canonical reviewer or approver seat live on
every call. A token bound to one task cannot address another task, and a caller
that no longer holds a decision seat is rejected without a write.
**Return.** On success the tool returns exactly these seven fields, observed by
**Transport boundary.** The runtime handler remains thin. A small composition
adapter translates the runtime request and response types and calls
`DashboardService.RecordAgentDecision`; the dashboard service continues to own
validation order, decision publication, activity, reactivity, and delegation
to the workflow engine's decision path. The runtime layer does not implement a
second quorum or persistence path.
**Return.** On success the command returns exactly these seven fields, observed by
AC-TASKS-QUORUM-RECORDING-001.11:
| Field | Meaning |
@@ -264,6 +281,11 @@ approver precedence, as required by AC-TASKS-QUORUM-RECORDING-001.4a. The two
paths can therefore persist different roles for the same caller, task, and
step. Future implementations must preserve this boundary.
**MCP boundary.** `record_step_decision_kandev` is absent from the
`SurfaceOfficeTask` registry and first-turn MCP inventory after this command is
wired. Normal Kanban profiles remain unchanged and never receive the Office
decision skill or CLI instruction.
**Reason column.** The reason is written to `workflow_step_decisions.comment`,
the column every Office reader already projects (`office/dashboard/decisions.go:141,428`).
It is NOT written to `note`, which `Engine.RecordParticipantDecision` currently
@@ -277,7 +299,7 @@ task timeline — this is the concrete bug this paragraph exists to prevent.
| Condition | Behavior |
|---|---|
| Agent not a participant | Permission error; no row written (AC-TASKS-QUORUM-RECORDING-001.3) |
| Verdict outside `approved`/`rejected` on the tool | Validation error; no row (AC-TASKS-QUORUM-RECORDING-001.6) |
| Verdict outside `approved`/`rejected` on the command or API | Validation error; no row (AC-TASKS-QUORUM-RECORDING-001.6) |
| Empty reason | Validation error; no row (AC-TASKS-QUORUM-RECORDING-001.7) |
| Task has no bound step | Error naming the unbound step; no row (AC-TASKS-QUORUM-RECORDING-001.8) |
| Decision store not wired | Guard does not fire; reason recorded (AC-TASKS-QUORUM-DIAGNOSTICS-001.1) |
@@ -312,7 +334,7 @@ task timeline — this is the concrete bug this paragraph exists to prevent.
| AC-TASKS-QUORUM-DIAGNOSTICS-001.4 read while the engine dispatcher is not wired | Endpoint errors; no office-side fallback evaluation (AC-TASKS-QUORUM-REEVALUATION-001.14, AC-TASKS-QUORUM-REEVALUATION-001.13) |
| One guard's store read errors, siblings healthy | That entry alone reports `evaluation_error`; the others are still returned (AC-TASKS-QUORUM-REEVALUATION-001.14, AC-TASKS-QUORUM-DIAGNOSTICS-001.1) |
| Decision recorded at a step that also configures a side-effect action on `on_turn_complete` | Only guarded transitions are evaluated; no callback re-fires (AC-TASKS-QUORUM-REEVALUATION-001.8) |
| Tool called at a step with no guarded transition | `guards` is an empty list, not an error (AC-TASKS-QUORUM-RECORDING-001.11) |
| Command called at a step with no guarded transition | `guards` is an empty list, not an error (AC-TASKS-QUORUM-RECORDING-001.11) |
| Decision row id needed for the AC-TASKS-QUORUM-CONCURRENCY-001.6 operation id | Pre-generated caller-side into `DecisionInfo.ID`; never read back after the write (AC-TASKS-QUORUM-CONCURRENCY-001.8) |
| Office needs `decision_id` / `created_at` to publish | Both returned by the AC-TASKS-QUORUM-REEVALUATION-001.10 entry point (AC-TASKS-QUORUM-REEVALUATION-001.11) |
@@ -340,7 +362,8 @@ Each of the following is a deliberate exclusion, not an oversight.
- **Workspace `approvals` subsystem.** `POST /approvals/:id/decide` and its
`"approved" | "rejected"` shape (`apps/web/lib/api/domains/office-api.ts:534`)
are a separate feature from task step decisions. Untouched.
are a separate feature from task step decisions. The
`agentctl kandev approvals decide` command remains untouched and is not reused.
- **New thresholds.** `all_approve`, `all_decide`, `any_reject`,
`majority_approve`, `n_approve:<N>` are the complete set. No new threshold.
- **Guard variants beyond `wait_for_quorum`.** `TransitionGuard` stays
@@ -359,10 +382,11 @@ Each of the following is a deliberate exclusion, not an oversight.
- **Timeouts and escalation.** A quorum that is never reached waits
indefinitely. No auto-advance, no reminder, no deadline.
- **Delegating or reassigning a pending decision.** Not addressed.
- **Kanban-workflow decisions.** The decision tool is Office-surface only
(AC-TASKS-QUORUM-RECORDING-001.9); Kanban keeps `step_complete_kandev`.
- **Kanban-workflow decisions.** The decision command is Office-run only
(AC-TASKS-QUORUM-RECORDING-001.9); Kanban keeps `step_complete_kandev` and
its existing MCP profile.
- **Backfilling the zero existing decision rows.** There are none to migrate.
- **Client-supplied idempotency keys on the decision tool.** Excluded by AC-TASKS-QUORUM-CONCURRENCY-001.7.
- **Client-supplied idempotency keys on the decision command.** Excluded by AC-TASKS-QUORUM-CONCURRENCY-001.7.
Transition application is idempotent (AC-TASKS-QUORUM-CONCURRENCY-001.6); recording is not.
- **Giving the singleton human user a participant row.** AC-TASKS-QUORUM-VERDICT-001.3 makes a human
rejection count without one. Seating the user in the slate would change who