diff --git a/src/registry/clients/github-browser.ts b/src/registry/clients/github-browser.ts index 257e596..faff6a9 100644 --- a/src/registry/clients/github-browser.ts +++ b/src/registry/clients/github-browser.ts @@ -12,7 +12,7 @@ */ import type { HttpClient } from "#/core"; -import type { ResolvedRegistry } from "../registry.types"; +import type { ResolvedGitHubRegistry } from "../registry.types"; import type { RegistryBrowser, BrowseResult, BrowsedArtifact } from "../browse.types"; import { ArtifactManifestSchema } from "#/schemas"; import { parse as parseYaml } from "yaml"; @@ -48,11 +48,7 @@ export class GitHubRepositoryBrowser implements RegistryBrowser { private token?: string; private http: HttpClient; - constructor(registry: ResolvedRegistry, http: HttpClient) { - if (!registry.project) { - throw new Error("GitHub browser requires 'project' field (owner/repo format)"); - } - + constructor(registry: ResolvedGitHubRegistry, http: HttpClient) { this.apiHost = resolveApiHost(registry.host); this.token = registry.token; this.http = http; diff --git a/src/registry/clients/github.test.ts b/src/registry/clients/github.test.ts index 9c21ad5..e887c9b 100644 --- a/src/registry/clients/github.test.ts +++ b/src/registry/clients/github.test.ts @@ -7,14 +7,18 @@ import { createMockTarOperations, jsonResponse, } from "#/test-utils/mocks"; -import type { ResolvedRegistry } from "../registry.types"; +import type { ResolvedGitHubRegistry } from "../registry.types"; describe("GitHubRegistryClient", () => { - const createClient = (registry: Partial = {}) => { - const fullRegistry: ResolvedRegistry = { - type: "github", - host: "ghcr.io", - project: "myorg", + const defaultGitHubRegistry: ResolvedGitHubRegistry = { + type: "github", + host: "ghcr.io", + project: "myorg", + }; + + const createClient = (registry: Partial = {}) => { + const fullRegistry: ResolvedGitHubRegistry = { + ...defaultGitHubRegistry, ...registry, }; const http = createMockHttpClient(); @@ -31,37 +35,8 @@ describe("GitHubRegistryClient", () => { }; }; - describe("constructor", () => { - test("throws when project field is missing", () => { - const registry: ResolvedRegistry = { - type: "github", - host: "ghcr.io", - // project is missing - }; - const http = createMockHttpClient(); - const fs = createMockFileSystem(); - const shell = createMockShellExecutor(); - - expect( - () => new GitHubRegistryClient(registry, http, fs, shell, createMockTarOperations()) - ).toThrow("GitHub registry requires 'project' field in config"); - }); - - test("accepts registry with project field", () => { - const registry: ResolvedRegistry = { - type: "github", - host: "ghcr.io", - project: "myorg", - }; - const http = createMockHttpClient(); - const fs = createMockFileSystem(); - const shell = createMockShellExecutor(); - - expect( - () => new GitHubRegistryClient(registry, http, fs, shell, createMockTarOperations()) - ).not.toThrow(); - }); - }); + // constructor: project is now required by the type system (discriminated union) + // so runtime null checks are no longer needed describe("prefix configuration", () => { test("prepends prefix to repository name when configured", async () => { @@ -78,7 +53,7 @@ describe("GitHubRegistryClient", () => { return jsonResponse({ tags: [] }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -104,7 +79,7 @@ describe("GitHubRegistryClient", () => { return jsonResponse({ tags: [] }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -130,7 +105,7 @@ describe("GitHubRegistryClient", () => { return jsonResponse({ tags: [] }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -175,7 +150,7 @@ describe("GitHubRegistryClient", () => { const shell = createMockShellExecutor({ tar: "" }); fs.files.set("/target/file.md", { content: "content", isDirectory: false }); - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -201,7 +176,7 @@ describe("GitHubRegistryClient", () => { return jsonResponse({}); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -221,7 +196,7 @@ describe("GitHubRegistryClient", () => { return jsonResponse({}); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -290,7 +265,7 @@ describe("GitHubRegistryClient", () => { }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -362,7 +337,7 @@ describe("GitHubRegistryClient", () => { return new Response("Forbidden", { status: 403 }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -395,7 +370,7 @@ describe("GitHubRegistryClient", () => { }); test("returns error when oras is not installed", async () => { - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", @@ -426,7 +401,7 @@ describe("GitHubRegistryClient", () => { }); test("passes relative tarball path to oras", async () => { - const registry: ResolvedRegistry = { + const registry: ResolvedGitHubRegistry = { type: "github", host: "ghcr.io", project: "myorg", diff --git a/src/registry/clients/github.ts b/src/registry/clients/github.ts index 6ad4cc8..2e77bde 100644 --- a/src/registry/clients/github.ts +++ b/src/registry/clients/github.ts @@ -14,7 +14,7 @@ import { relative } from "path"; import { validateTarballContents, generateSecureTempPath, type FileSystem, type HttpClient, type ShellExecutor, type TarOperations } from "#/core"; import type { RegistryClient, - ResolvedRegistry, + ResolvedGitHubRegistry, DownloadResult, PublishResult, RegistryArtifactInfo, @@ -38,18 +38,12 @@ export class GitHubRegistryClient implements RegistryClient { private ociClient: OciClient; constructor( - registry: ResolvedRegistry, + registry: ResolvedGitHubRegistry, http: HttpClient, fs: FileSystem, shell: ShellExecutor, tar: TarOperations ) { - if (!registry.project) { - throw new Error( - "GitHub registry requires 'project' field in config (your GHCR namespace, e.g., 'myorg' for ghcr.io/myorg/*)" - ); - } - this.host = registry.host || DEFAULT_GHCR_HOST; this.token = registry.token; this.prefix = registry.prefix; diff --git a/src/registry/clients/gitlab-browser.ts b/src/registry/clients/gitlab-browser.ts index e7f2cc5..25fbbde 100644 --- a/src/registry/clients/gitlab-browser.ts +++ b/src/registry/clients/gitlab-browser.ts @@ -12,7 +12,7 @@ */ import type { HttpClient } from "#/core"; -import type { ResolvedRegistry } from "../registry.types"; +import type { ResolvedGitLabRegistry } from "../registry.types"; import type { RegistryBrowser, BrowseResult, BrowsedArtifact } from "../browse.types"; import { ArtifactManifestSchema } from "#/schemas"; import { parse as parseYaml } from "yaml"; @@ -34,11 +34,7 @@ export class GitLabRepositoryBrowser implements RegistryBrowser { private token?: string; private http: HttpClient; - constructor(registry: ResolvedRegistry, http: HttpClient) { - if (!registry.project) { - throw new Error("GitLab browser requires 'project' field in config"); - } - + constructor(registry: ResolvedGitLabRegistry, http: HttpClient) { this.host = normalizeHost(registry.host); this.encodedProject = encodeURIComponent(normalizeProject(registry.project)); this.token = registry.token; diff --git a/src/registry/clients/gitlab.test.ts b/src/registry/clients/gitlab.test.ts index df95472..313747e 100644 --- a/src/registry/clients/gitlab.test.ts +++ b/src/registry/clients/gitlab.test.ts @@ -8,17 +8,21 @@ import { binaryResponse, errorResponse, } from "#/test-utils/mocks"; -import type { ResolvedRegistry } from "../registry.types"; +import type { ResolvedGitLabRegistry } from "../registry.types"; describe("GitLabRegistryClient", () => { + const defaultGitLabRegistry: ResolvedGitLabRegistry = { + type: "gitlab", + host: "gitlab.com", + project: "group/project", + }; + const createClient = ( - registry: Partial = {}, + registry: Partial = {}, httpResponses = new Map Response)>() ) => { - const fullRegistry: ResolvedRegistry = { - type: "gitlab", - host: "gitlab.com", - project: "group/project", + const fullRegistry: ResolvedGitLabRegistry = { + ...defaultGitLabRegistry, ...registry, }; const http = createMockHttpClient(httpResponses); @@ -34,31 +38,6 @@ describe("GitLabRegistryClient", () => { }; describe("constructor", () => { - test("throws when project field is missing", () => { - const registry: ResolvedRegistry = { - type: "gitlab", - host: "gitlab.com", - // project is missing - }; - const http = createMockHttpClient(); - const fs = createMockFileSystem(); - expect( - () => new GitLabRegistryClient(registry, http, fs, createMockTarOperations()) - ).toThrow("GitLab registry requires 'project' field in config"); - }); - - test("accepts registry with project field", () => { - const registry: ResolvedRegistry = { - type: "gitlab", - host: "gitlab.com", - project: "group/project", - }; - const http = createMockHttpClient(); - const fs = createMockFileSystem(); - expect( - () => new GitLabRegistryClient(registry, http, fs, createMockTarOperations()) - ).not.toThrow(); - }); test("normalizes host by stripping https:// prefix", async () => { let requestedUrl = ""; @@ -69,7 +48,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "https://gitlab.example.com", project: "group/project", @@ -91,7 +70,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "http://gitlab.internal", project: "group/project", @@ -113,7 +92,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/subgroup/project", @@ -134,7 +113,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "/group/subgroup/project", @@ -155,7 +134,7 @@ describe("GitLabRegistryClient", () => { throw new Error("fetch failed"); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.unreachable.com", project: "group/project", @@ -172,7 +151,7 @@ describe("GitLabRegistryClient", () => { const http = createMockHttpClient(); http.fetch = async () => errorResponse(401, "Unauthorized"); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -269,7 +248,7 @@ describe("GitLabRegistryClient", () => { const fs = createMockFileSystem(); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.mycompany.com", project: "team/artifacts", @@ -328,7 +307,7 @@ describe("GitLabRegistryClient", () => { return errorResponse(404, "Not Found"); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -361,7 +340,7 @@ describe("GitLabRegistryClient", () => { "/path/to/tarball.tar.gz": Buffer.from("tarball content"), }); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -480,7 +459,7 @@ describe("GitLabRegistryClient", () => { return errorResponse(404, "Not Found"); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -496,7 +475,7 @@ describe("GitLabRegistryClient", () => { const http = createMockHttpClient(); http.fetch = async () => errorResponse(404, "Not Found"); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -519,7 +498,7 @@ describe("GitLabRegistryClient", () => { return new Response(null, { status: 200 }); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -600,7 +579,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -623,7 +602,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -646,7 +625,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -671,7 +650,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -694,7 +673,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -730,7 +709,7 @@ describe("GitLabRegistryClient", () => { const fs = createMockFileSystem(); fs.files.set("/target/file.md", { content: "content", isDirectory: false }); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -762,7 +741,7 @@ describe("GitLabRegistryClient", () => { "/path/to/tarball.tar.gz": Buffer.from("tarball content"), }); - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", @@ -786,7 +765,7 @@ describe("GitLabRegistryClient", () => { return jsonResponse([]); }; - const registry: ResolvedRegistry = { + const registry: ResolvedGitLabRegistry = { type: "gitlab", host: "gitlab.com", project: "group/project", diff --git a/src/registry/clients/gitlab.ts b/src/registry/clients/gitlab.ts index 4481c48..8a323d8 100644 --- a/src/registry/clients/gitlab.ts +++ b/src/registry/clients/gitlab.ts @@ -13,7 +13,7 @@ import { validateTarballContents, generateSecureTempPath, type FileSystem, type HttpClient, type TarOperations } from "#/core"; import type { RegistryClient, - ResolvedRegistry, + ResolvedGitLabRegistry, DownloadResult, PublishResult, RegistryArtifactInfo, @@ -40,15 +40,11 @@ export class GitLabRegistryClient implements RegistryClient { private tar: TarOperations; constructor( - registry: ResolvedRegistry, + registry: ResolvedGitLabRegistry, http: HttpClient, fs: FileSystem, tar: TarOperations ) { - if (!registry.project) { - throw new Error("GitLab registry requires 'project' field in config"); - } - this.host = normalizeHost(registry.host); // URL-encode the project path for API calls // GitLab API accepts both numeric IDs and URL-encoded paths diff --git a/src/registry/factory.test.ts b/src/registry/factory.test.ts index df7351a..9b82dd5 100644 --- a/src/registry/factory.test.ts +++ b/src/registry/factory.test.ts @@ -53,17 +53,8 @@ describe("factory", () => { expect(client).toBeInstanceOf(DefaultRegistryClient); }); - test("throws when gitlab registry missing project", () => { - const registry: ResolvedRegistry = { - type: "gitlab", - host: "gitlab.com", - // project is missing - }; - - expect(() => createRegistryClient(registry, http, fs, shell, tar)).toThrow( - "GitLab registry requires 'project' field in config" - ); - }); + // project is now required by the type system for gitlab/github types, + // so missing-project tests are no longer needed test("passes token to gitlab client", () => { const registry: ResolvedRegistry = { diff --git a/src/registry/registry.types.ts b/src/registry/registry.types.ts index 10f0759..05db5c9 100644 --- a/src/registry/registry.types.ts +++ b/src/registry/registry.types.ts @@ -12,18 +12,31 @@ export type { LocalConfig, RegistryEntry } from "#/schemas"; export type RegistryType = "gitlab" | "github" | "default"; /** - * Normalized registry configuration. - * Created by resolver from raw config, used by factory to create clients. + * Shared fields for all resolved registry configurations. */ -export interface ResolvedRegistry { - type: RegistryType; +interface ResolvedRegistryBase { host: string; - project?: string; token?: string; - prefix?: string; // Package name prefix (e.g., "frontend" → "frontend-artifact-name") + prefix?: string; // Artifact name prefix (e.g., "frontend" → "frontend-artifact-name") apiBasePath?: string; // REST API base path for default registry (e.g., "/functions/v1") } +/** + * Normalized registry configuration. + * Created by resolver from raw config, used by factory to create clients. + * + * Uses discriminated union: project is guaranteed for gitlab/github types, + * eliminating runtime null checks in registry clients. + */ +export type ResolvedGitLabRegistry = ResolvedRegistryBase & { type: "gitlab"; project: string }; +export type ResolvedGitHubRegistry = ResolvedRegistryBase & { type: "github"; project: string }; +export type ResolvedDefaultRegistry = ResolvedRegistryBase & { type: "default"; project?: string }; + +export type ResolvedRegistry = + | ResolvedGitLabRegistry + | ResolvedGitHubRegistry + | ResolvedDefaultRegistry; + /** * Result from download operation */ diff --git a/src/registry/resolver.ts b/src/registry/resolver.ts index f435bb5..3cdf124 100644 --- a/src/registry/resolver.ts +++ b/src/registry/resolver.ts @@ -105,14 +105,17 @@ export function resolveRegistry( } } - return { - type: entry.type, - host: entry.host || getDefaultHost(entry.type), - project: entry.project, - token, - prefix: entry.prefix, - apiBasePath: entry.type === "default" ? DEFAULT_REGISTRY_API_PATH : undefined, - }; + const host = entry.host || getDefaultHost(entry.type); + const base = { host, token, prefix: entry.prefix }; + + switch (entry.type) { + case "gitlab": + return { ...base, type: "gitlab" as const, project: entry.project }; + case "github": + return { ...base, type: "github" as const, project: entry.project }; + case "default": + return { ...base, type: "default" as const, project: entry.project, apiBasePath: DEFAULT_REGISTRY_API_PATH }; + } } /** diff --git a/src/schemas/index.test.ts b/src/schemas/index.test.ts index fae1a53..5a81e49 100644 --- a/src/schemas/index.test.ts +++ b/src/schemas/index.test.ts @@ -607,7 +607,7 @@ describe("schemas", () => { }); describe("RegistryEntrySchema", () => { - test("parses gitlab registry", () => { + test("parses gitlab registry with required project", () => { const entry = { type: "gitlab" as const, project: "myteam/artifacts", @@ -632,13 +632,37 @@ describe("schemas", () => { expect(result.host).toBe("gitlab.company.com"); }); - test("accepts all valid types", () => { - const types = ["gitlab", "github", "default"] as const; + test("parses github registry with required project", () => { + const entry = { + type: "github" as const, + project: "myorg", + }; - for (const type of types) { - const result = RegistryEntrySchema.parse({ type }); - expect(result.type).toBe(type); - } + const result = RegistryEntrySchema.parse(entry); + + expect(result.type).toBe("github"); + expect(result.project).toBe("myorg"); + }); + + test("parses default registry without project", () => { + const entry = { type: "default" as const }; + + const result = RegistryEntrySchema.parse(entry); + + expect(result.type).toBe("default"); + expect(result.project).toBeUndefined(); + }); + + test("rejects gitlab without project", () => { + const entry = { type: "gitlab" as const }; + + expect(() => RegistryEntrySchema.parse(entry)).toThrow(); + }); + + test("rejects github without project", () => { + const entry = { type: "github" as const }; + + expect(() => RegistryEntrySchema.parse(entry)).toThrow(); }); }); diff --git a/src/schemas/index.ts b/src/schemas/index.ts index 4cbcbeb..58dcf7f 100644 --- a/src/schemas/index.ts +++ b/src/schemas/index.ts @@ -208,14 +208,32 @@ export const ArtifactMetadataSchema = z.object({ }); export type ArtifactMetadata = z.infer; -// Registry entry for local config (.grekt/config.yaml) -export const RegistryEntrySchema = z.object({ - type: z.enum(["gitlab", "github", "default"]), - project: z.string().optional(), // Required for gitlab/github, validated at runtime +// Shared optional fields for all registry types +const registrySharedFields = { host: z.string().optional(), // Optional, has defaults (gitlab.com, github.com) token: z.string().optional(), // Can also be set via env vars - prefix: z.string().optional(), // Package name prefix (e.g., "frontend" → "frontend-artifact-name") -}); + prefix: z.string().optional(), // Artifact name prefix (e.g., "frontend" → "frontend-artifact-name") +}; + +// Registry entry for local config (.grekt/config.yaml) +// Uses discriminated union: project is required for gitlab/github, optional for default +export const RegistryEntrySchema = z.discriminatedUnion("type", [ + z.object({ + type: z.literal("gitlab"), + project: z.string(), // Required: GitLab project path (e.g., "myteam/artifacts") + ...registrySharedFields, + }), + z.object({ + type: z.literal("github"), + project: z.string(), // Required: GHCR namespace (e.g., "myorg") + ...registrySharedFields, + }), + z.object({ + type: z.literal("default"), + project: z.string().optional(), + ...registrySharedFields, + }), +]); export type RegistryEntry = z.infer; // Session stored in local config (generated by grekt login)