Skip to content

build(deps): add embedded refinery for control migrations - #370

Merged
Bnjoroge1 merged 4 commits into
mainfrom
Bnjoroge/refinery-dependency
Oct 8, 2026
Merged

Bnjoroge1 merged 4 commits into
mainfrom
Bnjoroge/refinery-dependency

Conversation

@Bnjoroge1

@Bnjoroge1 Bnjoroge1 commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Dedicated dependency-only PR stacked on #369. Adds refinery 0.10.0 with rusqlite and tokio-postgres support; no application code or migration behavior. The upstream crate supports the existing driver versions and Rust 1.97. The parent policy-only PR permits exactly manifest+lock dependency diffs and records three version-pinned cargo-vet exemptions (not audits). Verified cargo metadata resolved only three refinery packages, cargo vet --locked succeeded, and cargo check --locked -p preloop-runner-server passed. This PR must merge before control-migration implementation.


Summary by cubic

Adds refinery 0.10.0 as a workspace dependency with rusqlite and tokio-postgres features, wires it into preloop-runner-server, and updates Cargo.lock. This dependency-only prep must merge before the control-migration implementation; no application or migration behavior changes. The lockfile also bumps transitive getrandom 0.3.4 → 0.4.3.

Written for commit 805d09b. Summary will update on new commits.

View guided diff Turn on auto-fix

Summary by CodeRabbit

  • Chores
    • Updated internal database migration support for SQLite and PostgreSQL. No user-facing changes are included in this release.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4882b3d0-c252-42c2-a9ee-78f01de59d9b
📥 Commits

Reviewing files that changed from the base of the PR and between b59413d and d563c09.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • Cargo.toml
  • crates/preloop-runner-server/Cargo.toml

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The workspace adds refinery with the rusqlite and tokio-postgres features enabled. The preloop-runner-server crate adds refinery as a workspace dependency.

Changes

Refinery dependency setup

Layer / File(s) Summary
Declare and use refinery
Cargo.toml, crates/preloop-runner-server/Cargo.toml
The workspace declares refinery version 0.10.0 with default features disabled and enables rusqlite and tokio-postgres. The server crate adds the workspace dependency.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to d563c

The dependency resolution is committed, so the repository’s locked build workflows are not blocked by this change.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the change: adding Refinery for control migrations. It is concise and specific.
Description check ✅ Passed The description explains the dependency change, its purpose, and the reported verification. It is mostly complete, but it does not explicitly mark the protocol-surface YES/NO item or complete the requ…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@socket-security

socket-security Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​refinery@​0.10.010010093100100

View full report

@Bnjoroge1
Bnjoroge1 force-pushed the Bnjoroge/refinery-dependency branch from d563c09 to 9962f58 Compare October 7, 2026 04:33
Bnjoroge1 added a commit that referenced this pull request Oct 7, 2026
…e CLI

Add forward-only migration sets (migrations/{sqlite,postgres}) applied by
embedded refinery 0.10: frozen v1 baseline, fork-approval sweep index, rerun
history attempt keys (lite rebuild / pg PK swap + run_attempt backfill),
environment deployment columns and the durable review audit table.

- control/migrations.rs: ledger-only boot guard (refinery_schema_history is
  the sole version authority; schema_meta keeps key_fingerprint), legacy/
  foreign/unledgered classification, exact applied-set verification.
- control/migrate_runner.rs: the one schema writer (init, upgrade, explicit
  --adopt-baseline after shape verification), plus SQLite/Postgres
  fresh-vs-upgraded structural fingerprints.
- lite/pg open verify instead of creating; test-support initializes through
  the same migration SQL; preloop serve refuses uninitialized/older/newer/
  divergent databases with the recovery command.
- preloop store migrate|status (store_admin) with a VACUUM INTO pre-migration
  backup for SQLite; brand-new local run/init/installer paths initialize an
  absent store, never upgrade an existing one.
- populated baseline fixtures + migration gate tests (fresh init, upgrade,
  rollback/refusal, adoption, divergence, failed-batch retry), wired into
  just test-ci and the control-plane PG16/17/18 matrix with a zero-PG guard.

