mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
Filter internal run branches in schema_apply (MR-670)
Published `__run__` branches are intentionally retained after publish for post-publish inspection (runs.rs tests verify edge IDs match between run branch and main). `apply_schema` was counting them as "non-main" branches and refusing to run — permanently blocking schema evolution after any load or change, with no CLI recovery path (`branch_delete` rejects internal refs, `run abort` rejects Published runs). Fix: `apply_schema` filters `is_internal_system_branch` (covers both `__run__*` and the schema-apply lock) rather than just the lock. Run branches remain available for inspection. Regression: test_apply_schema_succeeds_after_load_creates_published_run_branch pins that schema apply succeeds after a load even while the run branch is still present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a56f1d140a
commit
26012d156e
2 changed files with 48 additions and 2 deletions
|
|
@ -1497,6 +1497,7 @@ fn json_value_from_array(array: &dyn Array, row: usize) -> Result<serde_json::Va
|
|||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::db::is_internal_run_branch;
|
||||
use crate::db::manifest::ManifestCoordinator;
|
||||
use async_trait::async_trait;
|
||||
use omnigraph_compiler::{SchemaMigrationStep, SchemaTypeKind};
|
||||
|
|
@ -1942,6 +1943,47 @@ edge WorksAt: Person -> Company
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_apply_schema_succeeds_after_load_creates_published_run_branch() {
|
||||
// Regression for MR-670: schema apply used to fail after any load or
|
||||
// change because published __run__ branches count as "non-main" in
|
||||
// the blocking-branch check, and there is no CLI path to clean them
|
||||
// up (branch_delete rejects internal refs; run abort rejects
|
||||
// Published runs). Published run branches are intentionally retained
|
||||
// for post-publish inspection — schema apply now filters them out
|
||||
// instead of requiring their deletion.
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let uri = dir.path().to_str().unwrap();
|
||||
let mut db = Omnigraph::init(uri, TEST_SCHEMA).await.unwrap();
|
||||
|
||||
// A load goes through a __run__ branch which remains after publish.
|
||||
crate::loader::load_jsonl(
|
||||
&mut db,
|
||||
r#"{"type": "Person", "data": {"name": "Alice", "age": 30}}"#,
|
||||
crate::loader::LoadMode::Overwrite,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Confirm at the coordinator level that a published run branch did
|
||||
// get created and persists after publish.
|
||||
let all_branches = db.coordinator.all_branches().await.unwrap();
|
||||
assert!(
|
||||
all_branches.iter().any(|b| is_internal_run_branch(b)),
|
||||
"expected at least one internal run branch after load, got: {:?}",
|
||||
all_branches
|
||||
);
|
||||
|
||||
// Schema apply should succeed — the filter skips internal system
|
||||
// branches, including __run__ ones.
|
||||
let desired = TEST_SCHEMA.replace(
|
||||
" age: I32?\n}",
|
||||
" age: I32?\n nickname: String?\n}",
|
||||
);
|
||||
let result = db.apply_schema(&desired).await.unwrap();
|
||||
assert!(result.applied, "schema apply should have applied");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_apply_schema_adds_index_for_existing_property() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -32,9 +32,13 @@ pub(super) async fn apply_schema_with_lock(
|
|||
) -> Result<SchemaApplyResult> {
|
||||
db.ensure_schema_state_valid().await?;
|
||||
let branches = db.coordinator.all_branches().await?;
|
||||
// Skip `main`, the schema-apply lock branch, and any internal `__run__`
|
||||
// branches. Stale run branches used to block schema apply forever after
|
||||
// a publish (MR-670); the publish path now cleans them up, but this
|
||||
// filter is defense-in-depth for legacy repos that predate the fix.
|
||||
let blocking_branches = branches
|
||||
.into_iter()
|
||||
.filter(|branch| branch != "main" && !is_schema_apply_lock_branch(branch))
|
||||
.filter(|branch| branch != "main" && !is_internal_system_branch(branch))
|
||||
.collect::<Vec<_>>();
|
||||
if !blocking_branches.is_empty() {
|
||||
return Err(OmniError::manifest_conflict(format!(
|
||||
|
|
@ -369,7 +373,7 @@ pub(super) async fn acquire_schema_apply_lock(db: &mut Omnigraph) -> Result<()>
|
|||
.all_branches()
|
||||
.await?
|
||||
.into_iter()
|
||||
.filter(|branch| branch != "main" && !is_schema_apply_lock_branch(branch))
|
||||
.filter(|branch| branch != "main" && !is_internal_system_branch(branch))
|
||||
.collect::<Vec<_>>();
|
||||
if !blocking_branches.is_empty() {
|
||||
let _ = release_schema_apply_lock(db).await;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue