fix(harbor): fail-closed split access, budget/tier reconciliation, off-loop ledger flush [fixes 1/4 core]

[shehabyasser-scale]

Stacks on #3 (harbor-1-core). Addresses the core trust-boundary findings from the review of that PR. 1 of 4 fix PRs (one per stacked Varun PR).

What this fixes

#	Finding	Fix
C1	no_access gating was implicit — a split was blocked only because it was absent from the budget ledger, so reserve() raised. The engine never inspected SplitAccess.	Added resolve_split_access() in core/dataset/base.py (fail-closed: an unlisted split resolves to no_access; no vero.harbor import). EvaluationEngine.evaluate() now resolves the tier and hard-rejects no_access for non-admin callers before the ledger. Gated on split_accesses being configured, so direct-construction callers are unchanged.
C2	Policy budgets and split_accesses were two independent author lists, never reconciled — a budgeted split could be viewable (leak) or unlisted.	At init, auto-tier every agent-budgeted split to non_viewable when unlisted (evaluable, aggregate-only — no label leak, and the train_budget convenience path keeps working), then reject any budgeted split explicitly tiered viewable or no_access.
C3	BudgetLedger.reserve() held the asyncio.Lock across a synchronous write_text()+replace(), stalling the event loop (Greptile P1).	Flush now runs via asyncio.to_thread() so it no longer blocks the loop, kept under the lock so concurrent flushes can't race the shared temp file and decrement/flush ordering is preserved (a crash never persists more remaining budget than was committed).

On the architectural question ("should split access live on the dataset with a real-time check?")

SplitAccess/SplitAccessLevel already live in core/dataset/base.py; this PR adds the fail-closed resolver next to them and a real-time check in evaluate() — i.e. the engine-side embodiment of that direction. The recommended end-state (per-split tier carried on DatasetInfo, Policy.split_accesses becoming an override-only field) is a clean follow-up that doesn't change the dataset store schema and isn't needed to close these findings. Full write-up to follow in the design doc / a PR comment.

Tests

• test_dataset_access.py (new): resolver fail-closed semantics.
• test_engine.py: no_access gate fires before the ledger; admin bypasses; unlisted fails closed; non_viewable still evaluable.
• test_policy_budget_splits.py (new): auto-tier + reject viewable/no_access; train_budget default path passes.
• test_budget.py: reserve flushes durably and off the loop (to_thread), and concurrent reserves don't overspend or race the temp file.

656 passed on the full suite. The 6 failures / 8 errors present are pre-existing environment issues (litellm auth, subprocess/uv-build integration tests, a path assertion) — confirmed identical on the clean harbor-1-core baseline.

🤖 Generated with Claude Code

Greptile Summary

This PR tightens split access handling and moves durable budget flushes off the event loop. The main changes are:

• Added fail-closed split access resolution in the dataset core.
• Enforced no_access rejection in EvaluationEngine for non-admin evaluations.
• Reconciled budgeted splits with split_accesses during policy initialization.
• Moved persistent ledger flushes in async reserve() through asyncio.to_thread().
• Added tests for split access resolution, policy budget tiers, engine gating, and budget persistence.

Confidence Score: 5/5

The change appears safe to merge based on the described implementation and test coverage.

The access-control and budget-ledger changes are covered by focused tests across resolver behavior, engine enforcement, policy reconciliation, durability, and concurrent reserve behavior.

T-Rex Logs

What T-Rex did

• Compared split-access gating behavior before and after the change, and captured the before/after logs plus the probe source used for validation.
• Compared policy-budget reconcile behavior before and after the change; after, unlisted train is auto-added as non_viewable and dev budgets (viewable/no_access) are rejected with ValueError, while train_budget is accepted with train non_viewable; head-focused pytest was run and passed 5/5.
• Measured reserve timing and event-loop behavior before and after the change; the before run showed base reserve ~0.45 seconds with no slow-flush ticks, while the after run took ~0.469 seconds with ticker_count_during_slow_flush around 9, and two concurrent reserves succeeded with remaining_sample_budget staying at 8 after the reserve and 1 after the two successes.

_{Ran code and verified through T-Rex}

_{Reviews (2): Last reviewed commit: "fix(harbor): fail-closed split access, b..." | Re-trigger Greptile}

[shehabyasser-scale]

Should split access be a dataset property with a real-time check?

Yes, and this PR lands the minimal version of exactly that. The three access-related review findings are three symptoms of one root cause: access had no single owner and no real-time gate, so it was approximated by two proxies (an author split_accesses list and budget-ledger membership) resolved fail-open.

Where it stood:

• Policy.budget and Policy.split_accesses were independent lists, never reconciled (finding 1).
• tier_for_split returned viewable for any unlisted split, so a split you forgot to declare leaked full per-sample labels (finding 2).
• EvaluationEngine.evaluate() did no access check; no_access was enforced only because such splits happened to be absent from the budget ledger, so reserve() raised (finding 3). That overloads ledger membership to mean "agent-evaluable", an invariant nobody enforced.

What this PR does (the single move that subsumes all three):

1. A single fail-closed resolver, resolve_split_access(), in core/dataset/base.py next to the SplitAccess types (an unlisted split resolves to no_access, no vero.harbor import, so the engine can use it).
2. EvaluationEngine.evaluate() does the real-time check: it resolves the tier and rejects no_access for non-admin callers before metering. admin=True bypasses, so the verifier path is unaffected.
3. Policy reconciles the two lists at init: every agent-budgeted split is auto-tiered non_viewable when unlisted (evaluable, aggregate-only, no leak, and the train_budget convenience path keeps working), and a budgeted split explicitly tiered viewable or no_access is rejected.

Result: finding 1 cannot be constructed (budgeted splits are reconciled, not two free lists), finding 3 is explicit (the engine denies on an access decision, not the absence of a budget row), and finding 2 is fail-closed by construction. The sidecar's tier_for_split is flipped fail-closed in the #4 fix PR so write-routing inherits the same default.

Keep BUDGET and ACCESS as separate axes. Budget is a quota (how many evals); access is visibility (what an allowed eval returns: nothing / aggregate-only / full per-sample). They are orthogonal. The actual bug behind finding 3 was that access free-rode on budget via ledger membership; giving access its own gate fixes it without merging the axes. base.py already says the non_viewable/no_access distinction is meant to be enforced in the engine, this finishes that contract.

Trust ownership. The declared tier belongs with the dataset and scorer (sidecar-owned, baked sidecar-only by the compiler), not on the agent-adjacent Policy. The sidecar already re-materializes split_accesses at runtime, so authority is correctly sidecar-side; this only flips the default to safe and adds the real-time gate.

Flexibility (same dataset, different visibility across runs). Keep the dataset tier as a default and let the sidecar-side split_accesses (from ServeConfig) override it, with the fail-closed floor underneath. Exploration is a one-line override; a leaderboard run needs none. The override stays where the dataset is owned, never on the agent's Policy.

Deferred follow-up (not needed to close the findings): carry per-split access on DatasetInfo and persist it in the dataset store, making Policy.split_accesses / ServeConfig.split_accesses override-only. That changes the persisted dataset schema and every ad-hoc DatasetInfo construction, which is far more surface than these findings require.

One behavior change to flag: flipping the unlisted default from viewable to no_access is intentional and the safe direction; the documented default_split_accesses already lists test and validation, and the policy auto-tier keeps the common train_budget path working.