Skip to content

Refactor automation mutations behind typed command writer - #1627

Open
open-inspect[bot] wants to merge 2 commits into
mainfrom
fix/a05-automation-commands
Open

Refactor automation mutations behind typed command writer#1627
open-inspect[bot] wants to merge 2 commits into
mainfrom
fix/a05-automation-commands

Conversation

@open-inspect

@open-inspect open-inspect Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements audit finding A05 by moving automation mutation parsing and aggregate persistence out of the HTTP route.

  • Adds distinct runtime create and update ingress schemas that reuse canonical shared schemas while preserving update-only null/clear semantics.
  • Resolves accepted mutations into typed CreateAutomationCommand and UpdateAutomationCommand values.
  • Adds D1AutomationAggregateWriter as the single atomic persistence boundary for automation rows, repositories, environments, provider selections, and Slack watched-channel indexes.
  • Removes raw AutomationRow construction, Record<string, unknown> update assembly, and cross-store SQL statement composition from the route.
  • Uses one command timestamp across scalar and relationship writes.
  • Keeps authentication, raw request reading, identity enforcement, external repository/environment/provider resolution, and HTTP response/error mapping in the route.

Preserved Contracts

  • HTTP paths, request bodies, statuses, response hydration, and one-time webhook/Sentry response material remain unchanged.
  • Canonical attribution continues to come only from the verified principal; identity enforcement receives the raw pre-schema body.
  • Create and update remain separate schemas and commands.
  • Update omission remains distinct from explicit clear or replacement for trigger config, event type, reasoning effort, repositories, environments, and provider selections.
  • Slack trigger config remains non-clearable and its watched-channel index is synchronized in the same atomic batch.
  • Schedule validation and next_run_at recomputation, target final-state validation, provider-account policy, repository/environment resolution, and in-flight invocation semantics are unchanged.

TDD Evidence

Red

Before production changes, focused tests were added for the missing ingress and writer seams. The initial run failed exactly as expected because these modules did not exist:

  • Cannot find module './automation-mutation'
  • Cannot find module '/src/db/automation-aggregate-writer'

A later HTTP-compatibility regression test was also added first and failed with name is required instead of the preserved update response name cannot be empty.

Green

The minimum parser, typed commands, and aggregate writer implementation made the focused tests pass. Route tests then confirmed unchanged statuses and payload behavior.

Refactor

After green, the code-simplifier review removed a redundant repository-selection wrapper and duplicate post-schema type guards. Focused tests were rerun after simplification. A final review added explicit triggerConfig: null writer coverage and operation-specific scalar error mapping.

Verification

  • npm test -w @open-inspect/control-plane -- src/automation/automation-mutation.test.ts src/db/automation-aggregate-writer.test.ts src/routes/automations.test.ts src/db/automation-store.test.ts (126 passed before final compatibility additions)
  • npm test -w @open-inspect/control-plane (214 files, 3,260 tests passed)
  • npm run test:integration -w @open-inspect/control-plane -- test/integration/automation-store.test.ts test/integration/automations-slack-route.test.ts test/integration/automation-invocations.test.ts (3 files, 86 tests passed)
  • npm run typecheck -w @open-inspect/control-plane
  • npm run lint -w @open-inspect/control-plane
  • npm run build -w @open-inspect/control-plane
  • Post-commit focused run: 3 files, 97 tests passed
  • git diff --check

Risks

  • Runtime update ingress intentionally permits null for eventType and triggerConfig because those clears are existing HTTP behavior, even though the shared request schema is narrower. This PR does not expand or alter shared client contracts.
  • Atomic rollback relies on the existing SqlDatabase.batch() all-or-nothing contract; targeted integration tests exercise the real D1 batch path and Slack synchronization.

A04 Interaction

This change does not modify Scheduler transport/result contracts or depend on A04. The only likely overlap is the large routes/automations.ts file; A05 changes are confined to create/update parsing and persistence, while scheduler call/result handling is left untouched.


Created with Open-Inspect

@github-actions

Copy link
Copy Markdown

Terraform Validation Results

Step Status
Format
Init
Validate
Tests

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

Comment thread packages/control-plane/src/automation/automation-mutation.ts Outdated
Comment thread packages/control-plane/src/routes/automations.ts Outdated
@github-actions

Copy link
Copy Markdown

Terraform Validation Results

Step Status
Format
Init
Validate
Tests

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @open-inspect[bot], Action: pull_request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant