Add team support: sync teamId from API, use team-based dashboard URLs

[jschen5]

• Update API mutations to request claudeCodeTeamId from response
• Change upsertSession/upsertSessions to return UpsertResult with teamId
• Store teamId in local DB on successful sync
• Require organization for sync (getSyncConfig returns null if empty)
• Update setup to require --organization when sync is enabled
• Show team dashboard URL in report and setup output
• Update getDashboardUrl to accept teamId instead of userId
• Combine report header/footer into single branded header

[jschen5]

@claude review this pr

[claude]

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

---

PR Review: Add team support

• Gather context and understand the request
• Read changed files
• Analyze code quality, bugs, security, and best practices
• Post review feedback

---

Overall this is a clean, well-structured PR. The logic is consistent across all layers (API → sync → data store → UI), the migration is sound, and the simplified project identifier is a nice cleanup. A few issues worth addressing:

---

Issues

1. storeTeamId silently skips null — no way to clear a stale team ID

plugins/carbon/src/sync.ts:45

function storeTeamId(db: Database, teamId: string | null): void {
    if (teamId) {
        setConfig(db, 'claude_code_team_id', teamId);
    }
}

If the API returns null for teamId (e.g., a user is temporarily removed from a team), the old value is never cleared. A user could be shown a stale dashboard URL pointing to a team they no longer belong to. Consider calling deleteConfig or setConfig unconditionally when the value is null. Fix this →

---

2. Batch sync extracts teamId from first result only — fragile assumption is undocumented

plugins/carbon/src/api-client.ts:231

teamId: result?.upsertClaudeCodeSessions?.[0]?.claudeCodeUser?.claudeCodeTeamId ?? null

This silently assumes all sessions in a batch belong to the same user/team. That's a reasonable assumption today, but since the batch API returns an array of results (each with its own claudeCodeUser), it's worth a comment explaining the assumption, or validating that [0] is defined (though a successful non-empty batch request should always return at least one element).

---

3. Migration v6 — no test coverage

The new migration is the most complex one added so far (it mutates existing data in three different ways), but there's no test for it in data-store.test.ts. All existing migration tests just verify that user_version reaches MIGRATIONS.length after init. A regression test that simulates an old-format database and verifies the migration correctly renames the key and truncates project identifiers would significantly reduce the risk of edge cases. Fix this →

---

4. getSyncConfig behavioral change silently breaks existing users with empty org

plugins/carbon/src/sync.ts:36

const organization = getConfig(db, 'claude_code_organization') ?? '';
if (!organization) return null;

This is a new guard. Before this PR, getSyncConfig only checked that userId and userName were non-null. Now an empty organization makes sync return null (effectively disabling it). Any existing user who had sync_enabled=true but never set an organization will silently stop syncing after upgrading. There's no migration to detect this state and warn the user.

This seems intentional given the setup now requires --organization, but it's worth a comment in getSyncConfig and/or handling in the migration (e.g., disabling sync if org is missing, so the user gets a clear message at next setup check).

---

Minor Notes

5. Stale test fixtures use old-format project identifiers

plugins/carbon/src/data-store.test.ts:46, 217, etc.

makeSession uses projectIdentifier: 'test_project_abcd1234' (a 22-char old-format string), and filter tests use 'org_project-a_aaaa1111'. With the new convention being 8-char hashes only, these fixtures are confusing and slightly misleading. They still pass because in-memory test DBs skip migration v6 (which only runs on upgrade), but they could trip up someone reading the tests. Consider updating them to use 8-char hashes like 'abcd1234'.

6. columnExists uses string interpolation for table name

plugins/carbon/src/data-store.ts:265

const cols = db.prepare(`PRAGMA table_info(${table})`).all() as { name: string }[];

SQLite PRAGMA statements don't support parameterized binding, so interpolation is the only option here. All callers pass hardcoded literals, so there's no real risk — but a brief comment noting this would prevent a future reader from thinking it's a mistake.

---

Summary

The PR is logically sound and the team-based dashboard URL approach is well integrated throughout the stack. The migration v6 and getSyncConfig tightening are the two riskier changes for existing users — coverage of migration v6 and documentation of the new organization requirement would make this production-ready.