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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 43 additions & 90 deletions README.md

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,12 @@ var globalFlagSpecs = []globalFlagSpec{
{"mode", "Workflow", "Execution profile: fast or best.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "mode", &o.Mode) }},
{"max-cycles", "Workflow", "Maximum fix-findings attempts per review phase.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "max-cycles", &o.MaxCycles) }},
{"max-ci-recoveries", "Workflow", "Maximum CI recovery attempts.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "max-ci-recoveries", &o.MaxCIRecoveries) }},
{"ci-timeout", "Workflow", "Maximum time to wait for applicable CI (default 60m).", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "ci-timeout", &o.CITimeout) }},
{"review-model", "Stage overrides", "Review model.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "review-model", &o.ReviewModel) }},
{"review-reasoning-effort", "Stage overrides", "Review reasoning effort.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "review-reasoning-effort", &o.ReviewEffort) }},
{"fix-model", "Stage overrides", "Fix-findings model.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "fix-model", &o.FixModel) }},
{"fix-reasoning-effort", "Stage overrides", "Fix-findings reasoning effort.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "fix-reasoning-effort", &o.FixEffort) }},
{"fix-prompt-file", "Stage overrides", "Fix-findings prompt file.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "fix-prompt-file", &o.FixPromptPath) }},
{"finalize-model", "Stage overrides", "Finalization model.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "finalize-model", &o.FinalizeModel) }},
{"finalize-reasoning-effort", "Stage overrides", "Finalization reasoning effort.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "finalize-reasoning-effort", &o.FinalizeEffort) }},
{"finalize-prompt-file", "Stage overrides", "Finalization prompt file.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "finalize-prompt-file", &o.FinalizePromptPath) }},
{"ci-fix-model", "Stage overrides", "CI-fix model.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "ci-fix-model", &o.CIFixModel) }},
{"ci-fix-reasoning-effort", "Stage overrides", "CI-fix reasoning effort.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "ci-fix-reasoning-effort", &o.CIFixEffort) }},
{"ci-fix-prompt-file", "Stage overrides", "CI-fix prompt file.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "ci-fix-prompt-file", &o.CIFixPromptPath) }},
Expand Down
88 changes: 55 additions & 33 deletions internal/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestConfigCommand(t *testing.T) {
"mode: best (cli; built-in: fast)",
"max-cycles: 4 (cli; built-in: 10)",
"review-model: gpt-5.6-sol (best profile; built-in: gpt-5.6-terra)",
"finalize-reasoning-effort: medium (best profile)",
"ci-timeout: 60m (built-in default)",
"ci-fix-reasoning-effort: high (best profile; built-in: medium)",
} {
if !strings.Contains(stdout.String(), want) {
Expand Down Expand Up @@ -245,21 +245,49 @@ type appFakeRunner struct {
reviewMsg string
skipReviewMsg bool
status runner.Result
statusResults []runner.Result
statusCalls int
statusErr error
finalizeMsg string
err error
}

func codexInvocationsForApp(invocations []runner.Invocation) []runner.Invocation {
var result []runner.Invocation
for _, invocation := range invocations {
if invocation.Executable == "" {
result = append(result, invocation)
}
}
return result
}

func (f *appFakeRunner) Run(_ context.Context, invocation runner.Invocation) (runner.Result, error) {
f.invocations = append(f.invocations, invocation)
if invocation.Executable == "gh" {
if strings.HasPrefix(strings.Join(invocation.Args, " "), "pr create ") {
return runner.Result{Stdout: "https://github.com/dapi/code-converge/pull/40\n"}, nil
}
if strings.HasPrefix(strings.Join(invocation.Args, " "), "api ") {
return runner.Result{Stdout: `{"check_runs":[]}`}, nil
}
return runner.Result{Stdout: "[]"}, nil
}
if invocation.Executable == "git" {
args := strings.Join(invocation.Args, " ")
switch {
case strings.HasPrefix(args, "status "):
if f.statusCalls < len(f.statusResults) {
result := f.statusResults[f.statusCalls]
f.statusCalls++
return result, f.statusErr
}
return f.status, f.statusErr
case args == "add -A", args == "commit -m chore: publish reviewed changes", strings.HasPrefix(args, "push "):
return runner.Result{}, nil
case args == "branch --show-current":
return runner.Result{Stdout: "feature\n"}, nil
case args == "rev-parse HEAD":
return runner.Result{Stdout: "published-sha\n"}, nil
case args == "symbolic-ref --quiet --short HEAD":
return runner.Result{Stdout: "feature"}, nil
case args == "config --get branch.feature.pushRemote", args == "config --get remote.pushDefault":
Expand All @@ -268,6 +296,8 @@ func (f *appFakeRunner) Run(_ context.Context, invocation runner.Invocation) (ru
return runner.Result{Stdout: "origin"}, nil
case args == "remote get-url --push --all origin":
return runner.Result{Stdout: "git@github.com:dapi/code-converge.git"}, nil
case args == "remote get-url --push --all origin":
return runner.Result{Stdout: "git@github.com:dapi/code-converge.git"}, nil
case args == "remote get-url --all origin":
return runner.Result{Stdout: "git@github.com:dapi/code-converge.git"}, nil
case args == "config --get branch.feature.gh-merge-base":
Expand All @@ -290,13 +320,10 @@ func (f *appFakeRunner) Run(_ context.Context, invocation runner.Invocation) (ru
isReview := strings.Contains(invocation.Stdin, "prepared private Git index")
for i, arg := range invocation.Args {
if arg == "--output-last-message" && i+1 < len(invocation.Args) && f.err == nil {
message := f.finalizeMsg
if isReview {
if f.skipReviewMsg {
continue
}
message = f.reviewMsg
if !isReview || f.skipReviewMsg {
continue
}
message := f.reviewMsg
if err := os.WriteFile(invocation.Args[i+1], []byte(message), 0o600); err != nil {
f.t.Fatalf("write output message: %v", err)
}
Expand All @@ -311,10 +338,9 @@ func (f *appFakeRunner) Run(_ context.Context, invocation runner.Invocation) (ru
func TestNilStreamsAndCwdDoNotPanic(t *testing.T) {
root, home := testRepo(t)
fake := &appFakeRunner{
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
finalizeMsg: `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`,
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
}
code := (App{Cwd: root, Home: home, Runner: fake}).Run(context.Background(), nil)
if code != workflow.ExitSuccess {
Expand Down Expand Up @@ -348,11 +374,10 @@ func TestAppWorkflowSuccessWithFakeRunner(t *testing.T) {
root, home := testRepo(t)
var stdout, stderr bytes.Buffer
fake := &appFakeRunner{
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
status: runner.Result{Stdout: " M changed.go\n"},
finalizeMsg: `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`,
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
statusResults: []runner.Result{{}, {Stdout: " M changed.go\n"}, {Stdout: " M changed.go\n"}},
}
code := (App{Stdout: &stdout, Stderr: &stderr, Cwd: root, Home: home, Runner: fake}).Run(context.Background(), []string{"--log-format=kv"})
if code != workflow.ExitSuccess || stderr.Len() != 0 {
Expand All @@ -361,8 +386,8 @@ func TestAppWorkflowSuccessWithFakeRunner(t *testing.T) {
if !strings.Contains(stdout.String(), "event=run_completed status=success exit_code=0") {
t.Fatalf("stdout:\n%s", stdout.String())
}
if len(fake.invocations) < 2 {
t.Fatalf("expected review and finalize invocations, got %d", len(fake.invocations))
if len(codexInvocationsForApp(fake.invocations)) != 1 {
t.Fatalf("expected exactly one Codex review invocation, got %#v", fake.invocations)
}
var review runner.Invocation
for _, invocation := range fake.invocations {
Expand Down Expand Up @@ -396,7 +421,7 @@ func TestAppWorkflowSuccessWithFakeRunner(t *testing.T) {
}
}

func TestAppNoChangeSkipsFinalize(t *testing.T) {
func TestAppNoChangeSkipsPublication(t *testing.T) {
root, home := testRepo(t)
var stdout, stderr bytes.Buffer
fake := &appFakeRunner{
Expand All @@ -408,7 +433,7 @@ func TestAppNoChangeSkipsFinalize(t *testing.T) {
if code != workflow.ExitSuccess || stderr.Len() != 0 {
t.Fatalf("code=%d stderr=%q", code, stderr.String())
}
if !strings.Contains(stdout.String(), "event=review_completed") || !strings.Contains(stdout.String(), "status=clean") || !strings.Contains(stdout.String(), "findings_total=0") || !strings.Contains(stdout.String(), "event=run_completed status=success exit_code=0") || strings.Contains(stdout.String(), "stage=finalize") {
if !strings.Contains(stdout.String(), "event=review_completed") || !strings.Contains(stdout.String(), "status=clean") || !strings.Contains(stdout.String(), "findings_total=0") || !strings.Contains(stdout.String(), "event=run_completed status=success exit_code=0") || strings.Contains(stdout.String(), "stage=publish") {
t.Fatalf("stdout:\n%s", stdout.String())
}
last := fake.invocations[len(fake.invocations)-1]
Expand Down Expand Up @@ -451,10 +476,9 @@ func TestAppHumanNonTTYWorkflow(t *testing.T) {
root, home := testRepo(t)
var stdout, stderr bytes.Buffer
fake := &appFakeRunner{
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
finalizeMsg: `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`,
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
}
code := (App{Stdout: &stdout, Stderr: &stderr, Cwd: root, Home: home, Runner: fake}).Run(context.Background(), nil)
if code != workflow.ExitSuccess || !strings.Contains(stdout.String(), "Done (") || strings.Contains(stdout.String(), "\x1b") || strings.Contains(stdout.String(), "event=") || strings.Contains(stdout.String(), "No findings") {
Expand Down Expand Up @@ -566,10 +590,9 @@ func TestAppHumanDevNullWorkflow(t *testing.T) {
defer device.Close()
var stderr bytes.Buffer
fake := &appFakeRunner{
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
finalizeMsg: `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`,
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"no findings","overall_confidence_score":0.99}`,
}
code := (App{Stdout: device, Stderr: &stderr, Cwd: root, Home: home, Runner: fake}).Run(context.Background(), nil)
if code != workflow.ExitSuccess || stderr.Len() != 0 {
Expand All @@ -581,10 +604,9 @@ func TestAppHumanDumbTerminalUsesPermanentProgress(t *testing.T) {
root, home := testRepo(t)
var stdout, stderr bytes.Buffer
fake := &appFakeRunner{
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: cleanReviewJSONForApp,
finalizeMsg: `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`,
t: t,
review: runner.Result{Stdout: "No findings.\n"},
reviewMsg: cleanReviewJSONForApp,
}
code := (App{
Stdout: &stdout, Stderr: &stderr, Cwd: root, Home: home, Runner: fake,
Expand Down
107 changes: 1 addition & 106 deletions internal/codex/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ type structuredLineRange struct {
End *int `json:"end"`
}

type Finalization struct {
Verdict string `json:"verdict"`
Commit string `json:"commit"`
Push string `json:"push"`
ChangeRequest string `json:"change_request"`
CI string `json:"ci"`
}

type Adapter struct {
Runner runner.Runner
Config config.Config
Expand Down Expand Up @@ -231,33 +223,6 @@ func (a Adapter) FixCI(ctx context.Context) error {
return err
}

func (a Adapter) Finalize(ctx context.Context, checkpointed bool) (Finalization, error) {
dir, err := os.MkdirTemp("", "code-converge-finalize-")
if err != nil {
return Finalization{}, fmt.Errorf("create finalization workspace: %w", err)
}
defer os.RemoveAll(dir)
schemaPath := filepath.Join(dir, "schema.json")
messagePath := filepath.Join(dir, "message.json")
if err := os.WriteFile(schemaPath, []byte(finalizationSchema), 0o600); err != nil {
return Finalization{}, fmt.Errorf("write finalization schema: %w", err)
}
prompt := a.Config.FinalizePrompt
if checkpointed {
prompt += "\n\nSuccessful findings fixes were already committed as local checkpoints. Do not create an empty commit; publish the current branch, create a change request if needed, and verify applicable CI."
}
prompt += "\n\nReturn only the JSON object required by the supplied output schema. Report the actual outcomes of commit, push, change_request, and ci."
args := append(modelArgs(a.Config.FinalizeModel, a.Config.FinalizeEffort), "exec", "--output-schema", schemaPath, "--output-last-message", messagePath, "-")
if _, err := a.Runner.Run(ctx, runner.Invocation{Args: args, Stdin: prompt, Output: a.output()}); err != nil {
return Finalization{}, err
}
message, err := os.ReadFile(messagePath)
if err != nil {
return Finalization{}, fmt.Errorf("read finalization response: %w", err)
}
return ParseFinalization(message)
}

func (a Adapter) output() func(runner.Output) {
if a.Output == nil {
return nil
Expand Down Expand Up @@ -376,32 +341,13 @@ func validateStructuredReview(response structuredReview) error {
return nil
}

func ParseFinalization(data []byte) (Finalization, error) {
if err := rejectDuplicateJSONKeys(data); err != nil {
return Finalization{}, fmt.Errorf("parse finalization response: %w", err)
}
decoder := json.NewDecoder(bytes.NewReader(data))
decoder.DisallowUnknownFields()
var result Finalization
if err := decoder.Decode(&result); err != nil {
return Finalization{}, fmt.Errorf("parse finalization response: %w", err)
}
if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) {
return Finalization{}, errors.New("finalization response contains trailing data")
}
if err := validateFinalization(result); err != nil {
return Finalization{}, err
}
return result, nil
}

func rejectDuplicateJSONKeys(data []byte) error {
decoder := json.NewDecoder(bytes.NewReader(data))
if err := scanJSONValue(decoder); err != nil {
return err
}
if _, err := decoder.Token(); !errors.Is(err, io.EOF) {
return errors.New("finalization response contains trailing data")
return errors.New("structured review response contains trailing data")
}
return nil
}
Expand Down Expand Up @@ -455,44 +401,6 @@ func scanJSONValue(decoder *json.Decoder) error {
return nil
}

func validateFinalization(result Finalization) error {
validStep := func(value string) bool {
return value == "success" || value == "skipped" || value == "failed" || value == "unknown"
}
if !validStep(result.Commit) || !validStep(result.Push) || !validStep(result.ChangeRequest) || !validStep(result.CI) {
return errors.New("finalization response contains an invalid step status")
}
switch result.Verdict {
case "SUCCESS":
if !oneOf(result.Commit, "success", "skipped") || !oneOf(result.Push, "success", "skipped") || !oneOf(result.ChangeRequest, "success", "skipped") || !oneOf(result.CI, "success", "skipped") {
return errors.New("SUCCESS is inconsistent with step outcomes")
}
case "CI_FAILED":
if !oneOf(result.Commit, "success", "skipped") || !oneOf(result.Push, "success", "skipped") || !oneOf(result.ChangeRequest, "success", "skipped") || result.CI != "failed" {
return errors.New("CI_FAILED is inconsistent with step outcomes")
}
case "FAILED":
if oneOf(result.Commit, "success", "skipped") && oneOf(result.Push, "success", "skipped") && oneOf(result.ChangeRequest, "success", "skipped") && oneOf(result.CI, "success", "skipped") {
return errors.New("FAILED is inconsistent with successful step outcomes")
}
if oneOf(result.Commit, "success", "skipped") && oneOf(result.Push, "success", "skipped") && oneOf(result.ChangeRequest, "success", "skipped") && result.CI == "failed" {
return errors.New("FAILED is inconsistent with a CI-only failure")
}
default:
return errors.New("finalization response contains an unknown verdict")
}
return nil
}

func oneOf(value string, choices ...string) bool {
for _, choice := range choices {
if value == choice {
return true
}
}
return false
}

const reviewSchema = `{
"type": "object",
"additionalProperties": false,
Expand Down Expand Up @@ -534,16 +442,3 @@ const reviewSchema = `{
"overall_confidence_score": {"type": "number"}
}
}`

const finalizationSchema = `{
"type": "object",
"additionalProperties": false,
"required": ["verdict", "commit", "push", "change_request", "ci"],
"properties": {
"verdict": {"type": "string", "enum": ["SUCCESS", "CI_FAILED", "FAILED"]},
"commit": {"type": "string", "enum": ["success", "skipped", "failed", "unknown"]},
"push": {"type": "string", "enum": ["success", "skipped", "failed", "unknown"]},
"change_request": {"type": "string", "enum": ["success", "skipped", "failed", "unknown"]},
"ci": {"type": "string", "enum": ["success", "skipped", "failed", "unknown"]}
}
}`
Loading