<!-- task=m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission plan=1 tag=REVIEW_API milestone-task=workspace-binding -->

# Close Workspace Admission Review Findings

## For the Implementing Agent

Implement Required R1-R5 exactly within `Modified Files Summary`, run every verification command, and fill all implementation-owned sections in `CODE_REVIEW-cloud-G09.md` with actual notes and stdout/stderr. Keep the active pair in place and report ready for review. If blocked, record only the exact blocker, attempted command/output, and resume condition in implementation-owned evidence; do not ask the user, call user-input tools, create stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The first workspace-admission review found a public bypass, operation-incompatible limit validation, absent race/admission coverage, stale living-spec text, and an entirely unfilled implementation evidence artifact. The follow-up keeps the original S04 boundary: bind one approved opaque ref to its configured ready Node generation before executor startup, expose only closed capability ids and effective limits, and never reselect. Concrete Node tool execution and wire transport remain later packets.

## Archive Evidence Snapshot

- Prior plan: `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/plan_cloud_G08_0.log`
- Prior review: `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/code_review_cloud_G09_0.log`
- Verdict: `FAIL`; routing signals: `review_rework_count=1`, `evidence_integrity_failure=false`.
- Required findings: R1 remove the zero-value service admission bypass; R2 accept valid operation-specific zero limits; R3 add the missing admission matrix and real reconnect/refresh race evidence; R4 synchronize the runtime living spec; R5 fill the implementation-owned review evidence and record the `service.go` boundary deviation.
- Fresh reviewer verification: packet 03 and packet 07 dependencies passed; registry race command passed; admission race command returned `ok ... [no tests to run]`; focused packages and vet passed; `git diff --check` passed; the spec search exposed only generic admission text and the stale concrete-workspace deferral.
- Roadmap carryover: `milestone-task=workspace-binding`, SDD scenario S04 and its fail-closed workspace admission evidence remain the completion target.

## Finding Resolution Map

| Finding | Mode | Exact resolution | Changed precondition |
|---------|------|------------------|----------------------|
| R1 | `direct-fix` | Remove the `registry == nil && store == nil` public bypass in `service.go`; use `startSingleRequest` directly only in coordinator-only unit tests. | Every public `Service.StartSingleRequest` call must pass workspace admission before executor startup. |
| R2 | `direct-fix` | Make workspace binding validation conditional on enabled operation ids in `single_request_types.go` and `single_request_workspace.go`; retain zero only for disabled-operation limits. | Valid read/list/write/delete/command subsets can be admitted without weakening enabled-operation bounds. |
| R3 | `direct-fix` | Create `single_request_workspace_test.go` and make the registry reconnect case concurrent, with deterministic synchronization and executor call assertions. | The exact focused commands execute named tests and exercise reconnect/refresh races rather than empty or sequential selections. |
| R4 | `direct-fix` | Update `edge-node-execution.md` with the implemented ref-to-owner/generation admission and the narrower deferred executor/wire boundary. | The living spec describes current code instead of deferring the admission already present. |
| R5 | `direct-fix` | Fill `CODE_REVIEW-cloud-G09.md` item/checklist, deviation, design, and command-output fields after fresh verification. | The next review receives judgeable implementation-owned evidence with no `[fill]` or placeholder sections. |

## Analysis

### Files Read

- `agent-ops/rules/project/rules.md`
- `agent-ops/rules/common/rules-roadmap.md`
- `agent-ops/rules/common/rules-agent-spec.md`
- `agent-ops/rules/project/domain/edge/rules.md`
- `agent-ops/rules/project/domain/testing/rules.md`
- `agent-test/local/rules.md`
- `agent-test/local/edge-smoke.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`
- `agent-contract/inner/edge-config-runtime-refresh.md`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `agent-spec/runtime/edge-node-execution.md`
- `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/03+02_single_request_coordinator/complete.log`
- `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/07+04_workspace_catalog/complete.log`
- `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/plan_cloud_G08_0.log`
- `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/code_review_cloud_G09_0.log`
- `apps/edge/internal/node/registry.go`
- `apps/edge/internal/node/registry_test.go`
- `apps/edge/internal/node/store.go`
- `apps/edge/internal/node/store_test.go`
- `apps/edge/internal/service/service.go`
- `apps/edge/internal/service/single_request.go`
- `apps/edge/internal/service/single_request_test.go`
- `apps/edge/internal/service/single_request_types.go`
- `apps/edge/internal/service/single_request_types_test.go`
- `apps/edge/internal/service/single_request_workspace.go`
- `apps/edge/internal/openai/single_request_preset_binding.go`
- `apps/edge/internal/openai/anthropic_handler.go`
- `apps/edge/internal/openai/server.go`
- `packages/go/config/edge_types.go`
- `packages/go/config/load.go`

