From 5f7b033d03c4c9cbb207af4fc3e508d362acbe47 Mon Sep 17 00:00:00 2001 From: Mengye Ren Date: Thu, 1 Oct 2026 00:00:26 -0400 Subject: [PATCH 1/2] Author overrides can give an author longer sessions Optional session_minutes (10-240) and session_max_turns (10-300) on an OUTERLOOP_AUTHOR_OVERRIDES entry, for authors on slower self-hosted models. Operator-only: a contract can still lower them. The limits bind at claim into the run record, every session path of the run uses them, and the job walltime follows the session within the operator job cap. Judges are unchanged, and settings without the fields behave as before. --- CHANGELOG.md | 9 ++ docs/install.md | 22 ++++- src/outerloop/attempt.py | 45 +++++++++ src/outerloop/author_overrides.py | 36 ++++++- src/outerloop/limits.py | 79 ++++++++++++++- src/outerloop/runstate.py | 3 + src/outerloop/tick.py | 64 ++++++++++--- tests/test_author_overrides.py | 153 +++++++++++++++++++++++++++++- tests/test_limits.py | 82 ++++++++++++++++ 9 files changed, 467 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bc18bae..46428f47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ Versions follow [SemVer](https://semver.org). ## [Unreleased] +- Author overrides accept operator-only `session_minutes` (10–240) and + `session_max_turns` (10–300). Limits bind with the author selection and survive + settings changes across wakes and review replies. Contracts can still lower + budgets; job walltime follows session duration and the operator job cap. + Judge budgets are unchanged. +- Upgrading: the new override fields are optional; existing settings and legacy + run records need no migration. Finish runs using extended limits before + rolling back to a version without bound author limits. + - Startup validation of `OUTERLOOP_AUTHOR_OVERRIDES` (the tick and `outerloop start`) uses the image sessions actually run with, the default image when `OUTERLOOP_IMAGE` is unset. Before, a codex override on a deployment without `OUTERLOOP_IMAGE` failed validation and stopped the tick. diff --git a/docs/install.md b/docs/install.md index b2b4b1fc..7d39ff94 100644 --- a/docs/install.md +++ b/docs/install.md @@ -607,6 +607,20 @@ appear twice: OUTERLOOP_AUTHOR_OVERRIDES='{"owner/repo":[{"backend":"claude","model":"served-model[endpoint=onprem]","slots":["agent-04"]},{"backend":"codex","model":"served-model[endpoint=onprem]","slots":["agent-03"]}]}' ``` +Either form also accepts optional integer `session_minutes` (10–240) and +`session_max_turns` (10–300) on each entry, for authors that need longer sessions. +For example, add `"session_minutes":180,"session_max_turns":250` to an entry. +Values outside these ranges fail startup validation. Only operator settings can +raise these limits; a contract's explicit session budget can still lower them. +The session uses the smaller of the contract value and the override, subject to +the existing floors. An overridden session duration gets a job budget of that +duration plus 20 minutes of overhead; an explicit contract job budget can lower +it. Panel work keeps its additional allowance. `OUTERLOOP_MAX_JOB_MINUTES` still +caps the job and shortens the session when needed to leave overhead; at the cap, +the panel allowance is what gets cut. A run keeps the limits it was claimed +with, so its wake and review-reply jobs are sized the same way and are longer +than a default run's. + This is deployment configuration, not a contract setting. The setting is parsed and validated at startup. Endpoint overrides select their own profile in `model`; they do not inherit `OUTERLOOP_AUTHOR_ENDPOINT`. Native overrides use @@ -614,9 +628,11 @@ the selected backend's author credential. Normal author/judge credential separation still applies to the effective override credential. Queued climbs bind the selection when submitted (before an intake claim is -launched); direct climbs bind at startup. Backend, model and key path are saved -in the run record. Changing overrides cannot switch an existing run, even at a -resume or wake. Panel inheritance and CI reviewers still use the fleet author, +launched); direct climbs bind at startup. Backend, model, key path and resolved +override limits are saved in the run record. Bound limits also apply to author-sleep wakes, resumed legs +and author replies to reviews; judge budgets are unchanged. Entries without the +new fields and older records retain their existing limits. Changing overrides +cannot switch an existing run, even at a resume or wake. Panel inheritance and CI reviewers still use the fleet author, exactly as for a run without an override. Per-run board details show the author backend/model and mark overrides. Remove the setting (or use `{}`) to stop selecting overrides for new work. diff --git a/src/outerloop/attempt.py b/src/outerloop/attempt.py index 31898ed6..f850bb62 100644 --- a/src/outerloop/attempt.py +++ b/src/outerloop/attempt.py @@ -83,6 +83,7 @@ observe_target, queue_pending, ) +from outerloop.limits import bound_limits, capped_limits, clamp_bound_limits, effective_limits from outerloop.markers import has_marker, marker from outerloop.measure import DispatchedMeasurer, DispatchSettings from outerloop.orchestrator import ( @@ -4388,6 +4389,7 @@ def live_attempt( author_model: str = "", author_key_file: str = "", author_overridden: bool = False, + author_limits: dict[str, int] | None = None, task_hypothesis: str = "", spec: RoleSpec | None = None, panel_lenses: tuple[PanelLens, ...] = (), @@ -4418,6 +4420,7 @@ def live_attempt( author_backend=author_backend, author_model=author_model, author_overridden=author_overridden, + author_limits=author_limits, author_key_file=author_key_file, run_job_id=_os.environ.get("SLURM_JOB_ID", ""), stage={"hypothesis": redact(task_hypothesis, secrets)} if task_hypothesis else {}, @@ -4459,6 +4462,23 @@ def live_attempt( _exclude_merge_artifacts(workspace) contract_text = contract_text_in_tree(workspace) contract = load_contract(contract_text, config.target) + if (stored := bound_limits(record.author_limits)) is not None: + # Direct climbs learn the trusted contract only after cloning. Queued + # climbs already carry its clamp; neither path rereads operator settings. + bound = clamp_bound_limits(stored, contract.budgets) + record = dc_replace(record, author_limits=asdict(bound)) + save_record(run_root, record, now) + spec = author_spec( + max_turns=bound.session_max_turns, walltime_s=bound.session_minutes * 60 + ) + from outerloop.harness import ClaudeCodeHarness, CodexHarness, HermesHarness + + if isinstance(harness, (ClaudeCodeHarness, HermesHarness)): + harness = dc_replace( + harness, max_turns=spec.budget.max_turns, timeout_s=spec.budget.walltime_s + ) + elif isinstance(harness, CodexHarness): + harness = dc_replace(harness, timeout_s=spec.budget.walltime_s) # Load the brief budget from the contract and run state: callers do # not supply it (the dataclass default rendered "0.0 GPU-hours" and # honest agents refused to launch). Same weekly counting rule as the @@ -5157,6 +5177,7 @@ def _run_id(value: str) -> str: ) parser.add_argument("--author-bound", action="store_true", help=argparse.SUPPRESS) parser.add_argument("--author-overridden", action="store_true", help=argparse.SUPPRESS) + parser.add_argument("--author-limits", type=json.loads, default=None, help=argparse.SUPPRESS) args = parser.parse_args() from outerloop.author_overrides import select_override from outerloop.gpu_lanes import gpu_lanes_from_env @@ -5168,6 +5189,23 @@ def _run_id(value: str) -> str: if selected: args.author_backend, args.model = selected.backend, selected.resolved_model() args.author_overridden = True + if selected.session_minutes is not None or selected.session_max_turns is not None: + from outerloop.tick import _max_job_minutes_from_env + + limits = effective_limits( + session_minutes=selected.session_minutes, + session_max_turns=selected.session_max_turns, + ) + if args.job_minutes: # 0 keeps the self-deadline off + args.job_minutes = min(args.job_minutes, _max_job_minutes_from_env()) + limits = capped_limits(limits, args.job_minutes) + args.author_limits = asdict(limits) + if args.author_limits is not None and not args.resume: + passed = bound_limits(args.author_limits) + if passed is None: + raise ValueError("--author-limits must carry every session limit") + args.max_turns = passed.session_max_turns + args.session_minutes = passed.session_minutes except ValueError as exc: parser.error(str(exc)) logging.basicConfig(level=logging.INFO, format="%(asctime)s %(message)s") @@ -5224,6 +5262,12 @@ def _run_id(value: str) -> str: # a wake must never crash on an unreadable/odd record — fall back to # the claude author (resume_author), same fail-safe as the sweep _wake_record = None + wake_limits = bound_limits(getattr(_wake_record, "author_limits", None)) + if wake_limits is not None: + if args.job_minutes: + wake_limits = capped_limits(wake_limits, args.job_minutes) + args.max_turns = wake_limits.session_max_turns + args.session_minutes = wake_limits.session_minutes try: explicit_model = args.model # what the operator typed, before any env fill-in if not getattr(_wake_record, "author_model", "") and not args.model: @@ -5475,6 +5519,7 @@ def _run_id(value: str) -> str: author_backend=args.author_backend, author_model=args.model, author_overridden=args.author_overridden, + author_limits=args.author_limits, author_key_file=args.key_file, task_hypothesis=( __import__("base64").b64decode(args.hypothesis_b64).decode() diff --git a/src/outerloop/author_overrides.py b/src/outerloop/author_overrides.py index a4dcb594..70f6636c 100644 --- a/src/outerloop/author_overrides.py +++ b/src/outerloop/author_overrides.py @@ -10,6 +10,12 @@ from functools import lru_cache from types import MappingProxyType +from outerloop.limits import ( + OVERRIDE_SESSION_MINUTES_CEILING, + OVERRIDE_SESSION_TURNS_CEILING, + SESSION_MINUTES_FLOOR, +) + SETTING = "OUTERLOOP_AUTHOR_OVERRIDES" @@ -19,6 +25,9 @@ class AuthorOverride: model: str slots: tuple[str, ...] | None = None + session_minutes: int | None = None + session_max_turns: int | None = None + def resolved_model(self) -> str: """Bind profile model defaults without inheriting the fleet endpoint.""" from outerloop.endpoints import resolve_endpoint @@ -58,8 +67,17 @@ def parse_overrides(raw: str) -> Mapping[str, tuple[AuthorOverride, ...]]: def _parse_one(target: str, value: object, listed: bool) -> AuthorOverride: - if not isinstance(value, dict) or set(value) - {"backend", "model", "slots"}: - raise ValueError(f"{target}: expected backend, model and optional slots") + if not isinstance(value, dict) or set(value) - { + "backend", + "model", + "slots", + "session_minutes", + "session_max_turns", + }: + raise ValueError( + f"{target}: expected backend, model and optional slots, " + "session_minutes, session_max_turns" + ) backend, model, slots = value.get("backend"), value.get("model"), value.get("slots") if backend not in ("claude", "codex", "hermes"): raise ValueError(f"{target}: backend must be claude, codex or hermes") @@ -88,7 +106,19 @@ def _parse_one(target: str, value: object, listed: bool) -> AuthorOverride: raise ValueError(f"{target}: slots must be distinct agent identities (agent-01, ...)") if listed and slots is None: raise ValueError(f"{target}: each override in a list must name its slots") - return AuthorOverride(backend, model, None if slots is None else tuple(slots)) + for name, floor, ceiling in ( + ("session_minutes", SESSION_MINUTES_FLOOR, OVERRIDE_SESSION_MINUTES_CEILING), + ("session_max_turns", 10, OVERRIDE_SESSION_TURNS_CEILING), + ): + if name in value and (type(value[name]) is not int or not floor <= value[name] <= ceiling): + raise ValueError(f"{target}: {name} must be an integer between {floor} and {ceiling}") + return AuthorOverride( + backend, + model, + None if slots is None else tuple(slots), + value.get("session_minutes"), + value.get("session_max_turns"), + ) def overrides( diff --git a/src/outerloop/limits.py b/src/outerloop/limits.py index 46d1a493..5981d280 100644 --- a/src/outerloop/limits.py +++ b/src/outerloop/limits.py @@ -5,13 +5,13 @@ — shorter sessions, tighter job walltimes — but must never be able to raise it: every contract value is clamped into [floor, ceiling], and the ceilings are code on the orchestrator side, not configuration a target -can reach. Absent values fall back to the defaults the pilot has run with -all along. +can reach. Validated operator author overrides may raise the session ceilings. +Absent values fall back to the defaults the pilot has run with all along. """ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import asdict, dataclass, replace from typing import Any # (default, floor, ceiling) per knob. Floors keep a hostile-or-typo'd @@ -45,6 +45,10 @@ # the tick's cap warning uses it as the no-runway threshold too. ATTEMPT_OVERHEAD_MINUTES = 20 +# Operator author overrides may raise sessions up to these, never further. +OVERRIDE_SESSION_MINUTES_CEILING = 240 +OVERRIDE_SESSION_TURNS_CEILING = 300 + @dataclass(frozen=True) class EffectiveLimits: @@ -61,11 +65,15 @@ def _clamp(name: str, value: int | None) -> int: return max(floor, min(int(value), ceiling)) -def effective_limits(budgets: Any = None) -> EffectiveLimits: +def effective_limits( + budgets: Any = None, *, session_minutes: int | None = None, session_max_turns: int | None = None +) -> EffectiveLimits: """Resolve a contract's optional budget knobs into enforceable limits. `budgets` is the contract's Budgets model (or None for pure defaults); - unknown/absent attributes read as None. The session is finally shrunk + unknown/absent attributes read as None. Optional session ceilings come only + from validated operator overrides (or their persisted binding), never the + contract. The session is finally shrunk to fit inside the climb job with room for the orchestrator's overhead — a session that outlives its job ends as a kill, not a report. """ @@ -73,8 +81,69 @@ def effective_limits(budgets: Any = None) -> EffectiveLimits: name: _clamp(name, getattr(budgets, name, None) if budgets is not None else None) for name in _BOUNDS } + for name, override in ( + ("session_minutes", session_minutes), + ("session_max_turns", session_max_turns), + ): + if override is not None: + requested = getattr(budgets, name, None) + values[name] = max( + _BOUNDS[name][1], min(requested, override) if requested is not None else override + ) + if session_minutes is not None: + wanted = values["session_minutes"] + ATTEMPT_OVERHEAD_MINUTES + requested_job = getattr(budgets, "attempt_job_minutes", None) + values["attempt_job_minutes"] = ( + min(wanted, max(ATTEMPT_JOB_MINUTES_FLOOR, requested_job)) + if requested_job is not None + else wanted + ) max_session = values["attempt_job_minutes"] - ATTEMPT_OVERHEAD_MINUTES if values["session_minutes"] > max_session: floor = _BOUNDS["session_minutes"][1] values["session_minutes"] = max(floor, max_session) return EffectiveLimits(**values) + + +def capped_limits(limits: EffectiveLimits, job_minutes: int) -> EffectiveLimits: + """Shrink the author session to leave overhead inside the actual job.""" + return replace( + limits, + session_minutes=max( + SESSION_MINUTES_FLOOR, + min(limits.session_minutes, job_minutes - ATTEMPT_OVERHEAD_MINUTES), + ), + ) + + +def clamp_bound_limits(limits: EffectiveLimits, budgets: Any) -> EffectiveLimits: + """Apply a newly loaded trusted contract without raising a bound budget.""" + values = asdict(limits) + for name in values: + requested = getattr(budgets, name, None) + if requested is not None: + values[name] = min(values[name], max(_BOUNDS[name][1], requested)) + if values["session_minutes"] < limits.session_minutes: + values["attempt_job_minutes"] = min( + values["attempt_job_minutes"], values["session_minutes"] + ATTEMPT_OVERHEAD_MINUTES + ) + return capped_limits(EffectiveLimits(**values), values["attempt_job_minutes"]) + + +def bound_limits(value: Any) -> EffectiveLimits | None: + """A run's persisted limits, or None (today's defaults) when absent or unreadable.""" + if not isinstance(value, dict): + return None + try: + values = {name: int(value[name]) for name in _BOUNDS} + except (KeyError, TypeError, ValueError): + return None + ceilings = { + **{name: ceiling for name, (_, _, ceiling) in _BOUNDS.items()}, + "session_minutes": OVERRIDE_SESSION_MINUTES_CEILING, + "session_max_turns": OVERRIDE_SESSION_TURNS_CEILING, + "attempt_job_minutes": OVERRIDE_SESSION_MINUTES_CEILING + ATTEMPT_OVERHEAD_MINUTES, + } + for name, (_, floor, _) in _BOUNDS.items(): + values[name] = max(floor, min(values[name], ceilings[name])) + return EffectiveLimits(**values) diff --git a/src/outerloop/runstate.py b/src/outerloop/runstate.py index 086c2e2a..3e6ee75e 100644 --- a/src/outerloop/runstate.py +++ b/src/outerloop/runstate.py @@ -137,6 +137,7 @@ class RunRecord: # the harness strips it only when constructing the backend session. author_backend: str = "" author_model: str = "" + author_limits: dict[str, int] | None = None # bound operator session budgets author_overridden: bool = False # judges inherit the fleet author for this run # The resolved author key FILE PATH (not the key) this run used, so a wake or # follow-up reproduces the exact key — an explicit --key-file survives, and an @@ -265,6 +266,8 @@ def _save_record(root: Path, record: RunRecord, now: float) -> None: # same tmp file before the atomic replace tmp = directory / f".{RECORD_NAME}.{os.getpid()}.tmp" payload = asdict(stamped) + if stamped.author_limits is None: + payload.pop("author_limits") if not stamped.author_overridden: payload.pop("author_overridden") # No-setting records retain their exact wire shape. path = directory / RECORD_NAME diff --git a/src/outerloop/tick.py b/src/outerloop/tick.py index b407544c..9f333a8d 100644 --- a/src/outerloop/tick.py +++ b/src/outerloop/tick.py @@ -46,7 +46,7 @@ from outerloop.housekeeping import shed_ended_workspaces from outerloop.job_names import run_job_name from outerloop.ledger_branch import RESEARCH_LOG_BRANCH as RESEARCH_LOG_BRANCH -from outerloop.limits import EffectiveLimits, effective_limits +from outerloop.limits import EffectiveLimits, bound_limits, capped_limits, effective_limits from outerloop.markers import has_marker, marker from outerloop.operator_limits import ( attempt_width, @@ -2446,7 +2446,26 @@ def _selected_author(spec: ServiceSpec, agent_id: str = "agent-01") -> tuple[str return backend, fleet_author_model(backend) -def _climb_author_argv(spec: ServiceSpec, agent_id: str = "agent-01") -> list[str]: +def _selected_author_limits( + spec: ServiceSpec, agent_id: str = "agent-01", budgets: Any = None +) -> EffectiveLimits | None: + from outerloop.author_overrides import select_override + + selected = select_override(spec.target, agent_id) + if selected is None or ( + selected.session_minutes is None and selected.session_max_turns is None + ): + return None + return effective_limits( + budgets, + session_minutes=selected.session_minutes, + session_max_turns=selected.session_max_turns, + ) + + +def _climb_author_argv( + spec: ServiceSpec, agent_id: str = "agent-01", limits: EffectiveLimits | None = None +) -> list[str]: from outerloop.attempt import effective_author_credential from outerloop.author_overrides import overrides, select_override @@ -2454,7 +2473,15 @@ def _climb_author_argv(spec: ServiceSpec, agent_id: str = "agent-01") -> list[st return [] backend, model = _selected_author(spec, agent_id) credential = effective_author_credential(backend, model) + selected_limits = _selected_author_limits(spec, agent_id) + limit_argv = [] + if selected_limits is not None: + from dataclasses import asdict + + bound = capped_limits(limits or selected_limits, spec.max_job_minutes) + limit_argv = ["--author-limits", json.dumps(asdict(bound))] return [ + *limit_argv, "--author-bound", "--author-backend", backend, @@ -2725,14 +2752,12 @@ def _climb_limit_argv(limits: EffectiveLimits, job_minutes: int) -> list[str]: CAPPED job with the same rule limits.effective_limits applies to contract values — better a short session that ends cleanly than a full one the self-deadline kills mid-flight.""" - from outerloop.limits import ATTEMPT_OVERHEAD_MINUTES, SESSION_MINUTES_FLOOR - - session = min(limits.session_minutes, job_minutes - ATTEMPT_OVERHEAD_MINUTES) + session = capped_limits(limits, job_minutes).session_minutes return [ "--max-turns", str(limits.session_max_turns), "--session-minutes", - str(max(SESSION_MINUTES_FLOOR, session)), + str(session), "--job-minutes", str(job_minutes), ] @@ -2857,6 +2882,7 @@ def service_self_initiated( return None if dry_run: return (benchmark, "dry-run") + limits = _selected_author_limits(spec, slot_agent, contract.budgets) or limits job_minutes = _attempt_job_minutes(spec, limits) argv = [ *_interpreter(spec.home), @@ -2873,7 +2899,7 @@ def service_self_initiated( slot_agent, *_climb_limit_argv(limits, job_minutes), *_climb_panel_argv(spec), - *_climb_author_argv(spec, slot_agent), + *_climb_author_argv(spec, slot_agent, limits), ] if spec.pat_file: argv += ["--pat-file", spec.pat_file] @@ -3135,8 +3161,9 @@ def service_intake( return None if dry_run: return (f"issue-{task.number}", "dry-run") + limits = _selected_author_limits(spec, budgets=contract.budgets) or limits job_minutes = _attempt_job_minutes(spec, limits) - author_argv = _climb_author_argv(spec) + author_argv = _climb_author_argv(spec, limits=limits) # claim BEFORE submit: Slurm queueing can take minutes, and the next # tick must not re-claim the same issue in that window from outerloop.intake import CLAIM_MARKER, MAX_INTAKE_ATTEMPTS, RELEASE_MARKER @@ -3258,8 +3285,11 @@ def dispatch(self, record: RunRecord, reason: str) -> str: *_climb_panel_argv(self.spec), # session budget for the depth-axis REVISION (a blocking panel # finding wakes the author to revise). - "--max-turns", - str(self.spec.max_turns), + *( + ["--max-turns", str(self.spec.max_turns)] + if bound_limits(record.author_limits) is None + else [] + ), ] panel_skip = ( _panel_preflight_error(self.spec, record=record) if self.spec.panel.strip() else "" @@ -3276,7 +3306,19 @@ def dispatch(self, record: RunRecord, reason: str) -> str: from outerloop.limits import ATTEMPT_OVERHEAD_MINUTES from outerloop.roles import author_spec - if record.pr_url: + if (limits := bound_limits(record.author_limits)) is not None: + if record.pr_url or record.stage.get("phase") == "author-sleep": + job_minutes = _attempt_job_minutes(self.spec, limits) + else: + from outerloop.panel import panel_read_minutes + + allowance = panel_read_minutes(self.spec.panel) + job_minutes = min( + self.wake_minutes + allowance + limits.session_minutes, + self.spec.max_job_minutes, + ) + argv += _climb_limit_argv(limits, job_minutes) + elif record.pr_url: job_minutes = min(self.spec.time_minutes, self.spec.max_job_minutes) session_minutes = max(1, job_minutes - ATTEMPT_OVERHEAD_MINUTES) argv += ["--session-minutes", str(session_minutes)] diff --git a/tests/test_author_overrides.py b/tests/test_author_overrides.py index 08921383..11b12c34 100644 --- a/tests/test_author_overrides.py +++ b/tests/test_author_overrides.py @@ -10,6 +10,7 @@ from outerloop import attempt from outerloop.author_overrides import parse_overrides, select_override +from outerloop.compute import Compute from outerloop.runstate import RunRecord, load_record, save_record from outerloop.tick import ServiceSpec, _climb_author_argv, _panel_preflight_error @@ -127,7 +128,20 @@ def panel_args(spec, **kwargs): ) -def test_panel_independence(deployment): +def test_panel_independence(deployment, monkeypatch): + monkeypatch.setenv( + "OUTERLOOP_AUTHOR_OVERRIDES", + json.dumps( + { + "owner/repo": { + "backend": "codex", + "model": "served-model[endpoint=onprem]", + "session_minutes": 240, + "session_max_turns": 300, + } + } + ), + ) ordinary, secrets = attempt._panel_lenses_from_args( panel_args(deployment, author_backend="claude", model="claude-fleet") ) @@ -137,6 +151,8 @@ def test_panel_independence(deployment): author_backend="codex", model="served-model[endpoint=onprem]", author_overridden=True, + session_minutes=240, + max_turns=300, ) ) assert [(x.kind, x.harness) for x in ordinary] == [(x.kind, x.harness) for x in overridden] @@ -166,7 +182,8 @@ def test_override_credential_separation(deployment, same_path): @pytest.mark.parametrize("backend", ["claude", "codex", "hermes"]) @pytest.mark.parametrize("queued", [False, True]) -def test_binding_survives_setting_change(deployment, monkeypatch, backend, queued): +@pytest.mark.parametrize("long_session", [False, True]) +def test_binding_survives_setting_change(deployment, monkeypatch, backend, queued, long_session): monkeypatch.setenv( "OUTERLOOP_AUTHOR_OVERRIDES", json.dumps( @@ -175,6 +192,7 @@ def test_binding_survives_setting_change(deployment, monkeypatch, backend, queue "backend": backend, "model": "served-model[endpoint=onprem]", "slots": ["agent-05"], + **({"session_minutes": 180, "session_max_turns": 250} if long_session else {}), } } ), @@ -186,12 +204,16 @@ def test_binding_survives_setting_change(deployment, monkeypatch, backend, queue monkeypatch.setenv( "OUTERLOOP_AUTHOR_OVERRIDES", '{"owner/repo":{"backend":"claude","model":"claude-new"}}' ) - seen = {} + seen: dict[str, Any] = {} monkeypatch.setattr( attempt, "resolve_bot_auth", lambda *a: SimpleNamespace(token=lambda: "bot") ) monkeypatch.setattr(attempt, "_dispatch_settings", lambda *a: None) - monkeypatch.setattr(attempt, "build_harness", lambda *a, **kw: seen.update(kw) or object()) + monkeypatch.setattr( + attempt, + "build_harness", + lambda *a, **kw: seen.update(kw, spec=a[1]) or object(), + ) real_launch = attempt.live_attempt @@ -236,6 +258,12 @@ def launch(**kw): assert (seen["backend"], seen["model"]) == (backend, "served-model[endpoint=onprem]") record = load_record(deployment.run_root, seen["record"].run_id) assert record.author_overridden + if long_session: + assert record.author_limits is not None + assert record.author_limits["session_minutes"] == 180 + assert record.author_limits["session_max_turns"] == 250 + else: + assert record.author_limits is None assert attempt.resume_author(record, "claude-new", "claude")[:2] == ( backend, "served-model[endpoint=onprem]", @@ -266,6 +294,10 @@ def launch(**kw): assert attempt.main() == 0 assert (seen["backend"], seen["model"]) == (backend, "served-model[endpoint=onprem]") + if long_session: + assert seen["spec"].budget.walltime_s == 180 * 60 + assert seen["spec"].budget.max_turns == 250 + def test_no_setting_keeps_launch_bytes(deployment, monkeypatch): monkeypatch.delenv("OUTERLOOP_AUTHOR_OVERRIDES") @@ -285,6 +317,7 @@ def test_legacy_record_tolerated_idempotently(tmp_path, state): (directory / "state.json").write_text(json.dumps(data)) for _ in range(3): # first read, idempotent pass, interrupted writer retry record = load_record(tmp_path, data["run_id"]) + assert record.author_limits is None assert not record.author_overridden assert attempt.resume_author(record, "other", "claude") == ( "codex", @@ -521,3 +554,115 @@ def test_start_validates_with_the_launched_tick_image( with contextlib.suppress(SystemExit): cli.main(["start", "--dry-run", "--root", str(tmp_path / "state")]) assert seen and seen[0] == expected + + +@pytest.mark.parametrize("listed", [False, True]) +@pytest.mark.parametrize("name,ceiling", [("session_minutes", 240), ("session_max_turns", 300)]) +@pytest.mark.parametrize("value", [None, True, "120", 120.5, 9, 0, -1, 301]) +def test_invalid_session_limits_at_startup(listed, name, ceiling, value): + from outerloop.author_overrides import validate_overrides + + entry = {"backend": "claude", "model": "served-model", "slots": ["agent-01"], name: value} + raw = json.dumps({"owner/repo": [entry] if listed else entry}) + with pytest.raises(ValueError, match=f"{name} must be an integer between 10 and {ceiling}"): + validate_overrides({"OUTERLOOP_AUTHOR_OVERRIDES": raw}, "image.sif") + + +@pytest.mark.parametrize("listed", [False, True]) +@pytest.mark.parametrize("minutes,turns", [(10, 10), (240, 300)]) +def test_session_limit_boundaries(listed, minutes, turns, monkeypatch): + from outerloop.author_overrides import validate_overrides + + entry = { + "backend": "claude", + "model": "served-model", + "slots": ["agent-01"], + "session_minutes": minutes, + "session_max_turns": turns, + } + raw = json.dumps({"owner/repo": [entry] if listed else entry}) + monkeypatch.setattr(attempt, "author_config_error", lambda *a, **kw: "") + validate_overrides({"OUTERLOOP_AUTHOR_OVERRIDES": raw}, "image.sif") + selected = parse_overrides(raw)["owner/repo"][0] + assert (selected.session_minutes, selected.session_max_turns) == (minutes, turns) + + +@pytest.mark.parametrize("phase", ["author-sleep", "candidate", "reply"]) +@pytest.mark.parametrize("cap", [360, 100]) +def test_bound_wake_limits(deployment, monkeypatch, phase, cap): + from dataclasses import asdict + + from outerloop.limits import effective_limits + from outerloop.tick import JobWakeDispatcher + + jobs = [] + + def submit(job): + jobs.append(job) + return "123" + + compute = cast(Compute, SimpleNamespace(submit=submit)) + spec = replace(deployment, panel="", max_job_minutes=cap) + record = RunRecord( + run_id="long-session", + target=spec.target, + task_title="trial", + state="parked", + stage={"phase": phase}, + pr_url="https://github.com/owner/repo/pull/1" if phase == "reply" else "", + author_limits=asdict(effective_limits(session_minutes=180, session_max_turns=250)), + ) + monkeypatch.setenv("OUTERLOOP_AUTHOR_OVERRIDES", "{}") + JobWakeDispatcher(compute, spec, 1).dispatch(record, "ready") + job = jobs[0] + assert job.time_minutes == min(200, cap) + assert f"--session-minutes {min(180, cap - 20)}" in job.command + assert "--max-turns 250" in job.command + assert f"--job-minutes {job.time_minutes}" in job.command + + +@pytest.mark.parametrize("cap", [360, 100]) +def test_fresh_climb_uses_slot_limits(deployment, monkeypatch, cap): + from outerloop.contract import load_contract + from outerloop.tick import service_self_initiated + + monkeypatch.setenv( + "OUTERLOOP_AUTHOR_OVERRIDES", + json.dumps( + { + "owner/repo": { + "backend": "claude", + "model": "served-model[endpoint=onprem]", + "slots": ["agent-01"], + "session_minutes": 180, + "session_max_turns": 250, + } + } + ), + ) + contract = load_contract( + """ +benchmarks: + - {name: bench, command: c, metric: m, direction: min} +budgets: {gpu_hours_per_run: 1, runs_per_week: 3} +scope: {allowed: [src/]} +roadmap: docs/roadmap.md +""", + "owner/repo", + ) + jobs = [] + + def submit(job): + jobs.append(job) + return "123" + + compute = cast(Compute, SimpleNamespace(submit=submit)) + spec = replace(deployment, panel="", max_job_minutes=cap) + assert service_self_initiated(spec.run_root, compute, spec, contract, 1_000_000) == ( + "bench", + "123", + ) + assert jobs[0].time_minutes == min(200, cap) + assert f"--session-minutes {min(180, cap - 20)}" in jobs[0].command + assert "--max-turns 250" in jobs[0].command + assert "--author-limits" in jobs[0].command diff --git a/tests/test_limits.py b/tests/test_limits.py index d163d6b0..1300dbc5 100644 --- a/tests/test_limits.py +++ b/tests/test_limits.py @@ -69,3 +69,85 @@ def test_contract_rejects_nonpositive_knobs() -> None: # pydantic surfaces schema violations as ValueError subclasses with pytest.raises(ValueError): load_contract(BASE % ", session_minutes: 0", "org/pilot") + + +def test_operator_session_limits_and_contract_clamp(): + from types import SimpleNamespace + + assert effective_limits(session_minutes=240, session_max_turns=300) == EffectiveLimits( + 300, 240, 260, 90 + ) + for requested, minutes, turns in [ + (None, 180, 250), + (150, 150, 150), + (999, 180, 250), + (1, 10, 10), + ]: + limits = effective_limits( + SimpleNamespace(session_minutes=requested, session_max_turns=requested), + session_minutes=180, + session_max_turns=250, + ) + assert limits == EffectiveLimits(turns, minutes, minutes + 20, 90) + assert effective_limits(session_max_turns=300) == EffectiveLimits(300, 90, 120, 90) + assert effective_limits(session_minutes=180) == EffectiveLimits(120, 180, 200, 90) + + +def test_contract_job_budget_still_lowers_an_overridden_session(): + from types import SimpleNamespace + + limits = effective_limits(SimpleNamespace(attempt_job_minutes=60), session_minutes=180) + assert limits.session_minutes == 40 + assert limits.attempt_job_minutes == 60 + for requested, expected in [(150, 150), (999, 200)]: + limits = effective_limits( + SimpleNamespace(attempt_job_minutes=requested), session_minutes=180 + ) + assert limits.attempt_job_minutes == expected + assert limits.session_minutes == expected - 20 + + +def test_contract_discovered_after_binding_cannot_raise_limits(): + from types import SimpleNamespace + + from outerloop.limits import clamp_bound_limits + + for bound in [effective_limits(session_max_turns=300), effective_limits(session_minutes=180)]: + assert clamp_bound_limits(bound, None) == bound + assert ( + clamp_bound_limits(bound, SimpleNamespace(session_minutes=999, session_max_turns=999)) + == bound + ) + bound = effective_limits(session_minutes=180, session_max_turns=250) + assert clamp_bound_limits( + bound, SimpleNamespace(session_minutes=60, session_max_turns=30) + ) == EffectiveLimits(30, 60, 80, 90) + assert clamp_bound_limits(bound, SimpleNamespace(attempt_job_minutes=60)) == EffectiveLimits( + 250, 40, 60, 90 + ) + + +def test_bound_limits_tolerates_damaged_records(): + from outerloop.limits import bound_limits, effective_limits + + assert bound_limits(None) is None + assert bound_limits({"session_minutes": 180}) is None + assert bound_limits("180") is None + full = {**effective_limits().__dict__, "session_minutes": 180, "future_knob": 1} + kept, floored = bound_limits(full), bound_limits({**full, "session_max_turns": 0}) + assert kept is not None and kept.session_minutes == 180 + assert floored is not None and floored.session_max_turns == 10 + + +def test_bound_limits_caps_at_the_override_ceilings(): + from outerloop.limits import bound_limits, effective_limits + + huge = {**effective_limits().__dict__, "session_minutes": 10**9, "session_max_turns": 10**9} + huge["attempt_job_minutes"] = 10**9 + capped = bound_limits(huge) + assert capped is not None + assert (capped.session_minutes, capped.session_max_turns, capped.attempt_job_minutes) == ( + 240, + 300, + 260, + ) From 7a38873b544153bfd4a3167a15b84dbf5750995e Mon Sep 17 00:00:00 2001 From: Mengye Ren Date: Thu, 1 Oct 2026 00:16:26 -0400 Subject: [PATCH 2/2] Session limits: read stored limits strictly, test the ceiling+1 boundary, document Codex's lack of a turn cap --- docs/install.md | 5 +++-- src/outerloop/limits.py | 10 ++++++---- tests/test_author_overrides.py | 4 +++- tests/test_limits.py | 3 +++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/docs/install.md b/docs/install.md index 7d39ff94..ac9a55b5 100644 --- a/docs/install.md +++ b/docs/install.md @@ -618,8 +618,9 @@ duration plus 20 minutes of overhead; an explicit contract job budget can lower it. Panel work keeps its additional allowance. `OUTERLOOP_MAX_JOB_MINUTES` still caps the job and shortens the session when needed to leave overhead; at the cap, the panel allowance is what gets cut. A run keeps the limits it was claimed -with, so its wake and review-reply jobs are sized the same way and are longer -than a default run's. +with, and its wake and review-reply jobs are sized from those limits too. +Codex has no turn cap, so `session_max_turns` applies to Claude Code and Hermes +authors only; Codex sessions are bounded by `session_minutes`. This is deployment configuration, not a contract setting. The setting is parsed and validated at startup. Endpoint overrides select their own profile in diff --git a/src/outerloop/limits.py b/src/outerloop/limits.py index 5981d280..8aebd1e5 100644 --- a/src/outerloop/limits.py +++ b/src/outerloop/limits.py @@ -134,10 +134,12 @@ def bound_limits(value: Any) -> EffectiveLimits | None: """A run's persisted limits, or None (today's defaults) when absent or unreadable.""" if not isinstance(value, dict): return None - try: - values = {name: int(value[name]) for name in _BOUNDS} - except (KeyError, TypeError, ValueError): - return None + values: dict[str, int] = {} + for name in _BOUNDS: + stored = value.get(name) + if not isinstance(stored, int) or isinstance(stored, bool): + return None # missing, bool, float or string: never coerced + values[name] = stored ceilings = { **{name: ceiling for name, (_, _, ceiling) in _BOUNDS.items()}, "session_minutes": OVERRIDE_SESSION_MINUTES_CEILING, diff --git a/tests/test_author_overrides.py b/tests/test_author_overrides.py index 11b12c34..833522e4 100644 --- a/tests/test_author_overrides.py +++ b/tests/test_author_overrides.py @@ -558,10 +558,12 @@ def test_start_validates_with_the_launched_tick_image( @pytest.mark.parametrize("listed", [False, True]) @pytest.mark.parametrize("name,ceiling", [("session_minutes", 240), ("session_max_turns", 300)]) -@pytest.mark.parametrize("value", [None, True, "120", 120.5, 9, 0, -1, 301]) +@pytest.mark.parametrize("value", [None, True, "120", 120.5, 9, 0, -1, "ceiling+1"]) def test_invalid_session_limits_at_startup(listed, name, ceiling, value): from outerloop.author_overrides import validate_overrides + if value == "ceiling+1": + value = ceiling + 1 entry = {"backend": "claude", "model": "served-model", "slots": ["agent-01"], name: value} raw = json.dumps({"owner/repo": [entry] if listed else entry}) with pytest.raises(ValueError, match=f"{name} must be an integer between 10 and {ceiling}"): diff --git a/tests/test_limits.py b/tests/test_limits.py index 1300dbc5..c5e6096e 100644 --- a/tests/test_limits.py +++ b/tests/test_limits.py @@ -133,6 +133,9 @@ def test_bound_limits_tolerates_damaged_records(): assert bound_limits(None) is None assert bound_limits({"session_minutes": 180}) is None assert bound_limits("180") is None + base = effective_limits().__dict__ + for bad in ("250", 180.9, 1e10000, True): + assert bound_limits({**base, "session_minutes": bad}) is None full = {**effective_limits().__dict__, "session_minutes": 180, "future_knob": 1} kept, floored = bound_limits(full), bound_limits({**full, "session_max_turns": 0}) assert kept is not None and kept.session_minutes == 180