Skip to content

Commit 534fffa

Browse files
committed
fix bugs from coderabbit
1 parent 8feb4c8 commit 534fffa

7 files changed

Lines changed: 46 additions & 27 deletions

File tree

apps/webapp/app/components/navigation/EnvironmentSelector.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,12 @@ function Branches({
235235
? "no-active-branches"
236236
: "has-branches";
237237

238-
const currentBranchIsArchived = environment.archivedAt !== null;
238+
// Only surface the active environment's archived-branch item in the submenu it
239+
// actually belongs to. Both Development and Preview render this component, so
240+
// without the parent check an archived dev branch would leak into the Preview
241+
// submenu (and vice-versa).
242+
const currentBranchIsArchived =
243+
environment.archivedAt !== null && environment.parentEnvironmentId === parentEnvironment.id;
239244

240245
const envTextClassName = environmentTextClassName(parentEnvironment);
241246

apps/webapp/app/presenters/v3/BranchesPresenter.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export class BranchesPresenter {
138138
// The branchable parent is the root env (no parent). For dev that's
139139
// derivable; for preview we trust the isBranchableEnvironment column.
140140
...(envType === "DEVELOPMENT"
141-
? { parentEnvironmentId: null }
141+
? { parentEnvironmentId: null, orgMember: { userId } }
142142
: { isBranchableEnvironment: true }),
143143
},
144144
});

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dev-branches/route.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export default function Page() {
202202
className={cn("size-4", isSelected && "text-dev")}
203203
/>
204204
<CopyableText
205-
value={branch.branchName ?? ""}
205+
value={branch.branchName}
206206
className={cn(isSelected && "text-dev")}
207207
/>
208208
{isSelected && <Badge variant="extra-small">Current</Badge>}

apps/webapp/app/services/apiAuth.server.ts

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -318,26 +318,26 @@ function getApiKeyResult(apiKey: string): {
318318
const type = isPublicApiKey(apiKey)
319319
? "PUBLIC"
320320
: isSecretApiKey(apiKey)
321-
? "PRIVATE"
322-
: isPublicJWT(apiKey)
323-
? "PUBLIC_JWT"
324-
: "PRIVATE"; // Fallback to private key
321+
? "PRIVATE"
322+
: isPublicJWT(apiKey)
323+
? "PUBLIC_JWT"
324+
: "PRIVATE"; // Fallback to private key
325325
return { apiKey, type };
326326
}
327327

328328
export type AuthenticationResult =
329329
| {
330-
type: "personalAccessToken";
331-
result: PersonalAccessTokenAuthenticationResult;
332-
}
330+
type: "personalAccessToken";
331+
result: PersonalAccessTokenAuthenticationResult;
332+
}
333333
| {
334-
type: "organizationAccessToken";
335-
result: OrganizationAccessTokenAuthenticationResult;
336-
}
334+
type: "organizationAccessToken";
335+
result: OrganizationAccessTokenAuthenticationResult;
336+
}
337337
| {
338-
type: "apiKey";
339-
result: ApiAuthenticationResult;
340-
};
338+
type: "apiKey";
339+
result: ApiAuthenticationResult;
340+
};
341341

342342
type AuthenticationMethod = "personalAccessToken" | "organizationAccessToken" | "apiKey";
343343

@@ -354,11 +354,11 @@ type FilteredAuthenticationResult<
354354
T extends AllowedAuthenticationMethods = AllowedAuthenticationMethods
355355
> =
356356
| (T["personalAccessToken"] extends true
357-
? Extract<AuthenticationResult, { type: "personalAccessToken" }>
358-
: never)
357+
? Extract<AuthenticationResult, { type: "personalAccessToken" }>
358+
: never)
359359
| (T["organizationAccessToken"] extends true
360-
? Extract<AuthenticationResult, { type: "organizationAccessToken" }>
361-
: never)
360+
? Extract<AuthenticationResult, { type: "organizationAccessToken" }>
361+
: never)
362362
| (T["apiKey"] extends true ? Extract<AuthenticationResult, { type: "apiKey" }> : never);
363363

