diff --git a/.env.example b/.env.example index 0e10f5c..a1474fc 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,9 @@ TYPESAFE_API_KEY= # Live steering uses Codex App Server by default. Use `exec` as a non-steerable fallback. # FOREMAN_CODEX_BACKEND=app-server +# Optional worker settings, independent of FOREMAN_DECISION_MODEL. +# FOREMAN_CODEX_EXECUTABLE=codex +# FOREMAN_CODEX_MODEL=your-available-worker-model # FOREMAN_STEERING_ENABLED=true # FOREMAN_MAX_STEERS_PER_WORKER=1 # FOREMAN_STEERING_GRACE_SECONDS=30 diff --git a/README.md b/README.md index 6ac0919..b8fd02b 100644 --- a/README.md +++ b/README.md @@ -475,6 +475,8 @@ The most useful environment overrides are: | `FOREMAN_MAX_ITERATIONS` | `20` | Semantic decision ceiling | | `FOREMAN_MAX_CONSECUTIVE_ASSESSMENT_FAILURES` | `3` | Tolerated supervisor failures before escalation | | `FOREMAN_CODEX_BACKEND` | `app-server` | `app-server` for steering or `exec` fallback | +| `FOREMAN_CODEX_EXECUTABLE` | `codex` | Codex CLI executable or absolute path | +| `FOREMAN_CODEX_MODEL` | unset | Explicit Codex worker model; otherwise use native Codex configuration | | `FOREMAN_WORKER_BACKEND` | `codex` | `codex`, `opencode`, `hermes`, or `deepagents` worker backend | | `FOREMAN_DEEPAGENTS_EXECUTABLE` | `dcode` | Installed Deep Agents Code executable | | `FOREMAN_DEEPAGENTS_MODEL` | unset | Native provider/model identifier | diff --git a/docs/decision-model-validation.md b/docs/decision-model-validation.md deleted file mode 100644 index 4b5d9dc..0000000 --- a/docs/decision-model-validation.md +++ /dev/null @@ -1,74 +0,0 @@ -# Decision-model adapter validation - -Validation performed on 2026-10-13 for the decision-model adapter changes. This records actual -results from both live decision providers; it does not establish general model accuracy. - -## Offline and package validation - -- 466 Python tests passed against the source checkout and a wheel installed into a fresh Python - 3.12 environment. The clean environment used TypeSafe SDK 0.7.4 and OpenAI SDK 2.54.0. -- All 27 Pi/Pi Durable integration tests passed against the installed wheel. -- Ruff, local documentation links, and `git diff --check` passed. -- OpenAI contract tests use the real async SDK with an `httpx.MockTransport`. They exercise the - Decisions URL and payload, evidence grouping, responsibility/group routing, refusals, malformed - answers, HTTP errors, cancellation, client ownership, credentials, and both CLI supervision paths. - These tests do not make live OpenAI API requests. - -## Live Jev comparison - -The existing `foreman-test-app` calculator was copied into isolated directories. Its original five -source/configuration files were hashed before testing and matched afterward; its pre-existing -working-tree changes were preserved. The test copies replace the repository's old smoke-test -instructions with instructions authorizing this isolated test. - -The job adds `Calculator.square_root`, negative-input errors, parametrized tests, and README -examples, with independent verification. Actual pytest executions supply the evidence: - -| Scenario | Test evidence | Implementation complete | Requirements satisfied | Ready to finish | Tests sufficient | -| --- | --- | ---: | ---: | ---: | ---: | -| Before implementation | 25 existing tests passed; requested feature absent | 0.06 | 0.04 | 0.13 | 0.27 | -| Implemented and independently verified | 33 tests passed | 0.95 | 0.91 | 0.91 | 0.94 | -| Intentional regression: return the input instead of its square root | 3 failed, 30 passed | 0.03 | 0.03 | 0.02 | 0.03 | - -The adapter requested `jev-latest`; the API returned `jev-1.13.0`. Live routing activated the -conditional documentation responsibility with a score of 0.99. These are observations from one -small repository, not assertions of calibrated probabilities. - -A separate full managed run used a real Codex App Server coding worker and independent verifier. -Both workers completed, and the verifier passed all 33 tests. The factory nevertheless ended in -`ESCALATED` at its two-worker limit because its final completion judgments did not meet policy -thresholds. Native Foreman hooks were also active inside the worker sessions. This run validates -live routing, assessments, worker integration, and verification transport; it does not validate a -successful end-to-end `FINISHED` outcome. No thresholds or policy were changed to force completion. -The coding worker used `gpt-5.5`, independently of Jev. Earlier worker connection attempts were -rejected by the local Codex CLI/account and produced no calculator changes. - -## Live OpenAI comparison - -The OpenAI adapter made real requests to `POST /v1/decisions` using the same frozen calculator -observations and the same six completion, verification, and documentation predicates as Jev. -The requested and returned model was `gpt-6-luna`. No worker was launched or worker model selected -for this comparison. A key from the user-authorized sibling repository was loaded only into the -test process; no credentials were copied into Foreman's repository or committed. - -| Scenario | Implementation complete | Requirements satisfied | Ready to finish | Tests sufficient | -| --- | ---: | ---: | ---: | ---: | -| Before implementation | 0.00 | 0.00 | 0.00 | 0.02 | -| Implemented and independently verified | 0.99 | 0.95 | 0.97 | 0.97 | -| Intentional square-root regression | 0.01 | 0.00 | 0.00 | 0.00 | - -Live OpenAI routing activated the conditional documentation responsibility with a score of 1.00. -The original calculator files still match the recorded hashes. Both providers scored the incomplete -and regressed states below the configured completion minimums and the verified state above them -in this controlled comparison. This validates the router/check adapters on this example, not a -successful full managed OpenAI worker lifecycle or calibrated model accuracy across repositories. - -## Worker-model scope - -`FOREMAN_DECISION_PROVIDER` and `FOREMAN_DECISION_MODEL` configure only routing and checks. -Worker-model fields and worker selection in the runtime are unchanged. The temporary worker -model overrides used in the separate managed experiment were local test-harness files outside -this repository and have been removed. Regression tests verify that central decision configuration -cannot change a managed worker's backend, model, existing OpenAI key, or existing OpenAI endpoint. - -The PR remains unmerged for review, and no merge-gated release receipt is claimed. diff --git a/docs/decision-models.md b/docs/decision-models.md index 5db6d7f..ac13057 100644 --- a/docs/decision-models.md +++ b/docs/decision-models.md @@ -117,8 +117,3 @@ envelope differs, then compose `DecisionForemanModel` and `DecisionResponsibilit adapter. Add configuration selection and offline contract tests for successful predicates, refusals, malformed responses, deadlines, and client ownership. No worker or policy changes are needed to support a provider with the same predicate semantics. - -## Validation evidence - -See [adapter validation](decision-model-validation.md) for offline coverage, live Jev results, -and the pending OpenAI live test. diff --git a/docs/evidence.md b/docs/evidence.md index e9928a8..2d239e5 100644 --- a/docs/evidence.md +++ b/docs/evidence.md @@ -17,7 +17,7 @@ Foreman currently models these built-in providers: | `git.untracked` | Bounded, non-sensitive untracked-file excerpts | | `repository.instructions` | Applicable root instruction file and contents | | `tests` | Parsed test summaries from worker output | -| `verification` | Independent verifier-worker results | +| `verification` | Independent verifier-worker results and started/completed lifecycle flags | | `events` | Recent bounded lifecycle events | | `history` | Previous decision model result, intervention, and factory errors | @@ -70,7 +70,8 @@ request can use the hard limits and retain optional evidence within those limits its entire work and question definitions and only uses hard validation. When selected evidence exceeds the effective target, Foreman drops oldest history first, then -shortens optional strings while retaining their beginning and end. Optional +shortens bulky worker transcripts and lifecycle output before reducing source, test, or command +evidence. It retains the beginning and end of shortened strings. Optional fields can be removed as a final step. The request includes an `evidence_budget` record naming affected fields and explaining that omitted evidence is unknown, not evidence of success or absence. Compaction changes only the outgoing copy; saved session history remains intact. diff --git a/docs/hooks.md b/docs/hooks.md index 6d4b902..63dee0b 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -194,3 +194,10 @@ Command evidence is likewise assistant-neutral: a Codex, Pi, or Pi Durable bridg lifecycle events to `foreman hook`. Foreman owns provider invocation and check-specific evidence, so a CLI verifier does not need a separate SDK integration for every coding assistant. See [Evidence providers](evidence.md). + +## Hooks inside managed workers + +`foreman run` owns the supervision of the workers it launches. It marks their process tree with +`FOREMAN_MANAGED_WORKER=1`; native `foreman hook` commands in that tree return `{}` immediately. +This prevents duplicate routing, assessments, and completion guidance from the attached integration. +Ordinary attached sessions continue to use the configured native hooks. See [worker backends](workers.md). diff --git a/docs/releases/0.5.1.md b/docs/releases/0.5.1.md new file mode 100644 index 0000000..1a255ca --- /dev/null +++ b/docs/releases/0.5.1.md @@ -0,0 +1,24 @@ +# Foreman 0.5.1 + +This release adds provider-independent decision models and fixes managed Codex worker supervision. + +- Select Jev or OpenAI for responsibility routing and semantic checks through + `FOREMAN_DECISION_PROVIDER` and `FOREMAN_DECISION_MODEL`. The OpenAI adapter uses the native + Decisions API. Install `foreman-core[openai]` to include its SDK. +- Existing Jev configuration and public compatibility facades continue to work. Generic decision + request budgets and provider/model usage metadata support both adapters. +- Keep decision credentials scoped to the supervisor, including credentials loaded from central + configuration. Existing worker credentials and model settings retain their own scope. +- Set `FOREMAN_CODEX_EXECUTABLE` or `FOREMAN_CODEX_MODEL` to explicitly configure Codex coding + and verifier workers. Defaults still use the installed CLI and native model configuration. + Unsupported worker-model errors escalate with guidance; Foreman does not silently pick a fallback. +- Prevent native Foreman hooks from supervising a factory-owned worker a second time. Normalize + completed App Server output so actual test results and agent conclusions remain readable, and + include explicit independent-verification lifecycle evidence. Evidence compaction reduces bulky + transcripts before cutting source or test evidence. Completion thresholds are unchanged. + +See [decision-model configuration](../decision-models.md) and +[worker configuration](../workers.md) for setup and troubleshooting. + +These notes prepare 0.5.1 for review. Publishing the matching GitHub release after merge triggers +Foreman's existing Trusted Publisher workflow for PyPI. diff --git a/docs/releasing.md b/docs/releasing.md index 9c6e927..24f4638 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -3,7 +3,7 @@ Foreman's PyPI distribution is `foreman-core`. It provides the `foreman` import package and `foreman` command. -Prepared release notes: [Foreman 0.5.0](releases/0.5.0.md). +Prepared release notes: [Foreman 0.5.1](releases/0.5.1.md). ## One-time setup diff --git a/docs/workers.md b/docs/workers.md index a4f8f3d..98e677e 100644 --- a/docs/workers.md +++ b/docs/workers.md @@ -25,6 +25,38 @@ one capability and three coroutines: | Hermes | `FOREMAN_WORKER_BACKEND=hermes` | No — stop/retry only | | Deep Agents Code | `FOREMAN_WORKER_BACKEND=deepagents` | No — stop/retry only | +## Codex executable and model + +By default, Foreman runs `codex` from `PATH` and leaves model selection to native Codex +configuration. Set `FOREMAN_CODEX_EXECUTABLE` to select another installed CLI and +`FOREMAN_CODEX_MODEL` to explicitly select the model for both coding and verifier workers. +The same settings are available as `codex_executable` and `codex_model` in `FactoryConfig`. +The App Server transport sends the model in `thread/start`; the exec transport passes `--model`. +`FOREMAN_DECISION_PROVIDER` and `FOREMAN_DECISION_MODEL` configure only the router and checks. + +Model availability depends on the CLI version, authentication method, and account access. If Codex +rejects a model as unavailable or unsupported, Foreman escalates immediately with configuration +guidance. It does not choose another model or spend the retry budget repeating that request. +Check the installed CLI version and available models, then start a new run with a supported +configuration. Ordinary worker failures and transient transport errors keep their existing policy. +See the official [Codex model documentation](https://learn.chatgpt.com/docs/models) and +[CLI reference](https://learn.chatgpt.com/docs/cli/reference). + +## Supervision ownership and output + +Workers launched by `foreman run` receive `FOREMAN_MANAGED_WORKER=1`. A native `foreman hook` +invocation in that process tree returns an empty response before loading configuration or calling +a decision provider. The factory owns supervision for those workers, including the independent +verifier. This prevents a second supervisor from injecting competing completion guidance. +Standalone attached sessions retain their normal hook behavior. Custom worker implementations +should reuse `worker_environment()` so the same ownership marker applies. + +The App Server transport preserves streamed output once. If Codex supplies only a completed item, +Foreman records its actual command output and exit code, agent text, or changed paths, rather than +an escaped copy of the full protocol object and patch. Test summaries remain readable by the +pytest parser. Verification lifecycle flags accompany the verifier results in decision evidence; +they do not replace the completion and test checks or change their thresholds. + ## Pi and Pi Durable Pi and Pi Durable use [attached supervision through their native bridges](../integrations/pi/README.md). @@ -70,7 +102,8 @@ for model credentials, backend settings, and the separate native-hook path. subprocess template: bounded streaming, `start_new_session` process groups, interrupt-then-kill termination). 2. Keep the agent's environment clean: reuse `worker_environment()`, which - inherits the process environment minus Foreman's `TYPESAFE_*` credentials. + inherits the process environment minus Foreman's `TYPESAFE_*` and `FOREMAN_OPENAI_*` + credentials and marks factory-owned children to prevent duplicate hook supervision. 3. Wire it into `FactoryRuntime`'s default worker factory (or pass your own `worker_factory`) and set `supports_steering` accurately. Unknown custom workers default to non-steerable for backward compatibility. diff --git a/pyproject.toml b/pyproject.toml index 2760f0d..c4d76f7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "foreman-core" -version = "0.5.0" +version = "0.5.1" description = "An agent supervisor and software factory foreman powered by decision models" readme = "README.md" requires-python = ">=3.11" diff --git a/src/foreman/cli.py b/src/foreman/cli.py index d07216c..1c08124 100644 --- a/src/foreman/cli.py +++ b/src/foreman/cli.py @@ -312,6 +312,11 @@ def hook( ) -> None: """Process one coding-assistant hook event from stdin.""" + if os.getenv("FOREMAN_MANAGED_WORKER") == "1": + # The owning factory already supplies routing, checks, and completion control. + sys.stdout.write("{}\n") + return + extensions: ExtensionManager | None = None runtime_started = False try: diff --git a/src/foreman/config.py b/src/foreman/config.py index 3bc4341..4d24492 100644 --- a/src/foreman/config.py +++ b/src/foreman/config.py @@ -77,6 +77,8 @@ class FactoryConfig(BaseModel): hermes_max_turns: int = Field(default=200, ge=1) hermes_toolsets: str | None = None max_consecutive_assessment_failures: int = Field(default=3, ge=1) + codex_executable: str = Field(default="codex", min_length=1) + codex_model: str | None = Field(default=None, min_length=1) codex_backend: Literal["app-server", "exec"] = "app-server" steering_enabled: bool = True max_steers_per_worker: int = Field(default=1, ge=0) @@ -177,6 +179,8 @@ def from_environment(cls) -> FactoryConfig: "max_consecutive_assessment_failures", int, ), + "FOREMAN_CODEX_EXECUTABLE": ("codex_executable", str), + "FOREMAN_CODEX_MODEL": ("codex_model", str), "FOREMAN_CODEX_BACKEND": ("codex_backend", str), "FOREMAN_STEERING_ENABLED": ( "steering_enabled", diff --git a/src/foreman/evidence.py b/src/foreman/evidence.py index 0f18861..eb5eea4 100644 --- a/src/foreman/evidence.py +++ b/src/foreman/evidence.py @@ -32,7 +32,7 @@ class EvidenceSelection(Protocol): "git.untracked": ("untracked_evidence",), "repository.instructions": ("agents_md_path", "agents_md_instructions"), "tests": ("test_results",), - "verification": ("verification_results",), + "verification": ("verification_started", "verification_completed", "verification_results"), "events": ("recent_events",), "history": ("previous_result", "previous_intervention", "failures"), } diff --git a/src/foreman/models/worker.py b/src/foreman/models/worker.py index 1de56f7..a85afff 100644 --- a/src/foreman/models/worker.py +++ b/src/foreman/models/worker.py @@ -35,6 +35,7 @@ class WorkerRecord(BaseModel): stdout: str = "" stderr: str = "" termination_reason: str | None = None + configuration_error: str | None = Field(default=None, max_length=4000) client: str | None = None client_session_id: str | None = None client_turn_id: str | None = None diff --git a/src/foreman/observation.py b/src/foreman/observation.py index f527c0a..e4c5e67 100644 --- a/src/foreman/observation.py +++ b/src/foreman/observation.py @@ -39,6 +39,8 @@ class FactoryObservation(BaseModel): agents_md_path: str | None = None agents_md_instructions: str = "" test_results: list[dict[str, Any]] + verification_started: bool = False + verification_completed: bool = False verification_results: list[dict[str, Any]] command_evidence: list[dict[str, Any]] = Field(default_factory=list) recent_events: list[dict[str, Any]] @@ -146,6 +148,7 @@ def _bounded_worker(worker: WorkerRecord, output_limit: int) -> dict[str, Any]: "stdout_tail": _tail(worker.stdout, output_limit), "stderr_tail": _tail(worker.stderr, output_limit), "termination_reason": worker.termination_reason, + "configuration_error": worker.configuration_error, "client": worker.client, "client_session_id": worker.client_session_id, "client_turn_id": worker.client_turn_id, @@ -402,6 +405,8 @@ async def build_observation( f"{latest.stdout}\n{latest.stderr}" if latest else "" ) ], + verification_started=state.verification_started, + verification_completed=state.verification_completed, verification_results=[ result.model_dump(mode="json") for result in state.verification_results ], diff --git a/src/foreman/policy.py b/src/foreman/policy.py index dffeb27..3e1a27e 100644 --- a/src/foreman/policy.py +++ b/src/foreman/policy.py @@ -36,10 +36,22 @@ def _runtime_directive( priority=priority, ) + def configuration_error(self, state: FactoryState) -> Directive | None: + for worker in state.workers: + if worker.configuration_error: + return self._runtime_directive( + state, InterventionType.ESCALATE, worker.configuration_error, priority=1000, + ) + return None + def evaluate(self, state: FactoryState, result: ForemanResult) -> ForemanResult: assert self.responsibilities is not None proposed = self.responsibilities.directives(state, result) + configuration_error = self.configuration_error(state) + if configuration_error is not None: + proposed.append(configuration_error) + # Hard iteration limits remain a runtime invariant, not responsibility semantics. if self.enforce_iteration_limit and state.iteration >= state.max_iterations: proposed.append( diff --git a/src/foreman/request_budget.py b/src/foreman/request_budget.py index 7c87180..e196988 100644 --- a/src/foreman/request_budget.py +++ b/src/foreman/request_budget.py @@ -43,7 +43,13 @@ def _text_candidates( "original_job", "work", "current_operation", "agents_md_path", "agents_md_instructions", "routing_bindings", "active_extension_ids", "extension_snapshot_revisions", "run_id", "factory_status", "iteration", - "attempts", "elapsed_factory_seconds", + "attempts", "elapsed_factory_seconds", "verification_started", "verification_completed", +}) + + +# Reduce bulky transcripts before cutting source/test evidence in the middle. +TRANSCRIPT_FIELDS = frozenset({ + "active_workers", "worker_history", "latest_worker_output", "recent_events", }) @@ -159,7 +165,10 @@ def record() -> dict[str, int]: _text_candidates(value, field, result, field, candidates) if not candidates: break - _, field, parent, key, value = max(candidates, key=lambda item: item[0]) + _, field, parent, key, value = max( + candidates, + key=lambda item: (item[1] in TRANSCRIPT_FIELDS and item[0] > 1024, item[0]), + ) keep = max(64, len(value) // 4) parent[key] = value[:keep] + "\n[... evidence omitted ...]\n" + value[-keep:] omitted.add(field) @@ -173,7 +182,11 @@ def record() -> dict[str, int]: field for field in result if field not in PROTECTED_FIELDS and field != "evidence_budget" ] - for field in sorted(optional, key=lambda key: json_bytes(result[key]), reverse=True): + for field in sorted( + optional, + key=lambda key: (key in TRANSCRIPT_FIELDS, json_bytes(result[key])), + reverse=True, + ): del result[field] omitted.add(field) sizes = record() diff --git a/src/foreman/runtime.py b/src/foreman/runtime.py index df2968d..7f4f914 100644 --- a/src/foreman/runtime.py +++ b/src/foreman/runtime.py @@ -192,10 +192,14 @@ def _default_worker_factory(self, worker_type: WorkerType) -> Worker: ) if self.config.codex_backend == "app-server": return CodexAppServerWorker( + executable=self.config.codex_executable, + model=self.config.codex_model, output_limit=self.config.output_limit, graceful_termination_seconds=self.config.graceful_termination_seconds, ) return CodexWorker( + executable=self.config.codex_executable, + model=self.config.codex_model, output_limit=self.config.output_limit, graceful_termination_seconds=self.config.graceful_termination_seconds, ) @@ -385,6 +389,16 @@ async def start_worker(self, worker_type: WorkerType) -> WorkerRecord: async def _assess(self) -> Intervention | None: self.state.iteration += 1 self.state.touch() + configuration_error = self.policy.configuration_error(self.state) + if configuration_error is not None: + self.state.latest_intervention = configuration_error + self.state.intervention_history.append(configuration_error) + self.store.save_state(self.state) + await self.emit( + EventType.FOREMAN_INTERVENED, + configuration_error.model_dump(mode="json"), notify_foreman=False, + ) + return configuration_error observation = await self.observer.build(self.state) await self.emit( EventType.FOREMAN_OBSERVED, diff --git a/src/foreman/workers/base.py b/src/foreman/workers/base.py index aa7cfe6..362f22f 100644 --- a/src/foreman/workers/base.py +++ b/src/foreman/workers/base.py @@ -11,12 +11,14 @@ def worker_environment() -> dict[str, str]: - """Return the inherited environment without Foreman's dedicated decision-model credentials.""" + """Scope decision credentials to the supervisor and mark its managed process tree.""" - return { + environment = { name: value for name, value in os.environ.items() if not name.startswith(("TYPESAFE_", "FOREMAN_OPENAI_")) } + environment["FOREMAN_MANAGED_WORKER"] = "1" + return environment def codex_environment() -> dict[str, str]: diff --git a/src/foreman/workers/codex.py b/src/foreman/workers/codex.py index c5050ac..1fa173c 100644 --- a/src/foreman/workers/codex.py +++ b/src/foreman/workers/codex.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import json import os import signal from datetime import UTC, datetime @@ -8,6 +9,7 @@ from foreman.models import EventType, WorkerRecord, WorkerStatus, WorkerType from foreman.workers.base import EventCallback, codex_environment +from foreman.workers.codex_errors import model_configuration_error def coding_mission(job: str) -> str: @@ -47,11 +49,13 @@ def __init__( self, *, executable: str = "codex", + model: str | None = None, sandbox: str = "workspace-write", output_limit: int = 50_000, graceful_termination_seconds: float = 5.0, ) -> None: self.executable = executable + self.model = model self.sandbox = sandbox self.output_limit = output_limit self.graceful_termination_seconds = graceful_termination_seconds @@ -69,6 +73,7 @@ def command(self, repository: Path, mission: str) -> list[str]: "--color", "never", "--json", + *(["--model", self.model] if self.model is not None else []), mission, ] @@ -146,6 +151,16 @@ async def run( finally: await asyncio.gather(stdout_task, stderr_task, return_exceptions=True) record.finished_at = datetime.now(UTC) + if record.termination_reason == "nonzero_exit": + for line in record.stdout.splitlines(): + try: + event = json.loads(line) + except ValueError: + continue + if isinstance(event, dict) and event.get("type") in {"error", "turn.failed"}: + record.configuration_error = model_configuration_error(event) + if record.configuration_error: + break return record async def terminate(self, reason: str) -> None: diff --git a/src/foreman/workers/codex_app_server.py b/src/foreman/workers/codex_app_server.py index f310fcc..d36b2eb 100644 --- a/src/foreman/workers/codex_app_server.py +++ b/src/foreman/workers/codex_app_server.py @@ -12,6 +12,7 @@ from foreman.models import EventType, WorkerRecord, WorkerStatus from foreman.version import __version__ from foreman.workers.base import EventCallback, codex_environment +from foreman.workers.codex_errors import model_configuration_error class CodexAppServerError(RuntimeError): @@ -27,12 +28,14 @@ def __init__( self, *, executable: str = "codex", + model: str | None = None, sandbox: str = "workspace-write", output_limit: int = 50_000, graceful_termination_seconds: float = 5.0, request_timeout_seconds: float = 10.0, ) -> None: self.executable = executable + self.model = model self.sandbox = sandbox self.output_limit = output_limit self.graceful_termination_seconds = graceful_termination_seconds @@ -176,8 +179,30 @@ async def _handle_notification(self, message: Mapping[str, Any]) -> None: elif isinstance(item_id, str) and item_id in self._streamed_item_ids: await self._flush_item_output(item_id) text = f"{item_type} completed\n" + elif item_type == "agentMessage" and isinstance(item.get("text"), str): + text = str(item["text"]) + "\n" + elif item_type == "commandExecution": + output = item.get("aggregatedOutput") + command = item.get("command") + text = f"commandExecution completed (exit code {item.get('exitCode')})\n" + if isinstance(command, str): + text += f"command: {command[:1000]}\n" + if isinstance(output, str): + text += output + ("" if output.endswith("\n") else "\n") + elif item_type == "fileChange": + text = f"fileChange completed ({item.get('status', 'unknown')})\n" + changes = item.get("changes") + if isinstance(changes, list): + for change in changes[:100]: + if isinstance(change, Mapping): + text += f"file: {str(change.get('path', 'unknown'))[:1000]}\n" else: text = f"{json.dumps(dict(item), ensure_ascii=False, default=str)}\n" + if ( + self._record is not None and self._record.stdout + and not self._record.stdout.endswith("\n") + ): + text = "\n" + text await self._emit_output("stdout", text) return @@ -315,6 +340,7 @@ async def run( "approvalPolicy": "never", "sandbox": self.sandbox, "serviceName": "foreman", + **({"model": self.model} if self.model is not None else {}), }, ) thread = thread_response.get("thread") @@ -354,6 +380,7 @@ async def run( record.termination_reason = "codex_turn_failed" record.exit_code = 1 if error: + record.configuration_error = model_configuration_error(error) await self._emit_output( "stderr", f"Codex turn failed: {json.dumps(error, default=str)}\n" ) @@ -381,6 +408,7 @@ async def run( else: record.status = WorkerStatus.FAILED record.termination_reason = "app_server_error" + record.configuration_error = model_configuration_error(str(error)) record.exit_code = 1 await self._emit_output("stderr", f"{type(error).__name__}: {error}\n") finally: diff --git a/src/foreman/workers/codex_errors.py b/src/foreman/workers/codex_errors.py new file mode 100644 index 0000000..246df8f --- /dev/null +++ b/src/foreman/workers/codex_errors.py @@ -0,0 +1,52 @@ +"""Recognize terminal model-configuration errors from Codex transport errors.""" +from __future__ import annotations + +import json +import re +from collections.abc import Mapping +from typing import Any + +_MODEL_UNAVAILABLE = re.compile( + r"(?:the\s+.+?\s+model\s+is\s+not\s+supported" + r"|model\s+.+?\s+(?:does\s+not\s+exist|is\s+not\s+(?:supported|available))" + r"|(?:do\s+not|don't)\s+have\s+access\s+to\s+(?:the\s+)?model)", + re.IGNORECASE, +) + + +def model_configuration_error(error: Any, *, depth: int = 0) -> str | None: + """Inspect provider errors, never arbitrary agent output, with bounded decoding.""" + if depth > 5: + return None + if isinstance(error, Mapping): + code = error.get("code") or error.get("type") + message = error.get("message") + if isinstance(code, str) and code in { + "model_not_found", "unsupported_model", "invalid_model", + }: + return _hint(str(message or code)) + for name in ("error", "message"): + result = model_configuration_error(error.get(name), depth=depth + 1) + if result: + return result + elif isinstance(error, str): + text = error[:8000] + if _MODEL_UNAVAILABLE.search(text): + return _hint(text) + start = text.find("{") + if start >= 0: + try: + nested = json.loads(text[start:]) + except (ValueError, RecursionError): + return None + return model_configuration_error(nested, depth=depth + 1) + return None + + +def _hint(message: str) -> str: + return ( + f"Codex rejected the worker model: {message[:2000]}. " + "Check the installed Codex version and account model access. " + "Choose an available worker model with FOREMAN_CODEX_MODEL or native Codex configuration; " + "FOREMAN_DECISION_MODEL controls only router/check decisions. No fallback was attempted." + ) diff --git a/tests/test_config.py b/tests/test_config.py index 2294478..13f78cc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -83,3 +83,14 @@ def test_request_soft_target_environment_overrides(monkeypatch) -> None: def test_invalid_soft_targets_are_rejected(field) -> None: with pytest.raises(ValueError): FactoryConfig(**{field: 0}) + + +def test_codex_model_and_executable_environment_are_independent(monkeypatch): + monkeypatch.setenv("FOREMAN_CODEX_EXECUTABLE", "/custom/codex") + monkeypatch.setenv("FOREMAN_CODEX_MODEL", "worker-model") + monkeypatch.setenv("FOREMAN_DECISION_MODEL", "decision-model") + config = FactoryConfig.from_environment() + assert config.codex_executable == "/custom/codex" + assert config.codex_model == "worker-model" + assert config.decision_model == "decision-model" + assert FactoryConfig().codex_model is None diff --git a/tests/test_hooks.py b/tests/test_hooks.py index d7d259f..a7c98d8 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -750,3 +750,17 @@ async def test_pi_assessment_failure_denies_tool(tmp_path, client): output = adapter.render(event, await supervisor.handle(event)) assert output['action'] == 'block' assert 'Jev unavailable' in output['reason'] + + +def test_factory_owned_worker_hooks_do_not_start_duplicate_supervision(tmp_path, monkeypatch): + monkeypatch.setenv("FOREMAN_MANAGED_WORKER", "1") + monkeypatch.setenv("FOREMAN_CONFIG", "/must/not/be/read") + def forbidden(*args, **kwargs): + raise AssertionError("Managed workers must not route or assess again through native hooks") + monkeypatch.setattr("foreman.cli.ExtensionManager", forbidden) + result = CliRunner().invoke( + app, ["hook", "--data-dir", str(tmp_path)], input='{"event": "stop"}' + ) + assert result.exit_code == 0 + assert json.loads(result.stdout) == {} + assert not list(tmp_path.iterdir()) diff --git a/tests/test_observation.py b/tests/test_observation.py index 37bf223..6a43f4f 100644 --- a/tests/test_observation.py +++ b/tests/test_observation.py @@ -97,3 +97,17 @@ async def test_observation_includes_non_secret_routing_bindings(tmp_path) -> Non } assert observation.active_extension_ids == ["example"] assert observation.extension_snapshot_revisions == {"example": "revision-7"} + + +@pytest.mark.asyncio +async def test_verification_lifecycle_is_in_selected_evidence(state, tmp_path): + state.repository = str(tmp_path) + state.verification_started = True + state.verification_completed = True + store = RunStore(tmp_path) + store.initialize(state) + value = await ObservationBuilder(store, FactoryConfig()).build(state) + selected = value.state_for(("verification",)) + assert selected["verification_started"] is True + assert selected["verification_completed"] is True + assert "verification_completed" not in value.state_for(("git.diff",)) diff --git a/tests/test_request_budget.py b/tests/test_request_budget.py index 2620e29..256a02a 100644 --- a/tests/test_request_budget.py +++ b/tests/test_request_budget.py @@ -217,3 +217,28 @@ async def test_routing_rejects_aggregate_question_overflow_before_network_call() with pytest.raises(ResponsibilityRoutingError, match="whole request"): await router.route_groups("job", routes) assert client.calls == [] + + +@pytest.mark.parametrize("profile", ["jev", "openai"]) +def test_bulky_transcripts_do_not_hide_a_small_implementation_diff(profile): + from foreman.decision_models.openai import OpenAIRequestBudget + + budget = JevRequestBudget() if profile == "jev" else OpenAIRequestBudget() + diff = "before\n" * 180 + "def square_root(value): return math.sqrt(value)\n" + "after\n" * 180 + state = { + "original_job": "Add square_root, document errors, and independently verify it", + "git_diff": diff, + "latest_worker_output": "command output\n" * 1200 + "34 passed in 0.01s\n", + "worker_history": [{"stdout_tail": "other output\n" * 1200, "status": "completed"}], + "test_results": [{"passed": 34, "failed": 0}], + "verification_started": True, + "verification_completed": True, + } + before = copy.deepcopy(state) + fitted = budget.fit(state, {"complete": "Is the implementation complete?"}) + assert fitted.state["git_diff"] == diff + assert fitted.state["test_results"] == state["test_results"] + assert fitted.state["verification_completed"] is True + assert fitted.measurements["pair_bytes"] <= 12_000 + assert "latest_worker_output" in fitted.state["evidence_budget"]["omitted_fields"] + assert state == before diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 6e555be..94dc46d 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -221,3 +221,34 @@ async def test_assessment_failure_budget_exhaustion_escalates(tmp_path) -> None: escalations = [i for i in state.intervention_history if i.action is InterventionType.ESCALATE] assert len(escalations) == 1 assert "semantic assessment unavailable" in escalations[0].reason + + +@pytest.mark.parametrize("backend", ["app-server", "exec"]) +def test_codex_worker_model_is_independent_of_decision_model(tmp_path, backend): + runtime = FactoryRuntime( + repository=tmp_path, job="work", model=FakeForemanModel(), + config=FactoryConfig(codex_backend=backend, codex_model="worker-model", + codex_executable="/custom/codex", decision_model="decision-model"), + ) + from foreman.models import WorkerType + worker = runtime._default_worker_factory(WorkerType.CODING) + assert worker.model == "worker-model" + assert worker.executable == "/custom/codex" + + +@pytest.mark.asyncio +async def test_worker_configuration_error_escalates_without_a_model_call(tmp_path): + from foreman.models import WorkerRecord, WorkerStatus, WorkerType + + model = FakeForemanModel() + runtime = FactoryRuntime(repository=tmp_path, job="work", model=model) + runtime.store.initialize(runtime.state) + runtime.state.workers.append(WorkerRecord( + worker_id="worker-1", worker_type=WorkerType.CODING, mission="work", + status=WorkerStatus.FAILED, configuration_error="Choose an available model", + )) + intervention = await runtime._assess() + assert intervention.action is InterventionType.ESCALATE + assert intervention.reason == "Choose an available model" + assert model.calls == [] + assert len(runtime.state.workers) == 1 diff --git a/tests/test_worker.py b/tests/test_worker.py index da580ec..5c4943c 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -375,3 +375,109 @@ async def emit(event_type, payload) -> None: assert worker._record.stdout == "tests passed\ncommandExecution completed\n" assert len(events) == 2 + + +def test_codex_exec_model_is_an_explicit_argument(tmp_path): + command = CodexWorker(model="chosen-worker-model").command(tmp_path, "work") + assert command[command.index("--model") + 1] == "chosen-worker-model" + assert "--model" not in CodexWorker().command(tmp_path, "work") + + +@pytest.mark.asyncio +async def test_app_server_preserves_explicit_worker_model_and_reports_rejection( + monkeypatch, tmp_path +): + process = AppServerProcess() + async def create(*args, **kwargs): + return process + monkeypatch.setattr(asyncio, "create_subprocess_exec", create) + worker = CodexAppServerWorker(model="account-unavailable-model") + async def emit(*args): + pass + task = asyncio.create_task(worker.run(record(), tmp_path, emit, 1)) + await asyncio.sleep(0.02) + process.feed({"method": "turn/completed", "params": { + "threadId": "thread-1", "turn": {"id": "turn-1", "status": "failed", "error": { + "message": json.dumps({"status": 400, "error": {"type": "invalid_request_error", + "message": "The 'account-unavailable-model' model is not supported when using " + "Codex with a ChatGPT account."}}), + }}, + }}) + result = await task + params = next(m["params"] for m in process.messages if m.get("method") == "thread/start") + assert params["model"] == "account-unavailable-model" + assert result.status is WorkerStatus.FAILED + assert "FOREMAN_CODEX_MODEL" in result.configuration_error + assert "No fallback" in result.configuration_error + assert len([m for m in process.messages if m.get("method") == "turn/start"]) == 1 + + +@pytest.mark.asyncio +async def test_completed_items_expose_actual_test_output_and_agent_conclusion(): + from foreman.test_summary import parse_pytest_summaries + + worker = CodexAppServerWorker() + worker._record = record() + async def emit(*args): + pass + worker._emit = emit + for item in [ + {"id": "command", "type": "commandExecution", "command": "python -m pytest -q", + "aggregatedOutput": "33 passed in 0.01s\n", "exitCode": 0}, + {"id": "patch", "type": "fileChange", "status": "completed", + "changes": [{"path": "calculator.py", "diff": "huge diff" * 1000}]}, + {"id": "agent", "type": "agentMessage", "text": "Independent verification passed."}, + ]: + await worker._handle_notification({"method": "item/completed", "params": {"item": item}}) + output = worker._record.stdout + assert parse_pytest_summaries(output)[0].passed == 33 + assert "exit code 0" in output + assert "calculator.py" in output + assert output.endswith("Independent verification passed.\n") + assert "huge diff" not in output + assert "aggregatedOutput" not in output + + +@pytest.mark.asyncio +async def test_unterminated_stream_summary_keeps_its_own_line(): + from foreman.test_summary import parse_pytest_summaries + + worker = CodexAppServerWorker() + worker._record = record() + async def emit(*args): + pass + worker._emit = emit + await worker._handle_notification({"method": "item/commandExecution/outputDelta", "params": { + "itemId": "command", "delta": "33 passed in 0.01s", + }}) + await worker._handle_notification({"method": "item/completed", "params": { + "item": {"id": "command", "type": "commandExecution"}, + }}) + assert parse_pytest_summaries(worker._record.stdout)[0].passed == 33 + + +@pytest.mark.asyncio +async def test_exec_distinguishes_provider_model_error_from_agent_text(monkeypatch, tmp_path): + error = { + "message": "The 'blocked' model is not supported when using Codex with a ChatGPT account." + } + async def emit(*args): + pass + for kind, expected in [("error", True), ("item.completed", False)]: + process = Process((json.dumps({"type": kind, "error": error}) + "\n").encode(), b"", 1) + async def create(*args, test_process=process, **kwargs): + return test_process + monkeypatch.setattr(asyncio, "create_subprocess_exec", create) + result = await CodexWorker().run(record(), tmp_path, emit, 1) + assert bool(result.configuration_error) is expected + + +@pytest.mark.parametrize("error", [ + {"code": "server_error", "message": "Model temporarily overloaded"}, + {"message": "Connection reset by peer"}, + {"message": "Invalid API key"}, + {"message": "{not json"}, +]) +def test_transient_and_unrelated_errors_are_not_model_configuration_errors(error): + from foreman.workers.codex_errors import model_configuration_error + assert model_configuration_error(error) is None