From 0ec9fae2f7c07026c29934650c9a20e2ebcb3689 Mon Sep 17 00:00:00 2001 From: yeonigi Date: Sat, 11 Jul 2026 18:25:03 +0900 Subject: [PATCH] refactor: pass publish overrides to resolveConfig without re-listing (#50) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runPublish hand-listed every Overrides field when building the object it passed to resolveConfig. Each new Overrides field had to be added there too, and forgetting silently dropped it in publish only — exactly the footgun that hid the `--mode` bug fixed in #48. Redefine PublishArgs as `{ path; slug?; title?; force?; dryRun?; profile? } & Overrides` and pass `args` straight to resolveConfig (which reads only the Overrides keys), matching how open/rm/list already thread overrides. Single source of truth, no second sync point. Add command-level threading coverage (publish/open/rm/list) so a dropped override field is caught at the command layer, not just in resolveConfig unit tests. Closes #50 --- src/commands/publish.ts | 35 ++++++++++++----------------------- test/list.test.ts | 12 ++++++++++++ test/open.test.ts | 6 ++++++ test/publish.test.ts | 14 ++++++++++++++ test/rm.test.ts | 18 ++++++++++++++++++ 5 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/commands/publish.ts b/src/commands/publish.ts index 5733e7e..ed36eb5 100644 --- a/src/commands/publish.ts +++ b/src/commands/publish.ts @@ -1,5 +1,5 @@ import { readFile } from "node:fs/promises"; -import { resolveConfig, type Mode } from "../lib/config.js"; +import { resolveConfig, type Overrides } from "../lib/config.js"; import { ensureHost } from "../lib/host.js"; import { makeBackend, type StorageBackend } from "../lib/backend.js"; import { generateCode, isValidPath } from "../lib/code.js"; @@ -11,23 +11,20 @@ import { mapLimit } from "../lib/concurrency.js"; const UPLOAD_CONCURRENCY = 8; -export interface PublishArgs { +/** + * Publish-specific fields plus the shared config Overrides. Intersecting with + * Overrides (rather than re-listing bucket/region/domain/... by hand) keeps a + * single source of truth: a new Overrides field flows straight to resolveConfig + * without a second place to update. + */ +export type PublishArgs = { path: string; slug?: string; title?: string; force?: boolean; dryRun?: boolean; - region?: string; profile?: string; - bucket?: string; - domain?: string; - distribution?: string; - serveRoot?: string; - host?: string; - port?: number; - scheme?: "http" | "https"; - mode?: Mode; -} +} & Overrides; async function uniqueCode(backend: StorageBackend): Promise { for (let i = 0; i < 5; i++) { @@ -44,17 +41,9 @@ async function deriveTitle(uploads: Upload[]): Promise { } export async function runPublish(args: PublishArgs): Promise { - const cfg = await ensureHost(resolveConfig({ - bucket: args.bucket, - region: args.region, - domain: args.domain, - distribution: args.distribution, - serveRoot: args.serveRoot, - host: args.host, - port: args.port, - scheme: args.scheme, - mode: args.mode, - })); + // args is `PublishArgs & Overrides`; resolveConfig reads only the Overrides + // keys and ignores the publish-specific extras (path, slug, profile, ...). + const cfg = await ensureHost(resolveConfig(args)); const backend = makeBackend(cfg, { profile: args.profile }); const uploads = await collectUploads(args.path); diff --git a/test/list.test.ts b/test/list.test.ts index 0ed329c..844f5f4 100644 --- a/test/list.test.ts +++ b/test/list.test.ts @@ -67,6 +67,18 @@ describe("listDocs", () => { expect(rows[0].url).toBe("http://b.s3-website-us-east-1.amazonaws.com/team/q1/report/"); }); + it("threads a region override through to resolveConfig", async () => { + s3mock + .on(ListObjectsV2Command) + .resolves({ Contents: [{ Key: "_meta/a.json" }], IsTruncated: false }); + s3mock + .on(GetObjectCommand, { Key: "_meta/a.json" }) + .resolves({ Body: { transformToString: async () => JSON.stringify({ code: "a", slug: null, title: "A", createdAt: "2026-01-01T00:00:00Z", files: 1, bytes: 1, sourcePath: "/a" }) } as any }); + + const rows = await listDocs({ region: "eu-west-1" }); + expect(rows[0].url).toBe("http://b.s3-website-eu-west-1.amazonaws.com/a/"); + }); + it("skips a sidecar missing createdAt without crashing the sort", async () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); s3mock diff --git a/test/open.test.ts b/test/open.test.ts index c288b05..5f9acfa 100644 --- a/test/open.test.ts +++ b/test/open.test.ts @@ -67,6 +67,12 @@ describe("resolveOpenUrl", () => { "http://b.s3-website-us-east-1.amazonaws.com/team/q1/report/", ); }); + + it("threads bucket/region overrides through to resolveConfig", async () => { + expect( + await resolveOpenUrl({ id: "abc", bucket: "override-b", region: "eu-west-1" }), + ).toBe("http://override-b.s3-website-eu-west-1.amazonaws.com/abc/"); + }); }); describe("describeConfig", () => { diff --git a/test/publish.test.ts b/test/publish.test.ts index f471c26..de51ffe 100644 --- a/test/publish.test.ts +++ b/test/publish.test.ts @@ -106,6 +106,20 @@ describe("runPublish", () => { delete process.env.HOSTDOC_DISTRIBUTION; }); + it("threads a bucket override through to resolveConfig", async () => { + // A --bucket flag (highest precedence) must reach resolveConfig and win + // over the ambient env bucket. Guards against publish dropping an Overrides + // field on its way to resolveConfig. + writeFileSync(join(dir, "index.html"), "x"); + const url = await runPublish({ + path: dir, + slug: "doc1", + dryRun: true, + bucket: "override-b", + }); + expect(url).toBe("http://override-b.s3-website-us-east-1.amazonaws.com/doc1/"); + }); + it("invalidates //* when overwriting in cloudfront mode", async () => { process.env.HOSTDOC_DOMAIN = "shared.example.com"; process.env.HOSTDOC_DISTRIBUTION = "DIST1"; diff --git a/test/rm.test.ts b/test/rm.test.ts index 0fd5306..c2e10b9 100644 --- a/test/rm.test.ts +++ b/test/rm.test.ts @@ -82,6 +82,24 @@ describe("runRm", () => { delete process.env.HOSTDOC_DISTRIBUTION; }); + it("threads domain/distribution overrides through to resolveConfig (cloudfront)", async () => { + // ambient env is s3-website (bucket+region). Passing domain+distribution as + // flags must reach resolveConfig and derive cloudfront, which invalidates. + s3mock + .on(ListObjectsV2Command) + .resolves({ Contents: [{ Key: "doc1/index.html" }], IsTruncated: false }); + s3mock.on(DeleteObjectsCommand).resolves({}); + + await runRm({ + id: "doc1", + yes: true, + domain: "shared.example.com", + distribution: "DIST9", + }); + + expect(cfMock.commandCalls(CreateInvalidationCommand)).toHaveLength(1); + }); + it.each(["_meta", "../escape", "a b", "x//y", "x/_y", "x?y"])( "rejects invalid id %j before deleting anything", async (id) => {