From 8cb291ad7cdb317ff80947278ee055b1a4925b41 Mon Sep 17 00:00:00 2001 From: Claude Sonnet 5 Date: Sat, 4 Jul 2026 05:05:36 +0000 Subject: feat(story,role): add retro ceremony -- closes the self-improvement loop (Phase 8) The final mechanism the versioned role-config model (Phase 5) was built for: when a story reaches DONE, StoryOrchestrator spawns a retro-role task that reflects on the story's full history and proposes draft role_configs versions for a human to review and activate via the existing (unchanged) POST /api/roles/{role}/activate. - AgentChannel gains a 6th method, ProposeRoleConfig(ctx, role.RoleConfig) (version, err), following ProposeEpic's precedent (Phase 7c): a structured tool call, not summary-parsing. storeChannel.ProposeRoleConfig calls the same Store.CreateRoleConfig the human-facing POST /api/roles/{role}/versions endpoint already uses (proposed_by: "retro"), landing a new draft row without touching whatever's currently active. Wired through both transports exactly like ProposeEpic: internal/agentloop/tools.go (native loop) and internal/executor/agentmcp.go (MCP). - StoryOrchestrator.Tick now routes a story at status DONE to a new processRetro stage instead of processStory -- a sibling stage, not a continuation, since the Builder->Evaluators->Arbitration chain is long settled by then. processRetro only *reads* that settled pipeline (read-only findEvaluators/findArbitration counterparts to ensureEvaluators/ensureArbitration -- it never spawns/mutates Builder-pipeline tasks) to locate the Arbitration task the retro task depends on, then spawns (idempotently -- checks for an existing retro-role dependent first) one retro-role task with instructions assembled from the story's spec/acceptance-criteria, full task tree, per- task cost/escalation history, active role_configs per role encountered, and the story's own event stream (evaluator verdicts, arbitration decision). - event.KindRetroCaptured (attached to the story's ID, matching KindEvalVerdict/KindArbitrationDecided's convention) fires once the retro task completes (auto-accepted like every other pipeline task), aggregating every event.KindRoleConfigProposed the retro task recorded (one per propose_role_config call) into {task_id, proposals: [{role, version}], summary} -- the summary is the "capturing lessons" half of this ceremony, the proposals are the versioned-config half. - Human activation is completely untouched: drafts land through the identical CreateRoleConfig/config_json path Phase 5's endpoints already handle, confirmed via existing role-endpoint tests passing unmodified. go build/vet/test -race -count=1 all pass, full suite (20 packages) -- one run hit a known, pre-existing, intermittent flake under full-suite load (unrelated to this phase's files) that did not reproduce on two immediate reruns, both in isolation and full-suite. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01V1moSNCJRcP6kykA4tyUSs --- internal/executor/channel_test.go | 128 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) (limited to 'internal/executor/channel_test.go') diff --git a/internal/executor/channel_test.go b/internal/executor/channel_test.go index e05228a..522f23a 100644 --- a/internal/executor/channel_test.go +++ b/internal/executor/channel_test.go @@ -9,6 +9,8 @@ import ( "github.com/thepeterstone/claudomator/internal/agentchannel" "github.com/thepeterstone/claudomator/internal/event" + "github.com/thepeterstone/claudomator/internal/role" + "github.com/thepeterstone/claudomator/internal/storage" "github.com/thepeterstone/claudomator/internal/story" "github.com/thepeterstone/claudomator/internal/task" ) @@ -26,6 +28,13 @@ type fakeChannelStore struct { stories map[string]*story.Story updateStoryErr error updatedStories []*story.Story + + // roleConfigs backs CreateRoleConfig (Phase 8), keyed by role name, in + // creation order — mirrors storage.DB.CreateRoleConfig's own + // "auto-assign next version number for that role" behavior without a + // real database. + roleConfigs map[string][]*storage.RoleConfigRow + createRoleConfigErr error } func (f *fakeChannelStore) CreateTask(t *task.Task) error { @@ -81,6 +90,26 @@ func (f *fakeChannelStore) UpdateStory(st *story.Story) error { return nil } +func (f *fakeChannelStore) CreateRoleConfig(roleName, configJSON, proposedBy string) (*storage.RoleConfigRow, error) { + if f.createRoleConfigErr != nil { + return nil, f.createRoleConfigErr + } + if f.roleConfigs == nil { + f.roleConfigs = make(map[string][]*storage.RoleConfigRow) + } + version := len(f.roleConfigs[roleName]) + 1 + row := &storage.RoleConfigRow{ + ID: roleName + "-" + string(rune('0'+version)), + Role: roleName, + Version: version, + Status: "draft", + ConfigJSON: configJSON, + ProposedBy: proposedBy, + } + f.roleConfigs[roleName] = append(f.roleConfigs[roleName], row) + return row, nil +} + func TestStoreChannel_AskUser_BuffersAndBlocks(t *testing.T) { ch := newStoreChannel(&fakeChannelStore{}, "task-1") answer, err := ch.AskUser(context.Background(), `{"text":"q"}`) @@ -410,3 +439,102 @@ func TestStoreChannel_ProposeEpic_EmitsEpicProposedEvent(t *testing.T) { t.Errorf("payload.StoryIDs should only include the resolved story, got %+v", payload.StoryIDs) } } + +// TestStoreChannel_ProposeRoleConfig_CreatesDraftRow proves verification +// item (c): calling ProposeRoleConfig for a role creates a new +// draft-status role_configs row without touching whatever's currently +// active (the fake store here has no notion of "active" at all -- draft +// creation via CreateRoleConfig never reads or writes activation state, +// mirroring storage.CreateRoleConfig/ActivateRoleConfigVersion being +// entirely separate code paths). +func TestStoreChannel_ProposeRoleConfig_CreatesDraftRow(t *testing.T) { + store := &fakeChannelStore{} + ch := newStoreChannel(store, "retro-task-1") + + version, err := ch.ProposeRoleConfig(context.Background(), role.RoleConfig{ + Role: "builder", + SystemPrompt: "Be more careful about edge cases.", + }) + if err != nil { + t.Fatalf("ProposeRoleConfig: %v", err) + } + if version != 1 { + t.Errorf("expected version 1 for a role with no existing versions, got %d", version) + } + + rows := store.roleConfigs["builder"] + if len(rows) != 1 { + t.Fatalf("expected 1 role_configs row created, got %d", len(rows)) + } + row := rows[0] + if row.Status != "draft" { + t.Errorf("expected status draft, got %q", row.Status) + } + if row.ProposedBy != "retro" { + t.Errorf("expected proposed_by retro, got %q", row.ProposedBy) + } + var decoded role.RoleConfig + if err := json.Unmarshal([]byte(row.ConfigJSON), &decoded); err != nil { + t.Fatalf("unmarshal config_json: %v", err) + } + if decoded.Role != "builder" || decoded.SystemPrompt != "Be more careful about edge cases." { + t.Errorf("config_json round-trip mismatch: %+v", decoded) + } +} + +// TestStoreChannel_ProposeRoleConfig_VersionsIncrement proves a second +// proposal for the same role gets the next version number, mirroring +// storage.CreateRoleConfig's own auto-increment behavior. +func TestStoreChannel_ProposeRoleConfig_VersionsIncrement(t *testing.T) { + store := &fakeChannelStore{} + ch := newStoreChannel(store, "retro-task-1") + + v1, err := ch.ProposeRoleConfig(context.Background(), role.RoleConfig{Role: "builder"}) + if err != nil { + t.Fatalf("first ProposeRoleConfig: %v", err) + } + v2, err := ch.ProposeRoleConfig(context.Background(), role.RoleConfig{Role: "builder"}) + if err != nil { + t.Fatalf("second ProposeRoleConfig: %v", err) + } + if v1 != 1 || v2 != 2 { + t.Errorf("expected versions 1 then 2, got %d then %d", v1, v2) + } +} + +// TestStoreChannel_ProposeRoleConfig_EmitsRoleConfigProposedEvent proves +// verification item (d)'s prerequisite: each ProposeRoleConfig call records +// a KindRoleConfigProposed event attached to the *calling task's* ID (not a +// role or story ID -- roles have no first-class row of their own), payload +// {role, version}, which internal/scheduler.StoryOrchestrator's +// finalizeRetro later aggregates into KindRetroCaptured. +func TestStoreChannel_ProposeRoleConfig_EmitsRoleConfigProposedEvent(t *testing.T) { + store := &fakeChannelStore{} + ch := newStoreChannel(store, "retro-task-1") + + version, err := ch.ProposeRoleConfig(context.Background(), role.RoleConfig{Role: "evaluator_quality"}) + if err != nil { + t.Fatalf("ProposeRoleConfig: %v", err) + } + + if len(store.createdEvents) != 1 { + t.Fatalf("expected 1 event, got %d", len(store.createdEvents)) + } + ev := store.createdEvents[0] + if ev.Kind != event.KindRoleConfigProposed || ev.Actor != event.ActorAgent { + t.Errorf("got kind=%v actor=%v want role_config_proposed/agent", ev.Kind, ev.Actor) + } + if ev.TaskID != "retro-task-1" { + t.Errorf("event should be attached to the calling task ID, got %q", ev.TaskID) + } + var payload struct { + Role string `json:"role"` + Version int `json:"version"` + } + if err := json.Unmarshal(ev.Payload, &payload); err != nil { + t.Fatalf("unmarshal payload: %v", err) + } + if payload.Role != "evaluator_quality" || payload.Version != version { + t.Errorf("payload mismatch: %+v (want role=evaluator_quality version=%d)", payload, version) + } +} -- cgit v1.2.3