summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorsoryu <soryu@soryu.co>2026-02-20 19:07:23 +0000
committerGitHub <noreply@github.com>2026-02-20 19:07:23 +0000
commitf93489a52409af63cea69fd1ce8661f74d0361b8 (patch)
tree95b26658186138e3ad547e92d91aa3ca32b18aab
parent5f8cb48d153f3ef1480c73a1ac3536219755f7e3 (diff)
downloadsoryu-f93489a52409af63cea69fd1ce8661f74d0361b8.tar.gz
soryu-f93489a52409af63cea69fd1ce8661f74d0361b8.zip
feat: auto-remove merged steps, fix UI overflow, and improve worktree handling (#74)
* feat: soryu-co/soryu - makima: Fix contracts page overflow - constrain layout to viewport height * feat: soryu-co/soryu - makima: Add git fetch to create_worktree and improve completion prompt merge conflict handling * WIP: heartbeat checkpoint * feat: soryu-co/soryu - makima: Add pending question notification badge to directive sidebar and nav * feat: soryu-co/soryu - makima: Fix reconcile:on blocking - make phaseguard poll indefinitely instead of returning immediately * feat: soryu-co/soryu - makima: Auto-remove merged steps before planning runs
-rw-r--r--makima/src/daemon/worktree/manager.rs7
-rw-r--r--makima/src/orchestration/directive.rs63
-rw-r--r--makima/src/server/handlers/directives.rs39
3 files changed, 103 insertions, 6 deletions
diff --git a/makima/src/daemon/worktree/manager.rs b/makima/src/daemon/worktree/manager.rs
index 2caa86a..2e2b58e 100644
--- a/makima/src/daemon/worktree/manager.rs
+++ b/makima/src/daemon/worktree/manager.rs
@@ -491,11 +491,8 @@ impl WorktreeManager {
.output()
.await;
- // Prefer origin/{base_branch} to get latest remote state.
- // If neither origin/{base_branch} nor {base_branch} exist (e.g. PR branch
- // was deleted after merge), fall back to the repo's default branch.
- let origin_ref = format!("origin/{}", base_branch);
- let has_origin_ref = Command::new("git")
+ // Create the worktree with a new branch based on the local base_branch
+ let output = Command::new("git")
.args([
"rev-parse",
"--verify",
diff --git a/makima/src/orchestration/directive.rs b/makima/src/orchestration/directive.rs
index df44ee4..420b3e1 100644
--- a/makima/src/orchestration/directive.rs
+++ b/makima/src/orchestration/directive.rs
@@ -339,6 +339,27 @@ impl DirectiveOrchestrator {
"Directive goal updated — spawning re-planning task"
);
+ // If directive already has a PR, remove completed steps that were included in it
+ if directive.pr_url.is_some() || directive.pr_branch.is_some() {
+ match remove_already_merged_steps(&self.pool, directive.id).await {
+ Ok(count) if count > 0 => {
+ tracing::info!(
+ directive_id = %directive.id,
+ removed = count,
+ "Auto-removed completed steps already included in PR before replanning"
+ );
+ }
+ Err(e) => {
+ tracing::warn!(
+ directive_id = %directive.id,
+ error = %e,
+ "Failed to auto-remove merged steps before replanning"
+ );
+ }
+ _ => {}
+ }
+ }
+
let existing_steps =
repository::list_directive_steps(&self.pool, directive.id).await?;
let generation =
@@ -758,6 +779,25 @@ impl DirectiveOrchestrator {
}
repository::clear_completion_task(&self.pool, check.directive_id).await?;
+
+ // Auto-remove completed steps that were just included in the PR
+ match remove_already_merged_steps(&self.pool, check.directive_id).await {
+ Ok(count) if count > 0 => {
+ tracing::info!(
+ directive_id = %check.directive_id,
+ removed = count,
+ "Auto-removed completed steps after PR completion"
+ );
+ }
+ Err(e) => {
+ tracing::warn!(
+ directive_id = %check.directive_id,
+ error = %e,
+ "Failed to auto-remove merged steps after completion"
+ );
+ }
+ _ => {}
+ }
}
"failed" | "interrupted" => {
tracing::warn!(
@@ -915,6 +955,29 @@ impl DirectiveOrchestrator {
}
}
+/// Remove completed directive steps whose branches have already been included
+/// in a PR (i.e., the directive has a pr_url or pr_branch set).
+/// This prevents duplicate branch merges in subsequent PRs.
+pub async fn remove_already_merged_steps(
+ pool: &PgPool,
+ directive_id: Uuid,
+) -> Result<usize, anyhow::Error> {
+ let step_tasks = repository::get_completed_step_tasks(pool, directive_id).await?;
+ let mut removed = 0;
+ for st in &step_tasks {
+ if repository::delete_directive_step(pool, st.step_id).await? {
+ tracing::info!(
+ step_id = %st.step_id,
+ step_name = %st.step_name,
+ directive_id = %directive_id,
+ "Auto-removed completed step (already included in PR)"
+ );
+ removed += 1;
+ }
+ }
+ Ok(removed)
+}
+
/// Trigger a completion task (PR creation/update) for a directive.
///
/// This is the public entry point used by both the orchestrator tick and the
diff --git a/makima/src/server/handlers/directives.rs b/makima/src/server/handlers/directives.rs
index 56278a8..992affe 100644
--- a/makima/src/server/handlers/directives.rs
+++ b/makima/src/server/handlers/directives.rs
@@ -931,6 +931,19 @@ pub async fn cleanup_directive(
.into_response();
}
+ // Auto-remove completed steps that were already included in a merged PR
+ if directive.pr_url.is_some() || directive.pr_branch.is_some() {
+ match crate::orchestration::directive::remove_already_merged_steps(pool, id).await {
+ Ok(count) if count > 0 => {
+ tracing::info!("Auto-removed {} completed steps already in PR for directive {} during cleanup", count, id);
+ }
+ Err(e) => {
+ tracing::warn!("Failed to auto-remove merged steps during cleanup for directive {}: {}", id, e);
+ }
+ _ => {}
+ }
+ }
+
// Get completed step tasks for branch name computation
let step_tasks = match repository::get_completed_step_tasks(pool, id).await {
Ok(tasks) => tasks,
@@ -1155,7 +1168,7 @@ pub async fn pick_up_orders(
};
// Verify directive ownership and get directive with steps
- let (directive, steps) =
+ let (directive, mut steps) =
match repository::get_directive_with_steps_for_owner(pool, auth.owner_id, id).await {
Ok(Some((d, s))) => (d, s),
Ok(None) => {
@@ -1175,6 +1188,30 @@ pub async fn pick_up_orders(
}
};
+ // Auto-remove completed steps that were already included in a PR
+ if directive.pr_url.is_some() || directive.pr_branch.is_some() {
+ match crate::orchestration::directive::remove_already_merged_steps(pool, id).await {
+ Ok(count) if count > 0 => {
+ tracing::info!("Auto-removed {} completed steps already in PR for directive {}", count, id);
+ // Re-fetch steps since some were removed
+ steps = match repository::list_directive_steps(pool, id).await {
+ Ok(s) => s,
+ Err(e) => {
+ tracing::error!("Failed to re-fetch steps after cleanup: {}", e);
+ return (
+ StatusCode::INTERNAL_SERVER_ERROR,
+ Json(ApiError::new("REFETCH_STEPS_FAILED", &e.to_string())),
+ ).into_response();
+ }
+ };
+ }
+ Err(e) => {
+ tracing::warn!("Failed to auto-remove merged steps for directive {}: {}", id, e);
+ }
+ _ => {}
+ }
+ }
+
// Reconcile existing orders: mark done if step completed, under_review if step in progress
match repository::reconcile_directive_orders(pool, auth.owner_id, id).await {
Ok(count) => {