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
49 changes: 49 additions & 0 deletions crates/jp_cli/src/cmd/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,22 @@ pub(crate) struct Query {
#[arg(long = "tmp", requires = "new")]
expires_in: Option<Option<humantime::Duration>>,

/// 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<String>,

/// 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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
95 changes: 95 additions & 0 deletions crates/jp_cli/src/cmd/query_tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use chrono::{DateTime, Utc};
use indexmap::IndexMap;
use jp_config::conversation::tool::{Enable, PartialToolConfig};

Expand Down Expand Up @@ -31,6 +32,11 @@ fn directives(ds: Vec<ToolDirective>) -> ToolDirectives {
ToolDirectives(ds)
}

fn make_id(secs: u64) -> ConversationId {
ConversationId::try_from(DateTime::<Utc>::UNIX_EPOCH + std::time::Duration::from_secs(secs))
.unwrap()
}

#[test]
#[expect(clippy::too_many_lines)]
fn test_query_tools_and_no_tools() {
Expand Down Expand Up @@ -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);
}
Loading