Skip to content

Commit 4856576

Browse files
authored
Auto-suffix session names on collision instead of failing (#10)
When a session name already exists, create appends a 6-char base36 suffix (e.g. my-plan-k3a9f2) instead of bailing. create_with_registry now returns the actual session name so chain and create_or_join propagate it correctly.
1 parent b671b17 commit 4856576

3 files changed

Lines changed: 66 additions & 13 deletions

File tree

src/command_surface.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ pub fn run(args: &CommandRunArgs, registry: &WorkflowRegistry) -> Result<i32> {
7474
return Ok(0);
7575
}
7676
crate::workflow::RunCommandResolution::Session {
77-
session_name,
77+
session_name: _,
7878
agent_name,
7979
create_args,
8080
} => {
81-
create_or_join(&create_args, registry)?;
81+
let session_name = create_or_join(&create_args, registry)?;
8282
ensure_agent_registered(&session_name, &agent_name, registry)?;
8383
let code = watch::next_with_registry(
8484
&session_name,
@@ -976,14 +976,14 @@ fn resolve_run_context(
976976
fn create_or_join(
977977
create_args: &crate::cli::CreateArgs,
978978
registry: &WorkflowRegistry,
979-
) -> Result<()> {
979+
) -> Result<String> {
980980
match commands::create_with_registry(create_args, registry) {
981-
Ok(()) => Ok(()),
981+
Ok(name) => Ok(name),
982982
Err(err) => {
983983
// NOTE: matches the bail! message in state.rs create_session_state()
984984
// ("session '{}' already exists"). If that message changes, update here.
985985
if err.to_string().contains("already exists") {
986-
Ok(())
986+
Ok(create_args.name.clone())
987987
} else {
988988
Err(err)
989989
}

src/commands.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use crate::{
1414
state::{
1515
AgentState, Config, SessionHandle, SessionPhase, SessionState, SessionType,
1616
create_session_state, ensure_dirs_for_session, list_sessions, now, session_dir,
17+
unique_session_name,
1718
},
1819
watch,
1920
workflow::{
@@ -22,7 +23,7 @@ use crate::{
2223
},
2324
};
2425

25-
pub fn create_with_registry(args: &CreateArgs, registry: &WorkflowRegistry) -> Result<()> {
26+
pub fn create_with_registry(args: &CreateArgs, registry: &WorkflowRegistry) -> Result<String> {
2627
let session_type = match args.session_type {
2728
SessionTypeArg::Plan => SessionType::Plan,
2829
SessionTypeArg::Implement => SessionType::Implement,
@@ -142,9 +143,10 @@ pub fn create_with_registry(args: &CreateArgs, registry: &WorkflowRegistry) -> R
142143
workflow_state,
143144
};
144145

145-
let created = create_session_state(&args.name, state)?;
146+
let session_name = unique_session_name(&args.name);
147+
let created = create_session_state(&session_name, state)?;
146148
let setup = (|| -> Result<SessionState> {
147-
let handle = SessionHandle::open(&args.name)?;
149+
let handle = SessionHandle::open(&session_name)?;
148150
let mut current = handle.load_state()?;
149151
dispatch_create_and_init_hooks(&mut current, &handle.session_dir, registry)?;
150152
handle.save_state(&current)?;
@@ -155,18 +157,18 @@ pub fn create_with_registry(args: &CreateArgs, registry: &WorkflowRegistry) -> R
155157
Ok(current) => current,
156158
Err(setup_err) => {
157159
rollback_failed_create(&created)
158-
.with_context(|| format!("rolling back failed create for '{}'", args.name))?;
160+
.with_context(|| format!("rolling back failed create for '{}'", session_name))?;
159161
return Err(setup_err);
160162
}
161163
};
162164

163165
println!(
164166
"created {:?} session '{}' at {}",
165167
current.session_type,
166-
args.name,
168+
session_name,
167169
created.state_path.display()
168170
);
169-
Ok(())
171+
Ok(session_name)
170172
}
171173

172174
pub fn join_with_registry(args: &JoinArgs, registry: &WorkflowRegistry) -> Result<()> {
@@ -454,10 +456,10 @@ pub fn chain_with_registry(args: &ChainArgs, registry: &WorkflowRegistry) -> Res
454456
review_timeout_secs: 1200,
455457
};
456458

457-
create_with_registry(&create_args, registry)?;
459+
let session_name = create_with_registry(&create_args, registry)?;
458460
println!(
459461
"chained plan '{}' into implement session '{}'",
460-
args.plan, args.implement
462+
args.plan, session_name
461463
);
462464
Ok(())
463465
}

src/state.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::{
22
fs::{self, File, OpenOptions},
33
io::{Read, Write},
44
path::{Path, PathBuf},
5+
time::{SystemTime, UNIX_EPOCH},
56
};
67

78
use anyhow::{Context, Result, anyhow, bail};
@@ -56,6 +57,33 @@ mod tests {
5657
assert!(parsed.workflow_state.is_none());
5758
}
5859

60+
#[test]
61+
fn session_suffix_produces_6_char_alphanumeric_string() {
62+
let s = super::session_suffix();
63+
assert_eq!(s.len(), 6);
64+
assert!(s.chars().all(|c| c.is_ascii_alphanumeric()));
65+
}
66+
67+
#[test]
68+
fn unique_session_name_returns_base_when_no_collision() {
69+
let name = format!("no-collision-test-{}", std::process::id());
70+
assert_eq!(unique_session_name(&name), name);
71+
}
72+
73+
#[test]
74+
fn unique_session_name_appends_suffix_on_collision() {
75+
let name = format!("collision-test-{}", std::process::id());
76+
let dir = session_dir(&name);
77+
std::fs::create_dir_all(&dir).unwrap();
78+
std::fs::write(dir.join("state.json"), "{}").unwrap();
79+
80+
let result = unique_session_name(&name);
81+
assert!(result.starts_with(&format!("{name}-")));
82+
assert_eq!(result.len(), name.len() + 1 + 6);
83+
84+
std::fs::remove_dir_all(&dir).unwrap();
85+
}
86+
5987
#[test]
6088
fn custom_workflow_fields_round_trip_with_opaque_state() {
6189
let mut state = sample_state();
@@ -180,6 +208,29 @@ pub struct CreatedSessionState {
180208
pub lock_created: bool,
181209
}
182210

211+
fn session_suffix() -> String {
212+
let nanos = SystemTime::now()
213+
.duration_since(UNIX_EPOCH)
214+
.unwrap_or_default()
215+
.as_nanos();
216+
let seed = nanos ^ (std::process::id() as u128);
217+
let mut n = seed;
218+
const ALPHABET: &[u8] = b"0123456789abcdefghijklmnopqrstuvwxyz";
219+
let mut buf = [0u8; 6];
220+
for byte in &mut buf {
221+
*byte = ALPHABET[(n % 36) as usize];
222+
n /= 36;
223+
}
224+
String::from_utf8_lossy(&buf).into_owned()
225+
}
226+
227+
pub fn unique_session_name(base: &str) -> String {
228+
if !session_dir(base).join("state.json").exists() {
229+
return base.to_string();
230+
}
231+
format!("{}-{}", base, session_suffix())
232+
}
233+
183234
pub fn create_session_state(name: &str, mut state: SessionState) -> Result<CreatedSessionState> {
184235
ensure_rally_home()?;
185236
let dir = session_dir(name);

0 commit comments

Comments
 (0)