[ac-pplx]

Summary

template-manager's node_dirty_refs outbox worker (template/mgr/src/node_state_sync.rs) lists a batch, sends MarkFacetDirty to node-ctl without holding the database lock, then deletes the listed refs by key. A re-enqueue of the same ref in between updates that row in place, so the delete dropped it until the ref was marked again or reconciliation caught up. Now it stays pending for the next pass.

• Migration db/template-manager/20261031050126_node_dirty_ref_revisions (additive, sandbox-manager's outbox shape): node_dirty_refs.revision INTEGER NOT NULL DEFAULT 0, a singleton counter node_dirty_ref_revision, and insert and update triggers that stamp each enqueue with the next value. The update trigger fires on UPDATE OF requested_at, updated_at, which every upsert sets, so a same-second re-enqueue gets a new revision.
• template/mgr/src/db.rs: list_node_dirty_refs reads revision. delete_node_dirty_refs deletes only rows still at the listed revision. Enqueues and MarkFacetDirty are unchanged.
• sqlite/src/template_manager.rs: the module doc now says a migration ships only in a release after one that lists it, so the image a rollback returns to can reverse it; 20261031050126's check never refuses, so that rollback is image-only. template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277's DIRTY_REF_REVISIONS entry stays, documented as bundled from here on; the bundled up SQL is byte-identical to its history copy, which template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277's checksum tests pin.
• Tests, template-manager: the regression test; a stale ack against a recreated key; a same-timestamp write (recursive_triggers off and on); revision exhaustion.
• Tests, pplx-space-sqlite:
  • template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277's reversal tests roll back to UNREVISIONED (this catalog without 20261031050126: the previous release).
  • The round trip starts from that release's database: this catalog applies the migration and keeps it on restart, then it is reversed, re-applied and reversed again.
  • a_bundled_schema_is_left_untouched checks recorded revisions survive a restart.
  • Retired-migration tests derive history from the bundled catalog; HISTORICAL covers upgrading from the release that recorded it.

Stack

Stacked on #1277; ship in a release after one that contains #1277. That release reverses 20261031050126 when a database records it without the catalog bundling it. Merge and release #1277 first, then retarget to main.

Validation

Linux KVM devbox, at ea3ddcc6. This head differs from it only in the commit message and one module-doc comment in sqlite/src/template_manager.rs.

• Regression: acking_a_sent_dirty_ref_keeps_a_re_enqueue_made_during_the_send fails on template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277's code (left: 1, right: 0: the stale ack deleted the re-enqueue) and passes here; the other three new template-manager tests also fail there.
• Mutations (local only, then restored):
  • no insert trigger: the stale-ack test fails;
  • WHEN OLD.updated_at IS NOT NEW.updated_at on the update trigger: the same-timestamp, regression and exhaustion tests fail;
  • reversing listed migrations even when bundled fails a_bundled_schema_is_left_untouched: revisions (3, [3, 2]) become (0, [0, 0]).
• cargo nextest run -p pplx-space-sqlite -p pplx-space-template-mgr -p pplx-space-db-migrate: 383 passed, 0 skipped (privileged group included); with -p pplx-space-archcheck: 113 passed.
• clippy -D warnings, cargo fmt --check, scripts/check-migration-versions.sh and cargo arch (0 required findings) pass.
• Real binaries, one database file (space-db-migrator from this branch and template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277, separate target dirs):
  1. this branch's up: history [20260930000000, 20261031050126], column, counter and triggers; seeded revisions [3, 2];
  2. template-manager: reverse the next release's node_dirty_ref_revisions migration so its rollback stays image-only #1277's up: logged the reversal, history [20260930000000], revision schema gone, both rows kept;
  3. this branch's up: re-applied, revisions restart at 0; a re-enqueue's revision 1 survives another up.
• Kind, fresh clusters from this branch; all four template-manager databases recorded [20260930000000, 20261031050126].
  • Datadog telemetry (bring-up 4 min 47 s) passed: local.template_startup_recovery_decision_table, local.template_cache_lru_eviction_reclaims, local.unanswered_template_deleted_after_wait, local.ecw_template_build_from_oci_boots, local.private_template_build_isolation, local.node_ctl_fence_refuses_and_reconciles, ecw.background_process_control. local.btrfs_send_restore_pulls_parent_template: infra_error (needs Prometheus).
  • otel telemetry, as in CI (bring-up 3 min 15 s): local.btrfs_send_restore_pulls_parent_template passed.
  • scripts/tests/smoke-kind-typed.sh all there (62 min 27 s): ECW 87 passed, 2 inconclusive (ecw.cold_create_environment, ecw.warm_pool_environment: the script doesn't set SPACE_SMOKE_KIND_COLD_RECEIPTS; both passed with it set as in kind-ci.yml); native 35 passed; 0 failed.

Not validated: a multi-node fleet rollout, or a Kind rollback to #1277's image (covered by the binary round trip).

Severity and risk

Severity: medium. A lost ack delays a policy or template-inventory change on a node until it is marked again or reconciled; rare and unreported.

Risk: two small trigger updates per enqueue, in its existing transaction. A stale ack leaves the row to be resent, at-least-once delivery node-ctl already handles.

Rollout

Node-local SQLite; template-manager migrates at startup. The previous release works on the migrated schema (explicit columns; the triggers stamp its writes). Existing rows keep revision 0.

Rollback to a release containing #1277 is image-only. Its startup (or migrator up) reverses 20261031050126 in one transaction and keeps every outbox row. That release acks by key again, as before this change. A release without #1277 refuses the database (unbundled migration), so roll back from this release only to one that contains #1277.

Overlaps: #1075 deletes sqlite/src/template_manager.rs and its tests.rs (changed here and in #1277) and re-lands the older 20261026220935; whichever lands second resolves that. Its db.rs and db/tests.rs hunks don't touch this PR's code. #1164 edits another section of db/README.md, untouched here.

AI assistance: Claude Code (an AI agent) wrote the code, the migration, the tests and this description, and ran the validation above.

[github-actions]

Thanks for opening this pull request! A SPACE maintainer will take a look.

CONTRIBUTING.md explains how we review outside contributions.

Withdrawing this approval for now. Any feedback above still stands.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts