> 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/phase20-session07-file-executor-integration/code-review.md).

# Code Review and Repair Report

**Session ID**: `phase20-session07-file-executor-integration` **Package**: null **Reviewed**: 2026-06-28 **Scope**: All uncommitted changes in the working tree **Result**: RESOLVED

## Review Surface

**Files reviewed** (all uncommitted changes):

* `.spec_system/state.json` - tracked-modified
* `.spec_system/specs/phase20-session07-file-executor-integration/implementation-notes.md` - untracked
* `.spec_system/specs/phase20-session07-file-executor-integration/spec.md` - untracked
* `.spec_system/specs/phase20-session07-file-executor-integration/tasks.md` - untracked
* `apps/server/README_server.md` - tracked-modified
* `apps/server/src/lib/guardedActionValidation.ts` - tracked-modified
* `apps/server/src/lib/orchestrationValidation.ts` - tracked-modified
* `apps/server/src/managers/executorRegistry.ts` - tracked-modified
* `apps/server/src/managers/fileMutationManager.ts` - tracked-modified
* `apps/server/src/managers/guardedActions.ts` - tracked-modified
* `apps/server/src/managers/planCampaignManager.ts` - tracked-modified
* `apps/server/src/managers/taskExecutionCoordinator.ts` - tracked-modified
* `apps/server/src/managers/taskQueue.ts` - tracked-modified
* `apps/server/src/routes/orchestration.ts` - tracked-modified
* `apps/server/src/server.ts` - tracked-modified
* `apps/server/tests/commandCenterRoutes.test.ts` - tracked-modified
* `apps/server/tests/executorRegistry.test.ts` - tracked-modified
* `apps/server/tests/guardedActions.test.ts` - tracked-modified
* `apps/server/tests/orchestration.test.ts` - tracked-modified
* `apps/server/tests/planCampaignManager.test.ts` - tracked-modified
* `apps/server/tests/taskExecutionCoordinator.test.ts` - tracked-modified
* `apps/web/README_web.md` - tracked-modified
* `apps/web/src/components/orchestration/CommandCenterPanes.tsx` - tracked-modified
* `apps/web/src/components/orchestration/ExecutorRunHistoryDrawer.tsx` - tracked-modified
* `apps/web/src/components/orchestration/FileGitWorkbench.tsx` - tracked-modified
* `apps/web/src/components/orchestration/OrchestrationShell.tsx` - tracked-modified
* `apps/web/src/components/orchestration/OrchestrationTabs.tsx` - tracked-modified
* `apps/web/src/components/orchestration/QueueWorkbench.tsx` - tracked-modified
* `apps/web/src/lib/commandCenterUi.ts` - tracked-modified
* `apps/web/src/lib/orchestrationApi.ts` - tracked-modified
* `apps/web/src/lib/orchestrationUi.ts` - tracked-modified
* `apps/web/tests/CommandCenterPanes.test.tsx` - tracked-modified
* `apps/web/tests/ExecutorFamiliesPanel.test.tsx` - tracked-modified
* `apps/web/tests/FileGitWorkbench.test.tsx` - tracked-modified
* `apps/web/tests/orchestrationApi.test.ts` - tracked-modified
* `apps/web/tests/orchestrationUi.test.ts` - tracked-modified
* `docs/api/README_api.md` - tracked-modified
* `packages/protocol/src/guardedActions.ts` - tracked-modified
* `packages/protocol/src/orchestrationCommandCenter.ts` - tracked-modified
* `packages/protocol/src/rest.ts` - tracked-modified
* `packages/protocol/src/taskQueue.ts` - tracked-modified
* `packages/protocol/tests/guardedActions.test.ts` - tracked-modified
* `packages/protocol/tests/orchestration.test.ts` - tracked-modified
* `packages/protocol/tests/orchestrationCommandCenter.test.ts` - tracked-modified
* `packages/protocol/tests/taskQueue.test.ts` - untracked
* `tests/e2e/orchestration-command-center.e2e.ts` - tracked-modified

**Inventory commands**: `git status`, `git diff HEAD`, `git diff --cached`, `git ls-files --others --exclude-standard`

## Findings by Severity

### Critical

* `apps/server/src/lib/guardedActionValidation.ts:95` and `apps/server/src/managers/executorRegistry.ts:509` - Guarded file actions trusted the outer action kind separately from `fileMutation.operation`, so a `file_read` approval could carry a delete/edit/move mutation and execute it through the manager-backed adapter. | Fix: schema validation now rejects mismatched file action kinds, and the registry fail-closes malformed stored proposals before mutation. Added route and registry regression tests in `apps/server/tests/guardedActions.test.ts` and `apps/server/tests/executorRegistry.test.ts`. | Status: FIXED

### High

* `apps/server/src/managers/taskQueue.ts:615`, `apps/server/src/managers/taskExecutionCoordinator.ts:852`, and `apps/server/src/managers/fileMutationManager.ts:825` - File path display labels were copied into public queue/execution metadata without server-side redaction, allowing token-like repo-relative paths such as `token=secret-token.txt` to leak through REST responses and WebSocket frames. | Fix: all public file path labels now pass through `redactSensitiveText` while raw filesystem paths remain private to manager execution. Added route coverage proving REST, WebSocket, and execution-list output redacts token-like paths and never returns file contents. | Status: FIXED
* `apps/server/src/managers/taskExecutionCoordinator.ts:450`, `apps/server/src/routes/orchestration.ts:178`, and `apps/web/src/lib/orchestrationApi.ts:1333` - File rollback accepted `expectedRevision` in the web type but dropped it before the coordinator, so stale queue rows could roll back an old backup after the entry had advanced. | Fix: rollback request parsing, web serialization, route forwarding, and coordinator checks now preserve and enforce `expectedRevision`. Added stale rollback tests at API, route, and coordinator layers. | Status: FIXED

