From de2542e39199fabce32b3ab00aee5bb98ed28c4c Mon Sep 17 00:00:00 2001 From: Sergiy Dybskiy Date: Sun, 25 Jan 2026 21:12:02 +0000 Subject: [PATCH 1/4] fix: return proper HTTP status codes for delete/undelete errors The delete and undelete handlers for skills and souls were catching all errors and returning 401 Unauthorized, even for errors like: - 'Skill not found' (should be 404) - 'Forbidden' (should be 403) - Other validation errors (should be 400) This change updates the error handling to return appropriate status codes: - 401 Unauthorized: authentication failures - 403 Forbidden: authorization failures (not owner/admin/moderator) - 404 Not Found: skill/soul/user not found - 400 Bad Request: other errors with descriptive message Fixes #34 --- convex/httpApiV1.ts | 56 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/convex/httpApiV1.ts b/convex/httpApiV1.ts index 993ea28..80d8316 100644 --- a/convex/httpApiV1.ts +++ b/convex/httpApiV1.ts @@ -461,8 +461,18 @@ async function skillsPostRouterV1Handler(ctx: ActionCtx, request: Request) { deleted: false, }) return json({ ok: true }, 200, rate.headers) - } catch { - return text('Unauthorized', 401, rate.headers) + } catch (error) { + const message = error instanceof Error ? error.message : 'Undelete failed' + if (message === 'Unauthorized') { + return text('Unauthorized', 401, rate.headers) + } + if (message === 'Forbidden') { + return text('Forbidden', 403, rate.headers) + } + if (message === 'Skill not found' || message === 'User not found') { + return text('Not found', 404, rate.headers) + } + return text(message, 400, rate.headers) } } @@ -483,8 +493,18 @@ async function skillsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) { deleted: true, }) return json({ ok: true }, 200, rate.headers) - } catch { - return text('Unauthorized', 401, rate.headers) + } catch (error) { + const message = error instanceof Error ? error.message : 'Delete failed' + if (message === 'Unauthorized') { + return text('Unauthorized', 401, rate.headers) + } + if (message === 'Forbidden') { + return text('Forbidden', 403, rate.headers) + } + if (message === 'Skill not found' || message === 'User not found') { + return text('Not found', 404, rate.headers) + } + return text(message, 400, rate.headers) } } @@ -1052,8 +1072,18 @@ async function soulsPostRouterV1Handler(ctx: ActionCtx, request: Request) { deleted: false, }) return json({ ok: true }, 200, rate.headers) - } catch { - return text('Unauthorized', 401, rate.headers) + } catch (error) { + const message = error instanceof Error ? error.message : 'Undelete failed' + if (message === 'Unauthorized') { + return text('Unauthorized', 401, rate.headers) + } + if (message === 'Forbidden') { + return text('Forbidden', 403, rate.headers) + } + if (message === 'Soul not found' || message === 'User not found') { + return text('Not found', 404, rate.headers) + } + return text(message, 400, rate.headers) } } @@ -1074,8 +1104,18 @@ async function soulsDeleteRouterV1Handler(ctx: ActionCtx, request: Request) { deleted: true, }) return json({ ok: true }, 200, rate.headers) - } catch { - return text('Unauthorized', 401, rate.headers) + } catch (error) { + const message = error instanceof Error ? error.message : 'Delete failed' + if (message === 'Unauthorized') { + return text('Unauthorized', 401, rate.headers) + } + if (message === 'Forbidden') { + return text('Forbidden', 403, rate.headers) + } + if (message === 'Soul not found' || message === 'User not found') { + return text('Not found', 404, rate.headers) + } + return text(message, 400, rate.headers) } } From f1a525475502c10088c7d21e8ff56aa507c016a3 Mon Sep 17 00:00:00 2001 From: Sergiy Dybskiy Date: Sun, 25 Jan 2026 21:39:54 +0000 Subject: [PATCH 2/4] fix(cli): use proper Error objects in abort timeouts When AbortController.abort() receives a string instead of an Error, the string itself is thrown. pRetry then wraps it in a confusing message: 'Non-error was thrown: Timeout' Changed all 3 occurrences in http.ts: - apiRequest (line 57) - apiRequestForm (line 106) - downloadZip (line 141) Now timeouts will surface as proper Error objects with clear messages. --- packages/clawdhub/src/http.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/clawdhub/src/http.ts b/packages/clawdhub/src/http.ts index 9c72a59..647b8f5 100644 --- a/packages/clawdhub/src/http.ts +++ b/packages/clawdhub/src/http.ts @@ -54,7 +54,7 @@ export async function apiRequest( body = JSON.stringify(args.body ?? {}) } const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) const response = await fetch(url, { method: args.method, headers, @@ -103,7 +103,7 @@ export async function apiRequestForm( const headers: Record = { Accept: 'application/json' } if (args.token) headers.Authorization = `Bearer ${args.token}` const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) const response = await fetch(url, { method: args.method, headers, @@ -138,7 +138,7 @@ export async function downloadZip(registry: string, args: { slug: string; versio } const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) const response = await fetch(url.toString(), { method: 'GET', signal: controller.signal }) clearTimeout(timeout) if (!response.ok) { From 1ae04985959a4061461fa055a01df48da9d41d27 Mon Sep 17 00:00:00 2001 From: Sergiy Dybskiy Date: Mon, 26 Jan 2026 12:39:06 +0000 Subject: [PATCH 3/4] test: add e2e test for delete error handling Verifies that deleting a non-existent skill returns a proper 'not found' error instead of a generic 'Unauthorized' message. --- e2e/clawdhub.e2e.test.ts | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/e2e/clawdhub.e2e.test.ts b/e2e/clawdhub.e2e.test.ts index 436510d..d648154 100644 --- a/e2e/clawdhub.e2e.test.ts +++ b/e2e/clawdhub.e2e.test.ts @@ -491,4 +491,49 @@ describe('clawdhub e2e', () => { await rm(cfg.dir, { recursive: true, force: true }) } }, 180_000) + + it('delete returns proper error for non-existent skill', async () => { + const registry = process.env.CLAWDHUB_REGISTRY?.trim() || 'https://clawdhub.com' + const site = process.env.CLAWDHUB_SITE?.trim() || 'https://clawdhub.com' + const token = mustGetToken() ?? (await readGlobalConfig())?.token ?? null + if (!token) { + throw new Error('Missing token. Set CLAWDHUB_E2E_TOKEN or run: bun clawdhub auth login') + } + + const cfg = await makeTempConfig(registry, token) + const workdir = await mkdtemp(join(tmpdir(), 'clawdhub-e2e-delete-')) + const nonExistentSlug = `non-existent-skill-${Date.now()}` + + try { + const del = spawnSync( + 'bun', + [ + 'clawdhub', + 'delete', + nonExistentSlug, + '--yes', + '--site', + site, + '--registry', + registry, + '--workdir', + workdir, + ], + { + cwd: process.cwd(), + env: { ...process.env, CLAWDHUB_CONFIG_PATH: cfg.path, CLAWDHUB_DISABLE_TELEMETRY: '1' }, + encoding: 'utf8', + }, + ) + // Should fail with non-zero exit code + expect(del.status).not.toBe(0) + // Error should mention "not found" - not generic "Unauthorized" + const output = (del.stdout + del.stderr).toLowerCase() + expect(output).toMatch(/not found|404|does not exist/i) + expect(output).not.toMatch(/unauthorized/i) + } finally { + await rm(workdir, { recursive: true, force: true }) + await rm(cfg.dir, { recursive: true, force: true }) + } + }, 30_000) }) From eb9a67f2af8c2502a66bad9e49bf4088260397f9 Mon Sep 17 00:00:00 2001 From: Sergiy Dybskiy Date: Mon, 26 Jan 2026 12:41:29 +0000 Subject: [PATCH 4/4] fix: use Error for timeout abort in e2e helper --- e2e/clawdhub.e2e.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/clawdhub.e2e.test.ts b/e2e/clawdhub.e2e.test.ts index d648154..6f4cff5 100644 --- a/e2e/clawdhub.e2e.test.ts +++ b/e2e/clawdhub.e2e.test.ts @@ -47,7 +47,7 @@ async function makeTempConfig(registry: string, token: string | null) { async function fetchWithTimeout(input: RequestInfo | URL, init?: RequestInit) { const controller = new AbortController() - const timeout = setTimeout(() => controller.abort('Timeout'), REQUEST_TIMEOUT_MS) + const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS) try { return await fetch(input, { ...init, signal: controller.signal }) } finally {