Skip to content
Open
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
166 changes: 117 additions & 49 deletions src/cli/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,18 +520,35 @@ fn handle_add(cli: &Cli, mode: &OutputMode, args: &AddArgs) -> Result<()> {
let actual_bookmark_id = db.insert_bookmark(&bookmark)?;
let is_new = actual_bookmark_id == bookmark_id;

// Insert annotation with notes and context if provided
if args.note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: args.note.clone(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
// Insert annotations with notes and context if provided
if !args.note.is_empty() || args.context.is_some() {
// If context is provided, attach it to the first note
let first_note = args.note.first();
if first_note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: first_note.cloned(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
// Additional notes without context
for note in args.note.iter().skip(1) {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: Some(note.clone()),
context: None,
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
}
Comment on lines +523 to 552
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make repeatable-note writes atomic and shared.

This turns one codemark add invocation into several insert_annotation calls, but the loop is duplicated here and in handle_add_from_snippet, handle_add_from_query, and handle_annotate. If one insert fails midway, the command leaves a partially applied annotation set behind. Please move this fan-out into one helper and run it inside a single DB transaction so the operation is all-or-nothing across all four commands.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/handlers.rs` around lines 436 - 465, Extract the duplicated
annotation-fanout logic (the loop that creates Annotation structs and calls
db.insert_annotation) into a single helper function (e.g.,
insert_annotations_for_bookmark(bookmark_id, args, db)) and replace the inline
blocks in this handler plus handle_add_from_snippet, handle_add_from_query, and
handle_annotate to call that helper; within the helper start a single DB
transaction (using your DB's transaction API on the db instance), perform all
insert_annotation calls for args.note and args.context inside the transaction,
and commit only if all inserts succeed so the whole multi-note write is atomic
and shared across those four callers. Ensure the helper still sets fields the
same way (id via uuid::Uuid::new_v4, bookmark_id, added_at via now_iso(),
added_by from args.created_by, notes/context/source) and returns/propagates
errors from the transaction if any insert fails.


// Insert tags if provided
Expand Down Expand Up @@ -684,18 +701,35 @@ fn handle_add_from_snippet(cli: &Cli, mode: &OutputMode, args: &AddFromSnippetAr
let actual_bookmark_id = db.insert_bookmark(&bookmark)?;
let is_new = actual_bookmark_id == bookmark_id;

// Insert annotation with notes and context if provided
if args.note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: args.note.clone(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
// Insert annotations with notes and context if provided
if !args.note.is_empty() || args.context.is_some() {
// If context is provided, attach it to the first note
let first_note = args.note.first();
if first_note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: first_note.cloned(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
// Additional notes without context
for note in args.note.iter().skip(1) {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: Some(note.clone()),
context: None,
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
}

// Insert tags if provided
Expand Down Expand Up @@ -852,18 +886,35 @@ fn handle_add_from_query(cli: &Cli, mode: &OutputMode, args: &AddFromQueryArgs)
let actual_bookmark_id = db.insert_bookmark(&bookmark)?;
let is_new = actual_bookmark_id == bookmark_id;

// Insert annotation with notes and context if provided
if args.note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: args.note.clone(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
// Insert annotations with notes and context if provided
if !args.note.is_empty() || args.context.is_some() {
// If context is provided, attach it to the first note
let first_note = args.note.first();
if first_note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: first_note.cloned(),
context: args.context.clone(),
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
// Additional notes without context
for note in args.note.iter().skip(1) {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: actual_bookmark_id.clone(),
added_at: now_iso(),
added_by: Some(args.created_by.clone()),
notes: Some(note.clone()),
context: None,
source: Some("cli".to_string()),
};
db.insert_annotation(&annotation)?;
}
}

// Insert tags if provided
Expand Down Expand Up @@ -1163,7 +1214,7 @@ fn handle_annotate(cli: &Cli, mode: &OutputMode, args: &AnnotateArgs) -> Result<
let db = open_db_for_write(cli)?;

// Validate that at least one of note, context, or tag is provided
if args.note.is_none() && args.context.is_none() && args.tag.is_empty() {
if args.note.is_empty() && args.context.is_none() && args.tag.is_empty() {
return Err(Error::Input(
"At least one of --note, --context, or --tag must be provided".to_string(),
));
Expand All @@ -1173,18 +1224,35 @@ fn handle_annotate(cli: &Cli, mode: &OutputMode, args: &AnnotateArgs) -> Result<
let id = extract_id(&args.id);
let mut bm = find_bookmark(&db, id)?;

// Create annotation if note or context is provided
if args.note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: bm.id.clone(),
added_at: now_iso(),
added_by: Some(args.added_by.clone()),
notes: args.note.clone(),
context: args.context.clone(),
source: Some(args.source.clone()),
};
db.insert_annotation(&annotation)?;
// Create annotations if notes or context is provided
if !args.note.is_empty() || args.context.is_some() {
// If context is provided, attach it to the first note
let first_note = args.note.first();
if first_note.is_some() || args.context.is_some() {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: bm.id.clone(),
added_at: now_iso(),
added_by: Some(args.added_by.clone()),
notes: first_note.cloned(),
context: args.context.clone(),
source: Some(args.source.clone()),
};
db.insert_annotation(&annotation)?;
}
// Additional notes without context
for note in args.note.iter().skip(1) {
let annotation = Annotation {
id: uuid::Uuid::new_v4().to_string(),
bookmark_id: bm.id.clone(),
added_at: now_iso(),
added_by: Some(args.added_by.clone()),
notes: Some(note.clone()),
context: None,
source: Some(args.source.clone()),
};
db.insert_annotation(&annotation)?;
}

// Re-fetch bookmark to get updated annotations
bm = find_bookmark(&db, id)?;
Expand Down
16 changes: 8 additions & 8 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ pub struct AddArgs {
#[arg(long)]
pub tag: Vec<String>,

/// Semantic annotation
/// Semantic annotation; repeatable for multiple notes
#[arg(long)]
pub note: Option<String>,
pub note: Vec<String>,

/// Agent context at time of bookmarking
#[arg(long)]
Expand Down Expand Up @@ -164,9 +164,9 @@ pub struct AddFromSnippetArgs {
#[arg(long)]
pub tag: Vec<String>,

/// Semantic annotation
/// Semantic annotation; repeatable
#[arg(long)]
pub note: Option<String>,
pub note: Vec<String>,

/// Agent context
#[arg(long)]
Expand Down Expand Up @@ -203,9 +203,9 @@ pub struct AddFromQueryArgs {
#[arg(long)]
pub tag: Vec<String>,

/// Semantic annotation
/// Semantic annotation; repeatable for multiple notes
#[arg(long)]
pub note: Option<String>,
pub note: Vec<String>,

/// Agent context at time of bookmarking
#[arg(long)]
Expand Down Expand Up @@ -566,9 +566,9 @@ pub struct AnnotateArgs {
/// Bookmark ID (full UUID or unambiguous prefix)
pub id: String,

/// Semantic annotation to add
/// Semantic annotation to add; repeatable for multiple notes
#[arg(long)]
pub note: Option<String>,
pub note: Vec<String>,

/// Agent context to add
#[arg(long)]
Expand Down
Loading