### Medium

* `apps/server/src/managers/taskExecutionCoordinator.ts:386` - Queue-dispatched file mutations asked `FileMutationManager` to save a standalone execution, then the coordinator saved the queue-targeted execution, producing duplicate command-center history for one queue action. | Fix: `FileMutationManager` now supports `recordExecution: false`; queue dispatch and rollback suppress manager-owned standalone records while direct guarded/manager file execution still records normally. Added coordinator assertions for exactly one dispatch run and one rollback run. | Status: FIXED
* `apps/web/src/components/orchestration/FileGitWorkbench.tsx:211` - File queue draft creation allowed move/rename without a destination and reused operation/path-only idempotency keys for different drafts on the same file. | Fix: move/rename drafts now require a destination, and file mutation idempotency keys include a compact draft hash so distinct write/edit/move drafts do not collide. Added File/Git workbench regression tests. | Status: FIXED

### Low

* `apps/web/src/lib/commandCenterUi.ts:2587` - Queue dispatch labels used the `retryable` capability flag alone, so a newly queued executable file task could display `Retry file` before any failed or unavailable execution existed. | Fix: retry labels now require both `retryable: true` and a blocked/failed/unavailable state. Added UI helper coverage proving queued file work still displays `Dispatch file`. | Status: FIXED

## Assumptions and Deliberate Non-Fixes

None. Each finding was repo-fixable and resolved within the uncommitted scope.

## Behavior Changes

* Guarded file proposals with a `fileMutation.operation` that does not match the guarded action kind now fail validation or execution instead of mutating files.
* Public file path labels in file queue and execution metadata are redacted consistently for token-like and path-like values.
* File rollback now honors stale revision protection.
* Queue-owned file dispatch and rollback now produce only queue-owned execution history entries.
* File workbench move/rename queue creation is disabled until a destination is provided; distinct file drafts get distinct idempotency keys.
* Queued executable work now uses retry labels only after retryable failure/unavailable states.

## Verification

* Analyzer: `if [ -d .spec_system/scripts ]; then bash .spec_system/scripts/analyze-project.sh --json; else bash /home/aiwithapex/.codex/plugins/cache/apex-spec-system/apex-spec/2.1.3-codex/scripts/analyze-project.sh --json; fi` - PASS - current session resolved as `phase20-session07-file-executor-integration`.
* Focused tests: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npx vitest run apps/server/tests/guardedActions.test.ts apps/server/tests/executorRegistry.test.ts apps/server/tests/orchestration.test.ts apps/server/tests/taskExecutionCoordinator.test.ts apps/server/tests/fileMutationManager.test.ts apps/web/tests/FileGitWorkbench.test.tsx apps/web/tests/orchestrationUi.test.ts apps/web/tests/commandCenterUi.test.ts apps/web/tests/CommandCenterPanes.test.tsx` - PASS - 9 files, 81 tests.
* Focused rollback/UI/API tests: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npx vitest run apps/server/tests/orchestration.test.ts apps/server/tests/taskExecutionCoordinator.test.ts apps/web/tests/orchestrationApi.test.ts apps/web/tests/FileGitWorkbench.test.tsx apps/web/tests/orchestrationUi.test.ts apps/web/tests/commandCenterUi.test.ts apps/web/tests/CommandCenterPanes.test.tsx` - PASS - 7 files, 95 tests.
* Focused server tests after duplicate-record fix: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npx vitest run apps/server/tests/taskExecutionCoordinator.test.ts apps/server/tests/fileMutationManager.test.ts apps/server/tests/executorRegistry.test.ts apps/server/tests/guardedActions.test.ts apps/server/tests/orchestration.test.ts` - PASS - 5 files, 46 tests.
* Formatter: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm run format:check` - PASS.
* Type checker: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm --workspace packages/protocol run typecheck` - PASS.
* Type checker: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm --workspace apps/server run typecheck` - PASS.
* Type checker: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm --workspace apps/web run typecheck` - PASS.
* Linter: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm run lint` - PASS.
* Aggregate type checker: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm run typecheck --workspaces --if-present` - PASS.
* Tests: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm test` - PASS - 3059 passed, 1 skipped.
* Browser e2e: `PATH="$HOME/.nvm/versions/node/v26.2.0/bin:$PATH" npm run test:e2e -- --project=app-desktop tests/e2e/orchestration-command-center.e2e.ts` - PASS - 5 passed.
* Whitespace: `git diff --check` - PASS.
* Final diff re-read: no remaining issues found.

## Summary

1. Reviewed all uncommitted changes: 46 files across protocol, server, web, docs, e2e, and session artifacts.
2. Resolved 1 critical, 2 high, 2 medium, and 1 low findings with targeted code fixes and regression tests.
3. No deliberate non-fixes remain.
4. Formatting, lint, typechecking, full Vitest, focused Playwright e2e, and whitespace checks all pass.


---

# 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/phase20-session07-file-executor-integration/code-review.md?ask=<question>&goal=<endgoal>
```

`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.