<!-- task=m-iop-owned-single-request-agent-execution/11+10_workspace_command plan=2 tag=REVIEW_API milestone-task=tool-executor -->

# Workspace Command Terminal Contract Repair

## For the Implementing Agent

Resolve Required R1 and R2 exactly within this packet. Keep the command executor's fixed-template and security boundaries unchanged, run every verification command with fresh results, and fill `CODE_REVIEW-cloud-G08.md`. Do not start or finalize the review loop, create control-plane artifacts, or broaden this packet into provider execution, cleanup, or credentialed Claude smoke work.

## Background

The command executor implementation passes its Node-local verification, but the real Node terminal response shapes do not satisfy the connected Edge response validator. The same review also found that context cancellation is tested only before process launch, leaving the required live process-group cancellation path unproven. This follow-up repairs the shared private-wire terminal contract and adds the missing active cancellation evidence without redesigning the executor.

## Archive Evidence Snapshot

- The closed plan/review pair is preserved at `agent-task/m-iop-owned-single-request-agent-execution/11+10_workspace_command/plan_cloud_G09_1.log` and `agent-task/m-iop-owned-single-request-agent-execution/11+10_workspace_command/code_review_cloud_G10_1.log`.
- The review verdict is `FAIL` with 2 Required findings and 0 Suggested findings: R1 covers incompatible Node/Edge terminal response validation, and R2 covers missing live context-cancel process-group evidence.
- Fresh reviewer runs passed the focused workspace and Node race tests, selected package regression, full Node test profile, Node vet, Edge workspace-wire tests, Darwin workspace cross-compile, contract search, and `git diff --check`; these passes do not exercise the two failed semantic preconditions.

## Finding Resolution Map

| Finding | Disposition | Direct-Fix Targets | Changed Precondition / Proof |
|---------|-------------|--------------------|------------------------------|
| Required R1 | `direct-fix` | `packages/go/workspaceprotocol/terminal.go`, `packages/go/workspaceprotocol/terminal_test.go`, `apps/node/internal/node/workspace_handler.go`, `apps/node/internal/node/workspace_handler_test.go`, `apps/edge/internal/service/workspace_wire.go`, `apps/edge/internal/service/workspace_wire_test.go`, `agent-contract/inner/edge-node-runtime-wire.md` | Replace independently defined producer/consumer terminal rules with one closed shared mapping used by both production paths; valid typed non-success outcomes then survive the wire while contradictory pairs and raw text still fail closed. |
| Required R2 | `direct-fix` | `apps/node/internal/workspace/command_executor_test.go` | Replace the pre-cancel-only assertion with a deterministic live blocking command and descendant-process cancellation assertion, changing the missing active-process precondition. |

`ownership_closed=true`: every inherited Required finding has one repository-owned direct fix in this packet.

## Analysis

### Files Read

- `AGENTS.md`
- `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/node/rules.md`
- `agent-ops/rules/project/domain/platform-common/rules.md`
- `agent-ops/rules/project/domain/testing/rules.md`
- `agent-ops/rules/project/domain/edge/rules.md`
- `agent-test/local/rules.md`
- `agent-test/local/node-smoke.md`
- `agent-test/local/edge-smoke.md`
- `agent-contract/index.md`
- `agent-spec/index.md`
- `agent-roadmap/current.md`
- `agent-roadmap/phase/knowledge-tool-optimization-extension/PHASE.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-task/m-iop-owned-single-request-agent-execution/11+10_workspace_command/plan_cloud_G09_1.log`
- `agent-task/m-iop-owned-single-request-agent-execution/11+10_workspace_command/code_review_cloud_G10_1.log`
- `apps/node/internal/workspace/command_executor.go`
- `apps/node/internal/workspace/command_process_unix.go`
- `apps/node/internal/workspace/command_process_other.go`
- `apps/node/internal/workspace/command_executor_test.go`
- `apps/node/internal/workspace/runtime.go`
- `apps/node/internal/node/workspace_handler.go`
- `apps/node/internal/node/workspace_handler_test.go`
- `apps/node/internal/transport/session.go`
- `apps/edge/internal/service/workspace_wire.go`
- `apps/edge/internal/service/workspace_wire_test.go`
- `agent-contract/inner/edge-node-runtime-wire.md`
- `agent-contract/inner/execution-runtime.md`
- `agent-spec/runtime/edge-node-execution.md`

