azurenoops / azurenoops/spin_agent

chore: add lint/test guard enforcing idempotency on all EnsureSchemaAdditions DDL scripts

Open
#877 0 comments 0 reactions 0 assignees View on GitHub
area:testing debt priority:medium
Dominant language
C#
Stars
3
Forks
1
Avg merge
18h 9m
Merged PRs (30d)
43

Description

## Background

PR #870 (merged 2026-09-09, commit `2ae0f7a5`) converted every `EnsureSchemaAdditions` catch block from `LogWarning`-and-swallow to `LogError` + `throw InvalidOperationException`. This is the correct fail-fast posture for a compliance product — booting on a structurally incomplete schema is a data-integrity hazard.

However, this makes a previously implicit invariant **load-bearing**:

> Every DDL script in `EnsureSchemaAdditions` MUST be idempotent (`CREATE TABLE/INDEX IF NOT EXISTS`, PRAGMA-guarded ALTER, etc.). Any exception now hard-aborts startup.

A future contributor who adds a non-idempotent script will cause a hard startup abort on the second run, with no compile-time or test-time warning.

## Problem

There is currently no automated guard that enforces this invariant. The risk is silent regression: a new DDL block that works on first boot will crash the service on every subsequent boot.

## Proposed Solution

Add one or both of:

1. **Unit test** — A test that boots a fresh SQLite DB, runs `EnsureSchemaAdditionsAsync` twice in a row, and asserts no exception on the second run. This catches non-idempotent scripts at test time.

2. **Lint/analyzer rule** — A Roslyn analyzer or custom lint step that scans `EnsureSchemaAdditions/*.cs` and flags any raw `ALTER TABLE`, `CREATE TABLE`, or `CREATE INDEX` that lacks the appropriate `IF NOT EXISTS` / `IF EXISTS` guard.

At minimum, the unit test (option 1) should be added as it requires no new tooling.

## Acceptance Criteria

- AC1: Running `EnsureSchemaAdditionsAsync` twice on a clean SQLite DB succeeds without exception.
- AC2: The test is included in the unit test project and runs in CI.
- AC3: (Optional stretch) A lint rule or comment convention documents the idempotency requirement inline in each `ApplyAsync` method signature.

## References

- PR #870 (merged) — introduced fail-fast contract
- Tony Stark architecture artifact `3dc72ec6b28c4513` — Risk #2: "Idempotency invariant is now load-bearing"
- `src/Ato.Copilot.Core/Data/Migrations/EnsureSchemaAdditions/`

Contributor guide

Open the contributing guide

Research direction

Start in src/Ato.Copilot.Core/Data/Migrations/EnsureSchemaAdditions/ and find the EnsureSchemaAdditionsAsync entry point and the existing unit test project. Run the current migration tests, then exercise a fresh SQLite database through two successive runs. Done means the second run succeeds without exception and the test runs in CI.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, sqlite
Domain
databases, testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.