diff --git a/agent-contract/index.md b/agent-contract/index.md index 7242d449..ac517ce3 100644 --- a/agent-contract/index.md +++ b/agent-contract/index.md @@ -13,7 +13,7 @@ | id | 읽는 조건 | 원본 경로 | path | |----|-----------|-----------|------| | `iop.openai-compatible-api` | OpenAI-compatible API, Responses API, Chat Completions, legacy Completions, error envelope/SSE terminal error, `model` route, managed projection principal auth and slot-route binding, managed-versus-legacy provider credential selection, model-driven passthrough/normalized routing, provider-pool admission/unavailable error, Gemini Chat thought-signature tool continuation, safe credential-slot attribution, standard metadata, and provider-native extension fields such as `chat_template_kwargs` | `apps/edge/internal/openai/*`, `apps/edge/internal/authprojection/*`, `apps/edge/internal/service/provider_tunnel.go`, `packages/go/config/config.go`, `configs/edge.yaml` | `agent-contract/outer/openai-compatible-api.md` | -| `iop.anthropic-compatible-api` | Anthropic Messages API, count_tokens, models list, bearer or `X-Api-Key` principal auth, active managed projection auth and slot-route binding, `anthropic-version` routing, native Anthropic tunnel, Chat bridge, provider-pool-only admission, profile capability checks, managed-versus-legacy provider credentials, marked-preset single-request admission with Edge-owned internal Plan/Review template customization that leaves caller I/O unchanged, and current no-OpenAI-metric status | `apps/edge/internal/openai/anthropic_handler.go`, `apps/edge/internal/openai/anthropic_native.go`, `apps/edge/internal/openai/anthropic_bridge.go`, `apps/edge/internal/openai/anthropic_stream.go`, `apps/edge/internal/openai/anthropic_types.go`, `apps/edge/internal/openai/routes.go`, `apps/edge/internal/openai/principal.go`, `apps/edge/internal/authprojection/*`, `apps/edge/internal/openai/provider_tunnel.go`, `apps/edge/internal/openai/provider_model_rewrite.go`, `apps/edge/internal/openai/single_request_preset_binding.go`, `apps/edge/internal/openai/single_request_plan_stage.go`, `apps/edge/internal/openai/single_request_review_stage.go`, `packages/go/singlerequesttemplate/template.go`, `packages/go/config/protocol_profile.go` | `agent-contract/outer/anthropic-compatible-api.md` | +| `iop.anthropic-compatible-api` | Anthropic Messages API, count_tokens, models list, bearer or `X-Api-Key` principal auth, active managed projection auth and slot-route binding, `anthropic-version` routing, native Anthropic tunnel, Chat bridge, provider-pool-only admission, profile capability checks, managed-versus-legacy provider credentials, unmarked caller-workspace light continuation, marked-preset single-request admission with Edge-owned internal Plan/Review template customization that leaves caller I/O unchanged, and current no-OpenAI-metric status | `apps/edge/internal/openai/anthropic_handler.go`, `apps/edge/internal/openai/anthropic_native.go`, `apps/edge/internal/openai/anthropic_bridge.go`, `apps/edge/internal/openai/anthropic_stream.go`, `apps/edge/internal/openai/anthropic_types.go`, `apps/edge/internal/openai/routes.go`, `apps/edge/internal/openai/principal.go`, `apps/edge/internal/authprojection/*`, `apps/edge/internal/openai/provider_tunnel.go`, `apps/edge/internal/openai/provider_model_rewrite.go`, `apps/edge/internal/openai/artifact_pair.go`, `apps/edge/internal/openai/hot_path_light.go`, `apps/edge/internal/openai/hot_path_review.go`, `apps/edge/internal/openai/single_request_preset_binding.go`, `apps/edge/internal/openai/single_request_plan_stage.go`, `apps/edge/internal/openai/single_request_review_stage.go`, `packages/go/singlerequesttemplate/template.go`, `packages/go/config/protocol_profile.go` | `agent-contract/outer/anthropic-compatible-api.md` | | `iop.gemini-compatible-api` | Gemini Developer API `streamGenerateContent`, route-qualified Gemini-native ingress, `x-goog-api-key` principal auth, `GOOGLE_GEMINI_BASE_URL`, official agy 1.1.12 API-key transport, Gemini function calls/thought signatures/SSE, and direct-versus-execution-preset binding | `apps/edge/internal/openai/routes.go`, `apps/edge/internal/openai/principal.go`, `apps/edge/internal/openai/gemini_handler.go`, `apps/edge/internal/openai/gemini_bridge.go`, `apps/edge/internal/openai/gemini_types.go` | `agent-contract/outer/gemini-compatible-api.md` | | `iop.a2a-json-rpc-api` | A2A JSON-RPC API, `message/send`, `tasks/get`, `tasks/cancel`, A2A task state, agent card, `a2a.bearer_token`, Edge A2A input surface | `apps/edge/internal/input/a2a/*`, `packages/go/config/config.go`, `configs/edge.yaml` | `agent-contract/outer/a2a-json-rpc-api.md` | diff --git a/agent-contract/outer/anthropic-compatible-api.md b/agent-contract/outer/anthropic-compatible-api.md index edb10c35..de48c1e0 100644 --- a/agent-contract/outer/anthropic-compatible-api.md +++ b/agent-contract/outer/anthropic-compatible-api.md @@ -22,6 +22,10 @@ - `apps/edge/internal/openai/single_request_work_stage.go` - `apps/edge/internal/openai/single_request_review_stage.go` - `apps/edge/internal/openai/provider_normalization.go` + - `apps/edge/internal/openai/artifact_pair.go` + - `apps/edge/internal/openai/hot_path_light.go` + - `apps/edge/internal/openai/hot_path_review.go` + - `apps/edge/internal/openai/hot_path_stage_input.go` - `packages/go/singlerequesttemplate/template.go` - `apps/edge/internal/openai/anthropic_types.go` - `apps/edge/internal/openai/routes.go` @@ -85,6 +89,39 @@ across the native Messages tunnel and Chat bridge. Ordinary native routes preser provider response model and body bytes; the Chat bridge emits its converted Anthropic response model semantics. +### Unmarked light preset: caller-workspace continuation + +An unmarked `light` preset with `workspace_tools` uses only the tool schemas +admitted by the caller request. Edge never receives or configures a raw caller +workspace path. Before issuing caller writes, Edge validates the selector's +default lightweight PLAN and pending REVIEW seed and retains the PLAN `P1..Pn` +inventory request-locally. Work must read PLAN, perform and verify caller-workspace +work, and replace REVIEW with a completed handoff covering every PLAN item. Review +must read both artifacts and successfully inspect an ordinary caller result before +it can return a non-empty caller-visible terminal. It never rewrites REVIEW; repair +and re-verification remain in the same Review binding. Cleanup targets only the +request's `.iop/job/` directory. + +After Edge issues the request-local artifact paths, it adds one bounded Edge-owned, +phase-specific selector instruction to the actual provider body. A binding whose +write operation cannot create parents receives a `prepare-only` initial instruction: +the provider must emit exactly one admitted prepare call for +`.iop/job/` and must not write PLAN or REVIEW. After the successful +prepare receipt, the resumed selector receives `pair-write` and must emit exactly +the two admitted writes for PLAN and REVIEW with the accepted default PLAN grammar +and deterministic pending REVIEW grammar. A parent-creating write binding receives +`pair-write` immediately on its initial selector turn. In Chat Completions this is +the final leading `system` message; in native Messages it is the final top-level +`system` text block. The instruction is not caller input, does not alter the retained +caller request snapshot, and adds no public request/config field or raw workspace +path. Work, Review, cleanup, and marked `single_request` provider bodies do not +receive it. + +A preset marked with `single_request` is excluded from this continuation before +caller tool binding. Config validation rejects a preset that declares both +`single_request` and `workspace_tools`, so the operator-owned Node workspace and +caller-owned workspace modes cannot cross or fall back into one another. + ### Marked preset: single-request admission An authorized fixed single-request preset compiles one service-owned admission value diff --git a/agent-spec/input/openai-compatible-surface.md b/agent-spec/input/openai-compatible-surface.md index 3b6f862e..b6278b7c 100644 --- a/agent-spec/input/openai-compatible-surface.md +++ b/agent-spec/input/openai-compatible-surface.md @@ -15,6 +15,18 @@ source_evidence: - type: code path: apps/edge/internal/openai/chat_handler.go notes: Chat Completions request validation, route dispatch, tool/reasoning 정책 + - type: code + path: apps/edge/internal/openai/hot_path_light.go + notes: unmarked light preset의 caller-workspace PLAN→Work REVIEW handoff→Review inspection/repair 상태 + - type: code + path: apps/edge/internal/openai/provider_model_rewrite.go + notes: unmarked selector의 frontier에 따라 prepare-only 또는 pair-write relative-path grammar를 실제 Chat/Messages provider body에 주입하는 경계 + - type: test + path: apps/edge/internal/openai/hot_path_light_test.go + notes: OpenAI/Anthropic caller-workspace handoff, reviewer inspection, repair, terminal, cleanup 회귀 검증 + - type: test + path: apps/edge/internal/openai/hot_path_direct_test.go + notes: Chat/Messages selector body의 prepare-only/pair-write 상태 결속과 swapped-operation 거부 회귀 검증 - type: code path: apps/edge/internal/openai/gemini_handler.go notes: Gemini-native request validation과 official agy tool continuation의 Chat bridge 변환 @@ -196,6 +208,7 @@ Edge가 OpenAI-compatible HTTP 요청을 받아 내부 `adapter + target` 실행 | managed projection auth | `credential_plane.enabled=true` uses the fresh Control Plane projection for inbound token auth and principal route discovery. Static principal/bearer fallback is disabled. | | managed slot route | Public model id/alias resolves to one projected route, exact slot/profile/upstream model/resource selector, and immutable revisions/generation. Unknown, cross-principal, stale, revoked, or ambiguous bindings fail closed. | | marked preset single-request admission | An authorized fixed single-request preset compiles one service-owned admission value at request start: requested public model, canonical plan/work/review bindings resolved through managed authorization, opaque workspace capability, and absolute resource caps. Later refresh cannot mutate the admitted shape. No private binding is echoed to the caller. Compiled only after every canonical reference is verified through its catalog binding for the authenticated principal; missing, duplicate, unauthorized, dynamically selected, or option-inconsistent inputs are rejected without fallback. | +| unmarked caller-workspace light route | An unmarked `light` preset binds only admitted caller `workspace_tools`; it never accepts a raw caller path. Edge derives a phase-specific selector instruction from the locked artifact frontier. A non-parent-creating write binding first receives `prepare-only` for exactly one admitted `.iop/job/` prepare call with PLAN/REVIEW writes prohibited; its successful receipt resumes the same selector with `pair-write` for exactly the PLAN/REVIEW writes and accepted grammar. A parent-creating binding receives `pair-write` immediately. The instruction is the final leading Chat `system` message or final Anthropic top-level `system` text block, does not mutate the retained caller snapshot or add a public field, and is absent from Work, Review, cleanup, and marked `single_request` bodies. The selector's lightweight PLAN and pending REVIEW seed are validated before caller writes. Work reads PLAN, executes/verifies in the caller workspace, and replaces REVIEW with a completed `P1..Pn` handoff. Review reads both artifacts, successfully inspects an ordinary caller result, repairs/re-verifies in the same binding when needed, and owns the non-empty final output without rewriting REVIEW. Cleanup removes only the request `.iop/job/` directory. Marked `single_request` presets bypass this route and retain the operator-owned Node workspace. | | marked single-request provider normalization | Plan/Work/Review derive caller-neutral effort/tool/structured-output requirements and let the selected protocol profile choose Chat Completions or Responses. Effort exact misses fall only to the nearest declared lower grade (`max` → `xhigh` when `max` is absent). Explicit resource selectors keep exact provider-ID verification; a `default` selector leaves provider choice to the pool while model group, profile, upstream model, credential slot/revision, and tunnel path remain frozen. Both Chat and Responses results are converted into the private common Chat-shaped stage codec before Plan/Work/Review validation. Chat conversion discards only bounded standard/provider bookkeeping (`service_tier`, `system_fingerprint`, provider `timings`, choice `logprobs`, message `annotations`, null `refusal`) and rejects a non-null refusal or unknown/duplicate fields. | | marked single-request internal templates | The admission also freezes the operator-configured effective Plan/Review Markdown templates. They are internal artifact shapes only: the Plan stage first performs a bounded read/list-only workspace inspection and requires one successful result in the same request-local conversation, then obtains a strict one-line `goal` plus bounded one-line `steps`/`verification` arrays. Edge owns the bullet formatting and renders `plan.md`; no separate analysis artifact is created. The Review template shapes the private `review.md` artifact. Callers cannot supply, name, or select a template, and template paths, contents, and digests never appear in a response, error, log, or metric label. The caller-visible Messages request/response schema is unchanged and the final text stays the model's `decision.output`. | | marked single-request ingress | One validated and authorized Messages POST enters the separate service coordinator capability before legacy provider/caller continuation and increments `iop_anthropic_single_request_ingress_total` once. Non-streaming returns one buffered final-only message. Streaming keeps one envelope across the coordinator lifetime, exposes only fixed plan/work/review/repair text blocks plus `event: ping`, and commits one final text/error terminal. Internal reasoning/tool wire never becomes caller `tool_use`; success is acknowledged only after the complete terminal write succeeds. | @@ -357,6 +370,9 @@ sequenceDiagram ## 변경 기록 +- 2026-08-14: Split the unmarked selector instruction into frontier-derived `prepare-only` and `pair-write` operations. Non-parent-creating bindings prepare the request job directory first and receive the exact artifact-pair grammar only after the successful receipt; parent-creating bindings receive the pair operation immediately. +- 2026-08-14: Added the Edge-owned provider-side selector instruction for unmarked caller-workspace initial/resume turns. Actual Chat/Messages bodies now carry the exact request-local relative paths and accepted PLAN/pending REVIEW grammar without changing caller snapshots or public schemas. +- 2026-08-14: Added the restored unmarked caller-workspace light route, including template-validated PLAN/pending REVIEW creation, worker-owned completed REVIEW handoff, reviewer reads/result inspection/repair, reviewer-owned non-empty terminal, and strict separation from marked Node-owned `single_request` presets. - 2026-08-14: Added bounded read/list-first workspace analysis inside the marked Plan stage. Successful inspection is required before structured Plan completion, while mutation tools, a separate analysis artifact, and caller-visible schema changes remain absent. - 2026-08-14: Synchronized marked single-request artifact-only PLAN→Work→REVIEW→Review handoff, reviewer-owned repair/re-verification, reviewer zero-write, and strict terminal output provenance. - 2026-08-14: Added operation-scoped `normalization.tool_calls` and Gemini-only Chat thought-signature round trips across standard OpenAI-compatible callers, including non-stream, SSE, and recovery-selected dispatches. Effort mapping and caller identity remain independent (`packages/go/config/protocol_profile.go`, `apps/edge/internal/openai/provider_model_rewrite.go`). diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log new file mode 100644 index 00000000..5caf001a --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log @@ -0,0 +1,182 @@ + + +# Code Review Reference - REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> Complete the implementation-owned sections, leave the active pair in place, and report ready for review. Finalization, log renames, `complete.log`, and verdict routing are review-agent-only. + +## Overview + +date=2026-08-14 +task=hybrid_caller_workspace_repair, plan=1, tag=REVIEW_REFACTOR + +## Archive Evidence Snapshot + +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G08_0.log`: the original repair separated marked Node-owned execution from unmarked caller-workspace execution and required an exact PLAN → worker REVIEW → reviewer flow. +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log`: official review verdict `FAIL`; Required R1 found that all fresh deterministic tests passed while the real provider never received the generated paths or required grammar because the scripted provider read internal `Run.Metadata` out of band. + +## For the Review Agent + +Run applicable verification fresh, append the verdict and routing signals, then archive this file to `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log` and the plan to `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G07_1.log`. PASS writes `complete.log` and finishes the task per the code-review skill; WARN/FAIL writes the next required active state. This task is not Milestone-linked. + +## Implementation Item Completion + +| Item | Status | +|---|---| +| FIX-R1 Add the provider-bound selector instruction without changing caller snapshots or marked execution. | [x] | +| TEST-R1 Remove the test metadata privilege and prove both real protocol bodies. | [x] | +| DOC-R1 Synchronize contract and implementation spec. | [x] | + +## Implementation Checklist + +- [x] [FIX-R1] Add and apply the bounded selector provider instruction exactly as planned. +- [x] [TEST-R1] Derive scripted selector output only from actual provider-bound bodies and add protocol/turn/marked regressions. +- [x] [DOC-R1] Update the matching contract and implementation spec. +- [x] Fill implementation-owned sections in this review file with actual notes and command output. + +## Review-Only Checklist + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure`. +- [x] Verify verdict, dimensions, and finding classifications are consistent. +- [x] Run applicable verification fresh and record output. +- [x] Record Evidence, Root Cause, and one Selected Fix for every Required/Suggested finding before any follow-up plan. +- [x] Archive this review to `code_review_cloud_G08_1.log`. +- [x] Archive `PLAN-cloud-G07.md` to `plan_cloud_G07_1.log`. +- [x] Verify the Agent-Ops `.gitignore` managed block. +- [ ] If PASS, write `complete.log`, leave no active pair, and archive the active task directory as required. +- [x] If WARN/FAIL, write the next filesystem state and do not write `complete.log`. + +## Deviations from Plan + +Added `apps/edge/internal/openai/hot_path_chat_gate_test.go` to the test changes. The final boundary search found that this adjacent scripted selector fixture still derived its response from `Run.Metadata`; it now materializes the same provider tunnel body and derives the request-local path from that body like the two planned fixture files. No production scope was added. + +## Key Design Decisions + +- The instruction is built only while the Edge-owned artifact frontier for the trusted logical request is retained. Initial selector and prepare-resume requests retain that frontier; local/review/cleanup requests bypass the ordinary selector pool, and marked `single_request` never pins it. +- Provider preparation runs first. The instruction is then injected according to the selected wire operation: the last leading Chat `system` message or the final Anthropic top-level `system` text block. Existing system authority and unknown top-level caller fields remain preserved in the provider-only body; the ingress snapshot is unchanged. +- The bounded instruction contains only the two issued relative paths and an accepted deterministic two-item instance of the default PLAN/pending REVIEW grammar. It contains no raw caller workspace path and introduces no public schema/config field. +- Scripted selector services now call `PrepareProtocolTunnel` and `BuildBody`, parse the actual selected protocol body, validate the instruction grammar, and derive the request ID from its relative path. Later light-stage fixtures derive paths from their actual provider prompt, never selector metadata. + +## Reviewer Checkpoints + +- Actual provider-bound bodies carry exact request-local paths and accepted grammar without test-only metadata. +- Chat and Anthropic preserve existing caller/system content and canonical shapes. +- Initial/resume selector turns are covered; later phases and marked `single_request` are unchanged. +- No raw workspace path or new public API/config field appears. +- Existing handoff, inspection/repair, terminal, and cleanup invariants stay green. + +## Verification Results + +### Formatting and focused tests + +```bash +gofmt -w apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/chat_handler.go apps/edge/internal/openai/anthropic_handler.go apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_direct_test.go +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +``` + +```text +ok iop/apps/edge/internal/openai 0.155s +``` + +### Package and repository + +```bash +go test -count=1 ./apps/edge/internal/openai +go test -count=1 ./... +``` + +```text +ok iop/apps/edge/internal/openai 8.666s + +Repository suite: exit 0. All Go packages passed uncached, including: +ok iop/apps/edge/internal/openai 9.105s +ok iop/apps/edge/internal/service 8.310s +ok iop/apps/node/internal/workspace 1.067s +ok iop/packages/go/singlerequesttemplate 0.015s +ok iop/packages/go/streamgate 0.902s +``` + +### Boundary search and diff + +```bash +rg --sort path -n 'iop_logical_request_id|scriptedArtifactPair|selector.*instruction|pending REVIEW' apps/edge/internal/openai agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +git diff --check +``` + +```text +Expected selector-boundary matches include: +apps/edge/internal/openai/provider_model_rewrite.go:21: instruction := fmt.Sprintf(`IOP caller-workspace selector instruction... +apps/edge/internal/openai/provider_model_rewrite.go:36: The pending REVIEW content must be exactly this deterministic seed: +apps/edge/internal/openai/request_identity_ingress.go:23: requestID := strings.TrimSpace(metadata["iop_logical_request_id"]) +agent-contract/outer/anthropic-compatible-api.md:106: selector instruction to the actual provider body for the initial selector and its +agent-spec/input/openai-compatible-surface.md:208: ... bounded selector-only provider instruction ... + +The targeted privilege check +`rg -n 'req\.Run\.Metadata\["iop_logical_request_id"\]' apps/edge/internal/openai/hot_path_direct_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_chat_gate_test.go` +produced no output. `git diff --check` also produced no output; both commands exited 0. +``` + +### Live OpenCode smoke pause + +Do not execute during this implementation. Resume condition remains `dev deployment complete and user explicitly resumes live call`. + +### Reviewer fresh verification + +```bash +go version +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go test -count=1 ./apps/edge/internal/openai +go vet ./apps/edge/internal/service +go test ./apps/edge/internal/service -count=1 +go vet ./packages/go/... +go test ./packages/go/... -count=1 +go test -count=1 ./... +rg -n 'req\.Run\.Metadata\["iop_logical_request_id"\]' apps/edge/internal/openai/hot_path_direct_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_chat_gate_test.go +git diff --check +``` + +```text +go version go1.26.2 linux/arm64 +ok iop/apps/edge/internal/openai 0.184s +ok iop/apps/edge/internal/openai 8.846s +edge_service_vet_exit=0 +ok iop/apps/edge/internal/service 8.263s +packages_vet_exit=0 +ok iop/packages/go/... (all packages; no test files where applicable) +ok iop/... (all repository packages; no test files where applicable) +targeted metadata privilege search: no output, exit 1 (no match) +git diff --check: no output, exit 0 +``` + +Fresh tests pass, but source-level contract tracing found that the provider instruction contradicts the retained prepare frontier. `artifact_pair.go:201-205,272-280` requires an initial selector with a non-parent-creating write binding to emit exactly one prepare call. `provider_model_rewrite.go:21` instead directs that same initial provider body to emit exactly two write calls. The scripted provider does not execute the body instruction: `hot_path_direct_test.go:207-214,305` chooses prepare or pair from the test call index, so its passing result does not prove a real provider can follow the required stage-specific operation. + +--- + +## Section Ownership + +| Section | Owner | +|---|---| +| Fixed header, Overview, archive snapshot, reviewer instructions/checkpoints | Stub creation; read-only for implementer | +| Implementation completion/checklist, deviations, decisions, verification results | Implementing agent | +| Review-only checklist and appended Code Review Result | Review agent only | + +## Code Review Result + +- Overall Verdict: FAIL +- Dimension Assessment: + - Correctness: Fail + - Completeness: Fail + - Test coverage: Fail + - API contract: Fail + - Code quality: Pass + - Implementation deviation: Fail + - Verification trust: Fail +- Findings: + - Required R1 — The provider-visible instruction requires the wrong operation on the initial selector turn when the admitted write tool cannot create parent directories. + - Evidence: Fresh focused, package, vet, and repository tests all pass, but direct source tracing proves the contradiction. `apps/edge/internal/openai/provider_model_rewrite.go:21` tells every retained selector to return exactly two write calls. `apps/edge/internal/openai/artifact_pair.go:201-205,272-280` requires the initial `artifactPhasePinned` turn to return exactly one `prepare` call whenever `record.binding.createsParents()` is false. The handler regression fixture configures exactly that case (`hot_path_direct_test.go:440-442`) yet `scriptedArtifactPoolService` ignores instruction semantics and selects `scriptedArtifactPrepare` for call 1 and `scriptedArtifactPair` for call 2 by index (`hot_path_direct_test.go:207-214,305`). A real provider following the initial body receives an impossible instruction and its two writes are rejected as `artifact prepare turn must contain exactly one call`. + - Root Cause: `hotPathSelectorProviderInstruction` exposes only frontier existence and `buildHotPathSelectorProviderInstruction` accepts only the request ID. The instruction builder therefore cannot distinguish an initial prepare-only frontier from a pair-ready selector resume. The new test seam verifies paths/grammar but never verifies that the provider response is derived from the instruction's required operation. + - Selected Fix: Add a lock-safe artifact-frontier instruction state that distinguishes `prepare-only` from `pair-write`. For an initial pinned frontier whose write operation cannot create parents, build an instruction requiring exactly one prepare call for `.iop/job/` and no writes. After the successful prepare receipt changes the frontier to `artifactPhasePairReady`, build the existing exact two-write PLAN/REVIEW instruction. A parent-creating initial binding may receive the pair-write instruction immediately. Update the Chat and Anthropic scripted handler regression so its response operation is selected from the actual provider-bound instruction, not call order, and assert prepare-only and pair-write instructions cannot be swapped. Synchronize the Anthropic contract and living spec with this phase-specific instruction contract. Acceptance: fresh focused selector/artifact/hot-path tests, Edge OpenAI package, service/shared vet and tests, the full repository suite, deterministic instruction/fixture search, targeted metadata privilege search, and `git diff --check` pass. +- Routing Signals: + - review_rework_count=2 + - evidence_integrity_failure=true +- Next Step: Implement the closed phase-specific selector-instruction fix from the routed follow-up PLAN; do not execute the paused live OpenCode smoke. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_2.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_2.log new file mode 100644 index 00000000..54508bce --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G08_2.log @@ -0,0 +1,155 @@ + + +# Code Review Reference - REVIEW_REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> Complete implementation-owned sections and stop with the active pair in place. Execute the selected fix as written; do not change ownership or scope. If blocked, record exact evidence and resume condition only. Do not ask the user, create control-plane stop files, archive logs, write `complete.log`, or execute the paused live OpenCode call. + +## Overview + +date=2026-08-14 +task=hybrid_caller_workspace_repair, plan=2, tag=REVIEW_REVIEW_REFACTOR + +## Archive Evidence Snapshot + +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G07_1.log`: the prior follow-up required actual provider-bound selector knowledge for initial/resume turns while preserving caller snapshots and marked execution. +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log`: official review verdict `FAIL`; Required R1 proved that the initial provider instruction requires two writes while the retained frontier accepts exactly one prepare call, and that the fake chooses its response by call index instead of instruction semantics. + +## For the Review Agent + +Run applicable verification fresh, append verdict and routing signals, then archive this review to `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G08_2.log` and the plan to `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G08_2.log`. PASS writes `complete.log` and finishes the task per the code-review skill; WARN/FAIL writes the next required active state. This task is not Milestone-linked. + +## Implementation Item Completion + +| Item | Status | +|---|---| +| FIX-R1 Make selector provider instructions phase-specific and consistent with the retained artifact frontier. | [x] | +| TEST-R1 Make Chat/Anthropic scripted selectors choose prepare or pair from the actual provider-bound instruction and cover swapped-operation rejection. | [x] | +| DOC-R1 Synchronize the Anthropic contract and living spec with prepare-only initial and pair-write resume semantics. | [x] | + +## Implementation Checklist + +- [x] [FIX-R1] Make selector provider instructions phase-specific and consistent with the retained artifact frontier. +- [x] [TEST-R1] Make Chat/Anthropic scripted selectors choose prepare or pair from the actual provider-bound instruction and cover swapped-operation rejection. +- [x] [DOC-R1] Synchronize the Anthropic contract and living spec with prepare-only initial and pair-write resume semantics. +- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +## Review-Only Checklist + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified routing signals. +- [x] Verify verdict, dimensions, and finding classifications are consistent. +- [x] Run applicable verification fresh and record output. +- [x] Record Evidence, Root Cause, and one Selected Fix for every Required/Suggested finding before any follow-up plan. +- [x] Archive this review to `code_review_cloud_G08_2.log`. +- [x] Archive `PLAN-cloud-G08.md` to `plan_cloud_G08_2.log`. +- [x] Verify the Agent-Ops `.gitignore` managed block. +- [x] If PASS, write `complete.log`, leave no active pair, and archive the active task directory as required. +- [ ] If WARN/FAIL, write the next filesystem state and do not write `complete.log`. + +## Deviations from Plan + +없음. 외부 live OpenCode smoke는 PLAN의 중지 경계를 지켜 실행하지 않았다. + +## Key Design Decisions + +- `artifactFrontierStore`가 잠금 아래에서 binding의 parent 생성 capability와 현재 phase를 읽고 닫힌 `none`, `prepare-only`, `pair-write` 상태만 반환한다. +- non-parent-creating initial frontier의 instruction은 request job directory에 대한 정확히 한 번의 prepare call만 허용하고 PLAN/REVIEW write와 pair path 노출을 금지한다. successful prepare receipt 뒤와 parent-creating initial frontier는 exact pair grammar를 가진 `pair-write`를 사용한다. +- Chat/Anthropic scripted service는 실제 protocol preparation과 body build를 거친 provider body의 system instruction에서 상태와 request ID를 파싱한다. call count는 예상 순서 assertion에만 사용하며 응답 operation 선택 근거로 쓰지 않는다. +- 두 protocol에서 prepare-only body에 pair output을 반환하는 경우와 pair-write body에 prepare output을 반환하는 경우를 각각 fail-closed 회귀로 고정했다. +- Anthropic outer contract와 `input/openai-compatible-surface` living spec을 같은 phase semantics로 동기화했다. public field, caller snapshot, raw workspace path 경계는 바꾸지 않았다. + +## Reviewer Checkpoints + +- The initial non-parent-creating binding tells the actual provider to emit exactly one prepare call for the request job directory. +- The successful prepare receipt changes the resumed selector instruction to the exact two-write PLAN/REVIEW pair grammar. +- Parent-creating initial bindings may receive pair-write immediately. +- Chat and Anthropic preserve caller/system content; marked `single_request`, Work, Review, and cleanup remain instruction-free. +- Scripted response operation comes from the actual provider-bound instruction, never metadata or call index. + +## Verification Results + +### Phase-specific focused tests + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPairHandlerDisposition' +``` + +_Paste actual stdout/stderr._ + +```text +ok iop/apps/edge/internal/openai 0.055s +``` + +### Final repository verification + +```bash +gofmt -w apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/hot_path_direct_test.go +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go test -count=1 ./apps/edge/internal/openai +go vet ./apps/edge/internal/service +go test ./apps/edge/internal/service -count=1 +go vet ./packages/go/... +go test ./packages/go/... -count=1 +go test -count=1 ./... +rg --sort path -n 'prepare-only|pair-write|selector.*instruction|scriptedArtifactPrepare|scriptedArtifactPair' apps/edge/internal/openai agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +test -z "$(rg -n 'req\.Run\.Metadata\["iop_logical_request_id"\]' apps/edge/internal/openai/hot_path_direct_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_chat_gate_test.go)" +git diff --check +``` + +Actual results: + +```text +gofmt: exit 0 (no stdout/stderr) +focused OpenAI tests: ok iop/apps/edge/internal/openai 0.161s +OpenAI package: ok iop/apps/edge/internal/openai 8.647s +service vet: exit 0 (no stdout/stderr) +service tests: ok iop/apps/edge/internal/service 8.219s +shared Go vet: exit 0 (no stdout/stderr) +shared Go tests: all packages passed; packages without tests reported [no test files] +repository tests: all packages passed; iop/apps/edge/internal/openai 8.912s, iop/apps/edge/internal/service 8.297s +deterministic prepare-only/pair-write search: exit 0; production, Chat/Anthropic fixtures, contract, and spec matches present +metadata privilege search: exit 0; no forbidden req.Run.Metadata["iop_logical_request_id"] match +git diff --check: exit 0 (no stdout/stderr) +``` + +Local verification context: `go version go1.26.2 linux/arm64`. Repo 내부 edge-node 진단과 보조 E2E smoke는 이 bounded selector instruction rework에서 실행하지 않았다. 외부 full-cycle OpenCode call은 `dev deployment complete and user explicitly resumes live call` 전까지 paused 상태이며 이 구현 pass에서는 실행 금지다. + +--- + +## Section Ownership + +| Section | Owner | +|---|---| +| Fixed header, Overview, archive snapshot, reviewer instructions/checkpoints | Stub creation; read-only for implementer | +| Implementation completion/checklist, deviations, decisions, verification results | Implementing agent | +| Review-only checklist and appended Code Review Result | Review agent only | + +## Code Review Result + +- Overall Verdict: PASS +- Dimension Assessment: + - Correctness: Pass + - Completeness: Pass + - Test coverage: Pass + - API contract: Pass + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- Findings: None +- Routing Signals: + - review_rework_count=2 + - evidence_integrity_failure=false +- Reviewer-Fresh Verification: + - `go version`: `go version go1.26.2 linux/arm64` + - `go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPairHandlerDisposition'`: PASS (`ok iop/apps/edge/internal/openai 0.086s`) + - focused OpenAI regression command from Final Verification: PASS (`ok iop/apps/edge/internal/openai 0.198s`) + - `go test -count=1 ./apps/edge/internal/openai`: PASS (`ok iop/apps/edge/internal/openai 8.845s`) + - `go vet ./apps/edge/internal/service`: PASS (exit 0, no output) + - `go test ./apps/edge/internal/service -count=1`: PASS (`ok iop/apps/edge/internal/service 8.227s`) + - `go vet ./packages/go/...`: PASS (exit 0, no output) + - `go test ./packages/go/... -count=1`: PASS (all shared packages passed; packages without tests reported `[no test files]`) + - `go test -count=1 ./...`: PASS (all repository packages passed; `iop/apps/edge/internal/openai 9.587s`, `iop/apps/edge/internal/service 8.324s`) + - deterministic prepare-only/pair-write search: PASS; production, Chat/Anthropic fixtures, contract, and living spec matches present + - metadata privilege search: PASS; no forbidden `req.Run.Metadata["iop_logical_request_id"]` match + - `git diff --check`: PASS (exit 0, no output) +- Next Step: Archive the reviewed pair, write `complete.log`, and finish the non-Milestone task. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log new file mode 100644 index 00000000..9672ff8a --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log @@ -0,0 +1,331 @@ + + +# Code Review Reference - REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt. +> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. +> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state. +> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## Overview + +date=2026-08-14 +task=hybrid_caller_workspace_repair, plan=0, tag=REFACTOR + +## Archive Evidence Snapshot + +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-hot-path-one-shot-execution.md`: route-01 implemented caller tool continuation, `.iop/job//plan.md` and `review.md` in the caller workspace, ordinary work tools, reviewer repair, and request-artifact-only cleanup. +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md`: route-02 intentionally replaced that product default with a marked `single_request` preset and an operator-approved Node `workspace_ref`; it must remain available as a distinct server-owned path. +- Current source confirms the discriminator: `compilePresetArtifactBinding` bypasses caller `workspace_tools` only when `SingleRequest` is present, while unmarked `light` presets still compile the caller tool binding. +- Current dev evidence showed the benchmark output persisted in the fixed IOP workspace while the OpenCode workspace stayed empty. The repair therefore changes ownership selection and handoff enforcement, not Node cleanup. + +## For the Review Agent + +> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section. + +Compare implementation of each item against source files. Run the applicable verification commands directly and record fresh output in `Verification Results`; implementation-owned output is handoff evidence, not a substitute for reviewer verification. If implementation is present, repair missing or stale verification output instead of failing solely for insufficient recorded evidence. When verification exposes a defect, collect the necessary data, determine the exact root cause, and select one concrete fix before generating the follow-up plan; never delegate investigation or remedy selection to the worker. + +Review completion means the following steps are finished: + +1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. +2. Archive `CODE_REVIEW-cloud-G09.md` → `code_review_cloud_G09_0.log` and `PLAN-cloud-G08.md` → `plan_cloud_G08_0.log`. +3. If PASS, write `complete.log` and move the active task directory to `agent-task/archive/YYYY/MM/hybrid_caller_workspace_repair/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. +4. This task is not Milestone-linked, so no roadmap completion aggregation event is emitted. +5. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. + +--- + +## Implementation Item Completion + +| Item | Status | +|------|---------| +| REFACTOR-1 Separate caller-workspace and IOP-owned preset ownership without adding caller-selected raw paths. | [x] | +| REFACTOR-2 Enforce template PLAN → worker REVIEW handoff → reviewer inspection/repair and trustworthy final output on the caller-tool Hot Path. | [x] | +| TEST-3 Add deterministic regression coverage, run fresh local verification, and stop before the live IOP/OpenCode call with the resume condition recorded. | [x] | + +## Implementation Checklist + +- [x] [REFACTOR-1] Separate caller-workspace and IOP-owned preset ownership without adding caller-selected raw paths. +- [x] [REFACTOR-2] Enforce template PLAN → worker REVIEW handoff → reviewer inspection/repair and trustworthy final output on the caller-tool Hot Path. +- [x] [TEST-3] Add deterministic regression coverage, run fresh local verification, and stop before the live IOP/OpenCode call with the resume condition recorded. +- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +## Review-Only Checklist + +> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent. +> Implementing agents must not modify or check this section. + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. +- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. +- [x] Run applicable required verification and record fresh command/output; repair reviewer-reconstructable evidence gaps instead of forwarding them to another plan. +- [x] For every Required/Suggested finding, record reviewer-collected `Evidence`, exact `Root Cause`, and one `Selected Fix` with affected files/symbols/tests and acceptance commands before creating a follow-up plan. +- [x] Archive `CODE_REVIEW-cloud-G09.md` to `code_review_cloud_G09_0.log`. +- [x] Archive `PLAN-cloud-G08.md` to `plan_cloud_G08_0.log`. +- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/hybrid_caller_workspace_repair/` to `agent-task/archive/YYYY/MM/hybrid_caller_workspace_repair/` and update this checklist at the final archive path. +- [ ] If PASS and task group is `m-`, preserve and report `milestone-task` metadata for runtime aggregation, without modifying roadmap or directly calling `update-roadmap` (not applicable to this task). +- [ ] If PASS for split work, remove the empty active parent or verify it was kept due to remaining siblings/files (not applicable to this single-plan task). +- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`. + +## Deviations from Plan + +- 라이브 IOP/OpenCode smoke는 PLAN의 명시적 중단 조건에 따라 실행하지 않았다. 재개 조건은 `dev deployment complete and user explicitly resumes live call`이다. +- 상태 상수와 공용 scripted fixture가 바뀌어 직접 나열된 세 테스트 외에 `hot_path_direct_test.go`, stream gate 테스트, cleanup/observation 코드와 테스트의 기대 상태를 같은 계약으로 동기화했다. 제품 범위는 확장하지 않았다. +- `agent-contract/index.md`는 Anthropic 계약의 읽기 조건/원본 포인터가 새 unmarked caller-workspace 경계를 포함하도록 함께 갱신했다. + +## Key Design Decisions + +- `presetAdmitsCallerWorkspaceTools`를 단일 ownership 판별점으로 두었다. unmarked `light`만 caller tool binding을 허용하고, marked `single_request`는 기존 Node-owned workspace 경로를 유지한다. +- selector가 낸 PLAN과 pending REVIEW seed를 caller write 발행 전에 `singlerequesttemplate` 기본 문법으로 검증하고 PLAN `P1..Pn` inventory를 request-local state에 보존한다. +- Work는 성공한 PLAN read 뒤 ordinary work/verification tool과 정확한 completed REVIEW handoff를 발행해야만 Review로 전이한다. Reviewer는 PLAN/REVIEW read와 ordinary caller-result inspection을 모두 완료해야 하며 REVIEW를 다시 쓰지 않는다. +- repair는 동일 Review stage binding에 남고, terminal은 non-empty reviewer output만 허용한다. cleanup의 기존 `.iop/job/` delete 경계는 유지했다. + +## Reviewer Checkpoints + +- Caller-workspace presets are unmarked and depend only on admitted caller tools; marked `single_request` presets retain Node-owned `workspace_ref` behavior. +- No raw caller filesystem path crosses the API/config boundary. +- PLAN and REVIEW follow the shared lightweight templates; Work owns the REVIEW handoff and Review does not rewrite it. +- Reviewer finalization requires actual artifact reads, successful ordinary result inspection, completed repair/verification frontiers, and non-empty final output. +- Cleanup removes only `.iop/job/` and preserves requested user files. +- No benchmark harness, 9-path campaign, Node executor redesign, or provider/effort routing change entered the diff. + +## Verification Results + +### Ownership-focused regression + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|Test.*Preset.*Workspace' +``` + +```text +ok iop/apps/edge/internal/openai 0.095s +``` + +### Focused regression + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +``` + +```text +ok iop/apps/edge/internal/openai 0.152s +``` + +### Formatting + +```bash +gofmt -w apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/hot_path_stage_input.go apps/edge/internal/openai/hot_path_light.go apps/edge/internal/openai/hot_path_review.go apps/edge/internal/openai/artifact_pair_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_review_test.go +``` + +```text +(stdout/stderr 없음, exit 0) +``` + +### Edge package + +```bash +go test -count=1 ./apps/edge/internal/openai +``` + +```text +ok iop/apps/edge/internal/openai 8.820s +``` + +### Full repository + +```bash +go test -count=1 ./... +``` + +```text +ok iop/apps/control-plane/cmd/control-plane 3.466s +ok iop/apps/control-plane/internal/credentiallease 0.231s +ok iop/apps/control-plane/internal/credentialops 0.402s +ok iop/apps/control-plane/internal/credentialseal 0.264s +ok iop/apps/control-plane/internal/credentialstore 0.369s +ok iop/apps/control-plane/internal/wire 2.027s +ok iop/apps/edge/cmd/edge 0.248s +ok iop/apps/edge/internal/authprojection 0.068s +ok iop/apps/edge/internal/bootstrap 0.571s +ok iop/apps/edge/internal/configrefresh 0.117s +ok iop/apps/edge/internal/controlplane 6.651s +ok iop/apps/edge/internal/edgecmd 0.139s +ok iop/apps/edge/internal/edgevalidate 0.080s +ok iop/apps/edge/internal/events 0.051s +ok iop/apps/edge/internal/input 0.124s +ok iop/apps/edge/internal/input/a2a 0.096s +ok iop/apps/edge/internal/node 0.100s +ok iop/apps/edge/internal/openai 9.586s +ok iop/apps/edge/internal/opsconsole 0.130s +ok iop/apps/edge/internal/service 8.346s +ok iop/apps/edge/internal/transport 4.869s +ok iop/apps/node/cmd/node 0.169s +ok iop/apps/node/internal/adapters 0.120s +? iop/apps/node/internal/adapters/mock [no test files] +ok iop/apps/node/internal/adapters/ollama 0.063s +ok iop/apps/node/internal/adapters/openai_compat 0.182s +ok iop/apps/node/internal/adapters/vllm 0.169s +ok iop/apps/node/internal/bootstrap 1.484s +ok iop/apps/node/internal/node 1.147s +ok iop/apps/node/internal/router 0.522s +ok iop/apps/node/internal/store 0.028s +ok iop/apps/node/internal/transport 5.605s +ok iop/apps/node/internal/workspace 0.813s +? iop/apps/worker/cmd/worker [no test files] +ok iop/packages/go/audit 0.015s +ok iop/packages/go/auth 10.026s +ok iop/packages/go/config 0.198s +ok iop/packages/go/credentiallease 0.022s +? iop/packages/go/events [no test files] +ok iop/packages/go/execution 0.006s +ok iop/packages/go/hostsetup 0.007s +? iop/packages/go/jobs [no test files] +? iop/packages/go/metadata [no test files] +ok iop/packages/go/observability 0.029s +? iop/packages/go/policy [no test files] +ok iop/packages/go/singlerequesttemplate 0.013s +ok iop/packages/go/streamgate 0.884s +? iop/packages/go/version [no test files] +ok iop/packages/go/workspaceprotocol 0.021s +? iop/proto/gen/iop [no test files] +ok iop/scripts/inventory-query 0.011s +``` + +### Deterministic boundary search + +```bash +rg --sort path -n 'single_request|workspace_tools|hotPathPendingReview|review\.md' apps/edge/internal/openai configs/edge.yaml agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +``` + +```text +apps/edge/internal/openai/anthropic_handler.go:25:const anthropicSingleRequestTerminalRejectionLogMessage = "edge_single_request_terminal_rejection" +apps/edge/internal/openai/hot_path_light.go:64: hotPathPendingReviewInspection hotPathPendingKind = "review_inspection" +apps/edge/internal/openai/hot_path_light.go:65: hotPathPendingReviewRepair hotPathPendingKind = "review_repair" +apps/edge/internal/openai/hot_path_light.go:602: case hotPathPendingReviewInspection: +apps/edge/internal/openai/hot_path_light.go:607: case hotPathPendingReviewRepair: +apps/edge/internal/openai/hot_path_light.go:701: if pendingKind == hotPathPendingReviewInspection || pendingKind == hotPathPendingReviewRepair { +apps/edge/internal/openai/hot_path_light.go:714: case hotPathPendingReviewInspection: +apps/edge/internal/openai/hot_path_light.go:764: case hotPathPendingReviewInspection: +apps/edge/internal/openai/hot_path_light.go:766: case hotPathPendingReviewRepair: +apps/edge/internal/openai/hot_path_review.go:73: return hotPathPendingReviewRepair, false, nil +apps/edge/internal/openai/hot_path_review.go:75: return hotPathPendingReviewInspection, false, nil +apps/edge/internal/openai/hot_path_review.go:92: return hotPathPendingReviewRepair, false, nil +apps/edge/internal/openai/hot_path_review_test.go:58: if kind, cleanup, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewActive, repair, evidence); err != nil || kind != hotPathPendingReviewRepair || cleanup { +apps/edge/internal/openai/hot_path_selector.go:39: ReviewPath string // e.g. ".iop/job//review.md" +apps/edge/internal/openai/hot_path_selector.go:49: ReviewPath: jobDir + "/review.md", +apps/edge/internal/openai/openai_auth_routes_models_test.go:234: // A legacy, unmarked preset (no single_request policy) must keep resolving and +apps/edge/internal/openai/single_request_metrics.go:12: Name: "iop_anthropic_single_request_ingress_total", +apps/edge/internal/openai/single_request_plan_stage.go:72: Name: "single_request_plan", +apps/edge/internal/openai/single_request_review_stage.go:356: Name: "single_request_review", +apps/edge/internal/openai/single_request_work_stage.go:153: Name: "single_request_work", +configs/edge.yaml:554:# Do not add single_request or a raw workspace path to this preset; the caller's +configs/edge.yaml:555:# workspace_tools alternative binds the caller-opened workspace at request time. +configs/edge.yaml:568:# workspace_tools: +configs/edge.yaml:597:# single_request: +configs/edge.yaml:617:# review_file: "templates/review.md" # relative to this edge.yaml +agent-contract/outer/anthropic-compatible-api.md:94:An unmarked `light` preset with `workspace_tools` uses only the tool schemas +agent-contract/outer/anthropic-compatible-api.md:105:A preset marked with `single_request` is excluded from this continuation before +agent-contract/outer/anthropic-compatible-api.md:107:`single_request` and `workspace_tools`, so the operator-owned Node workspace and +agent-contract/outer/anthropic-compatible-api.md:150: Edge renders and validates the one `review.md` handoff. Review reads both stored +agent-contract/outer/anthropic-compatible-api.md:151: artifacts, never accepts a memory work payload, and never rewrites `review.md`. +agent-spec/input/openai-compatible-surface.md:205:| unmarked caller-workspace light route | An unmarked `light` preset binds only admitted caller `workspace_tools`; it never accepts a raw caller path. The selector's lightweight PLAN and pending REVIEW seed are validated before caller writes. Work reads PLAN, executes/verifies in the caller workspace, and replaces REVIEW with a completed `P1..Pn` handoff. Review reads both artifacts, successfully inspects an ordinary caller result, repairs/re-verifies in the same binding when needed, and owns the non-empty final output without rewriting REVIEW. Cleanup removes only the request `.iop/job/` directory. Marked `single_request` presets bypass this route and retain the operator-owned Node workspace. | +agent-spec/input/openai-compatible-surface.md:367:- 2026-08-14: Added the restored unmarked caller-workspace light route, including template-validated PLAN/pending REVIEW creation, worker-owned completed REVIEW handoff, reviewer reads/result inspection/repair, reviewer-owned non-empty terminal, and strict separation from marked Node-owned `single_request` presets. +``` + +### Diff check + +```bash +git diff --check +``` + +```text +(stdout/stderr 없음, exit 0) +``` + +### Live OpenCode smoke pause + +Implementation must not execute the live IOP/OpenCode call. Record the local-ready state and exact resume condition (`dev deployment complete and user explicitly resumes live call`) here. After resume, the reviewer must attach the exact command output and caller-workspace/cleanup assertions from PLAN TEST-3. + +```bash +mkdir -p build/test-tmp/hybrid-caller-workspace-repair +opencode run --dir "$PWD/build/test-tmp/hybrid-caller-workspace-repair" --model iop/gpt-hybrid --agent build --format json 'Create a small self-contained index.html that shows a centered card with the exact text IOP HYBRID OK. Verify the file exists and is valid HTML. Return a concise final summary.' +test -s build/test-tmp/hybrid-caller-workspace-repair/index.html +test "$(find build/test-tmp/hybrid-caller-workspace-repair/.iop/job -mindepth 1 -maxdepth 1 -type d 2>/dev/null | wc -l | tr -d ' ')" = "0" +``` + +실행하지 않음. 로컬 deterministic 검증은 완료되었고, 정확한 재개 조건은 `dev deployment complete and user explicitly resumes live call`이다. 재개 후 위 명령의 stdout/stderr와 caller-workspace `index.html`/cleanup assertion을 reviewer가 첨부해야 한다. + +### Reviewer fresh verification + +```bash +go version +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|Test.*Preset.*Workspace' +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go vet ./apps/edge/internal/service +go test ./apps/edge/internal/service -count=1 +go vet ./packages/go/... +go test ./packages/go/... -count=1 +go test -count=1 ./apps/edge/internal/openai +go test -count=1 ./... +git diff --check +``` + +```text +go version go1.26.2 linux/arm64 +ok iop/apps/edge/internal/openai 0.112s +ok iop/apps/edge/internal/openai 0.183s +ok iop/apps/edge/internal/service 8.282s +ok iop/packages/go/... (all packages; no test files where applicable) +ok iop/apps/edge/internal/openai 8.783s +ok iop/... (all repository packages; no test files where applicable) +git diff --check: stdout/stderr 없음, exit 0 +``` + +The reviewer did not execute the paused live OpenCode call because the active PLAN explicitly requires deployment and user continuation first. + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these | +| Archive Evidence Snapshot | Fixed at stub creation from plan | Read only cited archive files when more detail is required | +| Implementation Item Completion | Implementing agent | Check `[ ]` → `[x]` only | +| Implementation Checklist | Implementing agent | Check `[ ]` → `[x]` only | +| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section | +| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content | +| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan | +| Verification Results | Implementing agent, then review agent | Record actual output; command changes require a Deviations entry | +| Code Review Result | Review agent appends | Not included in stub | + +## Code Review Result + +- Overall Verdict: FAIL +- Dimension Assessment: + - Correctness: Fail + - Completeness: Fail + - Test coverage: Fail + - API contract: Fail + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- Findings: + - Required R1 — The real selector is never given the generated artifact paths or the newly required exact PLAN/pending REVIEW grammar. + - Evidence: Reviewer source tracing shows `joinPresetChatIngress`/`joinPresetAnthropicIngress` allocate the random logical request ID only after decoding the caller body, while the provider-bound Chat and Anthropic bodies remain caller-derived. `mapArtifactOutput` then requires the selector to emit the exact generated `.iop/job//plan.md` and `review.md` paths and byte-matching pending REVIEW. The deterministic fixture masks this boundary by reading `req.Run.Metadata["iop_logical_request_id"]` inside `scriptedLightPoolService.SubmitProviderPool` and synthesizing `scriptedArtifactPair`; a real provider receives the HTTP body, not that internal test-service metadata privilege. Commands: `rg -n 'req.Run.Metadata\["iop_logical_request_id"\]|scriptedArtifactPair\(|mapArtifactOutput\(|validatePendingReviewSeed\(|canonicalBody\(|SubmitProviderPool' ...` plus direct inspection of `chat_handler.go`, `anthropic_handler.go`, `artifact_pair.go`, and `hot_path_light_test.go`. + - Root Cause: Validation was added at the post-provider artifact mapper, but no provider-side selector instruction was added after request-ID allocation. The tests generate the missing provider knowledge out of band from internal dispatch metadata, so all deterministic tests pass without exercising the production wire precondition. + - Selected Fix: Add one bounded IOP-owned selector instruction to every initial/resume unmarked caller-workspace selector request before provider submission. For Chat, insert it as the final leading system message before the first non-system message in the provider-derived body; for Anthropic, append it as a text block to the provider-derived top-level `system`. The instruction must contain the exact issued relative PLAN/REVIEW paths, default PLAN grammar, and deterministic pending REVIEW grammar, without changing the caller snapshot or adding a raw workspace path. Update the scripted provider fixtures to derive their selector output only from the actual provider-bound body/instruction, never `Run.Metadata`, and assert OpenAI and Anthropic wire bodies expose the exact paths/grammar while marked `single_request` remains unchanged. Acceptance: the focused Hot Path tests, full OpenAI package, full repository, deterministic boundary search, and `git diff --check` all pass fresh. +- Routing Signals: + - review_rework_count=1 + - evidence_integrity_failure=false +- Next Step: Create and implement the closed R1 follow-up plan through the plan skill; rerun final routing before writing the next active pair. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/complete.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/complete.log new file mode 100644 index 00000000..06c23e6f --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/complete.log @@ -0,0 +1,43 @@ + + +# Complete - hybrid_caller_workspace_repair + +## 완료 일시 + +2026-08-14 + +## 요약 + +3회 리뷰 루프에서 caller-workspace selector의 provider-bound 경로·문법과 phase별 `prepare-only`/`pair-write` 계약을 복구했으며 최종 판정은 PASS다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_cloud_G08_0.log` | `code_review_cloud_G09_0.log` | FAIL | 실제 provider body에 request-local artifact 경로와 문법이 전달되지 않는 문제를 확인했다. | +| `plan_cloud_G07_1.log` | `code_review_cloud_G08_1.log` | FAIL | initial prepare frontier와 동일한 pair-write instruction의 모순을 확인했다. | +| `plan_cloud_G08_2.log` | `code_review_cloud_G08_2.log` | PASS | frontier 상태별 instruction과 provider-body 기반 Chat/Anthropic 회귀가 일치함을 확인했다. | + +## 구현/정리 내용 + +- 잠금 아래의 artifact frontier에서 `none`, `prepare-only`, `pair-write` 상태를 계산하고 실제 provider body에 phase별 instruction을 주입한다. +- Chat/Anthropic scripted selector가 call index나 내부 metadata가 아니라 provider-bound instruction에서 operation과 request ID를 선택하도록 변경했다. +- swapped-operation 거부 회귀를 추가하고 Anthropic outer contract와 OpenAI-compatible living spec을 현재 구현에 맞췄다. + +## 최종 검증 + +- `go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPairHandlerDisposition'` - PASS; `ok iop/apps/edge/internal/openai 0.086s`. +- `go test -count=1 ./apps/edge/internal/openai` - PASS; `ok iop/apps/edge/internal/openai 8.845s`. +- `go vet ./apps/edge/internal/service && go test ./apps/edge/internal/service -count=1` - PASS; vet exit 0, service tests passed. +- `go vet ./packages/go/... && go test ./packages/go/... -count=1` - PASS; 모든 shared Go package가 통과했다. +- `go test -count=1 ./...` - PASS; 모든 repository Go package가 통과했다. +- deterministic instruction/search, metadata privilege no-match, `git diff --check` - PASS. +- 외부 live OpenCode smoke - 실행하지 않음; 계획의 `dev deployment complete and user explicitly resumes live call` 중지 경계를 준수했다. + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G07_1.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G07_1.log new file mode 100644 index 00000000..c19b9987 --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G07_1.log @@ -0,0 +1,129 @@ + + +# Hybrid Selector Provider Instruction Follow-up Plan + +## For the Implementing Agent + +Fill every implementation-owned section in `CODE_REVIEW-cloud-G08.md`, run the listed verification, and leave both active files in place for review. Do not archive logs, write `complete.log`, run orchestration, ask the user for a design choice, or execute the paused live OpenCode call. + +## Background + +The first repair restored caller-workspace ownership and tightened PLAN/REVIEW handoff validation, but the production selector request never receives the random request-local artifact paths or the exact seed grammar it must return. Tests passed because their fake provider synthesized those values from internal run metadata that a real provider cannot see. This follow-up closes only that provider-bound knowledge gap while preserving caller snapshots, public schemas, and marked `single_request` behavior. + +## Archive Evidence Snapshot + +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G08_0.log`: the original repair separated marked Node-owned execution from unmarked caller-workspace execution and required an exact PLAN → worker REVIEW → reviewer flow. +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log`: official review verdict `FAIL`; Required R1 found that all fresh deterministic tests passed while the real provider never received the generated paths or required grammar because the scripted provider read internal `Run.Metadata` out of band. + +## Analysis + +### Files Read + +- `agent-ops/rules/project/domain/edge/rules.md` +- `agent-ops/rules/project/domain/platform/rules.md` +- `agent-ops/rules/project/domain/testing/rules.md` +- `agent-test/local/rules.md` +- `agent-test/local/profiles/edge-smoke.md` +- `agent-test/local/profiles/platform-common-smoke.md` +- `agent-contract/index.md` +- `agent-contract/outer/anthropic-compatible-api.md` +- `agent-spec/index.md` +- `agent-spec/input/openai-compatible-surface.md` +- `apps/edge/internal/openai/request_identity_ingress.go` +- `apps/edge/internal/openai/chat_handler.go` +- `apps/edge/internal/openai/anthropic_handler.go` +- `apps/edge/internal/openai/provider_model_rewrite.go` +- `apps/edge/internal/openai/artifact_pair.go` +- `apps/edge/internal/openai/hot_path_light_test.go` +- `apps/edge/internal/openai/hot_path_direct_test.go` +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G08_0.log` +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G09_0.log` + +### SDD Criteria + +Not applicable. There is no active Milestone and this is a bounded review rework that preserves the previously selected ownership and protocol boundaries. + +### Verification Context + +- Reviewer-fresh focused, package, repository, vet, and diff checks passed before this plan. +- The defect is a production-wire precondition missed by the test double, not a nondeterministic test failure. +- The live OpenCode smoke remains paused until `dev deployment complete and user explicitly resumes live call`. + +### Test Coverage Gap + +The scripted pool service derives artifact paths from `req.Run.Metadata["iop_logical_request_id"]`. It therefore does not prove that the actual provider-bound Chat or Anthropic request carries enough information for a real selector to emit the accepted artifact pair. + +### Scope and Split Judgment + +Keep one plan. Provider instruction construction, both protocol encodings, and removal of the privileged fixture path are one boundary invariant. Do not change caller request snapshots, public API/config schemas, raw workspace path handling, marked `single_request`, worker/reviewer state transitions, cleanup, routing, or the benchmark harness. + +### Final Routing + +- `evaluation_mode=review-rework`; `finalizer=finalize-task-policy.sh`; `finalizer_mode=pair`. +- Build route: base `local-fit`, risk signatures `temporal_state`, `boundary_contract`, `structured_interpretation`, `variant_product`; scores `2,1,1,1,2`; final `risk-boundary`, `worker/cloud/G07`, `PLAN-cloud-G07.md`. +- Review route: scores `2,1,1,2,2`; `official-review`, `review/cloud/G08`, `CODE_REVIEW-cloud-G08.md`. +- `large_indivisible_context=false`; `review_rework_count=1`; `evidence_integrity_failure=false`. + +## Finding Resolution Map + +| Finding | Evidence | Root Cause | Selected Fix | Mode | Changed Precondition | Acceptance | +|---|---|---|---|---|---|---| +| Required R1 | Production bodies are caller-derived while `mapArtifactOutput` requires random exact paths and seed bytes; the fake reads internal run metadata. | Post-provider validation was added without an instruction after logical request-ID allocation. | Add a bounded request-local selector instruction to the actual provider body for unmarked initial/resume selector turns, and make fixtures derive output only from that body. | direct-fix | The real provider receives exact relative paths and grammar before it must emit the artifact pair. | Focused wire/Hot Path tests, OpenAI package, repository suite, deterministic search, and diff check pass fresh. | + +## Implementation Checklist + +- [ ] [FIX-R1] Construct one request-local instruction from the issued artifact pair and inject it only into unmarked caller-workspace selector provider bodies: final leading Chat system message before the first non-system message, and an appended Anthropic top-level `system` text block. Include exact relative PLAN/REVIEW paths, default PLAN grammar, and deterministic pending REVIEW grammar; preserve caller snapshots and omit raw workspace paths. +- [ ] [TEST-R1] Change scripted selectors to materialize and parse the actual provider-bound body through the existing pool/tunnel seam, never `Run.Metadata`; cover OpenAI and Anthropic initial/resume selector turns and assert marked `single_request` wire behavior is unchanged. +- [ ] [DOC-R1] Synchronize the Anthropic contract and current implementation spec with the provider-side instruction boundary, without adding a public request/config field. +- [ ] Fill all implementation-owned sections in `CODE_REVIEW-cloud-G08.md` with actual changes and fresh command output. + +## Implementation Details + +### [FIX-R1] Supply selector knowledge on the real provider boundary + +Add a deterministic helper near the existing ingress/artifact body logic that accepts the request-local artifact binding and protocol-derived provider body. Its text must enumerate both issued relative paths, reproduce the exact default PLAN and pending REVIEW seed formats accepted by `artifact_pair.go`, and direct the selector to return that pair. Apply it after logical request-ID allocation and only when the unmarked caller-workspace artifact frontier is pinned. + +For Chat, retain all existing leading system messages and insert the IOP instruction as the last leading system message. For Anthropic, preserve the current string-or-block `system` semantics and append one text block. Initial and selector-resume requests get the same request-local instruction; Work, Review, cleanup, and marked `single_request` requests do not. The immutable ingress/caller snapshot must remain the lineage source. + +### [TEST-R1] Exercise the provider body rather than internal dispatch metadata + +Refactor the common scripted provider helper so it invokes the same provider body builder/preparation seam used by the real service, parses the protocol body, extracts the bounded selector instruction, and derives its artifact pair from that instruction. Remove every selector-output dependency on `req.Run.Metadata["iop_logical_request_id"]`. Assert message ordering, Anthropic system block preservation, exact paths/grammar, initial/resume coverage, and absence from marked `single_request` bodies. + +### [DOC-R1] Record the boundary + +State that request-local path/grammar knowledge is an Edge-owned provider instruction, not caller input or a public schema field, in the matching contract and implementation spec. + +## Modified Files Summary + +| File | Item | +|---|---| +| `apps/edge/internal/openai/request_identity_ingress.go` | FIX-R1 | +| `apps/edge/internal/openai/chat_handler.go` | FIX-R1 | +| `apps/edge/internal/openai/anthropic_handler.go` | FIX-R1 | +| `apps/edge/internal/openai/provider_model_rewrite.go` | FIX-R1 | +| `apps/edge/internal/openai/hot_path_light_test.go` | TEST-R1 | +| `apps/edge/internal/openai/hot_path_direct_test.go` | TEST-R1 | +| `agent-contract/outer/anthropic-compatible-api.md` | DOC-R1 | +| `agent-spec/input/openai-compatible-surface.md` | DOC-R1 | +| `agent-task/hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md` | implementation evidence | + +## Reviewer Checkpoints + +- The provider-bound body itself, not test-only metadata, carries both random issued paths and the accepted exact grammar. +- Chat and Anthropic preserve existing caller/system content and canonical wire shape. +- Both initial and selector-resume turns are covered; Work/Review/cleanup turns do not receive the selector instruction. +- Marked `single_request` remains unchanged and no raw caller workspace path or public field is introduced. +- Existing handoff, reviewer inspection/repair, terminal, and cleanup invariants remain green. + +## Final Verification + +```bash +gofmt -w apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/chat_handler.go apps/edge/internal/openai/anthropic_handler.go apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_direct_test.go +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go test -count=1 ./apps/edge/internal/openai +go test -count=1 ./... +rg --sort path -n 'iop_logical_request_id|scriptedArtifactPair|selector.*instruction|pending REVIEW' apps/edge/internal/openai agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +git diff --check +``` + +Expected: all tests pass uncached; provider-wire assertions cover both protocols and selector turns; the search shows no scripted selector deriving output from internal logical-request metadata; the diff is clean. Do not execute the live OpenCode smoke during this implementation. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_0.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_0.log new file mode 100644 index 00000000..5f35f47a --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_0.log @@ -0,0 +1,267 @@ + + +# Hybrid Caller Workspace Repair Plan + +## For the Implementing Agent + +Filling implementation-owned sections in `CODE_REVIEW-cloud-G09.md` is mandatory. Run the verification commands, paste actual output, keep both active files in place, and report ready for review; finalization belongs only to the code-review skill. If blocked, record the exact blocker, attempted command/output, and resume condition in the implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`. + +## Background + +The live agent-facing hybrid aliases currently select the marked `single_request` path, so Work writes into an operator-owned IOP Node workspace instead of the workspace opened by OpenCode or another caller agent. The repository still contains the caller-tool Hot Path, but its old Review flow predates the current PLAN→worker REVIEW handoff→review/repair template contract. This repair keeps the IOP-owned single-request path intact under separate presets and restores a caller-workspace hybrid path without rebuilding the benchmark harness. + +## Archive Evidence Snapshot + +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-hot-path-one-shot-execution.md`: route-01 implemented caller tool continuation, `.iop/job//plan.md` and `review.md` in the caller workspace, ordinary work tools, reviewer repair, and request-artifact-only cleanup. +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md`: route-02 intentionally replaced that product default with a marked `single_request` preset and an operator-approved Node `workspace_ref`; it must remain available as a distinct server-owned path. +- Current source confirms the discriminator: `compilePresetArtifactBinding` bypasses caller `workspace_tools` only when `SingleRequest` is present, while unmarked `light` presets still compile the caller tool binding. +- Current dev evidence showed the benchmark output persisted in the fixed IOP workspace while the OpenCode workspace stayed empty. The repair therefore changes ownership selection and handoff enforcement, not Node cleanup. + +## Analysis + +### Files Read + +- `agent-ops/rules/project/domain/edge/rules.md` +- `agent-ops/rules/project/domain/node/rules.md` +- `agent-ops/rules/project/domain/testing/rules.md` +- `agent-test/local/rules.md` +- `agent-roadmap/ROADMAP.md` +- `agent-roadmap/current.md` +- `agent-roadmap/priority-queue.md` +- `agent-roadmap/phase/knowledge-tool-optimization-extension/PHASE.md` +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-hot-path-one-shot-execution.md` +- `agent-roadmap/archive/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md` +- `agent-contract/index.md` +- `agent-contract/outer/openai-compatible-api.md` +- `agent-contract/outer/anthropic-compatible-api.md` +- `agent-contract/inner/edge-config-runtime-refresh.md` +- `agent-contract/inner/edge-node-runtime-wire.md` +- `agent-spec/index.md` +- `agent-spec/input/openai-compatible-surface.md` +- `apps/edge/internal/openai/request_identity_ingress.go` +- `apps/edge/internal/openai/workspace_tool_binding.go` +- `apps/edge/internal/openai/artifact_pair.go` +- `apps/edge/internal/openai/hot_path_stage_input.go` +- `apps/edge/internal/openai/hot_path_light.go` +- `apps/edge/internal/openai/hot_path_review.go` +- `apps/edge/internal/openai/chat_handler.go` +- `apps/edge/internal/openai/artifact_pair_test.go` +- `apps/edge/internal/openai/hot_path_light_test.go` +- `apps/edge/internal/openai/hot_path_review_test.go` +- `packages/go/config/execution_preset_types.go` +- `packages/go/singlerequesttemplate/template.go` +- `configs/edge.yaml` +- `agent-ops/skills/project/dev-runtime-deploy/SKILL.md` + +### SDD Criteria + +Not applicable. `agent-roadmap/current.md` has no active Milestone, and the user explicitly requested a bounded repair rather than a new Milestone. This plan preserves both previously approved ownership modes and does not reopen the Node workspace executor design. + +### Verification Context + +- No separate handoff packet was supplied. Repository code, current/archived roadmap documents, contracts, tests, the observed workspace mismatch, and read-only runtime probes are the evidence sources. +- Local criteria: unmarked `light` + `workspace_tools` uses caller continuation; marked `single_request` remains Node-owned; PLAN and REVIEW use the shared lightweight template grammar; Work cannot transition without a completed REVIEW handoff; Review cannot pass without reading both artifacts and inspecting the caller result; repair remains in the Review stage; cleanup removes only `.iop/job/`. +- Local command availability: `opencode` is installed at version `1.18.3`; `opencode run` supports `--model`, `--agent`, `--format`, and `--dir`. +- External Verification Preflight: runner is the dev runtime host; repo root is `/Users/toki/agent-work/iop-dev`; branch is clean `dev`; local HEAD and `origin/dev` both resolve to `4b1f08452d91bd135ea95fab314e334a2b84bd9a`; OS/arch is Darwin/arm64; Edge binary and `build/dev-runtime/edge.yaml` exist; the OpenAI-compatible listener is active on port `18083`. Source is intentionally pre-change and must be clean-synced, rebuilt, and restarted by `dev-runtime-deploy` after merge. +- Constraint: per the user instruction, implementation must stop before the first live IOP/OpenCode call and wait for deployment/continuation approval. Local deterministic tests are mandatory before that stop. +- Confidence: high for the ownership root cause and existing route separation; medium for the live caller-tool schema until the isolated OpenCode request confirms the deployed `workspace_tools` alternative. + +### Test Coverage Gaps + +- Existing config and ingress tests cover mutual exclusion of `single_request` and `workspace_tools`, but do not prove live aliases select different ownership modes. +- Existing artifact tests prove exact Plan/Review paths and caller receipts, but accept arbitrary artifact contents. +- Existing light tests prove local→review→repair state transitions for both OpenAI and Anthropic, but the reviewer currently authors REVIEW and may finalize without a required result inspection. +- No existing isolated OpenCode test proves that the requested `index.html` remains in the caller workspace while `.iop/job/` is cleaned. One post-deploy smoke is required; no 9-path campaign is included. + +### Symbol References + +- No public symbol is renamed or removed. +- Internal phase/pending constants may be replaced to express worker REVIEW handoff and reviewer inspection. All references are confined to `hot_path_light.go`, `hot_path_review.go`, `hot_path_stage_input.go`, and their listed tests; run `rg --sort path` before deleting any constant. + +### Split Judgment + +Keep one plan. Preset ownership selection, artifact template validation, worker handoff, reviewer inspection/repair, cleanup, and terminal projection form one correctness invariant; splitting them would leave a deployable intermediate route that either writes to the wrong workspace or can report success without verified output. + +### Scope Rationale + +- Keep the marked IOP-owned single-request executor, Node workspace runtime, protobuf wire, effort normalization, and provider routing unchanged. +- Do not recreate the benchmark harness, result template, scoring pipeline, or 9-path matrix. Verification is one deterministic local suite plus one isolated OpenCode HTML smoke after deployment approval. +- Do not add raw caller paths to IOP config or request metadata. Caller ownership is realized only through the caller's admitted tools. +- Do not create a new Milestone. The user explicitly selected a bounded compatibility repair; roadmap-wide ownership wording can be synchronized separately after the implementation proves the route. + +### Final Routing + +- `evaluation_mode=first-pass`; `finalizer=finalize-task-policy.sh`; `finalizer_mode=pair`. +- Build closures: scope/context/verification/evidence/ownership/decision all `true`. Scores `scope=2,state=2,blast=1,evidence=1,verification=2`, grade `G08`; base `local-fit`, final route `risk-boundary`, `worker/cloud/G08`, file `PLAN-cloud-G08.md`. +- Review closures: scope/context/verification/evidence/ownership/decision all `true`. Scores `scope=2,state=2,blast=1,evidence=2,verification=2`, grade `G09`; route `official-review`, `review/cloud/G09`, file `CODE_REVIEW-cloud-G09.md`. +- `large_indivisible_context=false`; positive loop-risk signatures: `temporal_state`, `boundary_contract`, `structured_interpretation`, `variant_product`; count `4`. +- Recovery signals: `review_rework_count=0`, `evidence_integrity_failure=false`. +- Capability gap: none. Cloud routing is caused by the combined state/contract/variant risk, not missing context. + +## Implementation Checklist + +- [ ] [REFACTOR-1] Separate caller-workspace and IOP-owned preset ownership without adding caller-selected raw paths. +- [ ] [REFACTOR-2] Enforce template PLAN → worker REVIEW handoff → reviewer inspection/repair and trustworthy final output on the caller-tool Hot Path. +- [ ] [TEST-3] Add deterministic regression coverage, run fresh local verification, and stop before the live IOP/OpenCode call with the resume condition recorded. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Separate execution ownership + +**Problem** + +`apps/edge/internal/openai/request_identity_ingress.go:389-406` correctly distinguishes marked `single_request` from unmarked caller `workspace_tools`, but the operational example only presents the Node-owned form (`configs/edge.yaml:552-596`). That makes it easy for agent-facing hybrid aliases to bind to the fixed Node workspace and hides the already-supported caller workspace boundary. + +**Solution** + +Keep the existing structural discriminator; do not introduce a new ownership enum or caller path field. Make caller-tool admission an explicit helper/invariant and document two non-overlapping preset examples: agent-bound hybrid is unmarked `light` with `workspace_tools`, while server-owned one-shot retains `single_request.workspace_ref`. Fail closed if a preset attempts to mix the two. + +Before (`request_identity_ingress.go:389-406`): + +```go +if preset.SingleRequest != nil || dispatch.SingleRequest != nil { + return nil, false, nil +} +if !isModeAllowed(preset, modeLight) { + return nil, false, nil +} +``` + +After: retain this behavior behind a named ownership/admission decision used by tests and comments; unmarked light compiles caller tools, marked single-request never does. + +**Modified Files and Checklist** + +- [ ] `apps/edge/internal/openai/request_identity_ingress.go`: make the ownership decision explicit without changing request schemas. +- [ ] `apps/edge/internal/openai/artifact_pair_test.go`: assert marked and unmarked presets cannot cross ownership paths. +- [ ] `configs/edge.yaml`: add a concise caller-workspace preset example beside the existing server-owned example. + +**Test Strategy** + +Write regression cases in `artifact_pair_test.go` for unmarked admission with matching caller tools, marked bypass, and mixed-config rejection through the existing config validator fixture. No new test harness. + +**Verification** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|Test.*Preset.*Workspace' +``` + +Expected: caller-tool cases pin an artifact frontier, marked cases do not, and mixed ownership fails closed. + +### [REFACTOR-2] Restore template handoff and reviewer responsibility + +**Problem** + +`hot_path_stage_input.go:127-138` tells Local to read both artifacts and tells Review to author REVIEW itself. `hot_path_light.go:795-819` transitions on any tool-free Local completion, and `hot_path_review.go:43-90` lets Review finalize after its own write/read cycle without proving it inspected the actual caller result. This reverses the intended handoff: Plan should analyze and write PLAN, Worker should execute and write REVIEW, and Reviewer should read both, inspect the result, repair/re-verify if needed, then own the final response. + +**Solution** + +Reuse `packages/go/singlerequesttemplate` defaults and validators; do not create a third result template. Validate the selector's PLAN plus pending REVIEW seed before caller writes are issued, retain the PLAN item IDs request-locally, require Work to read PLAN and replace REVIEW with a valid completed handoff before its completion can transition, then require Review to read both artifacts and complete at least one successful ordinary caller-result inspection. Review may issue ordinary repair/verification tools and only a later non-empty tool-free completion becomes the terminal output. Cleanup remains limited to the request `.iop/job` directory. + +Before (`hot_path_stage_input.go:127-138`): + +```go +case "local": + b.WriteString("Use the available caller tools to read both issued artifacts...") +case "review": + b.WriteString("Inspect the completed local work ... Then write the review...") +``` + +After: Local reads PLAN, works/verifies, and writes the template REVIEW handoff; Review reads PLAN+REVIEW, inspects actual output, repairs/re-verifies when needed, and supplies the final caller-visible result without rewriting REVIEW. + +**Modified Files and Checklist** + +- [ ] `apps/edge/internal/openai/artifact_pair.go`: validate/capture the template PLAN and pending REVIEW seed before issuing caller writes. +- [ ] `apps/edge/internal/openai/hot_path_stage_input.go`: provide phase-specific worker and reviewer instructions using the shared lightweight templates. +- [ ] `apps/edge/internal/openai/hot_path_light.go`: retain PLAN IDs, require a valid worker REVIEW handoff, track successful reviewer inspection, and preserve one repair stage. +- [ ] `apps/edge/internal/openai/hot_path_review.go`: remove reviewer-owned REVIEW rewrite and gate finalization on artifact reads, result inspection, non-empty final output, and completed repair tool frontiers. +- [ ] `apps/edge/internal/openai/artifact_pair_test.go`: reject malformed PLAN/pending REVIEW content before caller effects. +- [ ] `apps/edge/internal/openai/hot_path_light_test.go`: cover worker handoff, reviewer reads/inspection, repair, final output, and cleanup for OpenAI and Anthropic. +- [ ] `apps/edge/internal/openai/hot_path_review_test.go`: reject skipped inspection, empty terminal, malformed handoff, and second review cycle. +- [ ] `agent-contract/outer/anthropic-compatible-api.md`: distinguish unmarked caller continuation from marked single-request and record the caller-tool handoff/terminal invariant. +- [ ] `agent-spec/input/openai-compatible-surface.md`: update the current implementation map after code and contract agree. + +**Test Strategy** + +Write table-driven regressions for both protocols. Assertions: exact PLAN/REVIEW paths; template grammar and P1..Pn inventory; Work cannot finish before REVIEW handoff; Review must read PLAN and REVIEW and inspect at least one ordinary result; repair stays on the same reviewer binding; final content is non-empty; user output remains; cleanup removes only request artifacts; marked single-request behavior is unchanged. + +**Verification** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +``` + +Expected: all focused tests pass for OpenAI and Anthropic with no cached result. + +### [TEST-3] Verify locally, then pause before live IOP + +**Problem** + +The previous benchmark mixed concurrent runs and accepted summaries without checking caller-workspace provenance. Repeating the full harness would obscure this repair. + +**Solution** + +Run fresh repository tests and deterministic searches. Prepare exactly one isolated OpenCode HTML smoke directory and command, but do not call IOP during implementation. Record `awaiting deployed dev runtime and user continuation` in the review evidence; after deployment approval, run the exact command once and require caller-workspace `index.html`, non-empty final output, and no surviving `.iop/job` request directory. + +**Modified Files and Checklist** + +- [ ] `build/test-tmp/hybrid-caller-workspace-repair/index.html`: post-deploy smoke output; do not create it before the approved live call. +- [ ] `agent-task/hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G09.md`: record actual local output and the live-smoke pause/resume condition. + +**Test Strategy** + +Use existing package tests plus one isolated live OpenCode call. Do not run parallel callers, repeat runs, score quality, or invoke the discarded benchmark harness. + +**Verification** + +Local: + +```bash +gofmt -w apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/hot_path_stage_input.go apps/edge/internal/openai/hot_path_light.go apps/edge/internal/openai/hot_path_review.go apps/edge/internal/openai/artifact_pair_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_review_test.go +go test -count=1 ./apps/edge/internal/openai +go test -count=1 ./... +git diff --check +``` + +Expected: formatting is stable, all fresh tests pass, and the diff has no whitespace errors. + +Post-deploy, only after the user resumes the live step: + +```bash +mkdir -p build/test-tmp/hybrid-caller-workspace-repair +opencode run --dir "$PWD/build/test-tmp/hybrid-caller-workspace-repair" --model iop/gpt-hybrid --agent build --format json 'Create a small self-contained index.html that shows a centered card with the exact text IOP HYBRID OK. Verify the file exists and is valid HTML. Return a concise final summary.' +test -s build/test-tmp/hybrid-caller-workspace-repair/index.html +test "$(find build/test-tmp/hybrid-caller-workspace-repair/.iop/job -mindepth 1 -maxdepth 1 -type d 2>/dev/null | wc -l | tr -d ' ')" = "0" +``` + +Expected: one isolated run succeeds, `index.html` is non-empty in the caller workspace, the terminal result is non-empty, and no request artifact directory survives. + +## Modified Files Summary + +| File | Item | +|------|------| +| `apps/edge/internal/openai/request_identity_ingress.go` | REFACTOR-1 | +| `apps/edge/internal/openai/artifact_pair.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_stage_input.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_light.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_review.go` | REFACTOR-2 | +| `apps/edge/internal/openai/artifact_pair_test.go` | REFACTOR-1, REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_light_test.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_review_test.go` | REFACTOR-2 | +| `configs/edge.yaml` | REFACTOR-1 | +| `agent-contract/outer/anthropic-compatible-api.md` | REFACTOR-2 | +| `agent-spec/input/openai-compatible-surface.md` | REFACTOR-2 | +| `build/test-tmp/hybrid-caller-workspace-repair/index.html` | TEST-3 | +| `agent-task/hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G09.md` | TEST-3 | + +## Final Verification + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go test -count=1 ./apps/edge/internal/openai +go test -count=1 ./... +rg --sort path -n 'single_request|workspace_tools|hotPathPendingReview|review\.md' apps/edge/internal/openai configs/edge.yaml agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +git diff --check +``` + +Expected: focused and full fresh tests pass; deterministic search shows separate marked/unmarked ownership and the new worker-handoff/reviewer-inspection flow; no formatting error exists. Cached Go test output is not acceptable (`-count=1`). The live OpenCode command remains paused until deployment and explicit user continuation, then must satisfy the caller-workspace and cleanup assertions in TEST-3. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_2.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_2.log new file mode 100644 index 00000000..68e07b2d --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/plan_cloud_G08_2.log @@ -0,0 +1,231 @@ + + +# Phase-Specific Selector Instruction Repair Plan + +## For the Implementing Agent + +Fill every implementation-owned section in `CODE_REVIEW-cloud-G08.md`, run the listed verification exactly, and leave both active files in place for review. If blocked, record only the exact blocker, attempted command/output, and resume condition in implementation-owned evidence. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, write `complete.log`, or execute the paused live OpenCode call. + +## Background + +The provider now receives request-local artifact paths and grammar, but the same pair-write instruction is used for both selector phases. A non-parent-creating workspace binding requires an initial prepare-only call, so a real provider following the instruction is rejected even though the call-index-driven test fake passes. + +## Archive Evidence Snapshot + +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G07_1.log`: the prior follow-up required actual provider-bound selector knowledge for initial/resume turns while preserving caller snapshots and marked execution. +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log`: official review verdict `FAIL`; Required R1 proved that the initial provider instruction requires two writes while the retained frontier accepts exactly one prepare call, and that the fake chooses its response by call index instead of instruction semantics. + +## Finding Resolution Map + +| Finding | Evidence | Root Cause | Selected Fix | Mode | Changed Precondition | Acceptance | +|---|---|---|---|---|---|---| +| Required R1 | `provider_model_rewrite.go:21` requires two writes, while `artifact_pair.go:201-205,272-280` requires one prepare call for a pinned non-parent-creating binding; `hot_path_direct_test.go:207-214,305` hides the conflict with call-index response selection. | The instruction builder receives only request ID and cannot observe the locked frontier phase/binding capability. | Expose a lock-safe `prepare-only` versus `pair-write` instruction state, render the exact phase operation, and make both protocol fixtures derive their operation from the provider-bound instruction. | direct-fix | A real initial selector can follow the body instruction and produce exactly the operation accepted by the current frontier; resume then receives the pair grammar. | Fresh focused/package/repository tests, vet, deterministic searches, and diff check pass. | + +## Analysis + +### Files Read + +- `apps/edge/internal/openai/provider_model_rewrite.go` +- `apps/edge/internal/openai/request_identity_ingress.go` +- `apps/edge/internal/openai/artifact_pair.go` +- `apps/edge/internal/openai/hot_path_direct_test.go` +- `apps/edge/internal/openai/hot_path_light_test.go` +- `apps/edge/internal/openai/hot_path_chat_gate_test.go` +- `agent-contract/outer/anthropic-compatible-api.md` +- `agent-spec/input/openai-compatible-surface.md` +- `agent-task/archive/hybrid_caller_workspace_repair/plan_cloud_G07_1.log` +- `agent-task/archive/hybrid_caller_workspace_repair/code_review_cloud_G08_1.log` + +### SDD Criteria + +Not applicable. This is non-Milestone bounded review rework and does not change the selected ownership or public schema boundary. + +### Verification Context + +Reviewer-fresh focused OpenAI tests, the package test, service/shared vet and tests, the uncached repository suite, metadata privilege search, and `git diff --check` ran locally. All commands passed (the no-match privilege search exited 1 as expected), but source tracing found contradictory provider instructions. External live OpenCode verification remains paused with the exact resume condition `dev deployment complete and user explicitly resumes live call`; this follow-up must not execute it. Confidence is high because the rejected call cardinalities are explicit in production source and the fake selection is explicit in test source. + +### Test Coverage Gaps + +- Existing tests verify provider-body paths and grammar, but do not require the scripted provider response operation to follow the instruction. +- Add prepare-only and pair-write instruction assertions for both Chat and Anthropic, including the non-parent-creating initial/resume handler sequence. + +### Symbol References + +No public symbol is renamed or removed. Internal instruction helper signatures may change; update all package-local references found by `rg --sort path -n 'buildHotPathSelectorProviderInstruction|hotPathSelectorProviderInstruction|scriptedSelectorInstruction' apps/edge/internal/openai`. + +### Split Judgment + +Keep one plan. Frontier phase observation, provider text, and the fake response oracle form one indivisible correctness invariant and must pass together. + +### Scope Rationale + +Do not change caller snapshots, public API/config fields, workspace receipt mapping, Work/Review/cleanup phases, marked `single_request`, routing, or the paused live deployment path. Do not redesign tool schemas; use the already admitted prepare/write capabilities. + +### Final Routing + +- `evaluation_mode=isolated-reassessment`; all build/review closures are true; no capability gap. +- Build scores `2,2,1,2,1` produce `G08`, base `local-fit`; `review_rework_count=2` and `evidence_integrity_failure=true` select `recovery-boundary`, `cloud/G08`, `PLAN-cloud-G08.md`. +- Review scores `2,2,1,2,1` produce `official-review`, `cloud/G08`, `CODE_REVIEW-cloud-G08.md`. +- `large_indivisible_context=false`; positive signatures: `temporal_state`, `boundary_contract`, `structured_interpretation`, `variant_product` (`loop_risk_count=4`). Finalizer: `finalize-task-policy.sh pair`. + +## Implementation Checklist + +- [ ] [FIX-R1] Make selector provider instructions phase-specific and consistent with the retained artifact frontier. +- [ ] [TEST-R1] Make Chat/Anthropic scripted selectors choose prepare or pair from the actual provider-bound instruction and cover swapped-operation rejection. +- [ ] [DOC-R1] Synchronize the Anthropic contract and living spec with prepare-only initial and pair-write resume semantics. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [FIX-R1] Bind instruction semantics to the frontier phase + +**Problem** + +At `provider_model_rewrite.go:21`, every instruction requires two writes. At `artifact_pair.go:201-205,272-280`, a pinned binding whose write cannot create parents accepts exactly one prepare call. + +**Solution** + +Add a lock-safe artifact-frontier query that returns no instruction, `prepare-only`, or `pair-write` from the record phase and `binding.createsParents()`. Pass that closed state into the instruction builder. The prepare instruction must require exactly one admitted prepare call for `.iop/job/` and prohibit PLAN/REVIEW writes. The pair instruction retains exact paths and grammar. `artifactPhasePairReady`, or initial pinned with parent-creating writes, selects pair-write. + +Before (`provider_model_rewrite.go:21`): + +```go +instruction := fmt.Sprintf(`IOP caller-workspace selector instruction. Return the request-local artifact pair through exactly two write tool calls ...`) +``` + +After: + +```go +switch instructionState { +case selectorInstructionPrepareOnly: + // exactly one prepare call for issued.JobDir; no writes +case selectorInstructionPairWrite: + // exact PLAN/REVIEW pair and grammar +} +``` + +**Modified Files and Checklist** + +- [ ] `apps/edge/internal/openai/artifact_pair.go`: expose the lock-safe instruction state without leaking the binding. +- [ ] `apps/edge/internal/openai/request_identity_ingress.go`: request the exact current state and build only that instruction. +- [ ] `apps/edge/internal/openai/provider_model_rewrite.go`: render bounded prepare-only and pair-write text. + +**Test Strategy** + +Covered by TEST-R1; do not add another harness. + +**Verification** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPairHandlerDisposition' +``` + +Expected: initial prepare and resumed pair are both accepted for Chat and Anthropic. + +### [TEST-R1] Remove call-index operation privilege + +**Problem** + +`hot_path_direct_test.go:207-214,305` selects prepare/pair from call order after parsing only the request ID, so the fake can contradict the provider instruction and still pass. + +**Solution** + +Parse a closed operation directive from the actual provider-bound Chat system message or Anthropic system block. Make the scripted service choose `scriptedArtifactPrepare` only for `prepare-only` and `scriptedArtifactPair` only for `pair-write`; retain call count only as an assertion, not an oracle. Add direct tests that both protocols preserve caller system content and reject/flag swapped operation instructions. Assert marked `single_request` remains instruction-free. + +Before (`hot_path_direct_test.go:207-214`): + +```go +switch call { +case 1: + return scriptedArtifactPrepare(endpoint, requestID) +case 2: + return scriptedArtifactPair(endpoint, requestID) +} +``` + +After: + +```go +switch instructionState { +case selectorInstructionPrepareOnly: + return scriptedArtifactPrepare(endpoint, requestID) +case selectorInstructionPairWrite: + return scriptedArtifactPair(endpoint, requestID) +} +``` + +**Modified Files and Checklist** + +- [ ] `apps/edge/internal/openai/hot_path_direct_test.go`: derive the scripted operation from the provider body and add both-protocol regressions. + +**Test Strategy** + +Extend `TestHotPathSelectorInstruction` and `TestArtifactPairHandlerDisposition`. The fixture uses its existing non-parent-creating write plus prepare tool, proves first body says prepare-only, second says pair-write, and proves no response depends on internal metadata or call index. + +**Verification** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPairHandlerDisposition' +rg --sort path -n 'scriptedArtifactPrepare|scriptedArtifactPair|selectorInstructionPrepareOnly|selectorInstructionPairWrite' apps/edge/internal/openai/hot_path_direct_test.go apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/artifact_pair.go +``` + +Expected: both protocols pass and the deterministic search shows provider-body state driving the fixture operation. + +### [DOC-R1] Record phase-specific provider knowledge + +**Problem** + +The contract and spec currently say initial/resume bodies carry the pair grammar without describing the mandatory prepare-only initial variant. + +**Solution** + +Document that a binding requiring parent preparation receives one prepare-only instruction first, then the exact pair instruction after the receipt; parent-creating bindings receive the pair instruction immediately. Preserve the no-public-field/no-raw-path boundary. + +**Modified Files and Checklist** + +- [ ] `agent-contract/outer/anthropic-compatible-api.md`: update the provider instruction contract. +- [ ] `agent-spec/input/openai-compatible-surface.md`: update the current implementation map and change record. + +**Test Strategy** + +No document-only test; deterministic search plus source tests validate synchronization. + +**Verification** + +```bash +rg --sort path -n 'prepare-only|pair-write|selector instruction' agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +``` + +Expected: both documents state the same phase-specific operation boundary. + +## Modified Files Summary + +| File | Item | +|---|---| +| `apps/edge/internal/openai/artifact_pair.go` | FIX-R1 | +| `apps/edge/internal/openai/request_identity_ingress.go` | FIX-R1 | +| `apps/edge/internal/openai/provider_model_rewrite.go` | FIX-R1 | +| `apps/edge/internal/openai/hot_path_direct_test.go` | TEST-R1 | +| `agent-contract/outer/anthropic-compatible-api.md` | DOC-R1 | +| `agent-spec/input/openai-compatible-surface.md` | DOC-R1 | +| `agent-task/hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md` | implementation evidence | + +## Final Verification + +Fresh results are required; Go test caching is not acceptable. + +```bash +gofmt -w apps/edge/internal/openai/artifact_pair.go apps/edge/internal/openai/request_identity_ingress.go apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/hot_path_direct_test.go +go test -count=1 ./apps/edge/internal/openai -run 'TestHotPathSelectorInstruction|TestArtifactPair|TestHotPathLight|TestHotPathReview|TestHotPathStageInput' +go test -count=1 ./apps/edge/internal/openai +go vet ./apps/edge/internal/service +go test ./apps/edge/internal/service -count=1 +go vet ./packages/go/... +go test ./packages/go/... -count=1 +go test -count=1 ./... +rg --sort path -n 'prepare-only|pair-write|selector.*instruction|scriptedArtifactPrepare|scriptedArtifactPair' apps/edge/internal/openai agent-contract/outer/anthropic-compatible-api.md agent-spec/input/openai-compatible-surface.md +test -z "$(rg -n 'req\.Run\.Metadata\["iop_logical_request_id"\]' apps/edge/internal/openai/hot_path_direct_test.go apps/edge/internal/openai/hot_path_light_test.go apps/edge/internal/openai/hot_path_chat_gate_test.go)" +git diff --check +``` + +Expected: all tests and vet commands pass; both protocols derive prepare/pair from the actual provider body; no selector fixture reads internal request metadata; docs match source; diff is clean. Do not execute the live OpenCode smoke. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/archive/2026/08/hybrid_caller_workspace_repair/work_log_0.log b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/work_log_0.log new file mode 100644 index 00000000..6ab4c92f --- /dev/null +++ b/agent-task/archive/2026/08/hybrid_caller_workspace_repair/work_log_0.log @@ -0,0 +1,18 @@ +# Milestone Work Log + +> Dispatcher-owned execution timeline. Workers and reviewers do not edit this file. + +| seq | time | event | task | loop | role | attempt | model | result | locator | +|---:|---|---|---|---:|---|---:|---|---|---| +| 1 | 26-08-14 20:59:39 KST | START | hybrid_caller_workspace_repair/PLAN-cloud-G08.md | 0 | worker | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T205939+0900__hybrid_caller_workspace_repair__p0__worker__a00/locator.json | +| 2 | 26-08-14 21:26:08 KST | FINISH | hybrid_caller_workspace_repair/PLAN-cloud-G08.md | 0 | worker | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T205939+0900__hybrid_caller_workspace_repair__p0__worker__a00/locator.json | +| 3 | 26-08-14 21:26:09 KST | START | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G09.md | 0 | review | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T212609+0900__hybrid_caller_workspace_repair__p0__review__a00/locator.json | +| 4 | 26-08-14 21:39:16 KST | FINISH | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G09.md | 0 | review | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T212609+0900__hybrid_caller_workspace_repair__p0__review__a00/locator.json | +| 5 | 26-08-14 21:39:41 KST | START | hybrid_caller_workspace_repair/PLAN-cloud-G07.md | 1 | worker | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T213941+0900__hybrid_caller_workspace_repair__p1__worker__a00/locator.json | +| 6 | 26-08-14 21:55:10 KST | FINISH | hybrid_caller_workspace_repair/PLAN-cloud-G07.md | 1 | worker | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T213941+0900__hybrid_caller_workspace_repair__p1__worker__a00/locator.json | +| 7 | 26-08-14 21:55:10 KST | START | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md | 1 | review | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T215510+0900__hybrid_caller_workspace_repair__p1__review__a00/locator.json | +| 8 | 26-08-14 22:06:53 KST | FINISH | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md | 1 | review | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T215510+0900__hybrid_caller_workspace_repair__p1__review__a00/locator.json | +| 9 | 26-08-14 22:07:19 KST | START | hybrid_caller_workspace_repair/PLAN-cloud-G08.md | 2 | worker | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T220719+0900__hybrid_caller_workspace_repair__p2__worker__a00/locator.json | +| 10 | 26-08-14 22:20:23 KST | FINISH | hybrid_caller_workspace_repair/PLAN-cloud-G08.md | 2 | worker | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T220719+0900__hybrid_caller_workspace_repair__p2__worker__a00/locator.json | +| 11 | 26-08-14 22:20:23 KST | START | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md | 2 | review | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T222023+0900__hybrid_caller_workspace_repair__p2__review__a00/locator.json | +| 12 | 26-08-14 22:26:11 KST | FINISH | hybrid_caller_workspace_repair/CODE_REVIEW-cloud-G08.md | 2 | review | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T222023+0900__hybrid_caller_workspace_repair__p2__review__a00/locator.json | diff --git a/apps/edge/internal/openai/anthropic_handler.go b/apps/edge/internal/openai/anthropic_handler.go index 8bf86dff..211ef7ed 100644 --- a/apps/edge/internal/openai/anthropic_handler.go +++ b/apps/edge/internal/openai/anthropic_handler.go @@ -655,6 +655,10 @@ func (s *Server) buildAnthropicPoolRequest( if dispatch.Managed { poolReq.AcceptCandidate = composeCandidatePredicates(poolReq.AcceptCandidate, dispatch.CandidatePredicate()) } + selectorInstruction, err := s.hotPathSelectorProviderInstruction(metadata) + if err != nil { + return edgeservice.ProviderPoolDispatchRequest{}, presetIngressResult{}, err + } poolReq.PrepareProtocolTunnel = func(tunnelReq edgeservice.SubmitProviderTunnelRequest, selected edgeservice.ProviderPoolCandidate) (edgeservice.SubmitProviderTunnelRequest, error) { if selected.ProtocolProfile == nil { return tunnelReq, newAnthropicClientError("not_supported_error", fmt.Errorf("selected provider has no concrete protocol profile")) @@ -707,7 +711,7 @@ func (s *Server) buildAnthropicPoolRequest( default: return tunnelReq, newAnthropicClientError("not_supported_error", fmt.Errorf("unsupported protocol driver %q", profile.Driver)) } - return tunnelReq, nil + return prepareHotPathSelectorProviderInstruction(tunnelReq, selectorInstruction) } return poolReq, presetIngress, nil } diff --git a/apps/edge/internal/openai/artifact_pair.go b/apps/edge/internal/openai/artifact_pair.go index 66430a33..be93a13b 100644 --- a/apps/edge/internal/openai/artifact_pair.go +++ b/apps/edge/internal/openai/artifact_pair.go @@ -7,6 +7,8 @@ import ( "net/http" "strings" "sync" + + "iop/packages/go/singlerequesttemplate" ) const defaultArtifactFrontierCapacity = 1024 @@ -21,6 +23,14 @@ const ( artifactPhaseLocalEligible artifactFrontierPhase = "local_eligible" ) +type selectorInstructionState string + +const ( + selectorInstructionNone selectorInstructionState = "" + selectorInstructionPrepareOnly selectorInstructionState = "prepare-only" + selectorInstructionPairWrite selectorInstructionState = "pair-write" +) + type artifactDispositionKind string const ( @@ -31,6 +41,7 @@ const ( type artifactDisposition struct { Kind artifactDispositionKind SelectorStageID string + PlanItemIDs []string PrimaryError *hotPathEndpointError } @@ -74,6 +85,7 @@ type artifactFrontierRecord struct { pendingHash string consumedHashes map[string]struct{} consumedIDs map[string]struct{} + planItemIDs []string } // artifactFrontierStore owns the request-local workspace binding and the sole @@ -159,6 +171,32 @@ func (s *artifactFrontierStore) has(requestID, ownerEdgeID string) bool { return record != nil && record.ownerEdgeID == ownerEdgeID } +// selectorInstructionState returns the exact operation the retained selector +// may emit at the current artifact frontier. The binding capability and phase +// stay under the store lock and are never reconstructed from request metadata. +func (s *artifactFrontierStore) selectorInstructionState(requestID, ownerEdgeID string) selectorInstructionState { + if s == nil || requestID == "" { + return selectorInstructionNone + } + s.mu.Lock() + defer s.mu.Unlock() + record := s.records[requestID] + if record == nil || record.ownerEdgeID != ownerEdgeID || record.binding == nil { + return selectorInstructionNone + } + switch record.phase { + case artifactPhasePinned: + if record.binding.createsParents() { + return selectorInstructionPairWrite + } + return selectorInstructionPrepareOnly + case artifactPhasePairReady: + return selectorInstructionPairWrite + default: + return selectorInstructionNone + } +} + // pairRequired reports whether the retained selector may only author the // exact Plan/Review pair. The store owns the phase and keeps this observation // lock-safe so a handler cannot infer it from untrusted request metadata. @@ -295,6 +333,7 @@ func mapArtifactOutput( orderedPaths := []string{issued.PlanPath, issued.ReviewPath} mappedCalls := make([]normalizedToolCall, 0, 2) payloads := make(map[string]*workspaceEncodedPayload, 2) + var planItemIDs []string for _, requiredPath := range orderedPaths { call, ok := byPath[cleanRelativePath(requiredPath)] if !ok { @@ -304,12 +343,82 @@ func mapArtifactOutput( if err != nil { return normalizedStageOutput{}, nil, err } + content, err := artifactWriteContent(record.binding, call) + if err != nil { + return normalizedStageOutput{}, nil, err + } + if cleanRelativePath(requiredPath) == cleanRelativePath(issued.PlanPath) { + plan, err := singlerequesttemplate.ParsePlan( + singlerequesttemplate.DefaultPlanTemplate, + content, + singlerequesttemplate.MaxTemplateBytes, + ) + if err != nil { + return normalizedStageOutput{}, nil, fmt.Errorf("artifact plan does not match the lightweight template: %w", err) + } + planItemIDs, err = singlerequesttemplate.PlanItemIDs(plan) + if err != nil { + return normalizedStageOutput{}, nil, fmt.Errorf("artifact plan item inventory is invalid: %w", err) + } + } else if err := validatePendingReviewSeed(content, planItemIDs); err != nil { + return normalizedStageOutput{}, nil, err + } mappedCalls = append(mappedCalls, mapped) payloads[mapped.ID] = payload } + record.planItemIDs = append([]string(nil), planItemIDs...) return artifactResponseOutput(output, mappedCalls), payloads, nil } +func artifactWriteContent(binding *workspaceBinding, call normalizedToolCall) (string, error) { + if binding == nil { + return "", fmt.Errorf("artifact write binding is unavailable") + } + write := binding.operation(opKindWrite) + if write == nil || strings.TrimSpace(write.contentField) == "" { + return "", fmt.Errorf("artifact write content binding is unavailable") + } + value, ok := lookupMappedArgument(call.Arguments, write.contentField) + if !ok { + return "", fmt.Errorf("artifact write is missing template content") + } + content, ok := value.(string) + if !ok || strings.TrimSpace(content) == "" { + return "", fmt.Errorf("artifact write template content must be a non-empty string") + } + return content, nil +} + +func pendingReviewSeed(planItemIDs []string) ([]byte, error) { + if len(planItemIDs) == 0 { + return nil, singlerequesttemplate.ErrMalformedReview + } + statuses := make([]string, len(planItemIDs)) + for i, id := range planItemIDs { + statuses[i] = fmt.Sprintf("- %s: pending", id) + } + return singlerequesttemplate.RenderReview( + singlerequesttemplate.DefaultReviewTemplate, + singlerequesttemplate.ReviewFields{ + ItemStatus: strings.Join(statuses, "\n"), + Changes: "Pending worker execution.", Verification: "Pending worker verification.", + Deviations: "None recorded.", + }, + singlerequesttemplate.MaxTemplateBytes, + ) +} + +func validatePendingReviewSeed(content string, planItemIDs []string) error { + want, err := pendingReviewSeed(planItemIDs) + if err != nil { + return fmt.Errorf("build pending review seed: %w", err) + } + if strings.TrimSpace(content) != strings.TrimSpace(string(want)) { + return fmt.Errorf("artifact review seed does not match the lightweight pending template") + } + return nil +} + func mapArtifactCall( binding *workspaceBinding, providerCall normalizedToolCall, @@ -509,6 +618,7 @@ func (s *artifactFrontierStore) consume( if primaryFailure != nil { return snap, artifactDisposition{ Kind: artifactDispositionLocalEligible, SelectorStageID: record.selectorStageID, + PlanItemIDs: append([]string(nil), record.planItemIDs...), PrimaryError: primaryFailure, }, true, nil } @@ -523,7 +633,10 @@ func (s *artifactFrontierStore) consume( return snap, artifactDisposition{Kind: artifactDispositionResumeSelector, SelectorStageID: record.selectorStageID}, true, nil case artifactPhasePairPending: record.phase = artifactPhaseLocalEligible - return snap, artifactDisposition{Kind: artifactDispositionLocalEligible, SelectorStageID: record.selectorStageID}, true, nil + return snap, artifactDisposition{ + Kind: artifactDispositionLocalEligible, SelectorStageID: record.selectorStageID, + PlanItemIDs: append([]string(nil), record.planItemIDs...), + }, true, nil default: return logicalRequestSnapshot{}, artifactDisposition{}, true, fmt.Errorf("artifact frontier phase cannot consume results") } diff --git a/apps/edge/internal/openai/artifact_pair_test.go b/apps/edge/internal/openai/artifact_pair_test.go index ab7e4c00..1f03badb 100644 --- a/apps/edge/internal/openai/artifact_pair_test.go +++ b/apps/edge/internal/openai/artifact_pair_test.go @@ -12,8 +12,42 @@ import ( "testing" "iop/packages/go/config" + "iop/packages/go/singlerequesttemplate" ) +func testLightweightPlan(t *testing.T) string { + t.Helper() + plan, err := singlerequesttemplate.RenderPlan(singlerequesttemplate.DefaultPlanTemplate, singlerequesttemplate.PlanFields{ + Goal: "Complete the caller workspace task", Steps: []string{"Inspect the requested result", "Implement and verify the result"}, + Verification: []string{"Confirm the caller workspace result"}, + }, singlerequesttemplate.MaxTemplateBytes) + if err != nil { + t.Fatal(err) + } + return string(plan) +} + +func testPendingReview(t *testing.T) string { + t.Helper() + review, err := pendingReviewSeed([]string{"P1", "P2"}) + if err != nil { + t.Fatal(err) + } + return string(review) +} + +func testCompletedReview(t *testing.T) string { + t.Helper() + review, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, singlerequesttemplate.ReviewFields{ + ItemStatus: "- P1: completed\n- P2: completed", Changes: "Implemented the requested caller workspace result.", + Verification: "Verified the requested result with caller tools.", Deviations: "None.", + }, singlerequesttemplate.MaxTemplateBytes) + if err != nil { + t.Fatal(err) + } + return string(review) +} + func TestArtifactPairFrontierMatrix(t *testing.T) { for _, endpoint := range []string{"openai", "anthropic"} { endpoint := endpoint @@ -189,6 +223,32 @@ func TestArtifactPairFrontierMatrix(t *testing.T) { } } +func TestArtifactPairRejectsMalformedTemplatesBeforeCallerEffects(t *testing.T) { + fixture := newArtifactPairFixture(t, "openai", true) + _, err := fixture.issue([]normalizedToolCall{ + artifactProviderWrite("provider_plan", fixture.paths.PlanPath, "not a plan"), + artifactProviderWrite("provider_review", fixture.paths.ReviewPath, testPendingReview(t)), + }) + if err == nil || !strings.Contains(err.Error(), "lightweight template") { + t.Fatalf("malformed plan error = %v", err) + } + if fixture.server.artifactFrontiers.has(fixture.requestID, fixture.ownerEdgeID) || fixture.server.lightFlows.has(fixture.requestID, fixture.ownerEdgeID) { + t.Fatal("malformed artifact retained a caller-effect frontier") + } +} + +func TestPresetWorkspaceOwnershipAdmission(t *testing.T) { + unmarked := hotPathSelectorPreset([]string{config.ModeLight}) + if !presetAdmitsCallerWorkspaceTools(unmarked, routeDispatch{Preset: unmarked}) { + t.Fatal("unmarked light preset did not admit caller workspace tools") + } + marked := unmarked.Clone() + marked.SingleRequest = &config.ExecutionSingleRequestPolicy{} + if presetAdmitsCallerWorkspaceTools(marked, routeDispatch{Preset: marked}) { + t.Fatal("marked single-request preset crossed into caller workspace tools") + } +} + type artifactPairFixture struct { t *testing.T endpoint string @@ -274,8 +334,8 @@ func (f *artifactPairFixture) issuePrepare() []string { func (f *artifactPairFixture) issuePair() []string { f.t.Helper() ids, err := f.issue([]normalizedToolCall{ - artifactProviderWrite("provider_plan", f.paths.PlanPath, "plan"), - artifactProviderWrite("provider_review", f.paths.ReviewPath, "review"), + artifactProviderWrite("provider_plan", f.paths.PlanPath, testLightweightPlan(f.t)), + artifactProviderWrite("provider_review", f.paths.ReviewPath, testPendingReview(f.t)), }) if err != nil { f.t.Fatalf("issue pair: %v", err) diff --git a/apps/edge/internal/openai/chat_handler.go b/apps/edge/internal/openai/chat_handler.go index 86747ccb..324b0417 100644 --- a/apps/edge/internal/openai/chat_handler.go +++ b/apps/edge/internal/openai/chat_handler.go @@ -382,12 +382,22 @@ func (s *Server) handleChatCompletionsProviderPool(w http.ResponseWriter, dc *ch return tunnelReq, nil } baseProtocolPreparer := s.protocolTunnelPreparer(r, config.OperationChatCompletions) + selectorInstruction, err := s.hotPathSelectorProviderInstruction(dc.runMetadata) + if err != nil { + dc.finishUsageRequest(usageStatusError, responseModePassthrough) + writeError(w, http.StatusInternalServerError, "run_error", "selector provider instruction is unavailable") + return + } poolReq.PrepareProtocolTunnel = func(tunnelReq edgeservice.SubmitProviderTunnelRequest, selected edgeservice.ProviderPoolCandidate) (edgeservice.SubmitProviderTunnelRequest, error) { prepared, err := baseProtocolPreparer(tunnelReq, selected) if err != nil { return tunnelReq, err } - return prepareProviderChatToolCallNormalization(prepared, selected) + prepared, err = prepareProviderChatToolCallNormalization(prepared, selected) + if err != nil { + return tunnelReq, err + } + return prepareHotPathSelectorProviderInstruction(prepared, selectorInstruction) } // strict-output output policy only applies to normalized dispatch, diff --git a/apps/edge/internal/openai/hot_path_anthropic_gate_test.go b/apps/edge/internal/openai/hot_path_anthropic_gate_test.go index c8fa986c..a3b90e32 100644 --- a/apps/edge/internal/openai/hot_path_anthropic_gate_test.go +++ b/apps/edge/internal/openai/hot_path_anthropic_gate_test.go @@ -152,11 +152,11 @@ func TestHotPathAnthropicDirectStreamPreservesEmptyToolInput(t *testing.T) { func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { fixture := newScriptedLightFixture(t, "anthropic", false) - fixture.service.responses[3] = func(string) string { + fixture.service.responses[4] = func(string) string { return scriptedLightCompletionWithUsage("anthropic", "local-visible", "local-reason", 5, 3) } - fixture.service.responses[4] = func(requestID string) string { - return scriptedReviewWriteWithUsage("anthropic", requestID, 7, 4) + fixture.service.responses[5] = func(requestID string) string { + return scriptedReviewReadsAndInspectionWithUsage(requestID, 7, 4) } prepare := fixture.request() @@ -165,6 +165,8 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { fixture.consumeToolResponse(pair, []string{`{"written":true}`, `{"written":true}`}) localRead := fixture.request() fixture.consumeToolResponse(localRead, []string{`{"written":true}`}) + workerHandoff := fixture.request() + fixture.consumeToolResponse(workerHandoff, []string{`{"ok":true}`, `{"written":true}`}) before := len(fixture.service.snapshots()) response := fixture.requestWithOptions(64, true) @@ -178,7 +180,7 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { assertCapturedHotPathBudget(t, requests[len(requests)-2], fixture.service.candidate, 64) assertCapturedHotPathBudget(t, requests[len(requests)-1], fixture.service.candidate, 61) events := decodeHotPathAnthropicSSE(t, response.Body.String()) - assertHotPathAnthropicBlockIndexes(t, events, 5) + assertHotPathAnthropicBlockIndexes(t, events, 7) wantNames := []string{ "message_start", @@ -187,6 +189,8 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { "content_block_start", "content_block_delta", "content_block_delta", "content_block_stop", "content_block_start", "content_block_delta", "content_block_stop", "content_block_start", "content_block_delta", "content_block_stop", + "content_block_start", "content_block_delta", "content_block_stop", + "content_block_start", "content_block_delta", "content_block_stop", "message_delta", "message_stop", } if got := hotPathAnthropicEventNames(events); strings.Join(got, ",") != strings.Join(wantNames, ",") { @@ -198,17 +202,18 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { t.Fatalf("outer identity mismatch: message=%+v logical_request=%s", startMessage, requestID) } - wantKinds := []string{"thinking", "text", "thinking", "text", "tool_use"} + wantKinds := []string{"thinking", "text", "thinking", "text", "tool_use", "tool_use", "tool_use"} var gotKinds, thinking, text []string - var toolID, toolName, toolArgs, stopReason string + var toolIDs, toolNames, toolArgs []string + var stopReason string for _, event := range events { switch event.name { case "content_block_start": block := hotPathAnthropicMap(t, event.payload["content_block"]) gotKinds = append(gotKinds, fmt.Sprint(block["type"])) if block["type"] == "tool_use" { - toolID, _ = block["id"].(string) - toolName, _ = block["name"].(string) + toolIDs = append(toolIDs, fmt.Sprint(block["id"])) + toolNames = append(toolNames, fmt.Sprint(block["name"])) } case "content_block_delta": delta := hotPathAnthropicMap(t, event.payload["delta"]) @@ -218,7 +223,7 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { case "text_delta": text = append(text, fmt.Sprint(delta["text"])) case "input_json_delta": - toolArgs += fmt.Sprint(delta["partial_json"]) + toolArgs = append(toolArgs, fmt.Sprint(delta["partial_json"])) } case "message_delta": delta := hotPathAnthropicMap(t, event.payload["delta"]) @@ -231,15 +236,15 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { } if strings.Join(gotKinds, ",") != strings.Join(wantKinds, ",") || strings.Join(thinking, "") != "local-reasonreview-reason" || strings.Join(text, "") != "local-visiblereview-visible" || - toolName != "write_file" || !json.Valid([]byte(toolArgs)) || stopReason != "tool_use" { + strings.Join(toolNames, ",") != "read_file,read_file,run_command" || len(toolArgs) != 3 || stopReason != "tool_use" { t.Fatalf("multi-stage output mismatch: kinds=%v thinking=%v text=%v tool=%q/%q/%q stop=%q body=%s", - gotKinds, thinking, text, toolID, toolName, toolArgs, stopReason, response.Body.String()) + gotKinds, thinking, text, toolIDs, toolNames, toolArgs, stopReason, response.Body.String()) } - if len(snapshot.ExpectedCallIDs) != 1 || snapshot.ExpectedCallIDs[0] != toolID { - t.Fatalf("tool correlation mismatch: tool=%q snapshot=%+v", toolID, snapshot) + if len(snapshot.ExpectedCallIDs) != 3 { + t.Fatalf("tool correlation mismatch: tools=%q snapshot=%+v", toolIDs, snapshot) } - if toolID != "msg-light-complete-tool-1" || strings.Contains(response.Body.String(), "msg-review-write") { - t.Fatalf("public identity/tool namespace leaked a later provider id: tool=%q body=%s", toolID, response.Body.String()) + if strings.Join(toolIDs, ",") != "msg-light-complete-tool-1,msg-light-complete-tool-2,msg-light-complete-tool-3" || strings.Contains(response.Body.String(), "msg-review-inspection") { + t.Fatalf("public identity/tool namespace leaked a later provider id: tools=%q body=%s", toolIDs, response.Body.String()) } } diff --git a/apps/edge/internal/openai/hot_path_chat_gate_test.go b/apps/edge/internal/openai/hot_path_chat_gate_test.go index 2dfae1c0..ce371553 100644 --- a/apps/edge/internal/openai/hot_path_chat_gate_test.go +++ b/apps/edge/internal/openai/hot_path_chat_gate_test.go @@ -145,10 +145,6 @@ func TestHotPathChatCallerCapAndNonStream(t *testing.T) { } func TestHotPathChatMixedProviderStages(t *testing.T) { - decodedReview, err := decodeAnthropicPresetSSE([]byte(hotPathChatMixedReviewSSE("req-decode-check"))) - if err != nil || len(decodedReview.ToolCalls) != 1 || len(decodedReview.Deltas) != 3 { - t.Fatalf("mixed review fixture decode: tools=%d deltas=%d err=%v output=%+v", len(decodedReview.ToolCalls), len(decodedReview.Deltas), err, decodedReview) - } openAICandidate := anthropicTestCandidate(t, "openai") anthropicCandidate := anthropicTestCandidate(t, "anthropic") service := &hotPathChatGateScriptedService{} @@ -156,8 +152,9 @@ func TestHotPathChatMixedProviderStages(t *testing.T) { {candidate: openAICandidate, body: func(requestID string) string { return scriptedArtifactPrepare("openai", requestID) }}, {candidate: openAICandidate, body: func(requestID string) string { return scriptedArtifactPair("openai", requestID) }}, {candidate: openAICandidate, body: func(requestID string) string { return scriptedArtifactLocalRead("openai", requestID) }}, + {candidate: openAICandidate, body: func(requestID string) string { return scriptedLocalWorkAndHandoff("openai", requestID) }}, {candidate: openAICandidate, contentType: "text/event-stream", body: func(string) string { return hotPathChatMixedLocalSSE() }}, - {candidate: anthropicCandidate, contentType: "text/event-stream", body: hotPathChatMixedReviewSSE}, + {candidate: anthropicCandidate, body: func(requestID string) string { return scriptedReviewReadsAndInspectionWithUsage(requestID, 7, 3) }}, } preset := hotPathSelectorPreset([]string{config.ModeDirect, config.ModeLight}) @@ -195,6 +192,7 @@ func TestHotPathChatMixedProviderStages(t *testing.T) { consume(request(false), []string{`{"written":true}`}) consume(request(false), []string{`{"written":true}`, `{"written":true}`}) consume(request(false), []string{`{"written":true}`}) + consume(request(false), []string{`{"ok":true}`, `{"written":true}`}) response := request(true) if response.Code != http.StatusOK { t.Fatalf("status=%d body=%s", response.Code, response.Body.String()) @@ -205,9 +203,8 @@ func TestHotPathChatMixedProviderStages(t *testing.T) { } requestID, snapshot := soleHotPathSnapshot(t, srv) assertHotPathChatChunks(t, chunks, hotPathChatChunkExpectation{ - ResponseID: "chatcmpl-mixed-local", Model: "virtual-model", Content: "local-A local-Breview-visible", Reasoning: "local-think review-think ", - Kinds: []string{"content", "reasoning", "content", "reasoning", "content", "tool", "terminal"}, - ToolName: "write_file", ToolArgs: hotPathChatReviewArguments(requestID), + ResponseID: "chatcmpl-mixed-local", Model: "virtual-model", Content: "local-A local-Breview-visible", Reasoning: "local-think review-reason", + Kinds: []string{"content", "reasoning", "content", "reasoning", "content", "tool", "tool", "tool", "terminal"}, FinishReason: "tool_calls", PromptTokens: 12, CompletionTokens: 7, }) for _, chunk := range chunks { @@ -215,13 +212,13 @@ func TestHotPathChatMixedProviderStages(t *testing.T) { t.Fatalf("logical request identity became the public response id: %+v", chunk) } } - for _, internalID := range []string{snapshot.ActiveStageID, "run-chat-gate-4", "run-chat-gate-5", "msg-mixed-review"} { + for _, internalID := range []string{snapshot.ActiveStageID, "run-chat-gate-5", "run-chat-gate-6", "msg-review-inspection"} { if strings.Contains(response.Body.String(), internalID) { t.Fatalf("internal or later-stage identity %q leaked: %s", internalID, response.Body.String()) } } - if got := service.requestCount(); got != 5 { - t.Fatalf("provider submissions=%d, want 5", got) + if got := service.requestCount(); got != 6 { + t.Fatalf("provider submissions=%d, want 6", got) } } @@ -623,7 +620,7 @@ func assertHotPathChatChunks(t *testing.T, chunks []map[string]any, want hotPath index := int(tool["index"].(float64)) if toolIndex == -1 { toolIndex = index - } else if toolIndex != index { + } else if want.ToolName != "" && toolIndex != index { t.Fatalf("tool index changed from %d to %d", toolIndex, index) } if id, _ := tool["id"].(string); id != "" { @@ -739,7 +736,20 @@ func (s *hotPathChatGateScriptedService) SubmitProviderPool(ctx context.Context, } frames := step.frames if frames == nil { - body := step.body(req.Run.Metadata["iop_logical_request_id"]) + providerBody, operation, err := materializeScriptedProviderBody(req, step.candidate) + if err != nil { + return nil, err + } + var requestID string + if strings.Contains(string(providerBody), "IOP caller-workspace selector instruction") { + requestID, err = scriptedSelectorRequestID(providerBody, operation) + } else { + requestID, err = scriptedRequestIDFromProviderBody(providerBody) + } + if err != nil { + return nil, err + } + body := step.body(requestID) frames = hotPathTunnelFrames(body, contentType, dispatch.RunID, 1_777_001_100_000_000_000+int64(index)) } return &edgeservice.ProviderPoolDispatchResult{ diff --git a/apps/edge/internal/openai/hot_path_cleanup.go b/apps/edge/internal/openai/hot_path_cleanup.go index 6044fd78..87dfd48d 100644 --- a/apps/edge/internal/openai/hot_path_cleanup.go +++ b/apps/edge/internal/openai/hot_path_cleanup.go @@ -98,7 +98,7 @@ func (s *hotPathLightStore) beginCleanupWithOuter( if record == nil || record.ownerEdgeID != ownerEdgeID || !record.running || record.pending != nil { return normalizedStageOutput{}, fmt.Errorf("review completion cannot enter cleanup") } - if record.phase != hotPathPhaseReviewResolution && record.phase != hotPathPhaseReviewRepair { + if record.phase != hotPathPhaseReviewActive && record.phase != hotPathPhaseReviewRepair { return normalizedStageOutput{}, fmt.Errorf("review completion is not resolution or repair") } return s.beginCleanupLocked(ctx, record, record.reviewStageID, intent, outer, coordinator) @@ -147,7 +147,7 @@ func (r *hotPathLightRecord) primaryErrorCleanupSource() (string, error) { return "", fmt.Errorf("primary-error cleanup local source is mismatched") } return r.localStageID, nil - case hotPathPhaseReviewActive, hotPathPhaseReviewAwaitRead, hotPathPhaseReviewResolution, hotPathPhaseReviewRepair: + case hotPathPhaseReviewActive, hotPathPhaseReviewRepair: if !r.artifactReady || !validLogicalRequestID(r.localStageID) || !validLogicalRequestID(r.reviewStageID) || r.localCommit.StageID != r.localStageID || strings.TrimSpace(r.localCommit.ResponseID) == "" { return "", fmt.Errorf("primary-error cleanup review source is mismatched") diff --git a/apps/edge/internal/openai/hot_path_cleanup_test.go b/apps/edge/internal/openai/hot_path_cleanup_test.go index 69c51a88..30057e54 100644 --- a/apps/edge/internal/openai/hot_path_cleanup_test.go +++ b/apps/edge/internal/openai/hot_path_cleanup_test.go @@ -186,27 +186,27 @@ func TestHotPathCleanupPrimaryErrorStageMatrix(t *testing.T) { }, { name: "review-dispatch", wantStatus: http.StatusBadGateway, - wantMessage: "review dispatch sentinel", wantProviderCalls: 5, + wantMessage: "review dispatch sentinel", wantProviderCalls: 6, prepare: func(fixture *scriptedLightFixture) { fixture.server.service = &primaryErrorPoolService{ - scriptedLightPoolService: fixture.service, failAt: 4, failure: errors.New("review dispatch sentinel"), + scriptedLightPoolService: fixture.service, failAt: 5, failure: errors.New("review dispatch sentinel"), } }, }, { name: "review-classification", wantStatus: http.StatusBadRequest, - wantMessage: "review stage completed before writing the issued review artifact", wantProviderCalls: 5, + wantMessage: "review completion requires both artifact reads and a successful ordinary result inspection", wantProviderCalls: 6, prepare: func(fixture *scriptedLightFixture) { - fixture.service.responses[4] = func(string) string { + fixture.service.responses[5] = func(string) string { return scriptedLightCompletion(endpoint, "review completed without its required write") } }, }, { name: "review-tool-frontier", wantStatus: http.StatusBadRequest, - wantMessage: "stage tool \"cleanup_unknown_tool\" is not in the immutable caller tool set", wantProviderCalls: 5, + wantMessage: "stage tool \"cleanup_unknown_tool\" is not in the immutable caller tool set", wantProviderCalls: 6, prepare: func(fixture *scriptedLightFixture) { - fixture.service.responses[4] = func(string) string { return primaryErrorUnknownToolOutput(endpoint) } + fixture.service.responses[5] = func(string) string { return primaryErrorUnknownToolOutput(endpoint) } }, }, } { @@ -347,6 +347,8 @@ func preparePrimaryErrorStage(t *testing.T, fixture *scriptedLightFixture, revie if review { localRead := fixture.request() fixture.consumeToolResponse(localRead, []string{`{"written":true}`}) + workerHandoff := fixture.request() + fixture.consumeToolResponse(workerHandoff, []string{`{"ok":true}`, `{"written":true}`}) } } @@ -432,10 +434,10 @@ func TestHotPathCleanupCancellationStopsWork(t *testing.T) { fixture.consumeToolResponse(pair, []string{`{"written":true}`, `{"written":true}`}) localRead := fixture.request() fixture.consumeToolResponse(localRead, []string{`{"written":true}`}) - reviewWrite := fixture.request() - fixture.consumeToolResponse(reviewWrite, []string{`{"written":true}`}) - reviewRead := fixture.request() - fixture.consumeToolResponse(reviewRead, []string{`{"written":true}`}) + workerHandoff := fixture.request() + fixture.consumeToolResponse(workerHandoff, []string{`{"ok":true}`, `{"written":true}`}) + reviewInspection := fixture.request() + fixture.consumeToolResponse(reviewInspection, []string{`{"written":true}`, `{"written":true}`, `{"ok":true}`}) raw := scriptedArtifactRequestBody(t, endpoint, fixture.tools, fixture.history) dispatch, err := fixture.server.resolveRouteDispatchForPrincipal(context.Background(), "virtual-model") diff --git a/apps/edge/internal/openai/hot_path_direct_test.go b/apps/edge/internal/openai/hot_path_direct_test.go index c0851009..920f7971 100644 --- a/apps/edge/internal/openai/hot_path_direct_test.go +++ b/apps/edge/internal/openai/hot_path_direct_test.go @@ -12,9 +12,114 @@ import ( edgeservice "iop/apps/edge/internal/service" "iop/packages/go/config" + "iop/packages/go/singlerequesttemplate" iop "iop/proto/gen/iop" ) +func TestHotPathSelectorInstruction(t *testing.T) { + prepareInstruction, err := buildHotPathSelectorProviderInstruction("req_wire_123", selectorInstructionPrepareOnly) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(prepareInstruction, "Operation: prepare-only") || + !strings.Contains(prepareInstruction, "JOB directory: .iop/job/req_wire_123") || + strings.Contains(prepareInstruction, "PLAN path:") || strings.Contains(prepareInstruction, "REVIEW path:") { + t.Fatalf("prepare-only selector instruction is not operation-bounded: %q", prepareInstruction) + } + pairInstruction, err := buildHotPathSelectorProviderInstruction("req_wire_123", selectorInstructionPairWrite) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(pairInstruction, "Operation: pair-write") || + !strings.Contains(pairInstruction, "PLAN path: .iop/job/req_wire_123/plan.md") || + !strings.Contains(pairInstruction, "REVIEW path: .iop/job/req_wire_123/review.md") { + t.Fatalf("pair-write selector instruction is incomplete: %q", pairInstruction) + } + + t.Run("chat_last_leading_system", func(t *testing.T) { + original := []byte(`{"model":"selector","messages":[{"role":"system","content":"caller-system"},{"role":"user","content":"task"}],"caller_extension":{"kept":true}}`) + body, err := injectHotPathChatSelectorInstruction(original, pairInstruction) + if err != nil { + t.Fatal(err) + } + var payload struct { + Messages []struct { + Role string `json:"role"` + Content string `json:"content"` + } `json:"messages"` + CallerExtension map[string]bool `json:"caller_extension"` + } + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatal(err) + } + if len(payload.Messages) != 3 || payload.Messages[0].Content != "caller-system" || payload.Messages[1].Content != pairInstruction || payload.Messages[2].Role != "user" { + t.Fatalf("unexpected Chat selector authority ordering: %+v", payload.Messages) + } + if !payload.CallerExtension["kept"] || strings.Contains(string(original), "req_wire_123") { + t.Fatalf("caller body was not preserved independently: original=%s provider=%s", original, body) + } + }) + + t.Run("anthropic_appended_system_block", func(t *testing.T) { + original := []byte(`{"model":"selector","system":"caller-system","messages":[{"role":"user","content":"task"}],"caller_extension":{"kept":true}}`) + body, err := injectHotPathAnthropicSelectorInstruction(original, pairInstruction) + if err != nil { + t.Fatal(err) + } + var payload struct { + System []struct { + Type string `json:"type"` + Text string `json:"text"` + } `json:"system"` + CallerExtension map[string]bool `json:"caller_extension"` + } + if err := json.Unmarshal(body, &payload); err != nil { + t.Fatal(err) + } + if len(payload.System) != 2 || payload.System[0].Text != "caller-system" || payload.System[1].Type != "text" || payload.System[1].Text != pairInstruction { + t.Fatalf("unexpected Messages selector authority ordering: %+v", payload.System) + } + if !payload.CallerExtension["kept"] || strings.Contains(string(original), "req_wire_123") { + t.Fatalf("caller body was not preserved independently: original=%s provider=%s", original, body) + } + }) + + t.Run("frontier_state_tracks_binding_and_phase", func(t *testing.T) { + prepareFixture := newArtifactPairFixture(t, "openai", false) + if got := prepareFixture.server.artifactFrontiers.selectorInstructionState(prepareFixture.requestID, prepareFixture.ownerEdgeID); got != selectorInstructionPrepareOnly { + t.Fatalf("initial non-parent-creating state = %q", got) + } + prepareIDs := prepareFixture.issuePrepare() + if _, _, _, err := prepareFixture.continueWithResult([]artifactTestResult{{id: prepareIDs[0], body: `{"written":true}`}}, nil); err != nil { + t.Fatal(err) + } + if got := prepareFixture.server.artifactFrontiers.selectorInstructionState(prepareFixture.requestID, prepareFixture.ownerEdgeID); got != selectorInstructionPairWrite { + t.Fatalf("resumed state = %q", got) + } + + pairFixture := newArtifactPairFixture(t, "anthropic", true) + if got := pairFixture.server.artifactFrontiers.selectorInstructionState(pairFixture.requestID, pairFixture.ownerEdgeID); got != selectorInstructionPairWrite { + t.Fatalf("initial parent-creating state = %q", got) + } + }) + + t.Run("marked_single_request_omitted", func(t *testing.T) { + original := []byte(`{"model":"stage-model","messages":[{"role":"user","content":"private stage"}]}`) + tunnel := edgeservice.SubmitProviderTunnelRequest{ + Operation: string(config.OperationChatCompletions), + BuildBody: func(string) ([]byte, error) { return append([]byte(nil), original...), nil }, + } + prepared, err := prepareHotPathSelectorProviderInstruction(tunnel, "") + if err != nil { + t.Fatal(err) + } + body, err := prepared.BuildBody("stage-model") + if err != nil || string(body) != string(original) || strings.Contains(string(body), "IOP caller-workspace selector instruction") { + t.Fatalf("marked single-request provider body changed: err=%v body=%s", err, body) + } + }) +} + func TestHotPathDirect(t *testing.T) { srv := NewServer(config.EdgeOpenAIConf{}, nil, nil) srv.SetEdgeID("edge-direct-test") @@ -132,16 +237,25 @@ func TestArtifactPairHandlerDisposition(t *testing.T) { t.Run(endpoint+" prepare resumes selector and pair reaches local handoff", func(t *testing.T) { candidate := anthropicTestCandidate(t, map[string]string{"openai": "openai", "anthropic": "anthropic"}[endpoint]) service := &scriptedArtifactPoolService{endpoint: endpoint, candidate: candidate} - service.response = func(requestID string, call int) string { - switch call { - case 1: + service.selectorResponse = func(requestID string, state selectorInstructionState, call int) string { + switch state { + case selectorInstructionPrepareOnly: + if call != 1 { + t.Fatalf("prepare-only instruction arrived on call %d", call) + } return scriptedArtifactPrepare(endpoint, requestID) - case 2: + case selectorInstructionPairWrite: + if call != 2 { + t.Fatalf("pair-write instruction arrived on call %d", call) + } return scriptedArtifactPair(endpoint, requestID) - case 3: + case selectorInstructionNone: + if call != 3 { + t.Fatalf("instruction-free local handoff arrived on call %d", call) + } return scriptedArtifactLocalRead(endpoint, requestID) default: - t.Fatalf("unexpected selector provider submission %d", call) + t.Fatalf("unexpected selector instruction state %q", state) return "" } } @@ -180,14 +294,33 @@ func TestArtifactPairHandlerDisposition(t *testing.T) { } }) - t.Run(endpoint+" pair-ready rejects direct selector output", func(t *testing.T) { + t.Run(endpoint+" rejects pair-write output for prepare-only instruction", func(t *testing.T) { candidate := anthropicTestCandidate(t, map[string]string{"openai": "openai", "anthropic": "anthropic"}[endpoint]) service := &scriptedArtifactPoolService{endpoint: endpoint, candidate: candidate} - service.response = func(requestID string, call int) string { - if call == 1 { - return scriptedArtifactPrepare(endpoint, requestID) + service.selectorResponse = func(requestID string, state selectorInstructionState, call int) string { + if state != selectorInstructionPrepareOnly || call != 1 { + t.Fatalf("unexpected swapped-operation setup: state=%q call=%d", state, call) + } + return scriptedArtifactPair(endpoint, requestID) + } + srv := newScriptedArtifactHandlerServer(t, service) + response := serveScriptedArtifactRequest(t, srv, endpoint, scriptedArtifactRequestBody(t, endpoint, scriptedArtifactTools(endpoint), []any{map[string]any{"role": "user", "content": "write a plan"}})) + if response.Code != http.StatusBadRequest || !strings.Contains(response.Body.String(), "prepare turn must contain exactly one call") { + t.Fatalf("prepare-only accepted pair-write output: status=%d body=%s", response.Code, response.Body.String()) + } + }) + + t.Run(endpoint+" rejects prepare output for pair-write instruction", func(t *testing.T) { + candidate := anthropicTestCandidate(t, map[string]string{"openai": "openai", "anthropic": "anthropic"}[endpoint]) + service := &scriptedArtifactPoolService{endpoint: endpoint, candidate: candidate} + service.selectorResponse = func(requestID string, state selectorInstructionState, call int) string { + switch state { + case selectorInstructionPrepareOnly, selectorInstructionPairWrite: + return scriptedArtifactPrepare(endpoint, requestID) + default: + t.Fatalf("unexpected instruction-free selector call %d", call) + return "" } - return scriptedArtifactDirect(endpoint) } srv := newScriptedArtifactHandlerServer(t, service) tools := scriptedArtifactTools(endpoint) @@ -200,8 +333,8 @@ func TestArtifactPairHandlerDisposition(t *testing.T) { history = append(history, assistant) history = scriptedArtifactAppendResults(endpoint, history, prepareIDs, []string{`{"written":true}`}) second := serveScriptedArtifactRequest(t, srv, endpoint, scriptedArtifactRequestBody(t, endpoint, tools, history)) - if second.Code != http.StatusBadRequest || service.calls != 2 || !strings.Contains(second.Body.String(), "requires the exact Plan/Review pair") { - t.Fatalf("pair-ready direct downgrade: status=%d calls=%d body=%s", second.Code, service.calls, second.Body.String()) + if second.Code != http.StatusBadRequest || service.calls != 2 || !strings.Contains(second.Body.String(), "pair turn must contain exactly two calls") { + t.Fatalf("pair-write accepted prepare output: status=%d calls=%d body=%s", second.Code, service.calls, second.Body.String()) } }) } @@ -209,16 +342,37 @@ func TestArtifactPairHandlerDisposition(t *testing.T) { type scriptedArtifactPoolService struct { providerFakeRunService - endpoint string - candidate edgeservice.ProviderPoolCandidate - calls int - response func(requestID string, call int) string + endpoint string + candidate edgeservice.ProviderPoolCandidate + calls int + response func(requestID string, call int) string + selectorResponse func(requestID string, state selectorInstructionState, call int) string } func (s *scriptedArtifactPoolService) SubmitProviderPool(_ context.Context, req edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { s.calls++ - requestID := req.Run.Metadata["iop_logical_request_id"] - body := s.response(requestID, s.calls) + providerBody, operation, err := materializeScriptedProviderBody(req, s.candidate) + if err != nil { + return nil, err + } + var requestID string + instructionState := selectorInstructionNone + if strings.Contains(string(providerBody), "IOP caller-workspace selector instruction") { + requestID, instructionState, err = scriptedSelectorDirective(providerBody, operation) + } else { + requestID, err = scriptedRequestIDFromProviderBody(providerBody) + } + if err != nil { + return nil, err + } + var body string + if s.selectorResponse != nil { + body = s.selectorResponse(requestID, instructionState, s.calls) + } else if s.response != nil { + body = s.response(requestID, s.calls) + } else { + return nil, fmt.Errorf("scripted provider response is unavailable") + } dispatch := edgeservice.RunDispatch{ RunID: fmt.Sprintf("run-scripted-%d", s.calls), NodeID: "node-scripted", ModelGroupKey: req.Run.ModelGroupKey, ProviderID: s.candidate.ProviderID, ExecutionPath: string(edgeservice.ProviderPoolPathTunnel), @@ -235,6 +389,155 @@ func (s *scriptedArtifactPoolService) SubmitProviderPool(_ context.Context, req }, nil } +func materializeScriptedProviderBody(req edgeservice.ProviderPoolDispatchRequest, candidate edgeservice.ProviderPoolCandidate) ([]byte, string, error) { + prepared := req.Tunnel + var err error + if req.PrepareProtocolTunnel != nil { + prepared, err = req.PrepareProtocolTunnel(prepared, candidate) + if err != nil { + return nil, "", err + } + } + if prepared.BuildBody != nil { + target := candidate.ActualModel + if strings.TrimSpace(target) == "" { + target = "served-selector" + } + body, err := prepared.BuildBody(target) + return body, prepared.Operation, err + } + if len(prepared.Body) == 0 { + return nil, "", fmt.Errorf("scripted provider body is unavailable") + } + return append([]byte(nil), prepared.Body...), prepared.Operation, nil +} + +func scriptedSelectorDirective(providerBody []byte, operation string) (string, selectorInstructionState, error) { + instruction, err := scriptedSelectorInstruction(providerBody, operation) + if err != nil { + return "", selectorInstructionNone, err + } + state := selectorInstructionNone + pathPrefix := "" + switch { + case strings.Contains(instruction, "Operation: prepare-only"): + state = selectorInstructionPrepareOnly + pathPrefix = "JOB directory: .iop/job/" + case strings.Contains(instruction, "Operation: pair-write"): + state = selectorInstructionPairWrite + pathPrefix = "PLAN path: .iop/job/" + default: + return "", selectorInstructionNone, fmt.Errorf("provider selector instruction is missing a closed operation") + } + start := strings.Index(instruction, pathPrefix) + if start < 0 { + return "", selectorInstructionNone, fmt.Errorf("provider selector instruction is missing operation path") + } + start += len(pathPrefix) + remainder := instruction[start:] + end := strings.IndexAny(remainder, "/\n\r\t ") + if end < 0 { + end = len(remainder) + } + if end == 0 { + return "", selectorInstructionNone, fmt.Errorf("provider selector instruction has an invalid operation path") + } + requestID := remainder[:end] + paths := newReservedPaths(requestID) + if state == selectorInstructionPrepareOnly { + required := []string{ + "Return exactly one admitted prepare tool call", + "JOB directory: " + paths.JobDir, + "Do not write PLAN or REVIEW in this turn.", + } + for _, fragment := range required { + if !strings.Contains(instruction, fragment) { + return "", selectorInstructionNone, fmt.Errorf("provider prepare-only instruction is missing fragment %q", fragment) + } + } + if strings.Contains(instruction, "PLAN path:") || strings.Contains(instruction, "REVIEW path:") { + return "", selectorInstructionNone, fmt.Errorf("provider prepare-only instruction contains pair paths") + } + return requestID, state, nil + } + required := []string{ + "Return exactly two admitted write tool calls", + "PLAN path: " + paths.PlanPath, + "REVIEW path: " + paths.ReviewPath, + "The default PLAN grammar accepts 2-6 consecutive steps and 1-3 verification bullets.", + "# Plan\n\n## Goal\n\n\n## Steps\n- [P1] \n- [P2] ", + "The pending REVIEW content must be exactly this deterministic seed:", + "# Review\n\n## Worker Item Status\n- P1: pending\n- P2: pending", + "## Worker Changes\nPending worker execution.", + "## Worker Verification\nPending worker verification.", + "## Deviations\nNone recorded.", + } + for _, fragment := range required { + if !strings.Contains(instruction, fragment) { + return "", selectorInstructionNone, fmt.Errorf("provider pair-write instruction is missing grammar fragment %q", fragment) + } + } + return requestID, state, nil +} + +func scriptedSelectorRequestID(providerBody []byte, operation string) (string, error) { + requestID, _, err := scriptedSelectorDirective(providerBody, operation) + return requestID, err +} + +func scriptedSelectorInstruction(providerBody []byte, operation string) (string, error) { + switch config.ProtocolOperation(operation) { + case config.OperationChatCompletions: + var payload struct { + Messages []struct { + Role string `json:"role"` + Content any `json:"content"` + } `json:"messages"` + } + if err := json.Unmarshal(providerBody, &payload); err != nil { + return "", err + } + for _, message := range payload.Messages { + if message.Role != "system" { + break + } + text := extractMessageContentString(message.Content) + if strings.Contains(text, "IOP caller-workspace selector instruction") { + return text, nil + } + } + case config.OperationMessages: + var payload struct { + System any `json:"system"` + } + if err := json.Unmarshal(providerBody, &payload); err != nil { + return "", err + } + text := extractMessageContentString(payload.System) + if strings.Contains(text, "IOP caller-workspace selector instruction") { + return text, nil + } + default: + return "", fmt.Errorf("unexpected scripted selector operation %q", operation) + } + return "", fmt.Errorf("actual provider body is missing the selector instruction") +} + +func scriptedRequestIDFromProviderBody(providerBody []byte) (string, error) { + const prefix = ".iop/job/" + text := string(providerBody) + start := strings.Index(text, prefix) + if start < 0 { + return "", fmt.Errorf("actual provider body is missing a request-local artifact path") + } + start += len(prefix) + end := strings.IndexByte(text[start:], '/') + if end <= 0 { + return "", fmt.Errorf("actual provider body has an invalid request-local artifact path") + } + return text[start : start+end], nil +} + func newScriptedArtifactHandlerServer(t *testing.T, service *scriptedArtifactPoolService) *Server { t.Helper() preset := hotPathSelectorPreset([]string{config.ModeDirect, config.ModeLight}) @@ -333,11 +636,16 @@ func scriptedArtifactPrepare(endpoint, requestID string) string { func scriptedArtifactPair(endpoint, requestID string) string { paths := newReservedPaths(requestID) + plan, _ := singlerequesttemplate.RenderPlan(singlerequesttemplate.DefaultPlanTemplate, singlerequesttemplate.PlanFields{ + Goal: "Complete the caller workspace task", Steps: []string{"Inspect the requested result", "Implement and verify the result"}, + Verification: []string{"Confirm the caller workspace result"}, + }, singlerequesttemplate.MaxTemplateBytes) + review, _ := pendingReviewSeed([]string{"P1", "P2"}) if endpoint == "anthropic" { - return fmt.Sprintf(`{"id":"msg-scripted-pair","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-plan","name":"write_file","input":{"path":%q,"content":"plan"}},{"type":"tool_use","id":"provider-review","name":"write_file","input":{"path":%q,"content":"review"}}],"stop_reason":"tool_use"}`, paths.PlanPath, paths.ReviewPath) + return fmt.Sprintf(`{"id":"msg-scripted-pair","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-plan","name":"write_file","input":{"path":%q,"content":%q}},{"type":"tool_use","id":"provider-review","name":"write_file","input":{"path":%q,"content":%q}}],"stop_reason":"tool_use"}`, paths.PlanPath, string(plan), paths.ReviewPath, string(review)) } - planArgs, _ := json.Marshal(map[string]string{"path": paths.PlanPath, "content": "plan"}) - reviewArgs, _ := json.Marshal(map[string]string{"path": paths.ReviewPath, "content": "review"}) + planArgs, _ := json.Marshal(map[string]string{"path": paths.PlanPath, "content": string(plan)}) + reviewArgs, _ := json.Marshal(map[string]string{"path": paths.ReviewPath, "content": string(review)}) return fmt.Sprintf(`{"id":"chatcmpl-scripted-pair","created":2,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":"provider-plan","type":"function","function":{"name":"write_file","arguments":%q}},{"id":"provider-review","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(planArgs), string(reviewArgs)) } diff --git a/apps/edge/internal/openai/hot_path_light.go b/apps/edge/internal/openai/hot_path_light.go index 75535fe1..ae5aea50 100644 --- a/apps/edge/internal/openai/hot_path_light.go +++ b/apps/edge/internal/openai/hot_path_light.go @@ -12,6 +12,7 @@ import ( edgeservice "iop/apps/edge/internal/service" "iop/packages/go/config" + "iop/packages/go/singlerequesttemplate" ) const defaultHotPathLightCapacity = 1024 @@ -48,22 +49,19 @@ func applyHotPathOutputTokenCap(metadata map[string]string, candidates ...*int) type hotPathLightPhase string const ( - hotPathPhaseAwaitArtifacts hotPathLightPhase = "await_artifacts" - hotPathPhaseLocalActive hotPathLightPhase = "local_active" - hotPathPhaseReviewActive hotPathLightPhase = "review_active" - hotPathPhaseReviewAwaitRead hotPathLightPhase = "review_write_wait" - hotPathPhaseReviewResolution hotPathLightPhase = "review_resolution_active" - hotPathPhaseReviewRepair hotPathLightPhase = "review_repair_active" - hotPathPhaseCleanupPending hotPathLightPhase = "cleanup_pending" + hotPathPhaseAwaitArtifacts hotPathLightPhase = "await_artifacts" + hotPathPhaseLocalActive hotPathLightPhase = "local_active" + hotPathPhaseReviewActive hotPathLightPhase = "review_active" + hotPathPhaseReviewRepair hotPathLightPhase = "review_repair_active" + hotPathPhaseCleanupPending hotPathLightPhase = "cleanup_pending" ) type hotPathPendingKind string const ( hotPathPendingLocalTools hotPathPendingKind = "local_tools" + hotPathPendingLocalHandoff hotPathPendingKind = "local_handoff" hotPathPendingReviewInspection hotPathPendingKind = "review_inspection" - hotPathPendingReviewWrite hotPathPendingKind = "review_write" - hotPathPendingReviewRead hotPathPendingKind = "review_read" hotPathPendingReviewRepair hotPathPendingKind = "review_repair" hotPathPendingCleanup hotPathPendingKind = "cleanup" ) @@ -107,6 +105,12 @@ type hotPathLightRecord struct { phase hotPathLightPhase artifactReady bool + planItemIDs []string + localPlanRead bool + workerReviewWritten bool + reviewerPlanRead bool + reviewerReviewRead bool + reviewerInspected bool running bool pendingKind hotPathPendingKind pending map[string]hotPathPendingCall @@ -252,7 +256,7 @@ func (s *hotPathLightStore) has(requestID, ownerEdgeID string) bool { return record != nil && record.ownerEdgeID == ownerEdgeID } -func (s *hotPathLightStore) updateArtifactLineage(requestID, ownerEdgeID string, lineage logicalRequestLineage, localEligible bool) error { +func (s *hotPathLightStore) updateArtifactLineage(requestID, ownerEdgeID string, lineage logicalRequestLineage, localEligible bool, planItemIDs []string) error { if s == nil { return fmt.Errorf("light flow is unavailable") } @@ -264,7 +268,11 @@ func (s *hotPathLightStore) updateArtifactLineage(requestID, ownerEdgeID string, } record.lineage = lineage if localEligible { + if len(planItemIDs) == 0 { + return fmt.Errorf("light flow plan item inventory is unavailable") + } record.artifactReady = true + record.planItemIDs = append([]string(nil), planItemIDs...) } return nil } @@ -355,7 +363,7 @@ func (r *hotPathLightRecord) dispatchValues() (config.ExecutionRouteStage, route return config.ExecutionRouteStage{}, routeDispatch{}, "", hotPathStageInput{}, nil, fmt.Errorf("local stage binding is unavailable") } return stage, binding, r.localStageID, buildLocalStageInput(r.immutableTask, paths, r.selectorCommit), r.localTranscript, nil - case hotPathPhaseReviewActive, hotPathPhaseReviewAwaitRead, hotPathPhaseReviewResolution, hotPathPhaseReviewRepair: + case hotPathPhaseReviewActive, hotPathPhaseReviewRepair: stage := route.Stages[1].Clone() binding, ok := r.dispatch.PresetResolvedBindings[stage.Model] if !ok { @@ -456,7 +464,7 @@ func (s *hotPathLightStore) issueTools( return normalizedStageOutput{}, err } stageID := record.localStageID - if kind != hotPathPendingLocalTools { + if kind != hotPathPendingLocalTools && kind != hotPathPendingLocalHandoff { stageID = record.reviewStageID } if outer != nil { @@ -574,9 +582,6 @@ func hotPathToolAllowed(tools []any, name string) bool { func hotPathWorkspaceCall(phase hotPathLightPhase, kind hotPathPendingKind, paths reservedPaths, call normalizedToolCall) (workspaceOperationKind, string, bool, error) { reserved := reservedPathsFromToolCall(call) if len(reserved) == 0 { - if kind == hotPathPendingReviewWrite || kind == hotPathPendingReviewRead { - return "", "", false, fmt.Errorf("review control turn must use the exact review path") - } return "", "", false, nil } if len(reserved) != 1 { @@ -584,21 +589,21 @@ func hotPathWorkspaceCall(phase hotPathLightPhase, kind hotPathPendingKind, path } observed := cleanRelativePath(reserved[0]) switch kind { - case hotPathPendingLocalTools, hotPathPendingReviewInspection: + case hotPathPendingLocalTools: + if observed != cleanRelativePath(paths.PlanPath) { + return "", "", false, fmt.Errorf("worker may only read the issued plan artifact") + } + return opKindRead, observed, true, nil + case hotPathPendingLocalHandoff: + if observed != cleanRelativePath(paths.ReviewPath) { + return "", "", false, fmt.Errorf("worker handoff targets a non-review path") + } + return opKindWrite, paths.ReviewPath, true, nil + case hotPathPendingReviewInspection: if observed != cleanRelativePath(paths.PlanPath) && observed != cleanRelativePath(paths.ReviewPath) { return "", "", false, fmt.Errorf("stage read targets an unissued reserved path") } return opKindRead, observed, true, nil - case hotPathPendingReviewWrite: - if observed != cleanRelativePath(paths.ReviewPath) { - return "", "", false, fmt.Errorf("review write targets a non-review path") - } - return opKindWrite, paths.ReviewPath, true, nil - case hotPathPendingReviewRead: - if observed != cleanRelativePath(paths.ReviewPath) { - return "", "", false, fmt.Errorf("review resolution read targets a non-review path") - } - return opKindRead, paths.ReviewPath, true, nil case hotPathPendingReviewRepair: return "", "", false, fmt.Errorf("repair cannot start a second reserved review cycle") default: @@ -681,11 +686,41 @@ func (s *hotPathLightStore) consume(ownerEdgeID, principalRef, protocol string, stageResults = append(stageResults, hotPathStageToolResult{ProviderCallID: providerID, Body: string(result.body), IsError: result.status == "error"}) } exchange := hotPathStageExchange{Output: cloneNormalizedStageOutput(record.pendingOutput), Results: stageResults} - if record.pendingKind == hotPathPendingLocalTools { + pendingKind := record.pendingKind + if pendingKind == hotPathPendingLocalTools || pendingKind == hotPathPendingLocalHandoff { record.localTranscript = append(record.localTranscript, exchange) } else { record.reviewTranscript = append(record.reviewTranscript, exchange) } + for publicID, pending := range record.pending { + result := byPublic[publicID] + if result.status == "error" { + continue + } + if pending.payload == nil { + if pendingKind == hotPathPendingReviewInspection || pendingKind == hotPathPendingReviewRepair { + record.reviewerInspected = true + } + continue + } + path := cleanRelativePath(pending.payload.safePath) + switch pendingKind { + case hotPathPendingLocalTools: + if path == cleanRelativePath(newReservedPaths(record.requestID).PlanPath) { + record.localPlanRead = true + } + case hotPathPendingLocalHandoff: + record.workerReviewWritten = true + case hotPathPendingReviewInspection: + paths := newReservedPaths(record.requestID) + if path == cleanRelativePath(paths.PlanPath) { + record.reviewerPlanRead = true + } + if path == cleanRelativePath(paths.ReviewPath) { + record.reviewerReviewRead = true + } + } + } for id := range record.pending { record.consumedIDs[id] = struct{}{} } @@ -695,7 +730,7 @@ func (s *hotPathLightStore) consume(ownerEdgeID, principalRef, protocol string, record.pendingHash = "" record.pendingOutput = normalizedStageOutput{} previousPhase := record.phase - record.phase = phaseAfterHotPathResult(record.pendingKind) + record.phase = phaseAfterHotPathResult(pendingKind) record.pendingKind = "" stageID := record.localStageID if record.phase != hotPathPhaseLocalActive { @@ -724,14 +759,10 @@ func (s *hotPathLightStore) cleanupStage(requestID, ownerEdgeID string) string { func phaseAfterHotPathResult(kind hotPathPendingKind) hotPathLightPhase { switch kind { - case hotPathPendingLocalTools: + case hotPathPendingLocalTools, hotPathPendingLocalHandoff: return hotPathPhaseLocalActive case hotPathPendingReviewInspection: return hotPathPhaseReviewActive - case hotPathPendingReviewWrite: - return hotPathPhaseReviewAwaitRead - case hotPathPendingReviewRead: - return hotPathPhaseReviewResolution case hotPathPendingReviewRepair: return hotPathPhaseReviewRepair default: @@ -802,6 +833,9 @@ func (s *hotPathLightStore) commitLocal(requestID, ownerEdgeID string, output no if record == nil || record.ownerEdgeID != ownerEdgeID || record.phase != hotPathPhaseLocalActive || !record.running || len(output.ToolCalls) != 0 { return hotPathLightDisposition{}, fmt.Errorf("local completion cannot transition to review") } + if !record.localPlanRead || !record.workerReviewWritten { + return hotPathLightDisposition{}, fmt.Errorf("worker completion requires a successful plan read and completed review handoff") + } reviewStageID, err := coordinator.newStageID() if err != nil { return hotPathLightDisposition{}, err @@ -819,6 +853,77 @@ func (s *hotPathLightStore) commitLocal(requestID, ownerEdgeID string, output no return hotPathLightDisposition{RequestID: requestID, StageID: reviewStageID, Phase: record.phase}, nil } +func (s *hotPathLightStore) classifyLocalOutput(requestID, ownerEdgeID string, output normalizedStageOutput) (hotPathPendingKind, error) { + if s == nil { + return "", fmt.Errorf("light flow is unavailable") + } + s.mu.Lock() + defer s.mu.Unlock() + record := s.records[requestID] + if record == nil || record.ownerEdgeID != ownerEdgeID || record.phase != hotPathPhaseLocalActive || !record.running { + return "", fmt.Errorf("local flow state is unavailable") + } + if len(output.ToolCalls) == 0 { + if !record.localPlanRead || !record.workerReviewWritten { + return "", fmt.Errorf("worker completion requires a successful plan read and completed review handoff") + } + return "", nil + } + paths := newReservedPaths(requestID) + reviewWrites := 0 + for _, call := range output.ToolCalls { + for _, observed := range reservedPathsFromToolCall(call) { + if cleanRelativePath(observed) == cleanRelativePath(paths.ReviewPath) { + reviewWrites++ + } + } + } + if reviewWrites == 0 { + return hotPathPendingLocalTools, nil + } + if reviewWrites != 1 || !record.localPlanRead { + return "", fmt.Errorf("worker review handoff must contain one exact review write after reading the plan") + } + var reviewCall normalizedToolCall + for _, call := range output.ToolCalls { + for _, observed := range reservedPathsFromToolCall(call) { + if cleanRelativePath(observed) == cleanRelativePath(paths.ReviewPath) { + reviewCall = call + } + } + } + content, err := artifactWriteContent(record.binding, reviewCall) + if err != nil { + return "", err + } + if err := singlerequesttemplate.ValidateReviewHandoff([]byte(content), record.planItemIDs); err != nil { + return "", fmt.Errorf("worker review handoff is malformed: %w", err) + } + return hotPathPendingLocalHandoff, nil +} + +type hotPathReviewEvidence struct { + planRead bool + reviewRead bool + inspected bool +} + +func (s *hotPathLightStore) reviewEvidence(requestID, ownerEdgeID string) (hotPathReviewEvidence, error) { + if s == nil { + return hotPathReviewEvidence{}, fmt.Errorf("light flow is unavailable") + } + s.mu.Lock() + defer s.mu.Unlock() + record := s.records[requestID] + if record == nil || record.ownerEdgeID != ownerEdgeID { + return hotPathReviewEvidence{}, fmt.Errorf("review flow state is unavailable") + } + return hotPathReviewEvidence{ + planRead: record.reviewerPlanRead, reviewRead: record.reviewerReviewRead, + inspected: record.reviewerInspected, + }, nil +} + func (s *Server) runHotPathLocalEligible(w http.ResponseWriter, r *http.Request, dispatch routeDispatch, protocol string, stream bool, metadata map[string]string) error { requestID := strings.TrimSpace(metadata["iop_logical_request_id"]) if requestID == "" { @@ -917,7 +1022,12 @@ func (s *Server) runHotPathLightStage(w http.ResponseWriter, r *http.Request, di switch snapshot.Phase { case hotPathPhaseLocalActive: if len(output.ToolCalls) > 0 { - mapped, err := s.lightFlows.issueTools(r.Context(), requestID, s.edgeIDValue(), output, visible, hotPathPendingLocalTools, outer, s.requestCoordinator) + kind, err := s.lightFlows.classifyLocalOutput(requestID, s.edgeIDValue(), output) + if err != nil { + return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, + hotPathLightEndpointError(protocol, http.StatusBadRequest, err.Error())) + } + mapped, err := s.lightFlows.issueTools(r.Context(), requestID, s.edgeIDValue(), output, visible, kind, outer, s.requestCoordinator) if err != nil { return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, hotPathLightEndpointError(protocol, http.StatusBadRequest, err.Error())) @@ -946,6 +1056,9 @@ func (s *Server) runHotPathLightStage(w http.ResponseWriter, r *http.Request, di s.observeHotPathLightTransition(r.Context(), hotPathStageKindReview, hotPathAttemptFirst, disposition.RequestID, disposition.StageID, dispatch.Preset.ID) } + // Only the reviewer owns caller-visible terminal output. Local-stage + // prose is not carried into the review completion projection. + visible = normalizedStageOutput{} continue default: final, done, err := s.advanceHotPathReview(r.Context(), requestID, snapshot.Phase, output, visible, outer, protocol) diff --git a/apps/edge/internal/openai/hot_path_light_test.go b/apps/edge/internal/openai/hot_path_light_test.go index 611e3543..de7b7278 100644 --- a/apps/edge/internal/openai/hot_path_light_test.go +++ b/apps/edge/internal/openai/hot_path_light_test.go @@ -12,6 +12,7 @@ import ( edgeservice "iop/apps/edge/internal/service" "iop/packages/go/config" + "iop/packages/go/singlerequesttemplate" ) func TestHotPathLightLocalTransition(t *testing.T) { @@ -32,6 +33,52 @@ func TestHotPathLightLocalTransition(t *testing.T) { } } +func TestHotPathLightRequiresCompletedWorkerHandoff(t *testing.T) { + for _, endpoint := range []string{"openai", "anthropic"} { + endpoint := endpoint + t.Run(endpoint+" missing handoff", func(t *testing.T) { + fixture := newScriptedLightFixture(t, endpoint, false) + fixture.service.responses[3] = func(string) string { + return scriptedLightCompletion(endpoint, "worker tried to finish early") + } + driveScriptedLightToLocalAfterPlanRead(t, fixture) + assertLightFailureAfterCleanup(t, fixture, "completed review handoff") + }) + + t.Run(endpoint+" malformed handoff", func(t *testing.T) { + fixture := newScriptedLightFixture(t, endpoint, false) + fixture.service.responses[3] = func(requestID string) string { + return scriptedMalformedWorkerHandoff(endpoint, requestID) + } + driveScriptedLightToLocalAfterPlanRead(t, fixture) + assertLightFailureAfterCleanup(t, fixture, "handoff is malformed") + }) + } +} + +func assertLightFailureAfterCleanup(t *testing.T, fixture *scriptedLightFixture, message string) { + t.Helper() + cleanup := fixture.request() + if cleanup.Code != http.StatusOK || !strings.Contains(cleanup.Body.String(), "delete_file") { + t.Fatalf("failure cleanup response: status=%d body=%s", cleanup.Code, cleanup.Body.String()) + } + fixture.consumeToolResponse(cleanup, []string{`{"written":true}`}) + final := fixture.request() + if final.Code != http.StatusBadRequest || !strings.Contains(final.Body.String(), message) { + t.Fatalf("failure terminal response: status=%d body=%s", final.Code, final.Body.String()) + } +} + +func driveScriptedLightToLocalAfterPlanRead(t *testing.T, fixture *scriptedLightFixture) { + t.Helper() + prepare := fixture.request() + fixture.consumeToolResponse(prepare, []string{`{"written":true}`}) + pair := fixture.request() + fixture.consumeToolResponse(pair, []string{`{"written":true}`, `{"written":true}`}) + localRead := fixture.request() + fixture.consumeToolResponse(localRead, []string{`{"written":true}`}) +} + func TestHotPathStageInputIsolation(t *testing.T) { paths := newReservedPaths("req_stage_isolation") selector := hotPathStageCorrelation{StageID: "stg_selector", ResponseID: "provider:selector.actual/1", RunID: "run-selector", ProviderID: "provider.actual", Terminal: "stop,done\"quoted\""} @@ -244,7 +291,19 @@ func (s *scriptedLightPoolService) SubmitProviderPool(ctx context.Context, req e endpoint := s.endpoint s.mu.Unlock() - requestID := req.Run.Metadata["iop_logical_request_id"] + providerBody, operation, err := materializeScriptedProviderBody(req, candidate) + if err != nil { + return nil, err + } + var requestID string + if strings.Contains(string(providerBody), "IOP caller-workspace selector instruction") { + requestID, err = scriptedSelectorRequestID(providerBody, operation) + } else { + requestID, err = scriptedRequestIDFromProviderBody(providerBody) + } + if err != nil { + return nil, err + } body := response(requestID) dispatch := edgeservice.RunDispatch{ RunID: fmt.Sprintf("run-light-%d", index+1), NodeID: "node-light", ModelGroupKey: req.Run.ModelGroupKey, @@ -285,9 +344,9 @@ func newScriptedLightFixture(t *testing.T, endpoint string, repair bool) *script func(requestID string) string { return scriptedArtifactPrepare(endpoint, requestID) }, func(requestID string) string { return scriptedArtifactPair(endpoint, requestID) }, func(requestID string) string { return scriptedArtifactLocalRead(endpoint, requestID) }, + func(requestID string) string { return scriptedLocalWorkAndHandoff(endpoint, requestID) }, func(string) string { return scriptedLightCompletion(endpoint, "local-complete-visible") }, - func(requestID string) string { return scriptedReviewWrite(endpoint, requestID) }, - func(requestID string) string { return scriptedReviewRead(endpoint, requestID) }, + func(requestID string) string { return scriptedReviewReadsAndInspection(endpoint, requestID) }, } if repair { service.responses = append(service.responses, @@ -355,10 +414,10 @@ func (f *scriptedLightFixture) runToCleanup() *httptest.ResponseRecorder { f.consumeToolResponse(pair, []string{`{"written":true}`, `{"written":true}`}) localRead := f.request() f.consumeToolResponse(localRead, []string{`{"written":true}`}) - reviewWrite := f.request() - f.consumeToolResponse(reviewWrite, []string{`{"written":true}`}) - reviewRead := f.request() - f.consumeToolResponse(reviewRead, []string{`{"written":true}`}) + workerHandoff := f.request() + f.consumeToolResponse(workerHandoff, []string{`{"ok":true}`, `{"written":true}`}) + reviewInspection := f.request() + f.consumeToolResponse(reviewInspection, []string{`{"written":true}`, `{"written":true}`, `{"ok":true}`}) resolution := f.request() if !f.repair { return resolution @@ -412,11 +471,11 @@ func (f *scriptedLightFixture) assertCleanupCommitted(wantCalls int) { if localStage == "" || requests[3].Run.Metadata["iop_stage_id"] != localStage { f.t.Fatalf("local stage was not resumed: %#v %#v", requests[2].Run.Metadata, requests[3].Run.Metadata) } - reviewStage := requests[4].Run.Metadata["iop_stage_id"] + reviewStage := requests[5].Run.Metadata["iop_stage_id"] if reviewStage == "" || reviewStage == localStage { f.t.Fatalf("review stage identity is not fixed and distinct: local=%q review=%q", localStage, reviewStage) } - for index := 4; index < len(requests); index++ { + for index := 5; index < len(requests); index++ { if requests[index].Run.ModelGroupKey != "review-model" || requests[index].Run.Metadata["iop_stage_id"] != reviewStage { f.t.Fatalf("review dispatch %d changed binding: group=%q metadata=%#v", index, requests[index].Run.ModelGroupKey, requests[index].Run.Metadata) } @@ -435,13 +494,15 @@ func (f *scriptedLightFixture) assertCleanupCommitted(wantCalls int) { // Regression: local stage must carry selector correlation and must NOT // carry local correlation in both normalized Run.Input and tunnel body. - assertLocalCorrelationRegression(f.t, requests[2], f.service.candidate, selectorStage, selectorResponse) - assertLocalCorrelationRegression(f.t, requests[3], f.service.candidate, selectorStage, selectorResponse) + for index := 2; index <= 4; index++ { + assertLocalCorrelationRegression(f.t, requests[index], f.service.candidate, selectorStage, selectorResponse) + } // Regression: review stage must carry both selector and local correlations // in both normalized Run.Input and tunnel body. - assertReviewCorrelationRegression(f.t, requests[4], f.service.candidate, selectorStage, selectorResponse, localStage, localResponse) - assertReviewCorrelationRegression(f.t, requests[5], f.service.candidate, selectorStage, selectorResponse, localStage, localResponse) + for index := 5; index < len(requests); index++ { + assertReviewCorrelationRegression(f.t, requests[index], f.service.candidate, selectorStage, selectorResponse, localStage, localResponse) + } // Regression: forbidden data must not appear in any provider-visible payload. for index, req := range requests { @@ -693,19 +754,95 @@ func scriptedLightCompletionWithUsage(endpoint, content, reasoning string, input func scriptedReviewWrite(endpoint, requestID string) string { path := newReservedPaths(requestID).ReviewPath + review := testCompletedReviewText() if endpoint == "anthropic" { - return fmt.Sprintf(`{"id":"msg-review-write","type":"message","role":"assistant","content":[{"type":"text","text":"review-write-visible"},{"type":"tool_use","id":"provider-review-write","name":"write_file","input":{"path":%q,"content":"review body"}}],"stop_reason":"tool_use"}`, path) + return fmt.Sprintf(`{"id":"msg-review-write","type":"message","role":"assistant","content":[{"type":"text","text":"worker-handoff-visible"},{"type":"tool_use","id":"provider-review-write","name":"write_file","input":{"path":%q,"content":%q}}],"stop_reason":"tool_use"}`, path, review) } - args, _ := json.Marshal(map[string]string{"path": path, "content": "review body"}) - return fmt.Sprintf(`{"id":"chatcmpl-review-write","created":5,"choices":[{"message":{"role":"assistant","content":"review-write-visible","tool_calls":[{"id":"provider-review-write","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(args)) + args, _ := json.Marshal(map[string]string{"path": path, "content": review}) + return fmt.Sprintf(`{"id":"chatcmpl-review-write","created":5,"choices":[{"message":{"role":"assistant","content":"worker-handoff-visible","tool_calls":[{"id":"provider-review-write","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(args)) +} + +func testCompletedReviewText() string { + review, _ := pendingCompletedReview() + return string(review) +} + +func pendingCompletedReview() ([]byte, error) { + return singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, singlerequesttemplate.ReviewFields{ + ItemStatus: "- P1: completed\n- P2: completed", Changes: "Implemented the requested caller workspace result.", + Verification: "Verified the requested result with caller tools.", Deviations: "None.", + }, singlerequesttemplate.MaxTemplateBytes) +} + +func scriptedLocalWorkTool(endpoint string) string { + return scriptedOrdinaryTool(endpoint, "provider-local-work", "implement requested result") +} + +func scriptedLocalWorkAndHandoff(endpoint, requestID string) string { + path := newReservedPaths(requestID).ReviewPath + review := testCompletedReviewText() + if endpoint == "anthropic" { + return fmt.Sprintf(`{"id":"msg-worker-handoff","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-local-work","name":"run_command","input":{"command":"implement requested result"}},{"type":"tool_use","id":"provider-review-write","name":"write_file","input":{"path":%q,"content":%q}}],"stop_reason":"tool_use"}`, path, review) + } + workArgs, _ := json.Marshal(map[string]string{"command": "implement requested result"}) + reviewArgs, _ := json.Marshal(map[string]string{"path": path, "content": review}) + return fmt.Sprintf(`{"id":"chatcmpl-worker-handoff","created":4,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":"provider-local-work","type":"function","function":{"name":"run_command","arguments":%q}},{"id":"provider-review-write","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(workArgs), string(reviewArgs)) +} + +func scriptedMalformedWorkerHandoff(endpoint, requestID string) string { + path := newReservedPaths(requestID).ReviewPath + if endpoint == "anthropic" { + return fmt.Sprintf(`{"id":"msg-worker-handoff-bad","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-review-write-bad","name":"write_file","input":{"path":%q,"content":"not a review handoff"}}],"stop_reason":"tool_use"}`, path) + } + args, _ := json.Marshal(map[string]string{"path": path, "content": "not a review handoff"}) + return fmt.Sprintf(`{"id":"chatcmpl-worker-handoff-bad","created":4,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":"provider-review-write-bad","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(args)) +} + +func scriptedReviewInspection(endpoint string) string { + return scriptedOrdinaryTool(endpoint, "provider-review-inspect", "inspect caller result") +} + +func scriptedOrdinaryTool(endpoint, id, command string) string { + if endpoint == "anthropic" { + return fmt.Sprintf(`{"id":"msg-ordinary","type":"message","role":"assistant","content":[{"type":"tool_use","id":%q,"name":"run_command","input":{"command":%q}}],"stop_reason":"tool_use"}`, id, command) + } + args, _ := json.Marshal(map[string]string{"command": command}) + return fmt.Sprintf(`{"id":"chatcmpl-ordinary","created":4,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":%q,"type":"function","function":{"name":"run_command","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, id, string(args)) +} + +func scriptedReviewArtifactReads(endpoint, requestID string) string { + paths := newReservedPaths(requestID) + if endpoint == "anthropic" { + return fmt.Sprintf(`{"id":"msg-review-reads","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-review-plan-read","name":"read_file","input":{"path":%q}},{"type":"tool_use","id":"provider-review-handoff-read","name":"read_file","input":{"path":%q}}],"stop_reason":"tool_use"}`, paths.PlanPath, paths.ReviewPath) + } + planArgs, _ := json.Marshal(map[string]string{"path": paths.PlanPath}) + reviewArgs, _ := json.Marshal(map[string]string{"path": paths.ReviewPath}) + return fmt.Sprintf(`{"id":"chatcmpl-review-reads","created":6,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":"provider-review-plan-read","type":"function","function":{"name":"read_file","arguments":%q}},{"id":"provider-review-handoff-read","type":"function","function":{"name":"read_file","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(planArgs), string(reviewArgs)) +} + +func scriptedReviewReadsAndInspection(endpoint, requestID string) string { + paths := newReservedPaths(requestID) + if endpoint == "anthropic" { + return fmt.Sprintf(`{"id":"msg-review-inspection","type":"message","role":"assistant","content":[{"type":"tool_use","id":"provider-review-plan-read","name":"read_file","input":{"path":%q}},{"type":"tool_use","id":"provider-review-handoff-read","name":"read_file","input":{"path":%q}},{"type":"tool_use","id":"provider-review-inspect","name":"run_command","input":{"command":"inspect caller result"}}],"stop_reason":"tool_use"}`, paths.PlanPath, paths.ReviewPath) + } + planArgs, _ := json.Marshal(map[string]string{"path": paths.PlanPath}) + reviewArgs, _ := json.Marshal(map[string]string{"path": paths.ReviewPath}) + inspectArgs, _ := json.Marshal(map[string]string{"command": "inspect caller result"}) + return fmt.Sprintf(`{"id":"chatcmpl-review-inspection","created":6,"choices":[{"message":{"role":"assistant","tool_calls":[{"id":"provider-review-plan-read","type":"function","function":{"name":"read_file","arguments":%q}},{"id":"provider-review-handoff-read","type":"function","function":{"name":"read_file","arguments":%q}},{"id":"provider-review-inspect","type":"function","function":{"name":"run_command","arguments":%q}}]},"finish_reason":"tool_calls"}]}`, string(planArgs), string(reviewArgs), string(inspectArgs)) +} + +func scriptedReviewReadsAndInspectionWithUsage(requestID string, inputTokens, outputTokens int) string { + paths := newReservedPaths(requestID) + return fmt.Sprintf(`{"id":"msg-review-inspection","type":"message","role":"assistant","content":[{"type":"thinking","thinking":"review-reason","signature":"sig-review"},{"type":"text","text":"review-visible"},{"type":"tool_use","id":"provider-review-plan-read","name":"read_file","input":{"path":%q}},{"type":"tool_use","id":"provider-review-handoff-read","name":"read_file","input":{"path":%q}},{"type":"tool_use","id":"provider-review-inspect","name":"run_command","input":{"command":"inspect caller result"}}],"stop_reason":"tool_use","usage":{"input_tokens":%d,"output_tokens":%d}}`, paths.PlanPath, paths.ReviewPath, inputTokens, outputTokens) } func scriptedReviewWriteWithUsage(endpoint, requestID string, inputTokens, outputTokens int) string { path := newReservedPaths(requestID).ReviewPath + review := testCompletedReviewText() if endpoint == "anthropic" { - return fmt.Sprintf(`{"id":"msg-review-write","type":"message","role":"assistant","content":[{"type":"thinking","thinking":"review-reason","signature":"sig-review"},{"type":"text","text":"review-visible"},{"type":"tool_use","id":"provider-review-write","name":"write_file","input":{"path":%q,"content":"review body"}}],"stop_reason":"tool_use","usage":{"input_tokens":%d,"output_tokens":%d}}`, path, inputTokens, outputTokens) + return fmt.Sprintf(`{"id":"msg-review-write","type":"message","role":"assistant","content":[{"type":"thinking","thinking":"review-reason","signature":"sig-review"},{"type":"text","text":"review-visible"},{"type":"tool_use","id":"provider-review-write","name":"write_file","input":{"path":%q,"content":%q}}],"stop_reason":"tool_use","usage":{"input_tokens":%d,"output_tokens":%d}}`, path, review, inputTokens, outputTokens) } - args, _ := json.Marshal(map[string]string{"path": path, "content": "review body"}) + args, _ := json.Marshal(map[string]string{"path": path, "content": review}) return fmt.Sprintf(`{"id":"chatcmpl-review-write","created":5,"choices":[{"message":{"role":"assistant","content":"review-visible","reasoning_content":"review-reason","tool_calls":[{"id":"provider-review-write","type":"function","function":{"name":"write_file","arguments":%q}}]},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":%d,"completion_tokens":%d,"total_tokens":%d}}`, string(args), inputTokens, outputTokens, inputTokens+outputTokens) } diff --git a/apps/edge/internal/openai/hot_path_observation.go b/apps/edge/internal/openai/hot_path_observation.go index 0fd99016..c11ff0b0 100644 --- a/apps/edge/internal/openai/hot_path_observation.go +++ b/apps/edge/internal/openai/hot_path_observation.go @@ -656,7 +656,7 @@ func hotPathStageKindForPhase(phase hotPathLightPhase) hotPathStageKind { switch phase { case hotPathPhaseLocalActive: return hotPathStageKindLocal - case hotPathPhaseReviewActive, hotPathPhaseReviewAwaitRead, hotPathPhaseReviewResolution, hotPathPhaseReviewRepair: + case hotPathPhaseReviewActive, hotPathPhaseReviewRepair: return hotPathStageKindReview case hotPathPhaseCleanupPending: return hotPathStageKindCleanup diff --git a/apps/edge/internal/openai/hot_path_observation_test.go b/apps/edge/internal/openai/hot_path_observation_test.go index 8a729113..783ec406 100644 --- a/apps/edge/internal/openai/hot_path_observation_test.go +++ b/apps/edge/internal/openai/hot_path_observation_test.go @@ -1496,10 +1496,10 @@ func hotPathPassTrace() []hotPathTracePoint { return []hotPathTracePoint{ {Event: hotPathEventClassDispatch}, {Event: hotPathEventClassStage, Stage: hotPathStageKindLocal, Attempt: hotPathAttemptFirst, Disposition: hotPathTerminalDispositionToolTurn}, + {Event: hotPathEventClassStage, Stage: hotPathStageKindLocal, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionToolTurn}, {Event: hotPathEventClassStage, Stage: hotPathStageKindLocal, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionSuccess}, {Event: hotPathEventClassLight, Stage: hotPathStageKindReview, Attempt: hotPathAttemptFirst}, {Event: hotPathEventClassStage, Stage: hotPathStageKindReview, Attempt: hotPathAttemptFirst, Disposition: hotPathTerminalDispositionToolTurn}, - {Event: hotPathEventClassStage, Stage: hotPathStageKindReview, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionToolTurn}, {Event: hotPathEventClassStage, Stage: hotPathStageKindReview, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionSuccess}, {Event: hotPathEventClassLight, Stage: hotPathStageKindCleanup, Attempt: hotPathAttemptFirst}, {Event: hotPathEventClassCleanup, Cleanup: hotPathCleanupOutcomeSuccess}, diff --git a/apps/edge/internal/openai/hot_path_review.go b/apps/edge/internal/openai/hot_path_review.go index a39ab9ea..b02af66c 100644 --- a/apps/edge/internal/openai/hot_path_review.go +++ b/apps/edge/internal/openai/hot_path_review.go @@ -3,6 +3,7 @@ package openai import ( "context" "fmt" + "strings" ) func (s *Server) advanceHotPathReview( @@ -14,7 +15,11 @@ func (s *Server) advanceHotPathReview( outer *hotPathOuterTurn, protocol string, ) (normalizedStageOutput, bool, error) { - kind, cleanup, err := classifyHotPathReviewOutput(requestID, phase, output) + evidence, err := s.lightFlows.reviewEvidence(requestID, s.edgeIDValue()) + if err != nil { + return normalizedStageOutput{}, false, err + } + kind, cleanup, err := classifyHotPathReviewOutput(requestID, phase, output, evidence) if err != nil { return normalizedStageOutput{}, false, err } @@ -37,56 +42,46 @@ func (s *Server) advanceHotPathReview( return mapped, true, nil } -func classifyHotPathReviewOutput(requestID string, phase hotPathLightPhase, output normalizedStageOutput) (hotPathPendingKind, bool, error) { +func classifyHotPathReviewOutput(requestID string, phase hotPathLightPhase, output normalizedStageOutput, evidence hotPathReviewEvidence) (hotPathPendingKind, bool, error) { paths := newReservedPaths(requestID) switch phase { case hotPathPhaseReviewActive: if len(output.ToolCalls) == 0 { - return "", false, fmt.Errorf("review stage completed before writing the issued review artifact") - } - writeCount := 0 - reservedCount := 0 - for _, call := range output.ToolCalls { - observed := reservedPathsFromToolCall(call) - if len(observed) == 0 { - continue + if !evidence.planRead || !evidence.reviewRead || !evidence.inspected { + return "", false, fmt.Errorf("review completion requires both artifact reads and a successful ordinary result inspection") } - reservedCount++ - if len(observed) == 1 && cleanRelativePath(observed[0]) == cleanRelativePath(paths.ReviewPath) { - writeCount++ + if strings.TrimSpace(output.Content) == "" { + return "", false, fmt.Errorf("review terminal output must be non-empty") } - } - if writeCount == 0 && reservedCount == 0 { - return hotPathPendingReviewInspection, false, nil - } - if writeCount == 1 && reservedCount == 1 && len(output.ToolCalls) == 1 { - return hotPathPendingReviewWrite, false, nil - } - return "", false, fmt.Errorf("review write must be one exact review-path tool call") - - case hotPathPhaseReviewAwaitRead: - if len(output.ToolCalls) != 1 { - return "", false, fmt.Errorf("review write result must be followed by one exact review read") - } - observed := reservedPathsFromToolCall(output.ToolCalls[0]) - if len(observed) != 1 || cleanRelativePath(observed[0]) != cleanRelativePath(paths.ReviewPath) { - return "", false, fmt.Errorf("review write result must be followed by the issued review read") - } - return hotPathPendingReviewRead, false, nil - - case hotPathPhaseReviewResolution: - if len(output.ToolCalls) == 0 { return "", true, nil } for _, call := range output.ToolCalls { - if len(reservedPathsFromToolCall(call)) > 0 { - return "", false, fmt.Errorf("review resolution cannot start another reserved review cycle") + observed := reservedPathsFromToolCall(call) + for _, path := range observed { + clean := cleanRelativePath(path) + if clean != cleanRelativePath(paths.PlanPath) && clean != cleanRelativePath(paths.ReviewPath) { + return "", false, fmt.Errorf("review inspection targets an unissued artifact") + } } } - return hotPathPendingReviewRepair, false, nil + if evidence.planRead && evidence.reviewRead && evidence.inspected { + for _, call := range output.ToolCalls { + if len(reservedPathsFromToolCall(call)) > 0 { + return "", false, fmt.Errorf("review repair cannot restart artifact inspection") + } + } + return hotPathPendingReviewRepair, false, nil + } + return hotPathPendingReviewInspection, false, nil case hotPathPhaseReviewRepair: if len(output.ToolCalls) == 0 { + if !evidence.planRead || !evidence.reviewRead || !evidence.inspected { + return "", false, fmt.Errorf("repair completion requires retained review evidence") + } + if strings.TrimSpace(output.Content) == "" { + return "", false, fmt.Errorf("review terminal output must be non-empty") + } return "", true, nil } for _, call := range output.ToolCalls { diff --git a/apps/edge/internal/openai/hot_path_review_test.go b/apps/edge/internal/openai/hot_path_review_test.go index 53a3f134..fe8f4658 100644 --- a/apps/edge/internal/openai/hot_path_review_test.go +++ b/apps/edge/internal/openai/hot_path_review_test.go @@ -47,14 +47,33 @@ func TestHotPathReviewDefectRepair(t *testing.T) { func TestHotPathReviewStructureIgnoresProseVerdict(t *testing.T) { completion := normalizedStageOutput{Content: "DEFECT FAIL words do not control state"} - if kind, cleanup, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewResolution, completion); err != nil || kind != "" || !cleanup { + evidence := hotPathReviewEvidence{planRead: true, reviewRead: true, inspected: true} + if kind, cleanup, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewActive, completion, evidence); err != nil || kind != "" || !cleanup { t.Fatalf("completion structure did not pass: kind=%q cleanup=%t err=%v", kind, cleanup, err) } repair := normalizedStageOutput{ Content: "PASS words do not control state", ToolCalls: []normalizedToolCall{{ID: "provider_repair", Name: "run_command", Arguments: map[string]any{"command": "go test"}}}, } - if kind, cleanup, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewResolution, repair); err != nil || kind != hotPathPendingReviewRepair || cleanup { + if kind, cleanup, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewActive, repair, evidence); err != nil || kind != hotPathPendingReviewRepair || cleanup { t.Fatalf("repair structure did not stay active: kind=%q cleanup=%t err=%v", kind, cleanup, err) } } + +func TestHotPathReviewRequiresInspectionAndNonEmptyTerminal(t *testing.T) { + completion := normalizedStageOutput{Content: "reviewed"} + if _, _, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewActive, completion, hotPathReviewEvidence{}); err == nil || !strings.Contains(err.Error(), "artifact reads") { + t.Fatalf("skipped inspection error = %v", err) + } + evidence := hotPathReviewEvidence{planRead: true, reviewRead: true, inspected: true} + if _, _, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewActive, normalizedStageOutput{}, evidence); err == nil || !strings.Contains(err.Error(), "non-empty") { + t.Fatalf("empty terminal error = %v", err) + } + reservedRepair := normalizedStageOutput{ToolCalls: []normalizedToolCall{{ + ID: "provider_second_review", Name: "read_file", + Arguments: map[string]any{"path": newReservedPaths("req_review").ReviewPath}, + }}} + if _, _, err := classifyHotPathReviewOutput("req_review", hotPathPhaseReviewRepair, reservedRepair, evidence); err == nil || !strings.Contains(err.Error(), "second review cycle") { + t.Fatalf("second review cycle error = %v", err) + } +} diff --git a/apps/edge/internal/openai/hot_path_stage_input.go b/apps/edge/internal/openai/hot_path_stage_input.go index 585b0b5d..80070eff 100644 --- a/apps/edge/internal/openai/hot_path_stage_input.go +++ b/apps/edge/internal/openai/hot_path_stage_input.go @@ -125,17 +125,13 @@ func (in hotPathStageInput) prompt(phase hotPathLightPhase) (string, error) { switch in.Role { case "local": - b.WriteString("Use the available caller tools to read both issued artifacts. Perform the task and its verification in the caller workspace. Keep using ordinary tool calls until the work is complete, then return a completion without a tool call.") + b.WriteString("Read the exact issued PLAN before doing any work. Perform the task and verification in the caller workspace with ordinary caller tools. Then overwrite the exact issued REVIEW path using this closed handoff grammar: '# Review', '## Worker Item Status' with every PLAN item exactly once as '- Pn: completed', '## Worker Changes', '## Worker Verification', and '## Deviations'; every section must be non-empty. Do not complete without a successful PLAN read and REVIEW write.") case "review": switch phase { case hotPathPhaseReviewActive: - b.WriteString("Inspect the completed local work with ordinary caller tools. Then write the review to the exact issued review path. Do not decide from a hidden marker or a prose verdict supplied by the Edge.") - case hotPathPhaseReviewAwaitRead: - b.WriteString("The review write completed. Read the exact issued review path with the caller read tool before resolving the review. Stay in this same review stage.") - case hotPathPhaseReviewResolution: - b.WriteString("Resolve the review using the returned tool evidence. If no repair is needed, complete without a tool call. If repair is needed, use ordinary caller tools to repair and verify, then complete without starting another review.") + b.WriteString("Read both exact issued PLAN and REVIEW artifacts. Inspect at least one actual caller-workspace result with an ordinary caller tool. Do not rewrite REVIEW. If the work is correct, return a concise non-empty final result without a tool call. If repair is needed, use ordinary caller tools to repair and re-verify in this same review stage.") case hotPathPhaseReviewRepair: - b.WriteString("Continue the same review-stage repair and verification with ordinary caller tools. When finished, complete without another review write/read cycle.") + b.WriteString("Continue the same review-stage repair and verification with ordinary caller tools. Do not read or rewrite the reserved artifacts again. When finished, return a concise non-empty final result without a tool call.") default: return "", fmt.Errorf("review input cannot run in phase %q", phase) } diff --git a/apps/edge/internal/openai/provider_model_rewrite.go b/apps/edge/internal/openai/provider_model_rewrite.go index f771afbb..c40a140c 100644 --- a/apps/edge/internal/openai/provider_model_rewrite.go +++ b/apps/edge/internal/openai/provider_model_rewrite.go @@ -11,6 +11,185 @@ import ( "iop/packages/go/config" ) +const maxHotPathSelectorProviderInstructionBytes = 4096 + +func buildHotPathSelectorProviderInstruction(requestID string, state selectorInstructionState) (string, error) { + if !validLogicalRequestID(requestID) { + return "", fmt.Errorf("selector artifact request identity is invalid") + } + paths := newReservedPaths(requestID) + var instruction string + switch state { + case selectorInstructionPrepareOnly: + instruction = fmt.Sprintf(`IOP caller-workspace selector instruction. +Operation: prepare-only +Return exactly one admitted prepare tool call for this request-local job directory and no other tool call: +JOB directory: %s +Do not write PLAN or REVIEW in this turn. Do not mention or infer an absolute workspace path.`, paths.JobDir) + case selectorInstructionPairWrite: + instruction = fmt.Sprintf(`IOP caller-workspace selector instruction. +Operation: pair-write +Return exactly two admitted write tool calls for the request-local artifact pair and no other tool call. Do not mention or infer an absolute workspace path. +PLAN path: %s +REVIEW path: %s +The default PLAN grammar accepts 2-6 consecutive steps and 1-3 verification bullets. For this deterministic seed, use exactly the following two-step, one-verification form and replace only angle-bracketed text: +# Plan + +## Goal + + +## Steps +- [P1] +- [P2] + +## Verification +- +The pending REVIEW content must be exactly this deterministic seed: +# Review + +## Worker Item Status +- P1: pending +- P2: pending + +## Worker Changes +Pending worker execution. + +## Worker Verification +Pending worker verification. + +## Deviations +None recorded.`, paths.PlanPath, paths.ReviewPath) + default: + return "", fmt.Errorf("selector provider instruction state is invalid") + } + if len(instruction) > maxHotPathSelectorProviderInstructionBytes { + return "", fmt.Errorf("selector provider instruction exceeds bounded size") + } + return instruction, nil +} + +func prepareHotPathSelectorProviderInstruction(tunnel edgeservice.SubmitProviderTunnelRequest, instruction string) (edgeservice.SubmitProviderTunnelRequest, error) { + if strings.TrimSpace(instruction) == "" { + return tunnel, nil + } + rewrite := func(body []byte) ([]byte, error) { + switch config.ProtocolOperation(tunnel.Operation) { + case config.OperationChatCompletions: + return injectHotPathChatSelectorInstruction(body, instruction) + case config.OperationMessages: + return injectHotPathAnthropicSelectorInstruction(body, instruction) + default: + return nil, fmt.Errorf("selector provider instruction does not support operation %q", tunnel.Operation) + } + } + if tunnel.BuildBody != nil { + build := tunnel.BuildBody + tunnel.BuildBody = func(target string) ([]byte, error) { + body, err := build(target) + if err != nil { + return nil, err + } + return rewrite(body) + } + return tunnel, nil + } + if len(tunnel.Body) == 0 { + return tunnel, fmt.Errorf("selector provider body is unavailable") + } + body, err := rewrite(tunnel.Body) + if err != nil { + return tunnel, err + } + tunnel.Body = body + return tunnel, nil +} + +func injectHotPathChatSelectorInstruction(body []byte, instruction string) ([]byte, error) { + var envelope struct { + Messages []json.RawMessage `json:"messages"` + } + if err := json.Unmarshal(body, &envelope); err != nil { + return nil, fmt.Errorf("decode selector Chat provider body: %w", err) + } + if len(envelope.Messages) == 0 { + return nil, fmt.Errorf("selector Chat provider body has no messages") + } + insertAt := 0 + for insertAt < len(envelope.Messages) { + var message struct { + Role string `json:"role"` + } + if err := json.Unmarshal(envelope.Messages[insertAt], &message); err != nil || message.Role != "system" { + break + } + insertAt++ + } + injected, err := json.Marshal(map[string]string{"role": "system", "content": instruction}) + if err != nil { + return nil, err + } + messages := make([]json.RawMessage, 0, len(envelope.Messages)+1) + messages = append(messages, envelope.Messages[:insertAt]...) + messages = append(messages, injected) + messages = append(messages, envelope.Messages[insertAt:]...) + messagesJSON, err := json.Marshal(messages) + if err != nil { + return nil, err + } + plan, err := planTopLevelJSONPatches(body, []topLevelJSONPatch{{name: "messages", value: messagesJSON}}) + if err != nil { + return nil, err + } + return plan.apply(), nil +} + +func injectHotPathAnthropicSelectorInstruction(body []byte, instruction string) ([]byte, error) { + fields, _, err := scanTopLevelJSONObject(body) + if err != nil { + return nil, fmt.Errorf("decode selector Messages provider body: %w", err) + } + blocks := make([]json.RawMessage, 0, 2) + for _, field := range fields { + if field.name != "system" { + continue + } + raw := bytes.TrimSpace(body[field.valueFrom:field.valueTo]) + if len(raw) == 0 || bytes.Equal(raw, []byte("null")) { + break + } + if raw[0] == '"' { + var text string + if err := json.Unmarshal(raw, &text); err != nil { + return nil, fmt.Errorf("decode selector Messages system text: %w", err) + } + block, err := json.Marshal(map[string]string{"type": "text", "text": text}) + if err != nil { + return nil, err + } + blocks = append(blocks, block) + break + } + if err := json.Unmarshal(raw, &blocks); err != nil { + return nil, fmt.Errorf("decode selector Messages system blocks: %w", err) + } + break + } + injected, err := json.Marshal(map[string]string{"type": "text", "text": instruction}) + if err != nil { + return nil, err + } + blocks = append(blocks, injected) + systemJSON, err := json.Marshal(blocks) + if err != nil { + return nil, err + } + plan, err := planTopLevelJSONPatches(body, []topLevelJSONPatch{{name: "system", value: systemJSON}}) + if err != nil { + return nil, err + } + return plan.apply(), nil +} + type openAIProviderBodyBuilder struct { mu sync.Mutex build func(string) (*openAIRebuiltLease, error) diff --git a/apps/edge/internal/openai/request_identity_ingress.go b/apps/edge/internal/openai/request_identity_ingress.go index 05603353..342b835a 100644 --- a/apps/edge/internal/openai/request_identity_ingress.go +++ b/apps/edge/internal/openai/request_identity_ingress.go @@ -6,6 +6,8 @@ import ( "fmt" "net/http" "strings" + + "iop/packages/go/config" ) const hotPathInitialAdmissionMetadata = "iop_hot_path_initial_admission" @@ -14,6 +16,18 @@ func isInitialHotPathAdmission(metadata map[string]string) bool { return metadata != nil && metadata[hotPathInitialAdmissionMetadata] == "true" } +func (s *Server) hotPathSelectorProviderInstruction(metadata map[string]string) (string, error) { + if s == nil || s.artifactFrontiers == nil || metadata == nil { + return "", nil + } + requestID := strings.TrimSpace(metadata["iop_logical_request_id"]) + state := s.artifactFrontiers.selectorInstructionState(requestID, s.edgeIDValue()) + if state == selectorInstructionNone { + return "", nil + } + return buildHotPathSelectorProviderInstruction(requestID, state) +} + func (s *Server) joinPresetChatIngress(r *http.Request, dispatch routeDispatch, rawBody []byte, runMeta map[string]string) (presetIngressResult, error) { delete(runMeta, hotPathInitialAdmissionMetadata) s.sweepLogicalRequestTTL() @@ -54,7 +68,7 @@ func (s *Server) joinPresetChatIngress(r *http.Request, dispatch routeDispatch, return presetIngressResult{}, fmt.Errorf("artifact continuation rejected: %w", err) } if disposition.PrimaryError != nil { - if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false, nil); err != nil { return presetIngressResult{}, err } cleanup, err := s.lightFlows.beginPrimaryErrorCleanup(requestContext, snap.ID, ownerEdgeID, *disposition.PrimaryError, nil, s.requestCoordinator) @@ -71,7 +85,7 @@ func (s *Server) joinPresetChatIngress(r *http.Request, dispatch routeDispatch, if err := s.applyArtifactDisposition(snap, disposition, runMeta); err != nil { return presetIngressResult{}, err } - if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible, disposition.PlanItemIDs); err != nil { return presetIngressResult{}, err } return presetIngressResult{Artifact: disposition}, nil @@ -205,7 +219,7 @@ func (s *Server) joinPresetAnthropicIngress(r *http.Request, dispatch routeDispa return presetIngressResult{}, fmt.Errorf("artifact continuation rejected: %w", err) } if disposition.PrimaryError != nil { - if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false, nil); err != nil { return presetIngressResult{}, err } cleanup, err := s.lightFlows.beginPrimaryErrorCleanup(requestContext, snap.ID, ownerEdgeID, *disposition.PrimaryError, nil, s.requestCoordinator) @@ -222,7 +236,7 @@ func (s *Server) joinPresetAnthropicIngress(r *http.Request, dispatch routeDispa if err := s.applyArtifactDisposition(snap, disposition, metadata); err != nil { return presetIngressResult{}, err } - if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible, disposition.PlanItemIDs); err != nil { return presetIngressResult{}, err } return presetIngressResult{Artifact: disposition}, nil @@ -327,7 +341,7 @@ func (s *Server) applyLightDisposition(snap logicalRequestSnapshot, disposition metadata["iop_logical_request_id"] = disposition.RequestID metadata["iop_call_id"] = callID metadata["iop_stage_id"] = disposition.StageID - if disposition.TransitionFrom == hotPathPhaseReviewResolution && disposition.Phase == hotPathPhaseReviewRepair { + if disposition.TransitionFrom == hotPathPhaseReviewActive && disposition.Phase == hotPathPhaseReviewRepair { s.observeHotPathLightTransition(context.Background(), hotPathStageKindReview, hotPathAttemptRetry, disposition.RequestID, disposition.StageID, presetID) } @@ -386,13 +400,7 @@ func (s *Server) compilePresetArtifactBinding(dispatch routeDispatch, protocol s preset = found } } - // Marked single-request presets use the operator-owned internal workspace - // capability compiled into dispatch.SingleRequest. They intentionally reject - // legacy caller workspace_tools, so artifact binding must not run here. - if preset.SingleRequest != nil || dispatch.SingleRequest != nil { - return nil, false, nil - } - if !isModeAllowed(preset, modeLight) { + if !presetAdmitsCallerWorkspaceTools(preset, dispatch) { return nil, false, nil } tools, err := decodeArtifactTools(protocol, rawBody) @@ -406,6 +414,17 @@ func (s *Server) compilePresetArtifactBinding(dispatch routeDispatch, protocol s return binding, true, nil } +// presetAdmitsCallerWorkspaceTools is the ownership discriminator for the two +// intentionally separate hybrid paths. An unmarked light preset may compile +// only caller-admitted tools; a marked single-request preset always retains its +// operator-owned workspace capability and never enters caller continuation. +func presetAdmitsCallerWorkspaceTools(preset config.ExecutionPreset, dispatch routeDispatch) bool { + if preset.SingleRequest != nil || dispatch.SingleRequest != nil { + return false + } + return isModeAllowed(preset, modeLight) +} + func hasChatContinuationStructure(rawBody []byte) bool { var env struct { Messages []struct { diff --git a/configs/edge.yaml b/configs/edge.yaml index 0e3a73f2..3a4f1f03 100644 --- a/configs/edge.yaml +++ b/configs/edge.yaml @@ -549,6 +549,28 @@ nodes: # max_write_bytes: 524288 # max_output_bytes: 8388608 # +# === Caller-workspace hybrid preset example (commented) === +# Agent-facing aliases use an unmarked light preset plus admitted caller tools. +# Do not add single_request or a raw workspace path to this preset; the caller's +# workspace_tools alternative binds the caller-opened workspace at request time. +# execution_presets: +# - id: "preset-caller-hybrid" +# selector: +# model: "qwen3.6:35b" +# allowed_modes: ["light"] +# routes: +# light: +# stages: +# - role: "work" +# model: "qwen3.6:35b" +# - role: "review" +# model: "qwen3.6:35b" +# workspace_tools: +# # Ownership-only excerpt: replace operations with one complete closed +# # read/write/delete/prepare mapping for the caller's actual tool schemas. +# - name: "caller-workspace" +# operations: {} +# # === Fixed single-request preset example (commented) === # execution_presets[] entry with operator-owned fixed single-request policy. # Live-apply on refresh; affects only new request snapshots.