diff --git a/CHANGELOG.md b/CHANGELOG.md index efd900d..59ca8e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.2.4 - 2026-03-18 + +- Added actionable allow-rule hints in blocked-command error output so operators can quickly fix permission configs. +- Added PR merge gate for `tt land` steps: when `TT_ENFORCE_MERGE_GATE=1` is set, land fails closed unless required CI checks are green and all PR review threads are resolved. +- Added `docs/pr-review-loop.md` documenting the canonical PR review and merge loop for automation. + +Release impact: PATCH — both features are opt-in and non-breaking. Existing `tt land` behavior is unchanged unless the merge gate env var is explicitly enabled. Permission hints are purely additive CLI output. + ## 0.2.0 - 2026-03-14 Highlights: diff --git a/Cargo.lock b/Cargo.lock index 815603b..ad3fda9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1767,7 +1767,7 @@ checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" [[package]] name = "tutti" -version = "0.2.3" +version = "0.2.4" dependencies = [ "chrono", "clap", diff --git a/Cargo.toml b/Cargo.toml index 959693f..77d764d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tutti" -version = "0.2.3" +version = "0.2.4" edition = "2024" # intentional: codebase uses Rust 2024 let-chain syntax description = "Multi-agent orchestration CLI — your agents, all together" license = "MIT" diff --git a/README.md b/README.md index 9ba07af..0bc06cf 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,8 @@ tt permissions check git status # evaluate team command policy tt permissions check git status --json # machine-readable policy decision +tt permissions suggest verify-app + # suggest allow-rules for all commands in a workflow tt logs backend -f # follow captured output for an agent ``` @@ -137,6 +139,7 @@ source ~/.zshrc - OpenClaw skill contract: `docs/OPENCLAW_SKILL_CONTRACT.md` - OpenClaw skill starter: `skills/openclaw/SKILL.md` - OpenClaw integration bundle: `integrations/openclaw/README.md` +- PR reproducibility loop (CodeRabbit + checks + merge gate): `docs/pr-review-loop.md` - Versioning and release policy: `VERSIONING.md` ## tutti.toml @@ -299,6 +302,7 @@ Reusable prompt components and skills are **phrases**. A phrase might be a CLAUD - Run checkpoints persisted at `.tutti/state/workflow-checkpoints/.json` + `tt run --resume ` - Workflow step types: `prompt`, `command`, `ensure_running`, `workflow` (nested), `review`, `land` - Workflow `review`/`land` steps auto-start required sessions when they are not already running +- Workflow `land` steps enforce a GitHub merge gate (required checks green + all PR review threads resolved) - `workflow_complete` hooks for deterministic chaining - Auto-reclaim of newly-started `persistent = false` sessions at workflow end - `tt serve` local control API endpoints: @@ -353,7 +357,8 @@ Reusable prompt components and skills are **phrases**. A phrase might be a CLAUD ### Permissions Policy (Built, Opt-in) - Team-shared command allowlist in `~/.config/tutti/config.toml` under `[permissions]` - Policy entries may be shell command prefixes (`git status`, `cargo test`) and/or Claude tool names (`Read`, `Edit`, `Write`) -- `tt permissions check ` evaluates command prefixes against policy +- `tt permissions check ` evaluates command prefixes against policy (blocked commands include allow-rule hints) +- `tt permissions suggest ` scans workflow steps and suggests allow-rules for batch pre-approval - `tt permissions export --runtime claude` emits a Claude settings scaffold - `tt up` auto-wires constrained non-interactive policy for Claude sessions - Codex/OpenClaw/Aider constrained mode is hard-enforced via Tutti shell-policy shims plus runtime flags/prompt guidance diff --git a/docs/CODEX_SDLC_ORCHESTRATION.md b/docs/CODEX_SDLC_ORCHESTRATION.md index 684f12a..175162d 100644 --- a/docs/CODEX_SDLC_ORCHESTRATION.md +++ b/docs/CODEX_SDLC_ORCHESTRATION.md @@ -59,6 +59,8 @@ Use `docs/examples/tutti-codex-sdlc.toml` as a starting point. - Enforce docs/version updates in implementation prompt - Require test pass before PR open and before merge/land - If CodeRabbit fails, gather feedback and route to Codex fix step +- Follow the canonical PR reproducibility loop in `docs/pr-review-loop.md` +- Automation `land` steps enforce merge gate checks (required checks + resolved review threads) ## Suggested runbook @@ -72,4 +74,5 @@ Use `docs/examples/tutti-codex-sdlc.toml` as a starting point. - Start with `fail_mode = "closed"` on command steps - Keep PR creation and land as explicit steps (no hidden auto-merge) -- Add approval gate before land for now +- Keep approval checks in your reviewer/final summary prompts +- Keep merge gate enabled before `land` (required checks green + no unresolved review threads) diff --git a/docs/examples/tutti-codex-sdlc.toml b/docs/examples/tutti-codex-sdlc.toml index 6f88f2c..1325816 100644 --- a/docs/examples/tutti-codex-sdlc.toml +++ b/docs/examples/tutti-codex-sdlc.toml @@ -222,3 +222,10 @@ inject_files = [".tutti/state/auto/selected_issue.json", ".tutti/state/auto/bran wait_for_idle = true wait_timeout_secs = 900 text = "Summarize final readiness, residual risks, and merge recommendation." + +# 10) Land (merge gate enforced by automation) +[[workflow.step]] +id = "land_changes" +type = "land" +agent = "implementer" +fail_mode = "closed" diff --git a/docs/pr-review-loop.md b/docs/pr-review-loop.md new file mode 100644 index 0000000..1407e46 --- /dev/null +++ b/docs/pr-review-loop.md @@ -0,0 +1,41 @@ +# PR Review Loop ("Choir" Reproducibility) + +This is the exact merge loop for Tutti automation. + +## Objective + +Make PR handling deterministic: +1. Resolve CodeRabbit threads. +2. Let CodeRabbit auto re-review after push. +3. Wait for required checks to be green. +4. Confirm approvals are present. +5. Merge/land only after gates pass. + +## Canonical loop + +1. Open or update PR from the issue branch. +2. Wait for CodeRabbit feedback (`wait_and_collect_coderabbit.py`). +3. Apply actionable feedback and push. +4. Repeat until no unresolved feedback remains. +5. Run final validation (`cargo test --quiet` + reviewer packet). +6. **Before `land`/merge, enforce merge gate**: + - Required checks must be green. + - PR review threads must all be resolved (including CodeRabbit threads). +7. Merge/land. + +## Enforcement + +Automation land steps now run with: + +- `TT_ENFORCE_MERGE_GATE=1` + +When enabled, `tt land` fails closed if: +- no open PR exists for the landed branch, +- any required check is not green, +- any PR review thread remains unresolved. + +## Notes + +- CodeRabbit re-review is push-triggered; do not manually force review unless required. +- Keep this gate in automation even if humans occasionally merge manually. +- If a gate fails, fix the PR state and rerun the land step. diff --git a/src/automation/mod.rs b/src/automation/mod.rs index 3b3048b..9bdfe7a 100644 --- a/src/automation/mod.rs +++ b/src/automation/mod.rs @@ -1004,10 +1004,13 @@ impl<'a> WorkflowExecutor<'a> { let policy_decision = evaluate_workflow_command_policy(policy_ctx, &rendered); if !policy_decision.allowed { - let message = format!( + let mut message = format!( "command blocked by permissions policy: '{}'", policy_decision.command ); + if let Some(rule) = policy_decision.suggested_rule.as_deref() { + message.push_str(&format!(" (hint: add allow rule '{rule}')")); + } match fail_mode { WorkflowFailMode::Open => { step_results.push(StepResult { @@ -1462,7 +1465,7 @@ impl<'a> WorkflowExecutor<'a> { } match with_project_root(self.project_root, || { - crate::cli::land::run(agent, *pr, *force) + crate::cli::land::run_with_options(agent, *pr, *force, true) }) { Ok(()) => step_results.push(StepResult { index: step_index, @@ -1857,6 +1860,7 @@ fn evaluate_workflow_command_policy( "step_index": ctx.step_index, "command": decision.command, "matched_rule": decision.matched_rule, + "suggested_rule": decision.suggested_rule, })), }, ); @@ -2692,7 +2696,8 @@ fn execute_control_step( if *force { args.push("--force".to_string()); } - let run_result = run_tt_subcommand(project_root, &args); + let run_result = + run_tt_subcommand_with_env(project_root, &args, &[("TT_ENFORCE_MERGE_GATE", "1")]); match (run_result, *fail_mode) { (Ok(_), _) => Ok(ControlStepOutcome { index: step_index, @@ -2753,11 +2758,21 @@ fn control_error_outcome( } fn run_tt_subcommand(project_root: &Path, args: &[String]) -> Result<()> { + run_tt_subcommand_with_env(project_root, args, &[]) +} + +fn run_tt_subcommand_with_env( + project_root: &Path, + args: &[String], + env: &[(&str, &str)], +) -> Result<()> { let bin = std::env::current_exe()?; - let output = Command::new(bin) - .args(args) - .current_dir(project_root) - .output()?; + let mut cmd = Command::new(bin); + cmd.args(args).current_dir(project_root); + for (key, value) in env { + cmd.env(key, value); + } + let output = cmd.output()?; if output.status.success() { return Ok(()); } @@ -3475,6 +3490,12 @@ mod tests { .as_deref() .is_some_and(|m| m.contains("blocked by permissions policy")) ); + assert!( + result.step_results[0] + .message + .as_deref() + .is_some_and(|m| m.contains("hint: add allow rule")) + ); let decisions = crate::state::load_policy_decisions(&dir).unwrap(); assert!(decisions.iter().any(|d| { diff --git a/src/cli/land.rs b/src/cli/land.rs index 90ae6f5..90ea9bd 100644 --- a/src/cli/land.rs +++ b/src/cli/land.rs @@ -1,8 +1,20 @@ use crate::error::{Result, TuttiError}; +use serde_json::Value; use std::path::Path; use std::process::Command; +const ENFORCE_MERGE_GATE_ENV: &str = "TT_ENFORCE_MERGE_GATE"; + pub fn run(agent_ref: &str, pr: bool, force: bool) -> Result<()> { + run_with_options(agent_ref, pr, force, merge_gate_enabled()) +} + +pub fn run_with_options( + agent_ref: &str, + pr: bool, + force: bool, + enforce_merge_gate: bool, +) -> Result<()> { let resolved = super::agent_ref::resolve(agent_ref)?; let agent = resolved.agent_config()?; let branch = agent.resolved_branch(); @@ -24,6 +36,7 @@ pub fn run(agent_ref: &str, pr: bool, force: bool) -> Result<()> { ensure_git_clean(&resolved.project_root)?; } ensure_branch_exists(&resolved.project_root, &branch)?; + maybe_enforce_merge_gate(&resolved.project_root, &branch, enforce_merge_gate)?; let wip_committed = commit_wip_if_needed(&worktree_path, &resolved.agent_name)?; if pr { @@ -105,6 +118,217 @@ pub fn run(agent_ref: &str, pr: bool, force: bool) -> Result<()> { Ok(()) } +fn maybe_enforce_merge_gate(project_root: &Path, branch: &str, enabled: bool) -> Result<()> { + if !enabled { + return Ok(()); + } + + if which::which("gh").is_err() { + return Err(TuttiError::ConfigValidation( + "merge gate enabled, but `gh` is not installed. Install GitHub CLI or disable TT_ENFORCE_MERGE_GATE." + .to_string(), + )); + } + + let pr_number = find_open_pr_number(project_root, branch)?; + ensure_required_checks_green(project_root, pr_number)?; + ensure_all_review_threads_resolved(project_root, pr_number)?; + + println!( + "Merge gate passed for PR #{} on branch '{}': required checks green, review threads resolved.", + pr_number, branch + ); + Ok(()) +} + +fn merge_gate_enabled() -> bool { + matches!( + std::env::var(ENFORCE_MERGE_GATE_ENV) + .ok() + .as_deref() + .map(|v| v.trim().to_ascii_lowercase()), + Some(v) if v == "1" || v == "true" || v == "yes" || v == "on" + ) +} + +fn find_open_pr_number(project_root: &Path, branch: &str) -> Result { + let output = Command::new("gh") + .args([ + "pr", + "list", + "--head", + branch, + "--state", + "open", + "--limit", + "1", + "--json", + "number,url", + ]) + .current_dir(project_root) + .output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(TuttiError::Git(format!( + "merge gate failed to query PR for branch '{}': {}", + branch, + stderr.trim() + ))); + } + + let prs: Value = serde_json::from_slice(&output.stdout).map_err(|e| { + TuttiError::State(format!( + "merge gate failed to parse `gh pr list` output: {e}" + )) + })?; + let number = parse_pr_number(&prs).ok_or_else(|| { + TuttiError::Git(format!( + "merge gate requires an open PR for branch '{}', but none was found.", + branch + )) + })?; + + Ok(number) +} + +fn ensure_required_checks_green(project_root: &Path, pr_number: u64) -> Result<()> { + let output = Command::new("gh") + .args(["pr", "checks", &pr_number.to_string(), "--required"]) + .current_dir(project_root) + .output()?; + + if output.status.success() { + return Ok(()); + } + + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + let detail = if !stderr.is_empty() { stderr } else { stdout }; + + Err(TuttiError::Git(format!( + "merge gate blocked: required checks are not green for PR #{}{}", + pr_number, + if detail.is_empty() { + String::new() + } else { + format!(" ({detail})") + } + ))) +} + +fn ensure_all_review_threads_resolved(project_root: &Path, pr_number: u64) -> Result<()> { + let repo = gh_repo_name_with_owner(project_root)?; + let (owner, name) = split_name_with_owner(&repo)?; + + let query = r#"query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { + isResolved + } + } + } + } +}"#; + + let output = Command::new("gh") + .args(["api", "graphql", "-f", &format!("query={query}")]) + .args(["-F", &format!("owner={owner}")]) + .args(["-F", &format!("name={name}")]) + .args(["-F", &format!("number={pr_number}")]) + .current_dir(project_root) + .output()?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(TuttiError::Git(format!( + "merge gate failed to query review threads for PR #{}: {}", + pr_number, + stderr.trim() + ))); + } + + let payload: Value = serde_json::from_slice(&output.stdout).map_err(|e| { + TuttiError::State(format!( + "merge gate failed to parse review thread payload for PR #{}: {e}", + pr_number + )) + })?; + + let unresolved = unresolved_review_thread_count(&payload).ok_or_else(|| { + TuttiError::State(format!( + "merge gate could not determine unresolved review thread count for PR #{}", + pr_number + )) + })?; + + if unresolved == 0 { + Ok(()) + } else { + Err(TuttiError::Git(format!( + "merge gate blocked: PR #{} has {} unresolved review thread(s). Resolve all threads before land/merge.", + pr_number, unresolved + ))) + } +} + +fn gh_repo_name_with_owner(project_root: &Path) -> Result { + let output = Command::new("gh") + .args([ + "repo", + "view", + "--json", + "nameWithOwner", + "--jq", + ".nameWithOwner", + ]) + .current_dir(project_root) + .output()?; + + if output.status.success() { + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + Err(TuttiError::Git(format!( + "merge gate failed to detect repo owner/name: {}", + stderr.trim() + ))) + } +} + +fn split_name_with_owner(name_with_owner: &str) -> Result<(String, String)> { + let (owner, name) = name_with_owner.split_once('/').ok_or_else(|| { + TuttiError::State(format!( + "invalid GitHub repo slug '{}'; expected owner/name", + name_with_owner + )) + })?; + Ok((owner.to_string(), name.to_string())) +} + +fn parse_pr_number(payload: &Value) -> Option { + payload.as_array()?.first()?.get("number")?.as_u64() +} + +fn unresolved_review_thread_count(payload: &Value) -> Option { + let nodes = payload + .get("data")? + .get("repository")? + .get("pullRequest")? + .get("reviewThreads")? + .get("nodes")? + .as_array()?; + + Some( + nodes + .iter() + .filter(|node| node.get("isResolved").and_then(Value::as_bool) == Some(false)) + .count(), + ) +} + fn push_and_open_pr(project_root: &Path, branch: &str) -> Result<()> { run_git(&["push", "-u", "origin", branch], project_root)?; @@ -274,3 +498,51 @@ fn git_success(args: &[&str], cwd: &Path, exit_one_is_false: bool) -> Result, + suggested_rule: Option, reason: Option, } @@ -49,6 +50,7 @@ fn run_check(parts: &[String], as_json: bool) -> Result<()> { decision.allowed, decision.policy_configured, decision.matched_rule.as_deref(), + decision.suggested_rule.as_deref(), decision.reason.as_deref(), ); @@ -58,6 +60,7 @@ fn run_check(parts: &[String], as_json: bool) -> Result<()> { allowed: decision.allowed, policy_configured: decision.policy_configured, matched_rule: decision.matched_rule.clone(), + suggested_rule: decision.suggested_rule.clone(), reason: decision.reason.clone(), }; println!("{}", serde_json::to_string_pretty(&report)?); @@ -74,15 +77,22 @@ fn run_check(parts: &[String], as_json: bool) -> Result<()> { } } else if let Some(reason) = decision.reason.as_deref() { eprintln!("{reason}: '{}'", decision.command); + if let Some(rule) = decision.suggested_rule.as_deref() { + eprintln!("hint: add this allow rule: {rule}"); + } } if decision.allowed { Ok(()) } else { - Err(TuttiError::ConfigValidation(format!( + let mut message = format!( "command blocked by permissions policy: '{}'", decision.command - ))) + ); + if let Some(rule) = decision.suggested_rule.as_deref() { + message.push_str(&format!(" (hint: add allow rule '{rule}')")); + } + Err(TuttiError::ConfigValidation(message)) } } @@ -108,6 +118,7 @@ fn persist_permission_check_decision( allowed: bool, policy_configured: bool, matched_rule: Option<&str>, + suggested_rule: Option<&str>, reason: Option<&str>, ) { let Some(ctx) = workspace_ctx else { @@ -137,7 +148,8 @@ fn persist_permission_check_decision( reason: reason.map(ToString::to_string), data: Some(json!({ "command": command, - "matched_rule": matched_rule + "matched_rule": matched_rule, + "suggested_rule": suggested_rule })), }, ); @@ -191,7 +203,9 @@ fn collect_blocked_commands( if !decision.allowed && seen_commands.insert(cmd.clone()) { blocked.push(PermissionSuggestion { command: cmd.clone(), - suggested_rule: format!("{cmd} *"), + suggested_rule: decision + .suggested_rule + .unwrap_or_else(|| format!("{cmd} *")), reason: decision.reason, }); } @@ -436,6 +450,7 @@ mod tests { true, Some("git status"), None, + None, ); let records = load_policy_decisions(&dir).unwrap(); diff --git a/src/permissions/mod.rs b/src/permissions/mod.rs index af1c13f..6f7efb6 100644 --- a/src/permissions/mod.rs +++ b/src/permissions/mod.rs @@ -34,6 +34,7 @@ pub struct CommandPolicyDecision { pub allowed: bool, pub policy_configured: bool, pub matched_rule: Option, + pub suggested_rule: Option, pub reason: Option, } @@ -48,6 +49,7 @@ pub fn evaluate_command_policy( allowed: true, policy_configured: false, matched_rule: None, + suggested_rule: None, reason: Some("policy not configured".to_string()), }; }; @@ -58,19 +60,30 @@ pub fn evaluate_command_policy( allowed: true, policy_configured: true, matched_rule: Some(matched_rule.to_string()), + suggested_rule: None, reason: None, }; } CommandPolicyDecision { - command: normalized, + command: normalized.clone(), allowed: false, policy_configured: true, matched_rule: None, + suggested_rule: suggested_wildcard_prefix_rule(&normalized), reason: Some("blocked by permissions policy".to_string()), } } +fn suggested_wildcard_prefix_rule(command_line: &str) -> Option { + let tokens: Vec<&str> = command_line.split_whitespace().collect(); + if tokens.len() < 2 { + return None; + } + + Some(format!("{} {} *", tokens[0], tokens[1])) +} + pub fn render_claude_settings(policy: &PermissionsConfig) -> Result { let allow: Vec = policy .allow @@ -247,6 +260,16 @@ mod tests { decision.reason.as_deref(), Some("blocked by permissions policy") ); + assert_eq!(decision.suggested_rule.as_deref(), Some("git stash *")); + } + + #[test] + fn evaluate_command_policy_suggests_rule_from_first_two_tokens_only() { + let policy = PermissionsConfig { + allow: vec!["git status".to_string()], + }; + let decision = evaluate_command_policy(Some(&policy), "cargo test --quiet --all"); + assert_eq!(decision.suggested_rule.as_deref(), Some("cargo test *")); } #[test]