diff --git a/crates/jp_cli/src/cmd/query.rs b/crates/jp_cli/src/cmd/query.rs index f2918513..e8f022a8 100644 --- a/crates/jp_cli/src/cmd/query.rs +++ b/crates/jp_cli/src/cmd/query.rs @@ -260,6 +260,22 @@ pub(crate) struct Query { #[arg(long = "tmp", requires = "new")] expires_in: Option>, + /// Set a custom title for the conversation. + /// + /// Applied to the resolved conversation (new, forked, or resumed) before + /// the turn runs. Skips title auto-generation for new conversations — + /// your title wins. Mutually exclusive with `--no-title`. + #[arg(long = "title", conflicts_with = "no_title")] + title: Option, + + /// Disable the title for the conversation. + /// + /// Clears any existing title on the resolved conversation (new, forked, + /// or resumed) and skips auto-generation for this run. Mutually + /// exclusive with `--title`. + #[arg(long = "no-title", conflicts_with = "title")] + no_title: bool, + /// The tool to use. /// /// If a value is provided, the tool matching the value will be used. @@ -291,6 +307,11 @@ impl Query { // 3. Lock contention: user picks "new" or "fork" from the prompt. let lock = self.acquire_lock(ctx, handle).await?; + // The two flags are mutually exclusive (enforced by clap), and the + // resolved conversation may be new, freshly forked (which clones the + // source's metadata, including any title), or resumed. + apply_title_override(&lock, self.title.as_deref(), self.no_title); + // Record this conversation as the session's active conversation. if let Some(session) = &ctx.session && let Err(error) = ctx @@ -351,8 +372,13 @@ impl Query { let stream = lock.events().clone(); // Generate title for new or empty conversations (including forks). + // Skip when `--title` or `--no-title` was provided (the user already + // expressed an intent for the title), or when the resolved config + // has auto-generation disabled. if (self.is_new() || self.fork.is_some() || stream.is_empty()) && ctx.term.args.persist + && self.title.is_none() + && !self.no_title && cfg.conversation.title.generate.auto { debug!("Generating title for new conversation"); @@ -920,6 +946,27 @@ fn fork_conversation( }) } +/// Apply `--title` / `--no-title` to the resolved conversation. +/// +/// Both flags act on `metadata.title` directly so the run ends with the +/// title the user asked for, regardless of whether the conversation is new, +/// freshly forked (which inherits the source's title), or resumed: +/// +/// - `--title T` sets the title to `Some(T)`. +/// - `--no-title` clears any existing title. +/// - Neither flag is a no-op. +fn apply_title_override(lock: &ConversationLock, title: Option<&str>, no_title: bool) { + if let Some(title) = title { + lock.as_mut().update_metadata(|m| { + m.title = Some(title.to_owned()); + }); + } else if no_title { + lock.as_mut().update_metadata(|m| { + m.title = None; + }); + } +} + fn get_config_delta_from_cli( cfg: &AppConfig, lock: &ConversationLock, @@ -968,6 +1015,8 @@ impl IntoPartialAppConfig for Query { expires_in: _, target: _, fork: _, + title: _, + no_title: _, } = &self; apply_model(&mut partial, model.as_deref(), merged_config); diff --git a/crates/jp_cli/src/cmd/query_tests.rs b/crates/jp_cli/src/cmd/query_tests.rs index 83a883aa..32667db2 100644 --- a/crates/jp_cli/src/cmd/query_tests.rs +++ b/crates/jp_cli/src/cmd/query_tests.rs @@ -1,3 +1,4 @@ +use chrono::{DateTime, Utc}; use indexmap::IndexMap; use jp_config::conversation::tool::{Enable, PartialToolConfig}; @@ -31,6 +32,11 @@ fn directives(ds: Vec) -> ToolDirectives { ToolDirectives(ds) } +fn make_id(secs: u64) -> ConversationId { + ConversationId::try_from(DateTime::::UNIX_EPOCH + std::time::Duration::from_secs(secs)) + .unwrap() +} + #[test] #[expect(clippy::too_many_lines)] fn test_query_tools_and_no_tools() { @@ -459,3 +465,92 @@ fn test_interleaved_three_step_composition() { Some(Enable::Off), ); } + +fn lock_with_title( + workspace: &mut Workspace, + id: ConversationId, + title: Option<&str>, +) -> jp_workspace::ConversationLock { + let conversation = Conversation { + title: title.map(str::to_owned), + ..Default::default() + }; + workspace.create_conversation_with_id(id, conversation, Arc::new(AppConfig::new_test())); + let handle = workspace.acquire_conversation(&id).unwrap(); + workspace.test_lock(handle) +} + +#[test] +fn apply_title_override_no_title_clears_existing_title() { + // `--no-title` should clear an inherited title (the + // `--fork --no-title` case from PR #600 review): a forked + // conversation inherits the source's title via + // `fork_conversation`, and `--no-title` is supposed to leave + // the run with no title at all. + let mut workspace = Workspace::new("/tmp/test"); + let lock = lock_with_title(&mut workspace, make_id(1000), Some("inherited")); + + apply_title_override(&lock, None, true); + + assert_eq!(lock.metadata().title, None); +} + +#[test] +fn apply_title_override_no_title_clears_resumed_title() { + // `--no-title` is symmetric with `--title T`: both write the + // user's intent into `metadata.title`, regardless of whether + // the conversation is new, forked, or resumed. + let mut workspace = Workspace::new("/tmp/test"); + let lock = lock_with_title(&mut workspace, make_id(1001), Some("existing")); + + apply_title_override(&lock, None, true); + + assert_eq!(lock.metadata().title, None); +} + +#[test] +fn apply_title_override_title_overwrites_existing_title() { + let mut workspace = Workspace::new("/tmp/test"); + let lock = lock_with_title(&mut workspace, make_id(1002), Some("old")); + + apply_title_override(&lock, Some("new"), false); + + assert_eq!(lock.metadata().title.as_deref(), Some("new")); +} + +#[test] +fn apply_title_override_neither_flag_is_noop() { + let mut workspace = Workspace::new("/tmp/test"); + let lock = lock_with_title(&mut workspace, make_id(1003), Some("keep")); + + apply_title_override(&lock, None, false); + + assert_eq!(lock.metadata().title.as_deref(), Some("keep")); +} + +#[test] +fn no_title_does_not_persist_into_partial_config() { + // Regression for the persistence concern in PR #600: routing + // `--no-title` through `apply_cli_config` previously wrote + // `conversation.title.generate.auto = Some(false)` into the + // partial, which would then flow into the conversation's + // `config_delta` via `get_config_delta_from_cli` and persist + // for every future query on that conversation. The flag is + // now strictly invocation-scoped, so the partial must be + // untouched relative to a run without the flag. + let base = PartialAppConfig::empty(); + + let with_flag = Query { + no_title: true, + ..Default::default() + } + .apply_cli_config(None, base.clone(), None) + .unwrap(); + let without_flag = Query::default().apply_cli_config(None, base, None).unwrap(); + + assert_eq!( + with_flag.conversation.title.generate.auto, + without_flag.conversation.title.generate.auto, + ); + assert_eq!(with_flag.conversation.title.generate.auto, None); +}