Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ kill <stale-pid>
rm -f ~/.nxd/nxd.lock ~/.nxd/events.jsonl ~/.nxd/nxd.db
```

## Current State (2026-06-24) — audit hardening

- **Planner degenerate-plan guard**: `Planner.plan` (`internal/engine/planner.go`) now rejects an empty story list and any story with an empty id/title *before* emitting REQ_PLANNED/STORY_CREATED, so a requirement can no longer be stranded with nothing to dispatch. Tests: `internal/engine/planner_validation_test.go`.
- **Conflict path quoting fix**: `git.ConflictedFiles` (`internal/git/conflict.go`) uses `git status --porcelain -z` and parses NUL-delimited records, returning raw paths instead of git's octal-escaped `core.quotepath` form. Filenames with spaces/non-ASCII (e.g. `résumé draft.txt`) now flow correctly into SniffBinary/StageFiles/conflict resolver. Test: `internal/git/conflict_quotepath_test.go`.
- **Post-merge integration fixer wired**: `runResume` (`internal/cli/resume.go`) now constructs `engine.NewTechLeadFixer` (Tech-Lead model/stage) and attaches it via `WithMonTechLeadFixer`. The feature + monitor logic existed but the setter was never called, so the post-merge build-validation stage never ran. Source-scan regression test: `internal/cli/resume_wiring_test.go`.

## Current State (2026-06-24)

- **Human-readable acceptance criteria**: `internal/criteria/format.go` (`Format`/`FormatMarkdown`, pure, fully unit-tested) segments a run-on acceptance-criteria blob into discrete items — sentence-splitting that guards abbreviations (`e.g.`) and identifier periods (`WorldState.copy()`) and strips `-`/`*`/`•`/`1.` markers. Surfaces: `nxd review <story>` prints a `Description:` block + a bulleted `Acceptance Criteria:` block; the web snapshot exposes `acceptance_criteria_items` (story_id → []string) and the dashboard expands a detail row (description + criteria checklist) when a story title is clicked (`storyDetailRow`/`toggleStoryDetail` in `app.js`). The Tech-Lead prompt (`prompts.go`) and planner tool schema (`planner_tools.go`) now require 3-6 intent-first criteria, one per line.
Expand Down
12 changes: 12 additions & 0 deletions internal/cli/resume.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,12 +454,24 @@ func runResume(cmd *cobra.Command, args []string) error {
conflictResolver = engine.NewConflictResolver(mergerClient, seniorModel.Model, seniorModel.MaxTokens, s.Events)
}

// Optional post-merge integration-build validation. After a wave merges, if
// main no longer compiles the Tech Lead diagnoses the cross-story break and
// records a focused fix suggestion. The feature was implemented and
// unit-tested but never wired here, so the stage never ran in production.
var techLeadFixer *engine.TechLeadFixer
if llmClient != nil {
techLead := s.Config.Models.TechLead
fixerClient := metrics.LabelStage(llmClient, "tech_lead")
techLeadFixer = engine.NewTechLeadFixer(fixerClient, techLead.Model, techLead.MaxTokens, s.Events, s.Proj)
}

monitor.Configure(
engine.WithMonMemPalace(mp),
engine.WithMonBayesianRouter(bayesianRouter),
engine.WithMonArtifactStore(artStore),
engine.WithMonCodeGraph(cg),
engine.WithMonConflictResolver(conflictResolver),
engine.WithMonTechLeadFixer(techLeadFixer),
engine.WithMonDryRun(dryRun),
// Auto-resume: when a wave completes, dispatch the next ready wave
// without waiting for the user to re-run "nxd resume".
Expand Down
27 changes: 27 additions & 0 deletions internal/cli/resume_wiring_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package cli

import (
"os"
"strings"
"testing"
)

// TestResume_WiresTechLeadFixer guards against a dead-wire regression: the
// post-merge integration-build feature (WithMonTechLeadFixer + TechLeadFixer)
// was fully implemented and unit-tested, but runResume never wired the fixer
// into the monitor, so the stage never ran in production. The option's own
// wiring test could not catch this. This test scans the resume source to
// confirm the fixer is actually constructed and attached.
func TestResume_WiresTechLeadFixer(t *testing.T) {
src, err := os.ReadFile("resume.go")
if err != nil {
t.Fatalf("read resume.go: %v", err)
}
code := string(src)

for _, want := range []string{"NewTechLeadFixer(", "WithMonTechLeadFixer("} {
if !strings.Contains(code, want) {
t.Errorf("resume.go must wire the post-merge integration fixer: missing %q", want)
}
}
}
20 changes: 20 additions & 0 deletions internal/engine/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,26 @@ architecture and conventions when planning stories.`, profileContext)
}
}

