Skip to content
This repository was archived by the owner on Sep 2, 2026. It is now read-only.

Commit 9884b36

Browse files
committed
Add runtime-provider scrutiny synthesis
1 parent 74521f3 commit 9884b36

16 files changed

Lines changed: 406 additions & 0 deletions

.factory/library/architecture.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ This mission should extend the existing Rust workspace, not create a parallel sa
8686
- The facade crate stays thin; runtime ownership lives below it.
8787
- Stable role behavior belongs on `Agent`/`SandboxAgent`; request- and environment-specific execution belongs on `Runner`, `RunConfig`, and `SandboxRunConfig`, while `RunOptions` is reserved for per-call overrides rather than long-lived sandbox definition.
8888
- `RunState` remains the canonical public pause/resume boundary, even if sandbox uses dedicated nested session-state types internally.
89+
- When adding new default hooks or behavior to `ModelProvider`, audit wrapper providers such as `MultiProvider` and forward the new behavior explicitly so routed models preserve provider-specific metadata and request shaping.
8990
- Runner-created sandbox sessions are runner-managed; caller-injected live sessions remain caller-managed.
9091
- Sandbox file, shell, and patch operations must stay rooted to the sandbox workspace and must not escape onto the host filesystem.
9192
- Optional hosted backends must not force third-party provider dependencies into the core runtime crate.

.factory/library/environment.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Environment variables, external dependencies, and setup notes.
1212
- Network access to crates.io for `cargo fetch`, `cargo test` on cold machines, packaging, and publish verification.
1313
- Docker is currently unavailable in this environment; Docker-backed sandbox validation is blocked until that changes.
1414
- `rg` is currently unavailable in this environment; `docs/scripts/check_links.sh` depends on it today and must be fixed or shimmed during the mission.
15+
- Run `.factory/init.sh` before heavy mission work when you need a quick preflight: it performs `cargo fetch --locked` and warns if `rg` or Docker are unavailable in the current environment.
1516

1617
## Environment Variables
1718
- `OPENAI_API_KEY`: required for live OpenAI-backed example/integration validation.
@@ -26,3 +27,6 @@ Environment variables, external dependencies, and setup notes.
2627
- Do not touch or depend on local services already using ports such as `5000`, `7000`, `8790`, or `8791`.
2728
- Do not store credentials in repo files or commit generated secrets.
2829
- Do not assume Docker, ripgrep, provider CLIs, audio hardware, or hosted sandbox accounts exist unless the feature explicitly establishes that prerequisite.
30+
31+
## Validator Quirks
32+
- `cargo test <filter> -- --exact` does **not** match crate-local unit tests by bare test name alone; for `src/lib.rs` unit tests you need the fully qualified module path, or an integration test with a bare exact name if the mission contract expects a short filter.

.factory/services.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
commands:
2+
init: .factory/init.sh
23
install: cargo fetch --locked
34
typecheck: cargo check --workspace
45
build: cargo build --workspace --examples
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"featureId": "agent-tools-and-approval-flow-parity",
3+
"reviewedAt": "2026-04-16T16:03:14Z",
4+
"commitId": "325191a",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "pass",
8+
"codeReview": {
9+
"summary": "Pass. Commit `325191a` stays within the intended `agents-core` boundary and fixes the feature's exact-match verification gap by adding public-API integration coverage in `crates/agents-core/tests/tool_approval_semantics.rs:1-194`. The underlying approval resume implementation already enforces bound approvals before replaying the pending tool call in `crates/agents-core/src/run.rs:1059-1127`, and the existing facade coverage for nested agent-as-tool state/input preservation remains in `crates/openai-agents/tests/runner_semantics.rs:138-178`. I did not find correctness regressions or missing in-scope contract behavior in the reviewed change.",
10+
"issues": []
11+
},
12+
"sharedStateObservations": [
13+
{
14+
"area": "knowledge",
15+
"observation": "The mission shared state does not currently call out that `cargo test <filter> -- --exact` will not match crate-local unit tests unless the fully qualified module path is used. For parity features that publish short exact-name verification commands, workers may need integration tests or fully qualified test names to make the command execute real tests.",
16+
"evidence": "This feature's verification steps in `features.json:697-698` use short exact-name filters for `openai-agents-core-rs`. The handoff for commit `325191a` explicitly states the change was needed so those commands would execute real tests instead of filtering down to zero unit tests, and the worker added `crates/agents-core/tests/tool_approval_semantics.rs` for that purpose."
17+
}
18+
],
19+
"addressesFailureFrom": null,
20+
"summary": "Reviewed the feature entry, handoff, transcript skeleton, commit `325191a`, and the relevant runtime/facade code paths. The feature passes review: approval-gated resume semantics remain correctly bound in core, the facade nested agent-as-tool parity coverage is still present, and the new integration test target makes the milestone's exact verification commands map to real tests."
21+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"featureId": "mcp-discovery-execution-and-safety-parity",
3+
"reviewedAt": "2026-04-16T16:02:28.184034Z",
4+
"commitId": "502f9bcc0a2170c94346f998f558823fdbb561f2",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "pass",
8+
"codeReview": {
9+
"summary": "The implementation adds local preflight validation for discovered MCP tool arguments in agents-core before remote execution, preserves approval gating and namespace metadata, and the surrounding MCP semantics tests still cover non-streamed execution, reconnect cleanup, resource connection gating, and sanitized failure outputs.",
10+
"issues": []
11+
},
12+
"sharedStateObservations": [],
13+
"addressesFailureFrom": null,
14+
"summary": "Reviewed the feature handoff, transcript skeleton, commit 502f9bcc0a2170c94346f998f558823fdbb561f2, and the affected MCP code/tests. The feature matches the contract and I did not find blocking or non-blocking correctness issues in the reviewed scope."
15+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"featureId": "openai-hosted-tool-helper-and-replay-parity",
3+
"reviewedAt": "2026-04-16T16:03:26.083506+00:00",
4+
"commitId": "6b6fe4c",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "fail",
8+
"codeReview": {
9+
"summary": "Fail. The commit adds the expected hosted-tool replay and websocket parity regressions, and the public facade still exposes the five hosted helper constructors. But the core change now sanitizes hosted tool artifacts on every `prepare_model_input_items()` call, not just replay/resume input. Because normal run preparation in `crates/agents-core/src/run.rs:557-560` uses this helper, fresh caller history can silently lose completed hosted tool items without matching outputs. That is broader than the feature contract's replay-only sanitation, and it diverges from the upstream split where only resumed/generated history is pruned while caller items are preserved.",
10+
"issues": [
11+
{
12+
"file": "crates/agents-core/src/internal/items.rs",
13+
"line": 45,
14+
"severity": "blocking",
15+
"description": "`compose_replay_input_items()` now unconditionally calls `sanitize_hosted_tool_replay_items(base_items)`, and `prepare_model_input_items()` routes ordinary turn input through this path. As a result, non-resume runs can drop caller-supplied completed hosted tool items such as `shell_call` or `tool_search_call` when their outputs are absent, even though the feature only called for replay sanitation. Upstream keeps normal caller items intact and only prunes resumed/generated history, so this changes live run semantics rather than just fixing replay behavior."
16+
}
17+
]
18+
},
19+
"sharedStateObservations": [
20+
{
21+
"area": "skills",
22+
"observation": "`rust-runtime-worker` currently requires a failing-test-first flow even when part of the assigned parity slice is already implemented and the remaining work is to add exact regression coverage. The worker deviated for that reason, and the handoff's suggested skill update appears justified.",
23+
"evidence": "Handoff `skillFeedback.followedProcedure` is false only for step 3 ('Write failing tests first'), with the explanation that nearby websocket behavior was already green and the worker added the missing exact regression coverage plus targeted validation instead. The transcript skeleton also shows the worker otherwise followed the required mission/library-reading procedure."
24+
}
25+
],
26+
"addressesFailureFrom": null,
27+
"summary": "Reviewed the feature handoff, transcript skeleton, commit 6b6fe4c, and the relevant core/openai files. The feature does not pass scrutiny because the replay sanitizer is wired into normal model-input preparation, so it can drop caller-provided hosted tool history outside resume/replay flows."
28+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"featureId": "provider-routing-transport-and-request-metadata",
3+
"reviewedAt": "2026-04-16T16:02:24.966938Z",
4+
"commitId": "c8efbc9",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "pass",
8+
"codeReview": {
9+
"summary": "Pass. The feature stays within the intended crate boundaries and adds precise regression coverage for the three assigned parity assertions without uncovering correctness or contract gaps in the underlying routing and OpenAI transport/request-shaping code.",
10+
"issues": []
11+
},
12+
"sharedStateObservations": [],
13+
"addressesFailureFrom": null,
14+
"summary": "Reviewed handoff, transcript skeleton, commit c8efbc9 diff, and the relevant runtime/provider code paths. The feature correctly validates MultiProvider prefix routing in agents-core and OpenAI Responses transport isolation plus websocket metadata/header/query/conversation forwarding in agents-openai; I did not find blocking or non-blocking issues in scope."
15+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
{
2+
"featureId": "realtime-session-and-transport-parity",
3+
"reviewedAt": "2026-04-16T16:04:56.440710+00:00",
4+
"commitId": "8dfb31c26fc9fdc81d5d180833df56c047da869f",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "fail",
8+
"codeReview": {
9+
"summary": "Fail. The commit adds the requested facade regressions and correctly preserves previously-applied non-null realtime settings across partial updates, but the new merge-based implementation still misses an important part of the parity contract: callers cannot explicitly clear previously-applied optional settings, and mixed flat-vs-nested audio updates can leave the public session state internally inconsistent with the payload the model actually sees.",
10+
"issues": [
11+
{
12+
"file": "crates/agents-realtime/src/config.rs",
13+
"line": 190,
14+
"severity": "blocking",
15+
"description": "`RealtimeSessionModelSettings::merge()` and its nested helpers always keep the old value whenever the update field is `None` (for example `update.voice.clone().or_else(|| self.voice.clone())`, `update.speed.or(self.speed)`, and the nested `current.merge(next)` branches at lines 20-168). That means a caller cannot explicitly clear a previously-applied setting such as `voice`, `speed`, `turn_detection`, `transcription`, `noise_reduction`, or `tracing` once it has been set. The feature description says prior settings should be preserved unless explicitly replaced; after this change, replacing a field with 'unset/disabled' is impossible, which is a parity gap versus the Python session-update surface that accepts explicit `None`/null values in its settings dictionaries."
16+
},
17+
{
18+
"file": "crates/agents-realtime/src/session.rs",
19+
"line": 289,
20+
"severity": "non_blocking",
21+
"description": "`update_agent()` stores the merged settings object verbatim in `state.model_settings`, but `session_payload_from_settings()` later resolves audio values by preferring nested `audio.output.*` fields over the legacy flat aliases (`voice`, `speed`, `output_audio_format`) at `crates/agents-realtime/src/openai_realtime.rs:211-222`. If the existing state was configured through the flat aliases and a later partial update uses the nested `audio` shape (or vice versa), `session.model_settings()` can report stale flat alias values even though the payload actually sent to the model uses the newer nested values. Given the docs explicitly support both representations, the public session state should stay normalized instead of exposing contradictory values."
22+
}
23+
]
24+
},
25+
"sharedStateObservations": [
26+
{
27+
"area": "skills",
28+
"observation": "The `rust-runtime-worker` skill still claims workers should add failing tests before implementation, but this worker's transcript shows code edits landed before any of the exact parity tests were executed, while the handoff still reported `followedProcedure: true`.",
29+
"evidence": "The transcript skeleton for session `c4810726-24ef-45dd-bac9-7f55150e7099` shows multiple `ApplyPatch` calls before the first `cargo test -p openai-agents-rs --test realtime_semantics ... --exact` executions, yet the handoff JSON marks `skillFeedback.followedProcedure` as true with no deviations."
30+
},
31+
{
32+
"area": "services",
33+
"observation": "Workers appear to treat `.factory/init.sh` as a standard preflight step for runtime-provider work, but that command is not captured in `.factory/services.yaml` alongside the other baseline validation/setup commands.",
34+
"evidence": "This feature's transcript skeleton includes an early Execute call for `/Users/staticpayload/Mainframe/openai-agents-rust/.factory/init.sh`, while `.factory/services.yaml` only lists install/typecheck/build/test/lint/docs/package commands and no init/preflight entry."
35+
}
36+
],
37+
"addressesFailureFrom": null,
38+
"summary": "Reviewed the feature metadata, handoff, transcript skeleton, skill file, mission guidance, and commit `8dfb31c26fc9fdc81d5d180833df56c047da869f`. The feature does not pass scrutiny because the new merge semantics preserve prior non-null values but still cannot express explicit clearing of realtime settings, and mixed flat/nested audio updates can leave the public session state inconsistent with the applied model payload."
39+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"featureId": "runtime-conversation-tracking-and-session-input",
3+
"reviewedAt": "2026-04-16T16:03:21Z",
4+
"commitId": "f8f58af",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "fail",
8+
"codeReview": {
9+
"summary": "Fail. The commit usefully preserves typed `AgentsError` values for streamed completions (`crates/agents-core/src/internal/streaming.rs`) and adds focused regressions for conversation delta replay, automatic previous-response chaining, session conflict handling, replay limits, and callback-based history rewriting (`crates/agents-core/src/run.rs`). However, the feature is still incomplete for `session_input_callback`: the session preparation path force-restores the original input whenever the callback returns an empty list, so callers cannot intentionally drop replay input and the session store will persist the dropped new-turn items anyway. That still misses the contract's requirement that callback rewrites be deterministic and not corrupt persisted items.",
10+
"issues": [
11+
{
12+
"file": "crates/agents-core/src/internal/session_persistence.rs",
13+
"line": 111,
14+
"severity": "blocking",
15+
"description": "After `session_input_callback` runs, `prepare_input_with_session()` overwrites an empty callback result with `original_input` (`prepared = original_input.clone(); session_input_items = original_input.clone();`). A callback that intentionally returns `[]` therefore cannot suppress replay input, and the supposedly dropped new-turn items are still appended back into the session store. Upstream parity keeps the callback's empty result instead of restoring the original input, so this feature still does not fully satisfy the `session_input_callback` rewrite contract."
16+
}
17+
]
18+
},
19+
"sharedStateObservations": [
20+
{
21+
"area": "skills",
22+
"observation": "The `rust-runtime-worker` skill requires one manual shell verification when public runtime behavior changes, but this feature handoff/transcript only recorded automated Cargo checks and no manual smoke step.",
23+
"evidence": "`.factory/skills/rust-runtime-worker/SKILL.md` step 6 says to perform one manual shell verification for user-visible public-facade behavior. The handoff for session `7183a076-d165-4821-a3b0-e3688019b350` lists only `commandsRun`, and the transcript skeleton shows cargo test/check/fmt commands but no temp-crate/example/manual smoke execution."
24+
}
25+
],
26+
"addressesFailureFrom": null,
27+
"summary": "Reviewed the mission artifacts, transcript skeleton, and commit `f8f58af`. The conversation-tracking and streamed-session error work is mostly solid, but the feature should not pass yet because empty `session_input_callback` rewrites are still overridden back to the original input and persisted session items."
28+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"featureId": "runtime-default-runner-and-sync-semantics",
3+
"reviewedAt": "2026-04-16T16:02:24.033520+00:00",
4+
"commitId": "59fc07b",
5+
"transcriptSkeletonReviewed": true,
6+
"diffReviewed": true,
7+
"status": "pass",
8+
"codeReview": {
9+
"summary": "Pass. The commit correctly aligns the parity verification surface with the already-implemented runtime behavior. `agents_core::run()` and `openai_agents::run()` still resolve through the configurable default runner (`crates/agents-core/src/run.rs:1345-1395` and `crates/openai-agents/src/lib.rs:2-76`), and `Runner::run_sync()` still rejects active Tokio runtimes with a user-facing error (`crates/agents-core/src/run.rs:136-161`). The new integration test in `crates/agents-core/tests/run_sync_semantics.rs:1-16` closes the exact-name gap in the feature's verification command, while the facade test rename in `crates/openai-agents/tests/runner_semantics.rs:66-88` matches the contract wording without regressing behavior.",
10+
"issues": []
11+
},
12+
"sharedStateObservations": [
13+
{
14+
"area": "knowledge",
15+
"observation": "The mission shared state does not document that `cargo test <filter> -- --exact` will not match crate-local unit tests unless the fully qualified module path is used. This feature's verification step used a short exact name, so the worker had to discover the mismatch and add an integration test with a bare test name to make the contract command executable as written.",
16+
"evidence": "Feature verification step at features.json:486 is `cargo test -p openai-agents-core-rs run_sync_rejects_active_runtime -- --exact`. The worker transcript shows that after the first targeted run they inspected `cargo test ... --list`, checked the crate layout, and then added `crates/agents-core/tests/run_sync_semantics.rs` so the exact filter would match."
17+
}
18+
],
19+
"addressesFailureFrom": null,
20+
"summary": "Reviewed handoff, transcript skeleton, commit 59fc07b, and the relevant runtime/facade files. The feature passes review: it preserves the intended default-runner and active-runtime semantics while making the milestone's exact verification commands map cleanly onto real targeted tests."
21+
}

0 commit comments

Comments
 (0)