Reopen on duplicate content in tsk push instead of clobbering

+158 -7

1 changed file

expand all

src

workspace.rs

+158 -7

src/workspace.rs

··· 85 85 86 86 /// User-facing task: human id (in active namespace) + content + properties. 87 87 /// Each property holds zero or more text values. 88 + #[derive(Debug)] 88 89 pub struct Task { 89 90 #[allow(dead_code)] // exposed for callers; constructed by workspace 90 91 pub id: Id, ··· 225 226 } 226 227 } 227 228 229 + /// Create a task — or, when the content matches an existing task, 230 + /// reopen / re-bind it instead of clobbering. 231 + /// 232 + /// Stable id is content-addressed (SHA-1 of the content blob), so two 233 + /// `new_task` calls with the same body produce the same stable id and 234 + /// would collide on the task ref. Branches: 235 + /// 236 + /// - ref doesn't exist → fresh create. 237 + /// - ref exists, bound in active namespace, status=done → reopen 238 + /// (flip status back to `open`, return the existing human id). 239 + /// - ref exists, bound in active namespace, status=open → idempotent; 240 + /// return the existing human id without touching the task tree. 241 + /// - ref exists, bound in another namespace but not the active one → 242 + /// error with a hint to use `tsk share` or `tsk reopen -T`. 243 + /// - ref exists, unbound everywhere → bind it in the active namespace. 228 244 pub fn new_task(&self, title: String, body: String) -> Result<Task> { 229 245 let repo = self.repo()?; 230 246 let content = if body.is_empty() { ··· 232 248 } else { 233 249 format!("{}\n\n{}", title.trim(), body.trim()) 234 250 }; 235 - let mut task_obj = TaskObj::new(content); 236 - task_obj 237 - .properties 238 - .insert(STATUS_KEY.into(), vec![STATUS_OPEN.into()]); 239 - let stable = object::create(&repo, &task_obj, "create")?; 240 - properties::reindex_task(&repo, &stable, &task_obj.properties)?; 241 - let human = namespace::assign_id(&repo, &self.namespace(), stable.clone(), "assign-id")?; 251 + // Compute the stable id without writing anything. 252 + let content_oid = repo.blob(content.as_bytes())?; 253 + let stable = StableId(content_oid.to_string()); 254 + let active_ns = self.namespace(); 255 + 256 + let exists = repo.find_reference(&stable.refname()).is_ok(); 257 + if !exists { 258 + let mut task_obj = TaskObj::new(content); 259 + task_obj 260 + .properties 261 + .insert(STATUS_KEY.into(), vec![STATUS_OPEN.into()]); 262 + let stable = object::create(&repo, &task_obj, "create")?; 263 + properties::reindex_task(&repo, &stable, &task_obj.properties)?; 264 + let human = 265 + namespace::assign_id(&repo, &active_ns, stable.clone(), "assign-id")?; 266 + return Ok(Task { 267 + id: Id(human), 268 + stable, 269 + title: task_obj.title().to_string(), 270 + body: task_obj.body().to_string(), 271 + attributes: task_obj.properties, 272 + }); 273 + } 274 + 275 + // Ref already exists. Decide between reopen / idempotent / bind / error. 276 + if let Some(human) = namespace::human_for(&repo, &active_ns, &stable)? { 277 + // Bound in active namespace. 278 + let mut task_obj = object::read(&repo, &stable)? 279 + .ok_or_else(|| Error::Parse(format!("task {stable} content missing")))?; 280 + let is_done = task_obj 281 + .properties 282 + .get(STATUS_KEY) 283 + .map(|v| v.iter().any(|s| s == STATUS_DONE)) 284 + .unwrap_or(false); 285 + if is_done { 286 + task_obj 287 + .properties 288 + .insert(STATUS_KEY.into(), vec![STATUS_OPEN.into()]); 289 + object::update(&repo, &stable, &task_obj, "reopen")?; 290 + properties::reindex_task(&repo, &stable, &task_obj.properties)?; 291 + } 292 + return Ok(Task { 293 + id: Id(human), 294 + stable, 295 + title: task_obj.title().to_string(), 296 + body: task_obj.body().to_string(), 297 + attributes: task_obj.properties, 298 + }); 299 + } 300 + 301 + // Not bound here. Refuse if it lives in another namespace; otherwise bind. 302 + let mut elsewhere = Vec::new(); 303 + for ns_name in namespace::list_names(&repo)? { 304 + if ns_name == active_ns { 305 + continue; 306 + } 307 + if let Some(h) = namespace::human_for(&repo, &ns_name, &stable)? { 308 + elsewhere.push(format!("{ns_name}-{h}")); 309 + } 310 + } 311 + if !elsewhere.is_empty() { 312 + return Err(Error::Parse(format!( 313 + "task with this content is already bound at {} — use `tsk share {active_ns} -T <id>` or `tsk reopen -T <id>` to bind it into '{active_ns}'", 314 + elsewhere.join(", ") 315 + ))); 316 + } 317 + let task_obj = object::read(&repo, &stable)? 318 + .ok_or_else(|| Error::Parse(format!("task {stable} content missing")))?; 319 + let human = namespace::assign_id(&repo, &active_ns, stable.clone(), "assign-id")?; 242 320 Ok(Task { 243 321 id: Id(human), 244 322 stable, ··· 1294 1372 // The state files should live under <git-dir>/tsk/. 1295 1373 assert!(dir.path().join(".git/tsk/namespace").exists()); 1296 1374 assert!(dir.path().join(".git/tsk/queue").exists()); 1375 + } 1376 + 1377 + #[test] 1378 + fn duplicate_content_in_active_ns_done_reopens() { 1379 + let (_d, ws) = fresh_workspace(); 1380 + let t = ws.new_task("clean kitchen".into(), "".into()).unwrap(); 1381 + let original_id = t.id; 1382 + let stable = t.stable.clone(); 1383 + ws.push_task(t).unwrap(); 1384 + ws.drop(TaskIdentifier::Id(original_id)).unwrap(); 1385 + // Task is now status=done. Re-creating with the same content should 1386 + // reopen rather than mint a new id. 1387 + let again = ws.new_task("clean kitchen".into(), "".into()).unwrap(); 1388 + assert_eq!(again.id, original_id, "reopened task keeps its human id"); 1389 + assert_eq!(again.stable, stable); 1390 + assert_eq!( 1391 + again.attributes.get(STATUS_KEY).unwrap(), 1392 + &vec![STATUS_OPEN.to_string()], 1393 + "reopen flips status back to open" 1394 + ); 1395 + } 1396 + 1397 + #[test] 1398 + fn duplicate_content_open_in_active_ns_is_idempotent() { 1399 + let (_d, ws) = fresh_workspace(); 1400 + let first = ws.new_task("write report".into(), "".into()).unwrap(); 1401 + let id = first.id; 1402 + ws.push_task(first).unwrap(); 1403 + let second = ws.new_task("write report".into(), "".into()).unwrap(); 1404 + assert_eq!(second.id, id, "same content returns the same id"); 1405 + } 1406 + 1407 + #[test] 1408 + fn duplicate_content_bound_only_in_other_ns_errors() { 1409 + let (_d, ws) = fresh_workspace(); 1410 + let t = ws.new_task("file taxes".into(), "".into()).unwrap(); 1411 + let id = t.id; 1412 + ws.push_task(t).unwrap(); 1413 + // Move the binding from the default `tsk` namespace into `alpha`, 1414 + // leaving the active `tsk` namespace without a binding. 1415 + ws.share(TaskIdentifier::Id(id), "alpha").unwrap(); 1416 + // Manually unbind from `tsk` (simulating "this content lives only 1417 + // in another namespace"). 1418 + let repo = ws.repo().unwrap(); 1419 + namespace::unassign_id(&repo, "tsk", id.0, "test-unbind").unwrap(); 1420 + // Now creating with the same content should refuse. 1421 + let err = ws 1422 + .new_task("file taxes".into(), "".into()) 1423 + .expect_err("must error when content lives only in another ns"); 1424 + let msg = format!("{err}"); 1425 + assert!( 1426 + msg.contains("alpha-"), 1427 + "error should reference the foreign binding: {msg}" 1428 + ); 1429 + } 1430 + 1431 + #[test] 1432 + fn duplicate_content_unbound_everywhere_binds_in_active_ns() { 1433 + let (_d, ws) = fresh_workspace(); 1434 + let t = ws.new_task("legacy task".into(), "".into()).unwrap(); 1435 + let id = t.id; 1436 + let stable = t.stable.clone(); 1437 + ws.push_task(t).unwrap(); 1438 + // Forcibly unbind everywhere to simulate an orphaned task ref. 1439 + let repo = ws.repo().unwrap(); 1440 + namespace::unassign_id(&repo, "tsk", id.0, "test-unbind").unwrap(); 1441 + // Same content again should re-bind into active ns with a new id. 1442 + let again = ws.new_task("legacy task".into(), "".into()).unwrap(); 1443 + assert_eq!(again.stable, stable, "stable id must match the orphaned ref"); 1444 + assert_ne!( 1445 + again.id, id, 1446 + "rebinding allocates a fresh human id from `next`" 1447 + ); 1297 1448 } 1298 1449 1299 1450 #[test]

Configure Feed

Configure Feed