diff --git a/CHANGELOG.md b/CHANGELOG.md index 5442a8cfb..94a2e215d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +### Fixed +- Users: sync handle on ensure when GitHub login changes (#293) (thanks @christianhpoe). + ## 0.6.1 - 2026-02-13 ### Added diff --git a/convex/users.test.ts b/convex/users.test.ts new file mode 100644 index 000000000..0c65a3416 --- /dev/null +++ b/convex/users.test.ts @@ -0,0 +1,93 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('./lib/access', async () => { + const actual = await vi.importActual('./lib/access') + return { ...actual, requireUser: vi.fn() } +}) + +const { requireUser } = await import('./lib/access') +const { ensureHandler } = await import('./users') + +describe('ensureHandler', () => { + afterEach(() => { + vi.mocked(requireUser).mockReset() + }) + + it('updates handle and display name when GitHub login changes', async () => { + vi.mocked(requireUser).mockResolvedValue({ + userId: 'users:1', + user: { + _creationTime: 1, + handle: 'old-handle', + displayName: 'old-handle', + name: 'new-handle', + email: 'old@example.com', + role: 'user', + createdAt: 1, + }, + } as never) + + const patch = vi.fn() + const get = vi.fn() + const ctx = { db: { patch, get } } + + await ensureHandler(ctx as never) + + expect(patch).toHaveBeenCalledWith('users:1', { + handle: 'new-handle', + displayName: 'new-handle', + updatedAt: expect.any(Number), + }) + }) + + it('does not override a custom display name when syncing handle', async () => { + vi.mocked(requireUser).mockResolvedValue({ + userId: 'users:2', + user: { + _creationTime: 1, + handle: 'old-handle', + displayName: 'Custom Name', + name: 'new-handle', + role: 'user', + createdAt: 1, + }, + } as never) + + const patch = vi.fn() + const get = vi.fn() + const ctx = { db: { patch, get } } + + await ensureHandler(ctx as never) + + expect(patch).toHaveBeenCalledWith('users:2', { + handle: 'new-handle', + updatedAt: expect.any(Number), + }) + }) + + it('fills display name from existing handle when missing', async () => { + vi.mocked(requireUser).mockResolvedValue({ + userId: 'users:3', + user: { + _creationTime: 1, + handle: 'steady-handle', + displayName: undefined, + name: undefined, + email: undefined, + role: 'user', + createdAt: 1, + }, + } as never) + + const patch = vi.fn() + const get = vi.fn() + const ctx = { db: { patch, get } } + + await ensureHandler(ctx as never) + + expect(patch).toHaveBeenCalledWith('users:3', { + displayName: 'steady-handle', + updatedAt: expect.any(Number), + }) + }) +}) diff --git a/convex/users.ts b/convex/users.ts index f2e857399..88c399b59 100644 --- a/convex/users.ts +++ b/convex/users.ts @@ -74,26 +74,45 @@ export const me = query({ export const ensure = mutation({ args: {}, - handler: async (ctx) => { - const { userId, user } = await requireUser(ctx) - const updates: Record = {} - - const handle = user.handle || user.name || user.email?.split('@')[0] - if (!user.handle && handle) updates.handle = handle - if (!user.displayName) updates.displayName = handle - if (!user.role) { - updates.role = handle === ADMIN_HANDLE ? 'admin' : DEFAULT_ROLE - } - if (!user.createdAt) updates.createdAt = user._creationTime + handler: ensureHandler, +}) - if (Object.keys(updates).length > 0) { - updates.updatedAt = Date.now() - await ctx.db.patch(userId, updates) - } +export async function ensureHandler(ctx: MutationCtx) { + const { userId, user } = await requireUser(ctx) + const updates: Record = {} - return ctx.db.get(userId) - }, -}) + const existingHandle = user.handle?.trim() || undefined + const nameHandle = user.name?.trim() || undefined + const emailHandle = user.email?.split('@')[0]?.trim() || undefined + // `user.name` is the GitHub login (see convex/auth.ts profile mapping). + // Only fall back to deriving from email if we don't already have a handle. + const derivedHandle = nameHandle || (!existingHandle ? emailHandle : undefined) + const baseHandle = derivedHandle ?? existingHandle + + if (derivedHandle && (!existingHandle || existingHandle !== derivedHandle)) { + updates.handle = derivedHandle + } + + const displayName = user.displayName?.trim() || undefined + if (!displayName && baseHandle) { + updates.displayName = baseHandle + } else if (derivedHandle && displayName === existingHandle) { + updates.displayName = derivedHandle + } + + if (!user.role) { + updates.role = baseHandle === ADMIN_HANDLE ? 'admin' : DEFAULT_ROLE + } + + if (!user.createdAt) updates.createdAt = user._creationTime + + if (Object.keys(updates).length > 0) { + updates.updatedAt = Date.now() + await ctx.db.patch(userId, updates) + } + + return ctx.db.get(userId) +} export const updateProfile = mutation({ args: { diff --git a/packages/clawdhub/src/skills.test.ts b/packages/clawdhub/src/skills.test.ts index 067e37c03..e9f7b3eda 100644 --- a/packages/clawdhub/src/skills.test.ts +++ b/packages/clawdhub/src/skills.test.ts @@ -20,15 +20,18 @@ import { describe('skills', () => { it('extracts zip into directory and skips traversal', async () => { - const dir = await mkdtemp(join(tmpdir(), 'clawhub-')) + const parent = await mkdtemp(join(tmpdir(), 'clawhub-zip-')) + const dir = join(parent, 'dir') + await mkdir(dir) + const evilName = `evil-${parent.split('/').pop() ?? 'file'}.txt` const zip = zipSync({ 'SKILL.md': strToU8('hello'), - '../evil.txt': strToU8('nope'), + [`../${evilName}`]: strToU8('nope'), }) await extractZipToDir(new Uint8Array(zip), dir) expect((await readFile(join(dir, 'SKILL.md'), 'utf8')).trim()).toBe('hello') - await expect(stat(join(dir, '..', 'evil.txt'))).rejects.toBeTruthy() + await expect(stat(join(parent, evilName))).rejects.toBeTruthy() }) it('writes and reads lockfile', async () => {