> For the complete documentation index, see [llms.txt](https://faction-os.gitbook.io/faction-os-docs/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://faction-os.gitbook.io/faction-os-docs/.spec_system/archive/sessions/phase19-session12-terminal-and-containers/code-review.md). # Code Review and Repair Report Session ID: phase19-session12-terminal-and-containers Reviewed: 2026-06-26 Scope: All uncommitted implementation, test, documentation, package, and spec changes for Phase 19 Session 12 terminal and container runtime support. Result: RESOLVED Package: mixed monorepo scope; no single active package ## Review Surface Reviewed 47 uncommitted files: * `.spec_system/specs/phase19-session12-terminal-and-containers/implementation-notes.md` * `.spec_system/specs/phase19-session12-terminal-and-containers/spec.md` * `.spec_system/specs/phase19-session12-terminal-and-containers/tasks.md` * `.spec_system/state.json` * `apps/server/README_server.md` * `apps/server/package.json` * `apps/server/src/lib/isolationDiagnostics.ts` * `apps/server/src/lib/terminalContainerValidation.ts` * `apps/server/src/managers/containerRuntimeManager.ts` * `apps/server/src/managers/executorRegistry.ts` * `apps/server/src/managers/orchestrationCommandCenter.ts` * `apps/server/src/managers/terminalSessionManager.ts` * `apps/server/src/routes/commandCenter.ts` * `apps/server/src/routes/diagnostics.ts` * `apps/server/src/routes/terminalContainers.ts` * `apps/server/src/server.ts` * `apps/server/tests/containerRuntimeManager.test.ts` * `apps/server/tests/executorRegistry.test.ts` * `apps/server/tests/isolationDiagnostics.test.ts` * `apps/server/tests/terminalContainerRoutes.test.ts` * `apps/server/tests/terminalSessionManager.test.ts` * `apps/server/tests/websocket.test.ts` * `apps/web/README_web.md` * `apps/web/src/components/orchestration/CommandCenterPanes.tsx` * `apps/web/src/components/orchestration/ExecutorFamiliesPanel.tsx` * `apps/web/src/components/orchestration/IsolationCapabilityPanel.tsx` * `apps/web/src/components/orchestration/OrchestrationShell.tsx` * `apps/web/src/components/orchestration/OrchestrationTabs.tsx` * `apps/web/src/components/orchestration/TerminalRuntimeDrawer.tsx` * `apps/web/src/lib/commandCenterUi.ts` * `apps/web/src/lib/orchestrationApi.ts` * `apps/web/src/lib/terminalContainers.ts` * `apps/web/src/store/useGameStore.ts` * `apps/web/tests/IsolationCapabilityPanel.test.tsx` * `apps/web/tests/OrchestrationPanel.test.tsx` * `apps/web/tests/TerminalRuntimeDrawer.test.tsx` * `apps/web/tests/commandCenterStore.test.ts` * `apps/web/tests/orchestrationApi.test.ts` * `apps/web/tests/terminalContainers.test.ts` * `docs/api/README_api.md` * `docs/isolation-and-sandbox.md` * `package-lock.json` * `package.json` * `packages/protocol/src/isolation.ts` * `packages/protocol/src/orchestrationCommandCenter.ts` * `packages/protocol/tests/isolation.test.ts` * `packages/protocol/tests/orchestrationCommandCenter.test.ts` ## Findings ### Critical None. ### High None. ### Medium 1. Fixed: Direct terminal executions could remain stuck in `executing`. * Files: `apps/server/src/routes/terminalContainers.ts:78`, `apps/server/src/routes/terminalContainers.ts:263`, `apps/server/src/managers/terminalSessionManager.ts:215` * Problem: `POST /command-center/terminal/run` saved the initial command-center execution immediately. When a PTY command was still running at response time, the route had no follow-up path to save the final execution state after the asynchronous exit event. A long-running direct terminal run could therefore remain visible as `executing` even after the terminal session completed. * Repair: Added `TerminalSessionManager.waitForCompletion()` and route-level completion scheduling that re-saves the execution after the PTY exits, using the execution id without reusing the original route idempotency key. * Coverage: Added a delayed PTY route regression proving the response starts as `executing` and later updates the execution detail to `executed` with a safe terminal attachment. ### Low 1. Fixed: Web isolated-spawn response validation accepted raw enum-like strings. * Files: `apps/web/src/lib/orchestrationApi.ts:92`, `apps/web/src/lib/orchestrationApi.ts:2698`, `apps/web/tests/orchestrationApi.test.ts` * Problem: `failureCode` and `unavailableReason` in isolated spawn run responses were checked only as strings. That bypassed the protocol enum parsers and could allow malformed server data into the store. * Repair: Reused `parseCommandCenterErrorCode()` and `parseCommandCenterUnavailableReason()` in the client response guard. * Coverage: Added a malformed isolated-spawn run response test that now fails with `bad_response`. 2. Fixed: The optional `node-pty` native install script was not allowlisted. * File: `package.json:107` * Problem: Session 12 introduces package-managed PTY support, but the root `allowScripts` map did not include `node-pty@1.1.0`. Installs that enforce `allowScripts` could skip the native build step and leave the package-managed PTY unavailable. * Repair: Added `"node-pty@1.1.0": true` to the root `allowScripts` map. * Coverage: Verified package metadata and full workspace checks. `package-lock.json` does not mirror the root `allowScripts` map, so no lockfile update was required for this repair. ## Assumptions * The direct terminal and container REST routes are local operator command-center controls delivered by this session. Guarded-action registry integration remains the separate approval path for executor-approved actions. * Terminal and container details should expose bounded, protocol-shaped summaries and attachments, not raw process payloads or unbounded output. * `package-lock.json` package metadata does not store the root `allowScripts` map, so the package manager allowlist repair is correctly limited to `package.json`. ## Behavior Changes * Direct terminal execution records now transition from `executing` to a final `executed`, `failed`, or `unavailable` state after asynchronous PTY completion and emit the normal `command_center_execution_update` event. * The web API client now rejects malformed isolated-spawn `failureCode` and `unavailableReason` fields as bad responses. * `npm install` can run the `node-pty@1.1.0` install script when the project allowlist is enforced. ## Security and Privacy Review * Terminal output remains bounded by the terminal session output cap and is returned through protocol-shaped session snapshots. * Execution details expose safe terminal/container attachments rather than raw PTY or container payloads. * New client response guards reject malformed runtime enum fields before they reach application state. * Container runtime probing and isolated spawn responses preserve local-only posture and cleanup labels without exposing host secrets. * Secret scan passed across tracked text files. ## Verification Focused repair checks: * PASS: `npm --workspace apps/server run typecheck` * PASS: `npm test -- terminalContainerRoutes terminalSessionManager` * PASS: `npm --workspace apps/web run typecheck` * PASS: `npm test -- orchestrationApi` Full validation checks: * PASS: `npm run typecheck --workspaces --if-present` * PASS: `npm run format` * PASS: `npm run format:check` * PASS: `npm run lint` * PASS: `npm test` (245 test files passed, 1 skipped; 2881 tests passed, 1 skipped) * PASS: `git diff --check` * PASS: `npm run security:secrets` (1822 tracked text files checked) * PASS: changed-file ASCII/LF scan before writing this report (47 files) * PASS: changed-file ASCII/LF scan after writing this report (48 files) ## Summary * Critical findings: 0 * High findings: 0 * Medium findings: 1 fixed * Low findings: 2 fixed * Remaining blockers: none --- # Agent Instructions This documentation is published with GitBook. GitBook is the documentation platform designed so that both humans and AI agents can read, navigate, and reason over technical content effectively. Learn more at gitbook.com. ## Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter, and the optional `goal` query parameter: ``` GET https://faction-os.gitbook.io/faction-os-docs/.spec_system/archive/sessions/phase19-session12-terminal-and-containers/code-review.md?ask=&goal= ``` `ask` is the immediate question: it should be specific, self-contained, and written in natural language. `goal` is optional and describes the broader end goal you are ultimately trying to accomplish on behalf of the user. GitBook uses it to tailor the answer towards what is most useful for that goal. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.