### SDD Criteria

- SDD: `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.md`; status `[승인됨]`, lock released.
- Milestone scope: `milestone-task=workspace-binding`.
- Acceptance Scenario S04 requires approved Mac workspace admission and rejection of unapproved ref, foreign Node/path, and escape candidates before provider/tool execution.
- Evidence Map S04 requires a workspace route/path/symlink admission table and fail-closed evidence. This packet supplies route/owner/generation admission and executor non-invocation; filesystem path/symlink enforcement remains the later tool-executor packet.

### Verification Context

- No separate verification handoff was supplied. Repository-native fallback came from the active plan, local test rules, Edge smoke profile, source/tests, and fresh reviewer commands.
- Current host: `/config/workspace/iop-s0`, Linux arm64, Go `1.26.2`; external services and credentials are not required.
- Preconditions: packet 03 and packet 07 each have one archived `complete.log`, confirmed at the exact paths above.
- Required deterministic evidence: named node/service race tests, focused package tests, full Edge regression, vet, spec search, formatting, and whitespace checks with cache bypass where applicable.
- External Verification Preflight: not applicable. This packet stops at Edge admission and does not implement Node executor/wire or actual Claude/Mac execution; those later packets own field smoke.
- Gap from the failed loop: the service command selected no tests, the registry reconnect case was sequential, and all implementation evidence fields were blank. Confidence is high because fresh output and direct source inspection agree.

### Test Coverage Gaps

- Public admission bypass: uncovered; add a nil runtime-dependency test proving executor non-invocation.
- Exact approved ref and no fallback/foreign owner: uncovered; add a table using real `NodeStore` and `Registry`.
- Pending/stale/reconnect/refresh immutability: uncovered; add synchronized race tests with a recording executor.
- Partial operation capability limits: uncovered; add read-only, write-only, list-only, and command-bound cases.
- Snapshot copy isolation: covered sequentially, but reconnect concurrency is missing.

### Symbol References

- No symbol is renamed or removed.
- `Service.StartSingleRequest` callers: `apps/edge/internal/openai/anthropic_handler.go:223` and `:258`; service tests call it directly.
- `ReadyOwnerSnapshot` callers: `apps/edge/internal/service/single_request_workspace.go:35` and `apps/edge/internal/node/registry_test.go:312,319,350`.

### Split Judgment

- Keep one atomic follow-up. Public admission, operation-aware projection, executor non-invocation, generation race evidence, and living-spec truth form one S04 correctness boundary.
- Subtask `08+03,07_workspace_admission` depends on packet 03 and packet 07. Both are satisfied by the exact archived `complete.log` paths listed above; there is no dependency wait.

### Scope Rationale

- Include only Edge registry/service admission tests, service binding validation, coordinator-test adaptation, living spec, and review evidence.
- Exclude protobuf, Node-private workspace config transport, filesystem/symlink containment, actual tool execution, provider stages, outer HTTP behavior, config schema changes, and roadmap mutation. Their source contracts remain unchanged.

### Final Routing

