<!-- task=m-iop-owned-single-request-agent-execution/21+20_single_request_executor plan=1 tag=REVIEW_API milestone-task=review-stage -->

# Composite isolation and terminal waiter evidence

## For the Implementing Agent

Filling the implementation-owned sections of `CODE_REVIEW-cloud-G06.md` is the mandatory final step. Run every verification command, paste actual output, keep both active files in place, and report ready for review. Only the official code-review skill may append a verdict, archive files, write `complete.log`, or classify the next state. If blocked, record the exact blocker, attempted commands/output, and resume condition in implementation-owned evidence fields; do not ask the user, call user-input tools, create control-plane stop files, or change the owner or scope.

## Background

The composite executor passed its deterministic package checks, but the first official review found that its named concurrency and waiter-cleanup fixtures do not prove the claimed request isolation or terminal cleanup behavior. This follow-up keeps production behavior unchanged and adds path-faithful service-backed evidence for the SDD S10 composite boundary.

## Archive Evidence Snapshot

- `agent-task/m-iop-owned-single-request-agent-execution/21+20_single_request_executor/plan_local_G08_0.log` defines the original composite lifecycle scope and verification contract.
- `agent-task/m-iop-owned-single-request-agent-execution/21+20_single_request_executor/code_review_cloud_G08_0.log` records `FAIL` with Required R1: concurrent requests use no tool continuation and assert one shared output, while waiter cleanup calls `clearRequest` directly instead of exercising a live terminal path.
- Fresh reviewer runs passed the dependency check, focused executor race tests, service compatibility tests, OpenAI vet/regression, activation guard, and `git diff --check`; the reviewer also removed one unused executor error sentinel as a repaired Nit.

## Finding Resolution Map

| Finding | Mode | Exact fix evidence | Changed precondition |
|---------|------|--------------------|----------------------|
| Required R1 | direct-fix | Replace the weak fixtures in `apps/edge/internal/openai/single_request_executor_test.go` with request-distinguishing, service-backed composite tool/cancel/failure tests. | The repeated race and cleanup verification will execute real correlated waiters and can fail on cross-request delivery or terminal cleanup leaks. |

## Analysis

### Files Read

- `apps/edge/internal/openai/single_request_executor.go`
- `apps/edge/internal/openai/single_request_executor_test.go`
- `apps/edge/internal/openai/single_request_plan_stage.go`
- `apps/edge/internal/openai/single_request_work_stage.go`
- `apps/edge/internal/openai/single_request_review_stage.go`
- `apps/edge/internal/openai/single_request_provider_stage.go`
- `apps/edge/internal/openai/single_request_provider_stage_test.go`
- `apps/edge/internal/openai/single_request_work_stage_test.go`
- `apps/edge/internal/openai/single_request_review_stage_test.go`
- `apps/edge/internal/service/single_request.go`
- `apps/edge/internal/service/single_request_types.go`
- `apps/edge/internal/service/single_request_tool_loop.go`
- `apps/edge/internal/service/single_request_artifact.go`
- `agent-spec/input/openai-compatible-surface.md`
- `agent-spec/runtime/edge-node-execution.md`
- `agent-contract/outer/anthropic-compatible-api.md`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `agent-roadmap/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.md`
- `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`

### SDD Criteria

- SDD: `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`, status `[approved]`, lock released.
- Milestone contribution: `review-stage`; targeted scenario S10 and its Evidence Map row require Review pass/defect/repair plus finalization evidence.
- The follow-up specifically proves that the composite preserves request/tool identity under concurrency and removes request-local continuation state on success, failure, and cancellation before S10 evidence is accepted.

### Verification Context

