From a14301e42a79ed657e8657d7d051506517fcf1d2 Mon Sep 17 00:00:00 2001 From: Fabian von Feilitzsch Date: Fri, 31 Jul 2026 12:34:40 -0400 Subject: [PATCH 1/2] :sparkles: Tell agents to stage ephemeral files in /tmp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent's working directory is the git worktree whose commits the harness pushes to the user's branch at the end of a stage. Nothing tells the agent that, so it treats the worktree as scratch space. Given a skill that said only "write the script to a file, make it executable, run it", claude-sonnet-5 wrote to /workspace/repo/verify.sh and left /tmp empty. Re-running the same skill with only this change: before: tool: write · /workspace/repo/verify.sh after: tool: shell · cat > /tmp/verify.sh << 'EOF' The rule goes in the harness rather than in each SkillCard because it describes the execution environment, not any particular skill — relying on every skill author to restate it is how it gets forgotten. It is emitted first so a skill that does say where to write cannot read as overriding it. Prompt assembly moves out of main into internal/prompt, alongside the other harness packages. Build is a pure function over an explicit Layers struct; the prompt env vars join the rest in config.LoadFromEnv, so env access stays in one package and the composition is testable without touching the environment. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Fabian von Feilitzsch --- changes/unreleased/staging-rules.yaml | 7 ++ harness/cmd/migration-harness/main.go | 41 +++-------- harness/cmd/migration-harness/main_test.go | 23 ------- harness/internal/config/config.go | 9 +++ harness/internal/config/config_test.go | 25 +++++++ harness/internal/prompt/prompt.go | 79 ++++++++++++++++++++++ harness/internal/prompt/prompt_test.go | 73 ++++++++++++++++++++ 7 files changed, 201 insertions(+), 56 deletions(-) create mode 100644 changes/unreleased/staging-rules.yaml create mode 100644 harness/internal/prompt/prompt.go create mode 100644 harness/internal/prompt/prompt_test.go diff --git a/changes/unreleased/staging-rules.yaml b/changes/unreleased/staging-rules.yaml new file mode 100644 index 00000000..86f3ca4b --- /dev/null +++ b/changes/unreleased/staging-rules.yaml @@ -0,0 +1,7 @@ +kind: feature + +description: > + Tell agents to write ephemeral files (scripts they run, scratch notes, + intermediate output) to /tmp rather than into the repository working tree, + whose commits the harness pushes to the user's branch. Prompt assembly moves + to internal/prompt with an explicit Layers API. diff --git a/harness/cmd/migration-harness/main.go b/harness/cmd/migration-harness/main.go index 19bd3d9f..166b0991 100644 --- a/harness/cmd/migration-harness/main.go +++ b/harness/cmd/migration-harness/main.go @@ -19,6 +19,7 @@ import ( "github.com/konveyor/migration-harness/internal/goose" "github.com/konveyor/migration-harness/internal/hub" "github.com/konveyor/migration-harness/internal/logging" + "github.com/konveyor/migration-harness/internal/prompt" "github.com/konveyor/migration-harness/internal/watcher" ) @@ -153,7 +154,12 @@ func runStage(cmd *cobra.Command, args []string) error { } // 7. Build prompt from context layers - prompt := buildPrompt(skillContent) + stagePrompt := prompt.Build(prompt.Layers{ + Agent: cfg.Prompt, + PlaybookContext: cfg.PlaybookInstructions, + Skill: skillContent, + StageTask: cfg.Instructions, + }) // 8. Start filesystem watcher BEFORE blocking prompt pushFn := func() error { @@ -172,7 +178,7 @@ func runStage(cmd *cobra.Command, args []string) error { logging.Header("Running Stage") logging.Info("max turns: %d", cfg.MaxTurns) _, err = session.SendPrompt(ctx, sessionID, []acp.ContentBlock{ - {Type: "text", Text: prompt}, + {Type: "text", Text: stagePrompt}, }, cfg.MaxTurns) if err != nil { @@ -250,37 +256,6 @@ func discoverSkills() (string, []string, error) { return combined.String(), matches, nil } -func buildPrompt(skillContent string) string { - var b strings.Builder - - if v := os.Getenv("KONVEYOR_PROMPT"); v != "" { - b.WriteString(v) - b.WriteString("\n\n") - } - - if v := os.Getenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS"); v != "" { - b.WriteString("## Migration Context\n\n") - b.WriteString(v) - b.WriteString("\n\n") - } - - if skillContent != "" { - b.WriteString("## Skill Instructions\n\n") - b.WriteString(skillContent) - b.WriteString("\n\n") - } else { - b.WriteString("## Working Guidelines\n\n") - b.WriteString("Commit your changes to git with a descriptive message when your work is complete.\n\n") - } - - if v := os.Getenv("KONVEYOR_INSTRUCTIONS"); v != "" { - b.WriteString("## Stage Task\n\n") - b.WriteString(v) - } - - return b.String() -} - func resolveFromHub(cfg *config.Config) (*git.Credentials, *hub.Client, error) { logging.Header("Hub Resolution") diff --git a/harness/cmd/migration-harness/main_test.go b/harness/cmd/migration-harness/main_test.go index a46b17b2..543adae7 100644 --- a/harness/cmd/migration-harness/main_test.go +++ b/harness/cmd/migration-harness/main_test.go @@ -69,26 +69,3 @@ func TestDiscoverSkills_EmptySkillFile(t *testing.T) { t.Errorf("expected 1 path (skill is mounted), got: %v", paths) } } - -func TestBuildPrompt_NoSkills(t *testing.T) { - t.Setenv("KONVEYOR_PROMPT", "hello") - t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "") - t.Setenv("KONVEYOR_INSTRUCTIONS", "do work") - - prompt := buildPrompt("") - expected := "hello\n\n## Working Guidelines\n\nCommit your changes to git with a descriptive message when your work is complete.\n\n## Stage Task\n\ndo work" - if prompt != expected { - t.Errorf("unexpected prompt:\n%s", prompt) - } -} - -func TestBuildPrompt_WithSkills(t *testing.T) { - t.Setenv("KONVEYOR_PROMPT", "hello") - t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "") - t.Setenv("KONVEYOR_INSTRUCTIONS", "do work") - - prompt := buildPrompt("skill content here") - if got := prompt; got != "hello\n\n## Skill Instructions\n\nskill content here\n\n## Stage Task\n\ndo work" { - t.Errorf("unexpected prompt:\n%s", got) - } -} diff --git a/harness/internal/config/config.go b/harness/internal/config/config.go index bbbe98e2..23aaa71d 100644 --- a/harness/internal/config/config.go +++ b/harness/internal/config/config.go @@ -23,6 +23,11 @@ type Config struct { ACPSecretKey string TargetBranch string + + // Prompt context layers, composed by internal/prompt. + Prompt string + PlaybookInstructions string + Instructions string } func LoadFromEnv() (*Config, error) { @@ -51,6 +56,10 @@ func LoadFromEnv() (*Config, error) { AppID: required["APP_ID"], ACPSecretKey: required["KONVEYOR_ACP_SECRET_KEY"], TargetBranch: required["TARGET_BRANCH"], + + Prompt: os.Getenv("KONVEYOR_PROMPT"), + PlaybookInstructions: os.Getenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS"), + Instructions: os.Getenv("KONVEYOR_INSTRUCTIONS"), } if n, err := strconv.Atoi(os.Getenv("KONVEYOR_PARAM_MAX_TURNS")); err == nil && n > 0 { diff --git a/harness/internal/config/config_test.go b/harness/internal/config/config_test.go index debc7197..c45465d2 100644 --- a/harness/internal/config/config_test.go +++ b/harness/internal/config/config_test.go @@ -19,6 +19,9 @@ func clearKonveyorEnv(t *testing.T) { "APP_ID", "KONVEYOR_ACP_SECRET_KEY", "TARGET_BRANCH", + "KONVEYOR_PROMPT", + "KONVEYOR_PLAYBOOK_INSTRUCTIONS", + "KONVEYOR_INSTRUCTIONS", } { t.Setenv(k, "") os.Unsetenv(k) @@ -131,3 +134,25 @@ func TestLoadFromEnv(t *testing.T) { } }) } + +func TestLoadFromEnvReadsPromptLayers(t *testing.T) { + clearKonveyorEnv(t) + setRequiredEnv(t) + t.Setenv("KONVEYOR_PROMPT", "AGENT PROMPT") + t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "PLAYBOOK CONTEXT") + t.Setenv("KONVEYOR_INSTRUCTIONS", "STAGE TASK") + + cfg, err := LoadFromEnv() + if err != nil { + t.Fatalf("LoadFromEnv: %v", err) + } + if cfg.Prompt != "AGENT PROMPT" { + t.Errorf("Prompt = %q", cfg.Prompt) + } + if cfg.PlaybookInstructions != "PLAYBOOK CONTEXT" { + t.Errorf("PlaybookInstructions = %q", cfg.PlaybookInstructions) + } + if cfg.Instructions != "STAGE TASK" { + t.Errorf("Instructions = %q", cfg.Instructions) + } +} diff --git a/harness/internal/prompt/prompt.go b/harness/internal/prompt/prompt.go new file mode 100644 index 00000000..7d2b6437 --- /dev/null +++ b/harness/internal/prompt/prompt.go @@ -0,0 +1,79 @@ +// Package prompt assembles the prompt sent to the agent for a stage. +// +// The prompt is layered: environment rules the harness imposes, then the +// Agent's standing prompt, then playbook context, then the skill, then the +// stage task. Later layers are more specific; the environment rules come first +// because they constrain everything after them. +package prompt + +import "strings" + +// stagingRules apply to every agent and skill. The working directory is the git +// worktree whose commits the harness pushes on exit, so ephemeral files left +// there are ambiguous at best, and get swept in if the agent stages broadly. +const stagingRules = `## Working Environment + +Your working directory is a git repository, and you decide what gets committed. +The harness pushes your commits to the user's branch when the stage ends. + +Write ephemeral files to /tmp, never into the repository working tree: + - scripts you need to run: write them to /tmp, make them executable there, + and run them from there + - scratch notes, plans, logs, and intermediate output + +Only files the user actually asked for belong in the repository. Keep the +worktree clean: anything left there is ambiguous, and gets swept into your +commits if you stage broadly. This applies even when a skill's instructions do +not say where to put something. +` + +// Layers are the context layers composed into a stage prompt, ordered from +// least to most specific. Any of them may be empty except Skill. +type Layers struct { + // Agent is the Agent's standing prompt. + Agent string + // PlaybookContext is the playbook's ambient guide. + PlaybookContext string + // Skill is the content discovered from the mounted SkillCards. + Skill string + // StageTask is the task for this stage. + StageTask string +} + +// Build composes the stage prompt. The harness's environment rules always come +// first, so a skill cannot be read as overriding them. +func Build(l Layers) string { + var b strings.Builder + + b.WriteString(stagingRules) + b.WriteString("\n") + + if l.Agent != "" { + b.WriteString(l.Agent) + b.WriteString("\n\n") + } + + if l.PlaybookContext != "" { + b.WriteString("## Migration Context\n\n") + b.WriteString(l.PlaybookContext) + b.WriteString("\n\n") + } + + // Skills are optional (#82): with none mounted the agent still needs to be + // told to commit, since nothing else in the prompt says so. + if l.Skill != "" { + b.WriteString("## Skill Instructions\n\n") + b.WriteString(l.Skill) + b.WriteString("\n\n") + } else { + b.WriteString("## Working Guidelines\n\n") + b.WriteString("Commit your changes to git with a descriptive message when your work is complete.\n\n") + } + + if l.StageTask != "" { + b.WriteString("## Stage Task\n\n") + b.WriteString(l.StageTask) + } + + return b.String() +} diff --git a/harness/internal/prompt/prompt_test.go b/harness/internal/prompt/prompt_test.go new file mode 100644 index 00000000..b6942ef3 --- /dev/null +++ b/harness/internal/prompt/prompt_test.go @@ -0,0 +1,73 @@ +package prompt + +import ( + "strings" + "testing" +) + +func TestBuildPutsStagingRulesFirst(t *testing.T) { + got := Build(Layers{ + Agent: "AGENT PROMPT", + PlaybookContext: "PLAYBOOK CONTEXT", + Skill: "SKILL BODY", + StageTask: "STAGE TASK", + }) + + if !strings.Contains(got, "Write ephemeral files to /tmp") { + t.Fatalf("staging rules missing from prompt:\n%s", got) + } + + // Order matters as much as presence: the rules have to precede the skill, + // so a skill that says where to write cannot read as overriding them. + rules := strings.Index(got, "Working Environment") + for _, later := range []string{"AGENT PROMPT", "PLAYBOOK CONTEXT", "SKILL BODY", "STAGE TASK"} { + if strings.Index(got, later) < rules { + t.Errorf("%q appears before the staging rules; rules must come first", later) + } + } +} + +func TestBuildOrdersLayersLeastToMostSpecific(t *testing.T) { + got := Build(Layers{ + Agent: "AGENT PROMPT", + PlaybookContext: "PLAYBOOK CONTEXT", + Skill: "SKILL BODY", + StageTask: "STAGE TASK", + }) + + order := []string{"AGENT PROMPT", "PLAYBOOK CONTEXT", "SKILL BODY", "STAGE TASK"} + for i := 1; i < len(order); i++ { + if strings.Index(got, order[i]) < strings.Index(got, order[i-1]) { + t.Errorf("%q should come after %q", order[i], order[i-1]) + } + } +} + +func TestBuildOmitsEmptyLayers(t *testing.T) { + got := Build(Layers{Skill: "SKILL BODY"}) + + for _, header := range []string{"## Migration Context", "## Stage Task"} { + if strings.Contains(got, header) { + t.Errorf("empty layer produced %q header", header) + } + } + if !strings.Contains(got, "## Skill Instructions") { + t.Error("skill content should always be included") + } +} + +// Skills are optional (#82). With none mounted the agent still needs to be told +// to commit, since nothing else in the prompt says so. +func TestBuildFallsBackWhenNoSkills(t *testing.T) { + got := Build(Layers{Agent: "AGENT PROMPT"}) + + if strings.Contains(got, "## Skill Instructions") { + t.Error("empty skill should not produce a Skill Instructions header") + } + if !strings.Contains(got, "## Working Guidelines") { + t.Fatalf("no skills should fall back to Working Guidelines:\n%s", got) + } + if !strings.Contains(got, "Commit your changes to git") { + t.Error("fallback should tell the agent to commit") + } +} From 261421ca716ea3ffd0db1dce704933b895db0545 Mon Sep 17 00:00:00 2001 From: Fabian von Feilitzsch Date: Fri, 31 Jul 2026 15:33:37 -0400 Subject: [PATCH 2/2] :seedling: Address review: naming, workflow guide, prompt accuracy Rename the config fields to say what they are: AgentPrompt, WorkflowGuide, StageInstructions. cfg.Instructions was ambiguous next to the model and provider fields. Read KONVEYOR_WORKFLOW_GUIDE, falling back to KONVEYOR_PLAYBOOK_INSTRUCTIONS. that merge rather than depending on merge order. Drop the fallback once #80 has landed everywhere. Correct the staging rules: the harness commits .gitignore and .konveyor/analysis.json itself, so "you decide what gets committed" overstated the agent's control. Rename the ## Migration Context header to ## Workflow Guide. The prompt package is general purpose and shouldn't hardcode migration. Always end the prompt with exactly one newline. Sections appended "\n\n" but StageTask appended nothing, so the ending varied with whether a stage task was set. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Fabian von Feilitzsch --- harness/cmd/migration-harness/main.go | 8 ++-- harness/internal/config/config.go | 24 ++++++++--- harness/internal/config/config_test.go | 46 ++++++++++++++++---- harness/internal/prompt/prompt.go | 28 +++++++------ harness/internal/prompt/prompt_test.go | 58 +++++++++++++++++++------- 5 files changed, 118 insertions(+), 46 deletions(-) diff --git a/harness/cmd/migration-harness/main.go b/harness/cmd/migration-harness/main.go index 166b0991..5c3a87aa 100644 --- a/harness/cmd/migration-harness/main.go +++ b/harness/cmd/migration-harness/main.go @@ -155,10 +155,10 @@ func runStage(cmd *cobra.Command, args []string) error { // 7. Build prompt from context layers stagePrompt := prompt.Build(prompt.Layers{ - Agent: cfg.Prompt, - PlaybookContext: cfg.PlaybookInstructions, - Skill: skillContent, - StageTask: cfg.Instructions, + AgentPrompt: cfg.AgentPrompt, + WorkflowGuide: cfg.WorkflowGuide, + Skill: skillContent, + StageTask: cfg.StageInstructions, }) // 8. Start filesystem watcher BEFORE blocking prompt diff --git a/harness/internal/config/config.go b/harness/internal/config/config.go index 23aaa71d..4d4a9c5d 100644 --- a/harness/internal/config/config.go +++ b/harness/internal/config/config.go @@ -25,9 +25,9 @@ type Config struct { TargetBranch string // Prompt context layers, composed by internal/prompt. - Prompt string - PlaybookInstructions string - Instructions string + AgentPrompt string + WorkflowGuide string + StageInstructions string } func LoadFromEnv() (*Config, error) { @@ -57,9 +57,9 @@ func LoadFromEnv() (*Config, error) { ACPSecretKey: required["KONVEYOR_ACP_SECRET_KEY"], TargetBranch: required["TARGET_BRANCH"], - Prompt: os.Getenv("KONVEYOR_PROMPT"), - PlaybookInstructions: os.Getenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS"), - Instructions: os.Getenv("KONVEYOR_INSTRUCTIONS"), + AgentPrompt: os.Getenv("KONVEYOR_PROMPT"), + WorkflowGuide: workflowGuideFromEnv(), + StageInstructions: os.Getenv("KONVEYOR_INSTRUCTIONS"), } if n, err := strconv.Atoi(os.Getenv("KONVEYOR_PARAM_MAX_TURNS")); err == nil && n > 0 { @@ -68,3 +68,15 @@ func LoadFromEnv() (*Config, error) { return cfg, nil } + +// workflowGuideFromEnv reads the workflow guide the controller injects. +// +// konveyor/agentic-controller#80 renames KONVEYOR_PLAYBOOK_INSTRUCTIONS to +// KONVEYOR_WORKFLOW_GUIDE. Reading both means the harness works either side of +// that merge; drop the fallback once #80 has landed everywhere. +func workflowGuideFromEnv() string { + if v := os.Getenv("KONVEYOR_WORKFLOW_GUIDE"); v != "" { + return v + } + return os.Getenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS") +} diff --git a/harness/internal/config/config_test.go b/harness/internal/config/config_test.go index c45465d2..72bd3b91 100644 --- a/harness/internal/config/config_test.go +++ b/harness/internal/config/config_test.go @@ -21,6 +21,7 @@ func clearKonveyorEnv(t *testing.T) { "TARGET_BRANCH", "KONVEYOR_PROMPT", "KONVEYOR_PLAYBOOK_INSTRUCTIONS", + "KONVEYOR_WORKFLOW_GUIDE", "KONVEYOR_INSTRUCTIONS", } { t.Setenv(k, "") @@ -139,20 +140,51 @@ func TestLoadFromEnvReadsPromptLayers(t *testing.T) { clearKonveyorEnv(t) setRequiredEnv(t) t.Setenv("KONVEYOR_PROMPT", "AGENT PROMPT") - t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "PLAYBOOK CONTEXT") + t.Setenv("KONVEYOR_WORKFLOW_GUIDE", "WORKFLOW GUIDE") t.Setenv("KONVEYOR_INSTRUCTIONS", "STAGE TASK") cfg, err := LoadFromEnv() if err != nil { t.Fatalf("LoadFromEnv: %v", err) } - if cfg.Prompt != "AGENT PROMPT" { - t.Errorf("Prompt = %q", cfg.Prompt) + if cfg.AgentPrompt != "AGENT PROMPT" { + t.Errorf("AgentPrompt = %q", cfg.AgentPrompt) } - if cfg.PlaybookInstructions != "PLAYBOOK CONTEXT" { - t.Errorf("PlaybookInstructions = %q", cfg.PlaybookInstructions) + if cfg.WorkflowGuide != "WORKFLOW GUIDE" { + t.Errorf("WorkflowGuide = %q", cfg.WorkflowGuide) } - if cfg.Instructions != "STAGE TASK" { - t.Errorf("Instructions = %q", cfg.Instructions) + if cfg.StageInstructions != "STAGE TASK" { + t.Errorf("StageInstructions = %q", cfg.StageInstructions) + } +} + +// #80 renames the env var; the harness reads either so merge order does not +// matter. Remove with the fallback once #80 has landed everywhere. +func TestLoadFromEnvFallsBackToPlaybookInstructions(t *testing.T) { + clearKonveyorEnv(t) + setRequiredEnv(t) + t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "OLD NAME") + + cfg, err := LoadFromEnv() + if err != nil { + t.Fatalf("LoadFromEnv: %v", err) + } + if cfg.WorkflowGuide != "OLD NAME" { + t.Errorf("WorkflowGuide = %q, want the KONVEYOR_PLAYBOOK_INSTRUCTIONS value", cfg.WorkflowGuide) + } +} + +func TestLoadFromEnvPrefersWorkflowGuide(t *testing.T) { + clearKonveyorEnv(t) + setRequiredEnv(t) + t.Setenv("KONVEYOR_PLAYBOOK_INSTRUCTIONS", "OLD NAME") + t.Setenv("KONVEYOR_WORKFLOW_GUIDE", "NEW NAME") + + cfg, err := LoadFromEnv() + if err != nil { + t.Fatalf("LoadFromEnv: %v", err) + } + if cfg.WorkflowGuide != "NEW NAME" { + t.Errorf("WorkflowGuide = %q, want the KONVEYOR_WORKFLOW_GUIDE value", cfg.WorkflowGuide) } } diff --git a/harness/internal/prompt/prompt.go b/harness/internal/prompt/prompt.go index 7d2b6437..3ffd34e9 100644 --- a/harness/internal/prompt/prompt.go +++ b/harness/internal/prompt/prompt.go @@ -1,7 +1,7 @@ // Package prompt assembles the prompt sent to the agent for a stage. // // The prompt is layered: environment rules the harness imposes, then the -// Agent's standing prompt, then playbook context, then the skill, then the +// Agent's standing prompt, then the workflow guide, then the skill, then the // stage task. Later layers are more specific; the environment rules come first // because they constrain everything after them. package prompt @@ -13,8 +13,8 @@ import "strings" // there are ambiguous at best, and get swept in if the agent stages broadly. const stagingRules = `## Working Environment -Your working directory is a git repository, and you decide what gets committed. -The harness pushes your commits to the user's branch when the stage ends. +Your working directory is a git repository. Anything you commit is pushed to the +user's branch when the stage ends. Write ephemeral files to /tmp, never into the repository working tree: - scripts you need to run: write them to /tmp, make them executable there, @@ -30,10 +30,10 @@ not say where to put something. // Layers are the context layers composed into a stage prompt, ordered from // least to most specific. Any of them may be empty except Skill. type Layers struct { - // Agent is the Agent's standing prompt. - Agent string - // PlaybookContext is the playbook's ambient guide. - PlaybookContext string + // AgentPrompt is the Agent's standing prompt. + AgentPrompt string + // WorkflowGuide is the workflow's ambient guide. + WorkflowGuide string // Skill is the content discovered from the mounted SkillCards. Skill string // StageTask is the task for this stage. @@ -48,14 +48,14 @@ func Build(l Layers) string { b.WriteString(stagingRules) b.WriteString("\n") - if l.Agent != "" { - b.WriteString(l.Agent) + if l.AgentPrompt != "" { + b.WriteString(l.AgentPrompt) b.WriteString("\n\n") } - if l.PlaybookContext != "" { - b.WriteString("## Migration Context\n\n") - b.WriteString(l.PlaybookContext) + if l.WorkflowGuide != "" { + b.WriteString("## Workflow Guide\n\n") + b.WriteString(l.WorkflowGuide) b.WriteString("\n\n") } @@ -75,5 +75,7 @@ func Build(l Layers) string { b.WriteString(l.StageTask) } - return b.String() + // Normalise the ending: sections above end with either "\n\n" or, for + // StageTask, no newline at all. Always finish with exactly one. + return strings.TrimRight(b.String(), "\n") + "\n" } diff --git a/harness/internal/prompt/prompt_test.go b/harness/internal/prompt/prompt_test.go index b6942ef3..beb0a78d 100644 --- a/harness/internal/prompt/prompt_test.go +++ b/harness/internal/prompt/prompt_test.go @@ -5,13 +5,17 @@ import ( "testing" ) +func fullLayers() Layers { + return Layers{ + AgentPrompt: "AGENT PROMPT", + WorkflowGuide: "WORKFLOW GUIDE", + Skill: "SKILL BODY", + StageTask: "STAGE TASK", + } +} + func TestBuildPutsStagingRulesFirst(t *testing.T) { - got := Build(Layers{ - Agent: "AGENT PROMPT", - PlaybookContext: "PLAYBOOK CONTEXT", - Skill: "SKILL BODY", - StageTask: "STAGE TASK", - }) + got := Build(fullLayers()) if !strings.Contains(got, "Write ephemeral files to /tmp") { t.Fatalf("staging rules missing from prompt:\n%s", got) @@ -20,7 +24,7 @@ func TestBuildPutsStagingRulesFirst(t *testing.T) { // Order matters as much as presence: the rules have to precede the skill, // so a skill that says where to write cannot read as overriding them. rules := strings.Index(got, "Working Environment") - for _, later := range []string{"AGENT PROMPT", "PLAYBOOK CONTEXT", "SKILL BODY", "STAGE TASK"} { + for _, later := range []string{"AGENT PROMPT", "WORKFLOW GUIDE", "SKILL BODY", "STAGE TASK"} { if strings.Index(got, later) < rules { t.Errorf("%q appears before the staging rules; rules must come first", later) } @@ -28,14 +32,9 @@ func TestBuildPutsStagingRulesFirst(t *testing.T) { } func TestBuildOrdersLayersLeastToMostSpecific(t *testing.T) { - got := Build(Layers{ - Agent: "AGENT PROMPT", - PlaybookContext: "PLAYBOOK CONTEXT", - Skill: "SKILL BODY", - StageTask: "STAGE TASK", - }) + got := Build(fullLayers()) - order := []string{"AGENT PROMPT", "PLAYBOOK CONTEXT", "SKILL BODY", "STAGE TASK"} + order := []string{"AGENT PROMPT", "WORKFLOW GUIDE", "SKILL BODY", "STAGE TASK"} for i := 1; i < len(order); i++ { if strings.Index(got, order[i]) < strings.Index(got, order[i-1]) { t.Errorf("%q should come after %q", order[i], order[i-1]) @@ -46,7 +45,7 @@ func TestBuildOrdersLayersLeastToMostSpecific(t *testing.T) { func TestBuildOmitsEmptyLayers(t *testing.T) { got := Build(Layers{Skill: "SKILL BODY"}) - for _, header := range []string{"## Migration Context", "## Stage Task"} { + for _, header := range []string{"## Workflow Guide", "## Stage Task"} { if strings.Contains(got, header) { t.Errorf("empty layer produced %q header", header) } @@ -59,7 +58,7 @@ func TestBuildOmitsEmptyLayers(t *testing.T) { // Skills are optional (#82). With none mounted the agent still needs to be told // to commit, since nothing else in the prompt says so. func TestBuildFallsBackWhenNoSkills(t *testing.T) { - got := Build(Layers{Agent: "AGENT PROMPT"}) + got := Build(Layers{AgentPrompt: "AGENT PROMPT"}) if strings.Contains(got, "## Skill Instructions") { t.Error("empty skill should not produce a Skill Instructions header") @@ -71,3 +70,30 @@ func TestBuildFallsBackWhenNoSkills(t *testing.T) { t.Error("fallback should tell the agent to commit") } } + +// The ending differed depending on whether StageTask was set: sections append +// "\n\n" but StageTask appended nothing. +func TestBuildEndsWithExactlyOneNewline(t *testing.T) { + cases := map[string]Layers{ + "with stage task": fullLayers(), + "without stage task": {Skill: "SKILL BODY"}, + } + for name, layers := range cases { + t.Run(name, func(t *testing.T) { + got := Build(layers) + if !strings.HasSuffix(got, "\n") { + t.Errorf("prompt does not end with a newline: %q", tail(got)) + } + if strings.HasSuffix(got, "\n\n") { + t.Errorf("prompt ends with more than one newline: %q", tail(got)) + } + }) + } +} + +func tail(s string) string { + if len(s) < 20 { + return s + } + return s[len(s)-20:] +}