From cfd69ea6c5b6bfb37444b0a9facc21b5a88c666e Mon Sep 17 00:00:00 2001 From: Marcus Vorwaller Date: Sun, 5 Apr 2026 04:06:46 -0700 Subject: [PATCH] fix: separate commit-normalize guardrails from trailers Centralize the commit message contract for planning and implementation prompts, and keep commit-normalize safety guidance outside the literal trailer block. Refresh the task definition, docs, and regression tests so the safer prompt-level scope stays aligned across the built-in task surface. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- internal/orchestrator/orchestrator.go | 40 +++++++-- internal/orchestrator/orchestrator_test.go | 99 ++++++++++++++++++++++ internal/tasks/tasks.go | 2 +- internal/tasks/tasks_test.go | 16 ++++ website/docs/task-reference.md | 4 +- 5 files changed, 151 insertions(+), 10 deletions(-) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 6141c95..4699ac6 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -711,6 +711,34 @@ func (o *Orchestrator) PlanPrompt(task *tasks.Task) string { return o.buildPlanPrompt(task) } +func commitTrailerTask(task *tasks.Task) string { + if task == nil { + return "" + } + if task.Type != "" { + return string(task.Type) + } + return task.ID +} + +func commitMessageContract(task *tasks.Task) string { + var b strings.Builder + b.WriteString("If you create commits, format each message consistently:\n") + b.WriteString(" Subject: short imperative summary. Use a conventional prefix such as `fix:` or `docs:` when it fits the repo's style.\n") + b.WriteString(" Body: optional brief rationale or context when the subject alone is not enough.\n") + b.WriteString(" Trailers: keep these exact lines at the end of the message:\n") + fmt.Fprintf(&b, " Nightshift-Task: %s\n", commitTrailerTask(task)) + b.WriteString(" Nightshift-Ref: https://github.com/marcus/nightshift") + + if task != nil && task.Type == tasks.TaskCommitNormalize { + b.WriteString("\n\n For `commit-normalize`, these are work guardrails, not commit trailers:\n") + b.WriteString(" - Allowed scope: normalize only commits you create for this task or commits that exist only on your feature branch and are not yet shared.\n") + b.WriteString(" - Rewrite safety: do not rewrite the primary branch, protected branches, tags, or already-shared history; explain the limitation instead.") + } + + return b.String() +} + func (o *Orchestrator) buildPlanPrompt(task *tasks.Task) string { branchInstruction := "" if o.runMeta != nil && o.runMeta.Branch != "" { @@ -728,9 +756,7 @@ Description: %s 0. You are running autonomously. If the task is broad or ambiguous, choose a concrete, minimal scope that delivers value and state any assumptions in the description. 1. Work on a new branch and plan to submit a PR. Never work directly on the primary branch.%s 2. Before creating your branch, record the current branch name and plan to switch back after the PR is opened. -3. If you create commits, include a concise message with these git trailers: - Nightshift-Task: %s - Nightshift-Ref: https://github.com/marcus/nightshift +3. %s 4. Analyze the task requirements 5. Identify files that need to be modified 6. Create step-by-step implementation plan @@ -741,7 +767,7 @@ Description: %s "files": ["file1.go", "file2.go", ...], "description": "overall approach" } -`, task.ID, task.Title, task.Description, branchInstruction, task.Type) +`, task.ID, task.Title, task.Description, branchInstruction, commitMessageContract(task)) } func (o *Orchestrator) buildImplementPrompt(task *tasks.Task, plan *PlanOutput, iteration int) string { @@ -771,9 +797,7 @@ Description: %s ## Instructions 0. Before creating your branch, record the current branch name. Create and work on a new branch. Never modify or commit directly to the primary branch.%s When finished, open a PR. After the PR is submitted, switch back to the original branch. If you cannot open a PR, leave the branch and explain next steps. -1. If you create commits, include a concise message with these git trailers: - Nightshift-Task: %s - Nightshift-Ref: https://github.com/marcus/nightshift +1. %s 2. Implement the plan step by step 3. Make all necessary code changes 4. Ensure tests pass @@ -783,7 +807,7 @@ Description: %s "files_modified": ["file1.go", ...], "summary": "what was done" } -`, task.ID, task.Title, task.Description, plan.Description, plan.Steps, iterationNote, branchInstruction, task.Type) +`, task.ID, task.Title, task.Description, plan.Description, plan.Steps, iterationNote, branchInstruction, commitMessageContract(task)) } func (o *Orchestrator) buildReviewPrompt(task *tasks.Task, impl *ImplementOutput) string { diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 45bff5c..cffd511 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -430,6 +430,7 @@ func TestBuildPrompts(t *testing.T) { ID: "prompt-test", Title: "Build Prompts", Description: "Test prompt generation", + Type: tasks.TaskLintFix, } // Test plan prompt @@ -440,6 +441,16 @@ func TestBuildPrompts(t *testing.T) { if !containsIgnoreCase(planPrompt, "prompt-test") { t.Error("plan prompt should contain task ID") } + for _, want := range []string{ + "Subject: short imperative summary.", + "Body: optional brief rationale or context when the subject alone is not enough.", + "Nightshift-Task: lint-fix", + "Nightshift-Ref: https://github.com/marcus/nightshift", + } { + if !strings.Contains(planPrompt, want) { + t.Errorf("plan prompt missing %q\nGot:\n%s", want, planPrompt) + } + } // Test implement prompt plan := &PlanOutput{ @@ -450,6 +461,16 @@ func TestBuildPrompts(t *testing.T) { if !containsIgnoreCase(implPrompt, "implementation") { t.Error("implement prompt should mention implementation") } + for _, want := range []string{ + "Subject: short imperative summary.", + "Body: optional brief rationale or context when the subject alone is not enough.", + "Nightshift-Task: lint-fix", + "Nightshift-Ref: https://github.com/marcus/nightshift", + } { + if !strings.Contains(implPrompt, want) { + t.Errorf("implement prompt missing %q\nGot:\n%s", want, implPrompt) + } + } // Test implement prompt iteration 2 implPrompt2 := o.buildImplementPrompt(task, plan, 2) @@ -860,6 +881,84 @@ func TestBuildMetadataBlock_NoBranch(t *testing.T) { } } +func TestBuildPlanPrompt_CommitNormalizeGuardrails(t *testing.T) { + o := New() + o.SetRunMetadata(&RunMetadata{Branch: "main"}) + + task := &tasks.Task{ + ID: "commit-normalize:/repo", + Title: "Commit Message Normalizer", + Description: "Normalize commit messages safely", + Type: tasks.TaskCommitNormalize, + } + + prompt := o.buildPlanPrompt(task) + for _, want := range []string{ + "Create your feature branch from `main`.", + "Subject: short imperative summary.", + "Nightshift-Task: commit-normalize", + "For `commit-normalize`, these are work guardrails, not commit trailers:", + "- Allowed scope: normalize only commits you create for this task or commits that exist only on your feature branch and are not yet shared.", + "- Rewrite safety: do not rewrite the primary branch, protected branches, tags, or already-shared history; explain the limitation instead.", + } { + if !strings.Contains(prompt, want) { + t.Errorf("plan prompt missing %q\nGot:\n%s", want, prompt) + } + } + if strings.Contains(prompt, "\n Scope:") || strings.Contains(prompt, "\n Safety:") { + t.Errorf("plan prompt should not render scope/safety as trailer lines\nGot:\n%s", prompt) + } +} + +func TestBuildImplementPrompt_CommitNormalizeGuardrails(t *testing.T) { + o := New() + o.SetRunMetadata(&RunMetadata{Branch: "main"}) + + task := &tasks.Task{ + ID: "commit-normalize:/repo", + Title: "Commit Message Normalizer", + Description: "Normalize commit messages safely", + Type: tasks.TaskCommitNormalize, + } + plan := &PlanOutput{ + Steps: []string{"update prompt guidance"}, + Description: "Normalize only safe-to-rewrite commits", + } + + prompt := o.buildImplementPrompt(task, plan, 1) + for _, want := range []string{ + "Checkout `main` before creating your feature branch.", + "Subject: short imperative summary.", + "Nightshift-Task: commit-normalize", + "For `commit-normalize`, these are work guardrails, not commit trailers:", + "- Allowed scope: normalize only commits you create for this task or commits that exist only on your feature branch and are not yet shared.", + "- Rewrite safety: do not rewrite the primary branch, protected branches, tags, or already-shared history; explain the limitation instead.", + } { + if !strings.Contains(prompt, want) { + t.Errorf("implement prompt missing %q\nGot:\n%s", want, prompt) + } + } + if strings.Contains(prompt, "\n Scope:") || strings.Contains(prompt, "\n Safety:") { + t.Errorf("implement prompt should not render scope/safety as trailer lines\nGot:\n%s", prompt) + } +} + +func TestBuildPlanPrompt_NonCommitNormalizeOmitsRewriteGuardrails(t *testing.T) { + o := New() + + task := &tasks.Task{ + ID: "lint-fix:/repo", + Title: "Lint Fix", + Description: "Fix lint", + Type: tasks.TaskLintFix, + } + + prompt := o.buildPlanPrompt(task) + if strings.Contains(prompt, "work guardrails, not commit trailers") { + t.Errorf("plan prompt should not include commit-normalize-specific guardrails\nGot:\n%s", prompt) + } +} + func TestCurrentBranch(t *testing.T) { // Create a temp git repo dir := t.TempDir() diff --git a/internal/tasks/tasks.go b/internal/tasks/tasks.go index 2c7dabb..b2c97f7 100644 --- a/internal/tasks/tasks.go +++ b/internal/tasks/tasks.go @@ -332,7 +332,7 @@ Apply safe updates directly, and leave concise follow-ups for anything uncertain Type: TaskCommitNormalize, Category: CategoryPR, Name: "Commit Message Normalizer", - Description: "Standardize commit message format", + Description: "Standardize commit messages created by Nightshift to use a short imperative subject, optional body, and required trailers without rewriting shared history", CostTier: CostLow, RiskLevel: RiskLow, DefaultInterval: 24 * time.Hour, diff --git a/internal/tasks/tasks_test.go b/internal/tasks/tasks_test.go index 03cdf18..10b891f 100644 --- a/internal/tasks/tasks_test.go +++ b/internal/tasks/tasks_test.go @@ -97,6 +97,15 @@ func TestGetDefinition(t *testing.T) { t.Errorf("GetDefinition(TaskLintFix).CostTier = %d, want %d", def.CostTier, CostLow) } + commitNormalize, err := GetDefinition(TaskCommitNormalize) + if err != nil { + t.Fatalf("GetDefinition(TaskCommitNormalize) returned error: %v", err) + } + wantDescription := "Standardize commit messages created by Nightshift to use a short imperative subject, optional body, and required trailers without rewriting shared history" + if commitNormalize.Description != wantDescription { + t.Errorf("GetDefinition(TaskCommitNormalize).Description = %q, want %q", commitNormalize.Description, wantDescription) + } + // Unknown task type _, err = GetDefinition("unknown-task") if err == nil { @@ -136,10 +145,17 @@ func TestGetTasksByCategory(t *testing.T) { if len(prTasks) == 0 { t.Error("GetTasksByCategory(CategoryPR) returned empty slice") } + foundCommitNormalize := false for _, task := range prTasks { if task.Category != CategoryPR { t.Errorf("GetTasksByCategory(CategoryPR) returned task with category %d", task.Category) } + if task.Type == TaskCommitNormalize { + foundCommitNormalize = true + } + } + if !foundCommitNormalize { + t.Error("GetTasksByCategory(CategoryPR) should include TaskCommitNormalize") } // Verify all 6 categories have tasks diff --git a/website/docs/task-reference.md b/website/docs/task-reference.md index cb241df..c015433 100644 --- a/website/docs/task-reference.md +++ b/website/docs/task-reference.md @@ -23,7 +23,7 @@ Fully formed, review-ready artifacts. These tasks create branches and open pull | `backward-compat` | Backward-Compatibility Checks | Check and ensure backward compatibility | Medium | Low | 7d | | `build-optimize` | Build Time Optimization | Optimize build configuration for faster builds | High | Medium | 7d | | `docs-backfill` | Documentation Backfiller | Generate missing documentation | Low | Low | 7d | -| `commit-normalize` | Commit Message Normalizer | Standardize commit message format | Low | Low | 24h | +| `commit-normalize` | Commit Message Normalizer | Standardize commit messages created by Nightshift to use a short imperative subject, optional body, and required trailers without rewriting shared history | Low | Low | 24h | | `changelog-synth` | Changelog Synthesizer | Generate changelog from commits | Low | Low | 7d | | `release-notes` | Release Note Drafter | Draft release notes from changes | Low | Low | 7d | | `adr-draft` | ADR Drafter | Draft Architecture Decision Records | Medium | Low | 7d | @@ -33,6 +33,8 @@ Fully formed, review-ready artifacts. These tasks create branches and open pull `td-review` is **disabled by default** and must be explicitly opted in via `tasks.enabled`. It requires the td integration to be enabled (see [Integrations](/docs/integrations)). ::: +`commit-normalize` is intentionally scoped to safe normalization. Agents should standardize commit messages they create for the task, or private feature-branch commits that have not been shared yet, using a short imperative subject, an optional explanatory body, and the required `Nightshift-Task` and `Nightshift-Ref` trailers. They should not rewrite protected or already-shared history. + ## Analysis Tasks — "Here's what I found" Completed analysis with conclusions. These tasks produce reports without modifying code.