Skip to content
Merged
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
34 changes: 27 additions & 7 deletions src/commands/pair/agent.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::ffi::OsString;
use std::path::PathBuf;

use crate::error::{self, Result};
Expand Down Expand Up @@ -49,9 +50,11 @@ impl Agent {
}

/// The prompt goes in as a single argument. It carries no token — only the
/// path to one — so it is safe in `ps` and in shell history.
pub fn command(&self, prompt: &str) -> tokio::process::Command {
/// path to one — so it is safe in `ps` and in shell history. Extra args go
/// in front of it, matching the agents' `[options] [prompt]` grammar.
pub fn command(&self, prompt: &str, extra_args: &[OsString]) -> tokio::process::Command {
let mut command = tokio::process::Command::new(self.binary());
command.args(extra_args);
match self {
Agent::Claude | Agent::Codex => command.arg(prompt),
Agent::Opencode => command.args(["--prompt", prompt]),
Expand Down Expand Up @@ -205,8 +208,9 @@ mod tests {
}

/// The prompt is one argv element, never split and never a shell string.
fn argv(agent: Agent) -> Vec<String> {
let command = agent.command("pair with me");
fn argv(agent: Agent, extra_args: &[&str]) -> Vec<String> {
let extra_args: Vec<OsString> = extra_args.iter().map(OsString::from).collect();
let command = agent.command("pair with me", &extra_args);
std::iter::once(command.as_std().get_program())
.chain(command.as_std().get_args())
.map(|arg| arg.to_string_lossy().into_owned())
Expand All @@ -215,18 +219,34 @@ mod tests {

#[test]
fn claude_and_codex_take_the_prompt_as_their_first_argument() {
assert_eq!(argv(Agent::Claude), ["claude", "pair with me"]);
assert_eq!(argv(Agent::Codex), ["codex", "pair with me"]);
assert_eq!(argv(Agent::Claude, &[]), ["claude", "pair with me"]);
assert_eq!(argv(Agent::Codex, &[]), ["codex", "pair with me"]);
}

#[test]
fn opencode_takes_the_prompt_behind_its_prompt_flag() {
assert_eq!(
argv(Agent::Opencode),
argv(Agent::Opencode, &[]),
["opencode", "--prompt", "pair with me"]
);
}

#[test]
fn extra_args_go_before_the_prompt() {
assert_eq!(
argv(Agent::Claude, &["--model", "opus"]),
["claude", "--model", "opus", "pair with me"]
);
assert_eq!(
argv(Agent::Codex, &["--model", "opus"]),
["codex", "--model", "opus", "pair with me"]
);
assert_eq!(
argv(Agent::Opencode, &["--model", "opus"]),
["opencode", "--model", "opus", "--prompt", "pair with me"]
);
}

#[test]
fn an_explicit_agent_is_used_when_it_is_ready() {
let selected = select(Some(Agent::Opencode), given([READY, READY, READY])).unwrap();
Expand Down
71 changes: 59 additions & 12 deletions src/commands/pair/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ mod prompt;
mod session;
mod target;

use std::ffi::OsString;

use clap::Args;
use serde::Serialize;

Expand All @@ -12,21 +14,24 @@ use crate::error::Result;
use agent::{select, Agent};
use prompt::{build_prompt, write_token};
use session::Sessions;
use target::{resolve_dataset_editor, resolve_editor, PairTarget};
use target::{resolve, PairTarget};

#[derive(Args, Debug, Serialize)]
#[command(about = "Pair an agent CLI with a workspace's marimo notebook")]
pub struct Pair {
/// The workspace, or dataset with --dataset, to pair on, as
/// "{owner}/{slug}" with an optional "@{version}". Defaults to the newest
/// draft version.
/// "{owner}/{slug}" with an optional "@{version}", or a workspace version,
/// dataset or runner id. Defaults to the newest draft version.
target: String,
/// Pair on a dataset's notebook instead of a workspace's
#[arg(long)]
dataset: bool,
/// The notebook to open, defaulting to the workspace's overview notebook
#[arg(long)]
notebook: Option<String>,
/// The marimo session to target, defaulting to the one live session
#[arg(long)]
session: Option<String>,
/// Pair with Claude Code instead of the first agent found
#[arg(long, group = "agent")]
claude: bool,
Expand All @@ -42,6 +47,19 @@ pub struct Pair {
/// Print the prompt instead of launching an agent
#[arg(long, conflicts_with = "agent")]
prompt_only: bool,
/// Extra arguments passed through to the agent command
#[arg(last = true, conflicts_with = "prompt_only")]
#[serde(serialize_with = "lossy_strings")]
agent_args: Vec<OsString>,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// An `OsString` serializes as platform-tagged bytes, which is noise in the
/// command context sent to Sentry; the text is what anyone reading it wants.
fn lossy_strings<S: serde::Serializer>(
args: &[OsString],
serializer: S,
) -> Result<S::Ok, S::Error> {
serializer.collect_seq(args.iter().map(|arg| arg.to_string_lossy()))
}

impl Pair {
Expand Down Expand Up @@ -70,11 +88,7 @@ pub async fn pair(args: Pair, global: GlobalArgs) -> Result<()> {
.spinner()
.with_message(format!("Resolving the editor for {target}"));
let client = global.graphql_client().await?;
let editor = if args.dataset {
resolve_dataset_editor(&client, &target, args.notebook).await?
} else {
resolve_editor(&client, &target, args.notebook).await?
};
let editor = resolve(&client, &target, args.dataset, args.notebook).await?;
pb.set_message(format!("Editor for {target} is {}", editor.phase));

let (token_dir, token_path) = write_token(&editor.token)?;
Expand All @@ -85,7 +99,8 @@ pub async fn pair(args: Pair, global: GlobalArgs) -> Result<()> {
// A session only exists while the notebook is open in a browser, so open it
// — but not if the user already has it open.
let sessions = Sessions::new(&editor, global.allow_insecure_host)?;
if !sessions.is_ready().await {
let mut live = sessions.live().await;
if live.is_empty() {
if args.no_open {
pb.println(format!("Please open {editor_page} to start the notebook"));
} else {
Expand All @@ -97,14 +112,19 @@ pub async fn pair(args: Pair, global: GlobalArgs) -> Result<()> {
}
}
pb.set_message("Waiting for the notebook to connect");
sessions.wait(&pb, &editor_page).await?;
live = sessions.wait(&pb, &editor_page).await?;
}

let prompt = build_prompt(&editor, &token_path, &editor_page);
let session = session::choose(args.session.as_deref(), &live)?;
let prompt = build_prompt(&editor, &token_path, &editor_page, session);
match agent {
Some(agent) => {
pb.finish_with_message(format!("Launching {}", agent.display_name()));
agent.command(&prompt).spawn()?.wait().await?;
agent
.command(&prompt, &args.agent_args)
.spawn()?
.wait()
.await?;
// The agent is done with the token now.
drop(token_dir);
}
Expand Down Expand Up @@ -143,6 +163,27 @@ mod tests {
assert_eq!(args.agent(), None);
assert!(!args.no_open);
assert!(!args.prompt_only);
assert!(args.agent_args.is_empty());
}

#[test]
fn parses_extra_agent_args_after_a_double_dash() {
let args = parse(&["alice/ws", "--", "--model", "opus"]).unwrap();
assert_eq!(args.agent_args, ["--model", "opus"]);
}

/// The command is sent to Sentry as JSON; an `OsString` would arrive as
/// platform-tagged bytes rather than the text it holds.
#[test]
fn agent_args_serialize_as_strings() {
let args = parse(&["alice/ws", "--", "--model", "opus"]).unwrap();
let json = serde_json::to_value(&args).unwrap();
assert_eq!(json["agent_args"], serde_json::json!(["--model", "opus"]));
}

#[test]
fn rejects_extra_agent_args_with_prompt_only() {
assert!(parse(&["alice/ws", "--prompt-only", "--", "--model", "opus"]).is_err());
}

#[test]
Expand All @@ -158,6 +199,12 @@ mod tests {
assert!(parse(&["alice/ds", "--dataset"]).unwrap().dataset);
}

#[test]
fn parses_the_session_flag() {
let args = parse(&["alice/ws", "--session", "s_1"]).unwrap();
assert_eq!(args.session.as_deref(), Some("s_1"));
}

#[test]
fn rejects_two_agent_flags() {
assert!(parse(&["alice/ws", "--claude", "--codex"]).is_err());
Expand Down
70 changes: 57 additions & 13 deletions src/commands/pair/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,44 @@ fn quote(value: impl std::fmt::Display) -> String {
format!("'{}'", value.to_string().replace('\'', r"'\''"))
}

pub fn build_prompt(editor: &PairEditor, token_path: &Path, editor_page: &Url) -> String {
/// `session` is the one session to target, when there is exactly one; the
/// scripts resolve it themselves otherwise.
pub fn build_prompt(
editor: &PairEditor,
token_path: &Path,
editor_page: &Url,
session: Option<&str>,
) -> String {
let session_flag = session
.map(|id| format!(" --session {}", quote(id)))
.unwrap_or_default();
let execute_cmd = format!(
"execute-code.sh --url {}{session_flag}",
quote(&editor.base_url)
);
// The id is current now, but marimo renames a session when the browser
// reconnects, so the agent needs a way out.
let session_hint = if session.is_some() {
" If the script reports the session id is stale, drop --session and try again."
} else {
""
};
format!(
"Use the /marimo-pair skill to pair-program on a running marimo notebook.

Connect to the notebook at: {base_url}

Use `execute-code.sh --url {quoted_url}` from the marimo-pair skill to execute code in the \
notebook.
Use `{execute_cmd}` from the marimo-pair skill to execute code in the notebook.

An auth token is stored at {token_path}. Pass it via `execute-code.sh --url {quoted_url} \
An auth token is stored at {token_path}. Pass it via `{execute_cmd} \
--token \"$(cat {quoted_token_path})\"`.

The notebook must be open in a browser for a session to exist. If the server reports no \
active sessions, ask the user to open {editor_page} and then try again.
active sessions, ask the user to open {editor_page} and then try again.{session_hint}

Once you are connected, send a fun toast (mo.status.toast(...)) to the user inside marimo \
letting them know you're ready to pair.",
base_url = editor.base_url,
quoted_url = quote(&editor.base_url),
token_path = token_path.display(),
quoted_token_path = quote(token_path.display()),
editor_page = editor_page,
Expand Down Expand Up @@ -96,17 +115,27 @@ fn open_private(path: &Path) -> Result<std::fs::File> {
mod tests {
use super::*;

#[test]
fn commands_carry_a_quoted_url_and_token_path() {
let editor = PairEditor {
base_url: Url::parse("http://host/runner/it's/").unwrap(),
fn editor(base_url: &str) -> PairEditor {
PairEditor {
base_url: Url::parse(base_url).unwrap(),
token: "unused".into(),
phase: "READY".into(),
editor_page_id: "workspace-id".into(),
};
let editor_page = Url::parse("https://aqora.io/workspaces/workspace-id/edit").unwrap();
}
}

let prompt = build_prompt(&editor, Path::new("/tmp/it's dir/token.txt"), &editor_page);
fn editor_page() -> Url {
Url::parse("https://aqora.io/workspaces/workspace-id/edit").unwrap()
}

#[test]
fn commands_carry_a_quoted_url_and_token_path() {
let prompt = build_prompt(
&editor("http://host/runner/it's/"),
Path::new("/tmp/it's dir/token.txt"),
&editor_page(),
None,
);

assert!(
prompt.contains(r#"--url 'http://host/runner/it'\''s/'"#),
Expand All @@ -116,6 +145,21 @@ mod tests {
prompt.contains(r#"cat '/tmp/it'\''s dir/token.txt'"#),
"{prompt}"
);
assert!(!prompt.contains("--session"), "{prompt}");
}

#[test]
fn commands_carry_the_session_when_it_is_known() {
let prompt = build_prompt(
&editor("http://host/runner/abc/"),
Path::new("/tmp/token.txt"),
&editor_page(),
Some("s_1"),
);

// Both the bare command and the one with the token target the session.
assert_eq!(prompt.matches("--session 's_1'").count(), 2, "{prompt}");
assert!(prompt.contains("stale"), "{prompt}");
}

#[test]
Expand Down
Loading
Loading