### SDD Criteria

- The approved, unlocked Milestone retains `milestone-task=tool-executor`; SDD S05 requires command success, failure, timeout, and cancellation to return consistent typed outcomes across the private execution boundary.
- The SDD Evidence Map requires Node tool operation/process/output integration and synchronized Edge-Node contract evidence. Passing isolated Node tests is insufficient when the connected Edge validator rejects Node's legitimate terminal forms.
- The fixed-template command, admitted cwd, minimal environment, bounded output, and process-group security constraints remain acceptance conditions and must not regress.

### Verification Context

- No external verification handoff was supplied. Repository-native local rules, the Node/Edge smoke profiles, source tests, and fresh read-only reviewer runs provide the verification context.
- The current host is Linux/arm64 with Go 1.26.2. Darwin compatibility is covered by compile-only checks for the changed Node/Edge packages; no credential, device, remote runner, or user-controlled action is required.
- Fresh reviewer evidence passed: workspace race tests (`ok`, 5.541s), Node handler race tests (`ok`, 2.249s), selected package regression, full `./apps/node/...`, Node vet, Edge workspace-wire tests, Darwin workspace compile, deterministic contract search, and whitespace validation.
- The Milestone's credentialed Mac/Claude full-cycle belongs to the separate `claude-smoke` packet and is not a completion condition for this isolated repair.

### State and Concurrency Findings

- `applyToolFailure` returns bounded stdout/stderr, exit code, truncation, and duration on typed command failure, timeout, or cancellation, but the Edge validator discards the entire response before those fields can be consumed.
- `OnWorkspaceCancel` returns `CANCELLED/CANCELLED` with a stable generic message for both initial and duplicate cancellation, while Edge currently admits only `CANCELLED/UNSPECIFIED` with an empty message.
- Explicit cancellation already starts a blocking command before signaling it. Context cancellation does not: the current test cancels the context before `ExecuteCommand`, so process-group kill and reap are not reached.

### Test Coverage Gaps

- No test proves that every Node-produced canonical tool/cancel terminal pair is accepted by the Edge production validator while contradictory status/code/message combinations remain rejected without raw-text leakage.
- No test cancels a live command through `context.Context` after both the command and descendant process have started, then proves typed cancellation and complete process-group termination.

### Symbol References

- No symbol is renamed or removed by this plan. The new shared terminal helper must be referenced only by the Node workspace handler and Edge workspace-wire validator, with exhaustive helper tests covering closed enum combinations.

### Split Judgment

- Keep one follow-up packet. R1 and R2 are both terminal-outcome correctness for the same command lifecycle, share the Node handler/executor verification surface, and must pass together before S05 evidence is credible.
- Packet 10 completion remains satisfied by exactly one archived `complete.log`; no additional split predecessor is introduced.

### Scope Rationale

- Include a small cross-component terminal-contract package because no existing package owns canonical workspace response triples and Node/Edge `internal` import boundaries prevent either side from importing the other.
- Include Node response mapping, Edge response validation, their focused tests, the active context-cancel process test, and the private wire contract.
- Exclude command template/catalog behavior, file operations, cleanup, provider execution/cancellation, coordinator/public projection, artifact lifecycle, and credentialed Claude smoke.

### Final Routing

- `evaluation_mode=isolated-reassessment`; `review_rework_count=1`; `evidence_integrity_failure=false`; build and review closures are complete.
- Build scores are scope/state/blast/evidence/verification `2/2/1/2/1 = G08`; positive loop risks are `temporal_state`, `concurrent_consistency`, and `boundary_contract` (3); `large_indivisible_context=false`.
- Finalizer result: build base/final basis `local-fit`, lane `local`, filename `PLAN-local-G08.md`, catalog route `worker/local/G08`; no risk or recovery boundary matched.
- Review scores are `2/2/1/2/1 = G08`; route basis `official-review`, lane `cloud`, filename `CODE_REVIEW-cloud-G08.md`, catalog route `review/cloud/G08`.