364364
/**
@@ -539,10 +539,18 @@ export async function authenticatedEnvironmentForAuthentication(
539539
const environment = await $replica.runtimeEnvironment.findFirst({
540540
where: {
541541
projectId: project.id,
542+
slug: slug,
542543
type: {
543544
in: ["PREVIEW", "DEVELOPMENT"],
544545
},
545546
branchName: resolvedBranch,
547+
...(slug === "dev"
548+
? {
549+
orgMember: {
550+
userId: user.id,
551+
},
552+
}
553+
: {}),
546554
archivedAt: null,
547555
},
548556
include: authIncludeWithParent,
@@ -605,9 +613,9 @@ export async function authenticatedEnvironmentForAuthentication(
605613
const environment = await $replica.runtimeEnvironment.findFirst({
606614
where: {
607615
projectId: project.id,
608-
type: {
609-
in: ["PREVIEW", "DEVELOPMENT"],
610-
},
616+
slug: slug,
617+
// No Development branches for OAT
618+
type: "PREVIEW",
611619
branchName: resolvedBranch,
612620
archivedAt: null,
613621
},

packages/cli-v3/src/commands/dev.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ async function startDev(options: StartDevOptions) {
241241
logger.debug("Starting dev CLI", { options });
242242

243243
let watcher: Awaited<ReturnType<typeof watchConfig>> | undefined;
244+
let removeLockFile: (() => void) | undefined;
244245

245246
try {
246247
if (options.logLevel) {
@@ -269,7 +270,7 @@ async function startDev(options: StartDevOptions) {
269270
const envVars = resolveLocalEnvVars(options.envFile);
270271
const branch = getDevBranch({ specified: options.branch ?? envVars.TRIGGER_DEV_BRANCH });
271272

272-
const removeLockFile = await createLockFile(options.cwd, branch);
273+
removeLockFile = await createLockFile(options.cwd, branch);
273274

274275
let devInstance: DevSessionInstance | undefined;
275276

@@ -348,11 +349,12 @@ async function startDev(options: StartDevOptions) {
348349
stop: async () => {
349350
devInstance?.stop();
350351
await watcher?.stop();
351-
removeLockFile();
352+
removeLockFile?.();
352353
},
353354
waitUntilExit,
354355
};
355356
} catch (error) {
357+
removeLockFile?.();
356358
await watcher?.stop();
357359
throw error;
358360
}

packages/cli-v3/src/dev/lock.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,21 @@ import { devBranchPathSegment } from "../utilities/devBranch.js";
55
import { logger } from "../utilities/logger.js";
66
import { mkdir, writeFile } from "node:fs/promises";
77
import { existsSync, unlinkSync } from "node:fs";
8+
import { createHash } from "node:crypto";
89
import { onExit } from "signal-exit";
910

1011
const LOCK_FILE_NAME = "dev.lock";
1112

1213
/**
1314
* Builds the lock file name for a given branch. The default branch keeps the
1415
* original `dev.lock` name (backwards compatible).
16+
* Throws in a SHA1 of the non-sanitized filename to prevent collision
1517
*/
1618
function lockFileName(branch?: string) {
1719
const safeBranch = devBranchPathSegment(branch);
18-
return safeBranch ? `dev.${safeBranch}.lock` : LOCK_FILE_NAME;
20+
if (!safeBranch || !branch) return LOCK_FILE_NAME;
21+
const branchHash = createHash("sha1").update(branch).digest("hex").slice(0, 8);
22+
return `dev.${safeBranch}.${branchHash}.lock`;
1923
}
2024

2125
export async function createLockFile(cwd: string, branch?: string) {

packages/cli-v3/src/utilities/analyze.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ function formatSize(bytes: number): string {
209209
function normalizePath(path: string): string {
210210
// Remove .trigger/tmp/build-<hash>/ prefix (tmp root may be branch-scoped,
211211
// e.g. .trigger/tmp-feature-foo/build-<hash>/)
212-
return path.replace(/(^|\/)\.trigger\/tmp(-[^/]+)?\/build-[^/]+\//, "");
212+
return path.replace(/(^|\/)\.trigger\/tmp(-[^/]+)?\/build-[^/]+\//, "$1");
213213
}
214214

215215
interface BundleTreeData {

0 commit comments

Comments
 (0)