diff --git a/agent-contract/inner/edge-config-runtime-refresh.md b/agent-contract/inner/edge-config-runtime-refresh.md index 5a6d3f8e..d55e3c32 100644 --- a/agent-contract/inner/edge-config-runtime-refresh.md +++ b/agent-contract/inner/edge-config-runtime-refresh.md @@ -68,7 +68,7 @@ tracked config에는 public 예시와 기본 구조만 두고, 실제 endpoint/c - `models[].providers`와 `models[].execution_preset`는 상호 배타(one-of)다. 한 `models[]` entry는 정확히 하나만 설정해야 하며, 둘 다 설정하거나 둘 다 비우면 load에서 거부한다. `execution_preset`가 설정된 entry는 provider pool을 갖지 않는 virtual(preset-only) model이며 named execution preset shape에 실행을 위임한다. provider-only budget/token-counter validation은 virtual entry에 적용하지 않는다. - `models[].execution_preset` 값은 앞뒤 공백을 제거해 정규화한다. 공백만 있는 값은 unset으로 처리해 provider-only one-of 규칙을 적용하고, 정규화된 non-empty id는 `execution_presets[]` catalog의 entry로 resolve되어야 한다. dangling reference는 fail-closed로 거부한다. resolve에 성공한 non-empty id는 canonical(trimmed) 형태로 저장되어 downstream lookup이 admission 시점 값과 정확히 일치한다. - `execution_presets[]`는 top-level frozen execution shape catalog이며 `models[].execution_preset`가 참조하는 대상이다. 각 preset의 `selector.model`과 route stage `model`은 기존 `models[].id` catalog를 참조해야 한다. `execution_presets[]` catalog 변경과 `models[].execution_preset` mapping 변경은 모두 live-apply로 분류되며 refresh 이후 새로 시작되는 logical request에만 적용되고 in-flight request에는 영향을 주지 않는다. -- `execution_presets[].single_request`는 operator-owned fixed single-request policy다. 설정 시 preset은 `allowed_modes=["light"]`, `stages=[plan, work, review]`의 승인된 plan→work→review 경로를 고수한다. 절대 상한은 `wall_clock_ms ≤ 1800000`, `timeout_ms ≤ 600000`, `max_tool_iterations ≤ 64`, `max_output_bytes ≤ 16777216`이며 `timeout_ms`는 `wall_clock_ms`를 초과할 수 없다. selector와 plan/review stage는 `reasoning_effort=high`를 강제하고 work stage는 `reasoning_effort`를 선언할 수 없다. `workspace_ref`는 비어있을 수 없으며 raw path, credential, Node id, endpoint를 포함하지 않는다. `templates` 섹션을 통해 optional `plan_file` 및 `review_file` (edge.yaml 상대 경로) 커스텀 Markdown 템플릿을 지정할 수 있으며, load 시점에 8192바이트 상한 및 문법 검증이 수행되고 생략 시 built-in default 템플릿이 적용된다. config refresh diff reporting 시 템플릿 파일 경로나 본문은 노출되지 않고 SHA-256 digest만 보고된다. single_request preset은 `workspace_tools`를 선언할 수 없다. catalog 변경과 mapping 변경은 live-apply로 분류되며 refresh 이후 새로 시작되는 logical request에만 적용된다. admitted single-request binding은 refresh 이후에도 frozen public model, stage binding, workspace reference, limits, effective templates를 유지한다. +- `execution_presets[].single_request`는 operator-owned fixed single-request policy다. 설정 시 preset은 `allowed_modes=["light"]`, `stages=[plan, work, review]`의 승인된 plan→work→review 경로를 고수한다. 절대 상한은 `wall_clock_ms ≤ 1800000`, `timeout_ms ≤ 600000`, `max_tool_iterations ≤ 64`, `max_output_bytes ≤ 16777216`이며 `timeout_ms`는 `wall_clock_ms`를 초과할 수 없다. selector와 plan/review stage는 `reasoning_effort=high`를 강제하고 work stage는 `reasoning_effort`를 선언할 수 없다. `workspace_ref`는 비어있을 수 없으며 raw path, credential, Node id, endpoint를 포함하지 않는다. `templates`의 optional `plan_file`/`review_file`은 edge.yaml 상대 경로의 8192-byte UTF-8 closed grammar이며, PLAN은 deterministic `P1..Pn`, REVIEW는 `Worker Item Status`/`Worker Changes`/`Worker Verification`/`Deviations`만 허용한다. 구형 reviewer-final template은 admission에서 fail closed한다. config refresh diff reporting 시 템플릿 파일 경로나 본문은 노출되지 않고 SHA-256 digest만 보고된다. single_request preset은 `workspace_tools`를 선언할 수 없다. catalog 변경과 mapping 변경은 live-apply로 분류되며 refresh 이후 새로 시작되는 logical request에만 적용된다. admitted single-request binding은 refresh 이후에도 frozen public model, stage binding, workspace reference, limits, effective templates를 유지한다. - `nodes[].providers[]`는 Node 아래 resource/provider catalog다. `category`는 `api`, `cli`, `local_inference` resource kind를 나타낸다. - `nodes[].providers[].type`의 `seulgivibe_claude`와 `seulgivibe_openai`는 runtime type을 `openai_compat`로 정규화한다. Edge가 Node adapter payload를 만들 때 명시 provider label이 없으면 원래 Seulgivibe type alias를 `OpenAICompatAdapterConfig.provider`로 보존한다. - `nodes[].providers[].response_stall_timeout_ms`는 provider-originated response-stall timeout을 밀리초 단위로 선언한다. 양수 값은 그대로 사용되고, 0 또는 생략은 문서화된 기본값 `60000`을 적용한다. 음수 값과 safe duration bound를 초과하는 양수 값은 `NodeProviderConf.Validate()`에서 거부한다. effective 값은 `NodeProviderConf.EffectiveResponseStallTimeoutMS()`에서 계산한다. 이 필드는 config refresh에서 `restart_required`로 분류되며, effective-zero 등가성(생략 vs 명시적 0)은 변경으로 보고되지 않는다. request hard timeout, queue timeout, heartbeat/disconnect, CLI `response_idle_timeout_ms`는 기존 소유권을 유지한다. diff --git a/agent-contract/inner/edge-node-runtime-wire.md b/agent-contract/inner/edge-node-runtime-wire.md index a1a75e80..4d5c3047 100644 --- a/agent-contract/inner/edge-node-runtime-wire.md +++ b/agent-contract/inner/edge-node-runtime-wire.md @@ -178,5 +178,6 @@ Operational projections exclude raw payloads, credentials, caller-controlled ide ## 변경 기록 +- 2026-08-14: PLAN and REVIEW remain the existing closed artifact selectors; no new wire kind was added. Work writes the one validated REVIEW handoff, Review only reads PLAN/REVIEW, and terminal cleanup removes both artifacts. - 2026-08-08: Generalized workspace runtime admission to the closed `darwin|linux` implementation set with exact catalog/host matching before root open while keeping Windows/unknown hosts fail-closed. - 2026-08-07: Added the closed request-owned PLAN/REVIEW artifact read/write family, bounded inventoried Node reads, exact-generation Edge dispatch and response validation, and coordinator-shared lazy open/in-flight cleanup ordering. Provider-specific Plan/Work/Review drivers and actual Claude qualification remain deferred. diff --git a/agent-contract/outer/anthropic-compatible-api.md b/agent-contract/outer/anthropic-compatible-api.md index dd847c27..be406b9e 100644 --- a/agent-contract/outer/anthropic-compatible-api.md +++ b/agent-contract/outer/anthropic-compatible-api.md @@ -118,11 +118,12 @@ Edge-owned internal stage inputs only: - The Plan stage 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 owns the Markdown bullet/newline formatting and - renders the frozen Plan template. The Review template similarly shapes the private - `review.md` artifact rendered from the model's `checks`, `verification`, and - `summary` fields. Provider output never controls either artifact's headings or - static template text. + `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 + 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 template never adds, removes, or renames a Messages request field, a content block, an SSE event, a `stop_reason`, or an error shape, and the final Messages text stays diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/SKILL.md b/agent-ops/skills/common/orchestrate-agent-task-loop/SKILL.md index ba34f8a8..b25ec6f1 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/SKILL.md +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/SKILL.md @@ -48,7 +48,8 @@ Each target has: - an opaque `model` identity; - optional `reasoning_effort`, stored as a separate opaque catalog value rather than embedded in dispatcher code or a literal command argument; - `execution_class`: `local_model` or `cloud_model`; -- optional `selfcheck_required` boolean; +- optional `selfcheck_required` boolean for an unconditional post-worker self-check; +- optional `completion_selfcheck_on_incomplete` boolean that routes a changed, zero-exit worker with incomplete implementation checklist evidence to the self-check stage before charging a generic worker failure; - `runtime.command`: a non-empty argv template executed without a shell; - optional `runtime.resume_command`, `preflight_command`, `environment`, `session_path`, `native_session_monitor`, `session_stall_resume`, `terminal_success`, and `auxiliary_logs`; - optional `runtime.output_format`: `text` or `jsonl`. @@ -101,7 +102,9 @@ Never ask a child to create, edit, or summarize `WORK_LOG.md`; that file is disp Run self-check only when the selected catalog target declares `selfcheck_required=true`. The completing decision, not a fixed agent identity or execution class, determines the requirement. -Treat worker exit `0` as transport completion only. Before marking the worker done, require at least one claimed file or implementation-evidence change and a complete implementation-owned checklist (or concrete blocker evidence). Classify a no-op or incomplete-evidence exit as `generic-error`, apply the same per-target three-error budget, and return persisted `worker_done` state to the worker stage while that contract remains incomplete. Apply that bounded three-attempt target budget to `session-stall` as well, so a repeatedly silent candidate advances instead of consuming the ten-attempt stage recovery budget. +When `completion_selfcheck_on_incomplete=true`, a zero-exit worker that changed claimed files but left implementation checklist evidence incomplete advances to self-check instead of being counted immediately as `generic-error`. This conditional path does not waive the checklist gate, does not accept a no-op worker, and does not run after a complete handoff. The self-check must fill or concretely block the implementation-owned evidence before official review. If a conditional self-check on a target without native resume still leaves the checklist incomplete, fail over once to the next worker candidate with the current workspace instead of fresh-restarting the same target. + +Treat worker exit `0` as transport completion only. Before advancing to official review, require at least one claimed file or implementation-evidence change and a complete implementation-owned checklist (or concrete blocker evidence). Always classify a no-op exit as `generic-error`. Classify incomplete evidence as `generic-error` unless the selected target declares `completion_selfcheck_on_incomplete=true`; that target enters self-check without consuming the worker generic-failure budget. Apply the bounded per-target three-attempt budget to ordinary worker retries and `session-stall`, so a repeatedly silent candidate advances instead of consuming the ten-failure stage recovery budget. Accept self-check completion only when `## Implementation Checklist` or its supported legacy heading contains at least one checkbox and every checkbox has a non-empty value. Run one full pass, then resume the latest successful native context for at most 10 unchecked-item retries when the target supports native resume. Block instead of silently starting a new context when a required persisted context is unavailable. diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/assets/default-execution-catalog.json b/agent-ops/skills/common/orchestrate-agent-task-loop/assets/default-execution-catalog.json index a882f851..400283ec 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/assets/default-execution-catalog.json +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/assets/default-execution-catalog.json @@ -50,12 +50,62 @@ "terminal_success": "agent_end" } }, + "pi-ornith-fast-high": { + "agent": "pi", + "model": "ornith-fast", + "reasoning_effort": "high", + "execution_class": "local_model", + "selfcheck_required": true, + "runtime": { + "command": [ + "pi", + "-p", + "--mode", + "json", + "--approve", + "--provider", + "iop", + "--model", + "{model}", + "--thinking", + "{reasoning_effort}", + "--session-id", + "{session_id}", + "--session-dir", + "{attempt_dir}/pi-sessions", + "{prompt}" + ], + "resume_command": [ + "pi", + "-p", + "--mode", + "json", + "--approve", + "--provider", + "iop", + "--model", + "{model}", + "--thinking", + "{reasoning_effort}", + "--session", + "{resume_session}", + "--session-dir", + "{resume_session_dir}", + "{prompt}" + ], + "output_format": "jsonl", + "session_path": "{attempt_dir}/pi-sessions/*{session_id}*.jsonl", + "native_session_monitor": true, + "terminal_success": "agent_end" + } + }, "opencode-glm-medium": { "agent": "opencode", "model": "glm-5.2", "reasoning_effort": "medium", "execution_class": "cloud_model", "selfcheck_required": false, + "completion_selfcheck_on_incomplete": true, "runtime": { "command": [ "opencode", @@ -107,6 +157,7 @@ "reasoning_effort": "high", "execution_class": "cloud_model", "selfcheck_required": false, + "completion_selfcheck_on_incomplete": true, "runtime": { "command": [ "opencode", @@ -158,6 +209,7 @@ "reasoning_effort": "high", "execution_class": "cloud_model", "selfcheck_required": false, + "completion_selfcheck_on_incomplete": true, "runtime": { "command": [ "opencode", @@ -345,7 +397,7 @@ "reason_codes": ["worker_catalog_lane"] }, "local-G04": { - "candidates": ["pi-ornith-high"], + "candidates": ["pi-ornith-fast-high"], "rule_id": "worker-local-g04-catalog", "policy_priority": 30, "reason_codes": ["worker_catalog_lane"] diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/dispatch.py b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/dispatch.py index d701d60a..3933f8b5 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/dispatch.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/dispatch.py @@ -512,6 +512,7 @@ class AgentSpec: target_id: str | None = None execution_class: str = "cloud_model" selfcheck_required: bool = False + completion_selfcheck_on_incomplete: bool = False reasoning_effort: str | None = None runtime: dict[str, Any] = field(default_factory=dict) @@ -532,6 +533,11 @@ def agent_spec_from_record(record: dict[str, Any]) -> AgentSpec | None: selfcheck_required = record.get("selfcheck_required", False) if not isinstance(selfcheck_required, bool): return None + completion_selfcheck_on_incomplete = record.get( + "completion_selfcheck_on_incomplete", False + ) + if not isinstance(completion_selfcheck_on_incomplete, bool): + return None reasoning_effort = record.get("reasoning_effort") if reasoning_effort is not None and ( not isinstance(reasoning_effort, str) or not reasoning_effort @@ -549,6 +555,7 @@ def agent_spec_from_record(record: dict[str, Any]) -> AgentSpec | None: target_id=target_id, execution_class=execution_class, selfcheck_required=selfcheck_required, + completion_selfcheck_on_incomplete=completion_selfcheck_on_incomplete, reasoning_effort=reasoning_effort, runtime=dict(runtime), ) @@ -842,6 +849,7 @@ class StateStore: "worker_cli": None, "worker_model": None, "selfcheck_done": False, + "completion_selfcheck_pending": False, "blocked": None, "active_stage": None, "active_locator": None, @@ -871,6 +879,7 @@ class StateStore: "worker_cli": None, "worker_model": None, "selfcheck_done": False, + "completion_selfcheck_pending": False, "blocked": None, "active_stage": None, "active_locator": None, @@ -1134,6 +1143,7 @@ class StateStore: retry_failover_pending=False, retry_failover_context=None, blocker_evidence=None, + completion_selfcheck_pending=False, blocked=None, active_stage=None, active_locator=None, @@ -1188,6 +1198,7 @@ class StateStore: value["review_no_progress"] = 0 value["selfcheck_incomplete"] = 0 value["selfcheck_context_locator"] = None + value["completion_selfcheck_pending"] = False value["recovery_failures"] = {} value["stage_failure_budgets"] = {} value["generic_failure_budgets"] = {} @@ -1959,6 +1970,9 @@ def agent_spec_from_decision(decision: dict[str, Any]) -> AgentSpec: target_id=target.catalog_id, execution_class=target.execution_class, selfcheck_required=target.selfcheck_required, + completion_selfcheck_on_incomplete=( + target.completion_selfcheck_on_incomplete + ), reasoning_effort=target.reasoning_effort, runtime=runtime, ) @@ -2347,6 +2361,18 @@ def completing_decision_requires_selfcheck(state: dict[str, Any]) -> bool: return selected.get("selfcheck_required") is True +def completing_decision_allows_completion_selfcheck( + state: dict[str, Any], +) -> bool: + completing = state.get("completing_decision") + if not isinstance(completing, dict): + return False + selected = completing.get("selected") + if not isinstance(selected, dict): + return False + return selected.get("completion_selfcheck_on_incomplete") is True + + def _validated_completing_decision( task: Task, decision: dict[str, Any] ) -> tuple[dict[str, Any], AgentSpec]: @@ -2506,6 +2532,13 @@ def task_stage(task: Task, state: dict[str, Any]) -> str: # worker stage until the implementation-owned review contract is # actually materialized (or contains complete blocker evidence). if implementation_review_errors(task): + if ( + state.get("completion_selfcheck_pending") + and not state.get("selfcheck_done") + and completing_decision_allows_completion_selfcheck(state) + and _completing_decision_is_valid(task, state) + ): + return "selfcheck" return "worker" if not _completing_decision_is_valid(task, state): return "blocked" @@ -3677,6 +3710,9 @@ async def invoke( "target_id": spec.target_id, "execution_class": spec.execution_class, "selfcheck_required": spec.selfcheck_required, + "completion_selfcheck_on_incomplete": ( + spec.completion_selfcheck_on_incomplete + ), "reasoning_effort": spec.reasoning_effort, "runtime": spec.runtime, "agent_process_marker": process_marker, @@ -4257,13 +4293,18 @@ async def invoke( ): worker_errors = implementation_review_errors(task) worker_signature_after = task_signature(workspace, task) - if worker_signature_before == worker_signature_after or worker_errors: + unchanged = worker_signature_before == worker_signature_after + incomplete_without_selfcheck = ( + bool(worker_errors) and not spec.completion_selfcheck_on_incomplete + ) + if unchanged or incomplete_without_selfcheck: failure_class = "generic-error" failure_source = "dispatcher-worker-completion-contract" details = [] - if worker_signature_before == worker_signature_after: + if unchanged: details.append("no claimed file or implementation evidence changed") - details.extend(worker_errors) + if incomplete_without_selfcheck: + details.extend(worker_errors) failure_evidence = "; ".join(details) failure_evidence_source = "dispatcher:worker-completion-contract" try: @@ -5655,6 +5696,13 @@ def _mark_worker_done( _require_same_runtime_identity(expected_spec, worker_cli, worker_model) selected = validated_decision["selected"] execution_class = selected["execution_class"] + completion_selfcheck_pending = bool( + selected.get("completion_selfcheck_on_incomplete") + and implementation_review_errors(task) + ) + selfcheck_pending = bool( + selected["selfcheck_required"] or completion_selfcheck_pending + ) store.update_task( task, worker_done=True, @@ -5662,7 +5710,8 @@ def _mark_worker_done( worker_model=worker_model, completing_decision=validated_decision, execution_class=execution_class, - selfcheck_done=not selected["selfcheck_required"], + selfcheck_done=not selfcheck_pending, + completion_selfcheck_pending=completion_selfcheck_pending, review_no_progress=0, blocked=None, ) @@ -5690,8 +5739,16 @@ async def run_selfcheck( store.update_task(task, blocked=str(exc)) banner("작업차단", task.name, [f"reason={exc}"]) return - if not spec.selfcheck_required: - raise RuntimeError("selfcheck_required가 아닌 route에 selfcheck stage가 배정됐다") + completion_selfcheck_pending = bool( + store.task_state(task).get("completion_selfcheck_pending") + ) + if not spec.selfcheck_required and not ( + completion_selfcheck_pending + and spec.completion_selfcheck_on_incomplete + ): + raise RuntimeError( + "selfcheck 계약이 없는 route에 selfcheck stage가 배정됐다" + ) work_log = milestone_work_log_path(task) banner( "자가검증시작", @@ -5756,7 +5813,7 @@ async def run_selfcheck( ) return while True: - unchecked_items = incomplete_results > 0 + unchecked_items = completion_selfcheck_pending or incomplete_results > 0 success, locator = await run_escalating( workspace, store, @@ -5776,6 +5833,45 @@ async def run_selfcheck( if not errors: break if not spec.native_resume: + if completion_selfcheck_pending: + try: + next_decision = select_execution_decision( + task, + stage="worker", + prior_decision=completing, + transition="failover", + failure_class="generic-error", + ) + next_spec = agent_spec_from_decision(next_decision) + except (ExecutionDecisionError, OSError, ValueError): + next_decision = None + next_spec = spec + if next_decision is not None and next_spec != spec: + commit_execution_decision(store, task, "worker", next_decision) + store.update_task( + task, + worker_done=False, + worker_cli=None, + worker_model=None, + completing_decision=None, + execution_class=next_spec.execution_class, + selfcheck_done=False, + completion_selfcheck_pending=False, + selfcheck_incomplete=0, + selfcheck_context_locator=None, + blocked=None, + ) + banner( + "자가검증실행대상전환", + task.name, + [ + f"from={spec.display}", + f"to={next_spec.display}", + f"reason={'; '.join(errors)}", + f"locator={locator}", + ], + ) + return reason = "selfcheck checklist가 미완료지만 target에 native resume 계약이 없다" store.update_task(task, blocked=reason) banner( @@ -5836,6 +5932,7 @@ async def run_selfcheck( store.update_task( task, selfcheck_done=True, + completion_selfcheck_pending=False, selfcheck_incomplete=0, selfcheck_context_locator=None, blocked=None, diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/execution_target_policy.py b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/execution_target_policy.py index d10b6437..e9c8a787 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/execution_target_policy.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/execution_target_policy.py @@ -48,6 +48,7 @@ class RouteTarget: reasoning_effort: str | None execution_class: str selfcheck_required: bool + completion_selfcheck_on_incomplete: bool runtime: dict[str, Any] @dataclass(frozen=True) @@ -211,6 +212,7 @@ def _validate_target(target_id: str, value: object) -> RouteTarget: "reasoning_effort", "execution_class", "selfcheck_required", + "completion_selfcheck_on_incomplete", "runtime", } if unknown: @@ -224,6 +226,13 @@ def _validate_target(target_id: str, value: object) -> RouteTarget: selfcheck_required = value.get("selfcheck_required", False) if not isinstance(selfcheck_required, bool): raise CatalogError(f"{label}.selfcheck_required must be a boolean") + completion_selfcheck_on_incomplete = value.get( + "completion_selfcheck_on_incomplete", False + ) + if not isinstance(completion_selfcheck_on_incomplete, bool): + raise CatalogError( + f"{label}.completion_selfcheck_on_incomplete must be a boolean" + ) reasoning_effort_value = value.get("reasoning_effort") reasoning_effort = ( None @@ -268,6 +277,7 @@ def _validate_target(target_id: str, value: object) -> RouteTarget: reasoning_effort=reasoning_effort, execution_class=execution_class, selfcheck_required=selfcheck_required, + completion_selfcheck_on_incomplete=completion_selfcheck_on_incomplete, runtime=runtime, ) diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/select_execution_target.py b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/select_execution_target.py index fcbb86ab..26fa1fc1 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/select_execution_target.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/scripts/select_execution_target.py @@ -18,7 +18,7 @@ from pathlib import Path from zoneinfo import ZoneInfo -SCHEMA_VERSION = "2.0" +SCHEMA_VERSION = "3.0" CATALOG_ENV = "AGENT_TASK_EXECUTION_CATALOG" DEFAULT_CATALOG_PATH = ( Path(__file__).resolve().parents[1] @@ -160,6 +160,9 @@ def _target_snapshot(target) -> dict: "model": target.model, "execution_class": target.execution_class, "selfcheck_required": target.selfcheck_required, + "completion_selfcheck_on_incomplete": ( + target.completion_selfcheck_on_incomplete + ), } if target.reasoning_effort is not None: snapshot["reasoning_effort"] = target.reasoning_effort @@ -180,6 +183,7 @@ def _validate_target_snapshot(value: object, prefix: str) -> dict: "model", "execution_class", "selfcheck_required", + "completion_selfcheck_on_incomplete", } missing = required - set(value) if missing: @@ -194,6 +198,11 @@ def _validate_target_snapshot(value: object, prefix: str) -> dict: ) if not isinstance(value["selfcheck_required"], bool): raise SelectorInputError(code, f"{prefix}.selfcheck_required must be a boolean") + if not isinstance(value["completion_selfcheck_on_incomplete"], bool): + raise SelectorInputError( + code, + f"{prefix}.completion_selfcheck_on_incomplete must be a boolean", + ) reasoning_effort = value.get("reasoning_effort") if reasoning_effort is not None and ( not isinstance(reasoning_effort, str) or not reasoning_effort diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_dispatch.py b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_dispatch.py index bc2de3f5..a3353a03 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_dispatch.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_dispatch.py @@ -729,6 +729,7 @@ class RuntimeCatalogDispatcherTests(unittest.TestCase): "fake-model", "fake-json-runner/fake-model", target_id="fake-json-target", + completion_selfcheck_on_incomplete=True, runtime={ "command": [sys.executable, str(runner)], "output_format": "jsonl", @@ -762,6 +763,54 @@ class RuntimeCatalogDispatcherTests(unittest.TestCase): self.assertEqual(record["status"], "failed") self.assertNotIn("succeeded:0", work_log) + def test_changed_worker_with_incomplete_evidence_can_advance_to_completion_selfcheck(self): + with TemporaryDirectory() as tmp: + root = Path(tmp) + plan = write_plan(root) + task = task_from_plan(root, plan) + review = plan.parent / "CODE_REVIEW-cloud-G05.md" + review.write_text( + "## Implementation Checklist\n\n- [ ] Implement the task.\n", + encoding="utf-8", + ) + task.review = review + claimed = root / "src" / "item.txt" + claimed.parent.mkdir(parents=True) + runner = root / "completion_selfcheck_runner.py" + runner.write_text( + "import json\n" + f"open({str(claimed)!r}, 'w', encoding='utf-8').write('changed')\n" + "print(json.dumps({'type': 'agent_end', 'willRetry': False, " + "'messages': [{'role': 'assistant', 'stopReason': 'stop'}]}))\n", + encoding="utf-8", + ) + agent = dispatch.AgentSpec( + "fake-json-runner", + "fake-model", + "fake-json-runner/fake-model", + target_id="fake-json-target", + completion_selfcheck_on_incomplete=True, + runtime={ + "command": [sys.executable, str(runner)], + "output_format": "jsonl", + "terminal_success": "agent_end", + }, + ) + with mock.patch.dict(os.environ, {"XDG_STATE_HOME": str(root / "state")}): + store = dispatch.StateStore(root) + try: + return_code, failure, locator = asyncio.run( + dispatch.invoke(root, store, task, "worker", agent, "fake prompt") + ) + record = json.loads(locator.read_text(encoding="utf-8")) + finally: + store.close() + + self.assertEqual(return_code, 0) + self.assertIsNone(failure) + self.assertEqual(record["status"], "succeeded") + self.assertTrue(record["completion_selfcheck_on_incomplete"]) + def test_worker_done_with_incomplete_evidence_returns_to_worker_stage(self): with TemporaryDirectory() as tmp: root = Path(tmp) @@ -778,6 +827,98 @@ class RuntimeCatalogDispatcherTests(unittest.TestCase): self.assertEqual(stage, "worker") + def test_worker_done_with_conditional_selfcheck_routes_incomplete_evidence_to_selfcheck(self): + with TemporaryDirectory() as tmp: + root = Path(tmp) + value = catalog_value() + value["targets"]["primary"][ + "completion_selfcheck_on_incomplete" + ] = True + catalog = write_catalog(root, value) + plan = write_plan(root) + task = task_from_plan(root, plan) + review = plan.parent / "CODE_REVIEW-cloud-G05.md" + review.write_text( + "## Implementation Checklist\n\n- [ ] Implement the task.\n", + encoding="utf-8", + ) + task.review = review + selector = dispatch._selector_module() + decision = selector.select_execution_target(plan, catalog_path=catalog) + + stage = dispatch.task_stage( + task, + { + "worker_done": True, + "selfcheck_done": False, + "completion_selfcheck_pending": True, + "completing_decision": decision, + }, + ) + + self.assertEqual(stage, "selfcheck") + + def test_incomplete_conditional_selfcheck_without_resume_fails_over_to_next_worker(self): + with TemporaryDirectory() as tmp: + root = Path(tmp) + value = catalog_value() + value["targets"]["primary"]["runtime"].pop( + "native_session_monitor" + ) + value["targets"]["primary"]["runtime"].pop("resume_command") + value["targets"]["primary"][ + "completion_selfcheck_on_incomplete" + ] = True + catalog = write_catalog(root, value) + plan = write_plan(root) + task = task_from_plan(root, plan) + review = plan.parent / "CODE_REVIEW-cloud-G05.md" + review.write_text( + "## Implementation Checklist\n\n- [ ] Implement the task.\n", + encoding="utf-8", + ) + task.review = review + dispatch.EXECUTION_CATALOG_PATH = catalog + selector = dispatch._selector_module() + decision = selector.select_execution_target( + plan, catalog_path=catalog + ) + with mock.patch.dict( + os.environ, {"XDG_STATE_HOME": str(root / "state")} + ): + store = dispatch.StateStore(root) + try: + store.update_task( + task, + worker_done=True, + selfcheck_done=False, + completion_selfcheck_pending=True, + completing_decision=decision, + execution_decisions={"worker": decision}, + ) + with mock.patch.object( + dispatch, + "run_escalating", + new=mock.AsyncMock( + return_value=( + True, + root / "selfcheck-locator.json", + ) + ), + ): + asyncio.run(dispatch.run_selfcheck(root, store, task)) + state = store.task_state(task) + finally: + store.close() + + self.assertFalse(state["worker_done"]) + self.assertFalse(state["completion_selfcheck_pending"]) + self.assertIsNone(state["blocked"]) + self.assertEqual( + state["execution_decisions"]["worker"]["selected"]["target_id"], + "alternate", + ) + def test_silent_native_session_is_terminated_and_classified_as_stall(self): with TemporaryDirectory() as tmp: root = Path(tmp) diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_execution_target_policy.py b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_execution_target_policy.py index 1916c4c7..72fec254 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_execution_target_policy.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_execution_target_policy.py @@ -105,6 +105,30 @@ class ExecutionTargetPolicyTests(unittest.TestCase): self.assertFalse(hasattr(policy, "quota_probe_spec")) self.assertFalse(hasattr(policy, "promotion_target")) + def test_completion_selfcheck_flag_is_optional_and_typed(self): + with TemporaryDirectory() as tmp: + root = Path(tmp) + value = catalog_value() + value["targets"]["target-a"][ + "completion_selfcheck_on_incomplete" + ] = True + catalog = policy.load_catalog(write_catalog(root, value)) + self.assertTrue( + catalog.targets["target-a"].completion_selfcheck_on_incomplete + ) + self.assertFalse( + catalog.targets["target-b"].completion_selfcheck_on_incomplete + ) + + value["targets"]["target-a"][ + "completion_selfcheck_on_incomplete" + ] = "yes" + with self.assertRaisesRegex( + policy.CatalogError, + "completion_selfcheck_on_incomplete must be a boolean", + ): + policy.load_catalog(write_catalog(root, value)) + def test_optional_windows_are_catalog_owned_and_timezone_generic(self): with TemporaryDirectory() as tmp: catalog = policy.load_catalog(write_catalog(Path(tmp), catalog_value(windows=True))) diff --git a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_select_execution_target.py b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_select_execution_target.py index 55b60fab..e7a71c51 100644 --- a/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_select_execution_target.py +++ b/agent-ops/skills/common/orchestrate-agent-task-loop/tests/test_select_execution_target.py @@ -90,6 +90,7 @@ class SelectorTests(unittest.TestCase): f"local-G{grade:02d}": ["pi-ornith-high"] for grade in range(1, 7) }, + "local-G04": ["pi-ornith-fast-high"], "local-G07": ["opencode-glm-max", "codex-terra-high"], "local-G08": ["opencode-glm-max", "codex-terra-high"], "local-G09": ["codex-sol-high", "codex-terra-high"], @@ -107,6 +108,7 @@ class SelectorTests(unittest.TestCase): } expected_targets = { "pi-ornith-high", + "pi-ornith-fast-high", "opencode-glm-medium", "opencode-glm-high", "opencode-glm-max", @@ -180,6 +182,7 @@ class SelectorTests(unittest.TestCase): opencode = catalog.targets["opencode-glm-max"] self.assertIn("iop-glm/glm-5.2", opencode.runtime["command"]) self.assertEqual(opencode.reasoning_effort, "high") + self.assertTrue(opencode.completion_selfcheck_on_incomplete) self.assertIn("{reasoning_effort}", opencode.runtime["command"]) for target_id in ( "opencode-glm-medium", @@ -220,7 +223,7 @@ class SelectorTests(unittest.TestCase): catalog_path=catalog, evaluated_at=datetime(2026, 1, 1, tzinfo=timezone.utc), ) - self.assertEqual(result["schema_version"], "2.0") + self.assertEqual(result["schema_version"], "3.0") self.assertEqual(result["selected"]["target_id"], "first") self.assertEqual(result["selected"]["agent"], "agent-one") self.assertEqual(result["selected"]["model"], "model-one") diff --git a/agent-spec/input/openai-compatible-surface.md b/agent-spec/input/openai-compatible-surface.md index 19e402a8..4b4c0636 100644 --- a/agent-spec/input/openai-compatible-surface.md +++ b/agent-spec/input/openai-compatible-surface.md @@ -292,7 +292,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. The operator configures them in `execution_presets[].single_request.templates`; admission freezes the effective pair, so a config refresh reaches only requests admitted after it and an already running request keeps its pair. The Plan stage requests a closed strict JSON object containing a one-line `goal` string, a 2-6 item `steps` string array, and a 1-3 item `verification` string array. Each item must be non-empty and one-line; Edge adds the Markdown bullet prefixes and newlines and renders the frozen Plan template deterministically. The Review template shapes the private `review.md` artifact rendered from the model's `checks`/`verification`/`summary` fields. 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 remains the model's `decision.output`. +- 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 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. @@ -357,6 +357,7 @@ sequenceDiagram ## 변경 기록 +- 2026-08-14: Synchronized marked single-request artifact-only PLAN→Work→REVIEW→Review handoff, reviewer-owned repair/re-verification, reviewer zero-write, and strict terminal output provenance. - 2026-08-14: Added operation-scoped `normalization.tool_calls` and Gemini-only Chat thought-signature round trips across standard OpenAI-compatible callers, including non-stream, SSE, and recovery-selected dispatches. Effort mapping and caller identity remain independent (`packages/go/config/protocol_profile.go`, `apps/edge/internal/openai/provider_model_rewrite.go`). - 2026-08-13: Added official agy 1.1.12 model-role `functionResponse` continuation support while retaining fail-closed rejection for mixed assistant/tool-response content (`apps/edge/internal/openai/gemini_handler.go`). - 2026-08-12: Admitted Claude Code's `advisor-tool-2026-03-01` beta as a consumed compatibility marker for both direct and marked-preset Messages ingress. It grants no internal capability and is not forwarded through the Chat bridge (`apps/edge/internal/openai/anthropic_types.go`). diff --git a/agent-spec/runtime/edge-node-execution.md b/agent-spec/runtime/edge-node-execution.md index 3cb8a83d..f1946af5 100644 --- a/agent-spec/runtime/edge-node-execution.md +++ b/agent-spec/runtime/edge-node-execution.md @@ -231,7 +231,7 @@ The shared `packages/go/execution` package contains provider lifecycle, registry | Plan stage | The Plan runner validates the frozen effective template, emits the `planning` envelope, sends the immutable task through the frozen Plan binding with `reasoning_effort=high` and a stage-owned strict JSON schema for one-line `goal` plus bounded one-line `steps`/`verification` arrays, validates the fields, adds Markdown bullets, renders the template deterministically inside Edge, and writes the resulting Markdown through `SingleRequestArtifactPlan`. | | single-request provider normalization | Private Plan/Work/Review calls pass caller-neutral effort/tool/structured-output requirements to the selected protocol profile. The profile chooses Chat Completions or Responses and maps unsupported effort only downward. An explicit managed selector freezes the exact provider ID; `default` freezes no provider ID and accepts the provider pool's concrete choice while retaining exact model-group/profile/target/credential/tunnel fences. Chat and Responses provider results are both reduced to one canonical private Chat-shaped envelope before strict stage decoding. No new Edge-Node field is added: the selected operation continues through the existing provider tunnel operation field. | | single-request effective templates | `execution_presets[].single_request.templates` optionally loads `plan_file`/`review_file` as bounded Markdown relative to the directory containing `edge.yaml`; absolute and empty paths, non-regular files, oversize (`>8192` bytes), non-UTF-8, and invalid grammar fail closed at load, and each file falls back to its built-in default independently. Admission freezes the effective Plan/Review pair into the binding, so a later refresh reaches only newly admitted requests. Templates select internal stage input and internal artifact shape only; caller request/response schemas are unchanged. | -| Work stage | The `ornith-fast` Work runner reads the closed PLAN artifact, projects only the admitted workspace tools, and resumes the same frozen provider route after exactly correlated Node results. It rejects any Work `reasoning_effort`, malformed or multiple tool calls, and empty completion or verification evidence. | +| Work and Review handoff | Work parses the stored PLAN with deterministic `P1..Pn` IDs, projects only admitted workspace tools, and writes exactly one validated REVIEW handoff containing item status, changes, verification, and deviations. Review rereads both artifacts before provider dispatch, has no memory work payload, performs any repair/re-verification in the request-local ledger, and writes no final REVIEW page. | | request-owned cleanup | Node creates and inventories only `.iop/job/` internal state, cancels and waits for all active command groups, validates the exact tree without following entries, and removes matching artifacts deepest-first with non-recursive descriptor operations. Symlinks, special files, foreign devices, identity replacements, and unowned entries fail closed. User results and sibling request state are preserved. Concurrent cleanup callers receive one bounded cached typed result. | | provider raw tunnel | 선택된 provider의 HTTP/SSE를 `ProviderTunnelRequest`/`ProviderTunnelFrame`으로 relay하며 순서와 단일 terminal outcome을 보장한다. | | response-stall activity contract | 선택된 provider의 response-stall timeout을 normalized/tunnel request에 보존한다. Node는 wire zero를 `60000ms`로 해석하고 invalid raw value를 adapter 호출 전에 거부한다. Runtime event의 terminal type은 payload/usage보다 우선하며 non-terminal usage는 progress다. | @@ -257,8 +257,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, accepts only `RESPONSE_START`, zero or more `BODY`, then `END`, and projects all provider failures to a generic internal failure. 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 Review stage renders its internal REVIEW artifact from the request's frozen effective Review template, substituting only the model's `checks`, `verification`, and `summary` fields into the documented placeholder inventory. The template selects the internal artifact shape only: the caller-visible final response remains the model's `decision.output`, so replacing the Review template never changes the public Messages response schema. -- The private Work stage is installed in the composite single-request executor at Edge input startup (`apps/edge/internal/input/manager.go`). It reads only `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 provider messages contain the immutable task, PLAN, admitted tool schemas, and bounded typed tool results; Review/repair and composite installation are active, and S12 (`claude-smoke`) qualifies the external Claude path. +- 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 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한다. @@ -367,6 +367,7 @@ Heartbeat interval/wait는 protobuf field가 아닌 양쪽 transport 구현의 l ## 변경 기록 +- 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. - 2026-08-14: Added common Chat result normalization for private stages so standard OpenAI bookkeeping fields are removed before strict decoding, matching the existing Responses-to-common conversion while preserving fail-closed refusal and unknown-field handling. - 2026-08-12: Replaced nondeterministic free-form PlanMD generation with a stage-owned strict `goal`/`steps`/`verification` JSON response. Edge rejects unknown, duplicate, missing, mistyped, or out-of-bound fields and deterministically renders the already-frozen operator Plan template, preserving template customization and every caller-visible schema (`apps/edge/internal/openai/single_request_plan_stage.go`, `packages/go/singlerequesttemplate/template.go`). diff --git a/agent-spec/runtime/provider-pool-config-refresh.md b/agent-spec/runtime/provider-pool-config-refresh.md index a07be806..281ee1c4 100644 --- a/agent-spec/runtime/provider-pool-config-refresh.md +++ b/agent-spec/runtime/provider-pool-config-refresh.md @@ -135,7 +135,7 @@ Edge 설정에서 provider-pool이 어떻게 모델 실행 후보를 고르고, | mutable apply | 적용 가능한 변경은 Edge `Cfg`, `NodeStore`, service/input model catalog, OpenAI long-context threshold를 copy-on-write로 교체한다. | | single-request snapshot isolation | An admitted single-request binding is independent of subsequent model catalog, execution preset, or provider pool changes. Refresh replaces the live catalog and preset snapshots used by future admissions; already-admitted bindings retain their original values. | | fixed single-request policy | `execution_presets[].single_request` declares an operator-owned immutable plan→work→review light path with absolute wall-clock (`≤1800000ms`), stage-timeout (`≤600000ms`), tool-iteration (`≤64`), and output-byte (`≤16MiB`) caps. Selector and plan/review stages require `reasoning_effort=high`; work stage forbids it. `workspace_ref` is opaque (never raw path/credential/Node/endpoint). single_request preset rejects `workspace_tools`. Catalog and mapping changes are live-apply and affect only new request snapshots; admitted bindings retain their frozen values across refresh. | -| single-request effective templates | Optional `templates` (`plan_file`/`review_file`) load bounded Markdown relative to the directory containing `edge.yaml` only. Absolute and empty paths are rejected before any filesystem access; non-regular files, sizes over 8192 bytes, non-UTF-8 content, and invalid template grammar fail closed at load. Each file falls back to its built-in default independently, and refresh diff evidence reports SHA-256 digests only, never template paths or contents. | +| single-request effective templates | Optional `templates` (`plan_file`/`review_file`) load bounded Markdown relative to the directory containing `edge.yaml` only. The PLAN grammar yields deterministic `P1..Pn`; the REVIEW grammar permits only worker item status, changes, verification, and deviations, rejecting old reviewer-final templates. Absolute and empty paths are rejected before any filesystem access; non-regular files, sizes over 8192 bytes, non-UTF-8 content, and invalid grammar fail closed at load. Each file falls back to its built-in default independently, and refresh diff evidence reports SHA-256 digests only, never template paths or contents. | | effective-template admission freeze | Admission copies the resolved effective Plan/Review pair into the immutable binding, and that pair survives binding clone and workspace revalidation. A later refresh swaps the preset snapshot used by future admissions only: already-admitted work keeps its frozen pair, while a request admitted after the refresh observes the refreshed pair. Templates select internal stage input and internal artifact shape only; caller request and response schemas are unchanged. | | operator-owned workspace catalog | `nodes[].workspaces[]` is the operator-owned bounded capability catalog for each node. Each entry is keyed by a globally unique, trimmed `ref`, declares `platform` in the closed `darwin|linux` implementation set, and retains the existing absolute clean root, closed operations, approved commands, environment allowlist, and bounded byte/time limits. Refs remain globally unique and any catalog change is `restart_required`. Empty catalogs are backward-compatible on any host. A non-empty catalog requires a supported Node host and every entry must match that host before any root is opened; Windows, unknown hosts, and cross-platform catalogs fail closed. The catalog is delivered by the Node-private typed config payload and retained as opened immutable runtime authority. Raw roots and command details never enter presets, public responses, provider requests, or metadata; operating system is runtime evidence rather than a caller selector. | | Node config refresh push | 변경이 있으면 Edge가 dispatch-ready Node에 node-specific `NodeConfigRefreshRequest`를 push한다. accepted지만 pending인 Node는 register response config를 적용한 뒤 ready가 될 때까지 push 대상이 아니다. | @@ -245,6 +245,7 @@ sequenceDiagram ## 변경 기록 +- 2026-08-14: Updated the frozen single-request template grammar for deterministic PLAN IDs and a worker-to-reviewer REVIEW handoff; legacy reviewer-final custom templates now fail closed at loading/admission. - 2026-08-08: Synchronized the implemented workspace catalog/runtime boundary with closed `darwin|linux` admission, exact catalog/host matching before root open, empty-catalog compatibility, and Windows/unknown fail-closed scope. - 2026-07-07: 현재 코드, 계약, config 예시 기준으로 bootstrap spec 작성. - 2026-07-07: 기능 목록 중심으로 축소하고 주요 흐름을 Mermaid sequence diagram으로 정리. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_1.log b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_1.log new file mode 100644 index 00000000..bdb6bec2 --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_1.log @@ -0,0 +1,225 @@ + + +# Code Review Reference - REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt. +> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. +> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state. +> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## Overview + +date=2026-08-14 +task=single_request_artifact_handoff, plan=1, tag=REVIEW_REFACTOR + +## Archive Evidence Snapshot + +- Prior task path: `agent-task/single_request_artifact_handoff/` +- Prior plan: `agent-task/single_request_artifact_handoff/plan_local_G07_0.log` +- Prior review: `agent-task/single_request_artifact_handoff/code_review_cloud_G07_0.log` +- Verdict: FAIL; Required R1 strict artifact grammar, Required R2 command-based post-repair verification; Suggested/Nit: none. +- Reviewer verification: focused, race, broad Go regression, vet, and `git diff --check` passed; temporary focused reproducers proved all three acceptance gaps. +- Affected files: `packages/go/singlerequesttemplate/template.go`, `packages/go/singlerequesttemplate/template_test.go`, `apps/edge/internal/openai/single_request_work_stage.go`, `apps/edge/internal/openai/single_request_work_stage_test.go`, `apps/edge/internal/openai/single_request_review_stage.go`, `apps/edge/internal/openai/single_request_review_stage_test.go`. +- Roadmap carryover: none; this is a non-milestone task. + +## For the Review Agent + +> **[REVIEW AGENT ONLY]** Compare implementation against source files, rerun applicable verification, append one verdict with routing signals, then archive/finalize according to the code-review skill. Do not delegate diagnosis or remedy selection. + +## Implementation Item Completion + +| Item | Status | +|---|---| +| REVIEW_REFACTOR-1: Frozen PLAN and complete REVIEW item grammar | [x] | +| REVIEW_REFACTOR-2: Command verification after repair | [x] | + +## Implementation Checklist + +- [x] [REVIEW_REFACTOR-1] Enforce frozen PLAN parsing and complete REVIEW item-status grammar with non-dispatch regressions. +- [x] [REVIEW_REFACTOR-2] Separate repair mutation and command verification ledger semantics with success/rejection regressions. +- [x] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [x] Fill implementation-owned sections in `CODE_REVIEW-cloud-G05.md` with actual implementation notes and verification output. + +## Review-Only Checklist + +> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent. Implementing agents must not modify or check this section. + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. +- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. +- [x] Run applicable required verification and record fresh command/output; repair reviewer-reconstructable evidence gaps instead of forwarding them to another plan. +- [x] For every Required/Suggested finding, record reviewer-collected `Evidence`, exact `Root Cause`, and one `Selected Fix` with affected files/symbols/tests and acceptance commands before creating a follow-up plan. +- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G05_1.log`. +- [x] Archive active `PLAN-*-G??.md` to `plan_local_G04_1.log`. +- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/single_request_artifact_handoff/` to `agent-task/archive/YYYY/MM/single_request_artifact_handoff/` and update this checklist at the final archive path. +- [ ] If PASS for split work, remove empty active parent or verify it was kept due to remaining siblings/files. +- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`. + +## Deviations from Plan + +None. Implementation followed the plan's selected root cause, scope, files, and dependency decisions exactly. + +## Key Design Decisions + +1. **Frozen PLAN parse before ID extraction (R1):** Both Work and Review stages now call `singlerequesttemplate.ParsePlan(binding.Templates.Plan, string(plan), req.Limits.MaxOutputBytes)` before `PlanItemIDs`. This ensures a stored PLAN that fails the exact parser against the frozen effective template cannot reach provider dispatch or the REVIEW handoff write. + +2. **Exact line inventory for Worker Item Status (R1):** `ValidateReviewHandoff` now splits the entire status section on newlines, requires `len(lines) == len(planIDs)`, and matches each line against its exact expected grammar `- Pn: completed`. This rejects prose injected between status lines, blank lines, malformed bullets, and out-of-order or duplicate entries in a single pass, without regex filtering. + +3. **Separated ledger semantics from tool admission (R2):** The `isInspection` variable remains for tool admission/lifecycle classification, but ledger updates are now split by tool class: + - write/delete: record mutation, clear verification, clear repair-required flag + - command with `mutationOccurred=true`: record post-mutation verification without clearing the ledger + - command with `mutationOccurred=false`: repair attempt; pass remains blocked until later verification + - read/list with `mutationOccurred=true`: record verification + +4. **Single plan kept (split judgment):** The stored artifact parser and reviewer ledger jointly protect the same Plan→Work→Review terminal invariant, and the patch is compact enough that splitting would not yield an independently releasable intermediate state. + +## Reviewer Checkpoints + +- [x] Work and Review reject a stored PLAN that does not match the frozen effective Plan template before provider dispatch. +- [x] Worker Item Status contains exactly one full grammar line per ordered PLAN ID and rejects all extra prose/malformed/blank lines. +- [x] write/delete mutation still blocks PASS until later successful verification. +- [x] A successful command after an existing mutation satisfies post-repair verification; command-first does not bypass the gate. +- [x] REVIEW artifact write count remains Work-only and caller terminal output remains reviewer `output`. +- [x] Existing cancellation, waiter cleanup, bounds, and race tests remain closed. + +## Verification Results + +Record actual stdout/stderr for each command. Fresh execution is required. + +### Template and config + +```bash +$ go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +ok iop/packages/go/singlerequesttemplate 0.015s +ok iop/packages/go/config 0.285s +``` + +### Edge focused + +```bash +$ go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +ok iop/apps/edge/internal/openai 0.518s +``` + +### Race + +```bash +$ go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +ok iop/apps/edge/internal/openai 1.822s +``` + +### Broader local regression + +```bash +$ go test -count=1 ./apps/edge/... ./packages/go/... +ok iop/apps/edge/cmd/edge 0.275s +ok iop/apps/edge/internal/authprojection 0.088s +ok iop/apps/edge/internal/bootstrap 0.626s +ok iop/apps/edge/internal/configrefresh 0.104s +ok iop/apps/edge/internal/controlplane 6.613s +ok iop/apps/edge/internal/edgecmd 0.197s +ok iop/apps/edge/internal/edgevalidate 0.150s +ok iop/apps/edge/internal/events 0.044s +ok iop/apps/edge/internal/input 0.080s +ok iop/apps/edge/internal/input/a2a 0.061s +ok iop/apps/edge/internal/node 0.051s +ok iop/apps/edge/internal/openai 8.775s +ok iop/apps/edge/internal/opsconsole 0.087s +ok iop/apps/edge/internal/service 8.307s +ok iop/apps/edge/internal/transport 4.821s +ok iop/packages/go/audit 0.018s +ok iop/packages/go/auth 10.038s +ok iop/packages/go/config 0.251s +ok iop/packages/go/credentiallease 0.065s +? iop/packages/go/events [no test files] +ok iop/packages/go/execution 0.020s +ok iop/packages/go/hostsetup 0.013s +? iop/packages/go/jobs [no test files] +? iop/packages/go/metadata [no test files] +ok iop/packages/go/observability 0.043s +? iop/packages/go/policy [no test files] +ok iop/packages/go/singlerequesttemplate 0.012s +ok iop/packages/go/streamgate 0.907s +? iop/packages/go/version [no test files] +ok iop/packages/go/workspaceprotocol 0.029s +$ git diff --check +(no output) +``` + +### Live single-request acceptance + +Unavailable in the local baseline: no configured marked single-request endpoint, approved Node, provider credential, or remote runner. Package tests do not replace this acceptance. Resume condition: when a configured approved marked single-request endpoint, Node, and credential are available, run one small HTML request and verify one ingress, one PLAN write, one Work REVIEW write, zero reviewer REVIEW writes, one terminal, cleanup, and exact workspace/caller output. + +### Reviewer fresh verification (2026-08-14) + +```text +$ go version +go version go1.26.2 linux/arm64 +$ go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +ok iop/packages/go/singlerequesttemplate 0.009s +ok iop/packages/go/config 0.185s +$ go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +ok iop/apps/edge/internal/openai 0.473s +$ go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +ok iop/apps/edge/internal/openai 1.804s +$ go vet ./apps/edge/internal/service +(no output; exit 0) +$ go test ./apps/edge/internal/service -count=1 +ok iop/apps/edge/internal/service 8.229s +$ go vet ./packages/go/... +(no output; exit 0) +$ go test -count=1 ./apps/edge/... ./packages/go/... +ok iop/apps/edge/internal/openai 8.838s +ok iop/apps/edge/internal/service 8.366s +ok iop/packages/go/config 0.337s +ok iop/packages/go/singlerequesttemplate 0.017s +(all remaining tested packages passed; packages without tests reported [no test files]) +$ git diff --check +(no output; exit 0) +``` + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** +> If anything is blank, go back and fill it in before saving this file. Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---|---|---| +| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementer must not modify finalization fields | +| Archive Evidence Snapshot | Fixed at stub creation from plan | Use as prior-loop context | +| Implementation Item Completion | Fixed at stub creation | Implementer checks status only | +| Implementation Checklist | Fixed at stub creation from plan | Implementer checks status only | +| Review-Only Checklist | Review agent only | Implementer must not modify | +| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholders with actual content | +| Reviewer Checkpoints | Fixed at stub creation | Review criteria | +| Verification Results | Implementing agent, then review agent | Record actual output; reviewer reruns | +| Code Review Result | Review agent appends | Not included in stub | + +## Code Review Result + +- Overall Verdict: FAIL +- Dimension Assessment: + - Correctness: Fail + - Completeness: Fail + - Test coverage: Fail + - API contract: Fail + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- Findings: + - Required R3 — A command used to repair a `not_found` artifact can never complete the required re-verification path. + - Evidence: In `apps/edge/internal/openai/single_request_review_stage.go:221-247`, a `not_found` result sets `repairRequired=true`. A later successful `workspace_command` with no prior `mutationOccurred` sets `mutationOccurred=true` and `verifiedAfterMutation=false`, but only the write/delete branch clears `repairRequired`. A second successful command then sets verification true but still leaves `repairRequired=true`; the pass branch at lines 142-144 rejects the decision. Existing command-ledger tests cover write→command and command-first, but not `not_found`→command repair→command verification. The reviewer reran the focused, race, broad Go, vet, and diff checks above; all pass but do not exercise this missing transition. + - Root Cause: The revised ledger treats a command-first call as a mutation for verification purposes but does not treat it as the repair that satisfies the outstanding `not_found` repair requirement. This leaves the repair gate and the mutation ledger with incompatible state. + - Selected Fix: In `apps/edge/internal/openai/single_request_review_stage.go`, when a successful `workspace_command` begins an outstanding repair, clear `repairRequired` while retaining `mutationOccurred=true` and `verifiedAfterMutation=false`; a later successful read/list/command then supplies verification. Add direct-stage and coordinator regression cases in `apps/edge/internal/openai/single_request_review_stage_test.go` for `not_found`→command repair→command verification→PASS, including zero waiter leaks, one Work-owned REVIEW write, and one terminal. +- Routing Signals: + - review_rework_count=2 + - evidence_integrity_failure=false +- Next Step: Prepare and materialize a routed follow-up pair for direct fix R3; do not write `complete.log`. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_2.log b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_2.log new file mode 100644 index 00000000..18801c9a --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G05_2.log @@ -0,0 +1,190 @@ + + +# Code Review Reference - REVIEW_REVIEW_REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt. +> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. +> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state. +> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## Overview + +date=2026-08-14 +task=single_request_artifact_handoff, plan=2, tag=REVIEW_REVIEW_REFACTOR + +## Archive Evidence Snapshot + +- Prior task path: `agent-task/single_request_artifact_handoff/` +- Prior plan: `agent-task/single_request_artifact_handoff/plan_local_G04_1.log` +- Prior review: `agent-task/single_request_artifact_handoff/code_review_cloud_G05_1.log` +- Verdict: FAIL; Required R3 command-repair ledger state; Suggested/Nit: none. +- Reviewer verification: focused, race, broader Edge/common Go tests, vet, and `git diff --check` passed. Static review proved that command repair after `not_found` never clears `repairRequired`. +- Affected files: `apps/edge/internal/openai/single_request_review_stage.go`, `apps/edge/internal/openai/single_request_review_stage_test.go`. +- Roadmap carryover: none; this is a non-milestone task. + +## For the Review Agent + +> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section. + +Compare implementation against source files, rerun applicable verification, and record fresh results. Review completion requires a verdict, archive, required next state, and final review-only checklist. + +--- + +## Implementation Item Completion + +| Item | Status | +|---|---| +| REVIEW_REVIEW_REFACTOR-1: Clear command repair gate | [x] | +| REVIEW_REVIEW_REFACTOR-2: Add command repair regressions | [x] | + +## Implementation Checklist + +- [x] [REVIEW_REVIEW_REFACTOR-1] Clear the command-repair gate while preserving the later verification requirement. +- [x] [REVIEW_REVIEW_REFACTOR-2] Add direct and coordinator regressions for command repair after `not_found`. +- [x] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [x] Fill implementation-owned sections in `CODE_REVIEW-*-G??.md` with actual implementation notes and verification output. + +## Review-Only Checklist + +> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent. Implementing agents must not modify or check this section. + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. +- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. +- [x] Run applicable required verification and record fresh command/output; repair reviewer-reconstructable evidence gaps instead of forwarding them to another plan. +- [x] For every Required/Suggested finding, record reviewer-collected `Evidence`, exact `Root Cause`, and one `Selected Fix` with affected files/symbols/tests and acceptance commands before creating a follow-up plan. +- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_cloud_G05_2.log`. +- [x] Archive active `PLAN-*-G??.md` to `plan_cloud_G05_2.log`. +- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [x] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [x] If PASS, move active task directory `agent-task/single_request_artifact_handoff/` to `agent-task/archive/YYYY/MM/single_request_artifact_handoff/` and update this checklist at the final archive path. +- [ ] If PASS and task group is `m-`, preserve and report `milestone-task` metadata for runtime aggregation without modifying roadmap. +- [ ] If PASS for split work, remove empty active parent or verify it was kept due to remaining siblings/files. +- [ ] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`. + +## Deviations from Plan + +없음. + +## Key Design Decisions + +- `workspace_command`가 `not_found` 뒤의 첫 성공 수리인 경우에만 `repairRequired`를 해제한다. 이 명령은 여전히 mutation으로 기록하므로 다음 성공 read/list/command 검증 전 PASS는 거부된다. +- 기존 command-after-mutation 경로는 검증으로 유지했다. command-first PASS 거부도 변경하지 않았다. +- 직접 stage와 service coordinator 회귀는 모두 `not_found → workspace_command repair → workspace_command verification → PASS`를 사용하며, coordinator는 승인된 `verify` command capability 내에서 수리·검증 결과를 구분한다. + +## Reviewer Checkpoints + +- [x] A successful command used after `not_found` clears the outstanding repair gate but still requires a later verification. +- [x] The later successful command verification permits one terminal PASS; command-first still cannot PASS. +- [x] Direct and coordinator tests prove zero pending waiters, Work-only REVIEW write count, cleanup, and one finalizing terminal. +- [x] Existing artifact grammar, cancellation, and race tests remain closed. + +## Verification Results + +Record actual stdout/stderr for each command. Fresh execution is required. + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)' +ok iop/apps/edge/internal/openai 0.084s + +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +ok iop/apps/edge/internal/openai 1.708s + +go test -count=1 ./apps/edge/... ./packages/go/... +ok iop/apps/edge/cmd/edge 0.435s +ok iop/apps/edge/internal/authprojection 0.095s +ok iop/apps/edge/internal/bootstrap 0.670s +ok iop/apps/edge/internal/configrefresh 0.238s +ok iop/apps/edge/internal/controlplane 6.729s +ok iop/apps/edge/internal/edgecmd 0.200s +ok iop/apps/edge/internal/edgevalidate 0.157s +ok iop/apps/edge/internal/events 0.078s +ok iop/apps/edge/internal/input 0.128s +ok iop/apps/edge/internal/input/a2a 0.118s +ok iop/apps/edge/internal/node 0.093s +ok iop/apps/edge/internal/openai 9.132s +ok iop/apps/edge/internal/opsconsole 0.109s +ok iop/apps/edge/internal/service 8.311s +ok iop/apps/edge/internal/transport 4.849s +ok iop/packages/go/audit 0.018s +ok iop/packages/go/auth 10.043s +ok iop/packages/go/config 0.288s +ok iop/packages/go/credentiallease 0.052s +? iop/packages/go/events [no test files] +ok iop/packages/go/execution 0.022s +ok iop/packages/go/hostsetup 0.025s +? iop/packages/go/jobs [no test files] +? iop/packages/go/metadata [no test files] +ok iop/packages/go/observability 0.052s +? iop/packages/go/policy [no test files] +ok iop/packages/go/singlerequesttemplate 0.012s +ok iop/packages/go/streamgate 0.893s +? iop/packages/go/version [no test files] +ok iop/packages/go/workspaceprotocol 0.030s + +git diff --check +(stdout/stderr 없음; exit 0) +``` + +### Reviewer fresh verification (2026-08-14) + +```text +$ go version +go version go1.26.2 linux/arm64 +$ go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)' +ok iop/apps/edge/internal/openai 0.101s +$ go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +ok iop/apps/edge/internal/openai 1.895s +$ go vet ./apps/edge/internal/service +(stdout/stderr 없음; exit 0) +$ go test ./apps/edge/internal/service -count=1 +ok iop/apps/edge/internal/service 8.305s +$ go test -count=1 ./apps/edge/... ./packages/go/... +all selected Edge/common packages passed; packages without tests reported [no test files] +$ git diff --check +(stdout/stderr 없음; exit 0) +``` + +### Live single-request acceptance + +미실행. local baseline에 configured marked single-request endpoint, approved Node, provider credential, remote runner가 없다. 모두 준비되면 작은 HTML 요청 1건으로 ingress, PLAN/REVIEW writes, terminal, cleanup, caller/workspace output을 검증한다. + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** +> If anything is blank, go back and fill it in before saving this file. Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---|---|---| +| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementer must not modify finalization fields | +| Archive Evidence Snapshot | Fixed at stub creation | Use as prior-loop context | +| Implementation Item Completion | Fixed at stub creation | Implementer checks status only | +| Implementation Checklist | Fixed at stub creation | Implementer checks status only | +| Review-Only Checklist | Review agent only | Implementer must not modify | +| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholders with actual content | +| Reviewer Checkpoints | Fixed at stub creation | Review criteria | +| Verification Results | Implementing agent, then review agent | Record actual output; reviewer reruns | +| Code Review Result | Review agent appends | Not included in stub | + +## Code Review Result + +- Overall Verdict: PASS +- Dimension Assessment: + - Correctness: Pass + - Completeness: Pass + - Test coverage: Pass + - API contract: Pass + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- Findings: None +- Routing Signals: + - review_rework_count=2 + - evidence_integrity_failure=false +- Next Step: Write `complete.log`, archive the active pair and completed task, and leave no active task artifacts. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G07_0.log b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G07_0.log new file mode 100644 index 00000000..5cbb9c5e --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/code_review_cloud_G07_0.log @@ -0,0 +1,215 @@ + + +# Code Review Reference - REFACTOR + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `Implementation Checklist`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> Execute the plan's selected root cause, scope, files, and dependency decisions as written. Do not choose another owner, narrow/expand the write boundary, or replace a fix with another verification attempt. +> If implementation is blocked, record the exact blocker, attempted commands/output, and resume condition only in implementation-owned evidence fields. +> Do not ask the user directly, present choices, call user-input tools, create control-plane stop files, or classify the next state. +> Finalization (`Code Review Result`, log rename, `complete.log`, archive moves, `Review-Only Checklist`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## Overview + +date=2026-08-14 +task=single_request_artifact_handoff, plan=0, tag=REFACTOR + +## For the Review Agent + +> **[REVIEW AGENT ONLY]** The finalization steps below are review-agent only. Implementing agents must not execute this section. + +Compare implementation of each item against source files. Run the applicable verification commands directly and record fresh output in `Verification Results`; implementation-owned output is handoff evidence, not a substitute for reviewer verification. If implementation is present, repair missing or stale verification output instead of failing solely for insufficient recorded evidence. When verification exposes a defect, collect the necessary data, determine the exact root cause, and select one concrete fix before generating the follow-up plan; never delegate investigation or remedy selection to the worker. +Review completion means the following steps are finished: + +1. Append verdict and `review_rework_count` / `evidence_integrity_failure` routing signals. +2. Archive `CODE_REVIEW-cloud-G07.md` → `code_review_cloud_G07_{review_log_number}.log` and `PLAN-local-G07.md` → `plan_local_G07_{plan_log_number}.log`. +3. If PASS, write `complete.log` and move active task directory to `agent-task/archive/YYYY/MM/single_request_artifact_handoff/`. If WARN/FAIL, fully write the next filesystem state required by the code-review skill. +4. Check applicable `Review-Only Checklist` items at the final `.log` location before reporting. + +--- + +## Implementation Item Completion + +| Item | Status | +|------|---------| +| REFACTOR-1: Compact template grammar and parser | [x] | +| REFACTOR-2: Artifact-only Work-to-Review handoff | [x] | +| REFACTOR-3: Reviewer-owned repair, terminal output, and cleanup | [x] | +| REFACTOR-4: Config and contract synchronization | [x] | + +## Implementation Checklist + +- [x] [REFACTOR-1] Implement the compact PLAN/REVIEW template grammar, renderers, and parsers with fail-closed tests. +- [x] [REFACTOR-2] Make Work write one REVIEW handoff and make Review consume stored PLAN/REVIEW artifacts without writing a final review page. +- [x] [REFACTOR-3] Make Review inherit the work, perform bounded repairs with re-verification, return strict terminal output, and rely on cleanup to discard PLAN/REVIEW. +- [x] [REFACTOR-4] Synchronize config tests and current contracts/specs without creating any RESULT artifact or benchmark harness. +- [x] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. + +## Review-Only Checklist + +> **[REVIEW AGENT ONLY]** This checklist is used only by the review agent. +> Implementing agents must not modify or check this section. + +- [x] Append one verdict of `PASS`, `WARN`, or `FAIL` and verified `review_rework_count`, `evidence_integrity_failure` to `Code Review Result`. +- [x] Verify that verdict, `Dimension Assessment`, and Required/Suggested/Nit classifications match. +- [x] Run applicable required verification and record fresh command/output; repair reviewer-reconstructable evidence gaps instead of forwarding them to another plan. +- [x] For every Required/Suggested finding, record reviewer-collected `Evidence`, exact `Root Cause`, and one `Selected Fix` with affected files/symbols/tests and acceptance commands before creating a follow-up plan. +- [x] Archive active `CODE_REVIEW-*-G??.md` to `code_review_{review_lane}_{review_grade}_{review_log_number}.log`. +- [x] Archive active `PLAN-*-G??.md` to `plan_{build_lane}_{build_grade}_{plan_log_number}.log`. +- [x] Verify that the Agent-Ops managed block in `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores `agent-roadmap/current.md`. +- [ ] If PASS, write `complete.log` based on `agent-ops/skills/common/code-review/templates/complete-log-template.md` and leave no active `.md` files. +- [ ] If PASS, move active task directory `agent-task/single_request_artifact_handoff/` to `agent-task/archive/YYYY/MM/single_request_artifact_handoff/` and update this checklist at the final archive path. +- [ ] If PASS for split work, remove empty active parent `agent-task/single_request_artifact_handoff/` or verify it was kept due to remaining siblings/files. +- [x] If WARN/FAIL, write the next filesystem state matching code-review verdict and do not write `complete.log`. + +## Deviations from Plan + +- 기존 service-level Work-only fixture와 기존 final-REVIEW snapshot fixture는 artifact-only executor coverage와 중복되어 skip 처리했다. 새 구현의 Work write/Review read/zero-write/repair 조건은 stage 및 executor 테스트가 검증한다. 별도 runtime artifact selector나 RESULT/benchmark harness는 만들지 않았다. +- 실제 Claude/Node endpoint와 credential이 현재 local baseline에 구성되지 않아 live acceptance는 실행하지 못했다. 재개 조건은 승인된 Node와 marked preset을 갖춘 endpoint에서 작은 HTML 1건을 한 ingress로 실행하는 것이다. + +## Key Design Decisions + +- PLAN renderer가 단계마다 `P1..Pn`을 결정적으로 부여하고 Work는 저장된 PLAN에서 해당 ID를 다시 파싱한다. +- Work의 strict JSON은 worker item status, changes, verification, deviations로 한정하고, Edge가 REVIEW handoff를 한 번 렌더·검증·기록한다. +- Review request에서 메모리 Work 결과를 제거했다. Review는 provider 호출 전에 controller로 PLAN/REVIEW를 다시 읽고 검증한다. +- Review는 final REVIEW artifact를 쓰지 않는다. repair mutation 뒤 성공한 inspection 없이는 PASS할 수 없고 terminal output은 reviewer `output` 원문만 사용한다. + +## Reviewer Checkpoints + +- [ ] Review request has no authoritative in-memory Work result payload. +- [ ] PLAN renders deterministic `P1..Pn` item IDs and REVIEW covers every ID exactly once with no missing, duplicate, or unknown item. +- [ ] Work validates PLAN and writes a REVIEW handoff containing every plan item's status, actual changes, verification evidence, and explicit deviations before stage success. +- [ ] Review reads and validates PLAN/REVIEW before provider dispatch. +- [ ] Reviewer inherits PLAN, REVIEW handoff, and bounded workspace tools; discovered defects are repaired directly and re-verified before PASS. +- [ ] Review performs zero REVIEW artifact writes and does not generate a final review page; total REVIEW writes equal the Work handoff write only. +- [ ] Caller terminal output is exactly reviewer `output`, never worker completion or reviewer `summary`. +- [ ] Success, failure, and cancellation use the existing cleanup path to discard transient PLAN/REVIEW artifacts. +- [ ] Old custom Review grammar fails closed with a documented migration boundary. +- [ ] No RESULT document/template or benchmark harness was created, and no new production artifact/wire was added. +- [ ] Failure, cancellation, repair, timeout, waiter cleanup, and concurrent request isolation remain closed. + +## Verification Results + +### Template and config tests + +```bash +go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +``` + +PASS (2026-08-14): `ok iop/packages/go/singlerequesttemplate`; `ok iop/packages/go/config`. + +### Edge focused tests + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +``` + +PASS (2026-08-14): `ok iop/apps/edge/internal/openai`. + +### Race tests + +```bash +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +``` + +PASS (2026-08-14): `ok iop/apps/edge/internal/openai`. + +### Broader local regression + +```bash +go test -count=1 ./apps/edge/... ./packages/go/... +git diff --check +``` + +PASS (2026-08-14): `go test -count=1 ./apps/edge/... ./packages/go/...` completed successfully; `git diff --check` passed. + +Reviewer fresh verification (2026-08-14): + +```text +$ go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +ok iop/packages/go/singlerequesttemplate +ok iop/packages/go/config +$ go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +ok iop/apps/edge/internal/openai +$ go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +ok iop/apps/edge/internal/openai +$ go test -count=1 ./apps/edge/... ./packages/go/... +ok iop/apps/edge/... and iop/packages/go/... (all listed packages passed) +$ git diff --check +(no output; exit 0) +$ go vet ./apps/edge/internal/service +(no output; exit 0) +$ go test ./apps/edge/internal/service -count=1 +ok iop/apps/edge/internal/service +$ go vet ./packages/go/... +(no output; exit 0) +``` + +Reviewer focused reproducers were added temporarily, executed, and removed after observation: + +```text +$ go test -count=1 -run 'TestReviewerRepro' -v ./packages/go/singlerequesttemplate +=== RUN TestReviewerReproStoredPlanTemplateMismatchIsAccepted +--- PASS: TestReviewerReproStoredPlanTemplateMismatchIsAccepted (0.00s) +=== RUN TestReviewerReproItemStatusProseIsAccepted +--- PASS: TestReviewerReproItemStatusProseIsAccepted (0.00s) +PASS +$ go test -count=1 -run 'TestReviewerReproCommandCannotVerifyRepair' -v ./apps/edge/internal/openai +=== RUN TestReviewerReproCommandCannotVerifyRepair +--- PASS: TestReviewerReproCommandCannotVerifyRepair (0.00s) +PASS +``` + +### Live single-request acceptance + +작은 HTML 한 건을 단독 실행해 PLAN write 1회, REVIEW handoff write 1회, reviewer REVIEW write 0회, terminal 1회, 종료 후 PLAN/REVIEW cleanup과 workspace/caller output 일치를 확인한다. + +미실행: local baseline에는 실제 marked single-request endpoint, 승인 Node, provider credential이 없다. 해당 환경에서 작은 HTML 1건으로 ingress 1회, PLAN write 1회, Work REVIEW write 1회, reviewer REVIEW write 0회, terminal 1회, cleanup 후 artifact 부재와 caller/workspace output 일치를 확인해야 한다. + +--- + +> **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, Overview, Review Agent Instructions | Fixed at stub creation | Implementing agent must not modify or execute these (archive, complete.log, and task-directory archive move are review-agent only) | +| Implementation Item Completion (item names) | Fixed at stub creation | Implementing agent checks `[ ]` → `[x]` only | +| Implementation Checklist (item text/order) | Fixed at stub creation from plan | Implementing agent checks `[ ]` → `[x]` only | +| Review-Only Checklist | Review agent only | Implementing agent must not modify or check this section | +| Deviations from Plan, Key Design Decisions | Implementing agent | Replace placeholder text with actual content | +| Reviewer Checkpoints | Fixed at stub creation | Pre-filled from plan | +| Verification Results (section headings + commands) | Implementing agent, then review agent | Implementing agent records initial output; review agent reruns applicable commands and may fill, replace, or append fresh verified output before verdict. Implementing-agent command changes require a `Deviations from Plan` entry | +| Code Review Result | Review agent appends | Not included in stub | + +## Code Review Result + +- Overall Verdict: FAIL +- Dimension Assessment: + - Correctness: Fail + - Completeness: Fail + - Test coverage: Fail + - API contract: Fail + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- Findings: + - Required R1 — Stored artifact grammar is not validated fail-closed end to end. + - Evidence: The reviewer repro `TestReviewerReproStoredPlanTemplateMismatchIsAccepted` showed that `ParsePlan(customTemplate, storedPlan)` rejects a stored PLAN that omits the frozen custom marker while the Work-stage parser `PlanItemIDs(storedPlan)` accepts it. `single_request_work_stage.go:221` and `single_request_review_stage.go:101` call only `PlanItemIDs`, so the frozen PLAN template is not checked before either provider dispatch. The second repro showed `ValidateReviewHandoff` accepts `unexpected prose` before otherwise valid status bullets because `template.go:558-584` validates regex matches but not every status-section line. This violates the plan's strict stored-PLAN parser and one-bullet-per-plan-item handoff invariants. + - Root Cause: `PlanItemIDs` was used as both an ID extractor and the complete stored PLAN parser, bypassing the existing exact `ParsePlan` contract; `ValidateReviewHandoff` compares only matched status lines and never requires the status section's complete line inventory to match the grammar. + - Selected Fix: In `single_request_work_stage.go` and `single_request_review_stage.go`, validate the stored PLAN with `singlerequesttemplate.ParsePlan(binding.Templates.Plan, string(plan), req.Limits.MaxOutputBytes)` before extracting IDs. In `template.go`, make `ValidateReviewHandoff` split the entire item-status section and require every line, in order, to match exactly `- Pn: completed`, with the line count equal to `planIDs`. Add regression cases in `template_test.go`, `single_request_work_stage_test.go`, and `single_request_review_stage_test.go` for frozen-template mismatch, prose/malformed status lines, and provider non-dispatch on malformed artifacts. + - Required R2 — A successful command cannot satisfy the documented post-repair verification path. + - Evidence: The reviewer repro `TestReviewerReproCommandCannotVerifyRepair` ran write → successful `workspace_command` → PASS and observed stage rejection. At `single_request_review_stage.go:162`, only read/list are inspections; at lines 220-228 every successful command resets `verifiedAfterMutation=false` and only read/list can set it true. This contradicts the acceptance invariant allowing a successful follow-up read/command verification. + - Root Cause: The ledger uses one `isInspection` classification for lifecycle stage selection, mutation tracking, and verification evidence. Because `workspace_command` belongs to the repair-tool set, it is always treated as a new mutation even when it follows an existing repair as the verification command. + - Selected Fix: Separate lifecycle/tool admission classification from ledger semantics in `single_request_review_stage.go`. Preserve write/delete as mutations; when a successful command follows an already-recorded mutation, count it as post-mutation verification without clearing the mutation ledger. Keep a command used before any prior mutation repair-owned and require later verification. Add direct stage and coordinator regression tests in `single_request_review_stage_test.go` for write→command→PASS success and command-first→PASS rejection. +- Routing Signals: + - review_rework_count=1 + - evidence_integrity_failure=false +- Next Step: Archive this pair and materialize the routed follow-up PLAN/review pair for direct fixes R1 and R2; do not write `complete.log`. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/complete.log b/agent-task/archive/2026/08/single_request_artifact_handoff/complete.log new file mode 100644 index 00000000..aed2c252 --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/complete.log @@ -0,0 +1,42 @@ + + +# Complete - single_request_artifact_handoff + +## 완료 일시 + +2026-08-14 + +## 요약 + +3회 리뷰 루프에서 strict artifact handoff와 command repair ledger를 보완했으며 최종 판정은 PASS다. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_local_G07_0.log` | `code_review_cloud_G07_0.log` | FAIL | frozen PLAN/REVIEW grammar와 command post-repair verification 보완 필요 | +| `plan_local_G04_1.log` | `code_review_cloud_G05_1.log` | FAIL | `not_found` 뒤 command repair가 `repairRequired`를 해제하지 않는 R3 확인 | +| `plan_cloud_G05_2.log` | `code_review_cloud_G05_2.log` | PASS | command repair gate 해제와 후속 verification 회귀를 검증 | + +## 구현/정리 내용 + +- `not_found` 뒤 첫 성공 `workspace_command`가 repair gate를 해제하되 mutation ledger를 유지하여 이후 검증 전 PASS를 차단하도록 수정했다. +- direct stage와 coordinator 수준에서 command repair, 재검증, 단일 terminal, cleanup, waiter 해제를 검증했다. + +## 최종 검증 + +- `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)'` - PASS; `ok iop/apps/edge/internal/openai 0.101s` +- `go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)'` - PASS; `ok iop/apps/edge/internal/openai 1.895s` +- `go vet ./apps/edge/internal/service` - PASS; 출력 없음 +- `go test ./apps/edge/internal/service -count=1` - PASS; `ok iop/apps/edge/internal/service 8.305s` +- `go test -count=1 ./apps/edge/... ./packages/go/...` - PASS; 선택한 Edge/common 패키지 전체 통과 +- `git diff --check` - PASS; 출력 없음 +- 실제 marked single-request Claude full-cycle - BLOCKED; local baseline에 configured endpoint, approved Node, provider credential, remote runner가 없으며 가용 시 1건의 작은 HTML 작업으로 재개한다. + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/plan_cloud_G05_2.log b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_cloud_G05_2.log new file mode 100644 index 00000000..e20f93ca --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_cloud_G05_2.log @@ -0,0 +1,137 @@ + + +# Plan - REVIEW_REVIEW_REFACTOR: Close Command Repair Ledger + +## For the Implementing Agent + +Implement only the selected R3 fix and its regressions. Run every command below, fill implementation-owned sections in `CODE_REVIEW-cloud-G05.md` with actual output, and leave the active pair in place. Do not reopen diagnosis, ask the user, classify the next state, archive files, or write `complete.log`. + +## Background + +The prior repair ledger correctly accepts write→command verification but leaves `repairRequired` set when a command is the repair after a `not_found` inspection. A subsequent verification command therefore cannot reach PASS. This follow-up closes that state transition without changing artifact grammar, caller output, or workspace authority. + +## Archive Evidence Snapshot + +- Prior task path: `agent-task/single_request_artifact_handoff/` +- Prior plan: `agent-task/single_request_artifact_handoff/plan_local_G04_1.log` +- Prior review: `agent-task/single_request_artifact_handoff/code_review_cloud_G05_1.log` +- Verdict: FAIL; Required R3 command-repair ledger state; Suggested/Nit: none. +- Reviewer verification: focused, race, broader Edge/common Go tests, vet, and `git diff --check` passed. Static review proved that command repair after `not_found` never clears `repairRequired`. +- Affected files: `apps/edge/internal/openai/single_request_review_stage.go`, `apps/edge/internal/openai/single_request_review_stage_test.go`. +- Roadmap carryover: none; this is a non-milestone task. + +## Finding Resolution Map + +| Finding | Reviewer evidence | Root cause | Selected fix | Mode | Changed precondition | Acceptance commands | +|---|---|---|---|---|---|---| +| Required R3 | `not_found` sets `repairRequired`; command-first sets mutation but does not clear it, so the final PASS guard rejects even after a later command verification. | Command repair is tracked as a mutation but is not allowed to discharge the outstanding repair gate. | On successful `workspace_command`, clear `repairRequired` when it is the outstanding repair; retain mutation and require a later verification. Add direct and coordinator regressions for not_found→command repair→command verification→PASS. | direct-fix | The repair gate is cleared only by a successful command repair before the later verification. | `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)'`; `go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)'` | + +## Analysis + +### Files Read + +- `apps/edge/internal/openai/single_request_review_stage.go` +- `apps/edge/internal/openai/single_request_review_stage_test.go` +- `agent-task/single_request_artifact_handoff/code_review_cloud_G05_1.log` + +### SDD Criteria + +Not applicable. The change repairs an existing request-local review ledger and does not add a subsystem, wire, schema, or external API. + +### Verification Context + +- Local Go tests use `-count=1`; the reviewer already confirmed focused, race, broader Edge/common, vet, and diff checks in the current worktree. +- The local baseline has no approved marked single-request endpoint, Node, credential, or remote runner. Preserve this live-acceptance resume condition; deterministic tests do not claim external qualification. +- The selected fix is source-consistent with the existing `repairRequired`, `mutationOccurred`, and `verifiedAfterMutation` state machine. + +### Split Judgment + +Keep one plan: the state transition and its direct/coordinator tests are one atomic review-ledger invariant. + +### Routing + +- evaluation_mode: isolated-reassessment +- build: cloud/G05, route basis `recovery-boundary` +- review: cloud/G05, route basis `official-review` +- grade scores: build `1+1+1+1+1=5`; review `1+1+1+1+1=5` +- large_indivisible_context=false +- matched loop risks: `temporal_state` (1) +- review_rework_count=2; evidence_integrity_failure=false + +## Implementation Items + +### [REVIEW_REVIEW_REFACTOR-1] Clear the repair gate after a successful command repair + +**Problem:** At `single_request_review_stage.go:235-244`, a successful command with no preceding mutation marks a repair mutation but leaves the `not_found`-derived `repairRequired` flag set. The PASS guard at lines 142-144 then rejects the request even after a subsequent verification command. + +**Solution:** In the successful command branch, when `repairRequired` is true and the command is serving as the repair, set it false while keeping `mutationOccurred=true` and `verifiedAfterMutation=false`. Preserve the existing command-after-mutation behavior as verification and do not relax the command-first PASS rejection. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/single_request_review_stage.go` — align command repair with the outstanding repair gate and verification ledger. + +**Test Strategy:** Extend existing direct-stage coverage with `not_found`→command repair→command verification→PASS and assert no reviewer REVIEW write or pending waiter. Extend coordinator coverage for the same sequence, asserting one Work-owned REVIEW write, one finalizing terminal, cleanup, and zero pending waiters. + +**Verification:** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)' +``` + +Expected: both command-repair transitions pass and command-first remains rejected. + +### [REVIEW_REVIEW_REFACTOR-2] Add regression coverage for the repaired command path + +**Problem:** Existing tests cover write→command and command-first, but not a `not_found` repair performed by an approved command followed by command verification. + +**Solution:** Add the direct and coordinator regression fixtures described above in the existing review-stage test file. Keep provider/tool sequences deterministic and assert lifecycle ownership rather than adding a live runner. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/single_request_review_stage_test.go` — cover the missing direct and coordinator command-repair transition. + +**Test Strategy:** New regressions must prove final PASS only after the second command and retain the existing command-first rejection behavior. + +**Verification:** + +```bash +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +``` + +Expected: race coverage passes with no waiter or terminal lifecycle leak. + +## Modified Files Summary + +| File | Action | +|---|---| +| `apps/edge/internal/openai/single_request_review_stage.go` | modify | +| `apps/edge/internal/openai/single_request_review_stage_test.go` | modify | + +## Reviewer Checkpoints + +- [ ] A successful command used after `not_found` clears the outstanding repair gate but still requires a later verification. +- [ ] The later successful command verification permits one terminal PASS; command-first still cannot PASS. +- [ ] Direct and coordinator tests prove zero pending waiters, Work-only REVIEW write count, cleanup, and one finalizing terminal. +- [ ] Existing artifact grammar, cancellation, and race tests remain closed. + +## Implementation Checklist + +- [ ] [REVIEW_REVIEW_REFACTOR-1] Clear the command-repair gate while preserving the later verification requirement. +- [ ] [REVIEW_REVIEW_REFACTOR-2] Add direct and coordinator regressions for command repair after `not_found`. +- [ ] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [ ] Fill implementation-owned sections in `CODE_REVIEW-*-G??.md` with actual implementation notes and verification output. + +## Final Verification + +Fresh execution is required. + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage(CommandVerificationAfterMutation|CoordinatorRepairsMissingArtifact)' +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +go test -count=1 ./apps/edge/... ./packages/go/... +git diff --check +``` + +Expected: every command exits 0. When an approved marked endpoint, Node, and credential are available, run one small HTML request and verify one ingress, one PLAN write, one Work REVIEW write, zero reviewer REVIEW writes, one terminal, cleanup, and exact workspace/caller output. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G04_1.log b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G04_1.log new file mode 100644 index 00000000..188f7831 --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G04_1.log @@ -0,0 +1,167 @@ + + +# Plan - REVIEW_REFACTOR: Close Artifact Grammar and Repair Verification + +## For the Implementing Agent + +Implement the selected fixes below without reopening diagnosis or changing ownership. Run every applicable verification command, fill the implementation-owned sections of `CODE_REVIEW-cloud-G05.md` with actual notes/output, keep the active files in place, and report ready for review. If blocked, record only the exact blocker, attempted command/output, and resume condition in implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`; finalization belongs to the code-review agent. + +## Background + +The first implementation moved Work→Review handoff authority to PLAN/REVIEW artifacts, but reviewer reproducers found two fail-closed gaps. Stored PLAN validation can bypass the frozen custom template, status prose can bypass the item grammar, and a command cannot serve as the documented verification after repair. This follow-up applies the reviewer-selected direct fixes only. + +## Archive Evidence Snapshot + +- Prior task path: `agent-task/single_request_artifact_handoff/` +- Prior plan: `agent-task/single_request_artifact_handoff/plan_local_G07_0.log` +- Prior review: `agent-task/single_request_artifact_handoff/code_review_cloud_G07_0.log` +- Verdict: FAIL; Required R1 strict artifact grammar, Required R2 command-based post-repair verification; Suggested/Nit: none. +- Reviewer verification: focused, race, broad Go regression, vet, and `git diff --check` passed; temporary focused reproducers proved all three acceptance gaps. +- Affected files: `packages/go/singlerequesttemplate/template.go`, `packages/go/singlerequesttemplate/template_test.go`, `apps/edge/internal/openai/single_request_work_stage.go`, `apps/edge/internal/openai/single_request_work_stage_test.go`, `apps/edge/internal/openai/single_request_review_stage.go`, `apps/edge/internal/openai/single_request_review_stage_test.go`. +- Roadmap carryover: none; this is a non-milestone task. + +## Finding Resolution Map + +| Finding | Reviewer evidence | Root cause | Selected fix | Mode | Changed precondition | Acceptance commands | +|---|---|---|---|---|---|---| +| Required R1 | Reviewer repro accepted a stored PLAN that failed `ParsePlan` against its frozen custom template and accepted prose inside item status. | Work/Review use `PlanItemIDs` as a complete parser; handoff validation checks regex matches rather than every status line. | Parse stored PLAN against `binding.Templates.Plan` before ID extraction in both stages; require the complete item-status line inventory to match ordered `- Pn: completed`; add non-dispatch regressions. | direct-fix | Strict parser behavior changes before verification repeats. | `go test -count=1 ./packages/go/singlerequesttemplate`; `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage)'` | +| Required R2 | Reviewer repro write→successful command→PASS was rejected by the ledger. | One `isInspection` flag conflates lifecycle/admission, mutation, and verification; command always resets verification. | Separate ledger semantics: write/delete record mutation; a successful command after an existing mutation records verification, while command-first remains repair-owned and still requires later verification; add stage/coordinator regressions. | direct-fix | The command-after-mutation transition changes before verification repeats. | `go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage'`; `go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)'` | + +## Analysis + +### Files Read + +- `packages/go/singlerequesttemplate/template.go` +- `packages/go/singlerequesttemplate/template_test.go` +- `apps/edge/internal/openai/single_request_work_stage.go` +- `apps/edge/internal/openai/single_request_work_stage_test.go` +- `apps/edge/internal/openai/single_request_review_stage.go` +- `apps/edge/internal/openai/single_request_review_stage_test.go` +- `agent-task/single_request_artifact_handoff/plan_local_G07_0.log` +- `agent-task/single_request_artifact_handoff/code_review_cloud_G07_0.log` + +### SDD Criteria + +Not applicable. This is a direct correction of the existing PLAN/REVIEW internal artifact invariant and adds no new external subsystem or wire. + +### Verification Context + +- Environment: local checkout, Go available (`go1.26.2 linux/arm64`); tests use `-count=1` where fresh execution matters. +- Repository-native evidence: focused, race, broad package tests, `go vet`, and `git diff --check` passed before this follow-up. +- Required external acceptance remains unavailable in the local baseline: no configured marked single-request endpoint, approved Node, provider credential, or remote runner. Do not treat package tests as a substitute; retain the exact resume condition in review evidence. +- Confidence: high for R1/R2 diagnosis and selected fixes because focused reproducers directly exercised the wrong branches. + +### Split Judgment + +Keep one plan. The stored artifact parser and reviewer ledger jointly protect the same Plan→Work→Review terminal invariant, and the patch is compact enough that splitting would not yield an independently releasable intermediate state. + +### Routing + +- evaluation_mode: isolated-reassessment +- build: local/G04, route basis `local-fit` +- review: cloud/G05, route basis `official-review` +- grade scores: build `1+1+1+0+1=4`; review `1+1+1+1+1=5` +- large_indivisible_context=false +- matched loop risks: `temporal_state`, `boundary_contract`, `structured_interpretation` (3) +- review_rework_count=1; evidence_integrity_failure=false + +## Implementation Items + +### [REVIEW_REFACTOR-1] Enforce frozen PLAN and complete REVIEW item grammar + +**Problem:** `single_request_work_stage.go:221` and `single_request_review_stage.go:101` extract IDs without checking the stored PLAN against `binding.Templates.Plan`. `template.go:558-584` ignores non-matching lines in Worker Item Status. + +**Solution:** Before `PlanItemIDs`, call the existing exact parser with the frozen template and stored bytes: + +```go +if _, err := singlerequesttemplate.ParsePlan(binding.Templates.Plan, string(plan), req.Limits.MaxOutputBytes); err != nil { + return quality.malformed(errSingleRequestWorkStage) +} +``` + +Use the equivalent Review-stage error path. In `ValidateReviewHandoff`, split the entire status section, require `len(lines) == len(planIDs)`, and match each complete line against its exact expected ID/status; do not filter unmatched lines before comparison. + +**Modified Files and Checklist:** + +- [ ] `packages/go/singlerequesttemplate/template.go` — make item-status validation consume every line. +- [ ] `packages/go/singlerequesttemplate/template_test.go` — add prose, malformed bullet, blank-line, and exact ordered status regressions. +- [ ] `apps/edge/internal/openai/single_request_work_stage.go` — parse stored PLAN against the frozen Plan template before dispatch. +- [ ] `apps/edge/internal/openai/single_request_work_stage_test.go` — prove a template-mismatched PLAN fails before provider dispatch/REVIEW write. +- [ ] `apps/edge/internal/openai/single_request_review_stage.go` — apply the same frozen PLAN parse before reading/dispatching Review. +- [ ] `apps/edge/internal/openai/single_request_review_stage_test.go` — prove mismatched PLAN and malformed REVIEW handoff fail before provider dispatch. + +**Test Strategy:** Write the named regression cases in the existing table/stage tests. Reuse existing controllers and custom-template fixtures; assert provider call count and artifact writes stay zero for malformed stored artifacts. + +**Verification:** + +```bash +go test -count=1 ./packages/go/singlerequesttemplate +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage)' +``` + +Expected: all tests pass and the new malformed-artifact cases observe no provider dispatch. + +### [REVIEW_REFACTOR-2] Accept command verification after an existing repair + +**Problem:** `single_request_review_stage.go:162` classifies only read/list as inspection, while lines 220-228 reset verification after every successful command. This rejects the specified write→command verification path. + +**Solution:** Keep tool admission/lifecycle classification separate from ledger updates. A successful write/delete records mutation and clears verification. A successful command with `mutationOccurred=true` records post-mutation verification without clearing the ledger. A command with no prior mutation remains repair-owned/mutating and therefore cannot immediately PASS without a later successful read/list/command verification. + +**Modified Files and Checklist:** + +- [ ] `apps/edge/internal/openai/single_request_review_stage.go` — separate command verification from write/delete mutation ledger updates. +- [ ] `apps/edge/internal/openai/single_request_review_stage_test.go` — add write→command→PASS success and command-first→PASS rejection at direct-stage and coordinator-relevant coverage. + +**Test Strategy:** Add deterministic scripted provider/tool sequences using the existing `verify` command fixture. Assert terminal/finalizing state only after valid command verification and zero pending waiters in both success and rejection paths. + +**Verification:** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequestReviewStage' +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +``` + +Expected: both command-ledger regressions and the existing repair/correlation/race tests pass. + +## Modified Files Summary + +| File | Action | +|---|---| +| `packages/go/singlerequesttemplate/template.go` | modify | +| `packages/go/singlerequesttemplate/template_test.go` | modify | +| `apps/edge/internal/openai/single_request_work_stage.go` | modify | +| `apps/edge/internal/openai/single_request_work_stage_test.go` | modify | +| `apps/edge/internal/openai/single_request_review_stage.go` | modify | +| `apps/edge/internal/openai/single_request_review_stage_test.go` | modify | + +## Reviewer Checkpoints + +- [ ] Work and Review reject a stored PLAN that does not match the frozen effective Plan template before provider dispatch. +- [ ] Worker Item Status contains exactly one full grammar line per ordered PLAN ID and rejects all extra prose/malformed/blank lines. +- [ ] write/delete mutation still blocks PASS until later successful verification. +- [ ] A successful command after an existing mutation satisfies post-repair verification; command-first does not bypass the gate. +- [ ] REVIEW artifact write count remains Work-only and caller terminal output remains reviewer `output`. +- [ ] Existing cancellation, waiter cleanup, bounds, and race tests remain closed. + +## Implementation Checklist + +- [ ] [REVIEW_REFACTOR-1] Enforce frozen PLAN parsing and complete REVIEW item-status grammar with non-dispatch regressions. +- [ ] [REVIEW_REFACTOR-2] Separate repair mutation and command verification ledger semantics with success/rejection regressions. +- [ ] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [ ] Fill implementation-owned sections in `CODE_REVIEW-cloud-G05.md` with actual implementation notes and verification output. + +## Final Verification + +Fresh execution is required; cached Go test output is not acceptable. + +```bash +go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +go test -count=1 ./apps/edge/... ./packages/go/... +git diff --check +``` + +Expected: every command exits 0. When a configured approved marked single-request endpoint, Node, and credential are available, run one small HTML request and verify one ingress, one PLAN write, one Work REVIEW write, zero reviewer REVIEW writes, one terminal, cleanup, and exact workspace/caller output. If unavailable, record the exact target/preflight gap and resume condition; do not claim local tests replace this acceptance. + +After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G07_0.log b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G07_0.log new file mode 100644 index 00000000..337c215f --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/plan_local_G07_0.log @@ -0,0 +1,207 @@ + + +# Plan - REFACTOR: Restore Model-to-Model Artifact Handoff + +## For the Implementing Agent + +이 계획은 single-request의 모델 간 전달을 메모리 객체가 아니라 검증된 `PLAN`/`REVIEW` 문서로 고정한다. 현재 구조와 기존 PLAN/REVIEW artifact selector를 재사용하고, 새 wire·artifact 종류·범용 하네스를 만들지 않는다. + +구현 완료 뒤 대응하는 `CODE_REVIEW-cloud-G07.md`의 구현 담당 구간을 채우고 리뷰 대기 상태로 남긴다. + +## Background + +현재 구현은 hybrid가 아니다. 서로 다른 모델을 세 번 순서대로 호출하지만, worker의 실제 작업 인계 문서가 없기 때문에 책임이 다음 모델로 이어지는 구조가 아니다. 현재 흐름은 정확히 다음과 같다. + +`planner JSON → PLAN artifact → worker JSON(completion, verification) → Go memory → reviewer JSON → 사후 REVIEW artifact` + +여기서 reviewer에게 전달되는 것은 worker가 작성한 REVIEW 문서가 아니라 Edge가 메모리에 들고 있는 두 개의 요약 필드뿐이다. workspace 변경 내용은 남더라도 reviewer는 worker의 의도, 수행 범위, 변경 근거와 검증 결과를 하나의 정형화된 인계물로 받지 못한다. 따라서 현재 상태는 "hybrid 실행"이 아니라 여러 모델을 직렬 호출한 pipeline이다. + +사용자가 요구한 실제 hybrid 실행은 다음 세 책임을 분리하고 문서로 연결해야 한다. + +1. planner가 간결한 PLAN을 작성하고 worker가 그 문서를 읽는다. +2. worker가 작업 내용과 검증 근거를 REVIEW 인계 문서에 기록하고 reviewer가 PLAN·REVIEW·workspace 작업 권한을 이어받아 검토, 필요한 repair, 재검증, 최종 출력을 완성한다. +3. RESULT는 벤치마크 결과지일 뿐이며 IOP 런타임 artifact나 모델 간 호출 계약에 포함하지 않는다. + +Plan stage만 JSON 응답을 템플릿으로 렌더링해 PLAN artifact를 쓰고 Work stage가 이를 읽는다. 그러나 Work stage의 `completion`/`verification`은 REVIEW 인계 문서로 저장되지 않고 `singleRequestWorkResult` 메모리 값으로 Executor를 거쳐 Review stage에 직접 전달된다. 현재 Review stage가 쓰는 REVIEW artifact는 reviewer PASS 뒤의 불필요한 사후 기록물이라 worker→reviewer handoff 역할을 하지 않으면서 추가 write 비용만 만든다. reviewer JSON의 strict `output`을 caller에게 반환하는 경로는 유지하되, worker handoff·repair 상태·재검증 조건을 닫아야 한다. + +## Analysis + +### Root Cause + +- `single_request_work_stage.go`는 PLAN artifact를 읽지만 렌더된 PLAN의 필수 구간을 다시 검증하지 않고, 작업 완료 후 REVIEW artifact를 쓰지 않는다. +- `single_request_executor.go`가 `singleRequestWorkResult`를 Review request에 넣어 전달하면서 저장된 artifact가 아닌 프로세스 메모리가 권위 있는 handoff가 됐다. +- `single_request_review_stage.go`는 PLAN만 읽고 worker 결과는 메모리 객체에서 가져온다. REVIEW template도 reviewer 전용 `Result/Checks/Verification/Summary`만 표현한다. +- `singlerequesttemplate`의 Review grammar가 reviewer 사후 기록용이라 worker item 상태·변경·검증·이탈을 다음 모델에 넘기는 인계 문서로 사용할 수 없다. + +### Selected Design + +- PLAN의 현재 최소 구조(`Goal`, `Steps`, `Verification`)는 유지한다. Edge renderer가 `Steps`에 `P1`, `P2` 같은 요청 내부 결정적 ID를 붙이고, Work stage가 저장된 PLAN을 strict parser로 검증한 뒤 문서 전체를 worker 모델에 전달한다. +- REVIEW template은 worker→reviewer 인계에만 사용한다. 필수 구간은 `Worker Item Status`, `Worker Changes`, `Worker Verification`, `Deviations` 네 개뿐이다. reviewer 판정이나 최종 페이지를 위한 구간은 두지 않는다. +- Work stage는 strict JSON으로 각 plan item ID와 완료 상태, 실제 변경 내용, 검증 명령/결과, 계획 이탈 여부를 반환하고 REVIEW 인계 문서로 렌더링한다. 모든 PLAN ID가 정확히 한 번 등장하고 성공 경로에서는 모두 `completed`여야 한다. `completion` 한 문장으로 축약하지 않고 REVIEW artifact를 한 번 쓴 뒤 종료한다. +- Review stage는 PLAN과 REVIEW를 controller에서 다시 읽고 grammar/크기/필수 구간을 검증한다. Review request에서 `Work *singleRequestWorkResult`를 제거해 artifact가 유일한 worker handoff가 되게 한다. +- reviewer는 draft를 읽은 뒤 승인만 하는 gate가 아니다. inspection에서 결함을 찾으면 허용된 workspace tool로 직접 repair하고 반드시 재검증한 뒤에만 PASS할 수 있다. Edge는 성공한 mutating tool call을 요청 내부 repair ledger에 기록한다. +- reviewer는 최종 REVIEW 페이지를 작성하거나 REVIEW artifact를 덮어쓰지 않는다. 성공한 mutation과 후속 검증은 요청 내부 ledger로만 추적해 PASS 조건에 사용하고, strict reviewer 응답의 `output`을 repair 이후 최종 결과로 반환한다. terminal 뒤 기존 cleanup 경로가 PLAN/REVIEW를 폐기한다. +- 기존 custom `review_file`의 옛 grammar는 조용히 호환하지 않는다. 새 grammar가 아니면 config load/admission에서 fail closed하고 계약 문서에 migration requirement를 기록한다. 저장소 내 tracked custom template은 현재 확인되지 않았다. +- RESULT는 이 작업에서 만들거나 복원하지 않는다. 벤치 결과지 재설계는 별도 측정 경계의 후속 작업이며 IOP runtime package/config/wire와 이번 변경 범위 밖이다. + +### Scope Boundary + +포함: + +- 기존 PLAN/REVIEW template grammar와 렌더/파싱 +- Work→REVIEW handoff→Reviewer의 artifact handoff +- reviewer의 inspection→repair→재검증 소유권과 strict terminal output provenance +- 관련 config grammar, runtime contract/spec, focused tests + +제외: + +- 새 proto, Node selector, artifact enum 또는 `.iop/job` 파일 종류 +- benchmark runner/harness, 점수 계산 엔진, 모델 호출 재측정 +- provider normalization, effort routing, 모델/에이전트 조합 변경 +- RESULT 문서/템플릿/채점표 재작성 +- roadmap/milestone 재개 또는 다음 route 구현 + +### Split Decision + +분리하지 않는다. template grammar, Work write, Review read/repair, terminal output, cleanup이 하나의 폐쇄된 불변식이며 일부만 배포하면 기존 handoff보다 더 불안정해진다. RESULT는 이 불변식에 포함되지 않으므로 작업에서도 제외한다. + +### SDD Criteria + +Not applicable. 기존 single-request 설계와 PLAN/REVIEW artifact 계약의 교정이며 새 외부 제품 개념이나 독립 하위 시스템을 만들지 않는다. + +## Acceptance Invariants + +- Work는 저장된 PLAN을 읽고 strict validation에 통과한 경우에만 모델을 호출한다. +- 렌더된 PLAN의 각 Step은 요청 내부에서 결정적인 `P1..Pn` ID를 가지며 중복되거나 비어 있을 수 없다. +- Work 성공은 non-empty REVIEW handoff write 1회 성공을 포함한다. write 실패 시 Review로 진행하지 않는다. +- Review는 저장된 PLAN과 REVIEW handoff만으로 입력을 구성하며 Work 결과 메모리 객체에 의존하지 않는다. +- REVIEW는 네 worker 구간이 모두 non-empty이고 PLAN의 각 `P1..Pn`을 정확히 한 번씩 `completed`로 참조하며 unknown/duplicate/missing ID가 없어야 한다. 계획 이탈이 없더라도 `Deviations`에는 명시적인 `None`이 있어야 한다. +- Edge가 `not_found` 등 repair-required typed 상태를 추적 중이면 이를 해소하는 mutation 전에는 PASS를 허용하지 않는다. mutation이 발생한 뒤에는 최소 한 번의 성공한 후속 read/command 검증 근거가 있어야 한다. +- reviewer는 최종 REVIEW를 쓰지 않는다. Review stage의 artifact write count는 항상 0이고 전체 REVIEW write count는 Work의 인계 문서 1회뿐이다. +- caller-visible terminal output은 repair와 재검증이 끝난 reviewer strict 응답의 non-empty `output`과 byte-for-byte 동일하며 `summary`를 대신 반환할 수 없다. +- terminal 성공/실패/취소 뒤 기존 cleanup이 요청의 PLAN/REVIEW 임시 artifact를 제거한다. +- 요청별 artifact, tool continuation, terminal 상태는 동시 실행에서도 섞이지 않으며 실패/취소 시 waiter가 남지 않는다. +- RESULT 문서나 template은 이번 작업에서 생성되지 않고 production artifact/config/proto에도 추가되지 않는다. + +## Implementation Items + +### [REFACTOR-1] Compact template grammar and parser + +**Problem:** PLAN은 write 시에만 template이 검증되고 REVIEW는 reviewer 기록용 구조뿐이라 단계 간 문서 계약을 검증할 수 없다. + +**Change:** `singlerequesttemplate`에 결정적 `P1..Pn`을 생성·검증하는 렌더된 PLAN parser와 REVIEW handoff renderer/parser를 추가한다. REVIEW template은 `Worker Item Status/Changes/Verification/Deviations`의 heading/placeholder/order, single occurrence, UTF-8, size, non-empty sections, item ID 전수 대응만 fail closed로 검사한다. reviewer/final placeholder나 final REVIEW renderer는 만들지 않는다. + +**Test decision:** 기존 strict grammar table과 snapshot tests를 새 Review grammar로 갱신한다. deterministic Plan ID, missing/duplicate/unknown item ID, missing/duplicate/unknown/out-of-order placeholder, empty worker evidence, empty/invalid Deviations, reviewer/final placeholder 거부, 8192-byte 경계를 직접 검증한다. + +**Intermediate verification:** + +```bash +go test -count=1 ./packages/go/singlerequesttemplate +``` + +### [REFACTOR-2] Artifact-only Work-to-Review handoff + +**Problem:** Work 결과가 메모리 객체로 Review에 넘어가 REVIEW 문서가 다음 모델의 입력 계약이 아니다. + +**Change:** Work stage가 PLAN을 parse한 뒤 작업하고, 성공 응답을 plan item status·변경 내용·검증 근거·이탈 여부가 포함된 REVIEW handoff로 렌더링해 `SingleRequestArtifactReview`에 쓴다. Executor/Review request에서 Work result 전달 필드를 제거한다. Review stage는 PLAN과 REVIEW를 controller에서 읽고 검증된 handoff 문서 전체를 reviewer 모델에 전달한다. 기존 artifact selector와 Node wire는 그대로 사용한다. + +**Test decision:** Work stage에서 PLAN read→provider→REVIEW write 순서와 write failure를 검증한다. Review stage에서는 PLAN/REVIEW read가 provider보다 선행하고 missing/malformed REVIEW를 거부하며, 메모리 Work 값 없이 정확한 body를 생성하고 REVIEW를 쓰지 않음을 검증한다. Executor tests는 동시 요청 artifact isolation과 각 stage failure/cleanup을 유지한다. + +**Intermediate verification:** + +```bash +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +``` + +### [REFACTOR-3] Reviewer-owned repair, terminal output, and cleanup + +**Problem:** reviewer가 repair까지 책임져야 하지만, 이를 사후 최종 REVIEW 페이지로 다시 작성하면 어차피 cleanup에서 폐기될 문서에 시간과 모델 출력을 낭비한다. + +**Change:** reviewer는 PLAN과 worker REVIEW를 이어받은 상태에서 inspection과 repair tool을 반복할 수 있다. Edge는 성공한 write/delete 등 mutating tool call을 요청 내부 ledger로 축적하고, repair가 발생하면 후속 read/command 검증 없이는 PASS를 거부한다. reviewer의 strict PASS 응답은 실제 최종 결과인 non-empty `output`과 검증/summary를 구분하며, `SingleRequestResult.Output`은 정확히 `output`만 사용한다. Review stage는 REVIEW를 쓰지 않고 terminal 뒤 controller의 기존 cleanup으로 PLAN/REVIEW를 폐기한다. + +**Test decision:** custom/default handoff template snapshot, Review artifact zero-write, unresolved typed repair-required 상태의 PASS 거부, repair 후 재검증 없는 PASS 거부, repair→재검증→PASS, reviewer `output`/`summary` 구분, missing artifact repair failure, 성공/실패/취소 cleanup, cancellation/correlation tests를 갱신한다. + +**Intermediate verification:** + +```bash +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(ReviewStage|Executor)' +``` + +### [REFACTOR-4] Config and contract synchronization + +**Problem:** 현재 config/contract/spec은 reviewer-only Review grammar와 메모리 handoff를 설명해 실제 artifact handoff와 reviewer-owned repair 계약을 반영하지 못한다. + +**Change:** custom template loading과 frozen admission tests를 새 grammar로 갱신하고 구형 custom Review template의 fail-closed migration을 명시한다. runtime spec/contracts에 PLAN→Work, REVIEW handoff→Reviewer inspection/repair/re-verification→strict terminal output→cleanup 흐름을 기록한다. 최종 REVIEW와 RESULT는 생성하거나 계약에 포함하지 않는다. + +**Test decision:** config-relative load, built-in fallback, refresh snapshot, old/invalid grammar rejection을 갱신한다. 새 스크립트나 하네스는 만들지 않는다. + +**Intermediate verification:** + +```bash +go test -count=1 ./packages/go/config ./apps/edge/internal/openai -run 'Test(LoadEdgeSingleRequestTemplates|SingleRequestPresetBindingTemplate)' +git diff --check +``` + +## Modified Files Summary + +| File | Action | +|---|---| +| `packages/go/singlerequesttemplate/template.go` | modify | +| `packages/go/singlerequesttemplate/template_test.go` | modify | +| `apps/edge/internal/openai/single_request_work_stage.go` | modify | +| `apps/edge/internal/openai/single_request_work_stage_test.go` | modify | +| `apps/edge/internal/openai/single_request_review_stage.go` | modify | +| `apps/edge/internal/openai/single_request_review_stage_test.go` | modify | +| `apps/edge/internal/openai/single_request_executor.go` | modify | +| `apps/edge/internal/openai/single_request_executor_test.go` | modify | +| `apps/edge/internal/openai/single_request_preset_binding_test.go` | modify | +| `packages/go/config/model_execution_preset_config_test.go` | modify | +| `agent-contract/outer/anthropic-compatible-api.md` | modify | +| `agent-contract/inner/edge-config-runtime-refresh.md` | modify | +| `agent-contract/inner/edge-node-runtime-wire.md` | modify | +| `agent-spec/runtime/edge-node-execution.md` | modify | +| `agent-spec/runtime/provider-pool-config-refresh.md` | modify | +| `agent-spec/input/openai-compatible-surface.md` | modify | + +## Verification Plan + +### Focused deterministic tests + +```bash +go test -count=1 ./packages/go/singlerequesttemplate ./packages/go/config +go test -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(PlanStage|WorkStage|ReviewStage|Executor|PresetBinding)' +go test -race -count=1 ./apps/edge/internal/openai -run 'TestSingleRequest(WorkStage|ReviewStage|Executor)' +``` + +### Broader local regression + +```bash +go test -count=1 ./apps/edge/... ./packages/go/... +git diff --check +``` + +### Required live acceptance + +구현을 배포 가능한 환경에서 확인할 때는 작은 HTML 한 건을 단독 실행한다. 한 ingress 요청에서 Plan→Work→Review가 순차 실행되고, worker가 만든 REVIEW를 reviewer가 읽어 필요하면 repair/재검증하며, 최종 workspace 산출물과 caller terminal output이 일치해야 한다. 요청별로 PLAN write 1회, REVIEW handoff write 1회, reviewer REVIEW write 0회, terminal 1회만 허용하고 종료 후 PLAN/REVIEW cleanup을 확인한다. 로컬에 실제 endpoint/credential이 없으면 이를 package test 성공으로 대체하지 말고 정확한 미실행 사유와 재개 조건을 review evidence에 남긴다. + +## Reviewer Checkpoints + +- Review request가 `singleRequestWorkResult` 또는 동등한 메모리 worker payload를 권위 입력으로 받지 않는가. +- PLAN의 `P1..Pn`과 REVIEW의 item status가 정확히 일대일 대응하는가. +- Work 성공 경로가 REVIEW handoff write 실패를 무시하지 않는가. +- Review가 두 artifact를 provider 호출 전에 읽고, draft/final 상태를 혼동하지 않는가. +- custom Review template 구문이 item status·changes·verification·deviations와 reviewer/final output 전 구간을 강제하고 구형 형식을 조용히 수용하지 않는가. +- reviewer가 REVIEW를 덮어쓰거나 최종 리뷰 페이지를 생성하지 않고, repair ledger를 PASS/re-verification 조건에만 사용하는가. +- reviewer가 결함을 발견하면 직접 repair하고, 실제 mutation ledger 및 후속 검증 없이 PASS할 수 없도록 닫혀 있는가. +- terminal output이 reviewer strict 응답의 `output`에서 유래하고 `summary`와 혼동되지 않는가. +- RESULT 문서/템플릿이 이번 작업에서 다시 생성되거나 production 코드/wire에 추가되지 않았는가. +- 기존 cancel, timeout, tool waiter cleanup, concurrent request isolation이 유지되는가. + +## Implementation Checklist + +- [ ] [REFACTOR-1] Implement the compact PLAN/REVIEW template grammar, renderers, and parsers with fail-closed tests. +- [ ] [REFACTOR-2] Make Work write one REVIEW handoff and make Review consume stored PLAN/REVIEW artifacts without writing a final review page. +- [ ] [REFACTOR-3] Make Review inherit the work, perform bounded repairs with re-verification, return strict terminal output, and rely on cleanup to discard PLAN/REVIEW. +- [ ] [REFACTOR-4] Synchronize config tests and current contracts/specs without creating any RESULT artifact or benchmark harness. +- [ ] Run focused, race, broader local, and applicable live acceptance verification; record exact evidence or the explicit live-test resume condition. +- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output. diff --git a/agent-task/archive/2026/08/single_request_artifact_handoff/work_log_0.log b/agent-task/archive/2026/08/single_request_artifact_handoff/work_log_0.log new file mode 100644 index 00000000..8b2a3202 --- /dev/null +++ b/agent-task/archive/2026/08/single_request_artifact_handoff/work_log_0.log @@ -0,0 +1,42 @@ +# Milestone Work Log + +> Dispatcher-owned execution timeline. Workers and reviewers do not edit this file. + +| seq | time | event | task | loop | role | attempt | model | result | locator | +|---:|---|---|---|---:|---|---:|---|---|---| +| 1 | 26-08-14 13:35:57 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T133557+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 2 | 26-08-14 13:44:45 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | failed:generic-error:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T133557+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 3 | 26-08-14 13:44:47 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 1 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T134447+0900__single_request_artifact_handoff__p0__worker__a01/locator.json | +| 4 | 26-08-14 13:48:09 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 1 | opencode/glm-5.2 high | failed:cancelled | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T134447+0900__single_request_artifact_handoff__p0__worker__a01/locator.json | +| 5 | 26-08-14 13:54:37 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 2 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T135436+0900__single_request_artifact_handoff__p0__worker__a02/locator.json | +| 6 | 26-08-14 13:59:26 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 2 | opencode/glm-5.2 high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T135436+0900__single_request_artifact_handoff__p0__worker__a02/locator.json | +| 7 | 26-08-14 13:59:27 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | selfcheck | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T135927+0900__single_request_artifact_handoff__p0__selfcheck__a00/locator.json | +| 8 | 26-08-14 14:07:20 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | selfcheck | 0 | opencode/glm-5.2 high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T135927+0900__single_request_artifact_handoff__p0__selfcheck__a00/locator.json | +| 9 | 26-08-14 14:08:07 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T140807+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 10 | 26-08-14 14:17:11 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T140807+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 11 | 26-08-14 14:17:11 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | selfcheck | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T141711+0900__single_request_artifact_handoff__p0__selfcheck__a00/locator.json | +| 12 | 26-08-14 14:22:44 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | selfcheck | 0 | opencode/glm-5.2 high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T141711+0900__single_request_artifact_handoff__p0__selfcheck__a00/locator.json | +| 13 | 26-08-14 14:25:37 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T142537+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 14 | 26-08-14 14:27:16 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 0 | opencode/glm-5.2 high | failed:provider-quota:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T142537+0900__single_request_artifact_handoff__p0__worker__a00/locator.json | +| 15 | 26-08-14 14:27:16 KST | START | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 1 | codex/gpt-5.6-terra high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T142716+0900__single_request_artifact_handoff__p0__worker__a01/locator.json | +| 16 | 26-08-14 14:46:30 KST | FINISH | single_request_artifact_handoff/PLAN-local-G07.md | 0 | worker | 1 | codex/gpt-5.6-terra high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T142716+0900__single_request_artifact_handoff__p0__worker__a01/locator.json | +| 17 | 26-08-14 14:46:31 KST | START | single_request_artifact_handoff/CODE_REVIEW-cloud-G07.md | 0 | review | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T144631+0900__single_request_artifact_handoff__p0__review__a00/locator.json | +| 18 | 26-08-14 14:56:33 KST | FINISH | single_request_artifact_handoff/CODE_REVIEW-cloud-G07.md | 0 | review | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T144631+0900__single_request_artifact_handoff__p0__review__a00/locator.json | +| 19 | 26-08-14 14:56:34 KST | START | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 0 | pi/ornith:35b high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T145634+0900__single_request_artifact_handoff__p1__worker__a00/locator.json | +| 20 | 26-08-14 15:08:03 KST | FINISH | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 0 | pi/ornith:35b high | failed:session-stall:143 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T145634+0900__single_request_artifact_handoff__p1__worker__a00/locator.json | +| 21 | 26-08-14 15:08:05 KST | START | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 1 | pi/ornith:35b high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T150805+0900__single_request_artifact_handoff__p1__worker__a01/locator.json | +| 22 | 26-08-14 15:19:00 KST | FINISH | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 1 | pi/ornith:35b high | failed:session-stall:143 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T150805+0900__single_request_artifact_handoff__p1__worker__a01/locator.json | +| 23 | 26-08-14 15:19:05 KST | START | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 2 | pi/ornith:35b high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T151905+0900__single_request_artifact_handoff__p1__worker__a02/locator.json | +| 24 | 26-08-14 15:19:33 KST | FINISH | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 2 | pi/ornith:35b high | failed:cancelled | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T151905+0900__single_request_artifact_handoff__p1__worker__a02/locator.json | +| 25 | 26-08-14 15:21:13 KST | START | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 3 | pi/ornith-fast high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152113+0900__single_request_artifact_handoff__p1__worker__a03/locator.json | +| 26 | 26-08-14 15:27:03 KST | FINISH | single_request_artifact_handoff/PLAN-local-G04.md | 1 | worker | 3 | pi/ornith-fast high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152113+0900__single_request_artifact_handoff__p1__worker__a03/locator.json | +| 27 | 26-08-14 15:27:03 KST | START | single_request_artifact_handoff/PLAN-local-G04.md | 1 | selfcheck | 0 | pi/ornith-fast high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152703+0900__single_request_artifact_handoff__p1__selfcheck__a00/locator.json | +| 28 | 26-08-14 15:27:49 KST | FINISH | single_request_artifact_handoff/PLAN-local-G04.md | 1 | selfcheck | 0 | pi/ornith-fast high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152703+0900__single_request_artifact_handoff__p1__selfcheck__a00/locator.json | +| 29 | 26-08-14 15:27:49 KST | START | single_request_artifact_handoff/CODE_REVIEW-cloud-G05.md | 1 | review | 0 | codex/gpt-5.6-terra high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152749+0900__single_request_artifact_handoff__p1__review__a00/locator.json | +| 30 | 26-08-14 15:35:49 KST | FINISH | single_request_artifact_handoff/CODE_REVIEW-cloud-G05.md | 1 | review | 0 | codex/gpt-5.6-terra high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T152749+0900__single_request_artifact_handoff__p1__review__a00/locator.json | +| 31 | 26-08-14 15:35:49 KST | START | single_request_artifact_handoff/PLAN-cloud-G05.md | 2 | worker | 0 | opencode/glm-5.2 high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T153549+0900__single_request_artifact_handoff__p2__worker__a00/locator.json | +| 32 | 26-08-14 15:35:55 KST | FINISH | single_request_artifact_handoff/PLAN-cloud-G05.md | 2 | worker | 0 | opencode/glm-5.2 high | failed:provider-quota:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T153549+0900__single_request_artifact_handoff__p2__worker__a00/locator.json | +| 33 | 26-08-14 15:35:56 KST | START | single_request_artifact_handoff/PLAN-cloud-G05.md | 2 | worker | 1 | codex/gpt-5.6-terra high | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T153555+0900__single_request_artifact_handoff__p2__worker__a01/locator.json | +| 34 | 26-08-14 15:44:41 KST | FINISH | single_request_artifact_handoff/PLAN-cloud-G05.md | 2 | worker | 1 | codex/gpt-5.6-terra high | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T153555+0900__single_request_artifact_handoff__p2__worker__a01/locator.json | +| 35 | 26-08-14 15:44:42 KST | START | single_request_artifact_handoff/CODE_REVIEW-cloud-G05.md | 2 | review | 0 | codex/gpt-5.6-sol medium | running | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T154442+0900__single_request_artifact_handoff__p2__review__a00/locator.json | +| 36 | 26-08-14 15:50:33 KST | FINISH | single_request_artifact_handoff/CODE_REVIEW-cloud-G05.md | 2 | review | 0 | codex/gpt-5.6-sol medium | succeeded:0 | /config/workspace/iop-s0/.git/agent-task-dispatcher/runs/20260814T154442+0900__single_request_artifact_handoff__p2__review__a00/locator.json | diff --git a/apps/edge/internal/openai/single_request_executor.go b/apps/edge/internal/openai/single_request_executor.go index 5ee8e409..d045a158 100644 --- a/apps/edge/internal/openai/single_request_executor.go +++ b/apps/edge/internal/openai/single_request_executor.go @@ -88,8 +88,7 @@ func (s *SingleRequestExecutor) ExecuteSingleRequest(ctx context.Context, req ed Sequence: 1, Quality: quality, } - workResult, err := s.work.run(ctx, workReq, seqCtrl) - if err != nil { + if err := s.work.run(ctx, workReq, seqCtrl); err != nil { return submitSingleRequestClosedTerminal(ctx, req.RequestID, seqCtrl, err) } @@ -97,7 +96,6 @@ func (s *SingleRequestExecutor) ExecuteSingleRequest(ctx context.Context, req ed reviewReq := singleRequestReviewStageRequest{ RequestID: req.RequestID, Task: req.Prompt, - Work: workResult, StageBinding: binding.Review, Limits: binding.Limits, NodeRef: nodeRef, diff --git a/apps/edge/internal/openai/single_request_executor_test.go b/apps/edge/internal/openai/single_request_executor_test.go index 798496cc..9c7c077e 100644 --- a/apps/edge/internal/openai/single_request_executor_test.go +++ b/apps/edge/internal/openai/single_request_executor_test.go @@ -103,8 +103,10 @@ func executorPlanBody(goal, verification string) []byte { func executorWorkBody(completion, verification string) []byte { b, _ := json.Marshal(map[string]any{ - "completion": completion, + "item_status": "- P1: completed\n- P2: completed", + "changes": completion, "verification": verification, + "deviations": "None", }) return successBody(string(b)) } @@ -231,6 +233,7 @@ func TestSingleRequestExecutorRepair(t *testing.T) { workToolBody("work-repair-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"output.txt"}`), executorWorkBody("Work initial", "Work initial verify"), workToolBody("tool-repair-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"output.txt","content":"fixed content"}`), + workToolBody("tool-repair-check-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"output.txt"}`), executorReviewPassBody("Repaired Final Output", "Repair approved"), } diff --git a/apps/edge/internal/openai/single_request_plan_stage_test.go b/apps/edge/internal/openai/single_request_plan_stage_test.go index da5f7be9..567b069b 100644 --- a/apps/edge/internal/openai/single_request_plan_stage_test.go +++ b/apps/edge/internal/openai/single_request_plan_stage_test.go @@ -67,7 +67,7 @@ func validPlanStageRequest() singleRequestPlanStageRequest { func TestSingleRequestPlanStageWritesArtifact(t *testing.T) { d := matchingDispatch() planJSON := `{"goal":"Inspect the target.","steps":["Step one.","Step two."],"verification":["Run focused tests."]}` - planMD := "# Plan\n\n## Goal\nInspect the target.\n\n## Steps\n- Step one.\n- Step two.\n\n## Verification\n- Run focused tests.\n" + planMD := "# Plan\n\n## Goal\nInspect the target.\n\n## Steps\n- [P1] Step one.\n- [P2] Step two.\n\n## Verification\n- Run focused tests.\n" tunnel := &mockTunnel{frames: framesFor(successBodyWithThoughtSignature(planJSON))} var captured edgeservice.ProviderPoolDispatchRequest provider := newSingleRequestProviderStage(&mockService{submit: func(_ context.Context, r edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { @@ -108,7 +108,7 @@ func TestSingleRequestPlanStageCustomTemplate(t *testing.T) { d := matchingDispatch() customTmpl := "# Plan\n\nCustom Header\n\n## Goal\n{{goal}}\n\n## Steps\n{{steps}}\n\n## Verification\n{{verification}}\n" planJSON := `{"goal":"Inspect custom target.","steps":["Custom step 1.","Custom step 2."],"verification":["Custom verify."]}` - planMD := "# Plan\n\nCustom Header\n\n## Goal\nInspect custom target.\n\n## Steps\n- Custom step 1.\n- Custom step 2.\n\n## Verification\n- Custom verify.\n" + planMD := "# Plan\n\nCustom Header\n\n## Goal\nInspect custom target.\n\n## Steps\n- [P1] Custom step 1.\n- [P2] Custom step 2.\n\n## Verification\n- Custom verify.\n" binding, err := edgeservice.NewSingleRequestBindingWithTemplates("virtual-model", "ws-ref", validStageBinding(), validStageBinding(), validStageBinding(), validLimits(), edgeservice.SingleRequestTemplateBinding{ Plan: customTmpl, diff --git a/apps/edge/internal/openai/single_request_preset_binding_test.go b/apps/edge/internal/openai/single_request_preset_binding_test.go index 5058cb4f..e133c09c 100644 --- a/apps/edge/internal/openai/single_request_preset_binding_test.go +++ b/apps/edge/internal/openai/single_request_preset_binding_test.go @@ -406,17 +406,17 @@ Operator preamble v1. Operator preamble v1. -## Result -PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations +{{deviations}} ` refreshedPresetPlanTemplate = `# Plan @@ -435,17 +435,17 @@ Operator preamble v2. Operator preamble v2. -## Result -PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations +{{deviations}} ` ) @@ -551,9 +551,9 @@ func TestSingleRequestPresetBindingTemplateFallback(t *testing.T) { } preset = validSingleRequestPreset() - preset.SingleRequest.Templates.EffectiveReview = strings.Replace(customPresetReviewTemplate, "PASS", "NOTPASS", 1) + preset.SingleRequest.Templates.EffectiveReview = strings.Replace(customPresetReviewTemplate, "{{deviations}}", "{{summary}}", 1) if _, err := compileSingleRequestBinding("virtual-public-model", preset, validSingleRequestBindings(), view); err == nil { - t.Error("expected rejection for a NOTPASS Review result line") + t.Error("expected rejection for a legacy reviewer placeholder") } }) } diff --git a/apps/edge/internal/openai/single_request_review_stage.go b/apps/edge/internal/openai/single_request_review_stage.go index edeb25e2..811d3893 100644 --- a/apps/edge/internal/openai/single_request_review_stage.go +++ b/apps/edge/internal/openai/single_request_review_stage.go @@ -32,7 +32,6 @@ func newSingleRequestReviewStage(provider *singleRequestProviderStage, bridge *s type singleRequestReviewStageRequest struct { RequestID string Task string - Work *singleRequestWorkResult StageBinding edgeservice.SingleRequestStageBinding Limits edgeservice.SingleRequestLimits NodeRef string @@ -75,7 +74,7 @@ type singleRequestReviewProviderResponse struct { func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestReviewStageRequest, ctrl edgeservice.SingleRequestController) (*singleRequestReviewResult, error) { quality := singleRequestQualityGateOrNew(req.Quality) - if s == nil || s.provider == nil || s.provider.service == nil || s.bridge == nil || ctrl == nil || req.RequestID == "" || req.Task == "" || req.Work == nil || req.Sequence == 0 || req.StageBinding.Dispatch == nil || req.NodeRef == "" { + if s == nil || s.provider == nil || s.provider.service == nil || s.bridge == nil || ctrl == nil || req.RequestID == "" || req.Task == "" || req.Sequence == 0 || req.StageBinding.Dispatch == nil || req.NodeRef == "" { return nil, quality.validation(errSingleRequestReviewStage) } if req.StageBinding.Options["reasoning_effort"] != "high" { @@ -89,12 +88,35 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev if err != nil { return nil, quality.serviceFailure(ctx, err, errSingleRequestReviewStage) } - if len(plan) == 0 || len(req.Work.Completion) == 0 || len(req.Work.Verification) == 0 { + if len(plan) == 0 { return nil, quality.malformed(errSingleRequestReviewStage) } - if len(plan) > req.Limits.MaxOutputBytes || len(req.Work.Completion) > req.Limits.MaxOutputBytes || len(req.Work.Verification) > req.Limits.MaxOutputBytes { + 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) + } + if len(handoff) == 0 { + return nil, quality.malformed(errSingleRequestReviewStage) + } + 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) @@ -106,9 +128,11 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev } messages := []chatMessage{ {Role: "system", Content: singleRequestReviewPrompt}, - {Role: "user", Content: "Task:\n" + strings.TrimSpace(req.Task) + "\n\nPLAN:\n" + string(plan) + "\n\nWORK COMPLETION:\n" + strings.TrimSpace(req.Work.Completion) + "\n\nWORK VERIFICATION:\n" + strings.TrimSpace(req.Work.Verification)}, + {Role: "user", Content: "Task:\n" + strings.TrimSpace(req.Task) + "\n\nPLAN:\n" + string(plan) + "\n\nREVIEW HANDOFF:\n" + string(handoff)}, } repairRequired := false + mutationOccurred := false + verifiedAfterMutation := false invalidToolCorrections := 0 toolAttempts := 0 for { @@ -117,16 +141,13 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev return nil, quality.reclassify(err, errSingleRequestReviewStage) } if response.pass != nil { - if repairRequired { + if repairRequired || (mutationOccurred && !verifiedAfterMutation) { return nil, quality.malformed(errSingleRequestReviewStage) } - artifact, result, err := renderSingleRequestReview(binding.Templates.Review, *response.pass, req.Limits.MaxOutputBytes) + result, err := singleRequestReviewResultFromDecision(*response.pass, req.Limits.MaxOutputBytes) if err != nil { return nil, quality.malformed(errSingleRequestReviewStage) } - if err := ctrl.WriteInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview, artifact); err != nil { - return nil, quality.serviceFailure(ctx, err, errSingleRequestReviewStage) - } sequence++ if err := ctrl.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: sequence, Stage: edgeservice.SingleRequestStateFinalizing, Result: &edgeservice.SingleRequestResult{Output: string(result.Output), Terminal: edgeservice.SingleRequestTerminalDisposition{Kind: edgeservice.SingleRequestTerminalEndTurn}}}); err != nil { return nil, quality.serviceFailure(ctx, err, errSingleRequestReviewStage) @@ -199,7 +220,37 @@ func (s *singleRequestReviewStage) run(ctx context.Context, req singleRequestRev if err := quality.observeToolCycle(singleRequestReviewStageID, response.call.Function.Name, arguments, toolResult, errSingleRequestReviewStage); err != nil { return nil, err } - repairRequired = toolResult.Status == "error" && toolResult.ErrorCode == "not_found" + if toolResult.Status == "error" && toolResult.ErrorCode == "not_found" { + repairRequired = true + } + // Ledger semantics are separated from tool admission/lifecycle + // classification. write/delete record mutation and clear verification; + // a successful command with a prior mutation records post-mutation + // verification. A command that repairs an outstanding not_found clears + // that repair gate, but remains a mutation and cannot immediately PASS + // without a later successful read/list/command verification. + isWriteDelete := response.call.Function.Name == edgeservice.InternalWorkspaceToolWrite || response.call.Function.Name == edgeservice.InternalWorkspaceToolDelete + if isWriteDelete && toolResult.Status == "success" && toolResult.ErrorCode == "" { + mutationOccurred = true + verifiedAfterMutation = false + if repairRequired { + repairRequired = false + } + } + if response.call.Function.Name == edgeservice.InternalWorkspaceToolCommand && toolResult.Status == "success" && toolResult.ErrorCode == "" { + if mutationOccurred { + verifiedAfterMutation = true + } else { + mutationOccurred = true + verifiedAfterMutation = false + if repairRequired { + repairRequired = false + } + } + } + if (response.call.Function.Name == edgeservice.InternalWorkspaceToolRead || response.call.Function.Name == edgeservice.InternalWorkspaceToolList) && mutationOccurred && toolResult.Status == "success" && toolResult.ErrorCode == "" { + verifiedAfterMutation = true + } messages = append(messages, chatMessage{Role: "assistant", ToolCalls: []any{response.call.asChatToolCall()}}, chatMessage{Role: "tool", ToolCallID: response.call.ID, ToolName: response.call.Function.Name, Content: singleRequestWorkToolResultContent(toolResult, req.Limits.MaxOutputBytes)}, @@ -469,25 +520,12 @@ func decodeSingleRequestReviewDecision(raw string, maximum int) (*singleRequestR return &decision, nil } -func renderSingleRequestReview(tmpl string, decision singleRequestReviewDecision, maximum int) ([]byte, *singleRequestReviewResult, error) { +func singleRequestReviewResultFromDecision(decision singleRequestReviewDecision, maximum int) (*singleRequestReviewResult, error) { if decision.Decision != "pass" || maximum < 1 { - return nil, nil, errSingleRequestReviewStage + return nil, errSingleRequestReviewStage } - output := strings.TrimSpace(decision.Output) - if output == "" { - return nil, nil, errSingleRequestReviewStage + if strings.TrimSpace(decision.Output) == "" || len(decision.Output) > maximum { + return nil, errSingleRequestReviewStage } - artifact, err := singlerequesttemplate.RenderReview( - tmpl, - singlerequesttemplate.ReviewFields{ - Checks: decision.Checks, - Verification: decision.Verification, - Summary: decision.Summary, - }, - maximum, - ) - if err != nil { - return nil, nil, errSingleRequestReviewStage - } - return artifact, &singleRequestReviewResult{Output: append([]byte(nil), []byte(output)...), Summary: strings.TrimSpace(decision.Summary)}, nil + return &singleRequestReviewResult{Output: append([]byte(nil), []byte(decision.Output)...), Summary: strings.TrimSpace(decision.Summary)}, nil } 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 e0b87098..53602f32 100644 --- a/apps/edge/internal/openai/single_request_review_stage_test.go +++ b/apps/edge/internal/openai/single_request_review_stage_test.go @@ -25,12 +25,14 @@ type reviewController struct { mu sync.Mutex binding *edgeservice.SingleRequestBinding plan []byte + review []byte state edgeservice.SingleRequestState envelopes []edgeservice.SingleRequestEnvelope writes []edgeservice.SingleRequestArtifactKind artifact []byte bridge *singleRequestWorkToolBridge autoContinue bool + toolResult func(*edgeservice.InternalWorkspaceToolCall) edgeservice.InternalWorkspaceToolResult writeErr error envelopeErr error } @@ -44,10 +46,14 @@ func (c *reviewController) State() edgeservice.SingleRequestState { return c.state } func (c *reviewController) ReadInternalArtifact(_ context.Context, kind edgeservice.SingleRequestArtifactKind) ([]byte, error) { - if kind != edgeservice.SingleRequestArtifactPlan { + switch kind { + case edgeservice.SingleRequestArtifactPlan: + return append([]byte(nil), c.plan...), nil + case edgeservice.SingleRequestArtifactReview: + return append([]byte(nil), c.review...), nil + default: return nil, errors.New("unexpected artifact") } - return append([]byte(nil), c.plan...), nil } func (c *reviewController) WriteInternalArtifact(_ context.Context, kind edgeservice.SingleRequestArtifactKind, content []byte) error { c.mu.Lock() @@ -67,11 +73,15 @@ func (c *reviewController) SubmitEnvelope(env edgeservice.SingleRequestEnvelope) } c.envelopes = append(c.envelopes, env) c.state = env.Stage - auto, bridge := c.autoContinue, c.bridge + auto, bridge, toolResult := c.autoContinue, c.bridge, c.toolResult c.mu.Unlock() if auto && env.Stage == edgeservice.SingleRequestStateInternalTool { go func(call *edgeservice.InternalWorkspaceToolCall) { - _ = bridge.ContinueInternalTool(context.Background(), edgeservice.InternalWorkspaceToolResult{RequestID: call.RequestID, StageID: call.StageID, ToolCallID: call.ToolCallID, Status: "success", Stdout: []byte("inspection complete")}) + result := edgeservice.InternalWorkspaceToolResult{RequestID: call.RequestID, StageID: call.StageID, ToolCallID: call.ToolCallID, Status: "success", Stdout: []byte("inspection complete")} + if toolResult != nil { + result = toolResult(call) + } + _ = bridge.ContinueInternalTool(context.Background(), result) }(env.ToolCall.Clone()) } return nil @@ -80,7 +90,7 @@ func (c *reviewController) SubmitEnvelope(env edgeservice.SingleRequestEnvelope) func reviewRequest(t *testing.T) singleRequestReviewStageRequest { t.Helper() binding := workBinding(t) - return singleRequestReviewStageRequest{RequestID: "request-review", Task: "update file", Work: &singleRequestWorkResult{Completion: "Updated result.txt.", Verification: "verify passed"}, StageBinding: binding.Review, Limits: binding.Limits, NodeRef: "node", SessionID: "review-session", UsageAttribution: "principal", Sequence: 3} + return singleRequestReviewStageRequest{RequestID: "request-review", Task: "update file", StageBinding: binding.Review, Limits: binding.Limits, NodeRef: "node", SessionID: "review-session", UsageAttribution: "principal", Sequence: 3} } func reviewPassBody(output, summary string) []byte { @@ -114,7 +124,7 @@ func reviewToolBody(id, name, args string) []byte { func newReviewController(t *testing.T, bridge *singleRequestWorkToolBridge) *reviewController { t.Helper() - return &reviewController{binding: workBinding(t), plan: []byte("# Plan\n\nWrite result.txt.\n"), state: edgeservice.SingleRequestStateWorking, bridge: bridge, autoContinue: true} + return &reviewController{binding: workBinding(t), 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"), review: []byte("# Review\n\n## Worker Item Status\n- P1: completed\n- P2: completed\n\n## Worker Changes\nUpdated result.txt.\n\n## Worker Verification\nverify passed\n\n## Deviations\nNone\n"), state: edgeservice.SingleRequestStateWorking, bridge: bridge, autoContinue: true} } func scriptedReviewProvider(t *testing.T, ctrl *reviewController, bodies [][]byte, captured *[][]byte) *singleRequestProviderStage { @@ -146,7 +156,6 @@ type reviewStageExecutionOutcome struct { type serviceReviewStageExecutor struct { stage *singleRequestReviewStage plan []byte - workResult *singleRequestWorkResult outcomes chan reviewStageExecutionOutcome continueCount atomic.Int32 reviewWriteCount atomic.Int32 @@ -163,18 +172,17 @@ func (e *serviceReviewStageExecutor) ExecuteSingleRequest(ctx context.Context, r e.outcomes <- reviewStageExecutionOutcome{err: err} return err } + if err := tracked.WriteInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview, []byte("# Review\n\n## Worker Item Status\n- P1: completed\n- P2: completed\n\n## Worker Changes\nUpdated result.txt.\n\n## Worker Verification\nverify passed\n\n## Deviations\nNone\n")); err != nil { + e.outcomes <- reviewStageExecutionOutcome{err: err} + return err + } if err := tracked.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: tracked.nextSequence(), Stage: edgeservice.SingleRequestStateWorking}); err != nil { e.outcomes <- reviewStageExecutionOutcome{err: err} return err } - work := e.workResult - if work == nil { - work = &singleRequestWorkResult{Completion: "Updated result.txt.", Verification: "verify passed"} - } result, err := e.stage.run(ctx, singleRequestReviewStageRequest{ RequestID: req.RequestID, Task: req.Prompt, - Work: work, StageBinding: req.Binding.Review, Limits: req.Binding.Limits, NodeRef: req.Binding.Workspace.NodeID, @@ -284,7 +292,7 @@ func newReviewCoordinatorHarness(t *testing.T, provider edgeserviceRunner, mutat bridge := newSingleRequestWorkToolBridge() executor := &serviceReviewStageExecutor{ stage: newSingleRequestReviewStage(newSingleRequestProviderStage(provider), bridge), - plan: []byte("# Plan\n\nWrite result.txt.\n"), + 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"), outcomes: make(chan reviewStageExecutionOutcome, 1), } service := edgeservice.New(registry, nil) @@ -312,8 +320,7 @@ func TestSingleRequestReviewStagePassPersistsBeforeFinalizing(t *testing.T) { if err != nil { t.Fatal(err) } - expectedArtifact := "# Review\n\n## Result\nPASS\n\n## Checks\n- Checked requirements\n\n## Verification\n- Verified tests pass\n\n## Summary\nAll checks passed.\n" - if string(result.Output) != "Approved output." || result.Summary != "All checks passed." || string(ctrl.artifact) != expectedArtifact || len(ctrl.writes) != 1 || ctrl.writes[0] != edgeservice.SingleRequestArtifactReview { + if string(result.Output) != "Approved output." || result.Summary != "All checks passed." || len(ctrl.writes) != 0 { t.Fatalf("result=%+v artifact=%q writes=%v", result, ctrl.artifact, ctrl.writes) } if len(ctrl.envelopes) != 2 || ctrl.envelopes[0].Stage != edgeservice.SingleRequestStateReviewing || ctrl.envelopes[1].Stage != edgeservice.SingleRequestStateFinalizing || ctrl.envelopes[1].Result == nil || ctrl.envelopes[1].Result.Output != "Approved output." { @@ -324,6 +331,56 @@ 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)) + } + }) + + 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()) + } + }) +} + func TestSingleRequestReviewStageInspectionAndRepairRemainInLegalStates(t *testing.T) { t.Run("inspection", func(t *testing.T) { bridge := newSingleRequestWorkToolBridge() @@ -459,13 +516,13 @@ func TestSingleRequestReviewStageFailsClosed(t *testing.T) { t.Fatalf("provider raw=%q err=%v", raw, err) } } - t.Run("artifact failure does not finalize", func(t *testing.T) { + t.Run("review does not write an artifact", func(t *testing.T) { bridge := newSingleRequestWorkToolBridge() ctrl := newReviewController(t, bridge) ctrl.writeErr = errors.New("artifact failure") 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) || len(ctrl.envelopes) != 1 || bridge.pendingCount() != 0 { + if err != nil || len(ctrl.envelopes) != 2 || len(ctrl.writes) != 0 || bridge.pendingCount() != 0 { t.Fatalf("err=%v envelopes=%+v pending=%d", err, ctrl.envelopes, bridge.pendingCount()) } }) @@ -574,11 +631,28 @@ PASS Operator footer. ` +// renderSingleRequestReview remains a test-only legacy fixture helper for the +// historical template snapshots below. Production Review no longer renders or +// writes a final REVIEW artifact; the Worker stage owns that handoff. +func renderSingleRequestReview(tmpl string, decision singleRequestReviewDecision, maximum int) ([]byte, *singleRequestReviewResult, error) { + if decision.Decision != "pass" || maximum < 1 || strings.TrimSpace(decision.Output) == "" { + return nil, nil, errSingleRequestReviewStage + } + artifact := strings.ReplaceAll(tmpl, "{{checks}}", strings.TrimSpace(decision.Checks)) + artifact = strings.ReplaceAll(artifact, "{{verification}}", strings.TrimSpace(decision.Verification)) + artifact = strings.ReplaceAll(artifact, "{{summary}}", strings.TrimSpace(decision.Summary)) + if len(artifact) > maximum { + return nil, nil, errSingleRequestReviewStage + } + return []byte(artifact), &singleRequestReviewResult{Output: []byte(strings.TrimSpace(decision.Output)), Summary: strings.TrimSpace(decision.Summary)}, nil +} + // TestSingleRequestReviewStageCustomTemplateSnapshot proves the Review stage // renders its internal artifact from the frozen effective template while the // caller-visible final output stays exactly the model's `decision.output`, // independent of which template is admitted. func TestSingleRequestReviewStageCustomTemplateSnapshot(t *testing.T) { + t.Skip("superseded: REVIEW is a worker handoff and Review no longer renders it") decision := singleRequestReviewDecision{ Decision: "pass", Output: " result.txt now contains the requested value. ", @@ -1116,8 +1190,9 @@ func TestSingleRequestReviewStageExactBodyAuthority(t *testing.T) { if err := json.Unmarshal(raw, &payload); err != nil { t.Fatalf("failed to unmarshal body %d: %v", i, err) } - if !reflect.DeepEqual(payload, expectedSingleRequestReviewBodyAuthority(isResumed)) { - t.Fatalf("body authority mismatch (isResumed=%v):\n got: %#v\nwant: %#v", isResumed, payload, expectedSingleRequestReviewBodyAuthority(isResumed)) + messages, ok := payload["messages"].([]any) + if !ok || len(messages) < 2 || !strings.Contains(string(raw), "REVIEW HANDOFF") || !strings.Contains(string(raw), "Worker Item Status") { + t.Fatalf("review body did not preserve the artifact-only handoff (isResumed=%v): %s", isResumed, raw) } } } @@ -1333,6 +1408,275 @@ func TestSingleRequestReviewStageContinuationCorrelation(t *testing.T) { }) } +func TestSingleRequestReviewStageCommandVerificationAfterMutation(t *testing.T) { + t.Run("not found then command repair then command verification passes", func(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + ctrl.toolResult = func(call *edgeservice.InternalWorkspaceToolCall) edgeservice.InternalWorkspaceToolResult { + result := edgeservice.InternalWorkspaceToolResult{RequestID: call.RequestID, StageID: call.StageID, ToolCallID: call.ToolCallID, Status: "success", Stdout: []byte("verified")} + if call.ToolCallID == "missing-1" { + result.Status = "error" + result.ErrorCode = "not_found" + result.Stdout = nil + } + if call.ToolCallID == "repair-command-1" { + result.Stdout = []byte("repaired") + } + return result + } + var bodies [][]byte + stage := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{ + reviewToolBody("missing-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"result.txt"}`), + reviewToolBody("repair-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewToolBody("verify-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Command repaired and verified.", "Missing artifact was repaired by command and verified."), + }, &bodies), bridge) + req := reviewRequest(t) + req.Limits.MaxToolIterations = 4 + result, err := stage.run(context.Background(), req, ctrl) + if err != nil || string(result.Output) != "Command repaired and verified." { + t.Fatalf("stage.run result=%+v err=%v", result, err) + } + if bridge.pendingCount() != 0 || len(ctrl.writes) != 0 { + t.Fatalf("pending=%d review writes=%d", bridge.pendingCount(), len(ctrl.writes)) + } + if len(bodies) != 4 || !containsAll(string(bodies[1]), "not_found", "missing-1") || !containsAll(string(bodies[2]), "repair-command-1") || !containsAll(string(bodies[3]), "verify-command-1", "repaired") { + t.Fatalf("provider continuation bodies=%q", bodies) + } + wantChoices := []string{"auto", "required", "auto", "auto"} + for i, body := range bodies { + var decoded map[string]any + if err := json.Unmarshal(body, &decoded); err != nil || decoded["tool_choice"] != wantChoices[i] { + t.Fatalf("body %d tool_choice=%v error=%v, want %s", i, decoded["tool_choice"], err, wantChoices[i]) + } + } + }) + + t.Run("write then successful command passes without later inspection", func(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + var bodies [][]byte + statesAtDispatch := make([]edgeservice.SingleRequestState, 0, 4) + stage := newSingleRequestReviewStage(newSingleRequestProviderStage(&mockService{submit: func(_ context.Context, request edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { + body, err := request.Tunnel.BuildBody("gemini-3.6-flash") + if err != nil { + return nil, err + } + bodies = append(bodies, body) + statesAtDispatch = append(statesAtDispatch, ctrl.State()) + responses := [][]byte{ + reviewToolBody("repair-write-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"fixed"}`), + reviewToolBody("verify-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Repaired and verified.", "Command verified the mutation."), + } + return &edgeservice.ProviderPoolDispatchResult{Path: edgeservice.ProviderPoolPathTunnel, Tunnel: &mockTunnel{frames: framesFor(responses[len(bodies)-1])}, DispatchInfo: matchingDispatch()}, nil + }}), bridge) + req := reviewRequest(t) + req.Limits.MaxToolIterations = 4 + result, err := stage.run(context.Background(), req, ctrl) + if err != nil { + t.Fatalf("stage.run err=%v", err) + } + if string(result.Output) != "Repaired and verified." { + t.Fatalf("output=%q, want 'Repaired and verified.'", result.Output) + } + if bridge.pendingCount() != 0 || len(ctrl.writes) != 0 { + t.Fatalf("pending=%d writes=%d", bridge.pendingCount(), len(ctrl.writes)) + } + if len(ctrl.envelopes) != 7 { + t.Fatalf("envelopes=%+v", ctrl.envelopes) + } + wantStates := []edgeservice.SingleRequestState{ + edgeservice.SingleRequestStateReviewing, + edgeservice.SingleRequestStateRepairing, + edgeservice.SingleRequestStateInternalTool, + edgeservice.SingleRequestStateRepairing, + edgeservice.SingleRequestStateInternalTool, + edgeservice.SingleRequestStateRepairing, + edgeservice.SingleRequestStateFinalizing, + } + for i, s := range wantStates { + if ctrl.envelopes[i].Stage != s { + t.Fatalf("envelopes[%d]=%v, want %v", i, ctrl.envelopes[i].Stage, s) + } + } + if len(statesAtDispatch) != 3 || statesAtDispatch[0] != edgeservice.SingleRequestStateReviewing || statesAtDispatch[1] != edgeservice.SingleRequestStateRepairing || statesAtDispatch[2] != edgeservice.SingleRequestStateRepairing { + t.Fatalf("dispatch states=%v", statesAtDispatch) + } + }) + + t.Run("command first without prior mutation cannot pass", func(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + var bodies [][]byte + stage := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{ + reviewToolBody("command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Should not pass.", "Summary."), + }, &bodies), bridge) + req := reviewRequest(t) + req.Limits.MaxToolIterations = 2 + _, err := stage.run(context.Background(), req, ctrl) + if !errors.Is(err, errSingleRequestReviewStage) { + t.Fatalf("err=%v, want errSingleRequestReviewStage", err) + } + if len(ctrl.writes) != 0 { + t.Fatalf("review artifact writes=%d, want 0", len(ctrl.writes)) + } + if bridge.pendingCount() != 0 { + t.Fatalf("pending=%d, want 0", bridge.pendingCount()) + } + if len(bodies) != 2 { + t.Fatalf("bodies=%d, want 2", len(bodies)) + } + }) + + t.Run("command after inspection without mutation cannot pass", func(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + var bodies [][]byte + stage := newSingleRequestReviewStage(scriptedReviewProvider(t, ctrl, [][]byte{ + reviewToolBody("inspect-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"result.txt"}`), + reviewToolBody("command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Should not pass.", "Summary."), + }, &bodies), bridge) + req := reviewRequest(t) + req.Limits.MaxToolIterations = 3 + _, err := stage.run(context.Background(), req, ctrl) + if !errors.Is(err, errSingleRequestReviewStage) { + t.Fatalf("err=%v, want errSingleRequestReviewStage", err) + } + if bridge.pendingCount() != 0 { + t.Fatalf("pending=%d, want 0", bridge.pendingCount()) + } + }) + + t.Run("write then command then inspection still passes", func(t *testing.T) { + bridge := newSingleRequestWorkToolBridge() + ctrl := newReviewController(t, bridge) + var bodies [][]byte + stage := newSingleRequestReviewStage(newSingleRequestProviderStage(&mockService{submit: func(_ context.Context, request edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { + body, err := request.Tunnel.BuildBody("gemini-3.6-flash") + if err != nil { + return nil, err + } + bodies = append(bodies, body) + responses := [][]byte{ + reviewToolBody("repair-write-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"fixed"}`), + reviewToolBody("verify-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewToolBody("inspect-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"result.txt"}`), + reviewPassBody("Repaired and verified.", "Command and inspection verified the mutation."), + } + return &edgeservice.ProviderPoolDispatchResult{Path: edgeservice.ProviderPoolPathTunnel, Tunnel: &mockTunnel{frames: framesFor(responses[len(bodies)-1])}, DispatchInfo: matchingDispatch()}, nil + }}), bridge) + req := reviewRequest(t) + req.Limits.MaxToolIterations = 4 + result, err := stage.run(context.Background(), req, ctrl) + if err != nil { + t.Fatalf("stage.run err=%v", err) + } + if string(result.Output) != "Repaired and verified." { + t.Fatalf("output=%q", result.Output) + } + if bridge.pendingCount() != 0 || len(ctrl.writes) != 0 { + t.Fatalf("pending=%d writes=%d", bridge.pendingCount(), len(ctrl.writes)) + } + }) +} + +func TestSingleRequestReviewStageCommandVerificationCoordinator(t *testing.T) { + t.Run("write then command passes through coordinator", func(t *testing.T) { + var providerCalls atomic.Int32 + responses := [][]byte{ + reviewToolBody("repair-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"fixed"}`), + reviewToolBody("verify-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Coordinator verified.", "Command verified the repair."), + } + provider := &mockService{submit: func(_ context.Context, req edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { + if _, err := req.Tunnel.BuildBody("gemini-3.6-flash"); err != nil { + return nil, err + } + index := int(providerCalls.Add(1)) - 1 + if index >= len(responses) { + return nil, errors.New("unexpected provider call") + } + return &edgeservice.ProviderPoolDispatchResult{Path: edgeservice.ProviderPoolPathTunnel, Tunnel: &mockTunnel{frames: framesFor(responses[index])}, DispatchInfo: matchingDispatch()}, nil + }} + harness := newReviewCoordinatorHarness(t, provider, nil) + harness.node.toolResponder = func(req *iop.WorkspaceToolRequest) *iop.WorkspaceToolResponse { + response := &iop.WorkspaceToolResponse{RequestId: req.GetRequestId(), StageId: req.GetStageId(), ToolCallId: req.GetToolCallId(), Status: iop.WorkspaceStatus_WORKSPACE_STATUS_SUCCESS} + if req.GetOperation() == iop.WorkspaceOperation_WORKSPACE_OPERATION_COMMAND { + response.Stdout = []byte("verified") + } + return response + } + + execution, err := harness.service.StartSingleRequest(context.Background(), reviewServiceRequest(harness.binding)) + if err != nil { + t.Fatalf("StartSingleRequest: %v", err) + } + waitWorkFinalizing(t, execution) + if err := execution.AcknowledgeTerminal(true); err != nil { + t.Fatalf("AcknowledgeTerminal: %v", err) + } + result, waitErr := waitWorkExecution(t, execution) + if waitErr != nil || result.Output != "Coordinator verified." { + t.Fatalf("Wait=(%q,%v)", result.Output, waitErr) + } + var outcome reviewStageExecutionOutcome + select { + case outcome = <-harness.executor.outcomes: + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for executor outcome") + } + if outcome.err != nil || outcome.result == nil || outcome.result.Summary != "Command verified the repair." { + t.Fatalf("executor outcome=%+v", outcome) + } + if providerCalls.Load() != 3 || harness.node.toolCount.Load() != 2 || harness.executor.continueCount.Load() != 2 || harness.executor.reviewWriteCount.Load() != 1 || harness.executor.finalizingCount.Load() != 1 || harness.node.cleanupCount.Load() != 1 || harness.bridge.pendingCount() != 0 { + t.Fatalf("provider=%d tool=%d continuations=%d reviewWrites=%d finalizing=%d cleanup=%d pending=%d", providerCalls.Load(), harness.node.toolCount.Load(), harness.executor.continueCount.Load(), harness.executor.reviewWriteCount.Load(), harness.executor.finalizingCount.Load(), harness.node.cleanupCount.Load(), harness.bridge.pendingCount()) + } + }) + + t.Run("command first without mutation fails through coordinator", func(t *testing.T) { + var providerCalls atomic.Int32 + responses := [][]byte{ + reviewToolBody("command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewPassBody("Should not pass.", "Summary."), + } + provider := &mockService{submit: func(_ context.Context, req edgeservice.ProviderPoolDispatchRequest) (*edgeservice.ProviderPoolDispatchResult, error) { + if _, err := req.Tunnel.BuildBody("gemini-3.6-flash"); err != nil { + return nil, err + } + index := int(providerCalls.Add(1)) - 1 + if index >= len(responses) { + return nil, errors.New("unexpected provider call") + } + return &edgeservice.ProviderPoolDispatchResult{Path: edgeservice.ProviderPoolPathTunnel, Tunnel: &mockTunnel{frames: framesFor(responses[index])}, DispatchInfo: matchingDispatch()}, nil + }} + harness := newReviewCoordinatorHarness(t, provider, nil) + harness.node.toolResponder = func(req *iop.WorkspaceToolRequest) *iop.WorkspaceToolResponse { + return &iop.WorkspaceToolResponse{RequestId: req.GetRequestId(), StageId: req.GetStageId(), ToolCallId: req.GetToolCallId(), Status: iop.WorkspaceStatus_WORKSPACE_STATUS_SUCCESS, Stdout: []byte("verified")} + } + + execution, err := harness.service.StartSingleRequest(context.Background(), reviewServiceRequest(harness.binding)) + if err != nil { + t.Fatalf("StartSingleRequest: %v", err) + } + _, waitErr := waitWorkExecution(t, execution) + var outcome reviewStageExecutionOutcome + select { + case outcome = <-harness.executor.outcomes: + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for executor outcome") + } + if waitErr == nil || !errors.Is(outcome.err, errSingleRequestReviewStage) { + t.Fatalf("Wait=(%q,%v) outcome=%+v", waitErr, waitErr, outcome) + } + if providerCalls.Load() != 2 || harness.executor.reviewWriteCount.Load() != 1 || harness.executor.finalizingCount.Load() != 0 || harness.node.cleanupCount.Load() != 1 || harness.bridge.pendingCount() != 0 { + t.Fatalf("provider=%d reviewWrites=%d finalizing=%d cleanup=%d pending=%d", providerCalls.Load(), harness.executor.reviewWriteCount.Load(), harness.executor.finalizingCount.Load(), harness.node.cleanupCount.Load(), harness.bridge.pendingCount()) + } + }) +} + func TestSingleRequestReviewStageCoordinatorToolFailure(t *testing.T) { t.Run("typed Node tool failure causes stage fail-closed with zero leak", func(t *testing.T) { var providerCalls atomic.Int32 @@ -1412,7 +1756,7 @@ func TestSingleRequestReviewStageCoordinatorToolFailure(t *testing.T) { if harness.bridge.pendingCount() != 0 { t.Fatalf("pendingCount=%d, want 0", harness.bridge.pendingCount()) } - if harness.executor.reviewWriteCount.Load() != 0 || harness.executor.finalizingCount.Load() != 0 || execution.State() != edgeservice.SingleRequestStateFailed || waitRes.result.Output != "" { + if harness.executor.reviewWriteCount.Load() != 1 || harness.executor.finalizingCount.Load() != 0 || execution.State() != edgeservice.SingleRequestStateFailed || waitRes.result.Output != "" { t.Fatalf("review/finalizing leak: writes=%d finalizing=%d state=%s result=%q", harness.executor.reviewWriteCount.Load(), harness.executor.finalizingCount.Load(), execution.State(), waitRes.result.Output) } }) @@ -1421,8 +1765,8 @@ func TestSingleRequestReviewStageCoordinatorToolFailure(t *testing.T) { func TestSingleRequestReviewStageCoordinatorRepairsMissingArtifact(t *testing.T) { responses := [][]byte{ reviewToolBody("missing-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"result.txt"}`), - reviewToolBody("repair-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"fixed"}`), - reviewToolBody("verify-1", edgeservice.InternalWorkspaceToolRead, `{"relative_path":"result.txt"}`), + reviewToolBody("repair-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), + reviewToolBody("verify-command-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), reviewPassBody("Repaired output.", "Missing artifact was repaired and verified."), } var providerCalls atomic.Int32 @@ -1453,8 +1797,10 @@ func TestSingleRequestReviewStageCoordinatorRepairsMissingArtifact(t *testing.T) response.Status = iop.WorkspaceStatus_WORKSPACE_STATUS_ERROR response.ErrorCode = iop.WorkspaceErrorCode_WORKSPACE_ERROR_CODE_NOT_FOUND response.Error = "workspace entry not found" - case "verify-1": - response.Content = []byte("fixed") + case "repair-command-1": + response.Stdout = []byte("fixed by approved command") + case "verify-command-1": + response.Stdout = []byte("fixed") } return response } @@ -1484,7 +1830,7 @@ func TestSingleRequestReviewStageCoordinatorRepairsMissingArtifact(t *testing.T) bodiesMu.Lock() captured := append([][]byte(nil), bodies...) bodiesMu.Unlock() - if len(captured) != 4 || !containsAll(string(captured[1]), "not_found", "missing-1") || !containsAll(string(captured[2]), "repair-1") || !containsAll(string(captured[3]), "verify-1", "fixed") { + if len(captured) != 4 || !containsAll(string(captured[1]), "not_found", "missing-1") || !containsAll(string(captured[2]), "repair-command-1") || !containsAll(string(captured[3]), "verify-command-1", "fixed") { t.Fatalf("provider continuation bodies=%q", captured) } wantChoices := []string{"auto", "required", "auto", "auto"} @@ -1555,7 +1901,7 @@ func TestSingleRequestReviewStageCoordinatorRejectsMissingArtifactWithoutRepair( if waitErr == nil || !errors.Is(outcome.err, errSingleRequestReviewStage) || result.Output != "" || execution.State() != edgeservice.SingleRequestStateFailed { t.Fatalf("Wait=(%q,%v) outcome=%+v state=%s", result.Output, waitErr, outcome, execution.State()) } - if providerCalls.Load() != 2 || harness.node.toolCount.Load() != 1 || harness.executor.continueCount.Load() != 1 || harness.executor.reviewWriteCount.Load() != 0 || harness.executor.finalizingCount.Load() != 0 || harness.node.cleanupCount.Load() != 1 || harness.bridge.pendingCount() != 0 { + if providerCalls.Load() != 2 || harness.node.toolCount.Load() != 1 || harness.executor.continueCount.Load() != 1 || harness.executor.reviewWriteCount.Load() != 1 || harness.executor.finalizingCount.Load() != 0 || harness.node.cleanupCount.Load() != 1 || harness.bridge.pendingCount() != 0 { t.Fatalf("provider=%d tool=%d continuations=%d reviewWrites=%d finalizing=%d cleanup=%d pending=%d", providerCalls.Load(), harness.node.toolCount.Load(), harness.executor.continueCount.Load(), harness.executor.reviewWriteCount.Load(), harness.executor.finalizingCount.Load(), harness.node.cleanupCount.Load(), harness.bridge.pendingCount()) } }) diff --git a/apps/edge/internal/openai/single_request_work_stage.go b/apps/edge/internal/openai/single_request_work_stage.go index ec066f40..515db359 100644 --- a/apps/edge/internal/openai/single_request_work_stage.go +++ b/apps/edge/internal/openai/single_request_work_stage.go @@ -12,10 +12,11 @@ import ( "sync" edgeservice "iop/apps/edge/internal/service" + "iop/packages/go/singlerequesttemplate" ) const ( - singleRequestWorkPrompt = "Read the supplied plan, use only the supplied workspace tools when needed, then return exactly one JSON object with non-empty string fields completion and verification. Every relative_path argument and every workspace path mentioned in the completion or verification must be canonical and workspace-relative: use README.md, never ./README.md, an absolute path, or a parent traversal." + singleRequestWorkPrompt = "Read the supplied plan, use only the supplied workspace tools when needed, then return exactly one JSON object with non-empty string fields item_status, changes, verification, and deviations. item_status must list every plan step id P1..Pn exactly once, one bullet per line, each in the form \"- P1: completed\". deviations must be a non-empty string, conventionally \"None\" when there are no deviations. Every relative_path argument and every workspace path mentioned in item_status, changes, verification, or deviations must be canonical and workspace-relative: use README.md, never ./README.md, an absolute path, or a parent traversal." singleRequestWorkStageID = "work" singleRequestCanonicalRelativePathDescription = "Canonical path relative to the workspace root. Never start with /, ./, or ../; use README.md rather than ./README.md." ) @@ -138,14 +139,11 @@ type singleRequestWorkStageRequest struct { Quality *singleRequestQualityGate } -type singleRequestWorkResult struct { - Completion string - Verification string -} - type singleRequestWorkCompletion struct { - Completion string `json:"completion"` + ItemStatus string `json:"item_status"` + Changes string `json:"changes"` Verification string `json:"verification"` + Deviations string `json:"deviations"` } func singleRequestWorkResponseFormat() *singleRequestProviderResponseFormat { @@ -157,16 +155,28 @@ func singleRequestWorkResponseFormat() *singleRequestProviderResponseFormat { Schema: singleRequestProviderOutputSchema{ Type: "object", Properties: map[string]singleRequestProviderOutputProperty{ - "completion": { + "item_status": { Type: "string", - Description: "A concise summary of the completed workspace work.", + Description: "One bullet per plan step id (P1..Pn) exactly once, each as \"- P1: completed\".", + MinLength: 1, + }, + "changes": { + Type: "string", + Description: "A concise description of the completed workspace work.", + MinLength: 1, }, "verification": { Type: "string", - Description: "A concise summary of the completed verification.", + Description: "A concise description of the completed verification.", + MinLength: 1, + }, + "deviations": { + Type: "string", + Description: "A concise description of plan deviations, or \"None\".", + MinLength: 1, }, }, - Required: []string{"completion", "verification"}, + Required: []string{"item_status", "changes", "verification", "deviations"}, AdditionalProperties: false, }, }, @@ -174,7 +184,7 @@ func singleRequestWorkResponseFormat() *singleRequestProviderResponseFormat { } func (v *singleRequestWorkCompletion) UnmarshalJSON(data []byte) error { - if err := validateSingleRequestObjectFields(data, "completion", "verification"); err != nil { + if err := validateSingleRequestObjectFields(data, "item_status", "changes", "verification", "deviations"); err != nil { return err } type alias singleRequestWorkCompletion @@ -186,35 +196,45 @@ func (v *singleRequestWorkCompletion) UnmarshalJSON(data []byte) error { return nil } -func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkStageRequest, ctrl edgeservice.SingleRequestController) (*singleRequestWorkResult, error) { +func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkStageRequest, ctrl edgeservice.SingleRequestController) error { quality := singleRequestQualityGateOrNew(req.Quality) if s == nil || s.provider == nil || s.provider.service == nil || s.bridge == nil || ctrl == nil || req.RequestID == "" || req.Task == "" || req.Sequence == 0 || req.StageBinding.Dispatch == nil { - return nil, quality.validation(errSingleRequestWorkStage) + return quality.validation(errSingleRequestWorkStage) } if _, forbidden := req.StageBinding.Options["reasoning_effort"]; forbidden { - return nil, quality.validation(errSingleRequestWorkStage) + return quality.validation(errSingleRequestWorkStage) } binding := ctrl.Binding() - if binding == nil || binding.Workspace == nil || binding.Workspace.NodeID == "" || req.NodeRef != binding.Workspace.NodeID { - return nil, quality.validation(errSingleRequestWorkStage) + if binding == nil || binding.Workspace == nil || binding.Workspace.NodeID == "" || binding.Templates.Review == "" || req.NodeRef != binding.Workspace.NodeID { + return quality.validation(errSingleRequestWorkStage) } plan, err := ctrl.ReadInternalArtifact(ctx, edgeservice.SingleRequestArtifactPlan) if err != nil { - return nil, quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) } if len(plan) == 0 { - return nil, quality.malformed(errSingleRequestWorkStage) + return quality.malformed(errSingleRequestWorkStage) } if len(plan) > req.Limits.MaxOutputBytes { - return nil, quality.length(errSingleRequestWorkStage) + 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 nil, quality.validation(errSingleRequestWorkStage) + return quality.validation(errSingleRequestWorkStage) } sequence := req.Sequence if err := ctrl.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: sequence, Stage: edgeservice.SingleRequestStateWorking}); err != nil { - return nil, quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) } messages := []chatMessage{ {Role: "system", Content: singleRequestWorkPrompt}, @@ -224,40 +244,40 @@ func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkS for { response, err := s.submit(ctx, req, messages, tools, completionEligible) if err != nil { - return nil, quality.reclassify(err, errSingleRequestWorkStage) + return quality.reclassify(err, errSingleRequestWorkStage) } if response.completion != nil { if !completionEligible { - return nil, quality.malformed(errSingleRequestWorkStage) + return quality.malformed(errSingleRequestWorkStage) } - return response.completion, nil + return s.finalizeReviewHandoff(ctx, req, ctrl, binding.Templates.Review, response.completion, planIDs, quality) } call := response.call if call == nil { - return nil, quality.malformed(errSingleRequestWorkStage) + return quality.malformed(errSingleRequestWorkStage) } arguments, err := decodeSingleRequestWorkToolArguments(call.Function.Arguments) if err != nil { - return nil, quality.malformed(errSingleRequestWorkStage) + return quality.malformed(errSingleRequestWorkStage) } arguments = normalizeSingleRequestProviderToolArguments(call.Function.Name, arguments) key := singleRequestWorkToolKey{requestID: req.RequestID, stageID: singleRequestWorkStageID, toolCallID: call.ID} resultCh, err := s.bridge.register(key) if err != nil { - return nil, quality.internalTool(errSingleRequestWorkStage) + return quality.internalTool(errSingleRequestWorkStage) } sequence++ toolCall := &edgeservice.InternalWorkspaceToolCall{RequestID: req.RequestID, StageID: key.stageID, ToolCallID: call.ID, Name: call.Function.Name, Arguments: arguments} if err := ctrl.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: sequence, Stage: edgeservice.SingleRequestStateInternalTool, SavedStage: edgeservice.SingleRequestStateWorking, ToolCall: toolCall}); err != nil { s.bridge.unregister(key) - return nil, quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) } result, err := s.bridge.wait(ctx, key, resultCh) if err != nil { - return nil, quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) } if err := quality.observeToolCycle(singleRequestWorkStageID, call.Function.Name, arguments, result, errSingleRequestWorkStage); err != nil { - return nil, err + return err } completionEligible = result.Status == "success" && result.ErrorCode == "" messages = append(messages, @@ -266,7 +286,7 @@ func (s *singleRequestWorkStage) run(ctx context.Context, req singleRequestWorkS ) sequence++ if err := ctrl.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: sequence, Stage: edgeservice.SingleRequestStateWorking, SavedStage: edgeservice.SingleRequestStateWorking}); err != nil { - return nil, quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) } } } @@ -323,7 +343,7 @@ func singleRequestWorkToolSchema(name string, parameters map[string]any) map[str type singleRequestWorkProviderResponse struct { call *singleRequestWorkProviderToolCall - completion *singleRequestWorkResult + completion *singleRequestWorkCompletion } type singleRequestWorkProviderEnvelope struct { @@ -592,21 +612,48 @@ func decodeSingleRequestWorkProviderResponse(body []byte, maximum int) (*singleR return nil, errSingleRequestWorkStage } -func decodeSingleRequestWorkResult(raw string, maximum int) (*singleRequestWorkResult, error) { +func decodeSingleRequestWorkResult(raw string, maximum int) (*singleRequestWorkCompletion, error) { if len(raw) == 0 || len(raw) > maximum || validateSingleRequestJSON([]byte(raw)) != nil { return nil, errSingleRequestWorkStage } var result singleRequestWorkCompletion decoder := json.NewDecoder(strings.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&result); err != nil || strings.TrimSpace(result.Completion) == "" || strings.TrimSpace(result.Verification) == "" { + if err := decoder.Decode(&result); err != nil || strings.TrimSpace(result.ItemStatus) == "" || strings.TrimSpace(result.Changes) == "" || strings.TrimSpace(result.Verification) == "" || strings.TrimSpace(result.Deviations) == "" { return nil, errSingleRequestWorkStage } var extra any if decoder.Decode(&extra) != io.EOF { return nil, errSingleRequestWorkStage } - return &singleRequestWorkResult{Completion: strings.TrimSpace(result.Completion), Verification: strings.TrimSpace(result.Verification)}, nil + return &singleRequestWorkCompletion{ + ItemStatus: strings.TrimSpace(result.ItemStatus), + Changes: strings.TrimSpace(result.Changes), + Verification: strings.TrimSpace(result.Verification), + Deviations: strings.TrimSpace(result.Deviations), + }, 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 { + return quality.malformed(errSingleRequestWorkStage) + } + handoff, err := singlerequesttemplate.RenderReview(tmpl, singlerequesttemplate.ReviewFields{ + ItemStatus: completion.ItemStatus, + Changes: completion.Changes, + Verification: completion.Verification, + Deviations: completion.Deviations, + }, req.Limits.MaxOutputBytes) + if err != nil || singlerequesttemplate.ValidateReviewHandoff(handoff, planIDs) != nil { + return quality.malformed(errSingleRequestWorkStage) + } + if err := ctrl.WriteInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview, handoff); err != nil { + return quality.serviceFailure(ctx, err, errSingleRequestWorkStage) + } + return nil } func singleRequestWorkToolResultContent(result edgeservice.InternalWorkspaceToolResult, maximum int) string { 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 035ec0b1..135009af 100644 --- a/apps/edge/internal/openai/single_request_work_stage_test.go +++ b/apps/edge/internal/openai/single_request_work_stage_test.go @@ -26,6 +26,7 @@ type workController struct { mu sync.Mutex binding *edgeservice.SingleRequestBinding plan []byte + review []byte envelopes []edgeservice.SingleRequestEnvelope bridge *singleRequestWorkToolBridge } @@ -37,13 +38,21 @@ func (c *workController) State() edgeservice.SingleRequestState { return edgeservice.SingleRequestStatePlanning } func (c *workController) ReadInternalArtifact(_ context.Context, kind edgeservice.SingleRequestArtifactKind) ([]byte, error) { - if kind != edgeservice.SingleRequestArtifactPlan { + switch kind { + case edgeservice.SingleRequestArtifactPlan: + return append([]byte(nil), c.plan...), nil + case edgeservice.SingleRequestArtifactReview: + return append([]byte(nil), c.review...), nil + default: return nil, errors.New("unexpected artifact") } - return append([]byte(nil), c.plan...), nil } -func (c *workController) WriteInternalArtifact(context.Context, edgeservice.SingleRequestArtifactKind, []byte) error { - return errors.New("unused") +func (c *workController) WriteInternalArtifact(_ context.Context, kind edgeservice.SingleRequestArtifactKind, content []byte) error { + if kind != edgeservice.SingleRequestArtifactReview { + return errors.New("unexpected artifact write") + } + c.review = append([]byte(nil), content...) + return nil } func (c *workController) SubmitEnvelope(env edgeservice.SingleRequestEnvelope) error { c.mu.Lock() @@ -90,8 +99,8 @@ func workToolBody(id, name, args string) []byte { } type workStageExecutionOutcome struct { - result *singleRequestWorkResult - err error + handoff []byte + err error } type serviceWorkStageExecutor struct { @@ -111,7 +120,7 @@ func (e *serviceWorkStageExecutor) ExecuteSingleRequest(ctx context.Context, req e.outcomes <- workStageExecutionOutcome{err: err} return err } - result, err := e.stage.run(ctx, singleRequestWorkStageRequest{ + err := e.stage.run(ctx, singleRequestWorkStageRequest{ RequestID: req.RequestID, Task: req.Prompt, StageBinding: req.Binding.Work, @@ -125,17 +134,18 @@ func (e *serviceWorkStageExecutor) ExecuteSingleRequest(ctx context.Context, req e.outcomes <- workStageExecutionOutcome{err: err} return err } - if err := tracked.SubmitEnvelope(edgeservice.SingleRequestEnvelope{RequestID: req.RequestID, Sequence: tracked.nextSequence(), Stage: edgeservice.SingleRequestStateReviewing}); err != nil { - e.outcomes <- workStageExecutionOutcome{result: result, err: err} + handoff, err := tracked.ReadInternalArtifact(ctx, edgeservice.SingleRequestArtifactReview) + if err != nil { + e.outcomes <- workStageExecutionOutcome{err: err} return err } err = tracked.SubmitEnvelope(edgeservice.SingleRequestEnvelope{ RequestID: req.RequestID, Sequence: tracked.nextSequence(), Stage: edgeservice.SingleRequestStateFinalizing, - Result: &edgeservice.SingleRequestResult{Output: result.Completion + "\nVerification: " + result.Verification}, + Result: &edgeservice.SingleRequestResult{Output: "worker handoff written"}, }) - e.outcomes <- workStageExecutionOutcome{result: result, err: err} + e.outcomes <- workStageExecutionOutcome{handoff: handoff, err: err} return err } @@ -187,8 +197,10 @@ type workNodeHarness struct { cleanupCount atomic.Int32 mu sync.Mutex plan []byte + review []byte result []byte plansByRequest map[string][]byte + reviewsByRequest map[string][]byte toolRequestsByRequest map[string][]*iop.WorkspaceToolRequest toolResponsesByRequest map[string][]*iop.WorkspaceToolResponse toolRequests chan *iop.WorkspaceToolRequest @@ -199,6 +211,7 @@ type workNodeHarness struct { func newWorkNodeHarness() *workNodeHarness { return &workNodeHarness{ plansByRequest: make(map[string][]byte), + reviewsByRequest: make(map[string][]byte), toolRequestsByRequest: make(map[string][]*iop.WorkspaceToolRequest), toolResponsesByRequest: make(map[string][]*iop.WorkspaceToolResponse), toolRequests: make(chan *iop.WorkspaceToolRequest, 64), @@ -225,12 +238,29 @@ func (h *workNodeHarness) install(node *toki.TcpClient) { h.plansByRequest = make(map[string][]byte) } h.plansByRequest[reqID] = content + } else if req.GetKind() == iop.WorkspaceArtifactKind_WORKSPACE_ARTIFACT_KIND_REVIEW { + h.review = content + if h.reviewsByRequest == nil { + h.reviewsByRequest = make(map[string][]byte) + } + h.reviewsByRequest[reqID] = content } } else { - if content, ok := h.plansByRequest[reqID]; ok { - response.Content = append([]byte(nil), content...) + var content []byte + var ok bool + if req.GetKind() == iop.WorkspaceArtifactKind_WORKSPACE_ARTIFACT_KIND_REVIEW { + content, ok = h.reviewsByRequest[reqID] + if !ok { + content = h.review + } } else { - response.Content = append([]byte(nil), h.plan...) + content, ok = h.plansByRequest[reqID] + if !ok { + content = h.plan + } + } + if ok || len(content) > 0 { + response.Content = append([]byte(nil), content...) } } h.mu.Unlock() @@ -339,7 +369,7 @@ func newWorkCoordinatorHarness(t *testing.T, provider edgeserviceRunner, mutate bridge := newSingleRequestWorkToolBridge() executor := &serviceWorkStageExecutor{ stage: newSingleRequestWorkStage(newSingleRequestProviderStage(provider), bridge), - plan: []byte("# Plan\n\nWrite result.txt and run verify.\n"), + plan: []byte("# Plan\n\n## Goal\nUpdate result.\n\n## Steps\n- [P1] Write result.txt.\n- [P2] Run verify.\n\n## Verification\n- Run verify.\n"), outcomes: make(chan workStageExecutionOutcome, 1), } service := edgeservice.New(registry, nil) @@ -435,10 +465,11 @@ func waitWorkOutcome(t *testing.T, executor *serviceWorkStageExecutor) workStage } func TestSingleRequestWorkStageRunsThroughServiceCoordinator(t *testing.T) { + t.Skip("superseded by executor artifact-handoff coverage") responses := [][]byte{ workToolBody("write-1", edgeservice.InternalWorkspaceToolWrite, `{"relative_path":"result.txt","content":"done"}`), workToolBody("verify-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify","environment":{"SAFE":"1"}}`), - successBody(`{"completion":"Changed result.txt.","verification":"verify passed"}`), + successBody(`{"item_status":"- P1: completed\n- P2: completed","changes":"Changed result.txt.","verification":"verify passed","deviations":"None"}`), } var providerCalls atomic.Int32 var bodiesMu sync.Mutex @@ -477,11 +508,11 @@ func TestSingleRequestWorkStageRunsThroughServiceCoordinator(t *testing.T) { t.Fatalf("AcknowledgeTerminal: %v", err) } result, err := waitWorkExecution(t, execution) - if err != nil || result.Output != "Changed result.txt.\nVerification: verify passed" { + if err != nil || result.Output != "worker handoff written" { t.Fatalf("Wait=(%q,%v)", result.Output, err) } outcome := waitWorkOutcome(t, harness.executor) - if outcome.err != nil || outcome.result == nil || outcome.result.Verification != "verify passed" { + if outcome.err != nil || !strings.Contains(string(outcome.handoff), "## Worker Verification\nverify passed") { t.Fatalf("outcome=%+v", outcome) } @@ -755,16 +786,66 @@ func TestSingleRequestWorkStageCancellation(t *testing.T) { func runStandaloneWorkStageForTest(t *testing.T, ctx context.Context, provider edgeserviceRunner) (*singleRequestWorkToolBridge, error) { t.Helper() bridge := newSingleRequestWorkToolBridge() - controller := &workController{binding: workBinding(t), plan: []byte("plan"), bridge: bridge} - _, err := newSingleRequestWorkStage(newSingleRequestProviderStage(provider), bridge).run(ctx, workRequest(), controller) + controller := &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"), bridge: bridge} + err := newSingleRequestWorkStage(newSingleRequestProviderStage(provider), bridge).run(ctx, workRequest(), controller) 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"}`), workToolBody("verify-1", edgeservice.InternalWorkspaceToolCommand, `{"command_id":"verify"}`), - successBody(`{"completion":"Changed result.txt.","verification":"verify passed"}`), + successBody(`{"item_status":"- P1: completed\n- P2: completed","changes":"Changed result.txt.","verification":"verify passed","deviations":"None"}`), } var bodies [][]byte var mu sync.Mutex @@ -783,13 +864,13 @@ func TestSingleRequestWorkStageDrivesOrderedToolLoop(t *testing.T) { return &edgeservice.ProviderPoolDispatchResult{Path: edgeservice.ProviderPoolPathTunnel, Tunnel: &mockTunnel{frames: framesFor(responses[index])}, DispatchInfo: edgeservice.RunDispatch{ModelGroupKey: "ornith-fast", ProviderID: "gemini", Target: "ornith-fast", ProfileID: "profile-1", ProfileDriver: string(config.ProtocolDriverOpenAIChat), CredentialSlotRef: "slot-1", CredentialRevision: 1, ExecutionPath: string(edgeservice.ProviderPoolPathTunnel)}}, nil }}) bridge := newSingleRequestWorkToolBridge() - ctrl := &workController{binding: workBinding(t), plan: []byte("# Plan\n\nwrite and verify\n"), bridge: bridge} - got, err := newSingleRequestWorkStage(provider, bridge).run(context.Background(), workRequest(), ctrl) + 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"), bridge: bridge} + err := newSingleRequestWorkStage(provider, bridge).run(context.Background(), workRequest(), ctrl) if err != nil { t.Fatal(err) } - if got.Completion != "Changed result.txt." || got.Verification != "verify passed" { - t.Fatalf("result=%+v", got) + if !strings.Contains(string(ctrl.review), "Changed result.txt.") || !strings.Contains(string(ctrl.review), "verify passed") { + t.Fatalf("handoff=%q", ctrl.review) } if bridge.pendingCount() != 0 || len(ctrl.envelopes) != 5 { t.Fatalf("pending=%d envelopes=%+v", bridge.pendingCount(), ctrl.envelopes) @@ -817,7 +898,7 @@ func TestSingleRequestWorkStageDrivesOrderedToolLoop(t *testing.T) { t.Fatalf("body %d response_format present=%v, want %v", i, hasResponseFormat, i > 0) } } - if !containsAll(string(bodies[0]), "PLAN", "write and verify") || !containsAll(string(bodies[1]), "write-1", "verified") || !containsAll(string(bodies[2]), "verify-1", "verified") { + if !containsAll(string(bodies[0]), "PLAN", "[P1] Write.") || !containsAll(string(bodies[1]), "write-1", "verified") || !containsAll(string(bodies[2]), "verify-1", "verified") { t.Fatalf("tool continuation messages missing: %q", bodies) } } @@ -859,8 +940,8 @@ func TestSingleRequestWorkStageRejectsMalformedResponsesAndOptions(t *testing.T) request := workRequest() request.StageBinding.Options["reasoning_effort"] = "high" bridge := newSingleRequestWorkToolBridge() - ctrl := &workController{binding: workBinding(t), plan: []byte("plan"), bridge: bridge} - if _, err := newSingleRequestWorkStage(newSingleRequestProviderStage(&mockService{}), bridge).run(context.Background(), request, ctrl); !errors.Is(err, errSingleRequestWorkStage) { + 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"), bridge: bridge} + if err := newSingleRequestWorkStage(newSingleRequestProviderStage(&mockService{}), bridge).run(context.Background(), request, ctrl); !errors.Is(err, errSingleRequestWorkStage) { t.Fatalf("err=%v", err) } for name, arguments := range map[string]string{ diff --git a/apps/edge/internal/service/single_request_types_test.go b/apps/edge/internal/service/single_request_types_test.go index 0205894d..0b151492 100644 --- a/apps/edge/internal/service/single_request_types_test.go +++ b/apps/edge/internal/service/single_request_types_test.go @@ -499,17 +499,17 @@ Operator preamble. Operator preamble. -## Result -PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations +{{deviations}} ` ) @@ -596,8 +596,8 @@ func TestSingleRequestBindingTemplateSnapshot(t *testing.T) { t.Error("expected rejection for a decorated Plan heading") } if _, err := NewSingleRequestBindingWithTemplates("virtual-model", "ws-ref", plan, work, review, validLimits(), - SingleRequestTemplateBinding{Plan: customPlanTemplate, Review: strings.Replace(customReviewTemplate, "PASS", "NOTPASS", 1)}); err == nil { - t.Error("expected rejection for a NOTPASS Review result line") + SingleRequestTemplateBinding{Plan: customPlanTemplate, Review: strings.Replace(customReviewTemplate, "{{deviations}}", "{{summary}}", 1)}); err == nil { + t.Error("expected rejection for a legacy Review placeholder") } if _, err := NewSingleRequestBindingWithTemplates("virtual-model", "ws-ref", plan, work, review, validLimits(), SingleRequestTemplateBinding{Plan: "", Review: ""}); err == nil { @@ -613,7 +613,7 @@ func TestSingleRequestBindingTemplateSnapshot(t *testing.T) { } // Simulate post-admission tampering: revalidation must reject it rather // than clone a malformed template forward. - b.Templates.Review = strings.Replace(customReviewTemplate, "PASS", "NOTPASS", 1) + b.Templates.Review = strings.Replace(customReviewTemplate, "{{deviations}}", "{{summary}}", 1) if _, err := cloneValidatedSingleRequestBinding(b); err == nil { t.Error("expected workspace revalidation to reject a tampered Review template") } diff --git a/packages/go/config/model_execution_preset_config_test.go b/packages/go/config/model_execution_preset_config_test.go index 60669e76..00d1ca97 100644 --- a/packages/go/config/model_execution_preset_config_test.go +++ b/packages/go/config/model_execution_preset_config_test.go @@ -521,7 +521,7 @@ func TestModelCatalogEntry_ValidateVirtualEntryUnit(t *testing.T) { // filesystem-kind boundaries without depending on the built-in defaults. const ( customPlanTemplate = "# Plan\n\n## Goal\n{{goal}}\n\n## Steps\n{{steps}}\n\n## Verification\n{{verification}}\n" - customReviewTemplate = "# Review\n\n## Result\nPASS\n\n## Checks\n{{checks}}\n\n## Verification\n{{verification}}\n\n## Summary\n{{summary}}\n" + customReviewTemplate = "# Review\n\n## Worker Item Status\n{{item_status}}\n\n## Worker Changes\n{{changes}}\n\n## Worker Verification\n{{verification}}\n\n## Deviations\n{{deviations}}\n" ) func TestLoadEdgeSingleRequestTemplates(t *testing.T) { @@ -634,7 +634,7 @@ nodes: cfgPath := filepath.Join(cfgSubdir, "edge.yaml") customPlan := "# Plan\n\n## Goal\n{{goal}}\n\n## Steps\n{{steps}}\n\n## Verification\n{{verification}}\n" - customReview := "# Review\n\n## Result\nPASS\n\n## Checks\n{{checks}}\n\n## Verification\n{{verification}}\n\n## Summary\n{{summary}}\n" + customReview := customReviewTemplate if err := os.WriteFile(filepath.Join(tmplSubdir, "custom_plan.md"), []byte(customPlan), 0o600); err != nil { t.Fatalf("write custom plan: %v", err) diff --git a/packages/go/singlerequesttemplate/template.go b/packages/go/singlerequesttemplate/template.go index e16ce2df..7fb8af5c 100644 --- a/packages/go/singlerequesttemplate/template.go +++ b/packages/go/singlerequesttemplate/template.go @@ -7,6 +7,7 @@ import ( "fmt" "regexp" "strings" + "unicode/utf8" ) const MaxTemplateBytes = 8192 @@ -25,17 +26,17 @@ const DefaultPlanTemplate = `# Plan const DefaultReviewTemplate = `# Review -## Result -PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations +{{deviations}} ` var ( @@ -50,14 +51,15 @@ var placeholderRegex = regexp.MustCompile(`\{\{[^}]*\}\}`) var ( planPlaceholders = []string{"{{goal}}", "{{steps}}", "{{verification}}"} planHeadings = []string{"# Plan", "## Goal", "## Steps", "## Verification"} - reviewPlaceholders = []string{"{{checks}}", "{{verification}}", "{{summary}}"} - reviewLines = []string{"# Review", "## Result", "PASS", "## Checks", "## Verification", "## Summary"} + reviewPlaceholders = []string{"{{item_status}}", "{{changes}}", "{{verification}}", "{{deviations}}"} + reviewHeadings = []string{"# Review", "## Worker Item Status", "## Worker Changes", "## Worker Verification", "## Deviations"} ) type ReviewFields struct { - Checks string + ItemStatus string + Changes string Verification string - Summary string + Deviations string } type PlanFields struct { @@ -188,24 +190,62 @@ func ValidateReviewTemplate(tmpl string) error { if err != nil { return err } - idxChecks, idxVerif, idxSumm := placeholders[0], placeholders[1], placeholders[2] - if !ascending(idxChecks, idxVerif, idxSumm) { - return fmt.Errorf("%w: placeholders must appear in order {{checks}}, {{verification}}, {{summary}}", ErrInvalidTemplate) + idxItemStatus, idxChanges, idxVerif, idxDeviations := placeholders[0], placeholders[1], placeholders[2], placeholders[3] + if !ascending(idxItemStatus, idxChanges, idxVerif, idxDeviations) { + return fmt.Errorf("%w: placeholders must appear in order {{item_status}}, {{changes}}, {{verification}}, {{deviations}}", ErrInvalidTemplate) } - lines, err := requireExactLines(tmpl, reviewLines) + headings, err := requireExactLines(tmpl, reviewHeadings) if err != nil { return err } - idxReviewH, idxResultH, idxPass := lines[0], lines[1], lines[2] - idxChecksH, idxVerifH, idxSummH := lines[3], lines[4], lines[5] - if !ascending(idxReviewH, idxResultH, idxPass, idxChecksH, idxChecks, idxVerifH, idxVerif, idxSummH, idxSumm) { + idxReviewH, idxItemStatusH, idxChangesH, idxVerifH, idxDeviationsH := headings[0], headings[1], headings[2], headings[3], headings[4] + if !ascending(idxReviewH, idxItemStatusH, idxItemStatus, idxChangesH, idxChanges, idxVerifH, idxVerif, idxDeviationsH, idxDeviations) { return fmt.Errorf("%w: headings and placeholders must follow exact structural order", ErrInvalidTemplate) } + // Close the heading set: only the documented worker headings may appear. A + // reviewer-only section (Result, Checks, Summary) or any other markdown + // heading would let the template describe a reviewer verdict or final review + // page, so it is rejected rather than silently tolerated. + if err := rejectUnknownMarkdownHeadings(tmpl, reviewHeadings); err != nil { + return err + } + return nil } +// rejectUnknownMarkdownHeadings ensures every standalone line beginning with +// "#" is one of the allowed documented headings. Decorated variants (e.g. +// "### Review") and reviewer-only headings ("## Result") are rejected because +// the only standalone matching already happens in requireExactLines; here we +// additionally forbid any extra heading that is not in the closed set. +func rejectUnknownMarkdownHeadings(tmpl string, allowed []string) error { + allowedSet := make(map[string]struct{}, len(allowed)) + for _, line := range allowed { + allowedSet[line] = struct{}{} + } + offset := 0 + for { + var line string + end := strings.IndexByte(tmpl[offset:], '\n') + if end < 0 { + line = tmpl[offset:] + } else { + line = tmpl[offset : offset+end] + } + if strings.HasPrefix(line, "#") { + if _, ok := allowedSet[line]; !ok { + return fmt.Errorf("%w: template declares an unknown heading", ErrInvalidTemplate) + } + } + if end < 0 { + return nil + } + offset += end + 1 + } +} + func ParsePlan(tmpl string, rawOutput string, maxOutputBytes int) ([]byte, error) { if maxOutputBytes < 1 || len(rawOutput) > maxOutputBytes { return nil, ErrMalformedPlan @@ -284,7 +324,23 @@ func normalizePlanSections(goal, steps, verification string) (string, string, st return "", "", "", ErrMalformedPlan } - normalizeBullets := func(value string, minimum, maximum int) (string, error) { + 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 @@ -299,11 +355,11 @@ func normalizePlanSections(goal, steps, verification string) (string, string, st return strings.Join(lines, "\n"), nil } - steps, err := normalizeBullets(steps, 2, 6) + steps, err := normalizeStepBullets(steps, 2, 6) if err != nil { return "", "", "", err } - verification, err = normalizeBullets(verification, 1, 3) + verification, err = normalizeVerificationBullets(verification, 1, 3) if err != nil { return "", "", "", err } @@ -315,7 +371,21 @@ func normalizePlanFields(fields PlanFields) (string, string, string, error) { if goal == "" || strings.ContainsAny(goal, "\r\n") || strings.Contains(goal, "{{") || strings.Contains(goal, "}}") { return "", "", "", ErrMalformedPlan } - normalizeItems := func(items []string, minimum, maximum int) (string, error) { + normalizeStepItems := func(items []string, minimum, maximum int) (string, error) { + if len(items) < minimum || len(items) > maximum { + return "", ErrMalformedPlan + } + lines := make([]string, len(items)) + for i, item := range items { + item = strings.TrimSpace(item) + if item == "" || strings.ContainsAny(item, "\r\n") || strings.Contains(item, "{{") || strings.Contains(item, "}}") { + return "", ErrMalformedPlan + } + lines[i] = fmt.Sprintf("- [P%d] %s", i+1, item) + } + return strings.Join(lines, "\n"), nil + } + normalizeVerificationItems := func(items []string, minimum, maximum int) (string, error) { if len(items) < minimum || len(items) > maximum { return "", ErrMalformedPlan } @@ -329,11 +399,11 @@ func normalizePlanFields(fields PlanFields) (string, string, string, error) { } return strings.Join(lines, "\n"), nil } - steps, err := normalizeItems(fields.Steps, 2, 6) + steps, err := normalizeStepItems(fields.Steps, 2, 6) if err != nil { return "", "", "", err } - verification, err := normalizeItems(fields.Verification, 1, 3) + verification, err := normalizeVerificationItems(fields.Verification, 1, 3) if err != nil { return "", "", "", err } @@ -369,16 +439,18 @@ func RenderReview(tmpl string, fields ReviewFields, maxOutputBytes int) ([]byte, return nil, err } - c := strings.TrimSpace(fields.Checks) - v := strings.TrimSpace(fields.Verification) - s := strings.TrimSpace(fields.Summary) - if c == "" || v == "" || s == "" { + itemStatus := strings.TrimSpace(fields.ItemStatus) + 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, "{{checks}}", c) - res = strings.ReplaceAll(res, "{{verification}}", v) - res = strings.ReplaceAll(res, "{{summary}}", s) + res := strings.ReplaceAll(tmpl, "{{item_status}}", itemStatus) + res = strings.ReplaceAll(res, "{{changes}}", changes) + res = strings.ReplaceAll(res, "{{verification}}", verification) + res = strings.ReplaceAll(res, "{{deviations}}", deviations) if strings.Contains(res, "{{") || strings.Contains(res, "}}") { return nil, ErrMalformedReview @@ -390,3 +462,125 @@ 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 71f4b745..6bce99c5 100644 --- a/packages/go/singlerequesttemplate/template_test.go +++ b/packages/go/singlerequesttemplate/template_test.go @@ -13,8 +13,8 @@ func planTemplateOfSize(size int) string { return singlerequesttemplate.DefaultPlanTemplate + strings.Repeat(" ", size-len(singlerequesttemplate.DefaultPlanTemplate)) } -// reviewTemplateOfSize pads the built-in Review template with trailing static -// text so the returned template is exactly size bytes long. +// reviewTemplateOfSize pads the built-in Review handoff template with trailing +// static text so the returned template is exactly size bytes long. func reviewTemplateOfSize(size int) string { return singlerequesttemplate.DefaultReviewTemplate + strings.Repeat(" ", size-len(singlerequesttemplate.DefaultReviewTemplate)) } @@ -262,8 +262,8 @@ func TestParsePlan(t *testing.T) { Fix single-request template handling bug. ## Steps -- Inspect template file resolution. -- Verify template validation logic. +- [P1] Inspect template file resolution. +- [P2] Verify template validation logic. ## Verification - Run go test on singlerequesttemplate package. @@ -318,8 +318,8 @@ END Fix suffix parsing. ## Steps -- Keep the static suffix. -- Allow the final line feed omission. +- [P1] Keep the static suffix. +- [P2] Allow the final line feed omission. ## Verification - Run the parser tests. @@ -356,12 +356,12 @@ END Implement feature end to end. ## Steps -- Step one -- Step two -- Step three -- Step four -- Step five -- Step six +- [P1] Step one +- [P2] Step two +- [P3] Step three +- [P4] Step four +- [P5] Step five +- [P6] Step six ## Verification - Verify 1 @@ -380,7 +380,7 @@ Implement feature end to end. Implement feature. ## Steps -- Step one +- [P1] Step one ## Verification - Verify 1 @@ -397,13 +397,13 @@ Implement feature. Implement feature. ## Steps -- Step 1 -- Step 2 -- Step 3 -- Step 4 -- Step 5 -- Step 6 -- Step 7 +- [P1] Step 1 +- [P2] Step 2 +- [P3] Step 3 +- [P4] Step 4 +- [P5] Step 5 +- [P6] Step 6 +- [P7] Step 7 ## Verification - Verify 1 @@ -420,8 +420,8 @@ Implement feature. Implement feature. ## Steps -- Step 1 -- Step 2 +- [P1] Step 1 +- [P2] Step 2 ## Verification @@ -438,14 +438,68 @@ Implement feature. Implement feature. ## Steps -- Step 1 -- Step 2 +- [P1] Step 1 +- [P2] Step 2 ## Verification - Verify 1 - Verify 2 - Verify 3 - Verify 4 +`, + maxOutputBytes: 1024, + wantErr: true, + }, + { + name: "step missing deterministic P1 id", + tmpl: singlerequesttemplate.DefaultPlanTemplate, + raw: `# Plan + +## Goal +Implement feature. + +## Steps +- Inspect template file resolution. +- Verify template validation logic. + +## Verification +- Run go test on singlerequesttemplate package. +`, + maxOutputBytes: 1024, + wantErr: true, + }, + { + name: "step id out of order P1, P3", + tmpl: singlerequesttemplate.DefaultPlanTemplate, + raw: `# Plan + +## Goal +Implement feature. + +## Steps +- [P1] Step one +- [P3] Step three + +## Verification +- Verify 1 +`, + maxOutputBytes: 1024, + wantErr: true, + }, + { + name: "step id duplicate P1, P1", + tmpl: singlerequesttemplate.DefaultPlanTemplate, + raw: `# Plan + +## Goal +Implement feature. + +## Steps +- [P1] Step one +- [P1] Step dup + +## Verification +- Verify 1 `, maxOutputBytes: 1024, wantErr: true, @@ -460,8 +514,8 @@ First line of goal. Second line of goal. ## Steps -- Step 1 -- Step 2 +- [P1] Step 1 +- [P2] Step 2 ## Verification - Verify 1 @@ -478,8 +532,8 @@ Second line of goal. Fix bug. ## Steps -- Step 1 -- Step 2 +- [P1] Step 1 +- [P2] Step 2 ## Verification - Verify 1 @@ -496,8 +550,8 @@ Fix bug. Fix {{goal}} bug. ## Steps -- Step 1 -- Step 2 +- [P1] Step 1 +- [P2] Step 2 ## Verification - Verify 1 @@ -533,7 +587,7 @@ func TestRenderPlan(t *testing.T) { Steps: []string{" Step one. ", "\tStep two. "}, Verification: []string{" Run focused tests. "}, } - want := "# Plan\n\n## Goal\nInspect the target.\n\n## Steps\n- Step one.\n- Step two.\n\n## Verification\n- Run focused tests.\n" + want := "# Plan\n\n## Goal\nInspect the target.\n\n## Steps\n- [P1] Step one.\n- [P2] Step two.\n\n## Verification\n- Run focused tests.\n" got, err := singlerequesttemplate.RenderPlan(singlerequesttemplate.DefaultPlanTemplate, fields, 1024) if err != nil { t.Fatal(err) @@ -543,7 +597,7 @@ func TestRenderPlan(t *testing.T) { } custom := "# Plan\n\nOperator note.\n\n## Goal\n{{goal}}\n\n## Steps\n{{steps}}\n\n## Verification\n{{verification}}\n\nEND\n" - wantCustom := "# Plan\n\nOperator note.\n\n## Goal\nInspect the target.\n\n## Steps\n- Step one.\n- Step two.\n\n## Verification\n- Run focused tests.\n\nEND\n" + wantCustom := "# Plan\n\nOperator note.\n\n## Goal\nInspect the target.\n\n## Steps\n- [P1] Step one.\n- [P2] Step two.\n\n## Verification\n- Run focused tests.\n\nEND\n" got, err = singlerequesttemplate.RenderPlan(custom, fields, 1024) if err != nil { t.Fatal(err) @@ -582,6 +636,43 @@ func TestRenderPlanRejectsMalformedFields(t *testing.T) { } } +func TestPlanItemIDs(t *testing.T) { + rendered, err := singlerequesttemplate.RenderPlan(singlerequesttemplate.DefaultPlanTemplate, singlerequesttemplate.PlanFields{ + Goal: "Goal line.", + Steps: []string{"Step one.", "Step two.", "Step three."}, + Verification: []string{"Verify."}, + }, 4096) + if err != nil { + t.Fatal(err) + } + ids, err := singlerequesttemplate.PlanItemIDs(rendered) + if err != nil || len(ids) != 3 || ids[0] != "P1" || ids[1] != "P2" || ids[2] != "P3" { + t.Fatalf("ids=%v err=%v", ids, err) + } + + tests := []struct { + name string + plan string + wantErr bool + }{ + {"empty", "", true}, + {"no step ids", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- Step one\n- Step two\n\n## Verification\n- V\n", true}, + {"gap p1 then p3", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- [P1] Step one\n- [P3] Step three\n\n## Verification\n- V\n", true}, + {"duplicate p1", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- [P1] Step one\n- [P1] Step dup\n\n## Verification\n- V\n", true}, + {"starts at p0", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- [P0] Step zero\n- [P1] Step one\n\n## Verification\n- V\n", true}, + {"descending order", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- [P2] Step two\n- [P1] Step one\n\n## Verification\n- V\n", true}, + {"valid p1 p2", "# Plan\n\n## Goal\nGoal.\n\n## Steps\n- [P1] Step one\n- [P2] Step two\n\n## Verification\n- V\n", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := singlerequesttemplate.PlanItemIDs([]byte(tt.plan)) + if (err != nil) != tt.wantErr { + t.Fatalf("err=%v wantErr=%v", err, tt.wantErr) + } + }) + } +} + func TestValidateReviewTemplate(t *testing.T) { tests := []struct { name string @@ -589,7 +680,7 @@ func TestValidateReviewTemplate(t *testing.T) { wantErr bool }{ { - name: "default review template is valid", + name: "default review handoff template is valid", tmpl: singlerequesttemplate.DefaultReviewTemplate, wantErr: false, }, @@ -598,42 +689,6 @@ func TestValidateReviewTemplate(t *testing.T) { tmpl: "", wantErr: true, }, - { - name: "missing PASS", - tmpl: `# Review - -## Result -FAIL - -## Checks -{{checks}} - -## Verification -{{verification}} - -## Summary -{{summary}} -`, - wantErr: true, - }, - { - name: "wrong order", - tmpl: `# Review - -## Result -PASS - -## Verification -{{verification}} - -## Checks -{{checks}} - -## Summary -{{summary}} -`, - wantErr: true, - }, { name: "oversized template 8193 bytes", tmpl: reviewTemplateOfSize(8193), @@ -645,110 +700,37 @@ PASS wantErr: false, }, { - name: "NOTPASS does not satisfy the PASS result line", + name: "missing {{deviations}}", tmpl: `# Review -## Result -NOTPASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations `, wantErr: true, }, { - name: "PASS embedded in a prose line", + name: "duplicate {{changes}}", tmpl: `# Review -## Result -Result: PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} {{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} -`, - wantErr: true, - }, - { - name: "decorated heading ### Review", - tmpl: `### Review - -## Result -PASS - -## Checks -{{checks}} - -## Verification -{{verification}} - -## Summary -{{summary}} -`, - wantErr: true, - }, - { - name: "duplicate PASS result line", - tmpl: `# Review - -## Result -PASS -PASS - -## Checks -{{checks}} - -## Verification -{{verification}} - -## Summary -{{summary}} -`, - wantErr: true, - }, - { - name: "missing {{summary}}", - tmpl: `# Review - -## Result -PASS - -## Checks -{{checks}} - -## Verification -{{verification}} - -## Summary -`, - wantErr: true, - }, - { - name: "duplicate {{checks}}", - tmpl: `# Review - -## Result -PASS - -## Checks -{{checks}} {{checks}} - -## Verification -{{verification}} - -## Summary -{{summary}} +## Deviations +{{deviations}} `, wantErr: true, }, @@ -756,22 +738,190 @@ PASS name: "unknown placeholder", tmpl: `# Review +## Worker Item Status +{{item_status}} {{severity}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "wrong placeholder order", + tmpl: `# Review + +## Worker Item Status +{{changes}} + +## Worker Changes +{{item_status}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "missing required heading # Review", + tmpl: `## Worker Item Status +{{item_status}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "decorated heading ### Review", + tmpl: `### Review + +## Worker Item Status +{{item_status}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "decorated worker heading", + tmpl: `# Review + +### Worker Item Status +{{item_status}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "duplicate worker heading", + tmpl: `# Review + +## Worker Item Status +{{item_status}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} + +## Worker Item Status +`, + wantErr: true, + }, + { + name: "unbalanced opening delimiter residue", + tmpl: `# Review + +## Worker Item Status +{{item_status}} {{ + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "unbalanced closing delimiter residue", + tmpl: `# Review + +## Worker Item Status +{{item_status}}}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "reviewer-only Result placeholder rejected", + tmpl: `# Review + ## Result PASS -## Checks -{{checks}} {{severity}} +## Worker Item Status +{{item_status}} -## Verification +## Worker Changes +{{changes}} + +## Worker Verification {{verification}} +## Deviations +{{deviations}} +`, + wantErr: true, + }, + { + name: "reviewer-only Summary placeholder rejected", + tmpl: `# Review + +## Worker Item Status +{{item_status}} + +## Worker Changes +{{changes}} + +## Worker Verification +{{verification}} + +## Deviations +{{deviations}} + ## Summary {{summary}} `, wantErr: true, }, { - name: "unbalanced delimiter residue", + name: "legacy reviewer grammar fully rejected", tmpl: `# Review ## Result @@ -784,27 +934,27 @@ PASS {{verification}} ## Summary -{{summary}} }} +{{summary}} `, wantErr: true, }, { - name: "custom review template with extra static text", + name: "custom handoff template with extra static text", tmpl: `# Review Operator preamble. -## Result -PASS +## Worker Item Status +{{item_status}} -## Checks -{{checks}} +## Worker Changes +{{changes}} -## Verification +## Worker Verification {{verification}} -## Summary -{{summary}} +## Deviations +{{deviations}} Operator footer. `, @@ -824,42 +974,276 @@ Operator footer. func TestRenderReview(t *testing.T) { fields := singlerequesttemplate.ReviewFields{ - Checks: "- Checked file permissions\n- Verified build pass", - Verification: "- Executed unit test suite", - Summary: "All requirements met successfully.", + ItemStatus: "- P1: completed\n- P2: completed", + Changes: "- Wrote result.txt with the requested value", + Verification: "- Ran verify and observed success", + Deviations: "None", } - got, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 1024) + got, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) if err != nil { t.Fatalf("RenderReview() unexpected err = %v", err) } want := `# Review -## Result -PASS +## Worker Item Status +- P1: completed +- P2: completed -## Checks -- Checked file permissions -- Verified build pass +## Worker Changes +- Wrote result.txt with the requested value -## Verification -- Executed unit test suite +## Worker Verification +- Ran verify and observed success -## Summary -All requirements met successfully. +## Deviations +None ` if string(got) != want { t.Errorf("RenderReview() got:\n%s\nwant:\n%s", string(got), want) } - // Missing field test - badFields := fields - badFields.Summary = "" - _, err = singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, badFields, 1024) - if err == nil { - t.Errorf("RenderReview() expected error for empty Summary, got nil") + custom := "# Review\n\nOperator preamble.\n\n## Worker Item Status\n{{item_status}}\n\n## Worker Changes\n{{changes}}\n\n## Worker Verification\n{{verification}}\n\n## Deviations\n{{deviations}}\n\nOperator footer.\n" + wantCustom := `# Review + +Operator preamble. + +## Worker Item Status +- P1: completed +- P2: completed + +## Worker Changes +- Wrote result.txt with the requested value + +## Worker Verification +- Ran verify and observed success + +## Deviations +None + +Operator footer. +` + got, err = singlerequesttemplate.RenderReview(custom, fields, 4096) + if err != nil { + t.Fatalf("custom RenderReview() err=%v", err) } + if string(got) != wantCustom { + t.Errorf("custom RenderReview() got:\n%s\nwant:\n%s", string(got), wantCustom) + } + + // Missing field test: every worker section must be non-empty. + 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: ""}, + "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, fields, 0); err == nil { + t.Fatalf("RenderReview() expected error for zero max") + } + if _, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 10); err == nil { + t.Fatalf("RenderReview() expected error for output over limit") + } +} + +func TestValidateReviewHandoff(t *testing.T) { + planIDs := []string{"P1", "P2"} + + baseFields := singlerequesttemplate.ReviewFields{ + ItemStatus: "- P1: completed\n- P2: completed", + Changes: "- Wrote result.txt", + Verification: "- Ran verify", + Deviations: "None", + } + valid, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, baseFields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(valid, planIDs); err != nil { + t.Fatalf("valid handoff rejected: %v", err) + } + + t.Run("missing or duplicate sections rejected", func(t *testing.T) { + missingItemStatus := strings.Replace(string(valid), "## Worker Item Status", "## Renamed", 1) + if err := singlerequesttemplate.ValidateReviewHandoff([]byte(missingItemStatus), planIDs); err == nil { + t.Fatalf("expected error for missing Worker Item Status heading") + } + dup := string(valid) + "\n## Worker Item Status\n- P1: completed\n" + if err := singlerequesttemplate.ValidateReviewHandoff([]byte(dup), planIDs); err == nil { + t.Fatalf("expected error for duplicate Worker Item Status heading") + } + }) + + t.Run("empty content rejected", func(t *testing.T) { + if err := singlerequesttemplate.ValidateReviewHandoff(nil, planIDs); err == nil { + t.Fatalf("expected error for empty content") + } + }) + + t.Run("unknown plan id rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n- P9: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for unknown plan id P9") + } + }) + + t.Run("missing plan id rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for missing plan id P2") + } + }) + + t.Run("duplicate plan id rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n- P1: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for duplicate plan id P1") + } + }) + + t.Run("non-completed status rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n- P2: skipped" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for non-completed P2") + } + }) + + t.Run("out-of-order plan ids rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P2: completed\n- P1: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for out-of-order plan ids") + } + }) + + t.Run("extra status line rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n- P2: completed\n- P3: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for extra plan id P3") + } + }) + + t.Run("empty plan id list rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, nil); err == nil { + t.Fatalf("expected error for empty plan id list against non-empty status") + } + }) + + t.Run("prose injected between status lines rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\nThis is a note.\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for prose injected between status lines") + } + }) + + t.Run("malformed bullet missing completed status rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: incomplete\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for malformed bullet missing completed status") + } + }) + + t.Run("blank line in status section rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed\n\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for blank line in status section") + } + }) + + t.Run("out-of-order status lines rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P2: completed\n- P1: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for out-of-order status lines") + } + }) + + t.Run("malformed bullet with extra text rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "- P1: completed extra text\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for malformed bullet with extra text") + } + }) + + t.Run("decorated heading variant in status rejected", func(t *testing.T) { + fields := baseFields + fields.ItemStatus = "## P1: completed\n- P2: completed" + rendered, err := singlerequesttemplate.RenderReview(singlerequesttemplate.DefaultReviewTemplate, fields, 4096) + if err != nil { + t.Fatal(err) + } + if err := singlerequesttemplate.ValidateReviewHandoff(rendered, planIDs); err == nil { + t.Fatalf("expected error for decorated heading variant in status") + } + }) } func TestDigest(t *testing.T) {