## Dependencies and Execution Order

1. Preserve the satisfied packet 10 dependency and current fixed-template executor behavior.
2. Define the closed shared terminal triples, then make Node production and Edge validation consume that authority.
3. Add cross-boundary terminal tests and live context-cancel group evidence before synchronizing the contract text.

## Implementation Checklist

- [ ] Define and exhaustively test one closed shared mapping for canonical workspace tool and cancel status/error-code/stable-message triples.
- [ ] Make Node tool and cancel response construction use the shared mapping without copying raw runtime or OS errors.
- [ ] Make Edge accept every canonical typed terminal response, retain bounded typed result fields, and reject identity mismatches, contradictory triples, and non-canonical raw text.
- [ ] Add a deterministic live context-cancel test that starts the blocking command and descendant before cancellation and proves the full process group is gone/reaped.
- [ ] Synchronize the private Edge-Node wire contract with the canonical triples and raw-error fence.
- [ ] Run dependency, focused race, full Node/Edge, vet, Darwin compile, contract-search, and whitespace verification with fresh results.
- [ ] Fill every implementation-owned section in `CODE_REVIEW-cloud-G08.md` with actual changes, decisions, deviations, and command output.

## Implementation Plan

### [REVIEW_API-1] Align canonical workspace terminal responses

**Problem**

- `apps/node/internal/node/workspace_handler.go` emits typed non-success command outcomes and `CANCELLED/CANCELLED` cancel acknowledgements with stable messages.
- `apps/edge/internal/service/workspace_wire.go` admits only successful tool responses and a different cancel pair, so real command failure/timeout/cancel evidence is rejected as a malformed response.
- Node and Edge cannot import each other's `internal` packages, and there is no current shared owner for the allowed response triples.

**Solution**

Add a small `packages/go/workspaceprotocol` authority that returns the exact stable message for an allowed `(response family, status, error_code)` pair and rejects every other pair. Node response construction uses it instead of a local message switch; Edge validates the echoed identities and requires the exact canonical triple before returning the response. Successful results remain `SUCCESS/UNSPECIFIED/empty`; typed failure, timeout, unsupported, and cancellation forms remain closed and preserve their bounded response fields. An unrecognized pair or message mismatch returns the existing stable internal wire error without interpolating Node text.

Before:

```go
// Node and Edge independently define incompatible terminal rules.
response.Status, response.ErrorCode = result.Status, result.Code
if response.Status != SUCCESS || response.ErrorCode != UNSPECIFIED || response.Error != "" {
    return nil, errWorkspaceWireResponse
}
```

After:

```go
message, ok := workspaceprotocol.ToolTerminal(status, code)
// Node emits message; Edge requires the same message and ok=true.
```

**Modified Files and Checklist**

- [ ] `packages/go/workspaceprotocol/terminal.go` — own canonical tool/cancel terminal triples and stable generic messages.
- [ ] `packages/go/workspaceprotocol/terminal_test.go` — exhaust allowed triples and reject contradictory/unknown combinations.
- [ ] `apps/node/internal/node/workspace_handler.go` — construct tool/cancel terminal fields from the shared authority and preserve bounded result data.
- [ ] `apps/node/internal/node/workspace_handler_test.go` — assert success, non-zero, timeout, cancellation, duplicate cancel, and not-found responses are canonical and raw-free.
- [ ] `apps/edge/internal/service/workspace_wire.go` — validate canonical tool/cancel triples while retaining typed non-success responses.
- [ ] `apps/edge/internal/service/workspace_wire_test.go` — prove every Node-shaped canonical response passes and contradictory/raw/mismatched responses fail closed.
- [ ] `agent-contract/inner/edge-node-runtime-wire.md` — specify the shared terminal-pair authority, stable messages, preserved bounded fields, and raw-text rejection.

**Test Strategy**

- Use exhaustive shared-helper cases and production Node/Edge package tests. Assert that canonical success, non-zero/internal failure, invalid request, not found, unsupported/not ready, timeout, command cancellation, cancel acknowledgement, and duplicate cancel survive with exact identities and typed fields. Mutate one status, code, or message at a time and assert the Edge returns only `errWorkspaceWireResponse` without the raw sentinel.