Docs: docs/control-migrations.md. Ledger name refinery_schema_history
(parent decision; supersedes the preloop_control_migrations placeholder).
No Cargo.toml/Cargo.lock changes (dependency PR #370 stacked).
@Bnjoroge1
Bnjoroge1 changed the base branch from Bnjoroge/dependency-pr-guard to main October 7, 2026 04:34
@Bnjoroge1 Bnjoroge1 closed this Oct 7, 2026
@Bnjoroge1 Bnjoroge1 reopened this Oct 7, 2026
The previous runs checked out the webhook's stale merge_commit_sha, i.e.
the merge of an older head (engine bug F2, fixed by #409). This empty
commit's synchronize carries GitHub's merge for the current tree.
The last run checked out the stale test-merge eaed4df (merge of 9962f58,
the previous head) and shard 2 hit the known PRELOOP_STORE_URL
test-isolation flake — `store_recovery_preserves_cross_run_queue_order`
opened a sibling test's deleted /tmp/.../from-env.db (#415). Same tree,
fresh run.
The engine restart on rc/prod-1 cancelled the required "Runner light
conformance" check (run 9a9b2f9f, check_run 112892218341) on 2e3df07.
The Checks API rerequest endpoint is App-only (404 for a user token), so
this empty commit re-runs the tree. Same tree as 09cfc58.
@Bnjoroge1
Bnjoroge1 merged commit 8bab45d into main Oct 8, 2026
25 checks passed
Bnjoroge1 added a commit that referenced this pull request Oct 8, 2026
…e CLI

Add forward-only migration sets (migrations/{sqlite,postgres}) applied by
embedded refinery 0.10: frozen v1 baseline, fork-approval sweep index, rerun
history attempt keys (lite rebuild / pg PK swap + run_attempt backfill),
environment deployment columns and the durable review audit table.

- control/migrations.rs: ledger-only boot guard (refinery_schema_history is
  the sole version authority; schema_meta keeps key_fingerprint), legacy/
  foreign/unledgered classification, exact applied-set verification.
- control/migrate_runner.rs: the one schema writer (init, upgrade, explicit
  --adopt-baseline after shape verification), plus SQLite/Postgres
  fresh-vs-upgraded structural fingerprints.
- lite/pg open verify instead of creating; test-support initializes through
  the same migration SQL; preloop serve refuses uninitialized/older/newer/
  divergent databases with the recovery command.
- preloop store migrate|status (store_admin) with a VACUUM INTO pre-migration
  backup for SQLite; brand-new local run/init/installer paths initialize an
  absent store, never upgrade an existing one.
- populated baseline fixtures + migration gate tests (fresh init, upgrade,
  rollback/refusal, adoption, divergence, failed-batch retry), wired into
  just test-ci and the control-plane PG16/17/18 matrix with a zero-PG guard.

Docs: docs/control-migrations.md. Ledger name refinery_schema_history
(parent decision; supersedes the preloop_control_migrations placeholder).
No Cargo.toml/Cargo.lock changes (dependency PR #370 stacked).
Bnjoroge1 added a commit that referenced this pull request Oct 8, 2026
…R state

No code.

- State the file-anchor convention (`crates/preloop-runner-server/src/`), so
  `control/lite/schema.sql` and `github.rs:1677` resolve for a reader; note
  that load-bearing anchors were re-checked against `public/main` @ `8bab45db`.
- #370 (embedded refinery) has landed and #372 (the migration cutover) is still
  open, so the deliverable-1 migration note now says what is actually on main.
Bnjoroge1 added a commit that referenced this pull request Oct 8, 2026
…e CLI

Add forward-only migration sets (migrations/{sqlite,postgres}) applied by
embedded refinery 0.10: frozen v1 baseline, fork-approval sweep index, rerun
history attempt keys (lite rebuild / pg PK swap + run_attempt backfill),
environment deployment columns and the durable review audit table.

- control/migrations.rs: ledger-only boot guard (refinery_schema_history is
  the sole version authority; schema_meta keeps key_fingerprint), legacy/
  foreign/unledgered classification, exact applied-set verification.
- control/migrate_runner.rs: the one schema writer (init, upgrade, explicit
  --adopt-baseline after shape verification), plus SQLite/Postgres
  fresh-vs-upgraded structural fingerprints.
- lite/pg open verify instead of creating; test-support initializes through
  the same migration SQL; preloop serve refuses uninitialized/older/newer/
  divergent databases with the recovery command.
- preloop store migrate|status (store_admin) with a VACUUM INTO pre-migration
  backup for SQLite; brand-new local run/init/installer paths initialize an
  absent store, never upgrade an existing one.
- populated baseline fixtures + migration gate tests (fresh init, upgrade,
  rollback/refusal, adoption, divergence, failed-batch retry), wired into
  just test-ci and the control-plane PG16/17/18 matrix with a zero-PG guard.

Docs: docs/control-migrations.md. Ledger name refinery_schema_history
(parent decision; supersedes the preloop_control_migrations placeholder).
No Cargo.toml/Cargo.lock changes (dependency PR #370 stacked).
Bnjoroge1 added a commit that referenced this pull request Oct 8, 2026
…e CLI

Add forward-only migration sets (migrations/{sqlite,postgres}) applied by
embedded refinery 0.10: frozen v1 baseline, fork-approval sweep index, rerun
history attempt keys (lite rebuild / pg PK swap + run_attempt backfill),
environment deployment columns and the durable review audit table.

- control/migrations.rs: ledger-only boot guard (refinery_schema_history is
  the sole version authority; schema_meta keeps key_fingerprint), legacy/
  foreign/unledgered classification, exact applied-set verification.
- control/migrate_runner.rs: the one schema writer (init, upgrade, explicit
  --adopt-baseline after shape verification), plus SQLite/Postgres
  fresh-vs-upgraded structural fingerprints.
- lite/pg open verify instead of creating; test-support initializes through
  the same migration SQL; preloop serve refuses uninitialized/older/newer/
  divergent databases with the recovery command.
- preloop store migrate|status (store_admin) with a VACUUM INTO pre-migration
  backup for SQLite; brand-new local run/init/installer paths initialize an
  absent store, never upgrade an existing one.
- populated baseline fixtures + migration gate tests (fresh init, upgrade,
  rollback/refusal, adoption, divergence, failed-batch retry), wired into
  just test-ci and the control-plane PG16/17/18 matrix with a zero-PG guard.

Docs: docs/control-migrations.md. Ledger name refinery_schema_history
(parent decision; supersedes the preloop_control_migrations placeholder).
No Cargo.toml/Cargo.lock changes (dependency PR #370 stacked).
Bnjoroge1 added a commit that referenced this pull request Oct 8, 2026
…e CLI (#372)

* feat(control): versioned refinery migrations, boot ledger guard, store CLI

Add forward-only migration sets (migrations/{sqlite,postgres}) applied by
embedded refinery 0.10: frozen v1 baseline, fork-approval sweep index, rerun
history attempt keys (lite rebuild / pg PK swap + run_attempt backfill),
environment deployment columns and the durable review audit table.

- control/migrations.rs: ledger-only boot guard (refinery_schema_history is
  the sole version authority; schema_meta keeps key_fingerprint), legacy/
  foreign/unledgered classification, exact applied-set verification.
- control/migrate_runner.rs: the one schema writer (init, upgrade, explicit
  --adopt-baseline after shape verification), plus SQLite/Postgres
  fresh-vs-upgraded structural fingerprints.
- lite/pg open verify instead of creating; test-support initializes through
  the same migration SQL; preloop serve refuses uninitialized/older/newer/
  divergent databases with the recovery command.
- preloop store migrate|status (store_admin) with a VACUUM INTO pre-migration
  backup for SQLite; brand-new local run/init/installer paths initialize an
  absent store, never upgrade an existing one.
- populated baseline fixtures + migration gate tests (fresh init, upgrade,
  rollback/refusal, adoption, divergence, failed-batch retry), wired into
  just test-ci and the control-plane PG16/17/18 matrix with a zero-PG guard.

Docs: docs/control-migrations.md. Ledger name refinery_schema_history
(parent decision; supersedes the preloop_control_migrations placeholder).
No Cargo.toml/Cargo.lock changes (dependency PR #370 stacked).

* fix(control): queue bucket alias, archive attempt keys, migration test/schema drift

- queue_stats: GROUP BY bucket — the `kind` alias was captured by the
  jobs.kind column, collapsing every queue bucket into one arbitrary row
- archive_finished_runs (lite+pg): job_history writes run_attempt, which
  migration 2026100503 made NOT NULL
- lite schema.sql: history tables quoted and jobs columns appended exactly
  as the migration chain leaves them (parity test)
- first test-compile fixes: GenericClient fingerprint, &mut adoption
  preflight, OptionalExtension import, unused muts
- populated-baseline upgrade goes through probe -> adopt like the CLI

* fix(control): baseline migration matches the current v5/v4 schema

main moved the control schema in place (Postgres v5 / SQLite v4, #365):
`check_run_updates` gained the outbox `version`/`lease_owner` columns and
dropped its foreign key to `jobs`. The frozen baseline was copied from the
pre-#365 tree, so a database created by the current runtime no longer
matched it: `preloop store migrate --adopt-baseline` refused every released
store, and the fresh-vs-upgraded parity checks drifted from `schema.sql`.

The baseline is now the shape the pre-migration runtime actually creates,
which is what adoption and the upgrade fixtures compare against. Header
names the commit it was frozen from.

* fix(control): upgrade a populated pre-ledger pg baseline through adopt

The Postgres arm of the migration gate ran the embedded set directly
against a populated pre-ledger store. That cannot work: the set starts at
the frozen baseline, whose first table (`schema_meta`) already exists, so
refinery aborted with `relation "schema_meta" already exists` — the gate
was red and the flow it claims to cover was never actually exercised.

The upgrade an operator runs is `preloop store migrate --adopt-baseline`:
probe the pre-ledger shape, stamp the ledger at the verified version, apply
the rest. The SQLite twin of this gate already runs exactly that probe →
adopt → apply sequence; the Postgres arm now does the same, and the
populated fixture rows are still checked on the upgraded database.

* docs(control): name the pre-ledger versions the guard meets

The current in-place schema moves are SQLite v4 / Postgres v5 (#365), but
the ledger refusal and the `--adopt-baseline` help text still said v3/v4 —
an operator reading the refusal would look for the wrong build. The
foreign-store refusal also named a `preloop store import` command that does
not exist; the command is `preloop store import-legacy`, as
docs/control-migrations.md and the legacy-store refusal already say.

* style(control): rustfmt the migration branch

The rust-lint job runs `cargo fmt --all -- --check`; the store CLI, the
lite/pg open guards and the migration gate tests landed unformatted.

* fix(conformance): initialize the replay server's control store

The control store is migration-ledgered: `preloop serve` refuses a database
that has no ledger instead of creating one, and the conformance harness
starts the replay server against a brand-new state dir — so the server
exited with "control database .../preloop.db is not initialized" and every
server-light run failed before the first scenario.

A fresh store is initialized the way an install does: `preloop store
migrate` (create-only on an empty database, embedded migrations + ledger)
now runs once in the harness before the server starts. The harness also
builds the CLI it uses for that.

* fix(cli): gate the install-time store init on the platform that installs

`prepare_control_store` is only called from the systemd installer path, so
on any other host it is dead code and `cargo clippy --workspace
--all-targets -- -D warnings` fails the lint job there (`function is never
used`). Its Linux-only siblings next to it (the env-file helpers, the smolvm
bootstraps) are gated the same way.

* fix(store): prepare the control store as the service account, never as root

A system install initialized the brand-new SQLite store as root *before*
the recursive chown, inside the state tree a previous install had already
handed to the `preloop` account. That account could plant
`state/preloop.db` (or `state` itself) as a symlink and aim root's
create, write and chmod at a target of its choosing — a dangling link read
as "absent" and an empty target read as "empty" for the brand-new check.

`install_systemd` now chowns first and runs the internal `store
init-local` as the service account (`setpriv --reuid/--regid preloop
--clear-groups`) with an emptied environment; a user-scope install runs
the same command as the invoker, so the create only ever happens with the
privileges the service itself has. The preparation now judges "brand-new"
with `symlink_metadata`, refuses a symlinked database path instead of
following it (a link to an existing store is still left untouched),
refuses non-regular files, and — for a privileged (euid 0) caller —
requires the parent directory to be exclusively the preparer's own.

Zero-config first start is unchanged: the database is still created at
install time and an existing store is still never touched.

* test(control): keep the ambient store URL out of test-build AppState

The store-URL precedence tests in tests/concurrency.rs mutate the
process-wide PRELOOP_STORE_URL under GITHUB_ENV_LOCK, but every other test
that builds an AppState resolves its store through the same variable inside
Backend::open, so a victim can open a sibling test temp store (deleted:
CANTOPEN panic; alive: foreign store). In test/test-support builds
AppState::new_with_store now pins a missing store URL to the state-dir
default; Backend::open keeps the production env fallback, and
test_open_backend still exercises that contract. A regression test pins the
hermetic AppState. Production selection is unchanged.

* feat(control): migration 0505 drops the unused control objects and aligns the lite indexes

Rebase onto main's schema trim (#368): the baseline still carries the dropped
objects and the old SQLite jobs_ready key, so forward-migrate them away.
Harden the drift guard for SQLite's quoted identifiers and one-line column
lists, and make the migration/schema parity test ignore DDL comments.

* fix(control): reconcile the environment columns with the migration column order

---------

Co-authored-by: Bnjoroge1 <Bnjoroge1@users.noreply.github.com>
Co-authored-by: Preloop Agent <agent@preloop.invalid>
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