From 2f3a2ffcec8fc5df4bc1a9c5e1dccbcb4d71f02c Mon Sep 17 00:00:00 2001 From: mmcguire Date: Fri, 14 Aug 2026 17:17:52 +0100 Subject: [PATCH] Move runtime identity into system prompts --- Dockerfile.advisor | 2 +- Dockerfile.student | 2 +- README.md | 10 ++- SPEC.md | 21 +++--- k8s/entrypoint-advisor.sh | 5 +- k8s/entrypoint-student.sh | 5 +- senpai_agent/controller.py | 51 +++++---------- senpai_agent/launch_context.py | 44 +++++++++++-- senpai_agent/supervisor.py | 36 ++++++++++- system_instructions/ADVISOR.md | 12 +++- system_instructions/STUDENT.md | 8 +++ tests/test_controller.py | 41 ++++++------ tests/test_delegation.py | 50 ++++++++++++++ tests/test_image_split.py | 5 ++ tests/test_launch_context.py | 115 +++++++++++++++++++++++++++++++++ tests/test_supervisor.py | 65 +++++++++++++++++-- 16 files changed, 384 insertions(+), 88 deletions(-) diff --git a/Dockerfile.advisor b/Dockerfile.advisor index 5abaa945c..e611243e6 100644 --- a/Dockerfile.advisor +++ b/Dockerfile.advisor @@ -15,7 +15,7 @@ ENV PATH="/opt/senpai-venv/bin:/home/senpai/.local/bin:${PATH}" \ RUN apt-get update && \ apt-get install -y --no-install-recommends \ - ca-certificates curl gettext-base git gnupg procps tmux && \ + ca-certificates curl git gnupg procps tmux && \ groupadd --gid 10001 senpai && \ useradd --uid 10001 --gid 10001 --create-home senpai && \ rm -rf /var/lib/apt/lists/* diff --git a/Dockerfile.student b/Dockerfile.student index be6fcd979..e94b397ef 100644 --- a/Dockerfile.student +++ b/Dockerfile.student @@ -13,7 +13,7 @@ ENV PATH="/opt/senpai-venv/bin:/home/senpai/.local/bin:${PATH}" \ RUN apt-get update && \ apt-get install -y --no-install-recommends \ - ca-certificates curl gettext-base git gnupg procps && \ + ca-certificates curl git gnupg procps && \ groupadd --gid 10001 senpai && \ useradd --uid 10001 --gid 10001 --create-home senpai && \ rm -rf /var/lib/apt/lists/* diff --git a/README.md b/README.md index 4a422224d..6cdc38c98 100644 --- a/README.md +++ b/README.md @@ -394,7 +394,15 @@ Pod startup and liveness probes read the supervisor lease. Container restarts re GitHub coordination works across Docker, cloud VMs, or local hosts without private networking. The current repository does not yet provide a Compose or direct-host launcher: the Kubernetes manifests perform the source clone, environment assembly, skill installation, token handoff, mounts, and entrypoint selection. -To build another launcher, reproduce [entrypoint-advisor.sh](k8s/entrypoint-advisor.sh) or [entrypoint-student.sh](k8s/entrypoint-student.sh), persist `/var/lib/senpai//advisor` for the advisor, and use the container healthcheck with a restart policy. Student execution requires Linux, an NVIDIA runtime, and compatible CUDA hardware; Docker Desktop on macOS cannot run the GPU student image. +To build another launcher, reproduce the bootstrap around +[entrypoint-advisor.sh](k8s/entrypoint-advisor.sh) or +[entrypoint-student.sh](k8s/entrypoint-student.sh), pass the built-in role +template and its explicit identity values to the Python supervisor, persist +`/var/lib/senpai//advisor` for the advisor, and use the container +healthcheck with a restart policy. The supervisor uses the same fail-on-missing, +non-secret role renderer for Kubernetes and direct-host launches. Student +execution requires Linux, an NVIDIA runtime, and compatible CUDA hardware; +Docker Desktop on macOS cannot run the GPU student image. ## Development and reference diff --git a/SPEC.md b/SPEC.md index 9ec9cd35a..c91470405 100644 --- a/SPEC.md +++ b/SPEC.md @@ -200,24 +200,29 @@ The model receives: 1. OpenHands' native base system prompt and tool schemas. 2. One stable system suffix assembled from: - `system_instructions/SENPAI-HARNESS.md`; and - - the rendered advisor or student role charter; and + - the rendered advisor or student role charter, including its non-secret + runtime identity; and - the selected target-repository `program.md` under `## program.md - `. A blank `program_path` searches root `program.md` and one-level `*/program.md` paths and requires exactly one total match. 3. Applicable target `AGENTS.md` and compatible `CLAUDE.md` project context. 4. A compact skill catalog whose bodies are loaded only when invoked. -5. User turns containing optional launch instructions, runtime identity, - current state, and current UTC time. +5. User turns containing optional launch instructions, current state, and + current UTC time. Harness and role remain separate source documents because they have different owners, but are merged into one system suffix so the agent knows both the OpenHands operating contract and its Senpai role. Before constructing a model -worker, the supervisor resolves the configured path or fails with the missing -or ambiguous candidates. The runner reads that file, formats the programme -section, and appends it to the system suffix. Delegated children inherit the -resolved repository-relative path and build the same suffix. The programme is -not duplicated in ordinary user messages. OpenHands includes the system suffix +worker, the supervisor renders the role's `{{VARIABLE}}` placeholders once +from an explicit non-secret allowlist and snapshots the result in role state. +Any referenced value that is absent fails the launch; unrelated environment +variables and credentials are never considered. The supervisor also resolves +the configured programme path or fails with the missing or ambiguous +candidates. The runner appends that programme section to the system suffix. +Delegated children reuse the rendered role snapshot and resolved +repository-relative programme path. Neither runtime identity nor the programme +is duplicated in ordinary user messages. OpenHands includes the system suffix on every inference, and current time is rendered for every controller wake. ### Proposed live programme refresh diff --git a/k8s/entrypoint-advisor.sh b/k8s/entrypoint-advisor.sh index 4dc7c4e19..7c4cc4d62 100644 --- a/k8s/entrypoint-advisor.sh +++ b/k8s/entrypoint-advisor.sh @@ -25,10 +25,7 @@ if [ -z "${GITHUB_TOKEN:-}" ] && [ -n "${SENPAI_GITHUB_TOKEN_FILE:-}" ]; then fi : "${GITHUB_TOKEN:?GitHub bootstrap token is required}" export SENPAI_OPENHANDS_STATE_DIR="$LOGDIR/openhands_state" -export SENPAI_OPENHANDS_ROLE_FILE="$LOGDIR/ADVISOR.md" -envsubst '$PROBLEM_DIR $TARGET_REPO_URL $GH_REPO $ADVISOR_BRANCH $RESEARCH_TAG $GPUS_PER_STUDENT $WANDB_ENTITY $WANDB_PROJECT' \ - < "$WORKDIR/system_instructions/ADVISOR.md" \ - > "$SENPAI_OPENHANDS_ROLE_FILE" +export SENPAI_OPENHANDS_ROLE_FILE="$WORKDIR/system_instructions/ADVISOR.md" echo "=== Senpai Advisor ===" echo "Runner repo: $REPO_URL (revision: $REPO_REVISION)" diff --git a/k8s/entrypoint-student.sh b/k8s/entrypoint-student.sh index 689fb15dd..1c4ac7cb0 100644 --- a/k8s/entrypoint-student.sh +++ b/k8s/entrypoint-student.sh @@ -88,10 +88,7 @@ ls \ export IS_SANDBOX=1 export SENPAI_OPENHANDS_STATE_DIR="$LOGDIR/openhands_state" -export SENPAI_OPENHANDS_ROLE_FILE="$LOGDIR/STUDENT.md" -envsubst '$PROBLEM_DIR $TARGET_REPO_URL $GH_REPO $ADVISOR_BRANCH $RESEARCH_TAG $STUDENT_NAME $GPUS_PER_STUDENT $WANDB_ENTITY $WANDB_PROJECT' \ - < "$WORKDIR/system_instructions/STUDENT.md" \ - > "$SENPAI_OPENHANDS_ROLE_FILE" +export SENPAI_OPENHANDS_ROLE_FILE="$WORKDIR/system_instructions/STUDENT.md" export SENPAI_OPENHANDS_WORKSPACE="$TARGET_WORKDIR" export SENPAI_OPENHANDS_HARNESS_FILE="$WORKDIR/system_instructions/SENPAI-HARNESS.md" export SENPAI_OPENHANDS_TIMEOUT_SECONDS="${SENPAI_OPENHANDS_TIMEOUT_SECONDS:-3600}" diff --git a/senpai_agent/controller.py b/senpai_agent/controller.py index 2dff0f6e5..afc929d64 100644 --- a/senpai_agent/controller.py +++ b/senpai_agent/controller.py @@ -17,11 +17,7 @@ from uuid import UUID from senpai_agent.agent_markdown import strip_spdx_header -from senpai_agent.advisor import ( - AdvisorEvent, - AdvisorEventStore, - compose_senpai_instructions, -) +from senpai_agent.advisor import AdvisorEvent, AdvisorEventStore from senpai_agent.github.mailbox import ActiveGitHubWatcher, GitHubMailbox from senpai_agent.inbox import ( DeliveryState, @@ -102,7 +98,11 @@ def _is_context_history_failure(error: Exception) -> bool: def _context_recovery_prompt(full_prompt: str, current_prompt: str) -> str: - launch_context = "" if full_prompt in current_prompt else f"{full_prompt}\n\n" + launch_context = ( + f"{full_prompt}\n\n" + if full_prompt and full_prompt not in current_prompt + else "" + ) return launch_context + ( "# Conversation context recovery\n\n" "The previous model-visible conversation branch exhausted or corrupted " @@ -124,8 +124,6 @@ def __init__( ): self.config = config self.full_prompt = full_prompt.strip() - if not self.full_prompt: - raise ValueError("full prompt must not be empty") self.github_mailbox = github_mailbox self.active_poll_interval_seconds = active_poll_interval_seconds @@ -722,8 +720,9 @@ def _prompt( ) -> str: now = datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%SZ") if not continuing: + launch_context = f"{self.full_prompt}\n\n" if self.full_prompt else "" return ( - f"{self.full_prompt}\n\nCurrent time (UTC): {now}\n\n" + f"{launch_context}Current time (UTC): {now}\n\n" "# Current GitHub state\n\n" "Actionable events follow as separately tracked messages." ) @@ -741,26 +740,15 @@ def _prompt( return prompt -def _full_prompt(role: Literal["advisor", "student"], env: Mapping[str, str]) -> str: - sections = [] +def _full_prompt(env: Mapping[str, str]) -> str: encoded_extra = env.get("EXTRA_INSTRUCTIONS_B64") - if encoded_extra: - extra = b64decode(encoded_extra, validate=True).decode() - sections.append( - "# Additional launch instructions\n\n" - f"{strip_spdx_header(extra).strip()}" - ) - identity = ( - f"Role: {role}; repository: {env['GH_REPO']}; " - f"advisor branch: {env['ADVISOR_BRANCH']}; " - f"W&B: {env['WANDB_ENTITY']}/{env['WANDB_PROJECT']}." + if not encoded_extra: + return "" + extra = b64decode(encoded_extra, validate=True).decode() + return ( + "# Additional launch instructions\n\n" + f"{strip_spdx_header(extra).strip()}" ) - if role == "advisor": - identity += f" Students: {env.get('STUDENT_NAMES', '')}." - else: - identity += f" Student: {env['STUDENT_NAME']}." - sections.append(f"# Runtime identity\n\n{identity}") - return "\n\n".join(sections) def _role_interval( @@ -787,7 +775,6 @@ def controller_main( from senpai_agent.delegation import reconcile_delegated_tasks from senpai_agent.openhands_runner import ( parse_runner_args, - read_role_instructions, resolve_config, scrub_model_credentials, ) @@ -879,13 +866,9 @@ def controller_main( token=runner_config.github_token, ) - full_prompt = _full_prompt(role, env) + full_prompt = _full_prompt(env) continuation_context = ( - f"{compose_senpai_instructions( - read_role_instructions(runner_config.harness_file), - read_role_instructions(runner_config.role_file), - ).strip()}\n\n" - f"# Current launch context\n\n{full_prompt}" + f"# Current launch context\n\n{full_prompt}" if full_prompt else "" ) inbox = PersistentInbox( runner_config.state_dir / "delivery-inbox.sqlite3", diff --git a/senpai_agent/launch_context.py b/senpai_agent/launch_context.py index 0092f95c0..c5c305362 100644 --- a/senpai_agent/launch_context.py +++ b/senpai_agent/launch_context.py @@ -5,7 +5,9 @@ """Render the launch-time role context stored in system_instructions/.""" import re +from collections.abc import Mapping from pathlib import Path +from typing import Literal from senpai_agent.agent_markdown import read_agent_markdown, strip_spdx_header @@ -13,17 +15,49 @@ RUNTIME_TEMPLATE = INSTRUCTIONS_ROOT / "SENPAI-LAUNCH-RUNTIME.md" ISOLATION_TEMPLATE = INSTRUCTIONS_ROOT / "SENPAI-LAUNCH-ISOLATION.md" OPERATOR_TEMPLATE = INSTRUCTIONS_ROOT / "SENPAI-OPERATOR-INSTRUCTIONS.md" -PLACEHOLDER = re.compile(r"{{([A-Z_]+)}}") +PLACEHOLDER = re.compile(r"{{([A-Z_][A-Z0-9_]*)}}") +ROLE_TEMPLATE_VALUES = { + "advisor": ( + "GH_REPO", + "ADVISOR_BRANCH", + "WANDB_ENTITY", + "WANDB_PROJECT", + "STUDENT_NAMES", + ), + "student": ( + "GH_REPO", + "ADVISOR_BRANCH", + "WANDB_ENTITY", + "WANDB_PROJECT", + "STUDENT_NAME", + ), +} -def _render(path: Path, values: dict[str, str]) -> str: +def _render(path: Path, values: Mapping[str, str]) -> str: template = read_agent_markdown(path) missing = sorted(set(PLACEHOLDER.findall(template)) - values.keys()) if missing: raise ValueError(f"Missing {path.name} values: {', '.join(missing)}") - for key, value in values.items(): - template = template.replace(f"{{{{{key}}}}}", value) - return template.strip() + return PLACEHOLDER.sub(lambda match: values[match.group(1)], template).strip() + + +def render_role_prompt( + path: Path, + role: Literal["advisor", "student"], + env: Mapping[str, str], +) -> str: + """Render one role charter from explicitly allowlisted non-secret values.""" + + values = { + "ROLE": role, + **{ + name: env[name] + for name in ROLE_TEMPLATE_VALUES[role] + if env.get(name) + }, + } + return _render(path, values) def render_launch_context( diff --git a/senpai_agent/supervisor.py b/senpai_agent/supervisor.py index f2104b059..9839c5076 100644 --- a/senpai_agent/supervisor.py +++ b/senpai_agent/supervisor.py @@ -14,10 +14,12 @@ from collections.abc import Mapping, Sequence from dataclasses import dataclass from pathlib import Path +from typing import Literal import psutil from pydantic import SecretStr +from senpai_agent.launch_context import render_role_prompt from senpai_agent.processes import terminate_process_group from senpai_agent.program_context import ( PROGRAM_PATH_ENV, @@ -346,7 +348,11 @@ def supervisor_main( return 0 if lease_is_healthy(args.lease_path) else 1 state_dir = Path(env["SENPAI_OPENHANDS_STATE_DIR"]).resolve() - worker_environment = prepare_program_context_environment(env) + worker_environment = prepare_system_context_environment( + args.command, + state_dir, + env, + ) github_token = _consume_github_token(env) stop = threading.Event() @@ -374,10 +380,12 @@ def request_stop(_signum: int, _frame: object) -> None: signal.signal(signum, handler) -def prepare_program_context_environment( +def prepare_system_context_environment( + role: Literal["advisor", "student"], + state_dir: Path, env: Mapping[str, str], ) -> dict[str, str]: - """Resolve program.md before any model process starts.""" + """Snapshot the stable system context before any model process starts.""" environment = dict(env) program = load_program_system_prompt( @@ -385,10 +393,32 @@ def prepare_program_context_environment( environment.get(PROGRAM_PATH_ENV, ""), ) environment[PROGRAM_PATH_ENV] = program.program_path + role_prompt = state_dir / "system-instructions" / f"{role}.md" + if role_prompt.exists(): + if not role_prompt.read_text(encoding="utf-8").strip(): + raise RuntimeError(f"persisted role prompt is empty: {role_prompt}") + else: + source_value = environment.get("SENPAI_OPENHANDS_ROLE_FILE") + if not source_value: + raise RuntimeError( + "OpenHands role instructions are required; set " + "SENPAI_OPENHANDS_ROLE_FILE" + ) + rendered = render_role_prompt( + Path(source_value).resolve(), + role, + environment, + ) + role_prompt.parent.mkdir(parents=True, exist_ok=True) + temporary = role_prompt.with_suffix(".tmp") + temporary.write_text(f"{rendered}\n", encoding="utf-8") + temporary.replace(role_prompt) + environment["SENPAI_OPENHANDS_ROLE_FILE"] = str(role_prompt) print( f"SENPAI_PROGRAM_CONTEXT path={program.program_path}", flush=True, ) + print(f"SENPAI_ROLE_PROMPT path={role_prompt}", flush=True) return environment diff --git a/system_instructions/ADVISOR.md b/system_instructions/ADVISOR.md index 82b838f39..0d2f59c23 100644 --- a/system_instructions/ADVISOR.md +++ b/system_instructions/ADVISOR.md @@ -15,6 +15,14 @@ Read the `program.md` identified in your system prompt before acting. It defines the research objective, metric direction, training constraints, protected files, and target-specific operating rules. +## Runtime identity + +- Role: `{{ROLE}}` +- GitHub repository: `{{GH_REPO}}` +- Advisor branch: `{{ADVISOR_BRANCH}}` +- W&B project: `{{WANDB_ENTITY}}/{{WANDB_PROJECT}}` +- Students: `{{STUDENT_NAMES}}` + ## Your Identity You are a senior researcher at a top ML lab. You oversee students who have access to expensive GPUs, and keeping those GPUs productively occupied is part of your responsibility. An idle GPU represents a missed research opportunity. @@ -160,8 +168,8 @@ Read student suggestions. The "Suggested follow-ups" section in a student's resu In multi-benchmark targets like `target/icml2026`, the default unit of work should be a hypothesis family that is tested across all relevant datasets, -not a one-off single-benchmark tweak. Use the student's $GPUS_PER_STUDENT GPUs to cover a -small matrix across datasets and nearby variants unless a single-dataset +not a one-off single-benchmark tweak. Use the student's allocated GPUs to cover +a small matrix across datasets and nearby variants unless a single-dataset frontier closure or best-checkpoint recovery run is clearly the highest-value use of that slot. diff --git a/system_instructions/STUDENT.md b/system_instructions/STUDENT.md index 5c723db71..2ae3836a0 100644 --- a/system_instructions/STUDENT.md +++ b/system_instructions/STUDENT.md @@ -14,6 +14,14 @@ body and every PR comment and review before editing. Together they define the hypothesis, allowed files, metric contract, run limits, and any requested revision. +## Runtime identity + +- Role: `{{ROLE}}` +- GitHub repository: `{{GH_REPO}}` +- Advisor branch: `{{ADVISOR_BRANCH}}` +- W&B project: `{{WANDB_ENTITY}}/{{WANDB_PROJECT}}` +- Student: `{{STUDENT_NAME}}` + ## Boundaries - Work only on the assigned PR and branch. Do not invent another assignment, diff --git a/tests/test_controller.py b/tests/test_controller.py index ce85bee25..a19d49a12 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -79,7 +79,7 @@ def controller(mailbox, turns, **overrides): mailbox=mailbox, turns=turns, conversation_id=overrides.pop("conversation_id", CONVERSATION_ID), - full_prompt="programme", + full_prompt=overrides.pop("full_prompt", "programme"), sleep=lambda _seconds: None, poll_interval_seconds=600, jitter_seconds=0, @@ -144,9 +144,8 @@ def research_base_event(current_sha="def"): ) -def test_first_turn_combines_launch_instructions_and_runtime_identity(): +def test_first_turn_contains_launch_instructions_without_runtime_identity(): prompt = _full_prompt( - "student", { "GH_REPO": "acme/widgets", "ADVISOR_BRANCH": "research", @@ -164,13 +163,16 @@ def test_first_turn_combines_launch_instructions_and_runtime_identity(): assert "# Student task" not in prompt assert "live-secret" not in prompt assert "# Additional launch instructions\n\nUse typed tools." in prompt - assert "Role: student; repository: acme/widgets" in prompt + assert "# Runtime identity" not in prompt + assert "acme/widgets" not in prompt + assert "research" not in prompt + assert "acme/cfd" not in prompt + assert "fern" not in prompt assert "SPDX-" not in prompt -def test_advisor_first_turn_contains_only_runtime_identity_without_launch_text(): +def test_first_turn_without_launch_instructions_has_no_user_level_identity(): prompt = _full_prompt( - "advisor", { "GH_REPO": "acme/widgets", "ADVISOR_BRANCH": "research", @@ -180,11 +182,18 @@ def test_advisor_first_turn_contains_only_runtime_identity_without_launch_text() }, ) - assert prompt == ( - "# Runtime identity\n\n" - "Role: advisor; repository: acme/widgets; advisor branch: research; " - "W&B: acme/cfd. Students: fern,frieren." - ) + assert prompt == "" + + +def test_controller_accepts_an_empty_optional_launch_prompt(): + prompt = controller( + Mailbox([]), + Turns(), + full_prompt="", + )._prompt((), continuing=False) + + assert prompt.startswith("Current time (UTC):") + assert "Runtime identity" not in prompt def test_empty_mailbox_does_not_start_a_model_turn(): @@ -669,7 +678,6 @@ def test_controller_main_does_not_derive_reminders_from_fast_polling( lambda _args, _env: config, ) monkeypatch.setattr(runner_module, "scrub_model_credentials", lambda *_: None) - monkeypatch.setattr(runner_module, "read_role_instructions", lambda _: "") monkeypatch.setattr(tools_module, "close_training_runtimes", lambda: None) monkeypatch.setattr(weave_module, "finish_weave_monitoring", lambda: None) monkeypatch.setattr( @@ -677,11 +685,6 @@ def test_controller_main_does_not_derive_reminders_from_fast_polling( "GitHubMailbox", lambda **_kwargs: Mailbox([]), ) - monkeypatch.setattr( - controller_module, - "compose_senpai_instructions", - lambda *_: "", - ) monkeypatch.setattr(controller_module, "_full_prompt", lambda *_: "programme") created = [] @@ -707,9 +710,7 @@ def run(self, *, max_cycles=None): assert created[0].event_reminder_seconds == 600 assert created[0].full_prompt == "programme" assert created[0].turns.full_prompt == "programme" - assert created[0].system_context.endswith( - "# Current launch context\n\nprogramme" - ) + assert created[0].system_context == "# Current launch context\n\nprogramme" assert "Test programme" not in created[0].system_context diff --git a/tests/test_delegation.py b/tests/test_delegation.py index a62ce9692..f1ffe1215 100644 --- a/tests/test_delegation.py +++ b/tests/test_delegation.py @@ -21,10 +21,14 @@ render_child_prompt, run_child_process, ) +from senpai_agent.launch_context import INSTRUCTIONS_ROOT, PLACEHOLDER +from senpai_agent.openhands_runner import delegation_config as runner_delegation_config from senpai_agent.program_context import ( PROGRAM_PATH_ENV, ProgramSystemPromptSnapshot, ) +from senpai_agent.supervisor import prepare_system_context_environment +from openhands_support import runtime_config def delegation_request( @@ -173,6 +177,9 @@ def test_child_command_selects_agent_model_effort_and_credential(tmp_path: Path) assert fast.command[fast.command.index("--api-key-env") + 1] == ( "ANTHROPIC_API_KEY" ) + assert fast.command[fast.command.index("--role-file") + 1] == str( + config.role_file + ) assert "anthropic/claude-opus-4-8" in smart.command assert smart.command[smart.command.index("--reasoning-effort") + 1] == "xhigh" assert "openai/gpt-5.6" in frontier.command @@ -231,6 +238,49 @@ def test_child_environment_carries_the_resolved_program_path(tmp_path: Path): assert child.environment[PROGRAM_PATH_ENV] == "senpai/program.md" +def test_child_reuses_the_supervisor_rendered_role_prompt(tmp_path: Path): + workspace = tmp_path / "target" + workspace.mkdir() + (workspace / "program.md").write_text("Research policy.\n") + prepared = prepare_system_context_environment( + "advisor", + tmp_path / "state", + { + "SENPAI_OPENHANDS_WORKSPACE": str(workspace), + "SENPAI_OPENHANDS_ROLE_FILE": str(INSTRUCTIONS_ROOT / "ADVISOR.md"), + "GH_REPO": "acme/widgets", + "ADVISOR_BRANCH": "research", + "WANDB_ENTITY": "acme", + "WANDB_PROJECT": "cfd", + "STUDENT_NAMES": "fern,frieren", + "GITHUB_TOKEN": "github-secret-sentinel", + "WANDB_API_KEY": "wandb-secret-sentinel", + }, + ) + role_file = Path(prepared["SENPAI_OPENHANDS_ROLE_FILE"]) + parent = runtime_config(tmp_path, role_file=role_file) + delegated = runner_delegation_config(parent) + child = OpenHandsChildProcess( + delegated, + delegation_request(), + ) + + assert delegated.role_file == parent.role_file + assert delegated.harness_file == parent.harness_file + assert delegated.program == parent.program + assert child.command[child.command.index("--role-file") + 1] == str(role_file) + assert child.command[child.command.index("--harness-file") + 1] == str( + parent.harness_file + ) + assert child.environment[PROGRAM_PATH_ENV] == parent.program.program_path + role_prompt = role_file.read_text() + assert "Role: `advisor`" in role_prompt + assert "Students: `fern,frieren`" in role_prompt + assert PLACEHOLDER.search(role_prompt) is None + assert "github-secret-sentinel" not in role_prompt + assert "wandb-secret-sentinel" not in role_prompt + + def test_child_environment_replaces_ambient_model_credentials( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_image_split.py b/tests/test_image_split.py index fb5bfd5b3..33fc45969 100644 --- a/tests/test_image_split.py +++ b/tests/test_image_split.py @@ -202,6 +202,11 @@ def test_entrypoints_delegate_runtime_lifecycle_to_the_python_supervisor( assert logdir in entrypoint assert "serve-events" not in entrypoint + assert "envsubst" not in entrypoint + assert ( + f'SENPAI_OPENHANDS_ROLE_FILE="$WORKDIR/system_instructions/{role.upper()}.md"' + in entrypoint + ) assert f"exec python -m senpai_agent.supervisor {role}" in entrypoint assert "wait_for_senpai_start_gate" not in entrypoint trust_runner = 'git config --global safe.directory "$WORKDIR"' diff --git a/tests/test_launch_context.py b/tests/test_launch_context.py index 9c7bb80cf..fdf14e843 100644 --- a/tests/test_launch_context.py +++ b/tests/test_launch_context.py @@ -4,6 +4,12 @@ import yaml from launch_test_support import launch, launch_args, render_role +from senpai_agent.advisor import compose_system_instructions +from senpai_agent.launch_context import ( + INSTRUCTIONS_ROOT, + PLACEHOLDER, + render_role_prompt, +) def test_default_fleet_is_four_students_with_one_gpu_each(): @@ -85,3 +91,112 @@ def test_each_role_receives_the_configured_program_path(role): assert yaml.safe_load(configmap)["data"]["SENPAI_PROGRAM_PATH"] == ( "senpai/program.md" ) + + +@pytest.mark.parametrize( + ("role", "template", "role_identity", "other_identity"), + [ + ( + "advisor", + "ADVISOR.md", + "Students: `fern,frieren`", + "Student: `stark`", + ), + ( + "student", + "STUDENT.md", + "Student: `stark`", + "Students: `fern,frieren`", + ), + ], +) +def test_role_system_prompt_contains_only_allowlisted_runtime_identity( + role, + template, + role_identity, + other_identity, +): + env = { + "GH_REPO": "acme/widgets", + "ADVISOR_BRANCH": "research", + "WANDB_ENTITY": "acme", + "WANDB_PROJECT": "cfd", + "STUDENT_NAMES": "fern,frieren", + "STUDENT_NAME": "stark", + "WANDB_API_KEY": "wandb-secret-sentinel", + "GITHUB_TOKEN": "github-secret-sentinel", + "EXTRA_INSTRUCTIONS_B64": "mutable-operator-sentinel", + } + + role_prompt = render_role_prompt(INSTRUCTIONS_ROOT / template, role, env) + system_prompt = compose_system_instructions( + "Harness.", + role_prompt, + "## program.md - program.md\n\nProgramme.", + ) + + assert "## Runtime identity" in system_prompt + assert f"Role: `{role}`" in system_prompt + assert "GitHub repository: `acme/widgets`" in system_prompt + assert "Advisor branch: `research`" in system_prompt + assert "W&B project: `acme/cfd`" in system_prompt + assert role_identity in system_prompt + assert other_identity not in system_prompt + assert PLACEHOLDER.search(system_prompt) is None + for excluded in ( + "WANDB_API_KEY", + "wandb-secret-sentinel", + "GITHUB_TOKEN", + "github-secret-sentinel", + "EXTRA_INSTRUCTIONS_B64", + "mutable-operator-sentinel", + ): + assert excluded not in system_prompt + + +def test_role_prompt_fails_for_a_missing_referenced_value(): + env = { + "GH_REPO": "acme/widgets", + "ADVISOR_BRANCH": "research", + "WANDB_ENTITY": "acme", + "STUDENT_NAMES": "fern,frieren", + } + + with pytest.raises(ValueError, match="Missing ADVISOR.md values: WANDB_PROJECT"): + render_role_prompt(INSTRUCTIONS_ROOT / "ADVISOR.md", "advisor", env) + + +def test_role_prompt_never_renders_a_secret_placeholder(tmp_path): + template = tmp_path / "ADVISOR.md" + template.write_text("Token: {{GITHUB_TOKEN}}\n") + + with pytest.raises(ValueError, match="Missing ADVISOR.md values: GITHUB_TOKEN"): + render_role_prompt( + template, + "advisor", + {"GITHUB_TOKEN": "github-secret-sentinel"}, + ) + + +def test_role_prompt_rejects_an_unmapped_placeholder_containing_a_digit(tmp_path): + template = tmp_path / "STUDENT.md" + template.write_text("Value: {{VALUE2}}\n") + + with pytest.raises(ValueError, match="Missing STUDENT.md values: VALUE2"): + render_role_prompt(template, "student", {}) + + +def test_role_prompt_does_not_render_placeholders_introduced_by_values(tmp_path): + template = tmp_path / "ADVISOR.md" + template.write_text("Repository: {{GH_REPO}}\n") + + rendered = render_role_prompt( + template, + "advisor", + { + "GH_REPO": "{{WANDB_PROJECT}}", + "WANDB_PROJECT": "must-not-be-rendered", + }, + ) + + assert rendered == "Repository: {{WANDB_PROJECT}}" diff --git a/tests/test_supervisor.py b/tests/test_supervisor.py index a3fcf4726..b2af38446 100644 --- a/tests/test_supervisor.py +++ b/tests/test_supervisor.py @@ -15,7 +15,7 @@ SupervisorConfig, WorkerLease, WorkerSupervisor, - prepare_program_context_environment, + prepare_system_context_environment, ) @@ -53,7 +53,7 @@ def test_supervisor_caps_repeated_restart_backoff_at_five_minutes(): assert SupervisorConfig().max_backoff_seconds == 300 -def test_supervisor_resolves_program_path_before_starting_workers( +def test_supervisor_snapshots_program_and_rendered_role_before_starting_workers( tmp_path: Path, capsys: pytest.CaptureFixture[str], ): @@ -61,16 +61,71 @@ def test_supervisor_resolves_program_path_before_starting_workers( program = workspace / "senpai" / "program.md" program.parent.mkdir(parents=True) program.write_text("Research policy.") - - environment = prepare_program_context_environment( - {"SENPAI_OPENHANDS_WORKSPACE": str(workspace)}, + role_template = tmp_path / "ADVISOR.md" + role_template.write_text( + "Role={{ROLE}} Repo={{GH_REPO}} Project={{WANDB_PROJECT}}\n" + ) + state_dir = tmp_path / "state" + + environment = prepare_system_context_environment( + "advisor", + state_dir, + { + "SENPAI_OPENHANDS_WORKSPACE": str(workspace), + "SENPAI_OPENHANDS_ROLE_FILE": str(role_template), + "GH_REPO": "acme/widgets", + "WANDB_PROJECT": "cfd", + "GITHUB_TOKEN": "github-secret-sentinel", + "WANDB_API_KEY": "wandb-secret-sentinel", + }, ) assert environment["SENPAI_PROGRAM_PATH"] == "senpai/program.md" + role_prompt = Path(environment["SENPAI_OPENHANDS_ROLE_FILE"]) + assert role_prompt == state_dir / "system-instructions" / "advisor.md" + assert role_prompt.read_text() == "Role=advisor Repo=acme/widgets Project=cfd\n" + assert role_template.read_text().startswith("Role={{ROLE}}") assert capsys.readouterr().out == ( "SENPAI_PROGRAM_CONTEXT path=senpai/program.md\n" + f"SENPAI_ROLE_PROMPT path={role_prompt}\n" ) + restarted = prepare_system_context_environment( + "advisor", + state_dir, + { + "SENPAI_OPENHANDS_WORKSPACE": str(workspace), + "GH_REPO": "changed/widgets", + "WANDB_PROJECT": "changed", + }, + ) + + assert restarted["SENPAI_OPENHANDS_ROLE_FILE"] == str(role_prompt) + assert role_prompt.read_text() == "Role=advisor Repo=acme/widgets Project=cfd\n" + + +def test_supervisor_fails_before_snapshotting_a_role_with_missing_values( + tmp_path: Path, +): + workspace = tmp_path / "target" + workspace.mkdir() + (workspace / "program.md").write_text("Research policy.") + role_template = tmp_path / "STUDENT.md" + role_template.write_text("Student={{STUDENT_NAME}} Repo={{GH_REPO}}\n") + + with pytest.raises(ValueError, match="Missing STUDENT.md values: STUDENT_NAME"): + prepare_system_context_environment( + "student", + tmp_path / "state", + { + "SENPAI_OPENHANDS_WORKSPACE": str(workspace), + "SENPAI_OPENHANDS_ROLE_FILE": str(role_template), + "GH_REPO": "acme/widgets", + }, + ) + + assert not (tmp_path / "state" / "system-instructions" / "student.md").exists() + def test_supervisor_does_not_start_a_worker_without_a_discoverable_program( tmp_path: Path,