diff --git a/.tickets/_docs/ROADMAP.md b/.tickets/_docs/ROADMAP.md index 7018945..79bf967 100644 --- a/.tickets/_docs/ROADMAP.md +++ b/.tickets/_docs/ROADMAP.md @@ -35,6 +35,18 @@ replaces the terminal. with a 60s budget and **never** hard-kills — exhausting it injects anyway and leaves the pane inspectable. `CLI_HEALTH_SPECS` now probes the interactive codex path so the same drift can't ship silently again. +- `merge_to_main` pushed the wrong thing (2026-08-22). It ran + `git push origin HEAD:refs/heads/main` from `resolve_working_dir`, which is the + **shared workspace checkout** whenever the task has no worktree — and terminal + -column cleanup removes worktrees while `branch_name` survives. With the repo + on `main` that is a no-op push that *succeeds*, so the trigger logged + "Pushed to origin/main" having merged nothing; on any other checked-out + branch it would have published unrelated work to main. Now pushes the task's + branch by name. Verified against a real bare origin: before the fix origin/main + never moved, after it advanced and carried the branch's file. + `auto_merge` was checked and does not share the bug (it passes the branch + explicitly). **Note:** `merge_to_main` is backend-only — it is absent from the + frontend `ActionType` union, so no UI can configure it. - Script agents verified end to end + tmux session env fixed (2026-08-22). argv, cwd and exit-code advancement were already correct (no prose in argv — strict `ARGC` check passed), but **every environment variable arrived ``**: @@ -97,6 +109,17 @@ Empty. The last item — the stale `/usr/local/bin/claude` — was removed ## 🟡 Important (rough edges, not blockers) +2b. **Trigger failures surface as "Execution failed" on the card.** The real + reason exists — `[create_pr] Failed …: git push --force failed: fatal: + 'origin' does not appear to be a git repository` — but only in the log. + `handle_trigger_failure` propagates a specific message and does show it + (e.g. "Cannot create PR: task has no branch_name"), while the paths going + through `mark_complete_with_error` with `error_detail: None` fall back to the + generic string. Thread the detail through so the card says what actually + happened. Found 2026-08-22 while testing `auto_setup` / `create_pr`, both of + which are otherwise **correct** — the first paths this sweep found that + behave properly. + 2. **Other inert settings surfaces.** Flagged while wiring Appearance, not fixed: custom keyboard shortcuts render but do nothing (`shortcuts-tab.tsx:54`), and the OpenRouter / Google / Ollama provider cards are "Coming soon". Framer diff --git a/src-tauri/src/pipeline/triggers.rs b/src-tauri/src/pipeline/triggers.rs index 0a9f288..8cca484 100644 --- a/src-tauri/src/pipeline/triggers.rs +++ b/src-tauri/src/pipeline/triggers.rs @@ -888,15 +888,22 @@ fn execute_merge_to_main( ); } - // The codex `on_entry` for this column was responsible for merging - // origin/ into the task branch and committing — so HEAD should - // already be a fast-forwardable descendant of origin/. - let push = run_command( - &repo_path, - "git", - &["push", "origin", &format!("HEAD:refs/heads/{}", base)], - ) - .map_err(AppError::CommandError)?; + // Push the task's branch by name, never `HEAD`. + // + // `repo_path` is the task's worktree *if it has one* and the shared + // workspace checkout otherwise — and a task can reach this column without a + // worktree (terminal-column cleanup removes it while `branch_name` + // survives). `HEAD` there is whatever the user happens to have checked out. + // In the best case that is `main`, so `HEAD:refs/heads/main` is a no-op + // push that *succeeds*, and the trigger logs "Pushed …" having merged + // nothing. In the worst case it publishes the user's unrelated working + // branch to main. + // + // The branch name is already validated above, and it is what the success + // log claims to have pushed — so push exactly that. + let refspec = merge_to_main_refspec(&branch_name, &base); + let push = run_command(&repo_path, "git", &["push", "origin", &refspec]) + .map_err(AppError::CommandError)?; if !push.status.success() { let stderr = command_stderr(&push); @@ -1122,6 +1129,15 @@ fn exclude_from_git(working_dir: &str, patterns: &[&str]) { } } +/// The refspec `merge_to_main` pushes: the task's branch onto the base branch. +/// +/// Extracted so the one thing that must never regress — that this names the +/// task's branch rather than `HEAD` — is pinned by a test. See the call site +/// for why `HEAD` was actively dangerous. +fn merge_to_main_refspec(branch_name: &str, base: &str) -> String { + format!("{}:refs/heads/{}", branch_name, base) +} + /// Look up the skills an agent references, dropping ids that no longer exist. /// /// Deliberately lenient: a deleted skill shows in the dossier as "missing @@ -3840,6 +3856,24 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn merge_to_main_pushes_the_task_branch_not_head() { + // This used to push `HEAD:refs/heads/main`. `repo_path` is the shared + // workspace checkout whenever the task has no worktree — which happens + // once terminal-column cleanup removes it — so HEAD was whatever the + // user had checked out. On `main` that is a no-op push that *succeeds*, + // and the trigger logged "Pushed …" having merged nothing; on any other + // branch it would publish unrelated work to main. + let spec = merge_to_main_refspec("kaitencode/feature-x", "main"); + assert_eq!(spec, "kaitencode/feature-x:refs/heads/main"); + assert!(!spec.starts_with("HEAD"), "must never push HEAD: {}", spec); + // A non-default base is honoured. + assert_eq!( + merge_to_main_refspec("feat/x", "develop"), + "feat/x:refs/heads/develop" + ); + } + #[test] fn columns_using_agent_sweeps_every_workspace() { // Agents are global, so deleting one has to account for boards the