diff --git a/apps/server/src/orchestration/decider.projectScripts.test.ts b/apps/server/src/orchestration/decider.projectScripts.test.ts index 22ff884d5f47..732605d00217 100644 --- a/apps/server/src/orchestration/decider.projectScripts.test.ts +++ b/apps/server/src/orchestration/decider.projectScripts.test.ts @@ -6,6 +6,7 @@ import { ProjectId, ThreadId, ProviderInstanceId, + type ProjectScript, } from "@t3tools/contracts"; import { createModelSelection } from "@t3tools/shared/model"; import { expect, it } from "@effect/vitest"; @@ -94,6 +95,115 @@ it.layer(NodeServices.layer)("decider project scripts", (it) => { }), ); + const script = (id: string): ProjectScript => ({ + id, + name: "Install dependencies", + command: "vp i", + icon: "configure", + runOnWorktreeCreate: false, + }); + + const projectWithScripts = (scripts: ReadonlyArray) => { + const now = "2026-01-01T00:00:00.000Z"; + return projectEvent(createEmptyReadModel(now), { + sequence: 1, + eventId: asEventId("evt-legacy-scripts"), + aggregateKind: "project", + aggregateId: asProjectId("project-scripts"), + type: "project.created", + occurredAt: now, + commandId: CommandId.make("cmd-legacy-scripts"), + causationEventId: null, + correlationId: CommandId.make("cmd-legacy-scripts"), + metadata: {}, + payload: { + projectId: asProjectId("project-scripts"), + title: "Scripts", + workspaceRoot: "/tmp/scripts", + defaultModelSelection: null, + scripts, + createdAt: now, + updatedAt: now, + }, + }); + }; + + for (const id of ["install-javascript-dependencies", "A", "a.b", "a b", "-a", "a".repeat(25)]) { + it.effect(`rejects a new script ID that cannot have a shortcut: ${id}`, () => + Effect.gen(function* () { + const readModel = yield* projectWithScripts([]); + const failure = yield* Effect.flip( + decideOrchestrationCommand({ + readModel, + command: { + type: "project.meta.update", + commandId: CommandId.make("cmd-invalid-script"), + projectId: asProjectId("project-scripts"), + scripts: [script("lint"), script(id)], + }, + }), + ); + expect(failure).toMatchObject({ _tag: "OrchestrationCommandInvariantError" }); + expect(failure.message).toContain("Script ID"); + expect(failure.message).toContain("24"); + expect(readModel.projects[0]?.scripts).toEqual([]); + }), + ); + } + + it.effect("accepts a script ID at the shortcut length limit", () => + Effect.gen(function* () { + const readModel = yield* projectWithScripts([]); + const scripts = [script("a".repeat(24))]; + const result = yield* decideOrchestrationCommand({ + readModel, + command: { + type: "project.meta.update", + commandId: CommandId.make("cmd-valid-script"), + projectId: asProjectId("project-scripts"), + scripts, + }, + }); + const event = Array.isArray(result) ? result[0] : result; + expect(event.payload).toMatchObject({ scripts }); + }), + ); + + it.effect( + "keeps legacy scripts readable, editable and removable while allowing valid additions", + () => + Effect.gen(function* () { + const legacy = script("install-javascript-dependencies"); + const readModel = yield* projectWithScripts([legacy]); + expect(readModel.projects[0]?.scripts).toEqual([legacy]); + for (const scripts of [[{ ...legacy, command: "vp install" }, script("lint")], []]) { + const result = yield* decideOrchestrationCommand({ + readModel, + command: { + type: "project.meta.update", + commandId: CommandId.make("cmd-repair-script"), + projectId: asProjectId("project-scripts"), + scripts, + }, + }); + const event = Array.isArray(result) ? result[0] : result; + expect(event.payload).toMatchObject({ scripts }); + } + const failure = yield* Effect.flip( + decideOrchestrationCommand({ + readModel, + command: { + type: "project.meta.update", + commandId: CommandId.make("cmd-new-invalid-script"), + projectId: asProjectId("project-scripts"), + scripts: [legacy, script("another.invalid.id")], + }, + }), + ); + expect(failure).toMatchObject({ _tag: "OrchestrationCommandInvariantError" }); + }), + ); + it.effect("propagates project icon metadata in project.meta.update", () => Effect.gen(function* () { const now = "2026-01-01T00:00:00.000Z"; diff --git a/apps/server/src/orchestration/decider.ts b/apps/server/src/orchestration/decider.ts index 37ab4730fb79..b93add311fbc 100644 --- a/apps/server/src/orchestration/decider.ts +++ b/apps/server/src/orchestration/decider.ts @@ -1,5 +1,7 @@ import { EventId, + MAX_SCRIPT_ID_LENGTH, + SCRIPT_RUN_COMMAND_PATTERN, MessageId, ThreadLinkedPullRequest, UserInputRequestedPayload, @@ -37,6 +39,8 @@ import { import { projectEvent } from "./projector.ts"; import { threadHasQueuedTurnStart } from "./ThreadSettlementPolicy.ts"; +const isScriptRunCommand = Schema.is(SCRIPT_RUN_COMMAND_PATTERN); + const nowIso = Effect.map(DateTime.now, DateTime.formatIso); const decodeUserInputRequestedPayload = Schema.decodeUnknownOption(UserInputRequestedPayload); const threadPullRequestLinksEqual = Schema.toEquivalence(Schema.NullOr(ThreadLinkedPullRequest)); @@ -240,11 +244,24 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" } case "project.meta.update": { - yield* requireProject({ + const project = yield* requireProject({ readModel, command, projectId: command.projectId, }); + if (command.scripts !== undefined) { + // Persisted IDs predate shortcut validation. Let users edit or remove them + // without allowing another invalid ID to enter the project. + const existingIds = new Set(project.scripts.map((script) => script.id)); + for (const script of command.scripts) { + if (!existingIds.has(script.id) && !isScriptRunCommand(`script.${script.id}.run`)) { + return yield* new OrchestrationCommandInvariantError({ + commandType: command.type, + detail: `Script ID '${script.id}' must be 1-${MAX_SCRIPT_ID_LENGTH} lowercase letters, digits or hyphens, starting with a letter or digit.`, + }); + } + } + } if (command.workspaceRoot !== undefined) { yield* requireActiveProjectWorkspaceRootAbsent({ readModel, diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 1415fffab190..6d1906ff975c 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -3732,7 +3732,7 @@ export default function ChatView(props: ChatViewProps) { previousScripts: ReadonlyArray; nextScripts: ReadonlyArray; keybinding?: string | null; - keybindingCommand: KeybindingCommand; + keybindingCommand: KeybindingCommand | null; }): Promise> => { const updateResult = mapAtomCommandResult( await updateProjectScriptSettings({ diff --git a/apps/web/src/keybindings.ts b/apps/web/src/keybindings.ts index 844984d4a02e..be5090fe2f76 100644 --- a/apps/web/src/keybindings.ts +++ b/apps/web/src/keybindings.ts @@ -259,9 +259,10 @@ export function formatShortcutLabel( export function shortcutLabelForCommand( keybindings: ResolvedKeybindingsConfig, - command: KeybindingCommand, + command: KeybindingCommand | null, options?: string | ResolvedShortcutLabelOptions, ): string | null { + if (command === null) return null; const resolvedOptions = typeof options === "string" ? ({ platform: options } satisfies ResolvedShortcutLabelOptions) diff --git a/apps/web/src/lib/projectScriptKeybindings.test.ts b/apps/web/src/lib/projectScriptKeybindings.test.ts index 69e4d77afe08..f9ad636e56d2 100644 --- a/apps/web/src/lib/projectScriptKeybindings.test.ts +++ b/apps/web/src/lib/projectScriptKeybindings.test.ts @@ -48,8 +48,17 @@ describe("projectScriptKeybindings", () => { ).toThrowError(PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE); }); + it("can edit or delete a legacy script without a shortcut", () => { + const command = commandForProjectScript("install-javascript-dependencies"); + expect(keybindingValueForCommand([], command)).toBeNull(); + expect(decodeProjectScriptKeybindingRule({ keybinding: null, command })).toBeNull(); + expect(() => decodeProjectScriptKeybindingRule({ keybinding: "mod+k", command })).toThrowError( + PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE, + ); + }); + it("reads latest matching keybinding value for a command", () => { - const command = commandForProjectScript("test"); + const command = "script.test.run" as const; const value = keybindingValueForCommand( [ { diff --git a/apps/web/src/lib/projectScriptKeybindings.ts b/apps/web/src/lib/projectScriptKeybindings.ts index 6f2048f9cd3b..dcd6a011d03f 100644 --- a/apps/web/src/lib/projectScriptKeybindings.ts +++ b/apps/web/src/lib/projectScriptKeybindings.ts @@ -19,11 +19,15 @@ function normalizeProjectScriptKeybindingInput( export function decodeProjectScriptKeybindingRule(input: { keybinding: string | null | undefined; - command: KeybindingCommand; + command: KeybindingCommand | null; }): KeybindingRule | null { const normalizedKey = normalizeProjectScriptKeybindingInput(input.keybinding); if (!normalizedKey) return null; + if (input.command === null) { + throw new Error(PROJECT_SCRIPT_KEYBINDING_INVALID_MESSAGE); + } + const decoded = decodeKeybindingRule({ key: normalizedKey, command: input.command, @@ -36,8 +40,9 @@ export function decodeProjectScriptKeybindingRule(input: { export function keybindingValueForCommand( keybindings: ResolvedKeybindingsConfig, - command: KeybindingCommand, + command: KeybindingCommand | null, ): string | null { + if (command === null) return null; for (let index = keybindings.length - 1; index >= 0; index -= 1) { const binding = keybindings[index]; if (!binding || binding.command !== command) continue; diff --git a/apps/web/src/projectScripts.test.ts b/apps/web/src/projectScripts.test.ts index 1f7a6bfaa9ff..a09a18d03b35 100644 --- a/apps/web/src/projectScripts.test.ts +++ b/apps/web/src/projectScripts.test.ts @@ -1,3 +1,5 @@ +import { MAX_SCRIPT_ID_LENGTH } from "@t3tools/contracts"; +import { shortcutLabelForCommand } from "./keybindings"; import { describe, expect, it } from "vite-plus/test"; import { projectScriptCwd, @@ -57,10 +59,28 @@ describe("projectScripts helpers", () => { it("builds and parses script run commands", () => { const command = commandForProjectScript("lint"); expect(command).toBe("script.lint.run"); - expect(projectScriptIdFromCommand(command)).toBe("lint"); + expect(projectScriptIdFromCommand(command ?? "")).toBe("lint"); expect(projectScriptIdFromCommand("terminal.toggle")).toBeNull(); }); + it.each(["install-javascript-dependencies", "A", "a.b", "a b", "-a", "", "a".repeat(25)])( + "omits the shortcut for legacy script ID %j without crashing script menus", + (id) => { + const commands = ["lint", id, "test"].map(commandForProjectScript); + expect(commands).toEqual(["script.lint.run", null, "script.test.run"]); + expect(commands.map((command) => shortcutLabelForCommand([], command))).toEqual([ + null, + null, + null, + ]); + }, + ); + + it("preserves the exact ID at the shortcut length limit", () => { + const id = "a".repeat(MAX_SCRIPT_ID_LENGTH); + expect(projectScriptIdFromCommand(commandForProjectScript(id) ?? "")).toBe(id); + }); + it("slugifies and dedupes project script ids", () => { expect(nextProjectScriptId("Run Tests", [])).toBe("run-tests"); expect(nextProjectScriptId("Run Tests", ["run-tests"])).toBe("run-tests-2"); diff --git a/apps/web/src/projectScripts.ts b/apps/web/src/projectScripts.ts index e3efc9e3a334..20d06c9c572a 100644 --- a/apps/web/src/projectScripts.ts +++ b/apps/web/src/projectScripts.ts @@ -47,8 +47,11 @@ function normalizeScriptId(value: string): string { return cleaned.slice(0, MAX_SCRIPT_ID_LENGTH).replace(/-+$/g, "") || "script"; } -export const commandForProjectScript = (scriptId: string): KeybindingCommand => - SCRIPT_RUN_COMMAND_PATTERN.make(`script.${scriptId}.run`); +/** Legacy script IDs may not support shortcuts; keep those scripts usable without one. */ +export function commandForProjectScript(scriptId: string): KeybindingCommand | null { + const command = `script.${scriptId}.run`; + return isScriptRunCommand(command) ? command : null; +} export function projectScriptIdFromCommand(command: string): string | null { const trimmed = command.trim();