// Reject a degenerate plan before any event is emitted. An empty story
// list (the LLM returned `[]`) would otherwise emit REQ_PLANNED with no
// stories, stranding the requirement forever with nothing to dispatch.
if len(stories) == 0 {
return PlanResult{}, fmt.Errorf("tech lead returned zero stories — requirement cannot be planned")
}

// Reject any story missing an id or a title. A blank story object (`{}`)
// from a small model carries no work; auto-assigning it an ID below would
// dispatch an agent against nothing. Validate before the auto-ID fill so an
// empty id is caught rather than masked.
for i, s := range stories {
if strings.TrimSpace(s.ID) == "" {
return PlanResult{}, fmt.Errorf("story %d has an empty id", i)
}
if strings.TrimSpace(s.Title) == "" {
return PlanResult{}, fmt.Errorf("story %s has an empty title", s.ID)
}
}

// Ensure all stories have IDs. Smaller models sometimes omit them.
for i, s := range stories {
if s.ID == "" {
Expand Down
78 changes: 78 additions & 0 deletions internal/engine/planner_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package engine_test

import (
"context"
"os"
"path/filepath"
"testing"

"github.com/tzone85/nexus-dispatch/internal/config"
"github.com/tzone85/nexus-dispatch/internal/engine"
"github.com/tzone85/nexus-dispatch/internal/llm"
"github.com/tzone85/nexus-dispatch/internal/state"
)

func newPlannerStores(t *testing.T, dir string) (state.EventStore, state.ProjectionStore) {
t.Helper()
_ = os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module test"), 0644)
eventStore, err := state.NewFileStore(filepath.Join(dir, "events.jsonl"))
if err != nil {
t.Fatalf("create event store: %v", err)
}
t.Cleanup(func() { _ = eventStore.Close() })
projStore, err := state.NewSQLiteStore(":memory:")
if err != nil {
t.Fatalf("create proj store: %v", err)
}
t.Cleanup(func() { _ = projStore.Close() })
return eventStore, projStore
}

// TestPlan_RejectsEmptyStoryList pins the fix for the silent "0 stories" plan:
// when the Tech-Lead LLM returns an empty array, Plan must error instead of
// emitting REQ_PLANNED with no stories (which strands the requirement forever).
func TestPlan_RejectsEmptyStoryList(t *testing.T) {
dir := t.TempDir()
eventStore, projStore := newPlannerStores(t, dir)

client := llm.NewReplayClient(llm.CompletionResponse{Content: "[]"})
cfg := config.DefaultConfig()
planner := engine.NewPlanner(client, cfg, eventStore, projStore)

_, err := planner.Plan(context.Background(), "r-empty", "Build nothing", dir)
if err == nil {
t.Fatal("expected error when the planner returns zero stories")
}

// No REQ_PLANNED event must have been emitted for a failed plan.
events, lerr := eventStore.List(state.EventFilter{})
if lerr != nil {
t.Fatalf("list events: %v", lerr)
}
for _, e := range events {
if e.Type == state.EventReqPlanned {
t.Errorf("REQ_PLANNED should not be emitted when planning yields 0 stories")
}
}
}

// TestPlan_RejectsStoryWithEmptyID pins per-story boundary validation: an empty
// or fieldless story object from the LLM must be rejected, not dispatched
// against nothing.
func TestPlan_RejectsStoryWithEmptyID(t *testing.T) {
dir := t.TempDir()
eventStore, projStore := newPlannerStores(t, dir)

// One well-formed story and one empty object.
response := `[
{"id": "s-001", "title": "Real", "description": "d", "acceptance_criteria": "ac", "complexity": 2, "owned_files": ["a.go"]},
{}
]`
client := llm.NewReplayClient(llm.CompletionResponse{Content: response})
cfg := config.DefaultConfig()
planner := engine.NewPlanner(client, cfg, eventStore, projStore)

if _, err := planner.Plan(context.Background(), "r-emptyid", "Has a blank story", dir); err == nil {
t.Fatal("expected error for a story with an empty id/title")
}
}
19 changes: 13 additions & 6 deletions internal/git/conflict.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,34 @@ func StartRebase(worktreePath, upstream string) error {
}

// ConflictedFiles returns the list of files with unresolved merge conflicts
// in the given worktree. It uses `git status --porcelain` which reliably
// in the given worktree. It uses `git status --porcelain -z` which reliably
// detects all unmerged states (UU, AA, DD, AU, UA, DU, UD), unlike
// `git diff --diff-filter=U` which can miss some conflict types.
//
// The `-z` flag emits NUL-terminated records with raw (unquoted) paths.
// Default porcelain output quotes and octal-escapes paths containing spaces
// or non-ASCII bytes (e.g. `"r\303\251sum\303\251 draft.txt"`), which would
// flow as a bogus path into SniffBinary/StageFiles and break the conflict
// resolver. Each record is "XY<space>path"; the path is rec[3:] verbatim —
// no TrimSpace, since a real path may legitimately contain trailing spaces.
func ConflictedFiles(worktreePath string) ([]string, error) {
cmd := exec.Command("git", "status", "--porcelain")
cmd := exec.Command("git", "status", "--porcelain", "-z")
cmd.Dir = worktreePath
out, err := cmd.CombinedOutput()
if err != nil {
return nil, fmt.Errorf("git status: %w", err)
}

var files []string
for _, line := range strings.Split(string(out), "\n") {
if len(line) < 4 {
for _, rec := range strings.Split(string(out), "\x00") {
if len(rec) < 4 {
continue
}
// Unmerged status codes: UU, AA, DD, AU, UA, DU, UD
xy := line[:2]
xy := rec[:2]
if xy == "UU" || xy == "AA" || xy == "DD" ||
xy == "AU" || xy == "UA" || xy == "DU" || xy == "UD" {
files = append(files, strings.TrimSpace(line[3:]))
files = append(files, rec[3:])
}
}
return files, nil
Expand Down
64 changes: 64 additions & 0 deletions internal/git/conflict_quotepath_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package git

import (
"os"
"os/exec"
"path/filepath"
"testing"
)

// TestConflictedFiles_NonASCIIPath verifies that a conflict on a filename with a
// space and a non-ASCII character is returned as its real path — not git's
// default core.quotepath-escaped form (e.g. `"r\303\251sum\303\251 a.txt"`).
// Without the fix, the escaped string flows into SniffBinary/StageFiles and the
// Tech-Lead conflict resolver silently fails.
func TestConflictedFiles_NonASCIIPath(t *testing.T) {
dir := t.TempDir()
helperRun(t, dir, "git", "init")
helperRun(t, dir, "git", "config", "user.email", "test@test.com")
helperRun(t, dir, "git", "config", "user.name", "Test")
// Intentionally leave core.quotepath at its default (true).

const name = "résumé draft.txt"
write := func(content string) {
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil {
t.Fatalf("write: %v", err)
}
}

write("initial\n")
helperRun(t, dir, "git", "add", ".")
helperRun(t, dir, "git", "commit", "-m", "init")
mainBranch := helperRun(t, dir, "git", "rev-parse", "--abbrev-ref", "HEAD")

helperRun(t, dir, "git", "checkout", "-b", "topic")
write("topic change\n")
helperRun(t, dir, "git", "add", ".")
helperRun(t, dir, "git", "commit", "-m", "topic")

helperRun(t, dir, "git", "checkout", mainBranch)
write("main change\n")
helperRun(t, dir, "git", "add", ".")
helperRun(t, dir, "git", "commit", "-m", "main")

helperRun(t, dir, "git", "checkout", "topic")
// Trigger the conflict (rebase exits non-zero — that's expected).
rebase := exec.Command("git", "rebase", mainBranch)
rebase.Dir = dir
_, _ = rebase.CombinedOutput()

files, err := ConflictedFiles(dir)
if err != nil {
t.Fatalf("ConflictedFiles: %v", err)
}
if len(files) != 1 {
t.Fatalf("expected exactly 1 conflicted file, got %d: %#v", len(files), files)
}
if files[0] != name {
t.Errorf("ConflictedFiles returned %q, want the real path %q", files[0], name)
}
// The returned path must resolve to a real file on disk.
if _, statErr := os.Stat(filepath.Join(dir, files[0])); statErr != nil {
t.Errorf("returned path does not resolve to a file: %v", statErr)
}
}
Loading