- `evaluation_mode=isolated-reassessment`; finalizer `finalize-task-policy.sh pair`.
- Build closures: scope/context/verification/evidence/ownership/decision all closed; scores `2/2/1/1/2` = `G08`; base `local-fit`, final basis `risk-boundary`, lane `cloud`, filename `PLAN-cloud-G08.md`.
- Review closures: all closed; scores `2/2/1/2/2` = `G09`; basis `official-review`, lane `cloud`, filename `CODE_REVIEW-cloud-G09.md`.
- `large_indivisible_context=false`; positive loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract`, `variant_product` (4).
- Recovery signals: `review_rework_count=1`, `evidence_integrity_failure=false`; no capability gap.

## Dependencies and Execution Order

1. Packet 03 completion: `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/03+02_single_request_coordinator/complete.log`.
2. Packet 07 completion: `agent-task/archive/2026/08/m-iop-owned-single-request-agent-execution/07+04_workspace_catalog/complete.log`.
3. Close R1-R2, add R3 tests, synchronize R4, then run fresh verification and complete R5 evidence.

## Implementation Checklist

- [ ] Remove the public zero-runtime admission bypass and keep coordinator-only unit tests on the internal helper while public service execution requires an exact configured store/registry owner.
- [ ] Make workspace capability limits operation-aware, preserve positive bounds for enabled operations, and apply preset minima without rejecting valid disabled-operation zeros.
- [ ] Add the complete admission matrix plus deterministic reconnect/refresh race coverage, proving frozen copies and executor non-invocation on every rejection.
- [ ] Synchronize the runtime living spec with implemented workspace admission while keeping Node executor/wire work explicitly deferred.
- [ ] Run exact dependency, formatting, focused race, package, full Edge, vet, documentation, and whitespace verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [FIX-1] Enforce universal operation-aware workspace admission

**Problem**

- `apps/edge/internal/service/service.go:85` bypasses admission when both runtime dependencies are nil and starts the executor.
- `apps/edge/internal/service/single_request_workspace.go:95` and `single_request_types.go:227` require all limits to be positive, contradicting the operation-specific catalog validation in `packages/go/config/load.go:629`.

**Solution**

Before (`apps/edge/internal/service/service.go:85`):

```go
if registry == nil && store == nil {
	return startSingleRequest(ctx, executor, req)
}
```

After:

```go
bound, err := bindSingleRequestWorkspace(req.Binding, store, registry)
if err != nil {
	return nil, err
}
```

Coordinator-only tests call `startSingleRequest` directly. Validate limit positivity only for operation ids that consume each limit, require command ids iff `command` is enabled, and retain zero for disabled-operation limits. Continue applying the lower preset output and stage-timeout bounds.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/service/service.go` — remove the public bypass and preserve the generation recheck.
- [ ] `apps/edge/internal/service/single_request_types.go` — validate cloned workspace limits against enabled operations.
- [ ] `apps/edge/internal/service/single_request_workspace.go` — compile operation-specific limits and command invariants.
- [ ] `apps/edge/internal/service/single_request_test.go` — move coordinator-only fixtures to the internal helper.
- [ ] `apps/edge/internal/service/single_request_workspace_test.go` — add bypass, partial-operation, effective-limit, and executor-call regressions.

**Test Strategy**

- Add `TestSingleRequestWorkspaceRejectsMissingRuntimeDependencies`, `TestSingleRequestWorkspaceOperationSpecificLimits`, and exact ready-owner admission cases with a recording executor. Assert rejected requests never call the executor and admitted bindings contain no root, executable, args, or environment surface.

**Verification**

- `go test -race ./apps/edge/internal/service -run 'TestSingleRequestWorkspace' -count=1`
- Expected: named admission tests execute, valid partial capabilities reach the executor once, and rejected cases reach it zero times.

### [FIX-2] Prove immutable admission across reconnect and refresh races

**Problem**

- `apps/edge/internal/node/registry_test.go:300` snapshots, unregisters, and reconnects sequentially, so `-race` proves no concurrent snapshot/reconnect property.
- No service test coordinates catalog refresh or generation replacement between snapshot and executor handoff.

**Solution**

Use barriers/channels rather than sleeps to overlap snapshot reads with unregister/register/ready transitions and runtime store refresh. Assert every observed snapshot is self-consistent, a stale generation is rejected before executor invocation, a successful executor receives one deep-copied binding, and neither refresh nor reconnect retargets it.

**Modified Files and Checklist**

- [ ] `apps/edge/internal/node/registry_test.go` — add a synchronized concurrent ready-owner snapshot/reconnect case.
- [ ] `apps/edge/internal/service/single_request_workspace_test.go` — add stale-generation, refresh-isolation, copy-isolation, foreign/missing/pending, and no-fallback table cases.