- Handoff source: Required R1 and routing signals from `code_review_cloud_G08_0.log`.
- Repository-native sources: Edge/testing domain rules, `agent-test/local/rules.md`, `agent-test/local/edge-smoke.md`, the SDD, contracts, source, and related tests.
- Fresh reviewer evidence: dependency resolution, `go test -race ./apps/edge/internal/openai -run 'TestSingleRequestExecutor' -count=1`, service compatibility, OpenAI vet/regression, activation guard, and `git diff --check` all passed.
- Environment: current checkout, Linux arm64, `go version go1.26.2 linux/arm64`; no remote runner, provider credential, model endpoint, or external service is needed.
- Constraint: package tests are deterministic internal S10 evidence only and do not claim S12 Claude/Mac qualification.
- Gap: existing composite tests cannot distinguish cross-request tool/result/artifact leakage and do not cancel a live registered waiter.
- Confidence: high; the gap is directly visible in the assertions and can be closed within one test file.

### Test Coverage Gaps

- Concurrent composite isolation: not covered meaningfully; the existing fixture uses no workspace tool and expects the same output for every request.
- Terminal waiter cleanup: not covered through the executor lifecycle; the existing fixture calls the internal cleanup helper directly.
- Plan/Work/Review sequencing and reviewer-only terminal provenance: covered by existing service-backed composite tests and retained as regression checks.

### Symbol References

- No production symbol is renamed or removed by this follow-up.
- `NewSingleRequestExecutor`, `SingleRequestExecutor`, and `SingleRequestToolContinuation` remain unchanged and production activation remains deferred.

### Split Judgment

Keep one compact test-only packet. Concurrent correlation and terminal cleanup share the same composite harness and race oracle; splitting would duplicate setup without yielding an independently useful implementation boundary.

### Scope Rationale

Include only composite test fixtures and their active review evidence. Exclude production executor/stage/service behavior, input-manager activation, spec/contract synchronization, broad Edge tests, and S12 external qualification because R1 is solely an evidence-quality defect.

### Final Routing

- `evaluation_mode=isolated-reassessment`; finalizer `finalize-task-policy.sh`, mode `pair`.
- Build closures `scope/context/verification/evidence/ownership/decision=true`; scores `1/2/0/2/1` => G06, base `local-fit`, final basis `recovery-boundary` because `review_rework_count=1` and `evidence_integrity_failure=true`; route `worker/cloud/G06`, `PLAN-cloud-G06.md`.
- Review closures `scope/context/verification/evidence/ownership/decision=true`; scores `1/2/0/2/1` => G06; route `official-review`, `review/cloud/G06`, `CODE_REVIEW-cloud-G06.md`.
- `large_indivisible_context=false`; positive loop risks `temporal_state`, `concurrent_consistency`, `boundary_contract` (3); no capability gap.

## Implementation Checklist

- [ ] Replace the composite concurrency fixture with request-distinguishing service-backed tool continuations that deliberately reuse one tool-call id and assert per-request artifacts, results, and final output.
- [ ] Replace direct helper cleanup coverage with live composite success, stage/tool failure, and cancellation cases that register real waiters, preserve an unaffected peer request, and finish with zero pending bridge entries.
- [ ] Run the dependency, focused race, service compatibility, vet/regression, fixture guard, production deferral, formatting, and diff checks.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Prove request isolation and terminal waiter cleanup

**Problem**

At `apps/edge/internal/openai/single_request_executor_test.go:256`, all concurrent requests avoid workspace tools and accept the same `"Review Approved node"` result, so request/result crossover is invisible. At `apps/edge/internal/openai/single_request_executor_test.go:497`, the test registers a bridge entry and invokes `clearRequest` directly, bypassing executor success, failure, and cancellation paths.

**Solution**

Replace the weak assertions with two path-faithful fixtures.

Before (`apps/edge/internal/openai/single_request_executor_test.go:309` and `:511`):

```go
expected := "Review Approved node"
bridge.clearRequest("req-cleanup")
```

After:

```go
// Concurrent requests intentionally reuse "colliding-tool-id" while their
// request-specific tool result, artifact, and reviewer output remain distinct.
if result.Output != expectedByRequest[reqID] { /* fail */ }

// Success, failure, and cancellation run through StartSingleRequest with a
// registered continuation waiter; no test calls clearRequest directly.
if executor.bridge.pendingCount() != 0 { /* fail */ }
```

