Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions plugins/app/ralphx-mcp-server/src/__tests__/tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,11 +805,15 @@ describe('New team tool definitions', () => {
describe('get_plan_verification', () => {
const tool = PLAN_TOOLS.find((t) => t.name === 'get_plan_verification');

it('should derive the parent session automatically for verifier-owned reads', () => {
it('should expose session_id for parent ideation reads while allowing verifier-owned derivation', () => {
expect(tool).toBeDefined();
expect(tool?.description).toContain('do not pass session_id');
expect((tool?.inputSchema.properties as any)).toEqual({});
expect((tool?.inputSchema as any).examples?.[0]).toEqual({});
expect(tool?.description).toContain('Parent ideation agents must pass session_id');
expect(tool?.description).toContain('Verifier child agents omit session_id');
expect((tool?.inputSchema.properties as any)).toHaveProperty('session_id');
expect((tool?.inputSchema as any).examples?.[0]).toMatchObject({
session_id: 'ideation-session-id',
});
expect((tool?.inputSchema as any).examples?.[1]).toEqual({});
expect(tool?.inputSchema.required).toEqual([]);
});
});
Expand Down
17 changes: 14 additions & 3 deletions plugins/app/ralphx-mcp-server/src/plan-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,23 @@ export const PLAN_TOOLS: Tool[] = [
{
name: "get_plan_verification",
description:
"Get the current verification status for the PARENT ideation session. Use this before and during verification to confirm the generation, in_progress flag, and current round before calling report_verification_round or complete_plan_verification. The canonical parent session is derived automatically from the active verification child context, so do not pass session_id. " +
"Get the current verification status for an ideation plan. Parent ideation agents must pass session_id. Verifier child agents omit session_id because the canonical parent session is derived automatically from the active verification child context. Use this before and during verification to confirm the generation, in_progress flag, and current round before calling report_verification_round or complete_plan_verification. " +
"If a verification update call is rejected, call this again on the parent session and copy the returned generation/in_progress values instead of guessing.",
inputSchema: {
type: "object",
examples: [{}],
properties: {},
examples: [
{
session_id: "ideation-session-id",
},
{},
],
properties: {
session_id: {
type: "string",
description:
"Parent ideation session ID. Required for parent ideation agents; verifier child agents omit it.",
},
},
required: [],
},
},
Expand Down
34 changes: 34 additions & 0 deletions src-tauri/src/application/chat_service/chat_service_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,16 @@ fn incomplete_review_action(
}
}

fn should_suppress_post_success_review_error_note(
context_type: ChatContextType,
existing_content: &str,
task_status: Option<InternalStatus>,
) -> bool {
context_type == ChatContextType::Review
&& !existing_content.trim().is_empty()
&& task_status.is_some_and(|status| status != InternalStatus::Reviewing)
}

pub(super) async fn apply_system_wide_provider_pause<R: Runtime>(
app_handle: &Option<AppHandle<R>>,
category: &super::ProviderErrorCategory,
Expand Down Expand Up @@ -1974,6 +1984,22 @@ pub(super) async fn handle_stream_error<R: Runtime + 'static>(
stream_error,
Some(StreamError::AgentExit { stderr, .. }) if is_nonfatal_mcp_tool_cancellation(stderr)
);
let review_task_status = if context_type == ChatContextType::Review {
match task_repo
.get_by_id(&TaskId::from_string(context_id.to_string()))
.await
{
Ok(Some(task)) => Some(task.internal_status),
_ => None,
}
} else {
None
};
let suppress_post_success_review_error_note = should_suppress_post_success_review_error_note(
context_type,
&existing_content,
review_task_status,
);
let error_note = if suppress_transcript_error_note {
tracing::info!(
conversation_id = conversation_id.as_str(),
Expand All @@ -1982,6 +2008,14 @@ pub(super) async fn handle_stream_error<R: Runtime + 'static>(
"Suppressing non-fatal MCP cancellation note from persisted assistant transcript"
);
existing_content
} else if suppress_post_success_review_error_note {
tracing::info!(
conversation_id = conversation_id.as_str(),
context_type = %context_type,
context_id,
"Suppressing terminal review stderr note because review content exists and task already left Reviewing"
);
existing_content
} else if existing_content.is_empty() {
format!("{} {}]", super::AGENT_ERROR_PREFIX, redacted_error)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,30 @@ fn test_incomplete_review_action_escalates_only_for_live_reviewing_tasks() {
);
}

#[test]
fn test_post_success_review_error_note_suppression_only_after_reviewing() {
assert!(should_suppress_post_success_review_error_note(
ChatContextType::Review,
"Review approved. No blocking issues.",
Some(InternalStatus::PendingMerge),
));
assert!(!should_suppress_post_success_review_error_note(
ChatContextType::Review,
"Review approved. No blocking issues.",
Some(InternalStatus::Reviewing),
));
assert!(!should_suppress_post_success_review_error_note(
ChatContextType::Review,
"",
Some(InternalStatus::PendingMerge),
));
assert!(!should_suppress_post_success_review_error_note(
ChatContextType::TaskExecution,
"Useful output",
Some(InternalStatus::PendingMerge),
));
}

#[tokio::test]
async fn test_apply_system_wide_provider_pause_pauses_mixed_active_task_states() {
let app_state = AppState::new_test();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1235,6 +1235,16 @@ pub fn spawn_send_message_background<R: Runtime>(ctx: BackgroundRunContext<R>) {
"Skipping post-loop finalization — {} turn(s) already finalized in stream loop",
turns_finalized,
);
finalize_no_output_assistant_message(
&chat_message_repo,
&chat_timeline_repo,
app_handle.as_ref(),
&event_ctx,
&conversation_id,
&pre_assistant_msg_id,
&assistant_role,
)
.await;
} else if has_output {
finalize_structured_assistant_message(
&chat_message_repo,
Expand Down
52 changes: 52 additions & 0 deletions src-tauri/src/application/git_service/tests/worktree_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,58 @@ async fn test_checkout_existing_branch_worktree_retries_after_locked_stale_entry
let _ = GitService::delete_worktree(repo, &wt_path).await;
}

/// checkout_existing_branch_worktree: target path is missing but still registered → prune + retry.
#[tokio::test]
async fn test_checkout_existing_branch_worktree_retries_after_missing_registered_entry() {
let temp_dir = tempfile::tempdir().unwrap();
let repo = temp_dir.path();
init_git_repo(repo);

let wt_path = temp_dir.path().join("worktrees").join("missing-registered-wt");

Command::new("git")
.args(["branch", "temp-branch"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args(["branch", "target-branch"])
.current_dir(repo)
.output()
.unwrap();
Command::new("git")
.args([
"worktree",
"add",
wt_path.to_str().unwrap(),
"temp-branch",
])
.current_dir(repo)
.output()
.unwrap();
assert!(wt_path.exists(), "Worktree should exist before deletion");

std::fs::remove_dir_all(&wt_path).unwrap();
assert!(
!wt_path.exists(),
"Directory should be gone while metadata remains"
);

let result =
GitService::checkout_existing_branch_worktree(repo, &wt_path, "target-branch").await;
assert!(
result.is_ok(),
"checkout_existing_branch_worktree should prune missing registered metadata and retry: {:?}",
result.err()
);

assert!(wt_path.exists(), "Worktree should exist after retry");
let branch = GitService::get_current_branch(&wt_path).await.unwrap();
assert_eq!(branch, "target-branch", "Worktree should be on target branch");

let _ = GitService::delete_worktree(repo, &wt_path).await;
}

/// checkout_existing_branch_worktree: branch already checked out at another path → recovery.
///
/// Scenario: a stale worktree from a prior execution still has the task branch checked out.
Expand Down
25 changes: 25 additions & 0 deletions src-tauri/src/application/git_service/worktree.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use super::git_cmd;
use super::*;

fn is_missing_registered_worktree_error(stderr: &str) -> bool {
stderr.contains("missing but already registered worktree")
}

impl GitService {
// =========================================================================
// Worktree Operations (Worktree mode only)
Expand Down Expand Up @@ -240,6 +244,27 @@ impl GitService {
return Ok(());
}

// Guard: git metadata still has a registered worktree entry whose
// directory has been deleted. Git suggests prune/remove; prune is
// sufficient for the stale-missing case and keeps the target branch.
if is_missing_registered_worktree_error(&stderr) {
debug!(
"checkout_existing_branch_worktree: missing registered worktree at {:?}, pruning stale metadata and retrying",
worktree
);
let _ = git_cmd::run(&["worktree", "prune"], repo).await;

let retry = git_cmd::run(&args, repo).await?;
if !retry.status.success() {
let retry_stderr = String::from_utf8_lossy(&retry.stderr);
return Err(AppError::GitOperation(format!(
"Failed to create worktree at {:?} for branch '{}' after missing-registered prune retry: {}",
worktree, branch, retry_stderr
)));
}
return Ok(());
}

// Guard: branch is already checked out in another worktree (stale/orphan from prior run).
// Extract the other worktree path from the error, delete it, prune, and retry.
//
Expand Down
Loading