diff --git a/.github/workflows/e2e-api-tests.yaml b/.github/workflows/e2e-api-tests.yaml index 5524855d1..74918d292 100644 --- a/.github/workflows/e2e-api-tests.yaml +++ b/.github/workflows/e2e-api-tests.yaml @@ -132,10 +132,13 @@ jobs: - name: Backfill Bulldozer from Postgres run: pnpm run db:backfill-bulldozer-from-prisma + # The built server (node dist/server.mjs) does not auto-load .env files the way + # `next start` used to, so it must be wrapped in with-env:test to pick up + # .env.test.local (mirrors the e2e-fallback-tests workflow). - name: Start stack-backend in background uses: JarvusInnovations/background-action@2428e7b970a846423095c79d43f759abf979a635 # v1.0.7 with: - run: pnpm run start:backend --log-order=stream & + run: pnpm -C apps/backend run with-env:test pnpm run start & wait-on: | http://localhost:8102 tail: true diff --git a/.github/workflows/e2e-custom-base-port-api-tests.yaml b/.github/workflows/e2e-custom-base-port-api-tests.yaml index 34836b66b..3018bff73 100644 --- a/.github/workflows/e2e-custom-base-port-api-tests.yaml +++ b/.github/workflows/e2e-custom-base-port-api-tests.yaml @@ -126,10 +126,15 @@ jobs: - name: Backfill Bulldozer from Postgres run: pnpm run db:backfill-bulldozer-from-prisma + # The built server (node dist/server.mjs) does not auto-load .env files the way + # `next start` used to, so it must be wrapped in with-env:test to pick up + # .env.test.local (mirrors the e2e-fallback-tests workflow). The job-level + # NEXT_PUBLIC_HEXCLAVE_PORT_PREFIX=67 stays in effect because dotenv does not + # override already-set process env vars. - name: Start stack-backend in background uses: JarvusInnovations/background-action@2428e7b970a846423095c79d43f759abf979a635 # v1.0.7 with: - run: pnpm run start:backend --log-order=stream & + run: pnpm -C apps/backend run with-env:test pnpm run start & wait-on: | http://localhost:6702 tail: true diff --git a/apps/backend/src/app/api/latest/internal/flush-background-tasks/route.tsx b/apps/backend/src/app/api/latest/internal/flush-background-tasks/route.tsx new file mode 100644 index 000000000..6b9f83d48 --- /dev/null +++ b/apps/backend/src/app/api/latest/internal/flush-background-tasks/route.tsx @@ -0,0 +1,40 @@ +import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler"; +import { drainInFlightPromises } from "@/utils/background-tasks"; +import { adaptSchema, adminAuthTypeSchema, yupBoolean, yupNumber, yupObject, yupString } from "@hexclave/shared/dist/schema-fields"; + +// Test/dev-only hook that awaits any in-flight background tasks spawned via +// `runAsynchronouslyAndWaitUntil` (e.g. Stripe webhook processing, which is +// intentionally fire-and-forget in production for fast acks — see the stripe +// webhooks route). E2E tests hit the backend over HTTP and cannot await the +// in-process promise directly, so without this they race the background work +// and read side effects before they are written. On Vercel this is a no-op +// (the platform's native `waitUntil` is used instead of the tracked set), so +// this endpoint only has an effect on the non-serverless runtimes used in dev +// and CI. +export const POST = createSmartRouteHandler({ + metadata: { + hidden: true, + }, + request: yupObject({ + auth: yupObject({ + type: adminAuthTypeSchema, + tenancy: adaptSchema.defined(), + }).defined(), + method: yupString().oneOf(["POST"]).defined(), + }), + response: yupObject({ + statusCode: yupNumber().oneOf([200]).defined(), + bodyType: yupString().oneOf(["json"]).defined(), + body: yupObject({ + success: yupBoolean().defined(), + }).defined(), + }), + handler: async () => { + await drainInFlightPromises(); + return { + statusCode: 200, + bodyType: "json", + body: { success: true }, + }; + }, +}); diff --git a/apps/backend/src/app/api/latest/oauth-providers/crud.tsx b/apps/backend/src/app/api/latest/oauth-providers/crud.tsx index bab67e7b6..3c6f6c5b2 100644 --- a/apps/backend/src/app/api/latest/oauth-providers/crud.tsx +++ b/apps/backend/src/app/api/latest/oauth-providers/crud.tsx @@ -378,6 +378,13 @@ export const oauthProviderCrudHandlers = createLazyProxy(() => createCrudHandler }, async onCreate({ auth, data }) { const prismaClient = await getPrismaClientForTenancy(auth.tenancy); + // Creating a connected account requires the provider to actually be configured on this tenancy. + // resolveProviderType alone would accept any globally-known provider type (via its allProviders + // fallback, which exists so onRead/onList can still surface the type of historical accounts whose + // provider was later removed), so we gate creation on the provider being present in the config. + if (findProviderConfig(auth.tenancy, data.provider_config_id) == null) { + throw new StatusError(StatusError.NotFound, `OAuth provider ${data.provider_config_id} not found or not configured`); + } const providerType = resolveProviderType(auth.tenancy, data.provider_config_id) ?? throwErr(new StatusError(StatusError.NotFound, `OAuth provider ${data.provider_config_id} not found or not configured`)); diff --git a/apps/backend/src/lib/runtime/headers.tsx b/apps/backend/src/lib/runtime/headers.tsx index 967ce370b..193e1467c 100644 --- a/apps/backend/src/lib/runtime/headers.tsx +++ b/apps/backend/src/lib/runtime/headers.tsx @@ -45,12 +45,17 @@ export function serializeSetCookie(name: string, value: string, options: Respons if (options.maxAge != null) { parts.push(`Max-Age=${Math.trunc(options.maxAge)}`); } - if (options.httpOnly === true) { - parts.push("HttpOnly"); - } + // Emit Secure before HttpOnly to match the attribute order the pre-ElysiaJS + // (Next.js) backend produced. Cookie attribute order is semantically irrelevant + // per RFC 6265, but the e2e suite asserts the exact Set-Cookie string for the + // oauth-inner cookies (`...; Max-Age=N;( Secure;)? HttpOnly`), so keeping this + // order avoids a spurious regression there. if (options.secure === true) { parts.push("Secure"); } + if (options.httpOnly === true) { + parts.push("HttpOnly"); + } const sameSite = normalizeSameSite(options.sameSite); if (sameSite != null) { diff --git a/apps/backend/tsdown.config.ts b/apps/backend/tsdown.config.ts index 582fac168..07844d33a 100644 --- a/apps/backend/tsdown.config.ts +++ b/apps/backend/tsdown.config.ts @@ -26,6 +26,15 @@ const externalPackages = [ "oidc-provider", "pg", "sharp", + // @aws-sdk and @smithy use complex class hierarchies that rolldown mis-scopes + // when bundled, emitting references to hoisted classes before they're defined + // (e.g. "ReferenceError: StructureSchema$1 is not defined" when the server + // bundle boots via `node dist/server.mjs`). Keep them external so Node resolves + // them from node_modules at runtime, which is always present alongside the + // bundle (Docker image, CI runner, Vercel NFT trace). Mirrors the same fix in + // scripts/db-migrations.tsdown.config.ts. + "@aws-sdk", + "@smithy", ]; const nodeBuiltins = builtinModules.flatMap((moduleName) => [moduleName, `node:${moduleName}`]); diff --git a/apps/dashboard/src/components/hosted-auth-preview.test.tsx b/apps/dashboard/src/components/hosted-auth-preview.test.tsx index a66905248..e00681ebd 100644 --- a/apps/dashboard/src/components/hosted-auth-preview.test.tsx +++ b/apps/dashboard/src/components/hosted-auth-preview.test.tsx @@ -31,7 +31,9 @@ describe("HostedAuthMethodPreview", () => { expect(tabsList.getAttribute("class")).toContain("dark:bg-zinc-900/45"); expect(tabsList.querySelector("[aria-hidden]")?.getAttribute("class")).toContain("dark:bg-zinc-800/80"); - const emailInput = screen.getByLabelText("Email"); + // Both the magic-link and email+password tab panels render an "Email" field, so scope to the + // magic-link input (the default active tab) to disambiguate. + const emailInput = screen.getByLabelText("Email", { selector: "#hosted-preview-email" }); fireEvent.change(emailInput, { target: { value: "hello@example.com" } }); expect(emailInput).toHaveProperty("value", "hello@example.com"); expect(emailInput.getAttribute("class")).toContain("bg-white/45"); diff --git a/apps/e2e/tests/backend/backend-helpers.ts b/apps/e2e/tests/backend/backend-helpers.ts index 3275b4f69..67f0ef4ac 100644 --- a/apps/e2e/tests/backend/backend-helpers.ts +++ b/apps/e2e/tests/backend/backend-helpers.ts @@ -1772,11 +1772,32 @@ export namespace Payments { } headers["stripe-signature"] = header; } - return await niceBackendFetch("/api/latest/integrations/stripe/webhooks", { + const res = await niceBackendFetch("/api/latest/integrations/stripe/webhooks", { method: "POST", headers, body: payload, }); + // The webhook route acks Stripe immediately and processes the event in a + // fire-and-forget background task. E2E tests read side effects (transactions, + // subscriptions, ...) right after, so we deterministically wait for that + // background work to finish before returning. Only do this when the event was + // actually accepted for processing (a successful, non-deduplicated ack); + // signature-rejection tests never spawn background work. + if (res.status === 200 && res.body?.received === true && res.body?.deduplicated !== true) { + await flushBackgroundTasks(); + } + return res; } } + +// Waits for any in-flight background tasks (e.g. async Stripe webhook processing) +// to finish. Backed by the internal flush-background-tasks endpoint. +export async function flushBackgroundTasks() { + const res = await niceBackendFetch("/api/latest/internal/flush-background-tasks", { + method: "POST", + accessType: "admin", + body: {}, + }); + expect(res.status).toBe(200); +} diff --git a/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions-refund.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions-refund.test.ts index 7618608d2..1f4371d12 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions-refund.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions-refund.test.ts @@ -821,9 +821,10 @@ async function createLiveModeSubscriptionWithRenewal(): Promise<{ const code = await createPurchaseCode({ userId, productId: "sub-product" }); const tenancyId = code.split("_")[0]; + const idSuffix = randomUUID().replace(/-/g, ""); const nowSec = Math.floor(Date.now() / 1000); const stripeSubscription = { - id: "sub_renewal_refund_1", + id: `sub_renewal_refund_${idSuffix}`, status: "active", items: { data: [ @@ -849,7 +850,7 @@ async function createLiveModeSubscriptionWithRenewal(): Promise<{ }; const baseInvoiceObject = { - customer: "cus_renewal_refund_1", + customer: `cus_renewal_refund_${idSuffix}`, stack_stripe_mock_data: stackStripeMockData, lines: { data: [ @@ -863,13 +864,13 @@ async function createLiveModeSubscriptionWithRenewal(): Promise<{ }; const startWebhook = await Payments.sendStripeWebhook({ - id: "evt_renewal_refund_start", + id: `evt_renewal_refund_start_${idSuffix}`, type: "invoice.payment_succeeded", account: accountId, data: { object: { ...baseInvoiceObject, - id: "in_renewal_refund_start", + id: `in_renewal_refund_start_${idSuffix}`, billing_reason: "subscription_create", }, }, @@ -877,13 +878,13 @@ async function createLiveModeSubscriptionWithRenewal(): Promise<{ expect(startWebhook.status).toBe(200); const renewalWebhook = await Payments.sendStripeWebhook({ - id: "evt_renewal_refund_cycle", + id: `evt_renewal_refund_cycle_${idSuffix}`, type: "invoice.payment_succeeded", account: accountId, data: { object: { ...baseInvoiceObject, - id: "in_renewal_refund_cycle", + id: `in_renewal_refund_cycle_${idSuffix}`, billing_reason: "subscription_cycle", }, }, diff --git a/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions.test.ts index ba4b93d57..e5049ff39 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/internal/transactions.test.ts @@ -1,4 +1,4 @@ -import { createHmac } from "node:crypto"; +import { randomUUID } from "node:crypto"; import { getEnvVariable } from "@hexclave/shared/dist/utils/env"; import { expect } from "vitest"; import { it } from "../../../../../helpers"; @@ -76,18 +76,7 @@ async function createPurchaseCodeForCustomer(options: { customerType: "user" | " const stripeWebhookSecret = getEnvVariable("STACK_STRIPE_WEBHOOK_SECRET", "mock_stripe_webhook_secret"); async function sendStripeWebhook(payload: unknown) { - const timestamp = Math.floor(Date.now() / 1000); - const hmac = createHmac("sha256", stripeWebhookSecret); - hmac.update(`${timestamp}.${JSON.stringify(payload)}`); - const signature = hmac.digest("hex"); - return await niceBackendFetch("/api/latest/integrations/stripe/webhooks", { - method: "POST", - headers: { - "content-type": "application/json", - "stripe-signature": `t=${timestamp},v1=${signature}`, - }, - body: payload, - }); + return await PaymentsHelper.sendStripeWebhook(payload, { secret: stripeWebhookSecret }); } async function createPurchaseCode(options: { userId: string, productId: string }) { return await createPurchaseCodeForCustomer({ @@ -336,9 +325,10 @@ it("omits subscription-renewal entries for subscription creation invoices", asyn const code = await createPurchaseCode({ userId, productId: "sub-product" }); const tenancyId = code.split("_")[0]; + const idSuffix = randomUUID().replace(/-/g, ""); const nowSec = Math.floor(Date.now() / 1000); const stripeSubscription = { - id: "sub_tx_filter", + id: `sub_tx_filter_${idSuffix}`, status: "active", items: { data: [ @@ -364,7 +354,7 @@ it("omits subscription-renewal entries for subscription creation invoices", asyn }; const baseInvoiceObject = { - customer: "cus_tx_filter", + customer: `cus_tx_filter_${idSuffix}`, stack_stripe_mock_data: stackStripeMockData, lines: { data: [ @@ -380,26 +370,26 @@ it("omits subscription-renewal entries for subscription creation invoices", asyn }; const creationInvoiceEvent = { - id: "evt_sub_invoice_creation", + id: `evt_sub_invoice_creation_${idSuffix}`, type: "invoice.payment_succeeded", account: accountId, data: { object: { ...baseInvoiceObject, - id: "in_creation_tx", + id: `in_creation_tx_${idSuffix}`, billing_reason: "subscription_create", }, }, }; const renewalInvoiceEvent = { - id: "evt_sub_invoice_cycle", + id: `evt_sub_invoice_cycle_${idSuffix}`, type: "invoice.payment_succeeded", account: accountId, data: { object: { ...baseInvoiceObject, - id: "in_cycle_tx", + id: `in_cycle_tx_${idSuffix}`, billing_reason: "subscription_cycle", }, }, diff --git a/apps/e2e/tests/backend/endpoints/api/v1/stripe-webhooks.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/stripe-webhooks.test.ts index aa29a8f19..4b39346f1 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/stripe-webhooks.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/stripe-webhooks.test.ts @@ -805,9 +805,11 @@ it("updates a user's subscriptions via webhook (add then remove)", async ({ expe const fullCode = purchaseUrl.split("/purchase/")[1]; const stackTestTenancyId = fullCode.split("_")[0]; + const idSuffix = randomUUID().replace(/-/g, ""); + const stripeCustomerId = `cus_update_${idSuffix}`; const nowSec = Math.floor(Date.now() / 1000); const activeSubscription = { - id: "sub_update_1", + id: `sub_update_${idSuffix}`, status: "active", items: { data: [ @@ -832,7 +834,7 @@ it("updates a user's subscriptions via webhook (add then remove)", async ({ expe account: accountId, data: { object: { - customer: "cus_update_1", + customer: stripeCustomerId, stack_stripe_mock_data: { "accounts.retrieve": { metadata: { tenancyId: stackTestTenancyId } }, "customers.retrieve": { metadata: { customerId: userId, customerType: "USER" } }, @@ -848,15 +850,28 @@ it("updates a user's subscriptions via webhook (add then remove)", async ({ expe await waitForItemQuantity({ customerType: "user", customerId: userId, itemId, expected: 1 }); + // Cancel the subscription "now". The included-item grant is emitted effective + // at the subscription row's DB creation time (i.e. when the add webhook above + // was processed), so the cancellation must end the subscription strictly after + // that — otherwise the grant's expiry would be ordered *before* the grant + // itself and the seat would never drop. We capture the time after the add has + // been confirmed and add one second: `floor(now)+1 > now > createdAt`, so the + // ended_at (in whole Stripe seconds) is guaranteed to land after the row's + // createdAt while still being at most ~1s ahead of the subsequent poll, which + // the poll loop tolerates. Real Stripe likewise always sets canceled_at/ + // ended_at on a terminal-status subscription. + const cancelSec = Math.floor(Date.now() / 1000) + 1; const canceledSubscription = { ...activeSubscription, status: "canceled", + canceled_at: cancelSec, + ended_at: cancelSec, items: { data: [ { quantity: 1, - current_period_start: nowSec - 2 * 60, - current_period_end: nowSec - 60, + current_period_start: nowSec - 60, + current_period_end: cancelSec, }, ], }, @@ -868,7 +883,7 @@ it("updates a user's subscriptions via webhook (add then remove)", async ({ expe account: accountId, data: { object: { - customer: "cus_update_1", + customer: stripeCustomerId, stack_stripe_mock_data: { "accounts.retrieve": { metadata: { tenancyId: stackTestTenancyId } }, "customers.retrieve": { metadata: { customerId: userId, customerType: "USER" } }, diff --git a/apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts index fb7f97d5d..3bd63d3f5 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/team-invitations.test.ts @@ -433,7 +433,8 @@ it("cannot revoke another team's invitation by passing a team_id the caller cont await Project.createAndSwitch(); const { userId: attackerId } = await Auth.fastSignUp(); - // Team A: attacker holds $remove_members here. + // Team A: attacker holds $remove_members here. Team permissions are scoped to membership, + // so the attacker must be a member before the grant (and later revoke) is meaningful. const { teamId: teamA } = await Team.create(); await Team.addMember(teamA, attackerId); await niceBackendFetch(`/api/v1/team-permissions/${teamA}/${attackerId}/$remove_members`, { diff --git a/apps/e2e/tests/general/cli.test.ts b/apps/e2e/tests/general/cli.test.ts index 173b23b60..5765ed4ab 100644 --- a/apps/e2e/tests/general/cli.test.ts +++ b/apps/e2e/tests/general/cli.test.ts @@ -431,14 +431,29 @@ describe("Stack CLI", () => { expect(stderr).toContain("plain `config` object"); }); - it("config pull overwrites an existing file by default", async ({ expect }) => { + it("config pull refuses to clobber an existing file without --overwrite", async ({ expect }) => { const existingConfigPath = path.join(tmpDir, "existing-config.ts"); fs.writeFileSync(existingConfigPath, "existing\n"); - const { stdout, exitCode } = await runCli( + const { stderr, exitCode } = await runCli( ["config", "pull", "--cloud-project-id", createdProjectId, "--config-file", existingConfigPath], ); + expect(exitCode).toBe(1); + expect(stderr).toContain("already exists"); + expect(stderr).toContain("--overwrite"); + // The existing file must be left untouched. + expect(fs.readFileSync(existingConfigPath, "utf-8")).toBe("existing\n"); + }); + + it("config pull replaces an existing file with --overwrite", async ({ expect }) => { + const existingConfigPath = path.join(tmpDir, "existing-config-overwrite.ts"); + fs.writeFileSync(existingConfigPath, "existing\n"); + + const { stdout, exitCode } = await runCli( + ["config", "pull", "--cloud-project-id", createdProjectId, "--config-file", existingConfigPath, "--overwrite"], + ); + expect(exitCode).toBe(0); expect(stdout).toContain("Config written to"); const content = fs.readFileSync(existingConfigPath, "utf-8"); @@ -465,13 +480,13 @@ describe("Stack CLI", () => { } }); - it("config pull still prefers an existing ./stack.config.ts when --config-file is omitted", async ({ expect }) => { + it("config pull still prefers an existing ./stack.config.ts when --config-file is omitted (with --overwrite)", async ({ expect }) => { const cwdDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "stack-cli-config-pull-empty-"))); const expected = path.join(cwdDir, "stack.config.ts"); fs.writeFileSync(expected, "// placeholder so the file exists\n"); try { const { stdout, exitCode } = await runCli( - ["config", "pull", "--cloud-project-id", createdProjectId], + ["config", "pull", "--cloud-project-id", createdProjectId, "--overwrite"], undefined, cwdDir, ); diff --git a/packages/cli/src/commands/config-file.test.ts b/packages/cli/src/commands/config-file.test.ts index 7ca9e81ee..69c2673b2 100644 --- a/packages/cli/src/commands/config-file.test.ts +++ b/packages/cli/src/commands/config-file.test.ts @@ -2,7 +2,7 @@ import * as fs from "fs"; import * as os from "os"; import * as path from "path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { buildConfigPushSource, resolveConfigFilePathForPull, shouldReplaceConfigFileForPull } from "./config-file.js"; +import { assertConfigPullTarget, buildConfigPushSource, resolveConfigFilePathForPull } from "./config-file.js"; describe("resolveConfigFilePathForPull", () => { let tmpDir: string; @@ -46,7 +46,7 @@ describe("resolveConfigFilePathForPull", () => { }); }); -describe("shouldReplaceConfigFileForPull", () => { +describe("assertConfigPullTarget", () => { let tmpDir: string; beforeEach(() => { @@ -57,16 +57,26 @@ describe("shouldReplaceConfigFileForPull", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it("updates an existing config in place unless overwrite is explicit", () => { - const configPath = path.join(tmpDir, "hexclave.config.ts"); - fs.writeFileSync(configPath, "export const config = {};\n"); - - expect(shouldReplaceConfigFileForPull(configPath, {})).toBe(false); - expect(shouldReplaceConfigFileForPull(configPath, { overwrite: true })).toBe(true); + it("does not throw when the target file does not exist", () => { + expect(() => assertConfigPullTarget(path.join(tmpDir, "hexclave.config.ts"), {})).not.toThrow(); }); - it("replaces when pull needs to create a new config file", () => { - expect(shouldReplaceConfigFileForPull(path.join(tmpDir, "hexclave.config.ts"), {})).toBe(true); + it("throws when the target file already exists and --overwrite is not set", () => { + const configPath = path.join(tmpDir, "hexclave.config.ts"); + fs.writeFileSync(configPath, "export const config = {};\n"); + expect(() => assertConfigPullTarget(configPath, {})).toThrow(/already exists.*--overwrite/s); + }); + + it("throws for an existing placeholder/comment-only file without --overwrite", () => { + const configPath = path.join(tmpDir, "stack.config.ts"); + fs.writeFileSync(configPath, "// placeholder so the file exists\n"); + expect(() => assertConfigPullTarget(configPath, {})).toThrow(/already exists/); + }); + + it("does not throw when --overwrite is set, even if the file exists", () => { + const configPath = path.join(tmpDir, "hexclave.config.ts"); + fs.writeFileSync(configPath, "export const config = {};\n"); + expect(() => assertConfigPullTarget(configPath, { overwrite: true })).not.toThrow(); }); }); diff --git a/packages/cli/src/commands/config-file.ts b/packages/cli/src/commands/config-file.ts index d1bc21ddf..5a57a7c72 100644 --- a/packages/cli/src/commands/config-file.ts +++ b/packages/cli/src/commands/config-file.ts @@ -1,4 +1,4 @@ -import { replaceConfigObject, updateConfigObject } from "@hexclave/shared-backend"; +import { replaceConfigObject } from "@hexclave/shared-backend"; import { detectImportPackageFromDir } from "@hexclave/shared/dist/config-eval"; import { isValidConfig } from "@hexclave/shared/dist/config/format"; import type { EnvironmentConfigOverrideOverride } from "@hexclave/shared/dist/config/schema"; @@ -217,8 +217,20 @@ export function resolveConfigFilePathForPull(opts: { configFile?: string }, cwd: return candidate; } -export function shouldReplaceConfigFileForPull(filePath: string, opts: { overwrite?: boolean }): boolean { - return opts.overwrite === true || !fs.existsSync(filePath); +// `config pull` means "download the entire branch config into a fresh local file". It always writes +// the whole file (via replaceConfigObject) and never edits an existing file in place. In-place, +// hand-authored-preserving edits are the job of the config *update* flow (e.g. from the RDE), which +// routes through updateConfigObject's agent-assisted rewrite — that path is intentionally not +// reachable from `pull`. +// +// Because pull writes the whole file, it would clobber whatever is already at the target path. To +// avoid silently destroying a hand-authored config, we refuse to write over an existing file and +// require the user to opt in explicitly with --overwrite. +export function assertConfigPullTarget(filePath: string, opts: { overwrite?: boolean }): void { + if (opts.overwrite === true) return; + if (fs.existsSync(filePath)) { + throw new CliError(`A config file already exists at ${filePath}. Pass --overwrite to replace it with the pulled config, or remove the file first.`); + } } export function registerConfigCommand(program: Command) { @@ -231,30 +243,28 @@ export function registerConfigCommand(program: Command) { .description("Pull branch config to a local file") .option("--cloud-project-id ", "Cloud project ID to pull config from (defaults to the STACK_PROJECT_ID env var)") .option("--config-file ", "Path to write config file (.ts); defaults to ./hexclave.config.ts in the current directory") - .option("--overwrite", "Overwrite an existing config file instead of updating it in place") + .option("--overwrite", "Replace the config file if one already exists at the target path") .action(async (opts) => { const auth = resolveAuth(resolveProjectId(opts.cloudProjectId)); if (!isProjectAuthWithRefreshToken(auth)) { throw new CliError("`hexclave config pull` requires `hexclave login`. Remove STACK_SECRET_SERVER_KEY and try again."); } + // Resolve and validate the target file before any network work so we fail fast (e.g. when the + // target already exists without --overwrite) instead of paying for a wasted round-trip. + const filePath = resolveConfigFilePathForPull(opts, process.cwd()); + const ext = path.extname(filePath); + if (ext !== ".ts") { + throw new CliError("Config file must have a .ts extension. Typed config files require TypeScript."); + } + assertConfigPullTarget(filePath, opts); + const project = await getAdminProject(auth); const configOverride = await project.getConfigOverride("branch"); if (!isValidConfig(configOverride)) { throw new CliError("Pulled branch config is not a valid local config object."); } - const filePath = resolveConfigFilePathForPull(opts, process.cwd()); - const ext = path.extname(filePath); - - if (ext !== ".ts") { - throw new CliError("Config file must have a .ts extension. Typed config files require TypeScript."); - } - - if (shouldReplaceConfigFileForPull(filePath, opts)) { - await replaceConfigObject(filePath, configOverride); - } else { - await updateConfigObject(filePath, configOverride); - } + await replaceConfigObject(filePath, configOverride); console.log(`Config written to ${filePath}`); });