-
Notifications
You must be signed in to change notification settings - Fork 405
Refactor automation mutations behind typed command writer #1627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
open-inspect
wants to merge
2
commits into
main
Choose a base branch
from
fix/a05-automation-commands
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
85 changes: 85 additions & 0 deletions
85
packages/control-plane/src/automation/automation-mutation.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { | ||
| AutomationMutationInputError, | ||
| parseCreateAutomationMutation, | ||
| parseUpdateAutomationMutation, | ||
| } from "./automation-mutation"; | ||
|
|
||
| describe("automation mutation ingress", () => { | ||
| it("parses and normalizes a create request with canonical nested schemas", () => { | ||
| const input = parseCreateAutomationMutation({ | ||
| name: "Daily review", | ||
| instructions: "Review open changes", | ||
| triggerType: "slack_event", | ||
| triggerConfig: { | ||
| conditions: [{ type: "slack_channel", operator: "any_of", value: ["C123"] }], | ||
| }, | ||
| repositories: [{ repoOwner: " Acme ", repoName: " API ", baseBranch: null }], | ||
| providerSelections: { openai: { mode: "provider_account", accountId: "a".repeat(32) } }, | ||
| actorDisplayName: "Automation Bot", | ||
| }); | ||
|
|
||
| expect(input).toMatchObject({ | ||
| triggerType: "slack_event", | ||
| repositories: [{ repoOwner: "acme", repoName: "api", baseBranch: null }], | ||
| triggerConfig: { | ||
| conditions: [{ type: "slack_channel", operator: "any_of", value: ["C123"] }], | ||
| }, | ||
| actorDisplayName: "Automation Bot", | ||
| }); | ||
| }); | ||
|
|
||
| it("rejects invalid trigger-specific shapes at ingress", () => { | ||
| expect(() => | ||
| parseCreateAutomationMutation({ | ||
| name: "Slack review", | ||
| instructions: "Review", | ||
| triggerType: "slack_event", | ||
| triggerConfig: { conditions: "C123" }, | ||
| }) | ||
| ).toThrowError(new AutomationMutationInputError("triggerConfig.conditions must be an array")); | ||
| }); | ||
|
|
||
| it("keeps create and update semantics distinct", () => { | ||
| expect(() => parseCreateAutomationMutation({ name: "Incomplete" })).toThrowError( | ||
| new AutomationMutationInputError("instructions is required") | ||
| ); | ||
|
|
||
| expect(parseUpdateAutomationMutation({ name: "Renamed" })).toEqual({ name: "Renamed" }); | ||
| }); | ||
|
|
||
| it("preserves update-specific scalar error messages", () => { | ||
| expect(() => parseUpdateAutomationMutation({ name: 42 })).toThrowError( | ||
| new AutomationMutationInputError("name cannot be empty") | ||
| ); | ||
| expect(() => parseUpdateAutomationMutation({ instructions: null })).toThrowError( | ||
| new AutomationMutationInputError("instructions cannot be empty") | ||
| ); | ||
| }); | ||
|
|
||
| it("preserves omitted, cleared, and replaced update fields", () => { | ||
| const omitted = parseUpdateAutomationMutation({ name: "Renamed" }); | ||
| expect("triggerConfig" in omitted).toBe(false); | ||
| expect("reasoningEffort" in omitted).toBe(false); | ||
| expect("repositories" in omitted).toBe(false); | ||
| expect("environmentIds" in omitted).toBe(false); | ||
| expect("providerSelections" in omitted).toBe(false); | ||
|
|
||
| const cleared = parseUpdateAutomationMutation({ | ||
| triggerConfig: null, | ||
| eventType: null, | ||
| reasoningEffort: null, | ||
| repositories: [], | ||
| environmentIds: [], | ||
| providerSelections: {}, | ||
| }); | ||
| expect(cleared).toEqual({ | ||
| triggerConfig: null, | ||
| eventType: null, | ||
| reasoningEffort: null, | ||
| repositories: [], | ||
| environmentIds: [], | ||
| providerSelections: {}, | ||
| }); | ||
| }); | ||
| }); |
146 changes: 146 additions & 0 deletions
146
packages/control-plane/src/automation/automation-mutation.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| import { z } from "zod"; | ||
| import { | ||
| automationRepositoriesInputSchema, | ||
| createAutomationRequestSchema, | ||
| updateAutomationRequestSchema, | ||
| } from "@open-inspect/shared/types/automations"; | ||
| import { triggerConfigSchema } from "@open-inspect/shared/triggers"; | ||
| import { modelProviderSelectionsSchema } from "@open-inspect/shared/types/provider-accounts"; | ||
|
|
||
| const automationTriggerTypeSchema = createAutomationRequestSchema.shape.triggerType.unwrap(); | ||
|
|
||
| const createAutomationMutationSchema = z | ||
| .object({ | ||
| name: createAutomationRequestSchema.shape.name, | ||
| instructions: createAutomationRequestSchema.shape.instructions, | ||
| triggerType: automationTriggerTypeSchema.nullable().optional(), | ||
| scheduleCron: createAutomationRequestSchema.shape.scheduleCron.nullable(), | ||
| scheduleTz: createAutomationRequestSchema.shape.scheduleTz.nullable(), | ||
| model: createAutomationRequestSchema.shape.model.nullable(), | ||
| reasoningEffort: createAutomationRequestSchema.shape.reasoningEffort, | ||
| eventType: createAutomationRequestSchema.shape.eventType.nullable(), | ||
| triggerConfig: triggerConfigSchema.optional(), | ||
| sentryClientSecret: createAutomationRequestSchema.shape.sentryClientSecret, | ||
| repositories: automationRepositoriesInputSchema.optional(), | ||
| environmentIds: z.array(z.string()).optional(), | ||
| providerSelections: modelProviderSelectionsSchema.nullable().optional(), | ||
| actorDisplayName: z.string().optional(), | ||
| actorEmail: z.string().optional(), | ||
| actorAvatarUrl: z.string().optional(), | ||
| }) | ||
| .transform(({ triggerType, providerSelections, ...input }) => ({ | ||
| ...input, | ||
| ...(triggerType ? { triggerType } : {}), | ||
| ...(providerSelections ? { providerSelections } : {}), | ||
| })); | ||
|
|
||
| const updateAutomationMutationSchema = z.object({ | ||
| name: updateAutomationRequestSchema.shape.name, | ||
| instructions: updateAutomationRequestSchema.shape.instructions, | ||
| scheduleCron: updateAutomationRequestSchema.shape.scheduleCron, | ||
| scheduleTz: updateAutomationRequestSchema.shape.scheduleTz, | ||
| model: updateAutomationRequestSchema.shape.model, | ||
| reasoningEffort: updateAutomationRequestSchema.shape.reasoningEffort, | ||
| eventType: z.string().nullable().optional(), | ||
| triggerConfig: triggerConfigSchema.nullable().optional(), | ||
| repositories: automationRepositoriesInputSchema.optional(), | ||
| environmentIds: z.array(z.string()).optional(), | ||
| providerSelections: modelProviderSelectionsSchema.optional(), | ||
| }); | ||
|
|
||
| export type CreateAutomationMutation = z.output<typeof createAutomationMutationSchema>; | ||
| export type UpdateAutomationMutation = z.output<typeof updateAutomationMutationSchema>; | ||
|
|
||
| export class AutomationMutationInputError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = "AutomationMutationInputError"; | ||
| } | ||
| } | ||
|
|
||
| function formatTriggerConfigIssue(value: unknown, issue: z.core.$ZodIssue): string { | ||
| if (issue.path.length === 1 && issue.path[0] === "conditions") { | ||
| return "triggerConfig.conditions must be an array"; | ||
| } | ||
|
|
||
| const path = ["triggerConfig", ...issue.path].map(String).join("."); | ||
| const conditionIndex = issue.path[0] === "conditions" ? issue.path[1] : undefined; | ||
| const rawConditions = | ||
| typeof value === "object" && value !== null && "conditions" in value | ||
| ? (value as { conditions?: unknown }).conditions | ||
| : undefined; | ||
| const rawCondition = | ||
| typeof conditionIndex === "number" && Array.isArray(rawConditions) | ||
| ? rawConditions[conditionIndex] | ||
| : undefined; | ||
| const conditionType = | ||
| typeof rawCondition === "object" && | ||
| rawCondition !== null && | ||
| "type" in rawCondition && | ||
| typeof rawCondition.type === "string" | ||
| ? `${rawCondition.type}: ` | ||
| : ""; | ||
| return `${path}: ${conditionType}${issue.message}`; | ||
| } | ||
|
|
||
| function formatMutationIssue( | ||
| value: unknown, | ||
| issue: z.core.$ZodIssue, | ||
| operation: "create" | "update" | ||
| ): string { | ||
| const field = issue.path[0]; | ||
| if (field === "name") return operation === "create" ? "name is required" : "name cannot be empty"; | ||
| if (field === "instructions") { | ||
| return operation === "create" ? "instructions is required" : "instructions cannot be empty"; | ||
| } | ||
| if (field === "triggerType") { | ||
| return `triggerType must be one of: ${automationTriggerTypeSchema.options.join(", ")}`; | ||
| } | ||
| if (field === "triggerConfig") { | ||
| const triggerConfig = | ||
| typeof value === "object" && value !== null && "triggerConfig" in value | ||
| ? (value as { triggerConfig?: unknown }).triggerConfig | ||
| : undefined; | ||
| return formatTriggerConfigIssue(triggerConfig, { ...issue, path: issue.path.slice(1) }); | ||
| } | ||
| if (field === "repositories") { | ||
| const index = issue.path[1]; | ||
| return `repositories${typeof index === "number" ? `[${index}]` : ""}: ${issue.message}`; | ||
| } | ||
| if (field === "environmentIds") { | ||
| return "environmentIds must be an array of environment ids (env_…)"; | ||
| } | ||
| if (field === "providerSelections") { | ||
| return ["providerSelections", ...issue.path.slice(1)].join(".") + `: ${issue.message}`; | ||
| } | ||
| return issue.message; | ||
| } | ||
|
|
||
| function parseMutation<T>(schema: z.ZodType<T>, value: unknown, operation: "create" | "update"): T { | ||
| const parsed = schema.safeParse(value); | ||
| if (parsed.success) return parsed.data; | ||
| const issue = parsed.error.issues[0]; | ||
| throw new AutomationMutationInputError( | ||
| issue ? formatMutationIssue(value, issue, operation) : "Invalid automation request" | ||
| ); | ||
| } | ||
|
|
||
| export function parseCreateAutomationMutation(value: unknown): CreateAutomationMutation { | ||
| return parseMutation(createAutomationMutationSchema, value, "create"); | ||
| } | ||
|
|
||
| export function parseUpdateAutomationMutation( | ||
| value: unknown, | ||
| currentTriggerType?: string | ||
| ): UpdateAutomationMutation { | ||
| if ( | ||
| currentTriggerType === "schedule" && | ||
| typeof value === "object" && | ||
| value !== null && | ||
| "triggerConfig" in value && | ||
| value.triggerConfig !== undefined | ||
| ) { | ||
| throw new AutomationMutationInputError("Cannot set triggerConfig on schedule automations"); | ||
| } | ||
| return parseMutation(updateAutomationMutationSchema, value, "update"); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.