Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/desktop/src/clientPersistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ const clientSettings: ClientSettings = {
confirmThreadArchive: true,
confirmThreadDelete: false,
diffWordWrap: true,
sidebarProjectGroupingMode: "repository_path",
sidebarProjectGroupingOverrides: {
"environment-1:/tmp/project-a": "separate",
},
sidebarProjectSortOrder: "manual",
sidebarThreadSortOrder: "created_at",
timestampFormat: "24-hour",
Expand Down
85 changes: 58 additions & 27 deletions apps/desktop/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,35 @@ const TITLEBAR_COLOR = "#01000000"; // #00000000 does not work correctly on Linu
const TITLEBAR_LIGHT_SYMBOL_COLOR = "#1f2937";
const TITLEBAR_DARK_SYMBOL_COLOR = "#f8fafc";

function normalizeContextMenuItems(source: readonly ContextMenuItem[]): ContextMenuItem[] {
const normalizedItems: ContextMenuItem[] = [];

for (const sourceItem of source) {
if (typeof sourceItem.id !== "string" || typeof sourceItem.label !== "string") {
Comment on lines +165 to +166
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low src/main.ts:165

When source contains null or undefined elements (possible via untrusted IPC input), accessing sourceItem.id on line 166 throws a TypeError instead of being filtered out. Consider adding a guard to skip non-object elements before property access.

   for (const sourceItem of source) {
+    if (!sourceItem || typeof sourceItem !== 'object') {
+      continue;
+    }
     if (typeof sourceItem.id !== "string" || typeof sourceItem.label !== "string") {
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/desktop/src/main.ts around lines 165-166:

When `source` contains `null` or `undefined` elements (possible via untrusted IPC input), accessing `sourceItem.id` on line 166 throws a `TypeError` instead of being filtered out. Consider adding a guard to skip non-object elements before property access.

Evidence trail:
apps/desktop/src/main.ts lines 162-167 (normalizeContextMenuItems function with typeof sourceItem.id check), apps/desktop/src/main.ts lines 1734-1737 (ipcMain.handle calling normalizeContextMenuItems with untrusted IPC items)

continue;
}

const normalizedItem: ContextMenuItem = {
id: sourceItem.id,
label: sourceItem.label,
destructive: sourceItem.destructive === true,
disabled: sourceItem.disabled === true,
};

if (sourceItem.children) {
const normalizedChildren = normalizeContextMenuItems(sourceItem.children);
if (normalizedChildren.length === 0) {
continue;
}
normalizedItem.children = normalizedChildren;
}

normalizedItems.push(normalizedItem);
}

return normalizedItems;
}

type WindowTitleBarOptions = Pick<
BrowserWindowConstructorOptions,
"titleBarOverlay" | "titleBarStyle" | "trafficLightPosition"
Expand Down Expand Up @@ -1705,14 +1734,7 @@ function registerIpcHandlers(): void {
ipcMain.handle(
CONTEXT_MENU_CHANNEL,
async (_event, items: ContextMenuItem[], position?: { x: number; y: number }) => {
const normalizedItems = items
.filter((item) => typeof item.id === "string" && typeof item.label === "string")
.map((item) => ({
id: item.id,
label: item.label,
destructive: item.destructive === true,
disabled: item.disabled === true,
}));
const normalizedItems = normalizeContextMenuItems(items);
if (normalizedItems.length === 0) {
return null;
}
Expand All @@ -1733,28 +1755,37 @@ function registerIpcHandlers(): void {
if (!window) return null;

return new Promise<string | null>((resolve) => {
const template: MenuItemConstructorOptions[] = [];
let hasInsertedDestructiveSeparator = false;
for (const item of normalizedItems) {
if (item.destructive && !hasInsertedDestructiveSeparator && template.length > 0) {
template.push({ type: "separator" });
hasInsertedDestructiveSeparator = true;
}
const itemOption: MenuItemConstructorOptions = {
label: item.label,
enabled: !item.disabled,
click: () => resolve(item.id),
};
if (item.destructive) {
const destructiveIcon = getDestructiveMenuIcon();
if (destructiveIcon) {
itemOption.icon = destructiveIcon;
const buildTemplate = (
entries: readonly ContextMenuItem[],
): MenuItemConstructorOptions[] => {
const template: MenuItemConstructorOptions[] = [];
let hasInsertedDestructiveSeparator = false;
for (const item of entries) {
if (item.destructive && !hasInsertedDestructiveSeparator && template.length > 0) {
template.push({ type: "separator" });
hasInsertedDestructiveSeparator = true;
}
const itemOption: MenuItemConstructorOptions = {
label: item.label,
enabled: !item.disabled,
};
if (item.children && item.children.length > 0) {
itemOption.submenu = buildTemplate(item.children);
} else {
itemOption.click = () => resolve(item.id);
}
if (item.destructive && (!item.children || item.children.length === 0)) {
const destructiveIcon = getDestructiveMenuIcon();
if (destructiveIcon) {
itemOption.icon = destructiveIcon;
}
}
template.push(itemOption);
}
template.push(itemOption);
}
return template;
};

const menu = Menu.buildFromTemplate(template);
const menu = Menu.buildFromTemplate(buildTemplate(normalizedItems));
menu.popup({
window,
...popupPosition,
Expand Down
26 changes: 26 additions & 0 deletions apps/server/src/project/Layers/RepositoryIdentityResolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
RepositoryIdentityResolverLive,
} from "./RepositoryIdentityResolver.ts";

const normalizePathSeparators = (value: string) => value.replaceAll("\\", "/");

const git = (cwd: string, args: ReadonlyArray<string>) =>
Effect.promise(() => runProcess("git", ["-C", cwd, ...args]));

Expand Down Expand Up @@ -41,13 +43,37 @@ it.layer(NodeServices.layer)("RepositoryIdentityResolverLive", (it) => {

expect(identity).not.toBeNull();
expect(identity?.canonicalKey).toBe("github.com/t3tools/t3code");
expect(normalizePathSeparators(identity?.rootPath ?? "")).toBe(normalizePathSeparators(cwd));
expect(identity?.displayName).toBe("t3tools/t3code");
expect(identity?.provider).toBe("github");
expect(identity?.owner).toBe("t3tools");
expect(identity?.name).toBe("t3code");
}).pipe(Effect.provide(RepositoryIdentityResolverLive)),
);

it.effect("returns the git top-level root path when resolving from a nested workspace", () =>
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
const repoRoot = yield* fileSystem.makeTempDirectoryScoped({
prefix: "t3-repository-identity-nested-root-test-",
});
const nestedWorkspace = `${repoRoot}/packages/web`;

yield* fileSystem.makeDirectory(nestedWorkspace, { recursive: true });
yield* git(repoRoot, ["init"]);
yield* git(repoRoot, ["remote", "add", "origin", "git@github.com:T3Tools/t3code.git"]);

const resolver = yield* RepositoryIdentityResolver;
const identity = yield* resolver.resolve(nestedWorkspace);

expect(identity).not.toBeNull();
expect(identity?.canonicalKey).toBe("github.com/t3tools/t3code");
expect(normalizePathSeparators(identity?.rootPath ?? "")).toBe(
normalizePathSeparators(repoRoot),
);
}).pipe(Effect.provide(RepositoryIdentityResolverLive)),
);

it.effect("returns null for non-git folders and repos without remotes", () =>
Effect.gen(function* () {
const fileSystem = yield* FileSystem.FileSystem;
Expand Down
4 changes: 3 additions & 1 deletion apps/server/src/project/Layers/RepositoryIdentityResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function pickPrimaryRemote(
function buildRepositoryIdentity(input: {
readonly remoteName: string;
readonly remoteUrl: string;
readonly rootPath: string;
}): RepositoryIdentity {
const canonicalKey = normalizeGitRemoteUrl(input.remoteUrl);
const hostingProvider = detectGitHostingProviderFromRemoteUrl(input.remoteUrl);
Expand All @@ -57,6 +58,7 @@ function buildRepositoryIdentity(input: {
remoteName: input.remoteName,
remoteUrl: input.remoteUrl,
},
rootPath: input.rootPath,
...(repositoryPath ? { displayName: repositoryPath } : {}),
...(hostingProvider ? { provider: hostingProvider.kind } : {}),
...(owner ? { owner } : {}),
Expand Down Expand Up @@ -108,7 +110,7 @@ async function resolveRepositoryIdentityFromCacheKey(
}

const remote = pickPrimaryRemote(parseRemoteFetchUrls(remoteResult.stdout));
return remote ? buildRepositoryIdentity(remote) : null;
return remote ? buildRepositoryIdentity({ ...remote, rootPath: cacheKey }) : null;
} catch {
return null;
}
Expand Down
19 changes: 15 additions & 4 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
import { useSettings } from "../hooks/useSettings";
import { resolveAppModelSelection } from "../modelSelection";
import { isTerminalFocused } from "../lib/terminalFocus";
import { deriveLogicalProjectKey } from "../logicalProject";
import { deriveLogicalProjectKeyFromSettings } from "../logicalProject";
import {
useSavedEnvironmentRegistryStore,
useSavedEnvironmentRuntimeStore,
Expand Down Expand Up @@ -843,10 +843,16 @@
const primaryEnvironmentId = usePrimaryEnvironmentId();
const savedEnvironmentRegistry = useSavedEnvironmentRegistryStore((s) => s.byId);
const savedEnvironmentRuntimeById = useSavedEnvironmentRuntimeStore((s) => s.byId);
const projectGroupingSettings = useSettings((settings) => ({
sidebarProjectGroupingMode: settings.sidebarProjectGroupingMode,
sidebarProjectGroupingOverrides: settings.sidebarProjectGroupingOverrides,
}));
const logicalProjectEnvironments = useMemo(() => {
if (!activeProject) return [];
const logicalKey = deriveLogicalProjectKey(activeProject);
const memberProjects = allProjects.filter((p) => deriveLogicalProjectKey(p) === logicalKey);
const logicalKey = deriveLogicalProjectKeyFromSettings(activeProject, projectGroupingSettings);
const memberProjects = allProjects.filter(
(p) => deriveLogicalProjectKeyFromSettings(p, projectGroupingSettings) === logicalKey,
);
const seen = new Set<string>();
const envs: Array<{
environmentId: EnvironmentId;
Expand Down Expand Up @@ -882,6 +888,7 @@
}, [
activeProject,
allProjects,
projectGroupingSettings,
primaryEnvironmentId,
savedEnvironmentRegistry,
savedEnvironmentRuntimeById,
Expand Down Expand Up @@ -911,7 +918,10 @@
throw new Error("No active project is available for this pull request.");
}
const activeProjectRef = scopeProjectRef(activeProject.environmentId, activeProject.id);
const logicalProjectKey = deriveLogicalProjectKey(activeProject);
const logicalProjectKey = deriveLogicalProjectKeyFromSettings(
activeProject,
projectGroupingSettings,
);
const storedDraftSession = getDraftSessionByLogicalProjectKey(logicalProjectKey);
if (storedDraftSession) {
setDraftThreadContext(storedDraftSession.draftId, input);
Expand Down Expand Up @@ -972,6 +982,7 @@
getDraftSessionByLogicalProjectKey,
isServerThread,
navigate,
projectGroupingSettings,
routeKind,
setDraftThreadContext,
setLogicalProjectDraftThreadId,
Expand Down Expand Up @@ -1537,7 +1548,7 @@
);

const focusComposer = useCallback(() => {
composerRef.current?.focusAtEnd();

Check warning on line 1551 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
}, []);
const scheduleComposerFocus = useCallback(() => {
window.requestAnimationFrame(() => {
Expand All @@ -1545,7 +1556,7 @@
});
}, [focusComposer]);
const addTerminalContextToDraft = useCallback((selection: TerminalContextSelection) => {
composerRef.current?.addTerminalContext(selection);

Check warning on line 1559 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
}, []);
const setTerminalOpen = useCallback(
(open: boolean) => {
Expand Down Expand Up @@ -2737,7 +2748,7 @@
};
});
promptRef.current = "";
composerRef.current?.resetCursorState({ cursor: 0 });

Check warning on line 2751 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
},
[activePendingProgress?.activeQuestion, activePendingUserInput],
);
Expand All @@ -2764,7 +2775,7 @@
),
},
}));
const snapshot = composerRef.current?.readSnapshot();

Check warning on line 2778 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
if (
snapshot?.value !== value ||
snapshot.cursor !== nextCursor ||
Expand Down Expand Up @@ -2827,7 +2838,7 @@
return;
}

const sendCtx = composerRef.current?.getSendContext();

Check warning on line 2841 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
if (!sendCtx) {
return;
}
Expand Down Expand Up @@ -2962,7 +2973,7 @@
return;
}

const sendCtx = composerRef.current?.getSendContext();

Check warning on line 2976 in apps/web/src/components/ChatView.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

eslint-plugin-react-hooks(exhaustive-deps)

React Hook useCallback has a missing dependency: 'composerRef.current'
if (!sendCtx) {
return;
}
Expand Down
Loading
Loading