fix(office): stop review and approval from seating the same agent (#3239)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -8,30 +8,67 @@ import (
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
"github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
"github.com/kandev/kandev/internal/workflow/engine"
|
||||
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||
)
|
||||
|
||||
// SeatCasterWorkflowRepo captures the workflow-repo subset the seat caster
|
||||
// needs to resolve the task's runner (REQ-OFFICE-REVIEW-SEATS-002's "the
|
||||
// task's runner" fallback and self-review comparison). The caller supplies
|
||||
// the immutable step that the task entered, so this adapter does not re-read
|
||||
// needs: resolving the task's runner (REQ-OFFICE-REVIEW-SEATS-002's "the
|
||||
// task's runner" fallback and self-review comparison), and listing every
|
||||
// participant seat already recorded for the task's current workflow, for
|
||||
// the cross-step exclusion in castFromCandidates. The caller supplies the
|
||||
// immutable step that the task entered, so this adapter does not re-read
|
||||
// mutable task state after the transition commits.
|
||||
type SeatCasterWorkflowRepo interface {
|
||||
ResolveCurrentRunner(ctx context.Context, stepID, taskID string) (string, error)
|
||||
ListParticipantsForTaskWorkflow(ctx context.Context, taskID, workflowID string) ([]*wfmodels.WorkflowStepParticipant, error)
|
||||
}
|
||||
|
||||
// seatRolePools maps a participant role to the set of agent roles eligible
|
||||
// for its seat, per AC-OFFICE-REVIEW-SEATS-002.1: approver draws from `ceo`;
|
||||
// reviewer draws from `ceo` ∪ `specialist` — and only those two. Any other
|
||||
// participant role (watcher, collaborator, ...) keeps today's ceo-only
|
||||
// behavior; the AC does not cover them, and the pool must not be widened
|
||||
// without an amended AC.
|
||||
var seatRolePools = map[string]map[models.AgentRole]bool{
|
||||
string(wfmodels.ParticipantRoleReviewer): {
|
||||
models.AgentRoleCEO: true,
|
||||
models.AgentRoleSpecialist: true,
|
||||
},
|
||||
string(wfmodels.ParticipantRoleApprover): {
|
||||
models.AgentRoleCEO: true,
|
||||
},
|
||||
}
|
||||
|
||||
// eligibleAgentRolesFor returns role's agent-role allowlist. It is always a
|
||||
// positive match, never a denylist: eligibleCandidates lists agents with no
|
||||
// Role filter (a union cannot be expressed as a single filter value), so
|
||||
// this allowlist is what keeps worker/assistant/empty-role agent profiles
|
||||
// from ever being seated.
|
||||
func eligibleAgentRolesFor(role string) map[models.AgentRole]bool {
|
||||
if pool, ok := seatRolePools[role]; ok {
|
||||
return pool
|
||||
}
|
||||
return map[models.AgentRole]bool{models.AgentRoleCEO: true}
|
||||
}
|
||||
|
||||
// SeatCasterAdapter implements engine.ParticipantSeatCaster by applying the
|
||||
// casting resolution algorithm of REQ-OFFICE-REVIEW-SEATS-002 (system design
|
||||
// "Casting resolution"):
|
||||
//
|
||||
// 1. List the task's workspace's Office agents whose role is `ceo` and whose
|
||||
// status is neither `stopped` nor `pending_approval`, ordered by
|
||||
// `created_at` then agent profile identifier.
|
||||
// 1. List the task's workspace's Office agents eligible for role's seat
|
||||
// (eligibleAgentRolesFor) and whose status is neither `stopped` nor
|
||||
// `pending_approval`, ordered by `created_at` then agent profile
|
||||
// identifier.
|
||||
// 2. Empty list: seat the task's runner (fallback provenance); no runner
|
||||
// resolves is unfillable.
|
||||
// 3. Otherwise seat the candidate list's first member unless that member is
|
||||
// the task's runner and an alternative exists, in which case seat the
|
||||
// second member instead — this is what keeps an ordinary cast from
|
||||
// being a self-review when a different reviewer is available.
|
||||
// 3. Otherwise seat the first candidate that is neither the task's runner
|
||||
// nor already holding another seat on this task's current workflow
|
||||
// (best-effort cross-step exclusion, AC-OFFICE-REVIEW-SEATS-002.3) —
|
||||
// this is what lets a reviewer and an approver land on different agents
|
||||
// when the workspace shape allows it. If no such candidate exists, fall
|
||||
// back to the runner-exclusion-only rule (skip the runner if any other
|
||||
// candidate remains, otherwise seat the runner) so a seat is never left
|
||||
// empty.
|
||||
//
|
||||
// The status exclusion is applied here, over ListAgentInstancesFiltered's
|
||||
// result, rather than by adding a filter to that shared method or changing
|
||||
@@ -51,7 +88,7 @@ func NewSeatCasterAdapter(office OfficeRepo, workflow SeatCasterWorkflowRepo) *S
|
||||
|
||||
// CastParticipantSeat satisfies engine.ParticipantSeatCaster.
|
||||
func (a *SeatCasterAdapter) CastParticipantSeat(
|
||||
ctx context.Context, taskID, stepID, role string,
|
||||
ctx context.Context, workflowID, taskID, stepID, role string,
|
||||
) (engine.ParticipantSeatCastResult, error) {
|
||||
if taskID == "" {
|
||||
return engine.ParticipantSeatCastResult{}, fmt.Errorf("task_id is required to cast a participant seat")
|
||||
@@ -72,29 +109,38 @@ func (a *SeatCasterAdapter) CastParticipantSeat(
|
||||
return engine.ParticipantSeatCastResult{}, fmt.Errorf("resolve task runner: %w", err)
|
||||
}
|
||||
|
||||
candidates, err := a.eligibleCEOCandidates(ctx, fields.WorkspaceID)
|
||||
candidates, err := a.eligibleCandidates(ctx, fields.WorkspaceID, role)
|
||||
if err != nil {
|
||||
return engine.ParticipantSeatCastResult{}, fmt.Errorf("list eligible candidates: %w", err)
|
||||
}
|
||||
|
||||
return castFromCandidates(candidates, runner, fields.WorkspaceID), nil
|
||||
return castFromCandidates(candidates, runner, a.alreadySeatedAgents(ctx, taskID, workflowID), fields.WorkspaceID), nil
|
||||
}
|
||||
|
||||
// eligibleCEOCandidates returns the workspace's Office agents whose role is
|
||||
// `ceo` and whose status is neither `stopped` nor `pending_approval`,
|
||||
// ordered by `created_at` ascending then agent profile identifier ascending
|
||||
// (AC-OFFICE-REVIEW-SEATS-002.1, -002.2).
|
||||
func (a *SeatCasterAdapter) eligibleCEOCandidates(
|
||||
ctx context.Context, workspaceID string,
|
||||
// eligibleCandidates returns the workspace's Office agents eligible for
|
||||
// role's seat (eligibleAgentRolesFor) whose status is neither `stopped` nor
|
||||
// `pending_approval`, ordered by `created_at` ascending then agent profile
|
||||
// identifier ascending (AC-OFFICE-REVIEW-SEATS-002.1, -002.2).
|
||||
//
|
||||
// ListAgentInstancesFiltered's Role filter is a single string and cannot
|
||||
// express the reviewer pool's ceo ∪ specialist union, so this lists with an
|
||||
// empty Role filter and applies the allowlist here instead of pushing a
|
||||
// union down to the shared listing method (AC-OFFICE-REVIEW-SEATS-002.2 —
|
||||
// a shared listing whose behavior changes for everyone is how an unrelated
|
||||
// Office surface silently loses agents).
|
||||
func (a *SeatCasterAdapter) eligibleCandidates(
|
||||
ctx context.Context, workspaceID, role string,
|
||||
) ([]*models.AgentInstance, error) {
|
||||
agents, err := a.Office.ListAgentInstancesFiltered(ctx, workspaceID, sqlite.AgentListFilter{
|
||||
Role: string(models.AgentRoleCEO),
|
||||
})
|
||||
agents, err := a.Office.ListAgentInstancesFiltered(ctx, workspaceID, sqlite.AgentListFilter{})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
allowedRoles := eligibleAgentRolesFor(role)
|
||||
eligible := make([]*models.AgentInstance, 0, len(agents))
|
||||
for _, ag := range agents {
|
||||
if !allowedRoles[ag.Role] {
|
||||
continue
|
||||
}
|
||||
if ag.Status == models.AgentStatusStopped || ag.Status == models.AgentStatusPendingApproval {
|
||||
continue
|
||||
}
|
||||
@@ -110,13 +156,47 @@ func (a *SeatCasterAdapter) eligibleCEOCandidates(
|
||||
return eligible, nil
|
||||
}
|
||||
|
||||
// alreadySeatedAgents returns the set of agent profile ids already holding a
|
||||
// participant seat anywhere on taskID's current workflow (workflowID), for
|
||||
// castFromCandidates' best-effort cross-step exclusion. Scoping to workflowID
|
||||
// keeps rows left over from a workflow the task has since switched away from
|
||||
// (switch_workflow durably keeps them; see ListParticipantsForTaskWorkflow's
|
||||
// doc comment) from ever counting as already seated. A read failure is
|
||||
// non-fatal: the exclusion signal is optional and must never block casting a
|
||||
// seat (AC-OFFICE-REVIEW-SEATS-002.3's best-effort clause), so this returns
|
||||
// an empty set instead of propagating the error, and the caller casts
|
||||
// without exclusion.
|
||||
func (a *SeatCasterAdapter) alreadySeatedAgents(ctx context.Context, taskID, workflowID string) map[string]bool {
|
||||
seated := map[string]bool{}
|
||||
participants, err := a.Workflow.ListParticipantsForTaskWorkflow(ctx, taskID, workflowID)
|
||||
if err != nil {
|
||||
return seated
|
||||
}
|
||||
for _, p := range participants {
|
||||
if p.AgentProfileID != "" {
|
||||
seated[p.AgentProfileID] = true
|
||||
}
|
||||
}
|
||||
return seated
|
||||
}
|
||||
|
||||
// castFromCandidates applies steps 2-5 of the casting resolution algorithm
|
||||
// to an already-ordered, already-filtered candidate list and a resolved
|
||||
// runner (which may be empty, meaning "does not resolve"). workspaceID is
|
||||
// stamped onto every result, including an Unfillable one, so the caller can
|
||||
// emit AC-OFFICE-REVIEW-SEATS-004.1's warning record without a second
|
||||
// lookup.
|
||||
func castFromCandidates(candidates []*models.AgentInstance, runner, workspaceID string) engine.ParticipantSeatCastResult {
|
||||
// to an already-ordered, already-filtered candidate list, a resolved runner
|
||||
// (which may be empty, meaning "does not resolve"), and the set of agent
|
||||
// profile ids already seated elsewhere on this task's current workflow.
|
||||
// workspaceID is stamped onto every result, including an Unfillable one, so
|
||||
// the caller can emit AC-OFFICE-REVIEW-SEATS-004.1's warning record without
|
||||
// a second lookup.
|
||||
//
|
||||
// The cross-step exclusion is best-effort and never blocking
|
||||
// (AC-OFFICE-REVIEW-SEATS-002.3): if every candidate is either the runner or
|
||||
// already seated, the runner-exclusion-only rule decides instead of leaving
|
||||
// the seat empty. SelfReview is then true for either reason, since both mean
|
||||
// the chosen agent is already participating in this task's review in some
|
||||
// capacity.
|
||||
func castFromCandidates(
|
||||
candidates []*models.AgentInstance, runner string, alreadySeated map[string]bool, workspaceID string,
|
||||
) engine.ParticipantSeatCastResult {
|
||||
if len(candidates) == 0 {
|
||||
if runner == "" {
|
||||
return engine.ParticipantSeatCastResult{Unfillable: true, WorkspaceID: workspaceID}
|
||||
@@ -128,16 +208,26 @@ func castFromCandidates(candidates []*models.AgentInstance, runner, workspaceID
|
||||
SelfReview: true,
|
||||
}
|
||||
}
|
||||
idx := 0
|
||||
if candidates[0].ID == runner && len(candidates) > 1 {
|
||||
idx = 1
|
||||
|
||||
idx := -1
|
||||
for i, c := range candidates {
|
||||
if c.ID != runner && !alreadySeated[c.ID] {
|
||||
idx = i
|
||||
break
|
||||
}
|
||||
}
|
||||
if idx == -1 {
|
||||
idx = 0
|
||||
if candidates[0].ID == runner && len(candidates) > 1 {
|
||||
idx = 1
|
||||
}
|
||||
}
|
||||
chosen := candidates[idx]
|
||||
return engine.ParticipantSeatCastResult{
|
||||
AgentProfileID: chosen.ID,
|
||||
WorkspaceID: workspaceID,
|
||||
Provenance: engine.SeatProvenanceEligiblePool,
|
||||
SelfReview: chosen.ID == runner,
|
||||
SelfReview: chosen.ID == runner || alreadySeated[chosen.ID],
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,12 +3,14 @@ package engine_adapters
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
"github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
"github.com/kandev/kandev/internal/workflow/engine"
|
||||
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||
)
|
||||
|
||||
// fakeSeatCasterWorkflowRepo is a minimal SeatCasterWorkflowRepo fake for
|
||||
@@ -17,6 +19,11 @@ type fakeSeatCasterWorkflowRepo struct {
|
||||
runner string
|
||||
runnerErr error
|
||||
|
||||
seatedAgentIDs []string
|
||||
participantsErr error
|
||||
gotParticipantsFor string
|
||||
gotParticipantsForWF string
|
||||
|
||||
gotRunnerStepID string
|
||||
gotRunnerTaskID string
|
||||
}
|
||||
@@ -27,12 +34,27 @@ func (f *fakeSeatCasterWorkflowRepo) ResolveCurrentRunner(_ context.Context, ste
|
||||
return f.runner, f.runnerErr
|
||||
}
|
||||
|
||||
func (f *fakeSeatCasterWorkflowRepo) ListParticipantsForTaskWorkflow(
|
||||
_ context.Context, taskID, workflowID string,
|
||||
) ([]*wfmodels.WorkflowStepParticipant, error) {
|
||||
f.gotParticipantsFor = taskID
|
||||
f.gotParticipantsForWF = workflowID
|
||||
if f.participantsErr != nil {
|
||||
return nil, f.participantsErr
|
||||
}
|
||||
participants := make([]*wfmodels.WorkflowStepParticipant, 0, len(f.seatedAgentIDs))
|
||||
for _, id := range f.seatedAgentIDs {
|
||||
participants = append(participants, &wfmodels.WorkflowStepParticipant{AgentProfileID: id})
|
||||
}
|
||||
return participants, nil
|
||||
}
|
||||
|
||||
func TestSeatCasterAdapter_EmptyCandidateListFallsBackToRunner(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"}}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -58,7 +80,7 @@ func TestSeatCasterAdapter_EmptyCandidateListAndNoRunnerIsUnfillable(t *testing.
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -84,7 +106,7 @@ func TestSeatCasterAdapter_SingleCandidateIsRunnerRecordsSelfReview(t *testing.T
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "agent-A"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -112,7 +134,7 @@ func TestSeatCasterAdapter_FirstCandidateIsRunnerSeatsSecond(t *testing.T) {
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "agent-A"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -140,7 +162,7 @@ func TestSeatCasterAdapter_FirstCandidateNotRunnerSeatsFirstAsEligiblePool(t *te
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "someone-else"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -168,7 +190,7 @@ func TestSeatCasterAdapter_ExcludesStoppedAndPendingApprovalCandidates(t *testin
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -200,7 +222,7 @@ func TestSeatCasterAdapter_OrdersCandidatesByCreatedAtThenIdentifier(t *testing.
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer")
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
@@ -211,7 +233,7 @@ func TestSeatCasterAdapter_OrdersCandidatesByCreatedAtThenIdentifier(t *testing.
|
||||
|
||||
func TestSeatCasterAdapter_RejectsEmptyTaskID(t *testing.T) {
|
||||
a := NewSeatCasterAdapter(&fakeOfficeRepo{}, &fakeSeatCasterWorkflowRepo{})
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "", "step-1", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "", "step-1", "reviewer"); err == nil {
|
||||
t.Fatal("expected error for empty task id")
|
||||
}
|
||||
}
|
||||
@@ -219,7 +241,7 @@ func TestSeatCasterAdapter_RejectsEmptyTaskID(t *testing.T) {
|
||||
func TestSeatCasterAdapter_PropagatesWorkspaceResolutionError(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fieldsErr: errors.New("boom")}
|
||||
a := NewSeatCasterAdapter(office, &fakeSeatCasterWorkflowRepo{})
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer"); err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
}
|
||||
@@ -227,7 +249,7 @@ func TestSeatCasterAdapter_PropagatesWorkspaceResolutionError(t *testing.T) {
|
||||
func TestSeatCasterAdapter_PropagatesTaskWithNoWorkspaceAsError(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fields: &sqlite.TaskExecutionFields{ID: "t-1"}}
|
||||
a := NewSeatCasterAdapter(office, &fakeSeatCasterWorkflowRepo{})
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer"); err == nil {
|
||||
t.Fatal("expected error for a task with no workspace")
|
||||
}
|
||||
}
|
||||
@@ -236,7 +258,7 @@ func TestSeatCasterAdapter_RejectsEmptyStepID(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"}}
|
||||
wf := &fakeSeatCasterWorkflowRepo{}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "", "reviewer"); err == nil {
|
||||
t.Fatal("expected error for empty step id")
|
||||
}
|
||||
}
|
||||
@@ -245,7 +267,7 @@ func TestSeatCasterAdapter_PropagatesRunnerResolutionError(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"}}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runnerErr: errors.New("boom")}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer"); err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
}
|
||||
@@ -257,7 +279,7 @@ func TestSeatCasterAdapter_PropagatesCandidateListingError(t *testing.T) {
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "step-1", "reviewer"); err == nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", "reviewer"); err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
}
|
||||
@@ -267,7 +289,7 @@ func TestSeatCasterAdapter_UsesEnteredStepToResolveRunner(t *testing.T) {
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "t-1", "step-42", "reviewer"); err != nil {
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-42", "reviewer"); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if wf.gotRunnerStepID != "step-42" || wf.gotRunnerTaskID != "t-1" {
|
||||
@@ -276,6 +298,293 @@ func TestSeatCasterAdapter_UsesEnteredStepToResolveRunner(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ScopesExclusionReadToCallersWorkflow covers the P1
|
||||
// fix: the cross-step exclusion read must scope to the workflowID the
|
||||
// caller passes, not read task-wide across every workflow the task has ever
|
||||
// been on. A workflow the task switched away from durably keeps its
|
||||
// workflow_step_participants rows (switch_workflow), so an unscoped read
|
||||
// would count a stale row from that abandoned workflow as "already seated"
|
||||
// for the current one.
|
||||
func TestSeatCasterAdapter_ScopesExclusionReadToCallersWorkflow(t *testing.T) {
|
||||
office := &fakeOfficeRepo{fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"}}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
if _, err := a.CastParticipantSeat(context.Background(), "wf-current", "t-1", "step-1", "reviewer"); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if wf.gotParticipantsFor != "t-1" || wf.gotParticipantsForWF != "wf-current" {
|
||||
t.Errorf("ListParticipantsForTaskWorkflow called with task=%q workflow=%q, want t-1/wf-current",
|
||||
wf.gotParticipantsFor, wf.gotParticipantsForWF)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ReviewerPoolIncludesSpecialists is proof shape (a)
|
||||
// from the task plan: 2 ceos + specialists. Reviewer is cast first with no
|
||||
// exclusions in play, so it seats the earliest candidate; approver's
|
||||
// narrower ceo-only pool then excludes that agent, landing on the other
|
||||
// ceo. Fails with the change reverted, since the old code seats ceo-1 for
|
||||
// both roles.
|
||||
func TestSeatCasterAdapter_ReviewerPoolIncludesSpecialists(t *testing.T) {
|
||||
tCEO1 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
tCEO2 := tCEO1.Add(time.Minute)
|
||||
tSpec := tCEO1.Add(2 * time.Minute)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "ceo-1", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: tCEO1},
|
||||
{ID: "ceo-2", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: tCEO2},
|
||||
{ID: "spec-1", Role: models.AgentRoleSpecialist, Status: models.AgentStatusIdle, CreatedAt: tSpec},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
reviewer, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-review", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("reviewer cast: %v", err)
|
||||
}
|
||||
if reviewer.AgentProfileID != "ceo-1" {
|
||||
t.Fatalf("reviewer = %q, want ceo-1 (earliest created_at)", reviewer.AgentProfileID)
|
||||
}
|
||||
// Pins the regression a reverted seat_caster.go:132 (Role: ceo) would
|
||||
// reintroduce: the pool union can only be expressed by listing with no
|
||||
// Role filter and applying the allowlist in Go (AC-OFFICE-REVIEW-SEATS-002.2).
|
||||
if office.gotFilter.Role != "" {
|
||||
t.Errorf("reviewer cast: filter role = %q, want empty (pool allowlist is applied in Go, not pushed to the shared listing)", office.gotFilter.Role)
|
||||
}
|
||||
|
||||
wf.seatedAgentIDs = []string{reviewer.AgentProfileID}
|
||||
approver, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("approver cast: %v", err)
|
||||
}
|
||||
if approver.AgentProfileID != "ceo-2" {
|
||||
t.Fatalf("approver = %q, want ceo-2 (ceo-1 excluded; the approver pool never includes spec-1)", approver.AgentProfileID)
|
||||
}
|
||||
if office.gotFilter.Role != "" {
|
||||
t.Errorf("approver cast: filter role = %q, want empty (pool allowlist is applied in Go, not pushed to the shared listing)", office.gotFilter.Role)
|
||||
}
|
||||
if reviewer.AgentProfileID == approver.AgentProfileID {
|
||||
t.Fatal("expected reviewer and approver to be different agents")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ReviewerWidensToOlderSpecialist is proof shape (b):
|
||||
// 1 ceo + 1 specialist, with the specialist older. Reviewer's widened pool
|
||||
// picks the specialist over the ceo by created_at; approver's ceo-only pool
|
||||
// never sees the specialist at all. Fails with the change reverted, since
|
||||
// the old code never considers the specialist for either role.
|
||||
func TestSeatCasterAdapter_ReviewerWidensToOlderSpecialist(t *testing.T) {
|
||||
tSpec := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
tCEO := tSpec.Add(time.Minute)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "spec-1", Role: models.AgentRoleSpecialist, Status: models.AgentStatusIdle, CreatedAt: tSpec},
|
||||
{ID: "ceo-1", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: tCEO},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
reviewer, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-review", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("reviewer cast: %v", err)
|
||||
}
|
||||
if reviewer.AgentProfileID != "spec-1" {
|
||||
t.Fatalf("reviewer = %q, want spec-1 (earliest created_at across ceo ∪ specialist)", reviewer.AgentProfileID)
|
||||
}
|
||||
// Pins the regression a reverted seat_caster.go:132 (Role: ceo) would
|
||||
// reintroduce: the pool union can only be expressed by listing with no
|
||||
// Role filter and applying the allowlist in Go (AC-OFFICE-REVIEW-SEATS-002.2).
|
||||
if office.gotFilter.Role != "" {
|
||||
t.Errorf("reviewer cast: filter role = %q, want empty (pool allowlist is applied in Go, not pushed to the shared listing)", office.gotFilter.Role)
|
||||
}
|
||||
|
||||
wf.seatedAgentIDs = []string{reviewer.AgentProfileID}
|
||||
approver, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("approver cast: %v", err)
|
||||
}
|
||||
if approver.AgentProfileID != "ceo-1" {
|
||||
t.Fatalf("approver = %q, want ceo-1 (the approver pool is ceo-only)", approver.AgentProfileID)
|
||||
}
|
||||
if office.gotFilter.Role != "" {
|
||||
t.Errorf("approver cast: filter role = %q, want empty (pool allowlist is applied in Go, not pushed to the shared listing)", office.gotFilter.Role)
|
||||
}
|
||||
if reviewer.AgentProfileID == approver.AgentProfileID {
|
||||
t.Fatal("expected reviewer and approver to be different agents")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ApproverPoolStaysCEOOnly pins the D-B ruling's
|
||||
// accepted non-divergence: a workspace with exactly one ceo (plus any
|
||||
// number of specialists) seats the same agent for both reviewer and
|
||||
// approver, because the approver pool never widens to specialists. This is
|
||||
// ruled behavior, not a regression — do not "fix" it by widening the
|
||||
// approver pool.
|
||||
func TestSeatCasterAdapter_ApproverPoolStaysCEOOnly(t *testing.T) {
|
||||
t0 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
agents := []*models.AgentInstance{
|
||||
{ID: "ceo-1", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
}
|
||||
for i := 0; i < 4; i++ {
|
||||
agents = append(agents, &models.AgentInstance{
|
||||
ID: fmt.Sprintf("spec-%d", i),
|
||||
Role: models.AgentRoleSpecialist,
|
||||
Status: models.AgentStatusIdle,
|
||||
CreatedAt: t0.Add(time.Duration(i+1) * time.Minute),
|
||||
})
|
||||
}
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: agents,
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: ""}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
reviewer, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-review", "reviewer")
|
||||
if err != nil {
|
||||
t.Fatalf("reviewer cast: %v", err)
|
||||
}
|
||||
if reviewer.AgentProfileID != "ceo-1" {
|
||||
t.Fatalf("reviewer = %q, want ceo-1 (earliest created_at)", reviewer.AgentProfileID)
|
||||
}
|
||||
|
||||
wf.seatedAgentIDs = []string{reviewer.AgentProfileID}
|
||||
approver, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("approver cast: %v", err)
|
||||
}
|
||||
if approver.AgentProfileID != "ceo-1" {
|
||||
t.Fatalf("approver = %q, want ceo-1 (the sole ceo; must not widen to specialists)", approver.AgentProfileID)
|
||||
}
|
||||
if !approver.SelfReview {
|
||||
t.Error("expected SelfReview when the only approver candidate is already seated as reviewer")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ApproverPoolExcludesSpecialistsEvenWhenCEOPoolEmpty
|
||||
// covers AC-OFFICE-REVIEW-SEATS-002.5/002.6 for the narrower approver pool:
|
||||
// a workspace with specialists but no ceo still falls back to the runner
|
||||
// for an approver seat, rather than reaching into the specialist pool that
|
||||
// only reviewer is entitled to.
|
||||
func TestSeatCasterAdapter_ApproverPoolExcludesSpecialistsEvenWhenCEOPoolEmpty(t *testing.T) {
|
||||
t0 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "spec-1", Role: models.AgentRoleSpecialist, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if got.AgentProfileID != "runner-agent" {
|
||||
t.Errorf("agent = %q, want runner-agent (approver pool is ceo-only and has no members here)", got.AgentProfileID)
|
||||
}
|
||||
if got.Provenance != engine.SeatProvenanceRunnerFallback {
|
||||
t.Errorf("provenance = %q, want %q", got.Provenance, engine.SeatProvenanceRunnerFallback)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ExclusionNeverEmptiesASeat covers
|
||||
// AC-OFFICE-REVIEW-SEATS-002.3's never-blocking guarantee: the sole
|
||||
// candidate is already seated at another step on this task, and must be
|
||||
// seated anyway rather than left unfillable.
|
||||
func TestSeatCasterAdapter_ExclusionNeverEmptiesASeat(t *testing.T) {
|
||||
t0 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "ceo-1", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "", seatedAgentIDs: []string{"ceo-1"}}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if got.Unfillable {
|
||||
t.Fatal("expected the sole already-seated candidate to be seated anyway, not left unfillable")
|
||||
}
|
||||
if got.AgentProfileID != "ceo-1" {
|
||||
t.Errorf("agent = %q, want ceo-1", got.AgentProfileID)
|
||||
}
|
||||
if !got.SelfReview {
|
||||
t.Error("expected SelfReview when the only candidate already holds another seat on this task")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_ExclusionReadFailureIsNonFatal covers
|
||||
// AC-OFFICE-REVIEW-SEATS-002.3's best-effort clause: a failure reading the
|
||||
// cross-step exclusion signal must not fail the cast — it degrades to
|
||||
// casting without exclusion.
|
||||
func TestSeatCasterAdapter_ExclusionReadFailureIsNonFatal(t *testing.T) {
|
||||
t1 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
t2 := t1.Add(time.Minute)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "ceo-1", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: t1},
|
||||
{ID: "ceo-2", Role: models.AgentRoleCEO, Status: models.AgentStatusIdle, CreatedAt: t2},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "", participantsErr: errors.New("boom")}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-approve", "approver")
|
||||
if err != nil {
|
||||
t.Fatalf("expected the seat to still be cast when the exclusion read fails: %v", err)
|
||||
}
|
||||
if got.AgentProfileID != "ceo-1" {
|
||||
t.Errorf("agent = %q, want ceo-1 (cast without exclusion when the read errors)", got.AgentProfileID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSeatCasterAdapter_PoolAllowlistIsPositiveNotADenylist covers the
|
||||
// guard called out in the task plan: the empty-Role listing this adapter
|
||||
// now issues also returns worker/assistant/qa/security/devops and
|
||||
// empty-role agent profiles, and the Go allowlist must exclude them by not
|
||||
// naming their roles, not by naming roles to reject.
|
||||
func TestSeatCasterAdapter_PoolAllowlistIsPositiveNotADenylist(t *testing.T) {
|
||||
t0 := time.Date(2026, 8, 20, 12, 0, 0, 0, time.UTC)
|
||||
office := &fakeOfficeRepo{
|
||||
fields: &sqlite.TaskExecutionFields{ID: "t-1", WorkspaceID: "ws-1"},
|
||||
agents: []*models.AgentInstance{
|
||||
{ID: "worker-1", Role: models.AgentRoleWorker, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
{ID: "assistant-1", Role: models.AgentRoleAssistant, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
{ID: "qa-1", Role: models.AgentRoleQA, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
{ID: "security-1", Role: models.AgentRoleSecurity, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
{ID: "devops-1", Role: models.AgentRoleDevOps, Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
{ID: "no-role-1", Role: "", Status: models.AgentStatusIdle, CreatedAt: t0},
|
||||
},
|
||||
}
|
||||
wf := &fakeSeatCasterWorkflowRepo{runner: "runner-agent"}
|
||||
a := NewSeatCasterAdapter(office, wf)
|
||||
|
||||
for _, role := range []string{"reviewer", "approver"} {
|
||||
got, err := a.CastParticipantSeat(context.Background(), "wf-1", "t-1", "step-1", role)
|
||||
if err != nil {
|
||||
t.Fatalf("role %q: unexpected error: %v", role, err)
|
||||
}
|
||||
if got.AgentProfileID != "runner-agent" {
|
||||
t.Errorf("role %q: agent = %q, want runner-agent — none of worker/assistant/qa/security/devops/empty-role may be seated",
|
||||
role, got.AgentProfileID)
|
||||
}
|
||||
if got.Provenance != engine.SeatProvenanceRunnerFallback {
|
||||
t.Errorf("role %q: provenance = %q, want %q", role, got.Provenance, engine.SeatProvenanceRunnerFallback)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSeatCasterAdapter_SatisfiesParticipantSeatCaster(t *testing.T) {
|
||||
var _ engine.ParticipantSeatCaster = (*SeatCasterAdapter)(nil)
|
||||
}
|
||||
|
||||
@@ -165,6 +165,12 @@ const (
|
||||
// resolved for the role at all. WorkspaceID is populated on every result,
|
||||
// including Unfillable ones, so the AC-OFFICE-REVIEW-SEATS-004.1 warning
|
||||
// record can identify the workspace without a second lookup.
|
||||
//
|
||||
// SelfReview is true both when the chosen agent is the task's runner and
|
||||
// when it already holds another seat on this task (the office seat
|
||||
// caster's best-effort cross-step exclusion, AC-OFFICE-REVIEW-SEATS-002.3,
|
||||
// ran out of alternatives). It is a counter label on RecordSeatProvenance,
|
||||
// not a persisted or user-visible field.
|
||||
type ParticipantSeatCastResult struct {
|
||||
AgentProfileID string
|
||||
WorkspaceID string
|
||||
@@ -175,13 +181,18 @@ type ParticipantSeatCastResult struct {
|
||||
|
||||
// ParticipantSeatCaster resolves which agent should fill a role's seat when
|
||||
// none exists yet, per REQ-002's five-step deterministic algorithm. The
|
||||
// office package implements this against the workspace's CEO-role agent
|
||||
// roster; the engine treats the result as opaque.
|
||||
// office package implements this against role-specific workspace agent pools;
|
||||
// the engine treats the result as opaque.
|
||||
type ParticipantSeatCaster interface {
|
||||
// stepID is the immutable workflow step that the task entered. Callers
|
||||
// must pass this value instead of asking the adapter to re-read mutable
|
||||
// task state after the transition commits.
|
||||
CastParticipantSeat(ctx context.Context, taskID, stepID, role string) (ParticipantSeatCastResult, error)
|
||||
// task state after the transition commits. workflowID scopes any
|
||||
// cross-step exclusion read the caster performs to the task's current
|
||||
// workflow, so participant rows left over from a workflow the task has
|
||||
// since switched away from (switch_workflow durably keeps them; see
|
||||
// ListParticipantsForTaskWorkflow's doc comment) never count as already
|
||||
// seated for the new workflow's casting decision.
|
||||
CastParticipantSeat(ctx context.Context, workflowID, taskID, stepID, role string) (ParticipantSeatCastResult, error)
|
||||
}
|
||||
|
||||
// AgentProfileResolver answers whether an agent profile id still resolves to
|
||||
|
||||
@@ -49,18 +49,20 @@ func (f *fakeParticipantSeatWriter) EnsureRoleSeat(
|
||||
// fakeParticipantSeatCaster records CastParticipantSeat calls for
|
||||
// EnsureParticipantSeatCallback assertions.
|
||||
type fakeParticipantSeatCaster struct {
|
||||
result ParticipantSeatCastResult
|
||||
err error
|
||||
calls int
|
||||
lastTaskID string
|
||||
lastStepID string
|
||||
lastRole string
|
||||
result ParticipantSeatCastResult
|
||||
err error
|
||||
calls int
|
||||
lastWorkflowID string
|
||||
lastTaskID string
|
||||
lastStepID string
|
||||
lastRole string
|
||||
}
|
||||
|
||||
func (f *fakeParticipantSeatCaster) CastParticipantSeat(
|
||||
_ context.Context, taskID, stepID, role string,
|
||||
_ context.Context, workflowID, taskID, stepID, role string,
|
||||
) (ParticipantSeatCastResult, error) {
|
||||
f.calls++
|
||||
f.lastWorkflowID = workflowID
|
||||
f.lastTaskID = taskID
|
||||
f.lastStepID = stepID
|
||||
f.lastRole = role
|
||||
@@ -149,9 +151,10 @@ func TestEnsureParticipantSeatCallback_SuccessfulCastWritesSeat(t *testing.T) {
|
||||
if _, err := cb.Execute(context.Background(), newEnsureSeatInput("reviewer")); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if caster.calls != 1 || caster.lastTaskID != "task-1" || caster.lastStepID != "step-1" || caster.lastRole != "reviewer" {
|
||||
t.Fatalf("expected caster invoked once for (task-1, step-1, reviewer), got calls=%d taskID=%q stepID=%q role=%q",
|
||||
caster.calls, caster.lastTaskID, caster.lastStepID, caster.lastRole)
|
||||
if caster.calls != 1 || caster.lastWorkflowID != "wf-1" || caster.lastTaskID != "task-1" ||
|
||||
caster.lastStepID != "step-1" || caster.lastRole != "reviewer" {
|
||||
t.Fatalf("expected caster invoked once for (wf-1, task-1, step-1, reviewer), got calls=%d workflowID=%q taskID=%q stepID=%q role=%q",
|
||||
caster.calls, caster.lastWorkflowID, caster.lastTaskID, caster.lastStepID, caster.lastRole)
|
||||
}
|
||||
if len(writer.ensureCalls) != 1 {
|
||||
t.Fatalf("expected exactly one seat write, got %+v", writer.ensureCalls)
|
||||
|
||||
@@ -572,7 +572,7 @@ func (c EnsureParticipantSeatCallback) Execute(ctx context.Context, in ActionInp
|
||||
c.recordSeatEnsureError(taskID, stepID, role, err)
|
||||
return ActionResult{}, err
|
||||
}
|
||||
cast, err := c.Caster.CastParticipantSeat(ctx, taskID, stepID, role)
|
||||
cast, err := c.Caster.CastParticipantSeat(ctx, workflowID, taskID, stepID, role)
|
||||
if err != nil {
|
||||
c.recordSeatEnsureError(taskID, stepID, role, err)
|
||||
return ActionResult{}, fmt.Errorf("ensure_participant_seat cast: %w", err)
|
||||
|
||||
@@ -21,8 +21,8 @@ This capability makes those seats exist by default. When a task **enters** a
|
||||
step that declares it, the system resolves which Office agent should fill the
|
||||
step's participant role and writes a seat for that task at that step, before
|
||||
the step's own fan-out reads the slate. Casting is **derived**, not configured:
|
||||
the workspace CEO reviews, and the task's runner is the fallback when there is
|
||||
no CEO to seat.
|
||||
the reviewer pool uses `ceo` and `specialist`, while other roles use `ceo`.
|
||||
The runner is the fallback when the pool is empty.
|
||||
|
||||
Seats are written for the step being **entered**, never for the step the task
|
||||
stands on when it is created; that distinction is the whole defect behind the
|
||||
@@ -72,8 +72,10 @@ quorum primitives written into here, not the casting decision.
|
||||
row order; AC-OFFICE-REVIEW-SEATS-002.10 states why that distinction is a
|
||||
requirement rather than an implementation detail.
|
||||
- **Eligible agent** - an Office agent in the task's workspace whose Office
|
||||
agent role is `ceo` and whose status is neither `stopped` nor
|
||||
`pending_approval`.
|
||||
agent role is in the participant role's pool (`ceo` and `specialist` for the
|
||||
reviewer role; `ceo` alone for the approver role and for every other
|
||||
participant role, per AC-OFFICE-REVIEW-SEATS-002.1) and whose status is
|
||||
neither `stopped` nor `pending_approval`.
|
||||
- **Warning record** - the structured record accompanying each counter
|
||||
increment in REQ-OFFICE-REVIEW-SEATS-004, carrying the identifiers a counter
|
||||
label may not.
|
||||
@@ -138,7 +140,9 @@ configured.
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.1:** The system shall resolve the candidate list
|
||||
for a participant role to the eligible agents of the task's workspace,
|
||||
for a participant role to the task's workspace agents whose agent role is in
|
||||
that participant role's pool — `ceo` and `specialist` for the reviewer role;
|
||||
`ceo` alone for the approver role and for every other participant role —
|
||||
ordered by `created_at` ascending and then by agent profile identifier
|
||||
ascending.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.2:** The system shall exclude from the candidate
|
||||
@@ -146,8 +150,11 @@ configured.
|
||||
and shall do so without changing the behavior of any other caller of the
|
||||
agent listing it uses.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.3:** When the candidate list holds more than one
|
||||
eligible agent and its first member is the task's runner, the system shall
|
||||
seat the next member instead.
|
||||
eligible agent, the system shall seat the first member that is neither the
|
||||
task's runner nor already seated in another participant role on the task.
|
||||
This exclusion is best-effort and shall not leave the seat empty: when no
|
||||
such member exists, the system shall fall back to seating the first member,
|
||||
or the second member when the first is the task's runner.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.4:** When the candidate list holds exactly one
|
||||
eligible agent and that agent is the task's runner, the system shall seat
|
||||
that agent and shall record the self-review.
|
||||
@@ -163,10 +170,14 @@ configured.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.8:** When resolution fails with an error rather
|
||||
than an empty result, the system shall write no seat, emit the unfillable
|
||||
signal, and not fail the step entry.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.9:** When the candidate list holds more than one
|
||||
eligible agent and its first member is not the task's runner, the system
|
||||
shall seat that first member and record the seating as coming from the
|
||||
eligible pool rather than the fallback.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.9:** When the candidate list holds exactly one
|
||||
eligible agent and that agent is not the task's runner, the system shall
|
||||
seat that agent, record the seating as coming from the eligible pool rather
|
||||
than the fallback, and record the self-review when that agent is already
|
||||
seated in another participant role on the task. This criterion's domain
|
||||
(exactly one non-runner candidate) is disjoint from
|
||||
AC-OFFICE-REVIEW-SEATS-002.3's (more than one candidate); the two do not
|
||||
compete over the same input.
|
||||
- **AC-OFFICE-REVIEW-SEATS-002.10:** Resolution of the task's runner shall
|
||||
produce the same result on every database engine the product supports, and
|
||||
its final precedence tier shall order candidate seats by a persisted column
|
||||
@@ -312,11 +323,14 @@ Each exclusion below is a contract, not an omission.
|
||||
bringing an existing workspace's step *configuration* up to date is required
|
||||
by AC-OFFICE-REVIEW-SEATS-005.6 and is not a sweep, because it touches
|
||||
workflow rows once at startup and touches no task.
|
||||
- **Widening the eligible pool beyond the CEO role.** Office agent roles and
|
||||
participant roles are disjoint vocabularies and no Office agent role names a
|
||||
reviewer. The shipped `security` role's description does mention approving or
|
||||
blocking at review gates, so this is a live choice rather than a vacuous one;
|
||||
widening it belongs to the configurable-slate capability.
|
||||
- **Widening either pool beyond the roles AC-OFFICE-REVIEW-SEATS-002.1 names.**
|
||||
The reviewer pool is `ceo` and `specialist`; the approver pool, and every
|
||||
other participant role's pool, stays `ceo` alone — a per-role ruling, not a
|
||||
general relaxation. Office agent roles and participant roles remain disjoint
|
||||
vocabularies otherwise: the shipped `security` role's description does
|
||||
mention approving or blocking at review gates, so admitting it is a live
|
||||
choice rather than a vacuous one; widening either pool further belongs to
|
||||
the configurable-slate capability.
|
||||
- **The `backlog` step declaring no events.** A task started there has no
|
||||
turn-complete and no children-completed handler and parks permanently. Real,
|
||||
and not fixed by seats.
|
||||
|
||||
@@ -254,54 +254,54 @@ task rejected at Approval and reworked, same seat, new review round.
|
||||
|
||||
Given workspace, task, step and role:
|
||||
|
||||
1. List the workspace's Office agents whose role is `ceo` and whose status is
|
||||
neither `stopped` nor `pending_approval`, ordered by `created_at` then
|
||||
identifier.
|
||||
1. List the workspace's Office agents whose agent role is in that role's pool
|
||||
(`ceo` + `specialist` for reviewer, `ceo` alone otherwise, per the D-B
|
||||
ruling, 2026-09-01) and whose status is neither `stopped` nor
|
||||
`pending_approval`, ordered by `created_at` then identifier
|
||||
(`AC-OFFICE-REVIEW-SEATS-002.1`). Both filters apply over the shared
|
||||
listing's result, not a change to it, so other callers are unaffected
|
||||
(`AC-OFFICE-REVIEW-SEATS-002.2`).
|
||||
2. If the list is empty, seat the task's runner, record the fallback
|
||||
provenance, and record self-review if the runner is the seated agent
|
||||
(`AC-OFFICE-REVIEW-SEATS-002.5`). If the runner does not resolve, write no
|
||||
seat and emit the unfillable signal (`AC-OFFICE-REVIEW-SEATS-002.6`).
|
||||
3. If the list holds one agent and it is the task's runner, seat it and record
|
||||
self-review (`AC-OFFICE-REVIEW-SEATS-002.4`).
|
||||
4. If the list holds more than one agent and the first is the task's runner,
|
||||
seat the second (`AC-OFFICE-REVIEW-SEATS-002.3`).
|
||||
5. Otherwise seat the first, recording the provenance as *eligible pool* rather
|
||||
than *fallback* (`AC-OFFICE-REVIEW-SEATS-002.9`). This is the ordinary case,
|
||||
and naming it is what makes the provenance counter's values exhaustive
|
||||
rather than exceptional.
|
||||
self-review (`AC-OFFICE-REVIEW-SEATS-002.4`) — including a one-`ceo`
|
||||
workspace seating the same agent for both reviewer and approver, accepted
|
||||
under the D-B ruling.
|
||||
4. If the list holds more than one agent, seat the first member that is
|
||||
neither the runner nor already seated in another role on the task,
|
||||
workflow-wide (`AC-OFFICE-REVIEW-SEATS-002.3`). Best-effort: a failed read
|
||||
is non-fatal, and if every member is excluded, casting falls back to the
|
||||
first member (second, if the first is the runner) as before. Self-review
|
||||
is recorded either way.
|
||||
5. Otherwise (exactly one candidate, not the runner) seat it, recording the
|
||||
provenance as *eligible pool* rather than *fallback*, and recording
|
||||
self-review if it is already seated in another role on the task
|
||||
(`AC-OFFICE-REVIEW-SEATS-002.9`). Disjoint from step 4's domain.
|
||||
|
||||
**The runner resolver does not work on Postgres, and is repaired here.** Step 2
|
||||
is the only path that seats anyone in a CEO-less workspace, and in this card's
|
||||
scenario it is the path that runs: the task stands at `review`, its `runner`
|
||||
seat was written at `work`, and `review` declares no primary agent profile, so
|
||||
the resolver's first two tiers miss and the third decides. That third tier
|
||||
orders by `rowid`, which does not exist on Postgres, in a file that branches on
|
||||
the dialect elsewhere - so the call errors instead of returning an agent,
|
||||
routing to `AC-OFFICE-REVIEW-SEATS-002.6`: no seat, unfillable signal, task
|
||||
parked. The defect this capability exists to remove would survive on Postgres
|
||||
while every SQLite test passed.
|
||||
is the path that seats the task's runner when the eligible pool is empty. In
|
||||
this card's no-eligible-candidate scenario it runs: the task stands at
|
||||
`review`, its `runner` seat was written at `work`, and `review` declares no
|
||||
primary agent profile, so the resolver's
|
||||
first two tiers miss and the third decides. That tier orders by `rowid`,
|
||||
absent on Postgres, so the call errors instead of returning an agent, routing
|
||||
to `AC-OFFICE-REVIEW-SEATS-002.6`: no seat, unfillable signal, task parked -
|
||||
the defect this capability removes would survive on Postgres while every
|
||||
SQLite test passed.
|
||||
|
||||
Swapping `rowid` for the table's `id` does not fix it: that column is a text
|
||||
identifier with no ordering meaning, so ordering by it is a random pick wearing
|
||||
a determinism costume, which `AC-OFFICE-REVIEW-SEATS-002.10` forbids. The table
|
||||
carries no timestamp, so **no existing column can answer "most recently
|
||||
written"**.
|
||||
Swapping `rowid` for `id` does not fix it: `id` is a text identifier with no
|
||||
ordering meaning, so ordering by it is a random pick, which
|
||||
`AC-OFFICE-REVIEW-SEATS-002.10` forbids, and the table carries no timestamp to
|
||||
order by instead.
|
||||
|
||||
**Design position: the repair is in scope, and it is a column.** The migration
|
||||
*Persistence* already requires also adds a creation timestamp, and the third
|
||||
tier orders by it descending with `agent_profile_id` ascending as tiebreak - a
|
||||
named column and a named tiebreak, identical on both engines. Pre-existing rows
|
||||
backfill to one constant, so they are mutually tied and resolved by the
|
||||
tiebreak: still deterministic. Scoping the repair out was considered and
|
||||
rejected; it leaves this capability's only CEO-less path broken on one of two
|
||||
supported engines.
|
||||
|
||||
**Where the status exclusion is applied.** `AC-OFFICE-REVIEW-SEATS-002.2`
|
||||
requires excluding `stopped` and `pending_approval` without changing any other
|
||||
caller of the agent listing. The exclusion is therefore applied in the caster,
|
||||
over the listing's result, not by adding a filter to the shared listing method
|
||||
or by changing its default. A shared listing whose behavior changes for
|
||||
everyone is how an unrelated Office surface silently loses agents.
|
||||
**Design position: the repair is in scope, and it is a column.** The
|
||||
migration *Persistence* already requires also adds a creation timestamp; the
|
||||
third tier orders by it descending, `agent_profile_id` ascending as tiebreak -
|
||||
identical on both engines. Pre-existing rows backfill to one constant, tied
|
||||
and resolved by the tiebreak. Scoping the repair out was rejected: it leaves
|
||||
this capability's only CEO-less path broken on one supported engine.
|
||||
|
||||
## Failure and recovery
|
||||
|
||||
|
||||
Reference in New Issue
Block a user