Use a request-indexed test artifact/result store rather than the existing single shared `plan` slot for the concurrent case. Block one real Node tool response long enough to cancel its request while a peer with the same tool-call id completes, then assert the cancelled request cannot consume or clear the peer continuation. Cover successful completion and a post-registration stage/tool failure with the same zero-waiter oracle.

**Modified Files and Checklist**

- [ ] Update `apps/edge/internal/openai/single_request_executor_test.go` with `TestSingleRequestExecutorConcurrentToolIsolation` and `TestSingleRequestExecutorTerminalWaiterCleanup`.
- [ ] Fill `agent-task/m-iop-owned-single-request-agent-execution/21+20_single_request_executor/CODE_REVIEW-cloud-G06.md` with actual implementation and command evidence.

**Test Strategy**

Write deterministic service-backed tests in `apps/edge/internal/openai/single_request_executor_test.go`. `TestSingleRequestExecutorConcurrentToolIsolation` must use at least two concurrent requests, the literal colliding tool id, request-specific tool results/artifacts, and distinct reviewer-approved outputs. `TestSingleRequestExecutorTerminalWaiterCleanup` must exercise success, failure, and cancellation after a waiter is registered; it must prove an unaffected peer completes and `pendingCount()` is zero. Run both under `-race`.

**Verification**

Run the focused race command and fixture guard in Final Verification; both must pass without direct test calls to `bridge.clearRequest`.

## Modified Files Summary

| File | Items |
|------|-------|
| `apps/edge/internal/openai/single_request_executor_test.go` | REVIEW_API-1 |
| `agent-task/m-iop-owned-single-request-agent-execution/21+20_single_request_executor/CODE_REVIEW-cloud-G06.md` | REVIEW_API-1 |

## Final Verification

Fresh Go test output is required; cached output is not acceptable.

1. `bash -c 'set -euo pipefail; shopt -s nullglob; candidates=(agent-task/m-iop-owned-single-request-agent-execution/20_*/complete.log agent-task/m-iop-owned-single-request-agent-execution/20+*/complete.log agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/20_*/complete.log agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/20+*/complete.log); printf "%s\n" "${candidates[@]}"; ((${#candidates[@]} == 1))'` — prints exactly one predecessor completion path and exits zero.
2. `go test -race ./apps/edge/internal/openai -run 'TestSingleRequestExecutor' -count=1` — composite pass/inspection/repair plus request-distinguishing concurrency and live terminal waiter cleanup pass without races.
3. `go test ./apps/edge/internal/service -run 'TestSingleRequest(InternalToolLoop|Cleanup|EnvelopeOrdering)' -count=1` — service lifecycle compatibility passes freshly.
4. `go vet ./apps/edge/internal/openai && go test ./apps/edge/internal/service ./apps/edge/internal/openai -count=1` — changed-path packages vet and regress cleanly.
5. `bash -c 'set -euo pipefail; rg --sort path -n "TestSingleRequestExecutorConcurrentToolIsolation|TestSingleRequestExecutorTerminalWaiterCleanup|colliding-tool-id|pendingCount" apps/edge/internal/openai/single_request_executor_test.go; if rg --sort path -n "bridge\\.clearRequest" apps/edge/internal/openai/single_request_executor_test.go; then exit 1; else test $? -eq 1; fi'` — finds both path-faithful fixtures and no direct cleanup-helper call.
6. `bash -c 'set -euo pipefail; if rg --sort path -n "NewSingleRequestExecutor|SetSingleRequestExecutor" apps/edge/internal/input/manager.go; then exit 1; else test $? -eq 1; fi'` — exits zero with no output while production activation remains deferred.
7. `test -z "$(gofmt -d apps/edge/internal/openai/single_request_executor_test.go)"` — the modified test is formatted.
8. `git diff --check` — no whitespace errors.

S12 external Claude/Mac qualification remains outside this test-only follow-up.

After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
