Skip to content
Merged
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
12 changes: 10 additions & 2 deletions apps/desktop/src/lib/trpc/routers/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,18 @@ function hasConfiguredScripts(
(s): s is string => typeof s === "string" && s.trim().length > 0,
)
: [];
return setup.length > 0 || teardown.length > 0;
const run = Array.isArray(config?.run)
? config.run.filter(
(s): s is string => typeof s === "string" && s.trim().length > 0,
)
: [];
return setup.length > 0 || teardown.length > 0 || run.length > 0;
}

const CONFIG_TEMPLATE = `{
"setup": [],
"teardown": []
"teardown": [],
"run": []
}
`;

Expand Down Expand Up @@ -450,6 +456,7 @@ export const createConfigRouter = () => {
projectId: z.string(),
setup: z.array(z.string()),
teardown: z.array(z.string()),
run: z.array(z.string()).optional(),
}),
)
.mutation(({ input }) => {
Expand Down Expand Up @@ -482,6 +489,7 @@ export const createConfigRouter = () => {
...existingConfig,
setup: input.setup,
teardown: input.teardown,
...(input.run !== undefined && { run: input.run }),
};

try {
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const createTerminalRouter = () => {
cols: z.number().optional(),
rows: z.number().optional(),
cwd: z.string().optional(),
command: z.string().trim().min(1).optional(),
skipColdRestore: z.boolean().optional(),
allowKilled: z.boolean().optional(),
themeType: z.enum(["dark", "light"]).optional(),
Expand All @@ -80,6 +81,7 @@ export const createTerminalRouter = () => {
cols,
rows,
cwd: cwdOverride,
command,
skipColdRestore,
allowKilled,
themeType,
Expand Down Expand Up @@ -133,7 +135,8 @@ export const createTerminalRouter = () => {
cwd,
cols,
rows,
skipColdRestore,
command,
skipColdRestore: skipColdRestore || !!command,
allowKilled,
themeType: resolvedThemeType,
});
Expand Down
6 changes: 6 additions & 0 deletions apps/desktop/src/lib/trpc/routers/ui-state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ const paneSchema = z.object({
targetPaneId: z.string(),
})
.optional(),
workspaceRun: z
.object({
workspaceId: z.string(),
state: z.enum(["running", "stopped-by-user", "stopped-by-exit"]),
})
.optional(),
});

/**
Expand Down
51 changes: 51 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { z } from "zod";
import { publicProcedure, router } from "../../..";
import { getWorkspace } from "../utils/db-helpers";
import { getProjectChildItems } from "../utils/project-children-order";
import { loadSetupConfig } from "../utils/setup";
import { computeVisualOrder } from "../utils/visual-order";
import { getWorkspacePath } from "../utils/worktree";

Expand Down Expand Up @@ -287,5 +288,55 @@ export const createQueryProcedures = () => {
: currentIndex + 1;
return orderedWorkspaceIds[nextIndex];
}),

getResolvedRunCommands: publicProcedure
.input(z.object({ workspaceId: z.string() }))
.query(({ input }) => {
const workspace = localDb
.select()
.from(workspaces)
.where(eq(workspaces.id, input.workspaceId))
.get();
if (!workspace) {
throw new TRPCError({
code: "NOT_FOUND",
message: `Workspace ${input.workspaceId} not found`,
});
}
Comment on lines +295 to +305
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if getWorkspace helper filters by deletingAt

ast-grep --pattern $'function getWorkspace($_) {
  $$$
}'

rg -n -A 10 'export.*function getWorkspace|export const getWorkspace' --type ts

Repository: superset-sh/superset

Length of output: 17111


🏁 Script executed:

# Check the actual code at line 44 of query.ts and around the getResolvedRunCommands procedure
sed -n '40,50p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts

# Also check the getResolvedRunCommands procedure around lines 291-340
sed -n '291,340p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts

Repository: superset-sh/superset

Length of output: 1506


🏁 Script executed:

# Check the getAllGrouped procedure to see if it actually filters by deletingAt
sed -n '190,210p' apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts

Repository: superset-sh/superset

Length of output: 684


Both get and getResolvedRunCommands lack workspace deletion filtering.

The get procedure uses getWorkspace (line 44), which does not filter by deletingAt. Similarly, getResolvedRunCommands (line 295) queries directly without a deletion filter. However, getAllGrouped (line 198) explicitly filters isNull(workspaces.deletingAt). This inconsistency means both procedures could operate on workspaces pending deletion, unlike getAllGrouped.

Consider using getWorkspaceNotDeleting() helper (available in db-helpers.ts) or adding isNull(workspaces.deletingAt) to both queries for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts` around
lines 295 - 305, The get and getResolvedRunCommands procedures are querying
workspaces without excluding ones pending deletion; update both to filter out
deleted workspaces by using the existing helper getWorkspaceNotDeleting() from
db-helpers.ts (or add isNull(workspaces.deletingAt) to their queries) so they
behave consistently with getAllGrouped; replace calls to getWorkspace or direct
selects on workspaces in get and getResolvedRunCommands with calls to
getWorkspaceNotDeleting() (or add the isNull(workspaces.deletingAt) clause) and
ensure subsequent logic still uses the returned workspace object.


const project = localDb
.select()
.from(projects)
.where(eq(projects.id, workspace.projectId))
.get();
if (!project) {
return { commands: [] };
}

const worktree = workspace.worktreeId
? localDb
.select()
.from(worktrees)
.where(eq(worktrees.id, workspace.worktreeId))
.get()
: null;

const worktreePath =
workspace.type === "worktree" && worktree?.path
? worktree.path
: workspace.type === "branch"
? project.mainRepoPath
: undefined;

const config = loadSetupConfig({
mainRepoPath: project.mainRepoPath,
worktreePath,
projectId: project.id,
});

return {
commands: config?.run ?? [],
};
}),
});
};
153 changes: 153 additions & 0 deletions apps/desktop/src/lib/trpc/routers/workspaces/utils/setup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,33 @@ describe("loadSetupConfig", () => {
expect(config).toEqual(worktreeConfig);
});

test("worktree config inherits missing keys from main repo config", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({
setup: ["./.superset/setup.sh"],
run: ["bun dev"],
}),
);

mkdirSync(join(WORKTREE, ".superset"), { recursive: true });
writeFileSync(
join(WORKTREE, ".superset", "config.json"),
JSON.stringify({
setup: ["scripts/setup-worktree.sh"],
}),
);

const config = loadSetupConfig({
mainRepoPath: MAIN_REPO,
worktreePath: WORKTREE,
});
expect(config).toEqual({
setup: ["scripts/setup-worktree.sh"],
run: ["bun dev"],
});
});

test("falls back to main repo when worktree has no config", () => {
const mainConfig = { setup: ["npm install"] };

Expand Down Expand Up @@ -132,6 +159,33 @@ describe("loadSetupConfig", () => {
expect(config).toEqual(userConfig);
});

test("user override inherits missing keys from lower-priority config", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({
setup: ["npm install"],
run: ["bun dev"],
}),
);

mkdirSync(USER_CONFIG_DIR, { recursive: true });
writeFileSync(
join(USER_CONFIG_DIR, "config.json"),
JSON.stringify({
setup: ["custom-setup.sh"],
}),
);

const config = loadSetupConfig({
mainRepoPath: MAIN_REPO,
projectId: PROJECT_ID,
});
expect(config).toEqual({
setup: ["custom-setup.sh"],
run: ["bun dev"],
});
});

test("user override takes priority over worktree config", () => {
const worktreeConfig = { setup: ["worktree-setup.sh"] };
const userConfig = { setup: ["user-override-setup.sh"] };
Expand Down Expand Up @@ -529,4 +583,103 @@ describe("mergeConfigs", () => {
);
expect(result).toEqual({ setup: ["x"], teardown: ["y"] });
});

test("run key override with array", () => {
const result = mergeConfigs(
{ run: ["npm run dev"] },
{ run: ["bun run dev"] },
);
expect(result).toEqual({ run: ["bun run dev"] });
});

test("run key merge with before/after", () => {
const result = mergeConfigs(
{ run: ["npm run dev"] },
{ run: { before: ["echo starting"], after: ["echo done"] } },
);
expect(result).toEqual({
run: ["echo starting", "npm run dev", "echo done"],
});
});

test("run key passes through when not in local config", () => {
const result = mergeConfigs(
{ setup: ["install"], run: ["dev"] },
{ setup: ["custom-install"] },
);
expect(result).toEqual({ setup: ["custom-install"], run: ["dev"] });
});
});

describe("run config", () => {
beforeEach(() => {
mkdirSync(join(MAIN_REPO, ".superset"), { recursive: true });
});

afterEach(() => {
if (existsSync(TEST_DIR)) {
rmSync(TEST_DIR, { recursive: true, force: true });
}
});

test("loads run commands from config", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({
setup: ["bun install"],
run: ["bun run dev"],
}),
);

const config = loadSetupConfig({ mainRepoPath: MAIN_REPO });
expect(config?.run).toEqual(["bun run dev"]);
});

test("returns config without run when run is not set", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({ setup: ["bun install"] }),
);

const config = loadSetupConfig({ mainRepoPath: MAIN_REPO });
expect(config?.run).toBeUndefined();
});

test("validates run field must be an array", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({ run: "not-an-array" }),
);

const config = loadSetupConfig({ mainRepoPath: MAIN_REPO });
expect(config).toBeNull();
});

test("local config can override run commands", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({ run: ["npm run dev"] }),
);
writeFileSync(
join(MAIN_REPO, ".superset", "config.local.json"),
JSON.stringify({ run: ["bun run dev"] }),
);

const config = loadSetupConfig({ mainRepoPath: MAIN_REPO });
expect(config?.run).toEqual(["bun run dev"]);
});

test("local config can merge run commands with before/after", () => {
writeFileSync(
join(MAIN_REPO, ".superset", "config.json"),
JSON.stringify({ run: ["npm run dev"] }),
);
writeFileSync(
join(MAIN_REPO, ".superset", "config.local.json"),
JSON.stringify({ run: { before: ["export DEBUG=1"] } }),
);

const config = loadSetupConfig({ mainRepoPath: MAIN_REPO });
expect(config?.run).toEqual(["export DEBUG=1", "npm run dev"]);
});
});
Loading
Loading