feat: apply permalink to cli and assets manager#68
Conversation
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| click.echo("✅ Successfully renamed permalink") | ||
| click.echo(f"📝 File: {source_path}") | ||
| click.echo(f"🔗 Permalink: {current_permalink or '(none)'} → {permalink}") | ||
| click.echo("📁 Asset directory renamed") |
There was a problem hiding this comment.
CLI reports success for permalink rename without verification
Medium Severity
In permalink rename mode, the CLI unconditionally reports success after calling command.execute() without verifying the operation actually succeeded. The underlying _rename_permalink method can fail silently (e.g., invalid frontmatter, empty permalink, update_permalink_in_file returning False) by logging an error and returning without raising an exception. Unlike new_command, remove_command, and file move mode which all verify outcomes before reporting success, permalink rename mode skips this check, potentially misleading users into thinking the operation succeeded when it failed.
🔬 Verification Test
Why verification test was not possible: This bug requires testing the CLI behavior with various failure conditions (e.g., files with invalid frontmatter, empty permalink values) and observing that it reports success even when the underlying operation fails. The bug is evident from code inspection: command.execute() at line 370 does not return a status, and _rename_permalink at line 443-445 returns without raising an exception when update_permalink_in_file fails. The CLI then proceeds to print success messages at lines 372-376 regardless of whether the rename actually occurred. Comparing with other commands (e.g., new_command at line 174 checks if note_path.exists()) confirms this is an inconsistency in error handling.
Additional Locations (1)
| f"Permalink changed but asset directory unchanged: {new_asset_dir}" | ||
| ) | ||
| except Exception as e: | ||
| log.error(f"Error renaming permalink: {e}") |
There was a problem hiding this comment.
Missing rollback leaves file and assets inconsistent on failure
High Severity
In _rename_permalink, if update_permalink_in_file succeeds but the subsequent shutil.move for the asset directory fails, the file's permalink is not rolled back to its original value. This leaves the system in an inconsistent state where the note file references a permalink-based asset directory that doesn't exist, while the actual assets remain at the old location. This could cause data loss if the clean command later runs and deletes the old asset directory as "orphaned" since no note references it anymore.
🔬 Verification Test
Why verification test was not possible: This bug requires simulating a filesystem failure during shutil.move (e.g., permission denied, disk full) after update_permalink_in_file has already succeeded. The bug is evident from code inspection: lines 438-445 modify the file, then lines 447-463 attempt to move the asset directory. If the move fails, the outer exception handler at lines 474-475 only logs the error without restoring the file's original permalink value. This creates an inconsistent state where the file points to a non-existent asset location while the actual assets remain at the old location, and could be incorrectly deleted by the clean command.
Commands Line Interface and Asset manager now use permalink to determine asset directory, instead of filename.
Keep forward compatibility if can't identify permalink from note file.
Affacted CLI commands:
new,remove,move. For users, only notice thatnewcommand now requires a permalink argument.Note
Shifts note asset management from filename-based to permalink-based and extends CLI capabilities.
newnow requiresPERMALINKand writes it to frontmatter; assets created atassets/<permalink>movegains permalink-rename mode (-p/--permalink); file moves now resolve asset dirs by permalink and keep assets in place on same-dir renamesremoveresolves assets via permalink when available; falls back to filename for backward compatibilityget_asset_directory_by_permalink,get_permalink_from_file,update_permalink_in_file; commands updated accordinglyusage/cli.md,getting-started.md,index.md,about/changelog.md) with new usage and behavior; version bumped to3.1.0inpyproject.tomlanduv.lockWritten by Cursor Bugbot for commit e04a63c. This will update automatically on new commits. Configure here.