Skip to content

ENG-1433: Rename plugin to carbonlog - #24

Closed
jschen5 wants to merge 2 commits into
mainfrom
eng-1433-rename-plugin-to-carbonlog
Closed

ENG-1433: Rename plugin to carbonlog#24
jschen5 wants to merge 2 commits into
mainfrom
eng-1433-rename-plugin-to-carbonlog

Conversation

@jschen5

@jschen5 jschen5 commented Mar 16, 2026

Copy link
Copy Markdown
Contributor
  • Rename directory plugins/carbon → plugins/carbonlog
  • Rename all carbon-* scripts to carbonlog-* (setup, report, uninstall, etc.)
  • Update plugin ID to carbonlog@cnaught-plugins, DB to carbonlog.db, settings key to carbonlog, log prefix to [carbonlog]
  • Add session-start migration that automatically renames the old carbon-tracker.db, migrates installed_plugins.json keys, settings entries (_carbonOriginalStatusLine, enabledPlugins, carbonTracker), and statusline command paths
  • All detection/lookup functions handle both old and new names for backwards compatibility until migration runs
  • Rename isCarbonStatusLine → isPluginStatusLine
  • Update all commands, tests, and README

- Rename directory plugins/carbon → plugins/carbonlog
- Rename all carbon-* scripts to carbonlog-* (setup, report, uninstall, etc.)
- Update plugin ID to carbonlog@cnaught-plugins, DB to carbonlog.db, settings key to carbonlog, log prefix to [carbonlog]
- Add session-start migration that automatically renames the old carbon-tracker.db, migrates installed_plugins.json keys, settings entries (_carbonOriginalStatusLine, enabledPlugins, carbonTracker), and statusline command paths
- All detection/lookup functions handle both old and new names for backwards compatibility until migration runs
- Rename isCarbonStatusLine → isPluginStatusLine
- Update all commands, tests, and README
@jschen5

jschen5 commented Mar 16, 2026

Copy link
Copy Markdown
Contributor Author

@claude can you review this pr

@claude

claude Bot commented Mar 16, 2026

Copy link
Copy Markdown

Claude finished @jschen5's task in 2m 15s —— View job


PR Review: ENG-1433 Rename plugin to carbonlog

  • Gather context and understand the PR
  • Read key changed files
  • Review migration logic
  • Review backwards compatibility
  • Review tests
  • Post review feedback

Overall this is a well-executed rename. The migration design is solid — idempotent, non-fatal, and race-safe. Backwards compatibility is handled consistently throughout. A few things worth addressing:


Issues

1. Missing migration tests (significant gap)

migration.ts is 178 lines of complex, stateful logic that runs on every session start, but there's no migration.test.ts. The functions migrateFromCarbon, migrateDatabaseFiles, migrateInstalledPlugins, migrateSettingsFile, and migrateProjectSettings are all untested directly. This is the most important piece of new code in the PR — consider adding a test file covering:

  • DB file rename (including WAL/SHM companions)
  • installed_plugins.json key migration
  • settings.json key migration (all three key renames)
  • Project settings migration
  • Idempotency (running twice has no effect)
  • Race condition (old path already gone)

Fix this →

2. carbon-output.ts / carbon-output.test.ts not renamed

Every other file was updated to use the carbonlog prefix, but these two files in plugins/carbonlog/src/statusline/ still use the carbon- prefix:

  • carbon-output.tscarbonlog-output.ts
  • carbon-output.test.tscarbonlog-output.test.ts

This is inconsistent with the rest of the rename and could confuse future contributors.

Fix this →

3. Stale config key in uninstall script

plugins/carbonlog/src/scripts/carbonlog-uninstall.ts:78 still deletes claude_code_user_name:

deleteConfig(cleanupDb, 'claude_code_user_name');

This key was renamed to claude_code_team (DB migration v7) and no longer exists in any current install. This is harmless (a no-op delete), but it's a stale reference that suggests the uninstall wasn't fully updated.

Fix this →


Minor Observations

migrateSettingsFile only migrates carbon-statuslinecarbonlog-statusline (migration.ts:138), not the older statusline-carbon pattern. This is fine in practice because updateStatuslinePath (called afterwards in session-start.ts) uses isPluginStatusLine which covers all patterns and updates the path. Just noting for completeness.

isPluginStatusLine matches statusline-wrapper broadly — if another plugin happened to use a file named statusline-wrapper, it would be treated as this plugin. Pre-existing issue, not introduced here.


What's Well Done

  • Migration design: Idempotent, non-fatal, race-safe with proper ENOENT handling for concurrent sessions
  • WAL/SHM handling in migrateDatabaseFiles is thoughtful and correct
  • Double-wrap prevention in configureSettings correctly reads the saved original on re-setup
  • Legacy key fallback is consistent throughout — every lookup in getInstalledPluginEntries, migrateProjectSettings, cleanupSettingsFile, etc. checks both keys
  • api-client.ts:39 correctly falls back to settings.carbonTracker?.apiUrl for users who haven't migrated yet
  • updateStatuslinePath runs after migration and handles stale paths from version updates, covering any migration gaps
  • Test coverage in setup-helpers.test.ts is thorough and well-structured

@jschen5

jschen5 commented Mar 16, 2026

Copy link
Copy Markdown
Contributor Author

Renaming actually will require uninstalling and reinstalling the plugin since claude code will treat the renamed plugin as a completely separate new plugin.

@jschen5 jschen5 closed this Mar 16, 2026
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