Skip to content

refactor: enforce SQLite persistence boundaries - #294

Merged
ecarreras merged 1 commit into
mainfrom
feat/issue-260-persistence-boundaries
Oct 8, 2026
Merged

ecarreras merged 1 commit into
mainfrom
feat/issue-260-persistence-boundaries

Conversation

@giscebot

@giscebot giscebot commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • move trigger-actor backfill reads/writes into a typed persistence repository
  • move fresh/upgrade database initialization SQL into the migrations package
  • add an AST architecture test that confines connections and SQL to the persistence, migration, and dashboard read-model boundaries
  • document the extension contract and retention/cleanup ownership

The actor backfill no longer performs feature-local DDL. A write pass now requires the current migrated schema and directs operators to gab migrate-db; --dry-run still inspects legacy databases without mutation.

No schema or public CLI/API shape changes are included.

Validation

  • pytest -q — 541 passed
  • pytest -q tests/test_persistence_architecture.py tests/test_actors.py tests/test_queue.py tests/test_migrations.py tests/test_packaged_resources.py — 73 passed
  • git diff --check

Requested by: @ecarreras

Relates to #260.

Move trigger-actor backfill storage and database initialization behind the persistence and migration layers, then pin the boundary with an architecture test and operational documentation.\n\nCo-authored-by: ecarreras <ecarreras@users.noreply.github.com>

@pilipilisbot pilipilisbot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the persistence-boundary refactor and this looks ready to merge.

What I checked:

  • The PR head matches 88dcc9e and the diff is scoped to actor backfill extraction, migration initialization ownership, boundary tests, and persistence docs.
  • backfill_trigger_actors() now reads/writes through ActorBackfillRepository; write mode validates the migrated schema first and no longer performs feature-local DDL. Dry-run still reads legacy databases without mutating them.
  • initialize_database() moved into the migrations package without changing the explicit migration path semantics.
  • The AST architecture test allows SQL only in persistence, migrations, and the dashboard read model, which matches the #260 boundary contract.

Validation run locally in a temporary venv:

  • git diff --check origin/main...HEAD
  • pytest -q tests/test_persistence_architecture.py tests/test_actors.py tests/test_queue.py tests/test_migrations.py tests/test_packaged_resources.py -> 74 passed
  • pytest -q -> 541 passed, 1 unrelated Starlette/httpx deprecation warning
  • gab --db <tmp>/bridge.sqlite3 migrate-db -> applied migrations 1 and 2 on a fresh temporary DB

No blocking findings from my side.

@ecarreras
ecarreras merged commit f71bbd3 into main Oct 8, 2026
4 checks passed
@ecarreras
ecarreras deleted the feat/issue-260-persistence-boundaries branch October 8, 2026 19:26
@giscebot

giscebot commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Post-merge follow-up: reviewed merge commit f71bd... (PR head 88dcc9e) and confirmed the dedicated worktree was clean, so it has been removed. The repository checkout was not modified.

Validation evidence: GitHub dashboard, pytest (3.11), and pytest (3.12) checks passed; local git diff --check passed on the merged PR head. No additional review findings or follow-up code changes are required.

@pilipilisbot

Copy link
Copy Markdown
Collaborator

Post-merge cleanup rerun: PR #294 is merged at f71bbd318c66d4058fdf00b53456a36d5f10d4c8, and the lingering local worktree /home/openclaw/projects/gisce-ti/worktrees/github-agent-bridge-pr-294 has now been removed after confirming it was clean. Canonical checkout left untouched.

@giscebot

giscebot commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Post-merge review follow-up: PR #294 is merged at f71bbd318c66d4058fdf00b53456a36d5f10d4c8. Current GitHub checks are green for dashboard, pytest (3.11), and pytest (3.12); the only remaining status is the expected external agent job still pending. No additional code or review action is required.

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.

3 participants