diff --git a/README.md b/README.md index e379bd9..08f1358 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ flowchart TD E -- no --> X1[Exit 1] D --> K["Commit local checkpoint when fixes changed the clean worktree"] K --> B - C -- no --> N{Unfinalized changes or local checkpoint?} + C -- no --> N{Unpublished changes or local checkpoint?} N -- no --> X0[Exit 0: no-op] N -- yes --> F[Commit, push, create change request if needed, check applicable CI] F --> G{Result} @@ -117,64 +117,33 @@ flowchart TD └───────┬─────────┬────┘ │ yes │ no ▼ └────────► run_completed success, exit 0 - ╔══════════════════════╗ - ║ FINALIZE STAGE ║ - ║ ║ - ║ codex exec - ║ - ║ --output-schema ║ - ║ --output-last-msg ║ - ║ stdin: finalize ║ - ║ prompt ║ - ╚════════╤═════════════╝ - │ - ▼ - ┌────────────────────┐ - │ Parse JSON verdict│ - │ │ - │ {verdict, commit, │ - │ push, cr, ci} │ - └─────────┬──────────┘ - │ - ┌──────┼──────────┐ - │ │ │ - ▼ ▼ ▼ - SUCCESS CI_FAILED FAILED - │ │ │ - │ │ └──► exit 2 (operational_failure) - │ │ - │ recoveries < max? - │ ┌────┴────┐ - │ yes no ──► exit 3 (ci_failure) - │ │ - │ ▼ - │ ╔══════════════╗ - │ ║ FIX-CI ║ - │ ║ ║ - │ ║ codex exec - ║ - │ ║ stdin: ci ║ - │ ║ fix prompt ║ - │ ╚══════╤═══════╝ - │ │ - │ recoveries++ - │ phase++ - │ cycle=1, fixes=0 - │ │ - │ └──────► back to REVIEW - │ - ▼ - ╔═══════════════╗ - ║ run_completed ║ - ║ status=success║ - ║ exit_code=0 ║ - ╚═══════════════╝ + ╔══════════════════════════════════╗ + ║ PUBLISH (host git/gh processes) ║ + ║ commit → direct-ref push → PR ║ + ╚═══════════════╤══════════════════╝ + │ published HEAD SHA + ▼ + ╔══════════════════════════════════╗ + ║ CI (host GitHub check-run poll) ║ + ║ exact SHA; deadline = ci-timeout ║ + ╚═════╤═══════════╤═══════════╤══════╝ + │ │ │ + green/N/A failed timeout/error + │ │ │ + │ Fix CI? └──► exit 2 (ci_timeout/operational_failure) + │ │ + │ yes ──► Codex Fix CI → review phase + 1 + │ no ──► exit 3 (ci_failure) + ▼ + run_completed success, exit 0 ``` 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. -- **Finalize** — `codex exec --output-schema`, strict JSON verdict with hard validation. -- **CI recovery** — on `CI_FAILED`, fixes CI, resets the fix cycle, and restarts from Review. +- **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. @@ -196,31 +165,19 @@ When the review has findings, `code-converge` detects whether the Git worktree i fix findings ``` -The default `fast` profile uses `gpt-5.6-luna` with reasoning effort `medium`. Before and after the agent runs, `code-converge` records `HEAD` and checks Git status. If the fix changed an initially clean worktree, it stages the changes and creates one local commit with the stable message `chore: checkpoint review fixes`; it never pushes this checkpoint. A commit made directly by the agent is also detected as a local checkpoint, even when the worktree is clean, so a later clean review still reaches finalization. A dirty pre-fix worktree continues through remediation but skips the automatic checkpoint and reports that reason if the budget is later exhausted. A no-change fix attempts no empty commit. A status, `HEAD`, staging, commit, branch, or commit-ID failure is operational (exit `2`) and the workflow does not start another review. After a successful checkpoint decision or skip, the workflow returns to **Review**. +The default `fast` profile uses `gpt-5.6-luna` with reasoning effort `medium`. Before and after the agent runs, `code-converge` records `HEAD` and checks Git status. If the fix changed an initially clean worktree, it stages the changes and creates one local commit with the stable message `chore: checkpoint review fixes`; it never pushes this checkpoint. A commit made directly by the agent is also detected as a local checkpoint, even when the worktree is clean, so a later clean review still reaches publication. A dirty pre-fix worktree continues through remediation but skips the automatic checkpoint and reports that reason if the budget is later exhausted. A no-change fix attempts no empty commit. A status, `HEAD`, staging, commit, branch, or commit-ID failure is operational (exit `2`) and the workflow does not start another review. After a successful checkpoint decision or skip, the workflow returns to **Review**. -`max-cycles` is the maximum number of fix-findings attempts in one review phase; its built-in default is `10` and it must be non-negative. The initial review does not consume this budget. After the final allowed fix attempt, `code-converge` always performs one verification review. If that review still has findings, `code-converge` reports that the limit has been reached, that clean-review finalization was not reached, and the latest local checkpoint state before exiting with code `1`. A failed fix-findings command is an operational failure and exits with code `2`. +`max-cycles` is the maximum number of fix-findings attempts in one review phase; its built-in default is `10` and it must be non-negative. The initial review does not consume this budget. After the final allowed fix attempt, `code-converge` always performs one verification review. If that review still has findings, `code-converge` reports that the limit has been reached, that clean-review publication was not reached, and the latest local checkpoint state before exiting with code `1`. A failed fix-findings command is an operational failure and exits with code `2`. ### 3. Commit, push, create a change request, and check CI -Once a review returns no findings, `code-converge` checks Git status for staged, unstaged and untracked changes. If there are none and this run created no local checkpoints, it completes successfully as a no-op without starting finalization or attempting an empty commit. If changes exist, or this run created a local checkpoint, it asks Codex to finalize them. In the latter case the finalizer is told not to create an empty commit; it still pushes the current branch, creates a change request if needed, and verifies CI. The default prompt is: +After a clean review, Code Converge—not Codex—performs publication. It creates a commit only when the run began with a clean worktree; pre-existing dirty content is never committed automatically. It uses a direct Git refspec push, so a local remote-tracking-ref refresh cannot make a successful remote publication look failed. It then reuses exactly one matching open pull request or creates one; ambiguous identity is operational failure. -```text -commit, push, create PR, ensure CI is green -``` - -The default `fast` profile uses `gpt-5.6-luna` with reasoning effort `medium` for this stage. The final agent response must report exactly one of these states: - -| State | Meaning | Next action | -| --- | --- | --- | -| `SUCCESS` | Changes are committed and pushed; a change request was created when needed; required CI is green or CI is not applicable. | Exit `0`. | -| `CI_FAILED` | Publication succeeded, but applicable required CI is red. | Run **Fix CI**. | -| `FAILED` | Any other failure (for example, unable to commit, push, or create a PR). | Exit `2`. | - -In addition to the single verdict, the final response reports the outcome of `commit`, `push`, `change_request`, and `ci` so `code-converge` can emit the required step records. Missing or internally inconsistent details cannot be interpreted as success and cause an operational failure (`2`). +Code Converge polls every page of GitHub check-runs for the exact published `HEAD` SHA. The applicable set is the returned check-runs: no returned runs is `skipped`; `success`, `skipped`, and `neutral` terminal conclusions are accepted; the first other completed conclusion is `failed`; pending runs continue waiting. `--ci-timeout` / `CODE_CONVERGE_CI_TIMEOUT` / `.code-converge/ci-timeout` use normal precedence and default to `60m`. Timeout is an explicit operational `ci_timeout` outcome (exit `2`), not failed CI and never invokes Fix CI. Transient provider failures are retried inside the same deadline; authentication and authorization failures are operational. ### 4. Fix CI -When finalization reports `CI_FAILED`, `code-converge` starts Codex with the configured CI-fix prompt. This stage is skipped when the target repository has no applicable required CI. Its built-in prompt is: +When deterministic CI polling reports a failed applicable check, `code-converge` starts Codex with the configured CI-fix prompt. This stage is skipped when no applicable check-run exists. Its built-in prompt is: ```text Исправь CI @@ -235,8 +192,8 @@ If the agent completes successfully, the entire workflow begins again with a new | Code | Meaning | | --- | --- | | `0` | The review is clean and either no staged, unstaged or untracked changes exist, or changes are committed and pushed; a change request exists if needed; required CI is green or CI is not applicable. `update` also returns `0` when the installed version is current or the user declines the update. | -| `1` | Review findings remain after the configured maximum number of fix-findings attempts. The terminal record states that finalization was not reached and gives the latest local checkpoint outcome. | -| `2` | An operational/configuration failure occurred, review output was ambiguous, fix-findings failed, or finalization failed for a reason other than red CI. `update` uses it for unsupported hosts, invalid release metadata, download/checksum failures, or replacement/permission failures. | +| `1` | Review findings remain after the configured maximum number of fix-findings attempts. The terminal record states that publication was not reached and gives the latest local checkpoint outcome. | +| `2` | An operational/configuration failure occurred, review output was ambiguous, fix-findings failed, publication/provider failure occurred, or CI timed out. `update` uses it for unsupported hosts, invalid release metadata, download/checksum failures, or replacement/permission failures. | | `3` | The CI-fix stage failed or the maximum number of CI-recovery attempts was exhausted. | ## Logging and metrics @@ -260,9 +217,9 @@ The required event catalog is: | `run_started` | No fields beyond `ts` and `event`. | | `stage_started` | `stage`, `model`, `reasoning_effort`; also `review_phase` and `cycle` for `review` and `fix-findings`, and `review_phase` for `fix-ci`. | | `review_completed` | `stage=review`, `model`, `reasoning_effort`, `review_phase`, `cycle`, `status=clean\|findings\|failed`, and `duration_ms`. A classified result (`clean` or `findings`) also requires all findings counters plus `review_scope=branch_and_worktree`, `review_base` (resolved commit SHA), `review_merge_base` and `review_base_source=explicit\|open_pr\|branch_merge_base\|remote_default`; on command or classification failure these fields and counters are omitted. This is the review stage's sole completion record. | -| `stage_completed` | `stage=fix-findings\|finalize\|fix-ci`, `model`, `reasoning_effort`, `status=success\|failed`, and `duration_ms`; `fix-findings` also has `review_phase` and `cycle`, while `fix-ci` has `review_phase`. A successfully parsed finalization response also requires `verdict=SUCCESS\|CI_FAILED\|FAILED`; an invocation or parsing failure uses `status=failed` and omits `verdict`. | -| `step_completed` | `stage=finalize`, `model`, `reasoning_effort`, `step=commit\|push\|change_request\|ci`, and `status=success\|skipped\|failed\|unknown`. Each finalization attempt emits one record for every listed step; a step that is inapplicable or not reached is `skipped`, while an outcome that cannot be established is `unknown`. | -| `run_completed` | `status=success\|findings_remaining\|operational_failure\|ci_failure\|cancelled`, `exit_code`, and `total_duration_ms`. `cancelled` always has `exit_code=130`. For `findings_remaining`, also `checkpoint_status=committed_local\|no_changes\|not_attempted`; `committed_local` additionally requires percent-encoded `checkpoint_branch` and `checkpoint_commit`, while `not_attempted` requires `checkpoint_reason=fix_budget_exhausted\|pre_existing_changes`. | +| `stage_completed` | `stage=fix-findings\|publish\|ci\|fix-ci`, `status`, and `duration_ms`; Codex-backed stages also include model and reasoning effort. `ci` status is `success`, `skipped`, `failed`, or `timeout`; a CI timeout additionally has `timeout_ms`, the configured deadline. | +| `step_completed` | `stage=publish`, `step=commit\|push\|change_request`, and `status=success\|skipped\|failed\|unknown`; CI emits its own `stage=ci` step with `success\|skipped\|failed\|timeout`. | +| `run_completed` | `status=success\|findings_remaining\|operational_failure\|ci_timeout\|ci_failure\|cancelled`, `exit_code`, and `total_duration_ms`. `cancelled` always has `exit_code=130`; `ci_timeout` has `exit_code=2`. For `findings_remaining`, also `checkpoint_status=committed_local\|no_changes\|not_attempted`; `committed_local` additionally requires percent-encoded `checkpoint_branch` and `checkpoint_commit`, while `not_attempted` requires `checkpoint_reason=fix_budget_exhausted\|pre_existing_changes`. | For example: @@ -271,7 +228,7 @@ ts=2026-07-21T10:04:05Z event=stage_started stage=review model=gpt-5.6-sol reaso ts=2026-07-21T10:06:18Z event=review_completed stage=review model=gpt-5.6-sol reasoning_effort=medium review_phase=1 cycle=2 status=findings findings_total=3 findings_critical=0 findings_high=1 findings_medium=2 findings_low=0 findings_unknown=0 duration_ms=133000 ts=2026-07-21T10:06:19Z event=stage_started stage=fix-findings model=gpt-5.6-luna reasoning_effort=medium review_phase=1 cycle=2 ts=2026-07-21T10:10:42Z event=stage_completed stage=fix-findings model=gpt-5.6-luna reasoning_effort=medium review_phase=1 cycle=2 status=success duration_ms=263000 -ts=2026-07-21T10:12:00Z event=step_completed stage=finalize model=gpt-5.3-codex-spark reasoning_effort=agent-default step=change_request status=skipped +ts=2026-07-21T10:12:00Z event=step_completed stage=publish step=change_request status=skipped ``` ### Review metrics @@ -284,7 +241,7 @@ The review-completion record is emitted even when there are no findings, for exa ts=2026-07-21T10:12:09Z event=review_completed stage=review model=gpt-5.6-sol reasoning_effort=medium review_phase=1 cycle=3 status=clean findings_total=0 findings_critical=0 findings_high=0 findings_medium=0 findings_low=0 findings_unknown=0 duration_ms=87000 ``` -This makes the trend across cycles directly measurable without requiring it to be monotonic: the `findings_*` fields show how the number and severity change, while `duration_ms` measures the cost of each review, fix, finalization, and CI-fix stage. `run_completed` contains `status`, `exit_code`, and `total_duration_ms`. +This makes the trend across cycles directly measurable without requiring it to be monotonic: the `findings_*` fields show how the number and severity change, while `duration_ms` measures the cost of each review, fix, publication, CI, and CI-fix stage. `run_completed` contains `status`, `exit_code`, and `total_duration_ms`. ### Human format @@ -300,11 +257,9 @@ When diagnostic session logging is enabled and its record directory has been cre | Review has findings | `22:14:05 [2/10] [gpt-5.6-sol/high] Review: 3 findings [P0:0; P1:1; P2:2] (2m 13s)` | | Review fails | `22:14:05 [2/10] [gpt-5.6-sol/high] Review failed (2m 13s)` | | Fix findings starts / succeeds / fails | `22:14:05 [2/10] [gpt-5.6-luna/medium] Fixing findings` / `22:14:05 [2/10] [gpt-5.6-luna/medium] Findings fixed (4m 23s)` / `22:14:05 [2/10] [gpt-5.6-luna/medium] Fixing findings failed (4m 23s)` | -| Finalization starts | `22:14:05 [gpt-5.3-codex-spark/agent-default] Finalizing` | -| Finalization step | `22:14:05 [gpt-5.3-codex-spark/agent-default] Commit: done` (and equivalent step status) | -| Finalization succeeds | `22:14:05 [gpt-5.3-codex-spark/agent-default] Finalized successfully (42s)` | -| Finalization reports red CI | `22:14:05 [gpt-5.3-codex-spark/agent-default] Finalized, but CI is failing (42s)` | -| Finalization fails | `22:14:05 [gpt-5.3-codex-spark/agent-default] Finalization failed (42s)` | +| Publication starts / steps / succeeds | `22:14:05 Publishing` / `22:14:05 Push: done` / `22:14:05 Published (42s)` | +| CI starts / succeeds / is skipped | `22:14:05 Waiting for CI` / `22:14:05 CI passed (3m 2s)` / `22:14:05 CI skipped: no applicable checks (0s)` | +| CI fails / times out | `22:14:05 CI failed (42s)` / `22:14:05 CI timed out (60m)` | | CI recovery starts / succeeds / fails | `22:14:05 [1/3] [agent-default/agent-default] CI recovery` / `22:14:05 [1/3] [agent-default/agent-default] CI recovery fixed (1m 8s)` / `22:14:05 [1/3] [agent-default/agent-default] CI recovery failed (1m 8s)` | | Run succeeds | `22:14:05 Done (8m 45s)` | | Findings remain | `22:14:05 Stopped: review findings remain (8m 45s, exit 1)` | @@ -365,7 +320,6 @@ The `fast` and `best` modes select these operative stage profiles. `fast` is the | --- | --- | --- | --- | | Review | `gpt-5.6-terra`, `medium` | `gpt-5.6-sol`, `high` | Not applicable: independent quality judgment is the stage's primary purpose. | | Fix findings | `gpt-5.6-luna`, `medium` | `gpt-5.6-terra`, `high` | Findings involve architecture, security, migrations, concurrency, or several connected modules. | -| Finalize | `gpt-5.6-luna`, `medium` | `gpt-5.6-luna`, `medium` | Finalization requires diagnosing an unusual Git, change-request, or CI workflow; otherwise route CI failures to Fix CI. | | Fix CI | `gpt-5.6-luna`, `medium` | `gpt-5.6-terra`, `high` | The cause is not localized by logs, spans multiple components, or persists after a repair. | ### Options and defaults @@ -378,14 +332,12 @@ The `fast` and `best` modes select these operative stage profiles. `fast` is the | Mode | `--mode` | `CODE_CONVERGE_MODE` | `mode` | `fast` | | Maximum fix-findings attempts per review phase | `--max-cycles` | `CODE_CONVERGE_MAX_CYCLES` | `max-cycles` | `10` | | Maximum CI recoveries | `--max-ci-recoveries` | `CODE_CONVERGE_MAX_CI_RECOVERIES` | `max-ci-recoveries` | `3` | +| CI wait timeout | `--ci-timeout` | `CODE_CONVERGE_CI_TIMEOUT` | `ci-timeout` | `60m` | | Review model | `--review-model` | `CODE_CONVERGE_REVIEW_MODEL` | `review-model` | selected profile | | Review reasoning effort | `--review-reasoning-effort` | `CODE_CONVERGE_REVIEW_REASONING_EFFORT` | `review-reasoning-effort` | selected profile | | Fix-findings model | `--fix-model` | `CODE_CONVERGE_FIX_MODEL` | `fix-model` | selected profile | | Fix-findings reasoning effort | `--fix-reasoning-effort` | `CODE_CONVERGE_FIX_REASONING_EFFORT` | `fix-reasoning-effort` | selected profile | | Fix-findings prompt | `--fix-prompt-file` | `CODE_CONVERGE_FIX_PROMPT_FILE` | `fix-findings.md` | `fix findings` | -| Finalization model | `--finalize-model` | `CODE_CONVERGE_FINALIZE_MODEL` | `finalize-model` | selected profile | -| Finalization reasoning effort | `--finalize-reasoning-effort` | `CODE_CONVERGE_FINALIZE_REASONING_EFFORT` | `finalize-reasoning-effort` | selected profile | -| Finalization prompt | `--finalize-prompt-file` | `CODE_CONVERGE_FINALIZE_PROMPT_FILE` | `finalize.md` | `commit, push, create PR, ensure CI is green` | | CI-fix model | `--ci-fix-model` | `CODE_CONVERGE_CI_FIX_MODEL` | `ci-fix-model` | selected profile | | CI-fix reasoning effort | `--ci-fix-reasoning-effort` | `CODE_CONVERGE_CI_FIX_REASONING_EFFORT` | `ci-fix-reasoning-effort` | selected profile | | CI-fix prompt | `--ci-fix-prompt-file` | `CODE_CONVERGE_CI_FIX_PROMPT_FILE` | `fix-ci.md` | `Исправь CI` | @@ -394,6 +346,8 @@ The `fast` and `best` modes select these operative stage profiles. `fast` is the | Diagnostic session-log retention | `--session-log-retention` | `CODE_CONVERGE_SESSION_LOG_RETENTION` | `session-log-retention` | `24h` | | Disable diagnostic logging for this run | `--no-session-log` | — | — | disabled only when flag supplied | +`--finalize-model`, `--finalize-reasoning-effort`, and `--finalize-prompt-file`, their `CODE_CONVERGE_FINALIZE_*` environment variables, and `finalize-*` / `finalize.md` configuration files were removed in this release. Remove them during migration: obsolete environment or configuration-file settings cause an actionable configuration error rather than being ignored, because Codex no longer performs publication or CI polling. + For example, a team can commit these files: ```text @@ -407,16 +361,15 @@ For example, a team can commit these files: ├── review-base ├── fix-model ├── fix-reasoning-effort -├── finalize-model -├── finalize-reasoning-effort ├── ci-fix-model ├── ci-fix-reasoning-effort ├── max-cycles ├── max-ci-recoveries + +├── ci-timeout ├── session-log-dir ├── session-log-retention ├── fix-findings.md -├── finalize.md └── fix-ci.md ``` @@ -455,7 +408,7 @@ fix-prompt: .code-converge/fix-findings.md (project; built-in: "fix findings") - `codex` must be installed, authenticated, and available on `PATH` when running `code-converge`. - The authenticated account must have access to every model selected by the effective profile and any explicit stage overrides. - The target directory must be a Git repository. -- `git` and any tooling or credentials required by the target repository's chosen remote-hosting workflow must be available to the finalization agent. No hosting provider is required by `code-converge`; provider-specific tooling is needed only when the selected finalization actions depend on it. +- `git`, `gh`, and GitHub credentials must be available to the Code Converge host process for deterministic publication and CI polling. ## Build and install diff --git a/internal/app/app.go b/internal/app/app.go index 00173be..f88261e 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -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) }}, diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 30078b3..bde2163 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -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) { @@ -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": @@ -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": @@ -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) } @@ -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 { @@ -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 { @@ -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 { @@ -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{ @@ -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] @@ -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") { @@ -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 { @@ -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, diff --git a/internal/codex/adapter.go b/internal/codex/adapter.go index 1e9f5e8..09c5112 100644 --- a/internal/codex/adapter.go +++ b/internal/codex/adapter.go @@ -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 @@ -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 @@ -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 } @@ -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, @@ -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"]} - } -}` diff --git a/internal/codex/adapter_test.go b/internal/codex/adapter_test.go index de6d8ae..7659d1f 100644 --- a/internal/codex/adapter_test.go +++ b/internal/codex/adapter_test.go @@ -21,7 +21,6 @@ import ( const ( cleanReviewJSON = `{"findings":[],"overall_correctness":"patch is correct","overall_explanation":"No changes to review.","overall_confidence_score":0.99}` findingsReviewJSON = `{"findings":[{"title":"[P0] critical","body":"body","confidence_score":0.9,"priority":0,"code_location":{"absolute_file_path":"/tmp/a.go","line_range":{"start":1,"end":1}}},{"title":"[P1] high","body":"body","confidence_score":0.8,"priority":1,"code_location":{"absolute_file_path":"/tmp/b.go","line_range":{"start":2,"end":2}}},{"title":"[P2] medium","body":"body","confidence_score":0.7,"priority":2,"code_location":{"absolute_file_path":"/tmp/c.go","line_range":{"start":3,"end":3}}},{"title":"[P3] low","body":"body","confidence_score":0.6,"priority":3,"code_location":{"absolute_file_path":"/tmp/d.go","line_range":{"start":4,"end":4}}}],"overall_correctness":"patch is incorrect","overall_explanation":"findings","overall_confidence_score":0.8}` - finalizationJSON = `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}` ) func TestParseStructuredReview(t *testing.T) { @@ -56,38 +55,6 @@ func TestParseStructuredReview(t *testing.T) { } } -func TestParseFinalization(t *testing.T) { - valid := []Finalization{ - {Verdict: "SUCCESS", Commit: "success", Push: "success", ChangeRequest: "success", CI: "success"}, - {Verdict: "SUCCESS", Commit: "success", Push: "success", ChangeRequest: "skipped", CI: "skipped"}, - {Verdict: "SUCCESS", Commit: "skipped", Push: "skipped", ChangeRequest: "skipped", CI: "skipped"}, - {Verdict: "CI_FAILED", Commit: "success", Push: "success", ChangeRequest: "skipped", CI: "failed"}, - {Verdict: "FAILED", Commit: "failed", Push: "skipped", ChangeRequest: "skipped", CI: "skipped"}, - } - for _, value := range valid { - data, _ := json.Marshal(value) - if _, err := ParseFinalization(data); err != nil { - t.Errorf("valid result %#v rejected: %v", value, err) - } - } - invalid := []string{ - `{}`, - `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"failed"}`, - `{"verdict":"CI_FAILED","commit":"success","push":"success","change_request":"skipped","ci":"success"}`, - `{"verdict":"FAILED","commit":"success","push":"success","change_request":"skipped","ci":"failed"}`, - `{"verdict":"FAILED","commit":"success","push":"success","change_request":"skipped","ci":"success"}`, - `{"verdict":"FAILED","commit":"skipped","push":"skipped","change_request":"skipped","ci":"skipped"}`, - `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped","extra":true}`, - `{"verdict":"FAILED","verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"}`, - `{"verdict":"SUCCESS","commit":"success","push":"success","change_request":"skipped","ci":"skipped"} trailing`, - } - for _, data := range invalid { - if _, err := ParseFinalization([]byte(data)); err == nil { - t.Errorf("invalid result accepted: %s", data) - } - } -} - type recordingRunner struct { invocations []runner.Invocation codexResult runner.Result @@ -149,8 +116,6 @@ func (r *recordingRunner) Run(_ context.Context, invocation runner.Invocation) ( if r.writeReview { _ = os.WriteFile(messagePath, r.reviewMessage, 0o600) } - } else { - _ = os.WriteFile(messagePath, []byte(finalizationJSON), 0o600) } } return r.codexResult, r.codexErr @@ -192,7 +157,6 @@ func TestAdapterInvocations(t *testing.T) { } a := newReviewAdapter(t, r, config.Config{ ReviewModel: "gpt-5.6-sol", ReviewEffort: "high", FixModel: "gpt-5.6-terra", FixEffort: "high", FixPrompt: "fix it", - FinalizeModel: "gpt-5.6-luna", FinalizeEffort: "medium", FinalizePrompt: "finalize", CIFixModel: "gpt-5.6-terra", CIFixEffort: "high", CIFixPrompt: "ci", }) result, err := a.Review(context.Background()) @@ -202,14 +166,11 @@ func TestAdapterInvocations(t *testing.T) { if err := a.FixFindings(context.Background(), result.Report); err != nil { t.Fatal(err) } - if _, err := a.Finalize(context.Background(), false); err != nil { - t.Fatal(err) - } if err := a.FixCI(context.Background()); err != nil { t.Fatal(err) } invocations := codexInvocations(r.invocations) - wantPairs := []struct{ model, effort string }{{"gpt-5.6-sol", "high"}, {"gpt-5.6-terra", "high"}, {"gpt-5.6-luna", "medium"}, {"gpt-5.6-terra", "high"}} + wantPairs := []struct{ model, effort string }{{"gpt-5.6-sol", "high"}, {"gpt-5.6-terra", "high"}, {"gpt-5.6-terra", "high"}} for i, want := range wantPairs { got := strings.Join(invocations[i].Args, " ") if !strings.Contains(got, `model="`+want.model+`"`) || !strings.Contains(got, `model_reasoning_effort="`+want.effort+`"`) { @@ -219,12 +180,11 @@ func TestAdapterInvocations(t *testing.T) { if got := strings.Join(invocations[0].Args, " "); !strings.Contains(got, " exec --output-schema ") || !strings.Contains(got, " --output-last-message ") || !strings.HasSuffix(got, " -") { t.Errorf("review args = %s", got) } - if invocations[1].Stdin != "fix it\n\nReview findings to address:\n\n"+findingsReviewJSON || invocations[3].Stdin != "ci" { + if invocations[1].Stdin != "fix it\n\nReview findings to address:\n\n"+findingsReviewJSON || invocations[2].Stdin != "ci" { t.Errorf("prompts not passed through: %#v", invocations) } - finalArgs := strings.Join(invocations[2].Args, " ") - if !strings.Contains(finalArgs, "--output-schema") || !strings.Contains(finalArgs, "--output-last-message") { - t.Errorf("finalization args = %s", finalArgs) + if got := strings.Join(invocations[2].Args, " "); strings.Contains(got, "--output-schema") || strings.Contains(got, "--output-last-message") { + t.Errorf("CI-fix invocation unexpectedly has a finalization schema: %s", got) } if r.reviewDirMode != 0o700 || r.reviewSchemaMode != 0o600 { t.Errorf("review workspace modes = dir %o schema %o", r.reviewDirMode, r.reviewSchemaMode) @@ -320,19 +280,6 @@ func TestScopedReviewArgsUseTOMLCompatibleEnvironmentEncoding(t *testing.T) { } } -func TestFinalizationSchemaIsStrictJSON(t *testing.T) { - var schema map[string]any - if err := json.Unmarshal([]byte(finalizationSchema), &schema); err != nil { - t.Fatal(err) - } - if schema["additionalProperties"] != false { - t.Fatalf("schema is not strict: %#v", schema) - } - if filepath.Ext("schema.json") != ".json" { // keep filepath import exercised on every supported OS - t.Fatal("unexpected filepath behavior") - } -} - func TestReviewSchemaIsStrictJSON(t *testing.T) { var schema map[string]any if err := json.Unmarshal([]byte(reviewSchema), &schema); err != nil { @@ -686,35 +633,6 @@ func TestFixCIWithModel(t *testing.T) { } } -func TestFinalizeReadMessageError(t *testing.T) { - r := &codexFakeRunner{result: runner.Result{}, writeFile: false} - a := Adapter{Runner: r, Config: config.Config{FinalizeModel: "m", FinalizePrompt: "p"}} - _, err := a.Finalize(context.Background(), false) - if err == nil || !strings.Contains(err.Error(), "read finalization response") { - t.Fatalf("error = %v", err) - } -} - -func TestFinalizeParseError(t *testing.T) { - r := &codexFakeRunner{result: runner.Result{}, writeFile: true, writeBytes: []byte(`not json`)} - a := Adapter{Runner: r, Config: config.Config{FinalizeModel: "m", FinalizePrompt: "p"}} - _, err := a.Finalize(context.Background(), false) - if err == nil { - t.Fatal("expected parse error") - } -} - -func TestFinalizeCheckpointPromptSkipsEmptyCommit(t *testing.T) { - r := &codexFakeRunner{result: runner.Result{}, writeFile: true, writeBytes: []byte(`{"verdict":"SUCCESS","commit":"skipped","push":"success","change_request":"skipped","ci":"skipped"}`)} - a := Adapter{Runner: r, Config: config.Config{FinalizeModel: "m", FinalizePrompt: "finalize"}} - if _, err := a.Finalize(context.Background(), true); err != nil { - t.Fatal(err) - } - if got := r.invocation.Stdin; !strings.Contains(got, "already committed as local checkpoints") || !strings.Contains(got, "Do not create an empty commit") { - t.Fatalf("checkpoint finalization prompt = %q", got) - } -} - func TestRejectDuplicateJSONKeysNestedCases(t *testing.T) { valid := []string{ `{"verdict":"SUCCESS","nested":{"a":1,"b":[1,2,{"c":3}]}}`, @@ -737,19 +655,3 @@ func TestRejectDuplicateJSONKeysNestedCases(t *testing.T) { } } } - -func TestValidateFinalizationEdgeCases(t *testing.T) { - invalid := []Finalization{ - {Verdict: "SUCCESS", Commit: "success", Push: "success", ChangeRequest: "success", CI: "ok"}, - {Verdict: "UNKNOWN", Commit: "success", Push: "success", ChangeRequest: "success", CI: "success"}, - {Verdict: "FAILED", Commit: "success", Push: "success", ChangeRequest: "skipped", CI: "success"}, - {Verdict: "FAILED", Commit: "success", Push: "success", ChangeRequest: "skipped", CI: "failed"}, - {Verdict: "FAILED", Commit: "skipped", Push: "skipped", ChangeRequest: "skipped", CI: "skipped"}, - } - for _, value := range invalid { - data, _ := json.Marshal(value) - if _, err := ParseFinalization(data); err == nil { - t.Errorf("invalid result accepted: %#v", value) - } - } -} diff --git a/internal/config/config.go b/internal/config/config.go index f372b9e..314da7c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -30,14 +30,12 @@ type Overrides struct { Mode OptionalString MaxCycles OptionalString MaxCIRecoveries OptionalString + CITimeout OptionalString ReviewModel OptionalString ReviewEffort OptionalString FixModel OptionalString FixEffort OptionalString FixPromptPath OptionalString - FinalizeModel OptionalString - FinalizeEffort OptionalString - FinalizePromptPath OptionalString CIFixModel OptionalString CIFixEffort OptionalString CIFixPromptPath OptionalString @@ -65,14 +63,12 @@ type Config struct { Mode string MaxCycles int MaxCIRecoveries int + CITimeout time.Duration ReviewModel string ReviewEffort string FixModel string FixEffort string FixPrompt string - FinalizeModel string - FinalizeEffort string - FinalizePrompt string CIFixModel string CIFixEffort string CIFixPrompt string @@ -96,10 +92,9 @@ type spec struct { } type stageProfile struct { - reviewModel, reviewEffort string - fixModel, fixEffort string - finalizeModel, finalizeEffort string - ciFixModel, ciFixEffort string + reviewModel, reviewEffort string + fixModel, fixEffort string + ciFixModel, ciFixEffort string } func profileFor(mode string) (stageProfile, bool) { @@ -108,14 +103,12 @@ func profileFor(mode string) (stageProfile, bool) { return stageProfile{ reviewModel: "gpt-5.6-terra", reviewEffort: "medium", fixModel: "gpt-5.6-luna", fixEffort: "medium", - finalizeModel: "gpt-5.6-luna", finalizeEffort: "medium", ciFixModel: "gpt-5.6-luna", ciFixEffort: "medium", }, true case "best": return stageProfile{ reviewModel: "gpt-5.6-sol", reviewEffort: "high", fixModel: "gpt-5.6-terra", fixEffort: "high", - finalizeModel: "gpt-5.6-luna", finalizeEffort: "medium", ciFixModel: "gpt-5.6-terra", ciFixEffort: "high", }, true default: @@ -136,6 +129,9 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { } projectDir := filepath.Join(root, ".code-converge") userDir := filepath.Join(home, ".code-converge") + if err := rejectObsoleteFinalizeSettings(userDir, projectDir); err != nil { + return Config{}, err + } logFormat, logFormatSetting, err := resolve(spec{ name: "log-format", file: "log-format", env: "CODE_CONVERGE_LOG_FORMAT", def: "human", builtIn: "human", defSource: SourceDefault, override: overrides.LogFormat, }, cwd, userDir, projectDir) @@ -182,14 +178,12 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { specs := []spec{ {name: "max-cycles", file: "max-cycles", env: "CODE_CONVERGE_MAX_CYCLES", def: "10", builtIn: "10", defSource: SourceDefault, override: overrides.MaxCycles}, {name: "max-ci-recoveries", file: "max-ci-recoveries", env: "CODE_CONVERGE_MAX_CI_RECOVERIES", def: "3", builtIn: "3", defSource: SourceDefault, override: overrides.MaxCIRecoveries}, + {name: "ci-timeout", file: "ci-timeout", env: "CODE_CONVERGE_CI_TIMEOUT", def: "60m", builtIn: "60m", defSource: SourceDefault, override: overrides.CITimeout}, {name: "review-model", file: "review-model", env: "CODE_CONVERGE_REVIEW_MODEL", def: profile.reviewModel, builtIn: fast.reviewModel, defSource: profileSource, override: overrides.ReviewModel}, {name: "review-reasoning-effort", file: "review-reasoning-effort", env: "CODE_CONVERGE_REVIEW_REASONING_EFFORT", def: profile.reviewEffort, builtIn: fast.reviewEffort, defSource: profileSource, override: overrides.ReviewEffort}, {name: "fix-model", file: "fix-model", env: "CODE_CONVERGE_FIX_MODEL", def: profile.fixModel, builtIn: fast.fixModel, defSource: profileSource, override: overrides.FixModel}, {name: "fix-reasoning-effort", file: "fix-reasoning-effort", env: "CODE_CONVERGE_FIX_REASONING_EFFORT", def: profile.fixEffort, builtIn: fast.fixEffort, defSource: profileSource, override: overrides.FixEffort}, {name: "fix-prompt", file: "fix-findings.md", env: "CODE_CONVERGE_FIX_PROMPT_FILE", def: "fix findings", builtIn: "fix findings", defSource: SourceDefault, override: overrides.FixPromptPath, promptFile: true}, - {name: "finalize-model", file: "finalize-model", env: "CODE_CONVERGE_FINALIZE_MODEL", def: profile.finalizeModel, builtIn: fast.finalizeModel, defSource: profileSource, override: overrides.FinalizeModel}, - {name: "finalize-reasoning-effort", file: "finalize-reasoning-effort", env: "CODE_CONVERGE_FINALIZE_REASONING_EFFORT", def: profile.finalizeEffort, builtIn: fast.finalizeEffort, defSource: profileSource, override: overrides.FinalizeEffort}, - {name: "finalize-prompt", file: "finalize.md", env: "CODE_CONVERGE_FINALIZE_PROMPT_FILE", def: "commit, push, create PR, ensure CI is green", builtIn: "commit, push, create PR, ensure CI is green", defSource: SourceDefault, override: overrides.FinalizePromptPath, promptFile: true}, {name: "ci-fix-model", file: "ci-fix-model", env: "CODE_CONVERGE_CI_FIX_MODEL", def: profile.ciFixModel, builtIn: fast.ciFixModel, defSource: profileSource, override: overrides.CIFixModel}, {name: "ci-fix-reasoning-effort", file: "ci-fix-reasoning-effort", env: "CODE_CONVERGE_CI_FIX_REASONING_EFFORT", def: profile.ciFixEffort, builtIn: fast.ciFixEffort, defSource: profileSource, override: overrides.CIFixEffort}, {name: "ci-fix-prompt", file: "fix-ci.md", env: "CODE_CONVERGE_CI_FIX_PROMPT_FILE", def: "Исправь CI", builtIn: "Исправь CI", defSource: SourceDefault, override: overrides.CIFixPromptPath, promptFile: true}, @@ -222,6 +216,10 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { if err != nil { return Config{}, err } + ciTimeout, err := time.ParseDuration(strings.TrimSpace(values["ci-timeout"])) + if err != nil || ciTimeout < time.Second { + return Config{}, fmt.Errorf("ci-timeout must be a duration of at least 1s") + } sessionLogDir, err := sessionLogPath(values["session-log-dir"], home) if err != nil { return Config{}, err @@ -239,7 +237,7 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { settings[index].DisplayDefault = settings[index].Default } } - for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "finalize-model", "finalize-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { + for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { if strings.TrimSpace(values[name]) == "" { return Config{}, fmt.Errorf("%s must not be empty", name) } @@ -247,15 +245,43 @@ func Load(cwd, home string, overrides Overrides) (Config, error) { return Config{ Root: root, LogFormat: logFormat, Heartbeat: heartbeat, Color: color, - Mode: mode, MaxCycles: maxCycles, MaxCIRecoveries: maxCI, + 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"], - FinalizeModel: values["finalize-model"], FinalizeEffort: values["finalize-reasoning-effort"], FinalizePrompt: values["finalize-prompt"], CIFixModel: values["ci-fix-model"], CIFixEffort: values["ci-fix-reasoning-effort"], CIFixPrompt: values["ci-fix-prompt"], Settings: settings, ReviewBase: values["review-base"], SessionLogDir: sessionLogDir, SessionLogRetention: sessionLogRetention, NoSessionLog: overrides.NoSessionLog, }, nil } +// rejectObsoleteFinalizeSettings makes the deliberate Finalize-stage removal +// actionable. Leaving a previously supported setting silently ignored would +// make an operator believe it still controls delivery behavior. +func rejectObsoleteFinalizeSettings(userDir, projectDir string) error { + for _, name := range []string{ + "CODE_CONVERGE_FINALIZE_MODEL", + "CODE_CONVERGE_FINALIZE_REASONING_EFFORT", + "CODE_CONVERGE_FINALIZE_PROMPT_FILE", + } { + if value, ok := os.LookupEnv(name); ok && strings.TrimSpace(value) != "" { + return fmt.Errorf("%s was removed; remove this obsolete Finalize-stage setting", name) + } + } + for _, directory := range []struct { + path string + source string + }{{userDir, "user"}, {projectDir, "project"}} { + for _, name := range []string{"finalize-model", "finalize-reasoning-effort", "finalize.md"} { + path := filepath.Join(directory.path, name) + if _, err := os.Stat(path); err == nil { + return fmt.Errorf("%s Finalize-stage setting %q was removed; delete it", directory.source, path) + } else if !os.IsNotExist(err) { + return fmt.Errorf("inspect obsolete Finalize-stage setting %q: %w", path, err) + } + } + } + return nil +} + func sessionLogPath(value, home string) (string, error) { value = strings.TrimSpace(value) if value == "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f126e89..29d10cb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -29,10 +29,10 @@ func clearGitRepositoryEnvironment() { var codeConvergeEnv = []string{ "CODE_CONVERGE_LOG_FORMAT", "CODE_CONVERGE_HEARTBEAT", "CODE_CONVERGE_COLOR", "CODE_CONVERGE_MODE", - "CODE_CONVERGE_MAX_CYCLES", "CODE_CONVERGE_MAX_CI_RECOVERIES", "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", + "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_CI_FIX_MODEL", "CODE_CONVERGE_CI_FIX_REASONING_EFFORT", "CODE_CONVERGE_CI_FIX_PROMPT_FILE", + "CODE_CONVERGE_FINALIZE_MODEL", "CODE_CONVERGE_FINALIZE_REASONING_EFFORT", "CODE_CONVERGE_FINALIZE_PROMPT_FILE", "CODE_CONVERGE_REVIEW_BASE", "CODE_CONVERGE_SESSION_LOG_DIR", "CODE_CONVERGE_SESSION_LOG_RETENTION", } @@ -56,6 +56,81 @@ func TestLoggingConfiguration(t *testing.T) { } } +func TestCITimeoutPrecedenceAndValidation(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + t.Setenv("CODE_CONVERGE_CI_TIMEOUT", "20m") + write(t, filepath.Join(home, ".code-converge", "ci-timeout"), "30m") + write(t, filepath.Join(root, ".code-converge", "ci-timeout"), "40m") + cfg, err := Load(root, home, Overrides{CITimeout: OptionalString{Value: "50m", Set: true}}) + if err != nil || cfg.CITimeout != 50*time.Minute || source(cfg, "ci-timeout") != SourceCLI { + t.Fatalf("ci timeout = %s (%s), %v", cfg.CITimeout, source(cfg, "ci-timeout"), err) + } + if _, err := Load(root, home, Overrides{CITimeout: OptionalString{Value: "0s", Set: true}}); err == nil { + t.Fatal("accepted invalid ci timeout") + } +} + +func TestCITimeoutSourcePrecedence(t *testing.T) { + for _, test := range []struct { + name string + env string + user string + project string + override OptionalString + want time.Duration + wantSource string + }{ + {"built-in default", "", "", "", OptionalString{}, 60 * time.Minute, SourceDefault}, + {"environment", "20m", "", "", OptionalString{}, 20 * time.Minute, SourceEnv}, + {"user", "20m", "30m", "", OptionalString{}, 30 * time.Minute, SourceUser}, + {"project", "20m", "30m", "40m", OptionalString{}, 40 * time.Minute, SourceProject}, + {"CLI", "20m", "30m", "40m", OptionalString{Value: "50m", Set: true}, 50 * time.Minute, SourceCLI}, + } { + t.Run(test.name, func(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + if test.env != "" { + t.Setenv("CODE_CONVERGE_CI_TIMEOUT", test.env) + } + if test.user != "" { + write(t, filepath.Join(home, ".code-converge", "ci-timeout"), test.user) + } + if test.project != "" { + write(t, filepath.Join(root, ".code-converge", "ci-timeout"), test.project) + } + cfg, err := Load(root, home, Overrides{CITimeout: test.override}) + if err != nil || cfg.CITimeout != test.want || source(cfg, "ci-timeout") != test.wantSource { + t.Fatalf("ci-timeout=%s (%s), err=%v; want %s (%s)", cfg.CITimeout, source(cfg, "ci-timeout"), err, test.want, test.wantSource) + } + }) + } +} + +func TestObsoleteFinalizeSettingsFailExplicitly(t *testing.T) { + for _, test := range []struct { + name string + set func(t *testing.T, root, home string) + }{ + {"environment", func(t *testing.T, _, _ string) { t.Setenv("CODE_CONVERGE_FINALIZE_MODEL", "gpt-legacy") }}, + {"user file", func(t *testing.T, _, home string) { + write(t, filepath.Join(home, ".code-converge", "finalize-reasoning-effort"), "medium") + }}, + {"project prompt", func(t *testing.T, root, _ string) { + write(t, filepath.Join(root, ".code-converge", "finalize.md"), "publish") + }}, + } { + t.Run(test.name, func(t *testing.T) { + cleanEnv(t) + root, home := repo(t) + test.set(t, root, home) + if _, err := Load(root, home, Overrides{}); err == nil || !strings.Contains(err.Error(), "Finalize-stage setting") { + t.Fatalf("Load obsolete setting error = %v", err) + } + }) + } +} + func TestLoggingConfigurationPrecedence(t *testing.T) { cleanEnv(t) root, home := repo(t) @@ -282,11 +357,11 @@ func TestProfileResolution(t *testing.T) { }{ { name: "default fast", wantMode: "fast", - want: []string{"gpt-5.6-terra", "medium", "gpt-5.6-luna", "medium", "gpt-5.6-luna", "medium", "gpt-5.6-luna", "medium"}, + want: []string{"gpt-5.6-terra", "medium", "gpt-5.6-luna", "medium", "gpt-5.6-luna", "medium"}, }, { name: "explicit best", overrides: Overrides{Mode: OptionalString{Value: "best", Set: true}}, wantMode: "best", - want: []string{"gpt-5.6-sol", "high", "gpt-5.6-terra", "high", "gpt-5.6-luna", "medium", "gpt-5.6-terra", "high"}, + want: []string{"gpt-5.6-sol", "high", "gpt-5.6-terra", "high", "gpt-5.6-terra", "high"}, }, } for _, test := range tests { @@ -297,11 +372,11 @@ func TestProfileResolution(t *testing.T) { if err != nil { t.Fatal(err) } - got := []string{cfg.ReviewModel, cfg.ReviewEffort, cfg.FixModel, cfg.FixEffort, cfg.FinalizeModel, cfg.FinalizeEffort, cfg.CIFixModel, cfg.CIFixEffort} + got := []string{cfg.ReviewModel, cfg.ReviewEffort, cfg.FixModel, cfg.FixEffort, cfg.CIFixModel, cfg.CIFixEffort} if cfg.Mode != test.wantMode || strings.Join(got, "|") != strings.Join(test.want, "|") { t.Fatalf("mode/profile = %s %q, want %s %q", cfg.Mode, got, test.wantMode, test.want) } - for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "finalize-model", "finalize-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { + for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { if gotSource := source(cfg, name); gotSource != test.wantMode+" profile" { t.Errorf("%s source = %q", name, gotSource) } @@ -360,8 +435,6 @@ func TestEveryStageOverrideSourceBeatsProfile(t *testing.T) { {"review-reasoning-effort", "CODE_CONVERGE_REVIEW_REASONING_EFFORT", func(o *Overrides, v string) { o.ReviewEffort = OptionalString{v, true} }, func(c Config) string { return c.ReviewEffort }}, {"fix-model", "CODE_CONVERGE_FIX_MODEL", func(o *Overrides, v string) { o.FixModel = OptionalString{v, true} }, func(c Config) string { return c.FixModel }}, {"fix-reasoning-effort", "CODE_CONVERGE_FIX_REASONING_EFFORT", func(o *Overrides, v string) { o.FixEffort = OptionalString{v, true} }, func(c Config) string { return c.FixEffort }}, - {"finalize-model", "CODE_CONVERGE_FINALIZE_MODEL", func(o *Overrides, v string) { o.FinalizeModel = OptionalString{v, true} }, func(c Config) string { return c.FinalizeModel }}, - {"finalize-reasoning-effort", "CODE_CONVERGE_FINALIZE_REASONING_EFFORT", func(o *Overrides, v string) { o.FinalizeEffort = OptionalString{v, true} }, func(c Config) string { return c.FinalizeEffort }}, {"ci-fix-model", "CODE_CONVERGE_CI_FIX_MODEL", func(o *Overrides, v string) { o.CIFixModel = OptionalString{v, true} }, func(c Config) string { return c.CIFixModel }}, {"ci-fix-reasoning-effort", "CODE_CONVERGE_CI_FIX_REASONING_EFFORT", func(o *Overrides, v string) { o.CIFixEffort = OptionalString{v, true} }, func(c Config) string { return c.CIFixEffort }}, } @@ -471,7 +544,7 @@ func source(cfg Config, name string) string { } func TestLoadEmptyStageSettingValidation(t *testing.T) { - for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "finalize-model", "finalize-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { + for _, name := range []string{"review-model", "review-reasoning-effort", "fix-model", "fix-reasoning-effort", "ci-fix-model", "ci-fix-reasoning-effort"} { t.Run(name, func(t *testing.T) { cleanEnv(t) root, home := repo(t) @@ -488,11 +561,9 @@ func TestResolveFileReadError(t *testing.T) { cleanEnv(t) root, home := repo(t) path := filepath.Join(home, ".code-converge", "max-cycles") - write(t, path, "5\n") - if err := os.Chmod(path, 0o000); err != nil { + if err := os.MkdirAll(path, 0o700); err != nil { t.Fatal(err) } - defer os.Chmod(path, 0o600) _, err := Load(root, home, Overrides{}) if err == nil { t.Fatal("expected read error") @@ -543,7 +614,6 @@ func TestFormatProfileAndEqualExplicitSources(t *testing.T) { "mode: best (cli; built-in: fast)", "review-model: gpt-5.6-terra (cli)", "fix-model: gpt-5.6-terra (best profile; built-in: gpt-5.6-luna)", - "finalize-model: gpt-5.6-luna (best profile)", } { if !strings.Contains(formatted, want) { t.Errorf("missing %q in:\n%s", want, formatted) diff --git a/internal/event/event.go b/internal/event/event.go index d3151d0..0602bfa 100644 --- a/internal/event/event.go +++ b/internal/event/event.go @@ -492,8 +492,10 @@ func renderHuman(eventName string, fields []Field, maxCycles, maxCIRecoveries in return "Review started", nil case "fix-findings": return "Fixing findings", nil - case "finalize": - return "Finalizing", nil + case "publish": + return "Publishing", nil + case "ci": + return "Waiting for CI", nil case "fix-ci": return "CI recovery", nil } @@ -563,23 +565,32 @@ func renderHuman(eventName string, fields []Field, maxCycles, maxCIRecoveries in case "failed": return fmt.Sprintf("CI recovery failed (%s)", d), nil } - case "finalize": - switch values["verdict"] { - case "SUCCESS": - return fmt.Sprintf("Finalized successfully (%s)", d), nil - case "CI_FAILED": - return fmt.Sprintf("Finalized, but CI is failing (%s)", d), nil - case "FAILED": - return fmt.Sprintf("Finalization failed (%s)", d), nil - case "": - if values["status"] == "failed" { - return fmt.Sprintf("Finalization failed (%s)", d), nil + case "publish": + if values["status"] == "success" { + return fmt.Sprintf("Published (%s)", d), nil + } + if values["status"] == "failed" { + return fmt.Sprintf("Publication failed (%s)", d), nil + } + case "ci": + switch values["status"] { + case "success": + return fmt.Sprintf("CI passed (%s)", d), nil + case "skipped": + return fmt.Sprintf("CI skipped: no applicable checks (%s)", d), nil + case "failed": + return fmt.Sprintf("CI failed (%s)", d), nil + case "timeout": + limit, err := duration("timeout_ms") + if err != nil { + return "", err } + return fmt.Sprintf("CI timed out after %s (limit %s)", d, limit), nil } } case "step_completed": labels := map[string]string{"commit": "Commit", "push": "Push", "change_request": "Change request", "ci": "CI"} - statuses := map[string]string{"success": "done", "skipped": "not needed", "failed": "failed", "unknown": "unknown"} + statuses := map[string]string{"success": "done", "skipped": "not needed", "failed": "failed", "timeout": "timed out", "unknown": "unknown"} label, labelOK := labels[values["step"]] status, statusOK := statuses[values["status"]] if !labelOK || !statusOK { @@ -601,17 +612,17 @@ func renderHuman(eventName string, fields []Field, maxCycles, maxCIRecoveries in if err != nil { return "", fmt.Errorf("decode checkpoint_branch: %w", err) } - return fmt.Sprintf("Stopped: fix budget exhausted; finalization was not reached; checkpoint committed locally on %s at %s and not pushed (%s, exit 1)", branch, values["checkpoint_commit"], d), nil + return fmt.Sprintf("Stopped: fix budget exhausted; publication was not reached; checkpoint committed locally on %s at %s and not pushed (%s, exit 1)", branch, values["checkpoint_commit"], d), nil case "no_changes": - return fmt.Sprintf("Stopped: fix budget exhausted; finalization was not reached; no checkpoint was needed (%s, exit 1)", d), nil + return fmt.Sprintf("Stopped: fix budget exhausted; publication was not reached; no checkpoint was needed (%s, exit 1)", d), nil case "not_attempted": switch values["checkpoint_reason"] { case "pre_existing_changes": - return fmt.Sprintf("Stopped: fix budget exhausted; finalization was not reached; checkpoint was skipped because the worktree had pre-existing changes (%s, exit 1)", d), nil + return fmt.Sprintf("Stopped: fix budget exhausted; publication was not reached; checkpoint was skipped because the worktree had pre-existing changes (%s, exit 1)", d), nil case "fix_budget_exhausted": - return fmt.Sprintf("Stopped: fix budget exhausted; finalization was not reached; checkpoint was not attempted because no fix ran (%s, exit 1)", d), nil + return fmt.Sprintf("Stopped: fix budget exhausted; publication was not reached; checkpoint was not attempted because no fix ran (%s, exit 1)", d), nil default: - return fmt.Sprintf("Stopped: fix budget exhausted; finalization was not reached; checkpoint was not attempted (%s, exit 1)", d), nil + return fmt.Sprintf("Stopped: fix budget exhausted; publication was not reached; checkpoint was not attempted (%s, exit 1)", d), nil } default: return fmt.Sprintf("Stopped: review findings remain (%s, exit 1)", d), nil @@ -622,6 +633,8 @@ func renderHuman(eventName string, fields []Field, maxCycles, maxCIRecoveries in return fmt.Sprintf("Cancelled (%s, exit 130)", d), nil case "ci_failure": return fmt.Sprintf("Stopped: CI is still failing (%s, exit 3)", d), nil + case "ci_timeout": + return fmt.Sprintf("Failed: CI timed out (%s, exit 2)", d), nil } } return "", fmt.Errorf("unsupported human event %s with fields %#v", eventName, fields) @@ -672,7 +685,8 @@ func (l *Logger) livenessLabel(stage StageContext, transient bool) string { labels := map[string][2]string{ "review": {"Reviewing", "Review"}, "fix-findings": {"Fixing findings", "Fixing findings"}, - "finalize": {"Finalizing", "Finalization"}, + "publish": {"Publishing", "Publication"}, + "ci": {"Waiting for CI", "CI"}, "fix-ci": {"CI recovery", "CI recovery"}, } label, ok := labels[stage.Stage] diff --git a/internal/event/event_test.go b/internal/event/event_test.go index c78596f..06d71f0 100644 --- a/internal/event/event_test.go +++ b/internal/event/event_test.go @@ -64,17 +64,17 @@ func TestHumanEventCatalog(t *testing.T) { {"fix start", "stage_started", []Field{F("stage", "fix-findings"), F("cycle", "2")}, "10:04:05 [2/10] [gpt-test/high] Fixing findings\n"}, {"fix done", "stage_completed", []Field{F("stage", "fix-findings"), F("cycle", "2"), F("status", "success"), F("duration_ms", "263000")}, "10:04:05 [2/10] [gpt-test/high] Findings fixed (4m 23s)\n"}, {"fix failed", "stage_completed", []Field{F("stage", "fix-findings"), F("cycle", "2"), F("status", "failed"), F("duration_ms", "1000")}, "10:04:05 [2/10] [gpt-test/high] Fixing findings failed (1s)\n"}, - {"finalize start", "stage_started", []Field{F("stage", "finalize")}, "10:04:05 [gpt-test/high] Finalizing\n"}, - {"step", "step_completed", []Field{F("stage", "finalize"), F("step", "change_request"), F("status", "skipped")}, "10:04:05 [gpt-test/high] Change request: not needed\n"}, - {"finalize success", "stage_completed", []Field{F("stage", "finalize"), F("status", "success"), F("verdict", "SUCCESS"), F("duration_ms", "42000")}, "10:04:05 [gpt-test/high] Finalized successfully (42s)\n"}, - {"finalize ci", "stage_completed", []Field{F("stage", "finalize"), F("status", "success"), F("verdict", "CI_FAILED"), F("duration_ms", "42000")}, "10:04:05 [gpt-test/high] Finalized, but CI is failing (42s)\n"}, - {"finalize failed", "stage_completed", []Field{F("stage", "finalize"), F("status", "failed"), F("duration_ms", "42000")}, "10:04:05 [gpt-test/high] Finalization failed (42s)\n"}, + {"publish start", "stage_started", []Field{F("stage", "publish")}, "10:04:05 [gpt-test/high] Publishing\n"}, + {"step", "step_completed", []Field{F("stage", "publish"), F("step", "change_request"), F("status", "skipped")}, "10:04:05 [gpt-test/high] Change request: not needed\n"}, + {"publish success", "stage_completed", []Field{F("stage", "publish"), F("status", "success"), F("duration_ms", "42000")}, "10:04:05 [gpt-test/high] Published (42s)\n"}, + {"ci start", "stage_started", []Field{F("stage", "ci")}, "10:04:05 [gpt-test/high] Waiting for CI\n"}, + {"ci timeout", "stage_completed", []Field{F("stage", "ci"), F("status", "timeout"), F("duration_ms", "42000"), F("timeout_ms", "3600000")}, "10:04:05 [gpt-test/high] CI timed out after 42s (limit 1h)\n"}, {"ci start", "stage_started", []Field{F("stage", "fix-ci"), F("review_phase", "1")}, "10:04:05 [1/3] [gpt-test/high] CI recovery\n"}, {"ci done", "stage_completed", []Field{F("stage", "fix-ci"), F("review_phase", "1"), F("status", "success"), F("duration_ms", "68000")}, "10:04:05 [1/3] [gpt-test/high] CI recovery fixed (1m 8s)\n"}, {"done", "run_completed", []Field{F("status", "success"), F("exit_code", "0"), F("total_duration_ms", "525000")}, "10:04:05 Done (8m 45s)\n"}, - {"findings remain", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "committed_local"), F("checkpoint_branch", "feature/checkpoints"), F("checkpoint_commit", "abc1234"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; finalization was not reached; checkpoint committed locally on feature/checkpoints at abc1234 and not pushed (8m 45s, exit 1)\n"}, - {"encoded checkpoint branch", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "committed_local"), F("checkpoint_branch", "feature%3Da"), F("checkpoint_commit", "abc1234"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; finalization was not reached; checkpoint committed locally on feature=a at abc1234 and not pushed (8m 45s, exit 1)\n"}, - {"checkpoint skipped", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "not_attempted"), F("checkpoint_reason", "pre_existing_changes"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; finalization was not reached; checkpoint was skipped because the worktree had pre-existing changes (8m 45s, exit 1)\n"}, + {"findings remain", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "committed_local"), F("checkpoint_branch", "feature/checkpoints"), F("checkpoint_commit", "abc1234"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; publication was not reached; checkpoint committed locally on feature/checkpoints at abc1234 and not pushed (8m 45s, exit 1)\n"}, + {"encoded checkpoint branch", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "committed_local"), F("checkpoint_branch", "feature%3Da"), F("checkpoint_commit", "abc1234"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; publication was not reached; checkpoint committed locally on feature=a at abc1234 and not pushed (8m 45s, exit 1)\n"}, + {"checkpoint skipped", "run_completed", []Field{F("status", "findings_remaining"), F("exit_code", "1"), F("checkpoint_status", "not_attempted"), F("checkpoint_reason", "pre_existing_changes"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: fix budget exhausted; publication was not reached; checkpoint was skipped because the worktree had pre-existing changes (8m 45s, exit 1)\n"}, {"operational", "run_completed", []Field{F("status", "operational_failure"), F("exit_code", "2"), F("total_duration_ms", "525000")}, "10:04:05 Failed due to an operational error (8m 45s, exit 2)\n"}, {"cancelled", "run_completed", []Field{F("status", "cancelled"), F("exit_code", "130"), F("total_duration_ms", "525000")}, "10:04:05 Cancelled (8m 45s, exit 130)\n"}, {"ci failure", "run_completed", []Field{F("status", "ci_failure"), F("exit_code", "3"), F("total_duration_ms", "525000")}, "10:04:05 Stopped: CI is still failing (8m 45s, exit 3)\n"}, @@ -484,7 +484,7 @@ func TestDiagnosticIsSuppressedWhenTransientClearFails(t *testing.T) { func TestHumanRendererRejectsUnknownStatus(t *testing.T) { logger := Logger{Out: ioDiscard{}, Format: "human"} - for _, stage := range []string{"fix-findings", "fix-ci", "finalize"} { + for _, stage := range []string{"fix-findings", "fix-ci", "publish", "ci"} { err := logger.Emit("stage_completed", F("stage", stage), F("status", "unexpected"), F("duration_ms", "1")) if err == nil || !strings.Contains(err.Error(), "unsupported human event") { t.Errorf("stage %s error = %v", stage, err) @@ -524,7 +524,7 @@ func TestShimmerHighlightTravelsAndReturnsWithoutWrapping(t *testing.T) { } func TestLivenessStageLabels(t *testing.T) { - for _, stage := range []string{"review", "fix-findings", "finalize", "fix-ci"} { + for _, stage := range []string{"review", "fix-findings", "publish", "ci", "fix-ci"} { t.Run(stage, func(t *testing.T) { var out bytes.Buffer logger := Logger{Out: &out, Format: "human"} diff --git a/internal/repository/status.go b/internal/repository/status.go index ad4914c..992a3e3 100644 --- a/internal/repository/status.go +++ b/internal/repository/status.go @@ -2,8 +2,13 @@ package repository import ( "context" + "encoding/json" + "errors" "fmt" + "net/url" + "strconv" "strings" + "time" "github.com/dapi/code-converge/internal/runner" ) @@ -11,16 +16,41 @@ import ( // Status reports whether Git sees staged, unstaged, or untracked changes. type Status struct { Runner runner.Runner + // Wait is injectable only to make polling-time behavior deterministic in + // tests. Production uses the context-aware timer below. + Wait func(context.Context, time.Duration) bool } // Checkpoint is the local commit created for a successful findings-fix stage. -// It is deliberately not pushed; publication remains finalization's job. +// It is deliberately not pushed; publication remains the host workflow's job. type Checkpoint struct { Created bool Branch string Commit string } +// Publication is the deterministic result of making the reviewed revision +// available to GitHub. The SHA is deliberately retained for CI pinning. +type Publication struct { + Commit string + Push string + ChangeRequest string + URL string + Head string + Repository string +} + +// CIResult is intentionally separate from publication: a deadline is an +// operational outcome, not a failed test run. +type CIResult string + +const ( + CISuccess CIResult = "success" + CIFailed CIResult = "failed" + CISkipped CIResult = "skipped" + CITimeout CIResult = "timeout" +) + func (s Status) HasChanges(ctx context.Context) (bool, error) { result, err := s.status(ctx) if err != nil { @@ -86,6 +116,315 @@ func (s Status) Checkpoint(ctx context.Context, initialHead string, canCommit bo return Checkpoint{Created: true, Branch: branchName, Commit: commitID}, nil } +// Publish commits only changes that appeared in a clean run, pushes through a +// direct refspec (so a local tracking-ref update cannot make publication look +// unsuccessful), then reuses or creates exactly one open pull request. +func (s Status) Publish(ctx context.Context, allowCommit bool) (Publication, error) { + result := Publication{Commit: "skipped", Push: "skipped", ChangeRequest: "skipped"} + dirty, err := s.HasChanges(ctx) + if err != nil { + return result, fmt.Errorf("inspect publication status: %w", err) + } + if dirty { + if !allowCommit { + return result, fmt.Errorf("refuse to commit pre-existing worktree changes") + } + if _, err := s.Runner.Run(ctx, runner.Invocation{Executable: "git", Args: []string{"add", "-A"}}); err != nil { + return result, fmt.Errorf("stage publication commit: %w", err) + } + if _, err := s.Runner.Run(ctx, runner.Invocation{Executable: "git", Args: []string{"commit", "-m", "chore: publish reviewed changes"}}); err != nil { + return result, fmt.Errorf("commit reviewed changes: %w", err) + } + result.Commit = "success" + } + branch, err := s.gitValue(ctx, "branch", "--show-current") + if err != nil { + return result, fmt.Errorf("resolve publication branch: %w", err) + } + if branch == "" { + return result, fmt.Errorf("resolve publication branch: detached HEAD") + } + remote, err := s.pushRemote(ctx, branch) + if err != nil { + return result, err + } + repository, err := s.githubRepository(ctx, remote) + if err != nil { + return result, err + } + if _, err := s.Runner.Run(ctx, runner.Invocation{Executable: "git", Args: []string{"push", remote, "HEAD:refs/heads/" + branch}}); err != nil { + return result, fmt.Errorf("push %s/%s: %w", remote, branch, err) + } + result.Push = "success" + result.Head, err = s.gitValue(ctx, "rev-parse", "HEAD") + if err != nil { + return result, fmt.Errorf("resolve published head: %w", err) + } + if result.Head == "" { + return result, fmt.Errorf("resolve published head: empty SHA") + } + url, err := s.openPR(ctx, repository, branch) + if err != nil { + return result, err + } + result.URL, result.Repository, result.ChangeRequest = url, repository, "success" + return result, nil +} + +func (s Status) pushRemote(ctx context.Context, branch string) (string, error) { + for _, args := range [][]string{{"config", "--get", "branch." + branch + ".pushRemote"}, {"config", "--get", "remote.pushDefault"}} { + value, err := s.gitValue(ctx, args...) + if err == nil && value != "" { + return value, nil + } + } + remotes, err := s.gitValue(ctx, "remote") + if err != nil { + return "", fmt.Errorf("resolve push remote: %w", err) + } + items := strings.Fields(remotes) + for _, remote := range items { + if remote == "origin" { + return remote, nil + } + } + if len(items) == 1 { + return items[0], nil + } + return "", fmt.Errorf("resolve push remote: ambiguous remotes") +} + +func (s Status) gitValue(ctx context.Context, args ...string) (string, error) { + result, err := s.Runner.Run(ctx, runner.Invocation{Executable: "git", Args: args}) + if err != nil { + return "", err + } + return strings.TrimSpace(result.Stdout), nil +} + +func (s Status) githubRepository(ctx context.Context, remote string) (string, error) { + urls, err := s.gitValue(ctx, "remote", "get-url", "--push", "--all", remote) + if err != nil { + return "", fmt.Errorf("resolve GitHub repository for push remote %q: %w", remote, err) + } + identities := map[string]struct{}{} + for _, value := range strings.Fields(urls) { + identity, ok := githubRepositoryFromURL(value) + if !ok { + return "", fmt.Errorf("resolve GitHub repository for push remote %q: unsupported URL %q", remote, value) + } + identities[identity] = struct{}{} + } + if len(identities) != 1 { + return "", fmt.Errorf("resolve GitHub repository for push remote %q: ambiguous URLs", remote) + } + for identity := range identities { + return identity, nil + } + panic("unreachable") +} + +func githubRepositoryFromURL(value string) (string, bool) { + value = strings.TrimSpace(value) + if strings.HasPrefix(value, "git@github.com:") { + return githubRepositoryPath(strings.TrimPrefix(value, "git@github.com:")) + } + parsed, err := url.Parse(value) + if err != nil || !strings.EqualFold(parsed.Hostname(), "github.com") { + return "", false + } + return githubRepositoryPath(parsed.Path) +} + +func githubRepositoryPath(path string) (string, bool) { + path = strings.TrimSuffix(strings.Trim(path, "/"), ".git") + parts := strings.Split(path, "/") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return "", false + } + return parts[0] + "/" + parts[1], true +} + +type pullRequest struct { + URL string `json:"url"` +} + +func (s Status) openPR(ctx context.Context, repository, branch string) (string, error) { + result, err := s.Runner.Run(ctx, runner.Invocation{Executable: "gh", Args: []string{"pr", "list", "--repo", repository, "--head", branch, "--state", "open", "--json", "url", "--limit", "2"}}) + if err != nil { + return "", fmt.Errorf("discover pull request: %w", err) + } + var prs []pullRequest + if err := json.Unmarshal([]byte(result.Stdout), &prs); err != nil { + return "", fmt.Errorf("parse pull request discovery: %w", err) + } + if len(prs) > 1 { + return "", fmt.Errorf("discover pull request: ambiguous open pull requests") + } + if len(prs) == 1 { + if !validPullRequestURL(prs[0].URL, repository) { + return "", fmt.Errorf("parse discovered pull request: expected GitHub pull request URL for %s", repository) + } + return prs[0].URL, nil + } + // gh pr create writes the created PR URL to stdout; unlike gh pr list it + // does not provide a JSON output mode. Keep parsing local and reject any + // unexpected response instead of guessing which PR was created. + result, err = s.Runner.Run(ctx, runner.Invocation{Executable: "gh", Args: []string{"pr", "create", "--repo", repository, "--head", branch, "--fill"}}) + if err != nil { + return "", fmt.Errorf("create pull request: %w", err) + } + url := strings.TrimSpace(result.Stdout) + if !validPullRequestURL(url, repository) { + return "", fmt.Errorf("parse created pull request: expected one GitHub pull request URL for %s", repository) + } + return url, nil +} + +func validPullRequestURL(value, repository string) bool { + parsed, err := url.Parse(strings.TrimSpace(value)) + if err != nil || parsed.Scheme != "https" || !strings.EqualFold(parsed.Hostname(), "github.com") || parsed.RawQuery != "" || parsed.Fragment != "" { + return false + } + parts := strings.Split(strings.Trim(parsed.Path, "/"), "/") + if len(parts) != 4 || strings.Join(parts[:2], "/") != repository || parts[2] != "pull" { + return false + } + number, err := strconv.Atoi(parts[3]) + return err == nil && number > 0 +} + +type checkRuns struct { + CheckRuns []checkRun `json:"check_runs"` +} +type checkRun struct { + Status string `json:"status"` + Conclusion *string `json:"conclusion"` +} + +// WaitCI selects GitHub check-runs returned by the exact published SHA. No +// check-runs is a documented skipped outcome. Transient command failures are +// retried inside ctx's deadline; authentication-like failures fail immediately. +func (s Status) WaitCI(ctx context.Context, publication Publication) (CIResult, error) { + interval := 5 * time.Second + for { + if err := ctx.Err(); err != nil { + if err == context.DeadlineExceeded { + return CITimeout, nil + } + return "", err + } + if publication.Repository == "" { + return "", errors.New("query CI checks: publication repository is empty") + } + if strings.TrimSpace(publication.Head) == "" { + return "", errors.New("query CI checks: published head is empty") + } + // Check-runs is paginated. Ask gh to collect every page rather than + // treating the default first page as the complete applicable set: a + // pending or failed run beyond that page must not yield a false green. + result, err := s.Runner.Run(ctx, runner.Invocation{Executable: "gh", Args: []string{"api", "--paginate", "--slurp", "repos/" + publication.Repository + "/commits/" + publication.Head + "/check-runs?per_page=100"}}) + if err != nil { + if permanentProviderError(err.Error()) { + return "", fmt.Errorf("query CI checks: %w", err) + } + if !s.wait(ctx, interval) { + if ctx.Err() == context.DeadlineExceeded { + return CITimeout, nil + } + return "", ctx.Err() + } + continue + } + checks, err := parseCheckRuns(result.Stdout) + if err != nil { + return "", fmt.Errorf("parse CI checks: %w", err) + } + if len(checks) == 0 { + return CISkipped, nil + } + pending := false + for _, check := range checks { + if check.Status != "completed" { + pending = true + continue + } + conclusion := "" + if check.Conclusion != nil { + conclusion = *check.Conclusion + } + switch conclusion { + case "success", "skipped", "neutral": + default: + return CIFailed, nil + } + } + if !pending { + return CISuccess, nil + } + if !s.wait(ctx, interval) { + if ctx.Err() == context.DeadlineExceeded { + return CITimeout, nil + } + return "", ctx.Err() + } + } +} + +// parseCheckRuns accepts gh api's --slurp response, which is an array of +// check-run pages. Accepting one object as well keeps the parser compatible +// with a runner that has already collapsed a one-page response. +func parseCheckRuns(output string) ([]checkRun, error) { + var pages []checkRuns + if err := json.Unmarshal([]byte(output), &pages); err == nil { + var runs []checkRun + for _, page := range pages { + runs = append(runs, page.CheckRuns...) + } + return runs, nil + } + var page checkRuns + if err := json.Unmarshal([]byte(output), &page); err != nil { + return nil, err + } + return page.CheckRuns, nil +} + +func permanentProviderError(message string) bool { + message = strings.ToLower(message) + // gh's diagnostic text varies by version and transport. These classes cannot + // recover through polling, so surface them immediately instead of spending + // the operator's CI deadline on an impossible retry. + for _, marker := range []string{ + "authentication", "authorization", "not logged in", "auth login", + "http 400", "http 401", "http 403", "http 404", "http 422", + "unsupported protocol", "protocol error", + } { + if strings.Contains(message, marker) { + return true + } + } + return false +} + +func (s Status) wait(ctx context.Context, duration time.Duration) bool { + if s.Wait != nil { + return s.Wait(ctx, duration) + } + return wait(ctx, duration) +} + +func wait(ctx context.Context, duration time.Duration) bool { + timer := time.NewTimer(duration) + defer timer.Stop() + select { + case <-ctx.Done(): + return false + case <-timer.C: + return true + } +} + func (s Status) status(ctx context.Context) (runner.Result, error) { result, err := s.Runner.Run(ctx, runner.Invocation{Executable: "git", Args: []string{"status", "--porcelain", "--untracked-files=all"}}) if err != nil { diff --git a/internal/repository/status_test.go b/internal/repository/status_test.go index 19f9bb4..2dfc55a 100644 --- a/internal/repository/status_test.go +++ b/internal/repository/status_test.go @@ -11,6 +11,7 @@ import ( "slices" "strings" "testing" + "time" "github.com/dapi/code-converge/internal/runner" ) @@ -113,6 +114,267 @@ func TestStatusPropagatesRunnerError(t *testing.T) { } } +func TestPublishUsesDirectRefspecAndReusesPR(t *testing.T) { + fake := &scriptedRunner{t: t, run: func(inv runner.Invocation) (runner.Result, error) { + switch strings.Join(inv.Args, " ") { + case "status --porcelain --untracked-files=all": + return runner.Result{}, nil + case "branch --show-current": + return runner.Result{Stdout: "feature/one\n"}, nil + case "config --get branch.feature/one.pushRemote", "config --get remote.pushDefault": + return runner.Result{}, errors.New("not configured") + case "remote": + return runner.Result{Stdout: "origin\n"}, nil + case "remote get-url --push --all origin": + return runner.Result{Stdout: "git@github.com:dapi/code-converge.git\n"}, nil + case "push origin HEAD:refs/heads/feature/one": + return runner.Result{}, nil + case "rev-parse HEAD": + return runner.Result{Stdout: "published-sha\n"}, nil + case "pr list --repo dapi/code-converge --head feature/one --state open --json url --limit 2": + return runner.Result{Stdout: `[{"url":"https://github.com/dapi/code-converge/pull/39"}]`}, nil + default: + t.Fatalf("unexpected invocation: %#v", inv) + return runner.Result{}, nil + } + }} + publication, err := (Status{Runner: fake}).Publish(context.Background(), true) + if err != nil || publication.Push != "success" || publication.Head != "published-sha" { + t.Fatalf("publication=%#v err=%v", publication, err) + } + for _, inv := range fake.invocations { + if strings.Contains(strings.Join(inv.Args, " "), "push origin") && strings.Contains(strings.Join(inv.Args, " "), "--set-upstream") { + t.Fatal("publication updated tracking state") + } + } +} + +func TestPublishCreatesPRFromGHURL(t *testing.T) { + fake := &scriptedRunner{t: t, run: func(inv runner.Invocation) (runner.Result, error) { + switch strings.Join(inv.Args, " ") { + case "status --porcelain --untracked-files=all": + return runner.Result{}, nil + case "branch --show-current": + return runner.Result{Stdout: "feature/one\n"}, nil + case "config --get branch.feature/one.pushRemote", "config --get remote.pushDefault": + return runner.Result{}, errors.New("not configured") + case "remote": + return runner.Result{Stdout: "origin\n"}, nil + case "remote get-url --push --all origin": + return runner.Result{Stdout: "git@github.com:dapi/code-converge.git\n"}, nil + case "push origin HEAD:refs/heads/feature/one": + return runner.Result{}, nil + case "rev-parse HEAD": + return runner.Result{Stdout: "published-sha\n"}, nil + case "pr list --repo dapi/code-converge --head feature/one --state open --json url --limit 2": + return runner.Result{Stdout: "[]"}, nil + case "pr create --repo dapi/code-converge --head feature/one --fill": + return runner.Result{Stdout: "https://github.com/dapi/code-converge/pull/40\n"}, nil + default: + t.Fatalf("unexpected invocation: %#v", inv) + return runner.Result{}, nil + } + }} + publication, err := (Status{Runner: fake}).Publish(context.Background(), true) + if err != nil || publication.ChangeRequest != "success" || publication.URL != "https://github.com/dapi/code-converge/pull/40" { + t.Fatalf("publication=%#v err=%v", publication, err) + } +} + +func TestOpenPRRejectsMalformedProviderIdentity(t *testing.T) { + for _, test := range []struct { + name, listed, created string + }{ + {name: "malformed discovered URL", listed: `[{"url":"not-a-pr"}]`}, + {name: "malformed created URL", listed: "[]", created: "created pull request"}, + } { + t.Run(test.name, func(t *testing.T) { + fake := &scriptedRunner{t: t, run: func(inv runner.Invocation) (runner.Result, error) { + switch strings.Join(inv.Args, " ") { + case "pr list --repo dapi/code-converge --head feature/one --state open --json url --limit 2": + return runner.Result{Stdout: test.listed}, nil + case "pr create --repo dapi/code-converge --head feature/one --fill": + return runner.Result{Stdout: test.created}, nil + default: + t.Fatalf("unexpected invocation: %#v", inv) + return runner.Result{}, nil + } + }} + if _, err := (Status{Runner: fake}).openPR(context.Background(), "dapi/code-converge", "feature/one"); err == nil { + t.Fatal("expected malformed provider identity error") + } + }) + } +} + +func TestPublishFromLinkedWorktreeUsesHostGitMetadata(t *testing.T) { + // The linked worktree owns its files while the branch/ref metadata lives in + // root/.git. Publish runs through runner.Exec (the Code Converge host), not + // a Codex workspace, so it can update that common Git directory. + root := t.TempDir() + remote := filepath.Join(t.TempDir(), "remote.git") + worktree := filepath.Join(t.TempDir(), "linked-worktree") + runGit := func(dir string, args ...string) { + t.Helper() + command := exec.Command("git", append([]string{"-C", dir}, args...)...) + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, output) + } + } + runBareGit := func(args ...string) { + t.Helper() + command := exec.Command("git", append([]string{"--git-dir", remote}, args...)...) + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("git --git-dir %s %v: %v: %s", remote, args, err, output) + } + } + runGit(root, "init", "-q") + runGit(root, "config", "user.email", "test@example.com") + runGit(root, "config", "user.name", "Test") + if err := os.WriteFile(filepath.Join(root, "tracked.txt"), []byte("base"), 0o600); err != nil { + t.Fatal(err) + } + runGit(root, "add", "tracked.txt") + runGit(root, "commit", "-qm", "base") + if output, err := exec.Command("git", "init", "--bare", "-q", remote).CombinedOutput(); err != nil { + t.Fatalf("init bare remote: %v: %s", err, output) + } + // Use a GitHub-shaped remote for repository identity, routed locally by the + // test SSH shim below so no network access is involved. + runGit(root, "remote", "add", "origin", "git@github.com:dapi/code-converge.git") + runGit(root, "worktree", "add", "-q", "-b", "feature/linked", worktree) + + bin := t.TempDir() + ssh := filepath.Join(bin, "ssh") + if err := os.WriteFile(ssh, []byte("#!/bin/sh\nif [ \"$1\" = -G ]; then exit 0; fi\nexec git-receive-pack \"$CODE_CONVERGE_TEST_REMOTE\"\n"), 0o700); err != nil { + t.Fatal(err) + } + gh := filepath.Join(bin, "gh") + if err := os.WriteFile(gh, []byte("#!/bin/sh\ncase \"$1 $2\" in\n 'pr list') printf '[]' ;;\n 'pr create') printf 'https://github.com/dapi/code-converge/pull/39\\n' ;;\n *) echo \"unexpected gh invocation: $*\" >&2; exit 1 ;;\nesac\n"), 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("GIT_SSH_COMMAND", ssh) + t.Setenv("CODE_CONVERGE_TEST_REMOTE", remote) + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + + publication, err := (Status{Runner: runner.Exec{Dir: worktree}}).Publish(context.Background(), true) + if err != nil { + t.Fatal(err) + } + if publication.Push != "success" || publication.ChangeRequest != "success" || publication.Head == "" { + t.Fatalf("publication=%#v", publication) + } + remoteHeadCommand := exec.Command("git", "--git-dir", remote, "rev-parse", "refs/heads/feature/linked") + remoteHead, err := remoteHeadCommand.Output() + if err != nil || strings.TrimSpace(string(remoteHead)) != publication.Head { + t.Fatalf("remote head=%q err=%v, publication=%#v", remoteHead, err, publication) + } + // Keep the helper used so its failures remain easy to diagnose when Git's + // transport invocation changes on a supported platform. + runBareGit("show-ref", "--verify", "refs/heads/feature/linked") +} + +func TestWaitCIClassifiesExactHeadRuns(t *testing.T) { + for _, test := range []struct { + name, body string + want CIResult + }{ + {"skipped", `[{"check_runs":[]}]`, CISkipped}, + {"green", `[{"check_runs":[{"status":"completed","conclusion":"success"},{"status":"completed","conclusion":"skipped"}]}]`, CISuccess}, + {"failed", `[{"check_runs":[{"status":"completed","conclusion":"failure"}]}]`, CIFailed}, + } { + t.Run(test.name, func(t *testing.T) { + fake := &fakeRunner{result: runner.Result{Stdout: test.body}} + got, err := (Status{Runner: fake}).WaitCI(context.Background(), Publication{Head: "published-sha", Repository: "dapi/code-converge"}) + if err != nil || got != test.want { + t.Fatalf("WaitCI=%q,%v", got, err) + } + query := strings.Join(fake.invocations[0].Args, " ") + if !strings.Contains(query, "repos/dapi/code-converge/commits/published-sha/check-runs?per_page=100") || !strings.Contains(query, "--paginate --slurp") { + t.Fatalf("CI query was not pinned to the published remote and SHA: %q", query) + } + }) + } +} + +func TestGitHubRepositoryFromURL(t *testing.T) { + for _, test := range []struct { + url string + want string + ok bool + }{ + {"git@github.com:dapi/code-converge.git", "dapi/code-converge", true}, + {"https://github.com/dapi/code-converge.git", "dapi/code-converge", true}, + {"ssh://git@github.com/dapi/code-converge", "dapi/code-converge", true}, + {"https://example.com/dapi/code-converge.git", "", false}, + {"git@github.com:dapi/code-converge/extra.git", "", false}, + } { + got, ok := githubRepositoryFromURL(test.url) + if got != test.want || ok != test.ok { + t.Errorf("githubRepositoryFromURL(%q) = %q, %t; want %q, %t", test.url, got, ok, test.want, test.ok) + } + } +} + +func TestWaitCIFailsImmediatelyForPermanentProviderErrors(t *testing.T) { + fake := &fakeRunner{err: errors.New("To get started with GitHub CLI, please run: gh auth login")} + result, err := (Status{Runner: fake}).WaitCI(context.Background(), Publication{Head: "published-sha", Repository: "dapi/code-converge"}) + if result != "" || err == nil || !strings.Contains(err.Error(), "query CI checks") { + t.Fatalf("result=%q err=%v", result, err) + } + if len(fake.invocations) != 1 { + t.Fatalf("permanent provider error retried: %#v", fake.invocations) + } +} + +func TestWaitCIRejectsEmptyPublishedHead(t *testing.T) { + fake := &fakeRunner{} + result, err := (Status{Runner: fake}).WaitCI(context.Background(), Publication{Repository: "dapi/code-converge"}) + if result != "" || err == nil || !strings.Contains(err.Error(), "published head is empty") { + t.Fatalf("result=%q err=%v", result, err) + } + if len(fake.invocations) != 0 { + t.Fatalf("queried CI with an empty head: %#v", fake.invocations) + } +} + +func TestWaitCIRetriesTransientProviderErrorWithinDeadline(t *testing.T) { + attempts, waits := 0, 0 + fake := &scriptedRunner{t: t, run: func(inv runner.Invocation) (runner.Result, error) { + attempts++ + if attempts == 1 { + return runner.Result{}, errors.New("temporary GitHub API outage") + } + return runner.Result{Stdout: `[{"check_runs":[{"status":"completed","conclusion":"success"}]}]`}, nil + }} + result, err := (Status{Runner: fake, Wait: func(context.Context, time.Duration) bool { + waits++ + return true + }}).WaitCI(context.Background(), Publication{Head: "published-sha", Repository: "dapi/code-converge"}) + if err != nil || result != CISuccess || attempts != 2 || waits != 1 { + t.Fatalf("result=%q err=%v attempts=%d waits=%d", result, err, attempts, waits) + } +} + +func TestWaitCIDistinguishesDeadlineFromCancellation(t *testing.T) { + t.Run("deadline", func(t *testing.T) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + result, err := (Status{Runner: &fakeRunner{}}).WaitCI(ctx, Publication{Head: "published-sha", Repository: "dapi/code-converge"}) + if err != nil || result != CITimeout { + t.Fatalf("result=%q err=%v", result, err) + } + }) + t.Run("cancelled", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + result, err := (Status{Runner: &fakeRunner{}}).WaitCI(ctx, Publication{Head: "published-sha", Repository: "dapi/code-converge"}) + if result != "" || !errors.Is(err, context.Canceled) { + t.Fatalf("result=%q err=%v", result, err) + } + }) +} + func TestStatusCheckpointCommitsLocallyWithoutPush(t *testing.T) { fake := &scriptedRunner{t: t, run: func(inv runner.Invocation) (runner.Result, error) { switch strings.Join(inv.Args, " ") { diff --git a/internal/workflow/workflow.go b/internal/workflow/workflow.go index ff4da20..c7ef9ba 100644 --- a/internal/workflow/workflow.go +++ b/internal/workflow/workflow.go @@ -2,6 +2,7 @@ package workflow import ( "context" + "fmt" "io" "net/url" "strconv" @@ -25,7 +26,6 @@ const ( type Agent interface { Review(context.Context) (codex.ReviewResult, error) FixFindings(context.Context, string) error - Finalize(context.Context, bool) (codex.Finalization, error) FixCI(context.Context) error } @@ -34,6 +34,8 @@ type Repository interface { IsClean(context.Context) (bool, error) Head(context.Context) (string, error) Checkpoint(context.Context, string, bool) (repository.Checkpoint, error) + Publish(context.Context, bool) (repository.Publication, error) + WaitCI(context.Context, repository.Publication) (repository.CIResult, error) } type Workflow struct { @@ -57,6 +59,15 @@ func (w *Workflow) Run(ctx context.Context) int { if !w.emit("run_started") { return ExitOperational } + initialWorktreeClean := true + if w.Repository != nil { + var err error + initialWorktreeClean, err = w.Repository.IsClean(ctx) + if err != nil { + w.diagnostic("initial repository status failed", err) + return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) + } + } phase, cycle := 1, 1 fixes, recoveries := 0, 0 @@ -242,114 +253,65 @@ func (w *Workflow) Run(ctx context.Context) int { } } - stageStarted = now() - if !w.emit("stage_started", event.F("stage", "finalize"), event.F("model", w.stageModel("finalize")), event.F("reasoning_effort", w.stageReasoningEffort("finalize"))) { + if w.Repository == nil { return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - stageCtx, cancelStage = context.WithCancel(ctx) - liveness = w.Log.StartLiveness(stageCtx, event.StageContext{Stage: "finalize", Model: w.stageModel("finalize"), ReasoningEffort: w.stageReasoningEffort("finalize"), ReviewPhase: phase, Cycle: cycle}, stageStarted, cancelStage) - if err := w.Log.StartAgent("finalize"); err != nil { - _ = liveness.Stop() - cancelStage() - w.diagnostic("render interactive view", err) + stageStarted = now() + if !w.emit("stage_started", event.F("stage", "publish")) { return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - finalization, err := w.Agent.Finalize(runner.WithStageContext(stageCtx, runner.StageContext{Stage: "finalize", ReviewPhase: phase, Cycle: cycle, Model: w.stageModel("finalize"), ReasoningEffort: w.stageReasoningEffort("finalize")}), checkpointed) - presentationErr = nil - if err != nil && ctx.Err() != nil { - presentationErr = w.Log.CompleteAgent("finalize cancelled") - } else if err != nil { - presentationErr = w.Log.CompleteAgent("finalize failed") - } else { - presentationErr = w.Log.CompleteAgent("finalize completed") + publication, err := w.Repository.Publish(ctx, initialWorktreeClean) + if err != nil { + if ctx.Err() != nil { + return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) + } + _ = w.emitPublicationSteps(publication, "failed") + _ = w.emit("stage_completed", event.F("stage", "publish"), event.F("status", "failed"), durationField(now().Sub(stageStarted))) + w.diagnostic("publication failed", err) + return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - livenessErr = liveness.Stop() - cancelStage() - if livenessErr != nil { - w.diagnostic("write liveness", livenessErr) + if !w.emitPublicationSteps(publication, "") || !w.emit("stage_completed", event.F("stage", "publish"), event.F("status", "success"), durationField(now().Sub(stageStarted))) { return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - if presentationErr != nil { - w.diagnostic("render interactive view", presentationErr) + stageStarted = now() + if !w.emit("stage_started", event.F("stage", "ci"), event.F("head", publication.Head)) { return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } + ciCtx, cancelCI := context.WithTimeout(ctx, w.Config.CITimeout) + ci, err := w.Repository.WaitCI(ciCtx, publication) + cancelCI() if err != nil { if ctx.Err() != nil { return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) } - if !w.emitUnknownSteps() || !w.emit("stage_completed", event.F("stage", "finalize"), event.F("model", w.stageModel("finalize")), event.F("reasoning_effort", w.stageReasoningEffort("finalize")), event.F("status", "failed"), durationField(now().Sub(stageStarted))) { - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - w.diagnostic("finalization failed", err) + w.diagnostic("CI polling failed", err) return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - if ctx.Err() != nil { - return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) + if ci != repository.CISuccess && ci != repository.CISkipped && ci != repository.CIFailed && ci != repository.CITimeout { + w.diagnostic("CI polling failed", fmt.Errorf("unknown CI result %q", ci)) + return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - if !w.emitSteps(finalization) || !w.emit("stage_completed", event.F("stage", "finalize"), event.F("model", w.stageModel("finalize")), event.F("reasoning_effort", w.stageReasoningEffort("finalize")), event.F("status", "success"), event.F("verdict", finalization.Verdict), durationField(now().Sub(stageStarted))) { + ciCompletion := []event.Field{event.F("stage", "ci"), event.F("status", string(ci)), durationField(now().Sub(stageStarted))} + if ci == repository.CITimeout { + ciCompletion = append(ciCompletion, durationFieldNamed("timeout_ms", w.Config.CITimeout)) + } + if !w.emit("step_completed", event.F("stage", "ci"), event.F("step", "ci"), event.F("status", string(ci))) || !w.emit("stage_completed", ciCompletion...) { return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) } - - switch finalization.Verdict { - case "SUCCESS": + switch ci { + case repository.CISuccess, repository.CISkipped: return w.complete("success", ExitSuccess, now().Sub(runStarted)) - case "FAILED": - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - case "CI_FAILED": - // CI_FAILED is a published finalization result. A subsequent review - // phase must not describe this already-pushed checkpoint as local. - checkpointed = false - lastCheckpoint = repository.Checkpoint{} - checkpointSkipReason = "" + case repository.CITimeout: + return w.complete("ci_timeout", ExitOperational, now().Sub(runStarted)) + case repository.CIFailed: + checkpointed, lastCheckpoint, checkpointSkipReason = false, repository.Checkpoint{}, "" if recoveries >= w.Config.MaxCIRecoveries { return w.complete("ci_failure", ExitCI, now().Sub(runStarted)) } - stageStarted = now() - if !w.emit("stage_started", event.F("stage", "fix-ci"), event.F("model", w.stageModel("fix-ci")), event.F("reasoning_effort", w.stageReasoningEffort("fix-ci")), intField("review_phase", phase)) { - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - stageCtx, cancelStage = context.WithCancel(ctx) - liveness = w.Log.StartLiveness(stageCtx, event.StageContext{Stage: "fix-ci", Model: w.stageModel("fix-ci"), ReasoningEffort: w.stageReasoningEffort("fix-ci"), ReviewPhase: phase, Cycle: cycle}, stageStarted, cancelStage) - if err := w.Log.StartAgent("fix-ci " + strconv.Itoa(phase)); err != nil { - _ = liveness.Stop() - cancelStage() - w.diagnostic("render interactive view", err) - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - err = w.Agent.FixCI(runner.WithStageContext(stageCtx, runner.StageContext{Stage: "fix-ci", ReviewPhase: phase, Cycle: cycle, Model: w.stageModel("fix-ci"), ReasoningEffort: w.stageReasoningEffort("fix-ci")})) - presentationErr = nil - if err != nil && ctx.Err() != nil { - presentationErr = w.Log.CompleteAgent("fix-ci cancelled") - } else if err != nil { - presentationErr = w.Log.CompleteAgent("fix-ci failed") - } else { - presentationErr = w.Log.CompleteAgent("fix-ci completed") - } - livenessErr = liveness.Stop() - cancelStage() - if livenessErr != nil { - w.diagnostic("write liveness", livenessErr) - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - if presentationErr != nil { - w.diagnostic("render interactive view", presentationErr) - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - if err != nil && ctx.Err() != nil { - return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) - } - if ctx.Err() != nil { - return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) - } - stageStatus := "success" - if err != nil { - stageStatus = "failed" - } - if !w.emit("stage_completed", event.F("stage", "fix-ci"), event.F("model", w.stageModel("fix-ci")), event.F("reasoning_effort", w.stageReasoningEffort("fix-ci")), intField("review_phase", phase), event.F("status", stageStatus), durationField(now().Sub(stageStarted))) { - return w.complete("operational_failure", ExitOperational, now().Sub(runStarted)) - } - if err != nil { - w.diagnostic("CI fix failed", err) + if w.runFixCI(ctx, phase, cycle, now) != nil { + if ctx.Err() != nil { + return w.complete("cancelled", ExitInterrupted, now().Sub(runStarted)) + } return w.complete("ci_failure", ExitCI, now().Sub(runStarted)) } recoveries++ @@ -378,17 +340,40 @@ func (w *Workflow) completeFindingsRemaining(elapsed time.Duration, checkpoint r return ExitFindingsRemaining } -func (w *Workflow) emitSteps(result codex.Finalization) bool { - for _, step := range []struct{ name, status string }{ - {"commit", result.Commit}, {"push", result.Push}, {"change_request", result.ChangeRequest}, {"ci", result.CI}, - } { - if !w.emit("step_completed", event.F("stage", "finalize"), event.F("model", w.stageModel("finalize")), event.F("reasoning_effort", w.stageReasoningEffort("finalize")), event.F("step", step.name), event.F("status", step.status)) { +func (w *Workflow) emitPublicationSteps(result repository.Publication, fallback string) bool { + for _, step := range []struct{ name, status string }{{"commit", result.Commit}, {"push", result.Push}, {"change_request", result.ChangeRequest}} { + if step.status == "" { + step.status = fallback + } + if step.status == "" { + step.status = "unknown" + } + if !w.emit("step_completed", event.F("stage", "publish"), event.F("step", step.name), event.F("status", step.status)) { return false } } return true } +func (w *Workflow) runFixCI(ctx context.Context, phase, cycle int, now func() time.Time) error { + started := now() + if !w.emit("stage_started", event.F("stage", "fix-ci"), event.F("model", w.stageModel("fix-ci")), event.F("reasoning_effort", w.stageReasoningEffort("fix-ci")), intField("review_phase", phase)) { + return fmt.Errorf("emit CI-fix start") + } + err := w.Agent.FixCI(runner.WithStageContext(ctx, runner.StageContext{Stage: "fix-ci", ReviewPhase: phase, Cycle: cycle, Model: w.stageModel("fix-ci"), ReasoningEffort: w.stageReasoningEffort("fix-ci")})) + status := "success" + if err != nil { + status = "failed" + } + if !w.emit("stage_completed", event.F("stage", "fix-ci"), event.F("model", w.stageModel("fix-ci")), event.F("reasoning_effort", w.stageReasoningEffort("fix-ci")), intField("review_phase", phase), event.F("status", status), durationField(now().Sub(started))) { + return fmt.Errorf("emit CI-fix completion") + } + if err != nil { + w.diagnostic("CI fix failed", err) + } + return err +} + func (w *Workflow) stageModel(stage string) string { switch stage { case "review": @@ -401,11 +386,6 @@ func (w *Workflow) stageModel(stage string) string { return "gpt-5.6-luna" } return w.Config.FixModel - case "finalize": - if w.Config.FinalizeModel == "" { - return "gpt-5.3-codex-spark" - } - return w.Config.FinalizeModel case "fix-ci": if w.Config.CIFixModel != "" { return w.Config.CIFixModel @@ -428,11 +408,6 @@ func (w *Workflow) stageReasoningEffort(stage string) string { return w.Config.FixEffort } return "medium" - case "finalize": - if w.Config.FinalizeEffort != "" { - return w.Config.FinalizeEffort - } - return "agent-default" case "fix-ci": if w.Config.CIFixEffort != "" { return w.Config.CIFixEffort @@ -443,10 +418,6 @@ func (w *Workflow) stageReasoningEffort(stage string) string { } } -func (w *Workflow) emitUnknownSteps() bool { - return w.emitSteps(codex.Finalization{Commit: "unknown", Push: "unknown", ChangeRequest: "unknown", CI: "unknown"}) -} - func (w *Workflow) emit(name string, fields ...event.Field) bool { if err := w.Log.Emit(name, fields...); err != nil { w.diagnostic("write event stream", err) @@ -472,6 +443,10 @@ func durationField(value time.Duration) event.Field { return event.F("duration_ms", milliseconds(value)) } +func durationFieldNamed(name string, value time.Duration) event.Field { + return event.F(name, milliseconds(value)) +} + func milliseconds(value time.Duration) string { ms := value.Milliseconds() if ms < 0 { diff --git a/internal/workflow/workflow_test.go b/internal/workflow/workflow_test.go index fa4c1de..e083ab2 100644 --- a/internal/workflow/workflow_test.go +++ b/internal/workflow/workflow_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "errors" - "reflect" "strings" "testing" "time" @@ -13,666 +12,194 @@ import ( "github.com/dapi/code-converge/internal/config" "github.com/dapi/code-converge/internal/event" "github.com/dapi/code-converge/internal/repository" - "github.com/dapi/code-converge/internal/runner" ) -type fakeAgent struct { - reviews []codex.ReviewResult - reviewFailures map[int]error - finalizations []codex.Finalization - finalizeErr error - fixErr error - ciFixErr error - fixReports []string - reviewCalls int - reviewWait bool - reviewStarted chan struct{} - fixCalls int - finalizeCalls int - checkpointedFinalize []bool - ciFixCalls int - ciFixWait bool - ciFixStarted chan struct{} - finalizeStages []runner.StageContext - ciFixStages []runner.StageContext +type workflowAgent struct { + reviews []codex.ReviewResult + fixes int + fixErr error + ciFixes int + ciFixErr error } -type fakeRepository struct { - hasChanges bool - err error - calls int - dirty bool - cleanResults []bool - cleanErr error - checkpoint repository.Checkpoint - checkpoints []repository.Checkpoint - checkpointErr error - cleanCalls int - checkpointCalls int - head string -} - -func (f *fakeRepository) HasChanges(context.Context) (bool, error) { - f.calls++ - return f.hasChanges, f.err -} - -func (f *fakeRepository) IsClean(context.Context) (bool, error) { - f.cleanCalls++ - if index := f.cleanCalls - 1; index < len(f.cleanResults) { - return f.cleanResults[index], f.cleanErr - } - return !f.dirty, f.cleanErr -} - -func (f *fakeRepository) Head(context.Context) (string, error) { return f.head, nil } - -func (f *fakeRepository) Checkpoint(context.Context, string, bool) (repository.Checkpoint, error) { - f.checkpointCalls++ - if index := f.checkpointCalls - 1; index < len(f.checkpoints) { - return f.checkpoints[index], f.checkpointErr - } - return f.checkpoint, f.checkpointErr -} - -func (f *fakeAgent) Review(ctx context.Context) (codex.ReviewResult, error) { - index := f.reviewCalls - f.reviewCalls++ - if f.reviewStarted != nil { - close(f.reviewStarted) - } - if f.reviewWait { - <-ctx.Done() - return codex.ReviewResult{}, ctx.Err() - } - if err := f.reviewFailures[index]; err != nil { - return codex.ReviewResult{}, err - } - if index >= len(f.reviews) { - return codex.ReviewResult{}, errors.New("missing review fixture") - } - return f.reviews[index], nil -} - -func (f *fakeAgent) FixFindings(_ context.Context, report string) error { - f.fixCalls++ - f.fixReports = append(f.fixReports, report) - return f.fixErr -} - -func (f *fakeAgent) Finalize(ctx context.Context, checkpointed bool) (codex.Finalization, error) { - if stage, ok := runner.StageContextFrom(ctx); ok { - f.finalizeStages = append(f.finalizeStages, stage) - } - index := f.finalizeCalls - f.finalizeCalls++ - f.checkpointedFinalize = append(f.checkpointedFinalize, checkpointed) - if f.finalizeErr != nil { - return codex.Finalization{}, f.finalizeErr - } - if index >= len(f.finalizations) { - return codex.Finalization{}, errors.New("missing finalization fixture") - } - return f.finalizations[index], nil -} - -func (f *fakeAgent) FixCI(ctx context.Context) error { - f.ciFixCalls++ - if stage, ok := runner.StageContextFrom(ctx); ok { - f.ciFixStages = append(f.ciFixStages, stage) - } - if f.ciFixStarted != nil { - close(f.ciFixStarted) - } - if f.ciFixWait { - <-ctx.Done() - return ctx.Err() - } - return f.ciFixErr -} - -func success() codex.Finalization { - return codex.Finalization{Verdict: "SUCCESS", Commit: "success", Push: "success", ChangeRequest: "skipped", CI: "success"} -} - -func ciFailed() codex.Finalization { - return codex.Finalization{Verdict: "CI_FAILED", Commit: "success", Push: "success", ChangeRequest: "success", CI: "failed"} -} - -func findings() codex.ReviewResult { - return codex.ReviewResult{Counts: codex.Counts{High: 1}, Report: "## Findings\n- [P1] a finding"} -} - -func clean() codex.ReviewResult { return codex.ReviewResult{Clean: true} } - -func run(t *testing.T, cfg config.Config, agent *fakeAgent) (int, string, string) { - return runWithRepository(t, cfg, agent, &fakeRepository{hasChanges: true}) -} - -func runWithRepository(t *testing.T, cfg config.Config, agent *fakeAgent, repository Repository) (int, string, string) { - t.Helper() - var out, stderr bytes.Buffer - tick := 0 - now := func() time.Time { - tick++ - return time.Date(2026, 7, 21, 10, 0, 0, tick*int(time.Millisecond), time.UTC) - } - w := Workflow{Config: cfg, Agent: agent, Repository: repository, Log: &event.Logger{Out: &out, Now: now, Format: cfg.LogFormat, Heartbeat: cfg.Heartbeat}, Err: &stderr, Now: now} - return w.Run(context.Background()), out.String(), stderr.String() -} - -func TestHumanHappyPath(t *testing.T) { - agent := &fakeAgent{ - reviews: []codex.ReviewResult{{Counts: codex.Counts{High: 1, Medium: 2}, Report: "findings"}, clean()}, - finalizations: []codex.Finalization{success()}, - } - code, output, stderr := run(t, config.Config{LogFormat: "human", MaxCycles: 1}, agent) - if code != ExitSuccess || stderr != "" { - t.Fatalf("code=%d stderr=%q", code, stderr) - } - for _, want := range []string{ - "10:00:00 [1/1] [gpt-5.6-sol/medium] Review started\n", "10:00:00 [1/1] [gpt-5.6-sol/medium] Review: 3 findings [P0:0; P1:1; P2:2] (0s)\n", - "10:00:00 [1/1] [gpt-5.6-luna/medium] Fixing findings\n", "10:00:00 [1/1] [gpt-5.6-luna/medium] Findings fixed (0s)\n", "10:00:00 [2/1] [gpt-5.6-sol/medium] Review: clean (0s)\n", - "10:00:00 [gpt-5.3-codex-spark/agent-default] Finalizing\n", "10:00:00 [gpt-5.3-codex-spark/agent-default] Commit: done\n", "10:00:00 [gpt-5.3-codex-spark/agent-default] Change request: not needed\n", "10:00:00 [gpt-5.3-codex-spark/agent-default] Finalized successfully (0s)\n", "10:00:00 Done (0s)\n", - } { - if !strings.Contains(output, want) { - t.Errorf("missing %q in:\n%s", want, output) - } - } - if strings.Contains(output, "event=") || strings.Contains(output, "findings_critical") || strings.Contains(output, "duration_ms") { - t.Fatalf("human output leaked kv fields:\n%s", output) - } -} - -func TestHumanTerminalPaths(t *testing.T) { - tests := []struct { - name string - cfg config.Config - agent *fakeAgent - code int - want string - }{ - {"findings", config.Config{LogFormat: "human", MaxCycles: 0}, &fakeAgent{reviews: []codex.ReviewResult{findings()}}, ExitFindingsRemaining, "fix budget exhausted; finalization was not reached; checkpoint was not attempted"}, - {"operational", config.Config{LogFormat: "human"}, &fakeAgent{reviewFailures: map[int]error{0: errors.New("bad")}}, ExitOperational, "Failed due to an operational error"}, - {"ci", config.Config{LogFormat: "human", MaxCIRecoveries: 0}, &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{ciFailed()}}, ExitCI, "Stopped: CI is still failing"}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - code, output, _ := run(t, test.cfg, test.agent) - if code != test.code || !strings.Contains(output, test.want) { - t.Fatalf("code=%d output=\n%s", code, output) - } - }) - } -} - -func TestHappyPath(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{success()}} - code, output, stderr := run(t, config.Config{MaxCycles: 10, MaxCIRecoveries: 3, ReviewModel: "review-model", ReviewEffort: "high", FixModel: "fix-model", FixEffort: "low", FinalizeModel: "finalize-model"}, agent) - if code != ExitSuccess || stderr != "" { - t.Fatalf("code=%d stderr=%q", code, stderr) - } - assertRecord(t, output, "event=review_completed", "status=clean", "findings_total=0") - assertRecord(t, output, "event=stage_started", "stage=review", "model=review-model") - assertRecord(t, output, "event=stage_started", "stage=review", "reasoning_effort=high") - assertRecord(t, output, "event=stage_started", "stage=finalize", "model=finalize-model") - assertRecord(t, output, "event=stage_started", "stage=finalize", "reasoning_effort=agent-default") - assertRecord(t, output, "event=step_completed", "stage=finalize", "model=finalize-model") - assertRecord(t, output, "event=run_completed", "status=success", "exit_code=0") - for _, step := range []string{"commit", "push", "change_request", "ci"} { - assertRecord(t, output, "event=step_completed", "step="+step) - } -} - -func TestCleanNoChangeCompletesWithoutFinalization(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}} - repository := &fakeRepository{} - code, output, stderr := runWithRepository(t, config.Config{}, agent, repository) - if code != ExitSuccess || stderr != "" || repository.calls != 1 || agent.finalizeCalls != 0 { - t.Fatalf("code=%d stderr=%q status calls=%d finalize calls=%d", code, stderr, repository.calls, agent.finalizeCalls) - } - assertRecord(t, output, "event=review_completed", "status=clean", "findings_total=0") - assertRecord(t, output, "event=run_completed", "status=success", "exit_code=0") - if strings.Contains(output, "stage=finalize") { - t.Fatalf("no-change run started finalization:\n%s", output) - } -} - -func TestReviewMetadataUsesResolvedCommitForEventSafety(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{{Clean: true, Scope: repository.ReviewTarget{Base: "release=1", BaseCommit: "0123456789abcdef", MergeBase: "abcdef0123456789", Source: "explicit"}}}, finalizations: []codex.Finalization{success()}} - code, output, stderr := run(t, config.Config{}, agent) - if code != ExitSuccess || stderr != "" { - t.Fatalf("code=%d stderr=%q", code, stderr) - } - assertRecord(t, output, "event=review_completed", "review_base=0123456789abcdef", "review_merge_base=abcdef0123456789", "review_base_source=explicit") - if strings.Contains(output, "release=1") { - t.Fatalf("raw ref leaked into event stream:\n%s", output) - } -} - -func TestRepositoryStatusFailureIsOperational(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}} - repository := &fakeRepository{err: errors.New("git unavailable")} - code, output, stderr := runWithRepository(t, config.Config{}, agent, repository) - if code != ExitOperational || agent.finalizeCalls != 0 { - t.Fatalf("code=%d finalize calls=%d", code, agent.finalizeCalls) - } - assertRecord(t, output, "event=review_completed", "status=clean") - assertRecord(t, output, "event=run_completed", "status=operational_failure", "exit_code=2") - if !strings.Contains(stderr, "repository status failed") { - t.Fatalf("stderr=%q", stderr) +func (a *workflowAgent) Review(context.Context) (codex.ReviewResult, error) { + if len(a.reviews) == 0 { + return codex.ReviewResult{}, errors.New("missing review result") } + result := a.reviews[0] + a.reviews = a.reviews[1:] + return result, nil } - -func TestMandatoryVerificationAndFindingsLimit(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), findings(), findings()}} - repository := &fakeRepository{checkpoint: repository.Checkpoint{Created: true, Branch: "feature/checkpoints", Commit: "abc1234"}} - code, output, _ := runWithRepository(t, config.Config{MaxCycles: 2}, agent, repository) - if code != ExitFindingsRemaining || agent.fixCalls != 2 || agent.reviewCalls != 3 { - t.Fatalf("code=%d fixes=%d reviews=%d", code, agent.fixCalls, agent.reviewCalls) - } - assertRecord(t, output, "event=stage_started", "stage=review", "cycle=3") - assertRecord(t, output, "event=run_completed", "status=findings_remaining", "exit_code=1") - assertRecord(t, output, "event=run_completed", "checkpoint_status=committed_local", "checkpoint_branch=feature%2Fcheckpoints", "checkpoint_commit=abc1234") +func (a *workflowAgent) FixFindings(context.Context, string) error { + a.fixes++ + return a.fixErr } - -func TestCheckpointBranchIsKVSafe(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), findings()}} - repository := &fakeRepository{checkpoint: repository.Checkpoint{Created: true, Branch: "feature=a", Commit: "abc1234"}} - code, output, stderr := runWithRepository(t, config.Config{MaxCycles: 1}, agent, repository) - if code != ExitFindingsRemaining || stderr != "" { - t.Fatalf("code=%d stderr=%q", code, stderr) - } - assertRecord(t, output, "event=run_completed", "checkpoint_branch=feature%3Da", "checkpoint_commit=abc1234") +func (a *workflowAgent) FixCI(context.Context) error { + a.ciFixes++ + return a.ciFixErr } -func TestCheckpointedFixFinalizesAfterCleanReview(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), clean()}, finalizations: []codex.Finalization{success()}} - repository := &fakeRepository{checkpoint: repository.Checkpoint{Created: true, Branch: "feature/checkpoints", Commit: "abc1234"}} - code, _, stderr := runWithRepository(t, config.Config{MaxCycles: 1}, agent, repository) - if code != ExitSuccess || stderr != "" || repository.cleanCalls != 1 || repository.checkpointCalls != 1 || agent.finalizeCalls != 1 || !agent.checkpointedFinalize[0] { - t.Fatalf("code=%d stderr=%q clean checks=%d checkpoints=%d finalizations=%d checkpointed=%v", code, stderr, repository.cleanCalls, repository.checkpointCalls, agent.finalizeCalls, agent.checkpointedFinalize) - } +type workflowRepository struct { + changes []bool + clean []bool + publication repository.Publication + publishErr error + ci []repository.CIResult + ciErr error + publishes int + ciWaits int } -func TestCheckpointFailureStopsBeforeNextReview(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), clean()}} - repository := &fakeRepository{checkpointErr: errors.New("commit failed")} - code, output, stderr := runWithRepository(t, config.Config{MaxCycles: 1}, agent, repository) - if code != ExitOperational || agent.reviewCalls != 1 || !strings.Contains(stderr, "findings checkpoint failed") { - t.Fatalf("code=%d reviews=%d stderr=%q", code, agent.reviewCalls, stderr) +func (r *workflowRepository) next(values []bool) bool { + if len(values) == 0 { + return false } - assertRecord(t, output, "event=run_completed", "status=operational_failure", "exit_code=2") -} - -func TestDirtyWorktreeSkipsCheckpointAndStillFixes(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), clean()}, finalizations: []codex.Finalization{success()}} - repository := &fakeRepository{hasChanges: true, dirty: true} - code, _, stderr := runWithRepository(t, config.Config{MaxCycles: 1}, agent, repository) - if code != ExitSuccess || stderr != "" || agent.fixCalls != 1 || repository.checkpointCalls != 1 || agent.finalizeCalls != 1 { - t.Fatalf("code=%d fixes=%d checkpoints=%d finalizations=%d stderr=%q", code, agent.fixCalls, repository.checkpointCalls, agent.finalizeCalls, stderr) + value := values[0] + if len(values) > 1 { + r.changes = values[1:] } + return value } - -func TestDirtyWorktreeReportsSkippedCheckpointOnExhaustion(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), findings()}} - repository := &fakeRepository{dirty: true} - code, output, stderr := runWithRepository(t, config.Config{MaxCycles: 1}, agent, repository) - if code != ExitFindingsRemaining || stderr != "" || agent.fixCalls != 1 || repository.checkpointCalls != 1 { - t.Fatalf("code=%d fixes=%d checkpoints=%d stderr=%q", code, agent.fixCalls, repository.checkpointCalls, stderr) +func (r *workflowRepository) HasChanges(context.Context) (bool, error) { + if len(r.changes) == 0 { + return false, nil } - assertRecord(t, output, "event=run_completed", "status=findings_remaining", "checkpoint_status=not_attempted", "checkpoint_reason=pre_existing_changes") + value := r.changes[0] + r.changes = r.changes[1:] + return value, nil } - -func TestCleanFixClearsEarlierCheckpointSkipReason(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), findings(), findings()}} - repository := &fakeRepository{cleanResults: []bool{false, true}, checkpoints: []repository.Checkpoint{{}}} - code, output, stderr := runWithRepository(t, config.Config{MaxCycles: 2}, agent, repository) - if code != ExitFindingsRemaining || stderr != "" || agent.fixCalls != 2 || repository.checkpointCalls != 2 { - t.Fatalf("code=%d fixes=%d checkpoints=%d stderr=%q", code, agent.fixCalls, repository.checkpointCalls, stderr) - } - assertRecord(t, output, "event=run_completed", "checkpoint_status=no_changes") - if strings.Contains(output, "checkpoint_reason=pre_existing_changes") { - t.Fatalf("stale checkpoint skip reason leaked:\n%s", output) - } -} - -func TestZeroFixBudget(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings()}} - code, _, _ := run(t, config.Config{MaxCycles: 0}, agent) - if code != ExitFindingsRemaining || agent.fixCalls != 0 || agent.reviewCalls != 1 { - t.Fatalf("code=%d fixes=%d reviews=%d", code, agent.fixCalls, agent.reviewCalls) +func (r *workflowRepository) IsClean(context.Context) (bool, error) { + if len(r.clean) == 0 { + return true, nil } + value := r.clean[0] + r.clean = r.clean[1:] + return value, nil } - -func TestFixReceivesReviewReport(t *testing.T) { - result := findings() - agent := &fakeAgent{reviews: []codex.ReviewResult{result, clean()}, finalizations: []codex.Finalization{success()}} - code, _, _ := run(t, config.Config{MaxCycles: 1}, agent) - if code != ExitSuccess || len(agent.fixReports) != 1 || agent.fixReports[0] != result.Report { - t.Fatalf("code=%d reports=%q", code, agent.fixReports) - } -} - -func TestCIRecoveryRestartsReviewPhase(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean(), clean()}, finalizations: []codex.Finalization{ciFailed(), success()}} - code, output, _ := run(t, config.Config{MaxCycles: 1, MaxCIRecoveries: 1}, agent) - if code != ExitSuccess || agent.ciFixCalls != 1 { - t.Fatalf("code=%d ci fixes=%d", code, agent.ciFixCalls) - } - assertRecord(t, output, "event=stage_started", "stage=review", "review_phase=2", "cycle=1") +func (*workflowRepository) Head(context.Context) (string, error) { return "head", nil } +func (*workflowRepository) Checkpoint(context.Context, string, bool) (repository.Checkpoint, error) { + return repository.Checkpoint{}, nil } - -func TestLaterStagesReceiveReviewPhaseAndCycle(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean(), clean()}, finalizations: []codex.Finalization{ciFailed(), success()}} - code, _, _ := run(t, config.Config{MaxCycles: 1, MaxCIRecoveries: 1}, agent) - if code != ExitSuccess { - t.Fatalf("code=%d", code) - } - if got, want := agent.finalizeStages, []runner.StageContext{ - {Stage: "finalize", ReviewPhase: 1, Cycle: 1, Model: "gpt-5.3-codex-spark", ReasoningEffort: "agent-default"}, - {Stage: "finalize", ReviewPhase: 2, Cycle: 1, Model: "gpt-5.3-codex-spark", ReasoningEffort: "agent-default"}, - }; !reflect.DeepEqual(got, want) { - t.Fatalf("finalize stages=%#v want=%#v", got, want) +func (r *workflowRepository) Publish(context.Context, bool) (repository.Publication, error) { + r.publishes++ + if r.publishErr != nil { + return repository.Publication{}, r.publishErr } - if got, want := agent.ciFixStages, []runner.StageContext{{Stage: "fix-ci", ReviewPhase: 1, Cycle: 1, Model: "agent-default", ReasoningEffort: "agent-default"}}; !reflect.DeepEqual(got, want) { - t.Fatalf("CI fix stages=%#v want=%#v", got, want) + if r.publication.Head == "" { + r.publication = repository.Publication{Commit: "skipped", Push: "success", ChangeRequest: "success", Head: "published"} } + return r.publication, nil } - -func TestCIRecoveryClearsPublishedCheckpointBeforeNextPhase(t *testing.T) { - agent := &fakeAgent{ - reviews: []codex.ReviewResult{findings(), clean(), findings(), findings()}, - finalizations: []codex.Finalization{ciFailed()}, - } - repository := &fakeRepository{checkpoints: []repository.Checkpoint{{Created: true, Branch: "feature/checkpoints", Commit: "abc1234"}, {}}} - code, output, stderr := runWithRepository(t, config.Config{MaxCycles: 1, MaxCIRecoveries: 1}, agent, repository) - if code != ExitFindingsRemaining || stderr != "" || agent.ciFixCalls != 1 { - t.Fatalf("code=%d ci fixes=%d stderr=%q", code, agent.ciFixCalls, stderr) +func (r *workflowRepository) WaitCI(context.Context, repository.Publication) (repository.CIResult, error) { + r.ciWaits++ + if r.ciErr != nil { + return "", r.ciErr } - assertRecord(t, output, "event=run_completed", "status=findings_remaining", "checkpoint_status=no_changes") - if strings.Contains(output, "checkpoint_commit=abc1234") { - t.Fatalf("published checkpoint leaked into next phase terminal result:\n%s", output) + if len(r.ci) == 0 { + return repository.CISuccess, nil } + value := r.ci[0] + r.ci = r.ci[1:] + return value, nil } -func TestStageModelsAreLogged(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{findings(), clean(), clean()}, finalizations: []codex.Finalization{ciFailed(), success()}} - _, output, _ := run(t, config.Config{MaxCycles: 1, MaxCIRecoveries: 1, ReviewModel: "review", ReviewEffort: "high", FixModel: "fix", FixEffort: "low", FinalizeModel: "final", FinalizeEffort: "medium", CIFixModel: "ci", CIFixEffort: "high"}, agent) - for _, stage := range []struct{ name, model, effort string }{{"review", "review", "high"}, {"fix-findings", "fix", "low"}, {"finalize", "final", "medium"}, {"fix-ci", "ci", "high"}} { - assertRecord(t, output, "event=stage_started", "stage="+stage.name, "model="+stage.model, "reasoning_effort="+stage.effort) - } -} +func cleanReview() codex.ReviewResult { return codex.ReviewResult{Clean: true} } -func TestCIFailurePaths(t *testing.T) { - tests := []struct { - name string - cfg config.Config - agent *fakeAgent - }{ - {"exhausted", config.Config{MaxCIRecoveries: 0}, &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{ciFailed()}}}, - {"fix failed", config.Config{MaxCIRecoveries: 1}, &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{ciFailed()}, ciFixErr: errors.New("red")}}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - code, output, _ := run(t, test.cfg, test.agent) - if code != ExitCI { - t.Fatalf("code=%d", code) - } - assertRecord(t, output, "event=run_completed", "status=ci_failure", "exit_code=3") - }) - } +func runWorkflow(t *testing.T, cfg config.Config, agent *workflowAgent, repo *workflowRepository) (int, string) { + t.Helper() + var output, stderr bytes.Buffer + now := func() time.Time { return time.Date(2026, 7, 31, 12, 0, 0, 0, time.UTC) } + w := Workflow{Config: cfg, Agent: agent, Repository: repo, Log: &event.Logger{Out: &output, Format: "kv", Now: now}, Err: &stderr, Now: now} + code := w.Run(context.Background()) + return code, output.String() } -func TestOperationalFailures(t *testing.T) { - tests := []struct { - name string - agent *fakeAgent - wantEvent []string - }{ - {"review", &fakeAgent{reviewFailures: map[int]error{0: errors.New("bad report")}}, []string{"event=review_completed", "status=failed"}}, - {"fix", &fakeAgent{reviews: []codex.ReviewResult{findings()}, fixErr: errors.New("fix failed")}, []string{"event=stage_completed", "stage=fix-findings", "status=failed"}}, - {"finalize", &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizeErr: errors.New("bad json")}, []string{"event=stage_completed", "stage=finalize", "status=failed"}}, - {"failed verdict", &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{{Verdict: "FAILED", Commit: "failed", Push: "skipped", ChangeRequest: "skipped", CI: "skipped"}}}, []string{"event=stage_completed", "verdict=FAILED"}}, +func TestCleanReviewPublishesAndWaitsForCI(t *testing.T) { + repo := &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CISuccess}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}}, repo) + if code != ExitSuccess || repo.publishes != 1 || repo.ciWaits != 1 { + t.Fatalf("code=%d publishes=%d waits=%d", code, repo.publishes, repo.ciWaits) } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - code, output, _ := run(t, config.Config{MaxCycles: 1}, test.agent) - if code != ExitOperational { - t.Fatalf("code=%d", code) - } - assertRecord(t, output, test.wantEvent...) - assertRecord(t, output, "event=run_completed", "status=operational_failure", "exit_code=2") - if test.name == "finalize" { - if countRecords(output, "event=step_completed") != 4 || countRecords(output, "status=unknown") != 4 { - t.Fatalf("unknown steps missing:\n%s", output) - } - } - }) - } -} - -func TestEveryRecordIsMachineSafe(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{success()}} - _, output, _ := run(t, config.Config{}, agent) - for number, line := range strings.Split(strings.TrimSpace(output), "\n") { - fields := strings.Fields(line) - if len(fields) < 2 || !strings.HasPrefix(fields[0], "ts=") || !strings.HasPrefix(fields[1], "event=") { - t.Fatalf("line %d has invalid prefix: %q", number+1, line) - } - for _, field := range fields { - if strings.Count(field, "=") != 1 { - t.Fatalf("line %d invalid field %q", number+1, field) - } + for _, want := range []string{"stage=publish", "step=push status=success", "stage=ci", "status=success"} { + if !strings.Contains(output, want) { + t.Fatalf("missing %q in:\n%s", want, output) } } } -type failingWriter struct{ writes int } - -func (w *failingWriter) Write(data []byte) (int, error) { - w.writes++ - if w.writes >= 2 { - return 0, errors.New("closed stdout") +func TestNoApplicableCISucceeds(t *testing.T) { + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}}, &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CISkipped}}) + if code != ExitSuccess || !strings.Contains(output, "stage=ci step=ci status=skipped") { + t.Fatalf("code=%d output=%s", code, output) } - return len(data), nil } -func TestEventFailureStopsBeforeAgentSideEffects(t *testing.T) { - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{success()}} - writer := &failingWriter{} - var stderr bytes.Buffer - w := Workflow{Config: config.Config{}, Agent: agent, Log: &event.Logger{Out: writer}, Err: &stderr} - if code := w.Run(context.Background()); code != ExitOperational { - t.Fatalf("code=%d", code) +func TestCIFailureRunsFixThenReviewsAndPublishesAgain(t *testing.T) { + repo := &workflowRepository{changes: []bool{true, true}, ci: []repository.CIResult{repository.CIFailed, repository.CISuccess}} + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview(), cleanReview()}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute, MaxCIRecoveries: 1}, agent, repo) + if code != ExitSuccess || agent.ciFixes != 1 || repo.publishes != 2 { + t.Fatalf("code=%d fixes=%d publishes=%d", code, agent.ciFixes, repo.publishes) } - if agent.reviewCalls != 0 || agent.fixCalls != 0 || agent.finalizeCalls != 0 || agent.ciFixCalls != 0 { - t.Fatalf("agent invoked after event failure: %#v", agent) + if !strings.Contains(output, "stage=fix-ci") { + t.Fatalf("missing fix-ci: %s", output) } - if !strings.Contains(stderr.String(), "write event stream") { - t.Fatalf("stderr=%q", stderr.String()) - } -} - -type configurableFailingWriter struct { - writes int - failAfter int } -func (w *configurableFailingWriter) Write(data []byte) (int, error) { - w.writes++ - if w.writes >= w.failAfter { - return 0, errors.New("closed stdout") +func TestFindingsFixThenCleanPublishes(t *testing.T) { + findings := codex.ReviewResult{Clean: false, Report: "fix this"} + agent := &workflowAgent{reviews: []codex.ReviewResult{findings, cleanReview()}} + repo := &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CISuccess}} + code, _ := runWorkflow(t, config.Config{CITimeout: time.Minute, MaxCycles: 1}, agent, repo) + if code != ExitSuccess || agent.fixes != 1 || repo.publishes != 1 { + t.Fatalf("code=%d fixes=%d publishes=%d", code, agent.fixes, repo.publishes) } - return len(data), nil } -func TestEmitFailureMidWorkflow(t *testing.T) { - tests := []struct { - name string - failAfter int - agent *fakeAgent - }{ - {"review_completed", 3, &fakeAgent{reviews: []codex.ReviewResult{clean()}}}, - {"fix-findings stage_started", 3, &fakeAgent{reviews: []codex.ReviewResult{findings()}}}, - {"finalize stage_started", 4, &fakeAgent{reviews: []codex.ReviewResult{clean()}}}, - {"run_completed", 10, &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{success()}}}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - writer := &configurableFailingWriter{failAfter: test.failAfter} - var stderr bytes.Buffer - w := Workflow{Config: config.Config{MaxCycles: 1}, Agent: test.agent, Log: &event.Logger{Out: writer}, Err: &stderr} - if code := w.Run(context.Background()); code != ExitOperational { - t.Fatalf("code=%d", code) - } - if !strings.Contains(stderr.String(), "write event stream") { - t.Fatalf("stderr=%q", stderr.String()) - } - }) +func TestCIRecoveryLimitRemainsEffective(t *testing.T) { + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute, MaxCIRecoveries: 0}, agent, &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CIFailed}}) + if code != ExitCI || agent.ciFixes != 0 || !strings.Contains(output, "status=ci_failure exit_code=3") { + t.Fatalf("code=%d fixes=%d output=%s", code, agent.ciFixes, output) } } -func TestMillisecondsNegative(t *testing.T) { - if got := milliseconds(-time.Second); got != "0" { - t.Fatalf("milliseconds(-1s) = %q, want 0", got) +func TestCIFixFailureStopsRecovery(t *testing.T) { + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}, ciFixErr: errors.New("repair failed")} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute, MaxCIRecoveries: 1}, agent, &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CIFailed}}) + if code != ExitCI || agent.ciFixes != 1 || !strings.Contains(output, "stage=fix-ci") || !strings.Contains(output, "status=ci_failure exit_code=3") { + t.Fatalf("code=%d fixes=%d output=%s", code, agent.ciFixes, output) } } -func TestEmitStepsFailure(t *testing.T) { - writer := &configurableFailingWriter{failAfter: 5} - var stderr bytes.Buffer - agent := &fakeAgent{reviews: []codex.ReviewResult{clean()}, finalizations: []codex.Finalization{success()}} - w := Workflow{Config: config.Config{}, Agent: agent, Log: &event.Logger{Out: writer}, Err: &stderr} - if code := w.Run(context.Background()); code != ExitOperational { - t.Fatalf("code=%d", code) +func TestCITimeoutIsOperationalAndDoesNotFix(t *testing.T) { + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, agent, &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{repository.CITimeout}}) + if code != ExitOperational || agent.ciFixes != 0 || !strings.Contains(output, "stage_completed stage=ci status=timeout") || !strings.Contains(output, "timeout_ms=60000") || !strings.Contains(output, "status=ci_timeout exit_code=2") { + t.Fatalf("code=%d fixes=%d output=%s", code, agent.ciFixes, output) } } -type failOnLivenessWriter struct{ bytes.Buffer } - -func (w *failOnLivenessWriter) Write(p []byte) (int, error) { - if strings.Contains(string(p), "CI recovery still running") { - return 0, errors.New("closed stdout") +func TestUnknownCIResultIsOperational(t *testing.T) { + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, agent, &workflowRepository{changes: []bool{true}, ci: []repository.CIResult{"unknown"}}) + if code != ExitOperational || agent.ciFixes != 0 || !strings.Contains(output, "status=operational_failure") { + t.Fatalf("code=%d fixes=%d output=%s", code, agent.ciFixes, output) } - return w.Buffer.Write(p) } -func TestCIFixLivenessWriteFailureIsOperational(t *testing.T) { - started := make(chan struct{}) - agent := &fakeAgent{ - reviews: []codex.ReviewResult{clean()}, - finalizations: []codex.Finalization{ciFailed()}, - ciFixWait: true, - ciFixStarted: started, - } - writer := &failOnLivenessWriter{} - var stderr bytes.Buffer - logger := &event.Logger{ - Out: writer, Format: "human", Heartbeat: time.Millisecond, - } - w := Workflow{Config: config.Config{LogFormat: "human", Heartbeat: time.Millisecond, MaxCIRecoveries: 1}, Agent: agent, Log: logger, Err: &stderr} - result := make(chan int, 1) - go func() { result <- w.Run(context.Background()) }() - <-started - select { - case code := <-result: - if code != ExitOperational { - t.Fatalf("code=%d output=%q stderr=%q", code, writer.String(), stderr.String()) - } - case <-time.After(time.Second): - t.Fatalf("CI liveness write failure did not stop the workflow: output=%q stderr=%q", writer.String(), stderr.String()) - } - if !strings.Contains(stderr.String(), "write liveness") { - t.Fatalf("stderr=%q", stderr.String()) +func TestPreexistingDirtyWorktreeIsNotCommitted(t *testing.T) { + repo := &workflowRepository{clean: []bool{false}, changes: []bool{true}, publishErr: errors.New("refuse dirty worktree")} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}}, repo) + if code != ExitOperational || repo.publishes != 1 || !strings.Contains(output, "status=operational_failure") { + t.Fatalf("code=%d publishes=%d output=%s", code, repo.publishes, output) } } -func TestCancellationStopsActiveLivenessWithoutLateWrites(t *testing.T) { - ticks := make(chan time.Time, 2) - started := make(chan struct{}) - agent := &fakeAgent{reviewWait: true, reviewStarted: started} - var output, stderr bytes.Buffer - logger := &event.Logger{ - Out: &output, Format: "human", Heartbeat: time.Second, - Tick: func(time.Duration) (<-chan time.Time, func()) { return ticks, func() {} }, - } - w := Workflow{Config: config.Config{LogFormat: "human", Heartbeat: time.Second}, Agent: agent, Log: logger, Err: &stderr} - ctx, cancel := context.WithCancel(context.Background()) - result := make(chan int, 1) - go func() { result <- w.Run(ctx) }() - <-started - cancel() - if code := <-result; code != ExitInterrupted { - t.Fatalf("code=%d output=%q stderr=%q", code, output.String(), stderr.String()) - } - before := output.String() - ticks <- time.Now().Add(time.Minute) - if after := output.String(); after != before { - t.Fatalf("late output after cancellation: before=%q after=%q", before, after) - } - if !strings.Contains(before, "Cancelled") || strings.Contains(before, "Review failed") || strings.Contains(before, "Failed due to an operational error") { - t.Fatalf("missing cancellation terminal output: %q", before) - } -} - -func TestCancellationEmitsCancelledKVResult(t *testing.T) { - started := make(chan struct{}) - agent := &fakeAgent{reviewWait: true, reviewStarted: started} - var output, stderr bytes.Buffer - w := Workflow{Config: config.Config{}, Agent: agent, Log: &event.Logger{Out: &output}, Err: &stderr} - ctx, cancel := context.WithCancel(context.Background()) - result := make(chan int, 1) - go func() { result <- w.Run(ctx) }() - <-started - cancel() - if code := <-result; code != ExitInterrupted { - t.Fatalf("code=%d output=%q stderr=%q", code, output.String(), stderr.String()) - } - assertRecord(t, output.String(), "event=run_completed", "status=cancelled", "exit_code=130") - if strings.Contains(output.String(), "event=review_completed") || stderr.Len() != 0 { - t.Fatalf("output=%q stderr=%q", output.String(), stderr.String()) - } -} - -func TestCIFixResetsPhaseAndFixes(t *testing.T) { - agent := &fakeAgent{ - reviews: []codex.ReviewResult{clean(), findings(), clean()}, - finalizations: []codex.Finalization{ciFailed(), success()}, - } - code, output, _ := run(t, config.Config{MaxCycles: 1, MaxCIRecoveries: 1}, agent) - if code != ExitSuccess { - t.Fatalf("code=%d", code) - } - assertRecord(t, output, "event=stage_started", "stage=review", "review_phase=2", "cycle=1") - if agent.fixCalls != 1 { - t.Fatalf("expected one fix attempt in second phase, got %d", agent.fixCalls) - } -} - -func assertRecord(t *testing.T, output string, fragments ...string) { - t.Helper() - for _, line := range strings.Split(output, "\n") { - matched := true - for _, fragment := range fragments { - if !strings.Contains(line, fragment) { - matched = false - break - } - } - if matched { - return - } - } - t.Fatalf("no record contains %v:\n%s", fragments, output) -} - -func countRecords(output, fragment string) int { - count := 0 - for _, line := range strings.Split(output, "\n") { - if strings.Contains(line, fragment) { - count++ - } +func TestCIProviderErrorIsOperationalAndDoesNotFix(t *testing.T) { + agent := &workflowAgent{reviews: []codex.ReviewResult{cleanReview()}} + code, output := runWorkflow(t, config.Config{CITimeout: time.Minute}, agent, &workflowRepository{changes: []bool{true}, ciErr: errors.New("authentication failed")}) + if code != ExitOperational || agent.ciFixes != 0 || !strings.Contains(output, "status=operational_failure") { + t.Fatalf("code=%d fixes=%d output=%s", code, agent.ciFixes, output) } - return count } diff --git a/memory-bank/adr/ADR-002-deterministic-delivery-orchestration.md b/memory-bank/adr/ADR-002-deterministic-delivery-orchestration.md new file mode 100644 index 0000000..11da772 --- /dev/null +++ b/memory-bank/adr/ADR-002-deterministic-delivery-orchestration.md @@ -0,0 +1,41 @@ +--- +title: "ADR-002: Deterministic delivery orchestration" +doc_kind: adr +doc_function: canonical +purpose: "Records the reusable ownership boundary between Code Converge and Codex for delivery lifecycle operations." +derived_from: + - ../features/FT-039/brief.md + - ../features/FT-039/design.md +status: active +decision_status: accepted +date: 2026-07-31 +audience: humans_and_agents +must_not_define: + - implementation_plan +--- + +# ADR-002: Deterministic delivery orchestration + +## Context + +Commit/push/PR/CI decisions were delegated to a Codex finalization session. That couples deterministic host operations to model-session duration and sandbox permissions, including linked-worktree Git metadata outside a model workspace. + +## Decision + +Code Converge owns deterministic repository and delivery lifecycle orchestration: repository inspection, safe checkpoint/commit decisions, remote/branch resolution, push, pull-request discovery/creation, and CI polling/classification. Codex owns review, code modification, and diagnosis/remediation of findings or failed CI. + +Deterministic operations may run `git` and `gh` child processes, but Code Converge constructs, observes, retries and classifies them. The Finalize Codex stage and its configuration are removed. + +## Consequences + +Publication and CI lifetime are governed by the CLI deadline and cancellation context, not a model turn. Linked-worktree mutations occur in the host process. Users must remove obsolete finalize settings. + +## Alternatives + +- Increase Codex finalizer timeout: rejected; it does not solve sandbox ownership or deterministic classification. +- Grant Codex broad filesystem access: rejected; it needlessly widens model command authority. + +## Related links + +- [FT-039 brief](../features/FT-039/brief.md) +- [FT-039 design](../features/FT-039/design.md) diff --git a/memory-bank/adr/README.md b/memory-bank/adr/README.md index cd9d037..3fa96db 100644 --- a/memory-bank/adr/README.md +++ b/memory-bank/adr/README.md @@ -21,6 +21,7 @@ audience: humans_and_agents ## Current records - [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. ## Naming diff --git a/memory-bank/domain/README.md b/memory-bank/domain/README.md index 2a1f419..d685348 100644 --- a/memory-bank/domain/README.md +++ b/memory-bank/domain/README.md @@ -34,7 +34,7 @@ Domain-документы не определяют market positioning, product Пример для `code-converge`: - Product: уменьшить ручную координацию agent-development loop. -- Domain: finalization начинается только после review без findings. +- Domain: publication начинается только после review без findings. ## Граница С Engineering diff --git a/memory-bank/domain/context-map.md b/memory-bank/domain/context-map.md index 3aebd1d..e7ab1de 100644 --- a/memory-bank/domain/context-map.md +++ b/memory-bank/domain/context-map.md @@ -23,7 +23,7 @@ The current product has one domain context. Codex, Git, repository hosting, and | Context | Owns language / rules for | Upstream contexts | Downstream contexts | Must not know | | --- | --- | --- | --- | --- | -| `Review Orchestration` | Run, stage, review cycle, finding/severity, finalization verdict, workflow transitions, exit outcomes, and configuration precedence | No other code-converge-owned context | No other code-converge-owned context | Internal state or credentials of Codex, Git, repository hosting, or CI | +| `Review Orchestration` | Run, stage, review cycle, finding/severity, publication and CI outcomes, workflow transitions, exit outcomes, and configuration precedence | No other code-converge-owned context | No other code-converge-owned context | Internal state or credentials of Codex, Git, repository hosting, or CI | ## Context Relationships @@ -34,7 +34,7 @@ The current product has one domain context. Codex, Git, repository hosting, and ## Shared Kernel / Published Language - Shared kernel: N/A while code-converge has one domain context. -- Published language: the root [`README.md`](../../README.md) solely owns public CLI option names, exit codes, finalization verdicts, and stdout fields. Domain documents only interpret their meaning. +- Published language: the root [`README.md`](../../README.md) solely owns public CLI option names, exit codes, publication/CI outcomes, and stdout fields. Domain documents only interpret their meaning. ## Boundary Rules diff --git a/memory-bank/domain/glossary.md b/memory-bank/domain/glossary.md index 85d1720..cdbb7f1 100644 --- a/memory-bank/domain/glossary.md +++ b/memory-bank/domain/glossary.md @@ -22,31 +22,32 @@ These terms are used consistently across product, feature, engineering, and oper | Term | Meaning | Context | Do not confuse with | | --- | --- | --- | --- | | `run` | One invocation of the main `code-converge` workflow from start to a terminal outcome. | Workflow, logs, exit policy | A single Codex subprocess invocation | -| `stage` | One review, fix-findings, finalization, or CI-fix operation within a run. | Workflow and timing | A deployment environment | +| `stage` | One review, fix-findings, publish, CI, or CI-fix operation within a run. | Workflow and timing | A deployment environment | | `review` | The stage that asks the configured agent to inspect the current repository and reports zero or more findings. | Review workflow | A hosted change-request approval or human review | | `finding` | One code-review issue reported for the current review. It contributes to the review's total and one severity bucket. | Review result and metrics | A persistent issue-tracker item | | `severity` | The finding classification counted as `critical`, `high`, `medium`, `low`, or `unknown` in the public reporting contract. | Review metrics | Agent reasoning effort or process exit status | -| `clean review` | A completed review with zero findings. | Transition into finalization | A successful overall run | +| `clean review` | A completed review with zero findings. | Transition into host-owned publication | A successful overall run | | `review cycle` | One review attempt and, when permitted and needed, its following fix-findings attempt. | Cycle limit and trend reporting | A CI-recovery attempt or the whole run | | `fix findings` | The stage that asks the agent to address findings from the preceding review. | Review loop | CI recovery | -| `finalization` | The stage after a clean review that asks the agent to commit, push, create a hosted change request when needed, and establish the CI result. | Publication workflow | Process cleanup or merely exiting the CLI | -| `finalization verdict` | One of `SUCCESS`, `CI_FAILED`, or `FAILED`, used to select the next workflow transition. | Finalization | The CLI process exit code | -| `CI recovery` | The fix-CI stage entered after finalization reports `CI_FAILED`; a successful recovery returns the run to review. | CI failure path | Re-running CI without reviewing resulting changes | +| `publication` | The host-owned stage after a clean review that safely commits eligible work, pushes, and reuses or creates one pull request. | Publication workflow | A Codex stage or a local checkpoint | +| `CI wait` | Host polling of applicable check-runs for the exact published head SHA. | CI workflow | A general repository-health query | +| `CI timeout` | The CI wait deadline elapsed before a terminal classification; it is operational, not red CI. | CI workflow | A failed check or CI recovery | +| `CI recovery` | The Fix-CI stage entered after deterministic CI polling finds a failed check; a successful recovery returns the run to review. | CI failure path | Re-running CI without reviewing resulting changes | | `effective configuration` | The resolved value and source for each setting after precedence is applied. | `code-converge config` and run setup | A single config file's contents | ## Naming Rules - Use `finding`, not `remark`, `comment`, or `issue`, when referring to a review result counted by the workflow. -- Use the stage names `review`, `fix-findings`, `finalize`, and `fix-ci` in externally visible records unless the public log contract changes. -- Do not use `success` without identifying whether it means a successful stage, finalization verdict, or terminal run outcome. +- Use the stage names `review`, `fix-findings`, `publish`, `ci`, and `fix-ci` in externally visible records unless the public log contract changes. +- Do not use `success` without identifying whether it means a successful stage, CI outcome, or terminal run outcome. ## Ambiguous Terms | Term | Allowed meaning | Forbidden / overloaded meaning | Replacement | | --- | --- | --- | --- | | `cycle` | Review cycle as defined above | Whole run or CI recovery | `run`, `review cycle`, or `CI recovery` | -| `success` | Qualified success of a named stage or run | Any agent process that exited without proving the required outcome | `stage success`, `SUCCESS` verdict, or `run success` | -| `CI failed` | The `CI_FAILED` finalization verdict when publication succeeded but CI is red | Any failure to invoke, inspect, or repair CI | Name the process/integration failure explicitly | +| `success` | Qualified success of a named stage or run | Any agent process that exited without proving the required outcome | `stage success` or `run success` | +| `CI failed` | A completed applicable check with an unaccepted conclusion | Timeout, provider failure, or cancellation | Name the process/integration failure explicitly | | `code-converge` | The CLI/project | A human code code-converge | `human code-converge` for the person | ## Source Documents diff --git a/memory-bank/domain/model.md b/memory-bank/domain/model.md index 8700a8f..c50d58b 100644 --- a/memory-bank/domain/model.md +++ b/memory-bank/domain/model.md @@ -22,11 +22,11 @@ canonical_for: | Review phase | value | A bounded review/fix convergence sequence | Starts initially and again after a successful CI recovery. | | Review cycle | value | One review followed by an optional finding fix | Belongs to a review phase. | | Finding | value | A code-review remark parsed from the schema-valid Codex final response | Has one normalized severity; contributes to cycle counts. | -| Stage | stateful operation | Review, fix findings, finalization, or CI fix | Produces a typed result and duration. | -| Finalization verdict | value | `SUCCESS`, `CI_FAILED`, or `FAILED` | Determines terminal success, CI recovery, or exit `2`. | +| Stage | stateful operation | Review, fix findings, publish, wait for CI, or CI fix | Produces a typed result and duration. | +| CI outcome | value | `success`, `skipped`, `failed`, or `timeout` | Determines terminal success, CI recovery, or operational exit `2`. | | Configuration value | value | Option plus source | Resolves once per run and is shown by `code-converge config`. | ## Boundaries -- Codex, Git remotes, hosting providers, and CI systems are external. `code-converge` owns their invocation contract and interpretation, not their internal state. No particular hosting provider is a required Memory Bank or domain boundary. +- Codex, Git remotes, hosting providers, and CI systems are external. `code-converge` owns deterministic Git/GitHub invocation, retry, and classification; Codex owns review and remediation. No particular hosting provider is a required Memory Bank or domain boundary. - A finding is not a persistent issue tracker item; it exists as a classified result for the current run. diff --git a/memory-bank/domain/rules.md b/memory-bank/domain/rules.md index cf44715..615f5b4 100644 --- a/memory-bank/domain/rules.md +++ b/memory-bank/domain/rules.md @@ -15,12 +15,12 @@ canonical_for: # Domain Rules -- `RULE-01`: Finalization starts only after a completed review with zero findings and either Git status confirms staged, unstaged or untracked changes or the run created a local findings-fix checkpoint. A clean worktree with no checkpoint exits successfully without finalization. -- `RULE-02`: Before an automatic findings-fix stage, Git status determines checkpoint eligibility. A clean worktree may receive one local checkpoint commit after a successful fix; a dirty worktree still receives remediation but skips the checkpoint to avoid capturing pre-existing work. Checkpoints never push, checkpoint-operation failures are operational, and publication remains finalization after clean review. +- `RULE-01`: Publication starts only after a completed review with zero findings and either Git status confirms staged, unstaged or untracked changes or the run created a local findings-fix checkpoint. A clean worktree with no checkpoint exits successfully without publication. +- `RULE-02`: Before an automatic findings-fix stage, Git status determines checkpoint eligibility. A clean worktree may receive one local checkpoint commit after a successful fix; a dirty worktree still receives remediation but skips the checkpoint to avoid capturing pre-existing work. Checkpoints never push, checkpoint-operation failures are operational, and Code Converge owns publication after clean review. - `RULE-03`: `max-cycles` limits fix-findings attempts in one review phase. The final allowed fix is followed by a verification review; remaining findings then exit `1`. - `RULE-04`: A successful CI fix starts a new review phase with a fresh review budget, preserving the possibility that the fix introduced findings. `max-ci-recoveries` bounds these restarts. -- `RULE-05`: Only finalization may produce `SUCCESS`, `CI_FAILED`, or `FAILED`; an unrecognized agent response is not any of these verdicts. -- `RULE-06`: A successful finalization exits `0` when required CI is green or CI is not applicable. Operational/finalization failure exits `2`; failed or exhausted CI recovery exits `3`. +- `RULE-05`: Code Converge classifies repository publication and exact-head CI itself; Codex only reviews, modifies code, and remediates failed CI. +- `RULE-06`: Green or skipped CI exits `0`; CI timeout and provider/publication failures exit `2`; failed or exhausted CI recovery exits `3`. - `RULE-07`: Each successfully classified review emits total findings and zero-filled counts for `critical`, `high`, `medium`, `low`, and `unknown`. A failed or ambiguous review emits no unreliable counters. - `RULE-08`: Each completed stage emits an elapsed duration; the terminal event emits total run duration and exit code. - `RULE-09`: Effective configuration follows the precedence contract owned by [`../../README.md`](../../README.md). diff --git a/memory-bank/domain/states.md b/memory-bank/domain/states.md index 82c42fe..adabcea 100644 --- a/memory-bank/domain/states.md +++ b/memory-bank/domain/states.md @@ -20,17 +20,19 @@ stateDiagram-v2 [*] --> Review: resolve base and private snapshot Review --> FixFindings: findings and fix budget remaining FixFindings --> Review: success - Review --> Finalize: clean report and changes exist + Review --> Publish: clean report and changes exist Review --> Exit0: clean report and no changes Review --> Exit1: findings after final fix Review --> Exit2: command/report failure FixFindings --> Exit2: command failure - Finalize --> Exit0: SUCCESS - Finalize --> FixCI: CI_FAILED and recovery budget remains - Finalize --> Exit3: CI_FAILED and recovery budget exhausted - Finalize --> Exit2: FAILED + Publish --> WaitCI: published revision + Publish --> Exit2: publication failure + WaitCI --> Exit0: all accepted or no applicable checks + WaitCI --> FixCI: failed check and recovery budget remains + WaitCI --> Exit3: failed check and recovery budget exhausted + WaitCI --> Exit2: timeout or provider failure FixCI --> Review: success FixCI --> Exit3: failure ``` -CI transitions are applicable only when the target repository has required CI. When no required CI exists, finalization reports success with the CI step marked `skipped`. Hosting-provider-specific behavior is an adapter concern, not a domain state. +CI polling is pinned to the published head SHA. When GitHub returns no check-runs for that SHA, Code Converge records `skipped`; CI timeout is operational and does not enter Fix CI. diff --git a/memory-bank/engineering/architecture.md b/memory-bank/engineering/architecture.md index 8c80bc5..0585922 100644 --- a/memory-bank/engineering/architecture.md +++ b/memory-bank/engineering/architecture.md @@ -22,7 +22,7 @@ The product is a Go CLI that coordinates a sequential state machine. It uses `go | --- | --- | --- | | CLI boundary (`cmd/code-converge`, `internal/app`) | Argument parsing, command selection, signal context, dependency wiring | Workflow transition policy and agent-report interpretation | | Configuration resolution (`internal/config`) | Settings sources, precedence, source metadata, validated Git root | Ad hoc per-stage configuration lookup | -| Codex boundary (`internal/codex`) | Schema-constrained command invocation with a prepared review target, strict final-response-file classification, strict finalization response parsing | Exit-code policy and workflow stdout formatting | +| Codex boundary (`internal/codex`) | Schema-constrained review invocation with a prepared review target and remediation invocation | Deterministic Git/GitHub lifecycle decisions, exit-code policy, and workflow stdout formatting | | Repository status and review discovery (`internal/repository`) | Git status query, local findings-fix checkpoint commit, deterministic base discovery and a disposable merge-base-to-worktree index snapshot | Workflow transition policy, remote publication, and Codex-output interpretation | | Workflow orchestration (`internal/workflow`) | State transitions, budgets, stage timing and exit outcomes | Subprocess mechanics | | Process runner (`internal/runner`) | Working directory, context cancellation, captured stdin/stdout/stderr, live observer chunks, exit status and private-stage context | Agent-report interpretation or terminal layout | @@ -33,7 +33,7 @@ Review uses `codex exec` with a caller-supplied strict schema and per-invocation The Codex boundary forces a wrapper-prefixed `PATH` plus neutral `SHELL`, `ZDOTDIR`, `BASH_ENV`, and `ENV` values through `shell_environment_policy.set`, disables login-shell startup, and removes inherited Git repository/index/config transports and exported shell functions for the review. Login and non-login startup files or caller state therefore cannot discard, replace, or redirect the scoped transport. Its private root, index and Git executable are sidecar data next to the wrapper, so an `include_only` policy that permits `PATH` needs no additional helper variables. `GIT_INDEX_FILE` is never exported to Codex. The PATH wrapper directory contains only the symlinked `git` helper, which runs from the installed executable rather than the temporary directory; all `git-*` helpers are linked into a separate child-only `GIT_EXEC_PATH` directory, and setup fails before review if either temporary directory contains a platform path-list separator or any sidecar path cannot be represented losslessly as UTF-8. The helper resolves documented Git global options including both `--namespace` and `--attr-source` forms plus `--list-cmds=`, while unknown or malformed options fail closed. It rejects reviewed-root commands and aliases that explicitly enable split-index before Git can create shared-index state. It applies the private index only after confirming the reviewed repository; other targets, repository-creation commands, and unclassifiable external subcommands use their normal index. It sets `GIT_EXEC_PATH` only within its child Git process so aliases and hooks continue through the wrapper without exposing that setting to Codex policy. Commands classified outside the review index carry a child-only no-index marker so helpers such as `git-submodule` cannot re-enable the scoped index through a descendant wrapper; any inherited copy of that marker is removed before Codex starts. All other user policy selections remain intact. -After a zero process exit, the Codex boundary classifies only the exact validated structured response file; terminal streams, prose, missing or invalid files, and non-zero invocations cannot select a result. Before automatic remediation, the repository collaborator checks whether a checkpoint can safely be attributed to the fix stage. A dirty baseline continues remediation but skips checkpointing; a clean baseline may create a local checkpoint commit and never publishes it. After a clean classification, repository status or a run-local checkpoint determines whether finalization is applicable. Finalization keeps the exact verdict contract from the root README because that verdict controls workflow transitions and is the only publication path. +After a zero process exit, the Codex boundary classifies only the exact validated review-response file; terminal streams, prose, missing or invalid files, and non-zero invocations cannot select a result. Before automatic remediation, the repository collaborator checks whether a checkpoint can safely be attributed to the fix stage. A dirty baseline continues remediation but skips checkpointing; a clean baseline may create a local checkpoint commit and never publishes it. After clean review, the repository collaborator owns commit eligibility, direct-ref push, pull-request discovery/creation, and exact-head GitHub check-run polling; the workflow selects Fix CI only for a deterministic failed CI result. External process execution is a trust boundary. The runner preserves the operator's invocation directory, captures stdin/stdout/stderr, emits optional live source-labelled chunks only to the interactive presentation observer, propagates context cancellation, and never forwards raw Codex output to workflow stdout. Code-Converge does not add a timeout or override Codex sandbox, approval, or network configuration. Publication behavior remains hosting-provider-neutral. diff --git a/memory-bank/engineering/git-workflow.md b/memory-bank/engineering/git-workflow.md index adc8de1..fcc6a79 100644 --- a/memory-bank/engineering/git-workflow.md +++ b/memory-bank/engineering/git-workflow.md @@ -29,7 +29,7 @@ The project's current remote default branch is `master` as of the Memory Bank in - Run applicable canonical checks from [`testing-policy.md`](testing-policy.md) before publication and record any unavailable check as a gap. - Use a short subject that identifies the delivered outcome. - Record what changed, verification evidence, and remaining risks or manual gaps in the hosted change request. -- When the target repository has applicable required CI, the `code-converge` workflow treats its green result as part of successful finalization. When it does not, the CI step is not applicable. Branch protection and required-check configuration belong to the target repository and hosting provider. +- When the target repository has applicable CI for the published revision, the `code-converge` workflow treats accepted terminal results as part of successful completion. When no check-runs exist for that revision, CI is skipped. Branch protection and required-check configuration belong to the target repository and hosting provider. ## Worktrees diff --git a/memory-bank/features/FT-039/README.md b/memory-bank/features/FT-039/README.md new file mode 100644 index 0000000..d00a87e --- /dev/null +++ b/memory-bank/features/FT-039/README.md @@ -0,0 +1,18 @@ +--- +title: "FT-039: Deterministic delivery orchestration" +doc_kind: feature +doc_function: index +purpose: "Routing index for Code Converge-owned publication and CI orchestration." +derived_from: + - ../../flows/feature.md + - ../../../README.md +status: active +audience: humans_and_agents +--- + +# FT-039: Deterministic delivery orchestration + +- [brief.md](brief.md) — canonical problem, scope and verification contract. +- [design.md](design.md) — selected publication, CI and failure semantics. +- [implementation-plan.md](implementation-plan.md) — execution and validation plan. +- [ADR-002](../../adr/ADR-002-deterministic-delivery-orchestration.md) — accepted reusable ownership rule. diff --git a/memory-bank/features/FT-039/brief.md b/memory-bank/features/FT-039/brief.md new file mode 100644 index 0000000..18794a3 --- /dev/null +++ b/memory-bank/features/FT-039/brief.md @@ -0,0 +1,66 @@ +--- +title: "FT-039: Deterministic delivery orchestration" +doc_kind: feature +doc_function: canonical +purpose: "Canonical problem, scope, validation profile and verification contract for GH-39." +derived_from: + - ../../flows/feature.md + - ../../engineering/testing-policy.md + - ../../../README.md + - https://github.com/dapi/code-converge/issues/39 +status: active +delivery_status: in_progress +audience: humans_and_agents +must_not_define: + - implementation_sequence + - solution_space +--- + +# FT-039: Deterministic delivery orchestration + +## What + +Codex currently owns deterministic commit, push, pull-request and CI operations. Its sandbox and session lifetime make those host-process responsibilities unreliable. Code Converge must perform and classify them after a clean review, retaining Codex for review and remediation. + +## Scope + +- `REQ-01` Remove the Codex Finalize stage and obsolete finalize configuration without silent no-op compatibility. +- `REQ-02` After a clean review, repository code safely commits only a clean worktree, resolves branch/remote, pushes, and finds or creates one matching GitHub PR. +- `REQ-03` Wait for checks pinned to the published head SHA, classifying green/skipped, failed, timeout, provider failure and cancellation deterministically. +- `REQ-04` Add `--ci-timeout`, `CODE_CONVERGE_CI_TIMEOUT`, and `.code-converge/ci-timeout`, defaulting to `60m` under existing precedence. +- `REQ-05` A failed check enters Fix CI; timeout is operational and never invokes Fix CI. + +## Non-Scope + +- `NS-01` Other hosting providers, CI providers, or broader Codex remediation redesign. +- `NS-02` Automatically committing a dirty worktree that existed before publication. + +## Design Requirement Decision + +| Decision | Reason | Downstream owner | +| --- | --- | --- | +| `Design required: yes` | CLI/config/event contracts, workflow transitions, provider connector and timeout semantics change. | `design.md` | + +## Validation Profile Decision + +Validation profile: `standard`. + +Triggers / rationale: public workflow/configuration/event contracts and GitHub integration require end-to-end fake coverage. + +Downgrade approval: none. + +## Verify + +| Scenario | Observable result | +| --- | --- | +| `SC-01` | Clean reviewed changes are committed/pushed and one matching PR is reused or created without a Codex finalizer. | +| `SC-02` | A failed head-pinned check promptly starts Fix CI; a clean fix returns to review and publication. | +| `SC-03` | All successful/skipped head-pinned checks succeed; no checks skip; deadline emits timeout/exit 2. | +| `SC-04` | Dirty worktree, ambiguous identity, provider errors and cancellation fail safely. | + +| Check | Evidence | +| --- | --- | +| `CHK-01` | `go test ./...` | +| `CHK-02` | `go vet ./...` | +| `CHK-03` | `make docs-lint` | +| `CHK-04` | `git diff --check` | diff --git a/memory-bank/features/FT-039/design.md b/memory-bank/features/FT-039/design.md new file mode 100644 index 0000000..2c23abe --- /dev/null +++ b/memory-bank/features/FT-039/design.md @@ -0,0 +1,77 @@ +--- +title: "FT-039: Design" +doc_kind: feature +doc_function: canonical +purpose: "Selected deterministic publication and CI orchestration design for GH-39." +derived_from: + - brief.md + - ../../engineering/architecture.md + - ../../adr/ADR-002-deterministic-delivery-orchestration.md +status: active +audience: humans_and_agents +must_not_define: + - ft_039_scope + - ft_039_acceptance_criteria + - implementation_sequence +--- + +# FT-039: Design + +## Design pack + +| Artifact | Role | Owns | +| --- | --- | --- | +| `design.md` | Feature solution | `SOL-*`, contracts, invariants and failure modes | +| [ADR-002](../../adr/ADR-002-deterministic-delivery-orchestration.md) | Reusable architectural rule | Ownership boundary | + +## C4 applicability + +`C4-00`: not required. Existing CLI, workflow, repository and runner components retain their boundaries; GitHub CLI is an existing child-process connector. + +## Selected solution + +- `SOL-01`: Replace `Agent.Finalize` with `Repository.Publish(ctx)`. It commits only when status is clean, detects no-op commits, resolves current branch/remote, then uses `gh` to reuse/create one open PR. +- `SOL-02`: `Repository.WaitCI(ctx, publishedSHA, timeout)` polls provider data for the exact head revision. It retries transient failures within the deadline, returns failure immediately, green only when all checks are terminal accepted states, skipped when there are no checks, and timeout otherwise. +- `SOL-03`: Workflow emits repository-owned publication and CI outcomes. `failed` starts Fix CI; `timeout` is operational. +- `SOL-04`: Remove finalization model/effort/prompt CLI/env/file/profile/config settings. This is a breaking removal, not a deprecated no-op. + +## Architecture coverage + +| Aspect | Status | Notes | +| --- | --- | --- | +| Components | covered | workflow selects transitions; repository executes Git/GitHub commands; event renders output. | +| Connectors | covered | synchronous `git` and `gh` child processes; JSON is parsed/classified locally. | +| Configuration | covered | `ci-timeout` follows the common resolver. | +| Behavioral semantics | covered | contracts, invariants and failure modes below. | +| Quality/evolution | covered | deadline, retry, cancellation and explicit breaking migration. | + +## Contracts, invariants and failures + +- `CTR-01`: Publication returns commit, push, PR and head SHA. A remote head observed after a push is success even if local tracking-ref refresh reports an error. +- `CTR-02`: CI only classifies checks for the exact published SHA; stale data is retried until deadline. +- `INV-01`: A dirty worktree is never automatically committed at publication. +- `INV-02`: Success requires every applicable check to be terminal `success|skipped|neutral`. +- `INV-03`: The first applicable failure enters Fix CI; timeout never does. +- `INV-04`: Context cancellation reaches active child processes and yields exit 130. +- `FM-01`: Ambiguous remote, branch or PR identity; provider auth/protocol error → operational failure. +- `FM-02`: CI deadline → `ci=timeout`, operational exit 2. +- `FM-03`: No applicable checks → `ci=skipped`. + +## Public and agent-contract compatibility + +- `--ci-timeout`, `CODE_CONVERGE_CI_TIMEOUT`, and `.code-converge/ci-timeout` resolve under the established CLI > project > user > environment > default precedence. The built-in default is `60m`; values below one second are configuration errors. +- `finalize-model`, `finalize-reasoning-effort`, `finalize-prompt-file`, their environment variables, profile entries, configuration files, help entries, and `config` output are removed. This is an intentional breaking migration: obsolete environment and configuration-file settings fail configuration with an actionable removal diagnostic; they are never silently ignored. +- Codex receives only Review, Fix findings, and Fix CI prompts. There is no finalization prompt, output schema, verdict, or Codex process after a clean review. +- The public state/event replacement is `publish` followed by `ci`. Publication emits deterministic `commit`, `push`, and `change_request` step outcomes. CI emits `success`, `skipped`, `failed`, or `timeout`; `timeout` produces `run_completed status=ci_timeout exit_code=2`. + +## Provider connector and temporal semantics + +- Publication resolves the current branch and one push remote; detached HEAD, missing or ambiguous remotes, malformed PR data, and multiple matching open PRs are operational errors. It uses `git push HEAD:refs/heads/` so local tracking-ref maintenance is not used as proof of remote publication. +- PR discovery, creation, and CI queries are bound to the resolved push remote's uniquely parsed GitHub `owner/repository` identity. A pre-existing matching open PR is reused; exactly one newly created PR URL is accepted. +- The CI deadline starts after the published SHA is known. Every poll collects every page from GitHub's check-runs endpoint for that SHA, never a branch or latest workflow run. Empty check-runs are `skipped`; only completed `success`, `skipped`, and `neutral` conclusions are accepted. +- Retryable transport/provider failures back off within the same deadline. Authentication, authorization, identity, malformed-protocol, and malformed-data failures are operational immediately. The first completed unaccepted check conclusion returns `failed` without waiting for other checks. +- The workflow's cancellation context reaches every `git` and `gh` child process. Cancellation wins over classification and produces exit `130`; deadline expiry is distinct from cancellation and CI failure. + +## Rollout and backout + +This is a breaking configuration migration. Release notes and root help direct operators to delete obsolete Finalize settings before upgrading. No remote data migration is required. Backout is a source revert; an already-published branch or PR is not deleted automatically. diff --git a/memory-bank/features/FT-039/implementation-plan.md b/memory-bank/features/FT-039/implementation-plan.md new file mode 100644 index 0000000..7abd027 --- /dev/null +++ b/memory-bank/features/FT-039/implementation-plan.md @@ -0,0 +1,21 @@ +--- +title: "FT-039: Implementation plan" +doc_kind: feature +doc_function: derived +purpose: "Execution sequence and verification for deterministic publication and CI orchestration." +derived_from: + - brief.md + - design.md +status: active +audience: humans_and_agents +--- + +# FT-039: Implementation plan + +| Step | Scope | Evidence / stop condition | +| --- | --- | --- | +| `STEP-01` | Accept ADR-002, remove Finalize settings/contract, add `ci-timeout` resolution and config/help migration. | Precedence and removal tests; stop if any obsolete setting remains operational. | +| `STEP-02` | Implement host-owned commit eligibility, direct-ref push, provider-bound PR reuse/create, and exact-SHA check-run polling. | Fake runner covers dirty baseline, detached/ambiguous identity, direct-ref push, PR ambiguity, URL parsing, no checks, green, first failure, stale SHA, transient/permanent provider failure, deadline, and cancellation. | +| `STEP-03` | Replace the workflow state/event transition with `publish` then `ci`; retain the bounded Fix-CI → Review loop. | Workflow tests cover success, skipped CI, failed CI recovery, exhausted recovery, timeout exit `2` without Fix CI, and exit `130`. | +| `STEP-04` | Verify a linked worktree publication path from the host process and document output/rollout/backout. | A linked-worktree test proves no Codex workspace write is needed for shared Git metadata; root README and canonical owners agree. | +| `STEP-05` | Run validation profile `standard`. | `go test ./...`, `go vet ./...`, `make docs-lint`, and `git diff --check`; record any environment-unavailable command as a gap. | diff --git a/memory-bank/features/README.md b/memory-bank/features/README.md index c045845..f01faf8 100644 --- a/memory-bank/features/README.md +++ b/memory-bank/features/README.md @@ -48,3 +48,4 @@ audience: humans_and_agents - [`FT-024/README.md`](FT-024/README.md) — completed local checkpoints for successful findings fixes, with publication deferred to clean-review finalization for issue #24. - [`FT-028/README.md`](FT-028/README.md) — active remediation of stale interactive liveness frames for issue #28 through footprint-aware clearing and deterministic reflow coverage. - [`FT-036/README.md`](FT-036/README.md) — planned discoverable root and subcommand CLI help for issue #36. +- [`FT-039/README.md`](FT-039/README.md) — active deterministic repository publication and CI orchestration for issue #39. diff --git a/memory-bank/ops/config.md b/memory-bank/ops/config.md index 45c83a5..0c1d54d 100644 --- a/memory-bank/ops/config.md +++ b/memory-bank/ops/config.md @@ -18,4 +18,4 @@ The root [`README.md`](../../README.md) solely owns configuration source precede `code-converge config` prints each effective value and its source. If the effective value differs from its built-in default, it prints that default too. -`codex` authentication and credentials for any configured Git remote or hosting provider are environment prerequisites, not `code-converge` configuration values. Provider-specific credentials are required only when the selected finalization workflow needs them. The application must not log secrets or token values. +`codex` authentication and credentials for any configured Git remote or GitHub provider are environment prerequisites, not `code-converge` configuration values. GitHub credentials are required for Code Converge's deterministic pull-request and CI operations. The application must not log secrets or token values. diff --git a/memory-bank/prd/PRD-001-code-converge-cli.md b/memory-bank/prd/PRD-001-code-converge-cli.md index fd16dbc..68f53e4 100644 --- a/memory-bank/prd/PRD-001-code-converge-cli.md +++ b/memory-bank/prd/PRD-001-code-converge-cli.md @@ -38,10 +38,10 @@ The project needs one bounded local workflow that drives this loop to an explici ## Goals -- `G-01` Provide one local CLI workflow that performs review, bounded finding remediation, finalization, and bounded CI recovery in the order required by the domain state machine. +- `G-01` Provide one local CLI workflow that performs review, bounded finding remediation, deterministic publication, and bounded CI recovery in the order required by the domain state machine. - `G-02` Produce an explicit terminal outcome that distinguishes success, remaining findings, operational failure, and CI-recovery failure. - `G-03` Treat ambiguous or internally inconsistent agent output as failure rather than inferred success. -- `G-04` Make stage progress, review severity counts, durations, finalization steps, terminal result and bounded long-stage liveness observable through the public stdout contract. +- `G-04` Make stage progress, review severity counts, durations, publication and CI outcomes, terminal result and bounded long-stage liveness observable through the public stdout contract. - `G-05` Let operators inspect and override effective configuration without starting a workflow run. ## Non-Goals @@ -59,8 +59,8 @@ The project needs one bounded local workflow that drives this loop to an explici - Invoke the configured local Codex review command with a strict final-response schema and safely classify only that response file as clean, findings, or failure. - Normalize finding priorities into the public severity buckets and report complete counters for every classified review. - Run bounded review/fix cycles, including the mandatory verification review after the final permitted fix. -- Finalize only after a clean review and interpret a constrained finalization result, including commit, push, change-request, and CI step outcomes. -- When publication succeeded but applicable required CI is red, run bounded CI recovery and restart review in a fresh review phase. +- After a clean review, deterministically commit eligible work, push, find or create a pull request, and classify CI for the published SHA. +- When applicable CI is red, run bounded CI recovery and restart review in a fresh review phase; a timeout remains operational. - Resolve settings from the documented CLI, project, user, environment, and built-in sources; expose them through `code-converge config`. - Emit the documented stdout records, diagnostics on stderr, and the specified exit codes. - Be buildable and distributable as a local Go CLI without requiring a Go runtime for a released binary. @@ -75,8 +75,8 @@ The project needs one bounded local workflow that drives this loop to an explici ## UX / Business Rules - `BR-01` Downstream delivery must preserve the workflow invariants and terminal outcomes owned by [`../domain/rules.md`](../domain/rules.md) and the transitions owned by [`../domain/states.md`](../domain/states.md). -- `BR-02` A clean review is necessary but not sufficient for run success; finalization must establish the documented successful terminal state. -- `BR-03` Unclassified review output, an unrecognized finalization verdict, or inconsistent finalization details must never be interpreted as success. +- `BR-02` A clean review is necessary but not sufficient for run success; deterministic publication and exact-head CI classification must establish the documented successful terminal state. +- `BR-03` Unclassified review output, ambiguous publication identity, or unclassified provider data must never be interpreted as success. - `BR-04` Fix-findings and CI-recovery loops are independently bounded; exhausting either budget produces its specified non-zero terminal outcome. - `BR-05` Operational stdout uses an explicitly selected human or structured format. Structured `kv` remains machine-readable and one-record-per-line; non-TTY human output is newline-safe and ANSI-free. Raw agent output and diagnostics do not contaminate either stream. - `BR-06` An operator can inspect every effective setting and its source before execution. @@ -97,7 +97,7 @@ These are contract-conformance targets for the complete utility. Adoption, time ## Risks And Open Questions - `RISK-01` Resolved by FT-022: review classification requires the exact schema-valid final-response file and never infers clean from Codex terminal prose. -- `RISK-02` Finalization delegates material Git, hosting, and CI actions to an agent; a process exit alone does not prove that required external outcomes occurred. +- `RISK-02` Publication and CI require deterministic observation of Git, hosting, and check outcomes; a child-process exit alone does not prove that required external outcomes occurred. - `RISK-03` The default models are not available to every Codex account, which can block first-run success unless configuration and diagnostics are clear. - `RISK-04` Repeated review and CI-recovery loops can consume substantial time and model budget even when correctly bounded. - `RISK-05` The intended user pain and adoption hypothesis are specified but not validated by interviews or usage analytics. @@ -108,10 +108,10 @@ These are contract-conformance targets for the complete utility. Adoption, time ## Downstream Delivery -The complete product is small enough to be delivered as one coherent delivery-unit through a single Feature Flow package. An Epic and separate feature packages for review, finalization, CI recovery, configuration, observability, or packaging would add coordination without creating independently useful product outcomes. +The complete product is small enough to be delivered as one coherent delivery-unit through a single Feature Flow package. An Epic and separate feature packages for review, deterministic publication, CI recovery, configuration, observability, or packaging would add coordination without creating independently useful product outcomes. Observability remains a cross-cutting acceptance requirement: every internal checkpoint that adds or changes a workflow transition must include its corresponding stdout records, stderr behavior, counters, and durations. The feature package must finish with contract-convergence verification across the complete run. | Delivery unit | Included outcome | Status | | --- | --- | --- | -| Code-Converge CLI complete delivery | Review/fix convergence, finalization, CI recovery, configuration inspection, operational records, terminal outcomes, reproducible binary, and distribution evidence | planned | +| Code-Converge CLI complete delivery | Review/fix convergence, deterministic publication, CI recovery, configuration inspection, operational records, terminal outcomes, reproducible binary, and distribution evidence | planned | diff --git a/memory-bank/product/README.md b/memory-bank/product/README.md index 08655b2..7491abc 100644 --- a/memory-bank/product/README.md +++ b/memory-bank/product/README.md @@ -34,7 +34,7 @@ Product-документы не определяют предметную мод Пример для `code-converge`: - Product: сократить ручную координацию review → fix → publish → CI. -- Domain: finalization разрешён только после review без findings. +- Domain: publication разрешена только после review без findings. ## Граница С PRD diff --git a/memory-bank/product/context.md b/memory-bank/product/context.md index a3979fa..1a633c0 100644 --- a/memory-bank/product/context.md +++ b/memory-bank/product/context.md @@ -31,7 +31,7 @@ The product boundary is orchestration of the local agent-development loop. It do - `PCON-01` The supported agent integration is the locally installed and authenticated Codex CLI. - `PCON-02` Every important step remains observable on stdout through an explicitly selected human or structured format; structured review trend data includes all severity counts and millisecond duration, while human output keeps the total, non-zero severities and readable duration. -- `PCON-03` Publication actions are delegated to the finalization agent and require credentials for the configured Git remote or hosting provider. No particular provider is required by the product contract. +- `PCON-03` Code Converge performs deterministic publication and CI polling through host `git` and GitHub CLI processes; Codex is limited to review and remediation. Publication requires credentials for the configured Git remote and GitHub provider. ## Sources diff --git a/memory-bank/product/metrics.md b/memory-bank/product/metrics.md index a113fe4..a9703a0 100644 --- a/memory-bank/product/metrics.md +++ b/memory-bank/product/metrics.md @@ -18,7 +18,7 @@ canonical_for: | --- | --- | --- | --- | | Findings total | Findings in a completed review | `findings_total` in `kv`; human review summary | Must be visible across cycles; it is not by itself a success metric. | | Findings by severity | Counts for critical/high/medium/low/unknown | `findings_*` in `kv`; non-zero buckets in human output | Shows how the remaining risk profile changes; monotonic decrease is not required. | -| Stage duration | Wall duration of a completed stage | `duration_ms` in `kv`; readable duration in human output | Measures review, fixing, finalization, and CI-recovery cost. | +| Stage duration | Wall duration of a completed stage | `duration_ms` in `kv`; readable duration in human output | Measures review, fixing, publication, CI polling, and CI-recovery cost. | | Run outcome | Final exit code and total duration | terminal event | Distinguishes successful completion from defined failure modes. | Metrics are emitted to stdout for a single run. Persistent collection, dashboards, and cross-run aggregation are out of scope until separately designed. diff --git a/memory-bank/product/roadmap.md b/memory-bank/product/roadmap.md index 635a3bc..3074d25 100644 --- a/memory-bank/product/roadmap.md +++ b/memory-bank/product/roadmap.md @@ -25,13 +25,13 @@ The roadmap contains one product outcome: deliver the complete `code-converge` C | Horizon | Theme | Intended outcome | Current owner | Dependency | Status | | --- | --- | --- | --- | --- | --- | -| `now` | Complete code-converge CLI | A distributable local Go CLI performs the full documented review/fix/finalization/CI-recovery workflow with observable terminal results | [`../features/FT-002/brief.md`](../features/FT-002/brief.md) | Implementation, deterministic tests, accepted PR and distribution evidence | completed | +| `now` | Complete code-converge CLI | A distributable local Go CLI performs the full documented review/fix/publication/CI-recovery workflow with observable terminal results | [`../features/FT-002/brief.md`](../features/FT-002/brief.md), [`../features/FT-039/brief.md`](../features/FT-039/brief.md) | Implementation, deterministic tests, accepted PR and distribution evidence | in progress | ## Roadmap Rules - Roadmap theme описывает product intent, а не implementation plan. - Deliver the documented CLI as one coherent delivery-unit. Internal checkpoints do not become separate feature packages or an Epic unless scope materially changes. -- Review/fix, finalization, CI recovery, configuration, observability, and packaging converge in the same feature-level acceptance contract. +- Review/fix, deterministic publication, CI recovery, configuration, observability, and packaging converge in the same feature-level acceptance contract. - Work beyond the documented CLI is not implied follow-up; route it separately only after an explicit product decision. - Если тема меняет предметную модель, сначала обнови [`../domain/model.md`](../domain/model.md), [`../domain/rules.md`](../domain/rules.md) или [`../domain/context-map.md`](../domain/context-map.md). diff --git a/memory-bank/product/vision.md b/memory-bank/product/vision.md index 51160f9..6840d9a 100644 --- a/memory-bank/product/vision.md +++ b/memory-bank/product/vision.md @@ -28,7 +28,7 @@ An operator should receive an explicit terminal result rather than having to int | Bet ID | Bet | Why now | Evidence | Review cadence | | --- | --- | --- | --- | --- | -| `BET-01` | Deliver the complete local review → fix → finalization → CI-recovery loop as one bounded utility. | This is the intended product outcome, not a stepping stone to broader scope. | [`context.md`](context.md), [`../../README.md`](../../README.md) | No ongoing feature cadence is planned. | +| `BET-01` | Deliver the complete local review → fix → deterministic publication → CI-recovery loop as one bounded utility. | This is the intended product outcome, not a stepping stone to broader scope. | [`context.md`](context.md), [`../../README.md`](../../README.md) | No ongoing feature cadence is planned. | | `BET-02` | Treat explicit outcomes and observable review trends as part of the product contract. | Operators must distinguish success from unresolved findings or stage failure. | [`metrics.md`](metrics.md), [`../../README.md`](../../README.md) | Revisit when implementation evidence exists. | ## Experience Principles