**Test Strategy**

- Extend `TestRegistryReadyOwnerSnapshot` with a deterministic concurrent subtest and make all service cases begin with `TestSingleRequestWorkspace` so the exact `-run` command cannot pass empty.

**Verification**

- `go test -race ./apps/edge/internal/node -run 'TestRegistryReadyOwnerSnapshot' -count=1`
- `go test -race ./apps/edge/internal/service -run 'TestSingleRequestWorkspace' -count=1`
- Expected: both commands run named race tests with no `[no tests to run]` marker and no race report.

### [FIX-3] Synchronize current-state documentation and review evidence

**Problem**

- `agent-spec/runtime/edge-node-execution.md:128` defers concrete Node/workspace admission despite this packet implementing exact catalog owner and ready-generation binding.
- The prior review artifact left every implementation-owned field blank and omitted the `service.go` write-boundary deviation.

**Solution**

Document workspace admission as implemented: opaque `workspace_ref` resolves only through the immutable configured catalog, freezes the exact ready Node generation plus closed ids/effective limits, rejects stale ownership without reselection, and exposes no raw root/template data. Defer only Node-private transport/executor/path enforcement. Fill the new review stub with actual decisions, deviations, and command output.

**Modified Files and Checklist**

- [ ] `agent-spec/runtime/edge-node-execution.md` — synchronize feature, scope, verification, and change record.
- [ ] `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/CODE_REVIEW-cloud-G09.md` — fill all implementation-owned evidence.

**Test Strategy**

- No separate documentation test file. Use deterministic search plus the service race tests as source evidence.

**Verification**

- `rg --sort path -n 'workspace_ref|connection generation|admission|reselect|defer' agent-spec/runtime/edge-node-execution.md`
- Expected: output explicitly describes current workspace admission/generation fencing/no reselection and limits deferral to executor/wire/path enforcement.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/edge/internal/service/service.go` | FIX-1 |
| `apps/edge/internal/service/single_request_types.go` | FIX-1 |
| `apps/edge/internal/service/single_request_workspace.go` | FIX-1 |
| `apps/edge/internal/service/single_request_test.go` | FIX-1 |
| `apps/edge/internal/service/single_request_workspace_test.go` | FIX-1, FIX-2 |
| `apps/edge/internal/node/registry_test.go` | FIX-2 |
| `agent-spec/runtime/edge-node-execution.md` | FIX-3 |
| `agent-task/m-iop-owned-single-request-agent-execution/08+03,07_workspace_admission/CODE_REVIEW-cloud-G09.md` | FIX-3 |

## Final Verification

1. `test -f agent-task/m-iop-owned-single-request-agent-execution/03+02_single_request_coordinator/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/03+02_single_request_coordinator/complete.log' | wc -l)" -eq 1`
2. `test -f agent-task/m-iop-owned-single-request-agent-execution/07+04_workspace_catalog/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/07+04_workspace_catalog/complete.log' | wc -l)" -eq 1`
3. `test -z "$(gofmt -l apps/edge/internal/service/service.go apps/edge/internal/service/single_request_types.go apps/edge/internal/service/single_request_workspace.go apps/edge/internal/service/single_request_test.go apps/edge/internal/service/single_request_workspace_test.go apps/edge/internal/node/registry_test.go)"`
4. `go test -race ./apps/edge/internal/node -run 'TestRegistryReadyOwnerSnapshot' -count=1`
5. `go test -race ./apps/edge/internal/service -run 'TestSingleRequestWorkspace' -count=1`
6. `go test ./apps/edge/internal/node ./apps/edge/internal/service -count=1`
7. `go test ./apps/edge/... -count=1`
8. `go vet ./apps/edge/...`
9. `rg --sort path -n 'workspace_ref|connection generation|admission|reselect|defer' agent-spec/runtime/edge-node-execution.md`
10. `git diff --check`

Expected: both predecessors resolve uniquely; formatting is clean; named node/service race tests execute; focused and full Edge packages pass uncached; vet and whitespace are clean; the living spec states current admission and narrow deferral. Cached Go test output is not acceptable.

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