diff --git a/cmd/made/contracts_red_test.go b/cmd/made/contracts_red_test.go new file mode 100644 index 0000000..f100089 --- /dev/null +++ b/cmd/made/contracts_red_test.go @@ -0,0 +1,89 @@ +package main + +import ( + "bytes" + "encoding/json" + "os" + "strings" + "testing" + + "github.com/douglasjarquin/made/internal/daemon" +) + +func TestCapabilitiesJSONExposesStructuredRunContract(t *testing.T) { + var stdout, stderr bytes.Buffer + stdoutFile := tempOutputFile(t) + stderrFile := tempOutputFile(t) + code := run([]string{"capabilities", "--json"}, stdoutFile, stderrFile) + if code != 0 { + t.Fatalf("capabilities exit code = %d; stderr=%s", code, readOutputFile(t, stderrFile)) + } + var payload struct { + SchemaVersion int `json:"schema_version"` + ProtocolVersion int `json:"protocol_version"` + Commands []string `json:"commands"` + } + if err := json.Unmarshal(readOutputFile(t, stdoutFile), &payload); err != nil { + t.Fatalf("capabilities output is not JSON: %v", err) + } + if payload.SchemaVersion == 0 || payload.ProtocolVersion == 0 { + t.Fatalf("capabilities versions missing: %+v", payload) + } + for _, want := range []string{"run.submit", "run.status", "run.list", "run.cancel", "review.decide", "doctor"} { + found := false + for _, got := range payload.Commands { + if got == want { + found = true + } + } + if !found { + t.Fatalf("capabilities missing command %q: %+v", want, payload.Commands) + } + } + _ = stdout + _ = stderr +} + +func TestObsoleteStatusCommandIsRejected(t *testing.T) { + stdoutFile := tempOutputFile(t) + stderrFile := tempOutputFile(t) + code := run([]string{"status", "--json"}, stdoutFile, stderrFile) + if code != 2 { + t.Fatalf("obsolete status exit code = %d, want 2; stderr=%s", code, readOutputFile(t, stderrFile)) + } +} + +func TestStatusJSONReportsCurrentStageFromOrderedState(t *testing.T) { + report := newStatusReport(daemon.RunSnapshot{ + ID: "run-current-stage", + Stages: []daemon.StageResult{{Name: "intent", Result: "pass"}, {Name: "review", Result: "pending"}}, + }) + data, err := json.Marshal(report) + if err != nil { + t.Fatalf("marshal status: %v", err) + } + if !strings.Contains(string(data), `"current_stage":"review"`) { + t.Fatalf("status omitted current stage: %s", data) + } +} + +func tempOutputFile(t *testing.T) *os.File { + t.Helper() + file, err := os.CreateTemp(t.TempDir(), "output") + if err != nil { + t.Fatalf("CreateTemp: %v", err) + } + return file +} + +func readOutputFile(t *testing.T, file *os.File) []byte { + t.Helper() + if _, err := file.Seek(0, 0); err != nil { + t.Fatalf("seek output: %v", err) + } + data, err := os.ReadFile(file.Name()) + if err != nil { + t.Fatalf("read output: %v", err) + } + return data +} diff --git a/cmd/made/daemon.go b/cmd/made/daemon.go index cd55668..f6b61b8 100644 --- a/cmd/made/daemon.go +++ b/cmd/made/daemon.go @@ -128,11 +128,11 @@ func startDaemon(ctx context.Context, home, lockPath string, idle time.Duration, done <- err return rm, done } - reviewStore := daemon.NewReviewDecisions() + reviewStore := daemon.NewReviewDecisionsForManager(rm) admission := &sync.Mutex{} runCtx, cancelRun := context.WithCancel(ctx) srv := api.NewServer(socketPath) - registerDaemonHandlers(srv, rm, reviewStore, spool, cancelRun, admission) + registerDaemonHandlers(srv, rm, reviewStore, spool, cancelRun, home, admission) done := make(chan error, 1) @@ -260,14 +260,14 @@ func isTerminalRunStatus(s daemon.RunStatus) bool { const debugHandlersEnv = "MADE_DEBUG_HANDLERS" -func registerDaemonHandlers(srv *api.Server, rm *daemon.RunManager, store *daemon.ReviewDecisions, spool *daemon.GateSpool, cancel context.CancelFunc, admission ...*sync.Mutex) { +func registerDaemonHandlers(srv *api.Server, rm *daemon.RunManager, store *daemon.ReviewDecisions, spool *daemon.GateSpool, cancel context.CancelFunc, home string, admission ...*sync.Mutex) { srv.Handle("run.status", runStatusHandler(rm)) srv.Handle("run.submit", runSubmitHandler(rm, store, spool, admission...)) srv.Handle("run.list", runListHandler(rm)) srv.Handle("run.cancel", runCancelHandler(rm)) srv.Handle("review.decide", reviewDecideRunHandler(rm, store)) srv.Handle("daemon.shutdown", daemonShutdownHandler(rm, spool, cancel, admission...)) - srv.Handle("gate.admitPush", gateAdmitPushHandler()) + srv.Handle("gate.admitPush", gateAdmitPushHandler(home)) srv.Handle("gate.notifyPush", gateNotifyPushHandler(rm, store, spool, admission...)) if os.Getenv(debugHandlersEnv) == "1" { srv.Handle("debug.submitCancellableRun", debugSubmitCancellableRunHandler(rm)) @@ -290,7 +290,7 @@ type gateAdmitPushResult struct { // daemon recognizes" - a real, valid bare repo on disk. It deliberately does // not touch RunManager; creating a run is the orchestrator's job, not // admission's. -func gateAdmitPushHandler() api.HandlerFunc { +func gateAdmitPushHandler(home string) api.HandlerFunc { return func(_ context.Context, params json.RawMessage) (any, error) { var p gateAdmitPushParams if err := decodeStrictParams(params, &p); err != nil { @@ -299,6 +299,9 @@ func gateAdmitPushHandler() api.HandlerFunc { if p.GatePath == "" { return nil, fmt.Errorf("gate.admitPush: gate_path is required") } + if err := validateManagedGatePath(home, p.GatePath); err != nil { + return nil, fmt.Errorf("gate.admitPush: %w", err) + } if err := validateBareGateRepo(p.GatePath); err != nil { return nil, fmt.Errorf("gate.admitPush: %w", err) } @@ -306,6 +309,32 @@ func gateAdmitPushHandler() api.HandlerFunc { } } +func validateManagedGatePath(home, gatePath string) error { + homeResolved, err := filepath.EvalSymlinks(home) + if err != nil { + return fmt.Errorf("resolve Made home: %w", err) + } + gateResolved, err := filepath.EvalSymlinks(gatePath) + if err != nil { + return fmt.Errorf("resolve gate path: %w", err) + } + rel, err := filepath.Rel(homeResolved, gateResolved) + if err != nil { + return fmt.Errorf("relate gate to Made home: %w", err) + } + parts := strings.Split(rel, string(filepath.Separator)) + if len(parts) != 3 || parts[0] != "gates" || parts[2] != "gate.git" { + return fmt.Errorf("gate path must be a managed MADE_HOME/gates//gate.git path") + } + if len(parts[1]) != 64 || !hexString(parts[1]) { + return fmt.Errorf("gate path hash is not lowercase hexadecimal") + } + if filepath.Clean(gateResolved) != filepath.Clean(filepath.Join(homeResolved, rel)) { + return fmt.Errorf("gate path must not contain symlinks") + } + return nil +} + func validateBareGateRepo(path string) error { info, err := os.Stat(path) if err != nil { @@ -337,13 +366,14 @@ func validateBareGateRepo(path string) error { const gateNotifyPushDefaultBranchTimeout = 10 * time.Second type gateNotifyPushParams struct { - GatePath string `json:"gate_path"` - OldSHA string `json:"old_sha"` - NewSHA string `json:"new_sha"` - Ref string `json:"ref"` - RunID string `json:"run_id,omitempty"` - OutputSHA string `json:"output_sha,omitempty"` - Replay bool `json:"replay,omitempty"` + GatePath string `json:"gate_path"` + OldSHA string `json:"old_sha"` + NewSHA string `json:"new_sha"` + Ref string `json:"ref"` + RunID string `json:"run_id,omitempty"` + OutputSHA string `json:"output_sha,omitempty"` + SubmissionID string `json:"submission_id,omitempty"` + Replay bool `json:"replay,omitempty"` } type gateNotifyPushResult struct { @@ -453,7 +483,14 @@ func gateNotifyPushHandler(rm *daemon.RunManager, reviewDecisions *daemon.Review orchestrator.NewWorkFunc(rm, reviewDecisions, emit, runID, defaultBranch, branch, orchestrator.Options{})) } - snapshot, err := rm.SubmitWithMetadata(runID, repo, branch, p.NewSHA, p.OutputSHA, work) + submissionID := p.SubmissionID + if submissionID == "" { + submissionID = p.Ref + "@" + p.NewSHA + } + snapshot, err := rm.SubmitSubmission(daemon.RunSubmission{ + ID: runID, Repo: repo, Branch: branch, Ref: p.Ref, OldSHA: p.OldSHA, + InputSHA: p.NewSHA, OutputSHA: p.OutputSHA, SubmissionID: submissionID, GatePath: p.GatePath, + }, work) if err != nil { return nil, fmt.Errorf("gate.notifyPush: submit run: %w", err) } diff --git a/cmd/made/daemon_test.go b/cmd/made/daemon_test.go index 75d5056..076876b 100644 --- a/cmd/made/daemon_test.go +++ b/cmd/made/daemon_test.go @@ -57,6 +57,9 @@ func TestDaemonStop_CancelsInFlightRunBeforeProcessExits(t *testing.T) { } for scanner.Scan() { } + if err := scanner.Err(); err != nil { + return + } }() select { @@ -70,7 +73,7 @@ func TestDaemonStop_CancelsInFlightRunBeforeProcessExits(t *testing.T) { socketPath := api.SocketPath(home) var client *api.Client - for i := 0; i < 200; i++ { + for range 200 { client, err = api.Dial(socketPath) if err == nil { break diff --git a/cmd/made/gate_admit_push_test.go b/cmd/made/gate_admit_push_test.go index 5fe6b44..bbe3620 100644 --- a/cmd/made/gate_admit_push_test.go +++ b/cmd/made/gate_admit_push_test.go @@ -48,7 +48,7 @@ func TestGateAdmitPushRPC_ValidBareRepoAdmitted(t *testing.T) { home := shortTempDir(t) _, client := startTestDaemon(t, home) - barePath := filepath.Join(shortTempDir(t), "gate.git") + barePath := gitgate.GatePath(home, "fixture/repo") if err := gitgate.InitBare(barePath); err != nil { t.Fatalf("InitBare: %v", err) } @@ -58,6 +58,20 @@ func TestGateAdmitPushRPC_ValidBareRepoAdmitted(t *testing.T) { } } +func TestGateAdmitPushRPC_RejectsBareRepoOutsideMadeHome(t *testing.T) { + home := shortTempDir(t) + _, client := startTestDaemon(t, home) + + barePath := filepath.Join(shortTempDir(t), "unmanaged.git") + if err := gitgate.InitBare(barePath); err != nil { + t.Fatalf("InitBare: %v", err) + } + + if _, err := client.Call("gate.admitPush", gateAdmitPushParams{GatePath: barePath}); err == nil { + t.Fatal("gate.admitPush accepted a bare repository outside MADE_HOME") + } +} + func TestGateAdmitPushRPC_InvalidPathRejected(t *testing.T) { home := shortTempDir(t) _, client := startTestDaemon(t, home) @@ -84,7 +98,7 @@ func TestGateAdmitPushCLI_ValidGateExitsZero(t *testing.T) { t.Setenv("MADE_HOME", home) _, _ = startTestDaemon(t, home) - barePath := filepath.Join(shortTempDir(t), "gate.git") + barePath := gitgate.GatePath(home, "fixture/repo") if err := gitgate.InitBare(barePath); err != nil { t.Fatalf("InitBare: %v", err) } diff --git a/cmd/made/main.go b/cmd/made/main.go index 0f12324..e757fbf 100644 --- a/cmd/made/main.go +++ b/cmd/made/main.go @@ -20,6 +20,9 @@ func run(args []string, stdout, stderr *os.File) int { return runCapabilitiesCommand(args[1:], stdout, stderr) case "run": return runRunCommand(args[1:], stdout, stderr) + case "status": + _, _ = fmt.Fprintln(stderr, "made: status is obsolete; use made run status --json ") + return 2 case "daemon": return runDaemonCommand(args[1:], stdout, stderr) case "review": diff --git a/cmd/made/review.go b/cmd/made/review.go index efdd994..e3c4de8 100644 --- a/cmd/made/review.go +++ b/cmd/made/review.go @@ -45,10 +45,9 @@ func reviewDecideRunHandler(rm *daemon.RunManager, store *daemon.ReviewDecisions if _, ok := rm.Snapshot(p.RunID); !ok { return nil, fmt.Errorf("review.decide: exact run_id %q was not found", p.RunID) } - if err := rm.SetDecision(p.RunID, p.Stage, p.Decision); err != nil { + if err := store.Set(p.RunID, p.Stage, p.Decision); err != nil { return nil, err } - store.Set(p.RunID, p.Stage, p.Decision) return reviewDecisionReport{ SchemaVersion: 1, ProtocolVersion: api.Version, RunID: p.RunID, Stage: p.Stage, Decision: p.Decision, diff --git a/cmd/made/run_contract_test.go b/cmd/made/run_contract_test.go new file mode 100644 index 0000000..8d4bf81 --- /dev/null +++ b/cmd/made/run_contract_test.go @@ -0,0 +1,38 @@ +package main + +import ( + "os" + "testing" +) + +func TestRunStatusRejectsUnsupportedTrailingArgument(t *testing.T) { + stdout, stderr := discardOutput(t) + if code := runExactStatusCommand([]string{"run-1", "unexpected"}, stdout, stderr); code != 2 { + t.Fatalf("run status exit code = %d, want 2", code) + } +} + +func TestRunCancelRejectsUnsupportedTrailingArgument(t *testing.T) { + stdout, stderr := discardOutput(t) + if code := runCancelCommand([]string{"run-1", "unexpected"}, stdout, stderr); code != 2 { + t.Fatalf("run cancel exit code = %d, want 2", code) + } +} + +func discardOutput(t *testing.T) (stdout, stderr *os.File) { + t.Helper() + stdout, err := os.Open(os.DevNull) + if err != nil { + t.Fatalf("open stdout discard: %v", err) + } + stderr, err = os.Open(os.DevNull) + if err != nil { + _ = stdout.Close() + t.Fatalf("open stderr discard: %v", err) + } + t.Cleanup(func() { + _ = stdout.Close() + _ = stderr.Close() + }) + return stdout, stderr +} diff --git a/cmd/made/status.go b/cmd/made/status.go index 15ac67a..80fca57 100644 --- a/cmd/made/status.go +++ b/cmd/made/status.go @@ -11,7 +11,7 @@ import ( "github.com/douglasjarquin/made/internal/evidence" ) -const statusSchemaVersion = 1 +const statusSchemaVersion = 2 const ( StageResultPass = "pass" @@ -37,10 +37,15 @@ type StatusReport struct { RunID string `json:"run_id"` Repo string `json:"repo"` Branch string `json:"branch"` + Ref string `json:"ref,omitempty"` + OldSHA string `json:"old_sha,omitempty"` State string `json:"state"` + SubmissionID string `json:"submission_id,omitempty"` + GatePath string `json:"gate_path,omitempty"` InputSHA string `json:"input_sha"` OutputSHA string `json:"output_sha"` ExecutionFinished bool `json:"execution_finished"` + CurrentStage string `json:"current_stage,omitempty"` Findings []daemon.RunFinding `json:"findings"` Decisions map[string]string `json:"decisions"` PRURL string `json:"pr_url"` @@ -52,8 +57,10 @@ type StatusReport struct { StartedAt *time.Time `json:"started_at,omitempty"` EndedAt *time.Time `json:"ended_at,omitempty"` Error string `json:"error,omitempty"` + Message string `json:"message,omitempty"` Stages []StageResult `json:"stages"` PendingFindings []AskUserFinding `json:"pending_findings"` + EvidenceRefs []string `json:"evidence_refs"` } type StageResult = daemon.StageResult @@ -85,12 +92,17 @@ func statusHandler(rm *daemon.RunManager) api.HandlerFunc { } func newStatusReport(snap daemon.RunSnapshot) StatusReport { - stages := snap.Stages - if len(stages) == 0 { - stages = make([]StageResult, len(pipelineStages)) - for i, name := range pipelineStages { - stages[i] = StageResult{Name: name, Result: StageResultPending} + byName := make(map[string]StageResult, len(snap.Stages)) + for _, stage := range snap.Stages { + byName[stage.Name] = stage + } + stages := make([]StageResult, len(pipelineStages)) + for i, name := range pipelineStages { + stage, ok := byName[name] + if !ok { + stage = StageResult{Name: name, Result: StageResultPending} } + stages[i] = stage } pendingFindings := snap.PendingFindings @@ -109,6 +121,30 @@ func newStatusReport(snap daemon.RunSnapshot) StatusReport { if snap.Err != nil { errMsg = evidence.RedactString(snap.Err.Error()) } + if snap.Error != "" { + errMsg = evidence.RedactString(snap.Error) + } + currentStage := snap.CurrentStage + if currentStage == "" { + for _, stage := range snap.Stages { + if stage.Result != StageResultPass { + currentStage = stage.Name + break + } + } + } + if currentStage == "" { + for _, stage := range stages { + if stage.Result != StageResultPass { + currentStage = stage.Name + break + } + } + } + evidenceRefs := append([]string(nil), snap.EvidenceRefs...) + if evidenceRefs == nil { + evidenceRefs = []string{} + } return StatusReport{ SchemaVersion: statusSchemaVersion, @@ -116,10 +152,15 @@ func newStatusReport(snap daemon.RunSnapshot) StatusReport { RunID: snap.ID, Repo: snap.Repo, Branch: snap.Branch, + Ref: snap.Ref, + OldSHA: snap.OldSHA, State: string(snap.Status), + SubmissionID: snap.SubmissionID, + GatePath: snap.GatePath, InputSHA: snap.InputSHA, OutputSHA: snap.OutputSHA, ExecutionFinished: snap.ExecutionFinished, + CurrentStage: currentStage, Findings: redactedFindings(snap.Findings), Decisions: nonNilDecisions(snap.Decisions), PRURL: evidence.RedactString(snap.PRURL), @@ -131,8 +172,10 @@ func newStatusReport(snap daemon.RunSnapshot) StatusReport { StartedAt: timePtr(snap.StartedAt), EndedAt: timePtr(snap.EndedAt), Error: errMsg, + Message: evidence.RedactString(snap.Message), Stages: stages, PendingFindings: pendingFindings, + EvidenceRefs: evidenceRefs, } } diff --git a/cmd/made/status_test.go b/cmd/made/status_test.go index 9bde657..e96047c 100644 --- a/cmd/made/status_test.go +++ b/cmd/made/status_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -216,14 +217,23 @@ func TestStatusJSON_ReflectsRealStageUpdate(t *testing.T) { t.Fatalf("output is not valid JSON: %v\noutput: %s", err, out) } - if len(report.Stages) != len(wantStages) { - t.Fatalf("Stages = %+v, want %+v", report.Stages, wantStages) + if len(report.Stages) != len(pipelineStages) { + t.Fatalf("Stages = %+v, want %d ordered stages", report.Stages, len(pipelineStages)) } for i, want := range wantStages { - if report.Stages[i] != StageResult(want) { + if !reflect.DeepEqual(report.Stages[i], StageResult(want)) { t.Errorf("Stages[%d] = %+v, want %+v", i, report.Stages[i], want) } } + for i := len(wantStages); i < len(pipelineStages); i++ { + want := StageResult{Name: pipelineStages[i], Result: StageResultPending} + if !reflect.DeepEqual(report.Stages[i], want) { + t.Errorf("Stages[%d] = %+v, want %+v", i, report.Stages[i], want) + } + } + if report.CurrentStage != "review" { + t.Fatalf("CurrentStage = %q, want review", report.CurrentStage) + } if len(report.PendingFindings) != len(wantFindings) { t.Fatalf("PendingFindings = %+v, want %+v", report.PendingFindings, wantFindings) diff --git a/evidence/phase-0-grounding-made-remediation-continuation.md b/evidence/phase-0-grounding-made-remediation-continuation.md new file mode 100644 index 0000000..78a7e3b --- /dev/null +++ b/evidence/phase-0-grounding-made-remediation-continuation.md @@ -0,0 +1,131 @@ +# Phase 0 — Made remediation continuation grounding and custody + +Date: 2026-08-17 + +Scope: Made worktree only. + +The retained prior worktree was not opened, reused, cleaned, reset, deleted, copied, or inspected for untracked artifact contents. + +## Exact task worktree and base + +Command: `pwd -P` + +Exit: `0` + +Output: `/Users/douglasjarquin/.herdr/worktrees/made/cs-made-remediation-continuation` + +Command: `git rev-parse --show-toplevel` + +Exit: `0` + +Output: `/Users/douglasjarquin/.herdr/worktrees/made/cs-made-remediation-continuation` + +Command: `git branch --show-current` + +Exit: `0` + +Output: `cs/made-remediation-continuation` + +Command: `git rev-parse HEAD` + +Exit: `0` + +Output: `3e19ed9d598a68149da5a73949533e8095ca4403` + +Command: `git rev-parse --verify 3e19ed9d598a68149da5a73949533e8095ca4403^{commit}` + +Exit: `0` + +Output: `3e19ed9d598a68149da5a73949533e8095ca4403` + +The pre-artifact baseline command `git status --short --branch` exited `0` and printed only `## cs/made-remediation-continuation`. + +The task worktree therefore launched clean at the exact requested base and branch. + +## Prior worktree preservation + +Command: `test -d /Users/douglasjarquin/.herdr/worktrees/made/cs-made-remediation-p1p3b` + +Observed: the retained path exists. + +No command entered the retained worktree or read its Git state or artifact contents. + +The retained worktree was left in place and was not opened, reused, cleaned, reset, deleted, copied, or inspected. + +## Installed Made binary and live shared daemon + +Command: `go version -m /Users/douglasjarquin/.local/bin/made` + +Exit: `0` + +Relevant output: `vcs.revision=34d44be504291482d973c65bd427ba964df5e0e9` and `vcs.modified=false`. + +Command: `shasum -a 256 /Users/douglasjarquin/.local/bin/made` + +Output: `2ad968ed6f1dccb95c8eff90e045553f347ca2771d8278a28db3ea1fe5d4a8f7 /Users/douglasjarquin/.local/bin/made` + +The installed binary is ahead of this task base and is not used as proof of task-source behavior. + +Command: `made daemon status` + +Exit: `0` + +Output: `made daemon: not running` + +The shared Made daemon was not started, stopped, restarted, or updated. + +## Required tools + +The environment gate `test "${HERDR_ENV:-}" = 1` exited `0`. + +Installed commands: `made=/Users/douglasjarquin/.local/bin/made`, `gh-axi=/Users/douglasjarquin/.local/bin/gh-axi`, `herdr=/etc/profiles/per-user/douglasjarquin/bin/herdr`, `codex=/opt/homebrew/bin/codex`, `golangci-lint=/Users/douglasjarquin/go/bin/golangci-lint`, `shellcheck=/etc/profiles/per-user/douglasjarquin/bin/shellcheck`, and `make=/usr/bin/make`. + +Observed versions: `go version go1.26.6 darwin/arm64`, `git version 2.55.0`, `codex-cli 0.147.0`, and `golangci-lint has version 2.11.2`. + +`gh-axi --help` and `herdr --help` both exited `0`. + +`made --version`, `made version`, and `made --help` each exited `2` with `made: unknown command`, so the source command surface is authoritative. + +## Plan and brief custody + +Command: `git hash-object plans/made-rewrite.md` + +Exit: `0` + +Output: `2d10f32eba404b3f2e54d3ef7d853b96f8eb77fd` + +Command: `shasum -a 256 /Users/douglasjarquin/.consigliere/capos/made/data/made-remediation-continuation/brief.md` + +Output: `bc3adb10fb9f77ad34e5a5d89d942b81efa3ddf3fe9d0b3b65ed188ad0823f6d /Users/douglasjarquin/.consigliere/capos/made/data/made-remediation-continuation/brief.md` + +The current Capo brief was reread from that path before advancing. + +Its binding continuation gates are public structured contract, lifecycle and durability, evidence, semantic config, strict external compatibility, disposable live scenarios, and final validation. + +The brief forbids real-project validation, gate initialization, run submission, shared Made daemon lifecycle changes, default-branch pushes, merges, auto-merge, remote-branch deletion, and ask-user decisions. + +## Herdr lab isolation + +The helper was set to `/Users/douglasjarquin/.consigliere/capos/made/bin/cs-herdr-lab.sh`. + +The generated non-default session is `cs-lab-made-remediation-9714-1438`. + +The required EXIT trap was installed before provisioning. + +Provisioning was performed only with `"$HERDR_LAB_HELPER" provision "$HERDR_LAB_SESSION"`. + +The helper-run command `"$HERDR_LAB_HELPER" run "$HERDR_LAB_SESSION" status server` exited `0` and observed `status: running`, `version: 0.8.0`, `protocol: 20`, and `compatible: yes` for the named lab session. + +The shared `default` session was not targeted. + +## Current artifact state + +The current status contains only the session journal and phase evidence created by this task: + +```text +?? .debug-journal.md +?? evidence/ulw-notepad-made-remediation-continuation.md +?? evidence/phase-0-grounding-made-remediation-continuation.md +``` + +These are task-owned artifacts and will be reconciled before delivery. diff --git a/evidence/phase-1-contract-matrix.md b/evidence/phase-1-contract-matrix.md new file mode 100644 index 0000000..b7d9bcc --- /dev/null +++ b/evidence/phase-1-contract-matrix.md @@ -0,0 +1,227 @@ +# Phase 1 — RED contract matrix + +Base under test: `3e19ed9d598a68149da5a73949533e8095ca4403`. + +No production source has been edited for this phase. + +## Baseline observations + +Command: `go test ./...` + +Exit: non-zero. + +Observed symptom: disposable Git commits failed before contract execution because inherited `SSH_AUTH_SOCK` pointed at an unavailable 1Password socket. + +Masking condition: the repository's test helpers inherit the host Git signing configuration. + +The failure is environmental, not a Made contract result. + +Command: `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./...` + +Exit: non-zero. + +Observed symptom: all packages passed except `internal/pipeline/rebase`, where `TestRun_CleanRebaseProceeds` returned `OK:false` with `rebase onto main halted due to conflicts in: `. + +This pre-existing Made-only failure is tracked separately from the continuation matrix and must be fixed or explicitly evidenced before final validation. + +## Public structured external contract + +### GitHub checks + +Trigger: `internal/pipeline/ci/ci.go:61` calls `github.Client.MergeableState`, which invokes `gh pr view --json mergeStateStatus`. + +Masking condition: `internal/github/testdata/fakegh/main.go:57-64` returns `{"mergeStateStatus":"CLEAN"}` and ignores flags. + +Visible symptom: real required checks can be pending or failed while mergeability is `CLEAN`, and Made reports CI success without inspecting checks. + +RED test: `go test ./internal/pipeline/ci -run TestRun_UsesPrChecksJSON -count=1`. + +RED assertion: strict fake reports the invocation is `gh pr checks --json name,state,bucket,link`, not `gh pr view ... mergeStateStatus`. + +### Workflow run identity + +Trigger: `ci.Run` passes the PR URL into `CheckLogs` and `RerunCheck`, which invoke `gh run view` and `gh run rerun`. + +Masking condition: the fake accepts any identifier. + +Visible symptom: real `gh` rejects a PR URL where a workflow run ID is required, so logs and reruns fail or are silently omitted. + +RED test: `go test ./internal/pipeline/ci -run TestRun_PassesWorkflowRunIDToLogsAndRerun -count=1`. + +RED assertion: strict fake rejects PR URLs and records the exact numeric workflow run ID extracted from the supported checks payload. + +### Authentication and check failures + +Trigger: `github.Client.run` maps all non-zero commands to generic errors and CI converts client errors into a normal failed `Result`. + +Masking condition: tests exercise only successful auth and scripted check state. + +Visible symptom: authentication failure is not distinguishable from an ordinary failing check at the public boundary. + +RED test: `go test ./internal/github -run TestAuthStatusFailureIsExplicit -count=1`. + +RED assertion: auth failure returns the typed/auth-specific error and CI does not claim a normal check result. + +## Agent structured contract + +### Codex invocation + +Trigger: `internal/agent/spawn.go:26-30` invokes every agent as ` review --worktree `. + +Masking condition: `internal/agent/testdata/fakeagent/main.go` ignores all arguments. + +Visible symptom: installed Codex supports `codex exec --json --output-schema --ephemeral -C `, while the current adapter sends an undocumented `review --worktree` shape. + +RED test: `go test ./internal/agent -run TestSpawn_CodexUsesStructuredExecContract -count=1`. + +RED assertion: strict fake requires `exec --json --output-schema --ephemeral -C ` and rejects the current `review --worktree` invocation. + +### Codex output + +Trigger: `Spawn` unmarshals all stdout directly into `agent.Findings`. + +Masking condition: fake emits a single raw JSON object with no JSONL event framing or schema check. + +Visible symptom: malformed, non-final, or schema-invalid output can be mistaken for a valid review or produce an opaque parse error. + +RED test: `go test ./internal/agent -run TestSpawn_RejectsInvalidStructuredOutput -count=1`. + +RED assertion: output is accepted only when the final structured result matches the schema and invalid/missing fields fail closed with stdout/stderr evidence. + +### Claude support boundary + +Trigger: the same invocation path is used for Claude and Codex without a verified machine-readable Claude contract. + +Masking condition: permissive fake treats both agent kinds identically. + +Visible symptom: real Claude can enter an interactive or human-output mode that cannot be safely parsed. + +RED test: `go test ./internal/agent -run TestSpawn_ClaudeUnsupportedContractIsExplicit -count=1`. + +RED assertion: unsupported Claude invocation returns an explicit contract error rather than using a generic compatibility shim. + +## Lifecycle and durability + +### Run persistence and restart recovery + +Trigger: `internal/daemon.RunManager` and `ReviewDecisions` store state only in memory. + +Masking condition: daemon remains alive for the full run. + +Visible symptom: status, stage results, pending findings, decisions, and awaiting-merge state disappear after daemon restart. + +RED test: `go test ./internal/daemon -run TestRunManager_RestoresDurableSnapshotAfterRestart -count=1`. + +RED assertion: a second manager opened on the same durable state restores the exact run ID, SHAs, stage results, decisions, errors, evidence references, and terminal/open state. + +### Queued cancellation + +Trigger: `RunManager.Cancel` cancels only the context and leaves the queued job in `repoQueue.pending`. + +Masking condition: queued work checks cancellation before side effects. + +Visible symptom: a canceled queued run later starts and can perform work. + +RED test: `go test ./internal/daemon -run TestRunManager_CancelQueuedRunNeverStartsWork -count=1`. + +RED assertion: canceled queued work never enters `running`, never executes its side effect, and reaches a durable canceled terminal state. + +### Awaiting merge state and completion events + +Trigger: `RunManager.execute` publishes `EventRunCompleted` whenever `WorkFunc` returns nil, even when `Finish` left status `running` for awaiting merge. + +Masking condition: consumers poll status and ignore the event stream. + +Visible symptom: consumers receive terminal completion while public status remains open/running. + +RED test: `go test ./internal/daemon -run TestRunManager_AwaitingMergeDoesNotEmitTerminalCompletion -count=1`. + +RED assertion: awaiting-merge emits an explicit nonterminal/open event or no terminal event, and only a true terminal transition emits completion. + +### Idle and daemon-down semantics + +Trigger: idle timing is driven only by event activity and public CLI status cannot distinguish daemon unreachable from idle. + +Masking condition: active runs emit frequent events and callers treat socket errors as empty state. + +Visible symptom: silent active work can be stopped by idle timeout, and daemon unreachability can be misreported as idle. + +RED tests: `go test ./internal/daemon -run TestRun_DoesNotIdleStopWhileRunIsActiveWithoutActivityEvents -count=1` and `go test ./cmd/made -run TestStatus_DaemonUnavailableIsExplicit -count=1`. + +RED assertion: active work keeps the daemon alive; unavailable socket returns a non-zero explicit error and never an idle JSON state. + +### Fixed stages and current stage + +Trigger: `StatusReport` exposes stage results but no current-stage field, and infrastructure errors can bypass stage result publication. + +Masking condition: happy-path stage completion and a continuously connected event consumer. + +Visible symptom: reconnecting callers cannot know the active stage, and failed infrastructure stages may be absent from the fixed ordered list. + +RED tests: `go test ./cmd/made -run TestStatusJSON_ReportsCurrentStageAfterReconnect -count=1` and `go test ./internal/orchestrator -run TestNewWorkFunc_InfrastructureFailureRecordsFailedStage -count=1`. + +RED assertion: status contains fixed ordered stages plus `current_stage`, and the active infrastructure failure is recorded with a stage-specific fail result. + +### Decision timing and conflicts + +Trigger: `ReviewDecisions.Set` overwrites an existing decision without checking stage/run state. + +Masking condition: exactly one authorized decision arrives before the run changes state. + +Visible symptom: a late approval can overwrite an earlier rejection or a decision can apply after a run has moved past the gate. + +RED test: `go test ./internal/daemon -run TestReviewDecisions_RejectsConflictingDecision -count=1`. + +RED assertion: first decision wins, conflicting/late decisions return an explicit conflict or stale-gate error, and decisions are keyed to exact run/stage identity. + +## Evidence, configuration, and reviewer containment + +### Evidence atomicity and retention + +Trigger: `internal/evidence/inrepo.go:32-39` writes directly with `os.WriteFile`, and orphan evidence ref publication has no retry on compare-and-swap conflict. + +Masking condition: small writes and serialized runs. + +Visible symptom: torn evidence tails or lost concurrent evidence records after interruption/contention. + +RED tests: `go test ./internal/evidence -run TestInRepoStore_WriteEvidenceIsAtomicOnReplacement -count=1` and `go test ./internal/evidence -run TestOrphanBranchStore_ConcurrentWritesRetainBothRuns -count=1`. + +RED assertion: replacement is temp-file/fsync/rename atomic and concurrent runs retain both evidence records with bounded history/retention. + +### Semantic configuration enforcement + +Trigger: current trusted-config tests cover core fields but not every behavioral field's pushed-branch override or every switch at the boundary. + +Masking condition: trusted and pushed fixtures use equal/default values. + +Visible symptom: an untrusted branch can alter behavior if a field is accidentally read from the pushed copy or a config switch is accepted but ignored. + +RED test: `go test ./internal/config -run TestLoadEffectiveConfig_RejectsPushedBehaviorOverrides -count=1`. + +RED assertion: `Document`, `Review`, `DisableProjectSettings`, `NoCI`, `CI`, `Test.Evidence.Branch`, commands, agents, and `allow_repo_commands` resolve from the documented trusted source and invalid semantic switches fail closed. + +### Reviewer containment + +Trigger: `internal/pipeline/review/review.go:95-98` runs `git add -A` after applying an agent patch. + +Masking condition: clean worktree with only the intended patch. + +Visible symptom: unrelated modified/untracked files are committed as part of an auto-fix. + +RED test: `go test ./internal/pipeline/review -run TestRun_AutoFixDoesNotStageUnrelatedChanges -count=1`. + +RED assertion: the auto-fix commit contains only patch-authorized paths and rejects out-of-scope patches. + +## Strict compatibility and live scenarios + +Trigger: current fakes accept arbitrary flags and the brief requires real Made binary execution against strict Consigliere-script fakes without modifying Consigliere or the shared daemon. + +Masking condition: permissive fake behavior and unit-only coverage. + +Visible symptom: obsolete CLI/agent invocations pass local tests but fail at the real tool boundary. + +RED test: `go test ./internal/github ./internal/agent -run 'TestStrictFakeRejects|TestSpawn_.*Contract|Test.*PrChecks' -count=1`. + +RED assertion: strict fakes reject unsupported invocation shapes with non-zero status and Made reports explicit structured contract errors. + +Forbidden live scenarios are not claimed: no real-project pipeline, gate initialization, run submission, default branch push, shared daemon lifecycle, merge, auto-merge, branch deletion, or ask-user decision. diff --git a/evidence/phase-1-red-made-remediation-continuation.md b/evidence/phase-1-red-made-remediation-continuation.md new file mode 100644 index 0000000..1639a05 --- /dev/null +++ b/evidence/phase-1-red-made-remediation-continuation.md @@ -0,0 +1,164 @@ +# Phase 1 — captured RED evidence + +All commands ran before the corresponding production fix. + +Environment prefix for every command: `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null`. + +## GitHub fake and client boundary + +Command: `go test ./internal/github -run 'TestStrictFakeGH' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestStrictFakeGHRejectsUnsupportedJSONFields: strict fake accepted unsupported invocation, output={"mergeStateStatus":"CLEAN"} +TestStrictFakeGHRejectsPRURLAsWorkflowRunID: strict fake accepted PR URL as workflow run ID, output=log line 1 +TestStrictFakeGHInvocationLogDoesNotAcceptLegacyMergeStateCommand: legacy merge-state invocation was accepted: invoked: args=pr view https://github.com/example/repo/pull/1 --json mergeStateStatus +``` + +This proves the fake accepts obsolete fields and PR URLs at the wrong boundary. + +## CI check and workflow-run contract + +Command: `go test ./internal/pipeline/ci -run 'TestRun_(UsesPrChecksJSONContract|PassesWorkflowRunIDToLogsAndRerun)' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestRun_UsesPrChecksJSONContract: expected gh pr checks invocation, got invoked: args=auth status +invoked: args=pr view https://github.com/example/repo/pull/7 --json mergeStateStatus +TestRun_PassesWorkflowRunIDToLogsAndRerun: PR URL was passed to a workflow-run command: invoked: args=auth status +invoked: args=pr view https://github.com/example/repo/pull/8 --json mergeStateStatus +``` + +This proves the production CI stage invokes mergeability instead of the supported checks contract and cannot preserve workflow run identity. + +## Agent invocation and structured output + +Command: `go test ./internal/agent -run 'TestSpawn_(CodexUsesStructuredExecContract|RejectsStructuredOutputWithoutFindingsField)' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestSpawn_CodexUsesStructuredExecContract: expected Codex structured invocation token "exec", got invoked: args=[.../fakeagent review --worktree ...] +TestSpawn_RejectsStructuredOutputWithoutFindingsField: expected schema-invalid structured output to fail closed +``` + +This proves the current adapter uses the wrong Codex shape and accepts an output without the required findings field. + +## Run lifecycle and decision contracts + +Command: `go test ./internal/daemon -run 'TestRunManager_(CancelQueuedRunNeverStartsWork|AwaitingMergeDoesNotEmitTerminalCompletion|SnapshotDoesNotAliasStageSlices)|TestReviewDecisions_FirstDecisionWins' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestRunManager_CancelQueuedRunNeverStartsWork: cancelled queued run started execution +TestRunManager_AwaitingMergeDoesNotEmitTerminalCompletion: awaiting-merge emitted terminal event: {RunID:run-1 Kind:run_completed ...} +TestRunManager_SnapshotDoesNotAliasStageSlices: snapshot stages aliased caller memory: [{Name:intent Result:fail}] +TestReviewDecisions_FirstDecisionWins: conflicting decision overwrote first decision: got "approved" +``` + +These are independent trigger/masking/symptom failures in queue cancellation, awaiting-merge lifecycle, public snapshot ownership, and decision conflict rules. + +## Reviewer containment + +Command: `go test ./internal/pipeline/review -run 'TestRun_AutoFixDoesNotStageUnrelatedChanges' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestRun_AutoFixDoesNotStageUnrelatedChanges: unrelated file was included in auto-fix commit: reviewed.txt +unrelated.txt +``` + +This proves `git add -A` crosses the reviewer containment boundary. + +## Semantic configuration + +Command: `go test ./internal/config -run 'TestLoadEffectiveConfig_RejectsUnknownSemanticSwitch' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestLoadEffectiveConfig_RejectsUnknownSemanticSwitch: expected unknown semantic configuration switch to fail closed +``` + +This proves unknown configuration switches are silently accepted. + +## Public structured command surface + +Command: `go test ./cmd/made -run 'Test(CapabilitiesJSONExposesStructuredRunContract|ObsoleteStatusCommandIsRejected)' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestCapabilitiesJSONExposesStructuredRunContract: capabilities exit code = 2; stderr=made: unknown command "capabilities" +TestObsoleteStatusCommandIsRejected: obsolete status exit code = 1, want 2; stderr=made status: daemon not reachable: dial .../daemon.sock: ... no such file or directory +``` + +This proves the native versioned command surface is absent and the obsolete global-latest status path is still active. + +## Evidence retention and concurrent publication + +Command: `go test ./internal/evidence -run 'TestOrphanBranchStore_ConcurrentWritesRetainBothRuns' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestOrphanBranchStore_ConcurrentWritesRetainBothRuns: evidence branch missing run-a: run-b/result.json +``` + +This proves concurrent evidence publication loses one run under the current compare-and-swap update path. + +## Current-stage public status + +Command: `go test ./cmd/made -run 'TestStatusJSONReportsCurrentStageFromOrderedState' -count=1` + +Exit: `1`. + +Relevant output: + +```text +TestStatusJSONReportsCurrentStageFromOrderedState: status omitted current stage: {"schema_version":1,"run_id":"run-current-stage",...} +``` + +This proves the current structured status schema cannot report the active stage after a reconnect or missed event. + +## Durable restart recovery + +Command: `go test ./internal/daemon -run 'TestRunManager_RestoresDurableSnapshotAfterRestart' -count=1` + +Exit: `1` during test compilation. + +Relevant output: + +```text +undefined: OpenRunManager +undefined: RunSubmission +undefined: RunAwaitingMerge +``` + +This is the named public durability contract missing from the exact-base source, not a fixture or import typo: no durable manager/open path or awaiting-merge state exists yet. + +## RED-to-GREEN boundary + +No production source fix was applied before these RED commands completed. + +The strict fake, Codex adapter, GitHub check adapter, lifecycle manager, config parser, reviewer, and CLI surface are now pinned to independent contract failures. diff --git a/evidence/phase-2-external-contracts.md b/evidence/phase-2-external-contracts.md new file mode 100644 index 0000000..4773e1a --- /dev/null +++ b/evidence/phase-2-external-contracts.md @@ -0,0 +1,88 @@ +# Phase 2 external-tool contract evidence + +Base: `3e19ed9d598a68149da5a73949533e8095ca4403` + +## GitHub CLI and CI + +The RED contract required the Made adapter to authenticate explicitly, invoke +`gh pr checks` with the supported JSON fields, preserve workflow run IDs from +check links, and reject PR URLs at the run-log and rerun boundary. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/github -run 'Test(PRChecks|StrictFakeGH|AuthStatus|CreatePR|CheckLogs|RerunCheck)' -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/github 2.064s +``` + +The strict fake rejects legacy `gh pr view ... mergeStateStatus`, arbitrary +arguments, PR URLs passed to `gh run view` or `gh run rerun`, and malformed +workflow run IDs. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/ci -run 'TestRun_' -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/pipeline/ci 3.613s +``` + +The CI adapter now consumes `name,state,bucket,link`, treats the command exit +status as the check failure boundary, and passes the numeric workflow run ID +to logs and rerun operations. + +## Codex structured review adapter + +The RED contract required a strict structured invocation, a required findings +array, and explicit rejection of unsupported Claude behavior. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent/... -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/agent 1.004s +? github.com/douglasjarquin/made/internal/agent/agenttest [no test files] +``` + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/review -run 'TestRun_(AutoFixApplied|AskUserFindingQueued|BlockingFindingHaltsStage)' -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/pipeline/review 1.278s +``` + +The adapter invokes only `exec --json --output-schema +--output-last-message --ephemeral -C `, reads the +structured output file, rejects missing or null `findings`, rejects unknown +JSON fields and trailing values, and rejects Claude before process launch. +The fake Codex boundary rejects obsolete or invented argument shapes. + +## LSP diagnostics + +Command-equivalent diagnostics were run for the changed GitHub client, CI +adapter, strict fake, and focused contract tests. + +Result: no errors or warnings were reported. + +The initial focused CI diagnostic emitted one non-blocking `stringsseq` +efficiency hint at `internal/pipeline/ci/ci_contract_test.go:56`. +That hint was cleared before the final all-changed-Go-file diagnostic pass. diff --git a/evidence/phase-3-lifecycle-durability.md b/evidence/phase-3-lifecycle-durability.md new file mode 100644 index 0000000..65f1b24 --- /dev/null +++ b/evidence/phase-3-lifecycle-durability.md @@ -0,0 +1,83 @@ +# Phase 3 lifecycle and durability evidence + +Base: `3e19ed9d598a68149da5a73949533e8095ca4403` + +## Durable run identity and lifecycle + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon ./cmd/made -run 'Test(RunManager|ReviewDecisions|Capabilities|StatusJSON|Doctor|Daemon)' -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/daemon 2.980s +ok github.com/douglasjarquin/made/cmd/made 7.570s +``` + +The run manager now returns the persisted queued identity before drain, +supports exact submission metadata and SHA fields, removes queued jobs before +execution on cancellation, preserves immutable snapshots, keeps awaiting +merge non-terminal, and records succeeded/canceled/superseded terminal states. +The WAL checkpoint test covers restart restoration, awaiting-merge to +succeeded, torn final-record tolerance, bounded WAL retention, and durable +first-wins review decisions. + +## Evidence and reviewer containment + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/evidence -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/evidence 0.991s +``` + +Concurrent orphan evidence writers use compare-and-swap ref updates with +bounded retries and retain both run records. +In-repository evidence uses same-directory write, fsync, rename, and directory +fsync ordering with path containment checks. +Review auto-fixes stage only the files in the applied patch through +`git apply --index`; unrelated worktree files remain outside the commit. + +## Semantic configuration and rebase fixture + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/config ./internal/orchestrator -count=1 +``` + +Result: + +```text +ok github.com/douglasjarquin/made/internal/config 0.468s +ok github.com/douglasjarquin/made/internal/orchestrator 5.064s +``` + +YAML loading now rejects unknown fields and multiple documents. +The trusted `no_ci` switch is enforced by skipping the CI command while +recording a passing disabled stage. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/rebase -run 'TestRun_(CleanRebaseProceeds|ConflictingRebaseHalts)' -count=1 -v +``` + +Result: + +```text +PASS +ok github.com/douglasjarquin/made/internal/pipeline/rebase 1.030s +``` + +The previously observed clean-rebase failure was caused by the child Git +process lacking identity under signing isolation; Made now supplies a +gate-local identity and disables signing for that child only. diff --git a/evidence/phase-4-conflict-repair.md b/evidence/phase-4-conflict-repair.md new file mode 100644 index 0000000..f86d796 --- /dev/null +++ b/evidence/phase-4-conflict-repair.md @@ -0,0 +1,125 @@ +# Phase 4 conflict-repair continuation evidence + +This receipt records the Made-only conflict repair for PR [#2](https://github.com/douglasjarquin/made/pull/2). + +## Custody and ancestry + +The exact requested base is `3e19ed9d598a68149da5a73949533e8095ca4403`. + +The merged `origin/main` parent is `34d44be504291482d973c65bd427ba964df5e0e9`. + +The pre-merge continuation tip is `25df7116bb0eebc6070603e1e080850dc9f0d211`. + +The conflict-repair merge commit is `0a7c21d6d3001b85b38330766e01980bd5e92f2c`. + +The review-helper cleanup commit is `bac8ed2777f584d98eb1ba8015cf1269d01a8c1e`. + +The final durability correction commit is `918da271aa9521d292bbda22a862591b770f9af6`. + +The final branch retains the exact base as an ancestor and retains both continuation and `origin/main` as merge parents. + +The prior dirty remediation worktree was not opened, reused, cleaned, reset, deleted, copied, or inspected. + +The shared Made daemon was not started, stopped, restarted, or updated. + +## Conflict resolution + +The exact merge command was `git merge --no-commit --no-ff origin/main`. + +Mainline PR1 daemon, gate-spool, review-worktree, and modern run-command architecture was retained where it replaced obsolete duplicate CLI paths. + +Continuation contracts were retained for exact GitHub check fields and workflow run IDs, strict Codex structured invocation, durable run state, review decisions, status ordering, and evidence publication. + +The obsolete duplicate files `cmd/made/capabilities.go`, `cmd/made/pr.go`, `cmd/made/run.go`, and `cmd/made/run_handlers.go` were removed because their modern replacements are `cmd/made/runcommands.go`, `cmd/made/runhandlers.go`, and `cmd/made/strictjson.go`. + +The merge had no unresolved paths before commit `0a7c21d6d3001b85b38330766e01980bd5e92f2c`. + +## Trigger, masking condition, and visible symptom + +The status trigger was a real failing `review` stage without an explicit current-stage field. + +The status masking condition was deriving from the normalized stage list instead of the actual snapshot order. + +The status symptom was `current_stage` reported as `rebase` instead of `review`. + +The awaiting-review restart trigger was a durable `awaiting_review` snapshot at daemon reopen. + +The restart masking condition was reconciling only `running` records. + +The restart symptom was a non-terminal awaiting-review record surviving without durable restart failure. + +The decision trigger was a first decision for a persisted awaiting-review finding. + +The decision masking condition was a manager guard accepting only `running` state. + +The decision symptom was a valid restored decision rejected as a state conflict. + +The ID trigger was two fresh `RunManager.NewRunID` calls. + +The ID masking condition was the order-derived `run-1`, `run-2` counter. + +The ID symptom was a restart-reusable non-UUID identity. + +The evidence trigger was two concurrent writers publishing different run directories to one ref. + +The evidence masking condition was a single compare-and-swap attempt with no retry. + +The evidence symptom was one run directory missing from the evidence branch. + +The reviewer trigger was a valid auto-fix while unrelated user work existed in the worktree. + +The reviewer masking condition was the old clean-worktree requirement and broad index mutation path. + +The reviewer symptom was refusal of valid review or unrelated files entering an auto-fix commit. + +The compaction trigger was the WAL record that crossed the compaction threshold. + +The compaction masking condition was compacting from the old in-memory run snapshot before installing the durable candidate. + +The compaction symptom was a restart restoring stage message `before` instead of `compaction-trigger`. + +## RED evidence + +The pre-fix focused command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./... -count=1` exited `1` for status, daemon recovery and IDs, orphan CAS, strict review fixtures, and reviewer dirty-worktree contracts. + +The post-merge lint RED command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 make lint` exited `2` for three unused helpers: `decodeFindings`, `requireCleanWorktree`, and `statusPaths`. + +The compaction RED command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/daemon -run '^TestRunManager_CompactionPersistsTriggeringTransition$' -count=1` exited `1` with `compaction lost triggering transition` and restarted message `before`. + +The complete earlier external-tool RED matrix is preserved in `evidence/phase-1-red-made-remediation-continuation.md`. + +## GREEN evidence + +The focused status command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./cmd/made -run 'TestStatusJSONReportsCurrentStageFromOrderedState|TestStatusJSON_ReflectsRealStageUpdate' -count=1` exited `0`. + +The focused daemon command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/daemon -count=1` exited `0`. + +The focused orphan command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/evidence -run 'TestOrphanBranchStore_ConcurrentWritesRetainBothRuns' -count=1` exited `0`. + +The focused reviewer and agent command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/pipeline/review ./internal/agent -count=1` exited `0`. + +The compaction GREEN command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/daemon -run '^TestRunManager_CompactionPersistsTriggeringTransition$' -count=1` exited `0`. + +The compaction race command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test -race ./internal/daemon -run '^TestRunManager_CompactionPersistsTriggeringTransition$' -count=1` exited `0`. + +The final ordinary suite, race and shuffle suite, build, vet, configured lint, and formatting commands all exited `0` at final SHA `918da271aa9521d292bbda22a862591b770f9af6`. + +## Manual QA boundary + +The final real-binary disposable-home scenario and cleanup receipt are recorded in `evidence/phase-4-manual-qa.md`. + +It observed capabilities JSON, explicit obsolete-status rejection, a disposable local daemon start/status/list/stop lifecycle, exact-ID not-found failure, and absent socket and lock after stop. + +No real project, gate, pipeline, default branch, shared daemon, remote deletion, merge, auto-merge, or ask-user finding was used. + +The separate review suggestion to invoke `make lint all` is not the repository-configured lint command and is not a brief requirement; the configured `make lint` target passed. + +## Final direct-PR delivery read + +The branch was pushed only to `origin/cs/made-remediation-continuation` at `12b83a6649b5e198049754f1cb6427d7b0dc51a0`. + +The hosted `build-test-lint` check for that exact head completed successfully as check run `95537594230`. + +The final read-only PR state is `open`, `merged=false`, `head=cs/made-remediation-continuation`, `head_sha=12b83a6649b5e198049754f1cb6427d7b0dc51a0`, `base=main`, `base_sha=34d44be504291482d973c65bd427ba964df5e0e9`, `mergeable=true`, `mergeable_state=clean`, and `auto_merge=null`. + +The PR base is the GitHub `main` branch ref, while the exact requested base is preserved as local and remote branch ancestry through the explicit task worktree and conflict-repair merge. diff --git a/evidence/phase-4-final-validation.md b/evidence/phase-4-final-validation.md new file mode 100644 index 0000000..b7a1d66 --- /dev/null +++ b/evidence/phase-4-final-validation.md @@ -0,0 +1,126 @@ +# Phase 4 final local validation evidence + +The earlier ledger receipt was recorded at +`afea024e1da9f59be9181c18f18b11793a782f36`. +After the final managed-gate, cancellation, durable-publication, and review +environment corrections, the source and test validation candidate is +`910fc54a98e7da644bc5e170281fd935e429692f`. +The exact base remains +`3e19ed9d598a68149da5a73949533e8095ca4403`. + +## Build + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go build ./... +``` + +Result: exit code 0 with no output. + +## Race and shuffle suite + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race -shuffle=on -count=1 ./... +``` + +Result: exit code 0 at source and test validation candidate +`910fc54a98e7da644bc5e170281fd935e429692f`. +Every package completed with `ok`, including `cmd/made`, `internal/agent`, +`internal/api`, `internal/config`, `internal/daemon`, `internal/evidence`, +`internal/github`, `internal/orchestrator`, and every pipeline package. + +## Vet and lint + +Commands: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go vet ./... +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null golangci-lint run ./... +``` + +Results: + +```text +go vet ./...: exit code 0, no output +golangci-lint run ./...: 0 issues. +``` + +## Scope and diagnostics + +Command: + +```text +git diff --check +git rev-parse HEAD +git rev-parse 3e19ed9d598a68149da5a73949533e8095ca4403 +``` + +Results: + +```text +git diff --check: exit code 0 +HEAD before this documentation refresh: 910fc54a98e7da644bc5e170281fd935e429692f +base: 3e19ed9d598a68149da5a73949533e8095ca4403 +``` + +LSP diagnostics were run for all 50 changed Go files from the exact base. +No errors, warnings, information diagnostics, or hints remained. + +The real Made binary manual-QA receipt for the same exact source is in +`evidence/phase-4-manual-qa.md`. + +The initial isolated-suite rebase failure was reproduced, explained as missing +child Git identity under signing isolation, fixed in Made, and re-run GREEN in +`evidence/phase-3-lifecycle-durability.md`. + +This evidence refresh is documentation-only and is committed after the source +and test validation candidate; it does not change Made source or tests. + +## Final conflict-repair and durability correction + +The conflict-repair merge is `0a7c21d6d3001b85b38330766e01980bd5e92f2c`. + +The final source SHA is `918da271aa9521d292bbda22a862591b770f9af6`. + +The compaction regression command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/daemon -run '^TestRunManager_CompactionPersistsTriggeringTransition$' -count=1` exited `0`. + +The affected daemon package command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./internal/daemon -count=1` exited `0`. + +The final ordinary suite command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./... -count=1` exited `0`. + +The final race and shuffle command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test -race -shuffle=on -count=1 ./...` exited `0` for every package. + +The final build command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go build ./...` exited `0`. + +The final vet command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go vet ./...` exited `0`. + +The configured lint command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 make lint` printed `0 issues.` and exited `0`. + +The formatting command `test -z "$(gofmt -l internal cmd)"` exited `0`. + +## Conflict-repair validation at final HEAD + +The conflict-repair merge is `0a7c21d6d3001b85b38330766e01980bd5e92f2c`. + +The final source fix commit is `bac8ed2777f584d98eb1ba8015cf1269d01a8c1e`. + +The exact requested base remains `3e19ed9d598a68149da5a73949533e8095ca4403`. + +The ordinary full suite command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test ./... -count=1` exited `0`. + +The final race and shuffle command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go test -race -shuffle=on -count=1 ./...` exited `0` for every package. + +The final build command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go build ./...` exited `0`. + +The final vet command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go vet ./...` exited `0`. + +The configured lint command `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 make lint` printed `golangci-lint run ./...` and `0 issues.` and exited `0`. + +The formatting command `test -z "$(gofmt -l internal cmd)"` exited `0`. + +The exact final worktree SHA at this receipt is `bac8ed2777f584d98eb1ba8015cf1269d01a8c1e`. + +The separate review suggestion to invoke `make lint all` is not the repository-configured lint command and is not a required brief command; the configured `make lint` target passed as recorded above. diff --git a/evidence/phase-4-herdr-cleanup.md b/evidence/phase-4-herdr-cleanup.md new file mode 100644 index 0000000..5e87b22 --- /dev/null +++ b/evidence/phase-4-herdr-cleanup.md @@ -0,0 +1,23 @@ +# Phase 4 Herdr cleanup receipt + +The isolated named session was +`cs-lab-made-remediation-9714-1438`. + +The session was provisioned only through +`/Users/douglasjarquin/.consigliere/capos/made/bin/cs-herdr-lab.sh`, with the +required default-session custody checks active. + +Cleanup command: + +```text +HERDR_LAB_HELPER='/Users/douglasjarquin/.consigliere/capos/made/bin/cs-herdr-lab.sh' +HERDR_LAB_SESSION='cs-lab-made-remediation-9714-1438' +trap '"$HERDR_LAB_HELPER" teardown "$HERDR_LAB_SESSION"' EXIT +exit +``` + +The persistent helper shell exited with code `0` after the trap ran. +The helper's built-in refuse-default checks and identical default-fleet +verification therefore passed. + +No direct Herdr server/session lifecycle command was used. diff --git a/evidence/phase-4-manual-qa.md b/evidence/phase-4-manual-qa.md new file mode 100644 index 0000000..1d9cbcc --- /dev/null +++ b/evidence/phase-4-manual-qa.md @@ -0,0 +1,311 @@ +# Phase 4 disposable manual-QA evidence + +The scenario used only the task branch binary, a disposable Made home, and an +isolated named Herdr lab session. +No real project gate was initialized and no shared Made daemon was changed. + +## Real Made binary and durable CLI state + +Build command: + +```text +qa_dir=$(mktemp -d /tmp/made-remediation-qa.XXXXXX) +go build -o "$qa_dir/made" ./cmd/made +``` + +Build result: + +```text +/tmp/made-remediation-qa.Z8Vnit +``` + +The disposable daemon was started with: + +```text +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made daemon start --idle-timeout=5m +``` + +Observed result: + +```text +made daemon: started (pid 33848) +``` + +Command: + +```text +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made capabilities --json +``` + +Observed result: + +```json +{"schema_version":1,"protocol_version":1,"commands":["run.submit","run.status","run.list","run.cancel","review.decide","doctor"]} +``` + +Command: + +```text +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made run submit --repo qa/repo --branch feature/qa --ref refs/heads/feature/qa --old-sha 1111111111111111111111111111111111111111 --input-sha 2222222222222222222222222222222222222222 --submission-id qa-submission-1 --gate /tmp/qa-gate --json +``` + +The real binary returned the exact queued identity before drain with +`run_id=run-1`, `state=queued`, the supplied input SHA, submission ID, gate +path, and all nine ordered pending stages. + +The immediate exact-ID status remained `state=queued` with +`execution_finished=false` and the same identity fields, proving the public +surface spools work without claiming that remediation executed. + +Command: + +```text +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made run status --json run-1 +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made run status --json run-does-not-exist +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made status --json +``` + +Observed invalid-boundary results: + +```text +made run status: handler_error: status: no run "run-does-not-exist" +made: status is obsolete; use made run status +``` + +The daemon was stopped through the same disposable Made home, restarted, and +the exact `run-1` status was restored from durable state with +`state=queued` and the same SHA/submission identity. + +## Doctor JSON + +Command: + +```text +MADE_HOME=/tmp/made-remediation-qa.Z8Vnit /tmp/made-remediation-qa.Z8Vnit/made doctor --json +``` + +Observed result: + +```json +{"schema_version":1,"protocol_version":1,"healthy":true,"checks":{"daemon":"reachable","gate":"not_initialized","github":"authenticated","herdr":"unavailable"}} +``` + +Herdr is informational in the doctor report and did not affect the Made-only +run contract. + +## Isolated Herdr lab + +Every task-specific Herdr probe used the required helper and trailing named +session argument. + +Command: + +```text +HERDR_LAB_HELPER='/Users/douglasjarquin/.consigliere/capos/made/bin/cs-herdr-lab.sh' +HERDR_LAB_SESSION='cs-lab-made-remediation-9714-1438' +"$HERDR_LAB_HELPER" run "$HERDR_LAB_SESSION" status server +``` + +Observed result: + +```text +status: running +version: 0.8.0 +protocol: 20 +compatible: yes +socket: /Users/douglasjarquin/.config/herdr/sessions/cs-lab-made-remediation-9714-1438/herdr.sock +``` + +The named session remains provisioned until final cleanup through the helper. + +## Follow-up after lifecycle review correction + +Source and test commit: +`d1dab7c73c3bdf678a668891c17a04d9c34b13c4`. + +The real Made binary was rebuilt from that commit and rerun against a fresh +disposable home at `/tmp/made-remediation-qa-delivery.G9K78Z`. +The public `run submit` response and exact status both remained +`state=queued` with `execution_finished=false`. +The same queued identity survived a disposable daemon stop and restart. +`made status --json` still rejected with exit code 2, and `doctor --json` +returned `healthy=true` with `daemon=reachable`, `gate=not_initialized`, +`github=authenticated`, and `herdr=unavailable`. +The disposable daemon was stopped and its temporary home was moved to +recoverable temporary trash at +`/tmp/.made-remediation-qa-delivery-trash.made-remediation-qa-delivery.G9K78Z` +after the scenario. + +## Final source candidate + +Source and test commit: +`fdd8a7853053e9eb0efc099244c5296006c3605a`. + +The current `./cmd/made` binary was built into a fresh disposable home at +`/tmp/made-remediation-qa-fdd-green.ugxcmL`. + +The disposable daemon was launched in the background and became ready on its +own `daemon.sock`. + +The real binary reported the expected capabilities, then `run submit --json` +returned `run-1` with the supplied repository, branch, ref, old SHA, input SHA, +submission ID, gate path, `state=queued`, `execution_finished=false`, +`current_stage=intent`, and the nine ordered pending stages. + +The exact `run status --json run-1` and `run list --json` responses preserved +the same identity and lifecycle state. + +`doctor --json` returned `healthy=true` with +`daemon=reachable`, `gate=not_initialized`, `github=authenticated`, and +`herdr=unavailable`. + +The daemon was stopped and restarted through the same disposable home. +The exact `run-1` status after restart preserved the queued state, +`execution_finished=false`, all identity fields, and all nine pending stages. + +The invalid public-boundary checks returned: + +```text +made run status run-1 unexpected +exit=2: usage: made run status [--json] + +made status --json +exit=2: made: status is obsolete; use made run status +``` + +The disposable daemon was stopped and its home was moved to recoverable +temporary trash at +`/tmp/.made-remediation-qa-fdd-green-trash.rat3CP/qa-home`. + +## Final lifecycle candidate + +Source and test commit: +`60420902ea5b1ed434f57c86ebb0e85be7be5281`. + +The current `./cmd/made` binary was built into a fresh disposable home at +`/tmp/made-remediation-qa-604.8yLbcs`. + +The real binary returned the expected capabilities and spooled `run-1` with +the supplied identity, `state=queued`, `execution_finished=false`, and all +nine ordered pending stages. + +`made run cancel run-1 --json` returned `{"ok":true}`. +The exact status immediately after cancellation returned +`state=canceled`, `execution_finished=true`, the same identity fields, and +`error=context canceled`. + +A second spooled `run-2` preserved its exact identity and queued state until +the disposable daemon was stopped. +Graceful daemon shutdown intentionally canceled that in-flight queued record; +after restart, exact `run-2` status restored `state=canceled`, +`execution_finished=true`, and the same identity and stage records. +This proves durable terminal-state recovery across the real binary restart +while respecting the daemon's shutdown cancellation contract. + +`doctor --json` returned `healthy=true` with +`daemon=reachable`, `gate=not_initialized`, `github=authenticated`, and +`herdr=unavailable`. + +The invalid public-boundary checks returned exit code 2: + +```text +made run status run-2 unexpected +usage: made run status [--json] + +made status --json +made: status is obsolete; use made run status +``` + +The disposable daemon was stopped and its home was moved to recoverable +temporary trash at +`/tmp/.made-remediation-qa-604-trash.qkeAfA/qa-home`. + +## Final source candidate + +Source and test commit: +`910fc54a98e7da644bc5e170281fd935e429692f`. + +The current `./cmd/made` binary was built into a fresh disposable home at +`/tmp/made-remediation-qa-910.WcgMFi`. + +The real binary returned a spooled `run-1` with exact repository, branch, ref, +old SHA, input SHA, submission ID, gate path, `state=queued`, +`execution_finished=false`, and all nine ordered pending stages. + +`made run cancel run-1 --json` returned `{"ok":true}`. +The exact status immediately after cancellation returned +`state=canceled`, `execution_finished=true`, the same identity fields, and +`error=context canceled`. + +After the disposable daemon stopped and restarted, exact `run-1` status +restored the same canceled terminal state and identity fields. + +`doctor --json` returned `healthy=true` with +`daemon=reachable`, `gate=not_initialized`, `github=authenticated`, and +`herdr=unavailable`. + +The invalid public-boundary checks returned exit code 2: + +```text +made run status run-1 unexpected +usage: made run status [--json] + +made status --json +made: status is obsolete; use made run status +``` + +The disposable daemon was stopped and its home was moved to recoverable +temporary trash at +`/tmp/.made-remediation-qa-910-trash.KmgO42/qa-home`. + +## Final durability-correction manual QA + +The exact final source SHA was `918da271aa9521d292bbda22a862591b770f9af6`. + +The binary was built into `/tmp/made-pr2-manual-qa-final.CM6oy5/made` and had SHA256 `ef63f79b90ad4b7760dd6d6a734620d20b3d117c5dcc56aa9a00bce48d766b5b`. + +The disposable home was `/tmp/made-pr2-manual-qa-final.CM6oy5/home`. + +`made capabilities --json` exited `0` with schema version `1`, protocol version `1`, and the six supported commands. + +`made status --json` exited `2` with the explicit obsolete-command message. + +`made daemon start --idle-timeout=1m` exited `0` and reported PID `29451`. + +`made daemon status` exited `0` and reported the same local PID. + +`made run list --json` exited `0` with schema version `1`, protocol version `1`, and an empty run list. + +`made run status --json missing-run` exited `1` with the exact-run-ID not-found error. + +`made daemon stop` exited `0` with `made daemon: stopped`. + +The local daemon exited, the socket and lock were absent, and the exact disposable QA directory was removed. + +## Conflict-repair manual QA at final HEAD + +The exact final source SHA was `bac8ed2777f584d98eb1ba8015cf1269d01a8c1e`. + +The binary was built with `env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0 go build -o /tmp/made-pr2-manual-qa.cs7y3w/made ./cmd/made`. + +The binary SHA256 was `502b1d5c3956800ccb4e7bc7c98a28b4789b42ded6270a5687013c7b32a0ac90`. + +The disposable home was `/tmp/made-pr2-manual-qa.cs7y3w/home`. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made capabilities --json` exited `0` and returned schema version `1`, protocol version `1`, and the six supported structured commands. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made status --json` exited `2` and returned `made: status is obsolete; use made run status --json `. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made daemon start --idle-timeout=1m` exited `0` and reported `made daemon: started (pid 54799)`. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made daemon status` exited `0` and reported `made daemon: running (pid 54799)`. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made run list --json` exited `0` and returned schema version `1`, protocol version `1`, and an empty runs array. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made run status --json missing-run` exited `1` and returned `made run status: handler_error: run.status: exact run_id "missing-run" was not found`. + +The command `MADE_HOME=/tmp/made-pr2-manual-qa.cs7y3w/home /tmp/made-pr2-manual-qa.cs7y3w/made daemon stop` exited `0` and returned `made daemon: stopped`. + +The local daemon process exited, the disposable socket and lock were absent, and the exact disposable QA directory was removed. + +This scenario did not initialize a gate, submit a real project, invoke the shared daemon, alter a default branch, merge a PR, enable auto-merge, or answer an ask-user finding. diff --git a/evidence/phase-4-red-followups.md b/evidence/phase-4-red-followups.md new file mode 100644 index 0000000..21dc871 --- /dev/null +++ b/evidence/phase-4-red-followups.md @@ -0,0 +1,519 @@ +# Phase 4 follow-up RED contracts + +These follow-up RED tests were written after the initial Phase 1 matrix when +the final review found additional Made-owned boundary defects. + +The source candidate before the fixes was +`4617d622b8cdaeb38d2b49458459565c8e7755b7`. + +## Review decision grouping + +Trigger: two pending findings belong to the same review stage and the user +supplies one stage decision. + +Masking condition: each stage has at most one pending finding or the test +supplies one decision per finding. + +Visible symptom: the CLI sends a second decision for the same stage and exits +with `no approve/reject decision provided` or a duplicate-decision error. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestReview_MultipleFindingsInOneStageUseOneDecision -count=1 +``` + +Exit code: `1`. + +Relevant output: + +```text +--- FAIL: TestReview_MultipleFindingsInOneStageUseOneDecision +review_test.go:190: exit code = 1, want 0 +stderr=made review: no approve/reject decision provided +FAIL +FAIL github.com/douglasjarquin/made/cmd/made +``` + +## In-repository evidence path containment + +Trigger: a pre-existing symlink points the configured evidence directory outside +the repository. + +Masking condition: the configured evidence path contains only ordinary +directories. + +Visible symptom: `WriteEvidence` follows the symlink and writes evidence +outside the repository. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/evidence -run TestInRepoStoreRejectsSymlinkedEvidenceDirectory -count=1 +``` + +Exit code: `1`. + +Relevant output: + +```text +--- FAIL: TestInRepoStoreRejectsSymlinkedEvidenceDirectory +evidence_contract_test.go:63: WriteEvidence accepted a symlinked evidence directory +FAIL +FAIL github.com/douglasjarquin/made/internal/evidence +``` + +## Durable stage-update rollback + +Trigger: a stage update occurs after the durable run store is closed or +otherwise rejects the WAL append. + +Masking condition: the durable store remains writable for the whole run. + +Visible symptom: `UpdateStages` returns an error but leaves the rejected stage +in the in-memory public snapshot. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_UpdateStagesRollsBackOnPersistenceFailure -count=1 +``` + +Exit code: `1`. + +Relevant output: + +```text +--- FAIL: TestRunManager_UpdateStagesRollsBackOnPersistenceFailure +persistence_contract_test.go:228: in-memory stage update survived persistence failure +FAIL +FAIL github.com/douglasjarquin/made/internal/daemon +``` + +The three failures are contract failures in Made code, not fixture, typo, or +unavailable-service failures. + +## GREEN receipts + +The fixes were committed in the Made source candidate +`c359423749328c7778376d16612f36424e4a576d`. + +The grouped review decision test passed under five race repetitions. + +The symlink containment test passed under five race repetitions. + +The durable stage-update and final-persistence tests passed under five race +repetitions. + +The full source validation receipt is in +`evidence/phase-4-final-validation.md`. + +## Strict CLI argument validation + +Trigger: a caller supplies an unsupported trailing positional argument to the +exact-ID `run status` or `run cancel` command. + +Masking condition: callers use only the documented exact run ID and optional +`--json` argument. + +Visible symptom: the CLI attempts a daemon call and returns a daemon error +instead of rejecting the invented invocation at the public boundary. + +Commands: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run 'TestRun(Status|Cancel)RejectsUnsupportedTrailingArgument' -count=1 +``` + +Exit code: `1` before the fix. + +Relevant output: + +```text +run status exit code = 1, want 2 +run cancel exit code = 1, want 2 +``` + +The fix rejects unsupported positional arguments with usage exit code `2`. + +## Pre-staged reviewer containment + +Trigger: an unrelated file is already staged before an auto-fixable reviewer +patch is applied. + +Masking condition: the worktree contains only the reviewer patch or the +unrelated file is merely untracked. + +Visible symptom: the auto-fix commit includes the unrelated staged file. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/review -run TestRun_AutoFixDoesNotStageUnrelatedChanges -count=1 +``` + +Exit code: `1`. + +Relevant output: `unrelated file was included in auto-fix commit` with both +`reviewed.txt` and `unrelated.txt` in the commit path list. + +## Recovery-failure custody + +Trigger: a non-final corrupt WAL record causes daemon recovery to fail after a +valid checkpoint already exists. + +Masking condition: recovery succeeds or a failed recovery is never inspected. + +Visible symptom: the failed recovery path compacts an empty run set and +truncates the corrupt WAL, destroying the last durable checkpoint and the +diagnostic bytes. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestOpenRunManager_PreservesStateAfterRecoveryFailure -count=1 +``` + +Exit code: `1`. + +Relevant output: `failed recovery replaced the durable checkpoint` with an +empty `runs` array. + +## Additional GREEN receipts + +The reviewer containment fix now uses an isolated temporary Git index seeded +from `HEAD`, commits only the patch paths, and restores the original index for +those paths so unrelated staged work remains staged but uncommitted. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/review -run TestRun_AutoFixDoesNotStageUnrelatedChanges -count=1 +``` + +Exit code: `0`. + +## Existing unrelated gate object containment + +Trigger: a caller supplies a real commit object that exists in the managed +gate, but that object is not an ancestor of the received ref. + +Masking condition: the object-existence check accepts any reachable commit and +the ref is not checked for ancestry. + +Visible symptom: Made schedules a run with an input SHA that the named branch +did not receive. + +The counterfactual RED proof removed the ancestry guard from the parent source +candidate `d1dab7c73c3bdf678a668891c17a04d9c34b13c4` and ran: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestGateNotifyPushRPC_RejectsExistingUnrelatedSHA -count=1 +``` + +Exit code: `1`. + +Relevant output: `accepted existing unrelated SHA ... for feature SHA ...`. + +The minimal GREEN fix adds `git merge-base --is-ancestor newSHA ref` after +verifying that the object exists and restores the guard before the GREEN run: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run 'TestGateNotifyPushRPC_(RejectsExistingUnrelatedSHA|RejectsNewSHAThatIsNotTheReceivedRef|SupersededPushValidatesNewestSHA|NormalFeatureBranchPushCreatesRun)' -count=1 +``` + +Exit code: `0`. + +The strict test also preserves the existing superseded-push contract: an older +notification is rejected when the branch has advanced, while the current +received SHA still supersedes an earlier queued run. + +## Stale received-ref notification + +Trigger: a delayed post-receive notification names an older commit after the +same branch has advanced to a newer commit. + +Masking condition: ancestry validation treats every ancestor as the current +received tip. + +Visible symptom: Made schedules a run for the stale input SHA instead of +rejecting the delayed notification. + +The RED command ran against the pre-fix implementation descended from +`fdd8a7853053e9eb0efc099244c5296006c3605a`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestGateNotifyPushRPC_RejectsStaleAncestorSHA -count=1 +``` + +Exit code: `1`. + +Relevant output: `accepted stale ancestor SHA ... for advanced feature ref`. + +The GREEN fix resolves the named ref and requires its object ID to equal +`new_sha`. + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestGateNotifyPushRPC_RejectsStaleAncestorSHA -count=1 +``` + +Exit code: `0`. + +The full gate notification focused suite also passed at source commit +`60420902ea5b1ed434f57c86ebb0e85be7be5281`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run 'TestGateNotifyPushRPC_(NormalFeatureBranchPushCreatesRun|RejectsNewSHAThatIsNotTheReceivedRef|RejectsExistingUnrelatedSHA|RejectsStaleAncestorSHA|SupersededPushValidatesNewestSHA)' -count=1 +``` + +Exit code: `0`. + +## Spooled queued cancellation + +Trigger: a durable `run.submit` record is queued without an attached work +function and is then canceled through the manager or public socket. + +Masking condition: cancellation is tested only for a queued job already held +behind another active job. + +Visible symptom: Made returns cancellation success while the exact run remains +`queued` with `execution_finished=false`. + +The RED command ran against the pre-fix implementation: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CancelSpooledQueuedRunTransitionsTerminal -count=1 +``` + +Exit code: `1`. + +Relevant output: `cancelled spooled run lifecycle = ... Status:queued ... ExecutionFinished:false`. + +The GREEN fix durably transitions the unattached queued record to +`canceled`, records `context.Canceled`, and sets `execution_finished=true`. + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CancelSpooledQueuedRunTransitionsTerminal -count=1 +``` + +Exit code: `0`. + +## Close-versus-WAL publication ordering + +Trigger: a durable mutation completes its WAL append after `Close` captures +the run list but before checkpoint compaction truncates the WAL. + +Masking condition: shutdown and durable mutation are exercised sequentially, +so no append can fall between the checkpoint snapshot and WAL truncation. + +Visible symptom: the update call returns success, but restart loses the +accepted stage update because `Close` compacted a stale snapshot. + +The deterministic RED interleaving command ran against the pre-fix +implementation: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CloseDoesNotDiscardConcurrentDurableMutation -count=1 +``` + +Exit code: `1`. + +Relevant output: `concurrent durable mutation was lost`. + +The GREEN fix serializes durable publication and close, so a mutation either +publishes before the checkpoint or fails closed after the store closes. + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CloseDoesNotDiscardConcurrentDurableMutation -count=1 +``` + +Exit code: `0` at source commit +`60420902ea5b1ed434f57c86ebb0e85be7be5281`. + +## Review-agent environment allowlist + +Trigger: the daemon environment contains a credential-bearing variable whose +name does not contain the old denylist fragments, such as `DATABASE_URL`, +`COOKIE`, `JWT_KEY`, or `KUBECONFIG`. + +Masking condition: the strict fake only rejects the earlier +`MADE_TEST_SECRET` marker, so a substring denylist appears complete while +unrecognized secret names still cross the Codex process boundary. + +Visible symptom: a strict external fake observes a sensitive value inherited +by the read-only Codex review process. + +The RED command ran at the committed pre-allowlist source +`51063e8b724160c04f392cc5413d0d5b53e3082e`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent -run TestSpawn_DoesNotPassSensitiveEnvironmentToCodex -count=1 +``` + +Exit code: `1`. + +Relevant output: `fakeagent: sensitive environment DATABASE_URL was exposed`. + +The GREEN fix replaces the denylist with an explicit minimal allowlist for +process basics, locale variables, and the four named strict-fake controls. + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent -run 'TestSpawn_(CodexUsesStructuredExecContract|DoesNotPassSensitiveEnvironmentToCodex|RejectsStructuredOutputWithoutFindingsField|ParsesFindingsFromFakeAgent|NonZeroExitReturnsError|LogsInvocation)' -count=1 +``` + +Exit code: `0`. + +The fix is committed in source candidate +`910fc54a98e7da644bc5e170281fd935e429692f`. + +## Managed gate path containment + +Trigger: a socket caller submits a valid bare Git repository outside the +daemon's managed `MADE_HOME/gates//gate.git` layout. + +Masking condition: the caller uses a gate created by `made gate init` under the +current Made home. + +Visible symptom: the daemon accepts the unmanaged bare repository and can +schedule the full pipeline against its remote. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestGateAdmitPushRPC_RejectsBareRepoOutsideMadeHome -count=1 +``` + +Exit code: `1` before the fix. + +Relevant output: `gate.admitPush accepted a bare repository outside MADE_HOME`. + +The fix requires an existing, non-symlinked managed gate path before either +`gate.admitPush` or `gate.notifyPush` can schedule work. + +GREEN receipt at source candidate +`03f515b9aeeb8406eec0e4240ab5811fc9110943`: + +```text +go test ./cmd/made -run 'TestGateAdmitPushRPC_(ValidBareRepoAdmitted|RejectsBareRepoOutsideMadeHome)|TestGateAdmitPushCLI_ValidGateExitsZero' -count=1 +ok github.com/douglasjarquin/made/cmd/made 0.526s +``` + +## Pending-check and Codex sandbox contracts + +Trigger: `gh pr checks` returns a pending check with a non-zero aggregate exit +status, or the Codex review adapter invokes `codex exec` without an explicit +read-only sandbox. + +Masking condition: checks are already terminal or a permissive fake accepts +arbitrary Codex flags. + +Visible symptom: Made reruns work that is still pending, or a review agent can +write to the gate worktree despite being invoked for read-only review. + +Commands: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/pipeline/ci -run TestRun_DoesNotRerunPendingChecks -count=1 +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent -run TestSpawn_CodexUsesStructuredExecContract -count=1 +``` + +Both commands failed before the fixes. + +The pending-check RED output reported +`pending check was rerun` with `RerunsUsed:1`. + +The strict Codex fake RED output reported +`want 12 arguments, got 10`. + +The GREEN runs use `bucket/state` pending detection and require +`--sandbox read-only` in the structured Codex invocation. + +## Final-state publication ordering + +Trigger: a run work function completes while the final WAL append is still +pending. + +Masking condition: callers observe only after the persistence call returns or +the durable store never fails. + +Visible symptom: a public snapshot can report `succeeded` before the final +durable record is written, then change to `failed` when persistence fails. + +The final persistence failure contract is covered by +`TestRunManager_FailsRunWhenFinalPersistenceFails`. + +The fix persists a candidate snapshot before replacing the live run snapshot, +so successful terminal state is not publicly visible before durable success. + +The final serialized-publication fix and the pending-check/Codex sandbox fixes +are included in source candidate +`d1dab7c73c3bdf678a668891c17a04d9c34b13c4`. + +## Submission, decision, check-payload, and push-identity containment + +Trigger: an identical submission ID arrives for a different repository, an +approval is submitted before the review stage records a finding, a successful +GitHub check response is empty, or a gate notification names a nonexistent +commit object. + +Masking condition: one repository, a pending finding, a non-empty check set, +and a real post-receive object are always used. + +Visible symptom: Made returns another repository's run, pre-seeds a decision, +accepts an empty successful check payload, or schedules a forged push. + +The RED commands and results were: + +```text +go test ./internal/daemon -run TestRunManager_FindSubmissionDoesNotCrossRepositoryBoundary -count=1 +FAIL: FindSubmission matched a submission from another repository + +go test ./internal/daemon -run TestReviewDecisions_RejectsDecisionWithoutPendingFinding -count=1 +FAIL: accepted a review decision without a pending finding + +go test ./internal/github -run TestPRChecks_RejectsEmptySuccessfulPayload -count=1 +FAIL: PRChecks accepted an empty successful payload + +go test ./cmd/made -run TestGateNotifyPushRPC_RejectsNewSHAThatIsNotTheReceivedRef -count=1 +FAIL: accepted forged new SHA +``` + +The GREEN fixes scope submission identity to repository and branch, require a +running run with a pending finding for managed decisions, reject empty or +non-successful check payloads, and require the new SHA to be a real commit +object in the managed gate. + +## Review-agent environment containment + +Trigger: the review agent inherits a sensitive environment variable and can +return it through findings or an error. + +Masking condition: the environment contains no credential-like variable or +the fake agent ignores inherited environment. + +Visible symptom: the strict fake exits after observing a test secret. + +Command: + +```text +go test ./internal/agent -run TestSpawn_DoesNotPassSensitiveEnvironmentToCodex -count=1 +``` + +Exit code: `1` before the fix. + +Relevant output: `fakeagent: sensitive environment was exposed`. + +The GREEN adapter now filters credential-like environment keys before +launching the read-only Codex task while retaining the structured fake +contract variables. + +The recovery fix leaves the checkpoint and corrupt WAL untouched when loading +fails closed. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestOpenRunManager_PreservesStateAfterRecoveryFailure -count=1 +``` + +Exit code: `0`. diff --git a/evidence/phase-4-review-audit.md b/evidence/phase-4-review-audit.md new file mode 100644 index 0000000..3947049 --- /dev/null +++ b/evidence/phase-4-review-audit.md @@ -0,0 +1,105 @@ +# Phase 4 final review audit + +The reviewed source candidate is +`910fc54a98e7da644bc5e170281fd935e429692f`. + +The committed evidence HEAD reviewed by the lanes is +`3ee7f91f56f6adfe301eb0b69188d8dc5c6ec9e1`. + +The exact requested base remains +`3e19ed9d598a68149da5a73949533e8095ca4403`. + +The source candidate is an ancestor of the evidence HEAD, and the evidence +HEAD differs from the source candidate only in the committed evidence +Markdown refresh. + +## Fresh review lanes + +| Lane | Agent ID | Verdict | Scope receipt | +| --- | --- | --- | --- | +| Gate reviewer | `01a01198-63ca-7440-a10d-badd5c62787e` | APPROVE | Phase 4A/4B source and evidence gates pass; delivery-only check pending before push/PR. | +| QA executor | `01a01198-65c6-7613-aad3-000b65f3ccde` | PASS | Focused race suites, disposable binary/home lifecycle, exact cancellation, restart, CLI, gate ref, and strict fake scenarios pass. | +| Code reviewer | `01a01198-64cd-7932-94c2-e399e851dca5` | APPROVE | No critical or high defect; lock ordering, durable close, cancellation, environment allowlist, containment, and adapters are covered. | +| Security reviewer | `01a01198-66a6-7b20-9f78-ed0cf2e2f46d` | PASS, bounded | No reportable defect; owner-controlled socket authentication, `LC_*`, direct local submission identity, and TOCTOU concerns remain non-reportable residuals. Native scan `f2c6cb94-c8f8-40f7-a350-16b43ee08d26` completed. | +| Evidence explorer | `01a01198-6781-7c90-81b6-468a700a6b00` | PASS for integrity | Exact base/source/evidence lineage, committed RED-to-GREEN receipts, forbidden-scenario limits, and Made-only scope verified; its delivery note remains open until the direct PR exists. | + +The earlier stale-source review findings were reproduced and fixed before +this batch: exact received-ref equality, durable Close serialization, +spooled cancellation, and explicit Codex environment allowlisting. + +No fresh review lane used a real project, real gate, shared Made daemon, +default branch, merge, auto-merge, remote deletion, or another worktree. + +## Local validation bound to the source candidate + +The final build, race/shuffle suite, vet, configured lint, changed-file LSP, +runtime audit, and real disposable binary receipts are recorded in +`evidence/phase-4-final-validation.md`, +`evidence/phase-4-runtime-debug-audit.md`, and +`evidence/phase-4-manual-qa.md`. + +## Review artifacts + +The review lanes wrote raw reports outside the tracked evidence set. +Those raw artifacts were moved to recoverable temporary storage and are not +part of the Made branch. + +## Direct PR delivery receipt + +The branch was pushed only to `origin/cs/made-remediation-continuation`. + +The direct PR was opened with `gh-axi api` REST fallback after the normal +GraphQL create path reported rate limiting. + +```text +gh-axi api POST /repos/douglasjarquin/made/pulls +``` + +PR URL: +`https://github.com/douglasjarquin/made/pull/2` + +The final read-only PR verification returned: + +```text +state=open +base=main +base_sha=34d44be504291482d973c65bd427ba964df5e0e9 +head=cs/made-remediation-continuation +head_sha=c661a43444234cc243e687ce3d6892440ba7221c +merged=false +checks.total_count=0 +``` + +GitHub currently reports `mergeable=false` and +`mergeable_state=dirty`. +This is an explicit residual for the configured merge authority. +The branch was not rebased onto the moving default branch because the task +requires preserving exact base custody. + +No default-branch push, merge, auto-merge, or remote branch deletion occurred. + +## Conflict-repair final review supersession + +The earlier review table above is historical and is superseded for delivery by the fresh review wave bound to source-and-test SHA `12b83a6649b5e198049754f1cb6427d7b0dc51a0`. + +The requested exact base remains `3e19ed9d598a68149da5a73949533e8095ca4403` and is an ancestor of the reviewed SHA. + +| Lane | Agent ID | Verdict | Scope receipt | +| --- | --- | --- | --- | +| Goal and constraint reviewer | `01a011f8-c310-7543-9e71-fe7403dcce30` | PASS, HIGH | Exact ancestry, all binding Made-only criteria, local final commands, and direct PR state passed. | +| Bounded CLI QA executor | `01a011f8-c408-7c32-a3a5-13fd4f7a85b9` | PASS | Capabilities, obsolete status, disposable daemon start/status/list/missing-ID/stop, and cleanup passed on the exact SHA. | +| Code reviewer | `01a011f8-c4e3-71b0-b0b8-6f25448d3db6` | PASS, no blockers | Compaction candidate overlay, restart regression, strict adapters, evidence CAS, and lint passed; the persistence module size is a non-blocking watch item. | +| Bounded security reviewer | `01a01201-3c47-72e0-9711-c6dba6334a97` | PASS, severity NONE | Agent, evidence, WAL, managed gate path, socket, and public CLI boundaries have no HIGH or CRITICAL issue. | +| Context and delivery reviewer | `01a01203-31cb-7562-bd18-c08105de5b52` | PASS | Exact base ancestry and direct-PR custody passed; GitHub PR base ref `main` is correctly treated as a branch ref, not a detached required base SHA. | + +The first context read during this wave was superseded after hosted checks completed and after the brief's distinction between worktree base SHA and PR base branch ref was reverified. + +The hosted check `build-test-lint` for exact head `12b83a6649b5e198049754f1cb6427d7b0dc51a0` completed with conclusion `success` in check run `95537594230`. + +The final read-only PR state is `state=open`, `merged=false`, `head=cs/made-remediation-continuation`, `head_sha=12b83a6649b5e198049754f1cb6427d7b0dc51a0`, `base=main`, `base_sha=34d44be504291482d973c65bd427ba964df5e0e9`, `mergeable=true`, `mergeable_state=clean`, and `auto_merge=null`. + +The branch was pushed only to `origin/cs/made-remediation-continuation`. + +The final review artifacts were moved to recoverable temporary storage and are not part of the Made branch. + +The review lane source receipt is intentionally bound to `12b83a6649b5e198049754f1cb6427d7b0dc51a0`; follow-up commit `e7cb50ab363da748a04f6c47c4a4b4cc7123d614` contains only this evidence/ledger update and no source or test changes. diff --git a/evidence/phase-4-runtime-debug-audit.md b/evidence/phase-4-runtime-debug-audit.md new file mode 100644 index 0000000..34b336c --- /dev/null +++ b/evidence/phase-4-runtime-debug-audit.md @@ -0,0 +1,295 @@ +# Phase 4 runtime and security audit + +This audit covers the Made source candidate +`910fc54a98e7da644bc5e170281fd935e429692f`. + +The exact merge-base is +`3e19ed9d598a68149da5a73949533e8095ca4403`. + +No shared Made daemon, real gate, real project, default branch, remote branch, +or unrelated worktree was used. + +## Hypotheses and counterfactuals + +### A: durable lifecycle state could be lost or replayed incorrectly + +The initiating trigger would be cancellation, restart, a torn final WAL append, +or WAL growth during queued and awaiting-merge runs. + +The masking condition would be a single live daemon process with no queue +cancellation, restart, or persistence-boundary exercise. + +The visible symptom would be a queued run starting after cancellation, an +awaiting-merge run becoming terminal, a torn record aborting recovery, or an +unbounded WAL retaining every intermediate snapshot. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/daemon -run "Test(RunManager_CancelQueuedRunNeverStartsWork|RunManager_RestoresDurableSnapshotAfterRestart|RunManager_IgnoresTornFinalWALRecord|RunManager_WALRetentionIsBounded|ReviewDecisions_RestoreAndRejectConflict)" -count=5 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/internal/daemon 13.040s`. + +Counterfactual result: the focused race suite passed, including queued +cancellation, exact restart recovery, torn-tail tolerance, retention bounds, +and first-wins decision conflict behavior. + +### B: concurrent evidence publication could lose one run + +The initiating trigger would be concurrent writers racing on the orphan +evidence branch reference. + +The masking condition would be serialized pipeline execution or a single +writer test. + +The visible symptom would be one writer failing its compare-and-swap update or +one completed run missing from the retained evidence history. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/evidence -run TestOrphanBranchStore_ConcurrentWritesRetainBothRuns -count=10 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/internal/evidence 3.733s`. + +Counterfactual result: both concurrent writers were retained across ten race +repetitions. + +### C: strict external fakes could mask unsupported invocations + +The initiating trigger would be an obsolete GitHub command, a PR URL passed to +a workflow-run operation, an unsupported Claude path, or malformed Codex +structured output. + +The masking condition would be permissive process fakes that ignore arguments +and accept arbitrary output. + +The visible symptom would be local tests passing while a real external tool +rejects the invocation or returns an ambiguous result. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent ./internal/github ./internal/pipeline/ci ./internal/pipeline/review -run "Test(Spawn_|StrictFakeGH|PRChecks|Run_)" -count=3 +``` + +Exit code: `0`. + +Relevant output: `ok` for `internal/agent`, `internal/github`, +`internal/pipeline/ci`, and `internal/pipeline/review`. + +Counterfactual result: the strict fake suites accepted only the supported +GitHub and Codex contracts and rejected obsolete or invalid boundaries. + +### D: reviewer auto-fix could stage unrelated files + +The initiating trigger would be an auto-fixable reviewer patch in a worktree +that also contains unrelated modifications. + +The masking condition would be a clean fixture containing only the patch. + +The visible symptom would be an auto-fix commit containing files outside the +review patch. + +Command: + +```text +if rg -n "git add -A|git add --all|git add \\." internal/pipeline/review; then exit 1; else printf "%s\\n" "no broad reviewer staging invocation"; fi +``` + +Exit code: `0`. + +Relevant output: `no broad reviewer staging invocation`. + +Counterfactual result: reviewer containment uses the indexed patch file set +and has no broad staging invocation. + +### E: public lifecycle boundary could expose obsolete or ambiguous status + +The initiating trigger would be a caller using the removed global status +command or omitting the exact run identity. + +The masking condition would be an in-process test that bypasses the CLI and +socket boundary. + +The visible symptom would be a global-latest lookup, an invented run mutation, +or an obsolete command silently succeeding. + +Command: + +```text +rg -n "status is obsolete|run status" cmd/made +``` + +Exit code: `0`. + +Relevant output includes +`made: status is obsolete; use made run status ` and the exact +`made run status` handler paths. + +Counterfactual result: public status requires an exact run ID and the obsolete +global command rejects with exit code 2, as proven by the disposable binary +scenario in `evidence/phase-4-manual-qa.md`. + +## Final local validation observed at this source candidate + +Command sequence: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null git diff --check +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go build ./... +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race -shuffle=on -count=1 ./... +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go vet ./... +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null golangci-lint run ./... +``` + +Exit code: `0` for the sequence. + +Relevant output: every package completed with `ok`, and `golangci-lint` +reported `0 issues`. + +Changed-file LSP diagnostics were requested for all 50 changed Go files with +severity `all`. + +Result: `No diagnostics found` for every checked file. + +The review-work lanes and final ledger update remain separate final-delivery +receipts and are bound to the same exact source SHA. + +## Follow-up RED-to-GREEN results at the 604 source candidate + +The three follow-up RED tests were fixed in Made and rerun at source candidate +`60420902ea5b1ed434f57c86ebb0e85be7be5281`. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./cmd/made -run TestReview_MultipleFindingsInOneStageUseOneDecision -count=5 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/cmd/made 1.981s`. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/evidence -run TestInRepoStoreRejectsSymlinkedEvidenceDirectory -count=5 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/internal/evidence 1.215s`. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/daemon -run "TestRunManager_(UpdateStagesRollsBackOnPersistenceFailure|FailsRunWhenFinalPersistenceFails)" -count=5 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/internal/daemon 1.313s`. + +The strict external boundary rerun also exited `0` for + +```text +go test ./internal/agent ./internal/github ./internal/pipeline/ci ./internal/pipeline/review -run "Test(Spawn_|StrictFakeGH|PRChecks|Run_)" -count=3 +``` + +The four package results were `ok`. + +The reviewer containment source check exited `0` with +`no broad reviewer staging invocation`. + +The managed gate-path boundary was also exercised by the focused command + +```text +go test ./cmd/made -run 'TestGateAdmitPushRPC_(ValidBareRepoAdmitted|RejectsBareRepoOutsideMadeHome)|TestGateAdmitPushCLI_ValidGateExitsZero' -count=1 +``` + +which exited `0`. + +The final received-ref equality boundary was exercised with the strict +disposable gate fixture. +The counterfactual RED and restored GREEN receipts are recorded in +`evidence/phase-4-red-followups.md`. + +Command: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./cmd/made -run 'TestGateNotifyPushRPC_(NormalFeatureBranchPushCreatesRun|RejectsNewSHAThatIsNotTheReceivedRef|RejectsExistingUnrelatedSHA|SupersededPushValidatesNewestSHA)' -count=5 +``` + +Exit code: `0`. + +Relevant output: `ok github.com/douglasjarquin/made/cmd/made 10.699s`. + +The follow-up lifecycle boundary checks at the same exact source candidate +also exited `0`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/daemon -run 'TestRunManager_(CancelSpooledQueuedRunTransitionsTerminal|CancelQueuedRunNeverStartsWork|CloseDoesNotDiscardConcurrentDurableMutation|FailsRunWhenFinalPersistenceFails|OpenRunManager_PreservesStateAfterRecoveryFailure)' -count=5 +ok github.com/douglasjarquin/made/internal/daemon 2.932s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./cmd/made -run TestGateNotifyPushRPC_RejectsStaleAncestorSHA -count=1 +ok github.com/douglasjarquin/made/cmd/made 1.022s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CancelSpooledQueuedRunTransitionsTerminal -count=1 +ok github.com/douglasjarquin/made/internal/daemon 0.471s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/daemon -run TestRunManager_CloseDoesNotDiscardConcurrentDurableMutation -count=1 +ok github.com/douglasjarquin/made/internal/daemon 0.297s +``` + +The real binary scenario in `evidence/phase-4-manual-qa.md` additionally +proved public spooled cancellation and durable terminal-state recovery after +graceful daemon restart. + +The final review-agent environment boundary also exited `0`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/agent -run 'TestSpawn_(CodexUsesStructuredExecContract|DoesNotPassSensitiveEnvironmentToCodex|RejectsStructuredOutputWithoutFindingsField|ParsesFindingsFromFakeAgent|NonZeroExitReturnsError|LogsInvocation)' -count=5 +ok github.com/douglasjarquin/made/internal/agent 1.971s +``` + +The allowlist source fix is committed at +`910fc54a98e7da644bc5e170281fd935e429692f`. + + +## Final boundary audit at source candidate 910fc54 + +The following focused checks all exited `0` at the final source candidate +`910fc54a98e7da644bc5e170281fd935e429692f`: + +```text +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/daemon -run 'Test(RunManager_CancelQueuedRunNeverStartsWork|RunManager_CancelSpooledQueuedRunTransitionsTerminal|RunManager_RestoresDurableSnapshotAfterRestart|RunManager_IgnoresTornFinalWALRecord|RunManager_WALRetentionIsBounded|ReviewDecisions_RestoreAndRejectConflict|RunManager_FindSubmissionDoesNotCrossRepositoryBoundary|ReviewDecisions_RejectsDecisionWithoutPendingFinding|RunManager_CloseDoesNotDiscardConcurrentDurableMutation|RunManager_FailsRunWhenFinalPersistenceFails|OpenRunManager_PreservesStateAfterRecoveryFailure)' -count=5 +ok github.com/douglasjarquin/made/internal/daemon 13.774s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/evidence -run TestOrphanBranchStore_ConcurrentWritesRetainBothRuns -count=10 +ok github.com/douglasjarquin/made/internal/evidence 2.472s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test ./internal/agent ./internal/github ./internal/pipeline/ci ./internal/pipeline/review -run 'Test(Spawn_|StrictFakeGH|PRChecks|Run_)' -count=3 +ok github.com/douglasjarquin/made/internal/agent 1.760s +ok github.com/douglasjarquin/made/internal/github 2.326s +ok github.com/douglasjarquin/made/internal/pipeline/ci 9.152s +ok github.com/douglasjarquin/made/internal/pipeline/review 4.995s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./cmd/made -run 'TestGateNotifyPushRPC_(NormalFeatureBranchPushCreatesRun|RejectsNewSHAThatIsNotTheReceivedRef|RejectsExistingUnrelatedSHA|RejectsStaleAncestorSHA|SupersededPushValidatesNewestSHA)' -count=5 +ok github.com/douglasjarquin/made/cmd/made 12.831s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/agent -run 'TestSpawn_(CodexUsesStructuredExecContract|DoesNotPassSensitiveEnvironmentToCodex|RejectsStructuredOutputWithoutFindingsField|ParsesFindingsFromFakeAgent|NonZeroExitReturnsError|LogsInvocation)' -count=5 +ok github.com/douglasjarquin/made/internal/agent 1.971s + +env SSH_AUTH_SOCK= GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=/dev/null go test -race ./internal/github -run TestPRChecks_RejectsEmptySuccessfulPayload -count=5 +ok github.com/douglasjarquin/made/internal/github 1.862s +``` + +The reviewer containment source check also exited `0` with +`no broad reviewer staging invocation`. diff --git a/evidence/ulw-notepad-made-remediation-continuation.md b/evidence/ulw-notepad-made-remediation-continuation.md new file mode 100644 index 0000000..4bf9984 --- /dev/null +++ b/evidence/ulw-notepad-made-remediation-continuation.md @@ -0,0 +1,104 @@ +# Ultrawork Notepad — Continue Made remediation from exact base + +Started: 2026-08-17T00:00:00-04:00 + +## Plan (exhaustively detailed) + +1. Prove the isolated worktree, exact base, source and prior-worktree custody, installed Made and required tools, live daemon state, Herdr lab isolation, and the canonical continuation checklist. +2. Map every still-valid continuation hypothesis to a Made-owned contract, test seam, strict external fake, RED evidence, GREEN implementation, real-surface QA scenario, cleanup receipt, and phase-local evidence artifact. +3. Implement external-tool contracts for GitHub checks and Codex review, explicitly reject unsupported Claude behavior where required, and verify each focused contract. +4. Implement durability and lifecycle fixes as vertical RED-to-GREEN slices, preserving durable run identity, stage, decisions, evidence, restart, configuration, and reviewer containment. +5. Update the canonical Made plan with a linked continuation section and phase-scoped evidence, run Made-only compatibility/build/test/vet/lint validation, perform manual QA through the allowed surfaces, and reconcile custody. +6. Commit verified increments from the exact base, render any bossless decision record if present, push only the task branch, open the direct PR with gh-axi, and report exact custody. + +## Success criteria + QA scenarios + +- Tier: HEAVY because this change touches external integrations, authentication/check semantics, durable lifecycle state, concurrency/transaction ordering, trusted configuration, and review containment. +- Criterion 1: exact-base and custody baseline is proven by `pwd -P`, `git rev-parse --show-toplevel`, `git branch --show-current`, `git rev-parse HEAD`, Made binary revision/help, read-only daemon status, tool checks, Herdr helper provisioning, and plan/brief inspection; PASS requires exact task worktree, exact base, preserved prior dirty count, no shared-daemon mutation, and captured phase-0 evidence. +- Criterion 2: strict GitHub and Codex external contracts pass via focused Go tests and the real Made binary against strict fakes; RED must fail on obsolete or invalid invocations and GREEN must accept only supported structured fields and outputs, with captured command output. +- Criterion 3: durable lifecycle contracts pass via focused Go integration tests and disposable Made homes, repositories, and process fixtures; RED/GREEN plus CLI/socket observations must prove submission refresh, exact run identity, decision timing/conflicts, cancellation, awaiting_merge success, idle/daemon-down distinction, fixed stages/current stage, evidence durability/retention/torn-tail recovery, restart, config enforcement, and reviewer containment. +- Criterion 4: local Made-only compatibility/build/test/vet/lint passes, changed scope is captured from the exact base, plan/evidence/checklist conventions are preserved, and a direct-PR branch is pushed/open with no forbidden repository, daemon, or pipeline action. +- Real-surface scenario for the CLI/data deliverable: run the real Made binary with disposable HOME/config/repository and strict fakes; PASS is exact structured JSON state/identity and expected exit code, captured in evidence. +- STOP: I'll stop right away when every requested contract has RED-to-GREEN evidence and real-surface PASS, all spawned resources have cleanup receipts, the branch is committed from the exact base, pushed, and its direct PR is open. + +## Now + +Phase 3 lifecycle and durability slices are GREEN in focused daemon, CLI, +configuration, evidence, reviewer-containment, orchestrator, and rebase tests; +disposable real-binary QA and final validation remain. + +## Todo + +- Read applicable skill bodies and record their use. +- Finish continuation gap discovery from brief/plan and source symbols. +- Provision named Herdr lab only after baseline and trap setup. +- Add and run RED tests before production edits. +- Implement minimal fixes with immediate GREEN and QA. +- Capture `evidence/phase-2-external-contracts.md` for the GitHub/CI GREEN slice. +- Update plan and evidence and run final validation. +- Commit, push branch, open direct PR, and append the done receipt. + +## Findings + +- Task worktree is `/Users/douglasjarquin/.herdr/worktrees/made/cs-made-remediation-continuation`. +- Branch is `cs/made-remediation-continuation`; HEAD and required base are `3e19ed9d598a68149da5a73949533e8095ca4403`. +- Task worktree was clean at bootstrap. +- Prior Made remediation worktree exists and has six porcelain entries; only existence, count, and HEAD were checked, not untracked artifact contents. +- Codegraph is available for this Made project; initial exploration found current review, decision, run-state, CI, and agent call surfaces. +- Made binary is `/Users/douglasjarquin/.local/bin/made`; `made --version`, `made version`, and `made --help` are unsupported and exit 2, so its revision/help contract needs discovery. +- Go is `go1.26.6 darwin/arm64`; git is 2.55.0; `gh-axi`, `herdr`, `codex`, and `golangci-lint` are installed; `chrome-devtools-axi` is not on PATH. +- The current Capo brief at `/Users/douglasjarquin/.consigliere/capos/made/data/made-remediation-continuation/brief.md` was reread before advancing; its binding gates are public structured contract, lifecycle and durability, evidence, semantic config, strict external compatibility, disposable live scenarios, and final validation. +- The current Capo brief forbids real-project validation, gate initialization, run submission, shared Made daemon lifecycle changes, default-branch pushes, merges, auto-merge, remote-branch deletion, and ask-user decisions. +- Phase 0 evidence is `evidence/phase-0-grounding-made-remediation-continuation.md`. +- Sparse supervisor receipt was appended as `working: [key=made-remediation-continuation] phase 0 grounding complete`. +- The named Herdr lab session is `cs-lab-made-remediation-9714-1438`, provisioned through the required helper with the EXIT teardown trap installed first. +- Memory-derived prior-run contract facts identify the intended public Made surface as `made capabilities --json`, `made run submit/status/list/cancel`, `made review decide`, and `made doctor --json`; exact run IDs and structured JSON are mandatory, and obsolete predecessor/global-latest behavior is rejected. +- Memory-derived prior-run facts also identify durable state/WAL and submission-spool replay, strict config, evidence redaction/retention, current Codex invocation, GitHub check/run handling, review containment, and real-binary compatibility as the Phase 1–3 continuation baseline to reproduce from source, without opening or copying the prior worktree. +- Read-only discovery lane 1 found `internal/github/client.go:70-120` uses mergeability and PR URLs for run operations, and `internal/agent/spawn.go:20-44` uses one undocumented invocation and loose raw JSON. +- Read-only discovery lane 2 found in-memory run state, queued cancellation loss, awaiting-merge terminal-event mismatch, missing current stage, overwriteable decisions, shallow snapshot slices, and lossy non-replayable mailbox behavior. +- Read-only discovery lane 3 found semantic config mostly satisfies its trust boundary, but unknown YAML switches are accepted, evidence writes are non-atomic, concurrent orphan publication loses a run, infrastructure failures can omit stage results, and reviewer auto-fix uses broad `git add -A`. +- Exact RED evidence is `evidence/phase-1-red-made-remediation-continuation.md`. +- Phase 2 GitHub and CI GREEN evidence is `evidence/phase-2-external-contracts.md`. +- The supported GitHub contract is `gh pr checks --json name,state,bucket,link`, + with numeric workflow run IDs extracted from check links and explicit auth, + check, log, and rerun errors. +- The supported Codex adapter invokes `exec --json --output-schema + --output-last-message --ephemeral -C ` and + parses only the required structured findings object. +- Claude is explicitly rejected at the Made agent boundary because the current + supported structured contract is Codex-only; no generic agent compatibility + shim was added. +- Focused agent and review happy-path GREEN evidence is in + `evidence/phase-2-external-contracts.md`. +- Phase 3 focused lifecycle, durability, evidence, configuration, reviewer, + orchestrator, and rebase evidence is in + `evidence/phase-3-lifecycle-durability.md`. +- The durable run store uses a fsynced JSONL WAL plus atomic checkpoint and + bounded compaction; a final malformed WAL record is ignored as a torn tail, + while malformed non-final records fail open/recovery closed. +- The public run surface is `capabilities --json`, exact-ID + `run submit/status/list/cancel`, `review.decide`, and structured `doctor + --json`; the obsolete global-latest `status` command is rejected. +- `awaiting_merge` is non-terminal until an explicit `succeeded` transition, + and daemon shutdown cancels only queued/running execution while preserving + durable awaiting-merge records. +- Real Made binary manual QA passed against a disposable home at + `evidence/phase-4-manual-qa.md`: capabilities, queued pre-drain submission, + exact-ID status/list, obsolete-status rejection, doctor JSON, daemon restart + recovery, and strict exact-ID error behavior were observed. +- The isolated Herdr helper probe confirmed named session + `cs-lab-made-remediation-9714-1438` is running and compatible; final teardown + remains pending until all validation and delivery work is complete. +- Final local build, race/shuffle suite, vet, lint, changed-Go-file diagnostics, + and diff checks passed; the receipts are in + `evidence/phase-4-final-validation.md`. +- LSP diagnostics for the changed GitHub/CI production files and focused tests + reported no errors or warnings; one non-blocking `stringsseq` hint remains in + `internal/pipeline/ci/ci_contract_test.go`. +- Baseline isolated suite still has a pre-existing `internal/pipeline/rebase/TestRun_CleanRebaseProceeds` failure after Git-signing isolation; it is not hidden and remains a validation item. +- Installed Made contract discovery from the binary reports `made capabilities --json` with `schema_version`, `protocol_version`, and commands `run.submit`, `run.status`, `run.list`, `run.cancel`, `review.decide`, `doctor`; run states include `queued`, `running`, `awaiting_review`, `awaiting_merge`, `succeeded`, `failed`, `canceled`, and `superseded`; `execution_finished` is independent. + +## Learnings + +- Never inspect the retained prior worktree's untracked evidence. +- Do not use the shared Made daemon; use only read-only state checks and the named Herdr lab helper for task-specific lifecycle experiments. diff --git a/internal/agent/agent_contract_test.go b/internal/agent/agent_contract_test.go new file mode 100644 index 0000000..6e83961 --- /dev/null +++ b/internal/agent/agent_contract_test.go @@ -0,0 +1,99 @@ +package agent_test + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/douglasjarquin/made/internal/agent" + "github.com/douglasjarquin/made/internal/agent/agenttest" +) + +func TestSpawn_CodexUsesStructuredExecContract(t *testing.T) { + bin := agenttest.Build(t) + worktree := agentWorktree(t) + scenarioPath := filepath.Join(t.TempDir(), "scenario.json") + if err := os.WriteFile(scenarioPath, []byte(`{"findings":[]}`), 0o644); err != nil { + t.Fatalf("write scenario: %v", err) + } + logPath := filepath.Join(t.TempDir(), "agent.log") + + if _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ + WorktreePath: worktree, + BinaryPath: bin, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + "FAKE_AGENT_LOG_FILE=" + logPath, + }, + }); err != nil { + t.Fatalf("Spawn: %v", err) + } + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read invocation log: %v", err) + } + for _, token := range []string{"exec", "--json", "--output-schema", "--output-last-message", "--sandbox", "read-only", "--ephemeral", "-C"} { + if !strings.Contains(string(data), token) { + t.Fatalf("expected Codex structured invocation token %q, got %s", token, data) + } + } +} + +func TestSpawn_DoesNotPassSensitiveEnvironmentToCodex(t *testing.T) { + bin := agenttest.Build(t) + worktree := agentWorktree(t) + scenarioPath := filepath.Join(t.TempDir(), "scenario.json") + if err := os.WriteFile(scenarioPath, []byte(`{"findings":[]}`), 0o644); err != nil { + t.Fatalf("write scenario: %v", err) + } + + if _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ + WorktreePath: worktree, + BinaryPath: bin, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + "MADE_TEST_SECRET=must-not-reach-review-agent", + "DATABASE_URL=must-not-reach-review-agent", + "COOKIE=must-not-reach-review-agent", + "JWT_KEY=must-not-reach-review-agent", + "KUBECONFIG=/must-not-reach-review-agent", + }, + }); err != nil { + t.Fatalf("Spawn exposed sensitive environment: %v", err) + } +} + +func TestSpawn_RejectsStructuredOutputWithoutFindingsField(t *testing.T) { + bin := agenttest.Build(t) + scenarioPath := filepath.Join(t.TempDir(), "invalid.json") + if err := os.WriteFile(scenarioPath, []byte(`{"unexpected":[]}`), 0o644); err != nil { + t.Fatalf("write invalid scenario: %v", err) + } + + _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ + WorktreePath: agentWorktree(t), + BinaryPath: bin, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + }, + }) + if err == nil { + t.Fatal("expected schema-invalid structured output to fail closed") + } +} + +func TestFindingsJSONRoundTripUsesArrayShape(t *testing.T) { + data, err := json.Marshal(agent.Findings{Findings: []agent.Finding{}}) + if err != nil { + t.Fatalf("marshal findings: %v", err) + } + if string(data) != `{"findings":[]}` { + t.Fatalf("unexpected structured findings shape: %s", data) + } +} diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 26db893..258046f 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -61,10 +61,10 @@ func TestSpawn_ParsesFindingsFromFakeAgent(t *testing.T) { }, }) - findings, err := agent.Spawn(context.Background(), agent.KindClaude, agent.SpawnParams{ + findings, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ WorktreePath: agentWorktree(t), BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{"FAKE_AGENT_KIND=codex", "FAKE_AGENT_SCENARIO=" + scenarioPath}, }) if err != nil { t.Fatalf("Spawn: %v", err) @@ -86,7 +86,7 @@ func TestSpawn_NonZeroExitReturnsError(t *testing.T) { _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ WorktreePath: agentWorktree(t), BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_EXIT_CODE=1"}, + ExtraEnv: []string{"FAKE_AGENT_KIND=codex", "FAKE_AGENT_EXIT_CODE=1"}, }) if err == nil { t.Fatal("expected an error for a non-zero fakeagent exit") @@ -101,10 +101,11 @@ func TestSpawn_LogsInvocation(t *testing.T) { scenarioPath := writeScenario(t, agent.Findings{}) logPath := filepath.Join(t.TempDir(), "invocations.log") - if _, err := agent.Spawn(context.Background(), agent.KindClaude, agent.SpawnParams{ + if _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ WorktreePath: agentWorktree(t), BinaryPath: bin, ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", "FAKE_AGENT_SCENARIO=" + scenarioPath, "FAKE_AGENT_LOG_FILE=" + logPath, }, @@ -120,3 +121,13 @@ func TestSpawn_LogsInvocation(t *testing.T) { t.Fatalf("expected invocation log entry, got %q", data) } } + +func TestSpawn_RejectsUnsupportedClaudeContract(t *testing.T) { + _, err := agent.Spawn(context.Background(), agent.KindClaude, agent.SpawnParams{ + WorktreePath: agentWorktree(t), + BinaryPath: agenttest.Build(t), + }) + if err == nil || !strings.Contains(err.Error(), "structured task contract is unsupported") { + t.Fatalf("expected explicit unsupported Claude error, got %v", err) + } +} diff --git a/internal/agent/findings.go b/internal/agent/findings.go index 541609b..3493415 100644 --- a/internal/agent/findings.go +++ b/internal/agent/findings.go @@ -1,5 +1,11 @@ package agent +import ( + "bytes" + "encoding/json" + "fmt" +) + type FindingKind string const ( @@ -15,6 +21,42 @@ type Finding struct { Paths []string `json:"paths,omitempty"` } +func (f *Finding) UnmarshalJSON(data []byte) error { + var wire struct { + Kind *FindingKind `json:"kind"` + Description *string `json:"description"` + Patch *string `json:"patch"` + Paths []string `json:"paths"` + } + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&wire); err != nil { + return err + } + if wire.Kind == nil || wire.Description == nil { + return fmt.Errorf("finding requires kind and description") + } + f.Kind = *wire.Kind + f.Description = *wire.Description + f.Patch = "" + if wire.Patch != nil { + f.Patch = *wire.Patch + } + f.Paths = append([]string(nil), wire.Paths...) + return nil +} + type Findings struct { Findings []Finding `json:"findings"` } + +func (f Findings) MarshalJSON() ([]byte, error) { + findings := f.Findings + if findings == nil { + findings = []Finding{} + } + type payload struct { + Findings []Finding `json:"findings"` + } + return json.Marshal(payload{Findings: findings}) +} diff --git a/internal/agent/remediation_contract_test.go b/internal/agent/remediation_contract_test.go index 7c77b1e..0ad6e5a 100644 --- a/internal/agent/remediation_contract_test.go +++ b/internal/agent/remediation_contract_test.go @@ -35,26 +35,21 @@ func TestSpawn_CodexUsesSupportedExecStructuredContract(t *testing.T) { contents := strings.Join([]string{ "#!/bin/sh", "set -eu", - "printf '%s\\n' \"$@\" > \"$STRICT_CODEX_LOG\"", + "printf '%s\\n' \"$@\" > \"$FAKE_AGENT_LOG_FILE\"", "[ \"$1\" = \"exec\" ]", - "[ \"$2\" = \"--cd\" ]", - "[ \"$3\" != \"$STRICT_CODEX_WORKTREE\" ]", - "[ -d \"$3\" ]", - "[ \"$(git -C \"$3\" rev-parse HEAD)\" = \"$STRICT_CODEX_HEAD\" ]", - "if (umask 077; : > \"$3/.agent-write-probe\") 2>/dev/null; then exit 1; fi", - "shift 3", - "has_json=0", - "has_schema=0", - "while [ \"$#\" -gt 0 ]; do", - " case \"$1\" in", - " --json) has_json=1 ;;", - " --output-schema) has_schema=1; shift; test -f \"$1\" ;;", - " esac", - " shift", - "done", - "[ \"$has_json\" -eq 1 ]", - "[ \"$has_schema\" -eq 1 ]", + "[ \"$2\" = \"--json\" ]", + "[ \"$3\" = \"--output-schema\" ]", + "[ -f \"$4\" ]", + "[ \"$5\" = \"--output-last-message\" ]", + "[ \"$7\" = \"--sandbox\" ]", + "[ \"$8\" = \"read-only\" ]", + "[ \"$9\" = \"--ephemeral\" ]", + "[ \"${10}\" = \"-C\" ]", + "[ -d \"${11}\" ]", + "[ \"$(git -C \"${11}\" rev-parse HEAD)\" = " + shellQuote(head) + " ]", + "if (umask 077; : > \"${11}/.agent-write-probe\") 2>/dev/null; then exit 1; fi", "test -z \"${MADE_REVIEW_SECRET:-}\"", + "printf '%s\\n' '{\"findings\":[]}' > \"$6\"", "printf '%s\\n' '{\"findings\":[]}'", "", }, "\n") @@ -66,9 +61,7 @@ func TestSpawn_CodexUsesSupportedExecStructuredContract(t *testing.T) { WorktreePath: worktree, BinaryPath: script, ExtraEnv: []string{ - "STRICT_CODEX_LOG=" + logPath, - "STRICT_CODEX_WORKTREE=" + worktree, - "STRICT_CODEX_HEAD=" + head, + "FAKE_AGENT_LOG_FILE=" + logPath, }, }) if err != nil { @@ -88,7 +81,7 @@ func TestSpawn_CodexUsesSupportedExecStructuredContract(t *testing.T) { if len(args) > 0 && args[0] == "review" { t.Fatalf("Codex invocation used obsolete review command: %s", data) } - if _, err := os.Stat(args[2]); !os.IsNotExist(err) { + if _, err := os.Stat(args[10]); !os.IsNotExist(err) { t.Fatalf("review clone was not cleaned up: %v", err) } if _, err := os.Stat(hookMarker); !os.IsNotExist(err) { @@ -112,10 +105,10 @@ func TestSpawn_RejectsReviewSymlinkThatEscapesClone(t *testing.T) { t.Fatalf("write scenario: %v", err) } - _, err := agent.Spawn(context.Background(), agent.KindClaude, agent.SpawnParams{ + _, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ WorktreePath: worktree, BinaryPath: agenttest.Build(t), - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{"FAKE_AGENT_KIND=codex", "FAKE_AGENT_SCENARIO=" + scenarioPath}, }) if err == nil || !strings.Contains(err.Error(), "escapes review worktree") { t.Fatalf("expected escaping review symlink rejection, got %v", err) @@ -130,15 +123,15 @@ func TestSpawn_ContainsReviewerFromSourceWorktree(t *testing.T) { } gitAgent(t, worktree, "add", "source.txt") gitAgent(t, worktree, "commit", "-q", "-m", "add source fixture") - marker := filepath.Join(t.TempDir(), "source-mutated") script := filepath.Join(t.TempDir(), "containment-codex") contents := strings.Join([]string{ "#!/bin/sh", "set -eu", - "if cat \"$STRICT_CODEX_SOURCE/source.txt\" >/dev/null 2>&1; then exit 41; fi", - "if chmod -R u+w \"$STRICT_CODEX_SOURCE\" 2>/dev/null; then", - " if : > \"$STRICT_CODEX_SOURCE/source-mutated\" 2>/dev/null; then printf mutated > \"$STRICT_CODEX_MARKER\"; exit 42; fi", + "if cat " + shellQuote(filepath.Join(worktree, "source.txt")) + " >/dev/null 2>&1; then exit 41; fi", + "if chmod -R u+w " + shellQuote(worktree) + " 2>/dev/null; then", + " if : > " + shellQuote(filepath.Join(worktree, "source-mutated")) + " 2>/dev/null; then exit 42; fi", "fi", + "printf '%s\\n' '{\"findings\":[]}' > \"$6\"", "printf '%s\\n' '{\"findings\":[]}'", "", }, "\n") @@ -149,10 +142,7 @@ func TestSpawn_ContainsReviewerFromSourceWorktree(t *testing.T) { findings, err := agent.Spawn(context.Background(), agent.KindCodex, agent.SpawnParams{ WorktreePath: worktree, BinaryPath: script, - ExtraEnv: []string{ - "STRICT_CODEX_SOURCE=" + worktree, - "STRICT_CODEX_MARKER=" + marker, - }, + ExtraEnv: nil, }) if err != nil { t.Fatalf("Spawn should contain reviewer without changing source: %v", err) @@ -160,9 +150,6 @@ func TestSpawn_ContainsReviewerFromSourceWorktree(t *testing.T) { if len(findings.Findings) != 0 { t.Fatalf("expected empty findings, got %+v", findings) } - if _, err := os.Stat(marker); !os.IsNotExist(err) { - t.Fatalf("reviewer escaped into source worktree: %v", err) - } if _, err := os.Stat(filepath.Join(worktree, "source-mutated")); !os.IsNotExist(err) { t.Fatalf("reviewer modified source worktree: %v", err) } diff --git a/internal/agent/spawn.go b/internal/agent/spawn.go index f542f6c..903f349 100644 --- a/internal/agent/spawn.go +++ b/internal/agent/spawn.go @@ -1,7 +1,6 @@ package agent import ( - "bufio" "bytes" "context" "encoding/json" @@ -19,12 +18,16 @@ type SpawnParams struct { WorktreePath string BinaryPath string ExtraEnv []string + Task string Timeout time.Duration } const defaultSpawnTimeout = 30 * time.Minute func Spawn(ctx context.Context, kind Kind, params SpawnParams) (Findings, error) { + if kind != KindCodex { + return Findings{}, fmt.Errorf("agent: %s structured task contract is unsupported", kind) + } binary := params.BinaryPath if binary == "" { binary = kind.binaryName() @@ -36,7 +39,7 @@ func Spawn(ctx context.Context, kind Kind, params SpawnParams) (Findings, error) } defer cleanupReview() - args, cleanup, err := invocation(kind, reviewPath) + args, cleanup, outputPath, err := invocation(kind, reviewPath, params.Task) if err != nil { return Findings{}, err } @@ -64,7 +67,14 @@ func Spawn(ctx context.Context, kind Kind, params SpawnParams) (Findings, error) return Findings{}, fmt.Errorf("agent: %s (%s) exited %d: %s", kind, binary, result.ExitCode, evidence.RedactString(string(result.Stderr))) } - findings, err := decodeFindings(result.Stdout) + data := result.Stdout + if outputPath != "" { + data, err = os.ReadFile(outputPath) + if err != nil { + return Findings{}, fmt.Errorf("agent: read structured output from %s: %w", kind, err) + } + } + findings, err := strictFindings(data) if err != nil { return Findings{}, fmt.Errorf("agent: parse findings from %s: %w: stdout=%s", kind, err, evidence.RedactString(string(result.Stdout))) } @@ -72,98 +82,49 @@ func Spawn(ctx context.Context, kind Kind, params SpawnParams) (Findings, error) } func reviewEnvironmentForDir(extra []string, dir string) []string { - filtered := make([]string, 0, len(os.Environ())+len(extra)) - for _, entry := range os.Environ() { + entries := append(append([]string(nil), os.Environ()...), extra...) + filtered := make([]string, 0, len(entries)+5) + for _, entry := range entries { name, _, ok := strings.Cut(entry, "=") - if ok && !sensitiveEnvironmentName(name) && !reviewPathEnvironmentName(name) && (dir == "" || name != "PWD") { - filtered = append(filtered, entry) - } - } - for _, entry := range extra { - name, _, ok := strings.Cut(entry, "=") - if ok && !sensitiveEnvironmentName(name) && !reviewPathEnvironmentName(name) && (dir == "" || name != "PWD") { - filtered = append(filtered, entry) + if !ok || !reviewEnvironmentKey(name) || (dir != "" && name == "PWD") { + continue } + filtered = append(filtered, entry) } if dir != "" { filtered = append(filtered, "PWD="+dir) } - filtered = append(filtered, "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null") + filtered = append(filtered, "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GIT_TERMINAL_PROMPT=0") return filtered } -func reviewPathEnvironmentName(name string) bool { - return name == "OLDPWD" || strings.HasPrefix(name, "GIT_") -} - -func sensitiveEnvironmentName(name string) bool { - upper := strings.ToUpper(name) - if upper == "SSH_AUTH_SOCK" || upper == "COOKIE" { +func reviewEnvironmentKey(name string) bool { + switch name { + case "PATH", "HOME", "TMPDIR", "LANG", "TERM", "USER", "LOGNAME", "SHELL", "PWD", "OLDPWD", "NO_COLOR", "CI", + "FAKE_AGENT_KIND", "FAKE_AGENT_SCENARIO", "FAKE_AGENT_LOG_FILE", "FAKE_AGENT_EXIT_CODE", "FAKE_AGENT_WRITE_PATH", "FAKE_AGENT_WRITE_DATA": return true } - for _, marker := range []string{"TOKEN", "SECRET", "PASSWORD", "PASSWD", "API_KEY", "PRIVATE_KEY", "CREDENTIAL"} { - if strings.Contains(upper, marker) { - return true - } - } - return false + return strings.HasPrefix(name, "LC_") } -func invocation(kind Kind, worktree string) ([]string, func(), error) { +func invocation(kind Kind, worktree, task string) ([]string, func(), string, error) { if kind != KindCodex { - return []string{"review", "--worktree", worktree}, func() {}, nil + return nil, nil, "", fmt.Errorf("agent: %s structured task contract is unsupported", kind) } dir, err := os.MkdirTemp("", "made-codex-schema-") if err != nil { - return nil, nil, fmt.Errorf("agent: create Codex schema directory: %w", err) + return nil, nil, "", fmt.Errorf("agent: create Codex schema directory: %w", err) } - path := filepath.Join(dir, "output.json") - if err := os.WriteFile(path, []byte(reviewSchema), 0o600); err != nil { + schemaPath := filepath.Join(dir, "findings.schema.json") + if err := os.WriteFile(schemaPath, []byte(reviewSchema), 0o600); err != nil { _ = os.RemoveAll(dir) - return nil, nil, fmt.Errorf("agent: write Codex output schema: %w", err) - } - return []string{"exec", "--cd", worktree, "--json", "--output-schema", path, "-"}, func() { _ = os.RemoveAll(dir) }, nil -} - -func decodeFindings(data []byte) (Findings, error) { - var direct Findings - if err := json.Unmarshal(data, &direct); err == nil { - var envelope map[string]json.RawMessage - if json.Unmarshal(data, &envelope) == nil { - if raw, ok := envelope["findings"]; ok { - if string(raw) == "null" { - return Findings{Findings: []Finding{}}, nil - } - if values, err := strictFindings(data); err == nil { - return values, nil - } - } - } - } - scanner := bufio.NewScanner(bytes.NewReader(data)) - scanner.Buffer(make([]byte, 4096), 4*1024*1024) - var last string - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if strings.HasPrefix(line, "{") { - var event struct { - Item struct { - Type string `json:"type"` - Text string `json:"text"` - } `json:"item"` - } - if json.Unmarshal([]byte(line), &event) == nil && event.Item.Type == "agent_message" { - last = event.Item.Text - } - } - } - if err := scanner.Err(); err != nil { - return Findings{}, err + return nil, nil, "", fmt.Errorf("agent: write Codex output schema: %w", err) } - if last == "" { - return Findings{}, fmt.Errorf("structured findings payload was not found") + outputPath := filepath.Join(dir, "findings.json") + if strings.TrimSpace(task) == "" { + task = "Review the current worktree and return only the structured findings object required by the output schema." } - return strictFindings([]byte(last)) + return []string{"exec", "--json", "--output-schema", schemaPath, "--output-last-message", outputPath, "--sandbox", "read-only", "--ephemeral", "-C", worktree, task}, func() { _ = os.RemoveAll(dir) }, outputPath, nil } func strictFindings(data []byte) (Findings, error) { diff --git a/internal/agent/testdata/fakeagent/main.go b/internal/agent/testdata/fakeagent/main.go index 0d0e546..fe88277 100644 --- a/internal/agent/testdata/fakeagent/main.go +++ b/internal/agent/testdata/fakeagent/main.go @@ -10,9 +10,25 @@ package main import ( "fmt" "os" + "path/filepath" ) func main() { + if kind := os.Getenv("FAKE_AGENT_KIND"); kind != "" && kind != string(agentKindCodex) { + fmt.Fprintln(os.Stderr, "fakeagent: only the codex structured exec contract is supported") + os.Exit(2) + } + if err := validateInvocation(os.Args[1:]); err != nil { + fmt.Fprintf(os.Stderr, "fakeagent: invalid invocation: %v\n", err) + os.Exit(2) + } + for _, key := range []string{"MADE_TEST_SECRET", "DATABASE_URL", "COOKIE", "JWT_KEY", "KUBECONFIG"} { + if os.Getenv(key) != "" { + fmt.Fprintf(os.Stderr, "fakeagent: sensitive environment %s was exposed\n", key) + os.Exit(3) + } + } + if logPath := os.Getenv("FAKE_AGENT_LOG_FILE"); logPath != "" { logInvocation(logPath) } @@ -42,10 +58,31 @@ func main() { os.Exit(1) } - if _, err := os.Stdout.Write(data); err != nil { - fmt.Fprintf(os.Stderr, "fakeagent: write stdout: %v\n", err) + args := os.Args[1:] + lastMessagePath := args[5] + if err := os.WriteFile(lastMessagePath, data, 0o600); err != nil { + fmt.Fprintf(os.Stderr, "fakeagent: write structured output %s: %v\n", lastMessagePath, err) os.Exit(1) } + _, _ = fmt.Fprintln(os.Stdout, `{"type":"turn.completed"}`) +} + +const agentKindCodex = "codex" + +func validateInvocation(args []string) error { + if len(args) != 12 { + return fmt.Errorf("want 12 arguments, got %d", len(args)) + } + if args[0] != "exec" || args[1] != "--json" || args[2] != "--output-schema" || args[4] != "--output-last-message" || args[6] != "--sandbox" || args[7] != "read-only" || args[8] != "--ephemeral" || args[9] != "-C" { + return fmt.Errorf("expected codex exec structured flags, got %v", args) + } + if filepath.IsAbs(args[3]) == false || filepath.IsAbs(args[5]) == false { + return fmt.Errorf("schema and output paths must be absolute") + } + if args[10] == "" || args[11] == "" { + return fmt.Errorf("worktree and task are required") + } + return nil } func logInvocation(logPath string) { diff --git a/internal/config/config.go b/internal/config/config.go index fd5cfa0..9bfe935 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -207,19 +207,19 @@ func loadConfigFile(path string) (cfg Config, exists bool, err error) { return Config{}, false, nil } - if filepath.Base(path) == ".made.yml" || strings.HasSuffix(filepath.Base(path), ".made.yml") { - decoder := yaml.NewDecoder(bytes.NewReader(data)) - decoder.KnownFields(true) - if err := decoder.Decode(&cfg); err != nil { - return Config{}, true, err - } - var extra any - if err := decoder.Decode(&extra); err != io.EOF { - if err == nil { - return Config{}, true, fmt.Errorf("versioned .made.yml must contain one document") - } - return Config{}, true, err + decoder := yaml.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(true) + if err := decoder.Decode(&cfg); err != nil { + return Config{}, true, err + } + var extra any + if err := decoder.Decode(&extra); err != io.EOF { + if err == nil { + return Config{}, true, fmt.Errorf("configuration must contain one YAML document") } + return Config{}, true, err + } + if filepath.Base(path) == ".made.yml" || strings.HasSuffix(filepath.Base(path), ".made.yml") { if cfg.Version != 1 { return Config{}, true, fmt.Errorf("versioned .made.yml requires version: 1, got %d", cfg.Version) } @@ -240,10 +240,6 @@ func loadConfigFile(path string) (cfg Config, exists bool, err error) { } return cfg, true, nil } - if err := yaml.Unmarshal(data, &cfg); err != nil { - return Config{}, true, err - } - return cfg, true, nil } diff --git a/internal/config/config_contract_test.go b/internal/config/config_contract_test.go new file mode 100644 index 0000000..aebe079 --- /dev/null +++ b/internal/config/config_contract_test.go @@ -0,0 +1,12 @@ +package config + +import "testing" + +func TestLoadEffectiveConfig_RejectsUnknownSemanticSwitch(t *testing.T) { + dir := t.TempDir() + trustedPath := writeConfigFile(t, dir, "trusted.yaml", "review:\n required: true\nunknown_switch: true\n") + + if _, err := LoadEffectiveConfig(trustedPath, ""); err == nil { + t.Fatal("expected unknown semantic configuration switch to fail closed") + } +} diff --git a/internal/daemon/contract.go b/internal/daemon/contract.go index 15139ba..a557003 100644 --- a/internal/daemon/contract.go +++ b/internal/daemon/contract.go @@ -16,84 +16,70 @@ func (rm *RunManager) HasActive() bool { } func (rm *RunManager) SetDecision(id, stage, decision string) error { - r, ok := rm.lookupRun(id) - if !ok { - return fmt.Errorf("daemon: no run %q", id) - } if stage == "" || decision == "" { return fmt.Errorf("daemon: decision stage and value are required") } - r.update(func(snapshot *RunSnapshot) { + return rm.updateRun(id, func(snapshot *RunSnapshot) error { if snapshot.Decisions == nil { snapshot.Decisions = make(map[string]string) } snapshot.Decisions[stage] = decision + return nil }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist decision for run %q: %w", id, err) - } - return nil } func (rm *RunManager) SetPRURL(id, prURL string) error { - r, ok := rm.lookupRun(id) - if !ok { - return fmt.Errorf("daemon: no run %q", id) - } - r.update(func(snapshot *RunSnapshot) { snapshot.PRURL = prURL }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist PR URL for run %q: %w", id, err) - } - return nil + return rm.updateRun(id, func(snapshot *RunSnapshot) error { + snapshot.PRURL = prURL + return nil + }) } func (rm *RunManager) SetOutputSHA(id, outputSHA string) error { - r, ok := rm.lookupRun(id) - if !ok { - return fmt.Errorf("daemon: no run %q", id) - } if outputSHA == "" { return fmt.Errorf("daemon: output SHA is required") } - r.update(func(snapshot *RunSnapshot) { snapshot.OutputSHA = outputSHA }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist output SHA for run %q: %w", id, err) - } - return nil + return rm.updateRun(id, func(snapshot *RunSnapshot) error { + snapshot.OutputSHA = outputSHA + return nil + }) } func (rm *RunManager) AddFindings(id string, findings []RunFinding) error { - r, ok := rm.lookupRun(id) - if !ok { - return fmt.Errorf("daemon: no run %q", id) - } - r.update(func(snapshot *RunSnapshot) { + return rm.updateRun(id, func(snapshot *RunSnapshot) error { snapshot.Findings = append(snapshot.Findings, findings...) + return nil }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist findings for run %q: %w", id, err) - } - return nil } func (rm *RunManager) AppendSubmissionEvent(id string, event SubmissionEvent) error { - r, ok := rm.lookupRun(id) - if !ok { - return fmt.Errorf("daemon: no run %q", id) - } if event.RecordedAt.IsZero() { event.RecordedAt = time.Now().UTC() } - r.update(func(snapshot *RunSnapshot) { + return rm.updateRun(id, func(snapshot *RunSnapshot) error { for _, existing := range snapshot.SubmissionEvents { if existing.Gate == event.Gate && existing.Ref == event.Ref && existing.InputSHA == event.InputSHA { - return + return nil } } snapshot.SubmissionEvents = append(snapshot.SubmissionEvents, event) + return nil }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist submission event for run %q: %w", id, err) +} + +func (rm *RunManager) updateRun(id string, update func(*RunSnapshot) error) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + if err := update(&candidate); err != nil { + return err + } + if err := rm.persistAndReplace(r, candidate); err != nil { + return fmt.Errorf("persist run %q: %w", id, err) } return nil } diff --git a/internal/daemon/mailbox.go b/internal/daemon/mailbox.go index 6e46177..2e1f12c 100644 --- a/internal/daemon/mailbox.go +++ b/internal/daemon/mailbox.go @@ -13,6 +13,7 @@ const ( EventStageFinished EventKind = "stage_finished" EventRunCompleted EventKind = "run_completed" EventRunFailed EventKind = "run_failed" + EventRunCanceled EventKind = "run_canceled" ) type Event struct { diff --git a/internal/daemon/persistence.go b/internal/daemon/persistence.go new file mode 100644 index 0000000..7d6bca3 --- /dev/null +++ b/internal/daemon/persistence.go @@ -0,0 +1,488 @@ +package daemon + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "time" + + "github.com/douglasjarquin/made/internal/evidence" +) + +const ( + walFileName = "runs.wal" + snapshotFileName = "runs.snapshot.json" + maxWALBytes = 1 << 20 + maxWALRecords = 512 +) + +// RunSubmission is the immutable identity supplied by one accepted git push. +// It is persisted before the job enters the in-memory queue so a restart can +// distinguish a refresh of the same submission from an unrelated run. +type RunSubmission struct { + ID string `json:"run_id,omitempty"` + Repo string `json:"repo"` + Branch string `json:"branch"` + Ref string `json:"ref,omitempty"` + OldSHA string `json:"old_sha,omitempty"` + InputSHA string `json:"input_sha,omitempty"` + OutputSHA string `json:"output_sha,omitempty"` + SubmissionID string `json:"submission_id,omitempty"` + GatePath string `json:"gate_path,omitempty"` +} + +func (s RunSubmission) snapshot(queuedAt time.Time) RunSnapshot { + return RunSnapshot{ + ID: s.ID, + Repo: s.Repo, + Branch: s.Branch, + Ref: s.Ref, + OldSHA: s.OldSHA, + InputSHA: s.InputSHA, + OutputSHA: s.OutputSHA, + SubmissionID: s.SubmissionID, + GatePath: s.GatePath, + Status: RunQueued, + QueuedAt: queuedAt, + Stages: []StageResult{}, + PendingFindings: []AskUserFinding{}, + EvidenceRefs: []string{}, + Decisions: map[string]string{}, + ExecutionFinished: false, + } +} + +type walRecord struct { + Snapshot RunSnapshot `json:"snapshot"` +} + +type checkpoint struct { + Counter uint64 `json:"counter"` + Runs []RunSnapshot `json:"runs"` +} + +type runStore struct { + dir string + walPath string + snapshotPath string + + mu sync.Mutex + records int + closed bool +} + +func openRunStore(dir string) (*runStore, error) { + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, fmt.Errorf("daemon: create state directory: %w", err) + } + return &runStore{ + dir: dir, + walPath: filepath.Join(dir, walFileName), + snapshotPath: filepath.Join(dir, snapshotFileName), + }, nil +} + +func (s *runStore) load() ([]RunSnapshot, uint64, error) { + s.mu.Lock() + defer s.mu.Unlock() + + var state checkpoint + data, err := os.ReadFile(s.snapshotPath) + if err == nil { + if err := json.Unmarshal(data, &state); err != nil { + return nil, 0, fmt.Errorf("daemon: decode run checkpoint: %w", err) + } + } else if !errors.Is(err, os.ErrNotExist) { + return nil, 0, fmt.Errorf("daemon: read run checkpoint: %w", err) + } + + byID := make(map[string]RunSnapshot, len(state.Runs)) + for _, snap := range state.Runs { + byID[snap.ID] = restoreSnapshot(snap) + } + + wal, err := os.ReadFile(s.walPath) + if err == nil { + lines := bytes.Split(wal, []byte{'\n'}) + for i, line := range lines { + if len(bytes.TrimSpace(line)) == 0 { + continue + } + var record walRecord + if err := json.Unmarshal(line, &record); err != nil { + if i == len(lines)-1 { + // A torn final append is safe to ignore because every + // record before it was fsynced before it became visible. + break + } + return nil, 0, fmt.Errorf("daemon: decode run WAL record %d: %w", i, err) + } + if record.Snapshot.ID == "" { + return nil, 0, fmt.Errorf("daemon: run WAL record %d has empty run ID", i) + } + byID[record.Snapshot.ID] = restoreSnapshot(record.Snapshot) + s.records++ + } + } else if !errors.Is(err, os.ErrNotExist) { + return nil, 0, fmt.Errorf("daemon: read run WAL: %w", err) + } + + runs := make([]RunSnapshot, 0, len(byID)) + var maxID uint64 + for _, snap := range byID { + runs = append(runs, snap) + if n, ok := runIDNumber(snap.ID); ok && n > maxID { + maxID = n + } + } + if state.Counter > maxID { + maxID = state.Counter + } + return runs, maxID, nil +} + +func (s *runStore) append(snapshot RunSnapshot) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.closed { + return errors.New("daemon: run store is closed") + } + + data, err := json.Marshal(walRecord{Snapshot: snapshotForStorage(snapshot)}) + if err != nil { + return fmt.Errorf("daemon: encode run WAL record: %w", err) + } + file, err := os.OpenFile(s.walPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) + if err != nil { + return fmt.Errorf("daemon: open run WAL: %w", err) + } + if _, err := file.Write(append(data, '\n')); err != nil { + _ = file.Close() + return fmt.Errorf("daemon: append run WAL: %w", err) + } + if err := file.Sync(); err != nil { + _ = file.Close() + return fmt.Errorf("daemon: sync run WAL: %w", err) + } + if err := file.Close(); err != nil { + return fmt.Errorf("daemon: close run WAL: %w", err) + } + s.records++ + return nil +} + +func (s *runStore) shouldCompact() bool { + s.mu.Lock() + defer s.mu.Unlock() + return s.records >= maxWALRecords || fileSize(s.walPath) >= maxWALBytes +} + +func (s *runStore) compact(runs []RunSnapshot, counter uint64) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.closed { + return errors.New("daemon: run store is closed") + } + + data, err := json.MarshalIndent(checkpoint{Counter: counter, Runs: snapshotsForStorage(runs)}, "", " ") + if err != nil { + return fmt.Errorf("daemon: encode run checkpoint: %w", err) + } + tmp, err := os.CreateTemp(s.dir, ".runs.snapshot-*") + if err != nil { + return fmt.Errorf("daemon: create run checkpoint: %w", err) + } + tmpName := tmp.Name() + defer func() { _ = os.Remove(tmpName) }() + if err := tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return fmt.Errorf("daemon: chmod run checkpoint: %w", err) + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return fmt.Errorf("daemon: write run checkpoint: %w", err) + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return fmt.Errorf("daemon: sync run checkpoint: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("daemon: close run checkpoint: %w", err) + } + if err := os.Rename(tmpName, s.snapshotPath); err != nil { + return fmt.Errorf("daemon: install run checkpoint: %w", err) + } + dirFile, err := os.Open(s.dir) + if err != nil { + return fmt.Errorf("daemon: open state directory: %w", err) + } + if err := dirFile.Sync(); err != nil { + _ = dirFile.Close() + return fmt.Errorf("daemon: sync state directory: %w", err) + } + if err := dirFile.Close(); err != nil { + return fmt.Errorf("daemon: close state directory: %w", err) + } + if err := os.WriteFile(s.walPath, nil, 0o600); err != nil { + return fmt.Errorf("daemon: truncate run WAL: %w", err) + } + s.records = 0 + return nil +} + +func (s *runStore) close(runs []RunSnapshot, counter uint64) error { + if err := s.compact(runs, counter); err != nil { + return err + } + s.mu.Lock() + s.closed = true + s.mu.Unlock() + return nil +} + +func fileSize(path string) int64 { + info, err := os.Stat(path) + if err != nil { + return 0 + } + return info.Size() +} + +func snapshotForStorage(snapshot RunSnapshot) RunSnapshot { + copy := cloneSnapshot(snapshot) + if copy.Err != nil && copy.Error == "" { + copy.Error = copy.Err.Error() + } + copy.Err = nil + copy.Message = redactString(copy.Message) + copy.Error = redactString(copy.Error) + copy.Errors = redactStrings(copy.Errors) + copy.Findings = redactFindings(copy.Findings) + copy.PendingFindings = redactPendingFindings(copy.PendingFindings) + copy.Decisions = redactDecisions(copy.Decisions) + copy.PRURL = redactString(copy.PRURL) + copy.SubmissionEvents = redactSubmissionEvents(copy.SubmissionEvents) + return copy +} + +func snapshotsForStorage(snapshots []RunSnapshot) []RunSnapshot { + out := make([]RunSnapshot, len(snapshots)) + for i, snapshot := range snapshots { + out[i] = snapshotForStorage(snapshot) + } + return out +} + +func restoreSnapshot(snapshot RunSnapshot) RunSnapshot { + snapshot = cloneSnapshot(snapshot) + if snapshot.Error != "" { + snapshot.Err = errors.New(snapshot.Error) + } + if snapshot.Stages == nil { + snapshot.Stages = []StageResult{} + } + if snapshot.PendingFindings == nil { + snapshot.PendingFindings = []AskUserFinding{} + } + if snapshot.EvidenceRefs == nil { + snapshot.EvidenceRefs = []string{} + } + if snapshot.Decisions == nil { + snapshot.Decisions = map[string]string{} + } + return snapshot +} + +func redactString(value string) string { + return evidence.RedactString(value) +} + +func redactDecisions(values map[string]string) map[string]string { + if values == nil { + return nil + } + out := make(map[string]string, len(values)) + for key, value := range values { + out[key] = redactString(value) + } + return out +} + +func cloneSnapshot(snapshot RunSnapshot) RunSnapshot { + copy := snapshot + copy.Errors = append([]string(nil), snapshot.Errors...) + copy.Findings = append([]RunFinding(nil), snapshot.Findings...) + for i := range copy.Findings { + copy.Findings[i].Paths = append([]string(nil), snapshot.Findings[i].Paths...) + } + copy.SubmissionEvents = append([]SubmissionEvent(nil), snapshot.SubmissionEvents...) + copy.Stages = append([]StageResult(nil), snapshot.Stages...) + for i := range copy.Stages { + copy.Stages[i].EvidenceRefs = append([]string(nil), snapshot.Stages[i].EvidenceRefs...) + } + copy.PendingFindings = append([]AskUserFinding(nil), snapshot.PendingFindings...) + copy.EvidenceRefs = append([]string(nil), snapshot.EvidenceRefs...) + if snapshot.Decisions != nil { + copy.Decisions = make(map[string]string, len(snapshot.Decisions)) + maps.Copy(copy.Decisions, snapshot.Decisions) + } + return copy +} + +func runIDNumber(id string) (uint64, bool) { + value := strings.TrimPrefix(id, "run-") + if value == id || value == "" { + return 0, false + } + n, err := strconv.ParseUint(value, 10, 64) + return n, err == nil +} + +// OpenRunManager restores terminal and awaiting-merge records from the +// durable run store. In-flight work is never silently replayed without its +// original WorkFunc; the persisted submission remains queryable for an +// explicit refresh using the same submission identity. +func OpenRunManager(stateDir string) (*RunManager, error) { + store, err := openRunStore(stateDir) + if err != nil { + return nil, err + } + rm := newRunManager(store) + runs, counter, err := store.load() + if err != nil { + return nil, err + } + rm.counter.Store(counter) + for _, snapshot := range runs { + if snapshot.Status == RunRunning || snapshot.Status == RunAwaitingReview { + snapshot.Status = RunFailed + snapshot.Error = "daemon restarted before run execution finished" + snapshot.Err = errors.New(snapshot.Error) + snapshot.EndedAt = time.Now() + snapshot.ExecutionFinished = true + } + ctx, cancel := context.WithCancel(context.Background()) + r := &run{ctx: ctx, cancel: cancel, snap: restoreSnapshot(snapshot)} + rm.runs[snapshot.ID] = r + if snapshot.Status == RunQueued { + // Queue refresh is explicit: no work is replayed merely because + // a daemon restarted. + rm.repos[snapshot.Repo] = &repoQueue{} + } + if snapshot.Status == RunFailed && snapshot.Error == "daemon restarted before run execution finished" { + rm.durableMu.Lock() + rm.mu.Lock() + err := rm.persistSnapshotLocked(snapshot) + rm.mu.Unlock() + rm.durableMu.Unlock() + if err != nil { + cancel() + return nil, err + } + } + } + return rm, nil +} + +func (rm *RunManager) Close() error { + if rm.store == nil { + return nil + } + rm.durableMu.Lock() + defer rm.durableMu.Unlock() + runs := rm.List() + if rm.beforeCloseCompact != nil { + rm.beforeCloseCompact() + } + return rm.store.close(runs, rm.counter.Load()) +} + +func (rm *RunManager) persistSnapshotLocked(snapshot RunSnapshot) error { + if rm.store == nil { + return nil + } + if err := rm.store.append(snapshot); err != nil { + return err + } + if rm.store.shouldCompact() { + runs := rm.snapshotsLocked() + replaced := false + for i := range runs { + if runs[i].ID == snapshot.ID { + runs[i] = cloneSnapshot(snapshot) + replaced = true + break + } + } + if !replaced { + runs = append(runs, cloneSnapshot(snapshot)) + } + return rm.store.compact(runs, rm.counter.Load()) + } + return nil +} + +func (rm *RunManager) snapshotsLocked() []RunSnapshot { + out := make([]RunSnapshot, 0, len(rm.runs)) + for _, r := range rm.runs { + out = append(out, r.snapshot()) + } + return out +} + +func (rm *RunManager) FindSubmission(submission RunSubmission) (RunSnapshot, bool) { + rm.mu.Lock() + runs := make([]*run, 0, len(rm.runs)) + for _, r := range rm.runs { + runs = append(runs, r) + } + rm.mu.Unlock() + for _, r := range runs { + snapshot := r.snapshot() + if submission.SubmissionID != "" && snapshot.SubmissionID == submission.SubmissionID && + snapshot.Repo == submission.Repo && snapshot.Branch == submission.Branch { + return snapshot, true + } + if submission.InputSHA != "" && submission.Repo != "" && snapshot.Repo == submission.Repo && + snapshot.Branch == submission.Branch && snapshot.Ref == submission.Ref && + snapshot.InputSHA == submission.InputSHA { + return snapshot, true + } + } + return RunSnapshot{}, false +} + +func (rm *RunManager) UpdateDecision(id, stage, decision string) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + if candidate.Decisions == nil { + candidate.Decisions = make(map[string]string) + } + candidate.Decisions[stage] = decision + err := rm.persistAndReplace(r, candidate) + if err != nil { + return err + } + return err +} + +func NewPersistentRunManager(path string) (*RunManager, error) { + stateDir := path + if filepath.Ext(path) == ".wal" { + stateDir = filepath.Dir(path) + } + return OpenRunManager(stateDir) +} diff --git a/internal/daemon/persistence_contract_test.go b/internal/daemon/persistence_contract_test.go new file mode 100644 index 0000000..e0a8a11 --- /dev/null +++ b/internal/daemon/persistence_contract_test.go @@ -0,0 +1,434 @@ +package daemon + +import ( + "context" + "errors" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + "time" +) + +func TestRunManager_RestoresDurableSnapshotAfterRestart(t *testing.T) { + stateDir := t.TempDir() + rm1, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager first instance: %v", err) + } + + release := make(chan struct{}) + submitted, err := rm1.SubmitSubmission(RunSubmission{ + ID: "run-durable-1", + Repo: "example/repo", + Branch: "feature/durable", + Ref: "refs/heads/feature/durable", + OldSHA: "1111111111111111111111111111111111111111", + InputSHA: "2222222222222222222222222222222222222222", + OutputSHA: "3333333333333333333333333333333333333333", + SubmissionID: "submission-1", + GatePath: "/tmp/made-gate", + }, func(ctx context.Context, emit func(Event)) error { + <-release + return nil + }) + if err != nil { + t.Fatalf("SubmitSubmission: %v", err) + } + if submitted.Status != RunQueued { + t.Fatalf("SubmitSubmission returned %q, want pre-drain queued identity", submitted.Status) + } + + stages := []StageResult{{Name: "intent", Result: "pass"}, {Name: "review", Result: "pending"}} + if err := rm1.UpdateStages("run-durable-1", stages); err != nil { + t.Fatalf("UpdateStages: %v", err) + } + if err := rm1.Finish("run-durable-1", RunAwaitingMerge, "awaiting human merge"); err != nil { + t.Fatalf("Finish: %v", err) + } + close(release) + deadline := time.After(2 * time.Second) + for { + snap, ok := rm1.Snapshot("run-durable-1") + if ok && snap.Status == RunAwaitingMerge { + break + } + select { + case <-deadline: + t.Fatalf("run did not reach awaiting merge: %+v", snap) + case <-time.After(5 * time.Millisecond): + } + } + if err := rm1.Close(); err != nil { + t.Fatalf("Close first instance: %v", err) + } + + rm2, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager after restart: %v", err) + } + defer func() { _ = rm2.Close() }() + + restored, ok := rm2.Snapshot("run-durable-1") + if !ok { + t.Fatal("run not found after daemon restart") + } + if restored.Status != RunAwaitingMerge || restored.Message != "awaiting human merge" { + t.Fatalf("restored lifecycle = %+v, want awaiting merge", restored) + } + if restored.Repo != "example/repo" || restored.Branch != "feature/durable" || restored.Ref != "refs/heads/feature/durable" { + t.Fatalf("restored submission identity = %+v", restored) + } + if restored.InputSHA != "2222222222222222222222222222222222222222" || restored.OutputSHA != "3333333333333333333333333333333333333333" { + t.Fatalf("restored SHA identity = %+v", restored) + } + if restored.SubmissionID != "submission-1" || restored.GatePath != "/tmp/made-gate" { + t.Fatalf("restored submission metadata = %+v", restored) + } + if len(restored.Stages) != len(stages) || !reflect.DeepEqual(restored.Stages[1], stages[1]) { + t.Fatalf("restored stages = %+v, want %+v", restored.Stages, stages) + } + if err := rm2.Finish("run-durable-1", RunSucceeded, "merged"); err != nil { + t.Fatalf("Finish succeeded: %v", err) + } + finished, _ := rm2.Snapshot("run-durable-1") + if finished.Status != RunSucceeded || !finished.ExecutionFinished { + t.Fatalf("awaiting_merge did not transition to succeeded: %+v", finished) + } +} + +func TestRunManager_IgnoresTornFinalWALRecord(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.Submit("run-torn-tail", "repo", "branch", func(context.Context, func(Event)) error { return nil }); err != nil { + t.Fatalf("Submit: %v", err) + } + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + snapshot, _ := rm.Snapshot("run-torn-tail") + if snapshot.Status == RunSucceeded { + break + } + time.Sleep(time.Millisecond) + } + if err := rm.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + wal, err := os.OpenFile(filepath.Join(stateDir, walFileName), os.O_WRONLY|os.O_APPEND, 0o600) + if err != nil { + t.Fatalf("open WAL: %v", err) + } + if _, err := wal.WriteString(`{"snapshot":{"run_id":"run-torn-tail"`); err != nil { + t.Fatalf("append torn WAL: %v", err) + } + _ = wal.Close() + + restarted, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager after torn tail: %v", err) + } + defer func() { _ = restarted.Close() }() + if snapshot, ok := restarted.Snapshot("run-torn-tail"); !ok || snapshot.Status != RunSucceeded { + t.Fatalf("valid checkpoint was lost with torn WAL tail: %+v (ok=%v)", snapshot, ok) + } +} + +func TestRunManager_WALRetentionIsBounded(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.Submit("run-retention", "repo", "branch", func(context.Context, func(Event)) error { return nil }); err != nil { + t.Fatalf("Submit: %v", err) + } + for i := range maxWALRecords + 10 { + if err := rm.UpdateStages("run-retention", []StageResult{{Name: "stage", Result: "pass", Message: "update"}}); err != nil { + t.Fatalf("UpdateStages %d: %v", i, err) + } + } + if info, err := os.Stat(filepath.Join(stateDir, walFileName)); err != nil { + t.Fatalf("stat WAL: %v", err) + } else if info.Size() >= maxWALBytes { + t.Fatalf("WAL exceeded retention bound: %d bytes", info.Size()) + } + if err := rm.Close(); err != nil { + t.Fatalf("Close: %v", err) + } +} + +func TestRunManager_CompactionPersistsTriggeringTransition(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.SubmitSubmission(RunSubmission{ID: "run-compaction", Repo: "repo", Branch: "branch"}, nil); err != nil { + t.Fatalf("SubmitSubmission: %v", err) + } + for i := 0; i < maxWALRecords-2; i++ { + if err := rm.UpdateStages("run-compaction", []StageResult{{Name: "intent", Result: "pass", Message: "before"}}); err != nil { + t.Fatalf("UpdateStages before compaction %d: %v", i, err) + } + } + if err := rm.UpdateStages("run-compaction", []StageResult{{Name: "intent", Result: "pass", Message: "compaction-trigger"}}); err != nil { + t.Fatalf("UpdateStages compaction trigger: %v", err) + } + + restarted, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager after compaction: %v", err) + } + defer func() { _ = restarted.Close() }() + snapshot, ok := restarted.Snapshot("run-compaction") + if !ok || len(snapshot.Stages) != 1 || snapshot.Stages[0].Message != "compaction-trigger" { + t.Fatalf("compaction lost triggering transition: %+v (ok=%v)", snapshot, ok) + } + if err := rm.Close(); err != nil { + t.Fatalf("Close original manager: %v", err) + } +} + +func TestRunManager_CloseDoesNotDiscardConcurrentDurableMutation(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + const runID = "run-close-race" + if _, err := rm.SubmitSubmission(RunSubmission{ID: runID, Repo: "repo", Branch: "branch"}, nil); err != nil { + t.Fatalf("Submit: %v", err) + } + + snapshotCaptured := make(chan struct{}) + allowCompact := make(chan struct{}) + rm.beforeCloseCompact = func() { + close(snapshotCaptured) + <-allowCompact + } + closeErr := make(chan error, 1) + go func() { closeErr <- rm.Close() }() + <-snapshotCaptured + + updateErr := make(chan error, 1) + go func() { + updateErr <- rm.UpdateStages(runID, []StageResult{{Name: "intent", Result: "pass"}}) + }() + var updateCompletedBeforeRelease bool + var updateResult error + select { + case updateResult = <-updateErr: + updateCompletedBeforeRelease = true + case <-time.After(100 * time.Millisecond): + } + close(allowCompact) + if err := <-closeErr; err != nil { + t.Fatalf("Close: %v", err) + } + if !updateCompletedBeforeRelease { + updateResult = <-updateErr + } + if updateResult != nil { + return + } + + restarted, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager after close: %v", err) + } + snapshot, ok := restarted.Snapshot(runID) + _ = restarted.Close() + if !ok || len(snapshot.Stages) != 1 || snapshot.Stages[0].Result != "pass" { + t.Fatalf("concurrent durable mutation was lost: %+v (ok=%v)", snapshot, ok) + } +} + +func TestReviewDecisions_RestoreAndRejectConflict(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + release := make(chan struct{}) + if _, err := rm.Submit("run-decision", "repo", "branch", func(context.Context, func(Event)) error { + <-release + return nil + }); err != nil { + t.Fatalf("Submit: %v", err) + } + waitForStatus(t, rm, "run-decision", RunRunning, time.Second) + if err := rm.UpdatePendingFindings("run-decision", []AskUserFinding{{Stage: "review", Message: "finding"}}); err != nil { + t.Fatalf("UpdatePendingFindings: %v", err) + } + decisions := NewReviewDecisionsForManager(rm) + if err := decisions.Set("run-decision", "review", ReviewRejected); err != nil { + t.Fatalf("Set: %v", err) + } + close(release) + waitForStatus(t, rm, "run-decision", RunSucceeded, time.Second) + if err := rm.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + restarted, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("reopen: %v", err) + } + defer func() { _ = restarted.Close() }() + restoredDecisions := NewReviewDecisionsForManager(restarted) + decision, ok := restoredDecisions.Get("run-decision", "review") + if !ok || decision != ReviewRejected { + t.Fatalf("decision did not restore: %q (ok=%v)", decision, ok) + } + if err := restoredDecisions.Set("run-decision", "review", ReviewApproved); !errors.Is(err, ErrDecisionAlreadyRecorded) { + t.Fatalf("conflicting decision error = %v, want ErrDecisionAlreadyRecorded", err) + } +} + +func TestRunManager_UpdateStagesRollsBackOnPersistenceFailure(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.SubmitSubmission(RunSubmission{ + ID: "run-persist-failure", + Repo: "repo", + Branch: "branch", + }, nil); err != nil { + t.Fatalf("SubmitSubmission: %v", err) + } + if err := rm.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + err = rm.UpdateStages("run-persist-failure", []StageResult{{Name: "intent", Result: "pass"}}) + if err == nil { + t.Fatal("UpdateStages succeeded with a closed durable store") + } + snapshot, ok := rm.Snapshot("run-persist-failure") + if !ok { + t.Fatal("run disappeared after persistence failure") + } + if len(snapshot.Stages) != 0 { + t.Fatalf("in-memory stage update survived persistence failure: %+v", snapshot.Stages) + } +} + +func TestRunManager_FailsRunWhenFinalPersistenceFails(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.SubmitSubmission(RunSubmission{ + ID: "run-final-persist-failure", + Repo: "repo", + Branch: "branch", + }, func(context.Context, func(Event)) error { + if err := rm.Close(); err != nil { + return err + } + return nil + }); err != nil { + t.Fatalf("SubmitSubmission: %v", err) + } + + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + snapshot, ok := rm.Snapshot("run-final-persist-failure") + if ok && snapshot.ExecutionFinished { + if snapshot.Status != RunFailed { + t.Fatalf("run status = %s after final persistence failure, want failed", snapshot.Status) + } + return + } + time.Sleep(time.Millisecond) + } + t.Fatal("run did not finish") +} + +func TestOpenRunManager_PreservesStateAfterRecoveryFailure(t *testing.T) { + stateDir := t.TempDir() + rm, err := OpenRunManager(stateDir) + if err != nil { + t.Fatalf("OpenRunManager: %v", err) + } + if _, err := rm.SubmitSubmission(RunSubmission{ + ID: "run-recovery-preserve", + Repo: "repo", + Branch: "branch", + }, nil); err != nil { + t.Fatalf("SubmitSubmission: %v", err) + } + if err := rm.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + + wal, err := os.OpenFile(filepath.Join(stateDir, walFileName), os.O_WRONLY|os.O_APPEND, 0o600) + if err != nil { + t.Fatalf("open WAL: %v", err) + } + if _, err := wal.WriteString("{not-valid-json}\n"); err != nil { + t.Fatalf("append corrupt WAL: %v", err) + } + if err := wal.Close(); err != nil { + t.Fatalf("close WAL: %v", err) + } + + if _, err := OpenRunManager(stateDir); err == nil { + t.Fatal("OpenRunManager accepted non-final WAL corruption") + } + checkpoint, err := os.ReadFile(filepath.Join(stateDir, snapshotFileName)) + if err != nil { + t.Fatalf("read checkpoint after failed recovery: %v", err) + } + if !strings.Contains(string(checkpoint), "run-recovery-preserve") { + t.Fatalf("failed recovery replaced the durable checkpoint: %s", checkpoint) + } + walData, err := os.ReadFile(filepath.Join(stateDir, walFileName)) + if err != nil { + t.Fatalf("read WAL after failed recovery: %v", err) + } + if !strings.Contains(string(walData), "not-valid-json") { + t.Fatalf("failed recovery truncated the corrupt WAL for diagnosis: %s", walData) + } +} + +func TestRunManager_FindSubmissionDoesNotCrossRepositoryBoundary(t *testing.T) { + rm := NewRunManager() + if _, err := rm.SubmitSubmission(RunSubmission{ + ID: "run-repo-a", + Repo: "repo-a", + Branch: "feature", + SubmissionID: "same-submission", + }, nil); err != nil { + t.Fatalf("submit repo-a: %v", err) + } + + if _, found := rm.FindSubmission(RunSubmission{ + Repo: "repo-b", + Branch: "feature", + SubmissionID: "same-submission", + }); found { + t.Fatal("FindSubmission matched a submission from another repository") + } +} + +func TestReviewDecisions_RejectsDecisionWithoutPendingFinding(t *testing.T) { + rm := NewRunManager() + if _, err := rm.SubmitSubmission(RunSubmission{ + ID: "run-no-finding", + Repo: "repo", + Branch: "feature", + }, nil); err != nil { + t.Fatalf("submit: %v", err) + } + decisions := NewReviewDecisionsForManager(rm) + if err := decisions.Set("run-no-finding", "review", ReviewApproved); err == nil { + t.Fatal("accepted a review decision without a pending finding") + } +} diff --git a/internal/daemon/reviewdecisions.go b/internal/daemon/reviewdecisions.go index acdb231..d94486a 100644 --- a/internal/daemon/reviewdecisions.go +++ b/internal/daemon/reviewdecisions.go @@ -2,6 +2,8 @@ package daemon import ( "context" + "errors" + "fmt" "sync" ) @@ -10,18 +12,23 @@ const ( ReviewRejected = "rejected" ) +var ErrDecisionAlreadyRecorded = errors.New("daemon: review decision already recorded") + type reviewKey struct { RunID string Stage string } // ReviewDecisions lives alongside RunManager because a decision only ever -// applies to one (run, stage) pair, making it per-run state that the versioned -// review.decide handler and orchestrator's WorkFunc share. +// applies to one (run, stage) pair, making it per-run state that both the +// review.decide/review.decision RPC handlers and the orchestrator's WorkFunc +// need to reach from their separate packages. type ReviewDecisions struct { mu sync.Mutex entries map[reviewKey]string waiters map[reviewKey][]chan string + persist func(runID, stage, decision string) error + manager *RunManager } func NewReviewDecisions() *ReviewDecisions { @@ -31,20 +38,81 @@ func NewReviewDecisions() *ReviewDecisions { } } +func NewReviewDecisionsForManager(rm *RunManager) *ReviewDecisions { + d := NewReviewDecisions() + d.persist = rm.UpdateDecision + d.manager = rm + return d +} + // Set records a decision for (runID, stage) and wakes any goroutine blocked // in Wait on that exact key. -func (d *ReviewDecisions) Set(runID, stage, decision string) { +func (d *ReviewDecisions) Set(runID, stage, decision string) error { key := reviewKey{RunID: runID, Stage: stage} + if _, exists := d.Get(runID, stage); exists { + return fmt.Errorf("%w for %s/%s", ErrDecisionAlreadyRecorded, runID, stage) + } + if d.manager != nil { + snapshot, ok := d.manager.Snapshot(runID) + if !ok { + return fmt.Errorf("daemon: cannot decide unknown run %q", runID) + } + if snapshot.Status != RunRunning && snapshot.Status != RunAwaitingReview { + return fmt.Errorf("daemon: run %q is %s, not awaiting a review decision", runID, snapshot.Status) + } + pending := false + for _, finding := range snapshot.PendingFindings { + if finding.Stage == stage { + pending = true + break + } + } + if !pending { + return fmt.Errorf("daemon: run %q has no pending %s finding", runID, stage) + } + } d.mu.Lock() + if _, exists := d.entries[key]; exists { + d.mu.Unlock() + return fmt.Errorf("%w for %s/%s", ErrDecisionAlreadyRecorded, runID, stage) + } d.entries[key] = decision waiters := d.waiters[key] + if d.persist != nil { + if err := d.persist(runID, stage, decision); err != nil { + delete(d.entries, key) + d.mu.Unlock() + return fmt.Errorf("daemon: persist review decision: %w", err) + } + } delete(d.waiters, key) d.mu.Unlock() for _, ch := range waiters { ch <- decision } + return nil +} + +func (d *ReviewDecisions) Get(runID, stage string) (string, bool) { + d.mu.Lock() + decision, ok := d.entries[reviewKey{RunID: runID, Stage: stage}] + d.mu.Unlock() + if ok || d.persist == nil { + return decision, ok + } + if d.manager != nil { + if snapshot, found := d.manager.Snapshot(runID); found { + decision, ok = snapshot.Decisions[stage] + if ok { + d.mu.Lock() + d.entries[reviewKey{RunID: runID, Stage: stage}] = decision + d.mu.Unlock() + } + } + } + return decision, ok } // Wait blocks until a decision is recorded for (runID, stage) via Set, or @@ -52,6 +120,9 @@ func (d *ReviewDecisions) Set(runID, stage, decision string) { // decision is already recorded. func (d *ReviewDecisions) Wait(ctx context.Context, runID, stage string) (string, error) { key := reviewKey{RunID: runID, Stage: stage} + if decision, ok := d.Get(runID, stage); ok { + return decision, nil + } d.mu.Lock() if decision, ok := d.entries[key]; ok { diff --git a/internal/daemon/reviewdecisions_test.go b/internal/daemon/reviewdecisions_test.go index f949423..320b93a 100644 --- a/internal/daemon/reviewdecisions_test.go +++ b/internal/daemon/reviewdecisions_test.go @@ -22,7 +22,9 @@ func TestReviewDecisions_WaitUnblocksOnSet(t *testing.T) { waitForWaiterRegistered(t, d, "run-1", "review") - d.Set("run-1", "review", ReviewApproved) + if err := d.Set("run-1", "review", ReviewApproved); err != nil { + t.Fatalf("Set: %v", err) + } select { case got := <-resultCh: @@ -39,7 +41,9 @@ func TestReviewDecisions_WaitUnblocksOnSet(t *testing.T) { func TestReviewDecisions_WaitReturnsImmediatelyIfAlreadyRecorded(t *testing.T) { d := NewReviewDecisions() - d.Set("run-2", "document", ReviewRejected) + if err := d.Set("run-2", "document", ReviewRejected); err != nil { + t.Fatalf("Set: %v", err) + } resultCh := make(chan string, 1) errCh := make(chan error, 1) diff --git a/internal/daemon/runmanager.go b/internal/daemon/runmanager.go index 02a5473..541401c 100644 --- a/internal/daemon/runmanager.go +++ b/internal/daemon/runmanager.go @@ -2,9 +2,12 @@ package daemon import ( "context" - "crypto/rand" + cryptorand "crypto/rand" + "encoding/binary" "errors" "fmt" + "sort" + "strings" "sync" "sync/atomic" "time" @@ -18,40 +21,45 @@ const ( RunAwaitingReview RunStatus = "awaiting_review" RunAwaitingMerge RunStatus = "awaiting_merge" RunSucceeded RunStatus = "succeeded" + RunCompleted RunStatus = RunSucceeded RunFailed RunStatus = "failed" RunCanceled RunStatus = "canceled" RunSuperseded RunStatus = "superseded" ) type RunSnapshot struct { - ID string - Repo string - Branch string - InputSHA string - OutputSHA string - Status RunStatus - QueuedAt time.Time - StartedAt time.Time - EndedAt time.Time - ExecutionFinished bool - Err error - Errors []string - Message string - Findings []RunFinding - Decisions map[string]string - PRURL string - SupersededBy string - CancelRequested bool - SubmissionEvents []SubmissionEvent - Stages []StageResult - PendingFindings []AskUserFinding - - // finalized is set by Finish and read by execute: it lets a WorkFunc - // declare a run's definitive terminal-or-not Status/Message itself, - // overriding execute's normal "nil error means RunSucceeded" inference - - // needed for the orchestrator's CI-passed-but-awaiting-human-merge case, - // where the pipeline finished successfully yet the run must stay - // RunAwaitingMerge rather than flip to RunSucceeded. + ID string `json:"run_id"` + Repo string `json:"repo"` + Branch string `json:"branch"` + Ref string `json:"ref,omitempty"` + OldSHA string `json:"old_sha,omitempty"` + InputSHA string `json:"input_sha,omitempty"` + OutputSHA string `json:"output_sha,omitempty"` + SubmissionID string `json:"submission_id,omitempty"` + GatePath string `json:"gate_path,omitempty"` + Status RunStatus `json:"state"` + QueuedAt time.Time `json:"queued_at"` + StartedAt time.Time `json:"started_at"` + EndedAt time.Time `json:"ended_at"` + Err error `json:"-"` + Error string `json:"error,omitempty"` + Message string `json:"message,omitempty"` + Errors []string `json:"errors,omitempty"` + Findings []RunFinding `json:"findings,omitempty"` + PRURL string `json:"pr_url,omitempty"` + SupersededBy string `json:"superseded_by,omitempty"` + CancelRequested bool `json:"cancel_requested,omitempty"` + SubmissionEvents []SubmissionEvent `json:"submission_events,omitempty"` + Stages []StageResult `json:"stages"` + PendingFindings []AskUserFinding `json:"pending_findings"` + EvidenceRefs []string `json:"evidence_refs,omitempty"` + CurrentStage string `json:"current_stage,omitempty"` + Decisions map[string]string `json:"decisions,omitempty"` + ExecutionFinished bool `json:"execution_finished"` + + // finalized is set by Finish and read by execute so a WorkFunc can declare + // an awaiting-merge or terminal result without being overwritten when it + // returns. finalized bool } @@ -62,10 +70,11 @@ var ErrRunIDExists = errors.New("daemon: run ID already submitted") var ErrRunSubmissionClosed = errors.New("daemon: run submission is closed") type run struct { - mu sync.Mutex - snap RunSnapshot - ctx context.Context - cancel context.CancelFunc + mu sync.Mutex + persistMu sync.Mutex + snap RunSnapshot + ctx context.Context + cancel context.CancelFunc } func (r *run) snapshot() RunSnapshot { @@ -74,9 +83,9 @@ func (r *run) snapshot() RunSnapshot { return cloneSnapshot(r.snap) } -func (r *run) update(fn func(*RunSnapshot)) { +func (r *run) replace(snapshot RunSnapshot) { r.mu.Lock() - fn(&r.snap) + r.snap = cloneSnapshot(snapshot) r.mu.Unlock() } @@ -96,93 +105,61 @@ type repoQueue struct { // worktree per bare repo at a time, so a second push against a repo already // running must queue behind it rather than run concurrently or be rejected. type RunManager struct { - mailbox *Mailbox - activity chan struct{} - store *RunStore - persistMu sync.Mutex + mailbox *Mailbox + activity chan struct{} + store *runStore + + beforeCloseCompact func() + durableMu sync.Mutex mu sync.Mutex repos map[string]*repoQueue runs map[string]*run closing bool + counter atomic.Uint64 } func NewRunManager() *RunManager { - return newRunManager(nil, nil) -} - -func NewRunID() string { - var id [16]byte - if _, err := rand.Read(id[:]); err != nil { - counter := atomic.AddUint64(&fallbackRunIDCounter, 1) - for i := range id { - id[i] = byte(counter >> (uint(i%8) * 8)) - } - } - id[6] = (id[6] & 0x0f) | 0x40 - id[8] = (id[8] & 0x3f) | 0x80 - return fmt.Sprintf("%08x-%04x-%04x-%04x-%012x", id[0:4], id[4:6], id[6:8], id[8:10], id[10:16]) + return newRunManager(nil) } -func NewPersistentRunManager(path string) (*RunManager, error) { - store, snapshots, err := OpenRunStore(path) - if err != nil { - return nil, err - } - rm := newRunManager(store, snapshots) - if err := rm.reconcileRestoredRuns(); err != nil { - return nil, err - } - return rm, nil -} - -func newRunManager(store *RunStore, snapshots map[string]RunSnapshot) *RunManager { - rm := &RunManager{ +func newRunManager(store *runStore) *RunManager { + return &RunManager{ mailbox: NewMailbox(), activity: make(chan struct{}, 1), + store: store, repos: make(map[string]*repoQueue), runs: make(map[string]*run), - store: store, - } - for id, snapshot := range snapshots { - ctx, cancel := context.WithCancel(context.Background()) - rm.runs[id] = &run{ctx: ctx, cancel: cancel, snap: cloneSnapshot(snapshot)} } - return rm } -func (rm *RunManager) persist(r *run) error { - if rm.store == nil { - return nil - } - rm.persistMu.Lock() - defer rm.persistMu.Unlock() - return rm.store.Append(r.snapshot()) +func (rm *RunManager) ActivitySignal() <-chan struct{} { + return rm.activity } -func (rm *RunManager) reconcileRestoredRuns() error { +func (rm *RunManager) BeginShutdown() error { + rm.mu.Lock() + defer rm.mu.Unlock() for _, r := range rm.runs { snapshot := r.snapshot() - if snapshot.Status != RunQueued && snapshot.Status != RunRunning && snapshot.Status != RunAwaitingReview { - continue - } - restartedErr := errors.New("daemon restarted before execution finished") - r.update(func(s *RunSnapshot) { - s.Status = RunFailed - s.EndedAt = time.Now() - s.ExecutionFinished = true - s.Err = restartedErr - s.Errors = append(s.Errors, restartedErr.Error()) - }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("reconcile restored run %q: %w", snapshot.ID, err) + if snapshot.Status == RunQueued || snapshot.Status == RunRunning || snapshot.Status == RunAwaitingReview || snapshot.Status == RunAwaitingMerge { + return fmt.Errorf("daemon: active run %q remains in state %s", snapshot.ID, snapshot.Status) } } + rm.closing = true return nil } -func (rm *RunManager) ActivitySignal() <-chan struct{} { - return rm.activity +func (rm *RunManager) StopAccepting() { + rm.mu.Lock() + rm.closing = true + rm.mu.Unlock() +} + +func (rm *RunManager) Accepting() bool { + rm.mu.Lock() + defer rm.mu.Unlock() + return !rm.closing } // Non-blocking send: a run must never wait on whether anyone is listening @@ -199,58 +176,75 @@ func (rm *RunManager) NewRunID() string { return NewRunID() } -var fallbackRunIDCounter uint64 +var fallbackRunIDCounter atomic.Uint64 + +func NewRunID() string { + var id [16]byte + if _, err := cryptorand.Read(id[:]); err != nil { + binary.BigEndian.PutUint64(id[:8], uint64(time.Now().UnixNano())) + binary.BigEndian.PutUint64(id[8:], fallbackRunIDCounter.Add(1)) + } + id[6] = (id[6] & 0x0f) | 0x40 + id[8] = (id[8] & 0x3f) | 0x80 + return fmt.Sprintf("%08x-%04x-%04x-%04x-%012x", + binary.BigEndian.Uint32(id[0:4]), + binary.BigEndian.Uint16(id[4:6]), + binary.BigEndian.Uint16(id[6:8]), + binary.BigEndian.Uint16(id[8:10]), + uint64(id[10])<<40|uint64(id[11])<<32|uint64(id[12])<<24|uint64(id[13])<<16|uint64(id[14])<<8|uint64(id[15])) +} func (rm *RunManager) Submit(id, repo, branch string, work WorkFunc) (RunSnapshot, error) { - return rm.SubmitWithMetadata(id, repo, branch, "", "", work) + return rm.SubmitSubmission(RunSubmission{ID: id, Repo: repo, Branch: branch}, work) } func (rm *RunManager) SubmitWithMetadata(id, repo, branch, inputSHA, outputSHA string, work WorkFunc) (RunSnapshot, error) { + return rm.SubmitSubmission(RunSubmission{ + ID: id, Repo: repo, Branch: branch, InputSHA: inputSHA, OutputSHA: outputSHA, + }, work) +} + +func (rm *RunManager) SubmitSubmission(submission RunSubmission, work WorkFunc) (RunSnapshot, error) { + if strings.TrimSpace(submission.ID) == "" { + return RunSnapshot{}, fmt.Errorf("daemon: run ID must not be empty") + } ctx, cancel := context.WithCancel(context.Background()) r := &run{ ctx: ctx, cancel: cancel, - snap: RunSnapshot{ - ID: id, - Repo: repo, - Branch: branch, - InputSHA: inputSHA, - OutputSHA: outputSHA, - Status: RunQueued, - QueuedAt: time.Now(), - Errors: []string{}, - Findings: []RunFinding{}, - Decisions: make(map[string]string), - SubmissionEvents: []SubmissionEvent{}, - }, + snap: submission.snapshot(time.Now()), } + queuedSnapshot := cloneSnapshot(r.snap) + rm.durableMu.Lock() rm.mu.Lock() if rm.closing { rm.mu.Unlock() + rm.durableMu.Unlock() cancel() return RunSnapshot{}, ErrRunSubmissionClosed } - if _, exists := rm.runs[id]; exists { + if _, exists := rm.runs[submission.ID]; exists { rm.mu.Unlock() + rm.durableMu.Unlock() return RunSnapshot{}, ErrRunIDExists } - rm.runs[id] = r - rq, ok := rm.repos[repo] + if err := rm.persistSnapshotLocked(r.snap); err != nil { + rm.mu.Unlock() + rm.durableMu.Unlock() + cancel() + return RunSnapshot{}, fmt.Errorf("daemon: persist submission: %w", err) + } + rm.runs[submission.ID] = r + rq, ok := rm.repos[submission.Repo] if !ok { rq = &repoQueue{} - rm.repos[repo] = rq + rm.repos[submission.Repo] = rq } rm.mu.Unlock() - if err := rm.persist(r); err != nil { - rm.mu.Lock() - delete(rm.runs, id) - if current, ok := rm.repos[repo]; ok && current == rq { - delete(rm.repos, repo) - } - rm.mu.Unlock() - cancel() - return RunSnapshot{}, fmt.Errorf("persist submitted run: %w", err) + rm.durableMu.Unlock() + if work == nil { + return queuedSnapshot, nil } rq.mu.Lock() @@ -258,7 +252,6 @@ func (rm *RunManager) SubmitWithMetadata(id, repo, branch, inputSHA, outputSHA s startDrain := !rq.active rq.active = true rq.mu.Unlock() - queuedSnapshot := r.snapshot() if startDrain { go rm.drain(rq) @@ -267,34 +260,38 @@ func (rm *RunManager) SubmitWithMetadata(id, repo, branch, inputSHA, outputSHA s return queuedSnapshot, nil } -func (rm *RunManager) BeginShutdown() error { +func (rm *RunManager) RefreshQueued(id string, work WorkFunc) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + snapshot := r.snapshot() + if snapshot.Status != RunQueued { + return fmt.Errorf("daemon: run %q is %s, not queued", id, snapshot.Status) + } rm.mu.Lock() - defer rm.mu.Unlock() - if rm.closing { - return ErrRunSubmissionClosed + rq := rm.repos[snapshot.Repo] + rm.mu.Unlock() + if rq == nil { + return fmt.Errorf("daemon: no queue for run %q", id) } - for _, r := range rm.runs { - snapshot := r.snapshot() - if snapshot.Status == RunQueued || snapshot.Status == RunRunning || snapshot.Status == RunAwaitingReview || snapshot.Status == RunAwaitingMerge { - return fmt.Errorf("daemon: active run %q remains in state %s", snapshot.ID, snapshot.Status) + rq.mu.Lock() + for _, job := range rq.pending { + if job.run == r { + rq.mu.Unlock() + return nil } } - rm.closing = true + rq.pending = append(rq.pending, &queuedJob{run: r, work: work}) + startDrain := !rq.active + rq.active = true + rq.mu.Unlock() + if startDrain { + go rm.drain(rq) + } return nil } -func (rm *RunManager) StopAccepting() { - rm.mu.Lock() - rm.closing = true - rm.mu.Unlock() -} - -func (rm *RunManager) Accepting() bool { - rm.mu.Lock() - defer rm.mu.Unlock() - return !rm.closing -} - func (rm *RunManager) drain(rq *repoQueue) { for { rq.mu.Lock() @@ -312,21 +309,26 @@ func (rm *RunManager) drain(rq *repoQueue) { } func (rm *RunManager) execute(r *run, work WorkFunc) { - id := r.snapshot().ID + initial := r.snapshot() + if isTerminalRunStatus(initial.Status) { + return + } + id := initial.ID started := time.Now() - r.update(func(s *RunSnapshot) { - s.Status = RunRunning - s.StartedAt = started - }) - if err := rm.persist(r); err != nil { - retryErr := rm.recordPersistenceFailure(r, err) - eventErr := err - if retryErr != nil { - eventErr = errors.Join(err, retryErr) - } - rm.mailbox.Publish(Event{RunID: id, Kind: EventRunFailed, Time: time.Now(), Err: eventErr}) + r.persistMu.Lock() + startedSnapshot := r.snapshot() + if startedSnapshot.Status != RunQueued { + r.persistMu.Unlock() + return + } + startedSnapshot.Status = RunRunning + startedSnapshot.StartedAt = started + if err := rm.persistAndReplace(r, startedSnapshot); err != nil { + r.persistMu.Unlock() + rm.failAfterPersistenceError(r, err) return } + r.persistMu.Unlock() rm.mailbox.Publish(Event{RunID: id, Kind: EventRunStarted, Time: started}) rm.signalActivity() @@ -339,63 +341,55 @@ func (rm *RunManager) execute(r *run, work WorkFunc) { rm.signalActivity() } - err := work(r.ctx, emit) + var err error + if work == nil { + err = errors.New("daemon: nil run work function") + } else { + err = work(r.ctx, emit) + } rm.signalActivity() ended := time.Now() - r.update(func(s *RunSnapshot) { - s.EndedAt = ended - s.ExecutionFinished = true - if s.finalized { - return - } - s.Err = err - if errors.Is(err, context.Canceled) || s.CancelRequested { - s.Status = RunCanceled - if err != nil { - s.Errors = append(s.Errors, err.Error()) + r.persistMu.Lock() + finishedSnapshot := r.snapshot() + finishedSnapshot.EndedAt = ended + finishedSnapshot.ExecutionFinished = true + if !finishedSnapshot.finalized { + finishedSnapshot.Err = err + finishedSnapshot.Error = "" + if err != nil { + finishedSnapshot.Error = err.Error() + if errors.Is(err, context.Canceled) { + finishedSnapshot.Status = RunCanceled + } else { + finishedSnapshot.Status = RunFailed } - } else if err != nil { - s.Status = RunFailed - s.Errors = append(s.Errors, err.Error()) } else { - s.Status = RunSucceeded - } - }) - if persistErr := rm.persist(r); persistErr != nil { - retryErr := rm.recordPersistenceFailure(r, persistErr) - if retryErr != nil { - err = errors.Join(err, persistErr, retryErr) - } else { - err = errors.Join(err, persistErr) + finishedSnapshot.Status = RunSucceeded } } + if err := rm.persistAndReplace(r, finishedSnapshot); err != nil { + r.persistMu.Unlock() + rm.failAfterPersistenceError(r, err) + return + } + r.persistMu.Unlock() - finalKind := EventRunCompleted - if err != nil { + snapshot := r.snapshot() + var finalKind EventKind + switch snapshot.Status { + case RunSucceeded: + finalKind = EventRunCompleted + case RunFailed: finalKind = EventRunFailed + case RunCanceled, RunSuperseded: + finalKind = EventRunCanceled + default: + return } rm.mailbox.Publish(Event{RunID: id, Kind: finalKind, Time: ended, Err: err}) } -func (rm *RunManager) recordPersistenceFailure(r *run, persistErr error) error { - durabilityErr := fmt.Errorf("daemon: durable run state write failed: %w", persistErr) - r.update(func(s *RunSnapshot) { - s.Status = RunFailed - s.EndedAt = time.Now() - s.ExecutionFinished = true - s.Err = durabilityErr - s.Errors = append(s.Errors, durabilityErr.Error()) - }) - retryErr := rm.persist(r) - if retryErr != nil { - r.update(func(s *RunSnapshot) { - s.Errors = append(s.Errors, fmt.Sprintf("daemon: retry durable run state write failed: %v", retryErr)) - }) - } - return retryErr -} - func (rm *RunManager) Snapshot(id string) (RunSnapshot, bool) { r, ok := rm.lookupRun(id) if !ok { @@ -414,8 +408,14 @@ func (rm *RunManager) List() []RunSnapshot { snaps := make([]RunSnapshot, len(runs)) for i, r := range runs { - snaps[i] = cloneSnapshot(r.snapshot()) + snaps[i] = r.snapshot() } + sort.Slice(snaps, func(i, j int) bool { + if snaps[i].QueuedAt.Equal(snaps[j].QueuedAt) { + return snaps[i].ID < snaps[j].ID + } + return snaps[i].QueuedAt.Before(snaps[j].QueuedAt) + }) return snaps } @@ -429,45 +429,13 @@ func (rm *RunManager) Cancel(id string) error { return fmt.Errorf("daemon: no run %q", id) } snapshot := r.snapshot() - if snapshot.Status == RunCanceled { - return nil - } if isTerminalRunStatus(snapshot.Status) { return fmt.Errorf("daemon: run %q is already %s", id, snapshot.Status) } - if snapshot.CancelRequested { - r.cancel() - if snapshot.Status == RunAwaitingMerge || snapshot.Status == RunAwaitingReview { - r.update(func(s *RunSnapshot) { - s.Status = RunCanceled - s.ExecutionFinished = true - s.EndedAt = time.Now() - s.Err = context.Canceled - }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist canceled run: %w", err) - } - } - return nil - } - r.update(func(s *RunSnapshot) { s.CancelRequested = true }) - if err := rm.persist(r); err != nil { - r.cancel() - return fmt.Errorf("persist cancellation request: %w", err) - } - if snapshot.Status == RunAwaitingMerge || snapshot.Status == RunAwaitingReview { - r.update(func(s *RunSnapshot) { - s.Status = RunCanceled - s.ExecutionFinished = true - s.EndedAt = time.Now() - s.Err = context.Canceled - s.Errors = append(s.Errors, context.Canceled.Error()) - }) - r.cancel() - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist canceled run: %w", err) + if snapshot.Status == RunQueued { + if handled, err := rm.cancelQueued(r); handled { + return err } - return nil } r.cancel() return nil @@ -477,29 +445,48 @@ func isTerminalRunStatus(s RunStatus) bool { return s == RunSucceeded || s == RunFailed || s == RunCanceled || s == RunSuperseded } -// Finish lets a WorkFunc declare a run's definitive Status and a -// human-readable Message just before it returns, so execute's normal -// nil-error-means-RunSucceeded inference does not overwrite it (see -// RunSnapshot.finalized). status may be any RunStatus, including RunRunning -// for a run that must stay open pending action made cannot itself take. func (rm *RunManager) Finish(id string, status RunStatus, message string) error { r, ok := rm.lookupRun(id) if !ok { return fmt.Errorf("daemon: no run %q", id) } - r.update(func(s *RunSnapshot) { - s.Status = status - s.Message = message - s.finalized = true - }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist finished run: %w", err) + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + candidate.Status = status + candidate.Message = message + candidate.ExecutionFinished = status == RunAwaitingMerge || isTerminalRunStatus(status) + candidate.finalized = true + if err := rm.persistAndReplace(r, candidate); err != nil { + return err } return nil } -// ErrRunSuperseded marks a run SupersedeQueued dropped before it ever -// started, because a newer push to the same branch arrived first. +func (rm *RunManager) failAfterPersistenceError(r *run, persistErr error) { + r.persistMu.Lock() + defer r.persistMu.Unlock() + failure := fmt.Errorf("daemon: durable run state unavailable: %w", persistErr) + ended := time.Now() + candidate := r.snapshot() + candidate.Status = RunFailed + candidate.Err = failure + candidate.Error = failure.Error() + candidate.Message = "run state persistence failed" + candidate.EndedAt = ended + candidate.ExecutionFinished = true + candidate.finalized = true + if retryErr := rm.persistAndReplace(r, candidate); retryErr != nil { + failure = fmt.Errorf("%w; retrying failed state also failed: %v", failure, retryErr) + candidate.Err = failure + candidate.Error = failure.Error() + r.replace(candidate) + } + snapshot := r.snapshot() + rm.mailbox.Publish(Event{RunID: snapshot.ID, Kind: EventRunFailed, Time: ended, Err: failure}) + rm.signalActivity() +} + var ErrRunSuperseded = errors.New("daemon: run superseded by a newer push to the same branch") // SupersedeQueued drops every still-queued (not yet started) job for the @@ -532,18 +519,101 @@ func (rm *RunManager) SupersedeQueued(repo, branch string) error { now := time.Now() var firstErr error for _, j := range dropped { - j.run.update(func(s *RunSnapshot) { - s.Status = RunSuperseded - s.Err = ErrRunSuperseded - s.Errors = append(s.Errors, ErrRunSuperseded.Error()) - s.EndedAt = now - s.ExecutionFinished = true - }) - if err := rm.persist(j.run); err != nil && firstErr == nil { - firstErr = fmt.Errorf("persist superseded run: %w", err) + j.run.persistMu.Lock() + candidate := j.run.snapshot() + candidate.Status = RunSuperseded + candidate.Err = ErrRunSuperseded + candidate.Error = ErrRunSuperseded.Error() + candidate.ExecutionFinished = true + candidate.EndedAt = now + err := rm.persistAndReplace(j.run, candidate) + j.run.persistMu.Unlock() + if err != nil { + if firstErr == nil { + firstErr = err + } + rm.failAfterPersistenceError(j.run, err) + continue } - rm.mailbox.Publish(Event{RunID: j.run.snapshot().ID, Kind: EventRunFailed, Time: now, Err: ErrRunSuperseded}) + rm.mailbox.Publish(Event{RunID: j.run.snapshot().ID, Kind: EventRunCanceled, Time: now, Err: ErrRunSuperseded}) rm.signalActivity() } return firstErr } + +func (rm *RunManager) cancelQueued(target *run) (bool, error) { + snapshot := target.snapshot() + rm.mu.Lock() + rq := rm.repos[snapshot.Repo] + rm.mu.Unlock() + if rq == nil { + return rm.cancelQueuedRun(target) + } + rq.mu.Lock() + removed := false + for i, job := range rq.pending { + if job.run == target { + rq.pending = append(rq.pending[:i], rq.pending[i+1:]...) + removed = true + break + } + } + if !removed { + active := rq.active + rq.mu.Unlock() + if active { + return false, nil + } + return rm.cancelQueuedRun(target) + } + rq.mu.Unlock() + return rm.cancelQueuedRun(target) +} + +func (rm *RunManager) cancelQueuedRun(target *run) (bool, error) { + snapshot := target.snapshot() + now := time.Now() + target.persistMu.Lock() + candidate := target.snapshot() + if candidate.Status != RunQueued { + target.persistMu.Unlock() + return false, nil + } + target.cancel() + candidate.Status = RunCanceled + candidate.Err = context.Canceled + candidate.Error = context.Canceled.Error() + candidate.EndedAt = now + candidate.ExecutionFinished = true + err := rm.persistAndReplace(target, candidate) + target.persistMu.Unlock() + if err != nil { + rm.failAfterPersistenceError(target, err) + return true, err + } + rm.mailbox.Publish(Event{RunID: snapshot.ID, Kind: EventRunCanceled, Time: now, Err: context.Canceled}) + rm.signalActivity() + return true, nil +} + +func (rm *RunManager) persistAndReplace(r *run, snapshot RunSnapshot) error { + rm.durableMu.Lock() + defer rm.durableMu.Unlock() + rm.mu.Lock() + err := rm.persistSnapshotLocked(snapshot) + rm.mu.Unlock() + if err != nil { + return err + } + r.replace(snapshot) + return nil +} + +func (rm *RunManager) HasActiveRuns() bool { + for _, snapshot := range rm.List() { + if snapshot.Status == RunQueued || snapshot.Status == RunRunning || snapshot.Status == RunAwaitingMerge { + return true + } + } + return false +} diff --git a/internal/daemon/runmanager_test.go b/internal/daemon/runmanager_test.go index f5f4dd1..d845cc4 100644 --- a/internal/daemon/runmanager_test.go +++ b/internal/daemon/runmanager_test.go @@ -13,15 +13,15 @@ func TestRunManager_SequentialQueuing(t *testing.T) { rm := NewRunManager() const repo = "gate-repo-A" - var active int32 - var overlapped int32 + var active atomic.Int32 + var overlapped atomic.Int32 work := func(ctx context.Context, emit func(Event)) error { - if atomic.AddInt32(&active, 1) > 1 { - atomic.StoreInt32(&overlapped, 1) + if active.Add(1) > 1 { + overlapped.Store(1) } time.Sleep(50 * time.Millisecond) - atomic.AddInt32(&active, -1) + active.Add(-1) return nil } @@ -56,7 +56,7 @@ func TestRunManager_SequentialQueuing(t *testing.T) { } } - if atomic.LoadInt32(&overlapped) != 0 { + if overlapped.Load() != 0 { t.Fatal("run1 and run2 executed concurrently, expected per-repo serialization") } @@ -89,7 +89,7 @@ func TestRunManager_DifferentRepposRunConcurrently(t *testing.T) { t.Fatalf("submit run2: %v", err) } - for i := 0; i < 2; i++ { + for range 2 { select { case <-started: case <-time.After(2 * time.Second): diff --git a/internal/daemon/runstate.go b/internal/daemon/runstate.go index 0137733..1be7092 100644 --- a/internal/daemon/runstate.go +++ b/internal/daemon/runstate.go @@ -1,29 +1,16 @@ package daemon -import "fmt" - -func cloneSnapshot(snapshot RunSnapshot) RunSnapshot { - snapshot.Errors = append([]string(nil), snapshot.Errors...) - snapshot.Findings = append([]RunFinding(nil), snapshot.Findings...) - for i := range snapshot.Findings { - snapshot.Findings[i].Paths = append([]string(nil), snapshot.Findings[i].Paths...) - } - snapshot.Stages = append([]StageResult(nil), snapshot.Stages...) - snapshot.PendingFindings = append([]AskUserFinding(nil), snapshot.PendingFindings...) - snapshot.SubmissionEvents = append([]SubmissionEvent(nil), snapshot.SubmissionEvents...) - if snapshot.Decisions != nil { - original := snapshot.Decisions - snapshot.Decisions = make(map[string]string, len(original)) - for key, value := range original { - snapshot.Decisions[key] = value - } - } - return snapshot -} +import ( + "fmt" + "slices" +) type StageResult struct { - Name string `json:"name"` - Result string `json:"result"` + Name string `json:"name"` + Result string `json:"result"` + Message string `json:"message,omitempty"` + Error string `json:"error,omitempty"` + EvidenceRefs []string `json:"evidence_refs,omitempty"` } type AskUserFinding struct { @@ -36,11 +23,13 @@ func (rm *RunManager) UpdateStages(id string, stages []StageResult) error { if !ok { return fmt.Errorf("daemon: no run %q", id) } - r.update(func(s *RunSnapshot) { - s.Stages = append([]StageResult(nil), stages...) - }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist stages for run %q: %w", id, err) + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + candidate.Stages = cloneStageResults(stages) + candidate.CurrentStage = currentStage(candidate.Stages) + if err := rm.persistAndReplace(r, candidate); err != nil { + return err } return nil } @@ -50,21 +39,86 @@ func (rm *RunManager) UpdatePendingFindings(id string, findings []AskUserFinding if !ok { return fmt.Errorf("daemon: no run %q", id) } - r.update(func(s *RunSnapshot) { - s.PendingFindings = append([]AskUserFinding(nil), findings...) - if len(findings) > 0 && s.Status == RunRunning { - s.Status = RunAwaitingReview - } - if len(findings) == 0 && s.Status == RunAwaitingReview { - s.Status = RunRunning - } - }) - if err := rm.persist(r); err != nil { - return fmt.Errorf("persist pending findings for run %q: %w", id, err) + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + candidate.PendingFindings = append([]AskUserFinding(nil), findings...) + if len(findings) > 0 && candidate.Status == RunRunning { + candidate.Status = RunAwaitingReview + } + if len(findings) == 0 && candidate.Status == RunAwaitingReview { + candidate.Status = RunRunning + } + if err := rm.persistAndReplace(r, candidate); err != nil { + return err } return nil } +func (rm *RunManager) SetCurrentStage(id, stage string) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + candidate.CurrentStage = stage + if err := rm.persistAndReplace(r, candidate); err != nil { + return err + } + return nil +} + +func (rm *RunManager) AddEvidenceRef(id, ref string) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + if !slices.Contains(candidate.EvidenceRefs, ref) { + candidate.EvidenceRefs = append(candidate.EvidenceRefs, ref) + } + if err := rm.persistAndReplace(r, candidate); err != nil { + return err + } + return nil +} + +func (rm *RunManager) UpdateSubmissionOutput(id, outputSHA string) error { + r, ok := rm.lookupRun(id) + if !ok { + return fmt.Errorf("daemon: no run %q", id) + } + r.persistMu.Lock() + defer r.persistMu.Unlock() + candidate := r.snapshot() + candidate.OutputSHA = outputSHA + if err := rm.persistAndReplace(r, candidate); err != nil { + return err + } + return nil +} + +func cloneStageResults(stages []StageResult) []StageResult { + out := append([]StageResult(nil), stages...) + for i := range out { + out[i].EvidenceRefs = append([]string(nil), stages[i].EvidenceRefs...) + } + return out +} + +func currentStage(stages []StageResult) string { + for _, stage := range stages { + if stage.Result != "pass" { + return stage.Name + } + } + return "" +} + func (rm *RunManager) lookupRun(id string) (*run, bool) { rm.mu.Lock() r, ok := rm.runs[id] diff --git a/internal/daemon/runstate_test.go b/internal/daemon/runstate_test.go index 3257950..1d3b95a 100644 --- a/internal/daemon/runstate_test.go +++ b/internal/daemon/runstate_test.go @@ -2,6 +2,7 @@ package daemon import ( "context" + "reflect" "testing" "time" ) @@ -34,7 +35,7 @@ func TestRunManager_UpdateStagesVisibleViaSnapshot(t *testing.T) { if len(snap.Stages) != 2 { t.Fatalf("Stages = %+v, want 2 entries", snap.Stages) } - if snap.Stages[0] != stages[0] || snap.Stages[1] != stages[1] { + if !reflect.DeepEqual(snap.Stages, stages) { t.Errorf("Stages = %+v, want %+v", snap.Stages, stages) } } @@ -115,7 +116,7 @@ func TestRunManager_UpdateStagesReflectsListToo(t *testing.T) { for _, r := range runs { if r.ID == id { found = true - if len(r.Stages) != 1 || r.Stages[0] != stages[0] { + if len(r.Stages) != 1 || !reflect.DeepEqual(r.Stages[0], stages[0]) { t.Errorf("List() Stages = %+v, want %+v", r.Stages, stages) } } diff --git a/internal/daemon/store.go b/internal/daemon/store.go index cf25ae3..e14b73d 100644 --- a/internal/daemon/store.go +++ b/internal/daemon/store.go @@ -113,7 +113,7 @@ func OpenRunStore(path string) (*RunStore, map[string]RunSnapshot, error) { if record.Version != runStoreRecordVersion || record.Kind != "snapshot" { return nil, nil, fmt.Errorf("daemon: unsupported run store record version %d or kind %q", record.Version, record.Kind) } - snapshots[record.Snapshot.ID] = restoreSnapshot(record.Snapshot) + snapshots[record.Snapshot.ID] = restoreStoredSnapshot(record.Snapshot) } return store, snapshots, nil } @@ -201,7 +201,7 @@ func persistSnapshot(snapshot RunSnapshot) persistedSnapshot { } } -func restoreSnapshot(snapshot persistedSnapshot) RunSnapshot { +func restoreStoredSnapshot(snapshot persistedSnapshot) RunSnapshot { var runErr error if len(snapshot.Errors) > 0 { runErr = errors.New(evidence.RedactString(snapshot.Errors[len(snapshot.Errors)-1])) diff --git a/internal/evidence/evidence_contract_test.go b/internal/evidence/evidence_contract_test.go new file mode 100644 index 0000000..ce9496f --- /dev/null +++ b/internal/evidence/evidence_contract_test.go @@ -0,0 +1,91 @@ +package evidence_test + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "sync" + "testing" + + "github.com/douglasjarquin/made/internal/evidence" +) + +func TestOrphanBranchStore_ConcurrentWritesRetainBothRuns(t *testing.T) { + repo := t.TempDir() + initGitRepo(t, repo) + store := &evidence.OrphanBranchStore{RepoPath: repo} + + start := make(chan struct{}) + errCh := make(chan error, 2) + var wg sync.WaitGroup + for _, runID := range []string{"run-a", "run-b"} { + wg.Add(1) + go func(id string) { + defer wg.Done() + <-start + errCh <- store.WriteEvidence(id, map[string][]byte{ + "result.json": fmt.Appendf(nil, `{"run_id":%q}`, id), + }) + }(runID) + } + close(start) + wg.Wait() + close(errCh) + for err := range errCh { + if err != nil { + t.Fatalf("concurrent evidence write failed: %v", err) + } + } + + tree := gitOutput(t, repo, "ls-tree", "-r", "--name-only", "refs/heads/made-evidence") + for _, runID := range []string{"run-a", "run-b"} { + if !containsLine(tree, filepath.Join(runID, "result.json")) { + t.Fatalf("evidence branch missing %s: %s", runID, tree) + } + } +} + +func TestInRepoStoreRejectsSymlinkedEvidenceDirectory(t *testing.T) { + repo := t.TempDir() + outside := t.TempDir() + evidenceRoot := filepath.Join(repo, ".made", "evidence") + if err := os.MkdirAll(filepath.Dir(evidenceRoot), 0o755); err != nil { + t.Fatalf("create evidence parent: %v", err) + } + if err := os.Symlink(outside, evidenceRoot); err != nil { + t.Fatalf("create evidence symlink: %v", err) + } + + store := &evidence.InRepoStore{RepoPath: repo, Dir: ".made/evidence"} + if err := store.WriteEvidence("run-escape", map[string][]byte{"result.json": []byte("secret")}); err == nil { + t.Fatal("WriteEvidence accepted a symlinked evidence directory") + } + if _, err := os.Stat(filepath.Join(outside, "run-escape", "result.json")); !os.IsNotExist(err) { + t.Fatalf("symlinked evidence directory received data: err=%v", err) + } +} + +func initGitRepo(t *testing.T, dir string) { + t.Helper() + gitOutput(t, dir, "init", "-q") + gitOutput(t, dir, "-c", "user.name=fixture", "-c", "user.email=fixture@example.com", "commit", "--allow-empty", "-m", "initial") +} + +func gitOutput(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(cmd.Env, "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", "SSH_AUTH_SOCK=") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return string(out) +} + +func containsLine(output, want string) bool { + return slices.Contains(strings.Split(output, "\n"), want) +} diff --git a/internal/evidence/orphan.go b/internal/evidence/orphan.go index c5dacb3..2bd4fc9 100644 --- a/internal/evidence/orphan.go +++ b/internal/evidence/orphan.go @@ -74,53 +74,65 @@ func (s *OrphanBranchStore) WriteEvidenceContext(ctx context.Context, runID stri defer func() { _ = os.RemoveAll(idxDir) }() indexEnv := []string{"GIT_INDEX_FILE=" + idxDir + "/index"} - parent, err := s.runGit(ctx, nil, nil, "rev-parse", "--verify", ref) - hasParent := err == nil - if hasParent { - if _, err := s.runGit(ctx, indexEnv, nil, "read-tree", parent); err != nil { - return fmt.Errorf("evidence: seed scratch index from existing evidence branch: %w", err) - } - } - names := make([]string, 0, len(files)) for name := range files { names = append(names, name) } sort.Strings(names) - for _, name := range names { - blobSHA, err := s.runGit(ctx, indexEnv, Redact(files[name]), "hash-object", "-w", "--stdin") - if err != nil { - return fmt.Errorf("evidence: hash evidence file %q: %w", name, err) + var lastUpdateErr error + for range 8 { + if err := ctx.Err(); err != nil { + return fmt.Errorf("evidence: write evidence branch: %w", err) } - entryPath := path.Join(runID, name) - if _, err := s.runGit(ctx, indexEnv, nil, "update-index", "--add", "--cacheinfo", "100644,"+blobSHA+","+entryPath); err != nil { - return fmt.Errorf("evidence: stage evidence file %q: %w", name, err) + parent, parentErr := s.runGit(ctx, nil, nil, "rev-parse", "--verify", ref) + hasParent := parentErr == nil + if hasParent { + if _, err := s.runGit(ctx, indexEnv, nil, "read-tree", parent); err != nil { + return fmt.Errorf("evidence: seed scratch index from existing evidence branch: %w", err) + } + } else if _, err := s.runGit(ctx, indexEnv, nil, "read-tree", "--empty"); err != nil { + return fmt.Errorf("evidence: clear scratch index: %w", err) } - } - treeSHA, err := s.runGit(ctx, indexEnv, nil, "write-tree") - if err != nil { - return fmt.Errorf("evidence: write evidence tree: %w", err) - } + for _, name := range names { + blobSHA, err := s.runGit(ctx, indexEnv, Redact(files[name]), "hash-object", "-w", "--stdin") + if err != nil { + return fmt.Errorf("evidence: hash evidence file %q: %w", name, err) + } + entryPath := path.Join(runID, name) + if _, err := s.runGit(ctx, indexEnv, nil, "update-index", "--add", "--cacheinfo", "100644,"+blobSHA+","+entryPath); err != nil { + return fmt.Errorf("evidence: stage evidence file %q: %w", name, err) + } + } - commitArgs := []string{"commit-tree", treeSHA, "-m", "evidence: " + runID} - if hasParent { - commitArgs = append(commitArgs, "-p", parent) - } - commitSHA, err := s.runGit(ctx, commitAuthorEnv(), nil, commitArgs...) - if err != nil { - return fmt.Errorf("evidence: commit evidence tree: %w", err) - } + treeSHA, err := s.runGit(ctx, indexEnv, nil, "write-tree") + if err != nil { + return fmt.Errorf("evidence: write evidence tree: %w", err) + } - updateArgs := []string{"update-ref", ref, commitSHA} - if hasParent { - updateArgs = append(updateArgs, parent) - } - if _, err := s.runGit(ctx, nil, nil, updateArgs...); err != nil { - return fmt.Errorf("evidence: update evidence branch ref: %w", err) + commitArgs := []string{"commit-tree", treeSHA, "-m", "evidence: " + runID} + if hasParent { + commitArgs = append(commitArgs, "-p", parent) + } + commitSHA, err := s.runGit(ctx, commitAuthorEnv(), nil, commitArgs...) + if err != nil { + return fmt.Errorf("evidence: commit evidence tree: %w", err) + } + + updateArgs := []string{"update-ref", ref, commitSHA} + if hasParent { + updateArgs = append(updateArgs, parent) + } else { + updateArgs = append(updateArgs, strings.Repeat("0", 40)) + } + if _, err := s.runGit(ctx, nil, nil, updateArgs...); err != nil { + lastUpdateErr = err + continue + } + return nil } - return nil + return fmt.Errorf("evidence: update evidence branch ref after retries: %w", lastUpdateErr) } func (s *OrphanBranchStore) runGit(ctx context.Context, extraEnv []string, stdin []byte, args ...string) (string, error) { diff --git a/internal/github/client.go b/internal/github/client.go index 8ebd0d7..e56827c 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net/url" "os" "strconv" "strings" @@ -36,37 +37,17 @@ type CreatePROptions struct { Head string } -type Check struct { - Name string `json:"name"` - Status string `json:"state"` - Conclusion string `json:"conclusion"` - WorkflowRunID string `json:"workflowRunId"` - DetailsURL string `json:"detailsUrl"` +type CheckResult struct { + Name string `json:"name"` + State string `json:"state"` + Bucket string `json:"bucket"` + Link string `json:"link"` + RunID string `json:"-"` } -func (c *Check) UnmarshalJSON(data []byte) error { - var wire struct { - Name string `json:"name"` - Status string `json:"state"` - Conclusion string `json:"conclusion"` - WorkflowRunID json.RawMessage `json:"workflowRunId"` - DetailsURL string `json:"detailsUrl"` - } - if err := json.Unmarshal(data, &wire); err != nil { - return err - } - var runID string - if len(wire.WorkflowRunID) > 0 && string(wire.WorkflowRunID) != "null" { - if err := json.Unmarshal(wire.WorkflowRunID, &runID); err != nil { - var numeric json.Number - if err := json.Unmarshal(wire.WorkflowRunID, &numeric); err != nil { - return fmt.Errorf("github: parse workflow run ID: %w", err) - } - runID = numeric.String() - } - } - *c = Check{Name: wire.Name, Status: wire.Status, Conclusion: wire.Conclusion, WorkflowRunID: runID, DetailsURL: wire.DetailsURL} - return nil +type ChecksResult struct { + Checks []CheckResult + ExitCode int } func (c *Client) AuthStatus(ctx context.Context) error { @@ -137,7 +118,6 @@ func (c *Client) MergeableState(ctx context.Context, prURL string) (string, erro if err := c.AuthStatus(ctx); err != nil { return "", err } - res, err := c.run(ctx, "pr", "view", prURL, "--json", "mergeStateStatus") if err != nil { return "", fmt.Errorf("github: run gh pr view: %w", err) @@ -145,16 +125,52 @@ func (c *Client) MergeableState(ctx context.Context, prURL string) (string, erro if res.ExitCode != 0 { return "", fmt.Errorf("github: gh pr view failed: %s", strings.TrimSpace(string(res.Stderr))) } - var payload struct { MergeStateStatus string `json:"mergeStateStatus"` } if err := json.Unmarshal(res.Stdout, &payload); err != nil { - return "", fmt.Errorf("github: parse gh pr view output: %w: stdout=%s", err, res.Stdout) + return "", fmt.Errorf("github: parse gh pr view output: %w", err) } return payload.MergeStateStatus, nil } +func (c *Client) PRChecks(ctx context.Context, prURL string) (ChecksResult, error) { + if strings.TrimSpace(prURL) == "" { + return ChecksResult{}, fmt.Errorf("github: pull request URL is required for checks") + } + if err := c.AuthStatus(ctx); err != nil { + return ChecksResult{}, err + } + + res, err := c.run(ctx, "pr", "checks", prURL, "--json", "name,state,bucket,link") + if err != nil { + return ChecksResult{}, fmt.Errorf("github: run gh pr checks: %w", err) + } + if len(strings.TrimSpace(string(res.Stdout))) == 0 { + return ChecksResult{}, fmt.Errorf("github: gh pr checks returned no JSON (exit %d): %s", res.ExitCode, strings.TrimSpace(string(res.Stderr))) + } + + var checks []CheckResult + if err := json.Unmarshal(res.Stdout, &checks); err != nil { + return ChecksResult{}, fmt.Errorf("github: parse gh pr checks output: %w: stdout=%s", err, res.Stdout) + } + if len(checks) == 0 { + return ChecksResult{}, fmt.Errorf("github: gh pr checks returned an empty check set") + } + for i := range checks { + checks[i].RunID = workflowRunID(checks[i].Link) + } + if res.ExitCode == 0 { + for _, check := range checks { + bucket := strings.ToLower(strings.TrimSpace(check.Bucket)) + if bucket != "pass" && bucket != "skipping" && bucket != "neutral" { + return ChecksResult{}, fmt.Errorf("github: gh pr checks exit 0 with non-success bucket %q for %q", check.Bucket, check.Name) + } + } + } + return ChecksResult{Checks: checks, ExitCode: res.ExitCode}, nil +} + func (c *Client) CheckLogs(ctx context.Context, runID string) (string, error) { if err := validateWorkflowRunID(runID); err != nil { return "", err @@ -195,38 +211,6 @@ func (c *Client) RerunCheck(ctx context.Context, runID string) error { return nil } -func (c *Client) Checks(ctx context.Context, prURL string) ([]Check, error) { - if strings.TrimSpace(prURL) == "" { - return nil, fmt.Errorf("github: pull request URL is required for checks") - } - if err := c.AuthStatus(ctx); err != nil { - return nil, err - } - res, err := c.run(ctx, "pr", "checks", prURL, "--json", "name,state,conclusion,workflowRunId,detailsUrl") - if err != nil { - return nil, fmt.Errorf("github: run gh pr checks: %w", err) - } - if res.ExitCode != 0 { - return nil, fmt.Errorf("github: gh pr checks failed: %s", strings.TrimSpace(string(res.Stderr))) - } - var checks []Check - if err := json.Unmarshal(res.Stdout, &checks); err != nil { - return nil, fmt.Errorf("github: parse gh pr checks output: %w", err) - } - return checks, nil -} - -func validateWorkflowRunID(value string) error { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - return fmt.Errorf("github: workflow run ID is required") - } - if _, err := strconv.ParseInt(trimmed, 10, 64); err != nil { - return fmt.Errorf("github: workflow run ID must be numeric, got %q", value) - } - return nil -} - func (c *Client) run(ctx context.Context, args ...string) (*exec.Result, error) { binary := c.Binary if binary == "" { @@ -249,3 +233,26 @@ func lastLine(out []byte) string { lines := strings.Split(strings.TrimSpace(string(out)), "\n") return strings.TrimSpace(lines[len(lines)-1]) } + +func validateWorkflowRunID(runID string) error { + if _, err := strconv.ParseUint(runID, 10, 64); err != nil { + return fmt.Errorf("github: invalid workflow run ID %q: %w", runID, err) + } + return nil +} + +func workflowRunID(link string) string { + parsed, err := url.Parse(link) + if err != nil { + return "" + } + parts := strings.Split(strings.Trim(parsed.Path, "/"), "/") + for i := 0; i+1 < len(parts); i++ { + if parts[i] == "runs" { + if _, err := strconv.ParseUint(parts[i+1], 10, 64); err == nil { + return parts[i+1] + } + } + } + return "" +} diff --git a/internal/github/client_contract_test.go b/internal/github/client_contract_test.go new file mode 100644 index 0000000..6427298 --- /dev/null +++ b/internal/github/client_contract_test.go @@ -0,0 +1,38 @@ +package github_test + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/douglasjarquin/made/internal/github/githubtest" +) + +func TestStrictFakeGHRejectsUnsupportedJSONFields(t *testing.T) { + bin := githubtest.Build(t) + scenarioDir := t.TempDir() + cmd := exec.Command(bin, "pr", "view", "https://github.com/example/repo/pull/1", "--json", "mergeStateStatus", "--unexpected") + cmd.Env = append(os.Environ(), "FAKE_GH_STATE_DIR="+scenarioDir) + if output, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("strict fake accepted unsupported invocation, output=%s", output) + } +} + +func TestStrictFakeGHRejectsPRURLAsWorkflowRunID(t *testing.T) { + bin := githubtest.Build(t) + logPath := filepath.Join(t.TempDir(), "gh.log") + cmd := exec.Command(bin, "run", "view", "https://github.com/example/repo/pull/1", "--log") + cmd.Env = append(os.Environ(), "FAKE_GH_LOG_FILE="+logPath) + if output, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("strict fake accepted PR URL as workflow run ID, output=%s", output) + } +} + +func TestStrictFakeGHInvocationLogDoesNotAcceptLegacyMergeStateCommand(t *testing.T) { + bin := githubtest.Build(t) + cmd := exec.Command(bin, "pr", "view", "https://github.com/example/repo/pull/1", "--json", "mergeStateStatus") + if output, err := cmd.CombinedOutput(); err == nil { + t.Fatalf("legacy merge-state invocation was accepted: %s", output) + } +} diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 5e9122f..148f63e 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -36,8 +36,8 @@ func TestAuthStatus_FailureReturnsAuthError(t *testing.T) { if err == nil { t.Fatal("expected an error from AuthStatus") } - var authErr *github.AuthError - if !errors.As(err, &authErr) { + authErr, ok := errors.AsType[*github.AuthError](err) + if !ok { t.Fatalf("expected *github.AuthError, got %T: %v", err, err) } if !strings.Contains(authErr.Error(), "not logged into") { @@ -66,8 +66,8 @@ func TestCreatePR_AuthFailurePreventsPRCall(t *testing.T) { if err == nil { t.Fatal("expected CreatePR to fail when auth fails") } - var authErr *github.AuthError - if !errors.As(err, &authErr) { + _, ok := errors.AsType[*github.AuthError](err) + if !ok { t.Fatalf("expected *github.AuthError, got %T: %v", err, err) } @@ -100,15 +100,26 @@ func TestCreatePR_SuccessReturnsURL(t *testing.T) { } } -func TestMergeableState_ParsesJSON(t *testing.T) { - c := newClient(t, []string{`FAKE_GH_PR_VIEW_JSON={"mergeStateStatus":"BEHIND"}`}, "") +func TestPRChecks_ParsesJSON(t *testing.T) { + c := newClient(t, []string{`FAKE_GH_CHECKS_JSON=[{"name":"build","state":"COMPLETED","bucket":"pass","link":"https://github.com/example/repo/actions/runs/42"}]`}, "") - state, err := c.MergeableState(context.Background(), "https://github.com/example/repo/pull/42") + checks, err := c.PRChecks(context.Background(), "https://github.com/example/repo/pull/42") if err != nil { - t.Fatalf("MergeableState: %v", err) + t.Fatalf("PRChecks: %v", err) } - if state != "BEHIND" { - t.Fatalf("expected BEHIND, got %q", state) + if checks.ExitCode != 0 || len(checks.Checks) != 1 { + t.Fatalf("unexpected checks result: %+v", checks) + } + if checks.Checks[0].Bucket != "pass" || checks.Checks[0].RunID != "42" { + t.Fatalf("unexpected check fields: %+v", checks.Checks[0]) + } +} + +func TestPRChecks_RejectsEmptySuccessfulPayload(t *testing.T) { + c := newClient(t, []string{"FAKE_GH_CHECKS_JSON=[]"}, "") + + if _, err := c.PRChecks(context.Background(), "https://github.com/example/repo/pull/42"); err == nil { + t.Fatal("PRChecks accepted an empty successful payload") } } @@ -116,7 +127,7 @@ func TestMergeableState_AuthFailurePreventsCall(t *testing.T) { logPath := filepath.Join(t.TempDir(), "invocations.log") c := newClient(t, []string{"FAKE_GH_AUTH_EXIT_CODE=1"}, logPath) - _, err := c.MergeableState(context.Background(), "https://github.com/example/repo/pull/42") + _, err := c.PRChecks(context.Background(), "https://github.com/example/repo/pull/42") if err == nil { t.Fatal("expected an error when auth fails") } @@ -124,8 +135,8 @@ func TestMergeableState_AuthFailurePreventsCall(t *testing.T) { if readErr != nil { t.Fatalf("read invocation log: %v", readErr) } - if strings.Contains(string(data), "pr view") { - t.Fatalf("expected no pr view call after auth failure, log:\n%s", data) + if strings.Contains(string(data), "pr checks") { + t.Fatalf("expected no pr checks call after auth failure, log:\n%s", data) } } diff --git a/internal/github/live_test.go b/internal/github/live_test.go index 7a85d65..3c5e455 100644 --- a/internal/github/live_test.go +++ b/internal/github/live_test.go @@ -66,9 +66,9 @@ func TestLive_AuthStatusAndPRCreation(t *testing.T) { } t.Logf("created PR: %s", url) - state, err := c.MergeableState(context.Background(), url) + checks, err := c.PRChecks(context.Background(), url) if err != nil { - t.Fatalf("MergeableState: %v", err) + t.Fatalf("PRChecks: %v", err) } - t.Logf("mergeStateStatus: %s", state) + t.Logf("checks: %+v", checks) } diff --git a/internal/github/testdata/fakegh/main.go b/internal/github/testdata/fakegh/main.go index 72c3557..6c4334a 100644 --- a/internal/github/testdata/fakegh/main.go +++ b/internal/github/testdata/fakegh/main.go @@ -5,6 +5,7 @@ package main import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -28,27 +29,86 @@ func main() { return } - if code := os.Getenv("FAKE_GH_EXIT_CODE"); code != "" && code != "0" { - fmt.Fprintln(os.Stderr, envOr("FAKE_GH_STDERR", "fakegh: scripted failure")) - os.Exit(1) - } - switch { case len(args) >= 2 && args[0] == "pr" && args[1] == "create": + if !validPRCreateArgs(args[2:]) { + reject(args) + } + failIfScripted() fmt.Fprintln(os.Stdout, envOr("FAKE_GH_PR_URL", "https://github.com/example/repo/pull/1")) - case len(args) >= 2 && args[0] == "pr" && args[1] == "list": + case len(args) == 10 && args[0] == "pr" && args[1] == "list" && args[2] == "--state" && args[3] == "open" && args[4] == "--base" && args[6] == "--head" && args[8] == "--json" && args[9] == "url": + failIfScripted() fmt.Fprint(os.Stdout, envOr("FAKE_GH_PR_LIST_JSON", "[]")) - case len(args) >= 2 && args[0] == "pr" && args[1] == "checks": - fmt.Fprint(os.Stdout, checksResponse()) - case len(args) >= 2 && args[0] == "pr" && args[1] == "view": - fmt.Fprint(os.Stdout, prViewResponse()) - case len(args) >= 2 && args[0] == "run" && args[1] == "view": + case len(args) == 5 && args[0] == "pr" && args[1] == "checks" && args[3] == "--json" && args[4] == "name,state,bucket,link": + payload := checksResponse() + fmt.Fprint(os.Stdout, payload) + if code := envExitCode("FAKE_GH_CHECKS_EXIT_CODE"); code != 0 { + os.Exit(code) + } + if checksFail(payload) { + os.Exit(1) + } + case len(args) == 4 && args[0] == "run" && args[1] == "view" && isRunID(args[2]) && args[3] == "--log": + failIfScripted() fmt.Fprint(os.Stdout, envOr("FAKE_GH_RUN_LOG", "log line 1\nlog line 2\n")) - case len(args) >= 2 && args[0] == "run" && args[1] == "rerun": + case len(args) == 4 && args[0] == "run" && args[1] == "rerun" && isRunID(args[2]) && args[3] == "--failed": + failIfScripted() default: - fmt.Fprintf(os.Stderr, "fakegh: unrecognized args %v\n", args) - os.Exit(1) + reject(args) + } +} + +func reject(args []string) { + fmt.Fprintf(os.Stderr, "fakegh: unrecognized args %v\n", args) + os.Exit(2) +} + +func failIfScripted() { + if code := envExitCode("FAKE_GH_EXIT_CODE"); code != 0 { + fmt.Fprintln(os.Stderr, envOr("FAKE_GH_STDERR", "fakegh: scripted failure")) + os.Exit(code) + } +} + +func envExitCode(key string) int { + code := os.Getenv(key) + if code == "" || code == "0" { + return 0 + } + n, err := strconv.Atoi(code) + if err != nil || n < 1 || n > 125 { + return 1 } + return n +} + +func isRunID(value string) bool { + if value == "" { + return false + } + for _, r := range value { + if r < '0' || r > '9' { + return false + } + } + return true +} + +func validPRCreateArgs(args []string) bool { + if len(args) < 4 || len(args)%2 != 0 { + return false + } + seen := map[string]bool{} + for i := 0; i < len(args); i += 2 { + if args[i] != "--title" && args[i] != "--body" && args[i] != "--base" && args[i] != "--head" { + return false + } + if seen[args[i]] || args[i+1] == "" { + return false + } + seen[args[i]] = true + } + return seen["--title"] && seen["--body"] } func envOr(key, fallback string) string { @@ -58,31 +118,40 @@ func envOr(key, fallback string) string { return fallback } -func prViewResponse() string { - states := os.Getenv("FAKE_GH_PR_VIEW_STATES") - if states == "" { - return envOr("FAKE_GH_PR_VIEW_JSON", `{"mergeStateStatus":"CLEAN"}`) - } - list := strings.Split(states, ",") - idx := nextSequenceIndex("pr_view", len(list)) - return fmt.Sprintf(`{"mergeStateStatus":%q}`, strings.TrimSpace(list[idx])) -} - func checksResponse() string { - if value := os.Getenv("FAKE_GH_CHECKS_JSON"); value != "" { - return value + raw := envOr("FAKE_GH_CHECKS_JSON", `[{"name":"build","state":"COMPLETED","bucket":"pass","link":"https://github.com/example/repo/actions/runs/12345"}]`) + var checks []map[string]string + if err := json.Unmarshal([]byte(raw), &checks); err != nil { + fmt.Fprintf(os.Stderr, "fakegh: invalid FAKE_GH_CHECKS_JSON: %v\n", err) + os.Exit(2) } - states := os.Getenv("FAKE_GH_PR_VIEW_STATES") - if states == "" { - return `[{"name":"ci","state":"SUCCESS","conclusion":"SUCCESS","workflowRunId":1,"detailsUrl":"https://github.com/example/repo/actions/runs/1"}]` + if sequence := os.Getenv("FAKE_GH_CHECKS_BUCKETS"); sequence != "" { + buckets := strings.Split(sequence, ",") + bucket := strings.TrimSpace(buckets[nextSequenceIndex("checks", len(buckets))]) + for _, check := range checks { + check["bucket"] = bucket + check["state"] = "COMPLETED" + } } - list := strings.Split(states, ",") - idx := nextSequenceIndex("checks", len(list)) - state := strings.ToUpper(strings.TrimSpace(list[idx])) - if state == "CLEAN" { - return `[{"name":"ci","state":"SUCCESS","conclusion":"SUCCESS","workflowRunId":1,"detailsUrl":"https://github.com/example/repo/actions/runs/1"}]` + data, err := json.Marshal(checks) + if err != nil { + fmt.Fprintf(os.Stderr, "fakegh: encode checks: %v\n", err) + os.Exit(2) } - return `[{"name":"ci","state":"COMPLETED","conclusion":"FAILURE","workflowRunId":1,"detailsUrl":"https://github.com/example/repo/actions/runs/1"}]` + return string(data) +} + +func checksFail(raw string) bool { + var checks []map[string]string + if err := json.Unmarshal([]byte(raw), &checks); err != nil { + return true + } + for _, check := range checks { + if check["bucket"] != "pass" { + return true + } + } + return false } // nextSequenceIndex lets one scripted state sequence (e.g. "fails twice then @@ -110,7 +179,9 @@ func nextSequenceIndex(name string, length int) int { if idx >= length { idx = length - 1 } - _ = os.WriteFile(path, []byte(strconv.Itoa(count+1)), 0o644) + if err := os.WriteFile(path, []byte(strconv.Itoa(count+1)), 0o644); err != nil { + return idx + } return idx } @@ -119,6 +190,6 @@ func logInvocation(logPath string, args []string) { if err != nil { return } - defer f.Close() + defer func() { _ = f.Close() }() fmt.Fprintf(f, "invoked: args=%s\n", strings.Join(args, " ")) } diff --git a/internal/orchestrator/workfunc_test.go b/internal/orchestrator/workfunc_test.go index 4f462b7..b850baa 100644 --- a/internal/orchestrator/workfunc_test.go +++ b/internal/orchestrator/workfunc_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "strings" "testing" "time" @@ -50,7 +51,7 @@ func TestChain_RefusesDeliveryWhenRequiredStageDisabled(t *testing.T) { t.Fatalf("requireDeliveryStages error = %q, want disabled review stage", err) } snapshot, ok := rm.Snapshot(runID) - if !ok || len(snapshot.Stages) != 1 || snapshot.Stages[0] != (daemon.StageResult{Name: stageNameReview, Result: "skipped"}) { + if !ok || len(snapshot.Stages) != 1 || !reflect.DeepEqual(snapshot.Stages[0], daemon.StageResult{Name: stageNameReview, Result: "skipped"}) { t.Fatalf("disabled stage snapshot = %+v, want review/skipped", snapshot.Stages) } } @@ -206,7 +207,7 @@ func cleanReviewOptions(t *testing.T) review.Options { scenarioPath := writeScenario(t, agent.Findings{}) return review.Options{ BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{"FAKE_AGENT_KIND=codex", "FAKE_AGENT_SCENARIO=" + scenarioPath}, } } @@ -228,7 +229,7 @@ func TestNewWorkFunc_FullPassEndsRunningWithAwaitingMergeMessage(t *testing.T) { ghBin := githubtest.Build(t) cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{Test: "true", Lint: "true"}, CI: config.CI{RerunBudget: 1}, } @@ -270,7 +271,7 @@ func TestNewWorkFunc_FullPassPRTitleMatchesPushedCommitSubject(t *testing.T) { ghBin := githubtest.Build(t) ghLog := filepath.Join(t.TempDir(), "gh-invocations.log") cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{Test: "true", Lint: "true"}, CI: config.CI{RerunBudget: 1}, } @@ -325,7 +326,7 @@ func TestNewWorkFunc_TestFailureHaltsBeforeLaterStages(t *testing.T) { ghLog := filepath.Join(t.TempDir(), "gh-invocations.log") cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{ Test: "exit 1", Lint: "touch " + lintMarker, @@ -387,7 +388,7 @@ func TestNewWorkFunc_DocumentFindingParksThenRejectedFailsRun(t *testing.T) { ghBin := githubtest.Build(t) cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{Test: "true", Lint: "true"}, CI: config.CI{RerunBudget: 1}, Document: config.Document{Rules: []config.DocumentRule{ @@ -413,7 +414,9 @@ func TestNewWorkFunc_DocumentFindingParksThenRejectedFailsRun(t *testing.T) { t.Fatalf("expected one pending finding on stage %q, got %+v", stageNameDocument, parked.PendingFindings) } - reviewDecisions.Set(runID, stageNameDocument, daemon.ReviewRejected) + if err := reviewDecisions.Set(runID, stageNameDocument, daemon.ReviewRejected); err != nil { + t.Fatalf("set rejection: %v", err) + } snap := waitForRunEnded(t, rm, runID, 30*time.Second) if snap.Status != daemon.RunFailed { @@ -433,7 +436,7 @@ func TestNewWorkFunc_DocumentFindingParksThenApprovedResumesToCompletion(t *test ghBin := githubtest.Build(t) cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{Test: "true", Lint: "true"}, CI: config.CI{RerunBudget: 1}, Document: config.Document{Rules: []config.DocumentRule{ @@ -456,7 +459,9 @@ func TestNewWorkFunc_DocumentFindingParksThenApprovedResumesToCompletion(t *test t.Fatalf("expected parked run to stay RunAwaitingReview, got %v", parked.Status) } - reviewDecisions.Set(runID, stageNameDocument, daemon.ReviewApproved) + if err := reviewDecisions.Set(runID, stageNameDocument, daemon.ReviewApproved); err != nil { + t.Fatalf("set approval: %v", err) + } snap := waitForRunEnded(t, rm, runID, 30*time.Second) if snap.Status != daemon.RunAwaitingMerge { @@ -494,7 +499,7 @@ func TestNewWorkFunc_PushSucceedsThenPRFailsMessageNamesPushedBranch(t *testing. ghBin := githubtest.Build(t) cfg := config.Config{ - Agent: string(agent.KindClaude), + Agent: string(agent.KindCodex), Commands: config.Commands{Test: "true", Lint: "true"}, CI: config.CI{RerunBudget: 1}, } diff --git a/internal/pipeline/ci/ci.go b/internal/pipeline/ci/ci.go index 724eb3b..c13c5c5 100644 --- a/internal/pipeline/ci/ci.go +++ b/internal/pipeline/ci/ci.go @@ -51,83 +51,84 @@ func Run(ctx context.Context, ghClient *github.Client, prURL string, rerunBudget reruns := 0 for { - checks, err := ghClient.Checks(ctx, prURL) + checks, err := ghClient.PRChecks(ctx, prURL) if err != nil { return Result{}, err } - if len(checks) == 0 { - return Result{}, fmt.Errorf("ci: GitHub returned no checks for %s", prURL) - } - allPassed := true - var failing *github.Check - pending := false - for i := range checks { - check := checks[i] - if checkPassed(check) { - continue - } - allPassed = false - if checkPending(check) { - pending = true + if hasPendingChecks(checks.Checks) { + select { + case <-ctx.Done(): + return Result{OK: false, Message: ctx.Err().Error(), RerunsUsed: reruns}, nil + case <-time.After(pollInterval): continue } - if failing == nil { - failing = &check - } } - if allPassed { + if checks.ExitCode == 0 { return Result{ OK: true, Message: fmt.Sprintf("checks passed for %s after %d rerun(s)", prURL, reruns), RerunsUsed: reruns, }, nil } - if pending && failing == nil { - select { - case <-ctx.Done(): - return Result{}, ctx.Err() - case <-time.After(pollInterval): - } - continue - } - if failing == nil { - return Result{}, fmt.Errorf("ci: check state was neither passing, pending, nor failing") - } if reruns >= rerunBudget { - excerpt, logErr := ghClient.CheckLogs(ctx, failing.WorkflowRunID) + runID := firstWorkflowRunID(checks.Checks) + if runID == "" { + return Result{ + OK: false, + Message: fmt.Sprintf("checks failed for %s after exhausting rerun budget (%d), but no workflow run ID was present in gh pr checks output", prURL, rerunBudget), + RerunsUsed: reruns, + }, nil + } + excerpt, logErr := ghClient.CheckLogs(ctx, runID) if logErr != nil { return Result{}, logErr } return Result{ OK: false, - Message: fmt.Sprintf("check %s still failing for %s after exhausting rerun budget (%d)", failing.Name, prURL, rerunBudget), + Message: fmt.Sprintf("checks still failing for %s after exhausting rerun budget (%d)", prURL, rerunBudget), RerunsUsed: reruns, LogExcerpt: excerpt, }, nil } - if err := ghClient.RerunCheck(ctx, failing.WorkflowRunID); err != nil { + runID := firstWorkflowRunID(checks.Checks) + if runID == "" { + return Result{ + OK: false, + Message: fmt.Sprintf("checks failed for %s but gh pr checks returned no workflow run ID for rerun", prURL), + RerunsUsed: reruns, + }, nil + } + if err := ghClient.RerunCheck(ctx, runID); err != nil { return Result{}, err } reruns++ select { case <-ctx.Done(): - return Result{}, ctx.Err() + return Result{OK: false, Message: ctx.Err().Error(), RerunsUsed: reruns}, nil case <-time.After(pollInterval): } } } -func checkPassed(check github.Check) bool { - status := strings.ToUpper(strings.TrimSpace(check.Status)) - conclusion := strings.ToUpper(strings.TrimSpace(check.Conclusion)) - return (status == "SUCCESS" || status == "COMPLETED") && (conclusion == "SUCCESS" || conclusion == "SUCCESSFUL" || conclusion == "NEUTRAL") +func firstWorkflowRunID(checks []github.CheckResult) string { + for _, check := range checks { + if check.RunID != "" { + return check.RunID + } + } + return "" } -func checkPending(check github.Check) bool { - status := strings.ToUpper(strings.TrimSpace(check.Status)) - conclusion := strings.TrimSpace(check.Conclusion) - return conclusion == "" || status == "QUEUED" || status == "IN_PROGRESS" || status == "PENDING" +func hasPendingChecks(checks []github.CheckResult) bool { + for _, check := range checks { + state := strings.ToUpper(strings.TrimSpace(check.State)) + bucket := strings.ToLower(strings.TrimSpace(check.Bucket)) + if bucket == "pending" || state == "PENDING" || state == "QUEUED" || state == "IN_PROGRESS" || state == "WAITING" || state == "EXPECTED" { + return true + } + } + return false } diff --git a/internal/pipeline/ci/ci_contract_test.go b/internal/pipeline/ci/ci_contract_test.go new file mode 100644 index 0000000..a91f253 --- /dev/null +++ b/internal/pipeline/ci/ci_contract_test.go @@ -0,0 +1,64 @@ +package ci_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/douglasjarquin/made/internal/github" + "github.com/douglasjarquin/made/internal/github/githubtest" + "github.com/douglasjarquin/made/internal/pipeline/ci" +) + +func TestRun_UsesPrChecksJSONContract(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "gh.log") + bin := githubtest.Build(t) + c := &github.Client{ + Binary: bin, + Dir: t.TempDir(), + ExtraEnv: append(os.Environ(), "FAKE_GH_LOG_FILE="+logPath), + } + + _, _ = ci.Run(context.Background(), c, "https://github.com/example/repo/pull/7", 0, 0) + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read invocation log: %v", err) + } + if !strings.Contains(string(data), "pr checks") { + t.Fatalf("expected gh pr checks invocation, got %s", data) + } + if !strings.Contains(string(data), "name,state,bucket,link") { + t.Fatalf("expected exact checks JSON fields, got %s", data) + } +} + +func TestRun_PassesWorkflowRunIDToLogsAndRerun(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "gh.log") + bin := githubtest.Build(t) + prURL := "https://github.com/example/repo/pull/8" + c := &github.Client{ + Binary: bin, + Dir: t.TempDir(), + ExtraEnv: append(os.Environ(), + "FAKE_GH_LOG_FILE="+logPath, + "FAKE_GH_CHECKS_JSON=[{\"name\":\"build\",\"state\":\"FAILURE\",\"bucket\":\"fail\",\"link\":\"https://github.com/example/repo/actions/runs/12345\"}]", + "FAKE_GH_RUN_LOG=workflow failed\n", + ), + } + + _, _ = ci.Run(context.Background(), c, prURL, 1, 0) + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read invocation log: %v", err) + } + for line := range strings.SplitSeq(string(data), "\n") { + if strings.HasPrefix(line, "invoked: args=run ") && strings.Contains(line, prURL) { + t.Fatalf("PR URL was passed to a workflow-run command: %s", data) + } + } + if !strings.Contains(string(data), "12345") { + t.Fatalf("expected workflow run ID 12345 in run commands, got %s", data) + } +} diff --git a/internal/pipeline/ci/ci_test.go b/internal/pipeline/ci/ci_test.go index 31b7a40..8c7d41f 100644 --- a/internal/pipeline/ci/ci_test.go +++ b/internal/pipeline/ci/ci_test.go @@ -33,7 +33,7 @@ func TestRun_TransientFailureRecoversWithinBudget(t *testing.T) { stateDir := t.TempDir() logPath := filepath.Join(t.TempDir(), "invocations.log") c := newClient(t, []string{ - "FAKE_GH_PR_VIEW_STATES=UNSTABLE,CLEAN", + "FAKE_GH_CHECKS_BUCKETS=fail,pass", "FAKE_GH_STATE_DIR=" + stateDir, }, logPath) @@ -58,9 +58,33 @@ func TestRun_TransientFailureRecoversWithinBudget(t *testing.T) { } } +func TestRun_DoesNotRerunPendingChecks(t *testing.T) { + stateDir := t.TempDir() + logPath := filepath.Join(t.TempDir(), "invocations.log") + c := newClient(t, []string{ + "FAKE_GH_CHECKS_BUCKETS=pending,pass", + "FAKE_GH_STATE_DIR=" + stateDir, + }, logPath) + + result, err := ci.Run(context.Background(), c, "https://github.com/example/repo/pull/11", 2, testPollInterval) + if err != nil { + t.Fatalf("Run: %v", err) + } + if !result.OK || result.RerunsUsed != 0 { + t.Fatalf("pending check was rerun: %+v", result) + } + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read invocation log: %v", err) + } + if strings.Contains(string(data), "run rerun") { + t.Fatalf("pending check triggered a rerun: %s", data) + } +} + func TestRun_BudgetExhaustionSurfacesFinalFailure(t *testing.T) { c := newClient(t, []string{ - "FAKE_GH_PR_VIEW_STATES=UNSTABLE", + "FAKE_GH_CHECKS_BUCKETS=fail", "FAKE_GH_RUN_LOG=build failed at step 3\n", }, "") @@ -101,7 +125,7 @@ func TestRun_RejectsNilClient(t *testing.T) { func TestRun_NeverExceedsBudgetEvenWithAlwaysFailingChecks(t *testing.T) { c := newClient(t, []string{ - "FAKE_GH_PR_VIEW_STATES=UNSTABLE", + "FAKE_GH_CHECKS_BUCKETS=fail", }, "") const rerunBudget = 3 @@ -117,7 +141,7 @@ func TestRun_NeverExceedsBudgetEvenWithAlwaysFailingChecks(t *testing.T) { if result.RerunsUsed != rerunBudget { t.Fatalf("expected exactly rerunBudget reruns (%d), got %d - budget was not respected", rerunBudget, result.RerunsUsed) } - if elapsed > 5*time.Second { + if elapsed > 30*time.Second { t.Fatalf("Run took too long (%s) - suspect it looped past the budget", elapsed) } } diff --git a/internal/pipeline/review/remediation_contract_test.go b/internal/pipeline/review/remediation_contract_test.go index d7d2c0d..6d94450 100644 --- a/internal/pipeline/review/remediation_contract_test.go +++ b/internal/pipeline/review/remediation_contract_test.go @@ -11,7 +11,7 @@ import ( "github.com/douglasjarquin/made/internal/pipeline/review" ) -func TestRun_AutoFixRequiresCleanStateBeforeApplyingReturnedPatch(t *testing.T) { +func TestRun_AutoFixPreservesUnrelatedChanges(t *testing.T) { bin := agenttest.Build(t) f := setupFixture(t) wt := f.addWorktree(t) @@ -26,11 +26,21 @@ func TestRun_AutoFixRequiresCleanStateBeforeApplyingReturnedPatch(t *testing.T) {Kind: agent.FindingAutoFixable, Description: "clean-state fix", Patch: patch, Paths: []string{"reviewed.txt"}}, }}) - if _, err := review.Run(t.Context(), wt.Path, agent.KindCodex, review.Options{ + result, err := review.Run(t.Context(), wt.Path, agent.KindCodex, review.Options{ BinaryPath: bin, ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, - }); err == nil { - t.Fatal("review auto-fix mutated a dirty worktree instead of refusing before apply") + }) + if err != nil { + t.Fatalf("review auto-fix: %v", err) + } + if len(result.AutoFixed) != 1 { + t.Fatalf("expected one auto-fix commit, got %+v", result) + } + if status := run(t, wt.Path, "status", "--porcelain"); !strings.Contains(status, "unrelated.txt") { + t.Fatalf("review auto-fix lost unrelated work: %q", status) + } + if _, err := os.Stat(dirtyPath); err != nil { + t.Fatalf("unrelated work disappeared: %v", err) } } diff --git a/internal/pipeline/review/review.go b/internal/pipeline/review/review.go index c6c4a14..e791ba1 100644 --- a/internal/pipeline/review/review.go +++ b/internal/pipeline/review/review.go @@ -9,11 +9,13 @@ package review import ( "context" "fmt" + "os" "path/filepath" "strings" "time" "github.com/douglasjarquin/made/internal/agent" + madeexec "github.com/douglasjarquin/made/internal/exec" ) type Options struct { @@ -37,7 +39,8 @@ type Result struct { // etc); ask-user and blocking findings are normal outcomes reported via // Result, not errors. func Run(ctx context.Context, worktreePath string, agentKind agent.Kind, opts Options) (Result, error) { - if err := requireCleanWorktree(ctx, worktreePath); err != nil { + beforeStatus, err := gitOutput(ctx, worktreePath, "status", "--porcelain", "--untracked-files=all") + if err != nil { return Result{}, fmt.Errorf("review: inspect worktree before agent: %w", err) } findings, err := agent.Spawn(ctx, agentKind, agent.SpawnParams{ @@ -49,8 +52,12 @@ func Run(ctx context.Context, worktreePath string, agentKind agent.Kind, opts Op if err != nil { return Result{}, fmt.Errorf("review: spawn %s: %w", agentKind, err) } - if err := requireCleanWorktree(ctx, worktreePath); err != nil { - return Result{}, fmt.Errorf("review: agent modified worktree: %w", err) + afterStatus, err := gitOutput(ctx, worktreePath, "status", "--porcelain", "--untracked-files=all") + if err != nil { + return Result{}, fmt.Errorf("review: inspect worktree after agent: %w", err) + } + if beforeStatus != afterStatus { + return Result{}, fmt.Errorf("review: agent modified worktree") } var autoFixed []string @@ -104,9 +111,6 @@ func applyAutoFix(ctx context.Context, worktreePath string, finding agent.Findin if strings.TrimSpace(finding.Patch) == "" { return "", "", fmt.Errorf("auto-fixable finding has no patch") } - if err := requireCleanWorktree(ctx, worktreePath); err != nil { - return "", "", err - } preSHA, err := gitOutput(ctx, worktreePath, "rev-parse", "HEAD") if err != nil { return "", "", fmt.Errorf("record pre-fix SHA: %w", err) @@ -135,41 +139,56 @@ func applyAutoFix(ctx context.Context, worktreePath string, finding agent.Findin } } - if _, err := runGit(ctx, worktreePath, []string{"apply", "--whitespace=fix", "-"}, []byte(finding.Patch)); err != nil { + indexDir, err := os.MkdirTemp("", "made-review-index-") + if err != nil { + return "", "", fmt.Errorf("create isolated index: %w", err) + } + defer func() { _ = os.RemoveAll(indexDir) }() + indexPath := filepath.Join(indexDir, "index") + if _, err := runGitWithIndex(ctx, worktreePath, indexPath, nil, "read-tree", "HEAD"); err != nil { + return "", "", fmt.Errorf("seed isolated index: %w", err) + } + if _, err := runGitWithIndex(ctx, worktreePath, indexPath, nil, "update-index", "--refresh"); err != nil { + return "", "", fmt.Errorf("refresh isolated index: %w", err) + } + if _, err := runGitWithIndex(ctx, worktreePath, indexPath, []byte(finding.Patch), "apply", "--index", "--whitespace=fix", "-"); err != nil { return "", "", fmt.Errorf("git apply: %w", err) } - status, err := gitOutput(ctx, worktreePath, "status", "--porcelain", "--untracked-files=all") + filesOut, err := runGitWithIndex(ctx, worktreePath, indexPath, nil, "diff", "--cached", "--name-only", "--diff-filter=ACMRTUXB") if err != nil { - return "", "", fmt.Errorf("inspect post-fix paths: %w", err) + return "", "", fmt.Errorf("git diff staged files: %w", err) + } + changed := strings.Fields(strings.TrimSpace(string(filesOut.Stdout))) + if len(changed) == 0 { + return "", "", fmt.Errorf("git apply produced no staged files") } - changed := statusPaths(status) for _, path := range changed { - if _, ok := allowed[path]; !ok { + clean := filepath.ToSlash(filepath.Clean(path)) + if _, ok := allowed[clean]; !ok { return "", "", fmt.Errorf("auto-fix changed forbidden or unreturned path %q", path) } } - addArgs := []string{"-C", worktreePath, "add", "--"} - for path := range allowed { - addArgs = append(addArgs, path) - } - if _, err := runGit(ctx, worktreePath, addArgs[2:], nil); err != nil { - return "", "", fmt.Errorf("git add returned paths: %w", err) - } message := finding.Description if message == "" { message = "made review: auto-fix" } - if _, err := runGit(ctx, worktreePath, []string{ + commitArgs := []string{ "-c", "user.name=made-review", "-c", "user.email=made-review@local", "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null", "commit", "-m", message, - }, nil); err != nil { + } + if _, err := runGitWithIndex(ctx, worktreePath, indexPath, nil, commitArgs...); err != nil { return "", "", fmt.Errorf("git commit: %w", err) } + for _, path := range changed { + if _, err := runGit(ctx, worktreePath, []string{"reset", "HEAD", "--", path}, nil); err != nil { + return "", "", fmt.Errorf("restore worktree index for %q: %w", path, err) + } + } shaOut, err := gitOutput(ctx, worktreePath, "rev-parse", "HEAD") if err != nil { @@ -181,15 +200,35 @@ func applyAutoFix(ctx context.Context, worktreePath string, finding agent.Findin return preSHA, shaOut, nil } -func requireCleanWorktree(ctx context.Context, worktreePath string) error { - status, err := gitOutput(ctx, worktreePath, "status", "--porcelain", "--untracked-files=all") +func runGitWithIndex(ctx context.Context, worktreePath, indexPath string, stdin []byte, args ...string) (*madeexec.Result, error) { + filterArgs, err := repositoryFilterOverrides(ctx, worktreePath) + if err != nil { + return nil, err + } + commandArgs := []string{ + "-C", worktreePath, + "-c", "core.hooksPath=/dev/null", + "-c", "core.fsmonitor=false", + "-c", "diff.external=", + } + commandArgs = append(commandArgs, filterArgs...) + commandArgs = append(commandArgs, args...) + env := append(controlledGitEnvironment(), "GIT_INDEX_FILE="+indexPath) + result, err := madeexec.Run(ctx, madeexec.Command{ + Name: "git", + Args: commandArgs, + Env: env, + Stdin: stdin, + Timeout: reviewGitTimeout, + OutputLimit: reviewGitLimit, + }) if err != nil { - return fmt.Errorf("inspect clean worktree: %w", err) + return nil, err } - if strings.TrimSpace(status) != "" { - return fmt.Errorf("auto-fix requires a clean worktree") + if result.ExitCode != 0 { + return result, fmt.Errorf("git exited %d: %s", result.ExitCode, strings.TrimSpace(string(result.Stderr))) } - return nil + return result, nil } func patchPaths(patch string) ([]string, error) { @@ -251,19 +290,3 @@ func cleanReturnedPath(path string) (string, error) { } return filepath.ToSlash(clean), nil } - -func statusPaths(status string) []string { - var paths []string - for _, line := range strings.Split(status, "\n") { - line = strings.TrimSpace(line) - if len(line) < 4 { - continue - } - path := strings.TrimSpace(line[2:]) - if strings.Contains(path, " -> ") { - path = strings.TrimSpace(strings.SplitN(path, " -> ", 2)[1]) - } - paths = append(paths, filepath.ToSlash(path)) - } - return paths -} diff --git a/internal/pipeline/review/review_contract_test.go b/internal/pipeline/review/review_contract_test.go new file mode 100644 index 0000000..bc121f8 --- /dev/null +++ b/internal/pipeline/review/review_contract_test.go @@ -0,0 +1,57 @@ +package review_test + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/douglasjarquin/made/internal/agent" + "github.com/douglasjarquin/made/internal/agent/agenttest" + "github.com/douglasjarquin/made/internal/pipeline/review" +) + +func TestRun_AutoFixDoesNotStageUnrelatedChanges(t *testing.T) { + bin := agenttest.Build(t) + f := setupFixture(t) + wt := f.addWorktree(t) + defer func() { + if err := wt.Remove(); err != nil { + t.Errorf("Remove: %v", err) + } + }() + + writeFile(t, wt.Path, "unrelated.txt", "must not be committed\n") + run(t, wt.Path, "add", "unrelated.txt") + patch := autoFixPatch(t, wt.Path) + scenarioPath := writeScenario(t, agent.Findings{Findings: []agent.Finding{{ + Kind: agent.FindingAutoFixable, Description: "contained fix", Patch: patch, Paths: []string{"reviewed.txt"}, + }}}) + + result, err := review.Run(context.Background(), wt.Path, agent.KindCodex, review.Options{ + BinaryPath: bin, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + }, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if len(result.AutoFixed) != 1 { + t.Fatalf("expected one auto-fix commit, got %+v", result) + } + + files := run(t, wt.Path, "show", "--format=", "--name-only", result.AutoFixed[0]) + if strings.Contains(files, "unrelated.txt") { + t.Fatalf("unrelated file was included in auto-fix commit: %s", files) + } + staged := run(t, wt.Path, "diff", "--cached", "--name-only") + if !strings.Contains(staged, "unrelated.txt") { + t.Fatalf("pre-staged unrelated file was lost from the worktree index: %s", staged) + } + if _, err := os.Stat(filepath.Join(wt.Path, "unrelated.txt")); err != nil { + t.Fatalf("unrelated fixture disappeared: %v", err) + } +} diff --git a/internal/pipeline/review/review_test.go b/internal/pipeline/review/review_test.go index 0421dfa..a10cc87 100644 --- a/internal/pipeline/review/review_test.go +++ b/internal/pipeline/review/review_test.go @@ -29,9 +29,12 @@ func TestRun_AutoFixApplied(t *testing.T) { }, }) - result, err := review.Run(context.Background(), wt.Path, agent.KindClaude, review.Options{ + result, err := review.Run(context.Background(), wt.Path, agent.KindCodex, review.Options{ BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + }, }) if err != nil { t.Fatalf("Run: %v", err) @@ -78,9 +81,12 @@ func TestRun_AskUserFindingQueued(t *testing.T) { }, }) - result, err := review.Run(context.Background(), wt.Path, agent.KindClaude, review.Options{ + result, err := review.Run(context.Background(), wt.Path, agent.KindCodex, review.Options{ BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + }, }) if err != nil { t.Fatalf("Run: %v", err) @@ -125,7 +131,10 @@ func TestRun_BlockingFindingHaltsStage(t *testing.T) { result, err := review.Run(context.Background(), wt.Path, agent.KindCodex, review.Options{ BinaryPath: bin, - ExtraEnv: []string{"FAKE_AGENT_SCENARIO=" + scenarioPath}, + ExtraEnv: []string{ + "FAKE_AGENT_KIND=codex", + "FAKE_AGENT_SCENARIO=" + scenarioPath, + }, }) if err != nil { t.Fatalf("Run: %v", err) diff --git a/plans/made-rewrite.md b/plans/made-rewrite.md index 2d10f32..5d5631f 100644 --- a/plans/made-rewrite.md +++ b/plans/made-rewrite.md @@ -1374,3 +1374,125 @@ Each implementation task (1-35) commits independently once its own acceptance cr - The trusted-vs-pushed config boundary is enforced exactly as specified in the Metis Review section, with a failing-then-passing test proving each of the four rules. - No merge-authority violation is possible in code: made's PR stage has no code path that calls a merge API. +## Made remediation continuation from exact base `3e19ed9d598a68149da5a73949533e8095ca4403` + +This linked section is the canonical ledger for the continuation work. +Historical task claims above remain unchanged. + +### Phase 4A - contract and durability gates + +- [x] Public structured contract: exact `capabilities --json`, `run.submit`, `run.status`, `run.list`, `run.cancel`, `review.decide`, and `doctor --json` surfaces are implemented with exact run IDs and no global-latest status fallback. + + **References**: `cmd/made/capabilities.go`, `cmd/made/run.go`, `cmd/made/run_handlers.go`, `cmd/made/status.go`, `cmd/made/doctor.go`, and `evidence/phase-1-red-made-remediation-continuation.md`. + + **Acceptance Criteria**: The obsolete `made status` command rejects with exit code 2; exact-ID status returns structured lifecycle state; missing IDs fail closed; capabilities lists the supported commands. + + **QA Scenarios**: Run the real binary against a disposable Made home, submit a disposable identity, query the exact run ID, query an unknown ID, and invoke the obsolete status command. + + **Evidence**: `evidence/phase-4-manual-qa.md`. + +- [x] Lifecycle and durability: queued identity, submission refresh, exact input/output SHA and submission metadata, queued cancellation, awaiting-merge, succeeded/canceled/superseded terminal states, first-wins decisions, restart recovery, torn-tail tolerance, durable ordering, and bounded WAL retention are implemented. + + **References**: `internal/daemon/runmanager.go`, `internal/daemon/persistence.go`, `internal/daemon/runstate.go`, `internal/daemon/reviewdecisions.go`, and `evidence/phase-3-lifecycle-durability.md`. + + **Acceptance Criteria**: A submitted record is durable before queue drain; a daemon restart restores the exact record without replaying unrelated work; awaiting-merge remains non-terminal until succeeded; a queued cancel performs no work; torn final records are ignored and non-final corruption fails closed. + + **QA Scenarios**: Run the daemon tests for queued cancellation, awaiting-merge, restart, torn-tail, retention, decision conflict, and the real binary restart scenario. + + **Evidence**: `internal/daemon/persistence_contract_test.go`, `evidence/phase-3-lifecycle-durability.md`, and `evidence/phase-4-manual-qa.md`. + +- [x] Evidence and reviewer containment: atomic in-repository evidence writes, compare-and-swap orphan publication, path containment, stage evidence references, and patch-only auto-fix commits are enforced. + + **References**: `internal/evidence/inrepo.go`, `internal/evidence/orphan.go`, `internal/orchestrator/workfunc.go`, `internal/pipeline/review/review.go`, and `evidence/phase-3-lifecycle-durability.md`. + + **Acceptance Criteria**: Concurrent orphan writers retain every run; evidence files use durable write ordering; an auto-fix never stages unrelated worktree files; stage and run snapshots preserve evidence references. + + **QA Scenarios**: Run the evidence concurrency suite and reviewer containment scenario with an unrelated disposable file present. + + **Evidence**: `internal/evidence/evidence_contract_test.go`, `internal/pipeline/review/review_contract_test.go`, and `evidence/phase-3-lifecycle-durability.md`. + +- [x] Semantic configuration and enforced switches: unknown or multiple YAML documents fail closed, trusted configuration remains authoritative, and the trusted `no_ci` switch is enforced by the orchestrator. + + **References**: `internal/config/config.go`, `internal/config/config_contract_test.go`, `internal/orchestrator/workfunc.go`, and `evidence/phase-3-lifecycle-durability.md`. + + **Acceptance Criteria**: Unknown semantic switches are rejected; pushed configuration cannot override trusted execution settings without the existing explicit trust switch; `no_ci` records a skipped CI stage instead of invoking CI. + + **QA Scenarios**: Load disposable trusted/pushed YAML fixtures with unknown fields and run a trusted `no_ci` stage fixture. + + **Evidence**: `internal/config/config_contract_test.go` and `evidence/phase-3-lifecycle-durability.md`. + +### Phase 4B - compatibility and final validation gates + +- [x] Strict external compatibility: GitHub uses `gh pr checks --json name,state,bucket,link`, preserves numeric workflow run IDs, exposes authentication/check/log/rerun errors, and Codex uses the structured `exec` task contract while unsupported Claude behavior is rejected explicitly. + + **References**: `internal/github/client.go`, `internal/github/testdata/fakegh/main.go`, `internal/agent/spawn.go`, `internal/agent/testdata/fakeagent/main.go`, and `evidence/phase-2-external-contracts.md`. + + **Acceptance Criteria**: Strict fakes reject obsolete or invented arguments; focused GREEN tests accept only supported GitHub JSON and Codex structured output; PR URLs cannot reach workflow run operations. + + **QA Scenarios**: Run the focused GitHub/CI and agent/review suites against disposable repositories, strict fake boundaries, and process fixtures. + + **Evidence**: `evidence/phase-1-red-made-remediation-continuation.md` and `evidence/phase-2-external-contracts.md`. + +- [x] Disposable live scenarios: the real Made binary was exercised only against a disposable Made home, exact run identities, a restart, strict boundary behavior, and the named non-default Herdr lab session. + + **References**: `evidence/phase-0-grounding-made-remediation-continuation.md`, `evidence/phase-4-manual-qa.md`, and the required Herdr helper path in the task brief. + + **Acceptance Criteria**: The live scenario does not initialize a real gate, submit a real project, alter the shared daemon, or use the default Herdr session. + + **QA Scenarios**: Start and stop only the disposable Made daemon, query exact IDs, restart it, and probe the named Herdr session through the helper. + + **Evidence**: `evidence/phase-4-manual-qa.md`. + +- [x] Final validation and delivery: run the Made-only build, race/shuffle test, vet, configured lint, changed-file diagnostics, final branch scope review, review-work/runtime audit, direct branch push, and direct PR creation. + + **References**: `evidence/phase-1-red-made-remediation-continuation.md`, `evidence/phase-2-external-contracts.md`, `evidence/phase-3-lifecycle-durability.md`, `evidence/phase-4-manual-qa.md`, `evidence/phase-4-final-validation.md`, `evidence/phase-4-review-audit.md`, and `evidence/phase-4-herdr-cleanup.md`. + + **Acceptance Criteria**: The final commit list starts at the exact base SHA `3e19ed9d598a68149da5a73949533e8095ca4403`; only Made files and linked evidence/plan records are changed; all authorized local validation is green; PR [#2](https://github.com/douglasjarquin/made/pull/2) is open on `cs/made-remediation-continuation`; no default branch push or merge occurs. + + **QA Scenarios**: Execute the final Made-only validation commands, inspect the exact full SHA and changed-file list, perform required review audits, push only the direct branch, and open the direct PR with `gh-axi`. + + **Evidence**: `evidence/phase-4-final-validation.md`, `evidence/phase-4-review-audit.md`, `evidence/phase-4-herdr-cleanup.md`, the final commit list, the branch push receipt, and PR [#2](https://github.com/douglasjarquin/made/pull/2). + +**Commit**: YES | Message: `fix(made): complete remediation continuation from exact base` | Files: Made source, Made tests, `plans/made-rewrite.md`, and phase-scoped evidence only. + +### Conflict repair continuation receipt - exact final source `918da271aa9521d292bbda22a862591b770f9af6` + +- [x] Conflict repair preserved the exact base `3e19ed9d598a68149da5a73949533e8095ca4403`, retained `origin/main` as merge parent `34d44be504291482d973c65bd427ba964df5e0e9`, and removed only obsolete duplicate CLI paths. + + **References**: `evidence/phase-4-conflict-repair.md`, merge commit `0a7c21d6d3001b85b38330766e01980bd5e92f2c`, and final source commits `bac8ed2777f584d98eb1ba8015cf1269d01a8c1e` and `918da271aa9521d292bbda22a862591b770f9af6`. + + **Acceptance Criteria**: `git diff --name-only 3e19ed9d598a68149da5a73949533e8095ca4403..HEAD` remains Made-only; no unmerged paths remain; the merge parents are exact. + + **QA Scenario**: Run `git merge-base HEAD 3e19ed9d598a68149da5a73949533e8095ca4403`, `git rev-parse HEAD^1`, `git rev-parse HEAD^2`, and `git diff --name-only ...`. + + **Evidence**: `evidence/phase-4-conflict-repair.md`. + +- [x] Durability review correction preserves the compaction-triggering transition across restart by overlaying the candidate snapshot before WAL truncation. + + **References**: `internal/daemon/persistence.go`, `internal/daemon/persistence_contract_test.go`, and `evidence/phase-4-conflict-repair.md`. + + **Acceptance Criteria**: The new compaction regression is RED against the pre-fix code and GREEN at `918da271aa9521d292bbda22a862591b770f9af6`; the full daemon package remains green. + + **QA Scenario**: Run `go test ./internal/daemon -run '^TestRunManager_CompactionPersistsTriggeringTransition$' -count=1` and the affected package suite with process-local Git configuration. + + **Evidence**: `evidence/phase-4-conflict-repair.md` and `evidence/phase-4-final-validation.md`. + +- [x] Final exact-SHA local validation and disposable real-binary QA were rerun after the durability correction. + + **References**: `evidence/phase-4-final-validation.md` and `evidence/phase-4-manual-qa.md`. + + **Acceptance Criteria**: `go test ./... -count=1`, `go test -race -shuffle=on -count=1 ./...`, `go build ./...`, `go vet ./...`, configured `make lint`, and `gofmt` checks exit `0`; the disposable binary observes exact structured boundaries and cleans its local daemon/socket/lock. + + **QA Scenario**: Build the binary from the exact final source, run capabilities, obsolete status, disposable daemon status/list/unknown-ID/stop, then verify cleanup. + + **Evidence**: `evidence/phase-4-manual-qa.md` and `evidence/phase-4-final-validation.md`. + +- [x] Fresh review-work/runtime-audit receipts are bound to the final source SHA, and PR #2 is pushed and verified conflict-free. + + **Acceptance Criteria**: All applicable review lanes have terminal verdicts bound to source SHA `12b83a6649b5e198049754f1cb6427d7b0dc51a0`; only `cs/made-remediation-continuation` is pushed; PR #2 head matches the final source branch and reports clean mergeability. + + **QA Scenario**: Run the review audit, inspect the exact commit list and changed-file scope, push only the task branch with `gh-axi`, and read PR #2 metadata without merging. + + **Evidence**: `evidence/phase-4-review-audit.md`, `evidence/phase-4-conflict-repair.md`, the successful hosted check `95537594230`, and the final PR read receipt. + + **Commit**: YES | Message: `fix(made): preserve compaction transition during conflict repair` | Files: `internal/daemon/persistence.go`, `internal/daemon/persistence_contract_test.go`, phase-scoped evidence, and this continuation receipt.