From ffa42a8cfd6f42fc49e6fa9207d77d52cf72b990 Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 8 Sep 2026 04:48:59 +0800 Subject: [PATCH] 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 --- .../office/engine_dispatcher/dispatcher.go | 14 ++ .../dispatcher_role_readonly_test.go | 61 +++++++ .../engine_dispatcher/dispatcher_test.go | 18 +++ .../internal/office/runtime/capabilities.go | 8 +- .../office/runtime/capabilities_test.go | 31 ++++ .../internal/office/runtime/context.go | 23 +-- .../office/runtime/context_builder.go | 47 +++++- .../office/runtime/context_builder_test.go | 149 ++++++++++++++++++ .../internal/office/service/decision_seat.go | 60 +++++++ .../office/service/decision_seat_test.go | 131 +++++++++++++++ .../office/service/prompt_builder_test.go | 31 ++++ .../office/service/scheduler_integration.go | 3 +- .../scheduler_integration_routing_test.go | 71 +++++++++ .../internal/workflow/engine/quorum.go | 37 ++++- .../engine/quorum_agent_resolver_test.go | 69 ++++++++ ...untime-permissions-and-advisory-actions.md | 54 +++++++ docs/decisions/INDEX.md | 1 + docs/specs/office/system-design/runtime-01.md | 8 +- .../office/system-design/unread-divider.md | 1 - 19 files changed, 792 insertions(+), 25 deletions(-) create mode 100644 apps/backend/internal/office/engine_dispatcher/dispatcher_role_readonly_test.go create mode 100644 apps/backend/internal/office/runtime/capabilities_test.go create mode 100644 apps/backend/internal/office/service/decision_seat.go create mode 100644 apps/backend/internal/office/service/decision_seat_test.go create mode 100644 docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md diff --git a/apps/backend/internal/office/engine_dispatcher/dispatcher.go b/apps/backend/internal/office/engine_dispatcher/dispatcher.go index 8cdd962732..75c0c98787 100644 --- a/apps/backend/internal/office/engine_dispatcher/dispatcher.go +++ b/apps/backend/internal/office/engine_dispatcher/dispatcher.go @@ -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) { diff --git a/apps/backend/internal/office/engine_dispatcher/dispatcher_role_readonly_test.go b/apps/backend/internal/office/engine_dispatcher/dispatcher_role_readonly_test.go new file mode 100644 index 0000000000..b2e57a4e18 --- /dev/null +++ b/apps/backend/internal/office/engine_dispatcher/dispatcher_role_readonly_test.go @@ -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) + } +} diff --git a/apps/backend/internal/office/engine_dispatcher/dispatcher_test.go b/apps/backend/internal/office/engine_dispatcher/dispatcher_test.go index b166f5b114..9bb18fb88c 100644 --- a/apps/backend/internal/office/engine_dispatcher/dispatcher_test.go +++ b/apps/backend/internal/office/engine_dispatcher/dispatcher_test.go @@ -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 } diff --git a/apps/backend/internal/office/runtime/capabilities.go b/apps/backend/internal/office/runtime/capabilities.go index e4860ca1c8..65741eaed4 100644 --- a/apps/backend/internal/office/runtime/capabilities.go +++ b/apps/backend/internal/office/runtime/capabilities.go @@ -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 { diff --git a/apps/backend/internal/office/runtime/capabilities_test.go b/apps/backend/internal/office/runtime/capabilities_test.go new file mode 100644 index 0000000000..bbd8f35d9d --- /dev/null +++ b/apps/backend/internal/office/runtime/capabilities_test.go @@ -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") + } +} diff --git a/apps/backend/internal/office/runtime/context.go b/apps/backend/internal/office/runtime/context.go index ccb2e6691e..e2046ac445 100644 --- a/apps/backend/internal/office/runtime/context.go +++ b/apps/backend/internal/office/runtime/context.go @@ -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. diff --git a/apps/backend/internal/office/runtime/context_builder.go b/apps/backend/internal/office/runtime/context_builder.go index 06bae5dc27..625ce3c4a3 100644 --- a/apps/backend/internal/office/runtime/context_builder.go +++ b/apps/backend/internal/office/runtime/context_builder.go @@ -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) diff --git a/apps/backend/internal/office/runtime/context_builder_test.go b/apps/backend/internal/office/runtime/context_builder_test.go index cf5bd0112b..e9d588e4f8 100644 --- a/apps/backend/internal/office/runtime/context_builder_test.go +++ b/apps/backend/internal/office/runtime/context_builder_test.go @@ -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) + } +} diff --git a/apps/backend/internal/office/service/decision_seat.go b/apps/backend/internal/office/service/decision_seat.go new file mode 100644 index 0000000000..065414e102 --- /dev/null +++ b/apps/backend/internal/office/service/decision_seat.go @@ -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 +} diff --git a/apps/backend/internal/office/service/decision_seat_test.go b/apps/backend/internal/office/service/decision_seat_test.go new file mode 100644 index 0000000000..b26af8c5b9 --- /dev/null +++ b/apps/backend/internal/office/service/decision_seat_test.go @@ -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") + } +} diff --git a/apps/backend/internal/office/service/prompt_builder_test.go b/apps/backend/internal/office/service/prompt_builder_test.go index 89bb327964..ab64b70609 100644 --- a/apps/backend/internal/office/service/prompt_builder_test.go +++ b/apps/backend/internal/office/service/prompt_builder_test.go @@ -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, diff --git a/apps/backend/internal/office/service/scheduler_integration.go b/apps/backend/internal/office/service/scheduler_integration.go index 769004474a..b9262aca7f 100644 --- a/apps/backend/internal/office/service/scheduler_integration.go +++ b/apps/backend/internal/office/service/scheduler_integration.go @@ -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 diff --git a/apps/backend/internal/office/service/scheduler_integration_routing_test.go b/apps/backend/internal/office/service/scheduler_integration_routing_test.go index 1baed15cee..b95424779e 100644 --- a/apps/backend/internal/office/service/scheduler_integration_routing_test.go +++ b/apps/backend/internal/office/service/scheduler_integration_routing_test.go @@ -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 diff --git a/apps/backend/internal/workflow/engine/quorum.go b/apps/backend/internal/workflow/engine/quorum.go index 1b976ba6cc..ad18ae90fd 100644 --- a/apps/backend/internal/workflow/engine/quorum.go +++ b/apps/backend/internal/workflow/engine/quorum.go @@ -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 diff --git a/apps/backend/internal/workflow/engine/quorum_agent_resolver_test.go b/apps/backend/internal/workflow/engine/quorum_agent_resolver_test.go index 80e48831f0..76d4d60b1f 100644 --- a/apps/backend/internal/workflow/engine/quorum_agent_resolver_test.go +++ b/apps/backend/internal/workflow/engine/quorum_agent_resolver_test.go @@ -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}, diff --git a/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md b/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md new file mode 100644 index 0000000000..26109791bb --- /dev/null +++ b/docs/decisions/2026-09-07-separate-runtime-permissions-and-advisory-actions.md @@ -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. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index f2e3e4da36..a40f323d25 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -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 | diff --git a/docs/specs/office/system-design/runtime-01.md b/docs/specs/office/system-design/runtime-01.md index 9a80f5d1c3..b5fc310fc2 100644 --- a/docs/specs/office/system-design/runtime-01.md +++ b/docs/specs/office/system-design/runtime-01.md @@ -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. diff --git a/docs/specs/office/system-design/unread-divider.md b/docs/specs/office/system-design/unread-divider.md index 1fe47882f2..6857d78f68 100644 --- a/docs/specs/office/system-design/unread-divider.md +++ b/docs/specs/office/system-design/unread-divider.md @@ -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. -