Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 +166 to +167

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 { getProviderModelCapabilities, resolveSelectableProvider } from "../prov
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 @@ export default function ChatView(props: ChatViewProps) {
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 @@ export default function ChatView(props: ChatViewProps) {
}, [
activeProject,
allProjects,
projectGroupingSettings,
primaryEnvironmentId,
savedEnvironmentRegistry,
savedEnvironmentRuntimeById,
Expand Down Expand Up @@ -911,7 +918,10 @@ export default function ChatView(props: ChatViewProps) {
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 @@ export default function ChatView(props: ChatViewProps) {
getDraftSessionByLogicalProjectKey,
isServerThread,
navigate,
projectGroupingSettings,
routeKind,
setDraftThreadContext,
setLogicalProjectDraftThreadId,
Expand Down
Loading
Loading