15 KiB
Request-owned Workspace Cleanup
For the Implementing Agent
Do not start until packet 12 has complete.log. Implement only request-owned process/artifact cleanup in the listed files, preserve user results, run every verification command, and fill CODE_REVIEW-cloud-G10.md. Do not broaden cleanup into rollback or own official review state.
Background
The internal loop can open a Node workspace and run processes, but success/error/cancel paths do not yet converge on one cleanup owner. This packet adds exactly-once coordinator cleanup and Node reclamation limited to active command groups and registered Node-owned artifacts under .iop/job/<request_id>.
Archive Evidence Snapshot
- The first-pass pair is preserved at
agent-task/m-iop-owned-single-request-agent-execution/13+12_workspace_cleanup/plan_cloud_G09_0.logandagent-task/m-iop-owned-single-request-agent-execution/13+12_workspace_cleanup/code_review_cloud_G10_0.log; it contains no implementation evidence or review verdict. - Self-review found that blind
os.Root.RemoveAllcan cross a mounted subtree and cannot distinguish Node-owned artifacts from injected/unowned entries. Plan 1 uses the immutablerequest_id, an in-memory ownership inventory, no-follow descriptor traversal, and deepest-first non-recursive removal that fails closed on any ownership or filesystem-boundary mismatch.
Analysis
Files Read
AGENTS.mdagent-ops/rules/project/rules.mdagent-ops/rules/common/rules-roadmap.mdagent-ops/rules/common/rules-agent-spec.mdagent-ops/rules/project/domain/edge/rules.mdagent-ops/rules/project/domain/node/rules.mdagent-test/local/rules.mdagent-test/local/edge-smoke.mdagent-test/local/node-smoke.mdagent-roadmap/phase/knowledge-tool-optimization-extension/milestones/iop-owned-single-request-agent-execution.mdagent-roadmap/sdd/knowledge-tool-optimization-extension/iop-owned-single-request-agent-execution/SDD.mdagent-task/m-iop-owned-single-request-agent-execution/03+02_single_request_coordinator/PLAN-local-G07.mdapps/edge/internal/service/service.goapps/node/internal/node/cancel_handler.goapps/node/internal/node/node.goagent-contract/inner/edge-node-runtime-wire.mdagent-spec/runtime/edge-node-execution.md
SDD Criteria
- S07 requires success, error, and cancel to remove request-owned processes and registered artifacts beneath the exact
.iop/job/<request_id>namespace while preserving requested workspace results. finalizingwaits for cleanup before committing the final terminal. Cleanup never rolls back user files.- S11 caller disconnect must cancel provider/tool processes and perform bounded cleanup without another external request.
Verification Context
- Packet 09 already defines typed cleanup; packets 10/11 own request contexts and process groups; packet 12 owns the coordinator loop.
- Race tests use temporary roots and helper processes. No external Mac runner is required for logic, while Darwin compile remains a final gate.
- Existing route-01 cleanup is caller-artifact state and is not reusable as Node filesystem ownership.
State and Concurrency Findings
- Success, executor error, request timeout, caller cancel, terminal write failure, and duplicate callbacks may all race to cleanup; only one wire cleanup is sent and all callers observe its result.
- Node cleanup cancels every active process for exactly one request, waits boundedly, removes only registered Node-owned internal artifacts without recursive traversal, closes the request context, and caches a bounded idempotent result.
- Cleanup failure converts a pending success to failure; existing failure/cancel remains primary while safe cleanup failure is internal evidence.
Test Coverage Gaps
- No Node operation removes request job artifacts or all request process groups.
- Coordinator finalization does not wait for workspace cleanup or prove exactly-once across terminal races.
Symbol References
- Add an optional lifecycle interface to packet 12's internal tool executor; do not break fakes that never opened a workspace.
- Provider
CancelRunand route-01 cleanup remain separate.
Split Judgment
- Node cleanup and coordinator exactly-once finalization form one end-to-end ownership invariant and must be reviewed together.
- Observation is packet 14 because it can instrument the stable lifecycle without changing cleanup outcomes.
Scope Rationale
- Include job namespace, process cancellation/wait, idempotence, coordinator deferral, user-file preservation, tests, wire contract/spec.
- Exclude general rollback, git reset, arbitrary directory cleanup, metrics/logging implementation, and real provider smoke.
Final Routing
evaluation_mode=isolated-reassessment;review_rework_count=0;evidence_integrity_failure=false; build closures true, scores 2/2/2/1/2 = G09.- Finalizer route
grade-boundary, lanecloud, filenamePLAN-cloud-G09.md; riskstemporal_state,concurrent_consistency,boundary_contract,variant_product(4). - Review scores 2/2/2/2/2 = G10; official filename
CODE_REVIEW-cloud-G10.md; no capability/recovery gap.
Dependencies and Execution Order
- Require packet 12 completion.
- Implement Node cleanup and race tests.
- Add optional Edge lifecycle cleanup and terminal ordering.
- Synchronize contract/spec only after end-to-end verification.
Implementation Checklist
- Create and validate only
.iop/job/<request_id>from the immutable coordinator identity, inventory every Node-owned artifact, and preserve every user or unowned result. - Cancel/wait all process groups and remove only inventoried artifacts plus empty owned directories exactly once per request with bounded concurrent/idempotent result ownership.
- Make coordinator success/error/cancel/disconnect paths converge on one typed cleanup before terminal commit, with fail-closed success handling.
- Prove cleanup races, symlink/mount/unowned-entry refusal, user-result preservation, cross-request isolation, failure handling, and synchronize cleanup contract/spec claims.
- Run dependency, focused race, package, vet, Darwin compile, documentation, and whitespace verification.
- Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
Implementation Plan
[API-1] Reclaim only Node request-owned state
Problem
- Packet 11 tracks commands per immutable request identity but has no all-process cleanup or artifact namespace reclamation.
apps/node/internal/node/cancel_handler.go:11owns provider run cancel only.
Solution
Add cleanup.go to workspace runtime. On request open, derive .iop/job/<validated-request-id> only from packet 10's immutable coordinator identity. Create each internal directory component through a no-follow descriptor helper, verify it remains on the admitted filesystem, and register the exact directory identity in the request's in-memory ownership inventory. Every later Node-owned internal artifact must be created through the same helper and registered by relative path, type, and stable file identity; caller file tools still cannot access .iop.
Cleanup atomically elects one owner, cancels all process groups for that request, and waits up to the admitted cleanup bound. It then compares a no-follow enumeration of the exact job tree with the inventory, rejects symlinks, special files, mount/device changes, identity replacements, and unregistered entries, and removes registered files followed by deepest-first empty directories with descriptor-relative non-recursive unlink/rmdir. Never call Root.RemoveAll, never follow an entry during validation/removal, and never widen the path after an error. A mismatch returns typed cleanup failure and preserves the suspect subtree for operator inspection. Concurrent/duplicate callers wait for and receive the same typed result. Retain completed results in a bounded cache; eviction may repeat an idempotent missing-directory check but never broadens scope. Runtime close invokes the same primitive for active requests.
Modified Files and Checklist
apps/node/internal/workspace/runtime.go— request lifecycle state, immutable request artifact inventory, and bounded completed-cleanup cache.apps/node/internal/workspace/cleanup.go— process cancel/wait, inventory validation, deepest-first non-recursive removal, and idempotent result ownership.apps/node/internal/workspace/cleanup_path_unix.go— Darwin/Linux descriptor-relative no-follow mkdir/enumerate/identity/unlink/rmdir primitives.apps/node/internal/workspace/cleanup_path_other.go— explicit unsupported fallback outside Darwin/Linux.apps/node/internal/workspace/cleanup_test.go— success/error/cancel races, duplicate calls, active child, timeout, symlink/mount/identity replacement/unowned entry, user-result/cross-request preservation, and invalid id.
Test Strategy
- Create user results beside
.iop, two requests, nested artifacts only through the internal ownership helper, and helper-process descendants; race cleanup/cancel/close and assert only the inventoried target paths disappear. Inject a sibling job, unregistered file, symlink, replaced inode, special file, and mount/substitute where supported; assert cleanup fails closed without deleting the suspect, user, or sibling content. Skip only the privileged mount fixture when unavailable while retaining deterministic filesystem-device mismatch coverage.
Verification
go test -race ./apps/node/internal/workspace -run 'TestWorkspaceCleanup' -count=1- Expected: exactly one cleanup result owns all registered target resources; suspect/unowned entries fail closed; no user/foreign request file, mounted content, or process is touched.
[API-2] Complete typed cleanup handling and coordinator finalization
Problem
- Packet 10 leaves
OnWorkspaceCleanupunsupported. - Packet 03's planned
finalizingstate waits only for endpoint acknowledgement, not workspace cleanup.
Solution
Before (packet 03 state contract):
successful candidate -> finalizing -> endpoint acknowledgement -> completed
After:
successful candidate -> finalizing -> exactly-once workspace cleanup
-> endpoint acknowledgement -> completed
Implement Node cleanup mapping. Add a separate optional SingleRequestWorkspaceLifecycle interface implemented by packet 12's tool loop (CleanupWorkspace). The coordinator invokes it once on every terminal/cancel path if the workspace was opened and waits within the frozen request deadline. Success plus cleanup failure becomes failed; existing failure/cancel keeps its category while recording only a safe cleanup code. Caller disconnect still cancels and awaits cleanup even when no response can be written.
Modified Files and Checklist
apps/node/internal/node/workspace_handler.go— map typed cleanup request/result without raw fields.apps/node/internal/node/workspace_handler_test.go— cleanup identity/status/idempotence and missing-runtime mapping.apps/edge/internal/service/single_request_tool_loop.go— implement optional workspace lifecycle cleanup using packet 09 wire.apps/edge/internal/service/single_request.go— exactly-once cleanup gate before terminal acknowledgement/return.apps/edge/internal/service/single_request_cleanup_test.go— success/error/cancel/write-failure races, one wire call, cleanup failure, unopened workspace.agent-contract/inner/edge-node-runtime-wire.md— define cleanup scope/idempotence/failure and preservation.agent-spec/runtime/edge-node-execution.md— synchronize finalizing order and named evidence.
Test Strategy
- Use counting lifecycle fakes plus a net-pipe Node cleanup handler. Race terminal candidates/cancel and assert one cleanup, no early completed state, and correct terminal category.
Verification
go test -race ./apps/node/internal/node ./apps/edge/internal/service -run 'Test(NodeWorkspaceCleanup|SingleRequestCleanup)' -count=1rg --sort path -n 'cleanup|\.iop/job|user result|finalizing|exactly' agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.md- Expected: Node and coordinator share the exact scoped cleanup/finalization contract.
Modified Files Summary
| File | Item |
|---|---|
apps/node/internal/workspace/runtime.go |
API-1 |
apps/node/internal/workspace/cleanup.go |
API-1 |
apps/node/internal/workspace/cleanup_path_unix.go |
API-1 |
apps/node/internal/workspace/cleanup_path_other.go |
API-1 |
apps/node/internal/workspace/cleanup_test.go |
API-1 |
apps/node/internal/node/workspace_handler.go |
API-2 |
apps/node/internal/node/workspace_handler_test.go |
API-2 |
apps/edge/internal/service/single_request_tool_loop.go |
API-2 |
apps/edge/internal/service/single_request.go |
API-2 |
apps/edge/internal/service/single_request_cleanup_test.go |
API-2 |
agent-contract/inner/edge-node-runtime-wire.md |
API-2 |
agent-spec/runtime/edge-node-execution.md |
API-2 |
agent-task/m-iop-owned-single-request-agent-execution/13+12_workspace_cleanup/CODE_REVIEW-cloud-G10.md |
API-1, API-2 |
Final Verification
test -f agent-task/m-iop-owned-single-request-agent-execution/12+05,06,08,11_internal_tool_loop/complete.log || test "$(compgen -G 'agent-task/archive/*/*/m-iop-owned-single-request-agent-execution/12+05,06,08,11_internal_tool_loop/complete.log' | wc -l)" -eq 1go test -race ./apps/node/internal/workspace -run 'TestWorkspaceCleanup' -count=1go test -race ./apps/node/internal/node ./apps/edge/internal/service -run 'Test(NodeWorkspaceCleanup|SingleRequestCleanup)' -count=1go test ./apps/node/internal/workspace ./apps/node/internal/node ./apps/edge/internal/service -count=1go vet ./apps/node/internal/workspace ./apps/node/internal/node ./apps/edge/internal/serviceGOOS=darwin GOARCH=arm64 go test -c -o /tmp/iop-workspace-cleanup-darwin.test ./apps/node/internal/workspacerg --sort path -n 'cleanup|request_id|\.iop/job|inventory|no-follow|user result|finalizing|exactly' agent-contract/inner/edge-node-runtime-wire.md agent-spec/runtime/edge-node-execution.mdgit diff --check
Expected: packet 12 is uniquely complete; cleanup is scoped to immutable request identity and inventoried Node artifacts, refuses symlink/mount/unowned replacement, is bounded and race-free, and precedes terminal completion; Darwin compile and package regressions pass. Cached tests are not acceptable.
After completing all code changes, fill implementation-owned sections in CODE_REVIEW-*-G??.md.