Skip to content

fix(cursor_desktop): support delete on pre-table schemas and KV-only sessions - #57

Open
Adityakk9031 wants to merge 5 commits into
skillsynchq:mainfrom
Adityakk9031:fix/cursor-desktop-delete-schema-compatibility
Open

Adityakk9031 wants to merge 5 commits into
skillsynchq:mainfrom
Adityakk9031:fix/cursor-desktop-delete-schema-compatibility

Conversation

@Adityakk9031

Copy link
Copy Markdown
Contributor

In CursorDesktopStore::delete(), the store ran an unconditional DELETE FROM composerHeaders WHERE composerId = ?1 and checked whether any rows were deleted from that table alone.

This caused two issues:

  1. On older Cursor databases that lack the composerHeaders table entirely (which discover() explicitly supports by falling back to scanning cursorDiskKV), delete() failed with a SQLite error no such table: composerHeaders.
  2. Even on modern databases with composerHeaders, if a session was discovered from cursorDiskKV without an accompanying header entry, delete() cleaned up the KV entries but returned Err(Error::Malformed("no such session")) because deleted from composerHeaders was 0.

This change:

  • Checks has_table(&conn, "composerHeaders")? before attempting deletion from composerHeaders.
  • Counts both deleted header rows and deleted KV keys.
  • Only returns Error::Malformed when both counts are 0, allowing pre-table databases and KV-only sessions to be deleted cleanly.

Testing

  • Added delete_roundtrip_removes_session_and_fails_on_duplicate.
  • Added delete_succeeds_on_database_without_composer_headers_table.
  • Added delete_succeeds_on_kv_only_session_in_modern_database.
  • cargo test -p txcript --no-default-features --features opencode,hermes,search --test integration cursor_desktop (12/12 passed).
  • cargo clippy -p txcript --no-default-features --features opencode,hermes,search --all-targets -- -D warnings (clean).

@NishantJoshi00

Copy link
Copy Markdown
Collaborator

Deleting session-1 also deletes session-10 because LIKE '%:session-1%' matches both. This PR extends that existing bug to older schemas. Can we match exact session IDs and add a regression test?

@Adityakk9031

Copy link
Copy Markdown
Contributor Author

Fixed! Updated \cursorDiskKV\ deletion to match exact session delimiters (%:\ and %::%) so deleting sibling sessions (such as \session-1\ when \session-10\ exists) will not delete other sessions. Added regression test \delete_exact_session_id_does_not_delete_prefixed_sibling\ in \ ests/integration/cursor_desktop.rs.

@NishantJoshi00

Copy link
Copy Markdown
Collaborator

The delimiter change fixes the session-1 / session-10 case, but LIKE still interprets _ and % inside an accepted session ID as wildcards. At a608c1913864ffb7a0ebf09420505191ef9fc6e2, I reproduced this through CursorDesktopStore::delete on a temporary database without composerHeaders:

  • Insert composerData:session_1, bubbleId:session_1:b1, composerData:sessionX1, and bubbleId:sessionX1:b1.
  • Delete session_1.
  • Both sessionX1 rows are deleted too; expected two remaining rows, got zero.

The existing 18 Cursor Desktop tests pass, but this added isolation regression fails. Please escape literal IDs in the LIKE patterns with an explicit ESCAPE clause, or compare parsed key components exactly, and cover _ and % IDs before merging. The wildcard hazard already exists on newer schemas in main; this PR extends it to the newly supported old schemas.

@Adityakk9031

Copy link
Copy Markdown
Contributor Author

Updated \CursorDesktopStore::delete\ to escape %, _, and \\ characters in the session ID and added an explicit \ESCAPE '\\'\ clause to all \LIKE\ patterns. Added regression test \delete_escapes_like_wildcards_in_session_id\ covering session IDs with _\ and %.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants