authorgravatar for saphid@gmail.comAlex Southwell <saphid@gmail.com> 2026-09-07 17:16:53+10:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-07 00:16:53-07:00
logd8bc6831cd822b294d875364cf3aa0a17c999258
treee728622000c2f96458fc907259acdaf82ec24d38
parent7376536b23e8db0d1b09054bbc0b919cf0c85405
signature Signed by PGP key B5690EEEBB952194

fix(projects): prevent invalid script IDs from crashing threads (#10019)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

8 files changed, 174 insertions(+), 9 deletions(-)

apps/server/src/orchestration/decider.projectScripts.test.ts+110
...@@ -6,6 +6,7 @@ import {...@@ -6,6 +6,7 @@ import {
6 ProjectId,6 ProjectId,
7 ThreadId,7 ThreadId,
8 ProviderInstanceId,8 ProviderInstanceId,
9 type ProjectScript,
9} from "@t3tools/contracts";10} from "@t3tools/contracts";
10import { createModelSelection } from "@t3tools/shared/model";11import { createModelSelection } from "@t3tools/shared/model";
11import { expect, it } from "@effect/vitest";12import { expect, it } from "@effect/vitest";
...@@ -94,6 +95,115 @@ it.layer(NodeServices.layer)("decider project scripts", (it) => {...@@ -94,6 +95,115 @@ it.layer(NodeServices.layer)("decider project scripts", (it) => {
94 }),95 }),
95 );96 );
9697
98 const script = (id: string): ProjectScript => ({
99 id,
100 name: "Install dependencies",
101 command: "vp i",
102 icon: "configure",
103 runOnWorktreeCreate: false,
104 });
105
106 const projectWithScripts = (scripts: ReadonlyArray<ProjectScript>) => {
107 const now = "2026-01-01T00:00:00.000Z";
108 return projectEvent(createEmptyReadModel(now), {
109 sequence: 1,
110 eventId: asEventId("evt-legacy-scripts"),
111 aggregateKind: "project",
112 aggregateId: asProjectId("project-scripts"),
113 type: "project.created",
114 occurredAt: now,
115 commandId: CommandId.make("cmd-legacy-scripts"),
116 causationEventId: null,
117 correlationId: CommandId.make("cmd-legacy-scripts"),
118 metadata: {},
119 payload: {
120 projectId: asProjectId("project-scripts"),
121 title: "Scripts",
122 workspaceRoot: "/tmp/scripts",
123 defaultModelSelection: null,
124 scripts,
125 createdAt: now,
126 updatedAt: now,
127 },
128 });
129 };
130
131 for (const id of ["install-javascript-dependencies", "A", "a.b", "a b", "-a", "a".repeat(25)]) {
132 it.effect(`rejects a new script ID that cannot have a shortcut: ${id}`, () =>
133 Effect.gen(function* () {
134 const readModel = yield* projectWithScripts([]);
135 const failure = yield* Effect.flip(
136 decideOrchestrationCommand({
137 readModel,
138 command: {
139 type: "project.meta.update",
140 commandId: CommandId.make("cmd-invalid-script"),
141 projectId: asProjectId("project-scripts"),
142 scripts: [script("lint"), script(id)],
143 },
144 }),
145 );
146 expect(failure).toMatchObject({ _tag: "OrchestrationCommandInvariantError" });
147 expect(failure.message).toContain("Script ID");
148 expect(failure.message).toContain("24");
149 expect(readModel.projects[0]?.scripts).toEqual([]);
150 }),
151 );
152 }
153
154 it.effect("accepts a script ID at the shortcut length limit", () =>
155 Effect.gen(function* () {
156 const readModel = yield* projectWithScripts([]);
157 const scripts = [script("a".repeat(24))];
158 const result = yield* decideOrchestrationCommand({
159 readModel,
160 command: {
161 type: "project.meta.update",
162 commandId: CommandId.make("cmd-valid-script"),
163 projectId: asProjectId("project-scripts"),
164 scripts,
165 },
166 });
167 const event = Array.isArray(result) ? result[0] : result;
168 expect(event.payload).toMatchObject({ scripts });
169 }),
170 );
171
172 it.effect(
173 "keeps legacy scripts readable, editable and removable while allowing valid additions",
174 () =>
175 Effect.gen(function* () {
176 const legacy = script("install-javascript-dependencies");
177 const readModel = yield* projectWithScripts([legacy]);
178 expect(readModel.projects[0]?.scripts).toEqual([legacy]);
179 for (const scripts of [[{ ...legacy, command: "vp install" }, script("lint")], []]) {
180 const result = yield* decideOrchestrationCommand({
181 readModel,
182 command: {
183 type: "project.meta.update",
184 commandId: CommandId.make("cmd-repair-script"),
185 projectId: asProjectId("project-scripts"),
186 scripts,
187 },
188 });
189 const event = Array.isArray(result) ? result[0] : result;
190 expect(event.payload).toMatchObject({ scripts });
191 }
192 const failure = yield* Effect.flip(
193 decideOrchestrationCommand({
194 readModel,
195 command: {
196 type: "project.meta.update",
197 commandId: CommandId.make("cmd-new-invalid-script"),
198 projectId: asProjectId("project-scripts"),
199 scripts: [legacy, script("another.invalid.id")],
200 },
201 }),
202 );
203 expect(failure).toMatchObject({ _tag: "OrchestrationCommandInvariantError" });
204 }),
205 );
206
97 it.effect("propagates project icon metadata in project.meta.update", () =>207 it.effect("propagates project icon metadata in project.meta.update", () =>
98 Effect.gen(function* () {208 Effect.gen(function* () {
99 const now = "2026-01-01T00:00:00.000Z";209 const now = "2026-01-01T00:00:00.000Z";
apps/server/src/orchestration/decider.ts+18-1
...@@ -1,5 +1,7 @@...@@ -1,5 +1,7 @@
1import {1import {
2 EventId,2 EventId,
3 MAX_SCRIPT_ID_LENGTH,
4 SCRIPT_RUN_COMMAND_PATTERN,
3 MessageId,5 MessageId,
4 ThreadLinkedPullRequest,6 ThreadLinkedPullRequest,
5 UserInputRequestedPayload,7 UserInputRequestedPayload,
...@@ -37,6 +39,8 @@ import {...@@ -37,6 +39,8 @@ import {
37import { projectEvent } from "./projector.ts";39import { projectEvent } from "./projector.ts";
38import { threadHasQueuedTurnStart } from "./ThreadSettlementPolicy.ts";40import { threadHasQueuedTurnStart } from "./ThreadSettlementPolicy.ts";
3941
42const isScriptRunCommand = Schema.is(SCRIPT_RUN_COMMAND_PATTERN);
43
40const nowIso = Effect.map(DateTime.now, DateTime.formatIso);44const nowIso = Effect.map(DateTime.now, DateTime.formatIso);
41const decodeUserInputRequestedPayload = Schema.decodeUnknownOption(UserInputRequestedPayload);45const decodeUserInputRequestedPayload = Schema.decodeUnknownOption(UserInputRequestedPayload);
42const threadPullRequestLinksEqual = Schema.toEquivalence(Schema.NullOr(ThreadLinkedPullRequest));46const threadPullRequestLinksEqual = Schema.toEquivalence(Schema.NullOr(ThreadLinkedPullRequest));
...@@ -240,11 +244,24 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand"...@@ -240,11 +244,24 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand"
240 }244 }
241245
242 case "project.meta.update": {246 case "project.meta.update": {
243 yield* requireProject({247 const project = yield* requireProject({
244 readModel,248 readModel,
245 command,249 command,
246 projectId: command.projectId,250 projectId: command.projectId,
247 });251 });
252 if (command.scripts !== undefined) {
253 // Persisted IDs predate shortcut validation. Let users edit or remove them
254 // without allowing another invalid ID to enter the project.
255 const existingIds = new Set(project.scripts.map((script) => script.id));
256 for (const script of command.scripts) {
257 if (!existingIds.has(script.id) && !isScriptRunCommand(`script.${script.id}.run`)) {
258 return yield* new OrchestrationCommandInvariantError({
259 commandType: command.type,
260 detail: `Script ID '${script.id}' must be 1-${MAX_SCRIPT_ID_LENGTH} lowercase letters, digits or hyphens, starting with a letter or digit.`,
261 });
262 }
263 }
264 }
248 if (command.workspaceRoot !== undefined) {265 if (command.workspaceRoot !== undefined) {
249 yield* requireActiveProjectWorkspaceRootAbsent({266 yield* requireActiveProjectWorkspaceRootAbsent({
250 readModel,267 readModel,
apps/web/src/components/ChatView.tsx+1-1
...@@ -3732,7 +3732,7 @@ export default function ChatView(props: ChatViewProps) {...@@ -3732,7 +3732,7 @@ export default function ChatView(props: ChatViewProps) {
3732 previousScripts: ReadonlyArray<ProjectScript>;3732 previousScripts: ReadonlyArray<ProjectScript>;
3733 nextScripts: ReadonlyArray<ProjectScript>;3733 nextScripts: ReadonlyArray<ProjectScript>;
3734 keybinding?: string | null;3734 keybinding?: string | null;
3735 keybindingCommand: KeybindingCommand;3735 keybindingCommand: KeybindingCommand | null;
3736 }): Promise<AtomCommandResult<void, unknown>> => {3736 }): Promise<AtomCommandResult<void, unknown>> => {
3737 const updateResult = mapAtomCommandResult(3737 const updateResult = mapAtomCommandResult(
3738 await updateProjectScriptSettings({3738 await updateProjectScriptSettings({
apps/web/src/keybindings.ts+2-1
...@@ -259,9 +259,10 @@ export function formatShortcutLabel(...@@ -259,9 +259,10 @@ export function formatShortcutLabel(
259259
260export function shortcutLabelForCommand(260export function shortcutLabelForCommand(
261 keybindings: ResolvedKeybindingsConfig,261 keybindings: ResolvedKeybindingsConfig,
262 command: KeybindingCommand,262 command: KeybindingCommand | null,
263 options?: string | ResolvedShortcutLabelOptions,263 options?: string | ResolvedShortcutLabelOptions,
264): string | null {264): string | null {
265 if (command === null) return null;
265 const resolvedOptions =266 const resolvedOptions =
266 typeof options === "string"267 typeof options === "string"
267 ? ({ platform: options } satisfies ResolvedShortcutLabelOptions)268 ? ({ platform: options } satisfies ResolvedShortcutLabelOptions)
apps/web/src/lib/projectScriptKeybindings.test.ts+10-1
...@@ -48,8 +48,17 @@ describe("projectScriptKeybindings", () => {...@@ -48,8 +48,17 @@ describe("projectScriptKeybindings", () => {
48 ).toThrowError(PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE);48 ).toThrowError(PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE);
49 });49 });
5050
51 it("can edit or delete a legacy script without a shortcut", () => {
52 const command = commandForProjectScript("install-javascript-dependencies");
53 expect(keybindingValueForCommand([], command)).toBeNull();
54 expect(decodeProjectScriptKeybindingRule({ keybinding: null, command })).toBeNull();
55 expect(() => decodeProjectScriptKeybindingRule({ keybinding: "mod+k", command })).toThrowError(
56 PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE,
57 );
58 });
59
51 it("reads latest matching keybinding value for a command", () => {60 it("reads latest matching keybinding value for a command", () => {
52 const command = commandForProjectScript("test");61 const command = "script.test.run" as const;
53 const value = keybindingValueForCommand(62 const value = keybindingValueForCommand(
54 [63 [
55 {64 {
apps/web/src/lib/projectScriptKeybindings.ts+7-2
...@@ -19,11 +19,15 @@ function normalizeProjectScriptKeybindingInput(...@@ -19,11 +19,15 @@ function normalizeProjectScriptKeybindingInput(
1919
20export function decodeProjectScriptKeybindingRule(input: {20export function decodeProjectScriptKeybindingRule(input: {
21 keybinding: string | null | undefined;21 keybinding: string | null | undefined;
22 command: KeybindingCommand;22 command: KeybindingCommand | null;
23}): KeybindingRule | null {23}): KeybindingRule | null {
24 const normalizedKey = normalizeProjectScriptKeybindingInput(input.keybinding);24 const normalizedKey = normalizeProjectScriptKeybindingInput(input.keybinding);
25 if (!normalizedKey) return null;25 if (!normalizedKey) return null;
2626
27 if (input.command === null) {
28 throw new Error(PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE);
29 }
30
27 const decoded = decodeKeybindingRule({31 const decoded = decodeKeybindingRule({
28 key: normalizedKey,32 key: normalizedKey,
29 command: input.command,33 command: input.command,
...@@ -36,8 +40,9 @@ export function decodeProjectScriptKeybindingRule(input: {...@@ -36,8 +40,9 @@ export function decodeProjectScriptKeybindingRule(input: {
3640
37export function keybindingValueForCommand(41export function keybindingValueForCommand(
38 keybindings: ResolvedKeybindingsConfig,42 keybindings: ResolvedKeybindingsConfig,
39 command: KeybindingCommand,43 command: KeybindingCommand | null,
40): string | null {44): string | null {
45 if (command === null) return null;
41 for (let index = keybindings.length - 1; index >= 0; index -= 1) {46 for (let index = keybindings.length - 1; index >= 0; index -= 1) {
42 const binding = keybindings[index];47 const binding = keybindings[index];
43 if (!binding || binding.command !== command) continue;48 if (!binding || binding.command !== command) continue;
apps/web/src/projectScripts.test.ts+21-1
...@@ -1,3 +1,5 @@...@@ -1,3 +1,5 @@
1import { MAX_SCRIPT_ID_LENGTH } from "@t3tools/contracts";
2import { shortcutLabelForCommand } from "./keybindings";
1import { describe, expect, it } from "vite-plus/test";3import { describe, expect, it } from "vite-plus/test";
2import {4import {
3 projectScriptCwd,5 projectScriptCwd,
...@@ -57,10 +59,28 @@ describe("projectScripts helpers", () => {...@@ -57,10 +59,28 @@ describe("projectScripts helpers", () => {
57 it("builds and parses script run commands", () => {59 it("builds and parses script run commands", () => {
58 const command = commandForProjectScript("lint");60 const command = commandForProjectScript("lint");
59 expect(command).toBe("script.lint.run");61 expect(command).toBe("script.lint.run");
60 expect(projectScriptIdFromCommand(command)).toBe("lint");62 expect(projectScriptIdFromCommand(command ?? "")).toBe("lint");
61 expect(projectScriptIdFromCommand("terminal.toggle")).toBeNull();63 expect(projectScriptIdFromCommand("terminal.toggle")).toBeNull();
62 });64 });
6365
66 it.each(["install-javascript-dependencies", "A", "a.b", "a b", "-a", "", "a".repeat(25)])(
67 "omits the shortcut for legacy script ID %j without crashing script menus",
68 (id) => {
69 const commands = ["lint", id, "test"].map(commandForProjectScript);
70 expect(commands).toEqual(["script.lint.run", null, "script.test.run"]);
71 expect(commands.map((command) => shortcutLabelForCommand([], command))).toEqual([
72 null,
73 null,
74 null,
75 ]);
76 },
77 );
78
79 it("preserves the exact ID at the shortcut length limit", () => {
80 const id = "a".repeat(MAX_SCRIPT_ID_LENGTH);
81 expect(projectScriptIdFromCommand(commandForProjectScript(id) ?? "")).toBe(id);
82 });
83
64 it("slugifies and dedupes project script ids", () => {84 it("slugifies and dedupes project script ids", () => {
65 expect(nextProjectScriptId("Run Tests", [])).toBe("run-tests");85 expect(nextProjectScriptId("Run Tests", [])).toBe("run-tests");
66 expect(nextProjectScriptId("Run Tests", ["run-tests"])).toBe("run-tests-2");86 expect(nextProjectScriptId("Run Tests", ["run-tests"])).toBe("run-tests-2");
apps/web/src/projectScripts.ts+5-2
...@@ -47,8 +47,11 @@ function normalizeScriptId(value: string): string {...@@ -47,8 +47,11 @@ function normalizeScriptId(value: string): string {
47 return cleaned.slice(0, MAX_SCRIPT_ID_LENGTH).replace(/-+$/g, "") || "script";47 return cleaned.slice(0, MAX_SCRIPT_ID_LENGTH).replace(/-+$/g, "") || "script";
48}48}
4949
50export const commandForProjectScript = (scriptId: string): KeybindingCommand =>50/** Legacy script IDs may not support shortcuts; keep those scripts usable without one. */
51 SCRIPT_RUN_COMMAND_PATTERN.make(`script.${scriptId}.run`);51export function commandForProjectScript(scriptId: string): KeybindingCommand | null {
52 const command = `script.${scriptId}.run`;
53 return isScriptRunCommand(command) ? command : null;
54}
5255
53export function projectScriptIdFromCommand(command: string): string | null {56export function projectScriptIdFromCommand(command: string): string | null {
54 const trimmed = command.trim();57 const trimmed = command.trim();