Skip to content

fix: unify group-2 backend prompt delivery into a single initial message#110

Open
lsk567 wants to merge 4 commits into
mainfrom
feature/unify-group2-prompt-delivery
Open

fix: unify group-2 backend prompt delivery into a single initial message#110
lsk567 wants to merge 4 commits into
mainfrom
feature/unify-group2-prompt-delivery

Conversation

@lsk567

@lsk567 lsk567 commented Apr 20, 2026

Copy link
Copy Markdown
Owner

Summary

Cursor, gemini, and opencode were receiving agent.md and the YOUR NAME / YOUR TASK header as two separate turns, triggering an unwanted round-trip. For opencode this also caused protocol drift (the worker emitted TEST COMPLETE instead of [TASK COMPLETE] and skipped notify_parent). This PR merges the two messages into a single initial user message for those backends; claude and codex keep the existing two-step flow unchanged.

Root cause

build_agent_command in src/manager/mod.rs injects agent.md per backend:

  • Group 1 (claude, codex) — real system prompt (--system-prompt, developer_instructions). Invisible to chat turns. The task header sent by the spawn path is the first user turn. One-turn interaction — works.
  • Group 2 (cursor, gemini, opencode) — first user message (cursor "Load '<file>'...", gemini -i, opencode --prompt). The backend responds to agent.md before the task header arrives.

Evidence from recent runs: gemini replied "Ready. Please provide YOUR NAME and YOUR TASK to begin."; opencode reasoned "I don't see a specific task mentioned in the message..." — both reasonable responses to what they actually received.

Fix

  1. New PromptDeliveryMode { SystemPrompt, InitialUserMessage } + prompt_delivery_mode(&base_command) helper classifies each backend.
  2. New build_worker_agent_command returns both the shell command and its delivery mode. For group-2 backends it materializes a combined prompt file (agent.md with substitutions applied + separator + YOUR NAME: <short_name>\nYOUR TASK: <task>\n appended) and points the backend's prompt flag at it — no sed layer needed since substitutions are already rendered.
  3. Callers in src/api/handlers.rs::spawn_agent and src/manager/mod.rs::spawn_worker consult delivery_mode.delivers_task_inline() to skip the post-spawn deliver_prompt when the task is already inline.
  4. Group-1 behaviour (claude, codex) is untouched — same command, same follow-up deliver_prompt.
  5. Unknown backends fall back to SystemPrompt — conservative, since that keeps the existing follow-up delivery so the task still reaches the agent.
  6. Safe-fallback on materialize failure (addressing Copilot feedback): materialize_combined_prompt now returns Option<PathBuf>. If rendering/writing the combined file fails, build_worker_agent_command reverts to the legacy build_agent_command path and reports SystemPrompt, so the task header still arrives via the follow-up deliver_prompt (with a visible stderr warning) rather than silently vanishing.

Tests

Added to src/manager/mod.rs:

  • test_prompt_delivery_mode_group1_claude / _codex — SystemPrompt, delivers_task_inline() == false.
  • test_prompt_delivery_mode_group2_cursor / _gemini / _opencode — InitialUserMessage, delivers_task_inline() == true.
  • test_prompt_delivery_mode_unknown_falls_back_to_system_prompt.
  • test_build_worker_agent_command_group1_claude_uses_system_prompt / _codex_... — returned command still uses --system-prompt / developer_instructions, and does NOT embed the task header.
  • test_build_worker_agent_command_cursor_combines_prompt_and_task / _gemini_... / _opencode_... — reads the rendered file back from disk and asserts it contains all three of: the agent.md body text ("You operate in one of two distinct roles"), YOUR NAME: <name>, and YOUR TASK: <task>.
  • test_build_worker_agent_command_group2_substitutions_rendered{{EA_ID}} inside agent.md is expanded in the combined file (no sed layer for group-2).
  • test_build_worker_agent_command_group2_falls_back_on_materialize_failure — when the source prompt can't be read, the returned command reverts to the two-step flow so the task is still delivered.

Validation

  • cargo fmt -- --check — clean
  • cargo clippy --bin omar --tests -- -D warnings — clean
  • cargo test -- --test-threads=1 — 202 passed, 0 failed

