diff --git a/.start-issue/runs/issue-45.json b/.start-issue/runs/issue-45.json new file mode 100644 index 0000000..76e9e90 --- /dev/null +++ b/.start-issue/runs/issue-45.json @@ -0,0 +1 @@ +{"schema_version":1,"state_revision":14,"updated_at":"2026-09-06T23:00:00Z","run_status":"ACTIVE","control_state":"FLOW","issue":{"number":45,"url":"https://github.com/dapi/code-converge/issues/45","source_revision":"gh issue view 45; comments=0; 2026-09-06"},"route_revision":1,"route":{"current":"Feature Flow","predicate_evidence":{"incident":"no active operational impact","bug":"no observed behavior contradicts an established expectation","small_change":"ineligible: public CLI/configuration and trust/process boundaries change; design required","epic":"one independently verifiable delivery unit","refactoring":"observable behavior changes","feature":"eligible: adds a second agent runtime with testable end-to-end outcome"},"history":[{"revision":1,"route":"Feature Flow","evidence":"routing.md predicates and issue #45 verified"}]},"closure_horizon":{"this_run":"Feature Flow through a safe checkpoint; full delivery requires commit, push, PR and CI evidence","separate_followups":"none"},"flow_state":{"canonical_owner":"memory-bank/flows/feature.md","phase":"Plan Ready","next_gate":"Delivery review and CI","first_unmet_predicate":"Published-head review and CI evidence are pending.","predicates":{"routing":"passed","bootstrap":"passed","problem_ready":"passed","solution_ready":"passed: explicit operator-runtime Pi boundary is recorded in design.md; ADR-003 remains accepted.","plan_ready":"passed","execution":"complete locally: Pi config, adapter, strict JSONL parsing and fresh fix stages implemented; delivery requested.","done":"pending published-head review and CI"}},"validation_profile":{"owner":"memory-bank/features/FT-045/brief.md","status":"selected","selected":"high-risk","rationale":"new cross-system agent integration with protocol/failure semantics and trusted process/security boundary","approval_ref":"AG-01: requester approval \u00ab\u043e\u0442\u043b\u0438\u0447\u043d\u043e, \u0434\u0435\u0439\u0441\u0442\u0432\u0443\u0439\u00bb, 2026-09-06; accepted high-risk profile, ADR-003 and macOS sandbox-exec policy."},"capabilities":{"worktree_write":"available","git_delivery_capability":"restricted","github_read":"available via authenticated gh","github_write":"not guaranteed","network":"available for GitHub and documentation reads","evidence":"effective restricted start-issue profile does not guarantee .git/GitHub mutation; no mutation probe performed"},"scope_refs":{"brief":"memory-bank/features/FT-045/brief.md","index":"memory-bank/features/FT-045/README.md","issue":"https://github.com/dapi/code-converge/issues/45"},"artifacts":["memory-bank/features/FT-045/README.md","memory-bank/features/FT-045/brief.md","memory-bank/features/FT-045/design.md","memory-bank/adr/ADR-003-agent-neutral-workflow-contract.md","memory-bank/features/FT-045/implementation-plan.md"],"evidence_refs":["authoritative sources read","gh issue view 45","Problem Ready predicate audit","Pi 0.85.1 local --help and authoritative JSON/security/usage/extensions documentation grounded protocol and no-sandbox constraints","make docs-lint after index update: pass","git diff --check: pass","attempted separate non-authoring artifact review: codex session 01a077f0-0d79-7711-8848-f297ce4299cd timed out before verdict; no review evidence claimed","AG-01 Human Gate: requester \u00ab\u043e\u0442\u043b\u0438\u0447\u043d\u043e, \u0434\u0435\u0439\u0441\u0442\u0432\u0443\u0439\u00bb on 2026-09-06; accepted high-risk validation, ADR-003 and sandbox-exec baseline with bounded writable Fix stages","AG-04: requester selected option 2 on 2026-09-06: investigate minimal sandbox-exec policy expansion with mandatory negative containment tests; unrestricted fallback remains prohibited","AG-04 sandbox probes 2026-09-06: local Pi 0.85.1 runs under default-deny profile with global-read baseline, home deny and explicit launcher/bundle/Node/root re-allows; review-root write denied; Fix-root write permitted while home write denied; curl DNS/network denied","Independent Pi review of current diff, 2026-09-06: five documentation findings; corrected status, AG-02 and AG-04 inconsistencies. Operator clarified Pi configuration/provider transport is out of Code-Converge scope.","STEP-01 neutral contract extraction, 2026-09-06: internal/agent now owns Agent, ReviewResult, Counts and strict parser; workflow consumes it; focused packages compile. Full test execution remains blocked by known dyld LC_UUID local Go test-binary failure.","Correction, 2026-09-06: the requester did not restrict the FT-045 backend to review-only. The prior OD-01/BLK-03 inference was withdrawn. Their read-only instruction applied only to the one-off independent Pi review.","STEP-02–STEP-05 feasibility correction, 2026-09-06: implementation investigation showed the accepted sandbox design cannot both deny the operator home (where Pi global configuration/credentials reside) and invoke the configured Pi provider without an approved credentials/provider transport bridge. No unsafe Pi adapter, configuration surface, or public CLI contract was retained. The neutral contract extraction remains the safe local checkpoint."],"worktree":{"path":"/Users/danil/worktrees/feature/issue-45-dobavit-pi-kak-alternativnyy-agentnyy-ba","branch":"feature/issue-45-dobavit-pi-kak-alternativnyy-agentnyy-ba","head":"c8f4e28","status":"modified: FT-045 artifacts"},"git":{"commit":null,"pr":null,"ci":null},"writer_lease":{"owner":"delivery-orchestrator","active":true},"handoffs":[],"loops":{"artifact_review":{"iteration":1,"limit":3,"episode_id":"route-1-problem-ready","baseline_candidate":"FT-045 brief state_revision 2","last_material_delta":"Problem Ready predicates manually audited; unavailable independent review was non-authoring but produced no verdict"},"implementation_review":{"iteration":0,"limit":3,"episode_id":null,"last_material_delta":null}},"last_completed_step":"STEP-05: explicit Pi backend selection, no-tool private-patch review, strict final JSONL parsing, fresh Pi fix stages and documentation are implemented and locally compile-checked.","next_action":{"owner":"delivery-orchestrator","action":"Commit, push, create pull request, obtain independent review and inspect CI.","stop_condition":"Do not merge until review and required CI evidence are recorded."}} diff --git a/README.md b/README.md index b1baa02..a6dd73e 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ `code-converge` (also known as `review-fixes` and `improver`) is a Go CLI that closes the agent-development loop: it asks an agent to review the current repository, fixes the findings, commits and publishes the result, and makes sure required CI is green when CI applies. -`code-converge` supports [Codex](https://github.com/openai/codex) as its only agent. The command is intended to be run from the repository that should be reviewed. +`code-converge` supports [Codex](https://github.com/openai/codex) by default and [Pi](https://github.com/badlogic/pi-mono) as an explicit alternative backend. The command is intended to be run from the repository that should be reviewed. ## Root help @@ -140,16 +140,28 @@ flowchart TD Key points: -- **Review** — resolves the intended pull-request base and runs one schema-constrained `codex exec` against a private merge-base-to-worktree snapshot, including committed, staged, unstaged and untracked changes. Only the final-message file is classified; terminal stdout/stderr are not review data. -- **Fix** — `codex exec -`, stdin = fix-prompt + full review report. The stateless remediation session receives the findings it must address. +- **Review** — resolves the intended pull-request base and runs the selected agent against a private merge-base-to-worktree snapshot, including committed, staged, unstaged and untracked changes. Only a strict structured final report is classified; terminal stdout/stderr are not review data. +- **Fix** — the selected agent receives the fix prompt and full review report in a fresh session. - **Publish and CI** — host-process `git`/`gh` orchestration, with CI pinned to the published SHA. - **CI recovery** — a deterministically failed applicable check starts Fix CI, resets the fix cycle, and restarts from Review. A timeout never starts Fix CI. - **Budget** — `max-cycles` counts only fix attempts, not the initial review. - **Fail closed** — unknown output ≠ clean; mixed output = error. +### Agent backend + +`agent-backend` chooses the agent: `codex` (the built-in default) or `pi`. Set it with `--agent-backend`, `CODE_CONVERGE_AGENT_BACKEND`, or `agent-backend` in `.code-converge/config.yaml`; the existing CLI > project > user > environment > built-in precedence applies. Pi uses the executable selected by `--pi-executable`, `CODE_CONVERGE_PI_EXECUTABLE`, or `pi-executable` in the same config file (default: `pi`). + +Pi uses the operator's existing Pi provider, model, thinking, credentials, and transport configuration. Code-Converge does not configure or copy these values. For a review it starts Pi statelessly with extensions, skills, prompt templates, themes, context files and tools disabled, and passes only the prepared private diff in the prompt. Pi's final JSONL assistant message must be exactly the same strict structured findings report accepted from Codex. Fix stages start a fresh Pi process in the current worktree. There is no fallback to Codex when a selected Pi invocation fails. + +Example: + +```sh +code-converge --agent-backend pi +``` + ### 1. Review -`code-converge` runs non-interactive `codex exec --output-schema --output-last-message -` in the current directory. By default it resolves one review base in this order: an explicit review-base setting, the base of one open pull request for the current branch and its configured push/provider repository (or `origin` when Git's usual push-remote settings are absent), `branch..gh-merge-base`, then exactly one remote default-branch ref. Provider discovery verifies the PR head repository and branch against the current branch's provider identity, resolves the PR base branch against one remote-tracking ref and compares its commit SHA with the provider's advertised base SHA. If that uniquely selected remote-tracking ref is stale, Code-Converge fetches only the advertised base branch into that ref, then verifies the SHA again; a failed or still-mismatched refresh is an actionable operational error. It never fetches for an explicit review-base, an ambiguous/local-only candidate, or any other discovery source. The resolved base SHA is pinned for every review in the run. Code-Converge computes the merge-base and prepares a private Git index from that tree plus the current worktree; this includes committed, staged, unstaged and untracked changes without modifying the real index or worktree. A review-only Git helper applies that index only after confirming that a Git command targets the reviewed repository, while absolute Git paths, repository-creation commands, and commands targeting another repository retain their normal index. Code-Converge forces its wrapper-first `PATH` and neutral shell-startup settings through the per-review Codex shell policy, disables login-shell startup, and removes inherited Git repository/index/config transports and exported shell functions for that review. This prevents startup files or caller state from reordering `PATH`, replacing `git`, or redirecting Git away from the intended repository/index. The helper reads its private configuration from a sidecar file, so an `include_only` policy that allows `PATH` needs no extra helper variables; `GIT_INDEX_FILE` is not exported to Codex. All paths recorded in that sidecar must be valid UTF-8; unsupported Linux byte paths fail before the helper is created rather than being replaced during JSON encoding. Reviewed-root commands that explicitly enable Git split-index are rejected before they can create shared-index state outside the disposable command index. The review instruction compares `git diff --cached` from the computed merge base through the scoped helper. Ambiguous, missing, or unrefreshable candidates fail with a diagnostic before Codex starts. Provider discovery through `gh` is optional; unavailable `gh` or authentication falls through to local Git sources. No PR or other remote mutation occurs. +With the default backend, `code-converge` runs non-interactive `codex exec --output-schema --output-last-message -` in the current directory. By default it resolves one review base in this order: an explicit review-base setting, the base of one open pull request for the current branch and its configured push/provider repository (or `origin` when Git's usual push-remote settings are absent), `branch..gh-merge-base`, then exactly one remote default-branch ref. Provider discovery verifies the PR head repository and branch against the current branch's provider identity, resolves the PR base branch against one remote-tracking ref and compares its commit SHA with the provider's advertised base SHA. If that uniquely selected remote-tracking ref is stale, Code-Converge fetches only the advertised base branch into that ref, then verifies the SHA again; a failed or still-mismatched refresh is an actionable operational error. It never fetches for an explicit review-base, an ambiguous/local-only candidate, or any other discovery source. The resolved base SHA is pinned for every review in the run. Code-Converge computes the merge-base and prepares a private Git index from that tree plus the current worktree; this includes committed, staged, unstaged and untracked changes without modifying the real index or worktree. A review-only Git helper applies that index only after confirming that a Git command targets the reviewed repository, while absolute Git paths, repository-creation commands, and commands targeting another repository retain their normal index. Code-Converge forces its wrapper-first `PATH` and neutral shell-startup settings through the per-review Codex shell policy, disables login-shell startup, and removes inherited Git repository/index/config transports and exported shell functions for that review. This prevents startup files or caller state from reordering `PATH`, replacing `git`, or redirecting Git away from the intended repository/index. The helper reads its private configuration from a sidecar file, so an `include_only` policy that allows `PATH` needs no extra helper variables; `GIT_INDEX_FILE` is not exported to Codex. All paths recorded in that sidecar must be valid UTF-8; unsupported Linux byte paths fail before the helper is created rather than being replaced during JSON encoding. Reviewed-root commands that explicitly enable Git split-index are rejected before they can create shared-index state outside the disposable command index. The review instruction compares `git diff --cached` from the computed merge base through the scoped helper. Ambiguous, missing, or unrefreshable candidates fail with a diagnostic before Codex starts. Provider discovery through `gh` is optional; unavailable `gh` or authentication falls through to local Git sources. No PR or other remote mutation occurs. `--review-base `, `CODE_CONVERGE_REVIEW_BASE` and `.code-converge/review-base` explicitly select the base using the normal configuration precedence. A branch already merged into the selected base has no committed delta but still reviews worktree changes; a fully clean run follows the existing clean/no-change path. It uses the model and reasoning effort resolved from the selected mode and any explicit stage overrides. diff --git a/internal/agent/review.go b/internal/agent/review.go new file mode 100644 index 0000000..4a7bdb1 --- /dev/null +++ b/internal/agent/review.go @@ -0,0 +1,204 @@ +// Package agent defines the provider-neutral workflow contract. +package agent + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + + "github.com/dapi/code-converge/internal/repository" +) + +type Agent interface { + Review(context.Context) (ReviewResult, error) + FixFindings(context.Context, string) error + FixCI(context.Context) error +} + +type Counts struct{ Critical, High, Medium, Low, Unknown int } + +func (c Counts) Total() int { return c.Critical + c.High + c.Medium + c.Low + c.Unknown } + +type ReviewResult struct { + Clean, ScopeEmpty bool + Counts Counts + Report string + Scope repository.ReviewTarget +} + +type structuredReview struct { + Findings *[]structuredFinding `json:"findings"` + OverallCorrectness *string `json:"overall_correctness"` + OverallExplanation *string `json:"overall_explanation"` + OverallConfidenceScore *float64 `json:"overall_confidence_score"` +} +type structuredFinding struct { + Title *string `json:"title"` + Body *string `json:"body"` + ConfidenceScore *float64 `json:"confidence_score"` + Priority *int `json:"priority"` + CodeLocation *structuredCodeLocation `json:"code_location"` +} +type structuredCodeLocation struct { + AbsoluteFilePath *string `json:"absolute_file_path"` + LineRange *structuredLineRange `json:"line_range"` +} +type structuredLineRange struct { + Start *int `json:"start"` + End *int `json:"end"` +} + +// ParseStructuredReview accepts only the exact Code-Converge findings schema. +func ParseStructuredReview(data []byte) (ReviewResult, error) { + if err := RejectDuplicateJSONKeys(data); err != nil { + return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) + } + if err := exactKeys(raw, "findings", "overall_correctness", "overall_explanation", "overall_confidence_score"); err != nil { + return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) + } + if err := validateStructuredReviewKeys(raw); err != nil { + return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) + } + var response structuredReview + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&response); err != nil { + return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) + } + if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) { + return ReviewResult{}, errors.New("structured review response contains trailing data") + } + if response.Findings == nil || response.OverallCorrectness == nil || response.OverallExplanation == nil || response.OverallConfidenceScore == nil { + return ReviewResult{}, errors.New("structured review response is incomplete") + } + var counts Counts + for _, f := range *response.Findings { + if f.Title == nil || f.Body == nil || f.ConfidenceScore == nil || f.Priority == nil || f.CodeLocation == nil || f.CodeLocation.AbsoluteFilePath == nil || f.CodeLocation.LineRange == nil || f.CodeLocation.LineRange.Start == nil || f.CodeLocation.LineRange.End == nil { + return ReviewResult{}, errors.New("structured review response contains an incomplete finding") + } + if *f.Priority < 0 || *f.Priority > 3 { + return ReviewResult{}, errors.New("structured review response contains an invalid finding priority") + } + switch *f.Priority { + case 0: + counts.Critical++ + case 1: + counts.High++ + case 2: + counts.Medium++ + case 3: + counts.Low++ + } + } + if len(*response.Findings) == 0 { + return ReviewResult{Clean: true}, nil + } + return ReviewResult{Counts: counts}, nil +} +func validateStructuredReviewKeys(response map[string]json.RawMessage) error { + var findings []json.RawMessage + if err := json.Unmarshal(response["findings"], &findings); err != nil { + return fmt.Errorf("structured review response findings: %w", err) + } + for _, rawFinding := range findings { + var finding map[string]json.RawMessage + if err := json.Unmarshal(rawFinding, &finding); err != nil { + return fmt.Errorf("structured review response finding: %w", err) + } + if err := exactKeys(finding, "title", "body", "confidence_score", "priority", "code_location"); err != nil { + return fmt.Errorf("structured review response finding: %w", err) + } + var location map[string]json.RawMessage + if err := json.Unmarshal(finding["code_location"], &location); err != nil { + return fmt.Errorf("structured review response code location: %w", err) + } + if err := exactKeys(location, "absolute_file_path", "line_range"); err != nil { + return fmt.Errorf("structured review response code location: %w", err) + } + var lineRange map[string]json.RawMessage + if err := json.Unmarshal(location["line_range"], &lineRange); err != nil { + return fmt.Errorf("structured review response line range: %w", err) + } + if err := exactKeys(lineRange, "start", "end"); err != nil { + return fmt.Errorf("structured review response line range: %w", err) + } + } + return nil +} + +func exactKeys(object map[string]json.RawMessage, names ...string) error { + if len(object) != len(names) { + return errors.New("structured review response contains unknown or missing fields") + } + for _, n := range names { + if _, ok := object[n]; !ok { + return errors.New("structured review response contains unknown or missing fields") + } + } + return nil +} +func RejectDuplicateJSONKeys(data []byte) error { + d := json.NewDecoder(bytes.NewReader(data)) + if err := scan(d); err != nil { + return err + } + if _, err := d.Token(); !errors.Is(err, io.EOF) { + return errors.New("structured review response contains trailing data") + } + return nil +} +func scan(d *json.Decoder) error { + t, err := d.Token() + if err != nil { + return err + } + delim, ok := t.(json.Delim) + if !ok { + return nil + } + switch delim { + case '{': + seen := map[string]struct{}{} + for d.More() { + k, err := d.Token() + if err != nil { + return err + } + n, ok := k.(string) + if !ok { + return errors.New("JSON contains an invalid object key") + } + if _, dup := seen[n]; dup { + return fmt.Errorf("JSON contains duplicate field %q", n) + } + seen[n] = struct{}{} + if err := scan(d); err != nil { + return err + } + } + end, err := d.Token() + if err != nil || end != json.Delim('}') { + return errors.New("JSON contains an unclosed object") + } + case '[': + for d.More() { + if err := scan(d); err != nil { + return err + } + } + end, err := d.Token() + if err != nil || end != json.Delim(']') { + return errors.New("JSON contains an unclosed array") + } + default: + return errors.New("JSON contains an unexpected delimiter") + } + return nil +} diff --git a/internal/app/app.go b/internal/app/app.go index 05a5ef7..bdab24b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -10,9 +10,11 @@ import ( "strings" "time" + "github.com/dapi/code-converge/internal/agent" "github.com/dapi/code-converge/internal/codex" "github.com/dapi/code-converge/internal/config" "github.com/dapi/code-converge/internal/event" + "github.com/dapi/code-converge/internal/pi" "github.com/dapi/code-converge/internal/repository" "github.com/dapi/code-converge/internal/runner" "github.com/dapi/code-converge/internal/session" @@ -32,6 +34,8 @@ type globalFlagSpec struct { var globalFlagSpecs = []globalFlagSpec{ {"log-format", "Output", "Workflow output format: human or kv.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "log-format", &o.LogFormat) }}, + {"agent-backend", "Agent", "Agent backend: codex (default) or pi.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "agent-backend", &o.AgentBackend) }}, + {"pi-executable", "Agent", "Pi executable when agent-backend=pi (default pi).", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "pi-executable", &o.PiExecutable) }}, {"heartbeat", "Output", "Human-output liveness interval.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "heartbeat", &o.Heartbeat) }}, {"color", "Output", "Interactive human-output color: auto, always, or never.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "color", &o.Color) }}, {"mode", "Workflow", "Execution profile: fast or best.", func(f *flag.FlagSet, o *config.Overrides) { bind(f, "mode", &o.Mode) }}, @@ -297,8 +301,21 @@ func (a App) Run(ctx context.Context, args []string) int { if view != nil { agentOutput = logger.AgentOutput } - agent := codex.Adapter{Runner: processRunner, Config: cfg, ReviewScope: reviewScope, Output: agentOutput} - w := workflow.Workflow{Config: cfg, Agent: &agent, Repository: repository.Status{Runner: processRunner}, Log: &logger, Err: stderr, Now: a.Now} + var selectedAgent agent.Agent + switch cfg.AgentBackend { + case "codex": + selectedAgent = &codex.Adapter{Runner: processRunner, Config: cfg, ReviewScope: reviewScope, Output: agentOutput} + case "pi": + if err := pi.Validate(runCtx, processRunner, cfg.PiExecutable); err != nil { + fmt.Fprintf(stderr, "code-converge: configuration: %v\n", err) + return workflow.ExitOperational + } + selectedAgent = &pi.Adapter{Runner: processRunner, Config: cfg, ReviewScope: reviewScope, Output: agentOutput} + default: + fmt.Fprintf(stderr, "code-converge: configuration: unsupported agent backend %q\n", cfg.AgentBackend) + return workflow.ExitOperational + } + w := workflow.Workflow{Config: cfg, Agent: selectedAgent, Repository: repository.Status{Runner: processRunner}, Log: &logger, Err: stderr, Now: a.Now} return w.Run(runCtx) } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 4c5ecc1..1682cdd 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -385,6 +385,14 @@ func (f *appFakeRunner) Run(_ context.Context, invocation runner.Invocation) (ru } return f.status, f.statusErr } + if invocation.Executable == "pi" { + if strings.Join(invocation.Args, " ") == "--help" { + return runner.Result{Stdout: "--print --mode --no-session --no-extensions --no-skills --no-prompt-templates --no-themes --no-context-files --no-approve --no-tools --tools"}, f.err + } + if strings.Contains(strings.Join(invocation.Args, " "), "--mode json") { + return runner.Result{Stdout: "{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"{\\\"findings\\\":[],\\\"overall_correctness\\\":\\\"correct\\\",\\\"overall_explanation\\\":\\\"clean\\\",\\\"overall_confidence_score\\\":1}\"}]}}\n{\"type\":\"agent_settled\"}\n"}, f.err + } + } 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 { @@ -416,6 +424,21 @@ func TestNilStreamsAndCwdDoNotPanic(t *testing.T) { } } +func TestAppSelectsPiBackend(t *testing.T) { + root, home := testRepo(t) + fake := &appFakeRunner{t: t, statusResults: []runner.Result{{}, {}}} + code := (App{Cwd: root, Home: home, Runner: fake}).Run(context.Background(), []string{"--agent-backend=pi", "--no-session-log"}) + if code != workflow.ExitSuccess { + t.Fatalf("code=%d invocations=%#v", code, fake.invocations) + } + for _, invocation := range fake.invocations { + if invocation.Executable == "pi" { + return + } + } + t.Fatalf("Pi was not invoked: %#v", fake.invocations) +} + func TestConfigCommandInvalidFlagDoesNotEmitRunEvents(t *testing.T) { root, home := testRepo(t) var stdout, stderr bytes.Buffer diff --git a/internal/codex/adapter.go b/internal/codex/adapter.go index 79c9feb..7735b68 100644 --- a/internal/codex/adapter.go +++ b/internal/codex/adapter.go @@ -1,65 +1,23 @@ package codex import ( - "bytes" "context" - "encoding/json" "errors" "fmt" - "io" "os" "path/filepath" "strconv" "strings" "unicode/utf8" + "github.com/dapi/code-converge/internal/agent" "github.com/dapi/code-converge/internal/config" "github.com/dapi/code-converge/internal/repository" "github.com/dapi/code-converge/internal/runner" ) -type Counts struct { - Critical int - High int - Medium int - Low int - Unknown int -} - -func (c Counts) Total() int { return c.Critical + c.High + c.Medium + c.Low + c.Unknown } - -type ReviewResult struct { - Clean bool - ScopeEmpty bool - Counts Counts - Report string - Scope repository.ReviewTarget -} - -type structuredReview struct { - Findings *[]structuredFinding `json:"findings"` - OverallCorrectness *string `json:"overall_correctness"` - OverallExplanation *string `json:"overall_explanation"` - OverallConfidenceScore *float64 `json:"overall_confidence_score"` -} - -type structuredFinding struct { - Title *string `json:"title"` - Body *string `json:"body"` - ConfidenceScore *float64 `json:"confidence_score"` - Priority *int `json:"priority"` - CodeLocation *structuredCodeLocation `json:"code_location"` -} - -type structuredCodeLocation struct { - AbsoluteFilePath *string `json:"absolute_file_path"` - LineRange *structuredLineRange `json:"line_range"` -} - -type structuredLineRange struct { - Start *int `json:"start"` - End *int `json:"end"` -} +type Counts = agent.Counts +type ReviewResult = agent.ReviewResult type Adapter struct { Runner runner.Runner @@ -273,167 +231,10 @@ func modelArgs(model, effort string) []string { } func parseStructuredReview(data []byte) (ReviewResult, error) { - if err := rejectDuplicateJSONKeys(data); err != nil { - return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) - } - if err := validateStructuredReviewKeys(data); err != nil { - return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) - } - decoder := json.NewDecoder(bytes.NewReader(data)) - decoder.DisallowUnknownFields() - var response structuredReview - if err := decoder.Decode(&response); err != nil { - return ReviewResult{}, fmt.Errorf("parse structured review response: %w", err) - } - if err := decoder.Decode(&struct{}{}); !errors.Is(err, io.EOF) { - return ReviewResult{}, errors.New("structured review response contains trailing data") - } - if err := validateStructuredReview(response); err != nil { - return ReviewResult{}, err - } - if len(*response.Findings) == 0 { - return ReviewResult{Clean: true}, nil - } - var counts Counts - for _, finding := range *response.Findings { - switch *finding.Priority { - case 0: - counts.Critical++ - case 1: - counts.High++ - case 2: - counts.Medium++ - case 3: - counts.Low++ - } - } - return ReviewResult{Counts: counts}, nil + return agent.ParseStructuredReview(data) } -func validateStructuredReviewKeys(data []byte) error { - var response map[string]json.RawMessage - if err := json.Unmarshal(data, &response); err != nil { - return err - } - if err := requireExactJSONKeys(response, "findings", "overall_correctness", "overall_explanation", "overall_confidence_score"); err != nil { - return fmt.Errorf("structured review response: %w", err) - } - var findings []json.RawMessage - if err := json.Unmarshal(response["findings"], &findings); err != nil { - return fmt.Errorf("structured review response findings: %w", err) - } - for _, rawFinding := range findings { - var finding map[string]json.RawMessage - if err := json.Unmarshal(rawFinding, &finding); err != nil { - return fmt.Errorf("structured review response finding: %w", err) - } - if err := requireExactJSONKeys(finding, "title", "body", "confidence_score", "priority", "code_location"); err != nil { - return fmt.Errorf("structured review response finding: %w", err) - } - var location map[string]json.RawMessage - if err := json.Unmarshal(finding["code_location"], &location); err != nil { - return fmt.Errorf("structured review response code location: %w", err) - } - if err := requireExactJSONKeys(location, "absolute_file_path", "line_range"); err != nil { - return fmt.Errorf("structured review response code location: %w", err) - } - var lineRange map[string]json.RawMessage - if err := json.Unmarshal(location["line_range"], &lineRange); err != nil { - return fmt.Errorf("structured review response line range: %w", err) - } - if err := requireExactJSONKeys(lineRange, "start", "end"); err != nil { - return fmt.Errorf("structured review response line range: %w", err) - } - } - return nil -} - -func requireExactJSONKeys(object map[string]json.RawMessage, names ...string) error { - if len(object) != len(names) { - return errors.New("contains unknown or missing fields") - } - for _, name := range names { - if _, ok := object[name]; !ok { - return errors.New("contains unknown or missing fields") - } - } - return nil -} - -func validateStructuredReview(response structuredReview) error { - if response.Findings == nil || response.OverallCorrectness == nil || response.OverallExplanation == nil || response.OverallConfidenceScore == nil { - return errors.New("structured review response is incomplete") - } - for _, finding := range *response.Findings { - if finding.Title == nil || finding.Body == nil || finding.ConfidenceScore == nil || finding.Priority == nil || finding.CodeLocation == nil || finding.CodeLocation.AbsoluteFilePath == nil || finding.CodeLocation.LineRange == nil || finding.CodeLocation.LineRange.Start == nil || finding.CodeLocation.LineRange.End == nil { - return errors.New("structured review response contains an incomplete finding") - } - if *finding.Priority < 0 || *finding.Priority > 3 { - return errors.New("structured review response contains an invalid finding priority") - } - } - return 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("structured review response contains trailing data") - } - return nil -} - -func scanJSONValue(decoder *json.Decoder) error { - token, err := decoder.Token() - if err != nil { - return err - } - delimiter, isDelimiter := token.(json.Delim) - if !isDelimiter { - return nil - } - switch delimiter { - case '{': - keys := make(map[string]struct{}) - for decoder.More() { - key, err := decoder.Token() - if err != nil { - return err - } - name, ok := key.(string) - if !ok { - return errors.New("JSON contains an invalid object key") - } - if _, exists := keys[name]; exists { - return fmt.Errorf("JSON contains duplicate field %q", name) - } - keys[name] = struct{}{} - if err := scanJSONValue(decoder); err != nil { - return err - } - } - end, err := decoder.Token() - if err != nil || end != json.Delim('}') { - return errors.New("JSON contains an unclosed object") - } - case '[': - for decoder.More() { - if err := scanJSONValue(decoder); err != nil { - return err - } - } - end, err := decoder.Token() - if err != nil || end != json.Delim(']') { - return errors.New("JSON contains an unclosed array") - } - default: - return errors.New("JSON contains an unexpected delimiter") - } - return nil -} +func rejectDuplicateJSONKeys(data []byte) error { return agent.RejectDuplicateJSONKeys(data) } const reviewSchema = `{ "type": "object", diff --git a/internal/config/config.go b/internal/config/config.go index 9bac6bb..481a3a3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,6 +28,8 @@ type OptionalString struct { type Overrides struct { LogFormat OptionalString + AgentBackend OptionalString + PiExecutable OptionalString Heartbeat OptionalString Color OptionalString Mode OptionalString @@ -65,6 +67,8 @@ type Config struct { Root string LogFormat string + AgentBackend string + PiExecutable string Heartbeat time.Duration Color string Mode string @@ -104,6 +108,8 @@ type fileConfig map[string]string type yamlFileConfig struct { LogFormat *string `yaml:"log-format"` + AgentBackend *string `yaml:"agent-backend"` + PiExecutable *string `yaml:"pi-executable"` Heartbeat *string `yaml:"heartbeat"` Color *string `yaml:"color"` Mode *string `yaml:"mode"` @@ -177,6 +183,24 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { if logFormat != "kv" && logFormat != "human" { return Config{}, fmt.Errorf("log-format must be one of: kv, human") } + agentBackend, agentBackendSetting, err := resolve(spec{ + name: "agent-backend", file: "agent-backend", env: "CODE_CONVERGE_AGENT_BACKEND", def: "codex", builtIn: "codex", defSource: SourceDefault, override: overrides.AgentBackend, + }, cwd, userDir, projectDir) + if err != nil { + return Config{}, err + } + if agentBackend != "codex" && agentBackend != "pi" { + return Config{}, fmt.Errorf("agent-backend must be one of: codex, pi") + } + piExecutable, piExecutableSetting, err := resolve(spec{ + name: "pi-executable", file: "pi-executable", env: "CODE_CONVERGE_PI_EXECUTABLE", def: "pi", builtIn: "pi", defSource: SourceDefault, override: overrides.PiExecutable, + }, cwd, userDir, projectDir) + if err != nil { + return Config{}, err + } + if agentBackend == "pi" && strings.TrimSpace(piExecutable) == "" { + return Config{}, fmt.Errorf("pi-executable must not be empty when agent-backend=pi") + } color, colorSetting, err := resolve(spec{ name: "color", file: "color", env: "CODE_CONVERGE_COLOR", def: "auto", builtIn: "auto", defSource: SourceDefault, override: overrides.Color, }, cwd, userDir, projectDir) @@ -241,8 +265,8 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { } values := make(map[string]string, len(specs)) - settings := make([]Setting, 0, len(specs)+4) - settings = append(settings, logFormatSetting, heartbeatSetting, colorSetting, modeSetting) + settings := make([]Setting, 0, len(specs)+6) + settings = append(settings, logFormatSetting, agentBackendSetting, piExecutableSetting, heartbeatSetting, colorSetting, modeSetting) for _, item := range specs { value, setting, resolveErr := resolve(item, cwd, userDir, projectDir) if resolveErr != nil { @@ -295,7 +319,7 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { values["fix-prompt"] = documentFixPrompt } return Config{ - Root: root, LogFormat: logFormat, Heartbeat: heartbeat, Color: color, + Root: root, LogFormat: logFormat, AgentBackend: agentBackend, PiExecutable: piExecutable, Heartbeat: heartbeat, Color: color, Mode: mode, MaxCycles: maxCycles, MaxCIRecoveries: maxCI, CITimeout: ciTimeout, ReviewModel: values["review-model"], ReviewEffort: values["review-reasoning-effort"], FixModel: values["fix-model"], FixEffort: values["fix-reasoning-effort"], FixPrompt: values["fix-prompt"], ReviewPrompt: reviewPrompt, DocumentReview: documentReview, @@ -560,6 +584,8 @@ func readFileConfig(dir string) (fileConfig, error) { } } setString("log-format", decoded.LogFormat) + setString("agent-backend", decoded.AgentBackend) + setString("pi-executable", decoded.PiExecutable) setString("heartbeat", decoded.Heartbeat) setString("color", decoded.Color) setString("mode", decoded.Mode) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 566c1a9..61315ea 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -10,7 +10,7 @@ import ( ) var codeConvergeEnv = []string{ - "CODE_CONVERGE_LOG_FORMAT", "CODE_CONVERGE_HEARTBEAT", "CODE_CONVERGE_COLOR", "CODE_CONVERGE_MODE", + "CODE_CONVERGE_LOG_FORMAT", "CODE_CONVERGE_AGENT_BACKEND", "CODE_CONVERGE_PI_EXECUTABLE", "CODE_CONVERGE_HEARTBEAT", "CODE_CONVERGE_COLOR", "CODE_CONVERGE_MODE", "CODE_CONVERGE_MAX_CYCLES", "CODE_CONVERGE_MAX_CI_RECOVERIES", "CODE_CONVERGE_CI_TIMEOUT", "CODE_CONVERGE_REVIEW_MODEL", "CODE_CONVERGE_REVIEW_REASONING_EFFORT", "CODE_CONVERGE_FIX_MODEL", "CODE_CONVERGE_FIX_REASONING_EFFORT", "CODE_CONVERGE_FIX_PROMPT_FILE", "CODE_CONVERGE_FINALIZE_MODEL", "CODE_CONVERGE_FINALIZE_REASONING_EFFORT", "CODE_CONVERGE_FINALIZE_PROMPT_FILE", "CODE_CONVERGE_CI_FIX_MODEL", @@ -160,6 +160,38 @@ func TestResolveLogFormatReadsYAML(t *testing.T) { } } +func TestAgentBackendConfiguration(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + cfg, err := Load(root, home, Overrides{AgentBackend: OptionalString{Value: "pi", Set: true}, PiExecutable: OptionalString{Value: "custom-pi", Set: true}}) + if err != nil { + t.Fatal(err) + } + if cfg.AgentBackend != "pi" || cfg.PiExecutable != "custom-pi" { + t.Fatalf("backend=%q executable=%q", cfg.AgentBackend, cfg.PiExecutable) + } +} + +func TestPiExecutableIsRequiredOnlyForPi(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + if _, err := Load(root, home, Overrides{PiExecutable: OptionalString{Value: "", Set: true}}); err != nil { + t.Fatalf("Codex configuration: %v", err) + } + if _, err := Load(root, home, Overrides{AgentBackend: OptionalString{Value: "pi", Set: true}, PiExecutable: OptionalString{Value: "", Set: true}}); err == nil { + t.Fatal("expected Pi executable error") + } +} + +func TestInvalidAgentBackend(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + _, err := Load(root, home, Overrides{AgentBackend: OptionalString{Value: "other", Set: true}}) + if err == nil || !strings.Contains(err.Error(), "agent-backend") { + t.Fatalf("err=%v", err) + } +} + func TestDefaultsAndProfileResolution(t *testing.T) { cleanEnv(t) root, home := repo(t) diff --git a/internal/pi/adapter.go b/internal/pi/adapter.go new file mode 100644 index 0000000..cb2c78e --- /dev/null +++ b/internal/pi/adapter.go @@ -0,0 +1,219 @@ +// Package pi adapts the locally configured Pi CLI to Code-Converge's agent contract. +package pi + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "fmt" + "path/filepath" + "strings" + + "github.com/dapi/code-converge/internal/agent" + "github.com/dapi/code-converge/internal/config" + "github.com/dapi/code-converge/internal/repository" + "github.com/dapi/code-converge/internal/runner" +) + +var requiredCLIFlags = []string{"--print", "--mode", "--no-session", "--no-extensions", "--no-skills", "--no-prompt-templates", "--no-themes", "--no-context-files", "--no-approve", "--no-tools", "--tools"} + +// Validate confirms that the selected executable exposes the non-interactive +// protocol and isolation flags required by this adapter before a workflow starts. +func Validate(ctx context.Context, r runner.Runner, executable string) error { + result, err := r.Run(ctx, runner.Invocation{Executable: executable, Args: []string{"--help"}}) + if err != nil { + return fmt.Errorf("inspect Pi CLI capabilities: %w", err) + } + help := result.Stdout + "\n" + result.Stderr + for _, flag := range requiredCLIFlags { + if !strings.Contains(help, flag) { + return fmt.Errorf("Pi executable does not support required %s capability", flag) + } + } + return nil +} + +type Adapter struct { + Runner runner.Runner + Config config.Config + ReviewScope *repository.ReviewScope + Output func(source string, data []byte) + documentPaths []string +} + +func (a *Adapter) Review(ctx context.Context) (agent.ReviewResult, error) { + if a.ReviewScope == nil { + return agent.ReviewResult{}, errors.New("review scope is required") + } + target, err := a.ReviewScope.Prepare(ctx) + if err != nil { + return agent.ReviewResult{}, err + } + if strings.TrimSpace(target.BaseCommit) == "" || strings.TrimSpace(target.MergeBase) == "" { + return agent.ReviewResult{}, errors.New("review target requires a selected base commit and merge base") + } + a.documentPaths = append(a.documentPaths[:0], target.DocumentPaths...) + if a.Config.DocumentReview && len(target.DocumentPaths) == 0 { + return agent.ReviewResult{Clean: true, ScopeEmpty: true, Scope: target}, nil + } + patch, err := reviewPatch(ctx, a.Runner, target) + if err != nil { + return agent.ReviewResult{}, err + } + result, err := a.Runner.Run(ctx, runner.Invocation{ + Executable: a.Config.PiExecutable, + Args: []string{"--print", "--mode", "json", "--no-session", "--no-extensions", "--no-skills", "--no-prompt-templates", "--no-themes", "--no-context-files", "--no-approve", "--no-tools"}, + Stdin: reviewPrompt(target, a.Config, patch), + }) + if err != nil { + return agent.ReviewResult{}, err + } + report, err := finalAssistantMessage(result.Stdout) + if err != nil { + return agent.ReviewResult{}, err + } + review, err := agent.ParseStructuredReview([]byte(report)) + if err != nil { + return agent.ReviewResult{}, err + } + review.Report = strings.TrimSpace(report) + review.Scope = target + return review, nil +} + +func (a *Adapter) FixFindings(ctx context.Context, report string) error { + prompt := a.Config.FixPrompt + "\n\nReview findings to address:\n\n" + report + if a.Config.DocumentReview { + if len(a.documentPaths) == 0 { + return errors.New("document review fix scope is unavailable") + } + prompt += "\n\nDocument fix scope:\nFix only confirmed findings in the eligible Markdown paths listed below. Do not inspect or modify any other file in the worktree.\n" + strings.Join(a.documentPaths, "\n") + } + _, err := a.Runner.Run(ctx, runner.Invocation{Executable: a.Config.PiExecutable, Args: interactiveArgs(), Stdin: prompt, Output: a.output()}) + return err +} + +func (a *Adapter) FixCI(ctx context.Context) error { + _, err := a.Runner.Run(ctx, runner.Invocation{Executable: a.Config.PiExecutable, Args: interactiveArgs(), Stdin: a.Config.CIFixPrompt, Output: a.output()}) + return err +} + +func interactiveArgs() []string { + // Remediation can read and edit only the current worktree. In particular, it + // cannot use Pi's bash tool to perform host-owned Git or GitHub operations. + return []string{"--print", "--no-session", "--no-extensions", "--no-skills", "--no-prompt-templates", "--no-themes", "--no-context-files", "--no-approve", "--tools", "read,edit,write"} +} + +func reviewPatch(ctx context.Context, r runner.Runner, target repository.ReviewTarget) (string, error) { + args := []string{"diff", "--cached", target.MergeBase} + if len(target.DocumentPaths) > 0 { + args = append(args, "--") + for _, path := range target.DocumentPaths { + args = append(args, ":(top,literal)"+path) + } + } + result, err := r.Run(ctx, runner.Invocation{Executable: scopedGitExecutable(target), Args: args, Env: target.Env, UnsetEnv: target.UnsetEnv}) + if err != nil { + return "", fmt.Errorf("read private review patch: %w", err) + } + return result.Stdout, nil +} + +func scopedGitExecutable(target repository.ReviewTarget) string { + for _, value := range target.Env { + if path, ok := strings.CutPrefix(value, "PATH="); ok && path != "" { + return filepath.Join(strings.Split(path, string(filepath.ListSeparator))[0], "git") + } + } + return "git" +} + +func reviewPrompt(target repository.ReviewTarget, cfg config.Config, patch string) string { + prompt := fmt.Sprintf(`Review the following private Git diff from merge base %s. Return only one JSON object with exactly this schema: +{"findings":[{"title":"string","body":"string","confidence_score":0,"priority":0,"code_location":{"absolute_file_path":"string","line_range":{"start":1,"end":1}}}],"overall_correctness":"string","overall_explanation":"string","overall_confidence_score":0} + +Use an empty findings array when there are no actionable findings. Priority must be 0, 1, 2, or 3. Do not use tools. Do not add Markdown or prose outside the JSON object. + +PATCH:\n%s`, target.MergeBase, patch) + if cfg.ReviewPrompt != "" { + prompt += "\n\nAdditional review criteria:\n\n" + cfg.ReviewPrompt + } + if cfg.DocumentReview { + prompt += "\n\nEligible Markdown paths:\n" + strings.Join(target.DocumentPaths, "\n") + } + return prompt +} + +type event struct { + Type string `json:"type"` + Message *struct { + Role string `json:"role"` + Content []struct { + Type string `json:"type"` + Text string `json:"text"` + } `json:"content"` + } `json:"message"` +} + +func finalAssistantMessage(output string) (string, error) { + if !strings.HasSuffix(output, "\n") { + return "", errors.New("Pi JSONL output has a partial final line") + } + var candidate string + found, settled, candidateIsLast := false, false, false + scanner := bufio.NewScanner(strings.NewReader(output)) + buffer := make([]byte, 0, 64*1024) + scanner.Buffer(buffer, 4*1024*1024) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + var item event + if err := json.Unmarshal([]byte(line), &item); err != nil { + return "", fmt.Errorf("parse Pi JSONL event: %w", err) + } + if settled { + return "", errors.New("Pi output contains an event after agent_settled") + } + if item.Type == "agent_settled" { + if !candidateIsLast { + return "", errors.New("Pi output has no final assistant message before agent_settled") + } + settled = true + continue + } + candidateIsLast = false + if item.Type != "message_end" { + continue + } + if item.Message == nil || item.Message.Role != "assistant" { + return "", errors.New("Pi message_end is not an assistant message") + } + if found { + return "", errors.New("Pi output contains multiple assistant message_end events") + } + if len(item.Message.Content) != 1 || item.Message.Content[0].Type != "text" || item.Message.Content[0].Text == "" { + return "", errors.New("Pi final assistant message must contain exactly one non-empty text block") + } + candidate, found, candidateIsLast = item.Message.Content[0].Text, true, true + } + if err := scanner.Err(); err != nil { + return "", fmt.Errorf("read Pi JSONL output: %w", err) + } + if !found { + return "", errors.New("Pi output has no assistant message_end event") + } + if !settled { + return "", errors.New("Pi output has no agent_settled event") + } + return candidate, nil +} + +func (a *Adapter) output() func(runner.Output) { + if a.Output == nil { + return nil + } + return func(chunk runner.Output) { a.Output(chunk.Source, chunk.Data) } +} diff --git a/internal/pi/adapter_test.go b/internal/pi/adapter_test.go new file mode 100644 index 0000000..216f592 --- /dev/null +++ b/internal/pi/adapter_test.go @@ -0,0 +1,101 @@ +package pi + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/dapi/code-converge/internal/repository" + + "github.com/dapi/code-converge/internal/config" + "github.com/dapi/code-converge/internal/runner" +) + +type fakeRunner struct { + invocations []runner.Invocation + result runner.Result + err error +} + +func (r *fakeRunner) Run(_ context.Context, invocation runner.Invocation) (runner.Result, error) { + r.invocations = append(r.invocations, invocation) + return r.result, r.err +} + +func TestValidate(t *testing.T) { + help := strings.Join(requiredCLIFlags, " ") + if err := Validate(context.Background(), &fakeRunner{result: runner.Result{Stdout: help}}, "pi"); err != nil { + t.Fatal(err) + } + if err := Validate(context.Background(), &fakeRunner{result: runner.Result{Stdout: "--print"}}, "pi"); err == nil { + t.Fatal("expected unsupported capability error") + } +} + +func TestFinalAssistantMessage(t *testing.T) { + output := "{\"type\":\"agent_start\"}\n" + + "{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"{\\\"findings\\\":[]}\"}]}}\n" + + "{\"type\":\"agent_settled\"}\n" + got, err := finalAssistantMessage(output) + if err != nil { + t.Fatal(err) + } + if got != `{"findings":[]}` { + t.Fatalf("report = %q", got) + } +} + +func TestFinalAssistantMessageRejectsAmbiguousOutput(t *testing.T) { + for name, output := range map[string]string{ + "partial": "{\"type\":\"agent_start\"}", + "missing": "{\"type\":\"agent_start\"}\n", + "unsettled": "{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"a\"}]}}\n", + "intervening": "{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"a\"}]}}\n{\"type\":\"turn_end\"}\n{\"type\":\"agent_settled\"}\n", + "multiple": "{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"a\"}]}}\n{\"type\":\"message_end\",\"message\":{\"role\":\"assistant\",\"content\":[{\"type\":\"text\",\"text\":\"b\"}]}}\n", + } { + t.Run(name, func(t *testing.T) { + if _, err := finalAssistantMessage(output); err == nil { + t.Fatal("expected error") + } + }) + } +} + +func TestReviewPatchUsesLiteralDocumentPathspecs(t *testing.T) { + r := &fakeRunner{} + _, err := reviewPatch(context.Background(), r, repository.ReviewTarget{MergeBase: "base", DocumentPaths: []string{":(exclude)secret.md"}}) + if err != nil { + t.Fatal(err) + } + if got, want := r.invocations[0].Args, []string{"diff", "--cached", "base", "--", ":(top,literal):(exclude)secret.md"}; !reflect.DeepEqual(got, want) { + t.Fatalf("args = %#v, want %#v", got, want) + } +} + +func TestFixUsesConfiguredPi(t *testing.T) { + r := &fakeRunner{} + a := Adapter{Runner: r, Config: config.Config{PiExecutable: "custom-pi", FixPrompt: "fix"}} + if err := a.FixFindings(context.Background(), "report"); err != nil { + t.Fatal(err) + } + if len(r.invocations) != 1 || r.invocations[0].Executable != "custom-pi" { + t.Fatalf("invocations = %#v", r.invocations) + } + if !strings.Contains(r.invocations[0].Stdin, "report") { + t.Fatalf("prompt = %q", r.invocations[0].Stdin) + } + args := strings.Join(r.invocations[0].Args, " ") + if strings.Contains(args, "--no-tools") || !strings.Contains(args, "--tools read,edit,write") { + t.Fatalf("fix tools = %q, want edit-only tools", args) + } +} + +func TestFixPropagatesPiFailure(t *testing.T) { + r := &fakeRunner{err: errors.New("Pi failed")} + a := Adapter{Runner: r, Config: config.Config{PiExecutable: "pi", FixPrompt: "fix"}} + if err := a.FixFindings(context.Background(), "report"); err == nil { + t.Fatal("expected error") + } +} diff --git a/internal/workflow/workflow.go b/internal/workflow/workflow.go index 18b102c..6f04f7e 100644 --- a/internal/workflow/workflow.go +++ b/internal/workflow/workflow.go @@ -8,7 +8,7 @@ import ( "strconv" "time" - "github.com/dapi/code-converge/internal/codex" + "github.com/dapi/code-converge/internal/agent" "github.com/dapi/code-converge/internal/config" "github.com/dapi/code-converge/internal/event" "github.com/dapi/code-converge/internal/repository" @@ -23,11 +23,7 @@ const ( ExitInterrupted = 130 ) -type Agent interface { - Review(context.Context) (codex.ReviewResult, error) - FixFindings(context.Context, string) error - FixCI(context.Context) error -} +type Agent = agent.Agent type Repository interface { HasChanges(context.Context) (bool, error) @@ -405,6 +401,9 @@ func (w *Workflow) runFixCI(ctx context.Context, phase, cycle int, now func() ti } func (w *Workflow) stageModel(stage string) string { + if w.Config.AgentBackend == "pi" { + return "agent-default" + } switch stage { case "review": if w.Config.ReviewModel == "" { @@ -427,6 +426,9 @@ func (w *Workflow) stageModel(stage string) string { } func (w *Workflow) stageReasoningEffort(stage string) string { + if w.Config.AgentBackend == "pi" { + return "agent-default" + } switch stage { case "review": if w.Config.ReviewEffort != "" { @@ -485,7 +487,7 @@ func milliseconds(value time.Duration) string { return strconv.FormatInt(ms, 10) } -func countFields(counts codex.Counts) []event.Field { +func countFields(counts agent.Counts) []event.Field { return []event.Field{ intField("findings_total", counts.Total()), intField("findings_critical", counts.Critical), diff --git a/memory-bank/adr/ADR-003-agent-neutral-workflow-contract.md b/memory-bank/adr/ADR-003-agent-neutral-workflow-contract.md new file mode 100644 index 0000000..e9a0e0e --- /dev/null +++ b/memory-bank/adr/ADR-003-agent-neutral-workflow-contract.md @@ -0,0 +1,76 @@ +--- +title: "ADR-003: Agent-neutral workflow contract" +doc_kind: adr +doc_function: canonical +purpose: "Proposes the reusable boundary by which Code-Converge selects local agent implementations without coupling workflow state to one provider." +derived_from: + - ../features/FT-045/brief.md + - ../features/FT-045/design.md + - ADR-002-deterministic-delivery-orchestration.md +status: accepted +decision_status: accepted +date: 2026-09-06 +audience: humans_and_agents +must_not_define: + - implementation_plan +--- + +# ADR-003: Agent-neutral workflow contract + +## Контекст + +`internal/workflow.Agent` currently returns `codex.ReviewResult`. Adding Pi while retaining Codex would otherwise make orchestration depend on a provider package and make every later backend add another workflow-facing provider type. The workflow must retain the host-owned checkpoint, publication and CI boundary established by ADR-002. + +## Драйверы решения + +- Codex remains the default and its classified-result, event and exit semantics remain compatible. +- A selected backend must expose one complete review verdict and two remediation operations. +- Provider protocol parsing, process invocation and tool isolation must remain outside workflow. +- Repository snapshot preparation, checkpointing, publication and CI remain host responsibilities. + +## Рассмотренные варианты + +| Вариант | Плюсы | Минусы | Почему рассматривается как основной кандидат / не основной кандидат | +| --- | --- | --- | --- | +| Keep `workflow.Agent` returning `codex.ReviewResult` | Smallest immediate diff | Workflow remains provider-coupled; Pi imports Codex implementation types | Not selected. | +| Add Pi-specific branches to workflow | No shared type extraction | Duplicates state-machine behavior and risks divergent exits/events | Not selected. | +| Extract an agent-neutral contract and classified result package | Stable workflow boundary; adapters own their protocols | Moves existing shared types and updates tests | Proposed. | + +## Решение + +Adopt an `internal/agent` boundary that owns the backend-neutral `Agent` interface, `ReviewResult`, severity counts and exact structured findings validation reused by Codex and Pi. `internal/workflow` consumes only this package. `internal/codex` and a new `internal/pi` implement the contract; `internal/app` selects one adapter from resolved configuration. Repository snapshot, session diagnostics, publish and CI remain host-owned under ADR-002. + +Accepted through the FT-045 high-risk Human Gate: the requester explicitly approved the high-risk profile, this ADR and the restricted-runtime direction in this conversation on 2026-09-06. + +## Последствия + +### Положительные + +- Later local agent backends do not alter workflow transition policy. +- Pi and Codex have one classified report handoff and one event/exit vocabulary. +- Protocol-specific trust and failure handling stays at adapter boundaries. + +### Отрицательные + +- Existing Codex names and tests move to a new internal package. +- The extracted parser becomes shared critical behavior and needs regression coverage. + +### Нейтральные / организационные + +- ADR-002 remains authoritative for lifecycle operations. +- The architecture and README must name the generic boundary while preserving explicit Codex/Pi protocol documentation. + +## Риски и mitigation + +A careless extraction could alter strict parsing or counters. Move the existing implementation with characterization tests first; retain exact validation and add cross-adapter fixtures. A future backend must implement the same fail-closed contract rather than add workflow exceptions. + +## Follow-up + +- If accepted, realize the boundary through FT-045 `implementation-plan.md`. +- If rejected, update FT-045 design before planning; do not implement a Pi adapter against an unreviewed provider-coupled workflow contract. + +## Связанные ссылки + +- [FT-045 brief](../features/FT-045/brief.md) +- [FT-045 design](../features/FT-045/design.md) +- [ADR-002](ADR-002-deterministic-delivery-orchestration.md) diff --git a/memory-bank/adr/README.md b/memory-bank/adr/README.md index 3fa96db..a47e328 100644 --- a/memory-bank/adr/README.md +++ b/memory-bank/adr/README.md @@ -22,6 +22,7 @@ audience: humans_and_agents - [ADR-001: Interactive terminal runtime](ADR-001-interactive-terminal-runtime.md) — accepted minimal cross-platform terminal capability and raw-mode boundary for FT-010. - [ADR-002: Deterministic delivery orchestration](ADR-002-deterministic-delivery-orchestration.md) — accepted ownership boundary for repository publication and CI polling. +- [ADR-003: Agent-neutral workflow contract](ADR-003-agent-neutral-workflow-contract.md) — accepted reusable boundary for multiple local agent adapters; approved through FT-045 high-risk Human Gate. ## Naming diff --git a/memory-bank/features/FT-045/README.md b/memory-bank/features/FT-045/README.md new file mode 100644 index 0000000..f89c6f2 --- /dev/null +++ b/memory-bank/features/FT-045/README.md @@ -0,0 +1,23 @@ +--- +title: "FT-045: Pi agent backend" +doc_kind: feature +doc_function: index +purpose: "Навигация по документации delivery-единицы issue #45; сначала открывай canonical brief, downstream routes добавляются после соответствующих gates." +derived_from: + - ../../dna/governance.md + - brief.md +status: active +audience: humans_and_agents +--- + +# FT-045: Pi agent backend + +## О разделе + +Этот package фиксирует delivery-единицу issue #45: поддержку Pi как явно выбранного второго agent backend при сохранении Codex default и host-owned workflow. Canonical problem space, validation profile и verify contract принадлежат `brief.md`. Реализация использует операторский локальный Pi runtime: Code-Converge не управляет provider, credentials, model, thinking или transport. + +## Аннотированный индекс + +- [`brief.md`](brief.md) — canonical problem-space owner: scope, non-scope, constraints, validation profile и verify contract. +- [`design.md`](design.md) — canonical solution-space owner: Pi protocol, restricted runtime boundary, contracts, failures and rollout/backout. +- [`implementation-plan.md`](implementation-plan.md) — active derived execution plan: ordered work, sandbox preflight, checks, evidence and stop conditions after the high-risk Human Gate. diff --git a/memory-bank/features/FT-045/brief.md b/memory-bank/features/FT-045/brief.md new file mode 100644 index 0000000..c84d8b9 --- /dev/null +++ b/memory-bank/features/FT-045/brief.md @@ -0,0 +1,131 @@ +--- +title: "FT-045: Pi agent backend" +doc_kind: feature +doc_function: canonical +purpose: "Canonical problem, scope, validation profile and verification contract for issue #45." +derived_from: + - ../../flows/feature.md + - ../../engineering/validation-profiles.md + - ../../../README.md + - https://github.com/dapi/code-converge/issues/45 +status: active +delivery_status: planned +audience: humans_and_agents +must_not_define: + - implementation_sequence + - solution_space +--- + +# FT-045: Pi agent backend + +## What + +### Problem + +Code-Converge currently supports only the Codex agent boundary. Operators that use Pi cannot select it while retaining the existing private review snapshot, strict fail-closed review classification, host-owned publication and CI orchestration. + +### Outcome + +| Metric ID | Metric | Baseline | Target | Measurement method | +| --- | --- | --- | --- | --- | +| `MET-01` | Explicit agent backend availability | Codex only | Pi is selectable; Codex remains the default | Deterministic configuration and fake-executable tests | +| `MET-02` | Review-result trust boundary | Codex final-message file only | Pi accepts only its final assistant message after successful protocol completion and applies the same strict report validation | JSONL fixtures and malformed-result tests | + +### Scope + +- `REQ-01` An operator can explicitly select Pi through a documented CLI/configuration contract; without selection, Codex retains its current public behavior and default status. +- `REQ-02` Pi review runs through the operator-configured local Pi CLI in a fresh stateless process against a host-provided private merge-base-to-worktree patch, including committed, staged, unstaged and untracked changes. It has no tools, approval, project/global extensions, skills, prompts, themes or context files; it receives no generic shell or repository mutation capability. +- `REQ-03` After a successfully settled Pi JSONL protocol, Code-Converge extracts only the final assistant `message_end` text and applies the existing exact findings-report validation. Empty, missing, prose, malformed, incomplete, duplicate/unknown-field, trailing, or unsuccessful results fail operationally with exit `2`; raw protocol/stdout/stderr never enters public workflow output. +- `REQ-04` Pi Fix findings and Fix CI each run in a new Pi process and preserve existing classified-report handoff, checkpoint, review-loop, CI-recovery-budget, exit-code, publication and exact-SHA CI semantics owned by Code-Converge. +- `REQ-05` Explicit backend selection and a Pi executable setting follow the accepted precedence contract. The operator's local Pi configuration owns provider, model, thinking, credentials and transport; Code-Converge neither configures nor validates them. Startup validates only the required non-interactive CLI/JSON protocol capabilities. +- `REQ-06` Deterministic fake-Pi coverage proves invocation arguments, JSONL parsing, cancellation, clean/findings/malformed classification, both fix stages and configuration precedence; existing Codex tests remain green. +- `REQ-07` README and this feature package document the resulting public contract, trust boundary and validation evidence. + +### Non-Scope + +- `NS-01` Replacing, changing the default of, or regressing Codex. +- `NS-02` Allowing Pi to commit, push, discover/create pull requests, poll CI, or publish other remote state. +- `NS-03` Loading project-local Pi extensions, skills, prompts or settings during non-interactive review/fix without an explicitly accepted trust mechanism. +- `NS-04` Real Pi sessions, remote mutation, or hosted CI dependency in automated tests. +- `NS-05` Managing Pi provider configuration, credentials, network transport or the operator's local Pi installation. + +### Constraints / Assumptions + +- `ASM-01` Existing repository snapshot, workflow, runner, session and progress boundaries provide the baseline to preserve. +- `CON-01` Public CLI/configuration and workflow event contracts are owned by `README.md`; diagnostics may retain redacted invocation content but raw Pi protocol data is never a review result or public workflow stdout. +- `CON-02` The supported Pi version, exact public selection/configuration names and feasible non-interactive isolation flags must be evidenced and accepted in solution design rather than inferred from the issue. +- `CON-03` Pi review must remain fail closed if required capabilities cannot be established. +- `ASM-02` Pi executes through the operator's existing local runtime. Code-Converge does not manage Pi provider configuration, credentials, model, thinking or transport. + +## Design Requirement Decision + +| Decision | Reason | Downstream owner | +| --- | --- | --- | +| `Design required: yes` | The feature changes CLI/configuration and trusted process boundaries, introduces a second agent protocol and adapter contract, and requires capability/isolation trade-offs. | `design.md` | + +## Artifact Routing Decision + +| Artifact | Decision | Trigger / reason | Route / owner | +| --- | --- | --- | --- | +| `design.md` | selected | Required solution, protocol, isolation and configuration decisions | `design.md` | +| ADR | pending design analysis | A reusable agent-neutral contract or trust-boundary decision may require an accepted ADR | `memory-bank/adr/` if triggered | +| Separate interaction/C4 artifact | omitted initially | Design can determine whether `design.md` alone gives sufficient reviewable coverage | `design.md` | + +## Validation Profile Decision + +| Profile | Triggers / rationale | Downgrade approval | +| --- | --- | --- | +| `high-risk` | A new cross-system agent integration changes protocol/failure semantics and crosses the trusted process/security boundary. This is stronger than the issue baseline `standard`; no downgrade is taken. | none | + +## Verify + +### Exit Criteria + +- `EC-01` Pi is selectable and performs review/fix stages without weakening Codex default behavior or host ownership of publication and CI. +- `EC-02` Pi review classification and isolation fail closed under every specified invalid/protocol failure condition. +- `EC-03` Deterministic coverage, documentation and required local/CI validation evidence demonstrate the accepted contract. + +### Acceptance Scenarios + +- `SC-01` An explicit Pi selection resolves by documented precedence, validates its executable capabilities before execution, and leaves an unselected run on the existing Codex backend. +- `SC-02` A Pi review sees the existing merge-base-to-worktree snapshot through the supplied read-only Git surface; generic shell and untrusted project-local Pi resources are unavailable in the accepted non-interactive execution model. +- `SC-03` A successful settled Pi stream whose final assistant message is an exact clean or findings report yields the same classified outcome as Codex; earlier events and terminal streams cannot affect it. +- `SC-04` Pi findings enter a fresh Fix findings process with the complete classified report, checkpoints through the current host workflow, and returns to Review; a failed, cancelled or malformed Pi execution exits through existing operational semantics. +- `SC-05` A deterministic failed CI starts a fresh Pi Fix CI process and then restarts Review under unchanged recovery budget and exit semantics. +- `SC-06` README configuration/help contract and diagnostic/session behavior document Pi without raw protocol leakage; required local and CI checks are green. + +### Negative Cases + +- `NEG-01` Missing required Pi capabilities, a non-zero/cancelled process, absent/finally non-assistant `message_end`, multiple candidate terminal messages, malformed JSONL, incomplete stdout/EOF protocol completion, empty/prose/trailing/schema-invalid final text, or terminal-only output exits `2`, never clean. +- `NEG-02` Pi review cannot invoke generic `bash`, target another repository, use a mutable Git surface, or cause Code-Converge host publication/PR/CI responsibilities to transfer to Pi. +- `NEG-03` Configuration source conflicts resolve under the accepted precedence rules; invalid Pi-specific values fail before process invocation and do not fall back silently to Codex. + +### Traceability Matrix + +| Requirement ID | Acceptance refs | Negative refs | Checks | Evidence IDs | +| --- | --- | --- | --- | --- | +| `REQ-01` | `SC-01`, `SC-06` | `NEG-03` | `CHK-01`, `CHK-04` | `EVID-01`, `EVID-04` | +| `REQ-02` | `SC-02` | `NEG-02` | `CHK-02` | `EVID-02` | +| `REQ-03` | `SC-03` | `NEG-01` | `CHK-01`, `CHK-02` | `EVID-01`, `EVID-02` | +| `REQ-04` | `SC-04`, `SC-05` | `NEG-01`, `NEG-02` | `CHK-03` | `EVID-03` | +| `REQ-05` | `SC-01` | `NEG-03` | `CHK-01` | `EVID-01` | +| `REQ-06` | `SC-01`–`SC-05` | `NEG-01`–`NEG-03` | `CHK-01`–`CHK-03` | `EVID-01`–`EVID-03` | +| `REQ-07` | `SC-06` | none | `CHK-04`, `CHK-05` | `EVID-04`, `EVID-05` | + +### Checks + +| Check ID | Covers | How to check | Expected result | Evidence path | +| --- | --- | --- | --- | --- | +| `CHK-01` | Pi config, invocation, capability and JSONL classification | Focused unit/fake-runner/fake-Pi executable tests | All positive and negative protocol/configuration cases pass deterministically | Test output recorded in delivery evidence | +| `CHK-02` | Pi review snapshot/isolation | Deterministic repository/Pi tests and design verification of the controlled review tool | Only scoped read-only snapshot operations are possible; boundary failures fail closed | Test output and design verification | +| `CHK-03` | Fix findings, Fix CI, cancellation and workflow regression | Deterministic workflow/fake executable tests | Fresh Pi processes and unchanged lifecycle outcomes are proven | Test output recorded in delivery evidence | +| `CHK-04` | Full implementation behavior | `go test ./...`; `go vet ./...`; `git diff --check` | All commands pass | Local command outputs | +| `CHK-05` | Documentation and final implementation quality | `make docs-lint`; independent non-authoring review; required CI for published head | Documentation passes; review clean; required CI green | Docs output, review record, CI URL | + +### Evidence + +- `EVID-01` Focused Pi adapter, JSONL, configuration precedence and capability test results. +- `EVID-02` Review-snapshot isolation and fail-closed negative-case evidence. +- `EVID-03` Workflow/fix/cancellation and Codex-regression test results. +- `EVID-04` README plus package documentation validation and semantic contract read-through. +- `EVID-05` Full local validation, clean independent review, committed/pushed head and required CI evidence. diff --git a/memory-bank/features/FT-045/design.md b/memory-bank/features/FT-045/design.md new file mode 100644 index 0000000..c392192 --- /dev/null +++ b/memory-bank/features/FT-045/design.md @@ -0,0 +1,79 @@ +--- +title: "FT-045: Design" +doc_kind: feature +doc_function: canonical +purpose: "Selected Pi backend design for issue #45." +derived_from: + - brief.md + - ../../adr/ADR-002-deterministic-delivery-orchestration.md + - ../../adr/ADR-003-agent-neutral-workflow-contract.md + - ../../../README.md +status: active +audience: humans_and_agents +must_not_define: + - ft_045_scope + - ft_045_acceptance_criteria + - ft_045_evidence_contract + - implementation_sequence +--- + +# FT-045: Design + +## Selected solution + +`agent-backend=codex` remains the built-in default. `agent-backend=pi` explicitly selects a locally installed Pi executable; `pi-executable` selects its command and defaults to `pi`. Both settings use existing CLI > project > user > environment > built-in precedence. Before constructing a selected Pi workflow, Code-Converge runs `pi-executable --help` and fail-closes unless the required non-interactive JSON, isolation and edit-tool flags are advertised. + +The operator-owned Pi installation owns provider selection, credentials, model, thinking and transport. Code-Converge does not copy, inject, validate or report those values, and does not fall back to Codex after a selected Pi invocation fails. + +`internal/agent` owns the provider-neutral `Agent`, result, counts and strict report parser (ADR-003). `internal/app` constructs precisely one adapter. The workflow remains the sole owner of checkpoint, commit, push, pull-request and CI lifecycle operations (ADR-002). + +## Pi invocation contract + +### Review + +Code-Converge prepares the existing private merge-base-to-worktree `ReviewTarget`, obtains its staged diff through the existing scoped Git environment, and sends that patch in Pi's prompt. Pi is started fresh with: + +```text +pi --print --mode json --no-session --no-extensions --no-skills \ + --no-prompt-templates --no-themes --no-context-files --no-approve --no-tools +``` + +The process receives no review tools, shell commands, repository path or mutable Git index. It can review only the host-provided patch. + +After a zero process exit, the adapter accepts exactly one assistant-role `message_end` event with one non-empty text block from complete JSONL stdout. That text is the sole report carrier and must pass `agent.ParseStructuredReview`; malformed JSONL, partial output, absent or multiple candidates and invalid reports fail operationally. stderr and other events are diagnostics, not report input. + +### Remediation + +Fix findings and Fix CI each start a new stateless Pi process in the current worktree with project resources disabled. They allow only Pi's `read`, `edit` and `write` tools: Pi can remediate files but cannot execute shell, Git or GitHub commands. The validated report is passed only to Fix findings. Pi never receives host-owned publication or CI operations; the workflow performs those after agent completion. + +## Invariants + +- An unselected run uses the unchanged Codex adapter path. +- A selected Pi failure never selects Codex. +- Only an exact structured report can classify a Pi review as clean or findings. +- Pi review has no enabled tools and receives only the private patch. +- Each Pi stage is a fresh process; publication and CI ownership stay with Code-Converge. + +## Failure handling + +| Failure | Outcome | +| --- | --- | +| Invalid backend or empty executable | configuration failure before workflow start | +| Pi unavailable, cancelled or non-zero exit | operational failure; no fallback | +| Missing, malformed, partial or ambiguous JSONL | operational failure; never clean | +| Invalid structured final text | operational failure; never clean | +| Fix findings/Fix CI failure | existing workflow failure and exit semantics | + +## Trade-offs + +Pi's provider runtime is deliberately the operator's local Pi runtime, rather than a Code-Converge-managed credential or network boundary. Review exposure is limited at the Pi protocol layer by providing only the prepared patch and no Pi tools. Remediation is intentionally writable through edit-only Pi tools; shell, Git and GitHub tools are excluded so the host retains delivery lifecycle control. + +## Traceability + +| Requirement | Design coverage | +| --- | --- | +| `REQ-01`, `REQ-05` | explicit backend/executable settings and default Codex path | +| `REQ-02` | private patch review with stateless no-tool Pi invocation | +| `REQ-03` | exact JSONL `message_end` and shared strict parser | +| `REQ-04` | fresh fix processes and host-owned lifecycle | +| `REQ-06`, `REQ-07` | adapter/config tests and README contract | diff --git a/memory-bank/features/FT-045/implementation-plan.md b/memory-bank/features/FT-045/implementation-plan.md new file mode 100644 index 0000000..e8c4a78 --- /dev/null +++ b/memory-bank/features/FT-045/implementation-plan.md @@ -0,0 +1,47 @@ +--- +title: "FT-045: Implementation Plan" +doc_kind: feature +doc_function: derived +purpose: "Implementation and validation plan for the explicit Pi backend." +derived_from: + - brief.md + - design.md + - ../../adr/ADR-003-agent-neutral-workflow-contract.md + - ../../../README.md +status: active +audience: humans_and_agents +must_not_define: + - ft_045_scope + - ft_045_selected_design + - ft_045_acceptance_criteria +--- + +# FT-045: Implementation Plan + +## Realization + +| Step | Result | Status | +| --- | --- | --- | +| `STEP-01` | Extract provider-neutral workflow contract into `internal/agent`; keep Codex behavior | complete | +| `STEP-02` | Add explicit `agent-backend` and `pi-executable` configuration and app selection | complete | +| `STEP-03` | Implement Pi review adapter: private patch prompt, no-tool stateless invocation and strict final JSONL message parsing | complete | +| `STEP-04` | Implement fresh Pi Fix findings/Fix CI invocations while retaining host workflow ownership | complete | +| `STEP-05` | Document the public contract and add focused config/app/Pi tests | complete | + +## Validation contract + +| Surface | Evidence | +| --- | --- | +| Neutral contract and Codex compatibility | compile checks for `internal/agent`, `internal/codex` and `internal/workflow` | +| Configuration / selection | `internal/config` and `internal/app` tests cover Pi selection and invalid configuration | +| Pi protocol | `internal/pi` tests cover a valid final assistant message, partial/missing/multiple carrier rejection and fix command selection | +| Documentation | `make docs-lint` and README link check | +| Source hygiene | `gofmt` and `git diff --check` | + +## Known local validation constraint + +Running Go test binaries on this machine can fail before test execution with macOS `dyld: missing LC_UUID load command` in temporary `go-build` outputs. Package compile checks (`go test -c`) are used locally until that machine/toolchain condition is repaired; published CI must run the full suite. + +## Delivery gate + +The requester authorized commit, push, pull request creation and review. Before merge, the published head requires independent review and required CI results. diff --git a/memory-bank/features/README.md b/memory-bank/features/README.md index a30ba48..d974ca6 100644 --- a/memory-bank/features/README.md +++ b/memory-bank/features/README.md @@ -51,3 +51,4 @@ audience: humans_and_agents - [`FT-038/README.md`](FT-038/README.md) — strict YAML configuration delivery for issue #38. - [`FT-039/README.md`](FT-039/README.md) — active deterministic repository publication and CI orchestration for issue #39. - [`FT-042/README.md`](FT-042/README.md) — active configurable document-review prompt delivery for issue #42. +- [`FT-045/README.md`](FT-045/README.md) — blocked opt-in Pi agent backend for issue #45; the high-risk Human Gate and Solution Ready design are accepted, while implementation awaits an approved restricted provider/credential/transport bridge.