-
Notifications
You must be signed in to change notification settings - Fork 16
fix(agent-challenge): unblock Terminal-Bench scoring and surface trial failures #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8ebbb56
22f7e6d
618e70a
0483d0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,7 +40,9 @@ | |
| import contextlib | ||
| import importlib | ||
| import inspect | ||
| from collections.abc import Awaitable, Callable | ||
| import os | ||
| import tempfile | ||
| from collections.abc import Awaitable, Callable, Iterator | ||
| from dataclasses import dataclass, field | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
@@ -73,6 +75,92 @@ | |
| #: agent-timeout code -- kept distinct from per-command crashes. | ||
| AGENT_TIMEOUT_REASON_CODE = "harbor_agent_timeout_error" | ||
|
|
||
| #: Maximum characters of captured construction output appended to an error. | ||
| CAPTURED_OUTPUT_LIMIT = 600 | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
| def _capture_process_output() -> Iterator[Callable[[], str]]: | ||
| """Capture fd 1/2 for the duration of the block, then replay them. | ||
|
|
||
| A miner constructor typically shells out (``pip install ...``). The child | ||
| writes to the *inherited* file descriptors, so Python-level redirection | ||
| never sees it and ``CalledProcessError`` keeps only argv + exit status — | ||
| the actual reason ("Directory is not installable", a resolver conflict, a | ||
| missing wheel) is lost from the trial record. Capturing at the fd level is | ||
| the only way to keep it. | ||
|
|
||
| Each stream is replayed to its original descriptor on exit so the runner's | ||
| own logs stay byte-complete; the returned callable yields the captured text | ||
| and is only meaningful after the block has exited. | ||
| """ | ||
|
|
||
| captured: dict[str, str] = {"text": ""} | ||
|
|
||
| def read() -> str: | ||
| return captured["text"] | ||
|
|
||
| saved: dict[int, int] = {} | ||
| buffers: dict[int, Any] = {} | ||
| try: | ||
| for fd in (1, 2): | ||
| handle = tempfile.TemporaryFile() | ||
| saved[fd] = os.dup(fd) | ||
| os.dup2(handle.fileno(), fd) | ||
| buffers[fd] = handle | ||
| except OSError: | ||
| # Never let observability break execution: restore and run uncaptured. | ||
| for fd, original in saved.items(): | ||
| with contextlib.suppress(OSError): | ||
| os.dup2(original, fd) | ||
| os.close(original) | ||
| for handle in buffers.values(): | ||
| with contextlib.suppress(OSError): | ||
| handle.close() | ||
| yield read | ||
| return | ||
|
|
||
| try: | ||
| yield read | ||
| finally: | ||
| chunks: list[str] = [] | ||
| for fd in (1, 2): | ||
| data = b"" | ||
| handle = buffers.get(fd) | ||
| if handle is not None: | ||
| try: | ||
| handle.flush() | ||
| handle.seek(0) | ||
| data = handle.read() | ||
| except OSError: | ||
| data = b"" | ||
| original = saved.get(fd) | ||
| if original is not None: | ||
| with contextlib.suppress(OSError): | ||
| os.dup2(original, fd) | ||
| os.close(original) | ||
| if data: | ||
| with contextlib.suppress(OSError): | ||
| os.write(fd, data) | ||
| chunks.append(data.decode("utf-8", "replace")) | ||
| if handle is not None: | ||
| with contextlib.suppress(OSError): | ||
| handle.close() | ||
| captured["text"] = "".join(chunks) | ||
|
|
||
|
|
||
| def _captured_detail(text: str) -> str: | ||
| """Render captured output as a single-line suffix for an error message.""" | ||
|
|
||
| lines = [line.strip() for line in text.splitlines() if line.strip()] | ||
| if not lines: | ||
| return "" | ||
| joined = " / ".join(lines) | ||
| if len(joined) > CAPTURED_OUTPUT_LIMIT: | ||
| joined = joined[-CAPTURED_OUTPUT_LIMIT:] | ||
| joined = f"…{joined}" | ||
| return f" | output: {joined}" | ||
|
|
||
|
|
||
| class AgentLoadError(Exception): | ||
| """The agent class could not be located / loaded (typed, fail-fast). | ||
|
|
@@ -246,24 +334,41 @@ async def drive( | |
| session: TmuxSession | None = None | ||
| try: | ||
| # 1. Load + construct the agent. Any failure here is the | ||
| # submission's fault -> submission_code_failed. | ||
| try: | ||
| agent_cls = self._agent_class or load_agent_class(self._import_path) | ||
| init_kwargs = dict(self._extra_init_kwargs) | ||
| if resolved_agent_env: | ||
| init_kwargs.setdefault("extra_env", dict(resolved_agent_env)) | ||
| agent = agent_cls( | ||
| logs_dir=Path(logs_dir) if logs_dir is not None else None, | ||
| model_name=model_name, | ||
| **init_kwargs, | ||
| # submission's fault -> submission_code_failed. Construction is | ||
| # wrapped in an fd-level capture because miner constructors shell | ||
| # out, and the child's diagnostics would otherwise never reach | ||
| # the trial record. | ||
| load_error: AgentLoadError | None = None | ||
| construction_error: Exception | None = None | ||
| with _capture_process_output() as captured_output: | ||
| try: | ||
| agent_cls = self._agent_class or load_agent_class(self._import_path) | ||
| init_kwargs = dict(self._extra_init_kwargs) | ||
| if resolved_agent_env: | ||
| init_kwargs.setdefault("extra_env", dict(resolved_agent_env)) | ||
| agent = agent_cls( | ||
| logs_dir=Path(logs_dir) if logs_dir is not None else None, | ||
| model_name=model_name, | ||
| **init_kwargs, | ||
| ) | ||
| except AgentLoadError as exc: | ||
| load_error = exc | ||
| except Exception as exc: | ||
| construction_error = exc | ||
| if load_error is not None: | ||
| return AgentRunResult( | ||
| status="failed", | ||
| reason_code=load_error.reason_code, | ||
| error=f"{load_error}{_captured_detail(captured_output())}", | ||
| ) | ||
| except AgentLoadError as exc: | ||
| return AgentRunResult(status="failed", reason_code=exc.reason_code, error=str(exc)) | ||
| except Exception as exc: | ||
| if construction_error is not None: | ||
| return AgentRunResult( | ||
| status="failed", | ||
| reason_code=AGENT_LOAD_FAILED_REASON_CODE, | ||
| error=f"agent construction failed: {exc}", | ||
| error=( | ||
| f"agent construction failed: {construction_error}" | ||
| f"{_captured_detail(captured_output())}" | ||
| ), | ||
|
Comment on lines
+358
to
+371
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift Redact captured constructor output before returning it. Both failure paths append raw stdout/stderr to As per coding guidelines, “Never log, document, or include evidence containing private keys, wallet mnemonics, API tokens, or full secret values; only names, digests, and SHAs may be exposed.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| ) | ||
|
|
||
| agent_info = self._safe_agent_info(agent) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound captured output before it consumes runner resources.
Line 107 writes arbitrary constructor output to an unbounded temporary file, and Lines 133-145 read it all into memory. The 600-character limit at Line 159 is applied too late; a failing submission can fill temporary storage or exhaust memory before diagnostics are truncated. Retain only a bounded tail while teeing output back to the original descriptors.
🤖 Prompt for AI Agents