🤖 Generated with Claude Code

For cursor, gemini, and opencode, agent.md arrived as a user message while
the YOUR NAME / YOUR TASK header came in a separate follow-up delivery.
Backends then responded to the system prompt before the task ever arrived
— gemini asked for the task back, and opencode emitted "TEST COMPLETE"
instead of the required "[TASK COMPLETE]" protocol marker.

Fold agent.md (with substitutions applied) and the task header into a
single rendered file, and feed that file to these backends as their
first user message. Claude and codex keep using --system-prompt /
developer_instructions with the existing follow-up deliver_prompt.

Callers consult `PromptDeliveryMode::delivers_task_inline()` to decide
whether to skip the post-spawn deliver_prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 20, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes prompt delivery for group-2 backends (cursor/gemini/opencode) by ensuring agent.md and the YOUR NAME/YOUR TASK header arrive as a single initial user message, avoiding an extra backend round-trip and preventing opencode protocol drift.

Changes:

  • Added PromptDeliveryMode classification to distinguish system-prompt vs initial-user-message backends.
  • Introduced build_worker_agent_command to generate backend-specific spawn commands plus delivery mode, materializing a combined prompt file for group-2 backends.
  • Updated both manager-driven worker spawning and API spawn_agent to skip follow-up deliver_prompt when the task header is already inline; added targeted tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/manager/mod.rs Adds prompt delivery mode logic, combined prompt materialization, new spawn-command API, and updates worker spawning + tests to avoid duplicate prompt turns.
src/api/handlers.rs Switches worker agent spawn command building to build_worker_agent_command and skips task delivery when already embedded for group-2 backends.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/manager/mod.rs Outdated
Comment on lines +193 to +220
let mut content = std::fs::read_to_string(prompt_file).unwrap_or_default();
for (pattern, replacement) in substitutions {
content = content.replace(pattern, replacement);
}
content.push_str(&format!(
"\n\n---\n\nYOUR NAME: {}\nYOUR TASK: {}\n",
short_name, task
));

let dir = std::env::temp_dir().join("omar-prompts");
std::fs::create_dir_all(&dir).ok();

let stem = prompt_file
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("prompt");
let ext = prompt_file
.extension()
.and_then(|s| s.to_str())
.unwrap_or("md");
let rendered = dir.join(format!("{}-combined-{}.{}", stem, Uuid::new_v4(), ext));

if std::fs::write(&rendered, &content).is_ok() {
rendered
} else {
prompt_file.to_path_buf()
}
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 8aeccb9: materialize_combined_prompt now returns Option<PathBuf> and build_worker_agent_command falls back to the legacy build_agent_command + PromptDeliveryMode::SystemPrompt when rendering fails, so the task header still arrives via the follow-up deliver_prompt (with a visible stderr warning). Added test_build_worker_agent_command_group2_falls_back_on_materialize_failure as a regression guard.

Comment thread src/manager/mod.rs Outdated
Comment on lines +202 to +216
let dir = std::env::temp_dir().join("omar-prompts");
std::fs::create_dir_all(&dir).ok();

let stem = prompt_file
.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("prompt");
let ext = prompt_file
.extension()
.and_then(|s| s.to_str())
.unwrap_or("md");
let rendered = dir.join(format!("{}-combined-{}.{}", stem, Uuid::new_v4(), ext));

