Skip to content

fix(opencode): scan sqlite database without legacy session dir#67

Open
mirsella wants to merge 2 commits into
arrufat:masterfrom
mirsella:fix-opencode-extra-session-files
Open

fix(opencode): scan sqlite database without legacy session dir#67
mirsella wants to merge 2 commits into
arrufat:masterfrom
mirsella:fix-opencode-extra-session-files

Conversation

@mirsella
Copy link
Copy Markdown
Contributor

Summary

  • keep scanning configured extra session files when a provider session directory is missing
  • restore Opencode collection from ~/.local/share/opencode/opencode.db for installs without storage/session
  • add regression coverage for extra-file-only providers

Tests

  • zig build
  • zig build test
  • zig build -Doptimize=ReleaseFast
  • zig-out/bin/tokenuze --agent opencode --sessions --log-level debug

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the session event collection in src/providers/provider.zig to ensure that extra session files are still scanned even if the main sessions directory is missing or cannot be opened. It also improves error logging when session IDs fail to build or are empty, and adds a unit test to verify this behavior. Feedback on the changes highlights a potential memory leak where a duplicated path string is not freed if appending to relative_paths fails with an out-of-memory error.

Comment on lines +1193 to +1194
const copy = try shared_allocator.dupe(u8, relative_path);
try relative_paths.append(shared_allocator, copy);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If relative_paths.append fails with error.OutOfMemory, the newly allocated copy will be leaked. Adding an errdefer right after the allocation ensures that copy is freed if the append operation fails.

                    const copy = try shared_allocator.dupe(u8, relative_path);
                    errdefer shared_allocator.free(copy);
                    try relative_paths.append(shared_allocator, copy);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 7143445 by freeing duplicated relative paths if append fails, and applying the same guard to configured extra session file paths.

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.

1 participant