diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..5a76eca --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,95 @@ +# Looper + +Looper is a local automation system for running role-based coding-agent workflows across GitHub projects. This context records the product language used when extending Looper's workflow behavior. + +## Language + +**Workflow Policy Pack**: +A replaceable set of role-bound workflow policies that Looper injects into agent prompts. +_Avoid_: skill pack, prompt bundle, Matt skills + +**Role**: +A built-in Looper responsibility lane that decides what kind of work an agent run performs. +_Avoid_: skill, agent type + +**Workflow Policy**: +A focused behavior rule that guides one role's agent prompt without changing Looper's lifecycle or output contracts. +_Avoid_: raw prompt, system prompt, skill + +**Prompt-Time Policy**: +A workflow policy that can change agent instructions but cannot change queueing, locks, GitHub side effects, lifecycle, recovery, or completion-marker behavior. +_Avoid_: runtime plugin, scheduler extension + +**Policy Pack ID**: +A stable machine-readable identifier used to bind a workflow policy pack in configuration. +_Avoid_: display name, title + +**Policy Pack Name**: +A human-friendly label for a workflow policy pack shown in CLI output, docs, and diagnostics. +_Avoid_: id, slug + +**Role Policy Binding**: +A configuration choice that assigns one workflow policy pack to one Looper role. +_Avoid_: per-policy selection, policy list + +**Role-Direct Policy Text**: +Workflow policy text authored directly for a Looper role inside a workflow policy pack. +_Avoid_: abstract policy graph, reusable policy atom + +**Policy Precedence**: +The prompt assembly order that places workflow policy pack instructions before user/project custom instructions and before Looper's final lifecycle contracts. +_Avoid_: priority, override order + +**Explicit Policy Binding**: +The v1 requirement that a role uses a workflow policy pack only when its role config names that pack. +_Avoid_: implicit global policy, default pack + +**Built-In Policy Pack**: +A workflow policy pack shipped with Looper and loaded through the same schema as file-based packs. +_Avoid_: hardcoded prompt constants + +**File Policy Pack**: +A workflow policy pack loaded from a user-provided local file. +_Avoid_: Go plugin, remote plugin + +**Policy Validation**: +The safety check that ensures workflow policy packs can guide role behavior without overriding Looper's protected lifecycle, safety, disclosure, or completion-marker contracts. +_Avoid_: lint only, schema only + +**Policy Visibility**: +The user-facing display of enabled workflow policy packs and role bindings before a loop runs. +_Avoid_: hidden prompt behavior + +## Relationships + +- A **Workflow Policy Pack** contains one or more **Workflow Policies**. +- A **Workflow Policy Pack** can be bound to one or more **Roles**. +- A **Role** can use a **Workflow Policy Pack** while still preserving Looper's lifecycle, safety, disclosure, and completion-marker contracts. +- A **Prompt-Time Policy** is the v1 boundary for **Workflow Policies**. +- A **Policy Pack ID** identifies a **Workflow Policy Pack** in config. +- A **Policy Pack Name** describes a **Workflow Policy Pack** for humans. +- A **Role Policy Binding** selects a whole **Workflow Policy Pack** for a **Role** in v1. +- **Role-Direct Policy Text** is the v1 content format for a **Workflow Policy Pack**. +- **Policy Precedence** lets user/project custom instructions refine a **Workflow Policy Pack** while Looper's lifecycle, safety, disclosure, and completion-marker contracts remain final. +- **Explicit Policy Binding** prevents **Workflow Policy Packs** from changing a **Role** unless the role names the pack. +- **Built-In Policy Packs** and **File Policy Packs** share the same pack schema and validation path. +- **Policy Validation** applies to both **Built-In Policy Packs** and **File Policy Packs**. +- **Policy Visibility** lets users inspect active **Role Policy Bindings** through config, status, or prompt preview commands. + +## Example dialogue + +> **Dev:** "Can we make the Matt skills replaceable?" +> **Domain expert:** "Yes, but in Looper they are **Workflow Policy Packs**. Users bind a pack to a **Role**, and Looper injects the matching **Workflow Policies** into that role's prompt." + +## Flagged ambiguities + +- "Matt skills" was used to mean Codex-local skills and Looper runtime behavior. Resolved: Looper should expose replaceable **Workflow Policy Packs**, not depend on Codex skill installation. +- "Pluggable component" could imply runtime plugins. Resolved: v1 **Workflow Policy Packs** are **Prompt-Time Policies** only. +- "name" could mean either a stable config key or a display label. Resolved: use **Policy Pack ID** for stable references and **Policy Pack Name** for human-friendly display. +- "select policies" could mean choosing individual policies from a pack. Resolved: v1 uses whole-pack **Role Policy Binding** only. +- "policy composition" could mean an abstract graph of reusable policy atoms. Resolved: v1 packs use **Role-Direct Policy Text**. +- "override" could imply policy packs outrank project-specific instructions. Resolved: **Policy Precedence** puts pack text before user/project custom instructions, and Looper contracts last. +- "enabled" could imply all roles automatically use a policy pack. Resolved: v1 requires **Explicit Policy Binding** per role. +- "built-in" could imply hardcoded Go prompt strings. Resolved: built-ins should be bundled files loaded like **File Policy Packs**. +- "validation" could mean only JSON shape validation. Resolved: **Policy Validation** also blocks attempts to alter Looper's protected contracts. +- "active policy" should not be hidden inside agent prompts. Resolved: **Policy Visibility** should show enabled packs and role bindings before runs execute. diff --git a/internal/api/handler.go b/internal/api/handler.go index 72ac133..6dc4adc 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -499,13 +499,14 @@ func (h *Handler) buildHealthResponse(ctx context.Context) (healthResponse, erro } type statusResponse struct { - Service statusService `json:"service"` - Storage statusStorage `json:"storage"` - Scheduler statusScheduler `json:"scheduler"` - Loops statusLoops `json:"loops"` - Safety statusSafety `json:"safety"` - Notifications statusNotifications `json:"notifications"` - Tools statusTools `json:"tools"` + Service statusService `json:"service"` + Storage statusStorage `json:"storage"` + Scheduler statusScheduler `json:"scheduler"` + Loops statusLoops `json:"loops"` + Safety statusSafety `json:"safety"` + WorkflowPolicies statusWorkflowPolicies `json:"workflowPolicies"` + Notifications statusNotifications `json:"notifications"` + Tools statusTools `json:"tools"` } type statusService struct { @@ -594,6 +595,17 @@ type statusSafety struct { OpenPRStrategy config.OpenPRStrategy `json:"openPrStrategy"` } +type statusWorkflowPolicies struct { + Enabled bool `json:"enabled"` + Bindings map[string]*statusWorkflowPolicyBinding `json:"bindings"` +} + +type statusWorkflowPolicyBinding struct { + ID string `json:"id"` + Name string `json:"name"` + Display string `json:"display"` +} + type statusNotifications struct { InAppEnabled bool `json:"inAppEnabled"` OsascriptEnabled bool `json:"osascriptEnabled"` @@ -606,19 +618,20 @@ type statusTools struct { } type configResponse struct { - Server configServerResponse `json:"server"` - Storage config.StorageConfig `json:"storage"` - Scheduler config.SchedulerConfig `json:"scheduler"` - Agent config.AgentConfig `json:"agent"` - Logging config.LoggingConfig `json:"logging"` - Notifications config.NotificationConfig `json:"notifications"` - Tools config.ToolPathsConfig `json:"tools"` - Daemon configDaemonResponse `json:"daemon"` - Package config.PackageConfig `json:"package"` - Defaults config.DefaultsConfig `json:"defaults"` - Reviewer config.ReviewerConfig `json:"reviewer"` - Roles config.RoleConfigs `json:"roles"` - Projects []config.ProjectRefConfig `json:"projects"` + Server configServerResponse `json:"server"` + Storage config.StorageConfig `json:"storage"` + Scheduler config.SchedulerConfig `json:"scheduler"` + Agent config.AgentConfig `json:"agent"` + Logging config.LoggingConfig `json:"logging"` + Notifications config.NotificationConfig `json:"notifications"` + Tools config.ToolPathsConfig `json:"tools"` + Daemon configDaemonResponse `json:"daemon"` + Package config.PackageConfig `json:"package"` + Defaults config.DefaultsConfig `json:"defaults"` + Reviewer config.ReviewerConfig `json:"reviewer"` + WorkflowPacks config.WorkflowPolicyPacksConfig `json:"workflowPolicyPacks"` + Roles config.RoleConfigs `json:"roles"` + Projects []config.ProjectRefConfig `json:"projects"` } type configServerResponse struct { @@ -665,11 +678,12 @@ func (h *Handler) buildConfigResponse() configResponse { WorkingDirectory: cfg.Daemon.WorkingDirectory, Environment: cfg.Daemon.Environment, }, - Package: cfg.Package, - Defaults: cfg.Defaults, - Reviewer: cfg.Reviewer, - Roles: cfg.Roles, - Projects: append([]config.ProjectRefConfig{}, cfg.Projects...), + Package: cfg.Package, + Defaults: cfg.Defaults, + Reviewer: cfg.Reviewer, + WorkflowPacks: cfg.WorkflowPolicyPacks, + Roles: cfg.Roles, + Projects: append([]config.ProjectRefConfig{}, cfg.Projects...), } } @@ -745,6 +759,7 @@ func (h *Handler) buildStatusResponse(ctx context.Context) (statusResponse, erro FixAllPullRequests: h.context.Config.Defaults.FixAllPullRequests, OpenPRStrategy: h.context.Config.Defaults.OpenPRStrategy, }, + WorkflowPolicies: buildWorkflowPolicyStatus(h.context.Config), Notifications: statusNotifications{ InAppEnabled: h.context.Config.Notifications.InApp, OsascriptEnabled: h.context.Config.Notifications.Osascript.Enabled, @@ -757,6 +772,31 @@ func (h *Handler) buildStatusResponse(ctx context.Context) (statusResponse, erro }, nil } +func buildWorkflowPolicyStatus(cfg config.Config) statusWorkflowPolicies { + bindings := map[string]*statusWorkflowPolicyBinding{ + "planner": workflowPolicyStatusBinding(cfg, cfg.Roles.Planner.PolicyPack), + "reviewer": workflowPolicyStatusBinding(cfg, cfg.Roles.Reviewer.PolicyPack), + "worker": workflowPolicyStatusBinding(cfg, cfg.Roles.Worker.PolicyPack), + "fixer": workflowPolicyStatusBinding(cfg, cfg.Roles.Fixer.PolicyPack), + } + return statusWorkflowPolicies{Enabled: cfg.WorkflowPolicyPacks.Enabled, Bindings: bindings} +} + +func workflowPolicyStatusBinding(cfg config.Config, packID string) *statusWorkflowPolicyBinding { + packID = strings.TrimSpace(packID) + if packID == "" { + return nil + } + name := packID + for _, ref := range cfg.WorkflowPolicyPacks.Packs { + if ref.ID == packID && strings.TrimSpace(ref.Name) != "" { + name = ref.Name + break + } + } + return &statusWorkflowPolicyBinding{ID: packID, Name: name, Display: fmt.Sprintf("%s (%s)", packID, name)} +} + type storageState struct { OK bool LatestAvailableID string diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index 5a4d8ed..3fc0cf9 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -71,6 +71,7 @@ func TestIsTerminalReviewerLoopRecordTreatsFailedAsTerminal(t *testing.T) { func TestHandlerStatusSuccessContainsExpectedSections(t *testing.T) { rt, cfg := startTestRuntime(t) + cfg.Roles.Worker.PolicyPack = "matt-series" seedStatusData(t, rt) seedStatusLoopCounts(t, rt) @@ -92,6 +93,7 @@ func TestHandlerStatusSuccessContainsExpectedSections(t *testing.T) { binaryInfo := service["binary"].(map[string]any) storageInfo := data["storage"].(map[string]any) scheduler := data["scheduler"].(map[string]any) + workflowPolicies := data["workflowPolicies"].(map[string]any) loops := data["loops"].(map[string]any) assertEqual(t, service["healthy"], true) @@ -111,6 +113,10 @@ func TestHandlerStatusSuccessContainsExpectedSections(t *testing.T) { } assertEqual(t, scheduler["totalRuns"], float64(1)) assertEqual(t, scheduler["activeRuns"], float64(1)) + workflowBindings := workflowPolicies["bindings"].(map[string]any) + workerBinding := workflowBindings["worker"].(map[string]any) + assertEqual(t, workflowPolicies["enabled"], true) + assertEqual(t, workerBinding["display"], "matt-series (Matt Series Engineering Workflow)") reviewer := loops["reviewer"].(map[string]any) assertEqual(t, reviewer["queued"], float64(1)) @@ -141,6 +147,7 @@ func TestHandlerConfigSuccessContainsExpectedSections(t *testing.T) { daemon := data["daemon"].(map[string]any) reviewer := data["reviewer"].(map[string]any) reviewerLoop := reviewer["loop"].(map[string]any) + workflowPacks := data["workflowPolicyPacks"].(map[string]any) assertEqual(t, server["host"], cfg.Server.Host) assertEqual(t, server["port"], float64(cfg.Server.Port)) @@ -157,6 +164,7 @@ func TestHandlerConfigSuccessContainsExpectedSections(t *testing.T) { assertEqual(t, threadResolution["mode"], string(cfg.Reviewer.ThreadResolution.Mode)) assertEqual(t, reviewerLoop["enabledByDefault"], cfg.Reviewer.Loop.EnabledByDefault) assertEqual(t, reviewerLoop["maxConsecutiveFailures"], float64(cfg.Reviewer.Loop.MaxConsecutiveFailures)) + assertEqual(t, workflowPacks["enabled"], cfg.WorkflowPolicyPacks.Enabled) if _, ok := daemon["shutdownTimeoutMs"]; ok { t.Fatalf("daemon.shutdownTimeoutMs should be omitted from config response: %#v", daemon) } diff --git a/internal/api/testdata/contracts/daemon-http.responses.compat.json b/internal/api/testdata/contracts/daemon-http.responses.compat.json index 9bfcfa8..7661bdb 100644 --- a/internal/api/testdata/contracts/daemon-http.responses.compat.json +++ b/internal/api/testdata/contracts/daemon-http.responses.compat.json @@ -138,6 +138,15 @@ "fixAllPullRequests": false, "openPrStrategy": "all_done" }, + "workflowPolicies": { + "enabled": true, + "bindings": { + "planner": null, + "reviewer": null, + "worker": null, + "fixer": null + } + }, "notifications": { "inAppEnabled": true, "osascriptEnabled": true @@ -273,6 +282,16 @@ "scope": "looper_authored_only" } }, + "workflowPolicyPacks": { + "enabled": true, + "packs": [ + { + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "source": "builtin" + } + ] + }, "roles": { "planner": { "autoDiscovery": true, diff --git a/internal/cliapp/app_test.go b/internal/cliapp/app_test.go index 2a7b9f9..3435330 100644 --- a/internal/cliapp/app_test.go +++ b/internal/cliapp/app_test.go @@ -798,6 +798,12 @@ func TestStatusWithoutJSONPrintsHumanReadableSections(t *testing.T) { "worker": map[string]any{"running": 2, "paused": 0, "failed": 1}, "fixer": map[string]any{"running": 0, "paused": 0, "failed": 0}, }, + "workflowPolicies": map[string]any{ + "enabled": true, + "bindings": map[string]any{ + "worker": map[string]any{"id": "matt-series", "name": "Matt Series Engineering Workflow", "display": "matt-series (Matt Series Engineering Workflow)"}, + }, + }, "tools": map[string]any{"git": true, "gh": false, "osascript": true}, "notifications": map[string]any{"inAppEnabled": true, "osascriptEnabled": false}, })) @@ -812,7 +818,7 @@ func TestStatusWithoutJSONPrintsHumanReadableSections(t *testing.T) { if stderr != "" { t.Fatalf("Run([status]) stderr = %q, want empty string", stderr) } - for _, want := range []string{"Service", "healthy : yes", "version : 1.2.3", "Storage", "Scheduler", "type", "reviewer", "Tools", "gh : no", "Notifications"} { + for _, want := range []string{"Service", "healthy : yes", "version : 1.2.3", "Storage", "Scheduler", "Workflow policies", "worker : matt-series (Matt Series Engineering Workflow)", "type", "reviewer", "Tools", "gh : no", "Notifications"} { if !strings.Contains(stdout, want) { t.Fatalf("Run([status]) stdout = %q, want to contain %q", stdout, want) } diff --git a/internal/cliapp/config_commands.go b/internal/cliapp/config_commands.go index b3ddb52..4465a86 100644 --- a/internal/cliapp/config_commands.go +++ b/internal/cliapp/config_commands.go @@ -36,6 +36,7 @@ var configFieldRegistry = map[string]configField{ "defaults.openPrStrategy": openPRStrategyField(), "instructions.enabled": boolField("instructions.enabled", "", "no-custom-instructions", func(c config.Config) any { return c.Instructions.Enabled }, func(p *config.PartialConfig) **bool { return &ensurePartialInstructions(p).Enabled }), "instructions.maxBytes": positiveIntField("instructions.maxBytes", "", "", func(c config.Config) any { return c.Instructions.MaxBytes }, func(p *config.PartialConfig) **int { return &ensurePartialInstructions(p).MaxBytes }), + "workflowPolicyPacks.enabled": boolField("workflowPolicyPacks.enabled", "", "", func(c config.Config) any { return c.WorkflowPolicyPacks.Enabled }, func(p *config.PartialConfig) **bool { return &ensurePartialWorkflowPolicyPacks(p).Enabled }), "reviewer.reviewEvents.clean": reviewerReviewEventField("reviewer.reviewEvents.clean", "LOOPER_REVIEWER_REVIEW_EVENTS_CLEAN", "reviewer-clean-review-event", func(c config.Config) any { return c.Reviewer.ReviewEvents.Clean }, func(p *config.PartialConfig) **config.ReviewerReviewEvent { return &ensurePartialReviewerReviewEvents(p).Clean }), @@ -44,6 +45,7 @@ var configFieldRegistry = map[string]configField{ }), "roles.planner.autoDiscovery": boolField("roles.planner.autoDiscovery", "LOOPER_ROLES_PLANNER_AUTO_DISCOVERY", "", func(c config.Config) any { return c.Roles.Planner.AutoDiscovery }, func(p *config.PartialConfig) **bool { return &ensurePartialPlannerRole(p).AutoDiscovery }), "roles.planner.instructions": stringField("roles.planner.instructions", "", "", func(c config.Config) any { return c.Roles.Planner.Instructions }, func(p *config.PartialConfig) **string { return &ensurePartialPlannerRole(p).Instructions }), + "roles.planner.policyPack": stringField("roles.planner.policyPack", "", "", func(c config.Config) any { return c.Roles.Planner.PolicyPack }, func(p *config.PartialConfig) **string { return &ensurePartialPlannerRole(p).PolicyPack }), "roles.planner.triggers.labels": stringListField("roles.planner.triggers.labels", "LOOPER_ROLES_PLANNER_TRIGGERS_LABELS", func(c config.Config) any { return c.Roles.Planner.Triggers.Labels }, func(p *config.PartialConfig) **[]string { return &ensurePartialPlannerTriggers(p).Labels }), "roles.planner.triggers.labelMode": labelModeField("roles.planner.triggers.labelMode", "LOOPER_ROLES_PLANNER_TRIGGERS_LABEL_MODE", func(c config.Config) any { return c.Roles.Planner.Triggers.LabelMode }, func(p *config.PartialConfig) **config.LabelMode { return &ensurePartialPlannerTriggers(p).LabelMode }), "roles.planner.triggers.requireAssigneeCurrentUser": boolField("roles.planner.triggers.requireAssigneeCurrentUser", "LOOPER_ROLES_PLANNER_TRIGGERS_REQUIRE_ASSIGNEE_CURRENT_USER", "", func(c config.Config) any { return c.Roles.Planner.Triggers.RequireAssigneeCurrentUser }, func(p *config.PartialConfig) **bool { @@ -51,6 +53,7 @@ var configFieldRegistry = map[string]configField{ }), "roles.worker.autoDiscovery": boolField("roles.worker.autoDiscovery", "LOOPER_ROLES_WORKER_AUTO_DISCOVERY", "", func(c config.Config) any { return c.Roles.Worker.AutoDiscovery }, func(p *config.PartialConfig) **bool { return &ensurePartialWorkerRole(p).AutoDiscovery }), "roles.worker.instructions": stringField("roles.worker.instructions", "", "", func(c config.Config) any { return c.Roles.Worker.Instructions }, func(p *config.PartialConfig) **string { return &ensurePartialWorkerRole(p).Instructions }), + "roles.worker.policyPack": stringField("roles.worker.policyPack", "", "", func(c config.Config) any { return c.Roles.Worker.PolicyPack }, func(p *config.PartialConfig) **string { return &ensurePartialWorkerRole(p).PolicyPack }), "roles.worker.triggers.labels": stringListField("roles.worker.triggers.labels", "LOOPER_ROLES_WORKER_TRIGGERS_LABELS", func(c config.Config) any { return c.Roles.Worker.Triggers.Labels }, func(p *config.PartialConfig) **[]string { return &ensurePartialWorkerTriggers(p).Labels }), "roles.worker.triggers.labelMode": labelModeField("roles.worker.triggers.labelMode", "LOOPER_ROLES_WORKER_TRIGGERS_LABEL_MODE", func(c config.Config) any { return c.Roles.Worker.Triggers.LabelMode }, func(p *config.PartialConfig) **config.LabelMode { return &ensurePartialWorkerTriggers(p).LabelMode }), "roles.worker.triggers.requireAssigneeCurrentUser": boolField("roles.worker.triggers.requireAssigneeCurrentUser", "LOOPER_ROLES_WORKER_TRIGGERS_REQUIRE_ASSIGNEE_CURRENT_USER", "", func(c config.Config) any { return c.Roles.Worker.Triggers.RequireAssigneeCurrentUser }, func(p *config.PartialConfig) **bool { @@ -58,6 +61,7 @@ var configFieldRegistry = map[string]configField{ }), "roles.reviewer.autoDiscovery": boolField("roles.reviewer.autoDiscovery", "LOOPER_ROLES_REVIEWER_AUTO_DISCOVERY", "", func(c config.Config) any { return c.Roles.Reviewer.AutoDiscovery }, func(p *config.PartialConfig) **bool { return &ensurePartialReviewerRole(p).AutoDiscovery }), "roles.reviewer.instructions": stringField("roles.reviewer.instructions", "", "", func(c config.Config) any { return c.Roles.Reviewer.Instructions }, func(p *config.PartialConfig) **string { return &ensurePartialReviewerRole(p).Instructions }), + "roles.reviewer.policyPack": stringField("roles.reviewer.policyPack", "", "", func(c config.Config) any { return c.Roles.Reviewer.PolicyPack }, func(p *config.PartialConfig) **string { return &ensurePartialReviewerRole(p).PolicyPack }), "roles.reviewer.triggers.includeDrafts": boolField("roles.reviewer.triggers.includeDrafts", "LOOPER_ROLES_REVIEWER_TRIGGERS_INCLUDE_DRAFTS", "", func(c config.Config) any { return c.Roles.Reviewer.Triggers.IncludeDrafts }, func(p *config.PartialConfig) **bool { return &ensurePartialReviewerRoleTriggers(p).IncludeDrafts }), "roles.reviewer.triggers.requireReviewRequest": boolField("roles.reviewer.triggers.requireReviewRequest", "LOOPER_ROLES_REVIEWER_TRIGGERS_REQUIRE_REVIEW_REQUEST", "", func(c config.Config) any { return c.Roles.Reviewer.Triggers.RequireReviewRequest }, func(p *config.PartialConfig) **bool { return &ensurePartialReviewerRoleTriggers(p).RequireReviewRequest @@ -70,6 +74,7 @@ var configFieldRegistry = map[string]configField{ "roles.reviewer.specReview.reviewingLabel": stringField("roles.reviewer.specReview.reviewingLabel", "LOOPER_ROLES_REVIEWER_SPEC_REVIEW_REVIEWING_LABEL", "", func(c config.Config) any { return c.Roles.Reviewer.SpecReview.ReviewingLabel }, func(p *config.PartialConfig) **string { return &ensurePartialReviewerSpecReview(p).ReviewingLabel }), "roles.fixer.autoDiscovery": boolField("roles.fixer.autoDiscovery", "LOOPER_ROLES_FIXER_AUTO_DISCOVERY", "", func(c config.Config) any { return c.Roles.Fixer.AutoDiscovery }, func(p *config.PartialConfig) **bool { return &ensurePartialFixerRole(p).AutoDiscovery }), "roles.fixer.instructions": stringField("roles.fixer.instructions", "", "", func(c config.Config) any { return c.Roles.Fixer.Instructions }, func(p *config.PartialConfig) **string { return &ensurePartialFixerRole(p).Instructions }), + "roles.fixer.policyPack": stringField("roles.fixer.policyPack", "", "", func(c config.Config) any { return c.Roles.Fixer.PolicyPack }, func(p *config.PartialConfig) **string { return &ensurePartialFixerRole(p).PolicyPack }), "roles.fixer.triggers.includeDrafts": boolField("roles.fixer.triggers.includeDrafts", "LOOPER_ROLES_FIXER_TRIGGERS_INCLUDE_DRAFTS", "", func(c config.Config) any { return c.Roles.Fixer.Triggers.IncludeDrafts }, func(p *config.PartialConfig) **bool { return &ensurePartialFixerRoleTriggers(p).IncludeDrafts }), "roles.fixer.triggers.labels": stringListField("roles.fixer.triggers.labels", "LOOPER_ROLES_FIXER_TRIGGERS_LABELS", func(c config.Config) any { return c.Roles.Fixer.Triggers.Labels }, func(p *config.PartialConfig) **[]string { return &ensurePartialFixerRoleTriggers(p).Labels }), "roles.fixer.triggers.labelMode": labelModeField("roles.fixer.triggers.labelMode", "LOOPER_ROLES_FIXER_TRIGGERS_LABEL_MODE", func(c config.Config) any { return c.Roles.Fixer.Triggers.LabelMode }, func(p *config.PartialConfig) **config.LabelMode { return &ensurePartialFixerRoleTriggers(p).LabelMode }), @@ -543,6 +548,13 @@ func ensurePartialInstructions(partial *config.PartialConfig) *config.PartialIns return partial.Instructions } +func ensurePartialWorkflowPolicyPacks(partial *config.PartialConfig) *config.PartialWorkflowPolicyPacksConfig { + if partial.WorkflowPolicyPacks == nil { + partial.WorkflowPolicyPacks = &config.PartialWorkflowPolicyPacksConfig{} + } + return partial.WorkflowPolicyPacks +} + func ensurePartialRoles(partial *config.PartialConfig) *config.PartialRoleConfigs { if partial.Roles == nil { partial.Roles = &config.PartialRoleConfigs{} @@ -668,6 +680,8 @@ func configFieldSet(partial config.PartialConfig, key string) bool { return partial.Instructions != nil && partial.Instructions.Enabled != nil case "instructions.maxBytes": return partial.Instructions != nil && partial.Instructions.MaxBytes != nil + case "workflowPolicyPacks.enabled": + return partial.WorkflowPolicyPacks != nil && partial.WorkflowPolicyPacks.Enabled != nil case "reviewer.reviewEvents.clean": return partial.Reviewer != nil && partial.Reviewer.ReviewEvents != nil && partial.Reviewer.ReviewEvents.Clean != nil case "reviewer.reviewEvents.blocking": @@ -676,6 +690,8 @@ func configFieldSet(partial config.PartialConfig, key string) bool { return partial.Roles != nil && partial.Roles.Planner != nil && partial.Roles.Planner.AutoDiscovery != nil case "roles.planner.instructions": return partial.Roles != nil && partial.Roles.Planner != nil && partial.Roles.Planner.Instructions != nil + case "roles.planner.policyPack": + return partial.Roles != nil && partial.Roles.Planner != nil && partial.Roles.Planner.PolicyPack != nil case "roles.planner.triggers.labels": return partial.Roles != nil && partial.Roles.Planner != nil && partial.Roles.Planner.Triggers != nil && partial.Roles.Planner.Triggers.Labels != nil case "roles.planner.triggers.labelMode": @@ -686,6 +702,8 @@ func configFieldSet(partial config.PartialConfig, key string) bool { return partial.Roles != nil && partial.Roles.Worker != nil && partial.Roles.Worker.AutoDiscovery != nil case "roles.worker.instructions": return partial.Roles != nil && partial.Roles.Worker != nil && partial.Roles.Worker.Instructions != nil + case "roles.worker.policyPack": + return partial.Roles != nil && partial.Roles.Worker != nil && partial.Roles.Worker.PolicyPack != nil case "roles.worker.triggers.labels": return partial.Roles != nil && partial.Roles.Worker != nil && partial.Roles.Worker.Triggers != nil && partial.Roles.Worker.Triggers.Labels != nil case "roles.worker.triggers.labelMode": @@ -696,6 +714,8 @@ func configFieldSet(partial config.PartialConfig, key string) bool { return partial.Roles != nil && partial.Roles.Reviewer != nil && partial.Roles.Reviewer.AutoDiscovery != nil case "roles.reviewer.instructions": return partial.Roles != nil && partial.Roles.Reviewer != nil && partial.Roles.Reviewer.Instructions != nil + case "roles.reviewer.policyPack": + return partial.Roles != nil && partial.Roles.Reviewer != nil && partial.Roles.Reviewer.PolicyPack != nil case "roles.reviewer.triggers.includeDrafts": return partial.Roles != nil && partial.Roles.Reviewer != nil && partial.Roles.Reviewer.Triggers != nil && partial.Roles.Reviewer.Triggers.IncludeDrafts != nil case "roles.reviewer.triggers.requireReviewRequest": @@ -712,6 +732,8 @@ func configFieldSet(partial config.PartialConfig, key string) bool { return partial.Roles != nil && partial.Roles.Fixer != nil && partial.Roles.Fixer.AutoDiscovery != nil case "roles.fixer.instructions": return partial.Roles != nil && partial.Roles.Fixer != nil && partial.Roles.Fixer.Instructions != nil + case "roles.fixer.policyPack": + return partial.Roles != nil && partial.Roles.Fixer != nil && partial.Roles.Fixer.PolicyPack != nil case "roles.fixer.triggers.includeDrafts": return partial.Roles != nil && partial.Roles.Fixer != nil && partial.Roles.Fixer.Triggers != nil && partial.Roles.Fixer.Triggers.IncludeDrafts != nil case "roles.fixer.triggers.labels": diff --git a/internal/cliapp/golden_test.go b/internal/cliapp/golden_test.go index 251b59b..cd21116 100644 --- a/internal/cliapp/golden_test.go +++ b/internal/cliapp/golden_test.go @@ -64,6 +64,12 @@ func TestCLIGoldenOutputs(t *testing.T) { "worker": map[string]any{"queued": 0, "running": 2, "waiting": 0, "paused": 0, "failed": 1, "terminated": 1, "stopped": 0}, "fixer": map[string]any{"queued": 0, "running": 0, "waiting": 0, "paused": 0, "failed": 0, "terminated": 0, "stopped": 1}, }, + "workflowPolicies": map[string]any{ + "enabled": true, + "bindings": map[string]any{ + "planner": map[string]any{"id": "matt-series", "name": "Matt Series Engineering Workflow", "display": "matt-series (Matt Series Engineering Workflow)"}, + }, + }, "tools": map[string]any{"git": true, "gh": false, "osascript": true}, "notifications": map[string]any{"inAppEnabled": true, "osascriptEnabled": false}, })) diff --git a/internal/cliapp/human_output.go b/internal/cliapp/human_output.go index 705a92d..29712cb 100644 --- a/internal/cliapp/human_output.go +++ b/internal/cliapp/human_output.go @@ -28,7 +28,8 @@ type statusOutput struct { QueuedItems int `json:"queuedItems"` RunningItems int `json:"runningItems"` } `json:"scheduler"` - Loops struct { + WorkflowPolicies *statusWorkflowPoliciesOutput `json:"workflowPolicies,omitempty"` + Loops struct { Planner statusLoopSummary `json:"planner"` Reviewer statusLoopSummary `json:"reviewer"` Worker statusLoopSummary `json:"worker"` @@ -45,6 +46,17 @@ type statusOutput struct { } `json:"tools"` } +type statusWorkflowPoliciesOutput struct { + Enabled bool `json:"enabled"` + Bindings map[string]*statusWorkflowPolicyBindingOutput `json:"bindings"` +} + +type statusWorkflowPolicyBindingOutput struct { + ID string `json:"id"` + Name string `json:"name"` + Display string `json:"display"` +} + type statusLoopSummary struct { Queued int `json:"queued"` Running int `json:"running"` @@ -205,6 +217,10 @@ func writeHumanStatus(w io.Writer, payload json.RawMessage) error { printSection(w, "Storage", [][2]any{{"dbPath", data.Storage.DBPath}, {"schemaVersion", data.Storage.SchemaVersion}, {"healthy", data.Storage.Healthy}, {"pendingMigrations", joinOrNone(data.Storage.PendingMigrations)}}) fmt.Fprintln(w) printSection(w, "Scheduler", [][2]any{{"healthy", data.Scheduler.Healthy}, {"queuedItems", data.Scheduler.QueuedItems}, {"runningItems", data.Scheduler.RunningItems}}) + if data.WorkflowPolicies != nil { + fmt.Fprintln(w) + printSection(w, "Workflow policies", [][2]any{{"enabled", data.WorkflowPolicies.Enabled}, {"planner", workflowPolicyDisplay(data.WorkflowPolicies.Bindings["planner"])}, {"reviewer", workflowPolicyDisplay(data.WorkflowPolicies.Bindings["reviewer"])}, {"worker", workflowPolicyDisplay(data.WorkflowPolicies.Bindings["worker"])}, {"fixer", workflowPolicyDisplay(data.WorkflowPolicies.Bindings["fixer"])}}) + } fmt.Fprintln(w) printTable(w, []string{"type", "queued", "running", "waiting", "paused", "failed", "terminated", "stopped"}, []tableRow{{"type": "planner", "queued": data.Loops.Planner.Queued, "running": data.Loops.Planner.Running, "waiting": data.Loops.Planner.Waiting, "paused": data.Loops.Planner.Paused, "failed": data.Loops.Planner.Failed, "terminated": data.Loops.Planner.Terminated, "stopped": data.Loops.Planner.Stopped}, {"type": "reviewer", "queued": data.Loops.Reviewer.Queued, "running": data.Loops.Reviewer.Running, "waiting": data.Loops.Reviewer.Waiting, "paused": data.Loops.Reviewer.Paused, "failed": data.Loops.Reviewer.Failed, "terminated": data.Loops.Reviewer.Terminated, "stopped": data.Loops.Reviewer.Stopped}, {"type": "worker", "queued": data.Loops.Worker.Queued, "running": data.Loops.Worker.Running, "waiting": data.Loops.Worker.Waiting, "paused": data.Loops.Worker.Paused, "failed": data.Loops.Worker.Failed, "terminated": data.Loops.Worker.Terminated, "stopped": data.Loops.Worker.Stopped}, {"type": "fixer", "queued": data.Loops.Fixer.Queued, "running": data.Loops.Fixer.Running, "waiting": data.Loops.Fixer.Waiting, "paused": data.Loops.Fixer.Paused, "failed": data.Loops.Fixer.Failed, "terminated": data.Loops.Fixer.Terminated, "stopped": data.Loops.Fixer.Stopped}}) fmt.Fprintln(w) @@ -214,6 +230,19 @@ func writeHumanStatus(w io.Writer, payload json.RawMessage) error { return nil } +func workflowPolicyDisplay(binding *statusWorkflowPolicyBindingOutput) string { + if binding == nil { + return "none" + } + if strings.TrimSpace(binding.Display) != "" { + return binding.Display + } + if strings.TrimSpace(binding.Name) == "" { + return binding.ID + } + return fmt.Sprintf("%s (%s)", binding.ID, binding.Name) +} + func writeHumanProjectList(w io.Writer, payload json.RawMessage) error { var data projectsListOutput if err := json.Unmarshal(payload, &data); err != nil { diff --git a/internal/cliapp/prompt_commands.go b/internal/cliapp/prompt_commands.go index 9016072..bfc5252 100644 --- a/internal/cliapp/prompt_commands.go +++ b/internal/cliapp/prompt_commands.go @@ -9,6 +9,7 @@ import ( "github.com/powerformer/looper/internal/agent" "github.com/powerformer/looper/internal/config" "github.com/powerformer/looper/internal/lifecycle" + "github.com/powerformer/looper/internal/workflowpolicy" "github.com/spf13/cobra" ) @@ -41,6 +42,14 @@ func (r *commandRuntime) promptPreview(cmd *cobra.Command, args []string) error sections = append(sections, "Repository context / AGENTS.md\n"+repoContext) order = append(order, "repository context / AGENTS.md") } + policyBlock, err := workflowpolicy.ResolveBlock(loaded.Config, projectID, role) + if err != nil { + return err + } + if policyBlock.Instructions != "" { + sections = append(sections, policyBlock.Instructions) + order = append(order, "workflow policy pack") + } if block.Text != "" { sections = append(sections, previewInstructionSources(block)+"\n\n"+block.Text) } else { @@ -53,7 +62,7 @@ func (r *commandRuntime) promptPreview(cmd *cobra.Command, args []string) error ) prompt := strings.Join(sections, "\n\n---\n\n") if getBoolFlag(cmd, "json") { - return writeJSON(cmd.OutOrStdout(), map[string]any{"project": projectID, "role": role, "order": order, "customInstructions": block, "prompt": prompt}) + return writeJSON(cmd.OutOrStdout(), map[string]any{"project": projectID, "role": role, "order": order, "workflowPolicy": policyBlock, "customInstructions": block, "prompt": prompt}) } _, err = fmt.Fprintln(cmd.OutOrStdout(), prompt) return err diff --git a/internal/cliapp/prompt_commands_test.go b/internal/cliapp/prompt_commands_test.go index cfa14b5..c60d395 100644 --- a/internal/cliapp/prompt_commands_test.go +++ b/internal/cliapp/prompt_commands_test.go @@ -119,6 +119,48 @@ func TestPromptPreviewWorkerHonorsManualOpenPRStrategy(t *testing.T) { } } +func TestPromptPreviewInjectsWorkflowPolicyPackThroughPublicWorkerPath(t *testing.T) { + t.Parallel() + + payload := promptPreviewConfigPayload(t.TempDir(), true) + payload["workflowPolicyPacks"] = map[string]any{ + "enabled": true, + "packs": []map[string]any{{ + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "source": "builtin", + }}, + } + payload["roles"] = map[string]any{ + "worker": map[string]any{ + "policyPack": "matt-series", + "instructions": "Custom worker instructions after policy.", + }, + } + configPath := writeEditableCLIConfigWithPayload(t, payload) + + exitCode, stdout, stderr := runApp(t, "prompt", "preview", "--project", "project_1", "--role", "worker", "--config", configPath) + if exitCode != 0 { + t.Fatalf("Run(prompt preview worker policy pack) exit code = %d, want 0; stderr=%q", exitCode, stderr) + } + for _, want := range []string{ + "Workflow policy pack: matt-series (Matt Series Engineering Workflow)", + "Work in vertical slices", + "Custom worker instructions after policy.", + "__LOOPER_RESULT__=", + } { + if !strings.Contains(stdout, want) { + t.Fatalf("prompt preview = %q, want to contain %q", stdout, want) + } + } + policyIndex := strings.Index(stdout, "Work in vertical slices") + customIndex := strings.Index(stdout, "Custom worker instructions after policy.") + completionIndex := strings.LastIndex(stdout, "__LOOPER_RESULT__=") + if policyIndex < 0 || customIndex < 0 || completionIndex < 0 || !(policyIndex < customIndex && customIndex < completionIndex) { + t.Fatalf("unexpected prompt order policy=%d custom=%d completion=%d\n%s", policyIndex, customIndex, completionIndex, stdout) + } +} + func TestPromptPreviewReviewerUsesReviewSubmitContract(t *testing.T) { t.Parallel() diff --git a/internal/cliapp/testdata/golden/status-human.stdout.golden b/internal/cliapp/testdata/golden/status-human.stdout.golden index a51e18a..43af2f7 100644 --- a/internal/cliapp/testdata/golden/status-human.stdout.golden +++ b/internal/cliapp/testdata/golden/status-human.stdout.golden @@ -15,6 +15,13 @@ Scheduler queuedItems : 2 runningItems : 1 +Workflow policies + enabled : yes + planner : matt-series (Matt Series Engineering Workflow) + reviewer : none + worker : none + fixer : none + type queued running waiting paused failed terminated stopped -------- ------ ------- ------- ------ ------ ---------- ------- planner 0 0 0 1 0 0 0 diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1ebbb01..8b18390 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -80,6 +80,79 @@ func TestRoleDefaultsMirrorCurrentDiscoveryPolicy(t *testing.T) { } } +func TestWorkflowPolicyPackDefaultsAreAvailableButNotBound(t *testing.T) { + t.Parallel() + + cfg, err := Normalize(t.TempDir()) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + if !cfg.WorkflowPolicyPacks.Enabled { + t.Fatal("workflowPolicyPacks.enabled = false, want true") + } + if len(cfg.WorkflowPolicyPacks.Packs) != 1 || cfg.WorkflowPolicyPacks.Packs[0].ID != "matt-series" || cfg.WorkflowPolicyPacks.Packs[0].Name == "" { + t.Fatalf("workflowPolicyPacks.packs = %#v", cfg.WorkflowPolicyPacks.Packs) + } + if cfg.Roles.Planner.PolicyPack != "" || cfg.Roles.Worker.PolicyPack != "" || cfg.Roles.Reviewer.PolicyPack != "" || cfg.Roles.Fixer.PolicyPack != "" { + t.Fatalf("role policy packs should require explicit bindings: %#v", cfg.Roles) + } +} + +func TestWorkflowPolicyPackConfigParsesGlobalAndProjectBindings(t *testing.T) { + t.Parallel() + + filePackPath := filepath.Join(t.TempDir(), "team-pack.json") + packs := []WorkflowPolicyPackRef{ + {ID: "matt-series", Name: "Matt Series Engineering Workflow", Source: WorkflowPolicyPackSourceBuiltin}, + {ID: "team-pack", Name: "Team Pack", Source: WorkflowPolicyPackSourceFile, Path: filePackPath}, + } + projectID := "demo" + cfg, err := Normalize(t.TempDir(), PartialConfig{ + WorkflowPolicyPacks: &PartialWorkflowPolicyPacksConfig{Packs: &packs}, + Roles: &PartialRoleConfigs{Worker: &PartialWorkerRoleConfig{PolicyPack: stringPtr("matt-series")}}, + Projects: &[]ProjectRefConfig{{ + ID: projectID, + Name: "Demo", + RepoPath: t.TempDir(), + Roles: &PartialRoleConfigs{Worker: &PartialWorkerRoleConfig{PolicyPack: stringPtr("team-pack")}}, + }}, + }) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + if got := cfg.Roles.Worker.PolicyPack; got != "matt-series" { + t.Fatalf("global worker policyPack = %q", got) + } + if got := ProjectRoleConfigs(cfg, projectID).Worker.PolicyPack; got != "team-pack" { + t.Fatalf("project worker policyPack = %q", got) + } +} + +func TestValidateRejectsInvalidWorkflowPolicyPackConfig(t *testing.T) { + t.Parallel() + + cfg, err := Normalize(t.TempDir(), PartialConfig{ + WorkflowPolicyPacks: &PartialWorkflowPolicyPacksConfig{Packs: &[]WorkflowPolicyPackRef{ + {ID: "dup", Name: "Dup", Source: WorkflowPolicyPackSourceBuiltin}, + {ID: "dup", Name: "Dup Again", Source: WorkflowPolicyPackSourceBuiltin}, + {ID: "file-pack", Name: "File Pack", Source: WorkflowPolicyPackSourceFile}, + }}, + Roles: &PartialRoleConfigs{Worker: &PartialWorkerRoleConfig{PolicyPack: stringPtr("unknown-pack")}}, + }) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + + err = ValidateWithOptions(cfg, ValidateOptions{DefaultWorktreeRoot: t.TempDir()}) + var validationErr *ConfigValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("ValidateWithOptions() error = %v, want ConfigValidationError", err) + } + assertValidationIssue(t, validationErr, "workflowPolicyPacks.packs[1].id", "duplicate workflow policy pack id: dup") + assertValidationIssue(t, validationErr, "workflowPolicyPacks.packs[2].path", "is required for file workflow policy packs") + assertValidationIssue(t, validationErr, "roles.worker.policyPack", "references unknown workflow policy pack: unknown-pack") +} + func TestAgentTimeoutConfigOverrides(t *testing.T) { cwd := t.TempDir() configPath := filepath.Join(cwd, "config.json") diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 27cd2e2..b0ca204 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -163,6 +163,14 @@ func DefaultConfig(cwd string) (Config, error) { }, }, Instructions: InstructionsConfig{Enabled: true, MaxBytes: 8192}, + WorkflowPolicyPacks: WorkflowPolicyPacksConfig{ + Enabled: true, + Packs: []WorkflowPolicyPackRef{{ + ID: "matt-series", + Name: "Matt Series Engineering Workflow", + Source: WorkflowPolicyPackSourceBuiltin, + }}, + }, Roles: RoleConfigs{ Planner: PlannerRoleConfig{ AutoDiscovery: true, diff --git a/internal/config/normalize.go b/internal/config/normalize.go index 2f3b611..b704b87 100644 --- a/internal/config/normalize.go +++ b/internal/config/normalize.go @@ -86,6 +86,10 @@ func mergeConfig(config *Config, partial PartialConfig) { mergeInstructionsConfig(&config.Instructions, *partial.Instructions) } + if partial.WorkflowPolicyPacks != nil { + mergeWorkflowPolicyPacksConfig(&config.WorkflowPolicyPacks, *partial.WorkflowPolicyPacks) + } + if partial.Roles != nil { mergeRoleConfigs(&config.Roles, *partial.Roles) } @@ -505,6 +509,15 @@ func mergeInstructionsConfig(config *InstructionsConfig, partial PartialInstruct } } +func mergeWorkflowPolicyPacksConfig(config *WorkflowPolicyPacksConfig, partial PartialWorkflowPolicyPacksConfig) { + if partial.Enabled != nil { + config.Enabled = *partial.Enabled + } + if partial.Packs != nil { + config.Packs = append([]WorkflowPolicyPackRef{}, (*partial.Packs)...) + } +} + func mergeRoleConfigs(config *RoleConfigs, partial PartialRoleConfigs) { if partial.Planner != nil { mergePlannerRoleConfig(&config.Planner, *partial.Planner) @@ -530,6 +543,9 @@ func mergePlannerRoleConfig(config *PlannerRoleConfig, partial PartialPlannerRol if partial.Instructions != nil { config.Instructions = *partial.Instructions } + if partial.PolicyPack != nil { + config.PolicyPack = *partial.PolicyPack + } } func mergeWorkerRoleConfig(config *WorkerRoleConfig, partial PartialWorkerRoleConfig) { @@ -542,6 +558,9 @@ func mergeWorkerRoleConfig(config *WorkerRoleConfig, partial PartialWorkerRoleCo if partial.Instructions != nil { config.Instructions = *partial.Instructions } + if partial.PolicyPack != nil { + config.PolicyPack = *partial.PolicyPack + } } func mergeReviewerRoleConfig(config *ReviewerRoleConfig, partial PartialReviewerRoleConfig) { @@ -557,6 +576,9 @@ func mergeReviewerRoleConfig(config *ReviewerRoleConfig, partial PartialReviewer if partial.Instructions != nil { config.Instructions = *partial.Instructions } + if partial.PolicyPack != nil { + config.PolicyPack = *partial.PolicyPack + } } func mergeFixerRoleConfig(config *FixerRoleConfig, partial PartialFixerRoleConfig) { @@ -569,6 +591,9 @@ func mergeFixerRoleConfig(config *FixerRoleConfig, partial PartialFixerRoleConfi if partial.Instructions != nil { config.Instructions = *partial.Instructions } + if partial.PolicyPack != nil { + config.PolicyPack = *partial.PolicyPack + } } func mergeIssueRoleTriggersConfig(config *IssueRoleTriggersConfig, partial PartialIssueRoleTriggersConfig) { diff --git a/internal/config/testdata/parity/cli-file-env-priority.json b/internal/config/testdata/parity/cli-file-env-priority.json index d03d445..32b971c 100644 --- a/internal/config/testdata/parity/cli-file-env-priority.json +++ b/internal/config/testdata/parity/cli-file-env-priority.json @@ -155,6 +155,16 @@ "looperPath": "/detected/looper", "osascriptPath": "/cli/osascript" }, + "workflowPolicyPacks": { + "enabled": true, + "packs": [ + { + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "source": "builtin" + } + ] + }, "daemon": { "mode": "foreground", "restartPolicy": "on-failure", diff --git a/internal/config/testdata/parity/default-path-missing.json b/internal/config/testdata/parity/default-path-missing.json index 134f938..3634619 100644 --- a/internal/config/testdata/parity/default-path-missing.json +++ b/internal/config/testdata/parity/default-path-missing.json @@ -90,6 +90,16 @@ "looperPath": "/detected/looper", "osascriptPath": "/envonly/osascript" }, + "workflowPolicyPacks": { + "enabled": true, + "packs": [ + { + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "source": "builtin" + } + ] + }, "daemon": { "mode": "foreground", "restartPolicy": "on-failure", diff --git a/internal/config/testdata/parity/env-config-path.json b/internal/config/testdata/parity/env-config-path.json index 928050b..a97ed7b 100644 --- a/internal/config/testdata/parity/env-config-path.json +++ b/internal/config/testdata/parity/env-config-path.json @@ -173,6 +173,16 @@ "looperPath": "/detected/looper", "osascriptPath": "/cfg/osascript" }, + "workflowPolicyPacks": { + "enabled": true, + "packs": [ + { + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "source": "builtin" + } + ] + }, "daemon": { "mode": "launchd", "restartPolicy": "on-failure", diff --git a/internal/config/types.go b/internal/config/types.go index f386451..e3062c2 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -195,6 +195,25 @@ type InstructionsConfig struct { MaxBytes int `json:"maxBytes"` } +type WorkflowPolicyPackSource string + +const ( + WorkflowPolicyPackSourceBuiltin WorkflowPolicyPackSource = "builtin" + WorkflowPolicyPackSourceFile WorkflowPolicyPackSource = "file" +) + +type WorkflowPolicyPackRef struct { + ID string `json:"id"` + Name string `json:"name"` + Source WorkflowPolicyPackSource `json:"source"` + Path string `json:"path,omitempty"` +} + +type WorkflowPolicyPacksConfig struct { + Enabled bool `json:"enabled"` + Packs []WorkflowPolicyPackRef `json:"packs"` +} + type RoleConfig struct { Instructions string `json:"instructions,omitempty"` } @@ -328,12 +347,14 @@ type PlannerRoleConfig struct { AutoDiscovery bool `json:"autoDiscovery"` Triggers IssueRoleTriggersConfig `json:"triggers"` Instructions string `json:"instructions,omitempty"` + PolicyPack string `json:"policyPack,omitempty"` } type WorkerRoleConfig struct { AutoDiscovery bool `json:"autoDiscovery"` Triggers IssueRoleTriggersConfig `json:"triggers"` Instructions string `json:"instructions,omitempty"` + PolicyPack string `json:"policyPack,omitempty"` } type ReviewerRoleConfig struct { @@ -341,12 +362,14 @@ type ReviewerRoleConfig struct { Triggers ReviewerRoleTriggersConfig `json:"triggers"` SpecReview ReviewerSpecReviewConfig `json:"specReview"` Instructions string `json:"instructions,omitempty"` + PolicyPack string `json:"policyPack,omitempty"` } type FixerRoleConfig struct { AutoDiscovery bool `json:"autoDiscovery"` Triggers FixerRoleTriggersConfig `json:"triggers"` Instructions string `json:"instructions,omitempty"` + PolicyPack string `json:"policyPack,omitempty"` } type RoleConfigs struct { @@ -368,21 +391,22 @@ type ProjectRefConfig struct { } type Config struct { - Server ServerConfig `json:"server"` - Storage StorageConfig `json:"storage"` - Scheduler SchedulerConfig `json:"scheduler"` - Agent AgentConfig `json:"agent"` - Logging LoggingConfig `json:"logging"` - Notifications NotificationConfig `json:"notifications"` - Disclosure DisclosureConfig `json:"disclosure"` - Tools ToolPathsConfig `json:"tools"` - Daemon DaemonConfig `json:"daemon"` - Package PackageConfig `json:"package"` - Defaults DefaultsConfig `json:"defaults"` - Reviewer ReviewerConfig `json:"reviewer"` - Instructions InstructionsConfig `json:"instructions"` - Roles RoleConfigs `json:"roles"` - Projects []ProjectRefConfig `json:"projects"` + Server ServerConfig `json:"server"` + Storage StorageConfig `json:"storage"` + Scheduler SchedulerConfig `json:"scheduler"` + Agent AgentConfig `json:"agent"` + Logging LoggingConfig `json:"logging"` + Notifications NotificationConfig `json:"notifications"` + Disclosure DisclosureConfig `json:"disclosure"` + Tools ToolPathsConfig `json:"tools"` + Daemon DaemonConfig `json:"daemon"` + Package PackageConfig `json:"package"` + Defaults DefaultsConfig `json:"defaults"` + Reviewer ReviewerConfig `json:"reviewer"` + Instructions InstructionsConfig `json:"instructions"` + WorkflowPolicyPacks WorkflowPolicyPacksConfig `json:"workflowPolicyPacks"` + Roles RoleConfigs `json:"roles"` + Projects []ProjectRefConfig `json:"projects"` } type PartialServerConfig struct { @@ -548,6 +572,11 @@ type PartialInstructionsConfig struct { MaxBytes *int `json:"maxBytes,omitempty"` } +type PartialWorkflowPolicyPacksConfig struct { + Enabled *bool `json:"enabled,omitempty"` + Packs *[]WorkflowPolicyPackRef `json:"packs,omitempty"` +} + type PartialIssueRoleTriggersConfig struct { Labels *[]string `json:"labels,omitempty"` LabelMode *LabelMode `json:"labelMode,omitempty"` @@ -582,12 +611,14 @@ type PartialPlannerRoleConfig struct { AutoDiscovery *bool `json:"autoDiscovery,omitempty"` Triggers *PartialIssueRoleTriggersConfig `json:"triggers,omitempty"` Instructions *string `json:"instructions,omitempty"` + PolicyPack *string `json:"policyPack,omitempty"` } type PartialWorkerRoleConfig struct { AutoDiscovery *bool `json:"autoDiscovery,omitempty"` Triggers *PartialIssueRoleTriggersConfig `json:"triggers,omitempty"` Instructions *string `json:"instructions,omitempty"` + PolicyPack *string `json:"policyPack,omitempty"` } type PartialReviewerRoleConfig struct { @@ -595,12 +626,14 @@ type PartialReviewerRoleConfig struct { Triggers *PartialReviewerRoleTriggersConfig `json:"triggers,omitempty"` SpecReview *PartialReviewerSpecReviewConfig `json:"specReview,omitempty"` Instructions *string `json:"instructions,omitempty"` + PolicyPack *string `json:"policyPack,omitempty"` } type PartialFixerRoleConfig struct { AutoDiscovery *bool `json:"autoDiscovery,omitempty"` Triggers *PartialFixerRoleTriggersConfig `json:"triggers,omitempty"` Instructions *string `json:"instructions,omitempty"` + PolicyPack *string `json:"policyPack,omitempty"` } type PartialRoleConfigs struct { @@ -611,19 +644,20 @@ type PartialRoleConfigs struct { } type PartialConfig struct { - Server *PartialServerConfig `json:"server,omitempty"` - Storage *PartialStorageConfig `json:"storage,omitempty"` - Scheduler *PartialSchedulerConfig `json:"scheduler,omitempty"` - Agent *PartialAgentConfig `json:"agent,omitempty"` - Logging *PartialLoggingConfig `json:"logging,omitempty"` - Notifications *PartialNotificationConfig `json:"notifications,omitempty"` - Disclosure *PartialDisclosureConfig `json:"disclosure,omitempty"` - Tools *PartialToolPathsConfig `json:"tools,omitempty"` - Daemon *PartialDaemonConfig `json:"daemon,omitempty"` - Package *PartialPackageConfig `json:"package,omitempty"` - Defaults *PartialDefaultsConfig `json:"defaults,omitempty"` - Reviewer *PartialReviewerConfig `json:"reviewer,omitempty"` - Instructions *PartialInstructionsConfig `json:"instructions,omitempty"` - Roles *PartialRoleConfigs `json:"roles,omitempty"` - Projects *[]ProjectRefConfig `json:"projects,omitempty"` + Server *PartialServerConfig `json:"server,omitempty"` + Storage *PartialStorageConfig `json:"storage,omitempty"` + Scheduler *PartialSchedulerConfig `json:"scheduler,omitempty"` + Agent *PartialAgentConfig `json:"agent,omitempty"` + Logging *PartialLoggingConfig `json:"logging,omitempty"` + Notifications *PartialNotificationConfig `json:"notifications,omitempty"` + Disclosure *PartialDisclosureConfig `json:"disclosure,omitempty"` + Tools *PartialToolPathsConfig `json:"tools,omitempty"` + Daemon *PartialDaemonConfig `json:"daemon,omitempty"` + Package *PartialPackageConfig `json:"package,omitempty"` + Defaults *PartialDefaultsConfig `json:"defaults,omitempty"` + Reviewer *PartialReviewerConfig `json:"reviewer,omitempty"` + Instructions *PartialInstructionsConfig `json:"instructions,omitempty"` + WorkflowPolicyPacks *PartialWorkflowPolicyPacksConfig `json:"workflowPolicyPacks,omitempty"` + Roles *PartialRoleConfigs `json:"roles,omitempty"` + Projects *[]ProjectRefConfig `json:"projects,omitempty"` } diff --git a/internal/config/validate.go b/internal/config/validate.go index b3bb27b..1fb8632 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -181,6 +181,8 @@ func ValidateWithOptions(config Config, options ValidateOptions) error { } validateInstructions(config, &issues) + validateWorkflowPolicyPackRefs(config.WorkflowPolicyPacks, &issues) + validateRolePolicyPackBindings(config, &issues) validateIssueRoleTriggers(config.Roles.Planner.Triggers, "roles.planner.triggers", &issues) validateIssueRoleTriggers(config.Roles.Worker.Triggers, "roles.worker.triggers", &issues) validateReviewerRoleTriggers(config.Roles.Reviewer.Triggers, "roles.reviewer.triggers", &issues) @@ -220,6 +222,7 @@ func ValidateWithOptions(config Config, options ValidateOptions) error { validateProjectRoleOverrides(project.Roles, prefix+".roles", config.Instructions.MaxBytes, &issues) effectiveProjectRoles := ProjectRoleConfigs(config, project.ID) + validateProjectPolicyPackBindings(config, project.Roles, prefix+".roles", &issues) for role, text := range project.Instructions { path := fmt.Sprintf("%s.instructions.%s", prefix, role) validateInstructionText(path, role, text, config.Instructions.MaxBytes, &issues) @@ -327,6 +330,114 @@ func validateProjectRoleOverrides(roles *PartialRoleConfigs, prefix string, maxI } } +func validateWorkflowPolicyPackRefs(policyConfig WorkflowPolicyPacksConfig, issues *[]ValidationIssue) { + ids := map[string]struct{}{} + for index, pack := range policyConfig.Packs { + prefix := fmt.Sprintf("workflowPolicyPacks.packs[%d]", index) + if strings.TrimSpace(pack.ID) == "" { + *issues = append(*issues, ValidationIssue{Path: prefix + ".id", Message: "must be a non-empty string"}) + } else if pack.ID != strings.TrimSpace(pack.ID) || !isSlugID(pack.ID) { + *issues = append(*issues, ValidationIssue{Path: prefix + ".id", Message: "must use lowercase letters, numbers, dots, underscores, or hyphens"}) + } else if _, exists := ids[pack.ID]; exists { + *issues = append(*issues, ValidationIssue{Path: prefix + ".id", Message: fmt.Sprintf("duplicate workflow policy pack id: %s", pack.ID)}) + } else { + ids[pack.ID] = struct{}{} + } + if strings.TrimSpace(pack.Name) == "" { + *issues = append(*issues, ValidationIssue{Path: prefix + ".name", Message: "must be a non-empty string"}) + } + switch pack.Source { + case WorkflowPolicyPackSourceBuiltin: + if strings.TrimSpace(pack.Path) != "" { + *issues = append(*issues, ValidationIssue{Path: prefix + ".path", Message: "must be empty for builtin workflow policy packs"}) + } + case WorkflowPolicyPackSourceFile: + if strings.TrimSpace(pack.Path) == "" { + *issues = append(*issues, ValidationIssue{Path: prefix + ".path", Message: "is required for file workflow policy packs"}) + } + default: + *issues = append(*issues, ValidationIssue{Path: prefix + ".source", Message: fmt.Sprintf("must be one of: %s, %s", WorkflowPolicyPackSourceBuiltin, WorkflowPolicyPackSourceFile)}) + } + } +} + +func validateRolePolicyPackBindings(config Config, issues *[]ValidationIssue) { + ids := workflowPolicyPackIDs(config.WorkflowPolicyPacks) + for _, binding := range rolePolicyPackBindings(config.Roles) { + validatePolicyPackBinding(binding.path, binding.value, ids, issues) + } +} + +func validateProjectPolicyPackBindings(config Config, roles *PartialRoleConfigs, prefix string, issues *[]ValidationIssue) { + if roles == nil { + return + } + ids := workflowPolicyPackIDs(config.WorkflowPolicyPacks) + if roles.Planner != nil && roles.Planner.PolicyPack != nil { + validatePolicyPackBinding(prefix+".planner.policyPack", *roles.Planner.PolicyPack, ids, issues) + } + if roles.Worker != nil && roles.Worker.PolicyPack != nil { + validatePolicyPackBinding(prefix+".worker.policyPack", *roles.Worker.PolicyPack, ids, issues) + } + if roles.Reviewer != nil && roles.Reviewer.PolicyPack != nil { + validatePolicyPackBinding(prefix+".reviewer.policyPack", *roles.Reviewer.PolicyPack, ids, issues) + } + if roles.Fixer != nil && roles.Fixer.PolicyPack != nil { + validatePolicyPackBinding(prefix+".fixer.policyPack", *roles.Fixer.PolicyPack, ids, issues) + } +} + +type rolePolicyPackBinding struct { + path string + value string +} + +func rolePolicyPackBindings(roles RoleConfigs) []rolePolicyPackBinding { + return []rolePolicyPackBinding{ + {path: "roles.planner.policyPack", value: roles.Planner.PolicyPack}, + {path: "roles.worker.policyPack", value: roles.Worker.PolicyPack}, + {path: "roles.reviewer.policyPack", value: roles.Reviewer.PolicyPack}, + {path: "roles.fixer.policyPack", value: roles.Fixer.PolicyPack}, + } +} + +func workflowPolicyPackIDs(config WorkflowPolicyPacksConfig) map[string]struct{} { + ids := map[string]struct{}{} + for _, pack := range config.Packs { + if strings.TrimSpace(pack.ID) != "" { + ids[pack.ID] = struct{}{} + } + } + return ids +} + +func validatePolicyPackBinding(path string, value string, ids map[string]struct{}, issues *[]ValidationIssue) { + value = strings.TrimSpace(value) + if value == "" { + return + } + if !isSlugID(value) { + *issues = append(*issues, ValidationIssue{Path: path, Message: "must use lowercase letters, numbers, dots, underscores, or hyphens"}) + return + } + if _, ok := ids[value]; !ok { + *issues = append(*issues, ValidationIssue{Path: path, Message: fmt.Sprintf("references unknown workflow policy pack: %s", value)}) + } +} + +func isSlugID(value string) bool { + if value == "" { + return false + } + for _, r := range value { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' || r == '.' { + continue + } + return false + } + return true +} + func validateProjectRoleInstruction(path, role string, text *string, maxBytes int, issues *[]ValidationIssue) { if text == nil { return diff --git a/internal/fixer/runner.go b/internal/fixer/runner.go index e08da11..82f3762 100644 --- a/internal/fixer/runner.go +++ b/internal/fixer/runner.go @@ -22,6 +22,7 @@ import ( "github.com/powerformer/looper/internal/infra/specpr" "github.com/powerformer/looper/internal/lifecycle" "github.com/powerformer/looper/internal/storage" + "github.com/powerformer/looper/internal/workflowpolicy" ) const ( @@ -2115,6 +2116,9 @@ func buildFixerPrompt(projectID string, instructionConfig config.Config, repo st "Fix items:\n"+strings.Join(encodedItems, "\n"), "Only perform repair changes for the listed fix items.", ) + if policyBlock, err := workflowpolicy.ResolveBlock(instructionConfig, projectID, "fixer"); err == nil && policyBlock.Instructions != "" { + parts = append(parts, policyBlock.Instructions) + } instructionBlock := config.BuildCustomInstructionBlock(instructionConfig, projectID, "fixer") if instructionBlock.Text != "" { parts = append(parts, instructionBlock.Text) diff --git a/internal/fixer/runner_test.go b/internal/fixer/runner_test.go index 8acb316..2631650 100644 --- a/internal/fixer/runner_test.go +++ b/internal/fixer/runner_test.go @@ -44,6 +44,30 @@ func TestBuildFixerPromptOmitsMissingAgentRuntime(t *testing.T) { } } +func TestBuildFixerPromptPlacesWorkflowPolicyBeforeCustomInstructions(t *testing.T) { + t.Parallel() + + packID := "matt-series" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Fixer: &config.PartialFixerRoleConfig{PolicyPack: &packID, Instructions: stringPtr("Custom fixer instructions.")}}}) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + detail := &checkpointDetail{State: "OPEN", HeadSHA: "abc123", BaseRefName: "main", HeadRefName: "feature"} + prompt, _ := buildFixerPrompt("project_1", cfg, "acme/looper", 42, detail, []FixItem{{ID: "fix-1", Summary: "repair disclosure"}}, true, config.DefaultDisclosureConfig(), "opencode", "openai/gpt-5.5") + + assertPromptOrder(t, prompt, "Verify or reproduce each listed fix item", "Custom fixer instructions.", "Agent-managed git/PR lifecycle policy") +} + +func assertPromptOrder(t *testing.T, prompt string, first string, second string, third string) { + t.Helper() + firstIndex := strings.Index(prompt, first) + secondIndex := strings.Index(prompt, second) + thirdIndex := strings.Index(prompt, third) + if firstIndex < 0 || secondIndex < 0 || thirdIndex < 0 || !(firstIndex < secondIndex && secondIndex < thirdIndex) { + t.Fatalf("unexpected prompt order %q=%d %q=%d %q=%d\n%s", first, firstIndex, second, secondIndex, third, thirdIndex, prompt) + } +} + func TestBuildFixerPromptIncludesMinimalPRSeedFetchContract(t *testing.T) { t.Parallel() diff --git a/internal/planner/runner.go b/internal/planner/runner.go index a92ac2a..805ffad 100644 --- a/internal/planner/runner.go +++ b/internal/planner/runner.go @@ -20,6 +20,7 @@ import ( "github.com/powerformer/looper/internal/infra/specpr" "github.com/powerformer/looper/internal/lifecycle" "github.com/powerformer/looper/internal/storage" + "github.com/powerformer/looper/internal/workflowpolicy" ) const ( @@ -1566,6 +1567,9 @@ func buildPlannerPrompt(project storage.ProjectRecord, instructionConfig config. if agentsBlock := readAgentsBlock(project.RepoPath); agentsBlock != "" { parts = append(parts, agentsBlock) } + if policyBlock, err := workflowpolicy.ResolveBlock(instructionConfig, project.ID, "planner"); err == nil && policyBlock.Instructions != "" { + parts = append(parts, policyBlock.Instructions) + } instructionBlock := config.BuildCustomInstructionBlock(instructionConfig, project.ID, "planner") if instructionBlock.Text != "" { parts = append(parts, instructionBlock.Text) diff --git a/internal/planner/runner_test.go b/internal/planner/runner_test.go index cbdfee2..035d1b3 100644 --- a/internal/planner/runner_test.go +++ b/internal/planner/runner_test.go @@ -44,6 +44,30 @@ func TestBuildPlannerPromptOmitsMissingAgentRuntime(t *testing.T) { } } +func TestBuildPlannerPromptPlacesWorkflowPolicyBeforeCustomInstructions(t *testing.T) { + t.Parallel() + + packID := "matt-series" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Planner: &config.PartialPlannerRoleConfig{PolicyPack: &packID, Instructions: stringPtr("Custom planner instructions.")}}}) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + repoPath := t.TempDir() + prompt, _ := buildPlannerPrompt(storage.ProjectRecord{ID: "project_1", RepoPath: repoPath}, cfg, &checkpointIssue{Repo: "acme/looper", IssueNumber: 156, Title: "plan policy", SpecPath: "docs/spec.md"}, &checkpointWorktree{Branch: "looper/fix", BaseBranch: "main"}, true, config.DefaultDisclosureConfig(), "opencode", "openai/gpt-5.5") + + assertPromptOrder(t, prompt, "Clarify the requested outcome", "Custom planner instructions.", "Agent-managed git/PR lifecycle policy") +} + +func assertPromptOrder(t *testing.T, prompt string, first string, second string, third string) { + t.Helper() + firstIndex := strings.Index(prompt, first) + secondIndex := strings.Index(prompt, second) + thirdIndex := strings.Index(prompt, third) + if firstIndex < 0 || secondIndex < 0 || thirdIndex < 0 || !(firstIndex < secondIndex && secondIndex < thirdIndex) { + t.Fatalf("unexpected prompt order %q=%d %q=%d %q=%d\n%s", first, firstIndex, second, secondIndex, third, thirdIndex, prompt) + } +} + func TestDiscoverIssuesEnqueuesEligibleWorkAndCreatesLoop(t *testing.T) { t.Parallel() fixture := newRunnerFixture(t) diff --git a/internal/reviewer/runner.go b/internal/reviewer/runner.go index b3e8061..e8a2c7c 100644 --- a/internal/reviewer/runner.go +++ b/internal/reviewer/runner.go @@ -25,6 +25,7 @@ import ( "github.com/powerformer/looper/internal/infra/specpr" "github.com/powerformer/looper/internal/storage" "github.com/powerformer/looper/internal/version" + "github.com/powerformer/looper/internal/workflowpolicy" ) const ( @@ -4071,6 +4072,9 @@ func buildReviewPromptWithInstructions(projectID string, instructionConfig confi parts = append(parts, fmt.Sprintf("Unresolved threads: %d", *checkpoint.Snapshot.UnresolvedThreadCount)) } } + if policyBlock, err := workflowpolicy.ResolveBlock(instructionConfig, projectID, "reviewer"); err == nil && policyBlock.Instructions != "" { + parts = append(parts, policyBlock.Instructions) + } instructionBlock := config.BuildCustomInstructionBlock(instructionConfig, projectID, "reviewer") if instructionBlock.Text != "" { parts = append(parts, instructionBlock.Text) diff --git a/internal/reviewer/runner_test.go b/internal/reviewer/runner_test.go index bd8b6f5..34c0659 100644 --- a/internal/reviewer/runner_test.go +++ b/internal/reviewer/runner_test.go @@ -5231,6 +5231,24 @@ func TestBuildReviewPromptOmitsReviewRequestGuardrailWhenDisabled(t *testing.T) } } +func TestBuildReviewPromptPlacesWorkflowPolicyBeforeCustomInstructions(t *testing.T) { + t.Parallel() + + packID := "matt-series" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Reviewer: &config.PartialReviewerRoleConfig{PolicyPack: &packID, Instructions: stringPtr("Custom reviewer instructions.")}}}) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + prompt, _ := buildReviewPromptWithInstructions("project_1", cfg, "acme/looper", 42, reviewerCheckpoint{Snapshot: &checkpointSnapshot{HeadSHA: "abc123"}}, "run_1", "reviewer:loop:abc123", config.ReviewerReviewEventsConfig{Clean: config.ReviewerReviewEventComment, Blocking: config.ReviewerReviewEventComment}, false, true, config.ReviewerScopeChangedRanges, config.DefaultDisclosureConfig(), "opencode", "", "/opt/looper/bin/looper") + + firstIndex := strings.Index(prompt, "Build a quick module map") + secondIndex := strings.Index(prompt, "Custom reviewer instructions.") + thirdIndex := strings.Index(prompt, "Idempotency requirement") + if firstIndex < 0 || secondIndex < 0 || thirdIndex < 0 || !(firstIndex < secondIndex && secondIndex < thirdIndex) { + t.Fatalf("unexpected prompt order policy=%d custom=%d submit=%d\n%s", firstIndex, secondIndex, thirdIndex, prompt) + } +} + func TestRunThreadResolutionStepCommentsAndResolvesObjectiveLooperThread(t *testing.T) { t.Parallel() policy := defaultThreadResolutionPolicy(t) diff --git a/internal/worker/runner.go b/internal/worker/runner.go index a6949c8..7305901 100644 --- a/internal/worker/runner.go +++ b/internal/worker/runner.go @@ -24,6 +24,7 @@ import ( "github.com/powerformer/looper/internal/infra/specpr" "github.com/powerformer/looper/internal/lifecycle" "github.com/powerformer/looper/internal/storage" + "github.com/powerformer/looper/internal/workflowpolicy" ) const ( @@ -2423,6 +2424,9 @@ func buildWorkerPromptWithInstructions(repoRootPath string, projectID string, in } parts = append(parts, strings.Join(lines, "\n")) } + if policyBlock, err := workflowpolicy.ResolveBlock(instructionConfig, projectID, "worker"); err == nil && policyBlock.Instructions != "" { + parts = append(parts, policyBlock.Instructions) + } instructionBlock := config.BuildCustomInstructionBlock(instructionConfig, projectID, "worker") if instructionBlock.Text != "" { parts = append(parts, instructionBlock.Text) diff --git a/internal/worker/runner_test.go b/internal/worker/runner_test.go index 734f791..b11f394 100644 --- a/internal/worker/runner_test.go +++ b/internal/worker/runner_test.go @@ -322,9 +322,10 @@ func TestBuildWorkerPromptOmitsMissingAgentRuntime(t *testing.T) { } } -func TestBuildWorkerPromptPlacesCustomInstructionsBeforeLifecycle(t *testing.T) { +func TestBuildWorkerPromptPlacesWorkflowPolicyAndCustomInstructionsBeforeLifecycle(t *testing.T) { t.Parallel() - cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Worker: &config.PartialWorkerRoleConfig{Instructions: stringPtr("Prefer small commits.")}}}) + packID := "matt-series" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Worker: &config.PartialWorkerRoleConfig{PolicyPack: &packID, Instructions: stringPtr("Prefer small commits.")}}}) if err != nil { t.Fatalf("Normalize() error = %v", err) } @@ -335,11 +336,12 @@ func TestBuildWorkerPromptPlacesCustomInstructionsBeforeLifecycle(t *testing.T) if len(block.Sources) != 1 { t.Fatalf("instruction sources = %#v", block.Sources) } + policyIndex := strings.Index(prompt, "Work in vertical slices") customIndex := strings.Index(prompt, "Prefer small commits.") lifecycleIndex := strings.Index(prompt, "Agent-managed git/PR lifecycle policy") completionIndex := strings.Index(prompt, "__LOOPER_RESULT__=") - if customIndex < 0 || lifecycleIndex < 0 || completionIndex < 0 || !(customIndex < lifecycleIndex && lifecycleIndex < completionIndex) { - t.Fatalf("unexpected prompt order custom=%d lifecycle=%d completion=%d\n%s", customIndex, lifecycleIndex, completionIndex, prompt) + if policyIndex < 0 || customIndex < 0 || lifecycleIndex < 0 || completionIndex < 0 || !(policyIndex < customIndex && customIndex < lifecycleIndex && lifecycleIndex < completionIndex) { + t.Fatalf("unexpected prompt order policy=%d custom=%d lifecycle=%d completion=%d\n%s", policyIndex, customIndex, lifecycleIndex, completionIndex, prompt) } } diff --git a/internal/workflowpolicy/builtin/matt-series.json b/internal/workflowpolicy/builtin/matt-series.json new file mode 100644 index 0000000..78b5482 --- /dev/null +++ b/internal/workflowpolicy/builtin/matt-series.json @@ -0,0 +1,19 @@ +{ + "id": "matt-series", + "name": "Matt Series Engineering Workflow", + "version": 1, + "roles": { + "planner": { + "instructions": "Clarify the requested outcome, scope, risks, validation path, and unresolved questions before writing the spec. Keep the spec actionable and avoid expanding beyond the issue." + }, + "reviewer": { + "instructions": "Build a quick module map before publishing findings. Inspect the full changed scope, collect evidence for each issue, continue after the first finding, and avoid shallow or subjective comments." + }, + "fixer": { + "instructions": "Verify or reproduce each listed fix item before changing code. Only repair the listed items, keep edits minimal, and validate that each handled item is actually resolved." + }, + "worker": { + "instructions": "Work in vertical slices. Prefer tests through public interfaces, implement only enough for the current behavior, validate before finishing, and avoid broad unrelated refactors." + } + } +} diff --git a/internal/workflowpolicy/doc.go b/internal/workflowpolicy/doc.go new file mode 100644 index 0000000..0597945 --- /dev/null +++ b/internal/workflowpolicy/doc.go @@ -0,0 +1,2 @@ +// Package workflowpolicy loads and resolves prompt-time workflow policy packs. +package workflowpolicy diff --git a/internal/workflowpolicy/policy.go b/internal/workflowpolicy/policy.go new file mode 100644 index 0000000..62036e6 --- /dev/null +++ b/internal/workflowpolicy/policy.go @@ -0,0 +1,181 @@ +package workflowpolicy + +import ( + "embed" + "encoding/json" + "fmt" + "os" + "path" + "strings" + + "github.com/powerformer/looper/internal/config" +) + +//go:embed builtin/*.json +var builtinPacks embed.FS + +type Pack struct { + ID string `json:"id"` + Name string `json:"name"` + Version int `json:"version"` + Roles map[string]RolePolicy `json:"roles"` +} + +type RolePolicy struct { + Instructions string `json:"instructions"` +} + +type Block struct { + Enabled bool `json:"enabled"` + Role string `json:"role"` + ProjectID string `json:"projectId,omitempty"` + PackID string `json:"packId,omitempty"` + PackName string `json:"packName,omitempty"` + Instructions string `json:"-"` +} + +func ResolveBlock(cfg config.Config, projectID, role string) (Block, error) { + block := Block{Enabled: cfg.WorkflowPolicyPacks.Enabled, Role: role, ProjectID: projectID} + if !cfg.WorkflowPolicyPacks.Enabled { + return block, nil + } + packID := strings.TrimSpace(rolePolicyPack(config.ProjectRoleConfigs(cfg, projectID), role)) + if packID == "" { + return block, nil + } + pack, err := LoadPack(cfg, packID) + if err != nil { + return block, err + } + policy, ok := pack.Roles[role] + if !ok || strings.TrimSpace(policy.Instructions) == "" { + return block, nil + } + block.PackID = pack.ID + block.PackName = pack.Name + block.Instructions = fmt.Sprintf("Workflow policy pack: %s (%s)\n%s", pack.ID, pack.Name, strings.TrimSpace(policy.Instructions)) + return block, nil +} + +func LoadPack(cfg config.Config, id string) (Pack, error) { + var ref *config.WorkflowPolicyPackRef + for index := range cfg.WorkflowPolicyPacks.Packs { + if cfg.WorkflowPolicyPacks.Packs[index].ID == id { + ref = &cfg.WorkflowPolicyPacks.Packs[index] + break + } + } + if ref == nil { + return Pack{}, fmt.Errorf("workflow policy pack %q is not configured", id) + } + pack, err := loadPackRef(*ref) + if err != nil { + return Pack{}, err + } + if err := ValidatePack(pack, cfg.Instructions.MaxBytes); err != nil { + return Pack{}, err + } + if pack.ID != ref.ID { + return Pack{}, fmt.Errorf("workflow policy pack %q loaded pack id %q", ref.ID, pack.ID) + } + if strings.TrimSpace(ref.Name) != "" && pack.Name != ref.Name { + pack.Name = ref.Name + } + return pack, nil +} + +func loadPackRef(ref config.WorkflowPolicyPackRef) (Pack, error) { + var raw []byte + var err error + switch ref.Source { + case config.WorkflowPolicyPackSourceBuiltin: + raw, err = builtinPacks.ReadFile(path.Join("builtin", ref.ID+".json")) + if err != nil { + return Pack{}, fmt.Errorf("load builtin workflow policy pack %q: %w", ref.ID, err) + } + case config.WorkflowPolicyPackSourceFile: + raw, err = os.ReadFile(ref.Path) + if err != nil { + return Pack{}, fmt.Errorf("load workflow policy pack file %q: %w", ref.Path, err) + } + default: + return Pack{}, fmt.Errorf("unsupported workflow policy pack source %q", ref.Source) + } + var pack Pack + if err := json.Unmarshal(raw, &pack); err != nil { + return Pack{}, fmt.Errorf("decode workflow policy pack %q: %w", ref.ID, err) + } + return pack, nil +} + +func ValidatePack(pack Pack, maxBytes int) error { + var issues []string + if strings.TrimSpace(pack.ID) == "" { + issues = append(issues, "id is required") + } + if strings.TrimSpace(pack.Name) == "" { + issues = append(issues, "name is required") + } + if pack.Version < 1 { + issues = append(issues, "version must be a positive integer") + } + if len(pack.Roles) == 0 { + issues = append(issues, "roles must contain at least one role") + } + for role, policy := range pack.Roles { + if !isRole(role) { + issues = append(issues, fmt.Sprintf("roles.%s is not a supported role", role)) + continue + } + text := strings.TrimSpace(policy.Instructions) + if text == "" { + issues = append(issues, fmt.Sprintf("roles.%s.instructions is required", role)) + continue + } + if maxBytes > 0 && len([]byte(text)) > maxBytes { + issues = append(issues, fmt.Sprintf("roles.%s.instructions must be at most %d bytes", role, maxBytes)) + } + if protected := protectedPhrase(text); protected != "" { + issues = append(issues, fmt.Sprintf("roles.%s.instructions must not override protected Looper contract %q", role, protected)) + } + } + if len(issues) > 0 { + return fmt.Errorf("workflow policy pack %q validation failed: %s", pack.ID, strings.Join(issues, "; ")) + } + return nil +} + +func rolePolicyPack(roles config.RoleConfigs, role string) string { + switch role { + case "planner": + return roles.Planner.PolicyPack + case "worker": + return roles.Worker.PolicyPack + case "reviewer": + return roles.Reviewer.PolicyPack + case "fixer": + return roles.Fixer.PolicyPack + default: + return "" + } +} + +func isRole(role string) bool { + switch role { + case "planner", "worker", "reviewer", "fixer": + return true + default: + return false + } +} + +func protectedPhrase(text string) string { + normalized := strings.ToLower(strings.Join(strings.Fields(text), " ")) + protected := []string{"systemprompt", "system prompt", "__looper_result__", "completion marker", "git_pr_lifecycle", "summary field", "commits field", "result json", "allowautopush", "allowautoapprove", "allow auto push", "allow auto approve", "auto approve", "auto push", "pr creation policy", "review submission policy", "looper review submit", "review submit wrapper", "gh pr review", "disclosure stamping", "auth requirement", "permission boundary", "state transition", "state machine", "ignore lifecycle", "override lifecycle", "custom completion"} + for _, phrase := range protected { + if strings.Contains(normalized, phrase) { + return phrase + } + } + return "" +} diff --git a/internal/workflowpolicy/policy_test.go b/internal/workflowpolicy/policy_test.go new file mode 100644 index 0000000..58dc7e6 --- /dev/null +++ b/internal/workflowpolicy/policy_test.go @@ -0,0 +1,103 @@ +package workflowpolicy + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/powerformer/looper/internal/config" +) + +func TestResolveBlockLoadsBuiltinMattSeries(t *testing.T) { + t.Parallel() + + packID := "matt-series" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{Roles: &config.PartialRoleConfigs{Worker: &config.PartialWorkerRoleConfig{PolicyPack: &packID}}}) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + + block, err := ResolveBlock(cfg, "", "worker") + if err != nil { + t.Fatalf("ResolveBlock() error = %v", err) + } + if block.PackID != "matt-series" || block.PackName != "Matt Series Engineering Workflow" { + t.Fatalf("block = %#v", block) + } + if !strings.Contains(block.Instructions, "Work in vertical slices") { + t.Fatalf("block instructions = %q, want worker policy", block.Instructions) + } +} + +func TestResolveBlockLoadsFilePack(t *testing.T) { + t.Parallel() + + packPath := filepath.Join(t.TempDir(), "pack.json") + raw := `{"id":"team-pack","name":"Team Pack","version":1,"roles":{"planner":{"instructions":"Plan with team context."}}}` + if err := os.WriteFile(packPath, []byte(raw), 0o644); err != nil { + t.Fatalf("os.WriteFile() error = %v", err) + } + packID := "team-pack" + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{ + WorkflowPolicyPacks: &config.PartialWorkflowPolicyPacksConfig{Packs: &[]config.WorkflowPolicyPackRef{{ID: packID, Name: "Team Pack", Source: config.WorkflowPolicyPackSourceFile, Path: packPath}}}, + Roles: &config.PartialRoleConfigs{Planner: &config.PartialPlannerRoleConfig{PolicyPack: &packID}}, + }) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + + block, err := ResolveBlock(cfg, "", "planner") + if err != nil { + t.Fatalf("ResolveBlock() error = %v", err) + } + if !strings.Contains(block.Instructions, "Plan with team context.") { + t.Fatalf("block instructions = %q", block.Instructions) + } +} + +func TestLoadPackRejectsMissingFile(t *testing.T) { + t.Parallel() + + cfg, err := config.Normalize(t.TempDir(), config.PartialConfig{ + WorkflowPolicyPacks: &config.PartialWorkflowPolicyPacksConfig{Packs: &[]config.WorkflowPolicyPackRef{{ID: "missing", Name: "Missing", Source: config.WorkflowPolicyPackSourceFile, Path: filepath.Join(t.TempDir(), "missing.json")}}}, + }) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + + if _, err := LoadPack(cfg, "missing"); err == nil || !strings.Contains(err.Error(), "load workflow policy pack file") { + t.Fatalf("LoadPack() error = %v, want missing file error", err) + } +} + +func TestValidatePackRejectsInvalidSchemaAndProtectedText(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pack Pack + want string + }{ + {name: "missing id", pack: Pack{Name: "Pack", Version: 1, Roles: map[string]RolePolicy{"worker": {Instructions: "Do work."}}}, want: "id is required"}, + {name: "missing name", pack: Pack{ID: "pack", Version: 1, Roles: map[string]RolePolicy{"worker": {Instructions: "Do work."}}}, want: "name is required"}, + {name: "missing version", pack: Pack{ID: "pack", Name: "Pack", Roles: map[string]RolePolicy{"worker": {Instructions: "Do work."}}}, want: "version must be a positive integer"}, + {name: "unknown role", pack: Pack{ID: "pack", Name: "Pack", Version: 1, Roles: map[string]RolePolicy{"architect": {Instructions: "Do work."}}}, want: "not a supported role"}, + {name: "missing instructions", pack: Pack{ID: "pack", Name: "Pack", Version: 1, Roles: map[string]RolePolicy{"worker": {}}}, want: "instructions is required"}, + {name: "protected text", pack: Pack{ID: "pack", Name: "Pack", Version: 1, Roles: map[string]RolePolicy{"worker": {Instructions: "Emit __LOOPER_RESULT__ yourself."}}}, want: "protected Looper contract"}, + {name: "byte limit", pack: Pack{ID: "pack", Name: "Pack", Version: 1, Roles: map[string]RolePolicy{"worker": {Instructions: "too long"}}}, want: "at most 3 bytes"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + maxBytes := 8192 + if tt.name == "byte limit" { + maxBytes = 3 + } + err := ValidatePack(tt.pack, maxBytes) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("ValidatePack() error = %v, want to contain %q", err, tt.want) + } + }) + } +}