diff --git a/.claude/notes/agents.md b/.claude/notes/agents.md index 74f486bc..c1c67c28 100644 --- a/.claude/notes/agents.md +++ b/.claude/notes/agents.md @@ -434,10 +434,13 @@ agent had produced; bounding it only by a fixed cycle count disconnected from `t path unreachable — the watchdog always wins, and the same spurious-orphan turn burns the full turn timeout before crashing with zero criteria evaluated. -The cycle cap applies alongside that deadline, whichever comes first, and is the SOLE -bound when a task sets no timeout at all. Under a long `turn_timeout` (1800 s gives a -1440 s deadline) the deadline alone let a never-ending command (a dev server, an -unanswered prompt) idle the turn for 24 minutes. It is deliberately not +The cycle cap (120 × 5 s) bounds the wait only when a task sets no timeout at all. With +one, the deadline alone bounds it, so a never-ending command (a dev server, an unanswered +prompt) can idle up to 80% of the turn, but a slow job gets the time the task author +budgeted for it. Applying the cap under a timeout as well force-closed real solvers and +simulations at 10 minutes: on the SkillsBench Gemini 4-arm campaign it ended 22 of 348 +rows, 4 of which had passed with the deadline alone (an exam-scheduling MIP solve that +passed at 1211 s among them). It is deliberately not "break after N consecutive empty polls": `receive_steps()` returns identically empty whether a backgrounded job is still running or will never resolve, and there is no signal that tells the two apart except waiting. A count small enough to matter would abort real @@ -464,10 +467,32 @@ after the model had already finished. `finalize` force-closes them as unresolved way it confines Claude Code. Without it Antigravity ran with every builtin, including `start_subagent` and `search_web`, under a list that gave Claude Code neither. `Skill` and `TodoWrite` have no builtin (skills load through `skills_paths`) and are skipped. `finish` -stays on under any allowlist: it returns structured output, not a capability. -`enable_subagents` is a separate switch from the toolset and follows whether -`start_subagent` survives. The SDK takes an allowlist OR a denylist, so with both set the -denied tools are removed from the allowlist. +and `schedule` stay on under any allowlist: `finish` returns structured output, and +`schedule` is the timer the model sleeps on while a backgrounded `run_command` finishes. +Without `schedule`, Gemini re-checks the background task every few seconds and each check +is a model call against `max_turns`: on the SkillsBench Gemini 4-arm campaign mean turns +rose 36.8 → 50.2 and max-turn rows 4 → 16 on the same 110 rows, and the skill arms were hit +2-3x harder than baseline. With `schedule` on they came back to 35.4 and 1. + +An allowlist only narrows the harness's default toolset (`BuiltinTools.default()`). `Glob`, +`Grep` and `LS` map to `find_file`, `search_directory` and `list_directory`, which the +harness ships off, so an allowlist naming them leaves them off. Turned on, Gemini used them +in place of shell scripts (1,429 calls in 333 of 348 SkillsBench rows, none before), often +one search per turn. + +`start_subagent` also stays on under any allowlist, because the harness builds its system +prompt from the toolset: with `start_subagent` off it drops the whole subagents section +(4,801 → 2,783 characters), and with it the prompt's only "you do NOT need to poll ... you +will be notified" guidance. With it on, the system prompt is byte-identical to 0.12.9's. +The section alone does not stop Gemini polling a long job with status checks: in a 48-row +SkillsBench A/B it made 13.4 checks per row with the section and 13.7 without (8.5 on +0.12.9). The tool descriptions are identical either way, and +`enable_subagents` alone does not restore the section. Subagents the tool lists withhold +(no `Task`) are denied by policy at `invoke_subagent` instead, which covers both the +built-in `research` subagent and one the model defines with `define_subagent`; both get +`search_web` regardless of the parent's toolset, and a parent policy on `search_web` does +not reach a model-defined one. The SDK takes an allowlist OR a denylist, so with both set +the denied tools are removed from the allowlist, except the always-enabled ones. ## Antigravity non-interactive commands @@ -478,7 +503,7 @@ behind keeps the turn waiting on a prompt no one answers. `_NONINTERACTIVE_ENV` (`CI`, `npm_config_yes`, `GIT_TERMINAL_PROMPT=0`, `DEBIAN_FRONTEND`, `PIP_NO_INPUT`, `PAGER`/`GIT_PAGER=cat`) rides the per-agent `env` seam, each variable only where the environment does not already set it, so those commands answer themselves or fail fast. It -cannot close the terminal: a bare `read` or a server still runs until the poll cap. +cannot close the terminal: a bare `read` or a server still runs until the poll deadline. ## The receive_steps re-entrancy window diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index ad6aefba..a44f4378 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -145,7 +145,9 @@ sandbox working directory plus any skill roots). onto the harness's builtin tools (`Bash` → `run_command`, `Read` → `view_file`, `Write` → `create_file`, `Edit` → `edit_file`, `Glob` → `find_file`, `Grep` → `search_directory`, `Task` → `start_subagent`, `WebSearch` → `search_web`, `WebFetch` → `read_url_content`). -`Skill` has no builtin and is skipped; `finish` always stays on. +`Skill` has no builtin and is skipped. `finish`, `schedule` and `start_subagent` always +stay on, the last so the harness keeps its default system prompt; when the lists do not +allow `Task`, running a subagent (`invoke_subagent`) is denied by policy instead. `run_command` also gets non-interactive environment variables (`CI=1`, `npm_config_yes=true`, `GIT_TERMINAL_PROMPT=0`, `DEBIAN_FRONTEND=noninteractive`, @@ -205,8 +207,8 @@ as every other agent. 10-second maximum synchronous wait; past it the command becomes a background task and the model gets a task id, not a result. The turn polls for that result instead of finalizing on an idle step stream, so slow work does complete — but only an - orphaned `run_command` is waited on, the wait is bounded by 10 minutes or 80% of - `turn_timeout` (whichever is shorter), and a job that outlives it (typically a server + orphaned `run_command` is waited on, the wait is bounded by 80% of `turn_timeout` + (10 minutes when the task sets none), and a job that outlives it (typically a server the model left running) is force-closed as `result_status: unknown` and graded normally rather than as a timeout. Measured in [Run-Limit Parity](HARNESS_PARITY.md). diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 3f28cb78..1f00afd1 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -95,11 +95,9 @@ # Rationale: .claude/notes/agents.md § Antigravity Step interleaving and the background poll _POLL_DEADLINE_TIMEOUT_FRACTION = 0.8 -# Cap on poll *cycles* -- the SOLE bound when a task sets no timeout at all, and -# a backstop against a very large one (applied alongside the deadline, whichever -# is reached first). 120 * 5s = 10 minutes, ~2x the worst real -# backgrounded-job duration observed (60-300s). Deliberately NOT "break after N -# consecutive empty polls". +# Cap on poll *cycles*, the bound only when a task sets no timeout at all; with +# one, the deadline alone bounds the wait. 120 * 5s = 10 minutes. Deliberately +# NOT "break after N consecutive empty polls". # Rationale: .claude/notes/agents.md § Antigravity Step interleaving and the background poll _MAX_BACKGROUND_POLLS = 120 @@ -148,9 +146,15 @@ "AskUserQuestion": "ask_question", } -# Kept on under any allowlist: `finish` is how a turn returns structured -# output, not a capability an allowlist is meant to grant or withhold. -_ALWAYS_ENABLED_TOOLS: frozenset[str] = frozenset({"finish"}) +# Kept on under any allowlist: `finish` returns a turn's structured output, +# `schedule` is how the model waits on a backgrounded command, and the harness +# drops its "you will be notified, do not poll" guidance with `start_subagent`. +# Withheld subagents are denied at `_SUBAGENT_CALL` instead. +# Rationale: .claude/notes/agents.md § Antigravity tool allowlist +_ALWAYS_ENABLED_TOOLS: frozenset[str] = frozenset({"finish", "schedule", "start_subagent"}) + +# The one call that runs a subagent, built-in or model-defined. +_SUBAGENT_CALL = "invoke_subagent" # Set on every run_command unless the environment already sets them, so a # command that would stop to ask (`npx` installing a package, git credentials, @@ -370,9 +374,12 @@ def _tool_capabilities(self, types: Any) -> Any: ``allowed_tools`` / ``disallowed_tools``, or ``None`` when neither is set. Names are mapped through ``_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP``; names with no - Antigravity builtin (``Skill``, ``TodoWrite``, MCP tools) are skipped. The - SDK takes an allowlist OR a denylist, so with both set the denied tools are - removed from the allowlist. + Antigravity builtin (``Skill``, ``TodoWrite``, MCP tools) are skipped. An + allowlist only narrows the harness's default toolset, so ``Glob`` / ``Grep`` / + ``LS`` never turn on the tools the harness ships off (``find_file``, + ``search_directory``, ``list_directory``). The SDK takes an allowlist OR a + denylist, so with both set the denied tools are removed from the allowlist. + ``_ALWAYS_ENABLED_TOOLS`` are never removed. Rationale: .claude/notes/agents.md § Antigravity tool allowlist """ @@ -380,27 +387,28 @@ def _tool_capabilities(self, types: Any) -> Any: if not allowed and not disallowed: return None builtin = {t.value for t in types.BuiltinTools} + harness_default = {t.value for t in types.BuiltinTools.default()} def to_builtin(names: list[str] | None) -> set[str]: mapped = {_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.get(n, n) for n in names or []} return mapped & builtin - # `enable_subagents` is a separate switch from the toolset; it follows - # whether `start_subagent` survives the filter. - subagent = types.BuiltinTools.START_SUBAGENT.value if allowed: - enabled = (to_builtin(allowed) | _ALWAYS_ENABLED_TOOLS) - to_builtin(disallowed) + enabled = ((to_builtin(allowed) & harness_default) - to_builtin(disallowed)) | _ALWAYS_ENABLED_TOOLS self._log.debug("Enabled builtin tools: %s", ", ".join(sorted(enabled))) - return types.CapabilitiesConfig( - enabled_tools=[types.BuiltinTools(t) for t in sorted(enabled)], - enable_subagents=subagent in enabled, - ) + return types.CapabilitiesConfig(enabled_tools=[types.BuiltinTools(t) for t in sorted(enabled)]) disabled = to_builtin(disallowed) - _ALWAYS_ENABLED_TOOLS self._log.debug("Disabled builtin tools: %s", ", ".join(sorted(disabled))) - return types.CapabilitiesConfig( - disabled_tools=[types.BuiltinTools(t) for t in sorted(disabled)], - enable_subagents=subagent not in disabled, - ) + return types.CapabilitiesConfig(disabled_tools=[types.BuiltinTools(t) for t in sorted(disabled)]) + + def _subagents_allowed(self) -> bool: + """Whether ``allowed_tools`` / ``disallowed_tools`` let the model run subagents (``Task``).""" + + def names_task(names: list[str] | None) -> bool: + return any(_CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.get(n, n) == "start_subagent" for n in names or []) + + allowed, disallowed = self.config.allowed_tools, self.config.disallowed_tools + return (not allowed or names_task(allowed)) and not names_task(disallowed) async def start( self, @@ -451,8 +459,9 @@ async def start( # policy would deny. ``permission_mode`` is deliberately NOT mapped # here — it does not confine this agent, exactly as on Codex, and # docs/agents/HARNESS_PARITY.md says so rather than leaving it - # silent. The isolation boundary is the driver. - policies=[policy.allow_all()], + # silent. The isolation boundary is the driver. Subagents the tool + # lists withhold are denied here, since `start_subagent` stays on. + policies=[policy.allow_all()] + ([] if self._subagents_allowed() else [policy.deny(_SUBAGENT_CALL)]), system_instructions=self.config.system_prompt or None, # Skill discovery: the search-path roots that parent the skill dirs. skills_paths=skills_paths, @@ -635,8 +644,11 @@ def _on_turn_timeout() -> None: and not state.max_turns_hit and not state.timeout_hit and state.has_orphaned_tool_call() - and poll_count < _MAX_BACKGROUND_POLLS - and (poll_deadline is None or time.monotonic() < poll_deadline) + and ( + poll_count < _MAX_BACKGROUND_POLLS + if poll_deadline is None + else time.monotonic() < poll_deadline + ) ): poll_count += 1 self._log.debug("Polling for backgrounded work (orphaned tool call); attempt %d", poll_count) @@ -662,9 +674,9 @@ def _on_turn_timeout() -> None: # stop/timeout: the call is force-closed as unresolved and # the turn is still graded normally on everything else. bound = ( - f"_MAX_BACKGROUND_POLLS ({_MAX_BACKGROUND_POLLS})" - if poll_count >= _MAX_BACKGROUND_POLLS - else f"poll_deadline ({_POLL_DEADLINE_TIMEOUT_FRACTION:.0%} of {timeout:g}s turn timeout)" + f"poll_deadline ({_POLL_DEADLINE_TIMEOUT_FRACTION:.0%} of {timeout:g}s turn timeout)" + if poll_deadline is not None + else f"_MAX_BACKGROUND_POLLS ({_MAX_BACKGROUND_POLLS})" ) msg = "Poll budget exhausted (%s, poll_count=%d) with a tool call still ACTIVE." self._log.warning(msg, bound, poll_count) diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index c5ddfebe..160f34fe 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -508,6 +508,11 @@ class _FakeBuiltinTools(enum.StrEnum): SCHEDULE = "schedule" FINISH = "finish" + @classmethod + def default(cls) -> list["_FakeBuiltinTools"]: + off = {cls.ASK_QUESTION, cls.LIST_DIR, cls.SEARCH_DIR, cls.FIND_FILE} + return [t for t in cls if t not in off] + def _install_fake_sdk(monkeypatch, sdk_agent_cls) -> None: """Stub ``google.antigravity`` in sys.modules so ``start()`` runs without the extra. @@ -883,10 +888,10 @@ async def _record_sleep(seconds: float) -> None: assert bash.result_status == "unknown" # force-closed as UNRESOLVED by finalize() -async def test_communicate_poll_cap_also_bounds_a_turn_with_a_large_timeout(monkeypatch): - """The cycle cap applies alongside the timeout-derived deadline, not only when - no timeout is set: under a large turn_timeout (1800s gives a 1440s deadline) a - never-closing job stops at _MAX_BACKGROUND_POLLS instead of the deadline.""" +async def test_communicate_poll_cap_does_not_cut_short_a_job_inside_the_deadline(monkeypatch): + """With a turn_timeout the deadline alone bounds the wait: a solver still running + past _MAX_BACKGROUND_POLLS cycles is waited on until it finishes. Seen live: a MIP + solve that passed at 1211s was force-closed at the 10-minute cap.""" from coder_eval.agents import antigravity_agent monkeypatch.setattr(antigravity_agent, "_MAX_BACKGROUND_POLLS", 3) @@ -897,22 +902,34 @@ async def _record_sleep(seconds: float) -> None: monkeypatch.setattr(antigravity_agent.asyncio, "sleep", _record_sleep) - never_closing = [ + started = [ _step( "TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", - tool_calls=[_tc("run_command", "stuck", {"command_line": "node server.js"})], + tool_calls=[_tc("run_command", "solve", {"command_line": "python solve.py"})], ), - _step("TEXT_RESPONSE", "DONE", content="server started", complete=True, usage=_usage(10, 0, 1, 0)), + _step("TEXT_RESPONSE", "DONE", content="solver running", complete=True, usage=_usage(10, 0, 1, 0)), ] - agent = _agent_with_steps([never_closing]) - tr = await agent.communicate("start it", timeout=1800.0) + finished = [ + _step( + "TOOL_CALL", + "DONE", + target="TARGET_ENVIRONMENT", + tool_calls=[ + _tc( + "run_command", "solve", {"command_line": "python solve.py", "exit_code": 0, "combined_output": "ok"} + ) + ], + ), + _step("TEXT_RESPONSE", "DONE", content="solved", complete=True, usage=_usage(10, 0, 1, 0)), + ] + agent = _agent_with_steps([started, [], [], [], [], finished]) + tr = await agent.communicate("solve it", timeout=1800.0) - assert len(sleep_calls) == 3 # the cap, long before the 1440s deadline + assert len(sleep_calls) == 5 bash = next(c for c in tr.commands if c.tool_name == "Bash") - assert bash.result_status == "unknown" - assert tr.agent_output == "server started" + assert bash.result_status == "success" async def test_communicate_does_not_poll_a_non_command_tool_left_active(monkeypatch): @@ -1536,45 +1553,76 @@ def test_tool_capabilities_none_without_tool_lists(): def test_allowed_tools_map_to_enabled_builtins(): - """The default experiment allowlist enables exactly the matching builtins, plus `finish`. + """The default experiment allowlist enables exactly the matching default builtins, plus + `finish`, `schedule` and `start_subagent`. - `Skill` has no builtin (skills load through skills_paths) and is skipped; subagents, - web search and the rest stay off, as they do for Claude Code under the same list. + `Skill` has no builtin (skills load through skills_paths) and is skipped; `Glob` / `Grep` + map to tools the harness ships off, so they stay off; web search and the rest stay off, as + they do for Claude Code under the same list. """ caps = _capabilities(allowed_tools=["Bash", "Read", "Write", "Edit", "Glob", "Grep", "Skill"]) assert [t.value for t in caps.enabled_tools] == [ "create_file", "edit_file", - "find_file", "finish", "run_command", - "search_directory", + "schedule", + "start_subagent", "view_file", ] - assert caps.enable_subagents is False -def test_allowed_task_keeps_subagents(): - caps = _capabilities(allowed_tools=["Bash", "Task"]) +def test_allowlist_never_enables_tools_the_harness_ships_off(): + caps = _capabilities(allowed_tools=["Bash", "Glob", "Grep", "LS", "AskUserQuestion"]) - assert {t.value for t in caps.enabled_tools} == {"run_command", "start_subagent", "finish"} - assert caps.enable_subagents is True + assert {t.value for t in caps.enabled_tools} == {"run_command", "finish", "schedule", "start_subagent"} def test_disallowed_tools_map_to_disabled_builtins(): caps = _capabilities(disallowed_tools=["Task", "WebSearch", "TodoWrite"]) - assert [t.value for t in caps.disabled_tools] == ["search_web", "start_subagent"] - assert caps.enable_subagents is False + assert [t.value for t in caps.disabled_tools] == ["search_web"] def test_disallowed_tools_are_removed_from_the_allowlist(): """The SDK takes an allowlist OR a denylist, so with both the denied tools leave the allowlist.""" - caps = _capabilities(allowed_tools=["Bash", "Read", "Task"], disallowed_tools=["Task"]) + caps = _capabilities(allowed_tools=["Bash", "Read", "WebSearch"], disallowed_tools=["WebSearch"]) + + assert {t.value for t in caps.enabled_tools} == {"run_command", "view_file", "finish", "schedule", "start_subagent"} + + +@pytest.mark.parametrize( + ("cfg", "denied"), + [ + ({}, []), + ({"allowed_tools": ["Bash", "Task"]}, []), + ({"allowed_tools": ["Bash", "Read"]}, ["invoke_subagent"]), + ({"disallowed_tools": ["Task"]}, ["invoke_subagent"]), + ({"allowed_tools": ["Bash", "Task"], "disallowed_tools": ["Task"]}, ["invoke_subagent"]), + ], +) +async def test_subagent_calls_are_denied_unless_task_is_allowed(monkeypatch, tmp_path, cfg, denied): + """`start_subagent` stays on so the harness keeps its default system prompt, whose only + "you will be notified, do not poll" guidance sits in its subagents section. A subagent the + tool lists withhold is denied at the call that runs it instead.""" + configs: list[Any] = [] + + class _FakeSdkAgent: + def __init__(self, cfg): + configs.append(cfg) + + async def __aenter__(self): + return self - assert {t.value for t in caps.enabled_tools} == {"run_command", "view_file", "finish"} - assert caps.enable_subagents is False + async def __aexit__(self, *exc): + return False + + _install_fake_sdk(monkeypatch, _FakeSdkAgent) + + await _agent(**cfg).start(str(tmp_path)) + + assert [p.tool for p in configs[0].policies if p.kind == "deny"] == denied async def test_start_passes_tool_capabilities_to_sdk_config(monkeypatch, tmp_path): @@ -1594,7 +1642,13 @@ async def __aexit__(self, *exc): await _agent(allowed_tools=["Bash", "Read"]).start(str(tmp_path)) - assert {t.value for t in configs[0].capabilities.enabled_tools} == {"run_command", "view_file", "finish"} + assert {t.value for t in configs[0].capabilities.enabled_tools} == { + "run_command", + "view_file", + "finish", + "schedule", + "start_subagent", + } def test_installed_sdk_accepts_the_tool_capabilities(): @@ -1602,10 +1656,12 @@ def test_installed_sdk_accepts_the_tool_capabilities(): every mapped name is a real builtin, so a renamed tool fails here, not live.""" types = pytest.importorskip("google.antigravity").types - assert set(agent_module._CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.values()) <= {t.value for t in types.BuiltinTools} + mapped = set(agent_module._CLAUDE_TO_ANTIGRAVITY_TOOL_MAP.values()) | agent_module._ALWAYS_ENABLED_TOOLS + assert mapped <= {t.value for t in types.BuiltinTools} caps = _agent(allowed_tools=["Bash", "Read", "Write", "Edit", "Glob", "Grep", "Skill"])._tool_capabilities(types) assert isinstance(caps, types.CapabilitiesConfig) - assert types.BuiltinTools.START_SUBAGENT not in caps.enabled_tools + assert types.BuiltinTools.START_SUBAGENT in caps.enabled_tools + assert types.BuiltinTools.FIND_FILE not in caps.enabled_tools # --- max_turns cap -------------------------------------------------------------------