if std::fs::write(&rendered, &content).is_ok() {
rendered

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid concern, but out of scope for this focused bugfix. The unbounded accumulation pattern is pre-existing in materialize_prompt_file (cursor's substitution-rendered prompt has written to ${TMPDIR}/omar-prompts/ since the original group-2 implementation); this PR doesn't change that mechanism, just adds opencode/gemini as additional callers. Realistic blast radius: ~20KB per spawn × many spawns, cleaned on reboot (most OSes sweep $TMPDIR). Filing as a follow-up to move these renders under the EA state dir and GC on agent kill, which is the right fix but touches a different layer of plumbing.

lsk567 and others added 2 commits April 20, 2026 12:33
…-step delivery

Addresses Copilot review comment on PR #110: the previous silent
fallback (returning the original prompt_file on write failure) left
callers marking delivery_mode = InitialUserMessage, so the follow-up
deliver_prompt was skipped and group-2 workers would silently miss
their task header.

`materialize_combined_prompt` now returns `Option<PathBuf>` and
`build_worker_agent_command` falls back to the legacy
`build_agent_command` + SystemPrompt mode when materialization fails,
so the follow-up deliver_prompt still carries the task header. A
warning is logged so the failure is visible.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 20, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an extra round-trip / protocol drift for group-2 backends (cursor, gemini, opencode) by ensuring agent.md and the YOUR NAME / YOUR TASK header are delivered as a single initial user message, while preserving the existing two-step flow for group-1 backends (claude, codex).

Changes:

  • Introduces PromptDeliveryMode + prompt_delivery_mode() to classify backend prompt delivery behavior.
  • Adds build_worker_agent_command() that materializes a combined prompt file for group-2 backends and signals callers to skip the follow-up deliver_prompt.
  • Updates spawn_worker() to conditionally skip post-spawn delivery and adds tests covering classification, command shape, substitution rendering, and fallback behavior.
Comments suppressed due to low confidence (1)

src/manager/mod.rs:1044

  • The comment about skipping require_initial_change and the _markers_proved_ready variable are currently misleading/dead: the result is computed but never used, and deliver_prompt doesn’t have a require_initial_change toggle. This extra wait_for_markers can also add up to 60s of delay before delivery if markers change or never appear. Consider either wiring marker success into the subsequent delivery behavior (or timeouts) or removing the unused variable/comment and using a shorter/consistent timeout strategy.
        return Ok(());
    }

    // Build command with worker system prompt. For group-2 backends
    // (cursor/gemini/opencode) the task header is folded into the initial
    // prompt; for group-1 backends (claude/codex) a follow-up
    // `deliver_prompt` carries the task header.
    let parent_name = "ea";
    let prompt_file = prompts_dir(omar_dir).join("agent.md");
    let spawn_cmd = build_worker_agent_command(
        command,
        &prompt_file,
        &[
            ("{{PARENT_NAME}}", parent_name),
            ("{{TASK}}", &agent.task),
            ("{{EA_ID}}", &ea_id.to_string()),
        ],
        &agent.name,
        &agent.task,
        &McpLaunchContext {
            omar_dir: omar_dir.to_path_buf(),
            ea_id,
            session_prefix: base_prefix.to_string(),
            default_command: command.to_string(),
            default_workdir: ".".to_string(),
            health_idle_warning: 15,
        },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/manager/mod.rs
Comment on lines 235 to 249
@@ -149,6 +248,57 @@ fn materialize_prompt_file(prompt_file: &Path, substitutions: &[(&str, &str)]) -
}
}
Comment thread src/manager/mod.rs
Comment on lines +85 to +111
/// How a backend consumes the agent.md prompt.
///
/// Group 1 backends accept a real system prompt (invisible to chat turns),
/// so the YOUR NAME / YOUR TASK header must arrive as a follow-up user
/// message via `TmuxClient::deliver_prompt`.
///
/// Group 2 backends receive agent.md as the first user message. To avoid
/// a wasted round-trip (and, for opencode, protocol drift) we materialize
/// a single combined prompt that already contains the task header and
/// skip the follow-up delivery.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PromptDeliveryMode {
/// Task header needs a follow-up `deliver_prompt` call (claude, codex).
SystemPrompt,
/// Task header is already inline in the spawn command (cursor, gemini, opencode).
InitialUserMessage,
}

impl PromptDeliveryMode {
/// Whether the spawn command already contains the task header.
///
/// Callers that would otherwise follow up with `deliver_prompt` must
/// skip that step when this returns `true`.
pub fn delivers_task_inline(self) -> bool {
matches!(self, PromptDeliveryMode::InitialUserMessage)
}
}
@KE7

KE7 commented May 8, 2026

Copy link
Copy Markdown
Collaborator

oh this is a good one. let me know when its ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants