fix(office): grant record_step_decision capability to decision-seat holders (#3463)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-09-08 04:48:59 +08:00
committed by GitHub
parent d17812fdb1
commit ffa42a8cfd
19 changed files with 792 additions and 25 deletions
@@ -59,6 +59,11 @@ type EngineHandle interface {
// 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)
// ResolveParticipantRoleReadOnly is ResolveParticipantRole's
// side-effect-free counterpart, for callers that only observe seat
// occupancy (e.g. a runtime capability grant) rather than evaluate a
// guard or record a decision. Reached via Dispatcher.ResolveParticipantRoleReadOnly.
ResolveParticipantRoleReadOnly(ctx context.Context, taskID, stepID, agentProfileID string) (role, participantID string, err error)
}
// RecordDecisionInput is what a transport must resolve before calling
@@ -367,6 +372,15 @@ func (d *Dispatcher) ResolveParticipantRole(
return d.engine.ResolveParticipantRole(ctx, taskID, stepID, agentProfileID)
}
// ResolveParticipantRoleReadOnly is ResolveParticipantRole's side-effect-free
// counterpart (see EngineHandle.ResolveParticipantRoleReadOnly): no session
// resolution is needed here either.
func (d *Dispatcher) ResolveParticipantRoleReadOnly(
ctx context.Context, taskID, stepID, agentProfileID string,
) (role, participantID string, err error) {
return d.engine.ResolveParticipantRoleReadOnly(ctx, taskID, stepID, agentProfileID)
}
// resolveActiveSessionID returns AC-16's active-session id, or "" when no
// such session is resolvable (AC-16a) — never an error for that case.
func (d *Dispatcher) resolveActiveSessionID(ctx context.Context, taskID string) (string, error) {
@@ -0,0 +1,61 @@
package engine_dispatcher
import (
"context"
"errors"
"testing"
"github.com/kandev/kandev/internal/common/logger"
)
// TestDispatcher_ResolveParticipantRoleReadOnly_ForwardsVerbatim mirrors
// TestDispatcher_ResolveParticipantRole_ForwardsVerbatim for the read-only
// entry point: same pass-through contract, distinct engine method.
func TestDispatcher_ResolveParticipantRoleReadOnly_ForwardsVerbatim(t *testing.T) {
eng := &fakeEngine{roleReadOnlyResult: "approver", roleReadOnlyParticipantID: "seat-1"}
d := New(eng, &fakeSessions{}, logger.Default())
role, participantID, err := d.ResolveParticipantRoleReadOnly(context.Background(), "task-1", "review", "agent-a")
if err != nil {
t.Fatalf("ResolveParticipantRoleReadOnly: %v", err)
}
if !eng.roleReadOnlyCalled {
t.Fatal("engine ResolveParticipantRoleReadOnly not invoked")
}
if eng.roleCalled {
t.Error("recording ResolveParticipantRole must not be invoked by the read-only entry point")
}
if eng.roleReadOnlyTaskID != "task-1" || eng.roleReadOnlyStepID != "review" || eng.roleReadOnlyAgentProfileID != "agent-a" {
t.Errorf("engine called with (%q, %q, %q), want (task-1, review, agent-a)",
eng.roleReadOnlyTaskID, eng.roleReadOnlyStepID, eng.roleReadOnlyAgentProfileID)
}
if role != "approver" || participantID != "seat-1" {
t.Errorf("role/participantID = %q/%q, want approver/seat-1", role, participantID)
}
}
// TestDispatcher_ResolveParticipantRoleReadOnly_PropagatesEngineError proves
// errors (including engine.ErrParticipantNotFound) surface unwrapped.
func TestDispatcher_ResolveParticipantRoleReadOnly_PropagatesEngineError(t *testing.T) {
notFound := errors.New("not a participant")
eng := &fakeEngine{roleReadOnlyErr: notFound}
d := New(eng, &fakeSessions{}, logger.Default())
_, _, err := d.ResolveParticipantRoleReadOnly(context.Background(), "task-1", "review", "agent-a")
if !errors.Is(err, notFound) {
t.Fatalf("err = %v, want %v", err, notFound)
}
}
// TestDispatcher_ResolveParticipantRoleReadOnly_NoSessionLookup proves this
// entry point never touches the session resolver, matching the recording
// variant's contract.
func TestDispatcher_ResolveParticipantRoleReadOnly_NoSessionLookup(t *testing.T) {
eng := &fakeEngine{roleReadOnlyResult: "reviewer", roleReadOnlyParticipantID: "seat-2"}
sessions := &fakeSessions{activeErr: errors.New("session store must not be consulted")}
d := New(eng, sessions, logger.Default())
if _, _, err := d.ResolveParticipantRoleReadOnly(context.Background(), "task-1", "review", "agent-a"); err != nil {
t.Fatalf("ResolveParticipantRoleReadOnly: %v", err)
}
}
@@ -71,6 +71,14 @@ type fakeEngine struct {
roleResult string
roleParticipantID string
roleErr error
roleReadOnlyCalled bool
roleReadOnlyTaskID string
roleReadOnlyStepID string
roleReadOnlyAgentProfileID string
roleReadOnlyResult string
roleReadOnlyParticipantID string
roleReadOnlyErr error
}
func (f *fakeEngine) HandleTrigger(_ context.Context, in engine.HandleInput) (engine.HandleResult, error) {
@@ -107,6 +115,16 @@ func (f *fakeEngine) ResolveParticipantRole(
return f.roleResult, f.roleParticipantID, f.roleErr
}
func (f *fakeEngine) ResolveParticipantRoleReadOnly(
_ context.Context, taskID, stepID, agentProfileID string,
) (string, string, error) {
f.roleReadOnlyCalled = true
f.roleReadOnlyTaskID = taskID
f.roleReadOnlyStepID = stepID
f.roleReadOnlyAgentProfileID = agentProfileID
return f.roleReadOnlyResult, f.roleReadOnlyParticipantID, f.roleReadOnlyErr
}
type realRunsAdapter struct {
svc *runsservice.Service
}
@@ -8,8 +8,8 @@ import (
// WildcardTaskScope grants task mutation access to any task in the run's workspace.
const WildcardTaskScope = "*"
// Runtime capability keys. These are the stable syscall vocabulary exposed to
// run tokens, prompts, and runtime API handlers.
// Runtime capability keys. These are the stable permission vocabulary exposed
// to run tokens and runtime API handlers.
const (
CapabilityPostComment = "post_comment"
CapabilityUpdateTaskStatus = "update_task_status"
@@ -27,6 +27,10 @@ const (
CapabilityDeleteSkills = "delete_skills"
)
// AvailableActionRecordStepDecision identifies the MCP decision tool 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.
func (c Capabilities) Allows(key string) bool {
switch key {
@@ -0,0 +1,31 @@
package runtime
import (
"testing"
"github.com/kandev/kandev/internal/office/models"
)
func TestDecisionToolIsNotRuntimeCapability(t *testing.T) {
caps := Capabilities{}
if caps.Allows(AvailableActionRecordStepDecision) {
t.Fatal("decision tool availability must not be treated as a runtime capability")
}
for _, key := range caps.AllowedKeys() {
if key == AvailableActionRecordStepDecision {
t.Fatalf("runtime capability list must not include the decision tool: %v", caps.AllowedKeys())
}
}
}
func TestFromAgentNeverGrantsRecordStepDecision(t *testing.T) {
ceo := &models.AgentInstance{
ID: "agent-ceo",
WorkspaceID: "ws-1",
Role: models.AgentRoleCEO,
}
caps := FromAgent(ceo)
if caps.Allows(AvailableActionRecordStepDecision) {
t.Fatal("Allows must not authorize the decision tool")
}
}
@@ -1,7 +1,8 @@
// Package runtime defines the narrow execution contract used by Office agent runs.
package runtime
// Capabilities describes what an agent run may do through the runtime action surface.
// Capabilities describes what an agent run may do through the runtime HTTP
// action surface. These permissions are serialized into the run JWT.
type Capabilities struct {
CanPostComments bool `json:"post_comment"`
CanUpdateTaskStatus bool `json:"update_task_status"`
@@ -20,15 +21,19 @@ type Capabilities struct {
AllowedTaskIDs []string `json:"allowed_task_ids"`
}
// RunContext is the identity and capability envelope for one agent execution.
// RunContext is the identity, runtime-permission, and advertised-action
// envelope for one agent execution. AvailableActions only describes tools the
// prompt can advertise. The backend performs live authorization when a tool is
// called.
type RunContext struct {
WorkspaceID string `json:"workspace_id"`
AgentID string `json:"agent_id"`
TaskID string `json:"task_id"`
RunID string `json:"run_id"`
SessionID string `json:"session_id"`
Reason string `json:"reason"`
Capabilities Capabilities `json:"capabilities"`
WorkspaceID string `json:"workspace_id"`
AgentID string `json:"agent_id"`
TaskID string `json:"task_id"`
RunID string `json:"run_id"`
SessionID string `json:"session_id"`
Reason string `json:"reason"`
Capabilities Capabilities `json:"capabilities"`
AvailableActions []string `json:"available_actions,omitempty"`
}
// CanMutateTask reports whether the run may mutate the given task.
@@ -20,10 +20,20 @@ type RunSnapshotStore interface {
) error
}
// DecisionSeatResolver reports whether an agent currently holds a decision
// 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.
type DecisionSeatResolver interface {
HoldsDecisionSeat(ctx context.Context, taskID, agentProfileID string) (bool, error)
}
// ContextBuilder builds the runtime context for a claimed run.
type ContextBuilder struct {
Agents shared.AgentReader
Runs RunSnapshotStore
Seats DecisionSeatResolver
}
// Build resolves the agent identity and capabilities for a run.
@@ -42,18 +52,41 @@ func (b *ContextBuilder) Build(ctx context.Context, run *models.Run) (RunContext
taskID := payload["task_id"]
sessionID := firstNonEmpty(run.SessionID, payload["session_id"])
caps := FromAgent(agent).WithTaskScope(taskID)
availableActions, err := b.resolveAvailableActions(ctx, taskID, agent.ID)
if err != nil {
return RunContext{}, err
}
runCtx := RunContext{
WorkspaceID: agent.WorkspaceID,
AgentID: agent.ID,
TaskID: taskID,
RunID: run.ID,
SessionID: sessionID,
Reason: run.Reason,
Capabilities: caps,
WorkspaceID: agent.WorkspaceID,
AgentID: agent.ID,
TaskID: taskID,
RunID: run.ID,
SessionID: sessionID,
Reason: run.Reason,
Capabilities: caps,
AvailableActions: availableActions,
}
return runCtx, nil
}
// resolveAvailableActions reports advisory tools for the run prompt. A
// taskless run or missing resolver has no seat-derived actions. A lookup error
// aborts context construction so the scheduler can retry instead of launching
// a prompt that requires a tool which it could not advertise.
func (b *ContextBuilder) resolveAvailableActions(ctx context.Context, taskID, agentID string) ([]string, error) {
if taskID == "" || b.Seats == nil {
return nil, nil
}
held, err := b.Seats.HoldsDecisionSeat(ctx, taskID, agentID)
if err != nil {
return nil, fmt.Errorf("resolve decision seat: %w", err)
}
if !held {
return nil, nil
}
return []string{AvailableActionRecordStepDecision}, nil
}
// BuildAndPersist builds context and stores its serialized snapshot on the run.
func (b *ContextBuilder) BuildAndPersist(ctx context.Context, run *models.Run) (RunContext, error) {
runCtx, err := b.Build(ctx, run)
@@ -3,11 +3,14 @@ package runtime
import (
"context"
"encoding/json"
"errors"
"testing"
"github.com/kandev/kandev/internal/office/models"
)
var errBoom = errors.New("boom")
func TestContextBuilderBuildsAndPersistsRuntimeSnapshot(t *testing.T) {
agents := &recordingAgentReader{
agent: &models.AgentInstance{
@@ -96,3 +99,149 @@ func (s *recordingRunSnapshotStore) UpdateRunRuntimeSnapshot(
})
return nil
}
type stubSeatResolver struct {
held bool
err error
taskID string
agentID string
}
func (r *stubSeatResolver) HoldsDecisionSeat(_ context.Context, taskID, agentID string) (bool, error) {
r.taskID = taskID
r.agentID = agentID
return r.held, r.err
}
func buildTestRunContext(t *testing.T, seats DecisionSeatResolver) RunContext {
t.Helper()
agents := &recordingAgentReader{
agent: &models.AgentInstance{ID: "agent-1", WorkspaceID: "ws-1", Role: models.AgentRoleWorker},
}
builder := ContextBuilder{Agents: agents, Runs: &recordingRunSnapshotStore{}, Seats: seats}
run := &models.Run{
ID: "run-1",
AgentProfileID: "agent-1",
Reason: "task_assigned",
Payload: `{"task_id":"task-1"}`,
}
runCtx, err := builder.Build(context.Background(), run)
if err != nil {
t.Fatalf("Build: %v", err)
}
return runCtx
}
func hasAvailableAction(runCtx RunContext, want string) bool {
for _, action := range runCtx.AvailableActions {
if action == want {
return true
}
}
return false
}
func TestContextBuilderGrantsRecordStepDecisionForSeatHolder(t *testing.T) {
runCtx := buildTestRunContext(t, &stubSeatResolver{held: true})
if !hasAvailableAction(runCtx, AvailableActionRecordStepDecision) {
t.Fatal("expected seat holder to be granted record_step_decision")
}
}
func TestContextBuilderDeniesRecordStepDecisionForNonHolder(t *testing.T) {
runCtx := buildTestRunContext(t, &stubSeatResolver{held: false})
if hasAvailableAction(runCtx, AvailableActionRecordStepDecision) {
t.Fatal("expected non-holder to be denied record_step_decision")
}
}
func TestContextBuilderDeniesRecordStepDecisionWhenResolverNil(t *testing.T) {
runCtx := buildTestRunContext(t, nil)
if hasAvailableAction(runCtx, AvailableActionRecordStepDecision) {
t.Fatal("expected nil resolver to deny record_step_decision")
}
}
func TestContextBuilderReturnsResolverError(t *testing.T) {
seats := &stubSeatResolver{held: true, err: errBoom}
builder := ContextBuilder{
Agents: &recordingAgentReader{
agent: &models.AgentInstance{ID: "agent-1", WorkspaceID: "ws-1", Role: models.AgentRoleWorker},
},
Seats: seats,
}
run := &models.Run{ID: "run-1", AgentProfileID: "agent-1", Payload: `{"task_id":"task-1"}`}
if _, err := builder.Build(context.Background(), run); !errors.Is(err, errBoom) {
t.Fatalf("expected resolver error to abort context build, got %v", err)
}
}
func TestContextBuilderDeniesRecordStepDecisionForTasklessRun(t *testing.T) {
agents := &recordingAgentReader{
agent: &models.AgentInstance{ID: "agent-1", WorkspaceID: "ws-1", Role: models.AgentRoleWorker},
}
builder := ContextBuilder{
Agents: agents,
Runs: &recordingRunSnapshotStore{},
Seats: &stubSeatResolver{held: true},
}
run := &models.Run{ID: "run-1", AgentProfileID: "agent-1", Reason: "heartbeat", Payload: "{}"}
runCtx, err := builder.Build(context.Background(), run)
if err != nil {
t.Fatalf("Build: %v", err)
}
if hasAvailableAction(runCtx, AvailableActionRecordStepDecision) {
t.Fatal("expected taskless run to deny record_step_decision")
}
}
func TestContextBuilderPersistsSeatActionAsAdvisoryMetadata(t *testing.T) {
store := &recordingRunSnapshotStore{}
seats := &stubSeatResolver{held: true}
builder := ContextBuilder{
Agents: &recordingAgentReader{
agent: &models.AgentInstance{
ID: "agent-1",
WorkspaceID: "ws-1",
Role: models.AgentRoleWorker,
},
},
Runs: store,
Seats: seats,
}
run := &models.Run{
ID: "run-1",
AgentProfileID: "agent-1",
Reason: "task_review_requested",
Payload: `{"task_id":"task-1"}`,
}
runCtx, err := builder.BuildAndPersist(context.Background(), run)
if err != nil {
t.Fatalf("BuildAndPersist: %v", err)
}
if seats.taskID != "task-1" || seats.agentID != "agent-1" {
t.Fatalf("seat resolver received task=%q agent=%q", seats.taskID, seats.agentID)
}
rawContext, err := json.Marshal(runCtx)
if err != nil {
t.Fatalf("marshal run context: %v", err)
}
var contextSnapshot map[string]any
if err := json.Unmarshal(rawContext, &contextSnapshot); err != nil {
t.Fatalf("decode run context: %v", err)
}
actions, ok := contextSnapshot["available_actions"].([]any)
if !ok || len(actions) != 1 || actions[0] != "record_step_decision" {
t.Fatalf("expected advisory record_step_decision action, got %#v", contextSnapshot["available_actions"])
}
var capabilities map[string]any
if err := json.Unmarshal([]byte(store.calls[0].Capabilities), &capabilities); err != nil {
t.Fatalf("decode capabilities: %v", err)
}
if _, ok := capabilities["record_step_decision"]; ok {
t.Fatalf("record_step_decision must not be serialized as a runtime capability: %s", store.calls[0].Capabilities)
}
}
@@ -0,0 +1,60 @@
package service
import (
"context"
"errors"
"fmt"
"github.com/kandev/kandev/internal/workflow/engine"
"go.uber.org/zap"
)
// decisionSeatDispatcher is the additive capability HoldsDecisionSeat needs
// from s.engineDispatcher. Named locally and reached via a type assertion
// rather than widening shared.WorkflowEngineDispatcher, mirroring
// dashboard.roleResolvingDispatcher. HoldsDecisionSeat only observes seat
// occupancy — it neither evaluates a guard nor records a decision — so it
// asserts on the read-only variant rather than ResolveParticipantRole:
// the recording variant would emit the unresolved-agent quorum counter and
// warning log, which are scoped to guard evaluations. A capability grant
// runs once per run launch, not per guard evaluation, so it must use the
// read-only variant to avoid inflating those metrics at an unrelated
// cadence.
type decisionSeatDispatcher interface {
ResolveParticipantRoleReadOnly(ctx context.Context, taskID, stepID, agentProfileID string) (role, participantID string, err error)
}
// 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
// matching capability without over- or under-stating who can actually
// decide.
func (s *Service) HoldsDecisionSeat(ctx context.Context, taskID, agentProfileID string) (bool, error) {
if taskID == "" || agentProfileID == "" {
return false, nil
}
dispatcher, ok := s.engineDispatcher.(decisionSeatDispatcher)
if !ok {
return false, nil
}
stepID, err := s.repo.GetTaskWorkflowStepID(ctx, taskID)
if err != nil {
s.logger.Warn("resolve task workflow_step_id failed, denying decision seat",
zap.String("task_id", taskID), zap.String("agent_profile_id", agentProfileID), zap.Error(err))
return false, fmt.Errorf("resolve task workflow_step_id: %w", err)
}
if stepID == "" {
return false, nil
}
_, _, err = dispatcher.ResolveParticipantRoleReadOnly(ctx, taskID, stepID, agentProfileID)
if err != nil {
if errors.Is(err, engine.ErrParticipantNotFound) {
return false, nil
}
s.logger.Warn("resolve participant role failed, denying decision seat",
zap.String("task_id", taskID), zap.String("agent_profile_id", agentProfileID), zap.Error(err))
return false, fmt.Errorf("resolve participant role: %w", err)
}
return true, nil
}
@@ -0,0 +1,131 @@
package service_test
import (
"context"
"errors"
"testing"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/workflow/engine"
)
// seatSpyDispatcher satisfies shared.WorkflowEngineDispatcher plus the
// package-local decisionSeatDispatcher capability HoldsDecisionSeat reaches
// via type assertion, mirroring dashboard's spyDecisionDispatcher.
type seatSpyDispatcher struct {
err error
}
func (s *seatSpyDispatcher) HandleTrigger(
_ context.Context, _ string, _ engine.Trigger, _ any, _ string,
) error {
return nil
}
func (s *seatSpyDispatcher) ResolveParticipantRoleReadOnly(
_ context.Context, _, _, _ string,
) (string, string, error) {
if s.err != nil {
return "", "", s.err
}
return "reviewer", "participant-1", nil
}
func insertTestTaskWithStep(t *testing.T, svc *service.Service, taskID, workspaceID, stepID string) {
t.Helper()
svc.ExecSQL(t,
`INSERT OR IGNORE INTO tasks (id, workspace_id, workflow_step_id) VALUES (?, ?, ?)`,
taskID, workspaceID, stepID)
}
func TestHoldsDecisionSeat_SeatHolderReturnsTrue(t *testing.T) {
svc := newTestService(t)
insertTestTaskWithStep(t, svc, "task-1", "ws-1", "step-1")
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{})
held, err := svc.HoldsDecisionSeat(context.Background(), "task-1", "agent-1")
if err != nil {
t.Fatalf("HoldsDecisionSeat: %v", err)
}
if !held {
t.Fatal("expected seat holder to return true")
}
}
func TestHoldsDecisionSeat_ParticipantNotFoundReturnsFalseNoError(t *testing.T) {
svc := newTestService(t)
insertTestTaskWithStep(t, svc, "task-1", "ws-1", "step-1")
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{err: engine.ErrParticipantNotFound})
held, err := svc.HoldsDecisionSeat(context.Background(), "task-1", "agent-1")
if err != nil {
t.Fatalf("expected no error for ErrParticipantNotFound, got %v", err)
}
if held {
t.Fatal("expected non-holder to return false")
}
}
func TestHoldsDecisionSeat_UnrelatedErrorPropagates(t *testing.T) {
svc := newTestService(t)
insertTestTaskWithStep(t, svc, "task-1", "ws-1", "step-1")
boom := errors.New("boom")
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{err: boom})
_, err := svc.HoldsDecisionSeat(context.Background(), "task-1", "agent-1")
if !errors.Is(err, boom) {
t.Fatalf("expected unrelated error to propagate, got %v", err)
}
}
func TestHoldsDecisionSeat_EmptyStepIDReturnsFalse(t *testing.T) {
svc := newTestService(t)
insertTestTaskWithStep(t, svc, "task-1", "ws-1", "")
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{})
held, err := svc.HoldsDecisionSeat(context.Background(), "task-1", "agent-1")
if err != nil {
t.Fatalf("HoldsDecisionSeat: %v", err)
}
if held {
t.Fatal("expected empty workflow step to return false")
}
}
func TestHoldsDecisionSeat_EmptyTaskIDReturnsFalse(t *testing.T) {
svc := newTestService(t)
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{})
held, err := svc.HoldsDecisionSeat(context.Background(), "", "agent-1")
if err != nil {
t.Fatalf("HoldsDecisionSeat: %v", err)
}
if held {
t.Fatal("expected empty task id to return false")
}
}
// legacyDispatcher satisfies shared.WorkflowEngineDispatcher but not the
// package-local decisionSeatDispatcher capability, exercising the type
// assertion's failure branch.
type legacyDispatcher struct{}
func (legacyDispatcher) HandleTrigger(
_ context.Context, _ string, _ engine.Trigger, _ any, _ string,
) error {
return nil
}
func TestHoldsDecisionSeat_DispatcherWithoutCapabilityReturnsFalse(t *testing.T) {
svc := newTestService(t)
insertTestTaskWithStep(t, svc, "task-1", "ws-1", "step-1")
svc.SetWorkflowEngineDispatcher(legacyDispatcher{})
held, err := svc.HoldsDecisionSeat(context.Background(), "task-1", "agent-1")
if err != nil {
t.Fatalf("HoldsDecisionSeat: %v", err)
}
if held {
t.Fatal("expected dispatcher without decisionSeatDispatcher to return false")
}
}
@@ -4,6 +4,7 @@ import (
"strings"
"testing"
officeruntime "github.com/kandev/kandev/internal/office/runtime"
"github.com/kandev/kandev/internal/office/service"
)
@@ -415,6 +416,36 @@ func TestBuildPrompt_ReviewStage(t *testing.T) {
}
}
// TestBuildPrompt_ReviewStageAllowedActionsIncludeRecordStepDecision is the
// anti-contradiction assertion: a reviewer whose run holds the decision seat
// must see record_step_decision in its own allowed-actions list, so it never
// reads the writeDecisionContract instruction below as excluded by its own
// stated permissions.
func TestBuildPrompt_ReviewStageAllowedActionsIncludeRecordStepDecision(t *testing.T) {
pc := &service.PromptContext{
Reason: service.RunReasonTaskAssigned,
TaskIdentifier: "KAN-10",
TaskTitle: "Auth service",
StageType: "review",
RunID: "run-1",
AgentID: "agent-1",
AllowedActions: []string{officeruntime.CapabilityPostComment, officeruntime.AvailableActionRecordStepDecision},
}
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)
}
allowedIdx := strings.Index(prompt, "- Allowed actions:")
if allowedIdx == -1 {
t.Fatalf("expected an allowed actions line:\n%s", prompt)
}
allowedLine := prompt[allowedIdx : strings.Index(prompt[allowedIdx:], "\n")+allowedIdx]
if !strings.Contains(allowedLine, officeruntime.AvailableActionRecordStepDecision) {
t.Fatalf("allowed actions line must list record_step_decision so it does not contradict the decision contract:\n%s", allowedLine)
}
}
func TestBuildPrompt_ApprovalStageUsesNeutralLifecycleLanguage(t *testing.T) {
prompt := service.BuildPrompt(&service.PromptContext{
Reason: service.RunReasonTaskAssigned,
@@ -291,6 +291,7 @@ func (si *SchedulerIntegration) prepareAndLaunch(
runCtx, err := (&officeruntime.ContextBuilder{
Agents: si.svc,
Runs: si.svc.repo,
Seats: si.svc,
}).BuildAndPersist(ctx, run)
if err != nil {
si.logger.Warn("runtime context build failed; retrying run",
@@ -377,7 +378,7 @@ func (si *SchedulerIntegration) assembleAgentPrompt(
pc.AgentID = runCtx.AgentID
pc.SessionID = runCtx.SessionID
pc.TaskScope = append([]string(nil), runCtx.Capabilities.AllowedTaskIDs...)
pc.AllowedActions = runCtx.Capabilities.AllowedKeys()
pc.AllowedActions = append(runCtx.Capabilities.AllowedKeys(), runCtx.AvailableActions...)
wakeContext := BuildPrompt(pc)
// Resume = the (task, agent_instance) session has run before. On resume
@@ -2,7 +2,9 @@ package service_test
import (
"context"
"encoding/json"
"errors"
"strings"
"sync"
"testing"
@@ -134,6 +136,75 @@ func TestSchedulerIntegration_RoutingReceivesBuiltPromptAndEnv(t *testing.T) {
}
}
func TestSchedulerIntegration_SeatActionFlowsToPromptOnly(t *testing.T) {
mock := &mockTaskStarter{}
svc := newTestService(t, service.ServiceOptions{TaskStarter: mock})
dispatcher := &captureDispatcher{}
svc.SetRoutingDispatcher(dispatcher)
svc.SetWorkflowEngineDispatcher(&seatSpyDispatcher{})
ctx := context.Background()
agent := &models.AgentInstance{
ID: "decision-agent-1",
WorkspaceID: "ws-1",
Name: "decision-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-decision", "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-decision", "ws-1", "step-decision", "Decision task", "Review the change")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-decision"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
service.RunSchedulerTick(svc, ctx)
if dispatcher.callCount() != 1 {
t.Fatalf("expected one routing dispatch, got %d", dispatcher.callCount())
}
prompt := dispatcher.lastCall().Prompt
allowedIdx := strings.Index(prompt, "- Allowed actions:")
if allowedIdx == -1 || !containsIgnoreCase(prompt[allowedIdx:], "record_step_decision") {
t.Fatalf("prompt must advertise the seat-derived action: %s", dispatcher.lastCall().Prompt)
}
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 decision run")
}
var capabilities map[string]any
if err := json.Unmarshal([]byte(run.Capabilities), &capabilities); err != nil {
t.Fatalf("decode persisted capabilities: %v", err)
}
if _, ok := capabilities["record_step_decision"]; ok {
t.Fatalf("seat-derived action must not be persisted as a runtime capability: %s", run.Capabilities)
}
var snapshot map[string]any
if err := json.Unmarshal([]byte(run.InputSnapshot), &snapshot); err != nil {
t.Fatalf("decode persisted input snapshot: %v", err)
}
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"])
}
}
// TestSchedulerIntegration_RoutingFallThrough_FallsBackToLegacy asserts
// the routing seam preserves the legacy fall-through behavior: when the
// dispatcher returns (launched=false, parked=false, err=nil), the
@@ -245,9 +245,12 @@ func (e *Engine) requiredSeatsForWorkflowRecording(
// every other optional Engine dependency's nil-safe default.
//
// collapseByRoleAgent already guarantees at most one seat per (role, agent
// profile id) pair here, and this function is scoped to one role and called
// once per guard evaluation, so at most one record is emitted per role and
// unresolved agent per evaluation (AC-004.10) without extra dedup state.
// profile id) pair here, and this function is scoped to one role, so at most
// one record is emitted per role and unresolved agent per call, without
// extra dedup state, as long as the caller passes record=true only for an
// actual guard evaluation or decision write. A read-only observation
// (ResolveParticipantRoleReadOnly) passes record=false precisely so it does
// not multiply this emission at its own, unrelated cadence.
//
// A resolver error is not the same condition as a confirmed deletion — it
// means the resolver could not answer, not that it answered "gone" — so it
@@ -911,8 +914,34 @@ func (e *Engine) evaluateGuardStateReadOnly(
// step cannot be loaded — matching the precedence in
// office/dashboard/decisions.go's resolveDeciderRole (AC-4). Returns
// ErrParticipantNotFound when the agent occupies neither seat.
//
// This is the recording variant: an unresolved-agent seat dropped from the
// slate emits a counter and warning log scoped to guard evaluations.
// Reserve it for callers that ARE a guard evaluation or a real decision
// write (RecordAgentDecision's authorization check). A caller that merely
// observes seat occupancy — without evaluating a guard or recording a
// decision — must use ResolveParticipantRoleReadOnly instead, or every such
// observation double-counts the same drop.
func (e *Engine) ResolveParticipantRole(
ctx context.Context, taskID, stepID, agentProfileID string,
) (role, participantID string, err error) {
return e.resolveParticipantRole(ctx, taskID, stepID, agentProfileID, true)
}
// ResolveParticipantRoleReadOnly is ResolveParticipantRole without the
// unresolved-agent counter/log side effect, mirroring
// evaluateGuardStateReadOnly's use of requiredSeatsForWorkflowRecording(...,
// false) for the same reason: the caller is observing seat occupancy, not
// evaluating a guard or recording a decision, so the emission stays scoped
// to actual guard evaluations rather than firing on every observation.
func (e *Engine) ResolveParticipantRoleReadOnly(
ctx context.Context, taskID, stepID, agentProfileID string,
) (role, participantID string, err error) {
return e.resolveParticipantRole(ctx, taskID, stepID, agentProfileID, false)
}
func (e *Engine) resolveParticipantRole(
ctx context.Context, taskID, stepID, agentProfileID string, record bool,
) (role, participantID string, err error) {
if agentProfileID == "" {
return "", "", ErrParticipantNotFound
@@ -928,7 +957,7 @@ func (e *Engine) ResolveParticipantRole(
wfmodels.ParticipantRoleApprover,
wfmodels.ParticipantRoleReviewer,
} {
seats, seatsErr := e.requiredSeatsForWorkflow(ctx, stepID, taskID, workflowID, string(r))
seats, seatsErr := e.requiredSeatsForWorkflowRecording(ctx, stepID, taskID, workflowID, string(r), record)
if seatsErr != nil {
if firstSeatsErr == nil {
firstSeatsErr = seatsErr
@@ -284,6 +284,75 @@ func TestResolveParticipantRole_ReviewerAtStepDefersApproverResolverError(t *tes
}
}
// TestResolveParticipantRoleReadOnly_UnresolvedAgentSeatEmitsNoCounter is
// the round-2 regression test: an observation-only caller (mirroring
// office/service.HoldsDecisionSeat, invoked once per run launch rather than
// once per guard evaluation) must not trip AC-OFFICE-REVIEW-SEATS-004.10's
// "per guard evaluation" counter/log just because some other seat's agent
// profile has been deleted. The queried agent-a holds no seat here — the
// dropped seat belongs to an unrelated deleted profile — so the only
// observable effect of resolving it must be ErrParticipantNotFound, with the
// counter and log left untouched.
func TestResolveParticipantRoleReadOnly_UnresolvedAgentSeatEmitsNoCounter(t *testing.T) {
participants := scopedParticipants{perTask: []ParticipantInfo{
{ID: "seat-gone", TaskID: "task-1", StepID: "review", Role: "reviewer", AgentProfileID: "rev-gone", DecisionRequired: true},
}}
resolver := &fakeAgentProfileResolver{unresolved: map[string]bool{"rev-gone": true}}
core, logs := observer.New(zap.WarnLevel)
zapLogger := zap.New(core)
log, err := logger.NewFromZap(zapLogger)
if err != nil {
t.Fatalf("build logger: %v", err)
}
eng := New(quorumStore(nil), MapRegistry{},
WithDecisionStore(newFakeDecisionStore()), WithParticipantStore(participants),
WithAgentProfileResolver(resolver), WithLogger(log))
before := readParticipantAgentUnresolvedCounter("reviewer")
_, _, err = eng.ResolveParticipantRoleReadOnly(context.Background(), "task-1", "review", "agent-a")
if !errors.Is(err, ErrParticipantNotFound) {
t.Fatalf("err = %v, want ErrParticipantNotFound", err)
}
if after := readParticipantAgentUnresolvedCounter("reviewer"); after != before {
t.Errorf("counter changed by %d, want 0 (read-only resolution must not emit AC-004.8's counter)", after-before)
}
if got := logs.Len(); got != 0 {
t.Errorf("expected no warning logs from a read-only resolution, got %d: %+v", got, logs.All())
}
}
// TestResolveParticipantRole_UnresolvedAgentSeatEmitsCounter pins the
// recording variant's unchanged behavior alongside the read-only test above:
// the same slate, resolved through the recording entry point RecordAgentDecision
// authorizes with, still emits AC-004.8's counter and log exactly once.
func TestResolveParticipantRole_UnresolvedAgentSeatEmitsCounter(t *testing.T) {
participants := scopedParticipants{perTask: []ParticipantInfo{
{ID: "seat-gone", TaskID: "task-1", StepID: "review", Role: "reviewer", AgentProfileID: "rev-gone", DecisionRequired: true},
}}
resolver := &fakeAgentProfileResolver{unresolved: map[string]bool{"rev-gone": true}}
core, logs := observer.New(zap.WarnLevel)
zapLogger := zap.New(core)
log, err := logger.NewFromZap(zapLogger)
if err != nil {
t.Fatalf("build logger: %v", err)
}
eng := New(quorumStore(nil), MapRegistry{},
WithDecisionStore(newFakeDecisionStore()), WithParticipantStore(participants),
WithAgentProfileResolver(resolver), WithLogger(log))
before := readParticipantAgentUnresolvedCounter("reviewer")
_, _, err = eng.ResolveParticipantRole(context.Background(), "task-1", "review", "agent-a")
if !errors.Is(err, ErrParticipantNotFound) {
t.Fatalf("err = %v, want ErrParticipantNotFound", err)
}
if after := readParticipantAgentUnresolvedCounter("reviewer"); after-before != 1 {
t.Errorf("counter delta = %d, want 1", after-before)
}
if got := logs.Len(); got != 1 {
t.Errorf("expected exactly one warning log, got %d: %+v", got, logs.All())
}
}
func TestComputeGuardOutcome_ContinuesEvaluatingAfterDroppingUnresolvedSeat(t *testing.T) {
parts := scopedParticipants{template: []ParticipantInfo{
{ID: "p1", StepID: "review", Role: "reviewer", AgentProfileID: "rev-A", DecisionRequired: true},
@@ -0,0 +1,54 @@
# ADR-2026-09-07-separate-runtime-permissions-and-advisory-actions: Keep MCP availability separate from runtime permissions
**Status:** accepted
**Date:** 2026-09-07
**Area:** backend, protocol
## Context
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.
Putting the MCP action in `runtime.Capabilities` makes `Allows` and JWT data
look like authorization even though the MCP handler does not enforce them.
## Decision
Keep `runtime.Capabilities` limited to permissions enforced by the runtime HTTP
handler. Store seat-derived MCP affordances 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.
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
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.
Seat changes after launch can make the prompt stale. This is safe because the
decision handler resolves the current step and participant seat again.
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` 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.
- 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.
+1
View File
@@ -252,6 +252,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
| 2026-09-06-opt-in-external-e2e-runners | [Opt in selected Linux CI jobs to external runners](2026-09-06-opt-in-external-e2e-runners.md) | accepted | infra, workflow, security | 2026-09-06 |
| 2026-09-06-explicit-pr-auto-fix-outcomes | [Require Explicit Outcomes for PR Auto-Fix Attempts](2026-09-06-explicit-pr-auto-fix-outcomes.md) | proposed | backend, frontend, protocol, workflow, GitHub | 2026-09-06 |
| 2026-09-05-bounded-progressive-storage-analysis | [Use bounded progressive storage analysis](2026-09-05-bounded-progressive-storage-analysis.md) | accepted | backend, frontend, protocol | 2026-09-05 |
| 2026-09-07-separate-runtime-permissions-and-advisory-actions | [Keep MCP availability separate from runtime permissions](2026-09-07-separate-runtime-permissions-and-advisory-actions.md) | accepted | backend, protocol | 2026-09-07 |
| 2026-09-07-isolate-fork-preview-builds | [Isolate contributor preview builds from deployment credentials](2026-09-07-isolate-fork-preview-builds.md) | accepted | infra, workflow, security | 2026-09-07 |
| 2026-09-05-script-capable-html-preview-isolation | [Capability-Free Runtime for Script-Capable HTML Preview](2026-09-05-script-capable-html-preview-isolation.md) | proposed | frontend, desktop, security | 2026-09-05 |
| 2026-09-05-trusted-browser-html-preview | [Treat HTML Preview as Trusted Workspace Code](2026-09-05-trusted-browser-html-preview.md) | accepted | frontend, backend, agentctl, security | 2026-09-05 |
@@ -19,6 +19,12 @@ This design preserves the technical source detail for `REQ-OFFICE-RUNTIME-001` d
| --- | --- |
| `REQ-OFFICE-RUNTIME-001` | [Migrated source detail](#migrated-source-detail) |
## Related decisions
[Keep MCP availability separate from runtime permissions](../../../decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md)
defines the boundary between enforced runtime permissions and seat-derived
prompt metadata.
## Migrated source detail
> **Amendment (2026-08-17):** The provider-classification and automatic
@@ -255,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`).
- **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`.
- **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.
@@ -117,4 +117,3 @@ agent update only masked failed initial placement.
- Desktop and mobile Playwright tests use completed tasks, delay task A's
mark-read response across a task switch, return to task A, and prove the stale
divider does not move the transcript away from the bottom.