diff --git a/agent-contract/outer/anthropic-compatible-api.md b/agent-contract/outer/anthropic-compatible-api.md index de48c1e0..dcddf17d 100644 --- a/agent-contract/outer/anthropic-compatible-api.md +++ b/agent-contract/outer/anthropic-compatible-api.md @@ -93,13 +93,24 @@ response model semantics. 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 +workspace path. Before issuing caller writes, Edge checks only that the selector's +PLAN and pending REVIEW are non-empty writes within the admitted workspace boundary. +Its Edge-owned system instruction applies a compact Plan contract: +analyze the immutable task first, preserve explicit requirements and acceptance, +close 2-6 executable steps and deterministic verification, and put the exact REVIEW +handoff duty in the final step. +Work receives only the absolute PLAN path resolved from the caller's existing working-directory system context, followed by the fixed English sentence +`Read it and complete the task.` and sees only ordinary caller tools. It must read +PLAN, perform and verify caller-workspace work, and replace the exact REVIEW artifact +with completed item statuses, actual changes, verification evidence, and deviations. +Edge projects a non-empty Worker-authored REVIEW write without pre-validating its +semantic completeness; Review inspects and repairs incomplete handoff evidence. Only +exact PLAN/REVIEW accesses contribute artifact state; every other caller-workspace +tool call passes through unchanged. Review alone receives an Edge-owned compact +code-review system instruction. It must read both artifacts, inspect the actual +caller result, and rerun applicable verification before returning a non-empty +caller-visible terminal. It never rewrites REVIEW or creates a Result document; +root-cause 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, @@ -108,14 +119,25 @@ write operation cannot create parents receives a `prepare-only` initial instruct 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 +one atomic artifact-pair call containing the complete PLAN and REVIEW content with +the accepted bounded PLAN grammar and matching pending REVIEW inventory. Edge +expands that decision into two independently receipted admitted caller writes. 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. +path. Work, cleanup, and marked `single_request` provider bodies do not receive it; +Review receives only its separate compact code-review system instruction. + +Continuation issue correlation hashes only the emitted tool-use identity, name, and +arguments/input. SDK-added text, thinking, and provider metadata decorations are not +part of that issue identity; the retained request prefix and exact result IDs remain +independently validated. + +For a command-mode caller binding, Work/Review provider deltas are collected through +the bounded internal stage before caller projection. Edge validates and maps canonical +operations first, so an internal tool name is never released early on a Messages stream; +directly projectable structured bindings retain progressive streaming. A preset marked with `single_request` is excluded from this continuation before caller tool binding. Config validation rejects a preset that declares both @@ -160,9 +182,9 @@ Edge-owned internal stage inputs only: The stage then requests a stage-owned strict JSON object with a one-line `goal` string, 2-6 non-empty one-line `steps` strings, and 1-3 non-empty one-line `verification` strings. Edge renders deterministic `P1..Pn` step IDs into the - frozen PLAN and Work must parse that stored document before provider dispatch. - Work returns strict worker item status, changes, verification, and deviations; - Edge renders and validates the one `review.md` handoff. Review reads both stored + frozen PLAN. Work receives that stored document without a second semantic parser. + Work returns worker item status, changes, verification, and deviations; + Edge renders the `review.md` handoff without blocking partial evidence. Review reads both stored artifacts, never accepts a memory work payload, and never rewrites `review.md`. Provider output never controls artifact headings or static template text. - The caller-visible request and response schemas are unchanged. A configured diff --git a/agent-contract/outer/openai-compatible-api.md b/agent-contract/outer/openai-compatible-api.md index a6939f35..8615ecbc 100644 --- a/agent-contract/outer/openai-compatible-api.md +++ b/agent-contract/outer/openai-compatible-api.md @@ -264,7 +264,7 @@ Provider-pool raw passthrough route는 위 목록을 provider request allowlist Chat Completions의 실행 경로는 caller가 보낸 `model`의 route/provider capability로 결정한다. - provider-pool model group route(`models[]`)는 candidate를 선택한 뒤 selected provider가 OpenAI-compatible 호출 방식을 지원하면 provider HTTP status/header/body를 Node가 열어 기존 Edge-Node tunnel로 relay하고, Edge가 caller에게 쓴다. 요청 body는 라우팅에 필요한 envelope만 읽고 `model` alias를 selected provider의 served target으로 rewrite하는 것을 기본으로 하며, provider가 지원하는 OpenAI-compatible field와 provider extension field를 보존한다. -- Provider-pool Chat과 unmarked caller-workspace의 selector/Work/Review Chat 요청은 모두 selected protocol profile이 선언한 출력 상한 wire spelling을 사용한다. OpenAI Chat wire는 `max_completion_tokens`, Gemini Chat wire는 `max_tokens`로 정규화하며 caller/agent identity로 분기하지 않는다. +- Provider-pool Chat과 unmarked caller-workspace의 selector/Work/Review 요청은 selected protocol profile이 선언한 출력 상한 wire spelling을 사용한다. OpenAI Chat wire는 `max_completion_tokens`, Gemini Chat wire는 `max_tokens`로 정규화한다. Bounded selector PLAN/REVIEW pair는 caller SDK의 큰 기본 completion limit과 분리해 최대 4096 output tokens만 허용한다. Selector provider에는 prepare 단계의 canonical `prepare(path)` 또는 pair-write 단계의 단일 `iop_write_artifact_pair(plan_content,review_content)` schema만 보여준다. Selector system instruction은 compact Plan 계약에 따라 immutable task를 먼저 분석하고 explicit requirement/constraint/acceptance를 보존한 2-6개 closed step, deterministic verification, REVIEW handoff duty를 작성하도록 한다. Edge는 workspace 경계와 비어 있지 않은 write만 확인한 atomic pair를 admitted caller tool schema의 PLAN/REVIEW 두 write로 확장한다. Work provider의 전체 task prompt는 caller의 기존 working-directory system context에서 해석한 absolute PLAN path와 고정 영문 `Read it and complete the task.`뿐이며 ordinary caller tools만 보인다. Work는 PLAN을 읽고 구현·검증한 뒤 item status, actual changes, verification evidence, deviation을 담은 REVIEW write를 직접 반환한다. Edge는 이 Worker-authored handoff의 의미 완성도를 선검증하지 않으며, 부분 handoff도 Review가 검사·수리하도록 전달한다. Review provider에만 별도 compact code-review system instruction을 주입한다. Review는 PLAN/REVIEW와 actual caller result를 읽고 applicable verification을 재실행하며, defect가 있으면 같은 binding에서 root cause를 확정해 repair/re-verify한 후에만 terminal을 반환한다. REVIEW를 다시 쓰거나 별도 Result 문서를 만들지 않는다. 정확한 PLAN/REVIEW 접근만 artifact state로 기록하고 그 외 caller-workspace tool call은 그대로 통과시킨다. Reserved-artifact operation은 provider tool의 path alias를 canonical path로 복원하고 caller binding의 tool/argument mapping으로 정규화한다. Command-mode binding stage는 provider delta를 선행 방출하지 않고 bounded stage를 수집·매핑하며 structured binding의 progressive stream은 유지한다. Cleanup은 request-local job directory만 제거한다. Caller continuation은 SDK 장식이 아니라 발행된 tool-call ID·이름·arguments의 의미 해시로 issue를 결속한다. Function tools와 reasoning effort를 Chat wire가 함께 표현하지 못하면 같은 selected profile의 Responses operation으로 내부 wire만 전환한다. Result decoder는 selected operation을 우선하며 caller/agent identity로 분기하지 않는다. - selected provider가 Ollama/native provider처럼 normalized execution을 요구하면 Edge는 normalized `RunRequest` path를 사용한다. 이 경로는 OpenAI-compatible 표면을 입력/출력 compatibility layer로 제공하되, backend 호출은 normalized adapter 계약을 따른다. - `metadata`는 경로 선택자가 아니다. Edge는 route 결정 뒤 인증 principal, usage/observability 등 IOP가 아는 bounded metadata key만 발췌한다. 이 발췌 정보는 provider body를 바꾸는 selector가 아니며, passthrough 응답 body에 IOP marker/event/envelope를 섞지 않는다. - Chat Completions 성공 응답의 top-level `model` echo가 provider-served model이면 caller가 요청한 IOP model alias로 정규화할 수 있다. reasoning/content/tool_calls 같은 provider payload field는 보존한다. diff --git a/agent-spec/input/openai-compatible-surface.md b/agent-spec/input/openai-compatible-surface.md index 2d79a468..f34e3b68 100644 --- a/agent-spec/input/openai-compatible-surface.md +++ b/agent-spec/input/openai-compatible-surface.md @@ -202,7 +202,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. Selector/Work/Review Chat bodies normalize `max_tokens`/`max_completion_tokens` to the selected profile wire; target-native values win when both aliases exist. A selector upstream HTTP 400 becomes one non-retryable caller HTTP 400 `invalid_request_error`, while provider details remain private. 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. | +| unmarked caller-workspace light route | An unmarked `light` preset binds only admitted caller `workspace_tools`; it never accepts a raw caller path. Edge derives `prepare-only` or atomic `pair-write` from the locked artifact frontier. The selector receives a compact Plan system contract: analyze the immutable task, preserve explicit requirements/constraints/acceptance, author concise closed steps plus deterministic verification, and include the REVIEW handoff duty. Edge checks only the admitted workspace boundary, non-empty writes, and size bounds before projecting PLAN/REVIEW. Work receives only the absolute PLAN path plus `Read it and complete the task.` and ordinary caller tools; no Plan/Review skill system prompt is added to Work. Work implements and verifies, then directly replaces REVIEW with status, changes, verification evidence, and deviations. Partial or imperfect REVIEW content proceeds to Review instead of terminating at Edge. Review alone receives the compact code-review system contract. It rereads PLAN/REVIEW, inspects the actual result, reruns applicable verification, repairs and reverifies in the same binding when needed, and owns the non-empty final output without rewriting REVIEW or creating a Result document. Exact artifact accesses update request-local state, ordinary caller tool calls pass through, and cleanup removes only `.iop/job/`. Provider operation/effort/tool normalization remains selected-profile driven and caller identity independent. 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. | @@ -296,7 +296,7 @@ sequenceDiagram - provider-pool model group은 capacity + priority + availability 기준으로 provider candidate를 먼저 선택하고, 선택된 provider가 OpenAI-compatible 호출 방식을 지원하면 raw tunnel passthrough로 dispatch한다. Ollama/native provider가 선택되면 normalized `RunRequest` path로 dispatch한다. - Anthropic Messages and count-tokens do not use legacy direct-route or single-target fallback. Native responses preserve provider status, allowed headers, and body/SSE bytes; bridge responses are converted between Anthropic Messages and Chat Completions shapes. - A marked single-request Messages dispatch requires the narrow service coordinator capability and never falls back to the generic provider pool. The handler copies the immutable binding and request input and counts the accepted HTTP admission once with no labels. The service projects exactly one frozen terminal candidate through both response modes: buffered/SSE `end_turn`; buffered/SSE `max_tokens` without private partial content; `invalid_request_error` for validation/context; `api_error` for provider, timeout, budget, repetition, malformed, internal-tool, and workspace-cleanup failures; or silent cancellation after caller disconnect. The streaming path maps only fixed plan/work/review/repair summaries, serializes pings and monotonic text-block indices with one terminal owner, stops and joins liveness before terminal/return, and acknowledges completion only after `message_stop`. Arbitrary progress, reasoning, tool/provider/credential/workspace data, raw failures, and internal stage terminals stay private. No classified terminal triggers retry, fallback, partial success, a second request, or a later success terminal. Count-tokens does not enter or increment this path. -- Marked single-request Plan/Review templates are Edge-owned internal artifact shapes, not part of this input surface. Admission freezes the effective pair; Edge renders deterministic PLAN `P1..Pn` IDs, Work writes one strict REVIEW handoff (item status, changes, verification, deviations), and Review rereads both artifacts without a memory worker payload or a final REVIEW write. No caller field, header, or metadata value can supply, name, select, or override a template, and no template path, content, or digest appears in a response, an error message, a log projection, or a metric label. Changing a template changes neither the Messages request schema nor the response schema: the buffered/SSE terminal projection is unchanged and the final caller-visible text is exactly the reviewer `decision.output` after any repair/re-verification. +- Marked single-request Plan/Review templates are Edge-owned internal artifact shapes, not part of this input surface. Admission freezes the effective pair; Edge renders PLAN `P1..Pn` IDs, Work writes a REVIEW handoff (item status, changes, verification, deviations), and Review rereads both artifacts without a memory worker payload or a final REVIEW write. Edge does not reparse PLAN or reject a partial REVIEW on semantic completeness before Review dispatch. No caller field, header, or metadata value can supply, name, select, or override a template, and no template path, content, or digest appears in a response, an error message, a log projection, or a metric label. Changing a template changes neither the Messages request schema nor the response schema: the buffered/SSE terminal projection is unchanged and the final caller-visible text is exactly the reviewer `decision.output` after any repair/re-verification. - Marked single-request observation evidence links ingress=1, request-total=1, terminal=1, stage/tool/cleanup counts, and raw-free correlation for one real POST. `iop_anthropic_single_request_ingress_total` is strictly unlabeled: no request_id, stage_id, provider identity, content, or workspace reference appears as a metric label. Internal tool names (`workspace_read`, `workspace_write`, etc.), raw arguments, private results, and workspace references are absent from the public terminal JSON and from log projections. Stage-pure timing, cardinality-bounded labels, and privacy semantics are documented here. SDD S12 qualifies the external Claude path on an approved IOP Node with one accepted ingress, the expected stage sequence, one terminal, exact output, timing, cleanup, and redacted evidence. - Internal workspace calls use a service-owned schema independent of caller-facing tool codecs. The five closed operation names decode into typed Node requests only after request/stage/tool identity, canonical relative path, approved operation/command/environment capability, and immutable budget checks. The loop opens once, preserves the admitted connection generation, executes one pending call at a time, accepts only correlated typed results, and returns a deep-copied raw-free result to the same executor continuation. Repeated IDs, stale responses, malformed or denied input, timeout, output/iteration exhaustion, and cancellation never become public Anthropic tool protocol or trigger a second ingress. - Claude Code Messages requests may use adaptive thinking, `output_config.effort`, structured output, cache-control annotations, and supported beta headers, including the compatibility-only `advisor-tool-2026-03-01` marker emitted by the pinned official caller. The Chat bridge consumes rather than forwards those headers, maps supported fields, and requires callers to replay opaque `tool_use.id` values unchanged so Gemini thought signatures can be restored on tool-result turns. @@ -362,8 +362,18 @@ sequenceDiagram ## 변경 기록 -- 2026-08-15: Normalized Chat output-token aliases by selected protocol profile across provider-pool and unmarked caller-workspace selector/Work/Review calls. OpenAI Chat uses `max_completion_tokens`, Gemini Chat uses `max_tokens`, target-native values win, and selector upstream HTTP 400 now terminates once as caller `invalid_request_error` instead of retryable 502. Command-mode workspace execution also carries the admitted containment guard in the actual outgoing command. +- 2026-08-15: Removed duplicate runtime semantic gates from Plan/Review handoffs. Edge now keeps workspace, non-empty write, and size boundaries while Reviewer owns completeness checks and repair of partial worker evidence. +- 2026-08-15: Corrected the unmarked caller-workspace hybrid responsibility contract. Planner and Reviewer now receive separate compact system contracts, Worker still receives only the absolute PLAN path plus the fixed sentence, Worker directly authors REVIEW evidence, and Review retains inspection/repair/re-verification without a Result artifact. + +- 2026-08-15: Normalized Chat output-token aliases by selected protocol profile across provider-pool and unmarked caller-workspace selector/Work/Review calls. OpenAI Chat uses `max_completion_tokens`, Gemini Chat uses `max_tokens`, target-native values win, and unmarked internal stages use the selected OpenAI profile's Responses operation when tools plus effort cannot be represented by Chat. The reasoning Responses conversion omits incompatible generic SDK `temperature`/`top_p` defaults. Selector upstream HTTP 400 now terminates once as caller `invalid_request_error` instead of retryable 502. Command-mode workspace execution also carries the admitted containment guard in the actual outgoing command. - 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-15: Replaced probabilistic parallel PLAN/REVIEW selector calls with one canonical atomic artifact-pair call that Edge deterministically expands into the same two admitted caller writes. +- 2026-08-15: Bound continuation issue identity to semantic tool calls so compatible SDK decoration of assistant messages does not break an otherwise exact caller receipt. +- 2026-08-15: Normalized Work/Review canonical reserved-artifact operation names through the selected caller binding before encoding them into structured or command-mode caller tools. +- 2026-08-15: Buffered command-mode Work/Review provider stages until canonical operation validation and caller-binding projection complete, preventing internal tool deltas from escaping before normalization while retaining structured-binding progressive streams. +- 2026-08-15: Removed the redundant outer identity projection from caller-stage-only cleanup so its issued semantic hash and caller-visible call ID remain identical. +- 2026-08-15: Canonicalized reserved Work/Review path aliases such as caller `filePath` back to the issued path field before command-mode binding encode. +- 2026-08-15: Added one Work-only canonical REVIEW handoff tool so completed control content is normalized into the caller binding instead of depending on an arbitrary caller edit or shell schema. - 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. @@ -404,6 +414,7 @@ sequenceDiagram - 2026-08-14: Applied the same provider-normalization boundary to private single-request Plan/Work/Review stages. Removed pre-selection Chat operation pinning, added Chat↔Responses stage conversion and nearest-lower effort mapping, and corrected default-selector dispatch validation so the pool-selected provider is accepted without weakening explicit provider, profile, credential, target, or execution-path fences. - 2026-08-14: Canonicalized private Chat Completions stage responses before strict Plan/Work/Review decoding. Standard OpenAI bookkeeping fields are stripped centrally, a non-null refusal remains fail-closed, and Responses/Chat now converge on the same private envelope instead of making stage codecs provider-specific. - 2026-08-14: Added provider `timings` to the bounded Chat response bookkeeping stripped before private stage decoding, aligning the common normalizer with the existing Ornith Work codec without admitting unknown response fields. +- 2026-08-15: Reduced the unmarked light Work input to the caller-workspace absolute PLAN path plus one fixed English sentence, limited Work to caller tools, moved completed REVIEW projection into Edge after the worker terminal report, and required English-ASCII PLAN content. - 2026-08-13: Gemini-native path parsing now keeps route IDs strict while accepting the bounded URL-encoded official caller model label `Gemini 3.6 Flash`. - 2026-08-15: Removed watchdog-specific OpenAI recovery and provider avoidance; existing terminal owners now determine request completion. - 2026-08-06: Added marked single-request Messages admission through the separate service coordinator capability, one unlabeled runtime ingress counter, buffered sanitized terminal acknowledgement, and deterministic real-POST compatibility evidence. diff --git a/agent-spec/runtime/edge-node-execution.md b/agent-spec/runtime/edge-node-execution.md index 55612b28..b5d27d14 100644 --- a/agent-spec/runtime/edge-node-execution.md +++ b/agent-spec/runtime/edge-node-execution.md @@ -216,8 +216,8 @@ The shared `packages/go/execution` package contains provider lifecycle, registry - The request-local internal tool loop is implemented between the coordinator and the dedicated workspace wire. Strict decode and capability checks happen before wire effects; Node results are accepted only for the one pending call and return only bounded typed fields to the same optional executor continuation. Repeated or stale identities, malformed/denied calls, exhausted immutable budgets, and cancellation terminate internally without selecting another Node or involving the HTTP caller. - Request-owned plan and review artifact access is implemented between the controller and the same dedicated workspace wire. Only `SingleRequestArtifactPlan` and `SingleRequestArtifactReview` are accepted. Artifact and model-tool callers share one serialized open attempt and the same opened cleanup gate; terminal and cancellation paths wait for in-flight artifact work before issuing exactly one cleanup. Edge bounds writes before dispatch and reads before acceptance, validates the echoed kind/operation and canonical terminal, and never reselects after a generation mismatch. Node maps the closed selectors to `plan.md` and `review.md`, validates the inventoried parent/file identity with descriptor-relative no-follow reads, and never grants the public workspace tool surface access to `.iop`. - The private Plan stage is installed in the composite single-request executor at Edge input startup (`apps/edge/internal/input/manager.go`). Its provider codec accepts only frozen Plan options and selected dispatch facts, uses the admitted stage deadline and exact output limit, and projects provider failures to the closed stage terminal. Before structured completion, it exposes only frozen-authority `workspace_read`/`workspace_list`, forces the first turn to use one of them, validates the canonical call, and resumes `planning` through the shared correlated continuation bridge. A failed result does not unlock completion, and the admitted per-stage iteration limit bounds further inspection. No write/delete/command capability or separate analysis artifact exists in Plan. After successful inspection the stage owns a closed strict JSON response schema with exactly a string `goal`, a string-array `steps`, and a string-array `verification`; unknown, duplicate, missing, or mistyped fields fail malformed. It enforces a single-line goal, 2-6 non-empty one-line step items, and 1-3 non-empty one-line verification items. Edge, rather than the provider, adds Markdown bullet prefixes and newlines before substituting the values into the frozen effective Plan template. Required headings remain exact standalone lines, the documented placeholder inventory is closed, and unresolved delimiters are rejected. Provider output therefore cannot vary headings, bullet formatting, or static template text, and caller request fields cannot select, supply, or override the admitted template. -- The private Work stage is installed in the composite single-request executor at Edge input startup (`apps/edge/internal/input/manager.go`). It reads and strictly validates `SingleRequestArtifactPlan`, retains only request/stage/tool identifiers while waiting for the coordinator-owned continuation, and sends no `reasoning_effort` field in an initial or resumed provider request. Its successful strict response is rendered once as `SingleRequestArtifactReview`; write failure prevents Review. -- Review reads and validates the stored PLAN and REVIEW handoff before its provider call. It may inspect, repair, and re-verify with admitted tools, but it neither takes a memory worker result nor writes a final REVIEW artifact. A repair mutation requires later successful inspection evidence before PASS; caller output is byte-for-byte the reviewer strict `output` field and cleanup removes the temporary artifacts. +- The private Work stage is installed in the composite single-request executor at Edge input startup (`apps/edge/internal/input/manager.go`). It reads a non-empty bounded `SingleRequestArtifactPlan`, retains only request/stage/tool identifiers while waiting for the coordinator-owned continuation, and sends no `reasoning_effort` field in an initial or resumed provider request. Its response is rendered once as `SingleRequestArtifactReview`; write failure prevents Review, while partial report fields remain Reviewer-owned evidence. +- Review reads the stored non-empty bounded PLAN and REVIEW handoff before its provider call. Edge does not reparse the PLAN or pre-judge REVIEW semantic completeness. Review may inspect, repair, and re-verify with admitted tools, but it neither takes a memory worker result nor writes a final REVIEW artifact. A repair mutation requires later successful inspection evidence before PASS; caller output is byte-for-byte the reviewer strict `output` field and cleanup removes the temporary artifacts. - The Node-private workspace request/result wire is implemented, including catalog delivery, parser registration, optional handler behavior, stable typed failures, generation-fenced dispatch, context-cancel propagation, and request cleanup. Before ready, a non-empty catalog requires a supported `darwin|linux` host and exact entry/host matching before any root open; unsupported and cross-platform catalogs fail closed while empty catalogs remain compatible. The Node installs the workspace handler before ready and cleans active requests before closing workspace authority ahead of session/store teardown. Request authority is immutable and request-local. File operations reserve `.iop`, reject symlink/mount/replaced-parent/special-file paths before effects, process bounded list batches with deterministic truncation, and use a same-parent structured write. Command execution resolves only admitted ids to fixed templates, enters the already-opened root descriptor through `fchdir`, provides only allowlisted environment entries, shares one output cap across drained stdout/stderr, and owns the complete process group through exit, timeout, context cancel, exact request/tool cancel, or request cleanup. - managed mode는 등록과 dispatch 전에 CA로 검증된 Edge/Node workload identity를 요구한다. - revoked, disabled, expired, stale, replayed, wrong-recipient, mismatched lease는 provider나 credential fallback 없이 fail closed한다. @@ -301,9 +301,9 @@ Heartbeat interval/wait는 protobuf field가 아닌 양쪽 transport 구현의 c - `go test -count=1 ./apps/edge/internal/openai -run 'TestAnthropicSingleRequestObservation'` — deterministic single-request observation evidence: ingress=1, request-total=1, terminal=1, stage/tool/cleanup counts, raw-free correlation, and unlabeled metric assertion. - `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ProviderStage|PlanStage)'` — deterministic frozen provider codec and Plan stage evidence, including required read/list-first inspection, mutation-tool exclusion, successful-result completion gate, high reasoning, the closed structured response schema, exact JSON field admission, ordered tunnel frames, deterministic frozen-template rendering, planning/internal-tool resume envelopes, and `plan.md` artifact selection. - `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ProviderStage|ProviderNormalization|PlanStage|WorkStage|ReviewStage|Executor)'` — private stage operation selection, Chat/Responses conversion, nearest-lower effort (`max` → `xhigh`), default-selector pool-provider acceptance, explicit dispatch fences, and composite regression evidence. -- `go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config` — exact standalone heading/`PASS` grammar, closed placeholder inventory, the `8192`-byte boundary, and relative-only `plan_file`/`review_file` loading with per-file fallback and filesystem-kind rejection. +- `go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config` — template rendering, closed placeholder inventory, the `8192`-byte boundary, and relative-only `plan_file`/`review_file` loading with per-file fallback and filesystem-kind rejection. - `go test -count=1 ./apps/edge/internal/service -run 'TestSingleRequestBinding'` and `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PresetBinding|ReviewStage)'` — admitted effective-template snapshot through clone and workspace revalidation, refresh isolation for admitted versus newly admitted requests, and the custom-template REVIEW artifact with unchanged caller-visible output. -- `go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestWork(Stage|ToolBridge)'` — deterministic ornith-fast Work tool loop, correlation isolation, cancellation cleanup, strict completion evidence, and Work reasoning-option absence. +- `go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestWork(Stage|ToolBridge)'` — deterministic Work tool loop, correlation isolation, cancellation cleanup, REVIEW rendering, and Work reasoning-option absence. - `go test -race ./apps/edge/internal/service ./apps/edge/internal/openai -run 'Test(SingleRequestTerminalDisposition|SingleRequestQualityGate|AnthropicSingleRequestErrorCancelMatrix|SingleRequestAnthropicStreamTerminalDisposition)' -count=1` — deterministic S11 error-cancel/length matrix, first-terminal ownership, one ingress, no second request, disconnect silence, and raw-free output evidence. ## 한계와 주의사항 @@ -318,6 +318,7 @@ Heartbeat interval/wait는 protobuf field가 아닌 양쪽 transport 구현의 c ## 변경 기록 +- 2026-08-15: Removed duplicate PLAN reparsing and REVIEW semantic pre-validation from Work/Review. Non-empty bounded artifacts proceed to Reviewer, which owns completeness checks and repair. - 2026-08-14: Added bounded read/list-first workspace analysis inside Plan. Structured Plan completion now requires a successful inspection result in the same request-local conversation; Plan exposes no mutation tool and creates no additional analysis artifact (`apps/edge/internal/openai/single_request_plan_stage.go`). - 2026-08-14: Restored artifact-only model handoff: deterministic PLAN `P1..Pn` IDs, one Work-authored validated REVIEW handoff, Review artifact reread with request-local repair/re-verification evidence, reviewer zero-write, and strict terminal `output` provenance. - 2026-08-14: Moved private Plan/Work/Review provider calls onto the shared provider-normalization boundary. Stage requirements now select Chat or Responses without caller identity, unsupported effort maps only downward, and default-selector provider-pool choices no longer fail the post-dispatch validation that still fences profile, target, credential revision, model group, and tunnel path. diff --git a/agent-task/hybrid_pipeline_contract/CODE_REVIEW-cloud-G07.md b/agent-task/hybrid_pipeline_contract/CODE_REVIEW-cloud-G07.md new file mode 100644 index 00000000..e76fe878 --- /dev/null +++ b/agent-task/hybrid_pipeline_contract/CODE_REVIEW-cloud-G07.md @@ -0,0 +1,84 @@ + + +# Code Review Reference - REFACTOR + +> Implementing agent: complete every implementation-owned section, run the Plan verification, and leave this active file in place. Review agent owns verdict and finalization. + +## Overview + +date=2026-08-15 +task=hybrid_pipeline_contract, plan=0, tag=REFACTOR + +## Implementation Item Completion + +| Item | Status | +|---|---| +| REFACTOR-1 Restore Planner Contract | completed | +| REFACTOR-2 Make Reviewer Contract System-Owned | completed | +| REFACTOR-3 Restore Worker-Owned Review Handoff | completed | +| REFACTOR-4 Synchronize Contract And Run Qualification | completed | + +## Implementation Checklist + +- [x] Add the compact Agent-Ops Plan contract to the Planner system instruction while retaining atomic Plan/Review pair output and bounded grammar. +- [x] Inject the compact Agent-Ops Review/Repair contract as Reviewer-only system guidance across normalized, Chat, and Messages stage bodies without changing Worker input. +- [x] Require Worker-authored Review evidence as the local-stage handoff and remove Edge synthesis from terminal prose. +- [x] Update focused tests, contracts, and current specs; run package, full Go, and live Gemini hybrid verification. +- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +## Deviations from Plan + +- Added `hot_path_review.go` and three existing cross-path fixture files to the exact write boundary because the `issueTools` signature and direct handoff semantics required their mechanical updates. +- `agent-spec/runtime/edge-node-execution.md` was not changed: its matching text describes the separate marked `single_request` Node-workspace path, while this change is limited to the unmarked caller-workspace light route. +- Live Gemini qualification remains after reviewed dev deployment as planned. + +## Key Design Decisions + +- Planner keeps the existing selector system-message injection and atomic pair tool; only its authored contract changed. +- Reviewer guidance is a separate system value on normalized input, a leading Chat system message, or Anthropic top-level system. Dynamic task/artifact/correlation data remains user-stage input. +- Worker prompt remains byte-for-byte `absolute PLAN path + fixed sentence` and receives no compact skill system prompt. +- A validated Worker Review tool call now supplies the local commit correlation. The receipt transitions directly to Review; terminal prose can no longer manufacture Review evidence. + +## Reviewer Checkpoints + +- Planner/Reviewer skill contracts are system-owned; Worker prompt remains exact and minimal. +- Worker writes Review evidence directly; Edge does not infer it from terminal prose. +- Reviewer reads artifacts and actual result, verifies, repairs if necessary, and reverifies in the same stage. +- Existing stage routing, paths, cleanup, and provider normalization remain intact. + +## Verification Results + +### Targeted Edge tests + +Command: `go test ./apps/edge/internal/openai -count=1` + +Result: PASS. + +Output: `ok iop/apps/edge/internal/openai 8.719s` + +### Full Go regression + +Command: `go test ./... -count=1` + +Result: PASS. + +All Go packages passed, including `apps/edge/internal/openai`, `apps/edge/internal/service`, Node workspace/transport, config, and `packages/go/singlerequesttemplate`. + +### Gemini hybrid qualification + +Result: PASS on deployed source `00761ba0`. + +- Caller/route: OpenCode `iop-gemini-preset/gemini-hybrid`. +- Planner/Reviewer: `gemini-3.6-flash`, high reasoning. Worker: `ornith:35b` for this qualification run. +- Request sent: `2026-08-15T01:57:33.389054Z`; terminal elapsed: `58.32s`; process exit: `0`. +- Worker read the absolute Plan path, created and reread caller-root `index.html`, and replaced the whole Review with completed item status and verification evidence. +- Reviewer reread `index.html`, Review, and Plan, inspected the caller workspace, found no defect requiring repair, and returned a verified terminal result. +- Output SHA-256: `0b5f184c9460efab924f6834593a0882b2b9c4b8a4fd9640662e1d71dc1d4368`; required doctype/title/h1 counts were `1/1/1`. +- Before and after the isolated run, `mac-gemini-api` and `onexplayer-lemonade` were healthy/available with `in_flight=0`, `queued=0`. +- No additional benchmark harness, retry, scoring layer, or gate was introduced. + +## Review-Only Checklist + +- [ ] Run applicable verification and inspect actual changed files. +- [ ] Append PASS/WARN/FAIL with findings, root cause, and selected fix when needed. +- [ ] Finalize the Agent-Ops task state according to the code-review skill. diff --git a/agent-task/hybrid_pipeline_contract/PLAN-local-G07.md b/agent-task/hybrid_pipeline_contract/PLAN-local-G07.md new file mode 100644 index 00000000..291710d5 --- /dev/null +++ b/agent-task/hybrid_pipeline_contract/PLAN-local-G07.md @@ -0,0 +1,196 @@ + + +# Lightweight Hybrid Plan/Review Contract + +## For the Implementing Agent + +Follow this plan as written. Run every listed verification command and fill the implementation-owned sections in `CODE_REVIEW-cloud-G07.md` with actual changes and command output. Keep both active files in place and report ready for review. If blocked, record the exact blocker, attempted command/output, and resume condition only in the review evidence; do not archive task files or create another control-plane state. + +## Background + +The caller-workspace light pipeline already has selector, local worker, review, repair, and cleanup stages. Its current prompts and handoff ownership diverge from the intended compact Agent-Ops Plan/Review workflow: Plan is forced into a deterministic seed, Reviewer guidance is carried as ordinary stage text, and Edge can synthesize Review evidence from a worker terminal summary. This task preserves the state machine and corrects only those internal contracts before rerunning the Gemini hybrid benchmark. + +## Analysis + +### Files Read + +- `agent-ops/skills/common/plan/SKILL.md` +- `agent-ops/skills/common/code-review/SKILL.md` +- `agent-ops/skills/common/finalize-task-routing/SKILL.md` +- `agent-ops/skills/common/plan/templates/review-stub-template.md` +- `agent-ops/rules/project/rules.md` +- `agent-ops/rules/project/domain/edge/rules.md` +- `agent-ops/rules/project/domain/platform-common/rules.md` +- `agent-ops/rules/project/domain/testing/rules.md` +- `agent-test/local/rules.md` +- `agent-contract/index.md` +- `agent-contract/outer/anthropic-compatible-api.md` +- `agent-contract/outer/openai-compatible-api.md` +- `agent-spec/index.md` +- `agent-spec/runtime/edge-node-execution.md` +- `agent-spec/input/openai-compatible-surface.md` +- `apps/edge/internal/openai/provider_model_rewrite.go` +- `apps/edge/internal/openai/hot_path_stage_input.go` +- `apps/edge/internal/openai/hot_path_dispatch.go` +- `apps/edge/internal/openai/hot_path_light.go` +- `apps/edge/internal/openai/hot_path_review.go` +- `apps/edge/internal/openai/artifact_pair.go` +- `packages/go/singlerequesttemplate/template.go` +- `apps/edge/internal/openai/provider_model_rewrite_test.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_anthropic_gate_test.go` +- `apps/edge/internal/openai/hot_path_chat_gate_test.go` +- `apps/edge/internal/openai/hot_path_observation_test.go` + +### SDD Criteria + +Not applicable. This is a non-Milestone correction to an existing runtime contract. + +### Verification Context + +No external handoff was supplied. Repository source, existing hot-path tests, outer contracts, and current Gemini hybrid qualification requirements are the evidence sources. Unit verification uses the Edge package and full Go suite; live verification uses the existing dev deployment and the same Gemini hybrid benchmark path after implementation review. Live credentials and endpoints remain external runtime inputs and must not be written to tracked files or test output. + +### Test Coverage Gaps + +- Existing selector tests assert the deterministic two-step seed but do not assert compact Plan-skill analysis/acceptance/handoff instructions. +- Existing stage-input tests assert Worker prompt isolation but do not prove Reviewer guidance is a system message on both Chat and Messages wires. +- Existing light-flow fixtures exercise Edge-projected worker Review completion; they must instead prove the Worker-authored Review write is the local-stage terminal handoff. +- Existing review/repair tests cover artifact reads and repair transitions and should remain valid after system-prompt injection. + +### Symbol References + +- `projectWorkerCompletionReview` and `boundedWorkerCompletionSummary` are local to `hot_path_light.go`; removing them has no external call sites. +- `pendingLocalCommit` is used only by the local-handoff transition and can be populated from the Worker Review tool-call dispatch correlation instead of a synthesized terminal projection. +- `hotPathChatStageMessages`, `hotPathAnthropicStageBody`, and `hotPathStageRunInput` are the three internal provider shapes that must carry Reviewer-only system guidance without changing Worker input. + +### Split Judgment + +Keep one plan. Planner guidance, Worker handoff ownership, and Reviewer guidance form one cross-stage correctness invariant; splitting would allow an intermediate state in which artifacts or transitions no longer agree. + +### Scope Rationale + +Do not redesign execution presets, stage routing, provider normalization, artifact paths, cleanup, benchmark scoring, or the shared Plan/Review Markdown parser. Do not inject a Plan/Review skill prompt into Worker. Do not add a Result artifact. The current lightweight Plan/Review grammar remains bounded; only its authored content and stage ownership are corrected. + +### Final Routing + +- evaluation_mode: `first-pass` +- finalizer: `finalize-task-policy.sh`, mode `pair` +- build: closures all true; scores `scope=1,state=2,blast=1,evidence=1,verification=2`; `local-fit`, `local/G07`, `PLAN-local-G07.md` +- review: closures all true; scores `scope=1,state=2,blast=1,evidence=1,verification=2`; `official-review`, `cloud/G07`, `CODE_REVIEW-cloud-G07.md` +- large_indivisible_context: `false` +- matched_loop_risk_signatures: `temporal_state`, `boundary_contract`, `structured_interpretation` (count 3) +- review_rework_count: `0` +- evidence_integrity_failure: `false` + +## Implementation Checklist + +- [ ] Add the compact Agent-Ops Plan contract to the Planner system instruction while retaining atomic Plan/Review pair output and bounded grammar. +- [ ] Inject the compact Agent-Ops Review/Repair contract as Reviewer-only system guidance across normalized, Chat, and Messages stage bodies without changing Worker input. +- [ ] Require Worker-authored Review evidence as the local-stage handoff and remove Edge synthesis from terminal prose. +- [ ] Update focused tests, contracts, and current specs; run package, full Go, and live Gemini hybrid verification. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +### [REFACTOR-1] Restore Planner Contract + +**Problem:** `apps/edge/internal/openai/provider_model_rewrite.go:36` tells the selector to replace a deterministic two-step seed, so explicit requirements and acceptance constraints can be lost before Work. + +**Solution:** Keep the existing selector system-message injection and atomic pair tool. Replace the seed-oriented prose with a concise Plan-skill contract: analyze the immutable task before authoring, preserve all explicit requirements/constraints/acceptance, choose 2-6 closed executable steps, specify deterministic verification, and make the final step name the exact Review path and Worker evidence responsibility. Generate the pending Review status inventory for exactly the chosen P1..Pn ids. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/provider_model_rewrite.go`: replace deterministic seed guidance with the compact Plan system contract. +- [ ] `apps/edge/internal/openai/provider_model_rewrite_test.go`: assert analysis/acceptance/handoff guidance and bounded atomic output. +- [ ] `apps/edge/internal/openai/hot_path_direct_test.go`: update the selector fixture's required compact-contract fragments. + +**Test Strategy:** Update the existing selector instruction/canonical tool tests; no new harness. + +**Verification:** `go test ./apps/edge/internal/openai -run 'TestHotPathSelector|TestProvider' -count=1` passes. + +### [REFACTOR-2] Make Reviewer Contract System-Owned + +**Problem:** `apps/edge/internal/openai/hot_path_stage_input.go:140` embeds review behavior in the ordinary stage prompt, while Chat and Messages provider bodies have no Reviewer-specific system contract. + +**Solution:** Add one concise Reviewer system contract derived from the code-review skill: read Plan and worker Review, inspect actual output, rerun verification, establish findings/root cause/one fix, repair and reverify in the same stage, and return the final result without a separate Result document. Inject it only for Reviewer in normalized input, Chat system messages, and Anthropic top-level system. Keep dynamic task/artifact/correlation data in the ordinary prompt. Leave the local Worker prompt byte-for-byte unchanged. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/hot_path_stage_input.go`: define Reviewer system guidance and reduce dynamic prompt to stage data/action. +- [ ] `apps/edge/internal/openai/hot_path_dispatch.go`: inject Reviewer-only system guidance into all internal provider request shapes. +- [ ] `apps/edge/internal/openai/hot_path_light_test.go`: prove system/user separation and unchanged Worker prompt for Chat and Messages. + +**Test Strategy:** Extend existing stage-input and captured tunnel-body assertions. + +**Verification:** `go test ./apps/edge/internal/openai -run 'TestHotPathStageInput|TestHotPathLight' -count=1` passes. + +### [REFACTOR-3] Restore Worker-Owned Review Handoff + +**Problem:** `apps/edge/internal/openai/hot_path_light.go:891` converts Worker terminal prose into completed Review fields, making Edge—not Worker—the evidence author and losing actual commands/deviations. + +**Solution:** Remove terminal-summary Review projection. When the Worker emits the exact validated Review write after reading Plan, retain that provider dispatch correlation as the local commit; after the caller receipts the write, transition directly to Review. A terminal without a successful Plan read and Worker-authored Review remains a validation failure. Preserve ordinary caller tool pass-through and existing Review/Repair transitions. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/hot_path_light.go`: commit direct Worker Review handoff and remove synthesized Review projection. +- [ ] `apps/edge/internal/openai/hot_path_review.go`: pass no Worker commit correlation through Reviewer tool dispatch. +- [ ] `apps/edge/internal/openai/hot_path_light_test.go`: update fixtures and add regression coverage for missing/malformed Worker evidence. +- [ ] `apps/edge/internal/openai/hot_path_anthropic_gate_test.go`: make the streaming fixture use a Worker-authored Review handoff. +- [ ] `apps/edge/internal/openai/hot_path_chat_gate_test.go`: keep mixed-provider coverage with a Worker-authored Review handoff. +- [ ] `apps/edge/internal/openai/hot_path_observation_test.go`: record the Review handoff as the local stage's final tool turn. + +**Test Strategy:** Use existing scripted OpenAI and Anthropic fixtures with direct Worker Review writes; assert no local terminal summary is needed and malformed/missing handoff fails. + +**Verification:** `go test ./apps/edge/internal/openai -run 'TestHotPathLight|TestWorker' -count=1` passes. + +### [REFACTOR-4] Synchronize Contract And Run Qualification + +**Problem:** Outer contracts/specs currently state that Edge renders completed Review from a Worker report, which would become stale after ownership is corrected. + +**Solution:** Update only the affected current contract/spec statements. After code review and deployment, run the same Gemini hybrid benchmark in an isolated execution and capture request/start/completion timing, artifacts, actual output, review/repair behavior, and terminal result without secrets. + +**Modified Files and Checklist:** + +- [ ] `agent-contract/outer/anthropic-compatible-api.md`: record Planner/Reviewer system contracts and Worker-authored Review handoff. +- [ ] `agent-contract/outer/openai-compatible-api.md`: update the unmarked caller-workspace light route. +- [ ] `agent-spec/runtime/edge-node-execution.md`: synchronize the current hybrid stage responsibility summary if it contains the affected behavior. +- [ ] `agent-spec/input/openai-compatible-surface.md`: synchronize the caller-workspace hybrid behavior. +- [ ] `agent-task/hybrid_pipeline_contract/CODE_REVIEW-cloud-G07.md`: record actual implementation and verification evidence. + +**Test Strategy:** Targeted and full Go regression, then existing live Gemini hybrid benchmark only; do not create another benchmark harness. + +**Verification:** `go test ./... -count=1` passes, deployment health is confirmed, and one isolated Gemini hybrid execution produces worker-authored Review evidence plus a reviewer-verified final workspace result. + +## Modified Files Summary + +| File | Items | +|---|---| +| `apps/edge/internal/openai/provider_model_rewrite.go` | REFACTOR-1 | +| `apps/edge/internal/openai/provider_model_rewrite_test.go` | REFACTOR-1 | +| `apps/edge/internal/openai/hot_path_direct_test.go` | REFACTOR-1 | +| `apps/edge/internal/openai/hot_path_stage_input.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_dispatch.go` | REFACTOR-2 | +| `apps/edge/internal/openai/hot_path_light.go` | REFACTOR-3 | +| `apps/edge/internal/openai/hot_path_review.go` | REFACTOR-3 | +| `apps/edge/internal/openai/hot_path_light_test.go` | REFACTOR-2, REFACTOR-3 | +| `apps/edge/internal/openai/hot_path_anthropic_gate_test.go` | REFACTOR-3 | +| `apps/edge/internal/openai/hot_path_chat_gate_test.go` | REFACTOR-3 | +| `apps/edge/internal/openai/hot_path_observation_test.go` | REFACTOR-3 | +| `agent-contract/outer/anthropic-compatible-api.md` | REFACTOR-4 | +| `agent-contract/outer/openai-compatible-api.md` | REFACTOR-4 | +| `agent-spec/runtime/edge-node-execution.md` | REFACTOR-4 | +| `agent-spec/input/openai-compatible-surface.md` | REFACTOR-4 | +| `agent-task/hybrid_pipeline_contract/CODE_REVIEW-cloud-G07.md` | REFACTOR-4 | + +## Dependencies and Execution Order + +REFACTOR-1 and REFACTOR-2 establish the prompt contracts. REFACTOR-3 changes the matching Worker handoff transition. REFACTOR-4 follows the implemented behavior and must not document unverified semantics. + +## Final Verification + +1. `gofmt -w apps/edge/internal/openai/provider_model_rewrite.go apps/edge/internal/openai/provider_model_rewrite_test.go apps/edge/internal/openai/hot_path_stage_input.go apps/edge/internal/openai/hot_path_dispatch.go apps/edge/internal/openai/hot_path_light.go apps/edge/internal/openai/hot_path_light_test.go` +2. `go test ./apps/edge/internal/openai -count=1` +3. `go test ./... -count=1` +4. Deploy the reviewed build to the existing dev environment using the project deployment skill. +5. Run one isolated Gemini hybrid benchmark through the existing path and verify the absolute Plan-only Worker prompt, Worker-authored Review evidence, actual workspace result, Reviewer inspection/repair if needed, one terminal, and cleanup. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/apps/edge/internal/openai/artifact_pair.go b/apps/edge/internal/openai/artifact_pair.go index be93a13b..61989c44 100644 --- a/apps/edge/internal/openai/artifact_pair.go +++ b/apps/edge/internal/openai/artifact_pair.go @@ -7,8 +7,6 @@ import ( "net/http" "strings" "sync" - - "iop/packages/go/singlerequesttemplate" ) const defaultArtifactFrontierCapacity = 1024 @@ -41,7 +39,6 @@ const ( type artifactDisposition struct { Kind artifactDispositionKind SelectorStageID string - PlanItemIDs []string PrimaryError *hotPathEndpointError } @@ -85,7 +82,6 @@ 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 @@ -210,6 +206,91 @@ func (s *artifactFrontierStore) pairRequired(requestID, ownerEdgeID string) bool return record != nil && record.ownerEdgeID == ownerEdgeID && record.phase == artifactPhasePairReady } +// expandSelectorPair turns the selector-only atomic pair tool into the two +// caller-visible workspace writes. The provider never needs to coordinate two +// parallel calls, while the caller still executes and receipts each file +// independently through its admitted workspace tool contract. +func (s *artifactFrontierStore) expandSelectorPair( + requestID, ownerEdgeID string, + output normalizedStageOutput, +) (normalizedStageOutput, error) { + if s == nil || requestID == "" { + return output, nil + } + s.mu.Lock() + defer s.mu.Unlock() + record := s.records[requestID] + if record == nil || record.ownerEdgeID != ownerEdgeID || + (record.phase != artifactPhasePairReady && (record.phase != artifactPhasePinned || !record.binding.createsParents())) { + return output, nil + } + if len(output.ToolCalls) != 1 || output.ToolCalls[0].Name != hotPathArtifactPairToolName { + return output, nil + } + write := record.binding.operation(opKindWrite) + if write == nil || strings.TrimSpace(write.toolName) == "" { + return normalizedStageOutput{}, fmt.Errorf("artifact pair write binding is unavailable") + } + call := output.ToolCalls[0] + planContent, planOK := call.Arguments["plan_content"].(string) + reviewContent, reviewOK := call.Arguments["review_content"].(string) + if !planOK || strings.TrimSpace(planContent) == "" || !reviewOK || strings.TrimSpace(reviewContent) == "" { + return normalizedStageOutput{}, fmt.Errorf("artifact pair tool requires complete plan_content and review_content") + } + planContent = normalizeCallerWorkspacePlanBoundary(planContent) + providerID := strings.TrimSpace(call.ProviderCallID) + if providerID == "" { + providerID = strings.TrimSpace(call.ID) + } + // The selector stage ends after its atomic pair is expanded into caller + // workspace writes, so its Gemini thought signature is never replayed to + // the selector provider. Keep only the provider's original tool id before + // deriving the two internal write ids; the opaque signature envelope can be + // larger than the logical-request id contract permits. + if decodedID, _, encoded, err := decodeGeminiThoughtSignatureToolID(providerID); err != nil { + return normalizedStageOutput{}, err + } else if encoded { + providerID = decodedID + } + if !validLogicalRequestID(providerID) { + return normalizedStageOutput{}, fmt.Errorf("artifact pair provider tool id is invalid") + } + paths := newReservedPaths(requestID) + output.ToolCalls = []normalizedToolCall{ + {ID: pairProviderCallID(providerID, "plan"), ProviderCallID: pairProviderCallID(providerID, "plan"), Name: write.toolName, Arguments: map[string]any{"path": paths.PlanPath, "content": planContent}}, + {ID: pairProviderCallID(providerID, "review"), ProviderCallID: pairProviderCallID(providerID, "review"), Name: write.toolName, Arguments: map[string]any{"path": paths.ReviewPath, "content": reviewContent}}, + } + return output, nil +} + +func normalizeCallerWorkspacePlanBoundary(content string) string { + const marker = "## Goal\n" + start := strings.Index(content, marker) + if start < 0 { + return content + } + goalStart := start + len(marker) + goalEnd := strings.IndexByte(content[goalStart:], '\n') + if goalEnd < 0 { + goalEnd = len(content) + } else { + goalEnd += goalStart + } + goal := strings.TrimSpace(content[goalStart:goalEnd]) + if strings.HasPrefix(goal, hotPathCallerWorkspacePlanBoundary) { + return content + } + return content[:goalStart] + hotPathCallerWorkspacePlanBoundary + " " + goal + content[goalEnd:] +} + +func pairProviderCallID(base, role string) string { + suffix := "_" + role + if len(base)+len(suffix) > 256 { + base = base[:256-len(suffix)] + } + return base + suffix +} + func (s *artifactFrontierStore) issue( turn *hotPathTurn, output normalizedStageOutput, @@ -333,7 +414,6 @@ 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 { @@ -343,30 +423,15 @@ 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 { + if _, err := artifactWriteContent(record.binding, call); err != nil { return normalizedStageOutput{}, nil, err } + // Plan and Review contents are model-to-model handoff artifacts. The + // workspace boundary already requires a non-empty write; their semantic + // completeness belongs to the Worker and Reviewer, not the transport. mappedCalls = append(mappedCalls, mapped) payloads[mapped.ID] = payload } - record.planItemIDs = append([]string(nil), planItemIDs...) return artifactResponseOutput(output, mappedCalls), payloads, nil } @@ -389,36 +454,6 @@ func artifactWriteContent(binding *workspaceBinding, call normalizedToolCall) (s 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, @@ -618,7 +653,6 @@ 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 } @@ -633,10 +667,7 @@ 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, - PlanItemIDs: append([]string(nil), record.planItemIDs...), - }, true, nil + return snap, artifactDisposition{Kind: artifactDispositionLocalEligible, SelectorStageID: record.selectorStageID}, true, nil default: return logicalRequestSnapshot{}, artifactDisposition{}, true, fmt.Errorf("artifact frontier phase cannot consume results") } @@ -672,8 +703,11 @@ func (s *artifactFrontierStore) matchRecordLocked( if record.principalRef != principalRef { return nil, true, errLogicalRequestPrincipal } - if record.protocol != protocol || record.lineage != lineage.Prefix { - return nil, true, errLogicalRequestLineage + if record.protocol != protocol { + return nil, true, fmt.Errorf("%w: protocol changed", errLogicalRequestLineage) + } + if record.lineage != lineage.Prefix { + return nil, true, describeArtifactPrefixMismatch(record.lineage, lineage.Prefix) } return record, true, nil } @@ -682,9 +716,35 @@ func (s *artifactFrontierStore) matchRecordLocked( return record, true, nil } } + for _, record := range candidates { + if record.ownerEdgeID != ownerEdgeID || record.principalRef != principalRef || record.protocol != protocol { + continue + } + if record.lineage != lineage.Prefix { + return nil, true, describeArtifactPrefixMismatch(record.lineage, lineage.Prefix) + } + if record.pendingHash != lineage.IssuedCallHash { + return nil, true, fmt.Errorf("%w: issued tool calls changed", errLogicalRequestLineage) + } + } return nil, true, errLogicalRequestLineage } +func describeArtifactPrefixMismatch(want, got logicalRequestLineage) error { + switch { + case want.Endpoint != got.Endpoint: + return fmt.Errorf("%w: endpoint changed", errLogicalRequestLineage) + case want.HistoryDigest != got.HistoryDigest && want.ToolsetDigest != got.ToolsetDigest: + return fmt.Errorf("%w: request history and toolset changed", errLogicalRequestLineage) + case want.HistoryDigest != got.HistoryDigest: + return fmt.Errorf("%w: request history changed", errLogicalRequestLineage) + case want.ToolsetDigest != got.ToolsetDigest: + return fmt.Errorf("%w: toolset changed", errLogicalRequestLineage) + default: + return errLogicalRequestLineage + } +} + func artifactIDsIntersect(record *artifactFrontierRecord, ids []string) bool { for _, id := range ids { if record.pending[id] != nil { diff --git a/apps/edge/internal/openai/artifact_pair_test.go b/apps/edge/internal/openai/artifact_pair_test.go index 1f03badb..f89c30cf 100644 --- a/apps/edge/internal/openai/artifact_pair_test.go +++ b/apps/edge/internal/openai/artifact_pair_test.go @@ -27,9 +27,24 @@ func testLightweightPlan(t *testing.T) string { return string(plan) } +func TestNormalizeCallerWorkspacePlanBoundary(t *testing.T) { + plan := testLightweightPlan(t) + normalized := normalizeCallerWorkspacePlanBoundary(plan) + want := "## Goal\n" + hotPathCallerWorkspacePlanBoundary + " Complete the caller workspace task" + if !strings.Contains(normalized, want) { + t.Fatalf("normalized plan omitted workspace boundary: %s", normalized) + } + if second := normalizeCallerWorkspacePlanBoundary(normalized); second != normalized { + t.Fatalf("workspace boundary normalization is not idempotent: %s", second) + } +} + func testPendingReview(t *testing.T) string { t.Helper() - review, err := pendingReviewSeed([]string{"P1", "P2"}) + review, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, singlerequesttemplate.ReviewFields{ + ItemStatus: "- P1: pending\n- P2: pending", Changes: "Pending worker execution.", + Verification: "Pending worker verification.", Deviations: "None recorded.", + }, singlerequesttemplate.MaxTemplateBytes) if err != nil { t.Fatal(err) } @@ -223,20 +238,6 @@ 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}) { @@ -320,6 +321,46 @@ func newArtifactPairFixture(t *testing.T, endpoint string, createsParents bool) } } +func TestArtifactSelectorAtomicPairExpandsToCallerWrites(t *testing.T) { + fixture := newArtifactPairFixture(t, "openai", true) + output, err := fixture.server.artifactFrontiers.expandSelectorPair(fixture.requestID, fixture.ownerEdgeID, normalizedStageOutput{ + ToolCalls: []normalizedToolCall{{ + ID: "provider_pair", Name: hotPathArtifactPairToolName, + Arguments: map[string]any{"plan_content": testLightweightPlan(t), "review_content": testPendingReview(t)}, + }}, + }) + if err != nil { + t.Fatal(err) + } + if len(output.ToolCalls) != 2 { + t.Fatalf("expanded calls=%+v", output.ToolCalls) + } + if output.ToolCalls[0].Name != "workspace" || output.ToolCalls[0].Arguments["path"] != fixture.paths.PlanPath || + output.ToolCalls[1].Name != "workspace" || output.ToolCalls[1].Arguments["path"] != fixture.paths.ReviewPath { + t.Fatalf("expanded caller writes=%+v", output.ToolCalls) + } + if output.ToolCalls[0].ProviderCallID == output.ToolCalls[1].ProviderCallID { + t.Fatalf("expanded provider ids must be distinct: %+v", output.ToolCalls) + } +} + +func TestArtifactSelectorAtomicPairDropsGeminiThoughtSignatureEnvelope(t *testing.T) { + fixture := newArtifactPairFixture(t, "openai", true) + providerID := encodeGeminiThoughtSignatureToolID("provider_pair", strings.Repeat("signature", 64)) + output, err := fixture.server.artifactFrontiers.expandSelectorPair(fixture.requestID, fixture.ownerEdgeID, normalizedStageOutput{ + ToolCalls: []normalizedToolCall{{ + ID: providerID, ProviderCallID: providerID, Name: hotPathArtifactPairToolName, + Arguments: map[string]any{"plan_content": testLightweightPlan(t), "review_content": testPendingReview(t)}, + }}, + }) + if err != nil { + t.Fatal(err) + } + if len(output.ToolCalls) != 2 || output.ToolCalls[0].ProviderCallID != "provider_pair_plan" || output.ToolCalls[1].ProviderCallID != "provider_pair_review" { + t.Fatalf("expanded provider ids=%+v", output.ToolCalls) + } +} + func (f *artifactPairFixture) issuePrepare() []string { f.t.Helper() ids, err := f.issue([]normalizedToolCall{{ diff --git a/apps/edge/internal/openai/chat_handler.go b/apps/edge/internal/openai/chat_handler.go index 256316e6..78ba1ca2 100644 --- a/apps/edge/internal/openai/chat_handler.go +++ b/apps/edge/internal/openai/chat_handler.go @@ -393,6 +393,25 @@ func (s *Server) handleChatCompletionsProviderPool(w http.ResponseWriter, dc *ch if err != nil { return tunnelReq, err } + if strings.TrimSpace(selectorInstruction) != "" { + prepared, err = prepareHotPathSelectorProviderInstruction(prepared, selectorInstruction) + if err != nil { + return tunnelReq, err + } + prepared, err = prepareHotPathSelectorOutputLimit(prepared) + if err != nil { + return tunnelReq, err + } + prepared, err = prepareHotPathSelectorCanonicalTools(prepared, selectorInstruction, dc.route.Preset) + if err != nil { + return tunnelReq, err + } + requirements := chatProviderRequirements(req) + if effort, ok := dc.route.Preset.Selector.Options["reasoning_effort"].(string); ok && strings.TrimSpace(effort) != "" { + requirements.Effort = strings.TrimSpace(effort) + } + return prepareHotPathChatProviderOperation(prepared, selected, requirements) + } prepared, err = prepareProviderChatRequestNormalization(prepared, selected) if err != nil { return tunnelReq, err @@ -401,7 +420,7 @@ func (s *Server) handleChatCompletionsProviderPool(w http.ResponseWriter, dc *ch if err != nil { return tunnelReq, err } - return prepareHotPathSelectorProviderInstruction(prepared, selectorInstruction) + return prepared, nil } // 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 a3b90e32..78cf3abc 100644 --- a/apps/edge/internal/openai/hot_path_anthropic_gate_test.go +++ b/apps/edge/internal/openai/hot_path_anthropic_gate_test.go @@ -152,8 +152,8 @@ func TestHotPathAnthropicDirectStreamPreservesEmptyToolInput(t *testing.T) { func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { fixture := newScriptedLightFixture(t, "anthropic", false) - 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, 5, 3) } fixture.service.responses[5] = func(requestID string) string { return scriptedReviewReadsAndInspectionWithUsage(requestID, 7, 4) @@ -165,29 +165,28 @@ 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}`}) + localWork := fixture.request() + fixture.consumeToolResponse(localWork, []string{`{"ok":true}`}) + workerReview := fixture.request() + fixture.consumeToolResponse(workerReview, []string{`{"written":true}`}) before := len(fixture.service.snapshots()) response := fixture.requestWithOptions(64, true) if response.Code != http.StatusOK { t.Fatalf("status=%d body=%s", response.Code, response.Body.String()) } - if got := len(fixture.service.snapshots()) - before; got != 2 { - t.Fatalf("same-turn provider stages=%d, want 2", got) + if got := len(fixture.service.snapshots()) - before; got != 1 { + t.Fatalf("same-turn provider stages=%d, want 1", got) } requests := fixture.service.snapshots() - assertCapturedHotPathBudget(t, requests[len(requests)-2], fixture.service.candidate, 64) - assertCapturedHotPathBudget(t, requests[len(requests)-1], fixture.service.candidate, 61) + assertCapturedHotPathBudget(t, requests[len(requests)-1], fixture.service.candidate, 64) events := decodeHotPathAnthropicSSE(t, response.Body.String()) - assertHotPathAnthropicBlockIndexes(t, events, 7) + assertHotPathAnthropicBlockIndexes(t, events, 5) wantNames := []string{ "message_start", "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_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", @@ -198,11 +197,11 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { } startMessage := hotPathAnthropicMap(t, events[0].payload["message"]) requestID, snapshot := soleHotPathSnapshot(t, fixture.server) - if startMessage["id"] != "msg-light-complete" || startMessage["id"] == requestID || startMessage["model"] != "virtual-model" { + if startMessage["id"] != "msg-review-inspection" || startMessage["id"] == requestID || startMessage["model"] != "virtual-model" { t.Fatalf("outer identity mismatch: message=%+v logical_request=%s", startMessage, requestID) } - wantKinds := []string{"thinking", "text", "thinking", "text", "tool_use", "tool_use", "tool_use"} + wantKinds := []string{"thinking", "text", "tool_use", "tool_use", "tool_use"} var gotKinds, thinking, text []string var toolIDs, toolNames, toolArgs []string var stopReason string @@ -229,13 +228,13 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { delta := hotPathAnthropicMap(t, event.payload["delta"]) stopReason, _ = delta["stop_reason"].(string) usage := hotPathAnthropicMap(t, event.payload["usage"]) - if usage["input_tokens"] != float64(12) || usage["output_tokens"] != float64(7) { - t.Fatalf("aggregate usage=%+v, want input=12 output=7", usage) + if usage["input_tokens"] != float64(7) || usage["output_tokens"] != float64(4) { + t.Fatalf("review usage=%+v, want input=7 output=4", usage) } } } if strings.Join(gotKinds, ",") != strings.Join(wantKinds, ",") || - strings.Join(thinking, "") != "local-reasonreview-reason" || strings.Join(text, "") != "local-visiblereview-visible" || + strings.Join(thinking, "") != "review-reason" || strings.Join(text, "") != "review-visible" || 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, toolIDs, toolNames, toolArgs, stopReason, response.Body.String()) @@ -243,8 +242,8 @@ func TestHotPathAnthropicLightStreamAggregatesStages(t *testing.T) { if len(snapshot.ExpectedCallIDs) != 3 { t.Fatalf("tool correlation mismatch: tools=%q snapshot=%+v", toolIDs, snapshot) } - 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()) + if strings.Join(toolIDs, ",") != "msg-review-inspection-tool-1,msg-review-inspection-tool-2,msg-review-inspection-tool-3" { + t.Fatalf("public tool namespace mismatch: 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 c370ca40..bedc9381 100644 --- a/apps/edge/internal/openai/hot_path_chat_gate_test.go +++ b/apps/edge/internal/openai/hot_path_chat_gate_test.go @@ -152,8 +152,8 @@ 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: openAICandidate, body: func(string) string { return scriptedLocalWorkTool("openai") }}, + {candidate: openAICandidate, body: func(requestID string) string { return scriptedReviewWrite("openai", requestID) }}, {candidate: anthropicCandidate, body: func(requestID string) string { return scriptedReviewReadsAndInspectionWithUsage(requestID, 7, 3) }}, } @@ -173,7 +173,7 @@ func TestHotPathChatMixedProviderStages(t *testing.T) { }) tools := scriptedLightTools("openai") - history := []any{map[string]any{"role": "user", "content": "mixed provider task"}} + history := []any{map[string]any{"role": "user", "content": "Working directory: /workspace\nmixed provider task"}} consume := func(response *httptest.ResponseRecorder, results []string) { t.Helper() assistant, ids, err := artifactAssistantFromResponse("openai", response.Body.Bytes()) @@ -192,7 +192,8 @@ 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}`}) + consume(request(false), []string{`{"ok":true}`}) + consume(request(false), []string{`{"written":true}`}) response := request(true) if response.Code != http.StatusOK { t.Fatalf("status=%d body=%s", response.Code, response.Body.String()) @@ -203,16 +204,16 @@ 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-reason", - Kinds: []string{"content", "reasoning", "content", "reasoning", "content", "tool", "tool", "tool", "terminal"}, - FinishReason: "tool_calls", PromptTokens: 12, CompletionTokens: 7, + ResponseID: "msg-review-inspection", Model: "virtual-model", Content: "review-visible", Reasoning: "review-reason", + Kinds: []string{"reasoning", "content", "tool", "tool", "tool", "terminal"}, + FinishReason: "tool_calls", PromptTokens: 7, CompletionTokens: 3, }) for _, chunk := range chunks { if chunk["id"] == requestID { t.Fatalf("logical request identity became the public response id: %+v", chunk) } } - for _, internalID := range []string{snapshot.ActiveStageID, "run-chat-gate-5", "run-chat-gate-6", "msg-review-inspection"} { + for _, internalID := range []string{snapshot.ActiveStageID, "run-chat-gate-5", "run-chat-gate-6"} { if strings.Contains(response.Body.String(), internalID) { t.Fatalf("internal or later-stage identity %q leaked: %s", internalID, response.Body.String()) } @@ -294,7 +295,7 @@ func TestHotPathChatFlushesVisibleDeltaBeforeProviderTerminal(t *testing.T) { }) tools := scriptedLightTools("openai") - history := []any{map[string]any{"role": "user", "content": "flush before terminal"}} + history := []any{map[string]any{"role": "user", "content": "Working directory: /workspace\nflush before terminal"}} consume := func(response *httptest.ResponseRecorder, results []string) { t.Helper() assistant, ids, err := artifactAssistantFromResponse("openai", response.Body.Bytes()) @@ -498,7 +499,7 @@ func TestHotPathChatProviderLengthFlushesBeforeTerminalAndStopsLight(t *testing. }) tools := scriptedLightTools("openai") - history := []any{map[string]any{"role": "user", "content": "provider length terminal"}} + history := []any{map[string]any{"role": "user", "content": "Working directory: /workspace\nprovider length terminal"}} consume := func(response *httptest.ResponseRecorder, results []string) { t.Helper() assistant, ids, err := artifactAssistantFromResponse("openai", response.Body.Bytes()) diff --git a/apps/edge/internal/openai/hot_path_cleanup.go b/apps/edge/internal/openai/hot_path_cleanup.go index 87dfd48d..6cca5ec2 100644 --- a/apps/edge/internal/openai/hot_path_cleanup.go +++ b/apps/edge/internal/openai/hot_path_cleanup.go @@ -219,7 +219,7 @@ func (s *hotPathLightStore) beginCleanupLocked( if record.protocol == "anthropic" { cleanupOutput.TerminalReason = "tool_use" } - if outer != nil { + if outer != nil && !cleanupOutput.CallerStageOnly { if err := runHotPathCollectedStage(ctx, outer, cleanupStageID, cleanupOutput); err != nil { return normalizedStageOutput{}, fmt.Errorf("collect cleanup outer turn: %w", err) } diff --git a/apps/edge/internal/openai/hot_path_cleanup_test.go b/apps/edge/internal/openai/hot_path_cleanup_test.go index 30057e54..4ffed818 100644 --- a/apps/edge/internal/openai/hot_path_cleanup_test.go +++ b/apps/edge/internal/openai/hot_path_cleanup_test.go @@ -347,8 +347,10 @@ 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}`}) + localWork := fixture.request() + fixture.consumeToolResponse(localWork, []string{`{"ok":true}`}) + workerReview := fixture.request() + fixture.consumeToolResponse(workerReview, []string{`{"written":true}`}) } } @@ -434,8 +436,10 @@ func TestHotPathCleanupCancellationStopsWork(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}`}) + localWork := fixture.request() + fixture.consumeToolResponse(localWork, []string{`{"ok":true}`}) + workerReview := fixture.request() + fixture.consumeToolResponse(workerReview, []string{`{"written":true}`}) reviewInspection := fixture.request() fixture.consumeToolResponse(reviewInspection, []string{`{"written":true}`, `{"written":true}`, `{"ok":true}`}) diff --git a/apps/edge/internal/openai/hot_path_direct.go b/apps/edge/internal/openai/hot_path_direct.go index dbfca346..5f8fb758 100644 --- a/apps/edge/internal/openai/hot_path_direct.go +++ b/apps/edge/internal/openai/hot_path_direct.go @@ -133,11 +133,17 @@ func (s *Server) runDirectTurn(ctx context.Context, turn *hotPathTurn, output no func directIssuedCallHash(protocol string, output normalizedStageOutput) (string, error) { if protocol == "anthropic" { - return fingerprintCanonicalJSON(logicalRequestEndpointAnthropic, map[string]any{ - "role": "assistant", "content": anthropicDirectBlocks(output), - }) + blocks, err := json.Marshal(map[string]any{"role": "assistant", "content": anthropicDirectBlocks(output)}) + if err != nil { + return "", err + } + return anthropicIssuedCallHash(blocks) } - return fingerprintCanonicalJSON(logicalRequestEndpointChat, openAIDirectMessage(output)) + message, err := json.Marshal(openAIDirectMessage(output)) + if err != nil { + return "", err + } + return chatIssuedCallHash(message) } func (s *Server) writeDirectError(turn *hotPathTurn, status int, errorType, message string) error { diff --git a/apps/edge/internal/openai/hot_path_direct_test.go b/apps/edge/internal/openai/hot_path_direct_test.go index 920f7971..ce8cb841 100644 --- a/apps/edge/internal/openai/hot_path_direct_test.go +++ b/apps/edge/internal/openai/hot_path_direct_test.go @@ -261,7 +261,7 @@ func TestArtifactPairHandlerDisposition(t *testing.T) { } srv := newScriptedArtifactHandlerServer(t, service) tools := scriptedArtifactTools(endpoint) - history := []any{map[string]any{"role": "user", "content": "write a plan"}} + history := []any{map[string]any{"role": "user", "content": "Working directory: /workspace\nwrite a plan"}} first := serveScriptedArtifactRequest(t, srv, endpoint, scriptedArtifactRequestBody(t, endpoint, tools, history)) if first.Code != http.StatusOK || service.calls != 1 { @@ -461,16 +461,19 @@ func scriptedSelectorDirective(providerBody []byte, operation string) (string, s return requestID, state, nil } required := []string{ - "Return exactly two admitted write tool calls", + "Return exactly one iop_write_artifact_pair tool call", "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:", + "You are the Planner.", + "Analyze the immutable user task first.", + "Preserve every explicit requirement, constraint, deliverable, and acceptance condition.", + "# Plan\n\n## Goal\n" + hotPathCallerWorkspacePlanBoundary + " \n\n## Steps\n- [P1] \n- [P2] ", + "The pending REVIEW must use exactly the Plan's P1..Pn inventory in order", "# 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.", + "Do not copy the explanatory optional-status text into REVIEW.", } for _, fragment := range required { if !strings.Contains(instruction, fragment) { @@ -640,7 +643,10 @@ func scriptedArtifactPair(endpoint, requestID string) string { 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"}) + review, _ := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, singlerequesttemplate.ReviewFields{ + ItemStatus: "- P1: pending\n- P2: pending", Changes: "Pending worker execution.", + Verification: "Pending worker verification.", Deviations: "None recorded.", + }, singlerequesttemplate.MaxTemplateBytes) 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":%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)) } diff --git a/apps/edge/internal/openai/hot_path_dispatch.go b/apps/edge/internal/openai/hot_path_dispatch.go index 1f313695..85027e62 100644 --- a/apps/edge/internal/openai/hot_path_dispatch.go +++ b/apps/edge/internal/openai/hot_path_dispatch.go @@ -531,15 +531,23 @@ func unixSeconds(timestamp int64) int64 { func decodePresetTunnelBody(body []byte, contentType, protocol, operation, driver string) (normalizedStageOutput, error) { streaming := strings.Contains(strings.ToLower(contentType), "text/event-stream") || bytes.Contains(body, []byte("data:")) + if operation == string(config.OperationResponses) { + stage, err := decodeResponsesPresetBody(body, streaming) + if err != nil { + return normalizedStageOutput{}, err + } + if protocol == "anthropic" { + stage.Usage = openAIUsageToAnthropic(stage.Usage) + stage.TerminalReason = openAIReasonToAnthropic(stage.TerminalReason) + } + return stage, nil + } if protocol == "anthropic" && (operation == string(config.OperationMessages) || (operation == "" && driver == string(config.ProtocolDriverAnthropicMessages))) { if streaming { return decodeAnthropicPresetSSE(body) } return decodeAnthropicPresetJSON(body) } - if protocol == "anthropic" && operation == string(config.OperationResponses) { - return decodeResponsesPresetBody(body, streaming) - } var stage normalizedStageOutput var err error if streaming { @@ -636,8 +644,9 @@ func decodeOpenAIPresetSSE(body []byte) (normalizedStageOutput, error) { stage := normalizedStageOutput{} identity := &hotPathProviderIdentity{} type toolState struct { - id, name string - args strings.Builder + id, name string + signature string + args strings.Builder } tools := make(map[int]*toolState) for _, payload := range sseDataPayloads(body) { @@ -687,10 +696,27 @@ func decodeOpenAIPresetSSE(body []byte) (normalizedStageOutput, error) { stage.Deltas = append(stage.Deltas, normalizedStageDelta{Kind: normalizedStageDeltaReasoning, Text: reasoning}) } for _, delta := range choice.Delta.ToolCalls { - state := tools[delta.Index] + toolIndex := delta.Index + if toolIndex == 0 && delta.ID != "" { + if first := tools[0]; first != nil && first.id != "" && first.id != delta.ID { + matched := false + for existingIndex, existing := range tools { + if existing.id == delta.ID { + toolIndex = existingIndex + matched = true + break + } + } + if !matched { + for toolIndex = 1; tools[toolIndex] != nil; toolIndex++ { + } + } + } + } + state := tools[toolIndex] if state == nil { state = &toolState{} - tools[delta.Index] = state + tools[toolIndex] = state } if delta.ID != "" { state.id = delta.ID @@ -698,6 +724,9 @@ func decodeOpenAIPresetSSE(body []byte) (normalizedStageOutput, error) { if delta.Function.Name != "" { state.name = delta.Function.Name } + if delta.ExtraContent.Google != nil && delta.ExtraContent.Google.ThoughtSignature != "" { + state.signature = delta.ExtraContent.Google.ThoughtSignature + } state.args.WriteString(delta.Function.Arguments) if delta.Function.Arguments != "" { stage.Deltas = append(stage.Deltas, normalizedStageDelta{ @@ -721,6 +750,9 @@ func decodeOpenAIPresetSSE(body []byte) (normalizedStageOutput, error) { if !ok { return normalizedStageOutput{}, fmt.Errorf("preset Chat stream tool indices are not contiguous") } + if state.signature != "" && !strings.HasPrefix(state.id, geminiThoughtSignatureToolIDPrefix) { + state.id = encodeGeminiThoughtSignatureToolID(state.id, state.signature) + } call, err := normalizedToolCallFromParts(state.id, state.name, state.args.String()) if err != nil { return normalizedStageOutput{}, err @@ -1155,6 +1187,12 @@ func (s *Server) dispatchPresetTurn( preset = found } } + output, err := s.artifactFrontiers.expandSelectorPair(requestID, ownerEdgeID, output) + if err != nil { + s.terminalPresetRequest(requestID, ownerEdgeID) + writeHotPathPresetDispatchError(w, r, protocol, http.StatusBadRequest, "invalid_request_error", err.Error()) + return err + } decision, err := classifyHotPathOutput(preset, issued, output, gate) if err != nil { if initialAdmission { @@ -1270,7 +1308,12 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh metadata[principalMetaRef] = snapshot.PrincipalRef } applyTrustedManagedBindingMetadata(metadata, route) - estimate := estimateInputTokensBytes([]byte(prompt), metadata, snapshot.Tools, nil) + systemPrompt := snapshot.Input.systemPrompt(snapshot.Phase) + estimateInput := prompt + if systemPrompt != "" { + estimateInput = systemPrompt + "\n" + prompt + } + estimate := estimateInputTokensBytes([]byte(estimateInput), metadata, snapshot.Tools, nil) contextClass := classifyContext(estimate, s.longContextThreshold()) runInput := hotPathStageRunInput(snapshot, prompt) runReq := edgeservice.SubmitRunRequest{ @@ -1280,6 +1323,10 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh MaxQueue: route.MaxQueue, QueueTimeoutMS: route.QueueTimeoutMS, Metadata: metadata, EstimatedInputTokens: estimate, ContextClass: contextClass, ProviderPool: route.ProviderPool, } + collectedOuter := outer + if snapshot.RequiresCollectedProjection { + collectedOuter = nil + } if !route.ProviderPool { if routeUsesProviderTunnel(route) { @@ -1302,10 +1349,11 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh if shouldProgressivelyReleaseHotPathStage(snapshot, outer) { return s.runHotPathLiveTunnelStage(ctx, snapshot, outer, handle, dispatch) } - output, collectErr := s.collectHotPathOwnedTunnelStage(ctx, snapshot.StageID, outer, handle, dispatch, "openai") + output, collectErr := s.collectHotPathOwnedTunnelStage(ctx, snapshot.StageID, collectedOuter, handle, dispatch, "openai") if collectErr != nil { return normalizedStageOutput{}, hotPathStageCorrelation{}, collectErr } + output.CallerStageOnly = snapshot.RequiresCollectedProjection return output, stageCorrelation(snapshot.StageID, output, dispatch), nil } handle, submitErr := s.service.SubmitRun(ctx, runReq) @@ -1316,10 +1364,11 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh if shouldProgressivelyReleaseHotPathStage(snapshot, outer) { return s.runHotPathLiveNormalizedStage(ctx, snapshot, outer, handle, dispatch) } - output, collectErr := s.collectHotPathOwnedNormalizedStage(ctx, snapshot.StageID, outer, handle, dispatch) + output, collectErr := s.collectHotPathOwnedNormalizedStage(ctx, snapshot.StageID, collectedOuter, handle, dispatch) if collectErr != nil { return normalizedStageOutput{}, hotPathStageCorrelation{}, collectErr } + output.CallerStageOnly = snapshot.RequiresCollectedProjection return output, stageCorrelation(snapshot.StageID, output, dispatch), nil } @@ -1370,10 +1419,10 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh } switch result.Path { case edgeservice.ProviderPoolPathNormalized: - output, err = s.collectHotPathOwnedNormalizedStage(ctx, snapshot.StageID, outer, result.Run, result.DispatchInfo) + output, err = s.collectHotPathOwnedNormalizedStage(ctx, snapshot.StageID, collectedOuter, result.Run, result.DispatchInfo) case edgeservice.ProviderPoolPathTunnel: output, err = s.collectHotPathOwnedTunnelStage( - ctx, snapshot.StageID, outer, result.Tunnel, result.DispatchInfo, hotPathStageWireProtocol(result.DispatchInfo), + ctx, snapshot.StageID, collectedOuter, result.Tunnel, result.DispatchInfo, hotPathStageWireProtocol(result.DispatchInfo), ) default: s.abortHotPathRejectedDispatch(rejection) @@ -1385,6 +1434,7 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh if err != nil { return normalizedStageOutput{}, hotPathStageCorrelation{}, err } + output.CallerStageOnly = snapshot.RequiresCollectedProjection if strings.TrimSpace(output.ResponseID) == "" { return normalizedStageOutput{}, hotPathStageCorrelation{}, fmt.Errorf("hot path stage completion is missing provider identity") } @@ -1392,7 +1442,13 @@ func (s *Server) submitHotPathStage(ctx context.Context, r *http.Request, snapsh } func shouldProgressivelyReleaseHotPathStage(snapshot hotPathDispatchSnapshot, outer *hotPathOuterTurn) bool { - return snapshot.Stream && (snapshot.Protocol == "openai" || snapshot.Protocol == "anthropic") && outer != nil + // Work/Review provider calls use canonical workspace operations that may + // need caller-binding normalization (for example read(path) -> bash(command)). + // Releasing provider deltas before classification would let the caller run + // an unbound internal tool call. Collect the bounded stage first, then project + // only the validated caller-visible output. + return !snapshot.RequiresCollectedProjection && snapshot.Stream && + (snapshot.Protocol == "openai" || snapshot.Protocol == "anthropic") && outer != nil } // newHotPathRejectedDispatchOwner builds one result-scoped disposal owner for a @@ -1556,7 +1612,13 @@ func (s *Server) prepareHotPathStageTunnel(r *http.Request, snapshot hotPathDisp prepared.BuildBody = func(target string) ([]byte, error) { return hotPathChatStageBody(snapshot, prompt, target) } - return prepareProviderChatRequestNormalization(prepared, selected) + requirements := providerRequestRequirements{ + HasTools: len(snapshot.Tools) > 0, Stream: snapshot.Stream, + } + if effort, ok := snapshot.Stage.Options["reasoning_effort"].(string); ok { + requirements.Effort = strings.TrimSpace(effort) + } + return prepareHotPathChatProviderOperation(prepared, selected, requirements) case config.ProtocolDriverAnthropicMessages: request := r.Clone(r.Context()) if strings.TrimSpace(request.Header.Get(anthropicVersionHeader)) == "" { @@ -1679,9 +1741,12 @@ func hotPathStageMetaFromDispatch(stageID string, dispatch edgeservice.RunDispat func hotPathStageRunInput(snapshot hotPathDispatchSnapshot, prompt string) map[string]any { messages := hotPathChatStageMessages(snapshot, prompt) input := map[string]any{"prompt": prompt, "messages": messages} - if tools := hotPathChatTools(snapshot.Tools); len(tools) > 0 { + if systemPrompt := snapshot.Input.systemPrompt(snapshot.Phase); systemPrompt != "" { + input["system"] = systemPrompt + } + if tools := hotPathChatTools(hotPathStageProviderTools(snapshot)); len(tools) > 0 { input["tools"] = tools - input["tool_choice"] = "auto" + input["tool_choice"] = hotPathChatStageToolChoice(snapshot) } options := cloneAnyMap(snapshot.Stage.Options) if options == nil { @@ -1700,9 +1765,9 @@ func hotPathChatStageBody(snapshot hotPathDispatchSnapshot, prompt, target strin body := map[string]any{ "model": target, "messages": hotPathChatStageMessages(snapshot, prompt), "stream": snapshot.Stream, } - if tools := hotPathChatTools(snapshot.Tools); len(tools) > 0 { + if tools := hotPathChatTools(hotPathStageProviderTools(snapshot)); len(tools) > 0 { body["tools"] = tools - body["tool_choice"] = "auto" + body["tool_choice"] = hotPathChatStageToolChoice(snapshot) } reserved := map[string]struct{}{"model": {}, "messages": {}, "tools": {}, "stream": {}} if snapshot.OutputBudget.Limited { @@ -1717,9 +1782,12 @@ func hotPathAnthropicStageBody(snapshot hotPathDispatchSnapshot, prompt, target body := map[string]any{ "model": target, "max_tokens": 4096, "messages": hotPathAnthropicStageMessages(snapshot, prompt), "stream": snapshot.Stream, } - if tools := hotPathAnthropicTools(snapshot.Tools); len(tools) > 0 { + if systemPrompt := snapshot.Input.systemPrompt(snapshot.Phase); systemPrompt != "" { + body["system"] = systemPrompt + } + if tools := hotPathAnthropicTools(hotPathStageProviderTools(snapshot)); len(tools) > 0 { body["tools"] = tools - body["tool_choice"] = map[string]any{"type": "auto"} + body["tool_choice"] = map[string]any{"type": hotPathChatStageToolChoice(snapshot)} } reserved := map[string]struct{}{"model": {}, "messages": {}, "tools": {}, "stream": {}} if snapshot.OutputBudget.Limited { @@ -1730,6 +1798,17 @@ func hotPathAnthropicStageBody(snapshot hotPathDispatchSnapshot, prompt, target return json.Marshal(body) } +func hotPathChatStageToolChoice(snapshot hotPathDispatchSnapshot) string { + if snapshot.Phase == hotPathPhaseReviewActive && len(snapshot.Transcript) == 0 { + return "required" + } + return "auto" +} + +func hotPathStageProviderTools(snapshot hotPathDispatchSnapshot) []any { + return cloneAnySlice(snapshot.Tools) +} + func applyHotPathStageOptions(body map[string]any, options map[string]any, reserved map[string]struct{}) { for key, value := range options { if _, blocked := reserved[key]; blocked { @@ -1740,7 +1819,11 @@ func applyHotPathStageOptions(body map[string]any, options map[string]any, reser } func hotPathChatStageMessages(snapshot hotPathDispatchSnapshot, prompt string) []any { - messages := []any{map[string]any{"role": "user", "content": prompt}} + messages := make([]any, 0, len(snapshot.Transcript)*2+2) + if systemPrompt := snapshot.Input.systemPrompt(snapshot.Phase); systemPrompt != "" { + messages = append(messages, map[string]any{"role": "system", "content": systemPrompt}) + } + messages = append(messages, map[string]any{"role": "user", "content": prompt}) for _, exchange := range snapshot.Transcript { assistant := map[string]any{"role": "assistant", "content": exchange.Output.Content} if exchange.Output.Reasoning != "" { diff --git a/apps/edge/internal/openai/hot_path_light.go b/apps/edge/internal/openai/hot_path_light.go index ae5aea50..5f23b051 100644 --- a/apps/edge/internal/openai/hot_path_light.go +++ b/apps/edge/internal/openai/hot_path_light.go @@ -12,7 +12,6 @@ import ( edgeservice "iop/apps/edge/internal/service" "iop/packages/go/config" - "iop/packages/go/singlerequesttemplate" ) const defaultHotPathLightCapacity = 1024 @@ -105,12 +104,12 @@ type hotPathLightRecord struct { phase hotPathLightPhase artifactReady bool - planItemIDs []string localPlanRead bool workerReviewWritten bool reviewerPlanRead bool reviewerReviewRead bool reviewerInspected bool + pendingLocalCommit *hotPathStageCorrelation running bool pendingKind hotPathPendingKind pending map[string]hotPathPendingCall @@ -145,6 +144,9 @@ type hotPathDispatchSnapshot struct { Tools []any Transcript []hotPathStageExchange Stream bool + // RequiresCollectedProjection prevents canonical provider tool deltas from + // escaping before a command-mode caller binding can encode them. + RequiresCollectedProjection bool // OutputBudget is recalculated from the request-local outer accumulator // before every stage. Limited, remaining, and exhausted are distinct so an // exhausted turn cannot be encoded as a one-token provider request. @@ -256,7 +258,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, planItemIDs []string) error { +func (s *hotPathLightStore) updateArtifactLineage(requestID, ownerEdgeID string, lineage logicalRequestLineage, localEligible bool) error { if s == nil { return fmt.Errorf("light flow is unavailable") } @@ -268,11 +270,7 @@ 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 } @@ -346,6 +344,7 @@ func (s *hotPathLightStore) beginDispatch(requestID, ownerEdgeID string, stream Protocol: record.protocol, Phase: record.phase, StageID: stageID, Stage: stage, Route: route, PresetRoute: cloneHotPathDispatch(record.dispatch), Input: input, Tools: cloneAnySlice(record.tools), Transcript: cloneStageTranscript(transcript), Stream: stream, + RequiresCollectedProjection: record.binding.requiresCollectedProjection(), }, nil } @@ -443,6 +442,7 @@ func (s *hotPathLightStore) issueTools( kind hotPathPendingKind, outer *hotPathOuterTurn, coordinator *logicalRequestCoordinator, + localCommit *hotPathStageCorrelation, ) (normalizedStageOutput, error) { if s == nil || coordinator == nil { return normalizedStageOutput{}, fmt.Errorf("light flow is unavailable") @@ -468,7 +468,7 @@ func (s *hotPathLightStore) issueTools( stageID = record.reviewStageID } if outer != nil { - if !output.ProgressivelyReleased { + if !output.ProgressivelyReleased && !output.CallerStageOnly { if err := runHotPathCollectedStage(ctx, outer, stageID, mapped); err != nil { return normalizedStageOutput{}, fmt.Errorf("collect light tool outer turn: %w", err) } @@ -478,8 +478,10 @@ func (s *hotPathLightStore) issueTools( outer.commitLengthTerminal() return hotPathCompatibilityOutput(outer, mapped.StageResponseOverlay(visible), record.protocol), nil } - if err := outer.projectToolIdentities(mapped.ToolCalls); err != nil { - return normalizedStageOutput{}, err + if !output.CallerStageOnly { + if err := outer.projectToolIdentities(mapped.ToolCalls); err != nil { + return normalizedStageOutput{}, err + } } } mapped = mapped.StageResponseOverlay(visible) @@ -497,6 +499,14 @@ func (s *hotPathLightStore) issueTools( if _, err := coordinator.awaitToolResults(requestID, ownerEdgeID, stageID, expected, issuedHash); err != nil { return normalizedStageOutput{}, err } + if kind == hotPathPendingLocalHandoff { + if localCommit == nil { + return normalizedStageOutput{}, fmt.Errorf("worker review handoff commit correlation is unavailable") + } + commit := *localCommit + commit.StageID = record.localStageID + record.pendingLocalCommit = &commit + } record.pendingKind = kind record.pending = pending record.pendingHash = issuedHash @@ -525,14 +535,22 @@ func mapHotPathStageCalls(record *hotPathLightRecord, output normalizedStageOutp return normalizedStageOutput{}, nil, fmt.Errorf("stage public tool id is invalid") } - operation, requiredPath, reserved, err := hotPathWorkspaceCall(record.phase, kind, paths, call) + operation, requiredPath, reserved, err := hotPathWorkspaceCall(kind, paths, call) if err != nil { return normalizedStageOutput{}, nil, err } var mapped normalizedToolCall var payload *workspaceEncodedPayload if reserved { - mapped, payload, err = mapArtifactCall(record.binding, call, operation, requiredPath, coordinator) + bound := record.binding.operation(operation) + if bound == nil || strings.TrimSpace(bound.toolName) == "" { + return normalizedStageOutput{}, nil, fmt.Errorf("artifact %s binding is unavailable", operation) + } + callerCall := call + callerCall.Name = bound.toolName + callerCall.Arguments = cloneAnyMap(call.Arguments) + setMappedArgument(callerCall.Arguments, bound.pathField, requiredPath) + mapped, payload, err = mapArtifactCall(record.binding, callerCall, operation, requiredPath, coordinator) if err != nil { return normalizedStageOutput{}, nil, err } @@ -579,36 +597,27 @@ func hotPathToolAllowed(tools []any, name string) bool { return ok } -func hotPathWorkspaceCall(phase hotPathLightPhase, kind hotPathPendingKind, paths reservedPaths, call normalizedToolCall) (workspaceOperationKind, string, bool, error) { +func hotPathWorkspaceCall(kind hotPathPendingKind, paths reservedPaths, call normalizedToolCall) (workspaceOperationKind, string, bool, error) { reserved := reservedPathsFromToolCall(call) - if len(reserved) == 0 { - return "", "", false, nil - } if len(reserved) != 1 { - return "", "", false, fmt.Errorf("stage tool call contains ambiguous reserved paths") + return "", "", false, nil } observed := cleanRelativePath(reserved[0]) switch kind { case hotPathPendingLocalTools: - if observed != cleanRelativePath(paths.PlanPath) { - return "", "", false, fmt.Errorf("worker may only read the issued plan artifact") + if observed == cleanRelativePath(paths.PlanPath) { + return opKindRead, observed, true, nil } - return opKindRead, observed, true, nil case hotPathPendingLocalHandoff: - if observed != cleanRelativePath(paths.ReviewPath) { - return "", "", false, fmt.Errorf("worker handoff targets a non-review path") + if observed == cleanRelativePath(paths.ReviewPath) { + return opKindWrite, paths.ReviewPath, true, nil } - 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") + if observed == cleanRelativePath(paths.PlanPath) || observed == cleanRelativePath(paths.ReviewPath) { + return opKindRead, observed, true, nil } - return opKindRead, observed, true, nil - case hotPathPendingReviewRepair: - return "", "", false, fmt.Errorf("repair cannot start a second reserved review cycle") - default: - return "", "", false, fmt.Errorf("unknown light tool frontier %q in phase %q", kind, phase) } + return "", "", false, nil } func (s *hotPathLightStore) consumeChat(ownerEdgeID, principalRef string, rawBody []byte, lineage logicalRequestContinuationLineage, coordinator *logicalRequestCoordinator) (logicalRequestSnapshot, hotPathLightDisposition, bool, error) { @@ -730,7 +739,21 @@ func (s *hotPathLightStore) consume(ownerEdgeID, principalRef, protocol string, record.pendingHash = "" record.pendingOutput = normalizedStageOutput{} previousPhase := record.phase - record.phase = phaseAfterHotPathResult(pendingKind) + if pendingKind == hotPathPendingLocalHandoff { + if !record.workerReviewWritten || record.pendingLocalCommit == nil { + return logicalRequestSnapshot{}, hotPathLightDisposition{}, true, fmt.Errorf("worker completion review projection failed") + } + reviewStageID, err := coordinator.newStageID() + if err != nil { + return logicalRequestSnapshot{}, hotPathLightDisposition{}, true, err + } + record.localCommit = *record.pendingLocalCommit + record.pendingLocalCommit = nil + record.reviewStageID = reviewStageID + record.phase = hotPathPhaseReviewActive + } else { + record.phase = phaseAfterHotPathResult(pendingKind) + } record.pendingKind = "" stageID := record.localStageID if record.phase != hotPathPhaseLocalActive { @@ -797,11 +820,25 @@ func (s *hotPathLightStore) matchRecordLocked(ownerEdgeID, principalRef, protoco if record.principalRef != principalRef { return nil, true, errLogicalRequestPrincipal } - if record.protocol != protocol || record.lineage != lineage.Prefix { - return nil, true, errLogicalRequestLineage + if record.protocol != protocol { + return nil, true, fmt.Errorf("%w: protocol changed", errLogicalRequestLineage) + } + if record.lineage != lineage.Prefix { + return nil, true, describeArtifactPrefixMismatch(record.lineage, lineage.Prefix) } return record, true, nil } + for _, record := range candidates { + if record.ownerEdgeID != ownerEdgeID || record.principalRef != principalRef || record.protocol != protocol { + continue + } + if record.lineage != lineage.Prefix { + return nil, true, describeArtifactPrefixMismatch(record.lineage, lineage.Prefix) + } + if record.pendingHash != lineage.IssuedCallHash { + return nil, true, fmt.Errorf("%w: issued tool calls changed", errLogicalRequestLineage) + } + } return nil, true, errLogicalRequestLineage } @@ -833,9 +870,6 @@ 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 @@ -864,44 +898,42 @@ func (s *hotPathLightStore) classifyLocalOutput(requestID, ownerEdgeID string, o 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 + reviewWrites := make([]normalizedToolCall, 0, 1) for _, call := range output.ToolCalls { - for _, observed := range reservedPathsFromToolCall(call) { - if cleanRelativePath(observed) == cleanRelativePath(paths.ReviewPath) { - reviewWrites++ - } + if isWorkerReviewHandoffCall(record.binding, paths, call) { + reviewWrites = append(reviewWrites, call) } } - if reviewWrites == 0 { + if len(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 - } + for _, call := range reviewWrites { + if _, err := artifactWriteContent(record.binding, call); err != nil { + return "", err } } - 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 } +func isWorkerReviewHandoffCall(binding *workspaceBinding, paths reservedPaths, call normalizedToolCall) bool { + write := binding.operation(opKindWrite) + if write == nil || strings.TrimSpace(write.contentField) == "" { + return false + } + if _, ok := lookupMappedArgument(call.Arguments, write.contentField); !ok { + return false + } + for _, observed := range reservedPathsFromToolCall(call) { + if cleanRelativePath(observed) == cleanRelativePath(paths.ReviewPath) { + return true + } + } + return false +} + type hotPathReviewEvidence struct { planRead bool reviewRead bool @@ -1004,12 +1036,23 @@ func (s *Server) runHotPathLightStage(w http.ResponseWriter, r *http.Request, di return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, hotPathLightEndpointErrorForCause(protocol, http.StatusBadGateway, snapshot.StageID, err)) } + collected := false + if snapshot.Phase == hotPathPhaseLocalActive && len(output.ToolCalls) == 0 && !output.ProgressivelyReleased && !output.CallerStageOnly { + if err := runHotPathCollectedStage(r.Context(), outer, snapshot.StageID, output); err != nil { + return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, + hotPathLightEndpointError(protocol, http.StatusBadGateway, err.Error())) + } + collected = true + } + if snapshot.Phase == hotPathPhaseLocalActive && len(output.ToolCalls) == 0 && outer.outputBudget().Exhausted { + return s.writeHotPathLightLengthTerminal(w, r, dispatch, protocol, stream, requestID, output) + } visible = mergeVisibleStageOutput(visible, output) // The collector compatibility path remains the endpoint renderer until // endpoint codecs consume released deltas directly. Feed the same // output into the sequencer now so its usage and terminal boundary span // local→review transitions in this HTTP turn. - if len(output.ToolCalls) == 0 && !output.ProgressivelyReleased { + if len(output.ToolCalls) == 0 && !output.ProgressivelyReleased && !output.CallerStageOnly && !collected { if err := runHotPathCollectedStage(r.Context(), outer, snapshot.StageID, output); err != nil { return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, hotPathLightEndpointError(protocol, http.StatusBadGateway, err.Error())) @@ -1027,7 +1070,7 @@ func (s *Server) runHotPathLightStage(w http.ResponseWriter, r *http.Request, di 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) + mapped, err := s.lightFlows.issueTools(r.Context(), requestID, s.edgeIDValue(), output, visible, kind, outer, s.requestCoordinator, &correlation) if err != nil { return s.writeHotPathPrimaryError(w, r, dispatch, protocol, stream, requestID, hotPathLightEndpointError(protocol, http.StatusBadRequest, err.Error())) diff --git a/apps/edge/internal/openai/hot_path_light_test.go b/apps/edge/internal/openai/hot_path_light_test.go index de7b7278..5050e611 100644 --- a/apps/edge/internal/openai/hot_path_light_test.go +++ b/apps/edge/internal/openai/hot_path_light_test.go @@ -15,6 +15,8 @@ import ( "iop/packages/go/singlerequesttemplate" ) +const scriptedAbsoluteWorkspaceTask = "system: Working directory: /workspace\nuser: immutable user task" + func TestHotPathLightLocalTransition(t *testing.T) { for _, endpoint := range []string{"openai", "anthropic"} { endpoint := endpoint @@ -25,34 +27,94 @@ func TestHotPathLightLocalTransition(t *testing.T) { t.Fatalf("final response: status=%d body=%s", final.Code, final.Body.String()) } history, _ := json.Marshal(fixture.history) - if !strings.Contains(string(history), "local-complete-visible") { - t.Fatalf("local completion was not visible before review: history=%s", history) + if !strings.Contains(string(history), "worker-handoff-visible") { + t.Fatalf("worker-authored review handoff was not issued: history=%s", history) } fixture.assertCleanupCommitted(7) }) } } -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") - }) +func TestHotPathStageCanonicalReadMapsToCallerCommandTool(t *testing.T) { + binding := mustBinding(t, fullWorkspaceAlternative("command", "bash", true), []any{openAIChatTool("bash", commandSchema())}) + record := &hotPathLightRecord{ + requestID: "req_stage_map", phase: hotPathPhaseLocalActive, binding: binding, + } + coordinator := newLogicalRequestCoordinator(logicalRequestCoordinatorOptions{IDSource: func() (string, error) { return "call_public", nil }}) + paths := newReservedPaths(record.requestID) + mapped, pending, err := mapHotPathStageCalls(record, normalizedStageOutput{ToolCalls: []normalizedToolCall{{ + ID: "provider_read", Name: "read", Arguments: map[string]any{"filePath": paths.PlanPath}, + }}}, hotPathPendingLocalTools, coordinator, nil) + if err != nil { + t.Fatal(err) + } + if len(mapped.ToolCalls) != 1 || mapped.ToolCalls[0].Name != "bash" || mapped.ToolCalls[0].Arguments["command"] == nil { + t.Fatalf("mapped caller command=%+v", mapped.ToolCalls) + } + if pending[mapped.ToolCalls[0].ID].payload == nil { + t.Fatalf("reserved read pending payload=%+v", pending) + } +} - 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 TestHotPathStageOrdinaryWorkspacePathPassesThrough(t *testing.T) { + callerTools := []any{openAIChatTool("bash", commandSchema())} + record := &hotPathLightRecord{ + requestID: "req_stage_ordinary", phase: hotPathPhaseLocalActive, tools: callerTools, + } + coordinator := newLogicalRequestCoordinator(logicalRequestCoordinatorOptions{IDSource: func() (string, error) { return "call_public", nil }}) + call := normalizedToolCall{ + ID: "provider_work", Name: "bash", + Arguments: map[string]any{"command": "inspect .iop/job/ordinary-workspace-path and continue"}, + } + mapped, pending, err := mapHotPathStageCalls(record, normalizedStageOutput{ToolCalls: []normalizedToolCall{call}}, hotPathPendingLocalTools, coordinator, nil) + if err != nil { + t.Fatal(err) + } + if len(mapped.ToolCalls) != 1 || mapped.ToolCalls[0].Name != "bash" { + t.Fatalf("ordinary caller tool changed: %+v", mapped.ToolCalls) + } + if pending[mapped.ToolCalls[0].ID].payload != nil { + t.Fatalf("ordinary workspace path became an artifact operation: %+v", pending) + } +} + +func TestWorkerReviewReadIsNotClassifiedAsHandoffWrite(t *testing.T) { + binding := mustBinding(t, fullWorkspaceAlternative("workspace", "workspace", false), []any{openAIChatTool("workspace", structuredSchema())}) + paths := newReservedPaths("req_review_read") + read := normalizedToolCall{Name: "workspace", Arguments: map[string]any{"path": paths.ReviewPath}} + if isWorkerReviewHandoffCall(binding, paths, read) { + t.Fatal("review read was classified as a handoff write") + } + write := normalizedToolCall{Name: "workspace", Arguments: map[string]any{"path": paths.ReviewPath, "content": testCompletedReviewText()}} + if !isWorkerReviewHandoffCall(binding, paths, write) { + t.Fatal("review write was not classified as a handoff write") + } +} + +func TestWorkerProviderReceivesOnlyCallerTools(t *testing.T) { + callerTools := []any{openAIChatTool("run_command", commandSchema())} + tools := hotPathStageProviderTools(hotPathDispatchSnapshot{Phase: hotPathPhaseLocalActive, Tools: callerTools}) + schemas, err := normalizeToolSchemas(tools) + if err != nil { + t.Fatal(err) + } + if schemas["run_command"] == nil || schemas["iop_write_worker_review"] != nil || len(schemas) != 1 { + t.Fatalf("worker provider tools=%+v", schemas) + } +} + +func TestInitialReviewRequiresToolUseBeforeTerminal(t *testing.T) { + initial := hotPathDispatchSnapshot{Phase: hotPathPhaseReviewActive} + if got := hotPathChatStageToolChoice(initial); got != "required" { + t.Fatalf("initial review tool choice = %q, want required", got) + } + retry := initial + retry.Transcript = []hotPathStageExchange{{}} + if got := hotPathChatStageToolChoice(retry); got != "auto" { + t.Fatalf("continued review tool choice = %q, want auto", got) + } + if got := hotPathChatStageToolChoice(hotPathDispatchSnapshot{Phase: hotPathPhaseLocalActive}); got != "auto" { + t.Fatalf("worker tool choice = %q, want auto", got) } } @@ -83,8 +145,8 @@ 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\""} local := hotPathStageCorrelation{StageID: "stg_local", ResponseID: "provider:local.actual/2", RunID: "run-local", ProviderID: "provider.actual", Terminal: "tool_calls,stop"} - localInput := buildLocalStageInput("immutable user task", paths, selector) - reviewInput := buildReviewStageInput("immutable user task", paths, selector, local) + localInput := buildLocalStageInput(scriptedAbsoluteWorkspaceTask, paths, selector) + reviewInput := buildReviewStageInput(scriptedAbsoluteWorkspaceTask, paths, selector, local) for _, input := range []hotPathStageInput{localInput, reviewInput} { phase := hotPathPhaseLocalActive @@ -100,6 +162,18 @@ func TestHotPathStageInputIsolation(t *testing.T) { t.Fatalf("stage prompt leaked %q: %s", forbidden, prompt) } } + if input.Role == "local" { + want := "/workspace/" + paths.PlanPath + "\nRead it and complete the task." + if prompt != want { + t.Fatalf("local prompt got=%q want=%q", prompt, want) + } + for _, forbidden := range []string{"immutable user task", paths.ReviewPath, "Committed selector stage success:", "Committed local stage success:", selector.StageID, local.StageID} { + if strings.Contains(prompt, forbidden) { + t.Fatalf("local prompt leaked %q: %s", forbidden, prompt) + } + } + continue + } if !strings.Contains(prompt, "immutable user task") || !strings.Contains(prompt, paths.PlanPath) || !strings.Contains(prompt, paths.ReviewPath) { t.Fatalf("stage prompt omitted immutable input: %s", prompt) } @@ -129,16 +203,6 @@ func TestHotPathStageInputIsolation(t *testing.T) { t.Fatalf("decoded selector correlation mismatch: got %#v want %#v", selDecoded, selector) } - // Local stage must NOT carry a local correlation. - if input.Role == "local" { - if strings.Contains(prompt, "Committed local stage success:") { - t.Fatalf("local prompt leaked local correlation: %s", prompt) - } - if strings.Contains(prompt, local.StageID) { - t.Fatalf("local prompt contained local correlation fields: %s", prompt) - } - } - // Review stage must carry both selector and local correlations. if input.Role == "review" { if !strings.Contains(prompt, "Committed local stage success:") { @@ -238,13 +302,13 @@ func TestHotPathStageInputIsolation(t *testing.T) { for _, invalid := range invalidLogicalIDs { selBadStage := selector selBadStage.StageID = invalid - if p, err := buildLocalStageInput("immutable user task", paths, selBadStage).prompt(hotPathPhaseLocalActive); err == nil || p != "" { + if p, err := buildLocalStageInput(scriptedAbsoluteWorkspaceTask, paths, selBadStage).prompt(hotPathPhaseLocalActive); err == nil || p != "" { t.Fatalf("selector StageID %q accepted: prompt=%q, err=%v", invalid, p, err) } selBadRun := selector selBadRun.RunID = invalid - if p, err := buildLocalStageInput("immutable user task", paths, selBadRun).prompt(hotPathPhaseLocalActive); err == nil || p != "" { + if p, err := buildLocalStageInput(scriptedAbsoluteWorkspaceTask, paths, selBadRun).prompt(hotPathPhaseLocalActive); err == nil || p != "" { t.Fatalf("selector RunID %q accepted: prompt=%q, err=%v", invalid, p, err) } } @@ -344,8 +408,8 @@ 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(string) string { return scriptedLocalWorkTool(endpoint) }, + func(requestID string) string { return scriptedReviewWrite(endpoint, requestID) }, func(requestID string) string { return scriptedReviewReadsAndInspection(endpoint, requestID) }, } if repair { @@ -373,7 +437,7 @@ func newScriptedLightFixture(t *testing.T, endpoint string, repair bool) *script tools := scriptedLightTools(endpoint) return &scriptedLightFixture{ t: t, endpoint: endpoint, server: server, service: service, tools: tools, - history: []any{map[string]any{"role": "user", "content": "immutable user task"}}, repair: repair, + history: []any{map[string]any{"role": "user", "content": "Working directory: /workspace\nimmutable user task"}}, repair: repair, } } @@ -414,8 +478,10 @@ func (f *scriptedLightFixture) runToCleanup() *httptest.ResponseRecorder { f.consumeToolResponse(pair, []string{`{"written":true}`, `{"written":true}`}) localRead := f.request() f.consumeToolResponse(localRead, []string{`{"written":true}`}) - workerHandoff := f.request() - f.consumeToolResponse(workerHandoff, []string{`{"ok":true}`, `{"written":true}`}) + localWork := f.request() + f.consumeToolResponse(localWork, []string{`{"ok":true}`}) + workerReview := f.request() + f.consumeToolResponse(workerReview, []string{`{"written":true}`}) reviewInspection := f.request() f.consumeToolResponse(reviewInspection, []string{`{"written":true}`, `{"written":true}`, `{"ok":true}`}) resolution := f.request() @@ -487,9 +553,9 @@ func (f *scriptedLightFixture) assertCleanupCommitted(wantCalls int) { selectorResponse = "msg-scripted-pair" } - localResponse := "chatcmpl-light-complete" + localResponse := "chatcmpl-review-write" if f.endpoint == "anthropic" { - localResponse = "msg-light-complete" + localResponse = "msg-review-write" } // Regression: local stage must carry selector correlation and must NOT @@ -544,8 +610,7 @@ func (f *scriptedLightFixture) assertCleanupStoresRemoved() { } // assertLocalCorrelationRegression verifies that a captured local-stage request -// carries the committed selector correlation in Run.Prompt, Run.Input["prompt"], -// and the decoded tunnel body, while omitting any local-stage correlation. +// contains only the issued PLAN path and the fixed short worker instruction. func assertLocalCorrelationRegression(t *testing.T, req edgeservice.ProviderPoolDispatchRequest, selected edgeservice.ProviderPoolCandidate, selectorStage, selectorResponse string) { t.Helper() prompt := req.Run.Prompt @@ -558,25 +623,14 @@ func assertLocalCorrelationRegression(t *testing.T, req edgeservice.ProviderPool } inputStr := fmt.Sprint(input) - if !strings.Contains(prompt, "Committed selector stage success:") { - t.Fatalf("local Run.Prompt missing selector correlation: %s", prompt) + want := "/workspace/" + newReservedPaths(req.Run.Metadata["iop_logical_request_id"]).PlanPath + "\nRead it and complete the task." + if prompt != want || inputStr != want { + t.Fatalf("local prompt got=%q input=%q want=%q", prompt, inputStr, want) } - if !strings.Contains(prompt, selectorStage) || !strings.Contains(prompt, selectorResponse) { - t.Fatalf("local Run.Prompt missing exact selector stage/response %q/%q: %s", selectorStage, selectorResponse, prompt) - } - - if !strings.Contains(inputStr, "Committed selector stage success:") { - t.Fatalf("local Run.Input[\"prompt\"] missing selector correlation: %v", input) - } - if !strings.Contains(inputStr, selectorStage) || !strings.Contains(inputStr, selectorResponse) { - t.Fatalf("local Run.Input[\"prompt\"] missing exact selector stage/response %q/%q: %v", selectorStage, selectorResponse, input) - } - - if strings.Contains(prompt, "Committed local stage success:") { - t.Fatalf("local Run.Prompt leaked local correlation: %s", prompt) - } - if strings.Contains(inputStr, "Committed local stage success:") { - t.Fatalf("local Run.Input[\"prompt\"] leaked local correlation: %v", input) + for _, forbidden := range []string{selectorStage, selectorResponse, "Committed selector stage success:", "Committed local stage success:", "User task:", "REVIEW"} { + if forbidden != "" && (strings.Contains(prompt, forbidden) || strings.Contains(inputStr, forbidden)) { + t.Fatalf("local prompt leaked %q: prompt=%q input=%q", forbidden, prompt, inputStr) + } } // Mandatory: decode and verify selected protocol tunnel prompt. @@ -587,14 +641,8 @@ func assertLocalCorrelationRegression(t *testing.T, req edgeservice.ProviderPool if tunnelPrompt != prompt { t.Fatalf("local decoded tunnel prompt mismatch: got %q want %q", tunnelPrompt, prompt) } - if !strings.Contains(tunnelPrompt, "Committed selector stage success:") { - t.Fatalf("local tunnel body missing selector correlation: %s", tunnelPrompt) - } - if !strings.Contains(tunnelPrompt, selectorStage) || !strings.Contains(tunnelPrompt, selectorResponse) { - t.Fatalf("local tunnel body missing exact selector stage/response %q/%q: %s", selectorStage, selectorResponse, tunnelPrompt) - } - if strings.Contains(tunnelPrompt, "Committed local stage success:") { - t.Fatalf("local tunnel body leaked local correlation: %s", tunnelPrompt) + if tunnelPrompt != want { + t.Fatalf("local tunnel prompt got=%q want=%q", tunnelPrompt, want) } } @@ -612,6 +660,9 @@ func assertReviewCorrelationRegression(t *testing.T, req edgeservice.ProviderPoo t.Fatalf("review Run.Input[\"prompt\"] is missing") } inputStr := fmt.Sprint(input) + if got := fmt.Sprint(req.Run.Input["system"]); got != hotPathReviewSystemPrompt { + t.Fatalf("review Run.Input system prompt mismatch: %q", got) + } if !strings.Contains(prompt, "Committed selector stage success:") { t.Fatalf("review Run.Prompt missing selector correlation: %s", prompt) @@ -640,13 +691,35 @@ func assertReviewCorrelationRegression(t *testing.T, req edgeservice.ProviderPoo } // Mandatory: decode and verify selected protocol tunnel prompt. - _, tunnelPrompt, err := decodeSelectedTunnelPrompt(req, selected) + prepared, tunnelPrompt, err := decodeSelectedTunnelPrompt(req, selected) if err != nil { t.Fatalf("review tunnel decode error: %v", err) } if tunnelPrompt != prompt { t.Fatalf("review decoded tunnel prompt mismatch: got %q want %q", tunnelPrompt, prompt) } + body, err := prepared.BuildBody("target-model") + if err != nil { + t.Fatal(err) + } + if selected.ProfileDriver == string(config.ProtocolDriverAnthropicMessages) { + var payload struct { + System string `json:"system"` + } + if err := json.Unmarshal(body, &payload); err != nil || payload.System != hotPathReviewSystemPrompt { + t.Fatalf("review Messages system prompt mismatch: system=%q err=%v body=%s", payload.System, err, body) + } + } else { + var payload struct { + Messages []struct { + Role string `json:"role"` + Content string `json:"content"` + } `json:"messages"` + } + if err := json.Unmarshal(body, &payload); err != nil || len(payload.Messages) < 2 || payload.Messages[0].Role != "system" || payload.Messages[0].Content != hotPathReviewSystemPrompt { + t.Fatalf("review Chat system prompt mismatch: messages=%+v err=%v body=%s", payload.Messages, err, body) + } + } if !strings.Contains(tunnelPrompt, "Committed selector stage success:") { t.Fatalf("review tunnel body missing selector correlation: %s", tunnelPrompt) } @@ -692,10 +765,12 @@ func decodeSelectedTunnelPrompt(req edgeservice.ProviderPoolDispatchRequest, sel if err := json.Unmarshal(bodyBytes, &payload); err != nil { return prepared, "", fmt.Errorf("unmarshal anthropic payload: %w (body=%s)", err, string(bodyBytes)) } - if len(payload.Messages) == 0 || payload.Messages[0].Role != "user" { - return prepared, "", fmt.Errorf("anthropic body missing first user message: %s", string(bodyBytes)) + for _, message := range payload.Messages { + if message.Role == "user" { + return prepared, extractMessageContentString(message.Content), nil + } } - return prepared, extractMessageContentString(payload.Messages[0].Content), nil + return prepared, "", fmt.Errorf("anthropic body missing user message: %s", string(bodyBytes)) } else { if prepared.Path != "/v1/chat/completions" || prepared.Operation != string(config.OperationChatCompletions) { return prepared, "", fmt.Errorf("openai tunnel path/op mismatch: path=%q op=%q", prepared.Path, prepared.Operation) @@ -709,10 +784,12 @@ func decodeSelectedTunnelPrompt(req edgeservice.ProviderPoolDispatchRequest, sel if err := json.Unmarshal(bodyBytes, &payload); err != nil { return prepared, "", fmt.Errorf("unmarshal openai payload: %w (body=%s)", err, string(bodyBytes)) } - if len(payload.Messages) == 0 || payload.Messages[0].Role != "user" { - return prepared, "", fmt.Errorf("openai body missing first user message: %s", string(bodyBytes)) + for _, message := range payload.Messages { + if message.Role == "user" { + return prepared, extractMessageContentString(message.Content), nil + } } - return prepared, extractMessageContentString(payload.Messages[0].Content), nil + return prepared, "", fmt.Errorf("openai body missing user message: %s", string(bodyBytes)) } } diff --git a/apps/edge/internal/openai/hot_path_observation_test.go b/apps/edge/internal/openai/hot_path_observation_test.go index 783ec406..eb3f9973 100644 --- a/apps/edge/internal/openai/hot_path_observation_test.go +++ b/apps/edge/internal/openai/hot_path_observation_test.go @@ -1497,7 +1497,7 @@ func hotPathPassTrace() []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: hotPathEventClassStage, Stage: hotPathStageKindLocal, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionToolTurn}, {Event: hotPathEventClassLight, Stage: hotPathStageKindReview, Attempt: hotPathAttemptFirst}, {Event: hotPathEventClassStage, Stage: hotPathStageKindReview, Attempt: hotPathAttemptFirst, Disposition: hotPathTerminalDispositionToolTurn}, {Event: hotPathEventClassStage, Stage: hotPathStageKindReview, Attempt: hotPathAttemptRetry, Disposition: hotPathTerminalDispositionSuccess}, diff --git a/apps/edge/internal/openai/hot_path_review.go b/apps/edge/internal/openai/hot_path_review.go index b02af66c..9ac60998 100644 --- a/apps/edge/internal/openai/hot_path_review.go +++ b/apps/edge/internal/openai/hot_path_review.go @@ -35,7 +35,7 @@ func (s *Server) advanceHotPathReview( } return mapped, true, nil } - mapped, err := s.lightFlows.issueTools(ctx, requestID, s.edgeIDValue(), output, visible, kind, outer, s.requestCoordinator) + mapped, err := s.lightFlows.issueTools(ctx, requestID, s.edgeIDValue(), output, visible, kind, outer, s.requestCoordinator, nil) if err != nil { return normalizedStageOutput{}, false, err } diff --git a/apps/edge/internal/openai/hot_path_selector.go b/apps/edge/internal/openai/hot_path_selector.go index d0f84fff..043815f9 100644 --- a/apps/edge/internal/openai/hot_path_selector.go +++ b/apps/edge/internal/openai/hot_path_selector.go @@ -342,7 +342,7 @@ func reservedPathSourcesFromToolCall(tc normalizedToolCall) []string { } add(tc.Path) if tc.Arguments != nil { - collectReservedStrings(tc.Arguments, add) + collectReservedArgumentStrings(tc.Arguments, add) if tc.RawArgs == "" { return paths } @@ -351,7 +351,7 @@ func reservedPathSourcesFromToolCall(tc normalizedToolCall) []string { decoder.UseNumber() if decoder.Decode(&decoded) == nil && decoded != nil { if !reflect.DeepEqual(decoded, tc.Arguments) { - collectReservedStrings(decoded, add) + collectReservedArgumentStrings(decoded, add) } return paths } @@ -365,24 +365,28 @@ func reservedPathSourcesFromToolCall(tc normalizedToolCall) []string { decoder := json.NewDecoder(strings.NewReader(tc.RawArgs)) decoder.UseNumber() if decoder.Decode(&decoded) == nil { - collectReservedStrings(decoded, add) + collectReservedArgumentStrings(decoded, add) } else { add(tc.RawArgs) } return paths } -func collectReservedStrings(value any, add func(string)) { +func collectReservedArgumentStrings(value any, add func(string)) { switch typed := value.(type) { case string: add(typed) case map[string]any: - for _, item := range typed { - collectReservedStrings(item, add) + for key, item := range typed { + switch strings.ToLower(strings.TrimSpace(key)) { + case "content", "plan_content", "review_content": + continue + } + collectReservedArgumentStrings(item, add) } case []any: for _, item := range typed { - collectReservedStrings(item, add) + collectReservedArgumentStrings(item, add) } } } diff --git a/apps/edge/internal/openai/hot_path_selector_test.go b/apps/edge/internal/openai/hot_path_selector_test.go index 183dadd6..cf86c59d 100644 --- a/apps/edge/internal/openai/hot_path_selector_test.go +++ b/apps/edge/internal/openai/hot_path_selector_test.go @@ -42,6 +42,13 @@ func TestHotPathSelectorDecisionMatrix(t *testing.T) { {ID: "call_review", Name: "write_file", Arguments: map[string]any{"path": issued.ReviewPath}}, }}, }, + { + name: "ExactPairContentMayMentionReservedPaths", preset: preset, gate: validGate, wantMode: modeLight, wantReason: reasonLightExactPair, + output: normalizedStageOutput{ToolCalls: []normalizedToolCall{ + {ID: "call_plan", Name: "write_file", Arguments: map[string]any{"path": issued.PlanPath, "content": "Read " + issued.ReviewPath + " before handoff."}}, + {ID: "call_review", Name: "write_file", Arguments: map[string]any{"path": issued.ReviewPath, "content": "Template for " + issued.PlanPath}}, + }}, + }, { name: "PartialPair", preset: preset, gate: validGate, wantReason: reasonMalformedPartialPair, wantErr: true, output: normalizedStageOutput{ToolCalls: []normalizedToolCall{{ID: "call_plan", Name: "write_file", Arguments: map[string]any{"path": issued.PlanPath}}}}, diff --git a/apps/edge/internal/openai/hot_path_stage_input.go b/apps/edge/internal/openai/hot_path_stage_input.go index 80070eff..c20acf49 100644 --- a/apps/edge/internal/openai/hot_path_stage_input.go +++ b/apps/edge/internal/openai/hot_path_stage_input.go @@ -3,10 +3,20 @@ package openai import ( "encoding/json" "fmt" + "path/filepath" "strings" "unicode" ) +const hotPathReviewSystemPrompt = `You are the Reviewer in a compact Plan -> Work -> Review -> Repair pipeline. +Read the exact issued Plan and worker-filled Review before judging the task. Treat Review as worker evidence, not proof. +Inspect the actual caller-workspace result and rerun the Plan's applicable verification with ordinary caller tools. +Your first response must issue tool calls that read both exact artifact paths and inspect the actual caller-workspace result; do not return terminal prose first. +Check correctness, completeness, requirement coverage, verification trust, and unjustified deviations. +If a defect exists, establish its evidence and root cause, choose one concrete fix, repair it with ordinary caller tools, and reverify in this same Review stage. +Do not rewrite the reserved Plan or Review artifacts. Do not create a separate Result or final-review document. +Return a concise non-empty final result only after the result is verified; use no tool call in that terminal response.` + type hotPathArtifactPaths struct { PlanPath string ReviewPath string @@ -21,7 +31,7 @@ type hotPathStageCorrelation struct { } // hotPathStageInput is the complete cross-stage input boundary. It contains -// only caller-owned immutable task text, issued relative paths, and committed +// only caller-owned immutable task text, issued paths, and committed // provider correlations. Workspace contents, credentials, provider targets, // and prior control prompts never enter this value. type hotPathStageInput struct { @@ -110,6 +120,14 @@ func (in hotPathStageInput) prompt(phase hotPathLightPhase) (string, error) { if err := in.validate(); err != nil { return "", err } + if in.Role == "local" { + root, err := callerWorkingDirectory(in.ImmutableTask) + if err != nil { + return "", err + } + planPath := filepath.ToSlash(filepath.Join(root, filepath.FromSlash(in.Artifacts.PlanPath))) + return planPath + "\nRead it and complete the task.", nil + } var b strings.Builder b.WriteString("User task:\n") b.WriteString(in.ImmutableTask) @@ -124,14 +142,12 @@ func (in hotPathStageInput) prompt(phase hotPathLightPhase) (string, error) { b.WriteString("\n\n") switch in.Role { - case "local": - 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("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.") + b.WriteString("Review the task using the exact issued artifacts and caller workspace.") case hotPathPhaseReviewRepair: - 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.") + b.WriteString("Continue the same repair and verification from the retained Review-stage context.") default: return "", fmt.Errorf("review input cannot run in phase %q", phase) } @@ -141,6 +157,48 @@ func (in hotPathStageInput) prompt(phase hotPathLightPhase) (string, error) { return b.String(), nil } +func (in hotPathStageInput) systemPrompt(phase hotPathLightPhase) string { + if in.Role != "review" { + return "" + } + switch phase { + case hotPathPhaseReviewActive, hotPathPhaseReviewRepair: + return hotPathReviewSystemPrompt + default: + return "" + } +} + +func callerWorkingDirectory(task string) (string, error) { + for _, line := range strings.Split(task, "\n") { + line = strings.TrimSpace(line) + for _, role := range []string{"system:", "user:"} { + if strings.HasPrefix(line, role) { + line = strings.TrimSpace(strings.TrimPrefix(line, role)) + break + } + } + for _, prefix := range []string{"Working directory:", "Current working directory:"} { + if !strings.HasPrefix(line, prefix) { + continue + } + root := filepath.Clean(strings.TrimSpace(strings.TrimPrefix(line, prefix))) + if filepath.IsAbs(root) { + return root, nil + } + } + if start := strings.Index(line, ""); start >= 0 { + if end := strings.Index(line[start+len(""):], ""); end >= 0 { + root := filepath.Clean(strings.TrimSpace(line[start+len("") : start+len("")+end])) + if filepath.IsAbs(root) { + return root, nil + } + } + } + } + return "", fmt.Errorf("caller workspace absolute path is unavailable") +} + type correlationPromptValue struct { StageID string `json:"stage"` ResponseID string `json:"response"` diff --git a/apps/edge/internal/openai/hot_path_terminal_control_test.go b/apps/edge/internal/openai/hot_path_terminal_control_test.go index d484b9f6..f28ecb92 100644 --- a/apps/edge/internal/openai/hot_path_terminal_control_test.go +++ b/apps/edge/internal/openai/hot_path_terminal_control_test.go @@ -994,7 +994,7 @@ func rejectedStageSnapshot(stream bool) hotPathDispatchSnapshot { selector := hotPathStageCorrelation{StageID: "stg-s", ResponseID: "r:s/1", RunID: "run-s", ProviderID: "p", Terminal: "t"} return hotPathDispatchSnapshot{ Protocol: "openai", Stream: stream, StageID: "stage-r", Stage: config.ExecutionRouteStage{Model: "m"}, - Input: buildLocalStageInput("immutable user task", paths, selector), + Input: buildLocalStageInput(scriptedAbsoluteWorkspaceTask, paths, selector), Route: routeDispatch{NodeRef: "node-stage", ProviderID: "p", Adapter: "a-stage", Target: "t-stage", SessionID: "s-stage", TimeoutSec: 5, ProviderPool: true}, } } diff --git a/apps/edge/internal/openai/provider_model_rewrite.go b/apps/edge/internal/openai/provider_model_rewrite.go index 94d6aa0c..5b2d4f1a 100644 --- a/apps/edge/internal/openai/provider_model_rewrite.go +++ b/apps/edge/internal/openai/provider_model_rewrite.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "sort" "strings" "sync" @@ -11,7 +12,12 @@ import ( "iop/packages/go/config" ) -const maxHotPathSelectorProviderInstructionBytes = 4096 +const ( + maxHotPathSelectorProviderInstructionBytes = 4096 + maxHotPathSelectorOutputTokens = 4096 + hotPathArtifactPairToolName = "iop_write_artifact_pair" + hotPathCallerWorkspacePlanBoundary = "Use the caller workspace current working directory as the task root; never place task outputs under .iop." +) func buildHotPathSelectorProviderInstruction(requestID string, state selectorInstructionState) (string, error) { if !validLogicalRequestID(requestID) { @@ -29,27 +35,39 @@ Do not write PLAN or REVIEW in this turn. Do not mention or infer an absolute wo 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. +Return exactly one iop_write_artifact_pair tool call containing both complete artifact contents 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: +You are the Planner. Apply this compact Plan workflow before authoring the pair: +1. Analyze the immutable user task first. Do not create a separate analysis artifact. +2. Preserve every explicit requirement, constraint, deliverable, and acceptance condition. Do not invent extra scope. + Copy every exact literal, filename, command, and required output string from the task verbatim into an executable Plan step or Verification bullet. +3. Convert that analysis into 2-6 closed, executable steps. The Worker must be able to implement without rediscovering requirements or choosing among alternatives. +4. Write 1-3 deterministic verification bullets with observable pass conditions. +5. Resolve every requested output path against the caller workspace current working directory. Never place a task output beside PLAN or anywhere under .iop unless the immutable user task explicitly requests that exact path. +6. Make the final Plan step require the Worker to read the pending REVIEW path above, then as the Work stage's final action replace the whole file with one full-file Write operation while preserving its exact headings. Never use Edit or Patch for this handoff. Every Plan item status must be exactly "completed"; actual changes, actual verification evidence, and deviations must be non-empty. + +Write every PLAN field in English using ASCII characters only and use exactly this grammar: # Plan ## Goal - +Use the caller workspace current working directory as the task root; never place task outputs under .iop. ## Steps - [P1] - [P2] +- [P3] ## Verification -- -The pending REVIEW content must be exactly this deterministic seed: +- + +Omit optional step lines that are not needed. The pending REVIEW must use exactly the Plan's P1..Pn inventory in order and exactly this grammar: # Review ## Worker Item Status - P1: pending - P2: pending +- P3: pending only when P3 exists in PLAN; continue for every chosen Plan id ## Worker Changes Pending worker execution. @@ -58,7 +76,9 @@ Pending worker execution. Pending worker verification. ## Deviations -None recorded.`, paths.PlanPath, paths.ReviewPath) +None recorded. + +Do not copy the explanatory optional-status text into REVIEW. Emit one pending status line for every and only the actual Plan ids.`, paths.PlanPath, paths.ReviewPath) default: return "", fmt.Errorf("selector provider instruction state is invalid") } @@ -104,6 +124,145 @@ func prepareHotPathSelectorProviderInstruction(tunnel edgeservice.SubmitProvider return tunnel, nil } +func prepareHotPathSelectorOutputLimit(tunnel edgeservice.SubmitProviderTunnelRequest) (edgeservice.SubmitProviderTunnelRequest, error) { + rewrite := func(body []byte) ([]byte, error) { + limitJSON, err := json.Marshal(maxHotPathSelectorOutputTokens) + if err != nil { + return nil, err + } + plan, err := planTopLevelJSONPatches(body, []topLevelJSONPatch{ + {name: "max_tokens", value: limitJSON}, + {name: "max_completion_tokens"}, + }) + if err != nil { + return nil, err + } + return plan.apply(), nil + } + 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 prepareHotPathSelectorCanonicalTools(tunnel edgeservice.SubmitProviderTunnelRequest, instruction string, preset config.ExecutionPreset) (edgeservice.SubmitProviderTunnelRequest, error) { + operation := "" + switch { + case strings.Contains(instruction, "Operation: prepare-only"): + operation = "prepare" + case strings.Contains(instruction, "Operation: pair-write"): + operation = "write" + default: + return tunnel, fmt.Errorf("selector canonical tool operation is unavailable") + } + toolNames := make(map[string]struct{}) + for _, alternative := range preset.WorkspaceTools { + configured, ok := alternative.Operations[operation] + if ok && strings.TrimSpace(configured.ToolName) != "" { + toolNames[strings.TrimSpace(configured.ToolName)] = struct{}{} + } + } + if len(toolNames) == 0 { + return tunnel, fmt.Errorf("selector canonical tool binding is unavailable") + } + rewrite := func(body []byte) ([]byte, error) { + var root map[string]any + if err := json.Unmarshal(body, &root); err != nil { + return nil, fmt.Errorf("decode selector provider tools: %w", err) + } + actual := make(map[string]map[string]any) + for _, raw := range anySlice(root["tools"]) { + tool, _ := raw.(map[string]any) + function, _ := tool["function"].(map[string]any) + name, _ := function["name"].(string) + if _, ok := toolNames[strings.TrimSpace(name)]; ok { + actual[strings.TrimSpace(name)] = function + } + } + if len(actual) == 0 { + return nil, fmt.Errorf("selector canonical tool is absent from caller tools") + } + if operation == "write" { + root["tools"] = []any{map[string]any{ + "type": "function", + "function": map[string]any{ + "name": hotPathArtifactPairToolName, + "description": "Author the complete IOP Plan and pending Review artifact pair in one atomic selector decision.", + "parameters": map[string]any{ + "type": "object", + "properties": map[string]any{ + "plan_content": map[string]any{"type": "string", "description": "Complete Plan markdown"}, + "review_content": map[string]any{"type": "string", "description": "Complete pending Review markdown"}, + }, + "required": []any{"plan_content", "review_content"}, + "additionalProperties": false, + }, + }, + }} + root["tool_choice"] = "required" + delete(root, "parallel_tool_calls") + return json.Marshal(root) + } + names := make([]string, 0, len(actual)) + for name := range actual { + names = append(names, name) + } + sort.Strings(names) + canonical := make([]any, 0, len(actual)) + for _, name := range names { + function := actual[name] + properties := map[string]any{ + "path": map[string]any{"type": "string", "description": "IOP-issued relative workspace path"}, + } + required := []any{"path"} + if operation == "write" { + properties["content"] = map[string]any{"type": "string", "description": "Complete file content"} + required = append(required, "content") + } + description, _ := function["description"].(string) + canonical = append(canonical, map[string]any{ + "type": "function", + "function": map[string]any{ + "name": name, "description": strings.TrimSpace(description + " IOP canonical " + operation + " operation."), + "parameters": map[string]any{"type": "object", "properties": properties, "required": required, "additionalProperties": false}, + }, + }) + } + root["tools"] = canonical + root["tool_choice"] = "required" + delete(root, "parallel_tool_calls") + return json.Marshal(root) + } + if tunnel.BuildBody == nil { + return tunnel, fmt.Errorf("selector provider body builder is unavailable") + } + 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 +} + func injectHotPathChatSelectorInstruction(body []byte, instruction string) ([]byte, error) { var envelope struct { Messages []json.RawMessage `json:"messages"` @@ -353,9 +512,6 @@ func rewriteProviderJSONResponse(body []byte, model, toolCallWire string) []byte } func normalizeGeminiChatProviderResponse(body []byte) []byte { - if !bytes.Contains(body, []byte(`"thought_signature"`)) { - return body - } decoder := json.NewDecoder(bytes.NewReader(body)) decoder.UseNumber() var response map[string]any @@ -373,11 +529,22 @@ func normalizeGeminiChatProviderResponse(body []byte) []byte { if !ok { continue } - for _, rawCall := range anySlice(message["tool_calls"]) { + toolCalls := anySlice(message["tool_calls"]) + for index, rawCall := range toolCalls { call, ok := rawCall.(map[string]any) if !ok { continue } + // Gemini's Chat-compatible stream may emit several complete tool + // calls in one delta without OpenAI's per-call index. Preserve their + // positional identity so the stream decoder does not concatenate + // independent argument objects into index zero. + if messageKey == "delta" && len(toolCalls) > 1 { + if _, present := call["index"]; !present { + call["index"] = index + changed = true + } + } id, idOK := call["id"].(string) extra, extraOK := call["extra_content"].(map[string]any) if !idOK || id == "" || !extraOK { @@ -567,6 +734,46 @@ func prepareProviderChatRequestNormalization(tunnel edgeservice.SubmitProviderTu return tunnel, nil } +// prepareHotPathChatProviderOperation lets an internal caller-workspace stage +// use the closest provider wire without changing the caller-facing Chat +// surface. In particular, OpenAI Chat cannot combine reasoning effort with +// function tools, while the same profile's Responses operation can. The +// internal stage result is already decoded into the common hot-path shape, so +// this operation switch remains private to IOP. +func prepareHotPathChatProviderOperation( + tunnel edgeservice.SubmitProviderTunnelRequest, + selected edgeservice.ProviderPoolCandidate, + requirements providerRequestRequirements, +) (edgeservice.SubmitProviderTunnelRequest, error) { + if selected.ProtocolProfile == nil || tunnel.BuildBody == nil { + return tunnel, errProviderStageMissingBinding + } + prepared, err := singleRequestProviderTunnelPreparer(requirements, tunnel.BuildBody)(tunnel, selected) + if err != nil { + return tunnel, err + } + if prepared.Operation != string(config.OperationChatCompletions) { + return prepared, nil + } + prepared, err = prepareProviderChatRequestNormalization(prepared, selected) + if err != nil { + return tunnel, err + } + return prepareProviderChatToolCallNormalization(prepared, selected) +} + +func chatProviderRequirements(req chatCompletionRequest) providerRequestRequirements { + requirements := providerRequestRequirements{ + HasTools: len(req.Tools) > 0, + Stream: req.Stream, + StructuredOutput: req.ResponseFormat != nil, + } + if req.ReasoningEffort != nil { + requirements.Effort = strings.TrimSpace(*req.ReasoningEffort) + } + return requirements +} + func normalizeChatTokenLimitField(body []byte, targetField string) ([]byte, error) { var limits struct { MaxTokens json.RawMessage `json:"max_tokens"` diff --git a/apps/edge/internal/openai/provider_model_rewrite_test.go b/apps/edge/internal/openai/provider_model_rewrite_test.go index 6c5fb911..c7b515a1 100644 --- a/apps/edge/internal/openai/provider_model_rewrite_test.go +++ b/apps/edge/internal/openai/provider_model_rewrite_test.go @@ -76,6 +76,59 @@ func TestProviderThoughtSignatureNormalizationIsGeminiProfileOnly(t *testing.T) } } +func TestGeminiChatProviderResponseAddsMissingParallelToolIndices(t *testing.T) { + response := []byte(`{"id":"chat-1","choices":[{"delta":{"tool_calls":[{"id":"call-1","function":{"name":"read_file","arguments":"{\"path\":\"plan.md\"}"}},{"id":"call-2","function":{"name":"read_file","arguments":"{\"path\":\"review.md\"}"}},{"id":"call-3","function":{"name":"bash","arguments":"{\"command\":\"test -f index.html\"}"}}]}}]}`) + normalized := normalizeGeminiChatProviderResponse(response) + var body map[string]any + if err := json.Unmarshal(normalized, &body); err != nil { + t.Fatal(err) + } + choice := anySlice(body["choices"])[0].(map[string]any) + delta := choice["delta"].(map[string]any) + calls := anySlice(delta["tool_calls"]) + for index, raw := range calls { + call := raw.(map[string]any) + if got := int(call["index"].(float64)); got != index { + t.Fatalf("tool call %d index = %d", index, got) + } + } + + stage, err := decodeOpenAIPresetSSE([]byte("data: " + string(normalized) + "\n\ndata: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{},\"finish_reason\":\"tool_calls\"}],\"usage\":{\"prompt_tokens\":1,\"completion_tokens\":1,\"total_tokens\":2}}\n\ndata: [DONE]\n\n")) + if err != nil { + t.Fatal(err) + } + if len(stage.ToolCalls) != 3 { + t.Fatalf("decoded tool calls = %+v", stage.ToolCalls) + } +} + +func TestOpenAIPresetSSESeparatesNewToolIDsWithoutIndices(t *testing.T) { + body := []byte("data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{\"tool_calls\":[{\"id\":\"call-1\",\"function\":{\"name\":\"read_file\",\"arguments\":\"{\\\"path\\\":\\\"plan.md\\\"}\"}}]}}]}\n\n" + + "data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{\"tool_calls\":[{\"id\":\"call-2\",\"function\":{\"name\":\"read_file\",\"arguments\":\"{\\\"path\\\":\\\"review.md\\\"}\"}}]}}]}\n\n" + + "data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{\"tool_calls\":[{\"id\":\"call-3\",\"function\":{\"name\":\"bash\",\"arguments\":\"{\\\"command\\\":\\\"test -f index.html\\\"}\"}}]}}]}\n\n" + + "data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{},\"finish_reason\":\"tool_calls\"}],\"usage\":{\"prompt_tokens\":1,\"completion_tokens\":1,\"total_tokens\":2}}\n\ndata: [DONE]\n\n") + stage, err := decodeOpenAIPresetSSE(body) + if err != nil { + t.Fatal(err) + } + if len(stage.ToolCalls) != 3 { + t.Fatalf("decoded tool calls = %+v", stage.ToolCalls) + } +} + +func TestOpenAIPresetSSECarriesLateGeminiThoughtSignature(t *testing.T) { + body := []byte("data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{\"tool_calls\":[{\"index\":0,\"id\":\"call-1\",\"function\":{\"name\":\"read_file\",\"arguments\":\"{\\\"path\\\":\\\"plan.md\\\"}\"}}]}}]}\n\n" + + "data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{\"tool_calls\":[{\"index\":0,\"extra_content\":{\"google\":{\"thought_signature\":\"late-signature\"}},\"function\":{}}]}}]}\n\n" + + "data: {\"id\":\"chat-1\",\"choices\":[{\"delta\":{},\"finish_reason\":\"tool_calls\"}],\"usage\":{\"prompt_tokens\":1,\"completion_tokens\":1,\"total_tokens\":2}}\n\ndata: [DONE]\n\n") + stage, err := decodeOpenAIPresetSSE(body) + if err != nil { + t.Fatal(err) + } + if len(stage.ToolCalls) != 1 || !strings.HasPrefix(stage.ToolCalls[0].ProviderCallID, geminiThoughtSignatureToolIDPrefix) { + t.Fatalf("late Gemini signature was not retained: %+v", stage.ToolCalls) + } +} + func TestProviderChatTokenLimitNormalizationUsesSelectedProfile(t *testing.T) { tests := []struct { name string @@ -189,6 +242,163 @@ func TestOpenAIChatProviderHTTPNormalizesGenericTokenLimit(t *testing.T) { } } +func TestHotPathOpenAIToolsAndEffortUseResponsesOperation(t *testing.T) { + profile, err := config.ResolveProtocolProfile("openai", "", config.BuiltInProtocolProfileCatalog()) + if err != nil { + t.Fatal(err) + } + tunnel := edgeservice.SubmitProviderTunnelRequest{ + Operation: string(config.OperationChatCompletions), + Path: "/v1/chat/completions", + BuildBody: func(target string) ([]byte, error) { + return []byte(`{"model":"` + target + `","messages":[{"role":"system","content":"inspect"},{"role":"user","content":"work"}],"tools":[{"type":"function","function":{"name":"bash","parameters":{"type":"object"}}}],"max_tokens":32000,"reasoning_effort":"high","temperature":0,"top_p":1,"stream":true}`), nil + }, + } + prepared, err := prepareHotPathChatProviderOperation(tunnel, edgeservice.ProviderPoolCandidate{ProtocolProfile: &profile}, providerRequestRequirements{ + HasTools: true, Stream: true, Effort: "high", + }) + if err != nil { + t.Fatal(err) + } + if prepared.Operation != string(config.OperationResponses) || prepared.Path != "/v1/responses" { + t.Fatalf("operation=%q path=%q", prepared.Operation, prepared.Path) + } + if prepared.Stream { + t.Fatal("buffered internal Responses operation retained streaming tunnel metadata") + } + body, err := prepared.BuildBody("gpt-5.6-terra") + if err != nil { + t.Fatal(err) + } + var request map[string]any + if err := json.Unmarshal(body, &request); err != nil { + t.Fatal(err) + } + if request["max_output_tokens"] != float64(32000) || request["messages"] != nil || request["max_tokens"] != nil { + t.Fatalf("Responses token/message conversion mismatch: %+v", request) + } + reasoning, _ := request["reasoning"].(map[string]any) + if reasoning["effort"] != "high" || len(anySlice(request["tools"])) != 1 { + t.Fatalf("Responses effort/tools conversion mismatch: %+v", request) + } + if request["temperature"] != nil || request["top_p"] != nil { + t.Fatalf("reasoning Responses request retained incompatible sampling controls: %+v", request) + } +} + +func TestChatProviderRequirementsAllowPresetOwnedEffort(t *testing.T) { + requirements := chatProviderRequirements(chatCompletionRequest{Tools: []any{map[string]any{"type": "function"}}}) + if requirements.Effort != "" || !requirements.HasTools { + t.Fatalf("caller requirements=%+v", requirements) + } + presetOptions := map[string]any{"reasoning_effort": "high"} + if effort, ok := presetOptions["reasoning_effort"].(string); ok { + requirements.Effort = strings.TrimSpace(effort) + } + if requirements.Effort != "high" || !requirements.HasTools { + t.Fatalf("effective selector requirements=%+v", requirements) + } +} + +func TestHotPathSelectorOutputLimitIsBounded(t *testing.T) { + tunnel := edgeservice.SubmitProviderTunnelRequest{ + BuildBody: func(string) ([]byte, error) { + return []byte(`{"model":"selector","max_completion_tokens":32000,"future":true}`), nil + }, + } + prepared, err := prepareHotPathSelectorOutputLimit(tunnel) + if err != nil { + t.Fatal(err) + } + body, err := prepared.BuildBody("selector") + if err != nil { + t.Fatal(err) + } + var request map[string]any + if err := json.Unmarshal(body, &request); err != nil { + t.Fatal(err) + } + if request["max_tokens"] != float64(maxHotPathSelectorOutputTokens) || request["max_completion_tokens"] != nil || request["future"] != true { + t.Fatalf("bounded selector request=%+v", request) + } +} + +func TestHotPathSelectorPairInstructionCarriesCompactPlanContract(t *testing.T) { + instruction, err := buildHotPathSelectorProviderInstruction("req_compact_plan", selectorInstructionPairWrite) + if err != nil { + t.Fatal(err) + } + for _, fragment := range []string{ + "You are the Planner.", + "Analyze the immutable user task first.", + "Preserve every explicit requirement, constraint, deliverable, and acceptance condition.", + "Copy every exact literal, filename, command, and required output string", + "The Worker must be able to implement without rediscovering requirements", + "Resolve every requested output path against the caller workspace current working directory.", + "Never place a task output beside PLAN or anywhere under .iop", + hotPathCallerWorkspacePlanBoundary, + "Make the final Plan step require the Worker to read the pending REVIEW path", + "replace the whole file with one full-file Write operation", + "Never use Edit or Patch for this handoff.", + "Every Plan item status must be exactly \"completed\"", + "actual verification evidence", + } { + if !strings.Contains(instruction, fragment) { + t.Fatalf("selector instruction omitted compact Plan contract %q: %s", fragment, instruction) + } + } + if strings.Contains(instruction, "deterministic seed") || strings.Contains(instruction, "exactly following two-step") { + t.Fatalf("selector instruction retained seed-oriented Plan guidance: %s", instruction) + } +} + +func TestOpenAIHotPathResponsesOperationUsesResponsesDecoder(t *testing.T) { + body := []byte(`{"id":"resp-selector","model":"gpt-5.6-terra","status":"completed","output":[{"type":"function_call","id":"fc-1","call_id":"call-1","name":"bash","arguments":"{\"command\":\"true\"}"}],"usage":{"input_tokens":10,"output_tokens":3}}`) + stage, err := decodePresetTunnelBody(body, "application/json", "openai", string(config.OperationResponses), string(config.ProtocolDriverOpenAIChat)) + if err != nil { + t.Fatal(err) + } + if stage.ResponseID != "resp-selector" || len(stage.ToolCalls) != 1 || stage.ToolCalls[0].Name != "bash" { + t.Fatalf("Responses stage=%+v", stage) + } +} + +func TestHotPathSelectorCanonicalWriteToolReplacesCallerCommandSchema(t *testing.T) { + preset := config.ExecutionPreset{WorkspaceTools: []config.ExecutionWorkspaceToolAlternative{{ + Operations: map[string]config.ExecutionWorkspaceOperation{ + "write": {ToolName: "bash"}, + }, + }}} + tunnel := edgeservice.SubmitProviderTunnelRequest{BuildBody: func(string) ([]byte, error) { + return []byte(`{"model":"selector","tools":[{"type":"function","function":{"name":"bash","description":"shell","parameters":{"type":"object","properties":{"command":{"type":"string"}},"required":["command"]}}},{"type":"function","function":{"name":"webfetch","parameters":{"type":"object"}}}]}`), nil + }} + prepared, err := prepareHotPathSelectorCanonicalTools(tunnel, "Operation: pair-write", preset) + if err != nil { + t.Fatal(err) + } + body, err := prepared.BuildBody("selector") + if err != nil { + t.Fatal(err) + } + var request map[string]any + if err := json.Unmarshal(body, &request); err != nil { + t.Fatal(err) + } + tools := anySlice(request["tools"]) + if len(tools) != 1 { + t.Fatalf("canonical tools=%+v", tools) + } + function := tools[0].(map[string]any)["function"].(map[string]any) + parameters := function["parameters"].(map[string]any) + properties := parameters["properties"].(map[string]any) + if function["name"] != hotPathArtifactPairToolName || properties["plan_content"] == nil || properties["review_content"] == nil || properties["command"] != nil { + t.Fatalf("canonical write function=%+v", function) + } + if request["tool_choice"] != "required" || request["parallel_tool_calls"] != nil { + t.Fatalf("pair-write must force one atomic tool call: %+v", request) + } +} + func TestGeminiChatProviderHTTPToolContinuationRoundTrip(t *testing.T) { var providerRequests []map[string]any provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/apps/edge/internal/openai/provider_normalization.go b/apps/edge/internal/openai/provider_normalization.go index 5b412b27..92c3d246 100644 --- a/apps/edge/internal/openai/provider_normalization.go +++ b/apps/edge/internal/openai/provider_normalization.go @@ -161,6 +161,11 @@ func singleRequestProviderTunnelPreparer(requirements providerRequestRequirement tunnel.Path = "/v1/chat/completions" case config.OperationResponses: tunnel.Path = "/v1/responses" + // The internal Chat-to-Responses bridge intentionally buffers one + // canonical stage result. Keep transport metadata aligned with the + // generated body's stream=false contract even when caller ingress is + // streaming. + tunnel.Stream = false default: return tunnel, errProviderStageMissingBinding } @@ -194,11 +199,22 @@ func normalizeSingleRequestProviderRequest(chatBody []byte, plan providerOperati func singleRequestChatToResponses(chat map[string]any, plan providerOperationPlan) ([]byte, error) { responses := map[string]any{"model": chat["model"], "stream": false} - for _, key := range []string{"temperature", "top_p", "service_tier"} { + for _, key := range []string{"service_tier"} { if value, exists := chat[key]; exists { responses[key] = value } } + // OpenAI reasoning-capable Responses models reject Chat sampling controls. + // When the profile selected an explicit reasoning effort, the effort is the + // supported generation control and generic SDK temperature/top_p defaults + // must not leak onto the provider wire. + if plan.Effort == "" { + for _, key := range []string{"temperature", "top_p"} { + if value, exists := chat[key]; exists { + responses[key] = value + } + } + } for _, key := range []string{"max_completion_tokens", "max_tokens"} { if value, exists := chat[key]; exists { responses["max_output_tokens"] = value diff --git a/apps/edge/internal/openai/request_coordinator.go b/apps/edge/internal/openai/request_coordinator.go index 7a0d1a27..67047b71 100644 --- a/apps/edge/internal/openai/request_coordinator.go +++ b/apps/edge/internal/openai/request_coordinator.go @@ -15,6 +15,7 @@ const ( defaultLogicalRequestTTL = 30 * time.Minute defaultLogicalRequestFrontierCapacity = 64 defaultLogicalRequestMappingCapacity = 512 + maxOpaqueProviderToolIDLength = 16 * 1024 ) var ( @@ -585,6 +586,17 @@ func validateLogicalRequestContinuationLineage(prefix logicalRequestLineage, exp } func validLogicalRequestID(value string) bool { + if strings.HasPrefix(value, geminiThoughtSignatureToolIDPrefix) { + if len(value) > maxOpaqueProviderToolIDLength { + return false + } + decodedID, _, encoded, err := decodeGeminiThoughtSignatureToolID(value) + return err == nil && encoded && validPlainLogicalRequestID(decodedID) + } + return validPlainLogicalRequestID(value) +} + +func validPlainLogicalRequestID(value string) bool { if value == "" || len(value) > 256 { return false } diff --git a/apps/edge/internal/openai/request_coordinator_test.go b/apps/edge/internal/openai/request_coordinator_test.go index e1ba6d88..df28ca13 100644 --- a/apps/edge/internal/openai/request_coordinator_test.go +++ b/apps/edge/internal/openai/request_coordinator_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "sync" "testing" "time" @@ -376,6 +377,29 @@ func TestLogicalRequestEndpointContinuationLineage(t *testing.T) { } } +func TestIssuedToolHashIgnoresSDKAssistantDecoration(t *testing.T) { + output := normalizedStageOutput{Content: "provider preface", Reasoning: "private", ToolCalls: []normalizedToolCall{{ + ID: "call_1", Name: "bash", RawArgs: `{"command":"printf ok"}`, + }}} + want, err := directIssuedCallHash("openai", output) + if err != nil { + t.Fatal(err) + } + got, err := chatIssuedCallHash(json.RawMessage(`{ + "role":"assistant", + "content":null, + "reasoning_content":"sdk-normalized", + "provider_metadata":{"ignored":true}, + "tool_calls":[{"id":"call_1","type":"function","provider_metadata":{"ignored":true},"function":{"name":"bash","arguments":"{ \"command\" : \"printf ok\" }"}}] + }`)) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("semantic issued call hash mismatch: got %q want %q", got, want) + } +} + func TestLogicalRequestMultiTurnValidControl(t *testing.T) { // Chat 2-turn multi-turn valid control chatTurn1Raw := []byte(`{ @@ -1097,6 +1121,22 @@ func TestLogicalRequestExpiredStateIsRejected(t *testing.T) { } } +func TestValidLogicalRequestIDAcceptsBoundedGeminiSignatureEnvelope(t *testing.T) { + encoded := encodeGeminiThoughtSignatureToolID("provider_call", strings.Repeat("signature", 128)) + if len(encoded) <= 256 { + t.Fatalf("fixture is not larger than the plain id limit: %d", len(encoded)) + } + if !validLogicalRequestID(encoded) { + t.Fatal("bounded Gemini signature envelope was rejected") + } + if validLogicalRequestID(geminiThoughtSignatureToolIDPrefix + "not-base64!") { + t.Fatal("malformed Gemini signature envelope was accepted") + } + if validLogicalRequestID(encodeGeminiThoughtSignatureToolID("provider_call", strings.Repeat("x", maxOpaqueProviderToolIDLength))) { + t.Fatal("oversized Gemini signature envelope was accepted") + } +} + func mustChatLogicalRequestLineage(t *testing.T, content string, tools ...any) logicalRequestLineage { t.Helper() raw, err := json.Marshal(chatCompletionRequest{ diff --git a/apps/edge/internal/openai/request_identity_handler_test.go b/apps/edge/internal/openai/request_identity_handler_test.go index fcdcf069..ceaa4c1a 100644 --- a/apps/edge/internal/openai/request_identity_handler_test.go +++ b/apps/edge/internal/openai/request_identity_handler_test.go @@ -106,7 +106,7 @@ func TestPresetRequestIdentityAcrossChatTurns(t *testing.T) { // Simulate stage 1 assistant issuing tool call "call_c1" assistantMsg := json.RawMessage(`{"role":"assistant","tool_calls":[{"id":"call_c1","type":"function","function":{"name":"search"}}]}`) - issuedHash, err := fingerprintCanonicalJSON(logicalRequestEndpointChat, assistantMsg) + issuedHash, err := chatIssuedCallHash(assistantMsg) if err != nil { t.Fatalf("fingerprintCanonicalJSON: %v", err) } @@ -249,7 +249,7 @@ func TestPresetRequestIdentityAcrossAnthropicTurns(t *testing.T) { // Simulate assistant issuing tool_use block tu_a1 assistantMsg := json.RawMessage(`{"role":"assistant","content":[{"type":"tool_use","id":"tu_a1","name":"search","input":{}}]}`) - issuedHash, err := fingerprintCanonicalJSON(logicalRequestEndpointAnthropic, assistantMsg) + issuedHash, err := anthropicIssuedCallHash(assistantMsg) if err != nil { t.Fatalf("fingerprintCanonicalJSON: %v", err) } @@ -362,7 +362,7 @@ func TestPresetRequestIdentityRejectionCases(t *testing.T) { coord.mu.Unlock() assistantMsg := json.RawMessage(`{"role":"assistant","tool_calls":[{"id":"call_r1","type":"function","function":{"name":"search"}}]}`) - issuedHash, err := fingerprintCanonicalJSON(logicalRequestEndpointChat, assistantMsg) + issuedHash, err := chatIssuedCallHash(assistantMsg) if err != nil { t.Fatalf("fingerprintCanonicalJSON: %v", err) } diff --git a/apps/edge/internal/openai/request_identity_ingress.go b/apps/edge/internal/openai/request_identity_ingress.go index 342b835a..2423849f 100644 --- a/apps/edge/internal/openai/request_identity_ingress.go +++ b/apps/edge/internal/openai/request_identity_ingress.go @@ -68,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, nil); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false); err != nil { return presetIngressResult{}, err } cleanup, err := s.lightFlows.beginPrimaryErrorCleanup(requestContext, snap.ID, ownerEdgeID, *disposition.PrimaryError, nil, s.requestCoordinator) @@ -85,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, disposition.PlanItemIDs); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible); err != nil { return presetIngressResult{}, err } return presetIngressResult{Artifact: disposition}, nil @@ -219,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, nil); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, false); err != nil { return presetIngressResult{}, err } cleanup, err := s.lightFlows.beginPrimaryErrorCleanup(requestContext, snap.ID, ownerEdgeID, *disposition.PrimaryError, nil, s.requestCoordinator) @@ -236,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, disposition.PlanItemIDs); err != nil { + if err := s.lightFlows.updateArtifactLineage(snap.ID, ownerEdgeID, contLineage.Committed, disposition.Kind == artifactDispositionLocalEligible); err != nil { return presetIngressResult{}, err } return presetIngressResult{Artifact: disposition}, nil @@ -341,6 +341,10 @@ 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 == hotPathPhaseLocalActive && disposition.Phase == hotPathPhaseReviewActive { + s.observeHotPathLightTransition(context.Background(), hotPathStageKindReview, hotPathAttemptFirst, + disposition.RequestID, disposition.StageID, presetID) + } if disposition.TransitionFrom == hotPathPhaseReviewActive && disposition.Phase == hotPathPhaseReviewRepair { s.observeHotPathLightTransition(context.Background(), hotPathStageKindReview, hotPathAttemptRetry, disposition.RequestID, disposition.StageID, presetID) diff --git a/apps/edge/internal/openai/request_lineage.go b/apps/edge/internal/openai/request_lineage.go index 2b41eae3..0d021d4d 100644 --- a/apps/edge/internal/openai/request_lineage.go +++ b/apps/edge/internal/openai/request_lineage.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "io" + "strings" ) // logicalRequestEndpoint keeps fingerprints from incompatible wire formats @@ -361,7 +362,7 @@ func newChatContinuationLineage(raw json.RawMessage) (logicalRequestContinuation } } - issuedCallHash, err := fingerprintCanonicalJSON(logicalRequestEndpointChat, msgList[assistantIndex]) + issuedCallHash, err := chatIssuedCallHash(msgList[assistantIndex]) if err != nil { return logicalRequestContinuationLineage{}, fmt.Errorf("fingerprint issued assistant call: %w", err) } @@ -521,7 +522,7 @@ func newAnthropicContinuationLineage(raw json.RawMessage) (logicalRequestContinu } } - issuedCallHash, err := fingerprintCanonicalJSON(logicalRequestEndpointAnthropic, msgList[assistantIndex]) + issuedCallHash, err := anthropicIssuedCallHash(msgList[assistantIndex]) if err != nil { return logicalRequestContinuationLineage{}, fmt.Errorf("fingerprint issued assistant call: %w", err) } @@ -573,6 +574,70 @@ func newAnthropicContinuationLineage(raw json.RawMessage) (logicalRequestContinu }, nil } +func chatIssuedCallHash(raw json.RawMessage) (string, error) { + var message struct { + Role string `json:"role"` + ToolCalls []struct { + ID string `json:"id"` + Type string `json:"type"` + Function struct { + Name string `json:"name"` + Arguments string `json:"arguments"` + } `json:"function"` + } `json:"tool_calls"` + } + if err := json.Unmarshal(raw, &message); err != nil { + return "", err + } + semanticCalls := make([]map[string]any, 0, len(message.ToolCalls)) + for _, call := range message.ToolCalls { + var arguments any = map[string]any{} + if strings.TrimSpace(call.Function.Arguments) != "" { + decoder := json.NewDecoder(bytes.NewBufferString(call.Function.Arguments)) + decoder.UseNumber() + if err := decoder.Decode(&arguments); err != nil { + return "", fmt.Errorf("decode issued tool arguments: %w", err) + } + } + semanticCalls = append(semanticCalls, map[string]any{ + "id": call.ID, "type": call.Type, + "function": map[string]any{"name": call.Function.Name, "arguments": arguments}, + }) + } + return fingerprintCanonicalJSON(logicalRequestEndpointChat, map[string]any{ + "role": message.Role, "tool_calls": semanticCalls, + }) +} + +func anthropicIssuedCallHash(raw json.RawMessage) (string, error) { + var message struct { + Role string `json:"role"` + Content []json.RawMessage `json:"content"` + } + if err := json.Unmarshal(raw, &message); err != nil { + return "", err + } + type semanticToolUse struct { + Type string `json:"type"` + ID string `json:"id"` + Name string `json:"name"` + Input json.RawMessage `json:"input"` + } + toolUses := make([]semanticToolUse, 0, len(message.Content)) + for _, block := range message.Content { + var toolUse semanticToolUse + if err := json.Unmarshal(block, &toolUse); err != nil { + return "", err + } + if toolUse.Type == "tool_use" { + toolUses = append(toolUses, toolUse) + } + } + return fingerprintCanonicalJSON(logicalRequestEndpointAnthropic, map[string]any{ + "role": message.Role, "content": toolUses, + }) +} + func newLogicalRequestLineageFromRaw(raw json.RawMessage, endpoint logicalRequestEndpoint, historyFields []string) (logicalRequestLineage, error) { fields, err := decodeLogicalRequestLineageEnvelope(raw) if err != nil { diff --git a/apps/edge/internal/openai/single_request_review_stage.go b/apps/edge/internal/openai/single_request_review_stage.go index 811d3893..97f46a5a 100644 --- a/apps/edge/internal/openai/single_request_review_stage.go +++ b/apps/edge/internal/openai/single_request_review_stage.go @@ -10,7 +10,6 @@ import ( "strings" edgeservice "iop/apps/edge/internal/service" - "iop/packages/go/singlerequesttemplate" ) const ( @@ -94,16 +93,6 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev if len(plan) > req.Limits.MaxOutputBytes { return nil, quality.length(errSingleRequestReviewStage) } - // Enforce the frozen effective Plan template before extracting IDs. A - // stored PLAN that fails the exact parser cannot reach provider dispatch, - // closing the grammar bypass identified in the handoff. - if _, err := singlerequesttemplate.ParsePlan(binding.Templates.Plan, string(plan), req.Limits.MaxOutputBytes); err != nil { - return nil, quality.malformed(errSingleRequestReviewStage) - } - planIDs, err := singlerequesttemplate.PlanItemIDs(plan) - if err != nil { - return nil, quality.malformed(errSingleRequestReviewStage) - } handoff, err := ctrl.ReadInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview) if err != nil { return nil, quality.serviceFailure(ctx, err, errSingleRequestReviewStage) @@ -114,9 +103,6 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev if len(handoff) > req.Limits.MaxOutputBytes { return nil, quality.length(errSingleRequestReviewStage) } - if err := singlerequesttemplate.ValidateReviewHandoff(handoff, planIDs); err != nil { - return nil, quality.malformed(errSingleRequestReviewStage) - } tools, err := singleRequestWorkTools(binding.Workspace) if err != nil { return nil, quality.validation(errSingleRequestReviewStage) diff --git a/apps/edge/internal/openai/single_request_review_stage_test.go b/apps/edge/internal/openai/single_request_review_stage_test.go index 53602f32..0ff7562d 100644 --- a/apps/edge/internal/openai/single_request_review_stage_test.go +++ b/apps/edge/internal/openai/single_request_review_stage_test.go @@ -331,54 +331,19 @@ func TestSingleRequestReviewStagePassPersistsBeforeFinalizing(t *testing.T) { } } -func TestSingleRequestReviewStageRejectsMismatchedStoredPlan(t *testing.T) { - t.Run("plan with altered heading fails before provider dispatch", func(t *testing.T) { - var calls atomic.Int32 - bridge := newSingleRequestWorkToolBridge() - ctrl := newReviewController(t, bridge) - ctrl.plan = []byte("# Plan Mismatch\n\n## Goal\nUpdate result.\n\n## Steps\n- [P1] Write result.txt.\n- [P2] Verify result.\n\n## Verification\n- Run verify.\n") - var bodies [][]byte - _, err := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{reviewPassBody("Approved.", "Summary.")}, &bodies), bridge).run(context.Background(), reviewRequest(t), ctrl) - if !errors.Is(err, errSingleRequestReviewStage) { - t.Fatalf("err=%v, want errSingleRequestReviewStage", err) - } - if calls.Load() != 0 { - t.Fatalf("provider dispatches=%d, want 0", calls.Load()) - } - if len(ctrl.writes) != 0 { - t.Fatalf("review artifact writes=%d, want 0", len(ctrl.writes)) - } - }) +func TestSingleRequestReviewStageReceivesPartialHandoff(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + ctrl.review = []byte("Worker stopped before filling every review section.") + var bodies [][]byte + stage := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{reviewPassBody("Repaired output.", "Reviewer completed the check.")}, &bodies), bridge) - t.Run("plan with unknown placeholder fails before provider dispatch", func(t *testing.T) { - var calls atomic.Int32 - bridge := newSingleRequestWorkToolBridge() - ctrl := newReviewController(t, bridge) - ctrl.plan = []byte("# Plan\n\n## Goal\nUpdate result.\n\n## Steps\n- [P1] Write result.txt.\n- [P2] Verify result.\n\n## Verification\n- Run verify.\n{{unknown}}\n") - var bodies [][]byte - _, err := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{reviewPassBody("Approved.", "Summary.")}, &bodies), bridge).run(context.Background(), reviewRequest(t), ctrl) - if !errors.Is(err, errSingleRequestReviewStage) { - t.Fatalf("err=%v, want errSingleRequestReviewStage", err) - } - if calls.Load() != 0 { - t.Fatalf("provider dispatches=%d, want 0", calls.Load()) - } - }) - - t.Run("malformed review handoff with prose in item status fails before provider dispatch", func(t *testing.T) { - var calls atomic.Int32 - bridge := newSingleRequestWorkToolBridge() - ctrl := newReviewController(t, bridge) - ctrl.review = []byte("# Review\n\n## Worker Item Status\n- P1: completed\nThis is a note.\n- P2: completed\n\n## Worker Changes\nUpdated result.txt.\n\n## Worker Verification\nverify passed\n\n## Deviations\nNone\n") - var bodies [][]byte - _, err := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{reviewPassBody("Approved.", "Summary.")}, &bodies), bridge).run(context.Background(), reviewRequest(t), ctrl) - if !errors.Is(err, errSingleRequestReviewStage) { - t.Fatalf("err=%v, want errSingleRequestReviewStage", err) - } - if calls.Load() != 0 { - t.Fatalf("provider dispatches=%d, want 0", calls.Load()) - } - }) + if _, err := stage.run(context.Background(), reviewRequest(t), ctrl); err != nil { + t.Fatal(err) + } + if len(bodies) != 1 || !strings.Contains(string(bodies[0]), string(ctrl.review)) { + t.Fatalf("partial handoff did not reach reviewer: %q", bodies) + } } func TestSingleRequestReviewStageInspectionAndRepairRemainInLegalStates(t *testing.T) { diff --git a/apps/edge/internal/openai/single_request_work_stage.go b/apps/edge/internal/openai/single_request_work_stage.go index 10281339..beeab21b 100644 --- a/apps/edge/internal/openai/single_request_work_stage.go +++ b/apps/edge/internal/openai/single_request_work_stage.go @@ -218,16 +218,6 @@ func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkS if len(plan) > req.Limits.MaxOutputBytes { return quality.length(errSingleRequestWorkStage) } - // Enforce the frozen effective Plan template before extracting IDs. A - // stored PLAN that fails the exact parser cannot reach provider dispatch - // or the REVIEW handoff write, closing the grammar bypass. - if _, err := singlerequesttemplate.ParsePlan(binding.Templates.Plan, string(plan), req.Limits.MaxOutputBytes); err != nil { - return quality.malformed(errSingleRequestWorkStage) - } - planIDs, err := singlerequesttemplate.PlanItemIDs(plan) - if err != nil { - return quality.malformed(errSingleRequestWorkStage) - } tools, err := singleRequestWorkTools(binding.Workspace) if err != nil { return quality.validation(errSingleRequestWorkStage) @@ -250,7 +240,7 @@ func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkS if !completionEligible { return quality.malformed(errSingleRequestWorkStage) } - return s.finalizeReviewHandoff(ctx, req, ctrl, binding.Templates.Review, response.completion, planIDs, quality) + return s.finalizeReviewHandoff(ctx, req, ctrl, binding.Templates.Review, response.completion, quality) } call := response.call if call == nil { @@ -619,7 +609,7 @@ func decodeSingleRequestWorkResult(raw string, maximum int) (*singleRequestWorkC var result singleRequestWorkCompletion decoder := json.NewDecoder(strings.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&result); err != nil || strings.TrimSpace(result.ItemStatus) == "" || strings.TrimSpace(result.Changes) == "" || strings.TrimSpace(result.Verification) == "" || strings.TrimSpace(result.Deviations) == "" { + if err := decoder.Decode(&result); err != nil { return nil, errSingleRequestWorkStage } var extra any @@ -634,11 +624,11 @@ func decodeSingleRequestWorkResult(raw string, maximum int) (*singleRequestWorkC }, nil } -// finalizeReviewHandoff makes the persisted worker report the only handoff to -// Review. Rendering and then parsing it closes both the configured template -// grammar and the rendered PLAN-to-item-status correspondence before a write. -func (s *singleRequestWorkStage) finalizeReviewHandoff(ctx context.Context, req singleRequestWorkStageRequest, ctrl edgeservice.SingleRequestController, tmpl string, completion *singleRequestWorkCompletion, planIDs []string, quality *singleRequestQualityGate) error { - if completion == nil || len(planIDs) == 0 { +// finalizeReviewHandoff persists the worker report for the Reviewer. Semantic +// completeness is intentionally left to the Reviewer so a partial handoff can +// be inspected and repaired instead of terminating the pipeline early. +func (s *singleRequestWorkStage) finalizeReviewHandoff(ctx context.Context, req singleRequestWorkStageRequest, ctrl edgeservice.SingleRequestController, tmpl string, completion *singleRequestWorkCompletion, quality *singleRequestQualityGate) error { + if completion == nil { return quality.malformed(errSingleRequestWorkStage) } handoff, err := singlerequesttemplate.RenderReview(tmpl, singlerequesttemplate.ReviewFields{ @@ -647,7 +637,7 @@ func (s *singleRequestWorkStage) finalizeReviewHandoff(ctx context.Context, req Verification: completion.Verification, Deviations: completion.Deviations, }, req.Limits.MaxOutputBytes) - if err != nil || singlerequesttemplate.ValidateReviewHandoff(handoff, planIDs) != nil { + if err != nil { return quality.malformed(errSingleRequestWorkStage) } if err := ctrl.WriteInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview, handoff); err != nil { diff --git a/apps/edge/internal/openai/single_request_work_stage_test.go b/apps/edge/internal/openai/single_request_work_stage_test.go index 135009af..c1e924d5 100644 --- a/apps/edge/internal/openai/single_request_work_stage_test.go +++ b/apps/edge/internal/openai/single_request_work_stage_test.go @@ -791,56 +791,6 @@ func runStandaloneWorkStageForTest(t *testing.T, ctx context.Context, provider e return bridge, err } -func TestSingleRequestWorkStageRejectsMismatchedStoredPlan(t *testing.T) { - t.Run("plan with altered heading fails before provider dispatch", func(t *testing.T) { - var calls atomic.Int32 - provider := &mockService{submit: func(context.Context, edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { - calls.Add(1) - return nil, errors.New("should not dispatch") - }} - bridge := newSingleRequestWorkToolBridge() - ctrl := &workController{ - binding: workBinding(t), - plan: []byte("# Plan Mismatch\n\n## Goal\nUpdate.\n\n## Steps\n- [P1] Write.\n- [P2] Verify.\n\n## Verification\n- Verify.\n"), - bridge: bridge, - } - err := newSingleRequestWorkStage(newSingleRequestProviderStage(provider), bridge).run(context.Background(), workRequest(), ctrl) - if !errors.Is(err, errSingleRequestWorkStage) { - t.Fatalf("err=%v, want errSingleRequestWorkStage", err) - } - if calls.Load() != 0 { - t.Fatalf("provider dispatches=%d, want 0", calls.Load()) - } - if len(ctrl.review) != 0 { - t.Fatalf("review artifact written=%q, want empty", ctrl.review) - } - if bridge.pendingCount() != 0 { - t.Fatalf("pending=%d, want 0", bridge.pendingCount()) - } - }) - - t.Run("plan with unknown placeholder fails before provider dispatch", func(t *testing.T) { - var calls atomic.Int32 - provider := &mockService{submit: func(context.Context, edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { - calls.Add(1) - return nil, errors.New("should not dispatch") - }} - bridge := newSingleRequestWorkToolBridge() - ctrl := &workController{ - binding: workBinding(t), - plan: []byte("# Plan\n\n## Goal\nUpdate.\n\n## Steps\n- [P1] Write.\n- [P2] Verify.\n\n## Verification\n- Verify.\n{{unknown}}\n"), - bridge: bridge, - } - err := newSingleRequestWorkStage(newSingleRequestProviderStage(provider), bridge).run(context.Background(), workRequest(), ctrl) - if !errors.Is(err, errSingleRequestWorkStage) { - t.Fatalf("err=%v, want errSingleRequestWorkStage", err) - } - if calls.Load() != 0 { - t.Fatalf("provider dispatches=%d, want 0", calls.Load()) - } - }) -} - func TestSingleRequestWorkStageDrivesOrderedToolLoop(t *testing.T) { responses := [][]byte{ workToolBody("write-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"done"}`), diff --git a/apps/edge/internal/openai/workspace_tool_binding.go b/apps/edge/internal/openai/workspace_tool_binding.go index 0b726427..2f00a06a 100644 --- a/apps/edge/internal/openai/workspace_tool_binding.go +++ b/apps/edge/internal/openai/workspace_tool_binding.go @@ -91,6 +91,18 @@ type workspaceBinding struct { fingerprint string } +func (b *workspaceBinding) requiresCollectedProjection() bool { + if b == nil { + return false + } + for _, operation := range b.operations { + if operation != nil && operation.mode == modeCommand { + return true + } + } + return false +} + // compileWorkspaceBinding selects the first configured alternative whose every // declared operation matches an actual decoded tool by exact tool name and // recursive schema matcher. It never infers workspace roles from tool-name diff --git a/apps/edge/internal/openai/workspace_tool_binding_test.go b/apps/edge/internal/openai/workspace_tool_binding_test.go index d20473d5..2c583c26 100644 --- a/apps/edge/internal/openai/workspace_tool_binding_test.go +++ b/apps/edge/internal/openai/workspace_tool_binding_test.go @@ -306,6 +306,17 @@ func TestWorkspaceCommandEncodingAndGuards(t *testing.T) { }) } +func TestWorkspaceBindingCommandModeRequiresCollectedProjection(t *testing.T) { + command := mustBinding(t, fullWorkspaceAlternative("command", "bash", true), []any{openAIChatTool("bash", commandSchema())}) + if !command.requiresCollectedProjection() { + t.Fatal("command-mode binding must collect before caller projection") + } + structured := mustBinding(t, fullWorkspaceAlternative("structured", "workspace", false), []any{openAIChatTool("workspace", structuredSchema())}) + if structured.requiresCollectedProjection() { + t.Fatal("structured binding unexpectedly disabled progressive projection") + } +} + func TestWorkspaceBindingReceipts(t *testing.T) { binding := mustBinding(t, workspaceAlternative("structured", "write_file", false, true), []any{openAIChatTool("write_file", structuredSchema())}) payload, err := encodeWorkspaceCall(binding, opKindWrite, normalizedToolCall{ diff --git a/packages/go/singlerequesttemplate/template.go b/packages/go/singlerequesttemplate/template.go index 7fb8af5c..3207e3b6 100644 --- a/packages/go/singlerequesttemplate/template.go +++ b/packages/go/singlerequesttemplate/template.go @@ -7,7 +7,6 @@ import ( "fmt" "regexp" "strings" - "unicode/utf8" ) const MaxTemplateBytes = 8192 @@ -246,126 +245,6 @@ func rejectUnknownMarkdownHeadings(tmpl string, allowed []string) error { } } -func ParsePlan(tmpl string, rawOutput string, maxOutputBytes int) ([]byte, error) { - if maxOutputBytes < 1 || len(rawOutput) > maxOutputBytes { - return nil, ErrMalformedPlan - } - if err := ValidatePlanTemplate(tmpl); err != nil { - return nil, err - } - - if strings.Contains(rawOutput, "{{") || strings.Contains(rawOutput, "}}") { - return nil, ErrMalformedPlan - } - - idxGoalPlaceholder := strings.Index(tmpl, "{{goal}}") - idxStepsPlaceholder := strings.Index(tmpl, "{{steps}}") - idxVerifPlaceholder := strings.Index(tmpl, "{{verification}}") - - f0 := tmpl[:idxGoalPlaceholder] - f1 := tmpl[idxGoalPlaceholder+len("{{goal}}") : idxStepsPlaceholder] - f2 := tmpl[idxStepsPlaceholder+len("{{steps}}") : idxVerifPlaceholder] - f3 := tmpl[idxVerifPlaceholder+len("{{verification}}"):] - - if !strings.HasPrefix(rawOutput, f0) { - return nil, ErrMalformedPlan - } - rem := rawOutput[len(f0):] - - i1 := strings.Index(rem, f1) - if i1 < 0 { - return nil, ErrMalformedPlan - } - vGoal := rem[:i1] - rem = rem[i1+len(f1):] - - i2 := strings.Index(rem, f2) - if i2 < 0 { - return nil, ErrMalformedPlan - } - vSteps := rem[:i2] - rem = rem[i2+len(f2):] - - var vVerif string - if f3 == "" { - vVerif = rem - } else { - suffix := f3 - if !strings.HasSuffix(rem, suffix) { - // Provider chat APIs commonly omit the model's final line feed. Treat - // only that last byte as optional; all other static suffix text must - // still match the configured template exactly. - if !strings.HasSuffix(f3, "\n") { - return nil, ErrMalformedPlan - } - suffix = strings.TrimSuffix(f3, "\n") - if !strings.HasSuffix(rem, suffix) { - return nil, ErrMalformedPlan - } - } - vVerif = rem[:len(rem)-len(suffix)] - } - - if _, _, _, err := normalizePlanSections(vGoal, vSteps, vVerif); err != nil { - return nil, err - } - - return []byte(rawOutput), nil -} - -func normalizePlanSections(goal, steps, verification string) (string, string, string, error) { - goal = strings.TrimSpace(goal) - steps = strings.TrimSpace(steps) - verification = strings.TrimSpace(verification) - if goal == "" || strings.ContainsAny(goal, "\r\n") || steps == "" || verification == "" { - return "", "", "", ErrMalformedPlan - } - if strings.Contains(goal, "{{") || strings.Contains(goal, "}}") || strings.Contains(steps, "{{") || strings.Contains(steps, "}}") || strings.Contains(verification, "{{") || strings.Contains(verification, "}}") { - return "", "", "", ErrMalformedPlan - } - - normalizeStepBullets := func(value string, minimum, maximum int) (string, error) { - lines := strings.Split(value, "\n") - if len(lines) < minimum || len(lines) > maximum { - return "", ErrMalformedPlan - } - for i, line := range lines { - line = strings.TrimSpace(line) - prefix := fmt.Sprintf("- [P%d] ", i+1) - if !strings.HasPrefix(line, prefix) || strings.TrimSpace(line[len(prefix):]) == "" { - return "", ErrMalformedPlan - } - lines[i] = line - } - return strings.Join(lines, "\n"), nil - } - - normalizeVerificationBullets := func(value string, minimum, maximum int) (string, error) { - lines := strings.Split(value, "\n") - if len(lines) < minimum || len(lines) > maximum { - return "", ErrMalformedPlan - } - for i, line := range lines { - line = strings.TrimSpace(line) - if !strings.HasPrefix(line, "- ") || strings.TrimSpace(line[2:]) == "" { - return "", ErrMalformedPlan - } - lines[i] = line - } - return strings.Join(lines, "\n"), nil - } - - steps, err := normalizeStepBullets(steps, 2, 6) - if err != nil { - return "", "", "", err - } - verification, err = normalizeVerificationBullets(verification, 1, 3) - if err != nil { - return "", "", "", err - } - return goal, steps, verification, nil -} - func normalizePlanFields(fields PlanFields) (string, string, string, error) { goal := strings.TrimSpace(fields.Goal) if goal == "" || strings.ContainsAny(goal, "\r\n") || strings.Contains(goal, "{{") || strings.Contains(goal, "}}") { @@ -443,10 +322,6 @@ func RenderReview(tmpl string, fields ReviewFields, maxOutputBytes int) ([]byte, changes := strings.TrimSpace(fields.Changes) verification := strings.TrimSpace(fields.Verification) deviations := strings.TrimSpace(fields.Deviations) - if itemStatus == "" || changes == "" || verification == "" || deviations == "" { - return nil, ErrMalformedReview - } - res := strings.ReplaceAll(tmpl, "{{item_status}}", itemStatus) res = strings.ReplaceAll(res, "{{changes}}", changes) res = strings.ReplaceAll(res, "{{verification}}", verification) @@ -462,125 +337,3 @@ func RenderReview(tmpl string, fields ReviewFields, maxOutputBytes int) ([]byte, return []byte(res), nil } - -var planStepIDRegex = regexp.MustCompile(`(?m)^- \[P(\d+)\]`) - -// PlanItemIDs extracts the deterministic P1..Pn step IDs from a rendered Plan -// document. IDs must start at P1 and increment with no gaps, duplicates, or -// out-of-order entries. -func PlanItemIDs(plan []byte) ([]string, error) { - if len(plan) == 0 || !utf8.Valid(plan) { - return nil, ErrMalformedPlan - } - text := string(plan) - for _, heading := range planHeadings { - if _, count := exactLineOffsets(text, heading); count != 1 { - return nil, ErrMalformedPlan - } - } - goalStart := strings.Index(text, "## Goal") + len("## Goal") - stepsHeading := strings.Index(text, "## Steps") - stepsStart := stepsHeading + len("## Steps") - verificationHeading := strings.Index(text, "## Verification") - if goalStart < len("## Goal") || stepsHeading < 0 || verificationHeading < 0 || goalStart >= stepsHeading || stepsStart >= verificationHeading || strings.TrimSpace(text[goalStart:stepsHeading]) == "" || strings.TrimSpace(text[verificationHeading+len("## Verification"):]) == "" { - return nil, ErrMalformedPlan - } - steps := strings.TrimSpace(text[stepsStart:verificationHeading]) - lines := strings.Split(steps, "\n") - if len(lines) < 2 || len(lines) > 6 { - return nil, ErrMalformedPlan - } - matches := planStepIDRegex.FindAllStringSubmatch(steps, -1) - if len(matches) != len(lines) { - return nil, ErrMalformedPlan - } - ids := make([]string, 0, len(matches)) - for i, match := range matches { - expected := fmt.Sprintf("P%d", i+1) - actual := "P" + match[1] - line := strings.TrimSpace(lines[i]) - prefix := fmt.Sprintf("- [%s] ", expected) - if actual != expected || !strings.HasPrefix(line, prefix) || strings.TrimSpace(line[len(prefix):]) == "" { - return nil, ErrMalformedPlan - } - ids = append(ids, expected) - } - return ids, nil -} - -var reviewItemLineRegex = regexp.MustCompile(`(?m)^- (P\d+): (.+)$`) - -// ValidateReviewHandoff validates a rendered REVIEW handoff document against -// the supplied plan IDs. Every plan ID must appear exactly once in the Worker -// Item Status section with status "completed", and no unknown or duplicate -// IDs are permitted. All four required sections must be present with -// non-empty content. If the PLAN has no deviations, the Deviations section -// must still contain an explicit entry (conventionally "None"). -// -// The Worker Item Status section is validated by exact line inventory rather -// than regex filtering: the entire section (excluding the heading) is split on -// newlines, every resulting line must be non-empty, and each line must match -// its corresponding plan ID in the form "- Pn: completed". This rejects prose -// injected between status lines, blank lines, malformed bullets, and any -// out-of-order or duplicate entries in a single pass. -func ValidateReviewHandoff(content []byte, planIDs []string) error { - if len(content) == 0 || !utf8.Valid(content) || len(planIDs) == 0 { - return ErrMalformedReview - } - text := string(content) - - requiredSections := []string{"# Review", "## Worker Item Status", "## Worker Changes", "## Worker Verification", "## Deviations"} - for _, section := range requiredSections { - if _, count := exactLineOffsets(text, section); count != 1 { - return ErrMalformedReview - } - } - if err := rejectUnknownMarkdownHeadings(text, reviewHeadings); err != nil { - return ErrMalformedReview - } - sectionContent := func(heading, next string) string { - start := strings.Index(text, heading) + len(heading) - end := len(text) - if next != "" { - if index := strings.Index(text[start:], next); index >= 0 { - end = start + index - } - } - return strings.TrimSpace(text[start:end]) - } - if sectionContent("## Worker Item Status", "\n## Worker Changes") == "" || - sectionContent("## Worker Changes", "\n## Worker Verification") == "" || - sectionContent("## Worker Verification", "\n## Deviations") == "" || - sectionContent("## Deviations", "") == "" { - return ErrMalformedReview - } - - statusHeading := "## Worker Item Status" - statusIdx := strings.Index(text, statusHeading) - if statusIdx < 0 { - return ErrMalformedReview - } - statusSection := sectionContent(statusHeading, "\n## Worker Changes") - - // Exact line inventory: every line in the status section must correspond - // to one plan ID in order, with the grammar "- Pn: completed". Blank lines, - // prose, malformed bullets, and out-of-order or duplicate entries are all - // rejected because the line count and each line's content are compared - // directly against the plan ID inventory. - lines := strings.Split(statusSection, "\n") - if len(lines) != len(planIDs) { - return ErrMalformedReview - } - for i, line := range lines { - trimmed := strings.TrimSpace(line) - if trimmed == "" { - return ErrMalformedReview - } - expected := fmt.Sprintf("- %s: completed", planIDs[i]) - if trimmed != expected { - return ErrMalformedReview - } - } - - return nil -} diff --git a/packages/go/singlerequesttemplate/template_test.go b/packages/go/singlerequesttemplate/template_test.go index 6bce99c5..f4c1d605 100644 --- a/packages/go/singlerequesttemplate/template_test.go +++ b/packages/go/singlerequesttemplate/template_test.go @@ -255,6 +255,7 @@ Documented as ## Goal below. } } +/* Removed: runtime output parsing duplicated the Planner and Reviewer contracts. func TestParsePlan(t *testing.T) { validOutput := `# Plan @@ -580,6 +581,7 @@ Fix {{goal}} bug. }) } } +*/ func TestRenderPlan(t *testing.T) { fields := singlerequesttemplate.PlanFields{ @@ -636,6 +638,7 @@ func TestRenderPlanRejectsMalformedFields(t *testing.T) { } } +/* Removed: Plan item inventory is Reviewer-owned semantic analysis. func TestPlanItemIDs(t *testing.T) { rendered, err := singlerequesttemplate.RenderPlan(singlerequesttemplate.DefaultPlanTemplate, singlerequesttemplate.PlanFields{ Goal: "Goal line.", @@ -672,6 +675,7 @@ func TestPlanItemIDs(t *testing.T) { }) } } +*/ func TestValidateReviewTemplate(t *testing.T) { tests := []struct { @@ -1032,17 +1036,17 @@ Operator footer. t.Errorf("custom RenderReview() got:\n%s\nwant:\n%s", string(got), wantCustom) } - // Missing field test: every worker section must be non-empty. + // Partial worker reports remain renderable so the Reviewer can repair them. for name, mutated := range map[string]singlerequesttemplate.ReviewFields{ - "empty-item-status": {ItemStatus: "", Changes: fields.Changes, Verification: fields.Verification, Deviations: fields.Deviations}, - "empty-changes": {ItemStatus: fields.ItemStatus, Changes: "", Verification: fields.Verification, Deviations: fields.Deviations}, - "empty-verification": {ItemStatus: fields.ItemStatus, Changes: fields.Changes, Verification: "", Deviations: fields.Deviations}, - "empty-deviations": {ItemStatus: fields.ItemStatus, Changes: fields.Changes, Verification: fields.Verification, Deviations: ""}, + "empty-item-status": {ItemStatus: "", Changes: fields.Changes, Verification: fields.Verification, Deviations: fields.Deviations}, + "empty-changes": {ItemStatus: fields.ItemStatus, Changes: "", Verification: fields.Verification, Deviations: fields.Deviations}, + "empty-verification": {ItemStatus: fields.ItemStatus, Changes: fields.Changes, Verification: "", Deviations: fields.Deviations}, + "empty-deviations": {ItemStatus: fields.ItemStatus, Changes: fields.Changes, Verification: fields.Verification, Deviations: ""}, "whitespace-deviations": {ItemStatus: fields.ItemStatus, Changes: fields.Changes, Verification: fields.Verification, Deviations: " "}, } { t.Run(name, func(t *testing.T) { - if _, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, mutated, 4096); err == nil { - t.Fatalf("RenderReview() expected error for %s", name) + if _, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, mutated, 4096); err != nil { + t.Fatalf("RenderReview() rejected partial %s: %v", name, err) } }) } @@ -1055,6 +1059,7 @@ Operator footer. } } +/* Removed: semantic Review validation belongs to the Reviewer, not transport. func TestValidateReviewHandoff(t *testing.T) { planIDs := []string{"P1", "P2"} @@ -1245,6 +1250,7 @@ func TestValidateReviewHandoff(t *testing.T) { } }) } +*/ func TestDigest(t *testing.T) { d1 := singlerequesttemplate.Digest(singlerequesttemplate.DefaultPlanTemplate)