From ce150fb0ca903296cd7f26512293d1b63a4fceec Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Mon, 8 Jun 2026 22:19:21 +0300 Subject: [PATCH 1/3] docs(testing): fix stale optimize test name in maintenance.rs row (#148) The maintenance.rs row referenced `optimize_reconciles_preexisting_manifest_head_drift`, which never existed (leftover from the reconcile-drift heuristic removed in #141). The actual second test is `optimize_defers_when_recovery_sidecar_is_pending`. Co-authored-by: Claude Opus 4.8 (1M context) --- docs/dev/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev/testing.md b/docs/dev/testing.md index f18600b..8974a9f 100644 --- a/docs/dev/testing.md +++ b/docs/dev/testing.md @@ -34,7 +34,7 @@ The engine's `tests/` is the principal coverage surface; most graph-shaped behav | `s3_storage.rs` | S3-backed graph (skipped unless `OMNIGRAPH_S3_TEST_BUCKET` is set) | | `lance_version_columns.rs` | Per-row `_row_last_updated_at_version` behavior | | `validators.rs` | Schema constraint enforcement (enum, range, unique, cardinality) across JSONL, insert, update paths | -| `maintenance.rs` | `optimize` (compaction) + `cleanup` (version GC): empty/idempotent/no-op edges, policy validation, head preservation; `optimize` publishes the compacted version so the manifest tracks the Lance HEAD and a subsequent schema apply succeeds (`optimize_publishes_compaction_to_manifest_so_schema_apply_succeeds`), and reconciles a pre-existing manifest-behind-HEAD drift forged via raw Lance compaction (`optimize_reconciles_preexisting_manifest_head_drift`) | +| `maintenance.rs` | `optimize` (compaction) + `cleanup` (version GC): empty/idempotent/no-op edges, policy validation, head preservation; `optimize` publishes the compacted version so the manifest tracks the Lance HEAD and a subsequent schema apply succeeds (`optimize_publishes_compaction_to_manifest_so_schema_apply_succeeds`), and refuses to run while a `__recovery` sidecar is pending so optimize only ever operates on a recovered graph (`optimize_defers_when_recovery_sidecar_is_pending`) | | `failpoints.rs` | Failure-injection coverage (gated on `failpoints` feature). Includes the five per-writer Phase B → recovery integration tests (`recovery_rolls_forward_after_finalize_publisher_failure`, `schema_apply_phase_b_failure_recovered_on_next_open`, `branch_merge_phase_b_failure_recovered_on_next_open`, `ensure_indices_phase_b_failure_recovered_on_next_open`, `optimize_phase_b_failure_recovered_on_next_open`). | | `recovery.rs` | Open-time recovery sweep — sidecar I/O, classifier dispatch (NoMovement / RolledPastExpected / UnexpectedAtP1 / UnexpectedMultistep / InvariantViolation), all-or-nothing decision, roll-forward via `ManifestBatchPublisher::publish`, roll-back via `Dataset::restore`, audit row in `_graph_commit_recoveries.lance`, `OpenMode::ReadOnly` skip path | | `composite_flow.rs` | Compositional/narrative end-to-end stories — multi-step flows that compose mechanics covered by other test files. Catches integration regressions where individual operations all pass their unit tests but their composition breaks (sequential merges, post-merge main writes, time-travel through merge DAG, reopen consistency over multi-merge histories, post-optimize and post-cleanup strict writes). | From c2a97f4559b1e2c6e048be72630844a64d60d9aa Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Mon, 8 Jun 2026 22:25:33 +0300 Subject: [PATCH 2/3] ci: drop per-PR Windows release build; bind to release tags (#155) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `test_windows_binaries` job ran a full Windows --release build + smoke test on every code PR. It was a non-required (non-blocking) check, so it never gated a merge — it only burned the slowest/most expensive runner (windows-latest, --release, 75-min ceiling) on every code change. Windows binary validation is already covered (better) on release tags: release.yml's `smoke_windows_installer` (on v* tags) builds the release binaries, installs via scripts/install.ps1, and smoke-runs `omnigraph.exe version` + `omnigraph-server.exe --help` — the same smoke test plus the real installer path. Nothing `needs:` the removed job. Trade-off (accepted): a PR that breaks the Windows build or install.ps1 syntax is now caught at release-cut rather than at PR time. install.ps1 and platform-specific code change rarely; the cost savings on every PR outweigh the earlier signal. Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 57 ---------------------------------------- 1 file changed, 57 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b7b7b2..bbe5893 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -261,63 +261,6 @@ jobs: if: needs.classify_changes.outputs.run_full_ci == 'true' run: cargo test --locked -p omnigraph-server --features aws - test_windows_binaries: - name: Test Windows release binaries - needs: classify_changes - runs-on: windows-latest - timeout-minutes: 75 - permissions: - contents: read - env: - CARGO_TERM_COLOR: always - steps: - - name: Skip for text-only changes - if: needs.classify_changes.outputs.run_full_ci != 'true' - run: Write-Host "Text-only change detected; skipping Windows binary build." - - - name: Checkout source - if: needs.classify_changes.outputs.run_full_ci == 'true' - uses: actions/checkout@v5.0.1 - - - name: Install system dependencies - if: needs.classify_changes.outputs.run_full_ci == 'true' - run: choco install protoc -y - - - name: Install Rust stable - if: needs.classify_changes.outputs.run_full_ci == 'true' - uses: dtolnay/rust-toolchain@stable - with: - toolchain: stable - - - name: Cache Rust build data - if: needs.classify_changes.outputs.run_full_ci == 'true' - uses: Swatinem/rust-cache@v2 - with: - workspaces: | - . -> target - key: windows-release-binaries - - - name: Build Windows binaries - if: needs.classify_changes.outputs.run_full_ci == 'true' - run: cargo build --release --locked -p omnigraph-cli -p omnigraph-server - - - name: Smoke test Windows binaries - if: needs.classify_changes.outputs.run_full_ci == 'true' - run: | - & ./target/release/omnigraph.exe version - & ./target/release/omnigraph-server.exe --help - - - name: Check PowerShell installer syntax - if: needs.classify_changes.outputs.run_full_ci == 'true' - run: | - $tokens = $null - $errors = $null - [System.Management.Automation.Language.Parser]::ParseFile("scripts/install.ps1", [ref]$tokens, [ref]$errors) | Out-Null - if ($errors.Count -gt 0) { - $errors | Format-List - exit 1 - } - rustfs_integration: name: RustFS S3 Integration needs: From 5eead8d29eb6a4e7dfb453603aa0efd8e6851c47 Mon Sep 17 00:00:00 2001 From: Andrew Altshuler Date: Mon, 8 Jun 2026 22:26:04 +0300 Subject: [PATCH 3/3] ci(branch-protection): let code owners bypass required PR review (#154) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit require_code_owner_reviews + count=1 with no bypass meant EVERY PR needed a code-owner approval — including code owners' own PRs, which can't be self-approved, so an owner's PR deadlocked on the other owner (forcing admin overrides). Intended behavior: review is required only for non-owners. Add bypass_pull_request_allowances for the two engineering owners (ragnorc, aaltshuler): they merge their own PRs after CI without a second review; non-owners still require a code-owner approval. CI status checks remain required for everyone. Applied live via scripts/apply-branch-protection.sh. Note: the bypass list mirrors codeowners-roles.yml engineering members by hand (render-codeowners.py doesn't generate it) — keep in sync on owner changes. Co-authored-by: Claude Opus 4.8 (1M context) --- .github/branch-protection.json | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/branch-protection.json b/.github/branch-protection.json index 7ca46b9..c039e32 100644 --- a/.github/branch-protection.json +++ b/.github/branch-protection.json @@ -1,5 +1,5 @@ { - "_comment": "Branch protection policy for main. Applied via scripts/apply-branch-protection.sh. See docs/branch-protection.md for rationale.", + "_comment": "Branch protection policy for main. Applied via scripts/apply-branch-protection.sh. See docs/branch-protection.md for rationale. NOTE: bypass_pull_request_allowances.users must mirror the engineering owners in .github/codeowners-roles.yml — code owners merge their own PRs without a second review; non-owners still need a code-owner approval. (render-codeowners.py does NOT generate this list; keep it in sync by hand.)", "required_status_checks": { "strict": true, "contexts": [ @@ -17,7 +17,12 @@ "dismiss_stale_reviews": true, "require_code_owner_reviews": true, "required_approving_review_count": 1, - "require_last_push_approval": false + "require_last_push_approval": false, + "bypass_pull_request_allowances": { + "users": ["ragnorc", "aaltshuler"], + "teams": [], + "apps": [] + } }, "restrictions": null, "required_linear_history": true,