### [REVIEW_API-2] Prove live context cancellation owns the process group

**Problem**

- `TestCommandExecutorTimeoutAndContextCancel` calls `cancel()` before invoking `ExecuteCommand`, so no target or descendant exists when cancellation is observed.
- The original acceptance requires context cancellation, timeout, and explicit cancellation to share one process-group termination and reap path.

**Solution**

Run the existing blocking helper in a goroutine with a live context, wait for its start sentinel and descendant pid evidence, cancel the context, then require one `CANCELLED/CANCELLED` result with `ExitCode=-1`. Poll the descendant/process-group identity using the existing bounded test helpers and fail if any member remains alive after the result. Keep the pre-start cancellation assertion as a separate fast-path case if useful, but do not treat it as active-process evidence.

**Modified Files and Checklist**

- [ ] `apps/node/internal/workspace/command_executor_test.go` — add deterministic active context-cancel and descendant process-group termination/reap assertions.

**Test Strategy**

- Reuse the exact configured Go helper binary, fixed `block` behavior, start sentinel, and child pid mechanisms. Synchronize on observable start state instead of sleeps, cancel once, bound every wait, and assert no cross-request process is signaled.

## Modified Files Summary

| File | Item |
|------|------|
| `packages/go/workspaceprotocol/terminal.go` | REVIEW_API-1 |
| `packages/go/workspaceprotocol/terminal_test.go` | REVIEW_API-1 |
| `apps/node/internal/node/workspace_handler.go` | REVIEW_API-1 |
| `apps/node/internal/node/workspace_handler_test.go` | REVIEW_API-1 |
| `apps/edge/internal/service/workspace_wire.go` | REVIEW_API-1 |
| `apps/edge/internal/service/workspace_wire_test.go` | REVIEW_API-1 |
| `agent-contract/inner/edge-node-runtime-wire.md` | REVIEW_API-1 |
| `apps/node/internal/workspace/command_executor_test.go` | REVIEW_API-2 |
| `agent-task/m-iop-owned-single-request-agent-execution/11+10_workspace_command/CODE_REVIEW-cloud-G08.md` | REVIEW_API-1, REVIEW_API-2 |

## Final Verification

1. `test -f agent-task/m-iop-owned-single-request-agent-execution/10+09_workspace_files/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/10+09_workspace_files/complete.log' | wc -l)" -eq 1`
2. `go test -race ./packages/go/workspaceprotocol ./apps/node/internal/node ./apps/edge/internal/service -run 'Test(WorkspaceTerminal|NodeWorkspace(Command|Cancel)|WorkspaceWire)' -count=1`
3. `go test -race ./apps/node/internal/workspace -run 'Test(CommandExecutor|WorkspaceCommandHelperProcess)' -count=1`
4. `go test -count=1 ./packages/go/workspaceprotocol ./apps/node/... ./apps/edge/...`
5. `go vet ./packages/go/workspaceprotocol ./apps/node/... ./apps/edge/...`
6. `GOOS=darwin GOARCH=arm64 go test -c -o /tmp/iop-node-workspace-command-darwin.test ./apps/node/internal/workspace && GOOS=darwin GOARCH=arm64 go test -c -o /tmp/iop-node-workspace-handler-darwin.test ./apps/node/internal/node && GOOS=darwin GOARCH=arm64 go test -c -o /tmp/iop-edge-workspace-wire-darwin.test ./apps/edge/internal/service`
7. `rg --sort path -n 'Workspace\*Response|canonical|terminal|error-code|non-zero|timeout|cancel|stdout|stderr' agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md packages/go/workspaceprotocol/terminal.go`
8. `git diff --check`

Expected: packet 10 is uniquely complete; shared terminal tests, Node/Edge production tests, live process-group cancellation, full local profiles, vet, and Darwin compilation pass uncached; canonical typed non-success responses retain bounded fields across Edge validation; contradictory or raw terminal forms remain fenced; documentation search and whitespace checks succeed.

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