Skip to content

Rename organization to team, add back auto-generated user display names#22

Merged
jschen5 merged 1 commit intomainfrom
improve-team-support
Mar 10, 2026
Merged

Rename organization to team, add back auto-generated user display names#22
jschen5 merged 1 commit intomainfrom
improve-team-support

Conversation

@jschen5
Copy link
Contributor

@jschen5 jschen5 commented Mar 9, 2026

  • Rename "organization" concept to "team" throughout (config keys, CLI flags, commands, UI text)
  • Add back unique-names-generator to create per-user display names (e.g. "bright-falcon") at setup time, sent as claudeCodeUserName
  • Split API fields: claudeCodeTeamName for team lookup, claudeCodeUserName for individual user identity
  • Add /carbon:rename-team command, remove /carbon:rename-user
  • Add migration v7 to rename claude_code_organization → claude_code_team
  • Lazily generate userName in getSyncConfig for existing users who set up before this change

@jschen5
Copy link
Contributor Author

jschen5 commented Mar 9, 2026

@claude can you review these changes

@claude
Copy link

claude bot commented Mar 9, 2026

Claude finished @jschen5's task in 1m 17s —— View job


Code Review

  • Gather context and understand request
  • Read changed files
  • Analyze changes for bugs, security issues, and best practices
  • Post review feedback

Overall this is a clean rename refactor with good migration hygiene. A few items worth addressing:

Issues

1. getSyncConfig has a hidden write side effect (plugins/carbon/src/sync.ts, lines 658–663)

let userName = getConfig(db, 'claude_code_user_name') ?? '';
if (!userName) {
    userName = generateUserName();
    setConfig(db, 'claude_code_user_name', userName);  // writes to DB inside a "get" function
}

A function named getSyncConfig writing to the database is a violation of the principle of least surprise. Any caller reading sync config for display or validation purposes (e.g. carbon-report.ts, carbon-setup-check.ts) will silently generate and persist a user name as a side effect. Consider:

  • Renaming to getOrInitSyncConfig to signal the write behavior, or
  • Moving the lazy init into a dedicated call during the sync loop, where a write is already expected

Fix this →


2. carbon-uninstall.ts doesn't clean up claude_code_user_name (plugins/carbon/src/scripts/carbon-uninstall.ts, line 522)

The pre-delete config cleanup now removes claude_code_team but doesn't remove the newly added claude_code_user_name:

deleteConfig(cleanupDb, 'sync_enabled');
deleteConfig(cleanupDb, 'claude_code_user_id');
deleteConfig(cleanupDb, 'claude_code_team');
// missing: deleteConfig(cleanupDb, 'claude_code_user_name');

The comment says "database is about to be deleted anyway" so this may be intentional — but for consistency with the other fields, it should be included. Fix this →


3. No test coverage for lazy userName generation in getSyncConfig

The new lazy-init path in getSyncConfig (generates and stores a user name for existing users on first sync) has no test coverage. This is the trickiest part of the migration logic — a test verifying that:

  • A user with sync_enabled=true, a userId, and a team but no userName gets a generated name on first call
  • The generated name is persisted (subsequent calls return the same name)

would give confidence in the backward-compatibility behavior.


Minor

4. Unrelated indentation change in package.json — the entire file changed from 2-space to 4-space indentation (plugins/carbon/package.json). This clutters the diff with noise. Consider reverting to the original indentation style.


Looks Good

  • Migration v7 is correct — the UPDATE ... WHERE key = 'claude_code_organization' approach safely no-ops for users who never had the old key.
  • The API field split (claudeCodeTeamName vs claudeCodeUserName) is clean and the SyncConfig interface change is consistent throughout.
  • The rename-team command and script rename are complete and consistent.
  • The unique-names-generator integration in name-generator.ts is simple and well-encapsulated.
  • Test updates in data-store.test.ts and carbon-output.test.ts correctly reflect the renamed fields.

@jschen5 jschen5 force-pushed the improve-team-support branch 2 times, most recently from 46ddb12 to 0da682d Compare March 10, 2026 03:44
- Rename "organization" concept to "team" throughout (config keys, CLI flags, commands, UI text)
- Add back unique-names-generator to create per-user display names (e.g. "bright-falcon") at setup time, sent as claudeCodeUserName
- Split API fields: claudeCodeTeamName for team lookup, claudeCodeUserName for individual user identity
- Add /carbon:rename-team command, remove /carbon:rename-user
- Add migration v7 to rename claude_code_organization → claude_code_team
- Lazily generate userName in getSyncConfig for existing users who set up before this change
@jschen5 jschen5 force-pushed the improve-team-support branch from 0da682d to ae4c4d1 Compare March 10, 2026 03:49
@jschen5 jschen5 merged commit 67a54ef into main Mar 10, 2026
1 check passed
@jschen5 jschen5 deleted the improve-team-support branch March 10, 2026 03:53
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